fix(cli): clarify status command scope - #5540
Conversation
Signed-off-by: HwangJohn <angelic805@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR clarifies global versus sandbox-scoped ChangesGlobal vs sandbox-scoped status clarity
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/reference/commands.mdx (1)
1743-1743: ⚡ Quick winReplace clause-level colon with sentence punctuation.
Line 1743 uses a colon between clauses instead of introducing a list, which violates the docs punctuation rule.
As per coding guidelines, "Colons should only introduce a list."
🤖 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 `@docs/reference/commands.mdx` at line 1743, The sentence in the docs file uses a colon after "host-wide" to separate two independent clauses rather than to introduce a list, which violates the punctuation guideline that colons should only introduce lists. Replace the colon separating "This command is host-wide:" from the rest of the sentence with a period to create two separate sentences, or restructure the sentence entirely to remove the clause-separating colon while maintaining clear documentation of what the command does.Source: Coding guidelines
docs/reference/commands-nemohermes.mdx (1)
1440-1440: ⚡ Quick winReplace clause-separating colon in this sentence.
Line 1440 uses a colon as general punctuation (
host-wide: it summarizes ...), but this style guide reserves colons for introducing lists.
Please rewrite this sentence with a period/comma (and update the generator source file so regeneration preserves it).As per coding guidelines, "Colons should only introduce a list. Flag colons used as general punctuation between clauses."
🤖 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 `@docs/reference/commands-nemohermes.mdx` at line 1440, The sentence at line 1440 uses a colon as general clause-separating punctuation ("host-wide: it summarizes..."), but according to the style guide, colons should only be used to introduce lists. Replace the colon after "host-wide" with a period or comma to properly separate the two independent clauses. Additionally, locate and update the generator source file that produces this documentation (not just the markdown file itself) so that when the documentation is regenerated, this punctuation change is preserved and not overwritten.Source: Coding guidelines
🤖 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 `@docs/reference/commands-nemohermes.mdx`:
- Line 1440: The sentence at line 1440 uses a colon as general clause-separating
punctuation ("host-wide: it summarizes..."), but according to the style guide,
colons should only be used to introduce lists. Replace the colon after
"host-wide" with a period or comma to properly separate the two independent
clauses. Additionally, locate and update the generator source file that produces
this documentation (not just the markdown file itself) so that when the
documentation is regenerated, this punctuation change is preserved and not
overwritten.
In `@docs/reference/commands.mdx`:
- Line 1743: The sentence in the docs file uses a colon after "host-wide" to
separate two independent clauses rather than to introduce a list, which violates
the punctuation guideline that colons should only introduce lists. Replace the
colon separating "This command is host-wide:" from the rest of the sentence with
a period to create two separate sentences, or restructure the sentence entirely
to remove the clause-separating colon while maintaining clear documentation of
what the command does.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 52b5da29-c1ed-4b87-a2d4-6a71b76926d4
📒 Files selected for processing (15)
docs/reference/cli-selection-guide.mdxdocs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxsrc/commands/sandbox/status.tssrc/commands/status.tssrc/lib/actions/root-help.tssrc/lib/actions/sandbox/status-flow.test.tssrc/lib/actions/sandbox/status.tssrc/lib/cli/public-dispatch.tssrc/lib/cli/public-display-defaults.tssrc/lib/inventory/index.test.tssrc/lib/inventory/index.tstest/cli-oclif-compatibility.test.tstest/cli/status-routing.test.tstest/root-help.test.ts
66cba2e to
1bf105b
Compare
Signed-off-by: HwangJohn <angelic805@gmail.com>
1bf105b to
3884237
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/actions/sandbox/status.ts (1)
153-154: 🏗️ Heavy liftAvoid blanket complexity suppression for
showSandboxStatus.Line 153 and Line 491 disable the complexity rule for the whole function instead of reducing branching. Please split
showSandboxStatusinto smaller helpers (e.g., preflight/header render, lookup-state handling, and post-status diagnostics) and keep complexity lint active.As per coding guidelines:
**/*.{js,ts,jsx,tsx}: Keep function complexity low.Also applies to: 491-491
🤖 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.ts` around lines 153 - 154, The showSandboxStatus function has blanket eslint-disable complexity comments on lines 153 and 491 instead of addressing the actual complexity. Refactor the showSandboxStatus function by splitting it into smaller helper functions that handle specific concerns: one for preflight and header rendering, one for lookup-state handling, and one for post-status diagnostics. After refactoring the function logic into these smaller helpers, remove the eslint-disable complexity comments so the linter can properly enforce low complexity on the smaller, focused functions.Source: Coding guidelines
src/lib/cli/public-dispatch.ts (1)
310-312: 🏗️ Heavy liftRefactor
dispatchCliinstead of suppressing complexity checks.Line 310 and Line 420 suppress complexity for the full dispatcher path. Please extract the global/sandbox subflows into dedicated helpers so
dispatchClistays orchestration-only and complexity lint can remain enabled.As per coding guidelines:
**/*.{js,ts,jsx,tsx}: Keep function complexity low.Also applies to: 420-420
🤖 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/cli/public-dispatch.ts` around lines 310 - 312, The dispatchCli function in src/lib/cli/public-dispatch.ts uses an eslint-disable complexity comment to suppress complexity checks. Instead of suppressing the check, refactor the function by extracting the global and sandbox subflow logic into separate dedicated helper functions. Keep dispatchCli as a thin orchestration layer that delegates to these new helpers, then remove the eslint-disable complexity comment to re-enable the linting rule. This will improve code maintainability and keep the complexity of individual functions within acceptable limits.Source: Coding guidelines
🤖 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/status.ts`:
- Around line 153-154: The showSandboxStatus function has blanket eslint-disable
complexity comments on lines 153 and 491 instead of addressing the actual
complexity. Refactor the showSandboxStatus function by splitting it into smaller
helper functions that handle specific concerns: one for preflight and header
rendering, one for lookup-state handling, and one for post-status diagnostics.
After refactoring the function logic into these smaller helpers, remove the
eslint-disable complexity comments so the linter can properly enforce low
complexity on the smaller, focused functions.
In `@src/lib/cli/public-dispatch.ts`:
- Around line 310-312: The dispatchCli function in
src/lib/cli/public-dispatch.ts uses an eslint-disable complexity comment to
suppress complexity checks. Instead of suppressing the check, refactor the
function by extracting the global and sandbox subflow logic into separate
dedicated helper functions. Keep dispatchCli as a thin orchestration layer that
delegates to these new helpers, then remove the eslint-disable complexity
comment to re-enable the linting rule. This will improve code maintainability
and keep the complexity of individual functions within acceptable limits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3da3abae-63b4-4953-bbbe-df96d7723e16
📒 Files selected for processing (4)
docs/reference/commands-nemohermes.mdxdocs/reference/commands.mdxsrc/lib/actions/sandbox/status.tssrc/lib/cli/public-dispatch.ts
✅ Files skipped from review due to trivial changes (2)
- docs/reference/commands.mdx
- docs/reference/commands-nemohermes.mdx
Signed-off-by: HwangJohn <angelic805@gmail.com>
|
✨ Thanks for the proposed fix addressing the wrong-form Related open issues: |
) Merge origin/main into fix/754-status-scope-help. - src/lib/actions/sandbox/status.ts: adopt main's refactored version and re-apply the sandbox-scope heading in status-text.ts printSandboxDetails, where main relocated the sandbox detail rendering block. - test/root-help.test.ts: keep both the PR's global-vs-sandbox help test and main's onboard-agent help test. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ce (NVIDIA#5540) - status bogus --json (flag after the name) still preserves --json in the hint - status --bogus alpha surfaces the unknown-flag error, not the scope hint Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
/ok to test 3c99b2d |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
/ok to test c61098b |
|
Maintainer disposition for the exact-head PR Review Advisor run 28548396812 at
|
Vitest E2E Target Results — ✅ All selected jobs passedRun: 28548560495
|
cjagwani
left a comment
There was a problem hiding this comment.
Approving.
Fixes #754 — all three acceptance criteria verified against the live CLI:
- Top-level help explains global vs sandbox-targeted commands ("Global commands inspect host-wide state; sandbox commands start with a sandbox name").
statusoutput labels its scope ("Global status (registered sandboxes and host services):" vs "Sandbox-scoped status for '':").- Wrong-form
status <name>prints a concrete reorder hint and exits 2, preserving--jsonin the suggestion.
Security review clean — flag-aware argv parsing, the sandbox name is echoed only into a display hint (never executed), no injection or content leak. CI is fully green: CodeRabbit, cli-tests, static-checks, and every E2E lane pass; the GPT-5.5 advisor reports no required-before-merge items (the helper-extraction hotspot is reasonable follow-up, not a blocker). Coverage includes the wrong-form, flag-ordering, and unknown-flag-precedence cases.
|
Follow-up on the uncanceled exact-head dual-advisor rerun 28548785363: both GPT-5.5 and Nemotron completed with Nemotron's non-blocking items are dispositioned as follows:
No further code change is warranted from these advisory items. |
Vitest E2E Target Results — ✅ All requested jobs passedRun: 28548570748
|
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - #6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - #6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - #5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - #6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - #6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - #5980 and #5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - #6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - #6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - #5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - #6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - #6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - #6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - #6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - #5976 and #5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - #5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - #5978 and #6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary Clarifies the difference between global `status` and sandbox-scoped `<name> status` in help, docs, and text output. Wrong-form `nemoclaw status <name>` now prints an actionable reorder suggestion while preserving existing JSON behavior. ## Related Issue Fixes NVIDIA#754 ## Changes - Update global and sandbox status command summaries, descriptions, examples, and root help. - Add a scope hint for wrong-form `status <name>` usage, including `--json` preservation. - Label global and sandbox text status output with explicit scope headings. - Update reference docs and CLI selection guidance. - Add and update routing, output, help, and compatibility tests. ## 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 Validated locally on Windows and on DGX Spark/Linux. `npm run docs` completed with 0 errors; Fern reported the existing 2 warnings. - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Git hooks passed during commit and push, or `npx prek run --from-ref main --to-ref HEAD` passes - [x] Targeted tests pass for changed behavior - [ ] Full `npm test` passes (broad runtime changes only) - [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) - [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) --- Signed-off-by: HwangJohn <angelic805@gmail.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved CLI help and reference text to clearly distinguish global `status` from per-sandbox `<name> status`, with updated examples and scope hints. * Enhanced status display labeling, including a dedicated global status header for registered sandboxes and host services. * **Bug Fixes** * Added clearer handling when a sandbox-like argument is provided to the global `status` command, showing a global-only usage hint and exiting with the correct error code. * **Tests** * Updated and expanded status-routing and output-label assertions to match the revised help text and formatting. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: HwangJohn <angelic805@gmail.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Charan Jagwani <cjagwani@nvidia.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
## Summary - Add the `v0.0.72` release-note section with links to the deeper docs pages for installer recovery, command diagnostics, inference, policy, and sandbox repair changes. - Document the custom preset `allowed_ips` guard for user-authored policy files. ## Related Issue None. ## Source summary - NVIDIA#6132 -> `docs/about/release-notes.mdx`: Summarizes installer and upgrade recovery before generic onboarding, with links to quickstart and lifecycle docs. - NVIDIA#6087 -> `docs/network-policy/customize-network-policy.mdx`: Documents that user-authored custom presets reject `allowed_ips` for ordinary endpoints; also summarized in release notes. - NVIDIA#5975 -> `docs/about/release-notes.mdx`: Summarizes safer curl-based inference probes that keep API keys out of process arguments. - NVIDIA#6044 -> `docs/about/release-notes.mdx`: Summarizes compact `channels status` configuration reporting. - NVIDIA#6096 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw EC2 metadata discovery disablement and links to security guidance. - NVIDIA#5980 and NVIDIA#5991 -> `docs/about/release-notes.mdx`: Summarizes `exec` multiline argument rejection and recovery guidance. - NVIDIA#6023 -> `docs/about/release-notes.mdx`: Summarizes registered-provider diagnostics for `inference set` failures. - NVIDIA#6074 -> `docs/about/release-notes.mdx`: Summarizes the refreshed NVIDIA Endpoints featured-model selection behavior. - NVIDIA#5969 -> `docs/about/release-notes.mdx`: Summarizes `credentials add` provider credential registration. - NVIDIA#6060 -> `docs/about/release-notes.mdx`: Summarizes mutable OpenClaw config permission restoration after `exec`. - NVIDIA#6134 -> `docs/about/release-notes.mdx`: Summarizes restored Tavily access for managed Python workflows. - NVIDIA#6089 -> `docs/about/release-notes.mdx`: Summarizes Hermes runtime version-scheme comparison during upgrade checks. - NVIDIA#6131 -> `docs/about/release-notes.mdx`: Summarizes OpenClaw gateway watchdog recovery behavior. - NVIDIA#5976 and NVIDIA#5990 -> `docs/about/release-notes.mdx`: Summarizes prompt stdin EOF cancellation behavior during onboarding. - NVIDIA#5540 -> `docs/about/release-notes.mdx`: Summarizes clarified host-level and per-sandbox status command scope. - NVIDIA#5978 and NVIDIA#6018 -> `docs/about/release-notes.mdx`: Summarizes policy-denial log breadcrumbs in connect shells. ## Testing - `npm run docs:sync-agent-variants` - `npm run docs` - Commit hooks passed during `git commit`, including commitlint and gitleaks. - Pre-push hook passed during `git push`, including TypeScript CLI and package/tag version sync. ## Checklist - [x] Documentation updated. - [x] `npm run docs` completed with 0 errors and 1 existing Fern warning. - [x] No source code or generated build artifacts committed. Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added release notes for v0.0.72 covering improved installer recovery, clearer CLI diagnostics, safer inference setup and provider switching, better credential handling, stronger policy boundaries, and more robust runtime repair behavior. * Updated network policy guidance to clarify when `allowed_ips` can be used, including a specific exception for the sandbox-to-host bridge endpoint. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Clarifies the difference between global
statusand sandbox-scoped<name> statusin help, docs, and text output. Wrong-formnemoclaw status <name>now prints an actionable reorder suggestion while preserving existing JSON behavior.Related Issue
Fixes #754
Changes
status <name>usage, including--jsonpreservation.Type of Change
Verification
Validated locally on Windows and on DGX Spark/Linux.
npm run docscompleted with 0 errors; Fern reported the existing 2 warnings.Verifiedin GitHubnpx prek run --from-ref main --to-ref HEADpassesnpm testpasses (broad runtime changes only)npm run docsbuilds without warnings (doc changes only)Signed-off-by: HwangJohn angelic805@gmail.com
Summary by CodeRabbit
New Features
statusfrom per-sandbox<name> status, with updated examples and scope hints.Bug Fixes
statuscommand, showing a global-only usage hint and exiting with the correct error code.Tests