Low-priority cleanup items surfaced during the #72 review. Grouping as one issue since each is small.
1. security-patterns.sh: per-pattern dedup test
`hooks_test.sh` verifies that running the same pattern twice produces no second warning, but doesn't verify that different patterns on the same file each warn independently. A regression that flattens the dedup key to a global "warn-once-ever" flag would pass the current test.
Fix: add a follow-up assertion that runs `pickle.load` then `eval(` on the same file and asserts both warn.
Source: `pr-review-toolkit:pr-test-analyzer`, criticality 6.
2. security-patterns.sh and post-validate.sh: missing `set -euo pipefail`
Per `~/.claude/rules/shell.md`, `set -euo pipefail` is required on all shell scripts. `rtk-rewrite.sh` now has it (after #72); `security-patterns.sh` and `post-validate.sh` do not. Pre-existing, out of scope for #72, but worth closing the gap.
Caveat: adding `set -e` requires the same jq-tolerance treatment as rtk-rewrite — any `VAR=$(cmd)` that can fail silently needs a `|| true` guard. See #72's rtk-rewrite.sh fix for the pattern.
Source: `pr-review-toolkit:code-reviewer` (nit, confidence 80).
3. Comment phrasing: trim historical narrative
Three spots flagged by `pr-review-toolkit:comment-analyzer` will read as archaeology once the fixes are ancient. They reference "the previous implementation," "silently fell back," "pins the X fix," etc.
- `hooks/post-validate.sh:53-59` — keep the GNU-ism note, drop "so the previous implementation silently fell back to the raw FILE_PATH and mis-classified every relative path on macOS as 'outside repo.'"
- `hooks/hooks_test.sh` dedup test opening — "This pins the $$→$PPID fix" → "This test asserts that dedup survives across hook invocations via $PPID."
- `hooks/hooks_test.sh` realpath test — trim "Pins the realpath -m GNU-ism fix: on macOS the old implementation's fallback kept $FILE_PATH unresolved…" to just the invariant.
The dedup-test topology comment (the multi-paragraph one about bash `$()` subshells) should stay — it prevents a real future bug and was called out by the reviewer as the best comment in the PR.
Source: `pr-review-toolkit:comment-analyzer`.
4. rtk-rewrite: rc 3 with rewrite == input
Low priority. The current tests cover rc 3 where the rewrite differs from input, and the rewrite-equality guard independently. A combined test (rc 3 + rewrite == input) would pin the interaction. Minor.
Source: `pr-review-toolkit:pr-test-analyzer`, criticality 5.
Low-priority cleanup items surfaced during the #72 review. Grouping as one issue since each is small.
1. security-patterns.sh: per-pattern dedup test
`hooks_test.sh` verifies that running the same pattern twice produces no second warning, but doesn't verify that different patterns on the same file each warn independently. A regression that flattens the dedup key to a global "warn-once-ever" flag would pass the current test.
Fix: add a follow-up assertion that runs `pickle.load` then `eval(` on the same file and asserts both warn.
Source: `pr-review-toolkit:pr-test-analyzer`, criticality 6.
2. security-patterns.sh and post-validate.sh: missing `set -euo pipefail`
Per `~/.claude/rules/shell.md`, `set -euo pipefail` is required on all shell scripts. `rtk-rewrite.sh` now has it (after #72); `security-patterns.sh` and `post-validate.sh` do not. Pre-existing, out of scope for #72, but worth closing the gap.
Caveat: adding `set -e` requires the same jq-tolerance treatment as rtk-rewrite — any `VAR=$(cmd)` that can fail silently needs a `|| true` guard. See #72's rtk-rewrite.sh fix for the pattern.
Source: `pr-review-toolkit:code-reviewer` (nit, confidence 80).
3. Comment phrasing: trim historical narrative
Three spots flagged by `pr-review-toolkit:comment-analyzer` will read as archaeology once the fixes are ancient. They reference "the previous implementation," "silently fell back," "pins the X fix," etc.
The dedup-test topology comment (the multi-paragraph one about bash `$()` subshells) should stay — it prevents a real future bug and was called out by the reviewer as the best comment in the PR.
Source: `pr-review-toolkit:comment-analyzer`.
4. rtk-rewrite: rc 3 with rewrite == input
Low priority. The current tests cover rc 3 where the rewrite differs from input, and the rewrite-equality guard independently. A combined test (rc 3 + rewrite == input) would pin the interaction. Minor.
Source: `pr-review-toolkit:pr-test-analyzer`, criticality 5.