Skip to content

fix(uninstall): remove managed CLI shims on destroy-user-data - #9288

Merged
prekshivyas merged 5 commits into
mainfrom
fix/9277-uninstall-destroy-shim
Aug 17, 2026
Merged

fix(uninstall): remove managed CLI shims on destroy-user-data#9288
prekshivyas merged 5 commits into
mainfrom
fix/9277-uninstall-destroy-shim

Conversation

@rluo8

@rluo8 rluo8 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Sibling-scoped nemoclaw uninstall kept installer-managed ~/.local/bin CLI shims even with --destroy-user-data, so a destructive uninstall could exit 0 while leaving nemoclaw on PATH. With --destroy-user-data, the scoped path now removes those managed shims and still keeps the shared npm CLI package for remaining gateways.

Related Issue

Fixes #9277

Changes

  • Extract removeManagedCliShims() from removeNemoclawCli() so shim cleanup can run without uninstalling the shared global npm package; return the removed count so scoped destroy logs only when a managed shim was actually deleted.
  • On sibling-scoped uninstall with --destroy-user-data, keep the shared npm package but remove managed user-local CLI shims (~/.local/bin/nemoclaw and agent-alias siblings); classification still preserves foreign files without claiming removal.
  • Extend run-plan-gateway-scan-entries.test.ts for destroy-user-data shim removal under unidentified ~/.nemoclaw/gateways/ entries ([Ubuntu 22.04][Install] destructive uninstall reports success but leaves NemoClaw shim on disk #9277), foreign-shim non-removal, and managed-gateway authority helpers.
  • Document the scoped --destroy-user-data shim behavior in uninstall and commands reference pages, including the flag table, decision matrix, env var, and CLI help.

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:
  • 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, docs/reference/commands.mdx, src/lib/actions/root-help.ts — flag table, sibling narrative, decision matrix, env var, and CLI help now state that --destroy-user-data also removes installer-managed user-local CLI shims under ~/.local/bin/ while keeping the shared npm package when siblings remain.
  • Agent:

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: Brev nemoclaw-518ae1 (Ubuntu 24.04): A/B probe showed shim survivors on main (NEMOCLAW_SHIM_ABSENT_EXIT=1) and zero survivors after the fix patch (FIX_DESTROY_SHIM_LOG=yes). vitest run src/lib/actions/uninstall/run-plan-gateway-scan-entries.test.ts → 10/10 passed on that host. Windows still cannot prove scoped sandbox namespace for the same vitest file.
  • 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 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

Summary by CodeRabbit

  • New Features
    • Uninstalling with --destroy-user-data now removes installer-managed CLI shims from ~/.local/bin/.
    • The equivalent environment variable supports the same cleanup behavior.
  • Bug Fixes
    • Shared CLI packages are preserved when confirmed sibling gateways still use them.
    • Foreign or user-managed shims remain untouched.
  • Documentation
    • Updated uninstall help and reference documentation to explain scoped cleanup, sibling gateway detection, and retained shared packages.

Signed-off-by: Rui Luo <ruluo@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: ffab72a5-69c8-4eda-9a2a-7a50d501fb02

📥 Commits

Reviewing files that changed from the base of the PR and between 8ff35b3 and 3339df3.

📒 Files selected for processing (1)
  • src/lib/actions/uninstall/run-plan.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/uninstall/run-plan.ts

Included review availability: Your plan includes up to 12 reviews per rolling hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Destructive uninstall now removes installer-managed CLI shims under ~/.local/bin/ when no confirmed sibling gateway remains. It preserves shared npm packages and foreign files. Tests, CLI help, and documentation cover the updated behavior.

Changes

Uninstall shim cleanup

Layer / File(s) Summary
Managed shim removal
src/lib/actions/uninstall/run-plan.ts
The uninstall plan removes recognized NemoClaw and agent-alias shims, preserves foreign files, and reports successful removals.
Scoped destructive cleanup
src/lib/actions/uninstall/run-plan.ts
Scoped cleanup passes confirmed sibling gateway ports, removes managed shims only when no confirmed sibling remains, and retains the shared CLI package.
Validation and uninstall contract
src/lib/actions/uninstall/run-plan-gateway-scan-entries.test.ts, src/lib/actions/root-help.ts, docs/manage-sandboxes/uninstall-nemoclaw.mdx, docs/reference/commands.mdx
Tests cover managed, foreign, and sibling-gateway cases. Help and documentation describe conditional shim removal and shared-package retention.

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

Merge Risk: ⚪ Minimal · up to 3339d

This localized uninstall change removes managed user-local CLI shims during destructive sibling-scoped cleanup while preserving the shared package for remaining gateways. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.78% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the uninstall fix: removing managed CLI shims during destructive cleanup.
Linked Issues check ✅ Passed The changes address issue #9277 by removing managed shims during destructive uninstall while preserving confirmed sibling gateway resources.
Out of Scope Changes check ✅ Passed The code, tests, documentation, and CLI help changes directly support the linked uninstall objective and contain no unrelated scope.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/9277-uninstall-destroy-shim

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

@github-code-quality

github-code-quality Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 3339df3 in the fix/9277-uninstall-d... branch remains at 96%, unchanged from commit fda9eb1 in the main branch.


Updated August 17, 2026 07:40 UTC

@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

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 · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 3 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 3 more warnings, the same number of suggestions.
5 terminology differences from the second opinion

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

  • installer-managed user-local CLI shims at docs/manage-sandboxes/uninstall-nemoclaw.mdx:28: primary classified it as justified; the second opinion classified it as define.
  • sibling evidence at docs/manage-sandboxes/uninstall-nemoclaw.mdx:28: selected only by the second-opinion lane as define.
  • managed CLI shims at src/lib/actions/root-help.ts:88: selected only by the second-opinion lane as replace.
  • shared npm CLI package at docs/manage-sandboxes/uninstall-nemoclaw.mdx:28: selected only by the second-opinion lane as define.
  • confirmed sibling gateway at docs/manage-sandboxes/uninstall-nemoclaw.mdx:28: selected only by the second-opinion lane as define.
3 additional E2E selections from the second opinion

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

  • bootstrap-install-smoke: 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.
  • state-backup-restore: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

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.

  • justified — installer-managed user-local CLI shims at docs/manage-sandboxes/uninstall-nemoclaw.mdx:28: Keep this term. The ownership modifier distinguishes removable shims from foreign files.
  • justified — sibling evidence is unidentified at docs/manage-sandboxes/uninstall-nemoclaw.mdx:233: Keep this term. It identifies the branch that permits managed-shim removal with --destroy-user-data.
  • justified — confirmed sibling gateway port at src/lib/actions/uninstall/run-plan.ts:1739: Keep this term. It identifies the branch that preserves the shared npm package and user-local shims.

E2E guidance

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

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.

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed commit 2610352c587e8eb1340aaebe0c31a37c47e186f8 against the full diff, #9277, current review threads, targeted uninstall tests, CI, and the sensitive-path security rubric.

Blocking lifecycle/ownership regression

When --destroy-user-data is used and any sibling gateway remains, this now removes all managed ~/.local/bin NemoClaw agent shims while intentionally retaining the shared CLI package. That is valid for the unidentified-directory false positive in #9277, but it also runs when otherGatewayPorts proves a real sibling gateway exists. The remaining environment can therefore lose its managed commands even though the shared installation is retained for it. Limit shim removal to the false-positive/unidentified-only case (or otherwise prove the shims are not shared), and add coverage with a confirmed sibling showing its shims remain.

Security review

PASS: secrets, input validation, authentication/authorization, dependencies, cryptography/data protection, and error handling. BLOCKING WARNING: system security/resource ownership—the destructive cleanup crosses the selected gateway boundary. Test coverage currently exercises the false-positive case but not a confirmed sibling.

Validation: the focused scan-entry and gateway-segregation suites passed locally (36/36). CI still had running jobs and a failing static-checks job at review time.

Scoped --destroy-user-data must not drop ~/.local/bin managed shims when
otherGatewayPorts proves a real sibling still shares them. Limit removal to
the unidentified scoped path and cover the confirmed-sibling retention case.

Signed-off-by: Rui Luo <ruluo@nvidia.com>
@rluo8

rluo8 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the blocking lifecycle/ownership finding in 8ff35b3b8.

Change: scoped --destroy-user-data now removes managed ~/.local/bin shims only when otherGatewayPorts.length === 0 (unidentified / unproven sibling evidence — the #9277 false-positive path). When otherGatewayPorts proves a real sibling, the shared package and managed shims are retained (same log as the non-destroy scoped path).

Coverage: added keeps managed CLI shims with --destroy-user-data when a confirmed sibling gateway remains (#9277) (live list includes nemoclaw-9123). Docs / root-help updated to match.

Local Windows cannot prove sandbox namespace for these scoped suites (exit 1 before CLI step); please re-run the focused scan-entry + segregation suites on Linux/CI.

static-checks failed because executePlan exceeded the run-plan cognitive-complexity budget after the confirmed-sibling shim split. Move that branch into a helper so the plan loop stays under the limit.

Signed-off-by: Rui Luo <ruluo@nvidia.com>

@prekshivyas prekshivyas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 3339df3. The requested ownership fix is present: confirmed sibling gateway ports preserve the shared npm CLI package and installer-managed shims, while the destroy-user-data shim cleanup remains limited to unidentified or unproven scoped evidence. I re-reviewed the destructive cleanup boundary, documentation, and regression coverage; the two focused uninstall suites pass 37/37 locally.

@prekshivyas
prekshivyas merged commit f10b259 into main Aug 17, 2026
72 of 73 checks passed
@prekshivyas
prekshivyas deleted the fix/9277-uninstall-destroy-shim branch August 17, 2026 07:52
ericksoa pushed a commit that referenced this pull request Aug 18, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry required before planning the
v0.0.110 release. The entry summarizes user-facing changes merged since
v0.0.109 and links each change to its published documentation route and
source PR.

## Changes

- Add `docs/changelog/2026-08-17.mdx` with the exact `## v0.0.110`
release heading.
- Cover managed local inference, endpoint validation, onboarding and
recovery, explicit experimental Portable OpenClaw, messaging and policy
cleanup, backup and security hardening, and release qualification.
- Preserve the documentation skip list and the current supported-agent
matrix; test-only refactors, dormant activation work, and Pi-only
changes are intentionally excluded.

### Source-to-doc mapping

- #8711 -> `docs/changelog/2026-08-17.mdx`: Add the Muse Glimmer
llama.cpp profile.
- #9099 -> `docs/changelog/2026-08-17.mdx`: Update the Muse Glimmer vLLM
runtime.
- #9319 -> `docs/changelog/2026-08-17.mdx`: Select the provider required
by an explicit serving profile.
- #9311 -> `docs/changelog/2026-08-17.mdx`: Report probe-image pull
failures separately.
- #9345 -> `docs/changelog/2026-08-17.mdx`: Reuse mirrored Windows
Ollama.
- #9284 -> `docs/changelog/2026-08-17.mdx`: Complete the required Ollama
upgrade.
- #9320 -> `docs/changelog/2026-08-17.mdx`: Reject unsafe custom
endpoint URLs before mutation.
- #9119 -> `docs/changelog/2026-08-17.mdx`: Reject unsupported custom
endpoint URL components.
- #9236 -> `docs/changelog/2026-08-17.mdx`: Require native Anthropic
tool-use evidence.
- #9347 -> `docs/changelog/2026-08-17.mdx`: Distinguish Gemini runtime
404 diagnostics.
- #9307 -> `docs/changelog/2026-08-17.mdx`: Preserve the recorded API
family when only the model drifts.
- #9233 -> `docs/changelog/2026-08-17.mdx`: Fail incomplete Hermes route
synchronization.
- #9185 -> `docs/changelog/2026-08-17.mdx`: Serialize Model Router
lifecycle work across gateways.
- #9112 -> `docs/changelog/2026-08-17.mdx`: Stop Model Router after the
last routed sandbox is destroyed.
- #9229 -> `docs/changelog/2026-08-17.mdx`: Verify fresh sandbox
execution readiness.
- #9299 -> `docs/changelog/2026-08-17.mdx`: Verify a separate agent API
host forward before reporting ready.
- #9318 -> `docs/changelog/2026-08-17.mdx`: Honor explicit sandbox
recreation.
- #9325 -> `docs/changelog/2026-08-17.mdx`: Measure readiness reuse
windows from collection completion.
- #9352 -> `docs/changelog/2026-08-17.mdx`: Guide users away from the
deprecated global start command.
- #9370 -> `docs/changelog/2026-08-17.mdx`: Persist managed OpenClaw
agent identity.
- #9366 -> `docs/changelog/2026-08-17.mdx`: Pass messaging dependencies
during reused onboarding.
- #9321 -> `docs/changelog/2026-08-17.mdx`: Detect proxied connect
sessions.
- #9285 -> `docs/changelog/2026-08-17.mdx`: Run probe-only recovery when
absent authority cannot be created.
- #9282 -> `docs/changelog/2026-08-17.mdx`: Complete probe-only recovery
without platform evidence.
- #8920 -> `docs/changelog/2026-08-17.mdx`: Preserve legacy gateway
identity.
- #9198 -> `docs/changelog/2026-08-17.mdx`: Report sandbox config-read
failures.
- #9201 -> `docs/changelog/2026-08-17.mdx`: Remove only the exact Docker
orphan on destroy.
- #9176 -> `docs/changelog/2026-08-17.mdx`: Use rootless Podman for
Portable lifecycle operations.
- #9197 -> `docs/changelog/2026-08-17.mdx`: Preflight Portable CPU
delegation.
- #9289 -> `docs/changelog/2026-08-17.mdx`: Narrow Portable policy
defaults.
- #9270 -> `docs/changelog/2026-08-17.mdx`: Preserve Portable model
intent.
- #9339 -> `docs/changelog/2026-08-17.mdx`: Reconcile timed-out Portable
stop state.
- #9209 -> `docs/changelog/2026-08-17.mdx`: Clean receipt-owned Portable
Podman resources.
- #9186 -> `docs/changelog/2026-08-17.mdx`: Separate Podman activation
readiness.
- #9376 -> `docs/changelog/2026-08-17.mdx`: Settle Portable OpenClaw
pairing before readiness.
- #9296 -> `docs/changelog/2026-08-17.mdx`: Retire messaging channel
presets the host no longer configures.
- #9327 -> `docs/changelog/2026-08-17.mdx`: Drop retired channels from
reused messaging selections.
- #9306 -> `docs/changelog/2026-08-17.mdx`: Remove gateway-enforced
presets without a local record.
- #9248 -> `docs/changelog/2026-08-17.mdx`: Activate Google Chat pairing
approval.
- #9374 -> `docs/changelog/2026-08-17.mdx`: Accept schema-owned
messaging plan fields.
- #9317 -> `docs/changelog/2026-08-17.mdx`: Accept safe hard-linked
package files during backup.
- #9288 -> `docs/changelog/2026-08-17.mdx`: Remove managed CLI shims
with destroyed user data.
- #9239 -> `docs/changelog/2026-08-17.mdx`: Read voice credentials from
fixed descriptors.
- #9269 -> `docs/changelog/2026-08-17.mdx`: Accept bounded native
OpenClaw device modes.
- #9371 -> `docs/changelog/2026-08-17.mdx`: Isolate OpenClaw
startup-guard output.
- #9351 -> `docs/changelog/2026-08-17.mdx`: Restore staging Launchable
validation.
- #9350 -> `docs/changelog/2026-08-17.mdx`: Retry transient
collaborator-permission reads.
- #9353 -> `docs/changelog/2026-08-17.mdx`: Retry transient
exact-artifact downloads.
- #9226 -> `docs/changelog/2026-08-17.mdx`: Add bounded Brev readiness
diagnostics.
- #9237 -> `docs/changelog/2026-08-17.mdx`: Report same-commit E2E
reliability.
- #9232 -> `docs/changelog/2026-08-17.mdx`: Execute native-runtime
qualification.
- #9275 -> `docs/changelog/2026-08-17.mdx`: Define E2E selection and
retry guidance.
- #9234 -> `docs/changelog/2026-08-17.mdx`: Move documentation review
after merge.
- #9365 -> `docs/changelog/2026-08-17.mdx`: Mount documentation reviewer
inputs before startup.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the dated release-entry
contract.
- [ ] Tests 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:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit: Not applicable; documentation-only change.
- Station profile/scenario: Not applicable.
- Result: Not applicable.
- Supporting evidence: Not applicable.

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] 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
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `npx vitest run
test/changelog-docs.test.ts` (7 passed)
- [x] 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 to one
prose-only changelog page; `npm run docs` passed the repository's strict
documentation gate.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — passed
with 0 errors and the 2 existing Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— the SPDX header is present; dated changelog pages intentionally do not
use frontmatter.

---
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Documentation**
  * Added release notes for v0.0.110.
* Documented experimental managed llama.cpp and Portable OpenClaw
profiles.
* Covered inference validation, onboarding and recovery improvements,
rootless lifecycle handling, messaging and policy updates, backups,
credential handling, filesystem protections, and release qualification
updates.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ubuntu 22.04][Install] destructive uninstall reports success but leaves NemoClaw shim on disk

3 participants