Skip to content

fix(auto-merge): wait for the verification checks to appear, not just to finish #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
forkwright merged 1 commit into from
Aug 21, 2026
Merged

fix(auto-merge): wait for the verification checks to appear, not just to finish #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 96 additions & 39 deletions .github/workflows/dependabot-auto-merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,51 +34,108 @@ jobs:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
set -euo pipefail
if ! gh pr checks "$PR_URL" --watch --interval 30 --required; then
echo "Required CI checks did not pass; refusing auto-merge." >&2
exit 1
fi
failed=0
# WHY match a normalised substring rather than an exact suffix: this
# file's header used to assert that required-check names are
# fleet-invariant. Measured, they are not -- the same OSV job is named
# `osv scanner` in one repo and `osv-scanner` in three others, and
# reusable-workflow checks arrive prefixed as "caller-job / leaf-job".
# `endswith("osv-scan")` matches neither `osv-scanner` (it ends in
# "scanner") nor `osv scanner`, so three repos could not adopt this
# workflow at all: it would refuse every PR whose OSV check had
# actually reported and passed. Stripping every non-alphanumeric from
# both sides and testing containment matches the whole spelling class
# instead of enumerating the two spellings seen so far -- an invariance
# asserted in a comment and enforced nowhere will drift again.

# Token groups in NORMALISED form: lowercase, alphanumerics only. A group is satisfied by
# any one of its spellings, because the same job is named differently across repos.
#
# WHY match a normalised substring rather than an exact suffix: this file's header used to
# assert that required-check names are fleet-invariant. Measured, they are not -- the same
# OSV job is named `osv scanner` in one repo and `osv-scanner` in three others, and
# reusable-workflow checks arrive prefixed as "caller-job / leaf-job". `endswith("osv-scan")`
# matches neither, so three repos could not adopt this workflow at all. Stripping every
# non-alphanumeric from both sides and testing containment matches the whole spelling class
# instead of enumerating the two spellings seen so far.
# WARNING: this array must NOT be named `GROUPS`. Bash maintains `GROUPS` itself as the
# current user's supplementary group IDs, and an assignment to it is silently ignored --
# the loop then iterates numeric GIDs, matches no check, and refuses every PR. That is the
# same always-refuse symptom this file is being fixed for, reached a different way, and it
# is invisible without RUNNING the script: the assignment raises no error. Caught by
# driving this step against synthetic check snapshots before it ever reached a runner.
CHECK_GROUPS=("gateattestation gategate" "cargodeny" "cargoaudit" "osvscan")

