From f136b2d0317eef442a516086b4cc97b0f04d8c40 Mon Sep 17 00:00:00 2001 From: Barry Schneider Date: Fri, 24 Jul 2026 17:18:34 -0400 Subject: [PATCH] fix(hooks): pre-pr-review-gate honors the /pre-doc-review marker The gate only checked .pre-pr-review-done, so a doc-only PR that passed /pre-doc-review (which writes .pre-doc-review-done) was still blocked. Accept either marker at the current HEAD; add test-gates.sh coverage. Closes #267. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01YKt6y3Rk498eBqneiMKHpM --- .claude/hooks/pre-pr-review-gate.sh | 17 ++++++++++++----- .claude/hooks/test-gates.sh | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/.claude/hooks/pre-pr-review-gate.sh b/.claude/hooks/pre-pr-review-gate.sh index 31c11d1..6b2e5b5 100644 --- a/.claude/hooks/pre-pr-review-gate.sh +++ b/.claude/hooks/pre-pr-review-gate.sh @@ -1,19 +1,26 @@ #!/usr/bin/env bash # Pre-PR review gate. # PreToolUse on `mcp__github__create_pull_request` and `Bash(gh pr create*)`. -# Denies PR creation unless /pre-pr-review ran for the current HEAD -# (marker .claude/.pre-pr-review-done contains the current HEAD SHA). +# Denies PR creation unless a review ran for the current HEAD — either /pre-pr-review +# (marker .claude/.pre-pr-review-done) or, for a doc-only PR, /pre-doc-review +# (marker .claude/.pre-doc-review-done). Either marker holding the current HEAD SHA clears it (#267). set -euo pipefail PROJECT_DIR="${CLAUDE_PROJECT_DIR:-$(pwd)}" -MARKER="$PROJECT_DIR/.claude/.pre-pr-review-done" +CODE_MARKER="$PROJECT_DIR/.claude/.pre-pr-review-done" +DOC_MARKER="$PROJECT_DIR/.claude/.pre-doc-review-done" HEAD="$(git -C "$PROJECT_DIR" rev-parse HEAD 2>/dev/null || echo '')" -if [[ -n "$HEAD" && -f "$MARKER" && "$(cat "$MARKER" 2>/dev/null | tr -d '[:space:]')" == "$HEAD" ]]; then +# A marker clears the gate when it exists and holds the current HEAD SHA. The code-review marker +# (/pre-pr-review) OR the doc-review marker (/pre-doc-review) is sufficient — a doc-only PR +# legitimately clears via /pre-doc-review, which the gate previously ignored (#267). +marker_matches() { [[ -f "$1" && "$(cat "$1" 2>/dev/null | tr -d '[:space:]')" == "$HEAD" ]]; } + +if [[ -n "$HEAD" ]] && { marker_matches "$CODE_MARKER" || marker_matches "$DOC_MARKER"; }; then exit 0 fi -REASON="Pre-PR review gate: run the /pre-pr-review skill before opening a PR (mandatory workflow — do not bypass). Address any Blockers it reports, then retry. It writes the current HEAD SHA (${HEAD}) to ${MARKER} on an APPROVE / APPROVE-WITH-NITS verdict, which clears this gate." +REASON="Pre-PR review gate: run /pre-pr-review (code) or /pre-doc-review (doc-only) before opening a PR (mandatory workflow — do not bypass). Address any Blockers it reports, then retry. On an APPROVE / APPROVE-WITH-NITS verdict it writes the current HEAD SHA (${HEAD}) to ${CODE_MARKER} (or ${DOC_MARKER} for a doc-only PR), which clears this gate." if command -v jq >/dev/null 2>&1; then jq -n --arg reason "$REASON" '{hookSpecificOutput:{hookEventName:"PreToolUse",permissionDecision:"deny",permissionDecisionReason:$reason}}' diff --git a/.claude/hooks/test-gates.sh b/.claude/hooks/test-gates.sh index 731b5e5..e2f7b35 100644 --- a/.claude/hooks/test-gates.sh +++ b/.claude/hooks/test-gates.sh @@ -66,6 +66,26 @@ date -u +%Y-%m-%dT%H:%M:%SZ > "$FAKE/.claude/.draft-issue-done" out=$(CLAUDE_PROJECT_DIR=$FAKE bash "$DIG" <<<'{"tool_name":"mcp__github__issue_write","tool_input":{"method":"create","title":"x"}}') check "issue_write method=create, fresh marker -> allow (regression)" allow "$out" $? +# --- pre-pr-review-gate: /pre-doc-review marker clears it too (#267) --- +# Needs a real HEAD SHA, so spin a throwaway git repo (the gate reads `git -C $PROJECT_DIR rev-parse HEAD`). +PPG=$REPO/.claude/hooks/pre-pr-review-gate.sh +GITREPO=$(mktemp -d) +( cd "$GITREPO" && git init -q && git -c user.email=t@t.co -c user.name=t commit -q --allow-empty -m init ) +mkdir -p "$GITREPO/.claude" +GHSHA=$(git -C "$GITREPO" rev-parse HEAD) +ppg() { CLAUDE_PROJECT_DIR=$GITREPO bash "$PPG" <<<'{"tool_name":"Bash","tool_input":{"command":"gh pr create"}}'; } + +out=$(ppg); check "pre-pr-gate: no marker -> deny" deny "$out" $? +echo "$GHSHA" > "$GITREPO/.claude/.pre-doc-review-done" +out=$(ppg); check "pre-pr-gate: doc marker=HEAD -> allow (#267)" allow "$out" $? +rm -f "$GITREPO/.claude/.pre-doc-review-done" +echo "$GHSHA" > "$GITREPO/.claude/.pre-pr-review-done" +out=$(ppg); check "pre-pr-gate: code marker=HEAD -> allow (regression)" allow "$out" $? +echo "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" > "$GITREPO/.claude/.pre-doc-review-done" +rm -f "$GITREPO/.claude/.pre-pr-review-done" +out=$(ppg); check "pre-pr-gate: stale doc marker (wrong sha) -> deny" deny "$out" $? +rm -rf "$GITREPO" + rm -rf "$FAKE" echo; echo "RESULT: $PASS passed, $FAIL failed" [[ $FAIL -eq 0 ]]