Create GitHub Releases after publish - #406
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe release workflow now creates or reconciles GitHub Releases after PyPI publication. Validators enforce tag, artifact, provenance, permission, retry, and promotion requirements. The sdist includes and structurally validates the documentation deployment workflow. ChangesRelease and promotion flow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant TagPush
participant ReleaseWorkflow
participant PyPI
participant GitHubRelease
participant DocsPromotion
TagPush->>ReleaseWorkflow: verify tag source commit
ReleaseWorkflow->>PyPI: publish distributions
ReleaseWorkflow->>GitHubRelease: generate provenance and create or reconcile release
DocsPromotion->>GitHubRelease: poll metadata and verify assets
DocsPromotion->>PyPI: verify non-yanked files and hashes
DocsPromotion->>DocsPromotion: allow promotion after matching checks
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 335-338: Update the existing-release branch around gh release view
to query and validate isDraft, name, and isPrerelease before uploading assets.
If the release state or metadata is incorrect, publish it with the required
metadata; otherwise preserve the asset refresh and successful retry path, and
fail the job when validation or correction cannot succeed.
In `@scripts/verify_ci_workflow.py`:
- Around line 826-850: The github-release workflow contract does not enforce the
no-artifact failure guard. In scripts/verify_ci_workflow.py lines 826-850,
extend the _require_job_contains assertion to require nullglob, the artifacts
array, and the zero-artifact exit check; in tests/test_verify_ci_workflow.py
lines 772-790, update the mutation test to remove that guard from the workflow
and assert validation fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 154c7bbd-51be-414d-a490-21b9db65e5ea
📒 Files selected for processing (5)
.github/workflows/deploy-docs-stg.yml.github/workflows/release.ymlscripts/verify_ci_workflow.pyspec/process/production-readiness.mdtests/test_verify_ci_workflow.py
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_verify_ci_workflow.py (1)
2109-2117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign the simulated shell options with the shell the workflow actually uses.
The simulations invoke
bash -e -o pipefail. Thegithub-releasesteps in.github/workflows/release.ymland the release gate step in.github/workflows/deploy-docs-stg.ymldeclare noshell:key, so GitHub Actions runs them with the defaultbash -e {0}and does not enablepipefail.The simulation is therefore stricter than production. A pipeline whose non-final element fails aborts in the test but continues in production, for example
sha256sum "$file" | cut -d ' ' -f 1. That failure mode stays untested.Choose one of two options:
- Drop
-o pipefailfrom the simulations so they match production.- Add
shell: bashto the affected workflow steps so production enablespipefail, and keep the simulations as they are.The second option is the safer one for the release path.
♻️ Option 2: make production match the simulations
- name: Prepare release provenance if: steps.release_state.outputs.immutable != 'true' + shell: bash env: GH_TOKEN: ${{ github.token }}Apply the same
shell: bashkey toInspect existing release,Create GitHub Release and attach distributions, andAwait GitHub Release and PyPI availability.Also applies to: 2135-2143, 2244-2252, 2452-2460, 2736-2744
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_verify_ci_workflow.py` around lines 2109 - 2117, Add shell: bash to the workflow steps corresponding to Inspect existing release, Create GitHub Release and attach distributions, and Await GitHub Release and PyPI availability, including every affected occurrence. Keep the existing bash -e -o pipefail simulations unchanged so production uses the same pipefail behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_verify_ci_workflow.py`:
- Around line 2109-2117: Add shell: bash to the workflow steps corresponding to
Inspect existing release, Create GitHub Release and attach distributions, and
Await GitHub Release and PyPI availability, including every affected occurrence.
Keep the existing bash -e -o pipefail simulations unchanged so production uses
the same pipefail behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b75fe48-e412-4b74-9441-05ebbbe93524
📒 Files selected for processing (8)
.github/workflows/deploy-docs-stg.yml.github/workflows/release.ymlpyproject.tomlscripts/verify_ci_workflow.pyscripts/verify_sdist.pyspec/process/production-readiness.mdtests/test_verify_ci_workflow.pytests/test_verify_sdist.py
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
scripts/verify_ci_workflow.py (1)
2654-2693: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport which invariant failed instead of one aggregated message.
This composite check combines about fifteen independent conditions, several of which depend on hard-coded logical-line offsets such as
tag_resolution_positions[0] + 10. When a maintainer edits.github/workflows/release.yml, the verifier emits one generic sentence that does not name the violated invariant. The same pattern repeats at lines 2130-2154 and 2426-2449.Consider evaluating a list of
(condition, message)pairs and appending each failed message. The check semantics stay the same, and the failure output identifies the broken invariant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify_ci_workflow.py` around lines 2654 - 2693, Replace the aggregated composite checks in the affected verifier sections, including the block around _has_exact_tag_source_function and its counterparts near the other reported ranges, with individual condition/message evaluations. Build condition-message pairs for each invariant, append the corresponding specific message whenever its condition fails, and preserve all existing boolean expressions and validation semantics, including offset-based checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 435-449: Reset double_bracket_open for each shell command record,
matching arithmetic_open’s per-record lifecycle, so multi-line [[ conditions do
not suppress command-start scanning on later logical lines. Preserve
expression_positions handling within the current record while ensuring command
substitutions such as $(...) are still inspected by the indirect-execution
check.
In `@tests/test_verify_ci_workflow.py`:
- Around line 1444-1450: Ensure every workflow marker substitution in the
affected tests—including the sites around the existing replacement and the other
listed cases—asserts that the marker exists before calling replace. Prefer
introducing a small shared helper that checks marker presence with a clear
assertion and performs a single replacement, then update each injection site to
use it.
---
Nitpick comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 2654-2693: Replace the aggregated composite checks in the affected
verifier sections, including the block around _has_exact_tag_source_function and
its counterparts near the other reported ranges, with individual
condition/message evaluations. Build condition-message pairs for each invariant,
append the corresponding specific message whenever its condition fails, and
preserve all existing boolean expressions and validation semantics, including
offset-based checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f1f75f6-e55b-4cfc-8412-89785be46fa2
📒 Files selected for processing (2)
scripts/verify_ci_workflow.pytests/test_verify_ci_workflow.py
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (2)
scripts/verify_ci_workflow.py (2)
1562-1577: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
_gh_api_write_requestsmatches only the bare tokengh.Line 1567 compares the raw token.
_has_gh_release_mutation(Line 1521) instead comparestoken.rsplit("/", 1)[-1]. A path-qualified invocation such as/usr/bin/gh api --method POST ...therefore returns no write requests from this helper.Every current caller runs
_has_indirect_shell_executionon the same shell first, and_has_unapproved_shell_commandrejects path-qualified command heads, so the release path stays fail-closed today. Align the two helpers so the guarantee does not depend on call order.♻️ Proposed alignment
- if token != "gh" or not _shell_token_starts_command(tokens, index): + if token.rsplit("/", 1)[-1] != "gh" or not _shell_token_starts_command(tokens, index): continue🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify_ci_workflow.py` around lines 1562 - 1577, Update _gh_api_write_requests to identify the gh executable by its basename, matching _has_gh_release_mutation’s token.rsplit("/", 1)[-1] behavior, so path-qualified invocations such as /usr/bin/gh are processed. Preserve the existing command-start, api-subcommand, and non-GET/HEAD filtering logic.
154-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_step_scalar_valuesignores the first-line- key:form.
_step_uses(Line 146) and_step_direct_keys(Line 178) both accept a key on the step's first line._step_scalar_valuesmatches only the 8-space continuation form. Therefore_step_declares_run,_step_scalar, and_step_has_exact_bash_runall return "absent" for a step written as- run: |or- shell: bash.The current call sites stay fail-closed, because the exact
_step_direct_keysandstep_identitiescomparisons reject such a step. The asymmetry is still a trap for any future call site that treats an absentrunas safe. Align the two parsers.♻️ Proposed alignment
def _step_scalar_values(step_text: str, key: str) -> list[str]: """Return every direct scalar value for one step key.""" values: list[str] = [] for match in re.finditer( - rf"^ {re.escape(key)}\s*:\s*(.*?)\s*$", + rf"^(?: -\s+| ){re.escape(key)}\s*:\s*(.*?)\s*$", step_text, re.MULTILINE, ): values.append(_strip_yaml_inline_comment(match.group(1))) return values🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/verify_ci_workflow.py` around lines 154 - 168, Update _step_scalar_values to also match direct scalar keys on a step’s first line using the 6-space “- key:” form, while preserving the existing 8-space continuation matching and comment stripping. Align it with _step_uses and _step_direct_keys so _step_declares_run, _step_scalar, and _step_has_exact_bash_run recognize first-line run and shell declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/verify_ci_workflow.py`:
- Around line 1562-1577: Update _gh_api_write_requests to identify the gh
executable by its basename, matching _has_gh_release_mutation’s
token.rsplit("/", 1)[-1] behavior, so path-qualified invocations such as
/usr/bin/gh are processed. Preserve the existing command-start, api-subcommand,
and non-GET/HEAD filtering logic.
- Around line 154-168: Update _step_scalar_values to also match direct scalar
keys on a step’s first line using the 6-space “- key:” form, while preserving
the existing 8-space continuation matching and comment stripping. Align it with
_step_uses and _step_direct_keys so _step_declares_run, _step_scalar, and
_step_has_exact_bash_run recognize first-line run and shell declarations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a5b3bcba-08a4-413c-8e71-837f05ec9d3c
📒 Files selected for processing (4)
.github/workflows/deploy-docs-stg.yml.github/workflows/release.ymlscripts/verify_ci_workflow.pytests/test_verify_ci_workflow.py
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/deploy-docs-stg.yml
- .github/workflows/release.yml
- tests/test_verify_ci_workflow.py
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Not reviewed (too large): scripts/verify_ci_workflow.py (~3,281 lines), tests/test_verify_ci_workflow.py (~2,798 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_verify_ci_workflow.py (2)
3296-3319: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a near-miss "release not found" case.
The step compares stderr for exact equality with
release not found\n. The current parameters cover an unrelated auth error, a timeout, and malformed metadata. They do not cover a message that containsrelease not foundplus extra text.That near-miss is the most likely regression for an equality check. A future
ghversion can emitrelease not found for tag v1.2.3. The step must still fail closed, and the test namedtest_release_state_preflight_treats_only_exact_not_found_as_absentcurrently does not prove it.Add a fake
ghmode that prints a superset message, then assert the fail-closed outcome.♻️ Proposed additional case
case "$GH_MODE" in success) printf '%s\\n' "$GH_RESPONSE" ;; not-found) printf 'release not found\\n' >&2 exit 1 ;; + not-found-superset) + printf 'release not found for tag %s\\n' "$TAG" >&2 + exit 1 + ;; auth-error)[ ("auth-error", ""), + ("not-found-superset", ""), ("timeout", ""), ("success", "{"),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_verify_ci_workflow.py` around lines 3296 - 3319, Add a parameterized near-miss case to test_release_state_preflight_treats_only_exact_not_found_as_absent (or the visible fail-closed test if that is the current name), using a fake gh mode that emits “release not found” with additional text. Assert the same fail-closed result: nonzero return code, empty output, and no leftovers, while preserving the exact-message absent-release behavior separately.
3228-3237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider an allowlisted environment instead of inheriting
os.environ.
os.environ | {...}passes every ambient variable into the simulated step. The overrides coverGITHUB_OUTPUTandRUNNER_TEMP, so the main collisions are handled. Other inherited variables remain, for exampleGITHUB_REPOSITORYandGH_HOST. If the step ever reads one of them, the assertions on exactgithub_outputcontent become environment dependent.Pass only the variables the step needs, plus
PATH.♻️ Proposed hermetic environment
- env = os.environ | { + env = { "GITHUB_OUTPUT": str(github_output), "GH_MODE": mode, "GH_RESPONSE": response, "GH_TOKEN": "test-token", - "PATH": f"{fake_bin}{os.pathsep}{os.environ.get('PATH', '')}", + "HOME": str(tmp_path), + "PATH": f"{fake_bin}{os.pathsep}{os.environ.get('PATH', '')}", "REPO": "reflex-dev/xy", "RUNNER_TEMP": str(runner_temp), "TAG": "v1.2.3", }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_verify_ci_workflow.py` around lines 3228 - 3237, Update the test environment setup around the env mapping to stop inheriting os.environ wholesale. Construct an allowlisted environment containing only the variables required by the simulated step, the existing test overrides, and PATH, preserving the current values for GITHUB_OUTPUT and RUNNER_TEMP so assertions remain hermetic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_verify_ci_workflow.py`:
- Around line 3296-3319: Add a parameterized near-miss case to
test_release_state_preflight_treats_only_exact_not_found_as_absent (or the
visible fail-closed test if that is the current name), using a fake gh mode that
emits “release not found” with additional text. Assert the same fail-closed
result: nonzero return code, empty output, and no leftovers, while preserving
the exact-message absent-release behavior separately.
- Around line 3228-3237: Update the test environment setup around the env
mapping to stop inheriting os.environ wholesale. Construct an allowlisted
environment containing only the variables required by the simulated step, the
existing test overrides, and PATH, preserving the current values for
GITHUB_OUTPUT and RUNNER_TEMP so assertions remain hermetic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3dab0d3d-2114-495e-8ddc-1729849cc57a
📒 Files selected for processing (4)
.github/workflows/release.ymlscripts/verify_ci_workflow.pyspec/process/production-readiness.mdtests/test_verify_ci_workflow.py
🚧 Files skipped from review as they are similar to previous changes (3)
- spec/process/production-readiness.md
- .github/workflows/release.yml
- scripts/verify_ci_workflow.py
There was a problem hiding this comment.
1 issue found across 4 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="scripts/verify_ci_workflow.py">
<violation number="1">
P2: The “exact” preflight check still accepts comment-only lines because `_step_run()` removes them; a comment after the timeout line’s trailing `\` makes Bash run `gh release view` separately and without the 30-second bound. Comparing comment-preserving text or rejecting comments in this protected step would close the bypass.
(Based on your team's feedback about exact protected step shape.) [FEEDBACK_USED]</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
What changed
github-releasejob after the verified PyPI publish job.workflow_dispatchruns, including dry runs, unable to create a release.Why
Docs promotion requires both the PyPI version and a GitHub Release, but the core release workflow previously created only the PyPI publication. No automated owner completed the second half of that contract.
Impact
A real
v*tag now produces the GitHub Release only after publication succeeds, while manual validation remains non-publishing.Validation
.venv/bin/pytest -q tests/test_verify_ci_workflow.py— 59 passed.venv/bin/python scripts/verify_ci_workflow.pyFixes #398
Summary by CodeRabbit
New Features
Documentation
Tests