Skip to content

fix(windows): enable Docker WSL integration and avoid Ubuntu first-run races#4346

Merged
ericksoa merged 11 commits into
mainfrom
fix/windows-docker-wsl-integration
May 28, 2026
Merged

fix(windows): enable Docker WSL integration and avoid Ubuntu first-run races#4346
ericksoa merged 11 commits into
mainfrom
fix/windows-docker-wsl-integration

Conversation

@zyang-dev
Copy link
Copy Markdown
Contributor

@zyang-dev zyang-dev commented May 27, 2026

Summary

Enables Docker Desktop WSL integration automatically in the Windows bootstrap flow. The bootstrap now waits for Ubuntu first-run account creation before touching Docker integration, avoiding WSL/OOBE races that can leave users in a root shell or break Docker’s distro proxy setup.

Changes

  • Install missing Ubuntu through the normal WSL first-run/OOBE path in a separate PowerShell window.
  • Wait for the Ubuntu default Unix user to be registered before continuing Docker setup.
  • Enable Docker Desktop WSL integration for Ubuntu-24.04 via settings.
  • Avoid changing the global WSL default distro.
  • Restart Docker Desktop only when it was already running before settings were patched.
  • Add/update Windows bootstrap tests for install ordering, Docker settings, and launch behavior.
  • Add WSL integration hints for Docker daemon access failures during onboard.

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

  • npx prek run --all-files passes
  • npm test passes
  • 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: zyang-dev 267119621+zyang-dev@users.noreply.github.com

Summary by CodeRabbit

  • New Features

    • Improved Windows/WSL Docker Desktop integration and WSL distro bootstrap with automated configuration, restart orchestration, and safer install handoffs.
  • New UX Guidance

    • Added contextual, WSL-specific error guidance and remediation hints for Docker connectivity issues.
  • Tests

    • Expanded Windows/WSL-focused tests covering PowerShell harness, distro registration/first-run flows, Docker Desktop orchestration, and final handoff scenarios.
  • Chores

    • CI/workflow updates to provision and run typed scenarios inside an ext4-backed WSL workspace with validation checks.

Review Change Stack

…n races

Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
@zyang-dev zyang-dev self-assigned this May 27, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 27, 2026

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: 8a7fd9ba-5aa9-47ab-aa7c-c36e97e6d6c3

📥 Commits

Reviewing files that changed from the base of the PR and between 8fed0e6 and 64a87fc.

📒 Files selected for processing (2)
  • .github/workflows/e2e-scenarios.yaml
  • tools/e2e-scenarios/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/e2e-scenarios.yaml

📝 Walkthrough

Walkthrough

Enables and configures Docker Desktop WSL integration from the Windows bootstrap script, rewrites WSL distro install/readiness orchestration, threads WSL-aware onboarding hints into error messages, updates tests to assert orchestration and hints, and adds CI workflow steps for WSL setup and ext4 workspace syncing.

Changes

Docker Desktop WSL Integration and Windows Bootstrap Orchestration

