fix(terminal): stop the backpressure refresh yanking and shrinking the buffer

Two further instances of the same root cause, both in _onSessionNeedsRefresh,
which is SERVER-triggered (it fires after SSE backpressure clears) so the user
has no gesture to blame the result on.

1. It ended in an unconditional scrollToBottom, so a user quietly reading
   scrollback was dropped to the live output by a background event. It now
   holds their place. The rewrite REPLACES the buffer, so an absolute viewportY
   captured beforehand is meaningless afterwards; distance from the bottom is
   the anchor that survives, via computeRewriteScrollLine().

2. It rebuilt the terminal from a 1MB TAIL. Measured end to end on a 900-line
   shell pane: an 869-row buffer came back as 158 rows, so the refresh meant to
   REPAIR the display was destroying most of the scrollback every time it ran.
   It now asks for full history, and falls back to the tail only when
   _replayWouldShrinkBuffer refuses the capture, which keeps repaint-mode panes
   (tmux holds roughly one frame for them) exactly as they were.

Also records truncation state here, so the #258 banner stops describing the
pre-refresh buffer.

Verified in a real browser against a live session: baseY 869 -> 869 where it
used to be 869 -> 158, a reader 200 lines up stays 200 lines up, and a follower
stays pinned to the bottom.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-08-14 00:51:58 +02:00
parent 9a0e665f72
commit 6866a617a8
3 changed files with 111 additions and 4 deletions
+31 -3
View File
@@ -2196,14 +2196,42 @@ class CodemanApp {
if (!this.activeSessionId || !this.terminal) return;
// Skip if buffer load already in progress — avoids competing clear+rewrite cycles
if (this._isLoadingBuffer) return;
const sessionId = this.activeSessionId;
try {
const res = await fetch(`/api/sessions/${this.activeSessionId}/terminal?tail=${TERMINAL_TAIL_SIZE}`);
const data = (await res.json())?.data ?? {};
// Recovery should restore the WHOLE picture, so ask for full history
// rather than a tail. Measured on a 900-line shell pane: the tail rewrite
// replaced an 869-row buffer with 158 rows, so every backpressure refresh
// silently destroyed most of the scrollback it was meant to repair.
//
// A repaint-mode pane is the opposite case (tmux keeps ~one frame for it),
// so the full capture can be SMALLER than what xterm already holds. Reuse
// the same downgrade guard as the scroll-to-top re-pull and fall back to
// the historical tail there, leaving that case exactly as it was.
let res = await fetch(`/api/sessions/${sessionId}/terminal?full=1`);
let data = (await res.json())?.data ?? {};
if (data.terminalBuffer && this._replayWouldShrinkBuffer(data.terminalBuffer)) {
res = await fetch(`/api/sessions/${sessionId}/terminal?tail=${TERMINAL_TAIL_SIZE}`);
data = (await res.json())?.data ?? {};
}
if (data.terminalBuffer) {
// This refresh is SERVER-triggered, so a user quietly reading scrollback
// did not ask for it and must not be dragged to the bottom by it (#259).
// The rewrite replaces the buffer, so an absolute viewportY is
// meaningless across it — distance from the bottom is what survives.
const before = this.terminal.buffer?.active;
const linesFromBottom = before ? Math.max(0, (before.baseY || 0) - (before.viewportY || 0)) : 0;
this.terminal.clear();
this.terminal.reset();
await this.chunkedTerminalWrite(data.terminalBuffer);
this.terminal.scrollToBottom();
// A tail fetch can be partial, and the banner would otherwise keep
// describing the pre-refresh buffer (#258).
if (this.activeSessionId === sessionId) this._setHistoryTruncation(sessionId, data);
const target = computeRewriteScrollLine({
linesFromBottom,
baseY: this.terminal.buffer?.active?.baseY ?? 0,
});
if (target === null || typeof this.terminal.scrollToLine !== 'function') this.terminal.scrollToBottom();
else this.terminal.scrollToLine(target);
// Re-position local echo overlay at new prompt location
this._localEchoOverlay?.rerender();
// Resize PTY to match actual browser dimensions (critical for OpenCode
+21 -1
View File
@@ -848,6 +848,26 @@ function computeHistoryTruncationNotice(state = {}) {
};
}
/**
* Where to land after a rewrite that REPLACES the whole buffer (#259).
*
* The backpressure refresh clears and reloads the terminal from a fresh tail,
* so an absolute viewportY captured beforehand means nothing afterwards: the
* line it pointed at may not even exist. Distance from the BOTTOM is the anchor
* that survives a rewrite, so a reader stays roughly where they were reading.
*
* Returns null when the user was following live output, which the caller reads
* as "scroll to bottom" — the historical behavior, kept for that case.
*
* @param {{linesFromBottom?: number, baseY?: number}} input
* @returns {number|null}
*/
function computeRewriteScrollLine(input) {
const linesFromBottom = input?.linesFromBottom || 0;
if (!(linesFromBottom > 0)) return null;
return Math.max(0, (input?.baseY || 0) - linesFromBottom);
}
if (typeof window !== 'undefined') {
window.CodemanHistoryFormat = { formatHistoryBytes, computeHistoryTruncationNotice };
window.CodemanHistoryFormat = { formatHistoryBytes, computeHistoryTruncationNotice, computeRewriteScrollLine };
}
+59
View File
@@ -155,3 +155,62 @@ describe('keyboard show/hide route through the intent-preserving path (static gu
expect(SOURCE).not.toContain('scrollToBottom: true');
});
});
describe('backpressure refresh keeps a reader in place (issue #259)', () => {
// _onSessionNeedsRefresh is SERVER-triggered: it fires after SSE backpressure
// clears and rewrites the whole buffer. A user quietly reading scrollback did
// not ask for it, so being dropped to the bottom by it is the same bug as the
// keyboard yank, with no gesture to blame it on.
const loadConstants = () => {
const context = vm.createContext({ console, window: {}, document: {}, navigator: { userAgent: 'test' } });
vm.runInContext(
`${readFileSync(resolve(PUBLIC, 'constants.js'), 'utf8')}\n;globalThis.__fn = computeRewriteScrollLine;`,
context,
{ filename: 'constants.js' }
);
return (context as any).__fn as (i: { linesFromBottom?: number; baseY?: number }) => number | null;
};
it('returns null (scroll to bottom) for someone following live output', () => {
const computeRewriteScrollLine = loadConstants();
expect(computeRewriteScrollLine({ linesFromBottom: 0, baseY: 900 })).toBeNull();
});
it('holds the reader the same distance from the bottom of the NEW buffer', () => {
const computeRewriteScrollLine = loadConstants();
// The rewrite replaces the buffer, so the old absolute line is meaningless;
// 50 lines up stays 50 lines up even though baseY changed.
expect(computeRewriteScrollLine({ linesFromBottom: 50, baseY: 900 })).toBe(850);
expect(computeRewriteScrollLine({ linesFromBottom: 50, baseY: 400 })).toBe(350);
});
it('clamps when the refreshed buffer is shorter than the old offset', () => {
const computeRewriteScrollLine = loadConstants();
expect(computeRewriteScrollLine({ linesFromBottom: 900, baseY: 100 })).toBe(0);
});
it('is wired into the refresh path instead of an unconditional scrollToBottom', () => {
const app = readFileSync(resolve(PUBLIC, 'app.js'), 'utf8');
const start = app.indexOf('async _onSessionNeedsRefresh()');
expect(start).toBeGreaterThan(-1);
const body = app.slice(start, app.indexOf('\n async _onSessionClearTerminal', start));
expect(body).toContain('computeRewriteScrollLine');
// The bottom is now one branch of a decision, never the whole story.
expect(body).toContain('this.terminal.scrollToLine(target)');
});
it('recovers FULL history, guarded against a repaint-pane downgrade', () => {
// Measured before the fix: this path rewrote an 869-row buffer from a 1MB
// tail and left 158 rows, so the refresh meant to REPAIR the terminal was
// destroying most of its scrollback. It asks for full history now, and
// falls back to the tail only when the full capture would shrink the buffer
// (a repaint-mode pane keeps roughly one frame in tmux).
const app = readFileSync(resolve(PUBLIC, 'app.js'), 'utf8');
const start = app.indexOf('async _onSessionNeedsRefresh()');
const body = app.slice(start, app.indexOf('\n async _onSessionClearTerminal', start));
expect(body).toContain('terminal?full=1');
expect(body).toContain('this._replayWouldShrinkBuffer(data.terminalBuffer)');
// The tail must survive as the fallback, not vanish.
expect(body).toContain('tail=${TERMINAL_TAIL_SIZE}');
});
});