fix(hooks): the pre-commit gate was failing open, twice

.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
 

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

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 <exit-code>
+  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