mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(terminal): merge-time fixes for #494
- Skip and latch a bounded Shell window once the browser is at xterm's
scrollback cap (scrollback + rows): a 1 MiB window of short lines can carry
more rows than the browser can ever hold, so it replayed and re-captured on
every scroll-to-top with no 60 s back-off.
- Label a replayed bounded window 'tail' even when the capture was byte-capped,
so the banner keeps offering Load full history instead of calling the rest
unrecoverable.
- Pin GET /terminal?full=1&tail=<n> in the route tests: full-history source,
truncationReason 'tail', and the closing relative cursor move survive the cut.
- Log the bounded skip via _logScrollRouting('repull-skipped-bounded').
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+21
-3
@@ -6379,18 +6379,29 @@ class CodemanApp {
|
||||
// is left as the load that produced it set it: re-labelling it from this
|
||||
// payload would call a terminal that holds ALL of a Load full history pull
|
||||
// "the most recent 1 MiB".
|
||||
if (boundedShellPull && windowRows <= this.terminal.buffer.active.length) {
|
||||
//
|
||||
// A browser already at xterm's cap buys nothing either. xterm keeps at most
|
||||
// `scrollback + rows` rows (DEFAULT_SCROLLBACK 50k) while tmux keeps 100k
|
||||
// lines by default, so a 1 MiB window of short lines can render to more rows
|
||||
// than the browser can ever hold, and `windowRows <= rowsNow` then never
|
||||
// comes true: without this every scroll-to-top would reset and re-parse it.
|
||||
const rowsNow = this.terminal.buffer.active.length;
|
||||
const scrollbackCap = this.terminal.options?.scrollback || 0;
|
||||
const browserFull = scrollbackCap > 0 && rowsNow >= scrollbackCap + this.terminal.rows;
|
||||
if (boundedShellPull && (windowRows <= rowsNow || browserFull)) {
|
||||
// An untruncated window IS all of tmux's history, so nothing is missing,
|
||||
// and the next burst of output can put more in tmux than the browser has:
|
||||
// keep the normal 4 s cooldown. A truncated one is the opposite case, since
|
||||
// the gesture can never reach anything older than what the browser already
|
||||
// shows, and every ask costs the server a synchronous capture-pane of the
|
||||
// whole history (`tail` is applied after the capture): back off to 60 s.
|
||||
// A full browser backs off too, since no window can ever fit in it.
|
||||
// Trade-off: only a successful replay clears that latch, so a tab switch or
|
||||
// burst that shrinks the browser's buffer below the window can leave a
|
||||
// scroll-to-top inert for up to a minute. Load full history (`force`)
|
||||
// bypasses the cooldown, and the latch is bounded, never permanent.
|
||||
if (payload.truncated) (this._fullHistoryRepullUseless ||= new Set()).add(sessionId);
|
||||
if (payload.truncated || browserFull) (this._fullHistoryRepullUseless ||= new Set()).add(sessionId);
|
||||
this._logScrollRouting?.('repull-skipped-bounded');
|
||||
return;
|
||||
}
|
||||
if (this._replayWouldShrinkBuffer(buffer, windowRows)) {
|
||||
@@ -6404,7 +6415,14 @@ class CodemanApp {
|
||||
this._setHistoryTruncation(sessionId, { ...payload, exhausted: true });
|
||||
return;
|
||||
}
|
||||
this._setHistoryTruncation(sessionId, payload);
|
||||
// A bounded window that was cut is always recoverable: a capture over the
|
||||
// byte cap keeps `truncationReason: 'capped'` through the tail cut, and that
|
||||
// would tell the user the rest "cannot be recovered" and drop Load full
|
||||
// history, whose unbounded pull returns up to the cap itself.
|
||||
this._setHistoryTruncation(
|
||||
sessionId,
|
||||
boundedShellPull && payload.truncated ? { ...payload, truncationReason: 'tail' } : payload
|
||||
);
|
||||
this._fullHistoryRepullUseless?.delete(sessionId);
|
||||
const rowsBefore = this.terminal.buffer.active.length;
|
||||
const replayStartedAt = performance.now();
|
||||
|
||||
@@ -920,6 +920,53 @@ describe('session-routes', () => {
|
||||
expect(res.headers['server-timing']).toMatch(/^capture;dur=\d+\.\d, prepare;dur=\d+\.\d, total;dur=\d+\.\d$/);
|
||||
});
|
||||
|
||||
it('full reload with a tail (?full=1&tail=) cuts the full capture to its newest bytes, cursor restore intact', async () => {
|
||||
// A Shell scroll-to-top asks for exactly this (`_maybeRefetchFullHistory`):
|
||||
// tmux's whole scrollback, bounded to the tab-switch tail size. The client
|
||||
// relies on all three answers below, so a refactor that dropped the tail on
|
||||
// a full capture (an unbounded pull from an ordinary scroll) or cut off the
|
||||
// closing cursor move (a caret parked below the prompt) must fail here.
|
||||
const tail = 1024 * 1024;
|
||||
const oldestMarker = 'BOUNDED_OLDEST_LINE_00001';
|
||||
const newestMarker = 'BOUNDED_NEWEST_LINE_40000';
|
||||
const rows: string[] = [oldestMarker];
|
||||
for (let i = 2; i < 40_000; i++) rows.push(`shell history line ${String(i).padStart(5, '0')} lorem ipsum`);
|
||||
rows.push(newestMarker);
|
||||
// What formatCursorRestore appends: up from the last row, then the column.
|
||||
const cursorRestore = '\x1b[3A\r\x1b[2C';
|
||||
const fullHistoryCapture = `${rows.join('\r\n')}${cursorRestore}`;
|
||||
expect(fullHistoryCapture.length).toBeGreaterThan(tail);
|
||||
|
||||
harness.ctx._session.mode = 'shell';
|
||||
harness.ctx._session.terminalBuffer = '';
|
||||
const captureSpy = vi.fn((_name: string, opts?: { fullHistory?: boolean }) =>
|
||||
opts?.fullHistory ? fullHistoryCapture : 'only the visible frame'
|
||||
);
|
||||
(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&tail=${tail}`,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
// Still the scrollback, not the visible frame a plain `?tail=` gets.
|
||||
expect(captureSpy).toHaveBeenCalledWith(
|
||||
harness.ctx._session.muxName,
|
||||
expect.objectContaining({ fullHistory: true })
|
||||
);
|
||||
expect(body.data.source).toBe('mux-full-history');
|
||||
// Recoverable, not 'capped': Load full history can still bring the rest back.
|
||||
expect(body.data.truncated).toBe(true);
|
||||
expect(body.data.truncationReason).toBe('tail');
|
||||
expect(body.data.fullSize).toBe(fullHistoryCapture.length);
|
||||
expect(body.data.terminalBuffer.length).toBeLessThanOrEqual(tail);
|
||||
expect(body.data.terminalBuffer).toContain(newestMarker);
|
||||
expect(body.data.terminalBuffer).not.toContain(oldestMarker);
|
||||
expect(body.data.terminalBuffer.endsWith(`${newestMarker}${cursorRestore}`)).toBe(true);
|
||||
});
|
||||
|
||||
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
|
||||
|
||||
@@ -12,7 +12,9 @@
|
||||
*
|
||||
* The gesture now pulls a BOUNDED window (`?full=1&tail=TERMINAL_TAIL_SIZE`),
|
||||
* the button stays the unbounded path, and a window the browser already holds
|
||||
* in full is not rewritten.
|
||||
* in full is not rewritten. Neither is one a browser at xterm's scrollback cap
|
||||
* could never hold, and a window cut from a byte-capped capture is still labelled
|
||||
* recoverable, since Load full history can reach past it.
|
||||
*
|
||||
* 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
|
||||
@@ -104,7 +106,12 @@ const TAIL_CUT = {
|
||||
|
||||
function makeApp(
|
||||
mode: string,
|
||||
{ bufferRows, capture, payload = {} }: { bufferRows: number; capture: string; payload?: Record<string, unknown> }
|
||||
{
|
||||
bufferRows,
|
||||
capture,
|
||||
payload = {},
|
||||
scrollback = 0,
|
||||
}: { bufferRows: number; capture: string; payload?: Record<string, unknown>; scrollback?: number }
|
||||
) {
|
||||
const urls: string[] = [];
|
||||
const app = {
|
||||
@@ -119,6 +126,8 @@ function makeApp(
|
||||
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 } },
|
||||
scrollToLine: vi.fn(),
|
||||
scrollToTop: vi.fn(),
|
||||
@@ -183,6 +192,60 @@ describe('shell scroll-up pulls a bounded window of tmux history', () => {
|
||||
expect(app._fullHistoryRepullUseless.has('s1')).toBe(false);
|
||||
// Nothing was written, so the banner state is left exactly as it was.
|
||||
expect(app._setHistoryTruncation).not.toHaveBeenCalled();
|
||||
// …but the skip is visible to someone diagnosing "scroll-to-top does nothing".
|
||||
expect(app._logScrollRouting).toHaveBeenCalledWith('repull-skipped-bounded');
|
||||
});
|
||||
|
||||
it("a browser at xterm's scrollback cap stops replaying a window it can never hold, and backs off", async () => {
|
||||
// xterm keeps at most `scrollback + rows` rows while tmux keeps 100k lines, so
|
||||
// a 1 MiB window of short lines can carry more rows than the browser ever will.
|
||||
// `windowRows <= rows held` then never comes true, and every scroll-to-top
|
||||
// past the cooldown reset and re-parsed the window. Untruncated on purpose:
|
||||
// the back-off has to come from the full browser, not from `truncated`.
|
||||
const { app } = makeApp('shell', { bufferRows: 40, capture: lines(2000), scrollback: 1000 });
|
||||
|
||||
// The first pull has room to grow, so it replays.
|
||||
await refetch.call(app);
|
||||
expect(app._resetTerminalForReplay).toHaveBeenCalledTimes(1);
|
||||
expect(app._fullHistoryRepullUseless.has('s1')).toBe(false);
|
||||
// xterm kept only the last `scrollback + rows` of the 2000 rows written.
|
||||
app.terminal.buffer.active.length = 1000 + 30;
|
||||
|
||||
// Past the 4 s cooldown: the browser is full, so nothing is replayed and the
|
||||
// session backs off for a minute.
|
||||
app._fullHistoryRepullAt.set('s1', Date.now() - 5000);
|
||||
await refetch.call(app);
|
||||
expect(app._fetchTerminalCapture).toHaveBeenCalledTimes(2);
|
||||
expect(app._resetTerminalForReplay).toHaveBeenCalledTimes(1);
|
||||
expect(app.chunkedTerminalWrite).toHaveBeenCalledTimes(1);
|
||||
expect(app._fullHistoryRepullUseless.has('s1')).toBe(true);
|
||||
|
||||
// So a scroll 10 s later does not even ask the server for another capture.
|
||||
app._fullHistoryRepullAt.set('s1', Date.now() - 10_000);
|
||||
await refetch.call(app);
|
||||
expect(app._fetchTerminalCapture).toHaveBeenCalledTimes(2);
|
||||
expect(app._resetTerminalForReplay).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('a replayed window cut from a byte-capped capture still offers Load full history', async () => {
|
||||
// The route keeps `truncationReason: 'capped'` through the tail cut when the
|
||||
// full capture exceeded the byte cap. On a bounded window that is not "gone for
|
||||
// good": the unbounded pull behind the button returns up to the cap itself.
|
||||
const capped = { ...TAIL_CUT, truncationReason: 'capped', fullSize: 40 * 1024 * 1024 };
|
||||
const { app } = makeApp('shell', { bufferRows: 40, capture: lines(300), payload: capped });
|
||||
|
||||
await refetch.call(app);
|
||||
|
||||
expect(app._resetTerminalForReplay).toHaveBeenCalledTimes(1);
|
||||
expect(app._setHistoryTruncation).toHaveBeenCalledWith('s1', expect.objectContaining({ truncationReason: 'tail' }));
|
||||
const notice = computeNotice(app._historyTruncation.get('s1') as Record<string, unknown>);
|
||||
expect(notice.visible).toBe(true);
|
||||
expect(notice.canLoadMore).toBe(true);
|
||||
|
||||
// The button's own unbounded pull is the one place 'capped' is the truth.
|
||||
const button = makeApp('shell', { bufferRows: 40, capture: lines(300), payload: capped });
|
||||
await refetch.call(button.app, { force: true });
|
||||
expect(computeNotice(button.app._historyTruncation.get('s1') as Record<string, unknown>).canLoadMore).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -114,7 +114,7 @@ describe('full-history re-pull downgrade guard (issue #205 round 2)', () => {
|
||||
// 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 boundedSkip = source.indexOf('boundedShellPull && (windowRows <= rowsNow || browserFull)', start);
|
||||
const reset = source.indexOf('this._resetTerminalForReplay()', start);
|
||||
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
|
||||
Reference in New Issue
Block a user