Skip to content

No CI run builds the combination that lands: a merge burst ships semantic conflicts untested #2412

Closed

Description

@rbuergi

The gap

No CI run ever builds the combination that lands. With strict: false (correct, and not what I am proposing to change) a PR is tested against the main it branched from. Merge several in quick succession and the tree that reaches main is a combination no run has ever compiled — while every individual PR was legitimately green.

It shipped today

Five PRs merged in fifteen seconds at 17:05:04–17:05:11. Main went red at 17:26:

memex/Memex.Portal.Shared/Api/MeshApiEndpoints.cs(335):
error CS0246: The type or namespace name 'MeshOperations' could not be found

A semantic conflict, so git had nothing to report and no merge was DIRTY:

Each is correct. Each compiles against its own base. Only together do they fail. Fixed in #2411 (one using), but the detection is the issue: it surfaced from a scheduled main build ~20 minutes later, and it hard-blocks CD, which refuses a red tip. Today that sat directly in front of the surface-manifest image.

Why the existing machinery does not cover it

  • strict: false is deliberate and worth keeping — requiring every branch to re-sync before merge costs a full CI cycle per PR and, with several in flight, most of the throughput.
  • The green-tree marker / skip-reuse makes it likelier, not less: a merge that reuses an already-green tree does not re-compile the combination either.
  • The binary-compatibility gate can now block a merge #2407 (binary-compat gate now blocking) does not catch this. That gate is about an already-published module binding a name; this is two source changes that disagree. Same family — a green solo run proves less than it looks — different mechanism. Worth stating plainly so nobody assumes it is covered.

Options, roughly by cost

  1. A post-merge tripwire on main that is fast and loud. Main already builds; the gap is latency and attribution. A compile-only job on every push to main, alerting with the failing file and git log -S <symbol> already run, turns a 20-minute silent window into a named alert. Cheapest, does not slow any PR, does not prevent the break.
  2. Re-test the merge result before it lands — a merge queue, or strict: true. Actually prevents it; costs a cycle per merge and serialises the queue. That is the trade strict: false was chosen against, so this needs the maintainer, not a PR.
  3. Bound the burst. Most of the risk is the fifteen-second window: merges close enough together that none saw the others. A minimum spacing, or "wait for main's run on your merge before the next", costs ~20 min per merge and is what we did manually today after the fact.

🚨 My recommendation is (1) now, (2) as a maintainer decision — and I want to be explicit that I am the one who created today's instance: I merged five PRs in fifteen seconds. The batching was sanctioned and each PR was sound; the burst is what made the untested combination. A tripwire that names the symbol would have caught it in ~2 minutes instead of 20, which is the difference between a footnote and a blocked deploy.

Activity

  1. rbuergi commented on Aug 26, 2026

    @rbuergi
    ContributorAuthor

    Correction: I was wrong about the green-tree marker

    The issue body says:

    The green-tree marker / skip-reuse makes it likelier, not less: a merge that reuses an already-green tree does not re-compile the combination either.

    That is incorrect, and I want it struck rather than left standing.

    The marker is keyed on the tree hash — the lookup is refs/ci-green/$tree/* — so it can only match a byte-identical tree. Verified on today's post-cascade green:

    reuse job log:   Tree under test: 9fa28824a91e5844620720d5206c6b566106431f
    git rev-parse 432703090^{tree}
                  → 9fa28824a91e5844620720d5206c6b566106431f
    

    So a skip-reuse green is not a weaker signal about the combination — it is a statement that this exact tree was already compiled and tested. Reuse therefore cannot manufacture an untested combination: it only fires where the combination was tested. If the merge produces a novel tree, no marker matches and the full run happens.

    Credit where due: meshweaver-c0 read the provenance and supplied the evidence.

    What survives, and it is the whole issue

    The real gap is unchanged and is not about reuse:

    No CI run builds the combination that lands. With strict: false, a PR is tested against the main it branched from. Merge two whose changes are individually correct but jointly inconsistent, and the resulting tree has never been compiled — as happened at 17:26 today (CS0246: 'MeshOperations' could not be found, a semantic conflict between #2283 and #2397 that no PR run could have seen).

    The marker does not help there, but it does not hurt either — a novel tree simply gets a full run. My original wording implied the mechanism was part of the problem; it is neutral.

    One thing today did demonstrate about the fix

    The clean marker on this tree existed because main did not move between #2395's full run and the merge — a hold happened to be in force. That is incidental, not a proposal. But it is a small piece of evidence for option (3) in this issue: spacing merges does not merely avoid untested combinations, it also lets markers match, so the next run is cheaper as well as more trustworthy. The tripwire (option 1) remains the cheap first move.

  2. rbuergi commented on Aug 26, 2026

    @rbuergi
    ContributorAuthor

    #2447 makes this DETECTABLE. It does not fix it — don't close this on that PR.

    #2447 stops main's runs cancelling each other, so from now on a run does exist for the tree that landed. That was necessary and it was missing: main's five consecutive runs from 20:28–20:38 today were all cancelled, so the combination was neither built nor shippable.

    But it changes when you find out, not whether it lands. The bad combination still merges, main still goes red, and someone still has to hotfix it. The CS0246 incident would have played out identically with #2447 in place — just with a red run naming it instead of silence.

    The structural fix is a merge queue, and the CI work is half done

    A merge queue tests the prospective combination before it lands, which is exactly this issue's title. Concretely, across the two repos:

    repo merge_group: trigger merge queue enabled
    MeshWeaver.Plugins ✅ present in ci.yml (added by Plugins#467, "CI fires on merge_group, so main can use a merge queue") — deliberately kept dormant ❌
    MeshWeaver (this repo) ❌ absent from dotnet-test.yml (push, pull_request, workflow_dispatch only) ❌

    So plugins is ready and core is not. 🚨 That asymmetry is a trap in its own right: a workflow that never fires on merge_group reports nothing, and a queue entry then waits out its timeout — the queue stalls silently rather than failing. Plugins#467's own comment says exactly this. Enabling a queue on this repo without first adding the trigger would be worse than not enabling it at all.

    The order, if this route is taken: add merge_group: to dotnet-test.yml → confirm the required check (Consolidate test results) actually reports on a merge_group event → only then enable the queue in branch protection. There is prior art on both repos being surprised by this: Plugins#561 and Plugins#702 both record AGENTS.md describing a merge queue that no repo actually has.

    Why this is worth doing beyond correctness

    The queue also dissolves the throughput problem measured on plugins today — 3.38 CI runs per merged PR against this repo's 1.98 — because strict: true there means each merge re-stales every other in-flight PR (Plugins#728). A queue batches instead, testing the batch once.

    Not enabling anything. Branch protection and repo settings are outward-facing policy. Recording the finding, the ordering constraint, and the fact that the trigger is missing here — so the decision is made against what the repos actually have rather than what AGENTS.md says they have.

  3. rbuergi commented on Aug 27, 2026

    @rbuergi
    ContributorAuthor

    Measured after #2447: it works, and the burst AMPLIFIES into CD

    #2447 does what it claimed. Main's runs now complete, and CD published end to end — 3.0.0-rc8.ci.6028 from cd72825, with the full staging set promoted:

    3.0.0-rc8.ci.6028
    staging-cd72825-33045482609
    staging-cd72825-33045482609-linux-x64
    staging-cd72825-33045482609-linux-arm64
    

    cd72825 is precisely the run that survived the five-push burst at 05:59 — the one still running while the four queued behind it were cancelled. Before #2447 it would have been killed too, and nothing would have published.

    The part that is new, and argues for the queue rather than against it

    A burst does not merely lose the intermediate runs — it multiplies CD runs. Six CD runs for the single SHA 5041d00e, every one cancelled:

    06:57:53 event=workflow_run concl=cancelled
    06:55:59 event=workflow_run concl=cancelled
    06:49:15 event=workflow_run concl=cancelled
    06:47:07 event=workflow_run concl=cancelled
    06:43:26 event=workflow_run concl=cancelled
    06:40:02 event=workflow_run concl=cancelled
    

    The mechanism, and none of it is a CD bug: CD subscribes with workflow_run: types: [completed], and a cancelled run counts as completed. So each cancelled build run spawns a CD run. main-cd.yml's own concurrency is already correct — group: main-cd-${{ github.ref }}, cancel-in-progress: false — but as established above, a concurrency group holds one in-progress plus one pending and each new entrant supersedes the pending one. So the CD runs then cancel each other.

    Net effect of one merge burst: N cancelled build runs → N CD runs → N−1 of those cancelled, and under the pre-#2470 verdict each surviving one announces "the required check settled RED" about a commit where nothing failed.

    Why this strengthens the case for a merge queue

    The runner cost of a burst is not "the tests we skipped" — it is a multiple of it, spent on runs that are cancelled before they decide anything, plus a false-alarm per survivor. A queue removes the burst at source: one combination, tested once, before it lands.

    Reiterating the ordering constraint from my earlier comment, because it is the thing that would go wrong: this repo has no merge_group: trigger. MeshWeaver.Plugins and MeshWeaver.Manufacturing both do. A queue enabled here without adding it first would leave entries waiting out their timeout rather than failing — a stall, not an error.

  4. rbuergi commented on Aug 27, 2026

    @rbuergi
    ContributorAuthor

    Reopened — this was auto-closed by a PR that does not close it, and the mistake is mine

    #2447's body said "Closes #2412", which GitHub acted on at merge. I wrote that line before I understood what the fix actually buys, and then established the opposite in this comment on this very issue — without going back to fix the keyword. So the issue closed itself on the strength of a claim its own thread already refutes.

    What #2447 does: stops main's in-progress run being killed, so a burst can no longer leave nothing completed and CD able to publish nothing.

    What it does not do — this issue's actual title: it does not make anything build the combination that lands. cancel-in-progress: false protects the run already running; a concurrency group holds one in-progress plus one pending, and each new push supersedes the pending one. Measured 2026-08-27, five pushes in fourteen seconds:

    33044271801 cd728254 created=05:59:22 started=05:59:22 concl=null       ← ran, later published ci.6028
    33044274800 4f773a47 created=05:59:25 started=05:59:25 concl=cancelled
    33044277810 507ba4e1 created=05:59:29 started=05:59:29 concl=cancelled
    33044280598 a3406b43 created=05:59:32 started=05:59:32 concl=cancelled
    33044283806 da50c51e created=05:59:36 started=05:59:36 concl=cancelled
    

    run_started_at == created_at on all four — they never executed a step. Four of five landed commits went uncompiled, which is precisely "no CI run builds the combination that lands".

    The CS0246 incident that opened this issue would play out identically today, just with a red run naming it instead of silence.

    Still required

    A merge queue, which tests the prospective combination before it lands — plus the ordering constraint: this repo has no merge_group: trigger (MeshWeaver.Plugins and MeshWeaver.Manufacturing both do). Adding it must come first; a queue enabled without it leaves entries waiting out their timeout rather than failing.

    Leaving this open until that lands. The measurement of how a burst amplifies into CD is in the comment above.

  5. rbuergi commented on Aug 28, 2026

    @rbuergi
    ContributorAuthor

    Classification note from the issue-zero walk: this is a maintainer decision, not an open defect — the remaining action is adopting a merge queue (merge_group: trigger, which MeshWeaver.Plugins already carries) so the prospective combination is built before it lands. #2447 closed the "burst leaves nothing completed" half; nothing currently builds the combination that lands, exactly as the reopen note above records. Leaving open awaiting that call.

  6. rbuergi commented on Aug 30, 2026

    @rbuergi
    ContributorAuthor

    Not closing this — the reopen note above is explicit that a run merely existing for the landed tree is not the fix, and I came within one command of repeating exactly that mistake on the strength of #2767. Recording today instead, because it is a clean fresh instance of the defect this issue describes.

    It happened again today, in the shape this issue predicts

    Two PRs, each legitimately green against the main it branched from:

    Neither run could see the other. The combination went red on main and stayed red for five consecutive runs (62d5af70c, dc64daef2, fdfbbed78, d46f08d2e, f52ff52e3) until #2792. It also propagated: #2781, a documentation-only Homebrew change, went red on my guard — a PR that cannot reach a threading primitive.

    Structurally identical to the CS0246: MeshOperations incident in the body: no git conflict, nothing DIRTY, each half correct, only the combination broken.

    What #2767 did and did not add

    For the record, so nobody reads it as the fix either:

    One thing today adds to the argument

    A guard is the worst possible change to land without a merge queue. An ordinary semantic conflict breaks the two PRs involved; a new guard converts every other in-flight PR into a break, because it changes what "valid" means repo-wide. The blast radius is not the diff, it is the ruleset — which is why this cost five main runs and reddened an unrelated docs PR rather than one file.

    merge_group: is already carried by MeshWeaver.Plugins, so the mechanism is proven in the fleet. Still a maintainer call; leaving it open for that.

  7. rbuergi commented on Aug 30, 2026

    @rbuergi
    ContributorAuthor

    The merge queue is enabled on main — the structural fix this issue asked for, not a fourth partial one.

    Maintainer decision recorded 2026-08-30 ("yes, merge queue"). Delivered in two steps, in the order the mechanism demands:

    1. ci: Build and Test fires on merge_group, so a merge queue can be enabled (#2412) #2799 (merged afb5a5ec9) — Build and Test fires on merge_group, and both diff gates resolve their base to main on a queue entry (github.base_ref is empty there). Without this an entry queues, times out and is ejected with no failing check to point at.
    2. merge_queue rule added to the main pr protection ruleset (id 2128472) via the API: ALLGREEN, up to 3 entries built / merged, squash, no minimum wait, 60-minute check timeout. The ruleset's other rules are unchanged.

    Verified live, not by the setting:

    query{repository(owner:"Systemorph",name:"MeshWeaver"){mergeQueue(branch:"main"){id}}}
    → {"mergeQueue":{"id":"MQ_kwDOLFKJ6c4AA-bW"}}
    

    (It was null this morning — available, not enabled. This repo is public, so the queue is available on the org's Team plan; MeshWeaver.Plugins carries the same trigger dormant because it is private and needs Enterprise Cloud — its own comment says so.)

    What this closes, precisely

    Every entry is now built on top of the entries ahead of it, so the combination that lands is compiled before it lands — the gap in the body (CS0246: MeshOperations, 2026-08-26) and today's instance (#2782 + #2792, each green alone, red together, five red main runs, a docs-only PR caught in the blast) cannot recur through this path.

    What it does not change: #2447 and #2767 stay, because they solve the other halves — a burst no longer cancels the running main run (#2447) and no longer evicts the queued ones (#2767).

    First real test

    The next PR to merge (gh pr merge now enqueues rather than merges) is the first entry through the queue; if it sits without a check, the merge_group trigger is the first thing to look at, per #2799's comment.

  8. rbuergi commented on Aug 30, 2026

    @rbuergi
    ContributorAuthor

    Closed by the merge queue — #2799, live on main since today

    This issue's requirement was "no CI run ever builds the combination that lands". That is no longer true: core main is behind a GitHub merge queue, so the combination is built before it lands, not after.

    Verified live, not from the ruleset alone — I enqueued #2800 while writing this:

    repository(Systemorph/MeshWeaver).pullRequest(2800)
      isInMergeQueue   : true
      mergeQueueEntry  : { state: QUEUED, position: 1 }
      mergeStateStatus : CLEAN
    

    and the enforcing configuration:

    ruleset 2128472 "main pr protection" (branch, ACTIVE)
      rules: deletion, non_fast_forward, pull_request, copilot_code_review,
             required_status_checks, merge_queue        ← the new one
    

    The queue builds each entry on gh-readonly-queue/main/pr-N-<sha> and waits for the required context (Consolidate test results) on that ref — i.e. on the merged result, which is precisely the artefact this issue said nothing ever compiled. #2799 made Build and Test fire on merge_group and resolve both diff gates' base to main (a queue entry has an empty github.base_ref), which is what makes the entry's run meaningful rather than vacuous.

    Why this closes it and #2447/#2767 did not

    The 2026-08-30 comment above was right to refuse those as the fix, and the distinction holds:

    what it changed
    #2447 main's in-progress run is no longer cancelled — when you find out
    #2767 a queued main run is no longer evicted by the next push — when you find out
    #2799 (queue) the combination is compiled before it lands — whether it lands

    Both incidents in this thread are covered by that: the CS0246: MeshOperations pair (08-26), and #2782 + #2792 on 08-30 — each green alone, red together, five consecutive red main runs, and a documentation-only PR turned red by a guard it could not reach. Under a queue the second entry is built on top of the first and fails in the queue, where it blocks nothing but itself.

    That last point answers the sharpest argument in the reopen note — "a guard is the worst possible change to land without a merge queue, because it changes what valid means repo-wide". It now cannot: a guard is validated against every other entry before any of them merge.

    What this does NOT cover, and where it lives

    • Cross-REPO pairs — deleting public surface here reds a plugin repo's trunk. A merge queue is per-repo and cannot see that. That is Nothing gates a cross-repo PAIR: deleting public surface here reds a plugin repo's trunk, twice today #2689, which stays open and is now the remaining member of this family.
    • MeshWeaver.Plugins has no queue and will not get one by ticking a setting: GitHub merge queues are available to public repos on Team, but a private repo needs Enterprise Cloud. Plugins is private on Team, so its merge_group: trigger is deliberately dormant and measures mergeQueue → null. Do not read that as a misconfiguration.

    Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions