fix(uninstall): preserve OpenShell gateway state - #7831
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe uninstall flow now preserves local gateway state when ChangesGateway state preservation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UninstallCommand
participant executePlan
participant GatewayState
UninstallCommand->>executePlan: Run uninstall with retention options
executePlan->>GatewayState: Preserve local gateway state
UninstallCommand->>executePlan: Run full uninstall
executePlan->>GatewayState: Remove local gateway state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-gateway-service.test.ts`:
- Around line 117-127: Add a sibling-gateway or unreadable-gateway-list scenario
alongside the test using fixture, writeManagedService, writeGatewayEnv, and
writeGatewayState, so uninstall enters the scoped cleanup branch while
keepOpenShell is enabled. Assert exitCode remains successful and
gatewayStatePath still exists, verifying selected gateway state is preserved;
keep the existing service and environment assertions as appropriate.
In `@src/lib/actions/uninstall/run-plan.ts`:
- Around line 1758-1760: Guard the scoped cleanup removal of
paths.selectedGatewayLocalStateDir with options.keepOpenShell in the uninstall
plan, matching the existing protection for paths.gatewayLocalStateDir. Add a
sibling/unreadable-list regression test covering scopedToSelectedGateway with
keepOpenShell enabled, ensuring the selected gateway state is retained.
- Around line 1758-1760: Update the uninstall plan around the keepOpenShell
conditional so it only selects whether gateway state should be preserved or
removed, and delegate the actual persisted gateway local-state deletion to the
relevant state-layer API under src/lib/state. Remove the direct removePath call
from the action while preserving the existing keepOpenShell logging behavior.
🪄 Autofix (Beta)
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: 533e7a05-fd9f-433c-8b35-a9028f845763
📒 Files selected for processing (4)
docs/manage-sandboxes/uninstall-nemoclaw.mdxdocs/reference/commands.mdxsrc/lib/actions/uninstall/run-plan-gateway-service.test.tssrc/lib/actions/uninstall/run-plan.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None 2 optional E2E recommendations
1 warning · 0 suggestionsWarningsWarnings do not block.
|
There was a problem hiding this comment.
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`:
- Around line 1722-1723: Update all three cleanup branches in run-plan.ts: at
lines 1722-1723, 1752-1753, and 1760-1762, require both the existing retention
condition and !externallySupervised before removing gateway state; otherwise
retain the corresponding path and emit an appropriate retention log. Apply this
to paths.selectedGatewayLocalStateDir in the shared-registry branch,
selected-gateway state in the scoped cleanup branch, and
paths.gatewayLocalStateDir in the non-scoped cleanup branch.
🪄 Autofix (Beta)
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: 8b6e8849-2f0f-4230-bd3f-e80bd4695125
📒 Files selected for processing (2)
src/lib/actions/uninstall/run-plan-gateway-service.test.tssrc/lib/actions/uninstall/run-plan.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/uninstall/run-plan-gateway-service.test.ts
There was a problem hiding this comment.
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-gateway-segregation.test.ts`:
- Line 142: Strengthen the assertion in the uninstall test around
gatewayStatePath to verify the file contents remain identical to the original
TOML after uninstall, not merely that the path exists. Capture the pre-uninstall
contents and compare them with the post-uninstall read while preserving the
existing existence check.
🪄 Autofix (Beta)
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: 452a2a8d-0418-48b7-aad2-37531b4538f3
📒 Files selected for processing (3)
src/lib/actions/uninstall/run-plan-gateway-segregation.test.tssrc/lib/actions/uninstall/run-plan-gateway-service.test.tssrc/lib/actions/uninstall/run-plan.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/actions/uninstall/run-plan.ts
- src/lib/actions/uninstall/run-plan-gateway-service.test.ts
|
CI is green for the changed behavior, reviews, docs receipt, security scans, builds, and selected E2E coverage. The focused uninstall suite passes 78/78. The remaining red checks ( No change to #7831 is warranted for that baseline issue. Maintainer approval plus the repository's normal waiver/fix-forward decision for the unrelated ratchet is the remaining merge action. |
5f46b8e to
dc44f3a
Compare
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
dc44f3a to
16e4646
Compare
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical pre-tag release entry for NemoClaw v0.0.98. The dated entry records the user-visible changes merged after v0.0.97 and links each release theme to its published documentation. ## Changes - Add `docs/changelog/2026-07-29.mdx` with the exact `## v0.0.98` release heading. - Summarize Hermes 0.19, Deep Agents Code automation and skill safety, readiness diagnostics, lifecycle recovery, uninstall behavior, messaging conflicts, dependency hardening, and bounded diagnostics. - Use the parser-safe MDX SPDX comment and root-absolute routes for published OpenClaw, Hermes, and Deep Agents documentation. ### Source summary - [#7849](#7849) -> `docs/changelog/2026-07-29.mdx`: Record the Hermes 0.19 runtime migration repairs for cron state, dashboard seeding, and MCP naming. - [#7662](#7662) -> `docs/changelog/2026-07-29.mdx`: Record bounded gateway and Docker subprocess diagnostics. - [#7850](#7850) -> `docs/changelog/2026-07-29.mdx`: Record verified no-clobber Deep Agents Code skill installation. - [#7848](#7848) -> `docs/changelog/2026-07-29.mdx`: Record post-reboot delivery-chain recovery for visible OpenClaw sandboxes. - [#7831](#7831) -> `docs/changelog/2026-07-29.mdx`: Record OpenShell gateway-state preservation during uninstall. - [#7827](#7827) -> `docs/changelog/2026-07-29.mdx`: Record the removal of upstream test sources from published Hermes images. - [#7775](#7775) -> `docs/changelog/2026-07-29.mdx`: Record the blocking diagnostic for unsupported `DOCKER_HOST` values. - [#7833](#7833) -> `docs/changelog/2026-07-29.mdx`: Record reviewed Python dependency baselines for Hermes and Deep Agents Code images. - [#7771](#7771) -> `docs/changelog/2026-07-29.mdx`: Record the managed Hermes Agent 0.19.0 upgrade. - [#7811](#7811) -> `docs/changelog/2026-07-29.mdx`: Record fail-closed messaging channel conflict handling. - [#7797](#7797) -> `docs/changelog/2026-07-29.mdx`: Record the managed non-interactive Deep Agents Code JSON envelope. - [#7782](#7782) -> `docs/changelog/2026-07-29.mdx`: Record the storage-remediation readiness capability. - [#7784](#7784) -> `docs/changelog/2026-07-29.mdx`: Record the 120-second OpenShell readiness budget for sandbox recreation. - [#7810](#7810) -> `docs/changelog/2026-07-29.mdx`: Record rejection of stale Deep Agents Code security inventories. ## 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 native changelog contract, including the version heading, MDX SPDX comment, and published routes. - [ ] Tests not applicable — justification: - [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: `docs/changelog/2026-07-29.mdx` was reviewed against `docs/CONTRIBUTING.md` and `WRITING.md` for release meaning, terminology, structure, voice, sentence form, MDX structure, published routes, and code-sample presentation. The changelog contract passed 6 tests. The docs build completed with 0 errors and 2 existing Fern warnings. - Agent: Codex CLI <!-- docs-review-head-sha: e3221d1 --> <!-- docs-review-agents-blob-sha: c052d60 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable. `scripts/prepare-dgx-station-host.sh` is unchanged. - 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` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not run for this documentation-only change. - [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) — The build completed with 0 errors and 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) — Native changelog entries use the required parser-safe MDX SPDX comment and do not use frontmatter. --- Signed-off-by: San Dang <sdang@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added managed Hermes upgrades with verified releases, version reporting, and preserved configuration contracts. - Improved Deep Agents Code JSON output and skill installation behavior. - Added clearer Docker host and system readiness reporting. - Improved post-reboot delivery recovery and sandbox readiness timing. - **Bug Fixes** - Preserved gateway state when uninstalling with `--keep-openshell`. - Prevented conflicting messaging credentials from blocking onboarding and rebuilds. - Improved gateway diagnostics, dependency security, runtime filesystem protection, and evidence handling. - **Documentation** - Published the v0.0.98 release notes. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
nemoclaw uninstall --keep-openshellnow preserves the local gateway state required by the retained OpenShell service and process. Uninstall also preserves that state whenever the gateway is externally supervised. Previously, cleanup could removeopenshell-gateway.tomlwhile leaving the gateway process or enabled user service running.Related Issue
Fixes #7830.
Changes
--keep-openshellis selected, including scoped cleanup when sibling gateways remain.Type of Change
Quality Gates
Documentation Writer Review
docs-updateddocs/manage-sandboxes/uninstall-nemoclaw.mdx,docs/reference/commands.mdx, anddocs/deployment/gateway-lifecycle-authority.mdxremain accurate after rebasing onto currentmain;npm run docspreviously passed with zero errors and two pre-existing Fern warnings. The rebased focused uninstall suite passes 78/78.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm exec -- vitest run --project cli src/lib/actions/uninstall/run-plan-gateway-service.test.ts src/lib/actions/uninstall/run-plan.test.ts src/lib/actions/uninstall/run-plan-gateway-segregation.test.ts src/lib/actions/uninstall/run-plan-dual-station.test.ts(78 passed)npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only) — zero errors and two pre-existing Fern warningsSigned-off-by: Senthil Ravichandran senthilr@nvidia.com
Summary by CodeRabbit
--keep-openshellis set, including scoped uninstall scenarios and cases involving external supervision.~/.local/state/nemoclawcontains on Linux and how$$ nemoclawn uninstalloptions (including--yesand--keep-openshell) affect it.