Skip to content

refactor(cli): improve sandbox diagnostic command metadata - #2906

Merged
cv merged 15 commits into
mainfrom
refactor/oclif-ux-sandbox-diagnostics
May 4, 2026
Merged

refactor(cli): improve sandbox diagnostic command metadata#2906
cv merged 15 commits into
mainfrom
refactor/oclif-ux-sandbox-diagnostics

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Improve the oclif metadata and parser-owned validation for read-only sandbox diagnostic commands. This keeps public sandbox-scoped help output stable while making doctor and config get validation stricter in the command adapter.

Stack Navigation

Changes

  • Added examples for connect, sandbox-scoped status, doctor, config get, policy-list, and channels list adapters.
  • Made sandbox:doctor a strict oclif command with a required sandbox arg and --json flag.
  • Moved config get --format validation into oclif with json|yaml options and removed the adapter-level manual format check.
  • Preserved public doctor --help output through legacy dispatch and expanded diagnostics validation coverage.
  • Updated the hidden command registry metadata for config get flags.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features

    • Added command usage examples to help documentation for configuration, status, policy, and channels commands.
    • Enhanced doctor command with explicit --json flag support.
  • Improvements

    • Updated configuration command help text to clearly advertise available options (--key and --format json|yaml).
    • Restricted --format flag to json and yaml values with stricter validation.

@cv cv self-assigned this May 3, 2026
@copy-pr-bot

copy-pr-bot Bot commented May 3, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 716c42fd-0b88-4ec4-aa97-8420fb0b0b43

📥 Commits

Reviewing files that changed from the base of the PR and between 12a3850 and 00cbd63.

📒 Files selected for processing (7)
  • src/lib/command-registry.ts
  • src/lib/connect-cli-command.ts
  • src/lib/legacy-oclif-dispatch.test.ts
  • src/lib/legacy-oclif-dispatch.ts
  • src/lib/sandbox-doctor-cli-command.ts
  • src/lib/sandbox-inspection-cli-command.ts
  • test/cli.test.ts

📝 Walkthrough

Walkthrough

The PR removes legacy dispatch handling and migrates sandbox commands to oclif-based argument and flag parsing with constrained options. The LegacyDispatch type is eliminated, doctor and logs commands gain explicit help handling, and CLI commands now declare arguments and flags explicitly instead of processing argv manually.

Changes

Command Parsing Modernization & Legacy Dispatch Removal

