feat(frontend): own the engine in a worker thread with a bounded queue

[xiaoyu-xyz]

Stacked on #34. This branch is built on the in-process engine contract and HTTP service, which is not merged yet, so the diff below still contains that PR's files (engine.rs, engine_service.rs). Only the eight files listed here are this change. Once #34 lands I will rebase and reopen against main.

Purpose

Stacked on #. That PR added the Engine contract and engine::app; this one makes them usable, and is where #3's acceptance criteria for persistent serving actually live: readiness that follows loading and warmup, a bounded queue, a request budget, and a shutdown that drains accepted work.

• src/frontend/src/worker.rs (450 lines) — the thread that owns the engine, the bounded queue, readiness, the request budget and drain.
• src/frontend/src/main.rs (+107) — mode selection and shutdown wiring. The forwarding path is unchanged.
• src/frontend/src/passthrough.rs (20 lines) — a stand-in engine that echoes requests, so the whole path can be run and tested with no model and no GPU.
• src/frontend/tests/worker_lifecycle.rs (449 lines) and the binary test added to engine_service.rs.

Model code is deliberately absent. A model supplies load, warmup and one request function; everything below comes from the transport.

How the lifecycle is defined

Event	Behaviour
Before load/warmup finishes	/health is 503 starting; inference is refused without touching the engine.
Ready	/health is 200 ok with queue depth and rejected.
Queue full	503 immediately. The queue refuses; it does not grow.
Budget expired (upload, queueing or inference)	503/504. The budget starts at the request headers, so a slow upload is charged to the caller.
Client gone before the worker dequeued it	Dropped without reaching the engine. No GPU work for an answer nobody will read.
Engine failed an accepted request	500, the engine is retired, /health becomes 503 failed with the reason, and the process keeps running so the failure is readable. A malformed request retires nothing.
SIGTERM / SIGINT	Admission closes at the signal, responses in flight get the same budget to finish, accepted work drains within it, then the process exits.
Drain budget exhausted	The process exits and says so. A wedged kernel is not interruptible and the code does not pretend otherwise.

spawn returns only after loading and warmup, so a caller holding a handle cannot observe a half-loaded engine, and the first request after /health is ready is not the one that pays for compilation.

Deliberate differences from #16

#16 already prototyped an in-process path (engine.rs, serve.rs) and was closed. This was written against the RFC text rather than derived from it, and the differences are the reason it is worth landing:

1. /health is observable. Add native Laya inference with Rust and CUDA #16's ready() is a bool, so an operator cannot tell a full queue from a dead engine. Here depth and a cumulative rejection count are published, x-queue-depth makes backpressure visible before it becomes a refusal, and a retired engine reports why without the server log.
2. SIGTERM actually drains. Add native Laya inference with Rust and CUDA #16's main returns on the signal without joining the engine thread, while its README claims to drain accepted work. Here shutdown takes the queue sender under the admission lock, waits out the drain budget, and reports whether it finished. That is the case the doc comment at the top of Add native Laya inference with Rust and CUDA #16's engine.rs warns about: GPU allocations outliving the process.
3. A total order between admission and shutdown. Without one lock spanning the readiness check and the send, a request can pass the check and then enqueue after the queue closed — accepted, with no worker left to answer it. submit holds that lock across both, and a test covers a drained worker refusing further work.
4. Failure isolation is stated, not implied. A panic in model code becomes a 500 for that caller and retires the engine instead of unwinding the worker thread; bad input does not retire anything.
5. Model code is one function, not its own worker loop. The queue, readiness and drain are written once, which is what [RFC] Support emerging System 1 models, with CLM as the second model after LAYA #9's second model needs.

Review follow-up

Both findings from the review are fixed, and each has a regression test that fails against
the code that was reviewed:

• [P2] Queue depth is now reserved before the job is published (worker.rs). The slot is
  taken before try_send and rolled back if the send fails, because the worker does not take
  the admission lock: the moment a job is in the queue it can be dequeued, answered and
  released, so counting afterwards could release a slot that had not been taken. Both
  rollback paths (Full, Closed) restore the count.
  depth_is_reserved_before_the_job_is_published submits 20,000 requests against a worker
  that is still draining and asserts that the depth never exceeds the number of requests the
  caller has accepted. Against the reviewed commit that assertion fires within the first
  hundred submissions (observed depth 26 with 27 accepted); against the fix it holds.
  The test asserts the one direction that is a hard invariant — a slot cannot be counted that
  was never taken — rather than equality, because a job may legitimately complete between the
  send and the read. I could not make the specific underflow branch deterministic: the
  window is a few instructions wide and forcing it would mean injecting a delay into submit,
  which is worse than the thing it tests.
