refactor(cli): reduce command adapter migration debt by cv · Pull Request #3833 · NVIDIA/NemoClaw

cv · 2026-05-19T22:02:26Z

Summary

Refactors CLI command adapters to keep the sandbox-first public grammar while reducing migration-era shim and argv translation debt. The changes move more command metadata and typed action inputs closer to oclif command definitions, and prevent source code from depending on packaged bin shims.

Changes

Import agent runtime implementation modules directly from src/lib/agent/runtime and add a layer check preventing non-JSON bin/lib/* shim imports from src/**.
Replace policy, channel, snapshot, and skill command argv reconstruction with typed action option objects.
Move selected sandbox public display metadata from public-display-defaults.ts onto the relevant oclif command classes.
Derive the direct global command fallback set from registered oclif metadata instead of maintaining a parallel hard-coded list.
Update command adapter tests to assert typed action options and direct action calls.

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
- CLI commands gained richer public help/metadata for improved discoverability.
- Sandbox channel, policy, snapshot, skill, and install workflows now accept structured option objects for clearer, typed inputs.
Refactor
- Internal command dispatch simplified and modernized imports to ESM.
- Public argument translation and display defaults updated for a sandbox-first experience.
Tests
- Test suites updated to assert typed option payloads instead of legacy argv arrays.

coderabbitai · 2026-05-19T22:02:40Z

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: de140a3e-a8a6-4623-892f-0856668d7d9a

📥 Commits

Reviewing files that changed from the base of the PR and between 58367c5 and 603b5e3.

📒 Files selected for processing (1)

src/lib/cli/command-registry.ts

🚧 Files skipped from review as they are similar to previous changes (1)

src/lib/cli/command-registry.ts

📝 Walkthrough

Walkthrough

This PR converts many sandbox CLI flows from legacy argv arrays to structured typed option/request objects, adds per-command publicDisplay metadata, migrates agentRuntime imports to ES modules, updates command-support helpers, refactors snapshot/skill actions, updates routing/dispatch/token logic, and adds a layer-import check for bin/lib shim imports.

Changes

Sandbox CLI Refactoring: Actions to Typed Options

Layer / File(s)	Summary
Import boundary validation `scripts/checks/layer-import-boundaries.ts`	Adds validation to detect and flag imports from `bin/lib/*` shim modules (non-.json) as `src-no-bin-lib-shims` violations.
AgentRuntime ES module migration `src/lib/actions/sandbox/connect.ts`, `doctor.ts`, `process-recovery.ts`, `rebuild.ts`, `skill-install.ts`, `status.ts`	Replaced CommonJS `require(...)` with ES module `import * as agentRuntime` from `../../agent/runtime`.
Typed option types and parsing `src/lib/domain/policy-channel.ts`, `src/lib/domain/policy-channel.test.ts`	Introduces `PolicyAddOptions`, `PolicyRemoveOptions`, and `ChannelMutationOptions`; adds `parsePolicyAddOptions` and helpers to map structured options into existing internal shapes and validations.
Action function refactoring `src/lib/actions/sandbox/policy-channel.ts`	Updates `addSandboxPolicy`, `removeSandboxPolicy`, `addSandboxChannel`, `removeSandboxChannel`, `startSandboxChannel`, `stopSandboxChannel` to accept typed option objects instead of argv arrays.
Snapshot and skill action refactoring `src/lib/actions/sandbox/snapshot.ts`, `src/lib/actions/sandbox/skill-install.ts`	Adds `SnapshotRequest` and `SkillInstallRequest` types; updates `runSandboxSnapshot` and `installSandboxSkill` to accept request objects and remove sub-arg parsing.
Command support module updates `src/lib/sandbox/channels-command-support.ts`, `src/lib/sandbox/policy-command-support.ts`, `src/lib/sandbox/snapshot-command-support.ts`	Replaces argv-building helpers with `channelMutationOptions(...)` and `commonPolicyOptions(...)`; updates runtime bridge method signatures; adds `snapshotCommandError` and `sandboxNameArg`.
CLI commands with publicDisplay metadata `src/commands/sandbox/channels/.ts`, `src/commands/sandbox/policy/.ts`, `src/commands/sandbox/skill/.ts`, `src/commands/sandbox/snapshot/.ts`	Adds `static publicDisplay` metadata to command classes and updates `run()` to call the action layer with typed options, adjusting imports and tests.
CLI routing and dispatch refactoring `src/lib/cli/public-argv-translation.ts`, `src/lib/cli/public-dispatch.ts`, `src/lib/cli/command-registry.ts`, `src/lib/cli/oclif-metadata.ts`, `src/lib/cli/command-display.ts`, `src/lib/cli/public-route-metadata.ts`	Switched to `publicTokens`-based sandbox route translation, added `directGlobalCommandIds()` and updated dispatch gating to derive direct/global command IDs dynamically; updated comments to sandbox-first terminology.
Hardcoded display defaults cleanup `src/lib/cli/public-display-defaults.ts`	Removed hardcoded `PUBLIC_DISPLAY_LAYOUT` entries for sandbox channels, policy, skill install, and snapshot commands so metadata is derived from command classes.
Test updates `src/commands/sandbox/channels/mutate.test.ts`, `src/commands/sandbox/policy/mutate.test.ts`, `src/commands/sandbox/skill.test.ts`, `src/commands/sandbox/snapshot.test.ts`, `test/channels-add-preset.test.ts`	Refactored tests to mock action-layer functions directly (vi.mock/hoisted), expect typed option/request objects, and updated call assertions and mock lifecycle handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

NVIDIA/NemoClaw#3392: Overlaps on channel/preset behavior and channel mutation code changes in policy-channel.ts.
NVIDIA/NemoClaw#3635: Related changes to public dispatch/command-registry and direct command ID derivation.
NVIDIA/NemoClaw#3452: Also modifies addSandboxChannel behavior and presets, touching the same action in policy-channel.ts.

Suggested labels

NemoClaw CLI, refactor, status: rfr

Suggested reviewers

ericksoa
cjagwani
jyaunches

"🐰 In argv arrays once secrets slept,
typed options wake where bytes once crept.
Commands now speak with clearer tone,
snapshots and skills find structured home.
Hooray — the sandbox runs on types well-kept!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 17.95% 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 summarizes the main change: refactoring CLI command adapters to reduce migration-era technical debt through modernization of command handling patterns.
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/cli-migration-debt

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

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

github-actions · 2026-05-19T22:07:11Z

E2E Advisor Recommendation

Required E2E: channels-stop-start-e2e, network-policy-e2e, snapshot-commands-e2e, skill-agent-e2e, sandbox-operations-e2e
Optional E2E: diagnostics-e2e, rebuild-openclaw-e2e, docs-validation-e2e

Dispatch hint: channels-stop-start-e2e,network-policy-e2e,snapshot-commands-e2e,skill-agent-e2e,sandbox-operations-e2e

Auto-dispatched E2E: channels-stop-start-e2e, network-policy-e2e, snapshot-commands-e2e, skill-agent-e2e, sandbox-operations-e2e via nightly-e2e.yaml at 603b5e3e4db1df537d6a6dc96be5dd5b20fcb68f — nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

channels-stop-start-e2e (high (~120 min timeout)): Directly covers channels add/remove/start/stop lifecycle across OpenClaw/Hermes, fake messaging credentials, gateway provider detach/reattach, policy preset side effects, and rebuild survival after the typed channel action API change.
network-policy-e2e (medium (~45 min timeout)): Required because policy add/remove/list and domain option parsing changed in code that enforces network policy boundaries; this job validates deny-by-default, whitelist presets, live policy-add, dry-run, hot reload, inference exemption, permissive mode, and SSRF validation.
snapshot-commands-e2e (medium (~30 min timeout)): Required because snapshot create/list/restore command and action interfaces changed; this job validates live snapshot creation, listing, restore by latest/selector, workspace recovery, and credential exclusion from snapshots.
skill-agent-e2e (medium (~30 min timeout)): Required because skill install command dispatch now calls the action directly with typed options; this job exercises a real SKILL.md install and verifies the assistant can use the installed skill in a live sandbox.
sandbox-operations-e2e (high (~60 min timeout)): Required because connect/status/process recovery/rebuild and public dispatch plumbing changed; this job validates sandbox list, connect/chat, status fields, logs, registry rebuild, process recovery, gateway recovery, destroy, multi-sandbox metadata, and cross-sandbox isolation.

Optional E2E

diagnostics-e2e (medium (~45 min timeout)): Useful adjacent confidence for changed sandbox status/doctor-related diagnostics and CLI output metadata; existing diagnostics coverage checks version, debug archives, credential redaction, sandbox config, and host status output, but it does not appear to specifically exercise doctor.
rebuild-openclaw-e2e (high (~60 min timeout)): Useful adjacent confidence because rebuild.ts and channel/policy rebuild prompts were touched; sandbox-operations covers registry/process recovery, while this validates state survival through a real OpenClaw rebuild/upgrade path.
docs-validation-e2e (low/medium): Optional because publicDisplay/root-help metadata changed for policy, channels, skill, and snapshot commands; this can catch CLI docs/help parity issues but is not the primary runtime risk.

New E2E recommendations

sandbox doctor command (medium): src/lib/actions/sandbox/doctor.ts changed, but existing inspected E2E coverage appears to exercise status/debug diagnostics rather than nemoclaw <name> doctor or equivalent doctor-specific checks.
- Suggested test: Add a doctor diagnostics E2E that onboards a sandbox, runs the doctor command in healthy and degraded states, validates JSON/text report structure, provider health reporting, and actionable nonzero behavior for failures.
public sandbox-first command grammar (medium): The PR adds/changes publicDisplay and public route/argv translation metadata for policy, channels, skill, and snapshot commands, but existing runtime E2E scripts mostly invoke established command aliases and may not cover every public grammar form shown in root help.
- Suggested test: Add a public CLI grammar E2E that dispatches representative sandbox-first forms from root help, including <name> channels add/remove/start/stop, <name> policy-add/policy-remove, <name> snapshot create/list/restore, and <name> skill install, asserting they route to the intended oclif commands.

Dispatch hint

Workflow: nightly-e2e.yaml
jobs input: channels-stop-start-e2e,network-policy-e2e,snapshot-commands-e2e,skill-agent-e2e,sandbox-operations-e2e

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/lib/actions/sandbox/policy-channel.ts (1)
41-44: 💤 Low value

Consider importing ChannelMutationOptions from channels-command-support.ts to avoid type drift.

This type is defined identically in src/lib/sandbox/channels-command-support.ts. Duplicating type definitions risks them diverging during future maintenance. Since the action layer already imports from the domain layer, importing from the command-support module (or extracting to a shared types file) would ensure consistency.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/actions/sandbox/policy-channel.ts` around lines 41 - 44, Replace the
duplicated ChannelMutationOptions type in policy-channel.ts with an import from
the existing definition in channels-command-support.ts: remove the local type
alias and add an import for ChannelMutationOptions from the module that exports
it (channels-command-support.ts or a shared types module), then update any
references in policy-channel.ts to use the imported ChannelMutationOptions to
prevent type drift between layers.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/lib/cli/command-registry.ts`:
- Around line 142-149: The JSDoc above directGlobalCommandIds is stale and
misleading (it references sandbox action tokens); update the comment to
accurately describe that directGlobalCommandIds() returns a Set<string> of leaf
global command IDs (derived from oclif command IDs and explicit public route
overrides) used for dispatch decisions, and remove or replace references to
"sandbox" and "action tokens" so the documentation matches the function purpose.

---

Nitpick comments:
In `@src/lib/actions/sandbox/policy-channel.ts`:
- Around line 41-44: Replace the duplicated ChannelMutationOptions type in
policy-channel.ts with an import from the existing definition in
channels-command-support.ts: remove the local type alias and add an import for
ChannelMutationOptions from the module that exports it
(channels-command-support.ts or a shared types module), then update any
references in policy-channel.ts to use the imported ChannelMutationOptions to
prevent type drift between layers.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ba3ba010-5816-41a0-82f1-fca4f6bed7b4

📥 Commits

Reviewing files that changed from the base of the PR and between 32fac96 and 58367c5.

📒 Files selected for processing (41)

scripts/checks/layer-import-boundaries.ts
src/commands/sandbox/channels/add.ts
src/commands/sandbox/channels/list.ts
src/commands/sandbox/channels/mutate.test.ts
src/commands/sandbox/channels/remove.ts
src/commands/sandbox/channels/start.ts
src/commands/sandbox/channels/stop.ts
src/commands/sandbox/policy/add.ts
src/commands/sandbox/policy/list.ts
src/commands/sandbox/policy/mutate.test.ts
src/commands/sandbox/policy/remove.ts
src/commands/sandbox/skill.test.ts
src/commands/sandbox/skill.ts
src/commands/sandbox/skill/install.ts
src/commands/sandbox/snapshot.test.ts
src/commands/sandbox/snapshot.ts
src/commands/sandbox/snapshot/create.ts
src/commands/sandbox/snapshot/list.ts
src/commands/sandbox/snapshot/restore.ts
src/lib/actions/sandbox/connect.ts
src/lib/actions/sandbox/doctor.ts
src/lib/actions/sandbox/policy-channel.ts
src/lib/actions/sandbox/process-recovery.ts
src/lib/actions/sandbox/rebuild.ts
src/lib/actions/sandbox/skill-install.ts
src/lib/actions/sandbox/snapshot.ts
src/lib/actions/sandbox/status.ts
src/lib/cli/command-display.ts
src/lib/cli/command-registry.ts
src/lib/cli/oclif-metadata.ts
src/lib/cli/public-argv-translation.ts
src/lib/cli/public-dispatch.ts
src/lib/cli/public-display-defaults.ts
src/lib/cli/public-route-metadata.ts
src/lib/domain/policy-channel.test.ts
src/lib/domain/policy-channel.ts
src/lib/sandbox/channels-command-support.ts
src/lib/sandbox/policy-command-support.ts
src/lib/sandbox/skill-command-support.ts
src/lib/sandbox/snapshot-command-support.ts
test/channels-add-preset.test.ts

💤 Files with no reviewable changes (3)

src/lib/cli/public-display-defaults.ts
src/lib/sandbox/skill-command-support.ts
src/lib/sandbox/snapshot-command-support.ts

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

github-actions · 2026-05-19T22:39:14Z

Selective E2E Results — ✅ All requested jobs passed

Run: 26128228501
Target ref: 58367c50b34368817a2682d206b6c816edd8abb4
Workflow ref: main
Requested jobs: channels-stop-start-e2e,network-policy-e2e,snapshot-commands-e2e,skill-agent-e2e,sandbox-operations-e2e,diagnostics-e2e
Summary: 5 passed, 0 failed, 0 skipped

Job	Result
channels-stop-start-e2e	⚠️ cancelled
diagnostics-e2e	✅ success
network-policy-e2e	✅ success
sandbox-operations-e2e	✅ success
skill-agent-e2e	✅ success
snapshot-commands-e2e	✅ success

ericksoa

Reviewed head 603b5e3. CodeRabbit thread is resolved, focused CLI validation passed locally, and the refactor keeps the public dispatch/typed action behavior intact.

github-actions · 2026-05-19T23:11:59Z

Selective E2E Results — ❌ Some jobs failed

Run: 26129578640
Target ref: 603b5e3e4db1df537d6a6dc96be5dd5b20fcb68f
Workflow ref: main
Requested jobs: channels-stop-start-e2e,network-policy-e2e,snapshot-commands-e2e,skill-agent-e2e,sandbox-operations-e2e
Summary: 4 passed, 1 failed, 0 skipped

Job	Result
channels-stop-start-e2e	❌ failure
network-policy-e2e	✅ success
sandbox-operations-e2e	✅ success
skill-agent-e2e	✅ success
snapshot-commands-e2e	✅ success

Failed jobs: channels-stop-start-e2e. Check run artifacts for logs.

cv added 5 commits May 19, 2026 14:43

refactor(cli): remove initial adapter migration debt

6933d19

refactor(cli): pass typed channel mutation options

ade41f4

refactor(cli): call snapshot and skill actions directly

c97529b

refactor(cli): move sandbox display metadata to commands

1be69ee

refactor(cli): derive direct global dispatch fallback

da96d12

cv self-assigned this May 19, 2026

Merge branch 'main' into refactor/cli-migration-debt

58367c5

cv added the v0.0.46 Release target label May 19, 2026

cv requested review from cjagwani and ericksoa May 19, 2026 22:04

coderabbitai Bot reviewed May 19, 2026

View reviewed changes

Comment thread src/lib/cli/command-registry.ts

docs(cli): clarify direct global command registry

603b5e3

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>

ericksoa approved these changes May 19, 2026

View reviewed changes

ericksoa merged commit 9e00c58 into main May 19, 2026
30 of 31 checks passed

ericksoa deleted the refactor/cli-migration-debt branch May 19, 2026 23:01

coderabbitai Bot mentioned this pull request May 19, 2026

refactor(cli): preserve display registry and drop test bridges #3838

Merged

12 tasks

wscurran added the refactor PR restructures code without intended behavior change label Jun 8, 2026

Conversation

cv commented May 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of Change

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

github-actions Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E Advisor Recommendation

E2E Recommendation Advisor

Required E2E

Optional E2E

New E2E recommendations

Dispatch hint

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 19, 2026

Selective E2E Results — ✅ All requested jobs passed

Uh oh!

ericksoa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 19, 2026

Selective E2E Results — ❌ Some jobs failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cv commented May 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 19, 2026 •

edited

Loading

github-actions Bot commented May 19, 2026 •

edited

Loading