Repository navigation
feat(animation): report a missing node once per session with a SessionStart node-notice row - #6081
Merged
Merged
Conversation
…nStart node-notice row Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton
marked this pull request as ready for review
October 3, 2026 15:22
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
Claude finished @kyle-sexton's task in 3m 3s —— View job Claude's code review for PR #6081
Scope: Finding
The prerequisites convention defines Concretely: the fleet report Everything else checked out
Branch: |
Contributor
|
Claude finished @kyle-sexton's task in 1m 12s —— View job Security review completeReviewed PR #6081 (head
No security issues found. Notes on what was checked:
GitHub Actions workflow hardening is out of scope for this lane (zizmor's advisory lane); not applicable here regardless, as no workflow files are touched. |
Contributor
|
claude-security-review has reviewed this pull request through e2e8ab9; a later push is reviewed from there. |
Contributor
|
PR body contract — issue linkage This PR body does not yet satisfy the issue-linkage contract:
Edit the body and this comment updates itself on the next run. |
Contributor
|
claude-review has reviewed this pull request through e2e8ab9; a later push is reviewed from there. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa8f389777
ℹ️ 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".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Claude finished @kyle-sexton's task in 40s —— View job Claude's security review for PR #6081 (incremental)
Review scope: incremental, against this lane's last-reviewed commit
No security issues found.What changed and why it's not security-relevant:
Per the skip gate, this incremental change has no security-relevant surface, so no inline comments were posted. |
Contributor
|
Claude finished @kyle-sexton's task in 3m 8s —— View job Claude's code review for PR #6081 (incremental)
Scope: incremental — this lane last reviewed
Head reviewed: No findingsNothing meets this lane's high-signal bar (a careful senior reviewer would block or flag) in the incremental diff:
Everything else in this PR (the |
…drop the package-verification claim Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Claude finished @kyle-sexton's task in 32s —— View job Claude's security review for PR #6081 (incremental)
Review scope: incremental, against this lane's last-reviewed commit
Head reviewed: No security issues found.What changed and why none of it is security-relevant:
Per the skip gate, this incremental change has no security-relevant surface, so no inline comments were posted. |
Contributor
|
Claude finished @kyle-sexton's task in 1m 30s —— View job Claude's code review for PR #6081 (incremental)
Scope: incremental — this lane last reviewed
Head reviewed: No findingsThis commit resolves both P2 findings from Codex's earlier review on
Also checked and consistent:
Everything else in this PR (hooks.json row, evals.json, registry, README/cheat-sheet, manifest version bump) is unchanged since |
kyle-sexton
added a commit
that referenced
this pull request
Oct 3, 2026
…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>
kyle-sexton
added a commit
that referenced
this pull request
Oct 3, 2026
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](#6059 (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: - `scripts/affected-tests.test.sh`: PASS=134 FAIL=0 (the 4 Python-import cases are gone; the R8 cases now build suites with headers, including a declaration below code that declares nothing and a stale glob that fails only the run changing its suite; the R9 case, a Node suite reached through a mention, fails on the previous head's selector, 133/1) - `scripts/lib/gate-entry.test.sh`: 33/0 - Headers read back from the 87 suites equal the former list entry for entry, plus the three globs `scripts/affected-tests.test.sh` adds for the live files it reads. - The selector on `plugins/github/skills/advise/SKILL.md`, `plugins/planning/skills/interview/SKILL.md` and an autonomy reference doc selects `scripts/affected-tests.test.sh` through its header; the reference YAML (`plugins/toolchain/reference/ecosystems/go.yaml`, `docs/conventions/ecosystem-commands/examples/go.yaml`) stays UNMAPPED at exit 1. - shellcheck clean on the selector and the 81 changed shell suites; the pinned ruff check passes on the 6 changed Python suites. - `check-changelog-parity.sh` `--check`, `--check-bump`, `--check-preserved` and `--check-order` pass against origin/main. Where main released a plugin this PR also releases (planning in #6063; animation in #6081; code-metrics, harness-ops, repo-hygiene, session-flow and source-control in #6065; actionlint, animation, autonomy, context-guard, guardrails, harness-ops, source-control, speech and testing in #6064; speech in #6082; review in #6018; discovery in #6088), this PR's entry sits one patch above main's. - Main's #6064 deleted `scripts/lib/sync-cluster.sh`, the only file the `scripts/lib/sync-*.sh` glob in `scripts/affected-tests.test.sh`'s header matched; the glob is dropped, since the selector fails (exit 2) on any diff that changes a suite declaring a glob that matches nothing, as it did on the merge commit alone. ## Related - #3932: the CI performance program. - #6021 (wave 3 PR 1, `ci.yml` job rename) has landed. It edited `scripts/affected-tests-always.txt`, and this PR keeps that file deleted. - PR 3 should: - run `--unmapped-corpus` in place of the whole-shell-corpus fallback, which gives the 15,290 figure above; - route the pins (root `package*.json`, `.node-version`, Python pins) to their lanes (S8); - drop `--with-always` from `ci.yml`; - update the `ci.yml` comment that names `affected-tests-always.txt`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: ksextonmelodic <ksextonmelodic@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #5843
Summary
#5997 gave every hook plugin a SessionStart node-notice row but exempted
animationbecause it had noprerequisites.jsonand no check skill. #5979 added the manifest. This adds the row, a/animation:check-prerequisitesskill for the notice to name (animation has nocheckskill), and removes the exemption.Fix
plugins/animation/hooks/hooks.json: the canonical polyglot node-notice row beside the existing install row.plugins/animation/skills/check-prerequisites/SKILL.md: runs the shared checker through its stub; registered inscripts/skill-leaf-name-registry.txtnext to the other threecheck-prerequisitesplugins.scripts/node-notice-rows.test.sh: theEXEMPTlist and its note are gone, so the fleet check now covers animation.Verification
bash scripts/node-notice-rows.test.sh: 4 passed, 0 failed; 25 hook plugins carry exactly one canonical row.bash lib/hook-utils.test.sh: 597 passed, 0 failed.node scripts/check-declared-prerequisites.mjs,scripts/sync-shared-copies.sh --checkand--check-bump origin/main,node scripts/validate-plugin-contracts.mjs,node scripts/generate-catalog.mjs --check,bash scripts/check-skill-leaf-names.shandbash scripts/check-changelog-parity.sh --check-orderpass.Related
🤖 Generated with Claude Code