fix(desktop): verified update teardown on Windows+WSL, install integrity check, and resilient WSL readiness handshake - #5998
Open
Jgratton24 wants to merge 8 commits into
Conversation
…iness handshake Two user-facing failures shared one splash-hang symptom; both are fixed here. Update apply (half-applied install): - instance.stop() now reports whether the child fully finalized; the install path aborts (and restarts backends, surfacing the error through the update state machine) instead of running NSIS while a backend that holds install-dir handles may still be alive. - Before handing off to the installer on Windows, verify from inside the distro that no Linux process still holds cwd/exe/fd/mmap references under /mnt/c/<install dir> (ensureWindowsPathReleased: TERM, then KILL, then report busy). 9p handles held from WSL block file replacement on the Windows side, which is how app.asar and app.asar.unpacked ended up from different builds. - Post-apply integrity check: the build stamps apps/server/dist/desktop-build-manifest.json (asar-unpacked on Windows); startup compares it against the asar version and refuses to launch a half-applied install with an actionable repair dialog instead of an opaque ERR_MODULE_NOT_FOUND crash. WSL readiness handshake (indefinite 'Connecting to WSL'): - Networking mode now comes from wslinfo --networking-mode instead of inferring it from the FIRST hostname -I address (a WSL-side tailscaled putting a CGNAT 100.64/10 address first made mirrored hosts look NAT'd and pointed every probe at an unroutable IP). - Readiness races loopback and every enumerated distro address; the first candidate that answers wins and becomes the advertised base URL. - After all static candidates time out, the server's persisted server-runtime.json origin is consulted as a last-resort candidate. - A run that spawns but never answers any probe is terminated and retried; after 3 readiness timeouts the failure surfaces like an exhausted preflight (dialog + Windows fallback on the primary, inline error on the secondary) with the probed URLs named, instead of sitting on the splash forever. Resolved mode and candidates are logged at startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Effect service conventions review: one convention violation found — the new readiness fallback path uses Effect.catchTag, which the conventions replace with Effect.catchTags({ ... }) even when recovering a single tag. Everything else (subpath namespace imports, service-member additions on DesktopWslEnvironment with matching layerTest stubs, environment-based dependency acquisition in DesktopUpdates.make, Schema.TaggedErrorClass attributes with an attribute-derived message) matches the conventions.
Posted via Macroscope — Effect Service Conventions
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f49d4ea5f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
ApprovabilityVerdict: Needs human review This PR introduces significant new runtime behavior across startup (install integrity check), backend lifecycle (readiness probe racing, failure caps), and update installation (verified teardown). Despite good test coverage, the scope of changes to critical desktop infrastructure warrants human review. You can customize Macroscope's approvability policy. Learn more. |
- Recover known tags with Effect.catchTags (convention) in the readiness fallback path. - Anchor the install-dir holder scan: descendants match "$dir/" and the directory itself matches as an exact readlink line, so /mnt/c/application is no longer classified as a holder of /mnt/c/app. - Split the release-check result: "unavailable" (wsl.exe would not spawn — no running VM can hold /mnt handles) proceeds with a warning, while "unknown" (timeout/path-translation failure against a distro that was hosting this backend moments ago) now aborts the install. - abortInstall restarts only instances that were running before teardown, so a previously-stopped backend is not resurrected by a failed attempt. - readServerRuntimeState reads the state-dir flavor the spawned backend actually uses (dev/ for --dev-url runs, userdata/ for packaged) instead of preferring a possibly-stale packaged file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ess surfacing - A packaged Windows build with NO unpacked build manifest is now treated as a half-applied update (new app.asar over a pre-stamp unpacked tree) instead of skipping the check — every Windows artifact that ships the checker also ships the stamp. Platforms that pack the server dist inside the asar still skip. - A non-fatal surfacing on a primary that is already the Windows backend (only reachable via readiness exhaustion) now shows one dialog and stops instead of applying a no-op WSL fallback and looping dialog + restart forever. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…WSL release always aborts - The integrity check now distinguishes read failures: only a NotFound on packaged Windows is treated as a half-applied update. An unreadable manifest (access denied, I/O error) logs and skips instead of falsely bricking a healthy install. - Drop the "unavailable" proceed path from the WSL release check: a release-script spawn failure happens after wslpath already succeeded against the distro, so it cannot prove WSL is gone — every unverified outcome now reads "unknown" and aborts the install. A machine whose WSL layer is genuinely broken recovers on the next launch via the preflight Windows fallback (no WSL config in the pool means no release check). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nstead of restarting forever Ports the still-relevant piece of pingdotgg#3623: a backend that spawns fine but keeps exiting before it ever answers a readiness probe previously restarted silently forever (500ms-10s backoff, no cap, no surfacing). After 5 consecutive never-ready exits the failure now surfaces through onPreflightFailed — dialog + Windows fallback on the primary, inline Connections error on the WSL secondary — exactly like exhausted preflight and readiness failures. The counter is scoped strictly to post-spawn exits: pre-spawn preflight retries have their own counter, a crash after the backend was ready resets the streak (an established backend crashing is a different failure than one that never comes up), and the budget refreshes on ready, on external stop, and on a fresh manual start so one cap firing does not permanently remove the retry budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks
github-actions
Bot
added
size:XXL
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 02343e8. Configure here.
…r-ready exit cap A readiness-timeout termination closes the run scope, and the resulting child exit previously ALSO incremented exitFailureAttempt — every readiness timeout double-counted as a crash, so after a readiness-cap recovery the leftover streak could fire a second, misleading "exited N times" dialog just two real failures later. The readiness kill now marks the run stopRequested before closing its scope, which both excludes it from the exit cap and routes finalize to discardSession (the failure log was already captured via persistFailureSnapshot). Regression test drives five readiness cycles with a retrying onPreflightFailed and asserts the exit cap never fires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Jgratton24
force-pushed
the
fix/update-apply-wsl-handshake
branch
from
August 10, 2026 13:54
8e4c156 to
61c09f0
Compare
…SL-primary condition resolvePrimary only selects the WSL backend when BOTH wslOnly and wslBackendEnabled are set (wslRequested), but the non-fatal surfacing branch checked wslOnly alone. With wslOnly persisted true while wslBackendEnabled is false, a Windows primary exhausting readiness retries took the WSL branch: a misleading "WSL backend is still unavailable" dialog, an unrelated in-memory WSL settings mutation, and one wasted retry cycle before the stop branch finally fired. The guard now mirrors resolvePrimary's own condition, so a Windows primary surfaces the generic backend-unavailable dialog and stops immediately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live testing on a real Windows+WSL host (WSL 2.7, mirrored) showed that plain Linux fds and mmaps through 9p/DrvFs do NOT block Windows-side file replacement, while a Windows-side handle does (the backend child runs as the app executable and reads through app.asar). Comments now describe the WSL release check as the defensive guarantee it is, with the verified stop of Windows backend children as the confirmed-mechanism fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
Two Windows+WSL reliability fixes that share one user-visible symptom (an indefinite "Connecting to WSL…" splash), plus a guard that turns the corrupted-install crash into an actionable dialog. Desktop-only; no
apps/serverchanges.1. Update apply is verified before the installer runs (fixes the half-applied-update corruption).
DesktopBackendInstance.stop()now reports whether the child process actually finalized within the timeout instead of silently swallowing it (DesktopBackendManager.ts).DesktopUpdates.ts) aborts — restarting backends and surfacing the error through the update state machine — if any backend can't be verified stopped, instead of handing off to NSIS while a process still holds files under the install dir.wsl.exerelay does not mean the Linux-side server (or helpers it spawned, e.g. cloudflared) exited with it. NewensureWindowsPathReleased(DesktopWslEnvironment.ts) terminates and verifies from inside the distro that no Linux process still holds cwd/exe/fd/mmap references under/mnt/c/<install dir>— 9p/DrvFs handles held from WSL block file replacement on the Windows side, which is howapp.asarandapp.asar.unpackedend up from different builds.apps/server/dist/desktop-build-manifest.json(underapp.asar.unpackedon Windows); startup compares it against the asar's version and refuses to launch a mixed-build install with a clear repair dialog (DesktopInstallIntegrity.ts) instead of the opaqueERR_MODULE_NOT_FOUNDmain-process crash. Missing manifest (dev, macOS/Linux, pre-stamp installs) skips the check, so no false positives.2. WSL endpoint detection and the readiness handshake are no longer single-point-of-failure.
wslinfo --networking-mode(first-party, present on WSL 2.6.x and 2.7.x) instead of being inferred from the first address inhostname -I— whose ordering shifts whenever Tailscale, Docker bridges, or VPN adapters exist inside the distro. Heuristic fallback for older WSL matches any distro IP against Windows interfaces, never position one, and CGNAT100.64/10addresses are never advertised.server-runtime.jsonorigin is consulted as a last-resort candidate (gated on it naming this instance's port).25 new tests across
DesktopBackendManager,DesktopUpdates,DesktopWslEnvironment,DesktopBackendConfiguration, andDesktopInstallIntegritycover the regressions above (CGNAT-first ordering, docker0-first,wslinfoabsent, one-shot-probe limbo, unverified stop, WSL busy at install time, mixed-build detection). Full desktop suite: 473 passing.Why
Two field incidents on
0.0.33-nightlybuilds, plus these open issues:getDistroIpImpltakes the first IPv4 fromhostname -I; Docker bridge IPs sort first and are unreachable from Windows. The same mechanism breaks with a WSL-sidetailscaled(CGNAT address first under mirrored networking), which we reproduced on 0.0.33-nightly: the server was listening on0.0.0.0:3773, reachable via loopback, and logged zero client requests because every probe went to the CGNAT address.runBackendProcessforked a singlewaitForHttpReadywith a 60s budget; on timeoutonReadinessFailureonly logged, leaving the run alive, never ready, never retried, never surfaced.ERR_MODULE_NOT_FOUNDsignature; the integrity check turns the mixed-state launch into a clear "installation is damaged/incomplete" dialog whenever the unpacked stamp has diverged from the asar. It does not (and cannot, from app code) stop NSIS from leaving the exe launchable during extraction.The half-applied update we debugged: an in-app update left
app.asar+ the exe at the old build whileapp.asar.unpacked/node_moduleswas new →ERR_MODULE_NOT_FOUNDon every launch, with the updater having reported nothing. Root cause: install teardown killed thewsl.exerelay and proceeded on a 5s timeout that was silently swallowed, while the Linux-side server exited asynchronously still holding 9p handles into the install dir. Same version installed fine with the app closed.Sizing note: aware of the contribution guidelines — roughly half of this diff is tests, and the two fixes share the same touched files (
DesktopBackendManager/DesktopWslEnvironment), which is why they're one PR. Happy to split into updater-atomicity and handshake PRs if you'd prefer.This PR deliberately avoids every file touched by #2829 (orchestrator v2) — verified against its full file list, so it merges cleanly before or after.
UI Changes
No renderer UI changes. Two native dialogs are added/extended (update-install-aborted error via the existing update state machine; "installation is damaged" message box with an Open Download Page button). Not screenshotted — they require a packaged Windows build to trigger, which this dev environment can't produce; dialog copy is in
DesktopInstallIntegrity.tsandDesktopUpdates.ts.Checklist
🤖 Generated with Claude Code
Note
High Risk
Touches critical desktop paths: startup gate, update install handoff, backend lifecycle (
stopAPI change), and WSL endpoint selection—bugs could block launch, block updates, or misroute the renderer.Overview
Hardens Windows desktop startup and updates against half-applied installs and the “connecting to WSL forever” failure mode.
Install integrity at boot: Packaged Windows builds now compare the asar app version to a stamped
desktop-build-manifest.jsonunderapp.asar.unpacked. A mismatch or missing stamp (when expected) blocks bootstrap with a repair dialog (optional releases link) instead of an opaque main-process module crash. The build pipeline writes that manifest into staged server dist.Safer in-app updates: Before
quitAndInstall, every pooled backend must confirm shutdown within a longer timeout;stop()now returns whether the child actually finalized. Install aborts if any backend is still alive, restarts only backends that were running, and surfaces an install error while keeping the downloaded update. On Windows with WSL backends, a new in-distro check kills/verifies that nothing under/mntstill holds the install directory.WSL networking and readiness: Single-IP
getDistroIpis replaced by all IPv4s fromhostname -I,wslinfo --networking-mode, CGNAT filtering, and smarter mirrored vs NAT host choice. Readiness races loopback plus every candidate URL, can fall back toserver-runtime.json, rebinds the advertised base URL to whichever probe wins, terminates stuck runs, and caps repeated readiness timeouts / never-ready exits via existing preflight surfacing. Primary pool handling avoids infinite restart when the Windows primary exhausts readiness retries.Reviewed by Cursor Bugbot for commit 0fdb80d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add install integrity check, verified backend teardown, and resilient WSL readiness probing on Windows
desktop-build-manifest.jsonstamped intoapp.asar.unpackedat build time; on mismatch or missing manifest (Windows only), shows an error dialog and aborts startup.ensureWindowsPathReleasedto confirm WSL processes have released the install directory before invokingquitAndInstall; aborts if the result isbusyorunknown.getDistroIps) for readiness in parallel, avoids CGNAT addresses, and falls back to persisted server runtime state when all static candidates time out.DesktopBackendInstance.stopnow returnsboolean(true = process finalized before timeout); readiness and exit-before-ready failures are bounded by caps and surfaced viaonPreflightFailedrather than looping indefinitely.DesktopWslEnvironment.getDistroIpis removed and replaced bygetDistroIps; any code depending on the old single-Option API will break.Macroscope summarized 0fdb80d.