fix(server): clear the sandbox review blockers

[awtprod]

A code review of the merged range 88ea72a8c..20bee7554 returned BLOCK with 8 P1 and 4 P2 findings. This fixes all twelve. Sandboxes are not enabled in production, so none of these were live — but several destroy user work or strand resources the moment they are.

Two of the P1s were regressions from the sandbox work in that range, both introduced with passing tests. They are called out below rather than buried.

P1

1. A spawned worker provisioned twice, and reported ready from the wrong state. The worker dispatch omitted provisionsInline, so the decider emitted sandbox.provision-requested — the inline sandbox.provision.ready was then rejected, and the request queued a second provisioning attempt. Regression from fix(server): harden sandbox lifecycle against restarts, deletion, and provision failures #51, which added the flag to two of the three inline call sites. All three are now consistent.

2. Automatic settle silently destroyed uncommitted work. The backend computed a --binary patch and the manager persisted only the bundle and HEAD, so restore checked out the commit and everything dirty or untracked was gone. Export now pins the working tree as a snapshot commit inside the bundle, so the digest-verified artifact carries it; restore unpacks it back as dirty files.

3. Restart adoption left threads "ready" but unusable. Reconcile recognised label-verified survivors without rebuilding execution records, so the projection said ready while every exec threw "sandbox is not ready". Survivors are now reported as unresumable rather than active, preserving the deliberate fail-closed intent.

4. Deleting a thread mid-provision could leave its container running. Stop observed no container yet and completed as a no-op; provisioning then finished into a terminal lifecycle.

5. Deleting a thread under a takeover lease never stopped the sandbox. The decider refuses sandbox.stop while a human holds the desktop — correct in general, impossible for deletion, since nothing will ever release that lease and reconcile still counts the thread as expected. Deletion now sends a forced stop. force is server-only, absent from ClientSandboxStopCommand, so a remote caller still cannot pull the desktop out from under someone.

6. Deletion could resurrect exported transcripts. An export already in flight renamed fresh artifacts into place after removal had run. Serialised per thread, plus a deletion tombstone the export checks immediately before its renames.

7. Remote podman had no configuration that was both working and bounded. One flag gated volume o=size quotas and container --storage-opt; enabling it broke provisioning over --remote, disabling it also discarded the volume limits that do hold. Now two controls: volume quotas default on (the real per-thread bound), container quota opt-in. A host that cannot administer project quotas at all gets a message naming the opt-out instead of podman's bare "Filesystem does not support Project Quota" — found by running the canary on exactly such a host.

8. A failed runtime upgrade could leave the podman socket down. The socket was stopped before the binary swap but only restored at the end, so any failure in between exited under set -e with the host's socket off. Restoration is now an EXIT trap armed before anything stops.

P2

9. A stale resume cursor survived a failed restore. The cursor was kept whenever a storeSha256 was recorded, but the artifact can be swept and the extraction is best-effort — leaving a clean container and a cursor naming a conversation that is not in it. Provisioning now reports what it actually restored.

10. Provider execution could target a different daemon than provisioning. The provider spawn env was PATH+HOME while the command executor forwards a wider non-secret allowlist that rootless/remote podman needs. Related to the executor fix in fix(server): harden sandbox lifecycle against restarts, deletion, and provision failures #51, which widened one path and not the other. Both now share one allowlist; credential stripping unchanged.

11. The projection recorded docker while podman provisioned. Server-side inline provisions now carry the resolved runtime; the decider stays pure.

12. A same-version rerun could not repair a partial install. Installation was skipped on a version match alone, so a missing helper or absent manifest stayed broken. The manifest is now validated entry by entry.

Verification

• Full server suite: 3295 passed. The 4 failures are pre-existing and unrelated — 3 CodexAdapter tests that fail from any linked git worktree (reproduced on a branch with none of these commits) and one ProviderRegistry test that passes 43/43 in isolation and only fails under suite parallelism.
• vp check 0 errors, tsgo 0 errors, bash -n clean on the bootstrap script.
• Every new test revert-checked: the production change is reverted alone and the test must fail.
• Canary against real containers, 10/10, exit 0 (apps/server/integration/sandboxCanary.integration.ts): provision → settle without wedging → re-provision from export → delete with artifact removal. Run once with the new quota defaults, which surfaced finding 7's messaging gap, then again with the documented opt-out.
• Shell logic for 8 and 12 verified against spliced harnesses with stubbed systemctl; the bootstrap script was never executed against this host.

---

Round 2: eight findings in the fixes above

A second review pass found eight more defects in these fixes. All are fixed in the later commits on this branch. Three deserve calling out:

Upgrade break (the most serious). Splitting the quota flag left volumeStorageQuotaEnabled reading only the new variable — so a host configured with the previously-documented T3_SANDBOX_CONTAINER_STORAGE_QUOTA=disabled would silently get volume quotas re-enabled at upgrade and fail every provision. An explicit new-variable value now wins; an unset one honours the legacy value with a one-shot deprecation warning. Mirrored in the bootstrap script and all four config docs.

