fix(setup): unregister only a distro this install created

[SebTardif]

What Problem

If the target distro name was already registered, create returned failure before writing an ownership marker. Rollback still ran wsl --unregister for that name. A failed wsl --list still attempted unregister.

Why

Rollback is on by default. A pre-existing distro was removed.

User Impact

Unregister now happens only when the ownership marker matches this install path. An already-registered name is left alone. A failed list does not unregister.

Maintainer clarification: automatic unregister also retains the author's live-registration BasePath and current-attempt checks. Automatic rollback now preserves the install directory, ownership marker, and empty parent when this execution never started creating the distro. Reusing the step does not inherit cleanup authority. Explicit, consented uninstall remains separate.

Evidence

Current head 50f93e89d481f6c1041e832f7ce4cc0ffe8ca642. CleanupStaleDistroStep ran in an isolated data directory. The only distro it was allowed to name was OpenClawProof1479. Ubuntu-24.04 and OpenClawGateway were registered before both runs and after both runs.

Foreign registration: that disposable distro was imported outside the managed install path, while setup-managed-distro.json named the same distro at the managed path. The step returned FailedTerminal: the live registration is bound to a different base path, and the distro was preserved. It did not call wsl --terminate or wsl --unregister. A KEEP.txt file on the foreign directory was still there, and wsl --list --quiet still listed OpenClawProof1479. That refusal returns before either unregister attempt, so the retry cannot reach a registration this install does not own.

Owned registration: the same name was then imported at the managed path with a matching marker. The step terminated OpenClawProof1479, and wsl --unregister OpenClawProof1479 exited 0. The list that followed contained only Ubuntu-24.04 and OpenClawGateway. The first unregister succeeded, so the retry branch did not run.

The Setup and connect E2E job on this head was cancelled earlier and was not rerun.

Original author evidence, before the maintainer follow-up:

Red: two tests failed because --unregister ran for an existing distro and after a failed list. Green: the WSL-create filter passed 16. Setup-engine suite 1193 passed. The host wsl --version UTF-16 test failed and does not touch this change. .\build.ps1 passed.

Maintainer follow-up

Preserved the original contributor PR, branch, and all three author commits through 08e10063f5c3ed9ce1d6fd197f0da9c62a534a2c. Normally merged main 42c562f91a42ae0251ac971c44f6ce0f8028e88d in 7ab2cca5f3c6c767173d407f5ac18983e55c02ab, then added one repair commit, e1abdb5dc2abc33114bb19964a407886f43684de, and nonforce-pushed the original branch.

The repair adds 10 production lines at the existing CreateWslInstanceStep boundary, focused regressions, and 8 documentation lines. No registration-GUID framework, unrelated install-path rewrite, or workflow changes.

Required proof pools

• windows-wsl-gateway-e2e: real WSL creation and automatic rollback behavior. Not verified / blocked on the shared host; no runtime-proof authorization.
• windows-clean-installer-upgrade: signed installer/uninstall integration with the shared teardown path. Not verified / blocked; no disposable signed-artifact installer proof performed.

Validation

Repair head: e1abdb5dc2abc33114bb19964a407886f43684de. Validated source tree: 0e3c32052ee2458bb7021b05c546647165cb1dd7 (unchanged when committed). Native Windows ARM64, task-private .NET SDK 10.0.400.

Process-local isolation set OPENCLAW_REPO_ROOT to the isolated worktree and unique OPENCLAW_TRAY_DATA_DIR / OPENCLAW_TRAY_APPDATA_DIR; unset fixture-conflicting state/local-data overrides and E2E/integration flags. Explicitly built each test project first to prevent fresh-worktree --no-restore no-ops.

Command	Result
.\build.ps1	Passed all five project builds and documentation validation
dotnet build .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --nologo -v minimal	Passed
dotnet build .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --nologo -v minimal	Passed
dotnet build .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --nologo -v minimal	Passed
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore	4,096 passed, 35 skipped, 0 failed
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore	3,069 passed, 0 skipped, 0 failed
dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore	1,207 passed, 1 skipped, 0 failed
dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore --filter 'FullyQualifiedName~CreateWslInstance|FullyQualifiedName~UninstallPipeline_RemovesSafeLegacyDistro'	32 passed, 0 skipped, 0 failed

