From c5ff5f2157d232889e8f7014379429f1188dcc2b Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 18 Oct 2026 05:45:17 -0400 Subject: [PATCH 1/5] fix(pr-pipeline): let address-feedback edit its thread's eval case and sibling tests A kept thread's scope was its exact path, so a fix that added a sample in the same eval case, or a test beside the file, was refused by the promote step (#8103, #8142). Scope now also covers any file in the plugins/

/evals// directory a kept thread's path sits in, and a test file (test_*.py, *_test.py, *.test.*, *.spec.*) in that path's directory. .github/ stays refused. The rule lives in .github/scripts/pr-unblock/thread-scope.mjs, used by publish-feedback.mjs; the promote step imports a copy taken before the head checkout, from the bytes it checked against that copy's SHA-256. The review-thread tool's jq check applies the same rule. Closes #8109 Co-Authored-By: Claude Opus 5.5 --- ...fix-pr-unblock-eval-case-scope-ed5fddbc.md | 7 ++ .../scripts/pr-unblock/address-feedback.mjs | 6 +- .../scripts/pr-unblock/publish-feedback.mjs | 14 ++-- .../pr-unblock/publish-feedback.test.mjs | 16 ++++ .github/scripts/pr-unblock/thread-scope.mjs | 30 ++++++++ .../scripts/pr-unblock/thread-scope.test.mjs | 77 +++++++++++++++++++ .github/workflows/pr-run-activity-write.yml | 32 ++++++-- docs/conventions/pr-pipeline/CHANGELOG.md | 10 +++ .../pr-pipeline/pr-run-activity.md | 14 ++-- .../pull-request/reference/comments-ci.md | 18 +++-- .../pull-request/scripts/feedback-thread.sh | 23 ++++-- .../scripts/feedback-thread.test.sh | 19 +++++ 12 files changed, 229 insertions(+), 37 deletions(-) create mode 100644 .changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md create mode 100644 .github/scripts/pr-unblock/thread-scope.mjs create mode 100644 .github/scripts/pr-unblock/thread-scope.test.mjs diff --git a/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md b/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md new file mode 100644 index 00000000000..12f1f0e3838 --- /dev/null +++ b/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- The `comments --ci` review-thread tool's `check-paths` and `record` now also allow any file in the `plugins/

/evals//` directory a kept thread's path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in the same directory as a kept thread's path. `.github/` stays refused. diff --git a/.github/scripts/pr-unblock/address-feedback.mjs b/.github/scripts/pr-unblock/address-feedback.mjs index fcbf17e49de..0e4e063b1e9 100644 --- a/.github/scripts/pr-unblock/address-feedback.mjs +++ b/.github/scripts/pr-unblock/address-feedback.mjs @@ -19,6 +19,8 @@ import { appendFileSync, readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; +import { isProtectedPath } 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. export const DEFAULT_MAX_ROUNDS = 2; @@ -36,10 +38,6 @@ const FILES_CAP = 3000; const firstLine = (body) => String(body ?? "").split("\n")[0].replace(/\r$/, ""); -export function isProtectedPath(path) { - return /^\.github(\/|$)/i.test(String(path)); -} - // Round markers the lanes App posted; any other author's copy is ignored. export const isRoundMarker = (comment, appId) => comment.user?.id === appId && firstLine(comment.body) === ROUND_MARKER; diff --git a/.github/scripts/pr-unblock/publish-feedback.mjs b/.github/scripts/pr-unblock/publish-feedback.mjs index a2eed539121..9825930de76 100755 --- a/.github/scripts/pr-unblock/publish-feedback.mjs +++ b/.github/scripts/pr-unblock/publish-feedback.mjs @@ -10,7 +10,8 @@ // fields, within the limits below. Any failure posts no reply. // 2. For VALID replies, confirms every fix commit reached the PR branch // after the model job's gated head and that the branch changed no path -// but the kept threads' and none under .github/. +// outside the kept threads' scope (thread-scope.mjs) and none under +// .github/. // 3. Re-reads each thread and skips one that is resolved, moved, or has a // comment the model job did not see; posts the templated reply on its // root comment and resolves it only when it is VALID. The PR head is @@ -46,7 +47,6 @@ import { countRounds, createGitHubApi, currentThreads, - isProtectedPath, isRoundMarker, DEFAULT_MAX_ROUNDS, maxRoundsFrom, @@ -57,6 +57,7 @@ import { SHA, trustedHumansFrom, } from "./address-feedback.mjs"; +import { inThreadScope } from "./thread-scope.mjs"; export const LIMITS = { bytes: 262144, note: 2000, evidence: 10, files: 300 }; const THREAD_ID = /^PRRT_[A-Za-z0-9_-]{1,100}$/; @@ -168,9 +169,10 @@ export function renderReply({ disposition, evidence, fixSha, note }) { } // Whenever the PR branch moved since the model job's gated head, or a reply is -// VALID, that range may change only the kept threads' paths (run.json, whose -// SHA-256 the model job's runner recorded), and nothing under .github/. Every VALID -// fix must be a commit in that range that changes its own thread's path. +// VALID, that range may change only what inThreadScope allows around the kept +// threads' paths (run.json, whose SHA-256 the model job's runner recorded), and +// nothing under .github/. Every VALID fix must be a commit in that range that +// changes its own thread's path. async function fixesLanded({ api, run, headSha, replies }) { const valid = replies.filter((reply) => reply.disposition === "VALID"); if (valid.length === 0 && run.head_sha === headSha) { @@ -187,7 +189,7 @@ async function fixesLanded({ api, run, headSha, replies }) { files.length < LIMITS.files && files.every((file) => [file.filename, file.previous_filename].every( - (path) => path === undefined || (paths.has(path) && !isProtectedPath(path)), + (path) => path === undefined || inThreadScope(path, paths), ), ); if (!inRange) { diff --git a/.github/scripts/pr-unblock/publish-feedback.test.mjs b/.github/scripts/pr-unblock/publish-feedback.test.mjs index d799f61770a..664f57c7307 100644 --- a/.github/scripts/pr-unblock/publish-feedback.test.mjs +++ b/.github/scripts/pr-unblock/publish-feedback.test.mjs @@ -422,6 +422,22 @@ for (const [label, compare] of [ }); } +test("a VALID fix that also adds a sample in its thread's eval case lands; a sibling case's sample does not", async () => { + const caseRun = JSON.stringify({ ...RUN, threads: [{ ...RUN.threads[0], path: "plugins/p/evals/c/case.yaml" }] }); + for (const [sample, landed] of [ + ["plugins/p/evals/c/samples/new.json", true], + ["plugins/p/evals/other/samples/new.json", false], + ]) { + const api = fakeApi({ + compare: { status: "ahead", commits: [{ sha: FIX }], files: [{ filename: "plugins/p/evals/c/case.yaml" }, { filename: sample }] }, + commits: { [FIX]: { files: [{ filename: "plugins/p/evals/c/case.yaml" }, { filename: sample }] } }, + }); + const result = await run(api, lines(reply({ disposition: "VALID", fix_sha: FIX })), caseRun); + assert.equal(result.escalationReason, landed ? "" : "feedback-not-landed", sample); + assert.equal(api.posted().length, landed ? 1 : 0, sample); + } +}); + test("feedback-not-landed, nothing posted: a VALID fix commit that does not change its own thread's path", async () => { const api = fakeApi({ compare: { status: "ahead", commits: [{ sha: FIX }], files: [{ filename: "docs/b.md" }] }, diff --git a/.github/scripts/pr-unblock/thread-scope.mjs b/.github/scripts/pr-unblock/thread-scope.mjs new file mode 100644 index 00000000000..d034e9a3194 --- /dev/null +++ b/.github/scripts/pr-unblock/thread-scope.mjs @@ -0,0 +1,30 @@ +// Which paths an address-feedback fix may change, shared by the promote step +// of .github/workflows/pr-run-activity-write.yml (which imports a SHA-256 +// pinned copy, so it imports nothing itself) and publish-feedback.mjs. + +export function isProtectedPath(path) { + return /^\.github(\/|$)/i.test(String(path)); +} + +const EVAL_CASE = /^plugins\/[^/]+\/evals\/[^/]+\//; +const TEST_FILE = /^(test_[^/]*\.py|[^/]*_test\.py|[^/]*\.(test|spec)\.[^/]+)$/; +const dirOf = (path) => path.slice(0, path.lastIndexOf("/") + 1); + +// A kept thread's own path, any file in the plugins/

/evals// +// directory a kept thread's path sits in, or a test file in the same +// directory as a kept thread's path; never anything under .github/. +export function inThreadScope(path, threadPaths) { + if (isProtectedPath(path)) { + return false; + } + if (threadPaths.has(path)) { + return true; + } + const kept = [...threadPaths]; + const evalCase = EVAL_CASE.exec(path)?.[0]; + if (evalCase !== undefined && kept.some((p) => p.startsWith(evalCase))) { + return true; + } + const dir = dirOf(path); + return TEST_FILE.test(path.slice(dir.length)) && kept.some((p) => dirOf(p) === dir); +} diff --git a/.github/scripts/pr-unblock/thread-scope.test.mjs b/.github/scripts/pr-unblock/thread-scope.test.mjs new file mode 100644 index 00000000000..64f8da3fcc3 --- /dev/null +++ b/.github/scripts/pr-unblock/thread-scope.test.mjs @@ -0,0 +1,77 @@ +// Cases from issue #8109's Verification section. +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { inThreadScope, isProtectedPath } from "./thread-scope.mjs"; + +const kept = new Set(["plugins/p/evals/c/graders/x.md", "src/a.js"]); + +test("a kept thread's own path is in scope", () => { + assert.equal(inThreadScope("src/a.js", kept), true); + assert.equal(inThreadScope("plugins/p/evals/c/graders/x.md", kept), true); +}); + +test("a file in the same eval case as a kept thread's path is in scope, at any depth", () => { + assert.equal(inThreadScope("plugins/p/evals/c/samples/y.json", kept), true); + assert.equal(inThreadScope("plugins/p/evals/c/samples/deep/z.json", kept), true); +}); + +test("a sibling eval case, a skill file and an unnamed file stay out of scope", () => { + assert.equal(inThreadScope("plugins/p/evals/other/samples/y.json", kept), false); + assert.equal(inThreadScope("plugins/p/skills/s/SKILL.md", kept), false); + assert.equal(inThreadScope("src/b.js", kept), false); + assert.equal(inThreadScope("plugins/q/evals/c/samples/y.json", kept), false); +}); + +test("a file directly under evals/ is no case directory", () => { + const evalsFile = new Set(["plugins/p/evals/evals.json"]); + assert.equal(inThreadScope("plugins/p/evals/other.json", evalsFile), false); + assert.equal(inThreadScope("plugins/p/evals/c/case.yaml", evalsFile), false); +}); + +test("with no kept thread under evals/, an eval case file is refused", () => { + assert.equal(inThreadScope("plugins/p/evals/c/samples/y.json", new Set(["src/a.js"])), false); +}); + +// Names from the coordinator's addition; plugins/planning/surface/test_round.py +// is the file #8142's run 39113747960 refused. +test("a test file in the same directory as a kept thread's path is in scope", () => { + const surface = new Set(["plugins/planning/surface/round.py", "src/a.js", "README.md"]); + for (const path of [ + "plugins/planning/surface/test_round.py", + "plugins/planning/surface/round_test.py", + "src/a.test.js", + "src/a.spec.ts", + "test_root.py", + ]) { + assert.equal(inThreadScope(path, surface), true, path); + } +}); + +test("a test file in another directory, or a same-directory file that is no test, stays out of scope", () => { + const surface = new Set(["plugins/planning/surface/round.py", "src/a.js"]); + for (const path of [ + "plugins/planning/surface/tests/test_round.py", + "plugins/planning/test_round.py", + "plugins/planning/other/test_round.py", + "src/lib/a.test.js", + "plugins/planning/surface/helper.py", + "plugins/planning/surface/testing.py", + "plugins/planning/surface/test_round.sh", + "test_root.py", + ]) { + assert.equal(inThreadScope(path, surface), false, path); + } +}); + +test("a test file under .github/ is refused even beside a kept thread's path there", () => { + assert.equal(inThreadScope(".github/scripts/x.test.mjs", new Set([".github/scripts/x.mjs"])), false); +}); + +test(".github/ is refused even when a kept thread names a file there", () => { + const github = new Set([".github/workflows/x.yml", ".GitHub/evals/c/a.md"]); + assert.equal(inThreadScope(".github/workflows/x.yml", github), false); + assert.equal(inThreadScope(".GitHub/evals/c/b.md", github), false); + assert.equal(isProtectedPath(".github"), true); + assert.equal(isProtectedPath(".githubx/a"), false); +}); diff --git a/.github/workflows/pr-run-activity-write.yml b/.github/workflows/pr-run-activity-write.yml index f2a36683b7d..65cae6f2e94 100644 --- a/.github/workflows/pr-run-activity-write.yml +++ b/.github/workflows/pr-run-activity-write.yml @@ -471,7 +471,9 @@ jobs: # on a head a review lane's marker confirms, left by it alone or answered # by a trusted person after the lane escalated them, then # skips (none), escalates (a .github/** diff, or the round cap) or - # writes the thread file the skill's review-thread tool reads. + # writes the thread file the skill's review-thread tool reads. It also + # copies out the base's thread-scope.mjs with its SHA-256, which the + # promote step checks before it imports the copy. - name: Select the review threads id: feedback-select if: steps.config.outputs.applies == 'true' && steps.config.outputs.prepare == 'review-threads' @@ -482,10 +484,14 @@ jobs: APP_SENDER_ID: ${{ vars.AUTOMATION_LANES_APP_SENDER_ID }} LANE_BRANCH: ${{ format('lane-commits/{0}/{1}/{2}-{3}', inputs.lane, inputs.activity, github.run_id, github.run_attempt) }} FEEDBACK_DIR: ${{ runner.temp }}/lane-feedback + SCOPE_BIN: ${{ runner.temp }}/lane-feedback-bin TRUSTED_ACTORS: .base/.github/standards/trusted-actors/trusted-actors.json RESOLVED: ${{ runner.temp }}/resolved-config.json run: | - mkdir -p "$FEEDBACK_DIR" + mkdir -p "$FEEDBACK_DIR" "$SCOPE_BIN" + cp .base/.github/scripts/pr-unblock/thread-scope.mjs "$SCOPE_BIN/thread-scope.mjs" + sum=$(sha256sum <"$SCOPE_BIN/thread-scope.mjs") + echo "scope-sha256=${sum%% *}" >>"$GITHUB_OUTPUT" REVIEW_ROUNDS=$(jq -r '.loop["review-rounds"]' "$RESOLVED") export REVIEW_ROUNDS THREAD_FILE="$FEEDBACK_DIR/threads.json" node .base/.github/scripts/pr-unblock/address-feedback.mjs select @@ -1389,11 +1395,14 @@ jobs: MERGE_BASE: ${{ steps.config.outputs.prepare == 'merge-base' }} COMMIT_OUTCOME: ${{ steps.commit.outcome }} COMMIT_SHA: ${{ steps.commit.outputs.commit }} - # review-threads mode moves the PR branch only when the lane branch - # changes nothing but the kept threads' paths, none under .github/. + # review-threads mode moves the PR branch only when every path the + # lane branch changes is in the kept threads' scope, as the base's + # thread-scope.mjs decides, and none is under .github/. THREAD_FILE: >- ${{ steps.config.outputs.prepare == 'review-threads' && format('{0}/lane-feedback/threads.json', runner.temp) || '' }} + SCOPE_SCRIPT: ${{ runner.temp }}/lane-feedback-bin/thread-scope.mjs + SCOPE_SHA256: ${{ steps.feedback-select.outputs.scope-sha256 }} # dependency-review mode moves the PR branch only when the lane # branch changes nothing under .github/ and every commit's message # carries [dependabot skip], so Dependabot may still rebase the PR. @@ -1425,13 +1434,22 @@ jobs: return; } if (THREAD_FILE) { - const paths = new Set(JSON.parse(require('fs').readFileSync(THREAD_FILE, 'utf8')).threads.map((t) => t.path)); + const fs = require('fs'); + const paths = new Set(JSON.parse(fs.readFileSync(THREAD_FILE, 'utf8')).threads.map((t) => t.path)); + // Imported from the bytes just hashed, so a rewrite after the + // check cannot reach the import. + const scope = fs.readFileSync(SCOPE_SCRIPT); + if (!SCOPE_SHA256 || require('crypto').createHash('sha256').update(scope).digest('hex') !== SCOPE_SHA256) { + core.setFailed("thread-scope.mjs does not match the base's copy; the PR branch is not moved"); + return; + } + const { inThreadScope } = await import(`data:text/javascript;base64,${scope.toString('base64')}`); const { data } = await github.rest.repos.compareCommitsWithBasehead({ ...context.repo, basehead: `${HEAD_SHA}...${lane}` }); const bad = data.files.length >= 300 ? ['(300 or more files)'] : data.files .flatMap((f) => [f.filename, f.previous_filename].filter(Boolean)) - .filter((p) => !paths.has(p) || /^\.github(\/|$)/i.test(p)); + .filter((p) => !inThreadScope(p, paths)); if (bad.length > 0) { - core.setFailed(`The lane branch changes a path no kept thread names or under .github/ (${bad.slice(0, 5).join(', ')}); the PR branch is not moved`); + core.setFailed(`The lane branch changes a path outside the kept threads' scope or under .github/ (${bad.slice(0, 5).join(', ')}); the PR branch is not moved`); return; } } diff --git a/docs/conventions/pr-pipeline/CHANGELOG.md b/docs/conventions/pr-pipeline/CHANGELOG.md index 36d1ad47187..e63870c9cfe 100644 --- a/docs/conventions/pr-pipeline/CHANGELOG.md +++ b/docs/conventions/pr-pipeline/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog for the PR pipeline convention +## 2.11.4 - 2026-10-18 + +`pr-unblock` `address-feedback` widens a kept thread's scope from its path alone to also any file +in the `plugins/

/evals//` directory that path sits in, and a test file (`test_*.py`, +`*_test.py`, `*.test.*`, `*.spec.*`) in that path's directory +([#8109](https://github.com/melodic-software/claude-code-plugins/issues/8109)). The review-thread +tool, the promote step and the reply job apply the same rule; the promote step imports the base's +`thread-scope.mjs` from a copy taken before the head checkout, checked against its SHA-256. +`.github/` stays refused. `version` stays 2. + ## 2.11.2 - 2026-10-18 The broker's `workflow-modified` refusal has one exception, for a lane's post-run report job diff --git a/docs/conventions/pr-pipeline/pr-run-activity.md b/docs/conventions/pr-pipeline/pr-run-activity.md index c2f61d632d8..963b371d382 100644 --- a/docs/conventions/pr-pipeline/pr-run-activity.md +++ b/docs/conventions/pr-pipeline/pr-run-activity.md @@ -593,9 +593,13 @@ the base's `.github/scripts/pr-unblock/address-feedback.mjs` and `publish-feedba and Write are denied on the directory and on the tool's replies directory. The tool lists the kept threads, checks paths, and records a reply: it refuses a disposition file outside the scratch directory or off the template, and a `VALID` reply whose fix is not on the lane branch - or whose lane branch changes a file no kept thread names or any `.github/` path. It posts - nothing. The promote step then moves the PR branch only when the lane branch changes nothing - but kept-thread paths, none under `.github/`, in fewer than 300 files. + or whose lane branch changes a file outside every kept thread's scope or any `.github/` path. + A thread's scope is its path, any file in the `plugins/

/evals//` directory its path + sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in its path's + directory. It posts nothing. The promote step then moves the PR branch only when every path the + lane branch changes is in a kept thread's scope, none under `.github/`, in fewer than 300 + files, judged by a copy of the base's `thread-scope.mjs` taken before the head checkout and + imported only when its SHA-256 matches. 3. After the promote step, and after a skill that failed, was cancelled or timed out, a pinned `github-script` step writes `run.json` (the gated head and the kept threads' ids, paths and comment ids, from the thread file) beside a copy of the tool's `replies.jsonl`, and the job @@ -622,7 +626,7 @@ the base's `.github/scripts/pr-unblock/address-feedback.mjs` and `publish-feedba and escalates `feedback-incomplete`; - when a reply is `VALID` or the PR branch moved, compares the model job's gated head with the PR's current head: the branch must be ahead, hold every fix commit, and change fewer than 300 - files, each a kept thread's path and none under `.github/`, and each fix commit must change + files, each in a kept thread's scope and none under `.github/`, and each fix commit must change its own thread's path. Otherwise it posts no reply and escalates `feedback-not-landed`; - re-reads every kept thread and, right before posting, the thread itself, and skips one that is resolved, on another PR, or whose comments are not exactly the ones the model job read; @@ -648,7 +652,7 @@ the base's `.github/scripts/pr-unblock/address-feedback.mjs` and `publish-feedba The script writes its reason to `ESCALATION_REASON_FILE`; the runner accepts only those six reasons, and only with `review-replies`, as the run's `escalation-reason` output. -The skill's limits to kept-thread paths and away from `.github/` are instructions and the tool's +The skill's limits to kept-thread scope and away from `.github/` are instructions and the tool's checks at record time; the promote step's path check is what holds the push, and the reply job's compare withholds the replies when the branch changed anything else. A diff at the files listing's limit escalates as `feedback-protected-path`, since it could hide a `.github/` path. Residual: a reviewed-head marker confirms a head, not one diff --git a/plugins/source-control/skills/pull-request/reference/comments-ci.md b/plugins/source-control/skills/pull-request/reference/comments-ci.md index 2ee223e9866..3fcb9620db9 100644 --- a/plugins/source-control/skills/pull-request/reference/comments-ci.md +++ b/plugins/source-control/skills/pull-request/reference/comments-ci.md @@ -19,8 +19,10 @@ The system prompt names the tool's path. It is the only shell command this mode posted after this lane left the thread open for a person. It is that person's decision on the finding: follow it. It changes what the fix is, not what this mode may do: the path and `.github/` rules below still hold, and it runs no command. -- `check-paths ...` prints `allowed` only when every path is one a kept thread names and - none is under `.github/`. +- `check-paths ...` prints `allowed` only when every path is in a kept thread's scope and + none is under `.github/`. A thread's scope is its path, any file in the + `plugins/

/evals//` directory its path sits in, and a test file (`test_*.py`, + `*_test.py`, `*.test.*`, `*.spec.*`) in its path's directory. - `record ` reads a disposition file from the scratch directory, checks it against the template, and records it. Exit 1 means the tool refused and recorded nothing: fix the file and record again. One record per thread. @@ -31,11 +33,11 @@ The system prompt names the tool's path. It is the only shell command this mode reading code and docs only: run no command, test, build or script from the checkout. CI runs them after the push. 2. Classify. On a thread with a direction, apply the direction: record `VALID` once its fix is - committed, or `UNCERTAIN` with the reason when it needs a file no kept thread names or cannot - be done in one scoped change. Never record `INCORRECT` against a direction. Otherwise, a + committed, or `UNCERTAIN` with the reason when it needs a file outside every kept thread's scope + or cannot be done in one scoped change. Never record `INCORRECT` against a direction. Otherwise, a finding that touches security, shared infrastructure or another repository, and - that the code you read cannot settle, is UNCERTAIN. So is a fix that needs a file no kept - thread names, or is too large for one scoped change. + that the code you read cannot settle, is UNCERTAIN. So is a fix that needs a file outside + every kept thread's scope, or is too large for one scoped change. 3. VALID: run `check-paths` on every file the fix changes before you edit it, and edit only when it prints `allowed`. Commit the fix with the commit tool before you record it. One commit may fix several threads. @@ -53,8 +55,8 @@ The system prompt names the tool's path. It is the only shell command this mode `VALID` and `INCORRECT` need at least one `path:line` in the checkout, at most 10. `VALID-defer` is VALID (defer). The note is 1-2000 characters of plain prose: no control characters, no HTML comment (``), no token, no @-mention. It is posted in a code fence. -5. When `record` refuses a `VALID` because the branch changes a path no kept thread names, do not - edit more files to get past it. Record the remaining threads and report the refusal. +5. When `record` refuses a `VALID` because the branch changes a path outside every kept thread's + scope, do not edit more files to get past it. Record the remaining threads and report the refusal. The reply job resolves a `VALID` thread once its fix is on the pull request's branch. `INCORRECT`, `UNCERTAIN` and `VALID-defer` stay open, and the pull request then goes to a person. diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh index 1e91f873ed9..67b44810a74 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh @@ -7,8 +7,8 @@ # runner uploads and a later job with no model validates and posts. # # list the kept threads, as JSON -# check-paths ... exit 0 when every path is one a kept -# thread names and none is under .github/ +# check-paths ... exit 0 when every path is in a kept +# thread's scope (allowed below) # record check the disposition against the # template and append it to the replies # @@ -44,10 +44,19 @@ scratch_dir=${FEEDBACK_SCRATCH:-} workspace=${FEEDBACK_WORKSPACE:-} repo=$(jq -er '.repository | select(test("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$"))' "$threads_file") || die 2 "the thread file names no repository" -# A path a kept thread names, not under .github/ in any letter case. +# Not under .github/ in any letter case, and in a kept thread's scope: its +# path, any file in the plugins/

/evals// directory it sits in, or a +# test file in its directory. The same rule as .github/scripts/pr-unblock/ +# thread-scope.mjs, which the promote step and the reply job apply. allowed() { [[ ! ${1,,} =~ ^\.github(/|$) ]] && - jq -e --arg p "$1" 'any(.threads[]; .path == $p)' "$threads_file" >/dev/null + jq -e --arg p "$1" ' + def dir: sub("[^/]*$"; ""); + (first($p | capture("^(?plugins/[^/]+/evals/[^/]+/)").c) // null) as $case + | ($p | sub("^.*/"; "") | test("^(test_[^/]*\\.py|[^/]*_test\\.py|[^/]*\\.(test|spec)\\.[^/]+)$")) as $test + | any(.threads[].path; . == $p + or ($case != null and startswith($case)) + or ($test and dir == ($p | dir)))' "$threads_file" >/dev/null } cmd_list() { @@ -59,7 +68,7 @@ cmd_check_paths() { local refused=0 path for path in "$@"; do if ! allowed "$path"; then - printf 'refused: %s is not a path a kept thread names, or is under .github/\n' "$path" >&2 + printf "refused: %s is outside every kept thread's scope, or is under .github/\n" "$path" >&2 refused=1 fi done @@ -113,7 +122,7 @@ cmd_record() { done < <(jq -r '.evidence[]' <<<"$d") # A VALID reply names a fix commit on the lane branch, and every file the - # lane branch changed since the gated head is one a kept thread names. + # lane branch changed since the gated head is in a kept thread's scope. if [[ $disposition == VALID ]]; then local head lane cmp path head=$(jq -er '.head_sha | select(test("^[0-9a-f]{40}$"))' "$threads_file") || die 2 "the thread file names no head" @@ -123,7 +132,7 @@ cmd_record() { die 1 "the fix $fix_sha is not a commit on the lane branch" (($(jq '.files | length' <<<"$cmp") < 300)) || die 1 "the lane branch changes too many files to check" while IFS= read -r path; do - allowed "$path" || die 1 "the lane branch changes $path, which no kept thread names or which is under .github/; nothing is recorded" + allowed "$path" || die 1 "the lane branch changes $path, which is outside every kept thread's scope or under .github/; nothing is recorded" done < <(jq -r '.files[] | .filename, (.previous_filename // empty)' <<<"$cmp") fi diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh index 9731e04a4f7..5af83363878 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh @@ -84,6 +84,25 @@ run check-paths .github/workflows/ci.yml >/dev/null 2>&1 assert_exit "check-paths refuses .github/ even when a thread names it" 1 "$?" run check-paths .GITHUB/x.yml >/dev/null 2>&1 assert_exit "check-paths refuses .github/ in any letter case" 1 "$?" +run check-paths .github/workflows/ci.test.yml >/dev/null 2>&1 +assert_exit "check-paths refuses a test file beside a .github/ thread path" 1 "$?" + +# check-paths: the kept thread's eval case, and test files beside a thread path +setup +jq '.threads += [{"id":"PRRT_e","path":"plugins/p/evals/c/case.yaml","root_comment_id":400,"comments":[]}, + {"id":"PRRT_s","path":"plugins/planning/surface/round.py","root_comment_id":401,"comments":[]}]' \ + "$STUB/threads.json" >"$STUB/t2" && mv "$STUB/t2" "$STUB/threads.json" +for path in plugins/p/evals/c/samples/y.json plugins/p/evals/c/samples/deep/z.json \ + plugins/planning/surface/test_round.py plugins/planning/surface/round_test.py src/a.test.js src/a.spec.ts; do + run check-paths "$path" >/dev/null 2>&1 + assert_exit "check-paths allows $path" 0 "$?" +done +for path in plugins/p/evals/other/samples/y.json plugins/p/skills/s/SKILL.md plugins/p/evals/x.json \ + plugins/planning/surface/tests/test_round.py plugins/planning/test_round.py src/lib/a.test.js \ + plugins/planning/surface/helper.py plugins/planning/surface/test_round.sh test_root.py; do + run check-paths "$path" >/dev/null 2>&1 + assert_exit "check-paths refuses $path" 1 "$?" +done # record VALID: checked against the lane branch and appended, nothing posted setup From 95261dde48f16bc8d793698053971e72b1c8a8f8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 18 Oct 2026 05:59:00 -0400 Subject: [PATCH 2/5] fix(pr-pipeline): read the scope copy's env in promote and refuse non-normal paths The promote step named SCOPE_SCRIPT and SCOPE_SHA256 without reading them from process.env, so every review-threads promotion would have thrown and failed closed. A new test runs the step's script against fake clients and fails without the fix. Both scope rules now refuse a path with an empty, . or .. segment before any other check, so check-paths no longer answers allowed for plugins/p/evals/c/../../.github/x. Refs: #8109 Co-Authored-By: Claude Opus 5.5 --- .../scripts/pr-unblock/promote-scope.test.mjs | 110 ++++++++++++++++++ .github/scripts/pr-unblock/thread-scope.mjs | 8 +- .../scripts/pr-unblock/thread-scope.test.mjs | 12 ++ .github/workflows/pr-run-activity-write.yml | 1 + .../pull-request/scripts/feedback-thread.sh | 11 +- .../scripts/feedback-thread.test.sh | 4 +- 6 files changed, 138 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/pr-unblock/promote-scope.test.mjs diff --git a/.github/scripts/pr-unblock/promote-scope.test.mjs b/.github/scripts/pr-unblock/promote-scope.test.mjs new file mode 100644 index 00000000000..a31f1afa7d5 --- /dev/null +++ b/.github/scripts/pr-unblock/promote-scope.test.mjs @@ -0,0 +1,110 @@ +// Runs the review-threads path check of pr-run-activity-write.yml's promote +// step (the `script:` of the step with `id: promote`) against fake GitHub +// clients. A changed path in scope reaches the next gate, the hold-label read, +// which fails here because the case passes no hold labels. +import assert from "node:assert/strict"; +import { createHash } from "node:crypto"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { after, test } from "node:test"; + +const WORKFLOW = new URL("../../workflows/pr-run-activity-write.yml", import.meta.url); +const SCOPE = new URL("./thread-scope.mjs", import.meta.url); +const HEAD = "a".repeat(40); +const LANE = "b".repeat(40); + +function promoteScript() { + const lines = readFileSync(WORKFLOW, "utf8").split("\n"); + const id = lines.findIndex((line) => /^\s+id: promote$/.test(line)); + assert.notEqual(id, -1, "the workflow has the promote step"); + const start = lines.findIndex((line, i) => i > id && /^\s+script: \|$/.test(line)); + const keyIndent = lines[start].search(/\S/); + const body = []; + for (const line of lines.slice(start + 1)) { + if (line.trim() !== "" && line.search(/\S/) <= keyIndent) break; + body.push(line); + } + const indent = Math.min(...body.filter((l) => l.trim()).map((l) => l.search(/\S/))); + return body.map((l) => l.slice(indent)).join("\n"); +} + +const dir = mkdtempSync(join(tmpdir(), "promote-scope-")); +after(() => rmSync(dir, { recursive: true, force: true })); + +async function promote(files, { scope = readFileSync(SCOPE), sha256 } = {}) { + const threadFile = join(dir, "threads.json"); + writeFileSync(threadFile, JSON.stringify({ threads: [{ path: "plugins/p/evals/c/case.yaml" }, { path: "src/round.py" }] })); + const scopeScript = join(dir, "thread-scope.mjs"); + writeFileSync(scopeScript, scope); + const env = { + HEAD_SHA: HEAD, + HEAD_REF: "feature", + LANE_BRANCH: "lane-commits/x", + MERGE_BASE: "false", + THREAD_FILE: threadFile, + SCOPE_SCRIPT: scopeScript, + SCOPE_SHA256: sha256 ?? createHash("sha256").update(readFileSync(SCOPE)).digest("hex"), + DEPENDENCY_REVIEW: "false", + HOLD_LABELS: "", + }; + const saved = { ...process.env }; + Object.assign(process.env, env); + const failed = []; + const core = { setOutput() {}, info() {}, warning() {}, setFailed: (message) => failed.push(message) }; + const github = { + request: async () => ({ data: { object: { sha: LANE } } }), + rest: { + repos: { compareCommitsWithBasehead: async () => ({ data: { files } }) }, + git: { deleteRef: async () => {} }, + }, + }; + const context = { repo: { owner: "o", repo: "r" } }; + try { + const AsyncFunction = (async () => {}).constructor; + await new AsyncFunction("github", "context", "core", "require", promoteScript())( + github, + context, + core, + createRequire(import.meta.url), + ); + } finally { + process.env = saved; + } + return failed; +} + +const HOLD = /^No hold labels were read/; +const OUT = /^The lane branch changes a path outside the kept threads' scope or under \.github\//; + +test("in-scope changes pass the path check: the thread's path, its eval case, a sibling test", async () => { + const failed = await promote([ + { filename: "plugins/p/evals/c/case.yaml" }, + { filename: "plugins/p/evals/c/samples/deep/new.json" }, + { filename: "src/test_round.py" }, + ]); + assert.equal(failed.length, 1); + assert.match(failed[0], HOLD); +}); + +for (const file of [ + { filename: "plugins/p/evals/other/samples/new.json" }, + { filename: "src/lib/test_round.py" }, + { filename: ".github/workflows/x.yml" }, + { filename: "plugins/p/evals/c/x.json", previous_filename: ".github/x.yml" }, +]) { + test(`an out-of-scope change stops the move: ${file.previous_filename ?? file.filename}`, async () => { + const failed = await promote([{ filename: "plugins/p/evals/c/case.yaml" }, file]); + assert.equal(failed.length, 1); + assert.match(failed[0], OUT); + }); +} + +test("a rewritten scope copy, or no recorded hash, stops the move", async () => { + const permissive = "export const inThreadScope = () => true;\n"; + for (const options of [{ scope: permissive }, { sha256: "" }]) { + const failed = await promote([{ filename: ".github/workflows/x.yml" }], options); + assert.deepEqual(failed, ["thread-scope.mjs does not match the base's copy; the PR branch is not moved"]); + } +}); diff --git a/.github/scripts/pr-unblock/thread-scope.mjs b/.github/scripts/pr-unblock/thread-scope.mjs index d034e9a3194..5ec4ba7b55e 100644 --- a/.github/scripts/pr-unblock/thread-scope.mjs +++ b/.github/scripts/pr-unblock/thread-scope.mjs @@ -10,11 +10,15 @@ const EVAL_CASE = /^plugins\/[^/]+\/evals\/[^/]+\//; const TEST_FILE = /^(test_[^/]*\.py|[^/]*_test\.py|[^/]*\.(test|spec)\.[^/]+)$/; const dirOf = (path) => path.slice(0, path.lastIndexOf("/") + 1); +// An empty, . or .. segment, so a leading or doubled / too. +const BAD_SEGMENT = /(^|\/)\.{0,2}(\/|$)/; + // A kept thread's own path, any file in the plugins/

/evals// // directory a kept thread's path sits in, or a test file in the same -// directory as a kept thread's path; never anything under .github/. +// directory as a kept thread's path; never anything under .github/, and +// never a path that is not in normal form. export function inThreadScope(path, threadPaths) { - if (isProtectedPath(path)) { + if (isProtectedPath(path) || BAD_SEGMENT.test(path)) { return false; } if (threadPaths.has(path)) { diff --git a/.github/scripts/pr-unblock/thread-scope.test.mjs b/.github/scripts/pr-unblock/thread-scope.test.mjs index 64f8da3fcc3..be755d4b772 100644 --- a/.github/scripts/pr-unblock/thread-scope.test.mjs +++ b/.github/scripts/pr-unblock/thread-scope.test.mjs @@ -68,6 +68,18 @@ test("a test file under .github/ is refused even beside a kept thread's path the assert.equal(inThreadScope(".github/scripts/x.test.mjs", new Set([".github/scripts/x.mjs"])), false); }); +test("a path with an empty, . or .. segment, or a leading /, is refused before any other rule", () => { + for (const path of [ + "plugins/p/evals/c/../../../../.github/workflows/x.yml", + "plugins/p/evals/c/./samples/y.json", + "plugins/p/evals/c//y.json", + "/src/a.js", + "./src/a.js", + ]) { + assert.equal(inThreadScope(path, new Set([...kept, path])), false, path); + } +}); + test(".github/ is refused even when a kept thread names a file there", () => { const github = new Set([".github/workflows/x.yml", ".GitHub/evals/c/a.md"]); assert.equal(inThreadScope(".github/workflows/x.yml", github), false); diff --git a/.github/workflows/pr-run-activity-write.yml b/.github/workflows/pr-run-activity-write.yml index 65cae6f2e94..24c4db92500 100644 --- a/.github/workflows/pr-run-activity-write.yml +++ b/.github/workflows/pr-run-activity-write.yml @@ -1419,6 +1419,7 @@ jobs: script: | const { HEAD_SHA, HEAD_REF, LANE_BRANCH, MERGE_BASE, COMMIT_OUTCOME, COMMIT_SHA, THREAD_FILE } = process.env; const { DEPENDENCY_REVIEW, DEPENDENCY_OUTPUT, PR_NUMBER, HOLD_LABELS, READ_TOKEN } = process.env; + const { SCOPE_SCRIPT, SCOPE_SHA256 } = process.env; core.setOutput('skip-reason', ''); const tip = async (branch) => (await github.request('GET /repos/{owner}/{repo}/git/ref/{ref}', { ...context.repo, ref: `heads/${branch}` })) diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh index 67b44810a74..ef8f7c343d7 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh @@ -44,12 +44,13 @@ scratch_dir=${FEEDBACK_SCRATCH:-} workspace=${FEEDBACK_WORKSPACE:-} repo=$(jq -er '.repository | select(test("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$"))' "$threads_file") || die 2 "the thread file names no repository" -# Not under .github/ in any letter case, and in a kept thread's scope: its -# path, any file in the plugins/

/evals// directory it sits in, or a -# test file in its directory. The same rule as .github/scripts/pr-unblock/ -# thread-scope.mjs, which the promote step and the reply job apply. +# Not under .github/ in any letter case, with no empty, . or .. segment, and +# in a kept thread's scope: its path, any file in the +# plugins/

/evals// directory it sits in, or a test file in its +# directory. The same rule as .github/scripts/pr-unblock/thread-scope.mjs, +# which the promote step and the reply job apply. allowed() { - [[ ! ${1,,} =~ ^\.github(/|$) ]] && + [[ ! ${1,,} =~ ^\.github(/|$) && ! $1 =~ (^|/)\.{0,2}(/|$) ]] && jq -e --arg p "$1" ' def dir: sub("[^/]*$"; ""); (first($p | capture("^(?plugins/[^/]+/evals/[^/]+/)").c) // null) as $case diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh index 5af83363878..044d74e8df3 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh @@ -99,7 +99,9 @@ for path in plugins/p/evals/c/samples/y.json plugins/p/evals/c/samples/deep/z.js done for path in plugins/p/evals/other/samples/y.json plugins/p/skills/s/SKILL.md plugins/p/evals/x.json \ plugins/planning/surface/tests/test_round.py plugins/planning/test_round.py src/lib/a.test.js \ - plugins/planning/surface/helper.py plugins/planning/surface/test_round.sh test_root.py; do + plugins/planning/surface/helper.py plugins/planning/surface/test_round.sh test_root.py \ + plugins/p/evals/c/../../../../.github/workflows/x.yml plugins/p/evals/c/./y.json plugins/p/evals/c//y.json \ + /src/a.js ./src/a.js; do run check-paths "$path" >/dev/null 2>&1 assert_exit "check-paths refuses $path" 1 "$?" done From 07d2739f45f072bad1affafd80780ab48e0536f5 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 18 Oct 2026 06:08:57 -0400 Subject: [PATCH 3/5] fix(pr-pipeline): widen a kept thread's scope to its whole plugin directory A kept review thread's scope is now its path, any file in the plugins/

/ directory that path sits in, and a test file beside the path. This replaces the per-eval-case rule, which still refused real fixes: #8178 (plugins/planning/README.md, run 39117146913) on top of #8043, #8103 and #8142. .github/ stays refused. thread-scope.mjs and the feedback-thread.sh jq rule change together; tests, the contract, comments-ci.md, the convention CHANGELOG (2.11.4) and the source-control fragment are updated. Refs: #8109 Co-authored-by: Claude Opus 5.5 --- ...fix-pr-unblock-eval-case-scope-ed5fddbc.md | 2 +- .../scripts/pr-unblock/promote-scope.test.mjs | 6 ++- .../pr-unblock/publish-feedback.test.mjs | 5 +- .github/scripts/pr-unblock/thread-scope.mjs | 14 +++--- .../scripts/pr-unblock/thread-scope.test.mjs | 46 ++++++++----------- docs/conventions/pr-pipeline/CHANGELOG.md | 2 +- .../pr-pipeline/pr-run-activity.md | 4 +- .../pull-request/reference/comments-ci.md | 6 +-- .../pull-request/scripts/feedback-thread.sh | 12 ++--- .../scripts/feedback-thread.test.sh | 9 ++-- 10 files changed, 52 insertions(+), 54 deletions(-) diff --git a/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md b/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md index 12f1f0e3838..e644d640483 100644 --- a/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md +++ b/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- The `comments --ci` review-thread tool's `check-paths` and `record` now also allow any file in the `plugins/

/evals//` directory a kept thread's path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in the same directory as a kept thread's path. `.github/` stays refused. +- The `comments --ci` review-thread tool's `check-paths` and `record` now also allow any file in the `plugins/

/` directory a kept thread's path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in the same directory as a kept thread's path. `.github/` stays refused. diff --git a/.github/scripts/pr-unblock/promote-scope.test.mjs b/.github/scripts/pr-unblock/promote-scope.test.mjs index a31f1afa7d5..ab6a1ad957d 100644 --- a/.github/scripts/pr-unblock/promote-scope.test.mjs +++ b/.github/scripts/pr-unblock/promote-scope.test.mjs @@ -78,10 +78,12 @@ async function promote(files, { scope = readFileSync(SCOPE), sha256 } = {}) { const HOLD = /^No hold labels were read/; const OUT = /^The lane branch changes a path outside the kept threads' scope or under \.github\//; -test("in-scope changes pass the path check: the thread's path, its eval case, a sibling test", async () => { +test("in-scope changes pass the path check: the thread's path, its plugin's files, a sibling test", async () => { const failed = await promote([ { filename: "plugins/p/evals/c/case.yaml" }, { filename: "plugins/p/evals/c/samples/deep/new.json" }, + { filename: "plugins/p/evals/other/samples/new.json" }, + { filename: "plugins/p/README.md" }, { filename: "src/test_round.py" }, ]); assert.equal(failed.length, 1); @@ -89,7 +91,7 @@ test("in-scope changes pass the path check: the thread's path, its eval case, a }); for (const file of [ - { filename: "plugins/p/evals/other/samples/new.json" }, + { filename: "plugins/q/evals/c/samples/new.json" }, { filename: "src/lib/test_round.py" }, { filename: ".github/workflows/x.yml" }, { filename: "plugins/p/evals/c/x.json", previous_filename: ".github/x.yml" }, diff --git a/.github/scripts/pr-unblock/publish-feedback.test.mjs b/.github/scripts/pr-unblock/publish-feedback.test.mjs index 664f57c7307..c8a0255e125 100644 --- a/.github/scripts/pr-unblock/publish-feedback.test.mjs +++ b/.github/scripts/pr-unblock/publish-feedback.test.mjs @@ -422,11 +422,12 @@ for (const [label, compare] of [ }); } -test("a VALID fix that also adds a sample in its thread's eval case lands; a sibling case's sample does not", async () => { +test("a VALID fix that also adds a file in its thread's plugin lands; another plugin's file does not", async () => { const caseRun = JSON.stringify({ ...RUN, threads: [{ ...RUN.threads[0], path: "plugins/p/evals/c/case.yaml" }] }); for (const [sample, landed] of [ ["plugins/p/evals/c/samples/new.json", true], - ["plugins/p/evals/other/samples/new.json", false], + ["plugins/p/evals/other/samples/new.json", true], + ["plugins/q/evals/c/samples/new.json", false], ]) { const api = fakeApi({ compare: { status: "ahead", commits: [{ sha: FIX }], files: [{ filename: "plugins/p/evals/c/case.yaml" }, { filename: sample }] }, diff --git a/.github/scripts/pr-unblock/thread-scope.mjs b/.github/scripts/pr-unblock/thread-scope.mjs index 5ec4ba7b55e..578b4925711 100644 --- a/.github/scripts/pr-unblock/thread-scope.mjs +++ b/.github/scripts/pr-unblock/thread-scope.mjs @@ -6,17 +6,17 @@ export function isProtectedPath(path) { return /^\.github(\/|$)/i.test(String(path)); } -const EVAL_CASE = /^plugins\/[^/]+\/evals\/[^/]+\//; +const PLUGIN_DIR = /^plugins\/[^/]+\//; const TEST_FILE = /^(test_[^/]*\.py|[^/]*_test\.py|[^/]*\.(test|spec)\.[^/]+)$/; const dirOf = (path) => path.slice(0, path.lastIndexOf("/") + 1); // An empty, . or .. segment, so a leading or doubled / too. const BAD_SEGMENT = /(^|\/)\.{0,2}(\/|$)/; -// A kept thread's own path, any file in the plugins/

/evals// -// directory a kept thread's path sits in, or a test file in the same -// directory as a kept thread's path; never anything under .github/, and -// never a path that is not in normal form. +// A kept thread's own path, any file in the plugins/

/ directory a kept +// thread's path sits in, or a test file in the same directory as a kept +// thread's path; never anything under .github/, and never a path that is +// not in normal form. export function inThreadScope(path, threadPaths) { if (isProtectedPath(path) || BAD_SEGMENT.test(path)) { return false; @@ -25,8 +25,8 @@ export function inThreadScope(path, threadPaths) { return true; } const kept = [...threadPaths]; - const evalCase = EVAL_CASE.exec(path)?.[0]; - if (evalCase !== undefined && kept.some((p) => p.startsWith(evalCase))) { + const plugin = PLUGIN_DIR.exec(path)?.[0]; + if (plugin !== undefined && kept.some((p) => p.startsWith(plugin))) { return true; } const dir = dirOf(path); diff --git a/.github/scripts/pr-unblock/thread-scope.test.mjs b/.github/scripts/pr-unblock/thread-scope.test.mjs index be755d4b772..f77b9a96a3b 100644 --- a/.github/scripts/pr-unblock/thread-scope.test.mjs +++ b/.github/scripts/pr-unblock/thread-scope.test.mjs @@ -11,25 +11,28 @@ test("a kept thread's own path is in scope", () => { assert.equal(inThreadScope("plugins/p/evals/c/graders/x.md", kept), true); }); -test("a file in the same eval case as a kept thread's path is in scope, at any depth", () => { - assert.equal(inThreadScope("plugins/p/evals/c/samples/y.json", kept), true); - assert.equal(inThreadScope("plugins/p/evals/c/samples/deep/z.json", kept), true); +// plugins/planning/README.md is the file #8178's run 39117146913 refused. +test("any file in the same plugins/

/ directory as a kept thread's path is in scope, at any depth", () => { + for (const path of [ + "plugins/p/evals/c/samples/y.json", + "plugins/p/evals/other/samples/deep/z.json", + "plugins/p/evals/evals.json", + "plugins/p/skills/s/SKILL.md", + "plugins/p/README.md", + ]) { + assert.equal(inThreadScope(path, kept), true, path); + } + assert.equal(inThreadScope("plugins/planning/README.md", new Set(["plugins/planning/surface/round.py"])), true); }); -test("a sibling eval case, a skill file and an unnamed file stay out of scope", () => { - assert.equal(inThreadScope("plugins/p/evals/other/samples/y.json", kept), false); - assert.equal(inThreadScope("plugins/p/skills/s/SKILL.md", kept), false); - assert.equal(inThreadScope("src/b.js", kept), false); +test("another plugin's file and an unnamed file outside plugins/ stay out of scope", () => { assert.equal(inThreadScope("plugins/q/evals/c/samples/y.json", kept), false); + assert.equal(inThreadScope("plugins/pp/README.md", kept), false); + assert.equal(inThreadScope("src/b.js", kept), false); + assert.equal(inThreadScope("plugins/README.md", kept), false); }); -test("a file directly under evals/ is no case directory", () => { - const evalsFile = new Set(["plugins/p/evals/evals.json"]); - assert.equal(inThreadScope("plugins/p/evals/other.json", evalsFile), false); - assert.equal(inThreadScope("plugins/p/evals/c/case.yaml", evalsFile), false); -}); - -test("with no kept thread under evals/, an eval case file is refused", () => { +test("with no kept thread under the plugin, its files are refused", () => { assert.equal(inThreadScope("plugins/p/evals/c/samples/y.json", new Set(["src/a.js"])), false); }); @@ -48,18 +51,9 @@ test("a test file in the same directory as a kept thread's path is in scope", () } }); -test("a test file in another directory, or a same-directory file that is no test, stays out of scope", () => { - const surface = new Set(["plugins/planning/surface/round.py", "src/a.js"]); - for (const path of [ - "plugins/planning/surface/tests/test_round.py", - "plugins/planning/test_round.py", - "plugins/planning/other/test_round.py", - "src/lib/a.test.js", - "plugins/planning/surface/helper.py", - "plugins/planning/surface/testing.py", - "plugins/planning/surface/test_round.sh", - "test_root.py", - ]) { +test("outside plugins/, a test file in another directory, or a same-directory file that is no test, stays out of scope", () => { + const surface = new Set(["src/a.js"]); + for (const path of ["src/lib/a.test.js", "src/helper.js", "src/testing.js", "src/test_a.sh", "test_root.py"]) { assert.equal(inThreadScope(path, surface), false, path); } }); diff --git a/docs/conventions/pr-pipeline/CHANGELOG.md b/docs/conventions/pr-pipeline/CHANGELOG.md index e63870c9cfe..b4c8d8bb385 100644 --- a/docs/conventions/pr-pipeline/CHANGELOG.md +++ b/docs/conventions/pr-pipeline/CHANGELOG.md @@ -3,7 +3,7 @@ ## 2.11.4 - 2026-10-18 `pr-unblock` `address-feedback` widens a kept thread's scope from its path alone to also any file -in the `plugins/

/evals//` directory that path sits in, and a test file (`test_*.py`, +in the `plugins/

/` directory that path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in that path's directory ([#8109](https://github.com/melodic-software/claude-code-plugins/issues/8109)). The review-thread tool, the promote step and the reply job apply the same rule; the promote step imports the base's diff --git a/docs/conventions/pr-pipeline/pr-run-activity.md b/docs/conventions/pr-pipeline/pr-run-activity.md index 963b371d382..5f8c8f8c613 100644 --- a/docs/conventions/pr-pipeline/pr-run-activity.md +++ b/docs/conventions/pr-pipeline/pr-run-activity.md @@ -594,8 +594,8 @@ the base's `.github/scripts/pr-unblock/address-feedback.mjs` and `publish-feedba kept threads, checks paths, and records a reply: it refuses a disposition file outside the scratch directory or off the template, and a `VALID` reply whose fix is not on the lane branch or whose lane branch changes a file outside every kept thread's scope or any `.github/` path. - A thread's scope is its path, any file in the `plugins/

/evals//` directory its path - sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in its path's + A thread's scope is its path, any file in the `plugins/

/` directory its path sits in, and + a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in its path's directory. It posts nothing. The promote step then moves the PR branch only when every path the lane branch changes is in a kept thread's scope, none under `.github/`, in fewer than 300 files, judged by a copy of the base's `thread-scope.mjs` taken before the head checkout and diff --git a/plugins/source-control/skills/pull-request/reference/comments-ci.md b/plugins/source-control/skills/pull-request/reference/comments-ci.md index 3fcb9620db9..4d8e36a74a3 100644 --- a/plugins/source-control/skills/pull-request/reference/comments-ci.md +++ b/plugins/source-control/skills/pull-request/reference/comments-ci.md @@ -20,9 +20,9 @@ The system prompt names the tool's path. It is the only shell command this mode finding: follow it. It changes what the fix is, not what this mode may do: the path and `.github/` rules below still hold, and it runs no command. - `check-paths ...` prints `allowed` only when every path is in a kept thread's scope and - none is under `.github/`. A thread's scope is its path, any file in the - `plugins/

/evals//` directory its path sits in, and a test file (`test_*.py`, - `*_test.py`, `*.test.*`, `*.spec.*`) in its path's directory. + none is under `.github/`. A thread's scope is its path, any file in the `plugins/

/` + directory its path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, + `*.spec.*`) in its path's directory. - `record ` reads a disposition file from the scratch directory, checks it against the template, and records it. Exit 1 means the tool refused and recorded nothing: fix the file and record again. One record per thread. diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh index ef8f7c343d7..64d2b1d947b 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh @@ -45,18 +45,18 @@ repo=$(jq -er '.repository | select(test("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$"))' die 2 "the thread file names no repository" # Not under .github/ in any letter case, with no empty, . or .. segment, and -# in a kept thread's scope: its path, any file in the -# plugins/

/evals// directory it sits in, or a test file in its -# directory. The same rule as .github/scripts/pr-unblock/thread-scope.mjs, -# which the promote step and the reply job apply. +# in a kept thread's scope: its path, any file in the plugins/

/ +# directory it sits in, or a test file in its directory. The same rule as +# .github/scripts/pr-unblock/thread-scope.mjs, which the promote step and +# the reply job apply. allowed() { [[ ! ${1,,} =~ ^\.github(/|$) && ! $1 =~ (^|/)\.{0,2}(/|$) ]] && jq -e --arg p "$1" ' def dir: sub("[^/]*$"; ""); - (first($p | capture("^(?plugins/[^/]+/evals/[^/]+/)").c) // null) as $case + (first($p | capture("^(?plugins/[^/]+/)").c) // null) as $plugin | ($p | sub("^.*/"; "") | test("^(test_[^/]*\\.py|[^/]*_test\\.py|[^/]*\\.(test|spec)\\.[^/]+)$")) as $test | any(.threads[].path; . == $p - or ($case != null and startswith($case)) + or ($plugin != null and startswith($plugin)) or ($test and dir == ($p | dir)))' "$threads_file" >/dev/null } diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh index 044d74e8df3..abdbceb4f6c 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh @@ -87,19 +87,20 @@ assert_exit "check-paths refuses .github/ in any letter case" 1 "$?" run check-paths .github/workflows/ci.test.yml >/dev/null 2>&1 assert_exit "check-paths refuses a test file beside a .github/ thread path" 1 "$?" -# check-paths: the kept thread's eval case, and test files beside a thread path +# check-paths: any file in a kept thread's plugins/

