[ac-pplx]

Stacked on main. Stack, bottom to top: #438 → #418 → #413 → #416 → #421 → #417 → #414. #372 and #388 have merged to main.

Replaces #424, which closed automatically when its base branch merged.

Each update_state_from conversion first written on #388 wrote its own StateWriteOutcome → Status helper, with different names, message text and policies. This adds one shared mapping and one best-effort writer in service/cleanup_support.rs, with no production callers. The conversions, now stacked on this PR, delete their copies. The legacy require_state_write for update_state stays unchanged.

API

• require_state_write_from(result, context) -> Result<(), Status> is the primary mapping.
• state_write_result(outcome) -> Result<SandboxState, StateWriteRejected> is the typed form. It returns the overwritten state, or one of FromState { found }, Guard { refusal: StateWriteGuard, found } and Missing. into_status(context) turns a rejection into a status.
  • A caller that answers one case differently (for example warm start with FailedPrecondition) matches that case and passes the rest to into_status.
  • Guard carries found for callers that decide by edge.from().contains(found).
• update_state_from_best_effort(db, sandbox_id, edge) -> Option<StateWriteOutcome> is update_state_best_effort for update_state_from, with a wildcard-free match.
  • It logs with the same message text and field names, except that outcome is logged with Debug (?outcome), where the legacy function uses Display.
  • Only Applied marks the sandbox dirty.
  • None means a poisoned connection (failure signalled) or a database error.

Messages

Outcome	Code	Message
Applied { from }	—	Ok(()) (state_write_result: Ok(from))
Refused { refusal: FromState, found }	Aborted	{context}: sandbox state write was superseded: sandbox is {found}
Refused { refusal: Quarantined, .. }	Aborted	{context}: sandbox state write was not applied: blocked_by_quarantine
Refused { refusal: CleanupOwned, .. }	Aborted	{context}: sandbox state write was not applied: superseded_by_cleanup_episode
Missing	Aborted	{context}: sandbox state write was not applied: missing
database error	Internal	{context}: {error}

The guard, missing and database-error rows are byte-for-byte what require_state_write returns for update_state, at 8f12bd6 and now.

Dead-code expectations

Outside tests the new items are #[cfg_attr(not(test), expect(dead_code, ...))]. Adopting require_state_write_from leaves several expectations unfulfilled at once: the function, StateWriteRejected, StateWriteGuard, state_write_result and the impl. unfulfilled_lint_expectations is warn-by-default and an error only under CI's clippy -D warnings, so the adopting conversion must drop those attributes: #418, the first conversion above this PR, drops them. update_state_from_best_effort keeps its own expectation until a caller uses it: #421 drops it.

Deferred: StateWriteGuard duplicates StateWriteRefusal::{Quarantined, CleanupOwned}, and the legacy SandboxStateWriteOutcome is a third copy. The fix nests them: generic_state_write_refusal -> Option<StateWriteGuard> and StateWriteRefusal::Guard(StateWriteGuard). PR 6, which retires the legacy enum, does that, so this PR doesn't churn #388's types under the conversions stacked above it.

Tests (service/cleanup_support/tests.rs)

• Wildcard-free matches over every StateWriteOutcome × StateWriteRefusal × state assert the typed rejection and the exact code and message. The refusal list itself comes from a wildcard-free successor match, so a new variant does not compile until it is swept.
• On the same row, the new mapping and the update_state caller path return the same status, and every row stays unchanged. The rows are a quarantined row, a stopping row revived to started, a cleanup_required row with a delete_row intent, and a missing row. The row's own state is in the from-set, so the guard is what refuses. The messages are also checked against literals copied from 8f12bd6.
• The best-effort writer:
  • applies a write and marks only that sandbox dirty;
  • returns the refusal and changes nothing for a from-state refusal (stopped), a cleanup-owned row and a missing row;
  • returns None and changes nothing on a database error.
