fix(inference): honor compatible-endpoint max_model_len for Hermes context (#6177) - #6293
Merged
Conversation
…ntext (#6177) Compatible-endpoint / custom onboarding never probed the endpoint's runtime context window, and the Hermes image/config carried no context setting, so NemotronH models on a self-hosted vLLM endpoint fell back to a ~4K architecture default and Hermes errored with "Context length exceeded". - Probe the configured OpenAI-compatible endpoint's /v1/models for max_model_len and set NEMOCLAW_CONTEXT_WINDOW when the user has not set one; an explicit override always wins and is never downgraded. - Use strict model matching so a shared multi-model gateway never bakes an unrelated model's window; local vLLM keeps its single-model fallback. - Clear an auto-detected value when re-selecting away from the endpoint so it is never mistaken for a user override by dockerfile-patch. - Declare ARG/ENV NEMOCLAW_CONTEXT_WINDOW in the Hermes Dockerfile and write the resolved window as model.context_length in the generated config so it overrides Hermes' built-in metadata (Hermes ignores context_window). Signed-off-by: Yimo Jiang <yimoj@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 compatible-endpoint context probing and Hermes context-window plumbing so detected windows are written into generated config, propagated through onboarding, and left unset when auto-detection should remain in effect. ChangesContext window detection and propagation
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-6293.docs.buildwithfern.com/nemoclaw |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
PR Review Advisor — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
Review findings by urgency: 0 required fixes, 8 items to resolve/justify, 0 in-scope improvements
|
…ests The codebase-growth-guardrails check forbids changed test files from adding if statements. Rewrite the Hermes Dockerfile context-window test to drive NEMOCLAW_CONTEXT_WINDOW by direct assignment with clearing beforeEach/afterEach hooks (matching the sibling dockerfile-patch suites) instead of a branching env helper, and reword a comment so it reads without "if". Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
![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
🧹 Nitpick comments (1)
test/compatible-endpoint-context-probe.test.ts (1)
34-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd local issue-ref suffix to individual test titles.
Both
it(...)titles are missing the(#6177)suffix that the guideline requires, even though thedescribeblock has it.As per path instructions,
test/**/*.test.{js,ts}should "use behavior-oriented titles with local issue refs in a final(#1234)suffix."✏️ Proposed fix
- it("reads max_model_len from a live /v1/models endpoint into NEMOCLAW_CONTEXT_WINDOW", async () => { + it("reads max_model_len from a live /v1/models endpoint into NEMOCLAW_CONTEXT_WINDOW (`#6177`)", async () => { ... - it("keeps the default context window when the endpoint omits max_model_len", async () => { + it("keeps the default context window when the endpoint omits max_model_len (`#6177`)", async () => {🤖 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 `@test/compatible-endpoint-context-probe.test.ts` around lines 34 - 57, Add the required local issue reference suffix to each individual test title in the relevant `it(...)` cases within `compatible-endpoint-context-probe.test.ts`; update both test names so they end with the `(`#6177`)` suffix, matching the repository convention for behavior-oriented test titles, while leaving the test logic and surrounding `describe` block unchanged.Source: Path instructions
🤖 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 `@docs/inference/switch-inference-providers.mdx`:
- Line 191: The paragraph in the OpenAI-compatible endpoint section combines
multiple sentences on one line, which breaks the one-sentence-per-line MDX style
used elsewhere. Update the text in this block so each sentence is on its own
line, keeping the content unchanged while preserving the surrounding formatting
and the references to NEMOCLAW_CONTEXT_WINDOW, /v1/models, and
model.context_length.
In `@src/lib/onboard/dockerfile-patch-hermes-context.test.ts`:
- Around line 46-56: The withContextWindowEnv helper in
dockerfile-patch-hermes-context.test.ts is duplicating environment restore logic
that already exists in the shared env-test-helpers utilities. Replace the manual
prior/save/delete branches with restoreEnv or restoreEnvBulk from
test/helpers/env-test-helpers.ts, and update the test setup/teardown around
withContextWindowEnv to use the shared helper so the conditional restore logic
is removed.
---
Nitpick comments:
In `@test/compatible-endpoint-context-probe.test.ts`:
- Around line 34-57: Add the required local issue reference suffix to each
individual test title in the relevant `it(...)` cases within
`compatible-endpoint-context-probe.test.ts`; update both test names so they end
with the `(`#6177`)` suffix, matching the repository convention for
behavior-oriented test titles, while leaving the test logic and surrounding
`describe` block unchanged.
🪄 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: a29414b1-099e-49c9-a591-6cb2d92b1336
📒 Files selected for processing (16)
agents/hermes/Dockerfileagents/hermes/config/build-env.tsagents/hermes/config/hermes-config.tsdocs/inference/switch-inference-providers.mdxsrc/lib/inference/compatible-endpoint-context.test.tssrc/lib/inference/compatible-endpoint-context.tssrc/lib/inference/vllm-runtime-context.test.tssrc/lib/inference/vllm-runtime-context.tssrc/lib/onboard/dockerfile-patch-hermes-context.test.tssrc/lib/onboard/machine/handlers/provider-inference.tssrc/lib/onboard/setup-nim-selection.tstest/compatible-endpoint-context-probe.test.tstest/e2e/fixtures/fake-openai-compatible.tstest/e2e/lib/fake-openai-compatible-api.mtstest/generate-hermes-config.test.tstest/hermes-gateway-wrapper.test.ts
Apply PR review advisor feedback for #6177: - Add a real-server test asserting the compatible-endpoint probe transmits the Authorization header through curl's --config temp file (the fake endpoint now records header presence without logging the token). - Qualify the NEMOCLAW_CONTEXT_WINDOW docs default so OpenClaw's baked 131072 and Hermes' unset/auto-detect behavior are both accurate. - Document the vLLM auto-detect ceiling (4 MiB tokens, matches Ollama) and the host-side onboarding security model for the /v1/models probe. - Note the Ollama-mirrored auto-state duplication for future extraction. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/compatible-endpoint-context-probe.test.ts (1)
34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing local issue-ref suffix in test titles.
Per the repo's test-naming convention, only one of the three tests carries the
(#6177)suffix. The other two ("reads max_model_len..." and "keeps the default context window...") lack the required issue reference.As per coding guidelines: "Root-level integration tests under
test/should import source code, use ESM imports, and use behavior-oriented titles with local issue refs in a final(#1234)suffix."✏️ Proposed fix
- it("reads max_model_len from a live /v1/models endpoint into NEMOCLAW_CONTEXT_WINDOW", async () => { + it("reads max_model_len from a live /v1/models endpoint into NEMOCLAW_CONTEXT_WINDOW (`#6177`)", async () => {- it("keeps the default context window when the endpoint omits max_model_len", async () => { + it("keeps the default context window when the endpoint omits max_model_len (`#6177`)", async () => {Also applies to: 70-70
🤖 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 `@test/compatible-endpoint-context-probe.test.ts` at line 34, The two integration test titles in compatible-endpoint-context-probe.test.ts are missing the required local issue-reference suffix. Update the behavior-oriented titles in the relevant test cases (the ones for reading max_model_len into NEMOCLAW_CONTEXT_WINDOW and keeping the default context window) so they match the repo convention by ending with the same style of final issue ref suffix used by the existing (`#6177`) test.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.
Nitpick comments:
In `@test/compatible-endpoint-context-probe.test.ts`:
- Line 34: The two integration test titles in
compatible-endpoint-context-probe.test.ts are missing the required local
issue-reference suffix. Update the behavior-oriented titles in the relevant test
cases (the ones for reading max_model_len into NEMOCLAW_CONTEXT_WINDOW and
keeping the default context window) so they match the repo convention by ending
with the same style of final issue ref suffix used by the existing (`#6177`) test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9904371b-ae32-4d43-b303-f4fb362b721a
📒 Files selected for processing (6)
docs/inference/switch-inference-providers.mdxsrc/lib/inference/compatible-endpoint-context.tssrc/lib/inference/vllm-runtime-context.tstest/compatible-endpoint-context-probe.test.tstest/e2e/fixtures/fake-openai-compatible.tstest/e2e/lib/fake-openai-compatible-api.mts
✅ Files skipped from review due to trivial changes (1)
- docs/inference/switch-inference-providers.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/lib/inference/vllm-runtime-context.ts
- test/e2e/lib/fake-openai-compatible-api.mts
- src/lib/inference/compatible-endpoint-context.ts
Second round of PR review advisor feedback for #6177: - Skip the /v1/models context probe when the endpoint URL is a sandbox-internal (host.openshell.internal) or docker-internal host — a host-side GET cannot reach it — mirroring probeOpenAiLikeEndpoint, and leave Hermes auto-detect. - Add a source-boundary note documenting the tolerated invalid states and the removal condition for the probe. - Add a handler-level regression test: a stale auto-detected compatible-endpoint context window is cleared before a re-selected provider is patched. - Add a unit test for the sandbox-internal skip. - Split the compatible-endpoint docs paragraph into one sentence per line. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…tance (#6177) Address remaining PR review advisor notes for #6177: - Add an opt-in NEMOCLAW_FAKE_OPENAI_REQUIRE_AUTH_MODELS flag so the fake endpoint enforces auth on GET /v1/models, and cover it end-to-end: the probe sets the window only with the correct credential and skips it on 401. The readiness probe now accepts 401 (server up) so existing fixtures are unaffected. - Document why private/loopback endpoints are intentionally not blocked (the operator's own self-hosted vLLM on localhost is the primary target; a private IP blocklist would break local inference, and probeOpenAiLikeEndpoint does not filter them either); only unreachable sandbox-internal hosts are skipped. - Add the (#6177) local issue-ref suffix to a probe test title. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/compatible-endpoint-context-probe.test.ts (1)
70-95: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider asserting the request was actually rejected, not just that no window was set.
The wrong-key branch (Lines 78-85) only checks
NEMOCLAW_CONTEXT_WINDOWstays undefined. That outcome is also produced by any probe failure (network error, timeout), not specifically an auth rejection. Asserting onserver.requests()(e.g. that a/v1/modelsrequest without/with wrong auth was recorded) would more precisely confirm the "enforces auth" claim rather than only inferring it from the downstream unset variable.🤖 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 `@test/compatible-endpoint-context-probe.test.ts` around lines 70 - 95, The test for auth enforcement only checks that NEMOCLAW_CONTEXT_WINDOW remains unset, which can happen for non-auth probe failures too. Update the "enforces auth on /v1/models" case in compatible-endpoint-context-probe.test.ts to also assert the fake server recorded the /v1/models request for the wrong/absent credential path, using the server.requests() data from startFakeOpenAiCompatibleServer. Keep the existing applyCompatibleEndpointContextWindow and fetchCompatibleEndpointModels flow, but make the assertion verify the request was actually rejected by auth rather than inferred from the missing window.
🤖 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.
Nitpick comments:
In `@test/compatible-endpoint-context-probe.test.ts`:
- Around line 70-95: The test for auth enforcement only checks that
NEMOCLAW_CONTEXT_WINDOW remains unset, which can happen for non-auth probe
failures too. Update the "enforces auth on /v1/models" case in
compatible-endpoint-context-probe.test.ts to also assert the fake server
recorded the /v1/models request for the wrong/absent credential path, using the
server.requests() data from startFakeOpenAiCompatibleServer. Keep the existing
applyCompatibleEndpointContextWindow and fetchCompatibleEndpointModels flow, but
make the assertion verify the request was actually rejected by auth rather than
inferred from the missing window.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e78ca19f-6b86-4aa6-a691-439744257e15
📒 Files selected for processing (4)
src/lib/inference/compatible-endpoint-context.tstest/compatible-endpoint-context-probe.test.tstest/e2e/fixtures/fake-openai-compatible.tstest/e2e/lib/fake-openai-compatible-api.mts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/lib/fake-openai-compatible-api.mts
- src/lib/inference/compatible-endpoint-context.ts
- Use vi.stubEnv/vi.unstubAllEnvs in the Hermes Dockerfile context test instead of manual process.env deletion (CodeRabbit / advisor quick win). - Document that no separate auxiliary/compression context_length is needed: Hermes derives its compression threshold from the main model's context_length. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
…gth (#6177) Add a source-level regression that runs the same applyCompatibleEndpointContextWindow the onboarding boundary calls (injected fetcher, max_model_len 65536), then the real Hermes config generator, and asserts model.context_length is 65536 and model.context_window is absent — closing the probe→env→config chain in one test. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Address CodeRabbit review nits: - The auth-enforcement case now asserts the fake endpoint recorded the /v1/models request as auth=missing (rejected) for the wrong-key path and auth=ok for the keyed path, instead of only inferring rejection from an unset NEMOCLAW_CONTEXT_WINDOW (which a network failure could also produce). - Add the (#6177) local issue-ref suffix to the remaining probe test title. Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
…oundary (#6293) Custom / compatible-endpoint onboarding probed the operator-supplied endpoint URL before any private-IP screening. Both the chat-completions validation probe and the /v1/models context probe would issue host-side requests to a private/reserved address (SSRF), relying only on the downstream DNS-pinning config-write boundary as defense. Add source-boundary validation reusing the shared isPrivateHostname validator (src/lib/private-networks.ts), before any curl runs: - probeOpenAiLikeEndpoint (onboard-probes.ts): reject a private/internal endpoint after the allowed sandbox-internal / docker-internal hosts are handled, so every validation path (including reasoning mode) is covered (PRA-2). - applyCompatibleEndpointContextWindow (compatible-endpoint-context.ts): refuse the /v1/models probe for a private endpoint; this path issues its own GET independent of validation, so it screens the URL itself (PRA-1/PRA-3). Update the misleading JSDoc that claimed private IPs are intentionally not blocked (PRA-11). DNS-pinning at the config-write boundary is kept as defense-in-depth. Tests: - Extract the compatible-endpoint context-window handler case into provider-inference-context-window.test.ts plus a shared provider-inference.test-support.ts, keeping the primary handler spec within the growth guardrail (PRA-6). - Unit-test that applyCompatibleEndpointContextWindow rejects 10.0.0.1/127.0.0.1/169.254.169.254/172.16.0.1/192.168.1.1 before fetchModels, via it.each (PRA-8). - Add a real-server integration case proving a 127.0.0.1 endpoint is refused with no new /v1/models request; migrate the existing happy-path probe cases to present a public host to the guard while the injected fetcher still hits the loopback fixture (PRA-9). Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/inference/onboard-probes.ts (1)
638-664: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider extracting the hostname-extraction-plus-guard pattern into a shared helper.
The try/
new URL()/catch-to-hostname-then-isPrivateHostnamesequence here is the same shape the compatible-endpoint context probe path needs to enforce (per the test suite's SSRF assertions intest/compatible-endpoint-context-probe.test.ts). Consolidating into a singleisPrivateEndpointUrl(url)helper inprivate-networkswould prevent the two call sites from silently diverging as the guard evolves.🤖 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/inference/onboard-probes.ts` around lines 638 - 664, The hostname parsing plus private-address guard in the onboarding probe is duplicated and should be centralized. Extract the try/new URL/catch logic and the isPrivateHostname check into a shared helper such as isPrivateEndpointUrl in private-networks, then call that helper from onboard-probes and the compatible-endpoint context probe path. Keep the existing rejection behavior and message flow, but ensure both call sites use the same helper so SSRF validation stays consistent as it evolves.src/lib/onboard/machine/handlers/provider-inference.test-support.ts (1)
51-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnused
endpointUrlparam should be_-prefixed.It's never read in the mock body (the returned
endpointUrlis a hardcoded literal), unlike the sibling_provider/_credentialEnvparams.As per coding guidelines, "Use
_-prefixed names for intentionally unused variables in JavaScript/TypeScript code."🔧 Proposed fix
reupsertRoutedProvider: vi.fn( - (_provider: string, endpointUrl: string | null, _credentialEnv: string | null) => ({ + (_provider: string, _endpointUrl: string | null, _credentialEnv: string | null) => ({ ok: true as const, endpointUrl: "http://host.openshell.internal:4000/v1", }), ),🤖 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/machine/handlers/provider-inference.test-support.ts` around lines 51 - 56, The mock in provider-inference.test-support.ts has an unused endpointUrl parameter in reupsertRoutedProvider, so rename it to an _-prefixed name to match the sibling _provider and _credentialEnv params. Keep the vi.fn signature and returned object the same, and only change the parameter name in reupsertRoutedProvider so the intent to ignore it is explicit.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/inference/onboard-probes.ts`:
- Around line 644-664: The endpoint parsing in onboard-probes should fail closed
instead of falling through when URL parsing fails. Update the probe endpoint
handling around the new URL(String(endpointUrl)) logic so the catch path returns
openAiLikeFailureFromError(error) immediately, rather than setting probeHostname
to an empty string and continuing. Keep the private-host check in place for
valid hostnames, but ensure unparseable inputs in this flow do not proceed to
the curl-based probing logic.
---
Nitpick comments:
In `@src/lib/inference/onboard-probes.ts`:
- Around line 638-664: The hostname parsing plus private-address guard in the
onboarding probe is duplicated and should be centralized. Extract the try/new
URL/catch logic and the isPrivateHostname check into a shared helper such as
isPrivateEndpointUrl in private-networks, then call that helper from
onboard-probes and the compatible-endpoint context probe path. Keep the existing
rejection behavior and message flow, but ensure both call sites use the same
helper so SSRF validation stays consistent as it evolves.
In `@src/lib/onboard/machine/handlers/provider-inference.test-support.ts`:
- Around line 51-56: The mock in provider-inference.test-support.ts has an
unused endpointUrl parameter in reupsertRoutedProvider, so rename it to an
_-prefixed name to match the sibling _provider and _credentialEnv params. Keep
the vi.fn signature and returned object the same, and only change the parameter
name in reupsertRoutedProvider so the intent to ignore it is explicit.
🪄 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: 86a2f198-0199-40a4-96ea-4882cb1260b4
📒 Files selected for processing (7)
src/lib/inference/compatible-endpoint-context.test.tssrc/lib/inference/compatible-endpoint-context.tssrc/lib/inference/onboard-probes.tssrc/lib/onboard/machine/handlers/provider-inference-context-window.test.tssrc/lib/onboard/machine/handlers/provider-inference.test-support.tssrc/lib/onboard/machine/handlers/provider-inference.test.tstest/compatible-endpoint-context-probe.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/inference/compatible-endpoint-context.test.ts
- src/lib/inference/compatible-endpoint-context.ts
…x test titles Follow-up to the #6293 SSRF hardening — two CI breakages: - The isPrivateHostname block in probeOpenAiLikeEndpoint over-blocked host.docker.internal, a trusted sandbox->host bridge already gated by the allowHostDockerInternal check at the top of the function (Windows-host Ollama validation). Exempt the already-permitted hijacked-docker-internal alias. - Three new test titles put the issue ref mid-parens ("(SSRF, #6293)") which fails test-title-style; move to a clean final "(#6293)". cli project (635 tests) + npm run checks pass locally. SKIP=test-cli: full hook may trip on pre-existing macOS bash 3.2 noise if this branch predates #6140; CI runs bash 5.x green. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
) The private-address SSRF guard added to probeOpenAiLikeEndpoint rejected every private/reserved address, but that shared probe is the same one local inference uses to validate a locally-run Ollama/vLLM/NIM server on loopback (127.0.0.0/8, ::1, localhost). Loopback only reaches the probing host itself, so it is not an SSRF pivot to other internal infrastructure; non-loopback private ranges (LAN, link-local cloud-metadata) stay blocked. Adds isLoopbackHostname to private-networks plus unit coverage for the guard's block (LAN/metadata) and allow (loopback) paths. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/private-networks.ts (1)
207-216: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated hostname normalization logic.
The bracket-stripping/trailing-dot/lowercasing block (Lines 208-210) is identical to the one in
isPrivateHostname(Lines 184-186). Consider extracting a sharednormalizeHostnamehelper to avoid drift between the two functions.♻️ Proposed refactor
+function normalizeHostname(hostname: string): string { + const stripped = + hostname.startsWith("[") && hostname.endsWith("]") ? hostname.slice(1, -1) : hostname; + return stripped.replace(/\.$/, "").toLowerCase(); +} + export function isPrivateHostname(hostname: string): boolean { - const stripped = - hostname.startsWith("[") && hostname.endsWith("]") ? hostname.slice(1, -1) : hostname; - const normalised = stripped.replace(/\.$/, "").toLowerCase(); + const normalised = normalizeHostname(hostname); const { normalisedNames } = load(); ... } export function isLoopbackHostname(hostname: string): boolean { - const stripped = - hostname.startsWith("[") && hostname.endsWith("]") ? hostname.slice(1, -1) : hostname; - const normalised = stripped.replace(/\.$/, "").toLowerCase(); + const normalised = normalizeHostname(hostname); if (normalised === "localhost") return true; ... }🤖 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/private-networks.ts` around lines 207 - 216, The hostname normalization logic in isLoopbackHostname duplicates the same bracket-stripping, trailing-dot removal, and lowercasing used in isPrivateHostname, so extract that shared behavior into a normalizeHostname helper and have both functions call it. Keep the loopback checks in isLoopbackHostname unchanged, but route the initial hostname cleanup through the shared helper to prevent the two implementations from drifting apart.
🤖 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.
Nitpick comments:
In `@src/lib/private-networks.ts`:
- Around line 207-216: The hostname normalization logic in isLoopbackHostname
duplicates the same bracket-stripping, trailing-dot removal, and lowercasing
used in isPrivateHostname, so extract that shared behavior into a
normalizeHostname helper and have both functions call it. Keep the loopback
checks in isLoopbackHostname unchanged, but route the initial hostname cleanup
through the shared helper to prevent the two implementations from drifting
apart.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c17581e0-c9f8-4826-9939-d21476233504
📒 Files selected for processing (3)
src/lib/inference/onboard-probes.test.tssrc/lib/inference/onboard-probes.tssrc/lib/private-networks.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/inference/onboard-probes.ts
# Conflicts: # src/lib/onboard/machine/handlers/provider-inference.test.ts # src/lib/onboard/machine/handlers/provider-inference.ts
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
E2E Target Results —
|
| Job | Result |
|---|---|
| hermes-gpu-startup | |
| hermes-shields-config | |
| messaging-providers | |
| onboard-resume |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28906743092
|
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28906860835
|
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28906504389
|
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28907190421
|
Contributor
E2E Target Results — ✅ All default jobs passedRun: 28907190421
|
Collaborator
|
@cv Exact-head rereview requested for All items from the latest CV review are addressed:
Exact-head evidence:
The newer green scorecard supersedes the earlier red scorecard from attempt 1 on the same workflow run and SHA. No further code was pushed during the rerun. |
Collaborator
|
@cv Focused exact-head re-review requested for Since your last review at Current exact-head evidence:
Could you confirm approval or identify any remaining exact-head blocker? |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28957991760
|
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28958088095
|
Contributor
E2E Target Results — ❌ Some jobs failedRun: 28958085816
|
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Contributor
E2E Target Results — ✅ All selected jobs passedRun: 28960081025
|
Contributor
E2E Target Results — ✅ All requested jobs passedRun: 28960515762
|
Collaborator
|
@cv Exact-head re-review requested for The branch now contains current Exact-head evidence:
Could you review this exact head and either approve or identify the remaining blocker(s)? I will not merge while the existing change request or a substantive Advisor concern remains unresolved. |
This was referenced Jul 8, 2026
cv
pushed a commit
that referenced
this pull request
Jul 9, 2026
<!-- 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>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…ntext (NVIDIA#6177) (NVIDIA#6293) ## Summary Compatible-endpoint / custom onboarding never probed the endpoint's runtime context window, and the Hermes image/config carried no context setting. A self-hosted vLLM endpoint serving a NemotronH model (`max_model_len=65536`) therefore fell back to a ~4K architecture default, and Hermes failed with `Error: Context length exceeded (4,181 tokens). Cannot compress further.` This teaches onboarding to probe the endpoint's `max_model_len` (or honor an explicit `NEMOCLAW_CONTEXT_WINDOW`) and propagate the window into the generated Hermes config. ## Related Issue Fixes NVIDIA#6177 ## Changes - New `src/lib/inference/compatible-endpoint-context.ts`: probes the configured OpenAI-compatible endpoint's `/v1/models` for `max_model_len` and sets `NEMOCLAW_CONTEXT_WINDOW` when the user has not set one. Wired into the custom-endpoint validation-success path in `setup-nim-selection.ts` (keeps `src/lib/onboard.ts` net-zero for the growth guardrail). - An explicit `NEMOCLAW_CONTEXT_WINDOW` always wins and is never downgraded. - Strict model matching: a shared multi-model gateway with no exact `/v1/models` id match never bakes an unrelated model's window (local vLLM keeps its single-served-model fallback). - Auto-detected values are cleared before re-selecting a provider (`machine/handlers/provider-inference.ts`), so retrying away to another provider cannot leave endpoint A's probed window in the environment where `dockerfile-patch` would treat it as a user override. - Defensive filtering of non-object `/v1/models` entries so an arbitrary endpoint returning `{"data":[null]}` cannot crash onboarding. - Hermes propagation: declares `ARG/ENV NEMOCLAW_CONTEXT_WINDOW` (empty → auto-detect) in `agents/hermes/Dockerfile`, reads it in `build-env.ts`, and writes the resolved window as `model.context_length` in the generated `config.yaml`. Hermes reads only `context_length` (`context_window` is silently ignored upstream); the model-block key is its highest-priority override, above `/v1/models` discovery and its built-in NemotronH metadata. - Docs: documents the compatible-endpoint context probe and the Hermes `context_length` behavior in `switch-inference-providers.mdx`. ## Type of Change - [x] Code change with doc updates - [ ] Code change (feature, bug fix, or refactor) - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: self-review + `codex review --uncommitted` clean ("No discrete correctness issues"); credential handling reuses the existing curl auth-config (key in a 0600 tmpfile, never argv); endpoint URL is the already-validated/normalized onboarding value. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [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: `npx vitest run` for `compatible-endpoint-context`, `vllm-runtime-context`, `dockerfile-patch-hermes-context`, `provider-inference`, `compatible-endpoint-context-probe`, `generate-hermes-config`, `hermes-gateway-wrapper` → all pass (139+ tests); `npm run typecheck:cli` clean. - [ ] 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 - [x] `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) ### E2E evidence — real worktree CLI Verified end-to-end with the worktree CLI (`node ./bin/nemoclaw.js`) driving a real onboard against a fake OpenAI-compatible endpoint (bound to `127.0.0.1`, serving `/v1/models` with `max_model_len: 65536`). Docker was available on the host, so the full Hermes sandbox image was built and the sandbox created. Command (reporter's flow, fake endpoint substituted for the DGX Spark vLLM server): ``` NEMOCLAW_PROVIDER=custom \ NEMOCLAW_ENDPOINT_URL=http://127.0.0.1:$PORT/v1 \ NEMOCLAW_MODEL=nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-NVFP4 \ COMPATIBLE_API_KEY=dummy \ node ./bin/nemoclaw.js onboard --non-interactive --name ctx6177 --agent hermes --no-sandbox-gpu ``` Onboard transcript (post-fix): ``` [3/8] Configuring inference provider [non-interactive] Provider: custom Chat Completions API available — Hermes will use openai-completions. ✓ Using endpoint max_model_len: 65536 tokens Using Other OpenAI-compatible endpoint with model: nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-NVFP4 ... ✓ Sandbox 'ctx6177-3482376' created ### onboard exit: 0 ``` Generated Hermes `config.yaml` in the built sandbox image (post-fix): ```yaml model: default: nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-NVFP4 provider: custom base_url: "https://inference.local/v1" api_key: sk-OPENSHELL-PROXY-REWRITE context_length: 65536 ``` Pre-fix contrast (same image, `NEMOCLAW_CONTEXT_WINDOW` unset → the behavior before this PR): the `model:` block has **no `context_length`**, so Hermes falls back to its NemotronH metadata default (~4K) — the reported failure: ```yaml model: default: nvidia/NVIDIA-Nemotron-3-Super-120B-A12B-NVFP4 provider: custom base_url: "https://inference.local/v1" api_key: sk-OPENSHELL-PROXY-REWRITE # (no context_length) ``` The only part of the reporter's setup not reproducible here is the DGX Spark GPU serving the real 120B NemotronH weights and a live `hermes chat`; the fake endpoint reproduces the exact `/v1/models` `max_model_len` signal the fix consumes. Supporting unit/integration tests (real curl vs a live server, real Hermes config generator, real Hermes Dockerfile patch) are listed above. --- Signed-off-by: Yimo Jiang <yimoj@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Hermes now supports `NEMOCLAW_CONTEXT_WINDOW`, overriding auto-detection and emitting the resolved value as `config.yaml`’s `context_length`. * OpenAI-compatible endpoint onboarding can auto-derive context length from `/v1/models` (`max_model_len`) when `NEMOCLAW_CONTEXT_WINDOW` is unset. * **Bug Fixes** * Prevents previously auto-detected context windows from carrying over across provider re-selection. * Adds safer, more accurate `/v1/models` probing with SSRF protection for private/internal targets. * **Documentation** * Updated `NEMOCLAW_CONTEXT_WINDOW` guidance and override/probe priority rules. * **Tests** * Expanded unit, integration, and e2e coverage for overrides, probing, and SSRF behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Yimo Jiang <yimoj@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- 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>
21 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
7 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
Compatible-endpoint / custom onboarding never probed the endpoint's runtime context window, and the Hermes image/config carried no context setting. A self-hosted vLLM endpoint serving a NemotronH model (
max_model_len=65536) therefore fell back to a ~4K architecture default, and Hermes failed withError: Context length exceeded (4,181 tokens). Cannot compress further.This teaches onboarding to probe the endpoint'smax_model_len(or honor an explicitNEMOCLAW_CONTEXT_WINDOW) and propagate the window into the generated Hermes config.Related Issue
Fixes #6177
Changes
src/lib/inference/compatible-endpoint-context.ts: probes the configured OpenAI-compatible endpoint's/v1/modelsformax_model_lenand setsNEMOCLAW_CONTEXT_WINDOWwhen the user has not set one. Wired into the custom-endpoint validation-success path insetup-nim-selection.ts(keepssrc/lib/onboard.tsnet-zero for the growth guardrail).NEMOCLAW_CONTEXT_WINDOWalways wins and is never downgraded./v1/modelsid match never bakes an unrelated model's window (local vLLM keeps its single-served-model fallback).machine/handlers/provider-inference.ts), so retrying away to another provider cannot leave endpoint A's probed window in the environment wheredockerfile-patchwould treat it as a user override./v1/modelsentries so an arbitrary endpoint returning{"data":[null]}cannot crash onboarding.ARG/ENV NEMOCLAW_CONTEXT_WINDOW(empty → auto-detect) inagents/hermes/Dockerfile, reads it inbuild-env.ts, and writes the resolved window asmodel.context_lengthin the generatedconfig.yaml. Hermes reads onlycontext_length(context_windowis silently ignored upstream); the model-block key is its highest-priority override, above/v1/modelsdiscovery and its built-in NemotronH metadata.context_lengthbehavior inswitch-inference-providers.mdx.Type of Change
Quality Gates
codex review --uncommittedclean ("No discrete correctness issues"); credential handling reuses the existing curl auth-config (key in a 0600 tmpfile, never argv); endpoint URL is the already-validated/normalized onboarding value.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest runforcompatible-endpoint-context,vllm-runtime-context,dockerfile-patch-hermes-context,provider-inference,compatible-endpoint-context-probe,generate-hermes-config,hermes-gateway-wrapper→ all pass (139+ tests);npm run typecheck:cliclean.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)E2E evidence — real worktree CLI
Verified end-to-end with the worktree CLI (
node ./bin/nemoclaw.js) driving a real onboard against a fake OpenAI-compatible endpoint (bound to127.0.0.1, serving/v1/modelswithmax_model_len: 65536). Docker was available on the host, so the full Hermes sandbox image was built and the sandbox created.Command (reporter's flow, fake endpoint substituted for the DGX Spark vLLM server):
Onboard transcript (post-fix):
Generated Hermes
config.yamlin the built sandbox image (post-fix):Pre-fix contrast (same image,
NEMOCLAW_CONTEXT_WINDOWunset → the behavior before this PR): themodel:block has nocontext_length, so Hermes falls back to its NemotronH metadata default (~4K) — the reported failure:The only part of the reporter's setup not reproducible here is the DGX Spark GPU serving the real 120B NemotronH weights and a live
hermes chat; the fake endpoint reproduces the exact/v1/modelsmax_model_lensignal the fix consumes. Supporting unit/integration tests (real curl vs a live server, real Hermes config generator, real Hermes Dockerfile patch) are listed above.Signed-off-by: Yimo Jiang yimoj@nvidia.com
Summary by CodeRabbit
NEMOCLAW_CONTEXT_WINDOW, overriding auto-detection and emitting the resolved value asconfig.yaml’scontext_length./v1/models(max_model_len) whenNEMOCLAW_CONTEXT_WINDOWis unset./v1/modelsprobing with SSRF protection for private/internal targets.NEMOCLAW_CONTEXT_WINDOWguidance and override/probe priority rules.