feat(container-runner): report actors as crashed on unexpected platform SIGTERM

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/rivet

• #5539 feat(container-runner): report actors as crashed on unexpect … 👈
• #5536 feat(container-runner): log unexpected platform SIGTERM as a …
• #5530 fix(rivetkit-core): cancel driver alarm before sqlite teardo …
• #5531 feat(container-runner): keep instance warm instead of self-e …
• #5533 chore(container-runner): default stop grace to 10s
• #5528 fix(rivetkit): log and surface actor startup failures
• #5529 fix(rivetkit): treat empty actor input bytes as absent
• main

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

_{change lzuymmyk}

[claude]

Review

Solid, well-scoped change: registering live actor Ctxs so a platform-driven shutdown can proactively report them as crashed (StopCode::Error) instead of relying on the engine's Lost detection after a heartbeat timeout. The concurrency handling between crash_all_actors and the normal on_destroy/on_sleep teardown path is sound, since request_stop's destroy_requested swap makes a racing double-call harmless (logged at debug, not a functional issue).

One correctness issue I'd like addressed before merge, plus a couple of minor notes.

ACTOR_CTXS entries leak on a failed on_start

container-runner/src/actor.rs:87-92 registers the actor into ACTOR_CTXS near the top of on_start, before the child is spawned. But the only place an entry is ever removed is stop_child (invoked from on_destroy/on_sleep) or crash_all_actors at process shutdown.

If on_start fails after registration, e.g. the parts.is_empty() bail-out (actor.rs:119-123) or ChildProcess::spawn failing (actor.rs:143-153), on_destroy/on_sleep are never called for that generation. Looking at run_actor in rivetkit-rust/packages/rivetkit/src/start.rs:224-241, a startup error short-circuits straight to return Err(error) without invoking any teardown hook. So a failed start leaves a permanent stale entry in the static ACTOR_CTXS map for the remaining lifetime of the process.

Given the doc comment on request_exit says "the container stays warm and ready for the next placement," instances are now long-lived and can serve many sequential actor placements, so this becomes unbounded growth on any workload with a flaky child command, bad input, or repeated readiness-timeout failures (each retry is a new generation/actor id occupying a new map slot forever). It's a small per-entry cost (one Arc-cheap Ctx clone), but it's still an unbounded leak over the container's lifetime, which cuts against the repo's "no unbounded leaks" stance in CLAUDE.md.

Suggest only inserting into ACTOR_CTXS once on_start is about to return Ok (e.g. right before *self.child.lock().await = Some(child); Ok(()), and in the early "already running" branch), rather than up front, or wrap the early-return failure paths with cleanup that removes the just-inserted entry.

Crash message is misleading for non-signal-specific shutdowns

spawn_signal_handler (main.rs:419-431) sets SIGNAL_SHUTDOWN and drives EXIT on either SIGTERM or SIGINT, and (per the doc comment on request_exit, main.rs:169-173) this signal path is now the only way the process exits. That means crash_all_actors fires, and reports every still-live actor to the engine with StopCode::Error and the message "runner received unexpected platform SIGTERM, likely OOM or running longer than 60 minutes", even on a plain local Ctrl-C (SIGINT) during development or manual testing.

This doesn't change the engine's actual scheduling decision (both Error and the pre-existing Lost fallback map to the same Decision::Sleep branch in engine/packages/pegboard/src/workflows/actor2/runtime.rs:649-656), so it's not a functional regression, but the fixed diagnostic message will misattribute a deliberate SIGINT/manual stop as "likely OOM," which could send someone debugging a real incident down the wrong path. Worth either branching the message on which signal fired, or softening the wording so it doesn't assert a specific cause when it can't actually distinguish OOM/timeout from a deliberate external stop.

Test coverage

No tests accompany the new ACTOR_CTXS registration/crash-report logic. This crate's existing coverage is mostly e2e (examples/e2e-test) and CLAUDE.md's testing policy discourages mocking, so a full integration test may be impractical here, but at minimum the leak scenario above (on_start failure leaving a stale ACTOR_CTXS entry) seems like something a fixture-level test could catch cheaply if this module grows unit-test infra.

Nits

• Style/comment conventions (structured logging, comment phrasing, scc::HashMap usage for concurrent maps) all look consistent with CLAUDE.md.
• The remove-then-insert pattern in on_start (ACTOR_CTXS.remove_async then insert_async) is fine given the single-writer-per-actor-id invariant; just flagging it's not atomic in case that invariant ever loosens.