fix(status): emit JSON for status flag (Fixes #2790)#2822
Conversation
Fixes NVIDIA#2790 Signed-off-by: Deepak Jain <deepujain@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe PR adds a ChangesStatus JSON Output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/nemoclaw.ts (1)
1572-1605: Optional validation step: run the recommended E2E jobs for sandbox lifecycle confidence.Even though this PR is status-focused, this file is central to sandbox operations; running
sandbox-survival-e2eandsandbox-operations-e2eis a good safety net before merge.As per coding guidelines: "
src/nemoclaw.ts: This file contains CLI dispatch, status, recovery, and connect functions... E2E test recommendation: sandbox-survival-e2e, sandbox-operations-e2e."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/nemoclaw.ts` around lines 1572 - 1605, Add a short validation step to run the recommended sandbox E2E jobs before merging: run the `sandbox-survival-e2e` and `sandbox-operations-e2e` pipelines to gain confidence in sandbox lifecycle changes. Specifically, after the `showStatus`/CLI behavior is updated (look at the showStatus function, its use of `buildStatusJson()` and the call into `showStatusCommand` from `./lib/services`), trigger or document running these two E2E jobs and verify they pass; if either fails, investigate interactions between `showStatus`, `getLiveGatewayInference`, `checkMessagingBridgeHealth`, and `backfillAndFindOverlaps` before merging. Ensure the PR description or CI configuration notes that both E2E checks were executed and passed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/nemoclaw.ts`:
- Around line 1572-1605: Add a short validation step to run the recommended
sandbox E2E jobs before merging: run the `sandbox-survival-e2e` and
`sandbox-operations-e2e` pipelines to gain confidence in sandbox lifecycle
changes. Specifically, after the `showStatus`/CLI behavior is updated (look at
the showStatus function, its use of `buildStatusJson()` and the call into
`showStatusCommand` from `./lib/services`), trigger or document running these
two E2E jobs and verify they pass; if either fails, investigate interactions
between `showStatus`, `getLiveGatewayInference`, `checkMessagingBridgeHealth`,
and `backfillAndFindOverlaps` before merging. Ensure the PR description or CI
configuration notes that both E2E checks were executed and passed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bd3c56a7-8b29-4cb4-9aa2-ea79073e5781
📒 Files selected for processing (4)
src/lib/command-registry.tssrc/lib/services.tssrc/nemoclaw.tstest/cli.test.ts
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed updated head d18b593 after the conflict-resolution port onto current main. The fix is scoped to #2790: global status --json now goes through the current oclif status command, returns structured JSON from registry/live inference/service helpers, and preserves human status output. The JSON projection avoids credential hashes, raw logs, dashboard tokens, bearer/API key patterns, and messaging tokens.
Validation I ran locally on this exact tree: npm run build:cli; npx vitest run test/cli.test.ts -t status; npx vitest run src/lib/inventory-commands.test.ts; npm run typecheck:cli; npm run typecheck; npx biome check touched files; git diff --check. No blocking issues found.
## Summary Ports the useful behavior from #2626 onto current main without depending on #2881 or #2822. - reconcile sandbox/gateway state before probing or printing inference provider health - suppress `Inference: healthy` unless the selected sandbox/gateway is verified present - exit non-zero for degraded or unverified sandbox/gateway status states - remove only the selected stale registry entry when status verifies that named sandbox is absent from a healthy NemoClaw gateway; status does not sweep unrelated registry-only entries - keep `nemoclaw list` rendering registered sandboxes when runtime inference probing is degraded Fixes #2595 Refs #2666 Refs #2604 ## Validation - `npm run build:cli` - `npx vitest run test/cli.test.ts -t "status|runtime inference probing"` - `npx vitest run test/cli.test.ts -t "gateway|inference|orphan|verified"` - `npx vitest run test/cli.test.ts` - `npm run typecheck:cli` - `npm run typecheck` - `npx biome lint src/nemoclaw.ts test/cli.test.ts` - `git diff --check` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Automatic generation and application of an initial sandbox create policy based on enabled messaging channels; merged policy presets are applied at sandbox boot. * **Bug Fixes** * Status now only reports inference when gateway/sandbox state is verified; otherwise shows "Inference: not verified..." and exits non-zero for unverified states. * Status removes the selected stale/orphaned registry entry only when that named sandbox is verified absent from a healthy NemoClaw gateway; it does not sweep unrelated registry-only entries. List/status gracefully fall back when inference probing is degraded. * **Tests** * CLI, e2e, and unit tests updated to expect non-zero exits and verification-gated inference reporting; policy merge behavior covered. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
## Summary Catch up the docs for user-facing changes that landed over the weekend and today, so the published guidance matches current installer, onboarding, status, logs, local inference, rebuild backup behavior, the next docs version selector, and refreshed generated user skills. ## Related Issue None. ## Changes - Document WSL Windows-host Ollama onboarding actions, including use, start, restart, install, and `host.docker.internal` model pulls from #2800; clarify that onboard owns Ollama install and model pulls from #2952. - Document installer fail-fast behavior for non-TTY third-party software acceptance from #2706. - Update sandbox-name guidance and Brev deploy validation behavior from #2948. - Add `nemoclaw <name> logs --tail/--since` coverage from #2825. - Add global `nemoclaw status --json` coverage from #2822. - Document verified-gateway status behavior and non-zero degraded exits from #2884. - Clarify that rebuild stops before deleting the original sandbox when backup fails, including unreadable or root-owned state paths. - Bump docs switcher metadata from 0.0.33 to 0.0.34 without changing package versions or creating release tags. - Regenerate `.agents/skills/nemoclaw-user-*` from docs, including the new `nemoclaw-user-manage-sandboxes` generated skill and removal of the stale `nemoclaw-user-workspace` output. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [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) --- <!-- DCO sign-off required by CI. Run: git config user.name && git config user.email --> Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded Ollama/local inference guidance with detailed WSL and Windows-host workflows, proxy/token behavior, and onboarding options * Standardized sandbox name validation and updated troubleshooting, CLI, and deploy docs to surface the rules and validation timing * Added/rewrote Manage Sandboxes, policy management, backup/restore, messaging channels, workspace persistence, and CLI selection guides * Refreshed quickstart/Hermes guidance, skill mappings, and bumped docs version to 0.0.34 <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary `nemoclaw <name> status` now accepts `--json` and emits a structured per-sandbox report, mirroring what the global `nemoclaw status --json` (added in #2790 / #2822) already exposes. Automation can read `hostGpuDetected`, `sandboxGpuEnabled`, `sandboxGpuMode`, `sandboxGpuDevice`, `openshellDriver`, and `openshellVersion` for a specific sandbox without scraping the text renderer. ## Related Issue Fixes #4310. Enhances #2790 (resolved by #2822) by extending the JSON renderer to the per-sandbox status variant. ## Changes - `src/commands/sandbox/status.ts` opts into `static enableJsonFlag = true`, updates the usage/examples to advertise `--json`, and branches on `this.jsonEnabled()` so the existing text path is unchanged. - New `getSandboxStatusReport(sandboxName)` in `src/lib/actions/sandbox/status.ts` returns a `SandboxStatusReport`: `schemaVersion`, `name`, `found`, `model`, `provider`, `phase`, `gatewayState`, `inferenceHealth`, `hostGpuDetected`, `sandboxGpuEnabled`, `sandboxGpuMode`, `sandboxGpuDevice`, `openshellDriver`, `openshellVersion`, `policies`. `openshellDriver` and `openshellVersion` are normalised to the string `"unknown"` when missing so consumers can rely on `typeof` checks. - The JSON path sets `process.exitCode = 1` when the sandbox is missing locally or the gateway state is not `present`, so scripts can distinguish "ready" from "drift/unreachable" without parsing the body. - `test/cli.test.ts` covers the populated JSON shape, the `"unknown"` string fallback for `openshellDriver` / `openshellVersion`, the protobuf-mismatch path (asserts exit code `1`, `rpcIssue = { kind: "protobuf_mismatch" }`, `inferenceHealth = null`, and `model` / `provider` = `"unknown"`), and the `--help` advertising `--json`. - `collectSandboxStatusSnapshot` is shared by `showSandboxStatus` and `getSandboxStatusReport`; it fail-closes on `detectOpenShellStateRpcResultIssue` so the JSON path no longer emits inferred `model` / `provider` / `inferenceHealth` alongside an `rpcIssue`. The host-side gateway-chain subprobe is appended in the collector so both text and JSON paths surface it. - `docs/reference/commands.mdx` documents the `--json` flag in the per-sandbox status section (fields, fallback, exit-code contract) so the CLI/docs parity check stays green. ## 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) ## Verification - [X] `npx prek run --all-files` passes - [X] `npm test` passes - [X] Tests added or updated for new or changed behavior - [X] No secrets, API keys, or credentials committed - [X] Docs updated for user-facing behavior changes - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] 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) --- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * `sandbox status` gains a `--json` mode that emits a structured per-sandbox JSON report (gateway state, inference health, GPU details, policies, model/provider, OpenShell driver/version). * **Behavior** * `openshellDriver`/`openshellVersion` default to `"unknown"` when unset. * `--json` returns structured data and sets non‑zero exit codes for missing sandbox, non‑present gateway, or RPC/schema issues; text output unchanged without `--json`. * **Tests** * Added coverage for JSON output, defaults, RPC issue handling, exit codes, and help text. * **Documentation** * Command docs updated with `--json` behavior, fields, exit conditions, and examples. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/4323?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com>
Summary
Fixes
nemoclaw status --jsonso it emits structured JSON instead of the human status view.Changes
status --jsoncommand.Testing
npm install --ignore-scriptsnpm run build:clinpm run typecheck:clinpm test -- test/cli.test.tscd nemoclawthennpm install --ignore-scriptscd nemoclawthennpm run buildnpm test -- test/ssrf-parity.test.ts nemoclaw/src/commands/migration-state.test.tsnode bin/nemoclaw.js status --jsonnpm testwas also run; the remaining local failures are the existing installer/uninstall source-layout suites and are unrelated to this status JSON change.Evidence it works
The new test seeds provider credential hashes and messaging-channel metadata, runs
status --json, parses stdout withJSON.parse, and asserts the output does not contain credential patterns such as bearer tokens, API keys, or Slack-style tokens.Fixes #2790
Signed-off-by: Deepak Jain deepujain@gmail.com
Summary by CodeRabbit
statuscommand now supports a--jsonflag for structured JSON output, enabling programmatic access to sandbox and service status information, including schema version, default sandbox configuration, live inference details, and service states.