From 43bd8915b366cd9e1a5090ff1a3a46a4e242f7e4 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:31:06 -0700 Subject: [PATCH] fix(daemon): budget scrollback by retained cells --- .../daemon/daemon-scrollback-budget.test.ts | 128 ++++++++++-------- src/main/daemon/daemon-scrollback-budget.ts | 114 ++++++++++++---- .../terminal-host-scrollback-budget.test.ts | 80 ++++++++++- src/main/daemon/terminal-host.ts | 37 +++-- 4 files changed, 259 insertions(+), 100 deletions(-) diff --git a/src/main/daemon/daemon-scrollback-budget.test.ts b/src/main/daemon/daemon-scrollback-budget.test.ts index 30185869a97..499eece6f14 100644 --- a/src/main/daemon/daemon-scrollback-budget.test.ts +++ b/src/main/daemon/daemon-scrollback-budget.test.ts @@ -1,80 +1,96 @@ import { describe, expect, it } from 'vitest' import { - computeSessionScrollbackRows, - DAEMON_SCROLLBACK_BUDGET_ROWS, + allocateSessionScrollbackRows, + DAEMON_SCROLLBACK_BUDGET_CELLS, DAEMON_SCROLLBACK_FULL_ROWS, - DAEMON_SCROLLBACK_MIN_ROWS + DAEMON_SCROLLBACK_PREFERRED_MIN_ROWS, + resolveDaemonScrollbackBudgetCells } from './daemon-scrollback-budget' -describe('computeSessionScrollbackRows', () => { - it('keeps full depth while the budget is not binding', () => { - expect(computeSessionScrollbackRows(1, { budgetRows: 100_000 })).toBe( - DAEMON_SCROLLBACK_FULL_ROWS +function retainedCells(columns: readonly number[], rows: readonly number[]): number { + return rows.reduce((total, rowCount, index) => total + rowCount * columns[index], 0) +} + +describe('allocateSessionScrollbackRows', () => { + it('preserves full restore depth whenever every session fits', () => { + expect(allocateSessionScrollbackRows(Array(25).fill(80))).toEqual(Array(25).fill(5000)) + expect(allocateSessionScrollbackRows(Array(12).fill(160))).toEqual(Array(12).fill(5000)) + expect(allocateSessionScrollbackRows([400])).toEqual([5000]) + }) + + it('matches the row-budget behavior for equal-width sessions', () => { + expect(allocateSessionScrollbackRows(Array(26).fill(80))).toEqual( + Array(26).fill(Math.floor(DAEMON_SCROLLBACK_BUDGET_CELLS / 26 / 80)) ) - expect(computeSessionScrollbackRows(20, { budgetRows: 100_000 })).toBe( - DAEMON_SCROLLBACK_FULL_ROWS + expect(allocateSessionScrollbackRows(Array(40).fill(80), { budgetCells: 8_000_000 })).toEqual( + Array(40).fill(2500) ) }) - it('preserves the default 5000-row restore depth through 25 sessions', () => { - expect(computeSessionScrollbackRows(25)).toBe(DAEMON_SCROLLBACK_FULL_ROWS) - expect(computeSessionScrollbackRows(26)).toBe(Math.floor(DAEMON_SCROLLBACK_BUDGET_ROWS / 26)) + it('returns unused narrow-session capacity to wider sessions', () => { + const columns = [...Array(20).fill(40), ...Array(20).fill(120)] + const rows = allocateSessionScrollbackRows(columns, { budgetCells: 8_000_000 }) + + expect(rows.slice(0, 20)).toEqual(Array(20).fill(DAEMON_SCROLLBACK_FULL_ROWS)) + expect(new Set(rows.slice(20))).toEqual(new Set([1666])) + expect(retainedCells(columns, rows)).toBeLessThanOrEqual(8_000_000) }) - it('splits the budget evenly once it binds', () => { - expect(computeSessionScrollbackRows(40, { budgetRows: 100_000 })).toBe(2500) - expect(computeSessionScrollbackRows(50, { budgetRows: 100_000 })).toBe(2000) - }) - - it('never drops below the floor', () => { - expect(computeSessionScrollbackRows(10_000, { budgetRows: 100_000 })).toBe( - DAEMON_SCROLLBACK_MIN_ROWS - ) - }) - - it('holds total retained rows within the budget until the floor binds', () => { - for (const sessions of [30, 60, 120, 200]) { - const total = computeSessionScrollbackRows(sessions, { budgetRows: 100_000 }) * sessions - expect(total).toBeLessThanOrEqual(100_000) + it('holds configured scrollback cells within the budget across widths', () => { + for (const columns of [ + Array(100).fill(80), + Array(100).fill(160), + [...Array(60).fill(40), ...Array(60).fill(200)] + ]) { + const rows = allocateSessionScrollbackRows(columns) + expect(retainedCells(columns, rows)).toBeLessThanOrEqual(DAEMON_SCROLLBACK_BUDGET_CELLS) } }) - it('lets the floor override the budget, trading bounded growth for a usable restore', () => { - // Past ~200 sessions the even split would fall below MIN_ROWS, which reads as data loss on - // reattach. Growth past that point is deliberate but shallow: 500 rows, not 5000. - const perSession = computeSessionScrollbackRows(1000, { budgetRows: 100_000 }) - expect(perSession).toBe(DAEMON_SCROLLBACK_MIN_ROWS) - expect(perSession * 1000).toBeGreaterThan(100_000) - expect(perSession * 1000).toBeLessThan(DAEMON_SCROLLBACK_FULL_ROWS * 1000) + it('keeps the preferred floor while it fits', () => { + const columns = Array(200).fill(100) + const rows = allocateSessionScrollbackRows(columns) + + expect(Math.min(...rows)).toBeGreaterThanOrEqual(DAEMON_SCROLLBACK_PREFERRED_MIN_ROWS) }) - it('caps the session count that broke the Windows host well below its old retention', () => { - // 100+ live terminals previously retained 100 x 5000 = 500k rows (~1 GB of grid). - const perSession = computeSessionScrollbackRows(100) - expect(perSession).toBeLessThan(DAEMON_SCROLLBACK_FULL_ROWS) - expect(perSession * 100).toBeLessThanOrEqual(DAEMON_SCROLLBACK_BUDGET_ROWS) + it('lets the hard capacity win once the preferred floor cannot fit', () => { + const columns = Array(1000).fill(80) + const rows = allocateSessionScrollbackRows(columns) + + expect(new Set(rows)).toEqual(new Set([128])) + expect(retainedCells(columns, rows)).toBeLessThanOrEqual(DAEMON_SCROLLBACK_BUDGET_CELLS) }) - it('treats a degenerate session count as a single session', () => { - expect(computeSessionScrollbackRows(0)).toBe(DAEMON_SCROLLBACK_FULL_ROWS) - expect(computeSessionScrollbackRows(Number.NaN)).toBe(DAEMON_SCROLLBACK_FULL_ROWS) + it('uses a conservative standard width for invalid column counts', () => { + expect(allocateSessionScrollbackRows(Array(30).fill(Number.NaN))).toEqual( + allocateSessionScrollbackRows(Array(30).fill(80)) + ) }) - it('accepts an env override and ignores an unusable one', () => { - const env = { ORCA_DAEMON_SCROLLBACK_BUDGET_ROWS: '20000' } as NodeJS.ProcessEnv - expect(computeSessionScrollbackRows(1, { env })).toBe(DAEMON_SCROLLBACK_FULL_ROWS) - expect(computeSessionScrollbackRows(10, { env })).toBe(2000) - // Why: an unparseable or non-positive override must not disable the budget — unbounded is the bug. - for (const raw of ['nonsense', '0', '-5']) { - const bad = { ORCA_DAEMON_SCROLLBACK_BUDGET_ROWS: raw } as NodeJS.ProcessEnv - expect(computeSessionScrollbackRows(100, { env: bad })).toBe( - computeSessionScrollbackRows(100, { budgetRows: DAEMON_SCROLLBACK_BUDGET_ROWS }) - ) - } + it('accounts for the headless emulator minimum width', () => { + expect(allocateSessionScrollbackRows([1, ...Array(26).fill(80)])).toEqual( + allocateSessionScrollbackRows([2, ...Array(26).fill(80)]) + ) + }) +}) + +describe('resolveDaemonScrollbackBudgetCells', () => { + it('accepts a tighter base-10 integer override', () => { + expect( + resolveDaemonScrollbackBudgetCells({ + ORCA_DAEMON_SCROLLBACK_BUDGET_CELLS: '1600000' + } as NodeJS.ProcessEnv) + ).toBe(1_600_000) }) - it('honors a tightened budget that binds with one session', () => { - const env = { ORCA_DAEMON_SCROLLBACK_BUDGET_ROWS: '2000' } as NodeJS.ProcessEnv - expect(computeSessionScrollbackRows(1, { env })).toBe(2000) + it('rejects malformed values and values that weaken the bound', () => { + for (const raw of ['', 'nonsense', '0', '-5', '1.5', '1e6', '0x100', '999999999999']) { + expect( + resolveDaemonScrollbackBudgetCells({ + ORCA_DAEMON_SCROLLBACK_BUDGET_CELLS: raw + } as NodeJS.ProcessEnv) + ).toBe(DAEMON_SCROLLBACK_BUDGET_CELLS) + } }) }) diff --git a/src/main/daemon/daemon-scrollback-budget.ts b/src/main/daemon/daemon-scrollback-budget.ts index 7c04b5bac55..09c28b63f8c 100644 --- a/src/main/daemon/daemon-scrollback-budget.ts +++ b/src/main/daemon/daemon-scrollback-budget.ts @@ -1,43 +1,97 @@ -const DAEMON_SCROLLBACK_ENV_VAR = 'ORCA_DAEMON_SCROLLBACK_BUDGET_ROWS' +const DAEMON_SCROLLBACK_ENV_VAR = 'ORCA_DAEMON_SCROLLBACK_BUDGET_CELLS' +const DAEMON_SCROLLBACK_REFERENCE_COLUMNS = 80 +const HEADLESS_EMULATOR_MIN_COLUMNS = 2 /** Depth a session retains while the daemon is under budget; matches HeadlessEmulator's default. */ export const DAEMON_SCROLLBACK_FULL_ROWS = 5000 -// Why: below this a reattach loses the command that produced the visible screen, which reads as data loss -// rather than degradation. Bounded growth past the floor is preferable to an unusable restore. -export const DAEMON_SCROLLBACK_MIN_ROWS = 500 -// Why: retained rows are the daemon's dominant heap term (an xterm row is ~2 KB at typical widths), and -// session count is unbounded — a host owning 100+ terminals retained ~1 GB of grid and was OOM-killed, -// taking its sessions with it. ~128k rows caps the grid near 256 MB, which is ~25 sessions at full depth. -export const DAEMON_SCROLLBACK_BUDGET_ROWS = 128_000 +// Why: below this a reattach can lose the command that produced the visible screen. +export const DAEMON_SCROLLBACK_PREFERRED_MIN_ROWS = 500 +// 128k rows at 80 columns keeps 25 standard-width sessions at full depth. +export const DAEMON_SCROLLBACK_BUDGET_CELLS = 128_000 * DAEMON_SCROLLBACK_REFERENCE_COLUMNS -function parseBudgetOverrideRows(value: string | undefined): number | undefined { +function parseBudgetOverrideCells(value: string | undefined): number | undefined { if (value === undefined) { return undefined } - const parsed = Number(value.trim()) - // Why: ignore an unparseable or non-positive override rather than disabling the budget entirely — - // an unbounded daemon is the failure this module exists to prevent. - if (!Number.isFinite(parsed) || parsed <= 0) { + const normalized = value.trim() + if (!/^[1-9]\d*$/.test(normalized)) { return undefined } - return Math.floor(parsed) + const parsed = Number(normalized) + // The override may tighten the safety bound, never silently weaken it. + if (!Number.isSafeInteger(parsed) || parsed > DAEMON_SCROLLBACK_BUDGET_CELLS) { + return undefined + } + return parsed } -/** - * Rows each live session may retain so the daemon's total stays within the budget. Full depth until the - * budget binds, then an even split down to the floor. Pure so the tiers are unit-testable without a daemon. - */ -export function computeSessionScrollbackRows( - liveSessionCount: number, - options: { budgetRows?: number; env?: NodeJS.ProcessEnv } = {} -): number { - const budgetRows = - options.budgetRows ?? - parseBudgetOverrideRows((options.env ?? process.env)[DAEMON_SCROLLBACK_ENV_VAR]) ?? - DAEMON_SCROLLBACK_BUDGET_ROWS - if (!Number.isFinite(liveSessionCount) || liveSessionCount <= 0) { - return DAEMON_SCROLLBACK_FULL_ROWS +export function resolveDaemonScrollbackBudgetCells(env: NodeJS.ProcessEnv = process.env): number { + return parseBudgetOverrideCells(env[DAEMON_SCROLLBACK_ENV_VAR]) ?? DAEMON_SCROLLBACK_BUDGET_CELLS +} + +function normalizeColumns(columns: number): number { + return Number.isFinite(columns) && columns >= 1 + ? Math.max(HEADLESS_EMULATOR_MIN_COLUMNS, Math.floor(columns)) + : DAEMON_SCROLLBACK_REFERENCE_COLUMNS +} + +function rowsForCellShare(columns: number, cellShare: number, minimumRows: number): number { + return Math.min( + DAEMON_SCROLLBACK_FULL_ROWS, + Math.max(minimumRows, Math.floor(cellShare / columns)) + ) +} + +/** Max-min-fair row limits under a deterministic aggregate grid-cell capacity. */ +export function allocateSessionScrollbackRows( + sessionColumns: readonly number[], + options: { budgetCells?: number } = {} +): number[] { + if (sessionColumns.length === 0) { + return [] } - const evenShare = Math.floor(budgetRows / liveSessionCount) - return Math.min(DAEMON_SCROLLBACK_FULL_ROWS, Math.max(DAEMON_SCROLLBACK_MIN_ROWS, evenShare)) + const columns = sessionColumns.map(normalizeColumns) + const configuredBudget = options.budgetCells + const budgetCells = + configuredBudget !== undefined && + Number.isSafeInteger(configuredBudget) && + configuredBudget > 0 && + configuredBudget <= DAEMON_SCROLLBACK_BUDGET_CELLS + ? configuredBudget + : DAEMON_SCROLLBACK_BUDGET_CELLS + const fullDepthCells = columns.reduce( + (total, cols) => total + cols * DAEMON_SCROLLBACK_FULL_ROWS, + 0 + ) + if (fullDepthCells <= budgetCells) { + return columns.map(() => DAEMON_SCROLLBACK_FULL_ROWS) + } + const preferredFloorCells = columns.reduce( + (total, cols) => total + cols * DAEMON_SCROLLBACK_PREFERRED_MIN_ROWS, + 0 + ) + const minimumRows = preferredFloorCells <= budgetCells ? DAEMON_SCROLLBACK_PREFERRED_MIN_ROWS : 0 + + const fits = (cellShare: number): boolean => { + let totalCells = 0 + for (const cols of columns) { + totalCells += cols * rowsForCellShare(cols, cellShare, minimumRows) + if (totalCells > budgetCells) { + return false + } + } + return true + } + + let low = 0 + let high = budgetCells + while (low < high) { + const mid = Math.floor((low + high + 1) / 2) + if (fits(mid)) { + low = mid + } else { + high = mid - 1 + } + } + return columns.map((cols) => rowsForCellShare(cols, low, minimumRows)) } diff --git a/src/main/daemon/terminal-host-scrollback-budget.test.ts b/src/main/daemon/terminal-host-scrollback-budget.test.ts index eebdbf7bb88..25380a0c0c1 100644 --- a/src/main/daemon/terminal-host-scrollback-budget.test.ts +++ b/src/main/daemon/terminal-host-scrollback-budget.test.ts @@ -8,7 +8,7 @@ import { TerminalHost } from './terminal-host' import type { SubprocessHandle } from './session' import { HeadlessEmulator } from './headless-emulator' import { - computeSessionScrollbackRows, + allocateSessionScrollbackRows, DAEMON_SCROLLBACK_FULL_ROWS } from './daemon-scrollback-budget' @@ -57,10 +57,10 @@ describe('TerminalHost scrollback budget', () => { applyRows.mockRestore() }) - async function create(sessionId: string): Promise { + async function create(sessionId: string, cols = 80): Promise { await host.createOrAttach({ sessionId, - cols: 80, + cols, rows: 24, streamClient: { onData: vi.fn(), onExit: vi.fn() } }) @@ -85,7 +85,7 @@ describe('TerminalHost scrollback budget', () => { const lastFour = applyRows.mock.calls.slice(-4) expect(lastFour).toHaveLength(4) for (const call of lastFour) { - expect(call[0]).toBe(computeSessionScrollbackRows(4)) + expect(call[0]).toBe(DAEMON_SCROLLBACK_FULL_ROWS) } }) @@ -105,17 +105,85 @@ describe('TerminalHost scrollback budget', () => { await create(`session-${i}`) } const applied = lastAppliedRows() - expect(applied).toBe(computeSessionScrollbackRows(SESSIONS)) + expect(applied).toBe(allocateSessionScrollbackRows(Array(SESSIONS).fill(80))[0]) expect(applied).toBeLessThan(DAEMON_SCROLLBACK_FULL_ROWS) }) + it('accounts for each session width without stranding narrow-session capacity', async () => { + const columns: number[] = [] + for (let i = 0; i < 20; i++) { + await create(`narrow-${i}`, 40) + await create(`wide-${i}`, 120) + columns.push(40, 120) + } + + expect(applyRows.mock.calls.slice(-40).map((call) => call[0])).toEqual( + allocateSessionScrollbackRows(columns) + ) + }) + it('applies each concurrent create before yielding', async () => { const pending = Array.from({ length: 30 }, (_, i) => create(`concurrent-${i}`)) - expect(lastAppliedRows()).toBe(computeSessionScrollbackRows(30)) + expect(lastAppliedRows()).toBe(allocateSessionScrollbackRows(Array(30).fill(80))[0]) await Promise.all(pending) }) + it('applies a width-aware budget before resizing the emulator', async () => { + const resizeEmulator = vi.spyOn(HeadlessEmulator.prototype, 'resize') + try { + for (let i = 0; i < 25; i++) { + await create(`session-${i}`) + } + expect(lastAppliedRows()).toBe(DAEMON_SCROLLBACK_FULL_ROWS) + applyRows.mockClear() + + host.resize('session-0', 160, 24) + + expect(applyRows.mock.calls.map((call) => call[0])).toEqual( + allocateSessionScrollbackRows([160, ...Array(24).fill(80)]) + ) + expect(applyRows.mock.invocationCallOrder.at(-1)).toBeLessThan( + resizeEmulator.mock.invocationCallOrder[0] + ) + + applyRows.mockClear() + resizeEmulator.mockClear() + host.resize('session-0', 80, 24) + expect(applyRows.mock.calls.map((call) => call[0])).toEqual(Array(25).fill(5000)) + expect(applyRows.mock.invocationCallOrder.at(-1)).toBeLessThan( + resizeEmulator.mock.invocationCallOrder[0] + ) + } finally { + resizeEmulator.mockRestore() + } + }) + + it('does not scan sessions again for a row-only or rejected resize', async () => { + await create('a') + await create('b') + applyRows.mockClear() + + host.resize('a', 80, 40) + host.resize('a', Number.NaN, 40) + + expect(applyRows).not.toHaveBeenCalled() + }) + + it('budgets a one-column resize at the headless emulator applied width', async () => { + for (let i = 0; i < 27; i++) { + await create(`session-${i}`) + } + applyRows.mockClear() + + host.resize('session-0', 1, 24) + + expect(host.getAppliedSize('session-0')?.cols).toBe(2) + expect(applyRows.mock.calls.map((call) => call[0])).toEqual( + allocateSessionScrollbackRows([2, ...Array(26).fill(80)]) + ) + }) + it('restores depth to survivors when sessions exit', async () => { const SESSIONS = 60 for (let i = 0; i < SESSIONS; i++) { diff --git a/src/main/daemon/terminal-host.ts b/src/main/daemon/terminal-host.ts index 0ff3378bff3..700efc1e941 100644 --- a/src/main/daemon/terminal-host.ts +++ b/src/main/daemon/terminal-host.ts @@ -16,7 +16,11 @@ import { resolveTerminalHostSessionCwd } from './terminal-host-session-cwd' import { TerminalHostTombstones } from './terminal-host-tombstones' import { listLiveTerminalHostSessions } from './terminal-host-session-listing' import { createOrAttachTerminalSession } from './terminal-host-session-create' -import { computeSessionScrollbackRows } from './daemon-scrollback-budget' +import { + allocateSessionScrollbackRows, + resolveDaemonScrollbackBudgetCells +} from './daemon-scrollback-budget' +import { isValidPtySize } from './daemon-pty-size' import { isShellProcess } from '../../shared/agent-detection' export type { CreateOrAttachOptions, CreateOrAttachResult } from './terminal-host-create-contract' @@ -36,6 +40,7 @@ export class TerminalHost { private disposePromise: Promise | null = null private readonly agentSessionOwners = new ClaimedAgentPtyOwnerRegistry() private readonly agentSessionGenerations = new TerminalHostAgentSessionGenerations() + private readonly scrollbackBudgetCells = resolveDaemonScrollbackBudgetCells() constructor(opts: TerminalHostOptions) { this.spawnSubprocess = opts.spawnSubprocess @@ -81,12 +86,19 @@ export class TerminalHost { }) } - // Why: retained rows are O(sessions × depth) and session count is unbounded, so re-split the - // daemon's row budget whenever the count changes rather than letting every session hold full depth. - private applyScrollbackBudget(): void { - const rows = computeSessionScrollbackRows(this.sessions.size) - for (const session of this.sessions.values()) { - session.setRetainedScrollbackRows(rows) + // Why: retained grid is O(columns × depth), so keep one width-aware budget across live sessions. + private applyScrollbackBudget(proposedSize?: { sessionId: string; columns: number }): void { + const sessions = [...this.sessions.entries()] + const rowLimits = allocateSessionScrollbackRows( + sessions.map(([sessionId, session]) => + sessionId === proposedSize?.sessionId + ? proposedSize.columns + : (session.getAppliedSize()?.cols ?? 80) + ), + { budgetCells: this.scrollbackBudgetCells } + ) + for (let index = 0; index < sessions.length; index += 1) { + sessions[index][1].setRetainedScrollbackRows(rowLimits[index]) } } @@ -99,7 +111,16 @@ export class TerminalHost { } resize(sessionId: string, cols: number, rows: number): void { - this.getAliveSession(sessionId).resize(cols, rows) + const session = this.getAliveSession(sessionId) + if (!isValidPtySize(cols, rows)) { + return + } + const previousColumns = session.getAppliedSize()?.cols + if (cols !== previousColumns) { + // Tighten before xterm widens its retained lines; loosen before narrowing reflow can drop history. + this.applyScrollbackBudget({ sessionId, columns: cols }) + } + session.resize(cols, rows) } // Why null-not-throw (unlike write/resize): pause/resume are best-effort hints against a session that may have exited.