Skip to content

fix(ci): derive the coverage ratchet's merge base instead of trusting the event payload - #474

Merged
rubenvdlinde merged 2 commits into
mainfrom
S10/coverage-guard-merge-base
Aug 16, 2026
Merged

fix(ci): derive the coverage ratchet's merge base instead of trusting the event payload#474
rubenvdlinde merged 2 commits into
mainfrom
S10/coverage-guard-merge-base

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

The finding

Measure merge-base coverage in the shared quality.yml never computed a merge base. It ran

MERGE_BASE="$(git merge-base "$BASE_SHA" "$HEAD_SHA")"   # BASE_SHA = github.event.pull_request.base.sha

and both halves are wrong in a way that hid the other:

  1. pull_request.base.sha is frozen at PR-open time. GitHub does not refresh it as the base branch advances. Measured, not assumed — ConductionNL/zaakafhandelapp#247 ("Release: merge development into beta"), opened 2026-05-23:

    $ gh api repos/ConductionNL/zaakafhandelapp/pulls/247 --jq '{base_sha:.base.sha,created_at}'
    {"base_sha":"b233af9967dab5a2302c82924ddbecfcba523c1c","created_at":"2026-05-23T09:12:27Z"}
    $ gh api repos/ConductionNL/zaakafhandelapp/git/ref/heads/beta --jq .object.sha
    a92d87072f3d27c6acab397173c0adb1ff6d3a7b        # moved 2026-06-25
    

    Nearly three months of drift on a PR that is re-run nightly.

  2. On a pull_request event the checkout is refs/pull/N/merge, which already contains the base branch. So git merge-base <any base commit> <merge ref> returns that base commit unchanged. The call was a no-op that laundered a stale sha into something that reads like a computed merge base — while the step's own comment claims "the merge base is immutable for a given head, so it cannot go stale".

What it cost

b233af99 predates zaakafhandelapp committing a composer.lock and predates phpunit/phpunit being a dependency at all. composer install therefore resolved to latest, produced a 64-package set with no test runner, and the step died on:

/home/runner/work/_temp/….sh: line 9: ./vendor/bin/phpunit: No such file or directory
##[error]Could not measure coverage at merge base b233af99… — refusing to report a ratchet that did not run.

The message reads like an unreachable object. It is not — the object was checked out fine (HEAD is now at b233af9). The suite itself was green in the very same job: OK (268 tests, 825 assertions).

Run: zaakafhandelapp 31926377589, job quality / PHPUnit (PHP 8.3, NC stable34, pgsql).

The fix

The merge ref's parents are the authoritative answer and cost nothing: parent 1 is the current base tip, parent 2 is the PR head. They are trusted only when parent 2 actually equals pull_request.head.sha, so a PR head that happens itself to be a merge commit cannot be mistaken for the merge ref. Falls back to origin/<base ref>, then to the payload sha — which now says out loud that it may be stale.

Before / after — measured against the real refs of zaakafhandelapp#247

Merge ref cb1439f3 fetched locally from refs/pull/247/merge; ^1 = a92d8707 (beta tip), ^2 = 986730b8 (development tip).

resolved merge base can PHPUnit run there?
old logic b233af99 ❌ no phpunit/phpunit in require-dev, no composer.lockhard failure
new logic 27027b32 phpunit/phpunit: ^10.5, phpunit.xml, 55 files under tests/

All three resolution branches were exercised against those refs:

CASE A  merge ref checked out   -> resolved via the merge ref: base-tip=a92d8707 pr-head=986730b8
                                   merge-base=27027b32
CASE B  PR head checked out     -> resolved via origin/beta: base-tip=a92d8707
                                   merge-base=27027b32
CASE C  neither available       -> ::warning:: …falling back to the event payload's base.sha…
                                   merge-base=b233af99
OLD LOGIC on CASE A             -> merge-base=b233af99

Also: the error message is split in two

