diff --git a/CHANGELOG.md b/CHANGELOG.md index ca6053b9..409eb4ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed +- `mex graph` now fails with an actionable message naming the running Node version when the built-in `node:sqlite` module lacks FTS5 support, instead of surfacing SQLite's raw `no such module: fts5` on the first schema statement that needs it. FTS5 availability is not guaranteed by every Node build/version inside the documented `engines` range (#110). + ## [0.8.0] - 2026-09-02 ### Added diff --git a/src/graph/__tests__/database-fts5.test.ts b/src/graph/__tests__/database-fts5.test.ts new file mode 100644 index 00000000..56b4f7c1 --- /dev/null +++ b/src/graph/__tests__/database-fts5.test.ts @@ -0,0 +1,135 @@ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { assertFts5Available, openGraphDatabase } from "../db/database.js"; +import type { SqliteDatabase } from "../db/sqlite.js"; + +const roots: string[] = []; + +afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); + vi.doUnmock("../db/sqlite.js"); +}); + +/** Minimal SqliteDatabase fake: only `exec` is exercised by assertFts5Available. */ +function fakeDb(execImpl: (sql: string) => void): SqliteDatabase { + return { + prepare: () => { + throw new Error("not used by this test"); + }, + exec: execImpl, + pragma: () => {}, + transaction: (fn) => fn(), + close: () => {}, + open: true, + }; +} + +/** + * Load a fresh `database.js` whose `assertFts5Available` probes against + * `fakeExec` instead of a real `:memory:` connection, by mocking the + * `openSqlite` it imports from `sqlite.js`. `assertFts5Available` no longer + * takes a `SqliteDatabase` parameter (PR #168 review: it must not touch the + * caller's real graph database) — it opens its own throwaway connection + * internally, so exercising the error paths now goes through this module + * mock rather than an injected fake. + */ +async function assertFts5AvailableWith(fakeExec: (sql: string) => void) { + vi.resetModules(); + vi.doMock("../db/sqlite.js", () => ({ + openSqlite: () => fakeDb(fakeExec), + })); + const fresh = await import("../db/database.js"); + return fresh.assertFts5Available; +} + +describe("assertFts5Available", () => { + it("does not throw when FTS5 statements succeed", () => { + // node:sqlite is compiled with FTS5 in every environment these tests run + // in (see sqlite.ts's module comment), so this exercises the real probe + // against a real throwaway :memory: connection end to end. + expect(() => assertFts5Available()).not.toThrow(); + }); + + it("raises an actionable, Node-version-specific message on the exact SQLite error from issue #110", async () => { + const probe = await assertFts5AvailableWith(() => { + throw new Error("no such module: fts5"); + }); + + expect(() => probe()).toThrowError( + new RegExp(`Node \\(${process.version.replace(/[.+]/g, "\\$&")}\\).*FTS5.*no such module: fts5`, "s"), + ); + }); + + it("re-throws an unrelated exec failure unchanged, rather than misattributing it to FTS5", async () => { + const probe = await assertFts5AvailableWith(() => { + throw new Error("database is locked"); + }); + + expect(() => probe()).toThrowError("database is locked"); + expect(() => probe()).not.toThrow(/FTS5/); + }); +}); + +describe("openGraphDatabase FTS5 preflight", () => { + it("still opens normally on a machine whose SQLite build has FTS5 (the common case)", () => { + const root = mkdtempSync(join(tmpdir(), "mex-database-fts5-")); + roots.push(root); + + const db = openGraphDatabase(join(root, "graph.db")); + try { + // exercised implicitly by openGraphDatabase not throwing; assert the FTS5 + // tables schema.sql defines actually exist, confirming the preflight probe + // did not somehow prevent or corrupt the real schema application + const tables = db + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name LIKE '%_fts%'") + .all() as Array<{ name: string }>; + expect(tables.length).toBeGreaterThan(0); + } finally { + db.close(); + } + }); + + it("closes the database handle when the FTS5 preflight fails, instead of leaking it open", async () => { + const root = mkdtempSync(join(tmpdir(), "mex-database-fts5-close-")); + roots.push(root); + + vi.resetModules(); + vi.doMock("../db/sqlite.js", async () => { + const actual = await vi.importActual("../db/sqlite.js"); + let realGraphDb: SqliteDatabase | undefined; + return { + ...actual, + openSqlite: (path: string, options?: { readOnly?: boolean; immutable?: boolean }) => { + if (path === ":memory:") { + // The FTS5 preflight's own throwaway connection: fail it. + return { + prepare: () => { + throw new Error("not used by this test"); + }, + exec: () => { + throw new Error("no such module: fts5"); + }, + pragma: () => {}, + transaction: (fn: () => T) => fn(), + close: () => {}, + open: true, + } satisfies SqliteDatabase; + } + realGraphDb = actual.openSqlite(path, options); + return realGraphDb; + }, + __getRealGraphDb: () => realGraphDb, + }; + }); + + const fresh = await import("../db/database.js"); + const sqliteMock = (await import("../db/sqlite.js")) as unknown as { + __getRealGraphDb: () => SqliteDatabase | undefined; + }; + + expect(() => fresh.openGraphDatabase(join(root, "graph.db"))).toThrow(/FTS5/); + expect(sqliteMock.__getRealGraphDb()?.open).toBe(false); + }); +}); diff --git a/src/graph/db/database.ts b/src/graph/db/database.ts index 6e62fbea..7c51114e 100644 --- a/src/graph/db/database.ts +++ b/src/graph/db/database.ts @@ -57,6 +57,42 @@ function configureReadOnlyConnection(db: SqliteDatabase): void { db.pragma("query_only = ON"); } +/** + * Probe for FTS5 support and fail fast with an actionable message if it's missing. + * + * `node:sqlite`'s bundled SQLite is not guaranteed to be built with FTS5 on every + * Node build/version, even within the range `package.json`'s `engines` documents + * as supported (issue #110). Without this check, the first FTS5 statement in + * `schema.sql` throws SQLite's raw `no such module: fts5`, which reads like a mex + * bug rather than a Node/SQLite build limitation. Create-and-drop a throwaway + * virtual table rather than querying `pragma_module_list`, since that pragma is + * unavailable on some `node:sqlite` builds too and FTS5 usage is what actually + * needs to work. + * + * FTS5 availability is a property of the SQLite build the running Node binary + * embeds, not of any particular database file, so the probe runs against a + * throwaway `:memory:` connection rather than the caller's real database. + * Probing in place (an earlier version of this function took the caller's + * `SqliteDatabase`) rewrote the on-disk graph on every successful open, which + * broke a read-path non-mutation regression test in CI (PR #168 review). + */ +export function assertFts5Available(): void { + const probe = openSqlite(":memory:"); + try { + probe.exec("CREATE VIRTUAL TABLE __mex_fts5_probe USING fts5(x)"); + } catch (error) { + const msg = error instanceof Error ? error.message : String(error); + if (!/fts5/i.test(msg)) throw error; // a different problem; surface it unchanged + throw new Error( + `Your Node (${process.version}) has SQLite built without FTS5 support, which mex's code graph ` + + "requires. Try a different Node build/version - see COMPATIBILITY.md for which versions are " + + `known to work. Underlying error: ${msg}`, + ); + } finally { + probe.close(); + } +} + /** * Open the graph DB at `dbPath`, creating the file + parent dir and applying the * schema when absent. Idempotent: re-opening an existing DB re-applies PRAGMAs @@ -84,6 +120,7 @@ export function openGraphDatabase( configureConnection(db); try { + assertFts5Available(); initializeWritableGraphDatabase(db, readFileSync(schemaPath(), "utf-8"), options); return db; } catch (error) {