fix(inference): make shared route changes explicit - #6773
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added
area: inference
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
|
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:
📝 WalkthroughWalkthroughThe shared gateway route contract now distinguishes advisory provider/model differences from hard conflicts. Onboarding and connect flows warn and repoint valid routes, while status reports recorded routes and live-route drift through text and JSON output. Documentation and tests reflect these behaviors. ChangesShared route contract
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SandboxOnboarding
participant GatewayRouteCompatibility
participant SharedGateway
participant SandboxStatus
SandboxOnboarding->>GatewayRouteCompatibility: check requested provider/model route
GatewayRouteCompatibility-->>SandboxOnboarding: advisory conflict and affected sandboxes
SandboxOnboarding->>SharedGateway: repoint live inference route
SandboxStatus->>SharedGateway: probe live route
SharedGateway-->>SandboxStatus: live provider/model
SandboxStatus-->>SandboxStatus: compare live and recorded routes
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-6773.docs.buildwithfern.com/nemoclaw |
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 4 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/reference/commands.mdx (1)
1-1: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
connect's warning behavior is inconsistently documented — sometimes omitted entirely.
docs/reference/commands.mdx's connect section (lines 822-825) explicitly statesconnectwarns and re-points the shared route. Four other places describing the sameconnectre-pointing behavior omit the "warns" part, implying a silent re-point — which contradicts the PR's stated goal of eliminating silent cross-sandbox route changes (#6315).
docs/reference/commands.mdx#L2838-2844: line 2843 says "Connecting to a sandbox re-points the route..." — add "warns and" to match lines 822-825 in the same file.docs/inference/switch-models.mdx#L61-70: line 67 says "Connecting to a sandbox later re-points the route..." — state that connect also warns first.docs/inference/switch-providers.mdx#L101-112: line 106 says connect "re-points it again" — state that connect warns before re-pointing.docs/reference/troubleshooting.mdx#L358-370: line 362 says connect "re-points it" — state that connect warns before re-pointing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/commands.mdx` at line 1, Update the four documentation descriptions of connect’s route behavior in commands.mdx, switch-models.mdx, switch-providers.mdx, and troubleshooting.mdx to explicitly state that connect warns before re-pointing the shared route. Match the existing wording in the connect section of commands.mdx, preserving the surrounding documentation.
🧹 Nitpick comments (1)
src/lib/onboard/machine/handlers/provider-inference-route-containment.ts (1)
46-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the docstring to reflect the new advisory/unconstrained behavior.
The comment
// Reject structurally unsafe peer metadata, then exact-check complete route identities.no longer fully describes the function: it now also short-circuits on advisory conflicts (Line 60-62) and always discards the preflight-derivedrequiredModel/requiredEndpointUrl/requiredInferenceApivalues in favor ofunconstrainedGatewayRouteDiscovery()(Line 76), even on a successful (ok: true) preflight. This is validated by the new "does not constrain fresh selection to a valid peer route" test, so the behavior itself looks intentional, but the stale doc comment could mislead future readers into thinking real constraints are still propagated.📝 Suggested docstring update
-/** Reject structurally unsafe peer metadata, then exact-check complete route identities. */ +/** + * Reject structurally unsafe peer metadata, then exact-check complete route + * identities. Advisory conflicts (and the successful preflight path) return + * unconstrained discovery constraints; the actual repoint/warning happens at + * route mutation time (see setup-inference.ts). + */🤖 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-route-containment.ts` around lines 46 - 76, Update the docstring for guardProviderInferenceRouteSelection to describe advisory conflicts short-circuiting to unconstrained discovery and the function always returning unconstrainedGatewayRouteDiscovery(), rather than propagating preflight-derived constraints. Keep the existing validation behavior unchanged.
🤖 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-models.mdx`:
- Around line 61-70: The “Account for Shared Gateways” section should explicitly
document that connecting to a sandbox warns before re-pointing the shared route
to its recorded model. Update the sentence about connecting near the shared
gateway behavior description, preserving the existing re-pointing behavior and
matching the warning terminology used by the connect command documentation.
In `@docs/inference/switch-providers.mdx`:
- Around line 101-112: Update the shared-gateway documentation around the
statement that `$$nemoclaw <name> connect` re-points the route to explicitly
state that connect warns about affected sandboxes before re-pointing it,
matching the documented behavior in the connect command reference.
In `@docs/reference/commands.mdx`:
- Around line 2838-2844: Update the onboarding contract text around “Connecting
to a sandbox” to state that NemoClaw warns before re-pointing the shared route
when another sandbox has a different valid route, matching the established
connect behavior and warning details. Keep the existing recorded provider/model
and live-route drift statements unchanged.
In `@docs/reference/troubleshooting.mdx`:
- Around line 358-370: The troubleshooting documentation should state that
`$$nemoclaw <name> connect` warns before re-pointing the shared live route,
matching the documented behavior in the connect command section. Update the
affected sentence without changing the surrounding route, status, or onboarding
guidance.
In `@src/lib/actions/sandbox/status-snapshot.ts`:
- Around line 298-304: Update the currentModel and currentProvider selection in
the sandbox status snapshot so that when sb exists, each missing recorded field
renders as "unknown" rather than falling back to the live route; only use
live.model and live.provider when sb is absent. Add a regression test covering a
sandbox with partial provider/model metadata and verify routeDrift remains null.
In `@src/lib/inference/gateway-route-compatibility.ts`:
- Around line 346-348: Update the shared warning formatter around the target
assignment to use operation-neutral wording instead of “Onboarding,” so warnings
from both onboarding and connect flows are accurate. Preserve the safe
sandbox-name formatting and fallback behavior for unnamed runs.
---
Outside diff comments:
In `@docs/reference/commands.mdx`:
- Line 1: Update the four documentation descriptions of connect’s route behavior
in commands.mdx, switch-models.mdx, switch-providers.mdx, and
troubleshooting.mdx to explicitly state that connect warns before re-pointing
the shared route. Match the existing wording in the connect section of
commands.mdx, preserving the surrounding documentation.
---
Nitpick comments:
In `@src/lib/onboard/machine/handlers/provider-inference-route-containment.ts`:
- Around line 46-76: Update the docstring for
guardProviderInferenceRouteSelection to describe advisory conflicts
short-circuiting to unconstrained discovery and the function always returning
unconstrainedGatewayRouteDiscovery(), rather than propagating preflight-derived
constraints. Keep the existing validation behavior 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: dc17b9ac-4264-4574-9b89-2e54abf2bf1c
📒 Files selected for processing (23)
docs/inference/switch-models.mdxdocs/inference/switch-providers.mdxdocs/inference/view-active-inference-route.mdxdocs/reference/commands.mdxdocs/reference/troubleshooting.mdxsrc/lib/actions/sandbox/connect-inference-gateway.tssrc/lib/actions/sandbox/connect-route-containment.test.tssrc/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status-inference.test.tssrc/lib/actions/sandbox/status-snapshot-route-drift.test.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/actions/sandbox/status-text.tssrc/lib/inference/gateway-route-compatibility.test.tssrc/lib/inference/gateway-route-compatibility.tssrc/lib/onboard/machine/handlers/provider-inference-route-containment.test.tssrc/lib/onboard/machine/handlers/provider-inference-route-containment.tssrc/lib/onboard/machine/handlers/provider-inference.tssrc/lib/onboard/machine/handlers/sandbox-route-mutation-lock.test.tssrc/lib/onboard/machine/handlers/sandbox.tssrc/lib/onboard/setup-inference-route-containment.test.tssrc/lib/onboard/setup-inference.tstest/cli/sandbox-status-json.test.tstest/support/status-flow-test-harness.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
E2E Target Results — ✅ All requested tests passedRun: 29273924877
|
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
The exact-head CI and eight-target live run are green, but this head is not safe to merge yet.
-
[P1] The shared-route state accepted by onboarding is not connectable.
setup-inference.ts:242-258treats peer custom-endpoint/custom-API conflicts as advisory and proceeds, whileconnect-inference-gateway.ts:44-52accepts only provider/model conflicts and throws before reading the live route.status-text.ts:255-272nevertheless tells the user thatconnectwill realign it. After onboarding endpoint B beside endpoint A, the documented follow-up command can therefore fail even when B is already live. Either reject this state during onboarding or make connect restore the full custom route, and emit remediation only when it is actually executable. -
[P1] Shared provider endpoint/credential mutation is incomplete and non-transactional. The compatibility request at
setup-inference.ts:242-250omitscredentialEnv, andgateway-route-compatibility.test.ts:239-258explicitly treats KEY_A versus KEY_B as compatible. The provider path then updates endpoint/credential (providers.ts:424-435) beforeremote.ts:345-378runsinference set; a later failure leaves the gateway-global provider on the new endpoint/key with no rollback or route reservation. The impact warning also shows only provider/model, so endpoint/API/credential rotation is invisible. Include a credential-free provider fingerprint (endpoint, API family, and whether credential rotation occurs), fail closed or roll back atomically, and add credential-rotation/apply-failure regressions. -
[P2]
status --jsonchanges the existingmodelandproviderfields from live-route values to recorded-route values (status-snapshot.ts:297-305, 415-425) while retainingschemaVersion: 1. Existing consumers can now associate live health with the wrong route. Preserve the old fields and add explicitrecordedRoute/liveRoute, or version the JSON contract.
The focused 102-test suite passes, which confirms these states are currently outside the regression coverage.
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…rence-routing/ae Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
…rence-routing/ae Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
[P1] src/lib/inference/gateway-route-compatibility.ts:287-318 — keep incomplete custom peers out of the advisory path. Custom-route completeness is checked only when the recorded provider equals the requested provider. If an existing same-gateway sandbox records compatible-endpoint plus a model but lacks endpoint or API metadata, onboarding a different provider classifies that peer only as provider-model. isAdvisoryGatewayRouteConflict() then allows the shared route to be re-pointed, but the legacy sandbox cannot restore its route because connect rejects the incomplete requested custom route before any read or mutation. This strands the peer and contradicts the documented contract that incomplete custom-route metadata remains a hard error. Validate every configured custom peer is restorable before reducing cross-provider differences to advisory, and add a regression with an incomplete compatible-endpoint peer plus a different requested provider.
## Summary Add v0.0.83 release notes to `docs/about/release-notes.mdx` for pre-tag release prep. ## Source Summary - #6773 -> `docs/about/release-notes.mdx`: Shared inference route changes are explicit and fail-safe; status shows recorded route, live route, and drift. - #6875 -> `docs/about/release-notes.mdx`: DGX Station GB300 express setup restored; vLLM storage preflight narrowed. - #6770 -> `docs/about/release-notes.mdx`: Risky Spark vLLM server warning during onboarding. - #6856 -> `docs/about/release-notes.mdx`: Re-onboard reuse preserves tier-default brave/tavily presets. - #6867 -> `docs/about/release-notes.mdx`: Unreachable custom endpoint routed through transport-recovery path. - #6860 -> `docs/about/release-notes.mdx`: Rebuild preflight uses model-aware token field for o-series/GPT-5. - #6845 -> `docs/about/release-notes.mdx`: Corporate CA anchored for image build TLS. - #6833 -> `docs/about/release-notes.mdx`: SSH ControlMaster-delegated forwards recognized in fallback. - #6837 -> `docs/about/release-notes.mdx`: Hermes light skin writes via stdin on macOS. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [x] Tests not applicable — justification: doc-only release notes - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed - [ ] Non-success, skipped, or missing CI check accepted by maintainer ## 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 - [x] `npm run docs` passes with 0 errors Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com> Signed-off-by: Jessica Yaunches <jyaunches@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical `docs/changelog/2026-07-15.mdx` entry with the exact `## v0.0.84` heading for the release candidate range from `v0.0.83` through `710d2b36b9eebcb6bca3c2b2f796a1bdb69c3a31`. Fills two owner-page gaps for model-aware local inference health and pre-write OpenClaw candidate validation. ## Changes - Add the complete shared Fern changelog entry for `v0.0.84`, with literal CLI names and root-absolute OpenClaw and Hermes routes. - Document that sandbox status and doctor compare the configured Ollama or vLLM model with provider inventory without issuing a completion. - Document that host-side OpenClaw `config set` validates the complete candidate before replacing live config or reaching gateway restart. - Reconcile the `v0.0.84` release label with the commit range. PR #6773 is already contained in `v0.0.83` and remains documented there; CI, test-harness, docs-infrastructure, and `.js` to `.mts` migration-only changes require no additional user guidance. ### Source summary - [#6882](#6882) -> `docs/manage-sandboxes/backup-restore.mdx`, `docs/changelog/2026-07-15.mdx`: Explain that OpenClaw runtime identity and pairing state are excluded from snapshots and ignored during restore. - [#6873](#6873) -> `docs/inference/set-up-ollama.mdx`, `docs/changelog/2026-07-15.mdx`: Record the Ollama requested-model environment fallback and interactive default. - [#6835](#6835) -> `docs/changelog/2026-07-15.mdx`: Include the sandbox name in the documented rebuild resume-recovery behavior. - [#6886](#6886) -> `docs/inference/custom-endpoint-security.mdx`, `docs/inference/set-up-openai-compatible-endpoint.mdx`, `docs/changelog/2026-07-15.mdx`: Explain the exact-host trusted-private endpoint opt-in and retained SSRF boundaries. - [#6887](#6887) -> `docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Document Telegram channel health verdicts, summary behavior, and exit status. - [#6863](#6863) -> `docs/manage-sandboxes/lifecycle.mdx`, `docs/changelog/2026-07-15.mdx`: Add the missing model-inventory behavior for local status and doctor checks. - [#6902](#6902) -> `docs/manage-sandboxes/runtime-controls.mdx`, `docs/changelog/2026-07-15.mdx`: Add the missing pre-write OpenClaw candidate-validation contract. - [#6916](#6916) -> `docs/changelog/2026-07-15.mdx`: Preserve the failed-session fresh-install recovery correction in the release entry. - [#6934](#6934) -> `docs/reference/commands.mdx`, `docs/reference/troubleshooting.mdx`, `docs/security/credential-storage.mdx`, `docs/changelog/2026-07-15.mdx`: Summarize completed-prompt checkpointing and validated credential reuse during OpenClaw resume. - [#6898](#6898) -> `docs/inference/switch-models.mdx`, `docs/inference/switch-providers.mdx`, `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`: Explain Hermes dashboard convergence after in-place inference changes. - [#6711](#6711) -> `docs/manage-sandboxes/run-sandboxes.mdx`, `docs/manage-sandboxes/uninstall-nemoclaw.mdx`, `docs/reference/architecture.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Summarize port-scoped host state and uninstall preservation. - [#6767](#6767) -> `docs/inference/configure-model-limits.mdx`, `docs/inference/set-up-ollama.mdx`, `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`: Record the Hermes `64000`-token Ollama floor and unchanged OpenClaw floor. - [#6862](#6862) -> `docs/get-started/quickstart.mdx`, `docs/inference/verify-inference-route.mdx`, `docs/changelog/2026-07-15.mdx`: Explain retryable not-ready finalization for unhealthy inference routes. - [#6766](#6766) -> `docs/security/tcb-boundary.mdx`, `docs/changelog/2026-07-15.mdx`: Document definitive stale transition-lock recovery and fail-closed ambiguous cases. - [#6948](#6948) -> `docs/manage-sandboxes/manage-mcp-servers.mdx`, `docs/changelog/2026-07-15.mdx`: Include Hermes MCP apply-state race recovery in the release entry without changing the established user workflow. - [#6964](#6964) -> `docs/reference/troubleshooting.mdx`, `docs/changelog/2026-07-15.mdx`: Record complete agent-specific fresh-install and resume recovery commands. - [#6883](#6883) -> `docs/get-started/quickstart.mdx`, `docs/inference/set-up-vllm.mdx`, `docs/reference/platform-support.mdx`, `docs/changelog/2026-07-15.mdx`: Summarize the DGX Station Nemotron Ultra express path and pinned managed-vLLM recipe. - [#6985](#6985) -> `docs/inference/set-up-vllm.mdx`, `docs/reference/commands.mdx`, `docs/changelog/2026-07-15.mdx`: Capture the final automated and interactive storage-warning behavior. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — `test/changelog-docs.test.ts` validates the dated-entry structure, exact version heading, and preserved history. - [ ] Tests not applicable — justification: - [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 - [x] PR description includes a `Signed-off-by:` line 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 — `npx vitest run test/changelog-docs.test.ts` (6 passed) - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — not run for this doc-only change. - [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) — completed with 0 errors; Fern reported the unchanged unauthenticated redirect-check and light-theme 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) — the native changelog entry uses the required parser-safe MDX SPDX comment and intentionally has no frontmatter. --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added the v0.0.84 changelog entry covering setup, endpoint onboarding, model handling, sandbox readiness, recovery, channel status, and configuration safeguards. * Clarified that sandbox health checks validate configured models against local Ollama and vLLM provider inventories without generating completions or consuming tokens. * Documented that invalid runtime configuration changes are rejected while preserving the existing working configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
5 participants
Summary
OpenShell exposes one live inference route per gateway, so onboarding another sandbox could silently change the route used by existing sandboxes. This change keeps one gateway, warns immediately before a valid shared provider/model route change, fails closed before replacing provider-global identity used by another sandbox, and makes each sandbox's recorded route and any live-route drift explicit in status.
Related Issue
Fixes #6315
Changes
connectuse the same provider-global compatibility decision and only offer it as status remediation when it can restore the recorded route safely.1JSONmodelandprovideras live-first fields, and add explicitrecordedRoute,liveRoute, androuteDrift.canConnectfields.Type of Change
Quality Gates
canConnectfrom the same compatibility check. Focused regression coverage exercises the pre-mutation failure and status remediation decisions.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to this focused inference-route change;npm run check:diff, CLI typecheck, CLI build, and directly affected suites passed on the current-main combined tree.npm run docsbuilds without warnings (doc changes only) — command passed with 0 errors; Fern reported 2 existing suppressed warnings.Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
status --jsonand sandbox status now expose recorded vs live routing provenance (recordedRoute,liveRoute,routeDriftwithcanConnect).canConnectbehavior.