diff --git a/src/web/public/app.js b/src/web/public/app.js index 2c378b9d..1498e8cc 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -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 diff --git a/src/web/public/constants.js b/src/web/public/constants.js index a0609a2e..5988eeb6 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -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 }; } diff --git a/test/terminal-scroll-intent.test.ts b/test/terminal-scroll-intent.test.ts index b34f0020..717e34d6 100644 --- a/test/terminal-scroll-intent.test.ts +++ b/test/terminal-scroll-intent.test.ts @@ -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}'); + }); +});