fix(terminal): re-take the sticky-scroll baseline after a replay

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) <noreply@anthropic.com>
This commit is contained in:
Michael Grundberg
2026-09-18 16:43:04 +02:00
co-authored by Claude Opus 5
parent c9515b1d4c
commit 75a028e825
7 changed files with 203 additions and 1 deletions
@@ -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.
File diff suppressed because one or more lines are too long
+20
View File
@@ -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 {
+21
View File
@@ -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.
+3
View File
@@ -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);
+98
View File
@@ -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');
+45
View File
@@ -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();