diff --git a/changelog.d/fixes/7592-windows-packaged-smoke.md b/changelog.d/fixes/7592-windows-packaged-smoke.md new file mode 100644 index 0000000000..5ffffdd22c --- /dev/null +++ b/changelog.d/fixes/7592-windows-packaged-smoke.md @@ -0,0 +1 @@ +- **fix(electron):** the packaged Windows desktop build now passes the #7592 cold-restart smoke end to end. Five defects were found and fixed on the way: (1) optional-pack staging failed on any machine whose `tar` is GNU tar — it reads the drive letter in an absolute `-f C:\...` archive path as a remote rsh host (`Cannot connect to C:`), so staging now passes a bare filename with `cwd` at the tarball directory and surfaces tar stderr on failure; (2) the Electron `files` allowlist was missing `lib/loginHeaderCapture.js`, so the packaged main process crashed at startup with `Cannot find module './lib/loginHeaderCapture'`; (3) electron-builder ≥26 injects an `!**/node_modules/**` ignore into every extraResources pattern list that no positive filter can override, which silently dropped the staged runtime `node_modules` (including the better-sqlite3 N-API prebuild) from `resources/app` — a fresh v3.8.50 desktop build would have shipped with no native SQLite driver and reproduced the #7592 sql.js fallback on every machine; an `afterPack` hook now restores the staged `node_modules` after packing; (4) the packaged-app smoke harness redirected `USERPROFILE` into its temp DATA_DIR, but Electron resolves the Roaming profile from `%USERPROFILE%\AppData\Roaming\` (USERPROFILE takes precedence over the APPDATA env var) and the path service throws instead of creating it, so `requestSingleInstanceLock()` returned false and the app exited(0) silently before `app.whenReady()` — the harness now pre-creates the derived tree, and `ensureSmokeEnvDirs` is exported and unit-tested; (5) the #7592 driver assertion parsed a `[DB] Driver: ...` line that the server's primary DB path never printed (only the unused `openDatabaseAsync()` did) — `getDbInstance()` now emits the same line on open so the guard can actually assert the native driver. Also: the smoke env-allowlist unit test hardcoded POSIX paths and could only pass on Linux/macOS; expectations are now host-agnostic, with new regression tests for the USERPROFILE-derived tree and `tarPack` under absolute Windows-style paths diff --git a/electron/package.json b/electron/package.json index 0eb4fb3df4..1deee45f79 100644 --- a/electron/package.json +++ b/electron/package.json @@ -45,6 +45,7 @@ "copyright": "Copyright © 2025 OmniRoute", "buildDependenciesFromSource": false, "npmRebuild": false, + "afterPack": "../scripts/build/afterpack-copy-node-modules.mjs", "directories": { "output": "dist-electron", "buildResources": "assets" @@ -58,6 +59,7 @@ "main.js", "preload.js", "loginManager.js", + "lib/loginHeaderCapture.js", "processTree.js", "sqlite-inspection.js", "remoteServerPromptPreload.js", diff --git a/scripts/build/afterpack-copy-node-modules.mjs b/scripts/build/afterpack-copy-node-modules.mjs new file mode 100644 index 0000000000..8fbc817b69 --- /dev/null +++ b/scripts/build/afterpack-copy-node-modules.mjs @@ -0,0 +1,34 @@ +import { cpSync, existsSync, readdirSync, rmSync } from "node:fs"; +import { join } from "node:path"; + +// electron-builder >=26 injects an "!**/node_modules/**" ignore into every +// extraResources/extraFiles pattern list (app-builder-lib/out/fileMatcher.js), +// and that ignore cannot be overridden by any later positive filter pattern — +// verified empirically with a minimal fixture on 26.15.3. The standalone +// server resolves better-sqlite3 (and other runtime deps) from the *primary* +// node_modules at resources/app/node_modules (see +// prepare-electron-standalone.mjs: "Verify better-sqlite3 Node-API prebuilds in +// the primary node_modules"), so without this hook the packaged desktop app silently loses its native +// SQLite driver and falls back to sql.js — the exact regression guarded by +// issue #7592's cold-restart smoke check. +export default async function afterPack(context) { + const stagingNodeModules = join( + context.packager.projectDir, + "..", + ".build", + "electron-standalone", + "node_modules" + ); + const destNodeModules = join(context.appOutDir, "resources", "app", "node_modules"); + + if (!existsSync(stagingNodeModules)) { + console.warn(`[afterpack] no staged node_modules at ${stagingNodeModules} — skipping restore`); + return; + } + + rmSync(destNodeModules, { recursive: true, force: true }); + cpSync(stagingNodeModules, destNodeModules, { recursive: true }); + console.log( + `[afterpack] restored ${readdirSync(destNodeModules).length} runtime module(s) into resources/app/node_modules` + ); +} diff --git a/scripts/build/optionalPackStaging.mjs b/scripts/build/optionalPackStaging.mjs index f291aab44f..bb0f04c231 100644 --- a/scripts/build/optionalPackStaging.mjs +++ b/scripts/build/optionalPackStaging.mjs @@ -89,16 +89,20 @@ function moveTree(src, dest) { return { removedFiles: files, removedBytes: bytes }; } -function tarPack(packOutDir, tarballPath) { +export function tarPack(packOutDir, tarballPath) { // bsdtar ships with macOS, Linux images, and Windows runners (System32\tar.exe). + // GNU tar (common in Git-for-Windows environments) treats `C:\...` in `-f` as a + // remote rsh target ("Cannot connect to C:"), so always pass a bare filename + // and point cwd at the tarball directory instead. const result = spawnSync( process.platform === "win32" ? "tar.exe" : "tar", - ["-czf", tarballPath, "-C", packOutDir, "node_modules"], - { stdio: "pipe" } + ["-czf", path.basename(tarballPath), "-C", packOutDir, "node_modules"], + { stdio: "pipe", cwd: path.dirname(tarballPath) } ); if (result.status !== 0) { throw new Error( - `optional-pack tar failed for ${path.basename(tarballPath)} (exit ${result.status})` + `optional-pack tar failed for ${path.basename(tarballPath)} (exit ${result.status})` + + `: ${(result.stderr || result.stdout || "").toString().slice(-600)}` ); } } diff --git a/scripts/dev/smoke-electron-packaged.mjs b/scripts/dev/smoke-electron-packaged.mjs index 72afc2f4a7..1dfb4191c8 100644 --- a/scripts/dev/smoke-electron-packaged.mjs +++ b/scripts/dev/smoke-electron-packaged.mjs @@ -360,7 +360,7 @@ function isInsideDir(parentDir, candidateDir) { return candidate === parent || candidate.startsWith(parent + sep); } -async function ensureSmokeEnvDirs(smokeEnv, dataDir) { +export async function ensureSmokeEnvDirs(smokeEnv, dataDir) { const dirNames = [ "DATA_DIR", "HOME", @@ -388,6 +388,16 @@ async function ensureSmokeEnvDirs(smokeEnv, dataDir) { dirs.push(join(smokeEnv.APPDATA, subdir)); } } + // Electron resolves the Roaming profile from %USERPROFILE%\AppData\Roaming + // (USERPROFILE takes precedence over the APPDATA env var) and the path + // service throws — rather than creates — when that directory is missing, + // which makes requestSingleInstanceLock() return false and the app exit(0) + // before app.whenReady(). Pre-create the derived tree as well. + if (platform() === "win32" && smokeEnv.USERPROFILE) { + for (const subdir of ["omniroute-desktop", "OmniRoute", "omniroute"]) { + dirs.push(join(smokeEnv.USERPROFILE, "AppData", "Roaming", subdir)); + } + } await Promise.all(dirs.map((dir) => mkdir(dir, { recursive: true }))); } @@ -506,7 +516,14 @@ async function waitForReady({ logs, smokeUrl, timeoutMs, settleMs, exitState }) * by the single-launch path and the cold-restart (two-launch) path so both * exercise identical spawn/readiness/shutdown behavior. */ -async function launchAndCollectLogs({ appExecutable, smokeUrl, dataDir, timeoutMs, settleMs, streamLogs }) { +async function launchAndCollectLogs({ + appExecutable, + smokeUrl, + dataDir, + timeoutMs, + settleMs, + streamLogs, +}) { const smokeEnv = buildSmokeEnv({ dataDir }); await assertPortIsFree(smokeUrl); await ensureSmokeEnvDirs(smokeEnv, dataDir); @@ -568,7 +585,14 @@ async function main() { !process.env.ELECTRON_SMOKE_DATA_DIR && process.env.ELECTRON_SMOKE_KEEP_DATA !== "1"; try { - await launchAndCollectLogs({ appExecutable, smokeUrl, dataDir, timeoutMs, settleMs, streamLogs }); + await launchAndCollectLogs({ + appExecutable, + smokeUrl, + dataDir, + timeoutMs, + settleMs, + streamLogs, + }); if (!coldRestart) return; diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index cddde9dc87..48d63f8625 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -1284,6 +1284,11 @@ export function getDbInstance(): SqliteDatabase { } const db = openSqliteDatabase(sqliteFile); + // Emit the same "[DB] Driver: ..." line openDatabaseAsync() prints so the + // packaged-app smoke guard (#7592) can assert the native driver was + // selected on the server's primary DB path too, not only the backup-import + // route. + console.log(`[DB] Driver: ${db.driver} | file: ${sqliteFile}`); db.pragma("journal_mode = WAL"); // better-sqlite3 is synchronous, so a contended write parks the Node event loop for up to // busy_timeout ms (a 0-CPU freeze that stacks under load → /health stops responding). The diff --git a/tests/unit/electron-smoke-script.test.ts b/tests/unit/electron-smoke-script.test.ts index f7167859e1..fa24036fe5 100644 --- a/tests/unit/electron-smoke-script.test.ts +++ b/tests/unit/electron-smoke-script.test.ts @@ -1,22 +1,28 @@ import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import test from "node:test"; import { assertNativeDriverSelected, buildSmokeEnv, + ensureSmokeEnvDirs, FATAL_LOG_PATTERNS, LINUX_EXECUTABLE_NAMES, stopApp, } from "../../scripts/dev/smoke-electron-packaged.mjs"; +import { tarPack } from "../../scripts/build/optionalPackStaging.mjs"; test("electron smoke discovers the default Linux executable name", () => { assert.ok(LINUX_EXECUTABLE_NAMES.includes("omniroute-desktop")); }); test("electron smoke env allowlists runtime variables and drops secrets", () => { + const dataDir = path.join("/tmp", "omniroute-electron-smoke-test"); const env = buildSmokeEnv({ currentPlatform: "linux", - dataDir: "/tmp/omniroute-electron-smoke-test", + dataDir, parentEnv: { DISPLAY: ":99", GITHUB_TOKEN: "should-not-leak", @@ -25,17 +31,61 @@ test("electron smoke env allowlists runtime variables and drops secrets", () => }, }); - assert.equal(env.DATA_DIR, "/tmp/omniroute-electron-smoke-test"); + // Expected values are built with path.join so the assertions hold on every + // host platform: buildSmokeEnv() composes its redirected paths with join(), + // which yields backslashes on Windows even when currentPlatform is "linux". + assert.equal(env.DATA_DIR, dataDir); assert.equal(env.DISPLAY, ":99"); assert.equal(env.PATH, "/usr/bin"); - assert.equal(env.HOME, "/tmp/omniroute-electron-smoke-test/home"); - assert.equal(env.XDG_CONFIG_HOME, "/tmp/omniroute-electron-smoke-test/config"); + assert.equal(env.HOME, path.join(dataDir, "home")); + assert.equal(env.XDG_CONFIG_HOME, path.join(dataDir, "config")); assert.equal(env.ELECTRON_ENABLE_LOGGING, "1"); assert.equal(env.ELECTRON_ENABLE_STACK_DUMPING, "1"); assert.equal(env.GITHUB_TOKEN, undefined); assert.equal(env.SNYK_TOKEN, undefined); }); +test("electron smoke pre-creates the USERPROFILE-derived Roaming userData tree on Windows", async () => { + // #7592: Electron resolves userData from %USERPROFILE%\AppData\Roaming\ + // (USERPROFILE takes precedence over the APPDATA env var) and the path + // service throws — rather than creates — when the directory is missing, so + // requestSingleInstanceLock() returns false and the app exits(0) silently. + const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-smoke-env-test-")); + try { + const smokeEnv = buildSmokeEnv({ currentPlatform: "win32", dataDir }); + await ensureSmokeEnvDirs(smokeEnv, dataDir); + + for (const appName of ["omniroute-desktop", "OmniRoute", "omniroute"]) { + const derived = path.join(smokeEnv.USERPROFILE, "AppData", "Roaming", appName); + assert.ok(fs.existsSync(derived), `expected pre-created derived userData dir: ${derived}`); + const viaAppData = path.join(smokeEnv.APPDATA, appName); + assert.ok(fs.existsSync(viaAppData), `expected pre-created APPDATA dir: ${viaAppData}`); + } + } finally { + fs.rmSync(dataDir, { recursive: true, force: true }); + } +}); + +test("electron smoke tarPack handles absolute Windows-style tarball paths", () => { + // GNU tar treats `C:\...` in `-f` as a remote rsh target ("Cannot connect to + // C:"), which broke optional-pack staging on Windows. tarPack() must pass a + // bare filename with cwd at the tarball directory instead. + const staging = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-tar-pack-test-")); + try { + const nodeModules = path.join(staging, "pack", "node_modules"); + fs.mkdirSync(path.join(nodeModules, "fixture-pkg"), { recursive: true }); + fs.writeFileSync(path.join(nodeModules, "fixture-pkg", "index.js"), "module.exports = 1;"); + + const tarballPath = path.join(staging, "optional-pack-fixture.tar.gz"); + tarPack(path.join(staging, "pack"), tarballPath); + + assert.ok(fs.existsSync(tarballPath), "tarball should exist after tarPack"); + assert.ok(fs.statSync(tarballPath).size > 0, "tarball should not be empty"); + } finally { + fs.rmSync(staging, { recursive: true, force: true }); + } +}); + test("electron smoke treats Electron process errors as fatal startup logs", () => { const logs = [ "[Electron] Unhandled Rejection: Error: startup failed",