Layer / File(s) Summary
PowerShell utility helpers
scripts/bootstrap-windows.ps1
Adds ConvertTo-PowerShellLiteral for safe quoting, Test-DockerDesktopRunning for process detection, and removes legacy Resolve-UbuntuLauncher.
Docker Desktop WSL integration configuration
scripts/bootstrap-windows.ps1
Implements Get-DockerDesktopSettingsPath, Set-JsonProperty, and Enable-DockerDesktopWslIntegration to locate, back up, and modify Docker Desktop settings JSON; updates Start-DockerDesktop to restart when already running.
WSL distro registration & lifecycle management
scripts/bootstrap-windows.ps1
Adds registration polling (Wait-WslDistroRegistration), default-user readiness via Lxss registry (Get-WslDistroDefaultUid, Wait-WslDefaultUserReady), PowerShell-windowed install (Start-WslInstallInPowerShellWindow), distro lifecycle helpers (Stop-WslDistroForDockerIntegration, Assert-WslDistroStarts, Ensure-WslDockerCliConfigDirectory); rewrites Ensure-UbuntuWsl to wait inline for readiness and adds Open-WslInPowerShellWindow for Ubuntu handoff.
TypeScript WSL detection & remediation hints
src/lib/onboard/preflight.ts, src/lib/onboard/bridge-dns-preflight.ts, src/lib/onboard/gateway-sandbox-reachability.ts
Exports DOCKER_DESKTOP_WSL_INTEGRATION_HINT and FormatSandboxBridgeUnreachableMessageOptions; adds isRunningInWsl detection; threads optional host.isWsl parameter through printDockerBridgeContainerStartFailure, DNS probe paths, formatSandboxBridgeUnreachableMessage, and preflight remediation to conditionally append WSL integration guidance.
Test updates for bootstrap and onboarding
test/bootstrap-windows.test.ts, src/lib/onboard/bridge-dns-preflight.test.ts, src/lib/onboard/gateway-sandbox-reachability.test.ts
Updates Windows bootstrap tests to inject TEMP/TMP into PowerShell env, validate Docker Desktop orchestration sequences, inline distro registration/readiness flow, and assert settings backup/creation; adds onboarding tests for WSL-specific docker_daemon_unreachable hints.
E2E workflow validation and WSL testing setup
.github/workflows/e2e-scenarios.yaml, tools/e2e-scenarios/workflow-boundary.mts
Adds WSL distro initialization (install via wsl --install, set default, verify OS, install dependencies, install Node.js 22, rsync to ext4 workspace) for wsl-repo-cloud-openclaw scenario; updates in-WSL execution to run from WSL_WORKDIR and mirror artifacts back; validates workflow includes new WSL steps and directory changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related issues

  • #4301: Docker Desktop WSL-integration toggling and install-at-handoff behavior overlap with this PR’s changes to enabling integration for freshly-registered distros.

Possibly related PRs

  • NVIDIA/NemoClaw#4101: Overlaps on scripts/bootstrap-windows.ps1 WSL distro bootstrap and registration logic for Ubuntu-24.04.
  • NVIDIA/NemoClaw#4278: Modifies scripts/bootstrap-windows.ps1 handoff/install sequencing and Docker-in-WSL verification similar to this PR.
  • NVIDIA/NemoClaw#4227: Related changes to bridge/DNS fatal reporting and docker_daemon_unreachable remediation messaging.

Suggested labels

Docker, Getting Started, CI/CD, fix

Suggested reviewers

  • ericksoa
  • cv

"I hopped through scripts and JSON shores,
Whispered to WSL to open its doors,
I taught Docker Desktop to mind its floors,
Tests and workflows synced ext4 floors,
Hooray — the distro boots and the rabbit roars!"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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 changes: enabling Docker WSL integration and fixing Ubuntu first-run races in the Windows bootstrap flow.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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/windows-docker-wsl-integration

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

@zyang-dev zyang-dev added the Platform: Windows/WSL Support for Windows Subsystem for Linux label May 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

E2E Advisor Recommendation

Required E2E: wsl-repo-cloud-openclaw, ubuntu-repo-cloud-openclaw
Optional E2E: ubuntu-no-docker-preflight-negative, macos-repo-cloud-openclaw

Dispatch hint: wsl-repo-cloud-openclaw and ubuntu-repo-cloud-openclaw must be dispatched separately because the workflow rejects batches spanning multiple runner types; optional follow-ups: ubuntu-no-docker-preflight-negative, macos-repo-cloud-openclaw

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • wsl-repo-cloud-openclaw (high): Required because this PR changes both the Windows-hosted WSL scenario workflow and WSL-specific Docker/onboarding behavior. This existing scenario runs the OpenClaw cloud onboarding flow on a windows-latest runner through WSL and exercises the workflow path most likely to regress.
  • ubuntu-repo-cloud-openclaw (medium): Required because bridge/DNS preflight and sandbox bridge gateway reachability are core Linux Docker onboarding and sandbox lifecycle gates. The baseline OpenClaw cloud scenario exercises onboarding, gateway health, sandbox smoke, credentials, and inference on the standard Ubuntu path.

