feat(testing): scan test files a Bash call changed via bashEditDiff

[kyle-sexton]

Refs #5608

Summary

The testing plugin's opt-in test-scan saw only Write and Edit. Test files written through Bash (cat >, sed -i, generators) were never scanned. This probes PostToolUse bashEditDiff on Claude Code 2.1.285 and routes Bash-changed test files through test-scan.

Fix

• docs/specs/tautological-tests/probes.md: probe row. The field is absent in default, acceptEdits, auto and bypassPermissions without a setting; present only with bashEditDiffEnabled true (user, --settings, managed scope) or CLAUDE_CODE_BASH_EDIT_DIFF=1. Shape is changedFiles (absolute, all), files (first 5 with hunks), moreFiles; untracked new files are listed.
• plugins/testing/hooks/test-scan-bash.sh plus a generated PostToolUse Bash row in hooks.json: runs test-scan.sh on each changed test file (max 4). gen-hook-filters.sh generates the row.
• The row has no if (a Bash if matches the command string, not the changed files), so the node launcher starts on every Bash call, whatever test_guards_enabled says. Per the hook-budget convention the README states the row's k and cost in S, and .performance/ratchets.json holds two spawn ceilings: option off (1) and option on with no bashEditDiff (3).
• README subsection documents the opt-in limit and the cost. Version 0.12.0 and CHANGELOG entry, which say what the probe found for the field's modes.

Verification

• bash plugins/testing/hooks/test-scan.test.sh: 58 passed, 0 failed
• bash plugins/testing/scripts/gen-hook-filters.test.sh: 14 passed, 0 failed
• scripts/validate-plugins.sh: all manifests and catalog validated
• scripts/check-changelog-parity.sh --check, --check-order, --check-bump origin/main: pass
• markdownlint-cli2 on the README, CHANGELOG and probes.md: 0 issues.
• Budget, WSL2, S = bash -c : p50 1.0 ms, four arms interleaved, 50 samples, p50: option off 22 ms (about 22 S, k = 1); option on with no recorded change 30 ms (about 30 S, k = 2); one-file diff 116 ms (about 115 S). scripts/hook-census.sh spawns: 1, 3 and 96 (the last is the scan itself and is not ratcheted).
• ratchet.py check: all 14 counters at their ceilings on this host, including the two new ones. A mutant that runs jq before the script's substring test raised the no-diff count from 3 to 5 and the check reported it above its ceiling; the mutant is reverted. scripts/hook-census.test.sh: 18 passed.
• strace was not installed on this host. The counts came from an extracted .deb (strace 6.19, bash 5.3.9, jq 1.8.2) kept under the gitignored .work/. The CI runner's versions differ, so its "Check performance counter ceilings" step confirms the ceilings once this is ready.
• Latency arms and a live claude -p run are recorded in probes.md.

Not met: the issue asks for the shape on Windows Git Bash. That needs another host and was not probed, so the owner decides whether the Linux probe suffices. Hence Refs, not Closes.

Related

#5603 (Release 2b plan), #5605 (Release 2 judge).

🤖 Generated with Claude Code

[github-actions]

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

• Missing a native closing keyword (Closes/Fixes/Resolves #N). If this PR references an issue it must not close, put "Refs: #N" (or "Relates to: #N") on its own line. If it relates to no GitHub issue at all, state "No linked issue" (or "No related issue:") in the body instead.

Edit the body and this comment updates itself on the next run.

[chatgpt-codex-connector]

Codex Review Summary

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

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-30T20:34:43.724004Z	751a883	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 finished @kyle-sexton's task in 4m 29s —— View job

---

Claude finished the /review:code-review pass

• Gather context (PR body, changed files, comments)
• Understand the request: invoke /review:code-review for this PR
• Run /review:code-review skill per instructions
• Post inline comments for findings anchored to changed lines (none met the bar — see below)
• Post summary comment

Scope

Reviewed the PR's own commits (c16a820..751a883) against the ten changed files: test-scan-bash.sh (new router hook), the generated hooks.json row, gen-hook-filters.sh/.test.sh, test-scan.test.sh, probes.md, .performance/ratchets.json, and the README/CHANGELOG/plugin.json updates.

Findings

None met the high-signal bar (nothing a careful senior reviewer would block on). What I checked specifically, since this hook does non-trivial jq/bash plumbing:

• Glob routing (test-scan-bash.sh#L30-43): the jq program reliably emits all G-prefixed glob lines before the P-prefixed path lines (jq evaluates comma-separated generators left-to-right to exhaustion), so hook::path_matches always sees a fully populated globs array by the time paths are filtered. Basename extraction (${p##*[/\\]}) is consistent with every shipped Write(...) glob being basename-only (no slash) — verified none of the shipped if rows contain /.
• Created-file handling (test-scan-bash.sh#L57-70): a created file is synthesized as {type: "create", structuredPatch: []} even though bashEditDiff.files[].hunks would be populated for it — intentional, matches hook-precision rule 1 (a create reports every block, not just touched lines) and mirrors how a real Write payload reaches test-scan.sh.
• Path past the 5-file hunk cap: correctly falls through to $d = {} → an Edit with empty structuredPatch, which test-scan.sh's own line-extraction produces no line numbers for, so it reports nothing rather than the whole file (matches the documented scope rule and is covered by test (h) in test-scan.test.sh).
• Timeout budget: TEST_SCAN_TIMEOUT is computed once from the post-cap paths count (8 / ${#paths[@]}), so the division is against the already-capped (≤4) count — no risk of near-zero per-file timeouts from an uncapped denominator.
• docs merge step (test-scan-bash.sh#L78-88): assumes every non-empty per-file output is a {hookSpecificOutput: {additionalContext}} document. Confirmed against hook::finish in hook-utils.sh: the skip/error arms in test-scan.sh call hook::finish without --context, which emits nothing on stdout, so only genuine findings ever reach the docs array — the assumption holds.
• Hook budget honesty: the new Bash row in hooks.json has no if (correctly — a command-string if can't see changedFiles, as probes.md's end-to-end probe demonstrates with the Bash(*.test.ts*) false-negative on a heredoc cat). The two new .performance/ratchets.json ceilings (1, 3) match the numbers stated in the README's "Cost" section and probes.md.

One pre-existing stylistic inconsistency I noticed but did not flag as a finding (doesn't rise to block-worthy): test-scan-bash.sh labels its hook::require_jq call testing rather than a script-specific label like its sibling test-scan.sh uses (test-scan via hook::begin). This only affects the once-per-session jq-missing skip notice text/dedup key, no functional impact, and no collision exists today since it's the only testing-labeled call.

Verification note

Verification claims in the PR body (58/0 and 14/0 test-suite results, ratchet.py check, the mutant test, strace counts, latency numbers, markdownlint-cli2, check-changelog-parity.sh) are author-claimed; I did not independently re-run them and am not restating them as confirmed. The repo's shallow clone in this environment couldn't establish a merge-base against origin/main via git fetch (network commands required approval unavailable in this session), so the diff above was derived from this branch's own commit range rather than a full three-dot diff — file coverage matches the PR's listed changed files.

The PR body's own "Not met" section already discloses the Windows Git Bash gap, which is why it correctly uses Refs #5608 rather than Closes.

[claude]

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

---

Security review (/review:security-review)

• Gather context (PR diff, changed files)
• Run /review:security-review criteria over the diff
• Post findings or report clean

Scope: the new plugins/testing/hooks/test-scan-bash.sh PostToolUse Bash hook, its hooks.json/gen-hook-filters.sh wiring, and the docs/ratchet changes that accompany it.

No security issues found.

Notes from the review (for context, not findings):

• test-scan-bash.sh passes every changed-file path into jq via --arg/--arg binding rather than string interpolation, so there's no injection path from a bashEditDiff.changedFiles entry into the synthetic per-file payload it builds for test-scan.sh.
• tool_response.bashEditDiff is populated by Claude Code itself from the real filesystem diff of a completed Bash call, not free text the model can hand-craft as JSON, so this hook doesn't add a new trust boundary beyond "the agent can already read/write whatever Bash could touch."
• The glob/path extraction at L328-L339 line-splits jq -r output with a G/P prefix convention; a changed-file path containing a literal newline byte could in principle desynchronize a line and inject a spurious glob. I checked the blast radius: the only consumer of globs/paths is which files get scanned for test-quality findings, not a permission or execution gate, and the "attacker" and the "victim" are the same already-Bash-capable session — so there's no privilege crossing here. Not filing it as a finding, flagging only for awareness.
• The PostToolUse Bash row intentionally has no if (documented rationale: a command-string if can't filter on changed files), so it starts unconditionally on every Bash call; that's a documented, ratcheted performance cost, not a security gap — the option-gated script still exits immediately when test_guards_enabled is off or bashEditDiff is absent.
• No instruction-surface deletions: hooks.json only adds a new row; test-weaken (PreToolUse) staying Write/Edit-only is a documented pre-existing limitation, not a guardrail removed by this diff.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 751a88328d

ℹ️ 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".

[claude]

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

---

Security review (/review:security-review)

• Gather context (PR diff, changed files)
• Apply skip gate
• Run /review:security-review criteria over the diff
• Post findings or report clean

No security issues found.

Scope of this pass: HEAD is now fa12cdb, one commit ahead of 751a883 (the SHA the prior /review:security-review run on this PR already covered and cleared). The delta between those two commits is exactly:

• plugins/testing/hooks/test-scan-bash.sh — drops the 2>/dev/null on the bash "$HOOK_DIR/test-scan.sh" call, so a failing/timed-out scanner's test-scan: <file>: scanner exited <rc> diagnostic (written via >&2 at test-scan.sh:104) now reaches the hook's own stderr instead of being discarded — this is the fix for the Codex P2 finding on the prior commit.
• plugins/testing/hooks/test-scan.test.sh — adds a test asserting that diagnostic reaches stderr.

Neither introduces a new trust boundary or injection surface: the added stderr line is a static test-scan: <file>: <reason> string built from the file path already flowing through the hook and a scanner exit code, not attacker-influenceable content beyond what the existing tool_response.bashEditDiff payload already carries (which the prior review on 751a883 already assessed — jq --arg binding for the paths, no shell interpolation, no new privilege crossing). The test-only change has no runtime surface.

I re-read the full current test-scan-bash.sh to confirm nothing else shifted in this delta (jq path/glob extraction, the per-file jq -c --arg payload construction, the unconditional-start rationale for the if-less hooks.json row) — all match what the prior security review on this PR already cleared, and this commit doesn't touch any of it.

Note: shallow checkout meant origin/main/network gh calls weren't usable in this session (same limitation the prior review hit), so this pass diffed directly against the last-reviewed commit (751a883..fa12cdb) rather than a fresh three-dot diff against main — sufficient here since that prior diff's file coverage already matches this PR's full changed-file list.

[claude]

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

---

Claude finished the /review:code-review pass

• Gather context (PR body, changed files, comments)
• Invoke /review:code-review skill for PR feat(testing): scan test files a Bash call changed via bashEditDiff #5621 per custom instructions
• Post inline comments for findings (none met the bar — see below)
• Post summary comment

Scope

This lane already reviewed commit 751a883 with no findings. The head has since moved to fa12cdb, whose only delta is fix(testing): pass test-scan stderr through the Bash route — 2 files, 13 lines — so this pass focused on that commit.

What changed

test-scan-bash.sh#L77 drops the 2>/dev/null on the test-scan.sh child call, so a scanner failure's diagnostic (test-scan: <file>: scanner exited <rc>, written by test-scan.sh#L104) now reaches the hook's own stderr instead of being discarded — matching the existing Write/Edit route and resolving the Codex P2 finding on the prior commit.

This directly fixes the gap: the redirect removed is scoped to only the test-scan.sh invocation (the upstream jq call on line 76 keeps its own 2>/dev/null, which is unrelated and correct), so no new noise is introduced. Because the loop runs per-file with up to MAX_FILES=4 paths, and test-scan.sh's error message includes $FILE, multiple failures across files stay individually attributable on stderr rather than colliding.

New test (k) in test-scan.test.sh#L343-L350 exercises this directly: a scanner stubbed to exit 3 via TEST_SCAN_SCANNER, asserting scanner exited 3 appears on the Bash route's stderr. The assertion string matches the literal format test-scan.sh emits, so the test is not tautological.

Findings

None met the high-signal bar for this delta. The fix is narrowly scoped, correctly addresses the diagnostic-suppression gap Codex flagged, and is covered by a new test that checks the actual failure-message format rather than just "no crash."

Verification note

I read the diff and the surrounding hook code directly; running the test suite (test-scan.test.sh) was not available to me in this session (command execution required approval I don't have), so the PR body's 58/0 test-suite claim and other verification numbers remain author-claimed and unverified by me.
· branch feat/5608-bash-edit-diff-test-scan