kyle-sexton · Oct 23, 2026 · Oct 23, 2026 · Oct 23, 2026 · Oct 23, 2026 · Oct 23, 2026
diff --git a/.changes/pr-pipeline/feat-address-feedback-pr-paths-7c41e2a9.md b/.changes/pr-pipeline/feat-address-feedback-pr-paths-7c41e2a9.md
@@ -0,0 +1,7 @@
+---
+bump: minor
+---
+
+### Changed
+
+- `address-feedback` may fix a finding in any file the pull request already changes that the gated head holds, not only in the thread's own scope. The select job writes the PR's files, rename sources included, as `pr_paths` in the thread file and `run.json`, less `.github/workflows/`, `.gitmodules`, `CODEOWNERS`, `settings*.json`, package manifests, `requirements*.txt`, anything under `.github/`, `CLAUDE.md`, `AGENTS.md`, `.mcp.json`, `.lsp.json`, a `.claude` or `.claude-plugin` directory and a plugin's `hooks/`, `monitors/` and `bin/`. `apply-patch.mjs`'s `kept-threads` rule and the reply job admit them, and only as changes to an existing file: a file the fix creates keeps the old scope. A `VALID` fix commit lands when it changes the thread's path or a `pr_paths` file at a line the reply's evidence cites inside one of its hunks. Every apply-patch refusal still applies.
diff --git a/.changes/source-control/feat-address-feedback-pr-paths-3e9b0d51.md b/.changes/source-control/feat-address-feedback-pr-paths-3e9b0d51.md
@@ -0,0 +1,7 @@
+---
+bump: minor
+---
+
+### Changed
+
+- The review-thread tool (`feedback-thread.sh`) of `/source-control:pull-request comments --ci` admits a file in the thread file's `pr_paths`, the files the pull request already changes, in every kept thread's scope while the gated head holds it. `reference/comments-ci.md` tells the model to run `check-paths` before calling a fix out of scope, and to cite a changed line in such a file as evidence.
@@ -637,7 +637,7 @@ async function pushMerge({ api, cwd, gitBin, row, context, number, target, headS
       return escalate("invalid", `the patch changes ${entries.length} paths; the row allows ${row["max-files"]}`);
     }
     const allow = row.allow.map((name) => PREDICATES[name](context));
-    const outside = entries.find((entry) => !allow.every((admits) => admits(entry.path)));
+    const outside = entries.find((entry) => !allow.every((admits) => admits(entry.path, entry)));
     if (outside) {
       return reason("unexpected-path-staged", `not a conflicted path: ${outside.path}`);
     }
@@ -840,7 +840,7 @@ export async function push({
   if (row["max-files"] !== null && entries.length > row["max-files"]) {
     return escalate("invalid", `the patch changes ${entries.length} paths; the row allows ${row["max-files"]}`);
   }
-  const outside = entries.find((entry) => !allow.every((admits) => admits(entry.path)));
+  const outside = entries.find((entry) => !allow.every((admits) => admits(entry.path, entry)));
   if (outside) {
     return escalate("protected-path", `not allowed by ${row.allow.join(", ")}: ${outside.path}`);
   }
@@ -955,23 +955,30 @@ function need(env, keys) {
 }
 
 // The kept threads of a threads.json: each a path and, when a fix request
-// named files, its named_paths.
+// named files, its named_paths; and the files the PR already changes, its
+// pr_paths.
 export function keptThreadsFrom(file) {
   if (!file) {
     return undefined;
   }
-  const threads = JSON.parse(readFileSync(file, "utf8"))?.threads;
+  const { threads, pr_paths: prPaths = [] } = JSON.parse(readFileSync(file, "utf8")) ?? {};
   const plain = (value) => typeof value === "string" && value !== "";
   if (
     !Array.isArray(threads) ||
+    !Array.isArray(prPaths) ||
+    !prPaths.every(plain) ||
     !threads.every(
       (thread) =>
         plain(thread?.path) && (thread.named_paths === undefined || (Array.isArray(thread.named_paths) && thread.named_paths.every(plain))),
     )
   ) {
-    throw new Error("KEPT_THREADS_FILE is not a thread file of paths and named_paths");
+    throw new Error("KEPT_THREADS_FILE is not a thread file of paths, named_paths and pr_paths");
   }
-  return { paths: new Set(threads.map((thread) => thread.path)), threads: threads.map(({ path, named_paths }) => ({ path, named_paths })) };
+  return {
+    paths: new Set(threads.map((thread) => thread.path)),
+    threads: threads.map(({ path, named_paths }) => ({ path, named_paths })),
+    prPaths: new Set(prPaths),
+  };
 }
 
 // The base dependabot.yml's [ecosystem, directory] pairs, or null when the

@@ -703,6 +703,42 @@ test("review-threads: a file a kept thread's named_paths holds pushes; the same
   });
 });
 
