diff --git a/src/main/daemon/osc7-uri-extraction.test.ts b/src/main/daemon/osc7-uri-extraction.test.ts index 0f64c1c806d..8e9a62b8b02 100644 --- a/src/main/daemon/osc7-uri-extraction.test.ts +++ b/src/main/daemon/osc7-uri-extraction.test.ts @@ -33,9 +33,7 @@ describe('OSC-7 URI extraction', () => { expect(execSpy).not.toHaveBeenCalled() }) - // The tail is parked per-PTY (orca-runtime osc7ScanTailByPtyId) and per - // daemon scanner until the next chunk, so an attached slice pins one whole - // source chunk per tracked PTY. + // Production persists one scan tail per PTY and daemon scanner. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried OSC scan tail', () => { @@ -46,7 +44,6 @@ describe('OSC-7 URI extraction', () => { const before = process.memoryUsage().heapUsed const carried = new Map() for (let index = 0; index < ptys; index += 1) { - // No terminator yet, so the tail is carried to the next chunk. const chunk = `${'x'.repeat(chunkChars)}\x1b]7;file://host/Users/dev/orca/wt-${index}` carried.set(`pty-${index}`, extractOscScanTail(chunk, 4096)) } @@ -54,7 +51,6 @@ describe('OSC-7 URI extraction', () => { const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(carried.get('pty-0')).toBe('\x1b]7;file://host/Users/dev/orca/wt-0') - // 8 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/main/daemon/osc7-uri-extraction.ts b/src/main/daemon/osc7-uri-extraction.ts index 03361aa6a40..d74485a0e89 100644 --- a/src/main/daemon/osc7-uri-extraction.ts +++ b/src/main/daemon/osc7-uri-extraction.ts @@ -67,10 +67,7 @@ export function extractLastOsc7Uri(data: string): string | null { return lastUri } -// Callers park this tail until the next chunk arrives — osc7ScanTailByPtyId in -// orca-runtime.ts, TerminalOscCwdTitleScanner.scanTail in the daemon — so the -// returned slice is detached; otherwise each carried tail pins a whole source -// chunk for as long as its PTY or scanner stays alive. +// Persisted scan tails must not retain their source PTY chunks. export function extractOscScanTail(input: string, limit: number): string { const lastOsc = input.lastIndexOf('\x1b]') const lastEscape = input.endsWith('\x1b') ? input.length - 1 : -1 diff --git a/src/main/daemon/terminal-mouse-mode-mirror.test.ts b/src/main/daemon/terminal-mouse-mode-mirror.test.ts index 10f2e48a6d6..b5cbefff3a0 100644 --- a/src/main/daemon/terminal-mouse-mode-mirror.test.ts +++ b/src/main/daemon/terminal-mouse-mode-mirror.test.ts @@ -34,8 +34,7 @@ describe('TerminalMouseModeMirror', () => { expect(mirror.sgrMousePixelsMode).toBe(false) }) - // One mirror per daemon session holds the scan tail until the next chunk, so - // an attached slice pins a whole PTY chunk per live session. + // Production persists one scan tail per daemon session. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried private-mode tail', () => { @@ -46,18 +45,15 @@ describe('TerminalMouseModeMirror', () => { const before = process.memoryUsage().heapUsed const mirrors = Array.from({ length: sessions }, () => { const mirror = new TerminalMouseModeMirror() - // Ends mid-DECSET, so the params tail is carried into the next chunk. mirror.scan(`${'x'.repeat(chunkChars)}\x1b[?1002;1006;100`) return mirror }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The carry must still complete the split DECSET across the boundary. mirrors[0]!.scan('3h') expect(mirrors[0]!.mouseTrackingMode).toBe('any') expect(mirrors[0]!.sgrMouseMode).toBe(true) - // 8 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/main/daemon/terminal-mouse-mode-mirror.ts b/src/main/daemon/terminal-mouse-mode-mirror.ts index 901db52da6b..89dcea39016 100644 --- a/src/main/daemon/terminal-mouse-mode-mirror.ts +++ b/src/main/daemon/terminal-mouse-mode-mirror.ts @@ -93,8 +93,7 @@ export class TerminalMouseModeMirror { } } - // Detached: one mirror per daemon session holds this tail until the next - // chunk, so an attached slice pins a whole PTY chunk per live session. + // Persisted per-session tails must not retain their source PTY chunks. private extractScanTail(input: string): string { const start = Math.max(input.lastIndexOf('\x1b'), input.lastIndexOf('\x9b')) if (start === -1) { diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index a475d3979e8..c5cf739ed8c 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -11,6 +11,7 @@ import { } from '../../shared/agent-detection' import { detachString } from '../../shared/detached-string' import { extractOscTitleScanTail } from '../../shared/osc-title-scan-tail' +import { retainTerminalPendingAnsi } from './terminal-pending-ansi' import { isServerDriveListRequest, listWindowsDrives } from './windows-drive-listing' import { extractLastOsc7Uri, extractOscScanTail } from '../daemon/osc7-uri-extraction' import { parseFileUriPathParts } from '../daemon/osc7-file-uri' @@ -8927,8 +8928,7 @@ export class OrcaRuntimeService { } const appendedLower = appendedText.toLowerCase() const keywordHit = WAIT_BLOCKED_KEYWORD_PATTERN.test(`${state.keywordCarry}${appendedLower}`) - // Detached: keywordCarry is parked per PTY until the next chunk, so an - // attached slice would pin the whole lowercased chunk per tracked PTY. + // Persisted keyword carries must not retain their source PTY chunks. state.keywordCarry = detachString(appendedLower.slice(-WAIT_BLOCKED_KEYWORD_CARRY_CHARS)) // Why the cap keeps the tail: the accumulated text only anchors boundary- // spanning prompt detection; anything past the tail cap has scrolled out @@ -32477,7 +32477,6 @@ const WAIT_BLOCKED_KEYWORD_CARRY_CHARS = 31 const MAX_TAIL_LINES = 2000 const MAX_TAIL_CHARS = 256 * 1024 const MAX_TAIL_PARTIAL_CHARS = 4000 -const MAX_TAIL_PENDING_ANSI_CHARS = 4096 const DEFAULT_TERMINAL_READ_LIMIT = 120 const MAX_TERMINAL_READ_LIMIT = 2000 const MAX_TERMINAL_PREVIEW_CHARS = 32 * 1024 @@ -34693,13 +34692,16 @@ function normalizeTerminalChunk( if (char === '\x1b') { appendTerminalNormalizedSpan(parts, combined, textStart, index) if (index + 1 >= combined.length) { - return { text: parts.join(''), pendingAnsi: combined.slice(index) } + return { + text: parts.join(''), + pendingAnsi: retainTerminalPendingAnsi(combined.slice(index)) + } } const parsed = parseAnsiControlSequence(combined, index) if (!parsed) { return { text: parts.join(''), - pendingAnsi: trimPendingAnsiControl(combined.slice(index)) + pendingAnsi: retainTerminalPendingAnsi(combined.slice(index)) } } if (parsed.kind === 'csi' && isTerminalPreviewLineControl(parsed)) { @@ -34763,15 +34765,6 @@ function terminalChunkNeedsNormalization(chunk: string): boolean { return false } -function trimPendingAnsiControl(value: string): string { - if (value.length <= MAX_TAIL_PENDING_ANSI_CHARS) { - return value - } - const introducer = value.slice(0, Math.min(2, value.length)) - const suffixBudget = Math.max(0, MAX_TAIL_PENDING_ANSI_CHARS - introducer.length) - return `${introducer}${value.slice(-suffixBudget)}` -} - function isTerminalPreviewLineControl(parsed: { final: string params: string diff --git a/src/main/runtime/orchestration/setup-completion-signal.test.ts b/src/main/runtime/orchestration/setup-completion-signal.test.ts index b918d5638c0..d56ea6a0b8c 100644 --- a/src/main/runtime/orchestration/setup-completion-signal.test.ts +++ b/src/main/runtime/orchestration/setup-completion-signal.test.ts @@ -59,8 +59,7 @@ describe('orchestration setup completion signal', () => { }) }) -// The carry lives in the scanner closure — one per running setup — until the -// next chunk, so an attached slice pins a whole setup-output chunk each. +// Production persists one carry per running setup. describe('setup completion scanner retention', () => { const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip @@ -72,22 +71,18 @@ describe('setup completion scanner retention', () => { const before = process.memoryUsage().heapUsed const scanners = Array.from({ length: setups }, (_unused, index) => { const scanner = createSetupCompletionScanner(`token-${index}`, () => undefined) - // A distinct chunk per setup. Sharing one would leave a single parent - // alive and pass whether or not the carry is detached. scanner.scan(`${'x'.repeat(chunkChars)}setup-${index}`) return scanner }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The carry must still complete a marker split across the chunk boundary. const exitCodes: number[] = [] const split = createSetupCompletionScanner('tok', (code) => exitCodes.push(code)) split.scan(`${'x'.repeat(chunkChars)}__ORCA_SETUP_COMPLETE__:tok`) split.scan(':7\n') expect(exitCodes).toEqual([7]) expect(scanners).toHaveLength(setups) - // 8 MiB of source chunks stay alive if the carries are attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/main/runtime/orchestration/setup-completion-signal.ts b/src/main/runtime/orchestration/setup-completion-signal.ts index 50f23c721fb..f35da096bd3 100644 --- a/src/main/runtime/orchestration/setup-completion-signal.ts +++ b/src/main/runtime/orchestration/setup-completion-signal.ts @@ -67,8 +67,7 @@ export function createSetupCompletionScanner( return } } - // Detached: the carry is held per running setup until its next chunk, so - // an attached slice would pin a whole setup-output chunk each. + // Persisted setup carries must not retain their source output chunks. carry = detachString(combined.slice(-SETUP_COMPLETION_CARRY_LENGTH)) } } diff --git a/src/main/runtime/terminal-pending-ansi.test.ts b/src/main/runtime/terminal-pending-ansi.test.ts new file mode 100644 index 00000000000..37dd5ce8500 --- /dev/null +++ b/src/main/runtime/terminal-pending-ansi.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest' +import { createForceGc, resolveForcedGc } from '../../shared/forced-gc-for-retention-tests' +import { retainTerminalPendingAnsi } from './terminal-pending-ansi' + +describe('retainTerminalPendingAnsi', () => { + it('bounds an incomplete control while preserving its introducer and suffix', () => { + expect(retainTerminalPendingAnsi(`\x1b]${'x'.repeat(5000)}`)).toBe(`\x1b]${'x'.repeat(4094)}`) + }) + + const forcedGc = resolveForcedGc() + const itWithGc = forcedGc ? it : it.skip + itWithGc('does not pin PTY chunks behind pending ANSI state', () => { + const forceGc = createForceGc(forcedGc!) + forceGc() + const before = process.memoryUsage().heapUsed + const held = Array.from({ length: 512 }, (_unused, index) => { + const chunk = `${'x'.repeat(16 * 1024)}\x1b]0;partial-${index}` + return retainTerminalPendingAnsi(chunk.slice(chunk.lastIndexOf('\x1b'))) + }) + forceGc() + const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) + + expect(held[0]).toBe('\x1b]0;partial-0') + expect(retainedMiB).toBeLessThan(2) + }) +}) diff --git a/src/main/runtime/terminal-pending-ansi.ts b/src/main/runtime/terminal-pending-ansi.ts new file mode 100644 index 00000000000..ef6cae6ea71 --- /dev/null +++ b/src/main/runtime/terminal-pending-ansi.ts @@ -0,0 +1,12 @@ +import { detachString } from '../../shared/detached-string' + +const MAX_TERMINAL_PENDING_ANSI_CHARS = 4096 + +export function retainTerminalPendingAnsi(value: string): string { + if (value.length <= MAX_TERMINAL_PENDING_ANSI_CHARS) { + return detachString(value) + } + const introducer = value.slice(0, Math.min(2, value.length)) + const suffixBudget = MAX_TERMINAL_PENDING_ANSI_CHARS - introducer.length + return detachString(`${introducer}${value.slice(-suffixBudget)}`) +} diff --git a/src/main/stats/agent-detector.test.ts b/src/main/stats/agent-detector.test.ts index 22a0daceec1..e4a9a7a6fec 100644 --- a/src/main/stats/agent-detector.test.ts +++ b/src/main/stats/agent-detector.test.ts @@ -243,8 +243,7 @@ describe('AgentDetector', () => { expect(stats.onAgentStop).toHaveBeenCalledWith('pty-1', 120) }) - // meaningfulContentScanTailByPtyId parks the tail until the next chunk, so - // an attached slice pins one whole source chunk per tracked PTY. + // Production persists one meaningful-content tail per PTY. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin source chunks behind carried per-PTY scan tails', () => { @@ -260,14 +259,12 @@ describe('AgentDetector', () => { forceGc() const before = process.memoryUsage().heapUsed for (let index = 0; index < ptys; index += 1) { - // Ends mid-CSI, so the tail is carried into the next chunk. detector.onData(`pty-${index}`, `${'y'.repeat(chunkChars)}\x1b[38;2;12;34;5`, 200) } forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(detector.trackedPtyCount).toBe(ptys) - // 8 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/main/stats/agent-detector.ts b/src/main/stats/agent-detector.ts index 5ae1532ee80..ccd8fec0342 100644 --- a/src/main/stats/agent-detector.ts +++ b/src/main/stats/agent-detector.ts @@ -286,8 +286,7 @@ function isStTerminatedStringControlIntroducer(introducer: string): boolean { return introducer === 'P' || introducer === 'X' || introducer === '^' || introducer === '_' } -// Detached because meaningfulContentScanTailByPtyId parks this until the next -// chunk: an attached slice pins a whole source chunk per tracked PTY. +// Persisted per-PTY tails must not retain their source chunks. function trimMeaningfulContentScanTail(value: string): string { if (value.length <= MEANINGFUL_CONTENT_SCAN_TAIL_LIMIT) { return detachString(value) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 1cbf0d7eb48..e8d71fcdd99 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -21,10 +21,10 @@ import { createTerminalZeroDimensionsMessage } from '../../../../shared/terminal import { isWorktreeRemovalFenceError } from '../../../../shared/worktree-removal-fence-error' import { parseTerminalOscColorQuery } from '../../../../shared/terminal-osc-color-reply' import { - HIDDEN_STARTUP_RENDERER_QUERY_PENDING_CHARS, containsCsiRendererQuery, containsStatefulRendererQuery, extractHiddenStartupRendererQueryData, + extractHiddenStartupRendererQueryPending, findCsiFinalByteIndex, isStatefulRendererReplyCsiQuery, isStatelessRendererReplyCsiQuery @@ -5224,8 +5224,7 @@ export function connectPanePty( return '' } } - // Detached: this tail is parked per pane until the next chunk, so an - // attached slice would pin a whole PTY chunk for the pane's lifetime. + // Persisted per-pane tails must not retain their source PTY chunks. return detachString(tail.slice(-TERMINAL_RENDERER_RISK_SCAN_TAIL_CHARS)) } @@ -6260,7 +6259,7 @@ export function connectPanePty( if (input.startsWith('\x1b[')) { const finalByteIndex = findCsiFinalByteIndex(input, 2) if (finalByteIndex === -1) { - nextPending = input.slice(0, HIDDEN_STARTUP_RENDERER_QUERY_PENDING_CHARS) + nextPending = extractHiddenStartupRendererQueryPending(input, 0) consumedInputChars = input.length } else { const sequence = input.slice(0, finalByteIndex + 1) @@ -6274,7 +6273,7 @@ export function connectPanePty( } else if (input.startsWith('\x1b]')) { const query = parseTerminalOscColorQuery(input, 0) if (query.kind === 'partial') { - nextPending = input.slice(0, HIDDEN_STARTUP_RENDERER_QUERY_PENDING_CHARS) + nextPending = extractHiddenStartupRendererQueryPending(input, 0) consumedInputChars = input.length } else if (query.kind === 'match') { oscColorQueryData = input.slice(0, query.endIndex) diff --git a/src/renderer/src/components/terminal-pane/pty-title-retention.test.ts b/src/renderer/src/components/terminal-pane/pty-title-retention.test.ts index 0eb1d5328e4..8ce4bdd1429 100644 --- a/src/renderer/src/components/terminal-pane/pty-title-retention.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-title-retention.test.ts @@ -2,28 +2,12 @@ import { describe, expect, it } from 'vitest' import { createForceGc, resolveForcedGc } from '../../../../shared/forced-gc-for-retention-tests' import { createPtyOutputProcessor } from './pty-transport' -// What this proves — and what it does not. -// -// Invariant under test: the title a pane hands the store (setRuntimePaneTitle / -// updateTabTitle) is a standalone string, not a V8 SlicedString over the PTY -// frame it was parsed from. Frames are sized at the production renderer-write -// bound (PTY_BATCH_FLUSH_CHUNK_CHARS = 16 KiB, src/main/ipc/pty.ts), not the -// multi-MiB frames an earlier draft of this test assumed. -// -// Panes are held live simultaneously with no store overwrite, which is the -// unbounded-in-principle shape. Production writes `[paneId]: title`, an -// overwrite that releases the previous parent, so the real live set is -// O(panes). This is a regression guard on the retention property, NOT a -// reproduction of a shipped out-of-memory crash. -// +// This guards 16 KiB frame detachment, not production's O(panes) overwrite model. const MIB = 1024 * 1024 const CHUNK_CHARS = 16 * 1024 const PANES = 2048 -// '█' is 2 bytes/char in V8, so a pinned frame costs twice its char count. const PINNED_FRAME_MIB = (CHUNK_CHARS * PANES * 2) / MIB -// resolveForcedGc works without --expose-gc, so this runs in CI's plain -// `vitest run`. Skips only if a hardened host refuses to expose gc at all. const forcedGc = resolveForcedGc() const describeWithGc = forcedGc ? describe : describe.skip const forceGc = forcedGc ? createForceGc(forcedGc) : (): void => undefined @@ -36,7 +20,6 @@ describeWithGc('renderer PTY title retention', () => { it('does not pin each pane PTY frame behind the title handed to the store', async () => { forceGc() const before = process.memoryUsage().heapUsed - // Stands in for store.runtimePaneTitlesByTabId / tab titles: one live title per pane. const storedTitles: string[] = [] for (let pane = 0; pane < PANES; pane += 1) { @@ -46,7 +29,6 @@ describeWithGc('renderer PTY title retention', () => { } }) processor.processData(makeAgentFrame(pane), { onData: () => undefined }) - // Drain the deferred side-effect queue the way the real drain timer does. await new Promise((resolve) => setTimeout(resolve, 0)) } @@ -54,7 +36,6 @@ describeWithGc('renderer PTY title retention', () => { const retainedMiB = (process.memoryUsage().heapUsed - before) / MIB const titleChars = storedTitles.reduce((total, title) => total + title.length, 0) - // eslint-disable-next-line no-console console.log( `${storedTitles.length} stored pane titles (${titleChars} chars) -> heap delta ${retainedMiB.toFixed(2)} MiB; ` + `source frames total ${PINNED_FRAME_MIB} MiB if every title pinned its parent` @@ -62,8 +43,6 @@ describeWithGc('renderer PTY title retention', () => { expect(storedTitles).toHaveLength(PANES) expect(storedTitles[0]).toBe('✳ Working… (esc to interrupt) pane-0') - // Titles hold well under 1 MiB of text. A delta near the frame total means - // the slices still point at their parents. expect(retainedMiB).toBeLessThan(PINNED_FRAME_MIB / 8) }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.test.ts b/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.test.ts index 191865de0c3..2443567899b 100644 --- a/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.test.ts @@ -5,10 +5,7 @@ import { pasteTerminalText, sanitizeTerminalPasteText } from './terminal-bracketed-paste' -import { - createForceGc, - resolveForcedGc -} from '../../../../shared/forced-gc-for-retention-tests' +import { createForceGc, resolveForcedGc } from '../../../../shared/forced-gc-for-retention-tests' function createTerminal(bracketedPasteMode = true) { const terminal = { @@ -257,8 +254,7 @@ describe('terminal bracketed paste policy', () => { expect(splitCallCount).toBe(0) }) - // The tail sits in a per-terminal WeakMap until the pane sees a 2004 toggle, - // so an attached slice pins a whole PTY chunk per interrupted pane. + // Production persists one interrupted-mode tail per terminal. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind the interrupted-pane mode tail', () => { @@ -266,8 +262,7 @@ describe('terminal bracketed paste policy', () => { const panes = 512 const chunkChars = 16 * 1024 - // Plain stubs, not createTerminal: a vi.fn() per pane costs megabytes of - // mock bookkeeping and would swamp the retention signal being measured. + // Avoid mock bookkeeping that would swamp the retention signal. const terminals = Array.from({ length: panes }, () => ({ modes: { bracketedPasteMode: true } })) @@ -276,14 +271,12 @@ describe('terminal bracketed paste policy', () => { const before = process.memoryUsage().heapUsed for (const [index, terminal] of terminals.entries()) { markTerminalBracketedPasteInterrupted(terminal) - // No 2004 sequence, so the tail is carried rather than cleared. observeTerminalBracketedPasteModeOutput(terminal, `${'x'.repeat(chunkChars)}pane-${index}`) } forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(terminals).toHaveLength(panes) - // ~8 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.ts b/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.ts index 12bb326011f..91e3d74ef57 100644 --- a/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.ts +++ b/src/renderer/src/components/terminal-pane/terminal-bracketed-paste.ts @@ -99,8 +99,7 @@ export function observeTerminalBracketedPasteModeOutput( return } const combined = (bracketedPasteModeOutputTail.get(terminal) ?? '') + data - // Detached: this tail is parked in a per-terminal WeakMap until the pane sees - // a 2004 toggle, so an attached slice pins a whole PTY chunk per interrupted pane. + // Persisted per-terminal tails must not retain their source PTY chunks. bracketedPasteModeOutputTail.set( terminal, detachString(combined.slice(-BRACKETED_PASTE_MODE_TAIL_MAX)) diff --git a/src/renderer/src/lib/pane-manager/terminal-complex-script.test.ts b/src/renderer/src/lib/pane-manager/terminal-complex-script.test.ts index 36f9a81ca23..ec4de4ca451 100644 --- a/src/renderer/src/lib/pane-manager/terminal-complex-script.test.ts +++ b/src/renderer/src/lib/pane-manager/terminal-complex-script.test.ts @@ -406,13 +406,7 @@ describe('nativeWindowsRewriteNeedsFollowupRenderRefresh', () => { }) }) -// The rewrite-CSI scan tail is parked in per-pane closure state -// (foreground/hiddenRewriteCsiScanTail in pty-connection.ts) until the next -// chunk, so an attached slice pins a whole PTY chunk per open pane. -// -// Why the split CSI is long: V8 only builds a SlicedString past -// SlicedString::kMinLength (13 chars); a shorter tail is already copied flat -// and would make this assertion pass with or without the detach. +// The ≥13-char control arm proves detachment for one persisted rewrite tail per pane. describe('rewrite CSI scan tail retention', () => { const splitCsi = '\x1b[1;2;3;4;5;6;7;8;9' const forcedGc = resolveForcedGc() @@ -427,8 +421,6 @@ describe('rewrite CSI scan tail retention', () => { const tails = Array.from( { length: panes }, (_unused, index) => - // A distinct chunk per pane, each ending mid-CSI. Sharing one chunk - // would leave a single parent alive and pass either way. terminalRewriteOutputRenderRefreshDecision( `${'x'.repeat(chunkChars)}pane-${index}${splitCsi}`, { previousChunkEndsWithCarriageReturn: false, previousRewriteCsiScanTail: '' } @@ -437,7 +429,6 @@ describe('rewrite CSI scan tail retention', () => { forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The tail must still complete the erase sequence across the boundary. expect(tails[0]).toBe(splitCsi) expect( terminalRewriteOutputRenderRefreshDecision('K', { @@ -445,7 +436,6 @@ describe('rewrite CSI scan tail retention', () => { previousRewriteCsiScanTail: tails[0]! }).prefersRenderRefresh ).toBe(true) - // 8 MiB of source chunks stay alive if the carried tails are attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/renderer/src/lib/pane-manager/terminal-complex-script.ts b/src/renderer/src/lib/pane-manager/terminal-complex-script.ts index b5dcc4fbbf5..d47bb7bfe25 100644 --- a/src/renderer/src/lib/pane-manager/terminal-complex-script.ts +++ b/src/renderer/src/lib/pane-manager/terminal-complex-script.ts @@ -141,9 +141,7 @@ function containsRewriteEraseSequence(data: string): boolean { return false } -// Detached: callers park this tail in per-pane closure state (foreground/hidden -// RewriteCsiScanTail in pty-connection.ts) until the next chunk, so an attached -// slice pins a whole PTY chunk per open pane. +// Persisted per-pane rewrite tails must not retain their source PTY chunks. function trailingIncompleteRewriteCsiTail(data: string): string { const escapeIndex = data.lastIndexOf(ESCAPE_CHARACTER) if (escapeIndex === -1) { diff --git a/src/shared/agent-status-field-normalization.ts b/src/shared/agent-status-field-normalization.ts index cc72ef1336d..72468daa506 100644 --- a/src/shared/agent-status-field-normalization.ts +++ b/src/shared/agent-status-field-normalization.ts @@ -190,11 +190,7 @@ export function normalizeInteractivePromptField( if (typeof value !== 'string' || value.length === 0) { return undefined } - // Why detach: this field is cached in the store, so an over-cap value's - // truncated slice would otherwise pin the whole original string. - // Why only when truncation happened: an under-cap value is returned by - // identity, is already flat (JSON.parse / JSON.stringify output), and pins - // nothing — detaching it would copy the payload on every hook event. + // Only truncated values can retain oversized payloads; preserve hot-path identity otherwise. const truncated = truncatePreservingSurrogates(value, maxLength) if (truncated.length === 0) { return undefined diff --git a/src/shared/agent-status-osc.test.ts b/src/shared/agent-status-osc.test.ts index 67f0cb1a85e..07c70a6c856 100644 --- a/src/shared/agent-status-osc.test.ts +++ b/src/shared/agent-status-osc.test.ts @@ -35,9 +35,7 @@ describe('createAgentStatusOscProcessor', () => { ]) }) - // The unterminated-payload carry lives in the processor closure until the - // next chunk, and there is one processor per pane, so an attached slice - // would pin a whole PTY chunk per pane. + // Production persists one unterminated payload per pane. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried OSC 9999 payload', () => { @@ -48,14 +46,12 @@ describe('createAgentStatusOscProcessor', () => { const before = process.memoryUsage().heapUsed const processors = Array.from({ length: panes }, (_unused, index) => { const processChunk = createAgentStatusOscProcessor() - // No terminator, so the payload is carried into the next chunk. processChunk(`${'x'.repeat(chunkChars)}\x1b]9999;{"state":"working","prompt":"p-${index}"`) return processChunk }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The carry must still work: 8 MiB of chunks stay alive if it is attached. expect(processors[0]!('}\x07').payloads[0]).toEqual({ state: 'working', prompt: 'p-0' }) expect(retainedMiB).toBeLessThan(2) }) diff --git a/src/shared/agent-status-osc.ts b/src/shared/agent-status-osc.ts index b9ff3caa01d..edd720b9a1f 100644 --- a/src/shared/agent-status-osc.ts +++ b/src/shared/agent-status-osc.ts @@ -71,8 +71,7 @@ export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgen if (terminator === null) { const candidate = combined.slice(start) - // Detached: `pending` survives until the next chunk, so an attached - // slice would pin this whole chunk per processor (one per pane). + // Persisted per-pane payloads must not retain their source PTY chunks. pending = candidate.length > MAX_PENDING ? '' : detachString(candidate) break } diff --git a/src/shared/agent-status-types.test.ts b/src/shared/agent-status-types.test.ts index 074ca448b37..fd60782e6b9 100644 --- a/src/shared/agent-status-types.test.ts +++ b/src/shared/agent-status-types.test.ts @@ -242,8 +242,7 @@ Fix dispatch fallback preview for normalized status prompts` expect(AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH).toBe(16000) }) - // The capped value is cached in the store; if truncation returned a raw - // slice it would keep the whole oversized payload alive. + // The store caches capped interactive prompts. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the oversized payload behind a capped interactivePrompt', () => { @@ -262,13 +261,10 @@ Fix dispatch fallback preview for normalized status prompts` const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(held[0]!.interactivePrompt).toHaveLength(AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH) - // 128 MiB of source payloads stay alive if the capped slice is still attached. expect(retainedMiB).toBeLessThan(16) }) - // An under-cap prompt pins nothing (it is the caller's own flat string), so - // it must be passed through by identity rather than copied on every hook - // event — this path fires many times per second during a tool-use run. + // Under-cap prompts stay on the allocation-free hook path. itWithGc('returns an under-cap interactivePrompt without copying it', () => { const source = JSON.stringify({ questions: [{ question: 'Which approach?', options: ['a'.repeat(4000), 'b'.repeat(4000)] }] @@ -289,7 +285,6 @@ Fix dispatch fallback preview for normalized status prompts` const bytesPerCall = (process.memoryUsage().heapUsed - before) / repeats expect(held[0]).toBe(source) - // A copy would cost ~source.length bytes per call; identity costs a pointer. expect(bytesPerCall).toBeLessThan(source.length / 4) }) diff --git a/src/shared/codex-subagent-transcript.test.ts b/src/shared/codex-subagent-transcript.test.ts index eb6335eaa16..0bca7f3f9ed 100644 --- a/src/shared/codex-subagent-transcript.test.ts +++ b/src/shared/codex-subagent-transcript.test.ts @@ -157,10 +157,7 @@ describe('Codex subagent transcript reconciliation', () => { expect(roster.size).toBe(0) }) - // The trailing partial line is parked per cursor in - // codexSubagentTranscriptByPaneKey until the next poll, and `split` yields - // slices of the up-to-1 MiB read buffer, so an attached carry pins a whole - // buffer per tracked pane. + // Production persists one partial line per transcript cursor. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the read buffer behind a carried partial line', () => { @@ -172,7 +169,6 @@ describe('Codex subagent transcript reconciliation', () => { const states = Array.from({ length: panes }, (_unused, index) => { const parentPath = join(dir, `rollout-parent-${index}.jsonl`) - // Trailing partial line (no newline) is what gets carried. writeFileSync(parentPath, `${jsonl([activity('started')])}${bulkLine}\n{"partial":${index}`) return { state: createCodexSubagentTranscriptState(), parentPath } }) @@ -186,7 +182,6 @@ describe('Codex subagent transcript reconciliation', () => { const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(states[0]!.state.parent.carry).toBe('{"partial":0') - // ~64 MiB of read buffers stay alive if the carries are still attached. expect(retainedMiB).toBeLessThan(8) }) }) diff --git a/src/shared/codex-subagent-transcript.ts b/src/shared/codex-subagent-transcript.ts index a7af643b5e2..af98dc5e074 100644 --- a/src/shared/codex-subagent-transcript.ts +++ b/src/shared/codex-subagent-transcript.ts @@ -78,9 +78,7 @@ function readJsonlCursor(cursor: JsonlCursor): JsonRecord[] | undefined { const content = `${skippedPrefix ? '' : cursor.carry}${buffer.toString('utf8', 0, bytesRead)}` const lines = content.split('\n') cursor.offset = start + bytesRead - // Detached: `split` yields slices of the up-to-1 MiB read buffer, and this - // carry is parked per cursor (one per pane, plus one per tracked subagent, in - // codexSubagentTranscriptByPaneKey) until the next poll. + // Persisted cursor carries must not retain the transcript read buffer. cursor.carry = detachString(lines.pop() ?? '') if (skippedPrefix) { lines.shift() diff --git a/src/shared/command-code-output-status.test.ts b/src/shared/command-code-output-status.test.ts index 1a38c03491b..28f594d35d6 100644 --- a/src/shared/command-code-output-status.test.ts +++ b/src/shared/command-code-output-status.test.ts @@ -464,8 +464,7 @@ describe('terminal control stripping', () => { } }) - // Every pane runs this detector (the ring is written before the Command Code - // early-out), so an attached ring pins one whole PTY chunk per open pane. + // Every pane persists this ring before Command Code detection. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind the recent raw-text ring', () => { @@ -484,7 +483,6 @@ describe('terminal control stripping', () => { const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(detectors).toHaveLength(panes) - // ~8 MiB of source chunks stay alive if the rings are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/command-code-output-status.ts b/src/shared/command-code-output-status.ts index 09292432344..932eba7b95a 100644 --- a/src/shared/command-code-output-status.ts +++ b/src/shared/command-code-output-status.ts @@ -137,10 +137,7 @@ function rawTextMayContainCommandCodeBanner(rawText: string): boolean { return rawText.includes('C') && rawText.includes('o') && rawText.includes('d') } -// Detached: every pane parks this ring until its next chunk (the detector runs -// before the Command Code early-out), so an attached slice pins a whole PTY -// chunk per pane. The empty-prefix case matters too: V8 returns `data` itself -// from `'' + data`, so that branch slices the raw chunk as well. +// Every pane persists this ring before Command Code detection. function appendRecentRawText(previousRawText: string, data: string): string { if (data.length >= RECENT_TEXT_LIMIT) { return detachString(data.slice(-RECENT_TEXT_LIMIT)) diff --git a/src/shared/detached-string.test.ts b/src/shared/detached-string.test.ts index 84249839239..7ab1e4c481f 100644 --- a/src/shared/detached-string.test.ts +++ b/src/shared/detached-string.test.ts @@ -20,21 +20,13 @@ describe('detachString', () => { expect(detachString(value)).toBe(value) }) - // Why the control arm: V8 only builds a SlicedString when the result is at - // least SlicedString::kMinLength (13) chars; shorter slices are already - // copied flat. A retention assertion over a short result therefore passes - // with or without detachString and proves nothing. Measuring the raw slice - // in the same run makes the test fail if that premise ever stops holding. - // - // resolveForcedGc works without --expose-gc, so this guard runs under the - // plain `vitest run` CI uses instead of silently skipping. + // A ≥13-char raw control arm proves both SlicedString retention and default-run GC. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('detaches a slice from its parent buffer', () => { const chunkChars = 16 * 1024 const slices = 4096 const forceGc = createForceGc(forcedGc!) - // Comfortably over kMinLength so the untouched slice really is a SlicedString. const measure = (detach: boolean): number => { const held: string[] = [] forceGc() @@ -52,7 +44,6 @@ describe('detachString', () => { const attachedMiB = measure(false) const detachedMiB = measure(true) - // ~128 MiB of parents stay alive when the slices are left attached. expect(attachedMiB).toBeGreaterThan(16) expect(detachedMiB).toBeLessThan(attachedMiB / 8) }) diff --git a/src/shared/detached-string.ts b/src/shared/detached-string.ts index a29345740c2..4173f599a03 100644 --- a/src/shared/detached-string.ts +++ b/src/shared/detached-string.ts @@ -1,15 +1,4 @@ -/** - * Return a copy of `value` that shares no backing store with the string it was - * sliced from. - * - * Why: `String.prototype.slice` on a long string produces a V8 SlicedString - * that keeps a pointer to its parent, so a short slice retained in long-lived - * state (a store entry, a cached field) pins the entire source buffer — a raw - * PTY chunk or a hook JSON payload — for as long as the slice is reachable. - * Concatenating forces V8 to flatten into a fresh flat string, dropping the - * parent pointer. The leading space is stripped back off so the value is - * unchanged. - */ +/** Break V8 SlicedString parent retention by forcing a fresh flat string. */ export function detachString(value: string): string { if (value.length === 0) { return '' diff --git a/src/shared/draft-paste-ready-scanner.test.ts b/src/shared/draft-paste-ready-scanner.test.ts index fc3f629087a..ee062f789d7 100644 --- a/src/shared/draft-paste-ready-scanner.test.ts +++ b/src/shared/draft-paste-ready-scanner.test.ts @@ -138,9 +138,7 @@ describe('createDraftPasteReadyScanner', () => { }) }) -// The 512-char rings live in the scanner closure — one scanner per pane -// waiting to deliver a startup draft — so an attached slice pins a whole PTY -// chunk per waiter instead of the 512 chars the ring documents. +// Production persists two 512-char rings per startup-draft waiter. describe('draft paste scanner retention', () => { const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip @@ -152,17 +150,13 @@ describe('draft paste scanner retention', () => { const before = process.memoryUsage().heapUsed const scanners = Array.from({ length: waiters }, (_unused, index) => { const scanner = createDraftPasteReadyScanner('codex-composer-prompt') - // A distinct chunk per waiter. Sharing one would leave a single parent - // alive and pass whether or not the rings are detached. scanner.observe(`${DECSET_BRACKETED_PASTE}${'x'.repeat(chunkChars)}pane-${index}`) return scanner }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The rings must still detect the prompt glyph on the next chunk. expect(scanners[0]!.observe(CODEX_PROMPT)).toEqual({ ready: true, armQuietTimer: false }) - // 8 MiB of source chunks stay alive if the rings are attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/draft-paste-ready-scanner.ts b/src/shared/draft-paste-ready-scanner.ts index dfd5296d30a..31883732d86 100644 --- a/src/shared/draft-paste-ready-scanner.ts +++ b/src/shared/draft-paste-ready-scanner.ts @@ -62,9 +62,7 @@ export function createDraftPasteReadyScanner(readySignal: DraftPasteReadySignal) return { observe(data: string): DraftPasteReadyScanResult { const combined = recent + data - // Detached: both rings survive until the next chunk (and past settle, until - // the scanner itself is dropped), so an attached slice would pin a whole - // PTY chunk per waiting pane instead of the 512 chars the ring promises. + // Persisted scanner rings must not retain their source PTY chunks. recent = detachString(combined.slice(-RING_CHARS)) if (!saw2004) { const markerIndex = combined.indexOf(DECSET_BRACKETED_PASTE) diff --git a/src/shared/forced-gc-for-retention-tests.ts b/src/shared/forced-gc-for-retention-tests.ts index 9c0a49fc89e..de36da4125e 100644 --- a/src/shared/forced-gc-for-retention-tests.ts +++ b/src/shared/forced-gc-for-retention-tests.ts @@ -1,18 +1,7 @@ import { setFlagsFromString } from 'node:v8' import { runInNewContext } from 'node:vm' -/** - * Resolve a real forced-GC function for retention tests, without requiring the - * runner to pass `--expose-gc`. - * - * Why: CI runs plain `pnpm exec vitest run` (.github/workflows/pr.yml), so - * `globalThis.gc` is undefined and every `describe.skip`-on-missing-gc - * retention guard silently stops running — a reintroduced raw slice would land - * green. `v8.setFlagsFromString('--expose-gc')` plus a throwaway VM context - * hands back a real `gc` binding in the worker; the flag is reset immediately - * so nothing else in the process observes it. Returns null only if the host - * genuinely refuses (hardened runtimes), which keeps callers skippable. - */ +/** Resolve forced GC under plain Vitest; hardened runtimes may return null. */ export function resolveForcedGc(): (() => void) | null { const existing = (globalThis as { gc?: () => void }).gc if (typeof existing === 'function') { @@ -28,7 +17,7 @@ export function resolveForcedGc(): (() => void) | null { } } -/** Double-collect so a freshly unreachable parent is actually released. */ +/** Collect twice so freshly unreachable parents are released. */ export function createForceGc(gc: () => void): () => void { return () => { gc() diff --git a/src/shared/osc-title-extraction.retention.test.ts b/src/shared/osc-title-extraction.retention.test.ts index e841662c8e6..0a152b6fe1a 100644 --- a/src/shared/osc-title-extraction.retention.test.ts +++ b/src/shared/osc-title-extraction.retention.test.ts @@ -2,31 +2,17 @@ import { describe, expect, it } from 'vitest' import { createForceGc, resolveForcedGc } from './forced-gc-for-retention-tests' import { extractAllOscTitles } from './osc-title-extraction' -// What this proves — and what it does not. -// -// Invariant under test: an extracted OSC title is a standalone string, not a V8 -// SlicedString pinning the PTY chunk it was cut from. Chunks are sized at the -// production renderer-write bound (PTY_BATCH_FLUSH_CHUNK_CHARS = 16 KiB, see -// src/main/ipc/pty.ts), not the multi-MiB chunks an earlier draft assumed. -// -// This is the unbounded-in-principle case: N titles from N distinct chunks all -// held live at once. Production's per-pane store write is an overwrite, so the -// live set there is O(panes) and small. So this is a regression guard on the -// retention property, NOT a reproduction of a shipped out-of-memory crash. -// +// This guards 16 KiB chunk detachment, not a production OOM reproduction. const MIB = 1024 * 1024 const CHUNK_CHARS = 16 * 1024 const RETAINED_TITLES = 4096 const PINNED_CHUNK_MIB = (CHUNK_CHARS * RETAINED_TITLES) / MIB -// resolveForcedGc works without --expose-gc, so this runs in CI's plain -// `vitest run`. Skips only if a hardened host refuses to expose gc at all. const forcedGc = resolveForcedGc() const describeWithGc = forcedGc ? describe : describe.skip const forceGc = forcedGc ? createForceGc(forcedGc) : (): void => undefined function makePtyChunk(index: number): string { - // A realistic dense agent frame: bulk redraw plus one OSC 0 working title. return `${'x'.repeat(CHUNK_CHARS)}\x1b]0;✳ Working… (esc to interrupt) ${index}\x07` } @@ -36,7 +22,6 @@ describeWithGc('OSC title retention', () => { const before = process.memoryUsage().heapUsed const retained: string[] = [] for (let index = 0; index < RETAINED_TITLES; index += 1) { - // The chunk itself goes out of scope every iteration; only the title survives. const titles = extractAllOscTitles(makePtyChunk(index)) retained.push(titles.at(-1) as string) } @@ -44,7 +29,6 @@ describeWithGc('OSC title retention', () => { const retainedMiB = (process.memoryUsage().heapUsed - before) / MIB const titleChars = retained.reduce((total, title) => total + title.length, 0) - // eslint-disable-next-line no-console console.log( `retained ${retained.length} titles (${titleChars} chars total) -> heap delta ${retainedMiB.toFixed(2)} MiB; ` + `source chunks total ${PINNED_CHUNK_MIB} MiB if every title pinned its parent` @@ -52,8 +36,6 @@ describeWithGc('OSC title retention', () => { expect(retained).toHaveLength(RETAINED_TITLES) expect(retained[0]).toBe('✳ Working… (esc to interrupt) 0') - // The titles hold well under 1 MiB of actual text. A delta anywhere near the - // source-chunk total means the slices still point at their parents. expect(retainedMiB).toBeLessThan(PINNED_CHUNK_MIB / 8) }) }) diff --git a/src/shared/osc-title-extraction.ts b/src/shared/osc-title-extraction.ts index bc11d3f537d..ea0879e1f33 100644 --- a/src/shared/osc-title-extraction.ts +++ b/src/shared/osc-title-extraction.ts @@ -61,9 +61,7 @@ function parseOscTitleAt(data: string, index: number): OscTitleParseResult { function readBoundedOscTitle(data: string, titleStart: number, titleEnd: number): string { // Why: PTY output can contain pasted or remote-controlled OSC titles; keep // downstream title detection bounded while preserving trailing status words. - // Why detach here: titles outlive their chunk in pane/tab state, and a raw - // slice would pin the whole PTY chunk. Detaching at the extraction boundary - // means no consumer can bypass it. + // Extracted titles must not retain their source PTY chunks. const length = titleEnd - titleStart if (length <= MAX_OSC_TITLE_CHARS) { return detachString(data.slice(titleStart, titleEnd)) diff --git a/src/shared/osc-title-scan-tail.test.ts b/src/shared/osc-title-scan-tail.test.ts index 0c6efb78f54..c31637f86e6 100644 --- a/src/shared/osc-title-scan-tail.test.ts +++ b/src/shared/osc-title-scan-tail.test.ts @@ -16,16 +16,13 @@ describe('extractOscTitleScanTail', () => { expect(extractOscTitleScanTail('\x1b]133;D;0\x07\x1b')).toBe('\x1b') }) - // A carried tail lives in a per-PTY map (agent-detector.ts, - // orca-runtime.ts) until the next chunk, so a raw slice would pin one whole - // source chunk per tracked PTY. + // Production persists one title scan tail per PTY. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried tail', () => { const chunkChars = 16 * 1024 const ptys = 512 const forceGc = createForceGc(forcedGc!) - // A split title: no terminator yet, so the tail is carried to the next chunk. const makeChunk = (index: number): string => `${'x'.repeat(chunkChars)}\x1b]0;✳ Working… (esc to interrupt) pty-${index}` @@ -40,7 +37,6 @@ describe('extractOscTitleScanTail', () => { expect(carried.size).toBe(ptys) expect(carried.get('pty-0')).toBe('\x1b]0;✳ Working… (esc to interrupt) pty-0') - // 16 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/osc-title-scan-tail.ts b/src/shared/osc-title-scan-tail.ts index 4709f89bd95..3c61e9128e8 100644 --- a/src/shared/osc-title-scan-tail.ts +++ b/src/shared/osc-title-scan-tail.ts @@ -4,10 +4,7 @@ const OSC_TITLE_SCAN_TAIL_LIMIT = 4096 const OSC_TITLE_PREFIX_LENGTH = 4 const OSC_TITLE_CODES = new Set(['0', '1', '2']) -// Callers park the returned tail in a per-PTY map until the next chunk -// arrives. The non-empty tails are slices of the raw chunk, so they are -// detached (see trimOscTitleScanTail) — otherwise each carried tail would pin -// a whole source chunk for as long as its PTY stays tracked. +// Persisted per-PTY tails must not retain their source chunks. export function extractOscTitleScanTail(input: string): string { const lastOsc = input.lastIndexOf('\x1b]') if (lastOsc !== -1) { diff --git a/src/shared/terminal-color-scheme-protocol.test.ts b/src/shared/terminal-color-scheme-protocol.test.ts index 5e880536afd..572d86d5924 100644 --- a/src/shared/terminal-color-scheme-protocol.test.ts +++ b/src/shared/terminal-color-scheme-protocol.test.ts @@ -53,8 +53,7 @@ describe('terminal color scheme protocol', () => { }) }) - // The incomplete private-mode tail is carried in per-pane scan state until the - // next chunk, so an attached slice pins a whole PTY chunk per pane. + // Production persists one private-mode tail per pane. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried private-mode tail', () => { @@ -65,14 +64,12 @@ describe('terminal color scheme protocol', () => { const before = process.memoryUsage().heapUsed const tails = Array.from( { length: panes }, - // No final byte yet, so the params tail is carried to the next chunk. () => scanMode2031Sequences('', `${'x'.repeat(chunkChars)}\x1b[?1049;2004;2026;12345`).tail ) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(tails[0]).toBe('\x1b[?1049;2004;2026;12345') - // 8 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-color-scheme-protocol.ts b/src/shared/terminal-color-scheme-protocol.ts index 181767c976b..8ce431a8878 100644 --- a/src/shared/terminal-color-scheme-protocol.ts +++ b/src/shared/terminal-color-scheme-protocol.ts @@ -132,8 +132,7 @@ function hasMode2031(params: string): boolean { return params.split(';').some((param) => Number(param) === 2031) } -// Detached: the retained tail is carried in per-pane scan state until the next -// chunk, so an attached slice pins a whole PTY chunk per pane. +// Persisted per-pane tails must not retain their source PTY chunks. function extractPrivateModeScanTail(input: string): string { const start = Math.max(input.lastIndexOf('\x1b'), input.lastIndexOf('\x9b')) if (start === -1) { diff --git a/src/shared/terminal-github-pr-link-detector.test.ts b/src/shared/terminal-github-pr-link-detector.test.ts index e3b6dbe44b3..76b5e21f7bb 100644 --- a/src/shared/terminal-github-pr-link-detector.test.ts +++ b/src/shared/terminal-github-pr-link-detector.test.ts @@ -293,9 +293,7 @@ describe('createTerminalGitHubPRLinkDetector', () => { ]) }) - // The URL carry lives in the detector closure until the next chunk, and there - // is one detector per pane/PTY, so an attached slice pins a whole PTY chunk - // per pane for as long as the pane stays open. + // Production persists one URL carry per pane or PTY. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried PR-URL prefix', () => { @@ -306,14 +304,12 @@ describe('createTerminalGitHubPRLinkDetector', () => { const before = process.memoryUsage().heapUsed const detectors = Array.from({ length: panes }, (_unused, index) => { const observe = createTerminalGitHubPRLinkDetector() - // Ends mid-URL, so the prefix is carried into the next chunk. observe(`${'x'.repeat(chunkChars)}https://github.com/acme/repo-${index}/pul`) return observe }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The carry must still complete the split URL across the chunk boundary. expect(detectors[0]!('l/12\n')).toEqual([ { url: 'https://github.com/acme/repo-0/pull/12', @@ -321,7 +317,6 @@ describe('createTerminalGitHubPRLinkDetector', () => { number: 12 } ]) - // 8 MiB of source chunks stay alive if the carries are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-github-pr-link-detector.ts b/src/shared/terminal-github-pr-link-detector.ts index 6e396fa4eb8..ef3351733d8 100644 --- a/src/shared/terminal-github-pr-link-detector.ts +++ b/src/shared/terminal-github-pr-link-detector.ts @@ -66,8 +66,7 @@ function lastIndexOfHttpScheme(value: string, fromIndex?: number): number { return lastIndex } -// Detached: the carry survives in the detector closure until the next chunk, and -// there is one detector per pane/PTY, so an attached slice pins a whole chunk each. +// Long URL carries must not retain their source PTY chunk. function getPotentialGitHubPRCarry(value: string): string { // Why bounded: carry is always a suffix of at most MAX_CARRY_LENGTH, so a scheme // further back can only ever be dropped — scanning to it is O(chunk) per PTY write. @@ -83,11 +82,11 @@ function getPotentialGitHubPRCarry(value: string): string { const fragment = endsWithHttpSchemePrefixFragment(tailWindow) if (fragment === '' || windowStart === 0) { - return detachString(fragment) + return fragment } // Why look behind: an older scheme means the URL already overran the cap, so the // carry is abandoned rather than restarted from this fragment. - return lastIndexOfHttpScheme(value, windowStart - 1) === -1 ? detachString(fragment) : '' + return lastIndexOfHttpScheme(value, windowStart - 1) === -1 ? fragment : '' } function hasTerminalUrlWhitespace(value: string, start: number, end: number): boolean { diff --git a/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts b/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts index 0f41bb04dee..cf69d4d2ecc 100644 --- a/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts +++ b/src/shared/terminal-kitty-keyboard-mode-tracker.test.ts @@ -149,12 +149,7 @@ describe('TerminalKittyKeyboardModeTracker', () => { expect(ranAndExited.flags).toBe(0) }) - // One tracker per pane (renderer) and per PTY (main) holds the scan tail - // until the next chunk, so an attached slice pins a whole PTY chunk each. - // - // Why the tail is long: V8 only builds a SlicedString past - // SlicedString::kMinLength (13 chars); a shorter tail is already copied flat - // and would make this assertion pass with or without the detach. + // The ≥13-char control arm proves detachment for one persisted tail per tracker. const splitPrivateModeTail = '\x1b[?1049;2004;2026;1234' const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip @@ -167,24 +162,19 @@ describe('TerminalKittyKeyboardModeTracker', () => { const before = process.memoryUsage().heapUsed const trackers = Array.from({ length: panes }, (_unused, index) => { const tracker = new TerminalKittyKeyboardModeTracker() - // Ends mid-DECSET, so the params tail is carried into the next chunk. tracker.scan(`${'x'.repeat(chunkChars)}pty-${index}${splitPrivateModeTail}`) return tracker }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The carry must still complete the split sequence across the boundary. trackers[0]!.scan('9h') expect(trackers[0]!.isAlternateScreen).toBe(true) - // 8 MiB of source chunks stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) - // scanReplay is fed whole reattach snapshots, not 16 KiB PTY chunks, so an - // attached tail pins far more per pane than the live path does. + // Replay consumes whole reattach snapshots rather than 16 KiB live chunks. itWithGc('does not pin a reattach snapshot behind a carried kitty scan tail', () => { - // A default 5000-row scrollback snapshot at ~120 chars/row. const snapshotChars = 5000 * 120 const panes = 20 const forceGc = createForceGc(forcedGc!) @@ -200,7 +190,6 @@ describe('TerminalKittyKeyboardModeTracker', () => { trackers[0]!.scan('9h') expect(trackers[0]!.isAlternateScreen).toBe(true) - // ~11 MiB of snapshots stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-kitty-keyboard-mode-tracker.ts b/src/shared/terminal-kitty-keyboard-mode-tracker.ts index 64531a21f13..f88e751deec 100644 --- a/src/shared/terminal-kitty-keyboard-mode-tracker.ts +++ b/src/shared/terminal-kitty-keyboard-mode-tracker.ts @@ -171,10 +171,7 @@ export class TerminalKittyKeyboardModeTracker { } } - // Detached: this tracker is per-pane (renderer) and per-PTY (main), and the - // tail outlives its input until the next chunk. An attached slice would pin - // the whole input — a live PTY chunk, or a full reattach/replay snapshot on - // the scanReplay path — for as long as the pane stays open. + // Persisted tracker tails must not retain live chunks or replay snapshots. private extractScanTail(input: string): string { const start = Math.max(input.lastIndexOf('\x1b'), input.lastIndexOf('\x9b')) if (start === -1) { diff --git a/src/shared/terminal-osc133-command-finished.test.ts b/src/shared/terminal-osc133-command-finished.test.ts index 03c638b8c64..0b1f930c52b 100644 --- a/src/shared/terminal-osc133-command-finished.test.ts +++ b/src/shared/terminal-osc133-command-finished.test.ts @@ -34,8 +34,7 @@ describe('createOsc133CommandFinishedScanner', () => { expect(onCommandFinished).not.toHaveBeenCalled() }) - // The carry lives in the scanner closure until the next chunk, and there is - // one scanner per pane, so an attached slice pins a whole PTY chunk per pane. + // Production persists one unterminated payload per pane scanner. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the source chunk behind a carried OSC 133 payload', () => { @@ -47,17 +46,14 @@ describe('createOsc133CommandFinishedScanner', () => { const before = process.memoryUsage().heapUsed const scanners = Array.from({ length: panes }, (_unused, index) => { const scanner = createOsc133CommandFinishedScanner(finished) - // No terminator yet, so the payload is carried into the next chunk. scanner.scan(`${'x'.repeat(chunkChars)}\x1b]133;D;${index} pending payload tail text`) return scanner }) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The carry must still resolve across the chunk boundary. scanners[0]!.scan('\x07') expect(finished).toHaveBeenCalledWith(0) - // 8 MiB of source chunks stay alive if the carries are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-osc133-command-finished.ts b/src/shared/terminal-osc133-command-finished.ts index 33c1ddaec84..9a6a4ae2f9f 100644 --- a/src/shared/terminal-osc133-command-finished.ts +++ b/src/shared/terminal-osc133-command-finished.ts @@ -90,8 +90,7 @@ export function createOsc133CommandFinishedScanner( const terminator = findOscTerminator(combined, payloadStart) if (!terminator) { const pending = combined.slice(start) - // Detached: the carry outlives this chunk in the scanner closure (one per - // pane), so an attached slice would pin the whole chunk until the next one. + // Persisted per-pane carries must not retain their source PTY chunks. carry = detachString( pending.length > MAX_OSC_CARRY_LENGTH ? pending.slice(pending.length - MAX_OSC_CARRY_LENGTH) diff --git a/src/shared/terminal-partial-escape-tail.test.ts b/src/shared/terminal-partial-escape-tail.test.ts index 211bb3ea9d0..7ce2a8705d9 100644 --- a/src/shared/terminal-partial-escape-tail.test.ts +++ b/src/shared/terminal-partial-escape-tail.test.ts @@ -85,9 +85,7 @@ describe('advancePartialEscapeTail', () => { expect(advancePartialEscapeTail('', huge)).toBe('') }) - // The headless emulator parks this tail per session until the next write and - // ships it in every snapshot, so an attached slice pins a whole written - // stream per live session. + // Production persists and snapshots one partial escape tail per emulator. const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip itWithGc('does not pin the written stream behind a carried escape tail', () => { @@ -97,16 +95,13 @@ describe('advancePartialEscapeTail', () => { forceGc() const before = process.memoryUsage().heapUsed const tails = Array.from({ length: sessions }, (_unused, index) => - // Unterminated OSC, so the tail is carried into the next write. advancePartialEscapeTail('', `${'x'.repeat(chunkChars)}\x1b]0;pane-${index} title`) ) forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) expect(tails[0]).toBe('\x1b]0;pane-0 title') - // The fold must still complete the split sequence on the next chunk. expect(advancePartialEscapeTail(tails[0]!, '\x07rest')).toBe('') - // 8 MiB of source streams stay alive if the tails are still attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-partial-escape-tail.ts b/src/shared/terminal-partial-escape-tail.ts index 9d8e53b2b2d..340a4624759 100644 --- a/src/shared/terminal-partial-escape-tail.ts +++ b/src/shared/terminal-partial-escape-tail.ts @@ -147,9 +147,6 @@ export function extractPartialEscapeTail(stream: string): string { * (tracking abandoned) when the tail exceeds the cap — see the cap comment. */ export function advancePartialEscapeTail(pendingTail: string, chunk: string): string { const tail = extractPartialEscapeTail(pendingTail + chunk) - // Detached: this is the ingest-time fold, so callers park the result per - // emulator/session until the next write; an attached slice would pin the - // whole written stream. Detaching after the cap check keeps the discarded - // over-cap tail (an unterminated OSC can span the entire chunk) free. + // Persisted emulator tails must not retain the whole written stream. return tail.length > MAX_PARTIAL_ESCAPE_TAIL_LENGTH ? '' : detachString(tail) } diff --git a/src/shared/terminal-reply-query-extraction.test.ts b/src/shared/terminal-reply-query-extraction.test.ts index ed02cdf5c95..b1aa946cf22 100644 --- a/src/shared/terminal-reply-query-extraction.test.ts +++ b/src/shared/terminal-reply-query-extraction.test.ts @@ -30,13 +30,7 @@ describe('extractHiddenStartupRendererQueryData', () => { expect(pending.length).toBe(HIDDEN_STARTUP_RENDERER_QUERY_PENDING_CHARS) }) - // `pending` is parked per pane (hiddenStartupRendererQueryPending in - // pty-connection.ts) until the next chunk, so an attached slice pins a whole - // PTY chunk per open pane. - // - // Why the split CSI is long: V8 only builds a SlicedString past - // SlicedString::kMinLength (13 chars); a shorter prefix is already copied - // flat and would make this assertion pass with or without the detach. + // The ≥13-char control arm proves detachment for one persisted query per pane. const splitCsi = '\x1b[?1049;2004;2026' const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip @@ -50,8 +44,6 @@ describe('extractHiddenStartupRendererQueryData', () => { const pendings = Array.from( { length: panes }, (_unused, index) => - // A distinct chunk per pane, each ending mid-CSI. Sharing one chunk - // would leave a single parent alive and pass either way. extractHiddenStartupRendererQueryData( `${'x'.repeat(chunkChars)}pane-${index}${splitCsi}`, '' @@ -60,12 +52,10 @@ describe('extractHiddenStartupRendererQueryData', () => { forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The pending prefix must still assemble the query across the boundary. expect(pendings[0]).toBe(splitCsi) expect(extractHiddenStartupRendererQueryData('$p', pendings[0]!).statefulQueryData).toBe( `${splitCsi}$p` ) - // 8 MiB of source chunks stay alive if the pending prefixes are attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-reply-query-extraction.ts b/src/shared/terminal-reply-query-extraction.ts index fdca71cf94d..bf0cc0bf406 100644 --- a/src/shared/terminal-reply-query-extraction.ts +++ b/src/shared/terminal-reply-query-extraction.ts @@ -16,10 +16,8 @@ export type ExtractedRendererQueryData = { pending: string } -// Detached: callers park `pending` until the next chunk — one per pane in the -// renderer (hiddenStartupRendererQueryPending, pty-connection.ts) — so an -// attached slice would pin a whole PTY chunk for as long as the pane lives. -function detachedQueryPending(input: string, start: number): string { +// Pending query state must not retain the source PTY chunk. +export function extractHiddenStartupRendererQueryPending(input: string, start: number): string { return detachString(input.slice(start, start + HIDDEN_STARTUP_RENDERER_QUERY_PENDING_CHARS)) } @@ -53,7 +51,7 @@ export function extractHiddenStartupRendererQueryData( statelessQueryData, statefulQueryData, oscColorQueryData, - pending: detachedQueryPending(input, candidateIndex) + pending: extractHiddenStartupRendererQueryPending(input, candidateIndex) } } const sequence = input.slice(candidateIndex, finalByteIndex + 1) @@ -73,7 +71,7 @@ export function extractHiddenStartupRendererQueryData( statelessQueryData, statefulQueryData, oscColorQueryData, - pending: detachedQueryPending(input, candidateIndex) + pending: extractHiddenStartupRendererQueryPending(input, candidateIndex) } } if (query.kind === 'none') { diff --git a/src/shared/terminal-reply-query-scan.test.ts b/src/shared/terminal-reply-query-scan.test.ts index 13b8246ab82..7ff1e2edaba 100644 --- a/src/shared/terminal-reply-query-scan.test.ts +++ b/src/shared/terminal-reply-query-scan.test.ts @@ -40,13 +40,7 @@ describe('terminal reply query scan', () => { expect(second.queries).toEqual([]) }) - // The pending prefix is parked in per-stream scan state until the next chunk - // for the whole mobile-stream buffering window, so an attached slice pins a - // whole PTY chunk per buffering stream. - // - // Why the pending prefix is long: V8 only builds a SlicedString past - // SlicedString::kMinLength (13 chars); a shorter one is already copied flat - // and would make this assertion pass with or without the detach. + // The ≥13-char control arm proves detachment for one persisted mobile-stream query. const splitQueryPrefix = '\x1b[?1049;2004;2026' const forcedGc = resolveForcedGc() const itWithGc = forcedGc ? it : it.skip @@ -60,9 +54,6 @@ describe('terminal reply query scan', () => { const states = Array.from( { length: streams }, (_unused, index) => - // A distinct chunk per stream, each ending mid-CSI so the partial query - // is carried. Sharing one chunk would leave a single parent alive and - // make the assertion pass whether or not the pending prefix is detached. scanTerminalReplyQuerySequences( `${'x'.repeat(chunkChars)}pty-${index}${splitQueryPrefix}`, 0, @@ -72,7 +63,6 @@ describe('terminal reply query scan', () => { forceGc() const retainedMiB = (process.memoryUsage().heapUsed - before) / (1024 * 1024) - // The pending prefix must still assemble the query across the boundary. const pendingStartSeq = states[0]!.pendingStartSeq! const resumed = scanTerminalReplyQuerySequences( '$p', @@ -86,7 +76,6 @@ describe('terminal reply query scan', () => { endSeq: pendingStartSeq + splitQueryPrefix.length + 2 } ]) - // 8 MiB of source chunks stay alive if the pending prefixes are attached. expect(retainedMiB).toBeLessThan(2) }) }) diff --git a/src/shared/terminal-reply-query-scan.ts b/src/shared/terminal-reply-query-scan.ts index 78eb44ffa45..c588e706273 100644 --- a/src/shared/terminal-reply-query-scan.ts +++ b/src/shared/terminal-reply-query-scan.ts @@ -46,9 +46,7 @@ function isReplyElicitingCsi(sequence: string): boolean { ) } -// Detached: the returned pending prefix is parked in per-stream scan state -// until the next chunk, so an attached slice would pin a whole PTY chunk for -// every buffering terminal stream. +// Persisted query prefixes must not retain their source PTY chunks. function boundedPending(input: string, startIndex: number): string { return detachString(input.slice(startIndex, startIndex + MAX_PENDING_QUERY_CHARS)) }