fix: use bun canary to resolve oo file upload crash#243
Conversation
Stable Bun (v1.3.14, built with Zig) segfaults inside Blob.detach during FetchTasklet teardown when `oo file upload` runs, which leaves the published `oo` binaries unable to complete an upload at all. The crash is already addressed upstream in the canary channel, so build with canary in CI to ship a working `oo file upload` until the fix lands in a stable Bun release. Fixed: #242 Signed-off-by: Kevin Cui <bh@bugs.cc>
Summary by CodeRabbit
WalkthroughThis PR updates multiple GitHub Actions workflows to pass 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish.yaml (1)
99-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd Bun revision to the Bun install cache key for canary determinism
The cache key currently only varies by OS/arch and
bun.lock, so canary installs may reuse an incompatible.bun-install-cache. Include Bun identity in the key—add anidto the existing Setup Bun step and use${{ steps.<id>.outputs.bun-revision }}(preferred) or${{ steps.<id>.outputs.bun-version }}in thekey.🤖 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/publish.yaml around lines 99 - 105, Update the GitHub Actions cache key to include the Bun revision from the Setup Bun step: add an id (e.g., id: setup-bun) to the existing Setup Bun step so it exposes outputs.bun-revision (or fallback outputs.bun-version), then modify the Cache Bun install dependencies step to include that output in the key (use ${{ steps.setup-bun.outputs.bun-revision }} or ${{ steps.setup-bun.outputs.bun-version }}), ensuring the cache key now varies by OS/arch, bun.lock, and the Bun revision to avoid canary mismatches.
🤖 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.
Outside diff comments:
In @.github/workflows/publish.yaml:
- Around line 99-105: Update the GitHub Actions cache key to include the Bun
revision from the Setup Bun step: add an id (e.g., id: setup-bun) to the
existing Setup Bun step so it exposes outputs.bun-revision (or fallback
outputs.bun-version), then modify the Cache Bun install dependencies step to
include that output in the key (use ${{ steps.setup-bun.outputs.bun-revision }}
or ${{ steps.setup-bun.outputs.bun-version }}), ensuring the cache key now
varies by OS/arch, bun.lock, and the Bun revision to avoid canary mismatches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ab82aab4-abcf-4cd4-8404-545e80694290
📒 Files selected for processing (4)
.github/workflows/pr-check.yaml.github/workflows/publish.yaml.github/workflows/reject-large-external-pr.yaml.github/workflows/upload-release-binaries-to-oss.yaml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
contrib/ci/npm-packages.test.ts (1)
334-347: ⚡ Quick winExtract repeated compile metadata setup into a local test factory.
The new test adds another inline metadata fixture that’s already duplicated in this file. Please move it to a helper at the end of the file and reuse it.
♻️ Proposed refactor
@@ - { - buildTimestamp: 1_742_867_323_456, - gitCommit: "1234567890abcdef", - version: "1.2.3", - }, + createCompileBuildMetadataFixture(), @@ - { - buildTimestamp: 1_742_867_323_456, - gitCommit: "1234567890abcdef", - version: "1.2.3", - }, + createCompileBuildMetadataFixture(), @@ function createCompileSpawnResult(options: { @@ } + +function createCompileBuildMetadataFixture() { + return { + buildTimestamp: 1_742_867_323_456, + gitCommit: "1234567890abcdef", + version: "1.2.3", + }; +}As per coding guidelines, "In test files, extract repeated setup (mock, stub, or setup objects) into a local factory function at the bottom of the file."
🤖 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 `@contrib/ci/npm-packages.test.ts` around lines 334 - 347, This test duplicates the inline compile metadata object used elsewhere—extract that repeated metadata into a local factory helper (e.g., a function like makeCompileMetadata or compileMetadataFactory) placed at the bottom of the test file and update this test to call that helper instead of inlining the object; locate usages around buildCompileCommandArgs and getRequiredTarget to replace the inline literal with the helper return value and ensure other tests in the file also import the same helper to avoid duplication.
🤖 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.
Nitpick comments:
In `@contrib/ci/npm-packages.test.ts`:
- Around line 334-347: This test duplicates the inline compile metadata object
used elsewhere—extract that repeated metadata into a local factory helper (e.g.,
a function like makeCompileMetadata or compileMetadataFactory) placed at the
bottom of the test file and update this test to call that helper instead of
inlining the object; locate usages around buildCompileCommandArgs and
getRequiredTarget to replace the inline literal with the helper return value and
ensure other tests in the file also import the same helper to avoid duplication.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8838e736-c022-42bc-a8f4-d69586f6623c
📒 Files selected for processing (2)
contrib/ci/npm-packages.test.tscontrib/ci/npm-packages.ts
1 participant
Stable Bun (v1.3.14, built with Zig) segfaults inside Blob.detach during FetchTasklet teardown when
oo file uploadruns, which leaves the publishedoobinaries unable to complete an upload at all. The crash is already addressed upstream in the canary channel, so build with canary in CI to ship a workingoo file uploaduntil the fix lands in a stable Bun release.Fixed: #242