Skip to content

refactor(cli): use oclif flag relationships - #2927

Merged
cv merged 37 commits into
mainfrom
refactor/oclif-first-flag-relationships
May 5, 2026
Merged

refactor(cli): use oclif flag relationships#2927
cv merged 37 commits into
mainfrom
refactor/oclif-first-flag-relationships

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Move simple cross-flag validation into oclif metadata so NemoClaw does not maintain duplicate parser logic for relationships oclif already supports.

Stack Navigation

Changes

  • Made onboard --resume and --fresh mutually exclusive through oclif flag metadata.
  • Made policy --from-file and --from-dir mutually exclusive through oclif flag metadata.
  • Added command-level tests showing invalid flag combinations fail before action dispatch.
  • Updated policy CLI expectations for oclif's relationship validation wording.

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

  • Improvements

    • CLI flags updated so conflicting options are now mutually exclusive (prevents combining resume/fresh and from-file/from-dir).
    • Improved validation to surface errors earlier when incompatible flags are provided.
  • Documentation

    • Added example usages to policy-related CLI help output.
  • Tests

    • Added and updated tests covering the new validation and error messaging.

cv added 30 commits May 2, 2026 13:36
@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: ad9fd6fe-7225-4b96-9812-d19c0947bc94

📥 Commits

Reviewing files that changed from the base of the PR and between d0d2a70 and a5ea73b.

📒 Files selected for processing (2)
  • src/lib/onboard-cli-commands.ts
  • test/policies.test.ts

📝 Walkthrough

Walkthrough

Flags for onboard and policy CLI commands were made mutually exclusive where appropriate; policy command examples were added; tests were added/updated to verify flag validation and error messages, and a new test ensures onboard's resume/fresh combination rejects before action dispatch.

Changes

CLI Flag Exclusivity, Metadata, and Tests

Layer / File(s) Summary
Flag Definition & Validation
src/lib/onboard-cli-commands.ts, src/lib/policy-mutate-cli-commands.ts
resume and fresh flags now declare mutual exclusive constraints; from-file and from-dir flags in PolicyAddCommand now declare exclusive constraints.
Command Metadata
src/lib/policy-mutate-cli-commands.ts
PolicyAddCommand and PolicyRemoveCommand gained static examples arrays documenting usage.
New / Updated Unit Tests
src/lib/onboard-cli-commands.test.ts, src/lib/policy-mutate-cli-commands.test.ts
Added tests asserting: OnboardCliCommand.run(["--resume","--fresh"], ...) rejects before dispatching action; PolicyAddCommand rejects when --from-file value missing and when --from-file and --from-dir are provided together, also ensuring no runtime dispatch.
Integration Test Adjustment
test/policies.test.ts
Updated expectation for policy-add --from-file / --from-dir error text to match "cannot also be provided" wording.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

"I hop with joy at flags aligned,
Exclusive paths now well-defined,
Tests guard the hops and stops,
Examples show the proper props,
A little rabbit clap — refined!" 🐇✨

✨ 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-first-flag-relationships

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

@cv cv added the v0.0.34 label May 4, 2026
@wscurran wscurran added NemoClaw CLI refactor PR restructures code without intended behavior change and removed v0.0.34 labels May 4, 2026

@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 cv added v0.0.35 and removed v0.0.34 labels May 5, 2026
@cv
cv marked this pull request as ready for review May 5, 2026 19:15
@cv
cv changed the base branch from refactor/oclif-first-command-base to main May 5, 2026 19:15
@cv
cv enabled auto-merge (squash) May 5, 2026 19:15
@cv
cv disabled auto-merge May 5, 2026 19:16
@cv
cv changed the base branch from main to refactor/oclif-first-command-base May 5, 2026 19:16
cv added a commit that referenced this pull request May 5, 2026
## Summary
Introduce a shared NemoClaw oclif command base so common parser
conventions live in one place instead of every command class redefining
them.

## Stack Navigation
- Position: 33 of 60
- Previous PR: [#2925 — refactor(cli): render public oclif
help](#2925)
- Next PR: [#2927 — refactor(cli): use oclif flag
relationships](#2927)

## Changes
- Added `NemoClawCommand` with common `-h/--help` base flag handling.
- Centralized JSON formatting used by oclif JSON-enabled commands.
- Migrated low-risk list, status, and maintenance command adapters to
the shared base.

## 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
- [ ] 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**
* Restructured CLI command architecture for backup, upgrade, garbage
collection, list, and status operations. All commands continue to
function with consistent help and JSON output support.

<!-- 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>
@cv
cv changed the base branch from refactor/oclif-first-command-base to main May 5, 2026 19:21
@cv
cv enabled auto-merge (squash) May 5, 2026 19:21
@cv
cv merged commit e55c669 into main May 5, 2026
15 of 22 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Stop rebuilding argv strings in lifecycle oclif adapters and pass typed
option objects from the parsed oclif result into action facades.

## Stack Navigation
- Position: 35 of 60
- Previous PR: [#2927 — refactor(cli): use oclif flag
relationships](#2927)
- Next PR: [#2929 — refactor(cli): parse durations with oclif
flags](#2929)

## Changes
- Added typed lifecycle option shapes and compatibility normalizers for
destroy, rebuild, gc, and upgrade-sandboxes.
- Updated lifecycle command classes to pass typed options instead of
reconstructed legacy argv arrays.
- Kept action-level compatibility with existing internal argv callers.
- Replaced a gc source-shape assertion with behavior-level option
normalization coverage.

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

* **Refactor**
* Internal improvements to command option handling for enhanced type
safety and consistency across sandbox management operations.

* **Tests**
* Added test coverage for option normalization utilities to ensure
robust handling of configuration 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-first-flag-relationships branch May 27, 2026 21:18
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output and removed NemoClaw CLI labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants