Skip to content

perf(ci): select only the suites a change runs or reads - #6059

Merged
kyle-sexton merged 33 commits into
mainfrom
perf/precise-test-selection
Oct 3, 2026
Merged

kyle-sexton merged 33 commits into
mainfrom
perf/precise-test-selection

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

No related issue: wave 3 PR 2 of the CI performance program (precise test selection). Refs #3932, #6021.

Summary

scripts/affected-tests.sh now selects the suites a change runs or reads, in the change's own language, instead of every suite that mentions a file name anywhere, every shell suite of a touched plugin, and three always-run suites. Over the 895 pull requests merged to main in the 7 days to 9671ece it selects 9,534 suites where main's selector selects 34,365, within 5% of the design model's 9,095, and both real main breaks from the window are still selected. Every Node suite it selects also runs: a Node suite that CI runs through a sibling .test.sh brings that wrapper (R9).

Fix

Rules (full text in the script header):

  • Same-language edges (R3). A file in the changed file's language that names it on a code line is a dependent, transitively.
  • Another language counts only where the line runs or loads the file (R4): an interpreter or process API on the line, matched as a word (the .sh of x.sh is not one), or a path to the file. A chain takes at most one such transition. A changed data file (C#, Markdown, YAML, ...) reaches code of any language that names it without spending the transition.
  • Comment lines never count, in suites and in code. # shellcheck source= and JSDoc @import / import() still count.
  • Manifests select no suite through a mention. plugin.json, marketplace.json, hooks.json, settings.json, package.json, package-lock.json, CHANGELOG.md and LICENSE reach a suite only through R1, R2 or a declared scope; their gates own them.
  • Ambiguous names count only when the mention resolves. This covers basenames two or more files carry, plus README.md, SKILL.md, AGENTS.md, CLAUDE.md and index.md. A mention resolves when it comes from the file's own directory; when it is a bare name from a directory above the file with no other file of that name below; when it ends in the file's shortest unique path suffix of two or more components; or when it is a path relative to a directory below the root that holds both files and has a directory in it ($PLUGIN_DIR/skills/interview/SKILL.md). A name-only path ($SKILL_DIR/SKILL.md, $T/README.md) or a root-relative one ($ROOT/.github/workflows/ci.yml) does not resolve, because tests build those paths under temporary directories; the suites the strace saw reading such files declare them.
  • No Python import rule. import foo does not name foo.py, as in design rules S1-S9. A module that only an import reaches is unmapped and falls back to the Python corpus (S9).
  • Declared scopes replace R8's whole-plugin rule and the always list (S7). A suite that reads files it never names declares them in its leading comment block, before any code or docstring: # test-scope: <glob> [<glob>...], one or more lines. 87 suites carry 132 globs, seeded from an strace of every suite. scripts/affected-tests.test.sh also declares the live files its LIVE cases find by glob (the github advise and planning interview skill bodies, the autonomy reference docs), so renaming or deleting one runs it; its reference-YAML case runs in a fixture on probe files, so that YAML stays unmapped. A changed suite whose glob matches no file fails the run. scripts/affected-tests-always.txt is deleted, and --with-always is accepted and does nothing.
  • A wrapped Node suite brings its wrapper (R9). A selected <stem>.test.js or <stem>.test.mjs whose directory holds <stem>.test.sh selects that wrapper too, after every other rule and before --shard. CI runs such a suite only through the wrapper (scripts/run-outside-node-suites.sh reports it OWNED and runs nothing), and the walk stops at a reached suite, so a change reaching exec-bash.resolver.test.mjs through lib/exec-bash.mjs used to select the suite, not the wrapper, and CI ran neither.
  • An unmapped file runs only its own language's suites. --unmapped-corpus keeps the report, adds that language's corpus and exits 4. Wiring it into ci.yml is PR 3's job.
  • No-suite list. plugins/*/evals/* replaces the eval fixture entries. The Python module entries main listed (plugin_cache_versions.py, discover.py, docs_crosscheck.py, the session_bridge.py copies) are removed, so a change to one is unmapped and falls back to the Python corpus instead of selecting nothing. plugins/performance/lib/spawn_noise.py (a copy nothing imports) is added.
  • --replay <range> [--against <ref>] reruns the selector on each first-parent commit against its parent in a scratch clone, with this tree's no-suite list and declared scopes (handed to older commits through AFFECTED_TESTS_SCOPES), and, with --against, prints only the suites the two selectors disagree on, <ref> using its own headers.
  • Releases. The 30 plugins whose suites gained a header get a patch bump and a CHANGELOG entry naming those suites; nothing they run changed.

Verification

Replay: 7 days of merged pull requests

Every first-parent commit on main from 2026-09-26 to 9671ece (900; 895 change a file), selected against its parent three ways: main's selector at 9671ece, this PR's selector, and the design's model (selmodel.py, the script that produced the design's section 3 figures) re-run on the same commits.

measure (895 PRs) main this PR design model
suites per PR, p50 / p90 / p95 / max 22 / 83 / 142 / 523 5 / 24 / 33 / 281 4 / 23 / 31 / 273
suites selected, total 34,365 9,534 9,095
total with the language-scoped unmapped fallback (S9, PR 3) 38,094 15,290 27,360
total as CI runs it today (unmapped: whole shell corpus) 43,852 21,686 32,445
shell suites selected 32,420 7,946 7,352
PRs with an unmapped file 22 26 48
PRs with no shell change that start shell suites 491 of 491 (10,269) 351 of 491 (1,560) 312 of 491 (1,361)
PRs changing no code that start any suite 356 of 356 (7,181) 233 of 356 (974) 203 of 357 (856)
PRs selecting no suite 0 127 157

The design's section 3 figures (p50 3, p95 28, 7,348 total, 13,371 with the fallback) came from a different window, 826 PRs to 2026-10-02 20:52Z. The same model gives the last column on this window, so that column is the target the 5% bar applies to.

Design targets vs measured

this PR design model difference
suites selected, total 9,534 9,095 +439 (+4.8%)
p50 / p90 5 / 24 4 / 23 +1 / +1
p95 33 31 +2 (+6.5%)
max 281 273 +8 (+2.9%)

The total is within the 5% bar. p50 is 1 suite over the model and p95 2, and the declared scopes account for both: selections only this PR makes are 713 through a declared scope, 41 wrappers (R9), 31 through a mention and 16 siblings; selections only the model makes are 362 (its guessed plugin scans 190, its interpreter test matching the .sh of a file name 116, siblings 56). Without the 713 declared-scope selections, p95 would be 30. The declared scopes are what the strace saw the suites read, plus the live files scripts/affected-tests.test.sh reads by glob (18 of the 713: edits to the github advise and planning interview skill bodies and the autonomy reference docs).

What dropping the Python import rule cost, against the previous head: 903 (PR, suite) selections over 75 suites, 264 of which main also made; the strace saw the suite read a changed file in 60 of them. Where nothing else maps the module (discover.py, docs_crosscheck.py, plugin_cache_versions.py), the change is unmapped and the S9 fallback runs the Python corpus. Where the module maps through a sibling or a mention (hygiene.py, destructive_guard.py), the suites that only import it are not selected; the design accepts that and catches it with the twice-daily full run and the trace audit (PR 4).

The C# fixture case

plugins/code-metrics/scripts/fixtures/sources/CmSample.cs goes from 14 suites to 7. dispatch.test.sh, audit-complexity.test.sh and audit-type-debt.test.sh name the file. audit-coverage.test.sh, audit-duplication.test.sh and audit-size.test.sh declare plugins/code-metrics/scripts/fixtures/*, which the strace saw them read. setup-check.test.sh sits beside setup-check.sh, which copies the plugin's scripts/. The design predicted 4; the three declared scopes make the difference.

The two real main breaks are still selected

  • 88dd7e1 selects plugins/github/github.test.sh (its header declares plugins/github/*) and plugins/planning/tests/interview-defenses.test.sh.
  • e544012 selects plugins/planning/tests/interview-defenses.test.sh: $PLUGIN_DIR/skills/interview/SKILL.md resolves to the changed file.

The suite also pins both against the live tree.

Trace audit of every drop

Every suite ran under strace to record its file reads. Of 25,352 (PR, suite) pairs this PR drops against main, over 700 suites, the strace saw the suite read a changed file in 1,004 pairs over 53 suites:

  • Manifest identity reads: install_state, sync-run, the planning surface/watch suites and the *-format hook suites read a plugin's name or version from plugin.json, or Node reads the root package.json while resolving modules. The root package files are pins that PR 3 routes to the Node lanes (S8).
  • Live gates CI also runs as whole-tree gate steps: check-loop-lane-floor-drift, check-fixture-git-isolation, check-summary-reader-parity.
  • scripts/affected-tests.test.sh: its LIVE cases run the selector, whose git grep reads every file.
  • Plugin copies and walks: abort-boundary, check-guardrails-ps-differential and test_kill_switch_probe.py read a README or changelog, which does not change what they test.
  • Python imports: the 60 pairs above, where a test imports a changed module that something else maps.
  • Relations only today's tree has, since the trace ran on today's tree and each PR replays on its own: interview-defenses.test.sh began naming context/surface.md after 7 of its PRs. check-prerequisite-probes.test.sh has been deleted since.

The full list, each suite with its PR count, its trace verdict and main's reasons for selecting it, is in this comment (57 KB, too large for this body).

Every selected Node suite runs (R9)

The figures above are the previous head's per-PR selections with R9 applied. A real replay of this head's selector over the same 900 commits, with the same inputs as the previous head's replay, removes nothing and adds exactly those 41 (PR, suite) selections over 33 PRs, each the .test.sh wrapper of a Node suite already selected: lib/exec-bash.resolver.test.sh 31, plugins/guardrails/hooks/exec-bash.resolver.test.sh 7, plugins/autonomy/skills/setup/scripts/resolve-prerequisites.fixtures.test.sh 3. No PR's exit code or unmapped set changes; three --unmapped-corpus runs also gain the wrappers of the Node suites their corpus adds.

Selected Node suites that CI runs nowhere (outside the registered packages and the four sub-projects, with a wrapper that is not selected): 41 (PR, suite) pairs over 33 PRs at the previous head, 5 of which main covered by selecting the wrapper; 0 at this head.

On today's tree, lib/exec-bash.mjs plus plugins/guardrails/hooks/exec-bash.mjs selects both exec-bash.resolver.test.mjs suites and both wrappers, and run-outside-node-suites.sh --paths on the two Node suites (CI's exit-3 branch) reports each OWNED by its wrapper and exits 0. The workflow scripts of review, planning, testing, discovery and multi-agent and the autonomy fixture-harness.mjs and resolve-prerequisites.mjs select 9 wrapped Node suites, each with its wrapper.

Local runs

WSL (Ubuntu 26.04) at the head, main merged:

Related

🤖 Generated with Claude Code

kyle-sexton and others added 7 commits October 3, 2026 01:56
…eclared scopes

The selector now takes a reference across languages only where the line runs
or loads the file, ignores comment-only mentions in suites too, resolves
ambiguous and structural basenames to the file they mean, follows Python
imports, and replaces the whole-plugin rule and the always-run list with
test-scope headers on the suites that scan a directory. An unmapped file can
select only its own language's corpus (--unmapped-corpus, exit 4), and
--replay shows a selector change's effect on recent main commits.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ection

# Conflicts:
#	scripts/affected-tests-no-suite.txt
…re scanner scopes

Ambiguous and structural basenames resolve through a path relative to a
directory holding both files and through a bare name that is unique below its
namer; a shell suite runs a same-directory sibling it names bare; Python import
lines name the module they load. Suites the strace trace shows reading a
directory they never name declare it in a test-scope header.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 3, 2026 06:57
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-03T07:02:24.024909Z 20231e0 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (per custom instructions)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

View job run

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 10m 16s —— View job


Security review in progress

  • Load /review:security-review instructions
  • Gather PR diff (git diff origin/main...HEAD)
  • Check skip gate — not eligible to skip: this PR rewrites scripts/affected-tests.sh, the logic that decides which CI test suites run for a change, which is itself a security-relevant trust boundary (an under-selection bug lets a change ship without the suite that would have caught a regression)
  • Review changed files for security-relevant issues
  • Check for instruction-surface deletions against protected-class register
  • Adversarial verification pass on candidate findings
  • Post findings (inline comments) or confirm none found
  • Final summary

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 9m 27s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (per custom instructions)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20231e0e59

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/affected-tests.sh Outdated
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review has reviewed this pull request through 71ed6fc; a later push is reviewed from there.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 57s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 (per custom instructions)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Security review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 20231e0, limited to plugins/speech/scripts/speech.test.sh (the diff in .claude-lane/incremental.diff). Every other file in the PR was reviewed at 20231e0 and is unchanged since.

Change reviewed: speech.test.sh#L2 — the # test-scope: header comment drops the glob plugins/speech/skills/*/scripts/*.

Finding: none. This is a test-selection metadata comment consumed by scripts/affected-tests.sh, not executable code. I verified the removed glob matches zero paths in the repository — plugins/speech/skills/{narrate,setup,check} each contain only SKILL.md and evals/, no scripts/ subdirectory — so the change has no effect on which files trigger this test suite. It is exactly the dead-glob cleanup the commit message (fix(speech): drop a test-scope glob that matches no file) describes.

I also checked this against the instruction-surface-deletion lens (narrowing a standing rule/guardrail): a test-scope comment isn't a CLAUDE.md/AGENTS.md/rules-file/hook guardrail, and even under that lens a glob that already matched nothing removes no enforcement — no test coverage is lost, so there's no exploit path to report.

No inline comments were needed; no other security issues found in this incremental scope.

kyle-sexton and others added 2 commits October 3, 2026 03:19
Every change under plugins/<p>/ needs a version bump and a release entry, and
the 29 plugins whose suites declare a scope took about 240 bumps on main in a
day, so headers would conflict on nearly every merge. scripts/affected-tests-
scopes.txt holds the same declarations: one `<suite> <glob>...` line each, an
entry naming no suite failing every run, and a glob matching no file failing
the run that changes the list.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 8m 51s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (per custom instructions)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Working through the scripts/affected-tests.sh rewrite (dense bash+awk, ~1900 diff lines) and verifying the Codex bot's from . import finding by hand-tracing the awk. An independent review pass is running in the background for adversarial verification.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 55s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 29fb678)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Security review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 29fb6785fbaf19a4f7e79fd08255093c94d2c97a, read from .claude-lane/incremental.diff: docs/ci-runner-routing.md, scripts/affected-tests-scopes.txt, scripts/affected-tests.sh, scripts/affected-tests.test.sh. Every other file in the PR (including the large token_hits/py_hits rewrite in affected-tests.sh that Codex commented on) was already present at 29fb678 and reviewed then.

What changed: this slice replaces the per-suite # test-scope: <glob> header convention (R8) with a single centralized list, scripts/affected-tests-scopes.txt, parsed with the existing read_list::into --comments inline helper. The old scope_table/check_declarations functions in affected-tests.sh are replaced by build_tree_index's new list-reading loop and a new check_scope_globs (L992-L1017).

Trust-boundary check (R8 under-selection path): the concern this lane cares about for this file is a selector that silently under-selects, letting a change ship without the suite that would catch it.

  • The new validation at L517-L521 fails closed (exit 2) when a list entry names something that isn't a real suite path — strictly stronger than before, where a stale entry could only be caught if that suite file itself changed.
  • check_scope_globs now runs only when the scopes list itself is the changed file ([[ "$f" == "$SCOPES_LIST" ]], L1138), rather than the old trigger of "a changed file is a suite path." This is a faithful move of the same check to follow where the data now lives (scopes moved from scattered suite headers to one file), not a narrowing: both the old and new code only validate glob staleness when something touching that data is in the diff — a glob going stale because an unrelated file it matched was deleted elsewhere was already unchecked under the old header-based scheme too.
  • Parsing (read -r -a words <<<"$entry", [[ "$f" == $glob ]]) uses word-splitting and bash glob matching only — no eval, no command substitution of untrusted content, so no injection path from this (reviewed, human-authored) list file.

Finding: none. This slice is a behavior-preserving refactor (centralizing scope declarations) with one incidental strengthening (fail-closed validation of unknown suite names in the list) and no new under-selection or injection surface introduced.

Checked this diff against the instruction-surface-deletion lens too: nothing here touches a CLAUDE.md/AGENTS.md/rules file, hook allowlist, or similar standing guardrail — it's internal selector logic and its own doc comment, both updated consistently.

No inline comments were needed.

@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review has reviewed this pull request through 71ed6fc; a later push is reviewed from there.

kyle-sexton and others added 2 commits October 3, 2026 03:39
`$PLUGIN_DIR/README.md` spells only the basename after a variable, so the
selector matched it against every README.md whose path relative to a
common ancestor is `README.md`, including the repository root's. A change
to the root README.md then selected every suite that reads its own
plugin's README. A file of that name between the naming file and the
candidate now claims the mention.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 8s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 2473daa)
  • Read .claude-lane/incremental.diff and review the actual slice
  • Post summary comment with findings (or confirm none found)

Security review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 2473daa1c4436a59bf2198e362d7291069bd6079, read from .claude-lane/incremental.diff: scripts/affected-tests.sh and scripts/affected-tests.test.sh. Everything else in the PR (including the token_hits/py_hits rewrite and the Codex from . import finding) was already present at 2473daa and is out of this lane's scope.

What changed: the resolves() helper inside token_hits() — scripts/affected-tests.sh#L641-L663 — gains "nearest-file" shadowing for a path token that spells only a variable name plus a bare filename (e.g. $PLUGIN_DIR/README.md, $REPO_ROOT/README.md). It now tracks a claimed flag while walking ancestors of the referencing file's directory outward: once an ancestor directory is found to contain any file sharing the target's basename other than the target itself, that nearer file "claims" the name and blocks a farther-out candidate of the same basename from resolving. scripts/affected-tests.test.sh adds a matching regression test (root README.md vs. plugins/alpha/README.md).

Trust-boundary check (R8 under-selection path): the concern for this file is a selector that silently under-selects, letting a change ship without the suite that would have caught it.

  • I hand-traced resolves() against the new test's own scenario and an adversarial variant (a suite several directories below an intermediate plugin that itself owns a same-named file, referencing $REPO_ROOT/<name>). The claimed flag is monotonic (set, never cleared) and is checked before being updated for the current ancestor, so a directory containing the target itself is never mistakenly treated as shadowed by its own file.
  • Diffing against the prior version (the - lines this replaces): the old resolves() had no shadowing check at all — a bare .../<name> token would resolve to every file sharing that basename anywhere in an ancestor chain, i.e. it was already over-selecting rather than under-selecting for ambiguous names. The new logic narrows that to the nearest one, which is a precision fix, not a new gap: it can only make an already-over-broad match more exact, never drop the one correct match (the directory holding the real target is still checked, unshadowed, before any farther ancestor is considered).
  • This remains pure path-string matching over the repo's own tracked file list (here, built from git ls-files-derived allf) — no eval, no command substitution of grep/text content, so no injection surface from this change.

Finding: none.

Checked this slice against the instruction-surface-deletion lens too: it touches no CLAUDE.md/AGENTS.md/rules file/hook — it's internal selector logic and its own doc comment, updated together with a new test that pins the fixed behavior.

No inline comments needed for this incremental slice.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 27s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 2473daa)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Code review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 2473daa1c4436a59bf2198e362d7291069bd6079, read from .claude-lane/incremental.diff: scripts/affected-tests.sh and scripts/affected-tests.test.sh. That slice is commit 66bb5ab ("resolve a name-only path to the nearest file of that name"). Every other file/line in the PR (including the larger token_hits/py_hits rewrite, and the Codex from . import P2 comment on py_hits) was already present at 2473daa and is out of scope for this pass.

What changed: resolves() in affected-tests.sh#L641-L664 gains a claimed flag. Previously, a path token that only spells a structural basename via a directory variable ($PLUGIN_DIR/README.md) could resolve to any ancestor directory's same-named file reachable by climbing from the namer toward the repo root — including one farther away than a closer file of the same name. The fix walks from the namer's directory upward one level at a time and sets claimed = 1 the first time it passes a real file of that basename that isn't the current candidate t (L660); once claimed, a bare-basename match at a farther ancestor is rejected (L658), while a token that already spells more than the bare name (index(rel, "/") true) still matches unconditionally, since that's unambiguous regardless of nearer files.

Verification done: I hand-traced resolves() against the new test fixture in affected-tests.test.sh#L1786-L1803 (root README.md + plugins/alpha/README.md, referenced via $PLUGIN_DIR/README.md and $REPO_ROOT/README.md from plugins/alpha/tests/readme.test.sh and scripts/zz-root-readme.test.sh respectively):

  • Changing the plugin's README.md resolves only to the plugin's own test, not the root one — claimed is correctly set to 1 when the climb from plugins/alpha/tests/ passes plugins/alpha/README.md (a real, different file than the root target), blocking the farther root match.
  • Changing the root README.md resolves only to the root test — the climb from scripts/ never passes through plugins/alpha/, so claimed stays 0 and the root match succeeds.
  • The one intervening directory with no README of its own (plugins/alpha/tests/) correctly does not claim (the (a b) in here check only fires for a file that actually exists), so the climb isn't blocked prematurely.
  • claimed is scoped as an awk function-local, reinitialized per call, so there's no state leakage between distinct (namer, pt, t) evaluations.

Finding: none. This is a narrow, well-targeted fix for a real under-selection bug (a farther same-named file could previously shadow a nearer one, or vice versa depending on evaluation order), with a regression test that exercises both directions of the ambiguity. The doc-comment update in the AMBIGUOUS NAMES section (L139) accurately describes the new behavior.

No inline comments were needed for this slice.

kyle-sexton and others added 3 commits October 3, 2026 04:04
…ew selector picks none

`--replay --against` split its two selections with `FNR == 1 { side++ }`.
An empty new selection has no first line, so the old selection was read
as the new one and every suite <ref> ran was printed as added. The split
now keys on the file name.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ection

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ered by the drift step

The copy of lib/spawn_noise.py that sync-shared-copies.sh writes into the
performance plugin has no importer in code, so once the selector stopped
treating a plugin directory as one scope it mapped to no suite. It
changes only with its canonical, whose fan-out selects harness-ops's
spawn_noise suites, and the shared-copies --check step fails a copy that
drifts.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 6m 19s —— View job


Code review (/review:code-review)

Scope: this PR's true diff is git diff origin/main HEAD restricted to the 7 files in <changed_files> (+1310/−975, matching the PR's reported stats exactly). The branch's own merge commit (f57f6bf) pulled in an unrelated, already-landed rename from PR #6021 (changes→scope, lint-2→lint-shell, etc.); that noise is not this PR's content and I excluded it.

Of that diff, every prior review pass in this thread already covered the token_hits/py_hits rewrite, the affected-tests-scopes.txt (R8) refactor, and the resolves() nearest-file fix, each with no confirmed finding. The two commits not covered by any prior pass (fc49d42, cd900f3) are genuinely new since the last review checkpoint (21c56641a), so I reviewed those directly:

  • fc49d42's fix to the --replay --against diff awk (scripts/affected-tests.sh run_replay, ~L1233) replaces a FNR == 1 { side++ } file-boundary trick with FILENAME == ARGV[1]. I hand-traced the old code: when the new selector's selection for a commit is empty (0 lines), FNR == 1 never fires while reading that empty file, so the increment that was meant to mark "now reading file 1" instead fires on file 2's first line, mislabeling every one of the old selector's picks as the new selector's picks and reporting a dropped suite as + added instead of - dropped. FILENAME == ARGV[1] sidesteps this entirely since it keys off the actual filename rather than line-count side effects. The new affected-tests.test.sh case (--replay --against lists a commit's suites as dropped when this selector picks none) exercises exactly this empty-new-selection path and would fail under the old awk. Correct fix, correctly tested.
  • cd900f3's plugins/performance/lib/spawn_noise.py no-suite entry is consistent with scripts/shared-copies.txt (which lists it as a copy of lib/spawn_noise.py, same as the harness-ops copy) and with the stated rationale (shared-lib fan-out + drift check cover it).

On the open Codex P2 finding (from . import helper resolving to the wrong module in py_hits, scripts/affected-tests.sh#L793): I hand-traced the current awk against from . import helper and from ..pkg.sub import helper. In the current code, when the leading-dot strip empties base, the per-name loop falls back to cand(P, base == "" ? y : base "." y), i.e. cand(P, "helper") — so the module name is resolved correctly, not "from". This finding does not reproduce against cd900f3; it appears to predate a later revision of py_hits than the one Codex reviewed (20231e0). No action needed.

Cross-file consequence outside this PR's diff (reported here since no changed line carries it): this PR deletes scripts/affected-tests-always.txt and makes --with-always a no-op (every live-tree suite now declares itself in scripts/affected-tests-scopes.txt under R8 instead). .github/workflows/ci.yml isn't touched by this PR, but its comment at ci.yml#L2341 still says the live-tree suites "ride every selection through --with-always (scripts/affected-tests-always.txt)" — both the flag's behavior and the file it names are gone after this merges. Worth a follow-up line edit (in this PR, in #6021, or a quick fixup after whichever lands second), since the PR body already tracks that file's deletion as a point of coordination between the two PRs.

Finding: none that rises to this lane's bar (high-confidence correctness/maintainability issue a senior reviewer would block on). No inline comments posted.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 4s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 21c56641)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Security review (incremental scope)

Scope: the last review on this PR covered through 21c56641a46096fb2bf24dfbcbfe76a2247f3c8b. Since then the only new content is two commits, fc49d42 and cd900f3 (the intervening f57f6bf is a merge of origin/main with no content of its own — confirmed via git show --stat on each commit individually, since the raw commit-range diff is inflated by main's unrelated history landing through the merge).

fc49d42 — scripts/affected-tests.sh#L1228: fixes the --replay --against diff-summary awk to key the "new vs. old selection" split on FILENAME == ARGV[1] instead of FNR == 1 { side++ }, so a commit the new selector maps to zero suites is reported as all-dropped rather than (wrongly) all-added. This is a developer-facing diagnostic path (affected-tests.sh --replay), operates only on two fixed internal work-dir paths ($WORK_DIR/new.sel, $WORK_DIR/old.sel), and introduces no new input, no eval, no injection surface — a correctness fix with no security-relevant trust-boundary change.

cd900f3 — scripts/affected-tests-no-suite.txt#L148-L154: adds plugins/performance/lib/spawn_noise.py (a generated copy with no code importer) to the declared-no-suite allowlist, with a comment explaining the copy's drift is caught by the separate shared-copies --check step rather than by suite mapping. Checked against the instruction-surface-deletion lens: this is test-selection metadata, not a CLAUDE.md/AGENTS.md/rules file/hook allowlist, and it's additive (declaring an existing gap explicitly, not removing an existing check) — the drift mechanism it names still runs. No finding.

Result: no security issues found in this incremental slice. Everything else in the PR (the token_hits/py_hits rewrite, the scopes-list centralization, the resolves() nearest-file fix, etc.) was already reviewed in prior passes on this PR at 20231e0, 29fb678, 2473daa, and 21c56641, and is unchanged since.

No inline comments were needed for this slice.

…_base

scripts/lib/gate-entry.test.sh bans a hand-rolled `rev-parse --verify`
base-ref predicate outside lib/changed-files.sh; the replay's read of
<ref>'s declared scopes now validates the ref with the shared helper.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 38s —— View job


Claude is reviewing this PR

  • Gather context (PR body, HEAD SHA, lane header, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 0b339b5)
  • Read .claude-lane/incremental.diff and review the actual slice
  • Post summary comment with findings (or confirm none found)

Security review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 0b339b51982c0510fcd4a17390532c5552052f64, read from .claude-lane/incremental.diff: a single one-line change in scripts/affected-tests.sh (commit 40f1938, "resolve the --against ref through changed_files::verify_base"). Everything else in the PR was reviewed at 20231e0, 29fb678, 2473daa, 21c56641, cd900f3, 6dfc8f3, 469cdfb and 0b339b5 in prior passes on this thread.

What changed — scripts/affected-tests.sh#L1133, inside run_replay's --against <ref> handling:

-    if ! against_sha="$(git rev-parse --verify -q "$against_ref^{commit}")" ||
+    if ! changed_files::verify_base "$against_ref" || ! against_sha="$(git rev-parse "$against_ref")" ||

The commit message explains the motivation precisely: scripts/lib/gate-entry.test.sh#L330 bans a hand-rolled rev-parse --verify "<ref>^{commit}" base-ref predicate anywhere outside scripts/lib/changed-files.sh, and this was the one surviving offender. changed_files::verify_base is defined as exactly git rev-parse --verify --quiet "${1}^{commit}" >/dev/null 2>&1, i.e. the same predicate, just centralized.

Trust-boundary check (the under-selection/injection path this lane cares about):

  • $against_ref is a developer-supplied CLI argument to affected-tests.sh --replay --against <ref>, a local diagnostic tool — not data from an untrusted PR body, issue, or tracker item. No new input source is introduced.
  • Both changed_files::verify_base and the new git rev-parse "$against_ref" pass the ref as a single argv element to git, never through eval or shell interpolation into a command string — no injection surface, same as before.
  • I checked for a behavioral gap from splitting one call into two: the old code resolved "$against_ref^{commit}" (peels an annotated tag to its commit object); the new code verifies the same peeled form via changed_files::verify_base but then separately resolves the unpeeled "$against_ref" for against_sha. For an annotated tag this means against_sha would be the tag object's own SHA rather than the commit SHA. This does not change behavior in practice: git checkout --detach <sha> (the very next command, L1134) accepts a tag object and peels it to the commit automatically when setting HEAD, which is standard git behavior for anything that consumes a commit-ish (as opposed to rev-parse's raw-object semantics). For the actual callers of --against (branch names and commit SHAs, per the PR body's replay usage), the two forms are identical anyway.
  • No TOCTOU concern: this runs sequentially in a local scratch clone the tool itself created for one developer's replay session, not against shared/mutable state another actor could race.

Finding: none. This is a narrow, well-motivated consistency fix (routing the only surviving hand-rolled verify predicate through the shared helper the repo's own lint gate requires) with no new input source, no injection surface, and no observed behavioral regression for this tool's actual ref inputs.

Checked against the instruction-surface-deletion lens too: this slice touches no CLAUDE.md/AGENTS.md/rules file or hook allowlist — it's one line of internal selector/replay tooling.

No inline comments were needed for this slice.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 6s —— View job


Code review (incremental scope)

  • Gather context (PR body, lane header, HEAD SHA, CLAUDE.md/AGENTS.md instructions)
  • Read .claude-lane/incremental.diff (scope: scripts/affected-tests.sh only, since 0b339b5)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Scope: per the lane header, this pass covers only what changed since 0b339b51982c0510fcd4a17390532c5552052f64, read from .claude-lane/incremental.diff: a single-line change in scripts/affected-tests.sh (commit 40f1938, "resolve the --against ref through changed_files::verify_base"). Every other file in the PR was reviewed in prior passes on this thread through 0b339b5.

What changed: scripts/affected-tests.sh#L1133, inside run_replay's --against <ref> handling, replaces

against_sha="$(git rev-parse --verify -q "$against_ref^{commit}")"

with

changed_files::verify_base "$against_ref" || against_sha="$(git rev-parse "$against_ref")"

(actual code: ! changed_files::verify_base "$against_ref" || ! against_sha="$(git rev-parse "$against_ref")" inside the existing ||-chained error check).

Why: scripts/lib/gate-entry.test.sh#L298-L331 greps every script under scripts/ for a hand-rolled rev-parse.*(\^\{commit\}|--verify) base-ref predicate outside changed-files.sh and a small named allowlist; the old line matched that pattern and wasn't on the allowlist, so this script was failing that gate. Routing through the shared changed_files::verify_base helper (already used two call sites above, at L1010) satisfies it.

Correctness check (the thing worth verifying in a split like this): the new code separates "does $against_ref resolve to a commit" from "what SHA do I check out," and the second call drops the ^{commit} suffix, so for an annotated tag against_sha would hold the tag object's own SHA rather than the commit it points to — a behavior difference from before, worth tracing rather than assuming benign. I confirmed it's not a regression: git checkout --detach <sha> peels a tag object to its target commit automatically (checkout's commit-ish resolution always dereferences tags, whether given by name or raw SHA), and because changed_files::verify_base already short-circuits the || chain before the plain rev-parse runs, the removed --verify's only other effect (failing instead of echoing an unresolved ref back verbatim) is already covered. against_sha has exactly one use site (L1134, the checkout itself), so there's no second consumer that would see the un-peeled tag SHA and treat it differently than a commit SHA. The existing --replay --against HEAD tests (affected-tests.test.sh#L1838, #L1860) use a plain commit ref, not a tag, so this path isn't newly exercised by a test, but it isn't newly broken either.

Gate self-check: verified the new line doesn't re-trip the gate it fixes — rev-parse and --verify/^{commit} no longer co-occur on the same line (verify_base contains "verify" but not the literal --verify the pattern requires), and changed_files::verify_base is itself defined inside the excluded changed-files.sh.

Finding: none. This is a narrow, correctly-motivated lint-compliance fix with no behavioral regression on the paths this script actually exercises.

Checked against the instruction-surface-deletion lens too: this slice touches no CLAUDE.md/AGENTS.md/rules file or hook — it's one line of internal selector tooling.

No inline comments were needed for this slice.

…glob

The suite's LIVE cases read the github advise and planning interview skill
bodies and the autonomy reference docs through globs its test-scope header
did not declare. With the always list gone, a pull request renaming or
deleting one of those files selected nothing that ran this suite, and the
break surfaced on main. The header now declares all three globs.

The reference YAML case no longer reads the live files. It runs the
selector in a fixture that carries the live no-suite list, on probe files
in the two live directories. Declaring the live YAML instead would map it to
this suite and stop it failing loud, which the no-suite list wants until the
files get a real lane, and a live run failed whenever any other suite began
naming one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 20s —— View job


Claude is reviewing this PR

  • Gather context (PR body, lane header, HEAD SHA, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 40f1938)
  • Read .claude-lane/incremental.diff and review the actual slice
  • Post summary comment with findings (or confirm none found)

Code review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 40f19383119540e75534817d20710fe9ab0eb10f, read from .claude-lane/incremental.diff: scripts/affected-tests.test.sh only (commit 663d810, "declare the live files affected-tests.test.sh reads by glob"). scripts/affected-tests.sh is unchanged in this slice — it was already reviewed in prior passes on this thread through 40f1938. This is a test-only change; no production selector logic is touched.

What changed:

  1. Two new # test-scope: lines added to this suite's own leading comment block (affected-tests.test.sh#L12-13):
    • plugins/github/skills/advise/S*.md plugins/planning/skills/interview/S*.md
    • plugins/autonomy/reference/*.md
  2. The "LIVE repo: reference YAML with no lane is UNMAPPED" case (#L838-866) stops running the selector against the live repo tree and instead builds a throwaway fixture (mk_repo) seeded with two synthetic zz-probe.yaml files under the same two live directories, then asserts both come back UNMAPPED there.

Why this is correct, verified by hand-tracing against the live tree:

  • Both new globs are declarations of reads this suite already performs directly on $REPO_ROOT, confirmed by grepping the rest of the file: git ls-files 'plugins/autonomy/reference/*.md' at L1482, and git ls-files 'plugins/github/skills/advise/S*.md' / 'plugins/planning/skills/interview/S*.md' at L1628-1631. Both now also declare the glob in-place at those call sites via updated comments, consistent with the header addition.
  • Each glob currently matches at least one real tracked file (git ls-files locally returns 31 plugins/autonomy/reference/*.md files and the two SKILL.md files), so check_scope_globs' fail-closed check on this suite's own header won't trip.
  • The UNMAPPED fixture rewrite is a real improvement, not just a refactor: the old version ran the selector against the live plugins/toolchain/reference/ecosystems/*.yaml / docs/conventions/ecosystem-commands/examples/*.yaml files, which is fragile exactly the way the new comment explains — any future suite that starts naming or globbing one of those real files would flip it to MAPPED and silently invalidate this assertion on main, not just in this PR. Building a fixture with synthetic zz-probe.yaml files removes that dependency.
  • I checked the fixture is wired correctly rather than just trusting the comment: mk_repo (#L72-99) copies both the real selector script (--sut "$SCRIPT") and the live scripts/affected-tests-no-suite.txt into the fixture, so the fixture reproduces the exact condition the test needs: these two directories are verified absent from the no-suite list (scripts/affected-tests-no-suite.txt#L58-71 — a comment, not a live pattern, so it doesn't suppress UNMAPPED), and no suite in the fixture declares a scope over either directory, so the probes can only be UNMAPPED or silently MAPPED by a real bug, not by fixture noise.
  • has_line "$out" " - $y" matches the selector's actual UNMAPPED output format exactly (echo " - $f" >&2 at affected-tests.sh#L1282), and has_line does an exact newline-delimited match (#L128), so this isn't a loose substring check that could pass vacuously.
  • The probe basename zz-probe.yaml doesn't collide with any real file in the repo, so the AMBIGUOUS NAMES resolution rules can't accidentally make it resolve to something else.

Finding: none. This slice is a self-consistent, well-verified pair of changes: declaring reads the suite already made (closing an R8 gap this PR's own convention requires), and hardening a previously live-tree-coupled assertion into an isolated fixture. No production code changed.

No inline comments were needed for this slice.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 54s —— View job


Claude is reviewing this PR

  • Gather context (PR body, lane header, HEAD SHA, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 (incremental scope since 40f1938)
  • Read .claude-lane/incremental.diff and review the actual slice
  • Post summary comment with findings (or confirm none found)

Security review (incremental scope)

Scope: per the lane header, this pass covers only what changed since 40f19383119540e75534817d20710fe9ab0eb10f, read from .claude-lane/incremental.diff: scripts/affected-tests.test.sh only (commit 663d810, "declare the live files affected-tests.test.sh reads by glob"). scripts/affected-tests.sh itself is unchanged in this slice — it was reviewed through 40f1938 in prior passes on this thread.

What changed:

  1. Two # test-scope: lines are added to this suite's own header (affected-tests.test.sh#L12-L13), declaring the globs plugins/github/skills/advise/S*.md, plugins/planning/skills/interview/S*.md and plugins/autonomy/reference/*.md — the live probe files two existing LIVE-repo test cases already read by glob (discovered, never spelled, per the comments at L1473 and L1626).
  2. The "reference YAML with no covering lane is UNMAPPED" case (affected-tests.test.sh#L844-L872) moves from probing live repo files discovered via git ls-files to writing two synthetic zz-probe.yaml fixtures into a scratch repo built by the pre-existing mk_repo helper, then asserting the selector's output contains the exact - <path> UNMAPPED line.

Trust-boundary check: this is a test-only file; the relevant question is whether it introduces a new input-handling or injection surface, or silently weakens coverage (the under-selection concern this lane tracks for this script).

  • Both new eco_yaml paths are hardcoded string literals in the test, not derived from any external or untrusted input — no injection surface from the mkdir -p/printf/bash scripts/affected-tests.sh "${eco_yaml[@]}" calls.
  • mk_repo and has_line are pre-existing helpers already used throughout this suite (confirmed at L72-L87 and L128), not new code introduced by this slice.
  • The switch from a live-repo probe to a synthetic fixture is a precision improvement, not a coverage loss: the old version depended on plugins/toolchain/reference/ecosystems/*.yaml actually existing and staying unmapped by every suite in the live tree (a test that could silently degrade as the tree evolves); the new version pins the exact behavior (a reference YAML with no declared lane prints UNMAPPED with that path on its own - <path> line) against a controlled fixture, independent of what the live tree currently contains.
  • The two new # test-scope: header lines are additive (declaring more files this suite depends on), not a narrowing of any guardrail — nothing is removed from a CLAUDE.md/AGENTS.md/rules file or hook allowlist, so the instruction-surface-deletion lens doesn't apply here.

Finding: none. This slice is a test-fixture hardening (de-risking a live-repo-dependent probe into a controlled synthetic one) plus two declarative header additions, with no production code touched and no new input-handling surface.

No inline comments were needed for this slice.

cursoragent and others added 4 commits October 3, 2026 15:26
Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
…e (R9)

CI runs a Node suite that has a sibling <stem>.test.sh only through that
wrapper: scripts/run-outside-node-suites.sh reports it as owned by the
wrapper and runs nothing. The selector stops walking at a reached suite, so a
change that reaches the Node suite through its source file (lib/exec-bash.mjs
reaching exec-bash.resolver.test.mjs) selected the suite without its wrapper,
and CI ran neither. R9 adds the wrapper of every selected
<stem>.test.{js,mjs,cjs} after every other rule and before --shard.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ection

main's #6063 released planning 0.65.4; this branch's planning entry moves to
0.65.5 above it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 13s —— View job


Security review (/review:security-review)

  • Gather context (PR body, HEAD SHA, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 at b5e5df4
  • Determine the incremental slice (no .claude-lane/incremental.diff present in this checkout, so scoped manually against the last-reviewed SHA)
  • Post summary comment with findings (or confirm none found)

Scope: the last security pass on this thread covered through 663d810d78670df8ea06a0aa5db79af5470fe922. Since then, HEAD moved to b5e5df4 via two origin/main merges (which bring in already-reviewed, already-landed-on-main content with no unique diff of this PR's own) plus exactly one new content commit, 74fa5e2 ("select the .test.sh wrapper of every selected Node suite (R9)"). I confirmed this by diffing 663d810..HEAD restricted to scripts/affected-tests.sh / scripts/affected-tests.test.sh and verifying it matches 74fa5e2's own diff exactly (13+23 lines, pure addition). Everything else in the PR was reviewed in prior passes on this thread.

What changed: adds rule R9 — after every other selection rule and before --shard, a selected <stem>.test.js/.test.mjs/.test.cjs also selects its sibling <stem>.test.sh if one exists. The motivating bug (stated in the commit message): scripts/run-outside-node-suites.sh runs a Node suite that has such a wrapper only through that wrapper, so when the Node suite was reached indirectly (e.g. via an R3/mention edge like lib/exec-bash.mjs → exec-bash.resolver.test.mjs, rather than via the co-location rule that would already walk to the .sh sibling), the selector picked the .mjs suite but not its wrapper, and CI ran neither — a silent under-selection, which is exactly the trust-boundary failure mode this lane watches for in this script.

Trust-boundary check (R9 is itself an under-selection fix, so the question is whether it's correct, not whether it's safe):

  • scripts/affected-tests.sh#L1329-L1335: the loop snapshots "${!SUITES[@]}" into the for list before the loop starts adding to SUITES, so it can't iterate over its own insertions (the inline comment states this explicitly, and it's correct bash — command substitution/array expansion for a for ... in list happens once, up front).
  • add_suite (L559-567) gates on [[ -f "$suite" ]], so ${s%.test.*}.test.sh is pure string construction over an already-trusted, already-validated suite path (originating from git-tracked files elsewhere in the script) — no eval, no command substitution of untrusted content, and a non-existent wrapper is silently skipped (|| true) rather than erroring.
  • Placement is correct: after the unmapped-corpus fallback loop populates SUITES and before the sort -u/--shard partition, so a wrapper added by R9 is guaranteed to land in selected and isn't dropped by a shard that already computed its slice from a stale list.
  • Hand-traced the two new test cases in affected-tests.test.sh#L1020-L1040: the positive case (eco/wrapped.mjs → wrapped-cases.test.mjs via an R3 import-mention edge, not co-location) correctly also selects wrapped-cases.test.sh. The negative case reuses the pre-existing eco/probe.mjs/eco/probe.test.mjs fixture (already asserted selected at L1013-1014), which has no .test.sh sibling — so the assertion that $OUT contains no .test.sh string is non-vacuous (the output is non-empty; it genuinely tests that R9 adds nothing when there's no wrapper to add), not a trivially-passing empty-output check.

Finding: none. This is a narrow, correctly-placed, well-tested fix for a real silent under-selection bug, with no injection surface (pure string ops over already-validated paths) and no behavior that weakens an existing guarantee — it only ever adds a suite, never removes one.

Checked against the instruction-surface-deletion lens too: this slice touches no CLAUDE.md/AGENTS.md/rules file or hook allowlist — it's internal selector logic and its own test suite.

No inline comments were needed for this slice.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 6m 9s —— View job


Code review (/review:code-review)

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 (per custom instructions)
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Scope: every prior pass on this PR (through 663d810d78670df8ea06a0aa5db79af5470fe922) already reviewed the full selector rewrite exhaustively across ~18 incremental passes. The only content new since then is one commit, 74fa5e2 ("fix(scripts): select the .test.sh wrapper of every selected Node suite (R9)"), 36 lines across scripts/affected-tests.sh and scripts/affected-tests.test.sh — the rest of the range to b5e5df4 is merge-commit noise from origin/main (an unrelated already-landed feature plus merge mechanics), not this PR's content. I reviewed that commit directly rather than re-running the whole-file review this lane's instructions default to, since re-covering ~1900 already-reviewed diff lines would add nothing.

What R9 does: after every other rule and before --shard, any already-selected <stem>.test.js / .test.mjs / .test.cjs whose directory also holds <stem>.test.sh now selects that .test.sh wrapper too — fixing a real gap where scripts/run-outside-node-suites.sh treats such a Node suite as "owned by its wrapper" and runs nothing for it directly, so a selection that reached the Node suite without its wrapper (e.g. through a mention rather than co-location) ran neither.

Verification done (hand-traced, since shell execution is sandboxed in this environment):

  • Confirmed the for s in "${!SUITES[@]}" key list is captured once before the loop body's add_suite calls can mutate SUITES — correct single-pass bash semantics, matching the comment.
  • Confirmed placement: this runs after UNMAPPED/--unmapped-corpus handling (scripts/affected-tests.sh#L1286-1323) and before the sort/shard step, so it can't affect fail-loud detection and correctly also wraps suites added via the corpus fallback.
  • Traced "${s%.test.*}.test.sh" against the new fixture (eco/wrapped-cases.test.mjs → eco/wrapped-cases.test.sh) — correct shortest-suffix stripping.
  • Confirmed add_suite's [[ -f "$suite" ]] guard means a missing wrapper is a no-op, not an error, and || true is required under this script's set -e.
  • The new test block (scripts/affected-tests.test.sh around L1017-1045) exercises both the positive case (suite reached by mention still pulls in its wrapper) and the negative case (a suite with no wrapper adds nothing) — both pinned correctly against the existing eco/ fixture tree, no interaction with earlier assertions in the same block.

Finding posted inline (scripts/affected-tests.sh#L1332): the new *.test.cjs) arm can never fire. is_suite_path() (L574), colocated_suites()'s candidate list (L757-760), and the --unmapped-corpus awk pattern all omit .cjs, so nothing upstream can ever put a .test.cjs path into SUITES for R9 to match against. It's dead code today (no .test.cjs file exists in the repo), but scripts/run-outside-node-suites.sh (L129-132) already treats .test.cjs as a first-class wrapped Node suite type — so if one is ever added, CI's runner will correctly skip it as wrapper-owned while this selector silently never selects it or its wrapper at all, the exact failure mode this file's "fail loud, not open" design otherwise guards against.

No other findings. The rest of this PR was already reviewed to this lane's bar in prior passes and is unchanged.

Comment thread scripts/affected-tests.sh Outdated
kyle-sexton and others added 2 commits October 3, 2026 11:45
…ection

main's #6081 released animation 0.4.0; this branch's animation entry moves to
0.4.1 above it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ection

main's #6065 released code-metrics 0.5.3, harness-ops 3.6.1, repo-hygiene
0.19.3, session-flow 0.48.1 and source-control 0.79.1; this branch's entries
for those plugins move one patch above.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@kyle-sexton
kyle-sexton enabled auto-merge (squash) October 3, 2026 15:48
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 37s —— View job


Claude is reviewing this PR

  • Gather context (PR body, HEAD SHA, lane header, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 at 81dcdd4
  • Determine the incremental slice since the last-reviewed SHA
  • Post summary comment with findings (or confirm none found)

Security review

Scope determination: the last security pass on this thread covered through b5e5df49f7943daa044f999a6a6b62333cda6aff (commit 74fa5e2, R9 wrapper selection). HEAD is now 81dcdd4, reached via two origin/main merges that pulled in three unrelated main commits (1b8b560, ad9d51d, ce07376) — no new commit of this PR's own.

I verified this is pure merge noise rather than new content to review, three ways:

  1. git diff b5e5df4 81dcdd4 -- scripts/affected-tests.sh scripts/affected-tests.test.sh scripts/affected-tests-no-suite.txt is empty — the selector script and its test suite, the files every substantive finding in this thread has concerned, are byte-identical to what was already reviewed.
  2. git merge-base --is-ancestor b5e5df4 81dcdd4 succeeds — b5e5df4 is a strict ancestor of the current head, so nothing it covered was later reverted or altered by the merges.
  3. This PR's true diff, git diff --stat origin/main...81dcdd4, reports 151 files changed, 1494 insertions(+), 1037 deletions(-) — matching this PR's reported additions/deletions (1494/1037) exactly. The file set is the same R8-header-convention + selector-rewrite content already walked commit-by-commit across ~20 incremental passes on this thread (through 20231e0, 29fb678, 2473daa, 21c56641, cd900f3, 6dfc8f3, 469cdfb, 0b339b5, 40f1938, 663d810, and finally 74fa5e2/b5e5df4). Every file the new merge-base now reveals as part of the true PR diff (e.g. speech, testing, work-items, retro-audio, review, session-flow, source-control picking up a # test-scope: header) was already covered by the e5541a1/0b339b5 pass, which checked the 30-plugin release wave's headers against the deleted affected-tests-scopes.txt entries line-for-line rather than by file name — this is the same mechanical change, just newly visible in the diff because the merge-base shifted.

Result: no new security-relevant content since the last completed pass. This head already has a successful security review that still applies (skip-gate condition 3) — re-running the full hunt would re-review the same bytes already cleared in this thread, with no finding surviving verification across any of those passes.

No inline comments needed; nothing new to flag.

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 37s —— View job


Claude is reviewing this PR

  • Gather context (PR body, HEAD SHA, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 at 81dcdd4
  • Determine what's new since the last full review pass on this thread (b5e5df4/74fa5e2)
  • Review the genuinely new content
  • Post summary comment with findings (or confirm none found)

Code review

Scope determination: HEAD (81dcdd4262fd24a399d2b367e80a055b952a68c1) sits two merge commits past the last SHA fully reviewed on this thread (b5e5df49f7943daa044f999a6a6b62333cda6aff, after which 74fa5e2's R9 wrapper fix was reviewed). I diffed b5e5df4..HEAD (98 files) against the true PR diff origin/main...HEAD (151 files, 1494/−1037, matching the PR's reported stats exactly) to separate genuine new content from origin/main noise landing through the two merges.

The intersection is small: 4 plugins (animation, repo-hygiene, session-flow, source-control) each gained one mechanical commit — a patch version bump, a CHANGELOG.md entry, and one new # test-scope: header line on a suite that needed one after merging in origin/main's own new files. Everything else that changed since b5e5df4 (the eli5→illustrate rename, animation hooks, session-flow/source-control script changes, etc.) is origin/main's own unrelated history landing through the merge, identical between HEAD and origin/main, and not this PR's content — confirmed by diffing each of those paths against origin/main...HEAD and finding no PR-specific delta. scripts/affected-tests.sh and scripts/affected-tests.test.sh themselves are byte-identical to b5e5df4, so the whole selector rewrite this thread already reviewed exhaustively across ~20 incremental passes is unchanged.

Verification of the 4-plugin mechanical tail:

  • Each plugin.json bump is exactly one patch above origin/main's current version (animation 0.4.0→0.4.1, repo-hygiene 0.19.3→0.19.4, session-flow 0.48.1→0.48.2, source-control 0.79.1→0.79.2) — not stale, not a conflicting bump.
  • Each CHANGELOG.md entry names exactly the suites that gained a header in that plugin, matching this repo's review-discipline convention for a scope-declaration change.
  • I checked every new # test-scope: glob resolves to at least one real tracked file (git ls-files on each), so none would trip check_scope_globs's fail-closed validation on a future run touching these suites: plugins/animation/skills/*/SKILL.md etc., plugins/repo-hygiene/skills/*.md/skills/clean/SKILL.md, plugins/session-flow/scripts/tests/fixtures/*/skills/audit-sessions/scripts/tests/fixtures/*, plugins/source-control/scripts/*/skills/babysit-prs/scripts/babysit_*.py/*.md/skills/*/SKILL.md/plugins/source-control/*.
  • Each header is correctly placed inside the suite's leading comment block before any code (including test_guards.py's, where the new # test-scope: line precedes the module docstring — a comment before a module docstring doesn't change __doc__, so this is syntactically inert).

Finding: none. This slice is the same already-reviewed, already-verified mechanical pattern (0b339b5 on this thread) extended to four more suites that needed it after the merge. No selector logic changed.

Open item carried forward (not re-raised, already posted on this PR): the inline P2 finding on scripts/affected-tests.sh#L1332 — the R9 *.test.cjs) arm is dead code today (nothing upstream, is_suite_path(), colocated_suites(), or the --unmapped-corpus pattern, recognizes .test.cjs), while run-outside-node-suites.sh already treats .test.cjs as a wrapped-suite type — is still unaddressed in this HEAD. No action needed from this pass; flagging only so it isn't lost since the file hasn't changed to pick it up.
· branch perf/precise-test-selection

kyle-sexton and others added 3 commits October 3, 2026 11:57
is_suite_path admits .test.js and .test.mjs, not .test.cjs, so the .test.cjs
arm of R9 could never fire, and no .test.cjs suite exists.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ection

main's #6064 released actionlint, animation, autonomy, context-guard,
guardrails, harness-ops, source-control, speech and testing; this branch's
entries for those plugins move one patch above.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
#6064 deleted scripts/lib/sync-cluster.sh, the only file the glob matched, and
a declared glob that matches no file fails every run that changes its suite.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 53s —— View job


Security review (incremental scope)

  • Gather context (PR body, HEAD SHA, lane header, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 at cb357f3
  • Determine the incremental slice since the last-reviewed SHA
  • Post summary comment with findings (or confirm none found)

Scope: the last security pass on this thread covered through 81dcdd4262fd24a399d2b367e80a055b952a68c1. Since then, two real content commits land on top of two origin/main merges (the merges bring in already-landed, unrelated main history — #6064's hook-utils.sh shared-lib migration — with no diff of this PR's own beyond what these two commits touch):

  1. ee408ec — "limit R9 to the Node suites the selector recognizes"
  2. cb357f3 — "drop the scripts/lib/sync-*.sh scope main's build(shared-lib): migrate hook-utils.sh to the regen flow and retire the sync machinery #6064 emptied"

ee408ec — scripts/affected-tests.sh#L1332: drops the .test.cjs arm from R9's wrapper-selection case, matching the inline P2 finding raised earlier in this thread and the author's own reply acknowledging it. Verified directly against this exact head:

  • is_suite_path() still recognizes only .test.sh | .test.js | .test.mjs | .Tests.ps1 — .test.cjs was never selectable, so narrowing R9 to match removes genuinely dead code, not live coverage.
  • git ls-files '*.test.cjs' returns nothing in this tree — no suite of that type exists to be affected either way.
  • The header comment (L112-118) is updated to match, so the rule's stated contract and its implementation agree again.

This is a pure narrowing of unreachable code — no behavior change, no under-selection risk (the thing this lane watches for in this file), since the removed arm could never fire.

cb357f3 — scripts/affected-tests.test.sh#L11: drops the scripts/lib/sync-*.sh glob from this suite's own # test-scope: header, and removes a now-dead case branch in the shared-copy-fanout test that special-cased lib/hook-utils.sh to validate against the now-deleted scripts/sync-hook-utils.sh manifest. I verified both halves against the live tree at this head rather than taking the commit message at face value:

  • scripts/lib/sync-*.sh matches zero tracked files (confirmed via glob) — main's #6064 deleted scripts/lib/sync-cluster.sh, the one file the dropped portion of the glob ever matched, so leaving it in would trip check_scope_globs's fail-closed validation on the next run that touches this suite. Removing it is required, not optional, under this PR's own R8 contract ("a glob matching no file fails the run").
  • The remaining scripts/sync-*.sh portion of the same glob still resolves (scripts/sync-shared-copies.sh and its own test exist), so no live coverage is dropped, only the dead half.
  • lib/hook-utils.sh is now registered as a canonical source directly in scripts/shared-copies.txt#L282-302, which scripts/sync-shared-copies.sh --print-manifest reads — confirming the removed case ... lib/hook-utils.sh) manifest="scripts/sync-hook-utils.sh" branch is correctly superseded by the single shared manifest, not silently losing its check. scripts/sync-hook-utils.sh no longer exists in the tree and has no remaining live references (only historical CHANGELOG.md entries and unrelated eval fixtures mention the name).

Trust-boundary check: both commits are test-metadata/doc-comment and test-assertion adjustments reacting to an already-landed, independently-reviewed main change (#6064) — no new input source, no eval, no command substitution of untrusted content, and no weakening of the selector's fail-closed validation (if anything, the stale glob would otherwise break every run touching this suite, which is a correctness/availability concern, not a security gap).

Finding: none. Both commits are narrow, correctly-scoped reactions to prior review feedback and an upstream merge, verified against the live tree rather than assumed from the commit message.

Checked against the instruction-surface-deletion lens too: neither commit touches a CLAUDE.md/AGENTS.md/rules file or hook allowlist — both are internal selector/test metadata.

No inline comments were needed for this slice.
· branch perf/precise-test-selection

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 32s —— View job


Code review (/review:code-review)

  • Gather context (PR body, HEAD SHA, lane header, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 at cb357f3
  • Determine the incremental slice since the last-reviewed SHA
  • Review the genuinely new content
  • Post inline comments for findings anchored to changed lines
  • Post summary comment with overview + any cross-file findings

Scope determination: the last fully-reviewed SHA on this thread was 81dcdd4262fd24a399d2b367e80a055b952a68c1. HEAD is now cb357f3a267ab1fa2711837512d866035dc2ec48. This checkout is shallow and origin/main isn't fetchable here (network access denied in this lane), so I scoped by walking git log 81dcdd4..HEAD directly rather than diffing against origin/main.

Of the commits in that range, only two are this PR's own content:

d022d4b (#6064, "migrate hook-utils.sh to the regen flow") and the two merge commits (06e8cab, and the earlier 81dcdd4/eb50b83) are origin/main's own already-merged history landing through this branch's routine merges, not unique content of this PR — confirmed by their commit messages (d022d4b closes #5838, an unrelated issue) and by the fact cb357f3 exists specifically to react to #6064's effect on this tree.

ee408ec — scripts/affected-tests.sh#L1332: this is the fix for the P2 finding I raised in an earlier pass on this thread — the *.test.cjs) arm of R9 was dead code, since is_suite_path() (L574) never admits a .test.cjs suite, so nothing upstream could ever put one into SUITES for R9 to match. The fix removes .test.cjs from both the R9 case arm and the header's rule description, consistent with the finding and with kyle-sexton's inline reply. Verified is_suite_path() still recognizes only *.test.sh | *.test.js | *.test.mjs | *.Tests.ps1 plus test_*.py, so the two are now consistent and the dead branch is gone.

cb357f3 — scripts/affected-tests.test.sh#L11: this suite's own # test-scope: header drops the scripts/lib/sync-*.sh glob. #6064 deleted scripts/lib/sync-cluster.sh — the only file that glob ever matched — and a declared glob matching zero files fails every run that changes its declaring suite (check_scope_globs's fail-closed check). I confirmed on the current tree: scripts/lib/sync-*.sh now matches nothing, while the remaining scripts/sync-*.sh glob still matches real files (sync-shared-copies.sh, its own test). The companion change in the same commit drops the lib/hook-utils.sh → scripts/sync-hook-utils.sh special-case from the test body, since that script is deleted by #6064 and hook-utils.sh now goes through the same scripts/sync-shared-copies.sh manifest as every other shared-copy source — confirmed lib/hook-utils.sh is registered in scripts/shared-copies.txt (21 copy lines) exactly as #6064's description states.

Verification done: hand-traced both diffs against the current tree (ls/grep on the relevant globs and registries) rather than executing the shell test suite — Bash execution requires interactive approval in this lane and none was available. Both changes are narrow, correctly motivated, and internally consistent with the files they react to.

Finding: none. Both commits are small, well-targeted fixes: one closes out a previously-flagged dead-code arm, the other keeps a declared test-scope glob from going stale after an unrelated already-merged PR deleted the file it covered. No inline comments were needed for this slice.
· branch perf/precise-test-selection

kyle-sexton and others added 2 commits October 3, 2026 12:15
…ection

main's #6082 released speech 0.2.0; this branch's speech entry moves to 0.2.1
above it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ection

main released discovery review (feat(review): add review:explain-change with a digest policy and an interactive view (#6018); fix(discovery): run research gates as one plain command); this branch's entries for those plugins move one patch above.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 45s —— View job


Code review (/review:code-review)

  • Gather context (PR body, HEAD SHA, lane header, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:code-review for PR perf(ci): select only the suites a change runs or reads #6059 at 71ed6fc
  • Determine the incremental slice since the last fully-reviewed SHA (cb357f3)
  • Post summary comment with findings (or confirm none found)

Scope determination

The last code-review pass on this thread covered through cb357f3a267ab1fa2711837512d866035dc2ec48. HEAD is now 71ed6fc0f6dc063d8b7f2b69b029940fa31aff91, reached via two origin/main merges with no new commit of this PR's own:

  • git log cb357f3..71ed6fc --no-merges returns exactly two commits: db4cdd3 (feat(review): add review:explain-change with a digest policy and an interactive view #6018, review:explain-change skill) and d301b20 (feat(speech): add an optional elevenlabs backend with a cost estimate before every call #6082, speech ElevenLabs backend) — both unrelated, already-landed origin/main PRs, not this PR's content.
  • git diff cb357f3 71ed6fc -- scripts/affected-tests.sh scripts/affected-tests.test.sh scripts/affected-tests-no-suite.txt is empty — the selector script, its test suite, and the no-suite list (the files every substantive finding on this thread has concerned) are byte-identical to what was already reviewed.
  • The full diff since cb357f3 (40 files) touches only plugins/review/skills/explain-change/**, plugins/speech/** (ElevenLabs backend + its tests), plugins/discovery/reference/parent-contract.md, lib/html-escape.test.sh, and a handful of docs/catalog files — none of it is R8 header content, selector logic, or test-scope declarations belonging to this PR.
  • This PR's true diff, git diff origin/main...71ed6fc --stat, reports 151 files changed, 1494 insertions(+), 1037 deletions(-) — matching the PR's reported additions/deletions (1494/1037) exactly, confirming the PR's own content size hasn't changed since the last pass.

Result: no new content of this PR's own since the last completed code-review pass. Every file this thread has found anything worth flagging in (scripts/affected-tests.sh, scripts/affected-tests.test.sh, and the per-plugin # test-scope: header/version/CHANGELOG tails) is unchanged from cb357f3, which already cleared review with one confirmed-fixed finding (the dead .test.cjs R9 arm, resolved by ee408ec) and no other open findings.

No inline comments were needed — nothing new to review.
· branch perf/precise-test-selection

@claude

claude Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 9s —— View job


Security review (incremental scope)

  • Gather context (PR body, HEAD SHA, lane header, CLAUDE.md/AGENTS.md instructions)
  • Invoke /review:security-review for PR perf(ci): select only the suites a change runs or reads #6059 at 71ed6fc
  • Determine the incremental slice since the last-reviewed SHA
  • Post summary comment with findings (or confirm none found)

Scope determination: the last security pass on this thread covered through cb357f3a267ab1fa2711837512d866035dc2ec48. HEAD is now 71ed6fc, reached via two origin/main merges with no new commit of this PR's own:

  1. I diffed cb357f3..HEAD across every file and confirmed the 39 changed files are entirely plugins/review/skills/explain-change/*, plugins/review/skills/pr-explainer/*, plugins/review/tests/explain-change* (from main's db4cdd3, the review:explain-change skill) and plugins/speech/scripts/elevenlabs.py, plugins/speech/skills/narrate/*, plugins/speech/scripts/test_elevenlabs.py, etc. (from main's d301b20, the ElevenLabs backend) — two unrelated, already-landed main features, not new content authored on this branch.
  2. git diff --stat origin/main...HEAD reports 151 files changed, 1494 insertions(+), 1037 deletions(-) — matching this PR's reported additions/deletions (1494/1037) exactly, confirming GitHub's three-dot diff (and therefore this PR's actual content) is unchanged by the two merges: both upstream features are already on origin/main, so they net out of the PR's own diff entirely.
  3. scripts/affected-tests.sh, scripts/affected-tests.test.sh, scripts/affected-tests-no-suite.txt, and scripts/affected-tests-always.txt — the files every substantive finding on this thread has concerned — are byte-identical between cb357f3 and 71ed6fc.
  4. The one PR-owned file that did textually change across the merge, plugins/speech/scripts/speech.test.sh, merged cleanly: main's d301b20 added elevenlabs.py/test_elevenlabs.py to the suite's doc comment and unittest invocation at lines 2 and 26, while this PR's own # test-scope: header at line 8 (reviewed in the very first pass on this thread) passed through untouched.

Result: no new security-relevant content since the last completed pass. This head already has a successful security review that still applies (skip-gate condition 3) — re-running the full hunt would re-review the same bytes already cleared across the ~20 incremental passes on this thread.

No inline comments needed; nothing new to flag.
· branch perf/precise-test-selection

@kyle-sexton
kyle-sexton merged commit 6bb7cd8 into main Oct 3, 2026
34 of 35 checks passed
@kyle-sexton
kyle-sexton deleted the perf/precise-test-selection branch October 3, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants