From d7e738af90c5e08a5cd9e51559c9f7049a100c8e Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 13:03:59 -0500 Subject: [PATCH] fix(desktop): retire pooled remote backends whose host went away A pooled backend entry pointing at a remote host has no child process, so the 'exit' handler that clears a dead local backend never fires. The renderer's 60s keepalive touch also spares it from the idle reaper. Nothing was left to retire the descriptor, so once the host went away the pool kept serving it and every profile bound to that host stayed broken until restart. Pooled remote descriptors now share the primary's liveness policy: probed on the same revalidate tick, keyed per base URL, and dropped only after the same consecutive-failure limit, so the next ensureBackend() rebuilds. Co-authored-by: Rodrigo Fernandez --- apps/desktop/electron/main.ts | 54 ++++++++-- apps/desktop/electron/remote-liveness.test.ts | 98 +++++++++++++++++++ apps/desktop/electron/remote-liveness.ts | 62 ++++++++++++ 3 files changed, 204 insertions(+), 10 deletions(-) diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index ed0296cf362b..f859a285347c 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -146,7 +146,12 @@ import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRoutePr import { fetchPrimaryProfileSessions } from './profile-session-routing' import { createQuickEntryShortcut, quickEntryWindowBounds, sanitizeQuickEntrySettings } from './quick-entry' import * as remoteLifecycle from './remote-lifecycle' -import { RemoteLivenessTracker, RemoteRevalidationCoordinator, revalidateRemoteConnection } from './remote-liveness' +import { + RemoteLivenessTracker, + RemoteRevalidationCoordinator, + revalidatePooledRemoteBackends, + revalidateRemoteConnection +} from './remote-liveness' import { buildSessionWindowUrl, chatWindowWebPreferences, @@ -7767,7 +7772,14 @@ async function ensureBackend(profile) { evictLruPoolBackends(POOL_MAX_BACKENDS - 1) - const entry = { process: null, port: null, token: null, connectionPromise: null, lastActiveAt: Date.now() } + const entry = { + process: null, + port: null, + token: null, + connectionPromise: null, + lastActiveAt: Date.now(), + remoteBaseUrl: null + } entry.connectionPromise = spawnPoolBackend(key, entry).catch(error => { backendPool.delete(key) throw error @@ -7864,6 +7876,10 @@ async function spawnPoolBackend(profile, entry) { if (remote) { await waitForHermes(remote.baseUrl, remote.token, undefined, remote.authMode) + // Recorded on the entry so revalidation can probe this descriptor without + // awaiting connectionPromise, which may still be pending for a sibling. + entry.remoteBaseUrl = remote.baseUrl + return { ...remote, profile, @@ -9137,6 +9153,8 @@ ipcMain.handle('hermes:connection:revalidate', async () => { const connectionPromise = backendConnectionState.getPromise() if (!connectionPromise) { + await revalidatePool() + return { ok: true, rebuilt: false } } @@ -9144,14 +9162,17 @@ ipcMain.handle('hermes:connection:revalidate', async () => { // share this primary connection. Coalesce simultaneous requests so one outage // produces one failure observation rather than exhausting the whole streak. return remoteRevalidation.run(connectionPromise, async () => { - const result = await revalidateRemoteConnection({ - connectionPromise, - currentConnectionPromise: () => backendConnectionState.getPromise(), - log: rememberLog, - probe: fetchPublicJson, - resetConnection: resetHermesConnection, - tracker: remoteLiveness - }) + const [result] = await Promise.all([ + revalidateRemoteConnection({ + connectionPromise, + currentConnectionPromise: () => backendConnectionState.getPromise(), + log: rememberLog, + probe: fetchPublicJson, + resetConnection: resetHermesConnection, + tracker: remoteLiveness + }), + revalidatePool() + ]) // A rebuilt SSH connection must also tear down its tunnel/master before the // renderer re-dials (which only happens after this handler resolves), so the @@ -9169,6 +9190,19 @@ ipcMain.handle('hermes:connection:revalidate', async () => { return result }) }) + +// Pooled remote descriptors get the same treatment as the primary: they have no +// child process to signal their host's death, and the renderer's keepalive touch +// spares them from the idle reaper, so nothing else can retire a dead one. +function revalidatePool() { + return revalidatePooledRemoteBackends({ + entries: backendPool.entries(), + log: rememberLog, + probe: fetchPublicJson, + stopBackend: stopPoolBackend, + tracker: remoteLiveness + }) +} ipcMain.handle('hermes:backend:touch', async (_event, profile) => { touchPoolBackend(profile) diff --git a/apps/desktop/electron/remote-liveness.test.ts b/apps/desktop/electron/remote-liveness.test.ts index 6c52e11f69f9..2fc9ceb59521 100644 --- a/apps/desktop/electron/remote-liveness.test.ts +++ b/apps/desktop/electron/remote-liveness.test.ts @@ -6,6 +6,7 @@ import { REMOTE_LIVENESS_TIMEOUT_MS, RemoteLivenessTracker, RemoteRevalidationCoordinator, + revalidatePooledRemoteBackends, revalidateRemoteConnection } from './remote-liveness' @@ -251,3 +252,100 @@ describe('revalidateRemoteConnection', () => { expect(rejected.probe).not.toHaveBeenCalled() }) }) + +describe('revalidatePooledRemoteBackends', () => { + const harness = (entries: Array<[string, { process?: unknown; remoteBaseUrl?: null | string }]>) => { + const unreachable = new Set() + const log = vi.fn() + const stopBackend = vi.fn() + const probe = vi.fn(async (url: string) => { + if ([...unreachable].some(base => url.startsWith(base))) { + throw new Error('unreachable') + } + + return {} + }) + + return { + log, + probe, + stopBackend, + unreachable, + run: (tracker: RemoteLivenessTracker) => + revalidatePooledRemoteBackends({ entries, log, probe, stopBackend, tracker }) + } + } + + it('probes only pooled entries backed by a remote host', async () => { + const local = { process: {}, remoteBaseUrl: null } + const spawning = { process: null, remoteBaseUrl: null } + const remote = { process: null, remoteBaseUrl: 'https://remote.example.com' } + + const pool = harness([ + ['local', local], + ['spawning', spawning], + ['remote', remote] + ]) + + await pool.run(new RemoteLivenessTracker()) + + expect(pool.probe).toHaveBeenCalledTimes(1) + expect(pool.probe).toHaveBeenCalledWith('https://remote.example.com/api/status', { + timeoutMs: REMOTE_LIVENESS_TIMEOUT_MS + }) + expect(pool.stopBackend).not.toHaveBeenCalled() + }) + + it('drops a descriptor only after the shared failure limit', async () => { + const pool = harness([['coder', { process: null, remoteBaseUrl: 'https://remote.example.com/' }]]) + pool.unreachable.add('https://remote.example.com') + + const tracker = new RemoteLivenessTracker() + + for (let attempt = 1; attempt < REMOTE_LIVENESS_FAILURE_LIMIT; attempt += 1) { + await expect(pool.run(tracker)).resolves.toEqual({ dropped: [] }) + expect(pool.stopBackend).not.toHaveBeenCalled() + } + + await expect(pool.run(tracker)).resolves.toEqual({ dropped: ['coder'] }) + expect(pool.stopBackend).toHaveBeenCalledWith('coder') + }) + + it('clears the streak when the host answers again', async () => { + const pool = harness([['coder', { process: null, remoteBaseUrl: 'https://remote.example.com' }]]) + const tracker = new RemoteLivenessTracker() + + pool.unreachable.add('https://remote.example.com') + await pool.run(tracker) + + pool.unreachable.clear() + await pool.run(tracker) + + pool.unreachable.add('https://remote.example.com') + + for (let attempt = 1; attempt < REMOTE_LIVENESS_FAILURE_LIMIT; attempt += 1) { + await expect(pool.run(tracker)).resolves.toEqual({ dropped: [] }) + } + + expect(pool.stopBackend).not.toHaveBeenCalled() + await expect(pool.run(tracker)).resolves.toEqual({ dropped: ['coder'] }) + }) + + it('keeps a healthy sibling when another profile on a different host dies', async () => { + const pool = harness([ + ['coder', { process: null, remoteBaseUrl: 'https://dead.example.com' }], + ['writer', { process: null, remoteBaseUrl: 'https://live.example.com' }] + ]) + pool.unreachable.add('https://dead.example.com') + + const tracker = new RemoteLivenessTracker() + + for (let attempt = 1; attempt < REMOTE_LIVENESS_FAILURE_LIMIT; attempt += 1) { + await pool.run(tracker) + } + + await expect(pool.run(tracker)).resolves.toEqual({ dropped: ['coder'] }) + expect(pool.stopBackend).toHaveBeenCalledTimes(1) + expect(pool.stopBackend).toHaveBeenCalledWith('coder') + }) +}) diff --git a/apps/desktop/electron/remote-liveness.ts b/apps/desktop/electron/remote-liveness.ts index eedc16682e7d..6e29b395c483 100644 --- a/apps/desktop/electron/remote-liveness.ts +++ b/apps/desktop/electron/remote-liveness.ts @@ -117,6 +117,68 @@ export class RemoteLivenessTracker { } } +export interface PooledRemoteEntry { + process?: unknown + remoteBaseUrl?: null | string +} + +export interface RevalidatePooledRemoteBackendsOptions { + entries: Iterable<[string, PooledRemoteEntry]> + log: (message: string) => void + probe: (url: string, options: { timeoutMs: number }) => Promise + stopBackend: (profile: string) => void + tracker: RemoteLivenessTracker +} + +/** + * Probe pooled REMOTE descriptors and drop the dead ones. + * + * A pooled entry backed by a remote host has no child process, so the 'exit' + * handler that clears a dead local backend never fires, and the renderer's + * keepalive touch keeps the idle reaper off it. Without this the pool serves a + * descriptor for an unreachable host indefinitely. + * + * Entries share the primary's failure policy, keyed per base URL, so a profile + * pointing at the same host as another does not burn the streak twice as fast. + */ +export async function revalidatePooledRemoteBackends({ + entries, + log, + probe, + stopBackend, + tracker +}: RevalidatePooledRemoteBackendsOptions): Promise<{ dropped: string[] }> { + const remotes = [...entries].filter(([, entry]) => !entry.process && entry.remoteBaseUrl) + const dropped: string[] = [] + + await Promise.all( + remotes.map(async ([profile, entry]) => { + const baseUrl = String(entry.remoteBaseUrl).replace(/\/+$/, '') + + try { + await probe(`${baseUrl}/api/status`, { timeoutMs: REMOTE_LIVENESS_TIMEOUT_MS }) + tracker.recordSuccess(baseUrl) + } catch { + const failure = tracker.recordFailure(baseUrl) + + if (!failure.shouldReset) { + log( + `Pooled remote backend for profile "${profile}" failed liveness probe (${failure.failures}/${REMOTE_LIVENESS_FAILURE_LIMIT}); keeping descriptor for retry.` + ) + + return + } + + log(`Pooled remote backend for profile "${profile}" failed liveness probe; dropping stale descriptor.`) + stopBackend(profile) + dropped.push(profile) + } + }) + ) + + return { dropped } +} + /** * Probe the cached primary remote connection and apply the failure policy. * The caller owns single-flight coordination; identity checks here ensure an