Round-4 review caught the miss that matters most: #643 fixed the `jq -e`-on-empty
fail-open in .claude/hooks/pretooluse-merge-consent.sh, but the SAME construct sits in
.gitea/workflows/review-verdict.yml — and that is the copy that runs on the CI runner,
where jq is 1.6, and that feeds the branch-protection-required review-verdict/h10
status.
Reproduced: `printf "" | jq -e '.statuses | type == "array"'` exits 4 on jq 1.8.2 (guard
fires, correct) and 0 on jq 1.6 (guard passes). So on a transient API error `statusjson`
is empty, the guard lets it through, `existing` reads "", and the job posts `pending` —
or for a bot/docs-only PR an exemption `success` — over a possibly-existing human
verdict. That is precisely what the comment three lines above it says must never happen.
The hook version was harmless in practice because it runs on a dev Mac with jq 1.8. This
one is live. Fixed identically, with a comment naming why the sibling fix missed it, and
the same hardening applied to the changed-files read in the same workflow.
Also from round 4:
- LOW, reproduced — an ARRAY-valued `.status` dodged the closed allow-list. `index` is
polymorphic: with an array argument it does SUBSEQUENCE matching, so
`[...,"renamed",...] | index(["renamed"])` is truthy while `.status == "renamed"` is
false — the row passed the allow-list AND skipped the `previous_filename REQUIRED`
clause. The same `git mv code -> docs/` dodge the closed set exists to block, one type
away. Now requires `.status` to be a string first; mutation-verified.
- The record now carries all THREE jq rules rather than the one, and notes that the
durable fix is pinning/preflighting the runner's jq version rather than patching
constructs one at a time (tracked on #647).
200 tests pass under BOTH jq 1.8.2 and jq 1.6.
Refs #647, #643, #631
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5
bb1809fbf0