fix(desktop): verify better-sqlite3's shipped prebuild - #485
Conversation
Greptile SummaryThe PR updates desktop artifact verification for better-sqlite3 v13’s shipped N-API prebuild layout instead of expecting the legacy rebuilt binding.
Confidence Score: 5/5The PR appears safe to merge, with the verifier aligned to the shipped better-sqlite3 v13 prebuilds on every supported desktop target. The supported platform and architecture mappings resolve to the documented v13 prebuild filenames, unsupported platforms are rejected before verification, and the retained native-header checks continue to detect architecture mismatches.
|
| Filename | Overview |
|---|---|
| apps/desktop/scripts/verify-artifacts.mts | Updates better-sqlite3 verification to inspect the target-specific v13 N-API prebuild while preserving binary architecture checks. |
| apps/desktop/electron-builder.yml | Corrects comments to reflect that packaged applications load better-sqlite3’s shipped prebuild rather than a rebuilt Release binding. |
| apps/daemon/scripts/package-daemon.mts | Updates packaging documentation to distinguish better-sqlite3’s target prebuilds from host-specific native dependencies. |
| apps/daemon/AGENTS.md | Aligns daemon packaging guidance with the current native dependency distribution model. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Target platform and architecture] --> B[Map builder platform to Node platform]
B --> C[Resolve better-sqlite3 prebuild path]
C --> D[Check app.asar.unpacked]
D --> E[Validate Mach-O, PE, or ELF architecture]
E --> F[Accept or reject artifact]
Reviews (1): Last reviewed commit: "fix(desktop): verify better-sqlite3's sh..." | Re-trigger Greptile
There was a problem hiding this comment.
ℹ️ The fix itself is correct and I could not find a way for it to fail. Two comment-accuracy points inline, plus two more stale sites the sweep missed.
Reviewed changes — the whole 1-commit diff (4 files), plus independent verification of the better-sqlite3 v13 layout, its binding-resolution order, and electron-builder's smart-unpack behavior against the installed packages.
verify-artifacts.mtsasserts the shipped prebuild —NATIVE_BINDINGis replaced bysqliteBinding(platform, arch)over a newNODE_PLATFORMmap, targetingnode_modules/better-sqlite3/prebuilds/<nodePlatform>-<arch>.node; the Mach-O/PE/ELF arch check is retained and the entry becomes conditional, mirroringkeyringBinding.- Three comment corrections —
electron-builder.yml,apps/daemon/scripts/package-daemon.mts, andapps/daemon/AGENTS.mddrop theirprebuild-install/build/Releaseclaims.
What I verified independently, since the whole PR rests on it:
better-sqlite3@13.0.2ships eight flatprebuilds/<platform>-<arch>.nodefiles,gypfile: false, no install script, andbinding.gypsets both targets totype: nonewhen a prebuild exists — so@electron/rebuildreally is a no-op andbuild/holds only node-gyp stamps.lib/binding.js'sgetPrebuildPath()is tried beforebuild/Release, so the asserted file is genuinely the one the daemon loads.app-builder-lib/out/asar/unpackDetector.jsaddsstat.moduleRootPathtoautoUnpackDirson any.nodehit, so the wholebetter-sqlite3directory is smart-unpacked — the asserted path is correct for all six mac/win/linux × x64/arm64 targets, not just the arm64 mac the description covers.- Nothing in
.github/workflows/, the packaging scripts, or the E2E suite still referencesbuild/Release/better_sqlite3.node.
ℹ️ The stale-comment sweep stops two sites short
The PR corrects three comments but leaves two more carrying the same pre-v13 premise, one of which now directly contradicts an edit in this diff.
Technical details
# Two more `@electron/rebuild` / `build/Release` claims left stale
## Affected sites
- `docs/RELEASE.md:257` — describes this exact gate as "the smartUnpacked `better-sqlite3` binding is
present and its Mach-O/PE/ELF header targets the app's arch (**the tripwire for the CODE-107
Windows rebuild miss** …)". There is no rebuild left to miss: `binding.gyp` short-circuits to
`type: none` and the prebuild is named by arch, so the header check can no longer fail for that
reason. The sentence should describe what the gate now covers (the prebuild for the target is
present and smart-unpacked) rather than a tripwire that no longer exists.
- `apps/desktop/scripts/package-app.mts:116` — `pruneStaging`'s doc says "notably
better-sqlite3/deps must stay HERE in staging because `@electron/rebuild` compiles from it before
collection", presenting the from-source compile as the routine path. The `electron-builder.yml`
edit in this diff (new lines 61-63) now says `deps` is "only ever compiled if a from-source
`@electron/rebuild` happens". The two now disagree about the same fact.
## Required outcome
- Both sites state the post-v13 reality, and `electron-builder.yml` and `package-app.mts` agree on
whether the `better-sqlite3/deps` compile is routine or conditional.
## Open questions for the human
- Given the compile is now unreachable in practice, is keeping `better-sqlite3/deps` in staging
still worth stating as a constraint, or has it become a plain "harmless, excluded from the asar
anyway"? That answer decides how both comments should read.ℹ️ Nitpicks
NODE_PLATFORM(verify-artifacts.mts:79-83) has to stay key-for-key withEXPECTED; a fourth platform added toEXPECTEDwithout a matching entry makessqliteBindingreturnnulland the better-sqlite3 gate disappear silently instead of failing.Partial<Record<string, string>>gives no help here. Not worth a runtime guard — but the two maps being coupled is invisible from either one.
Claude Opus | 𝕏
PeronGH
left a comment
There was a problem hiding this comment.
Approving — this is necessary, not cosmetic. Without it the next tagged release fails at verify-artifacts.
Verified against a fresh clone of this branch with a real staging closure (pnpm --filter @linkcode/desktop --prod deploy --legacy --cpu=x64, pnpm 11.10.0):
- The closure has exactly one better-sqlite3, and
prebuilds/holds all eight flat files —darwin-arm64.node,win32-x64.node, … Theprebuilds/${nodePlatform}-${arch}.nodetemplate matches exactly (note it is not the prebuildify directory convention). - There is no
build/directory at all, so the oldbuild/Release/better_sqlite3.nodeassertion would fail on every artifact. - It won't come back via
@electron/rebuild: v13'sbinding.gypgates both targets onforce_build==1 or prebuild_exists==0, whereprebuild_existsshells out tonode lib/binding.js. In the staged package that prints1, so both targets collapse totype: noneand a plainnode-gyp rebuildproduces nothing. - The prebuild loads with no build step —
getPrebuildPath()→prebuilds/linux-x64.node, andnew Database(':memory:')+pragma('journal_mode = WAL')returns SQLite 3.53.4.
The revised deps/** rationale in electron-builder.yml also checks out: binding.gyp includes: ['deps/common.gypi'] unconditionally, so gyp needs deps/ to parse even in the no-op case.
Two non-blocking points.
1. The check is now close to tautological
Nothing prunes non-target prebuilds. pruneStaging (package-app.mts:118) only drops .map/.ts/.md, and the only better-sqlite3 exclusion in electron-builder.yml is deps/**. So all eight prebuilds ship in every artifact and all eight smart-unpack.
That means prebuilds/darwin-arm64.node is present in the Windows build too, and its header is arm64 by construction — readBinaryArch can never disagree with expectedArch. Under v12 the check was genuinely target-sensitive; it now only proves the tarball wasn't mangled.
Fix that restores it and saves space: prune the seven non-target prebuilds in pruneStaging. That's ~14.3 MB of dead weight per artifact (16.5 MB shipped, 2.2 MB used), and once only the target prebuild survives, the existence + arch assertion means something again.
2. Comment describes a failure mode that doesn't exist yet
verify-artifacts.mts:87 — "what breaks is the staging prune keeping the wrong target" — nothing currently prunes prebuilds per target. Accurate only after the change above.

The v0.27.0 release build failed on all three platforms with:
better-sqlite3 13 (#443) changed how the binding is distributed. v12 downloaded a per-Node-ABI
build into
build/Release/better_sqlite3.nodeviaprebuild-install; v13 ships eight N-API(
NAPI_VERSION=10) prebuilds in the tarball asprebuilds/<platform>-<arch>.node, and itsbinding.gypturns both targets intotype: nonewhenever a prebuild exists. node-gyp thereforeonly touches its stamps,
@electron/rebuildis a no-op, and nothing is ever written tobuild/Release— the packaged app is fine,verify-artifacts.mtswas just looking in the oldplace. Every other check in that run passed.
verify-artifacts.mtsnow asserts the target's prebuild instead, keeping the Mach-O/PE/ELF archcheck. Three stale comments about
prebuild-install/build/Releaseare corrected with it.Verification
@electron/rebuildstays a no-op andapp.asar.unpacked/node_modules/better-sqlite3/prebuilds/darwin-arm64.nodeis in place.darwin-x64.nodewhen pointed at an arm64 app as if it were the x64 one.daemon.db, so the shipped prebuild iswhat it loads.
pnpm check:cipasses.Follow-up (not in this PR)
v13's tarball carries all eight prebuilds and electron-builder smart-unpacks the whole module, so
every artifact now ships ~15 MB of foreign-platform
.nodefiles. Pruning the staging dir down tothe target (plus the host's, which is what keeps
binding.gypfrom compiling from source) removesthat; left out here to keep the release unblocking minimal.