refactor(cli): split share oclif commands - #2939
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 (4)
📝 WalkthroughWalkthroughA CLI command module reorganization moves the share command classes (ShareCommand, ShareMountCommand, ShareUnmountCommand, ShareStatusCommand) from share-command.ts to a new share-cli-commands.ts file, updates the import in oclif-commands.ts, and adds comprehensive unit tests for the moved commands. ChangesShare Command Module Reorganization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
cjagwani
left a comment
There was a problem hiding this comment.
Approved as part of the oclif refactor stack — deep review confirmed the smoke flag (+1 throw) was a test mock, not production.
## Summary Remove broad coverage ignores from sandbox oclif adapters whose parser behavior can now be tested in-process. ## Stack Navigation - Position: 40 of 60 - Previous PR: [#2937 — test(cli): cover global oclif command adapters](#2937) - Next PR: [#2939 — refactor(cli): split share oclif commands](#2939) ## Changes - Removed file-level V8 ignores from sandbox connect, lifecycle, inspection, doctor, logs, policy/channel, snapshot, skill, and shields adapters. - Added direct command-adapter coverage for connect/lifecycle option mapping, inspection actions, doctor flags, and shields flags. - Reused existing command-family tests for logs, policy/channel mutation, snapshot, and skill adapters. ## 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 comprehensive sandbox health diagnostics * Extended `sandbox logs` with `--tail`/`-n` and `--since` options for flexible log filtering * Added short flag aliases: `-y` for `--yes`, `-q` for `--quick`, `-o` for `--output`, `-n` for `--tail` * **Improvements** * Enhanced CLI documentation with expanded flag descriptions * Refactored `sandbox share` into discrete `mount`, `unmount`, and `status` subcommands * Improved command validation and error messaging <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
prekshivyas
left a comment
There was a problem hiding this comment.
LGTM. Clean file split — four share oclif Command classes (ShareCommand + Mount/Unmount/Status) move verbatim from share-command.ts into a new share-cli-commands.ts. SSHFS action helpers stay in share-command.ts and are imported by the new adapter module. Zero behavior change.
+60 lines of new direct adapter tests in share-cli-commands.test.ts.
CI: pr.yaml mostly green (lint/dco/check-hash/legacy-path-guard/changes PASS); macos-e2e/checks + pr-self-hosted builds still in flight at review time. No failures.
## Summary Replace the file-level coverage ignore on sandbox log actions with tested pure helpers and narrow ignores around subprocess-only paths. ## Stack Navigation - Position: 42 of 60 - Previous PR: [#2939 — refactor(cli): split share oclif commands](#2939) - Next PR: [#2942 — test(cli): cover remaining global and credentials adapters](#2942) ## Changes - Removed the broad V8 ignore from `sandbox-logs-action.ts`. - Exported and tested pure log option normalization and argv-building helpers. - Added narrow V8 ignores only around process-exit, OpenShell subprocess, and follow-streaming boundaries still covered by CLI subprocess tests. - Added helper coverage for log probe result descriptions and probe timeout environment handling. ## 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> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
Summary
Split share oclif command classes out of the SSHFS action/helper module so parser adapters and action logic are independently testable.
Stack Navigation
Changes
share-cli-commands.tsfor the share parent and mount/unmount/status oclif adapters.share-command.ts.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
Tests
Refactor