Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .changeset/fix-replay-output-that-arrived-after-the-capture.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
"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.

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.
1 change: 1 addition & 0 deletions config/test-suites.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
];

Expand Down
2 changes: 1 addition & 1 deletion docs/architecture-invariants.md

Large diffs are not rendered by default.

91 changes: 85 additions & 6 deletions src/web/public/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -1906,6 +1906,40 @@ 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.
*
* 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`.
*/
_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`);
Expand All @@ -1915,7 +1949,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
Expand Down Expand Up @@ -2498,9 +2532,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
Expand All @@ -2516,7 +2552,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);
Expand All @@ -2526,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
Expand All @@ -2552,6 +2597,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();
Expand All @@ -2561,7 +2607,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
Expand Down Expand Up @@ -5782,7 +5833,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
Expand All @@ -5800,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 {
Expand Down Expand Up @@ -6241,6 +6303,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;
Expand All @@ -6267,7 +6338,8 @@ class CodemanApp {
const { parsedAt: freshParsedAt } = await this.chunkedTerminalWrite(
data.terminalBuffer,
TERMINAL_CHUNK_SIZE,
bufferLoadOwner
bufferLoadOwner,
finishOpts
);
freshResetAndParseMs = freshParsedAt - replayStartedAt;
if (this._isStaleSelect(selectGen)) {
Expand Down Expand Up @@ -6317,7 +6389,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;
Expand Down
Loading
Loading