Skip to content

Make local WSL setup deterministic - #559

Closed
ranjeshj wants to merge 2 commits into
openclaw:masterfrom
ranjeshj:verifyubuntu
Closed

Make local WSL setup deterministic#559
ranjeshj wants to merge 2 commits into
openclaw:masterfrom
ranjeshj:verifyubuntu

Conversation

@ranjeshj

Copy link
Copy Markdown
Collaborator

Summary

This PR makes local gateway setup deterministic by removing the hidden dependency on an existing local Ubuntu distro. Setup now creates a fresh app-owned WSL gateway distro directly from the configured Ubuntu release every run, and fails explicitly when the host WSL version or install path cannot support that clean-start contract.

What changed

  • Replaced the old wsl --export Ubuntu-24.04 / wsl --import flow with direct named install:
    • wsl --install --distribution <BaseDistro> --name <DistroName> --location <app-owned path> --no-launch --web-download
  • Reinterpreted BaseDistro as the Ubuntu release to install from a clean WSL baseline, not a local source distro to export.
  • Added WSL capability preflight before destructive cleanup so unsupported hosts fail before setup removes existing app-owned state.
  • Reordered setup pipeline so OS/WSL preflight runs before cleanup, with the port check still after cleanup.
  • Hardened cleanup and rollback behavior:
    • removes stale app-owned install directories with retry handling
    • deletes stale regular files at the target install path
    • rejects reparse points/symlinks at the install path with a clear error
    • avoids global wsl --shutdown during partial install cleanup when --unregister succeeds
  • Improved unsupported/old WSL messaging so inbox WSL versions that do not support wsl --version tell users to update WSL.
  • Updated setup UI progress text and docs to describe a clean WSL gateway install rather than cloning/exporting Ubuntu.
  • Added regression tests covering direct install, no export/import behavior, no base distro mutation, install-path validation, WSL version parsing, cleanup behavior, and pipeline order.

Motivation

The previous setup path could be poisoned by a missing or corrupted local Ubuntu-24.04 distro because setup reused it as the export source. This PR makes setup predictable for users, support, and CI by always creating the gateway from a clean WSL install baseline and surfacing hard failures instead of falling back to hidden reuse paths.

Validation

  • ./build.ps1 — passed
  • dotnet test ./tests/OpenClaw.SetupEngine.Tests/OpenClaw.SetupEngine.Tests.csproj --no-restore --tl:off — passed, 181 tests
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore --tl:off — passed, 2022 passed / 29 skipped
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore --tl:off — passed, 843 tests
  • OPENCLAW_RUN_E2E=1 dotnet test ./tests/OpenClaw.E2ETests/OpenClaw.E2ETests.csproj -r win-arm64 --tl:off — passed 3 consecutive runs, each 6 passed / 0 failed / 0 skipped

Review notes

A Hanselman/adversarial review was run before finalizing. Follow-up fixes from that review are included: less disruptive partial cleanup, clearer old-WSL handling, and more robust install-path checks.

Replace the local setup path that exported/imported an existing Ubuntu distro with a clean, app-owned WSL install flow. Tighten preflight, cleanup, install-path validation, rollback, documentation, and regression coverage so setup has explicit success and failure conditions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed May 28, 2026, 12:14 PM ET / 16:14 UTC.

Summary
The PR replaces SetupEngine's WSL export/import local gateway creation with a clean direct named WSL install flow, reorders preflight before cleanup, hardens cleanup/rollback, updates setup UI/docs, and adds regression tests.

Reproducibility: no. high-confidence local reproduction was run in this read-only Linux review. Source inspection shows current main uses export/import while the PR hard-gates WSL 2.4.4 direct install, and the PR body covers only the happy-path Windows E2E runs.

Review metrics: 1 noteworthy metric.

  • Changed setup surface: 14 files, +766/-97. The patch changes setup engine behavior, setup UI, docs, and tests, so upgrade compatibility needs maintainer attention before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Resolve the WSL 2.4.4 compatibility finding with a fallback or explicit maintainer-approved minimum.
  • [P2] Add or link upgrade-matrix proof covering both fresh direct-install hosts and older WSL hosts affected by the behavior change.

Risk before merge

  • [P1] Merging can make local setup fail on hosts with WSL below 2.4.4 even though current main's export/import path can still work there.
  • [P1] The PR body reports happy-path E2E setup validation, but the diff and discussion do not include older-WSL upgrade proof or maintainer approval for dropping that compatibility path.

Maintainer options:

  1. Keep a compatible default path (recommended)
    Preserve the current export/import fallback for WSL versions that cannot direct named install, or gate the stricter clean-install contract behind an explicit approved path with tests for both behaviors.
  2. Approve the WSL minimum bump
    Maintainers can intentionally accept WSL 2.4.4 as the new local setup minimum if release notes, docs, and upgrade proof make the user impact acceptable.
  3. Pause for upgrade-matrix proof
    Hold the PR until the old-WSL and new-WSL setup matrix is verified enough to decide whether the fallback can be removed.

Next step before merge

  • [P2] A maintainer needs to choose between preserving an older-WSL fallback and intentionally accepting WSL 2.4.4 as the new minimum before automation should modify the branch.

Security
Cleared: No concrete security or supply-chain regression was found; the patch changes WSL install/cleanup behavior and adds reparse-point rejection without broadening secret handling.

Review findings

  • [P1] Preserve older WSL setup compatibility — src/OpenClaw.SetupEngine/SetupSteps.cs:402-403
Review details

Best possible solution:

