Skip to content
Merged
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
126 changes: 126 additions & 0 deletions apps/desktop/electron/gateway-stop-before-update.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import assert from 'node:assert/strict'

import { test } from 'vitest'

import { GATEWAY_STOP_TIMEOUT_MS, startGatewaysAfterUpdateAbort, stopGatewayBeforeUpdate } from './gateway-stop-before-update'

const CLI = 'C:\\Users\\x\\hermes\\hermes-agent\\venv\\Scripts\\hermes.exe'
const HOME = 'C:\\Users\\x\\hermes'

function fakeExec(ok: boolean) {
return (_command: string, _args: string[], _options: unknown) => {
if (!ok) {
throw new Error('spawn ENOENT')
}

return Buffer.from('')
}
}

test('non-Windows is a no-op and never invokes the CLI', () => {
const calls: Array<[string, string[]]> = []

const ran = stopGatewayBeforeUpdate(CLI, HOME, {
isWindows: false,
existsSync: () => true,
execFileSync: fakeExec(true) as never,
spy: (c, a) => calls.push([c, a])
})

assert.equal(ran, false)
assert.deepEqual(calls, [])
})

test('Windows with missing CLI shim returns false and does not exec', () => {
const calls: Array<[string, string[]]> = []

const ran = stopGatewayBeforeUpdate(CLI, HOME, {
isWindows: true,
existsSync: () => false,
execFileSync: fakeExec(true) as never,
spy: (c, a) => calls.push([c, a])
})

assert.equal(ran, false)
assert.deepEqual(calls, [[CLI, ['gateway', 'stop', '--all']]])
})

test('Windows with live CLI invokes "gateway stop --all" and returns true', () => {
let seenCommand = ''
let seenArgs: string[] = []

const ran = stopGatewayBeforeUpdate(CLI, HOME, {
isWindows: true,
existsSync: () => true,
execFileSync: ((command: string, args: string[]) => {
seenCommand = command
seenArgs = args

return Buffer.from('')
}) as never
})

assert.equal(ran, true)
assert.equal(seenCommand, CLI)
assert.deepEqual(seenArgs, ['gateway', 'stop', '--all'])
})

test('Windows with failing CLI returns false (best-effort, never throws)', () => {
const ran = stopGatewayBeforeUpdate(CLI, HOME, {
isWindows: true,
existsSync: () => true,
execFileSync: fakeExec(false) as never
})

assert.equal(ran, false)
})

test('passes a generous timeout with hidden console (taskkill window suppression)', () => {
let seenOptions: unknown
stopGatewayBeforeUpdate(CLI, HOME, {
isWindows: true,
existsSync: () => true,
execFileSync: ((_c: string, _a: string[], options: unknown) => {
seenOptions = options

return Buffer.from('')
}) as never
})
assert.deepEqual(seenOptions, {
timeout: GATEWAY_STOP_TIMEOUT_MS,
windowsHide: true,
stdio: 'ignore',
encoding: 'utf8'
})
})

test('abort-path counterpart invokes "gateway start --all" (drain-semantics restore)', () => {
let seenArgs: string[] = []

const ran = startGatewaysAfterUpdateAbort(CLI, {
isWindows: true,
existsSync: () => true,
execFileSync: ((_c: string, args: string[]) => {
seenArgs = args

return Buffer.from('')
}) as never
})

assert.equal(ran, true)
assert.deepEqual(seenArgs, ['gateway', 'start', '--all'])
})

test('abort-path counterpart is a no-op off Windows', () => {
const calls: Array<[string, string[]]> = []

const ran = startGatewaysAfterUpdateAbort(CLI, {
isWindows: false,
existsSync: () => true,
execFileSync: fakeExec(true) as never,
spy: (c, a) => calls.push([c, a])
})

assert.equal(ran, false)
assert.deepEqual(calls, [])
})
104 changes: 104 additions & 0 deletions apps/desktop/electron/gateway-stop-before-update.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
/**
* gateway-stop-before-update.ts
*
* Windows-only helper for the update hand-off (#70337): stop every
* separately-running messaging gateway BEFORE the venv-shim lock poll.
*
* Why not just tree-kill gateway.pid's PID:
* - gateway.pid records the uv WORKER process, but the venv shim lock is
* held by its parent LAUNCHER (venv\Scripts\python.exe). taskkill /T from
* the worker PID does not reach parents, so the lock could survive.
* - a single gateway.pid read misses multi-profile setups entirely.
*
* So we delegate to `hermes gateway stop --all`: the CLI discovers every
* profile's gateway processes (launcher + worker) via find_gateway_pids,
* drains in-flight agents (planned-stop marker -> resume_pending), and
* force-kills survivors — the same logic `hermes update`'s
* _pause_windows_gateways_for_update relies on.
*
* Pure + dependency-injected so the launcher/worker and multi-profile
* behavior is assertable without booting Electron.
*/

