diff --git a/apps/desktop/electron/connection-config.test.ts b/apps/desktop/electron/connection-config.test.ts index 4361678cb8fe..5bae2e8e708a 100644 --- a/apps/desktop/electron/connection-config.test.ts +++ b/apps/desktop/electron/connection-config.test.ts @@ -36,6 +36,7 @@ import { profileRemoteOverride, profileSshOverride, resolveAuthMode, + resolveProfileBackendRoute, resolveTestWsUrl, RT_COOKIE_VARIANTS, savedProfileSsh, @@ -187,6 +188,65 @@ test('saved SSH drafts are inactive and explicit overrides take precedence', () assert.equal(profileHasRemoteConnection(config, 'coder'), true) }) +// --- resolveProfileBackendRoute --- + +const ROUTES = [ + { + name: 'the primary profile owns the window backend', + profile: 'default', + opts: { primaryProfile: 'default' }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'a renamed primary profile still owns the window backend', + profile: ' coder ', + opts: { primaryProfile: 'coder', globalRemote: true }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'an unset profile resolves to the primary', + profile: '', + opts: { primaryProfile: 'default', globalRemote: true }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'a profile inheriting the app-global remote shares the primary backend, scoped per request', + profile: 'coder', + opts: { primaryProfile: 'default', globalRemote: true, profileRemoteOverride: false }, + expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true } + }, + { + name: 'a profile with its own remote override gets a pooled descriptor for that host', + profile: 'coder', + opts: { primaryProfile: 'default', globalRemote: true, profileRemoteOverride: true }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + }, + { + name: 'a local non-primary profile gets its own pooled backend', + profile: 'coder', + opts: { primaryProfile: 'default', globalRemote: false, profileRemoteOverride: false }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + } +] + +for (const route of ROUTES) { + test(`resolveProfileBackendRoute: ${route.name}`, () => { + assert.deepEqual(resolveProfileBackendRoute(route.profile, route.opts), route.expected) + }) +} + +test('resolveProfileBackendRoute only tags a descriptor when the backend is shared', () => { + // A pooled backend is already scoped to its profile, so tagging it would + // imply a second scope the caller must reconcile. Only the shared + // global-remote route carries one. + for (const route of ROUTES) { + const resolved = resolveProfileBackendRoute(route.profile, route.opts) + + assert.equal(Boolean(resolved.descriptorProfile), resolved.scopePath) + assert.ok(!resolved.descriptorProfile || resolved.backend === 'primary') + } +}) + // --- pathWithGlobalRemoteProfile --- test('pathWithGlobalRemoteProfile appends profile in global remote mode', () => { @@ -199,6 +259,17 @@ test('pathWithGlobalRemoteProfile appends profile in global remote mode', () => ) }) +test('pathWithGlobalRemoteProfile skips the primary profile, which the remote already serves', () => { + assert.equal( + pathWithGlobalRemoteProfile('/api/model/info', 'coder', { + globalRemote: true, + primaryProfile: 'coder', + profileRemoteOverride: false + }), + '/api/model/info' + ) +}) + test('pathWithGlobalRemoteProfile preserves existing query params', () => { assert.equal( pathWithGlobalRemoteProfile('/api/model/options?force=1', 'iris', { diff --git a/apps/desktop/electron/connection-config.ts b/apps/desktop/electron/connection-config.ts index f7b1ac2e0d42..c15644b112a0 100644 --- a/apps/desktop/electron/connection-config.ts +++ b/apps/desktop/electron/connection-config.ts @@ -350,16 +350,68 @@ function profileRemoteOverride(config, profile) { return { url, authMode: normAuthMode(entry.authMode), token: entry.token } } +export interface ProfileRouteOptions { + globalRemote?: boolean + primaryProfile?: null | string + profileRemoteOverride?: boolean +} + +export interface ProfileBackendRoute { + /** Which backend serves this profile: the window backend, or a pooled one. */ + backend: 'pool' | 'primary' + /** + * Profile to tag on the returned descriptor when the backend is shared and + * therefore not itself scoped to that profile. Null when the backend already + * belongs to the profile. + */ + descriptorProfile: null | string + /** Whether REST paths on this route must carry `?profile=` to be scoped. */ + scopePath: boolean +} + /** - * In global-remote mode one backend serves every Desktop profile, so REST calls - * that are scoped by renderer-side `request.profile` must carry that scope as a - * query parameter. Local pooled backends and per-profile remote overrides do not - * need this: they already run against a backend scoped to the target profile. + * The one place that answers "which backend serves profile P, and does its + * REST path need a profile scope?". Four routes, in precedence order: + * + * 1. The primary profile owns the window backend outright. + * 2. A profile with its own remote override gets a pooled descriptor for that + * host, which is already scoped to it. + * 3. A profile inheriting the app-global remote shares the primary backend — + * one host serves every profile — so it is scoped per request instead. + * 4. Any other local profile gets its own pooled backend, spawned with + * `--profile`, so its `HERMES_HOME` scopes it. + * + * Routing used to be spread across three overlapping predicates that each + * re-derived part of this table, which is how case 3 ended up registering + * reapable pool entries for backends it never owned. */ -function pathWithGlobalRemoteProfile(path, profile, opts: any = {}) { +function resolveProfileBackendRoute(profile, opts: ProfileRouteOptions = {}): ProfileBackendRoute { + const scopedProfile = connectionScopeKey(profile) + const primaryProfile = connectionScopeKey(opts.primaryProfile) || 'default' + + if (!scopedProfile || scopedProfile === primaryProfile) { + return { backend: 'primary', descriptorProfile: null, scopePath: false } + } + + if (opts.profileRemoteOverride) { + return { backend: 'pool', descriptorProfile: null, scopePath: false } + } + + if (opts.globalRemote) { + return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true } + } + + return { backend: 'pool', descriptorProfile: null, scopePath: false } +} + +/** + * Add renderer-side `request.profile` to a REST path when the route says the + * serving backend is not already scoped to that profile. + */ +function pathWithGlobalRemoteProfile(path, profile, opts: ProfileRouteOptions = {}) { const scopedProfile = connectionScopeKey(profile) - if (!scopedProfile || !opts.globalRemote || opts.profileRemoteOverride) { + if (!resolveProfileBackendRoute(profile, opts).scopePath) { return path } @@ -506,6 +558,7 @@ export { profileRemoteOverride, profileSshOverride, resolveAuthMode, + resolveProfileBackendRoute, resolveTestWsUrl, RT_COOKIE_VARIANTS, savedProfileSsh, diff --git a/apps/desktop/electron/crash-forensics.test.ts b/apps/desktop/electron/crash-forensics.test.ts new file mode 100644 index 000000000000..ea53493b4c1f --- /dev/null +++ b/apps/desktop/electron/crash-forensics.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it, vi } from 'vitest' + +import { describeCrashReason, installCrashForensics } from './crash-forensics' + +const harness = () => { + const listeners = new Map void>() + const flush = vi.fn() + const log = vi.fn() + + installCrashForensics({ + flush, + log, + target: { on: (event, listener) => listeners.set(event, listener) } + }) + + return { flush, listeners, log } +} + +describe('describeCrashReason', () => { + it('prefers a stack, then a message, for thrown errors', () => { + const withStack = new Error('boom') + withStack.stack = 'Error: boom\n at somewhere' + + expect(describeCrashReason(withStack)).toBe('Error: boom\n at somewhere') + + const withoutStack = new Error('boom') + withoutStack.stack = '' + + expect(describeCrashReason(withoutStack)).toBe('boom') + }) + + it('renders non-error rejections without throwing', () => { + expect(describeCrashReason('plain string')).toBe('plain string') + expect(describeCrashReason({ code: 'ECONNRESET' })).toBe('{"code":"ECONNRESET"}') + expect(describeCrashReason(undefined)).toBe('undefined') + + const circular: Record = {} + circular.self = circular + + expect(describeCrashReason(circular)).toBe('[object Object]') + }) +}) + +describe('installCrashForensics', () => { + it('records and synchronously flushes an uncaught exception', () => { + const { flush, listeners, log } = harness() + const error = new Error('renderer gone') + error.stack = 'Error: renderer gone\n at main' + + listeners.get('uncaughtException')?.(error) + + expect(log).toHaveBeenCalledWith('[main] Uncaught exception: Error: renderer gone\n at main') + expect(flush).toHaveBeenCalledTimes(1) + }) + + it('records and synchronously flushes an unhandled rejection', () => { + const { flush, listeners, log } = harness() + + listeners.get('unhandledRejection')?.('gateway ticket mint failed') + + expect(log).toHaveBeenCalledWith('[main] Unhandled rejection: gateway ticket mint failed') + expect(flush).toHaveBeenCalledTimes(1) + }) + + it('registers both handlers', () => { + const { listeners } = harness() + + expect([...listeners.keys()].sort()).toEqual(['uncaughtException', 'unhandledRejection']) + }) +}) diff --git a/apps/desktop/electron/crash-forensics.ts b/apps/desktop/electron/crash-forensics.ts new file mode 100644 index 000000000000..7ee0aedd48f3 --- /dev/null +++ b/apps/desktop/electron/crash-forensics.ts @@ -0,0 +1,51 @@ +/** + * Last-chance forensics for the Electron main process. + * + * Electron installs its own `uncaughtException` listener and only warns on + * unhandled rejections, so the app usually survives — but the reason lands on + * stderr alone, which is discarded entirely when the app is launched from + * Finder or the Start menu. Without a record in desktop.log, a main-process + * fault is invisible in a `hermes debug share` bundle and the user is left + * describing symptoms instead of showing a stack. + */ + +export interface CrashForensicsTarget { + on: (event: 'uncaughtException' | 'unhandledRejection', listener: (value: unknown) => void) => unknown +} + +export interface CrashForensicsOptions { + flush: () => void + log: (message: string) => void + target?: CrashForensicsTarget +} + +/** Render a thrown value for the log, preferring a stack over a bare message. */ +export function describeCrashReason(reason: unknown): string { + if (reason instanceof Error) { + return reason.stack || reason.message || reason.name || 'Error' + } + + if (typeof reason === 'string') { + return reason + } + + try { + return JSON.stringify(reason) ?? String(reason) + } catch { + return String(reason) + } +} + +/** + * Record main-process faults to desktop.log and flush synchronously, since a + * fault that does prove fatal leaves no chance for the batched async flush. + */ +export function installCrashForensics({ flush, log, target = process }: CrashForensicsOptions): void { + const record = (label: string) => (reason: unknown) => { + log(`[main] ${label}: ${describeCrashReason(reason)}`) + flush() + } + + target.on('uncaughtException', record('Uncaught exception')) + target.on('unhandledRejection', record('Unhandled rejection')) +} diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 5f6ba957ff8c..d88e1f787601 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -64,10 +64,12 @@ import { profileRemoteOverride, profileSshOverride, resolveAuthMode, + resolveProfileBackendRoute, resolveTestWsUrl, savedProfileSsh, tokenPreview } from './connection-config' +import { describeCrashReason, installCrashForensics } from './crash-forensics' import { adoptServedDashboardToken } from './dashboard-token' import { loadOrCreateInstallationId, sshOwnershipId } from './desktop-installation' import { @@ -142,9 +144,15 @@ import { createKeepAwake } from './power-save' import { FirstRunSetupResetError, runPrimaryBackendStartup } from './primary-backend-startup' import { rehomePrimaryConnection } from './primary-connection-rehome' import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing' +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, @@ -1212,6 +1220,14 @@ function rememberLog(chunk) { scheduleDesktopLogFlush() } +installCrashForensics({ flush: flushDesktopLogBufferSync, log: rememberLog }) + +// A rejected loadURL leaves a blank window and, unhandled, no trace anywhere +// the user can send us. `label` names the surface so the log says which one. +function loadWindowUrl(win, url, label) { + win.loadURL(url).catch(error => rememberLog(`${label} failed to load: ${describeCrashReason(error)}`)) +} + function openExternalUrl(rawUrl) { const raw = String(rawUrl || '').trim() @@ -7726,15 +7742,28 @@ function primaryProfileKey() { return readActiveDesktopProfile() || 'default' } -// Resolve a backend connection for the given profile. Routes the primary -// profile to startHermes() (the window backend: boot UI, bootstrap, remote -// mode), and any OTHER profile to a lazily-spawned pool backend. An empty / -// unknown profile resolves to the primary, so all legacy callers are unchanged. +// Options describing the current connection setup for `resolveProfileBackendRoute`. +function profileRouteOptions(profile) { + return { + globalRemote: globalRemoteActive(), + primaryProfile: primaryProfileKey(), + profileRemoteOverride: Boolean(profileHasRemoteOverride(profile)) + } +} + +// Resolve a backend connection for the given profile, per the routing table in +// resolveProfileBackendRoute(). An empty / unknown profile resolves to the +// primary, so legacy callers are unchanged. async function ensureBackend(profile) { const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey() + const route = resolveProfileBackendRoute(key, profileRouteOptions(key)) - if (key === primaryProfileKey()) { - return startHermes() + if (route.backend === 'primary') { + const connection = await startHermes() + + // A shared backend still owes the caller its profile scope, so renderer-side + // WebSocket, filesystem, and cache routing target the selected profile. + return route.descriptorProfile ? { ...connection, profile: route.descriptorProfile } : connection } const existing = backendPool.get(key) @@ -7747,7 +7776,15 @@ 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 @@ -7844,6 +7881,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, @@ -8434,12 +8475,14 @@ function spawnSecondaryWindow({ sessionId, watch }: { sessionId?: string; watch? wireCommonWindowHandlers(win, zoomWiringForWindowKind('chat')) - win.loadURL( + loadWindowUrl( + win, buildSessionWindowUrl(sessionId, { devServer: DEV_SERVER, rendererIndexPath: DEV_SERVER ? undefined : resolveRendererIndex(), watch - }) + }), + 'Session window' ) return win @@ -8519,11 +8562,7 @@ function createInstanceWindow() { instanceWindows.delete(win) }) - if (DEV_SERVER) { - win.loadURL(DEV_SERVER) - } else { - win.loadURL(pathToFileURL(resolveRendererIndex()).toString()) - } + loadWindowUrl(win, DEV_SERVER || pathToFileURL(resolveRendererIndex()).toString(), 'Instance window') return win } @@ -8635,7 +8674,7 @@ function spawnPetOverlayWindow(bounds) { } }) - win.loadURL(petOverlayUrl()) + loadWindowUrl(win, petOverlayUrl(), 'Pet overlay') return win } @@ -8787,7 +8826,7 @@ function spawnQuickEntryWindow() { } }) - win.loadURL(quickEntryUrl()) + loadWindowUrl(win, quickEntryUrl(), 'Quick entry') return win } @@ -9081,11 +9120,7 @@ function createWindow() { rememberLog(`[renderer console] ${text} (${src}:${lineNo})`) }) - if (DEV_SERVER) { - mainWindow.loadURL(DEV_SERVER) - } else { - mainWindow.loadURL(pathToFileURL(resolveRendererIndex()).toString()) - } + loadWindowUrl(mainWindow, DEV_SERVER || pathToFileURL(resolveRendererIndex()).toString(), 'Renderer') // Start the Python backend NOW, in parallel with the renderer load — not on // did-finish-load. The backend cold boot (spawn → port announce → /api/status) @@ -9117,6 +9152,8 @@ ipcMain.handle('hermes:connection:revalidate', async () => { const connectionPromise = backendConnectionState.getPromise() if (!connectionPromise) { + await revalidatePool() + return { ok: true, rebuilt: false } } @@ -9124,14 +9161,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 @@ -9149,6 +9189,20 @@ 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) @@ -9775,12 +9829,7 @@ async function fetchProfilesSessionSlice(searchParams, remoteProfiles) { return remoteSessionList(requested, searchParams) } - const primary = await ensureBackend(null) - - return fetchJson(`${primary.baseUrl}/api/profiles/sessions?${searchParams}`, primary.token, { - method: 'GET', - timeoutMs: DEFAULT_FETCH_TIMEOUT_MS - }).catch(() => ({ sessions: [], total: 0, profile_totals: {} })) + return fetchPrimaryProfileSessions(searchParams, fetchJsonForProfile) } return mergeRemoteProfileSessions(searchParams, remoteProfiles) @@ -9795,12 +9844,7 @@ async function mergeRemoteProfileSessions(searchParams, remoteProfiles) { const offset = Math.max(0, Number(searchParams.get('offset')) || 0) const order = searchParams.get('order') === 'created' ? 'started_at' : 'last_active' - const primary = await ensureBackend(null) - - const base = (await fetchJson(`${primary.baseUrl}/api/profiles/sessions?${searchParams}`, primary.token, { - method: 'GET', - timeoutMs: DEFAULT_FETCH_TIMEOUT_MS - }).catch(() => ({ sessions: [], total: 0, profile_totals: {} }))) as any + const base = (await fetchPrimaryProfileSessions(searchParams, fetchJsonForProfile)) as any // Over-fetch each remote from offset 0 (limit+offset rows) so the merged window // is correct for this page — mirrors the primary's per-profile over-fetch. @@ -9859,10 +9903,7 @@ ipcMain.handle('hermes:api', async (_event, request) => { const connection = await ensureBackend(routeProfile) const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS) - const requestPath = pathWithGlobalRemoteProfile(request.path, profile, { - globalRemote: globalRemoteActive(), - profileRemoteOverride: profileHasRemoteOverride(profile) - }) + const requestPath = pathWithGlobalRemoteProfile(request.path, profile, profileRouteOptions(profile)) const url = `${connection.baseUrl}${requestPath}` diff --git a/apps/desktop/electron/profile-session-routing.test.ts b/apps/desktop/electron/profile-session-routing.test.ts new file mode 100644 index 000000000000..199740519bd6 --- /dev/null +++ b/apps/desktop/electron/profile-session-routing.test.ts @@ -0,0 +1,30 @@ +import assert from 'node:assert/strict' + +import { test } from 'vitest' + +import { fetchPrimaryProfileSessions } from './profile-session-routing' + +test('primary session reads use the profile-aware request path', async () => { + const calls: Array<{ profile: string | null; path: string }> = [] + const expected = { sessions: [{ id: 'session-1' }], total: 1, profile_totals: { default: 1 } } + + const result = await fetchPrimaryProfileSessions( + new URLSearchParams({ profile: 'default', limit: '20' }), + async (profile, path) => { + calls.push({ profile, path }) + + return expected + } + ) + + assert.deepEqual(calls, [{ profile: null, path: '/api/profiles/sessions?profile=default&limit=20' }]) + assert.equal(result, expected) +}) + +test('primary session reads preserve the empty-list fallback', async () => { + const result = await fetchPrimaryProfileSessions(new URLSearchParams({ profile: 'all' }), async () => { + throw new Error('remote unavailable') + }) + + assert.deepEqual(result, { sessions: [], total: 0, profile_totals: {} }) +}) diff --git a/apps/desktop/electron/profile-session-routing.ts b/apps/desktop/electron/profile-session-routing.ts new file mode 100644 index 000000000000..f31e22ca52a3 --- /dev/null +++ b/apps/desktop/electron/profile-session-routing.ts @@ -0,0 +1,19 @@ +export interface ProfileSessionsResponse { + sessions: unknown[] + total: number + profile_totals: Record + [key: string]: unknown +} + +type FetchJsonForProfile = (profile: string | null, path: string) => Promise + +export async function fetchPrimaryProfileSessions( + searchParams: URLSearchParams, + fetchJsonForProfile: FetchJsonForProfile +): Promise { + try { + return (await fetchJsonForProfile(null, `/api/profiles/sessions?${searchParams}`)) as ProfileSessionsResponse + } catch { + return { sessions: [], total: 0, profile_totals: {} } + } +} diff --git a/apps/desktop/electron/remote-liveness.test.ts b/apps/desktop/electron/remote-liveness.test.ts index 6c52e11f69f9..950d6fb1f58a 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,102 @@ 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