/ directory, and test files beside a thread path setup jq '.threads += [{"id":"PRRT_e","path":"plugins/p/evals/c/case.yaml","root_comment_id":400,"comments":[]}, {"id":"PRRT_s","path":"plugins/planning/surface/round.py","root_comment_id":401,"comments":[]}]' \ "$STUB/threads.json" >"$STUB/t2" && mv "$STUB/t2" "$STUB/threads.json" for path in plugins/p/evals/c/samples/y.json plugins/p/evals/c/samples/deep/z.json \ + plugins/p/evals/other/samples/y.json plugins/p/skills/s/SKILL.md plugins/p/evals/x.json \ + plugins/planning/README.md plugins/planning/surface/helper.py plugins/planning/test_round.py \ plugins/planning/surface/test_round.py plugins/planning/surface/round_test.py src/a.test.js src/a.spec.ts; do run check-paths "$path" >/dev/null 2>&1 assert_exit "check-paths allows $path" 0 "$?" done -for path in plugins/p/evals/other/samples/y.json plugins/p/skills/s/SKILL.md plugins/p/evals/x.json \ - plugins/planning/surface/tests/test_round.py plugins/planning/test_round.py src/lib/a.test.js \ - plugins/planning/surface/helper.py plugins/planning/surface/test_round.sh test_root.py \ +for path in plugins/q/evals/c/samples/y.json plugins/pp/README.md plugins/README.md \ + src/lib/a.test.js src/helper.js src/test_a.sh test_root.py \ plugins/p/evals/c/../../../../.github/workflows/x.yml plugins/p/evals/c/./y.json plugins/p/evals/c//y.json \ /src/a.js ./src/a.js; do run check-paths "$path" >/dev/null 2>&1 From 970ba319778b068fe685fd7605915507a8cdcaaa Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 18 Oct 2026 07:07:29 -0400 Subject: [PATCH 4/5] fix(pr-pipeline): keep hooks, manifests, agent rules and code out of the plugin-wide scope The plugin-wide part of a kept thread's scope now leaves out anything under hooks/, .claude-plugin/ or a .claude/ directory, any CLAUDE.md or AGENTS.md, and any .sh, .py, .mjs, .js or .ts file. A thread's own path and a sibling test file stay in scope. thread-scope.mjs and feedback-thread.sh apply the same rule; tests cover the four recorded trips (#8043, #8103, #8142, #8178) and refusals for each excluded kind. Co-authored-by: Claude Opus 5.5 --- ...fix-pr-unblock-eval-case-scope-ed5fddbc.md | 2 +- .github/scripts/pr-unblock/thread-scope.mjs | 15 ++++-- .../scripts/pr-unblock/thread-scope.test.mjs | 49 +++++++++++++++++++ .../pr-pipeline/pr-run-activity.md | 5 +- .../pull-request/reference/comments-ci.md | 6 ++- .../pull-request/scripts/feedback-thread.sh | 9 ++-- .../scripts/feedback-thread.test.sh | 9 ++-- 7 files changed, 80 insertions(+), 15 deletions(-) diff --git a/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md b/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md index e644d640483..a696b9a58cf 100644 --- a/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md +++ b/.changes/source-control/fix-pr-unblock-eval-case-scope-ed5fddbc.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- The `comments --ci` review-thread tool's `check-paths` and `record` now also allow any file in the `plugins/

