diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 84bfeedd632cd..7ac20872631b5 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -206,7 +206,8 @@ import { resolveStagedUpdaterBinary, resolveUpdateScriptHandoff, spawnUpdaterProcess, - stagedUpdaterSupportsPrewrittenMarker + stagedUpdaterSupportsPrewrittenMarker, + wrapHandoffForDetachedConsole } from './updater-process' import { formatBlockerMessage, formatProbeFailedMessage, scanVenvBlockers } from './venv-blocker-scan' import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace' @@ -2997,8 +2998,14 @@ async function applyUpdates(opts = {}) { let child if (scriptHandoff) { - const scriptArgs = [ - ...scriptHandoff.args, + // A bare detached+hidden powershell spawn silently dies before -File + // processing (console-subsystem init failure — see + // wrapHandoffForDetachedConsole). Route through `cmd start` so the + // script gets its own minimized console and survives our exit. The + // wrapper cmd.exe exits immediately, so child.pid is NOT the script's + // pid — the script claims the update marker itself with its own $PID + // as its first action, and a relaunched Desktop parks on that. + const wrapped = wrapHandoffForDetachedConsole(scriptHandoff, [ '-InstallRoot', updateRoot, '-Branch', @@ -3007,9 +3014,9 @@ async function applyUpdates(opts = {}) { String(process.pid), '-RelaunchExe', process.execPath - ] + ]) - child = spawnUpdaterProcess(scriptHandoff.command, scriptArgs, { + child = spawnUpdaterProcess(wrapped.command, wrapped.args, { cwd: HERMES_HOME, env: { ...process.env, @@ -3020,11 +3027,13 @@ async function applyUpdates(opts = {}) { stdio: 'ignore' }) - // The script's own pid owns the marker. Unlike the stale-binary path - // there is NO adoption hazard: hermes_cli/update_lock.py accepts a live - // marker held by a process ANCESTOR (the script is the `hermes update` - // child's parent), so the pre-write is always safe here — no - // stagedUpdaterSupportsPrewrittenMarker() mtime heuristics needed. + // Bridge marker: child.pid is the short-lived cmd.exe WRAPPER, not the + // script (see wrapHandoffForDetachedConsole). Write it anyway to cover + // the first moments of the hand-off — the script's step 0 overwrites it + // with its own live $PID, and if the script never starts the wrapper's + // dead pid makes the marker read as stale and self-delete (no wedge). + // The `hermes update` child adopts the SCRIPT's claim via + // update_lock.py's process-ancestry rule; no mtime heuristics needed. if (Number.isInteger(child.pid)) { writeUpdateMarker(HERMES_HOME, child.pid) } diff --git a/apps/desktop/electron/updater-process.test.ts b/apps/desktop/electron/updater-process.test.ts index d6bda4e91a9ea..06e6ffc6cf8ec 100644 --- a/apps/desktop/electron/updater-process.test.ts +++ b/apps/desktop/electron/updater-process.test.ts @@ -9,7 +9,8 @@ import { resolveStagedUpdaterBinary, resolveUpdateScriptHandoff, spawnUpdaterProcess, - stagedUpdaterSupportsPrewrittenMarker + stagedUpdaterSupportsPrewrittenMarker, + wrapHandoffForDetachedConsole } from './updater-process' const DAY_MS = 24 * 60 * 60 * 1000 @@ -199,3 +200,22 @@ test('resolveUpdateScriptHandoff is Windows-only (POSIX updates in place)', () = assert.equal(handoff, null) }) + +test('wrapHandoffForDetachedConsole routes through cmd start with own console', () => { + const root = String.raw`C:\Users\hermes\AppData\Local\hermes\hermes-agent` + const expected = path.join(root, 'scripts', 'desktop-update.ps1') + const handoff = resolveUpdateScriptHandoff(root, { + isWindows: true, + fileExists: candidate => candidate === expected + }) + + assert.ok(handoff) + const wrapped = wrapHandoffForDetachedConsole(handoff, ['-InstallRoot', root, '-Branch', 'main']) + + assert.equal(wrapped.command, 'cmd.exe') + assert.deepEqual(wrapped.args, [ + '/d', '/s', '/c', 'start', '', '/min', + 'powershell', '-NoProfile', '-ExecutionPolicy', 'Bypass', '-File', expected, + '-InstallRoot', root, '-Branch', 'main' + ]) +}) diff --git a/apps/desktop/electron/updater-process.ts b/apps/desktop/electron/updater-process.ts index 66faf9316b48f..66a279a200646 100644 --- a/apps/desktop/electron/updater-process.ts +++ b/apps/desktop/electron/updater-process.ts @@ -61,6 +61,36 @@ export function resolveUpdateScriptHandoff( } } +/** + * Wrap a PowerShell hand-off invocation so it survives a detached, hidden + * spawn from Electron. + * + * Verified empirically (2026-08-09, Windows 11): `spawn('powershell', [..., + * '-File', script], { detached: true, stdio: 'ignore', windowsHide: true })` + * exits 0 WITHOUT executing a single line of the script. powershell.exe is a + * console-subsystem binary; detached+windowsHide gives it no console to + * attach to, and Windows PowerShell 5.1 dies during console init before + * -File processing (the same class of failure as #54220's conhost work, on + * the launch side). The same spawn with a visible console, or non-detached, + * runs fine — so unit tests and foreground use hide the bug. + * + * `cmd /c start "" /min powershell ...` was the variant that survived the + * full detached+hidden production shape in testing: `start` allocates the + * child its own (minimized) console and fully detaches it from cmd.exe, + * which exits immediately. The spawned pid is therefore the WRAPPER's — + * callers must not use it as a marker owner (the script claims the marker + * itself with its own $PID). + */ +export function wrapHandoffForDetachedConsole(handoff: UpdateScriptHandoff, extraArgs: string[]): { + command: string + args: string[] +} { + return { + command: 'cmd.exe', + args: ['/d', '/s', '/c', 'start', '', '/min', handoff.command, ...handoff.args, ...extraArgs] + } +} + export interface ResolveStagedUpdaterBinaryDeps { isWindows?: boolean fileExists?: (candidate: string) => boolean diff --git a/scripts/desktop-update.ps1 b/scripts/desktop-update.ps1 index fe90707975ec4..c7f3951f94e17 100644 --- a/scripts/desktop-update.ps1 +++ b/scripts/desktop-update.ps1 @@ -66,6 +66,25 @@ function Remove-Marker { try { Write-HandoffLog "hand-off start: root=$InstallRoot branch=$Branch desktopPid=$DesktopPid pid=$PID" + # -- 0. Claim the update marker with OUR pid --------------------------- + # The Desktop spawns us through a `cmd start` wrapper (a console-subsystem + # child needs its own console to survive the parent's exit), so the pid + # the Desktop observed is the short-lived cmd.exe -- useless as a marker + # owner. We claim it ourselves as step 0: `hermes update` (our child) + # adopts the claim via update_lock.py's process-ancestry rule, and a + # relaunched Desktop parks on it (update-marker.ts) instead of spawning a + # backend into the update window. Unix seconds + pid, same format as + # every other writer. + try { + $epoch = [int][double]::Parse((Get-Date -UFormat %s), [System.Globalization.CultureInfo]::InvariantCulture) + # WriteAllText for byte-exact LF framing: Set-Content/Add-Content emit + # CRLF, and the marker contract (Rust/TS/Python readers) is "\n\n". + [System.IO.File]::WriteAllText($MarkerPath, "$PID`n$epoch`n") + Write-HandoffLog "claimed update marker (pid $PID)" + } catch { + Write-HandoffLog "WARNING: could not write update marker: $($_.Exception.Message)" + } + # -- 1. Wait for the Desktop to actually exit ------------------------- # The Desktop quits right after spawning us, but Electron teardown is # asynchronous. Bounded wait; a Desktop that never exits is a bug we