From 75a028e825d8e38dc55bf42c4b618c92566df114 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Fri, 18 Sep 2026 16:43:04 +0200 Subject: [PATCH] fix(terminal): re-take the sticky-scroll baseline after a replay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A capture load now replays its queued tail, and that replay runs through `batchTerminalWrite`, which samples `_wasAtBottomBeforeWrite` before it queues. It runs inside `chunkedTerminalWrite`, before that promise resolves, with the terminal freshly reset and rewritten — so the sample is always true. The caller then restored the reader's position and the next `flushPendingWrites` scrolled straight back to the bottom off the latched flag, undoing it. The only thing in the way was `_hasRecentUserScrollUp()`, a 1500ms window a server-triggered refresh is usually past. `_syncStickyScrollBaseline()` re-takes the flag from wherever the viewport now sits, and the two paths that restore a position call it right after doing so: `_onSessionNeedsRefresh` and `_maybeRefetchFullHistory`. Those are the paths #259 and #205 exist for, and they are also where a non-empty queue is most likely, since a needsRefresh fires when output is flooding. Re-taking rather than suppressing the sampling: suppressing leaves whatever stale value the flag held from before the load, which on the full-history re-pull has no reason to be false. `selectSession` and `_onSessionClearTerminal` deliberately end at the bottom, so the sampled true is already the truth there and they do not call it. `_bufferLoadFinishOpts` gains the coverage the CI gate can see: both mux sources flush, `history` does not, and a payload naming no source does not. Its only coverage was the browser suite, which CI does not run. The JSDoc and the changeset now record the one duplicate window this cutoff cannot close. The server appends output to the byte buffer in the same tick it emits, but broadcasts on a batch timer — 8ms over WebSocket, 16 to 50ms over SSE — so a batch pending when `capture-pane` ran leaves the server after the reply and is replayed although the capture holds it. It is one batch interval wide against a recovery window spanning the whole chunked write, and closing it means flushing that batch server side before the capture. The second browser test asserts its session was created, so a failed create fails it instead of passing with zero hits. docs/architecture-invariants.md no longer claims the replay leaves the queued-event discard window alone. That clause now describes what decides how a load ends, the baseline rule, the batch window, and the three covering tests. Co-Authored-By: Claude Opus 5 (1M context) --- ...y-output-that-arrived-after-the-capture.md | 15 +++ docs/architecture-invariants.md | 2 +- src/web/public/app.js | 20 ++++ src/web/public/terminal-ui.js | 21 ++++ test/capture-load-window.browser.test.ts | 3 + test/terminal-buffer-flush.test.ts | 98 +++++++++++++++++++ test/terminal-flush-budget.test.ts | 45 +++++++++ 7 files changed, 203 insertions(+), 1 deletion(-) diff --git a/.changeset/fix-replay-output-that-arrived-after-the-capture.md b/.changeset/fix-replay-output-that-arrived-after-the-capture.md index 03e09189..0cb24af0 100644 --- a/.changeset/fix-replay-output-that-arrived-after-the-capture.md +++ b/.changeset/fix-replay-output-that-arrived-after-the-capture.md @@ -34,3 +34,18 @@ 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. + +A path that replays its queue and then restores a scroll position re-takes the +sticky-scroll baseline (`_syncStickyScrollBaseline`). The replay runs with the +terminal freshly reset, so it reads as sitting at the bottom, and the next flush +would scroll there and undo the restore. The backpressure refresh and the +full-history re-pull are the two paths that restore a position, and both are +ones a reader reaches while scrolled up. + +One duplicate window stays open and is not closable from the browser. The server +appends output to the byte buffer in the same tick it emits, but broadcasts on a +batch timer, 8ms over WebSocket and 16 to 50ms over SSE. A batch already pending +when `capture-pane` ran therefore leaves the server after the reply and is +replayed although the capture holds it. It is one batch interval wide, against a +recovery window that spans the whole chunked write, and closing it means +flushing that session's pending batch before taking the capture. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 20f6bda9..3a0e8733 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -116,7 +116,7 @@ Tests: `test/docker-hosts.test.ts`, `test/docker-exec-options.test.ts`, `test/do ### Full-scrollback replay -**Full-scrollback replay** (COD-164/#148, reworked for #205): `GET /api/sessions/:id/terminal?full=1` returns the ENTIRE tmux scrollback (capture-pane `-e -S -` bounded by the configured history limit, explicit `maxBuffer` from the terminal-history config, early byte-cap before normalization, CRLF-normalized for shell panes). On success the capture is returned ALONE (`source='mux-full-history'` — it supersedes the byte buffer; no duplication). The first load of each non-shell TUI session per page requests `full=1` (`_fullHistoryLoaded` Set in app.js — the old one-shot `_initialFullBufferLoad` flag was consumed by whichever tab auto-selected, leaving every other TUI tab one frame of history). Shell sessions instead load a bounded 1 MiB `?tail=` window on every selection and automatic drop recovery: a 100k-line shell capture can be tens of MiB, and automatically parsing it makes tab-switch latency scale with the entire session. Shell full history is explicit-button-only; reaching the top during an ordinary wheel/touch gesture must not reset xterm and replay the multi-megabyte capture on its main thread. Other modes may still re-pull `full=1` at the TOP, and pressing **Load full history** forces the request for any recoverably truncated session (`_maybeRefetchFullHistory`, 4s per-session gesture cooldown, in-flight + tab-switch guards, viewport position held across the replay); Shell full pulls are not retained in the tab cache, so the next switch stays bounded. Chunked replay enqueues 32 KiB pieces across safe yields, appends an xterm parse marker, then releases the live-output gate; output arriving after that release stays ordered behind the snapshot, while the marker callback supplies accurate parse timing without extending the pre-existing queued-event discard window. Live output is separately one-chunk-in-flight: xterm's callback releases each 32/64 KiB write before the next is submitted, keeping the remainder in the app queue where the 128 KiB cap can observe it instead of hiding an unbounded backlog in xterm's private WriteBuffer. While WebSocket owns terminal I/O, parallel SSE terminal/output-recovery events are discarded before JSON parsing; fallback recovery is single-flight per active session so backpressure cannot start overlapping reset+replay cycles. The route exposes capture/prepare totals in `Server-Timing`, while `[TERMINAL-PERF]` separates TTFB, body/JSON, reset+parse and total time for both selection and on-demand full pulls; parse completion is not a browser compositor/GPU paint measurement. The re-pull exists because xterm's buffer is only a WINDOW onto tmux's history and two things shrink it: tmux coalesces bursty output into pane REPAINTS that overwrite rows instead of emitting linefeeds (measured: a 60-line burst added 1 row of browser scrollback and destroyed 34), and a tab switch replays only the visible frame. tmux's own history is intact throughout — the browser just has to ask for it again. On-demand rather than automatic because at a 100k history limit the capture can be megabytes. ⚠️ **The capture ENDS with a cursor move back to the pane's own caret position** (`formatCursorRestore`, from the same `display-message` query the visible-frame path uses). The linear replay otherwise leaves the caret wherever the last character landed — the bottom-most row carrying text, which for an agent CLI is the status line — so the caret sat on the composer's border instead of its input line and every cursor-relative update the CLI sent afterwards was measured from the wrong row, until its next full redraw silently repaired it (that self-repair is why the report read as "it fixes itself as soon as Claude writes a line"). ⚠️ **The move is RELATIVE — up `rows - 1 - cursor_y`, then `\r`, then right `cursor_x` — never `CUP`.** `\x1b[;H` numbers rows from the top of the browser's screen, so it lands correctly only while the browser's row count equals `pane_height`, and nothing guarantees that: `resizeWindow` issues its tmux resize fire-and-forget and returns immediately, so a capture can be taken before a requested resize has applied, and `_onSessionNeedsRefresh` sends no resize at all. Counting up from the last replayed row anchors to the content both ends share. Restoring the cursor makes ROW ALIGNMENT load-bearing on this path: **no transform that can DELETE A LINE may run over a full-history capture**, because every deletion shifts the frame out from under the restored position. Four had accumulated — trailing blank rows stripped by `\n+$`, `stripInkRedrawBloat`, the `CLAUDE_BANNER_PATTERN` trim that cuts everything above the banner, and `LEADING_WHITESPACE_PATTERN` — each correct for a byte stream of successive frames and each wrong for a single rendered frame. ⚠️ **Those skips key on `isFullCapture`, meaning a capture actually came back — never on `?full=1` alone.** When `captureActivePaneBuffer` returns null (ENOBUFS, a timeout, a vanished pane, or a session with no mux at all) the reply falls back to `session.terminalBuffer`, which IS a byte stream and must still be stripped; gating on the query flag returned it whole, and a direct-PTY session takes that path on every first selection rather than only during an outage. ⚠️ A capture holding nothing visible (`hasVisibleContent`) returns `''`, because the caller reads an empty capture as "unavailable" and keeps its byte history — retaining trailing blank rows made an all-blank pane non-empty, which would have replaced real history with a blank screen from the server side, where `_replayWouldShrinkBuffer` cannot see it. ⚠️ **"One line per screen row" holds only where no row was hard-wrapped**: `-J` joins a wrapped row into its logical line (measured: a 100-character line in a 40-column pane captures as 10 lines against a 12-row pane), and the counts reconcile only once the browser xterm re-wraps at the same width — the same assumption `_estimateReplayRows` already documents. Tests: `test/tmux-capture-full-history.test.ts` covers the cursor move, the trim pairing and `hasVisibleContent`; `test/routes/session-routes.test.ts` covers a surviving blank first row, an unstripped byte-history fallback, and an empty capture leaving history intact. ⚠️ **The re-pull must never DOWNGRADE the buffer** (#205 round 2): the same reasoning that makes it a win for a shell pane makes it destructive for a repaint-mode CLI pane, where tmux keeps no history of its own (`history_size≈0` measured for a Claude pane) and the capture is roughly ONE frame while xterm may hold hundreds of rows of replayed frames — `_resetTerminalForReplay()` + rewrite then deletes history mid-scroll ("goes back a bit, repeats blocks, gets worse the further up I go"; measured A/B on a live pane: 341 rows → 42 with the guard off). `_replayWouldShrinkBuffer()` (terminal-ui.js) estimates the capture's rendered rows — escape sequences stripped, `capture-pane -J` re-wrapping accounted for — and the pull is skipped when that is more than one screen short of `buffer.active.length`. The one-screen tolerance matters: both sides are estimates (the buffer length counts trailing blank rows), so only a clear downgrade is refused. A refused session joins `_fullHistoryRepullUseless`, raising its cooldown from 4s to 60s so a hollow pane stops re-fetching megabytes on every scroll-up. Tests: `test/tmux-capture-full-history.test.ts`, `test/tmux-scrollback-eol.test.ts`, `test/terminal-scroll-routing.test.ts`, `test/terminal-flush-budget.test.ts`. +**Full-scrollback replay** (COD-164/#148, reworked for #205): `GET /api/sessions/:id/terminal?full=1` returns the ENTIRE tmux scrollback (capture-pane `-e -S -` bounded by the configured history limit, explicit `maxBuffer` from the terminal-history config, early byte-cap before normalization, CRLF-normalized for shell panes). On success the capture is returned ALONE (`source='mux-full-history'` — it supersedes the byte buffer; no duplication). The first load of each non-shell TUI session per page requests `full=1` (`_fullHistoryLoaded` Set in app.js — the old one-shot `_initialFullBufferLoad` flag was consumed by whichever tab auto-selected, leaving every other TUI tab one frame of history). Shell sessions instead load a bounded 1 MiB `?tail=` window on every selection and automatic drop recovery: a 100k-line shell capture can be tens of MiB, and automatically parsing it makes tab-switch latency scale with the entire session. Shell full history is explicit-button-only; reaching the top during an ordinary wheel/touch gesture must not reset xterm and replay the multi-megabyte capture on its main thread. Other modes may still re-pull `full=1` at the TOP, and pressing **Load full history** forces the request for any recoverably truncated session (`_maybeRefetchFullHistory`, 4s per-session gesture cooldown, in-flight + tab-switch guards, viewport position held across the replay); Shell full pulls are not retained in the tab cache, so the next switch stays bounded. Chunked replay enqueues 32 KiB pieces across safe yields, appends an xterm parse marker, then releases the live-output gate; output arriving after that release stays ordered behind the snapshot, and the marker callback supplies accurate parse timing. ⚠️ **How the load ENDS depends on where the payload came from**, and `_bufferLoadFinishOpts` (app.js) is the one place that decides it for all four fetch-and-write paths. A payload built from the server's accumulated byte history is current up to the response, so the events queued during the load already appear in it and stay DISCARDED; replaying them would duplicate output, most visibly Ink's cursor-up redraws. A pane capture (`mux-visible` or `mux-full-history`) is current only up to CAPTURE time, so `_finishBufferLoad` replays the queue from the response's own arrival timestamp (`since`) and the pre-capture events stay dropped. ⚠️ **A path that then restores a scroll position must re-take the sticky-scroll baseline** (`_syncStickyScrollBaseline`): the replay runs inside `chunkedTerminalWrite` before its promise resolves, with the terminal freshly reset, so `batchTerminalWrite` samples `_wasAtBottomBeforeWrite` as true and the next `flushPendingWrites` would scroll to the bottom over the restore. ⚠️ The cutoff is a client-side timestamp and the server broadcasts on a batch timer (8ms WebSocket, 16-50ms SSE), so a batch pending when the capture ran arrives after the response and replays although the capture holds it — bounded by one batch interval, and closable only server side by flushing that batch before the capture. Tests for the three: `test/terminal-flush-budget.test.ts` pins which sources flush, `test/terminal-buffer-flush.test.ts` pins the `since` cutoff and the baseline re-take, and `test/capture-load-window.browser.test.ts` drives both against a live server. Live output is separately one-chunk-in-flight: xterm's callback releases each 32/64 KiB write before the next is submitted, keeping the remainder in the app queue where the 128 KiB cap can observe it instead of hiding an unbounded backlog in xterm's private WriteBuffer. While WebSocket owns terminal I/O, parallel SSE terminal/output-recovery events are discarded before JSON parsing; fallback recovery is single-flight per active session so backpressure cannot start overlapping reset+replay cycles. The route exposes capture/prepare totals in `Server-Timing`, while `[TERMINAL-PERF]` separates TTFB, body/JSON, reset+parse and total time for both selection and on-demand full pulls; parse completion is not a browser compositor/GPU paint measurement. The re-pull exists because xterm's buffer is only a WINDOW onto tmux's history and two things shrink it: tmux coalesces bursty output into pane REPAINTS that overwrite rows instead of emitting linefeeds (measured: a 60-line burst added 1 row of browser scrollback and destroyed 34), and a tab switch replays only the visible frame. tmux's own history is intact throughout — the browser just has to ask for it again. On-demand rather than automatic because at a 100k history limit the capture can be megabytes. ⚠️ **The capture ENDS with a cursor move back to the pane's own caret position** (`formatCursorRestore`, from the same `display-message` query the visible-frame path uses). The linear replay otherwise leaves the caret wherever the last character landed — the bottom-most row carrying text, which for an agent CLI is the status line — so the caret sat on the composer's border instead of its input line and every cursor-relative update the CLI sent afterwards was measured from the wrong row, until its next full redraw silently repaired it (that self-repair is why the report read as "it fixes itself as soon as Claude writes a line"). ⚠️ **The move is RELATIVE — up `rows - 1 - cursor_y`, then `\r`, then right `cursor_x` — never `CUP`.** `\x1b[;H` numbers rows from the top of the browser's screen, so it lands correctly only while the browser's row count equals `pane_height`, and nothing guarantees that: `resizeWindow` issues its tmux resize fire-and-forget and returns immediately, so a capture can be taken before a requested resize has applied, and `_onSessionNeedsRefresh` sends no resize at all. Counting up from the last replayed row anchors to the content both ends share. Restoring the cursor makes ROW ALIGNMENT load-bearing on this path: **no transform that can DELETE A LINE may run over a full-history capture**, because every deletion shifts the frame out from under the restored position. Four had accumulated — trailing blank rows stripped by `\n+$`, `stripInkRedrawBloat`, the `CLAUDE_BANNER_PATTERN` trim that cuts everything above the banner, and `LEADING_WHITESPACE_PATTERN` — each correct for a byte stream of successive frames and each wrong for a single rendered frame. ⚠️ **Those skips key on `isFullCapture`, meaning a capture actually came back — never on `?full=1` alone.** When `captureActivePaneBuffer` returns null (ENOBUFS, a timeout, a vanished pane, or a session with no mux at all) the reply falls back to `session.terminalBuffer`, which IS a byte stream and must still be stripped; gating on the query flag returned it whole, and a direct-PTY session takes that path on every first selection rather than only during an outage. ⚠️ A capture holding nothing visible (`hasVisibleContent`) returns `''`, because the caller reads an empty capture as "unavailable" and keeps its byte history — retaining trailing blank rows made an all-blank pane non-empty, which would have replaced real history with a blank screen from the server side, where `_replayWouldShrinkBuffer` cannot see it. ⚠️ **"One line per screen row" holds only where no row was hard-wrapped**: `-J` joins a wrapped row into its logical line (measured: a 100-character line in a 40-column pane captures as 10 lines against a 12-row pane), and the counts reconcile only once the browser xterm re-wraps at the same width — the same assumption `_estimateReplayRows` already documents. Tests: `test/tmux-capture-full-history.test.ts` covers the cursor move, the trim pairing and `hasVisibleContent`; `test/routes/session-routes.test.ts` covers a surviving blank first row, an unstripped byte-history fallback, and an empty capture leaving history intact. ⚠️ **The re-pull must never DOWNGRADE the buffer** (#205 round 2): the same reasoning that makes it a win for a shell pane makes it destructive for a repaint-mode CLI pane, where tmux keeps no history of its own (`history_size≈0` measured for a Claude pane) and the capture is roughly ONE frame while xterm may hold hundreds of rows of replayed frames — `_resetTerminalForReplay()` + rewrite then deletes history mid-scroll ("goes back a bit, repeats blocks, gets worse the further up I go"; measured A/B on a live pane: 341 rows → 42 with the guard off). `_replayWouldShrinkBuffer()` (terminal-ui.js) estimates the capture's rendered rows — escape sequences stripped, `capture-pane -J` re-wrapping accounted for — and the pull is skipped when that is more than one screen short of `buffer.active.length`. The one-screen tolerance matters: both sides are estimates (the buffer length counts trailing blank rows), so only a clear downgrade is refused. A refused session joins `_fullHistoryRepullUseless`, raising its cooldown from 4s to 60s so a hollow pane stops re-fetching megabytes on every scroll-up. Tests: `test/tmux-capture-full-history.test.ts`, `test/tmux-scrollback-eol.test.ts`, `test/terminal-scroll-routing.test.ts`, `test/terminal-flush-budget.test.ts`. ### Terminal scrollback: strip flavors and wheel/touch forwarding diff --git a/src/web/public/app.js b/src/web/public/app.js index ab303da5..e96e78b7 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -1921,6 +1921,16 @@ class CodemanApp { * the moment the response arrived, compared only against other client-side * readings, so there is no clock skew to worry about. * + * What this cutoff does NOT cover: the server appends output to the byte + * buffer and emits it in the same tick, but it BROADCASTS on a batch timer — + * 8ms over WebSocket, 16 to 50ms over SSE. The terminal route runs + * synchronously from `capture-pane` to its return, so a batch that was + * already pending when the capture ran leaves the server after the reply, + * arrives after `headersReceivedAt`, and is replayed although the capture + * holds it. The duplicate is one batch interval wide, against a recovery + * window that spans the whole chunked write. Closing it belongs on the + * server: flush that session's pending batch before taking the capture. + * * @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`. @@ -2557,6 +2567,10 @@ class CodemanApp { }); if (target === null || typeof this.terminal.scrollToLine !== 'function') this.terminal.scrollToBottom(); else this.terminal.scrollToLine(target); + // The load's own replay sampled the sticky-scroll baseline while the + // terminal sat at the bottom of a just-rewritten buffer, so the next + // flush would scroll back down and undo the restore above. + this._syncStickyScrollBaseline(); // Re-position local echo overlay at new prompt location this._localEchoOverlay?.rerender(); // Resize PTY to match actual browser dimensions (critical for OpenCode @@ -5842,6 +5856,12 @@ class CodemanApp { const delta = parsedBufferLength - rowsBefore; if (delta > 0) this.terminal.scrollToLine(delta); else this.terminal.scrollToTop(); + // The load's own replay sampled the sticky-scroll baseline while the + // terminal sat at the bottom of a just-rewritten buffer, so the next + // flush would scroll back down and undo the restore above. This path is + // reached only from a scroll-up gesture, so being dragged down is the + // exact opposite of what the user asked for. + this._syncStickyScrollBaseline(); timing.totalMs = performance.now() - requestStartedAt; this._recordTerminalLoadTiming(timing); } catch { diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 1a265d61..b56f5510 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -3131,6 +3131,27 @@ Object.assign(CodemanApp.prototype, { return buffer.viewportY >= buffer.baseY - 2; }, + /** + * Re-take the sticky-scroll baseline from where the viewport now sits. + * + * `batchTerminalWrite` samples `_wasAtBottomBeforeWrite` before it queues + * data, and `flushPendingWrites` scrolls to the bottom off that sample. A + * buffer load that replays its queue samples at the worst possible moment: + * `_finishBufferLoad` runs inside `chunkedTerminalWrite`, before its promise + * resolves, with the terminal freshly reset and rewritten, so the sample is + * always true. A caller that then restores the reader's position would have + * that restore undone by the next flush. + * + * Every caller that scrolls the viewport somewhere other than the bottom + * after a load must call this, so the baseline describes the position the + * caller chose. `selectSession` and `_onSessionClearTerminal` deliberately + * end at the bottom, so for them the sampled true is already the truth and + * they do not call it. + */ + _syncStickyScrollBaseline() { + this._wasAtBottomBeforeWrite = this.isTerminalAtBottom(); + }, + // Record manual scroll gestures so sticky-scroll can give an upward scroll a // short grace window (see _hasRecentUserScrollUp). A downward scroll that // lands back at the bottom clears the suppression immediately. diff --git a/test/capture-load-window.browser.test.ts b/test/capture-load-window.browser.test.ts index 9f1ee810..8cea77ef 100644 --- a/test/capture-load-window.browser.test.ts +++ b/test/capture-load-window.browser.test.ts @@ -165,6 +165,9 @@ describe('output emitted during a capture load', () => { context = await browser.newContext({ viewport: { width: 1280, height: 800 } }); page = await context.newPage(); const sessionId = await openSession(page); + // Without this, a failed create passes the zero-hit assertion below + // vacuously — nothing was loaded, so nothing was replayed. + expect(sessionId).toBeTruthy(); expect(await runLoad(page, sessionId, 'history')).toBe(0); diff --git a/test/terminal-buffer-flush.test.ts b/test/terminal-buffer-flush.test.ts index 0500a3e6..36f652ef 100644 --- a/test/terminal-buffer-flush.test.ts +++ b/test/terminal-buffer-flush.test.ts @@ -84,6 +84,50 @@ function makeApp() { return { app, writes }; } +/** + * A stub carrying the REAL `batchTerminalWrite` on top of the real begin/finish + * methods, so a replay samples the sticky-scroll baseline exactly as it does in + * the browser. The terminal is a fake whose `buffer.active` the test moves by + * hand, which is what a caller's `scrollToLine` does to a real one. + */ +function makeScrollApp() { + const buffer = { viewportY: 0, baseY: 100 }; + const app = { + buffer, + terminal: { buffer: { active: buffer } }, + sessions: new Map(), + activeSessionId: null, + pendingWrites: [] as string[], + writeFrameScheduled: false, + _wasAtBottomBeforeWrite: false, + _bufferLoadSeq: 0, + _bufferLoadOwner: null as string | null, + _isLoadingBuffer: false, + _loadBufferQueue: null as { at: number; data: string }[] | null, + _scheduleTerminalWriteFlush: vi.fn(), + batchTerminalWrite: mixin.batchTerminalWrite as (data: string) => void, + isTerminalAtBottom: mixin.isTerminalAtBottom as () => boolean, + _syncStickyScrollBaseline: mixin._syncStickyScrollBaseline as () => void, + _beginBufferLoad: mixin._beginBufferLoad as BufferLoadApp['_beginBufferLoad'], + _finishBufferLoad: mixin._finishBufferLoad as BufferLoadApp['_finishBufferLoad'], + }; + return app; +} + +/** + * Slice one class method out of app.js, from its header to the next method's. + * + * Bounding the slice matters: the two methods checked below are not followed by + * a JSDoc block, so a scan for the next comment would run on into unrelated + * code and match its scroll calls instead of theirs. + */ +function methodBody(source: string, method: string): string { + const start = source.search(new RegExp(`^ {2}(?:async )?${method}\\(`, 'm')); + expect(start, `${method} not found in app.js`).toBeGreaterThan(-1); + const next = /^ {2}(?:async )?[A-Za-z_$][\w$]*\(/m.exec(source.slice(start + 1)); + return next ? source.slice(start, start + 1 + next.index) : source.slice(start); +} + /** * Simulate a live SSE event arriving while a buffer load is in progress. * Mirrors batchTerminalWrite's queue branch, which stamps each entry with its @@ -241,6 +285,60 @@ describe('buffer-load flush (COD-144)', () => { expect(writes).toEqual(['belongs-to-this-load']); }); + // ── The sticky-scroll baseline across a replay ── + // + // `batchTerminalWrite` samples `_wasAtBottomBeforeWrite` before queueing, and + // `flushPendingWrites` scrolls to the bottom off that sample. The replay runs + // inside `chunkedTerminalWrite` before its promise resolves, with the terminal + // freshly reset and rewritten, so the sample is always true. A caller that + // then restores the reader's position would have that restore undone. + + it('the replay latches the baseline true, and the viewport restore re-takes it', () => { + const app = makeScrollApp(); + const owner = app._beginBufferLoad('load-scroll'); + pushWhileLoading(app as unknown as BufferLoadApp, 'output-after-the-capture', 100); + + // The load ends with the terminal reset and rewritten, so it reads as bottom. + app.buffer.viewportY = app.buffer.baseY; + app._finishBufferLoad(owner, { flushQueued: true, since: 0 }); + expect(app._wasAtBottomBeforeWrite).toBe(true); + + // The caller now puts the reader back where they were reading. + app.buffer.viewportY = 40; + app._syncStickyScrollBaseline(); + + // The next flush must leave them there. + expect(app._wasAtBottomBeforeWrite).toBe(false); + }); + + it('a restore that lands back at the bottom keeps sticky scroll armed', () => { + const app = makeScrollApp(); + const owner = app._beginBufferLoad('load-scroll-bottom'); + pushWhileLoading(app as unknown as BufferLoadApp, 'output-after-the-capture', 100); + + app.buffer.viewportY = app.buffer.baseY; + app._finishBufferLoad(owner, { flushQueued: true, since: 0 }); + app._syncStickyScrollBaseline(); + + // A reader who was already at the bottom still wants to be carried along. + expect(app._wasAtBottomBeforeWrite).toBe(true); + }); + + it('both callers that restore a scroll position re-take the baseline', () => { + // The wiring lives in app.js, outside this file's vm harness. Without it the + // two methods below restore the viewport and the next flush undoes it. + const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8'); + + for (const method of ['_onSessionNeedsRefresh', '_maybeRefetchFullHistory']) { + const body = methodBody(source, method); + const restoreAt = body.lastIndexOf('scrollToLine('); + const syncAt = body.indexOf('this._syncStickyScrollBaseline()'); + expect(restoreAt, `${method} no longer restores a scroll position`).toBeGreaterThan(-1); + expect(syncAt, `${method} never re-takes the baseline`).toBeGreaterThan(-1); + expect(syncAt, `${method} re-takes the baseline before its restore`).toBeGreaterThan(restoreAt); + } + }); + it('empty queue + flushQueued is a no-op (no throw, no writes)', () => { const { app, writes } = makeApp(); const owner = app._beginBufferLoad('load-empty'); diff --git a/test/terminal-flush-budget.test.ts b/test/terminal-flush-budget.test.ts index 514e93da..aa103661 100644 --- a/test/terminal-flush-budget.test.ts +++ b/test/terminal-flush-budget.test.ts @@ -322,6 +322,51 @@ describe('terminal flush budget', () => { expect(app._bufferLoadOwner).toBe(null); }); + // ── Which payloads end their load by replaying the queue ── + // + // A pane capture is current only up to capture time, so the tail that arrived + // after the response exists nowhere else and has to be replayed. The server's + // accumulated byte history is current up to the response, so replaying on top + // of it would duplicate output. `_bufferLoadFinishOpts` is the one place that + // decides this, for all four paths that fetch a terminal buffer and write it. + + it('replays the tail for a visible-pane capture', () => { + const { CodemanApp } = loadAppHarness(); + const app = Object.create(CodemanApp.prototype) as any; + + expect(app._bufferLoadFinishOpts({ source: 'mux-visible' }, 1234)).toEqual({ + flushQueued: true, + since: 1234, + }); + }); + + it('replays the tail for a full-history capture', () => { + const { CodemanApp } = loadAppHarness(); + const app = Object.create(CodemanApp.prototype) as any; + + expect(app._bufferLoadFinishOpts({ source: 'mux-full-history' }, 1234)).toEqual({ + flushQueued: true, + since: 1234, + }); + }); + + it('discards the queue for the accumulated byte history', () => { + const { CodemanApp } = loadAppHarness(); + const app = Object.create(CodemanApp.prototype) as any; + + expect(app._bufferLoadFinishOpts({ source: 'history' }, 1234).flushQueued).toBe(false); + }); + + it('discards the queue for a payload that names no source', () => { + // Fails toward the safe answer: a duplicated Ink redraw corrupts the screen, + // while a dropped tail is repaired by the CLI's next full repaint. + const { CodemanApp } = loadAppHarness(); + const app = Object.create(CodemanApp.prototype) as any; + + expect(app._bufferLoadFinishOpts({}, 1234).flushQueued).toBe(false); + expect(app._bufferLoadFinishOpts(undefined, 1234).flushQueued).toBe(false); + }); + it('does not snap back to bottom during Codex Working redraws right after the user scrolls up', () => { const { app } = loadTerminalUiHarness('codex'); const scrollToBottom = vi.fn();