From aa4e901bd2090acaf124b05ee17955a3709b1b33 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Mon, 21 Sep 2026 22:20:34 -0700 Subject: [PATCH] ci: a release preflight, because the tag is already public when CI refuses THE WORKFLOW WAS NEVER BROKEN. v0.1.0-rc3 was pushed to all three remotes while every crate still said 0.1.0-rc2; dist matches a tag against the root package version, refused with "This workspace doesn't have anything for dist to Release!", and the four jobs behind `plan` skipped. Proved in both directions locally: `dist plan --tag=v0.1.0-rc2` gets past the check and `--tag=v0.1.0-rc3` reproduces the CI error exactly. So this changes no trigger and no job graph. What it adds is a check that runs BEFORE the irreversible step. dist named the right answer in its own error text; the problem is when that answer arrives -- a tag is public and immutable by the time CI reads it, so the repair is a new version rather than an edit. scripts/release-preflight.sh refuses a dirty tree, compares the intended tag against the root package version, runs `dist plan` as a second opinion when dist is on PATH (and says so when it is not, since an absent dist must not read as a pass), and prints the tag-and-push commands only once it passes. THE TEST'S FIRST VERSION WAS VACUOUS AND THE INJECTION IS WHAT SHOWED IT. release-preflight-test.sh originally asserted only "did it refuse". Every negative case still passed with the tag comparison replaced by `if false;`, because the script refuses a DIRTY WORKING TREE first and this repo's tree is almost always dirty -- bd auto-exports .beads/issues.jsonl on nearly every command. Four of five checks were answering "is the tree dirty" while claiming to answer "does it catch a bad tag". It now runs the negative cases against a clean throwaway checkout AND asserts the refusal names the version mismatch, so a refusal for the wrong reason fails. Two rival faults were injected against the repaired version -- the comparison disabled, and `expected` derived from the tag itself -- and each produces four failures and exit 1. That is design-principles section 7's "a check that looks like the answer, answering a neighbouring question", found inside a checker written to prevent that class, within an hour of writing it. Recorded in context/code/tooling.md. No version is bumped and no tag is cut here, per the user's instruction to make the workflow work without releasing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH --- .github/workflows/ci.yml | 4 + context/code/tooling.md | 54 ++++++++++++ scripts/release-preflight-test.sh | 76 ++++++++++++++++ scripts/release-preflight.sh | 140 ++++++++++++++++++++++++++++++ 4 files changed, 274 insertions(+) create mode 100755 scripts/release-preflight-test.sh create mode 100755 scripts/release-preflight.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d93793e9..c9c32466 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -264,6 +264,10 @@ jobs: # No trealla needed: pr-watch is driven entirely from fixtures. - name: The PR watcher refuses what it claims to run: ./scripts/pr-watch-test.sh + # The release preflight is the one guard whose absence is invisible until + # a tag is already public and immutable, so its refusals are checked here. + - name: The release preflight refuses what it claims to + run: ./scripts/release-preflight-test.sh # --------------------------------------------------------------------------- # Lints the one crate the `test` job cannot reach. diff --git a/context/code/tooling.md b/context/code/tooling.md index 02593116..7e8672ec 100644 --- a/context/code/tooling.md +++ b/context/code/tooling.md @@ -325,3 +325,57 @@ build already did some of that. Closing it properly means no two runs holding a `github-pages` artifact across a deployment, which is a real change to the job graph with real costs and is not attempted here. A retry is **not** a fix: the artifact is gone rather than transiently unlistable, so a retry cannot find it. + +### 4. The release preflight, and why a loud failure was not enough + +`scripts/release-preflight.sh` answers, before a tag exists, the question the +Release workflow answers after. + +**What happened.** `v0.1.0-rc3` was pushed to all three remotes while every +crate still said `0.1.0-rc2`. dist matches a tag against the root package's +version, refused with *"This workspace doesn't have anything for dist to +Release!"*, and the four jobs behind `plan` skipped. **Nothing was wrong with +the workflow**, which is worth stating plainly because the instinct on a failed +release run is to edit the YAML. The trigger is correct, the pattern matches, +the workflow is enabled; `dist plan --tag=v0.1.0-rc2` succeeds today and +`--tag=v0.1.0-rc3` reproduces the CI error exactly, which is the whole +diagnosis and both halves of it. + +**Why a loud, precise failure still cost something.** dist named the right +answer in its own error text — *"`--tag=v0.1.0-rc2` will Announce: improvise"*. +The problem is not the message, it is **when** it arrives: a tag is public and +immutable by the time CI reads it, so the repair is a new version rather than an +edit, and the tag that failed stays on three remotes forever. A check that runs +after the irreversible step is a check in the wrong place, however good its +message. So the preflight runs *before* tagging and prints the tag command only +once it has passed — the honest path made the easy one, the same shape as +`Basis` having no public constructor (gotcha 43). + +**The primary check is textual and the `dist plan` run is a second opinion.** +An absent `dist` must not read as a pass, so the script says which of the two +ran. It also tolerates a locally-installed dist whose version differs from the +one pinned in `dist-workspace.toml`: that stops dist before it plans, and +reporting it as a refusal would train the reader to ignore refusals. + +**Only the root package's version can disagree with a tag.** +`dist-workspace.toml` says `members = ["cargo:."]`, so `gp-tree` sitting at +`0.1.0` while everything else is on `0.1.0-rc2` is not a finding. The script +lists such crates and says so rather than failing, because a deliberate split +should stay deliberate and a silent one should be visible. + +**The test's first version was vacuous, and the fault injection is what showed +it.** `scripts/release-preflight-test.sh` originally asserted only *did it +refuse*. Every negative case passed with the tag comparison replaced by +`if false;` — because the script refuses a **dirty working tree** first, and +this repo's tree is almost always dirty, bd auto-exporting `.beads/issues.jsonl` +on nearly every command. Four of five checks were answering *is the tree dirty* +while claiming to answer *does it catch a bad tag*. The test now runs the +negative cases against a clean throwaway checkout **and asserts the refusal +names the version mismatch**, so a refusal for the wrong reason is a failure +rather than a pass. Two rival faults were injected against the repaired +version — the comparison disabled, and `expected` derived from the tag itself — +and each produces four failures and exit 1. + +That is `context/design-principles.md` §7's "a check that looks like the answer, +answering a neighbouring question", found in a checker written to prevent +exactly that class, within an hour of writing it. diff --git a/scripts/release-preflight-test.sh b/scripts/release-preflight-test.sh new file mode 100755 index 00000000..7664232c --- /dev/null +++ b/scripts/release-preflight-test.sh @@ -0,0 +1,76 @@ +#!/bin/sh +# check_the_checker for scripts/release-preflight.sh. +# +# It exists for the reason scripts/assemble-stamp-test.sh does: the guard is the +# whole value, and a guard that silently stopped guarding is invisible from the +# outside -- a preflight that accepted everything would print the same reassuring +# lines it prints today. +# +# IT ASSERTS THE REASON, NOT THE EXIT CODE, and that is the point. A first version +# checked only "did it refuse", and every negative case passed while the tag +# comparison was disabled -- because the script refuses a DIRTY WORKING TREE first, +# and this repo's tree is usually dirty (bd auto-exports .beads/issues.jsonl). Four +# of five checks were answering "is the tree dirty" while claiming to answer "does +# it catch a bad tag". Injecting `if false;` into the comparison is what exposed it. +# See context/design-principles.md section 7 -- a check adjacent to the thing it names. +set -eu + +root=$(cd "$(dirname "$0")/.." && pwd) +script="$root/scripts/release-preflight.sh" +fails=0 + +# Refuses, AND the refusal names the tag/version mismatch rather than the tree. +refuses_for_the_tag() { + what=$1; tag=$2 + out=$(sh "$script" "$tag" 2>&1) && { echo "FAIL: $what -- accepted" >&2; fails=$((fails+1)); return; } + case "$out" in + *"Cargo.toml has"*) echo " ok: $what (refused, naming the version)" ;; + *"uncommitted changes"*) echo "FAIL: $what -- refused for the DIRTY TREE, so this check proved nothing" >&2; fails=$((fails+1)) ;; + *) echo "FAIL: $what -- refused for an unrecognised reason:" >&2; echo "$out" | head -3 >&2; fails=$((fails+1)) ;; + esac +} + +version=$(awk '/^\[package\]/{p=1;next} /^\[/{p=0} p&&/^version *=/{gsub(/[" ]/,"");sub(/^version=/,"");print;exit}' \ + "$root/Cargo.toml") +test -n "$version" || { echo "FAIL: could not read the root version" >&2; exit 1; } +echo "release-preflight-test: root version is $version" + +# The dirty-tree guard runs first, so the negative cases are exercised against a +# CLEAN checkout of this tree rather than the working copy. Without this the test +# cannot reach the comparison it exists to check. +tmp=$(mktemp -d) +trap 'rm -rf "$tmp"' EXIT +git -C "$root" archive HEAD | (cd "$tmp" && tar x) +cp -f "$script" "$tmp/scripts/release-preflight.sh" +cp -f "$root/Cargo.toml" "$tmp/Cargo.toml" +(cd "$tmp" && git init -q . && git add -A && git -c user.email=t@t -c user.name=t commit -qm t) +script="$tmp/scripts/release-preflight.sh" + +if sh "$script" --self-test >/dev/null 2>&1; then + echo " ok: --self-test (accepted)" +else + echo "FAIL: --self-test refused" >&2; fails=$((fails+1)) +fi + +if sh "$script" >/dev/null 2>&1; then + echo "FAIL: no argument should be a usage error" >&2; fails=$((fails+1)) +else + echo " ok: no argument is a usage error (refused)" +fi + +refuses_for_the_tag "a tag one character off" "v${version}x" +refuses_for_the_tag "the shape that actually broke CI" "v0.1.0-rc99" +refuses_for_the_tag "a bare version with no v" "$version" + +# The positive direction, which only a clean tree can exercise. +if sh "$script" "v$version" >/dev/null 2>&1; then + echo " ok: the matching tag v$version (accepted)" +else + echo "FAIL: the matching tag v$version was refused" >&2; fails=$((fails+1)) +fi + +if [ "$fails" -ne 0 ]; then + echo "release-preflight-test: $fails failure(s)" >&2 + exit 1 +fi +echo "release-preflight-test: all checks passed" diff --git a/scripts/release-preflight.sh b/scripts/release-preflight.sh new file mode 100755 index 00000000..b8fe0248 --- /dev/null +++ b/scripts/release-preflight.sh @@ -0,0 +1,140 @@ +#!/bin/sh +# Answer, before a tag exists, the question the Release workflow answers after. +# +# Usage: scripts/release-preflight.sh vX.Y.Z[-suffix] +# scripts/release-preflight.sh --self-test +# +# WHY THIS EXISTS. `v0.1.0-rc3` was pushed to three remotes while every crate +# still said `0.1.0-rc2`. dist matches the tag against the root package version, +# refused with "This workspace doesn't have anything for dist to Release!", and +# the four jobs behind `plan` skipped. Nothing was wrong with the workflow. +# +# The cost is not the failure, it is WHEN the failure arrives: a tag is already +# public and immutable by the time CI reads it, so the repair is a new version +# rather than an edit. That is why this runs BEFORE tagging and prints the tag +# command only once it has passed -- the honest path made the easy one, the same +# shape as `Basis` having no public constructor (gotcha 43). +# +# The primary check is textual and always runs. `dist plan` is a SECOND opinion +# and is allowed to be unavailable; what it could not confirm is stated rather +# than assumed. Reasoning: context/code/tooling.md. scripts/release-preflight-test.sh +# is the check_the_checker. +set -eu + +root=$(cd "$(dirname "$0")/.." && pwd) + +# The root package's version, read from the FIRST [package] block of the root +# Cargo.toml. dist-workspace.toml says `members = ["cargo:."]`, so the root +# package is the only one dist announces and the only one whose version can +# disagree with a tag. gp-tree sitting at 0.1.0 is therefore not a finding. +root_version() { + awk '/^\[package\]/{p=1;next} /^\[/{p=0} p&&/^version *=/{gsub(/[" ]/,"");sub(/^version=/,"");print;exit}' \ + "$root/Cargo.toml" +} + +fail() { echo "release-preflight: $*" >&2; exit 1; } + +preflight() { + tag=$1 + version=$(root_version) + test -n "$version" || fail "could not read a version from Cargo.toml's [package] block" + + # A tag names a version; anything else is a spelling this repo does not use. + expected="v$version" + if [ "$tag" != "$expected" ]; then + echo "release-preflight: REFUSED" >&2 + echo " tag asked for : $tag" >&2 + echo " Cargo.toml has: $version (so the tag must be $expected)" >&2 + echo "" >&2 + echo " Bump every crate to the version you mean to release and commit that" >&2 + echo " FIRST. A tag pushed ahead of its version fails in CI after the tag is" >&2 + echo " already public, and a published tag should not be moved." >&2 + return 1 + fi + echo "release-preflight: tag $tag matches Cargo.toml version $version" + + # Every crate, because a workspace that disagrees with itself is the other + # half of the same error and dist says so in the same breath. + mismatched=$(grep -l '^version = ' "$root"/crates/*/Cargo.toml 2>/dev/null | while read -r f; do + v=$(awk '/^\[package\]/{p=1;next} /^\[/{p=0} p&&/^version *=/{gsub(/[" ]/,"");sub(/^version=/,"");print;exit}' "$f") + case "$v" in "$version"|"") ;; *) echo " $f: $v" ;; esac + done) + if [ -n "$mismatched" ]; then + echo "release-preflight: note -- these crates are on a different version:" + echo "$mismatched" + echo " Not fatal: dist announces only the root package (dist-workspace.toml" + echo " says members = [\"cargo:.\"]). Listed so a deliberate split stays deliberate." + fi + return 0 +} + +self_test() { + # check_the_checker. Both directions, because a checker that only ever passes + # and a checker that only ever fails are indistinguishable from one that works. + version=$(root_version) + test -n "$version" || { echo "self-test: could not read the root version" >&2; exit 1; } + + if ! preflight "v$version" >/dev/null 2>&1; then + echo "self-test FAILED: the matching tag v$version was refused" >&2 + exit 1 + fi + if preflight "v$version-nope" >/dev/null 2>&1; then + echo "self-test FAILED: a tag that does not match the version was accepted" >&2 + exit 1 + fi + # The exact shape that was actually pushed, spelled out, because this is the + # case the script was written for and a regression here is the whole failure. + if preflight "v0.1.0-rc3" >/dev/null 2>&1 && [ "$version" != "0.1.0-rc3" ]; then + echo "self-test FAILED: v0.1.0-rc3 accepted against version $version" >&2 + exit 1 + fi + echo "release-preflight: self-test passed (accepts v$version, refuses a mismatch)" +} + +case "${1:-}" in + --self-test) self_test; exit 0 ;; + "" | -h | --help) + echo "usage: release-preflight.sh vX.Y.Z[-suffix] | --self-test" >&2 + exit 2 ;; +esac + +tag=$1 + +# A dirty tree means the tag would not name what was tested. +if [ -n "$(git -C "$root" status --porcelain --untracked-files=no)" ]; then + fail "the working tree has uncommitted changes; tag a commit, not a guess" +fi + +preflight "$tag" || exit 1 + +# The second opinion. dist is the thing that actually refused in CI, so running +# it is worth more than any restatement of its rules -- but it is optional, and +# an absent dist must not read as a pass. +if command -v dist >/dev/null 2>&1; then + out=$(cd "$root" && dist plan "--tag=$tag" 2>&1) || true + case "$out" in + *"doesn't have anything for dist to Release"*) + echo "release-preflight: REFUSED -- dist rejected the tag:" >&2 + echo "$out" | sed -n '/×/,$p' | head -8 >&2 + exit 1 ;; + *"but 'cargo-dist-version"*) + echo "release-preflight: dist ran but is a different version from the one" + echo " pinned in dist-workspace.toml, so it stopped before planning. The" + echo " tag/version check above still passed; CI installs the pinned version." + ;; + *) echo "release-preflight: dist plan accepted $tag" ;; + esac +else + echo "release-preflight: dist not on PATH, so only the textual check ran." + echo " That is the check that catches the failure this script exists for." +fi + +cat <