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

[rluo8]

Summary

nemoclaw uninstall previously treated stale sibling gateway directories and registry rows as live siblings. It now reconciles those records with OpenShell's live gateway registry, removes host-shared resources when only orphaned state remains, and preserves those resources when a live sibling exists or liveness cannot be proved.

Related Issue

Fixes #7315

Product Scope

PASS — this fixes the existing supported nemoclaw uninstall contract described by #7315 and the current uninstall documentation. It adds no integration, recipe, custom image, third-party stack, configuration, API, or supported product surface. This verdict is independent of GitHub's mergeStateStatus.

Changes

• Query openshell gateway list -o json once per sibling inspection and use its names to distinguish live gateways from orphaned per-port directories and shared registry rows.
• Treat an unavailable OpenShell command, failed query, malformed or partially malformed JSON, unexpected filesystem object, and invalid port directory as uncertain. Uncertain evidence preserves host-shared resources; a missing command exits nonzero before OpenShell cleanup.
• Reinspect sibling gateways after confirmation and before cleanup. If a sibling appears, switch to gateway-scoped cleanup and preserve shared state.
• Cover orphaned, live, malformed, failed-query, and late-appearance cases in run-plan-gateway-segregation.test.ts.
• Document that shared-resource and provider cleanup requires confirmed no-sibling evidence.

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: Codex Desktop reviewed PR SHA c4edc0e13 against base SHA cd87ccc56 across all nine security categories. All categories passed with no findings. This is agent evidence, not human review or approval.
• 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: Updated and reviewed docs/manage-sandboxes/uninstall-nemoclaw.mdx and docs/reference/commands.mdx for fail-closed shared-resource and provider cleanup, including the unavailable-OpenShell nonzero retry path. The documentation writer verified completed head c4edc0e13 against base cd87ccc56, including the deterministic PTY-driver correction, signed main refreshes, behavior-neutral redundant-guard cleanup, and test-size budget restoration, with no findings. npm run docs 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 check:diff passed 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: npx vitest run --project cli src/lib/actions/uninstall/*.test.ts passed 91 tests in 7 files; npx vitest run --project integration test/uninstall.test.ts passed 6 tests; npx vitest run --project integration test/sandbox-provisioning.test.ts passed 62 tests; and npm run test-size:check passed across 1,788 files. The PTY driver completed its typed-yes full-cleanup path with exit 0; its Linux-only Vitest wrapper is skipped locally on macOS and will run in CI. The canonical-base diff-aware pre-push hook passed CLI type-checking.
• 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; the change is limited to uninstall sibling detection and its focused tests.
• 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: Rui Luo ruluo@nvidia.com

[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

Uninstall sibling detection now compares registry and per-port gateway entries with cached OpenShell gateway names. Confirmed orphans allow shared cleanup, while live or uncertain siblings preserve scoped cleanup. Preparation rechecks for siblings before teardown.

Changes

Uninstall sibling gateway detection

Layer / File(s)	Summary
OpenShell-backed sibling liveness src/lib/actions/uninstall/run-plan.ts	Sibling registry and filesystem entries are matched against cached openshell gateway list -o json results; unavailable or invalid results remain conservative.
Teardown scope and preparation recheck src/lib/actions/uninstall/run-plan.ts	Uninstall preparation rechecks sibling environments, switches to gateway-scoped cleanup when needed, and recomputes selected sandbox names.
Cleanup behavior validation src/lib/actions/uninstall/run-plan-gateway-segregation.test.ts	Tests cover orphaned and live siblings, malformed gateway-list output, late sibling detection, filesystem preservation, and gateway-selection assertions.

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

Sequence Diagram(s)

sequenceDiagram
  participant UninstallPlanner
  participant OpenShellCLI
  participant GatewayState
  UninstallPlanner->>OpenShellCLI: Query gateway list
  OpenShellCLI-->>UninstallPlanner: Live gateway names or unknown
  UninstallPlanner->>GatewayState: Inspect registry and per-port directories
  GatewayState-->>UninstallPlanner: Live, orphaned, or uncertain sibling status
  UninstallPlanner->>OpenShellCLI: Recheck and select gateway for scoped teardown

Loading

Suggested labels: area: cli, area: sandbox, bug-fix

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes address #7315 by cleaning host-shared binaries when siblings are orphaned while preserving live sibling resources.
Out of Scope Changes check	✅ Passed	The code and tests stay focused on uninstall sibling-detection and cleanup scoping, with no obvious unrelated additions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately summarizes the main uninstall change: treating OpenShell-orphaned siblings as non-scoping signals.

✨ 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/7315-uninstall-orphan-sibling-gateway

---

_{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 c4edc0e in the fix/7315-uninstall-o... branch remains at 96%, unchanged from commit cd87ccc in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit c4edc0e in the fix/7315-uninstall-o... branch remains at 80%, unchanged from commit cd87ccc in the main branch.

Show a code coverage summary of the most impacted files.

File	main cd87ccc	fix/7315-uninstall-o... c4edc0e	+/-
src/lib/sandbox...rce-identity.ts	87%	87%	0%
src/lib/state/m...ock-identity.ts	95%	95%	0%
src/lib/tunnel/services.ts	73%	73%	0%
src/lib/actions...all/run-plan.ts	84%	85%	+1%
src/lib/domain/.../connect-env.ts	89%	97%	+8%

---

_{Updated July 24, 2026 22:49 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 · 2 warnings · 0 suggestions
• Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 2 more warnings, the same number of suggestions.

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: None

1 optional E2E recommendation

• concurrent-gateway-ports

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 (2)

src/lib/actions/uninstall/run-plan.ts (1)

1106-1135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider isolating the OpenShell process call in an adapter.

As per path instructions (src/lib/README.md), "Isolate host-boundary calls (e.g., OpenShell gateway listing, filesystem scanning) in src/lib/adapters/** so actions/domain remain testable." collectLiveOpenShellGatewayNames is exactly this named example but lives directly in the actions file. This mirrors existing runtime.run("openshell", ...) calls already in this file, so it's consistent with current (non-adapter) convention rather than a net-new regression; still flagging per path instructions.

🤖 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/uninstall/run-plan.ts` around lines 1106 - 1135, Move the
host-boundary logic from collectLiveOpenShellGatewayNames into an appropriate
adapter under src/lib/adapters, including the openshell command invocation and
JSON parsing. Update the action to call the adapter while preserving the
existing Set<string> | null conservative behavior and runtime environment
handling.

Source: Path instructions

src/lib/actions/uninstall/run-plan-gateway-segregation.test.ts (1)

594-789: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract shared test setup to cut duplication.

The four new tests repeat the same tmpdir-create/sandboxes.json-write/mock-run/cleanup boilerplate almost verbatim, differing only in fixture content and expected OpenShell calls. A small helper (e.g. runUninstallWithFixture({ gatewaysDirPorts, sandboxes, liveGatewayNames })) would cut ~150 lines and make future orphan/sibling scenarios cheaper to add.

🤖 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/uninstall/run-plan-gateway-segregation.test.ts` around lines
594 - 789, Extract the repeated temporary-home setup, fixture writing, uninstall
invocation, OpenShell mocking, and cleanup from the four tests into a shared
helper near these tests, such as runUninstallWithFixture. Parameterize gateway
directory ports, sandbox registry data, and live gateway names, while returning
the uninstall result, logs, calls, and temporary state path needed by
assertions. Update each test to use the helper and preserve its existing
scenario-specific expectations.

🤖 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/uninstall/run-plan-gateway-segregation.test.ts`:
- Around line 594-789: Extract the repeated temporary-home setup, fixture
writing, uninstall invocation, OpenShell mocking, and cleanup from the four
tests into a shared helper near these tests, such as runUninstallWithFixture.
Parameterize gateway directory ports, sandbox registry data, and live gateway
names, while returning the uninstall result, logs, calls, and temporary state
path needed by assertions. Update each test to use the helper and preserve its
existing scenario-specific expectations.

In `@src/lib/actions/uninstall/run-plan.ts`:
- Around line 1106-1135: Move the host-boundary logic from
collectLiveOpenShellGatewayNames into an appropriate adapter under
src/lib/adapters, including the openshell command invocation and JSON parsing.
Update the action to call the adapter while preserving the existing Set<string>
| null conservative behavior and runtime environment handling.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 076f53a7-5ae2-4880-a3da-d1168dedbed9

📥 Commits

Reviewing files that changed from the base of the PR and between 5087eb4 and 328f0d5.

📒 Files selected for processing (2)

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

[cjagwani]

The new liveness probe needs to fail closed on partially malformed JSON. collectLiveOpenShellGatewayNames() accepts any array and silently skips entries whose name is missing or non-string, so [{}] or a mixed valid/malformed response becomes an empty or incomplete trusted set. Both downstream callers interpret absence from that set as proof that sibling state is orphaned, which can select the non-scoped uninstall path and remove host-shared resources even though the response did not prove the sibling absent.

Please return null when any gateway-list entry lacks a valid non-empty string name, and add a regression test with a malformed or mixed array proving that uninstall conservatively preserves sibling/shared state. This matches the function contract that unparseable output must not authorize cleanup.

[github-actions]

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

Dismissed as stale after the requested fail-closed fix landed. Current head b752fe1 rejects every null, non-object, missing, non-string, or empty gateway name and returns unknown liveness; the mixed valid/malformed regression proves shared resources stay scoped. This dismissal removes the superseded changes-requested state and is not an approval.

[senthilr-nv]

Approved PR #7351 at head 4eed094 against base 2e743da. I reviewed the Files changed diff, including the fail-closed behavior when OpenShell is unavailable, and approve the product scope. Merge remains conditional on fresh required CI, exact-pair E2E gates, Verified commits, and no actionable automated-review findings.

[senthilr-nv]

Approved PR #7351 at head 955fd5f against base d399e58. I reviewed the Files changed diff, including the fail-closed unavailable-OpenShell behavior, deterministic PTY fixture, and redundant gateway-guard cleanup, and approve the product scope. Merge remains conditional on fresh required CI, exact-pair E2E gates, Verified commits, and no actionable automated-review findings.

[senthilr-nv]

Approved PR #7351 at head c4edc0e against base cd87ccc. I reviewed the current diff and approve the product scope independently. Merge remains conditional on the exact-state audit, Verified commits, required CI, E2E gates, and no actionable automated-review findings.