Layer / File(s) Summary
Type & Dispatch Updates
src/lib/legacy-oclif-dispatch.ts
LegacyDispatch type removed; DispatchResult simplified to exclude legacy variants. doctor gains explicit help dispatch returning usage: "doctor [--json]"; logs help text expanded to document --follow, --tail, --since.
Command Parsing Implementation
src/lib/sandbox-doctor-cli-command.ts, src/lib/sandbox-inspection-cli-command.ts
SandboxDoctorCliCommand migrates to oclif Args/Flags with strict parsing; SandboxConfigGetCommand --format flag constrained to ["json", "yaml"] via oclif options; runtime validation removed.
CLI Metadata
src/lib/command-registry.ts, src/lib/connect-cli-command.ts
nemoclaw <name> config get command registry updated with flags metadata (`[--key ] [--format json
Command Examples & Metadata
src/lib/sandbox-inspection-cli-command.ts
SandboxStatusCommand, SandboxPolicyListCommand, and SandboxChannelsListCommand gain static examples entries. SandboxConfigGetCommand examples expanded.
Test Coverage
src/lib/legacy-oclif-dispatch.test.ts, test/cli.test.ts
New test validates doctor --help returns help dispatch. CLI test expectations updated for alpha doctor --help output; alpha config get --help now requires `--format json

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 The legacy chains now break and fade,
Each command stands in oclif's shade,
With flags that sparkle, constrained and bright,
We parse before we dispatch right!
No more wild argv, just structured delight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately reflects the primary changes: adding examples and improving oclif metadata for sandbox diagnostic commands (doctor, config get, status, policy-list, channels list), while also refactoring dispatch logic and validation rules.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/oclif-ux-sandbox-diagnostics

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

@cv cv added the v0.0.34 label May 4, 2026
cv added a commit that referenced this pull request May 4, 2026
## Summary
Move the sandbox logs command's `--tail`, `-n`, `--since`, and
`--follow` option shape into the oclif adapter so invalid flag values
fail before any OpenShell calls. This also carries the log filter
options through the logs action and updates help/registry metadata for
the new flags.

## Stack Navigation
- Position: 13 of 60
- Previous PR: [#2902 — refactor(cli): add oclif examples for utility
commands](#2902)
- Next PR: [#2906 — refactor(cli): improve sandbox diagnostic command
metadata](#2906)

## Changes
- Added `SandboxLogsOptions` and wired `sandbox:logs` to pass structured
log options instead of a bare follow boolean.
- Modeled `--tail`/`-n` with `Flags.integer({ min: 1 })` and kept custom
`--since` duration validation in the oclif command.
- Updated log source argument construction so tail counts flow to both
OpenClaw and OpenShell logs, while `--since` avoids an unfiltered
gateway tail.
- Expanded logs help metadata and root command registry flags.
- Added CLI and command-adapter coverage for tail, since, follow, help,
malformed values, and unknown flags.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## 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
- [ ] Docs updated for user-facing behavior changes
- [ ] `make 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: Carlos Villela <cvillela@nvidia.com>


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

## Release Notes

* **New Features**
* The `sandbox:logs` command now supports `--tail/-n` flag to display a
specific number of log lines
* Added `--since` flag to filter logs from a specified duration with
validation and helpful error messages
  * Updated command documentation with usage examples
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv changed the base branch from refactor/oclif-ux-logs-flags to main May 4, 2026 22:34
@cv
cv marked this pull request as ready for review May 4, 2026 22:34
@cv
cv enabled auto-merge (squash) May 4, 2026 22:34
Signed-off-by: Carlos Villela <cvillela@nvidia.com>

@prekshivyas prekshivyas 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.

LGTM after rebase. 7 files / +61 / -9 — small, well-tested.

Two intentional validation tightenings (both declared in the PR description, both well-covered by tests):

  • SandboxDoctorCliCommand: strict = falsestrict = true with explicit Args.string for sandboxName and Flags.boolean for --json. Unknown flags like --bogus are now rejected at parse time instead of silently ignored. Test added.
  • SandboxConfigGetCommand.--format: manual format check replaced with oclif's options: ["json", "yaml"]. Same non-zero exit, slightly different error message wording (still contains --format, json, yaml). Test added.

Plus examples added to connect/status/doctor/config get/policy-list/channels list and doctor --help now routes through legacy dispatch's help branch so user-facing help stays free of sandbox:doctor command-id leakage.

src/nemoclaw.ts untouched — cumulative ~16-orphan debt unchanged. CI: pr.yaml mostly green; pr-self-hosted builds and wsl-e2e/checks still in flight at review time. No failures.

@cv
cv merged commit 6cb0e1a into main May 4, 2026
13 of 15 checks passed
cv added a commit that referenced this pull request May 4, 2026
## Summary
Tighten oclif-owned validation and help metadata for policy and
messaging channel mutation commands. This moves missing custom policy
path and channel-name validation into strict command parsing before
action dispatch.

## Stack Navigation
- Position: 15 of 60
- Previous PR: [#2906 — refactor(cli): improve sandbox diagnostic
command metadata](#2906)
- Next PR: [#2908 — refactor(cli): improve snapshot command
metadata](#2908)

## Changes
- Added examples for `policy-add`, `policy-remove`, and channel
add/remove/start/stop commands.
- Removed the hidden raw `policy-add` adapter so missing `--from-file`
and `--from-dir` values are handled by oclif.
- Made channel mutation commands require a `<channel>` arg before
dispatch.
- Updated command registry metadata for channel mutation arguments.
- Added unit and CLI coverage for missing parser-owned values.

## Type of Change
- [x] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## 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
- [ ] Docs updated for user-facing behavior changes
- [ ] `make 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: Carlos Villela <cvillela@nvidia.com>


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

* **New Features**
* Added usage examples to channel and policy mutation command help
documentation.

* **Bug Fixes**
* Channel mutation commands now enforce the channel argument as
required.
* Policy add command validates that `--from-file` flag includes a path
value.

* **Chores**
  * Removed legacy command variant for policy operations.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv deleted the refactor/oclif-ux-sandbox-diagnostics branch May 27, 2026 21:18
@wscurran wscurran added the refactor PR restructures code without intended behavior change label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants