Skip to content

fix(onboard): reuse reachable Windows-host Ollama - #7476

Merged
prekshivyas merged 4 commits into
mainfrom
fix/7472-wsl-ollama-reuse
Jul 24, 2026
Merged

fix(onboard): reuse reachable Windows-host Ollama#7476
prekshivyas merged 4 commits into
mainfrom
fix/7472-wsl-ollama-reuse

Conversation

@sandl99

@sandl99 sandl99 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Windows WSL express install now reuses a reachable Windows-host Ollama daemon when PowerShell cannot find its executable. Previously, the provider menu kept the install action and onboarding attempted an unnecessary reinstall.

Related Issue

Fixes #7472

Changes

  • Suppress the Windows-host install action when Ollama already responds through host.docker.internal.
  • Add provider-menu coverage for a reachable daemon whose executable is not detected.
  • Add setup coordinator coverage that verifies express install selects the running Ollama provider.

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)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: The fix restores the documented Windows-host Ollama reuse behavior and changes no command, configuration, or user-facing text.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Exact-head security follow-up passed for 4b139ae77: fix(onboard): reuse reachable Windows-host Ollama #7476 (comment)
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: Head 4b139ae77 adds only an empty signed CI-retry commit. Its tree and effective PR diff match reviewed head 35d0bac66. Existing WSL Ollama documentation covers the corrected behavior. Focused tests, CLI build, CLI type-check, check:diff, and git diff --check passed.
  • Agent: Codex Desktop documentation writer subagent

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit:
  • Station profile/scenario:
  • Result:
  • Supporting evidence:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/onboard/provider-menu.test.ts src/lib/onboard/setup-nim-flow.test.ts src/lib/onboard/setup-nim-ollama.test.ts passed 41/41. npm run typecheck:cli, Biome, and the test-title check passed.
  • Applicable broad gate passed — exact-head CI, E2E, PR Gate, and coordination passed for 4b139ae77 on base 5faed2cbc.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • 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: San Dang sdang@nvidia.com

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 self-assigned this Jul 24, 2026
@copy-pr-bot

copy-pr-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The onboarding provider menu now avoids offering Windows Ollama installation when an existing Windows-host Ollama is configured. Tests cover menu filtering and non-interactive reuse of the reachable host instance.

Changes

Windows-host Ollama onboarding

Layer / File(s) Summary
Windows Ollama menu gating
src/lib/onboard/provider-menu.ts, src/lib/onboard/provider-menu.test.ts
The install-windows-ollama option now requires WSL, no detected Windows Ollama, and !isWindowsHostOllama; a test covers the reachable-host scenario.
Non-interactive Ollama reuse
src/lib/onboard/setup-nim-flow.test.ts, src/lib/onboard/setup-nim-ollama.test.ts
Tests verify that setup invokes the running-Ollama handler and uses the reachable Windows-host endpoint.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: hwangjohn

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes address #7472 by reusing reachable host.docker.internal Ollama and skipping the install action when it is already available.
Out of Scope Changes check ✅ Passed The diff stays focused on WSL Windows-host Ollama reuse and related tests, with no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: reusing a reachable Windows-host Ollama during onboarding.
✨ 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/7472-wsl-ollama-reuse

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 4b139ae in the fix/7472-wsl-ollama-... branch remains at 96%, unchanged from commit 5faed2c in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 4b139ae in the fix/7472-wsl-ollama-... branch remains at 80%, unchanged from commit 5faed2c in the main branch.

Show a code coverage summary of the most impacted files.
File main 5faed2c fix/7472-wsl-ollama-... 4b139ae +/-
src/lib/onboard...der-metadata.ts 100% 100% 0%
src/lib/onboard...rovider-menu.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/tunnel/services.ts 73% 73% 0%
src/lib/domain/.../connect-env.ts 89% 97% +8%

Updated July 24, 2026 17:13 UTC

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / medium confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · medium confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 1 suggestion
  • Model comparison: normalized findings differ; normalized E2E selections differ; Nemotron reported the same number of blockers, the same number of warnings, 1 more suggestion.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: onboard-repair, onboard-resume, cloud-onboard

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

@sandl99

sandl99 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Security review: PASS

Commit b2f64f190 is safe to merge from a security perspective. The change reuses the existing typed Windows-host reachability state, preserves the Docker Desktop support check and downstream model validation, and introduces no credential, network-policy, dependency, or privilege change.

Findings

The review found no security findings.