Optional E2E

  • ubuntu-no-docker-preflight-negative (low): Useful adjacent confidence for preflight failure handling because this PR changes Docker daemon/start remediation text and onboarding preflight behavior, though it does not specifically model Docker Desktop WSL integration.
  • macos-repo-cloud-openclaw (medium): Optional cross-platform confidence for Docker Desktop-style host handling and onboarding without relying on Linux systemd assumptions; not merge-blocking because the changed behavior is primarily WSL/Windows and Linux Docker preflight.

New E2E recommendations

  • windows-bootstrap-installer (high): Existing WSL scenario coverage exercises the CI WSL runner path, but not the real user Windows bootstrap script end-to-end: elevation/resume, Ubuntu first-run user creation, Docker Desktop settings-store mutation, Docker Desktop restart/minimize behavior, Docker reachability from WSL, and final installer handoff.
    • Suggested test: Add a Windows bootstrap E2E that runs scripts/bootstrap-windows.ps1 on a disposable Windows runner or VM with WSL and Docker Desktop, then verifies the target Ubuntu distro starts, Docker is reachable from WSL, Docker Desktop WSL integration includes the distro, and the installer handoff opens the expected WSL shell.
  • wsl-docker-desktop-negative-preflight (medium): The new WSL-specific remediation hints for Docker daemon access failures are covered by unit tests but not by a scenario that intentionally models Docker Desktop WSL integration disabled or unreachable from WSL.
    • Suggested test: Add a negative typed scenario for WSL Docker Desktop integration disabled/unreachable that expects onboarding to fail during preflight with the Docker Desktop > Settings > Resources > WSL integration remediation and no sandbox creation.

Dispatch hint

  • Workflow: E2E / Scenario Runner (.github/workflows/e2e-scenarios.yaml)
  • jobs input: wsl-repo-cloud-openclaw and ubuntu-repo-cloud-openclaw must be dispatched separately because the workflow rejects batches spanning multiple runner types; optional follow-ups: ubuntu-no-docker-preflight-negative, macos-repo-cloud-openclaw

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

E2E Scenario Advisor Recommendation

Required scenario E2E: e2e-scenarios-all
Optional scenario E2E: None

Dispatch required scenario E2E:

  • gh workflow run e2e-scenarios-all.yaml --ref <pr-head-ref>

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • e2e-scenarios-all: the reusable single-scenario workflow changed
    • Dispatch: gh workflow run e2e-scenarios-all.yaml --ref <pr-head-ref>

Optional scenario E2E

  • None.

Relevant changed files

  • .github/workflows/e2e-scenarios.yaml

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 27, 2026

PR Review Advisor

