mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(desktop): ignore stale backend exits
This commit is contained in:
parent
f0ff8d5097
commit
783003179a
3 changed files with 213 additions and 26 deletions
71
apps/desktop/electron/backend-connection-state.test.ts
Normal file
71
apps/desktop/electron/backend-connection-state.test.ts
Normal file
|
|
@ -0,0 +1,71 @@
|
|||
import assert from 'node:assert/strict'
|
||||
import test from 'node:test'
|
||||
|
||||
// Node 22's direct TypeScript runner requires the extension on Windows.
|
||||
// @ts-expect-error The Electron tsconfig intentionally disables TS extension imports.
|
||||
import { createBackendConnectionState } from './backend-connection-state.ts'
|
||||
|
||||
type FakeProcess = { id: string }
|
||||
|
||||
test('a stale backend exit cannot clear a newer connection attempt', () => {
|
||||
const state = createBackendConnectionState<FakeProcess, string>()
|
||||
const oldAttempt = state.startAttempt()
|
||||
const oldPromise = Promise.resolve('old')
|
||||
|
||||
state.setPromise(oldAttempt, oldPromise)
|
||||
const oldOwner = state.attachProcess(oldAttempt, { id: 'old' })
|
||||
assert.ok(oldOwner)
|
||||
|
||||
state.invalidate()
|
||||
|
||||
const newAttempt = state.startAttempt()
|
||||
const newPromise = Promise.resolve('new')
|
||||
const newProcess = { id: 'new' }
|
||||
|
||||
state.setPromise(newAttempt, newPromise)
|
||||
assert.ok(state.attachProcess(newAttempt, newProcess))
|
||||
|
||||
assert.equal(state.clearForCurrentProcess(oldOwner), false)
|
||||
assert.equal(state.getProcess(), newProcess)
|
||||
assert.equal(state.getPromise(), newPromise)
|
||||
})
|
||||
|
||||
test('the current backend exit clears its process and connection promise', () => {
|
||||
const state = createBackendConnectionState<FakeProcess, string>()
|
||||
const attempt = state.startAttempt()
|
||||
|
||||
state.setPromise(attempt, Promise.resolve('current'))
|
||||
const owner = state.attachProcess(attempt, { id: 'current' })
|
||||
assert.ok(owner)
|
||||
|
||||
assert.equal(state.clearForCurrentProcess(owner), true)
|
||||
assert.equal(state.clearPromiseForAttempt(attempt), true)
|
||||
assert.equal(state.getProcess(), null)
|
||||
assert.equal(state.getPromise(), null)
|
||||
})
|
||||
|
||||
test('a stale rejected attempt cannot clear a newer connection promise', () => {
|
||||
const state = createBackendConnectionState<FakeProcess, string>()
|
||||
const oldAttempt = state.startAttempt()
|
||||
|
||||
state.setPromise(oldAttempt, Promise.resolve('old'))
|
||||
state.invalidate()
|
||||
|
||||
const newAttempt = state.startAttempt()
|
||||
const newPromise = Promise.resolve('new')
|
||||
|
||||
state.setPromise(newAttempt, newPromise)
|
||||
|
||||
assert.equal(state.clearPromiseForAttempt(oldAttempt), false)
|
||||
assert.equal(state.getPromise(), newPromise)
|
||||
})
|
||||
|
||||
test('an invalidated attempt cannot attach a late-spawned process', () => {
|
||||
const state = createBackendConnectionState<FakeProcess, string>()
|
||||
const staleAttempt = state.startAttempt()
|
||||
|
||||
state.invalidate()
|
||||
|
||||
assert.equal(state.attachProcess(staleAttempt, { id: 'late' }), null)
|
||||
assert.equal(state.getProcess(), null)
|
||||
})
|
||||
84
apps/desktop/electron/backend-connection-state.ts
Normal file
84
apps/desktop/electron/backend-connection-state.ts
Normal file
|
|
@ -0,0 +1,84 @@
|
|||
export type BackendConnectionAttempt<TConnection> = {
|
||||
generation: number
|
||||
promise: Promise<TConnection> | null
|
||||
}
|
||||
|
||||
export type BackendProcessOwner<TProcess> = {
|
||||
generation: number
|
||||
process: TProcess
|
||||
}
|
||||
|
||||
export function createBackendConnectionState<TProcess, TConnection>() {
|
||||
let generation = 0
|
||||
let process: TProcess | null = null
|
||||
let promise: Promise<TConnection> | null = null
|
||||
|
||||
return {
|
||||
startAttempt(): BackendConnectionAttempt<TConnection> {
|
||||
return { generation, promise: null }
|
||||
},
|
||||
|
||||
setPromise(attempt: BackendConnectionAttempt<TConnection>, nextPromise: Promise<TConnection>): boolean {
|
||||
if (attempt.generation !== generation) {
|
||||
return false
|
||||
}
|
||||
|
||||
attempt.promise = nextPromise
|
||||
promise = nextPromise
|
||||
|
||||
return true
|
||||
},
|
||||
|
||||
attachProcess(
|
||||
attempt: BackendConnectionAttempt<TConnection>,
|
||||
nextProcess: TProcess
|
||||
): BackendProcessOwner<TProcess> | null {
|
||||
if (attempt.generation !== generation) {
|
||||
return null
|
||||
}
|
||||
|
||||
process = nextProcess
|
||||
|
||||
return { generation, process: nextProcess }
|
||||
},
|
||||
|
||||
clearForCurrentProcess(owner: BackendProcessOwner<TProcess>): boolean {
|
||||
if (owner.generation !== generation || owner.process !== process) {
|
||||
return false
|
||||
}
|
||||
|
||||
process = null
|
||||
promise = null
|
||||
|
||||
return true
|
||||
},
|
||||
|
||||
clearPromiseForAttempt(attempt: BackendConnectionAttempt<TConnection>): boolean {
|
||||
if (attempt.generation !== generation || (promise !== null && attempt.promise !== promise)) {
|
||||
return false
|
||||
}
|
||||
|
||||
promise = null
|
||||
|
||||
return true
|
||||
},
|
||||
|
||||
getProcess(): TProcess | null {
|
||||
return process
|
||||
},
|
||||
|
||||
getPromise(): Promise<TConnection> | null {
|
||||
return promise
|
||||
},
|
||||
|
||||
invalidate(): TProcess | null {
|
||||
const currentProcess = process
|
||||
|
||||
generation += 1
|
||||
process = null
|
||||
promise = null
|
||||
|
||||
return currentProcess
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -30,6 +30,7 @@ import nodePty from 'node-pty'
|
|||
|
||||
import { stopBackendChild as stopBackendChildImpl } from './backend-child'
|
||||
import { dashboardFallbackArgs, sourceDeclaresServe } from './backend-command'
|
||||
import { createBackendConnectionState } from './backend-connection-state'
|
||||
import { buildDesktopBackendEnv, normalizeHermesHomeRoot } from './backend-env'
|
||||
import { canImportHermesCli, verifyHermesCli } from './backend-probes'
|
||||
import { waitForDashboardPortAnnouncement } from './backend-ready'
|
||||
|
|
@ -807,14 +808,13 @@ function registerMediaProtocol() {
|
|||
}
|
||||
|
||||
let mainWindow = null
|
||||
let hermesProcess = null
|
||||
let connectionPromise = null
|
||||
const backendConnectionState = createBackendConnectionState<ReturnType<typeof spawn>, any>()
|
||||
// True while connection-config:apply soft-rehomes the primary — suppresses the
|
||||
// backend-exit toast so an intentional kill doesn't look like a crash.
|
||||
let softRehomeInProgress = false
|
||||
// Additional per-profile backends, keyed by profile name. The PRIMARY backend
|
||||
// (the desktop's launch profile) stays managed by hermesProcess +
|
||||
// connectionPromise + startHermes(); this pool only holds EXTRA profile
|
||||
// (the desktop's launch profile) stays managed by backendConnectionState +
|
||||
// startHermes(); this pool only holds EXTRA profile
|
||||
// backends spawned lazily when a session belongs to a different profile. A user
|
||||
// with no named profiles never populates this map, so their experience is
|
||||
// byte-for-byte the single-backend behavior.
|
||||
|
|
@ -2335,6 +2335,7 @@ async function releaseBackendLock(updateRoot, tag) {
|
|||
|
||||
// Collect every backend PID the desktop owns: primary window backend + pool.
|
||||
const pids = []
|
||||
const hermesProcess = backendConnectionState.getProcess()
|
||||
|
||||
if (hermesProcess && Number.isInteger(hermesProcess.pid)) {
|
||||
pids.push(hermesProcess.pid)
|
||||
|
|
@ -2376,8 +2377,10 @@ async function releaseBackendLock(updateRoot, tag) {
|
|||
// instead of trusting the initial sweep.
|
||||
const stragglers = []
|
||||
|
||||
if (hermesProcess && Number.isInteger(hermesProcess.pid)) {
|
||||
stragglers.push(hermesProcess.pid)
|
||||
const currentHermesProcess = backendConnectionState.getProcess()
|
||||
|
||||
if (currentHermesProcess && Number.isInteger(currentHermesProcess.pid)) {
|
||||
stragglers.push(currentHermesProcess.pid)
|
||||
}
|
||||
|
||||
for (const entry of backendPool.values()) {
|
||||
|
|
@ -2732,6 +2735,7 @@ async function applyUpdatesPosixInApp(opts: any) {
|
|||
// the update reaper. _kill_stale_dashboard_processes accepts a comma-separated
|
||||
// list (a single int still parses for back-compat).
|
||||
const desktopChildPids = []
|
||||
const hermesProcess = backendConnectionState.getProcess()
|
||||
|
||||
if (hermesProcess && Number.isInteger(hermesProcess.pid)) {
|
||||
desktopChildPids.push(hermesProcess.pid)
|
||||
|
|
@ -6333,13 +6337,10 @@ function stopBackendChild(child) {
|
|||
// (so skeletons retrigger) and re-dials. Distinct from hard re-home (profile
|
||||
// switch / crash recovery), which still resets boot progress + reloads.
|
||||
function resetHermesConnection({ soft = false } = {}) {
|
||||
connectionPromise = null
|
||||
backendStartFailure = null
|
||||
|
||||
const hermesProcess = backendConnectionState.invalidate()
|
||||
stopBackendChild(hermesProcess)
|
||||
|
||||
hermesProcess = null
|
||||
|
||||
if (!soft) {
|
||||
resetBootProgressForReconnect()
|
||||
}
|
||||
|
|
@ -6350,7 +6351,8 @@ function resetHermesConnection({ soft = false } = {}) {
|
|||
// startHermes() spawns fresh instead of racing the dying one. Shared by the
|
||||
// connection-config and profile switch flows.
|
||||
async function teardownPrimaryBackendAndWait({ soft = false } = {}) {
|
||||
// Capture the reference before resetHermesConnection() nulls hermesProcess.
|
||||
// Capture the reference before resetHermesConnection() invalidates it.
|
||||
const hermesProcess = backendConnectionState.getProcess()
|
||||
const dying = hermesProcess && !hermesProcess.killed ? hermesProcess : null
|
||||
|
||||
if (soft) {
|
||||
|
|
@ -6727,11 +6729,15 @@ async function startHermes() {
|
|||
throw backendStartFailure
|
||||
}
|
||||
|
||||
if (connectionPromise) {
|
||||
return connectionPromise
|
||||
const existingConnectionPromise = backendConnectionState.getPromise()
|
||||
|
||||
if (existingConnectionPromise) {
|
||||
return existingConnectionPromise
|
||||
}
|
||||
|
||||
connectionPromise = (async () => {
|
||||
const connectionAttempt = backendConnectionState.startAttempt()
|
||||
|
||||
const connectionPromise = (async () => {
|
||||
await advanceBootProgress('backend.resolve', 'Resolving Hermes backend', 8)
|
||||
// Resolve for the desktop's primary profile so a per-profile remote
|
||||
// override on the active profile is honored (falls back to env / global).
|
||||
|
|
@ -6793,7 +6799,7 @@ async function startHermes() {
|
|||
await advanceBootProgress('backend.spawn', `Starting Hermes backend via ${backend.label}`, 84)
|
||||
rememberLog(`Starting Hermes backend via ${backend.label}`)
|
||||
|
||||
hermesProcess = spawn(
|
||||
const hermesProcess = spawn(
|
||||
backend.command,
|
||||
backend.args,
|
||||
hiddenWindowsChildOptions({
|
||||
|
|
@ -6823,6 +6829,13 @@ async function startHermes() {
|
|||
})
|
||||
)
|
||||
|
||||
const processOwner = backendConnectionState.attachProcess(connectionAttempt, hermesProcess)
|
||||
|
||||
if (!processOwner) {
|
||||
stopBackendChild(hermesProcess)
|
||||
throw new Error('Hermes backend start was superseded by a newer connection attempt.')
|
||||
}
|
||||
|
||||
hermesProcess.stdout.on('data', rememberLog)
|
||||
hermesProcess.stderr.on('data', rememberLog)
|
||||
let backendReady = false
|
||||
|
|
@ -6833,6 +6846,13 @@ async function startHermes() {
|
|||
})
|
||||
|
||||
hermesProcess.once('error', error => {
|
||||
if (!backendConnectionState.clearForCurrentProcess(processOwner)) {
|
||||
rememberLog(`Ignoring stale Hermes backend error: ${error.message}`)
|
||||
rejectBackendStart?.(new Error('Hermes backend start was superseded by a newer connection attempt.'))
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
rememberLog(`Hermes backend failed to start: ${error.message}`)
|
||||
updateBootProgress(
|
||||
{
|
||||
|
|
@ -6843,15 +6863,21 @@ async function startHermes() {
|
|||
},
|
||||
{ allowDecrease: true }
|
||||
)
|
||||
hermesProcess = null
|
||||
connectionPromise = null
|
||||
sendBackendExit({ code: null, signal: null, error: error.message })
|
||||
rejectBackendStart?.(error)
|
||||
})
|
||||
hermesProcess.once('exit', (code, signal) => {
|
||||
if (!backendConnectionState.clearForCurrentProcess(processOwner)) {
|
||||
rememberLog(`Ignoring stale Hermes backend exit (${signal || code})`)
|
||||
|
||||
if (!backendReady) {
|
||||
rejectBackendStart?.(new Error('Hermes backend start was superseded by a newer connection attempt.'))
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
rememberLog(`Hermes backend exited (${signal || code})`)
|
||||
hermesProcess = null
|
||||
connectionPromise = null
|
||||
sendBackendExit({ code, signal })
|
||||
|
||||
if (!backendReady) {
|
||||
|
|
@ -6892,8 +6918,7 @@ async function startHermes() {
|
|||
backendStartFailure = null
|
||||
|
||||
const authToken = await adoptServedDashboardToken(baseUrl, token, {
|
||||
// The exit/error handlers null hermesProcess when the child dies.
|
||||
childAlive: () => hermesProcess !== null && hermesProcess.exitCode === null && !hermesProcess.killed,
|
||||
childAlive: () => hermesProcess.exitCode === null && !hermesProcess.killed,
|
||||
rememberLog
|
||||
})
|
||||
|
||||
|
|
@ -6916,6 +6941,10 @@ async function startHermes() {
|
|||
...getWindowState()
|
||||
}
|
||||
})().catch(error => {
|
||||
if (!backendConnectionState.clearPromiseForAttempt(connectionAttempt)) {
|
||||
throw error
|
||||
}
|
||||
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
backendStartFailure = error instanceof Error ? error : new Error(message)
|
||||
updateBootProgress(
|
||||
|
|
@ -6927,10 +6956,11 @@ async function startHermes() {
|
|||
},
|
||||
{ allowDecrease: true }
|
||||
)
|
||||
connectionPromise = null
|
||||
throw error
|
||||
})
|
||||
|
||||
backendConnectionState.setPromise(connectionAttempt, connectionPromise)
|
||||
|
||||
return connectionPromise
|
||||
}
|
||||
|
||||
|
|
@ -7352,7 +7382,7 @@ function createWindow() {
|
|||
|
||||
ipcMain.handle('hermes:connection', async (_event, profile) => ensureBackend(profile))
|
||||
// Reconnect-after-wake recovery. A REMOTE primary backend has no child process,
|
||||
// so the 'exit'/'error' handlers that would clear a dead connectionPromise never
|
||||
// so the 'exit'/'error' handlers that would clear a dead connection promise never
|
||||
// fire — once the remote becomes unreachable across a sleep/wake the renderer
|
||||
// re-dials the same dead descriptor forever and the composer stays stuck on
|
||||
// "Starting Hermes…". Before the renderer's backoff loop reconnects, it asks us
|
||||
|
|
@ -7360,6 +7390,8 @@ ipcMain.handle('hermes:connection', async (_event, profile) => ensureBackend(pro
|
|||
// not, we drop the cache so the next getConnection() rebuilds it. Local backends
|
||||
// self-heal via their child 'exit' handler, so we never touch them here.
|
||||
ipcMain.handle('hermes:connection:revalidate', async () => {
|
||||
const connectionPromise = backendConnectionState.getPromise()
|
||||
|
||||
if (!connectionPromise) {
|
||||
return { ok: true, rebuilt: false }
|
||||
}
|
||||
|
|
@ -7369,7 +7401,7 @@ ipcMain.handle('hermes:connection:revalidate', async () => {
|
|||
try {
|
||||
conn = await connectionPromise
|
||||
} catch {
|
||||
// The cached boot already rejected (its own catch nulls connectionPromise);
|
||||
// The cached boot already rejected (its own catch clears the promise);
|
||||
// nothing to revalidate — the next getConnection() builds fresh.
|
||||
return { ok: true, rebuilt: false }
|
||||
}
|
||||
|
|
@ -7387,7 +7419,7 @@ ipcMain.handle('hermes:connection:revalidate', async () => {
|
|||
} catch {
|
||||
// Unreachable remote: drop the stale cache so the renderer's next reconnect
|
||||
// tick rebuilds a fresh, reachable descriptor. resetHermesConnection only
|
||||
// nulls connectionPromise for a remote (no child to SIGTERM).
|
||||
// clears the connection promise for a remote (no child to SIGTERM).
|
||||
rememberLog('Cached remote Hermes backend failed liveness probe; dropping stale connection.')
|
||||
resetHermesConnection()
|
||||
|
||||
|
|
@ -9169,7 +9201,7 @@ app.on('before-quit', () => {
|
|||
disposeTerminalSession(id)
|
||||
}
|
||||
|
||||
stopBackendChild(hermesProcess)
|
||||
stopBackendChild(backendConnectionState.getProcess())
|
||||
stopAllPoolBackends()
|
||||
})
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue