Repository navigation
Fix bin/ci's skipped signoff and native builds racing for Homebrew - #12
Merged
Merged
Conversation
bin/ci decided whether gh-signoff is installed with `gh extension list | grep -q gh-signoff`. grep -q exits at its first match, gh then dies writing the rest of its list (SIGPIPE, 141), and pipefail reports the whole pipeline as failed. So a green full run usually printed "gh signoff not installed", skipped the signoff, and still exited 0: 4 of 5 checks failed here with the extension installed. Plain grep to /dev/null reads to the end; 10 of 10 pass. test/build's multiarch builder check has the same shape. Its input is small enough that it hasn't misfired, but it's the same latent bug, so it gets the same fix.
The definitions run `brew install` while ruby-build sources them, and `test/build native all` sources several at once. Each brew call auto-updates first, so they race for Homebrew's lock, and the losers die within seconds on "Another `brew update` process is already running": 1.9.3, 2.3.3 and 2.3.8 failed that way in one run, before compiling anything. Before launching native builds, install every formula the chosen definitions name, one at a time, then export HOMEBREW_NO_AUTO_UPDATE for the builds. Their own `brew install`s are then no-ops on installed formulae, which run concurrently without contending.
test/build builds its images from the repo root, which had no .dockerignore, so agent worktrees under .claude/ (whole extra checkouts) and .git were sent to the builder on every image build. The images copy only what they build from, so neither belongs in the context.
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. |
There was a problem hiding this comment.
🟢 Approved
Review of the changed hunks and relevant build context identified no unresolved correctness issues.
No open issues 🎉
What changed in this PR
This PR improves CI reliability by fixing extension detection, coordinating Homebrew setup before parallel native builds, and reducing Docker build context size.
Changes:
- Replaces early-exiting
grep -qchecks to avoid false failures underpipefail. - Installs required Homebrew formulae sequentially before native builds and disables auto-update for those builds.
- Excludes
.claudeand.gitfrom Docker build contexts.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
| File | Description |
|---|---|
test/build |
Fixes builder detection and prepares Homebrew dependencies before parallel native builds. |
bin/ci |
Prevents installed signoff extensions from being incorrectly treated as missing. |
.dockerignore |
Excludes agent worktrees and Git metadata from image build contexts. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three harness fixes found while running CI for #11.
bin/ciskipped the signoff on most green runsIt checks for gh-signoff with
gh extension list | grep -q gh-signoff.grep -qexits at the first match, soghdies writing the rest of its list (SIGPIPE, 141), andset -o pipefailreports the pipeline as failed. Sobin/ciprinted "gh signoff not installed", skipped the signoff, and still exited 0. With the extension installed, the check failed 4 of 5 times. It now uses plaingrepto/dev/null, which reads to the end, and passes 10 of 10.test/build's multiarch-builder check has the same shape. Its input is small enough that it hasn't misfired (10 of 10 before and after), but it's the same latent bug, so it gets the same fix.Parallel native builds raced for Homebrew's lock
The definitions run
brew installwhile ruby-build sources them, and everybrewcall auto-updates first. In atest/build native allrun, 1.9.3, 2.3.3 and 2.3.8 died within 5 seconds on "Anotherbrew updateprocess is already running" and "Failed to upgrade Homebrew Portable Ruby!". They passed when run one at a time.Native builds now install every formula the chosen definitions name (today
automakeandbasecamp/dev/openssl@1.0) once, one at a time, before launching. The builds then run withHOMEBREW_NO_AUTO_UPDATE=1, so their ownbrew installs are no-ops on installed formulae.Evidence:
brew install basecamp/dev/openssl@1.0, auto-update forced on: 3 printed the lock error. With auto-update off: none did.test/build native all, withHOMEBREW_AUTO_UPDATE_SECS=1: 6 of 6 pass on this branch.mainalso passed 6 of 6 under the same setting, so that run doesn't prove the fix either..dockerignoretest/buildbuilds its images from the repo root, and there was no.dockerignore. So.claude/(agent worktrees, whole extra checkouts) and.gitwent to the builder on every image build. The images copy only what they build from.Verified
bin/ci --lintis clean, shellcheck included.test/build native allpasses 6 of 6 on macOS 27 arm64.bin/ci, which should now sign off by itself, runs once Carry the Onigmo regexp compiler fixes into 1.9.3 through 2.7.8 #11's CI finishes on this machine.