feat(hermes): add managed tool gateway broker - #3556
Closed
shannonsands wants to merge 3 commits into
Closed
Conversation
Contributor
|
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a Hermes managed-tool gateway broker: host-side proxy and lifecycle, onboarding selection and Dockerfile build wiring, Hermes config and plugin broker-mode patches, policy presets for managed gateways, OAuth header change, registry/session plumbing, and comprehensive tests. ChangesHermes Managed Tool Gateway Broker System
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard.ts (1)
10153-10178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist cleared Hermes managed-tool selections too.
The comment above this helper says
nullclears andundefinedleaves unchanged, but the truthy guard drops both[]andnull. Switching away from Hermes or deselecting all managed tools therefore leaves the previous session value behind, and resume/drift handling can resurrect stale broker state.Proposed fix
- if (updates.hermesToolGateways) normalized.hermesToolGateways = updates.hermesToolGateways; + if (updates.hermesToolGateways !== undefined) { + normalized.hermesToolGateways = updates.hermesToolGateways; + }🤖 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 10153 - 10178, The guard for hermesToolGateways uses a truthy check so null and empty-array values get dropped; change the conditional to check for undefined instead (e.g., replace "if (updates.hermesToolGateways) normalized.hermesToolGateways = updates.hermesToolGateways;" with "if (updates.hermesToolGateways !== undefined) normalized.hermesToolGateways = updates.hermesToolGateways;") so that explicit null (to clear) and [] (to deselect all) are persisted; apply the same undefined-check pattern to any other fields using truthy guards like messagingChannels or policyPresets if present.
🤖 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/onboard.ts`:
- Around line 389-423: The managed-tool onboarding helpers (e.g., the
HERMES_TOOL_GATEWAY_PRESETS constant and HERMES_TOOL_GATEWAY_PRESET_NAMES set,
plus related helper functions in the ranges around lines 1686-1935 and
9424-9687) should be extracted out of src/lib/onboard.ts into one or more
focused modules under src/lib/onboard/ (for example
src/lib/onboard/hermesGateway.ts and src/lib/onboard/managedTools.ts); move the
constants and any helper functions that build/manipulate gateway selection or
policy wiring into those new files, export the necessary symbols
(HERMES_TOOL_GATEWAY_PRESETS, HERMES_TOOL_GATEWAY_PRESET_NAMES, and their helper
functions), and update onboard.ts to import these exports so onboard.ts only
orchestrates flow and does not contain the implementation details.
- Around line 7184-7185: Call setupHermesToolGateways with the recorded/previous
gateways so the managed-tool picker is seeded from the existing sandbox
selection (e.g., change the call from setupHermesToolGateways(provider,
hermesAuthMethod) to setupHermesToolGateways(provider, hermesAuthMethod,
existing) or the local variable that holds the recorded gateways), ensuring the
existing value is passed through to avoid hermesToolGatewayDrift and unnecessary
recreates.
---
Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 10153-10178: The guard for hermesToolGateways uses a truthy check
so null and empty-array values get dropped; change the conditional to check for
undefined instead (e.g., replace "if (updates.hermesToolGateways)
normalized.hermesToolGateways = updates.hermesToolGateways;" with "if
(updates.hermesToolGateways !== undefined) normalized.hermesToolGateways =
updates.hermesToolGateways;") so that explicit null (to clear) and [] (to
deselect all) are persisted; apply the same undefined-check pattern to any other
fields using truthy guards like messagingChannels or policyPresets if present.
🪄 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: 6483ead0-a5c2-418f-bc79-85832c4ec59f
📒 Files selected for processing (35)
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/initial-policy.test.tssrc/lib/onboard/initial-policy.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/policies.test.ts
Contributor
|
✨ Thanks for submitting this detailed PR to add Hermes managed Nous Tool Gateway support. This change aims to improve the onboarding process by keeping raw Nous OAuth tokens out of the sandbox and generating broker-mode Hermes config/env. |
Signed-off-by: Test User <test@example.com>
shannonsands
force-pushed
the
ns322/hermes-managed-tool-gateway-clean
branch
from
May 18, 2026 03:48
174904c to
179826c
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard.ts (1)
9746-9756:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResolve the sandbox name before re-running Hermes inference on resume.
This branch can execute on a resumed session that finished provider/inference setup but never reached sandbox creation, so
sandboxNameis still null by design.setupInference()now hard-fails on Line 7450 for Hermes, which turns a valid resume into an immediate abort.Suggested fix
if (provider === hermesProviderAuth.HERMES_PROVIDER_NAME) { + if (!sandboxName) { + sandboxName = await promptValidatedSandboxName(agent); + } startRecordedStep("inference", { provider, model }); const inferenceResult = await setupInference( sandboxName, model, provider,🤖 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 9746 - 9756, When provider === hermesProviderAuth.HERMES_PROVIDER_NAME ensure sandboxName is resolved before calling setupInference: check if sandboxName is null/undefined and, if so, await the existing sandbox resolution/creation routine used elsewhere in this module to populate sandboxName, then proceed to startRecordedStep("inference", ...) and call setupInference(sandboxName, ...); reference the symbols provider, hermesProviderAuth.HERMES_PROVIDER_NAME, sandboxName, and setupInference to locate where to insert the resolution call.
🧹 Nitpick comments (1)
agents/hermes/Dockerfile (1)
92-110: 💤 Low valueAcknowledge: Broker token baked into image layer by design.
Trivy flags
NEMOCLAW_HERMES_TOOL_BROKER_TOKENas potential secret exposure (DS-0031). Per the PR objectives, this is intentional—the broker token is an opaque, sandbox-visible credential that replaces raw Nous OAuth tokens. The same pattern already exists forNEMOCLAW_PROVIDER_KEY.For future consideration: if these tokens become more sensitive, consider injecting them at runtime via mounted secrets rather than build args, but the current approach aligns with the broker architecture.
🤖 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 - 110, Add a short inline comment above the ARG/ENV block stating that baking NEMOCLAW_HERMES_TOOL_BROKER_TOKEN into the image is intentional (matches existing NEMOCLAW_PROVIDER_KEY pattern) and that this is an opaque, sandbox-visible broker credential; reference the ARG NEMOCLAW_HERMES_TOOL_BROKER_TOKEN and ENV NEMOCLAW_HERMES_TOOL_BROKER_TOKEN by name and mention the Trivy DS-0031 false-positive, and optionally note the future alternative of runtime-mounted secrets if sensitivity increases.
🤖 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.
Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 9746-9756: When provider ===
hermesProviderAuth.HERMES_PROVIDER_NAME ensure sandboxName is resolved before
calling setupInference: check if sandboxName is null/undefined and, if so, await
the existing sandbox resolution/creation routine used elsewhere in this module
to populate sandboxName, then proceed to startRecordedStep("inference", ...) and
call setupInference(sandboxName, ...); reference the symbols provider,
hermesProviderAuth.HERMES_PROVIDER_NAME, sandboxName, and setupInference to
locate where to insert the resolution call.
---
Nitpick comments:
In `@agents/hermes/Dockerfile`:
- Around line 92-110: Add a short inline comment above the ARG/ENV block stating
that baking NEMOCLAW_HERMES_TOOL_BROKER_TOKEN into the image is intentional
(matches existing NEMOCLAW_PROVIDER_KEY pattern) and that this is an opaque,
sandbox-visible broker credential; reference the ARG
NEMOCLAW_HERMES_TOOL_BROKER_TOKEN and ENV NEMOCLAW_HERMES_TOOL_BROKER_TOKEN by
name and mention the Trivy DS-0031 false-positive, and optionally note the
future alternative of runtime-mounted secrets if sensitivity increases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 16f12a28-8e51-4a31-9ec6-1aa4c7c4e403
📒 Files selected for processing (32)
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.ts
✅ Files skipped from review due to trivial changes (2)
- src/lib/actions/inference-set.test.ts
- src/lib/oauth-device-code.test.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- src/lib/onboard/initial-policy.ts
- src/lib/onboard/dockerfile-patch.ts
- src/lib/hermes-provider-auth.ts
- nemoclaw-blueprint/policies/presets/nous-code.yaml
- docs/reference/commands.md
- src/lib/actions/sandbox/status.ts
- src/lib/actions/sandbox/connect.ts
- nemoclaw-blueprint/policies/presets/nous-web.yaml
- nemoclaw-blueprint/policies/presets/nous-image.yaml
- nemoclaw-blueprint/policies/presets/nous-audio.yaml
- agents/hermes/config/build-env.ts
- agents/hermes/generate-config.ts
- agents/hermes/host/managed-tool-gateway-matrix.json
- nemoclaw-blueprint/policies/presets/nous-browser.yaml
- src/lib/hermes-provider-auth.test.ts
- agents/hermes/config/managed-tool-gateway.ts
- src/lib/hermes-tool-gateway-broker.ts
- src/lib/oauth-device-code.ts
- agents/hermes/policy-additions.yaml
- src/lib/actions/sandbox/rebuild.ts
- agents/hermes/host/tool-gateway-broker.ts
5 tasks
cv
pushed a commit
that referenced
this pull request
May 19, 2026
## 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 - Resolves the Hermes resume correctness blocker where `setupInference()` could be called with `sandboxName === null` when the provider/model route was already selected and ready. - Preserves the #2753 behavior: the resumed sandbox name is resolved before Hermes inference reconciliation, but is not persisted to the onboard session before sandbox creation. - Updates the Hermes provider reuse test for the intentional provider-store preflight and adds a regression for resume with no recorded sandbox name. ## Original PR Supersedes/continues #3556. Original author: @shannonsands Original PR head before this maintainer patch: `4db9a302c3e385978a12eccc3d9f748c4a91939e` Maintainer patch commit: `3f767eeff503a1d36827022033ad3aa075b0b7e3` ## Verification - [x] `npm run build:cli` - [x] `npx vitest run test/onboard.test.ts -t "Hermes Provider|sandbox name before"` - [x] `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.ts` - [x] `npm run checks` - [x] `git diff --check` ## Merge Gate Before approval/merge, still require one live Hermes OAuth onboarding smoke with at least `nous-web`, broker health on `127.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> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Interactive onboarding and selection for five managed Nous tools (audio, browser, code, image, web). * Local host-side managed-tool gateway broker and runtime “broker-mode” plugin; new audio transcription handler. * **Improvements** * Broker-aware onboarding, OAuth/device flows, token registration/persistence, and sandbox lifecycle preservation of managed-tool selections. * Generated runtime config/env support for enabling broker and presets; new policy presets enforcing network and binary restrictions. * **Documentation** * Docs updated with broker behavior and environment options. * **Tests** * Added unit and integration tests for broker, plugin, and config generation. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3742?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 --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
wscurran
added
area: sandbox
Collaborator
Manual PR Review Advisor resultThis PR Review Advisor analysis was run manually via Run: https://github.com/NVIDIA/NemoClaw/actions/runs/28205896026 PR Review AdvisorThe managed-tool feature is well covered at unit level, but the new host credential broker currently exposes a sandbox-visible bearer token through an all-interface listener and image/env surfaces. Required before merge
Resolve or justify before merge
In-scope improvements
Test follow-ups to resolve or justify
What looks good
|
This was referenced Jul 9, 2026
Merged
Collaborator
|
Thanks for this contribution, @shannonsands. The managed tool gateway work from this branch was replayed with attribution, completed with the missing Hermes resume fix and regression coverage, and merged in #3742. Current main has since received additional secret-boundary hardening in #4771 and substantial onboarding, policy, messaging, and lifecycle refactors. Comparing this branch with current main found no desired behavior unique to #3556; bringing it forward would instead produce 31 conflicts and reintroduce older token-delivery and broker behavior that later changes intentionally replaced. I have removed the stale v0.0.81 target and am closing this PR as superseded by #3742. Any newly discovered gap in the current implementation should be handled as a focused change from current main. Thank you again for the original implementation and UAT work. |
](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3742?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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
Adds Hermes managed Nous Tool Gateway support for OAuth/subscription onboarding while keeping raw Nous OAuth tokens out of the sandbox. NemoClaw now starts a Hermes-owned host broker for selected managed tool planes, generates broker-mode Hermes config/env, and applies broker-scoped policy presets.
Related Issue
Related to NS-322.
Changes
agents/hermes/host/that refreshes Nous OAuth viax-nous-refresh-token, injects upstream auth, rotates inference agent keys, and uses a sandbox-visible opaque broker token..env/config for selected managed tools and preserve the selected presets through onboarding, rebuild, connect, and status recovery paths.nous-*policy presets that allow onlyhost.openshell.internal:11436service paths plus Browser Use CDP exceptions.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Additional checks run:
npm run build:clinpx vitest run test/hermes-tool-gateway-broker.test.ts test/generate-hermes-config.test.ts test/hermes-plugin-handlers.test.ts test/policies.test.ts src/lib/onboard/initial-policy.test.ts src/lib/hermes-provider-auth.test.ts --maxWorkers=1 --testTimeout=30000python3 -m py_compile agents/hermes/plugin/__init__.pygit diff --check --cachedLive UAT:
nous-web:web_searchandweb_extractpassed through Firecrawl.nous-browser: Browser Use cloud navigation/snapshot/click passed.nous-audio: TTS and STT passed through OpenAI audio gateway.nous-image: image generation passed through FAL queue.nous-code: skipped for this UAT because Modal was not selected andterminal.backend: localwas confirmed.NOUS_REFRESH_TOKENorNOUS_ACCESS_TOKEN; gateway env/config used broker mode.Note: I attempted the local pre-push/pre-commit hook path. The current macOS worktree does not have a complete root
node_modulesinstall, so the hook failed on local tooling resolution (biome,tsx,typescript,vitest/config) before it could provide useful PR-specific signal. The focused Hermes checks above and live UAT passed.Signed-off-by: Shannon Sands shannon@nousresearch.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests