From 5a352af80e4c7d34ce031b10fb31dc46d208b5cd Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Tue, 23 Jun 2026 15:53:42 -0700 Subject: [PATCH] Reduce renderer crash recovery loops --- .../process-gone-classification.test.ts | 50 +++++ .../process-gone-classification.ts | 7 +- .../process-gone-diagnostics.test.ts | 24 +++ src/main/index.ts | 23 ++- src/main/window/createMainWindow.test.ts | 187 +++++++++++++++++- src/main/window/createMainWindow.ts | 71 +++++-- .../window/renderer-recovery-guard.test.ts | 114 +++++++++++ src/main/window/renderer-recovery-guard.ts | 148 ++++++++++++++ .../task-page-github-work-item-status.ts | 3 - 9 files changed, 603 insertions(+), 24 deletions(-) create mode 100644 src/main/window/renderer-recovery-guard.test.ts create mode 100644 src/main/window/renderer-recovery-guard.ts diff --git a/src/main/crash-reporting/process-gone-classification.test.ts b/src/main/crash-reporting/process-gone-classification.test.ts index e143729fb29..6c40c952b1d 100644 --- a/src/main/crash-reporting/process-gone-classification.test.ts +++ b/src/main/crash-reporting/process-gone-classification.test.ts @@ -201,6 +201,36 @@ describe('shouldRecordProcessGoneCrash', () => { expectedTeardown: 'none' }) ).toBe(false) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'video_capture.mojom.VideoCaptureService', + reason: 'killed', + exitCode: 1, + expectedTeardown: 'none' + }) + ).toBe(false) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'video_capture.mojom.VideoCaptureService', + reason: 'crashed', + exitCode: -1, + expectedTeardown: 'none' + }) + ).toBe(false) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'video_capture.mojom.VideoCaptureService', + reason: 'abnormal-exit', + exitCode: 512, + expectedTeardown: 'none' + }) + ).toBe(false) }) it('still records unknown child process crashes', () => { @@ -236,6 +266,26 @@ describe('shouldRecordProcessGoneCrash', () => { expectedTeardown: 'none' }) ).toBe(true) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'video_capture.mojom.VideoCaptureService', + reason: 'oom', + exitCode: 1, + expectedTeardown: 'none' + }) + ).toBe(true) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'video_capture.mojom.VideoCaptureService', + reason: 'launch-failed', + exitCode: 18, + expectedTeardown: 'none' + }) + ).toBe(true) }) it('still records renderer kills from the recent Linux crash-report cluster', () => { diff --git a/src/main/crash-reporting/process-gone-classification.ts b/src/main/crash-reporting/process-gone-classification.ts index 577f40e2ab9..d031cbb048a 100644 --- a/src/main/crash-reporting/process-gone-classification.ts +++ b/src/main/crash-reporting/process-gone-classification.ts @@ -5,7 +5,8 @@ const WINDOWS_CONTROL_TERMINATION_EXIT_CODES = new Set([0xc000013a, 0x40010004]) const RECOVERABLE_CHILD_PROCESS_TYPES = new Set(['gpu']) const RECOVERABLE_UTILITY_SERVICE_NAMES = new Set([ 'audio.mojom.AudioService', - 'network.mojom.NetworkService' + 'network.mojom.NetworkService', + 'video_capture.mojom.VideoCaptureService' ]) const RECOVERABLE_CHILD_PROCESS_REASONS = new Set(['abnormal-exit', 'crashed', 'killed']) const NON_RECOVERABLE_RENDERER_REASONS = new Set(['integrity-failure', 'launch-failed']) @@ -60,8 +61,8 @@ export function shouldRecordProcessGoneCrash({ exitCode: number | null expectedTeardown: ExpectedTeardownScope }): boolean { - // Why: GPU, Network Service, and Audio Service exits are recoverable Chromium - // child-process churn; treating them as app crashes creates noisy user prompts. + // Why: GPU and named Chromium utility service exits are recoverable child + // process churn; treating them as app crashes creates noisy user prompts. if (isRecoverableChromiumChildProcess({ source, processType, serviceName, reason })) { return false } diff --git a/src/main/crash-reporting/process-gone-diagnostics.test.ts b/src/main/crash-reporting/process-gone-diagnostics.test.ts index 769ed618766..9960365fda7 100644 --- a/src/main/crash-reporting/process-gone-diagnostics.test.ts +++ b/src/main/crash-reporting/process-gone-diagnostics.test.ts @@ -64,6 +64,30 @@ describe('process gone diagnostics', () => { }) }) + it('preserves renderer recovery summary fields in persisted crash details', () => { + expect( + buildProcessGoneCrashDetails({ + processType: 'renderer', + rendererRecoveryTotalProcessGoneCount: 3, + rendererRecoveryRecentProcessGoneCount: 3, + rendererRecoverySuppressedRecoveryCount: 1, + rendererRecoveryLastReason: 'crashed', + rendererRecoveryLastExitCode: 5, + rendererRecoveryDegraded: true, + rendererRecoveryDegradedUntil: 123_456 + }) + ).toMatchObject({ + processType: 'renderer', + rendererRecoveryTotalProcessGoneCount: 3, + rendererRecoveryRecentProcessGoneCount: 3, + rendererRecoverySuppressedRecoveryCount: 1, + rendererRecoveryLastReason: 'crashed', + rendererRecoveryLastExitCode: 5, + rendererRecoveryDegraded: true, + rendererRecoveryDegradedUntil: 123_456 + }) + }) + it('preserves child process identity on suppressed breadcrumbs', () => { expect( buildSuppressedProcessGoneBreadcrumbData({ diff --git a/src/main/index.ts b/src/main/index.ts index d684845923a..dfa09e5f3c9 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -79,6 +79,7 @@ import { getInitialClaudeRateLimitTarget } from './rate-limits/claude-rate-limit import { getInitialCodexRateLimitTarget } from './rate-limits/codex-rate-limit-target' import { attachMainWindowServices } from './window/attach-main-window-services' import { createMainWindow, loadMainWindow } from './window/createMainWindow' +import type { RendererRecoverySummary } from './window/renderer-recovery-guard' import { createSystemTray, destroySystemTray } from './tray/system-tray' import { focusExistingMainWindow } from './window/focus-existing-window' import { CodexAccountService } from './codex-accounts/service' @@ -649,18 +650,22 @@ function openMainWindow(): BrowserWindow { isQuitting = false clearExpectedRendererReload() }, - onRendererProcessGone: (details, webContentsId) => { + onRendererProcessGone: (details, webContentsId, recoverySummary) => { recordProcessGoneCrash( 'renderer', 'renderer', details.reason, details.exitCode ?? null, { - processType: 'renderer' + processType: 'renderer', + ...toRendererRecoveryCrashDetails(recoverySummary) }, webContentsId ) }, + onRendererRecoveryEvent: (name, summary) => { + recordCrashBreadcrumb(name, toRendererRecoveryCrashDetails(summary)) + }, shouldRecordRendererCrash: (details, webContentsId) => shouldRecordProcessGoneCrash({ source: 'renderer', @@ -887,6 +892,20 @@ function sendOpenCrashReport(targetWindow?: BrowserWindow | null): void { webContents?.send('ui:openCrashReport') } +function toRendererRecoveryCrashDetails( + summary: RendererRecoverySummary +): Record { + return { + rendererRecoveryTotalProcessGoneCount: summary.totalProcessGoneCount, + rendererRecoveryRecentProcessGoneCount: summary.recentProcessGoneCount, + rendererRecoverySuppressedRecoveryCount: summary.suppressedRecoveryCount, + rendererRecoveryLastReason: summary.lastReason, + rendererRecoveryLastExitCode: summary.lastExitCode, + rendererRecoveryDegraded: summary.degraded, + rendererRecoveryDegradedUntil: summary.degradedUntil + } +} + function recordProcessGoneCrash( source: 'renderer' | 'child', processType: string, diff --git a/src/main/window/createMainWindow.test.ts b/src/main/window/createMainWindow.test.ts index 85a0e805c80..ffc3078e132 100644 --- a/src/main/window/createMainWindow.test.ts +++ b/src/main/window/createMainWindow.test.ts @@ -2262,7 +2262,16 @@ describe('createMainWindow', () => { const details = { reason: 'crashed', exitCode: 5 } as Electron.RenderProcessGoneDetails windowHandlers['render-process-gone']?.({} as never, details) - expect(onRendererProcessGone).toHaveBeenCalledWith(details, 142) + expect(onRendererProcessGone).toHaveBeenCalledWith( + details, + 142, + expect.objectContaining({ + totalProcessGoneCount: 1, + recentProcessGoneCount: 1, + lastReason: 'crashed', + lastExitCode: 5 + }) + ) }) it('passes the renderer webContents id through crash classification callbacks', () => { @@ -2313,7 +2322,14 @@ describe('createMainWindow', () => { vi.advanceTimersByTime(250) expect(shouldRecordRendererCrash).toHaveBeenCalledWith(details, 424) - expect(onRendererProcessGone).toHaveBeenCalledWith(details, 424) + expect(onRendererProcessGone).toHaveBeenCalledWith( + details, + 424, + expect.objectContaining({ + totalProcessGoneCount: 1, + recentProcessGoneCount: 1 + }) + ) expect(shouldRecoverRenderer).toHaveBeenCalledWith(details, 424) } finally { consoleError.mockRestore() @@ -2499,6 +2515,173 @@ describe('createMainWindow', () => { consoleError.mockRestore() }) + it('does not let expected renderer reload teardown consume automatic recovery budget', () => { + vi.useFakeTimers() + + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { browserWindowInstance, windowHandlers } = createRendererRecoveryWindowHarness() + const shouldRecoverRenderer = vi.fn( + (details: Electron.RenderProcessGoneDetails) => details.reason !== 'killed' + ) + + createMainWindow(null, { shouldRecoverRenderer }) + + const expectedReloadDetails = { + reason: 'killed', + exitCode: 1 + } as Electron.RenderProcessGoneDetails + windowHandlers['render-process-gone']?.({} as never, expectedReloadDetails) + windowHandlers['render-process-gone']?.({} as never, expectedReloadDetails) + vi.advanceTimersByTime(250) + + const crashDetails = { + reason: 'crashed', + exitCode: 5 + } as Electron.RenderProcessGoneDetails + windowHandlers['render-process-gone']?.({} as never, crashDetails) + vi.advanceTimersByTime(250) + + expect(browserWindowInstance.loadFile).toHaveBeenCalledTimes(2) + expect(browserWindowInstance.loadURL).not.toHaveBeenCalled() + expect(shouldRecoverRenderer).toHaveBeenCalledWith(crashDetails, 143) + + consoleError.mockRestore() + }) + + it('suppresses automatic recovery after repeated renderer losses in the loop window', () => { + vi.useFakeTimers() + + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { browserWindowInstance, windowHandlers } = createRendererRecoveryWindowHarness() + const onRendererRecoveryEvent = vi.fn() + const onRendererProcessGone = vi.fn() + + createMainWindow(null, { onRendererProcessGone, onRendererRecoveryEvent }) + + const details = { + reason: 'crashed', + exitCode: 5 + } as Electron.RenderProcessGoneDetails + for (let i = 0; i < 3; i += 1) { + windowHandlers['render-process-gone']?.({} as never, details) + vi.advanceTimersByTime(250) + } + + expect(browserWindowInstance.loadFile).toHaveBeenCalledTimes(3) + expect(browserWindowInstance.loadURL).not.toHaveBeenCalled() + expect(onRendererRecoveryEvent).toHaveBeenCalledWith( + 'renderer_recovery_suppressed', + expect.objectContaining({ + totalProcessGoneCount: 3, + recentProcessGoneCount: 3, + suppressedRecoveryCount: 1, + degraded: true, + lastReason: 'crashed', + lastExitCode: 5 + }) + ) + expect(onRendererProcessGone).toHaveBeenLastCalledWith( + details, + 143, + expect.objectContaining({ + totalProcessGoneCount: 3, + recentProcessGoneCount: 3, + suppressedRecoveryCount: 1, + degraded: true, + lastReason: 'crashed', + lastExitCode: 5 + }) + ) + + consoleError.mockRestore() + }) + + it('allows automatic recovery again after the renderer stays stable', () => { + vi.useFakeTimers() + + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { browserWindowInstance, windowHandlers } = createRendererRecoveryWindowHarness() + const onRendererRecoveryEvent = vi.fn() + + createMainWindow(null, { onRendererRecoveryEvent }) + + const details = { + reason: 'crashed', + exitCode: 5 + } as Electron.RenderProcessGoneDetails + windowHandlers['render-process-gone']?.({} as never, details) + vi.advanceTimersByTime(250) + windowHandlers['did-finish-load']?.() + vi.advanceTimersByTime(30_000) + + windowHandlers['render-process-gone']?.({} as never, details) + vi.advanceTimersByTime(250) + windowHandlers['render-process-gone']?.({} as never, details) + vi.advanceTimersByTime(250) + + expect(browserWindowInstance.loadFile).toHaveBeenCalledTimes(4) + expect(onRendererRecoveryEvent).toHaveBeenCalledWith( + 'renderer_recovery_stable', + expect.objectContaining({ + totalProcessGoneCount: 1, + recentProcessGoneCount: 0, + degraded: false + }) + ) + + consoleError.mockRestore() + }) + + it('does not emit stable recovery breadcrumbs after the window closes', () => { + vi.useFakeTimers() + + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { windowHandlers } = createRendererRecoveryWindowHarness() + const onRendererRecoveryEvent = vi.fn() + + createMainWindow(null, { onRendererRecoveryEvent }) + + windowHandlers['render-process-gone']?.( + {} as never, + { + reason: 'crashed', + exitCode: 5 + } as Electron.RenderProcessGoneDetails + ) + windowHandlers['did-finish-load']?.() + onRendererRecoveryEvent.mockClear() + windowHandlers.closed?.() + vi.advanceTimersByTime(30_000) + + expect(onRendererRecoveryEvent).not.toHaveBeenCalled() + + consoleError.mockRestore() + }) + + it('cancels pending automatic recovery when renderer webContents is destroyed', () => { + vi.useFakeTimers() + + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { browserWindowInstance, windowHandlers } = createRendererRecoveryWindowHarness() + + createMainWindow(null) + + windowHandlers['render-process-gone']?.( + {} as never, + { + reason: 'crashed', + exitCode: 5 + } as Electron.RenderProcessGoneDetails + ) + windowHandlers.destroyed?.() + vi.advanceTimersByTime(250) + + expect(browserWindowInstance.loadFile).toHaveBeenCalledTimes(1) + expect(browserWindowInstance.loadURL).not.toHaveBeenCalled() + + consoleError.mockRestore() + }) + it('does not reload after a clean renderer exit', () => { vi.useFakeTimers() diff --git a/src/main/window/createMainWindow.ts b/src/main/window/createMainWindow.ts index dec78fea5ac..3b436abe3ea 100644 --- a/src/main/window/createMainWindow.ts +++ b/src/main/window/createMainWindow.ts @@ -41,6 +41,11 @@ import { import { getMainE2EConfig } from '../e2e-config' import { buildEditableContextMenuTemplate } from './editable-context-menu' import { clearTrustedUIRendererWebContentsId, setTrustedUIRendererWebContentsId } from '../ipc/ui' +import { + createRendererRecoveryGuard, + type RendererRecoveryEventName, + type RendererRecoverySummary +} from './renderer-recovery-guard' function forceRepaint(window: BrowserWindow): void { if (window.isDestroyed()) { @@ -121,7 +126,12 @@ type CreateMainWindowOptions = { onQuitAborted?: () => void onRendererProcessGone?: ( details: Electron.RenderProcessGoneDetails, - webContentsId: number + webContentsId: number, + recoverySummary: RendererRecoverySummary + ) => void + onRendererRecoveryEvent?: ( + name: RendererRecoveryEventName, + summary: RendererRecoverySummary ) => void /** Returns true when a renderer loss should be reported as a crash. Why: * intentional reload/update/quit paths can emit crash-like `killed` @@ -608,23 +618,33 @@ export function createMainWindow( } let rendererProcessGone = false let rendererRecoveryTimer: ReturnType | null = null + const rendererRecoveryGuard = createRendererRecoveryGuard({ + onEvent: opts?.onRendererRecoveryEvent + }) const clearRendererRecoveryTimer = (): void => { if (rendererRecoveryTimer) { clearTimeout(rendererRecoveryTimer) rendererRecoveryTimer = null } } - const scheduleRendererRecovery = (details: Electron.RenderProcessGoneDetails): void => { - if ( - rendererRecoveryTimer || - !details || - !isCrashReportReason(details.reason) || - windowClosing || - opts?.getIsQuitting?.() || - opts?.shouldRecoverRenderer?.(details, rendererWebContentsId) === false || - mainWindow.isDestroyed() - ) { - return + const shouldAttemptRendererRecovery = (details: Electron.RenderProcessGoneDetails): boolean => + Boolean( + details && + isCrashReportReason(details.reason) && + !windowClosing && + !opts?.getIsQuitting?.() && + opts?.shouldRecoverRenderer?.(details, rendererWebContentsId) !== false && + !mainWindow.isDestroyed() + ) + const scheduleRendererRecovery = ( + details: Electron.RenderProcessGoneDetails + ): RendererRecoverySummary | null => { + if (rendererRecoveryTimer || !shouldAttemptRendererRecovery(details)) { + return null + } + const recoveryDecision = rendererRecoveryGuard.canScheduleAutomaticRecovery() + if (!recoveryDecision.allowed) { + return recoveryDecision.summary } rendererRecoveryTimer = setTimeout(() => { rendererRecoveryTimer = null @@ -641,6 +661,7 @@ export function createMainWindow( // back to a usable window instead of needing a full relaunch. loadMainWindow(mainWindow) }, 250) + return recoveryDecision.summary } mainWindow.webContents.on('render-process-gone', (_event, details) => { rendererProcessGone = true @@ -648,24 +669,38 @@ export function createMainWindow( resetTerminalInputFocus() resetFloatingTerminalInputFocus() resetShortcutRecorderFocus() + if (!details) { + return + } + // Why: expected reload/update teardown should not consume automatic + // recovery budget; otherwise manual reloads can suppress the next real crash. + let recoverySummary = rendererRecoveryGuard.getSummary() + if (shouldAttemptRendererRecovery(details)) { + rendererRecoveryGuard.recordProcessGone({ + reason: details.reason, + exitCode: details.exitCode ?? null + }) + recoverySummary = scheduleRendererRecovery(details) ?? rendererRecoveryGuard.getSummary() + } // Why: macOS can report BrowserWindow teardown as renderer `killed`/SIGKILL // after a confirmed close; that is window lifecycle noise, not a crash. if ( !windowClosing && opts?.shouldRecordRendererCrash?.(details, rendererWebContentsId) !== false ) { - opts?.onRendererProcessGone?.(details, rendererWebContentsId) + opts?.onRendererProcessGone?.(details, rendererWebContentsId, recoverySummary) } if (!windowClosing) { console.error('[window] Renderer process gone; close confirmation will be bypassed', details) } - scheduleRendererRecovery(details) }) mainWindow.webContents.on('destroyed', () => { resetMarkdownEditorFocus() resetTerminalInputFocus() resetFloatingTerminalInputFocus() resetShortcutRecorderFocus() + clearRendererRecoveryTimer() + rendererRecoveryGuard.dispose() }) mainWindow.webContents.on('did-start-navigation', (_e, _url, _isInPlace, isMainFrame) => { if (isMainFrame) { @@ -678,6 +713,9 @@ export function createMainWindow( mainWindow.webContents.on('did-finish-load', () => { rendererProcessGone = false clearRendererRecoveryTimer() + // Why: a load that crashes immediately is still part of the loop; only a + // stable main-frame period resets automatic recovery dampening. + rendererRecoveryGuard.onStableLoad() }) const doubleTapDetector = new ModifierDoubleTapDetector() @@ -1004,6 +1042,8 @@ export function createMainWindow( // would otherwise make the post-update relaunch come up at minWidth × // minHeight (issue surfaced in v1.3.26-rc2). windowClosing = true + clearRendererRecoveryTimer() + rendererRecoveryGuard.dispose() if (boundsTimer) { clearTimeout(boundsTimer) boundsTimer = null @@ -1015,6 +1055,8 @@ export function createMainWindow( // window:close-requested. Let Cmd+Q / OS close complete instead of // trapping the user in a blank, unquittable window. windowClosing = true + clearRendererRecoveryTimer() + rendererRecoveryGuard.dispose() if (boundsTimer) { clearTimeout(boundsTimer) boundsTimer = null @@ -1121,6 +1163,7 @@ export function createMainWindow( floatingTerminalInputFocused = false shortcutRecorderFocused = false clearRendererRecoveryTimer() + rendererRecoveryGuard.dispose() ipcMain.removeListener(trafficLightChannel, onSyncTrafficLights) ipcMain.removeListener(minimizeChannel, onMinimize) ipcMain.removeListener(maximizeChannel, onMaximize) diff --git a/src/main/window/renderer-recovery-guard.test.ts b/src/main/window/renderer-recovery-guard.test.ts new file mode 100644 index 00000000000..3a34f660ea0 --- /dev/null +++ b/src/main/window/renderer-recovery-guard.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it, vi } from 'vitest' +import { + createRendererRecoveryGuard, + type RendererRecoveryEventName +} from './renderer-recovery-guard' + +describe('renderer recovery guard', () => { + it('permits the first automatic recoveries and suppresses later events in the TTL', () => { + let now = 1_000 + const events: RendererRecoveryEventName[] = [] + const guard = createRendererRecoveryGuard({ + now: () => now, + loopWindowMs: 60_000, + maxAutomaticRecoveries: 2, + cooldownMs: 300_000, + onEvent: (name) => events.push(name) + }) + + guard.recordProcessGone({ reason: 'crashed', exitCode: 5 }) + expect(guard.canScheduleAutomaticRecovery().allowed).toBe(true) + now += 1_000 + guard.recordProcessGone({ reason: 'oom', exitCode: -1 }) + expect(guard.canScheduleAutomaticRecovery().allowed).toBe(true) + now += 1_000 + const summary = guard.recordProcessGone({ reason: 'killed', exitCode: 1 }) + const decision = guard.canScheduleAutomaticRecovery() + + expect(summary).toMatchObject({ + totalProcessGoneCount: 3, + recentProcessGoneCount: 3, + lastReason: 'killed', + lastExitCode: 1 + }) + expect(decision.allowed).toBe(false) + expect(decision.summary).toMatchObject({ + suppressedRecoveryCount: 1, + degraded: true, + degradedUntil: now + 300_000 + }) + expect(events).toEqual([ + 'renderer_recovery_scheduled', + 'renderer_recovery_scheduled', + 'renderer_recovery_suppressed' + ]) + }) + + it('allows recovery after the cooldown and loop window expire', () => { + let now = 10_000 + const guard = createRendererRecoveryGuard({ + now: () => now, + loopWindowMs: 60_000, + maxAutomaticRecoveries: 2, + cooldownMs: 300_000 + }) + + for (const reason of ['crashed', 'oom', 'killed']) { + guard.recordProcessGone({ reason, exitCode: 1 }) + guard.canScheduleAutomaticRecovery() + now += 1_000 + } + + now += 301_000 + guard.recordProcessGone({ reason: 'crashed', exitCode: 2 }) + expect(guard.canScheduleAutomaticRecovery()).toMatchObject({ + allowed: true, + summary: { recentProcessGoneCount: 1, degraded: false } + }) + }) + + it('resets recent loop state after a stable load period', () => { + vi.useFakeTimers() + const events: { name: RendererRecoveryEventName; recent: number; degraded: boolean }[] = [] + const guard = createRendererRecoveryGuard({ + stableLoadMs: 30_000, + onEvent: (name, summary) => + events.push({ name, recent: summary.recentProcessGoneCount, degraded: summary.degraded }) + }) + + guard.recordProcessGone({ reason: 'crashed', exitCode: 5 }) + guard.canScheduleAutomaticRecovery() + guard.onStableLoad() + vi.advanceTimersByTime(29_999) + expect(guard.getSummary().recentProcessGoneCount).toBe(1) + + vi.advanceTimersByTime(1) + + expect(guard.getSummary()).toMatchObject({ + totalProcessGoneCount: 1, + recentProcessGoneCount: 0, + degraded: false + }) + expect(events.at(-1)).toEqual({ + name: 'renderer_recovery_stable', + recent: 0, + degraded: false + }) + }) + + it('clears the stable load timer on dispose', () => { + vi.useFakeTimers() + const onEvent = vi.fn() + const guard = createRendererRecoveryGuard({ + stableLoadMs: 30_000, + onEvent + }) + + guard.recordProcessGone({ reason: 'crashed', exitCode: 5 }) + guard.onStableLoad() + guard.dispose() + vi.advanceTimersByTime(30_000) + + expect(onEvent).not.toHaveBeenCalledWith('renderer_recovery_stable', expect.anything()) + }) +}) diff --git a/src/main/window/renderer-recovery-guard.ts b/src/main/window/renderer-recovery-guard.ts new file mode 100644 index 00000000000..86f0bb2ddb2 --- /dev/null +++ b/src/main/window/renderer-recovery-guard.ts @@ -0,0 +1,148 @@ +export type RendererRecoveryEventName = + | 'renderer_recovery_scheduled' + | 'renderer_recovery_suppressed' + | 'renderer_recovery_stable' + +export type RendererRecoverySummary = { + totalProcessGoneCount: number + recentProcessGoneCount: number + suppressedRecoveryCount: number + lastReason: string + lastExitCode: number | null + degradedUntil: number | null + degraded: boolean +} + +export type RendererRecoveryGuardOptions = { + loopWindowMs?: number + maxAutomaticRecoveries?: number + cooldownMs?: number + stableLoadMs?: number + now?: () => number + setTimer?: (callback: () => void, delayMs: number) => NodeJS.Timeout + clearTimer?: (timer: NodeJS.Timeout) => void + onEvent?: (name: RendererRecoveryEventName, summary: RendererRecoverySummary) => void +} + +export const RECOVERY_LOOP_WINDOW_MS = 60_000 +export const MAX_AUTOMATIC_RECOVERIES_PER_WINDOW = 2 +export const RECOVERY_COOLDOWN_MS = 5 * 60_000 +export const STABLE_LOAD_MS = 30_000 + +export type RendererRecoveryGuard = ReturnType + +const EMPTY_LAST_REASON = 'none' + +function cloneSummary(summary: RendererRecoverySummary): RendererRecoverySummary { + return { ...summary } +} + +export function createRendererRecoveryGuard(options: RendererRecoveryGuardOptions = {}) { + const loopWindowMs = options.loopWindowMs ?? RECOVERY_LOOP_WINDOW_MS + const maxAutomaticRecoveries = + options.maxAutomaticRecoveries ?? MAX_AUTOMATIC_RECOVERIES_PER_WINDOW + const cooldownMs = options.cooldownMs ?? RECOVERY_COOLDOWN_MS + const stableLoadMs = options.stableLoadMs ?? STABLE_LOAD_MS + const now = options.now ?? Date.now + const setTimer = + options.setTimer ?? ((callback: () => void, delayMs: number) => setTimeout(callback, delayMs)) + const clearTimer = options.clearTimer ?? clearTimeout + + let stableLoadTimer: NodeJS.Timeout | null = null + let disposed = false + let totalProcessGoneCount = 0 + let suppressedRecoveryCount = 0 + let processGoneTimes: number[] = [] + let lastReason = EMPTY_LAST_REASON + let lastExitCode: number | null = null + let degradedUntil: number | null = null + + const clearStableLoadTimer = (): void => { + if (stableLoadTimer) { + clearTimer(stableLoadTimer) + stableLoadTimer = null + } + } + + const pruneRecentProcessGoneTimes = (timestamp: number): void => { + processGoneTimes = processGoneTimes.filter((seenAt) => timestamp - seenAt <= loopWindowMs) + } + + const getSummary = (timestamp = now()): RendererRecoverySummary => { + pruneRecentProcessGoneTimes(timestamp) + const degraded = degradedUntil !== null && timestamp < degradedUntil + return { + totalProcessGoneCount, + recentProcessGoneCount: processGoneTimes.length, + suppressedRecoveryCount, + lastReason, + lastExitCode, + degradedUntil, + degraded + } + } + + const emit = (name: RendererRecoveryEventName, timestamp = now()): void => { + options.onEvent?.(name, cloneSummary(getSummary(timestamp))) + } + + return { + recordProcessGone(details: { + reason: string + exitCode?: number | null + }): RendererRecoverySummary { + const timestamp = now() + clearStableLoadTimer() + pruneRecentProcessGoneTimes(timestamp) + totalProcessGoneCount += 1 + processGoneTimes.push(timestamp) + lastReason = details.reason + lastExitCode = details.exitCode ?? null + return cloneSummary(getSummary(timestamp)) + }, + + canScheduleAutomaticRecovery(): { allowed: boolean; summary: RendererRecoverySummary } { + const timestamp = now() + const summary = getSummary(timestamp) + if (summary.degraded || summary.recentProcessGoneCount > maxAutomaticRecoveries) { + if (!summary.degraded) { + degradedUntil = timestamp + cooldownMs + } + suppressedRecoveryCount += 1 + const suppressedSummary = cloneSummary(getSummary(timestamp)) + emit('renderer_recovery_suppressed', timestamp) + return { allowed: false, summary: suppressedSummary } + } + emit('renderer_recovery_scheduled', timestamp) + return { allowed: true, summary: cloneSummary(summary) } + }, + + onStableLoad(): void { + if (disposed) { + return + } + clearStableLoadTimer() + const hadRecoveryState = processGoneTimes.length > 0 || degradedUntil !== null + stableLoadTimer = setTimer(() => { + stableLoadTimer = null + if (disposed) { + return + } + processGoneTimes = [] + degradedUntil = null + if (hadRecoveryState) { + emit('renderer_recovery_stable') + } + }, stableLoadMs) + }, + + getSummary(): RendererRecoverySummary { + return cloneSummary(getSummary()) + }, + + dispose(): void { + disposed = true + clearStableLoadTimer() + } + } +} diff --git a/src/renderer/src/components/task-page-github-work-item-status.ts b/src/renderer/src/components/task-page-github-work-item-status.ts index a8028e6a43b..ab9d0361032 100644 --- a/src/renderer/src/components/task-page-github-work-item-status.ts +++ b/src/renderer/src/components/task-page-github-work-item-status.ts @@ -65,8 +65,5 @@ export function getTaskPageGitHubPRIconTone(item: GitHubWorkItemStatusItem): str return 'text-purple-600 dark:text-purple-300' case 'closed': return 'text-rose-600 dark:text-rose-300' - default: - // Fallback for any unexpected state - return 'text-muted-foreground' } }