diff --git a/changelog.d/fixes/13290-shared-temp-data-dir-teardown.md b/changelog.d/fixes/13290-shared-temp-data-dir-teardown.md new file mode 100644 index 0000000000..1c920f5300 --- /dev/null +++ b/changelog.d/fixes/13290-shared-temp-data-dir-teardown.md @@ -0,0 +1 @@ +- Fixed tests leaving temp `DATA_DIR` folders behind on Windows by closing the SQLite handle before removing the directory (#13290). diff --git a/tests/_setup/tempDataDir.ts b/tests/_setup/tempDataDir.ts new file mode 100644 index 0000000000..23045cbc8c --- /dev/null +++ b/tests/_setup/tempDataDir.ts @@ -0,0 +1,86 @@ +// Shared teardown for tests that own their own temporary DATA_DIR. +// +// Why this exists (#13290): a test that opens the SQLite DB and then removes its +// temp DATA_DIR in `test.after()` fails on Windows with EPERM, because Windows +// refuses to remove a directory while any handle on it is still open. The tell is +// `storage.sqlite-shm` / `-wal` left behind next to the database file. +// +// `maxRetries` cannot help: nothing ever closes the connection, so every retry +// hits the same open handle. The `process.on("exit")` fallback in isolateDataDir.ts +// does not cover these tests either — it only runs when the test does NOT set +// DATA_DIR itself, which is exactly the case that fails here. +// +// The cleanup is deliberately best-effort. A leftover temp directory is a disk +// nuisance the OS eventually reaps; a throwing teardown is a red test that hides +// a green assertion. Failing to remove the directory must never fail the test. +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +/** + * Close the SQLite singleton if the DB module was ever loaded. + * + * Imported lazily and defensively: most callers never touch the database, and a + * static import would drag the whole DB layer into every test that only needs a + * scratch directory. If the module was never loaded, there is nothing to close. + */ +async function closeDbIfOpen(): Promise { + try { + const core = await import("../../src/lib/db/core.ts"); + core.resetDbInstance?.(); + } catch { + // The DB module is absent, failed to load, or was never initialised. + // Nothing to close — cleanup continues. + } +} + +/** Remove a directory, swallowing any failure. Never throws. */ +function removeQuietly(dir: string): void { + try { + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch { + // Best-effort: see the file header. A leftover temp dir must not fail a test. + } +} + +/** + * Create a temporary DATA_DIR and point `process.env.DATA_DIR` at it. + * + * Returns the directory path plus a `cleanup()` that closes the database before + * removing it. Pass `cleanup` straight to `test.after()`: + * + * ```ts + * const { dir: TEST_DATA_DIR, cleanup } = createTempDataDir("my-suite-"); + * test.after(cleanup); + * ``` + */ +export function createTempDataDir(prefix = "omniroute-test-"): { + dir: string; + cleanup: () => Promise; +} { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + process.env.DATA_DIR = dir; + + return { + dir, + cleanup: async () => { + await closeDbIfOpen(); + removeQuietly(dir); + }, + }; +} + +/** + * Teardown for a temp DATA_DIR that was created by hand. + * + * For existing tests that already have their own `mkdtempSync` call and only need + * the close-then-remove ordering fixed: + * + * ```ts + * test.after(() => cleanupTempDataDir(TEST_DATA_DIR)); + * ``` + */ +export async function cleanupTempDataDir(dir: string): Promise { + await closeDbIfOpen(); + removeQuietly(dir); +} diff --git a/tests/integration/agent-bridge-bypass-flow.test.ts b/tests/integration/agent-bridge-bypass-flow.test.ts index e16f208658..d4e731c5cb 100644 --- a/tests/integration/agent-bridge-bypass-flow.test.ts +++ b/tests/integration/agent-bridge-bypass-flow.test.ts @@ -11,6 +11,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ab-bypass-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -33,12 +34,8 @@ test.beforeEach(() => { seedDefaultBypassPatterns(DEFAULT_PATTERNS); }); -test.after(() => { - try { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } catch { - /* noop */ - } +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); // ── POST patterns ────────────────────────────────────────────────────────── diff --git a/tests/integration/monitoring-health-cache.test.ts b/tests/integration/monitoring-health-cache.test.ts index 2b38924ff1..52dc912137 100644 --- a/tests/integration/monitoring-health-cache.test.ts +++ b/tests/integration/monitoring-health-cache.test.ts @@ -13,6 +13,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-health-cache-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -76,3 +77,7 @@ test("DELETE (circuit-breaker reset) invalidates the cache immediately", async ( const t2 = await healthTimestamp(); assert.notEqual(t2, t1, "a GET right after DELETE must rebuild (cache invalidated)"); }); + +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); +}); diff --git a/tests/integration/presets-dashboard-auth.test.ts b/tests/integration/presets-dashboard-auth.test.ts index 36499c64d9..8d165a5309 100644 --- a/tests/integration/presets-dashboard-auth.test.ts +++ b/tests/integration/presets-dashboard-auth.test.ts @@ -14,6 +14,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-presets-auth-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -50,3 +51,7 @@ test("REQUIRE_API_KEY=true: a tampered/invalid session token -> 401", async () = const res = await GET(new Request(BASE, { headers: { cookie: "auth_token=not.a.valid.jwt" } })); assert.equal(res.status, 401); }); + +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); +}); diff --git a/tests/unit/8510-adobe-firefly-edits-route.test.ts b/tests/unit/8510-adobe-firefly-edits-route.test.ts index edbb8c4b24..eda5b4db23 100644 --- a/tests/unit/8510-adobe-firefly-edits-route.test.ts +++ b/tests/unit/8510-adobe-firefly-edits-route.test.ts @@ -9,6 +9,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-adobe-firefly-edits-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -36,7 +37,7 @@ async function resetStorage() { globalThis.fetch = originalFetch; apiKeysDb.resetApiKeyState(); core.resetDbInstance(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); v1ModelsCatalog.__resetCatalogBuilderRunsForTest(); } @@ -80,11 +81,11 @@ test.beforeEach(async () => { await resetStorage(); }); -test.after(() => { +test.after(async () => { globalThis.fetch = originalFetch; apiKeysDb.resetApiKeyState(); core.resetDbInstance(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); test("#8510 v1 image edit POST uploads Adobe Firefly reference images and dispatches referenceBlobs", async () => { diff --git a/tests/unit/agentbridge-mitm-router-key-6403.test.ts b/tests/unit/agentbridge-mitm-router-key-6403.test.ts index 5f2009abaf..1755598b70 100644 --- a/tests/unit/agentbridge-mitm-router-key-6403.test.ts +++ b/tests/unit/agentbridge-mitm-router-key-6403.test.ts @@ -19,6 +19,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-ab-routerkey-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -41,13 +42,9 @@ test.beforeEach(() => { delete process.env.ROUTER_API_KEY; }); -test.after(() => { +test.after(async () => { delete process.env.ROUTER_API_KEY; - try { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); - } catch { - /* noop */ - } + await cleanupTempDataDir(TEST_DATA_DIR); }); test("resolveRouterApiKey: explicit apiKey field always wins", async () => { diff --git a/tests/unit/agenticConversations.test.ts b/tests/unit/agenticConversations.test.ts index 47e5fba6e2..6f462b8eac 100644 --- a/tests/unit/agenticConversations.test.ts +++ b/tests/unit/agenticConversations.test.ts @@ -8,6 +8,7 @@ import assert from "node:assert/strict"; import { mkdtempSync } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; process.env.DATA_DIR = mkdtempSync(join(tmpdir(), "omniroute-agentic-conv-db-")); process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "agentic-conversations-test-secret"; @@ -297,3 +298,7 @@ test("resolveCallLogIdsByCorrelationIds returns an empty map for an empty/all-fa assert.equal(resolveCallLogIdsByCorrelationIds([]).size, 0); assert.equal(resolveCallLogIdsByCorrelationIds(["", ""]).size, 0); }); + +test.after(async () => { + await cleanupTempDataDir(process.env.DATA_DIR!); +}); diff --git a/tests/unit/antigravity-family-auto-sync-default.test.ts b/tests/unit/antigravity-family-auto-sync-default.test.ts index 5422dc1606..577e290f8a 100644 --- a/tests/unit/antigravity-family-auto-sync-default.test.ts +++ b/tests/unit/antigravity-family-auto-sync-default.test.ts @@ -15,6 +15,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; type Row = Record; @@ -130,3 +131,7 @@ test("re-import enables autoSync when the existing row never had it", async () = assert.equal(asRecord((await findRowByEmail(email))?.providerSpecificData).autoSync, true); }); + +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); +}); diff --git a/tests/unit/api-key-lifecycle.test.ts b/tests/unit/api-key-lifecycle.test.ts index 5cb73eb767..b099e91845 100644 --- a/tests/unit/api-key-lifecycle.test.ts +++ b/tests/unit/api-key-lifecycle.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-apikey-lifecycle-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -27,8 +28,8 @@ test.beforeEach(() => { reset(); }); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); if (ORIGINAL_OMNIROUTE_API_KEY === undefined) delete process.env.OMNIROUTE_API_KEY; else process.env.OMNIROUTE_API_KEY = ORIGINAL_OMNIROUTE_API_KEY; if (ORIGINAL_ROUTER_API_KEY === undefined) delete process.env.ROUTER_API_KEY; diff --git a/tests/unit/api-key-regeneration.test.ts b/tests/unit/api-key-regeneration.test.ts index 920a835902..b0b3a835c4 100644 --- a/tests/unit/api-key-regeneration.test.ts +++ b/tests/unit/api-key-regeneration.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-apikey-regen-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -24,8 +25,8 @@ test.beforeEach(() => { reset(); }); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); test("regenerateApiKey creates a new key and invalidates the old one", async () => { diff --git a/tests/unit/api/compression/rtk-learn-discover-routes.test.ts b/tests/unit/api/compression/rtk-learn-discover-routes.test.ts index 6c9d020bd0..0cabfb0321 100644 --- a/tests/unit/api/compression/rtk-learn-discover-routes.test.ts +++ b/tests/unit/api/compression/rtk-learn-discover-routes.test.ts @@ -17,6 +17,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../../../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-rtk-ld-routes-")); const ORIGINAL_DATA_DIR = process.env.DATA_DIR; @@ -43,13 +44,16 @@ function get(url: string): Request { return new Request(url, { method: "GET" }); } -test.beforeEach(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.beforeEach(async () => { + // #13290: the DB from the previous test is still open here, and on Windows an + // open SQLite handle (plus its -wal/-shm) makes rmSync fail with EPERM before + // the test body even runs. Close it first, then recreate the directory. + await cleanupTempDataDir(TEST_DATA_DIR); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); }); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; else process.env.DATA_DIR = ORIGINAL_DATA_DIR; if (ORIGINAL_INITIAL_PASSWORD !== undefined) diff --git a/tests/unit/authz/client-api-policy-fallback.test.ts b/tests/unit/authz/client-api-policy-fallback.test.ts index 163e3e85f2..f41d2ed7a1 100644 --- a/tests/unit/authz/client-api-policy-fallback.test.ts +++ b/tests/unit/authz/client-api-policy-fallback.test.ts @@ -11,6 +11,7 @@ import test from "node:test"; import assert from "node:assert/strict"; import Module from "node:module"; +import { cleanupTempDataDir } from "../../_setup/tempDataDir.ts"; // ─── Mock validateApiKey via require interception (so the dynamic import in // the policy module returns our stub instead of hitting the real DB module) ─ @@ -59,13 +60,13 @@ fs.writeFileSync( (globalThis as unknown as { __mockValidateApiKey: ValidateFn }).__mockValidateApiKey = (key) => mockValidateApiKey(key); -test.after(() => { +test.after(async () => { try { fs.unlinkSync(STUB_PATH); } catch { /* ignore */ } - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; else process.env.DATA_DIR = ORIGINAL_DATA_DIR; }); diff --git a/tests/unit/authz/management-policy.test.ts b/tests/unit/authz/management-policy.test.ts index aedd82d81d..9a816c6bd1 100644 --- a/tests/unit/authz/management-policy.test.ts +++ b/tests/unit/authz/management-policy.test.ts @@ -4,6 +4,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { SignJWT } from "jose"; +import { cleanupTempDataDir } from "../../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-mgmt-policy-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -34,8 +35,8 @@ test.beforeEach(() => { reset(); }); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); if (ORIGINAL_JWT === undefined) delete process.env.JWT_SECRET; else process.env.JWT_SECRET = ORIGINAL_JWT; if (ORIGINAL_INITIAL === undefined) delete process.env.INITIAL_PASSWORD; diff --git a/tests/unit/batch_api.test.ts b/tests/unit/batch_api.test.ts index fa65b1e952..8808704195 100644 --- a/tests/unit/batch_api.test.ts +++ b/tests/unit/batch_api.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-batch-api-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -1345,3 +1346,7 @@ test("getTerminalBatches returns only terminal statuses ordered oldest first", a ); } }); + +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); +}); diff --git a/tests/unit/chatcore-reasoning-cache-write-guard.test.ts b/tests/unit/chatcore-reasoning-cache-write-guard.test.ts index 7113c8298e..6c99dedc33 100644 --- a/tests/unit/chatcore-reasoning-cache-write-guard.test.ts +++ b/tests/unit/chatcore-reasoning-cache-write-guard.test.ts @@ -18,6 +18,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync( path.join(os.tmpdir(), "omniroute-chatcore-reasoning-cache-write-guard-") @@ -161,14 +162,14 @@ async function invokeChatCoreStreaming(provider: string, model: string, toolCall } } -test.after(() => { +test.after(async () => { try { core.resetDbInstance(); } catch {} try { clearReasoningCacheAll(); } catch {} - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); test("non-streaming: a replay provider (xiaomi-mimo) populates the reasoning cache", async () => { diff --git a/tests/unit/claudeAuthImport-bootstrap-headers-10144.test.ts b/tests/unit/claudeAuthImport-bootstrap-headers-10144.test.ts index ccdd53010e..116e7b96b4 100644 --- a/tests/unit/claudeAuthImport-bootstrap-headers-10144.test.ts +++ b/tests/unit/claudeAuthImport-bootstrap-headers-10144.test.ts @@ -17,6 +17,7 @@ process.env.APP_LOG_TO_FILE = "false"; const { createConnectionFromAuthFile, enrichWithBootstrap, parseAndValidateClaudeAuth } = await import("../../src/lib/oauth/utils/claudeAuthImport.ts"); import { getClaudeCodeUserAgent } from "../../src/shared/constants/claudeCodeClient.ts"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const originalFetch = globalThis.fetch; @@ -24,8 +25,8 @@ test.afterEach(() => { globalThis.fetch = originalFetch; }); -test.after(() => { - fs.rmSync(testDataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(testDataDir); }); test("real enrichWithBootstrap sends the required CLI headers", async () => { diff --git a/tests/unit/codex-fingerprint-seed-persistence.test.ts b/tests/unit/codex-fingerprint-seed-persistence.test.ts index 82b88b17b9..3794ae4998 100644 --- a/tests/unit/codex-fingerprint-seed-persistence.test.ts +++ b/tests/unit/codex-fingerprint-seed-persistence.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-codex-seed-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -19,8 +20,8 @@ async function resetStorage() { } beforeEach(resetStorage); -after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); async function createCodexOAuthConnection(providerSpecificData?: Record) { diff --git a/tests/unit/compression/rtk-mcp-tools.test.ts b/tests/unit/compression/rtk-mcp-tools.test.ts index b79af21907..f44f3ed032 100644 --- a/tests/unit/compression/rtk-mcp-tools.test.ts +++ b/tests/unit/compression/rtk-mcp-tools.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../../_setup/tempDataDir.ts"; // T07 — omniroute_rtk_discover / omniroute_rtk_learn MCP tools (read-only; audited). @@ -38,16 +39,16 @@ function seedSamples() { }); } -beforeEach(() => { +beforeEach(async () => { core.resetDbInstance(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); core.getDbInstance(); // run migrations → mcp_tool_audit table exists }); -after(() => { +after(async () => { core.resetDbInstance(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); describe("RTK MCP tools (T07)", () => { diff --git a/tests/unit/db-adapters/driverFactory.test.ts b/tests/unit/db-adapters/driverFactory.test.ts index e4383ea955..4623ae96b5 100644 --- a/tests/unit/db-adapters/driverFactory.test.ts +++ b/tests/unit/db-adapters/driverFactory.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../../_setup/tempDataDir.ts"; import { createRequire } from "node:module"; import type * as NodePath from "node:path"; import { runtimeRequire } from "../../../src/lib/db/adapters/runtimeRequire.ts"; @@ -32,7 +33,7 @@ function forceNodeSqlite() { function createTempDatabasePath(t: TestContext) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-node-sqlite-")); const databasePath = path.join(dir, "database.sqlite"); - t.after(() => fs.rmSync(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 })); + t.after(async () => await cleanupTempDataDir(dir)); return databasePath; } diff --git a/tests/unit/executor-map-golden.test.ts b/tests/unit/executor-map-golden.test.ts index b3533e267b..6ad5007d3e 100644 --- a/tests/unit/executor-map-golden.test.ts +++ b/tests/unit/executor-map-golden.test.ts @@ -4,6 +4,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { fileURLToPath } from "node:url"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; // R0.3 GOLDEN LOCK (characterization BEFORE the ExecutorRegistry refactor): // freeze the full provider-id → executor mapping of open-sse/executors/index.ts — @@ -24,8 +25,8 @@ const { PROVIDERS } = await import("../../open-sse/config/constants.ts"); const { SEARCH_PROVIDERS } = await import("../../open-sse/config/searchRegistry.ts"); const { goldenSnapshot } = await import("../helpers/goldenSnapshot.ts"); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); // The specialized keys are not exported; enumerate them through the public diff --git a/tests/unit/executor-registry.test.ts b/tests/unit/executor-registry.test.ts index 6c381acff1..85ea5e26d9 100644 --- a/tests/unit/executor-registry.test.ts +++ b/tests/unit/executor-registry.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; // R0.3 — unit tests for the ExecutorRegistry seam itself (registration // semantics + wiring of the built-ins). Behavior parity of the full map is @@ -17,9 +18,7 @@ const { getExecutor, hasSpecializedExecutor, BaseExecutor, DefaultExecutor } = await import("../../open-sse/executors/index.ts"); const { getDefaultExecutor } = await import("../../open-sse/executors/defaultResolver.ts"); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); -}); +test.after(() => cleanupTempDataDir(TEST_DATA_DIR)); test("built-ins are registered at module load and resolve through the registry", async () => { const aliases = listExecutorAliases(); diff --git a/tests/unit/guardrails/vision-bridge-callmodel.test.ts b/tests/unit/guardrails/vision-bridge-callmodel.test.ts index f3041c00c2..d1f9d936f6 100644 --- a/tests/unit/guardrails/vision-bridge-callmodel.test.ts +++ b/tests/unit/guardrails/vision-bridge-callmodel.test.ts @@ -13,6 +13,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-vision-bridge-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -39,9 +40,9 @@ await createProviderConnection({ const originalFetch = globalThis.fetch; -test.after(() => { +test.after(async () => { globalThis.fetch = originalFetch; - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); test.afterEach(() => { diff --git a/tests/unit/issue-agent-route-execution.test.ts b/tests/unit/issue-agent-route-execution.test.ts index 1d90b6d9ab..bce085a021 100644 --- a/tests/unit/issue-agent-route-execution.test.ts +++ b/tests/unit/issue-agent-route-execution.test.ts @@ -44,6 +44,13 @@ test.afterEach(() => { delete process.env.OMNIROUTE_ISSUE_AGENT_ENABLED; }); +// Close the SQLite handle the route opened. Without this the connection stays +// open until process exit, and the isolateDataDir cleanup hook then fails with +// EPERM on Windows, leaving storage.sqlite(-shm/-wal) behind for the next run. +test.after(() => { + core.resetDbInstance(); +}); + test("issue-agent live triage traverses the normal chat-completions POST route", async () => { await seedOpenAiConnection(); const fetchCalls: Array<{ url: string; init: RequestInit }> = []; diff --git a/tests/unit/microsoft-designer-web-image-handler-block.test.ts b/tests/unit/microsoft-designer-web-image-handler-block.test.ts index 66ff9734c2..4ad8e1e5e5 100644 --- a/tests/unit/microsoft-designer-web-image-handler-block.test.ts +++ b/tests/unit/microsoft-designer-web-image-handler-block.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import test from "node:test"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-designer-image-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -10,8 +11,8 @@ process.env.DISABLE_SQLITE_AUTO_BACKUP = "true"; const { handleImageGeneration } = await import("../../open-sse/handlers/imageGeneration.ts"); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); test("image handler blocks exact retired providers before any upstream fetch", async () => { diff --git a/tests/unit/model-catalog-cache-swr-8728.test.ts b/tests/unit/model-catalog-cache-swr-8728.test.ts index 464986347f..1b7855b6d8 100644 --- a/tests/unit/model-catalog-cache-swr-8728.test.ts +++ b/tests/unit/model-catalog-cache-swr-8728.test.ts @@ -20,6 +20,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import test from "node:test"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-catalog-cache-8728-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -52,8 +53,8 @@ test.beforeEach(() => { catalogCache.__resetCatalogBuilderRunsForTest(); }); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); test("the SWR window is a bounded constant, not an unbounded accessor", () => { diff --git a/tests/unit/playground-key-policy-3503.test.ts b/tests/unit/playground-key-policy-3503.test.ts index fce4bb6446..e186a2c5cd 100644 --- a/tests/unit/playground-key-policy-3503.test.ts +++ b/tests/unit/playground-key-policy-3503.test.ts @@ -15,6 +15,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { SignJWT } from "jose"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-playground-key-3503-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -47,8 +48,8 @@ function req(headers: Record) { } as unknown as Request; } -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); test("#3503 — authenticated session + key-id header resolves the key secret server-side", async () => { diff --git a/tests/unit/provider-health-matrix.test.ts b/tests/unit/provider-health-matrix.test.ts index d3509935da..28dc9dfcfe 100644 --- a/tests/unit/provider-health-matrix.test.ts +++ b/tests/unit/provider-health-matrix.test.ts @@ -5,6 +5,7 @@ import os from "node:os"; import path from "node:path"; import { makeManagementSessionRequest } from "../helpers/managementSession.ts"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-health-matrix-")); const ORIGINAL_DATA_DIR = process.env.DATA_DIR; @@ -55,7 +56,7 @@ test.beforeEach(async () => { test.after(async () => { await resetStorage(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); if (ORIGINAL_DATA_DIR === undefined) delete process.env.DATA_DIR; else process.env.DATA_DIR = ORIGINAL_DATA_DIR; diff --git a/tests/unit/provider-limits-local-apikey-sync-spacing.test.ts b/tests/unit/provider-limits-local-apikey-sync-spacing.test.ts index c3bc8e9efa..f20ce61210 100644 --- a/tests/unit/provider-limits-local-apikey-sync-spacing.test.ts +++ b/tests/unit/provider-limits-local-apikey-sync-spacing.test.ts @@ -13,6 +13,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-apikey-spacing-sync-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -32,10 +33,10 @@ test.beforeEach(() => { delete process.env.PROVIDER_LIMITS_SYNC_SPACING_MS; }); -test.after(() => { +test.after(async () => { globalThis.fetch = originalFetch; delete process.env.PROVIDER_LIMITS_SYNC_SPACING_MS; - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); async function createGlmApiKeyConnection(i: number) { diff --git a/tests/unit/provider-limits-oauth-sequential-sync.test.ts b/tests/unit/provider-limits-oauth-sequential-sync.test.ts index 075b226501..1b97d74fde 100644 --- a/tests/unit/provider-limits-oauth-sequential-sync.test.ts +++ b/tests/unit/provider-limits-oauth-sequential-sync.test.ts @@ -14,6 +14,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-oauth-seq-sync-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -33,9 +34,9 @@ test.beforeEach(() => { fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); }); -test.after(() => { +test.after(async () => { globalThis.fetch = originalFetch; - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); async function createClaudeOAuth(i: number) { diff --git a/tests/unit/provider-limits-rotating-expired-guard.test.ts b/tests/unit/provider-limits-rotating-expired-guard.test.ts index 74ad93f080..beda4c7014 100644 --- a/tests/unit/provider-limits-rotating-expired-guard.test.ts +++ b/tests/unit/provider-limits-rotating-expired-guard.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; // providerLimits.ts touches the DB singleton at import time; give it a scratch dir. const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-rotating-expired-guard-")); @@ -12,8 +13,8 @@ process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "rotating-expired-gua const { quotaPathShouldMarkExpired, shouldAttemptRotatingRefresh } = await import("../../src/lib/usage/providerLimits.ts"); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); // Regression: the quota sync reuses a rotating provider's (possibly expired) diff --git a/tests/unit/provider-window-costs.test.ts b/tests/unit/provider-window-costs.test.ts index 8abc7a5a56..bcfdbb2c30 100644 --- a/tests/unit/provider-window-costs.test.ts +++ b/tests/unit/provider-window-costs.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-provider-costs-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -23,7 +24,7 @@ async function resetStorage() { core.resetDbInstance(); apiKeys.resetApiKeyState(); costRules.resetCostData(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); } @@ -31,11 +32,11 @@ test.beforeEach(async () => { await resetStorage(); }); -test.after(() => { +test.after(async () => { core.resetDbInstance(); apiKeys.resetApiKeyState(); costRules.resetCostData(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); test("Codex provider window costs use the weekly reset window and API key USD limit", async () => { diff --git a/tests/unit/proxy-fallback-ssrf.test.ts b/tests/unit/proxy-fallback-ssrf.test.ts index 49c904efc6..49d5d341a8 100644 --- a/tests/unit/proxy-fallback-ssrf.test.ts +++ b/tests/unit/proxy-fallback-ssrf.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; // Isolate DATA_DIR before importing validation.ts (it initializes the DB on load) // so the test never touches the developer's real ~/.omniroute database. @@ -12,8 +13,8 @@ process.env.DATA_DIR = TEST_DATA_DIR; const { isRetryableProxyTarget } = await import("../../src/lib/providers/validation.ts"); const { isPrivateHost } = await import("../../src/shared/network/outboundUrlGuard.ts"); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); /** diff --git a/tests/unit/reasoning-routing.test.ts b/tests/unit/reasoning-routing.test.ts index fc303ccc68..b5d024f3c5 100644 --- a/tests/unit/reasoning-routing.test.ts +++ b/tests/unit/reasoning-routing.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-reasoning-routing-")); process.env.DATA_DIR = TEST_DATA_DIR; @@ -55,7 +56,7 @@ test.beforeEach(resetStorage); test.after(async () => { await resetStorage(); - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + await cleanupTempDataDir(TEST_DATA_DIR); }); test("reasoning intent distinguishes missing, discrete effort, toggle, and budget-only signals", () => { diff --git a/tests/unit/responses-parse-once-4041.test.ts b/tests/unit/responses-parse-once-4041.test.ts index 27d41046cc..6f91cf1074 100644 --- a/tests/unit/responses-parse-once-4041.test.ts +++ b/tests/unit/responses-parse-once-4041.test.ts @@ -3,10 +3,11 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-responses-parse-once-")); process.env.DATA_DIR = dataDir; -after(() => fs.rmSync(dataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 })); +after(async () => await cleanupTempDataDir(dataDir)); // #4041: AI routes must parse each JSON body at most once and thread the parsed value // through model resolution and handleChat. /v1/responses now parses after raw-body admission; diff --git a/tests/unit/responses-route-early-keepalive-wiring.test.ts b/tests/unit/responses-route-early-keepalive-wiring.test.ts index 25be2970bb..a9f21f0e47 100644 --- a/tests/unit/responses-route-early-keepalive-wiring.test.ts +++ b/tests/unit/responses-route-early-keepalive-wiring.test.ts @@ -3,12 +3,13 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const routeSource = fs.readFileSync("src/app/api/v1/responses/route.ts", "utf8"); const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-responses-route-test-")); process.env.DATA_DIR = dataDir; process.env.REQUIRE_API_KEY = "false"; -after(() => fs.rmSync(dataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 })); +after(async () => await cleanupTempDataDir(dataDir)); test("Responses route wires dual-cadence neutral keepalives", () => { assert.match( diff --git a/tests/unit/settings-debugmode-default.test.ts b/tests/unit/settings-debugmode-default.test.ts index 2ded41c5a8..16e450c158 100644 --- a/tests/unit/settings-debugmode-default.test.ts +++ b/tests/unit/settings-debugmode-default.test.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; // Isolated DATA_DIR so persisted settings rows don't mask the default. const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-settings-debugmode-")); @@ -21,6 +22,6 @@ test("logToolSources defaults to false", async () => { assert.equal(settings.logToolSources, false, "logToolSources should default to false"); }); -test.after(() => { - fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +test.after(async () => { + await cleanupTempDataDir(TEST_DATA_DIR); }); diff --git a/tests/unit/temp-data-dir-teardown-13290.test.ts b/tests/unit/temp-data-dir-teardown-13290.test.ts new file mode 100644 index 0000000000..4c997e7163 --- /dev/null +++ b/tests/unit/temp-data-dir-teardown-13290.test.ts @@ -0,0 +1,54 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { createTempDataDir, cleanupTempDataDir } from "../_setup/tempDataDir.ts"; + +// #13290 — teardown that removes a temp DATA_DIR must close the SQLite handle +// first, or Windows refuses to remove the directory (EPERM). + +test("createTempDataDir points DATA_DIR at a fresh directory", () => { + const previous = process.env.DATA_DIR; + const { dir, cleanup } = createTempDataDir("omniroute-tempdatadir-test-"); + try { + assert.equal(process.env.DATA_DIR, dir); + assert.ok(fs.existsSync(dir), "directory must exist"); + } finally { + void cleanup(); + process.env.DATA_DIR = previous; + } +}); + +test("cleanup removes the directory after the database has been opened", async () => { + const previous = process.env.DATA_DIR; + const { dir, cleanup } = createTempDataDir("omniroute-tempdatadir-db-"); + + try { + // Open the DB so a real SQLite handle exists inside the directory. This is the + // exact shape that fails with EPERM when teardown skips the close. + const core = await import("../../src/lib/db/core.ts"); + await core.ensureDbInitialized(); + core.getDbInstance(); + + const sidecars = fs.readdirSync(dir).filter((f) => f.startsWith("storage.sqlite")); + assert.ok( + sidecars.length > 0, + "the database must actually be open for this test to mean anything" + ); + + await cleanup(); + + assert.equal(fs.existsSync(dir), false, "cleanup must remove the directory"); + } finally { + process.env.DATA_DIR = previous; + } +}); + +test("cleanup never throws when the directory is already gone", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-tempdatadir-missing-")); + fs.rmSync(dir, { recursive: true, force: true }); + + await assert.doesNotReject(() => cleanupTempDataDir(dir)); +}); diff --git a/tests/unit/zcode-executor.test.ts b/tests/unit/zcode-executor.test.ts index 68e450490b..e4a3d4092f 100644 --- a/tests/unit/zcode-executor.test.ts +++ b/tests/unit/zcode-executor.test.ts @@ -3,14 +3,13 @@ import assert from "node:assert/strict"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { cleanupTempDataDir } from "../_setup/tempDataDir.ts"; const fixture = join(process.cwd(), "tests/fixtures/fake-zcode-app-server.mjs"); const TEST_DATA_DIR = mkdtempSync(join(tmpdir(), "omniroute-zcode-")); process.env.DATA_DIR = TEST_DATA_DIR; -test.after(() => - rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }) -); +test.after(() => cleanupTempDataDir(TEST_DATA_DIR)); async function loadZcodeExecutor() { return import("../../open-sse/executors/zcode.ts");