fix(onboard): report an unhealthy inference route as not ready - #6862
Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
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:
📝 WalkthroughWalkthroughLoopback inference endpoints are rewritten for sandbox access. Deployment verification now probes and classifies the inference route, making failures affect readiness. Finalization reports readiness through the dashboard, process exit code, and returned state. ChangesInference endpoint normalization
Inference route verification
Finalization readiness reporting
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 |
laitingsheng
added
NV QA
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: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/helpers/onboard-final-flow-phases.ts (1)
237-287: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
isDeploymentHealthyshould reflect the verification result.
test/helpers/onboard-final-flow-phases.ts:253hardcodesisDeploymentHealthy: () => true, so a test that overrides onlyverifyDeploymentcannot exercise the unhealthy path. Derive it fromresult.healthyor add a recorder override hook.🤖 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/helpers/onboard-final-flow-phases.ts` around lines 237 - 287, Update the isDeploymentHealthy dependency in the finalizationDeps setup to reflect the result returned by verifyDeployment, using result.healthy or an equivalent shared recorder state. Preserve the default healthy behavior while allowing tests that override only verifyDeployment to exercise unhealthy deployment paths.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 `@src/lib/inference/local.ts`:
- Around line 64-69: Update rewriteHostLoopbackForSandbox to parse the URL first
and rewrite only when parsed.hostname equals localhost or 127.0.0.1, using
case-insensitive matching for hostname variants; otherwise return the original
URL. Add regression coverage for unrelated paths/hostnames and uppercase
LOCALHOST while preserving port, path, query, and hash handling.
In `@src/lib/verify-deployment.ts`:
- Around line 573-581: Update the healthy calculation in the deployment
verification flow to require inference.status to be neither "unhealthy" nor
"unreachable". Ensure isDeploymentHealthy() and resume readiness consume this
result so an unreachable inference proxy cannot report the deployment as ready,
while preserving the existing gateway and dashboard checks.
---
Outside diff comments:
In `@test/helpers/onboard-final-flow-phases.ts`:
- Around line 237-287: Update the isDeploymentHealthy dependency in the
finalizationDeps setup to reflect the result returned by verifyDeployment, using
result.healthy or an equivalent shared recorder state. Preserve the default
healthy behavior while allowing tests that override only verifyDeployment to
exercise unhealthy deployment paths.
🪄 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: ba2aff06-623f-4c7f-add3-11152bf1fa4e
📒 Files selected for processing (11)
src/lib/inference/local.test.tssrc/lib/inference/local.tssrc/lib/onboard.tssrc/lib/onboard/dashboard.tssrc/lib/onboard/finalization-deps.test.tssrc/lib/onboard/finalization-deps.tssrc/lib/onboard/machine/handlers/finalization.test.tssrc/lib/onboard/machine/handlers/finalization.tssrc/lib/verify-deployment.test.tssrc/lib/verify-deployment.tstest/helpers/onboard-final-flow-phases.ts
…iting compatible endpoints rewriteHostLoopbackForSandbox tested the raw endpoint string, so a remote URL merely containing "localhost" or "127.0.0.1" in its host, path, query, or fragment was wrongly rewritten. Parse the URL first and rewrite only an exact localhost/127.0.0.1 hostname. The custom and Anthropic-compatible endpoint branch in handleRemoteProviderSelection also wrapped its endpoint in this rewrite before gatewayReachableCompatibleEndpointUrl ran its own exact-authority, bundled-port gateway rewrite downstream. Rewriting twice replaced the raw loopback URL needed for host-side credential validation and for that gateway check, breaking both. Stop rewriting in handleRemoteProviderSelection and let the existing gateway-route check handle it, as it already did before this change. Restructure the growth-guardrail-triggering conditional mock in the new verify-deployment.ts unhealthy-inference-route test into a flat response. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
PR Review Advisor PRA-1 was checked against the current registration path and is not actionable for #6848. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> # Conflicts: # test/e2e/support/jetson-workflow-boundary.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-6862.docs.buildwithfern.com/nemoclaw |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/verify-deployment.test.ts (1)
200-210: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winIsolate the HTTP 5xx response to the inference probe.
The
executeSandboxCommandstub on Line 202 returns503for every command, including the gateway probe. Assert that the gateway remains reachable and return503only when the script containsinference.local; otherwise this test can pass without proving that the inference route is the cause of the unhealthy result.Proposed test adjustment
- executeSandboxCommand: () => ({ status: 0, stdout: "503", stderr: "" }), + executeSandboxCommand: (_name, script) => + script.includes("inference.local") + ? { status: 0, stdout: "503", stderr: "" } + : { status: 0, stdout: "200", stderr: "" }, ... + expect(result.verification.gatewayReachable).toBe(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/verify-deployment.test.ts` around lines 200 - 210, Update the executeSandboxCommand stub in the inference-route test so it returns 503 only when the requested script targets inference.local, while returning a successful response for other probes. Add an assertion that the gateway remains reachable, preserving the existing assertions that the inference route is unhealthy and its diagnostics include 503.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/get-started/quickstart.mdx`:
- Around line 325-329: Rewrite the onboarding verification and recovery guidance
in docs/get-started/quickstart.mdx lines 325-329 using direct second-person
instructions, including “verify,” “restore,” and “rerun” actions. Apply the same
instructional style to the route-reachability and recovery guidance in
docs/inference/verify-inference-route.mdx lines 37-39; update both sites
consistently without changing the documented behavior.
---
Outside diff comments:
In `@src/lib/verify-deployment.test.ts`:
- Around line 200-210: Update the executeSandboxCommand stub in the
inference-route test so it returns 503 only when the requested script targets
inference.local, while returning a successful response for other probes. Add an
assertion that the gateway remains reachable, preserving the existing assertions
that the inference route is unhealthy and its diagnostics include 503.
🪄 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: d13fe696-a936-4eec-97cd-e921cced1380
📒 Files selected for processing (5)
docs/get-started/quickstart.mdxdocs/inference/verify-inference-route.mdxsrc/lib/onboard.tssrc/lib/verify-deployment.test.tssrc/lib/verify-deployment.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/onboard.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
I am taking over the remaining follow-up on this PR. The path to completion is:\n\n- keep failed final readiness verification in a retryable onboarding state instead of completing the session;\n- add focused coverage plus a live resume scenario for route failure → repair → successful resume;\n- reconcile the docs and outstanding review-test gaps;\n- push and monitor required CI, E2E, CodeRabbit, and PR Review Advisor feedback until the PR is green;\n- from that green SHA, run the staging GCP image build in nemoclaw-image and validate the resulting image through a real Brev launchable.\n\nI will keep PR updates minimal. The next progress comment will be the successful staging build/launchable link, unless an external blocker requires maintainer input. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
# Conflicts: # docs/get-started/quickstart.mdx # docs/inference/verify-inference-route.mdx # src/lib/onboard.ts # src/lib/onboard/machine/final-flow-phases.runtime.test.ts # src/lib/onboard/machine/handlers/finalization.test.ts # src/lib/onboard/machine/handlers/finalization.ts # test/helpers/onboard-final-flow-phases.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
|
Status update:
|
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Maintainer review complete for commit adc90c0. Retryable unhealthy final verification is correct and security-reviewed; local-provider post-ready guidance and universal final-route guidance are now documented and tested separately. Focused docs test and Fern build pass.
<!-- 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 -->
6 participants
Summary
Onboard finalization now treats an unhealthy committed inference route as not ready. When the route is unreachable or returns HTTP 5xx, onboarding prints
<agent> is not ready, exits non-zero, and keeps the session paused at retryable final verification. After the route is repaired,onboard --resumerechecks the existing sandbox and completes without rebuilding it.Related Issue
Fixes #6849
Fixes #6848
Changes
verify-deployment: classifies HTTP 5xx and unreachable inference results as blocking readiness failures, while preserving independent gateway and dashboard health evidence.in_progressand resumable, and does not report completion.onboard-resumescenario that stops the committed inference route, proves not-ready state, restores the same endpoint, and proves successful resume.Type of Change
Quality Gates
Verification
Signed-off-by:lines and every added commit appears as Verified in GitHubnpm run docspassed with 0 errorsSigned-off-by: Tinson Lai tinsonl@nvidia.com
Signed-off-by: Julie Yaunches jyaunches@nvidia.com
Signed-off-by: Carlos Villela cvillela@nvidia.com