Publish releases with the runner's gh CLI instead of a third-party action - #427
Conversation
The release job is the most sensitive path in the repository: it holds the Developer ID signing identity, the notarization credentials and a token with contents: write. Until now it also handed all of that to a third-party action to do something the runner can already do. zizmor flags it as superfluous and CodeRabbit raised it on #413. `gh release create` covers every input the action was given, and the resulting release is identical in shape: --generate-notes sets both the body and the title, and GitHub's generate-notes API returns the tag as the name, which is what the action produced for 3.0.0rc1 and 3.0.0. Neither sends make_latest, so GitHub's default still applies. Neither drafts. Two things are deliberately stricter than what they replace. --verify-tag stops gh from inventing a tag off the default branch if the one it is handed does not exist; the tag is what triggered the run, so that could only mean something had gone wrong. And a glob that matches nothing now fails the step, where the action would happily have published a release with no binary attached to it. Everything reaches the script through the environment rather than being interpolated into it, and GH_REPO is set because this job has no checkout for gh to infer the repository from. Refs #414
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe release workflow replaces ChangesRelease publishing
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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:
- Line 172: Update the release creation flow around gh release create to query
whether $TAG already exists before creating it. Treat only a not-found result as
absent; propagate authentication and network failures, and for existing releases
use gh release upload with --clobber only when replacing matching assets,
followed by gh release edit as needed.
🪄 Autofix
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: d7b58642-5aa5-4b38-b839-aa2adc6749d9
📒 Files selected for processing (1)
.github/workflows/release.yml
Raised by CodeRabbit on #427. The old action would have replaced the assets of an already-published release; gh refuses. On a path that publishes signed, notarized binaries that is the behaviour we want, so it is worth writing down before someone reads the difference as a regression and adds a fallback.
The 3.0.1rc1 tag failed to publish: `no matches found for application/xchtmlreport-*`. `actions/download-artifact` with no `name` created a directory per artifact under v4, which is where the `application/` prefix came from, and 3.0.0 shipped that way. Under v8 it extracts straight into the workspace instead, so the archive has been landing one directory up since #404 bumped the version. Both versions log "An extra directory with the artifact name will be created"; only v4 does it. Nothing caught this at the time because the release action it was feeding defaults `fail_on_unmatched_files` to false. The next release would have published a signed, notarized build with no binary attached to it and gone green. It only surfaced now because the gh CLI step that replaced it in #427 fails on an unmatched pattern. Naming the artifact and its destination pins the layout instead of inheriting whatever the current major version does with it.
Verified — the release job ran for realReporting back as promised. It took two attempts, and the first one is the more interesting half. Attempt 1 (
|
| 3.0.0rc1 (action) | 3.0.1rc1 (gh) | |
|---|---|---|
| name | 3.0.0rc1 |
3.0.1rc1 ✅ |
| notes | generated, ## What's Changed |
generated, ## What's Changed ✅ |
| prerelease | true |
true ✅ |
| draft | false |
false ✅ |
| target_commitish | main |
main ✅ |
| asset | xchtmlreport-3.0.0rc1.zip |
xchtmlreport-3.0.1rc1.zip, state: uploaded ✅ |
3.0.0 also kept the Latest badge and the rc did not steal it, so the untouched make_latest default still behaves.
The published artifact itself
Downloaded the asset from the release and checked it on macOS, rather than trusting that the notarize step went green:
Architectures in the fat file: x86_64 arm64
--version 3.0.1rc1
codesign -vvv --strict valid on disk / satisfies its Designated Requirement
Authority Developer ID Application: Tyler Vick (352UZEKYPP)
Developer ID Certification Authority / Apple Root CA
spctl -a -vvv -t install accepted
source=Notarized Developer ID
Universal, correctly stamped, Developer ID-signed and notarized — the signing and notarization path is unaffected by the swap, as intended.
Not covered
The full-release branch (a non-rc tag) still hasn't run: --prerelease is omitted rather than passed, and bump_version runs instead of skipping. Those differ from what was exercised only in the boolean, but they have not literally executed. The next real release is the first time that path runs.
|
Tip For best results, initiate chat on the files or code changes.
The The first attempt found a pre-existing artifact-path defect. The strict unmatched-file failure prevented a green release with no binary. This validates the decision to fail when the asset glob has no matches. The remaining untested case is correctly limited to the non- 🐇 ✅ ✏️ Learnings added
You are interacting with an AI system. |
Closes #414.
Removes
softprops/action-gh-releasefrom the release job and publishes with the runner's preinstalledghCLI instead. That job holds the Developer ID signing identity, the notarization credentials and acontents: writetoken — it is the last place in the repository that should be reaching for a third-party action to do something the runner already does.zizmorreports it assuperfluous-actions; CodeRabbit raised the same thing on #413.Matching what the action actually did
I checked each input against the action's source at the pinned SHA and against the two releases this pipeline has produced, rather than against the sketch in the issue.
tag_namedefaults togithub.refgithub.ref_name, via$TAGfiles: application/xchtmlreport-*generate_release_notes: true--generate-notes--generate-notesmakes GitHub generate it, and the API returns the tagneeds.build.outputs.prerelease == 'true'--prereleaseappended when$PRERELEASEistruefalsemake_latestOn the title:
ghwithout--notes-start-tagsendsgenerate_release_notes: trueand noname, so GitHub generates both. I called the generate-notes API against this repo to confirm what it returns for a name —3.0.0rc2for a tag of that name, i.e. the tag itself, which is exactly what 3.0.0rc1 and 3.0.0 ended up with.On the assets: the
application/prefix is real, not incidental —download-artifactwith nonamecreates a directory per artifact. The 3.0.0 run log showsStarting download of artifact to: /home/runner/work/XCTestHTMLReport/XCTestHTMLReport/application, and the glob resolves from the workspace.Two deliberate differences
Both make the step stricter than what it replaces:
--verify-tag. Without it,gh release createwill invent the tag off the default branch when the one it is handed does not exist. A tag push is what triggers this job, so a missing tag could only mean something has gone badly wrong; better to stop.fail_on_unmatched_filesdefaults tofalse, so a build that produced no archive would have published a release with no binary attached to it and reported success.Also:
GH_REPOis set because this job deliberately has no checkout, soghhas no git remote to infer the repository from. Everything reaches the script through the environment rather than being interpolated into it.What has and has not been exercised
Not exercised: the release job has not run. It is guarded on
github.event_name == 'push', so theworkflow_dispatchdry run skips it entirely — a green run on this PR says nothing at all about this change. That is precisely why #413 deferred it. Do not read CI on this PR as verification.What I did check, all of it short of running the job:
actionlinton the workflow — clean apart from a pre-existing SC2129 on the dry-run summary step, which is untouched and present onmaintoo.shellcheckon therunblock in isolation — clean.zizmor --persona=pedantic, before and after:superfluous-actionsdrops from 2 findings to 1 (the remaining one iscreate-pull-requestinbump_version, out of scope here). Every other finding count is unchanged — in particular no newtemplate-injection, because nothing is interpolated into the script.The real verification is a
3.0.1rc1tag, cut immediately after this merges.rctags are correctly treated as prereleases since #396, do not reach Homebrew, and skipbump_version, so this is the safe rehearsal — and it exercises the whole path for real: build, sign, notarize, publish. I will report the outcome here.Until that tag has been cut and the release job has actually gone green, this is unverified.
Summary by CodeRabbit