diff --git a/docs/users/features/code-review.md b/docs/users/features/code-review.md index 3a6f574be50..92d1279bcfd 100644 --- a/docs/users/features/code-review.md +++ b/docs/users/features/code-review.md @@ -231,6 +231,8 @@ Confirmed findings are canonicalized into `.qwen/tmp/qwen-review--findin The command validates on write: a duplicate id, a finding with no failure scenario, an empty locations array, or an unknown severity is an error rather than a silently mangled entry. +Where do the images come from? For terminal-rendering claims, `qwen review capture-tui` drives the code under review in a **private tmux server** (it cannot touch your own tmux sessions), captures the pane bytes as `.ans`, and renders a `.png` via `freeze` when installed — degrading explicitly (`png` → `ans-only` → refused) and recording which rung it reached, because a verifier must say whether its verdict stands on pixels, bytes, or prose. + ## Evidence Images in PR Comments GitHub's API cannot attach images to review comments, so `/review` can host evidence images (TUI screenshots, rendered-output comparisons) in a repository you designate and embed them by URL: diff --git a/packages/cli/src/commands/review.test.ts b/packages/cli/src/commands/review.test.ts index 8da040a788e..6d5c789f31d 100644 --- a/packages/cli/src/commands/review.test.ts +++ b/packages/cli/src/commands/review.test.ts @@ -44,6 +44,7 @@ describe('reviewCommand', () => { 'match-remote', 'fetch-pr', 'capture-local', + 'capture-tui', 'plan-diff', 'repo-context', 'pr-context', diff --git a/packages/cli/src/commands/review.ts b/packages/cli/src/commands/review.ts index e027ae9459d..860bbb83e4a 100644 --- a/packages/cli/src/commands/review.ts +++ b/packages/cli/src/commands/review.ts @@ -15,6 +15,7 @@ import { composeReviewCommand } from './review/compose-review.js'; import { findingsCommand } from './review/findings.js'; import { fetchPrCommand } from './review/fetch-pr.js'; import { captureLocalCommand } from './review/capture-local.js'; +import { captureTuiCommand } from './review/capture-tui.js'; import { planDiffCommand } from './review/plan-diff.js'; import { repoContextCommand } from './review/repo-context.js'; import { prContextCommand } from './review/pr-context.js'; @@ -51,6 +52,7 @@ export const reviewCommand: CommandModule = { .command(matchRemoteCommand) .command(fetchPrCommand) .command(captureLocalCommand) + .command(captureTuiCommand) .command(planDiffCommand) .command(repoContextCommand) .command(prContextCommand) @@ -78,7 +80,7 @@ export const reviewCommand: CommandModule = { .command(cleanupCommand) .demandCommand( 1, - 'Specify a subcommand: run, parse-args, match-remote, fetch-pr, capture-local, plan-diff, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.', + 'Specify a subcommand: run, parse-args, match-remote, fetch-pr, capture-local, capture-tui, plan-diff, repo-context, pr-context, comment-status, load-rules, agent-prompt, build-test, base-tree, test-delta, drive, mock-provider, extract-step, script-lint, resolve-anchors, check-coverage, cost-ledger, presubmit, test-efficacy, test-plan, findings, publish-assets, compose-review, save-artifact, submit, or cleanup.', ) .version(false), handler: () => { diff --git a/packages/cli/src/commands/review/agent-prompt.test.ts b/packages/cli/src/commands/review/agent-prompt.test.ts index 47695e518b8..33cc8abff65 100644 --- a/packages/cli/src/commands/review/agent-prompt.test.ts +++ b/packages/cli/src/commands/review/agent-prompt.test.ts @@ -2599,6 +2599,13 @@ describe('verify and reverse-audit briefs — the Step 4/5 methodology, in code' expect(p).toContain('go read the claimed source first'); }); + it('wires capture-tui into the verify brief — the producer its rendering claims cite', () => { + // The brief teaches terminal-rendering claims to run `review capture-tui`; + // if the block drops out of the prompt the command exists with no consumer. + const p = buildRoleBrief(PLAN, 'verify'); + expect(p).toContain('"${QWEN_CODE_CLI:-qwen}" review capture-tui'); + }); + it('the verify brief is a verdict role: Exclusion Criteria yes, finding format no', () => { const p = buildRoleBrief(PLAN, 'verify'); expect(p).toContain('What is NOT a finding'); // the Exclusion Criteria heading diff --git a/packages/cli/src/commands/review/capture-tui.test.ts b/packages/cli/src/commands/review/capture-tui.test.ts new file mode 100644 index 00000000000..4b01e65c7ea --- /dev/null +++ b/packages/cli/src/commands/review/capture-tui.test.ts @@ -0,0 +1,2997 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + statSync, + writeFileSync, + symlinkSync, + lstatSync, +} from 'node:fs'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { tmpdir } from 'node:os'; +import { + captureTuiCommand, + freezeRender, + MATCH_BUDGET_MS, + holderInit, + probeBudget, + probes, + REAP_SIGNALS, + runCaptureTui, + tmuxControl, +} from './capture-tui.js'; +import { + tmuxSupportsCaptureN, + tmuxPadsWithCaptureN, +} from './lib/tui-capture.js'; + +const tmuxVersionProbe = spawnSync('tmux', ['-V'], { + encoding: 'utf8', + // Same belt as production probeOutput: a hanging shimmed binary here + // blocks the whole file at import time with no red test naming the cause. + timeout: 10_000, + killSignal: 'SIGKILL', +}); +// The suite needs capture-pane -N (tmux 3.1+); on an older tmux every +// capture would refuse with "too old", which is a skip-shaped outcome, not +// a red suite. +const hasTmux = + tmuxVersionProbe.status === 0 && + tmuxSupportsCaptureN(tmuxVersionProbe.stdout ?? '') !== false; +// --help, not --version: freeze <=0.1.6 has no --version flag and would be +// misdiagnosed as absent (mirrors the production probe). +const hasFreeze = + spawnSync('freeze', ['--help'], { timeout: 10_000, killSignal: 'SIGKILL' }) + .status === 0; +// The server-death and signal probes need pgrep; without it they would parse +// pid 0 and fail red on healthy code. error === undefined distinguishes +// "binary absent" from "no match" (a --version gate would misfire on BSD +// pgrep, which has none). +const hasPgrep = + spawnSync('pgrep', ['-f', 'no-such-process-anywhere'], { + timeout: 10_000, + killSignal: 'SIGKILL', + }).error === undefined; + +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +/** Capture the stdio written during `fn` — the refusal REASON is part of + * the contract, not just the exit code: two different refusal paths share + * the exit-3/no-artifacts shape, and only the reason tells them apart; and + * an agent consumer parses the refusal JSON from stdout, not stderr. */ +async function withStdio( + fn: () => Promise, +): Promise<{ stdout: string; stderr: string }> { + const sinks = { stdout: '', stderr: '' }; + const capture = (stream: 'stdout' | 'stderr') => + vi.spyOn(process[stream], 'write').mockImplementation((( + chunk: string | Uint8Array, + ) => { + sinks[stream] += + typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString(); + return true; + }) as never); + const outSpy = capture('stdout'); + const errSpy = capture('stderr'); + try { + await fn(); + } finally { + outSpy.mockRestore(); + errSpy.mockRestore(); + } + return sinks; +} + +// The no-tmux refusal fires exactly where the real-tmux block below is +// skipped, so it gets its own suite that runs EVERYWHERE, driving the probe +// seam instead of the real binary: a refactor that inverts the probe must +// fail here, not surface as a raw ENOENT on some tmux-less host. +describe('capture-tui without tmux (probe seam)', () => { + const realTmux = probes.tmux; + beforeEach(() => { + process.exitCode = undefined; + }); + afterEach(() => { + probes.tmux = realTmux; + process.exitCode = undefined; + }); + + it('refuses with the contract — exit 3, no artifacts, the RIGHT reason', async () => { + probes.tmux = () => ({ status: 'absent' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-notmux-')); + try { + const { stdout, stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + // The reason pins the PATH taken: an inverted probe would fall through + // to the mid-capture catch and say "tmux failed mid-capture" instead. + expect(stderr).toContain('tmux is not installed'); + // The refusal JSON rides on stdout too: an agent consumer must not + // have to scrape stderr to tell WHY the ladder stopped at none. + expect(JSON.parse(stdout.trim())).toEqual({ + captured: false, + evidence: 'none', + reason: expect.stringContaining('tmux is not installed'), + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('refuses a tmux too old for capture-pane -N, naming the version', async () => { + // -N landed in tmux 3.1; an older host passes -V and would otherwise + // die MID-capture on the unknown flag — blaming tmux for a version + // problem, after paying for a server start. + probes.tmux = () => ({ status: 'ok', out: 'tmux 2.8' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-oldtmux-')); + try { + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('tmux 2.8 is too old'); + expect(stderr).toContain('capture-pane -N'); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('leaves NO stale artifacts when a re-run refuses', async () => { + // The previous run's artifacts cannot survive a refused re-run: a stale + // manifest claiming a png rung whose .ans no longer exists is exactly + // the wrong-evidence failure this command exists to prevent. On POSIX + // the fake tmux passes the version probe and fails every real command + // (a MID-capture refusal); on win32 the shim is unreachable, the probe + // answers {status:'absent'}, and the refusal is the no-tmux one — BOTH + // land after the up-front clear, so the assertions pin the clear on + // every platform. + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-stale-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + writeFileSync(join(dir, 'cap.holder-ready'), ''); + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + writeFileSync( + join(binDir, 'tmux'), + '#!/bin/sh\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\nfor a in "$@"; do [ "$a" = "kill-server" ] && { echo "no server running on /tmp/x" >&2; exit 1; }; done\necho "fake tmux: refusing" >&2\nexit 1\n', + { mode: 0o755 }, + ); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + try { + await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + // The writability probe uses a unique sibling and removes it — it + // must not outlive the run either. + expect(readdirSync(dir).filter((f) => f.includes('write-probe'))).toEqual( + [], + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('clears stale artifacts even when the refusal is PRE-capture', async () => { + // The clear must precede every gate, not just the mid-capture ones: a + // refactor moving it below the validation chain leaves the previous + // run's png-claiming manifest next to a typo'd-flag refusal. + // A REAL-looking probe so the run reaches the --until compile gate its + // title claims (with the probe undefined, the no-tmux refusal fired + // first and every later gate stayed unpinned for the clear). + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-staleearly-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + writeFileSync(join(dir, 'cap.holder-ready'), ''); + await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: '[', + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32')( + 'refuses through the REAL probe when tmux is absent from PATH', + async () => { + // Every other seam test overrides probes.tmux; this one leaves the + // real probe in place and empties PATH — a probeOutput regression + // that stops distinguishing status!=0 would otherwise ship green and + // misdiagnose an absent tmux as "tmux failed mid-capture". + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-realprobe-')); + const emptyBin = join(dir, 'emptybin'); + mkdirSync(emptyBin, { recursive: true }); + const realPath = process.env['PATH']; + process.env['PATH'] = emptyBin; + let stderr = ''; + try { + ({ stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + )); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + rmSync(dir, { recursive: true, force: true }); + } + expect(process.exitCode).toBe(3); + expect(stderr).toContain('tmux is not installed'); + expect(stderr).not.toContain('mid-capture'); + }, + ); + + it('leaves UNRELATED files at the artifact paths alone on refusal', async () => { + // The artifact names are not reserved: a colliding --out must not + // force-delete unrelated files on a run that refuses (measured shape: + // --out package → the --cols 0 refusal deleted package.json). The + // clear keys on the manifest's evidence rung — a JSON that is not a + // capture manifest leaves its sibling files untouched too. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-unrelated-')); + try { + writeFileSync( + join(dir, 'cap.json'), + '{"name":"not-a-manifest","version":"1.0.0"}', + ); + writeFileSync(join(dir, 'cap.ans'), 'user file'); + writeFileSync(join(dir, 'cap.png'), 'user file'); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 0, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + // The refusal is now the COLLISION itself, taken before anything + // starts: a file the clear phase verified is not a capture artifact + // is not ours to replace, and a successful run used to rewrite it. + expect(stderr).toContain('collides with a file this capture did not'); + expect(readFileSync(join(dir, 'cap.json'), 'utf8')).toBe( + '{"name":"not-a-manifest","version":"1.0.0"}', + ); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toBe('user file'); + expect(readFileSync(join(dir, 'cap.png'), 'utf8')).toBe('user file'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('clears stale artifacts BEFORE the sentinel unlink can refuse', async () => { + // The sentinel unlink is the one clear that may THROW (a directory + // there gives EISDIR, which `force` does not suppress) and its throw + // refuses. Ordered first, it stranded the previous run's artifacts — + // manifest claiming "evidence":"png" — beside the refusal JSON, the + // wrong-evidence outcome the clear-first contract exists to prevent. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-sentinelorder-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + mkdirSync(join(dir, 'cap.holder-ready')); + writeFileSync(join(dir, 'cap.holder-ready', 'user-file'), 'not ours'); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('not writable'); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + // Fail-closed preserved: the user's directory is still theirs. + expect(statSync(join(dir, 'cap.holder-ready')).isDirectory()).toBe(true); + expect(existsSync(join(dir, 'cap.holder-ready', 'user-file'))).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('clears stale artifacts even when a SHAPE guard refuses', async () => { + // The measured R4-1 regression: an array-shaped --command refused at + // the shape guard BEFORE the clear, leaving a stale evidence:"png" + // manifest next to the refusal. The clear must precede the shape + // guards too — only an unnameable --out refuses without clearing. + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-staleshape-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + writeFileSync(join(dir, 'cap.holder-ready'), ''); + await withStdio(() => + runCaptureTui({ + command: ['a', 'b'], + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32')( + 'cuts a HANGING availability probe with the belt — absent, not stuck', + async () => { + // A tmux -V that hangs would otherwise block before the refusal + // contract or any signal handler exists; through the seam the belt is + // provable — the hardcoded-timeout mutant hangs past the wall bound. + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-hangprobe-')); + const binDir = join(dir, 'bin'); + mkdirSync(binDir, { recursive: true }); + // /bin/sleep by absolute path: PATH is binDir alone below, so a bare + // `sleep` would ENOENT and the shim would EXIT instantly instead of + // hanging — the test then never exercised the belt at all. + // TERM-immune, like the measured wedge: without killSignal SIGKILL + // the belt only SENDS a TERM this shim ignores, and the spawn blocks + // past any deadline — the SIGKILL half of the belt is what this pins. + writeFileSync( + join(binDir, 'tmux'), + "#!/bin/sh\ntrap '' TERM\n/bin/sleep 30\n", + { + mode: 0o755, + }, + ); + const realPath = process.env['PATH']; + const realBudget = probeBudget.timeoutMs; + process.env['PATH'] = binDir; + probeBudget.timeoutMs = 500; + const started = performance.now(); + let stderr = ''; + try { + ({ stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + )); + } finally { + probeBudget.timeoutMs = realBudget; + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + rmSync(dir, { recursive: true, force: true }); + } + expect(process.exitCode).toBe(3); + // A belt-killed probe is WEDGED, not absent — the refusal must not + // send an operator to reinstall a binary that exists. + expect(stderr).toContain('present but wedged'); + expect(stderr).not.toContain('not installed'); + const elapsed = performance.now() - started; + // Floor proves the shim actually hung to the belt; the tight ceiling + // kills a hardcoded 10s mutant. + expect(elapsed).toBeGreaterThanOrEqual(450); + expect(elapsed).toBeLessThan(2_500); + }, + ); + + it('clears stale artifacts when the ENVIRONMENT refuses — absent, hung, too old', async () => { + // The clear-first contract has ordering pins for the sentinel, the + // shape guards, the marker gate and the directory-shaped --out — but + // every one of them runs with an OK probe, so a regression that let the + // probe-refusal family bypass the clear shipped green while a stale + // manifest claiming "evidence":"png" sat beside the refusal JSON. + for (const [name, probe] of [ + ['absent', () => ({ status: 'absent' }) as const], + ['hung', () => ({ status: 'hung' }) as const], + ['too old', () => ({ status: 'ok', out: 'tmux 3.0a' }) as const], + ] as const) { + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-probestale-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + probes.tmux = probe; + await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + // The probe name rides IN the compared value: `expect(v, message)` + // is banned by vitest/valid-expect, and a bare false would not say + // which of the three refusals regressed. + expect({ + probe: name, + exitCode: process.exitCode, + ans: existsSync(join(dir, 'cap.ans')), + png: existsSync(join(dir, 'cap.png')), + manifest: existsSync(join(dir, 'cap.json')), + }).toEqual({ + probe: name, + exitCode: 3, + ans: false, + png: false, + manifest: false, + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + process.exitCode = undefined; + } + } + }); + + it('leaves an occupant BYTE-FOR-BYTE intact when it refuses', async () => { + // What pins this now is the collision gate, not a per-path write probe: + // that probe is gone (an append-mode open passes on a `chattr +a` file + // the truncating final write then fails on), and the gate refuses on + // any occupant before anything opens it. Content, not existence — a + // regression that truncates on the way to refusing stays green + // otherwise. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-notrunc-')); + try { + writeFileSync(join(dir, 'cap.json'), '{"name":"not-a-manifest"}'); + writeFileSync(join(dir, 'cap.ans'), 'user bytes'); + await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 0, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(readFileSync(join(dir, 'cap.json'), 'utf8')).toBe( + '{"name":"not-a-manifest"}', + ); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toBe('user bytes'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'refuses an unwritable MANIFEST path too, not just the .ans', + async () => { + // The probe loop covers both paths; dropping manifestPath from it + // passed the whole file. The brief template's `--out package` against + // a stage that left package.json mode-0444 is exactly this shape: + // every gate passes, the capture runs, and the manifest write fails + // at the very end with the pane text already thrown away. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-romanifest-')); + try { + writeFileSync(join(dir, 'cap.json'), '{"name":"not-a-manifest"}', { + mode: 0o444, + }); + const started = performance.now(); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 5_000, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 30_000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('collides with a file this capture did not'); + expect(stderr).toContain('cap.json'); + expect(performance.now() - started).toBeLessThan(3_000); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + + it('clears stale artifacts for the SHAPE-BOUNDS gate family too', async () => { + // Seven refusal gates have seeded-artifact ordering pins; the family + // between the probe gates and the marker gate — geometry bounds, an + // empty --command, a non-enterable --cwd, the settle/timeout bounds — + // had none, so hoisting any of them above the clear block shipped green + // while a stale manifest claiming "evidence":"png" survived beside the + // refusal. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + // The REASON, not just exit 3: without it, deleting a gate outright + // ships green — the run sails on and refuses for some other reason + // (a tmux-less lane refuses 'not installed'), which is still exit 3 + // with the artifacts cleared. The pin has to name the gate it pins. + for (const [name, over, reason] of [ + ['geometry', { rows: 9999 }, '--rows must'], + ['empty command', { command: ' ' }, '--command must not be empty'], + [ + 'cwd', + { cwd: join(tmpdir(), 'capture-tui-nope-does-not-exist') }, + '--cwd', + ], + ['settle bound', { settleMs: -1 }, '--settle-ms'], + ] as ReadonlyArray, string]>) { + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-boundstale-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + rows: 24, + ...(over as Record), + } as never), + ); + // The gate name rides IN the compared value: expect(v, message) is + // banned by vitest/valid-expect, and a bare miss would not say + // which gate stopped naming itself. + expect({ gate: name, named: stderr.includes(reason) }).toEqual({ + gate: name, + named: true, + }); + expect({ + gate: name, + exitCode: process.exitCode, + ans: existsSync(join(dir, 'cap.ans')), + png: existsSync(join(dir, 'cap.png')), + manifest: existsSync(join(dir, 'cap.json')), + }).toEqual({ + gate: name, + exitCode: 3, + ans: false, + png: false, + manifest: false, + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + process.exitCode = undefined; + } + } + }); + + it.skipIf(process.platform === 'win32')( + 'refuses a FIFO at an artifact path instead of blocking on it', + async () => { + // Reading a FIFO blocks until a writer appears — a HANG, not a throw, + // so no refusal is printed and no reap handler is installed yet. Run + // from a CHILD with a kill deadline: the block is a synchronous read + // on the main thread, so an in-process timeout cannot interrupt it + // (measured — a regression wedged the whole vitest run past its own + // 10s test timeout), and only an external killer turns it red. + let captureTuiTs = join( + process.cwd(), + 'src/commands/review/capture-tui.ts', + ); + if (!existsSync(captureTuiTs)) { + captureTuiTs = join( + process.cwd(), + 'packages/cli/src/commands/review/capture-tui.ts', + ); + } + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-fifo-')); + try { + spawnSync('mkfifo', [join(dir, 'cap.json')]); + if (!existsSync(join(dir, 'cap.json'))) return; // no mkfifo here + const driver = join(dir, 'driver-fifo.mts'); + writeFileSync( + driver, + [ + `const mod = await import(${JSON.stringify(pathToFileURL(captureTuiTs).href)});`, + `mod.probes.tmux = () => ({ status: 'absent' });`, + `await mod.runCaptureTui({ command: 'printf hi', cwd: ${JSON.stringify(dir)}, cols: 80, rows: 24, settleMs: 0, until: undefined, keys: undefined, out: ${JSON.stringify(join(dir, 'cap'))}, timeoutMs: 1000 } as never);`, + ].join('\n'), + ); + const { spawn } = await import('node:child_process'); + const child = spawn(process.execPath, ['--import', 'tsx', driver], { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'pipe'], + }); + let out = ''; + child.stdout.on('data', (b: Buffer) => (out += b.toString())); + child.stderr.on('data', (b: Buffer) => (out += b.toString())); + const killer = setTimeout(() => child.kill('SIGKILL'), 20_000); + const code = await new Promise((resolve) => + child.once('exit', (c) => resolve(c)), + ); + clearTimeout(killer); + // Exit 3 with the collision named — not a SIGKILL'd hang. + expect(code).toBe(3); + expect(out).toContain('collides with a file this capture did not'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + 60_000, + ); + + it('refuses a BARE --keys — no tokens is a template that drove nothing', async () => { + // yargs `array: true` turns `--keys` (bare), `--keys=` and an unquoted + // `--keys $EMPTY` into [], which was accepted silently: nothing typed, + // success reported, while the QUOTED form of the same template failure + // was refused. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-barekeys-')); + try { + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: [], + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('no tokens'); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('refuses an EMPTY --keys token — a keypress that never happens', async () => { + // `send-keys ''` types nothing, so the run reported success with the + // token in manifest.keys and keysSent true — a keypress a verdict can + // cite that never happened. A brief template expanding an empty + // variable produces exactly this token. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-emptykey-')); + try { + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: [''], + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('empty token'); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'refuses a READ-ONLY file at an artifact path before the capture window', + async () => { + // The sibling write probe proves the DIRECTORY writable, not the + // artifact paths. A mode-0444 (or foreign-owned, the shape a shared + // CI stage leaves) .ans passed every gate and refused only at the + // final write — after the whole settle/timeout window and a render, + // with the pane text produced and thrown away. Skipped as root, who + // writes through the mode bits. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-roartifact-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'not writable by us', { + mode: 0o444, + }); + const started = performance.now(); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 5_000, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 30_000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('collides with a file this capture did not'); + // BEFORE the window: the 5s settle never ran. + expect(performance.now() - started).toBeLessThan(3_000); + // And the file is still the user's, untouched. + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toBe( + 'not writable by us', + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + + it('SKIPS a directory squatting at an artifact path and clears the rest', async () => { + // A capture writes files, so a DIRECTORY at an artifact path is someone + // else's — the recursive EISDIR fallback deleted it and its contents on + // every re-run against the same --out (a stale shaped manifest is the + // normal state from the second run on). The directory survives; the + // unlink's throw must not abort the clear of the OTHER paths either. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-eisdir-')); + try { + mkdirSync(join(dir, 'cap.ans')); + writeFileSync(join(dir, 'cap.ans', 'user-file'), 'not ours'); + writeFileSync(join(dir, 'cap.png'), 'stale png'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + // The undeletable path is also occupied, so the collision gate + // refuses UP FRONT naming it — not after a full capture window at + // the final write. + expect(stderr).toContain('collides with a file this capture did not'); + expect(stderr).toContain('cap.ans'); + expect(statSync(join(dir, 'cap.ans')).isDirectory()).toBe(true); + expect(existsSync(join(dir, 'cap.ans', 'user-file'))).toBe(true); + // The throw on the directory does not strand the other stale evidence. + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + // skipIf(win32) like every shim-dependent sibling: an extensionless + // shebang script is not spawnable via CreateProcess and PATH joins with + // ';' there, so the real probe answers 'absent', the run refuses + // 'tmux is not installed' before either gate, and the call-log + // assertions pass VACUOUSLY — pinning nothing on the windows lane. + it.skipIf(process.platform === 'win32')( + 'starts NO process before the marker gates refuse — pinned by call log', + async () => { + // Location-invariant assertions could not see a mutant that moved the + // --until compile below plan.start: the refusal looked identical while + // a real private server ran the user's command. The call log can. + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-order-')); + try { + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + const callLog = join(dir, 'tmux-calls'); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\necho "$*" >> "${callLog}"\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + const realTmuxProbe = probes.tmux; + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + try { + await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: '[', + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 1000, + } as never), + ); + } finally { + probes.tmux = realTmuxProbe; + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + expect(process.exitCode).toBe(3); + const calls = existsSync(callLog) ? readFileSync(callLog, 'utf8') : ''; + expect(calls).not.toContain('new-session'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); + + it('refuses non-string argv shapes before anything else', async () => { + // yargs parses duplicated options into arrays and --no-X into booleans; + // both must refuse, not throw uncaught or silently corrupt the capture. + // Undefined required options are the exported-function vector of the + // same class: demandOption covers the CLI path only. + probes.tmux = () => ({ status: 'absent' }) as const; // never reached — shapes refuse first + // A test-owned out, not '/tmp/never-written': the hardcoded path + // routed most iterations through the mkdir+probe block before the + // guards under test, and on Windows resolve() lands it at the drive + // root (a stray :\tmp on admin lanes, EPERM on the others). + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-shapes-')); + try { + const base = { + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'never-written'), + timeoutMs: 1000, + }; + for (const [over, flag] of [ + [{ command: ['a', 'b'] }, '--command'], // --command A --command B + [{ command: false }, '--command'], // --no-command + [{ command: undefined }, '--command'], + [{ command: 'x', until: ['A', 'B'] }, '--until'], // --until A --until B + [{ command: 'x', keys: [false] }, '--keys'], // --keys false + [{ command: 'x', keys: false }, '--keys'], // --no-keys (boolean) + [{ command: 'x', keys: 'Enter' }, '--keys'], // bare string + [{ command: 'x', out: ['x', 'y'] }, '--out'], + [{ command: 'x', out: undefined }, '--out'], + [{ command: 'x', ready: ['A', 'B'] }, '--ready'], // --ready A --ready B + [{ command: 'x', cwd: ['a', 'b'] }, '--cwd'], + ] as const) { + process.exitCode = undefined; + const { stderr } = await withStdio(() => + runCaptureTui({ ...base, ...over } as never), + ); + expect(process.exitCode).toBe(3); + // The FLAG NAME, not just the shared word 'must': a label↔value + // swap in production's guard loop misnames the offending flag in + // the machine-parsed refusal JSON, sending an agent consumer to + // fix a flag it never duplicated (measured: with --until/--ready + // labels swapped, a duplicated --until blamed --ready). + expect(stderr).toContain(`${flag} must`); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it.skipIf(process.platform === 'win32')( + 'keeps unverifiable artifacts when the WRITE PROBE itself fails', + async () => { + // Fd exhaustion means the manifest CANNOT be read, so the capture + // signature cannot be verified — and unverified is not permission to + // delete: the artifact names are not reserved, so what sits there may + // be the user's. This run refuses; the files stay. (The sentinel is + // this tool's alone and clears unconditionally.) + // + // Driven from a CHILD, and that is not incidental: vitest runs test + // files in worker THREADS that share one process fd table, so + // exhausting it in-process starves whatever else happens to be + // running — measured, this test passed alone 3/3 while the full + // review suite failed here and timed out an unrelated hadolint test + // in the same run. A child contains the blast radius, and real + // exhaustion is still what drives the EMFILE (vi.spyOn on node:fs + // does not reach this module's named imports). + let captureTuiTs = join( + process.cwd(), + 'src/commands/review/capture-tui.ts', + ); + if (!existsSync(captureTuiTs)) { + captureTuiTs = join( + process.cwd(), + 'packages/cli/src/commands/review/capture-tui.ts', + ); + } + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-staleprobe-')); + try { + writeFileSync(join(dir, 'cap.ans'), 'old run'); + writeFileSync(join(dir, 'cap.png'), 'old run'); + writeFileSync(join(dir, 'cap.json'), '{"evidence":"png"}'); + writeFileSync(join(dir, 'cap.holder-ready'), ''); + const driver = join(dir, 'driver-emfile.mts'); + writeFileSync( + driver, + [ + `const { openSync, writeFileSync } = await import('node:fs');`, + `const mod = await import(${JSON.stringify(pathToFileURL(captureTuiTs).href)});`, + `mod.probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' });`, + `const fdSource = ${JSON.stringify(join(dir, 'fd-source'))};`, + `writeFileSync(fdSource, 'x');`, + `for (;;) { try { openSync(fdSource, 'r'); } catch { break; } }`, + `await mod.runCaptureTui({ command: 'printf hi', cwd: undefined, cols: 80, rows: 24, settleMs: 0, until: undefined, keys: undefined, out: ${JSON.stringify(join(dir, 'cap'))}, timeoutMs: 1000 } as never);`, + ].join('\n'), + ); + const { spawn } = await import('node:child_process'); + const child = spawn(process.execPath, ['--import', 'tsx', driver], { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'pipe'], + }); + let out = ''; + child.stdout.on('data', (b: Buffer) => (out += b.toString())); + child.stderr.on('data', (b: Buffer) => (out += b.toString())); + const code = await new Promise((resolve) => + child.once('exit', (c) => resolve(c)), + ); + expect(code).toBe(3); + expect(out).toContain('not writable'); + // Unverifiable is not permission to delete. + expect(existsSync(join(dir, 'cap.ans'))).toBe(true); + expect(existsSync(join(dir, 'cap.png'))).toBe(true); + expect(existsSync(join(dir, 'cap.json'))).toBe(true); + // The sentinel is plumbing this tool alone writes — cleared even + // here, by design, outside the signature guard. + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + 60_000, + ); + it('holds the exit-3 contract when the stdout reader is GONE — EPIPE-proof', async () => { + // withStdio mocks the streams, so no in-process test can raise a real + // EPIPE; a child whose stdout pipe closes early can. Without the guard + // the refusal crashed on the async 'error' event and exited 1. + let captureTuiTs = join( + process.cwd(), + 'src/commands/review/capture-tui.ts', + ); + if (!existsSync(captureTuiTs)) { + captureTuiTs = join( + process.cwd(), + 'packages/cli/src/commands/review/capture-tui.ts', + ); + } + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-epipe-')); + try { + const driver = join(dir, 'driver-epipe.mts'); + writeFileSync( + driver, + [ + `const mod = await import(${JSON.stringify(pathToFileURL(captureTuiTs).href)});`, + `mod.probes.tmux = () => ({ status: 'absent' }) as never;`, + `// Give the pipe a beat to be closed by the parent first.`, + `await new Promise((r) => setTimeout(r, 300));`, + `await mod.runCaptureTui({ command: 'printf hi', cwd: undefined, cols: 80, rows: 24, settleMs: 0, until: undefined, keys: undefined, out: ${JSON.stringify(join(dir, 'cap'))}, timeoutMs: 1000 } as never);`, + ].join('\n'), + ); + const { spawn } = await import('node:child_process'); + const child = spawn(process.execPath, ['--import', 'tsx', driver], { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'pipe'], + }); + // Kill the reader immediately: the child's refusal write hits EPIPE. + child.stdout.destroy(); + const code = await new Promise((resolve) => + child.once('exit', (c) => resolve(c)), + ); + expect(code).toBe(3); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, 30_000); + + it('clears stale artifacts before the directory-shaped --out refusal too', async () => { + // The last nameable gate without an ordering pin: a mutant hoisting the + // isDirectory check above the clears left the previous run's manifest + // beside the refusal. + probes.tmux = () => ({ status: 'ok', out: 'tmux 3.9' }) as const; + const dir = mkdtempSync(join(tmpdir(), 'capture-tui-dirout-')); + try { + writeFileSync(join(dir, 'adir.ans'), 'old'); + writeFileSync(join(dir, 'adir.png'), 'old'); + writeFileSync(join(dir, 'adir.json'), '{"evidence":"png"}'); + mkdirSync(join(dir, 'adir')); + const { stderr } = await withStdio(() => + runCaptureTui({ + command: 'printf hi', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: undefined, + keys: undefined, + out: join(dir, 'adir'), + timeoutMs: 1000, + } as never), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('existing directory'); + expect(existsSync(join(dir, 'adir.ans'))).toBe(false); + expect(existsSync(join(dir, 'adir.json'))).toBe(false); + expect(existsSync(join(dir, 'adir'))).toBe(true); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('registers the full REAP set — a pure pin, gated on nothing', () => { + // The set check needs neither tmux nor pgrep; buried in a skipIf test + // it vanished on slim hosts — where dropping SIGHUP/SIGQUIT (a + // regression that shipped green once before) would ship green again. + expect([...REAP_SIGNALS].sort()).toEqual([ + 'SIGHUP', + 'SIGINT', + 'SIGQUIT', + 'SIGTERM', + ]); + }); +}); + +// The command boundary drives REAL tmux — a private-server capture the mocks +// cannot vouch for (the isolation property IS the exec shape). Skipped where +// tmux is absent; the pure plan shapes stay pinned in tui-capture.test.ts +// everywhere. +describe.skipIf(!hasTmux)('capture-tui (real tmux)', () => { + let dir: string; + // The probe seams are restored HERE, not per test: a test that fakes the + // version and forgets to put it back leaves every later capture believing + // it, and the plan then sends flags the real tmux may not have. That is + // exactly what happened — a leaked 'tmux 3.9' made 20 captures send -T on + // a runner whose tmux is 3.2a, and it was invisible on a dev machine + // whose tmux accepts -T. A hook cannot be forgotten. + const realTmuxProbe = probes.tmux; + const realFreezeProbe = probes.freeze; + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'capture-tui-')); + process.exitCode = undefined; + }); + afterEach(() => { + probes.tmux = realTmuxProbe; + probes.freeze = realFreezeProbe; + rmSync(dir, { recursive: true, force: true }); + process.exitCode = undefined; + }); + + function run(over: Record = {}): Promise { + return runCaptureTui({ + command: 'printf "HELLO-\\033[31mRED\\033[0m-WORLD\\n"; sleep 30', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + // Settle on CONTENT, not a fixed delay: under CI load a fixed delay + // races the shell's startup, captures a blank pane, and the ladder + // assertions turn flaky (measured once: empty .ans → freeze bounds + // error → 'png' expectation failed). + until: 'WORLD', + keys: undefined, + out: join(dir, 'cap'), + timeoutMs: 10_000, + ...over, + } as never); + } + + it('captures the real rendering into .ans and records the ladder honestly', async () => { + await run(); + expect(process.exitCode).toBeUndefined(); + const ans = readFileSync(join(dir, 'cap.ans'), 'utf8'); + expect(ans).toContain('HELLO-'); + expect(ans).toContain('WORLD'); + // The escapes survived (-e): the red text carries its SGR bytes. + expect(ans).toContain('[31m'); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(['png', 'ans-only']).toContain(manifest.evidence); + // Field-omission contract in the HAPPY shape: no keys were given, so + // keysSent must be absent (a keysSent=false initialization mutant would + // report "keys withheld" on a keys-less run); until was given, so + // settleMs must be absent. + expect(manifest.keysSent).toBeUndefined(); + expect(manifest.settleMs).toBeUndefined(); + if (hasFreeze) { + // A present-but-broken freeze (--help exits 0, render dies) degrades + // to ans-only BY CONTRACT — that is a designed rung, not a failure. + expect(['png', 'ans-only']).toContain(manifest.evidence); + if (manifest.evidence === 'png') { + expect(manifest.pngPath).toBe(join(dir, 'cap.png')); + expect(existsSync(join(dir, 'cap.png'))).toBe(true); + } else { + expect(manifest.degradedBecause).toContain('freeze'); + } + } else { + expect(manifest.degradedBecause).toContain('freeze'); + } + }); + + it('captures a command that renders and EXITS — the one-shot fixture case', async () => { + // Without the pane holder, tmux destroys the session the moment the + // command exits (remain-on-exit off) and the obtainable frame is lost + // with a misleading "no server running" refusal (measured: 0/10). + await run({ command: 'printf "FAST-DONE\\n"', until: 'FAST-DONE' }); + expect(process.exitCode).toBeUndefined(); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toContain('FAST-DONE'); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + }); + + it('matches --until across an SGR attribute change', async () => { + // On the physical (-e) frame the escape bytes sit inside the marker and + // it can never match; the logical matching view has no escapes. + await run({ + command: 'printf "AA\\033[31mBB\\033[0m-DONE\\n"; sleep 30', + until: 'AABB-DONE', + }); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + // The saved frame is still the physical one, escapes and all. + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toContain('[31m'); + }); + + it('matches --until across a wrap boundary', async () => { + // A 60-char marker in a 40-column pane wraps; only the joined (-J) + // matching view can see it whole. The .ans stays physical: two lines. + await run({ + command: `s=$(printf 'M%.0s' $(seq 1 60)); printf "%sEND\\n" "$s"; sleep 30`, + cols: 40, + until: 'M{60}END', + }); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + const ans = readFileSync(join(dir, 'cap.ans'), 'utf8'); + // Physical evidence: the marker is split across lines, as rendered. + expect(ans).not.toMatch(/M{60}END/); + expect(ans).toContain('END'); + }); + + it('survives a catastrophic-backtracking --until pattern', async () => { + // The deadline is only checked between test() calls; the vm budget + // interrupts a superlinear match so the poll keeps expiring on time. + // Monotonic clock for every wall bound in this suite: Date.now() can be + // stepped by NTP mid-test and read a wrong elapsed value either way. + const started = performance.now(); + await run({ + command: `printf 'a%.0s' $(seq 1 79); printf '\\n'; sleep 30`, + until: '(a+)+b', + timeoutMs: 1500, + }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('timeout'); + // The budget cutoff is RECORDED, not swallowed: a backtracking-prone + // marker may be present, and "never matched" alone would hide that the + // match was cut off rather than the marker absent. + expect(manifest.degradedBecause).toContain('exceeded its'); + // Bounded TIGHT: vitest's own testTimeout kills anything over 15s, so a + // 30s bound would have zero bite. Healthy runs measure ~2s; the budget + // VALUE itself is declaration-pinned in the defaults test. + expect(performance.now() - started).toBeLessThan(8_000); + }); + + it('leaves no tmux server behind — the isolation is also the cleanup', async () => { + // TMUX_TMPDIR under the test dir, restored after: standard CI lanes + // set no TMUX_TMPDIR, so without this the production TMUX_TMPDIR + // branch of the socket-dir resolution never runs there — a + // /tmp-hardcoding mutant ships green on those lanes, and on hosts that + // DO set the variable it unlinks in /tmp while tmux created the socket + // under $TMUX_TMPDIR/tmux-/ (measured: tmux honors the variable). + const tmuxTmp = join(dir, 'tmux-tmp'); + mkdirSync(tmuxTmp, { mode: 0o700 }); + const realTmuxTmpdir = process.env['TMUX_TMPDIR']; + process.env['TMUX_TMPDIR'] = tmuxTmp; + try { + await run(); + } finally { + if (realTmuxTmpdir === undefined) delete process.env['TMUX_TMPDIR']; + else process.env['TMUX_TMPDIR'] = realTmuxTmpdir; + } + // Any server this run created is named qwen-review-capture--…; + // asking it for sessions must fail because the server is gone. Probe + // the SAME dir production resolved — the TMUX_TMPDIR this test set — + // so a regression in that branch cannot pass this probe vacuously. + const base = tmuxTmp; + // Quote the dir and grep the names: interpolating ${base} unquoted into + // a glob makes this assertion pass VACUOUSLY whenever TMUX_TMPDIR + // carries whitespace (measured with a planted orphan in such a dir). + const probe = spawnSync('bash', [ + '-c', + `ls "${base}/tmux-$(id -u)" 2>/dev/null | grep "^qwen-review-capture-${process.pid}-" || true`, + ]); + // Binary absence must be loud: with no bash, stdout is undefined and the + // empty-string assertion below would pass while checking nothing. + expect(probe.error).toBeUndefined(); + expect((probe.stdout ?? Buffer.from('')).toString().trim()).toBe(''); + }); + + it.skipIf(!hasPgrep)( + 'kills the tmux SERVER itself — pid probed while it was alive', + async () => { + // The socket probe above cannot distinguish "server reaped" from "we + // unlinked a live server's socket" (the cleanup unlinks it either way). + // This pins server DEATH: grab the server's pid mid-capture, then + // assert the process is gone after the run. + const inFlight = run({ until: 'NEVER-MATCHES', timeoutMs: 3000 }); + let serverPid = 0; + for (let i = 0; i < 100 && !serverPid; i++) { + const r = spawnSync( + 'pgrep', + ['-f', `qwen-review-capture-${process.pid}-`], + { encoding: 'utf8' }, + ); + const pid = Number((r.stdout ?? '').trim().split('\n')[0]); + if (Number.isInteger(pid) && pid > 1) serverPid = pid; + else await sleep(50); + } + await inFlight; + expect(serverPid).toBeGreaterThan(1); + let alive = true; + for (let i = 0; i < 40 && alive; i++) { + try { + process.kill(serverPid, 0); + await sleep(50); + } catch { + alive = false; + } + } + expect(alive).toBe(false); + }, + ); + + it('kills the processes the capture started — not just the socket file', async () => { + const pidFile = join(dir, 'shell.pid'); + await run({ + command: `echo $$ > "${pidFile}"; printf "PIDDED\\n"; sleep 30`, + until: 'PIDDED', + }); + const pid = Number(readFileSync(pidFile, 'utf8').trim()); + expect(Number.isInteger(pid) && pid > 1).toBe(true); + // kill-server delivers the reap asynchronously; give it a beat. + let alive = true; + for (let i = 0; i < 40 && alive; i++) { + try { + process.kill(pid, 0); + await sleep(50); + } catch { + alive = false; + } + } + expect(alive).toBe(false); + }); + + it('refuses mid-capture tmux failure with the contract, not a stack trace', async () => { + // A fake tmux that answers -V but fails every real command models the + // "probe passes, session fails" host (ancient tmux, unwritable socket + // dir). The catch must land on the refusal contract. + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + writeFileSync( + join(binDir, 'tmux'), + '#!/bin/sh\n[ "$1" = "-V" ] && exit 0\nfor a in "$@"; do [ "$a" = "kill-server" ] && { echo "no server running on /tmp/x" >&2; exit 1; }; done\necho "fake tmux: refusing" >&2\nexit 1\n', + { mode: 0o755 }, + ); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + let stderr = ''; + try { + ({ stderr } = await withStdio(() => run())); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + expect(process.exitCode).toBe(3); + expect(stderr).toContain('tmux failed mid-capture'); + // The DIAGNOSTIC rides the reason (stderr tail first): with the || + // operands swapped the reason degrades to the failed argv line and the + // real cause is lost to the consumer. + expect(stderr).toContain('fake tmux: refusing'); + // The start that threw created no server: reap() still attempts the + // kill (the flag precedes the call), and the goal-state answer keeps it + // silent — a warning here would send an operator hunting a socket that + // was never created. + expect(stderr).not.toContain('may still be running'); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + }); + + it('reaps a server whose START died on the belt — the flag precedes the call', async () => { + // plan.start forks the server in the SAME client call that creates the + // session: a start cut by the control belt throws with the server + // ALREADY UP. serverStarted must precede the call, or reap() skips + // exactly that window and orphans the server (measured shape on loaded + // runners). The fake's new-session hangs past the seam and its + // kill-server answers the goal state: the call log proves the kill was + // ATTEMPTED — with the flag after the call, reap() returns early and + // no kill-server ever reaches the log. + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + const callLog = join(dir, 'tmux-calls'); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\necho "$*" >> "${callLog}"\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\nfor a in "$@"; do [ "$a" = "new-session" ] && sleep 5; done\nfor a in "$@"; do [ "$a" = "kill-server" ] && { echo "no server running on /tmp/x" >&2; exit 1; }; done\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + const realBelt = tmuxControl.timeoutMs; + tmuxControl.timeoutMs = 500; + const started = performance.now(); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + let stderr = ''; + try { + ({ stderr } = await withStdio(() => + run({ until: undefined, settleMs: 0 }), + )); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + tmuxControl.timeoutMs = realBelt; + } + const elapsed = performance.now() - started; + // The belt cut the hung start — the 5s sleep never ran out. + expect(elapsed).toBeGreaterThanOrEqual(450); + expect(elapsed).toBeLessThan(4_000); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('tmux failed mid-capture'); + // The reap ATTEMPTED the kill despite the start never returning. + expect(readFileSync(callLog, 'utf8')).toContain('kill-server'); + expect(stderr).not.toContain('may still be running'); + }, 15_000); + + it('refuses a FAILED .ans write after capture — contract, not stack trace', async () => { + // The capture window legally runs up to an hour; the disk can fill (or + // the target turn hostile) inside it. The command itself creates a + // DIRECTORY at the .ans path mid-capture, so the final write fails + // EISDIR — real and deterministic, no fd-exhaustion harness needed. + const { stdout, stderr } = await withStdio(() => + run({ + command: 'mkdir cap.ans; printf "DIR-BLOCK\\n"; sleep 30', + until: 'DIR-BLOCK', + }), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('cannot write capture output'); + expect(JSON.parse(stdout.trim())).toEqual({ + captured: false, + evidence: 'none', + reason: expect.stringContaining('cannot write capture output'), + }); + // THIS run's artifacts or nothing — but a DIRECTORY at the path is not + // this run's artifact, and force-deleting it destroyed pre-existing + // user directories (measured). The blocker survives; the refusal names + // the cause; nothing of OURS remains. + expect(statSync(join(dir, 'cap.ans')).isDirectory()).toBe(true); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + }); + + it('refuses a FAILED manifest write and removes what it already wrote', async () => { + // Same seam aimed one write later: the command creates a DIRECTORY at + // the manifest path, so the .ans writes fine and the manifest write + // fails EISDIR — the run must not leave an undescribed .ans (and png) + // behind ("THIS run's artifacts or nothing"). + const { stdout, stderr } = await withStdio(() => + run({ + command: 'mkdir cap.json; printf "DIR-BLOCK2\\n"; sleep 30', + until: 'DIR-BLOCK2', + }), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('cannot write capture manifest'); + expect(JSON.parse(stdout.trim())).toEqual({ + captured: false, + evidence: 'none', + reason: expect.stringContaining('cannot write capture manifest'), + }); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + expect(statSync(join(dir, 'cap.json')).isDirectory()).toBe(true); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + }); + + it('a pre-existing DIRECTORY at the sentinel refuses — and survives', async () => { + // The sentinel clears as a PLAIN file: a directory at the path is a + // user's, so the unlink cannot remove it and the run refuses + // fail-closed — recursive removal deleted it on every run, fully + // successful ones included (measured). + mkdirSync(join(dir, 'cap.holder-ready')); + writeFileSync(join(dir, 'cap.holder-ready', 'user-file'), 'content'); + const { stderr } = await withStdio(() => run()); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('--out is not writable'); + expect(statSync(join(dir, 'cap.holder-ready')).isDirectory()).toBe(true); + expect( + readFileSync(join(dir, 'cap.holder-ready', 'user-file'), 'utf8'), + ).toBe('content'); + }); + + it('a pre-existing DIRECTORY at the png path survives the ans-only run', async () => { + // No manifest → shaped=false → the clear phase protects the directory; + // the freeze torn-png cleanup is a plain rmSync, so the directory's + // EISDIR is swallowed and the otherwise-successful run never deletes + // what it did not write (recursive removal destroyed the tree and the + // run still reported success — measured). + mkdirSync(join(dir, 'cap.png')); + writeFileSync(join(dir, 'cap.png', 'user-file'), 'content'); + await run(); + expect(process.exitCode).toBeUndefined(); + expect(statSync(join(dir, 'cap.png')).isDirectory()).toBe(true); + expect(readFileSync(join(dir, 'cap.png', 'user-file'), 'utf8')).toBe( + 'content', + ); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(existsSync(join(dir, 'cap.ans'))).toBe(true); + }); + + it('keeps the exit contract when the reap WARNING cannot be written', async () => { + // reap() runs from the finally and from onSignal; a throwing stderr + // write there turned an exit-3 refusal into an exit-1 stack trace (and + // an uncaughtException out of the signal handler, killing the re-raise). + // The write is incidental — its reader going away must not decide the + // command's disposition. + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\nfor a in "$@"; do [ "$a" = "kill-server" ] && { echo "wedged" >&2; exit 1; }; done\nfor a in "$@"; do [ "$a" = "new-session" ] && : > "${join(dir, 'cap.holder-ready')}"; done\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + // NOT withStdio: it installs its own stderr spy, which would replace a + // throwing one — the sinks are wired by hand so the WARNING write is + // the one that fails. + const sinks = { stdout: '', stderr: '' }; + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + const outSpy = vi.spyOn(process.stdout, 'write').mockImplementation((( + chunk: string | Uint8Array, + ) => { + sinks.stdout += String(chunk); + return true; + }) as never); + const errSpy = vi.spyOn(process.stderr, 'write').mockImplementation((( + chunk: string | Uint8Array, + ) => { + const text = String(chunk); + if (text.includes('WARNING')) { + throw Object.assign(new Error('write EPIPE'), { code: 'EPIPE' }); + } + sinks.stderr += text; + return true; + }) as never); + try { + await run({ until: undefined, settleMs: 0 }); + } finally { + outSpy.mockRestore(); + errSpy.mockRestore(); + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + const stdout = sinks.stdout; + // The capture still completed: no stack trace, no exit-1 disposition. + expect(process.exitCode).toBeUndefined(); + expect(existsSync(join(dir, 'cap.ans'))).toBe(true); + expect(JSON.parse(stdout.trim().split('\n').at(-1) ?? '')).toMatchObject({ + captured: true, + }); + }); + + it('WARNS when kill-server fails twice — never an unqualified success', async () => { + // A fake tmux that succeeds at everything except kill-server models the + // wedged-server shape: the reap retries once, then must say so — a + // presumed-alive private server holding a pane for up to three hours + // is not a silent outcome. + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + const callLogPath = join(dir, 'tmux-calls'); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\necho "$@" >> "${callLogPath}"\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\nfor a in "$@"; do [ "$a" = "kill-server" ] && { sleep 5; echo "wedged" >&2; exit 1; }; done\nfor a in "$@"; do [ "$a" = "new-session" ] && : > "${join(dir, 'cap.holder-ready')}"; done\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + // The control-call belt through its SEAM: the fake kill HANGS (sleep 5) + // and the shortened belt must cut it — a hardcoded-timeout mutant waits + // out both 5s hangs and blows the wall bound. + const realBelt = tmuxControl.timeoutMs; + tmuxControl.timeoutMs = 500; + const started = performance.now(); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + let stdout = ''; + let stderr = ''; + try { + ({ stdout, stderr } = await withStdio(() => + run({ until: undefined, settleMs: 0 }), + )); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + tmuxControl.timeoutMs = realBelt; + } + expect(performance.now() - started).toBeLessThan(8_000); + expect(stderr).toContain('WARNING'); + expect(stderr).toContain('kill-server failed twice'); + // The cap is ONE retry, pinned from ABOVE too: the assertions here hold + // for any cap up to ~13 under the shortened belt, and the "failed + // twice" wording blesses whatever the cap happens to be. Against a + // genuinely wedged server every extra attempt pays the full 15s belt + // while the capture is already done. + const killCalls = readFileSync(callLogPath, 'utf8') + .split('\n') + .filter((l) => l.includes('kill-server')); + expect(killCalls).toHaveLength(2); + // The other half of "never an unqualified success": a wedged reap is a + // WARNING next to a COMPLETE capture, not a failure — exit code clean, + // artifacts written, success JSON emitted (a mutant setting exitCode + // in the reap's !serverDead branch reports exit 3 next to a finished + // capture, and shipped green before these assertions). + expect(process.exitCode).toBeUndefined(); + expect(existsSync(join(dir, 'cap.ans'))).toBe(true); + expect(existsSync(join(dir, 'cap.json'))).toBe(true); + expect(JSON.parse(stdout.trim().split('\n').at(-1) ?? '')).toMatchObject({ + captured: true, + }); + // The sentinel is plumbing — removed on every exit path, including this + // degraded one whose fixture is the only one that creates it. + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + }); + + it('refuses when the holder never initializes — and sends NO key into the void', async () => { + // A fake tmux that succeeds every command but whose new-session writes + // no sentinel models a pane that died at startup: the 10s holder + // deadline must refuse (not hang, not fire keys at an unknown screen). + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + const callLog = join(dir, 'tmux-calls'); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\necho "$*" >> "${callLog}"\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + const realHolder = holderInit.timeoutMs; + holderInit.timeoutMs = 600; + const started = performance.now(); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + let stderr = ''; + try { + ({ stderr } = await withStdio(() => + run({ keys: ['C-c'], until: undefined, settleMs: 0 }), + )); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + holderInit.timeoutMs = realHolder; + } + expect(process.exitCode).toBe(3); + // The deadline actually elapsed (floor) and is seam-driven (ceiling — + // the hardcoded-10s mutant blows it). + const waited = performance.now() - started; + expect(waited).toBeGreaterThanOrEqual(550); + expect(waited).toBeLessThan(5_000); + expect(stderr).toContain('never initialized'); + // No key was fired into the uninitialized pane. + expect(readFileSync(callLog, 'utf8')).not.toContain('send-keys'); + }, 20_000); + + it('treats a kill answering "no server running" as the goal state — no WARNING', async () => { + // A server dying between the last capture and the reap is success, not + // a wedge: the always-false regex mutant printed a false WARNING that + // sends an operator hunting a server that does not exist. + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\nfor a in "$@"; do [ "$a" = "kill-server" ] && { echo "no server running on /tmp/x" >&2; exit 1; }; done\nfor a in "$@"; do [ "$a" = "new-session" ] && : > "${join(dir, 'cap.holder-ready')}"; done\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + let stderr = ''; + try { + ({ stderr } = await withStdio(() => + run({ until: undefined, settleMs: 0 }), + )); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + expect(process.exitCode).toBeUndefined(); + expect(stderr).not.toContain('WARNING'); + expect(existsSync(join(dir, 'cap.json'))).toBe(true); + }); + + it('records the LAUNCHER cwd when --cwd is omitted', async () => { + // Every other success capture passes an explicit cwd; the default + // branch feeds both new-session -c and the manifest — a mutant default + // would make the capture's only record name a directory the command + // never ran in. + await runCaptureTui({ + command: 'printf "CWDLESS\\n"; sleep 30', + cwd: undefined, + cols: 80, + rows: 24, + settleMs: 0, + until: 'CWDLESS', + keys: undefined, + out: join(dir, 'nocwd'), + timeoutMs: 10_000, + } as never); + const manifest = JSON.parse(readFileSync(join(dir, 'nocwd.json'), 'utf8')); + expect(manifest.cwd).toBe(process.cwd()); + }); + + it('survives a C-\\ sent through --keys — QUIT is trapped at layer 0', async () => { + // The wrapped holder trapped one layer deep: INT survived by shell + // wait semantics, QUIT killed the untrapped session leader (measured: + // exit 3, "no server running", zero artifacts). The unwrapped script + // traps both at the pane's own shell. + await runCaptureTui({ + command: 'cat', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 800, + until: undefined, + keys: ['C-\\'], + out: join(dir, 'cq'), + timeoutMs: 10_000, + } as never); + expect(process.exitCode).toBeUndefined(); + expect(existsSync(join(dir, 'cq.json'))).toBe(true); + }); + + it('renders WIDTH x HEIGHT, not height x width — the frame has rows lines', async () => { + // validGeometry accepts a transposed pair, so only a behavioral pin + // catches a cols/rows swap: a 30x10 pane captures as 10 physical lines. + await run({ + command: 'printf "GEOM\\n"; sleep 30', + until: 'GEOM', + cols: 30, + rows: 10, + out: join(dir, 'geom'), + }); + const ans = readFileSync(join(dir, 'geom.ans'), 'utf8'); + const lines = ans.replace(/\n$/, '').split('\n').length; + expect(lines).toBe(10); + }); + + it('reaps on the SECOND kill attempt without a WARNING — the retry is real', async () => { + // Every prior fixture failed both attempts or neither; the fail-once + // shape is what the retry exists for, and a retry-less mutant WARNs. + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + const marker = join(dir, 'kill-attempted'); + writeFileSync( + join(binDir, 'tmux'), + `#!/bin/sh\n[ "$1" = "-V" ] && { echo "tmux 3.9"; exit 0; }\nfor a in "$@"; do [ "$a" = "kill-server" ] && { if [ ! -e "${marker}" ]; then : > "${marker}"; echo "transient" >&2; exit 1; fi; exit 0; }; done\nfor a in "$@"; do [ "$a" = "new-session" ] && : > "${join(dir, 'cap.holder-ready')}"; done\necho ""\nexit 0\n`, + { mode: 0o755 }, + ); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + let stderr = ''; + try { + ({ stderr } = await withStdio(() => + run({ until: undefined, settleMs: 0 }), + )); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + expect(process.exitCode).toBeUndefined(); + expect(stderr).not.toContain('WARNING'); + expect(existsSync(marker)).toBe(true); + }); + + it('unlinks the socket from /tmp when TMUX_TMPDIR points somewhere unusable', async () => { + // tmux takes the first USABLE base, so the socket really lives under + // /tmp; the env-base-only unlink mutant left dead sockets littering it. + const realEnv = process.env['TMUX_TMPDIR']; + process.env['TMUX_TMPDIR'] = join(dir, 'no-such-base'); + try { + await run({ until: undefined, settleMs: 0, out: join(dir, 'tt') }); + } finally { + if (realEnv === undefined) delete process.env['TMUX_TMPDIR']; + else process.env['TMUX_TMPDIR'] = realEnv; + } + expect(process.exitCode).toBeUndefined(); + const probe = spawnSync('bash', [ + '-c', + `ls "/tmp/tmux-$(id -u)" 2>/dev/null | grep "^qwen-review-capture-${process.pid}-" || true`, + ]); + expect(probe.error).toBeUndefined(); + expect((probe.stdout ?? Buffer.from('')).toString().trim()).toBe(''); + }); + + it('settles by regex when --until matches, and says so', async () => { + await run({ until: 'WORLD', settleMs: 0 }); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + }); + + it('captures anyway on --until timeout and records the degraded settle', async () => { + await run({ until: 'NEVER-APPEARS', timeoutMs: 1500, settleMs: 0 }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('timeout'); + // timeoutMs recorded on an UNTIL-ONLY run too: every other timeoutMs + // assertion in this suite rides a --ready run, so a spread mutated to + // `args.ready !== undefined` alone shipped green while every until-only + // capture silently lost the record of its governing budget. + expect(manifest.timeoutMs).toBe(1500); + // The field whose contract is "why the ladder stopped" carries the late + // frame too, not just the freeze rung. + expect(manifest.degradedBecause).toContain('--until never matched'); + const ans = readFileSync(join(dir, 'cap.ans'), 'utf8'); + expect(ans).toContain('HELLO-'); + }); + + it('refuses NaN durations instead of hanging on them', async () => { + // A NaN deadline never expires — `--settle-ms abc` must refuse. + await run({ until: undefined, settleMs: Number.NaN }); + expect(process.exitCode).toBe(3); + process.exitCode = undefined; + await run({ timeoutMs: Number.NaN }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }); + + it('refuses negative and over-bound durations', async () => { + // The bounds are the guard's other half: without them a typo'd + // --timeout-ms of a day is accepted and the poll loop runs for a day. + await run({ until: undefined, settleMs: -1 }); + expect(process.exitCode).toBe(3); + process.exitCode = undefined; + // settle-ms's 600_000 ceiling was unpinned: only its negative side was + // tested, and a raised-max mutant accepted a 1-hour fixed delay. + await run({ until: undefined, settleMs: 600_001 }); + expect(process.exitCode).toBe(3); + process.exitCode = undefined; + await run({ timeoutMs: 3_600_001 }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }); + + it('refuses an unwritable --out on the contract, not a stack trace', async () => { + writeFileSync(join(dir, 'blocker'), 'x'); + await run({ out: join(dir, 'blocker', 'cap') }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'blocker', 'cap.ans'))).toBe(false); + }); + + it('sends --keys tokens verbatim, one per token', async () => { + await runCaptureTui({ + command: 'cat', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 800, + until: 'typed-input', + keys: ['typed-input', 'Enter'], + out: join(dir, 'keys'), + timeoutMs: 10_000, + } as never); + const ans = readFileSync(join(dir, 'keys.ans'), 'utf8'); + expect(ans).toContain('typed-input'); + // Per-token dispatch, not one joined call: joined, tmux types the + // literal string "typed-input Enter" (Enter is only a key NAME as its + // own token) and this line is what turns red. + expect(ans).not.toContain('typed-input Enter'); + // The manifest records the keys: a capture driven by keys shows a + // different screen than the bare command, and a reproducer must know. + const manifest = JSON.parse(readFileSync(join(dir, 'keys.json'), 'utf8')); + expect(manifest.keys).toEqual(['typed-input', 'Enter']); + expect(manifest.until).toBe('typed-input'); + expect(manifest.cwd).toBe(dir); + }); + + it('sends --keys only after --ready matches — early keys get eaten', async () => { + // The fixture DRAINS its input before printing READY, the way a + // slow-mounting TUI eats keystrokes fired at start (measured on this + // repo's own onboarding dialog: a Down consumed, the Enter behind it + // lost). The drain does NOT hide early keys from the pane — the kernel + // echoes them before the fixture's read -s begins (measured) — so the + // pin below is the ORDER (gated keys after READY): the one signal that + // actually discriminates gated from ungated. + await runCaptureTui({ + command: `bash -c 'sleep 0.7; IFS= read -rs -t 0.3 -n 10000 junk || true; printf "READY\\n"; cat'`, + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + ready: 'READY', + until: 'gated-input', + keys: ['gated-input', 'Enter'], + out: join(dir, 'ready'), + timeoutMs: 10_000, + } as never); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'ready.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + expect(manifest.keysSent).toBe(true); + expect(manifest.ready).toBe('READY'); + const ans = readFileSync(join(dir, 'ready.ans'), 'utf8'); + expect(ans).toContain('gated-input'); + // Order is the discriminating signal: ungated, the keys still echo + // into the pane, `until` matches on the echo, and every assertion + // above stays green against the exact regression this test was written + // to catch (measured: the no-gate mutant passed in 35ms). + expect(ans.indexOf('gated-input')).toBeGreaterThan(ans.indexOf('READY')); + }); + + it('withholds --keys when --ready never matches, and says so', async () => { + // Typing into a screen that never reached the expected state would + // drive an unknown UI; the keys are withheld and the manifest is honest + // about both the miss and the withholding. + await run({ + ready: 'NEVER-READY', + keys: ['DANGER', 'Enter'], + until: undefined, + settleMs: 0, + timeoutMs: 1500, + }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.keysSent).toBe(false); + expect(manifest.degradedBecause).toContain('--ready never matched'); + expect(manifest.degradedBecause).toContain('NOT sent'); + // The manifest tells the truth about HOW the run ended: it waited out + // --timeout-ms (a timeout settle, not a fixed delay), and the active + // duration recorded is the one that governed it. + expect(manifest.settledBy).toBe('timeout'); + expect(manifest.timeoutMs).toBe(1500); + expect(manifest.settleMs).toBeUndefined(); + const ans = readFileSync(join(dir, 'cap.ans'), 'utf8'); + // The pty would echo even unread keystrokes — absence proves withheld. + expect(ans).not.toContain('DANGER'); + // And the late frame is a real frame: dropping the readyFailed-branch + // capture shipped a 0-byte .ans whose degradation claimed "late frame + // captured" while a second entry said "pane captured empty". + expect(ans).toContain('HELLO-'); + }); + + it('gates --ready on the LOGICAL view — an SGR-split marker still opens it', async () => { + // Both prior ready tests used plain markers; on the physical (-e) view + // an escape lands inside the marker and the gate never opens — keys + // withheld on a healthy UI. + await runCaptureTui({ + command: `bash -c 'sleep 0.5; printf "GA\\033[31mTE\\033[0m\\n"; cat'`, + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + ready: 'GATE', + until: 'sgr-gated', + keys: ['sgr-gated', 'Enter'], + out: join(dir, 'sgr-ready'), + timeoutMs: 10_000, + } as never); + const manifest = JSON.parse( + readFileSync(join(dir, 'sgr-ready.json'), 'utf8'), + ); + expect(manifest.settledBy).toBe('until-match'); + expect(manifest.keysSent).toBe(true); + }); + + it('refuses an empty or invalid --ready like it refuses --until', async () => { + await run({ ready: ' ' }); + expect(process.exitCode).toBe(3); + process.exitCode = undefined; + await run({ ready: '[' }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }); + + it('sends --keys in fixed-delay mode too — keys are not an --until feature', async () => { + await runCaptureTui({ + command: 'cat', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 800, + until: undefined, + keys: ['typed-input', 'Enter'], + out: join(dir, 'keys-fixed'), + timeoutMs: 10_000, + } as never); + expect(readFileSync(join(dir, 'keys-fixed.ans'), 'utf8')).toContain( + 'typed-input', + ); + }); + + it('dispatches EVERY key token — a marker only a second token can produce', async () => { + // All prior keys fixtures settle on the FIRST token's echo, so a + // first-token-only mutant shipped green. This fixture's marker appears + // only after Enter completes the read. + await runCaptureTui({ + command: `bash -c 'IFS= read -r line; printf "GOT:%s\\n" "$line"; cat'`, + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + until: 'GOT:hello', + keys: ['hello', 'Enter'], + out: join(dir, 'twotok'), + timeoutMs: 10_000, + } as never); + const manifest = JSON.parse(readFileSync(join(dir, 'twotok.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + }); + + it('dispatches keys IN ORDER — reversal drives a different key sequence', async () => { + await runCaptureTui({ + command: 'cat', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 0, + until: 'LINE2', + keys: ['LINE1', 'Enter', 'LINE2'], + out: join(dir, 'order'), + timeoutMs: 10_000, + } as never); + const ans = readFileSync(join(dir, 'order.ans'), 'utf8'); + expect(ans.indexOf('LINE1')).toBeGreaterThan(-1); + expect(ans.indexOf('LINE2')).toBeGreaterThan(ans.indexOf('LINE1')); + }); + + it('refuses degenerate geometry with the refusal contract', async () => { + await run({ cols: 3 }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }); + + it('refuses an empty command', async () => { + await run({ command: ' ' }); + expect(process.exitCode).toBe(3); + }); + + it('refuses an invalid --until regex BEFORE anything starts', async () => { + const { stderr } = await withStdio(() => run({ until: '[' })); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + expect(existsSync(join(dir, 'cap.json'))).toBe(false); + expect(existsSync(join(dir, 'cap.holder-ready'))).toBe(false); + // The reason pins the path: validated up front, this reads "not a valid + // regex"; thrown after tmux started, it would read "tmux failed + // mid-capture: Invalid regular expression…" — a caller mistake blamed + // on tmux, from a server that was started for nothing. + expect(stderr).toContain('not a valid regex'); + expect(stderr).not.toContain('mid-capture'); + }); + + it('records a fixed-delay settle honestly when no --until is given', async () => { + // The wait itself is pinned by wall clock: sleep(0) captures before the + // TUI renders, sleep(timeoutMs) waits up to 20x longer than requested — + // both shipped green when only the manifest field was asserted. + const started = performance.now(); + await run({ until: undefined, settleMs: 600 }); + const elapsed = performance.now() - started; + // The 50ms of slack absorbs libuv starting the settle timer off a cached + // loop tick under load; the bound still catches the sleep(0) and + // sleep(timeoutMs) mutants it exists for. + expect(elapsed).toBeGreaterThanOrEqual(550); + expect(elapsed).toBeLessThan(5_000); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('fixed-delay'); + expect(manifest.settleMs).toBe(600); + // The capture happens AFTER the wait: a sleep↔capture swap published a + // pre-render frame as the settled rung. + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toContain('HELLO-'); + }); + + it('settles by FIXED DELAY after a matched --ready without --until', async () => { + // Every ready-matched test also passed --until, leaving this branch — + // fixed delay AFTER the gate opens, and its dual-duration manifest — + // unpinned: two mutants shipped green (skipping the settle sleep; + // dropping settleMs from this shape's manifest). + const started = performance.now(); + await runCaptureTui({ + command: `bash -c 'printf "READY-NO-UNTIL\\n"; sleep 0.3; printf "SETTLED-LATE\\n"; cat'`, + cwd: dir, + cols: 80, + rows: 24, + settleMs: 600, + until: undefined, + ready: 'READY-NO-UNTIL', + keys: undefined, + out: join(dir, 'readyfixed'), + timeoutMs: 10_000, + } as never); + const elapsed = performance.now() - started; + expect(process.exitCode).toBeUndefined(); + // Wall bound on the settle; the content check below is the real + // discriminator, and this bound catches a sleep(timeoutMs) mutant. + expect(elapsed).toBeGreaterThanOrEqual(550); + expect(elapsed).toBeLessThan(8_000); + const manifest = JSON.parse( + readFileSync(join(dir, 'readyfixed.json'), 'utf8'), + ); + expect(manifest.settledBy).toBe('fixed-delay'); + expect(manifest.settleMs).toBe(600); + // BOTH durations are active in this shape: ready spent the timeout + // budget AND settle governed the wait — omitting either misdescribes + // the run (the ACTIVE-durations contract). + expect(manifest.timeoutMs).toBe(10_000); + // The settle really waited: the frame carries the line that renders + // 300ms AFTER the ready marker matched — a skipped sleep captures + // before it exists. + const ans = readFileSync(join(dir, 'readyfixed.ans'), 'utf8'); + expect(ans).toContain('READY-NO-UNTIL'); + expect(ans).toContain('SETTLED-LATE'); + }); + + it('records an empty-pane capture honestly and never hands it to freeze', async () => { + // A pane that rendered nothing (sleep, settle 0) is the blank-capture + // branch: freeze on empty input fails with a misleading bounds error, + // so the ladder must stop at ans-only with the blank named as the why. + await run({ command: 'sleep 30', until: undefined, settleMs: 0 }); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(manifest.degradedBecause).toContain('pane captured empty'); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + }); + + /** Point the freeze render at a fake binary by ABSOLUTE path (a PATH shim + * is skipped by execvp when non-executable) with the probe seam forced + * open, so the real spawn runs and the degradation composition is pinned + * by real exec, not by reading. */ + async function withFakeFreeze( + script: string, + fn: () => Promise, + ): Promise { + const binDir = join(dir, 'fakebin'); + mkdirSync(binDir, { recursive: true }); + const bin = join(binDir, 'freeze'); + writeFileSync(bin, script, { + mode: script.startsWith('#!') ? 0o755 : 0o644, + }); + const realFreeze = probes.freeze; + const realBin = freezeRender.bin; + probes.freeze = () => ({ status: 'ok', out: '' }) as const; + freezeRender.bin = bin; + try { + await fn(); + } finally { + probes.freeze = realFreeze; + freezeRender.bin = realBin; + } + } + + it('renders only AFTER the .ans is on disk — text evidence survives a hang', async () => { + // The fake refuses to render unless the .ans already exists and is + // non-empty: a write-after-render mutant fails it. + await withFakeFreeze( + '#!/bin/sh\n[ -s "$3" ] || { echo "ans missing at render time" >&2; exit 9; }\nprintf x > "$5"\nexit 0\n', + () => run(), + ); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('png'); + }); + + it('records a freeze CRASH with its diagnostics, not just its absence', async () => { + await withFakeFreeze( + '#!/bin/sh\necho "boom: render exploded" >&2\nexit 9\n', + () => run(), + ); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.degradedBecause).toContain('freeze failed (exit 9'); + expect(manifest.degradedBecause).toContain('boom: render exploded'); + // A freeze failure never costs the text evidence: .ans was written first. + expect(existsSync(join(dir, 'cap.ans'))).toBe(true); + }); + + it('removes a TORN png when the render fails mid-write', async () => { + // A fake that writes bytes to the png path and THEN fails: without the + // failed-render cleanup a torn png persists at the very path the + // manifest denies (evidence 'ans-only', pngPath null), and a consumer + // globbing .png picks it up as evidence (probe-verified: the + // rmSync-deletion mutant left torn bytes at cap.png). + await withFakeFreeze('#!/bin/sh\nprintf torn > "$5"\nexit 9\n', () => + run(), + ); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + }); + + it('cuts a HANGING freeze with the timeout belt and keeps the .ans', async () => { + const realBelt = freezeRender.timeoutMs; + freezeRender.timeoutMs = 1000; + try { + await withFakeFreeze('#!/bin/sh\nsleep 40\n', () => run()); + } finally { + freezeRender.timeoutMs = realBelt; + } + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.degradedBecause).toContain('signal'); + expect(existsSync(join(dir, 'cap.ans'))).toBe(true); + }); + + it('REFUSES an oversized .json instead of reading it into the heap', async () => { + // A capture manifest is a few hundred bytes. The clear phase used to + // readFileSync + JSON.parse whatever regular file sat there, so a huge + // one killed the process with a heap OOM before any refusal could + // print (measured at ~479MB). lstat already holds the size, so the cap + // costs nothing — and past it the file is simply not ours, which the + // collision gate then refuses by name. + // VALID JSON, and shaped like a manifest, so the cap is what decides: + // without it the file parses, `shaped` is true, and the clear phase + // deletes the sibling artifacts. With it the file is simply too big to + // be ours, nothing is cleared, and the collision gate refuses by name. + // (A garbage payload would not discriminate — JSON.parse rejects it + // either way.) + const huge = JSON.stringify({ + evidence: 'png', + pngPath: join(dir, 'cap.png'), + pad: 'x'.repeat(2 * 1024 * 1024), + }); + writeFileSync(join(dir, 'cap.json'), huge); + writeFileSync(join(dir, 'cap.ans'), 'previous run text'); + const { stderr } = await withStdio(() => run({ settleMs: 0 })); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('collides with a file this capture did not'); + // Neither the oversized file nor its siblings were touched: a manifest + // this run could not verify is not authority to delete anything. + expect(statSync(join(dir, 'cap.json')).size).toBe(huge.length); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toBe( + 'previous run text', + ); + }); + + it('refuses a DANGLING SYMLINK at a mandatory path — writes must not escape', async () => { + // existsSync and statSync follow links, so a dangling one read as + // "nothing here": the collision gate never fired and writeFileSync's + // O_CREAT then created the pane text and the manifest at the links' + // TARGETS, outside the --out base, while the run reported success and + // the manifest named .json (probe-verified end to end). Occupancy + // is an lstat question — the link itself is the occupant. + const elsewhere = join(dir, 'elsewhere'); + mkdirSync(elsewhere, { recursive: true }); + symlinkSync(join(elsewhere, 'ans-victim.txt'), join(dir, 'cap.ans')); + const { stderr } = await withStdio(() => run({ settleMs: 0 })); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('collides with a file this capture did not'); + // Nothing escaped the base, and the link is still the user's. + expect(readdirSync(elsewhere)).toEqual([]); + expect(lstatSync(join(dir, 'cap.ans')).isSymbolicLink()).toBe(true); + }); + + it('REFUSES rather than rewrite a foreign file at a mandatory path', async () => { + // The collision the clear phase's own comment names: `--out package` in + // a Node project. package.json parses but carries no evidence rung, so + // the clear spares it — and a fully SUCCESSFUL run then rewrote it as a + // capture manifest at exit 0 with nothing recorded. Both files must + // come back byte-for-byte. + const pkg = '{"name":"my-pkg","version":"1.0.0"}'; + const notes = 'hand-written notes'; + writeFileSync(join(dir, 'cap.json'), pkg); + writeFileSync(join(dir, 'cap.ans'), notes); + const { stderr } = await withStdio(() => run({ settleMs: 0 })); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('collides with a file this capture did not'); + expect(readFileSync(join(dir, 'cap.json'), 'utf8')).toBe(pkg); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toBe(notes); + }); + + it('degrades rather than rewrite a foreign file at the PNG path', async () => { + // The png is a rung, not a requirement: an occupied png path stops the + // ladder at the text rung and says why, instead of failing a capture + // that can still produce evidence — or replacing the file. + const foreign = 'a foreign image'; + writeFileSync(join(dir, 'cap.png'), foreign); + await withFakeFreeze('#!/bin/sh\nprintf x > "$5"\n', () => run()); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(manifest.degradedBecause).toContain('did not write'); + expect(readFileSync(join(dir, 'cap.png'), 'utf8')).toBe(foreign); + }); + + it('never CREDITS a png the clear phase protected as this run evidence', async () => { + // A freeze that exits 0 without writing leaves the user's untouched + // file at .png. Existence alone credited it: success JSON with + // evidence 'png' pointing at bytes this run never produced, and a + // verifier would publish an unrelated image as the rendering. + writeFileSync(join(dir, 'cap.png'), 'the user file'); + await withFakeFreeze('#!/bin/sh\nexit 0\n', () => run()); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(manifest.degradedBecause).toBeTruthy(); + expect(readFileSync(join(dir, 'cap.png'), 'utf8')).toBe('the user file'); + }); + + it('never deletes a png the clear phase PROTECTED when the render fails', async () => { + // shaped=false (no capture manifest) deliberately leaves whatever sits + // at .png alone as possibly the user's. The torn-png cleanup then + // deleted it on a SUCCEEDING run whose freeze never opened its output + // (EMFILE, a belt kill) — silent data loss recorded as plain ans-only. + writeFileSync(join(dir, 'cap.png'), 'the user file'); + await withFakeFreeze('#!/bin/sh\nexit 9\n', () => run()); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(readFileSync(join(dir, 'cap.png'), 'utf8')).toBe('the user file'); + }); + + it('never manifests a png rung on exit code alone — the file must exist', async () => { + // A freeze that exits 0 without writing anything would otherwise ship + // "evidence": "png" pointing at nothing. + await withFakeFreeze('#!/bin/sh\nexit 0\n', () => run()); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(manifest.degradedBecause).toContain('wrote no image'); + }); + + it('never manifests a png rung on a 0-BYTE image either — size is checked', async () => { + // A freeze that exits 0 but leaves an empty/truncated png (ENOSPC + // mid-write — the shape the .ans write guard's comment names) would + // otherwise sail past an existence-only guard and publish zero pixels + // as "evidence": "png" (probe-verified end-to-end). + await withFakeFreeze('#!/bin/sh\n: > "$5"\nexit 0\n', () => run()); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + // The failed-render cleanup removes the empty shell too. + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + }); + + it('names a freeze that could not SPAWN, not "exit null"', async () => { + // A non-executable freeze produces neither status nor signal; the + // reason lives in r.error and the manifest must carry it. + await withFakeFreeze('not executable', () => run()); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.degradedBecause).toContain('spawn failed'); + expect(manifest.degradedBecause).not.toContain('exit null'); + }); + + it('probes freeze with --help — the flag freeze <=0.1.6 actually has', async () => { + // Both real-freeze tests override the seam, so the FLAG the real probe + // sends was unpinned: a --version mutant stays green wherever freeze + // >=0.2.2 or no freeze at all is installed, and only fails on a 2024 + // freeze — where it misdiagnoses it as absent. This fake accepts ONLY + // --help, so the mutant fails everywhere. + const binDir = join(dir, 'probebin'); + mkdirSync(binDir, { recursive: true }); + writeFileSync( + join(binDir, 'freeze'), + '#!/bin/sh\n[ "$1" = "--help" ] && exit 0\nexit 1\n', + { mode: 0o755 }, + ); + const realPath = process.env['PATH']; + process.env['PATH'] = `${binDir}:${realPath ?? ''}`; + try { + expect(probes.freeze().status).toBe('ok'); + } finally { + if (realPath === undefined) delete process.env['PATH']; + else process.env['PATH'] = realPath; + } + }); + + it('degrades to ans-only when freeze is WEDGED — and says wedged, not absent', async () => { + // The hung branch's message had no pin: a collapse to "not installed" + // sends an operator to reinstall a binary that exists but hangs. + const realFreeze = probes.freeze; + probes.freeze = () => ({ status: 'hung' }) as const; + try { + await run(); + } finally { + probes.freeze = realFreeze; + } + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.degradedBecause).toContain('wedged'); + expect(manifest.degradedBecause).not.toContain('not installed'); + }); + + it('degrades to ans-only when freeze is unavailable, and says why', async () => { + // Through the probe seam, so the freeze-less rung is pinned even on + // hosts that have freeze installed. + const realFreeze = probes.freeze; + probes.freeze = () => ({ status: 'absent' }) as const; + try { + await run(); + } finally { + probes.freeze = realFreeze; + } + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('ans-only'); + expect(manifest.pngPath).toBeNull(); + expect(manifest.degradedBecause).toContain('freeze is not installed'); + expect(existsSync(join(dir, 'cap.png'))).toBe(false); + }); + + it('survives a command with a trailing semicolon or comment — the holder is tail-proof', async () => { + // Appended with `;`, the hold would become `;;` (syntax error, pane + // dies instantly) after a trailing semicolon, and a trailing `#` + // comment would swallow it entirely — both re-creating the one-shot + // failure on commands that are themselves valid shell. + await run({ command: 'printf "TAIL-SEMI\\n";', until: 'TAIL-SEMI' }); + expect(process.exitCode).toBeUndefined(); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toContain('TAIL-SEMI'); + await run({ + command: 'printf "TAIL-HASH\\n" # keep-alive note', + until: 'TAIL-HASH', + out: join(dir, 'hash'), + }); + expect(process.exitCode).toBeUndefined(); + expect(readFileSync(join(dir, 'hash.ans'), 'utf8')).toContain('TAIL-HASH'); + }); + + it('captures a command that ENDS ITSELF with exit 0 — the inner shell absorbs it', async () => { + // Single-shell holder measured: `printf ...; exit 0` took pane, session + // and server down before capture — "no server running" on a valid + // command. The nested holder absorbs the exit. + await run({ command: 'printf "EXITY\\n"; exit 0', until: 'EXITY' }); + expect(process.exitCode).toBeUndefined(); + expect(readFileSync(join(dir, 'cap.ans'), 'utf8')).toContain('EXITY'); + }); + + it('survives a C-c sent through --keys — the holder traps SIGINT', async () => { + // Non-interactive shells stay in the pane's foreground process group, + // so a C-c delivered by this feature's own --keys path reaches the + // holder shell too; untrapped, it dies and takes pane → session → + // server down before the capture (measured: exit 3, zero artifacts, + // misattributed "no server running"). The holder's `trap : INT` is + // what this pins — and a trapped (not ignored) signal resets to + // default in the children, so ^C still lands in the pane. + await runCaptureTui({ + command: 'cat', + cwd: dir, + cols: 80, + rows: 24, + settleMs: 800, + until: undefined, + keys: ['C-c'], + out: join(dir, 'cc'), + timeoutMs: 10_000, + } as never); + expect(process.exitCode).toBeUndefined(); + expect(existsSync(join(dir, 'cc.ans'))).toBe(true); + const manifest = JSON.parse(readFileSync(join(dir, 'cc.json'), 'utf8')); + expect(manifest.keysSent).toBe(true); + // The ^C landed in the pane: delivered, not swallowed. + expect(readFileSync(join(dir, 'cc.ans'), 'utf8')).toContain('^C'); + }); + + it('survives a C-c AFTER a one-shot command exits — the hold is a loop', async () => { + // The trap protects only while the command is in the foreground: once + // a render-and-exit command is done, a --keys C-c landing in the hold + // killed a single sleep and ended the script — pane, session, server + // gone (measured 5/5 with the single-sleep hold). The loop re-enters + // sleep and the pane survives; a single-sleep mutant dies here. + await run({ + command: 'printf "CCLOOP\\n"; exit 0', + until: undefined, + settleMs: 800, + keys: ['C-c'], + out: join(dir, 'ccloop'), + }); + expect(process.exitCode).toBeUndefined(); + expect(existsSync(join(dir, 'ccloop.ans'))).toBe(true); + const manifest = JSON.parse(readFileSync(join(dir, 'ccloop.json'), 'utf8')); + expect(manifest.keysSent).toBe(true); + }); + + it('refuses --until/--ready patterns that would MATCH a blank pane', async () => { + // The blank pane's logical capture is rows of newlines, not the empty + // string — `.?`, `x*`, `\s` and `\n` all pass an empty-string-only + // oracle yet settle (or fire keys) before the UI rendered anything. + for (const until of ['.?', '(MARKER)?', 'x*', '\\s', '\\n']) { + process.exitCode = undefined; + const { stderr } = await withStdio(() => run({ until })); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('matches a blank pane'); + } + process.exitCode = undefined; + const { stderr } = await withStdio(() => + run({ until: 'REAL', ready: '\\s', keys: ['x'] }), + ); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('--ready'); + expect(stderr).toContain('matches a blank pane'); + }); + + it('reports the success JSON on stdout — captured, evidence, manifest path', async () => { + // The consumer is an agent: the success line is machine-read, and only + // the refusal side was pinned before. + const { stdout } = await withStdio(() => run()); + const line = stdout.trim().split('\n').at(-1) ?? ''; + // hasFreeze only proves --help answers; a broken render degrades to + // ans-only by contract, so the evidence field is shape-checked. + expect(JSON.parse(line)).toEqual({ + captured: true, + evidence: hasFreeze + ? expect.stringMatching(/^(png|ans-only)$/) + : 'ans-only', + manifest: join(dir, 'cap.json'), + }); + }); + + it('shares ONE deadline between the ready gate and the until poll', async () => { + // Two separate clocks would let a ready+until capture run to + // 2× --timeout-ms: ready matches late (~1.5s), until never matches, and + // the whole run must still end near the single 2s deadline, not 3.5s. + // The freeze render is NOT what this test measures: leaving it in the + // timed window spends up to a second of the bound on the render, and + // hosts with freeze would test a different window than hosts without. + const realFreeze = probes.freeze; + probes.freeze = () => ({ status: 'absent' }) as const; + const started = performance.now(); + try { + await run({ + command: 'sleep 1.5; printf "GATE-OPEN\\n"; sleep 30', + ready: 'GATE-OPEN', + until: 'NEVER-MATCHES', + timeoutMs: 2500, + }); + } finally { + probes.freeze = realFreeze; + } + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('timeout'); + // What this pins is the SINGLE deadline, not that the gate ran: with + // --ready matching and no keys, no observable here separates a gate + // that polled from one that skipped (a keys-gated skip mutant passes — + // the sibling test below pins the gate's own residue through the + // overrun accounting instead). The degradation names the until miss and + // timeoutMs was the active knob. + expect(manifest.degradedBecause).toContain('--until never matched'); + expect(manifest.timeoutMs).toBe(2500); + // Pristine ends near the single 2.5s deadline; the two-clock mutant + // needs ready(~1.6s) + until(2.5s) ≈ 4.1s and lands past the bound. + expect(performance.now() - started).toBeLessThan(3600); + }); + + it('accounts budget overruns in the READY loop too', async () => { + // Deleting matchOverruns++ from the ready loop shipped green — only the + // until loop's accounting was pinned. + await run({ + command: `printf 'a%.0s' $(seq 1 79); printf '\\n'; sleep 30`, + ready: '(a+)+b', + until: undefined, + settleMs: 0, + timeoutMs: 1500, + }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.degradedBecause).toContain('budget'); + }); + + it('polls --ready even with no keys and no --until — and says how it settled', async () => { + // Production deliberately spends the timeout budget on a ready-only + // run; a mutant gating the poll on keys-present settles instantly on a + // pre-render frame with settledBy 'fixed-delay' and no degradation — + // the false-settle shape --ready exists to prevent. + await run({ + ready: 'NEVER-READY', + until: undefined, + keys: undefined, + settleMs: 0, + timeoutMs: 1500, + }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.settledBy).toBe('timeout'); + expect(manifest.degradedBecause).toContain('--ready never matched'); + expect(manifest.timeoutMs).toBe(1500); + expect(manifest.settleMs).toBeUndefined(); + }); + + it('refuses an empty --until instead of settling on a blank frame', async () => { + // new RegExp('') matches ANY pane text: the first poll would settle + // "until-match" before anything rendered — a false settle claim. + await run({ until: ' ' }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }); + + it('refuses an empty --out instead of writing .ans', async () => { + await run({ out: '' }); + expect(process.exitCode).toBe(3); + }); + + it('refuses a --out naming an existing directory — artifacts land NEXT TO it', async () => { + // resolve('.') and resolve('./') are the cwd itself — the same shape + // the empty guard refuses — and any existing directory sails through + // an empty-string-only guard; artifacts would land as .ans next + // to it, silently clobbering whatever holds those names (measured: + // out '.' overwrote a pre-seeded .ans with the pane text). + const adir = join(dir, 'adir'); + mkdirSync(adir); + const { stderr } = await withStdio(() => run({ out: adir })); + expect(process.exitCode).toBe(3); + expect(stderr).toContain('must not name an existing directory'); + process.exitCode = undefined; + await run({ out: '.' }); + expect(process.exitCode).toBe(3); + }); + + it.skipIf(process.getuid?.() === 0)( + 'refuses an unwritable existing --out dir BEFORE the capture runs', + async () => { + // mkdirSync({recursive}) does no permission check on an existing dir: + // without the write probe the capture would run to completion and + // lose the pane text at the very last write. + const ro = join(dir, 'ro'); + mkdirSync(ro, { mode: 0o555 }); + const { stderr } = await withStdio(() => run({ out: join(ro, 'cap') })); + expect(process.exitCode).toBe(3); + expect(existsSync(join(ro, 'cap.ans'))).toBe(false); + // The reason pins WHERE the refusal landed: without the up-front + // probe, the capture runs to completion (a 1h --timeout-ms burns the + // whole window first) and refuses at the final write instead. + expect(stderr).toContain('not writable'); + expect(stderr).not.toContain('cannot write capture output'); + }, + ); + + it.skipIf(process.getuid?.() === 0)( + 'refuses a --cwd the process cannot ENTER, not just a missing one', + async () => { + // statSync alone passes a mode-644 directory; entering it needs +x — + // tmux would exit 0 and silently run the pane in the launcher's cwd + // while the manifest records the requested one. + const blocked = join(dir, 'blocked'); + mkdirSync(blocked, { mode: 0o644 }); + await run({ cwd: blocked }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }, + ); + + it('refuses a --cwd that is not a directory', async () => { + // tmux new-session -c with a nonexistent dir exits 0 and silently runs + // the pane somewhere else — evidence from the wrong directory. + await run({ cwd: join(dir, 'no-such-dir') }); + expect(process.exitCode).toBe(3); + expect(existsSync(join(dir, 'cap.ans'))).toBe(false); + }); + + it('accepts the exact documented duration maxima', async () => { + // The refusal message promises inclusive [0, max]: a `>=` off-by-one + // would refuse a legal exactly-one-hour timeout with a + // self-contradictory message. `until` settles these in ~1s. + await run({ timeoutMs: 3_600_000 }); + expect(process.exitCode).toBeUndefined(); + await run({ settleMs: 600_000, out: join(dir, 'max2') }); + expect(process.exitCode).toBeUndefined(); + }); + + // Skipped where production deliberately omits -N: on tmux 3.1-3.2.x its + // -N FABRICATES trailing spaces (it pads to the grid allocation) with no + // -T to undo it, so the ladder trims there by design and this assertion + // would red on every run — Ubuntu 22.04 ships 3.2a, and the local tmux is + // what decides. + it.skipIf(tmuxPadsWithCaptureN(tmuxVersionProbe.stdout ?? '') === true)( + 'preserves trailing spaces in the physical frame (-N)', + async () => { + // "A clipped right edge is trailing-space significant": without -N, + // capture-pane trims the trailing run and a padding/clipping claim + // reads trimmed output as evidence. + await run({ + command: 'printf "AB \\n"; sleep 30', + until: 'AB', + }); + const ans = readFileSync(join(dir, 'cap.ans'), 'utf8'); + // At least the three printed spaces survive (tmux may pad further); + // without -N the whole trailing run is trimmed to "AB\n". + expect(ans).toMatch(/AB {3,}(\r?\n|$)/); + }, + ); + + it('maps the yargs surface — hyphenated keys reach the right fields', async () => { + // Every other test hand-builds the args object; this drives the real + // handler mapping. A wrong key (e.g. argv['settleMs']) leaves the field + // undefined, the duration guard refuses, and this test turns red — the + // option-contract bug class test-plan.test.ts documents. + await (captureTuiCommand.handler as (argv: unknown) => Promise)({ + command: `bash -c 'sleep 0.3; printf "MAPPED\\n"; cat'`, + cwd: dir, + cols: 80, + rows: 24, + 'settle-ms': 0, + ready: 'MAPPED', + until: 'typed-by-map', + keys: ['typed-by-map', 'Enter'], + out: join(dir, 'mapped'), + 'timeout-ms': 10_000, + }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse(readFileSync(join(dir, 'mapped.json'), 'utf8')); + expect(manifest.settledBy).toBe('until-match'); + // Every mapped field observable in the manifest is pinned BY VALUE — a + // settle-ms/timeout-ms swap or a wrong ready/keys argv key ships green + // otherwise (keys/ready only shape-check when !== undefined). + expect(manifest.keysSent).toBe(true); + expect(manifest.keys).toEqual(['typed-by-map', 'Enter']); + expect(manifest.ready).toBe('MAPPED'); + expect(manifest.until).toBe('typed-by-map'); + expect(manifest.timeoutMs).toBe(10_000); + expect(manifest.cwd).toBe(dir); + // Identity fields too: a command/ansPath/cols/rows mutant self- + // consistently records the lie (measured: a transposed cols/rows pair + // passes validGeometry and every prior assertion). + expect(manifest.command).toBe( + `bash -c 'sleep 0.3; printf "MAPPED\\n"; cat'`, + ); + expect(manifest.ansPath).toBe(join(dir, 'mapped.ans')); + expect(manifest.cols).toBe(80); + expect(manifest.rows).toBe(24); + }); + + it('maps settle-ms where it is OBSERVABLE — the fixed-delay shape', async () => { + // With --until set, settleMs is structurally unobservable (omitted from + // the manifest); a settle-ms→timeout-ms swap mutant shipped green until + // this invocation, where the mapping is the active duration. + await (captureTuiCommand.handler as (argv: unknown) => Promise)({ + command: 'printf "FIXED\\n"; sleep 30', + cwd: dir, + cols: 80, + rows: 24, + 'settle-ms': 123, + until: undefined, + keys: undefined, + out: join(dir, 'mapped-fixed'), + 'timeout-ms': 10_000, + }); + expect(process.exitCode).toBeUndefined(); + const manifest = JSON.parse( + readFileSync(join(dir, 'mapped-fixed.json'), 'utf8'), + ); + expect(manifest.settledBy).toBe('fixed-delay'); + expect(manifest.settleMs).toBe(123); + // Symmetric omission pin: no marker was given, so the marker budget + // must be absent (an always-spread mutant recorded timeoutMs:10000 in a + // fixed-delay manifest). + expect(manifest.timeoutMs).toBeUndefined(); + }); + + it('declares the yargs surface — array keys, required command/out, defaults', () => { + // The mapping test drives the handler; this pins the BUILDER: dropping + // array:true from keys refuses the documented `--keys "/review" Enter` + // usage while every handler-level test stays green. + const options: Record> = {}; + const fake = { + option(name: string, cfg: Record) { + options[name] = cfg; + return this; + }, + }; + (captureTuiCommand.builder as (y: unknown) => unknown)(fake); + expect(options['keys']?.['array']).toBe(true); + // type:'string' is load-bearing for keys: yargs coerces UNTYPED array + // values to numbers (measured: `--keys 3 Enter` → [3, 'Enter']), and a + // numeric token is a legitimate send-keys shape — untyped, it hits the + // shape guard's misleading "--keys must be strings." refusal. + expect(options['keys']?.['type']).toBe('string'); + expect(options['command']?.['type']).toBe('string'); + expect(options['out']?.['type']).toBe('string'); + expect(options['command']?.['demandOption']).toBe(true); + expect(options['out']?.['demandOption']).toBe(true); + expect(options['cols']?.['default']).toBe(80); + expect(options['rows']?.['default']).toBe(24); + expect(options['settle-ms']?.['default']).toBe(3000); + expect(options['timeout-ms']?.['default']).toBe(60_000); + expect(options['ready']?.['type']).toBe('string'); + // until/cwd must be DECLARED at all: reviewCommand registers under + // .strict(), so a dropped .option('until') rejects the flagship + // documented usage with "Unknown argument" while every handler-level + // test stays green. And the numeric options must be type:'number' — + // as strings, '--settle-ms 600' parses to '600' and the duration + // guard refuses a legal value. + expect(options['until']?.['type']).toBe('string'); + expect(options['cwd']?.['type']).toBe('string'); + for (const numeric of ['cols', 'rows', 'settle-ms', 'timeout-ms']) { + expect(options[numeric]?.['type']).toBe('number'); + } + }); + + it('pins the production freeze render defaults — the belt is 30s, the bin is freeze', () => { + // The belt test overrides-and-restores; without this pin a mutant + // shipping timeoutMs: 5_000 (or a renamed bin) is invisible. + expect(freezeRender.timeoutMs).toBe(30_000); + expect(freezeRender.bin).toBe('freeze'); + // Same declaration-pin for the match budget: the wall-clock gate alone + // tolerates any value up to ~7s, silently inflating every poll + // iteration past the shared deadline. + expect(MATCH_BUDGET_MS).toBe(500); + expect(tmuxControl.timeoutMs).toBe(15_000); + expect(probeBudget.timeoutMs).toBe(10_000); + expect(holderInit.timeoutMs).toBe(10_000); + }); + + it.skipIf(!hasPgrep)( + 'reaps the private server when the capture is signalled mid-poll — SIGTERM and SIGINT', + async () => { + // The no-orphan guarantee cannot rest on finally alone — a signal + // skips it. Spawn the capture as a child, kill it mid --until poll, + // and assert nothing named for the CHILD's pid survives. BOTH + // signals: deleting only the SIGINT registration shipped green while + // an operator's Ctrl+C left server, socket and holder alive. + // vitest's transform does not guarantee a usable file: import.meta.url; + // resolve from the working directory (package root or repo root). + let captureTuiTs = join( + process.cwd(), + 'src/commands/review/capture-tui.ts', + ); + if (!existsSync(captureTuiTs)) { + captureTuiTs = join( + process.cwd(), + 'packages/cli/src/commands/review/capture-tui.ts', + ); + } + expect(existsSync(captureTuiTs)).toBe(true); + const { spawn } = await import('node:child_process'); + // The FULL registration set, behaviorally: a registration mutant + // dropping SIGHUP/SIGQUIT shipped green while the membership pin + // still saw all four members. + for (const signal of REAP_SIGNALS) { + const outBase = join(dir, `sig-${signal}`); + const driver = join(dir, `driver-${signal}.mts`); + writeFileSync( + driver, + [ + `const { runCaptureTui } = await import(${JSON.stringify(pathToFileURL(captureTuiTs).href)});`, + `await runCaptureTui({ command: 'sleep 300', cwd: ${JSON.stringify(dir)}, cols: 80, rows: 24, settleMs: 0, until: 'NEVER-MATCHES', keys: undefined, out: ${JSON.stringify(outBase)}, timeoutMs: 60_000 } as never);`, + ].join('\n'), + ); + const child = spawn(process.execPath, ['--import', 'tsx', driver], { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'pipe'], + }); + const childPid = child.pid as number; + // Attached BEFORE the discovery loop, not after the kill: if the + // child dies on its own during discovery — precisely the crash + // regression this test polices — the sole `exit` event fires + // unobserved, the disposition promise never settles, and the test + // fails as a bare 15s timeout naming neither exit code nor signal + // (probe-verified 10/10 in the attach-after-kill shape: the + // surviving server still satisfies the loop and child.kill() + // returns false silently). + const disposition = new Promise<[number | null, NodeJS.Signals | null]>( + (resolve) => child.once('exit', (c, sig) => resolve([c, sig])), + ); + let seen = false; + for (let i = 0; i < 200 && !seen; i++) { + const r = spawnSync( + 'pgrep', + ['-f', `qwen-review-capture-${childPid}-`], + { encoding: 'utf8' }, + ); + if ((r.stdout ?? '').trim() !== '') seen = true; + else await sleep(50); + } + // Kill on ANY exit from here, including a thrown assertion: the + // regression this test polices is exactly "the child did not do + // what it should", and leaving it inside a 60s capture orphaned a + // node process (and its tmux server) on every such failure. + const orphanGuard = setTimeout(() => child.kill('SIGKILL'), 90_000); + try { + expect(seen).toBe(true); + } catch (e) { + child.kill('SIGKILL'); + clearTimeout(orphanGuard); + throw e; + } + clearTimeout(orphanGuard); + child.kill(signal); + // Capture the disposition: the re-raise half of the contract — the + // handler reaps FIRST and then re-raises, so the child must die OF + // the signal (the conventional exit disposition). A dropped + // re-raise reads normal completion to a harness killing a wedged + // capture (probe-verified: the exact mutant passed the + // exit-event-only version of this wait). + const [code, exitSignal] = await disposition; + expect(exitSignal).toBe(signal); + expect(code).toBeNull(); + // The reap ran before the re-raise: no server named for the child. + let gone = false; + for (let i = 0; i < 40 && !gone; i++) { + const r = spawnSync( + 'pgrep', + ['-f', `qwen-review-capture-${childPid}-`], + { encoding: 'utf8' }, + ); + if ((r.stdout ?? '').trim() === '') gone = true; + else await sleep(50); + } + expect(gone).toBe(true); + } + }, + ); + + it(// No pgrep needed: sentinel + child disposition only. + 'dies OF the signal even when it lands during the render window', async () => { + // The tail after reap is synchronous (freeze render up to its belt); + // a queued signal must drain to the handler before the listeners go + // away — without the drain the process exited 0 with the success JSON + // as if the harness's kill never landed. + let captureTuiTs = join( + process.cwd(), + 'src/commands/review/capture-tui.ts', + ); + if (!existsSync(captureTuiTs)) { + captureTuiTs = join( + process.cwd(), + 'packages/cli/src/commands/review/capture-tui.ts', + ); + } + const slowFreeze = join(dir, 'slow-freeze'); + // The sentinel line is what the wait below keys on — one write, so the + // fixture a maintainer edits is the one the child runs. + const renderStarted = join(dir, 'render-started'); + writeFileSync( + slowFreeze, + `#!/bin/sh\n: > "${renderStarted}"\n/bin/sleep 4\nprintf x > "$5"\n`, + { mode: 0o755 }, + ); + const driver = join(dir, 'driver-render.mts'); + writeFileSync( + driver, + [ + `const mod = await import(${JSON.stringify(pathToFileURL(captureTuiTs).href)});`, + `mod.probes.freeze = () => ({ status: 'ok', out: '' });`, + `mod.freezeRender.bin = ${JSON.stringify(slowFreeze)};`, + `await mod.runCaptureTui({ command: 'printf "RSIG\\n"; sleep 30', cwd: ${JSON.stringify(dir)}, cols: 80, rows: 24, settleMs: 0, until: 'RSIG', keys: undefined, out: ${JSON.stringify(join(dir, 'rsig'))}, timeoutMs: 30_000 } as never);`, + ].join('\n'), + ); + const { spawn } = await import('node:child_process'); + const child = spawn(process.execPath, ['--import', 'tsx', driver], { + cwd: process.cwd(), + stdio: ['ignore', 'pipe', 'pipe'], + }); + let waited = 0; + while (!existsSync(renderStarted) && waited < 200) { + await sleep(50); + waited++; + } + expect(existsSync(renderStarted)).toBe(true); + child.kill('SIGTERM'); + const disposition = await new Promise<{ + code: number | null; + signal: string | null; + }>((resolve) => + child.once('exit', (code, signal) => resolve({ code, signal })), + ); + // Death BY the signal — not a swallowed exit 0 with success JSON. + expect(disposition.signal ?? `code:${disposition.code}`).toBe('SIGTERM'); + }); + + it('renders through a stdin-IGNORING spawn — a pipe stdin breaks freeze', async () => { + // The production spawn sets stdio ignore because freeze treats a + // non-/dev/null stdin as "the input is stdin" (measured: EOF'd pipe → + // "ERROR No input", exit 1). spawnSync's pipe stdin EOFs at spawn — it + // never blocks — so this fake discriminates by SHAPE: fd 0 must be the + // /dev/null character device, or it fails the way real freeze does. + await withFakeFreeze( + '#!/bin/sh\nif [ ! -c /dev/stdin ]; then echo "ERROR No input" >&2; exit 1; fi\nprintf x > "$5"\nexit 0\n', + () => run(), + ); + const manifest = JSON.parse(readFileSync(join(dir, 'cap.json'), 'utf8')); + expect(manifest.evidence).toBe('png'); + }); +}); diff --git a/packages/cli/src/commands/review/capture-tui.ts b/packages/cli/src/commands/review/capture-tui.ts new file mode 100644 index 00000000000..e3740be13db --- /dev/null +++ b/packages/cli/src/commands/review/capture-tui.ts @@ -0,0 +1,1373 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// `qwen review capture-tui`: run a command in a throwaway terminal and hand +// back what it actually rendered — the evidence producer for rendering claims. +// +// A verifier ruling on "the panel clips at 80 columns" without this command +// reads the layout code and imagines a terminal; measured on this repo, the +// imagining is where rendering verdicts go wrong. This command makes the +// terminal real and the evidence a file: +// +// tmux (PRIVATE server, -L) → .ans (pane text with escapes, always) +// → .png (freeze-rendered, when available) +// +// The safety property is isolation, and it is structural: every tmux call is +// scoped to a per-run private server socket, so the capture cannot see — +// let alone resize or kill — the user's own tmux sessions. The measured +// failure mode of desktop-automation verification was exactly "drives the +// user's own windows"; a private server makes that impossible rather than +// discouraged. `kill-server` at the end reaps everything the capture started. +// +// Degradation is explicit, not silent: the manifest names which evidence rung +// was reached (`png` or `ans-only`) and why, because a verifier must say +// which rung its verdict stands on — a PNG is publishable rendering evidence, +// an .ans proves bytes but not pixels, and prose is neither. A REFUSED +// capture writes no manifest at all — the bottom rung (`none`) is reported by +// the refusal JSON on stdout instead, so a missing manifest reads as a +// refusal, not corruption. + +import type { CommandModule } from 'yargs'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { randomBytes } from 'node:crypto'; +import { + accessSync, + closeSync, + constants as fsConstants, + existsSync, + mkdirSync, + openSync, + readFileSync, + rmSync, + lstatSync, + statSync, + writeFileSync, +} from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { createContext, runInContext, type Context } from 'node:vm'; +import { + writeStdoutLine, + writeStdoutLineSafe, + writeStderrLine, + writeStderrLineSafe, +} from '../../utils/stdioHelpers.js'; +import { + DEFAULT_COLS, + DEFAULT_ROWS, + captureServerName, + freezePlan, + tmuxPlan, + tmuxSupportsCaptureN, + tmuxSupportsCaptureT, + tmuxPadsWithCaptureN, + isNothingToKill, + validGeometry, + type CaptureManifest, +} from './lib/tui-capture.js'; + +interface CaptureTuiArgs { + command: string; + cwd: string | undefined; + cols: number; + rows: number; + settleMs: number; + until: string | undefined; + ready: string | undefined; + keys: string[] | undefined; + out: string; + timeoutMs: number; +} + +/** The availability-probe deadline, a seam like its two belt siblings: a + * hanging `tmux -V`/`freeze --help` would otherwise block runCaptureTui + * before the refusal contract or any signal handler exists — and without + * the seam the belt itself is untestable. */ +export const probeBudget = { timeoutMs: 10_000 }; + +/** The holder-init sentinel deadline, a seam like the other belts — a pane + * that dies at startup must refuse within it, and without the seam neither + * the floor nor the ceiling is provable. */ +export const holderInit = { timeoutMs: 10_000 }; + +type ProbeResult = + | { status: 'ok'; out: string } + | { status: 'absent' } + // Belt-killed: present but not answering. Reported distinctly — an + // operator told "not installed" for a wedged binary goes to fix an + // installation that exists (the freeze render path names its belt kill + // for the same reason). + // `code` is set when the probe answered neither cleanly nor with + // absence. `spawned` distinguishes the two ways that happens: false when + // the spawn itself failed (EMFILE/ENFILE/EACCES — the binary may be + // perfectly installed and this host could not start it), true when the + // binary RAN and failed (a non-zero exit, a signal). The messages say + // which, because "could not spawn it" is a false claim about the second. + | { status: 'hung'; code?: string; spawned?: boolean }; + +/** Probe the binary itself (`tmux -V` / `freeze --help`), not `which`: a + * host without `which` would otherwise misdiagnose an installed binary as + * missing, and the binary answering is the only fact that matters. Answers + * with a ProbeResult — `ok` carries the trimmed stdout, and a belt-killed + * binary is `hung`, NEVER `absent`: an operator told "not installed" for a + * wedged binary goes to fix an installation that exists. */ +function probeOutput(bin: string, flag: string): ProbeResult { + const r = spawnSync(bin, [flag], { + encoding: 'utf8', + timeout: probeBudget.timeoutMs, + // SIGKILL, not the default SIGTERM: a TERM-immune child (trap '' TERM) + // blocks the sync spawn past any belt — measured unkillable except by + // external SIGKILL. + killSignal: 'SIGKILL', + stdio: ['ignore', 'pipe', 'pipe'], + }); + if (r.status === 0) return { status: 'ok', out: (r.stdout ?? '').trim() }; + const code = r.error && (r.error as NodeJS.ErrnoException).code; + if (code === 'ETIMEDOUT') return { status: 'hung' }; + // A spawn that could not even be attempted is NOT an absent binary: under + // fd exhaustion (the transient condition this file names three times) + // both probes reported 'not installed', and that false environment claim + // then persisted into the manifest's degradedBecause. ENOENT is the only + // answer that means absent; the rest are this host, right now. + if (code && code !== 'ENOENT') { + return { status: 'hung', code, spawned: false }; + } + // Nor is a binary that RAN and failed: an installed freeze whose --help + // exits non-zero, or dies on a signal, spawned fine — reporting it as not + // installed sends an operator to install something already present. + if (r.status !== null && r.status !== 0) { + return { status: 'hung', code: `exit ${String(r.status)}`, spawned: true }; + } + if (r.signal) { + return { status: 'hung', code: `signal ${r.signal}`, spawned: true }; + } + return { status: 'absent' }; +} + +/** The freeze render invocation, exported as a seam: the 30s belt against a + * wedged freeze (measured hangs on this repo's own workflows) is otherwise + * untestable — a test cannot wait out the real value to prove the belt + * exists — and `bin` lets a test point the render at a fake binary by + * absolute path (a PATH shim is skipped by execvp when non-executable). + * Tests override and restore; production never does. */ +export const freezeRender = { bin: 'freeze', timeoutMs: 30_000 }; + +/** The availability probes, exported as a seam: the no-tmux refusal fires + * exactly where `describe.skipIf(!hasTmux)` skips the real-tmux tests, so + * without this seam that path is untestable in the one environment where it + * matters. Tests override a probe and restore it; production never does. */ +export const probes = { + // The VERSION LINE, not a boolean: capture-pane -N needs tmux 3.1, and a + // host with an older tmux must be told so up front — the real cause named, + // no server started — instead of dying mid-capture on the unknown flag. + tmux: (): ProbeResult => probeOutput('tmux', '-V'), + // `--help`, not `--version`: freeze ≤0.1.6 (the whole 2024 release line) + // has no --version flag and would be misdiagnosed as absent; --help exits + // 0 on both release lines (measured on v0.1.6 and v0.2.2). + freeze: (): ProbeResult => probeOutput('freeze', '--help'), +}; + +/** The signals Node lets JavaScript observe whose default action would + * otherwise terminate the process past the finally-based reap. NOT every + * such signal exists here — SIGUSR2, SIGALRM, SIGXCPU and friends still + * kill the process with the server standing, and SIGKILL cannot be caught + * at all; the holder's own watchdog is what bounds those: a closed terminal window HUPs the foreground process + * group (measured: exit 129 with server, socket and holder all surviving), + * and the capture window legally runs up to an hour. Exported so the signal + * tests iterate the REAL list. */ +/** A capture manifest is a few hundred bytes; anything past this is not + * one, and reading it would cost more than refusing does. */ +const MAX_MANIFEST_BYTES = 1024 * 1024; + +export const REAP_SIGNALS = ['SIGINT', 'SIGTERM', 'SIGHUP', 'SIGQUIT'] as const; + +/** The tmux control-call deadline, exported as a seam like its freezeRender + * sibling: belt-less, a wedged server blocks the first execFileSync forever + * — and in reap() the process could not even die on the re-raised signal. + * Tests shorten it; production never does. */ +export const tmuxControl = { timeoutMs: 15_000 }; + +function tmux(argv: string[]): string { + return execFileSync('tmux', argv, { + encoding: 'utf8', + // A pane of text is small; a runaway TUI writing a scrollback is not our + // problem — capture-pane returns the visible pane only. + maxBuffer: 8 * 1024 * 1024, + // Every tmux command here is a quick control call; a server wedged hard + // enough to sit on one this long should turn into a refusal, not hang + // the whole review agent behind it. + timeout: tmuxControl.timeoutMs, + killSignal: 'SIGKILL', + }) as string; +} + +/** Async on purpose: the waits dominate the capture's wall time, and an + * idle event loop is what lets the SIGINT/SIGTERM reap below actually run — + * a fully synchronous capture would queue the signal until after the work + * it was meant to interrupt. */ +function sleep(ms: number): Promise { + return new Promise((r) => setTimeout(r, ms)); +} + +/** One marker match's time budget: a backtracking-prone --until/--ready + * pattern can spin a single test() call past any deadline (the deadline is + * only checked BETWEEN calls). vm interrupts the match; an overrun is + * reported as such — it is "the match was cut off", NOT "no match". + * Exported so the declaration test can pin the VALUE — the wall-clock test + * alone tolerates anything up to ~7s, silently inflating every poll + * iteration past the shared deadline. */ +export const MATCH_BUDGET_MS = 500; + +// ONE context for every match of the run: the interrupt property lives in +// the timeout option, which runInContext keeps — runInNewContext built a +// fresh V8 context per poll (~14k of them across a 1h capture). +// createContext wires the sandbox object in place, so writing re/text before +// each match feeds the context without re-creating it. +const matchSandbox: { re: RegExp; text: string } = { re: /(?!)/, text: '' }; + +const matchContext: Context = createContext(matchSandbox); + +function testWithBudget( + re: RegExp, + text: string, +): 'match' | 'miss' | 'overrun' { + matchSandbox.re = re; + matchSandbox.text = text; + try { + return runInContext('re.test(text)', matchContext, { + timeout: MATCH_BUDGET_MS, + }) + ? 'match' + : 'miss'; + } catch { + return 'overrun'; + } +} + +let brokenPipeGuarded = false; +/** The contract writes (refusal/success JSON, stderr summary, the reap + * WARNING) must never flip the exit disposition: a gone reader raises an + * ASYNC EPIPE 'error' event that crashes the process at exit 1 with a stack + * trace where the machine-read contract promised exit 3 (or 0). Swallow + * EPIPE only; anything else stays loud. */ +/** Set once this run's artifacts are on disk AND described by a manifest. + * From that moment a stdio failure cannot change what happened: the + * evidence exists, so an exit 1 with a stack trace would report a + * successful capture as a failed command. */ +let artifactsComplete = false; + +function guardBrokenPipes(): void { + if (brokenPipeGuarded) return; + brokenPipeGuarded = true; + const swallow = (err: NodeJS.ErrnoException): void => { + // EPIPE always: a reader that left (`qwen … | head`) is not this + // command's failure. Anything else only once the evidence is complete — + // measured with an ENOSPC shim and stdout redirected to a file, the + // async 'error' event arrived AFTER a fully successful capture and + // rethrowing it exited 1 with the .ans and manifest both on disk. Before + // that point a stdio fault still propagates: it can mean the refusal + // itself never reached anyone. + if (err.code !== 'EPIPE' && !artifactsComplete) throw err; + }; + process.stdout.on('error', swallow); + process.stderr.on('error', swallow); +} + +export async function runCaptureTui(args: CaptureTuiArgs): Promise { + // Per RUN, not per process: the guard is installed once, but a second + // capture in the same process must not inherit the first one's completion. + artifactsComplete = false; + guardBrokenPipes(); + const refuse = (reason: string): void => { + // Exit code FIRST: it is the disposition a harness reads, and the + // writes below can throw synchronously on a closed fd. + process.exitCode = 3; + // TWO try blocks, not one: a synchronous throw from the stderr write — + // a file-backed stderr under ENOSPC throws sync in Node — used to skip + // the stdout JSON entirely, so a HEALTHY stdout got nothing (measured: + // exit 3 with empty stdout). The consumer is an agent that must tell an + // environment refusal from a caller mistake without scraping stderr, so + // the machine-readable half must not depend on the human-readable one. + try { + writeStderrLine(`capture-tui: refused — ${reason}`); + } catch { + // A gone reader cannot un-refuse: the exit code already carries it. + } + try { + // `evidence: 'none'` names the ladder's bottom rung. + writeStdoutLine( + JSON.stringify({ captured: false, evidence: 'none', reason }), + ); + } catch { + // Same. + } + }; + + // --out's shape FIRST, then the stale-artifact clear, then every other + // gate: any refusal that fires with a valid --out and stale artifacts + // still in place hands a consumer the PREVIOUS run's manifest next to + // this run's refusal JSON (measured: a duplicated --command flag against + // a reused --out left all three prior artifacts, manifest still claiming + // "evidence":"png"). Only an --out we cannot even name is allowed to + // refuse without clearing — there is nowhere to clear. + if (typeof args.out !== 'string') { + refuse('--out must be given exactly once, as a string.'); + return; + } + if (args.out.trim() === '') { + // resolve('') is the cwd: artifacts would land as .ans/.png/.json + // NEXT TO the working directory, silently clobbering whatever holds + // those names (the brief's template with an empty variable hits this). + refuse('--out must not be empty.'); + return; + } + const outBase = resolve(args.out); + const ansPath = `${outBase}.ans`; + const pngPath = `${outBase}.png`; + const manifestPath = `${outBase}.json`; + const holderReadyPath = `${outBase}.holder-ready`; + // What sat at each artifact path after the clear phase, by identity. + // `changed` answers the one question the evidence rung and every cleanup + // both ask: did THIS run put it there? + interface Stamp { + existed: boolean; + size: number; + mtimeMs: number; + ino: number; + } + // lstat, never stat: a SYMLINK is an occupant in its own right. Following + // it made a dangling link read as "nothing here" — the collision gate + // never fired and the .ans and manifest writes then followed the link + // OUT of the --out base (probe-verified end to end: a run reported + // success with the manifest naming .json while the bytes landed at + // the link's target). A live-target link was refused correctly; only the + // dangling shape slipped through. + const stampOf = (path: string): Stamp => { + try { + const st = lstatSync(path); + return { existed: true, size: st.size, mtimeMs: st.mtimeMs, ino: st.ino }; + } catch { + return { existed: false, size: 0, mtimeMs: 0, ino: 0 }; + } + }; + const changed = (path: string, stamp: Stamp): boolean => { + // Never taken: nothing here is ours to credit or remove. + if (stamp.size === UNSTAMPED && stamp.ino === UNSTAMPED) return false; + if (!stamp.existed) return occupied(path); + try { + const st = lstatSync(path); + return ( + st.ino !== stamp.ino || + st.size !== stamp.size || + st.mtimeMs !== stamp.mtimeMs + ); + } catch { + // Gone, or unstattable: nothing of ours to credit or remove. + return false; + } + }; + // Fail-safe by CONSTRUCTION, not by comment: `changed()` compares + // identity when `existed` is true, and an impossible ino made every real + // file look changed — the exact opposite of the promise, and it would + // have authorized deletes and credited an evidence rung. A sentinel that + // is byte-equal to nothing must instead answer "unchanged" for anything, + // which is what a NaN-free impossible SIZE plus a matching guard does: + // `changed()` returns false whenever the stamp is this sentinel. + const UNSTAMPED = -1; + const untouched: Stamp = { + existed: true, + size: UNSTAMPED, + mtimeMs: UNSTAMPED, + ino: UNSTAMPED, + }; + // Occupancy is a LINK-level question everywhere it is asked: existsSync + // follows symlinks, so a dangling one answered false at every gate below. + const occupied = (path: string): boolean => { + try { + lstatSync(path); + return true; + } catch { + return false; + } + }; + let ansStamp: Stamp = untouched; + let pngStamp: Stamp = untouched; + let manifestStamp: Stamp = untouched; + try { + // Clears FIRST — before even mkdir and before the directory-shaped + // --out refusal below: under fd exhaustion (EMFILE, measured on macOS) + // mkdirSync itself can throw, and EVERY nameable refusal must leave no + // stale capture evidence ("only an --out we cannot even name refuses + // without clearing"). But clear ONLY what looks like a previous + // capture: the artifact names are not reserved, and a colliding --out + // must not force-delete an unrelated file on a run that later refuses + // (measured: --out package deleted package.json at the --cols 0 + // refusal — the brief's template puts --out inside the plan dir). A + // manifest that parses with an evidence rung is the capture's own + // signature; .ans/.png/.json clear alongside it. The sentinel clears + // unconditionally: this tool alone writes it, and a stale one from a + // SIGKILL'd run would pass the ready gate before the new holder + // installs its trap. + const clearArtifact = (path: string): void => { + // Plain unlink only — no descriptor needed, so it survives EMFILE — + // and a throw is SWALLOWED rather than escalated to a recursive rm. + // A capture writes files; a DIRECTORY at an artifact path was made by + // someone else, and the recursive fallback destroyed it and its + // contents on every re-run against the documented same-`--out` usage + // (a stale shaped manifest is the normal state from the second run + // on — no fd exhaustion needed). Skipping it keeps the clear going + // for the other paths; a directory still standing where this run must + // write turns into the write-failure refusal downstream, which is the + // fail-closed outcome. + try { + rmSync(path, { force: true }); + } catch { + // Not ours to delete (EISDIR), or unlinkable for a reason the + // write probe below will name. + } + }; + let shaped = false; + let manifestHadPng = false; + try { + // A FIFO here would block readFileSync FOREVER — a hang, not a throw, + // so no refusal is printed, no reap handler is installed yet, and NO + // timeout inside the process can interrupt it: the read is + // synchronous on the main thread (measured — a vitest run wedged past + // its own 10s test timeout until the runner killed it). Only a + // regular file can be a capture manifest anyway; anything else is + // unverifiable, which the catch below already treats as "not ours". + const st = lstatSync(manifestPath); + if (!st.isFile()) throw new Error('not a file'); + // A capture manifest is a few hundred bytes. Reading an arbitrarily + // large regular file here killed the process before any refusal could + // print — measured, a 479MB dense JSON at .json hit + // `FATAL ERROR: Reached heap limit` — and lstat already has the size, + // so the cap costs nothing. Too big to be ours: treat as unverified. + if (st.size > MAX_MANIFEST_BYTES) throw new Error('too large'); + const m = JSON.parse(readFileSync(manifestPath, 'utf8')) as { + evidence?: unknown; + pngPath?: unknown; + }; + shaped = + m !== null && + typeof m === 'object' && + (m.evidence === 'png' || m.evidence === 'ans-only'); + // The png clear needs the manifest to have CLAIMED a png: either the + // png rung itself or a recorded pngPath. An ans-only manifest says + // this tool never wrote one, so whatever sits at .png belongs to + // someone else — and clearing it on the next run against the same + // --out (the documented reuse shape) destroyed a foreign file the + // previous run had deliberately spared. + manifestHadPng = + shaped && (m.evidence === 'png' || typeof m.pngPath === 'string'); + } catch { + // ANY read failure means the capture signature could not be verified — + // including fd exhaustion (EMFILE/ENFILE), which is transient under + // the concurrent captures the briefs encourage. Unverified is NOT + // permission to delete: clearing here deleted unrelated user files at + // the artifact names on a run that then refused (measured), against + // this block's own invariant. Stale evidence left beside a refusal is + // the lesser harm — the refusal names itself, a deleted file does not. + shaped = false; + } + if (shaped) { + clearArtifact(ansPath); + if (manifestHadPng) clearArtifact(pngPath); + clearArtifact(manifestPath); + } + // AFTER the clears, never before: this unlink is the one that may throw + // (a DIRECTORY at the sentinel path gives EISDIR, which `force` does not + // suppress) and its throw refuses. Run first, it stranded the previous + // run's artifacts beside the refusal (measured) — the exact wrong- + // evidence outcome the clear-first contract exists to prevent. The + // sentinel itself clears UNCONDITIONALLY and as a PLAIN file: this tool + // alone writes it, a stale one from a SIGKILL'd run would pass the ready + // gate before the new holder installs its trap, and a directory there is + // a user's — recursive removal would destroy it on every run (measured: + // a seeded content-bearing directory deleted even by successful runs). + rmSync(holderReadyPath, { force: true }); + // Anything still sitting at an artifact path is something this run did + // not write and did not clear (an unverifiable manifest, an unrelated + // file the signature check spared). Stamp all three BY IDENTITY: the + // write probe proves only that a path was writable BEFORE the capture + // window, and a write can still fail at open() — under the fd + // exhaustion this file names three times as measured — having + // truncated nothing, leaving the previous file whole. Deleting then + // destroys a file this run never touched; crediting one as this run's + // rendering evidence is worse still. + ansStamp = stampOf(ansPath); + pngStamp = stampOf(pngPath); + manifestStamp = stampOf(manifestPath); + mkdirSync(dirname(outBase), { recursive: true }); + // Probe the actual write target BEFORE any process starts: mkdirSync + // with `recursive` does no permission check on a directory that already + // exists, so an unwritable --out would otherwise run the full capture — + // up to the 1h ceiling — and lose the pane text at the very last write. + // The probe uses a UNIQUE sibling, never the .ans itself: truncating the + // real one would delete the previous run's text while its manifest/png + // survive to misdescribe it. + const probePath = `${outBase}.write-probe-${randomBytes(4).toString('hex')}`; + const fd = openSync(probePath, 'w'); + closeSync(fd); + rmSync(probePath, { force: true }); + // A file the clear phase SPARED still occupies a path this run must + // write. Refuse before anything starts rather than truncate it: the + // clear verified it is not a capture manifest's artifact, so it belongs + // to someone else, and a successful run silently replaced it (measured + // with the collision the clear's own comment names — `--out package` in + // a Node project rewrote package.json as a capture manifest at exit 0, + // no degradation recorded). A previous capture's OWN artifacts never + // reach here: the clear removed them. + // Only the MANDATORY writes refuse: the .ans and the manifest are + // written on every successful run, so a survivor there can only be + // replaced. The png is a RUNG, not a requirement — an occupied png path + // degrades the ladder instead (see the render below), which keeps a + // capture that can still produce text evidence from failing outright. + for (const path of [ansPath, manifestPath]) { + if (!occupied(path)) continue; + refuse( + `--out collides with a file this capture did not write: ${path}. ` + + "A previous capture's own artifacts are cleared automatically; " + + 'this one is not ours to replace. Pick another --out.', + ); + return; + } + // No per-path writability probe beyond this: the collision gate above + // refuses on ANY survivor at the two mandatory paths, so a directory, a + // read-only file, an append-only one (chattr +a, which an append-mode + // probe would have passed and the truncating final write would then + // have failed) — every shape that could fail the last write — is + // already refused before the capture window opens. + } catch (e) { + refuse( + `--out is not writable: ${e instanceof Error ? e.message : String(e)}`, + ); + return; + } + // resolve('.') and resolve('./') are the cwd itself — the same shape the + // empty guard above refuses — and any existing directory sails through: + // artifacts would land as .ans/.png/.json NEXT TO the directory, + // silently clobbering whatever holds those names. AFTER the clears, like + // every other nameable refusal. + let outIsDirectory = false; + try { + outIsDirectory = statSync(outBase).isDirectory(); + } catch { + // A nonexistent out base is the normal shape. + } + if (outIsDirectory) { + refuse(`--out must not name an existing directory: ${outBase}`); + return; + } + // Remaining shape guards: yargs parses a DUPLICATED string option into an + // array (`--command A --command B` → ['A','B']) and its default --no-X + // negation into a boolean (`--no-command` → false) — both sail through + // the `as` casts and either throw uncaught TypeErrors past the refusal + // contract or silently corrupt the capture (`--until A --until B` + // compiles /A,B/; `--no-keys` types the literal word "false" into the + // pane). --command also refuses undefined: demandOption covers the CLI + // path, but runCaptureTui is exported. + if (typeof args.command !== 'string') { + refuse('--command must be given exactly once, as a string.'); + return; + } + for (const [name, v] of [ + ['--cwd', args.cwd], + ['--until', args.until], + ['--ready', args.ready], + ] as const) { + if (v !== undefined && typeof v !== 'string') { + refuse(`${name} must be given exactly once, as a string.`); + return; + } + } + if (args.keys !== undefined) { + if ( + !Array.isArray(args.keys) || + args.keys.some((k) => typeof k !== 'string') + ) { + refuse('--keys must be strings.'); + return; + } + // An EXACTLY empty token types nothing (`send-keys ''` is a no-op), and + // the run then reports success with the token recorded in the manifest + // and keysSent true — a keypress a verdict can cite that never happened, + // the same evidence-corruption class as an unescaped trailing `;`. A + // brief template expanding an empty variable produces exactly this. A + // token of one SPACE is real input and stays legal (measured: it types). + if (args.keys.some((k) => k === '')) { + refuse('--keys must not contain an empty token.'); + return; + } + // A BARE `--keys` (or `--keys=`, or an unquoted `--keys $EMPTY`) parses + // to [] under yargs `array: true` and used to be accepted silently: the + // run typed nothing and reported success, while the quoted form of the + // very same template failure (['']) refused. Both are a brief that + // meant to drive the TUI and did not — the manifest must not carry a + // keys field the run never acted on. + if (args.keys.length === 0) { + refuse('--keys was given with no tokens.'); + return; + } + } + const tmuxProbe = probes.tmux(); + if (tmuxProbe.status === 'hung') { + refuse( + tmuxProbe.code + ? `tmux could not be probed (${tmuxProbe.code}) — ` + + (tmuxProbe.spawned + ? 'the binary ran and failed, so it is installed but not ' + + 'usable here. Fix the installation; rendering claims stay ' + + 'argued from the code until it answers.' + : 'the binary may be installed; this host could not spawn ' + + 'it. Retry once the condition clears; rendering claims ' + + 'stay argued from the code until it answers.') + : `tmux did not answer -V within ${probeBudget.timeoutMs}ms — present ` + + 'but wedged, not absent. Fix or restart the tmux binary; rendering ' + + 'claims stay argued from the code until it answers.', + ); + return; + } + if (tmuxProbe.status === 'absent') { + refuse( + 'tmux is not installed. Rendering claims stay argued from the code on ' + + 'this host; say so in the finding rather than describing an imagined ' + + 'terminal as evidence.', + ); + return; + } + const tmuxVersion = tmuxProbe.out; + if (tmuxSupportsCaptureN(tmuxVersion) === false) { + refuse( + `${tmuxVersion} is too old: capture-pane -N needs tmux 3.1 or newer.`, + ); + return; + } + const geometry = validGeometry(args.cols, args.rows); + if (!geometry.ok) { + refuse(geometry.reason); + return; + } + if (args.command.trim() === '') { + refuse('--command must not be empty.'); + return; + } + // No trailing-backslash gate: the two-shell holder single-quotes the + // command at every layer (esc balances every quote), so no shell ever + // parses the command text adjacent to the hold line — probe-verified with + // four odd-run shapes on the production plan, all executed correctly with + // the holder alive. The old gate protected the earlier single-shell + // holder and over-refused valid commands. + if (args.cwd !== undefined) { + // tmux new-session -c with an unusable directory exits 0 and silently + // runs the pane in the launching process's cwd — evidence from the + // wrong directory with nothing recording the swap. stat alone is not + // enough: a directory without +x stats fine but cannot be entered + // (measured: mode-644 --cwd passed the gate and the manifest lied). + let usable = false; + try { + const abs = resolve(args.cwd); + usable = statSync(abs).isDirectory(); + if (usable) accessSync(abs, fsConstants.X_OK); + } catch { + usable = false; + } + if (!usable) { + refuse(`--cwd is not an enterable directory: ${args.cwd}`); + return; + } + } + // yargs coerces a non-numeric `--settle-ms abc` to NaN, and a NaN + // deadline makes the --until poll loop unexpirable (`now >= NaN` is + // always false). Refuse, don't hang — and refuse the out-of-bounds + // values too, so a day-long timeout cannot be requested by typo. + for (const [name, v, max] of [ + ['--settle-ms', args.settleMs, 600_000], + ['--timeout-ms', args.timeoutMs, 3_600_000], + ] as const) { + if (!Number.isFinite(v) || v < 0 || v > max) { + refuse(`${name} must be a number in [0, ${max}], got ${String(v)}`); + return; + } + } + // Validate the regex BEFORE any process starts: an invalid pattern is a + // caller mistake and gets the refusal contract, not a stack trace thrown + // from inside a running capture. + // --ready: measured on this repo's own onboarding TUI, keys fired at start + // straddle the UI's mount — a Down was consumed and the Enter behind it + // lost — so key-driven captures of anything that takes a moment to render + // are unreliable without a gate. The gate is a marker, like --until. + // A marker that matches a BLANK pane settles before the TUI rendered + // anything — a false settle claim (or keys fired into a still-mounting UI, + // the exact failure --ready exists to prevent). Empty/whitespace patterns + // are the obvious case; `.?`, `x*`, `^`, `\s` and `\n` are the sneaky + // ones: the blank pane's logical capture is rows of newlines, not the + // empty string, so both oracles are probed (through the match budget — a + // backtracking pattern must not hang the gate itself). + const compileMarker = ( + name: '--ready' | '--until', + pattern: string, + ): RegExp | { error: string } => { + if (pattern.trim() === '') return { error: `${name} must not be empty.` }; + let re: RegExp; + try { + re = new RegExp(pattern); + } catch (e) { + return { + error: `${name} is not a valid regex: ${e instanceof Error ? e.message : String(e)}`, + }; + } + const blankPane = '\n'.repeat(args.rows); + if ( + testWithBudget(re, '') === 'match' || + testWithBudget(re, blankPane) === 'match' + ) { + return { + error: + `${name} ${JSON.stringify(pattern)} matches a blank pane — it ` + + `would settle before the UI rendered anything; pick a marker the ` + + `claim actually draws`, + }; + } + return re; + }; + let readyRe: RegExp | undefined; + if (args.ready !== undefined) { + const r = compileMarker('--ready', args.ready); + if ('error' in r) { + refuse(r.error); + return; + } + readyRe = r; + } + let untilRe: RegExp | undefined; + if (args.until !== undefined) { + const r = compileMarker('--until', args.until); + if ('error' in r) { + refuse(r.error); + return; + } + untilRe = r; + } + + const server = captureServerName(process.pid, randomBytes(4).toString('hex')); + const session = 'cap'; + const resolvedCwd = args.cwd ? resolve(args.cwd) : process.cwd(); + // Measured on 3.2a (what Ubuntu 22.04 ships): `capture-pane -p -N` padded + // a three-character line out to the grid's allocated width with 17 + // phantom spaces, and its tmux has no -T. Trailing spaces are dropped + // there rather than fabricated, and the manifest says so. + const capturePads = tmuxPadsWithCaptureN(tmuxVersion) === true; + const plan = tmuxPlan({ + server, + session, + cols: args.cols, + rows: args.rows, + command: args.command, + cwd: resolvedCwd, + // Only 3.4+ has the flag; older versions have nothing to trim. + captureTrim: tmuxSupportsCaptureT(tmuxVersion) === true, + // ...and 3.1-3.2.x invent trailing spaces with -N and cannot undo it. + captureTrailing: !capturePads, + readyFile: holderReadyPath, + }); + + // The no-orphan guarantee cannot rest on `finally` alone: a SIGINT/SIGTERM + // (an operator's Ctrl+C on an un-settling capture, a harness reaping a + // stuck one) skips finally and would leave the server, its socket, and the + // captured TUI alive. The handler reaps first and then re-raises so the + // exit code stays the conventional one for the signal — removing only + // itself before the re-raise, so any OTHER handler the host process + // installed sees the signal a second time; accepted for a leaf + // subcommand, which this is. + let serverStarted = false; + let reaped = false; + const reap = (): void => { + // serverStarted is set BEFORE the start call: the call forks the + // server before it returns, so a start that threw or was belt-cut can + // still have a live server behind it — skipping the reap on that path + // orphaned server, session and holder (measured on loaded runners). + // kill-server against a server that never came up answers "no server + // running" — the goal state below — so the attempt stays warning-free. + if (reaped || !serverStarted) return; + reaped = true; + // Unlink the socket ONLY when the server is known dead: kill can throw + // with the server alive (the tmux CLIENT failing to spawn — EMFILE, a + // wedged server outlasting the 15s timeout), and unlinking then makes + // the live server unreachable forever — nothing addressable by -L can + // ever kill it again, while it holds the pane holder (the bounded + // hold loop runs up to three hours). + // One retry before giving up: a transient client-spawn failure is the + // named shape, and a second attempt reaps it (measured). + let serverDead = false; + for (let attempt = 0; attempt < 2 && !serverDead; attempt++) { + try { + tmux(plan.kill); + serverDead = true; + } catch (e) { + // A kill failing because there was nothing to kill is the goal + // state — in every wording tmux uses for it, including the + // socket-directory-never-created one a start that failed before the + // socket existed produces (measured with a mode-0555 TMUX_TMPDIR: + // both attempts answered `couldn't create directory …` and the + // one-wording test printed a false orphan WARNING). + serverDead = isNothingToKill( + String((e as { stderr?: unknown }).stderr ?? ''), + ); + } + } + if (!serverDead) { + // A presumed-alive private server is never a silent outcome: the + // holder keeps it up for up to three hours, and the briefs encourage many + // captures per review — orphans would accumulate invisibly. + // SAFE, like refuse()'s writes: process.stderr.write throws + // synchronously on a closed fd, and reap() runs BOTH from the finally + // (where a throw turns the exit-3 refusal into an exit-1 stack trace + // and skips the sentinel cleanup below it) and from onSignal (where it + // becomes an uncaughtException — exit 1 instead of 128+sig, the + // re-raise never reached, and this very warning lost). + writeStderrLineSafe( + `capture-tui: WARNING — kill-server failed twice; the private tmux ` + + `server ${server} may still be running (tmux -L ${server} kill-server to reap it by hand).`, + ); + return; + } + // tmux does not always unlink the socket of a killed server; a review + // that captures often would litter the socket dir with dead sockets. + // tmux resolves that dir from TMUX_TMPDIR, falling back to /tmp — it + // does NOT consult TMPDIR, so neither do we. BOTH candidate bases, + // like the orphan sweep: tmux takes the first USABLE base, so a stale + // TMUX_TMPDIR pointing at an unusable path puts the socket under /tmp + // while a single-base unlink misses it. + try { + const uid = process.getuid?.(); + if (uid !== undefined) { + // Untrimmed, matching tmux (a padded value is used verbatim). + const envBase = process.env['TMUX_TMPDIR']; + for (const base of new Set([envBase || '/tmp', '/tmp'])) { + rmSync(join(base, `tmux-${uid}`, server), { force: true }); + } + } + } catch { + // Litter is cosmetic; never let cleanup mask the capture's own result. + } + }; + // (REAP_SIGNALS is module-level and exported so the signal tests iterate + // the REAL list — a dropped entry must fail a test, not ship silently.) + const releaseSignals = (): void => { + for (const s of REAP_SIGNALS) process.removeListener(s, onSignal); + }; + // The success tail after the reap is synchronous (freeze render up to its + // belt, two writes): a signal landing there is QUEUED in libuv's self-pipe, + // and removing the listeners before that pipe is read swallows it — the + // process then exits 0 as if nothing happened (measured on the bundled + // runtime: SIGTERM during a fake render → exit 0, handler never ran). + // TWO turns, and neither may be a 0ms timer. libuv delivers signals by + // writing to a pipe the POLL phase reads; the JS handler runs from there. + // A timer resolves in the timers phase, which precedes poll — it can + // release with the byte unread. setImmediate resolves in the check phase, + // which follows poll WITHIN THE SAME ITERATION: scheduled from a + // poll-phase continuation (where this tail resumes after the render's + // blocking spawnSync) its poll has ALREADY run, so one turn releases with + // the byte still queued and the signal is dropped when uv_signal_stop + // takes the watcher away. A second turn cannot land before the next + // iteration's poll, so a full poll phase — the one that dispatches the + // handler, which reaps and re-raises — always runs first. Measured on + // Linux/Node 22 through the real command: one turn swallowed the SIGTERM + // every time (exit 0, success JSON, handler never entered, kernel showing + // the signal delivered and no longer pending); two turns died 143. + const drainSignalsThenRelease = async (): Promise => { + for (let turn = 0; turn < 2; turn++) { + await new Promise((resolve) => setImmediate(resolve)); + } + releaseSignals(); + }; + function onSignal(sig: NodeJS.Signals): void { + reap(); + releaseSignals(); + process.kill(process.pid, sig); + } + for (const s of REAP_SIGNALS) process.on(s, onSignal); + + let ansText = ''; + let settledBy: CaptureManifest['settledBy'] = 'fixed-delay'; + let readyFailed = false; + let keysSent: boolean | undefined; + let matchOverruns = 0; + // How long the --until poll actually had — see where it is set. + let untilPolledMs = 0; + let captureFailed = false; + try { + // BEFORE the call: plan.start forks the server in the same client + // invocation that creates the session, so a start cut by the control + // belt throws with the server ALREADY UP — exactly the window an + // after-the-call flag left unreaped (measured orphan shape on loaded + // runners). kill-server against a server that never came up answers + // the goal state, so the early flag adds no false warnings. + serverStarted = true; + tmux(plan.start); + // Before ANY key can be sent, wait for the holder's ready sentinel: the + // pty's INTR fires the instant tmux writes 0x03 — a C-c racing the + // holder's own `trap : INT` line killed pane, session and server + // (measured; no in-script ordering can win, so the wait sits out here). + { + const holderDeadline = Date.now() + holderInit.timeoutMs; + while (!existsSync(holderReadyPath)) { + if (Date.now() >= holderDeadline) { + throw new Error( + 'the pane never initialized — its holder wrote no ready marker', + ); + } + await sleep(25); + } + } + // One deadline covers the ready gate AND the until poll: two separate + // clocks would let a capture run to 2× --timeout-ms. + const deadline = Date.now() + args.timeoutMs; + if (readyRe) { + readyFailed = true; + for (;;) { + const logical = tmux(plan.captureText); + const m = testWithBudget(readyRe, logical); + if (m === 'match') { + readyFailed = false; + break; + } + if (m === 'overrun') matchOverruns++; + if (Date.now() >= deadline) break; + await sleep(250); + } + } + if (args.keys !== undefined && args.keys.length > 0) { + if (readyFailed) { + // The UI never reached the state the keys were meant for: typing + // them anyway would drive an unknown screen. Withhold, and say so. + keysSent = false; + } else { + for (const key of args.keys) { + tmux(plan.sendKeys(key)); + } + keysSent = true; + } + } + if (readyFailed) { + // The deadline is spent; a late frame is all there is. This is a + // timeout settle even without --until — the run waited out + // --timeout-ms, and calling it 'fixed-delay' would misdescribe it. + settledBy = 'timeout'; + ansText = tmux(plan.capture); + } else if (untilRe) { + // What the marker search ACTUALLY got, for the degradation to report: + // the ready gate above shares this one deadline, so with --ready given + // the poll starts with only the remainder. + untilPolledMs = Math.max(0, deadline - Date.now()); + // Poll for the settle marker on the LOGICAL view (wraps joined, + // escapes absent): on the physical frame, a marker spanning a wrap + // boundary or an SGR attribute change can never match (measured: + // both miss forever). On timeout, capture anyway and SAY SO — a late + // frame is degraded evidence, not no evidence. The physical frame is + // captured in the same poll iteration as its matching logical view, + // so the `.ans` is the frame the match ruled on, give or take the + // milliseconds between two capture-pane calls. + settledBy = 'timeout'; + for (;;) { + const logical = tmux(plan.captureText); + const m = testWithBudget(untilRe, logical); + if (m === 'match') { + settledBy = 'until-match'; + ansText = tmux(plan.capture); + break; + } + if (m === 'overrun') matchOverruns++; + if (Date.now() >= deadline) { + ansText = tmux(plan.capture); + break; + } + await sleep(250); + } + } else { + await sleep(args.settleMs); + ansText = tmux(plan.capture); + } + } catch (e) { + // tmux failing mid-run (ancient tmux without a flag we use, a command + // tmux itself refuses, a server that died under us) is an environment + // that could not produce evidence — the refusal contract, not a stack + // trace. The finally below still reaps whatever did start. + const err = e as Error & { stderr?: string }; + const detail = + (err.stderr ?? '').trim().split('\n').slice(-1)[0] || + (err.message ?? String(e)).split('\n')[0]; + captureFailed = true; + refuse(`tmux failed mid-capture: ${detail}`); + return; + } finally { + // Always, even when the capture threw mid-run: the private server holds + // every process this capture launched, and an orphaned TUI outliving the + // review is the mess this command exists to make impossible. A start + // that threw has no server to reap, and reap() stays silent about it. + // The reap runs FIRST — releasing the listeners before it left a + // measured 25ms-to-death window with no listener. Honest limit, also + // measured: Node cannot dispatch a JS handler while reap()'s synchronous + // execFileSync blocks (up to 2×15s against a wedged server), so a signal + // landing in that window only dispatches once the block ends — a kill + // there reads normal completion, and the no-orphan guarantee is the + // reap's, not the handler's. The listeners cover the await windows, + // where dispatch works. On the success path they stay installed through + // the freeze render below, which can block up to its belt. + reap(); + // The sentinel is plumbing, not evidence — removed on EVERY exit path + // (a mid-capture refusal after the holder wrote it used to leave it + // stranded next to where evidence should be, measured). + try { + rmSync(holderReadyPath, { force: true }); + } catch { + // Litter is cosmetic. + } + // Same drain as the success tail: a signal queued while reap() blocked + // in execFileSync must dispatch to the handler (re-raise) before the + // listeners go away — bare release read as "it refused on its own" + // where the harness's kill actually landed. + if (captureFailed) await drainSignalsThenRelease(); + } + + try { + writeFileSync(ansPath, ansText, 'utf8'); + } catch (e) { + // The disk can fill (or the target turn hostile) during a long capture + // window; the same principle as the mkdir guard — refusal contract, not + // a stack trace. "THIS run's artifacts or nothing": a partial or 0-byte + // .ans from an interrupted write (measured with a real ENOSPC) must not + // persist undescribed. + try { + // Plain, never recursive: this run wrote a FILE (or nothing); a + // directory at the path is not ours to delete. And only if the file + // actually CHANGED: a write that failed at open() truncated nothing, + // and the previous file — which the clear phase may have deliberately + // spared as unrelated — must survive the refusal. A partial write + // does change it, and that truncated .ans left undescribed is the + // harm this catch exists for. + if (changed(ansPath, ansStamp)) rmSync(ansPath, { force: true }); + } catch { + // The refusal reason below is the primary signal either way. + } + await drainSignalsThenRelease(); + refuse( + `cannot write capture output: ${e instanceof Error ? e.message : String(e)}`, + ); + return; + } + + // .ans FIRST, then render: freeze has hung mid-render on this repo's own + // workflows, and the text evidence must already be on disk when it does. + let png: string | null = null; + // Collect every way this capture fell short of "settled png" — the field's + // contract is that a manifest reader learns WHY the ladder stopped where it + // did, and a late frame and a failed render can both be true at once. + const degradations: string[] = []; + // Probed ONCE and reused: each probe is a fresh 10s-belted spawn, and a + // wedged freeze paid the belt twice — worse, a stateful binary could + // answer differently between condition and message. + let freezeProbe: ReturnType; + if (readyFailed) { + degradations.push( + `--ready never matched within ${args.timeoutMs}ms — ${ + keysSent === false ? 'keys were NOT sent, ' : '' + }late frame captured`, + ); + } else if (settledBy === 'timeout') { + // The window ACTUALLY spent on the marker, not the whole budget: ONE + // deadline covers the ready gate and the until poll, so with --ready + // also given the gate consumes part of it and the poll runs for the + // remainder — reporting the full --timeout-ms overstated the search by + // up to the entire budget, and a reader deciding "the marker never + // appears" from that number is deciding from a window that never ran. + degradations.push( + `--until never matched within ${untilPolledMs}ms of the ` + + `${args.timeoutMs}ms budget — late frame captured`, + ); + } + if (capturePads) { + degradations.push( + `tmux ${tmuxVersion} pads capture-pane -N to the grid allocation and ` + + `has no -T — trailing spaces were TRIMMED rather than fabricated; ` + + `a trailing-space or right-edge claim needs tmux 3.3+`, + ); + } + if (matchOverruns > 0) { + // A budget cutoff is not the same as an absent marker: the match may + // have been interrupted mid-backtrack, and the field's contract is that + // a reader learns WHY the settle landed where it did. + degradations.push( + `marker matching exceeded its ${MATCH_BUDGET_MS}ms budget ${matchOverruns} time(s) — the marker may be present, its match cut off`, + ); + } + if (ansText.trim() === '') { + // A blank capture — zero bytes or nothing but whitespace — has no pixels + // worth rendering: freeze fails empty input with a misleading bounds + // error, and a blank image would be evidence-shaped noise anyway. + degradations.push( + 'pane captured empty — nothing to render, no image produced', + ); + } else if (pngStamp.existed) { + // Something this run did not write occupies the png path, and the clear + // phase spared it deliberately. Rendering would replace it — the same + // silent destruction the .ans/.json collision refuses over — so the + // ladder stops at the text rung and says why. + degradations.push( + `${pngPath} holds a file this capture did not write — no image ` + + 'rendered; clear it or pick another --out for a png rung', + ); + } else if ((freezeProbe = probes.freeze()).status !== 'ok') { + degradations.push( + freezeProbe.status === 'hung' + ? freezeProbe.code + ? // The SPAWN failed, which says nothing about installation: a + // false 'not installed' would persist in the manifest as an + // environment claim this run never established. + `freeze could not be probed (${freezeProbe.code}) — ${ + freezeProbe.spawned + ? 'it ran and failed, so it is installed but not usable here' + : 'it may be installed; this host could not spawn it' + }. .ans text captured, no image rendered` + : `freeze did not answer --help within ${probeBudget.timeoutMs}ms — present but wedged; .ans text captured, no image rendered` + : 'freeze is not installed — .ans text captured, no image rendered', + ); + } else { + // stdin MUST be /dev/null: freeze treats a pipe stdin — Node's spawnSync + // default — as "the input is stdin" and ignores the positional file. A + // pipe that EOFs promptly produces `ERROR No input` (exit 1); a pipe that + // stays open hangs freeze indefinitely. Both modes were measured on this + // machine in one evening — the historical "freeze hangs" incidents on + // this repo's workflows are this exact shape. The timeout stays as the + // second belt. + const r = spawnSync(freezeRender.bin, freezePlan(ansPath, pngPath), { + encoding: 'utf8', + timeout: freezeRender.timeoutMs, + killSignal: 'SIGKILL', + stdio: ['ignore', 'pipe', 'pipe'], + }); + if ( + r.status === 0 && + existsSync(pngPath) && + statSync(pngPath).size > 0 && + // ...and THIS run is what put those bytes there. Existence alone + // credited a file the clear phase deliberately spared — an unrelated + // .png with no capture manifest beside it — as this run's + // rendering evidence whenever freeze exited 0 without writing + // (measured end to end: success JSON with evidence 'png', pngPath + // pointing at the user's untouched file, no degradation recorded). + changed(pngPath, pngStamp) + ) { + // Exit code alone is not evidence: a freeze that exits 0 without + // writing the file — or leaving a 0-byte/truncated one (ENOSPC + // mid-write, the shape the .ans write guard's comment names) — would + // otherwise manifest a png rung with no pixels, and a verifier would + // publish it. + png = pngPath; + } else { + // The stderr tail rides along: a bare exit code is undiagnosable from + // a manifest, and the whole point of recording degradation is that a + // reader can tell WHY the ladder stopped. A spawn that never ran + // (EMFILE, a binary vanishing between probe and render) has neither + // status nor signal — its reason lives in r.error. + const errTail = `${r.stderr ?? ''} ${r.stdout ?? ''}` + .trim() + .split('\n') + .slice(-2) + .join(' '); + const why = + r.status === 0 + ? 'exited 0 but wrote no image' + : r.signal + ? // A belt kill carries BOTH signal and error (ETIMEDOUT); name + // the belt, or it reads as an unexplained external kill. + `signal ${r.signal}${ + r.error + ? ` after the ${freezeRender.timeoutMs}ms render belt` + : '' + }` + : r.status !== null + ? `exit ${String(r.status)}` + : `spawn failed: ${r.error ? r.error.message : 'unknown error'}`; + degradations.push( + `freeze failed (${why}${errTail ? `: ${errTail}` : ''}) — .ans text captured, no image rendered`, + ); + // A failed render can leave a partial/0-byte png at the very path the + // manifest is about to deny — remove it (measured: a fake freeze that + // wrote bytes then exited 9 left a torn png behind). Only when the + // clear phase left nothing there: freeze can fail without its spawn + // ever opening the output (EMFILE, a belt kill), and this SUCCEEDING + // run then silently deleted a user's untouched png and reported + // ans-only (measured). + try { + if (changed(pngPath, pngStamp)) rmSync(pngPath, { force: true }); + } catch { + // The degradation entry above is the primary signal. + } + } + } + + const degradedBecause = degradations.length + ? degradations.join('; ') + : undefined; + const manifest: CaptureManifest = { + command: args.command, + cwd: resolvedCwd, + cols: args.cols, + rows: args.rows, + ...(args.keys !== undefined ? { keys: args.keys } : {}), + ...(keysSent !== undefined ? { keysSent } : {}), + ...(args.ready !== undefined ? { ready: args.ready } : {}), + ...(args.until !== undefined ? { until: args.until } : {}), + // The ACTIVE durations: settleMs governed the run only when a fixed + // delay actually happened; timeoutMs governed it when either marker + // (--until OR --ready) was in play — a --ready-only run spends the + // timeout budget too, and recording settleMs alone would misdescribe it. + ...(args.until === undefined && !readyFailed + ? { settleMs: args.settleMs } + : {}), + ...(args.until !== undefined || args.ready !== undefined + ? { timeoutMs: args.timeoutMs } + : {}), + ansPath, + pngPath: png, + evidence: png ? 'png' : 'ans-only', + ...(degradedBecause ? { degradedBecause } : {}), + settledBy, + }; + try { + writeFileSync( + manifestPath, + `${JSON.stringify(manifest, null, 2)}\n`, + 'utf8', + ); + } catch (e) { + // "THIS run's artifacts or nothing": a refusal must not leave an .ans + // (and possibly a png) with no manifest to describe them — best-effort + // remove what this run already wrote before refusing. + try { + // Plain, never recursive — same rationale as the .ans catch above — + // and each path only if this run put it there. Reaching here, the + // Each path only if THIS run changed it: the .ans write succeeded so + // it is ours, the png is ours only when the render actually produced + // one, and a manifest write that failed at open() left the previous + // file whole. A PARTIAL manifest is worse than none — it parses or + // half-parses as an evidence description — and it, having changed, + // is removed. + // PER PATH, like the clear phase's own clearArtifact: one try around + // all three let a throw on the .ans or .png removal skip the manifest + // one — leaving exactly the partial manifest this block calls worse + // than none (probe-reproduced with a directory at the .png path). + for (const [path, stamp] of [ + [ansPath, ansStamp], + [pngPath, pngStamp], + [manifestPath, manifestStamp], + ] as const) { + try { + if (changed(path, stamp)) rmSync(path, { force: true }); + } catch { + // The refusal reason below is the primary signal either way. + } + } + } catch { + // Unreachable in practice; the per-path catches above own the risk. + } + await drainSignalsThenRelease(); + refuse( + `cannot write capture manifest: ${e instanceof Error ? e.message : String(e)}`, + ); + return; + } + await drainSignalsThenRelease(); + + // The evidence is on disk and described. Whatever happens to stdio now, + // this run SUCCEEDED. + artifactsComplete = true; + writeStderrLineSafe( + `capture-tui: ${manifest.evidence} at ${args.cols}x${args.rows} ` + + `(settled by ${settledBy})${degradedBecause ? ` — ${degradedBecause}` : ''}`, + ); + writeStdoutLineSafe( + JSON.stringify({ + captured: true, + evidence: manifest.evidence, + manifest: manifestPath, + }), + ); +} + +export const captureTuiCommand: CommandModule = { + command: 'capture-tui', + describe: + 'Run a command in a throwaway PRIVATE tmux server and capture what it rendered — .ans always, .png when freeze is available — as evidence for rendering claims', + builder: (yargs) => + yargs + .option('command', { + type: 'string', + demandOption: true, + describe: 'The command to run inside the capture terminal', + }) + .option('cwd', { + type: 'string', + describe: 'Working directory for the command (default: current)', + }) + .option('cols', { + type: 'number', + default: DEFAULT_COLS, + describe: 'Terminal width — layout claims are claims about a width', + }) + .option('rows', { + type: 'number', + default: DEFAULT_ROWS, + describe: 'Terminal height', + }) + .option('settle-ms', { + type: 'number', + default: 3000, + describe: 'Fixed delay before capturing (ignored when --until is set)', + }) + .option('until', { + type: 'string', + describe: + 'Capture as soon as the pane text matches this regex; on timeout, capture anyway and record that the marker never appeared', + }) + .option('ready', { + type: 'string', + describe: + 'Send --keys only after the pane matches this regex — keys fired at start straddle a slow-mounting UI and get partially eaten (measured); on timeout the keys are withheld and the manifest says so', + }) + .option('keys', { + type: 'string', + array: true, + describe: + 'tmux send-keys tokens sent after start (or after --ready matches), one per token (e.g. --keys "/review" Enter); a token starting with "-" must use the --keys= form or yargs parses it as an unknown flag', + }) + .option('out', { + type: 'string', + demandOption: true, + describe: + 'Output basename: .ans, .png (when rendered) and .json (the manifest) are written', + }) + .option('timeout-ms', { + type: 'number', + default: 60_000, + describe: + 'One shared deadline for the --ready gate and --until polling — a ready+keys capture without --until is still bounded by this', + }), + handler: (argv) => + runCaptureTui({ + command: argv['command'] as string, + cwd: argv['cwd'] as string | undefined, + cols: argv['cols'] as number, + rows: argv['rows'] as number, + settleMs: argv['settle-ms'] as number, + until: argv['until'] as string | undefined, + ready: argv['ready'] as string | undefined, + keys: argv['keys'] as string[] | undefined, + out: argv['out'] as string, + timeoutMs: argv['timeout-ms'] as number, + }), +}; diff --git a/packages/cli/src/commands/review/cleanup.test.ts b/packages/cli/src/commands/review/cleanup.test.ts index ce819ef0383..12689fe0595 100644 --- a/packages/cli/src/commands/review/cleanup.test.ts +++ b/packages/cli/src/commands/review/cleanup.test.ts @@ -1,12 +1,13 @@ // Copyright 2026 Qwen Team // SPDX-License-Identifier: Apache-2.0 -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { spawnSync } from 'node:child_process'; const mocks = vi.hoisted(() => ({ execFileSync: vi.fn(), - existsSync: vi.fn(() => false), - readdirSync: vi.fn(() => []), + existsSync: vi.fn((_path: string) => false), + readdirSync: vi.fn((_path: string): string[] => []), readFileSync: vi.fn((_path: string): string => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); }), @@ -94,6 +95,7 @@ import { type RawIssueComment, type RawReview, } from './cleanup.js'; +import { captureServerName } from './lib/tui-capture.js'; describe('runCleanup', () => { beforeEach(() => { @@ -153,6 +155,479 @@ describe('runCleanup', () => { ]); }); + describe.skipIf(process.platform === 'win32')( + 'orphaned capture-tui servers', + () => { + // win32: the implementation early-returns when process.getuid is + // undefined, so both halves under test are unreachable there and the + // fixtures (POSIX socket-dir layout) would fail for the wrong reason. + // A SIGKILL'd harness leaves the private tmux server alive; cleanup is + // the sweep that reclaims it, keyed on the launcher pid in the socket + // name. The pid liveness probe and the tmux kill are the two halves. + const uid = process.getuid?.(); + const dir = `/fake-tmp/tmux-${String(uid)}`; + // A pid that WAS alive and is not: spawn a process and let it exit. + const deadPid = String(spawnSync(process.execPath, ['-e', '']).pid ?? 0); + const deadPid2 = String(spawnSync(process.execPath, ['-e', '']).pid ?? 0); + // Built with the PRODUCER, not hand-spelled: the sweep's matcher + // (`^${CAPTURE_SERVER_PREFIX}(\\d+)-`) only works while the pid sits + // immediately after the prefix, and a captureServerName edit that + // inserted a segment before it would leave every hand-written fixture + // matching while the real sweep stopped recognising real sockets. + const orphan = captureServerName(Number(deadPid), 'aaaa'); + // Listed AFTER the wedged orphan: an unreapable entry must not stop the + // sweep (a continue→break mutant leaves this one alive for the + // holder's full bounded window — up to three hours — with no stderr trail). + const orphan2 = captureServerName(Number(deadPid2), 'cccc'); + const live = captureServerName(process.pid, 'bbbb'); + + beforeEach(() => { + process.env['TMUX_TMPDIR'] = '/fake-tmp'; + mocks.existsSync.mockImplementation((p: string) => p === dir); + mocks.readdirSync.mockImplementation((p: string) => + // The foreign socket comes FIRST: a continue→break mutant stops the + // sweep at the first non-matching name (typically the user's own + // socket), leaving every orphan after it alive. + // Live socket BEFORE the orphans: an `if (alive) continue` → + // `break` mutant would stop at the first live socket and leave + // every orphan after it holding its bounded pane hold. + p === dir ? ['some-other-socket', live, orphan, orphan2] : [], + ); + mocks.execFileSync.mockReturnValue(Buffer.from('')); + }); + + afterEach(() => { + delete process.env['TMUX_TMPDIR']; + }); + + it('reaps sockets whose launcher pid is dead and leaves live ones alone', () => { + runCleanup('local'); + + expect(mocks.execFileSync).toHaveBeenCalledWith( + 'tmux', + ['-L', orphan, 'kill-server'], + expect.objectContaining({ + stdio: 'pipe', + timeout: 15_000, + killSignal: 'SIGKILL', + // The kill runs in the base the socket was found under; the + // dedicated env tests pin the value. + env: expect.objectContaining({ TMUX_TMPDIR: expect.any(String) }), + }), + ); + expect(mocks.execFileSync).not.toHaveBeenCalledWith( + 'tmux', + ['-L', live, 'kill-server'], + expect.objectContaining({ + stdio: 'pipe', + timeout: 15_000, + killSignal: 'SIGKILL', + // The kill runs in the base the socket was found under; the + // dedicated env tests pin the value. + env: expect.objectContaining({ TMUX_TMPDIR: expect.any(String) }), + }), + ); + expect(mocks.rmSync).toHaveBeenCalledWith(`${dir}/${orphan}`, { + force: true, + }); + // And the LIVE socket is never announced as reaped: every stdout + // negative in this suite named the orphan, so a mutant printing the + // success line for a skipped live server escaped all of them. + expect(mocks.writeStdoutLine).not.toHaveBeenCalledWith( + `Reaped orphaned capture server: ${live}`, + ); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + // BOTH orphans: a break-after-first-reap mutant left every later + // orphan alive on multi-review hosts and shipped green. + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan2}`, + ); + // The foreign socket stands in for the USER's own tmux server: the + // regex gate keeps the sweep off it entirely — a deleted `continue` + // on non-match kill-server'd the user's default server in probe (the + // blast radius private -L isolation exists to prevent). + expect(mocks.execFileSync).not.toHaveBeenCalledWith( + 'tmux', + ['-L', 'some-other-socket', 'kill-server'], + expect.anything(), + ); + expect(mocks.rmSync).not.toHaveBeenCalledWith( + `${dir}/some-other-socket`, + expect.anything(), + ); + // Something WAS cleaned, so the nothing-to-clean claim must not print. + expect(mocks.writeStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('Nothing to clean'), + ); + }); + + it('notes a server it cannot kill and does not unlink a live server socket', () => { + // Throw for the FIRST orphan only (both retry attempts), so the sweep + // must note it and CONTINUE to the second one. + mocks.execFileSync.mockImplementation((bin: string, argv: string[]) => { + if (bin === 'tmux' && argv?.[1] === orphan) { + throw Object.assign(new Error('wedged'), { + stderr: 'tmux: server is wedged', + }); + } + return Buffer.from(''); + }); + + runCleanup('local'); + + expect(mocks.writeStderrLine).toHaveBeenCalledWith( + expect.stringContaining( + `could not reap orphaned capture server ${orphan}`, + ), + ); + // And the hand-reap command it suggests carries the base override + // the sweep itself needed: without it `-L` resolves elsewhere and + // answers 'no server running', reading as "already gone". + expect(mocks.writeStderrLine).toHaveBeenCalledWith( + expect.stringContaining( + `TMUX_TMPDIR="${dir.replace(/\/tmux-\d+$/, '')}"`, + ), + ); + expect(mocks.rmSync).not.toHaveBeenCalledWith( + `${dir}/${orphan}`, + expect.anything(), + ); + // ...and stdout must not claim it WAS reaped. Hoisting the success + // line above the failure branch escaped every other assertion here. + expect(mocks.writeStdoutLine).not.toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + // The sweep REACHED the orphan listed after the wedged one — a + // continue→break mutant left it alive for the holder's bounded window, unnoted. + expect(mocks.execFileSync).toHaveBeenCalledWith( + 'tmux', + ['-L', orphan2, 'kill-server'], + expect.objectContaining({ + stdio: 'pipe', + timeout: 15_000, + killSignal: 'SIGKILL', + // The kill runs in the base the socket was found under; the + // dedicated env tests pin the value. + env: expect.objectContaining({ TMUX_TMPDIR: expect.any(String) }), + }), + ); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan2}`, + ); + // The title's second clause, pinned directly: the LIVE server's + // socket is never unlinked (unlinking it would make the live server + // unreachable forever). + expect(mocks.rmSync).not.toHaveBeenCalledWith( + `${dir}/${live}`, + expect.anything(), + ); + // An unreapable orphan is a FAILURE, not a nothing: stdout must not + // contradict the stderr note with a "Nothing to clean" claim. + expect(mocks.writeStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('Nothing to clean'), + ); + }); + + it('treats "no server running" as reaped — socket unlinked, success printed', () => { + // The kill throwing because the server is ALREADY dead is the goal + // state, not a failure: the socket is litter and must still go. A + // `serverDead = false` mutant ships this branch green otherwise. + mocks.execFileSync.mockImplementation((bin: string) => { + if (bin === 'tmux') { + throw Object.assign(new Error('exited 1'), { + stderr: Buffer.from(`no server running on ${dir}/${orphan}`), + }); + } + return Buffer.from(''); + }); + + runCleanup('local'); + + expect(mocks.rmSync).toHaveBeenCalledWith(`${dir}/${orphan}`, { + force: true, + }); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + expect(mocks.writeStderrLine).not.toHaveBeenCalledWith( + expect.stringContaining('could not reap'), + ); + }); + + it('accumulates orphans across BOTH bases, not just the last one', () => { + // No fixture returned entries from both socket bases at once, so + // the cross-base `entries.concat(...)` was unpinned and an + // overwrite mutant shipped green — losing every orphan under the + // env base whenever /tmp also had one (and vice versa). + const tmpDir = `/tmp/tmux-${String(uid)}`; + mocks.existsSync.mockImplementation( + (p: string) => p === dir || p === tmpDir, + ); + mocks.readdirSync.mockImplementation((p: string) => { + if (p === dir) return [orphan]; + if (p === tmpDir) return [orphan2]; + return []; + }); + runCleanup('local'); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan2}`, + ); + }); + + it('scans the OTHER base after one of them cannot be read', () => { + // The unreadable-dir fixture makes both bases throw, so a + // catch-then-break mutant shipped green: an env-base tmux- + // that exists but is mode-000 would then hide every orphan under + // /tmp behind one stderr note. + const tmpDir = `/tmp/tmux-${String(uid)}`; + mocks.existsSync.mockImplementation( + (p: string) => p === dir || p === tmpDir, + ); + mocks.readdirSync.mockImplementation((p: string) => { + if (p === dir) { + throw Object.assign(new Error('EACCES: permission denied'), { + code: 'EACCES', + }); + } + if (p === tmpDir) return [orphan]; + return []; + }); + runCleanup('local'); + expect(mocks.writeStderrLine).toHaveBeenCalledWith( + expect.stringContaining('could not scan'), + ); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + }); + + it('sweeps under a pr- target too — the sweep is host-wide', () => { + // Every other fixture drives 'local'. The sweep is deliberately not + // target-scoped (an orphan belongs to the host, not to one review), + // so an edit that moves it into a local-only path would leave nine + // orphan tests green while PR runs — where captures actually + // happen — stopped reaping. + runCleanup('pr-8388'); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + }); + + it('falls back to /tmp when TMUX_TMPDIR is unset — the common host', () => { + // All other fixtures set TMUX_TMPDIR; the fallback branch governs + // standard CI lanes and dev machines, and a wrong-literal mutant + // scanned the wrong directory and returned clean forever. + delete process.env['TMUX_TMPDIR']; + const tmpDir = `/tmp/tmux-${String(uid)}`; + mocks.existsSync.mockImplementation((p: string) => p === tmpDir); + mocks.readdirSync.mockImplementation((p: string) => + p === tmpDir ? [orphan] : [], + ); + runCleanup('local'); + expect(mocks.readdirSync).toHaveBeenCalledWith(tmpDir); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + }); + + it('scans /tmp even when TMUX_TMPDIR points elsewhere — tmux fell back', () => { + // tmux takes the first USABLE base: a stale profile-exported + // TMUX_TMPDIR pointing at an unusable path puts the socket under + // /tmp while a single-base sweep `[envBase || '/tmp']` scans only + // the env base and reports clean with the orphan still live + // (measured end-to-end: 'Nothing to clean' beside a live orphan). + const tmpDir = `/tmp/tmux-${String(uid)}`; + mocks.existsSync.mockImplementation((p: string) => p === tmpDir); + mocks.readdirSync.mockImplementation((p: string) => + p === tmpDir ? [orphan] : [], + ); + runCleanup('local'); + expect(mocks.readdirSync).toHaveBeenCalledWith(tmpDir); + // And the KILL goes to the base the socket was FOUND under, not to + // this process's env: `-L` re-resolves the socket dir from the + // environment and tmux does NOT fall back when the env base exists + // (it creates it) — measured on 3.3a, the kill answered + // `error connecting to /tmux-/` and the orphan + // survived, while the same call under the found base reaped it. + // The mocked execFileSync cannot show that; the env it is called + // with can. + expect(mocks.execFileSync).toHaveBeenCalledWith( + 'tmux', + ['-L', orphan, 'kill-server'], + expect.objectContaining({ + env: expect.objectContaining({ + TMUX_TMPDIR: '/tmp', + // The parent environment rides along: `env: { TMUX_TMPDIR }` + // alone leaves tmux without a PATH, and every kill then fails + // for a reason that has nothing to do with the socket. + PATH: process.env['PATH'], + }), + }), + ); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + }); + + it('reads TMUX_TMPDIR UNTRIMMED — tmux uses a padded value verbatim', () => { + // Measured against real tmux 3.4: with a trailing space in + // TMUX_TMPDIR the socket landed under the PADDED path, while a + // trimming sweep scanned a directory tmux never used and reported + // clean — re-adding .trim() must turn this red. + process.env['TMUX_TMPDIR'] = '/fake-tmp '; + const paddedDir = `/fake-tmp /tmux-${String(uid)}`; + mocks.existsSync.mockImplementation((p: string) => p === paddedDir); + mocks.readdirSync.mockImplementation((p: string) => + p === paddedDir ? [orphan] : [], + ); + runCleanup('local'); + expect(mocks.readdirSync).toHaveBeenCalledWith(paddedDir); + // The kill carries the padded base too — trimming EITHER side + // sends tmux to a directory it never used. + expect(mocks.execFileSync).toHaveBeenCalledWith( + 'tmux', + ['-L', orphan, 'kill-server'], + expect.objectContaining({ + env: expect.objectContaining({ TMUX_TMPDIR: '/fake-tmp ' }), + }), + ); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + }); + + it('surfaces an unreadable socket dir — a scan failure is not a silent nothing', () => { + // A mode-000 tmux- or a filesystem hiccup makes readdirSync + // throw; the sweep must note the unreadable dir on stderr, must + // not claim 'Nothing to clean' while an orphan may be hiding, and + // must still clear the target-scoped lease. The swallowing mutant + // `catch {}` hid orphans for the holder's whole bounded window and + // shipped green. + mocks.existsSync.mockImplementation((p: string) => + p.endsWith(`/tmux-${String(uid)}`), + ); + mocks.readdirSync.mockImplementation((p: string) => { + if (p.endsWith(`/tmux-${String(uid)}`)) { + throw Object.assign(new Error('EACCES: permission denied'), { + code: 'EACCES', + }); + } + return []; + }); + runCleanup('local'); + expect(mocks.writeStderrLine).toHaveBeenCalledWith( + expect.stringContaining('could not scan'), + ); + expect(mocks.writeStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('Nothing to clean'), + ); + expect(mocks.clearReviewWorktreeLease).toHaveBeenCalledWith( + process.cwd(), + 'local', + ); + }); + + it('reaps on the SECOND kill attempt — the sweep retry is real', () => { + let calls = 0; + mocks.execFileSync.mockImplementation((bin: string) => { + if (bin === 'tmux') { + calls++; + if (calls === 1) { + throw Object.assign(new Error('transient'), { + stderr: 'transient client failure', + }); + } + } + return Buffer.from(''); + }); + runCleanup('local'); + expect(mocks.writeStdoutLine).toHaveBeenCalledWith( + `Reaped orphaned capture server: ${orphan}`, + ); + expect(mocks.writeStderrLine).not.toHaveBeenCalledWith( + expect.stringContaining('could not reap'), + ); + // The cap is ONE retry, pinned from ABOVE as well: every earlier + // assertion here holds for any cap >= 2, so a "robustness" edit + // could widen it silently — and against a genuinely wedged server + // each attempt pays the full 15s belt before the cleanup moves on. + const killCalls = mocks.execFileSync.mock.calls.filter( + (c: unknown[]) => + c[0] === 'tmux' && + Array.isArray(c[1]) && + (c[1] as string[]).includes('kill-server') && + (c[1] as string[]).includes(orphan), + ); + expect(killCalls).toHaveLength(2); + }); + + it('gives up after ONE retry — the cap, pinned from above', () => { + // The fixture above stops throwing after the first call, so the + // loop exits via serverDead on attempt 2 for ANY cap >= 2. Here + // every attempt throws, so the count IS the cap: a widened cap pays + // the full 15s belt per attempt against a genuinely wedged server + // while the cleanup waits. + mocks.execFileSync.mockImplementation((bin: string) => { + if (bin === 'tmux') { + throw Object.assign(new Error('wedged'), { + stderr: 'tmux: server is wedged', + }); + } + return Buffer.from(''); + }); + runCleanup('local'); + const killCalls = mocks.execFileSync.mock.calls.filter( + (c: unknown[]) => + c[0] === 'tmux' && + Array.isArray(c[1]) && + (c[1] as string[]).includes('kill-server') && + (c[1] as string[]).includes(orphan), + ); + expect(killCalls).toHaveLength(2); + }); + + it('reports an ONLY-unreapable-orphan sweep without "Nothing to clean" — and without holding the lease', () => { + // With a second reapable orphan in the fixture, removedAny masks + // the sweep.failed propagation — deleting it shipped green. Here + // the sole capture socket is unreapable: stdout must not claim + // nothing needed cleaning while stderr says the reap failed. + mocks.readdirSync.mockImplementation((p: string) => + p === dir ? [orphan] : [], + ); + mocks.execFileSync.mockImplementation((bin: string) => { + if (bin === 'tmux') { + throw Object.assign(new Error('wedged'), { + stderr: 'tmux: server is wedged', + }); + } + return Buffer.from(''); + }); + + runCleanup('local'); + + expect(mocks.writeStderrLine).toHaveBeenCalledWith( + expect.stringContaining('could not reap'), + ); + expect(mocks.writeStdoutLine).not.toHaveBeenCalledWith( + expect.stringContaining('Nothing to clean'), + ); + // The sweep is host-wide, the lease is target-scoped: an + // unreapable orphan from ANY capture must not wedge THIS target's + // worktree lease (measured complaint: an unrelated review's orphan + // blocked the lease release with nothing connecting the two). + expect(mocks.clearReviewWorktreeLease).toHaveBeenCalledWith( + process.cwd(), + 'local', + ); + }); + }, + ); + it('sweeps a stale base-tree build lock left by a killed builder', () => { // The lock is a plain directory (`mkdirSync` test-and-set), not a worktree, // so `releaseWorktree` never touches it; a builder killed mid-build leaves it @@ -311,6 +786,7 @@ describe('runCleanup — bypass-write audit', () => { mocks.readFileSync.mockImplementation(() => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); }); + mocks.currentUser.mockReturnValue('reviewer'); mocks.ghApiAll.mockReturnValue([]); }); diff --git a/packages/cli/src/commands/review/cleanup.ts b/packages/cli/src/commands/review/cleanup.ts index eecce237a9c..532fdcecabe 100644 --- a/packages/cli/src/commands/review/cleanup.ts +++ b/packages/cli/src/commands/review/cleanup.ts @@ -14,9 +14,16 @@ import type { CommandModule } from 'yargs'; import { execFileSync } from 'node:child_process'; -import { existsSync, readFileSync, readdirSync, rmSync } from 'node:fs'; -import { join } from 'node:path'; +import { + existsSync, + readFileSync, + readdirSync, + realpathSync, + rmSync, +} from 'node:fs'; +import { dirname, join } from 'node:path'; import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js'; +import { CAPTURE_SERVER_PREFIX, isNothingToKill } from './lib/tui-capture.js'; import { clearReviewWorktreeLease } from '../../services/review-worktree-lease.js'; import { currentUser, getGhHost, ghApiAll, setGhHost } from './lib/gh.js'; import { parseReceiptIds } from './lib/receipt.js'; @@ -371,6 +378,152 @@ function auditPrWrites(target: string, prNumber: string): void { } } +/** + * Reap the capture servers capture-tui's own reap could not reach: a + * SIGKILL'd or OOM'd harness skips finally and the signal net alike, and + * the private server then lives until its pane holder's bounded hold loop + * expires (up to three hours) — the config-free server has nothing else to + * destroy it, so this sweep (or a hand kill-server) is the only reaper in + * that window. The launcher's pid rides in the socket name for exactly + * this — a socket whose pid is dead is an orphan. A reap that fails is + * noted on stderr and suppresses the "Nothing to clean" claim — but does + * NOT hold the target-scoped worktree lease: the sweep is host-wide, and + * an unrelated review's orphan would otherwise wedge this review's lease + * with nothing connecting the two in the output. + */ +function reapOrphanedCaptureServers(): { reaped: boolean; failed: boolean } { + const uid = process.getuid?.(); + // tmux is POSIX-only, and so is the socket dir layout below. + if (uid === undefined) return { reaped: false, failed: false }; + // BOTH candidate socket dirs, not one: tmux's own resolution takes the + // first USABLE base (TMUX_TMPDIR, else /tmp) — a stale profile-exported + // TMUX_TMPDIR pointing at an unusable path means the real sockets live + // under /tmp while a single-base sweep scans the wrong directory forever + // (measured end-to-end: 'Nothing to clean' with a live orphan). + // UNTRIMMED, matching tmux: a whitespace-padded TMUX_TMPDIR is used + // verbatim by tmux (measured: socket under '/tmp/x /tmux-'), so a + // trimming sweep scanned a directory tmux never used. + const envBase = process.env['TMUX_TMPDIR']; + // De-duplicated by the directory the scan actually opens, not the raw + // string: an alias of /tmp (`/tmp/`, `/tmp/.`, `//tmp` — the same + // profile-exported family this fallback exists for) survived a + // string-keyed Set, so both entries joined to the same tmux- dir and + // every socket in it was listed, killed and reported TWICE. + const bases: string[] = []; + const seen = new Set(); + for (const base of [envBase || '/tmp', '/tmp']) { + const dir = join(base, `tmux-${uid}`); + // Keyed on the RESOLVED directory: string normalization collapses + // `/tmp/`, `/tmp/.` and `//tmp`, but a TMUX_TMPDIR that is a symlink to + // /tmp still named a different string while opening the same directory, + // so every socket in it was listed, killed and reported twice. + let key = dir; + try { + key = realpathSync(dir); + } catch { + // Not there (or unreadable): the raw path is a fine key, and the + // scan below reports what it cannot read. + } + if (seen.has(key)) continue; + seen.add(key); + bases.push(base); + } + let reapedAny = false; + let failedAny = false; + let entries: Array<{ dir: string; name: string }> = []; + for (const base of bases) { + const dir = join(base, `tmux-${uid}`); + try { + if (existsSync(dir)) { + entries = entries.concat( + readdirSync(dir).map((name) => ({ dir, name })), + ); + } + } catch (e) { + // A directory we cannot READ can be hiding an orphan — that is a + // failure to surface, not a silent nothing (the doc contract above: + // noted on stderr AND surfaced as failed). + failedAny = true; + writeStderrLine( + `note: could not scan ${dir} for orphaned capture servers: ${ + e instanceof Error ? e.message : String(e) + }`, + ); + } + } + const orphanRe = new RegExp(`^${CAPTURE_SERVER_PREFIX}(\\d+)-`); + for (const { dir, name } of entries) { + const m = orphanRe.exec(name); + if (!m) continue; + let alive = true; + try { + process.kill(Number(m[1]), 0); + } catch (e) { + // ESRCH = the pid is dead (the orphan signal). EPERM means the pid + // is alive under another user — not ours to reap. + alive = (e as NodeJS.ErrnoException).code === 'EPERM'; + } + if (alive) continue; + // Same rules as capture-tui's own reap: unlink the socket ONLY when + // the server is known dead — a kill that throws can leave it alive, + // and an unlinked socket makes a live server unreachable forever — and + // one retry before giving up: a transient client-spawn failure (EMFILE + // after a long review's many spawns) is the named shape, and the + // identical second attempt reaps what otherwise lives out the holder's + // bounded three-hour window. + let serverDead = false; + for (let attempt = 0; attempt < 2 && !serverDead; attempt++) { + try { + execFileSync('tmux', ['-L', name, 'kill-server'], { + stdio: 'pipe', + // The scan finds sockets under BOTH bases, but `-L` re-resolves + // the socket directory from THIS process's environment — and tmux + // does not fall back when the env base exists (it creates it). + // The two sides then disagree: an orphan found under /tmp while a + // stale profile-exported TMUX_TMPDIR points elsewhere answered + // `error connecting to /tmux-/` and survived + // (measured on 3.3a, with the same call succeeding under the base + // it was found in). Kill it where it was FOUND — `dir` is + // `/tmux-`, so its parent is the base tmux wants. + env: { ...process.env, TMUX_TMPDIR: dirname(dir) }, + // Same belt as capture-tui's own control calls: a wedged server + // must not hang the whole cleanup behind one socket — SIGKILL, + // because a TERM-immune child blocks the sync call past any belt. + timeout: 15_000, + killSignal: 'SIGKILL', + }); + serverDead = true; + } catch (e) { + serverDead = isNothingToKill( + String((e as { stderr?: unknown }).stderr ?? ''), + ); + } + } + if (!serverDead) { + failedAny = true; + writeStderrLine( + `note: could not reap orphaned capture server ${name} ` + + // WITH the base override: `-L` re-resolves the socket directory + // from the invoking environment and does not fall back, so on + // the very hosts where this note appears the bare command + // resolves elsewhere and answers 'no server running' — reading + // as "already gone" while the orphan runs out its window. + `(TMUX_TMPDIR=${JSON.stringify(dirname(dir))} tmux -L ${name} ` + + `kill-server to reap it by hand)`, + ); + continue; + } + try { + rmSync(join(dir, name), { force: true }); + } catch { + // Litter is cosmetic; the server itself is already gone. + } + writeStdoutLine(`Reaped orphaned capture server: ${name}`); + reapedAny = true; + } + return { reaped: reapedAny, failed: failedAny }; +} + export function runCleanup(target: string): void { let removedAny = false; // Tracked separately from `removedAny`, because a failure is neither. Without @@ -479,6 +632,24 @@ export function runCleanup(target: string): void { } } + // --- Orphaned capture servers (capture-tui) --------------------------- + // Not target-scoped: any crashed capture on this host left them, and + // Step 9's sweep is the only deterministic pass that reliably runs. Its + // failure therefore must NOT gate the target-scoped lease below — an + // orphan from an UNRELATED review, a wedged server outlasting the belt, + // or a host where tmux vanished after the socket dir was created would + // otherwise wedge THIS review's worktree lease forever, with nothing in + // the output connecting the two. It still suppresses "Nothing to clean": + // stderr saying "could not reap" next to stdout's "nothing to clean" is + // the two streams contradicting each other, and stdout is the one a + // script reads. + let sweepFailed = false; + { + const sweep = reapOrphanedCaptureServers(); + if (sweep.reaped) removedAny = true; + sweepFailed = sweep.failed; + } + if (!failedAny) { clearReviewWorktreeLease(process.cwd(), target); } @@ -486,7 +657,7 @@ export function runCleanup(target: string): void { // "Nothing to clean" is a claim about the tree, not about this run's luck. It // is only true when there was nothing there — not when there was and we could // not get rid of it. - if (!removedAny && !failedAny) { + if (!removedAny && !failedAny && !sweepFailed) { writeStdoutLine(`Nothing to clean for target "${target}".`); } } diff --git a/packages/cli/src/commands/review/lib/agent-briefs.ts b/packages/cli/src/commands/review/lib/agent-briefs.ts index dd190a0a2d5..78e864a2053 100644 --- a/packages/cli/src/commands/review/lib/agent-briefs.ts +++ b/packages/cli/src/commands/review/lib/agent-briefs.ts @@ -646,6 +646,25 @@ until [ -s /mock.json ]; do sleep 0.1; done # its port is in that rep For anything that is not one of those two wires — the project's own HTTP service, an MCP server, an OAuth endpoint — stand it up yourself and let \`drive\` own the lifecycle. +**When the claim is about what the terminal RENDERS, capture it — do not describe it.** A layout claim — "the panel clips at 80 columns", "the status line overlaps the prompt", "the colors are unreadable on dark themes" — is a claim about pixels, and reading the layout code only reproduces the author's own mental terminal, which is where rendering verdicts go wrong. Run the thing and capture what rendered: + +\`\`\`bash +"\${QWEN_CODE_CLI:-qwen}" review capture-tui \\ + --command "" \\ + --cols --until "" \\ + --out /qwen-review--capture-- +\`\`\` + +(\`\` is this review's artifact prefix — the same one the plan and findings files carry — so Step 9's sweep reclaims the captures and identical finding ids across different reviews cannot collide.) + +It drives the command in a **private tmux server** (it cannot see, resize or kill the user's own sessions — the isolation is structural, and it reaps everything it started), writes \`.ans\` (the pane bytes, always), renders \`.png\` when \`freeze\` is available, and records which it managed in \`.json\`. To drive the UI first, gate the keystrokes on a rendered marker — \`--ready '' --keys \` — keys fired into a still-mounting UI get partially eaten (measured on this repo's own onboarding dialog). A key token starting with \`-\` must use the \`--keys=\` form (\`--keys=-l Enter\`), or yargs rejects the invocation as an unknown flag. And pick markers unique to the claim: a substring that exists in BOTH arms of a pair (measured: a provider name that also appears in another entry's description) settles the control arm falsely. Three rules make the capture evidence: + +- **Capture at the width the claim names, and at a control width.** "Clips at 80 columns" is confirmed by a pair — clipped at 80, intact at 120 — not by one image; a single capture cannot distinguish "clips at 80" from "clips everywhere", and those are different findings. +- **The evidence rung is part of the verdict.** A \`png\` is rendering evidence: attach it to the finding via \`assetFiles\` (Step 7's \`publish-assets\` embeds it in the posted comment when the run is authorised; unpublished, the local path still reaches the terminal report). An \`ans-only\` capture proves the bytes but not the pixels — quote the relevant lines and say the pixel claim is unverified. A refused capture (no tmux) leaves the claim at its reading-based confidence floor; say what a capture would have measured, so the reader knows what the tooling would have bought. +- **Attach only what this verification launched.** The command's isolation makes capturing the user's own terminal impossible through it; do not go around it with bare tmux or OS-level screenshots — a capture of anything but the review's own processes is the leak the private server exists to prevent. And judge the pixels, not just their source: a TUI rendering a masked key, an absolute path carrying the user's name, or a \`git remote\` URL with a token in it is an env dump even when the capture is of the review's own process. + +A finding a capture settled cites the manifest and carries the image in \`assetFiles\`; like a probe, the observation is the verdict — "the 80-column capture shows the panel's right border at column 83" quotes pixels, not a reading. + **When the claim is about GITHUB's behaviour, neither tree can settle it — only GitHub can.** A claim like "this encoding renders identically and can never ping", "GitHub strips this tag", "this markdown shape closes the fold" is about the comment pipeline's parser, sanitizer allowlist and notification path, none of which exist in this environment — a local markdown library is a model of GitHub, and judging a sanitizer claim against a model of the authority is exactly the parser-divergence failure under review. Measured live: an \`@\` → \`@\` defusal read as sound in every local trace, and GitHub's real renderer registered the mention and fired the notification. So: - **If the environment variable \`QWEN_REVIEW_SCRATCH_REPO\` is set** (an \`owner/repo\` the user designated for disposable test posts), you may adjudicate on the real renderer: post the payload as an issue comment there — \`gh api repos/$QWEN_REVIEW_SCRATCH_REPO/issues//comments -f body=@\` against an issue you created there for this purpose — read it back with \`-H "Accept: application/vnd.github.html+json"\`, and rule on the returned HTML (and, for mention claims, the timeline events). The observation is the verdict; quote it. This is the ONLY write destination other than \`submit\`'s that any part of this review may touch, it is user-designated, and nothing about the PR under review, its code, or its authors may appear in what you post there — post the minimal payload shape, not the report. diff --git a/packages/cli/src/commands/review/lib/tui-capture.test.ts b/packages/cli/src/commands/review/lib/tui-capture.test.ts new file mode 100644 index 00000000000..186a3b4962d --- /dev/null +++ b/packages/cli/src/commands/review/lib/tui-capture.test.ts @@ -0,0 +1,444 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { + captureServerName, + freezePlan, + tmuxPlan, + tmuxSupportsCaptureN, + tmuxSupportsCaptureT, + tmuxPadsWithCaptureN, + validGeometry, +} from './tui-capture.js'; + +describe('captureServerName', () => { + it('scopes by pid and nonce so concurrent reviews cannot collide', () => { + expect(captureServerName(123, 'abcd')).toBe('qwen-review-capture-123-abcd'); + expect(captureServerName(123, 'abcd')).not.toBe( + captureServerName(123, 'efgh'), + ); + }); +}); + +describe('validGeometry', () => { + it('accepts sane terminals and refuses the degenerate ones', () => { + expect(validGeometry(80, 24).ok).toBe(true); + expect(validGeometry(500, 200).ok).toBe(true); + // The exact lower bounds are ACCEPTED — a `v < lo` → `v <= lo` mutation + // would refuse a legal 20×5 capture with a self-contradictory message. + expect(validGeometry(20, 5).ok).toBe(true); + for (const [c, r] of [ + [0, 24], + [80, 0], + [19, 24], + [80, 4], + [501, 24], + [80, 201], + [80.5, 24], + [Number.NaN, 24], + // The ROWS branch needs its own non-integer and NaN cases: with only + // the cols ones, an asymmetric mutant that drops `Number.isInteger` + // from the rows check shipped green — and `new-session -y 24.5` + // reaches real tmux, which is not a shape this command should send. + [80, 24.5], + [80, Number.NaN], + ] as const) { + const v = validGeometry(c, r); + expect(v.ok, `${c}x${r}`).toBe(false); + } + }); + + it('names the FLAG that violated, not its sibling', () => { + // The reason is user-facing: a flag-name swap once produced + // "--rows must be an integer in [20, 500], got 10" for a --cols + // violation — the caller then "fixes" the wrong flag. + const cols = validGeometry(10, 24); + if (!cols.ok) expect(cols.reason).toContain('--cols'); + const rows = validGeometry(80, 1000); + if (!rows.ok) expect(rows.reason).toContain('--rows'); + expect(cols.ok).toBe(false); + expect(rows.ok).toBe(false); + }); +}); + +describe('tmuxSupportsCaptureN', () => { + it('accepts 3.1 and later, refuses the whole 3.0 line, ignores the unparseable', () => { + // -N landed in 3.1 (upstream CHANGES lists it under "CHANGES FROM 3.0a + // TO 3.1"; the 3.0a man page has no -N) — 3.0a/3.0b are TOO OLD, and + // Ubuntu 20.04 ships 3.0a: accepting them would die mid-capture on the + // unknown flag after paying for a server start. + for (const line of ['tmux 3.1', 'tmux 3.1b', 'tmux 3.3a', 'tmux 4.0']) { + expect(tmuxSupportsCaptureN(line), `${line}`).toBe(true); + } + for (const line of [ + 'tmux 1.8', + 'tmux 2.8', + 'tmux 3.0', + 'tmux 3.0a', + 'tmux 3.0b', + ]) { + expect(tmuxSupportsCaptureN(line), `${line}`).toBe(false); + } + // Unparseable is undefined, not false: a version that cannot be named + // is not a reason to refuse. + expect(tmuxSupportsCaptureN('')).toBeUndefined(); + expect(tmuxSupportsCaptureN('no digits here')).toBeUndefined(); + }); +}); + +describe('tmuxPlan — every call is scoped to the private server', () => { + const plan = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/work', + readyFile: '/ready', + }); + + it('carries -L on every call — start, capture, captureText, kill', () => { + // One stray unscoped call is the entire isolation property gone: an + // unscoped kill-server would kill the USER's tmux server. + for (const argv of [ + plan.start, + plan.capture, + plan.captureText, + plan.kill, + ]) { + const i = argv.indexOf('-L'); + expect(i).toBeGreaterThan(-1); + expect(argv[i + 1]).toBe('srv'); + } + // POSITION is load-bearing in start, not just presence: tmux requires + // global flags BEFORE the first command name, so a -L displaced past + // the `;` separator dies "no server running" on real tmux while every + // indexOf probe above stays green. + expect(plan.start.slice(2, 4)).toEqual(['-L', 'srv']); + }); + + it('starts CONFIG-FREE with a POSIX pane shell, in ONE client invocation', () => { + // -f /dev/null: without it the private server loads ~/.tmux.conf + // (measured: destroy-unattached killed the detached session). The + // default-shell pin rides the SAME invocation as new-session, chained + // with `;`, because a session-less server exits the moment its first + // client leaves — and it must run BEFORE the pane exists (measured: + // tcsh as default-shell killed the holder instantly). + expect(plan.start.slice(0, 2)).toEqual(['-f', '/dev/null']); + const set = plan.start.indexOf('set-option'); + const sep = plan.start.indexOf(';'); + const news = plan.start.indexOf('new-session'); + expect(set).toBeGreaterThan(-1); + expect(plan.start.slice(set, set + 4)).toEqual([ + 'set-option', + '-g', + 'default-shell', + '/bin/sh', + ]); + expect(sep).toBeGreaterThan(set); + expect(news).toBeGreaterThan(sep); + }); + + it('kills the SERVER, not the session — reaping everything it started', () => { + expect(plan.kill).toEqual(['-L', 'srv', 'kill-server']); + }); + + it('sends each key as ONE token behind `--` — no joining, no flag-eating', () => { + // Without `--`, tmux consumes a dash-leading token as a send-keys flag: + // measured, `send-keys -t cap -l` exits 0 and types NOTHING — silent + // evidence corruption. `--` makes every token a key, verbatim. + expect(plan.sendKeys('C-c')).toEqual([ + '-L', + 'srv', + 'send-keys', + '-t', + 'cap', + '--', + 'C-c', + ]); + expect(plan.sendKeys('-l')[plan.sendKeys('-l').length - 1]).toBe('-l'); + }); + + it('escapes a TRAILING `;` on the user-derived key and cwd elements', () => { + // tmux's client splits any argv element ending in `;` into a separate + // command before dispatch — `--` ends option parsing but never reaches + // that splitter. Measured on tmux 3.3a: `send-keys -- 'x;'` typed only + // `x` (exit 0, no warning), and `-c '/tmp/foo;'` turned the cwd element + // into a command boundary and failed with a misleading socket error; + // `\;` round-trips (pane_current_path came back `/tmp/foo;`). + expect(plan.sendKeys('x;').at(-1)).toBe('x\\;'); + expect(plan.sendKeys(';').at(-1)).toBe('\\;'); + // Mid-string is literal to tmux already and passes through. + expect(plan.sendKeys('a;b').at(-1)).toBe('a;b'); + // A token that ALREADY ends in `\;` still gets escaped: tmux consumes + // that backslash (measured on 3.3a — the token `x\;` types `x;`, and + // `x\\;` types `x\;`), and nothing escapes these values upstream, so + // treating it as already-escaped silently typed the wrong keys. + expect(plan.sendKeys('q\\;').at(-1)).toBe('q\\\\;'); + const withCwd = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/tmp/foo;', + readyFile: '/tmp/out.holder-ready', + }); + expect(withCwd.start[withCwd.start.indexOf('-c') + 1]).toBe('/tmp/foo\\;'); + }); + + it('escapes `#` in the cwd — the start-directory is FORMAT-EXPANDED', () => { + // Measured on tmux 3.3a and 3.4: a real directory named + // `/tmp/fmt/#{session_name}` passed the usability gate, and the pane + // started in `/tmp/fmt` — the PARENT — with exit 0 and the manifest + // recording the literal path. `##` round-trips to a literal `#` + // (verified), so a plain `#` in a dirname survives the doubling. + const fmt = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/tmp/fmt/#{session_name}', + readyFile: '/tmp/out.holder-ready', + }); + expect(fmt.start[fmt.start.indexOf('-c') + 1]).toBe( + '/tmp/fmt/##{session_name}', + ); + const plainHash = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/tmp/d/a#b', + readyFile: '/tmp/out.holder-ready', + }); + expect(plainHash.start[plainHash.start.indexOf('-c') + 1]).toBe( + '/tmp/d/a##b', + ); + }); + + it('drops -N on the tmux versions that FABRICATE trailing spaces', () => { + // 3.1-3.2.x pad each line out to the grid's allocated cells and have no + // -T to undo it: measured on 3.2a (what Ubuntu 22.04 ships), a + // three-character line came back with 17 phantom spaces. Trimming + // understates a clipped right edge; padding INVENTS one, and the + // command records the caveat as a degradation. + const opts = { + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/work', + readyFile: '/ready', + }; + expect(tmuxPlan({ ...opts, captureTrailing: false }).capture).not.toContain( + '-N', + ); + expect(tmuxPlan({ ...opts, captureTrailing: true }).capture).toContain( + '-N', + ); + // Default stays -N: only the padding versions opt out. + expect(tmuxPlan(opts).capture).toContain('-N'); + expect(tmuxPadsWithCaptureN('tmux 3.2a')).toBe(true); + expect(tmuxPadsWithCaptureN('tmux 3.1')).toBe(true); + expect(tmuxPadsWithCaptureN('tmux 3.3a')).toBe(false); + expect(tmuxPadsWithCaptureN('tmux 3.4')).toBe(false); + expect(tmuxPadsWithCaptureN('tmux 4.0')).toBe(false); + expect(tmuxPadsWithCaptureN('tmux next')).toBeUndefined(); + }); + + it('adds capture-pane -T only when the tmux has it (3.4+)', () => { + // -N alone pads a line out to its grid line's ALLOCATED cells: measured + // on 3.4, a row that had held 24 characters and was erased and rewritten + // with `BBB` came back as `BBB` plus four phantom spaces, so a column or + // clipping verdict would judge allocation history. -T drops exactly + // those unwritten positions. The same probe on 3.3a shows no padding — + // and passing -T there fails the call ('unknown flag -T', measured), so + // the flag must follow the version. + const opts = { + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/work', + readyFile: '/ready', + }; + const trimmed = tmuxPlan({ ...opts, captureTrim: true }); + expect(trimmed.capture).toContain('-T'); + expect(trimmed.captureText).toContain('-T'); + // -N stays: the REAL trailing spaces are the evidence. + expect(trimmed.capture).toContain('-N'); + const old = tmuxPlan({ ...opts, captureTrim: false }); + expect(old.capture).not.toContain('-T'); + expect(old.captureText).not.toContain('-T'); + expect(tmuxSupportsCaptureT('tmux 3.4')).toBe(true); + expect(tmuxSupportsCaptureT('tmux 3.5a')).toBe(true); + expect(tmuxSupportsCaptureT('tmux 4.0')).toBe(true); + expect(tmuxSupportsCaptureT('tmux 3.3a')).toBe(false); + expect(tmuxSupportsCaptureT('tmux 3.1')).toBe(false); + expect(tmuxSupportsCaptureT('tmux next')).toBeUndefined(); + }); + + it('starts the command behind `--` so a dash-leading command is not getopt fodder', () => { + const i = plan.start.indexOf('--'); + expect(i).toBeGreaterThan(-1); + expect(plan.start[i + 1]).toContain('node cli.js'); + expect(i + 2).toBe(plan.start.length); + }); + + it('PROPAGATES geometry — a hardcoded 80x24 must not pass', () => { + // Every other plan call in this file uses 80x24, so a plan that + // ignored opts.cols/opts.rows and hardcoded them passed the whole + // suite — and the fake-tmux seam tests too, since they drive the same + // default. Distinct values are the only way to see it. + const p = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 132, + rows: 43, + command: 'node cli.js', + cwd: '/work', + readyFile: '/ready', + }); + expect(p.start[p.start.indexOf('-x') + 1]).toBe('132'); + expect(p.start[p.start.indexOf('-y') + 1]).toBe('43'); + }); + + it('starts detached at the requested geometry and cwd', () => { + // new-session and its `-s cap` are the join key every later call + // targets via `-t cap` — dropping them would only fail the tmux-gated + // integration tests, so they are pinned here too. + expect(plan.start).toContain('new-session'); + const s = plan.start.indexOf('-s'); + expect(plan.start[s + 1]).toBe('cap'); + expect(plan.start).toContain('-d'); + const x = plan.start.indexOf('-x'); + expect(plan.start[x + 1]).toBe('80'); + const y = plan.start.indexOf('-y'); + expect(plan.start[y + 1]).toBe('24'); + const c = plan.start.indexOf('-c'); + expect(plan.start[c + 1]).toBe('/work'); + }); + + it('holds the pane open past the command in a NESTED shell', () => { + // tmux's remain-on-exit off destroys the session the moment the command + // exits (measured: a render-and-exit fixture was uncapturable 0/10). + // TWO shells, not one: in a single shell a command ending in `exit N` + // (or `exec`, or its own `set -e`) takes the keep-alive down with it — + // measured, deterministic "no server running" on `printf ...; exit 0`. + // The inner sh absorbs the exit; the outer holds the pane, with the + // hold on its OWN LINE so no command tail (`;`, `#`) can void it — and + // `trap : INT` so one C-c through the capture's own --keys path kills + // neither the holder nor the server (measured: untrapped, pane → + // session → server died before the capture). + expect(plan.start[plan.start.length - 1]).toBe( + `trap : INT QUIT\n( trap '' INT QUIT; sleep 10800; kill -9 -$$ 2>/dev/null ) &\n: > '/ready'\nsh -c 'node cli.js'\ni=0; while [ $i -lt 180 ]; do sleep 60; i=$((i+1)); done`, + ); + }); + + it('quote-escapes the command inside the holder script', () => { + const p = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: `printf '%s' "it's"`, + cwd: '/work', + readyFile: '/ready', + }); + // ONE layer: the plan hands tmux the holder SCRIPT, whose single + // `sh -c ''` line is the only place the command is quoted. A + // single quote in the command must not close that quoting. The + // expectation is COMPOSED with the same POSIX escaping rule stated + // independently ('→'\''): dropping esc() breaks the equality + // (measured: the mutant produced a holder /bin/sh rejects with an + // unmatched quote, while the structural assertions all stayed green). + const esc = (v: string): string => v.replaceAll("'", "'\\''"); + const cmd = `printf '%s' "it's"`; + const inner = `sh -c '${esc(cmd)}'`; + const held = p.start[p.start.length - 1]; + expect(held).toBe( + `trap : INT QUIT\n( trap '' INT QUIT; sleep 10800; kill -9 -$$ 2>/dev/null ) &\n: > '${esc('/ready')}'\n${inner}\ni=0; while [ $i -lt 180 ]; do sleep 60; i=$((i+1)); done`, + ); + }); + + it('quote-escapes a user-derived readyFile in the holder script', () => { + // --out is user-derived (verify briefs build it from target/branch + // names; git refs may carry an apostrophe), and the holder shell + // re-parses the sentinel line — so the path needs its OWN esc(): + // unescaped, an apostrophe broke the quoting and burned the full + // sentinel deadline blaming tmux (measured: exit 3 at ~10s). + const readyFile = "/evidence/ca'p/ready"; + const p = tmuxPlan({ + server: 'srv', + session: 'cap', + cols: 80, + rows: 24, + command: 'node cli.js', + cwd: '/work', + readyFile, + }); + const esc = (v: string): string => v.replaceAll("'", "'\\''"); + const inner = `sh -c '${esc('node cli.js')}'`; + const held = p.start[p.start.length - 1]; + // Single layer now: the plan hands tmux the SCRIPT itself (default-shell + // is pinned to /bin/sh in the same invocation), so the trap lives at + // layer 0 — QUIT included — and only the sentinel path needs escaping. + expect(held).toBe( + `trap : INT QUIT\n( trap '' INT QUIT; sleep 10800; kill -9 -$$ 2>/dev/null ) &\n: > '${esc(readyFile)}'\n${inner}\ni=0; while [ $i -lt 180 ]; do sleep 60; i=$((i+1)); done`, + ); + }); + + it('matches --until on a joined, escape-free view while .ans stays physical', () => { + // -J joins wraps and no -e keeps escapes out: a marker spanning a wrap + // boundary or an SGR change can never match the physical frame + // (measured: both miss forever). + expect(plan.captureText).toEqual([ + '-L', + 'srv', + 'capture-pane', + '-p', + '-J', + '-t', + 'cap', + ]); + }); + + it('captures with escapes and trailing spaces, wraps NOT joined', () => { + // -e escapes (freeze needs them), -N trailing spaces (a clipped right + // edge is trailing-space significant). Deliberately no -J: joining wraps + // re-flows the pane, erasing the wrap structure a layout claim is about — + // measured on the smoke capture, where -J turned a wrapped 100-char line + // back into one long line. + expect(plan.capture).toEqual([ + '-L', + 'srv', + 'capture-pane', + '-p', + '-e', + '-N', + '-t', + 'cap', + ]); + }); +}); + +describe('freezePlan', () => { + it('renders the .ans as ansi to the named output', () => { + expect(freezePlan('/x/a.ans', '/x/a.png')).toEqual([ + '--language', + 'ansi', + '/x/a.ans', + '--output', + '/x/a.png', + ]); + }); +}); diff --git a/packages/cli/src/commands/review/lib/tui-capture.ts b/packages/cli/src/commands/review/lib/tui-capture.ts new file mode 100644 index 00000000000..7505192e846 --- /dev/null +++ b/packages/cli/src/commands/review/lib/tui-capture.ts @@ -0,0 +1,416 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// The deterministic half of `capture-tui`: names, argument shapes, and the +// degradation ladder for terminal-rendering evidence. +// +// A verify agent asked to rule on "the panel clips at 80 columns" has, until +// now, read the layout code and imagined the terminal. Phase 1 gave findings a +// place to carry image evidence (`assetFiles` → `publish-assets`); this file +// and its command are Phase 2's producer: drive the TUI in a throwaway tmux, +// capture what it actually rendered, and hand back files a finding can carry. +// +// Everything here is pure so the naming, geometry and ladder rules are +// unit-testable without tmux, freeze, or a filesystem. The command layer owns +// the processes. + +/** + * The private tmux server name for one capture run. + * + * `-L` scopes a whole tmux SERVER, not just a session: the capture must never + * enumerate, resize, or kill anything on the user's own tmux server — the + * measured failure mode of desktop-automation verification was exactly + * "drives the user's own windows". A pid+nonce-scoped socket name means even + * two concurrent reviews cannot collide. + */ +/** The one server-name prefix, shared by the producer (captureServerName) + * and the orphan sweep's matcher (cleanup.ts): as two independent literals, + * a prefix rename silently turns the sweep into a permanent no-op. */ +export const CAPTURE_SERVER_PREFIX = 'qwen-review-capture-'; + +/** Whether a failed `kill-server` means there was NOTHING to kill — the + * goal state, not a failure. tmux says it several ways depending on how far + * the server got: `no server running on `, `error connecting to + * (No such file or directory)`, `no such file or directory`, and — + * when the socket directory itself could never be created (measured with a + * mode-0555 TMUX_TMPDIR) — `couldn't create directory (Permission + * denied)`. Reading only the first wording printed a false orphan WARNING + * naming a server that never existed. */ +export function isNothingToKill(stderr: string): boolean { + return ( + /no server running/i.test(stderr) || + /error connecting to .*(no such file or directory)/i.test(stderr) || + /(couldn't|could not|can't|cannot) create directory/i.test(stderr) || + // A socket path past sun_path (~108 bytes): tmux answers this to + // new-session AND kill-server, so a start that never created a socket + // printed a false orphan WARNING (reproduced on 3.3a with a long + // TMUX_TMPDIR). + /file name too long/i.test(stderr) || + /no such file or directory/i.test(stderr) + ); +} + +export function captureServerName(pid: number, nonce: string): string { + return `${CAPTURE_SERVER_PREFIX}${pid}-${nonce}`; +} + +/** Default geometry: the classic terminal, which is also where most layout + * bugs live. Callers override for wide/narrow claims. */ +export const DEFAULT_COLS = 80; +export const DEFAULT_ROWS = 24; + +/** Bounds that keep a typo from asking tmux for a 0x0 or 9999x9999 pane. */ +export function validGeometry( + cols: number, + rows: number, +): { ok: true } | { ok: false; reason: string } { + const bad = (name: string, v: number, lo: number, hi: number) => + !Number.isInteger(v) || v < lo || v > hi + ? `${name} must be an integer in [${lo}, ${hi}], got ${String(v)}` + : null; + const c = bad('--cols', cols, 20, 500); + if (c) return { ok: false, reason: c }; + const r = bad('--rows', rows, 5, 200); + if (r) return { ok: false, reason: r }; + return { ok: true }; +} + +/** Whether a `tmux -V` line ("tmux 3.3a") names a tmux with capture-pane + * `-N`: the trailing-space flag the physical capture is load-bearing on + * landed in 3.1 — the whole 3.0 line, letters included, is too old. + * Undefined when the version does not parse — an unnameable version is not + * a reason to refuse, but a NAMED old one is. */ +export function tmuxSupportsCaptureN(versionLine: string): boolean | undefined { + const m = /(\d+)\.(\d+)([a-z]*)/i.exec(versionLine); + if (!m) return undefined; + const major = Number(m[1]); + const minor = Number(m[2]); + if (major !== 3) return major > 3; + // capture-pane -N landed in 3.1 (upstream CHANGES lists it under "CHANGES + // FROM 3.0a TO 3.1"; the 3.0a man page's synopsis has no -N) — the whole + // 3.0 line, letters included, is too old. Ubuntu 20.04 ships 3.0a. + return minor >= 1; +} + +/** Whether a `tmux -V` line names a tmux whose `capture-pane -N` PADS each + * line out to the grid line's allocated cells. Measured: 3.2a pads (`BBB` + * came back as `BBB` + 17 spaces) and has no `-T` to undo it; 3.3a does not + * pad; 3.4+ pads but takes `-T`. So the padding-without-a-remedy window is + * exactly 3.1–3.2.x — which Ubuntu 22.04 ships. Undefined when the version + * does not parse. */ +export function tmuxPadsWithCaptureN(versionLine: string): boolean | undefined { + const m = /(\d+)\.(\d+)([a-z]*)/i.exec(versionLine); + if (!m) return undefined; + const major = Number(m[1]); + const minor = Number(m[2]); + if (major !== 3) return false; + return minor < 3; +} + +/** Whether a `tmux -V` line names a tmux whose `capture-pane` takes `-T` + * ("ignore trailing positions that do not contain a character"), which + * landed in 3.4. Undefined when the version does not parse — the caller + * treats that as "no -T", the behaviour every pre-3.4 tmux needs anyway. */ +export function tmuxSupportsCaptureT(versionLine: string): boolean | undefined { + const m = /(\d+)\.(\d+)([a-z]*)/i.exec(versionLine); + if (!m) return undefined; + const major = Number(m[1]); + const minor = Number(m[2]); + if (major !== 3) return major > 3; + return minor >= 4; +} + +/** + * How far the capture got, in evidence terms. + * + * The ladder is explicit because each rung is a DIFFERENT claim in a review: + * a PNG is publishable rendering evidence; an `.ans` proves the bytes but not + * the pixels (and cannot be published — the assets allowlist is images only); + * `none` means the claim stays argued in prose. A verifier must say which + * rung its verdict stands on. + */ +export type CaptureEvidence = 'png' | 'ans-only' | 'none'; + +/** One capture's outcome, as the manifest records it. The manifest is the + * capture's ONLY record (stdout carries just a pointer to it), so it names + * every rendering-affecting input: a capture driven by `--keys` shows a + * different screen than the bare command, and a reproducer that does not + * know that judges honest evidence unreproducible. */ +export interface CaptureManifest { + command: string; + cwd: string; + cols: number; + rows: number; + keys?: string[]; + /** Whether the keys were actually typed — false when --ready never + * matched and they were withheld rather than fired at an unknown screen. */ + keysSent?: boolean; + ready?: string; + until?: string; + settleMs?: number; + timeoutMs?: number; + /** The raw pane text with escapes — always written; a refused capture + * writes no manifest at all. */ + ansPath: string; + /** The rendered image — null when freeze is unavailable or failed. */ + pngPath: string | null; + /** Never `none`: a refused capture writes no manifest at all — `none` is + * the rung a VERDICT stands on when there is no manifest to cite. */ + evidence: Exclude; + /** Why the ladder stopped where it did (freeze missing, timeout, …). */ + degradedBecause?: string; + /** How long the run waited before capturing, and why it stopped waiting. */ + settledBy: 'until-match' | 'timeout' | 'fixed-delay'; +} + +/** + * The tmux invocations for one capture, in order. Pure — the command layer + * execs them — so the exact argv shapes are pinned by tests, not by hope. + * Every call carries `-L `: one stray unscoped call is the entire + * isolation property gone. + */ +export function tmuxPlan(opts: { + server: string; + session: string; + cols: number; + rows: number; + command: string; + cwd: string; + /** Whether to ask for real trailing spaces at all (`-N`). False only on + * the tmux versions whose `-N` FABRICATES them and that have no `-T` to + * undo it — see tmuxPadsWithCaptureN. */ + captureTrailing?: boolean; + /** Whether this tmux takes `capture-pane -T` (3.4+) — see the capture + * argv below. False on older versions, which need no trimming and reject + * the flag. */ + captureTrim?: boolean; + /** Absolute path the holder touches AFTER its trap is installed — the + * command layer sends no key until this file exists, closing the race + * where a --keys C-c lands before the holder's first line has run + * (measured: the INTR fires the instant tmux writes 0x03 to the pty, + * not when the shell reads it — no in-script ordering can win). */ + readyFile: string; +}): { + start: string[]; + capture: string[]; + captureText: string[]; + kill: string[]; + sendKeys: (key: string) => string[]; +} { + const scope = ['-L', opts.server]; + const esc = (s: string): string => s.replaceAll("'", "'\\''"); + // The pane must outlive the command: tmux's default `remain-on-exit off` + // destroys pane → window → session the moment the command exits, so a + // one-shot command (render and exit — exactly what a verify fixture looks + // like) would be uncapturable (measured: 0/10 without the holder). + // `kill-server` reaps the holder along with everything else; for an + // UNREAPED holder the bounded hold loop below is the only other reaper — + // its periods cap the orphan at three hours, and a legal capture never + // outlives its reap. + // + // TWO nested shells, not one: in a single shell, a command ending in + // `exit N` (or opening with `exec`, or running under its own `set -e`) + // takes the keep-alive down with it — pane, session, and server gone + // before the capture (measured: deterministic "no server running" refusal + // on `printf ...; exit 0`). The inner sh absorbs the exit; the outer one + // holds the pane. + // + // The outer holder ALSO survives SIGINT: non-interactive shells stay in + // the pane's foreground process group, so one C-c — a canonical --keys + // token — delivers INTR to the holder itself and would take pane → + // session → server down before the capture (measured). `trap : INT`, NOT + // `trap '' INT`: SIG_IGN inherits across exec and would silently blunt + // C-c for targets without their own handler, while a trapped signal + // resets to default in children — the command keeps its normal Ctrl-C + // behavior, and only the holder is protected. + // + // The hold sits on its OWN LINE: appended with `;` it is voided by the + // command's own tail — a trailing `;` makes `;;` (syntax error, pane dies + // instantly), a trailing `#` comment swallows it (the one-shot failure + // recurs), and both blame tmux for a valid command. Trailing backslashes + // cannot fold the hold line either: the command sits single-quoted at + // every layer, so no shell parses its text adjacent to the hold + // (probe-verified with odd-run shapes on this exact plan). + const inner = `sh -c '${esc(opts.command)}'`; + // tmux's CLIENT splits any argv element ending in `;` into a separate + // command before dispatch (cmd_parse_from_arguments, unchanged since 3.1 — + // the lowest version this command's gate admits); `--` ends option parsing + // but does NOT reach the command splitter. Both user-derived elements need + // it, measured on 3.3a: `--keys 'x;'` typed only `x` — exit 0, no warning, + // silent corruption of the very evidence this command guarantees — and a + // `--cwd '/tmp/foo;'` (a legal POSIX dirname that passes the usability + // gate) made the `-c` element a command boundary, failing with a + // misleading socket error. `\;` is tmux's escape and round-trips (verified: + // pane_current_path came back `/tmp/foo;`); a mid-string `;` is literal + // already. EVERY trailing `;` is escaped, including one already preceded + // by a backslash: tmux CONSUMES that backslash (measured: the token + // `x\;` types `x;`, `x\\;` types `x\;`), and nothing escapes these + // values before they reach here — so treating `\;` as already-escaped + // silently corrupted a cwd or key token that legitimately ends in it. + const escapeTrailingSemicolon = (s: string): string => + s.endsWith(';') ? `${s.slice(0, -1)}\\;` : s; + // readyFile is user-derived (--out) and re-parsed by the holder shell — + // it gets its own esc() (an apostrophe in --out broke the quoting and + // burned the full sentinel deadline, measured). And the hold is a LOOP, + // not one sleep: after a one-shot command exits, a --keys C-c kills the + // running sleep; the trap runs and a single-sleep script would simply + // end — pane, session and server gone (measured 5/5). The loop re-enters + // sleep and the pane survives. The loop is BOUNDED — 180 periods of one + // minute — so an unreaped holder (SIGKILL'd harness, OOM) self-terminates + // after three hours instead of living indefinitely. The WATCHDOG carries + // that same cap for the other half of the lifetime: the loop only starts + // once the captured command has exited, so a command that keeps running + // (a TUI — the normal case) left the cap unreachable and an orphaned + // server lived on. `kill -9 -$$` takes the whole pane process group, and + // tmux tears the pane, session and server down behind it (probe-verified + // with a still-running command: `no server running` right after the + // watchdog fired). `$$` is the holder's pid inside the subshell — a + // subshell does not change it — and the holder is its group leader. It + // IGNORES INT and QUIT, and must: an async subshell in a non-interactive + // shell already ignores them per POSIX, so a `--keys C-c` killed only its + // `sleep` and it ran straight into the kill — measured end to end with + // bash 5.2 as /bin/sh, ONE C-c took pane, session and server down and the + // capture refused `no server running` with zero artifacts, while the same + // run with this trap captured its marker. (dash does not reproduce it, so + // a dash-only probe would have missed it.) `trap ''` is SIG_IGN and + // inherits across exec, but nothing here execs anything but `sleep`, so + // the captured command keeps its own Ctrl-C behaviour. MANY SHORT periods, + // not a few long ones: each post-exit signal consumes the period it + // interrupts, so with three hour-long sleeps the three C-c tokens this + // command explicitly supports exhausted the whole budget MID-CAPTURE — + // pane, session and server gone before capture-pane ran, refusing + // `tmux failed mid-capture: no server running` (measured 5/5 on + // `--keys C-c C-c C-c`) and blaming tmux for the holder's own budget. + // A minute per period keeps the same three-hour cap while making a + // keypress cost a minute of it. + // NO outer `sh -c` wrapper: the same invocation pins default-shell to + // /bin/sh, so tmux's direct child — the pane's session leader — runs this + // script ITSELF, and the trap lives at layer 0. Wrapped, the trap sat one + // layer deep: INT survived via the outer shell's wait-and-cooperate + // semantics, but a --keys C-\ (SIGQUIT) killed the untrapped layer 0 — + // pane, session, server gone (measured end-to-end). QUIT is trapped for + // the same reason INT is; both reset to default in the children. + const held = `trap : INT QUIT\n( trap '' INT QUIT; sleep 10800; kill -9 -$$ 2>/dev/null ) &\n: > '${esc(opts.readyFile)}'\n${inner}\ni=0; while [ $i -lt 180 ]; do sleep 60; i=$((i+1)); done`; + return { + // ONE client invocation, three properties: + // - `-f /dev/null` starts the server CONFIG-FREE: without it the + // private server loads ~/.tmux.conf, and user options reach into the + // capture — measured: `set -g destroy-unattached on` killed the + // detached session with a misattributed "no server running" refusal. + // - `set-option -g default-shell /bin/sh` runs BEFORE new-session (the + // `;` chains commands inside the same client): the holder string is + // parsed by tmux's default-shell, and an exotic login shell + // (measured: tcsh via $SHELL or passwd) chokes on it. + // - Both ride the same invocation as new-session because a session-less + // server exits the moment its first client leaves (exit-empty) — a + // separate bootstrap call left "no server running" for the next one. + start: [ + '-f', + '/dev/null', + ...scope, + 'set-option', + '-g', + 'default-shell', + '/bin/sh', + ';', + 'new-session', + '-d', + '-s', + opts.session, + '-x', + String(opts.cols), + '-y', + String(opts.rows), + '-c', + // BOTH escapes, in this order: `#` doubles first (tmux + // format-expands the start-directory — measured on 3.3a and 3.4, a + // real directory named `/tmp/fmt/#{session_name}` started the pane in + // `/tmp/fmt`, the PARENT, with exit 0 and the manifest recording the + // literal path the gate had stat()ed), then the trailing `;` for the + // client's command splitter. `##` round-trips to a literal `#`, so a + // plain `#` in a dirname is unaffected (measured both ways). + escapeTrailingSemicolon(opts.cwd.replaceAll('#', '##')), + // `--` ends option parsing: a command that happens to start with `-` + // must reach the shell, not tmux's getopt (measured: without it, + // send-keys silently ate `-l` as its literal flag — exit 0, nothing + // typed — the worst kind of evidence corruption). + '--', + held, + ], + // -p print, -e escapes, -N trailing spaces. Deliberately NOT -J: joining + // wrapped lines re-flows the pane into logical lines, and for a layout + // claim the wrap structure IS the evidence — a 100-char line in an + // 80-column pane must capture as two lines, exactly as rendered (measured: + // with -J the smoke capture showed one long unwrapped line, erasing the + // very clipping it was capturing). -N keeps column claims honest — a + // clipped right edge is trailing-space significant. + capture: [ + ...scope, + 'capture-pane', + '-p', + '-e', + // -N asks tmux to keep the REAL trailing spaces — dropped only where + // it would invent them instead (tmux 3.1-3.2.x, which pad to the grid + // allocation and have no -T): there, a trimmed line understates a + // clipped right edge, while a padded one FABRICATES evidence, and the + // manifest records the caveat as a degradation. + ...(opts.captureTrailing === false ? [] : ['-N']), + // -N alone pads each line out to the grid line's ALLOCATED cell count, + // not what it rendered: measured on tmux 3.4, a row that had held 24 + // characters and was then erased and rewritten with `BBB` came back as + // `BBB` plus four phantom spaces, so a verdict about column position, + // clipping or trailing-space significance would judge allocation + // history instead of rendering. -T drops those unwritten trailing + // positions while -N keeps the REAL trailing spaces. The flag landed + // in 3.4 and the same probe on 3.3a shows no padding to remove, so + // older versions are correct without it — and passing it there would + // fail the call outright ("unknown flag -T", measured). + ...(opts.captureTrim ? ['-T'] : []), + '-t', + opts.session, + ], + // The MATCHING view for --until: `-J` joins wrapped lines and no `-e` + // keeps escapes out, so a marker that spans a wrap boundary or an SGR + // attribute change still matches (measured: both miss forever on the + // physical view). The physical frame above stays what `.ans` records. + captureText: [ + ...scope, + 'capture-pane', + '-p', + '-J', + // Same padding hazard, and worse here: -J JOINS wrapped lines, so + // phantom trailing cells would be spliced into the middle of the text + // a --until/--ready marker is matched against. + ...(opts.captureTrim ? ['-T'] : []), + '-t', + opts.session, + ], + // kill-server, not kill-session: the server is ours alone (private -L), + // and killing it reaps every process the capture started — no orphaned + // TUI keeps running after the review. + kill: [...scope, 'kill-server'], + // One send-keys per token, verbatim — quoting-by-joining is how a key + // sequence silently becomes a different key sequence. `--` for the same + // reason as start: a dash-leading key token (`-l`) is otherwise consumed + // as a send-keys flag, silently. In the plan so the shape is pinned like + // the others: it was the one argv built ad hoc. + sendKeys: (key: string) => [ + ...scope, + 'send-keys', + '-t', + opts.session, + '--', + escapeTrailingSemicolon(key), + ], + }; +} + +/** freeze argv for rendering an .ans capture — pinned so "write the .ans + * FIRST, then render" survives (freeze has hung mid-render on this repo's + * own workflows; the text evidence must already be on disk when it does). */ +export function freezePlan(ansPath: string, pngPath: string): string[] { + return ['--language', 'ansi', ansPath, '--output', pngPath]; +} diff --git a/packages/cli/src/commands/review/run.test.ts b/packages/cli/src/commands/review/run.test.ts index 2cd8cdd0f63..129f6a9392c 100644 --- a/packages/cli/src/commands/review/run.test.ts +++ b/packages/cli/src/commands/review/run.test.ts @@ -339,8 +339,34 @@ describe('review run (handler)', () => { const done = runHandler(); // The capture poll snapshots the verdict while the child still runs... await vi.advanceTimersByTimeAsync(1_000); - // ...then Step 9 runs the REAL cleanup, which sweeps the verdict... - runCleanup('local'); + // ...then Step 9 runs the REAL cleanup, which sweeps the verdict. + // POINT ITS HOST-WIDE CAPTURE SWEEP AT AN EMPTY DIRECTORY FIRST: the + // sweep is real here while `execFileSync` is a bare mock that never + // throws, so every socket it finds reads as successfully killed and is + // then UNLINKED from the real filesystem. Until now only the unrelated + // `process.kill` spy — returning true for every pid, so no socket ever + // looked orphaned — stood between this test and a developer's live + // tmux sockets, and nothing said so. An isolated base plus a + // /tmp-shaped decoy keeps both scan bases inside the fixture. + const sweepBase = mkdtempSync(join(tmpdir(), 'review-run-sweep-')); + // BOTH bases: the sweep always scans /tmp as well, and a developer's + // /tmp/tmux- is full of live sockets. A uid nobody owns makes the + // /tmp leg find no directory at all, while the env leg lands in this + // empty fixture. + const fakeUid = 987_654; + mkdirSync(join(sweepBase, `tmux-${String(fakeUid)}`), { recursive: true }); + const realGetuid = process.getuid; + process.getuid = () => fakeUid; + const realTmuxTmpdir = process.env['TMUX_TMPDIR']; + process.env['TMUX_TMPDIR'] = sweepBase; + try { + runCleanup('local'); + } finally { + process.getuid = realGetuid; + if (realTmuxTmpdir === undefined) delete process.env['TMUX_TMPDIR']; + else process.env['TMUX_TMPDIR'] = realTmuxTmpdir; + rmSync(sweepBase, { recursive: true, force: true }); + } outs.length = 0; // drop cleanup's "Removed temp file" stdout noise // ...and only then does the child exit. child.emit('close', 0); diff --git a/packages/core/src/skills/bundled/review/SKILL.md b/packages/core/src/skills/bundled/review/SKILL.md index 6ff28dcbefa..cdc8088f046 100644 --- a/packages/core/src/skills/bundled/review/SKILL.md +++ b/packages/core/src/skills/bundled/review/SKILL.md @@ -1060,7 +1060,7 @@ Then reference each finding's `assets` URLs in its inline comment body as `![evi - **Immutable references** — files land on `pr-assets/-review` of the assets repo (the manual `pr-assets/-verify` convention, suffixed so the two flows never collide), and every URL is pinned to the **commit**, not the branch, so a posted comment's evidence cannot be changed from under it. Content-hashed remote names make a re-run idempotent rather than accumulative. - **Auditable** — the manifest names every file pushed and the commit they landed on, next to the other review artifacts, where Step 9's sweep and a curious human can find it. -**What you must still judge: the image's content.** The command checks extensions, sizes and image magic bytes (a shell script named `evidence.png` refuses on content) — that catches mislabeled or corrupted captures, not a deliberate payload riding behind a real image header; it cannot see that a terminal screenshot has an env dump in the scrollback. Publish only evidence the review itself produced — a capture of a rendering the verification ran, a before/after the A/B produced — and never a capture of the user's own terminal or editor. When in doubt, keep the finding's evidence as prose and local paths. +**What you must still judge: the image's content.** The command checks extensions, sizes and image magic bytes (a shell script named `evidence.png` refuses on content) — that catches mislabeled or corrupted captures, not a deliberate payload riding behind a real image header; it cannot see that a terminal screenshot has an env dump in the scrollback. Publish only evidence the review itself produced — and `qwen review capture-tui` is the sanctioned producer for terminal renderings: it drives the command in a **private tmux server** (structurally unable to see the user's own sessions), captures `.ans` always and `.png` when `freeze` is available, and manifests which evidence rung it reached. Never publish a capture of the user's own terminal or editor. When in doubt, keep the finding's evidence as prose and local paths. **Build the review JSON** with `write_file` to create `.qwen/tmp/qwen-review-{target}-review.json`. It carries three things and **no verdict** — `submit` computes the event and body itself, from the `state` you hand it and the comments you attach, and **refuses a payload that carries `event` or `body`** (a run that skipped the computation and typed its own Approve is exactly what that refusal stops). Every high-confidence Critical or Suggestion finding that maps to a diff line is an entry in `comments`: diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index cc4c8239182..88a59a082f5 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -294,10 +294,13 @@ describe('GlobTool', () => { it('should allow path outside workspace (external path support)', async () => { // Shared /tmp made this walk time out on loaded runners — keep this - // dir dedicated and empty. + // dir dedicated. Seed a real file: with an EMPTY dir the assertions + // below were vacuous — any outcome, including "found nothing at + // all", passed them. const outside = await fs.mkdtemp( path.join(os.tmpdir(), 'glob-external-'), ); + await fs.writeFile(path.join(outside, 'external.txt'), 'x'); try { const params: GlobToolParams = { pattern: '*.txt', path: outside }; const invocation = globTool.build(params); @@ -307,6 +310,9 @@ describe('GlobTool', () => { expect(result.returnDisplay).not.toContain( 'Path is not within workspace', ); + // The glob really walked the external path: the seeded file comes + // back (a regression to "nothing found" now fails, not passes). + expect(result.llmContent).toContain('Found 1 file(s)'); } finally { await fs.rm(outside, { recursive: true, force: true }); }