refactor(cli): split oclif commands by command tree#2970
Conversation
This reverts commit 4ebeae4.
|
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. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (86)
📝 WalkthroughWalkthroughThis PR restructures the CLI command architecture from consolidated barrel exports to a modular, per-file layout. Commands are migrated from monolithic ChangesCLI Command Modularization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
## Summary Start the CLI layer directory structure by grouping oclif core/runtime modules under `src/lib/cli/`. This makes argv normalization, dispatch, oclif runtime helpers, metadata lookup, public help rendering, and the shared command base visibly part of the CLI boundary. ## Stack Navigation - Position: 52 of 60 - Previous PR: [#2966 — refactor(cli): extract policy channel helpers](#2966) - Next PR: [#2970 — refactor(cli): split oclif commands by command tree](#2970) ## Changes - Moved oclif core modules and their tests into `src/lib/cli/`. - Updated imports from `src/nemoclaw.ts`, root help, and existing command adapters. - Kept command behavior unchanged; this PR is a file-structure/import move only. ## 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 <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Reorganized internal CLI module structure by moving related components into a dedicated subdirectory for improved code organization and maintainability. Updated import paths across multiple files to reflect the new hierarchy. Enhanced command metadata resolution to support additional packaging layouts. No changes to user-facing functionality or CLI behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <[email protected]> Co-authored-by: Prekshi Vyas <[email protected]>
Signed-off-by: Carlos Villela <[email protected]>
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM. Large but mechanical — 86 files / +1687 / -1442.
Eight aggregator files (oclif-commands.ts + 10 *-cli-commands.ts/-cli-command.ts files) deleted; their classes split one-per-file under src/lib/commands/<topic>/<command>.ts mirroring the oclif command tree. New commands/index.ts replaces the aggregator registry. package.json oclif target updated to point at it.
Verified the colon-separated command IDs (e.g. sandbox:policy:add, sandbox:gateway:token) match what the OLD registry already used — not a behavior change. User-facing <name> policy-add hyphenated usage strings preserved.
oclif-metadata.ts tiny path tweak (3+/3-) to find the new registry. Test files moved alongside their sources.
CI: pr.yaml fully green (lint/dco/check-hash/legacy-path-guard/macos-e2e/changes/checks/test-e2e-ollama-proxy PASS); pr-self-hosted builds + wsl-e2e still in flight at review time. Critical signals (typecheck, integration test, adapter+metadata Vitest) all green.
## Summary Move pure CLI/domain helper modules out of the flat `src/lib/` directory into `src/lib/domain/**` so parser, action, and domain boundaries are visible in paths. ## Stack Navigation - Position: 54 of 60 - Previous PR: [#2970 — refactor(cli): split oclif commands by command tree](#2970) - Next PR: [#2985 — refactor(cli): group openshell adapters](#2985) ## Changes - Moved lifecycle option normalizers to `src/lib/domain/lifecycle/options.ts`. - Moved duration parsing, maintenance image/upgrade helpers, sandbox destroy/log helpers, sandbox log options, and policy/channel arg helpers under `src/lib/domain/**`. - Moved helper tests with their modules and updated imports from actions, command adapters, and tests. - Kept behavior unchanged; this PR is a structural move plus import updates. ## 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 --stage pre-push` passes - [x] `npm run build:cli` - [x] `npm run typecheck:cli` - [x] Domain/helper targeted tests pass - [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 (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Carlos Villela <[email protected]> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Internal module structure reorganized to improve code organization and maintainability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <[email protected]> Co-authored-by: Prekshi Vyas <[email protected]>
Summary
Split the oclif command adapter layer out of the flat
src/lib/directory and mirror the command tree undersrc/lib/commands/**.Stack Navigation
Changes
src/lib/commands/index.tsand updated the oclif target inpackage.json.Type of Change
Verification
npx prek run --all-files --stage pre-pushpassesnpm run clean:cli && npm run build:cli && npm run dist:sourcemaps:checknpm run typecheck:climake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela [email protected]
Summary by CodeRabbit