Findings: 0 needs attention, 12 worth checking, 1 nice ideas
Since last review: 2 prior items resolved, 8 still apply, 1 new item found

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: Enable-DockerDesktopWslIntegration private Docker Desktop settings patch: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Enable-DockerDesktopWslIntegration` writes `wslEngineEnabled`, `enableIntegrationWithDefaultWslDistro`, and `integratedWslDistros` directly.
  • Source-of-truth review needed: Docker Desktop settings corrupt JSON tolerant handling: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Enable-DockerDesktopWslIntegration` catches `ConvertFrom-Json` failure, warns, and leaves settings unchanged.
  • Source-of-truth review needed: Wait-WslDefaultUserReady registry polling: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Wait-WslDefaultUserReady` polls `Get-WslDistroDefaultUid` until a value greater than zero is returned.
  • Source-of-truth review needed: Test-DockerDesktopRunning fallback: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Test-DockerDesktopRunning` catches and returns `false`; `Start-DockerDesktop` restarts only when `$wasRunning` is true.
  • Source-of-truth review needed: Nested PowerShell WSL install/launch compatibility handling: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Start-WslInstallInPowerShellWindow` and `Open-WslInPowerShellWindow` build nested `-Command` strings and call `Start-Process -FilePath 'powershell.exe'`.
  • Pin or verify the WSL Node.js installer before secret-bearing workflow steps (.github/workflows/e2e-scenarios.yaml:236): The WSL scenario path executes the remote NodeSource setup script with `curl -fsSL https://deb.nodesource.com/setup\_22.x | bash -` before the later WSL scenario step receives `NVIDIA_API_KEY`. Actions are SHA-pinned and `npm ci --ignore-scripts` is retained, but this new installer is not checksum-, digest-, package-version-, or signing-key-fingerprint verified in the workflow trusted-code boundary.
    • Recommendation: Use a deterministic Node installation path for WSL: pin and verify the apt signing key fingerprint plus package version constraints, use a checked-in installer with checksum verification, or use another verifiable mechanism. Update `tools/e2e-scenarios/workflow-boundary.mts` to reject unpinned `curl | bash` installers instead of requiring the `setup_22.x` string.
    • Evidence: `.github/workflows/e2e-scenarios.yaml` adds `Install Node.js 22 in WSL` with `curl -fsSL https://deb.nodesource.com/setup\_22.x | bash -`; the later `Run typed scenarios in WSL` step sets `NVIDIA_API_KEY` and invokes `wsl -d ... -- env ... bash -l $wslTmp`. `workflow-boundary.mts` only checks for `setup_22.x` and `npm --version`.
  • Document and harden the Docker Desktop private settings patch (scripts/bootstrap-windows.ps1:306): `Enable-DockerDesktopWslIntegration` directly edits Docker Desktop private settings to enable the WSL engine and grant the target distro Docker daemon access. That may be necessary for the Windows bootstrap, but it crosses a local trust boundary and relies on a private schema without an inline source-of-truth note or schema-drift handling coverage.
    • Recommendation: Add an inline source-of-truth comment describing the invalid state, the Docker Desktop private-config boundary, why a supported Docker Desktop API/CLI cannot be used here, the Docker-daemon access implication, schema-drift expectations, and the removal condition. Add negative tests for corrupt JSON, unexpected top-level JSON types, legacy `settings.json`, and scalar/object/mixed `integratedWslDistros` values.
    • Evidence: `Enable-DockerDesktopWslIntegration` writes `wslEngineEnabled`, `enableIntegrationWithDefaultWslDistro`, and `integratedWslDistros` under `%APPDATA%\Docker`; tests cover a normal existing object and a missing settings file but not corrupt/schema-drift cases.
  • Document and test the WSL DefaultUid readiness signal (scripts/bootstrap-windows.ps1:807): The bootstrap waits for HKCU Lxss `DefaultUid > 0` as the signal that first-run Unix user creation is complete before proceeding with Docker integration. The code does not explain why this registry field is authoritative for the WSL/OOBE race or why startup verification alone is insufficient, and malformed registry values can throw out of the wait path.
    • Recommendation: Document the WSL first-run invalid state, the Lxss registry boundary, why the source cannot be fixed in this PR, and when the workaround can be removed. Add tests for missing registry data, `DefaultUid = 0`, non-numeric/overflow `DefaultUid`, and timeout behavior.
    • Evidence: `Wait-WslDefaultUserReady` polls `Get-WslDistroDefaultUid` until it returns a value greater than zero; the missing-distro test stubs `Wait-WslDefaultUserReady` to return `1000` rather than exercising missing/zero/malformed registry states.
  • Do not treat Docker Desktop running-state probe failures as not running (scripts/bootstrap-windows.ps1:234): `Test-DockerDesktopRunning` catches process-enumeration failures and returns `false`. If Docker Desktop is actually running but the probe fails after settings were modified, `Start-DockerDesktop` takes the not-running branch and skips the restart intended to make the WSL integration settings take effect.
    • Recommendation: Represent the running-state probe as true, false, or indeterminate. After modifying Docker Desktop settings, conservatively restart Docker Desktop when the previous running state is indeterminate. Add a negative test where the running-state probe throws after settings are patched.
    • Evidence: `Test-DockerDesktopRunning` returns `false` from its catch block; `Start-DockerDesktop` calls `Restart-DockerDesktop` only when `$wasRunning` is true. Tests cover true and false stubs, not a throwing or indeterminate probe.
  • Resolve child PowerShell through a trusted absolute path (scripts/bootstrap-windows.ps1:139): The bootstrap launches child PowerShell processes by the bare executable name in self-elevation, RunOnce resume, WSL install, and final WSL launch paths. In installer/elevation code, executable-name resolution is more sensitive than using a validated absolute Windows PowerShell or pwsh path.
    • Recommendation: Resolve PowerShell once to a trusted absolute path, such as `%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe` or a validated `pwsh` path, and use that path in `Invoke-SelfElevation`, `Register-ResumeRunOnce`, `Start-WslInstallInPowerShellWindow`, and `Open-WslInPowerShellWindow`. Add adversarial tests for distro names and resolved paths containing quotes, semicolons, backticks, whitespace, leading dashes, and newlines.
    • Evidence: `Start-Process -FilePath 'powershell.exe'` remains in the bootstrap, and `Register-ResumeRunOnce` stores a command beginning with `powershell.exe`; nested `-Command` tests assert only normal `Ubuntu-24.04` command text.
  • Add targeted runtime validation for the WSL and Docker Desktop bootstrap path (scripts/bootstrap-windows.ps1:1094): The PowerShell harness tests improve branch coverage, but the changed behavior depends on real Windows, WSL first-run timing, HKCU registry updates, Docker Desktop private settings ingestion, Docker Desktop start/restart behavior, and Docker daemon reachability from WSL. Those boundaries are mocked or stubbed in unit tests.
    • Recommendation: Add or identify targeted manual/integration validation for a missing-distro first run, an existing distro with Docker Desktop integration disabled, corrupt/schema-drifted Docker Desktop settings, an indeterminate Docker Desktop running-state probe, and `-InstallDockerDesktop:$false`. Do not rely only on mocked PowerShell harness tests for these host-integration paths.
    • Evidence: `test/bootstrap-windows.test.ts` stubs WSL registration/readiness, `Start-Process`, Docker Desktop running state, Docker readiness, and WSL termination; deterministic test-depth analysis recommends runtime validation for the changed workflow/bootstrap/onboarding paths.
  • Update Windows setup docs or release notes for the new bootstrap behavior (docs/get-started/windows-preparation.mdx:25): The PR changes user-facing Windows bootstrap behavior: missing Ubuntu is installed through first-run setup before Docker integration, Docker Desktop WSL integration may be patched automatically, Docker Desktop may be restarted conditionally, and WSL-specific daemon-access hints are added. The existing Windows preparation page broadly describes the bootstrap but does not explain the new automatic private settings patch or its Docker-daemon access implication.
    • Recommendation: Update the Windows preparation docs or add a maintainer-facing release note explaining the first-run wait, automatic Docker Desktop WSL integration behavior, no-global-default behavior, and trust/permission implications. If no docs are needed, record that rationale in the PR.
    • Evidence: `scripts/bootstrap-windows.ps1` changes handoff and Docker integration behavior; `docs/get-started/windows-preparation.mdx` was not changed; the PR checklist leaves `Docs updated for user-facing behavior changes` unchecked.

