Skip to content

Add uninstall choice for local WSL gateway - #591

Merged
ranjeshj merged 2 commits into
openclaw:masterfrom
ranjeshj:fix/uninstall-gateway-choice
May 29, 2026
Merged

Add uninstall choice for local WSL gateway#591
ranjeshj merged 2 commits into
openclaw:masterfrom
ranjeshj:fix/uninstall-gateway-choice

Conversation

@ranjeshj

Copy link
Copy Markdown
Collaborator

Summary

This changes the Windows installer uninstall flow so users explicitly choose whether the local WSL gateway should be removed.

  • Adds an interactive uninstall prompt for the local OpenClawGateway WSL distro.
  • Keeps uninstall silent-safe: silent uninstall removes the local gateway automatically.
  • Avoids launching OpenClaw tray/setup binaries from {app} during uninstall, preventing install-directory file locks.
  • Uses direct WSL cleanup (--terminate, --shutdown, --unregister) and waits synchronously before uninstall continues.
  • Preserves generated gateway state when the user selects No so the registered WSL distro is not partially deleted.
  • Cleans setup-managed Windows-side artifacts after gateway removal: gateway records, identity directories, root device tokens when no external gateways remain, setup/run/keepalive markers, autostart, logs, and onboarding settings.
  • Adds installer/script structural assertions to guard the uninstall contract.

Screenshots

Standard uninstall confirmation

Standard uninstall confirmation

New local gateway choice prompt

Local gateway choice prompt

Gateway removal progress

Removing local WSL gateway

Completion dialog

Uninstall complete

Verification

Automated validation:

  • ./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
  • PowerShell parser validation for scripts/Uninstall-LocalGateway.ps1
  • Targeted InstallerIssAssertionTests
  • ARM64 Inno compile:
    • ./scripts/build-inno-local.ps1 -Arch arm64 -Fast -Version 0.0.0-local -NoPublish

Real installer smoke validation with disposable temp install roots and a disposable imported OpenClawGateway WSL distro:

  • Silent uninstall: removed the WSL distro, install/generated state, setup-managed gateway record, identity directory, root tokens, settings, logs, and markers.
  • Interactive No: showed the gateway prompt, skipped gateway cleanup, preserved the registered disposable WSL distro and generated gateway state, and removed installed app files.
  • Interactive Yes: showed the gateway prompt, removed the WSL distro, removed install/generated state, and cleaned gateway records/tokens/settings/logs.
  • Dialog capture smoke: repeated the interactive Yes flow and saved the cropped screenshots above.

Cleanup checks after smoke validation:

  • No disposable OpenClawGateway distro remained.
  • No test installer registration remained.
  • The pre-existing openclaw:// protocol registration was restored after each smoke run.

Prompt users during uninstall before removing the local WSL gateway and route the uninstall through a direct WSL cleanup helper so OpenClaw binaries are not launched from the install directory.

The helper unregisters OpenClawGateway, cleans setup-managed gateway artifacts, and preserves generated gateway state when the user declines removal.

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

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed May 29, 2026, 5:55 PM ET / 21:55 UTC.

Summary
The PR replaces the Inno [UninstallRun] gateway cleanup hook with [Code]-driven interactive/silent cleanup, rewrites Uninstall-LocalGateway.ps1 to unregister the local WSL gateway directly, adds structural assertions, and adds uninstall screenshots.

Reproducibility: not applicable. as a bug reproduction; this PR changes installer behavior. The PR body plus inspected screenshots give a high-confidence behavior path for the prompt/progress/completion flow, while cleanup postconditions are supported by contributor smoke notes.

Review metrics: 2 noteworthy metrics.

  • Files Changed: 7 files: 3 text files plus 4 screenshots. The patch combines installer logic, a rewritten cleanup helper, test assertions, and proof assets.
  • Cleanup Helper Size: 79-line wrapper replaced by 625-line direct cleanup script. The uninstall behavior now lives in a much larger PowerShell implementation that deserves maintainer ownership review.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P1] The PR intentionally moves destructive uninstall behavior from SetupEngine into an installer-owned PowerShell path that unregisters WSL and edits/deletes gateway identity and token state; unit tests do not fully settle that ownership choice.
  • [P1] I did not execute the Windows installer in this read-only Linux review, so runtime confidence relies on the contributor's real smoke notes plus the inspected screenshots.

Maintainer options:

  1. Accept Direct Cleanup With Proof (recommended)
    Merge if maintainers are comfortable that the posted installer smoke coverage and structural assertions are enough for the WSL unregister and credential cleanup path.
  2. Ask For Independent Windows Smoke
    Request or run one maintainer-side disposable WSL uninstall smoke that confirms distro removal, gateways.json, identity tokens, and the preserved No flow.
  3. Pause For SetupEngine Ownership
    Pause if maintainers want all uninstall artifact cleanup to remain in SetupEngine instead of maintaining a parallel PowerShell implementation.

Next step before merge

  • [P2] No narrow code repair remains; maintainers should decide whether to accept this security-sensitive direct uninstall cleanup path and then use normal merge validation.

Security
Cleared: No concrete security regression was found after the reparse-point guard was added, but the destructive uninstall path remains a maintainer-visible security-boundary merge risk.

Review details

Best possible solution:

Land the interactive uninstall flow once maintainers accept the installer-owned direct cleanup path, keeping the reparse-point guard and silent/Yes/No smoke coverage as the merge standard.

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

Not applicable as a bug reproduction; this PR changes installer behavior. The PR body plus inspected screenshots give a high-confidence behavior path for the prompt/progress/completion flow, while cleanup postconditions are supported by contributor smoke notes.

Is this the best way to solve the issue?

Yes, with maintainer acceptance: copying the helper to {tmp} and using direct WSL cleanup is a maintainable way to avoid install-directory file locks, and the reparse-point guard now protects the app-owned WSL directory. The remaining question is ownership of duplicated cleanup logic rather than a line-level defect.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 64b650cb3313.

Label changes

Label changes:

  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR includes inspected screenshots of the changed uninstall UI and reports real installer smoke validation for silent, interactive No, and interactive Yes flows.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
  • remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

  • P2: This is a normal-priority installer UX and cleanup improvement with limited blast radius but meaningful uninstall impact.
  • merge-risk: 🚨 security-boundary: The PR changes a destructive path that unregisters a local WSL distro and edits/deletes gateway identity and token state.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR includes inspected screenshots of the changed uninstall UI and reports real installer smoke validation for silent, interactive No, and interactive Yes flows.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR includes inspected screenshots of the changed uninstall UI and reports real installer smoke validation for silent, interactive No, and interactive Yes flows.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR includes inspected screenshots of the changed uninstall UI and reports real installer smoke validation for silent, interactive No, and interactive Yes flows.
Evidence reviewed

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.
  • [P1] PowerShell parser validation for scripts/Uninstall-LocalGateway.ps1.
  • [P1] Targeted InstallerIssAssertionTests.

What I checked:

  • Repository policy read: Read the full target AGENTS.md; its gateway/installer validation guidance applies, but this review stayed read-only and did not run build/test commands that create artifacts. (AGENTS.md:1, 64b650cb3313)
  • Current main behavior: Current master still uses [UninstallRun] to launch Uninstall-LocalGateway.ps1 from {app} and then deletes {app} through [UninstallDelete], so the new interactive choice is not already on main. (installer.iss:94, 64b650cb3313)
  • PR installer path: The PR moves uninstall handling into [Code], prompts non-silent users with Yes/No, copies the helper to {tmp}, waits synchronously, and only deletes generated app state after successful gateway cleanup. (installer.iss:94, 59f373a11975)
  • Reparse-point repair verified: The latest PR helper refuses to recursively delete the app-owned wsl\OpenClawGateway directory when it is a reparse point, addressing the previous review blocker. (scripts/Uninstall-LocalGateway.ps1:540, 59f373a11975)
  • Structural coverage: The updated installer assertions pin the prompt, silent cleanup, direct WSL unregister path, removal of the old CLI wrapper path, and the reparse-point guard string. (tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs:78, 59f373a11975)
  • Proof inspected: Downloaded and inspected the posted screenshots; they show the standard uninstall confirmation, the new local WSL gateway choice prompt, gateway removal progress, and completion dialog. The PR body also reports silent, interactive No, and interactive Yes installer smoke validation. (docs/images/uninstall-local-gateway-choice.png, 59f373a11975)

Likely related people:

  • ranjeshj: Prior current-main history shows Ranjesh on the SetupEngine redesign and recent installer uninstall cleanup, both central to this PR's behavior. (role: recent area contributor; confidence: high; commits: cefce3952ab1, 191f4d6f86cd; files: src/OpenClaw.SetupEngine/SetupSteps.cs, src/OpenClaw.SetupEngine/TrayArtifactCleanup.cs, installer.iss)
  • Scott Hanselman: Git history shows Scott introduced the local Inno installer helper and primary installer path that this PR modifies. (role: installer path introducer; confidence: medium; commits: 006fcd288b43, aaf50b758504, 5a555abf9e4c; files: installer.iss)
  • Mike Harsh: History shows Mike introduced the WSL gateway uninstall feature and related docs/scripts/tests that form the older behavior this installer hook replaced. (role: original uninstall feature contributor; confidence: medium; commits: 223db1fb847e; files: scripts/Uninstall-LocalGateway.ps1, docs/uninstall-portable.md, tests/OpenClaw.Tray.Tests/LocalGatewayUninstallTests.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. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels May 29, 2026
Refuse recursive gateway directory deletion when the target is a junction or symlink, matching the SetupEngine safety guard and preventing cleanup from crossing outside the app-owned WSL directory.

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

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review please. I added a reparse-point guard before recursive deletion of the app-owned WSL gateway directory and pinned it with installer assertion coverage. Validation rerun: PowerShell parser, targeted InstallerIssAssertionTests, ./build.ps1, Shared tests, Tray tests, reparse-point dry-run, and ARM64 Inno compile all passed.

@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: re-review please. I added a reparse-point guard before recursive deletion of the app-owned WSL gateway directory and pinned it with installer assertion coverage. Validation rerun: PowerShell parser, targeted InstallerIssAssertionTests, ./build.ps1, Shared tests, Tray tests, reparse-point dry-run, and ARM64 Inno compile all passed.

@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown

ClawSweeper assist: This needs a full correctness re-review, not a lightweight Q&A answer. The supplied context shows the prior blocker was the missing reparse-point guard, and the PR now has a new commit claiming to add that guard plus assertion coverage.

Evidence:

Suggested next action: Use @clawsweeper review on #591 so ClawSweeper can inspect the new diff and update the prior findings.


Source: #591 (comment)
Assist model: gpt-5.5, reasoning low.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels May 29, 2026
@ranjeshj
ranjeshj merged commit 8af4b70 into openclaw:master May 29, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant