ci: cancel superseded runs instead of banking one per commit

.github/workflows/build.yml

@@ -88,37 +88,24 @@ env:
   CACHE_IMAGE: ${{ vars.VOGT_BUILDKIT_CACHE_REGISTRY }}/vogt-buildcache
 
 concurrency:
-  # Keyed by *commit*, not by ref, since 2026-08-19.
+  # Newest-wins, by ref: a superseded build — pending or in progress — is
+  # cancelled when a newer push joins its group. Only the newest commit of a
+  # burst gets images, and that is the commit anything would deploy; a
+  # superseded commit's missing `sha-` image costs nothing, because nothing
+  # pins a digest it was never offered. The cost this accepts is the
+  # burst-tail gap: while a merge train runs, the branch has no *fresh*
+  # completed image until the final run lands — the previous per-commit
+  # keying banked a full build per merge to avoid that, and buried the small
+  # self-hosted pool instead.
   #
-  # "Only the newest commit on a branch is worth an image" is true of a repo
-  # that merges slowly. This one merged to main 23 times in 24 hours — a
-  # median gap of 16 minutes — while this workflow took a median of 19 minutes
-  # and a p90 of 41. Every build was therefore superseded before it finished:
-  # 12 of the last 13 main builds were cancelled, 36% of the repository's
-  # entire CI compute was spent on runs that were thrown away, and main
-  # frequently had no `sha-` image at all. Nothing was left for the estate
-  # workflow to pin, so shipping a fix meant waiting for a quiet window.
-  #
-  # Keying by SHA means no two commits share a group and none can supersede
-  # another — the same reasoning `ci.yml` already records, and for the same
-  # reason. It costs runner time on a burst, which is what the capacity freed
-  # by lifting the runners' four-core cap is for.
-  #
-  # The ref is in the key *as well*, and by SHA alone this is wrong now.
-  # Releases move `dev` → `main` → `prod` by fast-forward, so all three refs
-  # carry the identical commit and a SHA-only group put three different
-  # streams in one. It is not hypothetical: pushing `main` during the prod
-  # cutover cancelled `dev`'s build of the same commit, and `cancel-in-progress`
-  # did not save it — that setting governs runs already *in progress*, while a
-  # still-*pending* run is evicted whenever a newer run joins its group,
-  # always. On a small self-hosted pool almost every run is pending for a
-  # while. `ci.yml` records finding the same hole the same way.
-  #
-  # Each stream builds its own images (`dev-`, `sha-`, `prod-`), so a build
-  # cancelled by a sibling ref is an image that never exists — which for
-  # `prod` means nothing for the estate workflow to pin.
-  group: build-${{ github.ref }}-${{ github.sha }}
-  cancel-in-progress: false
+  # The ref stays in the key, and that part is not optional: releases move
+  # `dev` → `main` → `prod` by fast-forward, so the refs carry the identical
+  # commit, and a ref-free group let a `main` push during the prod cutover
+  # evict `dev`'s build of the same commit. Each stream builds its own
+  # images (`dev-`, `sha-`, `prod-`), so a build cancelled by a sibling ref
+  # is an image that never exists.
+  group: build-${{ github.ref }}
+  cancel-in-progress: true
 
 jobs:
   # Docker Hub is not on the critical path any more (#33): both Dockerfiles

.github/workflows/ci.yml

@@ -34,37 +34,26 @@ on:
   workflow_dispatch:
 
 concurrency:
-  # Keyed by *commit* for a push, and by pull request for a PR.
+  # Newest-wins, by ref: a superseded run — pending or in progress — is
+  # cancelled when a newer one joins its group, so a merge burst costs one
+  # full run instead of a bank of queued ones.
   #
-  # The first version of this keyed pushes by ref and set `cancel-in-progress`
-  # to false for them, on the reasoning below — and that is not sufficient,
-  # which was found by watching it fail rather than by reading the docs
-  # again. `cancel-in-progress` governs runs that are *in progress*. A run
-  # that is still **pending** is cancelled whenever a newer run joins its
-  # group, always, whatever that setting says. On a single self-hosted runner
-  # almost every run is pending for a while, so the hole stayed open and the
-  # workflow claimed it was shut.
+  # Cancelling is only safe because a run's coverage is fully contained in
+  # the run that replaces it. A pull request classifies against the merge
+  # base, so that was always true of PRs. A push is now classified as a
+  # change to *everything* (the classify step below), which restores the
+  # same property: the previous design classified a push by `before..sha`,
+  # and a cancelled incremental run meant its files were checked by nothing,
+  # ever — a lint error reached `dev` through exactly that gap twice. If
+  # push classification ever becomes incremental again, this key must go
+  # back to per-commit in the same change.
   #
-  # Keying by SHA means no two pushed commits ever share a group, so none can
-  # supersede another. It costs runner time on a burst — every commit is
-  # checked, which is the point.
-  # The ref is in the key too, for the reason `build.yml` records at length:
-  # `dev` and `main` carry the same commit once a release fast-forwards, so a
-  # SHA-only group let one branch's push evict the other's pending run — and a
-  # push run that never happens is a commit NFR-C1's path gating never checks.
-  group: ci-${{ github.workflow }}-${{ github.ref }}-${{ github.event.pull_request.number || github.sha }}
-  # Why any of this matters: NFR-C1's path gating classifies a push by
-  # `before..sha`, so each commit is checked by exactly one run and no later
-  # run ever looks at it again. Lose that run and those files are not
-  # "checked later" — they are checked never. It has happened twice:
-  # `tests/test_deploy.py` changed in one push whose run was superseded, and
-  # a lint error reached `dev` through the gap; then a `ruff format` failure
-  # went the same way while the supposed fix was in place.
-  #
-  # A pull request is different — its runs classify against the merge base,
-  # so a later run covers everything an earlier one would have, and
-  # cancelling the superseded one loses nothing.
-  cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+  # The ref stays in the key: `dev` and `main` carry the same commit once a
+  # release fast-forwards, and a SHA-free, ref-free group would let one
+  # branch's push evict the other's run. A PR groups by its own
+  # `refs/pull/N/merge` ref, so PRs never evict each other or a branch.
+  group: ci-${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
 
 permissions:
   contents: read
@@ -110,8 +99,6 @@ jobs:
         env:
           EVENT_NAME: ${{ github.event_name }}
           BASE_REF: ${{ github.base_ref }}
-          BEFORE: ${{ github.event.before }}
-          SHA: ${{ github.sha }}
         run: |
           set -euo pipefail
 
@@ -125,20 +112,23 @@ jobs:
             } >> "$GITHUB_OUTPUT"
           }
 
-          if [ "$EVENT_NAME" = "pull_request" ]; then
-            git fetch --no-tags origin "$BASE_REF"
-            base="$(git merge-base "origin/$BASE_REF" HEAD)"
-            range="$base..HEAD"
-          elif [ -z "$BEFORE" ] \
-            || [ "$BEFORE" = "0000000000000000000000000000000000000000" ] \
-            || ! git cat-file -e "$BEFORE^{commit}" 2>/dev/null; then
-            echo "no usable base commit; treating as a change to everything"
+          if [ "$EVENT_NAME" != "pull_request" ]; then
+            # A push checks everything, always. The concurrency group is
+            # newest-wins per ref, so a superseded push run is cancelled —
+            # which is only safe because the replacing run re-checks the
+            # whole tree. Classifying a push by `before..sha` under that
+            # policy would mean a cancelled run's files are checked by
+            # nothing, ever; the two escapes that found that hole are
+            # recorded in the concurrency comment at the top of this file.
+            echo "push: newest-wins concurrency, so every push checks everything"
             everything
             exit 0
-          else
-            range="$BEFORE..$SHA"
           fi
 
+          git fetch --no-tags origin "$BASE_REF"
+          base="$(git merge-base "origin/$BASE_REF" HEAD)"
+          range="$base..HEAD"
+
           files="$(git diff --name-only "$range")"
           echo "changed files:"
           echo "$files"

.github/workflows/codeql.yml

@@ -28,8 +28,11 @@ jobs:
     runs-on: [self-hosted]
     timeout-minutes: 45
     concurrency:
+      # Newest-wins: an analysis of a superseded commit is fully covered by
+      # the analysis that replaces it — CodeQL scans the whole tree, so
+      # cancelling loses nothing on any event.
       group: codeql-${{ github.ref }}-${{ matrix.language }}
-      cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+      cancel-in-progress: true
     permissions:
       contents: read
       security-events: write

tests/test_deploy.py

@@ -1040,37 +1040,40 @@ def test_the_opencode_registration_does_not_freeze_an_endpoint() -> None:
     )
 
 
-def test_no_two_pushed_commits_share_a_concurrency_group() -> None:
-    """NFR-C1's path gating makes a lost run lose coverage permanently.
-
-    A push is classified by `before..sha`, so each commit is checked by
-    exactly one run and no later run looks at it again. Lose that run and
-    those files are not checked later — they are checked never.
-
-    **`cancel-in-progress: false` is not enough, and believing it was cost a
-    second escape.** That setting governs runs which are *in progress*; a run
-    still **pending** is cancelled whenever a newer run joins its group,
-    unconditionally. On a single self-hosted runner nearly every run is
-    pending for a while, so a `ruff format` failure slipped through exactly
-    as the previous failure had, while the workflow carried a comment saying
-    the hole was shut.
-
-    Keying a push by its commit is what closes it: no two pushed commits
-    share a group, so none can supersede another. A pull request still
-    cancels its own superseded runs, which loses nothing — those classify
-    against the merge base.
+def test_newest_wins_ci_is_paired_with_full_classification_on_push() -> None:
+    """Cancelling a run is safe only when its replacement covers its ground.
+
+    CI is newest-wins per ref: a superseded run — pending or in progress —
+    is cancelled when a newer one joins its group, so a merge burst costs
+    one run instead of a bank of queued ones. That is only sound because a
+    push run classifies as a change to *everything*: under the earlier
+    incremental `before..sha` classification, each commit was examined by
+    exactly one run, and a cancelled run's files were checked by nothing,
+    ever — two failures escaped to `dev` through that gap. These two
+    properties must move together, so they are asserted together.
     """
     raw = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8")
     group = re.search(r"^  group: (.+)$", raw, re.MULTILINE)
     assert group, "ci.yml declares a concurrency group"
-    assert "github.sha" in group.group(1), (
-        f"pushes must be keyed by commit, not by ref: {group.group(1)!r}. Two "
-        "commits in one group means the older run can be cancelled while "
-        "pending, and its files are then checked by nothing, ever"
+    assert "github.ref" in group.group(1), (
+        "the ref keeps `dev` and `main` — the same commit after a "
+        "fast-forward release — from evicting each other's runs"
     )
-    assert "pull_request.number" in group.group(1), (
-        "a pull request still groups by PR, so its superseded runs can be "
-        "cancelled — they classify against the merge base and lose nothing"
+    assert "cancel-in-progress: true" in raw, (
+        "newest-wins is the policy: superseded runs are cancelled, not banked"
+    )
+    assert "github.sha" not in group.group(1), (
+        "per-commit keying banks a run per push; if that is being restored, "
+        "push classification may become incremental again in the same change"
+    )
+    classify = _without_comments(raw)
+    assert '"$BEFORE' not in classify, (
+        "a push must not classify by `before..sha`: newest-wins cancellation "
+        "would leave a superseded run's files checked by nothing, ever"
+    )
+    assert "everything" in classify, (
+        "the classify step's push arm must declare a change to everything, "
+        "which is what makes cancelling a superseded push run lose nothing"
     )
 
 
@@ -1229,17 +1232,15 @@ def test_the_gate_fails_on_anything_that_is_not_success_or_skipped() -> None:
 
 
 def test_a_workflow_may_cancel_only_when_a_later_run_covers_the_same_ground() -> None:
-    """The rule that tells `docs.yml` apart from `ci.yml`.
-
-    Both cancel superseded runs on a branch and only one of them can afford
-    to. `docs.yml` checks the whole tree every time, so a later run covers
-    everything a superseded one would have. `ci.yml` classifies a push by
-    `before..sha` — each commit is examined by exactly one run, and a lost
-    run is coverage lost permanently.
-
-    Asserted because the two files look the same at the point where they
-    differ, and the difference cost two escaped failures before it was
-    understood (§6.3 finding 19).
+    """A workflow that cancels superseded runs must re-cover their ground.
+
+    `docs.yml` checks the whole tree every time, so a later run covers
+    everything a superseded one would have — which is what makes its
+    cancellation safe, and the property the comment in the file exists to
+    protect. `ci.yml` now holds the same property by classifying every push
+    as a change to everything (asserted in the newest-wins test above);
+    historically it classified by `before..sha`, and cancelling under that
+    regime cost two escaped failures (§6.3 finding 19).
     """
     docs = (WORKFLOWS / "docs.yml").read_text(encoding="utf-8")
     assert "whole tree" in docs, (