🌱 Nice ideas

  • Consider extracting bootstrap helper responsibilities (scripts/bootstrap-windows.ps1:1): The Windows bootstrap script now contains WSL feature enablement, WSL install/OOBE orchestration, registry polling, Docker Desktop private settings mutation, process/window management, installer handoff, and test seams in one large file. The size makes trust-boundary behavior harder to review.
    • Recommendation: If this script continues to grow, consider extracting cohesive helpers or adding stronger section-level documentation around installer trust boundaries and host-integration workarounds.
    • Evidence: Diff stat shows `scripts/bootstrap-windows.ps1` grew substantially; drift analysis flags the bootstrap and related onboarding files as active large-file hotspots.
Since last review details

Current findings:

  • Source-of-truth review needed: Enable-DockerDesktopWslIntegration private Docker Desktop settings patch: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Enable-DockerDesktopWslIntegration` writes `wslEngineEnabled`, `enableIntegrationWithDefaultWslDistro`, and `integratedWslDistros` directly.
  • Source-of-truth review needed: Docker Desktop settings corrupt JSON tolerant handling: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Enable-DockerDesktopWslIntegration` catches `ConvertFrom-Json` failure, warns, and leaves settings unchanged.
  • Source-of-truth review needed: Wait-WslDefaultUserReady registry polling: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Wait-WslDefaultUserReady` polls `Get-WslDistroDefaultUid` until a value greater than zero is returned.
  • Source-of-truth review needed: Test-DockerDesktopRunning fallback: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Test-DockerDesktopRunning` catches and returns `false`; `Start-DockerDesktop` restarts only when `$wasRunning` is true.
  • Source-of-truth review needed: Nested PowerShell WSL install/launch compatibility handling: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: `Start-WslInstallInPowerShellWindow` and `Open-WslInPowerShellWindow` build nested `-Command` strings and call `Start-Process -FilePath 'powershell.exe'`.
  • Pin or verify the WSL Node.js installer before secret-bearing workflow steps (.github/workflows/e2e-scenarios.yaml:236): The WSL scenario path executes the remote NodeSource setup script with `curl -fsSL https://deb.nodesource.com/setup\_22.x | bash -` before the later WSL scenario step receives `NVIDIA_API_KEY`. Actions are SHA-pinned and `npm ci --ignore-scripts` is retained, but this new installer is not checksum-, digest-, package-version-, or signing-key-fingerprint verified in the workflow trusted-code boundary.
    • Recommendation: Use a deterministic Node installation path for WSL: pin and verify the apt signing key fingerprint plus package version constraints, use a checked-in installer with checksum verification, or use another verifiable mechanism. Update `tools/e2e-scenarios/workflow-boundary.mts` to reject unpinned `curl | bash` installers instead of requiring the `setup_22.x` string.
    • Evidence: `.github/workflows/e2e-scenarios.yaml` adds `Install Node.js 22 in WSL` with `curl -fsSL https://deb.nodesource.com/setup\_22.x | bash -`; the later `Run typed scenarios in WSL` step sets `NVIDIA_API_KEY` and invokes `wsl -d ... -- env ... bash -l $wslTmp`. `workflow-boundary.mts` only checks for `setup_22.x` and `npm --version`.
  • Document and harden the Docker Desktop private settings patch (scripts/bootstrap-windows.ps1:306): `Enable-DockerDesktopWslIntegration` directly edits Docker Desktop private settings to enable the WSL engine and grant the target distro Docker daemon access. That may be necessary for the Windows bootstrap, but it crosses a local trust boundary and relies on a private schema without an inline source-of-truth note or schema-drift handling coverage.
    • Recommendation: Add an inline source-of-truth comment describing the invalid state, the Docker Desktop private-config boundary, why a supported Docker Desktop API/CLI cannot be used here, the Docker-daemon access implication, schema-drift expectations, and the removal condition. Add negative tests for corrupt JSON, unexpected top-level JSON types, legacy `settings.json`, and scalar/object/mixed `integratedWslDistros` values.
    • Evidence: `Enable-DockerDesktopWslIntegration` writes `wslEngineEnabled`, `enableIntegrationWithDefaultWslDistro`, and `integratedWslDistros` under `%APPDATA%\Docker`; tests cover a normal existing object and a missing settings file but not corrupt/schema-drift cases.
  • Document and test the WSL DefaultUid readiness signal (scripts/bootstrap-windows.ps1:807): The bootstrap waits for HKCU Lxss `DefaultUid > 0` as the signal that first-run Unix user creation is complete before proceeding with Docker integration. The code does not explain why this registry field is authoritative for the WSL/OOBE race or why startup verification alone is insufficient, and malformed registry values can throw out of the wait path.
    • Recommendation: Document the WSL first-run invalid state, the Lxss registry boundary, why the source cannot be fixed in this PR, and when the workaround can be removed. Add tests for missing registry data, `DefaultUid = 0`, non-numeric/overflow `DefaultUid`, and timeout behavior.
    • Evidence: `Wait-WslDefaultUserReady` polls `Get-WslDistroDefaultUid` until it returns a value greater than zero; the missing-distro test stubs `Wait-WslDefaultUserReady` to return `1000` rather than exercising missing/zero/malformed registry states.
  • Do not treat Docker Desktop running-state probe failures as not running (scripts/bootstrap-windows.ps1:234): `Test-DockerDesktopRunning` catches process-enumeration failures and returns `false`. If Docker Desktop is actually running but the probe fails after settings were modified, `Start-DockerDesktop` takes the not-running branch and skips the restart intended to make the WSL integration settings take effect.
    • Recommendation: Represent the running-state probe as true, false, or indeterminate. After modifying Docker Desktop settings, conservatively restart Docker Desktop when the previous running state is indeterminate. Add a negative test where the running-state probe throws after settings are patched.
    • Evidence: `Test-DockerDesktopRunning` returns `false` from its catch block; `Start-DockerDesktop` calls `Restart-DockerDesktop` only when `$wasRunning` is true. Tests cover true and false stubs, not a throwing or indeterminate probe.
  • Resolve child PowerShell through a trusted absolute path (scripts/bootstrap-windows.ps1:139): The bootstrap launches child PowerShell processes by the bare executable name in self-elevation, RunOnce resume, WSL install, and final WSL launch paths. In installer/elevation code, executable-name resolution is more sensitive than using a validated absolute Windows PowerShell or pwsh path.
    • Recommendation: Resolve PowerShell once to a trusted absolute path, such as `%SystemRoot%\System32\WindowsPowerShell\v1.0\powershell.exe` or a validated `pwsh` path, and use that path in `Invoke-SelfElevation`, `Register-ResumeRunOnce`, `Start-WslInstallInPowerShellWindow`, and `Open-WslInPowerShellWindow`. Add adversarial tests for distro names and resolved paths containing quotes, semicolons, backticks, whitespace, leading dashes, and newlines.
    • Evidence: `Start-Process -FilePath 'powershell.exe'` remains in the bootstrap, and `Register-ResumeRunOnce` stores a command beginning with `powershell.exe`; nested `-Command` tests assert only normal `Ubuntu-24.04` command text.
  • Add targeted runtime validation for the WSL and Docker Desktop bootstrap path (scripts/bootstrap-windows.ps1:1094): The PowerShell harness tests improve branch coverage, but the changed behavior depends on real Windows, WSL first-run timing, HKCU registry updates, Docker Desktop private settings ingestion, Docker Desktop start/restart behavior, and Docker daemon reachability from WSL. Those boundaries are mocked or stubbed in unit tests.
    • Recommendation: Add or identify targeted manual/integration validation for a missing-distro first run, an existing distro with Docker Desktop integration disabled, corrupt/schema-drifted Docker Desktop settings, an indeterminate Docker Desktop running-state probe, and `-InstallDockerDesktop:$false`. Do not rely only on mocked PowerShell harness tests for these host-integration paths.
    • Evidence: `test/bootstrap-windows.test.ts` stubs WSL registration/readiness, `Start-Process`, Docker Desktop running state, Docker readiness, and WSL termination; deterministic test-depth analysis recommends runtime validation for the changed workflow/bootstrap/onboarding paths.
  • Update Windows setup docs or release notes for the new bootstrap behavior (docs/get-started/windows-preparation.mdx:25): The PR changes user-facing Windows bootstrap behavior: missing Ubuntu is installed through first-run setup before Docker integration, Docker Desktop WSL integration may be patched automatically, Docker Desktop may be restarted conditionally, and WSL-specific daemon-access hints are added. The existing Windows preparation page broadly describes the bootstrap but does not explain the new automatic private settings patch or its Docker-daemon access implication.
    • Recommendation: Update the Windows preparation docs or add a maintainer-facing release note explaining the first-run wait, automatic Docker Desktop WSL integration behavior, no-global-default behavior, and trust/permission implications. If no docs are needed, record that rationale in the PR.
    • Evidence: `scripts/bootstrap-windows.ps1` changes handoff and Docker integration behavior; `docs/get-started/windows-preparation.mdx` was not changed; the PR checklist leaves `Docs updated for user-facing behavior changes` unchecked.
  • Consider extracting bootstrap helper responsibilities (scripts/bootstrap-windows.ps1:1): The Windows bootstrap script now contains WSL feature enablement, WSL install/OOBE orchestration, registry polling, Docker Desktop private settings mutation, process/window management, installer handoff, and test seams in one large file. The size makes trust-boundary behavior harder to review.
    • Recommendation: If this script continues to grow, consider extracting cohesive helpers or adding stronger section-level documentation around installer trust boundaries and host-integration workarounds.
    • Evidence: Diff stat shows `scripts/bootstrap-windows.ps1` grew substantially; drift analysis flags the bootstrap and related onboarding files as active large-file hotspots.

