Repository navigation
fix(ci): commit the Go release's guests 0644 in the build job too - #1175
Conversation
|
|
Warning Review limit reachedOnly developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing. Next included review available in 51 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to This fixes the release build committing the Go guests as executable, which stopped the first release at the tree check. The change is small and tested, and no merge-blocking risk is apparent. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: committing the Go release guests with mode 0644 in the build job. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
cipherstash-agent
left a comment
cipherstash-agent
left a comment
There was a problem hiding this comment.
Review attestation: automated /code-review medium plus /cross-check-review-findings on head ff47eca.
Findings: none. No critical or blocking issues.
Checked:
.github/workflows/release-golang.yml:212:chmod 0644 "${guests[@]}"runs beforegit add -f, so the build job commits the guests as100644. That matches whatpublishwrites from the artifact, so the two trees agree.scripts/__tests__/go-release-artifact.test.mjs: the stepid: stageand thefetched=$(mktemp -d)cut marker both exist in the workflow (line 200 and line 217). The test makes the guests0755and compares the reported tree withexpectedTree(). It guards the regression, and the PR states it fails without the fix.- Scope is limited to the staged guests. No other file modes or release logic change.
I did not run the test suite here. I reviewed the diff and the surrounding workflow only.
Generated by Claude Code
tobyhede
left a comment
tobyhede
left a comment
There was a problem hiding this comment.
Review attestation: files reviewed: .github/workflows/release-golang.yml, scripts/__tests__/go-release-artifact.test.mjs. Models: Sonnet 5.5 (claude-sonnet-5-5), single pass, no verifier.
Generated by Claude Code
The first release, v0.1.0-alpha.1, refused itself: publish's tree check found 76409f6 where build had tested f274561. Cargo writes the .wasm guests executable, and the build job committed them as they were (100755), while publish writes them 0644 from the artifact (100644). The bytes matched; only the mode differed. Nothing was pushed. The build job now sets the guests to 0644 before it stages them, so both jobs build the same tree. A test runs the real staging step on executable guests and checks the tree it reports is the one publish builds; it fails without the chmod.
… real publish step Review follow-up. chmod made the two jobs agree only while both kept writing the files 0644 by their own route. Both jobs now stage the guests with git add --chmod=-x, so the committed mode is set in git whatever either job left on disk. The test no longer compares the build job's tree with a tree it builds itself at 100644. It runs the build job's staging on executable guests, then runs the real publish step on an executable artifact against the tree the build job reported, and expects publish to accept it.
ff47eca to
8311bcd
Compare
4 participants
Summary
The first Go module release,
v0.1.0-alpha.1(run 38258958016), stopped itself before pushing anything. The publish job only pushes a tag when the release commit it builds has exactly the git tree the build job tested. Here the trees differed only in file mode, so it refused. With this fix both jobs build the same tree, and the release can be dispatched again. No tag, release or proxy entry exists for the version, so0.1.0-alpha.1is still free.Changes
.github/workflows/release-golang.yml: the build job's "Stage the release commit" step runschmod 0644on the guests beforegit add. Cargo writes the.wasmguests executable, so the build job committed them as100755. The publish job writes them0644from the artifact, so it committed100644. The bytes always matched (sha256sum -cpassed in publish).scripts/__tests__/go-release-artifact.test.mjs: a new test runs the real staging step, up to the tree it reports, on executable guests. It checks that the reported tree is the one publish builds. The existing tests checked only publish's side, against a tree they built at100644themselves, so they could not see this.Verification
expected '721ff3a…' to be 'f872a54…').pnpm run test:scripts: 75 files pass. The othergo-release-artifactcases skip on macOS (nomapfilein bash 3.2), as before.actionlintpasses on the workflow, and Biome is clean.Related
Refs #1165, from which the alpha is cut.
https://claude.ai/code/session_01CA4pFfaN4DPNSTLnJVNxsu
Summary by CodeRabbit
Bug Fixes
Tests