[ac-pplx]

Stacked on #413 (on #418, #438 and main). Base: ac/sbxmgr-sw-pr3-wake-resume @ 38e424c. This PR is one commit on top of it. Stack, bottom to top: #438 → #418 → #413 → #416 → #421 → #417 → #414. #372 and #388 have merged to main.

This PR changes only sbx-manager's node-local SQLite sandboxes.state. It does not touch the control-plane Postgres public.sandboxes.lifecycle.

What merging does

It converts a periodic background generic writer, and the startup-recovery writers, from update_state to update_state_from with explicit from-sets. This is the plan's §5 PR 5, background/startup part (finding 7).

Call sites converted

Call site	Before	After (edge, from → to)
service/reconcile.rs run_due_auto_lifecycle, Stop of a suspended row (now stop_suspended_for_auto_lifecycle)	update_state(Stopped) through require_state_write	update_state_from(&edges::AUTO_LIFECYCLE_STOP_SUSPENDED), {suspended} → stopped. Refusals map through #438's state_write_result / StateWriteRejected::into_status
service/recovery.rs recover_interrupted_create	update_state(Created)?.require_applied()?	require_startup_state_write(&edges::INTERRUPTED_CREATE_RECOVERY), {creating} → created
service/recovery.rs recover_unlaunched_start	update_state(Stopped | Created)?.require_applied()?	require_startup_state_write(&edges::UNLAUNCHED_START_RECOVERY_STOPPED | _CREATED), {starting} → stopped / created. The recorded outcome is edge.to()
db/startup_recovery.rs recover_interrupted_startup_states, the restoring branch	update_state(Suspended), skip unless applied	update_state_from(&edges::INTERRUPTED_RESTORE_RECOVERY), {restoring} → suspended, skip unless applied

require_startup_state_write (new, in db/startup_recovery.rs) wraps update_state_from. Any outcome but Applied becomes the new RecoverStartupStatesError::StateWriteNotApplied { edge, outcome }.

Startup writers: behavior unchanged

All three startup writers run inside recover_interrupted_startup_states (run.rs ~884):

• Sockets and background loops start later. The sbx-manager and host-control sockets bind at ~1013–1020. The background loops (auto-lifecycle scheduler, eviction, the reconcilers) start at ~996–1008.
• Construction-spawned workers. Service construction (new_with_config) already spawns the snapshot scheduler and watchdog workers. They cannot race these writes, because they act only on sandboxes registered with them. recover_runtime_registrations (run.rs ~964) registers sandboxes with them only after these writes. The require_startup_state_write doc comment records this invariant.

So no RPC or worker can move a row between recovery's read and its write. Every row each writer reaches is already in its edge's from-set, and it applies as before. The existing pins listed below are green.

Auto-lifecycle Stop: the race fixed (plan finding 7)

Before. The Stop of a suspended row re-read the row under one DB mutex guard and wrote under a later one. It held only the wake lock. Stop admission, Delete admission and eviction do not take that lock (they use the cleanup fence and staging locks). With no from-set, the write overwrote whatever the generic guards admitted. A wildcard-free sweep against #388's code (moved to X, the row moved after the re-read) gave:

• creating, created, starting, started, stopping, stopped, pausing, paused, resuming, checkpointing, suspending, restoring, cleanup_required, cleanup_failed and failed were overwritten with stopped, and the Stop stage was recorded as fired.
• deleting (whose state owns delete_row) and quarantined returned Aborted, with superseded_by_cleanup_episode or blocked_by_quarantine. That errored the transition, and the loop persisted it as last_auto_error.

The concrete production interleaving is a Stop admitted between the re-read and the write. Stop admits a suspended row only while its runtime is live: in the window between Stop's fc.pid gate and its cleanup fence, or with a leaked runtime. The overwrite turned Stop's stopping into stopped. Stop's own completion (complete_row_intent_cleanup_for_job(MarkStopped)) then no longer owned the row, so it changed nothing and left the stopped runtime's fields (pid, uid, route) on a stopped row.

After.

