From 87e915907e453a629bfa74f2dbb7f50c2555595d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 13:47:27 +0000 Subject: [PATCH 1/2] fix(ci): stop cancelling main's CI so the base branch is actually verified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `concurrency.cancel-in-progress` was a blanket `true`, so every push to main cancelled the previous main run. Measured over the last 30 pushes to main: 23 cancelled (77%), 6 success, 1 failure — main was unverified four times out of five. That is not a cost control, it is a hole in the safety net. Supersession is sound for a PR branch, where a newer head really does replace the work in flight. A merged commit has no newer head; cancelling its run skips no redundant work and simply discards the only verification main ever gets. Three defects from a single afternoon trace to it, each reaching a feature branch as first detection instead of the branch that introduced it: - The Lighthouse baseline drifted ~163ms on desktop /therapy-compass with no alert, so the next PR to complete a Lighthouse run inherited the drift as its own "regression" and was blocked by it. - Two @mockup assertions in ui-tools-search-mode-mockup.spec.ts broke on main and stayed broken until an unrelated PR surfaced them (repaired by #2115). - ci-triage could not baseline a single failure, reporting "Compared with main CI run #12038/#12062/#12087 (cancelled)" three times in one afternoon. Base-branch pushes are now exempt. `on.push.branches` is [main, release/**], so `event_name == 'push'` is precisely "a base-branch push"; pull requests and merge-queue runs keep cancel-in-progress unchanged. The cost is bounded to one concurrent run per merge, each already path-scoped by the `changes` job. A blanket `true` reads as harmless and is not, so `tests/ci-cache-safety.test.ts` pins both the exemption and the push-trigger scoping that gives it its meaning: widening the push trigger to feature branches would silently unscope it. AGENTS.md carried "do not disable cancel-in-progress" as unqualified guidance, which now contradicts the fix; scoped to pull requests, with the measurement and the reason the exemption must not be simplified away. --- .github/workflows/ci.yml | 23 ++++++++++++++++++++--- AGENTS.md | 15 ++++++++++----- tests/ci-cache-safety.test.ts | 31 +++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b9e7622cc..861d6c5347 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,10 +23,27 @@ on: concurrency: # Deliberate one-off runs (workflow_dispatch, weekly schedule) get a per-run group so a # push to the same ref cannot cancel them mid-flight — release-browser-matrix takes up to - # 70 minutes and was repeatedly killed by main churn. Push/PR runs keep the shared ref - # group with cancel-in-progress so superseded heads still stop early. + # 70 minutes and was repeatedly killed by main churn. PR runs keep the shared ref group + # with cancel-in-progress so superseded heads still stop early. group: ${{ github.workflow }}-${{ (github.event_name == 'workflow_dispatch' || github.event_name == 'schedule') && github.run_id || github.ref }} - cancel-in-progress: true + # Base-branch pushes are EXEMPT from supersession. `on.push.branches` is [main, release/**], + # so `event_name == 'push'` is always a base-branch push — a commit that is already merged and + # can never be superseded by a "newer head" the way a PR branch can. Cancelling those runs + # does not save redundant work; it destroys the only verification `main` ever gets. + # + # Measured 2026-08-18 over the last 30 pushes to main: 23 cancelled (77%), 6 success, 1 failure. + # main was therefore unverified four times out of five, and three separate defects reached a + # feature branch as first detection rather than being caught on the branch that introduced them: + # - the Lighthouse baseline drifted ~163ms on desktop /therapy-compass with nobody alerted, + # so the next PR to complete a Lighthouse run inherited it as its own "regression"; + # - two @mockup assertions in ui-tools-search-mode-mockup.spec.ts broke on main and stayed + # broken (later repaired by #2115) after surfacing on an unrelated PR; + # - ci-triage could not baseline a single PR failure, reporting "Compared with main CI run + # #12038/#12062/#12087 (cancelled)" three times in one afternoon. + # + # The cost of this exemption is bounded: concurrent main runs, one per merge, each already + # scoped by the `changes` job. That is strictly cheaper than a defect landing unnoticed. + cancel-in-progress: ${{ github.event_name != 'push' }} permissions: contents: read diff --git a/AGENTS.md b/AGENTS.md index 81a7952bc5..e9d76e06dc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -649,7 +649,7 @@ Durable mitigations in this repo: merely behind and the merge tree is clean, let that run settle and sync once, late, after review/fix work is assembled. Preempt an in-flight run only when the branch is genuinely blocking-conflicted or the user explicitly asks for - an immediate sync; do not disable `cancel-in-progress`. + an immediate sync; do not disable `cancel-in-progress` for PR branches. - The historical review table is frozen during normal PR work. Write a new review with `ledger:append`, which creates an immutable record; never resolve a review conflict by editing the historical table. The repository deliberately leaves its @@ -748,8 +748,8 @@ Before opening a new branch, check whether the task can ride an **already-open P still own** or be bundled with **other currently-queued low-risk work** instead of minting a new one. If the target PR's CI is already running, either wait for it to settle before pushing the addition or assemble every commit before that PR's first push — -`.github/workflows/ci.yml` sets `cancel-in-progress: true`, so a push mid-run cancels and -restarts CI rather than saving an invocation, reproducing the exact cancellation waste +`.github/workflows/ci.yml` cancels in-progress runs for pull requests, so a push mid-run +cancels and restarts CI rather than saving an invocation, reproducing the exact cancellation waste this rule exists to cut (reproduced 2026-07-30 pushing a second commit to PR #1406: the in-flight `static-pr` run was cancelled, failing `pr-required` on the now-stale head). A settle-then-push addition also lands after this repo's one automatic Codex review may @@ -845,8 +845,13 @@ named PR). Future process only. to settle before pushing again. Apply the same settle-first rule to branch syncs: for a behind-but-clean PR with required CI in flight, wait, then perform at most one late `update-branch` / `git merge origin/main` after review and fix work is assembled. - `cancel-in-progress: true` should remain enabled, but every superseding push or sync cancels - Production UI mid-flight (~40% of recent PR CI runs were cancellations). + Cancel-in-progress should remain enabled **for pull requests**, where a newer head genuinely + supersedes the work in flight, but every superseding push or sync cancels Production UI + mid-flight (~40% of recent PR CI runs were cancellations). It is deliberately NOT enabled for + base-branch pushes: a merged commit cannot be superseded, so cancelling there destroyed the + only verification `main` received (23 of the last 30 main pushes cancelled, measured + 2026-08-18). Do not "simplify" that exemption back to a blanket `true` — it is pinned by + `tests/ci-cache-safety.test.ts`. - Before push: `npm run format` **and commit the result**, then `npm run verify:pr-local` (or the smallest gate that covers the change). Format is in `static-pr` but not in `verify:cheap`; an uncommitted format leaves CI red on the pushed diff --git a/tests/ci-cache-safety.test.ts b/tests/ci-cache-safety.test.ts index 1cefe98aad..622ba574b3 100644 --- a/tests/ci-cache-safety.test.ts +++ b/tests/ci-cache-safety.test.ts @@ -203,6 +203,37 @@ describe("CI cache safety", () => { expect(releaseHeader).toContain("needs.changes.outputs.lockfile_changed == 'true'"); expect(releaseHeader).toContain("startsWith(github.ref, 'refs/heads/release/')"); }); + + /* + * Base-branch pushes must never be cancelled by a later merge. + * + * `cancel-in-progress: true` is correct for a PR branch, where a newer head genuinely + * supersedes the work in flight. It is wrong for `main`: that commit is already merged and + * nothing supersedes it, so cancelling does not skip redundant work — it throws away the only + * verification `main` receives. Measured 2026-08-18 across the last 30 pushes to main, 23 were + * cancelled (77%) and only 6 completed, which is how a ~163ms Lighthouse drift on + * desktop /therapy-compass and two broken @mockup assertions both reached feature branches as + * first detection, and why ci-triage kept reporting a cancelled main run as its baseline. + * + * A blanket `true` here reads as a harmless cost control and is not one, so it is pinned with + * its own case rather than left to review. + */ + it("never cancels an in-flight run for a base-branch push", () => { + const concurrency = sourceSegment(workflow, "concurrency:", "permissions:", { + label: "workflow concurrency block", + }); + + expect(concurrency).toContain("cancel-in-progress: ${{ github.event_name != 'push' }}"); + expect(concurrency).not.toContain("cancel-in-progress: true"); + + // `on.push.branches` is what makes `event_name == 'push'` mean "base branch" — if a push + // trigger is ever widened to feature branches, this exemption silently stops being scoped + // and every branch keeps its superseded runs alive. + const pushTrigger = sourceSegment(workflow, " push:", " pull_request:", { + label: "workflow push trigger", + }); + expect(pushTrigger).toContain('branches: [main, "release/**"]'); + }); }); /* From 2859850029b3cf7286b8353349e166d345d42894 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 14:04:20 +0000 Subject: [PATCH 2/2] chore(ledger): record the CI concurrency review --- ...18252cda21f16744ba5373c7933d894c80cba5e425c53430b23.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/b1814fea1580f18252cda21f16744ba5373c7933d894c80cba5e425c53430b23.record.md diff --git a/docs/branch-review-records/b1814fea1580f18252cda21f16744ba5373c7933d894c80cba5e425c53430b23.record.md b/docs/branch-review-records/b1814fea1580f18252cda21f16744ba5373c7933d894c80cba5e425c53430b23.record.md new file mode 100644 index 0000000000..9c76c906a4 --- /dev/null +++ b/docs/branch-review-records/b1814fea1580f18252cda21f16744ba5373c7933d894c80cba5e425c53430b23.record.md @@ -0,0 +1 @@ +| 2026-08-18 | claude/ci-main-verification-blindspot | 63fb976bf9862160a04b9b784cbdec79e4f3f11f | CI concurrency: exempt base-branch pushes from cancel-in-progress | shipped | test:ci-workflows 326 passed; ci-cache-safety 50 passed; check:github-actions passed; check:ci-scope passed; prettier clean |