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 a queued event already appears in it and replaying it would duplicate output, most visibly Ink's cursor-up redraws. A tmux pane capture is a photograph, current only as of the instant `capture-pane` ran. Output printed afterwards was queued and then dropped, and nothing scheduled a re-fetch to recover it: `_onSessionNeedsRefresh` is wired only to the 128KB overflow path. The CLI's next partial redraw then landed on a frame the terminal never received. How much went missing depended on which capture the route served. A `?full=1` load returns the capture alone, with no history in front of it, so it lost everything from the capture to the end of the chunked write. A `?tail=` load returns history, a clear, and then the capture, and the route reads that history after the capture, so it lost everything from the response to the end of that write. The chunked write dominates either way. An agent CLI hides the loss on its next full redraw; a shell session does not, 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. The earlier events stay dropped, because a payload that carries history does hold those. All four paths that fetch a terminal buffer and write it now decide this the same way, through one `_bufferLoadFinishOpts` helper, so they cannot drift apart: `selectSession`, `_onSessionNeedsRefresh`, `_onSessionClearTerminal` and `_maybeRefetchFullHistory`. The second of those is the one that stings. It exists to restore output the client already dropped once under backpressure, and it was dropping more output while performing that recovery. The cache-hit write inside `selectSession` stays on discard deliberately: it runs before the fetch, so its queue holds only events the capture that follows already contains. Two further things had to change for that tail to still exist when the load ends, and a browser test is what found both. `chunkedTerminalWrite` is what ends the load for every non-empty buffer, so the flush policy travels to its own finish calls; the call in `selectSession` runs only when the write was skipped. `_beginBufferLoad` no longer empties the queue when one load re-enters it, which it does on every write, because that reset discarded the whole fetch window before anything could replay it. The response already distinguishes the sources. `source` reads `mux-visible` or `mux-full-history` for a capture and `history` for the byte stream. Follows #395, #396 and #397, which fixed the ways the replayed frame itself could disagree with the terminal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-10-07 07:59:42 +02:00 · 2026-09-15 18:14:18 +02:00
parent c03714eb74
commit c9515b1d4c
6 changed files with 413 additions and 26 deletions
@@ -0,0 +1,177 @@
+/**
+ * @fileoverview Output arriving after a pane capture survives the buffer load.
+ *
+ * `batchTerminalWrite` queues live terminal events while a buffer load runs,
+ * and `_finishBufferLoad` discards that queue by default. That is right when
+ * the loaded buffer is the server's accumulated byte history, which is current
+ * up to the response. A tmux pane capture is current only up to CAPTURE time,
+ * so anything arriving between the capture and the end of the chunked write is
+ * queued and then dropped, with nothing scheduling a re-fetch.
+ *
+ * The queue now stamps each entry with its arrival time, and a capture load
+ * replays the tail that arrived after the response headers. These drive the
+ * real client in chromium: the event is injected from inside the response's
+ * own `json()` call, which is the one place guaranteed to land after the
+ * headers and before the chunked write.
+ *
+ * Port: 3256 (capture load window)
+ *
+ * Run: npx vitest run --config config/vitest.browser.config.ts test/capture-load-window.browser.test.ts
+ */
+
+import { describe, it, expect, beforeAll, afterAll } from 'vitest';
+import { chromium, type Browser, type BrowserContext, type Page } from 'playwright';
+import { WebServer } from '../src/web/server.js';
+
+const PORT = 3256;
+const BASE_URL = `http://localhost:${PORT}`;
+const MARKER = 'ARRIVED-AFTER-THE-CAPTURE';
+
+let server: WebServer;
+let browser: Browser;
+
+beforeAll(async () => {
+  server = new WebServer(PORT, false, true); // testMode
+  await server.start();
+  browser = await chromium.launch({ headless: true });
+}, 60_000);
+
+afterAll(async () => {
+  await browser?.close();
+  await server?.stop();
+}, 30_000);
+
+/**
+ * Select the session with the terminal fetch stubbed, injecting one live event
+ * from inside `json()`. Returns how many terminal rows carry the marker, so a
+ * flush that replays too much fails as loudly as one that replays nothing.
+ */
+async function runLoad(page: Page, sessionId: string, source: string): Promise<number> {
+  return page.evaluate(
+    async ({ sid, src, marker }) => {
+      const app = (
+        window as unknown as {
+          app: {
+            selectSession: (id: string, o?: object) => Promise<void>;
+            _onSessionTerminal: (e: { id: string; data: string }) => void;
+            terminal: {
+              buffer: {
+                active: {
+                  length: number;
+                  getLine: (i: number) => { translateToString: (t: boolean) => string } | undefined;
+                };
+              };
+            };
+          };
+        }
+      ).app;
+
+      const realFetch = window.fetch.bind(window);
+      window.fetch = ((input: RequestInfo | URL, init?: RequestInit) => {
+        const url = String(typeof input === 'string' ? input : ((input as Request).url ?? input));
+        if (!url.includes('/terminal')) return realFetch(input as RequestInfo, init);
+        return Promise.resolve({
+          ok: true,
+          status: 200,
+          // `selectSession` timestamps the headers the moment this promise
+          // resolves, then calls json(). Injecting here puts the event after
+          // that timestamp and inside the load window, which is exactly the
+          // gap a pane capture cannot cover.
+          json: async () => {
+            app._onSessionTerminal({ id: sid, data: `\r\n${marker}\r\n` });
+            return {
+              success: true,
+              data: {
+                terminalBuffer: '\x1b[1;1Hcaptured frame line one\r\n',
+                status: 'idle',
+                fullSize: 512,
+                retainedBytes: 512,
+                truncated: false,
+                truncationReason: null,
+                source: src,
+                captureCols: 80,
+                captureRows: 24,
+              },
+            };
+          },
+        }) as unknown as Promise<Response>;
+      }) as typeof window.fetch;
+
+      try {
+        await app.selectSession(sid);
+        await new Promise((r) => setTimeout(r, 1200));
+        const buf = app.terminal.buffer.active;
+        let hits = 0;
+        for (let i = 0; i < buf.length; i++) {
+          if (buf.getLine(i)?.translateToString(true).includes(marker)) hits += 1;
+        }
+        return hits;
+      } finally {
+        window.fetch = realFetch;
+      }
+    },
+    { sid: sessionId, src: source, marker: MARKER }
+  );
+}
+
+async function openSession(page: Page): Promise<string> {
+  await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
+  await page.waitForFunction(() => document.body.classList.contains('app-loaded'), { timeout: 10_000 });
+  // xterm loads from /vendor, so the terminal appears a beat after the app.
+  // Without it every buffer assertion below would throw rather than compare.
+  await page.waitForFunction(() => (window as unknown as { app?: { terminal?: unknown } }).app?.terminal, null, {
+    timeout: 30_000,
+  });
+  return page.evaluate(async () => {
+    const res = await fetch('/api/sessions', {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({ workingDir: '/tmp', name: 'capture-load-window-test' }),
+    });
+    const body = await res.json();
+    return body.data?.session?.id ?? body.data?.id ?? body.id;
+  });
+}
+
+describe('output emitted during a capture load', () => {
+  let context: BrowserContext;
+  let page: Page;
+
+  afterAll(async () => {
+    await context?.close();
+  });
+
+  it('reaches the terminal exactly once when the buffer came from a pane capture', async () => {
+    context = await browser.newContext({ viewport: { width: 1280, height: 800 } });
+    page = await context.newPage();
+    const sessionId = await openSession(page);
+    expect(sessionId).toBeTruthy();
+
+    // Exactly once. The cutoff exists so the flush cannot also replay events the
+    // payload already carried, which would double the output rather than heal it.
+    expect(await runLoad(page, sessionId, 'mux-visible')).toBe(1);
+
+    await page.evaluate(
+      (sid: string) => fetch(`/api/sessions/${sid}`, { method: 'DELETE' }).then(() => undefined),
+      sessionId
+    );
+    await context.close();
+  }, 60_000);
+
+  it('stays dropped when the buffer came from the accumulated byte history', async () => {
+    // The byte history already contains everything up to the response, so
+    // replaying the queue on top of it would duplicate the output — most
+    // visibly Ink's cursor-up redraws. The discard has to survive this fix.
+    context = await browser.newContext({ viewport: { width: 1280, height: 800 } });
+    page = await context.newPage();
+    const sessionId = await openSession(page);
+
+    expect(await runLoad(page, sessionId, 'history')).toBe(0);
+
+    await page.evaluate(
+      (sid: string) => fetch(`/api/sessions/${sid}`, { method: 'DELETE' }).then(() => undefined),
+      sessionId
+    );
+    await context.close();
+  }, 60_000);
+});
@@ -56,10 +56,10 @@ type BufferLoadApp = {
  _bufferLoadSeq: number;
  _bufferLoadOwner: string | null;
  _isLoadingBuffer: boolean;
-  _loadBufferQueue: string[] | null;
+  _loadBufferQueue: { at: number; data: string }[] | null;
  batchTerminalWrite: (data: string) => void;
  _beginBufferLoad: (owner?: string) => string;
-  _finishBufferLoad: (owner?: string, opts?: { flushQueued?: boolean }) => boolean;
+  _finishBufferLoad: (owner?: string, opts?: { flushQueued?: boolean; since?: number }) => boolean;
 };

 /**
@@ -84,10 +84,13 @@ function makeApp() {
  return { app, writes };
 }

-/** Simulate live SSE events arriving while a buffer load is in progress (the queue path). */
-function pushWhileLoading(app: BufferLoadApp, data: string) {
-  // Mirrors batchTerminalWrite's queue branch: if loading, push to the queue.
-  if (app._isLoadingBuffer && app._loadBufferQueue) app._loadBufferQueue.push(data);
+/**
+ * Simulate a live SSE event arriving while a buffer load is in progress.
+ * Mirrors batchTerminalWrite's queue branch, which stamps each entry with its
+ * arrival time so a flush can replay only the tail (see the `since` tests).
+ */
+function pushWhileLoading(app: BufferLoadApp, data: string, at = performance.now()) {
+  if (app._isLoadingBuffer && app._loadBufferQueue) app._loadBufferQueue.push({ at, data });
 }

 describe('buffer-load flush (COD-144)', () => {
@@ -153,11 +156,91 @@ describe('buffer-load flush (COD-144)', () => {
    // State untouched — still loading, queue intact, nothing replayed.
    expect(app._isLoadingBuffer).toBe(true);
    expect(app._bufferLoadOwner).toBe('real-owner');
-    expect(app._loadBufferQueue).toEqual(['queued']);
+    expect(app._loadBufferQueue).toEqual([{ at: expect.any(Number), data: 'queued' }]);
    expect(app.batchTerminalWrite).not.toHaveBeenCalled();
    expect(writes).toEqual([]);
  });

+  // ── The tmux-capture tail: `since` ──
+  //
+  // A pane capture is a point-in-time frame taken part-way through the fetch, so
+  // it holds what arrived BEFORE the capture and nothing after. selectSession
+  // passes the response's arrival time as `since`, which splits the queue at
+  // exactly that line: pre-capture events are already painted and must stay
+  // dropped, post-capture events exist nowhere else and must be replayed.
+
+  it('flushes only the entries at or after `since`', () => {
+    const { app, writes } = makeApp();
+    const owner = app._beginBufferLoad('load-since');
+    pushWhileLoading(app, 'already-in-the-capture', 100);
+    pushWhileLoading(app, 'arrived-at-the-headers', 200);
+    pushWhileLoading(app, 'arrived-after-the-headers', 300);
+
+    app._finishBufferLoad(owner, { flushQueued: true, since: 200 });
+
+    // The pre-capture event stays dropped; the boundary entry counts as after.
+    expect(writes).toEqual(['arrived-at-the-headers', 'arrived-after-the-headers']);
+  });
+
+  it('flushQueued without `since` still replays the whole queue', () => {
+    // The COD-144 path: a brand-new session's first prompt predates the
+    // response, so cutting the queue would drop the only content it has.
+    const { app, writes } = makeApp();
+    const owner = app._beginBufferLoad('load-no-since');
+    pushWhileLoading(app, 'prompt', 10);
+    pushWhileLoading(app, 'more', 20);
+
+    app._finishBufferLoad(owner, { flushQueued: true });
+
+    expect(writes).toEqual(['prompt', 'more']);
+  });
+
+  it('a `since` past every entry flushes nothing', () => {
+    const { app, writes } = makeApp();
+    const owner = app._beginBufferLoad('load-since-late');
+    pushWhileLoading(app, 'old', 10);
+
+    app._finishBufferLoad(owner, { flushQueued: true, since: 999 });
+
+    expect(writes).toEqual([]);
+    expect(app.batchTerminalWrite).not.toHaveBeenCalled();
+  });
+
+  // ── Re-entering one load ──
+  //
+  // `selectSession` opens the load before its fetch, and `chunkedTerminalWrite`
+  // opens it again under the SAME owner when it starts writing. A reset on that
+  // second call would silently throw away everything queued during the fetch,
+  // which on the capture path is output no buffer holds.
+
+  it('re-entering the same load keeps what the queue already holds', () => {
+    const { app, writes } = makeApp();
+    const owner = app._beginBufferLoad('load-reenter');
+    pushWhileLoading(app, 'arrived-during-the-fetch', 100);
+
+    // chunkedTerminalWrite re-opens the load it was handed.
+    app._beginBufferLoad(owner);
+    pushWhileLoading(app, 'arrived-during-the-write', 200);
+
+    app._finishBufferLoad(owner, { flushQueued: true, since: 50 });
+
+    expect(writes).toEqual(['arrived-during-the-fetch', 'arrived-during-the-write']);
+  });
+
+  it('a genuinely different load still starts with an empty queue', () => {
+    const { app, writes } = makeApp();
+    app._beginBufferLoad('load-first');
+    pushWhileLoading(app, 'belongs-to-the-abandoned-load', 100);
+
+    // A tab switch starts a new load under a new owner. Its events are not ours.
+    const second = app._beginBufferLoad('load-second');
+    pushWhileLoading(app, 'belongs-to-this-load', 200);
+
+    app._finishBufferLoad(second, { flushQueued: true, since: 0 });
+
+    expect(writes).toEqual(['belongs-to-this-load']);
+  });
+
  it('empty queue + flushQueued is a no-op (no throw, no writes)', () => {
    const { app, writes } = makeApp();
    const owner = app._beginBufferLoad('load-empty');