refactor(cli): extract onboard command handler by cv · Pull Request #1710 · NVIDIA/NemoClaw

cv · 2026-04-09T21:34:42Z

Summary

Redo the old #1550 slice on top of the post-migration TypeScript CLI by extracting the onboard/setup/setup-spark command handling from src/nemoclaw.ts into a dedicated typed helper module.

Why

After the JS→TS migration, the original #1550 branch no longer mapped cleanly onto the codebase. The same refactor still makes sense, but the right source of truth is now src/nemoclaw.ts, not bin/nemoclaw.js.

This PR keeps the scope narrow:

extract the command parsing/deprecation flow
keep the existing ../bin/lib/onboard implementation untouched
update only the focused CLI/regression tests needed for the new seam

Changes

add src/lib/onboard-command.ts with:
- parseOnboardArgs()
- runOnboardCommand()
- runDeprecatedOnboardAliasCommand()
add src/lib/onboard-command.test.ts
reduce src/nemoclaw.ts onboard/setup/setup-spark handlers to thin wrappers
add CLI coverage for:
- onboard --help
- setup --help
- setup-spark --help
update the source-level regression guard in test/runner.test.ts

Notes

This supersedes the old migration-era attempt in #1550 with a smaller post-migration redo.

Testing

npm run build:cli
npx vitest run src/lib/onboard-command.test.ts test/cli.test.ts test/runner.test.ts

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

New Features
- Help flag shows usage for onboarding and deprecated aliases; aliases emit deprecation notice and delegate to onboarding.
Bug Fixes
- Clear error messages and exit code 1 for unknown options, missing paths, and unknown agents; third‑party acknowledgement honored via flag or env, defaults to false.
Refactor
- Centralized onboarding argument handling for consistent behavior across entry points.
Tests
- Added comprehensive tests for parsing, help/deprecation output, acknowledgement, errors, and delegation.

coderabbitai · 2026-04-09T21:34:57Z

📝 Walkthrough

Walkthrough

Centralizes onboarding CLI parsing/execution into src/lib/onboard-command.ts, updates src/nemoclaw.ts to delegate to it (including deprecated alias handling), and adds tests covering parsing, errors, help output, delegation, and alias deprecation.

Changes

Cohort / File(s)	Summary
Onboard command implementation & tests `src/lib/onboard-command.ts`, `src/lib/onboard-command.test.ts`	Adds `parseOnboardArgs`, `runOnboardCommand`, and `runDeprecatedOnboardAliasCommand`. Parses `--from <path>` and `--agent <name>` (validates required values and optional allowlist via `deps.listAgents()`), recognizes boolean flags, computes `acceptThirdPartySoftware` from flag or env, rejects unknown options, prints usage on errors and exits via injected wiring. Tests cover parsing, error exits, help behavior, delegation, and deprecated-alias messaging.
CLI entrypoint refactor `src/nemoclaw.ts`	Replaces inline onboard parsing with `runOnboardCommand(buildOnboardCommandDeps(args))`. Adds `buildOnboardCommandDeps(args)` to assemble args, env, callbacks, and exit wiring. Updates `setup` and `setup-spark` to call `runDeprecatedOnboardAliasCommand` with `kind: "setup"` / `"setup-spark"`.
Tests: CLI dispatch & guards `test/cli.test.ts`, `test/runner.test.ts`	Adds CLI dispatch tests asserting `onboard --help`, `setup --help`, and `setup-spark --help` exit 0 and print expected usage/deprecation text. Adjusts regression guard to assert presence of `runDeprecatedOnboardAliasCommand` and `kind: "setup-spark"` in `src/nemoclaw.ts`.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant CLI as CLI (args)
participant Parser as parseOnboardArgs
participant Deps as Deps (env, listAgents)
participant Runner as runOnboard
participant Exit as exit/console

CLI->>Parser: invoke with args (includes notice flag name)
Parser->>Deps: read env and call listAgents()
alt unknown option or missing value
    Parser->>Exit: print "Unknown onboard option(s)" + usage
    Parser->>Exit: exit(1)
