mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(terminal): skip a bounded Shell window before the downgrade guard
A window cut at the tail size can be smaller than the browser's buffer while tmux still holds more. The downgrade guard reads that as "tmux has nothing more to give", which is true of an unbounded capture only, so a bounded window reaching it marked the session exhausted and removed Load full history from the banner. The bounded skip now runs first, so such a window never reaches the exhausted path, and it no longer writes banner state: relabelling it from the bounded payload would call a terminal holding all of a Load full history pull "the most recent 1 MiB". A skipped window that came back truncated cannot reach anything older than the browser shows, and every ask costs the server a synchronous capture-pane of the whole history (tail is applied after the capture), so it puts the session on the 60 s cooldown. An untruncated one keeps 4 s. _replayWouldShrinkBuffer takes optional pre-estimated rows so a megabyte capture is not scanned twice. CLAUDE.md's Full-scrollback replay entry no longer says Shell never pulls on ordinary scroll. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
9676e90133
commit
f6aa50239f
@@ -14,6 +14,14 @@
|
||||
* the button stays the unbounded path, and a window the browser already holds
|
||||
* in full is not rewritten.
|
||||
*
|
||||
* ORDER MATTERS: that skip must run BEFORE the downgrade guard. The guard reads
|
||||
* "smaller than the browser" as "tmux has nothing more to give", which is true of
|
||||
* an unbounded capture and false of a window cut at the tail size, so a bounded
|
||||
* window that reached it marked the session exhausted and took Load full history
|
||||
* off the banner while tmux still held the rest. The second block below drives
|
||||
* the real `_setHistoryTruncation` and the real `computeHistoryTruncationNotice`
|
||||
* to pin what the user is actually told.
|
||||
*
|
||||
* 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.
|
||||
@@ -61,11 +69,43 @@ function loadRefetch(): (this: unknown, opts?: { force?: boolean }) => Promise<v
|
||||
return vm.runInContext(`({ ${body} })._maybeRefetchFullHistory`, context);
|
||||
}
|
||||
|
||||
/** The REAL `_setHistoryTruncation`, so the banner state a pull leaves behind is what production would hold. */
|
||||
function loadSetHistoryTruncation(): (this: unknown, sessionId: string, payload?: Record<string, unknown>) => void {
|
||||
const app = readFileSync(resolve(PUBLIC, 'app.js'), 'utf8');
|
||||
const body = methodSource(app, '_setHistoryTruncation');
|
||||
return vm.runInContext(`({ ${body} })._setHistoryTruncation`, vm.createContext({}));
|
||||
}
|
||||
|
||||
/** The REAL banner decision from constants.js: what the user is told, and whether Load full history is offered. */
|
||||
function loadNotice() {
|
||||
const context = vm.createContext({ console, window: {}, document: {}, navigator: { userAgent: 'test' } });
|
||||
vm.runInContext(
|
||||
`${readFileSync(resolve(PUBLIC, 'constants.js'), 'utf8')}\n;globalThis.__notice = computeHistoryTruncationNotice;`,
|
||||
context,
|
||||
{ filename: 'constants.js' }
|
||||
);
|
||||
return (context as { __notice: (s: Record<string, unknown>) => { visible: boolean; canLoadMore: boolean } }).__notice;
|
||||
}
|
||||
|
||||
const mixin = loadTerminalMixin();
|
||||
const refetch = loadRefetch();
|
||||
const setHistoryTruncation = loadSetHistoryTruncation();
|
||||
const computeNotice = loadNotice();
|
||||
const lines = (n: number) => Array.from({ length: n }, (_, i) => `line ${i}`).join('\r\n');
|
||||
|
||||
function makeApp(mode: string, { bufferRows, capture }: { bufferRows: number; capture: string }) {
|
||||
/** A `?full=1&tail=` answer whose window was CUT at the tail size: tmux holds ~3 MiB, the window carries 1 MiB. */
|
||||
const TAIL_CUT = {
|
||||
truncated: true,
|
||||
truncationReason: 'tail',
|
||||
fullSize: 3 * 1024 * 1024,
|
||||
retainedBytes: TERMINAL_TAIL_SIZE,
|
||||
source: 'mux-full-history',
|
||||
};
|
||||
|
||||
function makeApp(
|
||||
mode: string,
|
||||
{ bufferRows, capture, payload = {} }: { bufferRows: number; capture: string; payload?: Record<string, unknown> }
|
||||
) {
|
||||
const urls: string[] = [];
|
||||
const app = {
|
||||
activeSessionId: 's1',
|
||||
@@ -90,12 +130,18 @@ function makeApp(mode: string, { bufferRows, capture }: { bufferRows: number; ca
|
||||
return {
|
||||
headersAt: performance.now(),
|
||||
headers: { get: () => '' },
|
||||
json: { data: { terminalBuffer: capture, source: 'mux-full-history' } },
|
||||
json: { data: { terminalBuffer: capture, source: 'mux-full-history', ...payload } },
|
||||
};
|
||||
}),
|
||||
_recordTerminalLoadTiming: vi.fn(),
|
||||
_logScrollRouting: vi.fn(),
|
||||
_setHistoryTruncation: 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>(),
|
||||
_renderHistoryTruncationBanner: vi.fn(),
|
||||
_setHistoryTruncation: vi.fn((sessionId: string, p?: Record<string, unknown>): void => {
|
||||
setHistoryTruncation.call(app, sessionId, p);
|
||||
}),
|
||||
_resetTerminalForReplay: vi.fn(),
|
||||
_bufferLoadFinishOpts: vi.fn(() => ({})),
|
||||
chunkedTerminalWrite: vi.fn(async () => ({ parsedAt: performance.now(), bufferLength: 400, completed: true })),
|
||||
@@ -135,8 +181,117 @@ describe('shell scroll-up pulls a bounded window of tmux history', () => {
|
||||
expect(app.chunkedTerminalWrite).not.toHaveBeenCalled();
|
||||
// Not latched as useless: more output can put more history in tmux.
|
||||
expect(app._fullHistoryRepullUseless.has('s1')).toBe(false);
|
||||
// The truncation state is still recorded, so a window capped at the tail
|
||||
// size keeps offering the button.
|
||||
expect(app._setHistoryTruncation).toHaveBeenCalledTimes(1);
|
||||
// Nothing was written, so the banner state is left exactly as it was.
|
||||
expect(app._setHistoryTruncation).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('a skipped bounded window never damages the Load full history banner', () => {
|
||||
it('a tail-cut window smaller than the browser is not replayed and never marks the session exhausted', async () => {
|
||||
// The browser holds far more rows than a 1 MiB window carries, and tmux holds
|
||||
// ~3 MiB. The downgrade guard reads that as "tmux has nothing more to give",
|
||||
// which is true of an unbounded capture and false of a window cut at the tail.
|
||||
const { app } = makeApp('shell', { bufferRows: 5000, capture: lines(300), payload: TAIL_CUT });
|
||||
// The tab load that put this session on screen left it truncated and recoverable.
|
||||
app._setHistoryTruncation('s1', TAIL_CUT);
|
||||
app._setHistoryTruncation.mockClear();
|
||||
|
||||
await refetch.call(app);
|
||||
|
||||
expect(app._resetTerminalForReplay).not.toHaveBeenCalled();
|
||||
expect(app.chunkedTerminalWrite).not.toHaveBeenCalled();
|
||||
// Not even a relabel: a skipped window writes nothing, banner state included.
|
||||
expect(app._setHistoryTruncation).not.toHaveBeenCalled();
|
||||
const notice = computeNotice(app._historyTruncation.get('s1') as Record<string, unknown>);
|
||||
expect(notice.visible).toBe(true);
|
||||
// "Earlier output is no longer kept" would be a lie: tmux still holds ~2 MiB more.
|
||||
expect(notice.canLoadMore).toBe(true);
|
||||
});
|
||||
|
||||
it('a skip right after Load full history leaves the banner as that load set it', async () => {
|
||||
// Load full history replayed everything, so nothing is truncated any more.
|
||||
const afterLoadFullHistory = {
|
||||
truncated: false,
|
||||
fullSize: 3 * 1024 * 1024,
|
||||
retainedBytes: 3 * 1024 * 1024,
|
||||
source: 'mux-full-history',
|
||||
};
|
||||
const { app } = makeApp('shell', { bufferRows: 5000, capture: lines(300), payload: TAIL_CUT });
|
||||
app._setHistoryTruncation('s1', afterLoadFullHistory);
|
||||
const before = structuredClone(app._historyTruncation.get('s1'));
|
||||
app._setHistoryTruncation.mockClear();
|
||||
|
||||
await refetch.call(app);
|
||||
|
||||
// Relabelling it from the bounded payload would call a terminal that holds ALL
|
||||
// of the history "the most recent 1.0 MB".
|
||||
expect(app._setHistoryTruncation).not.toHaveBeenCalled();
|
||||
expect(app._historyTruncation.get('s1')).toEqual(before);
|
||||
expect(computeNotice(app._historyTruncation.get('s1') as Record<string, unknown>).visible).toBe(false);
|
||||
});
|
||||
|
||||
it('backs off for a minute after a truncated skip, and keeps the 4 s cooldown after an untruncated one', async () => {
|
||||
// Truncated: the gesture cannot reach anything older than the browser shows, and
|
||||
// every ask costs the server a synchronous capture of the whole history.
|
||||
// Only just larger than the window, so the downgrade guard does not fire here:
|
||||
// the back-off has to come from the skip itself.
|
||||
const cut = makeApp('shell', { bufferRows: 320, capture: lines(300), payload: TAIL_CUT });
|
||||
await refetch.call(cut.app);
|
||||
expect(cut.app._fullHistoryRepullUseless.has('s1')).toBe(true);
|
||||
expect(cut.app._fetchTerminalCapture).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Well past 4 s, still inside the minute: no second capture.
|
||||
cut.app._fullHistoryRepullAt.set('s1', Date.now() - 10_000);
|
||||
await refetch.call(cut.app);
|
||||
expect(cut.app._fetchTerminalCapture).toHaveBeenCalledTimes(1);
|
||||
|
||||
cut.app._fullHistoryRepullAt.set('s1', Date.now() - 61_000);
|
||||
await refetch.call(cut.app);
|
||||
expect(cut.app._fetchTerminalCapture).toHaveBeenCalledTimes(2);
|
||||
|
||||
// Untruncated: it IS all of tmux's history, and the next burst can add to it.
|
||||
const whole = makeApp('shell', { bufferRows: 320, capture: lines(300) });
|
||||
await refetch.call(whole.app);
|
||||
expect(whole.app._fullHistoryRepullUseless.has('s1')).toBe(false);
|
||||
whole.app._fullHistoryRepullAt.set('s1', Date.now() - 5000);
|
||||
await refetch.call(whole.app);
|
||||
expect(whole.app._fetchTerminalCapture).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('the downgrade guard still refuses an unbounded capture smaller than the browser, and still marks it exhausted', async () => {
|
||||
// Reordering must not weaken the guard it moved above: a repaint-mode pane's
|
||||
// capture really is one frame, and rewriting with it would destroy history.
|
||||
const oneFrame = makeApp('claude', { bufferRows: 300, capture: lines(36) });
|
||||
await refetch.call(oneFrame.app);
|
||||
expect(oneFrame.app._resetTerminalForReplay).not.toHaveBeenCalled();
|
||||
expect(oneFrame.app._setHistoryTruncation).toHaveBeenCalledWith('s1', expect.objectContaining({ exhausted: true }));
|
||||
expect(oneFrame.app._fullHistoryRepullUseless.has('s1')).toBe(true);
|
||||
|
||||
// The button is unbounded too, so the same guard governs it for a shell.
|
||||
const button = makeApp('shell', { bufferRows: 5000, capture: lines(300) });
|
||||
await refetch.call(button.app, { force: true });
|
||||
expect(button.app._resetTerminalForReplay).not.toHaveBeenCalled();
|
||||
expect(button.app._setHistoryTruncation).toHaveBeenCalledWith('s1', expect.objectContaining({ exhausted: true }));
|
||||
});
|
||||
});
|
||||
|
||||
describe('_replayWouldShrinkBuffer takes rows the caller already estimated', () => {
|
||||
const shrink = mixin._replayWouldShrinkBuffer as (this: unknown, capture: string, rows?: number) => boolean;
|
||||
const make = () => ({
|
||||
terminal: { cols: 80, rows: 30, buffer: { active: { length: 200 } } },
|
||||
_estimateReplayRows: vi.fn(mixin._estimateReplayRows as (t: string, c: number) => number),
|
||||
});
|
||||
|
||||
it('does not scan the capture again when handed the estimate', () => {
|
||||
const ctx = make();
|
||||
expect(shrink.call(ctx, lines(300), 300)).toBe(false);
|
||||
expect(shrink.call(ctx, lines(300), 5)).toBe(true);
|
||||
expect(ctx._estimateReplayRows).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still estimates for itself when called the old way', () => {
|
||||
const ctx = make();
|
||||
expect(shrink.call(ctx, lines(300))).toBe(false);
|
||||
expect(ctx._estimateReplayRows).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -111,12 +111,20 @@ describe('full-history re-pull downgrade guard (issue #205 round 2)', () => {
|
||||
// Anchor on the open paren, not the full empty signature: the method takes
|
||||
// options since #258 ({ force }) and this guard is about ORDER, not arity.
|
||||
const start = source.indexOf('async _maybeRefetchFullHistory(');
|
||||
const guard = source.indexOf('this._replayWouldShrinkBuffer(buffer)', start);
|
||||
// Also anchored on the open paren: the guard is handed the rows the caller
|
||||
// already estimated, and this test is about ORDER, not the argument list.
|
||||
const guard = source.indexOf('this._replayWouldShrinkBuffer(buffer', start);
|
||||
const boundedSkip = source.indexOf('boundedShellPull && windowRows <=', start);
|
||||
const reset = source.indexOf('this._resetTerminalForReplay()', start);
|
||||
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
expect(guard).toBeGreaterThan(start);
|
||||
expect(guard).toBeLessThan(reset); // refuse first, only then reset+rewrite
|
||||
// A bounded shell window is skipped BEFORE the guard sees it: the guard reads
|
||||
// "smaller than the browser" as "tmux has nothing more", which a window cut at
|
||||
// the tail size does not mean (see shell-scroll-history-pull.test.ts).
|
||||
expect(boundedSkip).toBeGreaterThan(start);
|
||||
expect(boundedSkip).toBeLessThan(guard);
|
||||
// A hollow pane must also stop re-fetching megabytes on every scroll-up.
|
||||
expect(source).toContain('this._fullHistoryRepullUseless');
|
||||
expect(source).toContain('this._fullHistoryRepullUseless?.has(sessionId) ? 60000 : 4000');
|
||||
|
||||
Reference in New Issue
Block a user