fix(recover): preserve state across supervisor relaunch

[HOYALIM]

Summary

Legacy supervisor recovery now backs up the complete manifest-declared sandbox state before recreating its container, restores that state only after exact replacement identity and managed health are proven, and rolls back on any backup, identity, or restore failure. Settled recovery and rollback transactions remove their temporary host backup, while total failure preserves it and prints actionable recovery guidance.

Related Issue

Addresses the workspace-loss recovery path in #7404 without closing the separate exit-0 crash-loop report.

Changes

• require a complete manifest-driven state backup before the legacy keepalive container is recreated
• pin restore to the exact replacement container and run it only after managed health succeeds
• roll back to the previous container when replacement identity or state restoration fails
• delete complete and partial temporary state backups only after a safe settled outcome
• retain the backup and print host recovery commands when both recovery and rollback fail
• document the preserved manifest-declared state and remaining writable-layer boundary

Type of Change

• Code change (feature, bug fix, or refactor)
• Code change with doc updates
• Doc only (prose changes, no code sample modifications)
• Doc only (includes code sample changes)

Quality Gates

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification:
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Nine-category exact-head security review passed at 6d9bb081b; diff fingerprint 2276e0fec546e501c81bb1108e8e9cc387ccaed1c05a1288d925235eb13d98d9; the stable patch ID is unchanged across the current-main merge.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: docs-updated
• Evidence: docs/manage-sandboxes/recover-rebuild-sandboxes.mdx and docs/reference/commands.mdx document manifest-declared state backup and restore, exact replacement identity, rollback cleanup, retained-backup guidance, and the remaining writable-layer boundary. Changed comments, test titles, and terminology conform to the writing guides. Exact-head current-main refresh preserves the reviewed patch byte-for-byte by stable patch ID, so no additional docs edit is needed; focused tests passed 14/14, affected tests passed 705/705, CLI typecheck passed, and check:diff passed.
• Agent: Codex Desktop

Verification

• PR description includes a Signed-off-by: line and every commit is signed for GitHub verification
• Normal pre-commit, commit-msg, and pre-push hooks passed
• Targeted behavior tests pass for the current change set — npx vitest run --project cli src/lib/actions/sandbox/supervisor-relaunch.test.ts (14 passed); npm run test:changed (705 passed); the earlier recovery integration suites remain 26/26
• Applicable broad gates passed — npm run typecheck:cli and npm run check:diff; normal hooks passed during commits and push
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only) — command exited 0; Fern reported 0 errors and 2 warnings
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Ho Lim subhoya@gmail.com

Summary by CodeRabbit

• Bug Fixes
  • Improved sandbox recovery to fail fast when state restoration doesn’t succeed, and to only proceed when the replacement container identity is verified.
  • Strengthened rollback behavior with clearer recovery messaging and safer temporary state-backup cleanup/retention when cleanup fails.
  • Added more precise warnings for both container backup and temporary state-backup removal outcomes.
• Documentation
  • Clarified transactional recover/rebuild flow, including when backups are committed vs retained and what guidance appears when restore+rollback both fail.
  • Updated command reference for Deep Agents JSON output and refined sandbox status/recovery descriptions.
• Tests
  • Expanded coverage for restore/rollback combinations, diagnostics output, and targeted state-backup removal behavior.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The managed sandbox relaunch now backs up and restores declared state, validates replacement container identity, reports restoration and rollback outcomes, securely cleans up backups, and stops recovery when state restoration fails.

Changes

Sandbox relaunch recovery

Layer / File(s)	Summary
State backup contract and pre-relaunch validation src/lib/actions/sandbox/supervisor-relaunch.ts, src/lib/actions/sandbox/supervisor-relaunch.test.ts	Relaunch dependencies support injected state handlers, sandbox state is backed up before recreation, and incomplete backups prevent recreation.
Transactional restoration and finalize outcomes src/lib/actions/sandbox/supervisor-relaunch.ts, src/lib/state/sandbox.ts, src/lib/actions/sandbox/supervisor-relaunch.test.ts, test/snapshot-recovery-validation.test.ts, docs/...	Finalization validates replacement-container identity, restores sandbox state when ready, securely removes completed backups, reports restoration and rollback outcomes, and documents the transactional flow.
Recovery failure propagation src/lib/actions/sandbox/process-recovery.ts, test/process-recovery-supervisor-relaunch.test.ts	Process recovery returns failure and skips readiness and forwarding when relaunch restoration fails or rolls back, while reporting cleanup failures and recovery hints.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: cv, cjagwani

Sequence Diagram(s)

sequenceDiagram
  participant checkAndRecoverSandboxProcesses
  participant relaunchManagedSupervisorSession
  participant sameContainerId
  participant sandboxState
  participant deps.finalize
  checkAndRecoverSandboxProcesses->>relaunchManagedSupervisorSession: start managed supervisor relaunch
  relaunchManagedSupervisorSession->>sandboxState: back up declared sandbox state
  relaunchManagedSupervisorSession->>sameContainerId: validate replacement container identity
  relaunchManagedSupervisorSession->>sandboxState: restore state from backup manifest
  sandboxState-->>relaunchManagedSupervisorSession: restoration and cleanup outcome
  relaunchManagedSupervisorSession->>deps.finalize: finalize relaunch outcome
  deps.finalize-->>checkAndRecoverSandboxProcesses: stateRestored or rolledBack result
  checkAndRecoverSandboxProcesses-->>checkAndRecoverSandboxProcesses: stop readiness and forwarding on failure

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: preserving sandbox state during supervisor relaunch recovery.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/actions/sandbox/process-recovery.ts (1)

1-1: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

The double-failure branch (state not restored AND rollback failed) is both unfixed and untested. The new failure-handling block in process-recovery.ts omits operator recovery hints for this worst-case outcome, and the new test only exercises the milder rolledBack: true case, so the gap in guidance has no regression coverage.

• src/lib/actions/sandbox/process-recovery.ts#L1281-1295: add the printHostManagedGatewayRecoveryHints(...) call in the !completion.rolledBack branch (see diff in the per-site comment above).
• test/process-recovery-supervisor-relaunch.test.ts#L223-266: add a companion test where finalize returns { backupRemoved: false, rolledBack: false, stateRestored: false } and assert the recovery hints are surfaced (e.g., via a printHostManagedGatewayRecoveryHints/console spy) alongside recovered: false.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/actions/sandbox/process-recovery.ts` at line 1, Update the
double-failure handling in the process-recovery completion path to call
printHostManagedGatewayRecoveryHints(...) when !completion.rolledBack, while
preserving recovered: false. Add companion coverage in the supervisor relaunch
tests with finalize returning backupRemoved: false, rolledBack: false, and
stateRestored: false, asserting both recovery hints and the unrecovered result.

🧹 Nitpick comments (1)

src/lib/actions/sandbox/supervisor-relaunch.ts (1)

183-222: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the duplicated rollback-outcome construction.

The !supervisorReady, !replacementOwned, and !stateRestored branches all build the identical outcome shape ({ ...finalize({ result, supervisorReady: false }), stateRestored: false }). In this critical transactional path, keeping three copies risks one being updated while the others are missed in a future change.

♻️ Proposed extraction

+    const rollback = (): DockerGpuPatchFinalizeOutcome & { stateRestored: boolean } => ({
+      ...finalize({ result, supervisorReady: false }),
+      stateRestored: false,
+    });
     return {
       containerId: result.newContainerId,
       finalize(supervisorReady) {
         if (completed) {
           if (completed.supervisorReady !== supervisorReady) {
             throw new Error(
               "Supervisor relaunch transaction was finalized with conflicting state.",
             );
           }
           return completed.outcome;
         }
         if (!supervisorReady) {
-          const outcome = { ...finalize({ result, supervisorReady: false }), stateRestored: false };
+          const outcome = rollback();
           completed = { supervisorReady, outcome };
           return outcome;
         }
         let replacementOwned = false;
         try {
           replacementOwned = sameContainerId(
             resolveContainer(sandboxName, driver),
             result.newContainerId,
           );
         } catch {
           replacementOwned = false;
         }
         if (!replacementOwned) {
-          const outcome = {
-            ...finalize({ result, supervisorReady: false }),
-            stateRestored: false,
-          };
+          const outcome = rollback();
           completed = { supervisorReady, outcome };
           return outcome;
         }
         let stateRestored = false;
         try {
           stateRestored = restoreState(sandboxName, backupManifest.backupPath).success;
         } catch {
           stateRestored = false;
         }
         if (!stateRestored) {
-          const outcome = {
-            ...finalize({ result, supervisorReady: false }),
-            stateRestored: false,
-          };
+          const outcome = rollback();
           completed = { supervisorReady, outcome };
           return outcome;
         }

As per coding guidelines, "**/*.{js,ts,tsx}: Keep function complexity low; existing complexity hotspots are tracked separately."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/actions/sandbox/supervisor-relaunch.ts` around lines 183 - 222,
Extract the repeated rollback outcome construction into a local helper within
the surrounding relaunch flow, using the existing result and supervisorReady
context to return the finalized outcome with stateRestored set to false. Replace
the !supervisorReady, !replacementOwned, and !stateRestored branches with calls
to this helper while preserving their completed assignment and early returns.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/actions/sandbox/process-recovery.ts`:
- Around line 1281-1295: Update the failure branch handling
completion.stateRestored === false || completion.rolledBack in the surrounding
recovery function to call printHostManagedGatewayRecoveryHints(...) when both
state restoration and rollback fail, matching the equivalent !gatewayReady path.
Also replace the “state restore failed” wording with a generic recovery-failure
message that remains accurate when restoration was never attempted.

In `@src/lib/actions/sandbox/supervisor-relaunch.ts`:
- Around line 135-158: The relaunch flow leaves backupManifest.backupPath behind
after recovery. Update the supervisor relaunch logic around the restore/rollback
completion paths to remove the backup after the container is successfully
settled, including rollback handling, while preserving cleanup behavior when
restore or recreation fails.

---

Outside diff comments:
In `@src/lib/actions/sandbox/process-recovery.ts`:
- Line 1: Update the double-failure handling in the process-recovery completion
path to call printHostManagedGatewayRecoveryHints(...) when
!completion.rolledBack, while preserving recovered: false. Add companion
coverage in the supervisor relaunch tests with finalize returning backupRemoved:
false, rolledBack: false, and stateRestored: false, asserting both recovery
hints and the unrecovered result.

---

Nitpick comments:
In `@src/lib/actions/sandbox/supervisor-relaunch.ts`:
- Around line 183-222: Extract the repeated rollback outcome construction into a
local helper within the surrounding relaunch flow, using the existing result and
supervisorReady context to return the finalized outcome with stateRestored set
to false. Replace the !supervisorReady, !replacementOwned, and !stateRestored
branches with calls to this helper while preserving their completed assignment
and early returns.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 38b8b51b-72fe-48be-b0e5-a4482544276a

📥 Commits

Reviewing files that changed from the base of the PR and between 3d88c04 and 3e91152.

📒 Files selected for processing (4)

• src/lib/actions/sandbox/process-recovery.ts
• src/lib/actions/sandbox/supervisor-relaunch.test.ts
• src/lib/actions/sandbox/supervisor-relaunch.ts
• test/process-recovery-supervisor-relaunch.test.ts

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume

1 optional E2E recommendation

• ubuntu-repo-docker-post-reboot-recovery

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

🧹 Nitpick comments (1)

test/snapshot-recovery-validation.test.ts (1)

56-65: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add a symlink-rejection regression case.

This verifies lexical containment but not the security boundary at removeSandboxStateBackup’s symlink check. Add a symlink inside the sandbox backup root pointing to outsidePath, then assert removal returns false and the outside directory remains untouched.

As per path instructions, destructive lifecycle operations must validate before mutation and cover failure/recovery behavior.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/snapshot-recovery-validation.test.ts` around lines 56 - 65, Extend the
test case around removeSandboxStateBackup to create a symlink within the sandbox
backup root that points to outsidePath, then assert removal returns false and
outsidePath still exists. Keep the existing exact-child success and
lexical-containment checks, ensuring the symlink rejection is validated before
any mutation.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/snapshot-recovery-validation.test.ts`:
- Around line 56-65: Extend the test case around removeSandboxStateBackup to
create a symlink within the sandbox backup root that points to outsidePath, then
assert removal returns false and outsidePath still exists. Keep the existing
exact-child success and lexical-containment checks, ensuring the symlink
rejection is validated before any mutation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b9dbce3f-af13-4ac7-882f-d79cf63e96c4

📥 Commits

Reviewing files that changed from the base of the PR and between 3e91152 and 3c8053c.

📒 Files selected for processing (6)

• src/lib/actions/sandbox/process-recovery.ts
• src/lib/actions/sandbox/supervisor-relaunch.test.ts
• src/lib/actions/sandbox/supervisor-relaunch.ts
• src/lib/state/sandbox.ts
• test/process-recovery-supervisor-relaunch.test.ts
• test/snapshot-recovery-validation.test.ts

🚧 Files skipped from review as they are similar to previous changes (4)

• src/lib/actions/sandbox/process-recovery.ts
• src/lib/actions/sandbox/supervisor-relaunch.test.ts
• test/process-recovery-supervisor-relaunch.test.ts
• src/lib/actions/sandbox/supervisor-relaunch.ts

[prekshivyas]

Security review: PASS at head 0c2a03033f0c6d70e7c43d714df1e36aa92b908f

1. Secrets and credentials — PASS. The transaction uses the existing sanitized manifest backup path and does not add credential propagation or logging.
2. Input and path validation — PASS. Cleanup is limited to an exact child of the sandbox backup root, rejects symlinks before mutation, and has outside-target regression coverage.
3. Authentication and authorization — PASS. No authentication or authorization behavior changes.
4. Dependencies and supply chain — PASS. No dependencies or generated artifacts are added.
5. Error handling and logging — PASS. Backup, identity, restore, and rollback failures fail closed; logs are redacted and preserve the original failure.
6. Cryptography — PASS / not applicable. No cryptographic behavior changes.
7. Configuration and environment — PASS. Existing recovery opt-out and driver constraints remain; reconstructed launch environment continues to omit credential variables.
8. Testing — PASS. Focused CLI tests pass 13/13, integration recovery tests pass 26/26, CLI typecheck passes, and the docs build passes. Coverage includes partial-backup cleanup, exact replacement identity, restore failure, rollback retention, and symlink rejection.
9. System security — PASS. Destructive recreation requires a complete backup, legacy-startup match, missing-supervisor confirmation, and exact container identity. State backups are removed only after successful restore or successful rollback and are retained for operator recovery after total failure.

No blocking security finding remains. Final approval should wait for required CI and automated review checks on this exact head.

[prekshivyas]

Exact-head correction for the preceding review: the reviewed and pushed head is 0c2a030. GitHub correctly anchored both reviews to that commit. The nine-category PASS assessment and CI-pending disposition are unchanged.

[prekshivyas]

Nine-category security review — exact head a8dc94a11a3f51b22300e15340bd1dd1f91d46ee

1. Prompt injection / unsafe tool use — PASS. Recovery is host-controlled and consumes no model output.
2. SSRF / network policy — PASS. No network policy or outbound request changes.
3. Secrets — PASS. Startup-command persistence retains the existing credential redaction behavior; failure diagnostics remain redacted.
4. Privilege / filesystem boundaries — PASS. Backup and restore remain limited to manifest-declared sandbox state.
5. Authorization / identity — PASS. Recreation and restore remain pinned to the selected old container and exact replacement identity.
6. Supply chain — PASS. No dependency or artifact-source changes.
7. Input validation / command injection — PASS. Existing validated sandbox names and reconstructed command boundaries are unchanged.
8. State integrity / failure ordering — PASS. A successful state backup is now removed if recreation throws before a replacement transaction exists; cleanup failure is isolated so the original recreation diagnostic is preserved.
9. Tests / operational handling — PASS. The new tests cover cleanup on recreation failure and cleanup-throw diagnostic preservation. Focused 14/14, affected 705/705, CLI typecheck, and check:diff passed.

Overall: PASS at this exact head. The prior PR Review Advisor warning about leaking the temporary backup when recreate throws is fixed. This is a COMMENT review while fresh CI and E2E run.

[prekshivyas]

Exact-head ordinary CI is green, but credentialed E2E is blocked by the repository's fork-approval environment configuration.

• Head: a8dc94a11a3f51b22300e15340bd1dd1f91d46ee
• Base: eeab81cc5542902538c97db63c132c0fdbd4341c
• Controller: https://github.com/NVIDIA/NemoClaw/actions/runs/30422188440
• Selected plan: onboard-repair, onboard-resume

The controller refused to start the plan because approve-credentialed-e2e-for-fork-pr has no required-reviewer protection rules, so GitHub recorded no protected approval. No selected E2E ran and no repository credential was exposed. A repository admin must configure required reviewers; the workflow then requires a new PR head and fresh CI. I have not weakened or changed the environment.

[cjagwani]

Exact-head security receipt for 6d9bb081b (diff fingerprint 2276e0fec546e501c81bb1108e8e9cc387ccaed1c05a1288d925235eb13d98d9): PASS. The verified current-main merge touched recovery code, so I compared the complete pre/post-refresh PR patch: the stable patch ID is identical (d9305656ce90729ed8ef5f7bb8bb4ee71988d0e7) and git range-diff reports all six non-merge commits unchanged. The existing nine-category review therefore remains valid: complete manifest backup precedes recreation; restore is bound to exact replacement identity and managed health; failed restore rolls back; settled backups are removed; total failure retains recovery evidence and prints bounded guidance. No new findings across credentials, paths, authorization, dependencies, error handling, state integrity, configuration, tests, or system security.

[cjagwani]

Approved exact head 6d9bb081bb9f439d26c711032f9dcd0b9d0d8884 after the maintainer gate. The reviewed recovery/supervisor patch has no remaining security blocker; all commits are validly signed, the primary advisor is green, no unresolved current review threads remain, and the trusted protected E2E plan completed successfully (including onboard repair/resume coverage). Superseded cancelled runs were not treated as current evidence.

[cjagwani]

Exact-head approval after refresh onto current main. Deterministic maintainer gate passes with all 42 current checks green, clean merge state, verified history, and no unresolved major findings. Protected onboard-repair and onboard-resume both passed on this head.