Two signals that were added but never consumed. unresumableThreadIds (restart survivors) and the provider-store boolean both existed only in tests. Survivors now get exported then stopped before their thread is failed — the export is what saves the user's work, so the ordering is load-bearing and is asserted directly. The boolean became an explicit preserved | restored | unavailable tri-state routed through one shared cursor helper, because the boolean also cleared a valid cursor whenever a container survived and no restore was needed.

The rest: snapshot refs are now uniquely named and SHA-validated so a stale one cannot be restored over newer state, and a dirty tree that cannot snapshot fails the export loudly instead of silently dropping work; stop() awaits in-flight provisioning and unwinds anything created after a stop tombstone; the provider archive no longer strips Codex's sessions transcripts (every credential pattern retained and verified against real tar); tombstones are not evicted while an export is queued; and the bootstrap completeness check verifies digests rather than mere file existence.

Verification (round 2)

• Canary 10/10, exit 0, run with only the legacy quota variable set — deliberately exercising the upgrade path the earlier run masked. The deprecation warning appears once in the log and provisioning succeeds.
• Whole-repo vp check 0 errors, tsgo 0 errors, vp run -r typecheck clean across 17 packages, bash -n clean.
• Server suite 3311 passed; the only failures are the 3 known CodexAdapter tests that fail from any linked git worktree. Contracts 282 passed; web + packages 4146 passed.
• Every new test revert-checked. Bootstrap logic exercised against an isolated fake install root covering intact / truncated / missing / de-symlinked / repointed / legacy / empty manifests. The bootstrap script was never executed against this host.

Two notes for the reviewer

• The manifest format change means the first bootstrap re-run on an already-provisioned host reinstalls the bundle once to rewrite its manifest with digests, rather than skipping. Intentional — an unverifiable manifest is treated as damage — but it is a one-time extra download.
• apps/web/src/terminal/ghostty/runtimeAbi.test.ts fails to load in this worktree on a missing .wasm build artifact. Pre-existing environment issue, unrelated to this branch, but it means the web suite's count excludes that file.

---

Round 3: four more, two of them in the round-2 fixes

• A recovery export was erased right after being written. The round-2 unresumable drain exported a thread's work, then built sandbox.reconcile.result from the pre-export snapshot — overwriting the fresh lastExport after the volume was already destroyed, so the next provision seeded from the base commit. Now re-reads projected state, but only for threads that actually exported (a deleted thread is invisible to the detail query, so its recorded sandbox stays the source).
• stop() only synchronized against part of provisioning. It awaited the backend's ensureReady, but the manager keeps starting services, preview, credential, and desktop sidecars afterwards — so a concurrent stop could tear down, return, and let later sidecars survive. The lock moved to SandboxRuntimeManager, where the whole lifecycle is visible; the backend tombstone stays as the inner guard.
• Later bundles resurrected deleted files. Round 2 gave each dirty export a unique persistent snapshot ref, but git bundle create --all swept every ref into every later bundle, so a file deleted after an earlier export stayed recoverable from a newer artifact. Bundles are now scoped to the thread branch plus the current snapshot namespace, and stale snapshot refs are pruned before the dirty check.
• Claude stores always failed the restore probe. The probe matched only Codex's depth-2 sessions directory; Claude nests transcripts under .claude/projects (see UsageService.ts:235). Since the probe feeds the round-2 providerStore tri-state, every valid Claude restore was classified unavailable and its resume cursor cleared — silently losing the conversation on each re-provision, for one of the two providers that can run sandboxed at all. The probe now accepts both layouts.

Verification (round 3)

• Canary 10/10, exit 0 against real containers, exercising the export → settle → re-provision round-trip these changes touch.
• Whole-repo vp check 0 errors, tsgo 0 errors, clean tree, 26 commits.
• Server suite 3316 passed (baseline 3310 + 6 new tests); the only failures are the known 3 CodexAdapter linked-worktree tests and the occasional ProviderRegistry parallelism flake.
• Every new test revert-checked. Two worth noting: the concurrency test initially passed without the fix because the stop fiber never got scheduled — it was reworked until it genuinely reproduced the orphaned sidecar; and the retention test asserts against the bundle bytes rather than a restored checkout, because the restore-side SHA check would otherwise mask deleted content still present in the artifact.

Open judgement call

The Claude/Codex store probe is a documented union of both layouts rather than provider-driven: provider identity is not available at that call site, and threading it through means adding a provider field to SandboxBootstrap and deciding what happens when a thread's model selection changes mid-life. The union errs permissive deliberately — a missed layout clears the cursor and loses a conversation, an over-eager match costs one turn a resume error.

---

Round 4: three more, one a hole in the round-3 fix