• Skip. A refusal whose found is outside the edge's from-set skips the transition. FromState or Guard, matched by !edge.from().contains(found). This predicate deliberately differs from the other PRs': here any refusal of a row that left suspended means "skip", including a guard refusal of a deleting or quarantined row.
• What a skip does. It writes an info log and returns Ok(()) before mark_transition. Nothing is recorded, and no auto-lifecycle error is persisted. The next scan re-evaluates the row in its new state.
• Everything else goes through StateWriteRejected::into_status: a guard refusal of a row still suspended (only CleanupOwned can occur there), and a missing row. These arms are defensive and not pinned by a test. Their Aborted texts are unchanged.
• Database errors are Internal {context}: {error}, as before.

AUTO_LIFECYCLE_STOP_SUSPENDED stays Request, as #388 classified it. As #388 noted, it bypasses every RPC. Its allowlist reason already says that.

Red/green tests (service/reconcile/tests.rs). They use a service-level two-phase barrier between the re-read and the write: a #[cfg(test)] AutoLifecycleCoordinator::suspended_stop_barrier (tokio::sync::Barrier), following the admission_barrier precedent. The services use for_test_in(tempdir) with in-memory SQLite and bounded 30 s waits.

• auto_lifecycle_stop_writes_only_over_a_still_suspended_row: a wildcard-free sweep over every SandboxState. The row is moved with force_state at the barrier.

  • suspended is the accepting control: the row ends stopped and the stage is recorded (next_auto_event_at = None).
  • Every other state must return Ok and leave the whole database unchanged.
  • On sbx-mgr: add the persisted state-transition table and update_state_from (no callers) #388's code it fails at the first case (moved to creating: the row was overwritten with stopped). It passes with this change.

• stop_admitted_during_auto_lifecycle_stop_keeps_its_completion: a runtime-seeded suspended row. At the barrier, Stop admission's state write runs (update_state_superseding_activation_in_transaction(Stopping); a blocking Stop persists no intent). The test asserts:

  • the auto-lifecycle run returns Ok and leaves the whole database unchanged;
  • Stop's completion then runs, and the row ends stopped with its runtime cleared.

  The completion's return value is not asserted: without a job id it reports Applied even for a row it does not own. On sbx-mgr: add the persisted state-transition table and update_state_from (no callers) #388's code the test fails, because stopping was overwritten with stopped. It passes with this change.

Neither test asserts a message.

Every behavior change

• Auto-lifecycle Stop over a row that left suspended (the race only).

  • It no longer writes stopped and no longer records the Stop stage.
  • For deleting and quarantined rows it no longer returns Aborted or persists last_auto_error.
  • It now skips and writes an info log.

  This is client-visible through the sandbox's auto_lifecycle_status. For a raced row:

  • last_auto_error, last_auto_transition and next_auto_event_at, and the ECW end_at derived from them, now read as unchanged by the skipped attempt;
  • a raced deleting/quarantined row no longer gets last_auto_error;
  • a row that left suspended records no Stop transition until the next scan re-evaluates it: about 5 s later, or after the 60 s gated defer if its new state is mid-transition.

  Rows that do not race are unaffected.

• Client-visible status messages: none changed. This caller never produces the canonical FromState text: a from-state refusal of a row that left suspended skips. A from-state refusal of a row still suspended is impossible, because the writer returns StatementChangedRows(0) instead. The Guard and Missing texts equal the old ones.

• Startup recovery errors (not client-visible; startup fails and process supervision retries either way):

  • A non-applied interrupted-create or unlaunched-start write surfaced as StartupRecoveryError::StateWrite, displayed as sandbox state write was not applied: <outcome>. It now surfaces as StateRecovery(StateWriteNotApplied), displayed as startup recovery write <EDGE> was not applied: <outcome>. This outcome is unreachable at startup. The now-unused StartupRecoveryError::StateWrite variant is removed.
  • A database error from those two writes displayed as startup recovery database operation failed: … (StartupRecoveryError::Database). It now displays as startup state recovery database operation failed: … (StartupRecoveryError::StateRecovery(RecoverStartupStatesError::Database)).

