ci(release-go): chain off Release Go FFI instead of polling the mirror#2463
Conversation
release-go and release-go-ffi both fired on the shared moq-ffi-v* tag and raced: release-go polled `git ls-remote` for the moq-go-ffi mirror tag with a ~5 min budget (30x10s), but release-go-ffi builds a 5-target matrix for ~25 min before it publishes that tag, so the wait always timed out and the wrapper never republished against a new ffi. Replace the poll with a workflow_run trigger on "Release Go FFI" completion: the upstream run only concludes success after publish-ffi.sh pushes the mirror tag, so by the time this workflow starts the tag is guaranteed present and there is nothing to wait for. The ffi version comes from the upstream tag ref (workflow_run.head_branch), gated on conclusion == success and a moq-ffi-v* head so PR/failed upstream runs are ignored. The main-push path (wrapper API / VERSION changes) keeps a single existence check instead of a loop: in steady state the current crate's ffi mirror tag already exists; if it doesn't, a release is mid-flight and the job skips cleanly (green no-op) since the workflow_run chain will republish once the ffi lands. Serialize all runs via a constant concurrency group so two runs can't race to compute the same patch tag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Go wrapper release workflow now triggers on pushes to 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-go.yml:
- Around line 25-32: Restrict the workflow_run-triggered release condition to
completed upstream push events by requiring github.event.workflow_run.event ==
'push' before evaluating head_branch or invoking the publish job. Update the
corresponding conditions in both referenced workflow sections while preserving
the existing branch-name checks for trusted push runs.
- Around line 113-119: Update the “Package wrapper” workflow step to build the
release artifact through the repository’s Nix output using nix build, while
preserving the existing gate condition and LINE/FFI_VERSION inputs. Only retain
the runner-installed Go packaging path if this package is demonstrably
unsupported by Nix.
- Around line 106-110: Update the mirror lookup in the release workflow to
distinguish git ls-remote failures from successful empty results: enable
pipeline failure propagation or explicitly capture and validate the command
status, exit non-zero when the lookup fails, and set skip=true only when it
succeeds without finding the v${FFI_VERSION} tag.
🪄 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: 1c37d444-bae7-4d27-b65f-a34e805293ef
📒 Files selected for processing (1)
.github/workflows/release-go.yml
| - name: Package wrapper | ||
| if: steps.gate.outputs.skip != 'true' | ||
| env: | ||
| LINE: ${{ steps.line.outputs.line }} | ||
| FFI_VERSION: ${{ steps.ffi.outputs.version }} | ||
| run: | | ||
| ./go/scripts/package-wrapper.sh --line "$LINE" --ffi-version "$FFI_VERSION" --output release-out |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Build the release artifact through Nix.
This packages the release artifact directly with the runner-installed Go toolchain. Use the repository’s Nix build output (nix build) for this artifact, unless Nix cannot support this package path. As per coding guidelines, “Prefer building release artifacts inside Nix with nix build rather than relying on runner-installed toolchains, except where Nix does not fit.”
🤖 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-go.yml around lines 113 - 119, Update the “Package
wrapper” workflow step to build the release artifact through the repository’s
Nix output using nix build, while preserving the existing gate condition and
LINE/FFI_VERSION inputs. Only retain the runner-installed Go packaging path if
this package is demonstrably unsupported by Nix.
Source: Coding guidelines
Address CodeRabbit review on #2463: - Require github.event.workflow_run.event == 'push' before trusting workflow_run.head_branch. Release Go FFI also runs on pull_request, so a fork PR from a branch named `moq-ffi-v*` could otherwise satisfy the head_branch check and reach the publish job's secrets in the base repo's privileged workflow_run context. - Fail loudly when the moq-go-ffi mirror lookup errors instead of treating a failed `git ls-remote` as "tag absent" and silently deferring. Capture the command status explicitly and skip only on a successful empty result; a wrapper-only push has no later trigger to retry, so a silent skip would drop the release. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Problem
release-goandrelease-go-ffiboth trigger on the sharedmoq-ffi-v*tag and race.release-go's "Wait for moq-go-ffi tag" step pollsgit ls-remotefor themoq-go-ffimirror tag with a ~5 min budget (30×10s), butrelease-go-ffibuilds a 5-target native matrix for ~25 min before it publishes that tag. So the wait always times out and the Go wrapper never republishes against a new ffi.This is why
moq-gois stuck atv0.2.32on the mirror even thoughgo/wrapper/VERSIONis0.5. (The current wedge is compounded by themoq-ffi-v0.3.0tag predating the uniffi-bindgen-go fix in #2452, sorelease-go-ffifails outright on it — that resolves on the nextmoq-ffi-v*tag, which the pending release PR #2453 cuts as0.3.1.)Fix
Replace the poll with a
workflow_runchain on Release Go FFI completion. The upstream run only concludessuccessafterpublish-ffi.shpushes the mirror tag, so by the time this workflow starts the tag is guaranteed present — nothing to poll for.package-wrapper.shalready usesGOPROXY=direct, sogo mod tidyresolves the freshly-pushed tag with no proxy lag.workflow_runpath (a new ffi published): ffi version comes from the upstream tag ref (workflow_run.head_branch), gated onconclusion == 'success'and amoq-ffi-v*head so PR/failed upstream runs are ignored.VERSIONchanges): keeps a single existence check instead of a loop. In steady state the current crate's ffi mirror tag already exists; if it doesn't, a release is mid-flight and the job skips cleanly (green no-op) because theworkflow_runchain republishes once the ffi lands.concurrencygroup so two runs can't race to compute the same patch tag.Verification
The fix can't be fully exercised until the next
moq-ffi-v*tag (via #2453). YAML validated (yaml.safe_load); theworkflow_runguard and version parsing follow standard GitHub expression semantics (head_branchcarries the tag name for tag-triggered upstream runs).🤖 Generated with Claude Code
(written by Opus 4.8)