Skip to content

refactor(cli): use oclif summaries in root help - #2922

Merged
cv merged 33 commits into
mainfrom
refactor/oclif-arch-root-help-metadata
May 5, 2026
Merged

refactor(cli): use oclif summaries in root help#2922
cv merged 33 commits into
mainfrom
refactor/oclif-arch-root-help-metadata

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Start generating root help content from oclif command metadata by using registered command summaries for visible help descriptions. The public usage/group display remains driven by the NemoClaw display metadata.

Stack Navigation

Changes

  • Added an oclif metadata lookup helper.
  • Updated root help rendering to prefer each command class summary by command ID.
  • Added tests for metadata lookup behavior.

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

    • Help system now displays registered command summaries alongside command descriptions, providing users with more detailed information when viewing help.
  • Tests

    • Added test suite validating command metadata retrieval and lookup functionality.
    • Enhanced help output tests to verify additional descriptive information is displayed correctly.

cv added 30 commits May 2, 2026 13:36
@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

📝 Walkthrough

Walkthrough

A new oclif command metadata lookup system is introduced, allowing registered command summaries to be loaded dynamically and used in help output with fallback logic. The help action is updated to display command summaries from the registry when multiple commands share the same ID but have different descriptions.

Changes

Oclif Metadata Lookup & Help Integration

Layer / File(s) Summary
Data Shape
src/lib/oclif-metadata.ts
New OclifCommandMetadata type defines optional description, examples, summary, and usage fields for command metadata.
Core Implementation
src/lib/oclif-metadata.ts
loadOclifCommands() loads the command registry from two fallback module paths using require. getRegisteredOclifCommandMetadata() and getRegisteredOclifCommandSummary() provide lookup helpers that return null on failure.
Help Integration
src/lib/root-help-action.ts
Help action gains hasDisplaySpecificDescription() to detect when a commandId has multiple distinct descriptions, and getDisplayDescription() to return either the command's own description or the registry summary. The help() renderer uses the computed display description.
Tests
src/lib/oclif-metadata.test.ts, test/cli.test.ts
Unit tests for metadata lookup functions and assertions extended to verify help output includes command summaries and flags.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 A registry hops into view,
Commands wear summaries, tried and true—
When descriptions collide, the registry sings,
Help text ascends on metadata's wings! ✨

🚥 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 title 'refactor(cli): use oclif summaries in root help' accurately captures the main change: integrating oclif command metadata summaries into root help rendering instead of relying solely on display metadata.
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-arch-root-help-metadata

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

@cv

cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator Author

Automated PR review summary

Reviewed PR #2922: refactor(cli): use oclif summaries in root help

Recommendation

  • Recommendation: Requires Changes
  • Highest observed severity: medium
  • Block merge: yes
  • Why: The PR claims public usage/group display remains driven by NemoClaw display metadata while visible help descriptions come from oclif summaries. In the installed CLI, rows with the same commandId but intentionally different display descriptions are overwritten by the same summary. This makes root help misleading for nemoclaw onboard --from, hiding its custom Dockerfile purpose.
  • Reviewer summary: Adversarial review found one blocking regression: command-ID-only summary lookup collapses distinct root-help rows that share one oclif command class. Specifically, nemoclaw onboard --from is now described as Configure inference endpoint and credentials instead of the display metadata's Use a custom Dockerfile for the sandbox image.

Installation and setup findings

  • NemoClaw is installed from the local checkout and the NemoClaw-managed sandbox nvf-nemoclaw-local is Ready and SSH-verifiable. Caveats: direct Docker access in the original session still fails until group membership is refreshed, but sg docker works; the NVIDIA build key was not present, so provider=build was not validated; model inference through the sandbox timed out with a 503 using the mock-provider setup.

What was validated

  • The PR revision was checked out in an isolated review environment.
  • The local checkout was installed using the repository installer flow as closely as the environment allowed.
  • Adversarial, PR-specific probes were then run against the installed environment and relevant repository context.
  • Diff summary:
 Dockerfile                                         |   28 -
 Dockerfile.base                                    |   30 -
 docs/get-started/quickstart.md                     |   15 +-
 docs/reference/commands.md                         |   30 +-
 scripts/generate-openclaw-config.py                |   22 +-
 scripts/nemoclaw-start.sh                          |   16 +-
 src/lib/agent-onboard.test.ts                      |   14 +-
 src/lib/agent-onboard.ts                           |   19 +-
 src/lib/channels-mutate-cli-commands.test.ts       |   14 +
 src/lib/channels-mutate-cli-commands.ts            |    6 +-
 src/lib/cli-argv-normalizer.test.ts                |   93 +
 src/lib/cli-argv-normalizer.ts                     |   88 +
 src/lib/command-display-metadata.test.ts           |   26 +
 src/lib/command-registry.ts                        |   60 +-
 src/lib/connect-cli-command.ts                     |    4 +
 src/lib/credentials-cli-command.ts                 |    9 +
 src/lib/debug-cli-command.ts                       |   36 +-
 src/lib/debug-command.test.ts                      |   33 +-
 src/lib/debug-command.ts                           |   88 +-
 src/lib/deploy-cli-command.ts
