Skip to content

refactor(cli): pass lifecycle typed options - #2928

Merged
cv merged 38 commits into
mainfrom
refactor/oclif-first-lifecycle-options
May 5, 2026
Merged

refactor(cli): pass lifecycle typed options#2928
cv merged 38 commits into
mainfrom
refactor/oclif-first-lifecycle-options

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Stop rebuilding argv strings in lifecycle oclif adapters and pass typed option objects from the parsed oclif result into action facades.

Stack Navigation

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

  • 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

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.

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

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR refactors sandbox lifecycle actions to accept typed options objects instead of CLI argument arrays. It introduces lifecycle-options.ts with four option interfaces and normalizer functions for backward compatibility, then updates all action files, CLI commands, and wrappers to use normalized options derived from either structured objects or string arrays of flags.

Changes

Lifecycle Options Refactoring

Layer / File(s) Summary
Type Definitions & Normalizers
src/lib/lifecycle-options.ts
Defines DestroySandboxOptions, RebuildSandboxOptions, GarbageCollectImagesOptions, and UpgradeSandboxesOptions interfaces. Each normalizer function accepts string[] | TypedOptions and converts flag strings (e.g., --force, --yes, --verbose, --dry-run, --auto, --check) to typed boolean fields, or returns the options object unchanged.
Core Action Functions
src/lib/sandbox-destroy-action.ts, src/lib/sandbox-rebuild-action.ts, src/lib/maintenance-actions.ts, src/lib/upgrade-sandboxes-action.ts
Each action now accepts options: string[] | TypedOptions and normalizes it via the appropriate normalizer. Behavior gates (skipConfirm, dryRun, verbose, etc.) derive from normalized option fields instead of direct flag inspection.
Runtime & Global Wrappers
src/lib/sandbox-runtime-actions.ts, src/lib/global-cli-actions.ts
Function signatures updated to accept options: string[] | TypedOptions and forward them to underlying actions. Type assertions for dynamic require() calls updated to reflect the new parameter shape.
CLI Command Classes
src/lib/destroy-cli-command.ts, src/lib/rebuild-cli-command.ts, src/lib/maintenance-cli-commands.ts
CLI commands updated to construct and pass typed options objects { force, yes, verbose, ... } to action functions instead of building legacy string arrays. DestroyCliCommand and RebuildCliCommand now extend NemoClawCommand instead of Command.
Tests & Option Normalization Coverage
src/lib/lifecycle-options.test.ts, test/image-cleanup.test.ts
New test file verifies normalization preserves typed objects and correctly maps flag arrays. Existing test updated to focus on option normalization for garbage-collect operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops of joy, this refactoring dance,
From string arrays to options true,
Type-safe flags in every stance,
A backward-compat encore, too!
Lifecycle flows now cleanly brew. 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main refactoring: replacing legacy argv string reconstruction with typed option objects for lifecycle CLI commands.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-lifecycle-options

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 added a commit that referenced this pull request May 5, 2026
## Summary
Move simple cross-flag validation into oclif metadata so NemoClaw does
not maintain duplicate parser logic for relationships oclif already
supports.

## Stack Navigation
- Position: 34 of 60
- Previous PR: [#2926 — refactor(cli): add shared oclif command
base](#2926)
- Next PR: [#2928 — refactor(cli): pass lifecycle typed
options](#2928)

## 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
- [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 doctor` command for comprehensive health diagnostics
* Enhanced logs streaming with `--follow`, `--tail`, and `--since`
filtering options

## Improvements
* Added short flag aliases: `-y` for `--yes`, `-q` for `--quick`, `-n`
for `--tail`, `-v` for `--verbose`, `-o` for `--output`
* Improved CLI help output with command examples across all subcommands
* Enhanced error messages and argument validation for better user
guidance
* Extended sandbox rebuild workflow with credential preflight and
workspace backup/restore

## Documentation
* Updated documented CLI flag syntax for all major commands with new
short forms and options

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv marked this pull request as ready for review May 5, 2026 19:26
@cv
cv changed the base branch from refactor/oclif-first-flag-relationships to main May 5, 2026 19:26
@cv
cv enabled auto-merge (squash) May 5, 2026 19:26
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv merged commit 400869e into main May 5, 2026
15 of 16 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Move duration syntax validation into shared custom oclif flags so logs
and shields adapters rely on oclif parsing instead of post-parse
validation.

## Stack Navigation
- Position: 36 of 60
- Previous PR: [#2928 — refactor(cli): pass lifecycle typed
options](#2928)
- Next PR: [#2930 — refactor(cli): project public help through
oclif](#2930)

## Changes
- Added shared custom oclif duration flag parsers for logs `--since` and
shields `--timeout`.
- Updated logs command parsing to receive an already-normalized
`--since` value from oclif.
- Updated hidden shields down command metadata to use the shared timeout
parser.
- Added unit coverage for accepted and rejected duration inputs.

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

* **Improvements**
* Enhanced validation and whitespace normalization for duration inputs
in CLI flags (`--since` and `--timeout`), with clearer error messages
for invalid values.

* **Tests**
* Added test coverage for duration flag parsing to ensure proper
validation and error handling.

<!-- 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-lifecycle-options 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