diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index e22971a4..da7d4e0a 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -42,6 +42,18 @@ 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. 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') + && 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 ae18d07f..f6cd4c3d 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -50,6 +50,21 @@ 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). 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') + && 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 53b67468..8cc29a21 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -59,6 +59,41 @@ 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. + # + # 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') + && 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. @@ -292,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" @@ -315,7 +355,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 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 git tag -a "$tag" -m "$tag" @@ -327,12 +371,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..e7046a00 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,39 @@ 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') + && github.event.head_commit.author.email == '41898282+github-actions[bot]@users.noreply.github.com') +``` + +(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: