Skip to content

refactor(cli): group pure domain helpers#2984

Merged
cv merged 79 commits intomainfrom
refactor/layer-domain-helpers
May 6, 2026
Merged

refactor(cli): group pure domain helpers#2984
cv merged 79 commits intomainfrom
refactor/layer-domain-helpers

Conversation

@cv
Copy link
Copy Markdown
Contributor

@cv cv commented May 4, 2026

Summary

Move pure CLI/domain helper modules out of the flat src/lib/ directory into src/lib/domain/** so parser, action, and domain boundaries are visible in paths.

Stack Navigation

Changes

  • Moved lifecycle option normalizers to src/lib/domain/lifecycle/options.ts.
  • Moved duration parsing, maintenance image/upgrade helpers, sandbox destroy/log helpers, sandbox log options, and policy/channel arg helpers under src/lib/domain/**.
  • Moved helper tests with their modules and updated imports from actions, command adapters, and tests.
  • Kept behavior unchanged; this PR is a structural move plus import updates.

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 --stage pre-push passes
  • npm run build:cli
  • npm run typecheck:cli
  • Domain/helper targeted tests pass
  • 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

  • Refactor
    • Internal module structure reorganized to improve code organization and maintainability.

cv added 30 commits May 2, 2026 13:36
@cv cv self-assigned this May 4, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 4, 2026

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
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 2026

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: 234cf970-033e-40a0-9f52-b8cae83f1c49

📥 Commits

Reviewing files that changed from the base of the PR and between 5ba48f4 and 79e05ff.

📒 Files selected for processing (29)
  • src/lib/commands/sandbox/logs.ts
  • src/lib/coverage-hotspots.test.ts
  • src/lib/domain/duration.test.ts
  • src/lib/domain/duration.ts
  • src/lib/domain/lifecycle/options.test.ts
  • src/lib/domain/lifecycle/options.ts
  • src/lib/domain/maintenance/images.test.ts
  • src/lib/domain/maintenance/images.ts
  • src/lib/domain/maintenance/upgrade.test.ts
  • src/lib/domain/maintenance/upgrade.ts
  • src/lib/domain/policy-channel.test.ts
  • src/lib/domain/policy-channel.ts
  • src/lib/domain/sandbox/destroy.test.ts
  • src/lib/domain/sandbox/destroy.ts
  • src/lib/domain/sandbox/log-options.ts
  • src/lib/domain/sandbox/logs.test.ts
  • src/lib/domain/sandbox/logs.ts
  • src/lib/duration-flags.ts
  • src/lib/global-cli-actions.ts
  • src/lib/maintenance-actions.ts
  • src/lib/policy-channel-actions.ts
  • src/lib/sandbox-destroy-action.ts
  • src/lib/sandbox-logs-action.ts
  • src/lib/sandbox-rebuild-action.ts
  • src/lib/sandbox-runtime-actions.ts
  • src/lib/shields.ts
  • src/lib/upgrade-sandboxes-action.ts
  • test/image-cleanup.test.ts
  • test/shields.test.ts

📝 Walkthrough

Walkthrough

The PR reorganizes helper modules from src/lib/ into a hierarchical src/lib/domain/ structure, consolidating functionality by domain. All import paths across the codebase are updated to reflect the new locations. One functional change: stripAnsi is inlined as a local utility in the sandbox destroy module instead of importing from openshell.

Changes

Module Reorganization and Import Path Updates

Layer / File(s) Summary
Domain Module Structure
src/lib/domain/lifecycle/options.ts, src/lib/domain/maintenance/images.ts, src/lib/domain/maintenance/upgrade.ts, src/lib/domain/sandbox/destroy.ts, src/lib/domain/sandbox/logs.ts, src/lib/domain/sandbox/log-options.ts, src/lib/domain/duration.ts, src/lib/domain/policy-channel.ts
Helper modules consolidated into domain subdirectories. Sandbox destroy module inlines ANSI_RE regex and local stripAnsi function (previously imported from openshell).
Action Layer Consumer Updates
src/lib/sandbox-destroy-action.ts, src/lib/sandbox-logs-action.ts, src/lib/sandbox-rebuild-action.ts, src/lib/upgrade-sandboxes-action.ts, src/lib/maintenance-actions.ts, src/lib/policy-channel-actions.ts, src/lib/sandbox-runtime-actions.ts
Import paths updated to pull helpers from new domain module locations (e.g., ./domain/lifecycle/options, ./domain/sandbox/destroy).
Utility Layer Updates
src/lib/duration-flags.ts, src/lib/shields.ts, src/lib/global-cli-actions.ts, src/lib/commands/sandbox/logs.ts
Import paths for domain utilities (duration, lifecycle options, sandbox logs options) updated to reference new domain structure.
Test Layer Updates
src/lib/domain/lifecycle/options.test.ts, src/lib/domain/maintenance/images.test.ts, src/lib/domain/maintenance/upgrade.test.ts, src/lib/domain/sandbox/destroy.test.ts, src/lib/domain/sandbox/logs.test.ts, src/lib/domain/policy-channel.test.ts, src/lib/domain/duration.test.ts, src/lib/coverage-hotspots.test.ts, test/image-cleanup.test.ts, test/shields.test.ts
Test import paths updated to reflect new module organization. test/shields.test.ts switches to dynamic imports for parseDuration within test cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 Our warren reorganizes with care,
Domain modules now arranged with flair,
Helpers nestled in structures so neat,
Import paths dancing to a rhythmic beat,
One stripAnsi inlined, the refactor's complete! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% 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 PR title clearly summarizes the main structural change: moving pure domain helpers into a domain-scoped directory organization.
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/layer-domain-helpers

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

@cv cv added the v0.0.34 Release target label May 4, 2026
@wscurran wscurran added NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). refactor This is a refactor of the code and/or architecture. and removed v0.0.34 Release target labels May 4, 2026
@cv cv requested a review from prekshivyas May 5, 2026 00:27
@cv cv added the v0.0.35 Release target label May 5, 2026
@prekshivyas prekshivyas self-assigned this May 5, 2026
cv added a commit that referenced this pull request May 6, 2026
## Summary
Split the oclif command adapter layer out of the flat `src/lib/`
directory and mirror the command tree under `src/lib/commands/**`.

## Stack Navigation
- Position: 53 of 60
- Previous PR: [#2967 — refactor(cli): group oclif core
modules](#2967)
- Next PR: [#2984 — refactor(cli): group pure domain
helpers](#2984)

## Changes
- Moved the explicit oclif registry to `src/lib/commands/index.ts` and
updated the oclif target in `package.json`.
- Split registered oclif command classes to one command per production
file, grouped by command family and sandbox scope.
- Moved command adapter tests beside the new command tree and updated
imports/mocks.
- Kept behavior unchanged; command IDs, public syntax, parser flags, and
action calls are preserved.

## 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 --stage pre-push` passes
- [x] `npm run clean:cli && npm run build:cli && npm run
dist:sourcemaps:check`
- [x] `npm run typecheck:cli`
- [x] Command adapter and metadata tests pass
- [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 (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>
@cv cv marked this pull request as ready for review May 6, 2026 00:13
@cv cv changed the base branch from refactor/layer-command-tree to main May 6, 2026 00:13
@cv cv enabled auto-merge (squash) May 6, 2026 00:13
Copy link
Copy Markdown
Contributor

@prekshivyas prekshivyas left a comment

Choose a reason for hiding this comment

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

LGTM. Same #2967-pattern PR — pure file-organization, 29 files / +41 / -37.

Eight pure-domain helper modules (lifecycle/options, duration, policy-channel, maintenance/images, maintenance/upgrade, sandbox/destroy, sandbox/logs, sandbox/log-options) moved under src/lib/domain/**. 10 consumer files (actions + rebuild) have one-line import path updates. Test files renamed alongside their sources.

Most files are 0+/0- pure renames; the +1/-1 files are import-path migrations. Slightly larger domain/sandbox/destroy.ts (+5/-1) and test/shields.test.ts (+5/-5) are import re-exports / multi-import updates.

CI: pr.yaml mostly green (lint/dco/check-hash/legacy-path-guard/changes PASS); CodeRabbit SUCCESS; macos-e2e/wsl-e2e/checks + pr-self-hosted builds still in flight at review time. No failures.

@cv cv merged commit 10d43f2 into main May 6, 2026
13 checks passed
cv added a commit that referenced this pull request May 6, 2026
## Summary
Move OpenShell-facing adapter modules out of the flat `src/lib/`
directory into `src/lib/adapters/openshell/**` so process/runtime
boundaries are visible in paths.

## Stack Navigation
- Position: 55 of 60
- Previous PR: [#2984 — refactor(cli): group pure domain
helpers](#2984)
- Next PR: [#2986 — test(cli): enforce initial layer import
boundaries](#2986)

## Changes
- Moved OpenShell command helpers to
`src/lib/adapters/openshell/client.ts`.
- Moved OpenShell runtime wrapper helpers to
`src/lib/adapters/openshell/runtime.ts`.
- Moved OpenShell binary resolution and timeout constants to
`resolve.ts` and `timeouts.ts`.
- Moved associated tests and updated action, command, and
integration-test imports.
- Kept behavior unchanged; this PR is a structural move plus import
updates.

## 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 --stage pre-push` passes
- [x] `npm run build:cli`
- [x] `npm run typecheck:cli`
- [x] OpenShell adapter targeted tests pass
- [x] `npm run ts-migration:guard -- --base
origin/refactor/layer-domain-helpers --head HEAD`
- [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 (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>
@coderabbitai coderabbitai Bot mentioned this pull request May 6, 2026
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). refactor This is a refactor of the code and/or architecture. v0.0.35 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants