From d4a234b80f7d08f7a79a4f7d5769ced6d97c2678 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:20:43 +0000 Subject: [PATCH 1/5] fix(release): verify the GitHub release before the post-release dispatch create-release took gh's exit status, and the mere existence of a release named for the tag, as proof the release was there. gh can succeed on a release whose asset upload did not, and any release on the tag -- a draft, one flagged or titled differently, one left partial by an earlier attempt -- was accepted and handed straight to the post-release workflow, which publishes docs and images against it. The release is now read back from GitHub after it is created: it has to be on that 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; a missing or stale manifest is attached again (the one partial state a re-run can finish by itself), and a release this run did not make stops the job for a human instead of being passed off as this version's release. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9 --- packages/reflex-release/README.md | 21 ++ ...rify-release-before-post-release.bugfix.md | 1 + .../src/reflex_release/commands.py | 181 +++++++++++- tests/units/reflex_release/test_commands.py | 277 +++++++++++++++++- 4 files changed, 472 insertions(+), 8 deletions(-) create mode 100644 packages/reflex-release/news/+verify-release-before-post-release.bugfix.md diff --git a/packages/reflex-release/README.md b/packages/reflex-release/README.md index 9d6304cae9b..90311419865 100644 --- a/packages/reflex-release/README.md +++ b/packages/reflex-release/README.md @@ -422,6 +422,21 @@ distribution files by bare filename — exactly the files that went to PyPI — `sha256sum -c SHA256SUMS` works in a directory holding the downloaded wheel and sdist, years later. +`create-release` then reads the release back from GitHub rather than trusting +`gh`'s exit status, since the next step hands the tag to your [post-release +workflow](#post-release-workflow) and a `gh` call can succeed on a release whose +asset upload did not: the release has to be on that tag, published rather than +drafted, flagged and titled the way this run asked for, and carrying the +manifest at exactly the size that was built. Anything else fails the job before +the tag is handed on. + +A re-run of a release whose tag already has a release verifies it the same way +instead of taking its existence as success. A missing or stale manifest is the +one partial state a re-run finishes by itself — the attempt that died during the +asset upload — so it is attached again; a release that is drafted, flagged or +titled differently is one this run did not make, and it stops the job for a +human to look at rather than being passed off as this version's release. + For cryptographic provenance rather than a self-attested manifest — a PyPI [attestation](https://docs.pypi.org/attestations/) tying the artifact to the workflow that built it — publish with `pypa/gh-action-pypi-publish` in place of @@ -742,6 +757,12 @@ The dispatch is the last thing a release does, so a failure there never leaves a half-published version — but it does fail the run, loudly, naming the tag whose follow-up did not start. +It is also reached only through a release that was verified after it was created +(see [what the approval covers](#what-the-approval-actually-covers)), so a workflow that +publishes docs or images against the release can rely on the release being +complete — the tag published, correctly flagged, with its checksum manifest +attached — rather than merely existing. + ## Keeping the workflows current Bump `cli-command` in `pyproject.toml`, run `reflex-release sync`, commit the diff --git a/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md b/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md new file mode 100644 index 00000000000..49642558f32 --- /dev/null +++ b/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md @@ -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. diff --git a/packages/reflex-release/src/reflex_release/commands.py b/packages/reflex-release/src/reflex_release/commands.py index 6b6540718f5..f86d76e4bcb 100644 --- a/packages/reflex-release/src/reflex_release/commands.py +++ b/packages/reflex-release/src/reflex_release/commands.py @@ -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 @@ -71,6 +72,11 @@ _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. +_RELEASE_FIELDS = "tagName,name,isDraft,isPrerelease,assets" + def _is_null_sha(sha: str) -> bool: """Return whether a git sha is the all-zero "no such commit" placeholder. @@ -933,6 +939,144 @@ def cmd_push_tag(config: Config, tag: str) -> None: git_push(f"refs/tags/{tag}", config.root) +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. + payload = gh_output( + ["release", "view", tag, "--json", _RELEASE_FIELDS], config.root, check=False + ) + if not payload: + return None + try: + return json.loads(payload) + except json.JSONDecodeError: + fail(f"gh reported release metadata for {tag} that is not JSON: {payload!r}") + + +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.get("tagName")) != tag: + problems.append(f"it points at tag {tag_name!r} rather than {tag!r}") + if release.get("isDraft"): + problems.append("it is still a draft, so the version is not released") + if bool(release.get("isPrerelease")) != prerelease: + problems.append( + f"it is marked prerelease={bool(release.get('isPrerelease'))} rather " + f"than prerelease={prerelease}" + ) + if (name := release.get("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.get("assets") or () if asset.get("name") == name), + None, + ) + 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 +1089,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 +1106,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 +1136,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. diff --git a/tests/units/reflex_release/test_commands.py b/tests/units/reflex_release/test_commands.py index d690d641155..273f472d40b 100644 --- a/tests/units/reflex_release/test_commands.py +++ b/tests/units/reflex_release/test_commands.py @@ -4,8 +4,9 @@ import json import shutil -from collections.abc import Callable +from collections.abc import Callable, Sequence from pathlib import Path +from typing import Any import pytest from reflex_release import commands @@ -792,11 +793,106 @@ def test_open_release_pr_summary_links_the_pull_request( assert f"::notice::release pull request opened: {url}" in capsys.readouterr().out +#: The ``gh release create`` flags that consume the argument after them, so a +#: fake gh can tell an option value from an asset to attach. +_VALUED_FLAGS = frozenset({"--title", "--notes-file", "--target"}) + + +def release_payload( + tag: str, + title: str, + assets: Sequence[Path | dict[str, Any]] = (), + **overrides: Any, +) -> str: + """Render the ``gh release view`` payload of a released version. + + Args: + tag: The tag the release points at. + title: The release title. + assets: Files attached to the release — a path is rendered as the asset + GitHub reports for it, a dict is used as the asset entry itself, to + model an upload that did not finish. + **overrides: Fields to replace, to model a partial or stale release. + + Returns: + The JSON ``gh release view --json`` would print. + """ + payload: dict[str, Any] = { + "tagName": tag, + "name": title, + "isDraft": False, + "isPrerelease": False, + "assets": [ + asset + if isinstance(asset, dict) + else { + "name": asset.name, + "state": "uploaded", + "size": asset.stat().st_size, + } + for asset in assets + ], + } + return json.dumps(payload | overrides) + + +def created_release_payload(create_args: list[str]) -> str: + """Render the release a ``gh release create`` invocation would leave behind. + + Args: + create_args: The arguments of the ``gh release create`` call. + + Returns: + The JSON ``gh release view --json`` would print for it. + """ + title = "" + assets: list[Path] = [] + index = 3 + while index < len(create_args): + arg = create_args[index] + if arg in _VALUED_FLAGS: + if arg == "--title": + title = create_args[index + 1] + index += 2 + continue + if not arg.startswith("--"): + assets.append(Path(arg)) + index += 1 + return release_payload( + create_args[2], + title, + assets, + isPrerelease="--prerelease" in create_args, + ) + + +def stub_gh(monkeypatch: pytest.MonkeyPatch, views: list[str]) -> list[list[str]]: + """Stub the two gh helpers the release commands use. + + Args: + monkeypatch: The pytest monkeypatch fixture. + views: The ``gh release view`` payloads to answer reads with, in order; + the last one answers every further read. + + Returns: + The list every ``gh`` command that is run is appended to. + """ + calls: list[list[str]] = [] + queued = iter(views[:-1]) + monkeypatch.setattr( + commands, "gh_output", lambda *args, **kwargs: next(queued, views[-1]) + ) + monkeypatch.setattr( + commands, "gh_run", lambda args, *rest, **kwargs: calls.append(args) or 0 + ) + return calls + + @pytest.fixture def release_args( monkeypatch: pytest.MonkeyPatch, ) -> Callable[[], list[str]]: - """Capture the ``gh release create`` arguments instead of running gh. + """Fake gh for a first release: no release yet, then the one just created. Args: monkeypatch: The pytest monkeypatch fixture. @@ -805,7 +901,12 @@ def release_args( A callable returning the arguments of the last ``gh`` invocation. """ captured: list[list[str]] = [] - monkeypatch.setattr(commands, "gh_output", lambda *args, **kwargs: "") + + def view(*args: object, **kwargs: object) -> str: + creates = [call for call in captured if call[:2] == ["release", "create"]] + return created_release_payload(creates[-1]) if creates else "" + + monkeypatch.setattr(commands, "gh_output", view) monkeypatch.setattr( commands, "gh_run", lambda args, *rest, **kwargs: captured.append(args) or 0 ) @@ -880,6 +981,176 @@ def test_create_release_without_a_manifest_still_releases( assert str(checksums) not in release_args() +def test_create_release_reports_the_verified_release( + config: Config, + tmp_path: Path, + release_args: Callable[[], list[str]], + capsys: pytest.CaptureFixture, +) -> None: + notes, checksums = release_inputs(tmp_path) + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert "Release v0.2.1 created and verified" in capsys.readouterr().out + + +def test_create_release_fails_when_the_release_cannot_be_read_back( + config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A zero exit from gh is not proof of the release GitHub actually holds.""" + notes, checksums = release_inputs(tmp_path) + stub_gh(monkeypatch, [""]) + with pytest.raises(ReleaseError, match="reading it back found no release"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + + +def test_create_release_rejects_unparseable_release_metadata( + config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + notes, checksums = release_inputs(tmp_path) + stub_gh(monkeypatch, ["", "not json"]) + with pytest.raises(ReleaseError, match="that is not JSON"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + + +@pytest.mark.parametrize( + ("overrides", "expected"), + [ + ({"isDraft": True}, "still a draft"), + ({"isPrerelease": True}, "prerelease=True rather than prerelease=False"), + ({"name": "v0.2.0"}, "titled 'v0.2.0' rather than 'v0.2.1'"), + ({"tagName": "v0.2.0"}, "points at tag 'v0.2.0'"), + ], +) +def test_create_release_rejects_a_release_that_is_not_the_one_asked_for( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + overrides: dict[str, Any], + expected: str, +) -> None: + """The next step hands this tag on, so a mismatch stops the job here.""" + notes, checksums = release_inputs(tmp_path) + created = release_payload("v0.2.1", "v0.2.1", [checksums], **overrides) + stub_gh(monkeypatch, ["", created]) + with pytest.raises( + ReleaseError, match="is not the one that was asked for" + ) as raised: + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert expected in str(raised.value) + + +@pytest.mark.parametrize( + ("assets", "expected"), + [ + ([], "manifest is not attached"), + ([{"name": "SHA256SUMS", "state": "new", "size": 42}], "state 'new'"), + ([{"name": "SHA256SUMS", "state": "uploaded", "size": 7}], "7 bytes rather"), + ], +) +def test_create_release_rejects_an_incomplete_checksum_asset( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + assets: list[dict[str, Any]], + expected: str, +) -> None: + """A release without the manifest of what was uploaded is not a record of it.""" + notes, checksums = release_inputs(tmp_path) + created = release_payload("v0.2.1", "v0.2.1", assets=assets) + stub_gh(monkeypatch, ["", created]) + with pytest.raises(ReleaseError, match=rf"is incomplete: .*{expected}"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + + +def test_create_release_skips_a_matching_existing_release( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture, +) -> None: + """The safe re-run: the release is already exactly the one this run makes.""" + notes, checksums = release_inputs(tmp_path) + existing = release_payload("v0.2.1", "v0.2.1", [checksums]) + calls = stub_gh(monkeypatch, [existing]) + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls == [] + assert "already matches this publish" in capsys.readouterr().out + + +def test_create_release_attaches_a_manifest_an_earlier_attempt_left_off( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An attempt that died during the asset upload is finished, not accepted.""" + notes, checksums = release_inputs(tmp_path) + partial = release_payload("v0.2.1", "v0.2.1") + repaired = release_payload("v0.2.1", "v0.2.1", [checksums]) + calls = stub_gh(monkeypatch, [partial, repaired]) + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls == [["release", "upload", "v0.2.1", str(checksums), "--clobber"]] + + +def test_create_release_fails_when_attaching_the_manifest_does_not_take( + config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + notes, checksums = release_inputs(tmp_path) + stub_gh(monkeypatch, [release_payload("v0.2.1", "v0.2.1")]) + with pytest.raises(ReleaseError, match="reading it back found that"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + + +@pytest.mark.parametrize( + "overrides", + [{"isDraft": True}, {"isPrerelease": True}, {"name": "something else"}], +) +def test_create_release_refuses_to_reuse_a_stale_release( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + overrides: dict[str, Any], +) -> None: + """A release nobody here made must not be handed on as this publish's.""" + notes, checksums = release_inputs(tmp_path) + existing = release_payload("v0.2.1", "v0.2.1", [checksums], **overrides) + calls = stub_gh(monkeypatch, [existing]) + with pytest.raises(ReleaseError, match=r"already exists for v0\.2\.1"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls == [] + + +def test_create_release_reuses_a_release_without_a_local_manifest( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Nothing to attach and nothing to verify: the metadata is the whole check.""" + notes, checksums = release_inputs(tmp_path) + checksums.unlink() + calls = stub_gh(monkeypatch, [release_payload("v0.2.1", "v0.2.1")]) + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls == [] + + def test_post_release_without_a_configured_workflow_does_nothing( config: Config, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture ) -> None: From f5a4b0b46f40701935892f31ad7420312306d1ac Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:33:42 +0000 Subject: [PATCH 2/5] fix(release): tell an absent release from an unreadable one The verification read used gh_output(..., check=False), which flattens every failure to an empty string: a transient API error, an expired token or a rate limit read exactly like "this tag has no release". That made a re-run try to create a release over one that already exists, and reported a release gh had just created as missing. gh_capture returns the exit status, stdout and stderr, so the read can classify: gh saying the release is not there means absent, anything else is a failure to read and says so, naming what gh reported. A phrasing the not-found markers miss would stop every ordinary release, so a bare 404 counts as absent too -- the same job pushed this tag through the same gh, so a not-found on the releases endpoint is about the release. The requested field list is now the one place the fields are named, and a payload missing any of them fails loudly instead of reading as a mismatch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9 --- .../src/reflex_release/commands.py | 56 +++++--- .../src/reflex_release/gitutil.py | 30 ++++- tests/units/reflex_release/test_commands.py | 122 +++++++++++++++--- 3 files changed, 170 insertions(+), 38 deletions(-) diff --git a/packages/reflex-release/src/reflex_release/commands.py b/packages/reflex-release/src/reflex_release/commands.py index f86d76e4bcb..f28c49d91b7 100644 --- a/packages/reflex-release/src/reflex_release/commands.py +++ b/packages/reflex-release/src/reflex_release/commands.py @@ -51,6 +51,7 @@ changed_files, commit_exists, configure_bot_identity, + gh_capture, gh_output, gh_run, git, @@ -74,8 +75,16 @@ #: 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. -_RELEASE_FIELDS = "tagName,name,isDraft,isPrerelease,assets" +#: 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 that mean the tag has no release, as opposed to a +#: release gh could not ask GitHub about. A bare 404 counts: the same job +#: pushed this tag through the same gh, so a not-found on the releases endpoint +#: is about the release rather than the repository — and reading a phrasing +#: this list misses as a hard failure would stop every ordinary release. +_NO_RELEASE_STDERR = ("release not found", "404") def _is_null_sha(sha: str) -> bool: @@ -952,15 +961,33 @@ def _release_view(config: Config, tag: str) -> dict[str, Any] | None: """ # A probe, not an action: keep its output (and its "not found" stderr on the # normal path) out of the job log. - payload = gh_output( - ["release", "view", tag, "--json", _RELEASE_FIELDS], config.root, check=False + returncode, payload, stderr = gh_capture( + ["release", "view", tag, "--json", ",".join(_RELEASE_FIELDS)], config.root ) - if not payload: - return None + 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. + lowered = stderr.lower() + if any(marker in lowered for marker in _NO_RELEASE_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: - return json.loads(payload) + release = json.loads(payload) except json.JSONDecodeError: fail(f"gh reported release metadata for {tag} that is not JSON: {payload!r}") + if missing := [field for field in _RELEASE_FIELDS if field not in release]: + fail( + f"the release metadata gh reported for {tag} carries no " + f"{', '.join(missing)}, so the release cannot be verified" + ) + return release def _metadata_problems( @@ -981,16 +1008,16 @@ def _metadata_problems( # repository rather than to one release, so a release published since this # one legitimately holds it. problems = [] - if (tag_name := release.get("tagName")) != tag: + if (tag_name := release["tagName"]) != tag: problems.append(f"it points at tag {tag_name!r} rather than {tag!r}") - if release.get("isDraft"): + if release["isDraft"]: problems.append("it is still a draft, so the version is not released") - if bool(release.get("isPrerelease")) != prerelease: + if (is_prerelease := bool(release["isPrerelease"])) != prerelease: problems.append( - f"it is marked prerelease={bool(release.get('isPrerelease'))} rather " - f"than prerelease={prerelease}" + f"it is marked prerelease={is_prerelease} rather than " + f"prerelease={prerelease}" ) - if (name := release.get("name")) != title: + if (name := release["name"]) != title: problems.append(f"it is titled {name!r} rather than {title!r}") return problems @@ -1010,8 +1037,7 @@ def _checksum_asset_problem( """ name = checksums_path.name asset = next( - (asset for asset in release.get("assets") or () if asset.get("name") == name), - None, + (asset for asset in release["assets"] if asset.get("name") == name), None ) if asset is None: return f"the {name} manifest is not attached to it" diff --git a/packages/reflex-release/src/reflex_release/gitutil.py b/packages/reflex-release/src/reflex_release/gitutil.py index f91bdb45da6..cb0860324e1 100644 --- a/packages/reflex-release/src/reflex_release/gitutil.py +++ b/packages/reflex-release/src/reflex_release/gitutil.py @@ -251,6 +251,26 @@ def gh_run(args: list[str], cwd: Path, check: bool = True) -> int: return returncode +def gh_capture(args: list[str], cwd: Path) -> tuple[int, str, str]: + """Run a GitHub CLI command, returning everything it reported. + + For callers that have to tell one failure from another: through an exit + status alone, "GitHub says there is no such release" and "GitHub could not + be asked" are the same answer. + + Args: + args: The ``gh`` arguments. + cwd: The repository directory. + + Returns: + The exit status, and stdout and stderr, both stripped. + """ + result = subprocess.run( + ["gh", *args], cwd=cwd, capture_output=True, text=True, check=False + ) + return result.returncode, result.stdout.strip(), result.stderr.strip() + + def gh_output(args: list[str], cwd: Path, check: bool = True) -> str: """Run a GitHub CLI command whose stdout is consumed by the caller. @@ -263,11 +283,9 @@ def gh_output(args: list[str], cwd: Path, check: bool = True) -> str: The command's stdout, stripped. Empty when it failed and ``check`` is False. """ - result = subprocess.run( - ["gh", *args], cwd=cwd, capture_output=True, text=True, check=False - ) - if result.returncode != 0: + returncode, stdout, stderr = gh_capture(args, cwd) + if returncode != 0: if check: - fail(f"gh {' '.join(args)} failed: {result.stderr.strip()}") + fail(f"gh {' '.join(args)} failed: {stderr}") return "" - return result.stdout.strip() + return stdout diff --git a/tests/units/reflex_release/test_commands.py b/tests/units/reflex_release/test_commands.py index 273f472d40b..bd8eaa21224 100644 --- a/tests/units/reflex_release/test_commands.py +++ b/tests/units/reflex_release/test_commands.py @@ -866,21 +866,50 @@ def created_release_payload(create_args: list[str]) -> str: ) -def stub_gh(monkeypatch: pytest.MonkeyPatch, views: list[str]) -> list[list[str]]: +#: What gh reports for a tag that simply has no release. +GH_NO_RELEASE = (1, "", "release not found") + +#: The same answer phrased as the bare HTTP status, as gh reports it when the +#: release lookup itself is what 404s. +GH_NO_RELEASE_404 = ( + 1, + "", + "gh: Not Found (HTTP 404) (https://api.github.com/repos/acme/widgets/releases/tags/v0.2.1)", +) + +#: What gh reports when it could not ask GitHub at all. +GH_UNREACHABLE = (1, "", "HTTP 503: Service Unavailable (https://api.github.com)") + + +def gh_found(payload: str) -> tuple[int, str, str]: + """Render what gh reports for a release it read successfully. + + Args: + payload: The ``gh release view --json`` output. + + Returns: + The exit status, stdout and stderr of that read. + """ + return (0, payload, "") + + +def stub_gh( + monkeypatch: pytest.MonkeyPatch, reads: list[tuple[int, str, str]] +) -> list[list[str]]: """Stub the two gh helpers the release commands use. Args: monkeypatch: The pytest monkeypatch fixture. - views: The ``gh release view`` payloads to answer reads with, in order; - the last one answers every further read. + reads: What ``gh release view`` reports, in order; the last entry + answers every further read. Returns: The list every ``gh`` command that is run is appended to. """ calls: list[list[str]] = [] - queued = iter(views[:-1]) + queued = iter(reads[:-1]) monkeypatch.setattr( - commands, "gh_output", lambda *args, **kwargs: next(queued, views[-1]) + commands, "gh_capture", lambda *args, **kwargs: next(queued, reads[-1]) ) monkeypatch.setattr( commands, "gh_run", lambda args, *rest, **kwargs: calls.append(args) or 0 @@ -902,11 +931,13 @@ def release_args( """ captured: list[list[str]] = [] - def view(*args: object, **kwargs: object) -> str: + def view(*args: object, **kwargs: object) -> tuple[int, str, str]: creates = [call for call in captured if call[:2] == ["release", "create"]] - return created_release_payload(creates[-1]) if creates else "" + if not creates: + return GH_NO_RELEASE + return gh_found(created_release_payload(creates[-1])) - monkeypatch.setattr(commands, "gh_output", view) + monkeypatch.setattr(commands, "gh_capture", view) monkeypatch.setattr( commands, "gh_run", lambda args, *rest, **kwargs: captured.append(args) or 0 ) @@ -999,7 +1030,7 @@ def test_create_release_fails_when_the_release_cannot_be_read_back( ) -> None: """A zero exit from gh is not proof of the release GitHub actually holds.""" notes, checksums = release_inputs(tmp_path) - stub_gh(monkeypatch, [""]) + stub_gh(monkeypatch, [GH_NO_RELEASE]) with pytest.raises(ReleaseError, match="reading it back found no release"): commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums @@ -1010,7 +1041,7 @@ def test_create_release_rejects_unparseable_release_metadata( config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: notes, checksums = release_inputs(tmp_path) - stub_gh(monkeypatch, ["", "not json"]) + stub_gh(monkeypatch, [GH_NO_RELEASE, gh_found("not json")]) with pytest.raises(ReleaseError, match="that is not JSON"): commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums @@ -1036,7 +1067,7 @@ def test_create_release_rejects_a_release_that_is_not_the_one_asked_for( """The next step hands this tag on, so a mismatch stops the job here.""" notes, checksums = release_inputs(tmp_path) created = release_payload("v0.2.1", "v0.2.1", [checksums], **overrides) - stub_gh(monkeypatch, ["", created]) + stub_gh(monkeypatch, [GH_NO_RELEASE, gh_found(created)]) with pytest.raises( ReleaseError, match="is not the one that was asked for" ) as raised: @@ -1064,7 +1095,7 @@ def test_create_release_rejects_an_incomplete_checksum_asset( """A release without the manifest of what was uploaded is not a record of it.""" notes, checksums = release_inputs(tmp_path) created = release_payload("v0.2.1", "v0.2.1", assets=assets) - stub_gh(monkeypatch, ["", created]) + stub_gh(monkeypatch, [GH_NO_RELEASE, gh_found(created)]) with pytest.raises(ReleaseError, match=rf"is incomplete: .*{expected}"): commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums @@ -1080,7 +1111,7 @@ def test_create_release_skips_a_matching_existing_release( """The safe re-run: the release is already exactly the one this run makes.""" notes, checksums = release_inputs(tmp_path) existing = release_payload("v0.2.1", "v0.2.1", [checksums]) - calls = stub_gh(monkeypatch, [existing]) + calls = stub_gh(monkeypatch, [gh_found(existing)]) commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums ) @@ -1097,7 +1128,7 @@ def test_create_release_attaches_a_manifest_an_earlier_attempt_left_off( notes, checksums = release_inputs(tmp_path) partial = release_payload("v0.2.1", "v0.2.1") repaired = release_payload("v0.2.1", "v0.2.1", [checksums]) - calls = stub_gh(monkeypatch, [partial, repaired]) + calls = stub_gh(monkeypatch, [gh_found(partial), gh_found(repaired)]) commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums ) @@ -1108,7 +1139,7 @@ def test_create_release_fails_when_attaching_the_manifest_does_not_take( config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: notes, checksums = release_inputs(tmp_path) - stub_gh(monkeypatch, [release_payload("v0.2.1", "v0.2.1")]) + stub_gh(monkeypatch, [gh_found(release_payload("v0.2.1", "v0.2.1"))]) with pytest.raises(ReleaseError, match="reading it back found that"): commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums @@ -1128,7 +1159,7 @@ def test_create_release_refuses_to_reuse_a_stale_release( """A release nobody here made must not be handed on as this publish's.""" notes, checksums = release_inputs(tmp_path) existing = release_payload("v0.2.1", "v0.2.1", [checksums], **overrides) - calls = stub_gh(monkeypatch, [existing]) + calls = stub_gh(monkeypatch, [gh_found(existing)]) with pytest.raises(ReleaseError, match=r"already exists for v0\.2\.1"): commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums @@ -1144,13 +1175,70 @@ def test_create_release_reuses_a_release_without_a_local_manifest( """Nothing to attach and nothing to verify: the metadata is the whole check.""" notes, checksums = release_inputs(tmp_path) checksums.unlink() - calls = stub_gh(monkeypatch, [release_payload("v0.2.1", "v0.2.1")]) + calls = stub_gh(monkeypatch, [gh_found(release_payload("v0.2.1", "v0.2.1"))]) commands.cmd_create_release( config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums ) assert calls == [] +@pytest.mark.parametrize("absent", [GH_NO_RELEASE, GH_NO_RELEASE_404]) +def test_create_release_creates_when_gh_says_there_is_no_release( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + absent: tuple[int, str, str], +) -> None: + """Either phrasing of "no release" has to reach the create, not a failure.""" + notes, checksums = release_inputs(tmp_path) + created = release_payload("v0.2.1", "v0.2.1", [checksums]) + calls = stub_gh(monkeypatch, [absent, gh_found(created)]) + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls[0][:3] == ["release", "create", "v0.2.1"] + + +def test_create_release_fails_when_reading_the_created_release_fails( + config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A gh that could not reach GitHub has not said the release is absent.""" + notes, checksums = release_inputs(tmp_path) + calls = stub_gh(monkeypatch, [GH_NO_RELEASE, GH_UNREACHABLE]) + with pytest.raises(ReleaseError, match="could not read the GitHub release"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls[0][:2] == ["release", "create"] + + +def test_create_release_does_not_create_over_an_unreadable_release( + config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Treating an unreadable release as absent would release over it.""" + notes, checksums = release_inputs(tmp_path) + calls = stub_gh(monkeypatch, [GH_UNREACHABLE]) + with pytest.raises(ReleaseError, match="re-run this job once gh can reach"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls == [] + + +def test_create_release_rejects_metadata_missing_a_requested_field( + config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A field that was asked for but not answered must not read as a mismatch.""" + notes, checksums = release_inputs(tmp_path) + payload = json.loads(release_payload("v0.2.1", "v0.2.1", [checksums])) + del payload["isDraft"] + stub_gh(monkeypatch, [gh_found(json.dumps(payload))]) + with pytest.raises(ReleaseError, match="carries no isDraft"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + + def test_post_release_without_a_configured_workflow_does_nothing( config: Config, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture ) -> None: From fbfb5944c2f318b2aefe2b3cb549376d6f4ff89e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:39:40 +0000 Subject: [PATCH 3/5] fix(release): corroborate a not-found before treating a release as absent Matching gh's stderr widely enough to survive a phrasing change means a bare 404 counts as "no release" -- but a 404 can also come from a repository that is missing, renamed or beyond the token's reach, and that says nothing about the release. A not-found is now believed only once the repository itself reads back: one extra gh call, on the path where a release is about to be created anyway. An unreadable repository fails the read instead, so a release is never created over one that may already exist. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9 --- packages/reflex-release/README.md | 6 +++ ...rify-release-before-post-release.bugfix.md | 2 +- .../src/reflex_release/commands.py | 40 ++++++++++++++--- tests/units/reflex_release/test_commands.py | 44 ++++++++++++++++--- 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/packages/reflex-release/README.md b/packages/reflex-release/README.md index 90311419865..35165444f68 100644 --- a/packages/reflex-release/README.md +++ b/packages/reflex-release/README.md @@ -430,6 +430,12 @@ drafted, flagged and titled the way this run asked for, and carrying the manifest at exactly the size that was built. Anything else fails the job before the tag is handed on. +A read that fails is not an absent release: gh answers "there is no such +release" and "GitHub could not be asked" the same way, so a not-found is +believed only once the repository itself reads back, and anything else stops +the job saying what gh reported. Otherwise a rate-limited or unauthenticated +read would look exactly like a tag with no release yet. + A re-run of a release whose tag already has a release verifies it the same way instead of taking its existence as success. A missing or stale manifest is the one partial state a re-run finishes by itself — the attempt that died during the diff --git a/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md b/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md index 49642558f32..99235e31d2a 100644 --- a/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md +++ b/packages/reflex-release/news/+verify-release-before-post-release.bugfix.md @@ -1 +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. +`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. diff --git a/packages/reflex-release/src/reflex_release/commands.py b/packages/reflex-release/src/reflex_release/commands.py index f28c49d91b7..ad2cf35a4f9 100644 --- a/packages/reflex-release/src/reflex_release/commands.py +++ b/packages/reflex-release/src/reflex_release/commands.py @@ -79,11 +79,11 @@ #: field can never be verified without having been requested. _RELEASE_FIELDS = ("tagName", "name", "isDraft", "isPrerelease", "assets") -#: Markers in gh's stderr that mean the tag has no release, as opposed to a -#: release gh could not ask GitHub about. A bare 404 counts: the same job -#: pushed this tag through the same gh, so a not-found on the releases endpoint -#: is about the release rather than the repository — and reading a phrasing -#: this list misses as a hard failure would stop every ordinary release. +#: 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") @@ -948,6 +948,33 @@ 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 + + def _release_view(config: Config, tag: str) -> dict[str, Any] | None: """Read a GitHub release's metadata and asset list. @@ -969,8 +996,7 @@ def _release_view(config: Config, tag: str) -> dict[str, Any] | None: # 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. - lowered = stderr.lower() - if any(marker in lowered for marker in _NO_RELEASE_STDERR): + if _reads_as_absent(config, stderr): return None fail( f"could not read the GitHub release for {tag}: gh exited " diff --git a/tests/units/reflex_release/test_commands.py b/tests/units/reflex_release/test_commands.py index bd8eaa21224..420cb55f010 100644 --- a/tests/units/reflex_release/test_commands.py +++ b/tests/units/reflex_release/test_commands.py @@ -893,8 +893,17 @@ def gh_found(payload: str) -> tuple[int, str, str]: return (0, payload, "") +#: What gh reports for a repository it read without trouble. +GH_REPO_READS = (0, '{"name": "widgets"}', "") + +#: What gh reports for a repository it could not read at all. +GH_REPO_UNREADABLE = (1, "", "gh: Not Found (HTTP 404)") + + def stub_gh( - monkeypatch: pytest.MonkeyPatch, reads: list[tuple[int, str, str]] + monkeypatch: pytest.MonkeyPatch, + reads: list[tuple[int, str, str]], + repo: tuple[int, str, str] = GH_REPO_READS, ) -> list[list[str]]: """Stub the two gh helpers the release commands use. @@ -902,15 +911,21 @@ def stub_gh( monkeypatch: The pytest monkeypatch fixture. reads: What ``gh release view`` reports, in order; the last entry answers every further read. + repo: What the ``gh repo view`` probe reports — the check that a + not-found is about the release and not about reaching GitHub. Returns: The list every ``gh`` command that is run is appended to. """ calls: list[list[str]] = [] queued = iter(reads[:-1]) - monkeypatch.setattr( - commands, "gh_capture", lambda *args, **kwargs: next(queued, reads[-1]) - ) + + def capture(args: list[str], *rest: object, **kwargs: object): + if args[:2] == ["repo", "view"]: + return repo + return next(queued, reads[-1]) + + monkeypatch.setattr(commands, "gh_capture", capture) monkeypatch.setattr( commands, "gh_run", lambda args, *rest, **kwargs: calls.append(args) or 0 ) @@ -931,7 +946,9 @@ def release_args( """ captured: list[list[str]] = [] - def view(*args: object, **kwargs: object) -> tuple[int, str, str]: + def view(args: list[str], *rest: object, **kwargs: object) -> tuple[int, str, str]: + if args[:2] == ["repo", "view"]: + return GH_REPO_READS creates = [call for call in captured if call[:2] == ["release", "create"]] if not creates: return GH_NO_RELEASE @@ -1199,6 +1216,23 @@ def test_create_release_creates_when_gh_says_there_is_no_release( assert calls[0][:3] == ["release", "create", "v0.2.1"] +@pytest.mark.parametrize("absent", [GH_NO_RELEASE, GH_NO_RELEASE_404]) +def test_create_release_will_not_believe_a_not_found_it_cannot_corroborate( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + absent: tuple[int, str, str], +) -> None: + """A 404 from an unreachable repository says nothing about the release.""" + notes, checksums = release_inputs(tmp_path) + calls = stub_gh(monkeypatch, [absent], repo=GH_REPO_UNREADABLE) + with pytest.raises(ReleaseError, match="could not read the GitHub release"): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + assert calls == [] + + def test_create_release_fails_when_reading_the_created_release_fails( config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From a11089e51d3fa2d46e42655155a4d6a1fc0afabf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:51:41 +0000 Subject: [PATCH 4/5] fix(release): establish the release payload's shape at the boundary json.loads accepts more than the object gh documents: a bare null or a list would reach the field check and raise TypeError, and a null assets field would raise on iteration. A traceback out of a job that has already published a version says nothing about what went wrong. The shape is now established once, where the payload enters -- an object, carrying every field that was requested, with assets as a list -- so each miss is a diagnostic failure and everything downstream can read the payload as the release it claims to be. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9 --- .../src/reflex_release/commands.py | 14 ++++++++ tests/units/reflex_release/test_commands.py | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/packages/reflex-release/src/reflex_release/commands.py b/packages/reflex-release/src/reflex_release/commands.py index ad2cf35a4f9..e6a1c617bf6 100644 --- a/packages/reflex-release/src/reflex_release/commands.py +++ b/packages/reflex-release/src/reflex_release/commands.py @@ -1008,11 +1008,25 @@ def _release_view(config: Config, tag: str) -> dict[str, Any] | None: 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]: fail( f"the release metadata gh reported for {tag} carries no " f"{', '.join(missing)}, so the release cannot be verified" ) + if not isinstance(release["assets"], list): + fail( + f"the release metadata gh reported for {tag} lists its assets as " + f"{release['assets']!r} rather than as a list, so what the release " + "carries cannot be verified" + ) return release diff --git a/tests/units/reflex_release/test_commands.py b/tests/units/reflex_release/test_commands.py index 420cb55f010..8e2b4772c33 100644 --- a/tests/units/reflex_release/test_commands.py +++ b/tests/units/reflex_release/test_commands.py @@ -1259,6 +1259,40 @@ def test_create_release_does_not_create_over_an_unreadable_release( assert calls == [] +#: A payload shaped like a release except that its assets are not a list. +NULL_ASSETS_PAYLOAD = json.dumps({ + "tagName": "v0.2.1", + "name": "v0.2.1", + "isDraft": False, + "isPrerelease": False, + "assets": None, +}) + + +@pytest.mark.parametrize( + ("payload", "expected"), + [ + ("null", "is not an object"), + ("[]", "is not an object"), + (NULL_ASSETS_PAYLOAD, "rather than as a list"), + ], +) +def test_create_release_rejects_metadata_of_the_wrong_shape( + config: Config, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + payload: str, + expected: str, +) -> None: + """A shape gh should never send has to read as a diagnostic, not a traceback.""" + notes, checksums = release_inputs(tmp_path) + stub_gh(monkeypatch, [gh_found(payload)]) + with pytest.raises(ReleaseError, match=expected): + commands.cmd_create_release( + config, "v0.2.1", "mypkg", "0.2.1", False, True, notes, checksums + ) + + def test_create_release_rejects_metadata_missing_a_requested_field( config: Config, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From 9e3828ade77106efcb7657d569b1f6be6d99a162 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:56:27 +0000 Subject: [PATCH 5/5] fix(release): require the assets gh reports to be assets The boundary check established that assets is a list but not what the list holds, so a null or a bare string among them still reached asset.get() and raised. Every entry now has to be an object for the payload to be accepted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9 --- .../src/reflex_release/commands.py | 7 +++-- tests/units/reflex_release/test_commands.py | 28 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/reflex-release/src/reflex_release/commands.py b/packages/reflex-release/src/reflex_release/commands.py index e6a1c617bf6..94b2e796ca1 100644 --- a/packages/reflex-release/src/reflex_release/commands.py +++ b/packages/reflex-release/src/reflex_release/commands.py @@ -1021,10 +1021,13 @@ def _release_view(config: Config, tag: str) -> dict[str, Any] | None: f"the release metadata gh reported for {tag} carries no " f"{', '.join(missing)}, so the release cannot be verified" ) - if not isinstance(release["assets"], list): + 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"{release['assets']!r} rather than as a list, so what the release " + f"{assets!r} rather than as a list of assets, so what the release " "carries cannot be verified" ) return release diff --git a/tests/units/reflex_release/test_commands.py b/tests/units/reflex_release/test_commands.py index 8e2b4772c33..97e03b9c0c7 100644 --- a/tests/units/reflex_release/test_commands.py +++ b/tests/units/reflex_release/test_commands.py @@ -1259,14 +1259,22 @@ def test_create_release_does_not_create_over_an_unreadable_release( assert calls == [] -#: A payload shaped like a release except that its assets are not a list. -NULL_ASSETS_PAYLOAD = json.dumps({ - "tagName": "v0.2.1", - "name": "v0.2.1", - "isDraft": False, - "isPrerelease": False, - "assets": None, -}) +def malformed_assets_payload(assets: Any) -> str: + """Render a release payload whose assets are not a list of assets. + + Args: + assets: The malformed value to report as the release's assets. + + Returns: + The JSON ``gh release view --json`` would print for it. + """ + return json.dumps({ + "tagName": "v0.2.1", + "name": "v0.2.1", + "isDraft": False, + "isPrerelease": False, + "assets": assets, + }) @pytest.mark.parametrize( @@ -1274,7 +1282,9 @@ def test_create_release_does_not_create_over_an_unreadable_release( [ ("null", "is not an object"), ("[]", "is not an object"), - (NULL_ASSETS_PAYLOAD, "rather than as a list"), + (malformed_assets_payload(None), "rather than as a list of assets"), + (malformed_assets_payload([None]), "rather than as a list of assets"), + (malformed_assets_payload(["SHA256SUMS"]), "rather than as a list of assets"), ], ) def test_create_release_rejects_metadata_of_the_wrong_shape(