fix(db): skip integrity scans during health polling (#13149)

* fix(db): skip integrity scans during health polling

* test(db): update health error fixture for scan-free polling

* docs(changelog): add fragment for health poll integrity skip

* fix(db): keep the #13149 dashboard skip inside the #13717 managed health check

Merge fallout only: runManagedDbHealthCheck moved behind the health
coordinator on the release tip, so the per-call skipIntegrityCheck now
travels through it. A waived integrity scan is part of the job identity,
so it is never replayed from the 60s diagnosis cache to a caller that
asked for the full scan.

Co-authored-by: cryptiklemur <cryptiklemur@users.noreply.github.com>

---------

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
Co-authored-by: cryptiklemur <cryptiklemur@users.noreply.github.com>

changelog.d/fixes/13149-health-poll-skip-integrity.md

@@ -0,0 +1 @@
+- **fix(db):** authenticated `GET /api/db/health` polls no longer run a SQLite `quick_check`. The health dashboard polls every 15 seconds, and that scan ran synchronously on the request-serving event loop, blocking it for the length of the scan. Reference and state checks still run, and explicit repair requests keep integrity checks unless `OMNIROUTE_SKIP_DB_HEALTHCHECK=1` is set ([#13149](https://github.com/diegosouzapw/OmniRoute/pull/13149))

src/app/api/db/health/route.ts

@@ -9,7 +9,9 @@ export async function GET(request: Request) {
   }
 
   try {
-    return NextResponse.json(await runManagedDbHealthCheck({ autoRepair: false }));
+    return NextResponse.json(
+      await runManagedDbHealthCheck({ autoRepair: false, skipIntegrityCheck: true })
+    );
   } catch (error) {
     const message = error instanceof Error ? error.message : String(error);
     console.error("[API] DB health diagnosis failed:", message);

src/lib/db/core.ts

@@ -943,9 +943,9 @@ function startDbHealthCheckScheduler(db: SqliteDatabase) {
 // The scheduler lives in ./walMaintenance (periodic TRUNCATE + busy warn + PASSIVE retry).
 
 const healthShutdown = new AbortController();
-const managedHealth = createDbHealthCoordinator(async (autoRepair) => {
+const managedHealth = createDbHealthCoordinator(async (autoRepair, skipIntegrity) => {
   const db = getDbInstance();
-  const skipIntegrityCheck = process.env.OMNIROUTE_SKIP_DB_HEALTHCHECK === "1";
+  const skipIntegrityCheck = skipIntegrity || process.env.OMNIROUTE_SKIP_DB_HEALTHCHECK === "1";
   const backupDir = DB_BACKUPS_DIR || path.join(DATA_DIR, "db_backups");
   const result =
     db.driver === "sql.js" || db.name === ":memory:" || !db.name
@@ -969,10 +969,10 @@ const managedHealth = createDbHealthCoordinator(async (autoRepair) => {
   if (result.repairedCount > 0) invalidateDbCache();
   return result;
 });
-
-export function runManagedDbHealthCheck(options?: { autoRepair?: boolean }) {
+type ManagedHealthCheckOptions = { autoRepair?: boolean; skipIntegrityCheck?: boolean };
+export function runManagedDbHealthCheck(options?: ManagedHealthCheckOptions) {
   if (getPagerCorruption()) managedHealth.invalidate();
-  return managedHealth.run(options?.autoRepair === true);
+  return managedHealth.run(options?.autoRepair === true, options?.skipIntegrityCheck === true);
 }
 
 export function getDbInstance(): SqliteDatabase {

src/lib/db/healthCheckRunner.ts

@@ -12,14 +12,23 @@ export interface DbHealthJob {
   pagerCorruption: PagerCorruptionNote | null;
 }
 
+/**
+ * `skipIntegrityCheck` (#13149): a caller may waive the integrity scan — the dashboard
+ * poll does. It is part of the job identity, so a waived scan is never replayed from the
+ * cache to a caller that asked for a full one (the reverse direction is safe).
+ */
 export function createDbHealthCoordinator(
-  execute: (autoRepair: boolean) => Promise<DbHealthCheckResult>,
+  execute: (autoRepair: boolean, skipIntegrityCheck: boolean) => Promise<DbHealthCheckResult>,
   options: { now?: () => number; cacheMs?: number } = {}
 ) {
   const now = options.now ?? Date.now;
   const cacheMs = options.cacheMs ?? 60_000;
   let stopping = false;
-  let active: { autoRepair: boolean; promise: Promise<DbHealthCheckResult> } | null = null;
+  let active: {
+    autoRepair: boolean;
+    skipIntegrityCheck: boolean;
+    promise: Promise<DbHealthCheckResult>;
+  } | null = null;
   let cached: { result: DbHealthCheckResult; expires: number } | null = null;
   return {
     get busy(): boolean {
@@ -34,10 +43,13 @@ export function createDbHealthCoordinator(
       cancel();
       await active?.promise.catch(() => {});
     },
-    run(autoRepair: boolean): Promise<DbHealthCheckResult> {
+    run(autoRepair: boolean, skipIntegrityCheck = false): Promise<DbHealthCheckResult> {
       if (stopping) return Promise.reject(new Error("Database health checks are stopping"));
       if (active) {
-        return active.autoRepair === autoRepair
+        // A run that DID scan integrity satisfies a caller that was willing to skip it,
+        // never the reverse (#13149) — so only widen, never narrow, the in-flight job.
+        return active.autoRepair === autoRepair &&
+          (active.skipIntegrityCheck === skipIntegrityCheck || !active.skipIntegrityCheck)
           ? active.promise
           : Promise.reject(new Error("Database health check already in progress"));
       }
@@ -49,9 +61,11 @@ export function createDbHealthCoordinator(
         resolve = yes;
         reject = no;
       });
-      active = { autoRepair, promise };
+      active = { autoRepair, skipIntegrityCheck, promise };
       const succeed = (result: DbHealthCheckResult) => {
-        if (!autoRepair) cached = { result, expires: now() + cacheMs };
+        // Only a full (integrity-scanning) diagnosis may be replayed from the cache:
+        // caching a skipped scan would silently downgrade a later full request (#13149).
+        if (!autoRepair && !skipIntegrityCheck) cached = { result, expires: now() + cacheMs };
         active = null;
         resolve(result);
       };
@@ -60,7 +74,7 @@ export function createDbHealthCoordinator(
         reject(error);
       };
       try {
-        execute(autoRepair).then(succeed, fail);
+        execute(autoRepair, skipIntegrityCheck).then(succeed, fail);
       } catch (error) {
         fail(error);
       }

tests/unit/db-health-poll-nonblocking.test.mjs

@@ -0,0 +1,103 @@
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { stripTypeScriptTypes } from "node:module";
+import test from "node:test";
+import vm from "node:vm";
+
+import { createDbHealthCoordinator } from "../../src/lib/db/healthCheckRunner.ts";
+
+// The managed health check is loaded from the REAL core.ts source (sliced, not
+// reimplemented) so this test cannot drift away from production. #13717 moved the
+// body behind a coordinator, so the slice starts at the coordinator construction.
+const core = readFileSync(new URL("../../src/lib/db/core.ts", import.meta.url), "utf8");
+const managedCheck = core
+  .slice(
+    core.indexOf("const healthShutdown = new AbortController();"),
+    core.indexOf("export function getDbInstance()")
+  )
+  .replace("export function", "function");
+const route = readFileSync(new URL("../../src/app/api/db/health/route.ts", import.meta.url), "utf8")
+  .replace(/^import .*;\n/gm, "")
+  .replaceAll("export async function", "async function");
+
+function loadCheck(skipEnv, authenticated = true) {
+  const calls = [];
+  const context = vm.createContext({
+    process: { env: { OMNIROUTE_SKIP_DB_HEALTHCHECK: skipEnv } },
+    AbortController,
+    createDbHealthCoordinator,
+    getPagerCorruption: () => null,
+    invalidateDbCache: () => {},
+    path: { join: (...parts) => parts.join("/") },
+    DB_BACKUPS_DIR: "/tmp/db_backups",
+    DATA_DIR: "/tmp",
+    writeManagedDbBackup: () => assert.fail("read-only polling must not create backups"),
+    runDbHealthInChild: () => assert.fail("stub DB must stay on the owning-connection path"),
+    // No `name`/`driver`: core takes the in-process branch, never forking a child.
+    getDbInstance: () => ({}),
+    runDbHealthCheck: (_db, options) => {
+      calls.push(options);
+      return { status: "healthy", repairedCount: 0 };
+    },
+    isAuthenticated: async () => authenticated,
+    NextResponse: { json: (body, options) => ({ body, status: options?.status ?? 200 }) },
+    sanitizeErrorMessage: (message) => message,
+    console,
+  });
+  vm.runInContext(stripTypeScriptTypes(managedCheck + route), context);
+  return { context, calls };
+}
+
+test("dashboard GET skips integrity scans even without a deployment opt-out", async () => {
+  const { context, calls } = loadCheck(undefined);
+  for (let poll = 0; poll < 3; poll++) {
+    const response = await vm.runInContext("GET({})", context);
+    assert.equal(response.status, 200, JSON.stringify(response.body));
+  }
+  // Three real calls: a skipped scan is never replayed from the diagnosis cache,
+  // otherwise a later full diagnosis would be silently downgraded.
+  assert.equal(calls.length, 3);
+  for (const options of calls) {
+    assert.equal(options.autoRepair, false);
+    assert.equal(options.skipIntegrityCheck, true);
+  }
+});
+
+test("managed checks honor the deployment opt-out, including manual repair", async () => {
+  const { context, calls } = loadCheck("1");
+  await vm.runInContext("runManagedDbHealthCheck({skipIntegrityCheck:false})", context);
+  const response = await vm.runInContext("POST({})", context);
+  assert.equal(response.status, 200, JSON.stringify(response.body));
+  assert.equal(calls.length, 2);
+  assert.equal(calls[0].skipIntegrityCheck, true);
+  assert.equal(calls[1].skipIntegrityCheck, true);
+  assert.equal(calls[1].autoRepair, true);
+});
+
+test("explicit repair retains integrity scans when the deployment allows them", async () => {
+  const { context, calls } = loadCheck("0");
+  const response = await vm.runInContext("POST({})", context);
+  assert.equal(response.status, 200, JSON.stringify(response.body));
+  assert.equal(Boolean(calls[0].skipIntegrityCheck), false);
+  assert.equal(calls[0].autoRepair, true);
+});
+
+test("a full diagnosis is still cached, a skipped one is not", async () => {
+  const { context, calls } = loadCheck(undefined);
+  await vm.runInContext("runManagedDbHealthCheck({})", context);
+  await vm.runInContext("runManagedDbHealthCheck({})", context);
+  assert.equal(calls.length, 1, "full diagnosis is served from the 60s cache");
+  assert.equal(Boolean(calls[0].skipIntegrityCheck), false);
+  await vm.runInContext("GET({})", context);
+  // The cached FULL result satisfies a caller willing to skip the scan.
+  assert.equal(calls.length, 1);
+});
+
+test("unauthenticated polling and repair do not touch the database", async () => {
+  const { context, calls } = loadCheck(undefined, false);
+  for (const method of ["GET", "POST"]) {
+    const response = await vm.runInContext(`${method}({})`, context);
+    assert.equal(response.status, 401);
+  }
+  assert.equal(calls.length, 0);
+});

tests/unit/rule12-error-sanitization-sweep.test.ts

@@ -77,17 +77,15 @@ function patchPrepareToThrow(sqlMatch: string): () => void {
   };
 }
 
-function patchPragmaToThrow(): () => void {
+// #13717 runs the managed health check in a child process for a real file-backed DB;
+// a child never inherits the JS stubs below, so force the owning-connection path.
+// (#13149 additionally makes the dashboard GET skip the integrity scan, so the throw
+// has to come from a prepare() on the schema-version probe, not from pragma().)
+function forceOwningConnectionPath(): () => void {
   const db = core.getDbInstance();
-  const orig = db.pragma.bind(db);
-  // Select the owning-connection path; child processes do not inherit JS stubs.
   const nameDescriptor = Object.getOwnPropertyDescriptor(db, "name");
   Object.defineProperty(db, "name", { configurable: true, value: ":memory:" });
-  (db as unknown as { pragma: unknown }).pragma = () => {
-    throw makeLeakyError();
-  };
   return () => {
-    (db as unknown as { pragma: unknown }).pragma = orig;
     if (nameDescriptor) Object.defineProperty(db, "name", nameDescriptor);
   };
 }
@@ -140,7 +138,8 @@ test("GET /api/cache/entries → 500 body is sanitized (shape { error })", async
 });
 
 test("GET /api/db/health → 500 body is sanitized (shape { error: { message } })", async () => {
-  const restore = patchPragmaToThrow();
+  const restoreName = forceOwningConnectionPath();
+  const restore = patchPrepareToThrow("SELECT value FROM db_meta WHERE key = 'schema_version'");
   try {
     const res = await dbHealthRoute.GET(makeRequest("http://localhost/api/db/health"));
     assert.equal(res.status, 500);
@@ -149,5 +148,6 @@ test("GET /api/db/health → 500 body is sanitized (shape { error: { message } }
     assertSanitized(body.error.message, "db/health GET");
   } finally {
     restore();
+    restoreName();
   }
 });