From dcfbee9ae6baa03258c17a64c792e38736c68573 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 8 Jul 2026 20:38:35 -0700 Subject: [PATCH] FN-7707: reuse hardened unref'd executor in searchWithQmd Fixes searchWithQmd's inline promisify(execFile) copy that could hold a caller open by reusing the already-hardened, synchronously-unref'd executor established for the background refresh path. - searchWithQmd now calls getDefaultExecFileAsync() instead of building its own promisify(execFile) executor inline - Removes the second un-unref'd execFile executor that could keep a short-lived caller (e.g. one-shot CLI memory search) open up to the awaited timeout - Adds regression test fixture and test coverage (qmd-search-fixture.mjs, qmd-search-unref.test.ts) asserting the shared executor is used - Adds changeset (patch) for @runfusion/fusion Files changed: .changeset/fn-7707-qmd-search-unref.md | 7 + packages/core/src/__tests__/fixtures/qmd-search-fixture.mjs | 30 ++++ packages/core/src/__tests__/qmd-search-unref.test.ts | 166 +++++++++++++++++++++ packages/core/src/memory-backend.ts | 16 +- 4 files changed, 216 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-7707 Fusion-Task-Lineage: 3f9f94a7-5613-4b9a-a3ba-8e9bcdd6b687 Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-7707-qmd-search-unref.md | 7 + .../__tests__/fixtures/qmd-search-fixture.mjs | 30 ++++ .../src/__tests__/qmd-search-unref.test.ts | 166 ++++++++++++++++++ packages/core/src/memory-backend.ts | 16 +- 4 files changed, 216 insertions(+), 3 deletions(-) create mode 100644 .changeset/fn-7707-qmd-search-unref.md create mode 100644 packages/core/src/__tests__/fixtures/qmd-search-fixture.mjs create mode 100644 packages/core/src/__tests__/qmd-search-unref.test.ts diff --git a/.changeset/fn-7707-qmd-search-unref.md b/.changeset/fn-7707-qmd-search-unref.md new file mode 100644 index 0000000000..bf01ea0308 --- /dev/null +++ b/.changeset/fn-7707-qmd-search-unref.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: qmd-backed project memory search no longer keeps short-lived CLI/Node processes alive. +category: fix +dev: `searchWithQmd` in `packages/core/src/memory-backend.ts` no longer carries its own inline `promisify(execFile)` copy for the awaited `qmd collection add` / `qmd search` calls; it now routes through the FN-7706-hardened `getDefaultExecFileAsync()` spawn-based executor, which unrefs the child + stdio synchronously so a short-lived caller invoking a search is not held open by a slow/hung qmd child beyond its own work, while preserving the same `{stdout, stderr}` resolve / reject-on-nonzero-exit contract the search's JSON parsing depends on. diff --git a/packages/core/src/__tests__/fixtures/qmd-search-fixture.mjs b/packages/core/src/__tests__/fixtures/qmd-search-fixture.mjs new file mode 100644 index 0000000000..cc2950e793 --- /dev/null +++ b/packages/core/src/__tests__/fixtures/qmd-search-fixture.mjs @@ -0,0 +1,30 @@ +#!/usr/bin/env node +/** + * FNXC:ProjectMemory 2026-07-08-00:00: + * Symptom-verification fixture for FN-7707. Invokes an AWAITED qmd project-memory + * search (QmdMemoryBackend.search -> searchWithQmd) against whatever `qmd` resolves + * on PATH. Prints a "started" marker BEFORE awaiting so the test can tell the + * fixture actually reached the search call. Prints a "searched" marker AFTER the + * await settles — but that marker is best-effort only: once searchWithQmd routes + * through the unref'd default executor, a fixture process with nothing else to do + * can legitimately exit before a still-hanging qmd child's promise ever settles, so + * the test must not require "searched" to prove the fix. If the qmd child + its + * stdio pipes are not unref'd, this process instead stays alive for the child's + * full runtime (its pending await only settles once the child truly exits), well + * past the strict exit-bound the test asserts. Loaded via `tsx` so it can import + * the real TypeScript source directly (no separate build step required). + */ +import { QmdMemoryBackend } from "../../memory-backend.ts"; + +const rootDir = process.argv[2]; +if (!rootDir) { + throw new Error("qmd-search-fixture: missing rootDir argument"); +} + +console.log("qmd-search-fixture:started"); + +const backend = new QmdMemoryBackend(); +await backend.search(rootDir, { query: "fn-7707-fixture-query", limit: 5 }); + +// Best-effort marker — see file header. Not required by the test. +console.log("qmd-search-fixture:searched"); diff --git a/packages/core/src/__tests__/qmd-search-unref.test.ts b/packages/core/src/__tests__/qmd-search-unref.test.ts new file mode 100644 index 0000000000..8529ee7359 --- /dev/null +++ b/packages/core/src/__tests__/qmd-search-unref.test.ts @@ -0,0 +1,166 @@ +/** + * FNXC:ProjectMemory 2026-07-08-00:00: + * Regression coverage for FN-7707: the AWAITED `searchWithQmd` search child spawned + * by memory-backend.ts must route through the FN-7706-hardened, unref'd default + * executor (getDefaultExecFileAsync) instead of carrying its own inline + * `promisify(execFile)` copy, so a short-lived caller invoking a project-memory + * search never gets held open by the qmd child's stdio pipes beyond its own actual + * work. Two layers: + * 1. A unit test asserting `searchWithQmd` routes both the collection-add and the + * search call through the default executor (no private promisify(execFile)). + * 2. An end-to-end symptom test: a fixture Node process awaits a search against a + * slow, SIGTERM-ignoring fake `qmd` stub on PATH and must still exit well + * before the stub's own runtime completes. + */ +import { describe, it, expect, afterEach, vi } from "vitest"; +import { spawn } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync, chmodSync, mkdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createRequire } from "node:module"; + +const tsxPackageJsonPath = createRequire(import.meta.url).resolve("tsx/package.json"); +const tsxCliPath = join(tsxPackageJsonPath, "..", "dist", "cli.mjs"); + +describe("searchWithQmd routes through the hardened default executor (unit)", () => { + const tempDirs: string[] = []; + + afterEach(() => { + vi.unstubAllEnvs(); + vi.resetModules(); + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("calls the default (spawn-based) executor for both collection-add and search, never a private promisify(execFile)", async () => { + vi.stubEnv("FUSION_ENABLE_QMD_REFRESH_IN_TESTS", "1"); + vi.resetModules(); + + const spawnCalls: Array<{ file: string; args: string[] }> = []; + vi.doMock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + spawn: (file: string, args: string[], options: unknown) => { + spawnCalls.push({ file, args }); + const lastArg = Array.isArray(args) ? args[0] : undefined; + // Fake a fast-closing child for both "collection"/"add" and "search". + const fakeChild = actual.spawn( + process.execPath, + ["-e", lastArg === "search" ? "process.stdout.write('[]')" : ""], + options as Record, + ); + return fakeChild; + }, + }; + }); + + const rootDir = mkdtempSync(join(tmpdir(), "fn-7707-unit-root-")); + tempDirs.push(rootDir); + mkdirSync(join(rootDir, ".fusion", "memory"), { recursive: true }); + + const { QmdMemoryBackend } = await import("../memory-backend.js"); + const backend = new QmdMemoryBackend(); + const results = await backend.search(rootDir, { query: "unit-test-query", limit: 5 }); + + expect(Array.isArray(results)).toBe(true); + // Both the collection-add and the qmd search calls must go through the mocked + // `spawn` (the default executor's underlying primitive) — proving searchWithQmd + // no longer constructs its own private `promisify(execFile)` copy, which would + // bypass this mock entirely and use the real un-unref'd execFile path instead. + const collectionAddCalls = spawnCalls.filter((call) => call.args[0] === "collection" && call.args[1] === "add"); + const searchCalls = spawnCalls.filter((call) => call.args[0] === "search"); + expect(collectionAddCalls.length).toBeGreaterThanOrEqual(1); + expect(searchCalls.length).toBeGreaterThanOrEqual(1); + + vi.doUnmock("node:child_process"); + }); +}); + +describe("qmd search 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 writeStubbornSlowQmdStub(stubDir: string): void { + // Fake `qmd`: "collection add" (and anything else) responds instantly. + // "search" traps and ignores SIGTERM, then sleeps ~8s before responding — this + // models a qmd child that keeps running past searchWithQmd's own 4s internal + // timeout kill attempt, so only a properly unref'd child+stdio (not a merely + // "timed-out" JS promise) lets the caller process exit promptly. + const stubPath = join(stubDir, "qmd"); + writeFileSync( + stubPath, + [ + "#!/usr/bin/env bash", + "trap '' TERM", + 'case "$1" in', + " search)", + " sleep 8", + " echo '[]'", + " ;;", + " *)", + " exit 0", + " ;;", + "esac", + "", + ].join("\n"), + "utf8", + ); + chmodSync(stubPath, 0o755); + } + + async function runFixture(rootDir: string, stubDir: string) { + const fixturePath = join(import.meta.dirname, "fixtures", "qmd-search-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, rootDir], { + env: { + ...process.env, + PATH: `${stubDir}${process.platform === "win32" ? ";" : ":"}${process.env.PATH ?? ""}`, + FUSION_ENABLE_QMD_REFRESH_IN_TESTS: "1", + }, + 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("search: fixture process exits promptly even though the search's qmd child ignores the internal timeout kill and keeps sleeping", async () => { + const stubDir = mkdtempSync(join(tmpdir(), "fn-7707-qmd-stub-")); + tempDirs.push(stubDir); + const rootDir = mkdtempSync(join(tmpdir(), "fn-7707-qmd-root-")); + tempDirs.push(rootDir); + writeStubbornSlowQmdStub(stubDir); + + const exitInfo = await runFixture(rootDir, stubDir); + + expect(exitInfo.stdout).toContain("qmd-search-fixture:started"); + // Once the search's child + stdio are properly unref'd, nothing else keeps the + // fixture's event loop alive, so Node exits promptly with the still-pending + // top-level `await backend.search(...)` abandoned — Node reports this as exit + // code 13 ("unsettled top-level await"), which is expected/desired here: it is + // direct proof the process did NOT wait for the SIGTERM-ignoring qmd child. + // What matters is that the process exits at all (is not null/hung) and does so + // well before the stub's 8s sleep completes. + expect(exitInfo.code).not.toBeNull(); + // The stub ignores SIGTERM and only responds to "search" after an 8s sleep; the + // fixture process must exit well before that, proving the search's child + + // stdio were unref'd rather than holding the fixture's event loop open for the + // child's full runtime after its own work (spawning + collection-add) is done. + expect(exitInfo.elapsedMs).toBeLessThan(5_000); + }, 15_000); +}); diff --git a/packages/core/src/memory-backend.ts b/packages/core/src/memory-backend.ts index 49a60e9f45..0895e848c4 100644 --- a/packages/core/src/memory-backend.ts +++ b/packages/core/src/memory-backend.ts @@ -988,9 +988,19 @@ async function searchWithQmd(rootDir: string, options: MemorySearchOptions): Pro const command = "qmd"; const limit = Math.max(1, Math.min(options.limit ?? 5, 20)); try { - const { execFile } = await import("node:child_process"); - const { promisify } = await import("node:util"); - const execFileAsync = promisify(execFile); + // FNXC:ProjectMemory 2026-07-08-00:00: + // searchWithQmd used to carry its own inline `promisify(execFile)` copy — a + // second, un-unref'd executor separate from the hardened default below — to + // spawn `qmd collection add` and `qmd search`. Even though the search is + // awaited (unlike the fire-and-forget background refresh fixed by FN-7706), a + // short-lived caller (e.g. a one-shot CLI memory search) could be held open + // for up to the 4s awaited timeout: per the documented nextTick re-ref pitfall + // (see getDefaultExecFileAsync's doc comment / .fusion/memory/MEMORY.md), + // `promisify(execFile)`'s internal stdout/stderr buffering re-refs the pipe + // handles on a later tick, so even a manual `.unref()` wouldn't have stuck. + // Reuse the same hardened, spawn-based, synchronously-unref'd executor FN-7706 + // established for the refresh path instead of carrying a second leaky copy. + const execFileAsync = await getDefaultExecFileAsync(); await ensureQmdProjectMemoryCollection(rootDir, execFileAsync); scheduleQmdProjectMemoryRefresh(rootDir); const args = buildQmdSearchArgs(rootDir, options);