Repository navigation
fix(scripts): map autonomy reference docs to the plugin-contract suite in affected-tests - #4428
Merged
Merged
Conversation
…e in affected-tests The plugin-contract validator bans vendor names under plugins/autonomy/reference/, but those files are markdown no suite names, so affected-tests reported them as the no-suite *.md class and selected nothing. Add rule R7, a path-class rule that selects the contract suite for any path under that directory, with fixture, scope, and live-repo cases. The suite path is assembled from two pieces so affected-tests.sh never carries its basename as one token and does not become an R4 dependent of it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ripwire Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton
marked this pull request as ready for review
September 24, 2026 04:40
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
Skill evidence: a gap at
|
…bare selection The live probe also reaches the contract suite on main through R4 fan-out, so asserting selection alone passed without R7. Assert the --explain path-class reason instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Sep 24, 2026
… race grep -q (#4461) No related issue: issue creation was denied to this unattended lane; the work item is the local handoff-inbox item 20260924-121500-affected-tests-live-r7-pipefail-flake. ## Summary `scripts/affected-tests.test.sh` runs under `set -uo pipefail` and matched captured output with `printf ... | grep -q ...`. When the output is large and the match is early, `grep -q` exits, the still-writing `printf` dies of SIGPIPE, and pipefail reports the assertion as failed although the text matched. The LIVE R7 case (from #4428) hit this: its live `--explain` output is 47884 bytes with the match on line 271, and CI on #4453 failed once on it with a broken pipe. Negated sites had the opposite failure: a present match read as absent, so the assertion passed. ## Fix - `has_line` and a new `contains` helper match in-shell with `[[ ]]` and a quoted needle, so no writer process exists for pipefail to report. - All 19 `printf | grep -q` assertion sites now use `contains`, with every `!` kept. The anchored `^src\t` check on the live `--print-manifest` output uses its own `[[ ]]` form. - A pin case feeds a 1 MiB string with the needle on line 1 to both helpers. - A guard case scans this suite's own source and fails on any non-comment line that pipes into `grep ... -q`. - The comment that claimed capturing stdout avoided the race, while the code still piped, is corrected. The two `git ls-files | head` sites are left as they are; their exit status is never read. A comment at each says so. ## Verification - Pin (three checks at the head) on Linux (WSL Ubuntu) with the pre-fix helper bodies: 2 FAIL, 1 PASS (found checks red, absent check green). With the new helpers: 3/3 PASS. - Under pipefail on Linux, with the needle on line 1, `printf | grep -qF` missed 50/50 runs at 1 MB and 0/50 at 8 KB. The rate between is timing-dependent: one run saw 7/50 at 64 KB, another 0/50. Looping the real 47884-byte LIVE R7 output 2000 times: old form failed 9/2000, new form 0/2000. - `bash scripts/affected-tests.test.sh` on Windows Git Bash: `PASS=93 FAIL=0`. The log has 93 `ok:` lines and 0 `FAIL:` lines. - Guard: the scan finds 21 hits on the base-commit copy of the file and 0 on the head. - `shellcheck scripts/affected-tests.test.sh` exits 0. Added lines contain 0 em dashes. - `scripts/check-changed-skills.sh` and `scripts/check-changelog-parity.sh --check-bump` against the fork point both pass. No plugin is touched, so there is no version bump or CHANGELOG entry. - skill-evidence block absent: the installed source-control and claude-ops predate #4210, so no ledger sha is stamped; the owed reviews ran as nested agents: code reviewer, simplification pass, security reviewer (`scripts/**` is a security path), verifier. - CI run 36046032017: all jobs green; the Linux shard reports `PASS=93 FAIL=0` for this suite. ## Related - Introduced by #4428; observed on #4453. - Sweep of the other `scripts/*.test.sh` suites: 13 other files use `| grep -q` under pipefail (the largest piped input measured is 3811 B). Every producer is either a checker over a synthetic fixture or a filtered read of a real file measured under 4 KB. None was converted. Deferred: `scripts/verify-security-review-evidence.sh.test.sh:156` (3811 B today) grows with its guard script, so it is the first one to watch. - Deferred: the in-file guard is best effort. It misses `--quiet`/`--silent`, a pipe split across a line continuation, a line with `#` before the pipe, and other early-exit readers (`grep -m1`, `head`, `awk ... exit`). Reviewers rated this low severity: it can miss a case but never passes one it should fail. - The contract was set in an unattended interview (no human answered questions); open questions took the defaults recorded in the lane plan. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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.
Closes #4427
Summary
scripts/affected-tests.shtreated every changed path underplugins/autonomy/reference/as the no-suite*.mdclass and selected nothing. Those files are gated by the plugin-contract validator's vendor-name ban, so a local selection never ran the one suite that can fail on them.Fix
casearm inselect_for. It sits after R1/R2 and before the structural-basename skip. It selectsscripts/validate-plugin-contracts.test.shfor any path underplugins/autonomy/reference/, nested directories included.affected-tests.shcarried the basename as one token, it would become an R4 dependent of that suite, and every edit to the suite would fan out to everything that namesaffected-tests.sh.plugins/autonomy/is not mapped.scripts/affected-tests.test.shgains three kinds of case:--explainreasonreference/and an autonomy file outsidereference/git ls-files, so its basename is never spelledVerification
bash scripts/affected-tests.sh --explain plugins/autonomy/reference/telemetry.mdprintedno-suite: ...andNo suites selected, exit 0. After the change it printsselect: scripts/validate-plugin-contracts.test.sh (path class: ...), exit 0.plugins/autonomy/README.mdand on a contract-suite edit:plugins/autonomy/README.mdstill selects no suites.scripts/validate-plugin-contracts.test.shselects only itself andscripts/affected-tests.test.sh.bash scripts/affected-tests.test.sh: PASS=89 FAIL=0, with all 6 R7 assertionsok.bash scripts/validate-plugin-contracts.test.sh: PASS=42 FAIL=0.shellcheckis clean on both files. Added lines contain 0 U+2014.check-changed-skills.shandcheck-changelog-parity.sh --check-bumpboth exit 0 against the fork point.--explain --baseover the branch diff reports nothing UNMAPPED, rc 0.affected-tests.shcarries no single-token suite basename. It did not run the pre-change script. The old-side evidence is the pre-change--explainreproduction above, measured on the unedited tree.scripts/**is in.github/claude-security-paths; no blockers, 2 findings addressed in fb9ef00), fresh-context verifier.Related
Queue item section 2 of
autonomy-telemetry-pillar3-native-join-and-affected-tests-reference-mapping. This was an unattended interview: each open question took the item's or the brief's default (see the open-question register in the lane's PLAN.md).Deferred, user-reserved: section 1 asks whether trace ids on OTLP event records count as the native surface for the Pillar 3 migration trigger in
plugins/autonomy/reference/telemetry.md. That is Kyle's decision, and no autonomy file changes here.Deferred: the validator's other scans are not mapped. They cover:
plugins/autonomy/**CI runs the contract suite and
validate-plugins.shin their own leg-1 steps wheneverrun_testsis true.No version bump: this is a repo-root script change, and the repo has no root CHANGELOG.
🤖 Generated with Claude Code