diff --git a/docs/superpowers/plans/2026-09-03-pty-replay-buffer-chunks.md b/docs/superpowers/plans/2026-09-03-pty-replay-buffer-chunks.md new file mode 100644 index 000000000..2b05eb7cb --- /dev/null +++ b/docs/superpowers/plans/2026-09-03-pty-replay-buffer-chunks.md @@ -0,0 +1,49 @@ +# Main: make the PTY replay buffers amortised O(chunk) + +Fixes #726. Refs #390, #321, #103, #327. + +## Problem + +`SessionManager` keeps two per-session replay buffers as plain strings — +`terminalBuffers` (256 KiB cap) and `agentPtyBuffers` (512 KiB cap) — and +appends with `appendCappedBuffer`: `prev + data`, a `charCodeAt` that +flattens the concat into a fresh (cap + chunk) copy, then `slice`, whose +result is a V8 SlicedString that keeps that whole flattened parent alive. +Once a buffer is full (minutes into any agent session), every chunk from +every session copies the whole cap on the main thread and the stored value +retains cap + chunk. A chunk larger than the cap is retained in full: the +2026-09-03 heap snapshot holds six identical 1.8 MB docker-build repaints +and ~10 identical 1 MB Claude screen frames for exactly this reason. + +## Design + +Replace the string with a small `CappedTextBuffer` (new module under +`src/main/sessions/`): a list of chunks plus a running length. + +- `append(chunk)` pushes the chunk and drops whole oldest chunks while the + total exceeds the cap — amortised O(chunk), no copy of the retained + bytes. Dropping whole chunks can never split a surrogate pair, which is + why the old code needed its low-surrogate check. +- A chunk larger than the cap on its own replaces the whole buffer with a + real copy of its tail (surrogate-safe cut, forced flat so the oversized + chunk is released — the #321 lesson). +- `read()` joins once. It runs only on attach, which is rare, so the join + is the right place to pay the O(cap) cost. +- `length` is exposed for tests and diagnostics. + +`SessionManager` swaps the two `Map` for +`Map`; the attach/replay contract (bytes +accumulate from process start, attach returns the current tail in the same +synchronous block that flips the attached flag, caps unchanged) does not +move. + +## Verification + +- New `cappedTextBuffer.test.ts`: retains the newest bytes under the cap, + never exceeds the cap, keeps whole chunks, handles an oversized chunk by + keeping a surrogate-safe tail that is not a slice of the input, and + `read()` reflects appends in order. +- `npx tsc -p tsconfig.node.json --noEmit`. +- Existing SessionManager suites. +- After: main `eventLoop.meanMs` and GC pressure during heavy multi-session + output should drop; no session should retain more than its cap. diff --git a/src/main/sessionManager.ts b/src/main/sessionManager.ts index 70443e6f8..11794d459 100644 --- a/src/main/sessionManager.ts +++ b/src/main/sessionManager.ts @@ -39,6 +39,7 @@ import { getToolPath, refreshToolchainFromState } from '@main/setup/toolchain.js import { resolveToolPath } from '@main/setup/binaryResolver.js' import { updateToolPaths } from '@main/setup/setupState.js' import { forgetFeedDebugSession } from '@main/storage/feedDebugLog.js' +import { CappedTextBuffer } from '@main/sessions/cappedTextBuffer.js' import type { ConditionCustomAction, ProviderConditionSnapshot, @@ -401,23 +402,6 @@ export type ResolveConditionResult = failedAtStep?: string } -function appendCappedBuffer(prev: string, data: string, cap: number): string { - let next = prev + data - if (next.length <= cap) return next - - // Naive slice by string length can split a UTF-16 surrogate pair. - // xterm will usually recover from a replacement character, but the - // replay buffer should not introduce corruption at its oldest edge - // when dropping the whole rune costs only one code unit. - let startIdx = next.length - cap - const firstCode = next.charCodeAt(startIdx) - if (firstCode >= 0xdc00 && firstCode <= 0xdfff) { - startIdx += 1 - } - next = next.slice(startIdx) - return next -} - export class SessionManager extends EventEmitter { private readonly sessions = new Map() private readonly spawningSessionGenerations = new Map() @@ -613,7 +597,10 @@ export class SessionManager extends EventEmitter { // duplicate bytes (once in the buffer, once as a live event). // Toggling the flag in the same synchronous tick as the buffer // read means no data can slip through between the two. - private readonly terminalBuffers = new Map() + // WHY CappedTextBuffer and not a string: see src/main/sessions/ + // cappedTextBuffer.ts — the string version copied the whole cap on every + // chunk and retained sliced-string parents (#726). + private readonly terminalBuffers = new Map() private readonly terminalAttached = new Set() // Agent PTY terminal replay state. Agent sessions already publish @@ -631,7 +618,7 @@ export class SessionManager extends EventEmitter { // terminal, and agent PTYs can be noisy. Buffer in main, broadcast // only after an attach, and let the renderer replay the buffer before // draining live bytes. - private readonly agentPtyBuffers = new Map() + private readonly agentPtyBuffers = new Map() private readonly agentPtyAttachCounts = new Map() private readonly agentPtyRestoreSizes = new Map() @@ -2571,7 +2558,7 @@ export class SessionManager extends EventEmitter { }) this.sessionSizes.set(sessionId, initialSize) - this.agentPtyBuffers.set(sessionId, '') + this.agentPtyBuffers.set(sessionId, new CappedTextBuffer(AGENT_PTY_BUFFER_CAP)) session.on('started', ({ projectDir }) => { if (!ownsEntry()) return this.markActivity(sessionId) @@ -2608,11 +2595,12 @@ export class SessionManager extends EventEmitter { session.on('pty-data', (data: string) => { if (!ownsEntry()) return this.markActivity(sessionId) - const prev = this.agentPtyBuffers.get(sessionId) ?? '' - this.agentPtyBuffers.set( - sessionId, - appendCappedBuffer(prev, data, AGENT_PTY_BUFFER_CAP), - ) + let replay = this.agentPtyBuffers.get(sessionId) + if (!replay) { + replay = new CappedTextBuffer(AGENT_PTY_BUFFER_CAP) + this.agentPtyBuffers.set(sessionId, replay) + } + replay.append(data) if ((this.agentPtyAttachCounts.get(sessionId) ?? 0) > 0) { this.emit('agent-pty-data', { sessionId, data }) } @@ -2925,7 +2913,7 @@ export class SessionManager extends EventEmitter { // and is replayed to the renderer on attach — see the block // comment on terminalBuffers above for the full reasoning. this.sessionSizes.set(sessionId, initialSize) - this.terminalBuffers.set(sessionId, '') + this.terminalBuffers.set(sessionId, new CappedTextBuffer(TERMINAL_BUFFER_CAP)) // Terminal sessions only emit started / data / exit. The 'data' // event carries raw PTY bytes for xterm.js on the renderer side; @@ -2951,11 +2939,12 @@ export class SessionManager extends EventEmitter { // replay the full history. Cap at TERMINAL_BUFFER_CAP — // longer sessions just lose the oldest bytes, which is the // standard terminal scrollback behavior. - const prev = this.terminalBuffers.get(sessionId) ?? '' - this.terminalBuffers.set( - sessionId, - appendCappedBuffer(prev, data, TERMINAL_BUFFER_CAP), - ) + let replay = this.terminalBuffers.get(sessionId) + if (!replay) { + replay = new CappedTextBuffer(TERMINAL_BUFFER_CAP) + this.terminalBuffers.set(sessionId, replay) + } + replay.append(data) // Only broadcast live events AFTER the renderer has attached. // Before attach, the data is still in the buffer and will be // replayed when the renderer calls attachTerminal. See the @@ -3118,7 +3107,7 @@ export class SessionManager extends EventEmitter { ) return '' } - const buffer = this.terminalBuffers.get(sessionId) ?? '' + const buffer = this.terminalBuffers.get(sessionId)?.read() ?? '' // Flip the attach flag in the SAME synchronous block as reading // the buffer. JavaScript is single-threaded and event emission // can only happen on a later tick, so nothing can sneak in. @@ -3166,7 +3155,7 @@ export class SessionManager extends EventEmitter { ) return null } - const buffer = this.agentPtyBuffers.get(sessionId) ?? '' + const buffer = this.agentPtyBuffers.get(sessionId)?.read() ?? '' const attachCount = this.agentPtyAttachCounts.get(sessionId) ?? 0 if (attachCount === 0) { const currentSize = this.sessionSizes.get(sessionId) diff --git a/src/main/sessions/cappedTextBuffer.test.ts b/src/main/sessions/cappedTextBuffer.test.ts new file mode 100644 index 000000000..95b0f0045 --- /dev/null +++ b/src/main/sessions/cappedTextBuffer.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, it } from 'vitest' + +import { CappedTextBuffer } from './cappedTextBuffer.js' + +describe('CappedTextBuffer', () => { + it('starts empty and ignores empty appends', () => { + const buffer = new CappedTextBuffer(8) + buffer.append('') + expect(buffer.length).toBe(0) + expect(buffer.read()).toBe('') + }) + + it('rejects a non-positive cap and a piece size that cannot hold a surrogate pair', () => { + expect(() => new CappedTextBuffer(0)).toThrow(RangeError) + expect(() => new CappedTextBuffer(8, 1)).toThrow(RangeError) + expect(() => new CappedTextBuffer(8, 2.5)).toThrow(RangeError) + }) + + it('keeps the newest whole chunks under the cap, oldest first', () => { + const buffer = new CappedTextBuffer(10) + buffer.append('abcd') + buffer.append('efgh') + expect(buffer.read()).toBe('abcdefgh') + buffer.append('ijkl') + // 12 > 10: the oldest whole chunk goes, not a partial one. + expect(buffer.read()).toBe('efghijkl') + expect(buffer.length).toBe(8) + }) + + it('never exceeds the cap and drops the minimum number of whole chunks', () => { + const cap = 1000 + const buffer = new CappedTextBuffer(cap) + const appended: string[] = [] + let seed = 7 + for (let i = 0; i < 5_000; i += 1) { + seed = (seed * 48271) % 2147483647 + const chunk = String.fromCharCode(97 + (i % 26)).repeat(1 + (seed % 300)) + appended.push(chunk) + buffer.append(chunk) + + expect(buffer.length).toBeLessThanOrEqual(cap) + // Reconstruct the expected retained suffix in whole chunks. + let expected = '' + let dropped: string | null = null + for (let j = appended.length - 1; j >= 0; j -= 1) { + const candidate = appended[j]! + if (expected.length + candidate.length > cap) { + dropped = candidate + break + } + expected = candidate + expected + } + expect(buffer.read()).toBe(expected) + if (dropped !== null) { + // Minimal drop: putting the last dropped chunk back would overflow. + expect(buffer.length + dropped.length).toBeGreaterThan(cap) + } + } + }) + + it('keeps only the tail of a chunk larger than the cap', () => { + const buffer = new CappedTextBuffer(8, 2) + buffer.append('old') + buffer.append('0123456789abcdef') + expect(buffer.read()).toBe('89abcdef') + expect(buffer.length).toBe(8) + // Later appends continue from that tail: the oversized chunk was stored + // as pieces, so only the oldest piece goes, not the whole replay. + buffer.append('xy') + expect(buffer.read()).toBe('abcdefxy') + }) + + it('bounds overflow loss to one piece even for a near-cap chunk', () => { + const buffer = new CappedTextBuffer(64, 8) + buffer.append('a'.repeat(60)) + buffer.append('bbbb') + expect(buffer.length).toBe(64) + buffer.append('c') + // 65 > 64: one 8-unit piece of the 60-unit chunk is dropped, not all of it. + expect(buffer.length).toBe(57) + expect(buffer.read()).toBe('a'.repeat(52) + 'bbbbc') + }) + + it('never splits a surrogate pair across pieces', () => { + const buffer = new CappedTextBuffer(64, 3) + // With a 3-unit piece the naive cuts would land INSIDE both pairs + // (ab|😀 → 'ab' + high, then …); the boundary must pull back so the + // pieces come out as 'ab', '😀c', 'd😀', 'e'. + const chunk = 'ab\u{1F600}cd\u{1F600}e' + buffer.append(chunk) + expect(buffer.read()).toBe(chunk) + // Evict exactly one piece at a time (cap 64, 9 units retained): each + // step's retained text pins the piece boundaries. Without the pull-back + // the pieces would be 'ab\uD83D', '\uDE00cd', … and the second step + // would leave a lone low surrogate at the head. + buffer.append('x'.repeat(57)) + expect(buffer.read()).toBe('\u{1F600}cd\u{1F600}e' + 'x'.repeat(57)) + buffer.append('xx') + expect(buffer.read()).toBe('d\u{1F600}e' + 'x'.repeat(59)) + buffer.append('xxx') + expect(buffer.read()).toBe('e' + 'x'.repeat(62)) + }) + + it('holds its invariants with piece cutting and oversized chunks under fuzz', () => { + const cap = 1000 + const pieceSize = 37 + const buffer = new CappedTextBuffer(cap, pieceSize) + let stream = '' + let seed = 11 + const alphabet = ['a', 'b', '\u{1F600}', 'c', '\u{1F4A9}', 'd'] + for (let i = 0; i < 4_000; i += 1) { + seed = (seed * 48271) % 2147483647 + const units = 1 + (seed % 1_500) + let chunk = '' + while (chunk.length < units) chunk += alphabet[(seed + chunk.length) % alphabet.length]! + stream += chunk + buffer.append(chunk) + + const text = buffer.read() + expect(text.length).toBe(buffer.length) + expect(buffer.length).toBeLessThanOrEqual(cap) + expect(stream.endsWith(text)).toBe(true) + if (stream.length > cap) expect(buffer.length).toBeGreaterThan(cap - pieceSize) + const first = text.charCodeAt(0) + expect(first >= 0xdc00 && first <= 0xdfff).toBe(false) + } + }) + + it('does not start the tail of an oversized chunk on a low surrogate', () => { + // 'ab' + 😀 (two code units) + 'cd' is six code units. + const chunk = 'ab\u{1F600}cd' + const onHigh = new CappedTextBuffer(4) + onHigh.append(chunk) + expect(onHigh.read()).toBe('\u{1F600}cd') + + const onLow = new CappedTextBuffer(3) + onLow.append(chunk) + // A 3-unit tail would begin with the low surrogate; drop it instead. + expect(onLow.read()).toBe('cd') + }) + + it('stays correct across lazy compaction of the dropped prefix', () => { + const buffer = new CappedTextBuffer(50) + const chunks: string[] = [] + for (let i = 0; i < 1_000; i += 1) { + const chunk = `${i}|`.padStart(5, '0') + chunks.push(chunk) + buffer.append(chunk) + } + expect(buffer.length).toBe(50) + expect(buffer.read()).toBe(chunks.slice(-10).join('')) + }) +}) diff --git a/src/main/sessions/cappedTextBuffer.ts b/src/main/sessions/cappedTextBuffer.ts new file mode 100644 index 000000000..6e5b05983 --- /dev/null +++ b/src/main/sessions/cappedTextBuffer.ts @@ -0,0 +1,159 @@ +// Bounded text replay buffer for PTY output (#726). +// +// WHY a chunk list and not a string: SessionManager keeps the last N KiB of +// every session's PTY output so a terminal that attaches later can replay +// the current screen. The previous implementation stored one string per +// session and appended with `prev + data` → `charCodeAt` → `slice`. That is +// three problems in one line once the buffer is full (minutes into any agent +// session, because TUIs repaint the whole screen): +// +// 1. `prev + data` is a ConsString; the `charCodeAt` needed for the +// surrogate check flattens it into a fresh (cap + chunk) copy. Every +// chunk from every session therefore copied the whole cap on the main +// thread — with 20 sessions repainting, on the order of 100 MB/s of +// short-lived allocation, i.e. the #390 GC-storm shape. +// 2. `slice` returns a V8 SlicedString that keeps that flattened parent +// alive, so the retained buffer was cap + chunk, not cap. +// 3. A chunk larger than the cap became the parent of the stored slice and +// was retained in full: the 2026-09-03 heap snapshot holds six identical +// 1.8 MB docker-build repaints and ~10 identical 1 MB Claude screen +// frames for exactly this reason (#321 is the same lesson elsewhere). +// +// A list of pieces fixes all three: append is amortised O(chunk), dropping +// whole oldest pieces never copies retained bytes and can never split a +// surrogate pair (PTY chunks arrive as already-decoded strings and pieces +// are cut on surrogate-safe boundaries), and the only O(cap) operation — +// the join — runs on attach, which is rare. +// +// WHY large chunks are split into pieces: eviction is whole-piece, so the +// piece size is the granularity of what an overflow discards. Storing a +// near-cap chunk as one piece would make the very next small append throw +// away the whole replay. Pieces are cut as forced flat copies (the utf16le +// Buffer round-trip is this codebase's established idiom, see +// src/main/subagents/shared.ts) so no piece keeps a large parent alive. + +const DEFAULT_MIN_PIECE = 4096 + +export class CappedTextBuffer { + private pieces: string[] = [] + // Index of the oldest live piece. Dropped pieces are blanked in place and + // the array is compacted lazily, so a drop is O(1) instead of an O(n) + // `shift` on every full-buffer append. + private head = 0 + private live = 0 + readonly pieceSize: number + + constructor(readonly cap: number, pieceSize?: number) { + if (!(cap > 0)) throw new RangeError(`CappedTextBuffer cap must be positive, got ${cap}`) + // A sixteenth of the cap bounds overflow loss to ~6% of the replay, with + // a floor so small caps (tests, tiny terminals) do not shred every chunk. + // At least 2 so a piece can always hold a whole surrogate pair — that is + // what lets cutPieces guarantee it never splits one. + this.pieceSize = pieceSize ?? Math.max(DEFAULT_MIN_PIECE, cap >> 4) + if (!Number.isInteger(this.pieceSize) || this.pieceSize < 2) { + throw new RangeError(`CappedTextBuffer pieceSize must be an integer >= 2, got ${this.pieceSize}`) + } + } + + /** Total retained code units. Never exceeds `cap`. */ + get length(): number { + return this.live + } + + // INVARIANT relied on here: a chunk never begins with a low surrogate or + // ends with a high one, i.e. no code point is split across chunks. node-pty + // guarantees that (its data events come through a StringDecoder), and all + // three emitters pass those strings straight through. The old string + // implementation could tolerate a split pair because it re-cut the combined + // string; this one cuts pieces on surrogate-safe boundaries within a chunk + // and evicts whole pieces, so it needs the chunks themselves to be whole. + // + // Chunks up to pieceSize are stored by reference. That is the right trade + // for node-pty's freshly decoded flat strings; a caller handing in a slice + // of a larger string would keep that parent alive for the piece's lifetime. + append(chunk: string): void { + if (chunk.length === 0) return + let start = 0 + if (chunk.length > this.cap) { + // The new chunk alone overflows the cap: everything older is gone by + // definition, and only a flat copy of the chunk's tail may be kept — + // never a slice, which would retain the whole oversized chunk (#321). + // Cutting straight from the tail offset copies each byte once. + start = surrogateSafeStart(chunk, chunk.length - this.cap) + this.pieces = [] + this.head = 0 + this.live = 0 + } + if (start === 0 && chunk.length <= this.pieceSize) { + this.push(chunk) + } else { + for (const piece of cutPieces(chunk, start, this.pieceSize)) this.push(piece) + } + // Compact once the dead prefix dominates so the array stays O(live + // pieces) rather than O(pieces ever appended). The threshold keeps the + // compaction amortised: at most one O(n) slice per n/2 drops. + if (this.head >= 64 && this.head * 2 >= this.pieces.length) { + this.pieces = this.pieces.slice(this.head) + this.head = 0 + } + } + + /** The retained tail as one string, oldest bytes first. */ + read(): string { + if (this.live === 0) return '' + const livePieces = this.head === 0 ? this.pieces : this.pieces.slice(this.head) + return livePieces.length === 1 ? livePieces[0]! : livePieces.join('') + } + + private push(piece: string): void { + this.pieces.push(piece) + this.live += piece.length + // Every pushed piece is at most `cap` long (oversized chunks were cut to + // the cap above), so this loop stops before reaching it: the newest + // piece is never dropped. + while (this.live > this.cap) { + const dropped = this.pieces[this.head]! + this.pieces[this.head] = '' + this.head += 1 + this.live -= dropped.length + } + } +} + +// Cut `chunk[start..]` into flat pieces of at most `size` code units on +// surrogate-safe boundaries. Each piece is a real copy so none of them keeps +// `chunk` alive. With size >= 2 (enforced by the constructor) pulling a +// boundary back off a high surrogate always leaves a non-empty piece, so the +// loop cannot stall and no pair is ever split. +function cutPieces(chunk: string, start: number, size: number): string[] { + const pieces: string[] = [] + while (start < chunk.length) { + let end = Math.min(chunk.length, start + size) + // Never end a piece on a high surrogate: pull the pair into the next + // piece so no piece boundary splits a code point. + if (end < chunk.length) { + const code = chunk.charCodeAt(end - 1) + if (code >= 0xd800 && code <= 0xdbff) end -= 1 + } + pieces.push(flatCopy(chunk, start, end)) + start = end + } + return pieces +} + +// Never start the retained tail on a low surrogate — the old string +// implementation made the same cut, for the same reason: a replay buffer must +// not introduce a replacement character at its oldest edge when dropping one +// more code unit costs nothing. +function surrogateSafeStart(chunk: string, start: number): number { + const code = chunk.charCodeAt(start) + return code >= 0xdc00 && code <= 0xdfff ? start + 1 : start +} + +// WHY the Buffer round-trip: `chunk.slice(a, b)` is a V8 SlicedString that +// keeps the whole source alive (#321). Round-tripping through a utf16le +// Buffer forces a flat copy of just the range, and utf16le preserves every +// code unit — including lone surrogates — unchanged. +function flatCopy(chunk: string, start: number, end: number): string { + return Buffer.from(chunk.slice(start, end), 'utf16le').toString('utf16le') +}