From 584910f64533bc3aa079d0637990d210b675dfa2 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 21 Jun 2026 14:47:21 -0400 Subject: [PATCH] COD-144 flush queued SSE output on empty buffer-load so new shells paint immediately A freshly created shell session rendered blank until a tab-switch. selectSession() fetches the terminal buffer, but for a just-started shell that fetch resolves before the PTY emits its prompt, so the buffer is empty; the prompt then arrives as a live SSE event queued during the load and _finishBufferLoad() discarded it. The discard is correct for an established session (its fetched buffer already contains that output), but harmful when the load painted nothing. _finishBufferLoad(owner, { flushQueued }) now REPLAYS the queued events through batchTerminalWrite (after _isLoadingBuffer is cleared, so they write through, not re-queue) instead of discarding. selectSession passes flushQueued only in the empty branch (no fresh buffer + no cache), so the established-session de-dup path is unchanged. TDD: test/terminal-buffer-flush.test.ts exercises the real begin/finish mixin (vm-harness, no jsdom). --- src/web/public/app.js | 14 ++- src/web/public/terminal-ui.js | 32 +++++- test/terminal-buffer-flush.test.ts | 172 +++++++++++++++++++++++++++++ 3 files changed, 209 insertions(+), 9 deletions(-) create mode 100644 test/terminal-buffer-flush.test.ts diff --git a/src/web/public/app.js b/src/web/public/app.js index fa35d98c..522b542b 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -3733,6 +3733,9 @@ class CodemanApp { // the buffer write, causing 70KB+ single-frame flushes that stall WebGL. // chunkedTerminalWrite also sets this, but we need it before the fetch too. const bufferLoadOwner = this._beginBufferLoad(selectGen); + // COD-144: track whether the load painted nothing (empty fetch + no cache). + // For that just-created-session case we flush (not discard) queued SSE events. + let bufferWasEmpty = false; try { // Fit terminal to container BEFORE writing any buffer data. // If the browser was resized while viewing another session, the terminal @@ -3889,13 +3892,16 @@ class CodemanApp { } else if (!cachedBuffer) { // No fresh buffer and no cache — clear any stale content this._resetTerminalForReplay(); + bufferWasEmpty = true; } - // Buffer load complete — unblock live SSE writes (queued events are discarded - // to prevent duplicate content). chunkedTerminalWrite calls _finishBufferLoad - // internally, but if we skipped the write (cache hit or empty), call it here. + // Buffer load complete — unblock live SSE writes. chunkedTerminalWrite calls + // _finishBufferLoad internally (discarding queued events to prevent duplicate + // content); if we skipped the write (cache hit or empty), call it here. + // 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); + this._finishBufferLoad(bufferLoadOwner, { flushQueued: bufferWasEmpty }); } // Drop the guard so user input clears state normally this._restoringFlushedState = false; diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index d44e5dbf..d38b7049 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -1925,11 +1925,25 @@ Object.assign(CodemanApp.prototype, { * Complete a buffer load: unblock live SSE writes. * Called when chunkedTerminalWrite finishes (or is skipped for empty buffers). * - * Queued SSE events are DISCARDED, not flushed. The loaded buffer from the API - * is the source of truth up to the response timestamp. SSE events queued during - * the fetch+write overlap with the buffer — flushing them writes duplicate data - * (especially Ink cursor-up redraws), corrupting the terminal display. + * By default queued SSE events are DISCARDED, not flushed. For an established + * session the loaded buffer from the API is the source of truth up to the + * response timestamp; SSE events queued during the fetch+write overlap already + * appear in that buffer, so flushing them writes duplicate data (especially Ink + * cursor-up redraws), corrupting the terminal display. + * + * COD-144: a brand-new session is the exception. Its terminal fetch can resolve + * BEFORE the PTY emits its first prompt, so the fetched buffer is empty and the + * prompt arrives only as a queued SSE event. Discarding it leaves the terminal + * blank until a tab-switch re-fetches a now-populated buffer. When the caller + * knows the load painted nothing (empty fetch + no cache), it passes + * `{ flushQueued: true }` so the queued events are REPLAYED through + * `batchTerminalWrite()` instead of dropped. Replay runs after `_isLoadingBuffer` + * is cleared, so the events write through normally and are not re-queued. + * * After unblocking, new SSE/WS events deliver subsequent output normally. + * + * @param {string} [owner] Load token from `_beginBufferLoad`; a stale owner is a no-op. + * @param {{ flushQueued?: boolean }} [opts] When `flushQueued` is true, replay any queued events. */ _beginBufferLoad(owner) { if (this._bufferLoadSeq === undefined) this._bufferLoadSeq = 0; @@ -1940,13 +1954,21 @@ Object.assign(CodemanApp.prototype, { return loadOwner; }, - _finishBufferLoad(owner) { + _finishBufferLoad(owner, opts) { if (owner !== undefined && this._bufferLoadOwner !== owner) { return false; } + const queued = this._loadBufferQueue; this._isLoadingBuffer = false; this._loadBufferQueue = null; this._bufferLoadOwner = null; + // COD-144: replay (rather than discard) queued live events when the load + // painted nothing — the queued prompt is the only content a new session has. + if (opts?.flushQueued && queued && queued.length) { + for (const data of queued) { + this.batchTerminalWrite(data); + } + } return true; }, diff --git a/test/terminal-buffer-flush.test.ts b/test/terminal-buffer-flush.test.ts new file mode 100644 index 00000000..54267beb --- /dev/null +++ b/test/terminal-buffer-flush.test.ts @@ -0,0 +1,172 @@ +/** + * @fileoverview Regression tests for the buffer-load flush path (COD-144). + * + * Bug: newly launched Shell sessions rendered BLANK until a tab-switch. The + * buffer-load path (`selectSession` → `_beginBufferLoad`/`_finishBufferLoad`) + * QUEUES live SSE terminal events while `_isLoadingBuffer` is true, then on + * completion DISCARDS the queue (`_loadBufferQueue = null`). That de-dup is + * correct for an established session (the fetched buffer already contains the + * queued output, so replaying it would duplicate Ink redraws). But for a + * brand-new shell the fetch resolves BEFORE the PTY emits its prompt — the + * fetched buffer is empty and the prompt arrives only as a queued event, which + * then gets discarded → blank terminal. + * + * Fix: `_finishBufferLoad(owner, { flushQueued })` REPLAYS the queued events + * through `batchTerminalWrite()` (after `_isLoadingBuffer` is cleared, so they + * write through normally) ONLY when the load painted nothing. The default path + * (no opts) still discards, preserving de-dup for established sessions. + * + * Loaded via `vm` with a stubbed context (no jsdom — jsdom is broken on this + * box; see connection-indicator.test.ts). We extract the REAL + * `_beginBufferLoad`/`_finishBufferLoad` mixin methods from terminal-ui.js by + * running it against a fake `CodemanApp` and capturing `CodemanApp.prototype`, + * then copy them onto a minimal stub whose `batchTerminalWrite` is a spy. This + * exercises the real flush/discard logic without a full xterm fake. + */ +import { readFileSync } from 'node:fs'; +import { performance } from 'node:perf_hooks'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +/** Run terminal-ui.js in a vm against a fake CodemanApp and return the captured prototype mixin. */ +function loadTerminalMixin(): Record { + const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + const FakeCodemanApp = function () {} as unknown as { prototype: Record }; + const context = vm.createContext({ + console, + performance, + setTimeout, + clearTimeout, + setInterval: vi.fn(), + clearInterval: vi.fn(), + requestAnimationFrame: vi.fn(), + CodemanApp: FakeCodemanApp, + // terminal-ui.js IIFE is invoked with `window`; it reads/writes a few globals. + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + document: { addEventListener: vi.fn() }, + }); + vm.runInContext(source, context); + return FakeCodemanApp.prototype; +} + +const mixin = loadTerminalMixin(); + +type BufferLoadApp = { + _bufferLoadSeq: number; + _bufferLoadOwner: string | null; + _isLoadingBuffer: boolean; + _loadBufferQueue: string[] | null; + batchTerminalWrite: (data: string) => void; + _beginBufferLoad: (owner?: string) => string; + _finishBufferLoad: (owner?: string, opts?: { flushQueued?: boolean }) => boolean; +}; + +/** + * Minimal stub carrying the buffer-load state plus the REAL begin/finish methods. + * `batchTerminalWrite` is a spy so flushed events are observable without a real + * xterm terminal. The real `batchTerminalWrite` would queue while loading, but + * the flush runs AFTER `_isLoadingBuffer` is cleared, so a spy is faithful here. + */ +function makeApp() { + const writes: string[] = []; + const app: BufferLoadApp = { + _bufferLoadSeq: 0, + _bufferLoadOwner: null, + _isLoadingBuffer: false, + _loadBufferQueue: null, + batchTerminalWrite: vi.fn((data: string) => { + writes.push(data); + }), + _beginBufferLoad: mixin._beginBufferLoad as BufferLoadApp['_beginBufferLoad'], + _finishBufferLoad: mixin._finishBufferLoad as BufferLoadApp['_finishBufferLoad'], + }; + 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); +} + +describe('buffer-load flush (COD-144)', () => { + it('finish WITHOUT flushQueued discards the queue (de-dup preserved for established sessions)', () => { + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-1'); + pushWhileLoading(app, 'chunk-a'); + pushWhileLoading(app, 'chunk-b'); + + const ok = app._finishBufferLoad(owner); // default: discard + expect(ok).toBe(true); + expect(app._isLoadingBuffer).toBe(false); + expect(app._loadBufferQueue).toBeNull(); + // Queued events were NOT replayed. + expect(app.batchTerminalWrite).not.toHaveBeenCalled(); + expect(writes).toEqual([]); + }); + + it('finish WITH { flushQueued: true } replays queued events in order, exactly once each', () => { + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-2'); + pushWhileLoading(app, 'prompt-1'); + pushWhileLoading(app, 'prompt-2'); + + const ok = app._finishBufferLoad(owner, { flushQueued: true }); + expect(ok).toBe(true); + expect(app._isLoadingBuffer).toBe(false); + expect(app._loadBufferQueue).toBeNull(); + // Both chunks replayed, IN ORDER, exactly once each. + expect(writes).toEqual(['prompt-1', 'prompt-2']); + expect(app.batchTerminalWrite).toHaveBeenCalledTimes(2); + expect(app.batchTerminalWrite).toHaveBeenNthCalledWith(1, 'prompt-1'); + expect(app.batchTerminalWrite).toHaveBeenNthCalledWith(2, 'prompt-2'); + }); + + it('flushed events are not re-queued (the queue is null when batchTerminalWrite runs)', () => { + const { app } = makeApp(); + const owner = app._beginBufferLoad('load-3'); + pushWhileLoading(app, 'only'); + + // Spy that, like the real method, would re-queue if loading were still active. + let reQueued = false; + app.batchTerminalWrite = vi.fn((data: string) => { + if (app._isLoadingBuffer && app._loadBufferQueue) { + app._loadBufferQueue.push(data); + reQueued = true; + } + }); + + app._finishBufferLoad(owner, { flushQueued: true }); + expect(reQueued).toBe(false); + expect(app._isLoadingBuffer).toBe(false); + expect(app._loadBufferQueue).toBeNull(); + }); + + it('owner mismatch returns false and does NOT flush or clear state', () => { + const { app, writes } = makeApp(); + app._beginBufferLoad('real-owner'); + pushWhileLoading(app, 'queued'); + + const ok = app._finishBufferLoad('wrong-owner', { flushQueued: true }); + expect(ok).toBe(false); + // 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.batchTerminalWrite).not.toHaveBeenCalled(); + expect(writes).toEqual([]); + }); + + it('empty queue + flushQueued is a no-op (no throw, no writes)', () => { + const { app, writes } = makeApp(); + const owner = app._beginBufferLoad('load-empty'); + // No events queued. + + expect(() => app._finishBufferLoad(owner, { flushQueued: true })).not.toThrow(); + expect(app._isLoadingBuffer).toBe(false); + expect(app._loadBufferQueue).toBeNull(); + expect(app.batchTerminalWrite).not.toHaveBeenCalled(); + expect(writes).toEqual([]); + }); +});