Repository navigation
build: fix packaged-app startup (npm migration + dependency boundary + smoke gate) - #41
Merged
Merged
Conversation
electron-builder's node-module collector is built around npm's node_modules
layout. Against pnpm's virtual store it silently dropped transitive production
dependencies (jsdom -> whatwg-url -> tr46, http-proxy-agent -> debug -> ms),
which is how v1.2.1 shipped an app.asar that crashed with MODULE_NOT_FOUND.
Earlier commits worked around the symptom by hand-promoting the dropped
transitives into `dependencies` and by bundling every bundleable main-process
package. This removes the cause instead.
- package manager: pnpm -> npm for dev, CI and packaging
- delete pnpm-lock.yaml, generate package-lock.json (lockfileVersion 3)
- .npmrc: drop shamefully-hoist / node-linker; npm warned about both
- release.yml: `npm ci` + `npm run ...` (extra args after `--`), npm cache
- package.json: drop the `packageManager` pin and the pnpm block
- `dependencies` is back to the 4 packages that genuinely cannot be bundled
(better-sqlite3, sqlite-vec, jsdom, @mixmark-io/domino). ms, punycode, tr46,
util-deprecate and wrappy are gone: they are now collected as ordinary
transitives of jsdom, verified present in app.asar.
- electron-builder.yml:
- add `allowMissingDependencies: false` so a collector miss fails the build
instead of only warning (v26 default) and shipping a broken app. Needs
electron-builder >= 26.16.0, hence the bump from ^26.4.0. Missing optional
dependencies are still allowed and never fail the build.
- drop `npmRebuild: false` so native modules are rebuilt for the target
Electron ABI and target arch during packaging (`build:mac --arm64` would
otherwise ship whatever ABI postinstall produced on the build host).
- exclude package-lock.json instead of the deleted pnpm-lock.yaml
- dependency hygiene:
- drop @types/dompurify (deprecated stub; dompurify ships its own types)
- drop @types/react-router-dom@5 (react-router-dom 7 ships its own types)
- drop a duplicate jsdom entry in devDependencies
- drop ollama-ai-provider (unused; code imports ollama-ai-provider-v2)
- add an `overrides` entry pinning qwen-ai-provider's zod peer to 4. It is an
AI SDK v4 era provider declaring zod ^3; pnpm only warned and resolved it
against root zod 4, npm's strict peer resolution refuses to build the tree
at all. The override makes npm reproduce the shape pnpm already shipped
(qwen-ai-provider -> provider-utils@2 -> root zod 4, no zod 3 anywhere).
Verified on this tree:
- npm install clean; better-sqlite3 rebuilt against Electron 39.8.10
- npm run typecheck passes (confirms the @types removals are safe)
- npm run build passes
- npm run build:unpack passes: collector reports pm=npm, allowMissingDependencies
does not trip, app.asar contains jsdom plus whatwg-url, tr46, punycode, debug,
ms, cssstyle, util-deprecate and wrappy, and better_sqlite3.node / vec0.so are
unpacked from the asar
- eslint reports only the 6 pre-existing renderer react-hooks errors
Not addressed here, pre-existing and introduced by 725b4a3: the packaged app
still raises `Cannot find module 'script-loader!sql.js'` at startup. Moving
anki-apkg-export to devDependencies made rollup inline it, and rollup hoists
that webpack-only conditional require to the top of the bundle unconditionally.
This is independent of the package manager.
Both packages were moved to devDependencies in 725b4a3 so rollup would inline them. Inlining each one breaks the packaged app at startup, and fixing the first crash only revealed the second, because the bundle aborted before evaluation reached it. anki-apkg-export its dist branches on `typeof window` to pick between `require('script-loader!sql.js')` (a webpack-only loader prefix) and a plain `require('sql.js')`. Inlined, rollup hoists that dead browser require to the top of the CJS bundle, so the main process died immediately with `Cannot find module 'script-loader!sql.js'`. External restores the Node branch. sql.js is its own dependency and is 0.5.0's asm.js build (`main: js/sql.js`), so no .wasm is involved. pdfjs-dist its Node branch polyfills DOMMatrix/ImageData/Path2D by `createRequire(import.meta.url)`-ing its *optional* dependency @napi-rs/canvas, then evaluates `const SCALE_MATRIX = new DOMMatrix()` at module scope (pdf.mjs:17006). Two things went wrong at once: - the polyfill ran but `require('@napi-rs/canvas')` failed, because @napi-rs/canvas is an optionalDependency of a devDependency and so was never collected into app.asar (0 files, verified in the asar listing); - so DOMMatrix stayed undefined and the module-scope construction threw `ReferenceError: DOMMatrix is not defined`. The previous config comment claimed pdfjs "degrades gracefully without @napi-rs/canvas, which is only used by page.render()". That is wrong: the dependency is imported at module scope regardless of whether the app renders, which this app never does. External makes pdfjs resolve its real optional peer; @napi-rs/canvas + @napi-rs/canvas-linux-x64-gnu are now collected as ordinary production dependencies (12 files) and the .node is unpacked by asarUnpack. Also drops `canvas` / `@napi-rs/canvas` from rollupOptions.external. They were only ever there to stop rollup tripping over pdfjs's and jsdom's lazy optional requires; with both packages external, nothing bundled imports them, so the entries were dead. Verified by removing them and rebuilding. Verified on Linux x64 (Electron 39.8.10, electron-builder 26.16.1): - npm run typecheck, npm run build and npm run build:unpack all pass - app.asar now carries pdfjs-dist (408 files, pdf.worker.mjs included), @napi-rs/canvas (12 files), anki-apkg-export, sql.js, jszip and sha1 - unpacked natives: better_sqlite3.node and skia.linux-x64-gnu.node - packaged app starts, registers every IPC handler, opens knownote.db, runs a sqlite-vec/RESTART checkpoint, and shuts down with a clean exit; no `Cannot find module`, no `ReferenceError`, no "Cannot polyfill" warnings - Anki export exercised end to end: a 2-card deck produced a valid .apkg (`file` reports "Anki APKG file") whose collection.anki2 has the SQLite magic Not verified here: macOS arm64 and Windows x64 packaging, and the real GUI renderer (this ran headless). qwen-ai-provider's zod peer is still overridden; replacing that provider is the next change.
qwen-ai-provider@0.1.1 is an AI SDK v4 era provider: it depends on
@ai-sdk/provider-utils@2 and declares `zod: ^3.25.49` as a peer, while this app
is on AI SDK 5 (`ai@5`) and zod 4. It only ever installed because pnpm silently
resolved the conflicting peer against root zod 4, and after the move to npm it
needed a hand-written `overrides` entry just to let npm build a tree at all.
That override was a compatibility shim, not a fix, so it goes away with the
package.
Replace it with the AI SDK team's official Alibaba/Qwen provider. Note the
version trap: @ai-sdk/alibaba publishes dist-tags per AI SDK major, and only the
`ai-v5` tag targets this app's SDK.
ai-v5 -> 0.0.31 provider@2.0.5 provider-utils@3.0.39
ai-v6 -> 1.0.59 provider-utils@4
latest -> 2.0.54 provider-utils@5 (AI SDK 6+)
0.0.31's dependency set is exactly what ai@5 already resolves (provider@2.0.5,
provider-utils@3.0.39) and it is itself built on @ai-sdk/openai-compatible@1.0.56,
which this project already uses. Its zod peer is `^3.25.76 || ^4.1.8`, so zod 4
satisfies it and no override is needed. Pinned with `^0.0.31`, which for a 0.0.x
range resolves to exactly 0.0.31 rather than drifting.
A plain `npm install @ai-sdk/alibaba` would have pulled 2.0.54 and quietly
dragged the whole AI SDK to v6, which is why this is pinned to the ai-v5 tag.
Source change is a straight swap in AISDKProvider: both settings
(`baseURL`, `apiKey`) and the call shapes used by the class exist unchanged.
The provider is callable as `provider(modelId)` for chat and exposes
`textEmbeddingModel(modelId)` for embeddings, exactly as the ollama/openai paths
assume.
Verified:
- npm install clean with the `overrides` block deleted - no ERESOLVE
- npm run typecheck, npm run build, npm run build:unpack all pass
- @ai-sdk/alibaba is bundled into out/main (it is a devDependency, like the
other providers); qwen-ai-provider has 0 occurrences in the bundle
- direct API check: createAlibaba(...) is callable, `('qwen-max')` and
`.textEmbeddingModel('text-embedding-v2')` both return specificationVersion
v2 models, the interface ai@5 expects
- packaged app still starts with no fatal errors and registers all IPC handlers
- eslint unchanged: the same 6 pre-existing renderer react-hooks errors
Not verified: a live Qwen API call (needs a DashScope key) and the embedding
endpoint, which this provider points at DashScope's native API rather than the
OpenAI-compatible base URL the chat path uses.
anki-apkg-export is Babel-compiled CJS: its factory lives on `exports.default`
and `module.exports` itself is only `{ default, Exporter }`. 725b4a3 switched
this import from `require('anki-apkg-export').default` to an ESM default import
in order to make rollup inline the package, but that inlining crashed first on a
hoisted `script-loader!sql.js` require, so the export path was never reached.
Now that the package is external again, rollup emits a bare
`const AnkiExport = require("anki-apkg-export")` for the ESM import and does not
unwrap the interop, so every export threw:
TypeError: AnkiExport is not a constructor
Unwrap `.default` explicitly instead of depending on bundler interop, which is
what the pre-725b4a3 code did.
The defect was latent from 725b4a3 onwards, hidden behind the module-load crash
that preceded it. It only became reachable once the package was externalised and
the app could finally start, which is why it was found by `npm run
smoke:packaged` (added next) rather than by any earlier check.
Verified with the packaged app: the smoke test's anki check now writes a valid
.apkg and the run exits 0; reverting only this change makes it fail again with
the error above.
`npm run build` and `npm run build:unpack` both succeed on an artifact that
cannot start. That is how this branch began: v1.2.1 shipped an app.asar that
died with MODULE_NOT_FOUND on every AI call, and the two commits after it
packaged cleanly while the main process threw
`Cannot find module 'script-loader!sql.js'` and then
`ReferenceError: DOMMatrix is not defined` on startup. Nothing in the build
pipeline can see those, because the modules only load when the app actually runs.
Add `--smoke-test`: the real packaged executable starts, exercises the paths that
have actually broken, prints what it checked and exits with a status code.
src/main/smokeTest.ts the checks, run from app.whenReady() before any
window is created
scripts/smoke-packaged.mjs finds the unpacked binary for the host platform,
runs it, propagates the exit code, enforces a
timeout
npm run smoke:packaged build:unpack && smoke:packaged
Checks, in order:
- better-sqlite3 opens the database and loads the sqlite-vec extension
- drizzle migrations apply
- the vec0 table can be created and vec_version() answers
- a full upsert + KNN query + delete round trip through the app's own
SQLiteVectorStore, so the extension is used, not merely loaded
- jsdom parses a document
- pdfjs-dist loads, which only works if its module-scope `new DOMMatrix()`
found @napi-rs/canvas
- anki-apkg-export writes a real .apkg (this check is what caught the
`AnkiExport is not a constructor` bug fixed in the previous commit)
The extra packages are imported dynamically so the smoke test does not change
what the app loads at startup and a failure is reported as a failed check rather
than taking down the module graph before anything can be printed.
The launcher runs the app with `--user-data-dir` pointed at a throwaway temp
directory, because startup opens and migrates a real SQLite database and a smoke
test must never touch a developer's own ~/.config/knownote. Electron's sandbox is
only disabled when running as root, which is the norm in CI containers.
eslint.config.mjs: the TS-only `explicit-function-return-type` rule was only off
for `**/*.{ts,tsx}`, so it fired as an error on the plain-JS launcher, where it
cannot be satisfied. Scoped it off for `scripts/**/*.mjs`, matching how source
files already treat it.
Verified on Linux x64 (Electron 39.8.10, electron-builder 26.16.1):
- npm run typecheck, npm run build, npm run build:unpack all pass
- npm run smoke:packaged exits 0 with all 8 checks passing, and the database,
migrations and sqlite-vec queries all run inside the temp profile
- the gate genuinely fails: hiding the unpacked vec0.so makes it exit 1 with
`cannot open shared object file`, and restoring it passes again
- eslint is back to exactly its previous baseline (6 pre-existing renderer
react-hooks errors, 156 warnings)
Not wired into CI here - release.yml builds installers per platform on tags, and
a PR-gating workflow is a separate decision. macOS arm64 and Windows x64 remain
unverified until CI runs on this branch.
…PR gate
Two changes, both closing the gap between "the package built" and "the app works".
1. Document import is now part of the smoke test.
test/fixtures/{sample.pdf,sample.docx,sample.html} each contain "KnowNote Import
Test", and the checks run the real loaders against them:
PDF -> FileParserService.parseFile -> PdfLoader (pdfjs-dist)
DOCX -> FileParserService.parseFile -> DocxLoader (mammoth)
HTML -> WebLoader.loadFromBuffer (jsdom + Readability)
asserting the marker survives extraction. This is the layer that already broke
once in a packaged build - the DOMMatrix crash came from pdfjs - and it is far
cheaper than a UI end-to-end test while still catching a broken upgrade of
pdfjs-dist, mammoth, jsdom or @napi-rs/canvas.
The HTML check calls WebLoader directly rather than going through
FileParserService: the latter dispatches by extension, and a local .html path
reaches WebLoader.loadFromPath(), which is unimplemented by design because
WebLoader consumes fetched HTML through loadFromBuffer. (That also means importing
a local .html file through the normal document flow cannot work today; noting it
rather than changing it here.)
The fixture directory arrives as --smoke-fixtures=<dir>, and a missing directory
fails the run instead of skipping the checks: a gate that silently drops its most
relevant checks is worse than no gate.
electron-builder.yml now excludes test/ and scripts/ from the package. Verified
against the built asar: 0 files from either directory.
2. A pull-request gate (.github/workflows/verify.yml).
release.yml is tag-triggered, so nothing was validating this branch or any future
PR. The workflow runs npm ci -> typecheck -> build:unpack -> smoke:packaged on
ubuntu-latest, macos-latest and windows-latest. All three matter because the
native addons are arch-specific: better_sqlite3.node, vec0.so/.dylib/.dll and
skia.*.node.
- Linux needs xvfb-run because GitHub's runners are headless.
- The launcher disables the Chromium sandbox when running as root or under CI:
it needs a setuid helper or user namespaces, and neither is dependable on CI
runners. The smoke test only loads our own code.
- CSC_IDENTITY_AUTO_DISCOVERY=false stops electron-builder looking for signing
material CI does not have.
- lint is deliberately excluded: the 6 pre-existing renderer react-hooks errors
are unrelated to build correctness and would make the gate red immediately.
Verified locally on Linux x64: typecheck, build:unpack and smoke:packaged exit 0
with all 11 checks passing (the previous 8 plus PDF, DOCX and HTML import), and
eslint stays at its baseline of 6 errors / 156 warnings. The macOS and Windows
results are what this new workflow is for.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The packaged application could not start, and nothing in the pipeline could tell us:
typecheck,buildandbuild:unpackall passed on artifacts that crashed on theuser's machine.
This was a layered failure:
collector silently dropped transitive production dependencies. v1.2.1 shipped an
app.asar whose AI paths died with
Cannot find module 'eventsource-parser/stream',and
jsdom -> whatwg-url -> tr46,http-proxy-agent -> debug -> ms,util-deprecate,wrappyhad been hand-promoted intodependenciesas a workaround.725b4a3bundled every bundleablemain-process package by moving it to
devDependencies. That fixed the missingmodules but broke startup twice over, because two packages cannot be inlined safely:
anki-apkg-export- rollup hoisted its dead webpack-onlyrequire('script-loader!sql.js')to the top of the bundle;pdfjs-dist- its module-scopenew DOMMatrix()could not polyfill from@napi-rs/canvas, which was never shipped either.Fixing the first crash only exposed the second, because the bundle aborted before
evaluation ever reached it.
The dependency boundary this restores
dependenciesis only for packages rollup genuinely cannot inline:better-sqlite3,sqlite-vecasarUnpackkeeps them loadablejsdomcssstyle@mixmark-io/dominoturndownrequires it while its module body runsanki-apkg-exportpdfjs-distcreateRequire(import.meta.url)+ the optional@napi-rs/canvasEverything else is bundled, and
allowMissingDependencies: falsenow turns any futurecollector miss into a build failure instead of a runtime
MODULE_NOT_FOUND.Commits
9f1a80e- replace pnpm with npm, and fail loudly on a missed dependencypnpm-lock.yaml->package-lock.json..npmrc: dropshamefully-hoist/node-linker(npm warned about both).packaged asar as ordinary transitives of
jsdom.electron-builder.yml: addallowMissingDependencies: false(needselectron-builder >= 26.16.0, hence the bump from
^26.4.0), and dropnpmRebuild: falseso native modules are rebuilt for the target Electron ABI andtarget arch during packaging (
build:mac --arm64).@types/dompurify(deprecated stub),@types/react-router-dom@5(v7 ships its own types), a duplicate
jsdomentry, and the unusedollama-ai-provider(code importsollama-ai-provider-v2).59b3dd6- externalizeanki-apkg-exportandpdfjs-distfor the main processanki-apkg-export: restoring the Node branch removes the hoistedscript-loader!sql.js.Its
sql.jsis 0.5.0's asm.js build, so no.wasmis involved.pdfjs-dist: added todependenciesso it resolves its own optional native peer.@napi-rs/canvas+ platform binary are now collected (12 files) and the.nodeisunpacked. Also corrected the
electron.vite.config.tscomment that claimed pdfjs"degrades gracefully without
@napi-rs/canvas" - it does not; the dependency isrequired at module scope regardless of whether the app renders.
0172c36- replace the community Qwen provider with@ai-sdk/alibabaqwen-ai-provider@0.1.1is an AI SDK v4 era provider (provider-utils@2, peerzod ^3) in an AI SDK 5 app. It only installed thanks to a hand-written npmoverridesentry, now deleted.@ai-sdk/alibabapublishes dist-tags per AI SDK major.latest(2.0.54) targets AI SDK 6+ and would silently drag the whole SDK to v6. Pinned to the
ai-v5tag,^0.0.31, whose dependency set (provider 2.0.5 / provider-utils 3.0.39)is exactly what
ai@5already resolves.11d162b- fixanki-apkg-exportCJS interopexports.default. With the package external, rollup emits a barerequire("anki-apkg-export")and does not unwrap it, so every export threwTypeError: AnkiExport is not a constructor. Latent since725b4a3, hidden behind themodule-load crash that preceded it. Found by the smoke test below.
a2b43dc- packaged-app smoke test gate--smoke-testruns the real packaged executable, exercises the paths that haveactually broken, and exits with a status code:
vec_version()answers;SQLiteVectorStore(the extension is used, not merely loaded);pdfjs-distloads (proves itsDOMMatrixpolyfill found@napi-rs/canvas);anki-apkg-exportwrites a real.apkg.npm run smoke:packageddrives it viascripts/smoke-packaged.mjs, which locates theunpacked binary per platform, propagates the exit code and enforces a timeout.
--user-data-dirat a throwaway temp directory, because startupopens and migrates a real SQLite database and a smoke test must never touch a
developer's own
~/.config/knownote.Verification
CI gate (
.github/workflows/verify.yml, added bya5da28f)npm ci->typecheck->build:unpack->smoke:packagedon three platforms.All green: run 35963026715.
dist/mac-arm64/KnowNote.appsqlite-vec-darwin-arm64/vec0.dylibdist/win-unpacked/knownote.exesqlite-vec-windows-x64/vec0.dlldist/linux-unpacked/knownotesqlite-vec-linux-x64/vec0.soEach platform ran the real packaged executable and passed all 11 checks, so
better-sqlite3,sqlite-vecand@napi-rs/canvas(via pdfjs'sDOMMatrixpolyfill) are confirmed to load per-architecture, which is exactly what a
Linux-only result could not tell us.
Other checks
electron-builder's collector reportspm=npmandallowMissingDependenciesdoes not trip.
handlers registered, shortcuts registered, no fatal errors.
test/andscripts/do not reach the package: 0 files from either in the asar.vec0.somakes itexit 1 with
cannot open shared object file.react-hookserrors, 156 warnings).
Not verified / follow-ups
specificationVersion: v2models for both chat and embeddings, and thedependency graph is now clean, but no request has been made against a real
DashScope key. Deliberately not a merge blocker: CI should not depend on a
live third-party service. Worth one manual
generateTextbefore release. Note@ai-sdk/alibabasends embeddings to DashScope's native endpoint rather thanthe OpenAI-compatible base URL used for chat.
^range toits latest satisfying release (
zod4.3.6 -> 4.6.5,ai5.0.123 -> 5.0.265,electron39.4.0 -> 39.8.10). Worth a look before the next tag;npm cipinsthese from here on.
.htmlfile through thenormal document flow cannot work -
FileParserServicedispatches by extensionand
WebLoader.loadFromPath()throws by design, sinceWebLoaderconsumesfetched HTML through
loadFromBuffer. The smoke test exercises the working(fetch/Readability) path.
TypeScript project-discovery issue, and any
jsdom->linkedom, Forge ormajor-version migrations.