From 2ad49c415c775a008c7f92b53dd7348cb4a8faae Mon Sep 17 00:00:00 2001 From: Pierre Date: Thu, 16 Jul 2026 16:05:04 +0200 Subject: [PATCH] fix: terminate Electron startup smoke reliably --- .../docs/developer-building-from-source.mdx | 4 +++- docs-web/developer/building-from-source.md | 4 +++- docs/deployment/electron-desktop.md | 4 ++-- src/electron/main.ts | 15 +++++++-------- src/electron/startup-smoke.ts | 8 ++++++++ tests/backend/electron-builder-config.test.ts | 2 +- tests/backend/electron/startup-smoke.test.ts | 17 +++++++++++++++-- 7 files changed, 39 insertions(+), 15 deletions(-) diff --git a/docs-web/content/docs/developer-building-from-source.mdx b/docs-web/content/docs/developer-building-from-source.mdx index c518b9c9b5..960b0a575a 100644 --- a/docs-web/content/docs/developer-building-from-source.mdx +++ b/docs-web/content/docs/developer-building-from-source.mdx @@ -169,7 +169,9 @@ Electron and npm package builds must include the `docs-web` runtime catalog. The Electron runtime dependency preparation runs a production-only pnpm 11 install with `--config.node-linker=hoisted` passed on the command line. A nested runtime `.npmrc` is not enough once pnpm discovers the enclosing workspace. Preparation rejects symbolic links, missing direct production packages, and failed MCP SDK/`zod` imports so Electron Builder cannot silently copy a broken peer-dependency layout. The workspace `allowBuilds` policy approves only `onnxruntime-node`; preparation sets `ONNXRUNTIME_NODE_INSTALL=skip` because the CPU bindings used by Code UX are bundled and the upstream Linux default fetches optional CUDA/TensorRT binaries from NuGet. This keeps desktop packaging deterministic without suppressing the dependency postinstall or pnpm's build-policy check. Keep that allowlist narrow and review any addition as release-executed code. The installed-Electron smoke uses each platform's native package and exits its readiness probe -immediately with code zero after the packaged backend and renderer are ready. On Windows it passes +through the Node process boundary immediately with code zero after the packaged backend and +renderer are ready and the readiness marker is durably written. This bypasses Electron/AppKit +termination paths that newer macOS runners can defer even after `app.exit()`. On Windows it passes the NSIS silent-install arguments verbatim so the required final `/D=` destination remains unquoted even when that destination contains spaces. On macOS it accepts the DMG's embedded MIT license through `hdiutil` stdin before mounting. Production Electron shutdowns still drain the embedded diff --git a/docs-web/developer/building-from-source.md b/docs-web/developer/building-from-source.md index 57d0fa5b9f..da8b194b37 100644 --- a/docs-web/developer/building-from-source.md +++ b/docs-web/developer/building-from-source.md @@ -169,7 +169,9 @@ Electron and npm package builds must include the `docs-web` runtime catalog. The Electron runtime dependency preparation runs a production-only pnpm 11 install with `--config.node-linker=hoisted` passed on the command line. A nested runtime `.npmrc` is not enough once pnpm discovers the enclosing workspace. Preparation rejects symbolic links, missing direct production packages, and failed MCP SDK/`zod` imports so Electron Builder cannot silently copy a broken peer-dependency layout. The workspace `allowBuilds` policy approves only `onnxruntime-node`; preparation sets `ONNXRUNTIME_NODE_INSTALL=skip` because the CPU bindings used by Code UX are bundled and the upstream Linux default fetches optional CUDA/TensorRT binaries from NuGet. This keeps desktop packaging deterministic without suppressing the dependency postinstall or pnpm's build-policy check. Keep that allowlist narrow and review any addition as release-executed code. The installed-Electron smoke uses each platform's native package and exits its readiness probe -immediately with code zero after the packaged backend and renderer are ready. On Windows it passes +through the Node process boundary immediately with code zero after the packaged backend and +renderer are ready and the readiness marker is durably written. This bypasses Electron/AppKit +termination paths that newer macOS runners can defer even after `app.exit()`. On Windows it passes the NSIS silent-install arguments verbatim so the required final `/D=` destination remains unquoted even when that destination contains spaces. On macOS it accepts the DMG's embedded MIT license through `hdiutil` stdin before mounting. Production Electron shutdowns still drain the embedded diff --git a/docs/deployment/electron-desktop.md b/docs/deployment/electron-desktop.md index 3dc75e5bfa..42ea9f7eb1 100644 --- a/docs/deployment/electron-desktop.md +++ b/docs/deployment/electron-desktop.md @@ -67,7 +67,7 @@ macOS DMG builds include the MIT license resource through `build/license_en.txt` - `pnpm run electron:dist:win`: build Windows targets. - `pnpm run electron:benchmark:runtime`: launch Electron with an isolated temporary user profile, navigate dashboard routes, probe backend endpoints, and write route/API/renderer/runtime metrics under `.cache/electron-runtime-benchmark/`. - `pnpm run electron:benchmark:win`: build Windows installers with `normal` and `store` compression and write timing/size data to `release/electron-benchmark/summary.json`. -- `pnpm run electron:smoke-installed`: select the native package in `release/electron/` whose artifact name matches the current `package.json` version, install it, start that installed app with an isolated profile, wait until its backend and renderer are ready, and require a deterministic zero-code probe exit. Linux requires passwordless `sudo` and `xvfb-run`, as provided by the release runners. Windows launches the silent NSIS installer with verbatim arguments because NSIS requires its final `/D=` destination segment to remain unquoted even when the path contains spaces. macOS supplies affirmative input to `hdiutil` so the CI-owned smoke can mount a DMG containing the embedded MIT license without an interactive terminal. +- `pnpm run electron:smoke-installed`: select the native package in `release/electron/` whose artifact name matches the current `package.json` version, install it, start that installed app with an isolated profile, wait until its backend and renderer are ready, and require a deterministic zero-code probe exit. After the atomic readiness marker is durably written, the isolated probe exits through the Node process boundary instead of Electron/AppKit shutdown, which newer macOS runners can defer even after `app.exit()`. Linux requires passwordless `sudo` and `xvfb-run`, as provided by the release runners. Windows launches the silent NSIS installer with verbatim arguments because NSIS requires its final `/D=` destination segment to remain unquoted even when the path contains spaces. macOS supplies affirmative input to `hdiutil` so the CI-owned smoke can mount a DMG containing the embedded MIT license without an interactive terminal. - `pnpm run electron:install-deps`: rebuild native app dependencies for Electron. The release output is written to `release/electron/`. @@ -129,7 +129,7 @@ Use `.github/workflows/release.yml` for published desktop releases. It is the la The no-secret release-candidate package lane is part of `.github/workflows/ci.yml`, named `Code UX CI Pipeline`. It runs for `main` validation and manual dispatches after package smoke, keeping the full desktop package proof out of the routine `dev` lane. -The `10 Release Candidate / desktop package` matrix starts as soon as the package smoke job passes, so desktop packaging can run beside the E2E and orchestration matrices instead of waiting for them to finish. It downloads the shared `codeux-build-linux` artifact, installs the cached Electron binary, rebuilds Electron native dependencies, prepares runtime assets, and runs Electron Builder directly with `--linux`, `--mac`, or `--win` plus `--publish never`. After compilation, every native runner installs its candidate—the Linux `.deb`, Windows NSIS `.exe`, or macOS app copied from the `.dmg`—and starts that installed copy with an isolated home and dashboard port. The Windows smoke preserves NSIS's required unquoted, final `/D=` destination segment when spawning the installer, and the macOS smoke accepts the DMG's embedded MIT license through `hdiutil` stdin before mounting. Success requires the packaged backend to start, the dashboard renderer to finish loading, an atomic readiness marker to be written, and the smoke process to exit immediately with code zero. The dedicated probe exit deliberately bypasses the normal AppKit quit cycle so a macOS runner cannot defer the completed startup check until another native termination event arrives. Production shutdowns still cancel the initial quit request while the embedded server drains, then exit directly with the resulting process code instead of starting a second macOS quit cycle. The package smoke job that precedes it runs `node scripts/verify-release-install.mjs` with `CODE_UX_SKIP_RELEASE_INSTALL_BUILD=1`, so the npm tarball install check uses the same compiled artifact instead of rebuilding. +The `10 Release Candidate / desktop package` matrix starts as soon as the package smoke job passes, so desktop packaging can run beside the E2E and orchestration matrices instead of waiting for them to finish. It downloads the shared `codeux-build-linux` artifact, installs the cached Electron binary, rebuilds Electron native dependencies, prepares runtime assets, and runs Electron Builder directly with `--linux`, `--mac`, or `--win` plus `--publish never`. After compilation, every native runner installs its candidate—the Linux `.deb`, Windows NSIS `.exe`, or macOS app copied from the `.dmg`—and starts that installed copy with an isolated home and dashboard port. The Windows smoke preserves NSIS's required unquoted, final `/D=` destination segment when spawning the installer, and the macOS smoke accepts the DMG's embedded MIT license through `hdiutil` stdin before mounting. Success requires the packaged backend to start, the dashboard renderer to finish loading, an atomic readiness marker to be written, and the smoke process to exit immediately with code zero. The dedicated probe exits through the Node process boundary only after the marker rename completes, bypassing Electron/AppKit termination paths that newer macOS runners can defer even after `app.exit()`. Production shutdowns still cancel the initial quit request while the embedded server drains, then exit directly with the resulting process code instead of starting a second macOS quit cycle. The package smoke job that precedes it runs `node scripts/verify-release-install.mjs` with `CODE_UX_SKIP_RELEASE_INSTALL_BUILD=1`, so the npm tarball install check uses the same compiled artifact instead of rebuilding. Release-candidate packaging sets `CSC_IDENTITY_AUTO_DISCOVERY=false` for unsigned Electron packaging and passes `--publish never` to Electron Builder. It does not require provider API keys, npm publishing credentials, Docker credentials, GitHub Release events, or real project state. When Electron output exists, the workflow uploads files from `release/electron/` as workflow artifacts only; it does not publish to npm or attach files to a GitHub Release. diff --git a/src/electron/main.ts b/src/electron/main.ts index e71735aea7..9bd5d97c88 100644 --- a/src/electron/main.ts +++ b/src/electron/main.ts @@ -18,7 +18,7 @@ import { createDebouncedSaver, loadWindowState, saveWindowState } from "./window import { ElectronCredentialKeyPersistence } from "./credential-key-persistence.js"; import { ElectronSafeStorageKeyProvider } from "../infrastructure/security/electron-safe-storage-key-provider.js"; import { setProcessCredentialKeyProvider } from "../services/credentials/key-provider-registry.js"; -import { writeElectronStartupSmoke } from "./startup-smoke.js"; +import { exitElectronStartupSmoke, writeElectronStartupSmoke } from "./startup-smoke.js"; const __filename = fileURLToPath(import.meta.url); const __dirname = path.dirname(__filename); @@ -306,13 +306,12 @@ function createMainWindow(url: string): BrowserWindow { rendererUrl: window.webContents.getURL(), }).then(() => { if (process.env.CODE_UX_ELECTRON_STARTUP_SMOKE_EXIT === "1") { - // This is an install/start probe, not a user-driven shutdown. On macOS, requesting the - // normal AppKit quit cycle from the did-finish-load promise can be deferred until another - // native termination event arrives, leaving an otherwise healthy RC alive on the runner. - // Electron documents app.exit() as the immediate, deterministic exit path; the regular - // before-quit handler below remains responsible for draining the embedded server during - // every production shutdown. - app.exit(0); + // This is an install/start probe, not a user-driven shutdown. Newer macOS runners can + // defer both the AppKit quit cycle and Electron's app.exit() after renderer readiness. + // The marker is already durably renamed at this point, so end only this isolated probe + // through the Node process boundary. Production shutdowns still drain the embedded + // server through the before-quit handler below. + exitElectronStartupSmoke(); } }).catch((error: unknown) => { process.exitCode = 1; diff --git a/src/electron/startup-smoke.ts b/src/electron/startup-smoke.ts index 2f193aa18e..55b214a2ee 100644 --- a/src/electron/startup-smoke.ts +++ b/src/electron/startup-smoke.ts @@ -24,6 +24,14 @@ export interface ElectronStartupSmokeInput { now?: () => Date; } +export type ElectronStartupSmokeExit = (code: number) => never; + +export function exitElectronStartupSmoke( + exitProcess: ElectronStartupSmokeExit = (code) => process.exit(code), +): never { + return exitProcess(0); +} + export async function writeElectronStartupSmoke( markerPath: string, input: ElectronStartupSmokeInput, diff --git a/tests/backend/electron-builder-config.test.ts b/tests/backend/electron-builder-config.test.ts index c4b1f2260f..b1bcfa9441 100644 --- a/tests/backend/electron-builder-config.test.ts +++ b/tests/backend/electron-builder-config.test.ts @@ -143,7 +143,7 @@ describe("electron-builder packaged defaults", () => { expect(mainProcessSource).toContain('window.webContents.once("did-finish-load"'); expect(mainProcessSource).toContain("writeElectronStartupSmoke"); expect(mainProcessSource).toContain('CODE_UX_ELECTRON_STARTUP_SMOKE_EXIT === "1"'); - expect(mainProcessSource).toContain("app.exit(0)"); + expect(mainProcessSource).toContain("exitElectronStartupSmoke()"); expect(mainProcessSource).toContain('app.on("before-quit"'); expect(mainProcessSource).toContain('app.exit(typeof process.exitCode === "number" ? process.exitCode : 0)'); }); diff --git a/tests/backend/electron/startup-smoke.test.ts b/tests/backend/electron/startup-smoke.test.ts index 8d8cb048d2..53fe73e57e 100644 --- a/tests/backend/electron/startup-smoke.test.ts +++ b/tests/backend/electron/startup-smoke.test.ts @@ -1,8 +1,11 @@ import { mkdtemp, readFile, rm } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; -import { writeElectronStartupSmoke } from "../../../src/electron/startup-smoke.js"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + exitElectronStartupSmoke, + writeElectronStartupSmoke, +} from "../../../src/electron/startup-smoke.js"; const temporaryDirectories: string[] = []; @@ -49,4 +52,14 @@ describe("Electron startup smoke marker", () => { rendererUrl: "http://127.0.0.1:4567/", })).rejects.toThrow("must be absolute"); }); + + it("terminates the isolated readiness probe with code zero", () => { + const exitProcess = vi.fn((_code: number): never => { + throw new Error("process exited"); + }); + + expect(() => exitElectronStartupSmoke(exitProcess)).toThrow("process exited"); + expect(exitProcess).toHaveBeenCalledOnce(); + expect(exitProcess).toHaveBeenCalledWith(0); + }); });