Conversation
The desktop's `applyUpdates()` flow on Windows spawns `hermes-setup.exe` (a Tauri app) detached, writes an update marker with the Tauri wrapper's PID, then waits 2.5s and quits. Tauri re-execs the actual updater logic into a different OS process; the lock's PID never matches that inner process's `std::process::id()`, so the self-PID adoption check in `apps/bootstrap-installer/src-tauri/src/update.rs:163-165` fails and the wrapper aborts before writing its own marker. Race window: the time between "desktop writes marker" and "desktop process actually exits." On a normal machine the desktop closes in <1s and the loop is rare. On slower machines (or with Defender real-time scans of the staged binary), the desktop takes 2-3+ seconds to exit — exactly long enough for the Tauri inner process to read the marker and bail. Verified locally: closing Hermes manually first makes the loop go away (empirically reproduced; v0.19.1 binary, which contains 160586f, still loops). This PR adds an opt-in PowerShell wrapper that automates "wait for the desktop to fully exit before running the real installer": - `apps/desktop/scripts/hermes-update-wrapper/hermes-update-wrapper.cmd` is the entry point the desktop spawns. - `apps/desktop/scripts/hermes-update-wrapper/hermes-update-wrapper.ps1` polls for `Hermes.exe` and Hermes-managed `node.exe` backend processes to exit (60s timeout, 1s poll, 1s grace), clears any stale `.hermes-update-in-progress` lock, then exec's the real installer (`hermes-setup-real.exe`) with the original args. - `apps/desktop/scripts/hermes-update-wrapper/README.md` documents the install / rollback / logs / long-term notes. Source changes: - `apps/desktop/electron/main.ts`: `resolveUpdaterBinary()` now prefers the wrapper when present. Behavior is unchanged when the wrapper is not staged (falls back to the original `hermes-setup.exe`). - `apps/desktop/electron/updater-process.ts`: adds `shell: true` to Windows spawn options so `.cmd` / `.bat` updater wrappers actually execute (CreateProcessW can't run a `.cmd` directly; `cmd.exe` is needed). The caller's `shell: false` opt-out is preserved. - `apps/desktop/electron/updater-process.test.ts`: updated for the new `shell: true` default plus a new test for the caller's explicit `shell: false` opt-out. The wrapper is opt-in: it is only used when present in `HERMES_HOME`. The upstream Tauri installer is the proper place to fix this in the long term, but since NousResearch#75556 was closed as "not planned" twice, this wrapper is the path of least resistance for users who can't wait. Refs NousResearch#75498, NousResearch#75556
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the detailed Windows reproduction and the reversible wrapper approach. The current in-app handoff still writes the spawned child PID into the update marker before the quit dwell (apps/desktop/electron/main.ts:2969-3000), so this is addressing a live seam.
Problems
apps/desktop/electron/updater-process.ts:37enablesshell: truefor the normalhermes-setup.exepath too. The desktop records that spawned child PID in the marker (main.ts:2988), while the Rust updater only adopts a marker matching its own PID (apps/bootstrap-installer/src-tauri/src/update.rs:161-165). Routing the executable throughcmd.exebreaks that identity contract and can recreate the self-lock for stock Windows installs.apps/desktop/scripts/hermes-update-wrapper/hermes-update-wrapper.ps1:131deletes any existing marker. Main treats a live foreign marker as the mutual-exclusion lock (update.rs:265-268), so this can permit an installer update to overlap a dashboard or terminal update.
Suggested changes
- Enable
shellonly for.cmd/.batlaunchers and cover both launcher types in tests. - Retain a live foreign marker; only remove a marker proven stale or owned by this handoff.
This is an automated hermes-sweeper review.
| // The caller can opt out by passing shell:false explicitly. safe because the | ||
| // spawn is detached + unref + the wrapper closes its own console window. | ||
| if (isWindows && !Object.prototype.hasOwnProperty.call(spawnOptions, 'shell')) { | ||
| spawnOptions.shell = true |
There was a problem hiding this comment.
This also routes the normal hermes-setup.exe path through cmd.exe. applyUpdates() records child.pid in the marker (main.ts:2988), but the Rust updater only adopts a marker whose PID is its own (update.rs:161-165); preserve direct spawning for .exe and enable shell only for .cmd/.bat wrappers.
| $lockFile = Join-Path $HERMES_HOME '.hermes-update-in-progress' | ||
| if (Test-Path -LiteralPath $lockFile) { | ||
| try { | ||
| Remove-Item -LiteralPath $lockFile -Force |
There was a problem hiding this comment.
Do not unconditionally remove this marker. It is main's cross-process update lock, and a live dashboard or terminal hermes update can own it without matching this script's Hermes/node process filter; deleting it permits concurrent mutation of the same checkout.
Two fixes per teknium1's review on PR NousResearch#75631: 1. `updater-process.ts`: only enable `shell: true` for `.cmd`/`.bat` updater paths on Windows. The previous patch enabled it unconditionally for ALL Windows spawns, which broke the default Tauri `.exe` path: `applyUpdates()` records `child.pid` in the update marker and the Rust updater's self-PID adoption check expects that PID to match its own. Routing the `.exe` through `cmd.exe` would have made `child.pid` the cmd.exe wrapper PID and broken adoption on stock Windows installs that don't use the wrapper. New test: `spawnUpdaterProcess enables shell:true for .cmd wrappers on Windows` and `... for .bat wrappers on Windows`. The original Windows test (`.exe`) now asserts `shell: true` is NOT added. 2. `hermes-update-wrapper.ps1`: only clear the lock file when it is ours or provably stale. The previous version deleted the lock unconditionally, which would clobber a live foreign lock held by a parallel dashboard or terminal `hermes update` and allow a second installer to race against the live one. New behavior: - lock PID in {our powershell.exe, parent cmd.exe} -> clear - lock PID is dead (no such process) -> clear - lock PID is alive and not us -> REFUSE, exit 4 - lock unreadable / no PID -> clear (stale) Updated `apps/desktop/scripts/hermes-update-wrapper/README.md` with the new lock semantics. Refs PR NousResearch#75631
|
Both review points are addressed in dee82ff (just pushed):
PS1 parses cleanly (929 tokens); updater-process tests 5/5 pass on the |
Follow-up to PR NousResearch#75631. The wrapper used to wait for both `Hermes.exe` AND Hermes-managed Python gateway backend `node.exe` processes to exit before exec'ing the real installer. On machines where the backends orphan after the desktop GUI quits (a separate Hermes bug — the desktop doesn't reap its child processes on quit), this caused the wrapper to hang for up to 60s and time out with exit 3. The Tauri installer handles orphaned backends itself; the only process that blocks the file replacement is the desktop GUI. The wrapper now waits only on `Hermes.exe`, with a 30s timeout (down from 60s — the new upper bound is only needed for very slow AV/Defender-scanned desktop shutdowns, not backend reaping). Typical end-to-end from "click Update" to "real installer launched" is now 1-5s instead of up to 60s. Other changes in the wrapper: - Polled at 500ms (was 1s) and logs one line per whole second so the user sees snappy progress without log spam - Removed the 1s "venv shim grace" period — it was a guess and is no longer needed since the backends are out of scope - Banner/header with process info, color-coded log levels ([info]/[ok]/[warn]/[error]), cleaner formatting throughout - Renamed `Get-HermesProcesses` to focused `Test-HermesRunning` / `Get-HermesPidList` helpers - Comments updated to explain WHY we only wait for Hermes.exe The lock-handling safety check (live foreign lock refuses with exit 4) and the source patch in `apps/desktop/electron/{main.ts,updater-process.ts, updater-process.test.ts}` from the previous commits are unchanged. Ref: NousResearch#75556, NousResearch#75631
Follow-up: drop the backend wait — wrapper now exits in 1-5s instead of up to 60s@teknium1 — new commit The change
The Tauri installer handles orphaned backends itself; the only process that blocks the file replacement is the desktop GUI. The wrapper now waits only on End-to-end timing from "click Update" to "real installer launched":
Why this is safe
Other wrapper improvements (cosmetic / DX)
Test results
Files
|
Two polish changes for the in-house update wrapper: 1. **hermes-update-wrapper.cmd** — add \-WindowStyle Hidden\ to the PowerShell invocation. The desktop already passes \windowsHide:true\ when it spawns the .cmd (see apps/desktop/electron/windows-child-options.ts), so the cmd window should already be hidden, but PowerShell was creating its own visible console on top of that. \-WindowStyle Hidden\ keeps PowerShell from popping a console too. Combined with the \windowsHide:true\ on the .cmd, the whole flow is silent and the user only sees the in-app "Updating Hermes..." modal. 2. **hermes-update-wrapper.ps1** — fix an edge case where the wrapper is invoked with no args (e.g. a manual smoke test). PowerShell 5.1's \Start-Process -ArgumentList\ rejects an empty array as "argument is null or empty", so building the \-ArgumentList\ only when args are present fixes the issue. Splatting the rest of the start args keeps the call clean. Both are non-functional changes for the in-app Update flow (the desktop always passes \--update --branch main\), but they make the wrapper behave correctly when invoked manually and stop the PowerShell window from showing during automated runs.
Fix Windows in-app Update loop with an opt-in PowerShell wrapper
Closes (community fix for) the regression tracked in
#75498 and
#75556 where the
in-app Update button on Windows gets stuck in a
"Another Hermes update is already running." loop. The
8c76fe19and160586ff8d7a069389a7d09849ef59434c6d48a1"fixes" that wereshipped in v0.19.1 are present in the source but the loop is reproducible
against a v0.19.1 binary on Windows 11 (verified locally; see empirical
section below).
What's in this PR
apps/desktop/electron/main.ts—resolveUpdaterBinary()now prefers ahermes-update-wrapper.cmdstaged inHERMES_HOMEwhen present.Behavior is unchanged when the wrapper is not staged.
apps/desktop/electron/updater-process.ts— addsshell: trueto theWindows spawn options so
.cmd/.batupdater wrappers actuallyexecute (CreateProcessW can't run a
.cmddirectly;cmd.exeis needed).apps/desktop/electron/updater-process.test.ts— updated for thenew
shell: truedefault plus a new test for the caller's explicitshell: falseopt-out.apps/desktop/scripts/hermes-update-wrapper/hermes-update-wrapper.cmd—the entry point the desktop spawns.
apps/desktop/scripts/hermes-update-wrapper/hermes-update-wrapper.ps1—the actual logic: wait for the desktop to fully exit, then exec the real
installer.
apps/desktop/scripts/hermes-update-wrapper/README.md— install /rollback / logs / long-term notes.
The wrapper is opt-in: it is only used when present in
HERMES_HOME.Without it, the desktop falls back to the original Tauri installer and
upstream behavior is preserved exactly.
Root cause (empirically verified)
The desktop's
applyUpdates()flow on Windows:spawnUpdaterProcess('hermes-setup.exe', ['--update', '--branch', 'main'], ...)child.pid(the Tauri wrapper's PID)UPDATE_HANDOFF_DWELL_MS)app.quit()hermes-setup.exeis a Tauri app. Tauri re-execs its actual updater logicinto a different OS process; the lock's PID does not match that inner
process's
std::process::id(), so the self-PID adoption check inapps/bootstrap-installer/src-tauri/src/update.rs:163-165fails and thewrapper aborts before it can write its own marker.
The race window is the time between "desktop writes the marker" and "desktop
process actually exits." On a normal machine the desktop closes in <1
second and the loop is rare. On slower machines (or with antivirus /
Defender real-time scans of the staged binary), the desktop takes 2-3+
seconds to exit — exactly long enough for the Tauri inner process to read
the marker and decide "this isn't me, bail."
The Tauri installer's self-PID check at
apps/bootstrap-installer/src-tauri/src/update.rs:163-165is theif pid == std::process::id() { return None; }guard. It only matcheswhen the desktop happens to die before the inner Tauri process reads the
lock. When the desktop stays alive longer than ~1s after writing the
lock, the inner process sees a foreign PID and aborts.
Empirical proof on Windows 11
Tested locally on Windows 11 Pro (Lenovo T490, Defender active). The
in-app Update button on a freshly-installed v0.19.1 binary reproduces the
loop 3/3 times.
Closing Hermes manually first removes the loop. The Tauri installer's
own
Updateflag was then run from a terminal after closing Hermes:The install completed cleanly in ~8 minutes (uv, venv rebuild 668 packages,
vite build, electron-builder, "bootstrap complete"). The key observation
is that the inner Tauri process never saw a foreign PID in the lock
because the desktop was already gone by the time the inner process started
reading.
This PR's wrapper reproduces the "Hermes fully exited before the inner
process reads" condition programmatically. Verified end-to-end:
The wrapper's "wait" is the entire fix. Everything else (clearing the
stale lock, the venv shim grace) is belt-and-suspenders.
Install (one-time setup)
Until the Tauri installer is updated to stage the wrapper, the user runs
the following once after
git pullandnpm run build && npm run pack:Rollback
Why not fix the installer directly?
The Tauri installer (Rust) is the proper place to fix this. Either:
the desktop's marker), or
or
We can't recompile the installer from this PR (no Rust toolchain in the
desktop's dev environment) and the upstream fix has been closed "not
planned" twice. This wrapper is the path of least resistance for users who
can't wait for an upstream fix.
When the installer is fixed, the wrapper becomes a no-op: the user can
delete the wrapper files and rename
hermes-setup-real.exeback tohermes-setup.exeto restore stock behavior.Why
shell: true?The wrapper is a
.cmd(PowerShell-launched). On Windows, Node'schild_process.spawn()usesCreateProcessWwhich only handles.exedirectly. A
.cmdrequirescmd.exeas the parent. Settingshell: truemakes Node route throughcmd.exe /d /s /c ...whichhandles
.cmdnatively. The caller'sshell: falseopt-out is preserved(checked with
hasOwnProperty).The Tauri installer's own
hermes-setup.exeworks fine withoutshell: truebecause it IS an.exe. The newshell: truedefault isonly relevant when the updater binary is a
.cmd(i.e. the wrapper is inplace).
Why this isn't invasive
resolveUpdaterBinary()is gated onIS_WINDOWSANDfileExists(wrapper); theshell: trueis gated on!hasOwnProperty(spawnOptions, 'shell'); the new test is purelyadditive).
resolveUpdaterBinary()returns theexact same
hermes-setup.exepath it always has.dependencies.