# WHY this polls for the groups to APPEAR instead of calling `gh pr checks --watch`:
# `--watch` waits only on the checks that exist WHEN WATCHING STARTS. On a fresh dependabot
# PR the fast checks report first, so `--watch --required` returned as soon as `cargo audit`
# and `cargo deny` passed while `gate / gate` was still building. Verification then ran
# against a check list that did not contain the gate yet and took its "no matching check"
# branch -- refusing a PR whose gate went on to pass green on the same head.
#
# The irony is the whole reason this file is preferred over branch protection: its
# distinguishing feature is noticing when a real verification check fails to report AT ALL,
# and it was evaluating that at a moment when a slow check legitimately had not reported.
# Gate Attestation trips it everywhere, because compiling the workspace makes it reliably
# the slowest -- the check most worth waiting for is the one most likely to be missed.
# See forkwright/.github#46.
#
# WHY every match must pass, rather than the first one found: looser
# matching means a token can select several reported checks, and
# picking `[0]` of those made the verdict depend on the order GitHub
# happened to return them -- a superseded run could answer for a live
# one. Requiring all of them removes that arbitrariness and is strictly
# the safer direction for a guard whose failure mode is auto-merging.
require_passed_check() {
# WHY a timeout that FAILS rather than falls through: a group that never reports is exactly
# the condition this guard exists to catch, so exhausting the wait must refuse. Falling
# through on timeout would turn the fix into the bug it replaces, in the unsafe direction.
WAIT_SECONDS="${AUTO_MERGE_WAIT_SECONDS:-5400}"
# WHY the poll interval is overridable: with it fixed at 30s, the only way to exercise the
# wait loop is to wait, so `tests/dependabot-auto-merge.sh` could not reach a third
# snapshot and reported the late-arriving-gate case as a refusal. A step that cannot be
# run faster than production cannot be tested, and this one is shell inside YAML that
# nothing else checks. Nothing in CI sets it.
POLL_SECONDS="${AUTO_MERGE_POLL_SECONDS:-30}"
deadline=$(( SECONDS + WAIT_SECONDS ))

# WHY one fetch per poll, reused across every group: the previous version called
# `gh pr checks` once PER TOKEN, so the four groups were answered from four different
# snapshots taken seconds apart. That is a second instance of the same race -- a check could
# be pending for one group's read and terminal for the next, and no single consistent view
# of the PR was ever evaluated.
snapshot=""
group_state() {
# Echoes: MISSING | PENDING | <space-separated terminal buckets>
local leaf buckets
for leaf in "$@"; do
buckets=$(gh pr checks "$PR_URL" --json name,bucket | jq -r --arg leaf "$leaf" \
for leaf in $1; do
buckets=$(printf '%s' "$snapshot" | jq -r --arg leaf "$leaf" \
'[.[] | select((.name | ascii_downcase | gsub("[^a-z0-9]"; "")) | contains($leaf)) | .bucket] | join(" ")')
[ -n "$buckets" ] || continue
for b in $buckets; do
if [ "$b" != "pass" ]; then
echo "::error::Verification check matching '${leaf}' finished in bucket '${b}'."
failed=1
fi
done
return 0
case " $buckets " in
*" pending "*) echo "PENDING"; return 0 ;;
esac
echo "$buckets"; return 0
done
echo "::error::No required verification check matching any of: $*."; failed=1
echo "MISSING"
}
# Tokens are the normalised form: lowercase, alphanumerics only.
require_passed_check "gateattestation" "gategate"
require_passed_check "cargodeny"
require_passed_check "cargoaudit"
require_passed_check "osvscan"

while :; do
snapshot=$(gh pr checks "$PR_URL" --json name,bucket)
waiting=""
for group in "${CHECK_GROUPS[@]}"; do
state=$(group_state "$group")
case "$state" in
MISSING|PENDING) waiting="${waiting}${group} (${state})\n" ;;
esac
done
[ -z "$waiting" ] && break
if [ "$SECONDS" -ge "$deadline" ]; then
printf '::error::Verification checks never reached a terminal state within %ss:\n%b' \
"$WAIT_SECONDS" "$waiting" >&2
echo "A check that never reports is the condition this guard exists to catch; refusing auto-merge." >&2
exit 1
fi
printf 'Waiting on:\n%b' "$waiting"
sleep "$POLL_SECONDS"
done

# Every group is present and terminal. Now judge it.
#
# WHY every match must pass, rather than the first one found: looser matching means a token
# can select several reported checks, and picking `[0]` of those made the verdict depend on
# the order GitHub happened to return them -- a superseded run could answer for a live one.
# Requiring all of them removes that arbitrariness and is strictly the safer direction for a
# guard whose failure mode is auto-merging.
failed=0
for group in "${CHECK_GROUPS[@]}"; do
for b in $(group_state "$group"); do
if [ "$b" != "pass" ]; then
echo "::error::Verification check matching '${group}' finished in bucket '${b}'."
failed=1
fi
done
done
[ "$failed" -eq 0 ] || { echo "Real verification checks missing/unsuccessful." >&2; exit 1; }
echo "Required real verification checks passed."

Expand Down
93 changes: 93 additions & 0 deletions tests/dependabot-auto-merge.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#!/usr/bin/env bash
# Drive dependabot-auto-merge.yml's wait-and-verify step against synthetic check snapshots.
#
# WHY this exists: the step is shell inside YAML, so nothing type-checks it and nothing runs it until
# a real dependabot PR does — at which point a mistake either refuses every green PR or, worse,
# merges an unverified one. Both failure modes are invisible to review.
#
# It has already earned its place. The first version of the polling fix named its array `GROUPS`,
# which bash maintains itself as the current user's supplementary group IDs; the assignment was
# silently ignored, the loop iterated numeric GIDs, and every PR was refused. The YAML parsed, the
# shell raised nothing, and reading it showed a correct-looking array. Only running it showed GIDs.
#
# Usage: bash tests/dependabot-auto-merge.sh
set -uo pipefail
W="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/.github/workflows/dependabot-auto-merge.yml"
SCRATCH=$(mktemp -d)
trap 'rm -rf "$SCRATCH"' EXIT

# Extract the `run:` block of the wait step, de-indented.
python3 - "$W" "$SCRATCH/step.sh" <<'PY'
import sys, pathlib
src = pathlib.Path(sys.argv[1]).read_text().splitlines()
start = next(i for i,l in enumerate(src) if l.strip() == "set -euo pipefail")
end = next(i for i,l in enumerate(src) if 'echo "Required real verification checks passed."' in l)
body = "\n".join(l[10:] if l.startswith(" "*10) else l for l in src[start:end+1])
pathlib.Path(sys.argv[2]).write_text(body + "\n")
PY

# Stub `gh`: echoes whichever snapshot the case selected.
mkdir -p "$SCRATCH/bin"
cat > "$SCRATCH/bin/gh" <<'GH'
#!/usr/bin/env bash
cat "$SNAPSHOT"
GH
chmod +x "$SCRATCH/bin/gh"
# WHY the wait budget is 1s here: cases that must REFUSE do so by exhausting it, and a
# production-length wait would make this suite take 90 minutes to prove that.
export PATH="$SCRATCH/bin:$PATH" PR_URL=stub AUTO_MERGE_WAIT_SECONDS=1 AUTO_MERGE_POLL_SECONDS=1

rc_all=0
run_case() {
local label="$1" json="$2" want="$3"
printf '%s' "$json" > "$SCRATCH/snap.json"
export SNAPSHOT="$SCRATCH/snap.json"
out=$(bash "$SCRATCH/step.sh" 2>&1); rc=$?
local got; [ $rc -eq 0 ] && got=ACCEPT || got=REFUSE
if [ "$got" = "$want" ]; then printf ' pass %-52s -> %s\n' "$label" "$got"
else printf ' FAIL %-52s -> %s (wanted %s)\n%s\n' "$label" "$got" "$want" "$(printf '%s' "$out" | head -4)"; rc_all=1; fi
}

ALL='[{"name":"gate / gate","bucket":"pass"},{"name":"cargo deny","bucket":"pass"},{"name":"cargo audit","bucket":"pass"},{"name":"osv-scanner","bucket":"pass"}]'
NOGATE='[{"name":"cargo deny","bucket":"pass"},{"name":"cargo audit","bucket":"pass"},{"name":"osv-scanner","bucket":"pass"}]'
PENDGATE='[{"name":"gate / gate","bucket":"pending"},{"name":"cargo deny","bucket":"pass"},{"name":"cargo audit","bucket":"pass"},{"name":"osv-scanner","bucket":"pass"}]'
FAILGATE='[{"name":"gate / gate","bucket":"fail"},{"name":"cargo deny","bucket":"pass"},{"name":"cargo audit","bucket":"pass"},{"name":"osv-scanner","bucket":"pass"}]'
ALTSPELL='[{"name":"Gate Attestation","bucket":"pass"},{"name":"cargo-deny","bucket":"pass"},{"name":"cargo audit","bucket":"pass"},{"name":"osv scanner","bucket":"pass"}]'
TWOGATE='[{"name":"gate / gate","bucket":"pass"},{"name":"other / gate-gate","bucket":"fail"},{"name":"cargo deny","bucket":"pass"},{"name":"cargo audit","bucket":"pass"},{"name":"osv-scanner","bucket":"pass"}]'

echo "== the bug this fixes =="
run_case "gate absent (the reported #46 failure)" "$NOGATE" REFUSE
run_case "gate still pending when fast checks passed" "$PENDGATE" REFUSE
echo "== it must still accept a genuinely green PR =="
run_case "every group present and passing" "$ALL" ACCEPT
run_case "alternate spellings across repos" "$ALTSPELL" ACCEPT
echo "== and still refuse the things it always refused =="
run_case "gate reported and failed" "$FAILGATE" REFUSE
run_case "two checks match one token, one of them fails" "$TWOGATE" REFUSE

echo "== the case the fix exists for: a gate that arrives late =="
mkdir -p "$SCRATCH/seq"
printf '%s' "$NOGATE" > "$SCRATCH/seq/0.json"
printf '%s' "$PENDGATE" > "$SCRATCH/seq/1.json"
printf '%s' "$ALL" > "$SCRATCH/seq/2.json"
echo 0 > "$SCRATCH/seq/n"
cat > "$SCRATCH/bin/gh" <<'GH'
#!/usr/bin/env bash
n=$(cat "$SEQDIR/n" 2>/dev/null || echo 0)
f="$SEQDIR/$n.json"
[ -f "$f" ] || f=$(ls "$SEQDIR"/[0-9]*.json | sort -V | tail -1)
echo $((n+1)) > "$SEQDIR/n"
cat "$f"
GH
chmod +x "$SCRATCH/bin/gh"
if SEQDIR="$SCRATCH/seq" AUTO_MERGE_WAIT_SECONDS=30 AUTO_MERGE_POLL_SECONDS=1 \
bash "$SCRATCH/step.sh" >/dev/null 2>&1; then
printf ' pass %-52s -> ACCEPT\n' "absent, then pending, then green"
else
printf ' FAIL %-52s -> REFUSE (wanted ACCEPT)\n' "absent, then pending, then green"
rc_all=1
fi

echo
[ "$rc_all" -eq 0 ] && echo "all cases pass" || echo "FAILURES above"
exit "$rc_all"