All test runs also supplied --logger 'trx;LogFileName=<suite>.trx' --results-directory '<session-artifacts>\results'. Full-suite files are Shared.trx, Tray.trx, and SetupEngine.trx; focused proof is green-rollback.trx. Actual TRX counters were checked. The unchanged CommandRunnerTests.RunAsync_WslVersion_DecodesOutputBeforeLogging passed: only its exact read-only wsl.exe --version metadata invocation was authorized. No tests were filtered out of the full SetupEngine suite. Its one skip requires a separately configured volume; Shared skips remain the existing integration/hardware gates.

Focused rubber-duck review: no blockers. Conservative preservation if setup stops between marker writing and install start is intentional; cleanup authority was not moved earlier to remove a harmless leftover marker. Existing defensive unregister checks were retained.

Exact-commit structured autoreview: python .\.agents\skills\autoreview\scripts\autoreview --mode commit --commit e1abdb5dc2abc33114bb19964a407886f43684de --engine codex --codex-bin <verified-private-Codex-0.156.1> with the bounded repair context and saved text/JSON outputs. gpt-5.6-sol, high reasoning; exit 0, no findings, no accepted/actionable issues. This is source-review evidence, not merge confidence or a runtime-gate waiver.

Real behavior proof

Current head 4422ba5357007afbc9ff1c159fa614d45db25ad1, 2026-09-27. ./build.ps1 exit 0. Shared 4099 passed, 32 skipped. Tray 3064 passed. Five source-contract tests failed on this LF checkout because they look for CRLF snippets in files this change does not touch. Focused cleanup tests 9 passed.

Uninstall whose --list --quiet already excludes the distro does not call --terminate or --unregister. When the same-name registration survives and the install directory is already absent, the ownership marker stays and unregister is not called. A failed list during uninstall still unregisters.

Not verified / blocked: a live wsl --unregister on this head. The counts above the next paragraph are the parent head e1abdb5dc2abc33114bb19964a407886f43684de.

The fake command log shows --unregister is absent when the distro already existed. Not verified: no live wsl --unregister.

The maintainer regression executes production ExecuteAsync and both direct RollbackAsync and real SetupPipeline.RunAsync rollback orchestration against actual task-owned temporary directories and marker files. WSL/process calls are faked, not executed.

• Red: five new cases failed on the integrated author code because the refused sentinel or pre-existing empty parent was deleted (red-rollback.trx: 5 failed, 0 passed).
• Green: refused nonempty directories preserve sentinel contents with and without a pre-existing path-bound marker, through direct and pipeline rollback. Existing marker contents remain identical; only the initial fake --list --quiet call occurs. A warning explicitly reports preservation.
• Direct rollback before Execute preserves an existing marker and empty parent without any command calls. A reused step cannot inherit the preceding execution's cleanup authority.
• Positive cases still create partial files through the fake install callback and clean them after an owned unregister, or preserve files/markers when registration inspection is missing, unavailable, malformed, duplicate, bound to another BasePath, or unregister fails. Explicit uninstall remains covered with fakes, including legacy names and a failed list.

Not verified / blocked: live Windows/WSL setup, distro creation or unregister, real VHD behavior, full Gateway/MXC E2E, WinUI interaction, and signed installer/uninstall/upgrade pools. No existing distro, service, probe, or normal installed app was manipulated. Temporary-filesystem and fake-process proof does not satisfy these runtime pools; merge remains a separately coordinated decision.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs real behavior proof before merge.

What this changes

This PR restricts automatic WSL rollback cleanup to the current creation attempt and a matching live registration, while preserving explicit uninstall.

Example: OpenClawGateway already exists when the create step starts and rollback is enabled.

• Before: Creation refuses the existing distro, but rollback can unregister it and delete its data.
• After: Rollback preserves the existing distro, install directory, ownership marker, and parent directory.

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The focused, approved repair has meaningful regression coverage and no open code findings, but supplied runtime evidence exercises a different owner and leaves the destructive authority boundary unproven.
Proof confidence	🧂 unranked krab (1/6)	Real behavior proof is necessary before merge. See Before merge.
Patch quality	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Failed setup can delete a WSL distro or files that the current installation did not create.
Reason: The data-loss path is source-verifiable and the maintainer explicitly approved this narrow repair; conservative preservation before creation starts is intentional.

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep this PR open: it repairs a current data-loss path, follows explicit maintainer direction, and addresses the earlier code findings.

Priority: P0
Reviewed head: 50f93e89d481f6c1041e832f7ce4cc0ffe8ca642

Before merge

• Add real behavior proof - Authority-chain proof required: the new current-head live account demonstrates shared ownership checks through unchanged CleanupStaleDistroStep, including preservation of foreign OpenClawProof1479 and successful owned unregister, but does not exercise CreateWslInstanceStep's changed attempt guard or automatic rollback. Its first unregister succeeded, so retry and reassignment were not exercised. Existing CreateWslInstanceStep/SetupPipeline evidence uses real temporary files with fake WSL calls; the four inspected screenshots belong to historical path-hardening work. Provide redacted current-head output through the changed production owner showing owned rollback and preservation of refused or foreign state, including reassignment before destructive retry; the declared signed-installer upgrade/uninstall pool remains blocked. Redact IPs, credentials, private paths, and non-public endpoints. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve security concern: Registration reassignment during destructive retry remains unproven - Automatic cleanup checks live ownership before entering TryUnregisterPartialInstall, whose terminate and unregister retry sequence consumes the distro name without another registration inspection; the new evidence tests a foreign registration before entry and a successful first unregister, leaving reassignment during retry unexercised.
• Resolve merge risk - Before merge, establish that reassignment of a same-name registration after the initial ownership check cannot make the subsequent unregister retry delete foreign data.

Findings

• [medium] Registration reassignment during destructive retry remains unproven — src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs:243

Tests

• Missing end-to-end proof: Supply current-head production-path Windows/WSL evidence for refused pre-existing state, owned automatic rollback, foreign-registration rejection and reassignment during retry, plus the declared signed-artifact upgrade/uninstall scenario.

Agent review details

How this fits together

SetupPipeline invokes CreateWslInstanceStep during WSL provisioning and failure rollback; the step uses persisted ownership evidence, current-user WSL registration metadata, and command results to authorize unregister and filesystem cleanup.

flowchart TD
 A[SetupPipeline] --> B[CreateWslInstanceStep]
 B --> C[Current attempt guard]
 C --> D[ManagedDistroOwnership]
 D --> E[Live WSL registration]
 E --> F[Targeted unregister]
 F --> G[Managed directory cleanup]

Loading

Technical review

Best possible solution:

Land the approved narrow repair after validating automatic rollback and consented teardown through their actual production paths.

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

Current main's create step refuses an existing distro, then SetupPipeline invokes failed-step rollback, whose cleanup unregisters that name without proving this attempt owns it; no target code was executed during this read-only review.

Is this the best way to solve the issue?

The attempt guard and reuse of ManagedDistroOwnership address the stated cause at the existing owner, preserve explicit uninstall, and match the maintainer's requested scope.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 817b0b586c1e.

Merge-risk options

Maintainer options:

1. Hold for the destructive-boundary evidence (recommended)
   Keep the repair scoped and paused until the changed rollback owner demonstrates rejection of a reassigned foreign registration before destructive retry.

Provenance checked

