Skip to content

fix(desktop): bundle DuckDB native binding into x64 macOS build #4694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 2 commits into from
May 19, 2026
Merged

fix(desktop): bundle DuckDB native binding into x64 macOS build #4694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
012c18e
fix(desktop): bundle DuckDB native binding into x64 macOS build
saddlepaddle May 18, 2026
c545de6
ci(desktop): assert DuckDB native binding is in the packaged app
saddlepaddle May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/build-desktop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,19 @@ jobs:
}
"$CLI_BIN" --version

- name: Verify native bindings packaged
working-directory: apps/desktop
run: |
APP_DIR=$(find release -maxdepth 2 -name "*.app" -type d | head -1)
UNPACKED="$APP_DIR/Contents/Resources/app.asar.unpacked/node_modules"
BINDING="$UNPACKED/@duckdb/node-bindings-darwin-${{ matrix.arch }}/duckdb.node"
test -f "$BINDING" || {
echo "::error::DuckDB native binding missing from packaged app — Intel/arm launch will crash. Expected: $BINDING"
ls -la "$UNPACKED/@duckdb" 2>/dev/null || echo " (no @duckdb directory in app.asar.unpacked)"
exit 1
}
echo "OK: bundled $BINDING ($(du -h "$BINDING" | cut -f1))"

- name: Upload DMG artifact
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
with:
Expand Down
13 changes: 13 additions & 0 deletions apps/desktop/runtime-dependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,19 @@ const externalizedRuntimeModules: ExternalizedRuntimeModule[] = [
],
asarUnpackGlobs: ["**/node_modules/@libsql/**/*"],
},
{
specifier: "@mastra/duckdb",
materialize: [
"@mastra/duckdb",
"@duckdb/node-api",
"@duckdb/node-bindings",
],
packagedCopies: [
copyWholeModule("@mastra/duckdb"),
copyWholeModule("@duckdb"),
],
asarUnpackGlobs: ["**/node_modules/@duckdb/**/*"],
},
Comment on lines +82 to +94
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Suggested change
{
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.

];

const packagedSupportModules = [
Expand Down
45 changes: 45 additions & 0 deletions apps/desktop/scripts/copy-native-modules.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,50 @@ function copyParcelWatcherPlatformPackages(nodeModulesDir: string): void {
}
}

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,
);
}
Comment on lines +473 to +515
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.


function prepareNativeModules() {
console.log("Preparing external runtime modules for electron-builder...");
console.log(
Expand All @@ -488,6 +532,7 @@ function prepareNativeModules() {
copyAstGrepPlatformPackages(nodeModulesDir);
copyParcelWatcherPlatformPackages(nodeModulesDir);
copyLibsqlDependencies(nodeModulesDir);
copyDuckdbPlatformPackages(nodeModulesDir);

console.log("\nDone!");
}
Expand Down
22 changes: 22 additions & 0 deletions apps/desktop/scripts/validate-native-runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,35 @@ function validateParcelWatcherPrepared(): void {
);
}

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})`,
);
}
Comment on lines +507 to +526
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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 main(): void {
validateWorkspacePackagesBundled();
validateOnlyExpectedExternalRequires();
validateLibsqlNotBundled();
validateParcelWatcherNotBundled();
validateNativeModulesPrepared();
validateParcelWatcherPrepared();
validateDuckdbPrepared();
console.log("[validate:native-runtime] All checks passed");
}

Expand Down
Loading