feat(sandbox): add serving process health leg to status and doctor - #7007
Merged
Conversation
…7003) Fresh-exec inference probes run with OpenShell's injected env (CA bundle, proxy, NODE_OPTIONS) and cannot attest what the long-running gateway process can reach. When that process was started without the injected env (e.g. manual recovery after #6635), every probe stays green while all real model calls fail. Adds the manifest contract and status/doctor surface to make this gap visible: - AgentSelfReport type + self_report field in agent manifests: agents that expose a self-report endpoint declare it here; NemoClaw reads and renders it. - ServingProcessHealth discriminated union in SandboxStatusSnapshot and SandboxStatusReport: { checked: false } when no self_report endpoint is declared, { checked: true, ok, detail } when one is probed. - status prints "Serving process (openclaw gateway): not checked" after the inference probe lines when the gateway is running but declares no self_report, replacing silent green with an honest "not checked" signal. - doctor's Inference group gains a "Serving process: not checked" info check for the same reason. No agent currently declares self_report, so all agents show "not checked" for now. The plumbing is extensible: adding self_report to a manifest wires the endpoint into status/doctor automatically. Refs #7003 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional agent ChangesServing process health
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
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 |
Contributor
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: 3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/actions/sandbox/doctor-inference.ts`:
- Around line 159-167: Update collectInferenceChecks to accept the agent’s
self_report declaration and conditionally add the “Serving process” info check
only when that declaration is absent. Preserve the existing diagnostic text for
agents without self_report, and add test coverage confirming declared
self_report agents do not receive this check.
In `@src/lib/agent/defs.test.ts`:
- Around line 411-427: Update the test “parses self_report url and timeout from
manifests (`#7003`)” to use a non-default timeout such as 7 and assert that parsed
value, then add a separate test case omitting timeout_seconds that verifies
readSelfReport() preserves the fallback value of 10.
🪄 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: b9ac499d-1816-4a32-8b76-70792b42142a
📒 Files selected for processing (13)
src/lib/actions/sandbox/doctor-inference.test.tssrc/lib/actions/sandbox/doctor-inference.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/actions/sandbox/status-text.tssrc/lib/actions/sandbox/status.tssrc/lib/agent/definition-types.tssrc/lib/agent/defs.test.tssrc/lib/agent/defs.tssrc/lib/agent/hermes-recovery-boundary-fixtures.tssrc/lib/agent/manifest-readers.tssrc/lib/agent/onboard.test.tssrc/lib/agent/runtime.test.tstest/helpers/base-image-test-harness.ts
…7003) Two findings from the PR review: 1. `collectInferenceChecks` was unconditionally appending the "not checked" serving-process leg. Agents that declare `self_report` should suppress it; `agentHasSelfReport` dep gates the check. `doctor.ts` resolves the flag by loading the agent manifest and passes it through. 2. The self_report parse test used timeout_seconds 10, which is also the fallback value, masking whether the parsed field was actually used. Changed to 7 for the explicit case and added a separate test for the fallback path (absent timeout_seconds → 10). Refs #7003 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/actions/sandbox/doctor.ts (1)
391-397: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the doctor-to-agent wiring with an integration test.
The added tests exercise
collectInferenceCheckswith a manually supplied flag, so they would still pass ifresolveAgentHasSelfReportor the newcollectDoctorCheckswiring regressed. Add a doctor-level behavior test that loads manifests with and withoutself_reportand verifies the resulting Serving process check.Also applies to: 413-415
🤖 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 391 - 397, Add an integration-style doctor test covering the wiring from manifest loading through resolveAgentHasSelfReport and collectDoctorChecks: load agent manifests both with and without self_report, then assert the resulting Serving process check reflects each manifest’s self-report configuration. Do not manually supply the self-report flag; exercise the actual doctor-to-agent path so regressions in either symbol are detected.
🤖 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 391-397: Add an integration-style doctor test covering the wiring
from manifest loading through resolveAgentHasSelfReport and collectDoctorChecks:
load agent manifests both with and without self_report, then assert the
resulting Serving process check reflects each manifest’s self-report
configuration. Do not manually supply the self-report flag; exercise the actual
doctor-to-agent path so regressions in either symbol are detected.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 67b9aceb-f046-4db7-9ebb-038aaa4f8585
📒 Files selected for processing (4)
src/lib/actions/sandbox/doctor-inference.test.tssrc/lib/actions/sandbox/doctor-inference.tssrc/lib/actions/sandbox/doctor.tssrc/lib/agent/defs.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/actions/sandbox/doctor-inference.ts
Refs #7003 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
apurvvkumaria
requested changes
Jul 17, 2026
apurvvkumaria
left a comment
apurvvkumaria
left a comment
Collaborator
There was a problem hiding this comment.
Exact-head review of 483b28833a728d432cbc108190d85fbc1f30c5c8 found that the declared feature is not wired through its observable contract.
When an agent declares self_report, Doctor suppresses the honest “Serving process: not checked” result, but no endpoint is probed. Status still unconditionally returns { checked: false }. The two surfaces therefore disagree, and the PR's claim that adding the manifest field wires health automatically is not true.
Please choose and implement one coherent contract:
- If this PR is manifest plumbing only, keep both Status and Doctor explicitly “not checked,” remove the automatic-health claim, and defer the health result types until a probe contract exists.
- If this PR delivers serving-process health, define and implement the response schema, success criteria, bounded timeout, bounded error detail, and failure mapping, then exercise the real manifest-to-Status/Doctor path for healthy, unhealthy, timeout, and malformed responses.
The parser must also constrain the future request boundary before it is usable. It currently accepts arbitrary or malformed URLs and zero, negative, or effectively unbounded finite timeouts. Validate an explicit safe endpoint scope (for example, a tightly defined sandbox-local origin/path), reject unsupported schemes/hosts and invalid timeout ranges, and add negative tests. Do not allow this declaration to become a generic SSRF or stall primitive when probing is connected.
Co-authored-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Co-authored-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-7007.docs.buildwithfern.com/nemoclaw |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/actions/sandbox/status-snapshot.ts (1)
404-410: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftProbe declared
self_reportendpoints instead of always reporting unchecked.The PR objective says declared endpoints are supported, but neither
statusnordoctorconsumesAgentDefinition.selfReport; configured and unconfigured gateway agents therefore produce identical results.
src/lib/actions/sandbox/status-snapshot.ts#L404-L410: probe a declared endpoint with its configured timeout and reserve{ checked: false }for missing declarations.src/lib/actions/sandbox/doctor.ts#L390-L417: propagate the resolved self-report configuration, not only the runtime kind.src/lib/actions/sandbox/doctor-inference.ts#L161-L173: map the probe result into the informational/healthy/failure diagnostic contract.src/lib/actions/sandbox/doctor-inference.test.ts#L167-L185: add declared-endpoint success and failure coverage.src/lib/actions/sandbox/doctor-flow.test.ts#L270-L302: stop asserting identical behavior with and withoutselfReport.docs/reference/commands.mdx#L1198-L1200: document checked results for declared endpoints.docs/reference/commands.mdx#L1361-L1362: document the corresponding doctor behavior.🤖 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.ts` around lines 404 - 410, Replace the unconditional unchecked serving-process result in src/lib/actions/sandbox/status-snapshot.ts:404-410 by probing declared selfReport endpoints with their configured timeout, retaining { checked: false } only when no declaration exists. In src/lib/actions/sandbox/doctor.ts:390-417, propagate resolved self-report configuration; in src/lib/actions/sandbox/doctor-inference.ts:161-173, map probe outcomes to the informational, healthy, and failure diagnostic contract. Add success and failure coverage in src/lib/actions/sandbox/doctor-inference.test.ts:167-185, update differing configured/unconfigured expectations in src/lib/actions/sandbox/doctor-flow.test.ts:270-302, and document checked endpoint results and corresponding doctor behavior in docs/reference/commands.mdx:1198-1200 and docs/reference/commands.mdx:1361-1362.
🤖 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.
Outside diff comments:
In `@src/lib/actions/sandbox/status-snapshot.ts`:
- Around line 404-410: Replace the unconditional unchecked serving-process
result in src/lib/actions/sandbox/status-snapshot.ts:404-410 by probing declared
selfReport endpoints with their configured timeout, retaining { checked: false }
only when no declaration exists. In src/lib/actions/sandbox/doctor.ts:390-417,
propagate resolved self-report configuration; in
src/lib/actions/sandbox/doctor-inference.ts:161-173, map probe outcomes to the
informational, healthy, and failure diagnostic contract. Add success and failure
coverage in src/lib/actions/sandbox/doctor-inference.test.ts:167-185, update
differing configured/unconfigured expectations in
src/lib/actions/sandbox/doctor-flow.test.ts:270-302, and document checked
endpoint results and corresponding doctor behavior in
docs/reference/commands.mdx:1198-1200 and docs/reference/commands.mdx:1361-1362.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: fbe21972-e60e-4ae3-a9a6-adae514f3365
📒 Files selected for processing (12)
docs/reference/commands.mdxsrc/lib/actions/sandbox/doctor-flow.test.tssrc/lib/actions/sandbox/doctor-inference.test.tssrc/lib/actions/sandbox/doctor-inference.tssrc/lib/actions/sandbox/doctor.tssrc/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status-inference.test.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/actions/sandbox/status-text.tssrc/lib/agent/defs.test.tssrc/lib/agent/manifest-readers.tstest/support/status-flow-test-harness.ts
apurvvkumaria
approved these changes
Jul 17, 2026
apurvvkumaria
left a comment
apurvvkumaria
left a comment
Collaborator
There was a problem hiding this comment.
Approved at exact head 9fe7989 after the serving-health contract was made plumbing-only, the self_report parser boundary was hardened, focused tests passed 115/115, CLI typecheck and docs validation passed, all ordinary CI and both automated advisors passed, and all review threads were resolved. Protected E2E remains coordinated separately for this exact revision.
Co-authored-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
wscurran
added
area: cli
# Conflicts: # docs/reference/commands.mdx
cv
approved these changes
Jul 18, 2026
cv
left a comment
cv
left a comment
Collaborator
There was a problem hiding this comment.
Approved at exact head eddacaf after refreshing onto current main, removing the unsupported self_report product contract, and adding unavailable-gateway status/doctor coverage. All 53 current checks and selected E2E signals pass; the deterministic maintainer gate reports allPass with clean merge state, DCO, verified commits, and no major CodeRabbit findings.
21 tasks
ericksoa
added a commit
that referenced
this pull request
Jul 18, 2026
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical `docs/changelog/2026-07-18.mdx` release-prep entry with the exact `## v0.0.88` heading. The entry summarizes every user-visible change on `main` since v0.0.87 and links each release theme to the focused user documentation. ## Changes - Add one parser-safe dated changelog entry for v0.0.88 covering DGX Station preparation, inference health, multi-gateway sandbox operations and recovery, onboarding policy defaults, and rebuild credential reuse. - Reconcile the changelog against the merged v0.0.88-labeled PRs and the complete `v0.0.87..origin/main` commit range. - Source mapping: - [#7152](#7152) -> `docs/changelog/2026-07-18.mdx`: Document RDMA-aware OpenIB service remediation during DGX Station preparation. - [#7155](#7155) -> `docs/changelog/2026-07-18.mdx`: Document stopped-container preservation and fail-closed restart-policy boundaries. - [#7158](#7158) -> `docs/changelog/2026-07-18.mdx`: Document bounded packaged CDI refresh for the exact AI Developer Tools Station profile. - [#7074](#7074) -> `docs/changelog/2026-07-18.mdx`: Document authenticated upstream model probes and precise route-reachability claims. - [#7007](#7007) -> `docs/changelog/2026-07-18.mdx`: Document the explicit serving-process health gap in `status` and `doctor`. - [#7113](#7113) -> `docs/changelog/2026-07-18.mdx`: Document owning-gateway selection for sandbox-scoped status and exec operations. - [#7092](#7092) -> `docs/changelog/2026-07-18.mdx`: Document idempotent recovery for target-owned active port forwards. - [#7133](#7133) -> `docs/changelog/2026-07-18.mdx`: Document web-search-aware policy preset defaults during onboarding. - [#7129](#7129) -> `docs/changelog/2026-07-18.mdx`: Document gateway-registered web-search credential reuse during rebuild preflight. ## 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 <!-- Check one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog contract, exact release heading, and parser-safe MDX structure. - [ ] 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 <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes 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 — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — completed successfully with 0 errors and 2 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) — not applicable because native changelog entries use the required parser-safe MDX SPDX comment without frontmatter. --- Signed-off-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added improved DGX Station preparation workflows. * Enhanced sandbox status and diagnostic reporting for inference health. * Improved state selection and recovery across multiple gateways. * Added safer onboarding defaults for web search policies. * Improved rebuild preflight handling for credential reuse and fail-closed behavior. * **Documentation** * Added release notes for version 0.0.88. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
statusanddoctorprobe inference health through fresh sandbox exec calls, which cannot attest that a long-running gateway process has equivalent inference access. This PR adds an explicitServing process: not checkedsignal for gateway runtimes. Terminal runtimes omit this signal because they have no long-running gateway process.A concrete serving-process probe remains deferred until its response schema, probe protocol, and failure mapping are designed and accepted.
Related Issue
Refs #7003
Changes
statusoutput — reachable gateway runtimes reportservingProcessHealth: { checked: false }and printServing process (<agent> gateway): not checked. Terminal runtimes reportnulland omit the line.doctoroutput — gateway runtimes receive an informationalServing process: not checkedcheck. Terminal runtimes omit it.{ checked: true, ... }state. The process-owned probe contract remains a separate design decision.Type of Change
Testing
npm run typecheck:clipassed.npm run docspassed with 0 errors.Checklist
General
Code Changes
docs/reference/commands.mdx.Signed-off-by: Dongni Yang dongniy@nvidia.com