perf(review-gate): cut the forks on the hook's hot path

[rlorenzo]

What

bin/review-gate is a PreToolUse hook, so it runs on every shell call the agent makes, not just on commits. Most of its work was spent on subprocesses that a shell builtin could do. This removes those, fixes a gate bypass found along the way, and fixes two output behaviors.

The fork removals were written on macOS against the pre-Windows base and never made it into #27. They are ported here onto the current main, so all of the Windows and PowerShell handling from that PR is untouched.

The gate bypass (pre-existing on main)

git commit --all was allowed outright when it ended a CRLF line.

jq is a native Windows binary under Git Bash and ends its lines CRLF, so the hook input reaches the tokenizer with CRs in it. The tokenizer splits on space, tab, and newline but not CR, so the flag tokenized as --all<CR>, matched none of the staging-flag cases, and the command read as a plain commit of an empty index. Same for --only, --include, and --patch. Verified against origin/main:

main: --all  -> ALLOWED (bypassed)
main: --patch -> ALLOWED (bypassed)

The fix normalizes CRLF to LF once on the hook input. Nothing here ever executes the command, so rewriting its line endings is free.

Sharing one jq capture made the same CR matter in a second place, which is how this surfaced. Git Bash's command substitution strips only the trailing CR, so every field but the last kept one. On the cwd that fails the -d test, and the gate inspects $PWD instead of the repo being committed to, which is another silent allow. On the tool name it stops PowerShell from being recognized. The first push here was green everywhere except the Windows CI job from #27, which caught exactly that.

Fork removals

Change	Why
IFS= read -r -d '' INPUT instead of $(cat)	Forked a subshell and exec'd cat on every invocation, including the ls and cat calls the gate exits on two lines later. The only change on the true hot path; the rest run once a commit is actually being gated
One jq for cwd, tool name, and command instead of three	Two fewer interpreter startups per gated commit
read -r for the lock and nonce files instead of $(cat)	Both are single-line
Generated-file membership by substring against a newline-delimited string	grep -qxF in a pipeline forked twice per changed file
classify_path sets CATEGORY instead of printing it	A command substitution forked a subshell per changed file

The combined jq puts the command last: it may legitimately contain newlines, which the tokenizer relies on as separators, so it takes everything after the second line and is never flattened. An empty command leaves no third line, which is why _REST is re-tested afterward.

Behavior fixes

Warn mode stops after the diagnosis. It was printing the full classify-and-choose menu, including the AskUserQuestion options and the escape hatches, for a commit that proceeds regardless. That asks the agent to decide something that has already been decided. It now prints the diff, the category counts, and the receipt status, then says the commit is proceeding and how to switch to block mode.

git commit --interactive is reported as --interactive. Both flags were being relabeled --patch in the reason, so the message did not match what the user typed.

Not included

The original local version also rewrote SCRIPT_DIR with parameter expansion. Dropped: it sits after the fast-path filter, so it never ran on the hot path this PR is about, and ${0%/*} does not match a backslash-separated $0. Two forks on a cold path are not worth the Windows exposure.

It also dropped the bash 3.2 ${TOKENS[@]+"${TOKENS[@]}"} idiom and reworded the nonce hint in a way that bypassed with_gate. Both were left alone here, since they would have undone #27.

Also

Names PATH_LIST_MAX and the six-hour lock bound, reuses the existing source_lib test helper in three places, and corrects the marker-list comment, which already covered BISECT_LOG without saying so.

Testing

• 170 BATS tests, 0 failures. Six are new: the CRLF staging-flag bypass, CRLF as a command separator, warn mode omitting the menu, block mode keeping it, and the two flag-label cases.
• shellcheck -x clean on both files.
• The gate fired on this branch's own commits and printed the new warn-mode output correctly.

[Copilot]

🟡 Changes recommended

There is a confirmed CRLF-related parsing issue risk in COMMAND handling (tokenizer doesn’t treat \r as whitespace) and a stated fork-removal discrepancy that should be resolved before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR optimizes bin/review-gate (a PreToolUse hook that runs on every agent shell invocation) by removing hot-path subprocess overhead, while also fixing two user-visible behaviors (warn-mode output and --interactive flag reporting) and expanding test coverage accordingly.

Changes:

• Reduce per-invocation overhead by replacing frequent forks (e.g., $(cat) and multiple jq invocations) with shell builtins and a single jq extraction.
• Fix behavior: warn mode now prints only the diagnosis (not the decision menu), and git commit --interactive is reported correctly.
• Add BATS tests covering warn/block output differences and --interactive/--patch labeling.

File summaries

File	Description
bin/review-gate	Cuts hot-path forks (read/jq/cat/grep) and adjusts output/flag reporting logic.
test/review-gate.bats	Adds regression tests for warn-mode output suppression and correct flag labeling.

Review details

• Files reviewed: 1/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Two new read usages likely won’t reliably suppress redirection errors due to redirection ordering, which can reintroduce noisy stderr output compared to the prior cat ... 2>/dev/null behavior.

Review details

• Files reviewed: 1/2 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are narrowly scoped, improve correctness and performance, and are backed by targeted new tests for the reported bypass and output regressions.

Review details

• Files reviewed: 1/2 changed files
• Comments generated: 0 new
• Review effort level: Lite