refactor(cli): remove legacy dispatch fallbacks

[cv]

Summary

Remove the remaining legacy dispatch fallback targets for policy-add, skill, and snapshot edge paths. These paths now route through hidden/raw oclif adapters that preserve the existing command-action behavior without using the legacy dispatch variant.

Stack Navigation

• Position: 10 of 60
• Previous PR: #2898 — refactor(cli): remove runtime bridge
• Next PR: #2901 — refactor(cli): expose explicit main entrypoint

Changes

• Added raw oclif adapters for policy-add missing-value handling, skill usage/unknown subcommands, and snapshot usage/unknown subcommands.
• Registered the raw adapters in the oclif command map.
• Removed LegacyDispatch and the legacy handling branch from src/nemoclaw.ts.
• Added dispatch tests covering the newly routed policy, skill, and snapshot fallback paths.

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

• Refactoring

  • Streamlined sandbox command routing so help and unknown subcommands are forwarded consistently and original arguments are preserved.
  • Unified behavior for policy, skill, and snapshot commands to improve predictability.

• New Features

  • Sandbox commands now accept raw argument forms and validate missing sandbox names with clearer exit behavior.

• Tests

  • Added coverage for the updated dispatch and routing behaviors.

[copy-pr-bot]

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]

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: 9dd0fd03-7755-4cb4-86fe-e94beb6f119c

📥 Commits

Reviewing files that changed from the base of the PR and between 3a3a65d and d3e97f4.

📒 Files selected for processing (3)

• src/lib/policy-mutate-cli-commands.ts
• src/lib/skill-install-cli-command.ts
• src/lib/snapshot-cli-commands.ts

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

• src/lib/snapshot-cli-commands.ts

---

📝 Walkthrough

Walkthrough

Removed legacy dispatch type/branching and migrated sandbox actions (policy-add, skill, snapshot) to new non-strict oclif command entrypoints; resolver now returns oclif targets and runtime executor no longer handles legacy dispatches. Tests added to assert arg-preserving oclif routing.

Changes

Legacy Dispatch Migration to Oclif Commands

Layer / File(s)	Summary
Type & Export Removals src/lib/legacy-oclif-dispatch.ts	Removed exported LegacyDispatch type and removed it from the DispatchResult union.
Dispatch Resolution src/lib/legacy-oclif-dispatch.ts	resolveSandboxOclifDispatch now maps policy-add, skill, and unknown snapshot cases to oclif targets (sandbox:policy-add:raw, sandbox:skill, sandbox:snapshot) instead of returning legacy dispatch values.
New Oclif Command Implementations src/lib/policy-mutate-cli-commands.ts, src/lib/skill-install-cli-command.ts, src/lib/snapshot-cli-commands.ts	Added non-strict, argv-forwarding oclif commands: PolicyAddRawCommand (sandbox:policy-add:raw), SkillCliCommand (sandbox:skill), and SnapshotCommand (sandbox:snapshot). Each validates a non-empty sandbox name and forwards remaining argv to the runtime bridge handlers.
Command Registry Updates src/lib/oclif-commands.ts	Imported and registered PolicyAddRawCommand and SkillCliCommand in the default oclif command map (retaining sandbox:skill:install).
Runtime Dispatch Handler src/nemoclaw.ts	Removed the "legacy" branch from runDispatchResult; runtime no longer directly routes legacy action targets.
Tests src/lib/legacy-oclif-dispatch.test.ts	Added tests asserting: policy-add routes to sandbox:policy-add:raw with preserved args; skill routes help/unknown subcommands to sandbox:skill preserving args; unknown snapshot routes to sandbox:snapshot preserving args.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant CLI
  participant Resolver
  participant OclifCmd as OclifCommand
  participant Runtime as RuntimeBridge

  CLI->>Resolver: parse action (policy-add | skill | snapshot) + argv
  Resolver->>CLI: return oclif dispatch (e.g., sandbox:policy-add:raw)
  CLI->>OclifCmd: invoke command id with raw argv (first = sandboxName, rest = actionArgs)
  OclifCmd->>Runtime: sandboxPolicyAdd / sandboxSkillInstall / sandboxSnapshot(sandboxName, actionArgs)
  Runtime-->>CLI: action result

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I hopped through code, clean and spry,
Old paths trimmed, new routes fly by,
Three little commands now take the lead,
Args preserved, no tangled weed,
A sandbox hummed beneath the sky.

