Skip to content

fix(status): probe inference.local route for cloud providers#6264

Closed
souvikDevloper wants to merge 8 commits into
NVIDIA:mainfrom
souvikDevloper:fix/6192-inference-local-route-probe-cloud
Closed

fix(status): probe inference.local route for cloud providers#6264
souvikDevloper wants to merge 8 commits into
NVIDIA:mainfrom
souvikDevloper:fix/6192-inference-local-route-probe-cloud

Conversation

@souvikDevloper

@souvikDevloper souvikDevloper commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

nemoclaw <sbx> status and nemoclaw <sbx> doctor reported inference as
healthy / [ok] by probing the upstream provider endpoint directly
(e.g. the NVIDIA Cloud /v1/models endpoint), instead of the
inference.local route the agent actually calls. When inference.local
was broken inside the sandbox but the upstream stayed reachable, both
commands still reported "healthy" — directly contradicting
nemoclaw <sbx> connect, which probes the real route and correctly
reports it BROKEN.

This is the same class of bug as the previously-fixed #3265 ("status
reports Inference: healthy by probing Ollama directly, bypassing the auth
proxy / gateway"). That fix added an inference.local gateway-chain
subprobe, but gated it to ollama-local / vllm-local providers only,
so the cloud / inference.local path still went unchecked.

Fixes #6192.

What changed

The inference.local gateway-chain subprobe is now run for every known
provider
, not just local ones, in both call sites:

  • src/lib/actions/sandbox/doctor.tscollectInferenceSubprobes no
    longer early-returns for non-local providers. Unknown providers still
    never reach it (collectInferenceChecks returns early on
    provider === "unknown").
  • src/lib/actions/sandbox/status-snapshot.ts — the gateway-chain gate
    changes from ollama-local || vllm-local to currentProvider !== "unknown".

Cloud providers route through the same in-sandbox openclaw gateway / auth
proxy as local ones, so a reachable upstream endpoint does not prove the
agent's route works. A broken inference.local route now surfaces as a
failing Provider health (gateway) check (doctor -> attention needed /
exit 1) and an unhealthy Inference (gateway) line (status), so
status, doctor, and connect agree.

Upstream reachability is still reported on its own line, so no signal is lost.

Testing

  • src/lib/actions/sandbox/doctor-flow.test.ts — new test: a cloud
    provider (nvidia-prod) with a reachable upstream but a broken
    inference.local route now produces a failing gateway check and a
    fail summary.
  • src/lib/actions/sandbox/status-snapshot.test.ts — new file: the
    gateway subprobe runs for a cloud provider (broken route -> ok: false
    subprobe) and is skipped when the provider is unknown.

Both new tests were confirmed to fail on main (before the fix) and pass
with it. Ran the affected cli-project suites locally: doctor-flow,
status-flow, and status-snapshot.

DCO Sign-Off

Signed-off-by: souvikDevloper gshsouvik01@gmail.com

Summary by CodeRabbit

  • Bug Fixes
    • Updated sandbox doctor and status snapshot inference checks to probe the in-app inference gateway chain for every known provider (not only local-style providers), surfacing gateway-route failures as a distinct “gateway chain” failure.
  • Documentation
    • Refreshed inference health guidance to clarify the inference.local route is used for all known providers and should be treated as unhealthy when broken.
  • Tests
    • Added Vitest coverage ensuring correct behavior for cloud providers vs unknown providers, including gateway-chain probe invocation and expected subprobe/report statuses.
  • Chores
    • Added a shared helper to build the “Inference gateway chain” health subprobe.

@copy-pr-bot

copy-pr-bot Bot commented Jul 4, 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.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Sandbox inference checks now probe the inference.local gateway chain for any known provider, not just local providers. A shared helper builds the gateway subprobe, and tests cover cloud-provider failures plus the unknown provider skip path.

Changes

Inference gateway probe scope fix

