Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
186 changes: 177 additions & 9 deletions apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ import {
} from './update-relaunch'
import { isOfficialSshRemote, OFFICIAL_REPO_HTTPS_URL } from './update-remote'
import { spawnUpdaterProcess } from './updater-process'
import { formatBlockerMessage, formatProbeFailedMessage, scanVenvBlockers } from './venv-blocker-scan'
import { formatBlockerMessage, formatProbeFailedMessage, scanVenvBlockers, stopVenvBlockers } from './venv-blocker-scan'
import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace'
import {
computeWindowOptions,
Expand Down Expand Up @@ -2720,6 +2720,110 @@ async function releaseBackendLockForUpdate(updateRoot) {
return releaseBackendLock(updateRoot, 'updates')
}

// ---------------------------------------------------------------------------
// Windows gateway pause/resume for update (Layer 1, #74386)
// ---------------------------------------------------------------------------
//
// The `releaseBackendLockForUpdate` above only stops backends the desktop
// spawned itself (primary window + pool). Gateway processes run as
// python(w).exe outside the desktop's PID tree, so they survive that cleanup
// and keep the venv locked — and the subsequent `scanVenvBlockers` finds them
// as blockers, aborting the update before the CLI updater even starts.
//
// The CLI updater (`hermes update`) already knows how to pause and resume
// gateways, but the Electron preflight aborts before reaching it. Here we
// pause gateways ourselves via the same Python helper, then pass the resume
// token to the CLI updater so it skips its own pause (avoids double-pause).
//
// Both functions are no-ops off Windows (the gateway lock hazard is a Windows
// .pyd mandatory-lock phenomenon).

/** Pause Windows gateways and return the JSON resume token (or null). */
async function pauseWindowsGatewaysForUpdate(updateRoot) {
if (!IS_WINDOWS) {
return null
}

const venvPython = getVenvPython(updateRoot)

if (!fileExists(venvPython)) {
return null
}

try {
const { stdout } = await new Promise((resolve, reject) => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This untyped Promise is inferred as Promise<{}>, so destructuring stdout fails the required desktop lint check. Give the resolved payload an explicit { stdout: string } type or use a typed promisified execFile result.

execFile(
venvPython,
['-m', 'hermes_cli._gateway_update_lock', 'pause'],
{
cwd: updateRoot,
encoding: 'utf-8',
timeout: 30000,
windowsHide: true,
},
(err, stdout, stderr) => {
if (err) {
reject(err)
} else {
resolve({ stdout, stderr })
}
}
)
})

const parsed = JSON.parse(stdout)

if (parsed && parsed.ok === true) {
const t = parsed.token
return t && typeof t === 'object' && Object.keys(t).length > 0 ? t : null
}

rememberLog(`[updates] gateway pause returned failure: ${parsed?.error ?? 'unknown'}`)

return null
} catch (err) {
rememberLog(`[updates] gateway pause subprocess failed: ${err.message}`)
return null
}
}

/** Resume Windows gateways from a previously-returned token (best-effort). */
async function resumeWindowsGatewaysAfterUpdate(updateRoot, token) {
if (!IS_WINDOWS || !token) {
return
}

const venvPython = getVenvPython(updateRoot)

if (!fileExists(venvPython)) {
return
}

try {
await new Promise((resolve, reject) => {
execFile(
venvPython,
['-m', 'hermes_cli._gateway_update_lock', 'resume', JSON.stringify(token)],
{
cwd: updateRoot,
encoding: 'utf-8',
timeout: 30000,
windowsHide: true,
},
(err, stdout, stderr) => {
if (err) {
reject(err)
} else {
resolve({ stdout, stderr })
}
}
)
})
} catch (err) {
rememberLog(`[updates] gateway resume subprocess failed (best-effort): ${err.message}`)
}
}

