feat(hermes): add managed tool gateway broker#3742
Merged
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
|
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:
📝 WalkthroughWalkthroughAdds end-to-end Hermes managed-tool gateway support: Docker/build wiring, managed-tool matrix and presets, host broker server and lifecycle helpers, Hermes config/messaging generation, Python plugin runtime patches, onboarding/policy selection integration, session/registry persistence, and tests. ChangesHermes Managed Tool Gateways
Sequence Diagram(s) sequenceDiagram
participant Client
participant Broker
participant Portal
participant OpenShell
participant Upstream
Client->>Broker: HTTP request with broker token or refresh header
Broker->>Portal: POST /api/oauth/token (x-nous-refresh-token)
Portal-->>Broker: access token / agent key
Broker->>OpenShell: openshell provider update (inference/agent key)
Broker->>Upstream: proxy request to service upstream URL (stripped headers)
Upstream-->>Broker: response (filtered headers/body)
Broker-->>Client: proxied response
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
|
🚀 Docs preview ready! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Auto-dispatched E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
agents/hermes/host/tool-gateway-broker.ts (1)
1-5: ⚡ Quick win
@ts-nocheckbypasses TypeScript safety in a.tsfile.The file uses
@ts-nocheckand CommonJSrequiresyntax, effectively treating this as untyped JavaScript despite the.tsextension. Consider either:
- Renaming to
.jsand adjusting the build config if types aren't needed, or- Adding proper TypeScript types and using ES module imports to benefit from type checking.
This is a credential-handling server where type safety would help catch subtle bugs.
🤖 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 `@agents/hermes/host/tool-gateway-broker.ts` around lines 1 - 5, The file begins with "`@ts-nocheck`" and appears to use CommonJS-style runtime in a .ts file, which disables TypeScript safety for a credential-handling server; remove the "`@ts-nocheck`" directive, convert CommonJS requires to ES module imports/exports, and add appropriate TypeScript types/interfaces for the main runtime symbols (e.g., the tool gateway broker initialization function and any exported handlers) so the compiler can catch type errors—if you truly do not want TypeScript, rename the file to .js and update the build config instead.src/lib/hermes-tool-gateway-broker.ts (1)
1-3: 💤 Low valueSPDX license header should appear before
@ts-nocheck.Per coding guidelines, the SPDX license header should be at the top of every source file. The
@ts-nocheckdirective should come after the license header.Suggested reordering
-// `@ts-nocheck` // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +// `@ts-nocheck` // // Thin lifecycle glue for the Hermes managed-tool host broker.As per coding guidelines: "Include SPDX license header at the top of every source file".
🤖 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/hermes-tool-gateway-broker.ts` around lines 1 - 3, Move the SPDX header lines so they appear before the TypeScript directive: place the SPDX‑FileCopyrightText and SPDX‑License-Identifier comment block at the very top of the file and then keep the // `@ts-nocheck` directive immediately after; update the header order near the existing top-of-file comments (the // `@ts-nocheck` line and the SPDX comment lines) so the SPDX header is the first thing in the file.src/lib/onboard.ts (1)
9150-10254: Please rerun the onboarding E2Es on this path.This change set touches core resume, inference, sandbox recreation, and policy application flow. I’d at least rerun the onboarding/sandbox lifecycle jobs plus the Hermes-specific smoke before merge.
As per coding guidelines: "
src/lib/onboard.ts: This file contains core onboarding logic. Changes here affect the full sandbox creation and configuration flow. E2E test recommendation: cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, channels-stop-start-e2e, messaging-compatible-endpoint-e2e, hermes-discord-e2e, hermes-slack-e2e, openshell-gateway-upgrade-e2e."🤖 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/onboard.ts` around lines 9150 - 10254, The diff modifies core onboarding flows (async function onboard) including resume handling, inference setup (setupInference), sandbox creation (createSandbox), policy application (setupPoliciesWithSelection), agent setup (agentOnboard.handleAgentSetup) and post-deploy verification (verifyDeploymentModule.verifyDeployment); rerun the full E2E suites that exercise resume, sandbox lifecycle, agent/Hermes integrations and gateway upgrades: specifically run cloud-e2e, sandbox-operations-e2e, rebuild-openclaw-e2e, channels-stop-start-e2e, messaging-compatible-endpoint-e2e, hermes-discord-e2e, hermes-slack-e2e and openshell-gateway-upgrade-e2e to validate resume/inference/sandbox recreation/policy application paths end-to-end and report any failures so we can iterate on fixes.docs/reference/commands.md (1)
1133-1133: ⚡ Quick winUse active voice for broker startup behavior.
Line 1133 is passive (“it is started only…”). Prefer active voice (for example, “NemoClaw starts this broker only…”).
As per coding guidelines, "Active voice required. Flag passive constructions."
🤖 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 `@docs/reference/commands.md` at line 1133, The sentence "it is started only for Hermes managed-tool gateway sessions" is passive; change it to active voice by naming the actor (e.g., "NemoClaw starts this broker only for Hermes-managed tool gateway sessions") and apply the hyphenation "Hermes-managed" and "tool gateway" as needed; update the sentence in the docs/reference/commands.md where that phrase appears (replace the passive clause with the active clause and ensure punctuation/capitalization matches surrounding style).
🤖 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 `@agents/hermes/config/messaging-config.ts`:
- Around line 36-39: The loop over managedToolGatewayPresets silently skips when
matrix[preset] is missing; change this to fail fast by throwing an error instead
of continuing: inside the for (const preset of managedToolGatewayPresets) loop,
replace the "if (!entry) continue;" behavior with a throw that includes the
preset name and context (e.g., which preset is missing for the
managedToolGatewayPresets → matrix mapping) so envLines only receives fully
validated entries (refer to managedToolGatewayPresets, matrix, envLines, and
entry.envKey/entry.envValue to locate the code).
In `@agents/hermes/Dockerfile`:
- Around line 92-94: Remove the build-time ARG/ENV usage for
NEMOCLAW_HERMES_TOOL_BROKER_TOKEN (and the duplicate usage around the later
ARGs) so the broker token is not baked into the image; stop declaring or setting
NEMOCLAW_HERMES_TOOL_BROKER_TOKEN in the Dockerfile (references: ARG
NEMOCLAW_HERMES_TOOL_BROKER_TOKEN and any subsequent ARG/ENV blocks), and
instead document/expect the secret to be provided at container start (via
runtime environment variable, Docker secrets, Kubernetes secret/volume, or a
credential file mounted into the container) and update any entrypoint/startup
logic to read the token from the runtime source rather than the Dockerfile.
In `@agents/hermes/host/tool-gateway-broker.ts`:
- Around line 549-560: The upstream fetch in the try block calling
fetch(route.upstreamUrl, ...) must include an AbortSignal timeout so hung
upstreams don't block; create a signal via AbortSignal.timeout(timeoutMs) (or an
AbortController if older runtime), pass it as the signal option to fetch
alongside method/headers/body, and update the catch to detect an abort/timeout
(e.g., AbortError) and return an appropriate response (e.g., 504) instead of the
generic 502; references: the fetch call where upstreamResp is assigned,
route.upstreamUrl, buildForwardHeaders(req, route, accessToken), and the
try/catch around upstreamResp.
In `@agents/hermes/plugin/__init__.py`:
- Around line 625-629: The _cleanup_browser_use_cdp_tunnels function currently
only terminates live subprocesses but never removes dead entries from the
_BROWSER_USE_CDP_TUNNELS cache, allowing it to grow unbounded; update this
function (and the equivalent cleanup block referenced around the other
occurrence) to iterate over list(_BROWSER_USE_CDP_TUNNELS.items()), call
proc.poll() to detect liveness, terminate live procs as now, and remove entries
for processes that are already dead (and optionally remove entries after
terminating), making sure to mutate the dict only using keys collected from the
list() to avoid in-loop mutation errors; reference symbols:
_cleanup_browser_use_cdp_tunnels and _BROWSER_USE_CDP_TUNNELS.
In `@src/lib/onboard.ts`:
- Around line 7465-7477: shouldPrepareHermesCredentials can skip credential
reconciliation when the sandbox/provider exists even though hermesToolGateways
selection has changed; update the logic to also force reconciliation when the
configured hermesToolGateways differ from what the provider currently has
registered. Use
getHermesToolGatewayBroker().getHermesToolGatewayProviderName(targetSandbox) to
fetch the provider name, query the provider's registered tool-gateway presets
(instead of relying solely on providerExistsInGateway) and compute a boolean
like toolGatewaySelectionChanged (compare the provider's registered presets to
the current hermesToolGateways array), then include that boolean in the
shouldPrepareHermesCredentials condition so
ensureHermesProvider*Credentials(...) runs when selection drift is detected.
- Around line 5004-5005: The persisted Hermes gateway presets loaded via
registry.getSandbox(sandboxName)?.hermesToolGateways are used directly (const
recordedHermesToolGateways ... ?? []), which can let non-arrays or unknown names
leak into drift checks (hermesToolGatewayDrift), policy merging and patching;
add and call a normalization/validation helper that (a) coerces non-array values
to an empty array, (b) filters out entries that are not valid preset names, and
(c) returns a deduplicated, sorted array, then replace the raw use of
registry.getSandbox(...)? .hermesToolGateways ?? [] with the normalized result
before computing hermesToolGatewayDrift and apply the same helper at the other
noted call sites (lines referenced by similar patterns).
In `@test/onboard.test.ts`:
- Around line 682-685: The test harness is leaking inherited messaging
credentials by spreading process.env into the child env; before the
onboarding/sandbox creation step (the code that sets up process.env and spawns
the resume harness), explicitly delete any DISCORD_* and TELEGRAM_* variables
(e.g. remove process.env.DISCORD_TOKEN, process.env.DISCORD_WEBHOOK_*,
process.env.TELEGRAM_TOKEN, process.env.TELEGRAM_CHAT_ID, and any other keys
matching /^DISCORD_/ or /^TELEGRAM_/) so the child env used by the
resume/onboard flow is hermetic and cannot activate messaging channels when the
test calls the functions that build the sandbox or spawn the child process.
---
Nitpick comments:
In `@agents/hermes/host/tool-gateway-broker.ts`:
- Around line 1-5: The file begins with "`@ts-nocheck`" and appears to use
CommonJS-style runtime in a .ts file, which disables TypeScript safety for a
credential-handling server; remove the "`@ts-nocheck`" directive, convert CommonJS
requires to ES module imports/exports, and add appropriate TypeScript
types/interfaces for the main runtime symbols (e.g., the tool gateway broker
initialization function and any exported handlers) so the compiler can catch
type errors—if you truly do not want TypeScript, rename the file to .js and
update the build config instead.
In `@docs/reference/commands.md`:
- Line 1133: The sentence "it is started only for Hermes managed-tool gateway
sessions" is passive; change it to active voice by naming the actor (e.g.,
"NemoClaw starts this broker only for Hermes-managed tool gateway sessions") and
apply the hyphenation "Hermes-managed" and "tool gateway" as needed; update the
sentence in the docs/reference/commands.md where that phrase appears (replace
the passive clause with the active clause and ensure punctuation/capitalization
matches surrounding style).
In `@src/lib/hermes-tool-gateway-broker.ts`:
- Around line 1-3: Move the SPDX header lines so they appear before the
TypeScript directive: place the SPDX‑FileCopyrightText and
SPDX‑License-Identifier comment block at the very top of the file and then keep
the // `@ts-nocheck` directive immediately after; update the header order near the
existing top-of-file comments (the // `@ts-nocheck` line and the SPDX comment
lines) so the SPDX header is the first thing in the file.
In `@src/lib/onboard.ts`:
- Around line 9150-10254: The diff modifies core onboarding flows (async
function onboard) including resume handling, inference setup (setupInference),
sandbox creation (createSandbox), policy application
(setupPoliciesWithSelection), agent setup (agentOnboard.handleAgentSetup) and
post-deploy verification (verifyDeploymentModule.verifyDeployment); rerun the
full E2E suites that exercise resume, sandbox lifecycle, agent/Hermes
integrations and gateway upgrades: specifically run cloud-e2e,
sandbox-operations-e2e, rebuild-openclaw-e2e, channels-stop-start-e2e,
messaging-compatible-endpoint-e2e, hermes-discord-e2e, hermes-slack-e2e and
openshell-gateway-upgrade-e2e to validate resume/inference/sandbox
recreation/policy application paths end-to-end and report any failures so we can
iterate on fixes.
🪄 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: 3176aa9c-40bd-4065-bf45-d0cd42fe0ff9
📒 Files selected for processing (39)
agents/hermes/Dockerfileagents/hermes/config/build-env.tsagents/hermes/config/hermes-config.tsagents/hermes/config/managed-tool-gateway.tsagents/hermes/config/messaging-config.tsagents/hermes/generate-config.tsagents/hermes/host/managed-tool-gateway-matrix.jsonagents/hermes/host/tool-gateway-broker.tsagents/hermes/plugin/__init__.pyagents/hermes/policy-additions.yamldocs/reference/commands.mdnemoclaw-blueprint/policies/presets/nous-audio.yamlnemoclaw-blueprint/policies/presets/nous-browser.yamlnemoclaw-blueprint/policies/presets/nous-code.yamlnemoclaw-blueprint/policies/presets/nous-image.yamlnemoclaw-blueprint/policies/presets/nous-web.yamlsrc/lib/actions/inference-set.test.tssrc/lib/actions/sandbox/connect.tssrc/lib/actions/sandbox/rebuild.tssrc/lib/actions/sandbox/status.tssrc/lib/hermes-provider-auth.test.tssrc/lib/hermes-provider-auth.tssrc/lib/hermes-tool-gateway-broker.tssrc/lib/oauth-device-code.test.tssrc/lib/oauth-device-code.tssrc/lib/onboard.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/hermes-managed-tools.tssrc/lib/onboard/initial-policy.test.tssrc/lib/onboard/initial-policy.tssrc/lib/onboard/policy-selection.tssrc/lib/onboard/summary.tssrc/lib/state/onboard-session.tssrc/lib/state/registry.tstest/generate-hermes-config.test.tstest/hermes-plugin-handlers.test.tstest/hermes-tool-gateway-broker.test.tstest/onboard.test.tstest/policies.test.ts
ericksoa
force-pushed
the
aerickson/pr-3556-hermes-resume-fix
branch
from
3f767ee to
a9b00c4
Compare
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26045638329
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
agents/hermes/plugin/__init__.py (1)
625-629:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrune stale Browser Use CDP tunnel cache entries.
Line 625 only terminates live subprocesses; it never removes dead/live entries from
_BROWSER_USE_CDP_TUNNELS. Over time, unique CDP URLs accumulate stale entries.Proposed fix
def _cleanup_browser_use_cdp_tunnels(): - for proc, _url in list(_BROWSER_USE_CDP_TUNNELS.values()): + for remote_url, (proc, _url) in list(_BROWSER_USE_CDP_TUNNELS.items()): if proc.poll() is None: proc.terminate() + _BROWSER_USE_CDP_TUNNELS.pop(remote_url, None) @@ def _start_browser_use_cdp_tunnel(cdp_url): + for remote_url, (proc, _url) in list(_BROWSER_USE_CDP_TUNNELS.items()): + if proc.poll() is not None: + _BROWSER_USE_CDP_TUNNELS.pop(remote_url, None) + parsed = urlparse(str(cdp_url or ""))Also applies to: 640-663
🤖 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 `@agents/hermes/plugin/__init__.py` around lines 625 - 629, The cleanup currently only calls proc.terminate() and never removes entries from the _BROWSER_USE_CDP_TUNNELS cache, so stale CDP URLs accumulate; update the _cleanup_browser_use_cdp_tunnels function to iterate over _BROWSER_USE_CDP_TUNNELS.items(), check each proc.poll(): if proc is alive, terminate it and then remove its key from the dict, and if proc is dead simply remove its key (or use a new dict/comprehension to rebuild only live entries), ensuring the cache no longer retains dead or terminated entries; apply the same fix to the other similar cleanup loop that also operates on _BROWSER_USE_CDP_TUNNELS in this module.agents/hermes/Dockerfile (1)
92-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not bake
NEMOCLAW_HERMES_TOOL_BROKER_TOKENinto build args/env.Line 94 and Line 110 make the broker token part of image build metadata/layers. Keep this token runtime-injected only (container env/secret mount at start), not Dockerfile ARG→ENV propagated.
Suggested minimal Dockerfile change
-ARG NEMOCLAW_HERMES_TOOL_BROKER_TOKEN= @@ - NEMOCLAW_HERMES_TOOL_GATEWAY_PRESETS_B64=${NEMOCLAW_HERMES_TOOL_GATEWAY_PRESETS_B64} \ - NEMOCLAW_HERMES_TOOL_BROKER_TOKEN=${NEMOCLAW_HERMES_TOOL_BROKER_TOKEN} + NEMOCLAW_HERMES_TOOL_GATEWAY_PRESETS_B64=${NEMOCLAW_HERMES_TOOL_GATEWAY_PRESETS_B64} +# NOTE: NEMOCLAW_HERMES_TOOL_BROKER_TOKEN must be injected at runtime only.Also applies to: 107-110
🤖 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 `@agents/hermes/Dockerfile` around lines 92 - 94, Remove any Dockerfile ARG or ENV that bakes NEMOCLAW_HERMES_TOOL_BROKER_TOKEN into the image (the ARG NEMOCLAW_HERMES_TOOL_BROKER_TOKEN declaration and any ENV propagation around lines where NEMOCLAW_HERMES_TOOL_BROKER_TOKEN is used, including the similar block at 107-110). Stop passing that secret as a build ARG or embedding it into labels/layers; instead delete the ARG/ENV lines and rely on runtime injection (container environment variable, Kubernetes secret, or secret mount) so the broker token is provided only at container start, not during docker build. Ensure no other Dockerfile instructions (e.g., LABEL, RUN echo, or ENV) reference that token so it cannot end up in image metadata.
🤖 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.
Duplicate comments:
In `@agents/hermes/Dockerfile`:
- Around line 92-94: Remove any Dockerfile ARG or ENV that bakes
NEMOCLAW_HERMES_TOOL_BROKER_TOKEN into the image (the ARG
NEMOCLAW_HERMES_TOOL_BROKER_TOKEN declaration and any ENV propagation around
lines where NEMOCLAW_HERMES_TOOL_BROKER_TOKEN is used, including the similar
block at 107-110). Stop passing that secret as a build ARG or embedding it into
labels/layers; instead delete the ARG/ENV lines and rely on runtime injection
(container environment variable, Kubernetes secret, or secret mount) so the
broker token is provided only at container start, not during docker build.
Ensure no other Dockerfile instructions (e.g., LABEL, RUN echo, or ENV)
reference that token so it cannot end up in image metadata.
In `@agents/hermes/plugin/__init__.py`:
- Around line 625-629: The cleanup currently only calls proc.terminate() and
never removes entries from the _BROWSER_USE_CDP_TUNNELS cache, so stale CDP URLs
accumulate; update the _cleanup_browser_use_cdp_tunnels function to iterate over
_BROWSER_USE_CDP_TUNNELS.items(), check each proc.poll(): if proc is alive,
terminate it and then remove its key from the dict, and if proc is dead simply
remove its key (or use a new dict/comprehension to rebuild only live entries),
ensuring the cache no longer retains dead or terminated entries; apply the same
fix to the other similar cleanup loop that also operates on
_BROWSER_USE_CDP_TUNNELS in this module.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5983c064-4eb5-414e-aa1b-7e15ad7a9cf4
📒 Files selected for processing (39)
agents/hermes/Dockerfileagents/hermes/config/build-env.tsagents/hermes/config/hermes-config.tsagents/hermes/config/managed-tool-gateway.tsagents/hermes/config/messaging-config.tsagents/hermes/generate-config.tsagents/hermes/host/managed-tool-gateway-matrix.jsonagents/hermes/host/tool-gateway-broker.tsagents/hermes/plugin/__init__.pyagents/hermes/policy-additions.yamldocs/reference/commands.mdnemoclaw-blueprint/policies/presets/nous-audio.yamlnemoclaw-blueprint/policies/presets/nous-browser.yamlnemoclaw-blueprint/policies/presets/nous-code.yamlnemoclaw-blueprint/policies/presets/nous-image.yamlnemoclaw-blueprint/policies/presets/nous-web.yamlsrc/lib/actions/inference-set.test.tssrc/lib/actions/sandbox/connect.tssrc/lib/actions/sandbox/rebuild.tssrc/lib/actions/sandbox/status.tssrc/lib/hermes-provider-auth.test.tssrc/lib/hermes-provider-auth.tssrc/lib/hermes-tool-gateway-broker.tssrc/lib/oauth-device-code.test.tssrc/lib/oauth-device-code.tssrc/lib/onboard.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/hermes-managed-tools.tssrc/lib/onboard/initial-policy.test.tssrc/lib/onboard/initial-policy.tssrc/lib/onboard/policy-selection.tssrc/lib/onboard/summary.tssrc/lib/state/onboard-session.tssrc/lib/state/registry.tstest/generate-hermes-config.test.tstest/hermes-plugin-handlers.test.tstest/hermes-tool-gateway-broker.test.tstest/onboard.test.tstest/policies.test.ts
✅ Files skipped from review due to trivial changes (2)
- docs/reference/commands.md
- src/lib/actions/inference-set.test.ts
🚧 Files skipped from review as they are similar to previous changes (34)
- nemoclaw-blueprint/policies/presets/nous-audio.yaml
- src/lib/onboard/initial-policy.ts
- src/lib/state/registry.ts
- agents/hermes/config/build-env.ts
- agents/hermes/host/managed-tool-gateway-matrix.json
- agents/hermes/generate-config.ts
- test/onboard.test.ts
- nemoclaw-blueprint/policies/presets/nous-image.yaml
- nemoclaw-blueprint/policies/presets/nous-browser.yaml
- test/generate-hermes-config.test.ts
- src/lib/actions/sandbox/status.ts
- src/lib/onboard/summary.ts
- src/lib/oauth-device-code.ts
- src/lib/onboard/initial-policy.test.ts
- src/lib/actions/sandbox/connect.ts
- src/lib/actions/sandbox/rebuild.ts
- agents/hermes/config/hermes-config.ts
- src/lib/onboard/dockerfile-patch.ts
- nemoclaw-blueprint/policies/presets/nous-code.yaml
- test/hermes-plugin-handlers.test.ts
- agents/hermes/policy-additions.yaml
- nemoclaw-blueprint/policies/presets/nous-web.yaml
- agents/hermes/config/messaging-config.ts
- agents/hermes/config/managed-tool-gateway.ts
- src/lib/hermes-provider-auth.ts
- src/lib/hermes-tool-gateway-broker.ts
- src/lib/hermes-provider-auth.test.ts
- test/hermes-tool-gateway-broker.test.ts
- src/lib/state/onboard-session.ts
- src/lib/onboard/hermes-managed-tools.ts
- src/lib/onboard/policy-selection.ts
- agents/hermes/host/tool-gateway-broker.ts
- src/lib/oauth-device-code.test.ts
- src/lib/onboard.ts
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 26046368765
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib/onboard.ts (1)
7480-7492:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReconcile Hermes managed-tool preset drift, not just provider existence.
toolGatewayProviderRegisteredonly checks whether the broker provider exists for this sandbox name. On a recreate/resume of the same sandbox, that provider can still hold the old preset set, soshouldPrepareHermesCredentialsgoes false and the newhermesToolGatewaysnever get pushed beforecreateSandbox()consumes them. Compare the provider’s registered preset set againsthermesToolGatewayshere and force reconciliation on drift.🤖 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/onboard.ts` around lines 7480 - 7492, The current toolGatewayProviderRegistered check only ensures a provider exists but not that its registered preset set matches the desired hermesToolGateways, which can miss recreate/resume drift; update the logic around toolGatewayProviderRegistered and shouldPrepareHermesCredentials to fetch the existing provider's preset set from getHermesToolGatewayBroker().getHermesToolGatewayProviderName(targetSandbox) (or a new broker helper like getProviderPresets/getRegisteredPresets) and compare it to the local hermesToolGateways (compare by preset IDs/names as sets), and if they differ force reconciliation by treating toolGatewayProviderRegistered as false (or add a new boolean like presetsDrift) so shouldPrepareHermesCredentials becomes true and pushes the new hermesToolGateways before createSandbox() runs.
🤖 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 `@agents/hermes/plugin/__init__.py`:
- Around line 633-638: The cleanup function _cleanup_browser_use_cdp_tunnels
currently terminates tunnel subprocesses but never reaps them, causing zombie
processes; update the function (and the similar blocks around the other
occurrences noted) to wait() on each subprocess after terminating (or if already
exited) to reap the child—i.e., after checking proc.poll() and calling
proc.terminate(), call proc.wait() (with a short timeout or handle exceptions)
or if proc.poll() is not None call proc.wait() to ensure the process is reaped
and then remove the entry from _BROWSER_USE_CDP_TUNNELS; apply the same pattern
to the other cleanup spots referenced.
---
Duplicate comments:
In `@src/lib/onboard.ts`:
- Around line 7480-7492: The current toolGatewayProviderRegistered check only
ensures a provider exists but not that its registered preset set matches the
desired hermesToolGateways, which can miss recreate/resume drift; update the
logic around toolGatewayProviderRegistered and shouldPrepareHermesCredentials to
fetch the existing provider's preset set from
getHermesToolGatewayBroker().getHermesToolGatewayProviderName(targetSandbox) (or
a new broker helper like getProviderPresets/getRegisteredPresets) and compare it
to the local hermesToolGateways (compare by preset IDs/names as sets), and if
they differ force reconciliation by treating toolGatewayProviderRegistered as
false (or add a new boolean like presetsDrift) so shouldPrepareHermesCredentials
becomes true and pushes the new hermesToolGateways before createSandbox() runs.
🪄 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: 6ab2f68e-0f3a-40ea-ac2d-074049a9d10a
📒 Files selected for processing (15)
agents/hermes/Dockerfileagents/hermes/config/build-env.tsagents/hermes/config/hermes-config.tsagents/hermes/config/messaging-config.tsagents/hermes/generate-config.tsagents/hermes/host/tool-gateway-broker.tsagents/hermes/plugin/__init__.pysrc/lib/hermes-provider-auth.test.tssrc/lib/hermes-provider-auth.tssrc/lib/hermes-tool-gateway-broker.tssrc/lib/onboard.tssrc/lib/onboard/dockerfile-patch.tstest/generate-hermes-config.test.tstest/hermes-tool-gateway-broker.test.tstest/onboard.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- test/onboard.test.ts
- agents/hermes/config/hermes-config.ts
- agents/hermes/config/build-env.ts
- src/lib/hermes-provider-auth.test.ts
- test/hermes-tool-gateway-broker.test.ts
- agents/hermes/host/tool-gateway-broker.ts
- src/lib/hermes-tool-gateway-broker.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
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 `@agents/hermes/plugin/__init__.py`:
- Around line 683-688: When port validation fails and you call proc.terminate(),
the child is not reaped; update the failure branch to properly reap the process:
after proc.terminate() call proc.wait() (with a short timeout) and on timeout
call proc.kill() and wait again, catching exceptions as needed; do this in the
same block where proc is validated (referencing the proc local and the
surrounding code that returns cdp_url) so the terminated process is never left
running and does not rely on _BROWSER_USE_CDP_TUNNELS for cleanup.
🪄 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: 417a0c3a-2c98-41f4-b4e9-7089c27074fa
📒 Files selected for processing (1)
agents/hermes/plugin/__init__.py
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26047443720
|
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26048024149
|
ericksoa
added
Integration: Hermes
Sandbox
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26048616718
|
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-3742.docs.buildwithfern.com/nemoclaw |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26064745453
|
…mes-resume-fix Signed-off-by: Aaron Erickson <aerickson@nvidia.com> # Conflicts: # src/lib/onboard.ts # test/policies.test.ts
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26079802117
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26104889073
|
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Contributor
Selective E2E Results — ✅ All requested jobs passedRun: 26115988321
|
cv
approved these changes
May 19, 2026
12 tasks
miyoungc
added a commit
that referenced
this pull request
May 20, 2026
## Summary Refreshes the NemoClaw docs for v0.0.46 by updating version metadata, release notes, and generated user skills. The refresh also keeps public docs aligned with the docs skip list by removing non-public experimental references from the generated output. ## Related Issue None. ## Changes - #3744 and #3824 -> `docs/about/release-notes.mdx`: Added Windows bootstrap and WSL express install coverage for v0.0.46. - #3392 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/reference/network-policies.mdx`, and policy examples: Refreshed public messaging channel docs around WhatsApp and matching policy presets. - #3742, #3767, #3732, #3786, #3777, and #3808 -> `docs/about/release-notes.mdx`: Added release-note coverage for Hermes managed tools, Bedrock Runtime endpoint detection, WSL Ollama proxying, Model Router Python fallback, plugin command registration, and tool-catalog latency improvements. - #3124 -> `docs/about/release-notes.mdx`: Added release-note coverage for hosted uninstall flag guidance. - Generated `nemoclaw-user-*` skills from the updated MDX docs for the v0.0.46 release. ## 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) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make 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) Verification notes: - Commit hooks passed, including markdownlint, gitleaks, docs-to-skills verification, env-var docs, and skills YAML checks. - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` passed. - `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-links --local-only --with-skills` passed. - `git diff --check` passed. - `make docs` was attempted but blocked before MDX validation because `npx` received HTTP 403 fetching `fern-api` from npm. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Released v0.0.46: improved Windows setup, WhatsApp messaging support, Hermes sandbox/tool routing, Anthropic endpoint compatibility, Ollama proxy routing, model-router fallback, OpenClaw plugin/backup compatibility, sandbox build tooling fixes, and updated uninstall flag behavior. * **Documentation** * Removed WeChat from messaging flows and presets across guides and CLI docs; clarified onboarding and channel setup for WhatsApp. Clarified runtime mutability and filesystem (Landlock) behavior — some changes require sandbox rebuilds; prefer host-side commands for durable config. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3911?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3911?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Maintainer replay of #3556 from @shannonsands / NousResearch, with one maintainer follow-up commit on top to make the branch shippable.
This keeps the original Hermes managed Nous Tool Gateway support: OAuth/subscription onboarding without raw Nous OAuth tokens in the sandbox, a Hermes-owned host broker for selected managed tool planes, broker-mode Hermes config/env generation, and broker-scoped policy presets.
Maintainer Follow-Up
setupInference()could be called withsandboxName === nullwhen the provider/model route was already selected and ready.Original PR
Supersedes/continues #3556.
Original author: @shannonsands
Original PR head before this maintainer patch:
4db9a302c3e385978a12eccc3d9f748c4a91939eMaintainer patch commit:
3f767eeff503a1d36827022033ad3aa075b0b7e3Verification
npm run build:clinpx vitest run test/onboard.test.ts -t "Hermes Provider|sandbox name before"npx vitest run test/onboard.test.ts test/onboard-resume-provider-recovery.test.ts test/onboard-policy-suggestions.test.ts src/lib/state/onboard-session.test.ts src/lib/onboard/dockerfile-patch.test.ts src/lib/hermes-provider-auth.test.ts src/lib/oauth-device-code.test.ts test/generate-hermes-config.test.ts test/hermes-plugin-handlers.test.ts test/hermes-tool-gateway-broker.test.ts test/policies.test.tsnpm run checksgit diff --checkMerge Gate
Before approval/merge, still require one live Hermes OAuth onboarding smoke with at least
nous-web, broker health on127.0.0.1:11436/health, and one managed-tool call through the sandbox. If local Nous OAuth/subscription is unavailable, get that exact smoke result from Nous before merging.Signed-off-by: Shannon Sands shannon@nousresearch.com
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests