From f71f5c60f38ff78ba36e4b355ce4e17f0bf88581 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 27 Jul 2026 12:59:54 -0500 Subject: [PATCH] fix(release): make the GitHub release step idempotent so a failed publish can actually be retried `gh release create` fails with "a release with the same tag name already exists" on any re-run. Because that step sits BEFORE the PyPI publish, one publish failure was terminal: the release now existed, so every subsequent re-run died at creation and SKIPPED the publish entirely. The retry could not reach the thing it was retrying. v0.3.1 walked the whole trap. Attempt 1 created the release, then failed at PyPI with a 403 (Trusted Publisher mismatch). Attempts 2 and 3 both died at "Create GitHub release" -- so attempt 3 looked like a test of the publisher fix and tested nothing, because the publish step never ran. Only deleting the public release by hand let attempt 4 through, and it then published cleanly. Now: probe with `gh release view`, then `edit` + `upload --clobber` if it exists, `create` if not. `--clobber` because a re-run regenerates every artifact with fresh signatures, so replacing assets is the correct semantic rather than an error. `edit` takes `--prerelease=true|false` explicitly -- a bare `--prerelease` only ever SETS the flag, so a re-run could never demote a mis-marked pre-release. THE STEP ORDER IS UNCHANGED, DELIBERATELY. Publishing to PyPI first would look like the tidier fix and is worse: a PyPI upload burns that version number forever, while a GitHub release is deletable. Doing the reversible half first is exactly what made the v0.3.1 failure recoverable -- we could delete a release and retry, instead of holding an un-retryable PyPI version with no release. The defect was never the ordering; it was that the reversible half could not be repeated. The existing ordering test now says so, so nobody "fixes" it later. The step was renamed to "Create or update the GitHub release", which broke that ordering test's marker -- caught before pushing. Same shape as the `version==tag comparison` canary in the previous release commit: a test pinned to the exact WORDING of the thing it guards fails the moment the thing improves. Also corrects a comment in dependabot-lock-resync.yml asserting the App credentials were "configured correctly ... nothing to add". True of the PRIVATE repo; it did not survive the cutover. MEFORORG carried only MEFOR_FORBIDDEN_TOKENS, so the job would have skipped on every Dependabot uv PR while the comment claimed otherwise -- and it cost me a wrong assumption today. The credentials were added to MEFORORG on 2026-07-27. A configuration fact stated without the repository it applies to becomes a confident lie at the next migration. Mutation-verified both ways: neutralise the `gh release view` probe and the test fails; replace `--prerelease=false` with a bare `--prerelease` and it fails. Shell validated with `bash -n` on the YAML-de-indented run block. --- .github/workflows/dependabot-lock-resync.yml | 10 ++++-- .github/workflows/release.yml | 38 +++++++++++++++----- tests/test_release_pipeline.py | 31 +++++++++++++++- 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/.github/workflows/dependabot-lock-resync.yml b/.github/workflows/dependabot-lock-resync.yml index d7bd7e9..eeb188c 100644 --- a/.github/workflows/dependabot-lock-resync.yml +++ b/.github/workflows/dependabot-lock-resync.yml @@ -58,8 +58,14 @@ jobs: # seconds earlier. Observed on #801: # 00:04:30 triggering_actor=dependabot[bot] -> success, pushed the resync commit # 00:04:48 triggering_actor=mefor-lock-resync[bot] -> failure, "client-id ... is required" - # Skipping the self-triggered re-run is correct: the work is already done. The secrets are - # configured correctly under Settings > Secrets and variables > Dependabot; nothing to add. + # Skipping the self-triggered re-run is correct: the work is already done. + # + # (This line used to end "the secrets are configured correctly ... nothing to add". True of the + # PRIVATE repo, and it did NOT survive the cutover: MEFORORG carried only MEFOR_FORBIDDEN_TOKENS, + # so the App credentials were absent and this job would have skipped on every Dependabot uv PR + # while the comment asserted it was configured. LOCK_RESYNC_APP_ID + LOCK_RESYNC_APP_PRIVATE_KEY + # were added to MEFORORG > Secrets > Dependabot on 2026-07-27. State a configuration fact WITH + # the repository it applies to, or a migration turns it into a confident lie.) if: >- ${{ github.event.pull_request.user.login == 'dependabot[bot]' && github.triggering_actor == 'dependabot[bot]' }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5c37d9b..d1938f3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -238,7 +238,18 @@ jobs: with: subject-path: "dist/*.tar.gz, dist/*.whl, messagefoundry-sbom.cdx.json, messagefoundry-vex.openvex.json" - - name: Create GitHub release + # CREATE-OR-UPDATE, not create. This was a bare `gh release create`, which fails with "a release + # with the same tag name already exists" on any re-run — and because it sits BEFORE the PyPI + # publish below, that turned one publish failure into a permanent retry deadlock: every re-run + # died here and SKIPPED the publish, so a re-run could not even test the fix it was meant to + # verify. Observed on v0.3.1 — attempt 1 created the release then failed at PyPI (403, publisher + # mismatch); attempts 2-3 died here; only deleting the public release by hand let attempt 4 through. + # + # The step ORDER is deliberate and stays: the reversible action (a GitHub release, deletable) + # precedes the irreversible one (a PyPI upload burns that version forever). Publishing first would + # leave a failure here with an un-retryable PyPI version and no release. The defect was never the + # ordering — it was that the reversible half could not be repeated. + - name: Create or update the GitHub release if: startsWith(github.ref, 'refs/tags/') env: GH_TOKEN: ${{ github.token }} @@ -248,13 +259,24 @@ jobs: # Release notes = this version's CHANGELOG section (fall back to a one-liner). awk -v v="$ver" 'BEGIN{f=0} /^## \[/{ if(f) exit; if(index($0,"["v"]")) f=1 } f{print}' CHANGELOG.md > notes.md [ -s notes.md ] || echo "MessageFoundry $tag" > notes.md - pre="" - case "$ver" in *-*) pre="--prerelease";; esac - gh release create "$tag" \ - dist/*.tar.gz dist/*.whl dist/*.sigstore* \ - messagefoundry-sbom.cdx.json messagefoundry-sbom.cdx.json.sigstore* \ - messagefoundry-vex.openvex.json messagefoundry-vex.openvex.json.sigstore* \ - --title "$tag" --notes-file notes.md $pre + # `edit` needs an explicit true/false: a bare `--prerelease` only ever SETS the flag, so a + # re-run could never demote a release that had been mis-marked as a pre-release. + case "$ver" in + *-*) pre_create="--prerelease"; pre_edit="--prerelease=true" ;; + *) pre_create=""; pre_edit="--prerelease=false" ;; + esac + assets=( dist/*.tar.gz dist/*.whl dist/*.sigstore* + messagefoundry-sbom.cdx.json messagefoundry-sbom.cdx.json.sigstore* + messagefoundry-vex.openvex.json messagefoundry-vex.openvex.json.sigstore* ) + if gh release view "$tag" >/dev/null 2>&1; then + echo "Release $tag already exists — updating in place (this is a re-run)." + gh release edit "$tag" --title "$tag" --notes-file notes.md "$pre_edit" + # --clobber: re-uploading an existing asset name errors otherwise, and a re-run regenerates + # every artifact (fresh signatures), so replacing is the correct semantic. + gh release upload "$tag" "${assets[@]}" --clobber + else + gh release create "$tag" "${assets[@]}" --title "$tag" --notes-file notes.md $pre_create + fi - name: Upload artifacts (dry-run / always) if: always() diff --git a/tests/test_release_pipeline.py b/tests/test_release_pipeline.py index 7a36684..ab890ff 100644 --- a/tests/test_release_pipeline.py +++ b/tests/test_release_pipeline.py @@ -199,6 +199,11 @@ def test_release_pypi_publish_is_last_step_and_tag_gated() -> None: ) # Publish (irreversible) must come AFTER build, leak-gate, sign and the GitHub release. + # + # This order is load-bearing, not cosmetic: the GitHub release is REVERSIBLE (deletable) and the + # PyPI upload is not (a version number is burned forever). Doing the reversible half first is what + # made the v0.3.1 publisher failure recoverable at all — a failed publish left a release we could + # delete and retry, rather than an un-retryable PyPI version with no release. def idx(tok: str) -> int: i = release_job.find(tok) assert i != -1, f"expected marker missing from release job: {tok!r}" @@ -208,7 +213,7 @@ def idx(tok: str) -> int: idx("Build sdist + wheel"), idx("Leak gate — sdist MUST be package-only"), idx("python -m sigstore sign"), - idx("Create GitHub release"), + idx("Create or update the GitHub release"), idx("Publish to PyPI"), ] assert order == sorted(order), ( @@ -270,6 +275,30 @@ def test_release_jobs_are_gated_ON_the_source_repo() -> None: assert "wshallwshall" not in rel, "release.yml must not reference the retired private vault" +def test_the_github_release_step_is_idempotent() -> None: + """A re-run must be able to repeat the release step, or a publish failure wedges the tag forever. + + The step sits BEFORE the irreversible PyPI upload (deliberately — see the ordering test above), so + when it was a bare `gh release create` the first publish failure was terminal: the release now + existed, so every re-run died on "a release with the same tag name already exists" and SKIPPED the + publish. The retry could not even reach the thing it was retrying. v0.3.1 needed a human to delete + a public release before attempt 4 could get through. + """ + rel = _release() + assert "gh release view" in rel, ( + "the release step no longer probes for an existing release — a re-run will fail on 'already " + "exists' and skip the PyPI publish below it" + ) + assert "gh release edit" in rel and "--clobber" in rel, ( + "create-or-update is incomplete: an existing release must be edited and its assets replaced " + "(--clobber), since a re-run regenerates every artifact with fresh signatures" + ) + # `gh release edit --prerelease` (bare) only ever SETS the flag; demoting needs an explicit value. + assert "--prerelease=true" in rel and "--prerelease=false" in rel, ( + "edit uses a bare --prerelease, so a re-run could never demote a mis-marked pre-release" + ) + + def test_no_self_referential_slug_rewrite_survives() -> None: """The README slug rewrite is GONE, and no `sed s#X#X#` may come back.