chore(ci): make release asset upload idempotent#10430
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe inline ChangesGitHub Release Asset Management
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
jdx
force-pushed
the
codex/idempotent-release-assets
branch
from
2702b59 to
2308ee6
Compare
Greptile SummaryThis PR replaces the single
Confidence Score: 4/5CI/release automation only; the logic is sound and the new guards (draft check, stale-asset pruning, post-upload verification) make the process more robust than before. The overall approach is correct and the three-phase lifecycle (create/edit → prune → upload → verify) addresses the root cause cleanly. The two findings are a portability nit (find -printf GNU-only) and a minor redundant API call; neither affects correctness in CI. The script is safe to merge. scripts/github-release-assets.sh — the find -printf portability issue is worth a quick look before someone tries to run the script locally on macOS. Important Files Changed
Reviews (1): Last reviewed commit: "chore(ci): make release asset upload ide..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| actual_assets="$(mktemp)" | ||
| trap 'rm -f "$expected_assets" "$actual_assets"' EXIT | ||
|
|
||
| find "$release_dir" -maxdepth 1 -type f -printf "%f\n" | sort >"$expected_assets" |
There was a problem hiding this comment.
find -printf "%f " is a GNU extension that isn't available on macOS/BSD find. If this script is ever run locally on a macOS developer machine (e.g., to debug a release), it will silently output nothing, causing the "release directory has no assets" guard to fire or producing an incorrect empty expected-assets list. A portable equivalent keeps the same semantics.
| find "$release_dir" -maxdepth 1 -type f -printf "%f\n" | sort >"$expected_assets" | |
| find "$release_dir" -maxdepth 1 -type f -exec basename {} \; | sort >"$expected_assets" |
| --draft | ||
| fi | ||
|
|
||
| release_id="$(gh api "repos/$repo/releases/tags/$version" --jq ".id")" |
There was a problem hiding this comment.
if branch), its JSON is already in $release_json, so release_id can be read from the cached file instead of making a third identical API call. The extra round-trip is harmless but unnecessary.
| release_id="$(gh api "repos/$repo/releases/tags/$version" --jq ".id")" | |
| if [[ -s "$release_json" ]]; then | |
| release_id="$(jq -r ".id" <"$release_json")" | |
| else | |
| release_id="$(gh api "repos/$repo/releases/tags/$version" --jq ".id")" | |
| fi |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.7 x -- echo |
18.2 ± 0.7 | 16.7 | 21.4 | 1.00 |
mise x -- echo |
19.0 ± 1.3 | 17.3 | 40.6 | 1.04 ± 0.08 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.7 env |
18.3 ± 0.8 | 16.6 | 22.4 | 1.00 |
mise env |
18.5 ± 0.8 | 17.1 | 22.1 | 1.01 ± 0.06 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.7 hook-env |
18.7 ± 0.8 | 17.0 | 23.2 | 1.00 |
mise hook-env |
19.4 ± 0.7 | 17.7 | 22.1 | 1.03 ± 0.06 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.6.7 ls |
15.7 ± 0.6 | 14.3 | 18.9 | 1.00 |
mise ls |
16.3 ± 0.7 | 14.9 | 20.1 | 1.04 ± 0.06 |
xtasks/test/perf
| Command | mise-2026.6.7 | mise | Variance |
|---|---|---|---|
| install (cached) | 133ms | 135ms | -1% |
| ls (cached) | 59ms | 59ms | +0% |
| bin-paths (cached) | 65ms | 64ms | +1% |
| task-ls (cached) | 124ms | 124ms | +0% |
1 participant
Summary
--clobberso reruns can repair stale or partial assetsRoot cause
A canceled release run can leave partial GitHub release assets behind, including
starterassets. A later rerun may build and publish CDN assets successfully while GitHub Releases remains incomplete.Validation
bash -n scripts/github-release-assets.shshellcheck scripts/github-release-assets.shruby -e 'require "yaml"; YAML.load_file(".github/workflows/release.yml"); puts "yaml ok"'\n-git diff --cached --check\n\nThis PR was generated by an AI coding assistant.Note
Medium Risk
Changes the release publish path for downloadable GitHub assets, but scope is CI-only with explicit verification and draft-only repair before the existing publish step.
Overview
Release workflow no longer creates the GitHub draft release and uploads all
releases/$VERSIONfiles in onegh release createcall. It now runsscripts/github-release-assets.sh, which treats asset upload as a repairable step.The script creates or updates a draft release (notes/title only), removes unexpected or non-uploaded assets on GitHub, uploads each local file with
--clobber, then fails the job unless the remote asset names exactly matchreleases/$version. It also refuses to run if the tag’s release is already published, since those assets are immutable.This addresses canceled runs that left partial GitHub release assets while later steps (e.g. CDN) could still succeed.
Reviewed by Cursor Bugbot for commit 2308ee6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Chores