diff --git a/.claude/hooks.json b/.claude/hooks.json new file mode 100644 index 0000000..df297ef --- /dev/null +++ b/.claude/hooks.json @@ -0,0 +1,16 @@ +{ + "hooks": { + "Stop": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "/home/dan/proj/skills/skills/review-gate/scripts/review-gate check", + "timeout": 30 + } + ] + } + ] + } +} diff --git a/skills/review-gate/scripts/review-gate b/skills/review-gate/scripts/review-gate index 3a358df..c552f92 100755 --- a/skills/review-gate/scripts/review-gate +++ b/skills/review-gate/scripts/review-gate @@ -59,27 +59,24 @@ cmd_check() { exit 0 ;; pending) - echo "⏳ BLOCKED: Review pending for session: $session_id" - echo "" - echo "To proceed, spawn a reviewer agent:" - echo " /task Review the work in this session. Check git diff and file changes." - echo " Then run: review-gate approve $session_id" - echo "" - echo "Or to skip review: review-gate approve $session_id" - exit 1 + # Exit 2 + JSON to stderr = block in Claude Code Stop hook + local reason="Review pending for session: $session_id. Spawn a reviewer agent with: /task Review the work. Check git diff and file changes. Then run: review-gate approve $session_id" + jq -n --arg reason "$reason" '{ + "decision": "block", + "reason": $reason + }' >&2 + exit 2 ;; rejected) - local reason=$(jq -r '.reason // "No reason provided"' "$state_file") - local issues=$(jq -r '.issues[]? // empty' "$state_file" | head -5) - echo "❌ BLOCKED: Review rejected for session: $session_id" - echo "Reason: $reason" - if [[ -n "$issues" ]]; then - echo "Issues:" - echo "$issues" | sed 's/^/ - /' - fi - echo "" - echo "Fix the issues and request re-review." - exit 1 + local reject_reason=$(jq -r '.reason // "No reason provided"' "$state_file") + local issues=$(jq -r '.issues | join(", ")' "$state_file" 2>/dev/null || echo "") + # Exit 2 + JSON to stderr = block in Claude Code Stop hook + local reason="Review rejected for session: $session_id. Reason: $reject_reason. Issues: $issues. Fix the issues and request re-review." + jq -n --arg reason "$reason" '{ + "decision": "block", + "reason": $reason + }' >&2 + exit 2 ;; *) echo "⚠ Unknown review status: $status" diff --git a/skills/review-gate/tests/test-hook-integration.sh b/skills/review-gate/tests/test-hook-integration.sh new file mode 100755 index 0000000..033c685 --- /dev/null +++ b/skills/review-gate/tests/test-hook-integration.sh @@ -0,0 +1,224 @@ +#!/usr/bin/env bash +# Integration test for review-gate Stop hook +# +# This test verifies that the Stop hook actually blocks Claude Code +# when a review is pending. +# +# Usage: +# ./test-hook-integration.sh [--debug] +# +# Requirements: +# - Claude CLI installed +# - API credits available +# - Run from the skills repo root (or set SKILLS_ROOT) + +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SKILLS_ROOT="${SKILLS_ROOT:-$(cd "$SCRIPT_DIR/../../.." && pwd)}" +REVIEW_GATE="$SKILLS_ROOT/skills/review-gate/scripts/review-gate" +DEBUG="${1:-}" + +# Colors +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[0;33m' +NC='\033[0m' + +# Test state (isolated) +TEST_STATE_DIR=$(mktemp -d) +TEST_HOOKS_DIR=$(mktemp -d) +export REVIEW_STATE_DIR="$TEST_STATE_DIR" + +cleanup() { + rm -rf "$TEST_STATE_DIR" "$TEST_HOOKS_DIR" +} +trap cleanup EXIT + +echo "=== Review Gate Hook Integration Test ===" +echo "Skills root: $SKILLS_ROOT" +echo "State dir: $TEST_STATE_DIR" +echo "" + +# --- Setup --- +echo "## Setup" + +# Create hooks.json that points to our review-gate +# Use the test state dir via environment variable +cat > "$TEST_HOOKS_DIR/hooks.json" <&1) +exit_code=$? + +if [[ $exit_code -eq 2 ]]; then + echo -e "${GREEN}PASS${NC}: Exit code is 2 (blocking)" +else + echo -e "${RED}FAIL${NC}: Exit code is $exit_code (expected 2)" +fi + +if echo "$output" | grep -q '"decision": "block"'; then + echo -e "${GREEN}PASS${NC}: Output contains decision: block" +else + echo -e "${RED}FAIL${NC}: Output missing decision: block" + echo "Output was: $output" +fi + +echo "" + +# --- Test 2: Hook allows when approved --- +echo "## Test 2: Hook script allows when approved" + +"$REVIEW_GATE" approve "$TEST_SESSION" > /dev/null +output=$("$REVIEW_GATE" check "$TEST_SESSION" 2>&1) +exit_code=$? + +if [[ $exit_code -eq 0 ]]; then + echo -e "${GREEN}PASS${NC}: Exit code is 0 (allowing)" +else + echo -e "${RED}FAIL${NC}: Exit code is $exit_code (expected 0)" +fi + +echo "" + +# --- Test 3: Live Claude integration (optional) --- +echo "## Test 3: Live Claude integration" +echo "" +echo "This test runs Claude with the Stop hook and verifies blocking." +echo "It requires API credits and takes ~10-30 seconds." +echo "" + +# Re-enable review for live test +"$REVIEW_GATE" enable "$TEST_SESSION" > /dev/null + +# Check if we should run the live test +if [[ "${SKIP_LIVE_TEST:-}" == "1" ]]; then + echo -e "${YELLOW}SKIP${NC}: SKIP_LIVE_TEST=1 set, skipping live test" + exit 0 +fi + +echo "Running: claude -p 'Say hello' with Stop hook..." +echo "" + +# Build the claude command +# We need to inject our hooks somehow. Options: +# 1. Use --mcp-config (doesn't help for hooks) +# 2. Run from a directory with .claude/hooks.json +# 3. Use debug mode to see what happens + +# Create a temp project dir with our hooks +TEST_PROJECT=$(mktemp -d) +mkdir -p "$TEST_PROJECT/.claude" +cp "$TEST_HOOKS_DIR/hooks.json" "$TEST_PROJECT/.claude/hooks.json" + +# Also need a way to pass REVIEW_STATE_DIR to the hook +# Rewrite hooks.json with absolute paths and env vars baked in +cat > "$TEST_PROJECT/.claude/hooks.json" <&1) +exit_code=$? +set -e + +echo "Exit code: $exit_code" +echo "Output:" +echo "$output" +echo "" + +# Cleanup test project +rm -rf "$TEST_PROJECT" + +# Analyze results +echo "## Analysis" + +if echo "$output" | grep -qi "block"; then + echo -e "${GREEN}PASS${NC}: Output contains 'block' - hook appears to have fired" +elif echo "$output" | grep -qi "review pending"; then + echo -e "${GREEN}PASS${NC}: Output contains 'review pending' - hook fired" +elif echo "$output" | grep -qi "credit"; then + echo -e "${YELLOW}SKIP${NC}: API credit issue - cannot complete live test" +elif [[ $exit_code -eq 124 ]]; then + echo -e "${YELLOW}SKIP${NC}: Timeout - Claude may be blocked in a loop (which could mean hook is working)" +else + echo -e "${YELLOW}INCONCLUSIVE${NC}: Could not determine if hook fired" + echo "Check output above for clues" +fi + +echo "" +echo "=== Test Complete ===" +echo "" +echo "To re-run live test when credits available:" +echo " $0" +echo "" +echo "To skip live test:" +echo " SKIP_LIVE_TEST=1 $0" diff --git a/skills/review-gate/tests/test-review-gate.sh b/skills/review-gate/tests/test-review-gate.sh index 3e6ca75..9c5d159 100755 --- a/skills/review-gate/tests/test-review-gate.sh +++ b/skills/review-gate/tests/test-review-gate.sh @@ -167,9 +167,9 @@ echo "## Check When Pending (Should Block)" run_gate enable test-pending > /dev/null output=$(run_gate check test-pending) exit_code=$? -assert_exit_code "Check returns 1 when pending" "1" "$exit_code" -assert_output_contains "Check shows BLOCKED" "BLOCKED" "$output" -assert_output_contains "Check shows pending status" "pending" "$output" +assert_exit_code "Check returns 2 when pending (blocks hook)" "2" "$exit_code" +assert_output_contains "Check outputs decision: block" '"decision": "block"' "$output" +assert_output_contains "Check reason mentions pending" "pending" "$output" # --- Approve Command --- echo "" @@ -206,9 +206,9 @@ echo "## Check When Rejected (Should Block)" output=$(run_gate check test-reject) exit_code=$? -assert_exit_code "Check returns 1 when rejected" "1" "$exit_code" -assert_output_contains "Check shows BLOCKED" "BLOCKED" "$output" -assert_output_contains "Check shows reason" "Missing tests" "$output" +assert_exit_code "Check returns 2 when rejected (blocks hook)" "2" "$exit_code" +assert_output_contains "Check outputs decision: block" '"decision": "block"' "$output" +assert_output_contains "Check reason mentions rejection reason" "Missing tests" "$output" # --- Reject With Issues --- echo "" @@ -288,7 +288,7 @@ assert_json_field "Status reset to pending" "pending" ".status" "$TEST_STATE_DIR output=$(run_gate check reset-session) exit_code=$? -assert_exit_code "Check blocks after re-enable" "1" "$exit_code" +assert_exit_code "Check blocks after re-enable" "2" "$exit_code" # --- Clean Command --- echo ""