Workflow run details

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 `@test/bootstrap-windows.test.ts`:
- Around line 256-257: The test currently sets $settingsDir to a fixed Join-Path
$env:TEMP 'docker-settings-test' which can leave stale files and make
backupCount assertions flaky; change the setup to create a unique settings
directory (e.g., append a GUID or random suffix) and assign that path to
$settingsDir and $env:APPDATA so each test run uses an isolated directory;
ensure the test creates the directory before use and (optionally) removes it
during teardown to avoid accumulation.
🪄 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: da6d23c3-5fd3-4071-a64d-e84e96f5de0e

📥 Commits

Reviewing files that changed from the base of the PR and between be6eefe and e8064f4.

📒 Files selected for processing (8)
  • scripts/bootstrap-windows.ps1
  • src/lib/onboard/bridge-dns-preflight.test.ts
  • src/lib/onboard/bridge-dns-preflight.ts
  • src/lib/onboard/gateway-sandbox-reachability.test.ts
  • src/lib/onboard/gateway-sandbox-reachability.ts
  • src/lib/onboard/preflight.test.ts
  • src/lib/onboard/preflight.ts
  • test/bootstrap-windows.test.ts

Comment thread test/bootstrap-windows.test.ts Outdated
zyang-dev added 3 commits May 27, 2026 14:12
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
Signed-off-by: zyang-dev <267119621+zyang-dev@users.noreply.github.com>
@zyang-dev zyang-dev added the v0.0.55 Release target label May 27, 2026
@ericksoa ericksoa added v0.0.54 Release target and removed v0.0.55 Release target labels May 27, 2026
Copy link
Copy Markdown
Contributor

@cjagwani cjagwani left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26554736464
Target ref: 3426c7727633b9bee88fa2d2fa6a4c6fad74d27f
Workflow ref: main
Requested jobs: cloud-onboard-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success

ericksoa added 2 commits May 27, 2026 21:41
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26555123831
Target ref: 93831c56eeb0435c09ce1b953103fbb866c8c198
Workflow ref: main
Requested jobs: cloud-onboard-e2e
Summary: 0 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ⚠️ cancelled

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26555280009
Target ref: 8fed0e6d479ab49eaedf1b879fad5d95f54bf37a
Workflow ref: main
Requested jobs: cloud-onboard-e2e
Summary: 0 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ⚠️ cancelled

@github-actions
Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 26555411779
Target ref: 64a87fca366b588ad41e9f58e93e88d749d74c33
Workflow ref: main
Requested jobs: cloud-onboard-e2e
Summary: 1 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success

@ericksoa ericksoa merged commit b936002 into main May 28, 2026
33 checks passed
@ericksoa ericksoa deleted the fix/windows-docker-wsl-integration branch May 28, 2026 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Platform: Windows/WSL Support for Windows Subsystem for Linux v0.0.54 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants