fix(recovery): retry state backup after restart

[apurvvkumaria]

Summary

Legacy supervisor recovery currently performs a one-shot state backup immediately after Docker restart, when OpenShell exec can be ready before its SSH transport. Retry only that typed transport lag before recreation while keeping integrity, audit, and cleanup failures terminal.

Changes

• Retry unreachable state backups up to five attempts with a two-second container-side wait.
• Remove partial backup state before each retry and stop closed if cleanup fails.
• Add focused coverage proving transport retry, cleanup, and recreation ordering.

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: Existing recovery docs already describe backup before recreation and fail-closed integrity behavior; no command or guarantee changed.
• 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: Maintainer security review passed for the current branch revision: fix(recovery): retry state backup after restart #8787 (comment)
• 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: no-docs-needed
• Evidence: Existing recovery guidance remains accurate in docs/manage-sandboxes/recover-rebuild-sandboxes.mdx and docs/manage-sandboxes/backup-restore.mdx; the bounded transport retry does not change supported state, commands, settings, final failure handling, or the recovery acceptance criterion. Documentation validation passed with 0 errors and 2 existing warnings.
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit:
• Station profile/scenario:
• Result:
• Supporting evidence:

Verification

• PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
• Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: 18 focused supervisor-relaunch tests passed after the final test-only change; the broader recovery suite passed 48 tests after the production change.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result: Not applicable; focused recovery boundary with targeted unit and integration coverage.
• 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)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Improved legacy sandbox recreation by retrying temporary state-backup connectivity failures up to five times.
  • Added a two-second delay between retry attempts.
  • Cleans up incomplete backup data before retrying while preserving immediate handling for other backup failures.
  • Improves recovery when temporary backup service interruptions occur.

• Tests

  • Added coverage for delayed retries, backup cleanup, retry ordering, and successful continuation after recovery.

[coderabbitai]

📝 Walkthrough

Walkthrough

Legacy sandbox recreation now retries temporary unreachable state-backup failures. Each retry removes the partial manifest and waits two seconds. The retry limit and sleep behavior are injectable and covered by tests.

Changes

Sandbox relaunch retry

Layer / File(s)	Summary
Retry limits and delay dependency src/lib/actions/sandbox/supervisor-relaunch.ts	State-backup retries are limited to five attempts with two-second delays. ManagedSupervisorRelaunchDeps now accepts an optional sleep function.
Backup retry execution and validation src/lib/actions/sandbox/supervisor-relaunch.ts, src/lib/actions/sandbox/supervisor-relaunch.test.ts	Temporary unreachable backup failures remove partial manifests before retrying. Removal errors and other backup failures remain terminal. Tests cover single and repeated retries before recreation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 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 and concisely describes the main change: retrying state backup after supervisor restart.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/recovery-backup-retry

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 99b1430 in the codex/recovery-backu... branch remains at 96%, unchanged from commit e76d319 in the main branch.

---

_{Updated August 11, 2026 09:41 UTC}

[coderabbitai]

Actionable comments posted: 2

🤖 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/supervisor-relaunch.test.ts`:
- Around line 162-190: Update the test “retries only transport-level state
backup failures after a container restart” to record dependency events from
backupState, removeBackup, sleep, and recreate, then assert the exact sequence:
initial backupState, removeBackup, sleep, retry backupState, and recreate.
Preserve the existing outcome assertions while verifying partial-backup cleanup
occurs before retrying.

In `@src/lib/actions/sandbox/supervisor-relaunch.ts`:
- Around line 35-36: Update the retry logic around backupState and
STATE_BACKUP_MAX_ATTEMPTS so the initial backup failure receives five additional
retries, allowing attempts 1 through 5; align the constant’s meaning with the
loop boundary. Add a boundary test covering five failed retries followed by a
successful result.

🪄 Autofix

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: df90ad20-1d4b-42ad-ac4e-7b0a1ce3a265

📥 Commits

Reviewing files that changed from the base of the PR and between 0cddc38 and 3e31f1d.

📒 Files selected for processing (2)

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

[github-actions]

PR Review Advisor — Informational

Advisor assessment: Informational / low confidence
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions
Status: Partial review preserved 1 canonical finding(s) and 3 terminology decision(s) before the advisor stopped.

Model lanes

• GPT-5.6 Terra (primary): Failed after a partial review · low confidence · 0 blockers · 1 warning · 0 suggestions
• Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 0 blockers · 1 warning · 0 suggestions

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• define — OpenShell exec relay at src/lib/actions/sandbox/supervisor-relaunch.ts:163: Keep the explanatory comment that defines the contrast between the OpenShell exec relay and SSH transport.
• replace — typed transport lag at src/lib/actions/sandbox/supervisor-relaunch.ts:164: Replace "typed transport lag" with "unreachable backup failure" to match the checked condition.
• established — state backup at src/lib/actions/sandbox/supervisor-relaunch.test.ts:162: Keep "state backup" in the added test titles.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: None

Manual-only E2E: onboard-repair, onboard-resume
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Cover cleanup refusal before a backup retry

• Location: src/lib/actions/sandbox/supervisor-relaunch.ts:172
• Category: tests
• Problem: The retry loop stops when partial-backup cleanup returns false or throws, but the added retry tests exercise only successful cleanup.
• Impact: A later change could continue retrying or recreate the sandbox after cleanup fails, leaving a partial backup in the recovery path.
• Recommendation: Add one retry-path test where an unreachable backup has a manifest and removeBackup returns false.
• Verification: Inspect supervisor-relaunch.test.ts for an unreachable backup fixture with removeBackup returning false and assertions that sleep, backupState, and recreate stop after the first attempt.
• Test coverage: Assert that cleanup refusal causes no sleep, no second backup attempt, and no recreation.
• Evidence: src/lib/actions/sandbox/supervisor-relaunch.ts:172-177 returns before sleep, a second backup attempt, or recreation when removeBackup fails. src/lib/actions/sandbox/supervisor-relaunch.test.ts:162-258 covers unreachable-backup retries with successful removeBackup mocks only.

Workflow run details

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

[apurvvkumaria]

Security review: PASS

Reviewed the complete PR diff through branch revision 10b6ec2bf against current main (0cddc38c8). The change adds a bounded retry for the existing sandbox-state backup when the typed result reports that the restarted container transport is temporarily unreachable.

Review checklist

1. Authentication and authorization: No identity, credential, role, permission, or authorization boundary changes.
2. Input validation and injection: The retry path accepts no new user-controlled command text. The fallback wait invokes docker exec with a fixed sleep command and a numeric constant through an argument vector, without a shell.
3. Secrets and credentials: Backup contents, diagnostics, and credential redaction are unchanged. The retry does not log backup data or add secret-bearing environment values.
4. Cryptography and integrity: Integrity and audit failures remain terminal. Only unreachable: true enables a retry, and a backup with failed directories/files still stops recreation.
5. Network and SSRF: No endpoint, network policy, URL, DNS, proxy, or SSRF behavior changes.
6. Filesystem and state safety: A partial backup is removed before every retry. Cleanup failure stops the transaction, and the original container remains unchanged unless a complete backup is available.
7. Sandbox and process safety: The retry is bounded to five attempts after the initial call, with a fixed two-second wait and per-wait timeout. The existing container identity and legacy-startup checks remain prerequisites.
8. Supply chain and dependencies: No dependency, workflow, image, action, lockfile, or build-provenance changes.
9. Privacy, logging, and denial of service: No user data or telemetry changes. The bounded loop prevents unbounded recovery work, and quiet/non-quiet failure reporting remains unchanged.

Verification

• Supervisor, auto-pair, and process-recovery suites: 48/48 passed.
• Focused supervisor suite: 18/18 passed.
• The tests prove cleanup-before-wait-before-retry ordering and success on the fifth retry after five failed backup calls.
• Formatting, CLI type-check, source-shape, test-size, conditional scan, repository hooks, commit-message, and pre-push gates passed.
• All three commits are signed and appear as Verified in GitHub.
• Diff size: 2 files, +102/-1. This is not a large LOC increase.

No security findings or security-review blocker remain for this branch revision.

[coderabbitai]

Actionable comments posted: 1

🤖 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/supervisor-relaunch.test.ts`:
- Around line 200-233: Strengthen the test around
relaunchManagedSupervisorSession by recording events in the backupState,
removeBackup, sleep, and recreate fakes, then assert the full retry ordering:
each of the five failed backupState calls is followed by removal of its partial
manifest and sleep(2), the sixth backup succeeds, and recreate runs afterward.
Retain the existing outcome assertions while replacing call-count-only coverage
with behavioral sequence checks.

🪄 Autofix

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: 6d7baa27-3838-4a48-9b01-019e99dab179

📥 Commits

Reviewing files that changed from the base of the PR and between 9174688 and 10b6ec2.

📒 Files selected for processing (2)

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

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

• src/lib/actions/sandbox/supervisor-relaunch.ts

[apurvvkumaria]

Security review refresh: PASS

Reviewed the complete PR diff through branch revision c2ce64433 against current main (0cddc38c8). The latest commit changes only the boundary test; the production retry implementation reviewed in the prior security assessment is unchanged.

The test now records all backup, partial-backup removal, fixed two-second wait, and recreation events. It proves that each of the five failed backup calls is cleaned up before waiting, the sixth backup succeeds, and container recreation occurs only afterward. This strengthens the fail-closed filesystem and sandbox-transaction evidence without adding runtime input, credentials, network access, privileges, dependencies, logging, or unbounded work.

Verification remains green locally: 18/18 focused tests, formatting, CLI type-check, test-size, repository hooks, commit-message, and pre-push checks passed. All four commits are signed and appear as Verified in GitHub. The complete diff is 2 files, +155/-1, so no large-LOC flag applies.

No security findings or security-review blocker remain for this branch revision.

[apurvvkumaria]

Post-merge CI classification: repository lifecycle/infrastructure, not a PR code failure.

All 12 CLI test shards passed. The final cli-tests job failed only when the coverage service rejected an upload after this PR had already merged; the aggregate checks job then reflected that upload failure. No test assertion failed.

No branch rerun is useful now that the PR is closed. The integrated revision remains subject to the normal main checks.