Skip to content

fix(cli): report inference health from a served request - #8731

Open
laitingsheng wants to merge 2 commits into
mainfrom
fix/inference-health-truthfulness
Open

fix(cli): report inference health from a served request#8731
laitingsheng wants to merge 2 commits into
mainfrom
fix/inference-health-truthfulness

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

nemoclaw <name> status reported inferenceHealth.ok: true and exited 0 while the in-sandbox inference route answered HTTP 401, and start exited 0 on the same route. The route probe reads any final HTTP 200-499 as reachable, so a rejected provider credential passed it. Status and start now send one inference request over the same route and report its result, so a rejected credential reports unauthorized and exits nonzero.

Related Issue

Fixes #8705

Changes

  • Rename src/lib/actions/sandbox/rebuild-inference-preflight.ts to inference-invocation-probe.ts and its exports to buildSandboxInferenceInvocationCommand and probeSandboxInferenceInvocation. The module already sent one minimal completion through inference.local on the sandbox's stored credential, but only Deep Agents Code rebuild preflight consumed it. Status and start are the second and third consumers, so the name now describes the behaviour rather than the first caller. The failure result adds httpStatus, and the timeout is a parameter: rebuild keeps 100 seconds, status and start use 30.
  • Compose inference health in src/lib/actions/sandbox/inference-route-health.ts. When the route probe reports the route reachable, status sends one inference request and its result drives inferenceHealth.ok; HTTP 401 or 403 sets failureLabel: "unauthorized". The route probe result becomes a route reachability subprobe so a failing verdict still shows that the route itself answered. A host-side upstream probe stays a diagnostic and does not change ok, keeping the sandbox route authoritative.
  • Gate startSandbox readiness on the same request inside verifyStarted, after the existing gateway and forward checks, and exit 1 when it fails. A sandbox with no recorded provider or model has nothing to request, so start skips it and exits 0.
  • Take the request's provider and model as one pair, preferring the live gateway route and falling back to the recorded route, so a partially readable live route cannot pair a live model with a recorded provider.
  • Update docs/reference/commands.mdx, including the Inference state table, and the pages whose acceptance criteria or failing-state lists this change invalidates: docs/inference/choose-local-inference-server.mdx, docs/inference/set-up-llama-cpp.mdx, docs/inference/set-up-ollama.mdx, docs/inference/set-up-vllm-on-two-dgx-stations.mdx, docs/deployment/deploy-to-headless-server.mdx, docs/monitoring/monitor-sandbox-activity.mdx, docs/reference/troubleshooting.mdx, and docs/security/credential-rotation.mdx.
  • Update the inference health entries in .agents/skills/_shared/controlled-words.md, which defined the term as the /v1/models classification alone.

Contract changes maintainers should weigh

  • This reverses part of nemoclaw status reports inference healthy on endpoint reachability, not model invocability — a green status can mask an unreachable model #6846. Two tests asserted the reported behaviour and are rewritten: the unit test expecting { ok: true, okLabel: "reachable" } beside a failed unauthorized subprobe, and the CLI test named "treats an inference.local HTTP %s as healthy", which asserted exit 0 for 401 and 403.
  • status and start now send a 16-token request through the stored provider credential on every run, so both wait up to 30 seconds for it and consume provider tokens on a hosted route.
  • doctor is unchanged and still classifies an HTTP 401 or 403 route response as reachable, so it exits 0 where status now reports unauthorized. The difference is documented; making doctor consistent needs its own change.
  • The issue also asks for phase to stop reporting Ready when inference is broken. phase is OpenShell's own container lifecycle value, parsed in src/lib/state/gateway.ts and shared with TERMINAL_SANDBOX_PHASES, so this change leaves it alone and reports the condition through inferenceHealth and the exit codes instead.
  • The serving-process gap in status inference health can only attest a fresh exec's environment, never the serving process's — surface process-level (self-reported) health #7003 remains open. The request runs in a fresh sandbox exec, so it attests the route and credential, not the long-running gateway process's own environment.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • 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:
  • Tests not applicable — justification:
  • 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:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: docs-updated
  • Evidence: docs/reference/commands.mdx, docs/inference/choose-local-inference-server.mdx, docs/inference/set-up-llama-cpp.mdx, docs/inference/set-up-ollama.mdx, docs/inference/set-up-vllm-on-two-dgx-stations.mdx, docs/deployment/deploy-to-headless-server.mdx, docs/monitoring/monitor-sandbox-activity.mdx, docs/reference/troubleshooting.mdx, docs/security/credential-rotation.mdx, .agents/skills/_shared/controlled-words.md
  • Agent: Claude Code

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run src/lib/actions/sandbox/status-snapshot-inference-health.test.ts src/lib/actions/sandbox/status-inference.test.ts src/lib/actions/sandbox/start.test.ts src/lib/actions/sandbox/inference-invocation-probe.test.ts src/lib/actions/sandbox/status-snapshot-recovery.test.ts — 5 files, 71 tests passed; npx vitest run test/cli — 35 files, 283 tests passed; the new CLI cases fail against a pre-fix build with expected +0 to be 1
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • New Features

    • Sandbox readiness and status checks now validate an actual inference request, not just route reachability.
    • Inference health results distinguish healthy, unauthorized, reachable, unhealthy, unreachable, and unverified states.
    • Startup reports actionable diagnostics and fails when configured inference cannot be served, while skipping validation when no provider or model is configured.
    • Status output includes clearer route, authentication, and inference details.
  • Documentation

    • Updated deployment, setup, monitoring, troubleshooting, and credential-rotation guidance to reflect authenticated inference verification.