import { execFileSync, type ExecFileSyncOptionsWithStringEncoding } from 'node:child_process'
import fs from 'node:fs'

export interface StopGatewayBeforeUpdateDeps {
/** Defaults to process.platform === 'win32'; injectable for tests. */
isWindows?: boolean
/** Defaults to fs.existsSync; injectable for tests. */
existsSync?: (p: string) => boolean
/** Defaults to execFileSync from node:child_process; injectable for tests. */
execFileSync?: (command: string, args: string[], options: ExecFileSyncOptionsWithStringEncoding) => Buffer | string
/** Observability hook for tests. */
spy?: (command: string, args: string[]) => void
}

export const GATEWAY_STOP_TIMEOUT_MS = 20_000

/**
* Best-effort stop of all-profile messaging gateways via the CLI.
* Never throws: a wedged/absent CLI must not abort the update hand-off
* (the shim-lock poll + the updater's venv-blocker scan still fail loudly
* if the venv stays held). Returns true when the CLI ran (or was invoked
* with the injected spy), false when skipped (non-Windows / missing CLI).
*/
export function stopGatewayBeforeUpdate(
hermesCliPath: string,
hermesHome: string,
deps: StopGatewayBeforeUpdateDeps = {}
): boolean {
return runGatewayLifecycleCommand(hermesCliPath, ['gateway', 'stop', '--all'], deps)
}

/**
* Drain-semantics counterpart (#76057 review): `gateway stop --all` before
* the lock gate takes gateways down even when the update later ABORTS
* (venv-blocked by a user terminal, probe failure, updater spawn failure).
* The updater's own pause machinery resumes what it pauses — the Desktop
* must mirror that on its abort paths, or a failed update strands every
* profile's gateway stopped. Best-effort, never throws.
*/
export function startGatewaysAfterUpdateAbort(
hermesCliPath: string,
deps: StopGatewayBeforeUpdateDeps = {}
): boolean {
return runGatewayLifecycleCommand(hermesCliPath, ['gateway', 'start', '--all'], deps)
}

function runGatewayLifecycleCommand(
hermesCliPath: string,
args: string[],
deps: StopGatewayBeforeUpdateDeps
): boolean {
const isWindows = deps.isWindows ?? process.platform === 'win32'

if (!isWindows) {
return false
}

const existsSync = deps.existsSync ?? fs.existsSync
const exec = deps.execFileSync ?? execFileSync

if (deps.spy) {
deps.spy(hermesCliPath, args)
}

if (!existsSync(hermesCliPath)) {
return false
}

try {
exec(hermesCliPath, args, {
timeout: GATEWAY_STOP_TIMEOUT_MS,
windowsHide: true,
stdio: 'ignore',
encoding: 'utf8'
})

return true
} catch {
// Best-effort (see header comment).
return false
}
}
88 changes: 88 additions & 0 deletions apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ import {
resolveGatewayFileBackend,
writeBufferToFile
} from './gateway-file-download'
import { startGatewaysAfterUpdateAbort, stopGatewayBeforeUpdate } from './gateway-stop-before-update'
import { probeGatewayWebSocket } from './gateway-ws-probe'
import { registerGitIpc } from './git-ipc'
import { clearStaleGitLocks } from './gitlock'
Expand Down Expand Up @@ -393,6 +394,7 @@ import {
scanVenvBlockers,
stopSafeVenvBlockers
} from './venv-blocker-scan'
import { isHermesOwnedVenvDaemon } from './venv-holder-select'
import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace'
import { createWakeIndicatorWindowController } from './wake-indicator-window'
import { enumerateWindowsFrontToBack, enumerationFailed, readWindowBelow } from './window-below'
Expand Down Expand Up @@ -3224,6 +3226,55 @@ function isShimLocked(shimPath) {
}
}

// Kill only Hermes-OWNED venv daemons (the memory plugin's hindsight daemon:
// exe under venv\Scripts AND cmdline referencing hindsight_api.main). The
// daemon is spawned DETACHED, so it outlives the backend tree-kill and keeps
// venv files mapped. External holders (a user terminal running `hermes`,
// unrelated scripts) are NOT killed — scanVenvBlockers reports them and the
// hand-off aborts, per existing design. Selection lives in the pure
// venv-holder-select module (ordinal path-prefix, no PowerShell -like
// wildcard hazards) so it's testable without Electron.
function killHermesOwnedVenvDaemons(updateRoot) {
if (!IS_WINDOWS) {
return
}

const scriptsDir = path.join(updateRoot, 'venv', 'Scripts')

let holders = []

try {
const out = execFileSync(
'powershell',
[
'-NoProfile',
'-Command',
'Get-CimInstance Win32_Process | Where-Object { $_.ExecutablePath -and $_.CommandLine } | Select-Object ProcessId, ExecutablePath, CommandLine | ConvertTo-Json -Compress'
],
hiddenWindowsChildOptions({ encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 15_000 })
)

const parsed = JSON.parse(String(out || '[]'))

holders = (Array.isArray(parsed) ? parsed : [parsed]).filter((p) =>
isHermesOwnedVenvDaemon(p?.ExecutablePath, p?.CommandLine, scriptsDir)
)
} catch {
// Best-effort: the venv-blocker scan downstream is the real backstop.
return
}

for (const holder of holders) {
const pid = Number(holder?.ProcessId)

if (Number.isInteger(pid) && pid > 0) {
rememberLog(`[updates] stopping Hermes-owned venv daemon (hindsight) PID ${pid} before hand-off`)
forceKillProcessTree(pid)
}
}
}


// Force-kill the entire process TREE rooted at each PID. Node's child.kill()
// only signals the direct child, so on Windows a backend `hermes.exe` that
// spawned its own grandchildren (a `hermes` REPL, a pty terminal session, the
Expand Down Expand Up @@ -3574,6 +3625,27 @@ async function releaseBackendLock(updateRoot, tag) {
stopAllPoolBackends
})

// Stop separately-running messaging gateways (all profiles) BEFORE the
// release gate. The gateway is launched by the gateway-launcher desktop
// plugin via /api/gateway/start and is NOT in backendConnectionState or
// backendPool, so the tree-kills above never see it — on Windows its
// launcher (venv\Scripts\python.exe) keeps the venv mandatory-locked and
// the 15s gate aborts the hand-off before the venv-blocker scan's
// pausable-gateway exemption ever gets a chance (#70337). Delegate to
// `hermes gateway stop --all`: the CLI discovers every profile's gateway
// (launcher + worker — gateway.pid records only the uv WORKER, and
// taskkill /T from the worker never reaches its parent), drains in-flight
// agents, and force-kills survivors. Best-effort; abort paths restore via
// startGatewaysAfterUpdateAbort. No-op off Windows.
stopGatewayBeforeUpdate(venvHermesShimPath(updateRoot), HERMES_HOME)

// Reap Hermes-OWNED venv daemons the tree-kill above cannot reach: the
// memory plugin's hindsight daemon is spawned DETACHED (it outlives the
// backend) yet runs off venv\Scripts\pythonw.exe, keeping venv files
// mapped past the backend teardown (#75477/#75478). Narrowly scoped
// (venv-holder-select) — external holders are never killed here.
killHermesOwnedVenvDaemons(updateRoot)

const shim = venvHermesShimPath(updateRoot)

const gate = await waitForBackendRelease(
Expand Down Expand Up @@ -3758,6 +3830,12 @@ async function applyUpdates(opts: { stopSafeBlockers?: boolean } = {}) {
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})

if (IS_WINDOWS) {
// The pre-gate `gateway stop --all` (#70337) took every profile's
// gateway down for an update that never happened — bring them back.
startGatewaysAfterUpdateAbort(venvHermesShimPath(updateRoot))
}

return { ok: false, error: message }
}

Expand Down Expand Up @@ -3807,6 +3885,9 @@ async function applyUpdates(opts: { stopSafeBlockers?: boolean } = {}) {
rememberLog(`[updates] venv-blocked: ${scanOutcome.result.processes.length} process(es) hold the install`)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})
// Restore the gateways the pre-gate stop took down (#70337 drain
// semantics): the update aborted, so nothing else will relaunch them.
startGatewaysAfterUpdateAbort(venvHermesShimPath(updateRoot))

return { ok: false, error: 'venv-blocked', message, blockers: scanOutcome.result.processes }
}
Expand All @@ -3817,6 +3898,8 @@ async function applyUpdates(opts: { stopSafeBlockers?: boolean } = {}) {
rememberLog(`[updates] venv-blocker probe failed: ${scanOutcome.error}`)
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})
// Same drain-semantics restore as the venv-blocked abort above.
startGatewaysAfterUpdateAbort(venvHermesShimPath(updateRoot))

return { ok: false, error: 'venv-probe-failed', message }
}
Expand Down Expand Up @@ -3945,6 +4028,11 @@ async function applyUpdates(opts: { stopSafeBlockers?: boolean } = {}) {
emitUpdateProgress({ stage: 'error', message, percent: null })
startHermes().catch(() => {})

if (IS_WINDOWS) {
// Same drain-semantics restore as the earlier abort paths (#70337).
startGatewaysAfterUpdateAbort(venvHermesShimPath(updateRoot))
}

return { ok: false, error: 'updater-spawn-failed', message }
}

Expand Down
Loading
Loading