Skip to content

feat(migration): add gated Inno-to-Store migration preview - #1461

Open
natalie-aguinaldo wants to merge 21 commits into
openclaw:mainfrom
natalie-aguinaldo:user/natalie-aguinaldo/inno-to-store-migration
Open

natalie-aguinaldo wants to merge 21 commits into
openclaw:mainfrom
natalie-aguinaldo:user/natalie-aguinaldo/inno-to-store-migration

Conversation

@natalie-aguinaldo

@natalie-aguinaldo natalie-aguinaldo commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Related: #1374

PR 1 of 2: migration foundation and gated end-to-end preview. Production Store migration remains disabled in this PR. PR 2 is planned to complete #1374, including shipping UX and production enablement once release prerequisites and acceptance proof are satisfied.

Current-main integration and proof refresh: 1e504ceba488f04d616f12d086374c25847eaff6 merges main at c7063081. Seven additive conflicts were resolved by preserving the migration and chat architecture rows and all six locales' resource entries. The full non-Dev Release Inno app and gated Debug MSIX were rebuilt after this commit, then exercised in the isolated Windows VM. Source tree: ff8c281a013e6f7ee1e979cf79fc50eaa8db1b67. Both earlier review fixes remain intact.

Latest head: 71c91d05 supersedes e47197b6. Two commits follow it. f91e6895 rejects foreign mutating grants on the uninstall receipt, including generic rights, and checks receipt ownership before trusting it. 71c91d05 fixes a regression this branch introduced in 6a8aeded: the gateway cleanup quoted every wsl.exe argument, so control flags such as --unregister were executed as commands inside the distro and the gateway distro was never unregistered. Both are proven at runtime below.

The long-standing "no registered WSL distro" gap is now closed. Earlier revisions could only prove the no-distro cleanup branch. A real registered WSL2 distro is now created, unregistered by the shipped helper, and confirmed gone, with an unrelated bystander distro preserved.

Product sources: 22db82746143542f3d69f69813f6884b43a3b780 removes the extra action legends from both preview dialogs. The body now refers directly to the native Yes/No buttons in all six locales, preserving disclosures, default No, and behavior. Required validation passed on these sources; the packaged journey and both screenshots below were freshly captured from this committed head using test package 2026.9.20.0. Source tree: da67d55bf6b3634a96841bedd8583034f39b8d30.

What Problem This Solves

Inno and Store installations share user state and cannot safely operate as independent production copies. Inno uninstall can also remove the gateway that the Store app needs to preserve.

User Impact

Ordinary Store builds do not enter migration. An explicitly enabled Debug package exercises a same-user, matching-architecture handoff while preserving existing state in place.

Ordinary Inno builds include the completion guard and uninstall-preservation safeguards. Normal Inno startup acquires a read-shared migration handle before checking completion and retains it until process exit. A contended handle proves a live migration and blocks startup. An unreadable one does not, so startup continues: the completion receipt check stays authoritative and still fails closed. A valid completed receipt blocks normal Inno startup; uninstall preserves generated state. Uninstall is never refused because migration state is unreadable. Only a contended lock stops it, and any other uncertainty suppresses destructive gateway cleanup instead of cancelling removal.

Why This Change Was Made

  • Detect the exact supported per-user Inno installation. Block ambiguous registrations, unsupported versions/layouts, and architecture mismatches.
  • Capture bounded metadata and hashes under exclusive ownership. Protect intent and completion with current-user DPAPI, without exporting credentials or moving gateway/Local AI payloads.
  • Record completion only after fresh inventory validation and canonical active-gateway credential resolution. This proves local credential availability, not a live gateway connection.
  • Serialize runtime state writers, Store migration, and Inno removal across Windows sessions. Retain completion until removal, inventory revalidation, and startup-preference application succeed.
  • Preserve completion across clock rollback and fail closed on uncertain source processes, inaccessible state, and operational errors.