Layer / File(s) Summary
Shared gateway subprobe helper
src/lib/inference/health.ts
Adds buildInferenceGatewaySubprobe for constructing the inference gateway chain subprobe from a gateway probe result.
Doctor probes gateway for any known provider
src/lib/actions/sandbox/doctor.ts, src/lib/actions/sandbox/doctor-flow.test.ts
Removes local-provider gating from inference subprobe collection, uses the shared helper for gateway results, and adds a flow test where a cloud provider reports upstream health as ok while the in-sandbox gateway probe fails.
Status snapshot probes any known provider
src/lib/actions/sandbox/status-snapshot.ts, src/lib/actions/sandbox/status-snapshot.test.ts
Broadens the status snapshot gateway condition to any non-unknown provider, uses the shared helper in the success path, and adds tests for cloud-provider failure and unknown provider skip behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested labels: bug-fix, v0.0.56

Suggested reviewers: jyaunches, prekshivyas, ericksoa

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main inference.local routing fix for cloud providers.
Linked Issues check ✅ Passed The PR makes status and doctor probe inference.local for known providers and skips unknown ones, matching the linked route bug fix.
Out of Scope Changes check ✅ Passed The changes are focused on the inference route fix and matching tests/helpers, with no unrelated feature work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/lib/actions/sandbox/doctor.ts (1)

355-366: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate subprobe-construction logic with status-snapshot.ts.

This object literal (ok, probed: true, providerLabel: "Inference gateway chain", endpoint, detail, probeLabel: "gateway", conditional failureLabel) is duplicated almost verbatim in src/lib/actions/sandbox/status-snapshot.ts (lines 230-238). Since this PR touches both call sites to broaden the same gating condition, it's a good opportunity to extract a shared helper (e.g. in src/lib/inference/health.ts or a small adapter-facing utility) so future changes to the subprobe shape don't need to be kept in sync manually across two files.

♻️ Proposed shared helper
+// src/lib/inference/health.ts (or similar shared module)
+export function buildInferenceGatewaySubprobe(gateway: {
+  ok: boolean;
+  endpoint: string;
+  detail: string;
+}): ProviderHealthStatus {
+  return {
+    ok: gateway.ok,
+    probed: true,
+    providerLabel: "Inference gateway chain",
+    endpoint: gateway.endpoint,
+    detail: gateway.detail,
+    probeLabel: "gateway",
+    ...(gateway.ok ? {} : { failureLabel: "unreachable" as const }),
+  };
+}

Then both doctor.ts and status-snapshot.ts call buildInferenceGatewaySubprobe(gateway).

