mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 16:39:42 +02:00
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).
This commit is contained in:
+10
-4
@@ -3733,6 +3733,9 @@ class CodemanApp {
|
|||||||
// the buffer write, causing 70KB+ single-frame flushes that stall WebGL.
|
// the buffer write, causing 70KB+ single-frame flushes that stall WebGL.
|
||||||
// chunkedTerminalWrite also sets this, but we need it before the fetch too.
|
// chunkedTerminalWrite also sets this, but we need it before the fetch too.
|
||||||
const bufferLoadOwner = this._beginBufferLoad(selectGen);
|
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 {
|
try {
|
||||||
// Fit terminal to container BEFORE writing any buffer data.
|
// Fit terminal to container BEFORE writing any buffer data.
|
||||||
// If the browser was resized while viewing another session, the terminal
|
// If the browser was resized while viewing another session, the terminal
|
||||||
@@ -3889,13 +3892,16 @@ class CodemanApp {
|
|||||||
} else if (!cachedBuffer) {
|
} else if (!cachedBuffer) {
|
||||||
// No fresh buffer and no cache — clear any stale content
|
// No fresh buffer and no cache — clear any stale content
|
||||||
this._resetTerminalForReplay();
|
this._resetTerminalForReplay();
|
||||||
|
bufferWasEmpty = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Buffer load complete — unblock live SSE writes (queued events are discarded
|
// Buffer load complete — unblock live SSE writes. chunkedTerminalWrite calls
|
||||||
// to prevent duplicate content). chunkedTerminalWrite calls _finishBufferLoad
|
// _finishBufferLoad internally (discarding queued events to prevent duplicate
|
||||||
// internally, but if we skipped the write (cache hit or empty), call it here.
|
// 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) {
|
if (this._isLoadingBuffer) {
|
||||||
this._finishBufferLoad(bufferLoadOwner);
|
this._finishBufferLoad(bufferLoadOwner, { flushQueued: bufferWasEmpty });
|
||||||
}
|
}
|
||||||
// Drop the guard so user input clears state normally
|
// Drop the guard so user input clears state normally
|
||||||
this._restoringFlushedState = false;
|
this._restoringFlushedState = false;
|
||||||
|
|||||||
@@ -1925,11 +1925,25 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
* Complete a buffer load: unblock live SSE writes.
|
* Complete a buffer load: unblock live SSE writes.
|
||||||
* Called when chunkedTerminalWrite finishes (or is skipped for empty buffers).
|
* Called when chunkedTerminalWrite finishes (or is skipped for empty buffers).
|
||||||
*
|
*
|
||||||
* Queued SSE events are DISCARDED, not flushed. The loaded buffer from the API
|
* By default queued SSE events are DISCARDED, not flushed. For an established
|
||||||
* is the source of truth up to the response timestamp. SSE events queued during
|
* session the loaded buffer from the API is the source of truth up to the
|
||||||
* the fetch+write overlap with the buffer — flushing them writes duplicate data
|
* response timestamp; SSE events queued during the fetch+write overlap already
|
||||||
* (especially Ink cursor-up redraws), corrupting the terminal display.
|
* 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.
|
* 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) {
|
_beginBufferLoad(owner) {
|
||||||
if (this._bufferLoadSeq === undefined) this._bufferLoadSeq = 0;
|
if (this._bufferLoadSeq === undefined) this._bufferLoadSeq = 0;
|
||||||
@@ -1940,13 +1954,21 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
return loadOwner;
|
return loadOwner;
|
||||||
},
|
},
|
||||||
|
|
||||||
_finishBufferLoad(owner) {
|
_finishBufferLoad(owner, opts) {
|
||||||
if (owner !== undefined && this._bufferLoadOwner !== owner) {
|
if (owner !== undefined && this._bufferLoadOwner !== owner) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
const queued = this._loadBufferQueue;
|
||||||
this._isLoadingBuffer = false;
|
this._isLoadingBuffer = false;
|
||||||
this._loadBufferQueue = null;
|
this._loadBufferQueue = null;
|
||||||
this._bufferLoadOwner = 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;
|
return true;
|
||||||
},
|
},
|
||||||
|
|
||||||
|
|||||||
@@ -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<string, unknown> {
|
||||||
|
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8');
|
||||||
|
const FakeCodemanApp = function () {} as unknown as { prototype: Record<string, unknown> };
|
||||||
|
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([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user