From e5af1879a6a21689197c5229c7f6d32d7042943f Mon Sep 17 00:00:00 2001 From: Andrew Churchill Date: Wed, 7 Oct 2026 13:13:16 -0700 Subject: [PATCH 1/4] Add optional files glob to filter check runs Co-Authored-By: Weave Router --- README.md | 15 ++++ src/file-glob.mjs | 179 +++++++++++++++++++++++++++++++++++++++++ src/file-glob.test.mjs | 73 +++++++++++++++++ src/local.mjs | 24 +++++- src/local.test.mjs | 40 +++++++++ src/parse.mjs | 17 +++- src/parse.test.mjs | 48 +++++++++++ src/runner.mjs | 31 +++++-- src/runner.test.mjs | 35 +++++--- src/worker.mjs | 88 +++++++++++++------- src/worker.test.mjs | 63 +++++++++++++++ 11 files changed, 559 insertions(+), 54 deletions(-) create mode 100644 src/file-glob.mjs create mode 100644 src/file-glob.test.mjs diff --git a/README.md b/README.md index 7df55e0..4f5208d 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,21 @@ The Markdown file defining a check must have frontmatter with the following meta - `name`: human-friendly name for the check. - `description`: explains what the check is for in one sentence. - `intelligence`: one of `low`, `medium`, `high`, or `maximum`. This is a cost optimization lever. Warning: any check that uses `high` or `maximum` will be extremely expensive over time if you run it on every single commit! +- `files` (optional): a repository-relative glob. The check runs only when a changed file matches, and its prompt receives only matching files from the diff. For scoped checks, repository-reading and shell tools are disabled so it cannot inspect files outside the matching diff. When no changed files match, the orchestrator skips the check without creating a per-check run. + +Glob syntax: `*` matches within one path segment, `**` matches across directories, `?` matches one character, and `[abc]` or `[!abc]` matches one character from (or not in) a set. Use forward slashes. For example: + +```yaml +--- +name: Frontend accessibility +description: Checks changed frontend code for accessibility issues. +intelligence: medium +files: frontend/** +--- +Look for accessibility regressions in changed frontend code. +``` + +A check without `files` keeps the existing behavior and receives the full review diff. After the frontmatter there is no required structure. However, we would recommend including a couple sections in your file to help the agent: diff --git a/src/file-glob.mjs b/src/file-glob.mjs new file mode 100644 index 0000000..50993e9 --- /dev/null +++ b/src/file-glob.mjs @@ -0,0 +1,179 @@ +// Per-check file glob matching and unified-diff scoping. +// +// Supported glob syntax: `*` matches within one path segment, `**` matches +// across path separators, `?` matches one non-separator character, and +// `[abc]`/`[!abc]` match one character. Paths and patterns use `/` separators. + +function escapeRegex(character) { + return character.replace(/[|\\{}()[\]^$+?.]/g, "\\$&"); +} + +export function validateFileGlob(pattern) { + if (typeof pattern !== "string" || pattern.trim() === "") { + throw new Error("files must be a non-empty glob pattern"); + } + if (pattern.startsWith("/") || pattern.includes("\\")) { + throw new Error("files glob must be repository-relative and use forward slashes"); + } + if (pattern.split("/").includes("..")) { + throw new Error("files glob must not escape the repository root"); + } + let bracketStart = -1; + for (let index = 0; index < pattern.length; index += 1) { + if (pattern[index] === "[") { + if (bracketStart !== -1) throw new Error("files glob has an unclosed character class"); + bracketStart = index; + } else if (pattern[index] === "]") { + if (bracketStart === -1) throw new Error("files glob has an unmatched ]"); + const contents = pattern.slice(bracketStart + 1, index); + if (contents === "" || contents === "!") { + throw new Error("files glob has an empty character class"); + } + bracketStart = -1; + } + } + if (bracketStart !== -1) throw new Error("files glob has an unclosed character class"); + return pattern; +} + +export function compileFileGlob(pattern) { + validateFileGlob(pattern); + let source = "^"; + for (let index = 0; index < pattern.length; index += 1) { + const character = pattern[index]; + if (character === "*") { + if (pattern[index + 1] === "*") { + index += 1; + if (pattern[index + 1] === "/") { + index += 1; + source += "(?:.*/)?"; + } else { + source += ".*"; + } + } else { + source += "[^/]*"; + } + } else if (character === "?") { + source += "[^/]"; + } else if (character === "[") { + const close = pattern.indexOf("]", index + 1); + let chars = pattern.slice(index + 1, close); + if (chars.startsWith("!")) chars = `^${chars.slice(1)}`; + source += `[${chars.replaceAll("\\", "\\\\")}]`; + index = close; + } else { + source += escapeRegex(character); + } + } + return new RegExp(`${source}$`); +} + +export function matchesFileGlob(filePath, pattern) { + return compileFileGlob(pattern).test(filePath); +} + +function decodeGitQuotedPath(value) { + if (!value.startsWith('"')) return value; + let decoded = ""; + for (let index = 1; index < value.length - 1; index += 1) { + const character = value[index]; + if (character !== "\\") { + decoded += character; + continue; + } + const next = value[++index]; + if (/[0-7]/.test(next ?? "")) { + const octal = `${next}${value[index + 1] ?? ""}${value[index + 2] ?? ""}`; + decoded += String.fromCharCode(Number.parseInt(octal, 8)); + index += octal.length - 1; + } else { + decoded += + { a: "\x07", b: "\b", t: "\t", n: "\n", v: "\v", f: "\f", r: "\r", "\\": "\\", '"': '"' }[ + next + ] ?? next; + } + } + return decoded; +} + +function pathFromHeader(line, prefix) { + const decoded = decodeGitQuotedPath(line.trimEnd().slice(prefix.length)); + if (decoded === "/dev/null") return null; + return decoded.startsWith("a/") || decoded.startsWith("b/") ? decoded.slice(2) : decoded; +} + +function pathsFromDiffHeader(line) { + const raw = line.slice("diff --git ".length); + if (raw.startsWith('"')) { + let escaped = false; + let end = 1; + for (; end < raw.length; end += 1) { + if (!escaped && raw[end] === '"') break; + if (!escaped && raw[end] === "\\") escaped = true; + else escaped = false; + } + const oldPath = decodeGitQuotedPath(raw.slice(0, end + 1)); + const newPath = decodeGitQuotedPath(raw.slice(end + 2)); + return [ + oldPath.startsWith("a/") ? oldPath.slice(2) : oldPath, + newPath.startsWith("b/") ? newPath.slice(2) : newPath, + ]; + } + const separator = raw.indexOf(" b/"); + if (separator === -1) return [null, null]; + return [raw.slice(2, separator), raw.slice(separator + 3)]; +} + +function diffSections(diff) { + const sections = []; + let current = null; + for (const line of diff.split(/(?<=\n)/)) { + if (line.startsWith("diff --git ")) { + if (current !== null) sections.push(current); + const [oldPath, newPath] = pathsFromDiffHeader(line.trimEnd()); + current = { lines: [line], oldPath, newPath }; + } else if (current !== null) { + current.lines.push(line); + if (line.startsWith("rename from ")) current.oldPath = pathFromHeader(line, "rename from "); + if (line.startsWith("rename to ")) current.newPath = pathFromHeader(line, "rename to "); + if (line.startsWith("--- ")) current.oldPath = pathFromHeader(line, "--- "); + if (line.startsWith("+++ ")) current.newPath = pathFromHeader(line, "+++ "); + } + } + if (current !== null) sections.push(current); + return sections; +} + +export function diffFilePaths(diff) { + return diffSections(diff) + .map(section => section.newPath ?? section.oldPath) + .filter(Boolean); +} + +function statForSection(section, filePath) { + let additions = 0; + let deletions = 0; + for (const line of section.lines) { + if (line.startsWith("+") && !line.startsWith("+++")) additions += 1; + else if (line.startsWith("-") && !line.startsWith("---")) deletions += 1; + } + const changes = additions + deletions; + return `${filePath} | ${changes} ${"+".repeat(additions)}${"-".repeat(deletions)}\n`; +} + +export function filterDiffByGlob(diff, pattern) { + const matcher = compileFileGlob(pattern); + const sections = diffSections(diff).filter(section => { + const paths = [section.oldPath, section.newPath].filter(Boolean); + return paths.some(filePath => matcher.test(filePath)); + }); + const scopedDiff = sections.map(section => section.lines.join("")).join(""); + const stat = sections + .map(section => statForSection(section, section.newPath ?? section.oldPath)) + .join(""); + return { + diff: scopedDiff, + stat, + paths: sections.map(section => section.newPath ?? section.oldPath), + }; +} diff --git a/src/file-glob.test.mjs b/src/file-glob.test.mjs new file mode 100644 index 0000000..455f951 --- /dev/null +++ b/src/file-glob.test.mjs @@ -0,0 +1,73 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { + diffFilePaths, + filterDiffByGlob, + matchesFileGlob, + validateFileGlob, +} from "./file-glob.mjs"; + +const DIFF = `diff --git a/frontend/App.jsx b/frontend/App.jsx +index 1111111..2222222 100644 +--- a/frontend/App.jsx ++++ b/frontend/App.jsx +@@ -1 +1 @@ +-old ++new +diff --git a/backend/api.js b/backend/api.js +index 3333333..4444444 100644 +--- a/backend/api.js ++++ b/backend/api.js +@@ -1 +1 @@ +-old ++new +`; + +describe("file globs", () => { + it("matches segment wildcards and recursive paths", () => { + assert.equal(matchesFileGlob("frontend/App.jsx", "frontend/**"), true); + assert.equal(matchesFileGlob("frontend/nested/App.jsx", "frontend/**"), true); + assert.equal(matchesFileGlob("backend/api.js", "frontend/**"), false); + assert.equal(matchesFileGlob("frontend/App.jsx", "**/*.jsx"), true); + assert.equal(matchesFileGlob("frontend/App.jsx", "frontend/*.jsx"), true); + assert.equal(matchesFileGlob("frontend/nested/App.jsx", "frontend/*.jsx"), false); + assert.equal(matchesFileGlob("frontend/App.jsx", "frontend/?.jsx"), false); + }); + + it("rejects empty, absolute, escaping, and backslash patterns", () => { + for (const pattern of ["", "/frontend/**", "../**", "frontend\\**", "frontend/[x"]) { + assert.throws(() => validateFileGlob(pattern), /files/); + } + }); + + it("extracts changed files and limits diff and stat to matches", () => { + assert.deepEqual(diffFilePaths(DIFF), ["frontend/App.jsx", "backend/api.js"]); + const scoped = filterDiffByGlob(DIFF, "frontend/**"); + assert.match(scoped.diff, /frontend\/App\.jsx/); + assert.doesNotMatch(scoped.diff, /backend\/api\.js/); + assert.match(scoped.stat, /frontend\/App\.jsx/); + assert.doesNotMatch(scoped.stat, /backend\/api\.js/); + assert.deepEqual(scoped.paths, ["frontend/App.jsx"]); + }); + + it("matches renamed and deleted paths", () => { + const rename = `diff --git a/backend/old.ts b/frontend/new.ts\nsimilarity index 100%\nrename from backend/old.ts\nrename to frontend/new.ts\n`; + assert.deepEqual(diffFilePaths(rename), ["frontend/new.ts"]); + assert.equal(filterDiffByGlob(rename, "frontend/**").diff, rename); + assert.equal(filterDiffByGlob(rename, "backend/**").diff, rename); + + const deletion = `diff --git a/frontend/gone.ts b/frontend/gone.ts\n--- a/frontend/gone.ts\n+++ /dev/null\n`; + assert.deepEqual(diffFilePaths(deletion), ["frontend/gone.ts"]); + assert.equal(filterDiffByGlob(deletion, "frontend/**").diff, deletion); + }); + + it("handles binary diffs and filenames containing spaces", () => { + const binary = `diff --git a/frontend/brand image.png b/frontend/brand image.png\nBinary files a/frontend/brand image.png and b/frontend/brand image.png differ\n`; + assert.deepEqual(diffFilePaths(binary), ["frontend/brand image.png"]); + assert.equal(filterDiffByGlob(binary, "frontend/**").diff, binary); + + const quoted = `diff --git "a/frontend/brand image.png" "b/frontend/brand image.png"\nBinary files a/frontend/brand image.png and b/frontend/brand image.png differ\n`; + assert.deepEqual(diffFilePaths(quoted), ["frontend/brand image.png"]); + }); +}); diff --git a/src/local.mjs b/src/local.mjs index 9d6d387..747d128 100644 --- a/src/local.mjs +++ b/src/local.mjs @@ -36,6 +36,7 @@ import process from "node:process"; import { pathToFileURL } from "node:url"; import { OUTCOME, RESULT_SCHEMA, parseAddedLines, publicOutcome } from "./parse.mjs"; +import { filterDiffByGlob } from "./file-glob.mjs"; import { PROVIDER, createProvider, parseProviderEnv } from "./provider.mjs"; import { evaluateCheck, @@ -101,6 +102,23 @@ export function positiveInteger(value, fallback) { export async function runChecks(config, { evaluate = evaluateCheck } = {}) { const addedLines = parseAddedLines(config.diff); const checkResults = await runPool(config.checks, config.parallel, async check => { + const scoped = + check.files === undefined ? + { diff: config.diff, stat: config.stat } + : filterDiffByGlob(config.diff, check.files); + if (check.files !== undefined && scoped.diff === "") { + const checkResult = { + outcome: OUTCOME.PASS, + reason: `No changed files match ${check.files}.`, + cost: 0, + duration: 0, + }; + writeFileSync( + resultPath(config.tempDir, check.slug, "result.json"), + `${JSON.stringify(checkResult, null, 2)}\n`, + ); + return summarizeCheck(check, checkResult); + } process.stderr.write(`Weave Checks: running ${check.slug}...\n`); const settingsPath = config.settingsDir === null ? @@ -119,9 +137,9 @@ export async function runChecks(config, { evaluate = evaluateCheck } = {}) { check, repoDir: config.repoDir, tempDir: config.tempDir, - diff: config.diff, - stat: config.stat, - addedLines, + diff: scoped.diff, + stat: scoped.stat, + addedLines: check.files === undefined ? addedLines : parseAddedLines(scoped.diff), schemaText: config.schemaText, // No history: a local run has no review threads to compare against. // diff --git a/src/local.test.mjs b/src/local.test.mjs index c89df1b..9cf7a57 100644 --- a/src/local.test.mjs +++ b/src/local.test.mjs @@ -269,6 +269,46 @@ describe("runChecks", () => { assert.equal(artifact.reason, "line one is bad"); }); + it("skips checks without matching changed files and scopes matching diff and stat", async () => { + const { env, tempDir } = fixture(); + const config = readConfig(env); + const backendDiff = [ + DIFF.trimEnd(), + "diff --git a/backend/api.go b/backend/api.go", + "--- a/backend/api.go", + "+++ b/backend/api.go", + "@@ -0,0 +1 @@", + "+backend change", + "", + ].join("\n"); + config.diff = backendDiff; + config.stat = "app/main.go | 2 ++\nbackend/api.go | 1 +"; + config.checks = [ + { ...CHECKS[0], files: "app/**" }, + { ...CHECKS[1], files: "db/**" }, + ]; + const seen = []; + + const summary = await runChecks(config, { + evaluate: async ({ check, diff, stat }) => { + seen.push(check.slug); + assert.match(diff, /app\/main\.go/); + assert.doesNotMatch(diff, /backend\/api\.go|backend change/); + assert.match(stat, /app\/main\.go/); + assert.doesNotMatch(stat, /backend\/api\.go/); + return passResult("scoped"); + }, + }); + + assert.deepEqual(seen, ["first-check"]); + assert.equal(summary.checks[1].outcome, "pass"); + assert.match(summary.checks[1].reason, /No changed files match db\/\*\*/); + assert.equal( + JSON.parse(readFileSync(path.join(tempDir, "second-check.result.json"), "utf8")).cost, + 0, + ); + }); + it("uses each check's intelligence as the generated settings filename", async () => { const { env, tempDir } = fixture(); env.SETTINGS_DIR = path.join(tempDir, "settings"); diff --git a/src/parse.mjs b/src/parse.mjs index 8261291..e7b4cc1 100644 --- a/src/parse.mjs +++ b/src/parse.mjs @@ -2,6 +2,8 @@ import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import path from "node:path"; +import { compileFileGlob } from "./file-glob.mjs"; + // Check-file parsing, diff parsing, and the fixed vocabularies Weave Checks // shares between the GitHub action, the worker, and the local CLI. // @@ -77,7 +79,7 @@ const FRONTMATTER = /^---\n([\s\S]*?)\n---\n?/; // misreading valid-but-unsupported YAML. // Keys the contract permits in a check's frontmatter. Anything else is // rejected so a misspelled or made-up field fails discovery loudly. -const FRONTMATTER_KEYS = new Set(["name", "description", "intelligence"]); +const FRONTMATTER_KEYS = new Set(["name", "description", "intelligence", "files"]); function parseFrontmatter(text) { const match = FRONTMATTER.exec(text); @@ -105,7 +107,7 @@ function parseFrontmatter(text) { `unknown frontmatter key "${key}" (allowed: ${[...FRONTMATTER_KEYS].sort().join(", ")})`, ); } - if (/^["'[{>|&*!%@`]/.test(value) || /(?:^|\s)#/.test(value)) { + if ((key !== "files" && /^["'[{>|&*!%@`]/.test(value)) || /(?:^|\s)#/.test(value)) { throw new Error( `${rawLine}: frontmatter values must be unquoted single-line scalars without comments or YAML structures`, ); @@ -235,6 +237,15 @@ export function parseCheckFile(text, filePath, policy = WEAVE_POLICY) { ); } + const files = fields.get("files"); + if (files !== undefined) { + try { + compileFileGlob(files); + } catch (error) { + throw new Error(`${filePath}: invalid files glob: ${error.message}`); + } + } + if (body.trim() === "") { throw new Error(`${filePath}: check body is empty`); } @@ -244,6 +255,7 @@ export function parseCheckFile(text, filePath, policy = WEAVE_POLICY) { name: fields.get("name"), description: fields.get("description"), intelligence, + ...(files === undefined ? {} : { files }), model: modelForIntelligence(intelligence), cluster: intelligence, path: filePath, @@ -627,6 +639,7 @@ export function checkSetDigest(checks, repoDir = ".") { slug: check.slug, intelligence: check.intelligence, model: check.model, + files: check.files ?? null, criteria: readFileSync(check.criteriaPath ?? path.join(repoDir, check.path), "utf8"), })) .sort((a, b) => diff --git a/src/parse.test.mjs b/src/parse.test.mjs index 6c46a71..99ee61f 100644 --- a/src/parse.test.mjs +++ b/src/parse.test.mjs @@ -62,9 +62,50 @@ describe("parseCheckFile", () => { assert.equal(check.intelligence, "low"); assert.equal(check.model, "haiku"); assert.equal(check.cluster, "low"); + assert.equal(check.files, undefined); assert.equal(check.body.trim(), "Body text."); }); + it("retains a validated file glob in the check matrix", () => { + const check = parseCheckFile( + VALID.replace("intelligence: low", "intelligence: low\nfiles: frontend/**"), + "checks/frontend.md", + ); + assert.equal(check.files, "frontend/**"); + const [entry] = buildMatrix([ + file( + VALID.replace("intelligence: low", "intelligence: low\nfiles: frontend/**"), + "checks/frontend.md", + ), + ]); + assert.equal(entry.files, "frontend/**"); + }); + + it("allows glob syntax that resembles YAML structures in the files field", () => { + assert.equal( + parseCheckFile( + VALID.replace("intelligence: low", "intelligence: low\nfiles: *.go"), + "checks/x.md", + ).files, + "*.go", + ); + }); + + it("rejects unsafe files globs", () => { + for (const pattern of [ + "", + "/frontend/**", + "../**", + "frontend\\\\**", + "frontend/[x", + "frontend/[]", + "frontend/[z-a].js", + ]) { + const text = VALID.replace("intelligence: low", `intelligence: low\nfiles: ${pattern}`); + assert.throws(() => parseCheckFile(text, "checks/scoped.md"), /invalid files glob/); + } + }); + it("maps each intelligence tier to a rolling model alias", () => { assert.deepEqual(MODEL_FOR_INTELLIGENCE, { low: "haiku", @@ -861,6 +902,13 @@ describe("checkSetDigest", () => { ), digest, ); + assert.notEqual( + checkSetDigest( + checks.map((check, index) => (index === 0 ? { ...check, files: "src/**" } : check)), + dir, + ), + digest, + ); } finally { fs.rmSync(dir, { recursive: true, force: true }); } diff --git a/src/runner.mjs b/src/runner.mjs index 96eb46d..52b5637 100644 --- a/src/runner.mjs +++ b/src/runner.mjs @@ -34,9 +34,11 @@ import { OUTCOME, VERDICT, interpretResult, + parseAddedLines, parseStructuredOutput, validateResult, } from "./parse.mjs"; +import { filterDiffByGlob } from "./file-glob.mjs"; import { DEFAULT_BRANDING } from "./branding.mjs"; import { childEnvironment } from "./provider.mjs"; import { splitStreamJson } from "./streamsplit.mjs"; @@ -85,6 +87,7 @@ export async function runClaude({ settingsPath = null, dropEnv = [], onCostError = null, + toolsDisabled = false, }) { const promptPath = resultPath(tempDir, slug, `${suffix}.prompt.txt`); const transcriptPath = resultPath(tempDir, slug, `${suffix}.transcript.jsonl`); @@ -112,12 +115,9 @@ export async function runClaude({ schemaText, "--permission-mode", "dontAsk", - "--allowedTools", - "Read", - "Glob", - "Grep", - "Bash(git diff *)", - "Bash(git show *)", + ...(toolsDisabled ? + ["--tools", "", "--strict-mcp-config"] + : ["--allowedTools", "Read", "Glob", "Grep", "Bash(git diff *)", "Bash(git show *)"]), ...(settingSources === null ? [] : ["--setting-sources", settingSources]), // `--settings` takes highest precedence and is merged over whatever // `--setting-sources` loaded. Local may pass a generated file here (a @@ -417,8 +417,15 @@ export function promptFor( `You are running the advisory ${productName} "${check.name}" on a pull request.`, "", "Review ONLY changed lines in the diff below.", - "- Read adjacent repository files only when needed to judge a changed line.", - "- Do NOT modify files. You have read-only tools.", + ...(check.files === undefined ? + [ + "- Read adjacent repository files only when needed to judge a changed line.", + "- Do NOT modify files. You have read-only tools.", + ] + : [ + `- This check is scoped to ${check.files}; the supplied diff and history include only matching files.`, + "- You have no repository-reading or shell tools; do not request or infer content from other files.", + ]), "- Do NOT report pre-existing issues this diff did not introduce.", "- Provide literal replacements only for lines present in the diff.", "- Set PASS when the changed lines do not violate the criteria.", @@ -501,6 +508,13 @@ export async function evaluateCheck({ onAttempt, onCostError, }) { + if (check.files !== undefined) { + const scoped = filterDiffByGlob(diff, check.files); + diff = scoped.diff; + stat = scoped.stat; + addedLines = parseAddedLines(diff); + } + const invoke = () => runClaude({ slug: check.slug, @@ -516,6 +530,7 @@ export async function evaluateCheck({ settingsPath, dropEnv, onCostError, + toolsDisabled: check.files !== undefined, }); const invocation = await invoke(); diff --git a/src/runner.test.mjs b/src/runner.test.mjs index 9e578e1..187dc64 100644 --- a/src/runner.test.mjs +++ b/src/runner.test.mjs @@ -739,20 +739,33 @@ describe("evaluateCheck", () => { assert.equal(result.duration, 2600); }); - it("does not retry a verdict that came back cleanly", async () => { + it("scopes filtered checks to matching diff files and disables repository tools", async () => { const space = workspace(); - space.stream( - successEvents({ - verdict: VERDICT.FAIL, - reason: "line one is bad", - suggestions: [{ file: "app/main.go", line: 1, comment: "rename this" }], - }), - ); + space.stream(successEvents({ verdict: VERDICT.PASS, reason: "scoped review" })); + const backendDiff = [ + DIFF.trimEnd(), + "diff --git a/backend/api.go b/backend/api.go", + "--- a/backend/api.go", + "+++ b/backend/api.go", + "@@ -0,0 +1 @@", + "+secret backend change", + "", + ].join("\n"); - const result = await evaluate(space); + await evaluate(space, { + check: { ...CHECK, files: "app/**" }, + diff: backendDiff, + stat: "app/main.go | 2 ++\\nbackend/api.go | 1 +", + }); - assert.equal(result.outcome, OUTCOME.FAIL); - assert.equal(space.callCount(), 1); + assert.match(space.stdin(), /app\/main\.go/); + assert.doesNotMatch(space.stdin(), /backend\/api\.go|secret backend change/); + assert.match(space.stdin(), /app\/main\.go \| 2 \+\+/); + const argv = space.argv(); + assert.ok(argv.includes("--tools")); + assert.equal(argv[argv.indexOf("--tools") + 1], ""); + assert.ok(argv.includes("--strict-mcp-config")); + assert.ok(!argv.includes("--allowedTools")); }); }); diff --git a/src/worker.mjs b/src/worker.mjs index 96958ed..19d57b9 100644 --- a/src/worker.mjs +++ b/src/worker.mjs @@ -70,6 +70,7 @@ import { validateResolutions, } from "./history.mjs"; import { isRetryableGraphQLBody, requestWithRetry, truncateBody } from "./githubapi.mjs"; +import { filterDiffByGlob, matchesFileGlob } from "./file-glob.mjs"; import { PROVIDER, createProvider, parseProviderEnv } from "./provider.mjs"; // The agent invocation, verdict pipeline, and cost/duration arithmetic are // shared with the local runner (local.mjs, behind `weave-checks run`) so the @@ -220,11 +221,19 @@ export async function runWorker(config, deps = {}) { const MARKERS = markerCodec(BRANDING.markerPrefix); const IGNORE_FILE = `${config.checksDir}/.ignore`; const CHECK_DIGEST = checkSetDigest(CHECKS, REPO_DIR); - // Bounds the review agent's suggestions: it only saw DIFF, so it may only - // comment on DIFF's lines. - const ADDED_LINES = parseAddedLines(DIFF); - // Evidence for the resolution judge. PR scope, never review scope. - const FULL_ADDED_LINES = parseAddedLines(FULL_DIFF); + const CHECK_SCOPES = new Map( + CHECKS.map(check => { + if (check.files === undefined) { + return [check.slug, { diff: DIFF, stat: STAT, fullDiff: FULL_DIFF, fullStat: FULL_STAT }]; + } + const review = filterDiffByGlob(DIFF, check.files); + const full = filterDiffByGlob(FULL_DIFF, check.files); + return [ + check.slug, + { diff: review.diff, stat: review.stat, fullDiff: full.diff, fullStat: full.stat }, + ]; + }), + ); mkdirSync(TEMP_DIR, { recursive: true }); @@ -397,19 +406,23 @@ export async function runWorker(config, deps = {}) { // check starts queued; the pool below takes them in order. function buildStates() { return new Map( - CHECKS.map(check => [ - check.slug, - { - check, - status: STATUS.QUEUED, - outcome: null, - cause: null, - note: "Waiting for an agent slot.", - checkRunId: null, - cost: 0, - durationMs: 0, - }, - ]), + CHECKS.map(check => { + const scope = CHECK_SCOPES.get(check.slug); + const skipped = check.files !== undefined && scope.diff === ""; + return [ + check.slug, + { + check, + status: skipped ? STATUS.COMPLETE : STATUS.QUEUED, + outcome: skipped ? OUTCOME.PASS : null, + cause: null, + note: skipped ? `No changed files match ${check.files}.` : "Waiting for an agent slot.", + checkRunId: null, + cost: 0, + durationMs: 0, + }, + ]; + }), ); } @@ -461,14 +474,18 @@ export async function runWorker(config, deps = {}) { return runAgent({ ...options, ...agentEnvironment() }); } - function resolutionPromptFor(check, openThreads) { + function resolutionPromptFor(check, openThreads, scope) { const criteria = readFileSync(check.criteriaPath ?? path.join(REPO_DIR, check.path), "utf8"); return [ `You are resolving previously-flagged findings for the ${BRANDING.productName} "${check.name}" on a pull request.`, "", "For each previously-flagged issue below, decide whether it STILL APPLIES at the PR's current HEAD.", - "Read the current file at the flagged location if you need to. Read adjacent files only when needed.", - "Do NOT modify files. You have read-only tools.", + check.files === undefined ? + "Read the current file at the flagged location if you need to. Read adjacent files only when needed." + : `This check is scoped to ${check.files}; only matching changed files and threads are included, and no repository-reading tools are available.`, + check.files === undefined ? + "Do NOT modify files. You have read-only tools." + : "Do NOT modify files.", "", "Set `resolved` to true ONLY in these cases:", "- The flagged code has been changed and now satisfies the check criteria.", @@ -495,18 +512,18 @@ export async function runWorker(config, deps = {}) { // still applies to the PR as a whole, so it must see every file the PR // touches. Handing it the incremental diff would make each file the // latest push skipped look like it had left the PR. - formatResolutionSection(openThreads, FULL_ADDED_LINES), + formatResolutionSection(openThreads, parseAddedLines(scope.fullDiff)), "", "## Changed files", "", "```", - FULL_STAT, + scope.fullStat, "```", "", "## Diff", "", "```diff", - FULL_DIFF, + scope.fullDiff, "```", ].join("\n"); } @@ -554,6 +571,7 @@ export async function runWorker(config, deps = {}) { // summary. async function judgeResolutions(check, openThreads, recordAttempt) { if (openThreads.length === 0) return { resolutions: [], error: null, cost: 0, duration: 0 }; + const scope = CHECK_SCOPES.get(check.slug); const invocation = await runCheckAgent({ slug: check.slug, @@ -561,7 +579,8 @@ export async function runWorker(config, deps = {}) { model: check.model, cluster: check.cluster, schemaText: JSON.stringify(RESOLUTION_SCHEMA), - promptText: resolutionPromptFor(check, openThreads), + promptText: resolutionPromptFor(check, openThreads, scope), + toolsDisabled: check.files !== undefined, }); recordAttempt("Resolution judge", invocation); const decoded = decodeAgentInvocation(invocation, { @@ -608,6 +627,7 @@ export async function runWorker(config, deps = {}) { cluster: DEDUP_CLUSTER, schemaText: JSON.stringify(DEDUP_SCHEMA), promptText: dedupPromptFor(check, checkThreads, accepted), + toolsDisabled: check.files !== undefined, }); recordAttempt("Dedup judge", invocation); const decoded = decodeAgentInvocation(invocation, { @@ -1093,7 +1113,12 @@ export async function runWorker(config, deps = {}) { state.status = STATUS.RUNNING; state.note = `${check.description} Agent is reviewing changed lines.`; - const checkThreads = threadsForCheck(rawThreadNodes, check.slug, MARKERS); + const checkScope = CHECK_SCOPES.get(check.slug); + const checkThreads = threadsForCheck(rawThreadNodes, check.slug, MARKERS).filter( + thread => + check.files === undefined || + (thread.path !== null && matchesFileGlob(thread.path, check.files)), + ); const openThreads = checkThreads.filter(thread => !isSettled(thread)); // Per-phase transcript evidence for the check-run summary's collapsible @@ -1149,9 +1174,9 @@ export async function runWorker(config, deps = {}) { [initialResult, judgment] = await Promise.all([ evaluate({ check, - diff: DIFF, - stat: STAT, - addedLines: ADDED_LINES, + diff: checkScope.diff, + stat: checkScope.stat, + addedLines: parseAddedLines(checkScope.diff), schemaText: SCHEMA, historySection: formatHistorySection(checkThreads), productName: BRANDING.productName, @@ -1374,7 +1399,10 @@ export async function runWorker(config, deps = {}) { states = buildStates(); await updateMasterBestEffort(); - const rawThreadNodes = await fetchAllReviewThreads(); + const rawThreadNodes = + [...states.values()].some(state => state.status === STATUS.QUEUED) ? + await fetchAllReviewThreads() + : []; await runQueuedChecks(rawThreadNodes); await updateMaster(true); diff --git a/src/worker.test.mjs b/src/worker.test.mjs index 613b24c..5c3b107 100644 --- a/src/worker.test.mjs +++ b/src/worker.test.mjs @@ -203,6 +203,69 @@ async function run(env, github, evaluate, extraDeps = {}) { }); } +describe("runWorker file-glob scoping", () => { + it("runs only matching checks and gives them scoped review diffs", async () => { + const fullDiff = [ + DIFF.trimEnd(), + "diff --git a/backend/api.go b/backend/api.go", + "--- a/backend/api.go", + "+++ b/backend/api.go", + "@@ -0,0 +1 @@", + "+backend change", + "", + ].join("\n"); + const matrix = [ + { ...CHECKS[0], files: "app/**" }, + { ...CHECKS[1], files: "db/**" }, + ]; + const { env } = workerEnv({}, { diff: fullDiff }); + writeFileSync(env.MATRIX_PATH, JSON.stringify({ check: matrix })); + writeFileSync(env.FULL_DIFF_PATH, fullDiff); + writeFileSync(env.STAT_PATH, "app/main.go | 2 ++\nbackend/api.go | 1 +"); + writeFileSync(env.FULL_STAT_PATH, "app/main.go | 2 ++\nbackend/api.go | 1 +"); + const github = fakeGitHub(); + const seen = []; + + const result = await run(env, github, async ({ check, diff, stat }) => { + seen.push(check.slug); + assert.match(diff, /app\/main\.go/); + assert.doesNotMatch(diff, /backend\/api\.go|backend change/); + assert.match(stat, /app\/main\.go/); + assert.doesNotMatch(stat, /backend\/api\.go/); + return pass(); + }); + + assert.equal(result.ok, true); + assert.deepEqual(seen, ["first-check"]); + assert.equal(github.childRuns().length, 1); + const results = JSON.parse(readFileSync(env.RESULTS_PATH, "utf8")); + assert.equal(results.checks.length, 2); + assert.match( + results.checks.find(check => check.slug === "second-check").detail, + /No changed files match db\/\*\*/, + ); + }); + + it("skips thread lookup when no scoped check matches the diff", async () => { + const matrix = CHECKS.map(check => ({ ...check, files: "db/**" })); + const { env } = workerEnv(); + writeFileSync(env.MATRIX_PATH, JSON.stringify({ check: matrix })); + const github = fakeGitHub(); + + const result = await run(env, github, async () => { + throw new Error("a non-matching check must not run"); + }); + + assert.equal(result.ok, true); + assert.equal(github.childRuns().length, 0); + assert.equal( + github.calls.some(call => call.route === "graphql"), + false, + ); + assert.equal(github.masterPatches().at(-1).body.conclusion, "success"); + }); +}); + describe("readWorkerConfig", () => { it("defaults to the weave-router provider and requires its key", () => { const { env } = workerEnv({ WEAVE_ROUTER_KEY: "rk" }); From 30f42a2649be76c47bdee9c23ea5972c9e19ef8e Mon Sep 17 00:00:00 2001 From: Andrew Churchill Date: Wed, 7 Oct 2026 13:16:48 -0700 Subject: [PATCH 2/4] fix: remove unused diff path helper Fixed: - Remove diffFilePaths, which had no production callers - Keep diff filtering coverage focused on active behavior Co-Authored-By: Weave Router --- src/file-glob.mjs | 6 ------ src/file-glob.test.mjs | 15 +++------------ 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/src/file-glob.mjs b/src/file-glob.mjs index 50993e9..6850d49 100644 --- a/src/file-glob.mjs +++ b/src/file-glob.mjs @@ -144,12 +144,6 @@ function diffSections(diff) { return sections; } -export function diffFilePaths(diff) { - return diffSections(diff) - .map(section => section.newPath ?? section.oldPath) - .filter(Boolean); -} - function statForSection(section, filePath) { let additions = 0; let deletions = 0; diff --git a/src/file-glob.test.mjs b/src/file-glob.test.mjs index 455f951..a15da22 100644 --- a/src/file-glob.test.mjs +++ b/src/file-glob.test.mjs @@ -1,12 +1,7 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; -import { - diffFilePaths, - filterDiffByGlob, - matchesFileGlob, - validateFileGlob, -} from "./file-glob.mjs"; +import { filterDiffByGlob, matchesFileGlob, validateFileGlob } from "./file-glob.mjs"; const DIFF = `diff --git a/frontend/App.jsx b/frontend/App.jsx index 1111111..2222222 100644 @@ -41,8 +36,7 @@ describe("file globs", () => { } }); - it("extracts changed files and limits diff and stat to matches", () => { - assert.deepEqual(diffFilePaths(DIFF), ["frontend/App.jsx", "backend/api.js"]); + it("limits diff and stat to matches", () => { const scoped = filterDiffByGlob(DIFF, "frontend/**"); assert.match(scoped.diff, /frontend\/App\.jsx/); assert.doesNotMatch(scoped.diff, /backend\/api\.js/); @@ -53,21 +47,18 @@ describe("file globs", () => { it("matches renamed and deleted paths", () => { const rename = `diff --git a/backend/old.ts b/frontend/new.ts\nsimilarity index 100%\nrename from backend/old.ts\nrename to frontend/new.ts\n`; - assert.deepEqual(diffFilePaths(rename), ["frontend/new.ts"]); assert.equal(filterDiffByGlob(rename, "frontend/**").diff, rename); assert.equal(filterDiffByGlob(rename, "backend/**").diff, rename); const deletion = `diff --git a/frontend/gone.ts b/frontend/gone.ts\n--- a/frontend/gone.ts\n+++ /dev/null\n`; - assert.deepEqual(diffFilePaths(deletion), ["frontend/gone.ts"]); assert.equal(filterDiffByGlob(deletion, "frontend/**").diff, deletion); }); it("handles binary diffs and filenames containing spaces", () => { const binary = `diff --git a/frontend/brand image.png b/frontend/brand image.png\nBinary files a/frontend/brand image.png and b/frontend/brand image.png differ\n`; - assert.deepEqual(diffFilePaths(binary), ["frontend/brand image.png"]); assert.equal(filterDiffByGlob(binary, "frontend/**").diff, binary); const quoted = `diff --git "a/frontend/brand image.png" "b/frontend/brand image.png"\nBinary files a/frontend/brand image.png and b/frontend/brand image.png differ\n`; - assert.deepEqual(diffFilePaths(quoted), ["frontend/brand image.png"]); + assert.equal(filterDiffByGlob(quoted, "frontend/**").diff, quoted); }); }); From 2f30c72aa7c16771a6a0f4eba81e2f83be8e668a Mon Sep 17 00:00:00 2001 From: Andrew Churchill Date: Wed, 7 Oct 2026 13:31:43 -0700 Subject: [PATCH 3/4] fix: handle skipped scoped checks with open findings Fixed: - Resolve prior findings even when a scoped check has no new matching files - Reject quoted file globs unsupported by the plain-scalar parser Co-Authored-By: Weave Router --- src/parse.mjs | 5 ++- src/parse.test.mjs | 10 ++++++ src/worker.mjs | 49 +++++++++++++++++++--------- src/worker.test.mjs | 79 +++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 124 insertions(+), 19 deletions(-) diff --git a/src/parse.mjs b/src/parse.mjs index e7b4cc1..3f61461 100644 --- a/src/parse.mjs +++ b/src/parse.mjs @@ -107,7 +107,10 @@ function parseFrontmatter(text) { `unknown frontmatter key "${key}" (allowed: ${[...FRONTMATTER_KEYS].sort().join(", ")})`, ); } - if ((key !== "files" && /^["'[{>|&*!%@`]/.test(value)) || /(?:^|\s)#/.test(value)) { + if ( + (key === "files" ? /^["']/.test(value) : /^["'[{>|&*!%@`]/.test(value)) || + /(?:^|\s)#/.test(value) + ) { throw new Error( `${rawLine}: frontmatter values must be unquoted single-line scalars without comments or YAML structures`, ); diff --git a/src/parse.test.mjs b/src/parse.test.mjs index 99ee61f..11bcd08 100644 --- a/src/parse.test.mjs +++ b/src/parse.test.mjs @@ -91,6 +91,16 @@ describe("parseCheckFile", () => { ); }); + it("rejects quoted files globs instead of treating quotes as pattern characters", () => { + for (const pattern of ['"frontend/**"', "'frontend/**'"]) { + const text = VALID.replace("intelligence: low", `intelligence: low\nfiles: ${pattern}`); + assert.throws( + () => parseCheckFile(text, "checks/scoped.md"), + /frontmatter values must be unquoted/, + ); + } + }); + it("rejects unsafe files globs", () => { for (const pattern of [ "", diff --git a/src/worker.mjs b/src/worker.mjs index 19d57b9..c81f774 100644 --- a/src/worker.mjs +++ b/src/worker.mjs @@ -1172,17 +1172,28 @@ export async function runWorker(config, deps = {}) { // no extra wall-clock and means a resolution still lands even if the // main agent's run this time comes back neutral. [initialResult, judgment] = await Promise.all([ - evaluate({ - check, - diff: checkScope.diff, - stat: checkScope.stat, - addedLines: parseAddedLines(checkScope.diff), - schemaText: SCHEMA, - historySection: formatHistorySection(checkThreads), - productName: BRANDING.productName, - onAttempt: (label, invocation) => recordAttempt(TRANSCRIPT_PHASE.MAIN, label, invocation), - ...agentEnvironment(), - }), + checkScope.diff === "" ? + Promise.resolve({ + outcome: OUTCOME.PASS, + reason: `No changed files match ${check.files}.`, + accepted: [], + proseFallbacks: [], + rejected: [], + cost: 0, + duration: 0, + }) + : evaluate({ + check, + diff: checkScope.diff, + stat: checkScope.stat, + addedLines: parseAddedLines(checkScope.diff), + schemaText: SCHEMA, + historySection: formatHistorySection(checkThreads), + productName: BRANDING.productName, + onAttempt: (label, invocation) => + recordAttempt(TRANSCRIPT_PHASE.MAIN, label, invocation), + ...agentEnvironment(), + }), judgeResolutions(check, openThreads, (label, invocation) => recordAttempt(TRANSCRIPT_PHASE.RESOLVE, label, invocation), ), @@ -1399,10 +1410,18 @@ export async function runWorker(config, deps = {}) { states = buildStates(); await updateMasterBestEffort(); - const rawThreadNodes = - [...states.values()].some(state => state.status === STATUS.QUEUED) ? - await fetchAllReviewThreads() - : []; + const rawThreadNodes = await fetchAllReviewThreads(); + for (const state of states.values()) { + if (state.status !== STATUS.COMPLETE || state.check.files === undefined) continue; + const hasOpenThreads = threadsForCheck(rawThreadNodes, state.check.slug, MARKERS) + .filter(thread => thread.path !== null && matchesFileGlob(thread.path, state.check.files)) + .some(thread => !isSettled(thread)); + if (hasOpenThreads) { + state.status = STATUS.QUEUED; + state.outcome = null; + state.note = "Waiting to resolve previously-flagged findings."; + } + } await runQueuedChecks(rawThreadNodes); await updateMaster(true); diff --git a/src/worker.test.mjs b/src/worker.test.mjs index 5c3b107..8a4e6fc 100644 --- a/src/worker.test.mjs +++ b/src/worker.test.mjs @@ -246,7 +246,7 @@ describe("runWorker file-glob scoping", () => { ); }); - it("skips thread lookup when no scoped check matches the diff", async () => { + it("fetches history before skipping non-matching checks", async () => { const matrix = CHECKS.map(check => ({ ...check, files: "db/**" })); const { env } = workerEnv(); writeFileSync(env.MATRIX_PATH, JSON.stringify({ check: matrix })); @@ -259,11 +259,84 @@ describe("runWorker file-glob scoping", () => { assert.equal(result.ok, true); assert.equal(github.childRuns().length, 0); assert.equal( - github.calls.some(call => call.route === "graphql"), - false, + github.calls.some( + call => call.route === "graphql" && call.body.query.includes("reviewThreads"), + ), + true, ); assert.equal(github.masterPatches().at(-1).body.conclusion, "success"); }); + + it("resolves matching open threads when no scoped files changed", async () => { + const matrix = CHECKS.map(check => ({ ...check, files: "db/**" })); + const { env } = workerEnv(); + writeFileSync(env.MATRIX_PATH, JSON.stringify({ check: matrix })); + const thread = { + id: "T-open", + isResolved: false, + isOutdated: false, + path: "db/tables.sql", + line: 1, + comments: { + nodes: [ + { + databaseId: 55, + body: appendMarker("Rename `one`.", "second-check"), + pullRequestReview: { + databaseId: 77, + id: "R77", + state: "COMMENTED", + isMinimized: false, + }, + }, + ], + }, + }; + const github = fakeGitHub({ threads: [thread] }); + const judged = []; + + await run( + env, + github, + async () => { + throw new Error("empty scoped diffs should not run a main review"); + }, + { + runAgent: async options => { + judged.push({ suffix: options.suffix, prompt: options.promptText }); + return { + code: 0, + sessionId: "judge-session", + cost: 0.01, + costLabel: "client-reported cost", + transcript: [], + cli: { + subtype: "success", + is_error: false, + structured_output: { + resolutions: [ + { thread_id: "T-open", resolved: true, evidence: "No longer applies." }, + ], + }, + duration_ms: 10, + }, + }; + }, + }, + ); + + assert.deepEqual( + judged.map(({ suffix }) => suffix), + ["resolve"], + ); + assert.match(judged[0].prompt, /\[id=T-open\]/); + assert.equal(github.childRuns().length, 1); + assert.ok( + github.calls.some( + call => call.route === "graphql" && call.body.query.includes("resolveReviewThread"), + ), + ); + }); }); describe("readWorkerConfig", () => { From 651f03ceea6f4ee1a057d2a7f64fa65b5fd88bc4 Mon Sep 17 00:00:00 2001 From: Andrew Churchill Date: Wed, 7 Oct 2026 13:38:08 -0700 Subject: [PATCH 4/4] fix: preserve file scope across diff metadata Fixed: - Ignore header-like content inside diff hunks when parsing file paths - Preserve directory prefixes in rename metadata - Clarify the judge invocation test variable name Co-Authored-By: Weave Router --- src/file-glob.mjs | 19 +++++++++++++------ src/file-glob.test.mjs | 10 ++++++++++ src/worker.test.mjs | 8 ++++---- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/file-glob.mjs b/src/file-glob.mjs index 6850d49..b7fdfc3 100644 --- a/src/file-glob.mjs +++ b/src/file-glob.mjs @@ -96,10 +96,12 @@ function decodeGitQuotedPath(value) { return decoded; } -function pathFromHeader(line, prefix) { +function pathFromHeader(line, prefix, stripDiffPrefix = false) { const decoded = decodeGitQuotedPath(line.trimEnd().slice(prefix.length)); if (decoded === "/dev/null") return null; - return decoded.startsWith("a/") || decoded.startsWith("b/") ? decoded.slice(2) : decoded; + return stripDiffPrefix && (decoded.startsWith("a/") || decoded.startsWith("b/")) ? + decoded.slice(2) + : decoded; } function pathsFromDiffHeader(line) { @@ -127,17 +129,22 @@ function pathsFromDiffHeader(line) { function diffSections(diff) { const sections = []; let current = null; + let inHunk = false; for (const line of diff.split(/(?<=\n)/)) { if (line.startsWith("diff --git ")) { if (current !== null) sections.push(current); const [oldPath, newPath] = pathsFromDiffHeader(line.trimEnd()); current = { lines: [line], oldPath, newPath }; + inHunk = false; } else if (current !== null) { current.lines.push(line); - if (line.startsWith("rename from ")) current.oldPath = pathFromHeader(line, "rename from "); - if (line.startsWith("rename to ")) current.newPath = pathFromHeader(line, "rename to "); - if (line.startsWith("--- ")) current.oldPath = pathFromHeader(line, "--- "); - if (line.startsWith("+++ ")) current.newPath = pathFromHeader(line, "+++ "); + if (!inHunk) { + if (line.startsWith("rename from ")) current.oldPath = pathFromHeader(line, "rename from "); + if (line.startsWith("rename to ")) current.newPath = pathFromHeader(line, "rename to "); + if (line.startsWith("--- ")) current.oldPath = pathFromHeader(line, "--- ", true); + if (line.startsWith("+++ ")) current.newPath = pathFromHeader(line, "+++ ", true); + } + if (line.startsWith("@@")) inHunk = true; } } if (current !== null) sections.push(current); diff --git a/src/file-glob.test.mjs b/src/file-glob.test.mjs index a15da22..6e6614a 100644 --- a/src/file-glob.test.mjs +++ b/src/file-glob.test.mjs @@ -50,10 +50,20 @@ describe("file globs", () => { assert.equal(filterDiffByGlob(rename, "frontend/**").diff, rename); assert.equal(filterDiffByGlob(rename, "backend/**").diff, rename); + const sameDirectoryRename = `diff --git a/a/old.ts b/a/new.ts\nsimilarity index 100%\nrename from a/old.ts\nrename to a/new.ts\n`; + assert.equal(filterDiffByGlob(sameDirectoryRename, "a/**").diff, sameDirectoryRename); + assert.equal(filterDiffByGlob(sameDirectoryRename, "old/**").diff, ""); + const deletion = `diff --git a/frontend/gone.ts b/frontend/gone.ts\n--- a/frontend/gone.ts\n+++ /dev/null\n`; assert.equal(filterDiffByGlob(deletion, "frontend/**").diff, deletion); }); + it("does not treat diff-header-like hunk content as file metadata", () => { + const diff = `diff --git a/backend/source.js b/backend/source.js\n--- a/backend/source.js\n+++ b/backend/source.js\n@@ -0,0 +1 @@\n+++ b/frontend/fake.js\n`; + assert.equal(filterDiffByGlob(diff, "frontend/**").diff, ""); + assert.equal(filterDiffByGlob(diff, "backend/**").diff, diff); + }); + it("handles binary diffs and filenames containing spaces", () => { const binary = `diff --git a/frontend/brand image.png b/frontend/brand image.png\nBinary files a/frontend/brand image.png and b/frontend/brand image.png differ\n`; assert.equal(filterDiffByGlob(binary, "frontend/**").diff, binary); diff --git a/src/worker.test.mjs b/src/worker.test.mjs index 8a4e6fc..1b84e0a 100644 --- a/src/worker.test.mjs +++ b/src/worker.test.mjs @@ -293,7 +293,7 @@ describe("runWorker file-glob scoping", () => { }, }; const github = fakeGitHub({ threads: [thread] }); - const judged = []; + const judgeInvocations = []; await run( env, @@ -303,7 +303,7 @@ describe("runWorker file-glob scoping", () => { }, { runAgent: async options => { - judged.push({ suffix: options.suffix, prompt: options.promptText }); + judgeInvocations.push({ suffix: options.suffix, prompt: options.promptText }); return { code: 0, sessionId: "judge-session", @@ -326,10 +326,10 @@ describe("runWorker file-glob scoping", () => { ); assert.deepEqual( - judged.map(({ suffix }) => suffix), + judgeInvocations.map(({ suffix }) => suffix), ["resolve"], ); - assert.match(judged[0].prompt, /\[id=T-open\]/); + assert.match(judgeInvocations[0].prompt, /\[id=T-open\]/); assert.equal(github.childRuns().length, 1); assert.ok( github.calls.some(