fix(terminal): restore the history anchor after xterm parses, not before

.changeset/terminal-history-anchor-after-parse.md

@@ -0,0 +1,5 @@
+---
+"aicodeman": patch
+---
+
+Keep the terminal anchored where you are reading while an agent streams (#358). Scrolling up during a Codex response could still be dragged back to the live bottom by the next redraw: the flush captured the viewport before writing and restored it immediately after, but xterm parses asynchronously, so at that moment the buffer had not moved yet, the restore compared the anchor against itself and did nothing, and the redraw landed a tick later with nothing left to pull the view back. The restore now runs inside xterm's own write callback, which is the first point at which the redraw's effect exists, and it holds across consecutive and chunked redraws. It is dropped if you switch sessions or a history replay starts before the write parses, since the anchor indexes the buffer it was captured from.

src/web/public/terminal-ui.js

@@ -3543,6 +3543,29 @@ Object.assign(CodemanApp.prototype, {
     this._sendInputAsync(this.activeSessionId, text);
   },
 
+  /**
+   * Re-assert a history anchor captured before a terminal write (#358).
+   *
+   * Called from xterm's write callback, never synchronously after write():
+   * xterm parses on its own schedule, so the buffer only carries the redraw's
+   * effect once that callback fires. A null anchor means the user was following
+   * live output and nothing needs restoring.
+   */
+  _restoreTerminalViewport(preserveViewportY, sessionId) {
+    if (preserveViewportY === null || preserveViewportY === undefined) return;
+    // The anchor is a row index into the buffer it was captured from. Now that
+    // this runs a parse later instead of synchronously, a session switch can land
+    // in between: selectSession() resets the terminal and chunk-loads the new
+    // session's scrollback, and scrolling THAT buffer to a row that meant
+    // something in the previous one is not a restore, it is a jump to an
+    // arbitrary place. Both checks cover one half of that window.
+    if (sessionId !== undefined && sessionId !== this.activeSessionId) return;
+    if (this._isLoadingBuffer) return;
+    if (typeof this.terminal?.scrollToLine !== 'function') return;
+    if (this.terminal.buffer?.active?.viewportY === preserveViewportY) return;
+    this.terminal.scrollToLine(preserveViewportY);
+  },
+
   /**
    * Flush pending writes to terminal, processing DEC 2026 sync markers.
    * Strips markers and writes content atomically within a single frame.
@@ -3578,6 +3601,8 @@ Object.assign(CodemanApp.prototype, {
     // scroll-to-bottom below, where it protects against a mid-flush race.
     const preserveViewportY =
       this.terminal.buffer?.active && !this.isTerminalAtBottom() ? this.terminal.buffer.active.viewportY : null;
+    // Which buffer the anchor belongs to, checked again when the write parses.
+    const flushSessionId = this.activeSessionId;
 
     const writeChunk = joined.slice(0, MAX_FRAME_BYTES);
     if (_joinedLen > MAX_FRAME_BYTES) {
@@ -3592,20 +3617,23 @@ Object.assign(CodemanApp.prototype, {
       this.terminal.write(writeChunk, () => {
         this._terminalWriteInFlight = false;
         this._terminalWriteInFlightBytes = 0;
+        // Restore INSIDE the callback (#358). xterm parses asynchronously, so
+        // the moment write() returns the buffer has not moved yet: the old
+        // restore ran here, found viewportY still equal to the anchor, and did
+        // nothing at all — then the parse landed and a cursor-addressed Codex
+        // redraw dragged the viewport to the live bottom with nothing left to
+        // pull it back. The callback is xterm's own "this chunk is parsed"
+        // signal, which is the earliest point the anchor can actually be
+        // reasserted. (The synchronous version passed its regression test only
+        // because the test's write mock moved the viewport synchronously.)
+        this._restoreTerminalViewport(preserveViewportY, flushSessionId);
         this._scheduleTerminalWriteFlush();
       });
     } catch (err) {
       this._terminalWriteInFlight = false;
       this._terminalWriteInFlightBytes = 0;
       throw err;
     }
-    if (
-      preserveViewportY !== null &&
-      this.terminal.buffer?.active?.viewportY !== preserveViewportY &&
-      typeof this.terminal.scrollToLine === 'function'
-    ) {
-      this.terminal.scrollToLine(preserveViewportY);
-    }
     const bytesThisFrame = deferred ? MAX_FRAME_BYTES : _joinedLen;
     const _dt = performance.now() - _t0;
     if (_dt > 100 || deferred)
@@ -3617,7 +3645,13 @@ Object.assign(CodemanApp.prototype, {
     // Give manual scroll-up gestures a short grace window so high-frequency
     // Codex status ticks do not snap the viewport back while the user is
     // trying to inspect earlier output.
-    if (this._wasAtBottomBeforeWrite && !this._hasRecentUserScrollUp()) {
+    //
+    // A live anchor wins outright. The two flags are captured at different
+    // moments (_wasAtBottomBeforeWrite at the frame's first batchTerminalWrite,
+    // the anchor at flush time), so a scroll-up in between leaves both set; now
+    // that the anchor is reasserted after the parse, running both would jump to
+    // the bottom and then back one frame later instead of simply staying put.
+    if (preserveViewportY === null && this._wasAtBottomBeforeWrite && !this._hasRecentUserScrollUp()) {
       this.terminal.scrollToBottom();
     }
 

test/terminal-flush-budget.test.ts

@@ -49,6 +49,39 @@ function loadTerminalUiHarness(mode: string) {
   return { app, writes };
 }
 
+/**
+ * Swap in a terminal whose write() parses ASYNCHRONOUSLY, the way xterm.js does.
+ *
+ * The real renderer queues the chunk and applies it later, firing the write
+ * callback once it has been parsed; a redraw that addresses a row past the
+ * viewport (Codex's status line) drags the viewport to the live bottom at that
+ * point, not when write() returns. `parse()` runs that pending work.
+ */
+function attachAsyncParsingTerminal(app: any, opts: { viewportY: number; baseY: number }) {
+  const buffer = { viewportY: opts.viewportY, baseY: opts.baseY };
+  const pending: Array<() => void> = [];
+  app.terminal.buffer = { active: buffer };
+  app.terminal.write = vi.fn((_data: string, callback?: () => void) => {
+    pending.push(() => {
+      buffer.viewportY = buffer.baseY; // the redraw lands
+      callback?.();
+    });
+  });
+  app.terminal.scrollToLine = vi.fn((line: number) => {
+    buffer.viewportY = line;
+  });
+  app.terminal.scrollToBottom = vi.fn(() => {
+    buffer.viewportY = buffer.baseY;
+  });
+  return {
+    buffer,
+    parse: () => {
+      const queued = pending.splice(0, pending.length);
+      for (const run of queued) run();
+    },
+  };
+}
+
 function loadAppHarness() {
   const dir = resolve(import.meta.dirname, '../src/web/public');
   const fetchMock = vi.fn();
@@ -337,20 +370,111 @@ describe('terminal flush budget', () => {
 
   it('restores the user scroll position when Codex Working redraws move the viewport', () => {
     const { app } = loadTerminalUiHarness('codex');
-    const buffer = { viewportY: 40, baseY: 100 };
-    app.terminal.buffer = { active: buffer };
-    app.terminal.write = vi.fn(() => {
-      buffer.viewportY = buffer.baseY;
-    });
-    app.terminal.scrollToLine = vi.fn((line: number) => {
-      buffer.viewportY = line;
-    });
+    const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
+    app._wasAtBottomBeforeWrite = true;
+    app._lastUserScrollUpAt = 0;
+    app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
+
+    app.flushPendingWrites();
+    parse();
+
+    expect(buffer.viewportY).toBe(40);
+  });
+
+  // Issue #358. xterm.js parses on its own schedule, so the buffer still holds
+  // the pre-write viewport the instant write() returns: restoring there compared
+  // the anchor against itself, did nothing, and left the redraw free to drag the
+  // viewport to the live bottom a tick later. The previous regression passed
+  // because its write mock moved the viewport synchronously, which real xterm
+  // never does. These drive the callback explicitly instead.
+  it('restores the history anchor only AFTER xterm has parsed the write (#358)', () => {
+    const { app } = loadTerminalUiHarness('codex');
+    const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
+    app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
+
+    app.flushPendingWrites();
+    // Nothing has parsed yet, so nothing may have been restored yet either.
+    expect(app.terminal.scrollToLine).not.toHaveBeenCalled();
+    expect(buffer.viewportY).toBe(40);
+
+    parse();
+
+    expect(app.terminal.scrollToLine).toHaveBeenCalledWith(40);
+    expect(buffer.viewportY).toBe(40);
+  });
+
+  it('holds the anchor across consecutive Codex redraws', () => {
+    const { app } = loadTerminalUiHarness('codex');
+    const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
+
+    for (const frame of ['\x1b[55;1H\x1b[2m• Working (6s)', '\x1b[55;1H\x1b[2m• Working (7s)']) {
+      app.pendingWrites.push(frame);
+      app.flushPendingWrites();
+      parse();
+      expect(buffer.viewportY).toBe(40);
+    }
+  });
+
+  it('holds the anchor across a chunked write whose remainder is deferred', () => {
+    const { app } = loadTerminalUiHarness('codex');
+    const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
+    // Over the 32KB codex frame budget, so the flush defers a remainder and the
+    // second chunk goes out from the write callback's reschedule.
+    app.pendingWrites.push('x'.repeat(40000));
+
+    app.flushPendingWrites();
+    parse();
+    expect(buffer.viewportY).toBe(40);
+
+    app.flushPendingWrites();
+    parse();
+    expect(buffer.viewportY).toBe(40);
+    expect(app.pendingWrites).toHaveLength(0);
+  });
+
+  it('drops the anchor when the user switched sessions before the write parsed', () => {
+    // The anchor indexes the buffer it came from. selectSession() resets the
+    // terminal and chunk-loads a different scrollback, so replaying row 40 into
+    // that one is a jump to an arbitrary place, not a restore. Only reachable now
+    // that the restore runs a parse later than the write.
+    const { app } = loadTerminalUiHarness('codex');
+    const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
+    app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
+
+    app.flushPendingWrites();
+    app.activeSessionId = 'session-2'; // the user clicked another tab
+    parse();
+
+    expect(app.terminal.scrollToLine).not.toHaveBeenCalled();
+    expect(buffer.viewportY).toBe(buffer.baseY);
+  });
+
+  it('drops the anchor while a buffer load is replaying history', () => {
+    const { app } = loadTerminalUiHarness('codex');
+    const { parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
+    app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
+
+    app.flushPendingWrites();
+    app._isLoadingBuffer = true; // chunkedTerminalWrite owns the viewport now
+    parse();
+
+    expect(app.terminal.scrollToLine).not.toHaveBeenCalled();
+  });
+
+  it('does not bounce off the bottom when the sticky flag and an anchor disagree', () => {
+    // _wasAtBottomBeforeWrite is captured at the frame's first batchTerminalWrite
+    // and the anchor at flush time, so a scroll-up in between leaves both live.
+    // The anchor wins: scrolling to the bottom and back would be a visible jump.
+    const { app } = loadTerminalUiHarness('codex');
+    const { buffer, parse } = attachAsyncParsingTerminal(app, { viewportY: 40, baseY: 100 });
     app._wasAtBottomBeforeWrite = true;
     app._lastUserScrollUpAt = 0;
     app.pendingWrites.push('\x1b[55;1H\x1b[2m• Working (6s)');
 
     app.flushPendingWrites();
+    parse();
 
+    expect(app.terminal.scrollToBottom).not.toHaveBeenCalled();
     expect(buffer.viewportY).toBe(40);
   });
 });