fix(desktop-update): complete Windows Desktop updates instead of self-locking the shim - #86326
Conversation
…on, not the hermes.exe shim `uv pip install -e .` has to replace the console-script shims, so _quarantine_running_hermes_exe must first rename the running hermes.exe out of the way. That rename fails whenever any child process spawned from that hermes.exe is still alive: on Windows a child inherits a handle on the parent image. It is the inherited handle, not the trampoline, that pins the file -- killing the child makes the identical rename succeed, and the shim flavour (uv trampoline vs distlib launcher) makes no difference. The updater spawns such children itself (npx cache warm, memory-provider refresh -- hindsight-api runs as a daemon with --idle-timeout 300 and outlives the step that started it), so this presents as a race rather than a hard failure: the same hand-off succeeds on one run and dies on the next. Step 2's shim-unlock preflight cannot catch it, because the shim genuinely is unlocked at that moment; the pinning child appears later, during the update. When the rename loses that race, _schedule_replace_on_reboot is the last resort -- and MOVEFILE_DELAY_UNTIL_REBOOT writes to HKLM, so it needs elevation. A Desktop-driven update is not elevated, so it returns ERROR_ACCESS_DENIED, `uv pip install -e .` exits 2, and the ZIP fallback repeats the identical sequence. The desktop build stage is then never reached while the pre-build clean has already removed apps/desktop/release, leaving an install whose Start Menu shortcut points at a Hermes.exe that no longer exists. Running the same code as `python.exe -m hermes_cli.main update` puts the inherited handles on python.exe, which uv never has to replace. posix.sh is deliberately untouched: unlinking a running executable is legal there, so the equivalent call is harmless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Source-level regression for the Windows Desktop update self-lock: assert every Invoke-HermesStep call in scripts/desktop-update/windows.ps1 drives $pythonExe via `python.exe -m hermes_cli.main`, never the hermes.exe shim. Driving the update through the shim keeps hermes.exe mapped as a running image, so uv's final `pip install -e .` shim rewrite fails with os error 32 and the update can never complete. Runs on Linux CI (no PowerShell execution needed). Co-authored-by: Sascha Haase <sascha.haase@textiletsg.com> Co-authored-by: adamcap926 <adamcap926@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 7 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Comment |
alt-glitch
added
type/bug
૮ >ﻌ< ა ci reviewrunning on 944616e — chore: map contributor email for the salvaged Windows update waiting for more jobs to start…
|
2 participants
On Windows, the in-app Desktop updater's hand-off (
scripts/desktop-update/windows.ps1) driveshermes updatethrough the venv'shermes.execonsole-script launcher. That launcher keepshermes.exemapped as its running image and spawnspython.exeas a child, so when the update finishes withuv pip install -e .-- which rewrites the console-script shims, including that samehermes.exe-- Windows refuses to replace a file mapped as a running image (os error 32). The rename fallback then defers to next reboot viaMOVEFILE_DELAY_UNTIL_REBOOT, which needs elevation a Desktop-driven (non-elevated) update doesn't have, souv pip install -e .exits non-zero, the ZIP fallback repeats the identical sequence, the desktop build stage is never reached, and the pre-build clean has already removedapps/desktop/release-- leaving an install whose Start Menu shortcut points at aHermes.exethat no longer exists (the "my Desktop app disappeared" reports).This runs the update as
python.exe -m hermes_cli.main updateinstead, at all threeInvoke-HermesStepcall sites (the update, its retry, and the desktop rebuild). The inherited image handle then lands onpython.exe, which uv never has to replace, so the shim is an ordinary unlocked file when uv rewrites it.scripts/desktop-update/posix.shis intentionally untouched: unlinking a running executable is legal there.Detection table
hermes updatefrom Desktop fails every run withos error 32onScripts/hermes.exehermes.exelauncher, which holds the shim mappedpython.exe; shim is unlocked when uv rewrites ithermes.exewith no replacement writtenhindsight-apidaemon) inheriting the shim handlepython.exehandle, not the shimWorked example
Jango's thread: the update "doesn't finish," produces no completion, and the Desktop app disappears.
logs/desktop-update-handoff.logshowshermes update exit code: 0then1on runs 18 min apart on one box -- the only variable being whether a long-lived child was alive during the quarantine. Driving the same code aspython.exe -m hermes_cli.main updatecompleted with exit 0 on the first attempt after three consecutive shim-driven failures on that install, and the subsequentpython.exe -m hermes_cli.main desktop --force-build --build-onlyrebuiltrelease/win-unpacked/Hermes.execleanly.Tests
Adds
tests/test_desktop_update_windows_python_handoff.py-- a source-level guard (Linux CI can't execute the PowerShell hand-off) asserting everyInvoke-HermesStepcall drives$pythonExe, the update/rebuild use-m hermes_cli.main, andInvoke-HermesStep $hermesExenever reappears. This fails on currentmain(three shim call sites) and passes here.Supersedes #85679 and #86101. Both authors landed on the same core insight (run the update off the shim, on
python.exe); this takes #85679's surgical hand-off change as the base and keeps the footprint on the update path minimal. #86101's separate detached-grandchild trampoline (which also covers CLI-typedhermes update) is deliberately not included here -- it's a larger, independent change to the core update path and can land on its own merits if wanted.Co-authored-by: Sascha Haase sascha.haase@textiletsg.com
Co-authored-by: adamcap926 adamcap926@users.noreply.github.com