• A deletion that beat the provision leaked its containers. The round-3 lifecycle lock only serialized provisions that had already entered the manager, so a stop could win the lock in the window after sandbox.provision was dispatched but before manager.provision() began, find no record, and return "nothing to do" — after which the queued provision built containers for a now-terminal thread. Compounding it, the sandbox.provision.ready dispatch had no error handling anywhere, so the decider's rejection was swallowed and the new containers leaked permanently. Both halves fixed: a stop tombstone re-read under the lock (cleared only once the decider has authorized a fresh provision), and every one of the three production provision.ready sites now tears the sandbox down if the transition is refused.
• An interrupted manifest write looked complete. write_podman_manifest truncated in place, so a partial write left a well-formed prefix that podman_install_is_complete accepted — a same-version rerun would then skip repairing every helper the manifest never got around to listing. Now built in a temp file, synced, atomically renamed, and closed with a version/count trailer that completeness verifies.
• A failed service restart was warn-only with no socket to recover it. When podman.service was active but podman.socket was not, a failed restart was tolerated on the reasoning that the socket would re-activate the daemon — which is false in exactly that state, so bootstrap exited 0 with the daemon down. Fatal now when the socket was inactive; still warn-only from the EXIT trap so it cannot mask an original error.

Verification (round 4)

• Canary 10/10, exit 0 against real containers.
• Whole-repo vp check 0 errors, tsgo 0 errors, bash -n clean, 29 commits, clean tree.
• Server suite 3320 passed; only the 3 known CodexAdapter linked-worktree failures.
• Every new test revert-checked individually. The concurrency case is structural rather than schedule-dependent — the stop runs to completion before the provision is issued, so there is no competing fiber that can fail to be scheduled, and the revert-check still shows it failing without the fix.
• Bootstrap changes exercised against an isolated fake install root (13/13 assertions) and a stubbed systemctl across all 9 unit-state × failure × severity combinations (9/9). The real script was never executed against this host.

---

Round 5: two more, both regressions from the rounds above

• Restore silently discarded a snapshot its own export recorded. #resolveExportSnapshot collapsed two different situations into one return undefined: "the export recorded no snapshot" (fine — clean tree) and "the export recorded one but it cannot be resolved" (not fine — provisioning proceeded from the branch head and dropped the dirty and untracked work the event log says exists). The failed-for-each-ref path had the same defect. Now, when a snapshot was recorded and cannot be resolved and verified, provisioning fails naming the expected commit and distinguishing the two causes; the stale-ref sweep stays on the no-snapshot path only, so a failure does not destroy the evidence. The throw reaches provision's existing cleanup, so the container is torn down rather than left half-seeded.
• A nested RETURN trap leaked the ~45MB podman install directory. install_podman_static and write_podman_manifest each installed one, and bash RETURN traps do not stack — the callee's replaced the caller's, reintroducing the very leak this branch's ancestor fixed, by a different mechanism. One function now owns the trap and cleans both temporaries; both carry a comment saying traps do not stack, since this has bitten twice. trap -p RETURN save/restore was tried and ruled out empirically: it reads back empty without set -T, and double-fires with it.

Verification (round 5)

• Canary 10/10, exit 0 against real containers, exercising the export → settle → re-provision round-trip the snapshot fix guards.
• Whole-repo vp check 0 errors, tsgo 0 errors, bash -n clean, 31 commits, clean tree.
• Server suite 3322 passed; only the 3 known CodexAdapter linked-worktree failures.
• Both new tests revert-checked: with the fix reverted, provisioning succeeded where it should have failed — the exact silent-degradation signature. The trap fix was verified with a probe that extracts the real functions, stubs every host-touching command, and asserts both temporaries are gone; it printed LEAKED install dir: … against the pre-fix script.

Two notes for the reviewer

• One commit is --no-verify, disclosed in its message. The pre-commit hook's only step is vp fmt, which has no shell handler and aborts a shell-only commit outright; earlier bootstrap commits passed only because they also touched a .md file. scripts/public-leak-scan.ts was run by hand instead (clean, 145 files + 43 revisions). The hook gap is worth fixing separately so shell-only commits are not forced through --no-verify.
• Pre-existing and out of scope: install_podman_static's die paths still leak their temp dir, because bash RETURN traps do not fire on exit. Unchanged by this branch and a different defect from the one reviewed.

[awtprod]

@coderabbitai review

[awtprod]

@codex review

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	13.6 KiB	13.6 KiB	+18 B (+0.1%)	15.1 KiB	✅
Codex	Thread snapshot wire	6.9 KiB	6.9 KiB	+2 B (+0.0%)	7.3 KiB	✅
Codex	Live turn WebSocket wire	6.7 KiB	6.7 KiB	+16 B (+0.2%)	7.8 KiB	✅
Codex	Live turn WebSocket decoded	54.2 KiB	54.2 KiB	0 B (0.0%)	66.4 KiB	✅
Codex	Live turn messages	17	17	0 (0.0%)	21	✅
Claude	Total thread wire	13.7 KiB	13.7 KiB	−6 B (−0.0%)	15.1 KiB	✅
Claude	Thread snapshot wire	7.0 KiB	7.0 KiB	−1 B (−0.0%)	7.3 KiB	✅
Claude	Live turn WebSocket wire	6.7 KiB	6.7 KiB	−5 B (−0.1%)	7.8 KiB	✅
Claude	Live turn WebSocket decoded	55.0 KiB	55.0 KiB	0 B (0.0%)	66.4 KiB	✅
Claude	Live turn messages	17	17	0 (0.0%)	21	✅

Baseline: 20bee75 · PR result: 801264f · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 107.8 KiB
• Claude decoded thread snapshot: 108.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.