• Best-effort partial-install unregister and directory cleanup changes intended behavior with a stated reason (unknown: The inherited code attempts best-effort cleanup while preserving files if unregister fails. The step extraction in refactor(setup): split setup steps and own keepalive processes #1093 explicitly preserved behavior; scoped git log -G encountered an unavailable older blob, so exact introduction remains unknown.)
• Live-registration ownership and marker lifecycle keeps the original intent (fd8901d: The earlier repair bound automatic destructive cleanup to durable evidence and canonical current-user WSL BasePath, while preserving explicit consent.)
• Managed path containment and unregister-failure preservation keeps the original intent (d0fca93: The path-hardening repair confined deletion to the app-owned WSL root and preserved VHD data unless registration absence or successful unregister was established.)

Testing

Proof path: in-process harness.

Security

Needs attention: No introduced exploit was established, but the destructive retry boundary still needs final-effect authority evidence.

Evidence

Security concerns:

• [medium] Registration reassignment during destructive retry remains unproven — src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs:243
  Automatic cleanup checks live ownership before entering TryUnregisterPartialInstall, whose terminate and unregister retry sequence consumes the distro name without another registration inspection; the new evidence tests a foreign registration before entry and a successful first unregister, leaving reassignment during retry unexercised.
  Confidence: 0.94

What I checked:

• Main still contains the unsafe rollback path: The pinned main blob unregisters an existing name or attempts unregister after a failed list without current-attempt or live-registration ownership checks. (src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs:202, 817b0b586c1e)
• Verified introduced change: The introduced diff resets attempt authority, binds automatic unregister to the existing live-registration helper, preserves refused state, skips known-absent uninstall unregister, and retains markers after refused cleanup. (src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs:330, 50f93e89d481)
• Maintainer-approved repair boundary: shanselman explicitly approved landing the contributor's narrow repair and requested current-attempt authority, the existing live-BasePath helper, and preservation of refused directories: fix(setup): unregister only a distro this install created #1479 (comment) and fix(setup): unregister only a distro this install created #1479 (comment).
• Current live evidence exercises an unchanged owner: The full current body describes foreign-registration preservation and successful owned unregister through CleanupStaleDistroStep at the reviewed head; it explicitly says the successful first unregister did not exercise retry. (src/OpenClaw.SetupEngine/CleanupStaleDistroStep.cs:34, 50f93e89d481)
• Supporting screenshots inspected: All four prepared images show historical path-hardening setup, uninstall, or unsafe-name rejection at another revision. They belong to improve(setup): keep WSL operations inside the app-owned directory #1006 (improve(setup): keep WSL operations inside the app-owned directory), rather than this changed rollback owner.
• Existing ownership contract: The architecture ledger requires durable evidence plus a matching readable current-user registration before automatic unregister; the earlier ownership repair is fix(setup): bind cleanup to live WSL registration #1230 (fix(setup): bind cleanup to live WSL registration). (docs/ARCHITECTURE.md:782, 817b0b586c1e)

Likely related people:

• Scott Hanselman: Raw commit fd8901d adds src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs:223 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: fd8901d85f44; files: src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs)
• Barbara Kudiess: Raw commit b37307c adds src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs:166 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: b37307cbd51f; files: src/OpenClaw.SetupEngine/CreateWslInstanceStep.cs)

Review metrics

Metric	Value	Why it matters
Patch distribution	Production +54/-5; tests +346/-25; docs +8/-0	Most growth covers destructive-boundary regressions; production changes stay within the existing create-step owner.

Labels

Label changes:

No label changes.

Label justifications:

• P0: The current main rollback path can permanently remove a pre-existing WSL distribution and its data.
• merge-risk: 🚨 security-boundary: The patch changes authority for destructive WSL and filesystem operations, with same-name reassignment across retry still unproven.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add final-effect proof for the nearest unauthorized same-name registration and show reassignment is rejected before destructive WSL or filesystem I/O.
• Show owned automatic rollback through CreateWslInstanceStep and complete the declared signed-artifact upgrade/uninstall proof.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (10 earlier review cycles; latest 8 shown)

• reviewed 2026-09-23T19:19:25.734Z sha 08e1006 :: needs real behavior proof before merge. :: [P2] Skip unregister when the WSL list proves the distro absent | [P2] Keep the marker when a registration survives rollback
• reviewed 2026-09-23T21:50:49.978Z sha 08e1006 :: needs real behavior proof before merge. :: [P2] Skip unregister when the WSL list proves the distro absent | [P2] Keep the marker when a registration survives rollback
• reviewed 2026-09-23T22:30:24.931Z sha 08e1006 :: needs real behavior proof before merge. :: [P2] Skip unregister when uninstall already proved the distro absent | [P2] Retain the ownership marker while a registration survives
• reviewed 2026-09-23T23:49:48.119Z sha 08e1006 :: needs real behavior proof before merge. :: [P2] Skip unregister when uninstall already proved the distro absent | [P2] Retain the ownership marker while a registration survives
• reviewed 2026-09-24T00:12:27.833Z sha e1abdb5 :: needs real behavior proof before merge. :: [P2] Skip unregister when uninstall already proved the distro absent | [P2] Keep the marker when a registration survives cleanup
• reviewed 2026-09-24T01:15:46.064Z sha e1abdb5 :: needs real behavior proof before merge. :: [P2] Skip unregister when uninstall already proved the distro absent | [P2] Keep the marker while a registration survives cleanup
• reviewed 2026-09-27T19:55:40.233Z sha 4422ba5 :: needs real behavior proof before merge. :: none
• reviewed 2026-10-13T22:36:20.649Z sha 50f93e8 :: needs real behavior proof before merge. :: none

Reviewed October 13, 2026, 8:51 PM ET / October 14, 2026, 00:51 UTC (Revision 11).

[shanselman]

