From 0bfc6090e3c4056f5f836eb64907256d7a562d6e Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Tue, 1 Sep 2026 10:47:17 -0400 Subject: [PATCH 1/2] ci: gate CI on the release commit's subject, not a keyword scan The release commit carried the CI-skip keyword so its own push would not retrigger the workflows on main. GitHub scans the WHOLE head-commit message for that keyword, body included, and a squash merge concatenates every commit message on the branch into the body. So any PR that merely discusses the keyword in prose disables every workflow for its merge commit. That is not hypothetical: it is what #89 did to itself. Its commits explained why the release commit carries the keyword, so the squash body contained the string four times and GitHub suppressed Release, PR checks and Bench for 4d6b57c. `gh run list --commit 4d6b57c` returns nothing at all -- no failure, no skipped run, no record -- so the merge that fixed the release looked identical to a merge that released. Five days and four releasable merges (#86, #87, #76, #89) sat unpublished before anyone noticed npm was stale. Each workflow's root job -- release.yml's `build`, pr-checks.yml's `detect-changes`, bench.yml's `gate` -- is now gated on github.event_name != 'push' || !startsWith(github.event.head_commit.message, 'chore(release): publish') and the release commit's message loses the keyword. Anchoring on the subject cannot be tripped by prose in a body: a commit can now discuss the mechanism, as this one does, without disabling CI. Every other job in all three workflows hangs off its root job and none use always(), so a skipped root skips the run. The trade is that a run is now created and skipped rather than never created, which is the point -- a skipped run is visible in the UI, an absent one is not. This makes the release commit's SUBJECT load-bearing across four files. Flagged at the `git commit` line, at each guard, and in tools/release/README.md. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/bench.yml | 10 +++++++++ .github/workflows/pr-checks.yml | 12 +++++++++++ .github/workflows/release.yml | 37 ++++++++++++++++++++++++++------- tools/release/README.md | 26 +++++++++++++++++++---- 4 files changed, 74 insertions(+), 11 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index e22971a4..96e49481 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -42,6 +42,16 @@ permissions: jobs: gate: + # Skip the release workflow's own version commit. Benching it would measure + # its parent's wasm a second time and seed a duplicate baseline, and there + # are no dist artifacts to wait for anyway: pr-checks skips that commit too. + # + # Subject-anchored rather than the `[skip ci]` this replaces -- GitHub's + # keyword scan reads the whole commit message, body included. See + # release.yml's build job for what that cost. + if: >- + github.event_name != 'push' + || !startsWith(github.event.head_commit.message, 'chore(release): publish') runs-on: ubuntu-latest # Generous because this job WAITS for the PR checks wasm builds to finish # (poll deadline 90 min below) before handing over to the bench. diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index ae18d07f..43489768 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -50,6 +50,18 @@ permissions: jobs: detect-changes: + # Skip the release workflow's own version commit -- it changes only + # package.json versions, CHANGELOGs and the lockfile, and was built and + # tested as its parent. Every job below needs this one, so skipping it + # skips the run. + # + # Subject-anchored rather than the `[skip ci]` this replaces: GitHub's + # keyword scan reads the whole commit message, so a squash merge whose body + # merely QUOTED `[skip ci]` disabled every workflow on main (PR #89, which + # is how a release fix shipped no release). See release.yml's build job. + if: >- + github.event_name != 'push' + || !startsWith(github.event.head_commit.message, 'chore(release): publish') # Decide what this run needs to do. Two outputs: # packages — what to build (and therefore what the test job can rely # on). dicom-codec's integration tests decode through EVERY sibling diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 53b67468..0484f809 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -59,6 +59,25 @@ permissions: jobs: build: + # Do not react to this workflow's OWN version commit. Every job here hangs + # off this one, so skipping it skips the run. + # + # This used to be a `[skip ci]` in the release commit message, and that is + # why PR #89 -- the one that fixed the release -- silently released nothing. + # GitHub scans the WHOLE head-commit message for the keyword, body included, + # and a squash merge concatenates every commit message on the branch into + # that body. #89's commits explained why the release commit carries + # `[skip ci]`, so the merge commit contained the string four times, in + # prose, and GitHub suppressed all three workflows for it. No run was + # created, so there was nothing to notice: no failure, no skipped run, just + # a merge that quietly produced no release for five days. + # + # startsWith on the subject cannot be tripped that way. Prose about the + # release commit is not the release commit, and a commit message can now + # discuss `[skip ci]`, or this guard, without disabling CI. + if: >- + github.event_name != 'push' + || !startsWith(github.event.head_commit.message, 'chore(release): publish') # Same wasm build as pr-checks.yml, minus the change detection: a release # publishes whatever moved, and dicom-codec ships ranges over every sibling, # so all dists must be current. @@ -315,7 +334,11 @@ jobs: # release-plan.json is a run artifact, not repo content. git add packages/*/package.json packages/*/CHANGELOG.md pnpm-lock.yaml - git commit -m "chore(release): publish [skip ci]" + # The subject is load-bearing: every workflow's root job is gated on + # `startsWith(head_commit.message, 'chore(release): publish')` so this + # push does not retrigger CI. Change the wording here and you must + # change it in release.yml, pr-checks.yml and bench.yml together. + git commit -m "chore(release): publish" jq -r '.[].tag' release-plan.json | while read -r tag; do git tag -a "$tag" -m "$tag" @@ -327,12 +350,12 @@ jobs: # rather than persisted, because checkout was told not to keep them in # .git/config while `pnpm install` runs. # - # The commit message carries [skip ci], and it is load-bearing on BOTH - # of the routes that can actually push. GitHub suppresses workflow runs - # only for pushes made with GITHUB_TOKEN; an App-token push and a - # deploy-key push are both ordinary pushes and WOULD retrigger this - # workflow on main. [skip ci] is what stops that being a release loop, - # so do not remove it -- on either route. + # This push DOES retrigger the workflows on main, on both routes that + # can reach here: GitHub suppresses runs only for pushes made with + # GITHUB_TOKEN, and an App-token push and a deploy-key push are both + # ordinary pushes. What stops that being a release loop is the + # `chore(release): publish` guard on each workflow's root job, not + # anything about this push -- see the build job above. # # --atomic: all refs land or none do. Without it git updates each ref # independently, and the 2026-08-24 run (32733067241) showed what that diff --git a/tools/release/README.md b/tools/release/README.md index 90c7d72e..8af74b62 100644 --- a/tools/release/README.md +++ b/tools/release/README.md @@ -55,7 +55,7 @@ token runs nothing but `npm`, the pinned actions and `publish-order.mjs` (node b release before anything is committed, tagged or published), then `version.mjs`, then regenerates `pnpm-lock.yaml` (pnpm records each importer's *specifier*, so rewriting dicom-codec's ranges strands the lockfile and the next `--frozen-lockfile` install fails), commits - `chore(release): publish [skip ci]` and one annotated tag per released package, and pushes to + `chore(release): publish` and one annotated tag per released package, and pushes to `main` with a token minted per-run from the release App. The token is passed to `git push` in the remote URL rather than persisted into `.git/config` by `actions/checkout`, so it is not sitting on disk while `pnpm install` runs. The job outputs the pushed commit SHA. @@ -206,9 +206,27 @@ stale reviews dismissed on push, last-push approval, no force pushes, no branch script's header for why the classic rule has to go rather than sit alongside the ruleset. One behavioural note that applies to both routes: GitHub suppresses workflow runs only for pushes -made with `GITHUB_TOKEN`. An App-token push and a deploy-key push are both ordinary pushes and -*would* retrigger the release workflow on `main`. The `[skip ci]` in the release commit message is -what prevents a loop — do not remove it, whichever route you set up. +made with `GITHUB_TOKEN`. An App-token push and a deploy-key push are both ordinary pushes, so the +release's own version commit *does* retrigger the workflows on `main`. + +What stops that being a loop is the guard on each workflow's root job: + +```yaml +if: >- + github.event_name != 'push' + || !startsWith(github.event.head_commit.message, 'chore(release): publish') +``` + +So the release commit's **subject line is load-bearing**. `release.yml` writes it, and `release.yml`, +`pr-checks.yml` and `bench.yml` all match on it — change the wording in one place and you must change +it in all four. + +This deliberately does not use `[skip ci]`. GitHub scans the entire head-commit message for that +keyword, body included, and a squash merge concatenates every commit message on the branch into the +body — so a PR that merely *mentions* `[skip ci]` in prose disables every workflow for its merge +commit, creating no run at all to notice. That is exactly what happened to +[#89](https://github.com/cornerstonejs/codecs/pull/89), whose commits explained why the release +commit carried the keyword. Anchoring on the subject cannot be tripped by prose. 3. **Verify**, then re-run the failed Release workflow: From eec46d751bedb515f58c4a732b8a1da2d38d4f55 Mon Sep 17 00:00:00 2001 From: Bill Wallace Date: Tue, 1 Sep 2026 10:57:58 -0400 Subject: [PATCH 2/2] ci: match the release commit by identity as well as subject Review finding on #90: the guards matched the release commit by subject prefix alone, so any push to main whose subject opened with `chore(release): publish` would skip the release, the checks and the bench. The suggested fix was an exact `==` on the message. Taking the concern but not that fix, for two reasons. It does not actually close the hole. `==` still matches on message content alone, and a human can type the exact subject as easily as a prefix -- the scenario is no less reachable, only narrower by one character class. And it introduces a silent failure. `github.event.head_commit.message` is the whole message, not the subject; GitHub does strip the trailing newline (checked against the API for 6635578, which returns "chore(release): publish [skip ci]" with nothing after it), so equality matches today. But it stops matching the first time the release commit grows a body -- a commit template, a prepare-commit-msg hook, someone adding a second `-m`. Nothing enforces that invariant, and when it breaks the release commit gets benched and seeds a duplicate CodSpeed baseline. Quiet wrong numbers is the failure mode this repo is least equipped to notice. So the guards now require the subject prefix AND the committer identity: && github.event.head_commit.author.email == '41898282+github-actions[bot]@users.noreply.github.com' Only the release job's `git config` can produce that, so a human commit sharing the subject no longer skips anything -- strictly narrower than the exact match in the direction that matters, and still tolerant of message reformatting in the direction that is fragile. Noted at the `git config` step that the address is load-bearing, and documented in tools/release/README.md. Also fixed a YAML bug in the original guards: the continuation line was indented deeper than the first, and a `>-` folded scalar preserves a newline before a more-indented line, so the expression reached GitHub with a literal newline in it. All three now fold to a single line, verified by parsing. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/bench.yml | 8 +++++--- .github/workflows/pr-checks.yml | 7 +++++-- .github/workflows/release.yml | 31 +++++++++++++++++++++++++----- tools/release/README.md | 34 ++++++++++++++++++++++----------- 4 files changed, 59 insertions(+), 21 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 96e49481..da7d4e0a 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -47,11 +47,13 @@ jobs: # are no dist artifacts to wait for anyway: pr-checks skips that commit too. # # Subject-anchored rather than the `[skip ci]` this replaces -- GitHub's - # keyword scan reads the whole commit message, body included. See - # release.yml's build job for what that cost. + # keyword scan reads the whole commit message, body included. The author + # clause keeps a human commit that happens to open with the same subject + # from skipping the bench. See release.yml's build job for what that cost. if: >- github.event_name != 'push' - || !startsWith(github.event.head_commit.message, 'chore(release): publish') + || !(startsWith(github.event.head_commit.message, 'chore(release): publish') + && github.event.head_commit.author.email == '41898282+github-actions[bot]@users.noreply.github.com') runs-on: ubuntu-latest # Generous because this job WAITS for the PR checks wasm builds to finish # (poll deadline 90 min below) before handing over to the bench. diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 43489768..f6cd4c3d 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -58,10 +58,13 @@ jobs: # Subject-anchored rather than the `[skip ci]` this replaces: GitHub's # keyword scan reads the whole commit message, so a squash merge whose body # merely QUOTED `[skip ci]` disabled every workflow on main (PR #89, which - # is how a release fix shipped no release). See release.yml's build job. + # is how a release fix shipped no release). The author clause keeps a human + # commit that happens to open with the same subject from skipping CI. See + # release.yml's build job for the full reasoning. if: >- github.event_name != 'push' - || !startsWith(github.event.head_commit.message, 'chore(release): publish') + || !(startsWith(github.event.head_commit.message, 'chore(release): publish') + && github.event.head_commit.author.email == '41898282+github-actions[bot]@users.noreply.github.com') # Decide what this run needs to do. Two outputs: # packages — what to build (and therefore what the test job can rely # on). dicom-codec's integration tests decode through EVERY sibling diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0484f809..8cc29a21 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -75,9 +75,25 @@ jobs: # startsWith on the subject cannot be tripped that way. Prose about the # release commit is not the release commit, and a commit message can now # discuss `[skip ci]`, or this guard, without disabling CI. + # + # The author clause is what makes the match precise. Anchoring on the + # subject alone would let any commit whose subject opens with that prefix + # skip CI, and an exact `==` on the message does not fix that either -- a + # human can type the exact subject just as easily as a prefix. Identity can + # only be produced by the `git config` two steps below, so the guard now + # asks the question it actually means: is this OUR version commit? + # + # Deliberately not `==` on the message. GitHub does strip the trailing + # newline (verified against the API for 6635578), so equality would work + # today -- but it silently stops matching the moment the release commit + # grows a body, which a commit template, a prepare-commit-msg hook or a + # second `-m` would do. That failure is invisible and lands in the worst + # place: the release commit gets benched, seeding a duplicate CodSpeed + # baseline. Prefix + identity fails safe where equality fails silent. if: >- github.event_name != 'push' - || !startsWith(github.event.head_commit.message, 'chore(release): publish') + || !(startsWith(github.event.head_commit.message, 'chore(release): publish') + && github.event.head_commit.author.email == '41898282+github-actions[bot]@users.noreply.github.com') # Same wasm build as pr-checks.yml, minus the change detection: a release # publishes whatever moved, and dicom-codec ships ranges over every sibling, # so all dists must be current. @@ -311,6 +327,11 @@ jobs: fi echo "Pushing via: $PUSH_VIA" + # This email is load-bearing, not cosmetic: it is the second half of + # the guard on every workflow's root job (the first is the subject + # below). Change it here and the version commit stops being + # recognised as ours, so the release commit gets a full CI run and a + # bench that seeds a duplicate baseline. git config user.name "github-actions[bot]" git config user.email "41898282+github-actions[bot]@users.noreply.github.com" @@ -334,10 +355,10 @@ jobs: # release-plan.json is a run artifact, not repo content. git add packages/*/package.json packages/*/CHANGELOG.md pnpm-lock.yaml - # The subject is load-bearing: every workflow's root job is gated on - # `startsWith(head_commit.message, 'chore(release): publish')` so this - # push does not retrigger CI. Change the wording here and you must - # change it in release.yml, pr-checks.yml and bench.yml together. + # The subject is load-bearing: every workflow's root job skips a push + # whose head commit starts with this subject AND carries the bot + # identity set above. Change the wording here and you must change it + # in release.yml, pr-checks.yml and bench.yml together. git commit -m "chore(release): publish" jq -r '.[].tag' release-plan.json | while read -r tag; do diff --git a/tools/release/README.md b/tools/release/README.md index 8af74b62..e7046a00 100644 --- a/tools/release/README.md +++ b/tools/release/README.md @@ -214,19 +214,31 @@ What stops that being a loop is the guard on each workflow's root job: ```yaml if: >- github.event_name != 'push' - || !startsWith(github.event.head_commit.message, 'chore(release): publish') + || !(startsWith(github.event.head_commit.message, 'chore(release): publish') + && github.event.head_commit.author.email == '41898282+github-actions[bot]@users.noreply.github.com') ``` -So the release commit's **subject line is load-bearing**. `release.yml` writes it, and `release.yml`, -`pr-checks.yml` and `bench.yml` all match on it — change the wording in one place and you must change -it in all four. - -This deliberately does not use `[skip ci]`. GitHub scans the entire head-commit message for that -keyword, body included, and a squash merge concatenates every commit message on the branch into the -body — so a PR that merely *mentions* `[skip ci]` in prose disables every workflow for its merge -commit, creating no run at all to notice. That is exactly what happened to -[#89](https://github.com/cornerstonejs/codecs/pull/89), whose commits explained why the release -commit carried the keyword. Anchoring on the subject cannot be tripped by prose. +(The continuation lines sit at the same indent on purpose. A `>-` folded scalar keeps a real newline +before any *more*-indented line, which would embed one in the expression.) + +Both halves are written by the `release` job, a few lines apart — the `git config user.email` and the +`git commit -m`. **Change either and you must change it in all four files**, or the version commit +stops being recognised and gets a full CI run plus a bench that seeds a duplicate CodSpeed baseline. + +Two things this deliberately is not: + +- **Not `[skip ci]`.** GitHub scans the entire head-commit message for that keyword, body included, + and a squash merge concatenates every commit message on the branch into the body — so a PR that + merely *mentions* `[skip ci]` in prose disables every workflow for its merge commit, creating no + run at all to notice. That is exactly what happened to + [#89](https://github.com/cornerstonejs/codecs/pull/89), whose commits explained why the release + commit carried the keyword. Anchoring on the subject cannot be tripped by prose. +- **Not `==` on the message.** GitHub strips the trailing newline, so equality would match today, but + it stops matching the moment the release commit grows a body — a commit template, a + `prepare-commit-msg` hook, a second `-m`. That failure is silent. Equality also would not buy the + precision it looks like it buys: a human can type the exact subject as easily as a prefix. The + author clause is what makes the match precise, because only the release job can produce that + identity. 3. **Verify**, then re-run the failed Release workflow: