Fix installer uninstall cleanup

[ranjeshj]

Summary

• Fix Inno AppId escaping to avoid malformed uninstall registry keys.
• Wait briefly during uninstall so WinUI/.NET file handles can release.
• Remove the installed app directory and generated runtime state on uninstall.
• Add installer assertions for AppId and uninstall cleanup behavior.

Testing

• Manually tested by installing and uninstalling OpenClaw; verified uninstall succeeded and removed everything.

[clawsweeper]

Codex review: found issues before merge. Reviewed May 28, 2026, 4:29 PM ET / 20:29 UTC.

Summary
The PR fixes Inno AppId escaping, adds a short uninstall delay, adds recursive app-directory cleanup, and pins those installer contracts with structural tests.

Reproducibility: yes. for the review finding from source: installing to a custom or existing {app} directory and then uninstalling would run the new recursive filesandordirs deletion. I did not run a Windows installer smoke in this read-only review.

Review metrics: 2 noteworthy metrics.

• Changed files: 2 files, +28/-1. The patch is small, so the merge decision centers on the new uninstall behavior rather than broad churn.
• Recursive uninstall delete: 1 {app} filesandordirs rule added. This single installer rule is the compatibility-sensitive behavior that structural tests do not make safe.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🌊 off-meta tidepool
Patch quality: 🧂 unranked krab
Result: blocked by patch quality or review findings.

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

Rank-up moves:

• Replace the full {app} recursive deletion with scoped or guarded cleanup that preserves uninstall diagnostics.
• [P2] Add installer coverage or documented validation for the custom-install and failed-cleanup cases.

Risk before merge

• [P1] Merging the PR as-is would make uninstall recursively delete the selected {app} directory, which can remove unrelated files for non-default/custom install locations.
• [P1] The same deletion removes the uninstall helper's result/error logs, so failed gateway cleanup may become harder to diagnose after uninstall.
• [P1] The PR body reports manual install/uninstall testing, but no build/test output or inspectable proof artifact is present in the discussion.

Maintainer options:

1. Scope The Uninstall Cleanup (recommended)
   Replace the full {app} recursive delete with targeted cleanup or a guard that only purges the known dedicated default directory after copying diagnostics somewhere durable.
2. Accept A Full Purge Deliberately
   Maintainers can choose to own the full-directory purge only if the installer prevents unsafe custom destinations and the PR documents the uninstall data-loss boundary.
3. Split The Risky Cleanup
   Land the AppId fix separately and pause the recursive cleanup until the installer deletion policy is settled.

Next step before merge

• [P2] A maintainer should decide the uninstall data-deletion policy before repair because the current patch changes destructive installer behavior.

Security
Needs attention: The diff introduces a destructive installer cleanup rule that needs scoping before merge.

Review findings

• [P1] Avoid recursively deleting the selected install directory — installer.iss:115

Review details

Best possible solution:

Keep the AppId correction and handle-release delay, but replace the full {app} purge with scoped or default-directory-guarded cleanup that preserves uninstall diagnostics and covers custom-install upgrade behavior.

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

Yes for the review finding from source: installing to a custom or existing {app} directory and then uninstalling would run the new recursive filesandordirs deletion. I did not run a Windows installer smoke in this read-only review.

Is this the best way to solve the issue?

No. The AppId fix is narrow, but full recursive deletion of {app} is not the safest uninstall cleanup path; the cleanup should be scoped or guarded and preserve diagnostics.

Full review comments:

• [P1] Avoid recursively deleting the selected install directory — installer.iss:115
  This adds [UninstallDelete] Type: filesandordirs; Name: "{app}" while the installer still uses {app} as the installation destination. On uninstall that removes every file under the selected app directory, including unrelated files if a user chose an existing/custom folder, and it also deletes the result/error logs written by Uninstall-LocalGateway.ps1. Please scope cleanup to known OpenClaw-generated paths or guard it to the dedicated default directory before deleting contents.
  Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

• add P1: The PR can cause destructive uninstall behavior for existing users if merged without scoping the recursive delete.
• add merge-risk: 🚨 compatibility: Changing uninstall cleanup to recursively remove {app} can break custom install layouts and erase diagnostic artifacts during upgrade/uninstall flows.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR, though the PR body only contains a manual-testing statement rather than an inspectable artifact.

Label justifications:

• P1: The PR can cause destructive uninstall behavior for existing users if merged without scoping the recursive delete.
• merge-risk: 🚨 compatibility: Changing uninstall cleanup to recursively remove {app} can break custom install layouts and erase diagnostic artifacts during upgrade/uninstall flows.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🌊 off-meta tidepool and patch quality is 🧂 unranked krab.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The external-contributor proof gate does not apply to this collaborator-authored PR, though the PR body only contains a manual-testing statement rather than an inspectable artifact.

Evidence reviewed

Security concerns:

• [medium] Broad recursive uninstall deletion — installer.iss:115
  The new uninstall rule removes all contents of {app}. Because the installer destination can represent a user-selected folder, this can erase unrelated local files and should be constrained before release.
  Confidence: 0.84

What I checked:

• Repository policy read: AGENTS.md was read in full; its validation policy is relevant, but this review was required to stay read-only so no build or tests were run locally. (AGENTS.md:1, dbb0e1e5a7a0)
• Current main installer state: Current main has the malformed-looking AppId and only the existing UninstallRun hook; there is no current-main UninstallDelete cleanup rule. (installer.iss:21, dbb0e1e5a7a0)
• PR diff adds broad uninstall deletion: The PR adds a second UninstallRun sleep and then adds [UninstallDelete] with Type: filesandordirs; Name: "{app}", which is the blocking review concern. (installer.iss:115, f6e19533a91a)
• Uninstall diagnostics live under app directory: The uninstall helper writes uninstall-gateway-result.json and uninstall-gateway-error.log under {app}, so deleting {app} also removes the post-mortem files the helper creates. (scripts/Uninstall-LocalGateway.ps1:44, dbb0e1e5a7a0)
• Installer history and ownership: The Inno installer and installer assertion tests were introduced and recently maintained in commits around the primary Inno installer path. (installer.iss:21, 9f7022651fe9)

Likely related people:

• Scott Hanselman: Git blame and follow-log history point to the Inno installer, AppId, UninstallRun hook, and installer assertion tests being introduced and recently maintained by this author. (role: introduced installer/uninstall path; confidence: high; commits: 9f7022651fe9, 5a555abf9e4c, aaf50b758504; files: installer.iss, tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs, scripts/Uninstall-LocalGateway.ps1)

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.