From c86e49259408ce30dc8122b5adc7bbe1794d2f76 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 29 Jul 2026 02:26:51 -0700 Subject: [PATCH] feat(diagnostics): name the nested container that grows in memory breadcrumbs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 53 renderer heap-exhaustion crash reports could not localize the leak they prove exists, because summarizeStateCollectionSizes walks only top-level state keys. Anything growing INSIDE a slice value, or outside the store entirely, is invisible — diffComments has no top-level key at all. Adds a bounded recursive walker registered as a contributor on the EXISTING memory-profile path (no new timer; runs at most twice per session, at the 60% and 80% one-shot highwater thresholds), plus a registry so long-lived module-level containers self-report their size. Diagnostics only. This stops no crash. Bounded four ways, because it runs on the main thread of a renderer already at 95% of its heap: entry-visit and key-scan budgets, depth 3, and a wall-clock deadline — for-in materializes V8's whole enumeration cache before yielding key one, so node budgets alone bounded count but not time (3353ms -> 112ms on a store of 300k-key objects). Costs 0.93ms and 17.6 KiB on a realistic store. Emits shapes, sizes and path labels only, never user values: dictionary keys collapse to `[]` so repo names, branch names and paths cannot reach a breadcrumb that gets uploaded with crash reports. --- .../src/components/PullRequestPage.tsx | 9 + .../terminal-delivery-watchdog.ts | 3 + .../module-singleton-size-registry.test.ts | 109 ++++ .../src/lib/module-singleton-size-registry.ts | 68 +++ .../lib/nested-collection-size-walker.test.ts | 493 ++++++++++++++++++ .../src/lib/nested-collection-size-walker.ts | 386 ++++++++++++++ .../src/lib/nested-container-shape.test.ts | 126 +++++ .../src/lib/nested-container-shape.ts | 151 ++++++ .../src/runtime/sync-runtime-graph.ts | 9 + .../src/runtime/web-session-tabs-sync.ts | 9 + src/renderer/src/store/index.ts | 17 + 11 files changed, 1380 insertions(+) create mode 100644 src/renderer/src/lib/module-singleton-size-registry.test.ts create mode 100644 src/renderer/src/lib/module-singleton-size-registry.ts create mode 100644 src/renderer/src/lib/nested-collection-size-walker.test.ts create mode 100644 src/renderer/src/lib/nested-collection-size-walker.ts create mode 100644 src/renderer/src/lib/nested-container-shape.test.ts create mode 100644 src/renderer/src/lib/nested-container-shape.ts diff --git a/src/renderer/src/components/PullRequestPage.tsx b/src/renderer/src/components/PullRequestPage.tsx index 029476d4e0d..ea8b45f61cb 100644 --- a/src/renderer/src/components/PullRequestPage.tsx +++ b/src/renderer/src/components/PullRequestPage.tsx @@ -226,6 +226,7 @@ import { type TaskSourceContext } from '../../../shared/task-source-context' import { translate } from '@/i18n/i18n' +import { registerModuleSingletonSize } from '@/lib/module-singleton-size-registry' import { getSettingsForRepoRuntimeOwner } from '@/lib/repo-runtime-owner' // Why: the item URL is the only host-aware repository identity present on every work item across IPC. @@ -1469,6 +1470,10 @@ const workItemDetailsCache = new Map() // Why: useSyncExternalStore snapshot stability relies on every cache write replacing the entry object identity (delete+set). const workItemDetailsCacheListeners = new Set<() => void>() +registerModuleSingletonSize('prPage.workItemDetailsCache', workItemDetailsCache) +// Why registered: a listener Set that only grows is a mounted-component leak, +// and it retains every closure's captured props. +registerModuleSingletonSize('prPage.workItemDetailsListeners', workItemDetailsCacheListeners) function subscribeWorkItemDetailsCache(listener: () => void): () => void { workItemDetailsCacheListeners.add(listener) return () => { @@ -1653,6 +1658,10 @@ type PRFileContentCacheEntry = { } const prFileContentCache = new Map() let prFileContentCacheBytes = 0 +// Why registered: byte-capped, but the cap only holds if eviction actually runs; +// reporting both entries and bytes proves it in a crash report rather than assuming. +registerModuleSingletonSize('prPage.fileContentCache', prFileContentCache) +registerModuleSingletonSize('prPage.fileContentCacheKB', () => prFileContentCacheBytes / 1024) function getUtf8ByteCount(value: string): number { let byteCount = 0 diff --git a/src/renderer/src/components/terminal-pane/terminal-delivery-watchdog.ts b/src/renderer/src/components/terminal-pane/terminal-delivery-watchdog.ts index c9b0d8c29e5..756dcb48d52 100644 --- a/src/renderer/src/components/terminal-pane/terminal-delivery-watchdog.ts +++ b/src/renderer/src/components/terminal-pane/terminal-delivery-watchdog.ts @@ -15,6 +15,7 @@ * output flows or while no PTY delivery is expected). */ import { e2eConfig } from '@/lib/e2e-config' +import { registerModuleSingletonSize } from '@/lib/module-singleton-size-registry' import type { PtyRendererDeliveryHealthReply } from '../../../../shared/pty-renderer-delivery-health' import { redactPtyIdForDiagnostics } from '../../../../shared/pty-delivery-diagnostics' import { deliverPulledPtyModelRestoreMarkers } from './pty-model-restore-channel' @@ -44,6 +45,8 @@ type TerminalDeliveryWatchdogDeps = { } const receivedPtyCharTotals = new Map() +// Why registered: one entry per PTY ever seen, deleted only on explicit detach. +registerModuleSingletonSize('watchdog.receivedPtyCharTotals', receivedPtyCharTotals) let receivedPtyDataEventCount = 0 let blackholePtyPushDelivery = false diff --git a/src/renderer/src/lib/module-singleton-size-registry.test.ts b/src/renderer/src/lib/module-singleton-size-registry.test.ts new file mode 100644 index 00000000000..4eed9539e8f --- /dev/null +++ b/src/renderer/src/lib/module-singleton-size-registry.test.ts @@ -0,0 +1,109 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { + _resetModuleSingletonSizesForTests, + collectModuleSingletonSizes, + registerModuleSingletonSize +} from './module-singleton-size-registry' + +afterEach(() => { + _resetModuleSingletonSizesForTests() +}) + +describe('collectModuleSingletonSizes', () => { + it('reports live container sizes, not the size at registration time', () => { + const cache = new Map() + registerModuleSingletonSize('runtime.cache', cache) + + expect(collectModuleSingletonSizes()).toEqual({}) + + cache.set('a', 1) + cache.set('b', 2) + + expect(collectModuleSingletonSizes()).toEqual({ 'runtime.cache': 2 }) + }) + + it('reads Maps and Sets alike, plus derived numeric sources', () => { + registerModuleSingletonSize('m', new Map([['a', 1]])) + registerModuleSingletonSize('s', new Set([1, 2, 3])) + registerModuleSingletonSize('derived', () => 42.6) + + expect(collectModuleSingletonSizes()).toEqual({ m: 1, s: 3, derived: 43 }) + }) + + it('contains a throwing source instead of losing the other counts', () => { + registerModuleSingletonSize('broken', () => { + throw new Error('boom') + }) + registerModuleSingletonSize('healthy', new Set([1, 2])) + + expect(collectModuleSingletonSizes()).toEqual({ 'broken.error': 1, healthy: 2 }) + }) + + it('skips non-finite sizes rather than emitting junk fields', () => { + registerModuleSingletonSize('nan', () => Number.NaN) + registerModuleSingletonSize('infinite', () => Number.POSITIVE_INFINITY) + registerModuleSingletonSize('real', () => 5) + + expect(collectModuleSingletonSizes()).toEqual({ real: 5 }) + }) + + it('unregisters cleanly and does not clobber a re-registered name', () => { + const dispose = registerModuleSingletonSize('a', new Set([1])) + registerModuleSingletonSize('a', new Set([1, 2])) + + // The stale disposer must not remove the newer registration. + dispose() + + expect(collectModuleSingletonSizes()).toEqual({ a: 2 }) + }) + + it('refuses registrations past the cap so one module cannot crowd out the rest', () => { + for (let index = 0; index < 32; index += 1) { + registerModuleSingletonSize(`s${index}`, new Set([1])) + } + registerModuleSingletonSize('overflow', new Set([1, 2, 3])) + + const counts = collectModuleSingletonSizes() + + expect(Object.keys(counts)).toHaveLength(32) + expect(counts.overflow).toBeUndefined() + }) + + it('rejects an oversized name that would blow the breadcrumb key budget', () => { + registerModuleSingletonSize('x'.repeat(49), new Set([1])) + + expect(collectModuleSingletonSizes()).toEqual({}) + }) + + it('returns a no-op disposer for a rejected registration', () => { + registerModuleSingletonSize('kept', new Set([1])) + const rejected = registerModuleSingletonSize('', new Set([1])) + + expect(() => rejected()).not.toThrow() + expect(collectModuleSingletonSizes()).toEqual({ kept: 1 }) + }) +}) + +describe('production wiring', () => { + // No reset here: the assertion depends on the module's own top-level + // registration, which is the thing under test. + it('registers the watchdog PTY table, and its count tracks real activity', async () => { + const watchdog = await import('@/components/terminal-pane/terminal-delivery-watchdog') + const read = (): number | undefined => + collectModuleSingletonSizes()['watchdog.receivedPtyCharTotals'] + + // Empty registrations are skipped, so a real count is what proves the wiring. + expect(read()).toBeUndefined() + try { + watchdog.recordPtyDataReceived('pty-a', 12) + watchdog.recordPtyDataReceived('pty-b', 34) + + expect(read()).toBe(2) + } finally { + watchdog.clearReceivedPtyCharTotal('pty-a') + watchdog.clearReceivedPtyCharTotal('pty-b') + } + + expect(read()).toBeUndefined() + }) +}) diff --git a/src/renderer/src/lib/module-singleton-size-registry.ts b/src/renderer/src/lib/module-singleton-size-registry.ts new file mode 100644 index 00000000000..e148afea51e --- /dev/null +++ b/src/renderer/src/lib/module-singleton-size-registry.ts @@ -0,0 +1,68 @@ +/** + * Self-reported sizes for long-lived module-level containers. + * + * Why: the Zustand store is only part of the renderer's retained graph. Module + * scope holds Maps/Sets that live for the whole session and are invisible to any + * store walk, so a leak there names nothing in a crash report. Registering one + * is a single call next to the declaration and costs a `.size` read per memory + * breadcrumb — no timer, no subscription, no retention of the values. + * + * Opt-in on purpose: only containers whose growth would be a real leak signal + * belong here. Diagnostics-only; nothing reads these counts to make decisions. + */ + +type SizedContainer = { readonly size: number } + +type SingletonSizeSource = SizedContainer | (() => number) + +const sources = new Map() + +// Why: these bound the breadcrumb, not the renderer — one misfiled registration +// must not crowd out the store counts that share the profile budget. +const MAX_SINGLETONS = 32 +const MAX_NAME_LENGTH = 48 + +/** + * Registers a module-level container to report its entry count in memory + * breadcrumbs. Call once at module scope; `name` becomes the breadcrumb key. + * The returned disposer exists for tests and HMR. + */ +export function registerModuleSingletonSize( + name: string, + source: SingletonSizeSource +): () => void { + if (name.length === 0 || name.length > MAX_NAME_LENGTH) { + return () => undefined + } + if (!sources.has(name) && sources.size >= MAX_SINGLETONS) { + return () => undefined + } + sources.set(name, source) + return () => { + if (sources.get(name) === source) { + sources.delete(name) + } + } +} + +/** Current entry count of every registered singleton; skips empty and broken ones. */ +export function collectModuleSingletonSizes(): Record { + const counts: Record = {} + for (const [name, source] of sources) { + // Why: a container with a throwing accessor must not lose the other counts. + try { + const size = typeof source === 'function' ? source() : source.size + // Why skip zero: an idle registration should not spend a breadcrumb slot. + if (typeof size === 'number' && Number.isFinite(size) && size > 0) { + counts[name] = Math.round(size) + } + } catch { + counts[`${name}.error`] = 1 + } + } + return counts +} + +export function _resetModuleSingletonSizesForTests(): void { + sources.clear() +} diff --git a/src/renderer/src/lib/nested-collection-size-walker.test.ts b/src/renderer/src/lib/nested-collection-size-walker.test.ts new file mode 100644 index 00000000000..f2704f482fa --- /dev/null +++ b/src/renderer/src/lib/nested-collection-size-walker.test.ts @@ -0,0 +1,493 @@ +import { describe, expect, it } from 'vitest' +import { walkNestedCollectionSizes } from './nested-collection-size-walker' +import { summarizeStateCollectionSizes } from './renderer-memory-profile' + +type PaneEntry = { paneKey: string; state: string; stateHistory: { state: string; at: number }[] } + +// MAX_ENTRY_VISITS + MAX_KEYS_SCANNED from the walker; nodesVisited sums both. +const NODES_CEILING = 34_000 + +/** Store shaped like the real one: pane dictionary whose entries hold histories. */ +function buildStoreState(options: { + panes: number + historyPerPane: number + worktreesPerRepo?: number + commentsPerWorktree?: number +}): Record { + const agentStatusByPaneKey: Record = {} + for (let pane = 0; pane < options.panes; pane += 1) { + agentStatusByPaneKey[`tab-${pane}:leaf-${pane}`] = { + paneKey: `tab-${pane}:leaf-${pane}`, + state: 'working', + stateHistory: Array.from({ length: options.historyPerPane }, (_, index) => ({ + state: 'idle', + at: index + })) + } + } + const worktreesByRepo: Record = {} + // Real repo ids are randomUUID(), which is what makes the label collapse to `[]`. + worktreesByRepo['3f2a1c9e-8b74-4d21-9f0a-6c5e2b7d1a83'] = Array.from( + { length: options.worktreesPerRepo ?? 0 }, + (_, index) => ({ + id: `wt-${index}`, + diffComments: Array.from({ length: options.commentsPerWorktree ?? 0 }, (_, c) => ({ + id: `c-${c}` + })) + }) + ) + return { agentStatusByPaneKey, worktreesByRepo, activeWorktreeId: 'wt-0' } +} + +describe('the gap the walker closes', () => { + it('top-level summary reports an unchanged number while nested entries grow 40x', () => { + const before = buildStoreState({ panes: 20, historyPerPane: 1 }) + const after = buildStoreState({ panes: 20, historyPerPane: 40 }) + + const summaryBefore = summarizeStateCollectionSizes(before, 20) + const summaryAfter = summarizeStateCollectionSizes(after, 20) + + // The existing breadcrumb: identical before and after 780 new retained objects. + expect(summaryBefore).toEqual(summaryAfter) + expect(summaryAfter.agentStatusByPaneKey).toBe(20) + + const walkBefore = walkNestedCollectionSizes(before, 12) + const walkAfter = walkNestedCollectionSizes(after, 12) + + expect(walkBefore.counts['agentStatusByPaneKey[].stateHistory']).toBe(20) + expect(walkAfter.counts['agentStatusByPaneKey[].stateHistory']).toBe(800) + }) + + it('names a three-level-deep container the top-level summary cannot reach', () => { + const state = buildStoreState({ + panes: 1, + historyPerPane: 0, + worktreesPerRepo: 4, + commentsPerWorktree: 250 + }) + + // diffComments has no top-level state key at all, so it is unreported today. + expect(summarizeStateCollectionSizes(state, 20)).toEqual({ + agentStatusByPaneKey: 1, + worktreesByRepo: 1 + }) + + expect(walkNestedCollectionSizes(state, 12).counts['worktreesByRepo[][].diffComments']).toBe( + 1000 + ) + }) + + it('ranks the largest nested container first', () => { + const state = { + small: { a: { items: [1, 2, 3] } }, + big: { a: { items: Array.from({ length: 500 }, (_, index) => index) } } + } + + expect(Object.keys(walkNestedCollectionSizes(state, 1).counts)).toEqual(['big.a.items']) + }) + + it('does not spend breadcrumb slots re-reporting top-level keys', () => { + const walk = walkNestedCollectionSizes({ worktrees: [1, 2, 3] }, 12) + + expect(walk.counts).toEqual({}) + }) +}) + +describe('path labels', () => { + it('collapses id-like dictionary keys so one path names the whole family', () => { + const state = { + byId: Object.fromEntries( + Array.from({ length: 30 }, (_, index) => [ + `2f9c-${index}-uuid-like`, + { history: [1, 2, 3, 4] } + ]) + ) + } + + // `byId` itself is a top-level key the existing summary already reports. + expect(walkNestedCollectionSizes(state, 12).counts).toEqual({ 'byId[].history': 120 }) + }) + + it('keeps named properties distinct from dictionary entries', () => { + const state = { slice: { settings: { tabs: [1, 2], panes: [1, 2, 3] } } } + + // `slice.settings` is a struct, so its field count is not a collection size. + expect(walkNestedCollectionSizes(state, 12).counts).toEqual({ + 'slice.settings.tabs': 2, + 'slice.settings.panes': 3 + }) + }) + + it('never leaks dictionary keys into the label, which crash reporting does not sanitize', () => { + const secretish = '/Users/someone/work/acme-secret-repo' + const state = { + byPath: { + [secretish]: { comments: [1, 2, 3] }, + 'C:\\Users\\someone\\proj': { comments: [1, 2] } + } + } + + const paths = Object.keys(walkNestedCollectionSizes(state, 12).counts) + + expect(paths).toEqual(['byPath[].comments']) + expect(paths.join('|')).not.toContain('someone') + }) + + it('collapses a SMALL user-keyed dictionary, whose keys are repo and branch names', () => { + // The dangerous case: key-count alone reads 3 keys as a struct and emits + // them verbatim into a breadcrumb that gets uploaded to Slack. Real users + // have a handful of repos, so this shape is the common one, not the exotic one. + const state = { + settingsByRepo: { + acme_billing_secret: { branches: [1, 2, 3] }, + project_darkstar: { branches: [1, 2] }, + clientwork: { branches: [1] } + } + } + + const paths = Object.keys(walkNestedCollectionSizes(state, 12).counts) + + expect(paths).toEqual(['settingsByRepo[].branches']) + expect(paths.join('|')).not.toMatch(/acme|darkstar|clientwork/) + }) + + it('collapses a ONE-entry dictionary, where there is no sibling to compare shape against', () => { + // Repeated-shape detection cannot fire on a single entry, so the key syntax + // rule is the only thing left; a branch name must still not reach the label. + const state = { draftsByBranch: { feature_ceo_comp_model: { comments: [1, 2, 3] } } } + + const paths = Object.keys(walkNestedCollectionSizes(state, 12).counts) + + expect(paths).toEqual(['draftsByBranch[].comments']) + expect(paths.join('|')).not.toContain('ceo_comp') + }) + + it('collapses an accidentally-camelCase branch name when a sibling gives the set away', () => { + // The residual risk in key-syntax filtering: `myFeature` is a legal branch + // name AND a legal field name. Its siblings are what disambiguate. + // Entry shapes DIFFER, so repeated-shape detection cannot fire; the + // camelCase key would pass on its own. Only the user-shaped sibling saves it. + const state = { + draftsByBranch: { + myFeature: { comments: [1, 2, 3] }, + 'fix/login-crash': { comments: [1], resolved: true } + } + } + + const paths = Object.keys(walkNestedCollectionSizes(state, 12).counts) + + expect(paths).toEqual(['draftsByBranch[].comments']) + expect(paths.join('|')).not.toContain('myFeature') + }) + + it('rejects non-camelCase key shapes that user data takes', () => { + const state = { + slice: { + 'kebab-branch-name': { items: [1] }, + snake_case_repo: { items: [1, 2] }, + '9f2a-4c81-uuid': { items: [1, 2, 3] }, + 'C:\\Users\\someone': { items: [1, 2, 3, 4] } + } + } + + for (const path of Object.keys(walkNestedCollectionSizes(state, 12).counts)) { + expect(path).not.toMatch(/kebab|snake|uuid|someone/) + } + }) + + it('drops a path label that would exceed the breadcrumb key budget', () => { + const longKey = 'k'.repeat(90) + const state = { slice: { [longKey]: { items: [1, 2, 3] } } } + + for (const path of Object.keys(walkNestedCollectionSizes(state, 12).counts)) { + expect(path.length).toBeLessThanOrEqual(64) + } + }) + + it('reports a dictionary field but not a struct field, at the same depth', () => { + const state = { + slice: { + struct: Object.fromEntries(Array.from({ length: 8 }, (_, i) => [`field${i}`, i])), + dictionary: Object.fromEntries(Array.from({ length: 400 }, (_, i) => [`id-${i}`, i])) + } + } + + expect(walkNestedCollectionSizes(state, 12).counts).toEqual({ 'slice.dictionary': 400 }) + }) +}) + +describe('safety rails', () => { + it('survives a cycle instead of hanging', () => { + const node: Record = { items: [1, 2, 3] } + node.self = node + const nested: Record = { child: node } + node.parent = nested + + const walk = walkNestedCollectionSizes({ root: nested }, 12) + + expect(walk.counts['root.child.items']).toBe(3) + }) + + it('counts a shared reference once rather than once per referrer', () => { + const shared = { entries: [1, 2, 3, 4, 5] } + const walk = walkNestedCollectionSizes({ a: { s: shared }, b: { s: shared } }, 12) + + expect(walk.counts['a.s.entries']).toBe(5) + expect(walk.counts['b.s.entries']).toBeUndefined() + }) + + it('never enters class instances, so xterm/fiber-shaped fan-out is unreachable', () => { + // Real fibers/xterm hold their fan-out in OWN instance fields, which for-in + // enumerates — a prototype getter would not exercise the guard at all. + class FiberLike { + readonly child = { grandchild: Array.from({ length: 500 }, () => 0) } + readonly stateNode = { buffers: Array.from({ length: 500 }, () => 0) } + readonly memoizedState = { queue: Array.from({ length: 500 }, () => 0) } + } + const walk = walkNestedCollectionSizes({ terminals: { pane: new FiberLike() } }, 12) + + // Nothing under the instance may be reported, at any depth. + expect(walk.counts).toEqual({}) + }) + + it('does not descend through a Map VALUE that is a class instance', () => { + class TerminalLike { + readonly buffer = { lines: Array.from({ length: 900 }, () => 0) } + } + const byPane = new Map([['pane-1', new TerminalLike()]]) + + const walk = walkNestedCollectionSizes({ slice: { byPane } }, 12) + + expect(walk.counts).toEqual({ 'slice.byPane': 1 }) + }) + + it('never enters DOM-shaped or React-element-shaped objects', () => { + const domLike = { nodeType: 1, childNodes: Array.from({ length: 500 }, () => ({})) } + const reactLike = { $$typeof: Symbol.for('react.element'), props: { children: [1, 2, 3] } } + + const walk = walkNestedCollectionSizes({ a: { dom: domLike }, b: { el: reactLike } }, 12) + + expect(walk.counts['a.dom.childNodes']).toBeUndefined() + expect(walk.counts['b.el.props']).toBeUndefined() + }) + + it('reports partial results with truncated set instead of throwing on a hostile getter', () => { + const hostile = { + list: [1, 2, 3], + get boom(): never { + throw new Error('nope') + } + } + const walk = walkNestedCollectionSizes({ slice: hostile }, 12) + + // Degrades to partial results: what it read before the throw, plus the flag. + expect(walk.truncated).toBe(true) + expect(walk.counts).toEqual({ 'slice.list': 3 }) + }) + + it('isolates a throwing slice so its SIBLING slices are still reported', () => { + const state = { + hostile: { + get boom(): never { + throw new Error('nope') + } + }, + healthy: { comments: Array.from({ length: 40 }, () => 0) }, + alsoHealthy: { history: Array.from({ length: 25 }, () => 0) } + } + + const walk = walkNestedCollectionSizes(state, 12) + + // Without per-frame isolation the throw would abort the whole walk and + // these siblings would vanish from the breadcrumb. + expect(walk.counts['healthy.comments']).toBe(40) + expect(walk.counts['alsoHealthy.history']).toBe(25) + expect(walk.truncated).toBe(true) + }) + + it('survives every exotic value a real store could hold', () => { + const nullPrototype = Object.create(null) as Record + nullPrototype.items = [1, 2, 3] + const selfReferential: unknown[] = [1] + selfReferential.push(selfReferential) + + const exotic: unknown[] = [ + new Proxy({}, { ownKeys: () => { throw new Error('nope') } }), + new Proxy({ a: {} }, { get: () => { throw new Error('nope') } }), + { get boom(): never { throw new Error('nope') } }, + { get nodeType(): never { throw new Error('nope') } }, + { [Symbol.toPrimitive]: () => { throw new Error('nope') }, items: [1, 2] }, + nullPrototype, + new Uint8Array(64), + new WeakMap(), + Promise.resolve(1), + Object.freeze({ a: Object.freeze({ items: [1, 2, 3] }) }), + selfReferential, + { d: new Date(), r: /x/g, items: [1] }, + Array.from({ length: 16 }) + ] + + for (const value of exotic) { + expect(() => walkNestedCollectionSizes({ root: value }, 12)).not.toThrow() + } + }) + + it('does not throw on non-object roots', () => { + for (const root of [null, undefined, 7, 'x', Symbol('s')]) { + expect(walkNestedCollectionSizes(root, 12).counts).toEqual({}) + } + }) +}) + +describe('node budget', () => { + it('stays under the budget on a pathologically wide store and flags truncation', () => { + const state = { + wide: Object.fromEntries( + Array.from({ length: 50_000 }, (_, index) => [`k${index}`, { items: [1, 2, 3] }]) + ) + } + + const walk = walkNestedCollectionSizes(state, 12) + const reported = walk.counts['wide[].items'] ?? 0 + + expect(walk.nodesVisited).toBeLessThanOrEqual(NODES_CEILING) + expect(walk.estimated).toBe(true) + // Past the key cap the contract is a flagged LOWER BOUND, not an estimate: + // truncated says "at least this much", which still names the container and + // still moves when it grows. Reporting the sample size would not. + expect(walk.truncated).toBe(true) + expect(reported).toBeGreaterThan(50_000) + expect(reported).toBeLessThanOrEqual(150_000) + }) + + it('keeps rising as a huge dictionary grows, instead of saturating', () => { + const build = (panes: number): Record => ({ + panes: Object.fromEntries( + Array.from({ length: panes }, (_, index) => [`p${index}`, { history: [1, 2, 3, 4, 5] }]) + ) + }) + + const at3k = walkNestedCollectionSizes(build(3000), 12).counts['panes[].history'] ?? 0 + const at9k = walkNestedCollectionSizes(build(9000), 12).counts['panes[].history'] ?? 0 + + // A saturating count would report the same number twice and hide the leak. + expect(at9k).toBeGreaterThan(at3k * 2) + }) + + it('bounds ENTRY visits on a wide-and-deep store whose keys are never scanned', () => { + // Why arrays specifically: the key-scan budget does not apply to them, so + // this is the only shape where MAX_ENTRY_VISITS is the sole thing standing + // between the walker and a half-million entry reads. Mutating that constant + // away leaves every other test in this file passing. + const state: Record = {} + for (let slice = 0; slice < 256; slice += 1) { + state[`slice${slice}`] = Array.from({ length: 2000 }, () => ({ items: [1, 2, 3] })) + } + + const walk = walkNestedCollectionSizes(state, 12) + + // 512_000 entries exist; an unbudgeted walk reads all of them. + expect(walk.nodesVisited).toBeLessThanOrEqual(10_000) + expect(walk.estimated).toBe(true) + // Bounded, but still names the container rather than going silent. + expect(Object.keys(walk.counts).some((path) => path.endsWith('[].items'))).toBe(true) + }) + + it('stays under the budget on a pathologically deep store', () => { + let deep: Record = { items: [1, 2, 3] } + for (let level = 0; level < 5000; level += 1) { + deep = { child: deep } + } + + const walk = walkNestedCollectionSizes({ root: deep }, 12) + + expect(walk.nodesVisited).toBeLessThanOrEqual(NODES_CEILING) + expect(Object.keys(walk.counts).every((path) => path.split('.').length <= 5)).toBe(true) + }) + + it('bounds the reported path count regardless of store fan-out', () => { + const state = Object.fromEntries( + Array.from({ length: 200 }, (_, index) => [ + `slice${index}`, + { [`child${index}`]: { items: [1, 2] } } + ]) + ) + + expect(Object.keys(walkNestedCollectionSizes(state, 12).counts)).toHaveLength(12) + }) + + it('scales a sampled count toward the true population', () => { + const state = { + panes: Object.fromEntries( + Array.from({ length: 2000 }, (_, index) => [`p${index}`, { history: [1, 2, 3, 4, 5] }]) + ) + } + const walk = walkNestedCollectionSizes(state, 12) + const reported = walk.counts['panes[].history'] ?? 0 + + expect(walk.estimated).toBe(true) + // True total is 10_000; sampling must land within 2x, not report the sample. + expect(reported).toBeGreaterThan(5000) + expect(reported).toBeLessThan(20_000) + }) + + it('bounds WALL TIME when a store holds objects too big for a node budget to help', () => { + // Why node budgets are not enough: `for...in` materializes V8's enumeration + // cache for the whole object before yielding key one, so an 8-key sample of + // a 300k-key value costs O(300k). This shape measured 3.3s unbounded. + const slice: Record = {} + for (let entry = 0; entry < 10; entry += 1) { + const huge: Record = {} + for (let field = 0; field < 300_000; field += 1) { + huge[`f${field}`] = field + } + slice[`entry${entry}`] = huge + } + + const startedAt = performance.now() + const walk = walkNestedCollectionSizes({ slice }, 12) + const elapsed = performance.now() - startedAt + + // Generous vs. the ~75ms measured: one for-in is uninterruptible, so the + // deadline bounds how many are STARTED, not how long one runs. + expect(elapsed).toBeLessThan(600) + expect(walk.truncated).toBe(true) + }) + + it('is exact and unflagged when the store fits inside the budget', () => { + const state = buildStoreState({ panes: 30, historyPerPane: 20 }) + const walk = walkNestedCollectionSizes(state, 12) + + expect(walk.estimated).toBe(false) + expect(walk.truncated).toBe(false) + expect(walk.counts['agentStatusByPaneKey[].stateHistory']).toBe(600) + }) +}) + +describe('cost on a realistically-sized store', () => { + it('completes a realistic store well inside a frame budget', () => { + const state = buildStoreState({ + panes: 300, + historyPerPane: 20, + worktreesPerRepo: 120, + commentsPerWorktree: 30 + }) + + // Warm up so the measurement is steady-state, not first-call JIT. + for (let run = 0; run < 20; run += 1) { + walkNestedCollectionSizes(state, 12) + } + const startedAt = performance.now() + const runs = 200 + for (let run = 0; run < runs; run += 1) { + walkNestedCollectionSizes(state, 12) + } + const msPerRun = (performance.now() - startedAt) / runs + const walk = walkNestedCollectionSizes(state, 12) + + // Generous vs. the measured cost so slow CI machines don't flake; the real + // guarantee is the node budget asserted above, which is deterministic. + expect(msPerRun).toBeLessThan(15) + expect(walk.nodesVisited).toBeLessThanOrEqual(NODES_CEILING) + }) +}) diff --git a/src/renderer/src/lib/nested-collection-size-walker.ts b/src/renderer/src/lib/nested-collection-size-walker.ts new file mode 100644 index 00000000000..1e0e1fa1a5a --- /dev/null +++ b/src/renderer/src/lib/nested-collection-size-walker.ts @@ -0,0 +1,386 @@ +/** + * Bounded recursive size walker for renderer_memory_highwater breadcrumbs. + * + * Why: summarizeStateCollectionSizes counts only top-level state keys, so a + * collection growing INSIDE a slice value — agentStatusByPaneKey[].stateHistory, + * worktreesByRepo[][].diffComments — is invisible in every crash report we have. + * This names the nested container instead. Pure diagnostics: it reads state, + * changes nothing, and runs only while a memory breadcrumb is already being built + * (at most twice per session, since each highwater threshold fires once). + * + * Counts can be MAGNITUDES rather than exact totals: past the caps the walker + * samples sibling entries and scales, setting `estimated`. A run that hits a + * budget reports what it has and sets `truncated` rather than throwing or hanging. + */ + +import { + hasOnlyFieldNameShapedKeys, + hasRepeatedEntryShape, + isCountableContainer, + isFieldNameShaped, + isWalkableContainer +} from './nested-container-shape' + +export type NestedCollectionSizeWalk = { + /** Largest nested containers by entry count, keyed by path label. */ + counts: Record + /** Entry reads plus key reads; the walker's entire cost is proportional to this. */ + nodesVisited: number + /** A budget cut coverage short, so counts are lower bounds. */ + truncated: boolean + /** At least one count was scaled up from a sample of sibling entries. */ + estimated: boolean +} + +// Two budgets because the two kinds of work cost differently: descending into an +// entry is real recursion, while counting a dictionary's keys is a flat ~0.12us +// per key. Both are measured in nested-collection-size-walker.test.ts. +const MAX_ENTRY_VISITS = 4000 +const MAX_KEYS_SCANNED = 30_000 +const MAX_KEYS_PER_CONTAINER = 20_000 +const MAX_FRAME_DEPTH = 3 +const MAX_FRAMES_PER_LEVEL = 128 +const MAX_ENTRIES_PER_FRAME = 256 +const MIN_ENTRIES_PER_FRAME = 8 +const MAX_TRACKED_PATHS = 96 +const MAX_PATH_LENGTH = 64 +/** + * Above this many keys a plain object is a dictionary (sample it, scale it, and + * report its size); at or below it the object is a struct, so every field is + * visited unsampled — otherwise a heterogeneous field like `stateHistory` could + * be missed entirely by sampling, and its field count is noise, not a leak. + */ +const MAX_STRUCT_KEYS = 32 +/** + * Wall-clock backstop, because the node budgets cannot bound time on their own: + * `for...in` materializes V8's enumeration cache for the WHOLE object before + * yielding the first key, so breaking after 8 keys still costs O(object size). + * A store holding 300k-key objects measured 3.3s without this. Blocking the + * main thread of an already-dying renderer is worse than a partial breadcrumb, + * so the walk reports what it has and sets `truncated`. + */ +const MAX_WALK_MS = 25 + +type Frame = { + container: object + path: string + depth: number + /** Entry count of `container`, measured when it was queued. */ + size: number + /** True when `container` is homogeneous, so sampling its entries is valid. */ + samplable: boolean + /** True when every sampled value is a container, i.e. keys are data, not code. */ + keysAreData: boolean + /** Population this frame stands in for, from sampled ancestors. */ + scale: number +} + +type WalkState = { + totals: Map + /** performance.now() past which the walk stops, whatever budget remains. */ + deadline: number + seen: WeakSet + entryVisits: number + keysScanned: number + /** Frames discarded at the current level's width cap; reset per level. */ + droppedFrames: number + truncated: boolean + estimated: boolean +} + +/** Walks `root` a few levels deep and reports its biggest nested collections. */ +export function walkNestedCollectionSizes(root: unknown, limit: number): NestedCollectionSizeWalk { + const state: WalkState = { + totals: new Map(), + deadline: now() + MAX_WALK_MS, + seen: new WeakSet(), + entryVisits: 0, + keysScanned: 0, + droppedFrames: 0, + truncated: false, + estimated: false + } + // Why: a diagnostic must never be the thing that breaks crash reporting. + try { + if (isWalkableContainer(root)) { + state.seen.add(root) + walkLevels(describeFrame(root, '', 0, 1, state), state) + } + } catch { + state.truncated = true + } + return { + counts: largestPaths(state.totals, limit), + nodesVisited: state.entryVisits + state.keysScanned, + truncated: state.truncated, + estimated: state.estimated + } +} + +function walkLevels(rootFrame: Frame, state: WalkState): void { + let level: Frame[] = [rootFrame] + while (level.length > 0) { + if (state.entryVisits >= MAX_ENTRY_VISITS || isOutOfTime(state)) { + state.truncated = true + return + } + // Why half: a fat top level must not consume the whole budget before + // reaching the depth where the leak actually hides. The last level has no + // deeper level to save for, so it may use everything that is left. + const remaining = MAX_ENTRY_VISITS - state.entryVisits + const levelAllowance = + level[0].depth >= MAX_FRAME_DEPTH ? remaining : Math.ceil(remaining / 2) + const perFrame = clamp( + Math.floor(levelAllowance / level.length), + MIN_ENTRIES_PER_FRAME, + MAX_ENTRIES_PER_FRAME + ) + const next: Frame[] = [] + state.droppedFrames = 0 + for (const frame of level) { + if (state.entryVisits >= MAX_ENTRY_VISITS || isOutOfTime(state)) { + state.truncated = true + break + } + scanFrame(frame, perFrame, next, state) + } + applyDroppedFrameScale(next, state.droppedFrames) + level = next + } +} + +/** Visits a sample of one container's entries and queues their containers. */ +function scanFrame(frame: Frame, perFrame: number, next: Frame[], state: WalkState): void { + // Why structs ignore perFrame: their fields are heterogeneous, so a partial + // scan could silently omit the one field that is leaking. + const target = frame.samplable ? Math.min(frame.size, perFrame) : frame.size + if (target <= 0) { + return + } + if (target < frame.size) { + state.estimated = true + } + const scale = (frame.scale * frame.size) / target + try { + scanFrameEntries(frame, target, scale, next, state) + } catch { + // Why: one hostile getter or exotic proxy must not lose the whole walk. + state.truncated = true + } +} + +function scanFrameEntries( + frame: Frame, + target: number, + scale: number, + next: Frame[], + state: WalkState +): void { + const container = frame.container + if (Array.isArray(container)) { + for (let index = 0; index < target; index += 1) { + state.entryVisits += 1 + visitChild(container[index], null, frame, scale, next, state) + } + return + } + let seen = 0 + if (container instanceof Map) { + for (const value of container.values()) { + if (seen >= target) { + return + } + seen += 1 + state.entryVisits += 1 + visitChild(value, null, frame, scale, next, state) + } + return + } + // Why no hasOwn: isWalkableContainer already proved the prototype is + // Object.prototype or null, neither of which has enumerable properties. + for (const key in container) { + if (seen >= target) { + return + } + seen += 1 + state.entryVisits += 1 + visitChild((container as Record)[key], key, frame, scale, next, state) + } +} + +/** Records a container child's scaled size and queues it for the next level. */ +function visitChild( + value: unknown, + key: string | null, + frame: Frame, + scale: number, + next: Frame[], + state: WalkState +): void { + if (!isCountableContainer(value)) { + return + } + const child = value as object + // Why: cycle safety, and shared references (EMPTY_* sentinels) counted once. + if (state.seen.has(child)) { + return + } + state.seen.add(child) + const path = childPath(frame, key) + if (path === null) { + return + } + const child_ = describeFrame(child, path, frame.depth + 1, scale, state) + if (child_.size <= 0) { + return + } + // Why depth >= 1 only: depth-0 children are the top-level keys the `store` + // contributor already reports exactly, so recording them here adds nothing. + // Why reportable: a struct's field count is not a collection size. + if (frame.depth >= 1 && isReportableContainer(child, child_.size)) { + addPathTotal(state, path, child_.size * scale) + } + if (frame.depth < MAX_FRAME_DEPTH && isWalkableContainer(child)) { + queueFrame(next, child_, state) + } +} + +/** Measures a container's size and classifies it as samplable or struct-shaped. */ +function describeFrame( + container: object, + path: string, + depth: number, + scale: number, + state: WalkState +): Frame { + if (Array.isArray(container)) { + // Array/Map indices are positional, never user strings, so `[]` always applies. + return { container, path, depth, size: container.length, samplable: true, keysAreData: true, scale } + } + if (container instanceof Map || container instanceof Set) { + return { container, path, depth, size: container.size, samplable: true, keysAreData: true, scale } + } + const size = countKeys(container, state) + const samplable = size > MAX_STRUCT_KEYS + // Why the deadline short-circuits to `true`: classification costs a for-in per + // sampled entry, and `true` is the privacy-safe answer — keys get collapsed to + // `[]` rather than printed. Running out of time must never start printing keys. + // Why both rules: repeated entry shapes catch a dictionary of records, and the + // key-syntax scan catches the rest (a dictionary whose entries differ, or has + // only one). Either one alone leaves a hole a real branch name fits through. + const keysAreData = + samplable || + isOutOfTime(state) || + hasRepeatedEntryShape(container, () => isOutOfTime(state)) || + !hasOnlyFieldNameShapedKeys(container) + return { container, path, depth, size, samplable, keysAreData, scale } +} + +/** + * Exact key count when the budget allows; otherwise a lower bound with + * `truncated` set, because a silently saturating count would hide the growth + * that is the entire signal. + * + * Why a per-container cap on top of the global one: without it the first huge + * dictionary eats the whole budget and every later container measures 0, which + * drops them from the report entirely — the same blindness this walker exists + * to fix. Capping per container keeps every path visible. + */ +function countKeys(container: object, state: WalkState): number { + let size = 0 + for (const _key in container) { + size += 1 + state.keysScanned += 1 + if (size >= MAX_KEYS_PER_CONTAINER || state.keysScanned >= MAX_KEYS_SCANNED) { + state.truncated = true + break + } + } + return size +} + +function childPath(frame: Frame, key: string | null): string | null { + if (frame.depth === 0) { + return key !== null && key.length <= MAX_PATH_LENGTH ? key : null + } + // Why the parent's shape decides: inside a dictionary every key is user data + // (repo name, branch, path), so collapsing to `[]` both yields one path for + // the family and keeps user values out of a breadcrumb we upload to Slack. + const named = !frame.keysAreData && key !== null && isFieldNameShaped(key) + const path = named ? `${frame.path}.${key}` : `${frame.path}[]` + return path.length <= MAX_PATH_LENGTH ? path : null +} + +function addPathTotal(state: WalkState, path: string, value: number): void { + const existing = state.totals.get(path) + if (existing !== undefined) { + state.totals.set(path, existing + value) + return + } + if (state.totals.size >= MAX_TRACKED_PATHS) { + state.truncated = true + return + } + state.totals.set(path, value) +} + +/** + * Queues a frame, reservoir-sampling once the level is full so that dropping + * frames scales the survivors instead of silently under-reporting the level. + */ +function queueFrame(next: Frame[], frame: Frame, state: WalkState): void { + if (next.length < MAX_FRAMES_PER_LEVEL) { + next.push(frame) + return + } + state.estimated = true + state.droppedFrames += 1 + // Deterministic stride keeps the sample spread across the level without an RNG. + const replaced = state.droppedFrames % MAX_FRAMES_PER_LEVEL + next[replaced] = frame +} + +/** Scales a level's surviving frames to stand in for the ones that were dropped. */ +function applyDroppedFrameScale(level: Frame[], dropped: number): void { + if (dropped <= 0 || level.length === 0) { + return + } + const factor = (level.length + dropped) / level.length + for (const frame of level) { + frame.scale *= factor + } +} + +/** Real collections only; a struct's field count is noise in a leak breadcrumb. */ +function isReportableContainer(value: object, size: number): boolean { + return ( + Array.isArray(value) || value instanceof Map || value instanceof Set || size > MAX_STRUCT_KEYS + ) +} + +function largestPaths(totals: Map, limit: number): Record { + if (limit <= 0 || totals.size === 0) { + return {} + } + const counts: Record = {} + for (const [path, value] of [...totals].sort((a, b) => b[1] - a[1]).slice(0, limit)) { + counts[path] = Math.round(value) + } + return counts +} + +function isOutOfTime(state: WalkState): boolean { + return now() >= state.deadline +} + +// Why not performance.now() directly: it is absent in some worker/test realms, +// and a diagnostic must degrade to "no deadline" rather than throw. +function now(): number { + return typeof performance === 'object' && typeof performance.now === 'function' + ? performance.now() + : 0 +} + +function clamp(value: number, min: number, max: number): number { + return Math.min(Math.max(value, min), max) +} diff --git a/src/renderer/src/lib/nested-container-shape.test.ts b/src/renderer/src/lib/nested-container-shape.test.ts new file mode 100644 index 00000000000..34dd57320ac --- /dev/null +++ b/src/renderer/src/lib/nested-container-shape.test.ts @@ -0,0 +1,126 @@ +import { describe, expect, it } from 'vitest' +import { + hasOnlyFieldNameShapedKeys, + hasRepeatedEntryShape, + isFieldNameShaped, + isPlainObjectShape, + isWalkableContainer +} from './nested-container-shape' + +describe('isFieldNameShaped', () => { + it('accepts the lowerCamelCase field names this store actually uses', () => { + for (const key of ['stateHistory', 'diffComments', 'tabs', 'browserUrlHistory']) { + expect(isFieldNameShaped(key)).toBe(true) + } + }) + + it('rejects every shape user data takes, because labels ship to Slack', () => { + for (const key of [ + 'my-feature-branch', + 'acme_billing_secret', + '/Users/someone/repo', + 'C:\\Users\\someone', + '9f2a1c9e-8b74-4d21-9f0a-6c5e2b7d1a83', + 'API_TOKEN', + 'Repo Name With Spaces', + 'café', + '' + ]) { + expect(isFieldNameShaped(key)).toBe(false) + } + }) + + it('caps key length at 32 so one label cannot crowd the breadcrumb budget', () => { + expect(isFieldNameShaped(`a${'b'.repeat(31)}`)).toBe(true) + expect(isFieldNameShaped(`a${'b'.repeat(32)}`)).toBe(false) + }) +}) + +describe('hasRepeatedEntryShape', () => { + it('calls repeated records a dictionary, so its user-derived keys collapse', () => { + expect( + hasRepeatedEntryShape({ + acme_secret: { branches: [1], head: 'x' }, + other_repo: { branches: [2], head: 'y' } + }) + ).toBe(true) + }) + + it('calls a mixed-field record a struct, so its field names survive', () => { + expect(hasRepeatedEntryShape({ paneKey: 'p', state: 'idle', stateHistory: [1, 2] })).toBe(false) + }) + + it('treats differing field sets as a struct rather than a dictionary', () => { + expect(hasRepeatedEntryShape({ a: { x: 1 }, b: { y: 2 } })).toBe(false) + }) + + it('cannot infer repetition from a single entry', () => { + expect(hasRepeatedEntryShape({ onlyOne: { branches: [1] } })).toBe(false) + }) + + it('treats a struct whose values are arrays as a struct', () => { + // Arrays carry no field names, so nothing proves the keys are interchangeable. + expect(hasRepeatedEntryShape({ tabs: [1, 2], panes: [3] })).toBe(false) + }) + + it('assumes keys are data when an entry cannot be read', () => { + const hostile = { + get boom(): never { + throw new Error('nope') + } + } + + expect(hasRepeatedEntryShape(hostile)).toBe(true) + }) + + it('ignores field order when matching entry shapes', () => { + expect(hasRepeatedEntryShape({ a: { x: 1, y: 2 }, b: { y: 3, x: 4 } })).toBe(true) + }) +}) + +describe('isPlainObjectShape', () => { + it('rejects class instances, which is what keeps xterm and fibers unreachable', () => { + class Terminal { + readonly buffer = {} + } + + expect(isPlainObjectShape(new Terminal())).toBe(false) + }) + + it('rejects DOM-shaped and React-element-shaped objects that are prototype-plain', () => { + expect(isPlainObjectShape({ nodeType: 1 })).toBe(false) + expect(isPlainObjectShape({ $$typeof: Symbol.for('react.element') })).toBe(false) + }) + + it('accepts plain and null-prototype data objects', () => { + expect(isPlainObjectShape({ a: 1 })).toBe(true) + expect(isPlainObjectShape(Object.create(null))).toBe(true) + }) + + it('rejects scalars, null, and exotic built-ins', () => { + for (const value of [null, undefined, 7, 'x', new Promise(() => {}), new Uint8Array(2)]) { + expect(isPlainObjectShape(value)).toBe(false) + } + }) +}) + +describe('isWalkableContainer', () => { + it('admits arrays, Maps, and plain objects only', () => { + expect(isWalkableContainer([1])).toBe(true) + expect(isWalkableContainer(new Map())).toBe(true) + expect(isWalkableContainer({})).toBe(true) + // A Set is countable but not walkable: its entries are values, not a shape. + expect(isWalkableContainer(new Set([1]))).toBe(false) + expect(isWalkableContainer(new WeakMap())).toBe(false) + }) +}) + +describe('hasOnlyFieldNameShapedKeys', () => { + it('condemns the whole container when one sibling key is user-shaped', () => { + expect(hasOnlyFieldNameShapedKeys({ myFeature: 1, 'fix/login': 2 })).toBe(false) + }) + + it('accepts a container whose keys are all field names', () => { + expect(hasOnlyFieldNameShapedKeys({ stateHistory: 1, diffComments: 2 })).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/nested-container-shape.ts b/src/renderer/src/lib/nested-container-shape.ts new file mode 100644 index 00000000000..07b25dc6206 --- /dev/null +++ b/src/renderer/src/lib/nested-container-shape.ts @@ -0,0 +1,151 @@ +/** + * Shape classification for the nested collection size walker. + * + * Two questions, both answered without reading a single user VALUE: is this + * object safe to walk at all, and are its keys field names (safe to print) or + * user data (must be collapsed to `[]`)? The second question is a privacy + * boundary — walker output is uploaded with crash reports — so it lives here + * rather than inline, where it is easy to test in isolation. + */ + +/** Entries sampled to decide dictionary-vs-struct; shape is uniform, so few suffice. */ +export const HOMOGENEITY_SAMPLE = 8 + +/** + * Strict lowerCamelCase, the convention every field name in this store follows. + * Deliberately narrower than "valid identifier": it rejects the shapes user data + * actually takes — snake_case and kebab branch names, paths, UUIDs, SCREAMING + * keys — so a one-entry dictionary cannot smuggle a key into a Slack-bound + * breadcrumb on the one path where repeated-shape detection has nothing to + * compare against. A rejected key costs a label; an accepted one costs a leak. + */ +const NAMED_PROPERTY_KEY = /^[a-z][A-Za-z0-9]{0,31}$/ + +export function isFieldNameShaped(key: string): boolean { + return NAMED_PROPERTY_KEY.test(key) +} + +/** Anything whose entries we can count: containers, plus Sets (counted, not entered). */ +export function isCountableContainer(value: unknown): boolean { + return value instanceof Set || isWalkableContainer(value) +} + +/** True for containers safe to iterate: arrays, Maps, and plain objects. */ +export function isWalkableContainer(value: unknown): value is object { + return Array.isArray(value) || value instanceof Map || isPlainObjectShape(value) +} + +/** + * Plain data objects only. Class instances (xterm Terminals, React fibers, DOM + * nodes, Promises, typed arrays, Zustand stores) fail the prototype test and are + * never entered, which is what keeps huge fan-out and getter side effects out. + */ +export function isPlainObjectShape(value: unknown): boolean { + if (typeof value !== 'object' || value === null) { + return false + } + const prototype = Object.getPrototypeOf(value) + if (prototype !== Object.prototype && prototype !== null) { + return false + } + const shape = value as { $$typeof?: unknown; nodeType?: unknown } + // Why: React elements and cross-realm DOM wrappers can be prototype-plain yet + // reach the entire tree through _owner / parentNode. + return shape.$$typeof === undefined && typeof shape.nodeType !== 'number' +} + +/** + * Distinguishes a dictionary from a struct by whether its ENTRIES REPEAT A + * SHAPE, because key count alone cannot: a user with three repos has a + * three-key dictionary whose keys are repo names, and those keys would + * otherwise be uploaded to Slack verbatim. + * + * A dictionary's values are interchangeable records — two or more plain objects + * with the same field names (`{[repo]: {branches}}`). A struct's fields are not + * (`{paneKey, state, stateHistory}` mixes scalars with an array; `{tabs, panes}` + * holds arrays, which carry no field names to match). Erring toward "dictionary" + * costs a field name in the label; erring the other way leaks user data. + */ +export function hasRepeatedEntryShape(container: object, isExpired?: () => boolean): boolean { + let signature: string | null = null + let sampled = 0 + try { + for (const key in container) { + // Why checked per entry: each one costs a for-in over a value that may be + // huge, so the cap alone bounds the entry COUNT but not the time. Bailing + // returns `true`, which collapses keys — the privacy-safe direction. + if (sampled >= HOMOGENEITY_SAMPLE || isExpired?.() === true) { + break + } + const value = (container as Record)[key] + // Why plain objects only: arrays and Maps expose no field names, so a + // struct of arrays is indistinguishable from a dictionary of arrays. + if (!isPlainObjectShape(value)) { + return false + } + const entrySignature = fieldSignature(value as object) + if (signature === null) { + signature = entrySignature + } else if (signature !== entrySignature) { + return false + } + sampled += 1 + } + } catch { + // Why: an unreadable entry proves nothing about shape; treat keys as data. + return true + } + // Why two: a single entry cannot demonstrate repetition, and a one-entry + // container is never the leak this walker is looking for. A run cut short by + // the deadline lands here with sampled < 2 only if it never got going, in + // which case the strict key-syntax rule is what keeps user data out. + return sampled >= 2 +} + +/** + * True when EVERY sampled key looks like a field name, not just the one being + * labelled. + * + * Why sibling-wide: a lone key is weak evidence, because user data can be + * accidentally camelCase — a branch named `myFeature`, a repo named `orcaWeb`. + * Siblings are what give it away: real branch and repo sets almost always + * contain at least one `feature/x`, `fix-y`, or `snake_name`, and one such + * sibling now condemns the whole container's keys instead of only itself. + */ +export function hasOnlyFieldNameShapedKeys(container: object): boolean { + let sampled = 0 + try { + for (const key in container) { + if (sampled >= HOMOGENEITY_SAMPLE) { + break + } + if (!isFieldNameShaped(key)) { + return false + } + sampled += 1 + } + } catch { + return false + } + return true +} + +/** + * Field names of one entry, order-independent. + * + * Why for-in rather than Object.keys: both are O(entry size), but for-in reuses + * V8's enumeration cache — which the pre-existing top-level summary has already + * paid for on these same objects — instead of allocating a fresh key array in a + * renderer that is at 95% of its heap. The per-entry cost is bounded by the + * caller's deadline, not by this cap. + */ +function fieldSignature(entry: object): string { + const fields: string[] = [] + for (const key in entry) { + if (fields.length >= HOMOGENEITY_SAMPLE) { + break + } + fields.push(key) + } + return fields.sort().join(',') +} diff --git a/src/renderer/src/runtime/sync-runtime-graph.ts b/src/renderer/src/runtime/sync-runtime-graph.ts index 33cb27ed74a..23d3d430302 100644 --- a/src/renderer/src/runtime/sync-runtime-graph.ts +++ b/src/renderer/src/runtime/sync-runtime-graph.ts @@ -7,6 +7,7 @@ import { import { warnTerminalLifecycleAnomaly } from '@/components/terminal-pane/terminal-lifecycle-diagnostics' import { getEagerPtyBufferHandle } from '@/components/terminal-pane/pty-dispatcher' import { createBrowserUuid } from '@/lib/browser-uuid' +import { registerModuleSingletonSize } from '@/lib/module-singleton-size-registry' import type { PaneManager } from '@/lib/pane-manager/pane-manager' import { resolveLeafIdForManager } from '@/lib/pane-manager/pane-key-resolution' import { getSystemPrefersDark, resolveEffectiveTerminalAppearance } from '@/lib/terminal-theme' @@ -81,6 +82,8 @@ type AgentStatusProjectionCache = { const registeredTabs = new Map() // Why: registration time suppresses the "no live transport" warning during the async PTY-connect window; after the grace period it's a real stuck state. const tabRegisteredAt = new Map() +registerModuleSingletonSize('syncGraph.registeredTabs', registeredTabs) +registerModuleSingletonSize('syncGraph.tabRegisteredAt', tabRegisteredAt) const NO_TRANSPORT_GRACE_MS = 10_000 const EMPTY_ACTIVE_BROWSER_TAB_ID_BY_WORKTREE: AppState['activeBrowserTabIdByWorktree'] = {} const EMPTY_BROWSER_TABS_BY_WORKTREE: AppState['browserTabsByWorktree'] = {} @@ -104,6 +107,12 @@ const mobileSessionSnapshotCacheByWorktree = new Map< string, { content: unknown; snapshot: RuntimeMobileSessionTabsSnapshot } >() +// Why registered: retains a full session snapshot per worktree, so unbounded +// growth here is heap, not just bookkeeping. +registerModuleSingletonSize( + 'syncGraph.mobileSnapshotCache', + mobileSessionSnapshotCacheByWorktree +) // Structural equality under JSON-serialization semantics (undefined-valued // keys are absent), so version reuse matches a JSON fingerprint exactly diff --git a/src/renderer/src/runtime/web-session-tabs-sync.ts b/src/renderer/src/runtime/web-session-tabs-sync.ts index 022ba9503b5..409084c4944 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync.ts @@ -31,6 +31,7 @@ import type { OpenFile } from '../store/slices/editor' import { isTerminalLeafId, makePaneKey, parsePaneKey } from '../../../shared/stable-pane-id' import { getRemoteRuntimePtyEnvironmentId, toRemoteRuntimePtyId } from './runtime-terminal-stream' import { sanitizeTerminalLayoutPaneTitlesForLabels } from '@/lib/terminal-pane-title-sanitization' +import { registerModuleSingletonSize } from '@/lib/module-singleton-size-registry' import { getExplicitRuntimeEnvironmentIdForWorktree, getRuntimeSessionMirrorEnvironmentIds @@ -104,6 +105,14 @@ const latestSessionTabsSnapshotByWorktree = new Map() const replayableSessionTabsSnapshotByWorktree = new Map() const lastHostTerminalTabCountByWorktree = new Map() const hostSessionTabIdByLocalKey = new Map() +// Why registered: keyed by worktree/tab with no eviction pass, so SSH sessions +// that churn worktrees would grow these silently. +registerModuleSingletonSize('webTabsSync.latestSnapshots', latestSessionTabsSnapshotByWorktree) +registerModuleSingletonSize( + 'webTabsSync.replayableSnapshots', + replayableSessionTabsSnapshotByWorktree +) +registerModuleSingletonSize('webTabsSync.hostTabIdByLocalKey', hostSessionTabIdByLocalKey) type TerminalSurface = RuntimeMobileSessionTerminalClientTab type ReadyTerminalSurface = RuntimeMobileSessionTerminalClientTab & { status: 'ready' } diff --git a/src/renderer/src/store/index.ts b/src/renderer/src/store/index.ts index 7565246db4c..1d8e8d217f9 100644 --- a/src/renderer/src/store/index.ts +++ b/src/renderer/src/store/index.ts @@ -48,6 +48,8 @@ import { registerRendererMemoryProfileContributor, summarizeStateCollectionSizes } from '@/lib/renderer-memory-profile' +import { walkNestedCollectionSizes } from '@/lib/nested-collection-size-walker' +import { collectModuleSingletonSizes } from '@/lib/module-singleton-size-registry' export const useAppStore = create()((...a) => { // Why: the inner api is only reachable here, before create() copies subscribe onto the hook. @@ -104,6 +106,21 @@ registerRendererMemoryProfileContributor('store', () => summarizeStateCollectionSizes(useAppStore.getState(), 20) ) +// Why: the top-level summary above cannot see growth INSIDE a slice value, which +// is why 53 heap-exhaustion reports named nothing. Diagnostics only — bounded +// walk, no fix. See nested-collection-size-walker.ts. +registerRendererMemoryProfileContributor('nested', () => { + const walk = walkNestedCollectionSizes(useAppStore.getState(), 12) + return { + ...walk.counts, + nodesVisited: walk.nodesVisited, + truncated: walk.truncated ? 1 : 0, + estimated: walk.estimated ? 1 : 0 + } +}) + +registerRendererMemoryProfileContributor('singleton', collectModuleSingletonSizes) + export type { AppState } from './types' // Why: exposes the Zustand store on window for console debugging (dev) and