The two ways this step can fail have different remedies and the single old message named neither (gate-59's unclosable-gate shape, in an error message rather than a finding):

  • base has no ./vendor/bin/phpunit after composer installmerge the base branch in / rebase onto a base that carries the suite;
  • base has the runner but produced no clover → read the PHPUnit output above.

Whether the base could run is recorded while the base is checked out — after the checkout back to HEAD the evidence is gone, and the old message asserted a cause the log did not support.

This does not weaken the ratchet

A base that cannot be measured is still a hard failure, and no coverage number is ever inferred from a missing measurement. The change only makes the step measure the commit it always claimed to measure.

Blast radius

quality.yml is consumed @main by all 18 app repos, so this lands everywhere at once. It is confined to one step that today only ever runs on pull_request, and its failure mode is unchanged (hard error) — only the commit it selects, and the wording, change.

Not done here

.coverage-baseline, coverage-guard.php and the push-side floor are untouched.


⚠️ Framing correction — it is quiet PRs that drift, not "base.sha never refreshes"

The finding above says GitHub "does not refresh" pull_request.base.sha. That is
too strong, and the correct statement is sharper:

base.sha DOES refresh — on a synchronize. A push to the head branch
regenerates the event payload. What accumulates staleness is a quiet pull
request: one nobody has pushed to, which is re-run nightly against a base tip
frozen at its last push.

Measured on this very pull request. It sat untouched while main advanced twice,
and a rerun did not refresh anythingrefs/pull/474/merge still carried
parent 1 = f6e56a35, the base as of PR-open, so the rerun's green result was
evidence about main@f6e56a35. One merge-and-push later:

before:  base.sha = f6e56a35…   merge ref 2db91ce6  parents = f6e56a35  ab4bd39e
after :  base.sha = da897594…   merge ref 4784e6a2  parents = da897594  e627ae1b
                    ^^^^^^^^ the live tip of main

zaakafhandelapp#247 is the extreme of the same mechanism, not a different one: a
release PR opened 2026-05-23 that nobody pushes to, so its base.sha has been
frozen for nearly three months while beta moved on.

What this means for the fix — CASE A's drift is BOUNDED, base.sha's is not

The merge ref's parent 1 is the base tip as of the last merge-ref computation,
not a live read of origin/<base>. So CASE A does not eliminate staleness; it
bounds it by GitHub's merge-ref refresh, where base.sha is unbounded from
PR-open. CASE B (origin/<base ref>) is the live tip and is what runs when
the merge ref is not checked out.

That is the honest claim, and it is still decisive: the failure this fixes is a
merge base predating the repo's composer.lock and phpunit itself. Bounded drift
cannot reach back that far; unbounded drift did.

Why it is worth landing now

development's PHPUnit (8.3, stable34, pgsql) red on zaakafhandelapp is this
exact defect — "refusing to report a ratchet that did not run", with zero
failing tests
in the same job. A freshly-cut branch is green on that cell purely
because its payload is new. Every quiet PR in the fleet is accumulating the same
false red.

… the event payload

`Measure merge-base coverage` computed
`git merge-base "$BASE_SHA" "$HEAD_SHA"` with BASE_SHA taken from
`github.event.pull_request.base.sha`. Both halves are wrong and they hid
each other.

`pull_request.base.sha` is the base branch tip snapshotted when the pull
request was OPENED; GitHub never refreshes it. On zaakafhandelapp#247
("Release: merge development into beta", opened 2026-05-23) it still reads
b233af99 — beta's tip that day — although beta moved to a92d8707 on
2026-06-25.

And on a `pull_request` event actions/checkout leaves `refs/pull/N/merge`
at HEAD. That merge commit already contains the base branch, so
`git merge-base <base commit> <merge ref>` returns the base commit
unchanged: the call was a no-op that laundered a stale sha into something
that reads like a computed merge base.

The consequence is a red job with a misleading error. b233af99 predates
zaakafhandelapp committing a composer.lock AND predates phpunit/phpunit
being a dependency at all, so `composer install` resolved to latest,
produced a 64-package set with no test runner, and the step died on
`./vendor/bin/phpunit: No such file or directory`. It reported
"Could not measure coverage at merge base", which reads like an
unreachable object. The suite itself was green in the same job:
`OK (268 tests, 825 assertions)`.

The merge ref's parents are the authoritative answer: parent 1 is the
CURRENT base tip, parent 2 is the PR head. They are trusted only when
parent 2 actually equals `pull_request.head.sha`, so a PR head that
happens to be a merge commit cannot be mistaken for the merge ref. When
the checkout is not the merge ref we use `origin/<base ref>`; the payload
sha remains as a last resort and now says out loud that it may be stale.

Verified against the real refs of zaakafhandelapp#247 (merge ref
cb1439f3):
  old logic -> b233af99  (no phpunit at that commit -> hard failure)
  new logic -> 27027b32  (merge-base of a92d8707 and 986730b8; carries
                          phpunit/phpunit ^10.5, phpunit.xml and 55 test
                          files, so the floor can actually be measured)
All three resolution branches were exercised against those refs.

The failure message is also split in two, because the two ways this can
fail have different remedies and the single old message named neither:
a base with no test runner tells you to merge the base branch in, and a
base whose suite produced nothing tells you to read the PHPUnit output.
Whether the base ran is recorded WHILE the base is checked out — after
the checkout back to HEAD the evidence is gone.

This does not weaken the ratchet: a base that cannot be measured is still
a hard failure, and no coverage number is ever inferred.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Blast radius, measured: 12 of the 18 standing development → beta release PRs carry a stale base.sha

For each repo: the open PR whose head is development and base is beta, its pull_request.base.sha, and the current beta tip.

repo PR opened base.sha beta tip
openregister #1711 2026-05-23 69e3edc3 69e3edc3 fresh
opencatalogi #538 2026-05-07 f0c56313 30a09b55 STALE
openconnector #1094 2026-07-29 01a15d4f 01a15d4f fresh
docudesk #71 2026-03-19 09cbdbb1 d8cca45c STALE
nldesign #45 2026-03-19 779fb48f bfc45be9 STALE
launchpad #22 2026-08-01 c15a41d1 c15a41d1 fresh
softwarecatalog #197 2026-03-19 934bda27 7b784d6b STALE
larpingapp #90 2026-03-19 58c46e71 70440b57 STALE
zaakafhandelapp #247 2026-05-23 b233af99 a92d8707 STALE
procest #18 2026-03-19 03e99948 3dfbb74f STALE
pipelinq #13 2026-03-19 3ead62b1 297f4bfb STALE
shillinq #63 2026-04-10 76af662a ea58e657 STALE
scholiq #4 2026-05-11 062a953a 93eefa54 STALE
portaliq #7 2026-07-23 334ee3f8 334ee3f8 fresh
decidesk #2 2026-04-10 b10823b8 3fb56d35 STALE
openbuild #79 2026-08-01 5ab286fb 5ab286fb fresh
doriath #126 2026-08-02 01682acd 01682acd fresh
hermiq #29 2026-07-23 8d99b239 8d99b239 fresh

The six "fresh" rows are simply PRs opened after beta last advanced — not a different behaviour, just drift that has not started yet. The oldest stale rows are from 2026-03-19: five months.

Why the fleet has not noticed

The bug is invisible on a short-lived PR, because there base.sha and the real merge base coincide. Control: shillinq run 31924886701, a normal fix/**development PR — Coverage Baseline Protection success, all six PHPUnit cells success.

It only bites where a PR is long-lived, which is exactly the standing release PRs — and it bit hardest in zaakafhandelapp, whose stale base predates both its composer.lock and its PHPUnit adoption.

Knock-on for the fleet debt programme

Two entries in the programme's ground-truth table were derived from these release-PR runs rather than from push runs on development, and both attributed a PHPUnit failure to the app:

  • zaakafhandelapp baseline 31926377589 → the push run on the same SHA 986730b8 is 31926374351: PHPUnit green.
  • openconnector baseline 31929160337 → the push run on the same SHA a2cb269a is 31929156734: PHPUnit green.

Neither app had a failing test. Both were this step.

Regenerates refs/pull/474/merge. A rerun re-uses the merge commit computed at the
last synchronize and does NOT re-merge a base that has moved, so the PR's green
run described main@f6e56a35 while main had advanced to da89759. Only a push
refreshes it.
@rubenvdlinde
rubenvdlinde merged commit 18fe6f9 into main Aug 16, 2026
34 of 35 checks passed
@rubenvdlinde
rubenvdlinde deleted the S10/coverage-guard-merge-base branch August 16, 2026 16:54
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.

1 participant