diff --git a/.github/actionlint.yaml b/.github/actionlint.yaml new file mode 100644 index 0000000..6e45547 --- /dev/null +++ b/.github/actionlint.yaml @@ -0,0 +1,8 @@ +paths: + # actionlint (<= 1.7.12) does not yet know the job.workflow_sha / job.workflow_repository properties. + .github/workflows/release-tag.yml: + ignore: + - 'property "workflow_(?:sha|repository)" is not defined in object type' + .github/workflows/release-github.yml: + ignore: + - 'property "workflow_(?:sha|repository)" is not defined in object type' diff --git a/.github/workflows/release-github.yml b/.github/workflows/release-github.yml index c080ec4..d2699ba 100644 --- a/.github/workflows/release-github.yml +++ b/.github/workflows/release-github.yml @@ -10,6 +10,13 @@ jobs: publish: runs-on: ubuntu-latest steps: + - name: Require the caller-pinned workflow commit + env: + WORKFLOW_SHA: ${{ job.workflow_sha }} + WORKFLOW_REPOSITORY: ${{ job.workflow_repository }} + run: | + [[ "$WORKFLOW_SHA" =~ ^[0-9a-f]{40}$ ]] || { echo 'Workflow commit SHA unavailable' >&2; exit 1; } + test "$WORKFLOW_REPOSITORY" = BehindTheMusicTree/github-workflows || { echo 'Unexpected workflow repository' >&2; exit 1; } - name: Require a tag push env: EVENT: ${{ github.event_name }} @@ -28,7 +35,7 @@ jobs: - uses: actions/checkout@v5 with: repository: BehindTheMusicTree/github-workflows - ref: bbd93e5c3ba2335515b35e1a163f305db51478d7 + ref: ${{ job.workflow_sha }} persist-credentials: false path: foundation - name: Validate and publish the existing tag diff --git a/.github/workflows/release-tag.yml b/.github/workflows/release-tag.yml index 3e293f4..3b2d6e6 100644 --- a/.github/workflows/release-tag.yml +++ b/.github/workflows/release-tag.yml @@ -1,4 +1,4 @@ -# Shared implementation is pinned separately so no caller code runs with App secrets. +# Helper scripts load from job.workflow_sha (the caller's pin of this workflow), so no caller code runs with App secrets. name: Validate and tag release on: @@ -27,6 +27,13 @@ jobs: tag: ${{ steps.intent.outputs.tag }} commit: ${{ steps.intent.outputs.commit }} steps: + - name: Require the caller-pinned workflow commit + env: + WORKFLOW_SHA: ${{ job.workflow_sha }} + WORKFLOW_REPOSITORY: ${{ job.workflow_repository }} + run: | + [[ "$WORKFLOW_SHA" =~ ^[0-9a-f]{40}$ ]] || { echo 'Workflow commit SHA unavailable' >&2; exit 1; } + test "$WORKFLOW_REPOSITORY" = BehindTheMusicTree/github-workflows || { echo 'Unexpected workflow repository' >&2; exit 1; } - name: Require a main push env: EVENT: ${{ github.event_name }} @@ -45,7 +52,7 @@ jobs: - uses: actions/checkout@v5 with: repository: BehindTheMusicTree/github-workflows - ref: bbd93e5c3ba2335515b35e1a163f305db51478d7 + ref: ${{ job.workflow_sha }} persist-credentials: false path: foundation - name: Validate the exact push @@ -65,10 +72,17 @@ jobs: runs-on: ubuntu-latest # Each event is independent: a global concurrency group can drop pending bumps. steps: + - name: Require the caller-pinned workflow commit + env: + WORKFLOW_SHA: ${{ job.workflow_sha }} + WORKFLOW_REPOSITORY: ${{ job.workflow_repository }} + run: | + [[ "$WORKFLOW_SHA" =~ ^[0-9a-f]{40}$ ]] || { echo 'Workflow commit SHA unavailable' >&2; exit 1; } + test "$WORKFLOW_REPOSITORY" = BehindTheMusicTree/github-workflows || { echo 'Unexpected workflow repository' >&2; exit 1; } - uses: actions/checkout@v5 with: repository: BehindTheMusicTree/github-workflows - ref: bbd93e5c3ba2335515b35e1a163f305db51478d7 + ref: ${{ job.workflow_sha }} persist-credentials: false - name: Require App configuration env: diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb9514..f809027 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,9 @@ All contributors (including maintainers) should update `CHANGELOG.md` when creat ### Fixed +- Tag creation tolerates GitHub read-after-write lag: it validates the write responses and re-reads the tag with backoff instead of failing a correct tag; the tests simulate the lag. +- Release workflows load their helper scripts from the caller-pinned workflow commit (`job.workflow_sha`) instead of a hard-coded pre-squash SHA, removing the two-step self-pin. + - Preserve complete release notes when Markdown examples contain version headings. - Release validation and publication preserve complete changelog notes containing fenced Markdown headings and ignore fenced version examples. diff --git a/README.md b/README.md index dad711a..d4c5c9a 100644 --- a/README.md +++ b/README.md @@ -407,9 +407,9 @@ Activation and changes to the contract must land in a separate push from a versi Prepare version and changelog in a PR using the project's own tools. In this repository, `.github/scripts/release.sh major|minor|patch` prepares a commit on a feature branch. Its legacy `publish` command remains available to repositories without the CI policy; once `.github/release-policy.json` exists, manual publication is refused. This repository declares its own `VERSION` policy and uses pinned callers for tagging and GitHub publication. Initial adoption keeps version 5.0.1 and produces no tag; App access and protections are activated separately in `github-config`. -Validation runs without App secrets and checks ancestry against fetched `origin/main`. A separate runner checks out only this repository's shared implementation at an immutable SHA, checks configuration, then requests an App token scoped to the caller repository and Contents write. The App-token action revokes it in its post step. No caller script or build runs in that job. Maintainers changing the Python implementation must commit it first and update **both** helper checkout SHAs in the reusable workflow to that reviewed snapshot. The outer workflow is independently pinned by each caller. +Validation runs without App secrets and checks ancestry against fetched `origin/main`. A separate runner checks out only this repository's shared implementation at an immutable SHA, checks configuration, then requests an App token scoped to the caller repository and Contents write. The App-token action revokes it in its post step. No caller script or build runs in that job. The helper checkouts use `job.workflow_sha`, the commit of this reusable workflow that the caller pinned, after a guard step requires a 40-hex SHA and `job.workflow_repository` equal to `BehindTheMusicTree/github-workflows`. Scripts therefore always match the reviewed workflow and a Python change needs no second self-pin; each caller still pins the outer workflow independently. -Each run creates an annotated tag object, then atomically creates its ref. An existing lightweight or annotated tag that dereferences to the same commit is a successful retry; a tag on another commit fails. Concurrent creation is accepted only after resolving the winner to the same SHA. API failures propagate. There is no global concurrency group that can discard a pending bump. A failed API response after ref creation can be retried with the same push event; never move a tag or bump again to recover. +Each run creates an annotated tag object, then atomically creates its ref, checking both write responses. Verification then re-reads the tag with backoff (about 23 seconds) because GitHub reads can briefly return 404 after a write; a mismatch fails immediately, and a tag still unreadable afterwards fails with a hint to rerun the job, which then reports an existing tag. An existing lightweight or annotated tag that dereferences to the same commit is a successful retry; a tag on another commit fails. Concurrent creation is accepted only after resolving the winner to the same SHA. API failures propagate. There is no global concurrency group that can discard a pending bump. A failed API response after ref creation can be retried with the same push event; never move a tag or bump again to recover. `outputs` records the publisher contract; this tagger does not assert that those artifacts are published. For a partial publication, rerun the existing tag publisher after checking each required artifact. In a multi-package project the publisher must skip an already published identical version and distinguish registry errors from an absent package. GitHub Release creation must have one owner and wait for every required package. Do not replace a missing publisher run with an arbitrary workflow dispatch. diff --git a/release/create-tag.py b/release/create-tag.py index 291e457..6fe26a4 100644 --- a/release/create-tag.py +++ b/release/create-tag.py @@ -5,6 +5,7 @@ import os import re import sys +import time from urllib.error import HTTPError from urllib.parse import quote from urllib.request import Request, urlopen @@ -65,7 +66,33 @@ def existing_commit(api, tag): return obj["sha"] -def create_tag(api, tag, commit): +def points_to(obj, kind, sha): + # GitHub adds fields such as "url" to object; compare only the identity. + return isinstance(obj, dict) and obj.get("type") == kind and obj.get("sha") == sha + + +def wait_for_commit(api, tag, commit, sleep=time.sleep, attempts=6): + # GitHub reads can lag right after a successful write: tolerate 404, never a mismatch. + delay = 1 + for attempt in range(attempts): + try: + current = existing_commit(api, tag) + except APIError as exc: + # The tag object itself can lag behind the ref that points to it. + if exc.status != 404: + raise + current = None + if current is not None: + if current != commit: + raise ValueError("release tag collision: tag targets another commit") + return current + if attempt < attempts - 1: + sleep(delay) + delay = min(delay * 2, 8) + return None + + +def create_tag(api, tag, commit, sleep=time.sleep): if not re.fullmatch(r"[A-Za-z0-9_-]+(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)\.(?:0|[1-9][0-9]*)", tag): raise ValueError("invalid stable release tag") if not re.fullmatch(r"[0-9a-f]{40}", commit) or commit == "0" * 40: @@ -81,20 +108,21 @@ def create_tag(api, tag, commit): sha = annotated["sha"] if not re.fullmatch(r"[0-9a-f]{40}", sha): raise ValueError("invalid tag object SHA returned by GitHub") + if not points_to(annotated.get("object"), "commit", commit): + raise ValueError("annotated tag does not target the expected commit") try: - api.request("POST", "/git/refs", {"ref": "refs/tags/" + tag, "sha": sha}) + created = api.request("POST", "/git/refs", {"ref": "refs/tags/" + tag, "sha": sha}) except APIError as exc: # Another run may win the atomic ref creation. Only the same commit is a retry. if exc.status != 422: raise - current = existing_commit(api, tag) - if current is None: + if wait_for_commit(api, tag, commit, sleep) is None: raise exc - if current != commit: - raise ValueError("release tag collision: concurrent tag targets another commit") from None return "existing" - if existing_commit(api, tag) != commit: - raise ValueError("created tag did not resolve to the expected commit") + if created.get("ref") != "refs/tags/" + tag or not points_to(created.get("object"), "tag", sha): + raise ValueError("created ref does not match the annotated tag") + if wait_for_commit(api, tag, commit, sleep) is None: + raise ValueError("created tag not readable yet; rerun the job to verify") return "created" diff --git a/release/test_create_tag.py b/release/test_create_tag.py index 06d8f3a..a10e855 100644 --- a/release/test_create_tag.py +++ b/release/test_create_tag.py @@ -21,6 +21,10 @@ def __init__(self): self.calls = [] self.failure = None self.race = None + self.lag = 0 + self.tag_lag = 0 + self.bad_tag_object = None + self.bad_ref_response = None def request(self, method, path, data=None): self.calls.append((method, path, data)) @@ -30,20 +34,28 @@ def request(self, method, path, data=None): name = path.removeprefix("/git/ref/tags/") if name not in self.refs: raise tagger.APIError(404) + if self.lag: + self.lag -= 1 + raise tagger.APIError(404) return {"object": self.refs[name]} if path.startswith("/git/tags/"): + if self.tag_lag: + self.tag_lag -= 1 + raise tagger.APIError(404) return {"object": self.tags[path.removeprefix("/git/tags/")]} if path == "/git/tags": sha = f"{len(self.tags) + 1:040x}" self.tags[sha] = {"type": "commit", "sha": data["object"]} - return {"sha": sha} + return {"sha": sha, "object": self.bad_tag_object or {**self.tags[sha], "url": "https://api.github.com/x"}} if path == "/git/refs": name = data["ref"].removeprefix("refs/tags/") if self.race: self.refs[name] = {"type": "commit", "sha": self.race} raise tagger.APIError(422) self.refs[name] = {"type": "tag", "sha": data["sha"]} - return {} + if self.bad_ref_response is not None: + return self.bad_ref_response + return {"ref": data["ref"], "object": {**self.refs[name], "url": "https://api.github.com/x"}} raise AssertionError((method, path, data)) @@ -113,7 +125,67 @@ def request(method, path, data=None): api.request = request with self.assertRaises(tagger.APIError): - tagger.create_tag(api, "v1.1.0", COMMIT) + tagger.create_tag(api, "v1.1.0", COMMIT, sleep=lambda _: None) + + def test_read_lag_after_creation_is_tolerated(self): + for lag in (1, 3, 5): + with self.subTest(lag=lag): + api = FakeGitHub() + api.lag = lag + waits = [] + self.assertEqual(tagger.create_tag(api, "v1.1.0", COMMIT, sleep=waits.append), "created") + self.assertEqual(waits, [1, 2, 4, 8, 8][:lag]) + + def test_tag_object_read_lag_is_tolerated(self): + api = FakeGitHub() + original = api.request + + def request(method, path, data=None): + if path == "/git/refs": + result = original(method, path, data) + api.tag_lag = 2 + return result + return original(method, path, data) + + api.request = request + waits = [] + self.assertEqual(tagger.create_tag(api, "v1.1.0", COMMIT, sleep=waits.append), "created") + self.assertEqual(waits, [1, 2]) + + def test_read_lag_beyond_attempts_fails_with_rerun_hint(self): + api = FakeGitHub() + api.lag = 6 + waits = [] + with self.assertRaisesRegex(ValueError, "rerun the job"): + tagger.create_tag(api, "v1.1.0", COMMIT, sleep=waits.append) + self.assertEqual(waits, [1, 2, 4, 8, 8]) + + def test_read_lag_on_atomic_collision_is_existing(self): + api = FakeGitHub() + api.race = COMMIT + api.lag = 2 + self.assertEqual(tagger.create_tag(api, "v1.1.0", COMMIT, sleep=lambda _: None), "existing") + + def test_mismatch_after_lag_fails_without_waiting_further(self): + api = FakeGitHub() + api.race = OTHER + api.lag = 1 + waits = [] + with self.assertRaisesRegex(ValueError, "collision"): + tagger.create_tag(api, "v1.1.0", COMMIT, sleep=waits.append) + self.assertEqual(waits, [1]) + + def test_malformed_write_responses_fail(self): + api = FakeGitHub() + api.bad_tag_object = {"type": "commit", "sha": OTHER} + with self.assertRaisesRegex(ValueError, "annotated tag"): + tagger.create_tag(api, "v1.1.0", COMMIT, sleep=lambda _: None) + for response in ({}, {"ref": "refs/tags/other", "object": {"type": "tag", "sha": "1" * 40}}): + with self.subTest(response=response): + api = FakeGitHub() + api.bad_ref_response = response + with self.assertRaisesRegex(ValueError, "created ref"): + tagger.create_tag(api, "v1.1.0", COMMIT, sleep=lambda _: None) def test_missing_token_and_invalid_inputs(self): with self.assertRaisesRegex(ValueError, "token missing"):