...[truncated]

Failing tests and unresolved impact

Failing test 1: Root help should not collapse distinct display rows that share one oclif command ID

  • What was tested: The root help refactor should not make separate public usage rows misleading when they map to the same oclif command class.
  • Why it matters: Users lose the only root-help cue that nemoclaw onboard --from is the custom-Dockerfile path; it is displayed identically to plain onboard.
  • Observed result: Help shows both nemoclaw onboard and nemoclaw onboard --from as Configure inference endpoint and credentials; source/display metadata says --from is Use a custom Dockerfile for the sandbox image; duplicateIdsWithDistinctDescriptions=1 for commandId onboard.
  • Command: bash /tmp/pr2922-test1.sh; bash /tmp/pr2922-test3.sh
  • Recommended follow-up coverage: Add a regression test for root help rows with duplicate commandId values and distinct display descriptions, or change metadata keys/registry entries so display variants can retain distinct summaries.

Passing tests and why they mattered

Passing test 1: Installed root help uses oclif summaries when they differ from display metadata

  • What was tested: Root help should prefer registered oclif command summaries for visible descriptions in the installed CLI.
  • Why it mattered: If wrong, users see stale or inconsistent help even though command metadata was refactored.
  • Observed result: Installed root help rows for status, doctor, share mount, and destroy all reported usesSummary=true and usesFallback=false.
  • Command: bash /tmp/pr2922-test2.sh
  • Recommended follow-up coverage: Add a root-help rendering regression test that includes commands whose oclif summary intentionally differs from command-registry display metadata, asserting the summary is used.

Passing test 2: Installed dist metadata lookup works without TypeScript source loading

  • What was tested: The new oclif metadata helper should resolve summaries in the installed/dist runtime shape, not only under the test harness.
  • Why it mattered: If dist lookup failed, root help would silently fall back or throw after installation.
  • Observed result: onboard, sandbox:status, sandbox:doctor, and sandbox:share:mount returned summaries; missing:nope returned null.
  • Command: node -e "require('/workspace/nemoclaw/dist/lib/oclif-metadata.js')..."
  • Recommended follow-up coverage: Keep or add a compiled-output integration smoke test for dist/lib/oclif-metadata.js to ensure the fallback module path works after packaging.

Bottom line

  • Based on the install evidence and adversarial probes, this PR should not be approved as-is.

@cv cv added the v0.0.34 label May 4, 2026
@wscurran wscurran added CI/CD refactor PR restructures code without intended behavior change and removed v0.0.34 labels May 4, 2026

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

Approved as part of stack from #2886 (auto-approve: CI green, no scope/behavior flags).

@cv cv added v0.0.35 and removed v0.0.34 labels May 5, 2026
cv added a commit that referenced this pull request May 5, 2026
## Summary
Introduce explicit public-display-to-oclif command IDs in the command
registry. This prepares root help and docs tooling to combine NemoClaw's
public syntax with oclif command metadata without changing runtime
behavior.

## Stack Navigation
- Position: 28 of 60
- Previous PR: [#2920 — refactor(cli): return typed debug parse
results](#2920)
- Next PR: [#2922 — refactor(cli): use oclif summaries in root
help](#2922)

## Changes
- Added `commandId` to command display metadata entries.
- Mapped every public and hidden display entry to the oclif command that
handles it.
- Added metadata coverage to ensure display entries point at registered
oclif commands.

## 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**
* Added `sandbox doctor` command for sandbox health diagnostics and
troubleshooting
* Added short flags across CLI commands (`-y` for yes, `-q` for quick,
`-o` for output)
* Enhanced `logs` command with `--tail/-n` and `--since` filtering
options
* Restructured `share` functionality into dedicated subcommands
(`mount`, `unmount`, `status`)

* **Documentation**
* Updated command reference with expanded flag support and usage
examples across all subcommands

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv marked this pull request as ready for review May 5, 2026 18:25
@cv
cv changed the base branch from refactor/oclif-arch-display-metadata to main May 5, 2026 18:26
@cv
cv enabled auto-merge (squash) May 5, 2026 18:26
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv merged commit 83238ed into main May 5, 2026
15 of 16 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Replace switch-heavy sandbox command dispatch with route tables. Public
CLI behavior is preserved while making the mapping between public
sandbox syntax and internal oclif command IDs easier to review and
extend.

## Stack Navigation
- Position: 30 of 60
- Previous PR: [#2922 — refactor(cli): use oclif summaries in root
help](#2922)
- Next PR: [#2924 — refactor(cli): normalize gateway token command
id](#2924)

## Changes
- Added flat sandbox route metadata for single-action commands.
- Added nested sandbox route metadata for skill, share, snapshot,
shields, channels, and config.
- Kept public help and compatibility routing behavior stable.

## 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

* **Refactor**
  * Restructured CLI command routing system
* Modified help behavior for nested commands (channels, config, shields)

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv deleted the refactor/oclif-arch-root-help-metadata branch May 27, 2026 21:17
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output and removed NemoClaw CLI labels Jun 3, 2026
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 refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants