fix(setup): delete only generated uninstall children

[karkarl]

Supersedes #1486 without rewriting @SebTardif's contributor fork. This maintainer-owned replacement rebases the smallest useful change onto current main (5a595352), including #1515 and #1518.

Summary

• Delete only known generated uninstall children instead of recursively deleting the generated-data root.
• Delete only the exact confirmed distro child after successful primary cleanup; preserve uncertain siblings and legacy state.
• Preserve current-main Store migration lock/admission semantics and fail closed on path-chain reparse points.
• Add executable coverage for custom roots, lookalikes, junctions, missing children, and migration preservation contracts.

Required proof pools

• windows-clean-installer-upgrade: Not verified / blocked. This session had no approved signed installer/upgrade pool host. Local current-head headless uninstall proof below does not substitute for the signed installer pool.

Validation

• OPENCLAW_REPO_ROOT=<worktree>; .\build.ps1 - passed.
• dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore - 4,107 passed, 32 skipped.
• dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore - 3,136 passed.
• dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --filter "FullyQualifiedName~Uninstall" - 30 passed.
• dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~InnoMigrationContractTests|FullyQualifiedName~InstallerIssAssertionTests" - 83 passed.
• Rubber-duck review: no high-confidence in-scope blocker.
• Structured autoreview command: python .agents\skills\autoreview\scripts\autoreview --mode branch --base origin/main --engine codex --model gpt-5.6-sol --thinking high --stream-engine-output - blocked before findings by repeated Codex API 401 Unauthorized responses.

Real behavior proof

Exact head: 5e5bd4bc0a3e2f2fe97c9ac958084a5353da733e.

• Built the uninstall host with -DevBuild and verified app-identity.txt was dev.
• The tray wrapper was dry-run only and exposed a limitation: it replaces explicit isolation arguments with shared dev defaults. Its two task-created log files were removed exactly; destructive wrapper execution was refused.
• Exercised the real public OpenClaw.SetupEngine.Program.Main through a disposable task-owned host with explicit GUID-scoped roots and identifiers.
• Dry-run exit 0; all 34 rollback steps previewed.
• Confirmed destructive exit 0 against OpenClawGateway-PR1486-263455e2dc4c.
• Verified task-owned run.marker and exec-approvals.json removed; custom WSL distro, scheduled task, and autostart value absent after completion.
• Proof artifacts: session-local pr1486-uninstall-263455e2dc4c logs (not uploaded because they contain host-local paths).

Review status

HOLD until structured autoreview can authenticate and windows-clean-installer-upgrade proof is supplied. No merge requested.

[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. Reviewed September 25, 2026, 7:38 PM ET / 23:38 UTC.

ClawSweeper review

What this changes

Narrows Windows uninstall cleanup to named generated files and one WSL distro child, with path guards, documentation, and filesystem tests.

Merge readiness

⛔ Blocked before merge - 12 items remain

Keep this PR open. Current main still deletes the selected install directory after successful gateway cleanup, so the repair remains useful. The replacement has unresolved deletion-safety defects and lacks signed-installer proof.

Priority: P0
Reviewed head: 5e5bd4bc0a3e2f2fe97c9ac958084a5353da733e

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The focused tests and primary-path guards are useful, but destructive admission and data-preservation gaps plus missing final-path proof prevent merge readiness.
Proof confidence	🦪 silver shellfish (2/6)	Needs stronger real behavior proof before merge: Authority-chain proof required: the PR reports current-head headless SetupEngine uninstall and production first-phase filesystem tests, but neither shows the introduced Inno final deletion rejecting Store-owned state before I/O. The signed-installer upgrade/uninstall pool is explicitly blocked. Provide redacted final-effect output for an allowed cleanup and a Store-owned rejection, then update the PR body for re-review. No stored-data format changes are introduced.
Patch quality	🦪 silver shellfish (2/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs stronger real behavior proof before merge: Authority-chain proof required: the PR reports current-head headless SetupEngine uninstall and production first-phase filesystem tests, but neither shows the introduced Inno final deletion rejecting Store-owned state before I/O. The signed-installer upgrade/uninstall pool is explicitly blocked. Provide redacted final-effect output for an allowed cleanup and a Store-owned rejection, then update the PR body for re-review. No stored-data format changes are introduced.
Evidence reviewed	9 items	Current main remains affected: After successful gateway cleanup, current main recursively deletes the entire selected install directory. Introduced deletion paths: The branch replaces whole-directory deletion with recursive deletion of named directories and adds a separate PowerShell invocation for the distro child. Child-only mode bypasses admission: The new switch enters destructive cleanup and exits before the existing migration lock and checker. Its deletion function removes the matching child at line 807.
Findings	3 actionable findings	[P1] Gate the child-only deletion on Store ownership [P1] Preserve unrelated files inside named directories [P2] Persist warnings from the secondary cleanup
Security	Needs attention	Store ownership check bypass: A direct child-only invocation branches before the migration lock and checker and can remove the matching distro directory even when a Store installation should own and preserve it.

How this fits together

The Inno uninstaller takes a user-selected install path and a choice to remove the local WSL Gateway. It calls a PowerShell helper to check Store migration ownership and unregister the distro before removing local files.

flowchart TD
  A[Uninstall choice] --> C[Inno uninstaller]
  B[Selected install path] --> C
  C --> D[Store ownership check]
  D --> E[WSL gateway cleanup]
  E --> F[Generated data cleanup]
  F --> G[Removed or preserved files]
  F --> H[Uninstall diagnostics]

Loading

Before merge

• Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: the PR reports current-head headless SetupEngine uninstall and production first-phase filesystem tests, but neither shows the introduced Inno final deletion rejecting Store-owned state before I/O. The signed-installer upgrade/uninstall pool is explicitly blocked. Provide redacted final-effect output for an allowed cleanup and a Store-owned rejection, then update the PR body for re-review. No stored-data format changes are introduced.
• Gate the child-only deletion on Store ownership (P1) - The new switch exits before the migration lock and checker, then can recursively remove the configured WSL child. The added direct-invocation test confirms that deletion is reachable without an ownership verdict. A Store-owned gateway would lose its local data through this entrypoint despite the normal preservation check. Keep this effect inside admitted cleanup or perform equivalent admission before deletion.
• Preserve unrelated files inside named directories (P1) - Matching the generated root does not prove that every file inside Logs, canvas, or WebView2 belongs to OpenClaw. The new recursive DelTree still removes ordinary pre-existing files under those names, an ownership gap raised in the earlier maintainer discussion. Delete confirmed artifacts only, or leave uncertain content with a warning.
• Persist warnings from the secondary cleanup (P2) - Passing {tmp} as the helper's AppRoot sends its preserved-sibling and uncertain-path warnings to a temporary log. Those diagnostics can disappear after uninstall, leaving no durable explanation for retained WSL files. Record them in the Inno log or a retained result.
• Resolve security concern: Store ownership check bypass - A direct child-only invocation branches before the migration lock and checker and can remove the matching distro directory even when a Store installation should own and preserve it.
• Resolve merge risk (P1) - A direct child-only helper call can delete the configured WSL data directory without the Store migration ownership check that protects normal uninstall.
• Resolve merge risk (P1) - An existing installation can contain unrelated files inside generic directories such as Logs or canvas; the new recursive named-child cleanup would remove them.
• Resolve merge risk (P1) - The declared signed-installer upgrade/uninstall pool remains unverified, including the final Inno deletion and Store-owned preservation cases.
• Complete next step (P2) - Guard child-only deletion with Store ownership admission, preserve unrelated named-child files and durable warnings, then supply signed-installer fresh/upgrade proof including a Store-owned rejection.
• Improve patch quality - Add final-effect proof for the allowed uninstall and nearest forbidden Store-owned case, showing rejection before filesystem deletion.
• Improve patch quality - Preserve unrelated files in named children and keep secondary warnings in a durable log.
• Improve patch quality - Run the signed installer pool against fresh and previous-version uninstall after the guards are repaired.

Findings

• [P1] Gate the child-only deletion on Store ownership — scripts/Uninstall-LocalGateway.ps1:868-873
• [P1] Preserve unrelated files inside named directories — installer.iss:596-600
• [P2] Persist warnings from the secondary cleanup — installer.iss:624-626
• [high] Store ownership check bypass — scripts/Uninstall-LocalGateway.ps1:868

Agent review details

Security

Needs attention: The new child-only PowerShell entrypoint reaches destructive WSL data removal without the Store migration ownership admission used by normal cleanup.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +227/-11 lines; tests +402/-10 lines; docs +9	The larger safety implementation has substantial regression coverage, but its final deletion boundary still needs repair and installer proof.

Merge-risk options

Maintainer options:

1. Complete the deletion guards (recommended)
   Gate the child-only final effect on Store ownership, preserve unrelated named-child content, retain warnings, and prove allowed and forbidden cases through the signed installer.
2. Hold for installer proof
   Keep the PR in draft until the required Windows pool can exercise fresh and upgrade uninstall without losing unrelated or Store-owned data.

Technical review

Best possible solution:

Keep distro-child deletion inside migration-protected cleanup, preserve files whose ownership is uncertain, retain warnings in a durable uninstall log, and verify fresh and upgrade uninstall with signed Windows artifacts.

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

Yes. Current-main source gives a clear path: complete gateway cleanup with a custom install folder containing an unrelated sentinel, then observe the whole-folder deletion. This read-only review did not execute it.

Is this the best way to solve the issue?

No. Narrowing deletion is appropriate, but the child-only mode needs ownership admission and generic named directories need content-level preservation.

Full review comments:

• [P1] Gate the child-only deletion on Store ownership — scripts/Uninstall-LocalGateway.ps1:868-873
  The new switch exits before the migration lock and checker, then can recursively remove the configured WSL child. The added direct-invocation test confirms that deletion is reachable without an ownership verdict. A Store-owned gateway would lose its local data through this entrypoint despite the normal preservation check. Keep this effect inside admitted cleanup or perform equivalent admission before deletion.
  Confidence: 0.96
• [P1] Preserve unrelated files inside named directories — installer.iss:596-600
  Matching the generated root does not prove that every file inside Logs, canvas, or WebView2 belongs to OpenClaw. The new recursive DelTree still removes ordinary pre-existing files under those names, an ownership gap raised in the earlier maintainer discussion. Delete confirmed artifacts only, or leave uncertain content with a warning.
  Confidence: 0.94
• [P2] Persist warnings from the secondary cleanup — installer.iss:624-626
  Passing {tmp} as the helper's AppRoot sends its preserved-sibling and uncertain-path warnings to a temporary log. Those diagnostics can disappear after uninstall, leaving no durable explanation for retained WSL files. Record them in the Inno log or a retained result.
  Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5a59535216ee.

Labels

Label changes:

No label changes.

Label justifications:

• P0: The affected uninstall path can irreversibly remove local WSL or unrelated user data.
• merge-risk: 🚨 compatibility: The introduced cleanup can remove pre-existing files inside generic named directories during upgrade or uninstall.
• merge-risk: 🚨 security-boundary: The new child-only entrypoint can reach destructive WSL filesystem I/O before Store ownership admission.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Authority-chain proof required: the PR reports current-head headless SetupEngine uninstall and production first-phase filesystem tests, but neither shows the introduced Inno final deletion rejecting Store-owned state before I/O. The signed-installer upgrade/uninstall pool is explicitly blocked. Provide redacted final-effect output for an allowed cleanup and a Store-owned rejection, then update the PR body for re-review. No stored-data format changes are introduced.

Evidence

Security concerns:

• [high] Store ownership check bypass — scripts/Uninstall-LocalGateway.ps1:868
  A direct child-only invocation branches before the migration lock and checker and can remove the matching distro directory even when a Store installation should own and preserve it.
  Confidence: 0.96

What I checked:

• Current main remains affected: After successful gateway cleanup, current main recursively deletes the entire selected install directory. (installer.iss:594, 5a59535216ee)
• Introduced deletion paths: The branch replaces whole-directory deletion with recursive deletion of named directories and adds a separate PowerShell invocation for the distro child. (installer.iss:599, 5e5bd4bc0a3e)
• Child-only mode bypasses admission: The new switch enters destructive cleanup and exits before the existing migration lock and checker. Its deletion function removes the matching child at line 807. (scripts/Uninstall-LocalGateway.ps1:868, 5e5bd4bc0a3e)
• Migration protection on the existing path: The normal cleanup acquires a migration lock and preserves gateway state when the checker reports completed migration or Store registration. (scripts/Uninstall-LocalGateway.ps1:876, 5e5bd4bc0a3e)
• Direct invocation demonstrates the boundary: The added test invokes the child-only switch directly and expects the configured VHD directory to be removed; it supplies no Store ownership state. (tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs:187, 5e5bd4bc0a3e)
• Prior maintainer direction: In fix(setup): delete only generated uninstall children #1486 (comment), shanselman requested preservation of unowned files inside generic named children. A later comment also requested durable leftover diagnostics.

Likely related people:

• Natalie Aguinaldo: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• shanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Ranjesh: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

[karkarl]

CI failure audit for exact head 5e5bd4bc0a3e2f2fe97c9ac958084a5353da733e:

• Core and CLI tests failed only MigrationRecordTests.CleanupScript_CompletedReceiptPreservesFilesWithoutCallingWsl(parentShare: None, expectedExit: 2, clockRollback: False): the held prepare.lock case logged the expected sharing violation but the PowerShell process exceeded the 30-second test bound. This is a baseline timing/process-lifetime failure. The exact same test and signature failed on exact base/main SHA 5a59535216eea603192f2df13d4eddf1e1a20267 in main push run 36174730820. The lock acquisition and catch/exit path are unchanged by this PR. A fresh local focused rerun after restore passed all 5 theory cases, including the exclusive-lock case.
• Setup and connect E2E and Revocation recovery E2E both failed during shared fixture setup, before their test bodies and before this PR's uninstall behavior. openclaw gateway restart refused restart-intent recording with StateDatabaseCoordinatorContentionError: another OpenClaw process owns state-lifecycle; the 23 setup/connect failures are fixture fanout. This exact hosted failure is already owned by fix(setup): retry guarded restart intent contention #1507 (fix(setup): retry guarded restart intent contention), based on the same current-main base SHA.
• Network recovery E2E passed. CI Gate is only the aggregate failure.

Classification: baseline/runner failures, not branch-caused. No uninstall-scope code change was made. #1521 remains draft with status: 📣 needs proof; no active-landing label and no merge. HOLD remains for clean CI after the baseline fixes/retry, authenticated structured autoreview (current Codex run is blocked by HTTP 401), and signed windows-clean-installer-upgrade proof.