fix(desktop): give the update hand-off script its own console - a detached hidden powershell dies before -File runs
Live failure on the first real use of #82328 (2026-08-09): clicking Update closed the Desktop with "an updater will happen", then nothing. desktop.log showed `launched repo hand-off script`, but desktop-update-handoff.log was never created - PowerShell exited 0 without executing a single line. Root cause, isolated by spawning the exact production shape against a sandbox HERMES_HOME: `spawn('powershell', [..., '-File', script], { detached: true, stdio: 'ignore', windowsHide: true })` kills powershell.exe during console-subsystem init, before -File processing. Variant matrix: plain pipes -> runs; hide only -> runs; detached only -> runs; detached+hide -> exits 0, script never starts. Unit tests and foreground invocations can't see this class of bug. Fix: wrapHandoffForDetachedConsole() routes the invocation through `cmd /d /s /c start "" /min powershell ...` - `start` allocates the script its own minimized console and fully detaches it; the cmd wrapper exits immediately. Verified the wrapped form survives the full detached+hidden production spawn. Knock-on: child.pid is now the short-lived wrapper, not the script, so the Electron-side marker pre-write can't represent the script. The script now claims the update marker itself as step 0 (its own $PID, byte-exact "<pid>\n<ts>\n" via WriteAllText - Set-Content emits CRLF and would break the three readers' framing). The Electron pre-write is kept as a bridge for the spawn window: the script overwrites it, and if the script never starts the wrapper's dead pid reads as stale and self-deletes (no wedge). `hermes update` adopts the script's claim via update_lock.py's process-ancestry rule, unchanged. E2E in exact production shape (cmd start wrapper, detached, hidden, parent exits 1.5s after spawn) against a sandbox HERMES_HOME with a compiled fake hermes.exe: script ran, claimed marker with its own pid (fake observed "<script-pid>|<ts>|" LF-framed DURING the update), desktop-pid wait worked, update invoked with correct argv, marker removed on completion. vitest 13/13 (new wrapper-shape test), 3-project typecheck clean, eslint clean, PS 5.1 parse + windows-footguns clean.
This commit is contained in:
parent
26b3918dd9
commit
3b08a0f9b5
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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'
|
||||
])
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 "<pid>\n<ts>\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
|
||||
|
|
|
|||
Loading…
Reference in New Issue