The per-commit clerk had three flaws, exposed by a 20-commit push where 6 of 7
rejections were false (incl. a CHANGELOG-only commit):
1. No-verdict / max-turns / timeout defaulted to REJECTED — an unfinished review
read as 'hard contradiction found'. Now a third outcome, INCOMPLETE, which is
non-blocking (the push proceeds with a warning); only a real REJECTED blocks.
2. Turn/time budget too tight (6 turns / 150s) for decision-heavy commits. Raised
defaults to 15 turns / 300s, and the prompt now biases to APPROVED when no
concrete contradiction is found ('unsure' means APPROVED, never REJECTED).
3. The skip valve matched its own feature commit because it scanned for the token
anywhere in the message. Moved to a trailer-line match so prose/subject mentions
no longer trip it.
Pre-push hook updated to treat INCOMPLETE as a non-blocking warning. The git-commit
skill documents the trailer form.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
225 lines
9.7 KiB
Bash
Executable File
225 lines
9.7 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Pre-push hook: lint GDScript and Rust before pushing.
|
|
# Installed via: git config core.hooksPath .config/hooks
|
|
set -euo pipefail
|
|
|
|
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
|
ERRORS=0
|
|
|
|
echo "pre-push: running lint checks..."
|
|
|
|
# --- Detect which directories have changes vs remote ---
|
|
# Prefer origin/<branch> as the baseline (what the remote already has),
|
|
# but fall back to origin/main for first-push of a new branch — otherwise
|
|
# every check runs against nothing and the hook treats the whole repo as
|
|
# changed, spending tens of seconds on linters and JSON validation that
|
|
# have no diff to cover (e.g. pushing a wiki-only branch rebuilds GDScript
|
|
# and runs clippy + ruff + validates all 2762 JSON files).
|
|
BRANCH=$(git branch --show-current)
|
|
if git rev-parse --verify "origin/$BRANCH" >/dev/null 2>&1; then
|
|
REMOTE_REF="origin/$BRANCH"
|
|
elif git rev-parse --verify "origin/main" >/dev/null 2>&1; then
|
|
REMOTE_REF="origin/main"
|
|
else
|
|
REMOTE_REF=""
|
|
fi
|
|
|
|
if [ -n "$REMOTE_REF" ]; then
|
|
CLIENT_CHANGED=$(git diff --name-only "$REMOTE_REF"..HEAD -- client/ 2>/dev/null | wc -l)
|
|
SERVER_CHANGED=$(git diff --name-only "$REMOTE_REF"..HEAD -- server/ 2>/dev/null | wc -l)
|
|
TOOLING_CHANGED=$(git diff --name-only "$REMOTE_REF"..HEAD -- tooling/ pyproject.toml 2>/dev/null | wc -l)
|
|
else
|
|
# No remote at all (e.g. fresh clone before first fetch) — be safe, run everything
|
|
CLIENT_CHANGED=1
|
|
SERVER_CHANGED=1
|
|
TOOLING_CHANGED=1
|
|
fi
|
|
|
|
# --- GDScript parse check (headless Godot) ---
|
|
GODOT="${GODOT:-godot}"
|
|
if [ "$CLIENT_CHANGED" -eq 0 ]; then
|
|
echo "pre-push: no client/ changes — skipping GDScript checks"
|
|
elif command -v "$GODOT" >/dev/null 2>&1 && [ -d "$REPO_ROOT/client/.godot" ]; then
|
|
echo "pre-push: checking GDScript (parse)..."
|
|
SCRIPT_ERRORS=$("$GODOT" --headless --path "$REPO_ROOT/client" --quit 2>&1 | grep -ci "SCRIPT ERROR" || true)
|
|
if [ "$SCRIPT_ERRORS" -gt 0 ]; then
|
|
echo "pre-push: FAIL — $SCRIPT_ERRORS GDScript error(s) found"
|
|
"$GODOT" --headless --path "$REPO_ROOT/client" --quit 2>&1 | grep -i "SCRIPT ERROR"
|
|
ERRORS=$((ERRORS + 1))
|
|
else
|
|
echo "pre-push: GDScript parse — OK"
|
|
fi
|
|
else
|
|
echo "pre-push: skipping GDScript parse (no .godot/ import — run Godot once to enable)"
|
|
fi
|
|
|
|
# --- GDScript lint (gdlint static analysis) — advisory only until codebase is clean ---
|
|
if [ "$CLIENT_CHANGED" -gt 0 ] && command -v gdlint >/dev/null 2>&1 && [ -d "$REPO_ROOT/client/scripts" ]; then
|
|
echo "pre-push: checking GDScript (gdlint — advisory)..."
|
|
LINT_COUNT=$(gdlint "$REPO_ROOT/client/scripts/" "$REPO_ROOT/client/ui/" 2>&1 | grep -c "Error:" || true)
|
|
if [ "$LINT_COUNT" -gt 0 ]; then
|
|
echo "pre-push: gdlint — $LINT_COUNT issue(s) (advisory, not blocking)"
|
|
else
|
|
echo "pre-push: gdlint — OK"
|
|
fi
|
|
fi
|
|
|
|
# --- GDScript format check (gdformat) — advisory only until codebase is clean ---
|
|
if [ "$CLIENT_CHANGED" -gt 0 ] && command -v gdformat >/dev/null 2>&1 && [ -d "$REPO_ROOT/client/scripts" ]; then
|
|
echo "pre-push: checking GDScript (gdformat — advisory)..."
|
|
FORMAT_COUNT=$(gdformat --check "$REPO_ROOT/client/scripts/" "$REPO_ROOT/client/ui/" 2>&1 | grep -c "would reformat" || true)
|
|
if [ "$FORMAT_COUNT" -gt 0 ]; then
|
|
echo "pre-push: gdformat — $FORMAT_COUNT file(s) need formatting (advisory, not blocking)"
|
|
else
|
|
echo "pre-push: gdformat — OK"
|
|
fi
|
|
fi
|
|
|
|
# --- Rust lint (clippy + fmt) ---
|
|
if [ "$SERVER_CHANGED" -eq 0 ]; then
|
|
echo "pre-push: no server/ changes — skipping Rust checks"
|
|
elif command -v cargo >/dev/null 2>&1 && [ -d "$REPO_ROOT/server" ]; then
|
|
# fmt only needs source files — always safe to run
|
|
echo "pre-push: checking Rust (fmt)..."
|
|
if ! (cd "$REPO_ROOT/server" && cargo fmt --check 2>&1); then
|
|
ERRORS=$((ERRORS + 1))
|
|
else
|
|
echo "pre-push: fmt — OK"
|
|
fi
|
|
|
|
# clippy needs a build — skip if target/ doesn't exist (cold worktree)
|
|
if [ -d "$REPO_ROOT/server/target" ]; then
|
|
echo "pre-push: checking Rust (clippy)..."
|
|
if ! (cd "$REPO_ROOT/server" && cargo clippy -- -D warnings 2>&1); then
|
|
ERRORS=$((ERRORS + 1))
|
|
else
|
|
echo "pre-push: clippy — OK"
|
|
fi
|
|
else
|
|
echo "pre-push: skipping clippy (no target/ — run 'cargo build' once to enable)"
|
|
fi
|
|
|
|
# --- Rust dependency audit (cargo deny) — requires deny.toml config ---
|
|
if command -v cargo-deny >/dev/null 2>&1 && [ -f "$REPO_ROOT/server/deny.toml" ]; then
|
|
echo "pre-push: checking Rust (cargo deny)..."
|
|
if ! (cd "$REPO_ROOT/server" && cargo deny check 2>&1); then
|
|
ERRORS=$((ERRORS + 1))
|
|
else
|
|
echo "pre-push: cargo deny — OK"
|
|
fi
|
|
fi
|
|
else
|
|
echo "pre-push: WARNING — cargo not found or server/ missing, skipping Rust lint"
|
|
fi
|
|
|
|
# --- Python lint (ruff) ---
|
|
if [ "$TOOLING_CHANGED" -eq 0 ]; then
|
|
echo "pre-push: no tooling/ changes — skipping Python lint"
|
|
elif command -v ruff >/dev/null 2>&1 && [ -d "$REPO_ROOT/tooling" ]; then
|
|
echo "pre-push: checking Python (ruff)..."
|
|
if ! (cd "$REPO_ROOT" && ruff check tooling/ 2>&1); then
|
|
ERRORS=$((ERRORS + 1))
|
|
else
|
|
echo "pre-push: ruff — OK"
|
|
fi
|
|
else
|
|
echo "pre-push: skipping Python lint (ruff not found — install with: pip install 'ruff>=0.9')"
|
|
fi
|
|
|
|
# --- JSON syntax validation ---
|
|
# Use the same REMOTE_REF the directory-change detection above settled on
|
|
# (origin/<branch> preferred, origin/main fallback). Without this, a first
|
|
# push of a new branch falls through to "validate every JSON in the repo,"
|
|
# which on this repo means 2762 Python parses — tens of seconds of churn
|
|
# for a push that might not have touched any JSON at all.
|
|
if [ -n "$REMOTE_REF" ]; then
|
|
JSON_FILES=$(git diff --name-only "$REMOTE_REF"..HEAD -- '*.json' 2>/dev/null || true)
|
|
else
|
|
JSON_FILES=$(git ls-files '*.json')
|
|
fi
|
|
if [ -n "$JSON_FILES" ]; then
|
|
echo "pre-push: checking JSON syntax..."
|
|
JSON_FAIL=0
|
|
while IFS= read -r f; do
|
|
if [ -f "$REPO_ROOT/$f" ] && ! python3 -m json.tool "$REPO_ROOT/$f" >/dev/null 2>&1; then
|
|
echo " FAIL: $f"
|
|
JSON_FAIL=$((JSON_FAIL + 1))
|
|
fi
|
|
done <<< "$JSON_FILES"
|
|
if [ "$JSON_FAIL" -gt 0 ]; then
|
|
echo "pre-push: FAIL — $JSON_FAIL JSON file(s) have syntax errors"
|
|
ERRORS=$((ERRORS + 1))
|
|
else
|
|
echo "pre-push: JSON — OK ($(echo "$JSON_FILES" | wc -l) file(s))"
|
|
fi
|
|
else
|
|
echo "pre-push: no JSON changes — skipping"
|
|
fi
|
|
|
|
# --- systems.db stamp check (#857) ---
|
|
# If the branch touches server/data/systems.db and the meta stamp does not
|
|
# match current generator sources, reject the push. Prevents pushing a
|
|
# stale DB snapshot where generator source was modified but the DB was not
|
|
# regenerated.
|
|
#
|
|
# Runs whenever systems.db was modified in ANY branch commit vs. main —
|
|
# including on a branch's very first push (review T5: the previous version
|
|
# skipped the check for new branches because it compared against origin/$BRANCH,
|
|
# which didn't exist yet, leaving a gap where a stale DB could ship via the
|
|
# first push). We compare against origin/main — which always exists — so the
|
|
# check covers the first-push case.
|
|
DB_IN_PUSH=$(git diff --name-only origin/main...HEAD -- server/data/systems.db 2>/dev/null | wc -l)
|
|
if [ "$DB_IN_PUSH" -gt 0 ] && [ -f "$REPO_ROOT/tooling/check-systems-db-stamp" ]; then
|
|
echo "pre-push: checking systems.db stamp..."
|
|
rc=0
|
|
python3 "$REPO_ROOT/tooling/check-systems-db-stamp" || rc=$?
|
|
if [ "$rc" -eq 1 ]; then
|
|
# rc=1 means stale / unknown generator / missing source; message on stderr
|
|
echo " Fix: run 'make regen-db' then stage server/data/systems.db"
|
|
echo " Or use /pr-push — it handles regen automatically before pushing."
|
|
ERRORS=$((ERRORS + 1))
|
|
elif [ "$rc" -eq 2 ]; then
|
|
# rc=2 means no meta table — treat as unstamped, warn but don't block.
|
|
# This is legitimate immediately after the meta table is introduced;
|
|
# the next `make regen-db` will populate it (H4).
|
|
echo "pre-push: WARNING — systems.db has no meta stamp — run 'make regen-db' now if this DB was generated by you"
|
|
else
|
|
echo "pre-push: systems.db stamp — OK"
|
|
fi
|
|
else
|
|
echo "pre-push: systems.db not in push — skipping stamp check"
|
|
fi
|
|
|
|
# --- Clerk review (D-221) ---
|
|
# Spawns the clerk agent to check D-record consistency against the diff.
|
|
# Optional: only runs if tooling/clerk-review exists and is executable.
|
|
# Set SR_SKIP_CLERK=1 to bypass (e.g. for trivial doc-only pushes).
|
|
if [ -x "$REPO_ROOT/tooling/clerk-review" ] && [ "${SR_SKIP_CLERK:-0}" != "1" ]; then
|
|
echo "pre-push: running clerk review..."
|
|
CLERK_VERDICT=$("$REPO_ROOT/tooling/clerk-review" 2>&1 | tee /dev/stderr | tail -1)
|
|
if [ "$CLERK_VERDICT" = "APPROVED" ]; then
|
|
echo "pre-push: clerk — APPROVED"
|
|
elif [ "$CLERK_VERDICT" = "REJECTED" ]; then
|
|
echo "pre-push: clerk — REJECTED (see .cache/pre-push-review.md)"
|
|
ERRORS=$((ERRORS + 1))
|
|
elif [ "$CLERK_VERDICT" = "INCOMPLETE" ]; then
|
|
echo "pre-push: clerk — INCOMPLETE (some reviews didn't finish; NOT blocking)"
|
|
echo " See .cache/pre-push-review.md. For a full verdict: raise SR_CLERK_MAX_TURNS / SR_CLERK_TIMEOUT,"
|
|
echo " or add a 'Clerk-Skip:' trailer to bulk-content commits."
|
|
else
|
|
echo "pre-push: clerk — '$CLERK_VERDICT' unrecognized; treating as block (see .cache/pre-push-review.md)"
|
|
ERRORS=$((ERRORS + 1))
|
|
fi
|
|
else
|
|
echo "pre-push: clerk review — skipped (not installed or SR_SKIP_CLERK=1)"
|
|
fi
|
|
|
|
if [ "$ERRORS" -gt 0 ]; then
|
|
echo ""
|
|
echo "pre-push: $ERRORS check(s) failed. Push aborted."
|
|
echo " Fix the errors above, then try again."
|
|
exit 1
|
|
fi
|
|
|
|
echo "pre-push: all checks passed."
|