From 363574d687127bc251375905781913ff86af0878 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 27 Jul 2026 01:43:46 +0200 Subject: [PATCH] fix(config): pre-push names which check failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook incremented a bare counter at 13 sites and ended with "N check(s) failed. Fix the errors above." — naming nothing. Six of those sites (fmt, clippy, cargo test, deny, ruff, tooling) print no FAIL line at all, so a failure was only inferable from the ABSENCE of an "— OK" line. Hit for real today: a push aborted on cargo fmt, and the verdict was indistinguishable from any other failure. Finding the cause meant scrolling past thousands of lines of unrelated test-fixture output, because the one actionable line said only that something, somewhere, had failed. Failed checks are now collected by name and printed in a self-contained final block, so tailing the log always shows WHAT broke — plus a pointer to grep the failing check's own output, and the reminder that fmt auto-fixes. Note this is NOT a verbosity reduction, which was the tempting fix. Detail is exactly what you want when something fails; the defect was that the verdict carried no information, not that the log carried too much. Co-Authored-By: Claude --- .config/hooks/pre-push | 57 +++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 15 deletions(-) diff --git a/.config/hooks/pre-push b/.config/hooks/pre-push index f3e234f0e..ecfb6e18e 100755 --- a/.config/hooks/pre-push +++ b/.config/hooks/pre-push @@ -5,6 +5,27 @@ set -euo pipefail REPO_ROOT="$(git rev-parse --show-toplevel)" ERRORS=0 +FAILED_CHECKS="" + +# Record a failed check BY NAME, not just as a tally. +# +# Why (2026-07-27): this hook used to increment a bare counter at 13 sites and +# end with "N check(s) failed. Fix the errors above." — which named nothing. Six +# of those sites (fmt, clippy, cargo test, deny, ruff, tooling) print no FAIL +# line at all, so a failure was only inferable from the ABSENCE of an "— OK". +# A real push aborted on `cargo fmt` and the verdict was indistinguishable from +# any other failure; finding it meant scrolling past thousands of lines of +# unrelated test-fixture output. +# +# The consequence that matters for tooling: the final block is now +# self-contained, so `tail` on this log always shows WHAT failed. Verbosity was +# never the problem — detail is exactly what you want when something breaks; +# the problem was that the verdict didn't say anything actionable. +fail_check() { + ERRORS=$((ERRORS + 1)) + FAILED_CHECKS="${FAILED_CHECKS} - $1 +" +} echo "pre-push: running lint checks..." @@ -45,7 +66,7 @@ elif command -v "$GODOT" >/dev/null 2>&1 && [ -d "$REPO_ROOT/client/.godot" ]; t 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)) + fail_check "GDScript parse (startup)" else echo "pre-push: GDScript parse — OK" fi @@ -92,7 +113,7 @@ if [ "$CLIENT_CHANGED" -gt 0 ] && [ -x "$REPO_ROOT/tooling/godot-parse-sweep" ]; echo "pre-push: sweeping GDScript parse (tooling/godot-parse-sweep)..." if ! "$REPO_ROOT/tooling/godot-parse-sweep"; then echo "pre-push: parse sweep FAILED — a script does not parse" - ERRORS=$((ERRORS + 1)) + fail_check "GDScript parse sweep" else echo "pre-push: parse sweep — OK" fi @@ -106,7 +127,7 @@ if [ "$CLIENT_CHANGED" -gt 0 ] && [ -x "$REPO_ROOT/tests/run-godot" ]; then echo "pre-push: running client test suite (tests/run-godot)..." if ! "$REPO_ROOT/tests/run-godot"; then echo "pre-push: client test suite FAILED" - ERRORS=$((ERRORS + 1)) + fail_check "client test suite (gdUnit4)" else echo "pre-push: client tests — OK" fi @@ -119,7 +140,7 @@ 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)) + fail_check "cargo fmt" else echo "pre-push: fmt — OK" fi @@ -130,7 +151,7 @@ elif command -v cargo >/dev/null 2>&1 && [ -d "$REPO_ROOT/server" ]; then if [ -d "$REPO_ROOT/server/target" ]; then echo "pre-push: checking Rust (clippy)..." if ! (cd "$REPO_ROOT/server" && cargo clippy --all-targets -- -D warnings 2>&1); then - ERRORS=$((ERRORS + 1)) + fail_check "cargo clippy" else echo "pre-push: clippy — OK" fi @@ -141,7 +162,7 @@ elif command -v cargo >/dev/null 2>&1 && [ -d "$REPO_ROOT/server" ]; then # guard with clippy so a cold worktree isn't forced into a full build. echo "pre-push: checking Rust (cargo test)..." if ! (cd "$REPO_ROOT/server" && cargo test --quiet 2>&1); then - ERRORS=$((ERRORS + 1)) + fail_check "cargo test" else echo "pre-push: cargo test — OK" fi @@ -153,7 +174,7 @@ elif command -v cargo >/dev/null 2>&1 && [ -d "$REPO_ROOT/server" ]; then 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)) + fail_check "cargo deny" else echo "pre-push: cargo deny — OK" fi @@ -168,7 +189,7 @@ if [ "$TOOLING_CHANGED" -eq 0 ]; then 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)) + fail_check "ruff (python lint)" else echo "pre-push: ruff — OK" fi @@ -185,7 +206,7 @@ if [ "$TOOLING_CHANGED" -eq 0 ]; then elif command -v make >/dev/null 2>&1; then echo "pre-push: running tooling tests (make test-tooling)..." if ! (cd "$REPO_ROOT" && make test-tooling); then - ERRORS=$((ERRORS + 1)) + fail_check "tooling tests (make test-tooling)" else echo "pre-push: tooling tests — OK" fi @@ -215,7 +236,7 @@ if [ -n "$JSON_FILES" ]; then done <<< "$JSON_FILES" if [ "$JSON_FAIL" -gt 0 ]; then echo "pre-push: FAIL — $JSON_FAIL JSON file(s) have syntax errors" - ERRORS=$((ERRORS + 1)) + fail_check "JSON syntax" else echo "pre-push: JSON — OK ($(echo "$JSON_FILES" | wc -l) file(s))" fi @@ -244,7 +265,7 @@ if [ "$DB_IN_PUSH" -gt 0 ] && [ -f "$REPO_ROOT/tooling/check-systems-db-stamp" ] # 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)) + fail_check "systems.db stamp (stale)" 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; @@ -275,14 +296,14 @@ if [ -x "$REPO_ROOT/tooling/clerk-review" ] && [ "${SR_RUN_CLERK:-0}" = "1" ]; t echo "pre-push: clerk — APPROVED" elif [ "$CLERK_VERDICT" = "REJECTED" ]; then echo "pre-push: clerk — REJECTED (see .cache/pre-push-review.md)" - ERRORS=$((ERRORS + 1)) + fail_check "clerk review (REJECTED)" 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)) + fail_check "clerk review (unrecognized verdict)" fi else echo "pre-push: clerk review — DISABLED (#965; force-run with SR_RUN_CLERK=1)" @@ -290,8 +311,14 @@ fi if [ "$ERRORS" -gt 0 ]; then echo "" - echo "pre-push: $ERRORS check(s) failed. Push aborted." - echo " Fix the errors above, then try again." + echo "==============================================================" + echo "pre-push: PUSH ABORTED — $ERRORS check(s) failed:" + printf '%s' "$FAILED_CHECKS" + echo "" + echo " Search the log above for the failing check's own output, e.g." + echo " grep -A20 'checking Rust (fmt)' " + echo " cargo fmt is auto-fixable: cd server && cargo fmt" + echo "==============================================================" exit 1 fi