feat(review-gate): gate agent commits on a clean code review

[rlorenzo]

What

Adds bin/review-gate, a PreToolUse hook that stops a coding agent from committing code nobody reviewed, plus the review receipt that makes "has this been reviewed" answerable in the first place.

code-review-loop ends on Waiting for manual review. by design, and until now nothing connected that to the commit. An agent could run the loop, ignore the verdict, and commit anyway, or skip the loop entirely.

How it works

The hook is sub-second git plumbing and string parsing. It never runs code-review-loop itself: that takes minutes, spawns nested agent CLIs, and is designed to hand back to a human. Three tiers, cheapest first, and most commits never reach the third.

Tier 1, deterministic allow. Nothing staged or an --amend that only rewords; a clean receipt for exactly this index on exactly this base; a history rewrite in progress (rebase, interactive rebase, cherry-pick, revert, merge, including every --continue step); an index tree identical to ORIG_HEAD or HEAD@{1}, which catches a rewrite whose markers are already cleaned up; a conflicted index; or a bypass. A twelve-commit rebase must not stop to ask twelve times.

Tier 2, deterministic deny with context. The reason carries the staged stat, the changed paths with generated ones flagged, added and deleted lines split into code / tests / docs / generated, why any receipt went stale, a single-use nonce, and the rubric.

Tier 3, agent judgment. No harness hook can render a multi-choice menu, so the decision tree renders one layer up: the agent classifies the change and either re-runs the commit with the nonce, saying out loud why it judged it trivial, or calls AskUserQuestion with the options. A line-count threshold cannot tell a one-line permission check from a two-hundred-line rename, so line counts are evidence handed to the agent, never the decision.

Details worth reviewing closely

• The nonce is read from the command string, not the environment. A PreToolUse hook is spawned before the shell runs the command, so an inline AI_REVIEW_GATE=<nonce> git commit ... never reaches the hook's own environment. Getting this wrong ships an infinite deny loop, so it is the first test in the suite.

• Command parsing is quote-aware. A naive split on ;, &&, || corrupts on the most ordinary input there is: git commit -m "fix: sanitize input; also docs && tests" is one command, not three, and the -a inside a message is not a staging flag.

• -a, --all, -o, --only, -i, --include, and pathspec forms are checked before every index-derived rule. Otherwise git commit -a with a clean index sails through the empty-index check and lands unreviewed work. There is a test for exactly that ordering.

• Unparseable commands fail closed. A spurious prompt is recoverable; a silent miss is not.

• Allow is silence plus exit 0, never an affirmative "allow", which would skip your own permission rules and auto-approve every shell command the agent runs.

• Headless detection is explicit, never a TTY check. Every harness spawns hooks with pipes on all three descriptors, so keying off [ -t 0 ] would degrade every interactive run and quietly turn the gate off while still looking installed. There is a test pinning the inverse: no TTY on any descriptor, no headless marker, still blocks.

• The recursion guard is two independent mechanisms. run_agent exports AI_REVIEW_GATE=off, and code-review-loop holds a lock file the gate validates by PID liveness and age rather than merely observing, since an EXIT trap does not run on SIGKILL and a stale lock would disable the gate forever in exactly the situation where nobody thinks to look.

• The command word is matched after normalisation, and wrappers are stepped through. A gate that only recognises a bare git enforces nothing: git.exe, env -i git, command git, sudo git, and env -S "git commit" are all ordinary commits. Each has a test.

Also in here

The reviewer-side prompts have always said "Do not stage, commit, or push." The two editor-side prompts did not, and they run with full write access in every harness. First commit fixes that. It is worth doing regardless of the gate, and becomes load-bearing once the gate exists.

A chore: commit also gitignores root-anchored PLAN-*.md, the working plan documents plan-review-loop operates on. The anchor matters: macOS and Windows check out with core.ignorecase set, where an unanchored pattern would also swallow prompts/plan-review.md and its neighbours, which this repo ships.

Defaults and scope

Ships in warn mode: prints the reason, lets the commit through. The rubric is untested against real commits and a wrong block is far more annoying than a wrong warn. Switch to REVIEW_GATE=block in ~/.ai-coding-setup.conf once a few weeks of warn output shows it is not crying wolf.

setup wires it for Claude Code only. The script speaks every harness's output shape via --format, but Copilot fails closed on a parse error, so writing an unverified schema into a user's config would deny every shell call in the session. The other four are documented for manual wiring, with their formats, once you confirm the paths against current docs.

Two deliberate omissions, both for the same reason: I would not ship a config schema or an environment marker I could not verify. Headless detection wanted per-CLI non-interactive markers; I could only verify CI, so the list is AI_REVIEW_HEADLESS=1 plus CI, with the gap noted in a comment.

Known blind spot

A commit made inside a script the agent invokes is invisible, because the gate only ever sees the command the agent typed. Accepted for v1. Nothing short of a git-level hook closes it, and a git-level hook cannot ask a question, so it would only ever warn after the fact.

Testing

164 bats tests, all passing on Linux and Windows; shellcheck -x clean; markdownlint clean; tools/generate --check in sync. The suite covers a real git rebase -i fixture, a linked worktree, an unborn HEAD, a corrupt receipts file, nonce reuse and cross-tree rejection, stale locks by dead PID and by age, each --format output shape, and one case per command form that can reach a commit.

Bugs caught by dogfooding the gate on this change:

• macOS still ships bash 3.2, which has no associative arrays.
• BSD sed has no \| alternation in a basic regex, which silently made every generated file classify as docs.

Bugs caught by review, all reproduced against the gate before being fixed:

• env -i and git.exe slipped past commit detection. Testing the shape turned up three more: env -u NAME, command, and sudo.
• env -S "git commit" was skipped as if it were an option's value, and once that was fixed, the operands after the split string were dropped. env -S "git" commit was a live bypass, and env -S "git commit" -a was worse: denied, but with the staging flag lost, so a receipt describing the index would have vouched for a working-tree commit.
• setup accepted a dangling symlink as an installed gate, and wrote the hook command with an unquoted path, which a profile directory containing a space would split into two arguments.

One review finding was a false positive and is documented rather than "fixed": the token arrays are expanded with ${arr[@]+"${arr[@]}"}, whose alternate value is quoted, so spaced commit messages are not split. There is now a test pinning that.

Windows

Verified on Windows, and covered by a windows-latest CI job running the full suite under Git Bash. The gate began as macOS-only in practice; this is what it took to make it real there.

Claude Code runs hook commands through Git Bash on Windows by default, falling back to PowerShell only when Git Bash is absent, so the Git Bash style path setup writes resolves as written. jq is the one dependency not bundled with Git for Windows, and without it the gate allows every commit rather than failing, so confirm it is on PATH.

• The matcher has to name both shell tools. Windows exposes a PowerShell tool alongside Bash, and a matcher of Bash alone lets every commit made through the other one past without even a warning. setup writes Bash|PowerShell, and widens an existing Bash-only entry in place when re-run.
• The bypass takes PowerShell syntax there. A bash AI_REVIEW_GATE=off git commit ... prefix is a parse error in PowerShell, so the gate reads the $env:AI_REVIEW_GATE = "..." statement form too and renders the nonce and escape hatch in whichever syntax matches the tool the commit came from. Either way it has to ride on the same command as the commit: the hook is a separate process spawned before the command runs, so it never inherits a variable set in an earlier call.
• Backslash is a path character, not an escape. For PowerShell commands the tokenizer stops folding it away, without which & "C:\Program Files\Git\bin\git.exe" commit became a word matching nothing.

Cost worth knowing: the hook fires on every shell call, and Git Bash process startup is slow. Roughly 160ms warm against 55ms for a bare bash -c true, and over a second on a cold file cache. Milliseconds on macOS and Linux.

setup falls back to copying when ln -s cannot make a real symlink, which is the default on Windows without Developer Mode. That works, but the installed script is then a snapshot rather than a link, so re-run setup to pick up changes.

[Copilot]

🟡 Changes recommended

The new gate’s commit detection can be bypassed by valid command forms (e.g., env -i … or git.exe …), which undermines enforcement in block mode.

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

Pull request overview

This PR introduces a new “review gate” mechanism that can be installed as a PreToolUse hook to prevent (or warn on) agent-driven git commit actions unless the staged index has a matching clean review receipt, and wires the receipt/lock plumbing into the existing review loop tooling.

Changes:

• Add bin/review-gate hook implementation plus a comprehensive BATS test suite covering parsing, receipts, nonce behavior, and platform-specific behaviors.
• Extend lib/lib-review-loop and bin/code-review-loop to record review receipts and maintain a lock so the gate can safely exempt commits during review-loop runs.
• Update setup/docs/prompts/config to document, install, and operationalize the gate (including Windows CI coverage for BATS).

File summaries

File	Description
test/review-gate.bats	New end-to-end BATS coverage for review-gate behavior (nonce, receipts, parsing, exemptions, formats).
test/lib-review-loop.bats	Adjust is_inside_dir tests to handle Windows symlink limitations and add conditional symlink test.
setup	Adds optional Claude Code hook wiring for review-gate and installs the script with the review-loop suite.
README.md	Documents review-gate behavior, modes, installation, escape hatches, and Windows caveats.
prompts/code-review-response.md	Adds explicit “do not stage/commit/push” guidance to reviewer response prompt.
prompts/code-refinement.md	Adds explicit “do not stage/commit/push” guidance to refinement prompt.
lib/lib-review-loop	Adds REVIEW_GATE config loading, recursion guard env export, receipt writing, and lock helpers.
bin/review-gate	New hook script implementing parsing + tiered allow/deny/warn behavior, receipts/nonces, and context output.
bin/code-review-loop	Acquires/releases gate lock and records a review receipt at loop completion.
.kimi-code/skills/code-refinement/SKILL.md	Mirrors “do not stage/commit/push” guidance for Kimi skill prompt.
.copilot/skills/code-refinement/SKILL.md	Mirrors “do not stage/commit/push” guidance for Copilot skill prompt.
.codex/skills/code-refinement/SKILL.md	Mirrors “do not stage/commit/push” guidance for Codex skill prompt.
.claude/commands/code-refinement.md	Mirrors “do not stage/commit/push” guidance for Claude command prompt.
.antigravity/skills/code-refinement/SKILL.md	Mirrors “do not stage/commit/push” guidance for Antigravity skill prompt.
.gitignore	Ignores root-anchored PLAN-*.md working artifacts.
.github/workflows/lint.yml	Adds a Windows GitHub Actions job to run the BATS suite under Git Bash.
.gitattributes	Marks generated skill/prompt outputs as linguist-generated for classification by review-gate.

Review details

• Files reviewed: 14/17 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

The Claude hook wiring in setup has correctness issues (dangling symlink accepted; hook command not robust to spaces in $HOME) that can break hook execution.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

setup:1130

• configure_review_gate_hook treats any symlink at ~/.local/bin/review-gate as sufficient, even if it is broken. [[ -L ... ]] is true for a dangling symlink, which would cause setup to wire a hook pointing at a non-existent executable and likely break Claude hook execution.

Prefer requiring an executable file (or an executable symlink target) via -x only.

This issue also appears on line 1180 of the same file.

    local gate="$HOME/.local/bin/review-gate"
    local settings_file="$HOME/.claude/settings.json"

    [[ -x "$gate" || -L "$gate" ]] || return 0

setup:1186

• The hook command string is written as $HOME/.local/bin/review-gate --format=claude without quoting the path. If $HOME contains spaces (common on Windows profiles and possible elsewhere), the hook runner will split the command and fail to execute the gate.

Quote the executable path when building the command string so it remains a single argv element under shell parsing.

    local updated
    updated=$(jq --arg cmd "$gate --format=claude" '
        .hooks = (.hooks // {})
        | .hooks.PreToolUse = ((.hooks.PreToolUse // []) + [{
            matcher: "Bash|PowerShell",
            hooks: [{type: "command", command: $cmd}]
          }])' "$settings_file") || {

• Files reviewed: 14/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

bin/review-gate currently allows a practical bypass via env -S/--split-string where git commit is embedded in the split-string argument but not detected.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test/review-gate.bats:226

• There’s no test covering the env -S/--split-string bypass case (where the command is embedded in the split-string argument). Given this is a realistic way to wrap git commit, the suite should pin that env -S "git commit ..." is still detected and denied.

@test "env -u NAME git commit is detected" {
    setup_gate; mkrepo; stage_code
    run gate 'env -u FOO git commit -m "add app"'
    assert_denied
}

• Files reviewed: 14/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The env -S/--split-string parsing currently ignores arguments appended after the split-string value, which can cause the gate to miss commits or staging flags in valid env -S forms.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

test/review-gate.bats:2

• The header comment calls this a “pre-commit” gate, but it’s a harness PreToolUse hook (not a git pre-commit hook). This is likely to confuse readers and future maintainers when debugging why it runs.

# Tests for bin/review-gate: the pre-commit code review gate.

• Files reviewed: 14/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

bin/review-gate’s token iteration/copying uses unquoted array expansions that can split quoted tokens containing spaces (e.g., commit messages), breaking the core quote-aware parsing guarantees.

Review details

• Files reviewed: 14/17 changed files
• Comments generated: 0 new
• Review effort level: Lite