From 4febee9415e7f07709a08f3029bfcedab639a570 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Sat, 29 Aug 2026 15:43:00 -0300 Subject: [PATCH] =?UTF-8?q?fix(release):=20drop=20the=20build=5Fref=20inpu?= =?UTF-8?q?t=20=E2=80=94=20a=20dispatch=20builds=20the=20ref=20it=20is=20d?= =?UTF-8?q?ispatched=20on=20(#12032)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twin of #12022 on main: CodeQL flagged the same input-controlled checkout + npm cache pattern (cache-poisoning/poisonable-step) on main since it's the default branch. Checkouts go back to github.ref; dispatch still works via --ref (documented in the workflow's own on: contract). Also fixes the packaged-app smoke: it now waits on /api/monitoring/health (which touches the DB) instead of /login (which doesn't), so the smoke can actually distinguish "native driver selected" from "database never opened." electron-smoke-script.test.ts 9/9 (2 new cases). --- .github/workflows/electron-release.yml | 17 ++---- scripts/dev/smoke-electron-packaged.mjs | 69 ++++++++++++++++++++++-- tests/unit/electron-smoke-script.test.ts | 44 ++++++++++++++- 3 files changed, 113 insertions(+), 17 deletions(-) diff --git a/.github/workflows/electron-release.yml b/.github/workflows/electron-release.yml index c7e4d6d1ef..4d5a36903a 100644 --- a/.github/workflows/electron-release.yml +++ b/.github/workflows/electron-release.yml @@ -4,6 +4,10 @@ on: push: tags: - "v*" + # A dispatch builds the ref it is dispatched ON (`gh workflow run … --ref v3.8.50` rebuilds + # that tag; `--ref main` builds the repaired line). The ref is deliberately NOT an input: + # CodeQL flags an input-controlled checkout next to the npm cache on the default branch as + # cache poisoning (actions/cache-poisoning/poisonable-step), and `github.ref` is trusted. workflow_dispatch: inputs: version: @@ -15,11 +19,6 @@ on: required: false default: true type: boolean - build_ref: - description: "Git ref to BUILD from (default: the version tag). Set to a branch when the tag itself cannot build — e.g. a lockfile that was already broken when it was cut — and the assets must come from the repaired line" - required: false - default: "" - type: string # Least-privilege default: read-only at the top level; each job grants the writes it # needs (build/release upload assets, publish-npm forwards npm provenance / packages @@ -86,9 +85,6 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - # workflow_dispatch: build the tag being (re)built, not the dispatching branch. On a - # tag push this resolves to the same commit. - ref: ${{ inputs.build_ref || needs.validate.outputs.version }} - name: Setup Node uses: actions/setup-node@v7 with: @@ -174,9 +170,6 @@ jobs: - uses: actions/checkout@v7 with: persist-credentials: false - # workflow_dispatch: build the tag being (re)built, not the dispatching branch. On a - # tag push this resolves to the same commit. - ref: ${{ inputs.build_ref || needs.validate.outputs.version }} - name: Setup Node uses: actions/setup-node@v7 with: @@ -363,8 +356,6 @@ jobs: with: persist-credentials: false fetch-depth: 0 - # Source archives + SBOM come from the tag being released, not the dispatching branch. - ref: ${{ inputs.build_ref || needs.validate.outputs.version }} # `merge-multiple` is deliberately OFF. It resolves same-name collisions by ARRIVAL # ORDER, and the two macOS jobs each emit their own `latest-mac.yml` listing only their diff --git a/scripts/dev/smoke-electron-packaged.mjs b/scripts/dev/smoke-electron-packaged.mjs index 72afc2f4a7..688524bf7b 100644 --- a/scripts/dev/smoke-electron-packaged.mjs +++ b/scripts/dev/smoke-electron-packaged.mjs @@ -123,6 +123,48 @@ function discoverPackagedExecutable() { throw new Error(`Packaged Electron smoke check does not support ${platform()}.`); } +/** + * The packaged app opens SQLite lazily: `/login` (the readiness URL) never touches the + * database, so a smoke that only waits for readiness sees no `[DB]` line at all. After + * readiness the smoke requests a DB-backed endpoint and waits for evidence that the + * database opened. The primary open path does NOT print "[DB] Driver: ..." (only the + * recovery path and the sql.js fallback do), so the evidence is any `[DB]`/`[Migration]` + * startup line — and the #7592 guard below rejects the fallback's own line explicitly. + */ +export const DB_TOUCH_PATH = "/api/monitoring/health"; +export const DB_OPEN_EVIDENCE_PATTERN = + /\[DB\] (Driver: |SQLite database ready|Added [^\n]* column|Changing cache_size|cache_size changed)|\[Migration\] (Applied|Pre-migration backup)/; + +export async function waitForDatabaseOpen(getLogs, { timeoutMs = 15_000, pollMs = 250 } = {}) { + const startedAt = Date.now(); + while (Date.now() - startedAt < timeoutMs) { + const logs = getLogs(); + assertNoFatalLogs(logs); + if (DB_OPEN_EVIDENCE_PATTERN.test(logs)) return logs; + await sleep(pollMs); + } + throw new Error( + `Packaged Electron app logged no [DB]/[Migration] startup line within ${timeoutMs}ms of ` + + `touching ${DB_TOUCH_PATH} — the database never opened, so the SQLite driver cannot be verified.` + ); +} + +async function openDatabaseForSmoke({ logs, smokeUrl }) { + const touchUrl = new URL(DB_TOUCH_PATH, smokeUrl).toString(); + try { + const response = await fetchWithTimeout(touchUrl, 5_000); + console.log( + `[electron-smoke] touched ${touchUrl} (HTTP ${response.status}) to open the database` + ); + } catch (error) { + console.log( + `[electron-smoke] touching ${touchUrl} failed (${error instanceof Error ? error.message : String(error)}) — waiting for the database anyway` + ); + } + await waitForDatabaseOpen(() => logs.value); + console.log("[electron-smoke] database opened"); +} + async function fetchWithTimeout(url, timeoutMs) { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); @@ -450,8 +492,13 @@ export function assertNativeDriverSelected(logs) { ); } + // The primary open path prints no "[DB] Driver: ..." line at all (only the recovery path and + // the sql.js fallback do), so a database that demonstrably opened WITHOUT the fallback's own + // line is the native driver — that is exactly what #7592 guards. + if (DB_OPEN_EVIDENCE_PATTERN.test(logs)) return; + throw new Error( - "Packaged Electron app logs contain no '[DB] Driver: ...' line — cannot confirm which SQLite " + + "Packaged Electron app logs show no database activity at all — cannot confirm which SQLite " + "driver loaded." ); } @@ -506,7 +553,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); @@ -538,6 +592,8 @@ async function launchAndCollectLogs({ appExecutable, smokeUrl, dataDir, timeoutM try { await waitForReady({ logs, smokeUrl, timeoutMs, settleMs, exitState }); + // Outside waitForReady on purpose: a missing database is a verdict, not a readiness retry. + await openDatabaseForSmoke({ logs, smokeUrl }); return logs.value; } catch (error) { if (!streamLogs) { @@ -568,7 +624,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/tests/unit/electron-smoke-script.test.ts b/tests/unit/electron-smoke-script.test.ts index f7167859e1..c302c19aae 100644 --- a/tests/unit/electron-smoke-script.test.ts +++ b/tests/unit/electron-smoke-script.test.ts @@ -7,6 +7,8 @@ import { FATAL_LOG_PATTERNS, LINUX_EXECUTABLE_NAMES, stopApp, + waitForDatabaseOpen, + DB_TOUCH_PATH, } from "../../scripts/dev/smoke-electron-packaged.mjs"; test("electron smoke discovers the default Linux executable name", () => { @@ -94,6 +96,46 @@ test("electron smoke flags a cold-restart fallback to the sql.js WASM driver", ( test("electron smoke flags startup logs missing any driver selection line", () => { assert.throws( () => assertNativeDriverSelected("[electron] [server] listening on 20128"), - /no '\[DB\] Driver: \.\.\.' line/ + /no database activity/ + ); +}); + +test("electron smoke waits for database-open evidence after touching a DB-backed endpoint", async () => { + assert.equal(DB_TOUCH_PATH, "/api/monitoring/health"); + let logs = "[electron] [Server] [STARTUP] ready\n"; + setTimeout(() => { + logs += "[electron] [Server] [DB] Added usage_history.combo_strategy column\n"; + }, 60); + const seen = await waitForDatabaseOpen(() => logs, { timeoutMs: 2_000, pollMs: 20 }); + assert.match(seen, /\[DB\] Added/); +}); + +test("electron smoke fails clearly when the database never opens", async () => { + await assert.rejects( + () => + waitForDatabaseOpen(() => "[electron] [Server] [STARTUP] ready\n", { + timeoutMs: 120, + pollMs: 20, + }), + /logged no \[DB\]\/\[Migration\] startup line within 120ms/ + ); +}); + +test("electron smoke driver guard: native line, DB evidence and sql.js fallback", () => { + assert.doesNotThrow(() => + assertNativeDriverSelected("[DB] Driver: better-sqlite3 | file: /tmp/x/storage.sqlite\n") + ); + assert.doesNotThrow(() => + assertNativeDriverSelected( + "[electron] [Server] [DB] Added call_logs.session_tag column\n[electron] [Server] [Migration] Applied: 046_database_settings\n" + ) + ); + assert.throws( + () => assertNativeDriverSelected("[DB] Driver: sql.js | file: /tmp/x/storage.sqlite\n"), + /sql\.js \(WASM\) driver/ + ); + assert.throws( + () => assertNativeDriverSelected("[STARTUP] nothing here\n"), + /no database activity/ ); });