Repository navigation
Conversation
…ot finish A timed-out or failed wsl --list --quiet after a successful install was treated as an unregistered distro. Cleanup then unregistered a distro that terminate and unregister both accepted. Retry that list on timeout or a non-zero exit, and leave the distro registered when every attempt fails. A completed list that omits the name still reports that the distro did not register. Install-failure cleanup and rollback still attempt unregister. Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed October 6, 2026, 3:09 AM ET / 07:09 UTC (Revision 4). ClawSweeper reviewWhat this changesThe PR retries failed checks for a newly installed WSL Linux distribution and preserves its registration and files through setup rollback when those checks remain inconclusive. Merge readiness✅ Ready for maintainer review This PR remains necessary: main and the latest release retain the destructive timeout behavior. The updated real Windows trace resolves the previous proof blockers, and no introduced blocking defect was found. Likely related people: shanselman and Barbara Kudiess are routing candidates based on merged setup work; exact introduction attribution remains unverified. Priority: P1 Review scores
Verification
How this fits togetherWindows setup creates a WSL Linux distribution to host the OpenClaw Gateway. Installation results and registration checks determine whether setup continues or cleans up the new distribution. flowchart TD
A[Windows setup] --> B[Install Linux distribution]
B --> C[Check registration with retries]
C --> D{Check completed}
D -->|Yes| E[Verify distribution and continue]
D -->|No| F[Preserve distribution through rollback]
F --> G[Ownership checked cleanup on next setup]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep registration uncertainty separate from confirmed installation failure, preserving the new distribution until ownership-checked recovery can safely clean it up. Do we have a high-confidence way to reproduce the issue? Yes, source provides a clear path: successful WSL installation followed by a failed quiet list enters destructive cleanup on main. This review did not execute a failing main build; the contributor supplied current-head real fault-injection evidence. Is this the best way to solve the issue? Yes, bounded retries and a preservation decision shared with rollback are a focused repair. Existing ownership-checked next-run cleanup handles the retained distribution without a new recovery mechanism. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 4895ed50e39e. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles)
|
When every post-install list fails, immediate cleanup leaves the distro registered. End-user setup then rolls that step back, and the second cleanup unregistered it. Rollback now reuses the same preservation decision. A completed list that omits the name still unregisters on rollback. SetupPipeline_PreservesFreshDistroWhenPostInstallListFailsAndRollbackRuns and SetupPipeline_UnregistersDistroOnRollbackWhenPostInstallListCompleted cover both paths. Validation: ./build.ps1 exit 0. Shared 4169 passed, 33 skipped. Tray 3184 passed. CreateWslInstance 18 passed. Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
|
Global triage: NEEDS_HUMAN_TEST. Take confidence 55%; recommendation confidence 97%; effort small; risk high. Reviewed exact head Maintainer or proof-pool owner: run every declared Windows proof pool at this exact head and replace blocked or historical evidence with current-head results. This is a triage classification, not a merge action. No branch changes were made. |
|
Thanks for working on this. Reviewed head: This is partially superseded by #1719, but the preservation behavior is still useful. Current Could you update this PR against current After resolving the conflict, please rerun the focused creation/rollback tests and refresh the timeout-preservation and ownership-checked recovery evidence at the new head. The existing-head results don't cover the integration changes. |
Related: #1479
What Problem This Solves
Fixes: fresh WSL setup reports that the new distro did not register, then unregisters it, when
wsl --list --quiettimes out or fails afterwsl --installalready exited 0.User Impact
User impact: a slow or failed distro list after a successful install no longer removes a distro whose registration could not be checked. That attempt still fails. The default config sets CleanBeforeRun to true, so the next setup runs stale-distro cleanup before create. A leftover that is not proven app-owned stops in that cleanup and stays registered. Headless recovery is
--confirm-destructive. The create step's "still exists after cleanup" refusal runs only when that cleanup is skipped.Why This Change Was Made
The post-install list is retried only when it times out or exits non-zero (15 seconds, then 30, then 60). A completed list that omits the name still fails immediately as "did not register". When every list attempt fails, cleanup leaves the distro and the app-owned install path in place, even if a later list then shows the name. Failed-step rollback reuses that same decision, so the second cleanup does not unregister the distro. Install-failure cleanup still attempts unregister. A completed list that omitted the name still unregisters during rollback.
#1479 also edits
CreateWslInstanceStep.csand the setup step tests. That pull limits unregister to a distro the current install created. This pull does not change that install-failure path. The two will conflict if both land.Evidence
A completed list that omits the name still stops with "did not register" and does not ask again.
A list that times out, then a later list that contains the name, continues to the WSL2 check and the root probe. Unregister is not called.
When the list never completes, the step says it could not list distros after installing, leaves the distro registered, and skips deleting the install path. The install command still includes
--no-launch.With rollback enabled, that same exhausted list still leaves the distro and the install path in place after
SetupPipelineruns failed-step rollback. The rollback list can print the distro name. Unregister and terminate are not called.A completed list that omits the name, followed by a rollback list that prints the name, still calls
--terminateand--unregister.Live install on this machine, same arguments as
WslInstallSupport.BuildDirectInstallArgs, nameOpenClawE2E-ocwn1548, location under the temp directory,--no-launch --web-download. Exit 0. The installer printedDownloading: Ubuntu 24.04 LTS,Installing: Ubuntu 24.04 LTS, andDistribution successfully installed.Post-install
wsl --list --quiet:The next default cleanup is
CleanupStaleDistroStepwithCleanBeforeRuntrue, soCanSkipis false. This run used an isolated data directory, no ownership marker, port 19041, headless, andConfirmDestructivefalse. It did not targetUbuntu-24.04orOpenClawGateway.wsl --list --quietafter that cleanup was unchanged. The list that followed the live install completed in 58 ms, so that run did not force the timeout-preserve path.A later run on the same head used
SetupPipelinewithCreateWslInstanceStepand rollback enabled. The disposable name wasOpenClawE2E-ocwn1548b. The production install arguments exited 0 in about 18 seconds, and the ownership marker was written before install.The three post-install checks were real
C:\Windows\System32\wsl.exe --list --quietprocesses. Each was suspended by the same user so it could not finish, andCommandRunnerhit its own timeout and killed it. The waits were 15 seconds, 30 seconds, and 60 seconds, each with exit -1. The step logged that the registration list was not ready and retried. This was not a substitute command runner. The rollback list was not suspended.That rollback error is the preserve path. Cleanup reports that the distro was left registered, and
RollbackAsynctreats that text as a failed cleanup instead of deleting the distro or its files.Ubuntu-24.04andOpenClawGatewaystayed registered.The next call was
CleanupStaleDistroStepin the same data directory.CleanBeforeRunwas true, soCanSkipwas false. It matched the ownership marker and the registration path, terminated and unregistered onlyOpenClawE2E-ocwn1548b, and deleted the emptywslparent.A following
wsl --list --quietshowed onlyUbuntu-24.04andOpenClawGateway.Change Type
Scope
winnodeRequired proof pools
windows-wsl-gateway-e2e: product WSL setup registration after install. A livewsl --installandwsl --list --quietwere collected. The full gateway setup in this pool was not run.Validation
./build.ps1: exit 0 (Shared, Cli, WinNodeCli, SetupEngine, WinUI)dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj: Failed 0, Passed 4169, Skipped 33, Total 4202dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj: Failed 0, Passed 3184, Skipped 0, Total 3184dotnet test ./tests/OpenClaw.SetupEngine.Tests/OpenClaw.SetupEngine.Tests.csproj --filter FullyQualifiedName~CreateWslInstance: Failed 0, Passed 18, Skipped 0, Total 18SetupPipeline_PreservesFreshDistroWhenPostInstallListFailsAndRollbackRunsandSetupPipeline_UnregistersDistroOnRollbackWhenPostInstallListCompleted: Failed 0, Passed 2Before the product change, the timeout scenario failed with "did not register" and then called
--unregister. The exhausted-list scenario did not contain "could not list distros after installing".Real behavior proof
wsl --installexits 0, a post-installwsl --list --quietthat times out or exits non-zero was reported as "did not register", and cleanup unregistered the distro.fix/wsl-create-list-retryatf7f2ce9a3d8419feb1e59f1dcaa538cfe13f0112. Livewsl --installofOpenClawE2E-ocwn1548exited 0.CreateWslInstanceStepwith a command stand-in. The first post-install--list --quiettimes out. The next list printsOpenClawGateway. The WSL2 verbose list and the root probe both exit 0.When every post-install list fails, and a later cleanup list prints the name:
The same exhausted-list stand-in, run through
SetupPipelinewith rollback enabled, then a cleanup list that printsOpenClawGateway:wsl.exeprocess untilCommandRunnertimed out. WSL itself answered a later list in 45 ms. A fullwindows-wsl-gateway-e2egateway setup was not run.f7f2ce9a3d8419feb1e59f1dcaa538cfe13f0112N/Awindows-wsl-gateway-e2egateway setup. The timeout was a suspended realwsl.exe --list --quiet, not a WSL service failure.Security Impact
No)No)No)No)No)Yes, explain the risk and mitigation: n/aCompatibility and Migration
Yes)No)No)Review Conversations