From 50e084a9672a009d194b3b7112aece90a6b04249 Mon Sep 17 00:00:00 2001 From: RECTOR Date: Sun, 30 Aug 2026 15:25:17 +0700 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20harden=20extractTouchedFiles=20bash-?= =?UTF-8?q?redirect=20parsing=20=E2=80=94=20junk=20tokens=20in=20the=20par?= =?UTF-8?q?tial=20narrative=20(#87)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the #87 garbling: the redirect/tee heuristic captured any token containing / or . — so ">" characters inside quoted strings, heredoc bodies, and echoed code text yielded junk ("cache.load(name,,", "[...active],,"), and the char class also swallowed trailing punctuation from code-y commands ("/tmp/e2e-out.txt),"). The budget-cut framing was incidental; the junk accumulated because the affected task was E2E-heavy. New shapeToken filter: strip wrapping quotes, trim trailing punctuation, require a real path shape (contains "/", dotted filename, or leading-dot file), and exclude /dev pseudo-devices (/dev/null was silently listed as a touched file before — arguably wrong too). extractTouchedFiles is now exported + unit-tested (6 tests pin the verbatim reported junk tokens, punctuation trimming, quote unwrapping, and the /dev/null exclusion). --- src/engine/spawnSubagent.ts | 27 ++++++++++++--- test/extract-touched-files.test.mts | 52 +++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 test/extract-touched-files.test.mts diff --git a/src/engine/spawnSubagent.ts b/src/engine/spawnSubagent.ts index 69209eb..cb0cd28 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,36 @@ 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(/^["']+/, "").replace(/["']+$/, ""); + tok = tok.replace(/[,;)\]}]+$/g, ""); + 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..500edf1 --- /dev/null +++ b/test/extract-touched-files.test.mts @@ -0,0 +1,52 @@ +// 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"]); +}); From d0cfaeb5664282395500ffc0ca0e0037616d7a09 Mon Sep 17 00:00:00 2001 From: RECTOR Date: Sun, 30 Aug 2026 15:28:52 +0700 Subject: [PATCH 2/2] fix: interleaved quote+punct trailing strip in shapeToken (review NIT 1) --- src/engine/spawnSubagent.ts | 9 +++++++-- test/extract-touched-files.test.mts | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/engine/spawnSubagent.ts b/src/engine/spawnSubagent.ts index cb0cd28..94bd271 100644 --- a/src/engine/spawnSubagent.ts +++ b/src/engine/spawnSubagent.ts @@ -244,8 +244,13 @@ export function extractTouchedFiles(toolName: string, args: unknown): string[] { * 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(/^["']+/, "").replace(/["']+$/, ""); - tok = tok.replace(/[,;)\]}]+$/g, ""); + 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; diff --git a/test/extract-touched-files.test.mts b/test/extract-touched-files.test.mts index 500edf1..5dec707 100644 --- a/test/extract-touched-files.test.mts +++ b/test/extract-touched-files.test.mts @@ -50,3 +50,8 @@ test("#87: bare words and non-path dots stay rejected; dotted filenames pass", ( 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"]); +});