Skip to content

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

Merged
forkwright merged 1 commit into
mainfrom
fix/46-auto-merge-check-race
Aug 21, 2026
Merged

fix(auto-merge): wait for the verification checks to appear, not just to finish#47
forkwright merged 1 commit into
mainfrom
fix/46-auto-merge-check-race

Conversation

@forkwright

Copy link
Copy Markdown
Owner

Closes #46.

Finding

gh pr checks --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. The reported-checks verification then
ran against a list that did not contain the gate yet, took its
No required verification check matching any of: gateattestation gategate branch, and refused a PR
whose gate went on to pass green on the same head.

@t0-akroasis diagnosed this from akroasis #454/#455 and filed it with the run log and the
normalised-token table.

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 evaluated that at
the one moment a slow check legitimately has 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.

What changes

Poll until every token group is present and terminal, then judge. Two properties are load-bearing
and both are commented at the site:

  • The timeout refuses rather than falling through. A group that never reports is precisely the
    condition this guard exists to catch, so exhausting the wait must refuse. Falling through would
    reintroduce the bug in the unsafe direction — auto-merging on an absent verification.
  • One gh pr checks fetch per poll, shared across all four groups. The previous code fetched once
    per token, so the four groups were answered from four snapshots taken seconds apart. That is a
    second instance of the same race, and it means no single consistent view of the PR was ever
    evaluated.

Matching, normalisation, and the every-match-must-pass rule are unchanged.

Verification

tests/dependabot-auto-merge.sh extracts the step's shell and drives it against synthetic check
snapshots:

== the bug this fixes ==
  pass  gate absent (the reported #46 failure)               -> REFUSE
  pass  gate still pending when fast checks passed           -> REFUSE
== it must still accept a genuinely green PR ==
  pass  every group present and passing                      -> ACCEPT
  pass  alternate spellings across repos                     -> ACCEPT
== and still refuse the things it always refused ==
  pass  gate reported and failed                             -> REFUSE
  pass  two checks match one token, one of them fails        -> REFUSE
== the case the fix exists for: a gate that arrives late ==
  pass  absent, then pending, then green                     -> ACCEPT

The last case is the one that matters — absent on poll 1, pending on poll 2, green on poll 3, and it
accepts. The old code refused at poll 1.

The harness earned its place before it was committed

The first version of this fix named its array GROUPS. Bash maintains GROUPS itself as the
current user's supplementary group IDs; the assignment was silently ignored, the loop iterated
numeric GIDs, matched no check, and refused every PR — the same always-refuse symptom being fixed
here, reached a different way.

The YAML parsed. The shell raised nothing. Reading it showed a correct-looking array. Only running it
showed 1000 (MISSING), 18 (MISSING), 39 (MISSING). That is recorded as a WARNING at the
declaration so the name cannot drift back.

The harness then found a second thing: a hardcoded 30-second poll made the step impossible to exercise
at test speed, so the late-arriving-gate case could not reach its third snapshot and reported a false
refusal. The interval now takes an env override that nothing in CI sets. A step that cannot be run
faster than production cannot be tested, and this one is shell inside YAML that nothing else checks.

shellcheck -s bash tests/dependabot-auto-merge.sh exits 0.

Scope

Adopting repos need no change — the reusable's interface is unchanged. Repos that adopted it and saw
auto-merge refuse green PRs (akroasis #454, #455 were merged by hand) should work from the next
dependabot PR without intervention.

… to finish

`gh pr checks --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, took its "no matching check" branch, and refused 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 evaluated that at the one moment a slow check legitimately has 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.

Now it polls until every token group is present AND terminal, then judges. Two
properties are load-bearing:

- The timeout REFUSES rather than falling through. A group that never reports is
  exactly the condition this guard exists to catch, so exhausting the wait must
  refuse; falling through would reintroduce the bug in the unsafe direction.
- One `gh pr checks` fetch per poll, reused across all four groups. The previous
  version fetched once PER TOKEN, so the groups were answered from four snapshots
  taken seconds apart — a second instance of the same race, and no single
  consistent view of the PR was ever evaluated.

`tests/dependabot-auto-merge.sh` drives the step against synthetic snapshots,
including the case the fix exists for: a gate absent on the first poll, pending
on the second, green on the third.

It earned its place before it was committed. The first version of this 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. The array is `CHECK_GROUPS` now, with that recorded at the site.

The harness also found that a hardcoded 30s poll made the step untestable at
speed, so the interval takes an env override that nothing in CI sets.

Closes #46
@forkwright
forkwright merged commit eba37bd into main Aug 21, 2026
1 check passed
@forkwright
forkwright deleted the fix/46-auto-merge-check-race branch August 21, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dependabot-auto-merge races its own reported-checks verification and refuses every green PR

1 participant