Thanks, Seb. This is a useful narrowing of a real data-loss path, and I want to land it on your PR rather than replace it.

Reviewed head: c7d80b4f5a73594d7237f6993077730a9b40bf61.

One ownership gap remains in CreateWslInstanceStep.CleanupPartialInstall: HasPathBoundMarkerEvidence proves what our persisted marker says, not which distro the name identifies in the live WSL registration. A stale marker plus a same-named replacement registered at another BasePath still allows automatic --unregister. This is an incomplete mitigation of the existing behavior, not a new deletion regression introduced here.

Please keep the repair small:

• Before automatic unregister, reuse the live-registration/canonical-BasePath check in ManagedDistroOwnership.HasRegisteredDistroEvidence. Keep explicit destructive uninstall consent separate from automatic rollback.
• Ensure failure before this run creates a distro cannot claim a pre-existing registration just because an old marker survives. A run-scoped creation/attempt guard can help, but cannot replace the live registration check against a name-replacement race.
• Add regressions for marker-present/different live BasePath, failed registration inspection, and a pre-existing registration encountered before creation. Those cases should preserve both the registration and its files. Retain a marker-present owned-unregister-failure test so VHD preservation is still exercised.

The existing architecture ledger already states the live-registration invariant, so this should use the current owner rather than add a parallel policy. After that, the remaining gate is isolated real Windows/WSL rollback proof, not a larger redesign.

[karkarl]

Global triage: HOLD_FOR_AUTHOR. Take confidence 40%; recommendation confidence 90%; effort small-to-moderate; risk high.

Reviewed exact head c7d80b4f5a73. The narrowing is right, but the destructive boundary still uses marker-only HasPathBoundMarkerEvidence instead of binding ownership to the live WSL registration. Reuse HasRegisteredDistroEvidence. Also fix uninstall when wsl --list fails: IsUninstalling does not cover the unknown-registration branch, turning recoverable cleanup into failed uninstall with retained VHDX. Add mismatch/failure regressions, then windows-wsl-gateway-e2e plus windows-clean-installer-upgrade proof.

[shanselman]

Thanks for the two follow-ups. At 08e10063f5c3ed9ce1d6fd197f0da9c62a534a2c, the automatic unregister path now reuses the live-BasePath ownership helper and requires this create attempt. That addresses the earlier registration concern.

One adjacent deletion path still bypasses the new attempt guard:

1. The distro is absent, but its proposed install directory already contains files.
2. EnsureInstallPathReady refuses that directory before the marker/attempt flag is established.
3. Pipeline rollback calls CleanupPartialInstall.
4. canDeleteInstallPath = registrationStateKnown && !distroExists becomes true, so the directory that was just refused is recursively deleted.

This is pre-existing behavior left open by the narrowing, not a new regression. Please extend the no-authority-before-this-attempt rule to directory/marker cleanup as well as unregister. A top-level automatic-cleanup guard before the attempt owns creation is a small option; explicit uninstall consent remains separate. Add the regression “nonempty orphan directory refused by Execute, then rollback preserves it.”

I am not asking for a new registration-GUID ownership architecture or a broad redesign. The immediate requirement is that refusing unknown pre-existing state must not be followed by deleting that state during automatic rollback.

[karkarl]

Global triage: HOLD_FOR_AUTHOR. Take confidence 10%; recommendation confidence 99%; effort small; risk high.

Reviewed exact head 4422ba535700. GitHub currently reports BLOCKED with 23 passing, 2 failing, and 0 pending checks. needs-proof label is present. Durable automated signal: Codex review: needs real behavior proof before merge..

Author: update from current main, resolve conflicts or branch-policy blockers, then rerun exact-head validation and request review.

This is a triage classification, not a merge action. No branch changes were made.

[karkarl]

Global triage: HOLD_FOR_AUTHOR. Take confidence 20%; recommendation confidence 96%; effort medium; risk high.

Reviewed exact head 50f93e89d481. The merge adds no new source change, but destructive scope remains unresolved: uninstall can still unregister a same-name distro without ownership evidence, and retry does not re-check ownership before another unregister attempt. Setup/connect E2E was cancelled and supplies no passing proof.

Author: ownership-gate uninstall and every retry (or explicitly narrow the product contract), then provide current-head owned rollback, foreign/pre-existing preservation, reassignment-before-retry, and signed installer/uninstall proof. No fixes or branch changes were made by this review.