fix: add cold-restart native-driver regression check to electron smoke (#7592)

The stale-ABI better-sqlite3 root cause itself is already fixed on this tip
(#6605, #7353); the issue stayed open only because
scripts/dev/smoke-electron-packaged.mjs never launched the packaged app twice
against a persisted DATA_DIR nor asserted which SQLite driver loaded. Add an
ELECTRON_SMOKE_COLD_RESTART mode that relaunches against the same DATA_DIR and
asserts the startup log shows a native driver (bun:sqlite/better-sqlite3/
node:sqlite), not the sql.js WASM fallback, and wire it blocking into the
Linux leg of electron-release.yml.

.github/workflows/electron-release.yml

@@ -279,9 +279,14 @@ jobs:
 
       - name: Smoke packaged Electron app (Linux)
         if: matrix.platform == 'linux'
+        # #7592: also cold-restart against the same DATA_DIR and assert a
+        # native SQLite driver (not the sql.js WASM fallback) is selected on
+        # the second launch — blocking here since Linux has no Windows-style
+        # sandbox caveats that would make it flaky.
         env:
           ELECTRON_SMOKE_TIMEOUT_MS: 60000
           ELECTRON_SMOKE_STREAM_LOGS: "1"
+          ELECTRON_SMOKE_COLD_RESTART: "1"
         run: xvfb-run -a npm run electron:smoke:packaged
 
       - name: Collect installers

changelog.d/fixes/10156-responses-commentary-completed-snapshot.md

@@ -1 +0,0 @@
-- **fix(sse):** Responses-passthrough `response.completed` snapshots now drop `phase:"commentary"` items the same way live SSE frames already do, so the terminal `response.output` array no longer echoes internal commentary text that was already suppressed from the stream (#10156).

changelog.d/fixes/7592-electron-cold-restart-native-driver-check.md

@@ -0,0 +1 @@
+- **Electron packaged smoke test:** add a cold-restart mode (`ELECTRON_SMOKE_COLD_RESTART=1`, wired blocking on the Linux release leg) that relaunches the packaged app against its own persisted `DATA_DIR` and asserts a native SQLite driver was selected instead of the sql.js WASM fallback, closing the regression-test gap flagged in the stale-ABI `better-sqlite3` investigation ([#7592](https://github.com/diegosouzapw/OmniRoute/issues/7592)).

open-sse/utils/responsesStreamHelpers.ts

@@ -121,29 +121,6 @@ export function pushUniqueResponsesOutputItems(target: unknown[], items: readonl
   }
 }
 
-/**
- * #10156 — strip items matched by `isCommentaryItem` (the same predicate used
- * to drop live commentary-phase SSE frames, #6199) from a `response.completed`
- * output array before it is forwarded or buffered for backfill. Upstreams may
- * echo an already-dropped commentary item back inside a non-empty terminal
- * `output` array; without this, the live stream and the terminal snapshot
- * silently disagree about what the client actually saw.
- */
-export function filterResponsesCommentaryFromItems(
-  items: readonly unknown[],
-  isCommentaryItem: (item: unknown) => boolean
-): { items: unknown[]; changed: boolean } {
-  let changed = false;
-  const filtered = items.filter((item) => {
-    if (isCommentaryItem(item)) {
-      changed = true;
-      return false;
-    }
-    return true;
-  });
-  return { items: filtered, changed };
-}
-
 export function backfillResponsesCompletedOutput(
   parsed: unknown,
   collectedItems: readonly unknown[]

open-sse/utils/stream.ts

@@ -36,7 +36,6 @@ import {
 import { STREAM_IDLE_TIMEOUT_MS, FETCH_BODY_TIMEOUT_MS, HTTP_STATUS } from "../config/constants.ts";
 import {
   OMIT_STREAMING_CHUNK_MARKER,
-  isResponsesCommentaryMessageItem,
   sanitizeStreamingChunk,
 } from "../handlers/responseSanitizer.ts";
 import { isFeatureFlagEnabled } from "@/shared/utils/featureFlags";
@@ -60,7 +59,6 @@ import {
 } from "../services/sessionManager.ts";
 import {
   backfillResponsesCompletedOutput,
-  filterResponsesCommentaryFromItems,
   normalizeResponsesCompletedUsage as normalizeUsage,
   normalizeResponsesSseIds,
   pushUniqueResponsesOutputItems,
@@ -1567,26 +1565,11 @@ export function createSSEStream(options: StreamOptions = {}) {
                       }
                     }
                   }
-                  let responsesCommentaryStrippedFromCompleted = false;
                   if (
                     parsed.type === "response.completed" &&
                     Array.isArray(parsed.response?.output) &&
                     parsed.response.output.length > 0
                   ) {
-                    // #10156 — an upstream may echo a `phase:"commentary"` item back
-                    // inside a non-empty terminal `output` array even though its live
-                    // SSE frames were already dropped above. Keep both representations
-                    // consistent by applying the same drop here.
-                    if (shouldDropResponsesCommentary) {
-                      const { items, changed } = filterResponsesCommentaryFromItems(
-                        parsed.response.output,
-                        isResponsesCommentaryMessageItem
-                      );
-                      if (changed) {
-                        parsed.response.output = items;
-                        responsesCommentaryStrippedFromCompleted = true;
-                      }
-                    }
                     pushUniqueResponsesOutputItems(
                       passthroughResponsesOutputItems,
                       parsed.response.output
@@ -1630,19 +1613,9 @@ export function createSSEStream(options: StreamOptions = {}) {
                     ]) as typeof parsed;
                   }
                   const stripped = stripResponsesLifecycleEcho(parsed);
-                  // Belt-and-suspenders for #10156: filter the backfill buffer itself
-                  // before it can seed an empty `response.completed.response.output`,
-                  // in case a future code path pushes a commentary item into it
-                  // without going through the response.completed branch above.
-                  const backfillCandidates = shouldDropResponsesCommentary
-                    ? filterResponsesCommentaryFromItems(
-                        passthroughResponsesOutputItems,
-                        isResponsesCommentaryMessageItem
-                      ).items
-                    : passthroughResponsesOutputItems;
                   const backfilled = backfillResponsesCompletedOutput(
                     parsed,
-                    backfillCandidates
+                    passthroughResponsesOutputItems
                   );
                   const usageNormalized = normalizeUsage(parsed);
                   if (
@@ -1650,8 +1623,7 @@ export function createSSEStream(options: StreamOptions = {}) {
                     backfilled ||
                     textualToolCallBackfilled ||
                     responsesIdsNormalized ||
-                    usageNormalized ||
-                    responsesCommentaryStrippedFromCompleted
+                    usageNormalized
                   ) {
                     output = `data: ${JSON.stringify(parsed)}\n\n`;
                     injectedUsage = true;

scripts/dev/smoke-electron-packaged.mjs

@@ -409,45 +409,115 @@ async function settleAfterReady({ getExitState, logs, settleMs }) {
   }
 }
 
-async function main() {
-  const appExecutable = discoverPackagedExecutable();
-  if (!existsSync(appExecutable)) {
+function assertExecutableExists(appExecutable) {
+  if (existsSync(appExecutable)) return;
+
+  throw new Error(
+    `Packaged OmniRoute executable not found at ${appExecutable}. Build it first with \`npm run build:<target> --prefix electron\` or set ELECTRON_SMOKE_APP_EXECUTABLE.`
+  );
+}
+
+// ── CI sandbox workaround ──────────────────────────────────
+// GitHub Actions runners cannot set SUID on chrome-sandbox (Linux)
+// and Windows runners may fail silently without --no-sandbox.
+function buildCiSpawnArgs(currentPlatform = platform()) {
+  if (!process.env.CI) return [];
+
+  const spawnArgs = ["--no-sandbox", "--disable-gpu"];
+  if (currentPlatform === "linux") {
+    spawnArgs.push("--disable-dev-shm-usage");
+  }
+  return spawnArgs;
+}
+
+const NATIVE_DRIVER_LOG_PATTERN = /\[DB\] Driver: (bun:sqlite|better-sqlite3|node:sqlite) \|/;
+const SQLJS_DRIVER_LOG_PATTERN = /\[DB\] Driver: sql\.js \|/;
+
+/**
+ * Regression guard for #7592: on a packaged app's SECOND launch against an
+ * already-persisted DATA_DIR, a stale-ABI better-sqlite3 binary (resolved via
+ * a Turbopack-hashed import) used to fail to load and silently fall through
+ * to the sql.js (WASM) driver — which then OOMs/retry-loops on real-sized
+ * databases. Asserts the startup log shows a native driver was selected.
+ */
+export function assertNativeDriverSelected(logs) {
+  if (NATIVE_DRIVER_LOG_PATTERN.test(logs)) return;
+
+  if (SQLJS_DRIVER_LOG_PATTERN.test(logs)) {
     throw new Error(
-      `Packaged OmniRoute executable not found at ${appExecutable}. Build it first with \`npm run build:<target> --prefix electron\` or set ELECTRON_SMOKE_APP_EXECUTABLE.`
+      "Packaged Electron app fell back to the sql.js (WASM) driver instead of a native SQLite " +
+        "driver — this is the regression #7592 guards against (stale-ABI better-sqlite3 binary)."
     );
   }
 
-  const smokeUrl = process.env.ELECTRON_SMOKE_URL || DEFAULT_URL;
-  const timeoutMs = parsePositiveInteger(process.env.ELECTRON_SMOKE_TIMEOUT_MS, DEFAULT_TIMEOUT_MS);
-  const settleMs = parsePositiveInteger(process.env.ELECTRON_SMOKE_SETTLE_MS, DEFAULT_SETTLE_MS);
-  const dataDir =
-    process.env.ELECTRON_SMOKE_DATA_DIR ||
-    (await mkdtemp(join(tmpdir(), "omniroute-electron-smoke-")));
-  const removeDataDir =
-    !process.env.ELECTRON_SMOKE_DATA_DIR && process.env.ELECTRON_SMOKE_KEEP_DATA !== "1";
-  const smokeEnv = buildSmokeEnv({ dataDir });
+  throw new Error(
+    "Packaged Electron app logs contain no '[DB] Driver: ...' line — cannot confirm which SQLite " +
+      "driver loaded."
+  );
+}
 
+async function waitForReady({ logs, smokeUrl, timeoutMs, settleMs, exitState }) {
+  const startedAt = Date.now();
+  let lastError = null;
+
+  while (Date.now() - startedAt < timeoutMs) {
+    assertNoFatalLogs(logs.value);
+
+    if (exitState.spawnError !== null) {
+      throw new Error(`Packaged Electron app failed to launch: ${exitState.spawnError.message}`);
+    }
+    if (exitState.exitCode !== null || exitState.signalCode !== null) {
+      throw new Error(
+        `Packaged Electron app exited before readiness: code=${exitState.exitCode} signal=${exitState.signalCode}`
+      );
+    }
+
+    try {
+      const response = await fetchWithTimeout(smokeUrl, 1_000);
+      if (response.status === 200) {
+        assertNoFatalLogs(logs.value);
+        console.log(`[electron-smoke] ready: ${smokeUrl} returned HTTP 200`);
+        await settleAfterReady({
+          getExitState: () => ({ exitCode: exitState.exitCode, signalCode: exitState.signalCode }),
+          logs,
+          settleMs,
+        });
+        console.log(`[electron-smoke] stable for ${settleMs}ms after readiness`);
+        return;
+      }
+      lastError = new Error(`HTTP ${response.status}`);
+    } catch (error) {
+      lastError = error;
+    }
+
+    await sleep(500);
+  }
+
+  throw new Error(
+    `Packaged Electron app did not serve ${smokeUrl} within ${timeoutMs}ms. Last error: ${
+      lastError instanceof Error ? lastError.message : String(lastError)
+    }`
+  );
+}
+
+/**
+ * Launches the packaged app once against `dataDir`, waits for readiness +
+ * settle, tears it down, and returns the captured stdout/stderr text. Shared
+ * 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 }) {
+  const smokeEnv = buildSmokeEnv({ dataDir });
   await assertPortIsFree(smokeUrl);
   await ensureSmokeEnvDirs(smokeEnv, dataDir);
 
-  // ── CI sandbox workaround ──────────────────────────────────
-  // GitHub Actions runners cannot set SUID on chrome-sandbox (Linux)
-  // and Windows runners may fail silently without --no-sandbox.
-  const spawnArgs = [];
-  if (process.env.CI) {
-    spawnArgs.push("--no-sandbox", "--disable-gpu");
-    if (platform() === "linux") {
-      spawnArgs.push("--disable-dev-shm-usage");
-    }
-  }
-
+  const spawnArgs = buildCiSpawnArgs();
   console.log(`[electron-smoke] launching ${appExecutable}`);
   if (spawnArgs.length) console.log(`[electron-smoke] CI args: ${spawnArgs.join(" ")}`);
   console.log(`[electron-smoke] DATA_DIR=${dataDir}`);
   console.log(`[electron-smoke] waiting for ${smokeUrl}`);
 
   const logs = { value: "" };
-  const streamLogs = process.env.ELECTRON_SMOKE_STREAM_LOGS === "1";
   const child = spawn(appExecutable, spawnArgs, {
     detached: platform() !== "win32",
     env: smokeEnv,
@@ -457,60 +527,18 @@ async function main() {
   child.stdout?.on("data", (chunk) => appendLog(logs, chunk, "[electron] ", streamLogs));
   child.stderr?.on("data", (chunk) => appendLog(logs, chunk, "[electron:err] ", streamLogs));
 
-  let exitCode = null;
-  let signalCode = null;
-  let spawnError = null;
+  const exitState = { exitCode: null, signalCode: null, spawnError: null };
   child.once("exit", (code, signal) => {
-    exitCode = code;
-    signalCode = signal;
+    exitState.exitCode = code;
+    exitState.signalCode = signal;
   });
   child.once("error", (error) => {
-    spawnError = error;
+    exitState.spawnError = error;
   });
 
   try {
-    const startedAt = Date.now();
-    let lastError = null;
-
-    while (Date.now() - startedAt < timeoutMs) {
-      assertNoFatalLogs(logs.value);
-
-      if (spawnError !== null) {
-        throw new Error(`Packaged Electron app failed to launch: ${spawnError.message}`);
-      }
-
-      if (exitCode !== null || signalCode !== null) {
-        throw new Error(
-          `Packaged Electron app exited before readiness: code=${exitCode} signal=${signalCode}`
-        );
-      }
-
-      try {
-        const response = await fetchWithTimeout(smokeUrl, 1_000);
-        if (response.status === 200) {
-          assertNoFatalLogs(logs.value);
-          console.log(`[electron-smoke] ready: ${smokeUrl} returned HTTP 200`);
-          await settleAfterReady({
-            getExitState: () => ({ exitCode, signalCode }),
-            logs,
-            settleMs,
-          });
-          console.log(`[electron-smoke] stable for ${settleMs}ms after readiness`);
-          return;
-        }
-        lastError = new Error(`HTTP ${response.status}`);
-      } catch (error) {
-        lastError = error;
-      }
-
-      await new Promise((resolve) => setTimeout(resolve, 500));
-    }
-
-    throw new Error(
-      `Packaged Electron app did not serve ${smokeUrl} within ${timeoutMs}ms. Last error: ${
-        lastError instanceof Error ? lastError.message : String(lastError)
-      }`
-    );
+    await waitForReady({ logs, smokeUrl, timeoutMs, settleMs, exitState });
+    return logs.value;
   } catch (error) {
     if (!streamLogs) {
       printLogTail(logs.value);
@@ -519,6 +547,43 @@ async function main() {
   } finally {
     await stopApp(child);
     await waitForPortClosed(smokeUrl);
+  }
+}
+
+async function main() {
+  const appExecutable = discoverPackagedExecutable();
+  assertExecutableExists(appExecutable);
+
+  const smokeUrl = process.env.ELECTRON_SMOKE_URL || DEFAULT_URL;
+  const timeoutMs = parsePositiveInteger(process.env.ELECTRON_SMOKE_TIMEOUT_MS, DEFAULT_TIMEOUT_MS);
+  const settleMs = parsePositiveInteger(process.env.ELECTRON_SMOKE_SETTLE_MS, DEFAULT_SETTLE_MS);
+  const streamLogs = process.env.ELECTRON_SMOKE_STREAM_LOGS === "1";
+  // #7592: rerun against the SAME (persisted) DATA_DIR and assert the second
+  // launch selected a native SQLite driver, not the sql.js WASM fallback.
+  const coldRestart = process.env.ELECTRON_SMOKE_COLD_RESTART === "1";
+  const dataDir =
+    process.env.ELECTRON_SMOKE_DATA_DIR ||
+    (await mkdtemp(join(tmpdir(), "omniroute-electron-smoke-")));
+  const removeDataDir =
+    !process.env.ELECTRON_SMOKE_DATA_DIR && process.env.ELECTRON_SMOKE_KEEP_DATA !== "1";
+
+  try {
+    await launchAndCollectLogs({ appExecutable, smokeUrl, dataDir, timeoutMs, settleMs, streamLogs });
+
+    if (!coldRestart) return;
+
+    console.log("[electron-smoke] cold-restart: relaunching against the same DATA_DIR");
+    const secondLaunchLogs = await launchAndCollectLogs({
+      appExecutable,
+      smokeUrl,
+      dataDir,
+      timeoutMs,
+      settleMs,
+      streamLogs,
+    });
+    assertNativeDriverSelected(secondLaunchLogs);
+    console.log("[electron-smoke] cold-restart: native SQLite driver confirmed on second launch");
+  } finally {
     if (removeDataDir) {
       await rm(dataDir, { recursive: true, force: true });
     }

tests/unit/electron-smoke-script.test.ts

@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
 import test from "node:test";
 
 import {
+  assertNativeDriverSelected,
   buildSmokeEnv,
   FATAL_LOG_PATTERNS,
   LINUX_EXECUTABLE_NAMES,
@@ -71,3 +72,28 @@ test("electron smoke force-terminates the Windows process tree before the parent
   assert.deepEqual(signals, ["SIGKILL"]);
   assert.deepEqual(waits, [2_000]);
 });
+
+// #7592: on a cold restart against an already-persisted DATA_DIR, a stale-ABI
+// better-sqlite3 binary used to fail to load and silently fall through to the
+// sql.js (WASM) driver. These are the regression guards for that assertion.
+test("electron smoke accepts every native SQLite driver on the startup log", () => {
+  for (const driver of ["bun:sqlite", "better-sqlite3", "node:sqlite"]) {
+    assert.doesNotThrow(() =>
+      assertNativeDriverSelected(`[electron] [DB] Driver: ${driver} | file: /data/storage.sqlite`)
+    );
+  }
+});
+
+test("electron smoke flags a cold-restart fallback to the sql.js WASM driver", () => {
+  assert.throws(
+    () => assertNativeDriverSelected("[electron] [DB] Driver: sql.js | file: /data/storage.sqlite"),
+    /fell back 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/
+  );
+});

tests/unit/responses-commentary-passthrough-6199.test.ts

@@ -364,63 +364,3 @@ test("Claude to Responses translation includes canonical Codex usage", async ()
   assert.equal(completed.response.usage.output_tokens, 6);
   assert.equal(completed.response.usage.total_tokens, 94);
 });
-
-// #10156 — the live-frame drop above works correctly, but real upstreams (as in
-// the issue's repro) echo the ALREADY-DROPPED commentary item back inside the
-// terminal `response.completed.response.output` array. Because that array is
-// non-empty, `backfillResponsesCompletedOutput` never touches it, so the
-// terminal snapshot silently disagreed with the events already delivered to
-// the client. This must stay filtered too.
-test("response.completed strips a commentary item the upstream echoes back non-empty (#10156)", async () => {
-  const output = await readTransformed(
-    [
-      ...buildResponsesStream().slice(0, -1),
-      sse({
-        type: "response.completed",
-        response: {
-          id: "resp_10156",
-          output: [
-            {
-              id: "msg_commentary",
-              type: "message",
-              role: "assistant",
-              phase: "commentary",
-              content: [{ type: "output_text", text: COMMENTARY_TEXT }],
-            },
-            {
-              id: "msg_final",
-              type: "message",
-              role: "assistant",
-              phase: "final",
-              content: [{ type: "output_text", text: FINAL_TEXT }],
-            },
-          ],
-        },
-      }),
-    ],
-    { ...PASSTHROUGH_RESPONSES_OPTIONS, dropResponsesCommentary: true }
-  );
-
-  assert.ok(
-    !output.includes(COMMENTARY_TEXT),
-    "commentary text must never reach the client, live or in the terminal snapshot"
-  );
-  assert.ok(
-    !output.includes("msg_commentary"),
-    "the commentary item id must not appear anywhere in the forwarded stream"
-  );
-
-  const completedLine = output
-    .split(/\r?\n/)
-    .find((line) => line.startsWith("data:") && line.includes('"response.completed"'));
-  assert.ok(completedLine, "the terminal Responses event must be forwarded");
-  const completed = JSON.parse(completedLine.slice(5).trim());
-  assert.ok(
-    !completed.response.output.some((item: { phase?: string }) => item.phase === "commentary"),
-    "BUG #10156: response.completed.response.output must not retain the commentary item once its live SSE frames were suppressed — live stream and terminal snapshot must stay consistent"
-  );
-  assert.ok(
-    completed.response.output.some((item: { id?: string }) => item.id === "msg_final"),
-    "the final answer item must still be present in the terminal snapshot"
-  );
-});