fix(uninstall): delete the scoped sandbox before removing its unit (#8220)

[harjothkhara]

Summary

A gateway-scoped nemoclaw uninstall removed the selected gateway's marked Linux unit before using that gateway's OpenShell gateway service to delete its sandbox. When deletion failed, later cleanup did not run and the sandbox container remained. This change deletes the selected gateway's OpenShell resources first and removes the marked Linux unit only after cleanup succeeds. Full uninstall behavior is unchanged.

Related Issue

Fixes #8220

Changes

• Move marked Linux unit removal for a gateway-scoped uninstall from Stopping services to OpenShell resources, after selected-sandbox cleanup succeeds.
• Keep the marked Linux unit and gateway process running when selected-sandbox cleanup fails so the user can inspect the remaining state and retry.
• Centralize the default-port, --keep-openshell, and external-supervision checks shared by full and gateway-scoped cleanup.
• Add regressions for successful cleanup ordering and failed selected-sandbox cleanup.
• Document the ordering, partial-completion risk, preserved state, and retry steps in the uninstall guide and command reference.

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: independent review of current PR commit 05294e7f5191bc7cb04950dfa827d29a24729d56 against current main 3a39ff352f98c4630ab8e18b83f4ec43ac56a2cc passed all nine security categories. The change reuses validated gateway selection, retains the existing cleanup guards, and does not change credentials, dependencies, network policy, or authorization.
• 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/uninstall-nemoclaw.mdx and docs/reference/commands.mdx accurately describe the cleanup order, possible partial completion, preserved service state, and retry steps. The implementation comment and regression titles state the behavior directly. No unrelated documentation changed.
• Agent: Codex Desktop

DGX Station Hardware Evidence

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

Security Review

• Verdict: PASS; no findings in the nine required categories.
• Reviewed commit: 05294e7f5191bc7cb04950dfa827d29a24729d56.
• Reviewed base: 3a39ff352f98c4630ab8e18b83f4ec43ac56a2cc.
• The destructive cleanup remains scoped to the selected gateway. Failed OpenShell cleanup returns before unit or process removal, and the default-port and external-supervision guards remain in effect.
• Regressions cover deletion-before-unit-removal and failure preservation.

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 — normal commit hooks passed. The linked worktree does not contain the generated dist/ files required by pre-push; GitHub Actions is the validation authority.
• Targeted behavior tests pass for the current change set, or tests are marked not applicable above — pending GitHub Actions.
• Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — not applicable to this bounded uninstall ordering fix.
• 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) — pending GitHub Actions documentation checks.
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: harjoth harjoth.khara@gmail.com

Summary by CodeRabbit

• Bug Fixes

  • Selected sandbox resources are now removed before stopping the gateway service.
  • Failed cleanup exits with an error while preserving the gateway service for retry.
  • Improved cleanup behavior for default gateway services during full and scoped uninstalls.
  • Cleanup now reports when partial removal may have occurred.

• Documentation

  • Added guidance on cleanup ordering, failure handling, partial cleanup, inspecting gateways, and retrying uninstall operations.

