fix(ci): install protoc for the release build, and allow republishing a tag - #4
Conversation
… a tag `bun run build` regenerates the proto codec through protoc, which is not on the runner. ci.yml never caught it because it calls wasm-pack and build:ts directly and skips `gen`, so the release job was the first thing to run the package's real build. It failed on v0.6.1 at that step. The gate held: the tag and the GitHub release exist, npm is untouched. Recovering from that is the second half of this change. Only the run that creates a tag carries `release_created`, and re-running it replays the workflow file of that same commit, which is the broken one. A dispatch trigger takes an existing tag and puts it through verify and publish with the workflow as it stands.
📝 WalkthroughWalkthroughThe release workflow now supports manual execution from existing tags. Verification validates and outputs the exact commit SHA. Publishing runs only after successful verification and uses that SHA. Both jobs install ChangesRelease workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseEvent
participant VerificationJob
participant PublishingJob
ReleaseEvent->>VerificationJob: provide release tag or dispatched tag ref
VerificationJob->>VerificationJob: check out and record validated commit SHA
VerificationJob->>PublishingJob: emit validated commit SHA
PublishingJob->>PublishingJob: check out SHA and install protoc with wasm-pack
PublishingJob->>PublishingJob: publish after successful verification
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad9796e761
ℹ️ 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".
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Taking the tag as a free-form input let a dispatch point the privileged publish job at any branch or commit, and a mutable ref could advance between the verify checkout and the publish one, shipping code that was never verified. npm's provenance compounded it: it records the event's ref and sha, not the workspace, so a tarball built from a tag while the event pointed at a branch would be attested to the wrong commit. The dispatch now carries no input. Select the tag as the ref instead: the picker only offers refs that exist, `ref_type` rejects anything that is not a tag, and the event then points at the same commit the provenance will name. Publish checks out the sha verify resolved rather than the ref again, so there is no window between the two. Publish also gated on `!cancelled()` rather than `always()`, which ran through cancellation as long as verify had already succeeded.
|
All four applied in Arbitrary ref through the privileged job (codex + cubic P1). A free-form Provenance attested to the wrong commit (codex). This is the one I would not have found: npm reads the event's ref and sha, not the workspace, so dispatching from Both are fixed by dropping the input entirely. The dispatch now takes no parameters — you select the tag in the ref picker. The picker only offers refs that exist,
One consequence worth deciding before merge:
I lean toward 1 — a release that exists on GitHub and not on npm is the kind of thing that confuses whoever looks next. Either way the recovery path works from the next tag onward, since those tags will carry this workflow. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cd4190435
ℹ️ 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".
Publish already consumed the sha verify resolved, but verify still resolved the ref itself. A tag force-updated between the event and that checkout would be built and shipped while npm provenance kept naming the commit the event recorded, so the attestation would describe code that never ran. The event sha is immutable, so use it on the dispatch path. The push path keeps the tag, where the event sha is the main commit rather than the release.
|
One applied, one already answered. Verify resolved the ref itself (
So the limitation stands and is worth stating plainly: this recovery path works from the next tag onward, because those tags will carry this workflow. |
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:
- Around line 112-123: Update the checkout ref expression in the release
workflow to use github.sha for the push path as well as workflow_dispatch,
rather than checking out the release tag. Retain the explicit tag dereference
and validation before publishing, ensuring the tag target equals the commit
recorded by the verified step for verification and provenance.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5199ebe4-e0cf-4b2d-88d0-b7285bc7d2a2
📒 Files selected for processing (1)
.github/workflows/release.yml
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The dispatch path already used the event's sha; the push path still resolved the tag name, so it kept the window the dispatch fix closed and built a ref while npm attested the event's commit. Both paths now check out that sha, which drops the output threading the resolved commit from verify to publish: there is nothing left to thread when neither job resolves anything. release-please tags the commit it merged, and verify asserts that rather than assuming it.
A dispatch runs the workflow file stored in the selected tag, so the recovery path only covers tags created from this commit onwards. That was answered in review and nowhere in the file, which is where the next person looks.
|
The mechanism described is correct, and it is the tradeoff I took knowingly — but the finding is right that the file did not say so. Fixed in Not taking the other suggested shape — free-form tag input, gated on the tag resolving to a verified commit. That is the design the earlier review round flagged twice as P1, and the gating does not fix what made it P1: npm reads the event's ref and sha for provenance, so dispatching from a branch with a tag parameter attests the tarball to the branch commit no matter how strictly the input is validated. A gate can stop the wrong code from being built; it cannot stop the attestation from naming the wrong source. Trading a correct attestation on every future release to rescue one tag is the wrong side of that trade. So |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6db11c9747
ℹ️ 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".
Three holes in the recovery path, all reachable only through it. The concurrency group keyed on `github.ref`, which differs between a release from main and a recovery on a tag, so both could reach npm publish at once and race over `latest`. One group for every run instead. `ref_type == 'tag'` proves the ref is a tag, not that it is a release: anyone able to push a tag could aim the job holding OIDC rights at arbitrary code and skip main. A dispatch now has to name a tag that carries a GitHub release, which is what release-please creates and a hand-pushed tag lacks, and whose version matches the tree. npm refuses `latest` implicitly once a higher version is published, so a recovery attempted after a newer release would have failed at the publish step, past the whole build. It now fails in the first seconds and says why.
The event sha and the release commit diverge when the run for a merged release PR fails or is superseded: a later push creates the release, and the tag names the earlier merge. Asserting equality against the event would then fail after the GitHub release exists, leaving that version unpublishable. The action reports the commit it tagged, so use it where it exists and fall back to the event sha on the dispatch path, where the event already points at the tag.
|
All four applied — three on the recovery path, one shared with the push path. Concurrency keyed per ref ( Any tag was accepted ( Recovery would die at the publish step ( Event sha assumed to be the release commit ( That last one also answers the P2 about retargeting during environment approval: both jobs pin a sha, so moving the tag while publish waits for its gate cannot change what ships. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adf85df7f3
ℹ️ 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".
…otoc npm attests the package to the event's sha whatever the workspace holds. Checking out the commit the action tagged fixed the tree but moved the mismatch onto provenance: a release created by a later push than the merge it names would ship a tarball attested to a commit that was never built. Neither choice is safe on the push path, so refuse there and say what to do: dispatching on the tag publishes the same release with the event pointing at it, which is the one shape where tree and attestation agree. protoc was also the last unpinned tool on this path. Verify and publish install it separately and publish can start much later, behind the environment gate, so a new default between them would test one generated codec and ship another.
|
Two applied, one declined with reasoning. Provenance vs the release commit (P1, protoc unpinned (P2, Concurrency evicting a pending recovery (P2). Accurate about the mechanism — |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a06a60316e
ℹ️ 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".
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| TAG: ${{ github.ref_name }} | ||
| run: | | ||
| if ! gh release view "$TAG" --repo "$GITHUB_REPOSITORY" >/dev/null 2>&1; then |
There was a problem hiding this comment.
Bind recovery releases to protected-main commits
Fresh evidence after the prior release-tag finding is that this new guard only tests whether gh release view "$TAG" succeeds. That command merely views any matching release, and GitHub allows people with repository write access to create releases manually, including drafts; when tags are mutable, the same access needed to dispatch this workflow can therefore tag arbitrary unmerged code with a matching package version, attach a release record, and pass both checks before that code runs in the OIDC-enabled publish job. Bind recovery to a commit known to have passed through protected main/release-please, or enforce immutable protected release tags, rather than treating the existence of a release record as authorization.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
A GitHub release proves nothing about where the code came from: anyone with write access can tag arbitrary unmerged code, set a matching version in that tree, attach a release record by hand, and dispatch. Both checks would pass and the code would run in the job holding npm OIDC rights, which is exactly the protected path being bypassed. Reachability from main is what the release record was standing in for. Check it first, and let the release and version checks narrow from there.
|
Valid, applied in Both reviewers landed on the same hole and they are right: a GitHub release record proves nothing about where the code came from. Anyone with write access can tag arbitrary unmerged code, set a matching version in that tree, attach a release by hand, and dispatch — passing both of my checks and running that code in the job holding npm OIDC rights. That is precisely the protected path the guard was supposed to enforce. Reachability from status=$(gh api "repos/$GITHUB_REPOSITORY/compare/main...$GITHUB_SHA" --jq .status)
case "$status" in
identical | behind) ;;
*) echo "::error::$TAG is at a commit that is not on main ($status)"; exit 1 ;;
esacThe release and version checks stay, narrowing from there: on main, carrying a release, version matching the tree. Verified the semantics against this repository rather than trusting the docs — |
The
v0.6.1release job failed at Build the wasm package withExecutable not found in $PATH: "protoc".bun run buildrunsgen→gen:proto-codec, which shells out toprotoc.ci.ymlnever caught this because it callswasm-pack buildandbun run build:tsdirectly and skipsgenentirely — so the release job was the first thing in CI to run the package's actual build. Installed through thetaiki-e/install-actionalready pinned in the workflow, so no new dependency.The gate held. The tag
v0.6.1and the GitHub release exist; npm is still on0.6.0. Nothing broken was published, which is what the verify/publish split is for.Recovering the tag
Re-running the failed job does not work here: only the run that creates a tag carries
release_created, and a re-run replays the workflow file of that commit — the broken one. So this adds aworkflow_dispatchtrigger taking an existing tag:It skips
release-pleaseand puts the tag throughverifythenpublishwith the workflow as it currently stands. Useful beyond today: a publish can fail for reasons that have nothing to do with the tag — a missing tool, a registry outage — and without this the only way out is a version bump.After merging, dispatch with
v0.6.1to publish it.Note on the gap this exposed
ci.ymland the release build are not running the same thing. CI skipsgen, so codegen breakage only surfaces at release time, which is the worst moment to find it. Worth closing separately — either CI runsbun run build, or the release stops regenerating and trusts the committed output. I did not fold that into a fix for a red release.Summary by cubic
Pins
protocfor the release build, hardens provenance by using immutable checkouts, and adds a safe tag republish flow restricted to release tags onmain. Fixes thev0.6.1build.Bug Fixes
protocviataiki-e/install-action, pinned toprotoc@35.1, sobun run buildcan regenerate the proto codec.main, the tag has a GitHub release, and its version matchespackage.json; fail fast ifnpmalready has a higher version.New Features
workflow_dispatchon a tag ref; select an existing tag to rebuild/publish it.Written for commit be326db. Summary will update on new commits.