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. |
|
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 (18)
📝 WalkthroughWalkthroughThis PR reorganizes CLI-related modules by consolidating imports under a ChangesCLI Module Reorganization
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 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)
Comment |
## Summary Extract pure policy-add argument parsing helpers from the policy/channel action module. ## Stack Navigation - Position: 51 of 60 - Previous PR: [#2965 — refactor(cli): extract sandbox logs helpers](#2965) - Next PR: [#2967 — refactor(cli): group oclif core modules](#2967) ## Changes - Added `policy-channel-helpers.ts` for custom policy source parsing, confirmation bypass detection, and policy-add argument normalization. - Updated `addSandboxPolicy` to use the extracted parser helpers while keeping filesystem and policy application logic in the action module. - Added direct helper coverage for `--from-file`, `--from-dir`, mutual exclusion, missing values, confirmation aliases, and preset argument parsing. ## 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]> --------- 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. Pure file-organization PR — 18 files / +18 / -14, with all renames tracked by git at 97–100% similarity.
Six oclif-core modules move src/lib/<name>.ts → src/lib/cli/<name>.ts. The only substantive content change is in oclif-metadata.ts: dynamic-require fallback paths shift to account for the new subdir location, plus a new ../oclif-commands.js entry to handle compiled-dist packaging layouts. Consumer files (6 + nemoclaw.ts) have one-line import-path updates.
Public CLI surface unchanged. Test files renamed alongside their sources.
CI: fully green across every check — pr.yaml (lint/dco/check-hash/legacy-path-guard/macos-e2e/changes/checks/test-e2e-ollama-proxy) + pr-self-hosted (build-sandbox-images/arm64, test-e2e-sandbox/gateway-isolation/port-overrides) + CodeRabbit. Cleanest hash on the stack.
## Summary Split the oclif command adapter layer out of the flat `src/lib/` directory and mirror the command tree under `src/lib/commands/**`. ## Stack Navigation - Position: 53 of 60 - Previous PR: [#2967 — refactor(cli): group oclif core modules](#2967) - Next PR: [#2984 — refactor(cli): group pure domain helpers](#2984) ## Changes - Moved the explicit oclif registry to `src/lib/commands/index.ts` and updated the oclif target in `package.json`. - Split registered oclif command classes to one command per production file, grouped by command family and sandbox scope. - Moved command adapter tests beside the new command tree and updated imports/mocks. - Kept behavior unchanged; command IDs, public syntax, parser flags, and action calls are preserved. ## 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 clean:cli && npm run build:cli && npm run dist:sourcemaps:check` - [x] `npm run typecheck:cli` - [x] Command adapter and metadata 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]> --------- Signed-off-by: Carlos Villela <[email protected]>
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
Changes
src/lib/cli/.src/nemoclaw.ts, root help, and existing command adapters.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela [email protected]
Summary by CodeRabbit