• Startup interrupted-restore write: it still skips a non-applied write. It now logs a warn on that skip. update_state used to log only its guard refusals.

• Locks and transactions: caller mutex and lifecycle-lock scopes are unchanged. The helper adds its own IMMEDIATE transaction in autocommit, around its guard reads and the UPDATE; update_state ran a bare UPDATE for these non-deleting targets. That is harmless on the single connection. Side columns are unchanged.

• Double count (not fixed, noted). The auto_lifecycle_transition_lateness metric and the "running auto lifecycle transition" log fire before the Stop's write. A stage that is skipped by this refusal and later retried is counted, and logged, twice. Emitting them only after a successful write would touch the Pause, Suspend and Delete arms too, so this PR leaves it.

Edges, allowlist and #438 items

• No edges added or deleted. None of my call sites had a KnownRace edge, so this branch does not retire the KnownRace kind.

• The doc comments of the five converted edges now name the new writers. No reordering or reformatting.

• Allowlist unchanged. I re-read the reasons for my pairs:

  • suspended → stopped ("bypasses every RPC");
  • starting → created/stopped ("startup recovery restore the entry state");
  • restoring → suspended ("restore rollback, cross-node publication, restore recovery").

  sbx-mgr: create completion and warm start write only over the state they expect #418 (PR 4), below this PR, dropped its warm-start clause.

• sbx-mgr: one refusal-to-status mapping for update_state_from #438 dead_code expectations. sbx-mgr: create completion and warm start write only over the state they expect #418, below this PR, already dropped them from the items this PR's production code uses: StateWriteRejected (and its impl), StateWriteGuard and state_write_result. update_state_from_best_effort keeps its expectation until sbx-mgr: snapshot completion and rollback write only over the state they expect #421, above this PR, calls it.

• require_state_write_from. At sbx-mgr: one refusal-to-status mapping for update_state_from #438's final head it returns Result<(), Status>. This PR does not call it; it matches state_write_result(..) with Ok(_).

• Test-fixture visibility. db/tests/state_write_fixtures.rs changes seed_runtime from pub(super) to pub(crate), for reuse by the service tests; sbx-mgr: create completion and warm start write only over the state they expect #418, below this PR, already widened force_state.

Overlaps with the other PRs in the stack

• db/state_write.rs. This PR changes doc-comment lines only, on AUTO_LIFECYCLE_STOP_SUSPENDED, INTERRUPTED_CREATE_RECOVERY, UNLAUNCHED_START_RECOVERY_STOPPED/_CREATED and INTERRUPTED_RESTORE_RECOVERY. The other PRs in the stack delete KnownRace edges elsewhere in the table, so the hunks are disjoint.
• service/cleanup_support.rs. sbx-mgr: create completion and warm start write only over the state they expect #418, below this PR, drops the dead_code expectations this PR relies on, so this PR no longer touches the file.
• state_write_fixtures.rs. sbx-mgr: create completion and warm start write only over the state they expect #418, below this PR, widens force_state; this PR widens only seed_runtime.
• Last KnownRace edge. sbx-mgr: pause writes only over the state it expects #417, above this PR, deletes the last ones and retires the kind. This PR removes none.
• service/recovery.rs. It is listed in architecture-baseline.toml with a cap of 2017 code lines and was already at 2059 on sbx-mgr: add the persisted state-transition table and update_state_from (no callers) #388's head, a pre-existing advisory overrun. This PR takes it to 2058 and raises no cap. Keep sandbox-manager out of retained btrfs send sources #246 also touches this file. Separately, service/mod.rs is 1782 against its 1757 cap, also pre-existing; this PR does not touch it.

Mutation proof

The mutations ran one at a time on this head, each from a saved copy of the file, which was restored and touched before the next. The selection was the state-write table tests plus the auto-lifecycle, startup-recovery, interrupted and unlaunched tests.

#	Mutation	Failing tests
M1	Restore the generic update_state(Stopped) call	auto_lifecycle_stop_writes_only_over_a_still_suspended_row, stop_admitted_during_auto_lifecycle_stop_keeps_its_completion
M2	AUTO_LIFECYCLE_STOP_SUSPENDED from-set → StateSet::ALL	both red/green tests, plus no_edge_lists_its_own_target_as_a_source and the parity test
M3	Drop the skip predicate, so every refusal errors through into_status	both red/green tests
M4	A skipped refusal records the transition (Ok(true))	both red/green tests
M5	Interrupted create uses a {created} edge	startup_recovery_finishes_interrupted_create_by_creating_cache_root
M6	Unlaunched start with a rootfs uses the _CREATED edge	startup_recovery_rewinds_unlaunched_start_with_rootfs_to_stopped
M7	Interrupted restore uses a {suspending} edge	startup_recovery_reconciles_interrupted_states_without_runtime_fields

Pins (green)

• Auto-lifecycle: auto_lifecycle_scheduler_*, auto_lifecycle_error_*, and the auto-lifecycle tests in service/tests.rs and auto_lifecycle/tests.rs.
• Startup recovery:
  • startup_recovery_finishes_interrupted_create_by_creating_cache_root;
  • startup_recovery_rewinds_unlaunched_start_without_rootfs_to_created / _with_rootfs_to_stopped;
  • startup_recovery_reconciles_interrupted_states_without_runtime_fields and the other startup_* tests.
• sbx-mgr: characterize the state writers before the state-write helper (tests only) #372 / sbx-mgr: add the persisted state-transition table and update_state_from (no callers) #388 / sbx-mgr: one refusal-to-status mapping for update_state_from #438: the matrix, accepted-shape, update_state_from, table and cleanup_support tests are unchanged and green.

Decisions made without an operator

• Test barrier on AutoLifecycleCoordinator. I put it there, not on cleanup_executor, because it is the auto-lifecycle path's own state.
• No deferral on a skip. The skip does not call defer_gated_auto_lifecycle_row. The next scan should evaluate the changed row promptly, as the wake-superseded skip already does.
• The Stop admission test instead of a Delete one. The sweep against sbx-mgr: add the persisted state-transition table and update_state_from (no callers) #388 showed that the generic guard already refused a deleting row: it only errored the loop. So the named regression test uses Stop admission, the interleaving the old code actually overwrote.
• Helper location. require_startup_state_write lives in db/startup_recovery.rs, so service/recovery.rs does not grow.
• Review finding 7. Noted rather than fixed: it is the smaller diff.

Validation of the restacked stack

The stack was restacked linearly on main (b18051f) after #372 and #388 merged: #438 → #418 → #413 → #416 → #421 → #417 → #414. The per-PR validation further down ran on this PR's pre-restack head.

This PR's head (cf519ce): pull CI is green. fmt, workspace clippy --all-targets --all-features -D warnings and cargo arch (0 required findings) all pass. SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-sbx-mgr --retries 0 passed 1641/1641, with 0 dependency skips. Tested at aeafa72; the final head differs from it only in architecture-baseline.toml, which only cargo arch reads, and cargo arch was rerun on the final head. Kind Pull CI ran on #417, which contains this PR: all 4 Firecracker smoke shards passed.

The top of the stack (#414 @ 9a0a251, which contains this PR):

Check	Result
sbx-mgr nextest ×3	1650/1650 each, 0 dependency skips
Whole-workspace nextest	8985/9021 passed, and every API crate passed. The 36 failures are all outside sbx-mgr. 34 fail identically on main: spaced-guest and spaced-host-cli process tests (the test box can't switch identity) and one net-gateway test. The other 2 are common-grpc retry-deadline tests that pass on rerun on both heads.
The stack's 30 new or changed tests, ×30	900/900
cargo test -p pplx-space-sbx-mgr (shared process)	1650/1650
Mutation spot-check on the top head	#417's quarantine-miss mutation and #418's warm-start admission mutation are both caught
Kind, fresh cluster; the deployed sbx-mgr binary is from this head	smoke-kind-e2e.sh passed. smoke-kind-typed.sh all: native 23/23; ECW 76 passed, 2 infra errors. local.ecw_template_build_from_oci_boots passed, and so did the leader-election brownout.
The 2 ECW infra errors	node_group_boundary_schedules and node_selector_extras_render_and_enforce. Both scenarios restart the API Deployment, and the same failure family appears on main in an A/B on the same cluster. In the failed runs the kill never reached sbx-mgr.
Lifecycle race soak on Kind	11 concurrent operation pairs × 40 iterations, on the stack and on main on the same fresh cluster, checking that sbx-mgr's SQLite sandboxes.state, control-plane Postgres lifecycle and the VM agree. No change-induced violation: across 880 sandboxes, no quarantine, panic, restart or stuck state. Every violation reproduces on main with the same signature: an ECW pause that loses to Stop answers 503 instead of 409; after Suspend vs Delete, the node row can outlive the control-plane row until the next eviction pass (pass timing, not the build); and a refused quarantine write after Pause vs Delete is still logged. The soak's 0–200 ms API-level offsets did not reproduce the interleavings this stack closes, so the unit-level race tests remain the direct evidence for those.

Validation on the pre-restack head

Check	Result
cargo fmt --all -- --check, scripts/check-rust-error-policy.sh	pass (on 6098e51)
cargo clippy --workspace --locked --all-targets --all-features -- -D warnings	pass, 0 warnings (on 6098e51)
cargo arch	0 required findings; the same 39 advisory findings as before. service/recovery.rs is 2058 against its 2017 cap, a pre-existing overrun (2059 on #388's head)
pre-commit (Gitleaks) on the amended commit	pass
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run --locked -p pplx-space-sbx-mgr --retries 0	× 3 on dce4736 (onto #438 @ 7e2e41c), then × 1 on 6098e51 (onto #438 @ f1d1c9b): 1632/1632 each run; 0 dependency skips (skip logs empty). The 1 skipped test is the #[ignore] privileged transfer_directory::tests::distinct_service_uids_can_exchange_downloads, which is unrelated
Mutation proof (M1–M7)	all caught on dce4736, all restored
Public-safety review of the staged diff	no findings in the reviewed scope
Kind: fresh cluster on the current harness	no cluster existed before. smoke-kind-e2e.sh bootstrapped space-local
Kind: deployed binary is this build	on dce4736: space-rust-build-kind:local and the running space-sandbox-manager pod's space-sbxmgr-svc both have sha256 9706f804…. The binary contains this change's new strings, and pods report "version":"dce4736d5b73"
Kind: Tilt quiet	the bring-up waited for every build and rollout; uiresources were all ok / not_applicable
smoke-kind-e2e.sh	PASS
smoke-kind-typed.sh all, with the cold-receipts dir and admin token file as in CI	ECW 78 passed, 0 failed, 0 inconclusive; native 23 passed, 0 failed, 0 inconclusive. No rerun needed
Kind teardown	infra/space/kind/scripts/teardown.sh: Tilt stopped, cluster deleted
Kind after the final rebase	not rerun, as instructed: the f1d1c9b rebase changed only #438's require_state_write_from return type, which this PR does not call
Kind coverage limit	no smoke scenario drives an auto-lifecycle Stop of a suspended row. The converted branch is covered by the unit tests above
CI	not yet run on this head

🤖 Generated with Claude Code

[nikhilgupta-pplx]

Approve. The auto-lifecycle skip returns before mark_transition, releases the wake lock/lease, and the next scan defers transient states and settles on stopped without a hot loop. Startup recovery edges ({suspended}, {creating}, {starting}, {restoring}) match their callers, and the new refusal is unreachable for episode-less rows, so no new crash-loop path.

Non-blocking:

• StateWriteNotApplied renders {outcome:?} in an operator-facing startup error; format found/refusal explicitly.
• reconcile/tests.rs only exercises the guard-refusal skip via quarantined; force_state(Deleting) has no delete_row intent, so it goes through FromState. A deleting row with a current delete_row intent would pin the CleanupOwned skip.

[the-gigi-pplx]

Review — 4 findings

Low

1. crates/libs/sbx/mgr/server/src/service/reconcile.rs:548: the new doc comment says a row that leaves suspended makes the write refuse and the function return false. A row that's deleted outright doesn't take that path.
   A Delete that finishes between the re-read and the write gives StateWriteOutcome::Missing. That falls to Err(rejected) => Err(rejected.into_status(CONTEXT)) at line 583, so run_due_auto_lifecycle returns Aborted, and handle_auto_lifecycle_error logs a failed transition for a sandbox that was deleted on purpose. The behavior is the same as before this PR (update_state on a missing row was Aborted too), and a deleting row one step earlier is now skipped quietly. Either treat Missing as a skip, since the next scan won't find the row at all, or narrow the comment to "leaves suspended for another state".

2. crates/libs/sbx/mgr/server/src/db/startup_recovery.rs:382: the require_startup_state_write rationale is broader than what makes it true.

   • "Runs before sbx-manager binds its sockets": start_takeover_health binds the health socket (run.rs:783) before recover_interrupted_startup_states (run.rs:885). That socket doesn't write sandbox state, so the conclusion holds, but the sentence as written is false. "Before it binds the sandbox-manager and host-control sockets" (run.rs:1013, run.rs:1020) is accurate.
   • "Nothing else can move the row … so any outcome but Applied fails recovery" covers only FromState. A Guard refusal (quarantine, cleanup-owned) needs no concurrent writer. The interrupted-restore branch in the same file (line 313) tolerates exactly that with a warn and continue. The two siblings fail startup on it, as require_applied did before.

   Failing startup on a guard refusal isn't new, but the comment now justifies it with an argument about concurrency, which doesn't apply to guards. One sentence saying why the restore branch tolerates what the other two refuse would settle it.

3. crates/libs/sbx/mgr/server/src/service/reconcile/tests.rs:855: the sweep can't tell the skip arm from the error arm.
   Every moved case goes through force_state, so no case reaches a Guard refusal of a row that's still suspended, or a Missing row. With the if !edge.from().contains(found) guard at reconcile.rs:573 replaced by if true, a still-suspended CleanupOwned refusal would be swallowed as Ok(false), and I ran both new tests against that mutation: they pass. The description says these arms aren't pinned. A third case with an armed cleanup episode on a suspended row, asserting Aborted and no write, would pin the distinction the match exists for.

4. crates/libs/sbx/mgr/server/src/auto_lifecycle.rs:134: the new hook is a tokio::sync::Barrier called twice. The precedent the description cites, admission_barrier, uses TestAdmissionBarrier (service/cleanup_executor.rs:739), and so does PR sbx-mgr: wake and resident resume write only over the state they expect #413's BEGIN_BARRIER (auth_lookup/resume_begin.rs:14). That makes two two-phase test-hook idioms in one crate. Also, a Barrier(2) that a test sets but never waits on blocks the production task until the 30 s timeout, instead of failing with an "entered but never released" message. Reusing TestAdmissionBarrier would keep them consistent.

What I checked and found sound:

• The skip returns before mark_transition, so nothing is recorded. A row moved to a mid-transition state is re-picked once, then deferred by the existing auto_lifecycle_action_can_act gate. A row moved to stopped gets the Stop recorded as satisfied on the next scan. So the backstop converges.
• write_state_from returns FromState only when found is outside the from-set, and errors with StatementChangedRows(0) otherwise. The "impossible" claim for a still-suspended FromState holds.
• The startup ordering matches the description. The admission lease is held through recovery, and runtime-registration recovery follows it (run.rs:964). The auto-lifecycle scheduler and reconcilers start only after the lease drops (run.rs:996).
• Nothing outside recovery.rs matches on the removed StartupRecoveryError::StateWrite variant or on the old display strings.

Verified against cf519cefcdc4. Reproduced locally: cargo clippy -p pplx-space-sbx-mgr --all-targets --all-features -D warnings (clean) and cargo nextest run -p pplx-space-sbx-mgr -p pplx-space-db-test-fixtures --all-features (1652 passed, 0 failed) at this head; for finding 3, replacing the guard at reconcile.rs:573 with if true leaves both new tests passing.

[the-gigi-pplx]

Approved. 4 non-blocking findings in the review comment above.