mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 15:39:41 +02:00
fix(review): wire full-scrollback replay to an explicit ?full=1, dedup + bound the capture (PR #148)
- Replace the 'missing ?tail means reload' overload with an explicit ?full=1 query param: the frontend's first buffer load after a page load (selectSession) now requests full=1, tab switches keep ?tail=, and the legacy no-param callers (response-viewer fallback, clearTerminal refresh) keep the cheap visible-frame path — the COD-47 feature was previously unreachable from a real reload. - When the full-history capture succeeds, return it ALONE instead of prepending the byte buffer + \x1b[H\x1b[2J: the capture is the rendered superset of the byte history, and ED2 clears only the viewport so the concat replayed the whole conversation twice in xterm scrollback. The history+clear+frame concat stays for the visible-frame/tab-switch path. - Pass an explicit execSync maxBuffer for the full-history capture (configured terminalBufferMaxBytes + slack) — the 1MB Node default ENOBUFS-killed exactly the multi-MB captures the feature exists for; log ENOBUFS concisely instead of dumping the truncated stdout. - Bound the capture itself via -S -<N> derived from the configured tmux history limit (was unbounded -S -), and add -J so lines hard-wrapped at the capture-time pane width reflow in the browser xterm. - Cap the concatenated buffer to terminalBufferMaxBytes EARLY (before the regex normalization passes) so multi-MB captures don't stall the event loop normalizing bytes that get sliced away. - Tests: route tests updated for ?full=1 semantics (capture-alone response, config-forwarded capture bounds, byte-history fallback, no-param requests stay on the visible-frame path); source-scan tests cover the bounded -J -S -<N> flags and explicit maxBuffer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -521,13 +521,12 @@ describe('session-routes', () => {
|
||||
expect(body.data.terminalBuffer.indexOf('hello world')).toBeLessThan(
|
||||
body.data.terminalBuffer.indexOf('visible tmux pane only')
|
||||
);
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, {
|
||||
fullHistory: true,
|
||||
});
|
||||
// No ?full=1 → visible-frame capture (no fullHistory opts).
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, undefined);
|
||||
});
|
||||
|
||||
// ── COD-47: full tmux scrollback replay on full page reload ──
|
||||
it('full reload (no tail) requests full tmux history and replays boundary markers', async () => {
|
||||
it('full reload (?full=1) requests full tmux history and replays boundary markers', async () => {
|
||||
// A realistic scrollback-length capture: ~5000 lines, well past one screen.
|
||||
const firstLine = 'SCROLLBACK_FIRST_LINE_0001';
|
||||
const lastLine = 'SCROLLBACK_LAST_LINE_5000';
|
||||
@@ -547,13 +546,21 @@ describe('session-routes', () => {
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal`,
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
// Full reload asked tmux for the entire scrollback.
|
||||
expect(captureSpy).toHaveBeenCalledWith(harness.ctx._session.muxName, { fullHistory: true });
|
||||
// Full reload asked tmux for the entire scrollback, with the configured
|
||||
// capture bounds (history-line limit + byte cap for exec maxBuffer).
|
||||
expect(captureSpy).toHaveBeenCalledWith(
|
||||
harness.ctx._session.muxName,
|
||||
expect.objectContaining({
|
||||
fullHistory: true,
|
||||
historyLimitLines: expect.any(Number),
|
||||
maxCaptureBytes: expect.any(Number),
|
||||
})
|
||||
);
|
||||
// Both boundary markers survived the capture → route pipeline.
|
||||
expect(body.data.terminalBuffer).toContain(firstLine);
|
||||
expect(body.data.terminalBuffer).toContain(lastLine);
|
||||
@@ -561,6 +568,74 @@ describe('session-routes', () => {
|
||||
expect(typeof body.data.fullSize).toBe('number');
|
||||
});
|
||||
|
||||
it('full reload (?full=1) returns the tmux capture ALONE — byte history is not duplicated', async () => {
|
||||
// The full-history capture is the rendered form of everything already in
|
||||
// the byte buffer; prepending the byte history would replay the whole
|
||||
// conversation twice (\x1b[2J clears the viewport, not xterm scrollback).
|
||||
harness.ctx._session.mode = 'claude';
|
||||
harness.ctx._session.terminalBuffer = 'BYTE_BUFFER_COPY of the conversation';
|
||||
const rendered = 'BYTE_BUFFER_COPY of the conversation\r\nplus older scrollback\r\n› prompt';
|
||||
const captureSpy = vi.fn((_name: string, opts?: { fullHistory?: boolean }) =>
|
||||
opts?.fullHistory ? rendered : 'visible frame only'
|
||||
);
|
||||
(harness.ctx.mux as { captureActivePaneBuffer?: unknown }).captureActivePaneBuffer = captureSpy;
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.data.source).toBe('mux-full-history');
|
||||
expect(body.data.terminalBuffer).toContain('plus older scrollback');
|
||||
// Capture alone: no history+clear-viewport concat, and the byte-buffer
|
||||
// content appears exactly once (from the capture, not a duplicate prepend).
|
||||
expect(body.data.terminalBuffer).not.toContain('\x1b[H\x1b[2J');
|
||||
expect(body.data.terminalBuffer.indexOf('BYTE_BUFFER_COPY')).toBe(
|
||||
body.data.terminalBuffer.lastIndexOf('BYTE_BUFFER_COPY')
|
||||
);
|
||||
});
|
||||
|
||||
it('full reload (?full=1) falls back to the byte history when the capture is unavailable', async () => {
|
||||
harness.ctx._session.mode = 'claude';
|
||||
harness.ctx._session.terminalBuffer = 'byte history survives';
|
||||
(harness.ctx.mux as { captureActivePaneBuffer?: unknown }).captureActivePaneBuffer = vi.fn(() => null);
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.data.source).toBe('history');
|
||||
expect(body.data.terminalBuffer).toContain('byte history survives');
|
||||
});
|
||||
|
||||
it('full reload forwards the configured history-line limit and byte cap to the capture', async () => {
|
||||
harness.ctx.getTerminalHistoryConfig = vi.fn(async () => ({
|
||||
terminalScrollbackLines: 60_000,
|
||||
tmuxHistoryLimit: 123_456,
|
||||
terminalBufferMaxBytes: 5 * 1024 * 1024,
|
||||
terminalBufferTrimBytes: 4 * 1024 * 1024,
|
||||
}));
|
||||
const captureSpy = vi.fn(() => 'full scrollback');
|
||||
(harness.ctx.mux as { captureActivePaneBuffer?: unknown }).captureActivePaneBuffer = captureSpy;
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(captureSpy).toHaveBeenCalledWith(harness.ctx._session.muxName, {
|
||||
fullHistory: true,
|
||||
historyLimitLines: 123_456,
|
||||
maxCaptureBytes: 5 * 1024 * 1024,
|
||||
});
|
||||
});
|
||||
|
||||
it('tab switch (with tail) uses the visible frame, not full history', async () => {
|
||||
harness.ctx._session.mode = 'codex';
|
||||
harness.ctx._session.terminalBuffer = 'accumulated history';
|
||||
@@ -603,7 +678,7 @@ describe('session-routes', () => {
|
||||
|
||||
const res = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal`,
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/terminal?full=1`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
@@ -637,9 +712,7 @@ describe('session-routes', () => {
|
||||
expect(body.data.terminalBuffer.indexOf('hello world')).toBeLessThan(
|
||||
body.data.terminalBuffer.indexOf('visible tmux pane only')
|
||||
);
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, {
|
||||
fullHistory: true,
|
||||
});
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, undefined);
|
||||
});
|
||||
|
||||
it('preserves one-time OAuth authorization URLs in Codex TUI replay history', async () => {
|
||||
@@ -713,9 +786,7 @@ describe('session-routes', () => {
|
||||
expect(body.data.terminalBuffer.indexOf('hello world')).toBeLessThan(
|
||||
body.data.terminalBuffer.indexOf('visible tmux pane only')
|
||||
);
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, {
|
||||
fullHistory: true,
|
||||
});
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, undefined);
|
||||
});
|
||||
|
||||
it('uses live mux pane capture only when the accumulated buffer is empty', async () => {
|
||||
@@ -734,9 +805,7 @@ describe('session-routes', () => {
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.data.terminalBuffer).toContain('visible restored tmux pane');
|
||||
expect(body.data.terminalBuffer).toContain('› current prompt');
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, {
|
||||
fullHistory: true,
|
||||
});
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, undefined);
|
||||
});
|
||||
|
||||
it('returns error for unknown session', async () => {
|
||||
@@ -764,9 +833,7 @@ describe('session-routes', () => {
|
||||
expect(buf).toContain('\x1b[H\x1b[2J');
|
||||
expect(buf).toContain('LIVE-PANE-FRAME');
|
||||
expect(buf.indexOf('history-bytes')).toBeLessThan(buf.indexOf('LIVE-PANE-FRAME'));
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, {
|
||||
fullHistory: true,
|
||||
});
|
||||
expect(harness.ctx.mux.captureActivePaneBuffer).toHaveBeenCalledWith(harness.ctx._session.muxName, undefined);
|
||||
});
|
||||
|
||||
it('falls back to the byte history when no live pane buffer is available', async () => {
|
||||
|
||||
@@ -4,7 +4,8 @@
|
||||
* Under VITEST, TmuxManager no-ops execSync (IS_TEST_MODE), so we can't drive
|
||||
* real tmux. Instead we assert the capture-arg construction directly from
|
||||
* source (same approach as tmux-capture-color.test.ts): a full-history capture
|
||||
* must use `capture-pane -p -e -S -` and skip the single-screen snapshot
|
||||
* must use `capture-pane -p -e -J -S -<N>` (bounded to the configured history
|
||||
* limit, with an explicit exec maxBuffer) and skip the single-screen snapshot
|
||||
* repaint, while the visible capture keeps `capture-pane -p -e`.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs';
|
||||
@@ -13,16 +14,27 @@ import { describe, expect, it } from 'vitest';
|
||||
|
||||
describe('tmux full-history pane capture (COD-47)', () => {
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/tmux-manager.ts'), 'utf8');
|
||||
const methodStart = source.indexOf('capturePaneBuffer(muxName: string');
|
||||
const methodBody = source.slice(methodStart, methodStart + 4000);
|
||||
|
||||
it('capturePaneBuffer accepts a fullHistory option', () => {
|
||||
const sig = source.indexOf('capturePaneBuffer(muxName: string');
|
||||
expect(sig).toBeGreaterThan(-1);
|
||||
// The method signature must carry the opts.fullHistory channel.
|
||||
expect(source.slice(sig, sig + 160)).toContain('fullHistory');
|
||||
it('capturePaneBuffer accepts pane-capture options with a fullHistory flag', () => {
|
||||
expect(methodStart).toBeGreaterThan(-1);
|
||||
// The method signature must carry the opts channel...
|
||||
expect(source.slice(methodStart, methodStart + 160)).toContain('PaneCaptureOptions');
|
||||
// ...and the body must branch on opts.fullHistory.
|
||||
expect(methodBody).toContain('opts?.fullHistory === true');
|
||||
});
|
||||
|
||||
it('full-history mode requests the entire scrollback with -S -', () => {
|
||||
expect(source).toContain('capture-pane -p -e -S -');
|
||||
it('full-history mode captures scrollback bounded to the configured history limit (-J -S -<N>)', () => {
|
||||
// `-S -<N>` (not unbounded `-S -`) keeps tmux from serializing more
|
||||
// scrollback than the configured history limit retains; `-J` re-joins
|
||||
// lines hard-wrapped at the capture-time pane width.
|
||||
expect(source).toContain('capture-pane -p -e -J -S -${historyLines}');
|
||||
});
|
||||
|
||||
it('full-history exec sets an explicit maxBuffer (default 1MB would ENOBUFS multi-MB dumps)', () => {
|
||||
expect(methodBody).toContain('maxBuffer');
|
||||
expect(methodBody).toContain('FULL_HISTORY_CAPTURE_SLACK_BYTES');
|
||||
});
|
||||
|
||||
it('still offers the visible single-screen capture for fast tab switches', () => {
|
||||
@@ -30,18 +42,16 @@ describe('tmux full-history pane capture (COD-47)', () => {
|
||||
});
|
||||
|
||||
it('returns full-history capture as raw scrollback (skips the single-screen repaint)', () => {
|
||||
const sig = source.indexOf('capturePaneBuffer(muxName: string');
|
||||
const body = source.slice(sig, sig + 2400);
|
||||
// When fullHistory, return the raw buffer BEFORE the formatPaneSnapshot
|
||||
// repaint (which is single-screen and would clip a multi-screen history).
|
||||
const earlyReturn = body.indexOf('if (fullHistory)');
|
||||
const snapshot = body.indexOf('formatPaneSnapshot(');
|
||||
const earlyReturn = methodBody.indexOf('return normalizeScrollbackEol(buffer);');
|
||||
const snapshot = methodBody.indexOf('formatPaneSnapshot(');
|
||||
expect(earlyReturn).toBeGreaterThan(-1);
|
||||
expect(snapshot).toBeGreaterThan(-1);
|
||||
expect(earlyReturn).toBeLessThan(snapshot);
|
||||
});
|
||||
|
||||
it('captureActivePaneBuffer forwards the fullHistory option', () => {
|
||||
it('captureActivePaneBuffer forwards the capture options', () => {
|
||||
const sig = source.indexOf('captureActivePaneBuffer(muxName: string');
|
||||
expect(sig).toBeGreaterThan(-1);
|
||||
const body = source.slice(sig, sig + 800);
|
||||
|
||||
Reference in New Issue
Block a user