diff --git a/docs/remote-sessions.md b/docs/remote-sessions.md index a592be7f..f050fed4 100644 --- a/docs/remote-sessions.md +++ b/docs/remote-sessions.md @@ -266,8 +266,8 @@ command itself (a remote tmux session that is gone) affects that session only. How much history exists is the remote tmux's own `history-limit`, which the launch command does not set (tmux's default is 2000 lines). -The same round trip also reads the remote SCREEN, right after the history, -because two seams would otherwise drop rows at the join: +The same round trip also reads the remote SCREEN, in the same tmux command list +as the history, because two seams would otherwise drop rows at the join: - **Rows that scroll during the round trip.** The local frame is captured only once the round trip is back, so rows that scrolled off the screen in between @@ -289,11 +289,23 @@ with two of them non-blank, and the highest match wins. A placement too low would REPEAT the rows between it and the true one, so a shorter overlap (a lone `$` row that happens to match after a spinner row changed) is not trusted. When the frame cannot be placed (more output scrolled than the screen held, or -nothing matches), the join is history over frame, as without the screen read. What is left: the history and screen captures are two -tmux commands, so a row that scrolls between them (milliseconds apart, not the -round trip) is in neither, and when the screen repeats a whole block of rows -the frame can be placed at the higher copy, which leaves out the rows between -the two copies. +nothing matches), the join is history over frame, as without the screen read. +What is left: when the screen repeats a whole block of rows, the frame can be +placed at the higher copy, which leaves out the rows between the two copies. + +The history, the history size and the screen are ONE tmux command list +(`capture-pane … \; display-message … \; capture-pane …`), not separate `tmux` +runs. tmux runs a client's command list back to back without reading pane +output in between, so the screen begins exactly where the history ends. As +separate runs the shell kept writing between them and the rows that scrolled in +those milliseconds were in neither capture (measured with output streaming: 1-2 +rows per load on a LAN host, every read on a local busy pane). This rests on how +tmux's command queue behaves, not on a documented guarantee. A random marker row +(`display-message`) separates the history from the size and the screen, since +tmux output cannot carry the NUL markers around it. The remote `tail -c` bound +applies to that whole output, history first, and is the configured byte cap +plus 1 MiB for the marker, size and screen, so a cut lands in the history; the +route trims what it returns to the cap itself. ## File access over SSH diff --git a/src/remote-pane-history.ts b/src/remote-pane-history.ts index 91f30ade..5f161744 100644 --- a/src/remote-pane-history.ts +++ b/src/remote-pane-history.ts @@ -11,15 +11,24 @@ * the local pane is what the live stream keeps drawing into, so its frame and caret * position are the ones the browser must end up with. * - * ONE ssh round trip prints the remote pane's `#{history_size}`, its scrollback - * rows (`capture-pane -S - -E -1`) and then its screen. The size is load-bearing: - * with no history at all, tmux clamps `-E -1` to the first VISIBLE row and would - * return it as history. The screen closes the seam between the two captures: the - * local frame is read only after the round trip, so rows that scrolled in between, + * ONE ssh round trip reads the remote pane's scrollback rows (`capture-pane -S - + * -E -1`), its `#{history_size}` and its screen. The size is load-bearing: with no + * history at all, tmux clamps `-E -1` to the first VISIBLE row and would return it + * as history. The screen closes the seam between the remote read and the local + * frame, which is read only after the round trip: rows that scrolled in between, * and the top of a remote screen taller than the local pane, are on that screen * alone, and the caller keeps the screen rows above where the local frame overlaps * it (`joinScrollbackAndFrame`). * + * ⚠️ The three reads are ONE tmux command list (`a \; b \; c`), not three tmux + * runs. tmux runs a client's command list back to back without reading pane output + * in between, so the history ends exactly where the screen begins. As separate runs + * the shell kept writing between them, and the rows that scrolled in those + * milliseconds were in neither capture (measured on a busy pane: every read lost + * rows). This rests on how tmux's command queue behaves, not on a documented + * guarantee. tmux output cannot carry the NUL markers, so a random marker line + * printed by `display-message` separates the history from the size and screen. + * * Bounded and fail-soft. Any failure (unreachable host, auth, missing session, odd * output) returns null and the caller keeps the local capture, so a remote load is * never worse than before. A transport failure also starts a short per-host back-off, @@ -34,6 +43,7 @@ */ import { exec } from 'node:child_process'; +import { randomBytes } from 'node:crypto'; import { promisify } from 'node:util'; import type { SessionRemote } from './types/session.js'; import { buildSshConnectionArgs, classifyRemoteAliveExit, remoteSshTarget, shellescape } from './remote-hosts.js'; @@ -59,11 +69,24 @@ const REMOTE_HISTORY_SLOT_WAIT_MS = 1_500; const REMOTE_HISTORY_BACKOFF_MS = 30_000; /** - * Headroom over the remote `tail -c` bound for the size line, the markers and - * the screen capture, which the bound does not cover (a wide, tall screen of - * colored rows runs to a few hundred KB). + * Headroom over the remote `tail -c` bound for what it does not cover: the two NUL + * markers and anything a login banner or rc file prints before the script. */ -const REMOTE_HISTORY_SLACK_BYTES = 1024 * 1024; +const REMOTE_HISTORY_SLACK_BYTES = 64 * 1024; + +/** + * Added to the configured byte cap for the remote `tail -c` bound. That bound + * covers the whole tmux output, and the marker, the size and the screen come last + * (a wide, tall screen of colored rows runs to a few hundred KB); without this, a + * small cap would cut into the screen and lose the read. The route trims what it + * returns to the configured cap anyway, keeping the newest bytes. + */ +const REMOTE_SCREEN_ALLOWANCE_BYTES = 1024 * 1024; + +/** The remote `tail -c` bound for a configured byte cap; 0 (unbounded) stays 0. */ +export function remoteTailBytes(maxBytes: number): number { + return maxBytes > 0 ? Math.trunc(maxBytes) + REMOTE_SCREEN_ALLOWANCE_BYTES : 0; +} /** Read ceiling when the configured byte cap is 0 (unbounded) and no `tail -c` runs. */ const UNBOUNDED_READ_MAX_BYTES = 64 * 1024 * 1024; @@ -75,17 +98,28 @@ const UNBOUNDED_READ_MAX_BYTES = 64 * 1024 * 1024; const backoffUntil = new Map(); /** - * The remote shell script. Its first byte is a NUL so a login banner or an rc-file - * `echo` printed before it runs is separated without guessing (the same marker - * `buildRemoteProbeCommand` uses); a second NUL ends the history size. + * A fresh marker line for one read. Random, so no row the pane really holds can be + * taken for it. + */ +export function newScrollbackMarker(): string { + return `codeman-history-${randomBytes(8).toString('hex')}`; +} + +/** + * The remote shell script. It prints a NUL, the tmux output, then a second NUL: + * the first separates a login banner or an rc-file `echo` printed before the + * script runs without guessing (the same marker `buildRemoteProbeCommand` uses), + * and both are printed whether or not tmux succeeds, so their presence says the + * script ran on the host. The tmux output is the history rows, `marker`, the + * history size, then the screen rows. */ export function buildRemoteScrollbackScript( socket: string, sessionName: string, historyLines: number, - maxBytes: number + tailBytes: number, + marker: string ): string { - const tmux = `tmux -L ${shellescape(socket)}`; // `=name:` is an EXACT session match, then its active pane. A bare name is // read as a pane index or window name first, so a discovered session named // `0` would capture some other session's pane. @@ -93,15 +127,18 @@ export function buildRemoteScrollbackScript( const lines = Math.max(1, Math.trunc(historyLines)); // The newest rows are the ones worth keeping, so the byte bound is applied on // the remote side (`tail -c`) rather than by killing the read at maxBuffer. - // A cap of 0 means unbounded, as in the route. - const bound = maxBytes > 0 ? ` | tail -c ${Math.trunc(maxBytes)}` : ''; - // The screen comes right after the history, in the same round trip: the - // local frame is captured only once this returns, so rows that scroll off - // in between are on this screen and nowhere else (joinScrollbackAndFrame). + // It covers the whole tmux output, history first, so with the screen + // allowance in `remoteTailBytes()` a cut lands in the history. 0 means + // unbounded, as in the route. + const bound = tailBytes > 0 ? ` | tail -c ${Math.trunc(tailBytes)}` : ''; + // ONE tmux command list, so no pane output is read between the history and + // the screen (see the @fileoverview). `\;` reaches tmux as a lone `;`. return ( - `printf '\\0' && ${tmux} display-message -p -t ${target} '#{history_size}' && printf '\\0' && ` + - `${tmux} capture-pane -p -e -J -S -${lines} -E -1 -t ${target}${bound} && printf '\\0' && ` + - `${tmux} capture-pane -p -e -J -t ${target}` + `printf '\\0'; tmux -L ${shellescape(socket)} ` + + `capture-pane -p -e -J -S -${lines} -E -1 -t ${target} \\; ` + + `display-message -p -t ${target} ${shellescape(marker)} \\; ` + + `display-message -p -t ${target} '#{history_size}' \\; ` + + `capture-pane -p -e -J -t ${target}${bound}; printf '\\0'` ); } @@ -110,10 +147,11 @@ export function buildRemoteScrollbackCommand( remote: SessionRemote, sessionId: string, historyLines: number, - maxBytes: number + tailBytes: number, + marker: string ): string { const { socket, sessionName } = remoteTmuxLocation(remote, sessionId); - const script = buildRemoteScrollbackScript(socket, sessionName, historyLines, maxBytes); + const script = buildRemoteScrollbackScript(socket, sessionName, historyLines, tailBytes, marker); // `exec` so a timeout kill reaches ssh itself, not only the local `sh -c`. return ['exec', ...buildSshConnectionArgs(remote), remoteSshTarget(remote), shellescape(script)].join(' '); } @@ -132,27 +170,41 @@ export function dropPartialFirstRow(scrollback: string, maxBytes: number): strin export interface RemoteScrollback { /** History rows above the screen, oldest first; '' when the pane has none. */ scrollback: string; - /** The screen's rows, read right after the scrollback. */ + /** The screen's rows, read in the same tmux command list, so they begin where the scrollback ends. */ screen: string; } /** * The scrollback and screen rows from the script's stdout, or null when the - * output is not the script's. + * output is not the script's or tmux printed nothing usable (no such session, + * or a byte cap so small it cut into the screen). */ -export function parseRemoteScrollbackOutput(stdout: string): RemoteScrollback | null { +export function parseRemoteScrollbackOutput( + stdout: string, + marker: string, + tailBytes: number +): RemoteScrollback | null { const start = stdout.indexOf('\0'); if (start === -1) return null; - const sizeEnd = stdout.indexOf('\0', start + 1); + const end = stdout.indexOf('\0', start + 1); + if (end === -1) return null; + // The marker is a whole row of its own, after the history's last row. + const markerRow = `${marker}\n`; + const raw = stdout.slice(start + 1, end); + // A cut that landed exactly on the marker row left it whole: there is no + // partial history row to drop, and dropping the first row would lose it. + const output = raw.startsWith(markerRow) ? raw : dropPartialFirstRow(raw, tailBytes); + const at = output.startsWith(markerRow) ? 0 : output.indexOf(`\n${markerRow}`) + 1; + if (at === 0 && !output.startsWith(markerRow)) return null; + const sizeStart = at + markerRow.length; + const sizeEnd = output.indexOf('\n', sizeStart); if (sizeEnd === -1) return null; - const historyEnd = stdout.indexOf('\0', sizeEnd + 1); - if (historyEnd === -1) return null; - const sizeText = stdout.slice(start + 1, sizeEnd).trim(); + const sizeText = output.slice(sizeStart, sizeEnd).trim(); if (!/^\d+$/.test(sizeText)) return null; - const screen = stdout.slice(historyEnd + 1); + const screen = output.slice(sizeEnd + 1); // With no history at all, tmux clamps `-E -1` to the first visible row. if (Number(sizeText) === 0) return { scrollback: '', screen }; - return { scrollback: stdout.slice(sizeEnd + 1, historyEnd), screen }; + return { scrollback: output.slice(0, at), screen }; } /** Runs the ssh command line and resolves its stdout; rejects like `exec` does. */ @@ -168,8 +220,8 @@ const execRunner: RemoteScrollbackRunner = async (command, opts) => * Only a TRANSPORT failure backs the host off (ssh's own exit 255, a timeout, a * spawn error, output that is not the script's): those say the host is * unreachable for every session on it. A remote command failing (a session - * that is gone: tmux exits 1, or answers nothing between the markers when the - * `| tail -c` pipe hides its status) is about this session only, so it returns + * that is gone: tmux answers nothing between the NULs, and the closing `printf` + * hides its exit status) is about this session only, so it returns * null and the next session on that host still tries. Same split as * `classifyRemoteAliveExit()`. * @@ -190,32 +242,42 @@ export async function fetchRemoteScrollback( if (Date.now() < until) return null; backoffUntil.delete(remote.hostId); } - const command = buildRemoteScrollbackCommand(remote, sessionId, historyLines, maxBytes); + const marker = newScrollbackMarker(); + const tailBytes = remoteTailBytes(maxBytes); + const command = buildRemoteScrollbackCommand(remote, sessionId, historyLines, tailBytes, marker); try { const stdout = await runWithRemoteSshLimit( () => runner(command, { timeout: REMOTE_HISTORY_TIMEOUT_MS, - maxBuffer: (maxBytes > 0 ? maxBytes : UNBOUNDED_READ_MAX_BYTES) + REMOTE_HISTORY_SLACK_BYTES, + maxBuffer: (tailBytes > 0 ? tailBytes : UNBOUNDED_READ_MAX_BYTES) + REMOTE_HISTORY_SLACK_BYTES, }), // The pool is shared with 20 s file probes; past this the local capture // is the better answer than waiting behind them. { maxWaitMs: REMOTE_HISTORY_SLOT_WAIT_MS } ); - const parsed = parseRemoteScrollbackOutput(stdout); + const parsed = parseRemoteScrollbackOutput(stdout, marker, tailBytes); if (parsed === null) { - // Both markers arrived, so the script ran on the host and only tmux had - // nothing to say: no such session. The `| tail -c` pipe hides tmux's - // exit status, so this comes back as exit 0 rather than exit 1, and it - // is about this session alone. Output without the markers is not the - // script's at all, which says the host is not answering as expected. - const ranOnHost = stdout.split('\0').length >= 3; + // Both NULs arrived, so the script ran on the host and only tmux had + // nothing to say: no such session. The script's last command is the + // closing `printf`, so this comes back as exit 0 rather than tmux's + // exit 1, and it is about this session alone. Output without the NULs + // is not the script's at all, which says the host is not answering as + // expected. + const parts = stdout.split('\0'); + const ranOnHost = parts.length >= 3; if (!ranOnHost) backoffUntil.set(remote.hostId, Date.now() + REMOTE_HISTORY_BACKOFF_MS); - const what = ranOnHost ? 'no such remote tmux session' : 'unexpected output'; + // tmux printed something, but not the marker, size and screen in order. + const unreadable = ranOnHost && parts[1] !== ''; + const what = !ranOnHost + ? 'unexpected output' + : unreadable + ? 'unreadable remote tmux output' + : 'no such remote tmux session'; console.warn(`[remote-history] ${remote.label}: ${what}; using the local capture`); return null; } - return { scrollback: dropPartialFirstRow(parsed.scrollback, maxBytes), screen: parsed.screen }; + return parsed; } catch (err) { // A busy pool says nothing about the host. if (err instanceof RemoteSshSlotTimeoutError) return null; diff --git a/test/remote-pane-history.test.ts b/test/remote-pane-history.test.ts index 667ab529..33629f33 100644 --- a/test/remote-pane-history.test.ts +++ b/test/remote-pane-history.test.ts @@ -19,7 +19,9 @@ import { buildRemoteScrollbackScript, dropPartialFirstRow, fetchRemoteScrollback, + newScrollbackMarker, parseRemoteScrollbackOutput, + remoteTailBytes, resetRemoteScrollbackBackoff, } from '../src/remote-pane-history.js'; import { @@ -55,9 +57,11 @@ describe('remoteTmuxLocation', () => { }); }); +const MARKER = 'codeman-history-0123456789abcdef'; + describe('buildRemoteScrollbackCommand', () => { it('goes through the shared ssh options and reads the history rows, then the screen', () => { - const command = buildRemoteScrollbackCommand(remote, SESSION_ID, 10000, 2048); + const command = buildRemoteScrollbackCommand(remote, SESSION_ID, 10000, 2048, MARKER); expect(command.startsWith('exec ssh -o BatchMode=yes')).toBe(true); expect(command).toContain('-p 2222'); expect(command).toContain('tim@proxx.lan'); @@ -66,13 +70,31 @@ describe('buildRemoteScrollbackCommand', () => { expect(command).toContain('codeman-ssh-663e3eae'); expect(command).toContain('capture-pane -p -e -J -S -10000 -E -1'); expect(command).toContain('#{history_size}'); + expect(command).toContain(MARKER); expect(command).toContain('| tail -c 2048'); - // The screen capture comes after the history and is not under the byte bound. expect(command.indexOf('-E -1')).toBeLessThan(command.lastIndexOf('capture-pane -p -e -J -t')); }); it('a byte cap of 0 (unbounded) runs no tail', () => { - expect(buildRemoteScrollbackCommand(remote, SESSION_ID, 10000, 0)).not.toContain('tail -c'); + expect(buildRemoteScrollbackCommand(remote, SESSION_ID, 10000, 0, MARKER)).not.toContain('tail -c'); + }); +}); + +describe('remoteTailBytes', () => { + it('adds room for the marker, size and screen to the cap, so a cut lands in the history', () => { + expect(remoteTailBytes(2048)).toBe(2048 + 1024 * 1024); + }); + + it('keeps 0 (unbounded) unbounded', () => { + expect(remoteTailBytes(0)).toBe(0); + }); +}); + +describe('newScrollbackMarker', () => { + it('is a fresh marker of a fixed shape on every read', () => { + const a = newScrollbackMarker(); + expect(a).toMatch(/^codeman-history-[0-9a-f]{16}$/); + expect(newScrollbackMarker()).not.toBe(a); }); }); @@ -86,9 +108,15 @@ describe('dropPartialFirstRow', () => { }); describe('parseRemoteScrollbackOutput', () => { + const read = (tmuxOutput: string) => `\x00${tmuxOutput}\x00`; + it('drops anything printed before the script (login banner, rc-file echo)', () => { expect( - parseRemoteScrollbackOutput('Welcome to proxx\n\x0042\n\x00line a\nline b\n\x00screen 1\nscreen 2\n') + parseRemoteScrollbackOutput( + `Welcome to proxx\n${read(`line a\nline b\n${MARKER}\n42\nscreen 1\nscreen 2\n`)}`, + MARKER, + 0 + ) ).toEqual({ scrollback: 'line a\nline b\n', screen: 'screen 1\nscreen 2\n', @@ -96,19 +124,48 @@ describe('parseRemoteScrollbackOutput', () => { }); it('an empty history is empty, not the first visible row tmux clamps -E -1 to', () => { - expect(parseRemoteScrollbackOutput('\x000\n\x00$ the prompt row\n\x00$ the prompt row\n')).toEqual({ + expect(parseRemoteScrollbackOutput(read(`$ the prompt row\n${MARKER}\n0\n$ the prompt row\n`), MARKER, 0)).toEqual({ scrollback: '', screen: '$ the prompt row\n', }); }); - it('rejects output that is not the script’s', () => { - expect(parseRemoteScrollbackOutput('')).toBeNull(); - expect(parseRemoteScrollbackOutput('no markers at all')).toBeNull(); - expect(parseRemoteScrollbackOutput('\x0042\n')).toBeNull(); - // The screen marker is missing: the script stopped after the history. - expect(parseRemoteScrollbackOutput('\x0042\n\x00rows\n')).toBeNull(); - expect(parseRemoteScrollbackOutput('\x00not a number\n\x00rows\x00screen')).toBeNull(); + it('a history the byte cap cut loses its partial first row, and the screen is kept whole', () => { + const output = `ow 1\nrow 2\n${MARKER}\n7\nscreen\n`; + expect(parseRemoteScrollbackOutput(read(output), MARKER, Buffer.byteLength(output))).toEqual({ + scrollback: 'row 2\n', + screen: 'screen\n', + }); + }); + + it('a cut that landed exactly on the marker row keeps it', () => { + const output = `${MARKER}\n3\nscreen\n`; + expect(parseRemoteScrollbackOutput(read(output), MARKER, Buffer.byteLength(output))).toEqual({ + scrollback: '', + screen: 'screen\n', + }); + }); + + it('finds the marker only as a whole row', () => { + const output = `echo ${MARKER}\n${MARKER}\n2\nscreen\n`; + expect(parseRemoteScrollbackOutput(read(output), MARKER, 0)).toEqual({ + scrollback: `echo ${MARKER}\n`, + screen: 'screen\n', + }); + }); + + it('rejects output that is not the script’s, or tmux output with nothing usable', () => { + expect(parseRemoteScrollbackOutput('', MARKER, 0)).toBeNull(); + expect(parseRemoteScrollbackOutput('no markers at all', MARKER, 0)).toBeNull(); + // The closing NUL is missing: the connection dropped mid-read. + expect(parseRemoteScrollbackOutput(`\x00rows\n${MARKER}\n2\nscreen\n`, MARKER, 0)).toBeNull(); + // Both NULs, nothing between: tmux found no such session. + expect(parseRemoteScrollbackOutput('\x00\x00', MARKER, 0)).toBeNull(); + // Another read's marker. + expect( + parseRemoteScrollbackOutput(read('rows\ncodeman-history-ffffffffffffffff\n2\nscreen\n'), MARKER, 0) + ).toBeNull(); + expect(parseRemoteScrollbackOutput(read(`rows\n${MARKER}\nnot a number\nscreen\n`), MARKER, 0)).toBeNull(); }); }); @@ -119,19 +176,27 @@ describe('the remote script under a real shell', () => { beforeAll(() => { dir = mkdtempSync(join(tmpdir(), 'remote-scrollback-')); callsFile = join(dir, 'calls'); - // Stub tmux: logs its argv (one per line), answers display-message with a - // history size and capture-pane with two rows. + // Stub tmux: logs its argv (one per line) and runs a `;`-separated command + // list in order, as tmux does: the history size, the marker it was given, + // two history rows, one screen row. writeFileSync( join(dir, 'tmux'), [ '#!/bin/sh', `for a in "$@"; do printf '%s\\n' "$a"; done >> '${callsFile}'`, `printf -- '--\\n' >> '${callsFile}'`, - 'case " $* " in', - ' *" display-message "*) echo 2 ;;', - ' *" -E -1 "*) printf "older row\\nnewer row\\n" ;;', - ' *" capture-pane "*) printf "screen row\\n" ;;', - 'esac', + 'run() {', + ' case " $1 " in', + ' *" #{history_size} "*) echo 2 ;;', + ` *" display-message "*) printf '%s\\n' "$2" ;;`, + ' *" -E -1 "*) printf "older row\\nnewer row\\n" ;;', + ' *" capture-pane "*) printf "screen row\\n" ;;', + ' esac', + '}', + "cmd=''; last=''", + `for a in "$@" ';'; do`, + ` if [ "$a" = ';' ]; then run "$cmd" "$last"; cmd=''; else cmd="$cmd $a"; last="$a"; fi`, + 'done', ].join('\n') ); chmodSync(join(dir, 'tmux'), 0o755); @@ -141,21 +206,47 @@ describe('the remote script under a real shell', () => { rmSync(dir, { recursive: true, force: true }); }); - it('prints the markers and rows the parser expects, with the target as one argument', () => { - const script = buildRemoteScrollbackScript('codeman', "codeman-it's", 500, 10); + it('prints the markers and rows the parser expects, from ONE tmux command list', () => { + // A bound that keeps "row\nnewer row\n" of the history: the cut lands in + // "older row", which the parser then drops. + const bound = Buffer.byteLength(`row\nnewer row\n${MARKER}\n2\nscreen row\n`); + const script = buildRemoteScrollbackScript('codeman', "codeman-it's", 500, bound, MARKER); const stdout = execFileSync('/bin/sh', ['-c', script], { encoding: 'utf-8', env: { ...process.env, PATH: `${dir}:${process.env.PATH ?? ''}` }, }); - // `tail -c 10` kept the newest history bytes only: "newer row\n"; the - // screen after it is not under that bound. - expect(parseRemoteScrollbackOutput(stdout)).toEqual({ scrollback: 'newer row\n', screen: 'screen row\n' }); + expect(parseRemoteScrollbackOutput(stdout, MARKER, bound)).toEqual({ + scrollback: 'newer row\n', + screen: 'screen row\n', + }); const calls = execFileSync('cat', [callsFile], { encoding: 'utf-8' }).split('--\n').filter(Boolean); - expect(calls).toHaveLength(3); + // One tmux run: separate runs would let the pane scroll between the reads. + expect(calls).toHaveLength(1); // One argv entry per line: the exact-match target with a quote in it arrived whole. - expect(calls[0].split('\n').join('|')).toBe("-L|codeman|display-message|-p|-t|=codeman-it's:|#{history_size}|"); - expect(calls[1].split('\n').join('|')).toBe("-L|codeman|capture-pane|-p|-e|-J|-S|-500|-E|-1|-t|=codeman-it's:|"); - expect(calls[2].split('\n').join('|')).toBe("-L|codeman|capture-pane|-p|-e|-J|-t|=codeman-it's:|"); + expect(calls[0].split('\n').join('|')).toBe( + [ + "-L|codeman|capture-pane|-p|-e|-J|-S|-500|-E|-1|-t|=codeman-it's:", + `display-message|-p|-t|=codeman-it's:|${MARKER}`, + "display-message|-p|-t|=codeman-it's:|#{history_size}", + "capture-pane|-p|-e|-J|-t|=codeman-it's:|", + ].join('|;|') + ); + }); + + it('prints both NULs when tmux fails, so a missing session reads as the script having run', () => { + const failing = mkdtempSync(join(tmpdir(), 'remote-scrollback-fail-')); + try { + writeFileSync(join(failing, 'tmux'), '#!/bin/sh\necho "can\'t find session" >&2\nexit 1\n'); + chmodSync(join(failing, 'tmux'), 0o755); + const stdout = execFileSync('/bin/sh', ['-c', buildRemoteScrollbackScript('codeman', 'gone', 500, 0, MARKER)], { + encoding: 'utf-8', + stdio: ['ignore', 'pipe', 'ignore'], + env: { ...process.env, PATH: `${failing}:${process.env.PATH ?? ''}` }, + }); + expect(stdout).toBe('\x00\x00'); + } finally { + rmSync(failing, { recursive: true, force: true }); + } }); }); @@ -253,7 +344,10 @@ describe('joinScrollbackAndFrame with the remote screen', () => { }); describe('fetchRemoteScrollback', () => { - const OK = '\x002\n\x00older row\nnewer row\n\x00screen row\n'; + /** The marker a read put in its command line. */ + const markerIn = (command: string) => /codeman-history-[0-9a-f]{16}/.exec(command)?.[0] ?? 'no-marker'; + /** A successful read's stdout, answering with the marker the command carries. */ + const OK = async (command: string) => `\x00older row\nnewer row\n${markerIn(command)}\n2\nscreen row\n\x00`; const READ = { scrollback: 'older row\nnewer row\n', screen: 'screen row\n' }; /** An exec-shaped rejection: `code` is the exit status, `killed` a timeout kill. */ const execError = (fields: { code?: number | string; killed?: boolean }) => @@ -274,12 +368,15 @@ describe('fetchRemoteScrollback', () => { }); it('returns the remote rows, running the built command with the round-trip bounds', async () => { - const run = vi.fn(async () => OK); + const run = vi.fn(OK); expect(await fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, run)).toEqual(READ); - expect(run).toHaveBeenCalledWith(buildRemoteScrollbackCommand(remote, SESSION_ID, 100, 1024), { + const marker = markerIn(run.mock.calls[0][0]); + const tailBytes = 1024 + 1024 * 1024; + expect(run).toHaveBeenCalledWith(buildRemoteScrollbackCommand(remote, SESSION_ID, 100, tailBytes, marker), { timeout: 5000, - // The byte cap covers the history; the slack covers the screen after it. - maxBuffer: 1024 + 1024 * 1024, + // The tail bound covers the whole tmux output; the slack covers the NULs + // and anything printed before the script. + maxBuffer: tailBytes + 64 * 1024, }); }); @@ -294,7 +391,7 @@ describe('fetchRemoteScrollback', () => { resetRemoteScrollbackBackoff(); expect(await fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, vi.fn(failure))).toBeNull(); // Another session on the same host is skipped without a round trip. - const next = vi.fn(async () => OK); + const next = vi.fn(OK); expect(await fetchRemoteScrollback(remote, 'other-session', 100, 1024, next)).toBeNull(); expect(next).not.toHaveBeenCalled(); } @@ -303,19 +400,19 @@ describe('fetchRemoteScrollback', () => { it('a remote command failure (tmux exit 1: the session is gone) is that session only, no back-off', async () => { const gone = vi.fn(async () => Promise.reject(execError({ code: 1 }))); expect(await fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, gone)).toBeNull(); - const next = vi.fn(async () => OK); + const next = vi.fn(OK); expect(await fetchRemoteScrollback(remote, 'other-session', 100, 1024, next)).toEqual(READ); expect(next).toHaveBeenCalledOnce(); }); - // Found live against a real host: the `| tail -c` pipe takes tail's exit - // status, so a missing session comes back as exit 0 with both markers and + // Found live against a real host: a missing session comes back as exit 0 + // (the script's closing `printf` hides tmux's status) with both NULs and // nothing between them. The script ran on the host; only this session is // gone, so the host must not be backed off. - it('a missing session hidden behind the tail pipe (exit 0, empty markers) is that session only', async () => { + it('a missing session hidden behind the closing printf (exit 0, empty NULs) is that session only', async () => { const masked = vi.fn(async () => '\x00\x00'); expect(await fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, masked)).toBeNull(); - const next = vi.fn(async () => OK); + const next = vi.fn(OK); expect(await fetchRemoteScrollback(remote, 'other-session', 100, 1024, next)).toEqual(READ); expect(next).toHaveBeenCalledOnce(); }); @@ -323,7 +420,7 @@ describe('fetchRemoteScrollback', () => { it('the back-off ends after 30 s', async () => { vi.useFakeTimers(); await fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, async () => Promise.reject(execError({ code: 255 }))); - const next = vi.fn(async () => OK); + const next = vi.fn(OK); vi.advanceTimersByTime(29_999); expect(await fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, next)).toBeNull(); vi.advanceTimersByTime(1); @@ -342,7 +439,7 @@ describe('fetchRemoteScrollback', () => { const holders = gates.map((gate) => runWithRemoteSshLimit(() => gate.promise)); await Promise.resolve(); - const run = vi.fn(async () => OK); + const run = vi.fn(OK); const pending = fetchRemoteScrollback(remote, SESSION_ID, 100, 1024, run); await vi.advanceTimersByTimeAsync(1499); expect(getQueuedRemoteSshCount()).toBe(1);