From 58b403ded708fff3987c96bdf952ad52da4ed6ee Mon Sep 17 00:00:00 2001 From: "Amagi:DDmxh" Date: Thu, 16 Jul 2026 03:47:53 +0800 Subject: [PATCH] fix(build): make native ABI switching interruption-safe --- CONTRIBUTING.md | 4 +- README.md | 3 +- docs/engineering-optimization-roadmap.md | 8 +-- package.json | 3 +- scripts/rebuild-better-sqlite3-node.mjs | 21 +++++-- scripts/with-electron-native.mjs | 73 +++++++++++++++++++----- 6 files changed, 82 insertions(+), 30 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 95e5e80e..9fa0e0fc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -15,10 +15,10 @@ pnpm dev `better-sqlite3` is used from both Node-based tests and the Electron app, and the workspace keeps a single copy that must match the runtime in use. The scripts manage the switching for you: -- `pnpm test` (and `pnpm check`) rebuild for Node first via the root `pretest`. +- `pnpm test` (and `pnpm check`) verifies the Node ABI via the root `pretest` and rebuilds only when it does not match. - `pnpm dev`, `pnpm test:e2e`, and the `package:*` scripts run through `scripts/with-electron-native.mjs`, which flips the binary to the Electron ABI for the wrapped command and restores the Node ABI afterwards. -An interrupted run (Ctrl-C skips the restore) can leave the wrong ABI behind. If you hit a `NODE_MODULE_VERSION` mismatch, rebuild manually for the runtime you are about to use: +The wrapper forwards interruption signals to the active child and restores the Node ABI before it exits. If a hard kill or machine shutdown interrupts that restoration and you hit a `NODE_MODULE_VERSION` mismatch, rebuild manually for the runtime you are about to use: ```bash pnpm run rebuild:native:node # Node / vitest / core tests diff --git a/README.md b/README.md index d34b027f..79de63ee 100644 --- a/README.md +++ b/README.md @@ -54,12 +54,11 @@ packages/ ```bash pnpm install -pnpm exec electron-rebuild -f -w better-sqlite3 # rebuild native modules for Electron pnpm dev # starts app + landing in dev mode pnpm test # runs all tests ``` -> **Note:** The `electron-rebuild` step is required whenever you run `pnpm install` or switch Node.js versions. Without it, the Electron app will crash at launch with a `NODE_MODULE_VERSION` mismatch error from `better-sqlite3`. +> **Note:** The dev, e2e, and packaging commands rebuild `better-sqlite3` for Electron and restore its Node ABI when they exit. `pnpm test` checks the Node ABI and rebuilds only when needed. If you switch between **Node-side tests** and **Electron app/e2e runs**, rebuild `better-sqlite3` for the matching runtime: diff --git a/docs/engineering-optimization-roadmap.md b/docs/engineering-optimization-roadmap.md index 59e1a6bb..48509058 100644 --- a/docs/engineering-optimization-roadmap.md +++ b/docs/engineering-optimization-roadmap.md @@ -38,8 +38,6 @@ ## Known follow-ups -Tracked in the PR descriptions: #415 — Ctrl-C skips the wrapper's ABI -restore; unconditional `pretest` rebuild costs ~1 min per run. #416 — -diacritic-folded FTS hits can render an unhighlighted snippet. #417 — -truncated-title mask residue; a re-mask migration for pre-#417 purges; -bulk-purge throughput at #344 scale. +Tracked in the PR descriptions: #416 — diacritic-folded FTS hits can render +an unhighlighted snippet. #417 — truncated-title mask residue; a re-mask +migration for pre-#417 purges; bulk-purge throughput at #344 scale. diff --git a/package.json b/package.json index 0c1a938b..e7d9ad4b 100644 --- a/package.json +++ b/package.json @@ -10,11 +10,12 @@ "package:linux": "turbo build --filter=@spool/app... && pnpm --filter @spool/app package:linux", "dev": "turbo dev", "typecheck": "turbo typecheck", - "pretest": "pnpm run rebuild:native:node", + "pretest": "pnpm run ensure:native:node", "test": "turbo test --concurrency=1", "test:core": "pnpm --filter @spool-lab/core test", "test:e2e": "pnpm --filter @spool/app test:e2e", "test:share-backend": "pnpm --filter @spool/share-backend test", + "ensure:native:node": "node scripts/rebuild-better-sqlite3-node.mjs --if-needed", "rebuild:native:node": "pnpm --filter @spool-lab/core run rebuild:native:node", "rebuild:native:electron": "pnpm --filter @spool/app run rebuild:native:electron", "lint": "oxlint --type-aware .", diff --git a/scripts/rebuild-better-sqlite3-node.mjs b/scripts/rebuild-better-sqlite3-node.mjs index d322369a..a856b8bd 100644 --- a/scripts/rebuild-better-sqlite3-node.mjs +++ b/scripts/rebuild-better-sqlite3-node.mjs @@ -10,17 +10,28 @@ const requireFromCore = createRequire(join(corePackageDir, 'package.json')) const packageJsonPath = requireFromCore.resolve('better-sqlite3/package.json') const packageDir = dirname(packageJsonPath) const npmBin = process.platform === 'win32' ? 'npm.cmd' : 'npm' +const ifNeeded = process.argv.includes('--if-needed') + +if (ifNeeded) { + try { + const Database = requireFromCore('better-sqlite3') + const db = new Database(':memory:') + db.prepare('SELECT 1').get() + db.close() + console.log('[rebuild-better-sqlite3-node] Node ABI already matches; skipping rebuild') + process.exit(0) + } catch (error) { + const reason = error instanceof Error ? error.message : String(error) + console.log(`[rebuild-better-sqlite3-node] Node ABI check failed; rebuilding (${reason})`) + } +} console.log(`[rebuild-better-sqlite3-node] rebuilding in ${packageDir}`) const result = spawnSync(npmBin, ['run', 'build-release'], { cwd: packageDir, stdio: 'inherit', - env: { - ...process.env, - npm_config_runtime: 'node', - npm_config_target: process.versions.node, - }, + env: process.env, }) if (result.status !== 0) { diff --git a/scripts/with-electron-native.mjs b/scripts/with-electron-native.mjs index bc8f46ff..051b800f 100644 --- a/scripts/with-electron-native.mjs +++ b/scripts/with-electron-native.mjs @@ -1,4 +1,5 @@ -import { spawnSync } from 'node:child_process' +import { spawn } from 'node:child_process' +import { constants as osConstants } from 'node:os' import { dirname, join } from 'node:path' import { fileURLToPath } from 'node:url' @@ -13,17 +14,48 @@ if (!command) { process.exit(2) } +let activeChild +let receivedSignal + +const signalHandlers = new Map( + ['SIGHUP', 'SIGINT', 'SIGTERM'].map((signal) => [ + signal, + () => { + receivedSignal ??= signal + if (activeChild?.exitCode === null && activeChild.signalCode === null) { + activeChild.kill(signal) + } + }, + ]), +) + +for (const [signal, handler] of signalHandlers) { + process.on(signal, handler) +} + function run(program, programArgs, cwd = repoRoot) { - return spawnSync(program, programArgs, { - cwd, - env: process.env, - stdio: 'inherit', + return new Promise((resolve) => { + const child = spawn(program, programArgs, { + cwd, + env: process.env, + stdio: 'inherit', + }) + activeChild = child + + child.once('error', (error) => { + console.error(`[with-electron-native] failed to start ${program}:`, error) + }) + child.once('close', (status, signal) => { + if (activeChild === child) activeChild = undefined + resolve({ status, signal }) + }) }) } -let commandStatus = 1 +let commandResult = { status: 1, signal: null } +let restoreResult try { - const electronRebuild = run(pnpmBin, [ + const electronRebuild = await run(pnpmBin, [ '--filter', '@spool/app', 'exec', @@ -33,18 +65,29 @@ try { 'better-sqlite3', ]) - if (electronRebuild.status === 0) { - const result = run(command, args, process.cwd()) - commandStatus = result.status ?? 1 + if (electronRebuild.status === 0 && electronRebuild.signal === null) { + commandResult = await run(command, args, process.cwd()) } else { - commandStatus = electronRebuild.status ?? 1 + commandResult = electronRebuild } } finally { - const restore = run(process.execPath, [join(scriptDir, 'rebuild-better-sqlite3-node.mjs')]) - if (restore.status !== 0) { + restoreResult = await run(process.execPath, [join(scriptDir, 'rebuild-better-sqlite3-node.mjs')]) + for (const [signal, handler] of signalHandlers) { + process.off(signal, handler) + } + + if (restoreResult.status !== 0 || restoreResult.signal !== null) { console.error('[with-electron-native] failed to restore the Node ABI') - commandStatus = restore.status ?? 1 } } -process.exit(commandStatus) +const restoreFailed = restoreResult.status !== 0 || restoreResult.signal !== null +const exitSignal = receivedSignal ?? commandResult.signal ?? restoreResult.signal +const signalNumber = exitSignal ? osConstants.signals[exitSignal] : undefined +const exitStatus = restoreFailed + ? (restoreResult.status ?? (signalNumber ? 128 + signalNumber : 1)) + : signalNumber + ? 128 + signalNumber + : (commandResult.status ?? 1) + +process.exit(exitStatus)