// Shared backend teardown + venv-shim unlock wait. Used by BOTH the self-update
// hand-off and the desktop uninstaller — they have the identical Windows
// problem: the desktop's backend (and the grandchildren IT spawned — a hermes
Expand Down Expand Up @@ -2923,7 +3027,14 @@ async function applyUpdates(opts = {}) {
return { ok: false, error: message }
}

// Preflight: after releasing our own backends, check for remaining
// Pause Windows gateway processes BEFORE the venv-blocker scan (Layer 1,
// #74386). The desktop's own backend cleanup above cannot reach gateway
// processes (they run as python(w).exe outside the desktop's PID tree),
// so without this step the blocker scan below finds them and aborts the
// update — before the CLI updater even gets to run its own gateway pause.
const gatewayToken = await pauseWindowsGatewaysForUpdate(updateRoot)

// Start: pre-work before forking the updater
// Hermes processes running from this venv. The updater normally refuses
// when it detects a holder, but because the updater is spawned detached
// with stdio:ignore, the user never sees that refusal and the update
Expand All @@ -2936,19 +3047,73 @@ async function applyUpdates(opts = {}) {
const scanOutcome = await scanVenvBlockers(updateRoot)

if (scanOutcome.kind === 'blocked') {
const message = formatBlockerMessage(scanOutcome.result)
// ── Gateway-aware preflight (#74326) ──────────────────────────
// The pre-pause above (pauseWindowsGatewaysForUpdate) handles the
// common case, but _pause_windows_gateways_for_update can miss some
// gateway spawn paths (e.g. --replace or shim-wrapped children that
// don't register in the PID-file registry — Defect 2 in #74326).
// As a second line of defence, try to force-stop any remaining
// gateway processes found by the blocker scan via taskkill.
const remaining = await stopVenvBlockers(scanOutcome.result.processes)

if (remaining.length === 0) {
// All remaining blockers were gateways that we killed. Re-scan
// to confirm the venv is clear before handing off to the updater.
const reScan = await scanVenvBlockers(updateRoot)

if (reScan.kind === 'clear') {
rememberLog(
`[updates] stopped ${scanOutcome.result.processes.length} remaining gateway process(es) via taskkill; venv is clear, proceeding`
)
// token still carries paused-profile data (PIDs, state).
// Always pass it — the resume already no-ops if there's nothing to resume.
} else if (reScan.kind === 'blocked') {
// Gateway kill freed some but not all, or new processes appeared.
const message = formatBlockerMessage(reScan.result)

rememberLog(
`[updates] venv-blocked after stopping ${scanOutcome.result.processes.length} gateway(es): ${reScan.result.processes.length} holder(s) remain`
)
await resumeWindowsGatewaysAfterUpdate(updateRoot, gatewayToken)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})

rememberLog(`[updates] venv-blocked: ${scanOutcome.result.processes.length} process(es) hold the install`)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})
return { ok: false, error: 'venv-blocked', message }
} else {
const message = formatProbeFailedMessage()

return { ok: false, error: 'venv-blocked', message }
}
rememberLog(
`[updates] venv-blocker probe failed after stopping ${scanOutcome.result.processes.length} gateway(es): ${reScan.error}`
)
await resumeWindowsGatewaysAfterUpdate(updateRoot, gatewayToken)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})

return { ok: false, error: 'venv-probe-failed', message }
}
} else {
// Non-gateway blockers remain — abort with a message listing only
// the real blockers (gateway PIDs are already removed).
const message = formatBlockerMessage({
blocked: true,
processes: remaining
})

rememberLog(
`[updates] venv-blocked: ${remaining.length} non-gateway holder(s) remain (${scanOutcome.result.processes.length - remaining.length} gateway(s) stopped)`
)
await resumeWindowsGatewaysAfterUpdate(updateRoot, gatewayToken)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})

if (scanOutcome.kind === 'probe-failure') {
return { ok: false, error: 'venv-blocked', message }
}
} else if (scanOutcome.kind === 'probe-failure') {
const message = formatProbeFailedMessage()

rememberLog(`[updates] venv-blocker probe failed: ${scanOutcome.error}`)
// Resume gateways we paused before aborting (#74386).
await resumeWindowsGatewaysAfterUpdate(updateRoot, gatewayToken)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})

