Skip to content

refactor(cli): parse durations with oclif flags - #2929

Merged
cv merged 39 commits into
mainfrom
refactor/oclif-first-duration-flags
May 5, 2026
Merged

refactor(cli): parse durations with oclif flags#2929
cv merged 39 commits into
mainfrom
refactor/oclif-first-duration-flags

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

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

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

  • 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

    • 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.

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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d40e0fd6-6d94-41ef-a487-c72ecad57108

📥 Commits

Reviewing files that changed from the base of the PR and between 400869e and 46b349d.

📒 Files selected for processing (4)
  • src/lib/duration-flags.test.ts
  • src/lib/duration-flags.ts
  • src/lib/sandbox-logs-cli-command.ts
  • src/lib/shields-cli-commands.ts

📝 Walkthrough

Walkthrough

A new shared module extracts duration parsing logic into reusable oclif flag adapters. parseLogsSinceDuration and parseShieldsTimeoutDuration validate and normalize duration inputs; corresponding flag adapters wire them into CLI commands, replacing local validation logic.

Changes

Duration Parsing Extraction

Layer / File(s) Summary
Duration Parsers
src/lib/duration-flags.ts
New parsers: parseLogsSinceDuration validates positive durations via regex; parseShieldsTimeoutDuration delegates to parseDuration and wraps errors as CLIError.
oclif Flag Adapters
src/lib/duration-flags.ts
Export logsSinceDurationFlag and shieldsTimeoutDurationFlag as Flags.custom<string> with parse hooks.
CLI Command Integration
src/lib/sandbox-logs-cli-command.ts, src/lib/shields-cli-commands.ts
Replace local LOGS_SINCE_DURATION_RE and Flags.string with imported flag adapters; remove normalizeSinceDuration method.
Tests
src/lib/duration-flags.test.ts
Verify whitespace trimming, regex validation, positive-duration enforcement, error messages, and maximum timeout limit detection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes


🐰 A rabbit hops with glee,
As parsers now are shared and free,
No duplication in sight—
Duration flags, now clean and right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 accurately describes the primary change: extracting duration parsing logic into shared oclif flag parsers for both logs and shields 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-duration-flags

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
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 marked this pull request as ready for review May 5, 2026 19:40
@cv
cv changed the base branch from refactor/oclif-first-lifecycle-options to main May 5, 2026 19:40
@cv
cv enabled auto-merge (squash) May 5, 2026 19:40
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv merged commit a3e91eb into main May 5, 2026
14 of 15 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Replace the transitional hand-written public sandbox help renderer with
an oclif `CommandHelp` projection that keeps NemoClaw's public syntax
while letting oclif render command details.

## Stack Navigation
- Position: 37 of 60
- Previous PR: [#2929 — refactor(cli): parse durations with oclif
flags](#2929)
- Next PR: [#2936 — test(cli): cover oclif metadata routing
helpers](#2936)

## Changes
- Extended oclif metadata lookup so public help can access args, flags,
base flags, usage, and examples.
- Reworked public sandbox help rendering to use `CommandHelp` with an
overridden public usage line.
- Preserved public `nemoclaw <name> action` syntax while rendering
oclif-managed flags, arguments, descriptions, and examples.
- Updated help tests to assert the projected oclif output does not leak
internal command IDs.

## Notes
Help output format changes from `Usage: nemoclaw ...` to `$ nemoclaw
...` (oclif default rendering). Public command syntax and flags are
unchanged; only the leading prefix in help/example lines differs.

## 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**
* Help output now displays with enhanced structure, including clearly
separated USAGE sections with command syntax examples, FLAGS sections
listing available options, and fully-resolved command examples to guide
users through commands.
* Help text generation now uses standardized formatting, ensuring
consistent and professional presentation across all CLI commands.
<!-- 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-duration-flags 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