Skip to content

Commit ace8716

Browse files
Jamesclaude
authored and
James
committed
Catch a drifting parser bump before it is pushed, and notice a red master
Two gaps that afb6f3a exposed, neither of them a missing check. The drift was caught by CI within the same minute as the push. It then sat red for 21 hours, through a second failing run, because nobody loads the Actions tab of a repository that is usually green. .githooks/pre-commit refuses a commit whose staged POMs disagree, so the mistake never leaves the machine. Opt in per clone with `git config core.hooksPath .githooks`; bypass with --no-verify. It checks the index rather than the working tree, because staging pom.xml while leaving the connector edits unstaged is exactly the drift being guarded against. red-master.yml opens one issue the first time a watched workflow fails on master, assigns it to whoever triggered the run, comments on later failures rather than opening more issues, and closes it once the newest completed run of every watched workflow is green. One green run is not enough: build.yml can pass while the nightly is still failing. The hook fails open -- no python, no readable index, no POM staged and it stands aside -- so a hook that quietly stopped working would be invisible until the next bad bump. test-pre-commit-hook.sh drives it through four cases in a throwaway clone and asserts on what it does, and build.yml runs that. Wire new things into CI in the same commit. Also corrects the stale test count in the README's CI table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PtAbQJuygzhHiveuCxCPnT
1 parent 7714794 commit ace8716

7 files changed

Lines changed: 500 additions & 3 deletions

File tree

‎.githooks/pre-commit‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Refuse a commit that would leave the parser version disagreeing across the
4+
# four POMs that write it down.
5+
#
6+
# This is the same check both workflows run, moved earlier. CI already catches
7+
# the drift -- when it last happened, on 2026-08-10, the push build went red
8+
# within the same minute as the commit -- but nobody read that build for 21
9+
# hours, and the nightly failed too before anyone noticed. A hook cannot be
10+
# ignored for 21 hours: it costs about 50ms and it runs before the mistake
11+
# leaves the machine.
12+
#
13+
# Install it (once per clone -- .git/hooks is not versioned, so this is the
14+
# only way a hook can ship in the repository):
15+
#
16+
# git config core.hooksPath .githooks
17+
#
18+
# Bypass it, if you are deliberately committing a half-done bump:
19+
#
20+
# git commit --no-verify
21+
#
22+
# It checks the *index*, not the working tree, because the index is what you
23+
# are about to commit. Staging pom.xml while leaving the matching
24+
# connector/*/pom.xml edits unstaged is precisely the drift being guarded
25+
# against, and a working-tree check would call that clean.
26+
27+
set -uo pipefail
28+
29+
# Nothing to say about a commit that touches no POM. Keeps the hook free for
30+
# the other 99% of commits.
31+
if ! git diff --cached --name-only --diff-filter=ACMR | grep -qE '(^|/)pom\.xml$'; then
32+
exit 0
33+
fi
34+
35+
# set-parser-version.sh needs python. Warn rather than block if it is missing:
36+
# a hook that refuses every commit on a machine without python would just get
37+
# uninstalled, and CI still covers this.
38+
if ! command -v python3 >/dev/null 2>&1 && ! command -v python >/dev/null 2>&1; then
39+
echo "pre-commit: python not found, skipping the parser version check" >&2
40+
exit 0
41+
fi
42+
43+
tmp=$(mktemp -d) || exit 0
44+
trap 'rm -rf "$tmp"' EXIT
45+
46+
# Extract the staged content of every POM plus the checker itself, then run
47+
# that copy against that copy: the script resolves its targets relative to its
48+
# own location, so it sees the index and nothing else.
49+
# A plain loop rather than mapfile: macOS still ships bash 3.2.
50+
staged_files=()
51+
while IFS= read -r f; do
52+
staged_files+=("$f")
53+
done < <(git ls-files -- '*pom.xml' '.github/scripts/set-parser-version.sh')
54+
if [ ${#staged_files[@]} -eq 0 ]; then
55+
exit 0
56+
fi
57+
if ! git checkout-index -f --prefix="$tmp/" -- "${staged_files[@]}" 2>/dev/null; then
58+
echo "pre-commit: could not read the index, skipping the parser version check" >&2
59+
exit 0
60+
fi
61+
62+
checker="$tmp/.github/scripts/set-parser-version.sh"
63+
[ -x "$checker" ] || exit 0
64+
65+
# The script prints ::error:: annotations, which are for GitHub's log viewer and
66+
# read as noise in a terminal.
67+
if output=$("$checker" --check 2>&1); then
68+
exit 0
69+
fi
70+
71+
sed 's/^::error:://' <<<"$output" >&2
72+
cat >&2 <<'EOF'
73+
74+
pre-commit: the parser version disagrees across the POMs you are committing.
75+
76+
.github/scripts/set-parser-version.sh <version> move all four together
77+
git add -u && git commit then commit again
78+
79+
Never edit a version by hand: it is written in pom.xml and in each of the three
80+
connector/*/pom.xml, which are separate builds with no parent to inherit it.
81+
In practice you should not be editing one at all -- the nightly tests the newest
82+
release and opens a pre-verified bump PR.
83+
84+
git commit --no-verify commit anyway
85+
EOF
86+
exit 1

‎.github/scripts/red-master-issue.sh‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Open, update, and close the one GitHub issue that tracks "master is red".
4+
#
5+
# Why this exists: on 2026-08-10 a commit bumped the parser version in pom.xml
6+
# and missed the three connector POMs. CI caught it *within the same minute* --
7+
# and it stayed broken for 21 hours, through a second failing run, because a red
8+
# build on a repository nobody is actively watching is a grey checkmark in a list
9+
# that nobody loads. The check was never the weak part; noticing was.
10+
#
11+
# So a failure gets something that follows you: an issue, assigned to whoever
12+
# triggered the run, which stays open until master is actually green again and
13+
# closes itself when it is.
14+
#
15+
# Called by .github/workflows/red-master.yml. Reads its input from the
16+
# environment so the workflow_run payload and a manual dry run can both drive it:
17+
#
18+
# CONCLUSION success | failure | cancelled | ...
19+
# BRANCH branch the run was on; anything but master is ignored
20+
# RUN_NAME display name of the workflow that finished
21+
# RUN_URL link to it
22+
# RUN_ID its id, used to avoid commenting twice about one run
23+
# SHA commit it ran against
24+
# ACTOR who to assign; best effort, a non-assignable login is not fatal
25+
# DRY_RUN 1 to print every mutation instead of making it
26+
#
27+
# Requires gh, authenticated with issues:write and actions:read.
28+
29+
set -uo pipefail
30+
31+
LABEL="ci-red"
32+
TITLE="CI is red on master"
33+
# The workflows whose result decides whether master is green. Must match the
34+
# `name:` of each, and the list in red-master.yml's `workflows:` trigger.
35+
WATCHED=("Build and test" "Nightly")
36+
37+
CONCLUSION="${CONCLUSION:-}"
38+
BRANCH="${BRANCH:-}"
39+
RUN_NAME="${RUN_NAME:-a workflow}"
40+
RUN_URL="${RUN_URL:-}"
41+
RUN_ID="${RUN_ID:-}"
42+
SHA="${SHA:-}"
43+
ACTOR="${ACTOR:-}"
44+
DRY_RUN="${DRY_RUN:-0}"
45+
46+
say() { printf '%s\n' "$*"; }
47+
48+
# Every mutation goes through here, so --dry-run cannot half-apply.
49+
run() {
50+
if [ "$DRY_RUN" = "1" ]; then
51+
say "DRY RUN would: $*"
52+
return 0
53+
fi
54+
"$@"
55+
}
56+
57+
if [ "$BRANCH" != "master" ]; then
58+
say "run was on '$BRANCH', not master; nothing to do"
59+
exit 0
60+
fi
61+
62+
short_sha="${SHA:0:7}"
63+
64+
# The open tracker, if there is one. `gh issue list` filters to open by default;
65+
# being explicit because this decides whether we create or comment.
66+
existing=$(gh issue list --label "$LABEL" --state open --limit 1 \
67+
--json number --jq '.[0].number // empty' 2>/dev/null)
68+
69+
case "$CONCLUSION" in
70+
failure|timed_out)
71+
if [ -z "$existing" ]; then
72+
# The label may not exist yet on a fresh repository. --force makes this
73+
# idempotent instead of failing the second time.
74+
run gh label create "$LABEL" \
75+
--color B60205 \
76+
--description "master is failing CI" \
77+
--force >/dev/null 2>&1 || true
78+
79+
body="**\`$RUN_NAME\` failed on \`master\`.**
80+
81+
- commit: \`$short_sha\`
82+
- run: $RUN_URL
83+
84+
This issue is opened automatically by [\`red-master.yml\`](https://github.com/${GITHUB_REPOSITORY:-sqlparser/gsp_demo_java}/blob/master/.github/workflows/red-master.yml) the first time a run fails on \`master\`, and closes itself once every watched workflow is green again. Later failures are added as comments rather than as new issues.
85+
86+
Please do not close it by hand while master is still red -- a closed issue is how the last one went unnoticed for 21 hours."
87+
88+
if [ "$DRY_RUN" = "1" ]; then
89+
say "DRY RUN would: create issue '$TITLE' (label $LABEL, assignee ${ACTOR:-none})"
90+
say "--- body ---"
91+
say "$body"
92+
say "------------"
93+
else
94+
# `gh issue create` prints the new issue's URL; there is no --json
95+
# on it. Assign in a second call: an actor who is not a repository
96+
# collaborator cannot be assigned, and that must not cost us the
97+
# issue itself.
98+
num=$(gh issue create --title "$TITLE" --label "$LABEL" --body "$body" \
99+
| sed -n 's|.*/issues/\([0-9][0-9]*\).*|\1|p')
100+
if [ -z "$num" ]; then
101+
say "::error::could not create the tracking issue"
102+
exit 1
103+
fi
104+
say "opened #$num"
105+
if [ -n "$ACTOR" ] && [[ "$ACTOR" != *"[bot]" ]]; then
106+
gh issue edit "$num" --add-assignee "$ACTOR" >/dev/null 2>&1 \
107+
&& say "assigned to @$ACTOR" \
108+
|| say "could not assign @$ACTOR (not a collaborator?); left unassigned"
109+
fi
110+
fi
111+
else
112+
# One comment per failing run, not per re-read of the same one.
113+
if [ -n "$RUN_ID" ] && gh issue view "$existing" --json comments \
114+
--jq '.comments[].body' 2>/dev/null | grep -qF "/runs/$RUN_ID"; then
115+
say "#$existing already mentions run $RUN_ID; not commenting twice"
116+
exit 0
117+
fi
118+
run gh issue comment "$existing" --body \
119+
"Still red: **\`$RUN_NAME\`** failed on \`$short_sha\`.
120+
121+
$RUN_URL"
122+
say "commented on #$existing"
123+
fi
124+
;;
125+
126+
success)
127+
if [ -z "$existing" ]; then
128+
say "master is green and no tracking issue is open; nothing to do"
129+
exit 0
130+
fi
131+
132+
# One green run does not mean master is green: build.yml can pass while the
133+
# nightly is still failing. Close only when the newest completed run of
134+
# every watched workflow succeeded.
135+
still_red=""
136+
for wf in "${WATCHED[@]}"; do
137+
latest=$(gh run list --workflow "$wf" --branch master --status completed \
138+
--limit 1 --json conclusion --jq '.[0].conclusion // empty' 2>/dev/null)
139+
say " latest completed '$wf' on master: ${latest:-none}"
140+
case "$latest" in
141+
""|success|skipped|neutral) ;;
142+
*) still_red="$still_red $wf" ;;
143+
esac
144+
done
145+
146+
if [ -n "$still_red" ]; then
147+
say "leaving #$existing open;$still_red is still failing"
148+
exit 0
149+
fi
150+
151+
run gh issue comment "$existing" --body \
152+
"Green again: every watched workflow passed on \`$short_sha\`.
153+
154+
$RUN_URL"
155+
run gh issue close "$existing" --reason completed
156+
say "closed #$existing"
157+
;;
158+
159+
*)
160+
say "conclusion '$CONCLUSION' is neither success nor failure; ignoring"
161+
;;
162+
esac

‎.github/scripts/test-pre-commit-hook.sh‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
#!/usr/bin/env bash
2+
#
3+
# Exercise .githooks/pre-commit against a throwaway clone.
4+
#
5+
# The hook exists to stop a parser-version bump that misses one of the four
6+
# POMs. A hook that silently stopped working would be invisible -- it fails
7+
# open by design, so the only symptom is that a bad commit sails through
8+
# months later. This drives it through four cases and asserts on what it
9+
# actually does:
10+
#
11+
# 1. a consistent POM edit -> commit succeeds
12+
# 2. a drifting POM edit -> commit is refused
13+
# 3. a commit touching no POM -> hook stays out of the way
14+
# 4. --no-verify on a drifting edit -> commit succeeds (the documented escape)
15+
#
16+
# Case 2 is the point; case 1 is what stops it from being a hook that just
17+
# refuses everything, and case 4 keeps the documented bypass honest.
18+
19+
set -uo pipefail
20+
21+
cd "$(dirname "${BASH_SOURCE[0]}")/../.." || exit 1
22+
SRC=$(pwd)
23+
24+
FAILED=0
25+
TMP=$(mktemp -d)
26+
trap 'rm -rf "$TMP"' EXIT
27+
28+
ok() { printf ' ok %s\n' "$1"; }
29+
fail() { printf '::error::%s\n' "$1"; FAILED=$((FAILED + 1)); }
30+
31+
# A clone rather than the checkout itself: these cases commit, and no test
32+
# should be able to write to the tree it is testing.
33+
git clone --quiet --no-hardlinks "$SRC" "$TMP/repo" || {
34+
echo "::error::could not clone the repository"
35+
exit 1
36+
}
37+
cd "$TMP/repo" || exit 1
38+
git config user.name "hook test"
39+
git config user.email "hook-test@example.invalid"
40+
git config commit.gpgsign false
41+
git config core.hooksPath .githooks
42+
43+
if [ ! -x .githooks/pre-commit ]; then
44+
echo "::error::.githooks/pre-commit is missing or not executable in the committed tree"
45+
echo " (git tracks the exec bit: chmod +x it and commit the mode change)"
46+
exit 1
47+
fi
48+
49+
version=$(.github/scripts/set-parser-version.sh --check | sed -n 's/^consistent: //p')
50+
if [ -z "$version" ]; then
51+
echo "::error::the checked-out tree is already inconsistent; fix that before testing the hook"
52+
.github/scripts/set-parser-version.sh --check
53+
exit 1
54+
fi
55+
echo "testing the pre-commit hook against a clone at parser $version"
56+
echo
57+
58+
# ---------------------------------------------------------------- case 1
59+
# A POM edit that keeps all four in agreement must not be blocked.
60+
printf '\n<!-- hook test: consistent edit -->\n' >> pom.xml
61+
git add pom.xml
62+
if git commit --quiet -m "consistent POM edit" >/dev/null 2>&1; then
63+
ok "a consistent POM edit commits"
64+
else
65+
fail "the hook blocked a consistent POM edit"
66+
fi
67+
68+
# ---------------------------------------------------------------- case 2
69+
# The real case: bump the root property and leave the connectors behind, which
70+
# is exactly what afb6f3a did.
71+
sed -i.bak "s|<gsp\.core\.version>$version</gsp\.core\.version>|<gsp.core.version>9.9.9</gsp.core.version>|" pom.xml
72+
rm -f pom.xml.bak
73+
if ! grep -q '9\.9\.9' pom.xml; then
74+
fail "could not stage a drifting edit -- the property is not where this test expects it"
75+
else
76+
git add pom.xml
77+
out=$(git commit -m "drifting POM edit" 2>&1)
78+
if [ $? -eq 0 ]; then
79+
fail "the hook let a drifting parser version through"
80+
elif grep -q "disagrees across files" <<<"$out"; then
81+
ok "a drifting POM edit is refused, and says why"
82+
else
83+
fail "the commit was refused but not by the version check: $out"
84+
fi
85+
fi
86+
87+
# ---------------------------------------------------------------- case 4
88+
# The documented escape hatch, tested while the index is still drifting.
89+
if git commit --quiet --no-verify -m "drifting POM edit, bypassed" >/dev/null 2>&1; then
90+
ok "--no-verify commits anyway"
91+
else
92+
fail "--no-verify did not bypass the hook"
93+
fi
94+
git reset --quiet --hard HEAD~1 # back to the consistent commit from case 1
95+
96+
# ---------------------------------------------------------------- case 3
97+
# A commit touching no POM must not pay for the check, or run it at all.
98+
printf '\nhook test\n' >> README.md
99+
git add README.md
100+
if git commit --quiet -m "a commit touching no POM" >/dev/null 2>&1; then
101+
ok "a commit touching no POM is left alone"
102+
else
103+
fail "the hook blocked a commit that touches no POM"
104+
fi
105+
106+
echo
107+
if [ "$FAILED" -gt 0 ]; then
108+
echo "::error::pre-commit hook: $FAILED case(s) failed"
109+
exit 1
110+
fi
111+
echo "pre-commit hook: all 4 cases pass"

‎.github/workflows/build.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,15 @@ jobs:
4747
- name: Parser version is consistent across all POMs
4848
run: .github/scripts/set-parser-version.sh --check
4949

50+
# The same check as a pre-commit hook, so the drift never reaches a push
51+
# in the first place. It is opt-in per clone (`git config core.hooksPath
52+
# .githooks`), which means nothing would notice if it quietly stopped
53+
# working -- it fails open by design. So CI drives it: a clone, a
54+
# deliberately drifting bump, and an assertion that the commit is refused.
55+
- name: The pre-commit hook still catches a drifting bump
56+
if: matrix.java == '21'
57+
run: .github/scripts/test-pre-commit-hook.sh
58+
5059
- name: Build
5160
run: mvn -B package -DskipTests
5261

0 commit comments

Comments
 (0)