[build] create a github release draft before running publish#17422
Open
titusfortner wants to merge 5 commits intotrunkfrom
Open
[build] create a github release draft before running publish#17422titusfortner wants to merge 5 commits intotrunkfrom
titusfortner wants to merge 5 commits intotrunkfrom
Conversation
Contributor
Review Summary by QodoSplit GitHub release into draft and publish stages for better recovery
WalkthroughsDescription• Split GitHub release workflow into draft and publish stages • Create draft release before publish job to improve failure recovery • Move nightly release cleanup to draft stage for better separation • Update job dependencies and conditions for new workflow structure File Changes1. .github/workflows/release.yml
|
Contributor
Code Review by Qodo
1. Non-idempotent tag creation
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adjusts the Selenium release GitHub Actions workflow so a GitHub Release draft is created ahead of publishing, making recovery easier if publishing fails after tag/release creation.
Changes:
- Adds a
github-release-draftjob to create (or update) a draft release beforepublishruns. - Replaces the prior
github-releasejob withgithub-release-publish, which uploads artifacts and then publishes the draft (sets--draft=false). - Updates downstream job dependencies to use
github-release-publish.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
titusfortner
force-pushed
the
split_gh_release
branch
from
5c40a80 to
34fb37e
Compare
titusfortner
force-pushed
the
split_gh_release
branch
from
34fb37e to
94f21c7
Compare
…for patch releases
…ion tag generation
titusfortner
force-pushed
the
split_gh_release
branch
from
7a6df15 to
4724ab8
Compare
Contributor
|
Persistent review updated to latest commit 4724ab8 |
Comment on lines
+155
to
+162
| github-release-publish: | ||
| name: GitHub Release Publish | ||
| needs: [parse-tag, publish, github-release-draft] | ||
| if: >- | ||
| needs.parse-tag.outputs.language == 'all' || | ||
| always() && !failure() && !cancelled() && | ||
| (needs.parse-tag.outputs.language == 'all' || | ||
| needs.parse-tag.outputs.language == 'java' || | ||
| needs.parse-tag.outputs.language == 'dotnet' | ||
| needs.parse-tag.outputs.language == 'dotnet') |
Comment on lines
200
to
+203
| run: | | ||
| BASE_VERSION="${VERSION%.*}.0" | ||
| gh release upload "selenium-$BASE_VERSION" build/dist/*.* --clobber | ||
| gh release edit "selenium-$BASE_VERSION" --draft=false |
Comment on lines
+70
to
+78
| - name: Create language-specific tag | ||
| env: | ||
| GH_TOKEN: ${{ secrets.SELENIUM_CI_TOKEN }} | ||
| TAG: ${{ needs.parse-tag.outputs.tag }} | ||
| SHA: ${{ github.event.pull_request.merge_commit_sha || github.sha }} | ||
| run: | | ||
| gh api -X POST /repos/${{ github.repository }}/git/refs \ | ||
| -f ref="refs/tags/${TAG}" \ | ||
| -f sha="${SHA}" |
Contributor
There was a problem hiding this comment.
1. Non-idempotent tag creation 🐞 Bug ☼ Reliability
create-language-tag unconditionally POSTs a new refs/tags/${TAG} without checking if it already
exists, so workflow_dispatch runs against an existing tag (or reruns after partial progress) can
fail before docs/other steps that require the tag.
Agent Prompt
### Issue description
`create-language-tag` always attempts to create `refs/tags/${TAG}` via `gh api -X POST .../git/refs`. This is not idempotent: if the tag already exists (manual pre-tagging, workflow_dispatch reruns, or reruns after partial success), the step fails and blocks downstream jobs.
### Issue Context
- Patch releases (`language != 'all'`) require a language-suffixed tag.
- Downstream docs uses this tag as the checkout ref.
### Fix Focus Areas
- .github/workflows/release.yml[62-78]
### Suggested fix approach
- Add a pre-check for the tag ref:
- `gh api /repos/${{ github.repository }}/git/ref/tags/${TAG}`
- If it exists:
- If it already points to `${SHA}`, treat as success and exit 0.
- Otherwise fail with a clear error (or optionally delete/recreate if that matches your release policy).
- If it does not exist, create it (current POST).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+207
to
+213
| needs: [parse-tag, publish, publish-python, github-release-publish, create-language-tag] | ||
| if: >- | ||
| always() && !cancelled() && | ||
| needs.publish.result == 'success' && | ||
| (needs.publish-python.result == 'success' || needs.publish-python.result == 'skipped') && | ||
| needs.github-release.result != 'failure' | ||
| needs.github-release-publish.result != 'failure' && | ||
| needs.create-language-tag.result != 'failure' |
Contributor
There was a problem hiding this comment.
2. Draft failure bypasses docs gate 🐞 Bug ≡ Correctness
For full releases, if github-release-draft fails, github-release-publish is skipped due to the !failure() guard; docs treats "skipped" as acceptable (!= 'failure') and can proceed even though the release tag may not exist, causing checkout failures in the docs workflow.
Agent Prompt
### Issue description
On full releases (`language == 'all'`), a failure in `github-release-draft` can still allow `docs` to run because `github-release-publish` becomes "skipped" (due to `!failure()`), and `docs` only checks `needs.github-release-publish.result != 'failure'`. This can lead to docs trying to checkout a tag that was never created.
### Issue Context
- Tag creation for full releases happens in `github-release-draft`.
- Docs generation checks out `inputs.tag`.
### Fix Focus Areas
- .github/workflows/release.yml[155-163]
- .github/workflows/release.yml[205-214]
### Suggested fix approach
Option A (recommended): explicitly require draft success for full releases
- Add `github-release-draft` to `docs.needs`.
- Update the `docs.if` condition to include something like:
- `(needs.parse-tag.outputs.language != 'all' || needs.github-release-draft.result == 'success')`
Option B: make `github-release-publish` fail (not skip) when draft fails
- Adjust `github-release-publish.if` to avoid converting a draft failure into a skipped job (e.g., remove `!failure()` and instead explicitly gate on `needs.publish.result == 'success'`), so downstream checks can reliably block on failure.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💥 What does this PR do?
The github release action is what we're using to create the tag, so if anything in the publish job breaks, the github release isn't prepped and it's harder to recover.
🤖 AI assistance