fix(onboard): render truthful Ollama status in the inference menu#6753
Conversation
Interactive onboarding gated the local daemon probes on the shared-gateway route preflight, so any unrelated registry route on the same gateway silently hid a running Ollama or vLLM daemon from the menu: the entry rendered as "Local Ollama (localhost:11434)" with no running suffix and no detection banner. The probe only drives status display; route conflicts stay enforced at selection time through assertRouteCompatible, so interactive runs now always probe while non-interactive runs keep the preflight gate. An installed-but-stopped daemon also rendered the status-style label "Local Ollama (WSL:11434)", which reads as a reachability claim while the daemon is down. Selecting that entry is the deliberate auto-start path, so it now renders the action label "Start local Ollama (WSL:11434)" instead. Suppressing the entry was rejected: on WSL with native Docker it is the only in-wizard path to a local Ollama. The Windows-host "(requires Docker Desktop WSL integration)" suffix is working as designed; the Windows preparation guide now states that requirement and the Ollama setup page describes the interactive re-prompt accurately instead of claiming an exit. Refs #6750 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 79%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInteractive onboarding now permits local provider probes despite route conflicts, while non-interactive runs retain gating. Ollama’s stopped-state menu label and Windows/WSL remediation guidance are updated with corresponding tests. ChangesOnboarding behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-6753.docs.buildwithfern.com/nemoclaw |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings This is an automated review. Required findings need action before merge. Warnings and optional suggestions do not require a response or follow-up. A human maintainer makes the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/onboard/setup-nim-provider-discovery.test.ts (1)
31-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the non-interactive vLLM gate.
The implementation has independent non-interactive Ollama and vLLM branches, but these tests only exercise the non-interactive Ollama intent. Add blocked and allowed vLLM cases so the
probeVllmroute-gating branch cannot regress silently.As per path instructions, tests should provide behavioral confidence for each changed path.
🤖 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/onboard/setup-nim-provider-discovery.test.ts` around lines 31 - 61, Add corresponding blocked and allowed non-interactive vLLM tests alongside the existing Ollama cases in the setup provider discovery test suite. Configure the non-interactive provider as vLLM and assert probeVllm is false when canProbeRoute returns false and true when it returns true, preserving the existing dependency and setup patterns.Source: Path instructions
src/lib/onboard/setup-nim-flow.test.ts (1)
358-386: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise selection-time conflict enforcement through the public boundary.
This test only proves that
canProbeRouteis not called; its selection stub choosesnvidia-prod, and no local-provider rejection throughassertRouteCompatibleis exercised. Add a local selection case that remains blocked by a route conflict while retaining the probe assertions.As per path instructions, tests should verify observable behavior through the public boundary rather than rely on mock-call assertions.
🤖 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/onboard/setup-nim-flow.test.ts` around lines 358 - 386, Update the test around createSetupNim and setupNim to exercise a conflicting local-provider selection that is rejected through the public setup flow by assertRouteCompatible, while preserving the interactive discovery behavior and expected detect:true:true probe event. Replace the canProbeRoute call-count assertion with observable outcome assertions showing local selection remains blocked, and retain the remote-selection setup only as needed to reach the public boundary.Source: Path instructions
🤖 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 `@docs/get-started/windows-preparation.mdx`:
- Line 165: Update the Windows Ollama onboarding guidance in the surrounding
paragraph to scope express install to Docker Desktop WSL integration being
available. Explicitly direct users running native Docker Engine inside WSL to
use the WSL Ollama instance and avoid the Windows-host path.
---
Nitpick comments:
In `@src/lib/onboard/setup-nim-flow.test.ts`:
- Around line 358-386: Update the test around createSetupNim and setupNim to
exercise a conflicting local-provider selection that is rejected through the
public setup flow by assertRouteCompatible, while preserving the interactive
discovery behavior and expected detect:true:true probe event. Replace the
canProbeRoute call-count assertion with observable outcome assertions showing
local selection remains blocked, and retain the remote-selection setup only as
needed to reach the public boundary.
In `@src/lib/onboard/setup-nim-provider-discovery.test.ts`:
- Around line 31-61: Add corresponding blocked and allowed non-interactive vLLM
tests alongside the existing Ollama cases in the setup provider discovery test
suite. Configure the non-interactive provider as vLLM and assert probeVllm is
false when canProbeRoute returns false and true when it returns true, preserving
the existing dependency and setup patterns.
🪄 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: 256064f7-d897-426a-9b9b-182c9ebe9c21
📒 Files selected for processing (7)
docs/get-started/windows-preparation.mdxdocs/inference/set-up-ollama.mdxsrc/lib/onboard/ollama-install-menu.test.tssrc/lib/onboard/ollama-install-menu.tssrc/lib/onboard/setup-nim-flow.test.tssrc/lib/onboard/setup-nim-provider-discovery.test.tssrc/lib/onboard/setup-nim-provider-discovery.ts
The express prompt appears on any WSL host, but the Windows-host Ollama path it selects requires Docker Desktop WSL integration, so native Docker Engine users need to decline it and pick WSL Ollama instead. Refs #6750 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
wscurran
added
area: cli
cjagwani
left a comment
cjagwani
left a comment
There was a problem hiding this comment.
Exact-head maintainer review complete on c92259b. CI is green, all commits are Verified with PR-body DCO, the trusted PR Review Advisor reports merge_as_is, and no unresolved actionable threads remain. Security review across the nine categories found no credential, input-validation, authorization, dependency, logging, cryptography, configuration, testing, or holistic-posture regression: interactive local-daemon probing only drives truthful menu status, while route compatibility is still enforced when a provider is selected. The added tests preserve the non-interactive fail-closed route gate and cover stopped-daemon labels.
4 participants
Summary
Interactive onboarding gated the local daemon probes on the shared-gateway route preflight, so any unrelated registry route on the same gateway silently hid a running Ollama or vLLM daemon from the step [3/8] inference menu (no
— runningsuffix, no detection banner). Interactive runs now always probe — route conflicts stay enforced at selection time viaassertRouteCompatible— and an installed-but-stopped daemon renders the action labelStart local Ollama (WSL:11434)instead of a status-style label that reads as a reachability claim.Related Issue
Refs #6750
Changes
src/lib/onboard/setup-nim-provider-discovery.ts: stop ANDingcanProbeRouteintoprobeOllama/probeVllmon the interactive path; non-interactive runs keep the preflight gate. Selection-time containment (assertRouteCompatibleinguardProviderInferenceRouteSelection) is unchanged and remains the enforcement point for shared-gateway route conflicts.src/lib/onboard/ollama-install-menu.ts: renderStart local Ollama (<host>)when the daemon is installed but not running; the running label (Local Ollama (<host>) — running (suggested)) is unchanged. The entry stays because selecting it is the deliberate auto-start path — suppressing it would leave WSL users on native Docker with no in-wizard path to a local Ollama.src/lib/onboard/setup-nim-flow.test.ts: update the probe-gating contract test to the new interactive behavior.src/lib/onboard/setup-nim-provider-discovery.test.ts(new) andsrc/lib/onboard/ollama-install-menu.test.ts: regression tests that fail without the fix, plus guards pinning the retained non-interactive gate.docs/get-started/windows-preparation.mdx,docs/inference/set-up-ollama.mdx: state the Docker Desktop WSL integration requirement for the Windows-host Ollama path and describe the interactive re-prompt accurately (the third symptom in [macOS][Onboard] onboard inference menu: Ollama items display incorrect status on macOS and WSL2 #6750 — the(requires Docker Desktop WSL integration)suffix — is working as designed; the expected(loopback only — switch to 0.0.0.0)wording never existed in the codebase).Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project cli src/lib/onboard/— 294/295 files pass (2905 tests); new regression tests confirmed red before the fix and green after; the one failing file (created-sandbox-finalization.test.ts, 2 tests, [All Platforms][Onboard] --fresh re-onboard keeps the old in-sandbox model — nemoclaw status shows the new model while the agent still runs the previous one #6311) fails identically on pristineupstream/mainand is unrelatednpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only) — 0 errors; the 2 reported warnings are pre-existing (identical count on a build without this change)Signed-off-by: Dongni Yang dongniy@nvidia.com
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
NEMOCLAW_NO_EXPRESS=1).