fix(config): pre-push names which check failed
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 <noreply@anthropic.com>
This commit is contained in:
+42
-15
@@ -5,6 +5,27 @@ set -euo pipefail
|
|||||||
|
|
||||||
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
REPO_ROOT="$(git rev-parse --show-toplevel)"
|
||||||
ERRORS=0
|
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..."
|
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
|
if [ "$SCRIPT_ERRORS" -gt 0 ]; then
|
||||||
echo "pre-push: FAIL — $SCRIPT_ERRORS GDScript error(s) found"
|
echo "pre-push: FAIL — $SCRIPT_ERRORS GDScript error(s) found"
|
||||||
"$GODOT" --headless --path "$REPO_ROOT/client" --quit 2>&1 | grep -i "SCRIPT ERROR"
|
"$GODOT" --headless --path "$REPO_ROOT/client" --quit 2>&1 | grep -i "SCRIPT ERROR"
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "GDScript parse (startup)"
|
||||||
else
|
else
|
||||||
echo "pre-push: GDScript parse — OK"
|
echo "pre-push: GDScript parse — OK"
|
||||||
fi
|
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)..."
|
echo "pre-push: sweeping GDScript parse (tooling/godot-parse-sweep)..."
|
||||||
if ! "$REPO_ROOT/tooling/godot-parse-sweep"; then
|
if ! "$REPO_ROOT/tooling/godot-parse-sweep"; then
|
||||||
echo "pre-push: parse sweep FAILED — a script does not parse"
|
echo "pre-push: parse sweep FAILED — a script does not parse"
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "GDScript parse sweep"
|
||||||
else
|
else
|
||||||
echo "pre-push: parse sweep — OK"
|
echo "pre-push: parse sweep — OK"
|
||||||
fi
|
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)..."
|
echo "pre-push: running client test suite (tests/run-godot)..."
|
||||||
if ! "$REPO_ROOT/tests/run-godot"; then
|
if ! "$REPO_ROOT/tests/run-godot"; then
|
||||||
echo "pre-push: client test suite FAILED"
|
echo "pre-push: client test suite FAILED"
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "client test suite (gdUnit4)"
|
||||||
else
|
else
|
||||||
echo "pre-push: client tests — OK"
|
echo "pre-push: client tests — OK"
|
||||||
fi
|
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
|
# fmt only needs source files — always safe to run
|
||||||
echo "pre-push: checking Rust (fmt)..."
|
echo "pre-push: checking Rust (fmt)..."
|
||||||
if ! (cd "$REPO_ROOT/server" && cargo fmt --check 2>&1); then
|
if ! (cd "$REPO_ROOT/server" && cargo fmt --check 2>&1); then
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "cargo fmt"
|
||||||
else
|
else
|
||||||
echo "pre-push: fmt — OK"
|
echo "pre-push: fmt — OK"
|
||||||
fi
|
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
|
if [ -d "$REPO_ROOT/server/target" ]; then
|
||||||
echo "pre-push: checking Rust (clippy)..."
|
echo "pre-push: checking Rust (clippy)..."
|
||||||
if ! (cd "$REPO_ROOT/server" && cargo clippy --all-targets -- -D warnings 2>&1); then
|
if ! (cd "$REPO_ROOT/server" && cargo clippy --all-targets -- -D warnings 2>&1); then
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "cargo clippy"
|
||||||
else
|
else
|
||||||
echo "pre-push: clippy — OK"
|
echo "pre-push: clippy — OK"
|
||||||
fi
|
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.
|
# guard with clippy so a cold worktree isn't forced into a full build.
|
||||||
echo "pre-push: checking Rust (cargo test)..."
|
echo "pre-push: checking Rust (cargo test)..."
|
||||||
if ! (cd "$REPO_ROOT/server" && cargo test --quiet 2>&1); then
|
if ! (cd "$REPO_ROOT/server" && cargo test --quiet 2>&1); then
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "cargo test"
|
||||||
else
|
else
|
||||||
echo "pre-push: cargo test — OK"
|
echo "pre-push: cargo test — OK"
|
||||||
fi
|
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
|
if command -v cargo-deny >/dev/null 2>&1 && [ -f "$REPO_ROOT/server/deny.toml" ]; then
|
||||||
echo "pre-push: checking Rust (cargo deny)..."
|
echo "pre-push: checking Rust (cargo deny)..."
|
||||||
if ! (cd "$REPO_ROOT/server" && cargo deny check 2>&1); then
|
if ! (cd "$REPO_ROOT/server" && cargo deny check 2>&1); then
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "cargo deny"
|
||||||
else
|
else
|
||||||
echo "pre-push: cargo deny — OK"
|
echo "pre-push: cargo deny — OK"
|
||||||
fi
|
fi
|
||||||
@@ -168,7 +189,7 @@ if [ "$TOOLING_CHANGED" -eq 0 ]; then
|
|||||||
elif command -v ruff >/dev/null 2>&1 && [ -d "$REPO_ROOT/tooling" ]; then
|
elif command -v ruff >/dev/null 2>&1 && [ -d "$REPO_ROOT/tooling" ]; then
|
||||||
echo "pre-push: checking Python (ruff)..."
|
echo "pre-push: checking Python (ruff)..."
|
||||||
if ! (cd "$REPO_ROOT" && ruff check tooling/ 2>&1); then
|
if ! (cd "$REPO_ROOT" && ruff check tooling/ 2>&1); then
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "ruff (python lint)"
|
||||||
else
|
else
|
||||||
echo "pre-push: ruff — OK"
|
echo "pre-push: ruff — OK"
|
||||||
fi
|
fi
|
||||||
@@ -185,7 +206,7 @@ if [ "$TOOLING_CHANGED" -eq 0 ]; then
|
|||||||
elif command -v make >/dev/null 2>&1; then
|
elif command -v make >/dev/null 2>&1; then
|
||||||
echo "pre-push: running tooling tests (make test-tooling)..."
|
echo "pre-push: running tooling tests (make test-tooling)..."
|
||||||
if ! (cd "$REPO_ROOT" && make test-tooling); then
|
if ! (cd "$REPO_ROOT" && make test-tooling); then
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "tooling tests (make test-tooling)"
|
||||||
else
|
else
|
||||||
echo "pre-push: tooling tests — OK"
|
echo "pre-push: tooling tests — OK"
|
||||||
fi
|
fi
|
||||||
@@ -215,7 +236,7 @@ if [ -n "$JSON_FILES" ]; then
|
|||||||
done <<< "$JSON_FILES"
|
done <<< "$JSON_FILES"
|
||||||
if [ "$JSON_FAIL" -gt 0 ]; then
|
if [ "$JSON_FAIL" -gt 0 ]; then
|
||||||
echo "pre-push: FAIL — $JSON_FAIL JSON file(s) have syntax errors"
|
echo "pre-push: FAIL — $JSON_FAIL JSON file(s) have syntax errors"
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "JSON syntax"
|
||||||
else
|
else
|
||||||
echo "pre-push: JSON — OK ($(echo "$JSON_FILES" | wc -l) file(s))"
|
echo "pre-push: JSON — OK ($(echo "$JSON_FILES" | wc -l) file(s))"
|
||||||
fi
|
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
|
# rc=1 means stale / unknown generator / missing source; message on stderr
|
||||||
echo " Fix: run 'make regen-db' then stage server/data/systems.db"
|
echo " Fix: run 'make regen-db' then stage server/data/systems.db"
|
||||||
echo " Or use /pr-push — it handles regen automatically before pushing."
|
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
|
elif [ "$rc" -eq 2 ]; then
|
||||||
# rc=2 means no meta table — treat as unstamped, warn but don't block.
|
# rc=2 means no meta table — treat as unstamped, warn but don't block.
|
||||||
# This is legitimate immediately after the meta table is introduced;
|
# 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"
|
echo "pre-push: clerk — APPROVED"
|
||||||
elif [ "$CLERK_VERDICT" = "REJECTED" ]; then
|
elif [ "$CLERK_VERDICT" = "REJECTED" ]; then
|
||||||
echo "pre-push: clerk — REJECTED (see .cache/pre-push-review.md)"
|
echo "pre-push: clerk — REJECTED (see .cache/pre-push-review.md)"
|
||||||
ERRORS=$((ERRORS + 1))
|
fail_check "clerk review (REJECTED)"
|
||||||
elif [ "$CLERK_VERDICT" = "INCOMPLETE" ]; then
|
elif [ "$CLERK_VERDICT" = "INCOMPLETE" ]; then
|
||||||
echo "pre-push: clerk — INCOMPLETE (some reviews didn't finish; NOT blocking)"
|
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 " 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."
|
echo " or add a 'Clerk-Skip:' trailer to bulk-content commits."
|
||||||
else
|
else
|
||||||
echo "pre-push: clerk — '$CLERK_VERDICT' unrecognized; treating as block (see .cache/pre-push-review.md)"
|
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
|
fi
|
||||||
else
|
else
|
||||||
echo "pre-push: clerk review — DISABLED (#965; force-run with SR_RUN_CLERK=1)"
|
echo "pre-push: clerk review — DISABLED (#965; force-run with SR_RUN_CLERK=1)"
|
||||||
@@ -290,8 +311,14 @@ fi
|
|||||||
|
|
||||||
if [ "$ERRORS" -gt 0 ]; then
|
if [ "$ERRORS" -gt 0 ]; then
|
||||||
echo ""
|
echo ""
|
||||||
echo "pre-push: $ERRORS check(s) failed. Push aborted."
|
echo "=============================================================="
|
||||||
echo " Fix the errors above, then try again."
|
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)' <logfile>"
|
||||||
|
echo " cargo fmt is auto-fixable: cd server && cargo fmt"
|
||||||
|
echo "=============================================================="
|
||||||
exit 1
|
exit 1
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user