Repository navigation
fix(ci): select mutation fixtures from the divergence point, not the merged base tip - #1590
Conversation
59fc293 to
cdd0d96
Compare
…merged base tip A pull request is checked out as `refs/pull/N/merge`, so HEAD is a merge commit whose FIRST parent is the base branch tip. HEAD therefore already contains that tip, and the selector's `<base>...HEAD` walk reported every commit the base branch gained after the fork as though the head had made it. The three-dot diff cannot help: once HEAD contains the base tip, the merge base is the fork point and main's later commits sit on the included side. PR #1520 read 30 changed paths for a four-file change. The shard that ran printed `2 fixture(s) selected from 427 (diff 30 record(s), 30 changed path(s), corpus 427)` and spent 146 minutes re-proving fixtures guarding commits the branch merely lacked, producing no fact about its own files. Across all twelve shards that selection was 21 fixtures; under the fix it is 2, and the shard that timed out is not fanned at all. Normalising the event's base with `git merge-base "$BASE" HEAD` does not fix this. That job was already handed the merge base, so the expression returns its own input and the selection does not move. When HEAD is a merge the first parent IS the base, and that matches GitHub's own file list exactly: 4 files on #1520 and 22 on #1524. The first-parent rule is gated on the event, because a merge head is not exclusive to pull requests. A push that lands several commits and ends on a merge would otherwise discard `github.event.before` and diff only the merge: replaying real push heads, an ungated rule loses `glama.json` at 934f2ca and four more paths at d956e57, all direct-to-main commits no pull request run ever proved. An unrecognised event is refused rather than guessed at. Both selector sites resolve through one script rather than two copies of the logic, since a site that resolved differently would re-prove a set the plan never fanned it for. The resolved base sha is printed beside the record count, because the counts alone could not explain themselves. An unresolvable base fails loud: a shallow checkout answers exactly as a repository with no common ancestor does, and diffing against nothing would select zero fixtures and report a gate that proved nothing. Refs #1582
…nnot know Review found the event gate named `pull_request_target` beside `pull_request` with no cell asserting that arm. Checking the arm rather than adding a cell for it showed the arm should not exist: its premise is false. GitHub documents `pull_request_target` as running with GITHUB_SHA at the last commit on the DEFAULT BRANCH, "rather than in the context of the merge commit, as the pull_request event does", and it carries a `pull_request` payload, which has no `before`, so the workflow's BASE expression hands the resolver an empty string. The pull request's head is not in that checkout at all. Measured on a repository whose default branch tip is a merge, which is this one: `allow_merge_commit` is on and main carries 28 merge commits since 2026-09-06. The checkout therefore does have a second parent, but it belongs to somebody else's merged pull request, so the first-parent rule resolved to main-before-that-unrelated-merge and selected THAT pull request's fixtures, exiting 0 and printing nothing unusual. Deleting only the arm is not a fix either: the empty-base fallback takes `HEAD~1`, which on a merge is the same commit, so it reaches the identical wrong answer with only a stderr note. Both spellings are silently wrong, which is exactly the defect this branch exists to remove, so the event is now refused and must be decided by whoever adds the trigger. The workflow has no such trigger today. The arm surviving its mutant was the symptom of this, not a coverage gap to be papered over with a cell: the arm was a no-op under the shipped BASE expression, so any cell written to the old premise would have passed with and without it. The cell now asserts the refusal, and the mutant that pins it re-admits the event to the allow-list, which is where the behaviour now lives. Also corrects the mutant retargeted in the previous revision, whose `expectRed` named a different cell and so graded WRONG-RED against a suite that had in fact gone red. Refs #1582
cdd0d96 to
5a6aba5
Compare
|
Corrections to the description, which is stale at the current head. The description is left as posted because CI is running against it; these supersede it. Two figures are stale and are now wrong.
One claim is withdrawn rather than re-counted. The description replayed two real push heads and named the paths an ungated first-parent rule would lose. That method is not sound: it assumed consecutive workflow-run head shas bound a single push, and they do not, because a commit marked A behaviour changed since the description was written. On a repository whose default branch tip is a merge, which is the case here, that checkout does have a second parent, but it belongs to another merged pull request. Measured both ways with an empty base: keeping the first-parent rule and deleting it both resolve to the same commit and select that other pull request's fixtures, exiting 0. Neither spelling can find the right base, so the event is now refused with a non-zero exit and an Not stale, and re-derived first-hand after being questioned: the Acceptance, from this pull request's own run at the current head. The plan job printed Refs #1582 |
|
The description has now been corrected in place, so the note above is no longer a supersede: the two stale figures are The reachability of the ungated-rule shape now rests only on facts that can be rechecked: merge commits are enabled on this repository, main has taken 30 of them since 2026-09-06, and 21 of the last 100 push-triggered runs of this workflow ran on a two-parent head. No count is claimed for how much an ungated rule would discard, because that depends on The head is unchanged at Refs #1582 |
1 participant
What was happening
A pull request is checked out as
refs/pull/N/merge, so the commit CI works on is a merge commit whose first parent is the base branch tip. The head therefore already contains the base branch. The re-proof selector diffed<base>...HEAD, and once the head contains the base tip, every commit the base branch gained after the fork lands on the included side of that walk and is read as part of the change.The three-dot diff cannot help here. Three-dot means "from the merge base", and the merge base of a base-branch commit and a head that already contains the base tip is the fork point, which is exactly the commit that makes the base branch's later work look like the pull request's.
So a branch that had fallen behind selected fixtures guarding commits it did not contain, spent runner hours re-proving them, and could be reddened by a fixture it had no relationship to.
Measured on a real run: a four-file change read 30 changed paths. The shard that ran printed
2 fixture(s) selected from 427 (diff 30 record(s), 30 changed path(s), corpus 427), then spent 146 minutes re-proving fixtures guarding commits the branch merely lacked before hitting the step cap, producing no fact about the files the pull request actually changed. Across all twelve shards that selection is 21 fixtures.Why the obvious fix does not work
Normalising the event's base with
git merge-base "$BASE" HEADlooks right and changes nothing. That job was already handed the merge base, so the expression returns its own input and the selection does not move. The same is true ofmerge-base HEAD^1 HEAD^2: under a merge ref it re-derives the very base that was passed in.Every formula that re-derives the base from the merge head finds the base it was already given. When the head is a merge, the first parent is the base branch tip the head was built on, and that is the commit to diff from. Checked against the API's own file list for the two affected pull requests: first-parent gives 4 and 22 files, and the API reports 4 and 22.
What now happens
scripts/mutation-reproof-base.sh. The base expression previously appeared twice, and two copies of the selector is the shape of this defect: a fix applied to one site would leave the plan and the shard disagreeing about which fixtures exist, with a green board either way.github.event.beforeand diff only the merge itself, silently dropping the rest of the pushed range. That shape is reachable rather than hypothetical: merge commits are enabled on this repository, main has taken 30 of them since 2026-09-06, and 21 of the last 100 push-triggered runs of this workflow ran on a two-parent head. An unrecognised or missing event is refused rather than guessed at.merge-base, which is a no-op when it is already the divergence point. Push, schedule, first-push (all-zero sha) and empty-base shapes keep their existing meaning and each has its own cell.fetch-depth: 0, and this refusal is what catches a future change that drops it.On the measured run, the whole-corpus selection goes from 21 fixtures across 30 paths to 2 fixtures across 4 paths, and the shard that ran for 146 minutes is not fanned at all.
Proof
pull_requesttakes its first parent;pull_request_targetis refused, because that event checks out the base and its merge head is not the pull request's; an unknown or missing event is refused; the resolved base appears in the transcript; an unresolvable base fails loud. Each was shown red with the fix reverted.bin/smoke/mutations/mutation-reproof-base.json: 11 mutants, each reddening its cell by name. Mutants revert each workflow site independently, take the second parent instead of the first, apply the first-parent rule on every event, guess at an unrecognised event, accept an empty merge base, drop the printed base, remove the all-zero fallback, and re-admitpull_request_target. All 11 were run against this head and all 11 were killed; that sweep is a local run, as the mutation proof is not part of this pull request's CI board.mutation-reproof smoke: 143 passed, 0 failed, alongside the full typecheck, the fixture-anchor gate, the corpus partition smoke, the full-sweep workflow smoke and the concurrency smoke.Not claimed
This does not change the selection on the other pull request where a fixture read WRONG-RED. Replaying that shard both ways selects the same four fixtures, because the fixture's guarded source is in that branch's own diff. That verdict is a real signal about that branch and this change neither creates nor clears it.
Refs #1582