fix(uninstall): treat an already-removed sandbox as removable state

[laitingsheng]

Summary

Scoped uninstall counted every non-zero openshell sandbox delete exit as a cleanup failure, so a host whose recorded sandbox had already been removed preserved the selected gateway state and exited 1 after the gateway registration had already been deleted. The delete now captures its output and classifies the outcome, so an already-removed sandbox finishes the remaining gateway cleanup and exits 0, while an unreachable or rejected delete still preserves the state for a retry and exits nonzero.

Related Issue

Fixes #7906

Changes

• Add deleteSelectedGatewaySandbox in src/lib/actions/uninstall/run-plan.ts, which runs openshell sandbox delete <name> with its output captured and treats an already-removed sandbox as success. The scoped delete loop calls it instead of runOptional, whose stdio: "ignore" spawn leaves no output to classify.
• Reuse the existing getSandboxDeleteOutcome classifier from src/lib/domain/sandbox/destroy.ts rather than adding a second absence pattern. nemoclaw destroy already depends on it, so both paths agree on what "already gone" means.
• Split the outcome wording in src/lib/domain/uninstall/messaging.ts: sandboxDeleteAbsentMessage for a sandbox that leaves nothing to clean up, sandboxDeleteFailureMessage for one that still leaves state behind. The previous single message named both states at once, so no caller could act on either.
• Raise the recorded run-plan.ts fan-out ceiling in ci/source-architecture-budget.json from 25 to 26 for the domain/sandbox/destroy import. The alternative was a duplicate absence pattern in a second file.
• Record the absent-versus-unreachable outcome contract in the uninstall page.

getSandboxDeleteOutcome also reports gatewayUnreachable, which this change deliberately keeps on the failure side: an unreachable gateway leaves real sandbox state on the host, so preserving the selected gateway for a retry stays correct. src/lib/actions/uninstall/run-plan-absent-sandbox.test.ts protects all three outcomes, and test/uninstall.test.ts protects the reported flow end to end.

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: Exact-head nine-category review PASS at 3a875ee253856a7c2ba007eebe01c0920c9baa07 against base fdd68285e2c3e9f7c09ac63029236e108d02d9d1; no security findings. The classifier and cleanup behavior are unchanged by the final documentation commit. The early return preserves retry authority after a failed sandbox deletion and introduces no new secret, input-validation, authorization, dependency, logging, cryptography, configuration, network, resource-control, or privilege risk.
• 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: Independently reviewed the complete code, test, and documentation change at exact head 3a875ee253856a7c2ba007eebe01c0920c9baa07 against base fdd68285e2c3e9f7c09ac63029236e108d02d9d1. The revised test title now describes outcome classification instead of claiming that failed cleanup completes. docs/manage-sandboxes/uninstall-nemoclaw.mdx now states that unreachable or refused deletion preserves the selected gateway registration, port-scoped state, and selected sandbox registry entry, including with --destroy-user-data, and scopes the non-preservation statement to successful or already-absent deletion. Focused tests passed 3/3, npm run docs passed with 0 errors and 2 unchanged existing warnings, and git diff --check passed.
• Agent: Codex Desktop documentation writer subagent

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 — at exact head 3a875ee253856a7c2ba007eebe01c0920c9baa07, the 3/3 absent-sandbox regression cases passed. Earlier validation of the unchanged behavior patch passed 158 action/domain tests and 11 integration tests. CLI and plugin builds passed, and normal signed commit and push hooks passed.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs passed with no PR-introduced warnings (doc changes only) — 0 errors and 2 unchanged existing warnings
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Tinson Lai tinsonl@nvidia.com
Signed-off-by: Senthil Ravichandran senthilr@nvidia.com

Summary by CodeRabbit

• Bug Fixes

  • Uninstall now treats already-removed sandboxes as successful cleanup.
  • Unreachable or refused sandbox deletion reports a failure and preserves gateway state.
  • Added clearer messages distinguishing absent sandboxes from deletion failures.

• Documentation

  • Updated uninstall guidance to explain sandbox cleanup outcomes.

• Tests

  • Added coverage for absent, unreachable, and rejected sandbox deletion scenarios.

[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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 31a57a0d-8868-40d8-b4ea-0c55e7e129a7

📥 Commits

Reviewing files that changed from the base of the PR and between 31e1d36 and 3a875ee.

📒 Files selected for processing (2)

• docs/manage-sandboxes/uninstall-nemoclaw.mdx
• src/lib/actions/uninstall/run-plan-absent-sandbox.test.ts

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

• docs/manage-sandboxes/uninstall-nemoclaw.mdx
• src/lib/actions/uninstall/run-plan-absent-sandbox.test.ts

---

📝 Walkthrough

Walkthrough

Uninstall now treats an already-absent OpenShell sandbox as successful cleanup. Other deletion failures preserve gateway state and return failure. Messages, tests, documentation, and the architecture budget reflect this behavior.

Changes

Selected gateway sandbox cleanup

Layer / File(s)	Summary
Sandbox deletion outcome handling src/lib/domain/uninstall/messaging.ts, src/lib/actions/uninstall/run-plan.ts	The uninstall plan distinguishes successful, absent, and failed sandbox deletion. Absent sandboxes allow cleanup to continue. Other failures preserve gateway state.
Cleanup behavior validation src/lib/actions/uninstall/run-plan-absent-sandbox.test.ts, test/uninstall.test.ts	Tests cover deletion outcomes, commands, messages, exit codes, gateway state, and registry updates.
Documentation and architecture budget docs/manage-sandboxes/uninstall-nemoclaw.mdx, ci/source-architecture-budget.json	Documentation describes cleanup outcomes. The fan-out budget increases from 25 to 26.

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

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant UninstallPlan
  participant OpenShellCLI
  participant GatewayState
  Operator->>UninstallPlan: run destructive uninstall
  UninstallPlan->>OpenShellCLI: delete selected sandbox
  OpenShellCLI-->>UninstallPlan: success, absent, or failure
  alt success or absent
    UninstallPlan->>GatewayState: remove selected gateway state
    UninstallPlan-->>Operator: successful cleanup
  else deletion failure
    UninstallPlan->>GatewayState: preserve selected gateway state
    UninstallPlan-->>Operator: failure with warning
  end

Loading

Possibly related PRs

• NVIDIA/NemoClaw#8259: Both changes modify scoped uninstall sandbox cleanup in run-plan.ts.

Suggested labels: area: install

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the primary uninstall change: treating an already-removed sandbox as removable state.
Linked Issues check	✅ Passed	[#7906] The implementation treats absent sandboxes as successful cleanup and preserves gateway state for failed deletions, with regression coverage.
Out of Scope Changes check	✅ Passed	The code, messages, documentation, budget update, and tests directly support the linked uninstall issue.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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/uninstall-idempotent-sandbox-delete

---

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

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-8247.docs.buildwithfern.com/nemoclaw

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 3a875ee in the fix/uninstall-idempo... branch remains at 96%, unchanged from commit ce2b4b9 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 3a875ee in the fix/uninstall-idempo... branch remains at 81%, unchanged from commit fdd6828 in the main branch.

Show a code coverage summary of the most impacted files.

File	main fdd6828	fix/uninstall-idempo... 3a875ee	+/-
src/lib/policy/...ne-exclusion.ts	96%	92%	-4%
src/lib/shields/index.ts	73%	70%	-3%
src/lib/private-networks.ts	93%	90%	-3%
src/lib/actions...all/run-plan.ts	84%	84%	0%
src/lib/domain/...ll/messaging.ts	100%	100%	0%
src/lib/sandbox...rce-identity.ts	87%	87%	0%

---

_{Updated August 05, 2026 06:35 UTC}

[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/uninstall/run-plan.ts`:
- Line 1166: The uninstall flow must stop before removeGatewayRegistration when
deleteSelectedGatewaySandbox reports failure, preserving the selected gateway
registration for retry; update run-plan.ts at line 1166 to return incomplete
cleanup before registration removal. Update run-plan-absent-sandbox.test.ts
lines 147-161 to assert failed deletions make no gateway remove call and retain
the selected-box registry entry.

🪄 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: 5db8680c-b536-4b6e-bcc4-ed320b4921a3

📥 Commits

Reviewing files that changed from the base of the PR and between 08435d3 and 91c8b36.

📒 Files selected for processing (6)

• ci/source-architecture-budget.json
• docs/manage-sandboxes/uninstall-nemoclaw.mdx
• src/lib/actions/uninstall/run-plan-absent-sandbox.test.ts
• src/lib/actions/uninstall/run-plan.ts
• src/lib/domain/uninstall/messaging.ts
• test/uninstall.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 terminology decisions differ; normalized E2E selections differ; severity counts match.

3 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

• unreachable at src/lib/domain/uninstall/messaging.ts:25: selected only by the second-opinion lane as established.
• already-removed at docs/manage-sandboxes/uninstall-nemoclaw.mdx:96: selected only by the second-opinion lane as justified.
• absent at src/lib/domain/uninstall/messaging.ts:20: selected only by the second-opinion lane as define.

6 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

• hermes-e2e: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• onboard-repair: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• onboard-resume: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• state-backup-restore: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• full-e2e: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
• spark-install: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

2 semantic terminology decisions

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

• established — already removed at docs/manage-sandboxes/uninstall-nemoclaw.mdx:96: Keep `already removed` for the absent-resource outcome.
• justified — selected gateway registration at docs/manage-sandboxes/uninstall-nemoclaw.mdx:98: Keep `selected gateway registration` where the scoped preservation contract must distinguish it from sibling registrations.

E2E guidance

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

Recommended E2E: None

1 optional E2E recommendation

• sandbox-operations

Workflow run details

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

[cv]

Changes required at exact head 91c8b36. When deleteSelectedGatewaySandbox reports a real failure, the run plan records cleanup failure but continues to removeGatewayRegistration. That removes the selected gateway state needed for the promised retry and contradicts the PR summary, documentation, and failure message. Return incomplete cleanup before registration removal on this path. Update the regression test to prove a failed delete makes no gateway-remove call and retains the selected sandbox registration. This is a blocking recovery-state integrity finding in the sensitive uninstall path. After the fix, refresh the exact-head security and documentation-writer receipts and rerun the owning tests. The already-absent success classification itself is not blocked.

[apurvvkumaria]

@cv The blocking recovery-state finding is addressed in commit 37608c38. A failed selected-sandbox deletion now returns before gateway registration removal, so the selected-box registry entry remains available for retry. The regression now verifies that failure makes no gateway-remove call and retains the selected registration.

Validation completed: the focused regression passed 3/3, the owning uninstall suites passed 158 and 11 tests, and CLI type checking plus push hooks passed. The branch was subsequently refreshed from current main in commit 57c8104a; the effective PR change is unchanged. Documentation review at that revision passed the docs build and verified all generated variants. The refreshed sensitive-path review found no new authorization, secret-handling, input-validation, dependency, logging, cryptography, configuration, or privilege risk. Current CI is still running.