🤖 fix: unpack sharp runtime assets for macOS attach_file#3088
🤖 fix: unpack sharp runtime assets for macOS attach_file#3088
Conversation
Unpack sharp and @img assets for Electron packaging and add a packaged macOS smoke check for attach_file runtime loading. --- _Generated with `mux` • Model: `openai:gpt-5.4` • Thinking: `high` • Cost: `$5.41`_ <!-- mux-attribution: model=openai:gpt-5.4 thinking=high costs=5.41 -->
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a27a453bf4
ℹ️ 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".
Run the macOS packaging step with publish disabled so the attach_file smoke test executes before any release artifacts are uploaded. --- _Generated with `mux` • Model: `openai:gpt-5.4` • Thinking: `high` • Cost: `$5.41`_ <!-- mux-attribution: model=openai:gpt-5.4 thinking=high costs=5.41 -->
|
@codex review Please take another look. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 995e479609
ℹ️ 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".
Prefer the packaged app bundle that matches the host architecture so the macOS attach_file smoke test does not false-fail on x64 runners. --- _Generated with `mux` • Model: `openai:gpt-5.4` • Thinking: `high` • Cost: `$5.41`_ <!-- mux-attribution: model=openai:gpt-5.4 thinking=high costs=5.41 -->
|
@codex review Please take another look. |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Unpack sharp's native runtime assets from the Electron ASAR on macOS and add a packaged-app smoke test so nightly/release builds fail before shipping broken image attachments.
Background
Nightly macOS builds could reach the real
attach_fileraster path and then fail when sharp tried to load libvips from inside the packaged app. The existing unit tests covered resize behavior in development mode, but nothing verified the packaged.apporapp.asar.unpackedcontents.Implementation
sharpand@imgruntime trees to Electron'sasarUnpacklistreadAttachmentFromPath()flow against oversized PNG/JPEG fixtures and exitsscripts/checkMacAttachFileRuntime.tsplusmake check-mac-attach-file-runtimeto verify unpacked native assets and launch the packaged app in smoke-test modemake dist-mac-releaseValidation
nix shell nixpkgs#hadolint -c make static-checkenv LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libstdc++.so.6 bun test src/node/services/tools/attach_file.test.tsRisks
This increases the unpacked macOS app footprint by shipping the full
sharpand@imgruntime trees outside the ASAR. The change is intentionally scoped to packaging and smoke-test plumbing; the production attachment logic remains unchanged.Pains
The Linux sandbox here cannot build or launch the packaged macOS
.app, so the new packaged smoke test is validated structurally in code and delegated to the macOS release lane for runtime execution. Localsharptesting in this environment also requiredLD_PRELOAD=/usr/lib/x86_64-linux-gnu/libstdc++.so.6to work around the sandbox's default C++ runtime path.📋 Implementation Plan
Fix macOS nightly image attachments by unpacking sharp native assets
Goal
Nightly macOS Electron builds should be able to attach raster images through
attach_filewithout failing to load sharp/libvips at runtime.Verified context
src/node/services/tools/attach_file.ts->src/node/utils/attachments/readAttachmentFromPath.ts->src/node/utils/attachments/resizeRasterImageAttachment.ts.resizeRasterImageAttachment.tslazy-loads sharp viacreateRequire(__filename)/requireSharp("sharp"), so the failure happens only when a raster image is processed.package.jsoncurrently unpacks native assets fornode-ptyand@duckdbunderbuild.asarUnpack, but not forsharpor the@img/*packages that hold sharp's platform binaries and libvips dylibs.@img/sharp-libvips-darwin-arm64/lib/libvips-cpp.8.17.3.dylib, which matches sharp 0.34.x's platform-package layout.node_modules/sharpandnode_modules/@img, which is a strong repo-local signal that runtime needs both trees available outside a compressed bundle.src/node/services/tools/attach_file.test.ts) but do not validate a packaged macOS app, so ASAR regressions currently slip through.Why this looks like packaging, not install-time dependency drift
The failure is surfacing in the packaged nightly app after build, not during
bun installor unit tests. Current evidence points to Electron ASAR packing as the primary breakage: native sharp artifacts are present during development/test installs, but the packaged app cannot resolve the darwin-arm64 dylib chain once those assets remain inside the ASAR. That makesbuild.asarUnpackthe minimal, first-line fix.Recommended approach (primary)
Approach A: unpack sharp's full runtime trees in Electron packaging
Net product-code estimate: +2 LoC in
package.jsonChange
package.jsonbuild.asarUnpackto add broad, low-risk globs for the full sharp runtime trees:**/node_modules/sharp/**/***/node_modules/@img/**/*Why this is the best first fix
node-pty,@duckdb).attach_filelogic.Fallback only if app size or packaging behavior becomes a problem
Approach B: narrow the unpack globs to sharp build/lib + native files under
@imgNet product-code estimate: +4 to +8 LoC in
package.jsonUse more targeted patterns such as:
**/node_modules/sharp/build/**/***/node_modules/sharp/lib/**/***/node_modules/@img/**/*.node**/node_modules/@img/**/*.dylib**/node_modules/@img/**/*.soOnly take this route if the broad globs materially bloat the unpacked app or if electron-builder's glob handling needs tighter targeting.
Implementation plan
Phase 1 — Fix the packaged runtime
Files:
package.jsonbuild.asarUnpackso Electron ships sharp's runtime artifacts outside the compressed ASAR.src/node/utils/attachments/resizeRasterImageAttachment.tsunless the packaging-only fix is proven insufficient.Quality gate after Phase 1
make dist-macor the nightly-equivalent release target).app.asar.unpackedcontains both:node_modules/sharp/...node_modules/@img/...sharp.node) and a darwin libvips dylib (libvips-cpp*.dylib).node_modulesdoes contain sharp +@imgbefore packaging, escalate to Approach B before touching runtime code.@imgpackages innode_modulesto begin with, inspect.github/actions/setup-mux/action.yml/ dependency-cache behavior as a second-order fix only after ruling out the ASAR unpack issue.Phase 2 — Add a regression guard that would have caught this
Likely files: packaged-app smoke test under
tests/e2e/**, a dedicated script, and/or CI wiring in.github/workflows/_desktop-release.ymlorMakefileattach_filewith a large PNG or JPEG so sharp must load and resize.ERR_DLOPEN_FAILED/ missinglibvips-cpperror is emitted.Quality gate after Phase 2
attach_file.Phase 3 — Wire the guard into the macOS nightly path
Likely files:
.github/workflows/_desktop-release.yml, supporting Makefile/script entrypointsQuality gate after Phase 3
Acceptance criteria
attach_filewithout sharp/libvips load errors..appcontains unpacked sharp runtime assets inapp.asar.unpacked.Dogfooding and reviewer evidence
attach_file:app.asar.unpackedshowing unpackedsharpand@imgtrees,Risks and notes
Generated with
mux• Model:openai:gpt-5.4• Thinking:high• Cost:$5.41