fix(cli): let strict backup-all skip stranded orphan sandboxes

[Dongni-Yang]

Summary

Reopen follow-up for #6520: nemoclaw uninstall --yes followed by a curl reinstall hard-aborts at the installer strict pre-upgrade backup. The uninstall removes the gateway registration and the sandbox container but preserves sandboxes.json, so the stranded record can only ever be skipped — and the strict gate (NEMOCLAW_REQUIRE_ALL_SANDBOX_BACKUPS=1, #6114) exits 1 before the installer recovery phase (recover_preexisting_sandboxes_before_onboard), the phase that knows how to surface orphans, ever runs. The #6539 orphan classifier is wired only into upgrade-sandboxes, which is downstream of the aborting backup.

What changed

• backupAll() now classifies stranded records with the existing classifyOrphanedRegistrySandboxes (unobserved on the selected gateway AND persisted binding resolving to that same gateway), gated on the new isSandboxContainerDefinitivelyAbsent(). Stranded records are tracked separately from skipped, so the strict gate keeps failing closed for every genuine skip, and the run ends with orphanedRegistrySummary + orphanedRegistryRemediation (same destroy/onboard guidance as the recovery phase). End-to-end, the installer now proceeds to its recovery phase, which reports the orphans and yields "completed with warnings" instead of a hard abort.
• The exemption is two-phase (PRA-1): after the backup loop, a confirming second pinned listing re-checks every stranded candidate — the same [All Platforms][Upgrade] v0.0.55 → v0.0.76 leaves pre-existing sandboxes stuck in Provisioning/Error — user data inaccessible until manual rebuild #6114 confirmation idiom as upgrade-sandboxes — and any candidate the gateway observes again reverts to a genuine strict skip. Container absence is itself checked per candidate at skip time.
• isSandboxContainerDefinitivelyAbsent() fails closed everywhere absence cannot be proven: non-docker or unknown driver (including a throwing registry read), and a failed or timed-out labeled listing. The listing is status-checked (docker ps -a with the OpenShell labels) rather than reusing findLabeledSandboxContainers, which swallows docker errors — a dead daemon yields null, never "absent" — and passes ignoreError so the probe cannot process.exit the run.
• backup-all now pins its sandbox listing to the selected gateway (same [All Platforms][Upgrade] v0.0.55 → v0.0.76 leaves pre-existing sandboxes stuck in Provisioning/Error — user data inaccessible until manual rebuild #6114 rationale and idiom as upgrade-sandboxes): an unpinned list taken from a sibling gateway selection must not feed a fail-open stranded decision.
• The exemption carries an in-code source-of-truth review block (PRA-2): source boundary (nemoclaw uninstall preserves sandboxes.json by design), source-fix constraint (backup-all must not reconcile the registry; record removal is owned by the recovery phase destroy/onboard flow), and removal condition (install/uninstall registry reconciliation, or running the recovery phase before the strict backup).

Constraints preserved (pinned by tests)

• Strict gate still aborts on any genuine skip ([All Platforms][Upgrade] v0.0.55 → v0.0.76 leaves pre-existing sandboxes stuck in Provisioning/Error — user data inaccessible until manual rebuild #6114 cases untouched and green).
• A sandbox bound to a sibling gateway is never claimed stranded, even when its container is absent on this host.
• An unobserved sandbox whose container still exists keeps the strict abort (reconnect race).
• A stranded candidate the confirming listing observes again reverts to a strict skip (lifecycle race).
• Real backup failures (EACCES, non-manifest paths) still re-throw/abort.
• A registry row with a null/unknown driver keeps the strict abort — fail closed, since absence cannot be proven for it.
• ORPHANED_SANDBOX_MARKER / summary / remediation wording unchanged; install.sh greps the marker only from the recovery-phase log, which this change does not touch.

Review responses

• PR Review Advisor PRA-1 (revalidate absence before exempting the gate): addressed by the confirming second pinned listing after the backup loop plus per-candidate absence checks at skip time; new test pins the reappeared-candidate revert.
• PR Review Advisor PRA-2 (bound the workaround to its source fix): addressed with the in-code source-of-truth review block (source boundary / fix constraint / removal condition).
• reviewed-npm-audit failure is upstream-wide (also fails on test(images): add security revision container verifier #7289) and is being remediated by fix(security): remediate OpenClaw 2026.6.10 transitive dependencies #7286; this PR adds no dependencies.

Verification

• vitest: maintenance.test.ts (32), stopped-sandbox-backup.test.ts (24), orphan-detection.test.ts, upgrade-sandboxes-recovery.test.ts, openshell-sandbox-list.test.ts — 108 tests green; installer lane test/install-orphaned-sandbox-recovery.test.ts (8, drives strict backup through recovery and the real install.sh orphan grep) green; typecheck:cli, lint, and biome check clean. The new maintenance tests pin GATEWAY_PORT so they stay green under an exported NEMOCLAW_GATEWAY_PORT.
• Live end-to-end on a workstation (real docker daemon, real registry file, openshell CLI shimmed to a healthy empty gateway, real built CLI): stranded record → warning + 0 skipped + exit 0 under strict mode (through both listings); same record bound to gatewayPort: 9999 → strict abort exit 1 with no orphan claim; same record with a labeled container present → strict abort exit 1 with no orphan claim.

Refs #6520

Signed-off-by: Dongni Yang dongniy@nvidia.com

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved backup-all orphan handling: potentially stranded sandboxes are skipped when their containers are definitively absent, then rechecked with the same selected gateway before remediation occurs.
  • Tightened strict-mode “fail closed” behavior for confirmed absent containers, including mismatched-gateway scenarios and reappearing candidates.
  • Reduced false “container absent” results by treating Docker/registry uncertainties as unknown rather than absent.
• Tests
  • Expanded coverage for stranded-orphan detection and definitive container absence (including fail-closed, reappearance, and gateway-pinning scenarios) with improved Docker mock observability.

Takeover review response

• The PR Review Advisor handoff warning is addressed by an installer boundary regression that drives strict backup-all, orphan recovery, and the completed-with-warnings result in order.

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: Reviewed the complete five-file diff at 71a02caebe9f. No documentation paths changed; existing docs already cover strict backup and stranded-record recovery and remediation.
• Agent: Codex Desktop
• PR: fix(cli): let strict backup-all skip stranded orphan sandboxes #7290

Signed-off-by: Julie Yaunches jyaunches@nvidia.com

[coderabbitai]

📝 Walkthrough

Walkthrough

backupAll() now performs gateway-scoped orphan classification and skips sandboxes whose Docker containers are definitively absent. A fail-closed absence classifier probes labeled Docker containers, while tests cover gateway binding, strict-mode behavior, probe failures, and remediation reporting.

Changes

Stranded orphan backup handling

Layer / File(s)	Summary
Definitive Docker container absence detection src/lib/actions/sandbox/stopped-sandbox-backup.ts, src/lib/actions/sandbox/stopped-sandbox-backup.test.ts	Adds a fail-closed, labeled docker ps -a probe and tests Docker, registry, timeout, command failure, and non-Docker cases.
Gateway-scoped backup integration and validation src/lib/actions/maintenance.ts, src/lib/actions/maintenance.test.ts	Pins sandbox listing to the selected gateway, skips definitively absent stranded orphans, confirms candidates with a second listing, reports remediation instructions, and preserves strict abort behavior for uncertain or reappearing classifications.

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

Sequence Diagram(s)

sequenceDiagram
  participant backupAll
  participant gatewaySandboxListing
  participant isSandboxContainerDefinitivelyAbsent
  participant dockerRun
  backupAll->>gatewaySandboxListing: capture selected-gateway sandbox list
  backupAll->>isSandboxContainerDefinitivelyAbsent: classify sandbox container absence
  isSandboxContainerDefinitivelyAbsent->>dockerRun: run labeled docker ps -a probe
  dockerRun-->>isSandboxContainerDefinitivelyAbsent: container listing or probe failure
  isSandboxContainerDefinitivelyAbsent-->>backupAll: definitive absence or uncertain state
  backupAll->>gatewaySandboxListing: confirm stranded candidate
  backupAll-->>backupAll: skip, report, or preserve strict 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: strict backup-all now skips stranded orphan sandboxes.

✨ 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 fix/6520-strict-backup-skip-stranded-sandbox

---

_{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 71a02ca in the fix/6520-strict-back... branch remains at 96%, unchanged from commit 5faed2c in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 71a02ca in the fix/6520-strict-back... branch remains at 80%, unchanged from commit 5faed2c in the main branch.

Show a code coverage summary of the most impacted files.

File	main 5faed2c	fix/6520-strict-back... 71a02ca	+/-
src/lib/domain/...an-detection.ts	100%	100%	0%
src/lib/onboard...der-metadata.ts	100%	100%	0%
src/lib/sandbox...rce-identity.ts	87%	87%	0%
src/lib/state/m...ock-identity.ts	95%	95%	0%
src/lib/state/g...way-registry.ts	94%	95%	+1%
src/lib/actions/maintenance.ts	79%	84%	+5%
src/lib/actions...ndbox-backup.ts	72%	85%	+13%

---

_{Updated July 24, 2026 17:00 UTC}

[github-actions]

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

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, state-backup-restore, upgrade-stale-sandbox

2 optional E2E recommendations

• gateway-guard-recovery
• sandbox-operations

Workflow run details

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

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/actions/maintenance.ts (1)

66-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting stranded-orphan classification out of backupAll.

The logic here is correct, but backupAll (lines 57-260) is already a very large function, and this change adds another self-contained concern (gateway-scoped orphan classification + stranded tracking + summary reporting) directly inline. As per coding guidelines, **/*.{js,ts,tsx} should "Keep function complexity low and avoid introducing unnecessary complexity hotspots." Pulling the orphan-classification setup (lines 79-96) and/or the stranded-check branch (115-120) into a small named helper would keep backupAll's control flow easier to follow without changing behavior.

Also applies to: 105-120, 224-227

🤖 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/maintenance.ts` around lines 66 - 96, Extract the
gateway-scoped orphan classification and stranded-sandbox handling from
backupAll into a small named helper, covering the setup around
classifyOrphanedRegistrySandboxes and the stranded-check branch and summary
reporting. Have the helper accept the existing sandbox/listing context and
return the values backupAll needs, preserving gateway selection,
container-absence checks, and current behavior.

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.

Nitpick comments:
In `@src/lib/actions/maintenance.ts`:
- Around line 66-96: Extract the gateway-scoped orphan classification and
stranded-sandbox handling from backupAll into a small named helper, covering the
setup around classifyOrphanedRegistrySandboxes and the stranded-check branch and
summary reporting. Have the helper accept the existing sandbox/listing context
and return the values backupAll needs, preserving gateway selection,
container-absence checks, and current behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2b8ee3a3-c3ca-42c2-b33e-db072b62a2ea

📥 Commits

Reviewing files that changed from the base of the PR and between d5fa194 and cd8fc00.

📒 Files selected for processing (4)

• src/lib/actions/maintenance.test.ts
• src/lib/actions/maintenance.ts
• src/lib/actions/sandbox/stopped-sandbox-backup.test.ts
• src/lib/actions/sandbox/stopped-sandbox-backup.ts