From 58a37fb11a8ae6883352e4c24f37a3ca99d8bb04 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:46:50 -0700 Subject: [PATCH] fix(diagnostics): retain renderer heap profiles --- .../crash-breadcrumb-store.test.ts | 38 +++++++++++++++ .../crash-reporting/crash-breadcrumb-store.ts | 39 +++++++++++++-- .../src/lib/crash-diagnostics.test.ts | 13 +++-- src/renderer/src/lib/crash-diagnostics.ts | 48 ++++++++++++------- .../src/lib/renderer-memory-profile.test.ts | 21 ++++++++ .../src/lib/renderer-memory-profile.ts | 22 +++++++-- src/renderer/src/popout.tsx | 2 +- 7 files changed, 152 insertions(+), 31 deletions(-) diff --git a/src/main/crash-reporting/crash-breadcrumb-store.test.ts b/src/main/crash-reporting/crash-breadcrumb-store.test.ts index 944109a195c..4668032e12f 100644 --- a/src/main/crash-reporting/crash-breadcrumb-store.test.ts +++ b/src/main/crash-reporting/crash-breadcrumb-store.test.ts @@ -24,6 +24,44 @@ describe('crash breadcrumb store', () => { expect(snapshot[29].name).toBe('event_31') }) + it('retains bounded renderer high-water profiles across later activity', () => { + vi.useFakeTimers() + vi.setSystemTime(new Date('2026-07-22T12:00:00.000Z')) + recordCrashBreadcrumb('renderer_memory_highwater', { + rendererSurface: 'main', + thresholdPct: 80, + 'store.agentStatusByPaneKey': 500 + }) + for (let index = 0; index < 32; index += 1) { + vi.advanceTimersByTime(60_000) + recordCrashBreadcrumb('renderer_memory', { index }) + } + + const snapshot = getCrashBreadcrumbSnapshot() + + expect(snapshot).toHaveLength(30) + expect(snapshot[0]).toEqual( + expect.objectContaining({ + name: 'renderer_memory_highwater', + data: expect.objectContaining({ thresholdPct: 80 }) + }) + ) + expect(snapshot.at(-1)?.data).toEqual({ index: 31 }) + }) + + it('caps retained high-water profiles', () => { + for (let index = 0; index < 5; index += 1) { + recordCrashBreadcrumb('renderer_memory_highwater', { + rendererSurface: `surface-${index}`, + thresholdPct: 80 + }) + } + + expect( + getCrashBreadcrumbSnapshot().map((breadcrumb) => breadcrumb.data?.rendererSurface) + ).toEqual(['surface-1', 'surface-2', 'surface-3', 'surface-4']) + }) + it('redacts sensitive breadcrumb fields before they can be snapshotted', () => { recordCrashBreadcrumb('workspace_opened', { path: '/Users/alice/project', diff --git a/src/main/crash-reporting/crash-breadcrumb-store.ts b/src/main/crash-reporting/crash-breadcrumb-store.ts index 76cf875619f..ca31e2068d7 100644 --- a/src/main/crash-reporting/crash-breadcrumb-store.ts +++ b/src/main/crash-reporting/crash-breadcrumb-store.ts @@ -5,14 +5,26 @@ import { } from '../../shared/crash-reporting' const MAX_BREADCRUMBS = 30 +// Why: retain two thresholds for each renderer surface without growing the ring. +const MAX_RETAINED_BREADCRUMBS = 4 // Why: coalesceKey embeds an open-string agentType (length-trimmed only, never // enum-checked), so the key space is unbounded over a long multi-agent/SSH session. // Bound the coalesce map the same way ProcessGoneDedupe bounds its key map. const MAX_COALESCE_KEYS = 128 let breadcrumbs: CrashReportBreadcrumb[] = [] +let retainedBreadcrumbs = new Map() let coalescedBreadcrumbs = new Map() +function retainedBreadcrumbKey(breadcrumb: CrashReportBreadcrumb): string | null { + if (breadcrumb.name !== 'renderer_memory_highwater') { + return null + } + const surface = breadcrumb.data?.rendererSurface + const threshold = breadcrumb.data?.thresholdPct + return `${breadcrumb.name}:${String(surface)}:${String(threshold)}` +} + export function recordCrashBreadcrumb(name: string, data?: CrashReportBreadcrumbData): void { const sanitized = sanitizeCrashReportBreadcrumbs([ { @@ -25,6 +37,19 @@ export function recordCrashBreadcrumb(name: string, data?: CrashReportBreadcrumb if (!breadcrumb) { return } + const retainedKey = retainedBreadcrumbKey(breadcrumb) + if (retainedKey) { + retainedBreadcrumbs.delete(retainedKey) + retainedBreadcrumbs.set(retainedKey, breadcrumb) + while (retainedBreadcrumbs.size > MAX_RETAINED_BREADCRUMBS) { + const oldestKey = retainedBreadcrumbs.keys().next() + if (oldestKey.done) { + break + } + retainedBreadcrumbs.delete(oldestKey.value) + } + return + } breadcrumbs.push(breadcrumb) if (breadcrumbs.length > MAX_BREADCRUMBS) { breadcrumbs.shift() @@ -72,14 +97,20 @@ export function recordCoalescedCrashBreadcrumb({ } export function getCrashBreadcrumbSnapshot(): CrashReportBreadcrumb[] { - return breadcrumbs.map((breadcrumb) => ({ - ...breadcrumb, - ...(breadcrumb.data ? { data: { ...breadcrumb.data } } : {}) - })) + // Why: long sessions must retain threshold profiles without growing the 30-entry budget. + const retained = [...retainedBreadcrumbs.values()] + const recent = breadcrumbs.slice(-(MAX_BREADCRUMBS - retained.length)) + return [...retained, ...recent] + .sort((left, right) => left.createdAt.localeCompare(right.createdAt)) + .map((breadcrumb) => ({ + ...breadcrumb, + ...(breadcrumb.data ? { data: { ...breadcrumb.data } } : {}) + })) } export function clearCrashBreadcrumbsForTest(): void { breadcrumbs = [] + retainedBreadcrumbs = new Map() coalescedBreadcrumbs = new Map() } diff --git a/src/renderer/src/lib/crash-diagnostics.test.ts b/src/renderer/src/lib/crash-diagnostics.test.ts index e0f4e2513f4..4d578178a12 100644 --- a/src/renderer/src/lib/crash-diagnostics.test.ts +++ b/src/renderer/src/lib/crash-diagnostics.test.ts @@ -174,11 +174,13 @@ describe('renderer crash diagnostics', () => { }) it('emits one-shot renderer_memory_highwater breadcrumbs with profile counts', async () => { - const memory = (window.performance as { memory: Record }).memory + const memory = (window.performance as unknown as { memory: Record }).memory memory.usedJSHeapSize = 0.7 * memory.jsHeapSizeLimit + const getElementsByTagName = vi.fn(() => ({ length: 4321 })) + const querySelectorAll = vi.fn(() => ({ length: 6 })) vi.stubGlobal('document', { - getElementsByTagName: () => ({ length: 4321 }), - querySelectorAll: () => ({ length: 6 }) + getElementsByTagName, + querySelectorAll }) const profile = await import('./renderer-memory-profile') const unregister = profile.registerRendererMemoryProfileContributor('store', () => ({ @@ -195,6 +197,7 @@ describe('renderer crash diagnostics', () => { name: 'renderer_memory_highwater', data: expect.objectContaining({ thresholdPct: 60, + rendererSurface: 'main', domNodes: 4321, terminalElements: 6, 'store.worktrees': 12 @@ -219,12 +222,14 @@ describe('renderer crash diagnostics', () => { recordBreadcrumbMock.mockClear() diagnostics.installRendererCrashDiagnostics() expect(highwaterCalls()).toHaveLength(2) + expect(getElementsByTagName).toHaveBeenCalledTimes(3) + expect(querySelectorAll).toHaveBeenCalledTimes(3) unregister() }) it('skips highwater emission when heap readings are not finite', () => { - const memory = (window.performance as { memory: Record }).memory + const memory = (window.performance as unknown as { memory: Record }).memory memory.usedJSHeapSize = Number.NaN diagnostics.installRendererCrashDiagnostics() diff --git a/src/renderer/src/lib/crash-diagnostics.ts b/src/renderer/src/lib/crash-diagnostics.ts index 910134a4042..81c326c083f 100644 --- a/src/renderer/src/lib/crash-diagnostics.ts +++ b/src/renderer/src/lib/crash-diagnostics.ts @@ -8,11 +8,11 @@ import { collectRendererMemoryProfileCounts } from './renderer-memory-profile' const RENDERER_MEMORY_SAMPLE_INTERVAL_MS = 60_000 const BYTES_PER_MEGABYTE = 1024 * 1024 -// Why: OOM crash reports show the heap pinned at the limit with no clue what -// grew; a one-shot detailed breadcrumb per threshold names the leaking -// subsystem while costing nothing in healthy sessions. +// Why: one detailed breadcrumb per threshold names what grew before an OOM. const RENDERER_MEMORY_HIGHWATER_RATIOS = [0.6, 0.8] as const +type RendererSurface = 'main' | 'dashboard-popout' + type BrowserPerformanceMemory = { usedJSHeapSize?: number totalJSHeapSize?: number @@ -21,6 +21,7 @@ type BrowserPerformanceMemory = { let rendererCrashDiagnosticsInstalled = false let rendererMemoryInterval: number | null = null +let rendererSurface: RendererSurface = 'main' const emittedHighwaterRatios = new Set() // Why re-exported from a leaf module: terminal modules and their e2e-visible @@ -28,12 +29,13 @@ const emittedHighwaterRatios = new Set() // webview-registry baggage. See crash-breadcrumb-recorder.ts. export { recordRendererCrashBreadcrumb } from './crash-breadcrumb-recorder' -export function installRendererCrashDiagnostics(): void { +export function installRendererCrashDiagnostics(surface: RendererSurface = 'main'): void { if (rendererCrashDiagnosticsInstalled || typeof window === 'undefined') { return } rendererCrashDiagnosticsInstalled = true + rendererSurface = surface window.addEventListener('error', recordRendererError) window.addEventListener('unhandledrejection', recordRendererUnhandledRejection) @@ -62,6 +64,7 @@ function disposeRendererCrashDiagnostics(): void { rendererMemoryInterval = null } emittedHighwaterRatios.clear() + rendererSurface = 'main' } if (typeof import.meta !== 'undefined' && import.meta.hot) { @@ -131,24 +134,35 @@ function recordRendererMemoryHighwater(memory: BrowserPerformanceMemory): void { return } const ratio = used / limit + let crossedThreshold = false + for (const threshold of RENDERER_MEMORY_HIGHWATER_RATIOS) { + if (ratio >= threshold && !emittedHighwaterRatios.has(threshold)) { + crossedThreshold = true + break + } + } + if (!crossedThreshold) { + return + } + // Why: a single sample can cross both thresholds; profile the large heap once. + const profile = compactBreadcrumbData({ + rendererSurface, + usedHeapMB: toMegabytes(used), + totalHeapMB: toMegabytes(memory.totalJSHeapSize), + heapLimitMB: toMegabytes(limit), + domNodes: document.getElementsByTagName('*').length, + terminalElements: document.querySelectorAll('.xterm').length, + ...collectRendererMemoryProfileCounts() + }) for (const threshold of RENDERER_MEMORY_HIGHWATER_RATIOS) { if (ratio < threshold || emittedHighwaterRatios.has(threshold)) { continue } emittedHighwaterRatios.add(threshold) - recordRendererCrashBreadcrumb( - 'renderer_memory_highwater', - compactBreadcrumbData({ - thresholdPct: Math.round(threshold * 100), - usedHeapMB: toMegabytes(used), - totalHeapMB: toMegabytes(memory.totalJSHeapSize), - heapLimitMB: toMegabytes(limit), - // Why: DOM census is O(nodes); acceptable at most twice per session. - domNodes: document.getElementsByTagName('*').length, - terminalElements: document.querySelectorAll('.xterm').length, - ...collectRendererMemoryProfileCounts() - }) - ) + recordRendererCrashBreadcrumb('renderer_memory_highwater', { + ...profile, + thresholdPct: Math.round(threshold * 100) + }) } } diff --git a/src/renderer/src/lib/renderer-memory-profile.test.ts b/src/renderer/src/lib/renderer-memory-profile.test.ts index 59a04c18d6f..476b5ffbde1 100644 --- a/src/renderer/src/lib/renderer-memory-profile.test.ts +++ b/src/renderer/src/lib/renderer-memory-profile.test.ts @@ -53,6 +53,27 @@ describe('collectRendererMemoryProfileCounts', () => { expect(Object.keys(collectRendererMemoryProfileCounts())).toHaveLength(32) }) + + it('stops reading a runaway contributor after the output budget', () => { + let reads = 0 + const contribution = Object.fromEntries( + Array.from({ length: 500 }, (_, index) => [ + `key${index}`, + { + enumerable: true, + get: () => { + reads += 1 + return index + } + } + ]) + ) + const counts = Object.defineProperties({}, contribution) as Record + register('runaway', () => counts) + + expect(Object.keys(collectRendererMemoryProfileCounts())).toHaveLength(32) + expect(reads).toBe(32) + }) }) describe('summarizeStateCollectionSizes', () => { diff --git a/src/renderer/src/lib/renderer-memory-profile.ts b/src/renderer/src/lib/renderer-memory-profile.ts index 18fc7a4588e..8d8dad8b06b 100644 --- a/src/renderer/src/lib/renderer-memory-profile.ts +++ b/src/renderer/src/lib/renderer-memory-profile.ts @@ -33,14 +33,19 @@ export function collectRendererMemoryProfileCounts(): RendererMemoryProfileCount for (const [name, contributor] of contributors) { // Why: a broken contributor must never take down memory reporting itself. try { - let kept = 0 - for (const [key, value] of Object.entries(contributor())) { - if (kept >= MAX_COUNTS_PER_CONTRIBUTOR) { + const contribution = contributor() + let inspected = 0 + for (const key in contribution) { + if (inspected >= MAX_COUNTS_PER_CONTRIBUTOR) { break } + inspected += 1 + if (!Object.hasOwn(contribution, key)) { + continue + } + const value = contribution[key] if (typeof value === 'number' && Number.isFinite(value)) { counts[`${name}.${key}`] = value - kept += 1 } } } catch { @@ -80,7 +85,14 @@ function collectionSize(value: unknown): number | null { return value.size } if (typeof value === 'object' && value !== null) { - return Object.keys(value).length + let size = 0 + // Why: Object.keys allocates an array proportional to the leaking collection. + for (const key in value) { + if (Object.hasOwn(value, key)) { + size += 1 + } + } + return size } return null } diff --git a/src/renderer/src/popout.tsx b/src/renderer/src/popout.tsx index fc07304e091..e68565f509c 100644 --- a/src/renderer/src/popout.tsx +++ b/src/renderer/src/popout.tsx @@ -21,7 +21,7 @@ import type { GlobalSettings } from '../../shared/types' // theme, i18n, error boundary) rather than inheriting anything from the main // window. It shares the preload/window.api but not the DOM or JS context. recordRendererCrashBreadcrumb('popout_bootstrap_started', { dev: import.meta.env.DEV }) -installRendererCrashDiagnostics() +installRendererCrashDiagnostics('dashboard-popout') function applyPopoutAppearance(settings: GlobalSettings | null): void { applyDocumentTheme(settings?.theme ?? 'system', { disableTransitions: false })