diff --git a/.antigravity/skills/code-refinement/SKILL.md b/.antigravity/skills/code-refinement/SKILL.md index bcb5593..708572e 100644 --- a/.antigravity/skills/code-refinement/SKILL.md +++ b/.antigravity/skills/code-refinement/SKILL.md @@ -14,3 +14,5 @@ Verify that the project's UI framework components and utility classes are used w Run the project's linting command and fix all reported errors and warnings. Discover the command from package scripts, a Makefile, CI config, or pre-commit config; if the project has no linter, note that and move on. Avoid using lint-suppression comments (e.g., eslint-disable, noqa, @ts-ignore) to make the lint pass unless absolutely necessary, and only with a clear justification in the code. Review tests and code coverage: check whether existing tests adequately cover the new or modified code, add tests for any gaps you find, and update any existing tests that must change to handle the new behavior correctly. When finished, ensure everything is ready for a high-quality code review. + +Do not stage, commit, or push. Leave every change in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. diff --git a/.claude/commands/code-refinement.md b/.claude/commands/code-refinement.md index 4eb105d..897bb28 100644 --- a/.claude/commands/code-refinement.md +++ b/.claude/commands/code-refinement.md @@ -13,3 +13,5 @@ Verify that the project's UI framework components and utility classes are used w Run the project's linting command and fix all reported errors and warnings. Discover the command from package scripts, a Makefile, CI config, or pre-commit config; if the project has no linter, note that and move on. Avoid using lint-suppression comments (e.g., eslint-disable, noqa, @ts-ignore) to make the lint pass unless absolutely necessary, and only with a clear justification in the code. Review tests and code coverage: check whether existing tests adequately cover the new or modified code, add tests for any gaps you find, and update any existing tests that must change to handle the new behavior correctly. When finished, ensure everything is ready for a high-quality code review. + +Do not stage, commit, or push. Leave every change in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. diff --git a/.codex/skills/code-refinement/SKILL.md b/.codex/skills/code-refinement/SKILL.md index bcb5593..708572e 100644 --- a/.codex/skills/code-refinement/SKILL.md +++ b/.codex/skills/code-refinement/SKILL.md @@ -14,3 +14,5 @@ Verify that the project's UI framework components and utility classes are used w Run the project's linting command and fix all reported errors and warnings. Discover the command from package scripts, a Makefile, CI config, or pre-commit config; if the project has no linter, note that and move on. Avoid using lint-suppression comments (e.g., eslint-disable, noqa, @ts-ignore) to make the lint pass unless absolutely necessary, and only with a clear justification in the code. Review tests and code coverage: check whether existing tests adequately cover the new or modified code, add tests for any gaps you find, and update any existing tests that must change to handle the new behavior correctly. When finished, ensure everything is ready for a high-quality code review. + +Do not stage, commit, or push. Leave every change in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. diff --git a/.copilot/skills/code-refinement/SKILL.md b/.copilot/skills/code-refinement/SKILL.md index bcb5593..708572e 100644 --- a/.copilot/skills/code-refinement/SKILL.md +++ b/.copilot/skills/code-refinement/SKILL.md @@ -14,3 +14,5 @@ Verify that the project's UI framework components and utility classes are used w Run the project's linting command and fix all reported errors and warnings. Discover the command from package scripts, a Makefile, CI config, or pre-commit config; if the project has no linter, note that and move on. Avoid using lint-suppression comments (e.g., eslint-disable, noqa, @ts-ignore) to make the lint pass unless absolutely necessary, and only with a clear justification in the code. Review tests and code coverage: check whether existing tests adequately cover the new or modified code, add tests for any gaps you find, and update any existing tests that must change to handle the new behavior correctly. When finished, ensure everything is ready for a high-quality code review. + +Do not stage, commit, or push. Leave every change in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. diff --git a/.gitattributes b/.gitattributes index a049eeb..dd672cf 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,12 @@ # Enforce Unix line endings for all text files in this repo * text=auto eol=lf + +# Derived from .claude/commands/ by tools/generate; never hand-edited. +# linguist-generated is what bin/review-gate reads to tell a generated +# artifact from a source change when it classifies a staged diff. +.codex/skills/** linguist-generated=true +.copilot/skills/** linguist-generated=true +.antigravity/skills/** linguist-generated=true +.kimi-code/skills/** linguist-generated=true +prompts/code-refinement.md linguist-generated=true +prompts/code-review.md linguist-generated=true diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 248ff67..10e1974 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -19,3 +19,24 @@ jobs: with: python-version: "3.x" - uses: pre-commit/action@v3.0.1 + + # The scripts here are Bash, but they run under Git Bash on Windows too, and + # bin/review-gate in particular has to cope with the `C:\...` paths and the + # separate PowerShell tool a Windows harness hands it. Only the test suite + # runs here; shellcheck and markdownlint are platform-independent and already + # covered above. + test-windows: + name: BATS on Windows + runs-on: windows-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: true + - name: Give git an identity for the throwaway repos the tests build + shell: bash + run: | + git config --global user.email ci@example.com + git config --global user.name CI + - name: Run BATS under Git Bash + shell: bash + run: test/run diff --git a/.gitignore b/.gitignore index 2c2eb8c..b7f5f2f 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,11 @@ agent-review-summary.md feedback-plan.md plan-review-summary.md +# Plan documents written for plan-review-loop. Working artifacts, not +# deliverables. Anchored to the root so the pattern cannot reach the prompts +# of similar name in prompts/, which Windows checkouts match case-insensitively. +/PLAN-*.md + # Impeccable hook installer artifacts (machine-specific absolute paths) .claude/settings.local.json .codex/hooks.json diff --git a/.kimi-code/skills/code-refinement/SKILL.md b/.kimi-code/skills/code-refinement/SKILL.md index bcb5593..708572e 100644 --- a/.kimi-code/skills/code-refinement/SKILL.md +++ b/.kimi-code/skills/code-refinement/SKILL.md @@ -14,3 +14,5 @@ Verify that the project's UI framework components and utility classes are used w Run the project's linting command and fix all reported errors and warnings. Discover the command from package scripts, a Makefile, CI config, or pre-commit config; if the project has no linter, note that and move on. Avoid using lint-suppression comments (e.g., eslint-disable, noqa, @ts-ignore) to make the lint pass unless absolutely necessary, and only with a clear justification in the code. Review tests and code coverage: check whether existing tests adequately cover the new or modified code, add tests for any gaps you find, and update any existing tests that must change to handle the new behavior correctly. When finished, ensure everything is ready for a high-quality code review. + +Do not stage, commit, or push. Leave every change in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. diff --git a/README.md b/README.md index 43e69af..9e85dba 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,7 @@ Trade-off to know about: a custom `Explore` loads your `CLAUDE.md`/user memory l ## Review Loops -Two multi-agent feedback loops live in [bin/](bin/): `code-review-loop` (for staged code) and `plan-review-loop` (for plan documents). Each loop pairs an **editor** agent with a different **reviewer** agent and iterates until the reviewer is satisfied or `--max-iterations` is hit. Using two different models for editing and reviewing surfaces issues a single agent tends to miss in its own output. +Two multi-agent feedback loops live in [bin/](bin/): `code-review-loop` (for staged code) and `plan-review-loop` (for plan documents), alongside `review-gate`, the hook that keeps an agent from committing before the first of those has run. Each loop pairs an **editor** agent with a different **reviewer** agent and iterates until the reviewer is satisfied or `--max-iterations` is hit. Using two different models for editing and reviewing surfaces issues a single agent tends to miss in its own output. Both scripts are installed onto your `PATH` by `./setup` and rely on the prompts in [prompts/](prompts/) (installed to `~/.local/share/ai-coding-setup/prompts/`). @@ -228,6 +228,82 @@ plan-review-loop --reviewer claude --editor codex PLAN-feature.md **Outputs (project root):** the plan file is edited in place; `feedback-plan.md` (latest feedback, removed when reviewer is satisfied); `plan-review-summary.md` (narrative). +### review-gate + +A hook that stops a coding agent from committing code nobody reviewed. + +Every harness has a pre-tool event that can deny a tool call. `review-gate` sits on that event, watches for a `git commit`, and answers one question in well under a second: does this staged change already have a clean review? If it does, or if the commit is not really a code change at all, the agent never sees the gate. If it does not, the gate denies the commit and hands the agent the staged diff, per-category line counts, and a rubric, and the agent either judges the change trivial and says so out loud, or asks you what to do. + +The gate never runs `code-review-loop` itself. That takes minutes and is designed to hand back to a human at the end; the gate is pure git plumbing, and the loop runs afterward as an ordinary foreground command if you pick that option. + +**What passes without a word:** + +- nothing staged, or an `--amend` that only rewords +- a clean review receipt for exactly this index on exactly this base, which is what `code-review-loop` records when it finishes +- a history rewrite: rebase, interactive rebase, cherry-pick, revert, or merge, including every `--continue` step. A twelve-commit rebase must not stop to ask twelve times +- an index tree identical to `ORIG_HEAD` or `HEAD@{1}`, which catches a rewrite whose in-progress markers are already cleaned up +- an unresolved merge conflict in the index, which means a merge is in progress anyway +- `AI_REVIEW_GATE=off` on the command, or `REVIEW_GATE=off` in the config + +**What always gets stopped:** `git commit -a`, `git commit `, and the `-o` / `--only` / `-i` / `--include` forms. They commit content that was not in the index when the gate ran, so a matching receipt describes something else. That check runs before every index-derived rule, or `git commit -a` with a clean index would sail through the empty-index check and land unreviewed work. + +**Modes**, set with `REVIEW_GATE` in `~/.ai-coding-setup.conf` or in the environment (the environment wins): + +| Mode | Behavior | +| --- | --- | +| `warn` | Default. Prints the reason and lets the commit through. | +| `block` | Denies the commit and hands the agent the reason. | +| `off` | Disabled. | + +It ships in `warn` because the rubric is untested against your commits and a wrong `block` is far more annoying than a wrong `warn`. Once a few weeks of warn output shows it is not crying wolf, switch to `block`. + +**Escape hatches**, in the order you are likely to want them: + +- `AI_REVIEW_GATE=off git commit -m "..."` bypasses one commit. In PowerShell, where that prefix form is a parse error, write it as `$env:AI_REVIEW_GATE = "off"; git commit -m "..."`. The gate reads the bypass off the command string rather than its own environment, so it has to sit on the same command as the commit either way. +- The gate issues a single-use nonce with every block, scoped to the index as it stands. The agent uses it to act on its own trivial-change judgment: `AI_REVIEW_GATE= git commit -m "..."`. Staging more work invalidates it, and it works once. +- `REVIEW_GATE=off` in `~/.ai-coding-setup.conf` turns the gate off everywhere. + +**Headless runs degrade to warn.** In CI, or under `AI_REVIEW_HEADLESS=1`, there is nobody to ask, and a gate that hard-blocks there deadlocks the build. Detection is explicit and never a TTY check: every harness spawns hooks with pipes on all three descriptors, so keying off `[ -t 0 ]` would degrade every interactive run too, and quietly turn the gate off everywhere while still looking installed. + +**Installation.** `./setup` offers to wire it into Claude Code, appending a `PreToolUse` hook to `~/.claude/settings.json` without disturbing hooks that are already there. The script speaks every harness's output shape via `--format`, but only Claude is wired automatically, because the other four take different config shapes and paths that are worth confirming against their current docs before writing into your config: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash|PowerShell", + "hooks": [ + { "type": "command", "command": "\"$HOME/.local/bin/review-gate\" --format=claude" } + ] + } + ] + } +} +``` + +Available formats: `claude` and `codex` emit the decision nested under `hookSpecificOutput`; `copilot` and `antigravity` emit a flat `{"permissionDecision": ..., "permissionDecisionReason": ...}`; `kimi` is exit-code driven, blocking with exit 2 and the reason on stderr. The allow path is always silence and exit 0, never an affirmative `"allow"`: an affirmative allow from a `PreToolUse` hook would skip your own permission rules and auto-approve every shell command the agent runs. + +**Windows.** The gate is a Bash script, so it needs Git Bash, like the rest of this repo. Everything it calls is bundled with Git for Windows except `jq`, which you install separately; without `jq` the gate allows every commit rather than failing, so check that it is on `PATH` before trusting the gate 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 into `~/.claude/settings.json` (`/c/Users/you/.local/bin/review-gate`) resolves as written. + +Three Windows specifics are worth knowing: + +- **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 straight past the gate. `./setup` writes `Bash|PowerShell`, and widens an existing `Bash`-only entry in place when you re-run it. +- **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 statement form too, and writes whichever one matches the tool the commit is coming from into its own deny message: `$env:AI_REVIEW_GATE = "off"; git commit ...`. Either way it has to ride on the same command as the commit. The gate is a separate process spawned before your command runs, so it never inherits a variable you set in an earlier call; it can only read what is on the command string in front of it. +- **It costs about 150ms per shell call.** The hook fires on every command the agent runs, not just commits, and process startup under Git Bash is far slower than on macOS or Linux. The non-commit fast path exits before any git call or subshell, but bash itself still has to start. Measured here: roughly 160ms warm against 55ms for a bare `bash -c true`, and over a second on a cold file cache. + +`./setup` falls back to copying when `ln -s` cannot make a real symlink, which is the default on Windows unless Developer Mode is on. That works, but `~/.local/bin/review-gate` is then a snapshot rather than a link, so re-run `./setup` to pick up changes to the script. + +**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. Nothing short of a git-level hook closes that, and a git-level hook cannot ask a question, so it would only ever warn after the fact. + +**State** lives in `$(git rev-parse --git-dir)/ai-review/`, so it is never committed, is per-worktree, and survives branch switches: + +| File | Contents | +| --- | --- | +| `receipts.json` | The last 10 review results, newest first: index tree, HEAD, verdict, cycles, agents, timestamp. | +| `nonce` | The outstanding single-use bypass and the index tree it was issued for. | +| `running` | The active `code-review-loop`'s PID and start time. The gate exempts commits while the loop runs, and prunes the file when the PID is dead or the timestamp is too old to trust. | + ### Configuration Defaults are `--editor claude --reviewer codex`. Override per-run with `-e` / `-r`, or persist defaults in `~/.ai-coding-setup.conf`: @@ -235,6 +311,7 @@ Defaults are `--editor claude --reviewer codex`. Override per-run with `-e` / `- ```ini EDITOR_AGENT=claude REVIEWER_AGENT=codex +REVIEW_GATE=warn ``` Supported agents: `claude`, `codex`, `copilot`, `antigravity`, `kimi`. Only the agents you actually have installed need to be referenced. @@ -250,6 +327,10 @@ Environment variables: | `CODE_REVIEW_LOOP_LOG_DIR` | `~/.cache/code-review-loop/` | Where `code-review-loop` writes its run logs. Setting it also turns off log pruning, on the grounds that a directory you named is yours to manage. | | `REVIEW_LOOP_LOG_DAYS` | `1` | Delete run logs older than this many days. Only applies to the default location. | | `AI_CODING_SETUP_PROMPTS_DIR` | `~/.local/share/ai-coding-setup/prompts` | Where the loops read their prompts from. | +| `REVIEW_GATE` | `warn` | `review-gate` mode: `off`, `warn`, or `block`. Overrides the config file. | +| `AI_REVIEW_GATE` | unset | `off` bypasses the gate for one invocation. Also carries the single-use nonce the gate issues. Read off the command string, as a `VAR=value` prefix or a PowerShell `$env:AI_REVIEW_GATE = "..."` statement, so it must sit on the same command as the commit. | +| `AI_REVIEW_HEADLESS` | unset | `1` degrades the gate to warn-only. `code-review-loop` sets it for its own run. | +| `REVIEW_GATE_LOCK_MAX_AGE` | `21600` | Seconds before the gate stops trusting a `code-review-loop` lock file and prunes it. | ### Shared prompts @@ -397,6 +478,8 @@ Delete the command/skill from the corresponding directory (or uninstall the plug - Antigravity: Run `agy plugin uninstall ai-coding-setup` - Kimi Code: `~/.kimi-code/skills/` (or `$KIMI_CODE_HOME/skills/`) +If you installed the review gate hook, remove its `PreToolUse` entry from `~/.claude/settings.json` and delete `~/.local/bin/review-gate`. Per-repository state under `.git/ai-review/` can go too; nothing else reads it. + The setup script only manages commands it originally installed. The upstream extras are removed separately: `gh extension remove gh-stack` for the `gh stack` extension, and for the `gh`/`gh-stack` agent skills, delete their directories as described in [`gh` Agent Skills](#gh-agent-skills). ## Contributing diff --git a/bin/code-review-loop b/bin/code-review-loop index 114ee49..9281f7f 100755 --- a/bin/code-review-loop +++ b/bin/code-review-loop @@ -166,7 +166,22 @@ fi # ---- temp directory with cleanup ----------------------------------------- setup_temp "code-review-loop" -trap cleanup_temp EXIT + +cleanup_run() { + cleanup_temp + review_gate_lock_release +} +trap cleanup_run EXIT + +# Tell bin/review-gate this loop is running, so an agent it spawns that decides +# to commit its own fixes cannot trip the gate from inside the loop. This +# survives an agent that sanitizes its environment, which the AI_REVIEW_GATE +# export in run_agent does not. +review_gate_lock_acquire + +# Agents spawned from here have no user to ask, so the gate degrades to +# warn-only for the whole run rather than deadlocking on a question. +export AI_REVIEW_HEADLESS=1 # ---- run logs ------------------------------------------------------------- # Outside TMPDIR_REVIEW, which the EXIT trap wipes, and outside the project, @@ -414,6 +429,13 @@ else final_status="MAX ITERATIONS REACHED - some issues remain" fi +# ---- review receipt ------------------------------------------------------ +# Recorded before the summary step, which only writes agent-review-summary.md +# and so leaves the index exactly as the developer will commit it. This is what +# lets bin/review-gate answer "has this been reviewed" for the retried commit. +if $is_clean; then receipt_verdict="clean"; else receipt_verdict="needs-review"; fi +write_review_receipt "$receipt_verdict" "$iteration" "$EDITOR_AGENT" "$REVIEWER_AGENT" + write_step "Final" "$EDITOR_AGENT: Writing improvement summary" # Clear any summary left by a previous run so the post-run existence check # below reflects this run, not a stale artifact diff --git a/bin/review-gate b/bin/review-gate new file mode 100755 index 0000000..824c732 --- /dev/null +++ b/bin/review-gate @@ -0,0 +1,883 @@ +#!/usr/bin/env bash +# source: ai-coding-setup +# --------------------------------------------------------------------------- +# review-gate: a pre-tool hook that stops a coding agent from committing +# unreviewed code. +# +# Wired as a PreToolUse hook on the shell tool. It reads the harness's hook +# JSON on stdin, works out whether the command about to run is a `git commit`, +# and either stays silent (allow) or denies with a reason the agent can act on. +# +# Three tiers, cheapest first: +# 1. Deterministic allow: bypass set, nothing staged, a matching review +# receipt, a history rewrite in progress, or an index tree identical to +# one the branch just had. +# 2. Deterministic deny, carrying the staged diff, per-category line counts, +# receipt status, a single-use nonce, and the classification rubric. +# 3. The agent judges the change trivial or not, and either re-runs the +# commit with the nonce or asks the user. +# +# The gate never runs code-review-loop itself: it is sub-second git plumbing +# and string parsing, nothing more. +# +# Usage: review-gate [--format=claude|codex|copilot|antigravity|kimi] +# --------------------------------------------------------------------------- +# +# Deliberately not `set -e`. A hook that dies mid-way is worse than one that +# lets a commit through: Copilot CLI treats any non-zero exit as a deny, so an +# unhandled error there would block every shell call in the session. Every +# fallible step below is guarded and falls back to allow. +set -o pipefail + +FORMAT="claude" + +usage() { + cat <<'USAGE' +Usage: review-gate [--format=FORMAT] + +A PreToolUse hook that denies an agent's `git commit` when the staged change +has not been reviewed. Reads the harness hook JSON on stdin. + +Options: + --format=FORMAT Output shape: claude (default), codex, copilot, + antigravity, or kimi + -h, --help Show this help message + +Configuration: + REVIEW_GATE=off|warn|block Environment, or ~/.ai-coding-setup.conf. + Default: warn + AI_REVIEW_GATE=off Bypass this one invocation. Read off the + command string, so it must sit on the same + command as the commit. In PowerShell, write it + as: $env:AI_REVIEW_GATE = "off"; git commit ... + AI_REVIEW_HEADLESS=1 Degrade to warn-only (no user to ask) +USAGE + exit 0 +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --format=*) FORMAT="${1#--format=}"; shift ;; + --format) FORMAT="${2:-claude}"; shift 2 ;; + -h|--help) usage ;; + *) echo "review-gate: unknown option: $1" >&2; exit 0 ;; + esac +done + +case "$FORMAT" in + claude|codex|copilot|antigravity|kimi) ;; + *) echo "review-gate: unknown format '$FORMAT', using claude" >&2; FORMAT="claude" ;; +esac + +# ---- fast path ------------------------------------------------------------ +# This hook fires on every shell call the agent makes, which is overwhelmingly +# ls, cat, and test runs. One substring test before any parsing, subshell, or +# git call keeps the common case to a read and a compare. +INPUT=$(cat) +case "$INPUT" in + *commit*) ;; + *) exit 0 ;; +esac + +# ---- configuration -------------------------------------------------------- + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +_ENV_REVIEW_GATE="${REVIEW_GATE:-}" + +# The shared library owns load_config, so the gate reads the same config file +# by the same rules as the review loops. It is optional: the gate must still +# work when only bin/review-gate was installed. +# shellcheck source=lib/lib-review-loop +[[ -r "$SCRIPT_DIR/../lib/lib-review-loop" ]] && source "$SCRIPT_DIR/../lib/lib-review-loop" +if declare -F load_config >/dev/null 2>&1; then + load_config +fi +# The environment wins over the config file. +[[ -n "$_ENV_REVIEW_GATE" ]] && REVIEW_GATE="$_ENV_REVIEW_GATE" + +MODE="${REVIEW_GATE:-warn}" +case "$MODE" in + off|warn|block) ;; + *) MODE="warn" ;; +esac +[[ "$MODE" == "off" ]] && exit 0 + +# The environment form of the per-invocation bypass, checked before any parsing +# so it also covers a command the parser would otherwise refuse to guess at. +# This is how the recursion guard in lib/lib-review-loop works. +[[ "${AI_REVIEW_GATE:-}" == "off" ]] && exit 0 + +# Headless detection is explicit, never a TTY test. Every harness spawns hooks +# with stdin, stdout, and stderr on pipes, so `[ -t 0 ]` is false in an +# interactive session too; keying off it would silently turn the gate into a +# no-op everywhere, which looks installed and does nothing. +# +# CI is the only harness marker trusted here. Per-CLI non-interactive markers +# (codex exec, claude -p, and friends) are deliberately absent until each one +# is confirmed against that CLI's own docs; AI_REVIEW_HEADLESS is the explicit +# opt-in that bin/code-review-loop and CI wrappers set instead. +if [[ "$MODE" == "block" ]]; then + if [[ "${AI_REVIEW_HEADLESS:-}" == "1" || -n "${CI:-}" ]]; then + MODE="warn" + fi +fi + +# ---- output --------------------------------------------------------------- + +# An allow is silence plus exit 0, never an explicit "allow" decision: an +# affirmative allow from a PreToolUse hook skips the user's own permission +# rules, which would auto-approve every shell command the agent runs. +allow() { exit 0; } + +deny() { + local reason="$1" + case "$FORMAT" in + claude|codex) + jq -nc --arg r "$reason" '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"deny",permissionDecisionReason:$r}}' + ;; + copilot|antigravity) + # Flat, not nested under hookSpecificOutput. Copilot requires the + # reason field on a deny; Antigravity accepts "force_ask" here too. + jq -nc --arg r "$reason" '{permissionDecision:"deny",permissionDecisionReason:$r}' + ;; + kimi) + # Exit-code driven: 2 is the blocking code, reason on stderr. + printf '%s\n' "$reason" >&2 + exit 2 + ;; + esac + exit 0 +} + +warn() { + local reason="$1" + case "$FORMAT" in + claude|codex) + # Context without a decision, so the normal permission flow still + # applies. Other harnesses get stderr only: an unrecognized field + # there risks a parse error, and Copilot fails closed on one. + jq -nc --arg r "$reason" '{hookSpecificOutput:{hookEventName:"PreToolUse",additionalContext:$r}}' 2>/dev/null + ;; + esac + printf '%s\n' "$reason" >&2 + exit 0 +} + +# Route a gate miss through the configured mode. +miss() { + if [[ "$MODE" == "block" ]]; then + deny "$1" + fi + warn "$1" +} + +command -v jq >/dev/null 2>&1 || allow + +# ---- hook input ----------------------------------------------------------- + +# Field names differ per harness, so try the documented shapes in turn rather +# than binding to one. No command field means nothing to gate. +COMMAND=$(printf '%s' "$INPUT" | jq -r ' + (.tool_input.command // .tool_input.cmd // .tool_input.script + // .params.command // .arguments.command // .command // "") + | if type == "array" then join(" ") else tostring end' 2>/dev/null) || allow +[[ -n "$COMMAND" && "$COMMAND" != "null" ]] || allow +case "$COMMAND" in + *commit*) ;; + *) allow ;; +esac + +# Which tool the harness is about to run the command with. On Windows a +# harness may expose a PowerShell tool alongside the Bash one, and the two take +# incompatible syntax for the very thing the gate asks the agent to type: a +# bash `VAR=value command` prefix is a hard parse error in PowerShell. Absent +# or unrecognized means POSIX, which is every other platform. +TOOL_NAME=$(printf '%s' "$INPUT" | jq -r '(.tool_name // .toolName // .tool // "")' 2>/dev/null) || TOOL_NAME="" +SHELL_KIND="posix" +case "$TOOL_NAME" in + PowerShell|powershell|Powershell|pwsh|PowerShellTool) SHELL_KIND="powershell" ;; +esac + +# Render the one-shot bypass in the shell the agent will actually type it in. +# The single quotes are deliberate: `$env:` is literal PowerShell to be printed, +# not something for this shell to expand. +# shellcheck disable=SC2016 +with_gate() { # with_gate + if [[ "$SHELL_KIND" == "powershell" ]]; then + printf '$env:AI_REVIEW_GATE = "%s"; %s' "$1" "$2" + else + printf 'AI_REVIEW_GATE=%s %s' "$1" "$2" + fi +} + +HOOK_CWD=$(printf '%s' "$INPUT" | jq -r '(.cwd // .workspace // .project_dir // "")' 2>/dev/null) || HOOK_CWD="" +[[ "$HOOK_CWD" == "null" ]] && HOOK_CWD="" +# A Windows harness reports `C:\Users\...`, which Git Bash cannot stat. The +# forward-slash form it can, so try that before falling back to the hook +# process's own directory. +if [[ -n "$HOOK_CWD" && ! -d "$HOOK_CWD" && "$HOOK_CWD" == [A-Za-z]:\\* ]]; then + HOOK_CWD="${HOOK_CWD//\\//}" +fi +[[ -n "$HOOK_CWD" && -d "$HOOK_CWD" ]] || HOOK_CWD="$PWD" + +# ---- command parsing ------------------------------------------------------ +# +# Quote-aware, because the most ordinary input there is -- a commit message -- +# breaks a naive split: `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. + +SEP=$'\001' +TOKENS=() +PARSE_FAILED=0 +HAS_SUBST=0 + +tokenize() { + local s="$1" + local n=${#s} i=0 c q="" tok="" have=0 + TOKENS=(); PARSE_FAILED=0; HAS_SUBST=0 + + while (( i < n )); do + c=${s:i:1} + if [[ -n "$q" ]]; then + if [[ "$q" == "'" ]]; then + if [[ "$c" == "'" ]]; then q=""; else tok+="$c"; fi + else + if [[ "$c" == "\\" && "$SHELL_KIND" != "powershell" ]]; then + (( i++ )); tok+="${s:i:1}" + elif [[ "$c" == '"' ]]; then + q="" + else + tok+="$c" + fi + fi + (( i++ )); continue + fi + case "$c" in + "'") q="'"; have=1 ;; + '"') q='"'; have=1 ;; + # In PowerShell a backslash is an ordinary path character, not an + # escape; its escape character is the backtick. Consuming the next + # character here would fold `C:\Program Files\Git\bin\git.exe` into + # an unrecognizable word and let the commit through. + \\) if [[ "$SHELL_KIND" == "powershell" ]]; then + tok+="$c" + else + (( i++ )); tok+="${s:i:1}" + fi + have=1 ;; + ' '|$'\t') + if (( have )); then TOKENS+=("$tok"); tok=""; have=0; fi ;; + ';'|$'\n'|'&'|'|'|'('|')'|'{'|'}') + if (( have )); then TOKENS+=("$tok"); tok=""; have=0; fi + TOKENS+=("$SEP") ;; + '$') + if [[ "${s:i+1:1}" == "(" ]]; then + HAS_SUBST=1; tok+="${c}("; (( i++ )) + else + tok+="$c" + fi + have=1 ;; + '`') HAS_SUBST=1; tok+="$c"; have=1 ;; + *) tok+="$c"; have=1 ;; + esac + (( i++ )) + done + + [[ -n "$q" ]] && PARSE_FAILED=1 + (( have )) && TOKENS+=("$tok") + return 0 +} + +COMMIT_FOUND=0 +GATE_INLINE="" +PS_GATE_ENV="" +GIT_C_DIR="" +OUTSIDE_INDEX="" + +# Look for a PowerShell environment assignment in one segment. +# +# PowerShell has no `VAR=value command` prefix form, so the bypass and the +# nonce arrive as their own statement: `$env:AI_REVIEW_GATE = "off"; git +# commit`. That is a separate segment from the commit, which is why this is +# scanned across segments rather than inside analyze_segment. +# +# Both spacings collapse to one of two token shapes, because the tokenizer +# strips quotes: `$env:AI_REVIEW_GATE=off` as a single token when the `=` has +# no space around it, or three tokens when it does. Matched case-insensitively +# via tr rather than ${var,,}, which macOS bash 3.2 does not have. +# +# As in with_gate, the single-quoted `$env:` is literal PowerShell to match +# against, not a shell expansion. +# shellcheck disable=SC2016 +scan_ps_env() { + local -a a=("$@") + local n=${#a[@]} i=0 t lower + while (( i < n )); do + t=${a[i]} + case "$t" in + '$'*) ;; + *) (( i++ )); continue ;; + esac + lower=$(printf '%s' "${t%%=*}" | tr '[:upper:]' '[:lower:]') + if [[ "$lower" == '$env:ai_review_gate' ]]; then + if [[ "$t" == *=* ]]; then + PS_GATE_ENV="${t#*=}" + elif [[ "${a[i+1]:-}" == "=" ]]; then + PS_GATE_ENV="${a[i+2]:-}" + (( i += 2 )) + fi + fi + (( i++ )) + done + return 0 +} + +# Inspect one command segment; sets the globals above when it is a git commit. +# Reduce a command word to its bare, lowercased name: strip any directory in +# either separator, and the .exe a Windows invocation carries. Both forms reach +# the gate for real, `C:\Program Files\Git\bin\git.exe` as readily as +# `/usr/bin/git`, and matching only the bare word lets either past. +strip_exe() { + local w="${1##*/}" + w="${w##*\\}" + w=$(printf '%s' "$w" | tr '[:upper:]' '[:lower:]') + printf '%s' "${w%.exe}" +} + +is_git_word() { + [[ "$(strip_exe "$1")" == "git" ]] +} + +# Analyze a command that arrived embedded in a single argument, as env's +# --split-string hands one over. Tokenized here rather than skipped, so the +# staging flags inside it are read like any other command's. TOKENS is saved +# across the call because analyze() is iterating a snapshot of it. +# On the `${arr[@]+"${arr[@]}"}` idiom used for every array expansion below: +# the outer expansion is unquoted, but its alternate value is not, so elements +# keep their spaces and a commit message stays one token. It reads like an +# unquoted expansion and is not one. The plain `"${arr[@]}"` form cannot be +# used here: this file targets macOS bash 3.2, where expanding an empty array +# that way is an unbound-variable error. +analyze_split() { + local s="$1"; shift + local -a rest=("$@") + (( ${#rest[@]} )) || [[ -n "$s" ]] || return 0 + local -a saved=(${TOKENS[@]+"${TOKENS[@]}"}) sub=() seg=() + local saved_failed=$PARSE_FAILED saved_subst=$HAS_SUBST t + tokenize "$s" + sub=(${TOKENS[@]+"${TOKENS[@]}"}) + TOKENS=(${saved[@]+"${saved[@]}"}) + PARSE_FAILED=$saved_failed + HAS_SUBST=$saved_subst + # env appends whatever operands follow the split string to the command it + # builds, so they are part of the same command: `env -S "git" commit -m x` + # commits, and `env -S "git commit" -a` stages the working tree. Dropping + # them loses the commit or, worse, the staging flag that decides whether a + # receipt can vouch for what lands. + sub=(${sub[@]+"${sub[@]}"} ${rest[@]+"${rest[@]}"}) + for t in ${sub[@]+"${sub[@]}"} "$SEP"; do + if [[ "$t" == "$SEP" ]]; then + (( ${#seg[@]} )) && analyze_segment ${seg[@]+"${seg[@]}"} + seg=() + continue + fi + seg+=("$t") + done + return 0 +} + +analyze_segment() { + local -a a=("$@") + local n=${#a[@]} i=0 t seg_gate="" c_dir="" + + # Leading environment assignments and the wrappers that take a command of + # their own. A PreToolUse hook is spawned before the shell runs the command, + # so an inline AI_REVIEW_GATE= never reaches this process's + # environment; it has to be read back off the command string here. + # + # The wrapper list is deliberately short: every entry runs the rest of the + # segment as a command, so skipping one lands on the real command word + # rather than on an argument. `env -i git commit` and `sudo git commit` are + # ordinary commits, and a gate that waves them through enforces nothing. + while (( i < n )); do + t=${a[i]} + case "$(strip_exe "$t")" in + env|command|nice|nohup|sudo|doas|stdbuf) + (( i++ )) + # The wrapper's own options, so `env -i` and `env -u NAME` do + # not read as the command word. + while (( i < n )); do + case "${a[i]}" in + # `env -S "git commit -m x"` carries a whole command in + # one argument. Skipping the value would step over the + # commit entirely, so it is parsed as its own segment. + -S|--split-string) + analyze_split "${a[i+1]:-}" "${a[@]:i+2}" + return 0 ;; + --split-string=*) + analyze_split "${a[i]#--split-string=}" "${a[@]:i+1}" + return 0 ;; + -S?*) + analyze_split "${a[i]#-S}" "${a[@]:i+1}" + return 0 ;; + -u|--unset|-C|--chdir) (( i += 2 )) ;; + -*) (( i++ )) ;; + *) break ;; + esac + done + continue ;; + esac + if [[ "$t" =~ ^[A-Za-z_][A-Za-z0-9_]*= ]]; then + [[ "$t" == AI_REVIEW_GATE=* ]] && seg_gate="${t#AI_REVIEW_GATE=}" + (( i++ )); continue + fi + break + done + + (( i < n )) || return 0 + is_git_word "${a[i]}" || return 0 + (( i++ )) + + # git's own options, ahead of the subcommand. + while (( i < n )); do + t=${a[i]} + case "$t" in + -C) c_dir="${a[i+1]:-}"; (( i += 2 )) ;; + -C*) c_dir="${t#-C}"; (( i++ )) ;; + -c|--git-dir|--work-tree|--namespace|--exec-path|--super-prefix|--config-env) + (( i += 2 )) ;; + -*) (( i++ )) ;; + *) break ;; + esac + done + + (( i < n )) || return 0 + # `git commit-tree` and a `git log --format` mentioning the word are not + # commits; only the exact subcommand counts. + [[ "${a[i]}" == "commit" ]] || return 0 + (( i++ )) + + COMMIT_FOUND=1 + [[ -n "$seg_gate" ]] && GATE_INLINE="$seg_gate" + [[ -n "$c_dir" ]] && GIT_C_DIR="$c_dir" + + local after_ddash=0 chars k ch rest + while (( i < n )); do + t=${a[i]} + if (( after_ddash )); then + OUTSIDE_INDEX="${OUTSIDE_INDEX:-a pathspec}" + (( i++ )); continue + fi + case "$t" in + --) after_ddash=1 ;; + # Value-taking long options: skip the value so a message or a + # template path is never read as a pathspec. + --message|--file|--reuse-message|--reedit-message|--author|--date|--cleanup|--template|--fixup|--squash|--trailer) + (( i++ )) ;; + --all) OUTSIDE_INDEX="--all" ;; + --include) OUTSIDE_INDEX="--include" ;; + --only) OUTSIDE_INDEX="--only" ;; + --patch|--interactive) OUTSIDE_INDEX="--patch" ;; + --pathspec-from-file*) OUTSIDE_INDEX="--pathspec-from-file" ;; + # --amend outside a rebase leaves the index unchanged, so a + # matching receipt stays valid and it needs no special case. + --*) ;; + [0-9]'>'*|'>'*|'<'*) + # A redirection is not an argument to the commit. A bare + # operator also swallows the target that follows it. + case "$t" in + [0-9]'>'|'>'|'>>'|'<') (( i++ )) ;; + esac + ;; + -?*) + # Short options cluster: -am "msg" is -a plus -m. + chars="${t#-}"; k=0 + while (( k < ${#chars} )); do + ch="${chars:k:1}" + case "$ch" in + a|o|i|p) OUTSIDE_INDEX="-$ch" ;; + m|F|C|c|t) + rest="${chars:k+1}" + [[ -z "$rest" ]] && (( i++ )) + k=${#chars}; continue ;; + S|u) + # Optional attached value; never consumes the next token. + k=${#chars}; continue ;; + esac + (( k++ )) + done + ;; + -) ;; + *) OUTSIDE_INDEX="${OUTSIDE_INDEX:-a pathspec}" ;; + esac + (( i++ )) + done + return 0 +} + +analyze() { + tokenize "$1" + local -a seg=() + local t + for t in ${TOKENS[@]+"${TOKENS[@]}"} "$SEP"; do + if [[ "$t" == "$SEP" ]]; then + if (( ${#seg[@]} )); then + # Only an assignment ahead of the commit counts, which the + # COMMIT_FOUND guard gets for free: `git commit; $env:X = "off"` + # sets the variable after the commit has already run. + (( COMMIT_FOUND )) || scan_ps_env ${seg[@]+"${seg[@]}"} + analyze_segment ${seg[@]+"${seg[@]}"} + fi + seg=() + continue + fi + seg+=("$t") + done + return 0 +} + +analyze "$COMMAND" + +# A bash-style inline prefix wins if both somehow appear; otherwise the +# PowerShell statement form stands in for it. +[[ -z "$GATE_INLINE" ]] && GATE_INLINE="$PS_GATE_ENV" + +# Fail closed on a command that mentions git and commit but whose shape could +# not be resolved: an unbalanced quote, or a substitution wrapping the git +# word. A spurious prompt is recoverable; a silent miss is not. +if (( ! COMMIT_FOUND )); then + if (( PARSE_FAILED || HAS_SUBST )) \ + && [[ "$COMMAND" =~ (^|[^[:alnum:]_-])git([^[:alnum:]_-]|$) ]] \ + && [[ "$COMMAND" =~ (^|[^[:alnum:]_-])commit ]]; then + miss "Code review gate: this command mentions \`git\` and \`commit\` but could not be parsed (unbalanced quotes or a command substitution around the git word), so the gate cannot tell what it will commit. + +Rewrite it as a plain \`git commit ...\` so the gate can inspect it, or set AI_REVIEW_GATE=off on the command if it genuinely does not commit." + fi + allow +fi + +# ---- repository state ----------------------------------------------------- + +# Absolute covers both shapes the gate can be handed: a POSIX path, and the +# `C:/...` or `C:\...` a Windows harness reports even when the gate itself runs +# under Git Bash. +is_absolute_path() { + case "$1" in + /*|[A-Za-z]:/*|[A-Za-z]:\\*) return 0 ;; + *) return 1 ;; + esac +} + +WORKDIR="$HOOK_CWD" +if [[ -n "$GIT_C_DIR" ]]; then + if is_absolute_path "$GIT_C_DIR"; then WORKDIR="$GIT_C_DIR"; else WORKDIR="$HOOK_CWD/$GIT_C_DIR"; fi +fi +[[ -d "$WORKDIR" ]] || WORKDIR="$HOOK_CWD" + +g() { git -C "$WORKDIR" "$@"; } + +# --absolute-git-dir rather than --git-dir plus a prefix: --git-dir returns a +# path relative to the cwd in the ordinary case but an absolute one in a linked +# worktree, and on Git for Windows that absolute path is `C:/...`, which no +# leading-slash test recognizes. Prefixing it there would build `C:/repo/C:/...` +# and silently scatter state into a directory that does not exist. +GIT_DIR_PATH=$(g rev-parse --absolute-git-dir 2>/dev/null) || GIT_DIR_PATH="" +if [[ -z "$GIT_DIR_PATH" ]]; then + # git < 2.13 has no --absolute-git-dir. + GIT_DIR_PATH=$(g rev-parse --git-dir 2>/dev/null) || allow + is_absolute_path "$GIT_DIR_PATH" || GIT_DIR_PATH="$WORKDIR/$GIT_DIR_PATH" +fi +STATE_DIR="$GIT_DIR_PATH/ai-review" + +# Tier 1.1: bypass, read off the command string. A PreToolUse hook is spawned +# before the shell runs the command, so an inline assignment never reaches this +# process's environment; the environment form is handled earlier. +[[ "$GATE_INLINE" == "off" ]] && allow + +# Tier 1.4: a history rewrite is in progress. These markers cover rebase, +# interactive rebase, cherry-pick, revert, and merge, including every +# --continue step and the amend a rebase stops for. A twelve-commit rebase +# must not stop to ask twelve times. +for _marker in rebase-merge rebase-apply CHERRY_PICK_HEAD MERGE_HEAD REVERT_HEAD BISECT_LOG; do + [[ -e "$GIT_DIR_PATH/$_marker" ]] && allow +done + +# Recursion guard: code-review-loop holds a lock for the length of its run, so +# an agent it spawns that decides to commit cannot trip the gate from inside +# the loop. The lock is validated, not merely observed: an EXIT trap does not +# run on SIGKILL or a reboot, and a stale file would disable the gate forever +# in exactly the situation where nobody thinks to look. +LOCK_MAX_AGE=${REVIEW_GATE_LOCK_MAX_AGE:-21600} +if [[ -f "$STATE_DIR/running" ]]; then + _lock=$(cat "$STATE_DIR/running" 2>/dev/null) || _lock="" + _lock_pid=${_lock%% *} + _lock_ts=${_lock##* } + _now=$(date +%s) + if [[ "$_lock_pid" =~ ^[0-9]+$ ]] && [[ "$_lock_ts" =~ ^[0-9]+$ ]] \ + && kill -0 "$_lock_pid" 2>/dev/null \ + && (( _now - _lock_ts < LOCK_MAX_AGE )); then + allow + fi + # Dead PID, or past the age bound that covers PID reuse across a reboot. + rm -f "$STATE_DIR/running" 2>/dev/null +fi + +# git write-tree fails on an unresolved merge conflict, which means a merge is +# in progress anyway. +INDEX_TREE=$(g write-tree 2>/dev/null) || allow +[[ -n "$INDEX_TREE" ]] || allow + +# A fresh repository has no HEAD to resolve; `git rev-parse HEAD` exits 128 +# there, so both the gate and the receipt writer store the literal "unborn". +HEAD_SHA=$(g rev-parse --verify -q HEAD 2>/dev/null) || HEAD_SHA="" +[[ -n "$HEAD_SHA" ]] || HEAD_SHA="unborn" + +EMPTY_TREE="4b825dc642cb6eb9a060e54bf8d69288fbee4904" + +# Tier 1: the nonce. Single-use and bound to the index tree it was issued for, +# so an agent that stages more work cannot reuse it, and the agent's own +# re-run after a trivial-change judgment does not trip the same deny forever. +if [[ -n "$GATE_INLINE" && -f "$STATE_DIR/nonce" ]]; then + _nonce_line=$(cat "$STATE_DIR/nonce" 2>/dev/null) || _nonce_line="" + _nonce_value=${_nonce_line%% *} + _nonce_tree=${_nonce_line##* } + if [[ -n "$_nonce_value" && "$GATE_INLINE" == "$_nonce_value" && "$_nonce_tree" == "$INDEX_TREE" ]]; then + rm -f "$STATE_DIR/nonce" 2>/dev/null + allow + fi +fi + +# Everything below describes the index, so it is only meaningful when the +# commit will commit the index and nothing else. Checked before the +# index-derived rules: otherwise `git commit -a` with a clean index would sail +# through the empty-tree check and commit unreviewed working-tree changes. +if [[ -z "$OUTSIDE_INDEX" ]]; then + if [[ "$HEAD_SHA" == "unborn" ]]; then + BASE_TREE="$EMPTY_TREE" + else + BASE_TREE=$(g rev-parse -q --verify "HEAD^{tree}" 2>/dev/null) || BASE_TREE="" + fi + + # Tier 1.2: nothing staged. Also covers an --amend that only rewords. + [[ -n "$BASE_TREE" && "$INDEX_TREE" == "$BASE_TREE" ]] && allow +fi + +# ---- review receipt ------------------------------------------------------- + +RECEIPTS="$STATE_DIR/receipts.json" +RECEIPT_STATUS="none: no review has been recorded for this repository" + +read_receipts() { + [[ -f "$RECEIPTS" ]] || { printf '[]'; return 0; } + jq -c 'if type == "array" then . else [] end' "$RECEIPTS" 2>/dev/null || printf '[]' +} + +if [[ -z "$OUTSIDE_INDEX" ]]; then + RECEIPT_JSON=$(read_receipts) + if [[ "$RECEIPT_JSON" != "[]" ]]; then + if printf '%s' "$RECEIPT_JSON" | jq -e --arg t "$INDEX_TREE" --arg h "$HEAD_SHA" \ + 'any(.[]; .index_tree == $t and .head == $h and .verdict == "clean")' >/dev/null 2>&1; then + # Tier 1.3: a clean review of exactly this index on exactly this base. + allow + fi + # Not a match. Say which field diverged so the agent can explain it. + RECEIPT_STATUS=$(printf '%s' "$RECEIPT_JSON" | jq -r --arg t "$INDEX_TREE" --arg h "$HEAD_SHA" ' + if any(.[]; .index_tree == $t and .head == $h) then + "stale: a review covered this exact index, but its verdict was not clean" + elif any(.[]; .index_tree == $t) then + "stale: a clean review covered this index, but HEAD has moved since (rebased or committed underneath it)" + elif any(.[]; .head == $h) then + "stale: a review ran on this base, but the index has changed since (files staged or unstaged after the review)" + else + "none: no recorded review matches this index or this base" + end' 2>/dev/null) || RECEIPT_STATUS="none: the receipts file could not be read" + fi + + # Tier 1.5: tree identity. A pure history rewrite produces an identical + # final tree by construction, which catches rewrites whose in-progress + # markers have already been cleaned up. Deliberately two entries and not + # the whole reflog: scanning every entry would exempt any tree that ever + # existed on this branch, including states that were dirty, broken, or + # never reviewed, and would turn two lookups into an unbounded walk. + for _ref in ORIG_HEAD 'HEAD@{1}'; do + _tree=$(g rev-parse -q --verify "$_ref^{tree}" 2>/dev/null) || continue + [[ "$_tree" == "$INDEX_TREE" ]] && allow + done +else + RECEIPT_STATUS="not applicable: this command commits content that was not in the index when the gate ran, so no receipt can describe it" +fi + +# ---- tier 2: deny with context -------------------------------------------- + +if [[ "$HEAD_SHA" == "unborn" ]]; then DIFF_BASE="$EMPTY_TREE"; else DIFF_BASE="HEAD"; fi + +STAT=$(g diff --cached --stat "$DIFF_BASE" 2>/dev/null) || STAT="" +NUMSTAT=$(g diff --cached --numstat --no-renames "$DIFF_BASE" 2>/dev/null) || NUMSTAT="" + +WORKTREE_STAT="" +if [[ -n "$OUTSIDE_INDEX" ]]; then + WORKTREE_STAT=$(g diff --stat 2>/dev/null) || WORKTREE_STAT="" +fi + +PATHS=$(printf '%s\n' "$NUMSTAT" | awk -F'\t' 'NF >= 3 { print $3 }') + +# Generated files are marked in .gitattributes, so the gate does not have to +# carry a per-project list. One batched call, not one per path. Two sed +# expressions rather than one alternation: BSD sed has no \| in a basic regex, +# and `linguist-generated` reports "set" while `linguist-generated=true` +# reports "true". +GENERATED_LIST="" +if [[ -n "$PATHS" ]]; then + GENERATED_LIST=$(printf '%s\n' "$PATHS" \ + | g check-attr --stdin linguist-generated 2>/dev/null \ + | sed -n -e 's/: linguist-generated: set$//p' \ + -e 's/: linguist-generated: true$//p') || GENERATED_LIST="" +fi + +classify_path() { + local p="$1" + if [[ -n "$GENERATED_LIST" ]] && printf '%s\n' "$GENERATED_LIST" | grep -qxF "$p"; then + printf 'generated'; return 0 + fi + case "$p" in + node_modules/*|*/node_modules/*|vendor/*|*/vendor/*|dist/*|*/dist/*|build/*|*/build/*|\ + *.min.js|*.min.css|*.lock|package-lock.json|pnpm-lock.yaml|\ + *.pb.go|*_pb2.py|*_pb2.pyi|*.generated.*|*.g.dart|*.snap) + printf 'generated'; return 0 ;; + esac + case "$p" in + test/*|tests/*|*/test/*|*/tests/*|spec/*|*/spec/*|\ + *_test.*|*.test.*|*_spec.*|*.spec.*|test_*.py|*.bats) + printf 'tests'; return 0 ;; + esac + case "$p" in + docs/*|*/docs/*|*.md|*.mdx|*.rst|*.txt|*.adoc|LICENSE|LICENSE.*|CHANGELOG*) + printf 'docs'; return 0 ;; + esac + printf 'code' +} + +# Four counter pairs rather than an associative array: macOS still ships +# bash 3.2, which has none. +_add_code=0 _del_code=0 _add_tests=0 _del_tests=0 +_add_docs=0 _del_docs=0 _add_generated=0 _del_generated=0 +PATH_LINES="" +FILE_COUNT=0 + +while IFS=$'\t' read -r _a _d _p; do + [[ -n "$_p" ]] || continue + (( FILE_COUNT++ )) + _cat=$(classify_path "$_p") + # A binary file reports "-" for both counts. + [[ "$_a" =~ ^[0-9]+$ ]] || _a=0 + [[ "$_d" =~ ^[0-9]+$ ]] || _d=0 + eval "_add_$_cat=\$(( _add_$_cat + _a ))" + eval "_del_$_cat=\$(( _del_$_cat + _d ))" + if (( FILE_COUNT <= 40 )); then + PATH_LINES+=$(printf ' %-56s %s' "$_p" "$_cat")$'\n' + fi +done <<< "$NUMSTAT" + +(( FILE_COUNT > 40 )) && PATH_LINES+=" ... and $(( FILE_COUNT - 40 )) more"$'\n' + +COUNT_LINES="" +for _k in code tests docs generated; do + eval "_a=\$_add_$_k; _d=\$_del_$_k" + (( _a || _d )) || continue + COUNT_LINES+=$(printf ' %-10s +%s -%s' "$_k" "$_a" "$_d")$'\n' +done +[[ -n "$COUNT_LINES" ]] || COUNT_LINES=" (no line changes in the index)"$'\n' + +# The nonce lets the agent act on its own trivial-change judgment without +# handing it a permanent bypass. Written with the tree it was issued for, so a +# later `git add` invalidates it. Write-then-rename, so a killed hook cannot +# leave a half-written file behind. Not issued in warn mode, where nothing was +# blocked in the first place. +NONCE="" +if [[ "$MODE" == "block" ]]; then + if mkdir -p "$STATE_DIR" 2>/dev/null; then + NONCE=$(od -An -tx1 -N12 /dev/urandom 2>/dev/null | tr -d ' \n') || NONCE="" + [[ -n "$NONCE" ]] || NONCE="$(date +%s)$$" + if _tmp=$(mktemp "$STATE_DIR/nonce.XXXXXX" 2>/dev/null); then + if printf '%s\t%s\n' "$NONCE" "$INDEX_TREE" > "$_tmp"; then + mv -f "$_tmp" "$STATE_DIR/nonce" || { rm -f "$_tmp"; NONCE=""; } + else + rm -f "$_tmp"; NONCE="" + fi + else + NONCE="" + fi + fi +fi + +SHORT_COMMAND="$COMMAND" +(( ${#SHORT_COMMAND} > 400 )) && SHORT_COMMAND="${SHORT_COMMAND:0:400}..." + +REASON="Code review gate: the staged change has no clean review on record. + +Command: $SHORT_COMMAND" + +if [[ -n "$OUTSIDE_INDEX" ]]; then + REASON+=" + +This command commits content beyond the index ($OUTSIDE_INDEX), so what lands +is not what the gate can inspect, and no review receipt can vouch for it. +Unstaged changes in the working tree: +${WORKTREE_STAT:- (none)}" +fi + +REASON+=" + +Staged diff: +${STAT:- (empty)} + +Lines changed by category: +$COUNT_LINES +Paths: +${PATH_LINES:- (none)} +Review receipt: $RECEIPT_STATUS + +Classify the change, then take exactly one of these actions. + +TRIVIAL, commit without review: + - documentation, comments, and formatting with no behavior change + - generated artifacts that mirror an already reviewed source change + - pure moves, renames, and reindentation + - reverting to a previously reviewed state + - version bumps and changelog entries" + +if [[ -n "$NONCE" ]]; then + REASON+=" + + Re-run the commit with the single-use nonce, and say out loud why you judged + it trivial so the user can object: + $(with_gate "$NONCE" "$SHORT_COMMAND") + The nonce works once, only for the index as it stands now. Staging more work + invalidates it." +fi + +REASON+=" + +NON-TRIVIAL, review first: + - any change to control flow, error handling, or a boundary condition + - anything touching auth, permissions, secrets, input parsing, or a + filesystem or network path, regardless of size + - new files containing logic + - dependency changes (defer to the dependency-review skill) + - anything you are unsure about; unsure is a non-trivial answer + + Ask the user with AskUserQuestion, offering: + 1. Run \`code-review-loop\` now, then commit if the review comes back clean. + A clean run records a receipt, so the retried commit passes with no + further prompting. + 2. Skip the review and commit anyway$([[ -n "$NONCE" ]] && printf ' (uses the nonce above)')." + +REASON+=" + 3. Something else, such as looking at the diff first or staging less. + +Escape hatches: $(with_gate off 'git commit ...') on a single command, or +REVIEW_GATE=off in ~/.ai-coding-setup.conf to turn the gate off entirely. +The bypass has to ride on the same command as the commit: the gate reads it off +the command string, so setting it in an earlier call does not carry over." + +miss "$REASON" diff --git a/lib/lib-review-loop b/lib/lib-review-loop index 37400c7..8fadec5 100644 --- a/lib/lib-review-loop +++ b/lib/lib-review-loop @@ -81,7 +81,8 @@ read_prompt_file() { # ---- config file loader --------------------------------------------------- -# Load agent defaults from ~/.ai-coding-setup.conf. +# Load defaults from ~/.ai-coding-setup.conf: the editor and reviewer agents, +# and REVIEW_GATE (off|warn|block) for bin/review-gate. # Only sets EDITOR_AGENT / REVIEWER_AGENT if not already overridden by CLI. # Uses safe line-by-line parsing; never sources the file directly. # shellcheck disable=SC2034 # variables are used by sourcing scripts @@ -104,6 +105,10 @@ load_config() { value="${line#REVIEWER_AGENT=}" REVIEWER_AGENT="${value//[\"\']/}" ;; + REVIEW_GATE=*) + value="${line#REVIEW_GATE=}" + REVIEW_GATE="${value//[\"\']/}" + ;; esac done < "$config_file" } @@ -237,6 +242,12 @@ agent_command() { # $3: allowed tools (only used by Claude; ignored by others) _dispatch_agent() { local agent="$1" prompt="$2" tools="${3:-}" + # Recursion guard for bin/review-gate. The loop itself never commits, but + # the editor agent it spawns is unconstrained and could decide to commit + # its own fixes; without this the gate would fire from inside the loop and + # offer to run the loop. Scoped to the call, so it never leaks into the + # developer's own shell. + local -x AI_REVIEW_GATE=off case "$agent" in claude) run_claude "$prompt" "$tools" ;; codex) run_codex "$prompt" ;; @@ -506,6 +517,114 @@ format_elapsed() { printf '%dm %ds' "$elapsed_min" "$elapsed_remainder" } +# ---- review gate state ---------------------------------------------------- +# +# bin/review-gate answers "has this been reviewed" from a receipt written here. +# All of it lives under $(git rev-parse --git-dir)/ai-review, so it is never +# committed, is per-worktree, survives branch switches, and resolves correctly +# in a linked worktree where .git is a file. + +# Number of receipts kept, newest first, so amend and reorder flows can still +# match an older review. +REVIEW_RECEIPT_KEEP=10 + +# Print the absolute git directory, or fail outside a repository. +# --absolute-git-dir rather than --git-dir plus a prefix: --git-dir returns a +# relative path in the ordinary case but an absolute one in a linked worktree, +# and on Git for Windows that absolute path is `C:/...`, which no leading-slash +# test recognizes. Prefixing it there would build `C:/repo/C:/...`. +review_gate_git_dir() { + local git_dir + git_dir=$(git rev-parse --absolute-git-dir 2>/dev/null) || git_dir="" + if [[ -z "$git_dir" ]]; then + # git < 2.13 has no --absolute-git-dir. + git_dir=$(git rev-parse --git-dir 2>/dev/null) || return 1 + case "$git_dir" in + /*|[A-Za-z]:/*|[A-Za-z]:\\*) ;; + *) git_dir="$PWD/$git_dir" ;; + esac + fi + printf '%s\n' "$git_dir" +} + +# Print the gate's state directory, creating it. +review_gate_state_dir() { + local git_dir + git_dir=$(review_gate_git_dir) || return 1 + mkdir -p "$git_dir/ai-review" 2>/dev/null || return 1 + printf '%s\n' "$git_dir/ai-review" +} + +# Print the current HEAD sha, or the literal "unborn" in a fresh repository. +# git rev-parse HEAD exits 128 there, which would otherwise break both the +# receipt writer and the gate on the very first commit. +review_gate_head() { + git rev-parse --verify -q HEAD 2>/dev/null || printf 'unborn\n' +} + +# Record that a review ran over the current index. +# Written on every completed run, clean or not: a needs-review verdict is +# useful evidence for the gate's deny reason. +# $1: verdict ("clean" or "needs-review") +# $2: cycles run +# $3: editor agent name +# $4: reviewer agent name +write_review_receipt() { + local verdict="$1" cycles="${2:-0}" editor="${3:-}" reviewer="${4:-}" + command -v jq >/dev/null 2>&1 || return 0 + [[ "$cycles" =~ ^[0-9]+$ ]] || cycles=0 + + local dir tree head existing tmp + dir=$(review_gate_state_dir) || return 0 + # Fails on an unresolved merge conflict; there is nothing to vouch for then. + tree=$(git write-tree 2>/dev/null) || return 0 + head=$(review_gate_head) + + existing='[]' + if [[ -f "$dir/receipts.json" ]]; then + existing=$(jq -c 'if type == "array" then . else [] end' "$dir/receipts.json" 2>/dev/null) \ + || existing='[]' + fi + + # Write-then-rename inside the same directory, so a killed loop or two + # concurrent worktree runs cannot leave a half-written file behind. + tmp=$(mktemp "$dir/receipts.XXXXXX" 2>/dev/null) || return 0 + if printf '%s' "$existing" | jq -c \ + --arg tree "$tree" --arg head "$head" --arg verdict "$verdict" \ + --argjson cycles "$cycles" --arg editor "$editor" --arg reviewer "$reviewer" \ + --arg ts "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" \ + --argjson keep "$REVIEW_RECEIPT_KEEP" \ + '[{index_tree: $tree, head: $head, verdict: $verdict, cycles: $cycles, + editor: $editor, reviewer: $reviewer, ts: $ts}] + . | .[0:$keep]' \ + > "$tmp" 2>/dev/null; then + mv -f "$tmp" "$dir/receipts.json" + else + rm -f "$tmp" + fi +} + +# Claim the loop's lock so the gate exempts commits made while it runs. +# Records the PID and start time; the gate validates both, since an EXIT trap +# does not run on SIGKILL and a stale lock would disable the gate silently. +review_gate_lock_acquire() { + local dir tmp + dir=$(review_gate_state_dir) || return 0 + tmp=$(mktemp "$dir/running.XXXXXX" 2>/dev/null) || return 0 + if printf '%s\t%s\n' "$$" "$(date +%s)" > "$tmp"; then + mv -f "$tmp" "$dir/running" || rm -f "$tmp" + else + rm -f "$tmp" + fi + return 0 +} + +review_gate_lock_release() { + local git_dir + git_dir=$(review_gate_git_dir) || return 0 + rm -f "$git_dir/ai-review/running" 2>/dev/null + return 0 +} + # ---- artifact cleanup ----------------------------------------------------- # Snapshot untracked files (for detecting artifacts created during an agent run) diff --git a/prompts/code-refinement.md b/prompts/code-refinement.md index fcdde7f..be081c4 100644 --- a/prompts/code-refinement.md +++ b/prompts/code-refinement.md @@ -9,3 +9,5 @@ Verify that the project's UI framework components and utility classes are used w Run the project's linting command and fix all reported errors and warnings. Discover the command from package scripts, a Makefile, CI config, or pre-commit config; if the project has no linter, note that and move on. Avoid using lint-suppression comments (e.g., eslint-disable, noqa, @ts-ignore) to make the lint pass unless absolutely necessary, and only with a clear justification in the code. Review tests and code coverage: check whether existing tests adequately cover the new or modified code, add tests for any gaps you find, and update any existing tests that must change to handle the new behavior correctly. When finished, ensure everything is ready for a high-quality code review. + +Do not stage, commit, or push. Leave every change in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. diff --git a/prompts/code-review-response.md b/prompts/code-review-response.md index e290d3b..94e46fb 100644 --- a/prompts/code-review-response.md +++ b/prompts/code-review-response.md @@ -20,6 +20,8 @@ You are a senior software engineer evaluating the code review feedback in `agent When in doubt about the size of a change, be conservative and respond instead of implementing. +Do not stage, commit, or push. Leave every fix in the working tree: the review loop stages what it needs on its own, and the commit is the developer's call. + ## Response Format Add your response directly below each finding in `agent-code-review.md`: diff --git a/setup b/setup index bd9bb4f..245a6c6 100755 --- a/setup +++ b/setup @@ -1116,6 +1116,88 @@ install_scripts() { echo -e " ${BOLD}Scripts:${NC} $installed installed, $updated updated, $current up to date, $skipped skipped" } +# ---- review gate hook ------------------------------------------------------ +# +# bin/review-gate is a PreToolUse hook that denies an agent's `git commit` when +# the staged change has no clean review on record. Opt-in, and wired only for +# Claude Code: the other harnesses each take a different config shape, so their +# wiring is documented in the README rather than guessed at here. + +configure_review_gate_hook() { + local gate="$HOME/.local/bin/review-gate" + local settings_file="$HOME/.claude/settings.json" + + # -x alone, deliberately: it follows the link, so a symlink to a real + # executable passes and a dangling one does not. Accepting -L too would + # wire a hook pointing at nothing, and a hook that cannot execute is worse + # than no hook. + [[ -x "$gate" ]] || return 0 + + echo "" + echo -e "${BOLD}── Code Review Gate ──${NC}" + + mkdir -p "$HOME/.claude" + [[ -f "$settings_file" ]] || echo '{}' > "$settings_file" + + if ! jq empty "$settings_file" 2>/dev/null; then + print_warning "$settings_file is not valid JSON, skipping" + return + fi + + # Idempotent: any existing review-gate hook, wherever it sits in the array, + # means this is already wired. Earlier versions wired the matcher as "Bash" + # alone, which on Windows misses every commit the agent runs through the + # separate PowerShell tool, so those entries are widened in place. + if jq -e '[.hooks.PreToolUse // [] | .[] | .hooks // [] | .[] | .command // ""] + | any(test("review-gate"))' "$settings_file" &>/dev/null; then + local widened + widened=$(jq ' + .hooks.PreToolUse |= map( + if ([.hooks // [] | .[] | .command // ""] | any(test("review-gate"))) + and (.matcher == "Bash") + then .matcher = "Bash|PowerShell" else . end)' "$settings_file") || widened="" + if [[ -n "$widened" ]] && ! diff -q <(jq -S . "$settings_file") <(printf '%s\n' "$widened" | jq -S .) &>/dev/null; then + printf '%s\n' "$widened" | jq . > "$settings_file" + print_success "Widened review gate matcher to ${BOLD}Bash|PowerShell${NC}" + else + echo -e " ${DIM}Review gate hook already configured${NC}" + fi + return + fi + + echo "" + echo " A PreToolUse hook that stops an agent from committing unreviewed code." + echo " It inspects the staged diff, allows rebases, reverts, doc-only churn" + echo " already reviewed, and anything code-review-loop signed off on, and" + echo " otherwise hands the agent the diff so it can classify the change or" + echo " ask you." + echo "" + echo -e " Default mode is ${BOLD}warn${NC} (logs, never blocks). Switch to" + echo " REVIEW_GATE=block in ~/.ai-coding-setup.conf once you trust it." + echo "" + + if ! prompt_yes_no " Install the review gate hook for Claude Code?" n; then + print_info "Skipped review gate hook" + return + fi + + local updated + # The path is quoted: the harness parses this string with a shell, and a + # Windows profile directory ("C:\Users\First Last") would otherwise split + # into two arguments and execute nothing. + updated=$(jq --arg cmd "\"$gate\" --format=claude" ' + .hooks = (.hooks // {}) + | .hooks.PreToolUse = ((.hooks.PreToolUse // []) + [{ + matcher: "Bash|PowerShell", + hooks: [{type: "command", command: $cmd}] + }])' "$settings_file") || { + print_warning "Could not update $settings_file, skipping" + return + } + printf '%s\n' "$updated" | jq . > "$settings_file" + print_success "Installed review gate hook (mode: ${BOLD}warn${NC})" +} + # ---- argument parsing ---------------------------------------------------- while [[ $# -gt 0 ]]; do @@ -1442,6 +1524,11 @@ if [[ -f "$_prompts_dest/plan-review.md" ]] \ && [[ -f "$_prompts_dest/plan-review-followup.md" ]]; then review_loop_scripts+=("bin/plan-review-loop") fi +# review-gate reads no prompts; it only needs the shared library, which is +# installed alongside the loop scripts below. +if [[ -f "bin/review-gate" ]] && [[ ${#review_loop_scripts[@]} -gt 0 ]]; then + review_loop_scripts+=("bin/review-gate") +fi if [[ ${#review_loop_scripts[@]} -gt 0 ]]; then # Install the shared library to ~/.local/lib/ (not bin/, since it's sourced, not executed) @@ -1461,6 +1548,10 @@ if [[ ${#review_loop_scripts[@]} -gt 0 ]]; then || cp "$_lib_src" "$_lib_dest/lib-review-loop" fi install_scripts "bin" "${review_loop_scripts[@]}" + # Needs the symlink in place, so this runs after install_scripts. + if [[ " ${selected_tools[*]} " == *" claude "* ]]; then + configure_review_gate_hook + fi elif [[ -d "bin" ]]; then echo "" echo -e " ${DIM}Review loop scripts require shared prompts, skipped${NC}" @@ -1499,6 +1590,11 @@ EDITOR_AGENT=$_default_editor # Agent for reviewing (claude, codex, copilot, antigravity, kimi) REVIEWER_AGENT=$_default_reviewer + +# Code review gate (bin/review-gate): off, warn, or block. +# warn logs an unreviewed commit and lets it through; block denies it and hands +# the agent the diff so it can classify the change or ask you. +REVIEW_GATE=warn CONF print_success "Created $config_file" else @@ -1526,6 +1622,9 @@ done if [[ ${#review_loop_scripts[@]} -gt 0 ]]; then echo "" echo -e " Review loops: run ${BOLD}code-review-loop${NC} or ${BOLD}plan-review-loop${NC} from any project" + if [[ " ${review_loop_scripts[*]} " == *" bin/review-gate "* ]]; then + echo -e " Review gate: set ${BOLD}REVIEW_GATE=block${NC} in ~/.ai-coding-setup.conf to enforce it" + fi fi if gh extension list 2>/dev/null | grep -qF 'github/gh-stack'; then diff --git a/test/lib-review-loop.bats b/test/lib-review-loop.bats index 8c6ac94..ae6f8e4 100644 --- a/test/lib-review-loop.bats +++ b/test/lib-review-loop.bats @@ -540,25 +540,38 @@ make_run_dir() { # make_run_dir [[ "$dir" == "$BATS_TEST_TMPDIR/deep/nested/root/"* ]] } -@test "is_inside_dir resolves relative, trailing-slash and symlinked paths" { +@test "is_inside_dir resolves relative and trailing-slash paths" { source_lib local root="$BATS_TEST_TMPDIR/root" mkdir -p "$root/logs" "$root/other" : > "$root/logs/a.log" : > "$root/other/b.txt" - ln -s "$root/logs" "$root/linked" cd "$root" || return 1 run is_inside_dir "logs/a.log" "logs"; assert_success run is_inside_dir "logs/a.log" "logs/"; assert_success run is_inside_dir "logs/a.log" "$root/logs"; assert_success - run is_inside_dir "logs/a.log" "linked"; assert_success run is_inside_dir "other/b.txt" "logs"; assert_failure # An empty or missing directory is never a container. run is_inside_dir "logs/a.log" ""; assert_failure run is_inside_dir "logs/a.log" "$root/nope"; assert_failure } +# Split out, because Git Bash copies rather than links unless Developer Mode is +# on, which leaves no symlink to resolve. Detected by trying, not by testing the +# platform name: a Windows box that can make real symlinks should still run it. +@test "is_inside_dir resolves a symlinked directory" { + source_lib + local root="$BATS_TEST_TMPDIR/root" + mkdir -p "$root/logs" + : > "$root/logs/a.log" + ln -s "$root/logs" "$root/linked" 2>/dev/null || true + [ -L "$root/linked" ] || skip "no real symlink support here" + cd "$root" || return 1 + + run is_inside_dir "logs/a.log" "linked"; assert_success +} + # ========================================================================= # Codex invocation # diff --git a/test/review-gate.bats b/test/review-gate.bats new file mode 100644 index 0000000..bec38e3 --- /dev/null +++ b/test/review-gate.bats @@ -0,0 +1,969 @@ +#!/usr/bin/env bats +# Tests for bin/review-gate: the code review gate. It is a harness PreToolUse +# hook, not a git pre-commit hook, which matters when working out why it ran. +# +# All of the gate's logic is git plumbing and string parsing, so the whole +# decision tree is reachable without invoking a single agent. +# +# A `@test "..." { ... }` body reads to the linter as a subshell, so every +# variable a test exports and then reads back looks like a modification that +# gets lost. bats runs each body in its own process by design; that is exactly +# the isolation these tests rely on. +# +# SC2016 is off for the same kind of reason: the PowerShell cases pass +# `$env:...` through as literal text for the gate to parse, so expanding it +# here is exactly what must not happen. +# shellcheck disable=SC2030,SC2031,SC2016 + +load test_helper + +GATE="$PROJECT_ROOT/bin/review-gate" + +# ---- fixtures ------------------------------------------------------------- + +# Create a throwaway repo at $REPO with one commit, and stage nothing. +mkrepo() { + REPO="$TEST_TMPDIR/repo" + rm -rf "$REPO" + mkdir -p "$REPO" + git -C "$REPO" init -q . + git -C "$REPO" config user.email t@t + git -C "$REPO" config user.name t + echo base > "$REPO/base.txt" + git -C "$REPO" add base.txt + git -C "$REPO" commit -qm init +} + +# Create a throwaway repo at $REPO with no commits at all. +mkrepo_unborn() { + REPO="$TEST_TMPDIR/repo" + rm -rf "$REPO" + mkdir -p "$REPO" + git -C "$REPO" init -q . + git -C "$REPO" config user.email t@t + git -C "$REPO" config user.name t +} + +# Stage a code change, so the gate has something to refuse. +stage_code() { + echo "def f(): return 1" > "$REPO/app.py" + git -C "$REPO" add app.py +} + +# Feed one command through the gate as the harness would. +gate() { # gate + # jq is a native Windows binary under Git Bash, and MSYS rewrites any + # argument that looks like a POSIX path on the way to one. Without these, + # `--arg c '/usr/bin/git commit'` reaches jq as the Git installation's own + # path, which on a runner is `C:/Program Files/Git/usr/bin/git` and + # tokenizes into two words, testing something nobody wrote. Both variables + # are inert off Windows. + MSYS_NO_PATHCONV=1 MSYS2_ARG_CONV_EXCL='*' \ + jq -nc --arg c "$1" --arg cwd "$REPO" --arg tool "${GATE_TOOL:-Bash}" \ + '{tool_name: $tool, tool_input: {command: $c}, cwd: $cwd}' \ + | "$GATE" "--format=${GATE_FORMAT:-claude}" +} + +# Feed one command through the gate as the Windows PowerShell tool would. +# Windows harnesses expose that tool alongside Bash, and it takes incompatible +# syntax for the bypass the gate hands back. +ps_gate() { # ps_gate + GATE_TOOL=PowerShell gate "$1" +} + +state_dir() { printf '%s\n' "$REPO/.git/ai-review"; } + +# Record a receipt through the library helper the review loop uses. +write_receipt() { # write_receipt + ( + cd "$REPO" || exit 1 + unset _LIB_REVIEW_LOOP_LOADED + # shellcheck source=lib/lib-review-loop + source "$PROJECT_ROOT/lib/lib-review-loop" + write_review_receipt "$1" 1 claude codex + ) +} + +# Pull the nonce out of a deny reason. +nonce_from_output() { + printf '%s' "$output" | grep -o 'AI_REVIEW_GATE=[0-9a-f]\{4,\}' | head -1 | cut -d= -f2 +} + +# Same, from the PowerShell statement form the gate emits for that tool. +ps_nonce_from_output() { + printf '%s' "$output" \ + | grep -o '\$env:AI_REVIEW_GATE = \\"[0-9a-f]\{4,\}\\"' \ + | head -1 | grep -o '[0-9a-f]\{4,\}' +} + +assert_allowed() { + assert_success + assert_output "" +} + +assert_denied() { + assert_success + assert_output --partial '"permissionDecision":"deny"' +} + +assert_warned() { + assert_success + assert_output --partial 'Code review gate' + refute_output --partial '"permissionDecision":"deny"' +} + +# Blocking is the interesting mode; the shipped default is warn. +setup_gate() { + export REVIEW_GATE=block + unset AI_REVIEW_GATE AI_REVIEW_HEADLESS CI GATE_FORMAT +} + +# ========================================================================= +# The infinite-loop test, first. +# +# A PreToolUse hook is spawned by the harness before the shell runs the +# command, so an inline `AI_REVIEW_GATE= git commit ...` assignment +# never reaches the hook's own environment. Reading it from the environment +# instead of the command string is the single most likely way to ship a gate +# that denies the agent's retry forever. +# ========================================================================= + +@test "nonce is read from an inline assignment in the command string" { + setup_gate; mkrepo; stage_code + + run gate 'git commit -m "add app"' + assert_denied + local n + n=$(nonce_from_output) + [ -n "$n" ] + + # The hook's own environment carries no nonce; only the command does. + run env -u AI_REVIEW_GATE bash -c \ + "REVIEW_GATE=block; export REVIEW_GATE; jq -nc --arg c 'AI_REVIEW_GATE=$n git commit -m \"add app\"' --arg cwd '$REPO' '{tool_input:{command:\$c},cwd:\$cwd}' | '$GATE'" + assert_allowed +} + +@test "nonce is single-use" { + setup_gate; mkrepo; stage_code + + run gate 'git commit -m "add app"' + assert_denied + local n + n=$(nonce_from_output) + + run gate "AI_REVIEW_GATE=$n git commit -m \"add app\"" + assert_allowed + + run gate "AI_REVIEW_GATE=$n git commit -m \"add app\"" + assert_denied +} + +@test "nonce is rejected for a different index tree" { + setup_gate; mkrepo; stage_code + + run gate 'git commit -m "add app"' + assert_denied + local n + n=$(nonce_from_output) + + # Staging more work moves the index out from under the nonce. + echo more > "$REPO/other.py" + git -C "$REPO" add other.py + + run gate "AI_REVIEW_GATE=$n git commit -m \"add app\"" + assert_denied +} + +# ========================================================================= +# Command detection +# ========================================================================= + +@test "plain git commit is detected" { + setup_gate; mkrepo; stage_code + run gate 'git commit -m "add app"' + assert_denied +} + +# A wrapper or a decorated command word must not be a way past the gate. Each +# of these is an ordinary commit that an agent can type without trying, and +# every one of them was allowed before. + +@test "git.exe is detected" { + setup_gate; mkrepo; stage_code + run gate 'git.exe commit -m "add app"' + assert_denied +} + +@test "a git.exe command word is matched case-insensitively" { + setup_gate; mkrepo; stage_code + run gate 'GIT.EXE commit -m "add app"' + assert_denied +} + +@test "an absolute path to git is detected" { + setup_gate; mkrepo; stage_code + run gate '/usr/bin/git commit -m "add app"' + assert_denied +} + +@test "a Windows path to git.exe is detected under PowerShell" { + setup_gate; mkrepo; stage_code + # A backslash is a path character there, not an escape. Folding it away + # turns the command word into something unrecognizable and lets it past. + run ps_gate '& "C:\Program Files\Git\bin\git.exe" commit -m "add app"' + assert_denied +} + +@test "env -i git commit is detected" { + setup_gate; mkrepo; stage_code + run gate 'env -i git commit -m "add app"' + assert_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 +} + +@test "env --split-string carrying a commit is detected" { + setup_gate; mkrepo; stage_code + # -S packs a whole command into one argument. Skipping that argument steps + # straight over the commit. + run gate 'env -S "git commit -m x"' + assert_denied + run gate 'env --split-string="git commit -m x"' + assert_denied +} + +@test "staging flags inside an env --split-string are still read" { + setup_gate; mkrepo; stage_code + run gate 'env -S "git commit -am x"' + assert_denied + assert_output --partial 'beyond the index' +} + +@test "operands after an env --split-string belong to the same command" { + setup_gate; mkrepo; stage_code + # env appends them to the command it builds, so `env -S "git" commit` is a + # commit and dropping the tail loses it entirely. + run gate 'env -S "git" commit -m x' + assert_denied +} + +@test "a staging flag after an env --split-string is still read" { + setup_gate; mkrepo; stage_code + # Losing this one is worse than losing the commit: the gate would let a + # receipt describing the index vouch for a working-tree commit. + run gate 'env -S "git commit" -a -m x' + assert_denied + assert_output --partial 'beyond the index' +} + +@test "a spaced message survives the split-string path intact" { + setup_gate; mkrepo; stage_code + # If the token arrays were expanded unquoted anywhere on this path, the + # message would split and its fragments would read as pathspecs, which the + # gate reports as committing beyond the index. Absence of that is the + # assertion: the message stayed one token. + run gate 'env -S "git commit" -m "one two three four"' + assert_denied + refute_output --partial 'beyond the index' +} + +@test "env --split-string without a commit still allows" { + setup_gate; mkrepo; stage_code + run gate 'env -S "ls -la"' + assert_allowed +} + +@test "command and sudo wrappers are detected" { + setup_gate; mkrepo; stage_code + run gate 'command git commit -m "add app"' + assert_denied + run gate 'sudo git commit -m "add app"' + assert_denied +} + +@test "a bypass value inside env -i is still only honoured if it is the real nonce" { + setup_gate; mkrepo; stage_code + run gate 'env -i AI_REVIEW_GATE=bogus git commit -m "add app"' + assert_denied +} + +@test "command -v git is not a commit" { + setup_gate; mkrepo; stage_code + run gate 'command -v git' + assert_allowed +} + +@test "git -C dir commit is detected" { + setup_gate; mkrepo; stage_code + run gate "git -C $REPO commit -m \"add app\"" + assert_denied +} + +@test "a commit buried in an && chain is detected" { + setup_gate; mkrepo; stage_code + run gate 'npm test && git commit -m "add app" && echo done' + assert_denied +} + +@test "a commit buried in a ; chain is detected" { + setup_gate; mkrepo; stage_code + run gate 'echo hi ; git commit -m "add app"' + assert_denied +} + +@test "git commit --amend is detected" { + setup_gate; mkrepo; stage_code + run gate 'git commit --amend --no-edit' + assert_denied +} + +@test "git commit-tree is not a commit" { + setup_gate; mkrepo; stage_code + run gate 'git commit-tree HEAD^{tree} -m x' + assert_allowed +} + +@test "git log with the word commit in a format string is not a commit" { + setup_gate; mkrepo; stage_code + run gate 'git log --format="%H is a commit" -n 5' + assert_allowed +} + +# ========================================================================= +# Quote awareness +# +# A naive split on ; && || corrupts on the most ordinary input there is. +# ========================================================================= + +@test "a message containing separators, -a, and a path parses as one plain commit" { + setup_gate; mkrepo; stage_code + run gate 'git commit -m "fix: sanitize input; also update docs && tests -a lib/foo.py"' + assert_denied + # If the message had been read as flags, the reason would name a staging flag. + refute_output --partial 'commits content beyond the index' +} + +@test "an unparseable command mentioning git and commit denies rather than guesses" { + setup_gate; mkrepo + # Unbalanced quote: the git word is swallowed into one opaque token. + run gate '"git commit -m x' + assert_denied + assert_output --partial 'could not be parsed' +} + +@test "a command substitution wrapping the git word denies" { + setup_gate; mkrepo + # Single quotes on purpose: the gate has to receive the literal $(...) text. + # shellcheck disable=SC2016 + run gate '$(echo git) commit -m x' + assert_denied + assert_output --partial 'could not be parsed' +} + +# ========================================================================= +# Commands that commit content beyond the index +# +# These are checked before any index-derived rule. Otherwise `git commit -a` +# with a clean index sails through the empty-index check and commits +# unreviewed working-tree changes. +# ========================================================================= + +@test "-a forces a miss even when the index is empty" { + setup_gate; mkrepo + echo changed > "$REPO/base.txt" + run gate 'git commit -a -m "x"' + assert_denied + assert_output --partial 'commits content beyond the index' +} + +@test "--all forces a miss even when the index is empty" { + setup_gate; mkrepo + run gate 'git commit --all -m "x"' + assert_denied +} + +@test "clustered -am forces a miss" { + setup_gate; mkrepo + run gate 'git commit -am "x"' + assert_denied + assert_output --partial 'commits content beyond the index' +} + +@test "--only forces a miss" { + setup_gate; mkrepo + run gate 'git commit --only base.txt -m "x"' + assert_denied +} + +@test "--include forces a miss" { + setup_gate; mkrepo + run gate 'git commit --include base.txt -m "x"' + assert_denied +} + +@test "-i forces a miss" { + setup_gate; mkrepo + run gate 'git commit -i base.txt -m "x"' + assert_denied +} + +@test "a positional pathspec forces a miss" { + setup_gate; mkrepo + run gate 'git commit base.txt -m "x"' + assert_denied + assert_output --partial 'commits content beyond the index' +} + +@test "a pathspec after -- forces a miss" { + setup_gate; mkrepo + run gate 'git commit -m "x" -- base.txt' + assert_denied +} + +@test "a redirection is not mistaken for a pathspec" { + setup_gate; mkrepo + run gate 'git commit -m "x" > /dev/null' + # Nothing staged, so this allows; the point is that it is not a pathspec miss. + assert_allowed +} + +# ========================================================================= +# Review receipts +# ========================================================================= + +@test "a clean receipt for this index and HEAD allows" { + setup_gate; mkrepo; stage_code + write_receipt clean + run gate 'git commit -m "add app"' + assert_allowed +} + +@test "a receipt goes stale when the index moves" { + setup_gate; mkrepo; stage_code + write_receipt clean + echo extra > "$REPO/extra.py" + git -C "$REPO" add extra.py + + run gate 'git commit -m "add app"' + assert_denied + assert_output --partial 'the index has changed since' +} + +@test "a receipt goes stale when HEAD moves" { + setup_gate; mkrepo; stage_code + write_receipt clean + # Move the base out from under the review without touching the index. + echo other > "$REPO/other.txt" + git -C "$REPO" -c core.hooksPath=/dev/null commit -q --only base.txt -m "unrelated" --allow-empty + + run gate 'git commit -m "add app"' + assert_denied + assert_output --partial 'HEAD has moved' +} + +@test "a needs-review receipt does not satisfy the gate" { + setup_gate; mkrepo; stage_code + write_receipt needs-review + run gate 'git commit -m "add app"' + assert_denied + assert_output --partial 'verdict was not clean' +} + +@test "a missing receipt misses" { + setup_gate; mkrepo; stage_code + run gate 'git commit -m "add app"' + assert_denied + assert_output --partial 'no review has been recorded' +} + +@test "a corrupt receipts file misses rather than crashing or allowing" { + setup_gate; mkrepo; stage_code + mkdir -p "$(state_dir)" + printf 'not json at all {{{' > "$(state_dir)/receipts.json" + + run gate 'git commit -m "add app"' + assert_denied +} + +@test "receipts are capped at ten, newest first" { + setup_gate; mkrepo + local i + for i in 1 2 3 4 5 6 7 8 9 10 11 12; do + echo "v$i" > "$REPO/app.py" + git -C "$REPO" add app.py + write_receipt clean + done + run jq 'length' "$(state_dir)/receipts.json" + assert_output "10" + + run jq -r '.[0].index_tree' "$(state_dir)/receipts.json" + local newest="$output" + run git -C "$REPO" write-tree + assert_output "$newest" +} + +# ========================================================================= +# Unborn HEAD +# +# `git rev-parse HEAD` exits 128 in a fresh repository, which would break both +# the receipt writer and the gate on the very first commit. +# ========================================================================= + +@test "the first commit in a fresh repo misses instead of crashing" { + setup_gate; mkrepo_unborn + echo "def f(): return 1" > "$REPO/app.py" + git -C "$REPO" add app.py + + run gate 'git commit -m "initial"' + assert_denied +} + +@test "a receipt written against an unborn HEAD satisfies the gate" { + setup_gate; mkrepo_unborn + echo "def f(): return 1" > "$REPO/app.py" + git -C "$REPO" add app.py + write_receipt clean + + run jq -r '.[0].head' "$(state_dir)/receipts.json" + assert_output "unborn" + + run gate 'git commit -m "initial"' + assert_allowed +} + +@test "an empty index in a fresh repo allows" { + setup_gate; mkrepo_unborn + run gate 'git commit -m "initial"' + assert_allowed +} + +# ========================================================================= +# Tier 1 exemptions +# ========================================================================= + +@test "nothing staged allows" { + setup_gate; mkrepo + run gate 'git commit -m "x"' + assert_allowed +} + +@test "an amend that only rewords allows" { + setup_gate; mkrepo + run gate 'git commit --amend -m "better message"' + assert_allowed +} + +@test "a rebase in progress allows" { + setup_gate; mkrepo + echo two > "$REPO/base.txt"; git -C "$REPO" commit -qam two + echo three > "$REPO/base.txt"; git -C "$REPO" commit -qam three + + # Stop the rebase on its first step, so .git/rebase-merge is present. + cat > "$TEST_TMPDIR/seq-editor" <<'EDITOR' +#!/bin/sh +printf 'break\n' > "$1.tmp" +cat "$1" >> "$1.tmp" +mv "$1.tmp" "$1" +EDITOR + chmod +x "$TEST_TMPDIR/seq-editor" + GIT_SEQUENCE_EDITOR="$TEST_TMPDIR/seq-editor" git -C "$REPO" rebase -q -i HEAD~2 + [ -d "$REPO/.git/rebase-merge" ] || [ -d "$REPO/.git/rebase-apply" ] + + stage_code + run gate 'git commit --amend --no-edit' + assert_allowed + + git -C "$REPO" rebase --abort +} + +@test "an index tree identical to ORIG_HEAD allows" { + setup_gate; mkrepo + echo two > "$REPO/base.txt"; git -C "$REPO" commit -qam two + # A soft reset leaves the index holding the tree ORIG_HEAD points at, + # which is the invariant a history rewrite preserves by construction. + git -C "$REPO" reset -q --soft HEAD~1 + + run gate 'git commit -m "recommit"' + assert_allowed +} + +@test "a tree that appears only in older reflog entries is not exempt" { + setup_gate; mkrepo + echo two > "$REPO/base.txt"; git -C "$REPO" commit -qam two + echo three > "$REPO/base.txt"; git -C "$REPO" commit -qam three + + # Put the first commit's tree back in the index. It is reachable in the + # reflog, but not from ORIG_HEAD or HEAD@{1}, so it must still be reviewed. + git -C "$REPO" read-tree HEAD~2 + + run gate 'git commit -m "revive"' + assert_denied +} + +@test "a conflicted index allows, since a merge is already in progress" { + setup_gate; mkrepo + git -C "$REPO" checkout -q -b other + echo theirs > "$REPO/base.txt"; git -C "$REPO" commit -qam theirs + git -C "$REPO" checkout -q - + echo ours > "$REPO/base.txt"; git -C "$REPO" commit -qam ours + git -C "$REPO" merge other >/dev/null 2>&1 || true + run git -C "$REPO" write-tree + assert_failure + + run gate 'git commit -m "resolve"' + assert_allowed +} + +# ========================================================================= +# Recursion guard +# ========================================================================= + +@test "a live lock file allows" { + setup_gate; mkrepo; stage_code + mkdir -p "$(state_dir)" + printf '%s\t%s\n' "$$" "$(date +%s)" > "$(state_dir)/running" + + run gate 'git commit -m "add app"' + assert_allowed +} + +@test "a lock file with a dead pid does not allow and is pruned" { + setup_gate; mkrepo; stage_code + mkdir -p "$(state_dir)" + bash -c 'exit 0' & local dead=$! + wait "$dead" 2>/dev/null || true + printf '%s\t%s\n' "$dead" "$(date +%s)" > "$(state_dir)/running" + + run gate 'git commit -m "add app"' + assert_denied + [ ! -f "$(state_dir)/running" ] +} + +@test "a lock file past the age bound does not allow and is pruned" { + setup_gate; mkrepo; stage_code + mkdir -p "$(state_dir)" + # Live pid, but started long enough ago that pid reuse is plausible. + printf '%s\t%s\n' "$$" "1" > "$(state_dir)/running" + + run gate 'git commit -m "add app"' + assert_denied + [ ! -f "$(state_dir)/running" ] +} + +@test "AI_REVIEW_GATE=off in the environment allows" { + setup_gate; mkrepo; stage_code + export AI_REVIEW_GATE=off + run gate 'git commit -m "add app"' + assert_allowed +} + +@test "AI_REVIEW_GATE=off inline on the command allows" { + setup_gate; mkrepo; stage_code + run gate 'AI_REVIEW_GATE=off git commit -m "add app"' + assert_allowed +} + +# ========================================================================= +# PowerShell. +# +# Windows harnesses route some commands through a PowerShell tool rather than +# the Bash one. PowerShell has no `VAR=value command` prefix, so the bypass and +# the nonce arrive as their own statement ahead of the commit, and the gate has +# to both read that form and emit it. +# ========================================================================= + +@test 'a PowerShell $env: bypass with spaces around = allows' { + setup_gate; mkrepo; stage_code + run ps_gate '$env:AI_REVIEW_GATE = "off"; git commit -m "add app"' + assert_allowed +} + +@test 'a PowerShell $env: bypass without spaces around = allows' { + setup_gate; mkrepo; stage_code + run ps_gate '$env:AI_REVIEW_GATE="off"; git commit -m "add app"' + assert_allowed +} + +@test 'a PowerShell $env: bypass is matched case-insensitively' { + setup_gate; mkrepo; stage_code + run ps_gate "\$Env:ai_review_gate = 'off'; git commit -m 'add app'" + assert_allowed +} + +@test 'a PowerShell $env: assignment after the commit does not bypass' { + setup_gate; mkrepo; stage_code + run ps_gate 'git commit -m "add app"; $env:AI_REVIEW_GATE = "off"' + assert_denied +} + +@test 'an unrelated PowerShell $env: assignment does not bypass' { + setup_gate; mkrepo; stage_code + run ps_gate '$env:SOMETHING_ELSE = "off"; git commit -m "add app"' + assert_denied +} + +@test "the PowerShell tool gets the bypass in PowerShell syntax" { + setup_gate; mkrepo; stage_code + run ps_gate 'git commit -m "add app"' + assert_denied + assert_output --partial '$env:AI_REVIEW_GATE = ' + refute_output --partial 'AI_REVIEW_GATE=off git commit' +} + +@test "the Bash tool still gets the bypass as a prefix assignment" { + setup_gate; mkrepo; stage_code + run gate 'git commit -m "add app"' + assert_denied + assert_output --partial 'AI_REVIEW_GATE=' + refute_output --partial '$env:AI_REVIEW_GATE' +} + +@test "a nonce issued to the PowerShell tool round-trips in its own syntax" { + setup_gate; mkrepo; stage_code + run ps_gate 'git commit -m "add app"' + assert_denied + local n + n=$(ps_nonce_from_output) + [ -n "$n" ] + run ps_gate "\$env:AI_REVIEW_GATE = \"$n\"; git commit -m \"add app\"" + assert_allowed +} + +@test "a nonce is single-use across shells" { + setup_gate; mkrepo; stage_code + run ps_gate 'git commit -m "add app"' + local n + n=$(ps_nonce_from_output) + run ps_gate "\$env:AI_REVIEW_GATE = \"$n\"; git commit -m \"add app\"" + assert_allowed + run ps_gate "\$env:AI_REVIEW_GATE = \"$n\"; git commit -m \"add app\"" + assert_denied +} + +@test "the review loop lock helpers round-trip" { + setup_gate; mkrepo; stage_code + ( + cd "$REPO" || exit 1 + unset _LIB_REVIEW_LOOP_LOADED + # shellcheck source=lib/lib-review-loop + source "$PROJECT_ROOT/lib/lib-review-loop" + review_gate_lock_acquire + ) + [ -f "$(state_dir)/running" ] + ( + cd "$REPO" || exit 1 + unset _LIB_REVIEW_LOOP_LOADED + # shellcheck source=lib/lib-review-loop + source "$PROJECT_ROOT/lib/lib-review-loop" + review_gate_lock_release + ) + [ ! -f "$(state_dir)/running" ] +} + +# ========================================================================= +# Modes +# ========================================================================= + +@test "REVIEW_GATE=off disables the gate entirely" { + setup_gate; mkrepo; stage_code + export REVIEW_GATE=off + run gate 'git commit -m "add app"' + assert_allowed +} + +@test "REVIEW_GATE=off in the config file disables the gate" { + setup_gate; mkrepo; stage_code + unset REVIEW_GATE + printf 'REVIEW_GATE=off\n' > "$HOME/.ai-coding-setup.conf" + run gate 'git commit -m "add app"' + assert_allowed +} + +@test "the environment beats the config file" { + setup_gate; mkrepo; stage_code + printf 'REVIEW_GATE=off\n' > "$HOME/.ai-coding-setup.conf" + export REVIEW_GATE=block + run gate 'git commit -m "add app"' + assert_denied +} + +@test "warn is the default mode" { + setup_gate; mkrepo; stage_code + unset REVIEW_GATE + run gate 'git commit -m "add app"' + assert_warned +} + +@test "AI_REVIEW_HEADLESS degrades block to warn" { + setup_gate; mkrepo; stage_code + export AI_REVIEW_HEADLESS=1 + run gate 'git commit -m "add app"' + assert_warned +} + +@test "CI degrades block to warn" { + setup_gate; mkrepo; stage_code + export CI=true + run gate 'git commit -m "add app"' + assert_warned +} + +@test "no TTY on any descriptor with no headless marker still blocks" { + setup_gate; mkrepo; stage_code + # bats already runs the gate with pipes on every descriptor; a TTY-based + # headless test would degrade this run to warn-only and quietly turn the + # gate off everywhere. + run gate 'git commit -m "add app"' + assert_denied +} + +@test "warn mode does not issue a nonce" { + setup_gate; mkrepo; stage_code + export REVIEW_GATE=warn + run gate 'git commit -m "add app"' + assert_warned + [ ! -f "$(state_dir)/nonce" ] +} + +# ========================================================================= +# Output shapes +# ========================================================================= + +@test "claude format nests the decision under hookSpecificOutput" { + setup_gate; mkrepo; stage_code + run gate 'git commit -m "add app"' + assert_success + run jq -r '.hookSpecificOutput.hookEventName' <<< "$output" + assert_output "PreToolUse" +} + +@test "codex format nests the decision under hookSpecificOutput" { + setup_gate; mkrepo; stage_code + export GATE_FORMAT=codex + run gate 'git commit -m "add app"' + run jq -r '.hookSpecificOutput.permissionDecision' <<< "$output" + assert_output "deny" +} + +@test "copilot format is flat and carries a reason" { + setup_gate; mkrepo; stage_code + export GATE_FORMAT=copilot + run gate 'git commit -m "add app"' + assert_success + local json="$output" + run jq -r '.permissionDecision' <<< "$json" + assert_output "deny" + run jq -r '.permissionDecisionReason | length > 0' <<< "$json" + assert_output "true" +} + +@test "copilot format exits 0 on allow, since a non-zero exit denies there" { + setup_gate; mkrepo + export GATE_FORMAT=copilot + run gate 'git commit -m "x"' + assert_success + assert_output "" +} + +@test "antigravity format is flat" { + setup_gate; mkrepo; stage_code + export GATE_FORMAT=antigravity + run gate 'git commit -m "add app"' + run jq -r '.permissionDecision' <<< "$output" + assert_output "deny" +} + +@test "kimi format denies by exit code with the reason on stderr" { + setup_gate; mkrepo; stage_code + export GATE_FORMAT=kimi + run gate 'git commit -m "add app"' + [ "$status" -eq 2 ] + assert_output --partial 'Code review gate' +} + +# ========================================================================= +# Fast path and hygiene +# ========================================================================= + +@test "a command with no commit substring allows without touching git" { + setup_gate + REPO="$TEST_TMPDIR/not-a-repo" + mkdir -p "$REPO" + run gate 'ls -la && cat README.md' + assert_allowed +} + +@test "a commit outside a git repository allows" { + setup_gate + REPO="$TEST_TMPDIR/not-a-repo" + mkdir -p "$REPO" + run gate 'git commit -m "x"' + assert_allowed +} + +@test "--help prints usage" { + run "$GATE" --help + assert_success + assert_output --partial "Usage: review-gate" +} + +@test "the deny reason classifies paths by kind" { + setup_gate; mkrepo + mkdir -p "$REPO/test" "$REPO/dist" + echo "def f(): return 1" > "$REPO/app.py" + echo "def test_f(): pass" > "$REPO/test/test_app.py" + echo "# notes" > "$REPO/NOTES.md" + echo "bundled" > "$REPO/dist/bundle.min.js" + git -C "$REPO" add -A + + run gate 'git commit -m "mixed"' + assert_denied + assert_output --partial 'app.py' + assert_output --partial 'code' + assert_output --partial 'tests' + assert_output --partial 'docs' + assert_output --partial 'generated' +} + +@test "a path marked linguist-generated in .gitattributes counts as generated" { + setup_gate; mkrepo + printf 'derived/** linguist-generated=true\n' > "$REPO/.gitattributes" + git -C "$REPO" add .gitattributes + git -C "$REPO" commit -qm attrs + mkdir -p "$REPO/derived" + echo "auto" > "$REPO/derived/out.py" + git -C "$REPO" add derived/out.py + + run gate 'git commit -m "regen"' + assert_denied + assert_output --partial 'derived/out.py' + assert_output --partial 'generated' + refute_output --partial 'derived/out.py code' +} + +@test "state lands in the right git dir inside a linked worktree" { + setup_gate; mkrepo + # `git rev-parse --git-dir` is absolute here, not relative, which is the + # case a leading-slash prefix test gets wrong on Git for Windows. + git -C "$REPO" worktree add -q "$TEST_TMPDIR/wt" -b wt + local main_repo="$REPO" + REPO="$TEST_TMPDIR/wt" + echo "def f(): return 1" > "$REPO/app.py" + git -C "$REPO" add app.py + + run gate 'git commit -m "add app"' + assert_denied + + local wt_git_dir + wt_git_dir=$(git -C "$REPO" rev-parse --absolute-git-dir) + [ -f "$wt_git_dir/ai-review/nonce" ] + [ ! -e "$main_repo/.git/ai-review/nonce" ] + + write_receipt clean + run gate 'git commit -m "add app"' + assert_allowed +}