🤖 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/actions/sandbox/doctor.ts` around lines 355 - 366, The gateway
subprobe object construction in doctor.ts is duplicated in status-snapshot.ts,
so extract the shared shape into a helper and have both call sites use it.
Create a small reusable function such as buildInferenceGatewaySubprobe that
takes the gateway result and returns the `ok`, `probed`, `providerLabel`,
`endpoint`, `detail`, `probeLabel`, and conditional `failureLabel` fields, then
replace the inline object literal in doctor.ts with that helper and update
status-snapshot.ts to use the same helper.
src/lib/actions/sandbox/status-snapshot.test.ts (1)

26-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Shared mutable fixture is mutated in place by the code under test.

reachableCloudHealth is a module-level const returned directly (not cloned) by probeProviderHealthImpl (line 77). collectSandboxStatusSnapshot then mutates that exact object's subprobes field in place (status-snapshot.ts line 239). Today only one test in this file exercises that branch, so it happens to pass, but the pattern is fragile: any future test reusing reachableCloudHealth (or a re-run within the same process) would see stale subprobes already appended from a prior invocation, causing spurious failures or false passes.

🛠️ Return a fresh clone per invocation
   const collectSandboxStatusSnapshot: CollectSandboxStatusSnapshot =
     requireDist(snapshotModulePath).collectSandboxStatusSnapshot;

   return {
     probeSandboxInferenceGatewayHealthSpy,
     run: () =>
       collectSandboxStatusSnapshot("alpha", {
         deps: {
           getSandbox: (() => sandboxEntry) as never,
           reconcile: async () => presentLookup,
           probeProviderHealthImpl: (provider: string) =>
-            provider === "unknown" ? null : reachableCloudHealth,
+            provider === "unknown" ? null : { ...reachableCloudHealth },
         },
       }),
   };

Also applies to: 76-79

🤖 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/actions/sandbox/status-snapshot.test.ts` around lines 26 - 32, The
shared reachableCloudHealth fixture is being returned by probeProviderHealthImpl
and then mutated in collectSandboxStatusSnapshot via its subprobes field, which
makes later tests/state reuse fragile. Update the test setup so
probeProviderHealthImpl returns a fresh cloned ProviderHealthStatus object each
time instead of the module-level reachableCloudHealth instance, and keep the
mutable subprobes append behavior isolated to that per-invocation copy.
🤖 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/actions/sandbox/doctor.ts`:
- Around line 355-366: The gateway subprobe object construction in doctor.ts is
duplicated in status-snapshot.ts, so extract the shared shape into a helper and
have both call sites use it. Create a small reusable function such as
buildInferenceGatewaySubprobe that takes the gateway result and returns the
`ok`, `probed`, `providerLabel`, `endpoint`, `detail`, `probeLabel`, and
conditional `failureLabel` fields, then replace the inline object literal in
doctor.ts with that helper and update status-snapshot.ts to use the same helper.

In `@src/lib/actions/sandbox/status-snapshot.test.ts`:
- Around line 26-32: The shared reachableCloudHealth fixture is being returned
by probeProviderHealthImpl and then mutated in collectSandboxStatusSnapshot via
its subprobes field, which makes later tests/state reuse fragile. Update the
test setup so probeProviderHealthImpl returns a fresh cloned
ProviderHealthStatus object each time instead of the module-level
reachableCloudHealth instance, and keep the mutable subprobes append behavior
isolated to that per-invocation copy.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6f8d2f9d-1ba9-4e72-9930-ce26d7488b14

📥 Commits

Reviewing files that changed from the base of the PR and between 3f89cd0 and 545f910.

📒 Files selected for processing (4)
  • src/lib/actions/sandbox/doctor-flow.test.ts
  • src/lib/actions/sandbox/doctor.ts
  • src/lib/actions/sandbox/status-snapshot.test.ts
  • src/lib/actions/sandbox/status-snapshot.ts

souvikDevloper pushed a commit to souvikDevloper/NemoClaw that referenced this pull request Jul 4, 2026
Extract the duplicated inference.local gateway-chain subprobe object,
built identically in doctor.ts and status-snapshot.ts, into a shared
buildInferenceGatewaySubprobe helper in inference/health.ts so the
subprobe shape stays in sync across both call sites.

Also clone the shared provider-health fixture per invocation in the
status-snapshot test: collectSandboxStatusSnapshot appends subprobes in
place, so returning the module-level object would leak state across runs.

Both changes address CodeRabbit review feedback on NVIDIA#6264; behavior and
assertions are unchanged.

Signed-off-by: souvikDevloper <gshsouvik01@gmail.com>
@souvikDevloper

souvikDevloper commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both CodeRabbit nitpicks in 790b718:

  • Duplicate subprobe construction (doctor.ts / status-snapshot.ts) — extracted a shared buildInferenceGatewaySubprobe helper in src/lib/inference/health.ts; both call sites now use it, so the subprobe shape stays in sync.
  • Shared mutable fixture (status-snapshot.test.ts)probeProviderHealthImpl now returns a fresh { ...reachableCloudHealth } clone per invocation, so the in-place subprobes append no longer leaks across runs.

Behavior and assertions are unchanged; the cli suites (doctor-flow, status-flow, status-snapshot) pass locally and the growth guardrail stays green.

@souvikDevloper
souvikDevloper force-pushed the fix/6192-inference-local-route-probe-cloud branch from 790b718 to 0136013 Compare July 4, 2026 03:48
souvikDevloper pushed a commit to souvikDevloper/NemoClaw that referenced this pull request Jul 4, 2026
Extract the duplicated inference.local gateway-chain subprobe object,
built identically in doctor.ts and status-snapshot.ts, into a shared
buildInferenceGatewaySubprobe helper in inference/health.ts so the
subprobe shape stays in sync across both call sites.

Also clone the shared provider-health fixture per invocation in the
status-snapshot test: collectSandboxStatusSnapshot appends subprobes in
place, so returning the module-level object would leak state across runs.

Both changes address CodeRabbit review feedback on NVIDIA#6264; behavior and
assertions are unchanged.

Signed-off-by: souvikDevloper <gshsouvik01@gmail.com>
status and doctor reported inference healthy by probing the upstream
provider endpoint directly, but the agent actually calls the in-sandbox
inference.local route. When that route was broken while the upstream was
reachable, both commands still reported healthy, contradicting connect.

The inference.local gateway-chain subprobe added in NVIDIA#3265 was gated to
ollama/vllm-local providers only. Extend it to every known provider in
both the status snapshot and doctor so a broken inference.local route
surfaces as a failing gateway check instead of a false all-clear.

Fixes NVIDIA#6192

Signed-off-by: souvikDevloper <gshsouvik01@gmail.com>
The codebase-growth-guardrails check requires changed test files not to
add if statements so test bodies stay linear. Replace the two-branch
captureOpenshell mock in the new cloud-provider test with a keyed
response lookup, dropping the added if statements without changing the
assertions.

Signed-off-by: souvikDevloper <gshsouvik01@gmail.com>
Extract the duplicated inference.local gateway-chain subprobe object,
built identically in doctor.ts and status-snapshot.ts, into a shared
buildInferenceGatewaySubprobe helper in inference/health.ts so the
subprobe shape stays in sync across both call sites.

Also clone the shared provider-health fixture per invocation in the
status-snapshot test: collectSandboxStatusSnapshot appends subprobes in
place, so returning the module-level object would leak state across runs.

Both changes address CodeRabbit review feedback on NVIDIA#6264; behavior and
assertions are unchanged.

Signed-off-by: souvikDevloper <gshsouvik01@gmail.com>
@souvikDevloper
souvikDevloper force-pushed the fix/6192-inference-local-route-probe-cloud branch from 5c0dbe6 to 9a3f8bd Compare July 4, 2026 03:56
@souvikDevloper

Copy link
Copy Markdown
Contributor Author

@cv hope u gonna take a look over these soon.

@cv cv added the v0.0.76 Release target label Jul 7, 2026

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exact-head advisor found a correctness blocker beyond the formatting failure: when the inference.local gateway subprobe fails, status appends the failed subcheck but leaves the aggregate inferenceHealth.ok value true. JSON and text output can therefore still report healthy even though the route the agent actually uses is broken. Propagate that failure into the aggregate or render an explicit degraded/unhealthy result, and add regression coverage asserting the top-level JSON and text status. Current static-checks also need the createRequire-budget failure resolved.

…s broken

When the inference.local gateway subprobe failed, status left the aggregate
inferenceHealth.ok true, so `status` (text and --json) could still report
inference healthy while `connect` reports the route BROKEN — the exact
contradiction NVIDIA#6192 set out to fix. Propagate the subprobe failure into the
aggregate (ok=false + failureLabel + explanatory detail) so the top-level
status reflects the route the agent actually uses.

Rework the status-snapshot test onto typed dependency injection (injectable
captureLiveInference + probeInferenceGateway) instead of the createRequire
seam, keeping it within the CLI createRequire budget, and add regression
coverage for the degraded aggregate (surfaced verbatim in --json) and the
degraded text output.

Addresses review feedback on NVIDIA#6264.

Signed-off-by: Souvik Ghosh <138186578+souvikDevloper@users.noreply.github.com>
@souvikDevloper

Copy link
Copy Markdown
Contributor Author

Thanks @cv — addressed in ead75a2.

Correctness blocker (aggregate stayed healthy): collectSandboxStatusSnapshot now propagates a failed inference.local gateway subprobe into the aggregate — it sets inferenceHealth.ok = false, a failureLabel, and an explanatory detail ("Provider endpoint is reachable, but the inference.local route the agent uses is broken…"). Because report.inferenceHealth is surfaced verbatim in --json and drives the text summary, both now report unhealthy instead of contradicting connect.

Regression coverage (top-level JSON + text):

  • status-snapshot.test.ts — asserts the aggregate degrades to ok: false + failureLabel: "unreachable" when the route is broken, stays ok: true when it is reachable, and skips the probe for unknown providers. That object is exactly what --json emits as inferenceHealth.
  • status-flow.test.ts — adds a text-render case asserting the top line shows Inference: unreachable (not Inference: healthy) alongside the Inference (gateway) hop.

createRequire budget: reworked status-snapshot.test.ts off the createRequire seam onto typed dependency injection — collectSandboxStatusSnapshot now accepts injectable captureLiveInference and probeInferenceGateway deps — so it passes test-create-require-budget with native imports. Biome format re-runs clean as well.

@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs area: routing Request routing, policy routing, model selection, or fallback logic bug-fix PR fixes a bug or regression labels Jul 7, 2026
@wscurran

wscurran commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@apurvvkumaria

apurvvkumaria commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Superseded by #6412, which consolidates the complete issue #6192 fix on current main. The typed dependency-injection/test-seam contribution by @souvikDevloper is explicitly retained and credited in #6412 through the signed commit Co-authored-by trailer, the PR description, and the contributor-credit comment. Thank you, Souvik, for the contribution.

apurvvkumaria added a commit that referenced this pull request Jul 8, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Make the in-sandbox `https://inference.local/v1/models` route
authoritative for inference health in `nemoclaw status`, `nemoclaw
doctor`, and `nemoclaw connect`. This supersedes #6203 and #6264 while
retaining credit for Harjoth's original implementation and Souvik's
typed test-seam contribution.

## Related Issue

Fixes #6192

## Changes

- Share one agent-aware `inference.local` probe and fail-closed parser
across connect, status, and doctor.
- Treat final HTTP 200-499 responses as route-reachable; treat interim
100-199 responses, HTTP 500-599, `000`, malformed output, timeout, and
unavailable probes as failing.
- Centralize route-failure labels and status exit decisions so text,
JSON, status, doctor, and connect cannot drift.
- Discard route-probe response bodies through `/dev/null` instead of a
persistent sandbox temp file.
- Verify route TLS with OpenShell's managed CA bundle; certificate
failures are authoritative unreachable results.
- Keep direct provider checks as explicitly labeled, non-authoritative
upstream diagnostics.
- Return nonzero status for unhealthy or unavailable inference routes in
text and JSON output.
- Add regression coverage for HTTP boundaries, unavailable probes,
framed OpenShell output, no-direct-probe providers, DCode, and CLI exit
behavior.
- Update command, monitoring, local-inference, and troubleshooting
documentation for the authoritative route semantics.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] 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: [nine-category review
passed after the only hardening finding was
fixed](#6412 (comment)).
- [ ] 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 — `npx vitest run --project cli` over
the nine route/connect/status/doctor/recovery/adapter files: 163 passed;
`npx vitest run --project integration` over the eight changed
CLI/recovery files: 43 passed.
- [x] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: `npm run checks` passed;
the exact-head PR CI suite completed without failures; [typed OpenClaw
and DCode live
targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28970519701)
passed; [inference-routing, diagnostics, and sandbox-operations live
jobs](https://github.com/NVIDIA/NemoClaw/actions/runs/28970503175)
passed.
- [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) — build
passed with 0 errors and two existing Fern 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)


## Architecture and Runtime-Boundary Review

- **Invalid state:** Host/provider checks can succeed while the
agent-visible `https://inference.local/v1/models` route is unusable.
This PR exposes that disagreement; it does not preserve or mask it.
- **Source boundary:** OpenShell owns sandbox exec, DNS, TLS CA
injection, and proxy provisioning. NemoClaw owns interpretation and
orchestration in `status`, `doctor`, and `connect`, so it probes the
exact in-sandbox route and fails closed when OpenShell cannot return
trusted `OK` or `BROKEN` evidence.
- **Source-fix constraint:** Read-only diagnostics cannot repair
OpenShell infrastructure, and an inconclusive result is inherently
lossy. `connect` must not open SSH or mutate route state on inconclusive
evidence. This fail-closed behavior is a permanent security boundary,
not a temporary workaround.
- **Regression and removal contract:** Checked-in parser, flow, CLI,
missing-CA, 401/403, DCode argv/proxy, and redaction tests cover
deterministic fault injection. The DCode wrapper may be removed only if
OpenShell provides an agent-independent structured route-health API that
preserves its CA and login-shell contract; fail-closed handling remains.
- **Runtime justification:** A separate standalone live artifact would
duplicate the repository's existing typed E2E lanes. Exact head
`130e74f3c520aa82db86fd80e830eff362390b08` passed the [required live
jobs](https://github.com/NVIDIA/NemoClaw/actions/runs/28970503175) and
[OpenClaw and DCode live
targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28970519701);
exact-head macOS E2E and arm64 image/build checks also passed. Induced
broken-route, auth, and CA faults remain deterministic checked-in tests
to avoid destructive shared-state mutation.
- **PR sequencing:** #6412 is foundational. Merge it first, then rebase
#6465 and integrate its additive route-drift warning into the final
status-snapshot shape.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added clearer, authoritative inference health checks and diagnostics
for sandbox status and doctor commands.
* `connect` now fails closed when the in-sandbox inference route cannot
be trusted, with safer redacted error details.

* **Bug Fixes**
* Improved handling of reachable, unhealthy, unreachable, and not-probed
inference states.
* Fixed status exit behavior so inference-route failures correctly
return a non-zero result.

* **Documentation**
* Updated troubleshooting and command docs to explain the new inference
health and proxy diagnostic output.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: harjoth <harjoth.khara@gmail.com>
Co-authored-by: Souvik Ghosh
<138186578+souvikDevloper@users.noreply.github.com>

---------

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Co-authored-by: harjoth <harjoth.khara@gmail.com>
Co-authored-by: Souvik Ghosh <138186578+souvikDevloper@users.noreply.github.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Make the in-sandbox `https://inference.local/v1/models` route
authoritative for inference health in `nemoclaw status`, `nemoclaw
doctor`, and `nemoclaw connect`. This supersedes NVIDIA#6203 and NVIDIA#6264 while
retaining credit for Harjoth's original implementation and Souvik's
typed test-seam contribution.

## Related Issue

Fixes NVIDIA#6192

## Changes

- Share one agent-aware `inference.local` probe and fail-closed parser
across connect, status, and doctor.
- Treat final HTTP 200-499 responses as route-reachable; treat interim
100-199 responses, HTTP 500-599, `000`, malformed output, timeout, and
unavailable probes as failing.
- Centralize route-failure labels and status exit decisions so text,
JSON, status, doctor, and connect cannot drift.
- Discard route-probe response bodies through `/dev/null` instead of a
persistent sandbox temp file.
- Verify route TLS with OpenShell's managed CA bundle; certificate
failures are authoritative unreachable results.
- Keep direct provider checks as explicitly labeled, non-authoritative
upstream diagnostics.
- Return nonzero status for unhealthy or unavailable inference routes in
text and JSON output.
- Add regression coverage for HTTP boundaries, unavailable probes,
framed OpenShell output, no-direct-probe providers, DCode, and CLI exit
behavior.
- Update command, monitoring, local-inference, and troubleshooting
documentation for the authoritative route semantics.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [x] Code change with doc updates
- [ ] 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: [nine-category review
passed after the only hardening finding was
fixed](NVIDIA#6412 (comment)).
- [ ] 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 — `npx vitest run --project cli` over
the nine route/connect/status/doctor/recovery/adapter files: 163 passed;
`npx vitest run --project integration` over the eight changed
CLI/recovery files: 43 passed.
- [x] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result: `npm run checks` passed;
the exact-head PR CI suite completed without failures; [typed OpenClaw
and DCode live
targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28970519701)
passed; [inference-routing, diagnostics, and sandbox-operations live
jobs](https://github.com/NVIDIA/NemoClaw/actions/runs/28970503175)
passed.
- [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) — build
passed with 0 errors and two existing Fern 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)


## Architecture and Runtime-Boundary Review

- **Invalid state:** Host/provider checks can succeed while the
agent-visible `https://inference.local/v1/models` route is unusable.
This PR exposes that disagreement; it does not preserve or mask it.
- **Source boundary:** OpenShell owns sandbox exec, DNS, TLS CA
injection, and proxy provisioning. NemoClaw owns interpretation and
orchestration in `status`, `doctor`, and `connect`, so it probes the
exact in-sandbox route and fails closed when OpenShell cannot return
trusted `OK` or `BROKEN` evidence.
- **Source-fix constraint:** Read-only diagnostics cannot repair
OpenShell infrastructure, and an inconclusive result is inherently
lossy. `connect` must not open SSH or mutate route state on inconclusive
evidence. This fail-closed behavior is a permanent security boundary,
not a temporary workaround.
- **Regression and removal contract:** Checked-in parser, flow, CLI,
missing-CA, 401/403, DCode argv/proxy, and redaction tests cover
deterministic fault injection. The DCode wrapper may be removed only if
OpenShell provides an agent-independent structured route-health API that
preserves its CA and login-shell contract; fail-closed handling remains.
- **Runtime justification:** A separate standalone live artifact would
duplicate the repository's existing typed E2E lanes. Exact head
`130e74f3c520aa82db86fd80e830eff362390b08` passed the [required live
jobs](https://github.com/NVIDIA/NemoClaw/actions/runs/28970503175) and
[OpenClaw and DCode live
targets](https://github.com/NVIDIA/NemoClaw/actions/runs/28970519701);
exact-head macOS E2E and arm64 image/build checks also passed. Induced
broken-route, auth, and CA faults remain deterministic checked-in tests
to avoid destructive shared-state mutation.
- **PR sequencing:** NVIDIA#6412 is foundational. Merge it first, then rebase
NVIDIA#6465 and integrate its additive route-drift warning into the final
status-snapshot shape.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Added clearer, authoritative inference health checks and diagnostics
for sandbox status and doctor commands.
* `connect` now fails closed when the in-sandbox inference route cannot
be trusted, with safer redacted error details.

* **Bug Fixes**
* Improved handling of reachable, unhealthy, unreachable, and not-probed
inference states.
* Fixed status exit behavior so inference-route failures correctly
return a non-zero result.

* **Documentation**
* Updated troubleshooting and command docs to explain the new inference
health and proxy diagnostic output.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: harjoth <harjoth.khara@gmail.com>
Co-authored-by: Souvik Ghosh
<138186578+souvikDevloper@users.noreply.github.com>

---------

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: cjagwani <cjagwani@nvidia.com>
Co-authored-by: harjoth <harjoth.khara@gmail.com>
Co-authored-by: Souvik Ghosh <138186578+souvikDevloper@users.noreply.github.com>
Co-authored-by: cjagwani <cjagwani@nvidia.com>
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 area: routing Request routing, policy routing, model selection, or fallback logic bug-fix PR fixes a bug or regression v0.0.76 Release target

Projects

None yet

4 participants