Repository navigation
ci: cancel superseded main runs — 28% of runner demand tested dead commits - #2316
Conversation
…mmits
`concurrency.group` was `run_id` for pushes, so every main push got its own group
and no main run ever superseded another. During a merge marathon that is close to
pure waste.
MEASURED over a 3.6 h window on 2026-08-25: 19 of 21 main runs were obsolete
before they finished. Merges landed 1–120 SECONDS apart, and each one spawned a
full ~70-runner-minute suite against a commit the next merge had already
replaced — ≈1,199 runner-minutes, ~28% of ALL runner demand in that window,
spent testing trees nothing would ever look at again.
That waste is not free, because the runner pool has a hard ceiling: the org is on
the Team plan (60 concurrent hosted jobs) and that day's volume doubled to 1,092
workflow runs. Above the ceiling a queue hop costs 25 MINUTES; below it, 3
seconds. Median run went 19 min (08-08 … 08-24, flat) to 46 min on 08-25 with the
WORK unchanged — build 453→429 s and longest shard 612→658 s across those days.
The whole regression was queue, so removing demand is the only thing that helps.
Both PR and push now group by `github.ref`; only `workflow_dispatch` keeps
`run_id` and never cancels, because a manual dispatch is a deliberate act (it is
the hand-bisect path below) and must not die to an unrelated push.
Safe for delivery, which is what the old comment was protecting:
* the NEWEST run is never the one cancelled, so main's tip always completes —
and the tip is exactly what main-cd's reconciler resolves and gates on;
* main-cd no longer needs a per-commit workflow_run: its hourly reconciler asks
whether main's tip has a complete image set and publishes if not, and
publication is batched anyway, so a superseded commit was never getting its
own image;
* a cancelled run writes no refs/ci-green marker — correct, since the tree it
would vouch for is one no future run will test;
* a cancelled run still fires main-cd's workflow_run, which takes the
`⛔ nothing will be built` branch: a step-summary line, no issue filed. Issue
filing lives only in the hourly reconcile path, so this cannot spam.
COST, deliberately accepted (maintainer, 2026-08-26): bisect resolution. A red
main tip now covers a whole merge batch instead of one commit. Every PR was still
tested as refs/pull/N/merge, so what goes untested is specifically the
INTERACTION between PRs merged seconds apart — real, and bisected by hand with
`gh workflow run "MeshWeaver Build and Test" --ref <sha>`. The comment records
the exact two lines that revert this.
No What's New entry: CI-only, no user-visible change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the “MeshWeaver Build and Test” GitHub Actions workflow to cancel superseded in-flight runs for push to main, reducing queued runner demand during high-merge periods while preserving manual bisect capability via workflow_dispatch.
Changes:
- Switches workflow-level
concurrency.groupto usegithub.reffor both PR andmainpush runs, so newer runs supersede older ones. - Enables
cancel-in-progressfor all non-workflow_dispatchevents, ensuring manual dispatch runs are never cancelled. - Expands and refreshes in-file rationale documenting the tradeoff (queue relief vs. bisect granularity) and the revert lines.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test Results (shard 3) 9 files 9 suites 5m 43s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Test Results (shard 5) 9 files 9 suites 5m 59s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Test Results (shard 0)593 tests 589 ✅ 17m 13s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Test Results (shard 1)2 046 tests 1 853 ✅ 8m 31s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Test Results (shard 4)2 064 tests 2 061 ✅ 14m 13s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Test Results (shard 2)2 831 tests 2 728 ✅ 9m 14s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Test Results 53 files 53 suites 1h 0m 55s ⏱️ Results for commit 80df02a. ♻️ This comment has been updated with latest results. |
Review caught that the first version asserted only that the value MENTIONED
`refs/heads/main` — so `${{ github.ref == 'refs/heads/main' }}` passed it while
cancelling on main, the exact inverse of the intent. A substring check is not a
semantic check, and a gate that cannot fail on the thing it exists to catch is
not a gate.
`Evaluate` is now a small recursive-descent evaluator for the slice of the
GitHub expression language a `cancel-in-progress` value can use (`&&`, `||`,
`!`, parentheses, `==`/`!=`, string literals, `true`/`false`, context lookups),
with Actions' precedence and truthiness. Anything outside that subset THROWS
rather than guessing — an unparseable expression must fail loudly, never
silently evaluate to "fine".
Both halves of the intent are now pinned, not just one: cancelling must be OFF
for a push to `refs/heads/main`, and must stay ON for a `pull_request` run.
Silently disabling superseding everywhere would hand back #2316's ~28% runner
saving with nobody noticing, which the previous assertion could not see.
The evaluator carries its own self-test rows, including the counterexample from
review and the `||`-instead-of-`&&` shape a rewrite would most plausibly reach
for.
Verified RED on five regression shapes, each with the right message: the
pre-fix expression, a bare `true`, the inverted `==` counterexample, the
disjunct variant, and a bare `false` (which trips the PR-side assertion).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ot QUEUED ones #2447 exempted `main` from cancel-in-progress, which is right and necessary — and it does not finish the job. `cancel-in-progress: false` protects a run that is ALREADY RUNNING. GitHub still keeps only ONE pending run per concurrency group and supersedes the older PENDING one whatever that setting says. That distinction is invisible while runners are free — a run starts in seconds, so "queued" barely exists — and decisive when they are not. On 2026-08-26 the shared 60-job pool was saturated by a sibling repo (12 concurrent runs in MeshWeaver.Plugins). Main's runs never reached in-progress; they sat pending, and each merge cancelled the previously pending one. `a26fc6d` — the very commit that merged the never-cancel fix — went pending -> cancelled WITHOUT EVER RUNNING, as did `cd97bde` and four before them. Consequence: six consecutive main commits produced no completed Build-and-Test, so main-cd's gate stayed `skipped` (`gate=skipped reason= publish= complete= green= sha=`), every CD run reported success having published nothing, and no image shipped for hours while production served ~11-hour-old code against newer landed modules. Grouping `main` by `github.sha` gives each commit its own group, so nothing can supersede it — queued or running — and CD always gets a completed run to gate on. PR runs keep grouping by ref: superseding there is correct, and it is where #2316's ~28% runner saving comes from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-progress: false was not enough (#2767) #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>
Requested by @orwell2000 after the CI timing review. Companion to #2313 — that one fixes ~168 s of work; this one is the change that actually relieves the queue.
The measurement
concurrency.groupwasrun_idfor pushes, so every main push got its own group and no main run ever superseded another.Over a 3.6 h window on 2026-08-25: 19 of 21 main runs were obsolete before they finished. Merges landed 1–120 seconds apart, each spawning a full ~70-runner-minute suite against a commit the next merge had already replaced — ≈1,199 runner-minutes, ~28% of all runner demand in that window, spent testing trees nothing would ever look at again.
That waste is not free, because the pool has a hard ceiling — the org is on the Team plan (60 concurrent hosted jobs) and that day's volume doubled:
Above the ceiling a queue hop costs 25 minutes; below it, 3 seconds. And the work did not change — this is the decisive table:
Median run was flat at 19–20 min from 08-08 through 08-24 — straight through modularization — then 46 min on 08-25. The regression is entirely queue, so removing demand is the only thing that helps.
The change
Both PR and push group by
github.ref(refs/pull/N/mergeandrefs/heads/main). Onlyworkflow_dispatchkeepsrun_idand never cancels — a manual dispatch is a deliberate act (it is the hand-bisect path) and must not die to an unrelated push.Why it is safe for delivery
That is what the old comment was protecting, and each leg was checked:
cancel-in-progresscancels the older group members. main's tip always completes, and the tip is exactly what main-cd's reconciler resolves and gates on.workflow_run— its hourly reconciler (cron: 23 * * * *) asks whether main's tip has a complete image set and publishes when it does not. Publication is batched anyway (CD_BATCH_WINDOW_MINUTES), so a superseded commit was never getting its own image.refs/ci-greenmarker from a cancelled run — correct: the tree it would vouch for is one no future run will test.workflow_run, which takes the⛔ … nothing will be builtbranch — a step-summary line only.gh issuefiling lives exclusively in the hourly reconcile path, so 19 cancellations/day cannot produce 19 issue comments. (Residual: a seconds-wide race where the reconciler resolves tip T and a push then cancels T's run could produce one "cannot publish" comment, self-healing on the next tick. Once an hour at worst.)What it costs — deliberately accepted
Bisect resolution. A red main tip now covers a whole merge batch instead of one commit. Every PR is still tested as
refs/pull/N/mergebefore merging, so what goes untested is specifically the interaction between PRs merged seconds apart — which is real. That is bisected by hand:which is why
workflow_dispatchis deliberately exempt from cancellation. The comment in the file records the exact two lines that revert this if the trade stops being worth it.Verification
>-scalar preserves deeper-indented continuation lines literally, which would have silently changed the string — caught and avoided, and the reason it is one line).dotnet build -c ReleaseonMeshWeaver.Documentation.Test→0 Error(s); the three guards that read.github/workflows/*.ymlpass 14/14 on a fresh.trx.No What's New entry — CI-only, no user-visible change.
Full analysis: https://claude.ai/code/artifact/427961ea-1f64-4a02-aadf-966a19b4625b
🤖 Generated with Claude Code