Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/engine/spawnSubagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
// Case-insensitive: pi tools are lowercase ("edit"), claude's are capitalized ("Edit").
Expand All @@ -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<SpawnResult> {
const track = opts.track ?? true;
const maxTurns = opts.maxTurns ?? DEFAULT_MAX_TURNS;
Expand Down
57 changes: 57 additions & 0 deletions test/extract-touched-files.test.mts
Original file line number Diff line number Diff line change
@@ -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, <<EOF` }),
["/tmp/ms-leg.txt"], "trailing comma stripped");
});

test("#87: quoted redirect targets are unwrapped", () => {
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"]);
});
Loading