feat(install): offer express install on Windows WSL#3824
Conversation
Signed-off-by: zyang-dev <267119621+zyang-dev@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: Enterprise Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds Windows WSL as a supported express-install platform: installer WSL detection and provider-selection branch, a typed provider-key fallback used by onboarding, updated help/docs, and tests validating detection and provider-selection (including pre-installed Ollama). ChangesWindows WSL Express Install Support
Sequence DiagramsequenceDiagram
participant User as CLI installer
participant scripts as scripts/install.sh
participant detect as detect_express_platform()
participant resolver as resolveProviderKeyFallback()
participant onboard as setupNim()
User->>scripts: run installer (interactive)
scripts->>detect: detect_express_platform()
detect->>scripts: "Windows WSL" (if is_wsl_host)
scripts->>scripts: set NEMOCLAW_PROVIDER=install-windows-ollama
scripts->>onboard: invoke onboarding with providerKey
onboard->>resolver: resolveProviderKeyFallback(options, providerKey, {isWindowsHostOllama})
resolver-->>onboard: selected provider option (or undefined)
onboard-->>User: perform selected provider action
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-3824.docs.buildwithfern.com/nemoclaw |
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/get-started/quickstart.md (1)
67-67: ⚡ Quick winUse present tense instead of modal verb.
Change "can offer" to "offers" to describe current behavior in present tense rather than possibility.
As per coding guidelines, use present tense for descriptions of current behavior.
Suggested fix
-On DGX Spark, DGX Station, and Windows WSL, an interactive installer can offer express install after you accept the third-party software notice. +On DGX Spark, DGX Station, and Windows WSL, an interactive installer offers express install after you accept the third-party software notice.🤖 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 `@docs/get-started/quickstart.md` at line 67, Update the sentence "On DGX Spark, DGX Station, and Windows WSL, an interactive installer can offer express install after you accept the third-party software notice." to use present tense by replacing "can offer" with "offers" so it reads "...an interactive installer offers express install..."; make this change in docs/get-started/quickstart.md where that sentence appears.docs/get-started/quickstart.mdx (1)
54-54: ⚡ Quick winUse present tense instead of modal verb.
Change "can offer" to "offers" to describe current behavior in present tense rather than possibility.
As per coding guidelines, use present tense for descriptions of current behavior.
Suggested fix
-On DGX Spark, DGX Station, and Windows WSL, an interactive installer can offer express install after you accept the third-party software notice. +On DGX Spark, DGX Station, and Windows WSL, an interactive installer offers express install after you accept the third-party software notice.🤖 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 `@docs/get-started/quickstart.mdx` at line 54, Update the sentence in the quickstart doc where it currently reads "On DGX Spark, DGX Station, and Windows WSL, an interactive installer can offer express install after you accept the third-party software notice." to use present tense by replacing "can offer" with "offers" so it reads "an interactive installer offers express install"; edit the line in docs/get-started/quickstart.mdx accordingly.
🤖 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 `@scripts/install.sh`:
- Around line 2121-2134: Docker preflight checks still rely on
environment-variable-only detection causing incorrect Linux Docker install paths
on some WSL setups; update the Docker preflight logic to call and reuse the
canonical is_wsl_host() function (instead of checking only
WSL_DISTRO_NAME/WSL_INTEROP) wherever the script currently branches for WSL vs
non-WSL Docker setup (e.g., the Docker install decision block and any checks
referencing WSL env vars); replace the env-var-only condition with a call to
is_wsl_host and ensure the resulting truthy/falsey result drives the
WSL-specific Docker branch so /proc-based WSL detection is honored.
In `@src/lib/onboard.ts`:
- Around line 6403-6417: The block that computes selected for providerKey values
"install-windows-ollama" and "start-windows-ollama" should be extracted into a
small helper (e.g., mapWindowsOllamaOption) under src/lib/onboard/ so this file
only makes a thin call; create a helper that accepts (providerKey, options,
ollamaHost, OLLAMA_HOST_DOCKER_INTERNAL) and returns the appropriate option (the
same logic that checks for "start-windows-ollama", "install-windows-ollama" and
picks "start-windows-ollama" or "ollama" when ollamaHost ===
OLLAMA_HOST_DOCKER_INTERNAL), then replace the large inline if/else in
onboard.ts with selected = mapWindowsOllamaOption(providerKey, options,
ollamaHost).
---
Nitpick comments:
In `@docs/get-started/quickstart.md`:
- Line 67: Update the sentence "On DGX Spark, DGX Station, and Windows WSL, an
interactive installer can offer express install after you accept the third-party
software notice." to use present tense by replacing "can offer" with "offers" so
it reads "...an interactive installer offers express install..."; make this
change in docs/get-started/quickstart.md where that sentence appears.
In `@docs/get-started/quickstart.mdx`:
- Line 54: Update the sentence in the quickstart doc where it currently reads
"On DGX Spark, DGX Station, and Windows WSL, an interactive installer can offer
express install after you accept the third-party software notice." to use
present tense by replacing "can offer" with "offers" so it reads "an interactive
installer offers express install"; edit the line in
docs/get-started/quickstart.mdx accordingly.
🪄 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: 033978cc-7462-4bf5-b54c-ffacbdfa4eb4
📒 Files selected for processing (11)
docs/get-started/quickstart.mddocs/get-started/quickstart.mdxdocs/get-started/windows-preparation.mddocs/get-started/windows-preparation.mdxdocs/reference/commands.mddocs/reference/commands.mdxinstall.shscripts/install.shsrc/lib/onboard.tstest/install-preflight.test.tstest/onboard-selection.test.ts
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed the WSL express install path, including installer detection, provider fallback behavior, docs, and targeted validation. The resolved CodeRabbit concerns are addressed at the current head, and the focused build/docs/test coverage passes locally.
## Summary Refreshes the NemoClaw docs for v0.0.46 by updating version metadata, release notes, and generated user skills. The refresh also keeps public docs aligned with the docs skip list by removing non-public experimental references from the generated output. ## Related Issue None. ## Changes - #3744 and #3824 -> `docs/about/release-notes.mdx`: Added Windows bootstrap and WSL express install coverage for v0.0.46. - #3392 -> `docs/manage-sandboxes/messaging-channels.mdx`, `docs/reference/commands.mdx`, `docs/reference/network-policies.mdx`, and policy examples: Refreshed public messaging channel docs around WhatsApp and matching policy presets. - #3742, #3767, #3732, #3786, #3777, and #3808 -> `docs/about/release-notes.mdx`: Added release-note coverage for Hermes managed tools, Bedrock Runtime endpoint detection, WSL Ollama proxying, Model Router Python fallback, plugin command registration, and tool-catalog latency improvements. - #3124 -> `docs/about/release-notes.mdx`: Added release-note coverage for hosted uninstall flag guidance. - Generated `nemoclaw-user-*` skills from the updated MDX docs for the v0.0.46 release. ## 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) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [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) Verification notes: - Commit hooks passed, including markdownlint, gitleaks, docs-to-skills verification, env-var docs, and skills YAML checks. - `python3 scripts/docs-to-skills.py docs/ .agents/skills/ --prefix nemoclaw-user --doc-platform fern-mdx` passed. - `bash test/e2e/e2e-cloud-experimental/check-docs.sh --only-links --local-only --with-skills` passed. - `git diff --check` passed. - `make docs` was attempted but blocked before MDX validation because `npx` received HTTP 403 fetching `fern-api` from npm. --- Signed-off-by: Miyoung Choi <miyoungc@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Released v0.0.46: improved Windows setup, WhatsApp messaging support, Hermes sandbox/tool routing, Anthropic endpoint compatibility, Ollama proxy routing, model-router fallback, OpenClaw plugin/backup compatibility, sandbox build tooling fixes, and updated uninstall flag behavior. * **Documentation** * Removed WeChat from messaging flows and presets across guides and CLI docs; clarified onboarding and channel setup for WhatsApp. Clarified runtime mutability and filesystem (Landlock) behavior — some changes require sandbox rebuilds; prefer host-side commands for durable config. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3911?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary - force the installer Docker bootstrap unit-test helper to exercise the non-WSL Linux path - keep the real installer behavior unchanged: WSL hosts still skip Linux Docker bootstrap ## Why PR #3824 broadened WSL detection from WSL env vars to `/proc` probes via `is_wsl_host`. That is correct for runtime behavior, but these unit tests are specifically validating Linux Docker bootstrap branches with stubbed `docker`, `id`, `sudo`, and `systemctl`. On the real WSL main-watch runner, `/proc` identifies WSL, so `ensure_docker` returns before touching the stubs and the tests fail with empty output. This overrides `is_wsl_host` inside that sourced-test harness only, so the tests keep validating the intended Linux branches on every host. ## Test plan - `npm ci --ignore-scripts` - `npx vitest run test/install-preflight.test.ts --testNamePattern "installer Docker bootstrap" --testTimeout 60000` Fixes the WSL-only failures in `CI / Platform Vitest Main Watch` after #3824. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Enhanced testing for Linux Docker bootstrap procedures to ensure comprehensive coverage of installation scenarios across different system configurations. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/NVIDIA/NemoClaw/pull/3899?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Enable express install for Windows WSL by routing the installer’s express path to Windows-host Ollama setup. This gives WSL users the same guided non-interactive setup path already available on DGX platforms.
Changes
NEMOCLAW_PROVIDER=install-windows-ollama.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: zyang-dev 267119621+zyang-dev@users.noreply.github.com
Summary by CodeRabbit
New Features
Documentation
Tests