[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.

[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: 9062526a-ffb9-47a3-8b0b-96f88b77938d

📥 Commits

Reviewing files that changed from the base of the PR and between d3fe2d5 and 05294e7.

📒 Files selected for processing (1)

• src/lib/actions/uninstall/run-plan-gateway-service.test.ts

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

• src/lib/actions/uninstall/run-plan-gateway-service.test.ts

---

📝 Walkthrough

Walkthrough

Gateway-scoped uninstall now deletes OpenShell resources before removing the managed Linux service. Tests cover cleanup ordering, Docker cleanup, service removal, and failure handling. Documentation describes partial cleanup and retry guidance.

Changes

Gateway-scoped uninstall

Layer / File(s)	Summary
Defer managed service removal src/lib/actions/uninstall/run-plan.ts	Service-removal conditions are centralized. Scoped cleanup keeps the service active during OpenShell deletion and removes it afterward.
Validate and document cleanup behavior src/lib/actions/uninstall/run-plan-gateway-service.test.ts, docs/manage-sandboxes/uninstall-nemoclaw.mdx, docs/reference/commands.mdx	Tests verify selected sandbox deletion order, Docker cleanup, service removal, and failure preservation. Documentation describes nonzero exits, partial cleanup, and retry guidance.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Uninstall as Uninstall run plan
  participant OpenShell
  participant Docker
  participant Systemd as Managed Linux service
  Uninstall->>OpenShell: Delete selected sandbox resources
  OpenShell->>Docker: Remove sandbox container
  OpenShell-->>Uninstall: Report cleanup result
  Uninstall->>Systemd: Remove managed service

Loading

Possibly related PRs

• NVIDIA/NemoClaw#8247: Both modify scoped gateway cleanup and sandbox deletion behavior.

🚥 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 describes the primary scoped-uninstall ordering fix.
Linked Issues check	✅ Passed	The changes address #8220 by deleting the selected sandbox before service removal and preserving state when cleanup fails.
Out of Scope Changes check	✅ Passed	The code, tests, and documentation changes directly support the scoped-uninstall fix and its failure handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[github-actions]

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
• Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
• Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 fewer warning, the same number of suggestions.

1 additional E2E selection from the second opinion

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

• onboard-repair: 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.

3 semantic terminology decisions

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

• established — gateway-scoped cleanup at docs/manage-sandboxes/uninstall-nemoclaw.mdx:73: Keep the established term.
• established — marked Linux unit at docs/manage-sandboxes/uninstall-nemoclaw.mdx:73: Keep the established term.
• justified — partially complete at docs/manage-sandboxes/uninstall-nemoclaw.mdx:76: Keep the modifier because it describes the non-transactional failure outcome.

E2E guidance

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

Recommended E2E: None

2 optional E2E recommendations

• concurrent-gateway-ports
• sandbox-operations

1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Cover registration-delete failure before service removal

• Location: src/lib/actions/uninstall/run-plan-gateway-service.test.ts:247
• Category: tests
• Problem: The new failure regression covers sandbox deletion failure, but not failure to remove the selected gateway registration after sandbox deletion succeeds.
• Impact: A later regression could again disable the marked gateway unit after registration cleanup fails, leaving the selected gateway unable to retry its incomplete cleanup.
• Recommendation: Add a scoped-uninstall regression that makes `openshell gateway delete` fail after successful sandbox deletion and asserts exit 1 with no `systemctl disable` call.
• Verification: Inspect a test double that returns failure for `openshell gateway delete` after returning success for `openshell sandbox delete`; verify the service-disable call is absent.
• Test coverage: A run-plan gateway-service test where selected sandbox deletion succeeds, selected gateway registration deletion fails, and the marked Linux unit remains present.
• Evidence: src/lib/actions/uninstall/run-plan.ts:1183-1199 returns false when selected gateway registration removal fails. src/lib/actions/uninstall/run-plan.ts:1890-1913 removes the managed unit only after `removeOpenShellResources` returns true. src/lib/actions/uninstall/run-plan-gateway-service.test.ts:247-289 covers failed sandbox deletion but no registration-delete failure.

Workflow run details

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

[wscurran]

✨ Thanks for the fix. This resolves the regression where scoped uninstall stranded the sandbox container and exited 1.

---

Related open issues:

• #8220 [Ubuntu 26.04][Install] scoped uninstall strands the sandbox container and exits 1 when a sibling gateway is present

---

Related open PRs:

• #7351 fix(uninstall): ignore OpenShell-orphaned siblings in cleanup scoping

---

Related open issues:

• #8220 [Ubuntu 26.04][Install] scoped uninstall strands the sandbox container and exits 1 when a sibling gateway is present

[apurvvkumaria]

The current CI failures are classified:

• static-checks and CLI shard 7 fail only because three architecture budgets are looser than the measured values.
• The aggregate cli-tests and checks failures follow from that shard.
• The protected E2E failure is the separate shared artifact-boundary defect tracked by fix(e2e): restore shared CLI artifact boundaries #8333.

A signed, locally validated CI-only fix is prepared to reduce these limits in ci/source-architecture-budget.json:

• src/lib/state/registry.ts fan-in: 99 to 98
• src/lib/onboard.ts fan-out: 222 to 219
• src/lib/onboard root files: 308 to 307

GitHub authorization currently prevents publishing that commit to the contributor branch. The branch owner or a maintainer with working contributor-fork authorization must apply those three mechanical reductions. I will resume CI monitoring once the branch is updated.