else help requested (`--help`/`-h`)
    Parser->>Exit: print usage/help
    Note right of Parser: return without calling Runner
else deprecated alias
    Parser->>Exit: log deprecation message
    Parser->>Runner: delegate with parsed options
else valid args
    Parser->>Runner: call runOnboard(parsedOptions)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibbled through flags beneath the log,
Found --from, --agent, and hopped on the cog.
Deprecations noted, help printed bright,
I handed options onward, hopping light. 🐇

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'refactor(cli): extract onboard command handler' accurately describes the main change: extracting onboard command handling from src/nemoclaw.ts into a dedicated module (src/lib/onboard-command.ts).

_{✏️ 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/onboard-command-ts-redo

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

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/lib/onboard-command.test.ts`:
- Around line 6-10: The test currently imports the compiled artifact from the
dist build; change the import so it loads the source implementation instead
(i.e., import the module that directly exports parseOnboardArgs,
runDeprecatedOnboardAliasCommand, and runOnboardCommand) so tests run against
source code rather than stale/absent build output; update the import statement
to point at the source module that defines those functions and remove the
dependency on the dist artifact.

🪄 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: Pro

Run ID: 5c7640e9-336f-47a0-907c-ca55eeaba667

📥 Commits

Reviewing files that changed from the base of the PR and between 55ba539 and 6ba95a4.

📒 Files selected for processing (5)

src/lib/onboard-command.test.ts
src/lib/onboard-command.ts
src/nemoclaw.ts
test/cli.test.ts
test/runner.test.ts

cv · 2026-04-10T00:47:21Z

I merged the latest main into this branch and resolved the conflict in the onboard command path by keeping the extracted helper structure while porting over the current --agent and --dangerously-skip-permissions onboard semantics from main.

Focused validation run:

npm run build:cli
npx vitest run src/lib/onboard-command.test.ts test/cli.test.ts test/runner.test.ts

Both passed. CI is rerunning now.

coderabbitai

🧹 Nitpick comments (1)

src/lib/onboard-command.ts (1)

82-85: Print onboard usage after unknown --agent errors for consistency.

This branch exits immediately, while other parse failures also print usage; matching behavior would make CLI errors more consistent.

Suggested patch

     if (knownAgents.length > 0 && !knownAgents.includes(agentValue)) {
       error(`  Unknown agent '${agentValue}'. Available: ${knownAgents.join(", ")}`);
+      printOnboardUsage(error, noticeAcceptFlag);
       exit(1);
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard-command.ts` around lines 82 - 85, When the agent argument is
unknown (the branch checking knownAgents and agentValue), print the CLI
usage/help before exiting to match other parse-failure behavior; modify the
branch so it calls the same usage-printing function used elsewhere in this
module (e.g., printUsage/showUsage/printOnboardUsage) immediately before
invoking exit(1) so the error message plus usage are both shown.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/onboard-command.ts`:
- Around line 82-85: When the agent argument is unknown (the branch checking
knownAgents and agentValue), print the CLI usage/help before exiting to match
other parse-failure behavior; modify the branch so it calls the same
usage-printing function used elsewhere in this module (e.g.,
printUsage/showUsage/printOnboardUsage) immediately before invoking exit(1) so
the error message plus usage are both shown.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2cc78336-f0bc-4d69-822e-77847ff6953e

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba95a4 and a8adfc6.

📒 Files selected for processing (4)

src/lib/onboard-command.test.ts
src/lib/onboard-command.ts
src/nemoclaw.ts
test/runner.test.ts

✅ Files skipped from review due to trivial changes (1)

src/lib/onboard-command.test.ts

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

test/runner.test.ts

cv · 2026-04-10T00:52:28Z

I merged the latest main into this branch and resolved the conflict in the onboard command path by keeping the extracted helper structure while porting over the current --agent and --dangerously-skip-permissions semantics from main.

Focused validation run:

npm run build:cli
npx vitest run src/lib/onboard-command.test.ts test/cli.test.ts test/runner.test.ts

Both passed. CI is rerunning now.

brandonpelfrey · 2026-04-10T04:04:13Z

🦞 NemoClaw Functional Review — PR #1710

Verdict: ✅ APPROVE

Summary

Clean refactoring — extracts onboard/setup/setup-spark command handling from src/nemoclaw.ts into src/lib/onboard-command.ts. No functional changes.

Functional Testing

Step	Result
Clone & checkout	✅
npm install	✅
TypeScript build	✅ Compiles clean
Test suite	✅ 1206 passed

Security

No dangerous patterns (eval, exec, prototype pollution) in diff
No hardcoded credentials
No new dependencies added
No permission changes

Architecture

Changes are well-scoped to the CLI layer:

nemoclaw/src/lib/onboard-command.ts — new module with parseOnboardArgs(), runOnboardCommand(), runDeprecatedOnboardAliasCommand()
nemoclaw/src/lib/onboard-command.test.ts — proper unit test coverage
nemoclaw/src/nemoclaw.ts — handlers reduced to thin wrappers (good)
test/cli.test.ts + test/runner.test.ts — regression guards updated

The existing bin/lib/onboard implementation is deliberately untouched, keeping the blast radius minimal. This follows the right pattern for incremental refactoring.

Notes

No blocking issues. Straightforward extraction with good test coverage for the new seam.

🦞 Auto-reviewed by Nemo.

…and-ts-redo # Conflicts: # src/nemoclaw.ts

coderabbitai

🧹 Nitpick comments (1)

test/runner.test.ts (1)

479-480: Tighten this regression guard to setupSpark() specifically.

These file-wide toContain() checks can still pass if setupSpark() stops delegating correctly and the same strings remain elsewhere in src/nemoclaw.ts. Matching the setupSpark function body would keep this guard anchored to the behavior it is supposed to protect.

🔍 Suggested tightening

-      expect(src).toContain("runDeprecatedOnboardAliasCommand");
-      expect(src).toContain('kind: "setup-spark"');
+      const setupSparkBlock = src.match(
+        /async function setupSpark\(args = \[\]\) \{[\s\S]*?runDeprecatedOnboardAliasCommand\([\s\S]*?kind: "setup-spark"[\s\S]*?\n\}/,
+      );
+      expect(setupSparkBlock).toBeTruthy();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/runner.test.ts` around lines 479 - 480, Tighten the regression guard by
asserting the actual setupSpark function body is present rather than searching
the whole file: locate the setupSpark function definition in src/nemoclaw.ts and
change the two loose expect(src).toContain(...) checks to assert that the
substring or regex matching the entire setupSpark(...) function (including
occurrences of runDeprecatedOnboardAliasCommand and the literal kind:
"setup-spark") exists in src; e.g., match the function declaration and its
closing brace so the test fails if setupSpark stops delegating or its body
changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/runner.test.ts`:
- Around line 479-480: Tighten the regression guard by asserting the actual
setupSpark function body is present rather than searching the whole file: locate
the setupSpark function definition in src/nemoclaw.ts and change the two loose
expect(src).toContain(...) checks to assert that the substring or regex matching
the entire setupSpark(...) function (including occurrences of
runDeprecatedOnboardAliasCommand and the literal kind: "setup-spark") exists in
src; e.g., match the function declaration and its closing brace so the test
fails if setupSpark stops delegating or its body changes.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4317859b-b941-4d9e-9489-36442e2999bd

📥 Commits

Reviewing files that changed from the base of the PR and between a8adfc6 and 455f31b.

📒 Files selected for processing (2)

src/nemoclaw.ts
test/runner.test.ts

coderabbitai

🧹 Nitpick comments (1)

src/lib/onboard-command.ts (1)

62-67: Consider extracting repeated fail-fast handling into a small helper.

error(...) + printOnboardUsage(...) + exit(1) is repeated in multiple branches; a local helper would reduce duplication and simplify future edits.

♻️ Optional refactor sketch

+  const fail = (message: string): never => {
+    error(message);
+    printOnboardUsage(error, noticeAcceptFlag);
+    exit(1);
+  };
+
   if (fromIdx !== -1) {
     fromDockerfile = parsedArgs[fromIdx + 1] || null;
     if (!fromDockerfile || fromDockerfile.startsWith("--")) {
-      error("  --from requires a path to a Dockerfile");
-      printOnboardUsage(error, noticeAcceptFlag);
-      exit(1);
+      fail("  --from requires a path to a Dockerfile");
     }

Also applies to: 76-79, 83-85, 94-96

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard-command.ts` around lines 62 - 67, Several branches in
onboard-command.ts repeat the pattern error(...); printOnboardUsage(error,
noticeAcceptFlag); exit(1); — extract a small helper (e.g., failWithUsage or
handleBadArg) that accepts the error message (and captures noticeAcceptFlag via
closure or as a parameter), calls error(msg), printOnboardUsage(error,
noticeAcceptFlag), then exit(1); replace repeated blocks around fromDockerfile,
parsedArgs checks and the other similar branches (the ones that currently call
error + printOnboardUsage + exit) to call this helper instead, keeping existing
symbols like fromDockerfile, parsedArgs, error, printOnboardUsage,
noticeAcceptFlag and exit unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/onboard-command.ts`:
- Around line 62-67: Several branches in onboard-command.ts repeat the pattern
error(...); printOnboardUsage(error, noticeAcceptFlag); exit(1); — extract a
small helper (e.g., failWithUsage or handleBadArg) that accepts the error
message (and captures noticeAcceptFlag via closure or as a parameter), calls
error(msg), printOnboardUsage(error, noticeAcceptFlag), then exit(1); replace
repeated blocks around fromDockerfile, parsedArgs checks and the other similar
branches (the ones that currently call error + printOnboardUsage + exit) to call
this helper instead, keeping existing symbols like fromDockerfile, parsedArgs,
error, printOnboardUsage, noticeAcceptFlag and exit unchanged.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7e1ff890-7ca8-41f9-a0d2-ede57c4f8156

📥 Commits

Reviewing files that changed from the base of the PR and between 455f31b and b1c9577.

📒 Files selected for processing (2)

src/lib/onboard-command.test.ts
src/lib/onboard-command.ts

✅ Files skipped from review due to trivial changes (1)

src/lib/onboard-command.test.ts

Main underwent a large-scale refactoring while this PR was open: - JS→TS migration: bin/lib/onboard.js → src/lib/onboard.ts, bin/nemoclaw.js → src/nemoclaw.ts (NVIDIA#1673) - Legacy shim removal: 18 bin/lib/*.js files deleted (NVIDIA#1713) - Command handler extraction: src/lib/onboard-command.ts (NVIDIA#1710) - Test renames: .js → .ts across all test files Conflict resolution: - bin/nemoclaw.js: accept main's 6-line launcher (logic in src/nemoclaw.ts) - src/lib/onboard.ts: take main's build context staging (fromDockerfile + agent + stageOptimized), keep Jetson gateway patch functions - docs/get-started/quickstart.md: use main's table format, add Jetson row - test/onboard.test.js: accept deletion (migrated to .ts) Re-ported Jetson features onto new TS structure: - Add setup-jetson to GLOBAL_COMMANDS, help text, and switch dispatch in src/nemoclaw.ts (not a deprecated alias — runs setup-jetson.sh) - Add Jetson to ci/platform-matrix.json (source of truth for docs table) - Add getGatewayImageTag and patchGatewayImageForJetson to exports in src/lib/onboard.ts - Port 4 Jetson tests to test/onboard.test.ts (dist/lib/onboard paths) Verified on Jetson Orin Nano Super (8GB, JetPack 6.x, Node 22.22.0): - Build: tsc passes - 4 Jetson tests: all pass - GPU: jetson=true, 7619 MB unified memory - Gateway: iptables v1.8.10 (legacy), io.nemoclaw.jetson-patched=true - Sandbox: Phase Ready - Inference: openclaw agent → "2 + 2 = 4." - Path: sandbox → host.openshell.internal:11434 → Ollama → nemotron-3-nano:4b Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

## Summary Redo the old NVIDIA#1550 slice on top of the post-migration TypeScript CLI by extracting the onboard/setup/setup-spark command handling from `src/nemoclaw.ts` into a dedicated typed helper module. ## Why After the JS→TS migration, the original NVIDIA#1550 branch no longer mapped cleanly onto the codebase. The same refactor still makes sense, but the right source of truth is now `src/nemoclaw.ts`, not `bin/nemoclaw.js`. This PR keeps the scope narrow: - extract the command parsing/deprecation flow - keep the existing `../bin/lib/onboard` implementation untouched - update only the focused CLI/regression tests needed for the new seam ## Changes - add `src/lib/onboard-command.ts` with: - `parseOnboardArgs()` - `runOnboardCommand()` - `runDeprecatedOnboardAliasCommand()` - add `src/lib/onboard-command.test.ts` - reduce `src/nemoclaw.ts` onboard/setup/setup-spark handlers to thin wrappers - add CLI coverage for: - `onboard --help` - `setup --help` - `setup-spark --help` - update the source-level regression guard in `test/runner.test.ts` ## Notes This supersedes the old migration-era attempt in NVIDIA#1550 with a smaller post-migration redo. ## Testing - [x] `npm run build:cli` - [x] `npx vitest run src/lib/onboard-command.test.ts test/cli.test.ts test/runner.test.ts` Signed-off-by: Carlos Villela <cvillela@nvidia.com>  ## Summary by CodeRabbit * **New Features** * Help flag shows usage for onboarding and deprecated aliases; aliases emit deprecation notice and delegate to onboarding. * **Bug Fixes** * Clear error messages and exit code 1 for unknown options, missing paths, and unknown agents; third‑party acknowledgement honored via flag or env, defaults to false. * **Refactor** * Centralized onboarding argument handling for consistent behavior across entry points. * **Tests** * Added comprehensive tests for parsing, help/deprecation output, acknowledgement, errors, and delegation.

refactor(cli): extract onboard command handler

6ba95a4

coderabbitai Bot reviewed Apr 9, 2026

View reviewed changes

Comment thread src/lib/onboard-command.test.ts Outdated

merge: sync main into onboard-command refactor

a8adfc6

coderabbitai Bot reviewed Apr 10, 2026

View reviewed changes

Merge branch 'main' into refactor/onboard-command-ts-redo

b8d0f75

brandonpelfrey approved these changes Apr 10, 2026

View reviewed changes

cv added the v0.0.12 label Apr 10, 2026

cv added 2 commits April 10, 2026 10:04

Merge remote-tracking branch 'origin/main' into refactor/onboard-comm…

455f31b

…and-ts-redo # Conflicts: # src/nemoclaw.ts

fix(cli): address onboard command review feedback

b1c9577

coderabbitai Bot reviewed Apr 10, 2026

View reviewed changes

cv merged commit aa52ed8 into main Apr 10, 2026
12 checks passed

cv deleted the refactor/onboard-command-ts-redo branch April 10, 2026 17:22

cv added v0.0.13 and removed v0.0.12 labels Apr 10, 2026

realkim93 mentioned this pull request Apr 12, 2026

feat: add Jetson Orin Nano support #405

Open

9 tasks

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(cli): extract onboard command handler#1710

refactor(cli): extract onboard command handler#1710
cv merged 5 commits into
mainfrom
refactor/onboard-command-ts-redo

cv commented Apr 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 9, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

cv commented Apr 10, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

cv commented Apr 10, 2026

Uh oh!

brandonpelfrey commented Apr 10, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

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

Uh oh!

Summary

Why

Changes

Notes

Testing

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cv commented Apr 10, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cv commented Apr 10, 2026

Uh oh!

brandonpelfrey commented Apr 10, 2026

🦞 NemoClaw Functional Review — PR #1710

Summary

Functional Testing

Security

Architecture

Notes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cv commented Apr 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 9, 2026 •

edited

Loading