mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 10:59:32 +01:00
fix(terminal): load tmux history instead of paging Claude after a hollow tab switch
After a server restart the byte recording a tab switch replays (`?tail=`) holds only the tmux client's attach repaint, so an inline Claude session came back with an empty local buffer while tmux still held its scrollback. The hollow-buffer fallback then sent PageUp/PageDown to Claude, which moved the composer cursor instead of the transcript, until a page reload. Pre-existing on an unmodified build; every restart of the server showed it. The paging fallback now stands down when the last capture reported tmux scrollback (`paneHistoryLines` > 0), dropping the gesture debt and any page keys already queued, so the gesture falls through to the local path, whose scroll-to-top pull fetches `full=1` and the gesture scrolls on from there. Zero and unknown counts keep paging (fullscreen Claude, codex, opencode, remote and Docker panes). The pull no longer inherits a downgrade refusal's 60 s latch for such a buffer, drops a completion that outlived a tab switch back to the same session (select generation, not only the session id), records no count from an error envelope (a JSON 5xx would otherwise forget it and hand the next gesture back to paging), and re-positions the local-echo overlay after its rewrite. The refresh path records its count only once it still owns the terminal, for the same reason. The `[scroll]` line prints `paneHistoryLines` so an unloaded buffer reads apart from a hollow one, and the invariant docs record the exception. Measured with phone emulation after a restart: before, the second visit landed at baseY 0 and every swipe sent PageUp; after, the first swipe fetched the history once with nothing sent to Claude and the gesture scrolled 21 rows, with an unsent draft intact on the overlay.
This commit is contained in:
1 parent
002de32a08
commit
84adb701d7
9 files changed
+301
-16
No files matched your search
@@ -180,3 +180,101 @@ describe('tab switch to a pane that keeps no scrollback', () => {
|
||||
await context.close();
|
||||
}, 90_000);
|
||||
});
|
||||
|
||||
describe('a tab switch whose tail replays no scrollback while the pane holds some', () => {
|
||||
it('scrolls by pulling the full history instead of paging the CLI', async () => {
|
||||
// After a server restart the byte recording is only the tmux client's attach
|
||||
// repaint (home + clear + absolute redraws), so the tail a second select
|
||||
// replays leaves baseY 0 while the response still reports tmux scrollback.
|
||||
// The scroll used to page the CLI (PageUp into an inline Claude's composer);
|
||||
// it now falls through to the local path, whose top-of-buffer pull fetches
|
||||
// `full=1` and the gesture then scrolls real history.
|
||||
const context = await browser.newContext({ viewport: { width: 1280, height: 800 }, deviceScaleFactor: 1 });
|
||||
const page = await context.newPage();
|
||||
await openApp(page);
|
||||
const inline = await createSession(page, 'inline-claude-after-restart');
|
||||
const other = await createSession(page, 'other');
|
||||
const log: string[] = [];
|
||||
const history = Array.from({ length: 200 }, (_, i) => `history line ${i}\r\n`).join('');
|
||||
await page.route('**/api/sessions/*/terminal*', async (route) => {
|
||||
const url = new URL(route.request().url());
|
||||
const id = url.pathname.split('/')[3];
|
||||
const full = url.searchParams.get('full') === '1';
|
||||
log.push(`${id}:${full ? 'full' : 'tail'}`);
|
||||
await route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({
|
||||
success: true,
|
||||
data: {
|
||||
terminalBuffer: full ? `${history}frame of ${id}\r\n` : `\x1b[H\x1b[2Jframe of ${id}\r\n`,
|
||||
status: 'idle',
|
||||
fullSize: 4096,
|
||||
retainedBytes: 4096,
|
||||
truncated: false,
|
||||
truncationReason: null,
|
||||
source: full ? 'mux-full-history' : 'mux-visible',
|
||||
paneHistoryLines: 200,
|
||||
},
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
await select(page, inline);
|
||||
await select(page, other);
|
||||
await select(page, inline);
|
||||
expect(log).toEqual([`${inline}:full`, `${other}:full`, `${inline}:tail`]);
|
||||
const baseYAfterTail = await page.evaluate(
|
||||
() => (window as unknown as { app: { terminal: any } }).app.terminal.buffer.active.baseY
|
||||
);
|
||||
expect(baseYAfterTail).toBe(0);
|
||||
// The prerequisite the guard answers: by the old rule this pane IS hollow.
|
||||
expect(await page.evaluate(() => (window as unknown as { app: any }).app._localScrollbackIsHollow())).toBe(true);
|
||||
|
||||
// Record what the scroll sends to the CLI, then wheel up over the terminal.
|
||||
await page.evaluate(() => {
|
||||
const app = (window as unknown as { app: any }).app;
|
||||
app.__sentToCli = [];
|
||||
app._sendInputEphemeral = (_id: string, data: string) => app.__sentToCli.push(data);
|
||||
});
|
||||
log.length = 0;
|
||||
const box = await page.locator('#terminalContainer').boundingBox();
|
||||
if (!box) throw new Error('terminal container not laid out');
|
||||
await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
|
||||
await page.mouse.wheel(0, -400);
|
||||
await page.waitForFunction(
|
||||
() => (window as unknown as { app: { terminal: any } }).app.terminal.buffer.active.baseY > 0,
|
||||
null,
|
||||
{ timeout: 10_000 }
|
||||
);
|
||||
expect(log).toEqual([`${inline}:full`]);
|
||||
|
||||
// The next gesture scrolls the recovered history itself: older text comes
|
||||
// into view, nothing further is fetched, and the CLI still receives nothing
|
||||
// (the page-key queue flushes 40 ms after a gesture, so settle first).
|
||||
await page.mouse.wheel(0, -400);
|
||||
await page.waitForFunction(
|
||||
() => {
|
||||
const b = (window as unknown as { app: { terminal: any } }).app.terminal.buffer.active;
|
||||
return b.viewportY < b.baseY;
|
||||
},
|
||||
null,
|
||||
{ timeout: 10_000 }
|
||||
);
|
||||
await page.waitForTimeout(200);
|
||||
const view = await page.evaluate(() => {
|
||||
const t = (window as unknown as { app: { terminal: any } }).app.terminal;
|
||||
const b = t.buffer.active;
|
||||
const lines: string[] = [];
|
||||
for (let i = b.viewportY; i < b.viewportY + t.rows; i++) lines.push(b.getLine(i)?.translateToString(true) ?? '');
|
||||
return lines.join('\n');
|
||||
});
|
||||
expect(view).toContain('history line ');
|
||||
expect(log).toEqual([`${inline}:full`]);
|
||||
expect(await page.evaluate(() => (window as unknown as { app: any }).app.__sentToCli)).toEqual([]);
|
||||
|
||||
await deleteSession(page, inline);
|
||||
await deleteSession(page, other);
|
||||
await context.close();
|
||||
}, 90_000);
|
||||
});
|
||||
@@ -77,4 +77,18 @@ describe('every terminal response feeds it (static guard)', () => {
|
||||
expect(methodSource(APP, '_onSessionNeedsRefresh')).toContain('this._notePaneHistory?.(sessionId, data);');
|
||||
expect(methodSource(APP, '_maybeRefetchFullHistory')).toContain('this._notePaneHistory?.(sessionId, payload);');
|
||||
});
|
||||
|
||||
it('the refresh records the pane only once it still owns the terminal, and neither it nor the pull records an error envelope', () => {
|
||||
// A refresh that lost its owner is older than what the switch back read; an
|
||||
// error envelope has no count, and forgetting on it would hand a scroll back
|
||||
// to the paging fallback (see _maybePageCliTranscript).
|
||||
const refresh = methodSource(APP, '_onSessionNeedsRefresh');
|
||||
const ownerCheck = refresh.indexOf('this._terminalRefreshOwner !== refreshOwner) return false;');
|
||||
expect(ownerCheck).toBeGreaterThan(-1);
|
||||
expect(refresh.indexOf('this._notePaneHistory?.(sessionId, data);')).toBeGreaterThan(ownerCheck);
|
||||
expect(refresh).toContain('capture.json?.success !== false) this._notePaneHistory?.(sessionId, data);');
|
||||
expect(methodSource(APP, '_maybeRefetchFullHistory')).toContain(
|
||||
'capture.json?.success !== false) this._notePaneHistory?.(sessionId, payload);'
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -24,6 +24,10 @@
|
||||
* the real `_setHistoryTruncation` and the real `computeHistoryTruncationNotice`
|
||||
* to pin what the user is actually told.
|
||||
*
|
||||
* The last block pins the pull's own tab-switch guard: a capture that lands
|
||||
* after A -> B -> A is dropped by the select generation, since the session id
|
||||
* alone matches again while the switch back is still replaying.
|
||||
*
|
||||
* The method is extracted from app.js and run in a `vm` against stubs (no jsdom
|
||||
* on this box; see connection-indicator.test.ts), with the REAL row estimators
|
||||
* from terminal-ui.js, which decide both the downgrade and the no-gain skip.
|
||||
@@ -118,17 +122,19 @@ function makeApp(
|
||||
activeSessionId: 's1',
|
||||
sessions: new Map([['s1', { mode }]]),
|
||||
detachedSessions: new Set<string>(),
|
||||
_selectGeneration: 7,
|
||||
_fullHistoryRepullInFlight: false,
|
||||
_isLoadingBuffer: false,
|
||||
_fullHistoryRepullAt: new Map<string, number>(),
|
||||
_fullHistoryRepullUseless: new Set<string>(),
|
||||
_paneHistoryLines: new Map<string, number>(),
|
||||
terminalBufferCache: new Map<string, string>(),
|
||||
terminal: {
|
||||
cols: 80,
|
||||
rows: 30,
|
||||
// xterm's scrollback option; 0 leaves the browser-cap check out of a test.
|
||||
options: { scrollback },
|
||||
buffer: { active: { length: bufferRows } },
|
||||
buffer: { active: { length: bufferRows, baseY: 0 } },
|
||||
scrollToLine: vi.fn(),
|
||||
scrollToTop: vi.fn(),
|
||||
},
|
||||
@@ -144,6 +150,7 @@ function makeApp(
|
||||
}),
|
||||
_recordTerminalLoadTiming: vi.fn(),
|
||||
_logScrollRouting: vi.fn(),
|
||||
_notePaneHistory: vi.fn(),
|
||||
// The real method behind a spy, so a test sees both what it was called with
|
||||
// and the banner state (`_historyTruncation`) it leaves behind.
|
||||
_historyTruncation: new Map<string, unknown>(),
|
||||
@@ -358,3 +365,77 @@ describe('_replayWouldShrinkBuffer takes rows the caller already estimated', ()
|
||||
expect(ctx._estimateReplayRows).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('a pull that outlives a tab switch never rewrites the terminal', () => {
|
||||
it('drops the capture when the select generation moved during the fetch, even back to the same session', async () => {
|
||||
// A -> B -> A while the request was out: the active id matches again, but
|
||||
// selectSession is replaying its own capture, and a reset here would land
|
||||
// in the middle of that write.
|
||||
const { app } = makeApp('claude', { bufferRows: 40, capture: lines(300) });
|
||||
app._fetchTerminalCapture.mockImplementationOnce(async () => {
|
||||
app._selectGeneration = 9;
|
||||
return {
|
||||
headersAt: performance.now(),
|
||||
headers: { get: () => '' },
|
||||
json: { data: { terminalBuffer: lines(300), source: 'mux-full-history' } },
|
||||
};
|
||||
});
|
||||
await refetch.call(app);
|
||||
expect(app._resetTerminalForReplay).not.toHaveBeenCalled();
|
||||
expect(app.chunkedTerminalWrite).not.toHaveBeenCalled();
|
||||
expect(app._fullHistoryRepullInFlight).toBe(false);
|
||||
// Nor does the stale response overwrite what the switch back recorded about the pane.
|
||||
expect(app._notePaneHistory).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('replays when no switch happened, and records the pane', async () => {
|
||||
const { app } = makeApp('claude', { bufferRows: 40, capture: lines(300) });
|
||||
await refetch.call(app);
|
||||
expect(app._resetTerminalForReplay).toHaveBeenCalledTimes(1);
|
||||
expect(app.chunkedTerminalWrite).toHaveBeenCalledTimes(1);
|
||||
expect(app._notePaneHistory).toHaveBeenCalledWith('s1', expect.objectContaining({ source: 'mux-full-history' }));
|
||||
});
|
||||
});
|
||||
|
||||
describe('an unloaded buffer is not held to an older downgrade refusal', () => {
|
||||
it('pulls on the normal cooldown when the buffer is one screen and the pane holds scrollback', async () => {
|
||||
// A refused pull latches a 60 s cooldown because the browser then held MORE
|
||||
// than tmux. A tab switch that replays a hollow tail voids that premise, and
|
||||
// with paging suppressed for such a buffer the latch would leave the gesture
|
||||
// dead for up to a minute.
|
||||
const { app, urls } = makeApp('claude', { bufferRows: 30, capture: lines(300) });
|
||||
app._fullHistoryRepullUseless.add('s1');
|
||||
app._fullHistoryRepullAt.set('s1', Date.now() - 10_000);
|
||||
app.terminal.buffer.active.baseY = 0;
|
||||
app._paneHistoryLines.set('s1', 11_344);
|
||||
await refetch.call(app);
|
||||
expect(urls).toEqual(['/api/sessions/s1/terminal?full=1']);
|
||||
expect(app._resetTerminalForReplay).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('keeps the latch while the buffer still holds the history the refusal was about', async () => {
|
||||
const { app, urls } = makeApp('claude', { bufferRows: 300, capture: lines(300) });
|
||||
app._fullHistoryRepullUseless.add('s1');
|
||||
app._fullHistoryRepullAt.set('s1', Date.now() - 10_000);
|
||||
app.terminal.buffer.active.baseY = 270;
|
||||
app._paneHistoryLines.set('s1', 11_344);
|
||||
await refetch.call(app);
|
||||
expect(urls).toEqual([]);
|
||||
});
|
||||
|
||||
it('records nothing from an error envelope, so the remembered count survives for the retry', async () => {
|
||||
// A JSON 5xx parses like any response. Recording its empty data would
|
||||
// forget the count and hand the next gesture back to the paging fallback.
|
||||
const { app } = makeApp('claude', { bufferRows: 30, capture: lines(300) });
|
||||
app._paneHistoryLines.set('s1', 11_344);
|
||||
app._fetchTerminalCapture.mockImplementationOnce(async () => ({
|
||||
headersAt: performance.now(),
|
||||
headers: { get: () => '' },
|
||||
json: { success: false, error: 'tmux capture failed' },
|
||||
}));
|
||||
await refetch.call(app);
|
||||
expect(app._notePaneHistory).not.toHaveBeenCalled();
|
||||
expect(app._resetTerminalForReplay).not.toHaveBeenCalled();
|
||||
expect(app._fullHistoryRepullInFlight).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -394,6 +394,42 @@ describe('PageUp/PageDown fallback for a hollow local buffer (issue #205 round 2
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, -18)).toBe(false);
|
||||
});
|
||||
|
||||
it('falls through to local scrolling when tmux still holds the scrollback the replay left out', () => {
|
||||
// After a server restart a tab switch replays a byte tail that is only the
|
||||
// tmux client's attach repaint: baseY 0, while the same response reported
|
||||
// tmux scrollback (`paneHistoryLines`). Paging there sent PageUp into an
|
||||
// inline Claude's composer; the local path's scroll-to-top pull
|
||||
// (_maybeRefetchFullHistory) is what loads that scrollback.
|
||||
const { app, sent, logs } = hollowApp();
|
||||
app._pageKeyGesture = { stale: true };
|
||||
// A page key queued before the count was known must not reach the CLI either.
|
||||
app._wheelSgrQueue = '\x1b[5~';
|
||||
app._paneHistoryLines = new Map([['sess-1', 64]]);
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, -18)).toBe(false);
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, 18)).toBe(false);
|
||||
app._flushWheelSgrQueue();
|
||||
expect(sent).toEqual([]);
|
||||
expect(app._pageKeyGesture).toBeNull();
|
||||
// No routing line of its own: the caller's `local-scrollback` line (with
|
||||
// `localScrollbackRows=0`) is the record, and a second line per frame
|
||||
// would alternate with it until the pull lands.
|
||||
expect(logs).toEqual([]);
|
||||
|
||||
// 0 is a pane that keeps no scrollback (fullscreen claude, codex, opencode):
|
||||
// paged as before, and so is unknown (no capture yet, an older server).
|
||||
app._paneHistoryLines.set('sess-1', 0);
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, -18)).toBe(true);
|
||||
app._paneHistoryLines.delete('sess-1');
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, -18)).toBe(true);
|
||||
delete app._paneHistoryLines;
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, -18)).toBe(true);
|
||||
|
||||
// An open dialog still wins: nothing scrolls under it and no pull starts.
|
||||
app._paneHistoryLines = new Map([['sess-1', 64]]);
|
||||
app.tabAlerts = new Map([['sess-1', 'action']]);
|
||||
expect(app._maybePageCliTranscript({ shiftKey: false }, -18)).toBe(true);
|
||||
});
|
||||
|
||||
it('pages a Codex transcript only while its local scrollback is empty', () => {
|
||||
// Codex without observed mouse tracking retains its PageUp/PageDown fallback.
|
||||
const { app, sent } = hollowApp();
|
||||
|
||||
Reference in new issue
Block a user