feat(hermes): request network access through policy local#3748
Open
cheese-head wants to merge 6 commits into
Open
feat(hermes): request network access through policy local#3748cheese-head wants to merge 6 commits into
cheese-head wants to merge 6 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR adds OpenShell "network-only" resource access support to NemoClaw and Hermes plugins. It introduces an access client for policy.local HTTP communication, provider profile integration into the policy system, onboarding synchronization with OpenShell, tool registration in both plugins, nine new provider profiles, and comprehensive Docker/build/test infrastructure. ChangesOpenShell Network Access Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 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)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
Contributor
|
✨ Thanks for submitting this detailed PR to add a Hermes plugin for OpenShell policy.local network access requests. This change aims to improve the integration of Hermes with OpenShell by exposing a new tool for requesting network access and handling policy.local proposals. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
docs/reference/nemoclaw-openshell-integration.md (4)
1-3: ⚡ Quick winAdd a brief intro paragraph immediately after the H1.
The page currently starts with a diagram; add a one- or two-sentence introduction first.
As per coding guidelines, a new docs page should "start with a one- or two-sentence introduction."
🤖 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/nemoclaw-openshell-integration.md` around lines 1 - 3, Add a one- or two-sentence introduction immediately after the H1 "NemoClaw OpenShell Integration" and before the opening ```mermaid block; the intro should briefly state the purpose of the page (what the integration covers and who it’s for) so the document starts with the required summary sentence(s) prior to the diagram.
76-76: ⚡ Quick winSplit this into one sentence per source line.
Line 76 contains multiple sentences on one line, which breaks docs diff/readability rules.
As per coding guidelines, "One sentence per line in source (makes diffs readable)."
🤖 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/nemoclaw-openshell-integration.md` at line 76, The line containing "Each agent adapter exposes the same response shape through the harness-native mechanism. OpenClaw uses its plugin API. Hermes uses its Python plugin API. Additional harnesses can implement the same proposal flow without changing the OpenShell policy API." must be split so each sentence is on its own source line; edit that paragraph to place each of the four sentences ("Each agent adapter exposes...", "OpenClaw uses its plugin API.", "Hermes uses its Python plugin API.", "Additional harnesses can implement...") on separate lines to satisfy the one-sentence-per-line docs rule.
78-81: ⚡ Quick winAdd a terminal “Next Steps” section with related links.
The page ends without the required closing navigation section.
As per coding guidelines, new docs pages require 'A "Next Steps" section at the bottom links to related pages.'
🤖 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/nemoclaw-openshell-integration.md` around lines 78 - 81, Add a terminal "Next Steps" section to the docs page that provides links to related pages and closes the navigation block; specifically, after the "## Provider Profiles" content add a new "## Next Steps" heading followed by a short bullet or link list to related docs (e.g., onboarding, policy presets, gateway integration) and ensure any required closing navigation markup is present so the page meets the docs guideline for a terminal navigation section.
80-80: ⚡ Quick winRewrite passive constructions in active voice and split sentences by line.
This line uses passive phrasing (e.g., “are left untouched”, “are skipped”) and packs multiple sentences into one line.
As per coding guidelines, "Active voice required. Flag passive constructions." and "One sentence per line in source."
🤖 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/nemoclaw-openshell-integration.md` at line 80, Replace the single passive sentence starting "NemoClaw imports OpenShell provider profiles..." with multiple active-voice sentences: keep the first sentence as "NemoClaw imports OpenShell provider profiles for its policy presets during onboarding," add a second active sentence like "NemoClaw leaves existing OpenShell profiles unchanged," a third like "NemoClaw skips profiles it already imported so onboarding is idempotent," and a final sentence for the fallback: "If the OpenShell gateway does not support provider-profile import, NemoClaw falls back to local presets." Ensure each sentence is on its own line and update the exact sentence block in docs/reference/nemoclaw-openshell-integration.md.src/lib/onboard.ts (1)
9751-9751: Run the onboarding E2E matrix for this flow change.Since this hook now executes in the core onboarding path, please run the recommended jobs for
src/lib/onboard.tschanges (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).As per coding guidelines:
src/lib/onboard.tschanges should be validated with the listed onboarding E2E workflows.🤖 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` at line 9751, This change adds ensureProviderProfilesAvailable() into the core onboarding path in src/lib/onboard.ts; run the full onboarding E2E matrix to validate it by executing the required jobs: 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 ensure no regressions when ensureProviderProfilesAvailable() runs during onboarding.nemoclaw/src/onboard/config.test.ts (1)
146-156: ⚡ Quick winAdd a whitespace-only config test case.
Line 146 covers empty content, but
loadOnboardConfig()also trims input first. Please add a case like" \n\t "to assert that whitespace-only files also returnnull.Proposed test addition
it("returns null when the config file is empty", () => { const configPath = `${homedir()}/.nemoclaw/config.json`; store.set(configPath, ""); expect(loadOnboardConfig()).toBeNull(); }); + it("returns null when the config file contains only whitespace", () => { + const configPath = `${homedir()}/.nemoclaw/config.json`; + store.set(configPath, " \n\t "); + expect(loadOnboardConfig()).toBeNull(); + }); + it("returns null when the config file is malformed", () => { const configPath = `${homedir()}/.nemoclaw/config.json`; store.set(configPath, "{"); expect(loadOnboardConfig()).toBeNull(); });🤖 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 `@nemoclaw/src/onboard/config.test.ts` around lines 146 - 156, Add a test in the existing config.test.ts that verifies loadOnboardConfig() returns null when the config file contains only whitespace; specifically, using the same configPath pattern (`${homedir()}/.nemoclaw/config.json`) call store.set(configPath, " \n\t ") (or similar whitespace string) and assert expect(loadOnboardConfig()).toBeNull(); place this new it(...) alongside the existing empty and malformed file tests so it covers the whitespace-only case.Dockerfile (1)
32-66: Run the recommended sandbox E2E matrix for this image-layer change.This change affects baked image layers and runtime tooling; please run the Dockerfile-targeted E2E jobs (
cloud-e2e,sandbox-survival-e2e,hermes-e2e,rebuild-openclaw-e2e) before merge.As per coding guidelines,
Dockerfilechanges are “only testable with a real container build” and include that E2E recommendation set.🤖 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 `@Dockerfile` around lines 32 - 66, The Dockerfile change touches baked image layers and runtime tooling (see the RUN blocks that modify apt packages and the call to /usr/local/lib/nemoclaw/install-provider-tools.sh), so before merging run the full Dockerfile-targeted E2E matrix: cloud-e2e, sandbox-survival-e2e, hermes-e2e and rebuild-openclaw-e2e; build the image locally or in CI using this Dockerfile, execute those test jobs against the resulting image, and only merge once all tests pass to validate the apt cleanup/conditional installs and the provider tools installer behavior.agents/hermes/plugin/__init__.py (1)
671-708: Run Hermes E2E coverage for this tool-path change.As per coding guidelines:
agents/hermes/**changes should runhermes-e2e,hermes-inference-switch-e2e,hermes-discord-e2e,hermes-slack-e2e,rebuild-hermes-e2e, andrebuild-hermes-stale-base-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 `@agents/hermes/plugin/__init__.py` around lines 671 - 708, This change adds a new tool registration (ctx.register_tool with name "openshell_network_access" and handler _handle_network_access) under agents/hermes/plugin/__init__.py and therefore requires running the Hermes E2E pipelines; please run or trigger hermes-e2e, hermes-inference-switch-e2e, hermes-discord-e2e, hermes-slack-e2e, rebuild-hermes-e2e, and rebuild-hermes-stale-base-e2e to validate the tool-path change and include test results or fix any failing tests before merging.
🤖 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 695-700: The schema entry "wait_timeout_ms" currently uses
DEFAULT_ACCESS_WAIT_MS (global default ~90000) which can cause action=check to
block; change the schema for "wait_timeout_ms" in
agents/hermes/plugin/__init__.py (the dict containing "wait_timeout_ms",
"minimum", "maximum", "default") to either remove the "default" key or set it to
0 so the action-multiplexed tool defaults to immediate (no wait) unless a caller
explicitly supplies a timeout; keep MAX_ACCESS_WAIT_MS and the min validation
intact.
In `@docs/reference/nemoclaw-openshell-integration.md`:
- Around line 1-2: Add the required YAML frontmatter block at the top of the new
Markdown file including at minimum title, description, and sidebar or permalink
fields, then immediately after that frontmatter add the SPDX Apache-2.0 license
header line (e.g. "SPDX-License-Identifier: Apache-2.0"); ensure the SPDX header
is present in the file and placed after the closing frontmatter delimiter (---)
per project guidelines so the page complies with docs frontmatter and license
requirements.
In `@nemoclaw/src/access-client.ts`:
- Around line 578-584: The target string is hardcoded to
"http://policy.local:80" and thus ignores a configured policyLocalUrl port;
update the target construction in the code that defines target so it derives
host and port from the configured policyLocalUrl (parse it with new
URL(policyLocalUrl) or use its origin) and fall back to proxy.port or default 80
only when policyLocalUrl has no explicit port; adjust the logic around target,
proxy.port and any use of base.pathname/requestPath so the final target uses the
correct hostname:port from policyLocalUrl rather than always :80.
In `@src/lib/onboard.ts`:
- Around line 1795-1805: The helper ensureProviderProfilesAvailable is adding
lines to onboard.ts and should be moved to a new module: create
src/lib/onboard/provider-profiles.ts exporting the function
ensureProviderProfilesAvailable which calls
providerProfileOnboard.ensureNemoClawProviderProfiles(runOpenshell, { log: note
}), handles the three result branches (unsupported, already-present with
skipped) using note, and calls policies.clearProviderProfileCache(); then
replace the body in src/lib/onboard.ts with a thin invocation that imports and
calls the exported ensureProviderProfilesAvailable to keep onboard.ts minimal
for the entrypoint budget gate.
---
Nitpick comments:
In `@agents/hermes/plugin/__init__.py`:
- Around line 671-708: This change adds a new tool registration
(ctx.register_tool with name "openshell_network_access" and handler
_handle_network_access) under agents/hermes/plugin/__init__.py and therefore
requires running the Hermes E2E pipelines; please run or trigger hermes-e2e,
hermes-inference-switch-e2e, hermes-discord-e2e, hermes-slack-e2e,
rebuild-hermes-e2e, and rebuild-hermes-stale-base-e2e to validate the tool-path
change and include test results or fix any failing tests before merging.
In `@Dockerfile`:
- Around line 32-66: The Dockerfile change touches baked image layers and
runtime tooling (see the RUN blocks that modify apt packages and the call to
/usr/local/lib/nemoclaw/install-provider-tools.sh), so before merging run the
full Dockerfile-targeted E2E matrix: cloud-e2e, sandbox-survival-e2e, hermes-e2e
and rebuild-openclaw-e2e; build the image locally or in CI using this
Dockerfile, execute those test jobs against the resulting image, and only merge
once all tests pass to validate the apt cleanup/conditional installs and the
provider tools installer behavior.
In `@docs/reference/nemoclaw-openshell-integration.md`:
- Around line 1-3: Add a one- or two-sentence introduction immediately after the
H1 "NemoClaw OpenShell Integration" and before the opening ```mermaid block; the
intro should briefly state the purpose of the page (what the integration covers
and who it’s for) so the document starts with the required summary sentence(s)
prior to the diagram.
- Line 76: The line containing "Each agent adapter exposes the same response
shape through the harness-native mechanism. OpenClaw uses its plugin API. Hermes
uses its Python plugin API. Additional harnesses can implement the same proposal
flow without changing the OpenShell policy API." must be split so each sentence
is on its own source line; edit that paragraph to place each of the four
sentences ("Each agent adapter exposes...", "OpenClaw uses its plugin API.",
"Hermes uses its Python plugin API.", "Additional harnesses can implement...")
on separate lines to satisfy the one-sentence-per-line docs rule.
- Around line 78-81: Add a terminal "Next Steps" section to the docs page that
provides links to related pages and closes the navigation block; specifically,
after the "## Provider Profiles" content add a new "## Next Steps" heading
followed by a short bullet or link list to related docs (e.g., onboarding,
policy presets, gateway integration) and ensure any required closing navigation
markup is present so the page meets the docs guideline for a terminal navigation
section.
- Line 80: Replace the single passive sentence starting "NemoClaw imports
OpenShell provider profiles..." with multiple active-voice sentences: keep the
first sentence as "NemoClaw imports OpenShell provider profiles for its policy
presets during onboarding," add a second active sentence like "NemoClaw leaves
existing OpenShell profiles unchanged," a third like "NemoClaw skips profiles it
already imported so onboarding is idempotent," and a final sentence for the
fallback: "If the OpenShell gateway does not support provider-profile import,
NemoClaw falls back to local presets." Ensure each sentence is on its own line
and update the exact sentence block in
docs/reference/nemoclaw-openshell-integration.md.
In `@nemoclaw/src/onboard/config.test.ts`:
- Around line 146-156: Add a test in the existing config.test.ts that verifies
loadOnboardConfig() returns null when the config file contains only whitespace;
specifically, using the same configPath pattern
(`${homedir()}/.nemoclaw/config.json`) call store.set(configPath, " \n\t ") (or
similar whitespace string) and assert expect(loadOnboardConfig()).toBeNull();
place this new it(...) alongside the existing empty and malformed file tests so
it covers the whitespace-only case.
In `@src/lib/onboard.ts`:
- Line 9751: This change adds ensureProviderProfilesAvailable() into the core
onboarding path in src/lib/onboard.ts; run the full onboarding E2E matrix to
validate it by executing the required jobs: 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 ensure no regressions when
ensureProviderProfilesAvailable() runs during onboarding.
🪄 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: a22b086d-b9e8-4c2b-8077-20979154aa94
📒 Files selected for processing (38)
DockerfileDockerfile.baseagents/hermes/plugin/__init__.pyagents/hermes/plugin/plugin.yamldocs/reference/nemoclaw-openshell-integration.mdnemoclaw-blueprint/policies/presets/github.yamlnemoclaw-blueprint/provider-profiles/brave.yamlnemoclaw-blueprint/provider-profiles/brew.yamlnemoclaw-blueprint/provider-profiles/discord.yamlnemoclaw-blueprint/provider-profiles/huggingface.yamlnemoclaw-blueprint/provider-profiles/jira.yamlnemoclaw-blueprint/provider-profiles/local-inference.yamlnemoclaw-blueprint/provider-profiles/npm.yamlnemoclaw-blueprint/provider-profiles/pypi.yamlnemoclaw-blueprint/provider-profiles/slack.yamlnemoclaw-blueprint/provider-profiles/telegram.yamlnemoclaw/src/access-client.test.tsnemoclaw/src/access-client.tsnemoclaw/src/index.tsnemoclaw/src/onboard/config.test.tsnemoclaw/src/onboard/config.tsnemoclaw/src/register.test.tsscripts/generate-openclaw-config.pyscripts/install-provider-tools.shsrc/lib/onboard.tssrc/lib/onboard/provider-profiles.tssrc/lib/policy/index.tssrc/lib/sandbox/build-context.tstest/e2e/hermes-policy-local-runner.pytest/e2e/nemoclaw-policy-local-runner.mjstest/e2e/test-hermes-policy-local-plugin.shtest/e2e/test-nemoclaw-policy-local-plugin.shtest/generate-openclaw-config.test.tstest/hermes-plugin-handlers.test.tstest/policies.test.tstest/provider-profile-onboard.test.tstest/sandbox-build-context.test.tstest/validate-blueprint.test.ts
Comment on lines
+695
to
+700
| "wait_timeout_ms": { | ||
| "type": "number", | ||
| "minimum": 0, | ||
| "maximum": MAX_ACCESS_WAIT_MS, | ||
| "default": DEFAULT_ACCESS_WAIT_MS, | ||
| }, |
Contributor
There was a problem hiding this comment.
Set wait_timeout_ms default to 0 (or remove schema default) for action-multiplexed tool.
Line 699 sets a global default of 90000, which can make action=check unexpectedly block if the caller applies schema defaults. check should default to immediate status unless explicitly asked to wait.
Suggested fix
"wait_timeout_ms": {
"type": "number",
"minimum": 0,
"maximum": MAX_ACCESS_WAIT_MS,
- "default": DEFAULT_ACCESS_WAIT_MS,
+ "default": 0,
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "wait_timeout_ms": { | |
| "type": "number", | |
| "minimum": 0, | |
| "maximum": MAX_ACCESS_WAIT_MS, | |
| "default": DEFAULT_ACCESS_WAIT_MS, | |
| }, | |
| "wait_timeout_ms": { | |
| "type": "number", | |
| "minimum": 0, | |
| "maximum": MAX_ACCESS_WAIT_MS, | |
| "default": 0, | |
| }, |
🤖 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 695 - 700, The schema entry
"wait_timeout_ms" currently uses DEFAULT_ACCESS_WAIT_MS (global default ~90000)
which can cause action=check to block; change the schema for "wait_timeout_ms"
in agents/hermes/plugin/__init__.py (the dict containing "wait_timeout_ms",
"minimum", "maximum", "default") to either remove the "default" key or set it to
0 so the action-multiplexed tool defaults to immediate (no wait) unless a caller
explicitly supplies a timeout; keep MAX_ACCESS_WAIT_MS and the min validation
intact.
Comment on lines
+1
to
+2
| # NemoClaw OpenShell Integration | ||
|
|
Contributor
There was a problem hiding this comment.
Add required frontmatter and SPDX header for a new docs page.
This new docs/** page is missing the required frontmatter block and SPDX license header placement.
As per coding guidelines, new docs pages must include frontmatter fields and an "SPDX license header is present after frontmatter," and every .md file must include an SPDX Apache-2.0 header.
🤖 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/nemoclaw-openshell-integration.md` around lines 1 - 2, Add the
required YAML frontmatter block at the top of the new Markdown file including at
minimum title, description, and sidebar or permalink fields, then immediately
after that frontmatter add the SPDX Apache-2.0 license header line (e.g.
"SPDX-License-Identifier: Apache-2.0"); ensure the SPDX header is present in the
file and placed after the closing frontmatter delimiter (---) per project
guidelines so the page complies with docs frontmatter and license requirements.
Comment on lines
+578
to
+584
| const target = `http://policy.local:80${base.pathname.replace(/\/$/, "")}${requestPath}`; | ||
| const timeoutMs = options.timeoutMs ?? 310_000; | ||
| const proxyPort = proxy.port ? Number(proxy.port) : 80; | ||
| const headerLines = Object.entries(headers).map(([key, value]) => `${key}: ${value}`); | ||
| const requestBytes = [ | ||
| `${method} ${target} HTTP/1.1`, | ||
| ...headerLines, |
Contributor
There was a problem hiding this comment.
Proxy target ignores configured policyLocalUrl port.
When proxying, the request target is hardcoded to policy.local:80, so a configured non-default policyLocalUrl port is silently dropped.
Suggested fix
- const target = `http://policy.local:80${base.pathname.replace(/\/$/, "")}${requestPath}`;
+ const target = `${base.protocol}//${base.host}${base.pathname.replace(/\/$/, "")}${requestPath}`;🤖 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 `@nemoclaw/src/access-client.ts` around lines 578 - 584, The target string is
hardcoded to "http://policy.local:80" and thus ignores a configured
policyLocalUrl port; update the target construction in the code that defines
target so it derives host and port from the configured policyLocalUrl (parse it
with new URL(policyLocalUrl) or use its origin) and fall back to proxy.port or
default 80 only when policyLocalUrl has no explicit port; adjust the logic
around target, proxy.port and any use of base.pathname/requestPath so the final
target uses the correct hostname:port from policyLocalUrl rather than always
:80.
Comment on lines
+1795
to
+1805
| function ensureProviderProfilesAvailable(): void { | ||
| const result = providerProfileOnboard.ensureNemoClawProviderProfiles(runOpenshell, { | ||
| log: note, | ||
| }); | ||
| if (result.status === "unsupported") { | ||
| note(` ${result.message}`); | ||
| } else if (result.status === "already-present" && result.skipped.length > 0) { | ||
| note(` NemoClaw provider profiles already registered: ${result.skipped.join(", ")}`); | ||
| } | ||
| policies.clearProviderProfileCache(); | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Move this wrapper out of src/lib/onboard.ts to pass the entrypoint budget gate.
The onboard-entrypoint-budget CI check is currently failing (net +16 lines in this file), and this new helper is the main contributor. Extract the wrapper into src/lib/onboard/provider-profiles.ts and keep src/lib/onboard.ts to a thin invocation point.
Proposed direction
-const providerProfileOnboard: typeof import("./onboard/provider-profiles") =
- require("./onboard/provider-profiles");
+const {
+ ensureProviderProfilesAvailable,
+}: typeof import("./onboard/provider-profiles") = require("./onboard/provider-profiles");
- function ensureProviderProfilesAvailable(): void {
- const result = providerProfileOnboard.ensureNemoClawProviderProfiles(runOpenshell, {
- log: note,
- });
- if (result.status === "unsupported") {
- note(` ${result.message}`);
- } else if (result.status === "already-present" && result.skipped.length > 0) {
- note(` NemoClaw provider profiles already registered: ${result.skipped.join(", ")}`);
- }
- policies.clearProviderProfileCache();
- }
+// moved to ./onboard/provider-profiles
- ensureProviderProfilesAvailable();
+ ensureProviderProfilesAvailable({
+ runOpenshell,
+ log: note,
+ clearProviderProfileCache: policies.clearProviderProfileCache,
+ });🤖 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 1795 - 1805, The helper
ensureProviderProfilesAvailable is adding lines to onboard.ts and should be
moved to a new module: create src/lib/onboard/provider-profiles.ts exporting the
function ensureProviderProfilesAvailable which calls
providerProfileOnboard.ensureNemoClawProviderProfiles(runOpenshell, { log: note
}), handles the three result branches (unsupported, already-present with
skipped) using note, and calls policies.clearProviderProfileCache(); then
replace the body in src/lib/onboard.ts with a thin invocation that imports and
calls the exported ensureProviderProfilesAvailable to keep onboard.ts minimal
for the entrypoint budget gate.
This was referenced May 21, 2026
cv
requested changes
May 27, 2026
cv
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed with the PR review advisor rubric (static patch review only; not evaluating CI/mergeability). Requesting changes:
agents/hermes/plugin/__init__.py:367usesquote(request_id)for proposal polling. Python's defaultsafe='/'leaves slashes unescaped, so a user/tool-provided request id containing/changes the policy.local route. Please usequote(request_id, safe="")and add a regression test.agents/hermes/plugin/__init__.py:301hardcodes proxy absolute-form requests tohttp://policy.local:80..., ignoringOPENSHELL_POLICY_LOCAL_URLhost/port/path in proxy mode. Please derive the proxy target from_policy_local_base().agents/hermes/plugin/__init__.py:120only normalizes GitHub hosts to canonical preset ids. Non-GitHub URLs such as PyPI/npm/Telegram become hostnames thatlist_presetsdoes not return. Please either require exact preset ids or map all supported provider hosts to canonical ids._all_presets()atagents/hermes/plugin/__init__.py:230-235preserves fallback rules on provider-profile name collisions and only copiesprovider_profile. If provider profiles are available, their rule should replace the fallback rule (or this needs a clear source-of-truth rationale).- This PR also inherits the TypeScript provider-profile/client blockers from the lower stack: non-unsupported OpenShell failures should not be downgraded to fallback, and provider endpoint ports need finite integer
1..65535validation.
Please add Hermes tests for slash-containing request ids, custom policy.local proxy URLs, invalid provider-profile ports, and non-GitHub resource URLs.
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
3 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 a Hermes plugin tool for OpenShell policy.local network access requests. Hermes exposes the single
openshell_network_accesstool withlist_presets,request, andcheckactions, matching the no-legacy-alias tool contract.Changes
agents/hermes/plugin/__init__.py.openshell_network_accessin the Hermes plugin manifest.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Patrick Riel priel@nvidia.com
Summary by CodeRabbit
Release Notes
New Features
Enhancements
ghCLI and includes Node/curl binaries.Documentation