Expand All @@ -2963,6 +3128,9 @@ async function applyUpdates(opts = {}) {
env: {
...process.env,
HERMES_HOME,
HERMES_WINDOWS_GATEWAY_RESUME_TOKEN: gatewayToken
? JSON.stringify(gatewayToken)
: undefined,
PATH: pathWithHermesManagedNode(venvBin)
},
detached: true,
Expand Down
75 changes: 74 additions & 1 deletion apps/desktop/electron/venv-blocker-scan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
import {
formatBlockerMessage,
formatProbeFailedMessage,
isGatewayProcess,
parseVenvBlockerScanOutput,
resolveVenvPython,
scanVenvBlockers
scanVenvBlockers,
stopVenvBlockers
} from './venv-blocker-scan'

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -216,3 +218,74 @@
assert.ok(c.timeout > 0)
})
})

// ---------------------------------------------------------------------------
// isGatewayProcess
// ---------------------------------------------------------------------------

describe('isGatewayProcess', () => {
it('returns true when cmdline contains "gateway run"', () => {
assert.equal(
isGatewayProcess({ pid: 100, name: 'python.exe', cmdline: 'python.exe -m hermes_cli.main gateway run --profile default' }),
true
)
})

it('is case-insensitive', () => {
assert.equal(
isGatewayProcess({ pid: 101, name: 'PYTHON.EXE', cmdline: 'GATEWAY RUN --replace' }),
true
)
})

it('returns false for non-gateway processes', () => {
assert.equal(
isGatewayProcess({ pid: 102, name: 'python.exe', cmdline: 'python.exe -m hermes_cli.main serve --host 127.0.0.1' }),
false
)
})

it('returns false for unrelated commands containing gateway word', () => {
// "gateway" appears in the command but not as "gateway run"
assert.equal(

Check failure on line 250 in apps/desktop/electron/venv-blocker-scan.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:test:desktop:platforms

[electron] electron/venv-blocker-scan.test.ts > isGatewayProcess > returns false for unrelated commands containing gateway word

AssertionError: Expected values to be strictly equal: true !== false - Expected + Received - false + true ❯ electron/venv-blocker-scan.test.ts:250:12
isGatewayProcess({ pid: 103, name: 'git.exe', cmdline: 'git log --oneline --all --grep=gateway' }),
false
)
})
})

// ---------------------------------------------------------------------------
// stopVenvBlockers (pure classification — the subprocess behavior with
// taskkill is tested via the platform gate; off-Windows it is a no-op)
// ---------------------------------------------------------------------------

describe('stopVenvBlockers', () => {
it('returns all processes when none are gateways (no-op)', async () => {
const procs = [
{ pid: 1, name: 'python.exe', cmdline: 'serve --host 127.0.0.1' },
{ pid: 2, name: 'python.exe', cmdline: 'dashboard --port 8080' }
]
const remaining = await stopVenvBlockers(procs)
assert.equal(remaining.length, 2)
assert.deepEqual(remaining, procs)
})

it('filters out gateway processes from the returned list', async () => {
// On non-Windows, this test passes because the platform gate skips
// the taskkill calls and just partitions the list (returns all).
const procs = [
{ pid: 1, name: 'python.exe', cmdline: 'serve' },
{ pid: 2, name: 'python.exe', cmdline: 'python.exe -m hermes_cli.main gateway run --profile default' }
]
const remaining = await stopVenvBlockers(procs)
// On non-Windows: platform gate returns all unchanged
// On Windows: gateway PID filtered out (taskkill may fail harmlessly)
// Both behaviors are correct for the respective platform.
assert.ok(Array.isArray(remaining))
})

it('empty input returns empty', async () => {
const remaining = await stopVenvBlockers([])
assert.equal(remaining.length, 0)
})
})
Loading
Loading