The in-sandbox route probe reads any final HTTP 200-499 as reachable, so a
rejected provider credential answered 401 while status reported healthy
inference and exited 0. Status and start now send one inference request over
the same route and report its result, and start exits nonzero when the
request fails.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 953ab908-a862-4272-8555-01afc066645e

📥 Commits

Reviewing files that changed from the base of the PR and between 514c16e and 420fd4e.

📒 Files selected for processing (4)
  • docs/inference/set-up-llama-cpp.mdx
  • docs/inference/set-up-ollama.mdx
  • src/lib/actions/sandbox/start.test.ts
  • src/lib/actions/sandbox/start.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • docs/inference/set-up-llama-cpp.mdx
  • src/lib/actions/sandbox/start.ts
  • docs/inference/set-up-ollama.mdx
  • src/lib/actions/sandbox/start.test.ts

📝 Walkthrough

Walkthrough

Sandbox inference health now includes an in-sandbox inference request after route probing. Startup readiness, status output, failure labels, diagnostics, tests, and documentation now reflect request results.

Changes

Sandbox inference health

Layer / File(s) Summary
Invocation probe API
src/lib/actions/sandbox/inference-invocation-probe.ts, src/lib/actions/sandbox/inference-invocation-probe.test.ts
Renames the probe API, adds HTTP status details, and supports configurable timeouts.
Route-health composition
src/lib/actions/sandbox/inference-route-health.ts
Combines route reachability, inference invocation results, provider diagnostics, and failure labels.
Status snapshot integration and coverage
src/lib/actions/sandbox/status-snapshot.ts, src/lib/actions/sandbox/status-inference.test.ts, src/lib/actions/sandbox/status-snapshot-inference-health.test.ts, src/lib/actions/sandbox/status-snapshot-recovery.test.ts
Status probes the selected provider/model pair and reports reachability as a nested subprobe. Tests cover successful, unauthorized, upstream-failure, and unreachable cases.
Startup and rebuild wiring
src/lib/actions/sandbox/start.ts, src/lib/actions/sandbox/start.test.ts, src/lib/actions/sandbox/rebuild-dcode-preflight.ts, test/helpers/rebuild-flow-harness.ts
Startup performs inference verification after gateway checks. Rebuild checks use the shared invocation probe.
CLI stubs and status validation
test/cli/helpers.ts, test/cli/sandbox-status-json.test.ts, test/cli/sandbox-status-text.test.ts
CLI tests simulate inference requests and validate request-aware status output.
Documentation and terminology
.agents/skills/_shared/controlled-words.md, docs/deployment/*, docs/inference/*, docs/monitoring/*, docs/reference/*, docs/security/*
Documentation defines healthy, unauthorized, and route-only reachability behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant SandboxStatus
  participant GatewayProbe
  participant InferenceProbe
  participant RouteHealth
  CLI->>SandboxStatus: request status
  SandboxStatus->>GatewayProbe: probe sandbox route
  GatewayProbe-->>SandboxStatus: reachability result
  SandboxStatus->>InferenceProbe: send provider/model inference request
  InferenceProbe-->>SandboxStatus: response status and invocation result
  SandboxStatus->>RouteHealth: combine probe results
  RouteHealth-->>CLI: inference health and diagnostic subprobes
Loading

Possibly related PRs

  • NVIDIA/NemoClaw#6412: Extends earlier inference route-health probing with in-sandbox inference validation.
  • NVIDIA/NemoClaw#7074: Also changes inference verification from reachability checks to authenticated model invocation.
  • NVIDIA/NemoClaw#8572: Also modifies sandbox status inference probing and related tests.

Suggested labels: area: routing

Suggested reviewers: jyaunches, cv

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR fixes inference health and start failures, but it explicitly leaves the OpenShell lifecycle phase unchanged when inference fails, contrary to issue #8705. Update lifecycle readiness so failed or unauthorized inference prevents the sandbox from reporting phase Ready, while preserving separate lifecycle semantics if required.
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: reporting inference health from a successful served inference request.
Out of Scope Changes check ✅ Passed The code, tests, and documentation changes directly support inference-request health reporting, unauthorized handling, start readiness, and issue #8705.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/inference-health-truthfulness

Comment @coderabbitai help to get the list of available commands.

@laitingsheng laitingsheng added area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression labels Aug 10, 2026
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: Review the warnings below.
Findings: 0 blockers · 1 warning · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 1 warning · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 fewer warning, the same number of suggestions.
3 terminology differences from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • route reachability at docs/reference/commands.mdx:1577: primary classified it as justified; the second opinion classified it as established.
  • unauthorized at .agents/skills/_shared/controlled-words.md:187: selected only by the second-opinion lane as define.
  • healthy at .agents/skills/_shared/controlled-words.md:187: selected only by the second-opinion lane as define.
2 additional E2E selections from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • cloud-inference: The completed second-opinion lane identified E2E coverage that the primary lane omitted.
  • gpu-e2e: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

3 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

  • established — inference health at .agents/skills/_shared/controlled-words.md:187: Keep `inference health` for the combined classification. Preserve `inference route reachability` for the route-probe result.
  • justified — route reachability at docs/reference/commands.mdx:1577: Use `route reachability` only as the concise subprobe label. Use `inference route reachability` when explanatory text needs the full established term.
  • justified — sandbox inference invocation probe at src/lib/actions/sandbox/inference-invocation-probe.ts:112: Keep the term for internal diagnostics and tests. Use `inference request` for user-facing explanatory text unless the probe implementation matters.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: managed-image-multiarch-startup, managed-image-protected-runtime, inference-routing, network-policy, onboard-repair, onboard-resume, rebuild-openclaw, state-backup-restore, ubuntu-repo-docker-post-reboot-recovery

2 optional E2E recommendations
  • ollama-auth-proxy
  • sandbox-operations
1 warning · 0 suggestions

Warnings

Warnings do not block.

PRA-1 Warning — Use the live route API family for the status invocation probe

  • Location: src/lib/actions/sandbox/status-snapshot.ts:567
  • Category: correctness
  • Problem: Status selects the live provider and model when the gateway route is readable, but it always passes the recorded sandbox preferredInferenceApi to the invocation probe.
  • Impact: A route drift between API families can send the health request to an endpoint that the live route does not support. Status can then report inference failure although the live route can serve agent requests.
  • Recommendation: Resolve the API family from the same live route selected for provider and model. If the live API family cannot be established, do not combine recorded API metadata with the live route.
  • Verification: Inspect a status snapshot where the recorded route uses anthropic-messages or openai-responses and the live route uses an OpenAI Chat Completions provider. Confirm that the generated probe endpoint matches the live route.
  • Test coverage: Add a status-snapshot test with route drift across API families. Assert that the invocation probe receives the live route API family, or is skipped when that family is unavailable.
  • Evidence: src/lib/actions/sandbox/status-snapshot.ts:555-574 selects live provider and model but passes sb?.preferredInferenceApi to runSandboxInferenceInvocationProbe. src/lib/inference/config.ts:242-266 selects the request endpoint from preferredInferenceApi and provider. src/lib/actions/sandbox/status-snapshot-route-drift.test.ts:45-71 proves provider/model drift is supported status behavior.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@copy-pr-bot

copy-pr-bot Bot commented Aug 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DGX Spark][CLI&UX] inferenceHealth.ok reports true and phase Ready while inference returns HTTP 401 (false health/readiness)

1 participant