feat(container-runner): log unexpected platform SIGTERM as an error

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/rivet

• #5548 feat(container-runner): log version and git sha at actor sta …
• #5549 feat(container-runner): setup instance memory and cpu usage …
• #5546 fix(rivetkit): keep actor event loop alive through shutdown …
• #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 5536
Push local edits: forklift submit
Merge when ready: forklift merge 5536

_{change zsrurkrz}

[claude]

Review

Small, focused change: sets PLATFORM_RECLAIM and upgrades the SIGTERM log line to error level to flag an unexpected reclaim. Re-verified against the current diff (head 4346f552, unchanged since the last pass). Overall reasonable.

Findings

1. PLATFORM_RECLAIM has no reader within this PR alone. It's written at main.rs:426 but nothing in this diff loads it. It is consumed by the stacked follow-up PR feat(container-runner): report actors as crashed on unexpected platform SIGTERM #5539 ("report actors as crashed on unexpected platform SIGTERM"), which gates crash_all_actors(...) on PLATFORM_RECLAIM.load(Ordering::Acquire). So this isn't dead code once the stack lands as-is, flagging only as a reminder in case the PRs land out of order or feat(container-runner): report actors as crashed on unexpected platform SIGTERM #5539's shape changes before merge, since in isolation this PR only adds a flag that's set and never read.

Correctness

• Ordering::Release store to PLATFORM_RECLAIM followed by the Ordering::Release store to SIGNAL_SHUTDOWN in the same task, paired with Ordering::Acquire loads on SIGNAL_SHUTDOWN downstream, correctly establishes happens-before for PLATFORM_RECLAIM too (the release sequence covers the prior same-thread write), so there's no ordering bug.
• The doc comment on PLATFORM_RECLAIM (main.rs:78-81) previews the downstream shutdown handling that lands in feat(container-runner): report actors as crashed on unexpected platform SIGTERM #5539, and matches what that PR actually does.
• Worth a second look: the file's own module doc (main.rs:12-17, predates this PR) describes "the platform sends a SIGTERM" as one of two normal reap paths for a warm, idle instance, alongside the engine draining /start once the request lifespan elapses. This PR's new log line instead frames any platform SIGTERM as unconditionally "unexpected," including the actor_ids.is_empty() branch where no actors are even running. That's likely intentional (the follow-up PR treats it as severe enough to crash every actor on the instance), but it reads as inconsistent with the older doc comment one screen up. If a raw SIGTERM is only expected to arrive for anomalies now that the engine drain path is supposed to handle graceful reclaims first, it would help to update main.rs:12-17 to say so explicitly so the two comments do not contradict the next reader.

Minor

• The log message hardcodes "likely hitting OOM or running longer than 60 minutes," which is Cloud Run-specific framing baked into a generic container-runner binary. If this runner is meant to run on other platforms eventually, consider making the wording more generic or platform-configurable. Minor nitpick, and the file already assumes Cloud Run defaults elsewhere (the 10s SIGTERM budget).
• CHILDREN.retain_async(|actor_id, _| { actor_ids.push(actor_id.clone()); true }) is used purely to collect keys (the predicate always returns true, so nothing is removed). If scc::HashMap exposes a read-only scan (e.g. scan_async), that would express the intent more directly than a mutating retain_async call that happens to never mutate.

Test coverage

No tests added, consistent with the rest of the signal-handling code in this file (no existing test harness for the signal path). Since this is an observability-only change (a log line plus a flag consumed by a later PR), that's acceptable.