diff --git a/src/tmux-manager.ts b/src/tmux-manager.ts index 0eba7b22..f6a5efaf 100644 --- a/src/tmux-manager.ts +++ b/src/tmux-manager.ts @@ -4295,7 +4295,8 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { } capturePaneBuffer(muxName: string, paneTarget?: string, opts?: PaneCaptureOptions): string | null { - if (IS_TEST_MODE) return ''; + const exec = this.captureRunner(); + if (!exec) return ''; const target = resolveTmuxPaneTarget(muxName, paneTarget); if (!target) { console.error('[TmuxManager] Invalid pane target in capturePaneBuffer:', { muxName, paneTarget }); @@ -4341,13 +4342,20 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { // caret belongs to the frame. Two separate calls raced the CLI's repaint: a // CLI that redraws its composer a few ms after input arrives (codex) could be // captured before the redraw and its caret queried after it, and guarded - // stage then refused a frame the keys had already reached. Any doubt about - // the combined output (a failed call, a last line that is not the query's) - // falls back to the two separate calls, which is what this always did. - const combined = fullHistory ? null : this.captureFrameWithCursor(`${captureCommand} \\; ${cursorQuery}`); + // stage then refused a frame the keys had already reached. Only output that + // splitFrameAndCursor() cannot split (a last line that is not the query's) + // falls back to the two separate calls, which is what this always did. A call + // that THREW (a hung tmux timing out) goes to the catch below like any other + // capture failure: retrying it would block the event loop for a second timeout + // on every `GET .../terminal` load. + const combined = fullHistory + ? null + : splitFrameAndCursor( + exec(`${captureCommand} \\; ${cursorQuery}`, { encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS }) + ); const rawCapture = joinScrollbackAndFrame( scrollbackOverride, - combined?.frame ?? execSync(captureCommand, execOpts), + combined?.frame ?? exec(captureCommand, execOpts), scrollbackOverride !== undefined ? opts?.scrollbackOverrideScreen : undefined ); // Query the cursor BEFORE deciding anything else. On the full-history path @@ -4357,7 +4365,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { const geometry = queryPaneCursor(() => combined ? combined.cursor - : execSync(`${this.tmux()} ${cursorQuery}`, { encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS }) + : exec(`${this.tmux()} ${cursorQuery}`, { encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS }) ); // Report the size the pane was really drawing at. The visible-frame path // below addresses every row absolutely, so a consumer whose terminal is @@ -4423,13 +4431,16 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer { } } - /** One combined capture-and-cursor-query call, split; null when it failed or is ambiguous. */ - private captureFrameWithCursor(command: string): { frame: string; cursor: string } | null { - try { - return splitFrameAndCursor(execSync(command, { encoding: 'utf-8', timeout: EXEC_TIMEOUT_MS })); - } catch { - return null; - } + /** + * How {@link capturePaneBuffer} runs tmux, or null where it must not (vitest). + * Split out like {@link inputRunner} so a test subclass can count and script the + * calls a capture makes. + */ + protected captureRunner(): + | ((cmd: string, opts: { encoding: 'utf-8'; timeout: number; maxBuffer?: number }) => string) + | null { + if (IS_TEST_MODE) return null; + return (cmd, opts) => execSync(cmd, opts); } /** diff --git a/test/tmux-capture-full-history.test.ts b/test/tmux-capture-full-history.test.ts index b9b721f6..cc927011 100644 --- a/test/tmux-capture-full-history.test.ts +++ b/test/tmux-capture-full-history.test.ts @@ -10,13 +10,14 @@ */ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { formatCursorRestore, formatPaneSnapshot, hasVisibleContent, queryPaneCursor, splitFrameAndCursor, + TmuxManager, } from '../src/tmux-manager.js'; describe('tmux full-history pane capture (COD-47)', () => { @@ -282,9 +283,9 @@ describe('the visible frame and its caret come from one tmux invocation', () => // after input): the frame could predate the caret, and guarded stage refused it. // Two backslashes in the source: one escaped backslash in the shell command, which hands tmux its `;`. const chained = '${captureCommand} ' + '\\'.repeat(2) + '; ${cursorQuery}'; - expect(methodBody).toContain('fullHistory ? null : this.captureFrameWithCursor(`' + chained + '`)'); - // Any doubt falls back to the two calls the capture always made. - expect(methodBody).toContain('combined?.frame ?? execSync(captureCommand, execOpts)'); + expect(methodBody).toContain('exec(`' + chained + '`'); + // Output it cannot split falls back to the two calls the capture always made. + expect(methodBody).toContain('combined?.frame ?? exec(captureCommand, execOpts)'); expect(methodBody).toContain('combined\n ? combined.cursor'); }); it('splits the combined output into the rows and the cursor line tmux prints last', () => { @@ -300,3 +301,54 @@ describe('the visible frame and its caret come from one tmux invocation', () => expect(splitFrameAndCursor('')).toBeNull(); }); }); + +describe('how many tmux calls one visible-frame capture makes', () => { + type Run = (cmd: string, opts: { encoding: 'utf-8'; timeout: number; maxBuffer?: number }) => string; + class ScriptedTmux extends TmuxManager { + calls: string[] = []; + constructor(private readonly answer: (cmd: string) => string) { + super(); + } + protected override captureRunner(): Run { + return (cmd) => { + this.calls.push(cmd.includes(' \\; ') ? 'combined' : cmd.includes('display-message') ? 'cursor' : 'capture'); + return this.answer(cmd); + }; + } + } + const quiet = () => vi.spyOn(console, 'error').mockImplementation(() => {}); + + it('makes ONE call when the combined output splits', () => { + const tmux = new ScriptedTmux(() => '› hello\n footer\n7 0 100 20 0 0\n'); + expect(tmux.capturePaneBuffer('codeman-12345678')).toContain('hello'); + expect(tmux.calls).toEqual(['combined']); + }); + + it('does not rerun a combined call that threw: a hung tmux costs one timeout, not two', () => { + const errors = quiet(); + try { + const tmux = new ScriptedTmux(() => { + throw Object.assign(new Error('spawnSync /bin/sh ETIMEDOUT'), { code: 'ETIMEDOUT' }); + }); + expect(tmux.capturePaneBuffer('codeman-12345678')).toBeNull(); + expect(tmux.calls).toEqual(['combined']); + expect(errors).toHaveBeenCalledTimes(1); + } finally { + errors.mockRestore(); + } + }); + + it('falls back to the two separate calls only for output it cannot split', () => { + const tmux = new ScriptedTmux((cmd) => + cmd.includes(' \\; ') ? '› hello\n footer\n' : cmd.includes('display-message') ? '7 0 100 20 0 0\n' : '› hello\n' + ); + expect(tmux.capturePaneBuffer('codeman-12345678')).toContain('hello'); + expect(tmux.calls).toEqual(['combined', 'capture', 'cursor']); + }); + + it('keeps the full-history path on its two calls', () => { + const tmux = new ScriptedTmux((cmd) => (cmd.includes('display-message') ? '0 0 80 24 0 0\n' : 'row\n')); + tmux.capturePaneBuffer('codeman-12345678', undefined, { fullHistory: true }); + expect(tmux.calls).toEqual(['capture', 'cursor']); + }); +});