Skip to content

refactor(cli): model debug flags with oclif - #2912

Merged
cv merged 24 commits into
mainfrom
refactor/oclif-ux-debug-flags
May 5, 2026
Merged

refactor(cli): model debug flags with oclif#2912
cv merged 24 commits into
mainfrom
refactor/oclif-ux-debug-flags

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Move debug flag parsing into its oclif adapter so unknown flags and missing flag values fail through the parser before diagnostic collection. The debug action helpers remain available for existing unit tests and now also accept already-parsed options.

Stack Navigation

Changes

  • Modeled --quick/-q, --output/-o, and --sandbox as oclif flags.
  • Added debug command examples.
  • Added runDebugCommandWithOptions for adapters that already have parsed options.
  • Updated CLI and unit tests for oclif-owned parse errors and parsed-option execution.

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)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • New Features

    • Debug command accepts shorthand flags (-q, -o) and explicit output/sandbox options.
    • Debug command can be invoked with pre-parsed options so sandbox defaults are applied when not provided.
  • Improvements

    • Stronger CLI parsing and clearer error messages for unknown flags and missing flag values.
  • Tests

    • CLI tests tightened to assert specific error messages for invalid flag usage.

@cv cv self-assigned this May 3, 2026
@copy-pr-bot

copy-pr-bot Bot commented May 3, 2026

Copy link
Copy Markdown

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.

@coderabbitai

coderabbitai Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 714c6655-a71f-424b-8d8d-c12d10840f69

📥 Commits

Reviewing files that changed from the base of the PR and between 928219c and 52eb5a1.

📒 Files selected for processing (1)
  • test/cli.test.ts

📝 Walkthrough

Walkthrough

The debug CLI was refactored to parse oclif flags strictly and build a DebugOptions object; a new runDebugCommandWithOptions helper centralizes defaulting of sandboxName (via getDefaultSandbox) before calling runDebug. Tests and CLI error assertions updated accordingly.

Changes

Debug Command Refactoring

Layer / File(s) Summary
Data Shape / Types
src/lib/debug-command.ts
Introduces/uses DebugOptions as the options shape passed to the runtime helper.
Core Helper
src/lib/debug-command.ts
Adds exported runDebugCommandWithOptions(options, deps) which clones options, fills sandboxName from deps.getDefaultSandbox() when missing, and calls deps.runDebug(opts). Keeps runDebugCommand(args, deps) as entry that parses args.
CLI Flag Definitions
src/lib/debug-cli-command.ts
Adds static flags (including `--quick
CLI Integration
src/lib/debug-cli-command.ts
Replaces run() to use await this.parse(DebugCliCommand), map parsed flags into DebugOptions, and call runDebugCommandWithOptions(options, buildDebugCommandDeps(this.config.root)).
Tests / Assertions
src/lib/debug-command.test.ts, test/cli.test.ts
Adds a test verifying runDebugCommandWithOptions falls back to default sandbox when none provided; tightens CLI error assertions to check for Nonexistent flag: --quik and presence of --sandbox in missing-name error output.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant CLI as "Debug CLI"
    participant Parser as "oclif parser"
    participant CmdHelper as "runDebugCommandWithOptions"
    participant Deps as "Deps (getDefaultSandbox, runDebug)"
    CLI->>Parser: parse flags (quick, output, sandbox)
    Parser-->>CLI: parsed flags
    CLI->>CmdHelper: call with DebugOptions (from flags)
    CmdHelper->>Deps: getDefaultSandbox() if sandbox missing
    Deps-->>CmdHelper: sandboxName (e.g., "alpha")
    CmdHelper->>Deps: runDebug(options with sandboxName)
    Deps-->>CmdHelper: runDebug result
    CmdHelper-->>CLI: complete
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 I nibbled flags and stitched them tight,
Short hops of -q and -o in sight,
Defaults fetched when the sandbox hides,
Clean options now take joyful strides. 🥕✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/oclif-ux-debug-flags

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

@cjagwani cjagwani left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved as part of stack from #2886 (auto-approve: CI green, no scope/behavior flags).

cv added a commit that referenced this pull request May 5, 2026
## Summary
Split the sandbox share command into explicit oclif subcommands for
mount, unmount, and status. This keeps the public `nemoclaw <name> share
...` syntax stable while moving subcommand routing and
unknown-subcommand handling into oclif.

## Stack Navigation
- Position: 19 of 60
- Previous PR: [#2910 — refactor(cli): add lifecycle confirmation flag
aliases](#2910)
- Next PR: [#2912 — refactor(cli): model debug flags with
oclif](#2912)

## Changes
- Added `sandbox:share`, `sandbox:share:mount`, `sandbox:share:unmount`,
and `sandbox:share:status` command adapters.
- Moved share mount/unmount/status behavior into exported action helpers
reused by the adapters and tests.
- Updated sandbox dispatch to route share subcommands to explicit oclif
command IDs.
- Added CLI coverage for unknown share subcommands failing before action
dispatch.

## 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

* **New Features**
* Added sandbox-scoped share subcommands: share mount/unmount/status and
improved share help/usage.
* Added `-y` short flag for `--yes` across destroy, rebuild, upgrade,
and garbage-collection; rebuild docs also show `-v/--verbose`.
* Added `sandbox:doctor` command and added CLI usage examples for
several commands.

* **Bug Fixes**
* Improved help/usage and argument validation to surface unexpected
arguments earlier.

* **Tests**
* Expanded tests for share help, destroy short-flag usage, and
maintenance command help.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com>
@cv
cv marked this pull request as ready for review May 5, 2026 00:47
@cv
cv changed the base branch from refactor/oclif-ux-share-subcommands to main May 5, 2026 00:47
@cv
cv enabled auto-merge (squash) May 5, 2026 00:47
cv and others added 2 commits May 4, 2026 17:47
@cv
cv merged commit 0f13026 into main May 5, 2026
12 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Model the onboard/setup/setup-spark flag surface in oclif while
preserving the existing onboarding action validation for Dockerfile
paths, agents, sessions, and third-party notice semantics. This lets
oclif own unknown flags and missing flag values before the onboarding
action runs.

## Stack Navigation
- Position: 21 of 60
- Previous PR: [#2912 — refactor(cli): model debug flags with
oclif](#2912)
- Next PR: [#2915 — docs: sync oclif UX command
reference](#2915)

## Changes
- Added oclif flags for onboard, setup, and setup-spark options.
- Added onboard examples for common interactive, resume, fresh, custom
Dockerfile, and non-interactive invocations.
- Reconstructed legacy argv for the existing onboarding action so domain
validation stays centralized.
- Kept deprecated setup/setup-spark help paths routed through the
existing deprecation messaging.
- Updated CLI expectations for parser-owned unknown flag errors.

## 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

* **Refactor**
* CLI commands now use stricter flag parsing and convert parsed flags
into the legacy argument format, improving validation and behavior for
onboarding and setup flows.

* **Documentation**
* Improved usage text, examples, and more predictable handling of help
requests for onboard and setup commands.

* **Tests**
* CLI tests updated for stricter parsing, standardized parser exit
codes, and updated error output expectations for invalid flags.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv deleted the refactor/oclif-ux-debug-flags branch May 27, 2026 21:18
@wscurran wscurran added the refactor PR restructures code without intended behavior change label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants