Repository navigation
fix(ci): give every main commit its own concurrency group — cancel-in-progress: false was not enough - #2767
Conversation
…-progress: false was not enough #2412 set `cancel-in-progress: false` for main so merges could not kill each other's runs. That protects the run already EXECUTING. GitHub also keeps exactly ONE PENDING run per concurrency group, and every main push shared the group `build-test-refs/heads/main` — so a third arrival EVICTED the queued one, which then reported `cancelled` having never run a step. Measured 2026-08-30, one merge burst (note run_started_at == created_at even while a run is only queued, which is what makes this read as cancelled mid-run): dca02bd queued 11:42:10 -> cancelled 11:53:02, 1s after the next push 57de73c queued 11:54:57 -> cancelled 12:08:54, 2s after the next push 637ee39 queued 12:08:52 -> cancelled 12:09:16, 1s after the next push Both failures #2412's own comment says it prevents happened anyway: 1. Nothing built the tree that landed. With strict:false each PR is tested against the main it branched from, so the MERGED combination is first compiled by main's own run. Three landed commits never got one. 2. Nothing published. main-cd gates delivery on `Consolidate test results` reaching success FOR THAT SHA; an evicted run never produces one. Three CD runs reported green with every job `skipped` while ACR's newest image stayed several commits behind. Fix: a push to main groups by `github.sha`, so no two main runs share a group and none can be evicted. PRs still group by ref and still supersede (#2316's ~28% saving is untouched). `cancel-in-progress` stays as the statement of intent and for any future ref that groups by name. MainRunsAreNeverCancelledGuard was GREEN through all of this: it asserted the `cancel-in-progress` expression, which was correct and beside the point. It now also evaluates the GROUP expression for two different main commits and requires them to differ — the invariant is about the group, not the flag. Its expression evaluator gains value-faithful `&&`/`||` (Actions returns the OPERAND, not a bool), which is what `group:` uses to select a string. Verified by MUTATION, not by passing: restoring the ref-grouped line makes the new test fail naming the collision; restoring the fix makes it pass (11/11). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the “MeshWeaver Build and Test” workflow’s concurrency grouping so that each push to main uses a unique concurrency group per commit SHA, preventing queued main runs from being evicted (and reported as cancelled) during merge bursts. It also strengthens the guard test to validate the concurrency.group behavior and documents the user-visible impact in a What’s New entry.
Changes:
- Change
dotnet-test.ymlconcurrencygroupsomainpushes group bygithub.sha(PRs still group by ref;workflow_dispatchstill groups byrun_id). - Extend
MainRunsAreNeverCancelledGuardto evaluategroup:and ensure main commits do not share a group while PR commits do. - Add a What’s New entry describing the fix and why it matters.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
test/MeshWeaver.Documentation.Test/MainRunsAreNeverCancelledGuard.cs |
Adds a guard asserting per-SHA grouping on main and updates the evaluator to preserve &&/` |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-every-commit-that-lands-on-main-is-tested.md |
Documents the operational impact and the change for the What’s New feed. |
.github/workflows/dotnet-test.yml |
Adjusts workflow concurrency grouping to avoid queued-run eviction on main while keeping PR superseding behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Correction to the record: this was not a novel finding —
That landed as Plugins #888 on 2026-08-29, driven by its own measurement — 22 of 25 main runs cancelled with Two things worth taking from that:
Filing that as the durable lesson rather than leaving this PR implying it was first. |
|
Follow-up on the "main-cd's push lane still groups by ref" note above: it stays that way, deliberately, and here is why it must NOT get this PR's fix. Observed today: four consecutive But CD's serialization is load-bearing in a way Build-and-Test's was not.
Group CD runs by SHA and two runs for consecutive commits can promote concurrently; whichever finishes last wins So the eviction of a queued CD run is a cost the design accepts, and the reconciler is its remedy — it heals HEAD on the hour, never a superseded commit. Measured today: three complete promoted sets ( |
…led (#2412) (#2799) Toward #2412 — the structural fix, not a third partial one. With strict:false every PR is tested against the main it branched from, so a burst of merges lands a COMBINATION no run ever compiled. Measured twice: `CS0246: MeshOperations` on 2026-08-26, and today a new guard (#2782) plus an independently-landed gate primitive (#2792) that were each green alone and red together — five consecutive red main runs and a documentation-only PR caught in the blast. #2447 and #2767 each changed WHEN you find out; neither builds the combination before it lands. A merge queue does. This repo is PUBLIC, so GitHub's merge queue is available on the org's Team plan (private repos need Enterprise Cloud — MeshWeaver.Plugins carries the same trigger dormant for exactly that reason, and its own comment says so). `mergeQueue(branch:"main")` currently resolves to null: available, not enabled. What this change does: - `merge_group:` trigger, so the queue's temporary gh-readonly-queue/main/… ref gets the required context. Without it an entry queues, times out and is ejected with no failing check to point at. - The two DIFF gates (record signatures, type forwards) run on merge_group too, with base resolved to main — github.base_ref is EMPTY on merge_group, and a queue entry's base IS main by construction. Resolved inline at all four uses so the two gates cannot drift. - Nothing else changes: concurrency groups the queue ref per entry (a superseded entry gets a new ref), the fork gates read null as "not a fork", HEAD_SHA falls through to github.sha, and the executed-and-green marker stays push-only. 🚨 Order matters: this must be ON MAIN before the `merge_queue` rule is added to the `main pr protection` ruleset, or the first queued PR sits forever. Enabling the rule is the next step, done via the ruleset API once this lands. Verified: YAML parses with triggers [push, pull_request, merge_group, workflow_dispatch]; MainRunsAreNeverCancelledGuard 11/11 (the concurrency expressions are untouched). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three commits landed on main today without ever being built or shipped
They report
cancelled, which reads like someone cancelled them. Nobody did — they were evicted while queued.cancel-in-progress: false(#2412) protects the run that is already executing. GitHub also keeps exactly one pending run per concurrency group, and every main push sharedbuild-test-refs/heads/main. So a third arrival evicted the second's queued run, which reportedcancelledwithout running a step.Measured 2026-08-30, one merge burst — note
run_started_at == created_ateven while a run is only queued, which is what makes this look like a mid-run cancellation:dca02bdd757de73c45637ee3921Both failures #2412 exists to prevent happened anyway
strict: falseeach PR is tested against the main it branched from, so the merged combination is first compiled by main's own run. Three landed commits never got one — this is the shape that putCS0246: MeshOperationson main on 2026-08-26.main-cdgates delivery onConsolidate test resultssucceeding for that SHA, which an evicted run never produces. Three CD runs reported green with every jobskippedwhile ACR's newest image stayed several commits behind.The fix
A push to
maingroups bygithub.sha, so no two main runs share a group and none can be evicted — which is exactly the cost #2412's comment says is deliberately accepted ("one full run per merge to main"). PRs still group by ref and still supersede, so #2316's ~28% saving is untouched.The guard was green the whole time
MainRunsAreNeverCancelledGuardasserted that thecancel-in-progressexpression evaluates false on main. That was true, and beside the point — the invariant is "a main run is never cancelled", and eviction violates it without touching that setting. A guard that checks the setting instead of the property is the same defect class this repo keeps finding.It now also evaluates the
groupexpression for two different main commits and requires the results to differ (and for two commits on one PR to match, so superseding is not silently disabled). Its evaluator gains value-faithful&&/||— Actions returns the operand, not a bool, which is precisely howgroup:selects a string.Verified by mutation, not by passing
build-test-refs/heads/mainfor two different commits"(My first mutation attempt silently failed to apply and the suite passed — that "green" proved nothing and was redone. Noting it because it is the same false-pass shape this PR is about.)
Not in scope
main-cd.yml's push lane still groups by ref and can evict its own runs the same way; it already carries a partial fix for this (#2490, the reconcile lane split). Flagging rather than widening.🤖 Generated with Claude Code