From c9515b1d4c6d2469179506b78bb9d4b3956582b7 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Tue, 15 Sep 2026 12:56:09 +0200 Subject: [PATCH] fix(terminal): keep the output a pane capture could not contain Live terminal events are queued while a buffer load runs, and the load discards that queue when it ends. That is right when the loaded buffer is the server's accumulated byte history. The route appends to that history right up to the moment it serializes the response, so a queued event already appears in it and replaying it would duplicate output, most visibly Ink's cursor-up redraws. A tmux pane capture is a photograph, current only as of the instant `capture-pane` ran. Output printed afterwards was queued and then dropped, and nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's next partial redraw then landed on a frame the terminal never received. How much went missing depended on which capture the route served. A `?full=1` load returns the capture alone, with no history in front of it, so it lost everything from the capture to the end of the chunked write. A `?tail=` load returns history, a clear, and then the capture, and the route reads that history after the capture, so it lost everything from the response to the end of that write. The chunked write dominates either way. An agent CLI hides the loss on its next full redraw; a shell session does not, because its output is linear and nothing repaints it. Queue entries now carry their arrival time, and `_finishBufferLoad` takes a `since` cutoff, so a capture load replays exactly the tail that arrived after the response headers. The earlier events stay dropped, because a payload that carries history does hold those. All four paths that fetch a terminal buffer and write it now decide this the same way, through one `_bufferLoadFinishOpts` helper, so they cannot drift apart: `selectSession`, `_onSessionNeedsRefresh`, `_onSessionClearTerminal` and `_maybeRefetchFullHistory`. The second of those is the one that stings. It exists to restore output the client already dropped once under backpressure, and it was dropping more output while performing that recovery. The cache-hit write inside `selectSession` stays on discard deliberately: it runs before the fetch, so its queue holds only events the capture that follows already contains. Two further things had to change for that tail to still exist when the load ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends the load for every non-empty buffer, so the flush policy travels to its own finish calls; the call in `selectSession` runs only when the write was skipped. `_beginBufferLoad` no longer empties the queue when one load re-enters it, which it does on every write, because that reset discarded the whole fetch window before anything could replay it. The response already distinguishes the sources. `source` reads `mux-visible` or `mux-full-history` for a capture and `history` for the byte stream. Follows #395, #396 and #397, which fixed the ways the replayed frame itself could disagree with the terminal. Co-Authored-By: Claude Opus 5 (1M context) --- ...y-output-that-arrived-after-the-capture.md | 36 ++++ config/test-suites.ts | 1 + src/web/public/app.js | 71 ++++++- src/web/public/terminal-ui.js | 57 ++++-- test/capture-load-window.browser.test.ts | 177 ++++++++++++++++++ test/terminal-buffer-flush.test.ts | 97 +++++++++- 6 files changed, 413 insertions(+), 26 deletions(-) create mode 100644 .changeset/fix-replay-output-that-arrived-after-the-capture.md create mode 100644 test/capture-load-window.browser.test.ts diff --git a/.changeset/fix-replay-output-that-arrived-after-the-capture.md b/.changeset/fix-replay-output-that-arrived-after-the-capture.md new file mode 100644 index 000000000..03e091892 --- /dev/null +++ b/.changeset/fix-replay-output-that-arrived-after-the-capture.md @@ -0,0 +1,36 @@ +--- +"aicodeman": patch +--- + +fix(terminal): keep the output a pane capture could not contain + +Live terminal events are queued while a buffer load runs, and the load discards +that queue when it ends. That is right when the loaded buffer is the server's +accumulated byte history: the route appends to that history right up to the +moment it serializes the response, so the queued events already appear in it and +replaying them would duplicate output. + +A tmux pane capture is a photograph, current only as of the instant +`capture-pane` ran. Output printed afterwards was queued and then dropped, with +nothing scheduling a re-fetch, and the CLI's next partial redraw landed on a +frame the terminal never received. A `?full=1` load returns the capture alone, +so it lost everything from the capture to the end of the chunked write. A +`?tail=` load carries the byte history in front of the capture, so it lost +everything from the response to the end of that write. A shell session shows +this most plainly, because its output is linear and nothing repaints it. + +Queue entries now carry their arrival time, and `_finishBufferLoad` takes a +`since` cutoff so a capture load replays exactly the tail that arrived after the +response headers. All four paths that fetch a terminal buffer and write it use +the same rule, through one shared `_bufferLoadFinishOpts` helper: selecting a +session, the backpressure refresh, the clear-terminal reload, and the +full-history re-pull. The backpressure refresh matters most, because it exists +to restore output the client already dropped once and could drop more while +doing it. + +Two things had to change for that tail to still exist when the load ends. +`chunkedTerminalWrite` is what ends the load for any non-empty buffer, so it +takes the flush policy and applies it at its own finish sites. +`_beginBufferLoad` no longer empties the queue when the same load re-enters it, +which it does on every write, because that reset discarded the fetch window +before anything could replay it. diff --git a/config/test-suites.ts b/config/test-suites.ts index 02cc154b2..233e0e7d1 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -27,6 +27,7 @@ export const BROWSER_TEST_GLOBS = [ 'test/webgl-fallback.test.ts', 'test/terminal-copy-shortcut.test.ts', 'test/terminal-keycode229-recovery.browser.test.ts', + 'test/capture-load-window.browser.test.ts', 'test/codex-predictive-echo.test.ts', // also needs a real codex binary ]; diff --git a/src/web/public/app.js b/src/web/public/app.js index f91f748bd..ab303da53 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -1906,6 +1906,30 @@ class CodemanApp { this._onSessionClearTerminal(data); } + /** + * How a buffer load that just fetched `payload` must end. + * + * A tmux pane capture is a point-in-time frame, so nothing that reached the + * browser after the response headers can already be in it. Such a load + * replays exactly that tail; discarding it drops the CLI's output for the + * rest of the load window, and its next partial redraw then lands on a frame + * the terminal never received. A payload built from the server's accumulated + * byte history needs the opposite: that history is current up to the + * response, so replaying the queue on top of it would duplicate output. + * + * `headersReceivedAt` is the caller's own `performance.now()` reading from + * the moment the response arrived, compared only against other client-side + * readings, so there is no clock skew to worry about. + * + * @param {{source?: string}} payload - The parsed `data` of a terminal response. + * @param {number} headersReceivedAt - When that response reached this client. + * @returns {{flushQueued: boolean, since: number}} Options for `_finishBufferLoad`. + */ + _bufferLoadFinishOpts(payload, headersReceivedAt) { + const capturedFromMux = payload?.source === 'mux-visible' || payload?.source === 'mux-full-history'; + return { flushQueued: capturedFromMux, since: headersReceivedAt }; + } + _onSessionTerminal(data) { if (data.id === this.activeSessionId) { if (data.data.length > 32768) _crashDiag.log(`TERMINAL: ${(data.data.length/1024).toFixed(0)}KB`); @@ -1915,7 +1939,7 @@ class CodemanApp { // jump over the cap. Dropped data is recovered from the canonical buffer. const queued = (this.pendingWrites?.reduce((s, w) => s + w.length, 0) || 0) + (this.flickerFilterBuffer?.length || 0) - + (this._loadBufferQueue?.reduce((s, w) => s + w.length, 0) || 0) + + (this._loadBufferQueue?.reduce((s, w) => s + w.data.length, 0) || 0) + (this._terminalWriteInFlightBytes || 0); if (queued + data.data.length > 131072) { // 128KB — drop to prevent accumulation // Schedule a self-recovery once the @@ -2498,9 +2522,11 @@ class CodemanApp { ? `/api/sessions/${sessionId}/terminal?full=1` : `/api/sessions/${sessionId}/terminal?tail=${TERMINAL_TAIL_SIZE}` ); + let headersReceivedAt = performance.now(); let data = (await res.json())?.data ?? {}; if (useFullHistory && data.terminalBuffer && this._replayWouldShrinkBuffer(data.terminalBuffer)) { res = await fetch(`/api/sessions/${sessionId}/terminal?tail=${TERMINAL_TAIL_SIZE}`); + headersReceivedAt = performance.now(); data = (await res.json())?.data ?? {}; } // Bail on a tab switch mid-fetch: writing here would paint this session's @@ -2516,7 +2542,12 @@ class CodemanApp { const linesFromBottom = before ? Math.max(0, (before.baseY || 0) - (before.viewportY || 0)) : 0; this.terminal.clear(); this.terminal.reset(); - await this.chunkedTerminalWrite(data.terminalBuffer); + await this.chunkedTerminalWrite( + data.terminalBuffer, + TERMINAL_CHUNK_SIZE, + undefined, + this._bufferLoadFinishOpts(data, headersReceivedAt) + ); // A tail fetch can be partial, and the banner would otherwise keep // describing the pre-refresh buffer (#258). this._setHistoryTruncation(sessionId, data); @@ -2552,6 +2583,7 @@ class CodemanApp { // Fetch buffer, clear terminal, write buffer, resize (no Ctrl+L needed) try { const res = await fetch(`/api/sessions/${data.id}/terminal`); + const headersReceivedAt = performance.now(); const termData = (await res.json())?.data ?? {}; this.terminal.clear(); @@ -2561,7 +2593,12 @@ class CodemanApp { // (markers don't help here - this is a static buffer reload, not live Ink redraws) const cleanBuffer = termData.terminalBuffer.replace(DEC_SYNC_STRIP_RE, ''); // Use chunked write to avoid UI freeze with large buffers (can be 1-2MB) - await this.chunkedTerminalWrite(cleanBuffer); + await this.chunkedTerminalWrite( + cleanBuffer, + TERMINAL_CHUNK_SIZE, + undefined, + this._bufferLoadFinishOpts(termData, headersReceivedAt) + ); } // Fire-and-forget resize — don't block on it @@ -5782,7 +5819,12 @@ class CodemanApp { parsedAt, bufferLength: parsedBufferLength, completed, - } = await this.chunkedTerminalWrite(buffer, TERMINAL_CHUNK_SIZE, sessionId); + } = await this.chunkedTerminalWrite( + buffer, + TERMINAL_CHUNK_SIZE, + sessionId, + this._bufferLoadFinishOpts(payload, headersReceivedAt) + ); timing.resetAndParseMs = parsedAt - replayStartedAt; if (!completed || this.activeSessionId !== sessionId) return; // Keep shell tab restores bounded too. A user-triggered full-history pull @@ -6241,6 +6283,15 @@ class CodemanApp { } const data = (await res.json())?.data ?? {}; const bodyParsedAt = performance.now(); + // How this load must end, decided here because `chunkedTerminalWrite` is + // what actually ends it for a non-empty buffer. A tmux pane capture is a + // point-in-time frame, so nothing that reached the browser after the + // response headers can already be in it. Replay exactly that tail; + // discarding it drops the CLI's output for the rest of the load window, + // and its next partial redraw then lands on a frame the terminal never + // received. `since` keeps the pre-capture events dropped, because the + // capture does hold those and replaying them would duplicate output. + const finishOpts = this._bufferLoadFinishOpts(data, headersReceivedAt); _crashDiag.log(`FETCH_DONE: ${data.terminalBuffer ? (data.terminalBuffer.length/1024).toFixed(0) + 'KB' : 'empty'} truncated=${data.truncated}`); let freshResetAndParseMs = 0; @@ -6267,7 +6318,8 @@ class CodemanApp { const { parsedAt: freshParsedAt } = await this.chunkedTerminalWrite( data.terminalBuffer, TERMINAL_CHUNK_SIZE, - bufferLoadOwner + bufferLoadOwner, + finishOpts ); freshResetAndParseMs = freshParsedAt - replayStartedAt; if (this._isStaleSelect(selectGen)) { @@ -6317,7 +6369,14 @@ class CodemanApp { // COD-144: when the load painted nothing, FLUSH the queued events instead of // discarding — a new session's prompt arrives only as a queued SSE event. if (this._isLoadingBuffer) { - this._finishBufferLoad(bufferLoadOwner, { flushQueued: bufferWasEmpty }); + // Only reached when the write was skipped. COD-144 lives here: a new + // session's first prompt exists only as a queued event that predates the + // response, so an empty paint replays its queue WHOLE rather than from + // the header timestamp. + this._finishBufferLoad( + bufferLoadOwner, + bufferWasEmpty ? { flushQueued: true, since: 0 } : finishOpts + ); } // Drop the guard so user input clears state normally this._restoringFlushedState = false; diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 85c133506..1a265d611 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -3267,7 +3267,11 @@ Object.assign(CodemanApp.prototype, { // to prevent interleaving historical buffer data with live SSE data. // This is critical: interleaving causes cursor position chaos with Ink redraws. if (this._isLoadingBuffer) { - if (this._loadBufferQueue) this._loadBufferQueue.push(data); + // Each entry records when it arrived. A flush of a tmux-capture load + // replays only what arrived after the capture; without the timestamp it + // would have to replay the whole queue, duplicating the events the + // capture already contains. See _finishBufferLoad's `since`. + if (this._loadBufferQueue) this._loadBufferQueue.push({ at: performance.now(), data }); return; } @@ -3747,9 +3751,14 @@ Object.assign(CodemanApp.prototype, { * and a tick-Worker so progress continues on occluded / idle-throttled tabs. * @param {string} buffer - The full terminal buffer to write * @param {number} chunkSize - Size of each chunk (default 32KB) + * @param {string} [loadOwner] - Load token to finish under + * @param {{ flushQueued?: boolean, since?: number }} [finishOpts] - Passed to + * `_finishBufferLoad`. This method ends the load for every non-empty buffer, + * so a caller that wants the queue replayed has to say so HERE; the call in + * `selectSession` only runs when the write was skipped entirely. * @returns {Promise<{parsedAt: number, bufferLength: number, completed: boolean}>} Parse marker snapshot */ - chunkedTerminalWrite(buffer, chunkSize = TERMINAL_CHUNK_SIZE, loadOwner) { + chunkedTerminalWrite(buffer, chunkSize = TERMINAL_CHUNK_SIZE, loadOwner, finishOpts) { // Generation counter: if a newer chunkedTerminalWrite starts (tab switch), // older writes abort instead of continuing to push stale data into the terminal. const writeGen = ++this._chunkedWriteGen; @@ -3762,7 +3771,7 @@ Object.assign(CodemanApp.prototype, { completed, }); if (!buffer || buffer.length === 0) { - this._finishBufferLoad(bufferLoadOwner); + this._finishBufferLoad(bufferLoadOwner, finishOpts); resolve(parseSnapshot()); return; } @@ -3776,7 +3785,7 @@ Object.assign(CodemanApp.prototype, { this.terminal.write(cleanBuffer, () => resolve(parseSnapshot())); // The write is now ordered in xterm's queue. Release live output before // parsing completes; subsequent writes stay behind it without being lost. - this._finishBufferLoad(bufferLoadOwner); + this._finishBufferLoad(bufferLoadOwner, finishOpts); return; } @@ -3807,7 +3816,7 @@ Object.assign(CodemanApp.prototype, { ); resolve(result); }); - this._finishBufferLoad(bufferLoadOwner); + this._finishBufferLoad(bufferLoadOwner, finishOpts); return; } @@ -3826,10 +3835,20 @@ Object.assign(CodemanApp.prototype, { * Called when chunkedTerminalWrite finishes (or is skipped for empty buffers). * * By default queued SSE events are DISCARDED, not flushed. For an established - * session the loaded buffer from the API is the source of truth up to the - * response timestamp; SSE events queued during the fetch+write overlap already - * appear in that buffer, so flushing them writes duplicate data (especially Ink - * cursor-up redraws), corrupting the terminal display. + * session whose buffer came from the server's accumulated byte history, that + * history is the source of truth up to the response timestamp; SSE events + * queued during the fetch+write overlap already appear in it, so flushing + * them writes duplicate data (especially Ink cursor-up redraws), corrupting + * the terminal display. + * + * A tmux PANE CAPTURE is the exception, and the reason `since` exists. A + * capture is a point-in-time frame taken part-way through the fetch, so it is + * the source of truth only up to CAPTURE time — not up to the response. Every + * event that arrives between the capture and the end of the chunked write is + * queued and, under a plain discard, lost outright: nothing re-fetches, and + * the CLI's next partial redraw lands on a frame the terminal never received. + * The caller passes the response's own arrival time as `since` so exactly + * that tail is replayed and the pre-capture events stay dropped. * * COD-144: a brand-new session is the exception. Its terminal fetch can resolve * BEFORE the PTY emits its first prompt, so the fetched buffer is empty and the @@ -3843,14 +3862,22 @@ Object.assign(CodemanApp.prototype, { * After unblocking, new SSE/WS events deliver subsequent output normally. * * @param {string} [owner] Load token from `_beginBufferLoad`; a stale owner is a no-op. - * @param {{ flushQueued?: boolean }} [opts] When `flushQueued` is true, replay any queued events. + * @param {{ flushQueued?: boolean, since?: number }} [opts] When `flushQueued` + * is true, replay queued events whose arrival timestamp is at or after + * `since` (default 0, meaning the whole queue). */ _beginBufferLoad(owner) { if (this._bufferLoadSeq === undefined) this._bufferLoadSeq = 0; const loadOwner = owner === undefined ? `buffer-${++this._bufferLoadSeq}` : owner; + // `selectSession` opens the load before its fetch, and `chunkedTerminalWrite` + // opens it again under the SAME owner when it starts writing. Resetting the + // queue on that second call would throw away everything that arrived during + // the fetch, which on the capture path is output no buffer holds. Re-entering + // one load keeps its queue; a genuinely new load still starts empty. + const reentering = this._bufferLoadOwner === loadOwner && Array.isArray(this._loadBufferQueue); this._bufferLoadOwner = loadOwner; this._isLoadingBuffer = true; - this._loadBufferQueue = []; + if (!reentering) this._loadBufferQueue = []; return loadOwner; }, @@ -3864,9 +3891,13 @@ Object.assign(CodemanApp.prototype, { this._bufferLoadOwner = null; // COD-144: replay (rather than discard) queued live events when the load // painted nothing — the queued prompt is the only content a new session has. + // A tmux-capture load replays too, but only the tail: `since` cuts the queue + // at the moment the capture stopped being able to contain what arrived. if (opts?.flushQueued && queued && queued.length) { - for (const data of queued) { - this.batchTerminalWrite(data); + const since = typeof opts.since === 'number' ? opts.since : 0; + for (const entry of queued) { + if (entry.at < since) continue; + this.batchTerminalWrite(entry.data); } } return true; diff --git a/test/capture-load-window.browser.test.ts b/test/capture-load-window.browser.test.ts new file mode 100644 index 000000000..9f1ee810c --- /dev/null +++ b/test/capture-load-window.browser.test.ts @@ -0,0 +1,177 @@ +/** + * @fileoverview Output arriving after a pane capture survives the buffer load. + * + * `batchTerminalWrite` queues live terminal events while a buffer load runs, + * and `_finishBufferLoad` discards that queue by default. That is right when + * the loaded buffer is the server's accumulated byte history, which is current + * up to the response. A tmux pane capture is current only up to CAPTURE time, + * so anything arriving between the capture and the end of the chunked write is + * queued and then dropped, with nothing scheduling a re-fetch. + * + * The queue now stamps each entry with its arrival time, and a capture load + * replays the tail that arrived after the response headers. These drive the + * real client in chromium: the event is injected from inside the response's + * own `json()` call, which is the one place guaranteed to land after the + * headers and before the chunked write. + * + * Port: 3256 (capture load window) + * + * Run: npx vitest run --config config/vitest.browser.config.ts test/capture-load-window.browser.test.ts + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { chromium, type Browser, type BrowserContext, type Page } from 'playwright'; +import { WebServer } from '../src/web/server.js'; + +const PORT = 3256; +const BASE_URL = `http://localhost:${PORT}`; +const MARKER = 'ARRIVED-AFTER-THE-CAPTURE'; + +let server: WebServer; +let browser: Browser; + +beforeAll(async () => { + server = new WebServer(PORT, false, true); // testMode + await server.start(); + browser = await chromium.launch({ headless: true }); +}, 60_000); + +afterAll(async () => { + await browser?.close(); + await server?.stop(); +}, 30_000); + +/** + * Select the session with the terminal fetch stubbed, injecting one live event + * from inside `json()`. Returns how many terminal rows carry the marker, so a + * flush that replays too much fails as loudly as one that replays nothing. + */ +async function runLoad(page: Page, sessionId: string, source: string): Promise { + return page.evaluate( + async ({ sid, src, marker }) => { + const app = ( + window as unknown as { + app: { + selectSession: (id: string, o?: object) => Promise; + _onSessionTerminal: (e: { id: string; data: string }) => void; + terminal: { + buffer: { + active: { + length: number; + getLine: (i: number) => { translateToString: (t: boolean) => string } | undefined; + }; + }; + }; + }; + } + ).app; + + const realFetch = window.fetch.bind(window); + window.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + const url = String(typeof input === 'string' ? input : ((input as Request).url ?? input)); + if (!url.includes('/terminal')) return realFetch(input as RequestInfo, init); + return Promise.resolve({ + ok: true, + status: 200, + // `selectSession` timestamps the headers the moment this promise + // resolves, then calls json(). Injecting here puts the event after + // that timestamp and inside the load window, which is exactly the + // gap a pane capture cannot cover. + json: async () => { + app._onSessionTerminal({ id: sid, data: `\r\n${marker}\r\n` }); + return { + success: true, + data: { + terminalBuffer: '\x1b[1;1Hcaptured frame line one\r\n', + status: 'idle', + fullSize: 512, + retainedBytes: 512, + truncated: false, + truncationReason: null, + source: src, + captureCols: 80, + captureRows: 24, + }, + }; + }, + }) as unknown as Promise; + }) as typeof window.fetch; + + try { + await app.selectSession(sid); + await new Promise((r) => setTimeout(r, 1200)); + const buf = app.terminal.buffer.active; + let hits = 0; + for (let i = 0; i < buf.length; i++) { + if (buf.getLine(i)?.translateToString(true).includes(marker)) hits += 1; + } + return hits; + } finally { + window.fetch = realFetch; + } + }, + { sid: sessionId, src: source, marker: MARKER } + ); +} + +async function openSession(page: Page): Promise { + await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction(() => document.body.classList.contains('app-loaded'), { timeout: 10_000 }); + // xterm loads from /vendor, so the terminal appears a beat after the app. + // Without it every buffer assertion below would throw rather than compare. + await page.waitForFunction(() => (window as unknown as { app?: { terminal?: unknown } }).app?.terminal, null, { + timeout: 30_000, + }); + return page.evaluate(async () => { + const res = await fetch('/api/sessions', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ workingDir: '/tmp', name: 'capture-load-window-test' }), + }); + const body = await res.json(); + return body.data?.session?.id ?? body.data?.id ?? body.id; + }); +} + +describe('output emitted during a capture load', () => { + let context: BrowserContext; + let page: Page; + + afterAll(async () => { + await context?.close(); + }); + + it('reaches the terminal exactly once when the buffer came from a pane capture', async () => { + context = await browser.newContext({ viewport: { width: 1280, height: 800 } }); + page = await context.newPage(); + const sessionId = await openSession(page); + expect(sessionId).toBeTruthy(); + + // Exactly once. The cutoff exists so the flush cannot also replay events the + // payload already carried, which would double the output rather than heal it. + expect(await runLoad(page, sessionId, 'mux-visible')).toBe(1); + + await page.evaluate( + (sid: string) => fetch(`/api/sessions/${sid}`, { method: 'DELETE' }).then(() => undefined), + sessionId + ); + await context.close(); + }, 60_000); + + it('stays dropped when the buffer came from the accumulated byte history', async () => { + // The byte history already contains everything up to the response, so + // replaying the queue on top of it would duplicate the output — most + // visibly Ink's cursor-up redraws. The discard has to survive this fix. + context = await browser.newContext({ viewport: { width: 1280, height: 800 } }); + page = await context.newPage(); + const sessionId = await openSession(page); + + expect(await runLoad(page, sessionId, 'history')).toBe(0); + + await page.evaluate( + (sid: string) => fetch(`/api/sessions/${sid}`, { method: 'DELETE' }).then(() => undefined), + sessionId + ); + await context.close(); + }, 60_000); +}); diff --git a/test/terminal-buffer-flush.test.ts b/test/terminal-buffer-flush.test.ts index 54267bebf..0500a3e6a 100644 --- a/test/terminal-buffer-flush.test.ts +++ b/test/terminal-buffer-flush.test.ts @@ -56,10 +56,10 @@ type BufferLoadApp = { _bufferLoadSeq: number; _bufferLoadOwner: string | null; _isLoadingBuffer: boolean; - _loadBufferQueue: string[] | null; + _loadBufferQueue: { at: number; data: string }[] | null; batchTerminalWrite: (data: string) => void; _beginBufferLoad: (owner?: string) => string; - _finishBufferLoad: (owner?: string, opts?: { flushQueued?: boolean }) => boolean; + _finishBufferLoad: (owner?: string, opts?: { flushQueued?: boolean; since?: number }) => boolean; }; /** @@ -84,10 +84,13 @@ function makeApp() { return { app, writes }; } -/** Simulate live SSE events arriving while a buffer load is in progress (the queue path). */ -function pushWhileLoading(app: BufferLoadApp, data: string) { - // Mirrors batchTerminalWrite's queue branch: if loading, push to the queue. - if (app._isLoadingBuffer && app._loadBufferQueue) app._loadBufferQueue.push(data); +/** + * Simulate a live SSE event arriving while a buffer load is in progress. + * Mirrors batchTerminalWrite's queue branch, which stamps each entry with its + * arrival time so a flush can replay only the tail (see the `since` tests). + */ +function pushWhileLoading(app: BufferLoadApp, data: string, at = performance.now()) { + if (app._isLoadingBuffer && app._loadBufferQueue) app._loadBufferQueue.push({ at, data }); } describe('buffer-load flush (COD-144)', () => { @@ -153,11 +156,91 @@ describe('buffer-load flush (COD-144)', () => { // State untouched — still loading, queue intact, nothing replayed. expect(app._isLoadingBuffer).toBe(true); expect(app._bufferLoadOwner).toBe('real-owner'); - expect(app._loadBufferQueue).toEqual(['queued']); + expect(app._loadBufferQueue).toEqual([{ at: expect.any(Number), data: 'queued' }]); expect(app.batchTerminalWrite).not.toHaveBeenCalled(); expect(writes).toEqual([]); }); + // ── The tmux-capture tail: `since` ── + // + // A pane capture is a point-in-time frame taken part-way through the fetch, so + // it holds what arrived BEFORE the capture and nothing after. selectSession + // passes the response's arrival time as `since`, which splits the queue at + // exactly that line: pre-capture events are already painted and must stay + // dropped, post-capture events exist nowhere else and must be replayed. + + it('flushes only the entries at or after `since`', () => { + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-since'); + pushWhileLoading(app, 'already-in-the-capture', 100); + pushWhileLoading(app, 'arrived-at-the-headers', 200); + pushWhileLoading(app, 'arrived-after-the-headers', 300); + + app._finishBufferLoad(owner, { flushQueued: true, since: 200 }); + + // The pre-capture event stays dropped; the boundary entry counts as after. + expect(writes).toEqual(['arrived-at-the-headers', 'arrived-after-the-headers']); + }); + + it('flushQueued without `since` still replays the whole queue', () => { + // The COD-144 path: a brand-new session's first prompt predates the + // response, so cutting the queue would drop the only content it has. + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-no-since'); + pushWhileLoading(app, 'prompt', 10); + pushWhileLoading(app, 'more', 20); + + app._finishBufferLoad(owner, { flushQueued: true }); + + expect(writes).toEqual(['prompt', 'more']); + }); + + it('a `since` past every entry flushes nothing', () => { + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-since-late'); + pushWhileLoading(app, 'old', 10); + + app._finishBufferLoad(owner, { flushQueued: true, since: 999 }); + + expect(writes).toEqual([]); + expect(app.batchTerminalWrite).not.toHaveBeenCalled(); + }); + + // ── Re-entering one load ── + // + // `selectSession` opens the load before its fetch, and `chunkedTerminalWrite` + // opens it again under the SAME owner when it starts writing. A reset on that + // second call would silently throw away everything queued during the fetch, + // which on the capture path is output no buffer holds. + + it('re-entering the same load keeps what the queue already holds', () => { + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-reenter'); + pushWhileLoading(app, 'arrived-during-the-fetch', 100); + + // chunkedTerminalWrite re-opens the load it was handed. + app._beginBufferLoad(owner); + pushWhileLoading(app, 'arrived-during-the-write', 200); + + app._finishBufferLoad(owner, { flushQueued: true, since: 50 }); + + expect(writes).toEqual(['arrived-during-the-fetch', 'arrived-during-the-write']); + }); + + it('a genuinely different load still starts with an empty queue', () => { + const { app, writes } = makeApp(); + app._beginBufferLoad('load-first'); + pushWhileLoading(app, 'belongs-to-the-abandoned-load', 100); + + // A tab switch starts a new load under a new owner. Its events are not ours. + const second = app._beginBufferLoad('load-second'); + pushWhileLoading(app, 'belongs-to-this-load', 200); + + app._finishBufferLoad(second, { flushQueued: true, since: 0 }); + + expect(writes).toEqual(['belongs-to-this-load']); + }); + it('empty queue + flushQueued is a no-op (no throw, no writes)', () => { const { app, writes } = makeApp(); const owner = app._beginBufferLoad('load-empty');