-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Verify GitHub release before handing to post-release workflow #6927
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d4a234b
f5a4b0b
fbfb594
a11089e
9e3828a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| `create-release` now reads the GitHub release back from GitHub instead of trusting `gh`'s exit status, so the tag handed to a `post-release-workflow` is one that was verified: on the expected tag, published rather than drafted, flagged and titled as this run asked, and carrying the `SHA256SUMS` manifest at the size that was built. A re-run over a tag that already has a release verifies it the same way rather than treating any existing release as success — a missing or stale manifest is attached again (the one partial state a re-run can finish by itself), and a release that is drafted, flagged or titled differently stops the job for a human instead of being passed off as this version's release. A failed read is kept distinct from an absent release — a not-found is believed only once the repository itself reads back, so a rate-limited or unauthenticated read can no longer pass for a tag that has no release yet. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| import os | ||
| import re | ||
| from pathlib import Path | ||
| from typing import Any | ||
| from urllib.parse import quote | ||
|
|
||
| from packaging.version import InvalidVersion, Version | ||
|
|
@@ -50,6 +51,7 @@ | |
| changed_files, | ||
| commit_exists, | ||
| configure_bot_identity, | ||
| gh_capture, | ||
| gh_output, | ||
| gh_run, | ||
| git, | ||
|
|
@@ -71,6 +73,19 @@ | |
|
|
||
| _PACKAGE_SELECTION_SPLIT = re.compile(r"[\s,]+") | ||
|
|
||
| #: The release fields read back after a release is created (or found already | ||
| #: created), to prove the thing ``post-release`` is about to be pointed at is | ||
| #: the release this run published. Asked for and read from one place, so a | ||
| #: field can never be verified without having been requested. | ||
| _RELEASE_FIELDS = ("tagName", "name", "isDraft", "isPrerelease", "assets") | ||
|
|
||
| #: Markers in gh's stderr for a release that is not there. Matched widely — a | ||
| #: bare 404 counts — because a phrasing this list misses would stop every | ||
| #: ordinary release; what makes the wide match safe is that a not-found is | ||
| #: only believed once the repository itself has been read (see | ||
| #: :func:`_reads_as_absent`). | ||
| _NO_RELEASE_STDERR = ("release not found", "404") | ||
|
|
||
|
|
||
| def _is_null_sha(sha: str) -> bool: | ||
| """Return whether a git sha is the all-zero "no such commit" placeholder. | ||
|
|
@@ -933,6 +948,204 @@ def cmd_push_tag(config: Config, tag: str) -> None: | |
| git_push(f"refs/tags/{tag}", config.root) | ||
|
|
||
|
|
||
| def _reads_as_absent(config: Config, stderr: str) -> bool: | ||
| """Decide whether a failed release read means the tag has no release. | ||
|
|
||
| gh answers "there is no such release" and "GitHub could not be asked" the | ||
| same way: a non-zero exit and a message. A not-found is only evidence that | ||
| the release is absent if GitHub could be reached and this repository read | ||
| at all, so that is established rather than inferred from the wording — a | ||
| 404 for a repository that is missing, renamed or beyond the token's reach | ||
| says nothing about the release. | ||
|
|
||
| Args: | ||
| config: The repository configuration. | ||
| stderr: What gh wrote to stderr. | ||
|
|
||
| Returns: | ||
| Whether the tag can be treated as having no release. | ||
| """ | ||
| lowered = stderr.lower() | ||
| if not any(marker in lowered for marker in _NO_RELEASE_STDERR): | ||
| return False | ||
| # One extra call, and only on the path where a release is about to be | ||
| # created anyway: if the repository reads back, the not-found was about the | ||
| # release rather than about reaching GitHub. | ||
| returncode, _, _ = gh_capture(["repo", "view", "--json", "name"], config.root) | ||
| return returncode == 0 | ||
|
greptile-apps[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When Prompt for AI agents
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not doing this one; the other two from this review are fixed in a11089e. You're right that a listing sees drafts where the tag endpoint cannot — A Generated by Claude Code |
||
|
|
||
|
|
||
| def _release_view(config: Config, tag: str) -> dict[str, Any] | None: | ||
| """Read a GitHub release's metadata and asset list. | ||
|
|
||
| Args: | ||
| config: The repository configuration. | ||
| tag: The tag the release points at. | ||
|
|
||
| Returns: | ||
| The parsed ``gh release view`` payload, or None when the tag has no | ||
| release. | ||
| """ | ||
| # A probe, not an action: keep its output (and its "not found" stderr on the | ||
| # normal path) out of the job log. | ||
| returncode, payload, stderr = gh_capture( | ||
| ["release", "view", tag, "--json", ",".join(_RELEASE_FIELDS)], config.root | ||
| ) | ||
| if returncode != 0: | ||
| # A tag with no release and a GitHub that could not be reached both | ||
| # exit non-zero. Only the first means there is nothing there; reading | ||
| # the second as "no release" would create a release over one that | ||
| # exists, or report a release that was just made as missing. | ||
| if _reads_as_absent(config, stderr): | ||
| return None | ||
| fail( | ||
| f"could not read the GitHub release for {tag}: gh exited " | ||
| f"{returncode} saying {stderr or '(nothing)'}. Nothing is known " | ||
| "about the release either way, so re-run this job once gh can " | ||
| "reach GitHub." | ||
| ) | ||
| try: | ||
| release = json.loads(payload) | ||
| except json.JSONDecodeError: | ||
| fail(f"gh reported release metadata for {tag} that is not JSON: {payload!r}") | ||
| # The shape is established once, here, so everything downstream can read the | ||
| # payload as the release it claims to be rather than re-checking it. Each | ||
| # miss is a diagnostic failure: a TypeError traceback out of a job that has | ||
| # already published a version says nothing about what went wrong. | ||
| if not isinstance(release, dict): | ||
| fail( | ||
| f"gh reported release metadata for {tag} that is not an object: {payload!r}" | ||
| ) | ||
| if missing := [field for field in _RELEASE_FIELDS if field not in release]: | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| fail( | ||
| f"the release metadata gh reported for {tag} carries no " | ||
| f"{', '.join(missing)}, so the release cannot be verified" | ||
| ) | ||
| assets = release["assets"] | ||
| if not isinstance(assets, list) or not all( | ||
| isinstance(asset, dict) for asset in assets | ||
| ): | ||
| fail( | ||
| f"the release metadata gh reported for {tag} lists its assets as " | ||
| f"{assets!r} rather than as a list of assets, so what the release " | ||
| "carries cannot be verified" | ||
| ) | ||
| return release | ||
|
|
||
|
|
||
| def _metadata_problems( | ||
| release: dict[str, Any], tag: str, title: str, prerelease: bool | ||
| ) -> list[str]: | ||
| """List the ways a GitHub release disagrees with what was published. | ||
|
|
||
| Args: | ||
| release: A ``gh release view`` payload. | ||
| tag: The tag the release has to point at. | ||
| title: The title the release has to carry. | ||
| prerelease: Whether the release has to be flagged as a prerelease. | ||
|
|
||
| Returns: | ||
| Human-readable problems, empty when the release matches. | ||
| """ | ||
| # The "Latest" marker is deliberately not checked: it belongs to the | ||
| # repository rather than to one release, so a release published since this | ||
| # one legitimately holds it. | ||
| problems = [] | ||
| if (tag_name := release["tagName"]) != tag: | ||
| problems.append(f"it points at tag {tag_name!r} rather than {tag!r}") | ||
| if release["isDraft"]: | ||
| problems.append("it is still a draft, so the version is not released") | ||
| if (is_prerelease := bool(release["isPrerelease"])) != prerelease: | ||
| problems.append( | ||
| f"it is marked prerelease={is_prerelease} rather than " | ||
| f"prerelease={prerelease}" | ||
| ) | ||
| if (name := release["name"]) != title: | ||
| problems.append(f"it is titled {name!r} rather than {title!r}") | ||
| return problems | ||
|
|
||
|
|
||
| def _checksum_asset_problem( | ||
| release: dict[str, Any], checksums_path: Path | ||
| ) -> str | None: | ||
| """Return why a release does not carry this run's checksum manifest, if so. | ||
|
|
||
| Args: | ||
| release: A ``gh release view`` payload. | ||
| checksums_path: The local manifest of everything that was uploaded. | ||
|
|
||
| Returns: | ||
| A human-readable problem, or None when the release carries exactly this | ||
| manifest. | ||
| """ | ||
| name = checksums_path.name | ||
| asset = next( | ||
| (asset for asset in release["assets"] if asset.get("name") == name), None | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| ) | ||
| if asset is None: | ||
| return f"the {name} manifest is not attached to it" | ||
| if (state := asset.get("state")) != "uploaded": | ||
| return f"its {name} asset is in state {state!r} rather than 'uploaded'" | ||
| if (size := asset.get("size")) != (expected := checksums_path.stat().st_size): | ||
| return f"its {name} asset is {size} bytes rather than {expected}" | ||
| return None | ||
|
|
||
|
|
||
| def _accept_existing_release( | ||
| config: Config, | ||
| tag: str, | ||
| title: str, | ||
| prerelease: bool, | ||
| checksums_path: Path, | ||
| release: dict[str, Any], | ||
| ) -> None: | ||
| """Accept a release an earlier attempt left behind, or fail loudly. | ||
|
|
||
| A re-run stands down for an existing release only once that release is the | ||
| one this run would have created: on this tag, published rather than drafted, | ||
| flagged the same way and carrying the manifest of exactly what was uploaded. | ||
| The one partial state a re-run can finish by itself is a missing or stale | ||
| manifest — an attempt that died during the asset upload — so that is | ||
| uploaded again; anything else is a release nobody here made, and the tag is | ||
| not handed on until a human has looked at it. | ||
|
|
||
| Args: | ||
| config: The repository configuration. | ||
| tag: The tag the release points at. | ||
| title: The title this run would have given the release. | ||
| prerelease: Whether this run would have flagged it as a prerelease. | ||
| checksums_path: The ``sha256sum`` manifest of everything that was | ||
| uploaded. | ||
| release: The existing release's ``gh release view`` payload. | ||
| """ | ||
| if problems := _metadata_problems(release, tag, title, prerelease): | ||
| fail( | ||
| f"a GitHub release already exists for {tag}, but {'; '.join(problems)}. " | ||
| "It is not the release this publish would have created, so it is not " | ||
| "handed to the post-release workflow: inspect it, then either fix it " | ||
| "or delete it and re-run this job." | ||
| ) | ||
| if checksums_path.is_file() and ( | ||
| problem := _checksum_asset_problem(release, checksums_path) | ||
| ): | ||
| notice(f"release {tag} exists but {problem}; attaching it now") | ||
| gh_run( | ||
| ["release", "upload", tag, str(checksums_path), "--clobber"], config.root | ||
| ) | ||
| reread = _release_view(config, tag) | ||
| problem = ( | ||
| "there is no release on the tag" | ||
| if reread is None | ||
| else _checksum_asset_problem(reread, checksums_path) | ||
| ) | ||
| if problem: | ||
| fail( | ||
| f"attached the checksum manifest to the release for {tag}, but " | ||
| f"reading it back found that {problem}" | ||
| ) | ||
| echo(f"Release {tag} already matches this publish; skipping (safe re-run).") | ||
|
|
||
|
|
||
| def cmd_create_release( | ||
| config: Config, | ||
| tag: str, | ||
|
|
@@ -945,6 +1158,11 @@ def cmd_create_release( | |
| ) -> None: | ||
| """Create the GitHub release for a published version. | ||
|
|
||
| The release is read back once it exists: the next step hands the tag to the | ||
| post-release workflow, which publishes docs and images against a release it | ||
| trusts to be complete, so a release that is a draft, flagged the wrong way | ||
| or missing the checksum manifest stops the job here instead. | ||
|
|
||
| Args: | ||
| config: The repository configuration. | ||
| tag: The tag the release points at. | ||
|
|
@@ -957,14 +1175,15 @@ def cmd_create_release( | |
| uploaded, attached to the release so the record of what a version | ||
| contains outlives the workflow artifact it was built as. | ||
| """ | ||
| # A probe, not an action: keep its output (and its "not found" stderr on the | ||
| # normal path) out of the job log. | ||
| if gh_output(["release", "view", tag, "--json", "name"], config.root, check=False): | ||
| echo(f"Release {tag} already exists; skipping (safe re-run).") | ||
| return | ||
| # The root package is the repository, so its tag already names the release | ||
| # unambiguously; only a sub-package needs to say which package it is. | ||
| title = tag if package == config.root_package else f"{package}@{version}" | ||
| existing = _release_view(config, tag) | ||
| if existing is not None: | ||
| _accept_existing_release( | ||
| config, tag, title, prerelease, checksums_path, existing | ||
| ) | ||
| return | ||
| args = [ | ||
| "release", | ||
| "create", | ||
|
|
@@ -986,6 +1205,27 @@ def cmd_create_release( | |
| notice(f"no checksum manifest at {checksums_path}; releasing without one") | ||
| gh_run(args, config.root) | ||
|
|
||
| # gh's exit status covers the request it made, not the state GitHub was | ||
| # left in — a release whose asset upload failed is still a release — so what | ||
| # the next step hands on is proven from GitHub's own copy of it. | ||
| release = _release_view(config, tag) | ||
| if release is None: | ||
| fail( | ||
| f"gh created the release for {tag}, but reading it back found no " | ||
| f"release on the tag; check {tag} on the releases page before the " | ||
| "post-release workflow is pointed at it" | ||
| ) | ||
| if problems := _metadata_problems(release, tag, title, prerelease): | ||
| fail( | ||
| f"the GitHub release created for {tag} is not the one that was " | ||
| f"asked for: {'; '.join(problems)}" | ||
| ) | ||
| if checksums_path.is_file() and ( | ||
| problem := _checksum_asset_problem(release, checksums_path) | ||
| ): | ||
| fail(f"the GitHub release created for {tag} is incomplete: {problem}") | ||
| echo(f"Release {tag} created and verified as {title!r}.") | ||
|
|
||
|
|
||
| def cmd_post_release(config: Config, tag: str, package: str, version: str) -> None: | ||
| """Dispatch the configured post-release workflow for a published tag. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.