From 66069029a558e1ba5c1e4429efdd0c58594c5753 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 8 Jul 2026 21:13:30 -0700 Subject: [PATCH] FN-7709: unref background integrity-check spawn and scheduling timer Prevents short-lived CLI processes from being held open by background SQLite integrity checks. - unref the sqlite3 child process (and its stdio) spawned by integrityCheckSqliteFileAsync via the shared unrefQmdChildProcess helper, immediately after spawn - unref the 60s scheduling timer in scheduleBackgroundIntegrityCheck so a short-lived caller isn't pinned waiting for a background check it never asked to block on - add regression test coverage (db-integrity-check-unref.test.ts) plus a CLI fixture (db-integrity-check-fixture.mjs) that exercises the fix in a real short-lived process - add changeset documenting the fix and the audit of other spawn sites across @fusion/core/@fusion/engine/@fusion/dashboard/cli confirming they are safe Files changed: .changeset/fn-7709-db-integrity-check-unref.md | 7 ++ .../src/__tests__/db-integrity-check-unref.test.ts | 135 +++++++++++++++++++++ .../fixtures/db-integrity-check-fixture.mjs | 28 +++++ packages/core/src/db.ts | 26 ++++ 4 files changed, 196 insertions(+) Fusion-Task-Id: FN-7709 Fusion-Task-Lineage: 6594aca4-0268-4bba-9a7f-af96d695f1e9 Co-authored-by: Fusion (runfusion.ai) --- .../fn-7709-db-integrity-check-unref.md | 7 + .../db-integrity-check-unref.test.ts | 135 ++++++++++++++++++ .../fixtures/db-integrity-check-fixture.mjs | 28 ++++ packages/core/src/db.ts | 26 ++++ 4 files changed, 196 insertions(+) create mode 100644 .changeset/fn-7709-db-integrity-check-unref.md create mode 100644 packages/core/src/__tests__/db-integrity-check-unref.test.ts create mode 100644 packages/core/src/__tests__/fixtures/db-integrity-check-fixture.mjs diff --git a/.changeset/fn-7709-db-integrity-check-unref.md b/.changeset/fn-7709-db-integrity-check-unref.md new file mode 100644 index 0000000000..986cb9bf38 --- /dev/null +++ b/.changeset/fn-7709-db-integrity-check-unref.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix background SQLite integrity checks holding short-lived CLI commands open unnecessarily. +category: fix +dev: `integrityCheckSqliteFileAsync`'s spawned `sqlite3` child (+ stdio) is now unref'd via the shared `unrefQmdChildProcess` helper, and `scheduleBackgroundIntegrityCheck`'s 60s scheduling timer is now `.unref()`'d, so a short-lived process (e.g. a `fn` one-shot CLI command) that opens a disk-backed `Database` exits promptly instead of being pinned by the background integrity check (FN-7706/FN-7707-class leak). Audited every other non-FN-7708 inline spawn site across `@fusion/core`/`@fusion/engine`/`@fusion/dashboard`/cli and found them SAFE (synchronous, awaited-as-own-work, or intentionally-tracked persistent processes) — see FN-7709's audit document. diff --git a/packages/core/src/__tests__/db-integrity-check-unref.test.ts b/packages/core/src/__tests__/db-integrity-check-unref.test.ts new file mode 100644 index 0000000000..732a1715dc --- /dev/null +++ b/packages/core/src/__tests__/db-integrity-check-unref.test.ts @@ -0,0 +1,135 @@ +/** + * FNXC:Database 2026-07-08-00:00: + * Regression coverage for FN-7709: the background sqlite3 integrity-check child + * spawned by `integrityCheckSqliteFileAsync` (reached fire-and-forget from + * `scheduleBackgroundIntegrityCheck`) must be unref'd (child + stdio), and the + * 60s scheduling `setTimeout` in `scheduleBackgroundIntegrityCheck` must be + * `.unref()`'d, so a short-lived caller (e.g. a `fn` one-shot CLI command that + * opens a disk-backed `Database` and exits without `close()`) is not pinned + * alive by either handle. Two layers, per FN-5048 (prefer bounded + * fixtures/fake timers over real multi-second waits): + * 1. A fast fake-timer unit test asserting the 60s scheduling timer is + * unref'd (`Timeout#hasRef()`) immediately after `init()` schedules it — + * no real 60s wait. + * 2. An end-to-end symptom test: a fixture Node process fires + * `integrityCheckSqliteFileAsync` (fire-and-forget, mirroring the real + * call site) against a slow fake `sqlite3` stub on PATH and must exit + * well before the stub does. + */ +import { describe, it, expect, afterEach, vi } from "vitest"; +import { spawn } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync, chmodSync, closeSync, openSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createRequire } from "node:module"; +import { Database } from "../db.js"; + +const tsxPackageJsonPath = createRequire(import.meta.url).resolve("tsx/package.json"); +const tsxCliPath = join(tsxPackageJsonPath, "..", "dist", "cli.mjs"); + +describe("scheduleBackgroundIntegrityCheck 60s timer is unref'd (unit)", () => { + const tempDirs: string[] = []; + + afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("the shared scheduling timer is unref'd right after init() so it can't pin a short-lived caller alive", () => { + vi.useFakeTimers(); + try { + const freshDir = mkdtempSync(join(tmpdir(), "fn-7709-db-unref-")); + tempDirs.push(freshDir); + const freshFusionDir = join(freshDir, ".fusion"); + const freshDb = new Database(freshFusionDir); + + try { + freshDb.init(); + + const shared = ( + Database as unknown as { + sharedIntegrityChecks: Map boolean } | null }>; + } + ).sharedIntegrityChecks.get((freshDb as unknown as { dbPath: string }).dbPath); + + expect(shared?.timer).toBeTruthy(); + // A ref'd (default) Node Timeout reports hasRef() === true; our fix must + // flip this to false immediately after scheduling, without waiting for + // the 60s delay to elapse. + expect(shared?.timer?.hasRef?.()).toBe(false); + } finally { + freshDb.close(); + } + } finally { + vi.useRealTimers(); + } + }); +}); + +describe("integrityCheckSqliteFileAsync does not keep a short-lived caller alive (symptom)", () => { + const tempDirs: string[] = []; + + afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + function writeSlowSqlite3Stub(stubDir: string): void { + // Fake `sqlite3`: sleeps well past our exit-bound assertion regardless of + // args — models a disk-stalled / slow integrity-check walk without a real + // multi-second wait dominating the test budget on the assertion side; only + // the *stub* sleeps long, the *test* just measures how fast the fixture + // process exits. + const stubPath = join(stubDir, "sqlite3"); + writeFileSync(stubPath, ["#!/usr/bin/env bash", "sleep 8", "exit 0", ""].join("\n"), "utf8"); + chmodSync(stubPath, 0o755); + } + + async function runFixture(dbPath: string, stubDir: string) { + const fixturePath = join(import.meta.dirname, "fixtures", "db-integrity-check-fixture.mjs"); + const startedAt = Date.now(); + return new Promise<{ code: number | null; elapsedMs: number; stdout: string }>((resolvePromise, reject) => { + let stdout = ""; + const child = spawn(process.execPath, [tsxCliPath, fixturePath, dbPath], { + env: { + ...process.env, + PATH: `${stubDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? ""}`, + }, + stdio: ["ignore", "pipe", "pipe"], + }); + + child.stdout.on("data", (chunk) => { + stdout += String(chunk); + }); + child.on("error", reject); + child.on("exit", (code) => { + resolvePromise({ code, elapsedMs: Date.now() - startedAt, stdout }); + }); + }); + } + + it("fixture process exits promptly even though the background sqlite3 stub is still sleeping", async () => { + const stubDir = mkdtempSync(join(tmpdir(), "fn-7709-sqlite3-stub-")); + tempDirs.push(stubDir); + const rootDir = mkdtempSync(join(tmpdir(), "fn-7709-sqlite3-root-")); + tempDirs.push(rootDir); + writeSlowSqlite3Stub(stubDir); + + // integrityCheckSqliteFileAsync only requires the path to exist — an empty + // file is enough since the real check never runs (the stub short-circuits + // it) and we only assert on process exit timing here. + const dbPath = join(rootDir, "fusion.db"); + closeSync(openSync(dbPath, "w")); + + const exitInfo = await runFixture(dbPath, stubDir); + + expect(exitInfo.stdout).toContain("db-integrity-check-fixture:scheduled"); + expect(exitInfo.code).toBe(0); + // The fake sqlite3 sleeps 8s; the fixture process must exit well before + // that, proving the background child + stdio were unref'd rather than + // holding the fixture's event loop open for the stub's full runtime. + expect(exitInfo.elapsedMs).toBeLessThan(5_000); + }, 15_000); +}); diff --git a/packages/core/src/__tests__/fixtures/db-integrity-check-fixture.mjs b/packages/core/src/__tests__/fixtures/db-integrity-check-fixture.mjs new file mode 100644 index 0000000000..e762e6a180 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/db-integrity-check-fixture.mjs @@ -0,0 +1,28 @@ +#!/usr/bin/env node +/** + * FNXC:Database 2026-07-08-00:00: + * Symptom-verification fixture for FN-7709. Fires + * `integrityCheckSqliteFileAsync` (fire-and-forget, mirroring how + * `scheduleBackgroundIntegrityCheck` invokes it in production) against + * whatever `sqlite3` resolves on PATH, then returns from main immediately. + * If the async spawn path does not unref its child + stdio, this process + * stays alive until the background `sqlite3` child exits (or is + * force-killed by the AbortSignal timeout) instead of exiting on its own + * once this script's own work is done. Loaded via `tsx` so it can import + * the real TypeScript source directly (no separate build step required). + */ +import { integrityCheckSqliteFileAsync } from "../../db.ts"; + +const dbPath = process.argv[2]; +if (!dbPath) { + throw new Error("db-integrity-check-fixture: missing dbPath argument"); +} + +// Intentionally NOT awaited — mirrors `scheduleBackgroundIntegrityCheck`'s +// `void (async () => { await integrityCheckSqliteFileAsync(...) })()` fire-and-forget +// call site in db.ts. +void integrityCheckSqliteFileAsync(dbPath); + +// Print a marker so the test can confirm the fixture actually reached this +// point (i.e. the call itself did not throw synchronously) before exiting. +console.log("db-integrity-check-fixture:scheduled"); diff --git a/packages/core/src/db.ts b/packages/core/src/db.ts index 1d9b3279b1..34cab37bd0 100644 --- a/packages/core/src/db.ts +++ b/packages/core/src/db.ts @@ -13,6 +13,7 @@ import { basename, dirname, isAbsolute, join } from "node:path"; import { mkdirSync, existsSync, statSync, renameSync, rmSync } from "node:fs"; import { spawn, spawnSync } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; +import { unrefQmdChildProcess } from "./memory-backend.js"; import { DEFAULT_PROJECT_SETTINGS } from "./types.js"; import type { PluginOnSchemaInit } from "./plugin-types.js"; import type { SteeringComment, TaskComment } from "./types.js"; @@ -1798,6 +1799,18 @@ export function quickCheckSqliteFile(dbPath: string): { ok: boolean; verified: b * would strand the background scheduler's shared entry and pin every * participant's `integrityCheckPending` true forever. AbortSignal.timeout's * internal timer is unref'd, so it never keeps the process alive on shutdown. + * + * FNXC:Database 2026-07-08-00:00: + * This spawn is reached fire-and-forget from `scheduleBackgroundIntegrityCheck`'s + * `void (async () => …)()` — nothing here is guaranteed to be awaited by a live + * caller. A short-lived process (e.g. a `fn` one-shot CLI command) that opens a + * disk-backed `Database` and exits before the 60s scheduling delay elapses must + * not be pinned alive by this child's stdio pipes. Unref the child (+ stdio) + * immediately after spawn via the shared `unrefQmdChildProcess` helper — see its + * doc comment in `memory-backend.ts` for why a plain `spawn()` (not + * `promisify(execFile)`) is required for the unref to actually stick (FN-7706). + * This does not affect resolve/reject semantics: long-lived callers that DO stay + * alive still see the promise settle normally on `close`/`error` (FN-7709). */ const INTEGRITY_CHECK_TIMEOUT_MS = 5 * 60 * 1000; @@ -1823,6 +1836,11 @@ export function integrityCheckSqliteFileAsync( // option-validation errors, e.g. an already-aborted signal.) signal: AbortSignal.timeout(INTEGRITY_CHECK_TIMEOUT_MS), }); + // FNXC:Database 2026-07-08-00:00: unref synchronously right after spawn — a + // manual unref later (e.g. inside a `data`/`close` handler) would race the + // execFile-style nextTick stdio re-ref; see the doc comment above this + // function (FN-7709 / FN-7706). + unrefQmdChildProcess(child); } catch { resolve({ ok: true, verified: false }); return; @@ -5914,6 +5932,14 @@ export class Database { Database.sharedIntegrityChecks.delete(this.dbPath); }); }, 60_000); + // FNXC:Database 2026-07-08-00:00: unref the 60s scheduling delay so a + // short-lived caller (e.g. a `fn` one-shot CLI command that opens a + // disk-backed Database and exits before this fires) is not pinned alive + // waiting for a background check it never asked to block on (FN-7709). + // The check itself still fires normally for any process that stays alive + // past the delay — unref only affects whether the handle keeps the event + // loop open, not whether/when the timer callback runs. + shared.timer.unref?.(); Database.sharedIntegrityChecks.set(this.dbPath, shared); }