Skip to content

test(cli): cover sandbox oclif command adapters - #2938

Merged
cv merged 43 commits into
mainfrom
refactor/coverage-sandbox-oclif-adapters
May 5, 2026
Merged

test(cli): cover sandbox oclif command adapters#2938
cv merged 43 commits into
mainfrom
refactor/coverage-sandbox-oclif-adapters

Conversation

@cv

@cv cv commented May 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remove broad coverage ignores from sandbox oclif adapters whose parser behavior can now be tested in-process.

Stack Navigation

Changes

  • Removed file-level V8 ignores from sandbox connect, lifecycle, inspection, doctor, logs, policy/channel, snapshot, skill, and shields adapters.
  • Added direct command-adapter coverage for connect/lifecycle option mapping, inspection actions, doctor flags, and shields flags.
  • Reused existing command-family tests for logs, policy/channel mutation, snapshot, and skill adapters.

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

    • Introduced sandbox:doctor command for sandbox and gateway health diagnostics
    • Added parent commands for skill and snapshot operations
    • Added --tail/-n and --since options to logs command
  • Improvements

    • Channel argument now required for all channel mutation commands
    • Format flag for config get strictly validates to json or yaml options
    • Added -y shorthand for yes flag in destroy and rebuild commands
    • CLI commands now display helpful usage examples

cv added 30 commits May 2, 2026 13:36
@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: 0b402844-f00b-4b11-91df-02546480a8c4

📥 Commits

Reviewing files that changed from the base of the PR and between 8a6ecbc and 6133caa.

📒 Files selected for processing (1)
  • src/lib/skill-install-cli-command.ts

📝 Walkthrough

Walkthrough

The PR updates CLI command infrastructure across multiple adapter files: requiring arguments in channel commands, adding CLI usage examples to 11 commands, migrating 2 commands to NemoClawCommand, introducing a new sandbox:doctor command, extending the logs runtime bridge to support tail and since options, and adding comprehensive test coverage for CLI command adapters.

Changes

CLI Command Metadata and Argument Updates

Layer / File(s) Summary
Argument Requirements
src/lib/channels-mutate-cli-commands.ts
channel argument made required (required: true) for all four channel mutation commands.
New Sandbox Doctor Command
src/lib/sandbox-doctor-cli-command.ts
New SandboxDoctorCliCommand class added with required sandboxName argument, optional --json flag, and implementation that invokes runSandboxDoctor with diagnostics output.
Base Class Migrations
src/lib/destroy-cli-command.ts, src/lib/rebuild-cli-command.ts
Both DestroyCliCommand and RebuildCliCommand migrated from Command to NemoClawCommand base class; yes flag gains -y shorthand in both.
Runtime Bridge Interface Updates
src/lib/sandbox-logs-cli-command.ts
SandboxLogsRuntimeBridge.sandboxLogs signature expanded from (sandboxName: string, follow: boolean) to (sandboxName: string, options: SandboxLogsOptions) to support follow, lines (tail), and since parameters.
Options Object Wiring
src/lib/destroy-cli-command.ts, src/lib/rebuild-cli-command.ts
run() methods updated to pass typed options objects ({ force, yes } and { force, verbose, yes } respectively) to sandbox actions instead of legacy string arrays.
Flag Refinements
src/lib/policy-mutate-cli-commands.ts, src/lib/shields-cli-commands.ts
PolicyAddCommand flags --from-file and --from-dir marked mutually exclusive; ShieldsDownCommand.timeout switched to use shieldsTimeoutDurationFlag helper.
Inspection Command Improvements
src/lib/sandbox-inspection-cli-command.ts
SandboxConfigGetCommand.format flag updated with constrained options: ["json", "yaml"]; manual runtime validation removed.
New Skill and Snapshot Commands
src/lib/skill-install-cli-command.ts, src/lib/snapshot-cli-commands.ts
Added SkillCliCommand and SnapshotCommand as parent commands; made skillPath argument required in SkillInstallCliCommand.
CLI Examples and Documentation
src/lib/channels-mutate-cli-commands.ts, src/lib/connect-cli-command.ts, src/lib/policy-mutate-cli-commands.ts, src/lib/sandbox-inspection-cli-command.ts, src/lib/sandbox-logs-cli-command.ts, src/lib/skill-install-cli-command.ts, src/lib/snapshot-cli-commands.ts
Added or expanded static examples arrays to 11 commands with representative CLI invocations; removed v8 ignore coverage suppression comments from 5 files.
Test Coverage
src/lib/sandbox-oclif-command-adapters.test.ts
New Vitest suite with 3 test cases verifying CLI flag-to-option mapping for connect/destroy/rebuild, inspection commands, doctor, and shields commands.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#2816: Concurrent refactoring of the same CLI adapter files and runtime-bridge dependencies, updating imports and usages alongside command metadata changes.

Suggested reviewers

  • cjagwani

Poem

🐰 CLI commands now dance with grace,
Examples guide each case,
Options flow where strings once lay,
Doctor checks the sandbox way,
A healthier interface, hip hip hooray! 🎉

✨ 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/coverage-sandbox-oclif-adapters

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 CI/CD refactor PR restructures code without intended behavior change 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
Remove broad coverage ignores from low-risk global oclif adapters and
cover their parser-to-action mapping directly.

## Stack Navigation
- Position: 39 of 60
- Previous PR: [#2936 — test(cli): cover oclif metadata routing
helpers](#2936)
- Next PR: [#2938 — test(cli): cover sandbox oclif command
adapters](#2938)

## Changes
- Removed file-level V8 ignores from list, status, maintenance, and
onboard oclif adapters.
- Added direct source-level tests for list/status adapter helpers and
maintenance typed option mapping.
- Covered onboard/setup/setup-spark compatibility argv generation
through mocked action facades.

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

* **Tests**
* Added comprehensive test coverage for command adapter behavior
verification.

* **Chores**
  * Code cleanup and maintenance improvements to internal directives.

<!-- 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 20:35
@cv
cv changed the base branch from refactor/coverage-global-oclif-adapters to main May 5, 2026 20:35
@cv
cv enabled auto-merge (squash) May 5, 2026 20:35
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv merged commit a3ee273 into main May 5, 2026
11 checks passed
cv added a commit that referenced this pull request May 5, 2026
## Summary
Split share oclif command classes out of the SSHFS action/helper module
so parser adapters and action logic are independently testable.

## Stack Navigation
- Position: 41 of 60
- Previous PR: [#2938 — test(cli): cover sandbox oclif command
adapters](#2938)
- Next PR: [#2940 — test(cli): narrow sandbox logs coverage
ignores](#2940)

## Changes
- Added `share-cli-commands.ts` for the share parent and
mount/unmount/status oclif adapters.
- Left SSHFS action helpers in `share-command.ts`.
- Updated the oclif command registry to import share command classes
from the new adapter module.
- Added direct share adapter tests with mocked share actions.

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

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv
cv deleted the refactor/coverage-sandbox-oclif-adapters branch May 27, 2026 21:17
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: cli Command line interface, flags, terminal UX, or output area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance feature PR adds or expands user-visible functionality and removed CI/CD refactor PR restructures code without intended behavior change feature PR adds or expands user-visible functionality labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: cli Command line interface, flags, terminal UX, or output area: e2e End-to-end tests, nightly failures, or validation infrastructure chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants