Merge pull request #65885 from NousResearch/bb/salvage-62308-stale-backend

fix(desktop): preserve active connection across stale backend exits (supersedes #62308)
This commit is contained in:
brooklyn! 2026-07-16 15:01:37 -04:00 committed by GitHub
commit bed46fcd5c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 247 additions and 33 deletions

View file

@ -0,0 +1,70 @@
import assert from 'node:assert/strict'
import { test } from 'vitest'
import { createBackendConnectionState } from './backend-connection-state'
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)
})

View 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
}
}
}

View file

@ -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()
})

View file

@ -0,0 +1,19 @@
import { describe, expect, it } from 'vitest'
import { savedCloudConnectionUrl } from './gateway-settings'
describe('savedCloudConnectionUrl', () => {
it('normalizes the URL of a persisted cloud connection', () => {
expect(savedCloudConnectionUrl({ mode: 'cloud', remoteUrl: ' HTTPS://AGENT.EXAMPLE/ ' })).toBe(
'https://agent.example'
)
})
it('does not treat a stale cloud URL on a local config as connected', () => {
expect(savedCloudConnectionUrl({ mode: 'local', remoteUrl: 'https://agent.example' })).toBe('')
})
it('does not treat a remote gateway URL as a connected cloud agent', () => {
expect(savedCloudConnectionUrl({ mode: 'remote', remoteUrl: 'https://agent.example' })).toBe('')
})
})

View file

@ -44,6 +44,10 @@ const EMPTY_STATE: GatewaySettingsState = {
cloudOrg: ''
}
export function savedCloudConnectionUrl(config: Pick<GatewaySettingsState, 'mode' | 'remoteUrl'>): string {
return config.mode === 'cloud' ? config.remoteUrl.trim().replace(/\/+$/, '').toLowerCase() : ''
}
function ModeCard({
active,
description,
@ -124,6 +128,12 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
const [state, setState] = useState<GatewaySettingsState>(EMPTY_STATE)
const [remoteToken, setRemoteToken] = useState('')
const [lastTest, setLastTest] = useState<null | string>(null)
const [connectedCloudUrl, setConnectedCloudUrl] = useState('')
const acceptSavedConfig = (config: GatewaySettingsState) => {
setState(config)
setConnectedCloudUrl(savedCloudConnectionUrl(config))
}
// --- Hermes Cloud (cloud mode) state ---
// One portal session powers discovery + the silent per-agent cascade. These
@ -191,7 +201,7 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
return
}
setState(config)
acceptSavedConfig(config)
})
.catch(err => notifyError(err, g.failedLoad))
.finally(() => {
@ -220,7 +230,6 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
// (trim, drop trailing slash, lowercase) or a host-casing difference would
// silently break the connected-highlight.
const normalizeCloudUrl = (url: string) => url.trim().replace(/\/+$/, '').toLowerCase()
const connectedCloudUrl = state.mode === 'cloud' ? normalizeCloudUrl(state.remoteUrl) : ''
const isConnectedAgent = (agent: DesktopCloudAgent) =>
Boolean(connectedCloudUrl && agent.dashboardUrl && normalizeCloudUrl(agent.dashboardUrl) === connectedCloudUrl)
@ -368,7 +377,7 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
? await window.hermesDesktop.applyConnectionConfig(payload())
: await window.hermesDesktop.saveConnectionConfig(payload())
setState(next)
acceptSavedConfig(next)
setRemoteToken('')
notify({
kind: 'success',
@ -404,13 +413,13 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
remoteUrl: trimmedUrl
})
setState(saved)
acceptSavedConfig(saved)
const result = await window.hermesDesktop.oauthLoginConnectionConfig(trimmedUrl)
if (result.connected) {
const refreshed = await window.hermesDesktop.getConnectionConfig(scope)
setState(refreshed)
acceptSavedConfig(refreshed)
notify({ kind: 'success', title: g.signedIn, message: g.connectedTo(providerLabel) })
} else {
notify({
@ -432,7 +441,7 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
try {
await window.hermesDesktop.oauthLogoutConnectionConfig(trimmedUrl || undefined)
const refreshed = await window.hermesDesktop.getConnectionConfig(scope)
setState(refreshed)
acceptSavedConfig(refreshed)
notify({ kind: 'success', title: g.signedOutTitle, message: g.signedOutMessage })
} catch (err) {
notifyError(err, g.signOutFailed)
@ -656,7 +665,7 @@ export function GatewaySettings({ embedded = false }: { embedded?: boolean } = {
cloudOrg: cloudOrgRef.current ?? undefined
})
setState(next)
acceptSavedConfig(next)
notify({ kind: 'success', title: g.cloudConnectedTitle, message: g.cloudConnectedTo(agent.name) })
} catch (err) {
if (err && typeof err === 'object' && 'needsCloudLogin' in err) {