🚥 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 pull request title 'refactor(cli): remove legacy dispatch fallbacks' accurately summarizes the main change: eliminating legacy dispatch fallback targets and replacing them with hidden oclif adapters.
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-remove-legacy-dispatch-fallbacks

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 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/policy-mutate-cli-commands.ts`:
- Around line 84-87: The run() method currently reads const [sandboxName,
...actionArgs] = this.argv and calls getRuntimeBridge().sandboxPolicyAdd without
validating sandboxName; add a guard at the start of run() that verifies
sandboxName is a non-empty string and, if missing, fails fast with a CLI usage
error (e.g., throw an informative Error or print a usage message and exit)
before calling getRuntimeBridge().sandboxPolicyAdd(sandboxName, actionArgs) so
invalid/empty sandbox names are never dispatched downstream.

In `@src/lib/skill-install-cli-command.ts`:
- Around line 29-32: The run() method extracts sandboxName from this.argv but
doesn't validate it before calling getRuntimeBridge().sandboxSkillInstall, which
can lead to missing-argument errors; update run() (in class/method where run is
declared) to check that sandboxName is present and non-empty, and if not throw
or return a deterministic error (or call this.error/reporting helper) rather
than invoking getRuntimeBridge().sandboxSkillInstall with undefined; ensure the
validation message clearly states the missing sandboxName and include the
original actionArgs only when sandboxName is valid.

In `@src/lib/snapshot-cli-commands.ts`:
- Around line 35-38: The run() method should validate that a sandbox name was
provided before dispatching; check this.argv for a present sandboxName (the
destructured const [sandboxName, ...actionArgs] = this.argv) and if sandboxName
is falsy, emit a usage/error message and stop (throw or return non-zero exit)
instead of calling getRuntimeBridge().sandboxSnapshot(sandboxName, actionArgs);
add this guard at the top of run() so sandboxSnapshot is only invoked with a
valid sandboxName.

🪄 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: 23106aa5-0538-4d0b-ab7a-821078403d76

📥 Commits

Reviewing files that changed from the base of the PR and between f46b99b and 3a3a65d.

📒 Files selected for processing (7)

• src/lib/legacy-oclif-dispatch.test.ts
• src/lib/legacy-oclif-dispatch.ts
• src/lib/oclif-commands.ts
• src/lib/policy-mutate-cli-commands.ts
• src/lib/skill-install-cli-command.ts
• src/lib/snapshot-cli-commands.ts
• src/nemoclaw.ts

💤 Files with no reviewable changes (1)

• src/nemoclaw.ts

[prekshivyas]

LGTM after rebase. Clean removal of the LegacyDispatch fallback path.

• Three legacy targets (policy-add, skill, snapshot) now route through new oclif command classes (PolicyAddRawCommand hidden, SkillCliCommand, SnapshotCommand). Each wraps getRuntimeBridge().sandbox*() which defaults to the same handler the old legacy path called (verified policy-mutate-cli-commands.ts:13-22 etc.) — same end-target, same args ordering.
• LegacyDispatch type, the five kind: "legacy" returns, and the case "legacy": branch in runDispatchResult (-38 lines in src/nemoclaw.ts) all removed cleanly.
• +33 lines of new dispatch tests cover the three new routes.
• PolicyAddRawCommand is correctly hidden: true (internal fallback). The user-visible SkillCliCommand/SnapshotCommand add proper summary/description/usage — small help-text improvement.
• CI: pr.yaml fully green; pr-self-hosted build-sandbox-images/arm64 PASS; three e2es (test-e2e-sandbox, test-e2e-gateway-isolation, test-e2e-port-overrides) still pending after the rebase — worth confirming green before merging.

Cumulative ~16 orphan-import debt in src/nemoclaw.ts is unchanged here, still pending the dedicated cleanup PR.