diff --git a/src/engine/spawnSubagent.ts b/src/engine/spawnSubagent.ts index 69209eb..94bd271 100644 --- a/src/engine/spawnSubagent.ts +++ b/src/engine/spawnSubagent.ts @@ -206,7 +206,7 @@ export interface SpawnResult { /** #49: extract file paths a tool event touched, for the structured partial-result report. * edit/write carry a `path` arg reliably; bash is best-effort (redirections `>`/`>>` + `tee` to a * path-like token). Reads are NOT mutations and are excluded. */ -function extractTouchedFiles(toolName: string, args: unknown): string[] { +export function extractTouchedFiles(toolName: string, args: unknown): string[] { if (!args || typeof args !== "object") return []; const a = args as Record; // Case-insensitive: pi tools are lowercase ("edit"), claude's are capitalized ("Edit"). @@ -224,19 +224,41 @@ function extractTouchedFiles(toolName: string, args: unknown): string[] { const redir = />>?\s+([^\s|;&<>]+)/g; let m: RegExpExecArray | null; while ((m = redir.exec(cmd)) !== null) { - const tok = m[1]; - if (tok && /[/.]/.test(tok)) out.push(tok); + const tok = m[1] ? shapeToken(m[1]) : undefined; + if (tok) out.push(tok); } const tee = /\btee\s+(?:-a\s+)?([^\s|;&<>]+)/g; while ((m = tee.exec(cmd)) !== null) { - const tok = m[1]; - if (tok && /[/.]/.test(tok)) out.push(tok); + const tok = m[1] ? shapeToken(m[1]) : undefined; + if (tok) out.push(tok); } return out; } return []; } +/** #87: decide whether a redirect/tee capture is plausibly a file path. The bare `/[/.]/` + * test false-positived on `>` characters inside quoted strings/heredocs/code text + * ("cache.load(name,,", "[...active],,") and swallowed trailing punctuation from code-y + * commands ("/tmp/out.txt),"). Now: strip wrapping quotes, trim trailing punctuation, + * then require a real path shape (contains `/`, or a dotted filename, or a leading-dot + * file like .gitignore). */ +function shapeToken(raw: string): string | undefined { + let tok = raw.trim().replace(/^["']+/, ""); + // trailing junk can interleave ("'/tmp/x.txt',") — strip quotes+punct until stable + let prev: string; + do { + prev = tok; + tok = tok.replace(/["',;)\]}]+$/g, ""); + } while (tok !== prev); + if (!tok) return undefined; + if (/^\/dev\/(null|stdout|stderr|stdin|tty|zero)$/.test(tok)) return undefined; // pseudo-devices, not touched files + if (tok.includes("/")) return tok; + if (/^\.[A-Za-z0-9._-]+$/.test(tok)) return tok; // .gitignore and friends + if (/^[A-Za-z0-9._-]+\.[A-Za-z0-9]+$/.test(tok)) return tok; // name.ext + return undefined; +} + export async function spawnSubagent(opts: SpawnOptions): Promise { const track = opts.track ?? true; const maxTurns = opts.maxTurns ?? DEFAULT_MAX_TURNS; diff --git a/test/extract-touched-files.test.mts b/test/extract-touched-files.test.mts new file mode 100644 index 0000000..5dec707 --- /dev/null +++ b/test/extract-touched-files.test.mts @@ -0,0 +1,57 @@ +// test/extract-touched-files.test.mts +// #87: the bash-redirect heuristic in extractTouchedFiles false-positived on ">" inside +// quoted strings/heredocs/code text and swallowed trailing punctuation — the turn-budget +// partial narrative then listed junk tokens ("cache.load(name,,", "[...active],,"). +import { test } from "node:test"; +import { deepStrictEqual, strictEqual } from "node:assert"; +import { extractTouchedFiles } from "../src/engine/spawnSubagent.ts"; + +test("edit/write: path + file_path args (both casings) extract the path", () => { + deepStrictEqual(extractTouchedFiles("edit", { path: "/repo/src/a.ts" }), ["/repo/src/a.ts"]); + deepStrictEqual(extractTouchedFiles("Edit", { file_path: "/repo/src/b.ts" }), ["/repo/src/b.ts"]); + deepStrictEqual(extractTouchedFiles("write", {}), []); +}); + +test("bash: plain redirects + tee capture the target", () => { + deepStrictEqual(extractTouchedFiles("bash", { command: "echo hi > /tmp/out.txt" }), ["/tmp/out.txt"]); + deepStrictEqual(extractTouchedFiles("bash", { command: "echo hi >> /var/log/app.log" }), ["/var/log/app.log"]); + deepStrictEqual(extractTouchedFiles("bash", { command: "cat in | tee out.txt" }), ["out.txt"]); + deepStrictEqual(extractTouchedFiles("bash", { command: "cat in | tee -a out.txt" }), ["out.txt"]); +}); + +test("#87: the REAL reported junk tokens are NOT captured", () => { + // verbatim shapes from the #87 report (garbled "Files modified before the cut" list) + deepStrictEqual(extractTouchedFiles("bash", { command: `echo "if (x > y) cache.load(name,, 3)" > /tmp/e2e-out.txt` }), + ["/tmp/e2e-out.txt"], "code text after an in-string > must not be captured"); + deepStrictEqual(extractTouchedFiles("bash", { command: `echo "spread: > [...active],," > /tmp/ms-leg.txt` }), + ["/tmp/ms-leg.txt"]); + deepStrictEqual(extractTouchedFiles("bash", { command: `node -e 'console.log("> registered.set(t.name,")' > /tmp/x.txt` }), + ["/tmp/x.txt"]); + strictEqual(extractTouchedFiles("bash", { command: `echo "> cache.load(name,," ` }).length, 0, + "junk token with no real path capture → nothing"); + strictEqual(extractTouchedFiles("bash", { command: `echo "> [...active],,"` }).length, 0); +}); + +test("#87: trailing punctuation from code-y commands is trimmed off real paths", () => { + deepStrictEqual(extractTouchedFiles("bash", { command: `run_suite() { ... } > /tmp/e2e-out.txt), 2>&1` }), + ["/tmp/e2e-out.txt"], "trailing ), stripped"); + deepStrictEqual(extractTouchedFiles("bash", { command: `tee /tmp/ms-leg.txt, < { + deepStrictEqual(extractTouchedFiles("bash", { command: `echo hi > '/tmp/quoted.txt'` }), ["/tmp/quoted.txt"]); + deepStrictEqual(extractTouchedFiles("bash", { command: `echo hi > "out.txt"` }), ["out.txt"]); +}); + +test("#87: bare words and non-path dots stay rejected; dotted filenames pass", () => { + strictEqual(extractTouchedFiles("bash", { command: `echo done > y` }).length, 0, "bare word, no / or ext"); + strictEqual(extractTouchedFiles("bash", { command: `cmp a b > /dev/null` }).length, 0, "/dev/null is not a touched file"); + deepStrictEqual(extractTouchedFiles("bash", { command: `git log > .gitignore` }), [".gitignore"], "leading-dot filenames allowed"); + deepStrictEqual(extractTouchedFiles("bash", { command: `obj.dump > dump.out` }), ["dump.out"]); +}); + +test("#87 review NIT: interleaved quote+punctuation junk strips fully", () => { + deepStrictEqual(extractTouchedFiles("bash", { command: `diff <(sort a) <(sort b) > '/tmp/x.txt',` }), ["/tmp/x.txt"]); + deepStrictEqual(extractTouchedFiles("bash", { command: `run > "/tmp/y.txt");` }), ["/tmp/y.txt"]); +});