• [P2] The drain budget now starts when shutdown is signalled (main.rs, worker.rs).
  Admission closes inside the shutdown future itself, and one absolute deadline — computed
  from the signal — bounds every step: the graceful listener shutdown, and the drain.
  Running::run_until_drained takes that deadline instead of measuring a fresh budget of its
  own.
  shutdown_is_bounded_by_the_drain_budget_not_the_request_budget opens a request whose body
  never arrives and sends SIGTERM. Against the reviewed commit the process took 3.009 s
  to exit with a 200 ms drain budget and a 3000 ms request budget — the request budget, not
  the drain budget, was bounding the shutdown. Against the fix it exits in well under a
  second.

Not in this PR

• CUDA Graph capture, RoPE, weights, the encoder and the scorer. The model half.
• Metrics beyond queue depth. /metrics is a separate decision about an exposition format; Engine::report is the extension point it would use.
• TLS, auth and ingress limits. Unchanged from the forwarding mode.

Test Plan

cargo fmt --all --check
cargo clippy --workspace --locked --all-targets -- -D warnings
cargo test --workspace --locked

tests/worker_lifecycle.rs covers readiness following warmup, startup failure, queue refusal with depth accounting, cancelled and expired requests never reaching the engine, an inference failure retiring the worker, panic isolation, drain reporting that it cannot finish, a drained worker refusing work, a zero queue capacity rejected at startup, and the depth reservation above.

Those tests synchronise on a gate the fake inference holds and a handshake signalling that it has started, so they assert on queue state rather than sleeping and hoping.

Test Result

System1-Omni Version / Commit: 3062243 (main) as base; head 1b63ed7, stacked on the in-process engine contract PR.

cargo fmt --all --check: clean. cargo clippy --workspace --locked --all-targets -- -D warnings: clean. cargo test --workspace --locked: 34 passed, 0 failed, run six times without a flake.

Verified against the built binary, not only in-process:

$ OMNI_SYSTEMONE_ENGINE=passthrough OMNI_JEV_BIND=127.0.0.1:18096 ./target/release/omni-jev
omni-jev listening on 127.0.0.1:18096 (engine passthrough, queue 32)

$ curl -s localhost:18096/health   → {"status":"ok","depth":0,"rejected":0}
$ curl -si -X POST --data '{"a":1}' localhost:18096/v1/systemone
                                   → 200, x-queue-depth: 1, body {"a":1}
$ kill -TERM <pid>                 → exit 0, within the drain budget

No GPU and no model weights were needed for any of it. Cargo.lock is unchanged.

Related: #14 (this is its HTTP integration step), #16 (closed reference implementation), #3, #9, #1.

[hsliuustc0106]

Reviewed commit 1b63ed7431ea4107852600d07799a826773e3bf4.

• [P2] Reserve queue depth before publishing the job. src/frontend/src/worker.rs:217 increments depth after try_send, but the consumer never takes the admission mutex. It can dequeue/process/decrement first, yielding depth=0 in a response and transient usize underflow in health. A standalone CPU probe against the built crate observed four zero-depth answers in 100,000 sequential immediate requests. Reserve before sending and roll back on send failure, or synchronize dequeue/accounting.
• [P2] Start the drain budget when shutdown is signaled. src/frontend/src/main.rs:68 awaits graceful HTTP shutdown before stop_accepting/run_until_drained. In-flight upload/response handlers can consume their entire request budget before the drain timer even begins, and uploads can still submit during this interval. A real binary with DRAIN_MS=50 and TIMEOUT_MS=3000 took 2.917 seconds to exit after SIGTERM with one partial upload. Stop admission on the signal and bound the complete shutdown sequence with the drain deadline.
  All 31 ordinary tests plus 1 doctest passed; these probes expose gaps in the existing suite.

[xiaoyu-xyz]

All three are fixed in a9f7fb83, rebased onto main (the merge conflict is resolved; it was the README sentence both sides had rewritten). Each fix has a test that fails against the code you reviewed.

[P1] The budget no longer starts at startup. You were right, and it was worse than the finding says: arming the timer around the whole serve future meant a healthy process retired once the budget elapsed with no signal sent, so the default was a 10-second service lifetime. One serve call now runs for the life of the process; its shutdown future waits for the signal, closes admission, and starts the budget there. the_service_stays_up_beyond_the_drain_budget_without_a_signal serves past five times the budget and then answers a request; against 3219451 it fails with the process exited on its own 200 after starting.

Two related things the fix had to get right, both of which I got wrong first: draining the worker and draining the transport now run concurrently (sequentially, an idle engine paid the full budget on every shutdown), and an unknown engine name is a startup error rather than a silent fall back to forwarding — that is [P2] below.