• A poisoned connection returns None and changes nothing. It also wakes the dirty-outbox worker, which exits on the poisoned connection and fails its critical-task guard. The test pauses the clock and only yields, so the worker's poll timer cannot fire and only the signal can wake it.
• No assertions on log fields, severity or text.

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 (bf43fd5): 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 1634/1634, with 0 dependency skips. 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	pass
cargo clippy --workspace --locked --all-targets -- -D warnings	pass
cargo arch	pass, 0 required findings; the only advisories are on files this PR does not touch
SPACE_CI_REQUIRE_TEST_DEPS=1 cargo nextest run -p pplx-space-sbx-mgr ×3	1630/1630 passed each run. The 1 skip is the #[ignore] root-only distinct_service_uids_can_exchange_downloads; 0 dependency skips
Mutation proof (20 mutants: every mapping arm swapped, both status codes, the from-state text, the database-error code, a swallowed rejection, found dropped from Guard, and each best-effort dirty, return and poison-signal arm)	20/20 killed
Kind	not run: no production callers

🤖 Generated with Claude Code

[nikhilgupta-pplx]

Approve. Mapping, typed rejection and best-effort writer match the legacy update_state paths (guard/missing/DB-error text is byte-identical; dirty only on Applied; poison still signals). No production callers, and the dead-code handoff to #418/#421 is consistent.

Non-blocking nits:

• update_state_from_best_effort logs Missing as "superseded" (inherited from the legacy writer); worth a distinct message while this is new.
• require_state_write_from takes &'static str while into_status takes &str; minor inconsistency.

[the-gigi-pplx]

Review — 2 findings

Low

1. crates/libs/sbx/mgr/server/src/service/cleanup_support/tests.rs:357 — best_effort_database_error_is_none_and_changes_nothing never reaches a database error.
   It passes a KnownRace edge, so update_state_from returns InvalidQuery from its precondition check (db/state_write.rs:554) before it touches SQLite. The Err arm is covered, but by the path that rejects a caller's mistake, and in production that mistake logs the same warn as a transient SQLite failure ("failed to update sandbox state"). Later in the stack the edge becomes ORPHAN_SUSPENDED_INSERT, the same pattern again. A trigger that fails inside SQLite itself would keep the test meaningful once every edge passed in is a valid one.
2. crates/libs/sbx/mgr/server/src/service/cleanup_support/tests.rs:42 — the literals cite 8f12bd6, which no branch of this repo contains.
   It's the pre-squash head of PR sbx-mgr: characterize the state writers before the state-write helper (tests only) #372, reachable only through that PR's ref, so git show 8f12bd6 in a clone fails. a7e700bc, PR sbx-mgr: characterize the state writers before the state-write helper (tests only) #372's squash on main, has the same require_state_write, SandboxStateWriteNotApplied message and as_str labels, so citing it keeps the provenance checkable. The commit message and PR body cite the same sha.

What I checked and found sound:

• require_state_write_from's guard, missing and database-error statuses are the same text and code as require_state_write's. Both format "{context}: {SandboxStateWriteNotApplied}" over the same as_str labels, and both map a database error to Internal "{context}: {error}".
• state_write_result and into_status have no wildcard arm, and refusals() builds its list from a wildcard-free successor match, so a new outcome or refusal fails to compile until it's handled.
• update_state_from_best_effort drops the connection guard before mark_sandbox_dirty, as update_state_best_effort does, and only Applied marks the sandbox dirty. snapshot covers every table, so the refusal cases also show that no dirty ref is written.
• connection_or_signal_failure calls notify_one, which stores a permit, so the poisoned-connection test doesn't depend on the worker already waiting when the signal fires.
• Nothing in the repo searches the outcome log field, so the Display-to-Debug change the body mentions has no consumer to break.

Verified against bf43fd5b1b77. 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 (1645 passed, 0 failed) at this head; two mutations: having update_state_from_best_effort lock the connection directly instead of connection_or_signal_failure fails best_effort_write_over_a_poisoned_connection_signals_the_failure, and mapping CleanupOwned to StateWriteGuard::Quarantined in state_write_result fails both mapping tests, so neither guard is vacuous.

[the-gigi-pplx]

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