fix(desktop): bundle DuckDB native binding into x64 macOS build#4694
Conversation
The Intel (x64) macOS build crashed on launch with `Cannot find module '@duckdb/node-bindings-darwin-x64/duckdb.node'`. mastracode is externalized, so Node resolves @mastra/duckdb → @duckdb/node-api → @duckdb/node-bindings → the platform binding from node_modules at runtime, but the desktop native-module pipeline never accounted for DuckDB. The binding is a cpu/os-gated optional dependency, so Bun on the arm64 build runner only installs the arm64 one — the x64 leg packaged no x64 binding at all. - runtime-dependencies.ts: externalize @mastra/duckdb and materialize + copy @mastra/duckdb / @duckdb/node-api / @duckdb/node-bindings, with asarUnpack for the native @duckdb files. - copy-native-modules.ts: add copyDuckdbPlatformPackages to fetch the TARGET_ARCH @duckdb/node-bindings-<platform>-<arch> package from npm when it is missing from the Bun store (cross-arch builds). - validate-native-runtime.ts: guard that the target-arch duckdb binding is present before packaging. Fixes #4666
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 4 individual chapters for you: 
Chapters generated by Stage for commit c545de6 on May 18, 2026 10:43pm UTC. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds comprehensive DuckDB native binding support to the desktop build pipeline. A configuration declares DuckDB modules for bundling, build-time logic materializes the correct platform/architecture-specific binding, and validation checks ensure the binding is present before the build succeeds. ChangesDuckDB Native Binding Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes an Intel (x64) macOS crash on launch caused by the DuckDB native binding (
Confidence Score: 4/5Safe to merge — the change correctly wires DuckDB into an existing, well-tested native-module pipeline and is mechanically consistent with the libsql, ast-grep, and parcel-watcher implementations. The core logic is sound and the cross-arch fetch path follows established patterns. The asarUnpack glob covers the entire @duckdb scope rather than just the native binding packages, and the validate function has no early-exit guard if DuckDB is ever made optional on a per-platform basis. Neither affects correctness for the current supported platforms. No files require special attention. copy-native-modules.ts is the most complex change but follows the existing copyAstGrepPlatformPackages pattern closely.
|
| Filename | Overview |
|---|---|
| apps/desktop/runtime-dependencies.ts | Adds @mastra/duckdb as an externalized runtime module with materialize, packagedCopies, and asarUnpackGlobs entries; the asarUnpack glob is broader than strictly necessary (covers all of @duckdb instead of only native binding packages). |
| apps/desktop/scripts/copy-native-modules.ts | Adds copyDuckdbPlatformPackages which resolves the target-arch binding from @duckdb/node-bindings optionalDependencies, fetching from npm when absent — correctly handles the cross-arch scenario. The targetVersion from optionalDeps is passed verbatim to the npm tarball URL, which could break if a range is ever used instead of an exact version. |
| apps/desktop/scripts/validate-native-runtime.ts | Adds validateDuckdbPrepared which checks for the target-arch duckdb.node file before packaging; lacks an early-exit guard for platforms/configs where DuckDB is not installed, unlike the existing parcel-watcher and libsql validators. |
Sequence Diagram
sequenceDiagram
participant CI as CI Runner (arm64)
participant Copy as copy-native-modules.ts
participant Bun as Bun Store
participant npm as npm Registry
participant Validate as validate-native-runtime.ts
participant Builder as electron-builder
CI->>Copy: "bun run copy:native-modules TARGET_ARCH=x64"
Copy->>Copy: copyDuckdbPlatformPackages()
Copy->>Copy: "read @duckdb/node-bindings optionalDependencies"
Copy->>Bun: "check for @duckdb/node-bindings-darwin-x64"
Bun-->>Copy: not found (arm64 runner x64 target)
Copy->>npm: "fetch @duckdb/node-bindings-darwin-x64 tarball"
npm-->>Copy: duckdb.node + libduckdb.dylib extracted
CI->>Validate: bun run validate:native-runtime
Validate->>Validate: validateNativeModulesPrepared()
Validate->>Validate: validateDuckdbPrepared()
Validate-->>CI: all checks passed
CI->>Builder: electron-builder package
Builder-->>CI: x64 DMG with correct native binding
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
apps/desktop/scripts/validate-native-runtime.ts:507-526
**`validateDuckdbPrepared` lacks a guard for when `@duckdb/node-bindings` is not installed**
All other similar validators (`validateParcelWatcherPrepared`, the ast-grep section of `validateNativeModulesPrepared`) return early or emit a warning when no candidates apply to the current platform. `validateDuckdbPrepared` unconditionally fails if `duckdb.node` is not present. In practice this is shielded by `validateNativeModulesPrepared` running first (since `@duckdb/node-bindings` is in `requiredMaterializedNodeModules`), but there is no analogous guard here if DuckDB is ever made optional per-platform. The failure message would then say "Missing platform-specific @duckdb/node-bindings package" rather than a clear skip.
### Issue 2 of 3
apps/desktop/scripts/copy-native-modules.ts:473-515
**`targetVersion` from `optionalDependencies` is passed verbatim to `fetchNpmPackage`**
`copyExactModuleVersion` → `fetchNpmPackage` constructs the tarball URL as `https://registry.npmjs.org/${pkg}/-/${bare}-${version}.tgz`. If `optionalDependencies` ever specifies a semver range instead of an exact version, the URL is malformed and the fetch silently falls through to `process.exit(1)`. The same pattern exists in `copyLibsqlDependencies`, so this is not unique to this PR, but the risk is worth noting given the cross-arch fetch path is the sole fallback for the x64 binding in CI.
### Issue 3 of 3
apps/desktop/runtime-dependencies.ts:82-94
**`asarUnpackGlobs` is broader than necessary — unpacks the pure-JS `@duckdb/node-api` package**
The glob `"**/node_modules/@duckdb/**/*"` asar-unpacks the entire `@duckdb` scope, including `@duckdb/node-api` which is a pure-JS module that does not need filesystem access outside the asar. Existing patterns in this file are scoped to the packages that actually require native access (e.g., `@ast-grep/napi*`, `@parcel/watcher*`). A more targeted glob like `"**/node_modules/@duckdb/node-bindings*/**/*"` reduces the unpacked asar size and more precisely documents what actually needs native filesystem access.
```suggestion
{
specifier: "@mastra/duckdb",
materialize: [
"@mastra/duckdb",
"@duckdb/node-api",
"@duckdb/node-bindings",
],
packagedCopies: [
copyWholeModule("@mastra/duckdb"),
copyWholeModule("@duckdb"),
],
asarUnpackGlobs: ["**/node_modules/@duckdb/node-bindings*/**/*"],
},
```
Reviews (1): Last reviewed commit: "fix(desktop): bundle DuckDB native bindi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function validateDuckdbPrepared(): void { | ||
| const nodeModulesDir = join(projectRoot, "node_modules"); | ||
| const targetArch = process.env.TARGET_ARCH || process.arch; | ||
| const targetPlatform = process.env.TARGET_PLATFORM || process.platform; | ||
| const bindingPackage = `@duckdb/node-bindings-${targetPlatform}-${targetArch}`; | ||
|
|
||
| if (!existsSync(join(nodeModulesDir, bindingPackage, "duckdb.node"))) { | ||
| fail( | ||
| [ | ||
| "Missing platform-specific @duckdb/node-bindings package.", | ||
| `Expected: ${bindingPackage}/duckdb.node`, | ||
| "Run `bun run copy:native-modules` and ensure optional dependencies are materialized.", | ||
| ].join("\n"), | ||
| ); | ||
| } | ||
|
|
||
| console.log( | ||
| `[validate:native-runtime] OK: platform duckdb binding present (${bindingPackage})`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
validateDuckdbPrepared lacks a guard for when @duckdb/node-bindings is not installed
All other similar validators (validateParcelWatcherPrepared, the ast-grep section of validateNativeModulesPrepared) return early or emit a warning when no candidates apply to the current platform. validateDuckdbPrepared unconditionally fails if duckdb.node is not present. In practice this is shielded by validateNativeModulesPrepared running first (since @duckdb/node-bindings is in requiredMaterializedNodeModules), but there is no analogous guard here if DuckDB is ever made optional per-platform. The failure message would then say "Missing platform-specific @duckdb/node-bindings package" rather than a clear skip.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/scripts/validate-native-runtime.ts
Line: 507-526
Comment:
**`validateDuckdbPrepared` lacks a guard for when `@duckdb/node-bindings` is not installed**
All other similar validators (`validateParcelWatcherPrepared`, the ast-grep section of `validateNativeModulesPrepared`) return early or emit a warning when no candidates apply to the current platform. `validateDuckdbPrepared` unconditionally fails if `duckdb.node` is not present. In practice this is shielded by `validateNativeModulesPrepared` running first (since `@duckdb/node-bindings` is in `requiredMaterializedNodeModules`), but there is no analogous guard here if DuckDB is ever made optional per-platform. The failure message would then say "Missing platform-specific @duckdb/node-bindings package" rather than a clear skip.
How can I resolve this? If you propose a fix, please make it concise.| function copyDuckdbPlatformPackages(nodeModulesDir: string): void { | ||
| const nodeBindingsPath = join(nodeModulesDir, "@duckdb", "node-bindings"); | ||
| const nodeBindingsPkgJsonPath = join(nodeBindingsPath, "package.json"); | ||
| if (!existsSync(nodeBindingsPkgJsonPath)) return; | ||
|
|
||
| type DuckdbBindingsPackageJson = { | ||
| optionalDependencies?: Record<string, string>; | ||
| }; | ||
| const nodeBindingsPkg = JSON.parse( | ||
| readFileSync(nodeBindingsPkgJsonPath, "utf8"), | ||
| ) as DuckdbBindingsPackageJson; | ||
| const optionalDeps = nodeBindingsPkg.optionalDependencies ?? {}; | ||
|
|
||
| console.log("\nPreparing duckdb platform package..."); | ||
|
|
||
| // The native binding is a `cpu`/`os`-gated optional dependency, so Bun only | ||
| // installs the host's. For the target arch, fetch it from npm when missing. | ||
| const targetSuffix = `${TARGET_PLATFORM}-${TARGET_ARCH}`; | ||
| const targetEntry = Object.entries(optionalDeps).find(([name]) => | ||
| name.endsWith(targetSuffix), | ||
| ); | ||
| if (!targetEntry) { | ||
| console.error( | ||
| ` [ERROR] No @duckdb/node-bindings optional dependency matched ${targetSuffix}`, | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const [targetName, targetVersion] = targetEntry; | ||
| const destPath = join(nodeModulesDir, targetName); | ||
| if (existsSync(destPath)) { | ||
| copyModuleIfSymlink(nodeModulesDir, targetName, true); | ||
| return; | ||
| } | ||
|
|
||
| copyExactModuleVersion( | ||
| nodeModulesDir, | ||
| targetName, | ||
| targetVersion, | ||
| destPath, | ||
| true, | ||
| ); | ||
| } |
There was a problem hiding this comment.
targetVersion from optionalDependencies is passed verbatim to fetchNpmPackage
copyExactModuleVersion → fetchNpmPackage constructs the tarball URL as https://registry.npmjs.org/${pkg}/-/${bare}-${version}.tgz. If optionalDependencies ever specifies a semver range instead of an exact version, the URL is malformed and the fetch silently falls through to process.exit(1). The same pattern exists in copyLibsqlDependencies, so this is not unique to this PR, but the risk is worth noting given the cross-arch fetch path is the sole fallback for the x64 binding in CI.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/scripts/copy-native-modules.ts
Line: 473-515
Comment:
**`targetVersion` from `optionalDependencies` is passed verbatim to `fetchNpmPackage`**
`copyExactModuleVersion` → `fetchNpmPackage` constructs the tarball URL as `https://registry.npmjs.org/${pkg}/-/${bare}-${version}.tgz`. If `optionalDependencies` ever specifies a semver range instead of an exact version, the URL is malformed and the fetch silently falls through to `process.exit(1)`. The same pattern exists in `copyLibsqlDependencies`, so this is not unique to this PR, but the risk is worth noting given the cross-arch fetch path is the sole fallback for the x64 binding in CI.
How can I resolve this? If you propose a fix, please make it concise.| { | ||
| specifier: "@mastra/duckdb", | ||
| materialize: [ | ||
| "@mastra/duckdb", | ||
| "@duckdb/node-api", | ||
| "@duckdb/node-bindings", | ||
| ], | ||
| packagedCopies: [ | ||
| copyWholeModule("@mastra/duckdb"), | ||
| copyWholeModule("@duckdb"), | ||
| ], | ||
| asarUnpackGlobs: ["**/node_modules/@duckdb/**/*"], | ||
| }, |
There was a problem hiding this comment.
asarUnpackGlobs is broader than necessary — unpacks the pure-JS @duckdb/node-api package
The glob "**/node_modules/@duckdb/**/*" asar-unpacks the entire @duckdb scope, including @duckdb/node-api which is a pure-JS module that does not need filesystem access outside the asar. Existing patterns in this file are scoped to the packages that actually require native access (e.g., @ast-grep/napi*, @parcel/watcher*). A more targeted glob like "**/node_modules/@duckdb/node-bindings*/**/*" reduces the unpacked asar size and more precisely documents what actually needs native filesystem access.
| { | |
| specifier: "@mastra/duckdb", | |
| materialize: [ | |
| "@mastra/duckdb", | |
| "@duckdb/node-api", | |
| "@duckdb/node-bindings", | |
| ], | |
| packagedCopies: [ | |
| copyWholeModule("@mastra/duckdb"), | |
| copyWholeModule("@duckdb"), | |
| ], | |
| asarUnpackGlobs: ["**/node_modules/@duckdb/**/*"], | |
| }, | |
| { | |
| specifier: "@mastra/duckdb", | |
| materialize: [ | |
| "@mastra/duckdb", | |
| "@duckdb/node-api", | |
| "@duckdb/node-bindings", | |
| ], | |
| packagedCopies: [ | |
| copyWholeModule("@mastra/duckdb"), | |
| copyWholeModule("@duckdb"), | |
| ], | |
| asarUnpackGlobs: ["**/node_modules/@duckdb/node-bindings*/**/*"], | |
| }, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/runtime-dependencies.ts
Line: 82-94
Comment:
**`asarUnpackGlobs` is broader than necessary — unpacks the pure-JS `@duckdb/node-api` package**
The glob `"**/node_modules/@duckdb/**/*"` asar-unpacks the entire `@duckdb` scope, including `@duckdb/node-api` which is a pure-JS module that does not need filesystem access outside the asar. Existing patterns in this file are scoped to the packages that actually require native access (e.g., `@ast-grep/napi*`, `@parcel/watcher*`). A more targeted glob like `"**/node_modules/@duckdb/node-bindings*/**/*"` reduces the unpacked asar size and more precisely documents what actually needs native filesystem access.
```suggestion
{
specifier: "@mastra/duckdb",
materialize: [
"@mastra/duckdb",
"@duckdb/node-api",
"@duckdb/node-bindings",
],
packagedCopies: [
copyWholeModule("@mastra/duckdb"),
copyWholeModule("@duckdb"),
],
asarUnpackGlobs: ["**/node_modules/@duckdb/node-bindings*/**/*"],
},
```
How can I resolve this? If you propose a fix, please make it concise.build-desktop.yml never inspected the packaged output, so a missing native binding only surfaced when users launched the app. Add a step that cracks open app.asar.unpacked and fails the build if the target-arch @duckdb/node-bindings-darwin-<arch>/duckdb.node is absent.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Verified — full desktop build greenTriggered x64 leg, the previously-broken one — fix chain confirmed on the CI runner:
Independent inspection of the downloaded x64 artifact (
The Follow-up (not blocking, pre-existing)The x64 build also ships the unused |
* fix(desktop): bundle DuckDB native binding into x64 macOS build The Intel (x64) macOS build crashed on launch with `Cannot find module '@duckdb/node-bindings-darwin-x64/duckdb.node'`. mastracode is externalized, so Node resolves @mastra/duckdb → @duckdb/node-api → @duckdb/node-bindings → the platform binding from node_modules at runtime, but the desktop native-module pipeline never accounted for DuckDB. The binding is a cpu/os-gated optional dependency, so Bun on the arm64 build runner only installs the arm64 one — the x64 leg packaged no x64 binding at all. - runtime-dependencies.ts: externalize @mastra/duckdb and materialize + copy @mastra/duckdb / @duckdb/node-api / @duckdb/node-bindings, with asarUnpack for the native @duckdb files. - copy-native-modules.ts: add copyDuckdbPlatformPackages to fetch the TARGET_ARCH @duckdb/node-bindings-<platform>-<arch> package from npm when it is missing from the Bun store (cross-arch builds). - validate-native-runtime.ts: guard that the target-arch duckdb binding is present before packaging. Fixes #4666 * ci(desktop): assert DuckDB native binding is in the packaged app build-desktop.yml never inspected the packaged output, so a missing native binding only surfaced when users launched the app. Add a step that cracks open app.asar.unpacked and fails the build if the target-arch @duckdb/node-bindings-darwin-<arch>/duckdb.node is absent.
Non-applicable to current fork structure: superset-sh#3960 and superset-sh#4068 require linux-arm64/full CLI dist targets that this fork does not ship; superset-sh#4678 targets a relay deploy script intentionally absent from the fork; superset-sh#4694 requires DuckDB native packaging but the fork has no DuckDB runtime dependency; superset-sh#4822 targets removed v1 import modal paths; superset-sh#4826 assumes upstream release-cli.yml while this fork uses build-cli.yml with draft release semantics.
2 participants
Problem
The Intel (x64) macOS build crashes immediately on launch:
Any Intel Mac user is fully blocked — the app cannot start. Reproduced across 1.9.5 and 1.9.6; a clean reinstall does not help.
Root cause
mastracodeis externalized from the desktop main bundle, so at runtime Node resolves@mastra/duckdb→@duckdb/node-api→@duckdb/node-bindings→ the platform binding fromnode_modules. The desktop native-module pipeline (runtime-dependencies.ts+copy-native-modules.ts) never accounted for DuckDB at all.@duckdb/node-bindings-darwin-x64is acpu: "x64"/os: "darwin"-gated optional dependency. Both macOS build legs run on an arm64 runner, so Bun only ever installs the arm64 binding — the x64 leg packages no x64 binding. arm64 was unaffected only because the runner arch happened to match.packages/cli/scripts/build-dist.tsandpackages/host-service/build.tsalready externalize the full@duckdb/node-bindings-*set, confirming the desktop config was the outlier.Fix
runtime-dependencies.ts— externalize@mastra/duckdband materialize + copy@mastra/duckdb/@duckdb/node-api/@duckdb/node-bindings, withasarUnpackfor the native@duckdbfiles (duckdb.node+libduckdb.dylib).copy-native-modules.ts— addcopyDuckdbPlatformPackages, modeled oncopyAstGrepPlatformPackages/copyLibsqlDependencies: resolve the@duckdb/node-bindingsoptionalDependencies, and fetch theTARGET_ARCHplatform package from npm at the pinned1.5.2-r.1version when it is missing from the Bun store (cross-arch build scenario).validate-native-runtime.ts— addvalidateDuckdbPrepared, a build-time guard that the target-arch binding is present before packaging, matching the existing libsql / ast-grep / parcel-watcher guards.Linux audit
Per the ticket gotcha — the Linux job runs on
ubuntu-latest(x64 host) withTARGET_ARCH=x64. Host arch matches target, so Bun installs@duckdb/node-bindings-linux-x64andcopyDuckdbPlatformPackagesfinds it in the Bun store. No gap, no workflow change needed.Verification
bun run scripts/copy-native-modules.ts(host arm64) materializes the three DuckDB modules and copies the arm64 binding from the Bun store.TARGET_ARCH=x64 bun run scripts/copy-native-modules.tsfetches@duckdb/node-bindings-darwin-x64@1.5.2-r.1(duckdb.node+libduckdb.dylib) from npm — the previously-missing piece.bun run lintandbun run typecheckpass.arch -x86_64).Fixes #4666
Summary by cubic
Fixes the Intel (x64) macOS desktop crash by bundling the DuckDB native binding into the x64 build and validating it at build time and in CI. Ensures cross-arch builds fetch the correct
@duckdb/node-bindings-*package so the app launches on x64. Fixes #4666.@mastra/duckdband materialized/copied@mastra/duckdb,@duckdb/node-api, and@duckdb/node-bindings; unpack native@duckdbfiles from ASAR.@duckdb/node-bindings-<platform>-<arch>; fetches the target-arch package from npm when Bun doesn’t install it.@duckdb/node-bindings-darwin-<arch>/duckdb.nodeis missing from the packaged app.Written for commit c545de6. Summary will update on new commits. Review in cubic
Summary by CodeRabbit