feat(container-runner): drain engine sleeps unless the idle timer fired

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/actors

• #5594 feat(container-runner): drain engine sleeps unless the idle … 👈
• #5595 feat(container-runner): jitter idle timeout to avoid teardow …
• #5585 feat(container-runner): self-sleep on repeated actor start
• #5586 feat(container-runner): sleep on startup idle timeout
• #5589 chore(container-runner): tighten comments
• #5587 feat(container-runner): exit process when the last child sto …
• #5588 feat(container-runner): drain child before SIGTERM on engine …
• #5584 feat(container-runner): drain children and engine concurrent …
• #5565 fix(envoy-client): ack terminating stop commands so pegboard …
• main

Get stack: forklift get 5594
Push local edits: forklift submit
Merge when ready: forklift merge 5594

_{change ktlrlsuw}

[claude]

Review

Small, focused change to container-runner/src/actor.rs: it fixes the prior bug where on_sleep skipped the drain for any sleep as long as idle-timeout mode was configured (dashboard/crash-policy/eviction sleeps included), not just idle-timer-triggered sleeps. Adding IDLE_SLEEPING as an explicit third state and gating the drain-skip on that specific state is the right fix, and the updated doc comments accurately describe the new behavior.

Correctness

• The CAS-based state transition is a genuine improvement over the old plain load()-then-sleep() check — the old code had a TOCTOU gap between reading idle_state and calling ctx.sleep(); the new compare_exchange(IDLE_ARMED, IDLE_SLEEPING, ...) makes the "did a request race the timer" decision atomic.
• Residual race worth being aware of (not introduced by this PR, but not fully closed either): if a request arrives after the idle timer's CAS has already flipped ARMED → SLEEPING and called ctx.sleep(), but before the engine actually delivers on_sleep, note_request's CAS (ARMED → REQUESTED) will fail silently and the state stays SLEEPING. When on_sleep eventually fires, it will still skip the drain even though a request came in during that window. This is a narrow timing window and the one-shot atomic can't represent "committed to sleep, but now there's also a pending request," so there's no cheap fix within this design — just flagging it as a known edge case rather than a blocker, since it's strictly better than the previous behavior (which skipped the drain unconditionally).
• Ordering::SeqCst is used for both CAS's, which is safe/conservative for a rarely-hit lifecycle transition; no performance concern here.

Test coverage

• No unit/integration test exercises the new IDLE_SLEEPING transition or on_sleep's branch on it (container-runner/tests/ only has boot_id.rs and input.rs). Given this is a state-machine correctness fix for a real production bug (dropping in-flight drains), a test that arms the idle timer, lets it fire, and asserts on_sleep skips the drain — plus a companion test asserting a non-idle sleep still drains — would give good regression coverage. Understand container-runner may not have much existing test scaffolding for actor lifecycle, so this may be consistent with the surrounding code, but worth considering.

Style

• Comments follow the repo's "complete sentences, no dash fragments" convention. No _ => fallthrough issues (no enum match involved). Doc comments were updated in sync with the code, which is appreciated.

Overall this is a solid, minimal bug fix. No blocking issues found; the residual race and missing test coverage are the two things worth a second look before merging.