refactor(cli): migrate commands to shared oclif base#3611
Conversation
|
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 (52)
📝 WalkthroughWalkthroughAll CLI command classes are migrated from Oclif's ChangesNemoClaw CLI Command Base Class Migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 docstrings
🧪 Generate unit tests (beta)
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
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
cjagwani
left a comment
There was a problem hiding this comment.
Approving. Clean mechanical pass to land the second half of the NemoClawCommand migration started in #3599, plus a self-enforcing guard so this can't silently regress.
What I checked:
- All 50 command-class edits are the same 3-line pattern (drop
Commandimport, addNemoClawCommandimport, swapextends). No file declared a localhelpflag, so the newbaseFlags = { help: Flags.help(...) }on the base is collision-free. src/lib/cli/oclif-command-metadata.test.ts:18-28loads the oclif config and walks every discovered command's prototype chain — exactly the verifiable goal of the PR.credentials.tsandonboard.tsare risky-area paths but only as thin oclif shells forwarding to existing actions; therun()bodies are unchanged.- One drive-by: a stray blank line removed at
src/lib/recover-cli-command.ts:4. Under-the-bar.
Nit (non-blocking): the prototype walker matches on current.name === "NemoClawCommand". instanceof NemoClawCommand would be slightly more robust to renames, but the stringly-typed match avoids a circular import at test-discovery time and is fine.
CI is fully green including test-e2e-sandbox, test-e2e-gateway-isolation, test-e2e-ollama-proxy, and macos-e2e. Ready to merge.
## Summary Uses the shared `NemoClawCommand` base for common oclif conventions now that #3611 has landed. It removes duplicated command-local help flags, routes internal JSON output through the base helper, and adds a guard so help stays centralized. ## Changes - Removed repeated `help: Flags.help({ char: "h" })` declarations now inherited from `NemoClawCommand.baseFlags`. - Replaced internal command `console.log(JSON.stringify(..., null, 2))` calls with `this.logJson(...)`. - Added an oclif metadata guard that fails when discovered command classes redeclare a local `help` flag. ## 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>
Summary
Migrates the remaining oclif command classes onto
NemoClawCommandso CLI-wide parser and output conventions have a single base class. Adds a metadata guard that loads every discovered oclif command and fails if any command bypasses the shared NemoClaw base.Changes
NemoClawCommand.NemoClawCommand.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
Release Notes