From f53a2fd223fa3acc321843c3bf970b4bed99a436 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Fri, 11 Sep 2026 15:35:30 +0500 Subject: [PATCH] fix(post-release-bump): trigger on the tag, which is the only signal 5 of its 6 repos raise (backend#3681) post-release-bump.yml was driven by the `release` event, on the assumption that callers would trigger on `release: published`. Measured 2026-09-11 across the six repos that declare a version_file in release-train/repos.yml, that event reaches exactly ONE of them: client gh_release: true -- the train runs `gh release create` under a PAT, so the event fires. FIRES cli, data-ingestors their own publish workflows create the Release under GITHUB_TOKEN, and GitHub does not start workflow runs from events raised by GITHUB_TOKEN. NEVER FIRES tracebloc-py-package no GitHub Release is created at all -- promote-repo.sh design-system POSTs a bare tag ref for every repo that does not set design-system-v2 gh_release. NEVER FIRES So the caller rollout this was parked for would have installed five workflows that can never run. Worse, the three raising no release event at all are exactly the repos the reusable was written for: design-system after v1.3.0 and v1.4.0, and design-system-v2 at v0.1.0, all cited in its own header. Callers now trigger on `push: tags: ['v*']`. The tag is the one signal all six produce, because the train always creates it on the way to publishing. The release path still works, so `client` is unaffected. THE PRERELEASE TEST HAD TO MOVE, AND IT IS THE SHARP EDGE HERE. It was a job-level `if:` on `github.event.release.prerelease`; a tag push has no such field, and the caller's `v*` glob matches every rc the train cuts -- once per hop per repo. The shape test already in the script does NOT catch them: it is a glob, `[0-9]*.[0-9]*.[0-9]*`, and `1.2.3-rc.1` satisfies it, because `*` matches the suffix too. Dropping the field without adding an explicit rc test would not have failed loudly; it would have bumped the version on every rc tag. An rc is now a green no-op, checked before the shape test. That trap is why this lands with a selftest rather than on inspection. scripts/tests/post-release-bump-selftest.sh extracts BOTH `case` blocks out of the YAML by anchor and runs them, so the workflow cannot drift from what the suite checks -- a selftest carrying its own copy of the logic is the inert-verification shape backend#1729 catalogued. Mutation-proven: neutering the rc arm turns exactly the four rc assertions red and leaves accept/refuse green. The suite also found a false green in its own first draft -- the three "refuse" cases were passing on an `unbound variable` abort rather than on the shape test, and would have kept passing with that test deleted. Noted in the file where the fix is. repo-inventory.yml's `post_release_bump_parked` row is updated rather than deleted: the callers are still pending, so the exemption still holds, but it now records WHY the rollout did not simply happen. The row still goes when the callers land. Refs backend#3681. Co-Authored-By: Claude Opus 5 --- .github/workflows/post-release-bump.yml | 47 ++++++++- Makefile | 11 ++ repo-inventory.yml | 11 +- scripts/tests/post-release-bump-selftest.sh | 107 ++++++++++++++++++++ 4 files changed, 174 insertions(+), 2 deletions(-) create mode 100755 scripts/tests/post-release-bump-selftest.sh diff --git a/.github/workflows/post-release-bump.yml b/.github/workflows/post-release-bump.yml index ce4d160..9a352cd 100644 --- a/.github/workflows/post-release-bump.yml +++ b/.github/workflows/post-release-bump.yml @@ -20,6 +20,32 @@ name: Post-release version bump # This opens the bump PR at the one moment the version is known for certain -- # immediately after the release that consumed it. # +# THE CALLER TRIGGERS ON THE TAG, NOT ON `release` (backend#3681). The first +# version of this reusable read `github.event.release.*` and assumed callers would +# trigger on `release: published`. Measured 2026-09-11 across the six repos that +# declare a version_file in release-train/repos.yml, that event reaches exactly ONE +# of them: +# +# client gh_release: true -> the train runs `gh release create` +# under a PAT, so the event fires. FIRES +# cli, data-ingestors their own publish workflows create the Release under +# GITHUB_TOKEN, and GitHub does not start workflow runs +# from events raised by GITHUB_TOKEN. NEVER FIRES +# tracebloc-py-package no GitHub Release is created at all -- promote-repo.sh +# design-system POSTs a bare tag ref (`git/refs`) for every repo that +# design-system-v2 does not set gh_release. NEVER FIRES +# +# The three that raise no release event at all are precisely the repos this +# reusable was written for -- `design-system` after v1.3.0 and v1.4.0, and +# `design-system-v2` at v0.1.0, all cited above. So callers wire it to +# `on: push: tags: ['v*']`: the TAG is the one signal all six actually produce, +# because the train always creates it on the way to publishing. +# +# THE PRERELEASE TEST THEREFORE MOVED INTO THE SCRIPT. On the release event it was +# a field (`prerelease`); on a tag push there is no such field, and `v*` matches +# every rc tag the train cuts per hop. An rc is a GREEN NO-OP, not an error -- it +# consumes no version and there is nothing to bump. +# # WHY A PR AND NOT A PUSH. `develop` is protected and the bump is a judgement # call: patch is the safe default, but the person who just shipped may know the # next cycle is a minor. A PR is reviewable, and editing the number before merge @@ -105,7 +131,10 @@ jobs: GH_TOKEN: ${{ steps.app-token.outputs.token }} # PR CREATION ONLY, so the PR has an author Bugbot will review. PR_AUTHOR_TOKEN: ${{ secrets.SYNC_PR_AUTHOR_TOKEN }} - TAG: ${{ github.event.release.tag_name }} + # BOTH EVENTS, because the release path stays supported for `client`, + # which is the one repo whose release event does fire. On a tag push + # `github.event.release` is absent and `github.ref_name` is the tag. + TAG: ${{ github.event.release.tag_name || github.ref_name }} VERSION_FILE: ${{ inputs.version-file }} BUMP: ${{ inputs.bump }} BASE: ${{ inputs.base-branch }} @@ -148,6 +177,22 @@ jobs: exit 1 fi + # AN rc CONSUMES NOTHING (backend#3681). On the release event this was the + # job-level `if: github.event.release.prerelease != true`; a tag push has + # no such field and the caller's `v*` glob matches every rc the train cuts, + # which is once per hop per repo. Green no-op, never an error. + # + # CHECKED BEFORE THE SHAPE TEST BELOW, WHICH DOES NOT CATCH IT. That test + # is a GLOB -- `[0-9]*.[0-9]*.[0-9]*` -- and `1.2.3-rc.1` satisfies it, + # since `*` matches the suffix too. The prerelease field was doing this + # work, so removing the field without adding this check would have bumped + # the version on every rc tag. + case "$TAG" in + *-rc.*|*-rc[0-9]*) + echo "::notice::$TAG is a release candidate - it consumes no version, nothing to bump." + exit 0 ;; + esac + RELEASED="${TAG#v}" case "$RELEASED" in [0-9]*.[0-9]*.[0-9]*) ;; diff --git a/Makefile b/Makefile index a5aee77..f8e758e 100644 --- a/Makefile +++ b/Makefile @@ -385,6 +385,7 @@ SELFTEST_TARGETS := selftest-caller-drift selftest-blocked-marker selftest-stand selftest-lint-targets \ selftest-fr-gate-walk \ selftest-extract-advanced-prs \ + selftest-post-release-bump \ selftest-dead-weight selftests: selftests-cover $(SELFTEST_TARGETS) @@ -812,6 +813,16 @@ selftest-git-reap: selftest-extract-advanced-prs: bash scripts/tests/extract-advanced-prs-selftest.sh +# The TAG DECISION in post-release-bump.yml (backend#3681). Its callers trigger on +# `push: tags: ['v*']`, not on `release`, so "is this a prerelease?" is decided from +# the tag text rather than an event field -- and the shape glob the workflow already +# had accepts `1.2.3-rc.1`, so the rc test has to be explicit or every rc tag bumps +# the version. The suite extracts both `case` blocks out of the YAML rather than +# restating them, so the workflow cannot drift away from what this runs. +.PHONY: selftest-post-release-bump +selftest-post-release-bump: + bash scripts/tests/post-release-bump-selftest.sh + # Branch OWNERSHIP, which git-reap above deliberately does not need: it reaps the # caller's OWN local branches, so "whose is it" never arises. Anything that # reasons about ownership across people -- a per-person report, a remote sweep -- diff --git a/repo-inventory.yml b/repo-inventory.yml index b1b7aed..4c7d233 100644 --- a/repo-inventory.yml +++ b/repo-inventory.yml @@ -350,7 +350,16 @@ shared_reasons: reviewed on their own, which is the whole risk surface. The callers are added in the follow-up rollout PR, and THIS ROW MUST BE DELETED at that point -- an exemption that outlives the reason for it is how a parked artifact becomes a permanently unaudited - one. Deliberately cites NO ticket number: the ticket this reusable was filed under + one. UPDATED backend#3681: the rollout was attempted and found blocked, which is the + thing this row was quietly hiding. The reusable was driven by the `release` event, + and measured 2026-09-11 that event reaches ONE of the six repos declaring a + version_file -- client, whose release the train creates under a PAT. cli and + data-ingestors create theirs under GITHUB_TOKEN, which never starts a workflow run; + tracebloc-py-package, design-system and design-system-v2 get a bare tag ref and no + release at all. Five callers would have been dead workflows, and the three with no + release event are the repos the reusable was written for. The trigger now accepts a + tag push, which is the one signal all six produce; the callers are the next PR and + this row still goes when they land. Deliberately cites NO ticket number: the ticket this reusable was filed under closed while it was still unmerged, and a reason that defers work to a shut ticket is the exact shape reason-citations exists to catch (it caught an earlier draft of this very text on its first run). The condition is written out instead of delegated, which diff --git a/scripts/tests/post-release-bump-selftest.sh b/scripts/tests/post-release-bump-selftest.sh new file mode 100755 index 0000000..f6c9e52 --- /dev/null +++ b/scripts/tests/post-release-bump-selftest.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# +# post-release-bump selftest (backend#3681) -- the TAG DECISION, extracted from the +# workflow and exercised directly. +# +# WHAT THIS PINS. The reusable used to decide "is this a prerelease?" from an event +# FIELD (`github.event.release.prerelease`). backend#3681 moved callers onto +# `on: push: tags: ['v*']`, because the release event reaches exactly one of the six +# repos that declare a version_file -- so the field is gone and the decision is made +# from the TAG TEXT instead. +# +# That swap has one trap, and it is the reason this file exists. The shape test the +# workflow already had is a GLOB: +# +# case "${TAG#v}" in [0-9]*.[0-9]*.[0-9]*) ;; *) error ;; esac +# +# and `1.2.3-rc.1` SATISFIES it -- `*` matches the `-rc.1` suffix too. So dropping +# the prerelease field without adding an explicit rc test would not have failed +# loudly; it would have bumped the version on every rc tag the train cuts, which is +# once per hop per repo. A silently-too-permissive glob is exactly the kind of thing +# a reader confirms by eye and gets wrong. +# +# THE LOGIC IS READ OUT OF THE WORKFLOW, NOT RETYPED HERE. A selftest that carries +# its own copy of the `case` statements passes forever while the workflow drifts -- +# the inert-verification shape backend#1729 catalogued. The two `case` blocks are +# extracted from the YAML by anchor, so editing the workflow either changes what +# this runs or breaks the extraction; it cannot quietly diverge. +# +# Run: bash scripts/tests/post-release-bump-selftest.sh +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +WF="$ROOT/.github/workflows/post-release-bump.yml" +[ -f "$WF" ] || { echo "FATAL: $WF not found"; exit 1; } + +pass=0 +fail=0 +ok() { printf ' ok %s\n' "$1"; pass=$((pass + 1)); } +no() { printf ' FAIL %s\n %s\n' "$1" "$2"; fail=$((fail + 1)); } + +# --- extract the two decisions from the workflow ----------------------------- +# +# Anchored on the literal `case` subjects rather than on line numbers, which move. +# Both extractions are REQUIRED to find something: a silent empty extraction would +# make every assertion below pass against an empty function. +rc_case="$(awk '/^ *case "\$TAG" in/{f=1} f{print} f&&/^ *esac/{exit}' "$WF")" +[ -n "$rc_case" ] || { echo "FATAL: could not extract the rc \`case \"\$TAG\"\` block from $WF"; exit 1; } + +shape_case="$(awk '/^ *case "\$RELEASED" in/{f=1} f{print} f&&/^ *esac/{exit}' "$WF")" +[ -n "$shape_case" ] || { echo "FATAL: could not extract the \`case \"\$RELEASED\"\` shape block from $WF"; exit 1; } + +# The workflow's own bodies call `echo ::notice`/`::error` and `exit`. Run them in a +# subshell and report the exit code, which IS the decision: 0 = skip or accept, +# 1 = refuse. +verdict() { # tag -> "skip" | "accept" | "refuse" + local TAG="$1" out rc + # VERSION_FILE is set because the workflow's REFUSAL MESSAGE interpolates it. + # Without it `set -u` aborts the subshell with "unbound variable" -- which is a + # non-zero exit, so every `refuse` assertion below passed for the wrong reason and + # would have kept passing with the shape check deleted. Caught on this file's own + # first run; it is the same false-green shape the header warns about, one layer in. + out=$( + set -uo pipefail + TAG="$TAG" + VERSION_FILE="package.json" + BASE="develop" + eval "$rc_case" + RELEASED="${TAG#v}" + eval "$shape_case" + echo "__ACCEPTED__" + ) ; rc=$? + if [ $rc -ne 0 ]; then echo refuse + elif grep -q '__ACCEPTED__' <<<"$out"; then echo accept + else echo skip + fi +} + +is() { # desc, expected, tag + local got; got="$(verdict "$3")" + if [ "$got" = "$2" ]; then ok "$1"; else no "$1" "expected $2, got $got (tag: $3)"; fi +} + +# --- a plain release consumes its version and must bump ---------------------- +is "v1.2.3 is accepted" accept v1.2.3 +is "v0.1.0 is accepted (the design-system-v2 case that started this)" accept v0.1.0 +is "v10.20.30 is accepted" accept v10.20.30 + +# --- an rc consumes nothing: GREEN no-op, never an error --------------------- +# +# THE REGRESSION THIS FILE EXISTS FOR. Each of these satisfies the `[0-9]*.[0-9]*. +# [0-9]*` glob, so without the rc `case` every one of them would read as `accept`. +is "v1.2.3-rc.1 is skipped, not bumped" skip v1.2.3-rc.1 +is "v1.2.3-rc.12 is skipped" skip v1.2.3-rc.12 +is "v1.9.115-rc.1 is skipped (client's real rc shape)" skip v1.9.115-rc.1 +is "v2.0.0-rc1 is skipped (no dot after rc)" skip v2.0.0-rc1 + +# --- anything that is not a version at all is REFUSED, loudly ----------------- +# +# Unchanged behaviour, asserted so the rc addition above cannot quietly widen into +# "skip everything I don't recognise" -- which would turn a malformed tag into a +# green no-op and lose the bump with no signal. +is "a non-version tag is refused" refuse vlatest +is "a two-part tag is refused" refuse v1.2 +is "an empty tag is refused" refuse "" + +printf '\npost-release-bump: %d passed, %d failed\n' "$pass" "$fail" +[ "$fail" -eq 0 ]