fix(uninstall): stop the selected gateway counting itself a sibling - #7993
Conversation
The gateways/ scan in inspectOtherGatewayEnvironments excluded the gateway being uninstalled by path identity, comparing each entry against the selected state root. That works only for a non-default port, whose state root is <shared>/gateways/<port>. For the default port the state root is the shared root itself, so a <shared>/gateways/8080 directory never matched and the selected gateway was reported as its own sibling. Every other sibling filter in this file, and listGatewayStateRoots in src/lib/state/gateway-registry.ts, already compare port identity instead. Align this scan with that contract: a per-port directory named for the gateway being uninstalled is that gateway's own state, never a sibling. On a single-gateway host the misdetection scoped cleanup to the selected gateway, printed "Sibling gateways remain" for shared helper services, the HTTPS Pin Runtime adapter, provider registrations, Docker images and host state, and left all of them behind. When the scoped teardown then could not delete an already-absent OpenShell sandbox, uninstall reported incomplete cleanup and exited nonzero. The conservative treatments are unchanged: a symlink or non-directory entry, a name that is not a valid port, and an unavailable OpenShell gateway list all still count as siblings. Fixes #7987 Signed-off-by: Yanyun Liao <yanyunl@nvidia.com>
📝 WalkthroughWalkthroughThe uninstall plan now excludes the selected gateway’s own port directory from sibling detection. Regression tests verify full teardown for a single gateway and scoped cleanup when another live gateway remains. ChangesUninstall sibling detection
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 479ce77 in the TypeScript / code-coverage/cliThe overall coverage in commit 479ce77 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 1 semantic terminology decisionTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
|
Security review for commit SHA 479ce77: PASS.
No security finding blocks merge. Required GitHub checks, automated review, E2E evidence, and approval for this commit remain separate merge gates. |
cv
left a comment
There was a problem hiding this comment.
Approved for commit 479ce77. The change restores the documented uninstall behavior, both commits are verified, the independent documentation writer review found no documentation update was needed, the security review passed, no review threads remain, and the required GitHub checks and E2E gate pass for this commit.
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical dated `v0.0.101` changelog entry that was missing when the release tag was cut. This post-release recovery records the shipped behavior on current `main` without changing or replacing the existing tag. ## Changes - Add `docs/changelog/2026-08-03.mdx` with the exact `## v0.0.101` heading, release summary, detailed behavior changes, support boundaries, and links to durable documentation. - [#7317](#7317) -> `docs/changelog/2026-08-03.mdx`: Records experimental OpenClaw Google Chat support and its restricted credential and webhook boundary. - [#7715](#7715) -> `docs/changelog/2026-08-03.mdx`: Records strict onboarding recovery state and authoritative resume identity. - [#7749](#7749) -> `docs/changelog/2026-08-03.mdx`: Records the provider-neutral policy seam and unchanged runtime support boundary. - [#7817](#7817) -> `docs/changelog/2026-08-03.mdx`: Records preserved Hermes home-channel assignments across rebuilds. - [#7820](#7820) -> `docs/changelog/2026-08-03.mdx`: Records the SSH-session status field correction. - [#7847](#7847) -> `docs/changelog/2026-08-03.mdx`: Records fail-closed credential filtering for migration and rebuild backups. - [#7870](#7870) -> `docs/changelog/2026-08-03.mdx`: Records sandbox-qualified in-sandbox host command hints. - [#7875](#7875) -> `docs/changelog/2026-08-03.mdx`: Records Microsoft Teams stop and start E2E coverage. - [#7885](#7885) -> `docs/changelog/2026-08-03.mdx`: Records Hermes managed gateway detection in status. - [#7889](#7889) -> `docs/changelog/2026-08-03.mdx`: Records policy-authenticated HTTPS Pin Runtime route revocation. - [#7891](#7891) -> `docs/changelog/2026-08-03.mdx`: Records default fallback for negative timeout and polling overrides. - [#7993](#7993) -> `docs/changelog/2026-08-03.mdx`: Records correct sibling detection during uninstall. - [#7995](#7995) -> `docs/changelog/2026-08-03.mdx`: Records absent configuration-hash handling before shields lock. - [#8001](#8001) -> `docs/changelog/2026-08-03.mdx`: Records the dormant atomic managed workload replacement foundation. - [#8029](#8029) -> `docs/changelog/2026-08-03.mdx`: Records repository terminology review in PR Review Advisor. - [#8031](#8031) -> `docs/changelog/2026-08-03.mdx`: Records provider-neutral managed snapshot authority. - [#8032](#8032) -> `docs/changelog/2026-08-03.mdx`: Records immutable managed clone handoff contracts. - [#8034](#8034) -> `docs/changelog/2026-08-03.mdx`: Records the dormant provider-owned clone transaction surface. - [#8035](#8035) -> `docs/changelog/2026-08-03.mdx`: Records the dormant Hermes managed clone broker boundary. - [#8036](#8036) -> `docs/changelog/2026-08-03.mdx`: Records the dormant transactional managed bootstrap boundary. - [#8037](#8037) -> `docs/changelog/2026-08-03.mdx`: Records dormant Docker bootstrap primitives and the unchanged provider support boundary. - [#8070](#8070) -> `docs/changelog/2026-08-03.mdx`: Records consolidated sandbox resource-limit E2E coverage. - [#8071](#8071) -> `docs/changelog/2026-08-03.mdx`: Records escaped and bounded CLI validation diagnostics. - [#8081](#8081) -> `docs/changelog/2026-08-03.mdx`: Records bounded linear snapshot Base64 validation. - [#8085](#8085) -> `docs/changelog/2026-08-03.mdx`: Records commit-bound workflow approval for eligible same-repository maintainers. - [#8088](#8088) -> `docs/changelog/2026-08-03.mdx`: Records Hermes managed-policy E2E selection. - [#8090](#8090) -> `docs/changelog/2026-08-03.mdx`: Records pinned CI search-tool provisioning. - [#8106](#8106) -> `docs/changelog/2026-08-03.mdx`: Records fallback from failed managed OpenShell gateway startup. - [#8107](#8107) -> `docs/changelog/2026-08-03.mdx`: Records Hermes adapter lifecycle E2E selection. - [#8128](#8128) -> `docs/changelog/2026-08-03.mdx`: Records the dormant transactional Docker bootstrap adapter and rollback authority. - [#8140](#8140) -> `docs/changelog/2026-08-03.mdx`: Records Slack conflict scope across independent OpenShell gateways. - [#8147](#8147) -> `docs/changelog/2026-08-03.mdx`: Records completion of durable v0.0.100 documentation audit follow-ups. ## 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 - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: This documentation-only recovery does not change executable behavior. - [x] 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 - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Independently reviewed `docs/changelog/2026-08-03.mdx` at commit `0bebe1f568e3dc85cf410aac1dfb8f8830070b85`. Its blob is `82887920f9720eafd75db6b2271c35f7477edb9b`. The entry follows the writing guide, controlled terminology, changelog structure, MDX SPDX format, literal CLI-name rule, and root-absolute route requirements. It accurately records the `v0.0.100...v0.0.101` release range, Announcement #8162, accepted scope boundaries, and shipped security behavior. There are no code samples. Focused changelog tests and the documentation build pass for this commit. - Agent: Codex Desktop independent documentation writer <!-- docs-review-head-sha: 0bebe1f --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## Security Review - Result: `PASS` - Reviewed commit: `0bebe1f568e3dc85cf410aac1dfb8f8830070b85` - Base commit: `643a4ab8b5f583d8555192a37927268b26022c51` - Findings: None. - Secrets and credentials: `PASS`. No credential values or secret files are present. - Input validation and data sanitization: `PASS`. No executable input path changes. - Authentication and authorization: `PASS`. No identity or permission logic changes. - Dependencies and third-party libraries: `PASS`. No dependency changes. - Error handling and logging: `PASS`. No runtime path changes; diagnostic-security claims are precise. - Cryptography and data protection: `PASS`. No implementation changes. - Configuration and security controls: `PASS`. No configuration, container, port, or HTTP changes. - Security testing: `PASS`. No coverage is removed; the entry records shipped test and security behavior. - System security: `PASS`. No runtime control changes; dormant and non-activation boundaries are explicit. - Agent: Codex Desktop independent security reviewer ## Verification - [ ] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub — verification is pending after commit `0bebe1f568e3dc85cf410aac1dfb8f8830070b85` is pushed. - [ ] 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 — commit hooks passed; pre-push is pending. - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — tests are not applicable to this documentation-only recovery. - [x] Applicable broad gate passed — not applicable to this documentation-only recovery. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, credentials, or private keys are added by this diff. - [ ] `npm run docs` builds without warnings (doc changes only) — GitHub documentation checks are pending. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) — independent documentation review passed. - [x] New doc pages include SPDX header and frontmatter (new pages only) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. GitHub CI is authoritative. Focused changelog tests and `npm run docs` passed after the merge refresh. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added experimental Google Chat support. * Improved runtime and session status visibility. * Added onboarding recovery and persistence safeguards. * Added snapshot validation and dormant managed-workload support. * **Bug Fixes** * Improved backup sanitization, route handling, and gateway reliability. * **Documentation** * Added the v0.0.101 changelog and related updates. * **Tests** * Expanded end-to-end coverage and strengthened trusted CI validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
On a host with one gateway,
nemoclaw uninstallcould detect the selected gateway as a sibling. That false detection scoped cleanup to the selected gateway and preserved shared host resources. This change excludes the selected gateway port from the sibling scan.Related Issue
Closes #7987.
Reproduction
Run on the contributor's Ubuntu 24.04 x86_64 test host without a GPU, against commit
387cb0864onmain.nemoclaw, port 8080) is registered.~/.nemoclaw/gateways/8080directory exists.nemoclaw uninstall --yes.Environment
387cb08644fe030bb85146255f4b77e3c54697d2onmainmy-assistant) and one gateway;openshell gateway list -o jsonreturns exactly[{"name":"nemoclaw"}]Observed on
mainbefore the fixEight
Sibling gateways remainlines appeared on a host with one gateway.Observed on
fix/uninstall-self-sibling-7987after the fixThe contributor used the same host and base commit with this change:
Sibling gateways remainoccurrences: 0. Scoped banner occurrences: 0.cleanup was incompleteoccurrences: 0.Analysis
inspectOtherGatewayEnvironmentsinsrc/lib/actions/uninstall/run-plan.tswalks<shared>/gateways/. For each entry, it determines whether the entry represents another gateway. It excluded the selected gateway by path identity:selectedRootisnemoclawStateRoot(home, GATEWAY_PORT). That helper is asymmetric insrc/lib/state/state-root.ts:For a non-default port, the selected root is
<shared>/gateways/<port>, so the comparison excludes it. For the default port, the selected root is the shared root. Therefore,<shared>/gateways/8080cannot equal it. The entry then reaches the liveness check:resolveGatewayName(8080)isnemoclaw, which is the selected gateway. Its own liveness therefore marked it as a sibling. A sibling is a different gateway, so this result was incorrect.otherGatewayEnvironmentsRemaincontrolsscopedToSelectedGateway. That value changes the banner toresources owned by gateway '<name>'and makes each cleanup step preserve shared host state. In scoped mode, uninstall also deletes sandboxes individually instead of usingsandbox delete --all. If a per-sandbox deletion cannot reach an absent sandbox,removedSelectedResourcesbecomes false.run-plan.tsthen reportsSelected gateway cleanup was incomplete; preserving its state for retry.and the reporter exits with status 1. The contributor's host could delete its sandbox, so it exited with status 0 while showing the same eight sibling messages.Fix
Compare port identity, which the other sibling filters in this file already use:
Every other sibling filter in
run-plan.tsusesGATEWAY_PORT. These include the registry scan and the per-entry checks in the sandbox and registry steps.listGatewayStateRootsinsrc/lib/state/gateway-registry.tsalso skipsgatewayPort === DEFAULT_GATEWAY_PORTwhile enumerating the same directory. The uninstall scan was the only place that used path identity and handled the default port incorrectly.Whole-class check.
selectedRootwas used for this comparison at one site. The other sibling filters are port-based. The only other reader of<shared>/gateways/,listGatewayStateRoots, already excludes the selected port. No other call site needed a change.Conservative behavior deliberately unchanged. A symbolic link, a non-directory entry, an invalid port name, and a
nullgateway list still report a sibling. Agateways/8080entry that is a symbolic link or plain file also remains an unexpected shape. This change removes the confirmed false positive without expanding deletion when inspection is uncertain.No documentation change required.
docs/manage-sandboxes/uninstall-nemoclaw.mdxanddocs/reference/commands.mdxalready state that uninstall removes shared host resources only when it confirms that no sibling gateways remain. The implementation did not follow that documented contract. This change restores the documented behavior.Tests. Two cases in
run-plan-gateway-segregation.test.tscover the change:gateways/8080directory with onlynemoclawlive produces full teardown. It emits noSibling gateways remainmessage or scoped banner, and it still runssandbox delete --all. Removing the one-line fix makes this test fail.gateways/8080directory with a livenemoclaw-8091gateway still reports a sibling and preservesgateways/8091.The existing #7315 orphan and live-sibling cases and the non-default-port segregation cases are unchanged. The contributor reported 106 passing tests across
src/lib/actions/uninstall/.Changes
src/lib/actions/uninstall/run-plan.tsto exclude the selected gateway port from the sibling scan.src/lib/actions/uninstall/run-plan-gateway-segregation.test.tsto cover the selected gateway and a different live gateway.Platform Scope
The reporter selected All Platforms after reproducing the defect on Ubuntu 22.04 and 24.04. The contributor verified the change on Ubuntu 24.04 x86_64. The defect is a path-and-port comparison and has no architecture-specific branch.
Type of Change
Quality Gates
Documentation Writer Review
no-docs-neededdocs/manage-sandboxes/uninstall-nemoclaw.mdxanddocs/reference/commands.mdx. Those pages already state that shared host resources are removed only when no sibling gateways remain, so no documentation file changed.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.sh.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailable — the contributor reportednpx prek run --all-files; final-commit evidence for all normal hooks is not recorded.479ce777465eb5fc9ca6db91de24e921e2a72cd9.479ce777465eb5fc9ca6db91de24e921e2a72cd9.npm run docsbuilds without warnings (doc changes only) — Not applicable; no documentation file changed.AI Disclosure
Signed-off-by: Yanyun Liao yanyunl@nvidia.com
Summary by CodeRabbit