fix(onboard): validate NEMOCLAW_PROVIDER before preflight checks#1772
Conversation
An invalid NEMOCLAW_PROVIDER value in non-interactive mode would fall through to preflight, which fails with a misleading 'Docker is not reachable' error. The user would fix Docker, re-run, and only then discover the real problem: an unsupported provider value. Move the provider validation call (getRequestedProviderHint) to run immediately after the third-party notice consent, before preflight and lock acquisition. Invalid values now fail immediately with: Unsupported NEMOCLAW_PROVIDER: <value> Valid values: build, openai, anthropic, ... Closes NVIDIA#1729 Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe onboarding flow now performs an early provider validation in non-interactive runs: after usage-consent is checked and before acquiring the onboarding lock, Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
4334-4336: Reduce branch count inonboard()while preserving fail-fast validation.This can be a direct call since
getRequestedProviderHint()already no-ops in interactive mode; same behavior, lower complexity in an already large function.♻️ Proposed simplification
- if (isNonInteractive()) { - getRequestedProviderHint(true); - } + getRequestedProviderHint();As per coding guidelines, "Limit cyclomatic complexity to 20 in JavaScript/TypeScript files, with target of 15".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 4334 - 4336, The conditional branch in onboard() that calls getRequestedProviderHint(true) only when isNonInteractive() is true should be simplified to a direct call to getRequestedProviderHint(true) because getRequestedProviderHint already no-ops in interactive mode; replace the if (isNonInteractive()) { getRequestedProviderHint(true); } with a straight getRequestedProviderHint(true) call to reduce cyclomatic complexity while preserving the fail-fast validation behavior (refer to onboard(), getRequestedProviderHint(), and isNonInteractive()).
🤖 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.ts`:
- Around line 4334-4336: The conditional branch in onboard() that calls
getRequestedProviderHint(true) only when isNonInteractive() is true should be
simplified to a direct call to getRequestedProviderHint(true) because
getRequestedProviderHint already no-ops in interactive mode; replace the if
(isNonInteractive()) { getRequestedProviderHint(true); } with a straight
getRequestedProviderHint(true) call to reduce cyclomatic complexity while
preserving the fail-fast validation behavior (refer to onboard(),
getRequestedProviderHint(), and isNonInteractive()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f897b4a-e306-4525-987d-64505573838f
📒 Files selected for processing (1)
src/lib/onboard.ts
getRequestedProviderHint() already no-ops in interactive mode, so the isNonInteractive() guard is redundant. Call it directly to reduce cyclomatic complexity in onboard(). Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
|
✨ Thanks for submitting this PR, which proposes a fix for a bug where invalid NEMOCLAW_PROVIDER values in non-interactive mode do not fail immediately with a clear error. Possibly related open issues: |
prekshivyas
left a comment
There was a problem hiding this comment.
Clean — moves existing validation earlier, no-op in interactive mode, no regressions. @latenighthackathon can you update branch with main?
Summary
Invalid
NEMOCLAW_PROVIDERvalues in non-interactive mode now fail immediately with a clear error instead of falling through to misleading preflight failures.Problem
NEMOCLAW_PROVIDER=invalid nemoclaw onboard --non-interactivereaches preflight (Docker/OpenShell checks) before provider validation. If Docker isn't installed, the user sees "Docker is not reachable" and installs Docker -- only to discover on the next run that the real issue was an invalid provider value.Fix
Call
getRequestedProviderHint()immediately after the third-party notice consent, before preflight and lock acquisition. The existinggetNonInteractiveProvider()validation already has the right error message andprocess.exit(1)-- it just needed to run earlier.Test plan
npm run build:clipassesnpm run typecheck:clipassesnpm run lintpassesCloses #1729
Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit