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