fix(cli): recommend and run 'openshell gateway remove' on all platforms (#6569) - #6570
Conversation
…ms (#6569) nemoclaw destroy gated the gateway removal command on process.platform: Linux used 'openshell gateway remove <name>' (correct), while macOS and other platforms used 'openshell gateway destroy -g <name>'. Modern OpenShell (>=0.0.44, incl. 0.0.72) has no 'gateway destroy' subcommand and rejects it as unrecognized, so: - the preserved-gateway hint recommended a command that fails on macOS (the reported symptom), and - --cleanup-gateway on macOS actually ran ONLY 'gateway destroy -g', which errors and is swallowed by ignoreError, silently leaving the gateway behind. Use 'gateway remove <name>' as the primary subcommand on every platform (matching the authoritative runOpenshell(["gateway","remove",...]) calls), keeping 'gateway destroy -g' only as a best-effort fallback for pre-0.0.44 builds. The Linux-only host gateway-process stop is unchanged. Update the tests that encoded the platform ternary and assert the hint recommends the real subcommand. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Jason Ma <jama@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughGateway cleanup now tries ChangesGateway Remove Command Fix
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
Advisor checklist — resolved / justified
Signed-off-by: Jason Ma jama@nvidia.com |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
E2E Target Results — ✅ All requested jobs passedRun: 29031341826
|
Exact-head runtime evidence after the
|
Co-authored-by: Jason Ma <jama@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
E2E Target Results — ✅ All requested jobs passedRun: 29032327832
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
E2E Target Results — ❌ Some jobs failedRun: 29033232462
|
E2E Target Results — ✅ All requested jobs passedRun: 29033232462
|
Exact-head follow-up verificationHead: The three advisor findings are now covered directly:
Verification on the final tree:
The first Signed-off-by: Carlos Villela cvillela@nvidia.com |
apurvvkumaria
left a comment
There was a problem hiding this comment.
Exact head ff2958a is ready for final human merge. Normal CI is green, the required onboard-repair, onboard-resume, and sandbox-operations live jobs passed after the infrastructure retry, both refreshed advisors recommend merge_as_is, and CodeRabbit has no actionable thread.
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - #6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - #6271 and #6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - #6465, #6539, #6570, and #6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - #6523, #6551, #6484, #6488, #6324, and #6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - #6559, #6538, #6560, #6568, #6552, #6567, and #6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - #6541, #5415, #6246, #6496, and #6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - #6253, #6572, #6444, #6536, and #5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - #6508, #6527, #5506, #6588, #6446, #6447, #6582, #6296, #6367, #6397, and #6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## 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 <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [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: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests not applicable, release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) - [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) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…ms (NVIDIA#6569) (NVIDIA#6570) ## Summary `nemoclaw <name> destroy` gated its gateway-removal on `process.platform`: Linux used `openshell gateway remove <name>` (correct), while **macOS and other non-Linux platforms used `openshell gateway destroy -g <name>`**. Modern OpenShell (≥0.0.44, including the reporter's 0.0.72) has **no `gateway destroy` subcommand** and rejects it as `unrecognized subcommand 'destroy'`. That mistaken platform assumption caused two bugs: 1. **The reported symptom** — the preserved-gateway hint printed a command that fails on macOS: `Shared NemoClaw gateway preserved. Re-run 'openshell gateway destroy -g nemoclaw' to remove it, …` 2. **A worse, silent functional bug** — `nemoclaw destroy --cleanup-gateway` on macOS ran **only** `gateway destroy -g` (`destroy-gateway.ts`), which errors and is swallowed by `ignoreError: true`, so the gateway was **silently never removed**. Both share the same root cause: the belief that macOS OpenShell uses `gateway destroy`. The codebase's own authoritative removal calls (`onboard.ts:1184`, `gateway-destroy.ts:41`) already use `gateway remove` unconditionally. Fixes NVIDIA#6569. ## Changes - **`src/lib/actions/sandbox/destroy-gateway.ts`** — run `gateway remove <name>` (with `gateway destroy -g` as a best-effort pre-0.0.44 fallback) on **all** platforms. The Linux-only host gateway-process stop (`stopHostGatewayProcesses`) is unchanged and stays Linux-gated. - **`src/lib/actions/sandbox/destroy.ts`** — the preserved-gateway hint now unconditionally recommends `openshell gateway remove <name>`. - **`test/cli/destroy-gateway-cleanup.test.ts`** — replaced the two `process.platform === "linux" ? "gateway remove" : "gateway destroy -g"` assertions with unconditional `gateway remove` (+ `not.toContain` the `destroy -g` form), and added an assertion that the preserved-gateway hint recommends `openshell gateway remove` and never `gateway destroy`. `gateway destroy -g` remains **only** as the resilient fallback for pre-0.0.44 builds; it is never the primary command or a user-facing recommendation. ## Verification - **What was proven (local + host, unit level):** - `npx vitest run test/cli/destroy-gateway-cleanup.test.ts` → **11/11 pass** (locally and on the `ipp2-0085` verify host, Node 22.23.1). - `npx vitest run src/lib/actions/sandbox/destroy-flow.test.ts test/cli/destroy-gateway-unreachable.test.ts` → 17/17 pass. - `npx biome check` clean; `npx tsx scripts/checks/run.ts` all structural checks pass; `npm run build:cli` succeeds; `test-conditionals:scan` flags nothing in the touched test file. - The removal command is now platform-independent, so the Linux-run tests exercise the exact `gateway remove`→fallback logic macOS now uses. - **What remains:** a live `nemoclaw destroy --cleanup-gateway` on a physical **macOS** host to observe the gateway actually removed. The verify hosts are Linux (x86_64 + aarch64); the macOS-specific branch was unified with the Linux path by this change, so the behavior is fully covered by the platform-independent unit tests above. Signed-off-by: Jason Ma <jama@nvidia.com> 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved gateway teardown during sandbox destruction to try `openshell gateway remove <name>` first, and only run `openshell gateway destroy -g <name>` when removal fails. * Standardized the manual gateway cleanup hint to always use `openshell gateway remove <name>`. * **Tests** * Updated CLI cleanup tests to enforce consistent remove/destroy behavior and verify the new command order and messaging across cleanup modes and environment opt-in. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Jason Ma <jama@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the pre-tag v0.0.79 release notes entry to `docs/about/release-notes.mdx` so the release plan can be generated after docs merge. The entry summarizes the merged v0.0.79 release train across inference, diagnostics, runtime hardening, policies, onboarding recovery, and release validation. ## Changes - Added the v0.0.79 release notes section with linked follow-up documentation for OpenRouter onboarding, managed vLLM changes, completion and logging, Deep Agents runtime limits, policy updates, onboarding recovery, and release validation. - Source summary: - NVIDIA#6461 -> `docs/about/release-notes.mdx`: Documents OpenRouter onboarding support and links to inference/provider references. - NVIDIA#6271 and NVIDIA#6272 -> `docs/about/release-notes.mdx`: Documents shell completion and structured logging highlights. - NVIDIA#6465, NVIDIA#6539, NVIDIA#6570, and NVIDIA#6528 -> `docs/about/release-notes.mdx`: Documents status route-drift, orphaned sandbox, gateway cleanup, and DGX Spark express-install diagnostics. - NVIDIA#6523, NVIDIA#6551, NVIDIA#6484, NVIDIA#6488, NVIDIA#6324, and NVIDIA#6542 -> `docs/about/release-notes.mdx`: Documents managed vLLM, Qwen3.6 tool parser, compaction, and timeout/readiness improvements. - NVIDIA#6559, NVIDIA#6538, NVIDIA#6560, NVIDIA#6568, NVIDIA#6552, NVIDIA#6567, and NVIDIA#6587 -> `docs/about/release-notes.mdx`: Documents runtime, credential, proxy, PID namespace, TOML, and provider-state hardening. - NVIDIA#6541, NVIDIA#5415, NVIDIA#6246, NVIDIA#6496, and NVIDIA#6573 -> `docs/about/release-notes.mdx`: Documents GitHub policy, Gmail policy, MCP allowlist, WhatsApp, and messaging-variant updates. - NVIDIA#6253, NVIDIA#6572, NVIDIA#6444, NVIDIA#6536, and NVIDIA#5860 -> `docs/about/release-notes.mdx`: Documents onboarding resume and create-step recovery improvements. - NVIDIA#6508, NVIDIA#6527, NVIDIA#5506, NVIDIA#6588, NVIDIA#6446, NVIDIA#6447, NVIDIA#6582, NVIDIA#6296, NVIDIA#6367, NVIDIA#6397, and NVIDIA#6505 -> `docs/about/release-notes.mdx`: Documents docs, release-risk, and E2E validation updates. ## 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 <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: Release-note prose only. - [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: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Tests not applicable, release-note prose only. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [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) - [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) Docs validation note: `npm run docs:check-agent-variants && npm run docs:check-routes && git diff --check` passed. Full `npm run docs` is currently blocked before Fern validation because the pinned `fern-api@5.65.2` package is unavailable from npm (`ETARGET No matching version found`). --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.79 with a new summary of recent improvements, including onboarding and inference options, operator/CLI diagnostics, sandbox recovery hardening, runtime limits, network policy behavior, and release validation updates. * Added updated references and links for the latest release. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
… verb Three onboarding-failure hints told operators to run 'openshell gateway destroy -g <name>'. OpenShell dropped that verb before 0.0.44 and now exits with "error: unrecognized subcommand 'destroy'". blueprint.yaml pins OpenShell to 0.0.85, so the line always fails. PR NVIDIA#6570 recorded that current OpenShell rejects 'gateway destroy' and fixed the sandbox destroy hint, but the final gateway-start failure, GPU-passthrough recovery, and unreadable-registry hints kept the legacy line. Each already prints 'openshell gateway remove <name>' first, so each hint still prints a command that OpenShell 0.0.85 accepts. The unreadable-registry hint had no test, which is why the stale line survived there. This adds one. Refs NVIDIA#8139 Signed-off-by: harjoth <harjoth.khara@gmail.com>
… verb Three onboarding-failure hints told operators to run 'openshell gateway destroy -g <name>'. OpenShell dropped that verb before 0.0.44 and now exits with "error: unrecognized subcommand 'destroy'". blueprint.yaml pins OpenShell to 0.0.85, so the line always fails. PR NVIDIA#6570 recorded that current OpenShell rejects 'gateway destroy' and fixed the sandbox destroy hint, but the final gateway-start failure, GPU-passthrough recovery, and unreadable-registry hints kept the legacy line. Each already prints 'openshell gateway remove <name>' first, so each hint still prints a command that OpenShell 0.0.85 accepts. The unreadable-registry hint had no test, which is why the stale line survived there. This adds one. Refs NVIDIA#8139 Signed-off-by: harjoth <harjoth.khara@gmail.com>
… verb (#8188) <!-- markdownlint-disable MD041 --> ## Summary Three onboarding-failure hints told operators to run `openshell gateway destroy -g <name>`. OpenShell dropped that verb before 0.0.44 and now exits with `error: unrecognized subcommand 'destroy'`, so an operator who follows the hint is stuck. `nemoclaw-blueprint/blueprint.yaml` sets `min_openshell_version` and `max_openshell_version` to `0.0.85`, so the line cannot work on any supported install. Each hint already prints `openshell gateway remove <name>` first, so removing the dead line leaves a command OpenShell 0.0.85 accepts. ## Related Issue Refs #8139 #8139's own repro is the sandbox destroy hint, which PR #6570 already fixed — `src/lib/actions/sandbox/destroy.ts:612-615` now emits `gateway remove` only and records the rule: "never recommend it (#6569)". #6570 did not visit these three sites, so the same broken instruction still reaches operators there. I am not claiming to close #8139; a maintainer should decide whether the reported symptom plus these three sites closes it. ## Changes Each site loses the same two lines. Nothing else in the hint blocks changes. - `src/lib/onboard/gateway-start-failure.ts` — final gateway-start failure hint. Reachable in ordinary non-GPU onboarding, so this is the main operator-facing surface. - `src/lib/onboard/gpu-recovery.ts` — GPU-passthrough recovery hint, empty-registry branch. - `src/lib/onboard/gateway-gpu-passthrough.ts` — unreadable sandbox registry hint. This one had no test at all, which is why the stale line survived here; this PR adds one. Not touched, because these run the legacy verb against a possibly-old CLI rather than recommending it to a person: - `src/lib/onboard/gateway-destroy.ts`, `src/lib/onboard.ts`, `src/lib/actions/uninstall/run-plan.ts`, `src/lib/actions/sandbox/destroy-gateway.ts` — execution fallbacks. - `scripts/install.sh:2428` — the manual pre-upgrade path, where the installed OpenShell genuinely can be pre-0.0.44. It runs `remove` first and only falls back on failure. - `src/lib/onboard/gateway-lifecycle.ts:31` — matches the verb in OpenShell output. ## Type of Change - [x] 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 - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: no page reproduces these hint blocks. The two mentions of the verb — `docs/deployment/gateway-lifecycle-authority.mdx:183` and `docs/reference/commands.mdx:3448` — describe the execution fallback that this PR leaves in place, so both stay accurate. `docs/reference/troubleshooting.mdx:2523` and `docs/reference/commands.mdx:830` refer to the printed guidance only generically and remain true. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex Desktop completed the nine-category security review for commit `40bb889bc72d25d68c7f136d7e5058e8f0d5e641` against base `e7f58e98e1046e697563f5def753cd0dde706723`; PASS with no findings. The change removes only unsupported operator guidance from three onboarding failure paths. It does not alter control flow, state, credentials, permissions, policy, network behavior, lifecycle safeguards, diagnostic redaction, fail-closed exits, supported cleanup guidance, or intentional executable compatibility fallbacks. - [ ] 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: `no-docs-needed` - Evidence: Reviewed all six changed files at commit `40bb889bc72d25d68c7f136d7e5058e8f0d5e641`. The PR removes only the unsupported `openshell gateway destroy` command from three onboarding recovery hints. Current documentation already uses `openshell gateway remove` for direct recovery; the remaining references to the legacy command describe unaffected executable compatibility fallbacks. The current-main merge adds only unrelated native-artifact workload-contract sources and tests; the six-file change and documentation conclusion remain unchanged. - Agent: Codex Desktop <!-- docs-review-head-sha: 40bb889 --> <!-- docs-review-agents-blob-sha: 3dd7c24 --> ## 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 - [ ] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: GitHub CI is running for commit `40bb889bc72d25d68c7f136d7e5058e8f0d5e641`; contributor evidence for the substantive change appears below. - [ ] 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; no runtime, test-harness, or coverage-configuration 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) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ### Red, then green Each of the three sites failed before the fix. The unreadable-registry site is the new test. ```text FAIL |cli| src/lib/onboard/gpu-recovery.test.ts > omits openshell gateway destroy for every registered-sandbox count (#8139) AssertionError: expected ' Existing gateway was started withou…' not to contain 'gateway destroy' + openshell gateway remove nemoclaw + # For OpenShell releases that still expose lifecycle commands: + openshell gateway destroy -g nemoclaw FAIL |cli| src/lib/onboard/gateway-gpu-passthrough.test.ts > prints openshell gateway remove without gateway destroy when the sandbox registry is unreadable (#8139) + openshell gateway remove nemoclaw + # For OpenShell releases that still expose lifecycle commands: + openshell gateway destroy -g nemoclaw FAIL |integration| test/gateway-final-failure-cleanup.test.ts > collects diagnostics before cleanup, then exits + If gateway cleanup did not complete, run: + openshell gateway remove nemoclaw + # For OpenShell releases that still expose lifecycle commands: + openshell gateway destroy -g nemoclaw ``` After: ```text npx vitest run --project cli src/lib/onboard/gpu-recovery.test.ts src/lib/onboard/gateway-gpu-passthrough.test.ts Test Files 2 passed (2) Tests 20 passed (20) npx vitest run --project integration test/gateway-final-failure-cleanup.test.ts Test Files 1 passed (1) Tests 2 passed (2) ``` ### Blast radius Grepped the whole repo for every changed symbol (`handleFinalGatewayStartFailure`, `gpuPassthroughRecoveryLines`, `reportGpuPassthroughRecovery`, `reconcileGatewayGpuReuseForGpuIntent`, `reportUnreadableSandboxRegistryForGpuGatewayReuse`) and for the removed literals, then ran every match. `test/gateway-final-failure-cleanup.test.ts` is in the `integration` project, which `npm run test:changed` does not run, so it was run explicitly. ```text npx vitest run --project cli src/lib/onboard before: Test Files 1 failed | 382 passed Tests 2 failed | 4535 passed | 1 skipped after: Test Files 1 failed | 382 passed Tests 2 failed | 4537 passed | 1 skipped ``` Both failures are `src/lib/onboard/created-sandbox-finalization.test.ts` (#6311), identical before and after, and they reproduce on pristine `origin/main`: ```text git checkout origin/main -- . npx vitest run --project cli src/lib/onboard/created-sandbox-finalization.test.ts Test Files 1 failed (1) Tests 2 failed | 8 passed (10) ``` Neither is in a file this PR touches. ```text npx vitest run --project integration test/gateway-final-failure-cleanup.test.ts test/onboard-inference-reconciliation.test.ts test/install-openshell-upgrade-prompt.test.ts Test Files 3 passed (3) Tests 53 passed | 5 skipped (58) npx vitest run --project integration test/cli/destroy-gateway-cleanup.test.ts Test Files 1 passed (1) Tests 10 passed | 2 skipped (12) npm run typecheck:cli pass npm --prefix nemoclaw run typecheck pass npx biome check <changed files> pass npm run checks:repository pass npx vitest run --project cli src/lib/onboard/gateway-gpu-passthrough.test.ts --sequence.shuffle Test Files 1 passed (1) Tests 9 passed (9) ``` The shuffled run is there because the new test sets implementations on two module mocks. `restoreMocks` restores `vi.spyOn` descriptors but not those, so the file resets them in `afterEach` and stays order-independent. ### Limits The contributor's results above cover the substantive six-file change. GitHub CI is running for the current refreshed commit. I did not run this against a real OpenShell 0.0.85 host, so the `unrecognized subcommand 'destroy'` behavior comes from the reporter's transcript in #8139 and from the configured version range in `nemoclaw-blueprint/blueprint.yaml`, not from a run on my machine. The E2E lanes are not runnable from a fork. --- Signed-off-by: Harjoth Khara <harjoth.khara@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated gateway and GPU recovery guidance to use the supported gateway removal command. * Removed obsolete `gateway destroy` instructions from startup failure and recovery messages. * Improved recovery behavior when sandbox registry data is unreadable or unavailable. * **Tests** * Added regression coverage to ensure recovery guidance excludes unsupported cleanup commands. * Improved test isolation and validation of process cleanup and re-onboarding steps. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: harjoth <harjoth.khara@gmail.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Apurv Kumaria <akumaria@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Summary
nemoclaw <name> destroygated its gateway-removal onprocess.platform: Linux usedopenshell gateway remove <name>(correct), while macOS and other non-Linux platforms usedopenshell gateway destroy -g <name>. Modern OpenShell (≥0.0.44, including the reporter's 0.0.72) has nogateway destroysubcommand and rejects it asunrecognized subcommand 'destroy'.That mistaken platform assumption caused two bugs:
Shared NemoClaw gateway preserved. Re-run 'openshell gateway destroy -g nemoclaw' to remove it, …nemoclaw destroy --cleanup-gatewayon macOS ran onlygateway destroy -g(destroy-gateway.ts), which errors and is swallowed byignoreError: true, so the gateway was silently never removed.Both share the same root cause: the belief that macOS OpenShell uses
gateway destroy. The codebase's own authoritative removal calls (onboard.ts:1184,gateway-destroy.ts:41) already usegateway removeunconditionally.Fixes #6569.
Changes
src/lib/actions/sandbox/destroy-gateway.ts— rungateway remove <name>(withgateway destroy -gas a best-effort pre-0.0.44 fallback) on all platforms. The Linux-only host gateway-process stop (stopHostGatewayProcesses) is unchanged and stays Linux-gated.src/lib/actions/sandbox/destroy.ts— the preserved-gateway hint now unconditionally recommendsopenshell gateway remove <name>.test/cli/destroy-gateway-cleanup.test.ts— replaced the twoprocess.platform === "linux" ? "gateway remove" : "gateway destroy -g"assertions with unconditionalgateway remove(+not.toContainthedestroy -gform), and added an assertion that the preserved-gateway hint recommendsopenshell gateway removeand nevergateway destroy.gateway destroy -gremains only as the resilient fallback for pre-0.0.44 builds; it is never the primary command or a user-facing recommendation.Verification
npx vitest run test/cli/destroy-gateway-cleanup.test.ts→ 11/11 pass (locally and on theipp2-0085verify host, Node 22.23.1).npx vitest run src/lib/actions/sandbox/destroy-flow.test.ts test/cli/destroy-gateway-unreachable.test.ts→ 17/17 pass.npx biome checkclean;npx tsx scripts/checks/run.tsall structural checks pass;npm run build:clisucceeds;test-conditionals:scanflags nothing in the touched test file.gateway remove→fallback logic macOS now uses.nemoclaw destroy --cleanup-gatewayon a physical macOS host to observe the gateway actually removed. The verify hosts are Linux (x86_64 + aarch64); the macOS-specific branch was unified with the Linux path by this change, so the behavior is fully covered by the platform-independent unit tests above.Signed-off-by: Jason Ma jama@nvidia.com
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
openshell gateway remove <name>first, and only runopenshell gateway destroy -g <name>when removal fails.openshell gateway remove <name>.Tests