From d0be3e462d3936afebc246b0cc2eacac0258643a Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 18 Jun 2026 01:19:26 -0700 Subject: [PATCH] FN-6610: harden engine test isolation recovery Harden shared test isolation seams so engine tests survive mid-run cleanup. - Recreate owned worker roots, HOME directories, and cwd before child-process launches. - Add regression coverage for tmpdir redirect, HOME, cwd, SQLite, and git config recovery. - Revalidate worktree database scratch directories before direct SQLite opens and document the rescue pattern. Files changed: docs/testing.md | 2 + packages/core/src/__test-utils__/vitest-setup.ts | 92 ++++++++++++++++++---- .../__tests__/vitest-setup-tmp-redirect.test.ts | 33 ++++++++ .../src/__tests__/executor-step-session.test.ts | 6 +- .../src/__tests__/worktree-db-hydrate.test.ts | 22 +++++- 5 files changed, 135 insertions(+), 20 deletions(-) Fusion-Task-Id: FN-6610 Fusion-Task-Lineage: 18233ee2-1dfe-4b0d-bd12-e4f5b7f9cc29 --- docs/testing.md | 2 + .../core/src/__test-utils__/vitest-setup.ts | 92 ++++++++++++++++--- .../vitest-setup-tmp-redirect.test.ts | 33 +++++++ .../__tests__/executor-step-session.test.ts | 6 +- .../src/__tests__/worktree-db-hydrate.test.ts | 22 ++++- 5 files changed, 135 insertions(+), 20 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index e87fb6c521..146c74d43e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -178,6 +178,8 @@ Legitimate legacy exceptions must be recorded in `scripts/lib/test-timeout-appea **2026-06-17 core cleanup rescue (FN-6600):** a broad `@fusion/core` timeout cluster was accompanied by `fusion-test-workers-*` `ENOTEMPTY`, while the named files passed in isolation and then under the package lane with the broad-run worker budget. The rescue hardened the shared worker-root teardown's bounded `ENOTEMPTY`/`EBUSY` retry window and added explicit cleanup-invariant coverage, then removed the same-day core quarantine entries in ledger/config lockstep after proving the unexcluded package lane. Reusable pattern: when multiple core files fail with a shared worker-root cleanup signature, fix or prove the shared cleanup seam first; only quarantine residual files after the loaded unexcluded core lane still fails without a seam fix. +**2026-06-18 engine isolation rescue (FN-6610):** a full `@fusion/engine` lane reported unrelated expectation drift, vanished-cwd/git-config errors, and SQLite `unable to open database file` failures. The reusable isolation fix is to revalidate the shared test cwd/HOME/worker-root seam at the operation boundary: subprocess wrappers recreate the owned worker root, HOME, and cwd immediately before `git`, direct SQLite setup helpers recreate their redirected `.fusion` parent before `DatabaseSync`, and regression coverage removes the redirect sink/HOME/cwd mid-test before proving `mkdtemp`, SQLite open, and git config all still work. Do not mask this class with retries, worker reductions, or timeout bumps; quarantine only residual files after the shared seam and direct-open parents are proven under package load. + **2026-06-16 rescue (FN-6514):** `packages/dashboard/app/components/__tests__/QuickEntryBox.test.tsx` was rescued before its 2026-06-30 deletion deadline. The file still caught real quick-entry behavior regressions, but it leaked jsdom descriptors for `window.innerWidth`, `window.matchMedia`, `document.visibilityState`, `URL.createObjectURL`, and `URL.revokeObjectURL`; a mobile viewport helper could leave later tests in the same dashboard backfill shard observing `innerWidth=375` and mismatched responsive assertions. The rescue removed the ledger/config quarantine entries in lockstep, captured each original `PropertyDescriptor` at module load, restored those descriptors (or deleted own properties that were originally absent) in `afterEach`, and added a guard test that mutates all rescued globals before asserting they return to their original descriptors. Reusable pattern: any test file that changes jsdom globals with `Object.defineProperty` or spies on replaceable globals must snapshot the original descriptor at the top of the file, restore it in every `afterEach`, and prove the invariant with a guard test; do not use timeout bumps, retries, worker changes, or blanket `vi.restoreAllMocks()` when module mocks depend on stable implementations. **Gate eviction:** a flake inside the merge gate cannot block all merges while red — it is evicted by removing its line from the `engine-core` allow-list (no quarantine entry needed unless it should also leave the non-blocking tier). diff --git a/packages/core/src/__test-utils__/vitest-setup.ts b/packages/core/src/__test-utils__/vitest-setup.ts index ffcb4531a3..ea53e03acc 100644 --- a/packages/core/src/__test-utils__/vitest-setup.ts +++ b/packages/core/src/__test-utils__/vitest-setup.ts @@ -219,6 +219,13 @@ const REAL_TMPDIR = (() => { return resolve(tmpdir()); } })(); +const REAL_WORKER_ROOT = (() => { + try { + return realpathSync(WORKER_ROOT); + } catch { + return resolve(WORKER_ROOT); + } +})(); const TMPDIR_REDIRECT_REGISTRY = join(WORKER_ROOT, ".redir-pids"); let tmpdirRedirectSink: string | null = null; @@ -370,7 +377,7 @@ function redirectTmpdirPrefix(prefix: T): T { return join(ensureTmpdirRedirectSink(), basename(prefix)) as T; } -function isCurrentWorkerHome(path: string | undefined): boolean { +function isWorkerHomePath(path: string | undefined): boolean { if (!path) return false; const resolved = (() => { try { @@ -379,11 +386,35 @@ function isCurrentWorkerHome(path: string | undefined): boolean { return resolve(path); } })(); - const relativeHome = relative(WORKER_ROOT, resolved); - return Boolean(relativeHome) - && !relativeHome.startsWith("..") - && !isAbsolute(relativeHome) - && basename(resolved).startsWith(TEST_HOME_PREFIX); + const workerRoots = Array.from(new Set([resolve(WORKER_ROOT), REAL_WORKER_ROOT])); + return workerRoots.some((root) => { + const relativeHome = relative(root, resolved); + return Boolean(relativeHome) + && !relativeHome.startsWith("..") + && !isAbsolute(relativeHome) + && basename(resolved).startsWith(TEST_HOME_PREFIX); + }); +} + +function isCurrentWorkerHome(path: string | undefined): boolean { + if (!isWorkerHomePath(path)) return false; + if (!existsSync(path!)) { + ensureWorkerRoot(); + mkdirSync(path!, { recursive: true }); + } + return existsSync(path!); +} + +function assignHomeEnv(tempHome: string): void { + process.env.HOME = tempHome; + process.env.USERPROFILE = tempHome; + if (process.platform === "win32") { + const match = tempHome.match(/^([A-Za-z]:)(.*)$/); + if (match) { + process.env.HOMEDRIVE = match[1]; + process.env.HOMEPATH = match[2] || "\\"; + } + } } function ensureIsolatedHome(): void { @@ -397,17 +428,13 @@ function ensureIsolatedHome(): void { FNXC:TestIsolation 2026-06-14-00:31: Nested or recursive Vitest lanes may inherit a parent worker's `fn-test-home-*` HOME value, which shares global settings/cache state across files and keeps CLI suites load-sensitive. Reuse HOME only when it belongs to this invocation's worker root; otherwise mint a fresh per-run HOME under `fusion-test-workers-*` so teardown removes it with the worker root. + + FNXC:TestIsolation 2026-06-18-07:22: + FN-6610 requires a live worker's HOME redirect to survive sibling teardown without leaking a new `fn-test-home-*` directory per subprocess. + Recreate the owned HOME path when it was swept so repeated git/config subprocesses keep one stable per-worker HOME. */ const tempHome = realpathSync(mkdtempSync(join(WORKER_ROOT, `${TEST_HOME_PREFIX}${process.pid}-`))); - process.env.HOME = tempHome; - process.env.USERPROFILE = tempHome; - if (process.platform === "win32") { - const match = tempHome.match(/^([A-Za-z]:)(.*)$/); - if (match) { - process.env.HOMEDRIVE = match[1]; - process.env.HOMEPATH = match[2] || "\\"; - } - } + assignHomeEnv(tempHome); } ensureIsolatedHome(); @@ -421,6 +448,34 @@ if (isMainThread) { process.chdir(workerTempDir); } +function ensureWorkerCwdForSubprocess(): void { + if (!isMainThread) return; + try { + originalCwd(); + return; + } catch { + // Recreate below. A child process launched while uv_cwd is invalid fails + // before its own command can run, so the subprocess seam must repair cwd. + } + + ensureWorkerRoot(); + if (!workerTempDir || !existsSync(workerTempDir)) { + workerTempDir = realpathSync(mkdtempSync(join(WORKER_ROOT, `w-${process.pid}-`))); + } + process.chdir(workerTempDir); +} + +function ensureRuntimeIsolationForSubprocess(): void { + /* + FNXC:TestIsolation 2026-06-18-07:22: + FN-6610 traced engine-lane git/config failures to live workers inheriting a swept cwd or `fn-test-home-*` directory after setup. + Revalidate cwd and HOME immediately before subprocess launch so real-git tests do not depend on setup-time paths surviving sibling teardown or recovery cleanup. + */ + ensureWorkerRoot(); + ensureIsolatedHome(); + ensureWorkerCwdForSubprocess(); +} + function installFsGuards(): void { const guardState = globalThis as typeof globalThis & { __fusionTestFsGuardInstalled?: boolean }; if (guardState.__fusionTestFsGuardInstalled) return; @@ -882,6 +937,7 @@ function installChildProcessGuards(): void { if (shouldBlockRealTestCli(commandLine)) { throw blockedCliError(commandLine); } + ensureRuntimeIsolationForSubprocess(); const proc = originalChildProcess.spawn(command, args, options); registerTrackedSubprocess(proc, commandLine); return proc; @@ -897,6 +953,7 @@ function installChildProcessGuards(): void { if (shouldBlockRealTestCli(commandLine)) { throw blockedCliError(commandLine); } + ensureRuntimeIsolationForSubprocess(); return originalChildProcess.spawnSync(command, args, options); }) as ChildProcessModule["spawnSync"]; @@ -907,6 +964,7 @@ function installChildProcessGuards(): void { if (shouldBlockRealTestCli(command)) { throw blockedCliError(command); } + ensureRuntimeIsolationForSubprocess(); return originalChildProcess.execSync(command, withDefaultTimeout(options)); }) as ChildProcessModule["execSync"]; @@ -920,6 +978,7 @@ function installChildProcessGuards(): void { if (shouldBlockRealTestCli(commandLine)) { throw blockedCliError(commandLine); } + ensureRuntimeIsolationForSubprocess(); return originalChildProcess.execFileSync(file, args, options); }) as ChildProcessModule["execFileSync"]; @@ -935,6 +994,7 @@ function installChildProcessGuards(): void { } const options = typeof optionsOrCallback === "function" ? undefined : optionsOrCallback; const callback = typeof optionsOrCallback === "function" ? optionsOrCallback : maybeCallback; + ensureRuntimeIsolationForSubprocess(); const proc = originalChildProcess.exec(command, withDefaultTimeout(options), callback); registerTrackedSubprocess(proc, command); return proc; @@ -968,6 +1028,7 @@ function installChildProcessGuards(): void { const callback = Array.isArray(argsOrOptions) ? (typeof optionsOrCallback === "function" ? optionsOrCallback : maybeCallback) : (typeof argsOrOptions === "function" ? argsOrOptions : typeof optionsOrCallback === "function" ? optionsOrCallback : maybeCallback); + ensureRuntimeIsolationForSubprocess(); const proc = originalChildProcess.execFile(file, args, withDefaultTimeout(options), callback); registerTrackedSubprocess(proc, commandLine); return proc; @@ -996,6 +1057,7 @@ function installChildProcessGuards(): void { if (shouldBlockRealTestCli(commandLine)) { throw blockedCliError(commandLine); } + ensureRuntimeIsolationForSubprocess(); const proc = originalChildProcess.fork(modulePath, args, options); registerTrackedSubprocess(proc, commandLine); return proc; diff --git a/packages/core/src/__tests__/vitest-setup-tmp-redirect.test.ts b/packages/core/src/__tests__/vitest-setup-tmp-redirect.test.ts index 4eaa544fbf..1042892080 100644 --- a/packages/core/src/__tests__/vitest-setup-tmp-redirect.test.ts +++ b/packages/core/src/__tests__/vitest-setup-tmp-redirect.test.ts @@ -1,9 +1,11 @@ +import { execSync } from "node:child_process"; import { existsSync, mkdtempSync, mkdirSync, rmSync, realpathSync, writeFileSync } from "node:fs"; import { mkdtemp } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join, sep } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { __fusionTmpdirRedirectTestHooks } from "../__test-utils__/vitest-setup"; +import { DatabaseSync } from "../sqlite-adapter.js"; const createdPaths: string[] = []; @@ -95,6 +97,37 @@ describe("vitest setup tmpdir mkdtemp redirect", () => { expect(existsSync(sink)).toBe(true); }); + it("revalidates cwd, HOME, tmpdir redirect, and SQLite opens after mid-run cleanup", () => { + const originalHome = process.env.HOME; + expect(originalHome).toBeTruthy(); + expect(existsSync(originalHome!)).toBe(true); + + const sink = __fusionTmpdirRedirectTestHooks.sinkForPid(process.pid); + const doomedCwd = remember(mkdtempSync(join(tmpdir(), "fn-redirect-cwd-"))); + process.chdir(doomedCwd); + rmSync(sink, { recursive: true, force: true }); + rmSync(originalHome!, { recursive: true, force: true }); + expect(existsSync(sink)).toBe(false); + expect(existsSync(originalHome!)).toBe(false); + + const sqliteProject = remember(mkdtempSync(join(tmpdir(), "fn-redirect-sqlite-"))); + const fusionDir = join(sqliteProject, ".fusion"); + mkdirSync(fusionDir, { recursive: true }); + const db = new DatabaseSync(join(fusionDir, "fusion.db")); + db.exec("CREATE TABLE smoke (id TEXT PRIMARY KEY)"); + db.prepare("INSERT INTO smoke (id) VALUES (?)").run("ok"); + expect(db.prepare("SELECT id FROM smoke").get()).toEqual({ id: "ok" }); + db.close(); + + const output = execSync("git config --global user.name fusion-test && git config --global --get user.name && pwd", { encoding: "utf8" }); + + expect(output).toContain("fusion-test"); + expect(output).toContain(process.env.FUSION_TEST_WORKER_ROOT!); + expect(process.env.HOME).toBe(originalHome); + expect(existsSync(process.env.HOME!)).toBe(true); + expect(existsSync(sink)).toBe(true); + }); + it("sweeps only dead redirect sinks and preserves current or alive pids", () => { const { registryPath, resetSweepForTest, sinkForPid, sweepDeadTmpdirRedirectSinks } = __fusionTmpdirRedirectTestHooks; const currentSink = rememberDir(sinkForPid(process.pid)); diff --git a/packages/engine/src/__tests__/executor-step-session.test.ts b/packages/engine/src/__tests__/executor-step-session.test.ts index dcbc0e0e1e..e0263db3b1 100644 --- a/packages/engine/src/__tests__/executor-step-session.test.ts +++ b/packages/engine/src/__tests__/executor-step-session.test.ts @@ -115,7 +115,11 @@ describe("Workflow Steps Execution", () => { // Should have been called four times: initial + 3 retries expect(mockedCreateFnAgent).toHaveBeenCalledTimes(4); - // Retries still didn't call fn_task_done, so it fails and requeues immediately. + /* + FNXC:EngineTests 2026-06-18-07:22: + FN-6610 confirmed the intended executor.ts no-fn_task_done exhaustion behavior: after three in-session retries, tasks with remaining requeue budget return to todo with progress preserved; only exhausted requeue budget parks them in review. + Keep these expectations aligned with Executor.execute()'s MAX_TASK_DONE_REQUEUE_RETRIES branch rather than treating the first in-session exhaustion as terminal. + */ expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "queued", error: null, diff --git a/packages/engine/src/__tests__/worktree-db-hydrate.test.ts b/packages/engine/src/__tests__/worktree-db-hydrate.test.ts index 39c6694ff9..d0241e9f3d 100644 --- a/packages/engine/src/__tests__/worktree-db-hydrate.test.ts +++ b/packages/engine/src/__tests__/worktree-db-hydrate.test.ts @@ -9,16 +9,29 @@ import { createHash } from "node:crypto"; import { Database, DatabaseSync } from "@fusion/core"; import { hydrateWorktreeDb } from "../worktree-db-hydrate.js"; +function ensureProjectFusionDir(projectDir: string): void { + /* + FNXC:EngineTests 2026-06-18-07:22: + FN-6610 isolated this suite's SQLite-open symptom to direct `Database` / `DatabaseSync` setup calls on paths minted through the shared tmpdir redirect, not to a subprocess cwd/HOME path. + Revalidate the redirected project scratch directory immediately before test SQLite opens so sibling worker-root cleanup cannot leave `new DatabaseSync(...)` pointed at a swept parent. + */ + const fusionDir = join(projectDir, ".fusion"); + mkdirSync(fusionDir, { recursive: true }); + if (!existsSync(join(fusionDir, "fusion.db"))) { + const db = new Database(fusionDir); + db.init(); + db.close(); + } +} + function makeProject(prefix: string): string { const dir = mkdtempSync(join(tmpdir(), prefix)); - mkdirSync(join(dir, ".fusion"), { recursive: true }); - const db = new Database(join(dir, ".fusion")); - db.init(); - db.close(); + ensureProjectFusionDir(dir); return dir; } function insertTask(projectDir: string, id: string, deletedAt: string | null = null): void { + ensureProjectFusionDir(projectDir); const db = new DatabaseSync(join(projectDir, ".fusion", "fusion.db")); const now = new Date().toISOString(); db.prepare( @@ -28,6 +41,7 @@ function insertTask(projectDir: string, id: string, deletedAt: string | null = n } function insertDoc(projectDir: string, taskId: string): void { + ensureProjectFusionDir(projectDir); const db = new DatabaseSync(join(projectDir, ".fusion", "fusion.db")); const now = new Date().toISOString(); db.prepare("INSERT OR REPLACE INTO task_documents (id, taskId, key, content, revision, author, metadata, createdAt, updatedAt) VALUES (?, ?, 'notes', 'hello', 1, 'test', NULL, ?, ?)")