[P2] The limit covers outstanding requests, running or queued. mpsc::try_send only bounds the channel's buffer, and the worker frees its slot the moment it dequeues, so a capacity-1 engine served two requests at once. Admission now takes the slot and checks it under the admission lock, before the handover; both failed-send paths roll it back. a_running_request_counts_against_capacity holds the first inference behind a gate so the channel is provably empty, then asserts the second is refused; against 3219451 it fails with a second request was accepted while one was already running.

[P3] An unknown engine name is a configuration error. OMNI_SYSTEMONE_ENGINE is matched explicitly now; unset and empty still forward, anything else is a startup error. an_unknown_engine_name_is_a_startup_error runs the binary with typo-engine and asserts a non-zero exit naming the value; against 3219451 it fails because the process starts a forwarding server instead.

A correction to my previous comment

I claimed the depth test "fires within the first hundred submissions" against 1b63ed7. That was wrong, and I should not have written it. When I re-checked the test I had by then, it passed against the buggy ordering — the observation I quoted came from an earlier version of the test, before I changed the fake to spin, and I reported it as if it still held.

What is true now: the depth fix itself is in place, and queue_depth_returns_to_zero_after_the_storm covers the consequence — a counter released before it was reserved stays wrapped and never drains — but it does not reproduce your interleaving, and it passes against the released-before-reserved ordering too. The test's own doc comment says so. I could not make that window deterministic here: it is a few instructions wide, and forcing it would mean injecting a delay into submit. The ordering is covered by your probe and by inspection, not by my test.

Verified on a clean checkout of a9f7fb83, with the changed files hashed against the pushed branch to confirm they are identical: cargo fmt --all --check clean, cargo clippy --workspace --locked --all-targets -- -D warnings clean, cargo test --workspace --locked 34 passed across repeated runs, release build passes.

[Levius-Fubuki]

Reviewed 3219451e57ab7ad96fc92ae47d4636fe404f9c7b, including the incremental changes on #34 and the two previous findings.

The depth reservation now precedes publication and rolls back on both failed-send paths, addressing the original counter-underflow mechanism. However, the shutdown follow-up introduces a reproducible service-lifetime regression. Changes are requested for the three issues below.

Formatting passed. Clippy, workspace tests and release build passed on Linux/Rust 1.98.1 after rebuilding in a dedicated target directory (an initial shared-target run had stale cross-checkout artifacts and is not treated as a PR failure). Independent probes reproduce the no-signal exit and invalid-mode fallback on Linux and macOS; a gated CPU worker reproduces the capacity breach. No GPU is needed for these cases. This remains a draft and also needs its existing merge conflicts resolved.

[Levius-Fubuki]

[P1] Start the drain deadline on the shutdown signal, not at startup

This deadline is created as soon as the listener starts and wraps the entire engine::serve future. Consequently every native process exits after the drain duration even if no SIGTERM/SIGINT is sent (10 seconds by default). With OMNI_SYSTEMONE_ENGINE=passthrough, OMNI_SYSTEMONE_DRAIN_MS=200 and OMNI_JEV_BIND=127.0.0.1:0, the release binary exited 0 after 0.206 seconds on Linux, logging shutdown exceeded its drain budget; the probe sent no signal. Wait for the signal while serving normally, then close admission and compute one absolute deadline for graceful HTTP shutdown plus worker drain. Add a test that remains healthy beyond the drain duration before sending any signal.

[Levius-Fubuki]

[P2] Enforce the documented limit on all outstanding requests

This increment reserves accounting but never checks the configured outstanding-request limit; only mpsc::try_send enforces capacity. Once the worker dequeues a job, the channel slot is free even while inference still runs. A deterministic probe with capacity 1 holds the first inference behind a gate, waits until it has started, then submits a second job: it is accepted and report().depth becomes 2. Both Options::queue_capacity and the README promise refusal once that many accepted requests are unfinished. Check/reserve the outstanding count under the admission lock (with rollback on send failure), and cover a running-plus-queued case instead of relying on whether the consumer has happened to dequeue.

[Levius-Fubuki]

[P2] Reject unsupported engine names instead of silently selecting forwarding

Every value other than passthrough enters forwarding mode here. With OMNI_SYSTEMONE_ENGINE=typo-engine, the built binary starts and logs forwarding to http://127.0.0.1:8000/, although the README says unknown modes are startup errors. A configuration typo can therefore route inference to a different backend instead of failing visibly. Match supported values explicitly, preserve the documented unset/default behavior, and return a configuration error for unknown values.