fix(tunnel): stop agent gateway forwards on shutdown - #6450
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:
📝 WalkthroughWalkthroughThis PR adds Estimated code review effort: 4 (Complex) | ~60 minutes ChangesAgent-aware stop flow
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/tunnel/services.ts (1)
636-658: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFailure-path message still says generic "in-sandbox gateway" instead of using
gatewayLabel.
reportStopResultnow threadsgatewayLabelthrough the success (${gatewayLabel} stopped inside sandbox.) and not-running (${gatewayLabel} was not running inside sandbox.) branches, but the failure branch at Line 654 still hardcodes"Could not stop in-sandbox gateway"instead of${gatewayLabel}. Given the linked issue explicitly calls for correcting the shutdown output to name the actual agent gateway, this branch should be updated too for consistency.🐛 Proposed fix
warn( - `Could not stop in-sandbox gateway (exit ${String(result.status ?? "unknown")}).` + + `Could not stop ${gatewayLabel} (exit ${String(result.status ?? "unknown")}).` + " The sandbox may be unreachable or the gateway may still be running." + (details ? ` Details: ${details}` : ""), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tunnel/services.ts` around lines 636 - 658, The failure branch in reportStopResult still hardcodes a generic “in-sandbox gateway” message instead of using gatewayLabel, so update the warn call in reportStopResult to interpolate the provided gatewayLabel consistently with the success and not-running branches. Keep the existing stderr/stdout details handling, but make the shutdown failure message identify the actual gateway being stopped.
🧹 Nitpick comments (2)
src/lib/tunnel/agent-forward-stop.test.ts (1)
20-126: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the default spawnSync runners and the "openshell not found" branch.
Every test here injects
runOpenshell/runCaptureOpenshell/resolveOpenshell, so the realmakeRunOpenshell/makeRunCaptureOpenshell(spawnSync command construction, stdio/timeout handling, throw-on-nonzero-status) and theopenshell not foundwarn-and-return path inagent-forward-stop.tsare never exercised. Given this module is being wired intostopAllto fix the port-8642 release bug (#6392), it's worth locking down the actual host-boundary behavior, not just the injected-fake path.🧪 Suggested additions
+ it("warns and returns when OpenShell cannot be resolved", () => { + const warn = vi.fn<(message: string) => void>(); + const runOpenshell = vi.fn(); + + stopAgentForwardPortsForStop("nemohermes", { + getSessionAgent: () => ({ displayName: "Hermes Agent", forward_ports: [8642] }), + getSandbox: () => null, + resolveOpenshell: () => null, + runOpenshell, + warn, + }); + + expect(runOpenshell).not.toHaveBeenCalled(); + expect(warn.mock.calls.map((call) => call[0]).join("\n")).toContain("openshell not found"); + }); + + it("invokes the real OpenShell binary via spawnSync for a stop command", () => { + // mock node:child_process spawnSync via vi.mock to assert args/encoding/stdio + // without exercising deps.runOpenshell/runCaptureOpenshell overrides. + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tunnel/agent-forward-stop.test.ts` around lines 20 - 126, The current tests only cover injected fakes, so they miss the real default runner behavior and the “openshell not found” branch in stopAgentForwardPortsForStop. Add coverage that exercises the default makeRunOpenshell and makeRunCaptureOpenshell paths so spawnSync command construction, stdio/timeout handling, and nonzero-status errors are validated, and add a case where resolveOpenshell returns nothing to assert the warn-and-return flow. Use the stopAgentForwardPortsForStop, makeRunOpenshell, and makeRunCaptureOpenshell symbols to target the relevant paths.src/lib/tunnel/services-sandbox.test.ts (1)
138-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider suffixing the test title with the tracked issue.
Behavior-oriented title reads well, but this test directly exercises the fix for the linked issue.
✏️ Suggested title
- it("uses the active agent gateway command for Hermes shutdown", () => { + it("uses the active agent gateway command for Hermes shutdown (`#6392`)", () => {As per coding guidelines, "
**/*.test.ts: Write behavior-oriented test titles, and put local issue references in a final(#1234)suffix."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tunnel/services-sandbox.test.ts` around lines 138 - 165, Update the test title in services-sandbox.test.ts so it remains behavior-oriented but ends with the tracked issue reference in the required suffix format, since this test directly covers the fix. Adjust the it(...) description for the Hermes shutdown test to include the local issue tag at the end, keeping the rest of the assertion logic unchanged and matching the naming convention used by the surrounding test suite.Source: Coding guidelines
🤖 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/tunnel/services-sandbox.test.ts`:
- Line 242: The assertion in services-sandbox.test should be content-aware
because `expect(args).not.toContain(expect.stringContaining(...))` only compares
array elements by identity and can miss matching substrings. Update the check
near the `args` assertion to use a content-based matcher such as
`toContainEqual(expect.stringContaining("find_gateway_pids"))` or a predicate
over `args` that inspects each string’s contents, so the test actually verifies
whether any argument includes `find_gateway_pids`.
---
Outside diff comments:
In `@src/lib/tunnel/services.ts`:
- Around line 636-658: The failure branch in reportStopResult still hardcodes a
generic “in-sandbox gateway” message instead of using gatewayLabel, so update
the warn call in reportStopResult to interpolate the provided gatewayLabel
consistently with the success and not-running branches. Keep the existing
stderr/stdout details handling, but make the shutdown failure message identify
the actual gateway being stopped.
---
Nitpick comments:
In `@src/lib/tunnel/agent-forward-stop.test.ts`:
- Around line 20-126: The current tests only cover injected fakes, so they miss
the real default runner behavior and the “openshell not found” branch in
stopAgentForwardPortsForStop. Add coverage that exercises the default
makeRunOpenshell and makeRunCaptureOpenshell paths so spawnSync command
construction, stdio/timeout handling, and nonzero-status errors are validated,
and add a case where resolveOpenshell returns nothing to assert the
warn-and-return flow. Use the stopAgentForwardPortsForStop, makeRunOpenshell,
and makeRunCaptureOpenshell symbols to target the relevant paths.
In `@src/lib/tunnel/services-sandbox.test.ts`:
- Around line 138-165: Update the test title in services-sandbox.test.ts so it
remains behavior-oriented but ends with the tracked issue reference in the
required suffix format, since this test directly covers the fix. Adjust the
it(...) description for the Hermes shutdown test to include the local issue tag
at the end, keeping the rest of the assertion logic unchanged and matching the
naming convention used by the surrounding test suite.
🪄 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: b1a6b452-1917-4186-91ab-b359c243e8dc
📒 Files selected for processing (5)
src/lib/tunnel/agent-forward-stop.test.tssrc/lib/tunnel/agent-forward-stop.tssrc/lib/tunnel/services-gateway-ownership.test.tssrc/lib/tunnel/services-sandbox.test.tssrc/lib/tunnel/services.ts
Signed-off-by: Rui Luo <ruluo@nvidia.com>
Signed-off-by: Rui Luo <ruluo@nvidia.com>
5aae2a6 to
7cd24b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/lib/tunnel/agent-forward-stop.ts (1)
84-138: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid re-listing OpenShell for every port.
stopAgentForwardPortsForStopcallsbestEffortForwardStopForSandboxonce per port, and each call runsopenshell forward listagain. Ifforward_portscan contain multiple entries, this adds one shell round-trip per port; hoist the list lookup out of the loop if the per-port refresh isn’t needed here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/tunnel/agent-forward-stop.ts` around lines 84 - 138, `stopAgentForwardPortsForStop` is re-running the OpenShell forward list for every entry in `forward_ports` via `bestEffortForwardStopForSandbox`, causing unnecessary shell round-trips. Refactor the `stopAgentForwardPortsForStop` flow to fetch the OpenShell forward list once outside the per-port loop (using the existing `resolveOpenshell`, `makeRunOpenshell`, and `makeRunCaptureOpenshell` wiring), then reuse that result while iterating ports, keeping the existing stop/warn/info behavior intact.
🤖 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/tunnel/agent-forward-stop.ts`:
- Around line 84-138: The legacy OpenClaw gateway-stop path is still being
selected alongside the new agent-aware cleanup, so remove the obsolete
`openclaw-gateway` / `openclaw gateway` handling from `services.ts` and ensure
`stopAll` no longer routes through that branch. Update the stop flow to use
`stopAgentForwardPortsForStop` and related agent-forward logic only, and keep
any old path behind an explicit compatibility gate if it must remain.
---
Nitpick comments:
In `@src/lib/tunnel/agent-forward-stop.ts`:
- Around line 84-138: `stopAgentForwardPortsForStop` is re-running the OpenShell
forward list for every entry in `forward_ports` via
`bestEffortForwardStopForSandbox`, causing unnecessary shell round-trips.
Refactor the `stopAgentForwardPortsForStop` flow to fetch the OpenShell forward
list once outside the per-port loop (using the existing `resolveOpenshell`,
`makeRunOpenshell`, and `makeRunCaptureOpenshell` wiring), then reuse that
result while iterating ports, keeping the existing stop/warn/info behavior
intact.
🪄 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: 359f40e1-aef7-4170-8f5a-f56815411dd5
📒 Files selected for processing (5)
src/lib/tunnel/agent-forward-stop.test.tssrc/lib/tunnel/agent-forward-stop.tssrc/lib/tunnel/services-gateway-ownership.test.tssrc/lib/tunnel/services-sandbox.test.tssrc/lib/tunnel/services.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/lib/tunnel/services-sandbox.test.ts
- src/lib/tunnel/services-gateway-ownership.test.ts
- src/lib/tunnel/agent-forward-stop.test.ts
- src/lib/tunnel/services.ts
Signed-off-by: Rui Luo <ruluo@nvidia.com>
|
Cross-PR coordination for #4960 / #4951: #4960 is now exact-head green and both review advisors recommend |
cv
left a comment
There was a problem hiding this comment.
Reviewed exact head 2b47214 with a focused security/process-boundary pass. Forward cleanup is sandbox-scoped and fails closed on ownership-list failure; port inputs are bounded; OpenShell calls use argv/stdin rather than shell interpolation; generated gateway regexes are escaped and shell-quoted; terminal-only agents are excluded. Focused tests cover cross-sandbox ownership, list failure, invalid/duplicate ports, Hermes launcher/re-exec patterns, fallback execution, and terminal-agent behavior. All 33 checks and contributor-compliance gates are green. No blocking findings.
Withdrawing approval pending the exact macOS/Colima acceptance evidence from #6392: on current head, nemohermes stop should release the openshell ssh-proxy listener on port 8642 within 5 seconds and emit Hermes-labeled output. The focused code/security review remains clean; this is an acceptance-evidence gap.
|
Focused code/security review is clean: ownership lookup fails closed, stops are sandbox-scoped, manifest-derived patterns are escaped/quoted, and terminal agents are protected. One acceptance gap remains before approval: the green macOS job did not exercise Docker/Colima or this repro. Please record current-head macOS/Apple Silicon evidence for issue #6392 showing (1) |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results — ✅ All selected jobs passedRun: 28968021725
|
|
Current-head macOS/Apple Silicon acceptance proof for #6392 (refreshed after the final helper-boundary validation):
This satisfies the requested real Apple Silicon/Colima proof on exact head |
E2E Target Results — ❌ Some jobs failedRun: 28968019581
|
cjagwani
left a comment
There was a problem hiding this comment.
Approved exact verified head 41033af2284ec3c90a18c8ddde2bca1f950e04a9. The shutdown path is sandbox- and gateway-scoped, fails closed when ownership or binding cannot be established, preserves supervised non-OpenClaw gateway runtimes, and verifies host-forward release before reporting success. Exact-head required CI is green; 55 focused tunnel/process-boundary tests and Node 22 CLI typechecking pass locally. Current-head macOS arm64/Colima/OpenShell 0.0.72 acceptance evidence also proves port 8642 is listening before stop, released on the first poll, and reported with the Hermes Agent label.
E2E Target Results — ✅ All requested jobs passedRun: 28968460032
|
ericksoa
left a comment
There was a problem hiding this comment.
Approved exact head 41033af2284ec3c90a18c8ddde2bca1f950e04a9 after focused correctness, process-boundary, and overlap review.
No blocking findings remain. This head scopes forward enumeration/stop to the sandbox's persisted gateway, confirms real listener release before reporting success, preserves sandbox-supervised Hermes gateway behavior, and integrates #4960's PID/start-time/owner/marker-gated OpenClaw shutdown plus exact pod selection.
Validation includes clean exact-head PR CI and CodeRabbit, 57 focused tests passing locally (15 Linux-only script cases skipped on macOS and passing in CI), real Apple Silicon/Colima proof that the 8642 listener is released immediately with Hermes Agent-labeled output, and passing advisor-required E2E coverage for tunnel lifecycle, Hermes dashboard, sandbox operations, and both typed live targets.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results — ✅ All requested jobs passedRun: 28968019581
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
@rluo8 Main advanced through #4960 and made this PR dirty. I prepared the narrow current-main refresh at signed commit The only conflicts were the expected GitHub reports git fetch https://github.com/NVIDIA/NemoClaw.git codex/pr-6450-main-refresh
git push origin FETCH_HEAD:fix/6392-nemohermes-stop-forwardI will recheck the exact relayed head and CI immediately afterward. |
E2E Target Results — ✅ All requested jobs passedRun: 28973140165
|
E2E Target Results —
|
| Job | Result |
|---|---|
| channels-stop-start |
E2E Target Results — ✅ All selected jobs passedRun: 28973716384
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results —
|
| Job | Result |
|---|---|
| channels-stop-start | |
| concurrent-gateway-ports | ✅ success |
| hermes-dashboard | |
| hermes-e2e | |
| sandbox-operations | |
| tunnel-lifecycle | ✅ success |
E2E Target Results — ❌ Some jobs failedRun: 28974149007
|
E2E Target Results — ✅ All requested jobs passedRun: 28974148865
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results —
|
| Job | Result |
|---|---|
| full-e2e |
E2E Target Results — ❌ Some jobs failedRun: 28975011600
|
E2E Target Results — ✅ All selected jobs passedRun: 28975084156
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results — ❌ Some jobs failedRun: 28975082325
|
E2E Target Results — ❌ Some jobs failedRun: 28975686326
|
E2E Target Results — ✅ All requested jobs passedRun: 28975948133
|
E2E Target Results — ✅ All requested jobs passedRun: 28976061199
|
E2E Target Results — ✅ All requested jobs passedRun: 28975540880
|
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user documentation for NemoClaw v0.0.78 by replacing the unreleased section with release highlights and synchronizing the affected inference, lifecycle, messaging, and CLI reference pages with merged behavior. ## Changes - Publish the v0.0.78 release-notes section with links to the most specific user guides for each shipped behavior. - Document authoritative Deep Agents route health, Nemotron Ultra profile behavior, and Hermes compatible-endpoint context metadata. - Document forced rebuild recovery after total backup failure and the ownership-safe tunnel/full-stop behavior. - Keep command examples and shared agent variants aligned with the current OpenClaw, Hermes, and Deep Agents interfaces. Source mapping: - [#3787](#3787) -> `docs/about/release-notes.mdx`: Record reliable workspace template seeding during sandbox startup. - [#4960](#4960) -> `docs/about/release-notes.mdx`: Record safer detection of rewritten OpenClaw gateway processes. - [#5676](#5676) -> `docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON handling. - [#5857](#5857) -> `docs/about/release-notes.mdx`: Record synchronization of explicit OpenClaw main-agent model state. - [#5929](#5929) -> `docs/about/release-notes.mdx`: Record copyable SSH port-forward guidance for remote dashboards. - [#6068](#6068) -> `docs/about/release-notes.mdx`: Record custom-image plugin provenance reconciliation. - [#6116](#6116) -> `docs/about/release-notes.mdx`: Record live-loopback dashboard-forward recovery. - [#6122](#6122) -> `docs/about/release-notes.mdx`: Announce validated, round-trippable policy YAML output. - [#6211](#6211) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild --force` recovery boundary. - [#6283](#6283) -> `docs/about/release-notes.mdx`: Record Hermes WebUI port alignment. - [#6293](#6293) -> `docs/inference/switch-inference-providers.mdx`, `docs/about/release-notes.mdx`: Document compatible-endpoint context-length probing for Hermes. - [#6320](#6320) -> `docs/about/release-notes.mdx`: Record bounded gateway-recovery waits. - [#6377](#6377) -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain rebuild diagnostics and prepared MCP-destroy recovery. - [#6412](#6412) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document authoritative agent-visible inference route health. - [#6421](#6421) -> `docs/about/release-notes.mdx`: Record the longer quiet-pull window for managed vLLM images. - [#6431](#6431) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document the version-pinned Nemotron Ultra profile plugin. - [#6439](#6439) -> `docs/about/release-notes.mdx`: Summarize the authenticated, pinned credential-capture helper boundary. - [#6450](#6450) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document host-forward cleanup and ownership-safe gateway-port release. - [#6474](#6474) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Record composable OpenClaw messaging runtime loaders. - [#6475](#6475) -> `docs/about/release-notes.mdx`: Record removal of the unavailable Kimi K2.6 production endpoint option. - [#6480](#6480) -> `docs/about/release-notes.mdx`: Record stderr routing for the plugin registration banner. - [#6481](#6481) -> `docs/about/release-notes.mdx`: Record post-pull Ollama model discovery checks. - [#6482](#6482) -> `docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon restart. - [#6486](#6486) -> `docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep Agents auto-approval boundary. - [#6490](#6490) -> `docs/about/release-notes.mdx`: Record diagnostics for custom images missing the managed runtime. - [#6494](#6494) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document nonempty tool-call content preservation and placeholder rejection. - [#6497](#6497) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document isolated Deep Agents route-probe output. - [#6506](#6506) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document observability-preserving managed route probes. - [#6508](#6508) -> `docs/about/release-notes.mdx`: Link the new extension taxonomy and SDK-readiness reference from the release summary. Release-source verification: GitHub reports all 29 cited source PRs as merged with base `main`, and every merge commit is an ancestor of `origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No source-mapping mismatches were found. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] 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: Documentation-only release-prep changes; `npm run docs` validates variants, routes, and Fern content. - [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 are not applicable to this documentation-only change set. - [ ] 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) — exited 0 with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast 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) --- <!-- 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: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 sentences: what this PR does and why. --> Fix `nemohermes stop` cleanup for Hermes sandboxes by stopping the active agent gateway and its host forwards instead of only looking for OpenClaw gateway processes. The fix also skips in-sandbox gateway shutdown for terminal-only agents, so terminal agent sessions are not accidentally killed. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Fixes NVIDIA#6392 ## Changes <!-- Bullet list of key changes. --> - Add agent-aware in-sandbox gateway stop patterns, including Hermes `hermes.real gateway run`. - Skip gateway process termination when the active agent has no gateway runtime. - Stop agent-declared host forwards plus the dashboard forward before releasing the managed OpenShell gateway port. - Run the OpenShell fallback stop script through stdin to avoid newline argv issues. - Add targeted coverage for Hermes stop, forward cleanup, terminal-agent guard behavior, and gateway stop pattern generation. ## 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 <!-- 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: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [√] Docs not applicable — justification: shutdown cleanup behavior changed, but no user-facing docs or commands changed. - [√] 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. --> - [√] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [√] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [√] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: - [√] 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](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- 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: Rui Luo <ruluo@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved shutdown handling for sandbox gateways and host port forwards, including better detection of which ports should be stopped. * **Bug Fixes** * Added safer fallbacks when gateway information is unavailable, reducing the chance of stopping the wrong forward. * Improved cleanup behavior when forwarded ports belong to another sandbox or can’t be confirmed released. * Refined shutdown flows for different agent/runtime types so unsupported cases are skipped cleanly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rui Luo <ruluo@nvidia.com> Signed-off-by: Aaron Erickson <aerickson@nvidia.com> Signed-off-by: cjagwani <cjagwani@nvidia.com> Co-authored-by: Aaron Erickson <aerickson@nvidia.com> Co-authored-by: cjagwani <cjagwani@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Prepares the user documentation for NemoClaw v0.0.78 by replacing the unreleased section with release highlights and synchronizing the affected inference, lifecycle, messaging, and CLI reference pages with merged behavior. ## Changes - Publish the v0.0.78 release-notes section with links to the most specific user guides for each shipped behavior. - Document authoritative Deep Agents route health, Nemotron Ultra profile behavior, and Hermes compatible-endpoint context metadata. - Document forced rebuild recovery after total backup failure and the ownership-safe tunnel/full-stop behavior. - Keep command examples and shared agent variants aligned with the current OpenClaw, Hermes, and Deep Agents interfaces. Source mapping: - [NVIDIA#3787](NVIDIA#3787) -> `docs/about/release-notes.mdx`: Record reliable workspace template seeding during sandbox startup. - [NVIDIA#4960](NVIDIA#4960) -> `docs/about/release-notes.mdx`: Record safer detection of rewritten OpenClaw gateway processes. - [NVIDIA#5676](NVIDIA#5676) -> `docs/about/release-notes.mdx`: Record warning-tolerant agent-list JSON handling. - [NVIDIA#5857](NVIDIA#5857) -> `docs/about/release-notes.mdx`: Record synchronization of explicit OpenClaw main-agent model state. - [NVIDIA#5929](NVIDIA#5929) -> `docs/about/release-notes.mdx`: Record copyable SSH port-forward guidance for remote dashboards. - [NVIDIA#6068](NVIDIA#6068) -> `docs/about/release-notes.mdx`: Record custom-image plugin provenance reconciliation. - [NVIDIA#6116](NVIDIA#6116) -> `docs/about/release-notes.mdx`: Record live-loopback dashboard-forward recovery. - [NVIDIA#6122](NVIDIA#6122) -> `docs/about/release-notes.mdx`: Announce validated, round-trippable policy YAML output. - [NVIDIA#6211](NVIDIA#6211) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain the explicit no-backup `rebuild --force` recovery boundary. - [NVIDIA#6283](NVIDIA#6283) -> `docs/about/release-notes.mdx`: Record Hermes WebUI port alignment. - [NVIDIA#6293](NVIDIA#6293) -> `docs/inference/switch-inference-providers.mdx`, `docs/about/release-notes.mdx`: Document compatible-endpoint context-length probing for Hermes. - [NVIDIA#6320](NVIDIA#6320) -> `docs/about/release-notes.mdx`: Record bounded gateway-recovery waits. - [NVIDIA#6377](NVIDIA#6377) -> `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Explain rebuild diagnostics and prepared MCP-destroy recovery. - [NVIDIA#6412](NVIDIA#6412) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document authoritative agent-visible inference route health. - [NVIDIA#6421](NVIDIA#6421) -> `docs/about/release-notes.mdx`: Record the longer quiet-pull window for managed vLLM images. - [NVIDIA#6431](NVIDIA#6431) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document the version-pinned Nemotron Ultra profile plugin. - [NVIDIA#6439](NVIDIA#6439) -> `docs/about/release-notes.mdx`: Summarize the authenticated, pinned credential-capture helper boundary. - [NVIDIA#6450](NVIDIA#6450) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/about/release-notes.mdx`: Document host-forward cleanup and ownership-safe gateway-port release. - [NVIDIA#6474](NVIDIA#6474) -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/about/release-notes.mdx`: Record composable OpenClaw messaging runtime loaders. - [NVIDIA#6475](NVIDIA#6475) -> `docs/about/release-notes.mdx`: Record removal of the unavailable Kimi K2.6 production endpoint option. - [NVIDIA#6480](NVIDIA#6480) -> `docs/about/release-notes.mdx`: Record stderr routing for the plugin registration banner. - [NVIDIA#6481](NVIDIA#6481) -> `docs/about/release-notes.mdx`: Record post-pull Ollama model discovery checks. - [NVIDIA#6482](NVIDIA#6482) -> `docs/about/release-notes.mdx`: Record Ollama model warm-up after daemon restart. - [NVIDIA#6486](NVIDIA#6486) -> `docs/about/release-notes.mdx`: Publish the opt-in, thread-scoped Deep Agents auto-approval boundary. - [NVIDIA#6490](NVIDIA#6490) -> `docs/about/release-notes.mdx`: Record diagnostics for custom images missing the managed runtime. - [NVIDIA#6494](NVIDIA#6494) -> `docs/inference/model-capability-audit.mdx`, `docs/about/release-notes.mdx`: Document nonempty tool-call content preservation and placeholder rejection. - [NVIDIA#6497](NVIDIA#6497) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document isolated Deep Agents route-probe output. - [NVIDIA#6506](NVIDIA#6506) -> `docs/get-started/quickstart-langchain-deepagents-code.mdx`, `docs/about/release-notes.mdx`: Document observability-preserving managed route probes. - [NVIDIA#6508](NVIDIA#6508) -> `docs/about/release-notes.mdx`: Link the new extension taxonomy and SDK-readiness reference from the release summary. Release-source verification: GitHub reports all 29 cited source PRs as merged with base `main`, and every merge commit is an ancestor of `origin/main` at `17bf9a6a9688b3b1d69cf4b37d3f23110acb055e`. No source-mapping mismatches were found. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] 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: Documentation-only release-prep changes; `npm run docs` validates variants, routes, and Fern content. - [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 are not applicable to this documentation-only change set. - [ ] 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) — exited 0 with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode contrast 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) --- <!-- 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: Charan Jagwani <cjagwani@nvidia.com> --------- Signed-off-by: cjagwani <cjagwani@nvidia.com>
Summary
Fix
nemohermes stopcleanup for Hermes sandboxes by stopping the active agent gateway and its host forwards instead of only looking for OpenClaw gateway processes. The fix also skips in-sandbox gateway shutdown for terminal-only agents, so terminal agent sessions are not accidentally killed.Related Issue
Fixes #6392
Changes
hermes.real gateway run.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Rui Luo ruluo@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes