fix(onboard): cancel onboarding when `exit` is typed at numbered menus by laitingsheng · Pull Request #4581 · NVIDIA/NemoClaw

laitingsheng · 2026-05-31T15:45:25Z

Summary

The inference provider menu, NIM model menu, and non-TTY policy tier menu all dispatched the reply through parseInt(rawChoice || String(default), 10) - 1 with no navigation check first. Typing exit at any of them parses to NaN, falls through to the bracketed default, and the wizard silently advances as if the operator had pressed Enter. Other prompts in the same flow honour the documented exit / quit cancellation contract via getNavigationChoice + exitOnboardFromPrompt.

This PR extracts a single selectFromNumberedMenu helper in src/lib/onboard/prompt-helpers.ts that wraps the navigation check around the parseInt-with-default selection, and routes the three menu sites through it. onboard.ts shrinks by three lines on net.

Related Issue

Fixes #4514.

Changes

src/lib/onboard/prompt-helpers.ts: add selectFromNumberedMenu<T>(rawChoice, defaultIdx, options) — runs getNavigationChoice → exitOnboardFromPrompt() on exit / quit, otherwise returns the parsed-index option (falling back to the bracketed default).
src/lib/onboard.ts: replace the inline parseInt selection at the inference provider menu, the NIM model menu, and the non-TTY policy tier menu with selectFromNumberedMenu. The provider menu now honours the exit / quit contract advertised by the wizard.
src/lib/onboard/prompt-helpers.test.ts: nine new cases pinning bare-Enter → default, valid number → option, out-of-range → default, every casing / whitespace variant of exit / quit / EXIT / Quit / " exit " → process.exit(1) with the Exiting onboarding. banner, and non-navigation words → numeric path.

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

npm run typecheck:cli passes
npm run build:cli passes
npx vitest run --project cli src/lib/onboard/prompt-helpers.test.ts test/onboard-selection.test.ts — 87 / 87 pass
Tests added or updated for new or changed behavior
No secrets, API keys, or credentials committed
Docs updated for user-facing behavior changes
npm run 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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

Refactor
- Standardized numbered-menu selection across onboarding flows by consolidating parsing and "exit" handling into a shared prompt helper for consistent behavior.
Tests
- Added thorough tests covering default selection, numeric choices, out-of-range fallback, exit/cancellation handling, and edge cases (falsy options, sparse arrays, and non-navigation inputs).

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

coderabbitai · 2026-05-31T15:45:36Z

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: 95980d04-0119-43e9-8471-c66a5c642e0b

📥 Commits

Reviewing files that changed from the base of the PR and between dfefa79 and 227e826.

📒 Files selected for processing (3)

src/lib/onboard.ts
src/lib/onboard/prompt-helpers.test.ts
src/lib/onboard/prompt-helpers.ts

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

src/lib/onboard.ts

📝 Walkthrough

Walkthrough

Adds a generic selectFromNumberedMenuOrExit helper that treats "exit" as cancellation, parses 1-based numeric choices with 0-based indexing into options, and falls back to a provided default. Integrates the helper into three onboarding prompts and adds comprehensive tests.

Changes

Numbered Menu Selection Helper Consolidation

Layer / File(s)	Summary
Helper definition and tests `src/lib/onboard/prompt-helpers.ts`, `src/lib/onboard/prompt-helpers.test.ts`	Adds `selectFromNumberedMenuOrExit<T>` which interprets "exit" as onboarding cancellation, parses 1-based numeric menu input to a 0-based option index, and returns `options[defaultIdx - 1]` for invalid selections. Tests cover default/empty input, valid selection, out-of-range fallback, exit-triggered cancellation (with process.exit and log), falsy option values, sparse arrays, and non-exit words like `back`.
Integration into onboarding flows `src/lib/onboard.ts`	Destructures and imports `selectFromNumberedMenuOrExit` and replaces local `parseInt` + fallback logic at inference-provider selection, Ollama NIM model selection, and non-TTY policy-tier selection sites.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

NemoClaw CLI

Suggested reviewers

cv
ericksoa

Poem

🐇
I nibble through menus, one to three,
"exit" now hops out — hooray for me!
Defaults catch crumbs that wander astray,
Numbers map true in a neat, calm way.
A rabbit claps for onboarding play.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.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 title accurately summarizes the main change: adding cancel-on-exit functionality to numbered menus in the onboarding flow.
Linked Issues check	✅ Passed	All coding objectives from `#4514` are met: the helper function enforces exit/quit cancellation at numbered menus, replacing inline parseInt logic in onboard.ts, with comprehensive test coverage validating the expected behavior.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to `#4514`: new helper function, refactoring numbered menu selection in onboard.ts, and adding test coverage; no unrelated modifications detected.

_{✏️ 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 fix/4514-onboard-exit-provider-menu

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-31T15:48:15Z

PR Review Advisor

Findings: 0 needs attention, 1 worth checking, 0 nice ideas
Since last review: 0 prior items resolved, 1 still applies, 0 new items found

Review findings

🛠️ Needs attention

None.

🔎 Worth checking

Add a regression test at the actual provider-menu prompt (src/lib/onboard.ts:4269): Issue [Ubuntu 24.04][Onboard] typing exit at inference provider selection does not cancel onboarding #4514 is specifically about typing `exit` at the inference provider `Choose [1]:` prompt and unexpectedly reaching the NVIDIA API key prompt. The helper tests cover `selectFromNumberedMenuOrExit` directly, and the provider menu now delegates to it, but the changed tests do not exercise the real `setupNim` provider-selection flow or prove that credential prompts, credential saves, and later onboarding steps are skipped.
- Recommendation: Add or identify a targeted `test/onboard-selection.test.ts`/`setupNim` regression that answers `exit` at the inference provider `Choose [...]` prompt and asserts `process.exit(1)`, the `Exiting onboarding.` banner, no NVIDIA/provider credential prompt, no credential save, and no later setup progression.
- Evidence: The diff changes `selected = selectFromNumberedMenuOrExit(choice, defaultIdx, options)` at the provider menu and adds helper unit tests in `src/lib/onboard/prompt-helpers.test.ts`, but no integration-style provider-menu exit test is added in the shown changed files. This is the same prior Advisor finding and still applies.

🌱 Nice ideas

None.

Since last review details

Current findings:

Add a regression test at the actual provider-menu prompt (src/lib/onboard.ts:4269): Issue [Ubuntu 24.04][Onboard] typing exit at inference provider selection does not cancel onboarding #4514 is specifically about typing `exit` at the inference provider `Choose [1]:` prompt and unexpectedly reaching the NVIDIA API key prompt. The helper tests cover `selectFromNumberedMenuOrExit` directly, and the provider menu now delegates to it, but the changed tests do not exercise the real `setupNim` provider-selection flow or prove that credential prompts, credential saves, and later onboarding steps are skipped.
- Recommendation: Add or identify a targeted `test/onboard-selection.test.ts`/`setupNim` regression that answers `exit` at the inference provider `Choose [...]` prompt and asserts `process.exit(1)`, the `Exiting onboarding.` banner, no NVIDIA/provider credential prompt, no credential save, and no later setup progression.
- Evidence: The diff changes `selected = selectFromNumberedMenuOrExit(choice, defaultIdx, options)` at the provider menu and adds helper unit tests in `src/lib/onboard/prompt-helpers.test.ts`, but no integration-style provider-menu exit test is added in the shown changed files. This is the same prior Advisor finding and still applies.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

github-actions · 2026-05-31T15:48:37Z

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, onboard-negative-paths-e2e
Optional E2E: cloud-e2e, network-policy-e2e

Dispatch hint: cloud-onboard-e2e,onboard-negative-paths-e2e

Auto-dispatched E2E: cloud-onboard-e2e, onboard-negative-paths-e2e via nightly-e2e.yaml at 969707033a7a6b5171df0567aadf8727c0c519af — nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

cloud-onboard-e2e (high): Required because src/lib/onboard.ts and onboarding prompt helpers changed. This is the targeted real cloud onboarding job that validates install/onboard, sandbox health, inference.local, security checks, and policy application on a live sandbox, catching runtime import/build regressions in the onboard path.
onboard-negative-paths-e2e (high): Required as the closest existing onboarding edge-case suite. It exercises onboard validation paths for provider/model/policy settings and friendly failures, which are adjacent to this PR's menu-selection changes in the central onboard flow.

Optional E2E

cloud-e2e (high): Optional broader confidence for the complete install → onboard → sandbox verify → live inference journey. Useful if maintainers want an additional end-to-end smoke beyond the more targeted cloud-onboard job.
network-policy-e2e (high): Optional because the policy tier non-TTY menu fallback changed. Existing network policy E2E mostly uses non-interactive policy configuration, so it is adjacent confidence rather than direct coverage of the prompt branch.

New E2E recommendations

interactive-onboarding-prompts (high): Existing E2E coverage is mostly non-interactive and does not directly exercise the changed numbered-menu helper in a real prompt/TTY flow. Add an expect/PTY-style E2E that drives onboarding through provider selection, validates bare Enter/default selection, valid numeric selection, out-of-range fallback, and exit/quit cancellation without creating an unintended sandbox.
- Suggested test: Add an interactive prompt E2E for onboarding numbered menus using expect or a pseudo-TTY harness.
policy-tier-non-tty-prompt (medium): The policy tier fallback for interactive non-TTY stdin/stdout now uses selectFromNumberedMenuOrExit, but existing policy E2Es configure policy non-interactively. Add coverage that invokes selectPolicyTier through a non-TTY prompt path and proves default, explicit tier number, out-of-range fallback, and exit behavior.
- Suggested test: Add a non-TTY policy tier onboarding E2E or focused integration test for the policy tier prompt path.

Dispatch hint

Workflow: .github/workflows/nightly-e2e.yaml
jobs input: cloud-onboard-e2e,onboard-negative-paths-e2e

github-actions · 2026-05-31T15:48:37Z

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw
Optional scenario E2E: ubuntu-repo-cloud-hermes, gpu-repo-local-ollama-openclaw

Dispatch required scenario E2E:

gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required scenario E2E

ubuntu-repo-cloud-openclaw: Core onboarding code and shared prompt helper code changed. Run the primary Ubuntu repo cloud OpenClaw scenario to validate the scenario onboarding path still loads and completes successfully with the updated helper wiring.
- Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw

Optional scenario E2E

ubuntu-repo-cloud-hermes: Optional adjacent coverage for the same cloud onboarding flow with the Hermes agent, in case the shared onboarding prompt-helper import affects agent-specific onboarding.
- Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-hermes
gpu-repo-local-ollama-openclaw: Optional special-runner coverage for the local-provider onboarding family, which is adjacent to the provider-selection code touched in onboard.ts.
- Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=gpu-repo-local-ollama-openclaw

Relevant changed files

src/lib/onboard.ts
src/lib/onboard/prompt-helpers.ts

coderabbitai

Actionable comments posted: 1

🤖 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/onboard/prompt-helpers.ts`:
- Around line 40-41: The current return uses a || fallback which treats valid
falsy option values as missing; in the block that computes idx from
rawChoice/defaultIdx (variables rawChoice, defaultIdx, options, idx), replace
the falsy-check fallback with an explicit bounds check: if idx is within
0..options.length-1 return options[idx], otherwise return options[defaultIdx -
1]. Ensure you compute idx as Number.parseInt(rawChoice || String(defaultIdx),
10) - 1 and use the explicit index range check rather than the || operator.

🪄 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: 4c3321c8-579e-4821-8aa5-fb2c0f652d12

📥 Commits

Reviewing files that changed from the base of the PR and between 9641ce0 and dfefa79.

📒 Files selected for processing (3)

src/lib/onboard.ts
src/lib/onboard/prompt-helpers.test.ts
src/lib/onboard/prompt-helpers.ts

github-actions · 2026-05-31T15:54:32Z

Selective E2E Results — ✅ All requested jobs passed

Run: 26717140181
Target ref: dfefa7933f3f73db0861f67f54eca0f9a588f7a2
Workflow ref: main
Requested jobs: cloud-onboard-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job	Result
cloud-onboard-e2e	✅ success

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

cjagwani

Approving. Clean helper extraction — the three menu sites now honor the documented exit/quit contract instead of silently advancing on a parse-NaN.

Test coverage is exemplary: 9 cases covering casing variants (EXIT/Quit/whitespace), non-navigation words, sparse arrays, and the falsy-option edge case where 1 mustn't silently swap to the bracketed default. cloud-onboard-e2e, macos-e2e, test-e2e-sandbox, and test-e2e-gateway-isolation all passed.

github-actions · 2026-06-01T18:57:16Z

Selective E2E Results — ✅ All requested jobs passed

Run: 26774993624
Target ref: 969707033a7a6b5171df0567aadf8727c0c519af
Workflow ref: main
Requested jobs: cloud-onboard-e2e,onboard-negative-paths-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job	Result
cloud-onboard-e2e	✅ success
onboard-negative-paths-e2e	✅ success

fix(onboard): cancel onboarding when exit is typed at numbered menus

dfefa79

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

laitingsheng added fix 04-25-regression Issues raised from the Apr 25 weekend regression analysis and removed 04-25-regression Issues raised from the Apr 25 weekend regression analysis labels May 31, 2026

coderabbitai Bot reviewed May 31, 2026

View reviewed changes

Comment thread src/lib/onboard/prompt-helpers.ts Outdated

fix(onboard): tighten numbered-menu selection bounds and clarify scope

227e826

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>

laitingsheng added the v0.0.56 Release target label May 31, 2026

cjagwani self-requested a review June 1, 2026 16:38

cv added v0.0.57 Release target and removed v0.0.56 Release target labels Jun 1, 2026

Merge branch 'main' into fix/4514-onboard-exit-provider-menu

9697070

cjagwani approved these changes Jun 1, 2026

View reviewed changes

cv merged commit 0b54a3e into main Jun 1, 2026
27 checks passed

cv deleted the fix/4514-onboard-exit-provider-menu branch June 1, 2026 20:10

wscurran added bug-fix PR fixes a bug or regression and removed fix labels Jun 3, 2026

coderabbitai Bot mentioned this pull request Jun 5, 2026

fix(onboard): early-validate NEMOCLAW_POLICY_TIER before preflight (#3741) #3788

Merged

Conversation

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

Uh oh!

Summary

Related Issue

Changes

Type of Change

Verification

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

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

Uh oh!

PR Review Advisor

🛠️ Needs attention

🔎 Worth checking

🌱 Nice ideas

Uh oh!

github-actions Bot commented May 31, 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!

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

Uh oh!

E2E Scenario Advisor Recommendation

E2E Scenario Advisor

Required scenario E2E

Optional scenario E2E

Relevant changed files

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented May 31, 2026

Selective E2E Results — ✅ All requested jobs passed

Uh oh!

cjagwani left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jun 1, 2026

Selective E2E Results — ✅ All requested jobs passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

laitingsheng commented May 31, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 31, 2026 •

edited

Loading

github-actions Bot commented May 31, 2026 •

edited

Loading

github-actions Bot commented May 31, 2026 •

edited

Loading

github-actions Bot commented May 31, 2026 •

edited

Loading