ci: run acceptance in the merge queue, serialise trunk - #43
Merged
Conversation
Three gaps, all of which showed up merging #40/#41/#42. Acceptance only ran after merge — image/acceptance/promote were gated on push to main, so a PR never built a candidate and never ran E2E. The rate-limit change in #41 was green on its PR and would have broken trunk: the suite runs three device passes from one address and trips the new per-IP credential limit. Caught by reading the feature files, not by CI. Nothing tested the combination. Each PR was verified against main as it was when the branch was cut, never against main as it would be. Promote was an unguarded race. No concurrency group, so two merges minutes apart give two overlapping trunk pipelines both retagging :latest, last writer wins. :latest can end up on the older commit with every job green. Two runs did overlap; the order held by luck. Now: pull_request runs the commit stage only (the PR head isn't what lands). merge_group runs the whole pipeline against trunk + the queued change — that's the gate. push to main runs it again and promotes, because squash-merging makes a new sha and the released artifact must be one that was actually tested. Trunk and queue runs never cancel; superseded PR runs do. Merge queue itself is enabled after this lands — turning it on first would deadlock, since the queue would run merge_group against a workflow that has no merge_group trigger. ADR in docs/adr/0001. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes "merge all the PRs at once" safe by construction instead of by hand-sequencing them. ADR:
docs/adr/0001-trunk-based-delivery-pipeline.md.what was wrong
Acceptance only ran after merge.
image/acceptance/promotewere gated onpushtomain, so a PR never built a candidate and never ran E2E — you can see it on any recent PR: Build candidate image — skipping, Acceptance / E2E — skipping. The rate-limit change in #41 was green on its PR and would have broken trunk: the suite runs three device passes against one server from one address and accumulates more credential failures than the new per-IP limit allowed. I caught it by reading the feature files, not because CI told me.Nothing tested the combination. Every PR was verified against
mainas it was when the branch was cut, never as it would be. #40 and #41 touched the same tree and were independent only by luck — I trial-merged them locally before merging, which is the merge queue's job.Promote was an unguarded race. No
concurrencygroup, so two merges minutes apart produce two overlapping trunk pipelines, both ending indocker buildx imagetools create --tag :latest. Last writer wins, nothing enforces order, so:latestcan land on the older commit with every job green — silent, no failure signal. Two runs did overlap while merging #41 and #42; the order happened to hold.what changes
pull_requestmerge_grouppush(main)Plus a
concurrencygroup that serialises trunk, cancels only superseded PR runs, and per-jobtimeout-minutesso a hung job can't sit on a runner.Acceptance now runs twice per change, once in the queue and once on trunk. That's deliberate — squash-merging makes a new sha, and promoting an artifact built from a sha that was never tested would give up the single-artifact property the pipeline exists to protect.
merge order matters
This PR must land before the merge queue is turned on. Enabling the queue first would deadlock it: the queue would run
merge_groupagainst a workflow that has nomerge_grouptrigger, so no check would ever report. I'll enable the ruleset right after this merges, with the admin role as a bypass actor so a stuck queue can never lock you out of your own repo.test-mpd-imageis deliberately not a required check — it's path-filtered, and a required check that doesn't run on every merge group hangs the queue.🤖 Generated with Claude Code