diff --git a/apps/desktop/electron/backend-connection-state.test.ts b/apps/desktop/electron/backend-connection-state.test.ts new file mode 100644 index 000000000000..dcec80445038 --- /dev/null +++ b/apps/desktop/electron/backend-connection-state.test.ts @@ -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() + 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() + 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() + 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() + const staleAttempt = state.startAttempt() + + state.invalidate() + + assert.equal(state.attachProcess(staleAttempt, { id: 'late' }), null) + assert.equal(state.getProcess(), null) +}) diff --git a/apps/desktop/electron/backend-connection-state.ts b/apps/desktop/electron/backend-connection-state.ts new file mode 100644 index 000000000000..07b289c7db64 --- /dev/null +++ b/apps/desktop/electron/backend-connection-state.ts @@ -0,0 +1,84 @@ +export type BackendConnectionAttempt = { + generation: number + promise: Promise | null +} + +export type BackendProcessOwner = { + generation: number + process: TProcess +} + +export function createBackendConnectionState() { + let generation = 0 + let process: TProcess | null = null + let promise: Promise | null = null + + return { + startAttempt(): BackendConnectionAttempt { + return { generation, promise: null } + }, + + setPromise(attempt: BackendConnectionAttempt, nextPromise: Promise): boolean { + if (attempt.generation !== generation) { + return false + } + + attempt.promise = nextPromise + promise = nextPromise + + return true + }, + + attachProcess( + attempt: BackendConnectionAttempt, + nextProcess: TProcess + ): BackendProcessOwner | null { + if (attempt.generation !== generation) { + return null + } + + process = nextProcess + + return { generation, process: nextProcess } + }, + + clearForCurrentProcess(owner: BackendProcessOwner): boolean { + if (owner.generation !== generation || owner.process !== process) { + return false + } + + process = null + promise = null + + return true + }, + + clearPromiseForAttempt(attempt: BackendConnectionAttempt): boolean { + if (attempt.generation !== generation || (promise !== null && attempt.promise !== promise)) { + return false + } + + promise = null + + return true + }, + + getProcess(): TProcess | null { + return process + }, + + getPromise(): Promise | null { + return promise + }, + + invalidate(): TProcess | null { + const currentProcess = process + + generation += 1 + process = null + promise = null + + return currentProcess + } + } +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index fb8ecd5613e1..1aec6d7e8e18 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -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, 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() })