+// A finding on a doc fixed in a script the pull request also changes: the
+// thread file's pr_paths carry the PR's files into every thread's scope.
+const docThread = (prPaths) => ({ ...keptOf([{ path: "README.md" }]), prPaths: new Set(prPaths) });
+const docPush = (fix, prPaths) => pushWith(fix, fakeApi(fix.repo, fix.head), { row: row("review-threads"), context: { keptThreads: docThread(prPaths) } });
+
+test("review-threads: a thread on one PR file fixed in another PR file pushes", async (t) => {
+  const fix = repoWithPatch(t, (repo) => writeFileSync(join(repo, "scripts/x.sh"), "#!/bin/sh\nfoo=2\n"));
+  assert.equal((await docPush(fix, ["README.md", "scripts/x.sh"])).outcome, "pushed");
+});
+
+test("review-threads: a fix in a file the PR does not change is refused", async (t) => {
+  const fix = repoWithPatch(t, (repo) => writeFileSync(join(repo, "scripts/x.sh"), "#!/bin/sh\nfoo=2\n"));
+  assert.deepEqual(await docPush(fix, ["README.md"]), { outcome: "escalate", escalationReason: "patch-protected-path" });
+});
+
+test("review-threads: a new file stays refused even when pr_paths lists it", async (t) => {
+  const fix = repoWithPatch(t, (repo) => writeFileSync(join(repo, "scripts/new.sh"), "#!/bin/sh\n"));
+  assert.deepEqual(await docPush(fix, ["README.md", "scripts/new.sh"]), { outcome: "escalate", escalationReason: "patch-protected-path" });
+});
+
+test("review-threads: a protected path the PR changes is still refused", async (t) => {
+  for (const path of [".github/workflows/ci.yml", "CODEOWNERS", ".github/actions/resolve-config/resolve.mjs"]) {
+    const fix = repoWithPatch(
+      t,
+      (repo) => writeFileSync(join(repo, path), "changed\n"),
+      {
+        head: (repo) => {
+          mkdirSync(join(repo, dirname(path)), { recursive: true });
+          writeFileSync(join(repo, path), "head\n");
+        },
+      },
+    );
+    assert.deepEqual(await docPush(fix, ["README.md", path]), { outcome: "escalate", escalationReason: "patch-protected-path" }, path);
+  }
+});
+
 test("review-threads: a symbolic link at an in-scope path is refused patch-invalid", async (t) => {
   const fix = repoWithPatch(t, (repo) => {
     mkdirSync(join(repo, "plugins/p"), { recursive: true });
@@ -731,9 +767,12 @@ test("keptThreadsFrom reads a threads.json's paths and named_paths, and refuses
       { path: "plugins/p/a.md", named_paths: ["plugins/p/b.js"] },
       { path: "src/c.py", named_paths: undefined },
     ],
+    prPaths: new Set(),
   });
+  writeFileSync(file, JSON.stringify({ threads: [{ path: "plugins/p/a.md" }], pr_paths: ["plugins/p/a.md", "src/d.sh"] }));
+  assert.deepEqual(keptThreadsFrom(file).prPaths, new Set(["plugins/p/a.md", "src/d.sh"]));
   assert.equal(keptThreadsFrom(""), undefined);
-  for (const text of [JSON.stringify(["plugins/p/a.md"]), JSON.stringify({ threads: [{ path: 7 }] }), JSON.stringify({ threads: [{ path: "a", named_paths: "b" }] })]) {
+  for (const text of [JSON.stringify(["plugins/p/a.md"]), JSON.stringify({ threads: [{ path: 7 }] }), JSON.stringify({ threads: [{ path: "a", named_paths: "b" }] }), JSON.stringify({ threads: [{ path: "a" }], pr_paths: "b" }), JSON.stringify({ threads: [{ path: "a" }], pr_paths: [7] })]) {
     writeFileSync(file, text);
     assert.throws(() => keptThreadsFrom(file), /KEPT_THREADS_FILE/, text);
   }

@@ -9,12 +9,17 @@ import { inThreadScope } from "../pr-unblock/thread-scope.mjs";
 export const PREDICATES = {
   any: () => () => true,
   // A kept thread's path, or a file thread-scope.mjs places in its scope,
-  // its named_paths included: `keptThreads` is { paths, threads }.
+  // its named_paths and the PR's own files the patch does not add included:
+  // `keptThreads` is { paths, threads, prPaths }.
   "kept-threads": ({ keptThreads }) => {
     if (!(keptThreads?.paths instanceof Set) || keptThreads.paths.size === 0 || !Array.isArray(keptThreads.threads)) {
       throw new Error("allow kept-threads needs the run's kept thread paths");
     }
-    return (path) => inThreadScope(path, keptThreads.paths, keptThreads.threads);
+    return (path, entry) =>
+      inThreadScope(path, keptThreads.paths, keptThreads.threads, {
+        prPaths: keptThreads.prPaths ?? new Set(),
+        added: entry === undefined || entry.status?.[0] === "A",
+      });
   },
   // An npm or pip manifest or lock file outside .github/, in a directory of
   // its ecosystem among the base dependabot.yml's [ecosystem, directory]

@@ -13,7 +13,8 @@
 //           the configured number of rounds already ran and no write-access
 //           fix request came after the newest one) or proceed, writing the thread file
 //           the skill's review-thread tool reads, with the same-plugin files
-//           a write-access fix request names (namedPaths), and run.json, the
+//           a write-access fix request names (namedPaths) and the files the
+//           PR already changes (pr_paths, prPathsOf), and run.json, the
 //           record the reply job checks the replies against (runRecord). On a PR carrying the
 //           human-gated label it keeps only threads a trusted person with
 //           write access asked it to fix (isFixRequest), and skips
@@ -31,7 +32,7 @@ import { join } from "node:path";
 import { pathToFileURL } from "node:url";
 
 import { commentItems, gateSource, listLabelEvents, ROUND_MARKER } from "./gate-source.mjs";
-import { isProtectedPath, namedInScope } from "./thread-scope.mjs";
+import { isProtectedPath, namedInScope, prPathsOf } from "./thread-scope.mjs";
 
 // The round cap is the config's `loop.review-rounds`, passed in as
 // REVIEW_ROUNDS; the resolver's default is the fallback.
@@ -498,7 +499,7 @@ export async function select({
     // put the label on.
     fixRequested: labeled,
     threads: worked,
-    file: { version: 1, repository, pr, head_sha: headSha, threads: worked },
+    file: { version: 1, repository, pr, head_sha: headSha, pr_paths: prPathsOf(files), threads: worked },
   };
 }
 
@@ -509,6 +510,7 @@ export function runRecord(file) {
   const run = {
     version: 1,
     head_sha: file.head_sha,
+    pr_paths: file.pr_paths,
     threads: file.threads.map((t) => ({
       id: t.id,
       path: t.path,

@@ -82,6 +82,7 @@ test("an unresolved reviewer-bot thread on a marker-confirmed head proceeds and
     repository: "o/r",
     pr: 42,
     head_sha: HEAD,
+    pr_paths: ["src/a.js"],
     threads: [
       {
         id: "PRRT_1",
@@ -95,6 +96,33 @@ test("an unresolved reviewer-bot thread on a marker-confirmed head proceeds and
   });
 });
 
+// A finding on a doc whose fix lives in a script the PR also changes: the
+// thread file lists every file the PR changes, rename sources included, but
+// none a fix may never change.
+test("the thread file's pr_paths hold the PR's files, rename sources included, less the refused ones", async () => {
+  const files = [
+    { filename: "plugins/p/skills/s/converge.md" },
+    { filename: "plugins/p/skills/s/scripts/converge-plan.sh" },
+    { filename: "docs/new.md", previous_filename: "docs/old.md", status: "renamed" },
+    { filename: "src/a.js" },
+    { filename: ".github/actions/resolve-config/resolve.mjs" },
+    { filename: ".github/standards/trusted-actors/trusted-actors.json" },
+    { filename: ".github/actions/check-trusted-trigger/action.yml" },
+    { filename: "CODEOWNERS" },
+    { filename: "plugins/p/.claude/settings.json" },
+    { filename: "a\\b.md" },
+  ];
+  const result = await run(fakeApi({ files }));
+  assert.equal(result.decision, "proceed");
+  assert.deepEqual(result.file.pr_paths, [
+    "docs/new.md",
+    "docs/old.md",
+    "plugins/p/skills/s/converge.md",
+    "plugins/p/skills/s/scripts/converge-plan.sh",
+    "src/a.js",
+  ]);
+});
+
 test("the security lane's marker also confirms a head", async () => {
   const result = await run(
     fakeApi({ issueComments: [{ id: 1, user: { id: ACTIONS_BOT }, body: marker("claude-security-review", HEAD) }] }),
@@ -699,10 +727,12 @@ test("main select writes outputs and the thread file; escalation writes no threa
   assert.equal(okFile.threads[0].id, "PRRT_1");
   // No lane branch: the model job commits nothing.
   assert.equal(Object.hasOwn(okFile, "lane_branch"), false);
+  // The files the PR changes, for every kept thread's scope.
+  assert.deepEqual(okFile.pr_paths, ["src/a.js"]);
   // run.json's bytes, in the shape the write runner's collect step wrote.
   assert.equal(
     readFileSync(join(dir, "ok.run.json"), "utf8"),
-    `{"version":1,"head_sha":"${HEAD}","threads":[{"id":"PRRT_1","path":"src/a.js","comment_ids":[100]}]}\n`,
+    `{"version":1,"head_sha":"${HEAD}","pr_paths":["src/a.js"],"threads":[{"id":"PRRT_1","path":"src/a.js","comment_ids":[100]}]}\n`,
   );
 
   // The people on the list come from TRUSTED_ACTORS.

@@ -123,7 +123,13 @@ export function parseRun(text) {
   } catch {
     throw new Invalid("run.json does not parse");
   }
-  check(run?.version === 1 && SHA.test(run.head_sha ?? "") && Array.isArray(run.threads), "run.json is malformed");
+  check(
+    run?.version === 1 &&
+      SHA.test(run.head_sha ?? "") &&
+      Array.isArray(run.threads) &&
+      (run.pr_paths === undefined || (Array.isArray(run.pr_paths) && run.pr_paths.every((path) => typeof path === "string"))),
+    "run.json is malformed",
+  );
   for (const thread of run.threads) {
     check(
       THREAD_ID.test(thread?.id ?? "") &&
@@ -199,9 +205,27 @@ export function renderReply({ disposition, evidence, fixSha, note }) {
 
 const SKILL_FILE = /^((?:.*\/)?skills\/[^/]+\/)SKILL\.md$/;
 
+// Whether a unified diff's new side (a commit file's `patch`) holds `line` in
+// one of its hunks; false with no patch, as for a binary or very large file.
+export function lineChanged(patch, line) {
+  for (const [, start, count] of String(patch ?? "").matchAll(/^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/gm)) {
+    const first = Number(start);
+    const size = count === undefined ? 1 : Number(count);
+    if (line >= first && line < first + size) {
+      return true;
+    }
+  }
+  return false;
+}
+
 // Whether a fix commit's changed files touch a thread's path: the path itself,
-// or for a skill's SKILL.md any file under that skill's directory.
-function touchesThread(changed, threadPath) {
+// a file the pull request already changes whose line the reply's evidence
+// cites inside a hunk of the fix (`cited`), so a finding on one file fixed in another of the PR's files
+// lands, or for a skill's SKILL.md any file under that skill's directory.
+function touchesThread(changed, threadPath, cited) {
+  if (changed.some((file) => cited.has(file))) {
+    return true;
+  }
   const skillDir = SKILL_FILE.exec(threadPath)?.[1];
   return skillDir === undefined ? changed.includes(threadPath) : changed.some((file) => file.startsWith(skillDir));
 }
@@ -226,11 +250,12 @@ export function fixCommitOf(commits, appId) {
 
 // Whenever the PR branch moved since the select job's head, or a reply is
 // VALID, that range may change only what inThreadScope allows around the kept
-// threads' paths (run.json, whose SHA-256 the select job recorded), and
-// nothing under .github/workflows/. With a VALID reply the range must hold
-// exactly one fix commit (fixCommitOf), and it must change every VALID thread's path, or
-// for a thread on a skill's SKILL.md any file in that skill's directory
-// (touchesThread). Returns the fix commit, "" when no reply is VALID, or null.
+// threads' paths and the PR's own files (run.json, whose SHA-256 the select
+// job recorded), adding none of those files, and nothing under
+// .github/workflows/. With a VALID reply the range must hold
+// exactly one fix commit (fixCommitOf), and it must change every VALID thread's path, a
+// file the PR already changes at a line the reply's evidence cites, or for a
+// thread on a skill's SKILL.md any file in that skill's directory (touchesThread). Returns the fix commit, "" when no reply is VALID, or null.
 async function fixesLanded({ api, run, headSha, replies, appId }) {
   const valid = replies.filter((reply) => reply.disposition === "VALID");
   if (valid.length === 0 && run.head_sha === headSha) {
@@ -240,22 +265,38 @@ async function fixesLanded({ api, run, headSha, replies, appId }) {
   const fixSha = valid.length === 0 ? "" : fixCommitOf(comparison.commits ?? [], appId);
   const threadPath = new Map(run.threads.map((thread) => [thread.id, thread.path]));
   const paths = new Set(threadPath.values());
+  const prPaths = new Set(run.pr_paths ?? []);
   const files = comparison.files ?? [];
   const inRange =
     ["ahead", "identical"].includes(comparison.status) &&
     fixSha !== null &&
     files.length < LIMITS.files &&
     files.every((file) =>
       [file.filename, file.previous_filename].every(
-        (path) => path === undefined || inThreadScope(path, paths, run.threads),
+        (path) =>
+          path === undefined ||
+          inThreadScope(path, paths, run.threads, {
+            prPaths,
+            added: path === file.filename && ["added", "renamed", "copied"].includes(file.status),
+          }),
       ),
     );
   if (!inRange) {
     return null;
   }
   if (fixSha) {
-    const changed = ((await api.getCommit(fixSha)).files ?? []).map((file) => file.filename);
-    if (!valid.every((reply) => touchesThread(changed, threadPath.get(reply.thread)))) {
+    const commitFiles = (await api.getCommit(fixSha)).files ?? [];
+    const changed = commitFiles.map((file) => file.filename);
+    const patches = new Map(commitFiles.map((file) => [file.filename, file.patch]));
+    // The pr_paths files whose cited line the fix commit changed.
+    const cited = (reply) =>
+      new Set(
+        reply.evidence
+          .map((ev) => [ev.slice(0, ev.lastIndexOf(":")), Number(ev.slice(ev.lastIndexOf(":") + 1))])
+          .filter(([path, line]) => prPaths.has(path) && lineChanged(patches.get(path), line))
+          .map(([path]) => path),
+      );
+    if (!valid.every((reply) => touchesThread(changed, threadPath.get(reply.thread), cited(reply)))) {
       return null;
     }
   }