Land the direct clean-install path only after preserving an older-WSL fallback/default or explicitly approving and documenting the new WSL minimum with upgrade proof.

Do we have a high-confidence way to reproduce the issue?

No high-confidence local reproduction was run in this read-only Linux review. Source inspection shows current main uses export/import while the PR hard-gates WSL 2.4.4 direct install, and the PR body covers only the happy-path Windows E2E runs.

Is this the best way to solve the issue?

No, not yet: direct clean install is a maintainable direction for determinism, but the unconditional WSL 2.4.4 gate changes upgrade behavior without a fallback or explicit compatibility decision.

Full review comments:

  • [P1] Preserve older WSL setup compatibility — src/OpenClaw.SetupEngine/SetupSteps.cs:402-403
    This gate makes setup fail for any host below WSL 2.4.4, while current main can still create the app-owned distro through the export/import path. Please preserve a fallback/default path for older supported WSL hosts, or get an explicit maintainer decision plus upgrade proof/docs for the new minimum.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6ca8484bc5b3.

Label changes

Label changes:

  • add P2: This is a normal setup reliability improvement with bounded blast radius, but it has a compatibility blocker before merge.
  • add merge-risk: 🚨 compatibility: The PR can block existing local setup hosts by replacing the older export/import path with a WSL 2.4.4 direct-install requirement.

Label justifications:

  • P2: This is a normal setup reliability improvement with bounded blast radius, but it has a compatibility blocker before merge.
  • merge-risk: 🚨 compatibility: The PR can block existing local setup hosts by replacing the older export/import path with a WSL 2.4.4 direct-install requirement.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body reports after-change build, focused tests, shared/tray tests, and three real E2E setup runs; that is sufficient happy-path proof but does not settle older-WSL compatibility.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports after-change build, focused tests, shared/tray tests, and three real E2E setup runs; that is sufficient happy-path proof but does not settle older-WSL compatibility.
Evidence reviewed

What I checked:

  • Repository policy read: AGENTS.md was read fully; its setup/onboarding guidance and validation expectations were applied as review context, with validation not run because this review is read-only. (AGENTS.md:1, 6ca8484bc5b3)
  • Current main compatibility path: Current main creates the app-owned WSL distro by exporting the configured base distro and importing it as OpenClawGateway, with a fallback install of the base distro before export. (src/OpenClaw.SetupEngine/SetupSteps.cs:416, 6ca8484bc5b3)
  • Current docs describe older minimum: The current setup redesign docs describe PreflightWsl as verifying WSL installed and version >= 2, followed by export base distro to import a new instance. (docs/SETUP_ENGINE_REDESIGN.md:164, 6ca8484bc5b3)
  • PR hard-gates direct install support: The PR head introduces a minimum direct named install version of 2.4.4 and returns a terminal failure when the parsed WSL version is below that threshold. (src/OpenClaw.SetupEngine/SetupSteps.cs:39, 770daa9f4965)
  • PR replaces export/import with direct install: The PR head builds direct install args using --distribution, --name, --location, --no-launch, and --web-download instead of the current export/import flow. (src/OpenClaw.SetupEngine/SetupSteps.cs:568, 770daa9f4965)
  • Regression tests cover new behavior but not compatibility fallback: The PR adds tests for unsupported WSL versions, direct fresh install, no base distro export/mutation, stale path handling, and version/list parsing; those tests assert the new hard-fail behavior rather than preserving older WSL compatibility. (tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs:227, 770daa9f4965)

Likely related people:

  • ranjeshj: Introduced the out-of-process SetupEngine and current WSL setup pipeline on main, and the PR also changes that same setup path. (role: feature owner; confidence: high; commits: cefce3952ab1, d01fdc6f001f, 770daa9f4965; files: src/OpenClaw.SetupEngine/SetupSteps.cs, src/OpenClaw.SetupEngine/SetupPipeline.cs, src/OpenClaw.SetupEngine/default-config.json)
  • Scott Hanselman: Recently touched setup UI flow and SetupEngine test quality, making him relevant for reviewing setup UX and regression-test expectations. (role: recent adjacent contributor; confidence: medium; commits: 0d4fcbd50ad5, 16399edfb05c; files: src/OpenClaw.SetupEngine.UI/Pages/ProgressPage.xaml.cs, tests/OpenClaw.SetupEngine.Tests/SetupPipelineTests.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 27, 2026
@clawsweeper

clawsweeper Bot commented May 27, 2026

Copy link
Copy Markdown

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels May 28, 2026
@ranjeshj

Copy link
Copy Markdown
Collaborator Author

Manual + automated validation completed after pulling in only the drain-refactor fix from #550 to unblock the latest gateway regression.

Automated validation

  • dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj -> passed (183)
  • dotnet test .\tests\OpenClaw.E2ETests\OpenClaw.E2ETests.csproj -r win-arm64 against latest 2026.5.27 -> passed (6/6)
  • Required repo validation also passed:
    • .\build.ps1
    • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore
    • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore

Manual validation

  • Normal tray/setup flow completed successfully on v2026.5.27:

    Normal tray success

  • Older WSL 2.3.26.0 hit the expected preflight-wsl failure path and surfaced the Update WSL -> link to https://aka.ms/wslstorepage:

    Older WSL failure path

Commit pushed: 770daa9

@shanselman

Copy link
Copy Markdown
Collaborator

Landed in master via the integrated Inno installer stack on branch shanselman/inno-installer-plan (master now at 8edf4eb). Closing this source PR as absorbed.

@shanselman shanselman closed this May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants