diff --git a/.claude/hooks/pre-commit.sh b/.claude/hooks/pre-commit.sh index 55c7057..687923f 100755 --- a/.claude/hooks/pre-commit.sh +++ b/.claude/hooks/pre-commit.sh @@ -5,21 +5,69 @@ # to scripts/quality.sh — the single source of truth for the quality suite. # Exits non-zero to block the commit if the suite fails. # +# This hook fails OPEN by design: it runs before every Bash call, so blocking +# on its own internal errors would wedge the session. The price of failing +# open is that a broken gate is invisible — you keep believing you are gated +# while nothing runs. So every path that gives up says so, loudly, on stderr. +# set -uo pipefail -command=$(python3 -c ' +payload=$(cat) + +# No payload at all: nothing to inspect, and no real invocation looks like +# this. Stay quiet rather than crying wolf on every command. +[[ -z "$payload" ]] && exit 0 + +# Extract .tool_input.command. Two parsers, because the hook previously used +# python3 alone and did not check whether it existed — on a host without it, +# the capture came back empty, matched nothing, and the gate silently turned +# itself off. +parsed='' +have_parser=0 + +if command -v python3 >/dev/null 2>&1; then + if parsed=$(printf '%s' "$payload" | python3 -c ' import json, sys -try: - data = json.load(sys.stdin) - print(data.get("tool_input", {}).get("command", "")) -except Exception: - pass -') - -case "$command" in - git\ commit*) ;; - *) exit 0 ;; -esac +data = json.load(sys.stdin) +sys.stdout.write(str(data.get("tool_input", {}).get("command", ""))) +' 2>/dev/null); then + have_parser=1 + fi +fi + +if (( ! have_parser )) && command -v jq >/dev/null 2>&1; then + if parsed=$(printf '%s' "$payload" | jq -r '.tool_input.command // ""' 2>/dev/null); then + have_parser=1 + fi +fi + +if (( ! have_parser )); then + echo "pre-commit: cannot read the hook payload — no working python3 or jq on PATH." >&2 + echo "pre-commit: THE QUALITY GATE IS NOT RUNNING. Install python3 or jq to re-enable it." >&2 + exit 0 +fi + +# Match `git commit` anywhere in the command, not only at the start. +# +# The previous prefix match (`git commit*`) meant the gate never fired for +# `git add -A && git commit -m ...` — the most common commit idiom there is — +# nor for `cd sub && git commit`, nor `git -C dir commit`. The gate looked +# present and was inert for the shapes people actually type. +# +# The regex allows flags and paths between `git` and `commit`, but not a +# command separator, so `git status && make commit-docs` does not match. +# +# Tradeoff: this does match a command that merely mentions committing, e.g. +# `git log --grep=commit`. That direction is the safe one — a needless quality +# run costs seconds, a skipped gate costs a broken commit — but it is a real +# tradeoff, so the block message below names the command that triggered it. +# Held in a variable: inside [[ =~ ]] an inline pattern containing `;` or `&` +# is parsed as shell syntax before the regex engine ever sees it. +commit_re='(^|[;&|[:space:]])git[[:space:]][^;&|]*commit' + +if [[ ! "$parsed" =~ $commit_re ]]; then + exit 0 +fi if [[ ! -x "./scripts/quality.sh" ]]; then echo "scripts/quality.sh missing or not executable; skipping pre-commit gate." >&2 @@ -29,6 +77,7 @@ fi if ! ./scripts/quality.sh; then echo "" >&2 echo "Commit blocked. Fix the issues above, then commit again." >&2 + echo "Triggered by: $parsed" >&2 exit 1 fi diff --git a/CHANGELOG.md b/CHANGELOG.md index 016516b..24b1f53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ The kit's evolution, kept for humans. Per-feature progress lives in `.claude/pla - **Hook logic is no longer triplicated.** The active-feature selection was copy-pasted into all three hooks, with a comment in `stop.sh` asking the reader to keep them "in lockstep" by hand — which is how they came to share an identical bug. `.claude/hooks/lib.sh` now owns `hook_mtime`, `hook_is_complete` and `hook_active_progress`. `hook_mtime` probes once for which `stat` the machine has and validates its own output is numeric, so no future platform can reintroduce this class of bug. +- **The pre-commit gate never fired for the most common way to commit.** `pre-commit.sh` matched `git commit*` as a *prefix*, so it only triggered when the command began with those words. `git add -A && git commit -m x` — the idiom most people and most agents actually type — sailed straight past it, as did `cd sub && git commit` and `git -C dir commit`. The gate looked present and was inert for the shapes that matter. It now matches `git … commit` anywhere in the command, allowing flags and paths between the two words but not a command separator. + + The tradeoff is that a command merely *mentioning* committing (`git log --grep=commit`) now runs the suite. That is the safe direction — a needless quality run costs seconds, a skipped gate costs a broken commit — and a block now names the command that triggered it, so a false positive explains itself. + +- **The pre-commit gate silently disabled itself without `python3`.** The payload was parsed with `python3 -c` and nothing checked whether the interpreter existed. On a host without it the capture came back empty, matched no pattern, and the hook exited 0 — no gate, no warning, no way to notice. It now tries `python3`, falls back to `jq`, and if neither is available says **"THE QUALITY GATE IS NOT RUNNING"** on stderr. It still does not block, because this hook fires before every Bash call and blocking on its own internal errors would wedge the session — but failing open silently and failing open loudly are very different things. + - **A flaky test in `test-stop.sh`.** The multi-feature case rewrote both fixtures with `sed -i.bak`, making their mtimes equal to the second, then `touch`ed the newer one to break the tie. Since mtime resolution here is whole seconds, that only won a sub-second race — which macOS happened to win and Linux consistently lost. It now pushes the older fixture into the past with `touch -t`, which is portable and deterministic. ## v1.0.3 — 2026-06-11 diff --git a/tests/hooks/test-pre-commit.sh b/tests/hooks/test-pre-commit.sh index 01f876d..cdcc905 100755 --- a/tests/hooks/test-pre-commit.sh +++ b/tests/hooks/test-pre-commit.sh @@ -6,7 +6,17 @@ # 1. non-commit Bash → exit 0 (passthrough) # 2. git commit + no scripts/quality.sh → exit 0 with warning (skip gate) # 3. git commit + passing quality.sh → exit 0 -# 4. git commit + failing quality.sh → exit 1 (block) +# 4. git commit + failing quality.sh → exit 1 (block), naming the command +# 5. compound / flagged commit forms still trigger the gate +# 6. no JSON parser available → exit 0, but loudly +# 7. empty payload → quiet passthrough +# +# Cases 5 and 6 exist because this hook had two ways of silently doing +# nothing. A prefix match meant `git add -A && git commit -m x` — the most +# common commit idiom there is — never triggered it. And parsing the payload +# with python3 alone meant a host without python3 got no gate and no warning. +# Both failed open *invisibly*, which is the worst property a gate can have: +# you keep the belief that you are covered. # set -uo pipefail @@ -15,6 +25,24 @@ source "$(dirname "${BASH_SOURCE[0]}")/../lib.sh" echo "pre-commit.sh" +# Everything that asserts the gate *fires* needs a JSON parser on PATH, because +# without one the hook cannot read the command and correctly declines to gate. +# Demanding a gate on a host with no parser would make this suite red for +# behaving exactly as designed — the same "the test encodes my machine" mistake +# that hid the Linux mtime bug. Case 6 covers the no-parser contract head-on. +if command -v python3 >/dev/null 2>&1 || command -v jq >/dev/null 2>&1; then + HAVE_PARSER=1 +else + HAVE_PARSER=0 +fi + +write_quality() { + # write_quality + mkdir -p scripts + printf '#!/usr/bin/env bash\nexit %s\n' "$1" > scripts/quality.sh + chmod +x scripts/quality.sh +} + # --- Case 1: non-commit Bash command → exit 0, no gate run --- setup_sandbox run_hook "pre-commit.sh" '{"tool_input":{"command":"ls -la"}}' @@ -27,39 +55,91 @@ run_hook "pre-commit.sh" '{"tool_input":{"command":"git status"}}' assert_exit_code "git status (not commit) → exit 0" 0 "$HOOK_EXIT" teardown_sandbox -# --- Case 3: git commit but no scripts/quality.sh → exit 0 with warning --- -setup_sandbox -run_hook "pre-commit.sh" '{"tool_input":{"command":"git commit -m foo"}}' -assert_exit_code "git commit, no quality.sh → exit 0" 0 "$HOOK_EXIT" -assert_contains "git commit, no quality.sh → warns on stderr" \ - "$HOOK_STDERR_TEXT" "quality.sh missing" -teardown_sandbox +if (( HAVE_PARSER )); then + + # --- Case 3: git commit but no scripts/quality.sh → exit 0 with warning --- + setup_sandbox + run_hook "pre-commit.sh" '{"tool_input":{"command":"git commit -m foo"}}' + assert_exit_code "git commit, no quality.sh → exit 0" 0 "$HOOK_EXIT" + assert_contains "git commit, no quality.sh → warns on stderr" \ + "$HOOK_STDERR_TEXT" "quality.sh missing" + teardown_sandbox + + # --- Case 4: git commit + passing quality.sh → exit 0 --- + setup_sandbox + write_quality 0 + run_hook "pre-commit.sh" '{"tool_input":{"command":"git commit -m feat"}}' + assert_exit_code "git commit, quality pass → exit 0" 0 "$HOOK_EXIT" + teardown_sandbox + + # --- Case 5: git commit + failing quality.sh → exit 1 (blocks commit) --- + setup_sandbox + write_quality 1 + run_hook "pre-commit.sh" '{"tool_input":{"command":"git commit -m bad"}}' + assert_exit_code "git commit, quality fail → exit 1" 1 "$HOOK_EXIT" + assert_contains "git commit, quality fail → block message" \ + "$HOOK_STDERR_TEXT" "Commit blocked" + # The widened match can fire on a command that merely mentions committing, + # so a block has to say what triggered it or the user cannot tell why. + assert_contains "git commit, quality fail → names the command" \ + "$HOOK_STDERR_TEXT" "git commit -m bad" + teardown_sandbox -# --- Case 4: git commit + passing quality.sh → exit 0 --- + # --- Case 6: commit shapes a prefix match would miss --- + # Every one of these bypassed the gate entirely before the match was widened. + for cmd in \ + "git add -A && git commit -m x" \ + "cd sub && git commit -m x" \ + "git -C /some/dir commit -m x" \ + "git commit --amend --no-edit" + do + setup_sandbox + write_quality 1 + run_hook "pre-commit.sh" "{\"tool_input\":{\"command\":\"$cmd\"}}" + assert_exit_code "gate fires on: $cmd" 1 "$HOOK_EXIT" + teardown_sandbox + done + + # …and shapes that must still pass straight through. + for cmd in "git status" "git log --oneline" "ls -la" "make build"; do + setup_sandbox + write_quality 1 + run_hook "pre-commit.sh" "{\"tool_input\":{\"command\":\"$cmd\"}}" + assert_exit_code "gate ignores: $cmd" 0 "$HOOK_EXIT" + teardown_sandbox + done + +else + echo " - skipped 14 gate-firing cases: no python3 or jq on PATH" +fi + +# --- Case 7: no JSON parser on PATH → fail open, but say so --- +# This path used to be silent: no python3 meant no gate and no warning. setup_sandbox -mkdir -p scripts -cat > scripts/quality.sh <<'EOF' -#!/usr/bin/env bash -exit 0 -EOF -chmod +x scripts/quality.sh -run_hook "pre-commit.sh" '{"tool_input":{"command":"git commit -m feat"}}' -assert_exit_code "git commit, quality pass → exit 0" 0 "$HOOK_EXIT" +write_quality 1 + +# A PATH with no python3 and no jq on it. +stub_bin="$SANDBOX/stub-bin" +mkdir -p "$stub_bin" +for tool in bash cat grep sed; do + real=$(command -v "$tool" 2>/dev/null) && ln -sf "$real" "$stub_bin/$tool" 2>/dev/null +done + +out=$(PATH="$stub_bin" "$KIT_ROOT/.claude/hooks/pre-commit.sh" \ + <<< '{"tool_input":{"command":"git commit -m x"}}' 2>&1) +rc=$? + +assert_exit_code "no parser → does not block" 0 "$rc" +assert_contains "no parser → warns the gate is off" "$out" "QUALITY GATE IS NOT RUNNING" teardown_sandbox -# --- Case 5: git commit + failing quality.sh → exit 1 (blocks commit) --- +# --- Case 8: empty payload → quiet passthrough --- +# Nothing to inspect, and no real invocation looks like this. Warning here +# would cry wolf on every command. setup_sandbox -mkdir -p scripts -cat > scripts/quality.sh <<'EOF' -#!/usr/bin/env bash -echo "phpcs found 1 error" >&2 -exit 1 -EOF -chmod +x scripts/quality.sh -run_hook "pre-commit.sh" '{"tool_input":{"command":"git commit -m bad"}}' -assert_exit_code "git commit, quality fail → exit 1" 1 "$HOOK_EXIT" -assert_contains "git commit, quality fail → block message" \ - "$HOOK_STDERR_TEXT" "Commit blocked" +run_hook "pre-commit.sh" "" +assert_exit_code "empty payload → exit 0" 0 "$HOOK_EXIT" +assert_empty "empty payload → no stderr noise" "$HOOK_STDERR_TEXT" teardown_sandbox exit $FAIL_COUNT