Detailed analysis

  1. PASS — Secrets and credentials: The diff adds no secret value, credential field, or credential-handling path.
  2. PASS — Input validation and data sanitization: The changed condition consumes existing internal boolean host-state fields and parses no new user input.
  3. PASS — Authentication and authorization: The diff adds no endpoint or authorization decision.
  4. PASS — Dependencies and third-party libraries: The diff changes no dependency or package metadata.
  5. PASS — Error handling and logging: The diff changes no error message or logging path.
  6. PASS — Cryptography and data protection: The diff adds no cryptographic or stored-data behavior.
  7. PASS — Configuration and security headers: The diff changes no runtime configuration, port exposure, container setting, or HTTP header.
  8. PASS — Security testing: The tests cover the reachable Windows-host reuse path. Existing menu coverage still proves that a host with no reachable or detected Ollama offers installation.
  9. PASS — System security: Reuse requires the existing isWindowsHostOllama state. The provider flow retains Docker Desktop support enforcement and downstream Ollama model validation.

Files reviewed

  • src/lib/onboard/provider-menu.ts
  • src/lib/onboard/provider-menu.test.ts
  • src/lib/onboard/setup-nim-flow.test.ts

@sandl99
sandl99 marked this pull request as ready for review July 24, 2026 10:53

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/setup-nim-flow.test.ts`:
- Around line 448-458: The test currently derives endpoint behavior from the
mocked handleRunningOllamaSelection implementation, so it cannot detect
regressions in production endpoint construction. Keep this test focused on
coordinator dispatch, and add or retain a public-boundary test that invokes the
real handler and asserts the observable provider and endpointUrl values,
including Windows-host reuse behavior.
🪄 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: 03a27cb2-733c-4325-a8b6-58b56b27d071

📥 Commits

Reviewing files that changed from the base of the PR and between 46ecb86 and b2f64f1.

📒 Files selected for processing (3)
  • src/lib/onboard/provider-menu.test.ts
  • src/lib/onboard/provider-menu.ts
  • src/lib/onboard/setup-nim-flow.test.ts

Comment thread src/lib/onboard/setup-nim-flow.test.ts
Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99

sandl99 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

Security review follow-up: PASS

Reviewed current head 020eac09b. The only change since the detailed security review is test restructuring: the coordinator test now asserts dispatch only, and a real-handler test asserts the Windows-host Ollama endpoint. Production code, dependencies, credentials, network policy, privileges, and runtime configuration are unchanged. All nine security categories remain PASS with no findings.

Detailed review: #7476 (comment)

@sandl99 sandl99 added bug-fix PR fixes a bug or regression VRDC Issues and PRs submitted by NVIDIA VRDC test team. VDR Linked to VDR finding v0.0.95 labels Jul 24, 2026
@senthilr-nv senthilr-nv assigned prekshivyas and unassigned sandl99 Jul 24, 2026
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

Exact-head security follow-up: PASS

Reviewed current head 35d0bac66 on base 5faed2cbc. The new commit only merges current main; the effective PR diff remains the same four onboarding source/test files reviewed at 020eac09b.

All nine security categories remain PASS with no findings:

  1. No secrets or credential handling changed.
  2. No new untrusted input or parsing was introduced.
  3. No authentication or authorization decision changed.
  4. No dependency changed.
  5. No error or logging behavior changed.
  6. No cryptography or stored-data behavior changed.
  7. No runtime configuration, port exposure, container setting, or header changed.
  8. Focused behavior coverage passed 41/41, including menu, coordinator, and real-handler endpoint coverage.
  9. Reuse still depends on the existing typed Windows-host reachability state and retains downstream Docker support and model validation.

Additional exact-head evidence: CLI build and type-check passed, npm run check:diff passed, the branch is conflict-free, and all three commits are GitHub Verified.

Detailed original review: #7476 (comment)

Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
@prekshivyas

Copy link
Copy Markdown
Collaborator

Exact-head security follow-up: PASS

Reviewed current head 4b139ae77. The only change since the detailed exact-base review at 35d0bac66 is an empty signed CI-retry commit after a transient OpenShell port-forward failure. The tree, effective four-file PR diff, dependencies, credentials, network behavior, privileges, and runtime configuration are identical.

All nine security categories remain PASS with no findings. Focused tests, CLI build, CLI type-check, npm run check:diff, and git diff --check passed on the identical tree.

Detailed exact-base review: #7476 (comment)

@prekshivyas
prekshivyas merged commit 88b3e35 into main Jul 24, 2026
108 of 109 checks passed
@prekshivyas
prekshivyas deleted the fix/7472-wsl-ollama-reuse branch July 24, 2026 17:36
@senthilr-nv senthilr-nv mentioned this pull request Jul 25, 2026
23 tasks
senthilr-nv added a commit that referenced this pull request Jul 25, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Adds the canonical pre-tag `## v0.0.95` release entry to
`docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The
entry summarizes approved user-visible changes merged since v0.0.94 and
excludes internal-only prerequisites.

## Changes

- Adds the v0.0.95 summary and detailed bullets for gateway lifecycle,
recovery, state transfer, inference compatibility, sandbox security,
Discord policy, and E2E evidence.
- Links each user-facing theme to the most specific published
documentation.
- Records the release entry in the shared native changelog used by the
OpenClaw, Hermes, and Deep Agents guides.

Source summary:

- [#7246](#7246),
[#7228](#7228),
[#7267](#7267),
[#7489](#7489),
[#7509](#7509),
[#7351](#7351), and
[#7290](#7290) ->
`docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and
retry, managed recovery, Hermes restart recovery, scoped uninstall, and
orphan-aware backup behavior.
- [#7344](#7344) and
[#7416](#7416) ->
`docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download
verification.
- [#7476](#7476),
[#7347](#7347),
[#7281](#7281),
[#7485](#7485),
[#7491](#7491), and
[#7422](#7422) ->
`docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback,
bounded OpenRouter connection setup, Nemotron-3 request compatibility,
and managed Deep Agents retry and provider-error behavior.
- [#6884](#6884),
[#7481](#7481),
[#6878](#6878),
[#7467](#7467),
[#7502](#7502),
[#7503](#7503),
[#7504](#7504), and
[#7486](#7486) ->
`docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local
rebuild images, runtime validation, config preservation, reviewed
package updates, and fewer final-image payload layers.
- [#7303](#7303) ->
`docs/changelog/2026-07-24.mdx`: Scoped Discord application-command
management.
- [#7488](#7488),
[#7465](#7465),
[#7497](#7497),
[#7464](#7464),
[#7501](#7501),
[#7494](#7494), and
[#7493](#7493) ->
`docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry
cleanup, full root-image validation, direct-main Hermes setup, executed
PR-gate evidence, nightly history, and runner wait reporting.
- [#7447](#7447) is an internal
pinned-runtime prerequisite and is intentionally excluded from canonical
supported-integration documentation.
- [#7370](#7370) adds
maintainer-only advisory reconciliation tooling and does not change
supported user behavior.
- [#7495](#7495) updates existing
documentation and does not add a new v0.0.95 behavior claim.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [x] Existing tests cover changed behavior — justification:
`test/changelog-docs.test.ts` validates the dated changelog structure,
heading uniqueness, and published links.
- [ ] Tests not applicable — justification:
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Documentation Writer Review

- [x] Documentation writer subagent reviewed the completed changes
- Result: `docs-updated`
- Evidence: `docs/changelog/2026-07-24.mdx`; writing rules,
documentation style, factual release meaning, and published links
reviewed at exact head `58b02f2bf`.
- Agent: Codex documentation writer reviewer
<!-- docs-review-head-sha: 58b02f2 -->
<!-- docs-review-agents-blob-sha: 9c9b36d -->

## DGX Station Hardware Evidence

- [ ] Tested on DGX Station
- Tested commit:
- Station profile/scenario:
- Result:
- Supporting evidence:

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — command/result or justification: `npx
vitest run test/changelog-docs.test.ts` passed 6 tests.
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — the
build passed with 0 errors and 2 Fern warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
  * Added a new v0.0.95 changelog entry above v0.0.94.
* Documented improved externally supervised gateway lifecycle ownership.
  * Improved snapshot restore reliability and SQLite state handling.
  * Tightened CLI `backup-all` behavior and host artifact verification.
* Updated Windows onboarding guidance (including Ollama service reuse
and CDI directory fallback).
* Noted inference compatibility fixes, deeper agent failure
classification, stricter base-image validation, updated Discord bot
command permissions, and refined E2E release automation evidence
handling.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@mercl-lau

Copy link
Copy Markdown
Contributor

QA verification update — PARTIAL / FAIL on v0.0.96

Tested on Windows 11 with WSL2 Ubuntu 24.04 x86_64, Docker Desktop 29.6.1, Windows-host Ollama 0.31.1, OpenShell 0.0.85, and OpenClaw 2026.7.1.

Two real onboarding paths were exercised with powershell.exe intentionally unavailable only to the test process, matching the condition where the Windows executable cannot be detected. The existing Windows Ollama daemon remained running and returned HTTP 200.

  1. WSL mirrored networking: the same Windows daemon was reachable through both 127.0.0.1:11434 and host.docker.internal:11434. Detection selected loopback first, treated the provider as install-windows-ollama, and reproduced:

    Installing Ollama on Windows host...
    Failed to spawn powershell.exe: spawn powershell.exe ENOENT
    Install did not produce ollama.exe on PATH.
    

    Onboarding exited 1.

  2. Explicit Windows-host route: with only the loopback probe suppressed for the test process, host.docker.internal:11434 remained HTTP 200. The same request normalized to ollama, skipped installation, and full onboarding exited 0. The sandbox reached Ready with healthy inference. The Windows Ollama process and executable timestamp were unchanged.

The fix works when the reachable daemon resolves as host.docker.internal, but WSL mirrored networking can expose the Windows daemon on loopback first and still take the install path. This remains reproducible on v0.0.96.

Reopening #7472 for the remaining mirrored-networking case.

from kookies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression VDR Linked to VDR finding VRDC Issues and PRs submitted by NVIDIA VRDC test team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[WSL2][Install] express install fails with "Install did not produce ollama.exe on PATH" when Ollama already installed

3 participants