/` directory a kept thread's path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in the same directory as a kept thread's path. `.github/` stays refused. +- The `comments --ci` review-thread tool's `check-paths` and `record` now also allow a file in the `plugins/

/` directory a kept thread's path sits in, except one under `hooks/`, `.claude-plugin/` or a `.claude/` directory, a `CLAUDE.md` or `AGENTS.md`, or a `.sh`, `.py`, `.mjs`, `.js` or `.ts` file, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in the same directory as a kept thread's path. `.github/` stays refused. diff --git a/.github/scripts/pr-unblock/thread-scope.mjs b/.github/scripts/pr-unblock/thread-scope.mjs index 578b4925711..2aab10f4da2 100644 --- a/.github/scripts/pr-unblock/thread-scope.mjs +++ b/.github/scripts/pr-unblock/thread-scope.mjs @@ -7,15 +7,22 @@ export function isProtectedPath(path) { } const PLUGIN_DIR = /^plugins\/[^/]+\//; +// In the plugin directory but run or loaded as instructions by the harness or +// a shell, so out of the plugin-wide part: hooks, manifests, script or code +// files, and agent rules (CLAUDE.md, AGENTS.md, anything under .claude/). +const PLUGIN_CODE = + /^plugins\/[^/]+\/(hooks|\.claude-plugin)\/|\.(sh|py|mjs|js|ts)$|(^|\/)(claude|agents)\.md$|(^|\/)\.claude\//i; const TEST_FILE = /^(test_[^/]*\.py|[^/]*_test\.py|[^/]*\.(test|spec)\.[^/]+)$/; const dirOf = (path) => path.slice(0, path.lastIndexOf("/") + 1); // An empty, . or .. segment, so a leading or doubled / too. const BAD_SEGMENT = /(^|\/)\.{0,2}(\/|$)/; -// A kept thread's own path, any file in the plugins/

/ directory a kept -// thread's path sits in, or a test file in the same directory as a kept -// thread's path; never anything under .github/, and never a path that is +// A kept thread's own path, a file in the plugins/

/ directory a kept +// thread's path sits in that is not under hooks/, .claude-plugin/ or a +// .claude/ directory, not a CLAUDE.md or AGENTS.md, and not a .sh, .py, .mjs, +// .js or .ts file, or a test file in the same directory as a +// kept thread's path; never anything under .github/, and never a path that is // not in normal form. export function inThreadScope(path, threadPaths) { if (isProtectedPath(path) || BAD_SEGMENT.test(path)) { @@ -26,7 +33,7 @@ export function inThreadScope(path, threadPaths) { } const kept = [...threadPaths]; const plugin = PLUGIN_DIR.exec(path)?.[0]; - if (plugin !== undefined && kept.some((p) => p.startsWith(plugin))) { + if (plugin !== undefined && !PLUGIN_CODE.test(path) && kept.some((p) => p.startsWith(plugin))) { return true; } const dir = dirOf(path); diff --git a/.github/scripts/pr-unblock/thread-scope.test.mjs b/.github/scripts/pr-unblock/thread-scope.test.mjs index f77b9a96a3b..526f109b631 100644 --- a/.github/scripts/pr-unblock/thread-scope.test.mjs +++ b/.github/scripts/pr-unblock/thread-scope.test.mjs @@ -25,6 +25,55 @@ test("any file in the same plugins/

/ directory as a kept thread's path is in assert.equal(inThreadScope("plugins/planning/README.md", new Set(["plugins/planning/surface/round.py"])), true); }); +// Each refused path and the kept thread paths of the four recorded trips. +test("the four recorded trips are in scope", () => { + const trips = [ + // #8043, run 39100052790: an eval sample beside the thread's case. + ["plugins/session-flow/evals/rr-skill-fit-names-owner/samples/names-skill-and-plugin.json", ["plugins/session-flow/evals/rr-skill-fit-names-owner/case.yaml"]], + // #8103, run 39107762307: the thread is on case.yaml:18. + ["plugins/discipline/evals/ror-second-error-way/samples/second-way-named.json", ["plugins/discipline/evals/ror-second-error-way/case.yaml"]], + // #8142, run 39113747960: a .py file, in scope only as a sibling test. + ["plugins/planning/surface/test_round.py", ["plugins/planning/surface/round.py"]], + // #8178, run 39117146913: the PR's kept thread paths. + [ + "plugins/planning/README.md", + ["plugins/session-flow/skills/retro/SKILL.md", "plugins/planning/skills/draft-goal-condition/SKILL.md", "plugins/planning/.claude-plugin/plugin.json"], + ], + ]; + for (const [path, threads] of trips) { + assert.equal(inThreadScope(path, new Set(threads)), true, path); + } +}); + +test("hooks, manifests, agent rules and non-test code in the same plugin are refused", () => { + const named = new Set(["plugins/p/skills/s/SKILL.md"]); + for (const path of [ + "plugins/p/hooks/hooks.json", + "plugins/p/hooks/guard.md", + "plugins/p/.claude-plugin/plugin.json", + "plugins/p/scripts/run.sh", + "plugins/p/skills/s/scripts/helper.py", + "plugins/p/lib/x.mjs", + "plugins/p/lib/x.js", + "plugins/p/lib/x.ts", + "plugins/p/lib/X.SH", + "plugins/p/AGENTS.md", + "plugins/p/skills/s/CLAUDE.md", + "plugins/p/.claude/rules/r.md", + "plugins/p/.claude/settings.json", + ]) { + assert.equal(inThreadScope(path, named), false, path); + } +}); + +test("a thread's own path stays in scope whatever its type, and a sibling test of code stays in scope", () => { + for (const path of ["plugins/p/hooks/guard.sh", "plugins/p/.claude-plugin/plugin.json", "plugins/p/AGENTS.md"]) { + assert.equal(inThreadScope(path, new Set([path])), true, path); + } + assert.equal(inThreadScope("plugins/p/hooks/guard.test.mjs", new Set(["plugins/p/hooks/guard.mjs"])), true); + assert.equal(inThreadScope("plugins/p/scripts/test_run.py", new Set(["plugins/p/README.md"])), false); +}); + test("another plugin's file and an unnamed file outside plugins/ stay out of scope", () => { assert.equal(inThreadScope("plugins/q/evals/c/samples/y.json", kept), false); assert.equal(inThreadScope("plugins/pp/README.md", kept), false); diff --git a/docs/conventions/pr-pipeline/pr-run-activity.md b/docs/conventions/pr-pipeline/pr-run-activity.md index 61b14f372cb..1ea1b2f1da9 100644 --- a/docs/conventions/pr-pipeline/pr-run-activity.md +++ b/docs/conventions/pr-pipeline/pr-run-activity.md @@ -600,8 +600,9 @@ the base's `.github/scripts/pr-unblock/address-feedback.mjs` and `publish-feedba kept threads, checks paths, and records a reply: it refuses a disposition file outside the scratch directory or off the template, and a `VALID` reply whose fix is not on the lane branch or whose lane branch changes a file outside every kept thread's scope or any `.github/` path. - A thread's scope is its path, any file in the `plugins/

/` directory its path sits in, and - a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in its path's + A thread's scope is its path; a file in the `plugins/

/` directory its path sits in, except + one under `hooks/`, `.claude-plugin/` or a `.claude/` directory, a `CLAUDE.md` or `AGENTS.md`, + or a `.sh`, `.py`, `.mjs`, `.js` or `.ts` file; and a test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in its path's directory. It posts nothing. The promote step then moves the PR branch only when every path the lane branch changes is in a kept thread's scope, none under `.github/`, in fewer than 300 files, judged by a copy of the base's `thread-scope.mjs` taken before the head checkout and diff --git a/plugins/source-control/skills/pull-request/reference/comments-ci.md b/plugins/source-control/skills/pull-request/reference/comments-ci.md index 4d8e36a74a3..3381fa632e0 100644 --- a/plugins/source-control/skills/pull-request/reference/comments-ci.md +++ b/plugins/source-control/skills/pull-request/reference/comments-ci.md @@ -20,8 +20,10 @@ The system prompt names the tool's path. It is the only shell command this mode finding: follow it. It changes what the fix is, not what this mode may do: the path and `.github/` rules below still hold, and it runs no command. - `check-paths ...` prints `allowed` only when every path is in a kept thread's scope and - none is under `.github/`. A thread's scope is its path, any file in the `plugins/

/` - directory its path sits in, and a test file (`test_*.py`, `*_test.py`, `*.test.*`, + none is under `.github/`. A thread's scope is its path; a file in the `plugins/

/` + directory its path sits in, except one under `hooks/`, `.claude-plugin/` or a `.claude/` + directory, a `CLAUDE.md` or `AGENTS.md`, or a `.sh`, `.py`, `.mjs`, `.js` or `.ts` file; and a + test file (`test_*.py`, `*_test.py`, `*.test.*`, `*.spec.*`) in its path's directory. - `record ` reads a disposition file from the scratch directory, checks it against the template, and records it. Exit 1 means the tool refused and recorded nothing: fix diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh index 64d2b1d947b..e4e391debd2 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.sh @@ -45,15 +45,18 @@ repo=$(jq -er '.repository | select(test("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$"))' die 2 "the thread file names no repository" # Not under .github/ in any letter case, with no empty, . or .. segment, and -# in a kept thread's scope: its path, any file in the plugins/

/ -# directory it sits in, or a test file in its directory. The same rule as +# in a kept thread's scope: its path, a file in the plugins/

/ directory +# it sits in (not under hooks/, .claude-plugin/ or a .claude/ directory, not +# a CLAUDE.md or AGENTS.md, and not a .sh, .py, .mjs, .js or .ts file), or a +# test file in its directory. The same rule as # .github/scripts/pr-unblock/thread-scope.mjs, which the promote step and # the reply job apply. allowed() { [[ ! ${1,,} =~ ^\.github(/|$) && ! $1 =~ (^|/)\.{0,2}(/|$) ]] && jq -e --arg p "$1" ' def dir: sub("[^/]*$"; ""); - (first($p | capture("^(?plugins/[^/]+/)").c) // null) as $plugin + (if $p | test("^plugins/[^/]+/(hooks|\\.claude-plugin)/|\\.(sh|py|mjs|js|ts)$|(^|/)(claude|agents)\\.md$|(^|/)\\.claude/"; "i") + then null else first($p | capture("^(?plugins/[^/]+/)").c) // null end) as $plugin | ($p | sub("^.*/"; "") | test("^(test_[^/]*\\.py|[^/]*_test\\.py|[^/]*\\.(test|spec)\\.[^/]+)$")) as $test | any(.threads[].path; . == $p or ($plugin != null and startswith($plugin)) diff --git a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh index abdbceb4f6c..a674bb01b04 100755 --- a/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh +++ b/plugins/source-control/skills/pull-request/scripts/feedback-thread.test.sh @@ -87,19 +87,22 @@ assert_exit "check-paths refuses .github/ in any letter case" 1 "$?" run check-paths .github/workflows/ci.test.yml >/dev/null 2>&1 assert_exit "check-paths refuses a test file beside a .github/ thread path" 1 "$?" -# check-paths: any file in a kept thread's plugins/

/ directory, and test files beside a thread path +# check-paths: a file in a kept thread's plugins/

/ directory other than hooks, manifests, +# agent rules and non-test code, and test files beside a thread path setup jq '.threads += [{"id":"PRRT_e","path":"plugins/p/evals/c/case.yaml","root_comment_id":400,"comments":[]}, {"id":"PRRT_s","path":"plugins/planning/surface/round.py","root_comment_id":401,"comments":[]}]' \ "$STUB/threads.json" >"$STUB/t2" && mv "$STUB/t2" "$STUB/threads.json" for path in plugins/p/evals/c/samples/y.json plugins/p/evals/c/samples/deep/z.json \ plugins/p/evals/other/samples/y.json plugins/p/skills/s/SKILL.md plugins/p/evals/x.json \ - plugins/planning/README.md plugins/planning/surface/helper.py plugins/planning/test_round.py \ - plugins/planning/surface/test_round.py plugins/planning/surface/round_test.py src/a.test.js src/a.spec.ts; do + plugins/planning/README.md plugins/planning/surface/test_round.py plugins/planning/surface/round_test.py src/a.test.js src/a.spec.ts; do run check-paths "$path" >/dev/null 2>&1 assert_exit "check-paths allows $path" 0 "$?" done for path in plugins/q/evals/c/samples/y.json plugins/pp/README.md plugins/README.md \ + plugins/p/hooks/hooks.json plugins/p/.claude-plugin/plugin.json plugins/p/scripts/run.sh \ + plugins/p/AGENTS.md plugins/p/skills/s/CLAUDE.md plugins/p/.claude/rules/r.md plugins/p/lib/X.TS \ + plugins/planning/surface/helper.py plugins/planning/test_round.py \ src/lib/a.test.js src/helper.js src/test_a.sh test_root.py \ plugins/p/evals/c/../../../../.github/workflows/x.yml plugins/p/evals/c/./y.json plugins/p/evals/c//y.json \ /src/a.js ./src/a.js; do From d52de51f2637911dceceefba1682e576b4e202e4 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:17:38 -0400 Subject: [PATCH 5/5] fix(pr-pipeline): drop tracker back-reference from thread-scope test comment-hygiene in lint-repo flagged the issue-reference comment on line 1. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/pr-unblock/thread-scope.test.mjs | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/scripts/pr-unblock/thread-scope.test.mjs b/.github/scripts/pr-unblock/thread-scope.test.mjs index 526f109b631..87582f7cd3a 100644 --- a/.github/scripts/pr-unblock/thread-scope.test.mjs +++ b/.github/scripts/pr-unblock/thread-scope.test.mjs @@ -1,4 +1,3 @@ -// Cases from issue #8109's Verification section. import assert from "node:assert/strict"; import { test } from "node:test";