Skip to content

fix(cli): render missing channel arg usage#3863

Open
yimoj wants to merge 1 commit into
NVIDIA:mainfrom
yimoj:fix/3704-channels-required-arg-usage
Open

fix(cli): render missing channel arg usage#3863
yimoj wants to merge 1 commit into
NVIDIA:mainfrom
yimoj:fix/3704-channels-required-arg-usage

Conversation

@yimoj
Copy link
Copy Markdown
Contributor

@yimoj yimoj commented May 20, 2026

Summary

Missing messaging channel arguments now render oclif's normal required-argument usage instead of leaking parser internals. The public channel command signatures also show <channel> in the positional usage slot for add/remove/start/stop.

Related Issue

Fixes #3704

Changes

  • Keep runOclifArgv's temporary process.argv aligned with the native oclif argv while oclif renders parse-error help, then restore it in a finally block.
  • Move <channel> into the public channel command usage metadata and leave --dry-run in the flags column.
  • Add regression coverage for missing channel args through the real CLI dispatch path and root help signatures.
  • Update CLI/docs parity checking so canonical required placeholders such as <channel> are preserved while docs-only suffixes are still tolerated.

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)

Additional verification run locally:

  • isolated real CLI repro for alpha channels add|remove|start|stop missing <channel>: each exited 2 with usage and no RequiredArgsError/validateArgs stack marker
  • npx -y -p node@22.16.0 node node_modules/vitest/vitest.mjs run src/lib/cli/oclif-runner.test.ts test/cli.test.ts test/root-help.test.ts test/cli-oclif-compatibility.test.ts -t "(runOclifArgv|restores process argv|policy and channel mutations reject missing parser-owned values before dispatch|shows channel as a required positional argument|renders native sandbox help without registry recovery|hands public sandbox execution to oclif as native argv|uses the alias binary name in native oclif help)"
  • npx -y -p node@22.16.0 -p npm@10 npm run typecheck:cli
  • npx -y -p node@22.16.0 -p npm@10 npm run build:cli
  • npx -y -p node@22.16.0 -p npm@10 npm run checks
  • bash -n test/e2e/e2e-cloud-experimental/check-docs.sh
  • bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-cli
  • cd nemoclaw && npx -y -p node@22.16.0 -p npm@10 npm test

Local broad CLI project note: NEMOCLAW_TEST_TIMEOUT=60000 NEMOCLAW_EXEC_TIMEOUT=60000 npx -y -p node@22.16.0 node node_modules/vitest/vitest.mjs run --project cli was otherwise green but failed an unrelated host-permission fixture in test/fetch-guard-patch-regression.test.ts; the fixture exits on rm -rf /usr/local/lib/node_modules/openclaw because this machine's /usr/local/lib/node_modules is root-owned 750.


Signed-off-by: Yimo Jiang yimoj@nvidia.com

Summary by CodeRabbit

  • Bug Fixes

    • CLI help for channel commands (add, remove, start, stop) now correctly shows the required positional in usage and displays flags without duplicating the positional.
  • Tests

    • Added and strengthened tests to verify CLI help output and to ensure process arguments are preserved and restored during command execution.
  • Documentation

    • CLI docs extraction/parity improved so command headings match canonical CLI signatures.

Review Change Stack

@yimoj yimoj self-assigned this May 20, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

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: b7d1a650-9ae6-40ef-9842-e63e8e356176

📥 Commits

Reviewing files that changed from the base of the PR and between 5299449 and 45b6d22.

📒 Files selected for processing (9)
  • src/commands/sandbox/channels/add.ts
  • src/commands/sandbox/channels/remove.ts
  • src/commands/sandbox/channels/start.ts
  • src/commands/sandbox/channels/stop.ts
  • src/lib/cli/oclif-runner.test.ts
  • src/lib/cli/oclif-runner.ts
  • test/cli.test.ts
  • test/e2e/e2e-cloud-experimental/check-docs.sh
  • test/root-help.test.ts
✅ Files skipped from review due to trivial changes (3)
  • src/commands/sandbox/channels/remove.ts
  • src/commands/sandbox/channels/stop.ts
  • src/commands/sandbox/channels/start.ts

📝 Walkthrough

Walkthrough

Move <channel> from flags into usage for sandbox channel commands; runOclifArgv temporarily sets a branded process.argv during executeOclif and restores it; tests and docs extraction updated to expect canonical channel signatures.

Changes

Channel Command Signatures and Oclif argv Branding

Layer / File(s) Summary
Channel command help metadata
src/commands/sandbox/channels/add.ts, src/commands/sandbox/channels/remove.ts, src/commands/sandbox/channels/start.ts, src/commands/sandbox/channels/stop.ts
PublicDisplay entries updated to include <channel> in usage and remove it from flags, leaving [--dry-run] as the flag fragment.
Process argv preservation in oclif-runner
src/lib/cli/oclif-runner.ts
runOclifArgv captures original process.argv, composes and sets a branded argv (defaulting missing entries), calls executeOclif, and restores the original argv in a finally block.
Process argv handling tests
src/lib/cli/oclif-runner.test.ts
Tests save/restore process.argv via hooks, assert the branded argv is passed to the mocked @oclif/core execute, and add a test ensuring argv restoration when execute throws.
Integration and help-output tests
test/cli.test.ts, test/root-help.test.ts
Dispatch/regression and root-help tests updated to require <channel> for add/remove/start/stop and to assert oclif-style missing-arg usage/error output.
Docs parity extraction
test/e2e/e2e-cloud-experimental/check-docs.sh
Docs extraction now consults canonical --dump-commands output, preserves canonical placeholders, and trims/exact-matches extracted headings before further normalization.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

fix, NemoClaw CLI

Suggested reviewers

  • cv
  • ericksoa

Poem

A rabbit hops through argv's tangled song,
Brands the CLI name where parse-errors belong,
Channels now demand their place in line,
Tests watch argv dance, then fall back in time,
🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% 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 'fix(cli): render missing channel arg usage' directly and clearly summarizes the main change: fixing how missing channel arguments are rendered in CLI usage output.
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 unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

Keep the native oclif dispatch argv in sync with process.argv while oclif handles parser errors, so missing channel args render command usage instead of leaking parser internals.

Move the required <channel> positional into public channel command signatures and cover add/remove/start/stop through CLI dispatch tests.

Signed-off-by: Yimo Jiang <yimoj@nvidia.com>
@yimoj yimoj force-pushed the fix/3704-channels-required-arg-usage branch from 5299449 to 45b6d22 Compare May 20, 2026 05:41
@yimoj yimoj added the v0.0.48 Release target label May 20, 2026
@wscurran wscurran added fix NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). labels May 20, 2026
@wscurran
Copy link
Copy Markdown
Contributor

@cv cv added v0.0.49 Release target and removed v0.0.48 Release target labels May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). v0.0.49 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][CLI&UX] nemoclaw channels add without <channel> argument dumps raw stack trace instead of actionable usage hint

3 participants