Preserve macos-latest while updating release assets - #502
Conversation
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe macOS release workflow now probes for the ChangesmacOS Release Publishing
Estimated code review effort: 4 (Complex) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubActions
participant GitHubAPI
participant ReleaseAssets
GitHubActions->>GitHubAPI: Probe macos-latest
GitHubAPI-->>GitHubActions: Existing release or verified 404
GitHubActions->>ReleaseAssets: Back up and replace DMG/ZIP assets
ReleaseAssets-->>GitHubActions: Upload result
GitHubActions->>GitHubAPI: Update metadata and retarget tag
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08e9f00696
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| exit 1 | ||
| fi | ||
|
|
||
| gh release edit macos-latest \ |
There was a problem hiding this comment.
If gh release edit or the following ref PATCH fails—for example because of a transient API error or tag protection—the replacement upload has already clobbered both live assets, but the rollback block only handles upload failures. Because this workflow uses GitHub Actions' default bash shell, which runs with -e, the step exits here and leaves macos-latest serving the new artifacts with stale or partially updated tag/notes metadata rather than preserving a coherent previous pointer; guard and retry these finalization calls and restore the backups if they cannot complete. See the documented default shell invocation, bash --noprofile --norc -e -o pipefail {0}.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-macos.yml (1)
15-15: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd an exclusive concurrency guard for this release job.
.github/workflows/release-macos.ymlcurrently has noconcurrency:block, so overlappingworkflow_dispatchruns can both probemacos-latest, then upload/clobber assets and update the same tag/release metadata. Usegroup: release-macosor a release-specific key withcancel-in-progress: false.🤖 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 @.github/workflows/release-macos.yml at line 15, Add an exclusive concurrency block to the release workflow near the job configuration, using a stable release-specific group such as release-macos and setting cancel-in-progress to false. Ensure manually triggered runs queue rather than overlap while probing runners or updating release assets and metadata.
🧹 Nitpick comments (2)
.github/workflows/release-macos.yml (2)
137-163: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReplacement/rollback retry loops have no backoff, unlike the build step's retry pattern.
The build step (Lines 90-96) sleeps 60s between attempts; these upload and rollback retry loops fire immediately three times in a row. Given the PR explicitly targets resilience to rate-limiting, back-to-back retries without delay increase the odds of tripping a secondary-rate-limit or hitting the same transient failure again.
💡 Suggested fix
if gh release upload macos-latest "${DMG}" "${ZIP}" \ --repo "${GITHUB_REPOSITORY}" \ --clobber; then upload_succeeded=true break fi echo "macos-latest replacement upload attempt ${attempt} failed" + sleep 15 done(similarly for the rollback loop at Lines 150-158)
🤖 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 @.github/workflows/release-macos.yml around lines 137 - 163, Add a 60-second delay between failed attempts in both the replacement upload loop and the rollback upload loop surrounding gh release upload, matching the existing build retry behavior; keep the delay out of the final attempt and preserve the current success and failure handling.
117-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffConsider extracting this ~70-line inline probe/backup/rollback/retarget script into a versioned shell script.
The line-range change details flag this hunk as high complexity. Moving the logic to e.g.
apps/macos/scripts/publish-latest-release.sh(invoked from the step) would make it independently testable/lintable withshellcheck/batsand easier to read in the YAML.🤖 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 @.github/workflows/release-macos.yml around lines 117 - 187, Extract the inline macos-latest release probe, backup, upload retry, rollback, and retarget logic from the workflow into a versioned shell script such as publish-latest-release.sh. Preserve the existing 404 creation path, failure handling, asset validation, retries, and tag retargeting behavior; update the workflow step to invoke the script with the required repository, SHA, tag, DMG, and ZIP context.
🤖 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-macos.yml:
- Around line 165-177: Wrap the finalization commands—gh release edit and the
git tag PATCH—in the same three-attempt retry and failure-recovery pattern used
by the upload step above. Ensure transient failures are retried, and if all
attempts fail, restore the existing backup release/tag state before exiting so
the release remains consistent.
---
Outside diff comments:
In @.github/workflows/release-macos.yml:
- Line 15: Add an exclusive concurrency block to the release workflow near the
job configuration, using a stable release-specific group such as release-macos
and setting cancel-in-progress to false. Ensure manually triggered runs queue
rather than overlap while probing runners or updating release assets and
metadata.
---
Nitpick comments:
In @.github/workflows/release-macos.yml:
- Around line 137-163: Add a 60-second delay between failed attempts in both the
replacement upload loop and the rollback upload loop surrounding gh release
upload, matching the existing build retry behavior; keep the delay out of the
final attempt and preserve the current success and failure handling.
- Around line 117-187: Extract the inline macos-latest release probe, backup,
upload retry, rollback, and retarget logic from the workflow into a versioned
shell script such as publish-latest-release.sh. Preserve the existing 404
creation path, failure handling, asset validation, retries, and tag retargeting
behavior; update the workflow step to invoke the script with the required
repository, SHA, tag, DMG, and ZIP context.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d46d92a5-8058-4322-bc78-d0db3898d8aa
📒 Files selected for processing (1)
.github/workflows/release-macos.yml
| gh release edit macos-latest \ | ||
| --repo "${GITHUB_REPOSITORY}" \ | ||
| --target "${GITHUB_SHA}" \ | ||
| --title "Burn for Mac (latest)" \ | ||
| --notes "Latest macOS app build — points at ${TAG}." | ||
| # `release edit --target` updates release metadata but does not move | ||
| # an existing Git tag. Retarget the lightweight tag in place only | ||
| # after the replacement assets are live. | ||
| gh api --method PATCH \ | ||
| "repos/${GITHUB_REPOSITORY}/git/refs/tags/macos-latest" \ | ||
| -F "sha=${GITHUB_SHA}" \ | ||
| -F force=true \ | ||
| --silent |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Finalization steps (metadata edit + tag retarget) have no retry or rollback, unlike the upload step right above them.
Under GitHub Actions' default bash invocation (-e is on by default for run: steps), a transient failure in gh release edit (Lines 165-169) or the tag PATCH (Lines 173-177) aborts the job immediately here — after the new assets have already been uploaded and the old backup already consumed. There is no retry and no path back to a consistent state: the release could end up with new assets but a metadata/tag still pointing at the old commit, with no automated remediation (unlike the upload path just above, which has 3 attempts and a backup-restore fallback).
💡 Suggested fix: wrap both calls in the same retry pattern used for uploads
- gh release edit macos-latest \
- --repo "${GITHUB_REPOSITORY}" \
- --target "${GITHUB_SHA}" \
- --title "Burn for Mac (latest)" \
- --notes "Latest macOS app build — points at ${TAG}."
- # `release edit --target` updates release metadata but does not move
- # an existing Git tag. Retarget the lightweight tag in place only
- # after the replacement assets are live.
- gh api --method PATCH \
- "repos/${GITHUB_REPOSITORY}/git/refs/tags/macos-latest" \
- -F "sha=${GITHUB_SHA}" \
- -F force=true \
- --silent
+ finalize_succeeded=false
+ for attempt in 1 2 3; do
+ if gh release edit macos-latest \
+ --repo "${GITHUB_REPOSITORY}" \
+ --target "${GITHUB_SHA}" \
+ --title "Burn for Mac (latest)" \
+ --notes "Latest macOS app build — points at ${TAG}." \
+ && gh api --method PATCH \
+ "repos/${GITHUB_REPOSITORY}/git/refs/tags/macos-latest" \
+ -F "sha=${GITHUB_SHA}" \
+ -F force=true \
+ --silent; then
+ finalize_succeeded=true
+ break
+ fi
+ echo "macos-latest finalization attempt ${attempt} failed" >&2
+ sleep 15
+ done
+ if [ "${finalize_succeeded}" != true ]; then
+ echo "macos-latest metadata/tag update failed after retries; assets were replaced but the release may be inconsistent" >&2
+ exit 1
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| gh release edit macos-latest \ | |
| --repo "${GITHUB_REPOSITORY}" \ | |
| --target "${GITHUB_SHA}" \ | |
| --title "Burn for Mac (latest)" \ | |
| --notes "Latest macOS app build — points at ${TAG}." | |
| # `release edit --target` updates release metadata but does not move | |
| # an existing Git tag. Retarget the lightweight tag in place only | |
| # after the replacement assets are live. | |
| gh api --method PATCH \ | |
| "repos/${GITHUB_REPOSITORY}/git/refs/tags/macos-latest" \ | |
| -F "sha=${GITHUB_SHA}" \ | |
| -F force=true \ | |
| --silent | |
| finalize_succeeded=false | |
| for attempt in 1 2 3; do | |
| if gh release edit macos-latest \ | |
| --repo "${GITHUB_REPOSITORY}" \ | |
| --target "${GITHUB_SHA}" \ | |
| --title "Burn for Mac (latest)" \ | |
| --notes "Latest macOS app build — points at ${TAG}." \ | |
| && gh api --method PATCH \ | |
| "repos/${GITHUB_REPOSITORY}/git/refs/tags/macos-latest" \ | |
| -F "sha=${GITHUB_SHA}" \ | |
| -F force=true \ | |
| --silent; then | |
| finalize_succeeded=true | |
| break | |
| fi | |
| echo "macos-latest finalization attempt ${attempt} failed" >&2 | |
| sleep 15 | |
| done | |
| if [ "${finalize_succeeded}" != true ]; then | |
| echo "macos-latest metadata/tag update failed after retries; assets were replaced but the release may be inconsistent" >&2 | |
| exit 1 | |
| fi |
🤖 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 @.github/workflows/release-macos.yml around lines 165 - 177, Wrap the
finalization commands—gh release edit and the git tag PATCH—in the same
three-attempt retry and failure-recovery pattern used by the upload step above.
Ensure transient failures are retried, and if all attempts fail, restore the
existing backup release/tag state before exiting so the release remains
consistent.
1 participant
Summary
macos-latestwith an explicit existence probe--clobberVerification
08e9f00696d0fd314e855786fc462a19485ff8ec: clean after fixing two findings: stale existing tag target, and--clobberasset loss on failed uploadgit diff --check: clean.github/workflows/release-macos.ymlonlyactionlintreaches the workflow and reports only the pre-existing SC2046 warning at line 70; the same warning reproduces onorigin/maingh api --includeprobe verified that a missing release exposes an HTTP 404 status line separately from stderrBoundary
This PR changes workflow logic only. It was not run. No release, tag, signing, notarization, upload, publish, or distribution action was performed.
macos-v*remains hard-blocked, and ONLY HUMANS CUT RELEASES.PR only: this must remain unmerged for the burn owner to review and merge when the seat revives.