Verify GitHub release before handing to post-release workflow - #6927
Verify GitHub release before handing to post-release workflow#6927masenf wants to merge 5 commits into
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThe PR verifies GitHub release metadata and checksum assets before allowing post-release processing, including safer handling of reruns and failed release reads.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| packages/reflex-release/src/reflex_release/commands.py | Adds release read-back, metadata validation, checksum verification, and rerun handling. |
| packages/reflex-release/src/reflex_release/gitutil.py | Adds a GitHub CLI capture helper preserving exit status, stdout, and stderr. |
| tests/units/reflex_release/test_commands.py | Expands release-command coverage across creation, read failures, malformed metadata, asset validation, and reruns. |
| packages/reflex-release/README.md | Documents verified release creation and safe rerun behavior. |
Reviews (5): Last reviewed commit: "fix(release): require the assets gh repo..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9
…sent 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-release/src/reflex_release/commands.py">
<violation number="1" location="packages/reflex-release/src/reflex_release/commands.py:975">
P2: When `gh release view` returns a 404 for an existing draft or otherwise hidden release, a successful `gh repo view` makes `_reads_as_absent` treat the release as absent. `cmd_create_release` then attempts `gh release create` for the existing tag instead of rejecting or completing the existing release. Use a draft-capable release listing or ID lookup before treating the tag as absent.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| # 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 |
There was a problem hiding this comment.
P2: When gh release view returns a 404 for an existing draft or otherwise hidden release, a successful gh repo view makes _reads_as_absent treat the release as absent. cmd_create_release then attempts gh release create for the existing tag instead of rejecting or completing the existing release. Use a draft-capable release listing or ID lookup before treating the tag as absent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-release/src/reflex_release/commands.py, line 975:
<comment>When `gh release view` returns a 404 for an existing draft or otherwise hidden release, a successful `gh repo view` makes `_reads_as_absent` treat the release as absent. `cmd_create_release` then attempts `gh release create` for the existing tag instead of rejecting or completing the existing release. Use a draft-capable release listing or ID lookup before treating the tag as absent.</comment>
<file context>
@@ -939,6 +948,33 @@ def cmd_push_tag(config: Config, tag: str) -> None:
+ # 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
+
+
</file context>
There was a problem hiding this comment.
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 — gh release view goes to get-a-release-by-tag, which returns published releases only. But the consequence is milder than "attempts gh release create ... instead of rejecting or completing the existing release": drafts don't reserve a tag, so the create succeeds, and the release it creates is then read back and verified against the tag, draft flag, title and checksum manifest before anything is dispatched. What's left behind is a stale draft someone else made — untidy, not a bad release, and not something this PR regressed (before it, the same 404 went to create with no verification at all). A draft that is visible is already rejected by the isDraft check this PR adds.
A gh release list scan would catch the invisible case, at the cost of another call on the path every ordinary release takes plus a pagination window that can't be made airtight. That's a behavioral addition worth its own change, so I've flagged it to the PR author rather than folding it in here. Greptile raised the same point on r3833580461 and landed in the same place.
Generated by Claude Code
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9
3 participants
Type of change
Description
create-releasenow reads the GitHub release back from GitHub instead of trustinggh's exit status. This ensures the tag handed to thepost-release-workflowis one that was verified to be complete and correctly configured.Why this matters:
gh release createcan succeed even if the asset upload fails, leaving a release without its checksum manifest. The next step (post-release workflow) publishes docs and images against the release, so it must be complete and correctly flagged. This change verifies:Re-run behavior: A re-run of a release whose tag already has a release verifies it the same way. A missing or stale manifest (from an attempt that died during asset upload) is the one partial state a re-run finishes by itself — it uploads the manifest 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 inspect rather than being passed off as this version's release.
Changes
Added helper functions to read and validate GitHub releases:
_release_view(): reads release metadata from GitHub_metadata_problems(): validates release matches requested configuration_checksum_asset_problem(): validates checksum manifest is attached correctly_accept_existing_release(): handles re-run scenarios with existing releasesUpdated
cmd_create_release()to:Added comprehensive test coverage for all validation scenarios:
Updated README documentation to explain the verification process and re-run behavior
Test Plan
All new functionality is covered by 11 new test cases in
tests/units/reflex_release/test_commands.py:test_create_release_reports_the_verified_releasetest_create_release_fails_when_the_release_cannot_be_read_backtest_create_release_rejects_unparseable_release_metadatatest_create_release_rejects_a_release_that_is_not_the_one_asked_for(4 parameterized cases)test_create_release_rejects_an_incomplete_checksum_asset(3 parameterized cases)test_create_release_skips_a_matching_existing_releasetest_create_release_attaches_a_manifest_an_earlier_attempt_left_offtest_create_release_fails_when_attaching_the_manifest_does_not_taketest_create_release_refuses_to_reuse_a_stale_release(3 parameterized cases)test_create_release_reuses_a_release_without_a_local_manifestExisting tests continue to pass with the updated
release_argsfixture, which now simulates the full release lifecycle including verification.https://claude.ai/code/session_01KRAGNRmLGo2g59foKazSb9