Review fixes

  • e547733c: normal Inno retains prepare.lock through process exit, including failed startup/shutdown. Store preparation/completion recheck source activity across sessions while holding the exclusive file lock. Busy completion supports Retry. The session-local mutex is no longer treated as sufficient cross-session ownership.

  • 46fd8b6e: consent in all six locales discloses protected records, the Inno startup block, required manual uninstall, Store restart, preservation, and no automatic uninstall. Close/Retry no longer promises that nothing has changed.

  • 7aef6b20: the preservation checker no longer falls through to an unbounded inline watchdog when its own watchdog cannot start; it warns and exits 2 (uncertain, preserve). Uninstall helpers no longer carry uninsneveruninstall, so they are removed with the app instead of being stranded on disk.

  • 4c5899b8: the migration directory DACL is now scrubbed of foreign grants on every acquisition, and explicit ACEs on the receipt file are scrubbed too. A previous churn optimization had skipped re-hardening for any protected, owner-matching DACL without inspecting its rules, which left a pre-existing directory able to grant Everyone: FullControl. A foreign principal deleting completed.dpapi would have been read as an absent receipt and allowed destructive gateway cleanup.

  • e47197b6: fixes a pre-existing uninstall defect that predates this PR (introduced in Add uninstall choice for local WSL gateway #591). Two problems, both in the gateway cleanup helper this PR already reworks:

    • wsl.exe writes UTF-16LE on many Windows builds while the redirected pipe was decoded as 8-bit, so captured output arrived NUL-interleaved. Every pattern in Test-DistroNotFound was matching T\0h\0e\0... and none could ever fire, leaving the "already unregistered, nothing to do" success path unreachable in practice.
    • A host with no WSL at all was treated as a hard failure, surfacing OpenClaw could not remove the local WSL gateway. Exit code: 1 with a Retry button against a condition no retry can fix.

    Why it is in this PR rather than a separate one: it is the blocker for the required windows-clean-installer-upgrade proof. The matrix remove cases exist specifically to exercise the no-distro cleanup branch, and that branch could not complete until this was fixed. It was invisible until now because earlier proof runs declined gateway removal and never executed the branch.

    How it was found: the real installer matrix on a WSL-less guest, not review. Neither ClawSweeper nor the human review caught it. The fix is covered by a test that executes the real functions extracted from the script rather than asserting on source text, because a source-text contract cannot detect an encoding defect; reverting either half fails it.

Maintainer decisions recorded

  • Inno safeguards stay always-on, outside the STORE_MIGRATION_PREVIEW gate. An Inno user who never opts into the preview can still have a completed Store migration, so gating the safeguard would let uninstall destroy preserved gateway state.
  • The silent/unattended uninstall path keeps suppression by design. Checker uncertainty deliberately preserves the gateway rather than removing it, no uninstaller override is offered, and the removal path is Settings -> Local Gateway -> Remove Local Gateway, or wsl --unregister OpenClawGateway. The silent path now logs the leftover distro and directory so an enterprise admin reading the uninstall log has something actionable.

Scope and review guide

Responsibility Owner / invariant
Eligibility and admission InnoInstallationDetector, version policy, startup coordinator: no guessed source or production opt-in
Records and inventory MigrationRecordCodec, MigrationInventory, MigrationPreparation: bounded same-user state, atomic records, source state preserved
Cross-session exclusion MigrationOperationLock, InnoSourceActivityVerifier, startup guard: process-lifetime reader versus exclusive Store writer; uncertain process inspection blocks
Consent and completion Consent/preparation/completion coordinators: explicit informed consent, fresh inventory, canonical credential resolution
Uninstall preservation installer.iss, Test-InnoMigration.ps1, cleanup helper: preservation check and destructive work cannot race completion
Finalization Removal verifier/finalizer: verify removal, apply saved startup preference, delete receipt last
Bootstrap Narrow guards run before ordinary activation/settings/services; App only composes and retains lifetime ownership

GatewayRegistry, CredentialResolver, and AutoStartManager remain canonical owners. There is no new node command, MCP tool, gateway client, or payload-relocation path.

Follow-ups, intentionally not included

PR 2 (complete #1374): shipping UI, Open Installed apps, Inno Store-launch/consent handoff, graceful shutdown/manual fallback, activation/accessibility coverage, and production enablement after identifying a verified safe Inno release and satisfying release prerequisites and acceptance proof.

The fixture's 2026.9.5.0 minimum is not a production compatibility claim. The eventual minimum must include the process-lifetime lock; process scanning alone cannot prevent an older nonparticipating binary starting after inspection.

Evidence

Merged-source automated validation and the rebuilt, locally test-signed x64 packaged journey passed. Separate actual Release-app proof crossed Windows sessions 0 and 1. These are isolated local proofs, not official signed-release or Store-distributed acceptance.

Change Type

  • Feature
  • Bug fix
  • Tests or validation
  • Docs or instructions
  • Security hardening

Scope

  • Tray or WinUI UX
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required proof pools

  • windows-clean-installer-upgrade: passed, 8 of 8 cases on f91e6895. Every case reached exitCode 0 with visible uninstall completion, and the guest was returned to baseline with matching hashes. The real uninstaller invoked the changed preservation checker in all eight runs (Migration preservation check returned 0.). Inputs are official signed v2026.9.4 and a full current-head unsigned local Release Inno installer. Because the installer is unsigned, this is not shipping-signature proof. This matrix ran on f91e6895; a rerun against 71c91d05 is pending and noted under Not verified / blocked.
  • windows-wsl-gateway-e2e: passed, 3 of 3 cases on 71c91d05. Real WSL2 distro registered, then removed by the shipped Uninstall-LocalGateway.ps1: distro unregistered, install directory removed, unrelated bystander distro preserved, second run idempotent. Synthetic Alpine rootfs rather than a provisioned gateway, so gateway runtime behavior and live pairing are still not claimed.
  • windows-winui-interactive: current-source English packaged consent/Retry captures and blocked Inno startup captures passed. Full shipping UX, localized layouts, and accessibility remain outstanding.
  • windows-11-arm64: not verified / blocked. No native ARM64 host used.

These are applicable host classes, not claims that maintainer-scheduled proof pools completed. No MXC execution, GPU, signing-policy, or Smart App Control change is claimed.

Validation

Command Result / provenance
.\build.ps1 Passed on final sources committed as 22db8274
dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore 4,087 passed; 33 skipped; 0 failed on 22db8274 sources
dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore 3,086 passed; 0 failed on 22db8274 sources
dotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --no-restore 1,068 passed; 1 skipped; 0 failed on 22db8274 sources
Tray filter FullyQualifiedName~InnoMigrationContractTests|FullyQualifiedName~LocalizationValidationTests 38 passed, including plain Yes/No assertions
Six-locale format and action-legend regressions Seven checks failed against the old implementation, then passed with the fix
Full non-Dev Release x64 self-contained app and Inno installer Passed; local proof version 2026.9.22.0, rebuilt from committed 22db8274
Gated Debug x64 production-identity MSIX publish Passed; package 2026.9.20.0, built from committed 22db8274
Six-locale three-way XML entry comparison Passed; 2,477 keys per locale, including both parents' additions
git diff --check Passed

Current head 71c91d05 revalidation: .\build.ps1 passed; Shared 4,087 passed / 33 skipped; Tray 3,103 passed; 0 failed. The Tray count rises by one against f91e6895: a new contract test asserts that wsl.exe control flags are passed unquoted. It is verified non-vacuous, since the previous implementation turns --list into "--list" and fails it.

On f91e6895: .\build.ps1 passed; Shared 4,087 passed / 33 skipped; Tray 3,102 passed; Connection 1,104 passed / 1 skipped; 0 failed. Connection rises by 24 over e47197b6, covering foreign-owner rejection, generic-rights masks, inherit-only ACE handling, and receipt ownership.

Earlier head e47197b6 revalidation: .\build.ps1 passed; Shared 4,087 passed / 33 skipped; Connection 1,077 passed / 1 skipped; Tray 3,102 passed; 0 failed. ActivationRouterTests passed 36/36 on five consecutive runs on this head, supporting a baseline-flake reading rather than a regression from this branch. New coverage pins the recovery contracts: contended versus unreadable lock handling, uninstall never cancelling on unreadable state, directory security applied only on create or drift, and the preservation notice carrying its own removal instructions.

Clock regressions execute the real Windows PowerShell 5.1 preservation checker and isolated cleanup helper without changing the host clock. Completed receipts survive simulated backward-clock correction; corrupt records, mismatched bindings, and invalid intents remain rejected. Cross-process tests cover shared readers, exclusive contention, normal/crash handle release, and source restart between preparation and completion.

Historical failures are not hidden: the original pre-merge full Connection run had two checker failures, followed by isolated and full-suite passes. An initial cross-session-development Shared run timed out waiting for the Piper extractor fixture PID; it passed alone, then complete required validation reruns passed. The current merged-source runs above passed. Skips are not counted as proof.

Rubber-duck / code review

Earlier non-Codex whole-branch/scoped reviews found and drove fixes for uninstall/completion serialization, excessive process-query access, and clock-rollback receipt rejection. The two latest feedback fixes received direct source review and regression/runtime proof.

ClawSweeper revision 4 reviewed 46fd8b6e, accepted both code fixes and the refreshed runtime evidence, and reported no actionable code or security findings. It requested current-main conflict resolution, installed-user compatibility proof, and maintainer acceptance of the always-on safeguards.

The additive merge resolution received direct review and exact three-way resource comparison. External structured-review attempt: python .\.agents\skills\autoreview\scripts\autoreview --mode local --engine claude --no-web-search --no-tools .... The merge bundle exceeded the helper's size limit. The subsequent narrow Yes/No bundle passed that gate but was blocked by the unavailable Claude executable. No independent post-merge or Yes/No model-review result is claimed, and no Codex fallback was run. This is not a security-audit certification.

Real behavior proof

UI scope: These native dialogs are intentional PR 1 preview surfaces for validating migration behavior and safety, not the final shipping design. PR 2 will replace the Store consent and close-Inno dialogs with dedicated migration UI and clearly labeled actions. The Inno-side blocked-startup guidance also needs explicit UX treatment in PR 2. Consent disclosures and startup safety checks will remain intact.

Environment: disposable local IXPTools / Hyper-V Windows 11 x64 VM; same-user synthetic state; interactive non-elevated app. Provider: local Hyper-V. Cloud lease ID and run URL: N/A. Harnesses are session-local, not committed repository entry points.

Actual Inno runtime, across Windows sessions

Command: pwsh -NoProfile -File run-cross-session-vm.ps1 from an elevated host controller. The full non-Dev Release x64 self-contained unpackaged app was rebuilt from 1e504ceb, launched in guest session 1, and checked from guest session 0. Output: cross-session-vm-results-r4.

Six cases passed: normal startup with no receipt; corrupt-receipt compatibility; exclusive-owner blocked startup; valid-completion blocked startup; unreadable-receipt blocked startup; reparse lock-path rejection without modifying the target. A running app excluded the other session's exclusive handle while allowing uninstall-style readers. Handles were available after process exit. Normal-app cases used forced process exit; blocked dialogs exited by dismissal. Graceful full-app shutdown is not separately claimed.

This cross-session evidence is from the merge commit, not a rerun on 22db8274. The later change only alters Store preview copy/formatting, its tests, and documentation; the Inno runtime guard is unchanged.

Merged-head Release Inno app blocked while another Windows session owns the exclusive migration lock

Actual packaged consent and migration journey

Commands: pwsh -NoProfile -File build-migration-startup-preview.ps1, then pwsh -NoProfile -File run-migration-startup-vm.ps1 -Resume.

  • Package: OpenClawFoundation.OpenClaw_2026.9.20.0_x64__rfcbke2p71se2, locally test-signed inside the VM and launched from its installed package location.
  • Clean build head: 22db82746143542f3d69f69813f6884b43a3b780. This is a post-commit rebuild, not a source-equivalence claim.
  • Unsigned package SHA-256: 1475B50CEDDA7AFE21DCF061064E9807E6DB18137FD4E9C9DA17E210E09DC036.
  • Inno fixture SHA-256: 6C98E61317C1BCB84D96DCA029DB0731424D3B075CB3FB52D3998E07F5890770. It uses current installer/codec/helper sources and a versioned test apphost, not an official released full installer.
  • Declining exited without creating intent/completion or changing synthetic settings/gateway catalog.
  • Accepting showed close-Inno/Retry, then wrote intent/completion only after release and validation. Restart retained the same receipt.
  • Actual silent Inno uninstall was refused with exit 1 under the exclusive lock, then succeeded after release while preserving state.
  • Store finalization applied saved AutoStart=false, cleared records, and continued normal packaged startup. Process identity/logs, not a normal-window screenshot, establish that final step.

Current-head packaged consent uses plain Yes and No without action legends and retains all migration disclosures

Current-head close-Inno guidance uses Yes to retry and No to exit without colon legends

Copied current packaged-run result (startup-preview-results-20260923-002654):

{
  "declinePreservedSetupWithoutRecords": true,
  "restartPreservedReceipt": true,
  "blockedUninstallExitCode": 1,
  "exclusiveLockBlockedUninstall": true,
  "uninstallPreservedState": true,
  "sourceSettingsUnchanged": true,
  "preparedIntentWritten": true,
  "finalized": true,
  "success": true,
  "cases": ["consent-decline", "consent-retry", "restart-waiting", "finalize"]
}

Screenshots were visually inspected locally. They are GitHub-hosted attachments uploaded through the contributor's public fork, where the account has the push access required for attachment upload. No credentials, raw settings/identity files, or unrelated desktop content are published.

All three image URLs were verified after publication with unauthenticated HTTP 200 responses and SHA-256 matches against the inspected local captures.

Actual Inno install/uninstall matrix, eight cases

Command: Run-InnoMatrixVm.ps1 -Execute -RunId 71c91d05-r1 -ExpectedManifestSha256 8260C3D0... from an elevated host controller against a disposable local Hyper-V guest. Payload pinned by manifest SHA256 8260C3D0CBB4E33282A6DC684CC687677574D9983B15317EDE16079CD5BE6576, built from current head 71c91d05. Output: results-71c91d05-r1. The guest summary records commit: 71c91d059fd4c9865c70e3975bfaeadb4c99aaa0, so the matrix is current-head rather than prior-head.

8 of 8 cases passed. Each case reached exitCode 0 with visibleCompletion: true under an elevated same-user interactive task, and the guest was restored to baseline with matching hashes (restoration: restored-with-matching-hashes).

The real uninstaller invoked the changed preservation checker in all eight runs, evidenced by Migration preservation check returned 0. appearing in every one of the eight uninstall.log files.

Case Kind AutoStart Gateway choice Exit Visible completion
fresh-autostart-false-retain fresh false retain 0 yes
fresh-autostart-false-remove fresh false remove 0 yes
fresh-autostart-true-retain fresh true retain 0 yes
fresh-autostart-true-remove fresh true remove 0 yes
upgrade-autostart-false-retain upgrade false retain 0 yes
upgrade-autostart-false-remove upgrade false remove 0 yes
upgrade-autostart-true-retain upgrade true retain 0 yes
upgrade-autostart-true-remove upgrade true remove 0 yes

Per-case evidence includes install logs, startup/registration state, UI screenshots, and UIA accessibility dumps for the uninstall confirmation and the gateway retain/remove choice.

Previous-release upgrade gateway removal prompt presented during uninstall

upgrade-autostart-true-remove: the previous-release upgrade path with autostart enabled and the gateway remove choice. The real Yes/No gateway prompt is presented during uninstall and clicked by the harness.

Previous-release upgrade remove case completing successfully

upgrade-autostart-true-remove: the same previous-release upgrade path reaches visible successful completion, exit code 0, no error dialog.

The four remove cases each ran the real gateway cleanup helper end to end. The earlier e47197b6-r1 run exposed the pre-existing uninstall defect fixed in e47197b6: on the prior head the first remove case blocked on a could not remove the local WSL gateway dialog and timed out. The 71c91d05-r1 run confirms that fix stays in place, with the same case passing with no error dialog.

That run also demonstrated why the watchdog in 7aef6b20 matters: on a WSL-less host wsl.exe emits an interactive "Press any key to install... this prompt will time out in 60 seconds" prompt during uninstall. The 120 s bound contains it; without a bound it is an indefinite hang in an uninstaller.

Deviations, stated plainly: the installer under test was unsigned, so this is not shipping-signature proof. Prerequisite install used an explicitly approved elevated step; first-time UAC remains unproven. The matrix guest has no registered WSL distro, so its remove cases prove the real no-distro cleanup branch; live distro deletion is proved separately below.

Actual WSL gateway removal against a real registered distro

This closes the long-standing "no registered WSL distro" gap in the matrix above.

Command: Run-GatewayProofVm.ps1 -Execute -RunId gateway-proof-r7 against a disposable local Hyper-V guest with real WSL2 installed (wsl.2.7.14.0.x64.msi, SHA-256 DB084E53...). Output: results-gateway-proof-r7. The proof invokes the shipped scripts/Uninstall-LocalGateway.ps1 directly, which is exactly how Inno invokes it.

3 of 3 cases passed on 71c91d05.

Case What it proves Result
registered A genuinely registered distro is terminated, unregistered, and its install directory removed exit 0, stillRegistered: false, installDirRemoved: true
bystander Cleanup removes only the distro it was asked to remove unrelated OpenClawProofBystander still registered
idempotent A second run on already-clean state is not an error exit 0

Registration state is read from the Lxss registry, the same key the product's ownership gating reads. Before cleanup:

[
  { "Name": "OpenClawProofBystander", "BasePath": "...\\OpenClawProofTray\\wsl\\OpenClawProofBystander" },
  { "Name": "OpenClawProofGateway",   "BasePath": "...\\OpenClawProofTray\\wsl\\OpenClawProofGateway" }
]

After cleanup, only the bystander remains:

[
  { "Name": "OpenClawProofBystander", "BasePath": "...\\OpenClawProofTray\\wsl\\OpenClawProofBystander" }
]

Copied result summary:

{
  "success": true,
  "stage": "complete",
  "wslVersion": 2,
  "cases": [
    { "case": "registered",  "pass": true, "exitCode": 0, "stillRegistered": false, "installDirRemoved": true,
      "reportedMessage": "Local WSL gateway 'OpenClawProofGateway' removed." },
    { "case": "bystander",   "pass": true, "stillRegistered": true },
    { "case": "idempotent",  "pass": true, "exitCode": 0 }
  ]
}

This proof caught a real regression that this branch introduced. The first run against a real distro failed every wsl.exe call:

[2026-09-23T20:42:50.5586866-07:00] wsl.exe --list --quiet exited 127.
/bin/sh: --list: not found
[2026-09-23T20:42:50.9269799-07:00] wsl.exe --terminate OpenClawProofGateway exited 127.
/bin/sh: --terminate: not found
[2026-09-23T20:42:53.2837185-07:00] wsl.exe --unregister OpenClawProofGateway exited 127.
/bin/sh: --unregister: not found

6a8aeded on this branch replaced Start-Process -ArgumentList (an array, never quoted) with a ProcessStartInfo command string, and quoted every argument to build it. wsl.exe matches its control flags against the raw command line without stripping quotes, so "--unregister" was passed through to the distro's shell and executed as a command. This was reproduced independently on a real WSL host outside the VM: wsl.exe --list --quiet exits 0 and lists distros, wsl.exe "--list" "--quiet" exits 127.

71c91d05 narrows ConvertTo-ProcessArgument to quote only values containing whitespace or quotes. After the fix, the same log reads:

[2026-09-23T20:54:26.8293752-07:00] wsl.exe --list --quiet exited 0.
OpenClawProofGateway
OpenClawProofBystander
[2026-09-23T20:54:27.0066321-07:00] wsl.exe --terminate OpenClawProofGateway exited 0.
[2026-09-23T20:54:29.2162236-07:00] wsl.exe --unregister OpenClawProofGateway exited 0.

The regression is now locked by GatewayUninstall_PassesWslControlFlagsUnquoted, which executes the real argument-building path rather than asserting on source text. It is verified non-vacuous: the previous implementation turns --list into "--list" and fails the test.

Deviations, stated plainly: the rootfs is a minimal Alpine image, not a provisioned gateway, so this proves cleanup and bystander preservation, not gateway runtime behavior or live pairing. Uninstall-LocalGateway.ps1 is invoked directly; SetupEngine ownership gating is covered by unit tests, not by this proof. The matrix cases above have not yet been rerun on 71c91d05.

Actual foreign-principal receipt authority, six cases

Command: Invoke-ForeignReceiptProof.ps1 from an elevated host controller. The proof creates a real local principal, applies real ACLs, and runs the shipped Windows PowerShell 5.1 preservation checker. Exit 2 means "preserve the gateway", exit 0 means "cleanup authorized".

6 of 6 cases matched expectations.

Case Setup Expected Actual
clean No receipt, no foreign access 0 0
foreign-dacl Foreign principal can modify the state directory 2 2
foreign-owned Receipt genuinely owned by the foreign principal 2 2
residual Acknowledged residual: owner deletes and leaves no trace 0 0
foreign-read Foreign principal has read/list only 0 0
foreign-generic Foreign principal holds GENERIC_ALL 2 2

foreign-generic is the case that matters most: the raw mask on the ACE is 0x001F01FF,0x10000000, which intersects no named FileSystemRights bit, yet the kernel maps it to FILE_ALL_ACCESS. A naive named-bit check would have passed it as harmless.

Two limits are disclosed rather than hidden. residual returns 0 because nothing observable remains, so the checker genuinely cannot distinguish a deleted receipt from one that never existed. A domain administrator holding inherited Modify rights will likewise strand cleanup at exit 2. Both are accepted properties of a same-user, filesystem-observable design, not defects the checker can close.

Actual uninstall authority, real distro, control and refusal

Command: Run-AuthorityProofVm.ps1 -Execute -RunId authority-proof-r2 against a disposable Hyper-V guest. Real current-head Inno install, a real registered WSL2 OpenClawGateway distro, and the real interactive uninstaller in both arms.

25 of 25 assertions passed.

Arm Receipt state Gateway prompt Checker Cleanup script Distro after
control-removed none appeared, answered Yes returned 0 invoked unregistered, directory removed
authority-refused owned by a foreign principal never appeared returned 2 never invoked still registered, directory intact

The control arm exists so the refusal is not vacuous: the same harness, same installer, and same real distro demonstrably do perform a destructive unregister when authority checks out.

The foreign principal is genuinely distinct from the uninstalling user. The receipt owner is ...-1002 (OpenClawAuthProof), while the signed-in user is ...-1000, recorded with full SDDL in foreign-receipt.json.

In the refusal arm the uninstaller logged its own decision and left recovery instructions:

Migration preservation check returned 2.
Migration preservation check unavailable: skipping destructive gateway cleanup.
The OpenClawGateway WSL distro and ...\wsl\OpenClawGateway were left in place.

The refusal is structural rather than a silently failed deletion: noWslCleanupLogExists count=0, so the cleanup script was never invoked and no wsl call was attempted at all. HKCU\...\Lxss still contains OpenClawGateway afterward, and the install directory is intact.

Control arm, the real destructive choice is offered, with No focused by default:

Control arm gateway removal prompt appears with No focused by default

Refusal arm, same installer and same point in the flow, the destructive choice is replaced by an acknowledgement-only warning:

Authority-refused uninstall shows an OK-only warning and offers no removal choice

Disclosed limits. The fixture imports a minimal Alpine rootfs rather than a provisioned gateway runtime, so this proves registration and directory lifecycle for the exact OpenClawGateway target, not gateway runtime behavior. Both uninstalls ran through an elevated interactive task, matching the matrix harness shape.

Existing gateway state survives Inno removal and is adopted by the Store build

Run on a clean Windows VM at head 71c91d05. Inno 2026.9.23.0 installed over a pre-existing gateway fixture, uninstalled with No to the local gateway prompt, then the current-head Store package installed and launched.

Check Result Establishes
fixture-had-state PASS, 7 files Non-vacuous. Real state existed to lose.
durable-state-byte-identical PASS No gateway, settings, or device-key file changed or was removed.
volatile-churn-reported 2 changed, 1 added openclaw-tray.log, Logs\diagnostics.jsonl, store-migration\prepare.lock.
distro-still-registered PASS Managed WSL distro survived uninstall.
distro-enterable PASS, openclaw-distro-ok Distro still boots, not merely registered.
store-distro-registered PASS Store build sees the same distro.

Store package verified as 2026.9.23.0, commit 71c91d059f..., sha256 52E9DF49.... Across the Store leg only openclaw-tray.log, Logs\diagnostics.jsonl, and ext4.vhdx changed; nothing was added or removed. gateways.json, settings.json, and the device key are byte-identical from the pre-uninstall baseline through to a running Store app.

User answers No to the local gateway prompt during Inno uninstall:

uninstall gateway prompt answered No

The current-head Store build lists the preserved gateway:

Store build adopted the preserved gateway

Final verdict, all checks pass:

final existing-state verdict

Limits. Credentials are synthetic and gateway.example.test does not resolve, so the Connection screenshot shows Authentication failed by design. This proves preserved state is read and adopted; it does not claim a live gateway handshake. Volatile files are reported by name rather than excluded.

Real gateway migration on a developer host

Run on a developer machine at head 71c91d05 against a real gateway: paired via device token, running for 3 days, with real credentials, device keys, and a registered WSL distro. This is the live counterpart to the synthetic VM run above, where the preserved state was a fixture.

Stage Result Establishes
Consent Preview dialog shown and accepted Migration is gated and explicit, not silent
Completion recorded completed.dpapi 694 b, intent.dpapi 6,550 b Receipt written only after consent
Inno uninstall Completed, no local gateway prompt Receipt suppressed the destructive prompt
gateways.json 130374774B before and after Gateway records byte identical
settings.json 33AEB8D49D before and after Settings byte identical
Device keys 3 keys unchanged, including A156E37B91 Pairing identity preserved
WSL distro OpenClawGateway-Dev still registered Uninstall did not touch the distro
Store adoption Connected, paired via device token, 1 client Preserved credential completed a real handshake
Finalization completed.dpapi and intent.dpapi removed Records cleaned only after Inno removal was verified

The missing gateway prompt is the designed path rather than luck. CheckCompletedStoreMigration returns MigrationResult = 10, which sets LocalGatewayCleanupRequested := False and exits before the prompt at installer.iss:354.

The Store app reports Install type: Packaged (MSIX) and version 2026.9.23.0, so the connected session is the Store package and not a surviving Inno process.

Consent dialog before any change

migration consent preview dialog

Inno uninstall completes with no local gateway prompt

inno uninstall complete, no gateway prompt

Store build connected using the preserved device token

store app connected via preserved device token

Packaged identity of the connected app

store app info, packaged MSIX 2026.9.23.0

Limits and disclosures.

  • Head. This ran before the completion-attestation fix. That fix changes only the receipt-absent branch of Test-InnoMigration.ps1. The path proven here is receipt-present (decode, then exit 10), which the fix leaves unchanged.
  • Node mode. The Store screenshot shows node mode disabled. Migration did not change this setting. settings.json is byte identical and contains EnableNodeMode: False, a value that was set about six minutes before migration began. The individual capability toggles (NodeCanvasEnabled, NodeScreenEnabled, NodeSystemRunEnabled) all carried over as True.
  • Distro ownership. The distro here is OpenClawGateway-Dev under OpenClawTray-Dev. The production uninstaller targets OpenClawGateway, so managed-distro ownership transfer is still not proven. This is a handoff and adoption proof, not an ownership-transfer proof.
  • Chat. Chat fails on this host with a Copilot HTTP 401 raised by the gateway's own token, predating this work and unrelated to migration.

Not verified / blocked

  • The installer/upgrade matrix now passes 8 of 8 on current head 71c91d05 (see above). Remaining gaps in that area: the installer under test is unsigned, and no logoff/reboot startup proof is claimed.
  • Matrix installation/uninstallation required an elevated test controller after the bundled VC runtime requested secure-desktop UAC. Apps ran with a limited token; UAC stayed enabled. First-time UAC interaction is not proven.
  • Independent structured review of the merged head: helper blocked before model invocation. ClawSweeper's accepted review applies to the preceding head.
  • Official signed x64/ARM64 artifacts, Store-distributed acceptance, and native ARM64 behavior.
  • Real managed-WSL preservation/continued management and live remote-only connectivity. Synthetic credentials, no live gateway handshake.
  • Runtime AutoStart=true, startup-platform refusal, real multi-user access-denied classification, and the complete interruption/failure matrix.
  • Full normal-launch/auto-start/openclaw: acceptance, shipping accessibility, non-English visual layout, and interactive Windows Settings uninstall. Fixture removal was silent Inno uninstall.
  • InitializeUninstall is Inno Pascal and no executing unit test runs it. Its runtime behavior is now exercised by the eight-case matrix and the two-arm authority proof above rather than by unit tests, so regressions in the Pascal path depend on that VM harness, which is session-local and not a committed repository entry point.
  • New MCP discovery/invocation proof is N/A: no node capability or MCP surface changes.

Security Impact

  • New permissions/capabilities: No.
  • Secrets: Existing same-user state is read through canonical owners. Protected records contain metadata/hashes, not a credential export.
  • Network: No migration-specific network call or live-connectivity prerequisite.
  • Execution: Inno invokes the shipped read-only preservation checker and existing cleanup helper. No new remote execution or node-command surface.
  • Data access: Bounded reads and atomic record writes. Reparse paths, mismatched bindings, and ambiguous sources are rejected. Same-user protection does not defend against malicious code already running as that user.

Compatibility and Migration

  • Ordinary Store migration remains disabled. Preview requires explicit build opt-in, a test minimum, Debug packaging, and production identity. No runtime setting enables it.
  • State/payloads stay in place. Credentials retain existing ownership and precedence.
  • Explicit --uninstall --confirm-destructive remains deliberate full removal and is never invoked by migration.
  • Checker uncertainty deliberately preserves the local gateway and offers no uninstaller override, because at that point nothing on the machine distinguishes a broken check from a real migration. The uninstall still completes. The preserved distro and directory are named in the notice, in the uninstall log for silent runs, and in docs/uninstall-portable.md.
  • Production enablement and completion of Migrate from the Inno (.exe) app to the Store MSIX #1374 belong to PR 2 after a verified safeguard-containing Inno release and required acceptance evidence.

Review Conversations

Both reported code findings are addressed by e547733c and 46fd8b6e: cross-session source exclusion and inaccurate consent copy. Maintainer acceptance of always-on Inno safeguards and release compatibility proof remain distinct rollout requirements. This PR is no longer a draft and is open for team review; maintainer edits remain enabled.

natalie-aguinaldo and others added 8 commits September 21, 2026 14:25
Add protected migration records, read-only source inventory, completion-aware Inno safeguards, exact installation discovery, and disabled-by-default Store startup admission. Consent, adoption, and finalization remain future work. Local build, focused tests, and x64 fixture proof passed before checkpoint; structured review remains deferred.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
Keep the preview build-time gated and read-only. Consent only observes the Inno mutex and stops before adoption, completion, uninstall, or normal startup. Required validation and x64 packaged VM proof passed; structured review remains deferred.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
Hold the migration operation lock across Inno removal and cleanup, and re-detect the source under the completion lock. Use limited-information process queries to exclude proven unrelated processes while failing closed on uncertain source candidates. Add regression coverage and document the ownership guarantees.

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

Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
Keep completed-receipt reads independent of the current wall clock while retaining write-time timestamp checks, intent policy, and all binding and structural validation. Add backward-clock regressions through the codec, Windows PowerShell checker, and cleanup preservation path.

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

Copilot-Session: a91896be-e75b-4a88-932e-61b78125ab4a
@clawsweeper

clawsweeper Bot commented Sep 22, 2026

Copy link
Copy Markdown

🦞👀
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 clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 22, 2026
@clawsweeper

clawsweeper Bot commented Sep 22, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 24, 2026, 3:07 AM ET / 07:07 UTC (Revision 24).

ClawSweeper review

What this changes

Adds a gated preview for moving an Inno-installed Windows Companion to the Store package, with protected migration records, startup and uninstall safeguards, localized dialogs, tests, and release guidance.

Merge readiness

Blocked before merge - 8 items remain

Keep this PR open. Current main lacks the migration handoff, and the branch has substantial real Windows proof. One observed receipt-loss path can still permit destructive gateway cleanup after migration, while the rollout and recovery contract needs maintainer approval.

Priority: P2
Reviewed head: 71c91d059fd4c9865c70e3975bfaeadb4c99aaa0
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Substantial current-head runtime evidence supports the staged path, but a demonstrated receipt-loss safety gap and production upgrade proof limit readiness.
Proof confidence 🦐 gold shrimp (3/6) Needs stronger real behavior proof before merge: Authority-chain proof required: exact-head Windows VM traces cover the Inno uninstaller, preservation checker, registered WSL removal, foreign-owned receipt refusal, fresh installs, and upgrades; a developer-host handoff also shows a preserved paired gateway. The disclosed lost-receipt case still authorizes cleanup, without a safe final-effect result for silent uninstall. The new persisted receipt and production upgrade contract remain unproven with signed x64 and ARM64 packages and a provisioned production gateway. 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.
Patch quality 🦐 gold shrimp (3/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: exact-head Windows VM traces cover the Inno uninstaller, preservation checker, registered WSL removal, foreign-owned receipt refusal, fresh installs, and upgrades; a developer-host handoff also shows a preserved paired gateway. The disclosed lost-receipt case still authorizes cleanup, without a safe final-effect result for silent uninstall. The new persisted receipt and production upgrade contract remain unproven with signed x64 and ARM64 packages and a provisioned production gateway. 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.
Evidence reviewed 8 items Introduced migration scope: The pinned merge-base-to-head diff adds 49 files or file changes, including migration coordinators, the Inno preservation checker, and installer integration.
Receipt-loss decision: When the completion file is absent and the remaining directory ACL is trusted, the new checker returns exit 0. It cannot distinguish a lost completed receipt from a migration that never occurred.
Destructive final effect: Checker exit 0 permits silent uninstall to request gateway cleanup, while nonzero verdicts suppress it; successful cleanup then removes generated app state.
Findings 1 actionable finding [P1] Keep lost completion receipts from authorizing gateway removal
Security Needs attention Receipt loss bypasses completion preservation: The checker bases final cleanup authority on the current presence and ACL of one file; a vanished completed receipt yields exit 0 and can reach WSL unregister during silent uninstall.

How this fits together

The Windows tray app and Inno uninstaller share settings, credentials, and a managed WSL gateway. The new migration records coordinate the Store app’s handoff and determine whether Inno may start or remove gateway state.

flowchart LR
A[Inno installation and saved state] --> B[Store preview admission]
B --> C[Consent and source checks]
C --> D[Protected migration receipt]
D --> E[Inno startup and uninstall decision]
E --> F[Preserve or remove gateway]
D --> G[Store finalization]
Loading

Decision needed

Question Recommendation
Should migration-aware controls ship in ordinary Inno builds before Store migration is enabled, and what recovery contract must protect a completed handoff if its receipt disappears? Stage with durable recovery: Approve the early Inno safeguard only after receipt-loss cleanup is made safe and signed-package upgrade and recovery proof passes.

Why: The staged release changes ordinary Inno behavior and defines a persistent destructive-cleanup authorization boundary; the contributor’s disclosed residual is not maintainer acceptance.

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: Authority-chain proof required: exact-head Windows VM traces cover the Inno uninstaller, preservation checker, registered WSL removal, foreign-owned receipt refusal, fresh installs, and upgrades; a developer-host handoff also shows a preserved paired gateway. The disclosed lost-receipt case still authorizes cleanup, without a safe final-effect result for silent uninstall. The new persisted receipt and production upgrade contract remain unproven with signed x64 and ARM64 packages and a provisioned production gateway. 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.
  • Keep lost completion receipts from authorizing gateway removal (P1) - After Store records completion, deleting completed.dpapi leaves a trusted directory but makes this branch return exit 0. The Inno uninstaller treats that as permission for gateway cleanup, including automatic cleanup during silent uninstall, even though the completed handoff still needs that gateway. Preserve the completed state through a durable recovery signal or refuse destructive cleanup when completion cannot be ruled out; prove the final WSL effect for the lost-receipt case.
  • Resolve security concern: Receipt loss bypasses completion preservation - The checker bases final cleanup authority on the current presence and ACL of one file; a vanished completed receipt yields exit 0 and can reach WSL unregister during silent uninstall.
  • Resolve merge risk (P1) - After a completed migration, loss of the sole completion receipt can make silent Inno uninstall authorize gateway removal; the reported checker result is exit 0, and a safe final WSL effect has not been shown for that case.
  • Resolve merge risk (P1) - Ordinary Inno builds gain migration-aware startup and uninstall behavior before Store migration is enabled. The supported upgrade and recovery contract needs explicit maintainer approval.
  • Resolve merge risk (P1) - The exact signed x64 and ARM64 production handoff, including continued management of a provisioned production gateway, remains unverified.
  • Complete next step (P2) - Repair the lost-receipt cleanup path, provide its final WSL-effect proof and signed existing-state upgrade proof, then obtain maintainer approval for the staged Inno recovery contract.
  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

  • [P1] Keep lost completion receipts from authorizing gateway removal — scripts/Test-InnoMigration.ps1:232-237
  • [high] Receipt loss bypasses completion preservation — scripts/Test-InnoMigration.ps1:237
Agent review details

Security

Needs attention: The new receipt-controlled uninstall boundary is generally hardened, but loss of the receipt can still authorize destructive cleanup after migration.

Review metrics

Metric Value Why it matters
Changed lines production +3562 net, tests +4133, docs +214 The large production addition has a stated staged migration purpose and substantial focused test coverage, making the safety boundary the main review concern.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #1374
Summary: This PR implements a staged portion of the open Inno-to-Store migration request; the earlier gateway-uninstall choice is related history with a distinct purpose.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Preserve after receipt loss (recommended)
    Add durable completion evidence or another fail-safe uninstall decision, then show that silent uninstall makes no WSL removal call after the receipt is lost.
  2. Accept the residual
    Explicitly accept that an untraceable lost receipt restores the old destructive silent-uninstall behavior and document the supported recovery procedure.
  3. Pause staged landing
    Hold this foundation until signed-package lifecycle proof and the later Store migration experience can be reviewed together.

Technical review

Best possible solution:

Keep Store migration gated, make completed-handoff preservation survive loss of its primary receipt, and validate the approved recovery path with signed production packages before rollout.

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

Yes for the review finding: source shows that an absent completion receipt can return exit 0, and the contributor reports that result in a real Windows check. Migration itself is a new capability, not an existing-behavior bug.

Is this the best way to solve the issue?

Unclear. The staged, build-gated foundation is a reasonable shape, but the receipt-loss decision and signed production upgrade contract need resolution before it is the safest landing path.

Full review comments:

  • [P1] Keep lost completion receipts from authorizing gateway removal — scripts/Test-InnoMigration.ps1:232-237
    After Store records completion, deleting completed.dpapi leaves a trusted directory but makes this branch return exit 0. The Inno uninstaller treats that as permission for gateway cleanup, including automatic cleanup during silent uninstall, even though the completed handoff still needs that gateway. Preserve the completed state through a durable recovery signal or refuse destructive cleanup when completion cannot be ruled out; prove the final WSL effect for the lost-receipt case.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 0e45bb673025.

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This is a substantial but still gated migration improvement with a blocking safety case, rather than an existing production outage.
  • merge-risk: 🚨 compatibility: Ordinary Inno startup and uninstall behavior changes before the Store migration feature is enabled.
  • merge-risk: 🚨 security-boundary: A persisted completion receipt determines whether uninstall may reach destructive gateway cleanup.
  • merge-risk: 🚨 availability: Receipt and lock recovery decisions can block app startup or leave gateway cleanup unavailable on affected installations.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • 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: exact-head Windows VM traces cover the Inno uninstaller, preservation checker, registered WSL removal, foreign-owned receipt refusal, fresh installs, and upgrades; a developer-host handoff also shows a preserved paired gateway. The disclosed lost-receipt case still authorizes cleanup, without a safe final-effect result for silent uninstall. The new persisted receipt and production upgrade contract remain unproven with signed x64 and ARM64 packages and a provisioned production gateway. 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.

Evidence

Security concerns:

  • [high] Receipt loss bypasses completion preservation — scripts/Test-InnoMigration.ps1:237
    The checker bases final cleanup authority on the current presence and ACL of one file; a vanished completed receipt yields exit 0 and can reach WSL unregister during silent uninstall.
    Confidence: 0.95

What I checked:

  • Introduced migration scope: The pinned merge-base-to-head diff adds 49 files or file changes, including migration coordinators, the Inno preservation checker, and installer integration. (src/OpenClaw.Connection/Migration/StoreMigrationCompletionCoordinator.cs:39, 71c91d059fd4)
  • Receipt-loss decision: When the completion file is absent and the remaining directory ACL is trusted, the new checker returns exit 0. It cannot distinguish a lost completed receipt from a migration that never occurred. (scripts/Test-InnoMigration.ps1:227, 71c91d059fd4)
  • Destructive final effect: Checker exit 0 permits silent uninstall to request gateway cleanup, while nonzero verdicts suppress it; successful cleanup then removes generated app state. (installer.iss:333, 71c91d059fd4)
  • Contributor runtime proof and disclosed limit: The PR body reports an exact-head 8-of-8 Inno install/uninstall matrix, registered-WSL removal and refusal arms, and a live developer-gateway handoff. Its six-case receipt authority check also reports that deleting a receipt without a trace returns cleanup-authorizing exit 0. The prepared media manifest contains screenshots from the earlier merged uninstall-choice PR, not these new migration runs; the copied runtime results are the relevant proof assessed here.
  • Release policy: The branch’s release guide requires signed x64 and ARM64 package, restart, storage, and recovery validation before migration rollout; the PR body identifies those acceptance gaps. (docs/RELEASING.md:119, 71c91d059fd4)
  • Current main and release boundary: Neither fetched main nor release v2026.9.4 contains the introduced migration checker or migration coordinator files, so this PR remains necessary. (0e45bb673025)

Likely related people:

  • karkarl: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • ranjeshj: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • natalie-aguinaldo: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

  • Prevent receipt loss from authorizing gateway cleanup and show the final WSL result under silent uninstall.
  • Add signed-package existing-state and provisioned-gateway handoff proof for supported architectures.
  • Obtain an explicit decision on the always-on Inno upgrade and recovery contract.

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.

History

Review history (23 earlier review cycles; latest 8 shown)
  • reviewed 2026-09-23T20:33:22.854Z sha c48af9d :: needs real behavior proof before merge. :: [P2] Keep the preservation check bounded when its watchdog cannot start | [P2] Remove retained installer helpers after preservation uninstall | [P2] Reject unsafe access rules on an existing migration directory
  • reviewed 2026-09-23T23:21:48.739Z sha e47197b :: needs real behavior proof before merge. :: [P1] Check receipt-file ownership before skipping ACL repair | [P2] Bring the release guide into line with the final recovery behavior
  • reviewed 2026-09-23T23:33:00.867Z sha e47197b :: needs real behavior proof before merge. :: [P1] Check the receipt owner before skipping ACL repair | [P2] Correct the unavailable-lock release guidance
  • reviewed 2026-09-24T04:05:45.957Z sha 71c91d0 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-24T04:17:50.952Z sha 71c91d0 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-24T04:35:55.207Z sha 71c91d0 :: needs real behavior proof before merge. :: none
  • reviewed 2026-09-24T05:52:22.967Z sha 71c91d0 :: needs real behavior proof before merge. :: [P1] Keep deleted completion receipts from authorizing gateway removal
  • reviewed 2026-09-24T06:22:01.345Z sha 71c91d0 :: needs real behavior proof before merge. :: [P1] Keep lost completion receipts from authorizing gateway cleanup

@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Sep 22, 2026
natalie-aguinaldo and others added 2 commits September 22, 2026 12:14
Hold an Inno runtime reader until process exit and validate source activity under Store's exclusive migration lock. Share process inspection, preserve fail-closed startup guidance, and cover contention, exit, and completion retry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
Explain protected migration records, the Inno startup block after successful validation, manual source uninstall, and the Store restart requirement. Clarify retry guidance and cover all six locales with disclosure regression tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. labels Sep 22, 2026
Resolve additive architecture-ledger and six-locale resource conflicts while preserving migration safeguards and current-main chat changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 22, 2026
Remove action legends and refer directly to native Yes/No buttons in all six locales without changing consent or retry semantics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
natalie-aguinaldo and others added 2 commits September 22, 2026 19:26
…cesses

Keep the existing 30-second deadline and parallel test execution. Run fixture scripts noninteractively, terminate timed-out child trees before teardown, and retain the original timeout when cleanup or diagnostic capture fails. Add synchronized parent/child regressions including an unreadable diagnostic log. This hardens the test harness without claiming to resolve the original CI-only timeout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
Replace the readiness-file handshake that hit a sharing violation on a Windows CI runner. Signal a named event after diagnostic output is written, and pass the child PID through stdout. This fixes the regression fixture only; the original cleanup timeout remains unresolved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Sep 23, 2026
@natalie-aguinaldo
natalie-aguinaldo marked this pull request as ready for review September 23, 2026 16:34
@karkarl

karkarl commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Global triage: NEEDS_HUMAN_TEST. Take confidence 20%; recommendation confidence 92%; effort large; risk high.

Reviewed exact head 479e567575f8. Architecture extraction, locale parity, and the PowerShell 5.1 codec are sound. Blocking behavior sits outside the preview gate: ordinary Inno startup and uninstall can fail closed on an inaccessible prepare.lock, and checker failure silently suppresses the user's gateway-removal choice. The windows-clean-installer-upgrade matrix remains 0 passed, 1 harness-blocked, 7 not run. Resolve or explicitly accept those recovery contracts before merge. The exact-head ActivationRouter timeout appears to be a baseline flake, so rerun rather than attribute it to this patch. The body also still says this is draft, but it is not.

Harden the Inno-to-Store migration safety path against outcomes that
previously authorized destructive gateway cleanup on incomplete evidence.

- Treat a present-but-unverifiable completion receipt as "preserve", not
  "absent". Only FileNotFoundException/DirectoryNotFoundException exit 0;
  every other validation failure exits 2.
- Bound every child process wait in both shipped scripts via a shared
  Start-BoundedProcess helper built on ProcessStartInfo, and retain the
  checker's diagnostics instead of discarding them.
- Launch the checker watchdog through the explicit System32 Windows
  PowerShell path, since $PSHOME under PowerShell 7 has no powershell.exe.
- Narrow the destructive admission phase to Enter-DestructivePhase so
  pre-destructive failures report uncertainty rather than completion.
- Route installer exit code 2 to preservation instead of a retry prompt,
  renumber internal failure codes to 102/103, and keep the cleanup script
  installed so the uninstaller no longer names a file it deletes.
- Allow startup after a failed or refused startup preference, and show a
  user-visible notice when Windows durably refuses the startup task, since
  only the user can re-enable it.

Validation: build.ps1; Shared 4087 passed; Tray 3098 passed;
Connection 1072 passed. Watchdog timeout and admission-phase mutation
proofs run locally.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. proof: sufficient Contributor real behavior proof is sufficient. labels Sep 23, 2026
Migration coordination is preview-only, but its lock sat outside the preview
gate on two ordinary paths. An unreadable prepare.lock refused to launch the
app and aborted the uninstaller outright, for users who never migrated.

- Inno startup now blocks only when the lock is genuinely contended, which
  proves a live migration. Any other acquisition failure logs and continues,
  because an unreachable lock says nothing about migration. The completion
  receipt check below remains authoritative and still fails closed.
- InitializeUninstall never returns False for inaccessible migration state.
  Only a contended lock stops an uninstall; otherwise removal proceeds and
  destructive gateway cleanup is suppressed instead. The reparse-point check
  stays sequential so a rejected path can never reach ForceDirectories.
- Apply directory security on create or on drift rather than on every
  acquisition, so an ordinary launch no longer needs WRITE_DAC/WRITE_OWNER.
- Make preservation self-contained: the notice now names the in-app removal
  path and the wsl --unregister command instead of pointing at documentation
  the uninstaller does not link, and the silent path logs the preserved
  distro and directory for enterprise audit.
- Document the installer preservation case and why no override is offered.

Validation: build.ps1; Shared 4087 passed; Connection 1075 passed;
Tray 3100 passed. ActivationRouterTests passed 5/5 on this head, consistent
with a baseline flake rather than a regression from this branch.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Sep 23, 2026
natalie-aguinaldo and others added 3 commits September 23, 2026 14:29
…pers

The checker exists because Add-Type can stall on a contended %TEMP% or an
antivirus scan, and installer.iss waits for it with ewWaitUntilTerminated.
Falling back to the inline check when the watchdog could not start
reintroduced exactly that unbounded stall. Report the uncertain verdict
instead; exit 2 already fails closed and preserves the gateway.

The three uninstall helpers also carried uninsneveruninstall, but they run
during usUninstall, which happens before Inno removes files. Generated
app-state deletion only runs after successful gateway cleanup, so any
uninstall that kept the local gateway, including the ordinary "No" choice
unrelated to migration, left executable helpers behind in the former app
directory.

The retained-helper contract test pinned the flags it should have rejected,
so it is inverted rather than deleted.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
RequiresHardening skipped re-hardening whenever the DACL was protected and
owned by the current user, without inspecting what those rules granted. A
pre-existing store-migration directory can satisfy both conditions and still
carry an Allow rule for an unrelated principal.

That principal could delete completed.dpapi. Test-InnoMigration.ps1 maps a
missing receipt to "no migration happened" and exits 0, which authorizes
destructive local gateway cleanup during uninstall. The same receipt gates
ordinary Inno startup.

Before the hardening shortcut landed, SetAccessControl ran on every
acquisition and would have removed such a grant. Restore that guarantee
without reintroducing per-launch DACL churn by re-hardening only when a rule
falls outside the owner, LocalSystem, and BuiltinAdministrators.

RepeatAcquisition_LeavesAnAlreadyHardenedDirectoryUntouched used an
Everyone: Read ACE as its no-rewrite canary, which is the grant this change
must scrub. Its canary is now a trusted principal with distinct inheritance
flags, so it still proves the absence of churn.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
Real uninstall proof on a guest without WSL surfaced two pre-existing
defects in the gateway cleanup helper, both dating to openclaw#591. Neither was
reachable before, because earlier proof runs declined gateway removal and
never executed this branch.

wsl.exe writes UTF-16LE on many Windows builds while the redirected pipe is
decoded as 8-bit, so captured output arrived NUL-interleaved. Every pattern
in Test-DistroNotFound matched against that text and none could ever fire,
leaving the "already unregistered" success path unreachable in practice.
Strip NUL in the captured output instead of pinning an encoding that varies
by Windows version.

A host with no WSL, or with WSL but no distributions, cannot be holding our
gateway. Treat both as nothing to remove rather than surfacing "OpenClaw
could not remove the local WSL gateway. Exit code: 1", which offered Retry
against a condition no retry can fix.

The new test executes the real functions extracted from the script rather
than asserting on source text, since a source-text contract cannot detect
an encoding defect. Reverting either half fails it.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Sep 23, 2026
@natalie-aguinaldo

Copy link
Copy Markdown
Contributor Author

Addressing clawsweepers new comments

natalie-aguinaldo and others added 2 commits September 23, 2026 17:38
The Inno uninstall checker decided whether to clean up gateway state based
on a receipt file it did not verify it could trust. A non-trusted principal
who could write the receipt (or its directory) could suppress or forge the
cleanup decision.

Trust is now established before the receipt is read:

- Owner must be the current user, SYSTEM, or Administrators.
- Any foreign Allow ACE that can mutate the receipt breaks authority.
  Read/list grants are tolerated, since installer.iss creates the state
  directory with plain ForceDirectories and inheritable read ACEs are
  common in managed fleets.
- Generic rights (GENERIC_ALL, GENERIC_WRITE) count as mutating. They
  intersect no named FileSystemRights bit but the kernel maps them onto
  FILE_ALL_ACCESS / FILE_ADD_FILE, so a named-bit mask alone missed them.
- Inherit-only ACEs are skipped; they do not apply to the object itself.

When authority cannot be established the checker exits 2 and the uninstaller
preserves state, rather than silently proceeding.

Also replaces Get-Acl and Get-ChildItem with .NET types throughout. The
uninstaller launches PowerShell in a constrained environment where
Microsoft.PowerShell.Security fails to autoload, which made the check throw
for every user and strand cleanup unconditionally.

Residual, deliberately not sealed: a receipt file's own owner can grant
itself DELETE and remove the receipt without altering the directory, which
leaves nothing observable to the checker. Inherited Modify grants held by a
domain administrators group will likewise strand cleanup rather than
authorize it. Both fail closed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a
Real VM proof on Windows 11 24H2 with WSL 2.7.14 showed every wsl.exe
call in the gateway cleanup failing with exit 127:

  wsl.exe --list --quiet exited 127.
  /bin/sh: --list: not found

wsl.exe matches its control flags against the raw command line and does
not strip quotes, so a quoted "--unregister" is not recognized as a flag
and is executed as a command inside the distro instead. The gateway
distro was therefore never unregistered, and uninstall reported
succeeded=false with exit 127.

This regressed in 6a8aede, which replaced Start-Process -ArgumentList
(an array, never quoted) with ProcessStartInfo (a single string) and
quoted every argument to build it. The migration-checker call site was
unaffected because it already passes bare flags and quotes only values.

Quote only values that need it. The escaping rules for values that do
need quoting are unchanged, including doubling trailing backslashes.

Reproduced independently against real WSL on a developer host:
  wsl.exe --list --quiet       -> exit 0, lists distros
  wsl.exe "--list" "--quiet"   -> exit 127, /bin/bash: --list: not found

Proof: real WSL2 gateway cleanup in a Hyper-V VM, before and after.
  before: exit 127, "Failed to unregister WSL distro", still registered
  after:  exit 0, "Local WSL gateway removed", unregistered,
          unrelated bystander distro preserved, second run idempotent

The new contract test executes the real function rather than asserting
on source text, matching the existing WSL-less-host test. Source-text
assertions cannot catch an argv-shape bug. Verified non-vacuous: the
previous implementation turns --list into "--list" and fails it.

Validation: build.ps1, Shared 4087 passed, Tray 3103 passed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d5c700f0-7fa2-46ed-bd1a-10a11521b63a

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. 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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants