From 91bf0a85eb16c73c3651520afe83754db273521b Mon Sep 17 00:00:00 2001 From: koraysrn Date: Fri, 9 Oct 2026 17:12:27 +0300 Subject: [PATCH] fix(sqlite): run on-disk statements in a killable child process (D227, A1) Run on-disk SQLite statements in a child process the provider can SIGKILL, so cancelQuery and the read-only deadline become preemptive (D227, A1). :memory: stays synchronous because a child process cannot share it. A LIBREDB_SQLITE_WORKER escape hatch restores the synchronous driver, used by the test suite because Bun 1.4.x hangs a SQLite child process a bun test run spawned. --- .env.example | 7 + docs/AGENT.md | 84 +-- docs/BACKLOG.md | 30 +- docs/MCP.md | 5 +- docs/providers/sqlite.md | 43 +- src/components/agent/AgentRail.tsx | 2 +- src/components/agent/ConsentCard.tsx | 8 +- src/lib/agent/deadline.ts | 11 +- src/lib/db/providers/sql/sqlite-worker.ts | 630 ++++++++++++++++++ src/lib/db/providers/sql/sqlite.ts | 611 ++++++++++++----- src/lib/db/query-cancel.ts | 4 +- tests/components/agent/AgentRail.test.tsx | 12 +- tests/components/agent/ConsentCard.test.tsx | 4 +- tests/integration/db/sqlite-provider.test.ts | 85 ++- tests/integration/db/sqlite-worker.test.ts | 297 +++++++++ tests/integration/mcp/run-read-query.test.ts | 16 +- tests/isolated/factory.test.ts | 8 +- .../security/agent-statement-boundary.test.ts | 6 + tests/unit/agent-documentation.test.ts | 6 +- tests/unit/lib/agent/auto-execute.test.ts | 6 +- .../lib/db/sqlite-worker-boundary.test.ts | 73 ++ 21 files changed, 1639 insertions(+), 309 deletions(-) create mode 100644 src/lib/db/providers/sql/sqlite-worker.ts create mode 100644 tests/integration/db/sqlite-worker.test.ts create mode 100644 tests/unit/lib/db/sqlite-worker-boundary.test.ts diff --git a/.env.example b/.env.example index 037b2f3e0..20f01ef31 100644 --- a/.env.example +++ b/.env.example @@ -324,6 +324,13 @@ STORAGE_PROVIDER=local # Set this only to force a specific driver (e.g. deterministic tests). # Options: bun, node # LIBREDB_SQLITE_DRIVER=node +# On-disk SQLite statements run in a killable child process so Cancel and the agent +# read-only deadline can stop them. Set "0" (also "false", "no", "off") to fall back to +# the synchronous driver, which blocks the server thread and has no Cancel. +# LIBREDB_SQLITE_WORKER=0 +# A watchdog (milliseconds) on each child-process request. A request that does not answer +# in this time is failed and the child is replaced. Unset = no watchdog. +# LIBREDB_SQLITE_WORKER_TIMEOUT_MS=30000 # =========================================== # Oracle Thick-mode Client (advanced) diff --git a/docs/AGENT.md b/docs/AGENT.md index 9a74d4df6..0c85080c2 100644 --- a/docs/AGENT.md +++ b/docs/AGENT.md @@ -91,36 +91,38 @@ Two companion pages carry what this one deliberately does not: ## Table of Contents -- [Turning it on](#turning-it-on) -- [What a run is](#what-a-run-is) - - [The conversation a run belongs to](#the-conversation-a-run-belongs-to) - - [What a plan run knows](#what-a-plan-run-knows) - - [What the inventory is an inventory OF](#what-the-inventory-is-an-inventory-of) - - [The statement a plan run drafts](#the-statement-a-plan-run-drafts) -- [Durability and resume](#durability-and-resume) - - [A drive that dies before the loop](#a-drive-that-dies-before-the-loop) -- [The tool set](#the-tool-set) - - [The query-optimization template](#the-query-optimization-template) - - [The database-assessment template](#the-database-assessment-template) - - [The operations template](#the-operations-template) - - [The data-analysis template](#the-data-analysis-template) - - [Presenting an answer](#presenting-an-answer) - - [Handing the answer to the editor (auto-execute)](#handing-the-answer-to-the-editor-auto-execute) - - [What the fence is proved to hold against](#what-the-fence-is-proved-to-hold-against) -- [What bounds a run](#what-bounds-a-run) -- [Supported models](#supported-models) -- [The model side](#the-model-side) - - [What a refused model looks like in the app](#what-a-refused-model-looks-like-in-the-app) -- [Whether the run answered](#whether-the-run-answered) - - [The eval harness](#the-eval-harness) -- [What the removed AI panels did that a run does not](#what-the-removed-ai-panels-did-that-a-run-does-not) -- [HTTP surface](#http-surface) -- [The surface in the app](#the-surface-in-the-app) -- [Deployment](#deployment) -- [Package boundary](#package-boundary) -- [Module map](#module-map) -- [Known limitations](#known-limitations) -- [Related documentation](#related-documentation) +- [Agent Runtime — LibreDB Studio](#agent-runtime--libredb-studio) + - [Table of Contents](#table-of-contents) + - [Turning it on](#turning-it-on) + - [What a run is](#what-a-run-is) + - [The conversation a run belongs to](#the-conversation-a-run-belongs-to) + - [What a plan run knows](#what-a-plan-run-knows) + - [What the inventory is an inventory OF](#what-the-inventory-is-an-inventory-of) + - [The statement a plan run drafts](#the-statement-a-plan-run-drafts) + - [Durability and resume](#durability-and-resume) + - [A drive that dies before the loop](#a-drive-that-dies-before-the-loop) + - [The tool set](#the-tool-set) + - [The query-optimization template](#the-query-optimization-template) + - [The database-assessment template](#the-database-assessment-template) + - [The operations template](#the-operations-template) + - [The data-analysis template](#the-data-analysis-template) + - [Presenting an answer](#presenting-an-answer) + - [Handing the answer to the editor (auto-execute)](#handing-the-answer-to-the-editor-auto-execute) + - [What the fence is proved to hold against](#what-the-fence-is-proved-to-hold-against) + - [What bounds a run](#what-bounds-a-run) + - [Supported models](#supported-models) + - [The model side](#the-model-side) + - [What a refused model looks like in the app](#what-a-refused-model-looks-like-in-the-app) + - [Whether the run answered](#whether-the-run-answered) + - [The eval harness](#the-eval-harness) + - [What the removed AI panels did that a run does not](#what-the-removed-ai-panels-did-that-a-run-does-not) + - [HTTP surface](#http-surface) + - [The surface in the app](#the-surface-in-the-app) + - [Deployment](#deployment) + - [Package boundary](#package-boundary) + - [Module map](#module-map) + - [Known limitations](#known-limitations) + - [Related documentation](#related-documentation) ## Turning it on @@ -1509,8 +1511,8 @@ function enumerated over all eight combinations by its test): access path is `index` or `mixed` — never `full-scan`, never `unknown` — and the reported `estimatedCost` is at most 50 000. **SQLite:** every step a `SEARCH`; any `SCAN`, a mixed plan or an unreadable one is risky. SQLite is stricter on purpose: `EXPLAIN QUERY PLAN` reports no cost - and no row estimate to weigh, the engine does not preempt a read that overruns, and a runaway read - blocks writers and this application until it finishes. Any other dialect is risky, the same + and no row estimate to weigh, and an overrunning read is stopped only by killing the child process + that runs it, not by an interrupt inside the engine. Any other dialect is risky, the same fail-closed posture `summarisePlan` takes. **A plan the server could only PARTLY read is risky too**, which is the same rule one level down. @@ -1792,10 +1794,10 @@ it run — `statementTimeoutMs` is clamped down to the time actually remaining, to the execution layer can never exceed what is left of the run. What that clamp is worth depends on the engine, and this is the one caveat a budget meter must not -hide: **PostgreSQL preempts an overrunning statement (`SET LOCAL statement_timeout`); SQLite does -not.** There the timeout is a post-execution deadline, so an overrunning SQLite statement is -detected after it finishes rather than being stopped mid-flight, and because the drivers are -synchronous it blocks the runtime while it runs. The rail states this where the meter is shown. +hide: **PostgreSQL preempts an overrunning statement (`SET LOCAL statement_timeout`); SQLite stops +it by killing the child process that runs it.** An overrunning on-disk SQLite statement is killed +mid-flight, so it stops at its timeout rather than running on; the in-memory database has no child +to kill and is refused only once it returns. The rail states this where the meter is shown. **The repair loop is bounded twice, and the two bounds are different.** A statement that already failed is never admitted again — keyed on a canonical fingerprint built from the shared SQL span @@ -2352,8 +2354,9 @@ Two further rules govern it: standing in for a capability: pause is offered on a running run and resume on a paused one, and neither is rendered where the service would refuse it. - **The meter reports only what is actually enforced** — statements, database time, the run deadline, - repair attempts — and states the SQLite non-preemption caveat rather than implying that an - overrunning statement is cut short. It reports no token budget because none is enforced. A statement that + repair attempts — and states that SQLite stops an overrunning statement by killing its child + process rather than implying the engine interrupts it. It reports no token budget because none is + enforced. A statement that failed at the database now carries the span the tracker charged for it, so the database-time figure no longer counts completed reads only, and a schema capture contributes the statements and the span the tracker charged it. What is left uncounted is the difference between an engine's elapsed time @@ -2605,8 +2608,9 @@ They are listed here so the honest boundary is visible from the behaviour docume from the tracker. **Inherited from the enforcement layer** — `docs/BACKLOG.md`, section "Agent M1 deferrals (#328)", -entries A1-A3. These bound what any agent statement can be promised: a SQLite statement is not -preempted, so its timeout is post-execution and an overrunning statement blocks the runtime (A1); +entries A1-A3. These bound what any agent statement can be promised: a SQLite statement has no +engine-side interrupt, so stopping one over its timeout means killing the child process that runs +it (A1); `VACUUM INTO` can create an empty file at a chosen path (A2); out-of-scope **reads** have no database-native control on either provider — the declared-target allowlist, the statement guard and the role's own grants are the whole boundary (A3). diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 9f66cd6b3..22f68d8cf 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -28,7 +28,7 @@ None of it is a GitHub issue. **Sections** - [SQL statement reading](#sql-statement-reading) — S2–S7 · 5 -- [Drivers and connections](#drivers-and-connections) — D1-D291, U17 · 194 +- [Drivers and connections](#drivers-and-connections) — D1-D291, U17 · 193 - [Value interpolation](#value-interpolation) — V1 - [Row editing](#row-editing) — R1–R3 · 3 - [Studio UI and query execution](#studio-ui-and-query-execution) — X2-X27, U2-U113 · 104 @@ -40,7 +40,7 @@ None of it is a GitHub issue. - [Security Phase 2 deferrals](#security-phase-2-deferrals) — C3–C11 · 6 - [Security Phase 3 deferrals](#security-phase-3-deferrals) — K4-K8 · 5 - [Security scanner triage](#security-scanner-triage) — SCAN1 · 1 -- [Agent M1 deferrals (#328)](#agent-m1-deferrals-328) — A1–A8 · 7 +- [Agent M1 deferrals (#328)](#agent-m1-deferrals-328) — A2–A8 · 6 - [Agent M2 deferrals (#329)](#agent-m2-deferrals-329) — B2-B106 · 42 - [Passkey deferrals (#785)](#passkey-deferrals-785) — PK1-PK9 · 8 - [MCP server deferrals (#246)](#mcp-server-deferrals-246) @@ -2500,17 +2500,6 @@ Found 2026-10-04 by the review of the #1351 fix; pre-existing, not measured end **Done when:** on a Materialize connection, search, inventory and the agent's grounding answer for tables, views and materialized views, with the two routine kinds reported as unavailable rather than failing the read. -### D227. A long SQLite statement blocks the whole Studio server, and no cancel or timeout reaches it - -`sqlite.ts` runs every statement through `bun:sqlite` or `node:sqlite`, both synchronous, on the server's one JavaScript thread, and neither exposes `sqlite3_interrupt` or a progress handler (Node 24.11's `DatabaseSync` has `setAuthorizer` and neither of those; Bun 1.4.2's `Database` has neither). -So a running statement cannot be stopped from inside the process, and while it runs no other request is served: measured 2026-10-03 on node:sqlite (SQLite 3.50.4), a 300M-row recursive CTE from the editor made `/api/health` answer after 69.7 s instead of 6 ms, and the 60 s query timeout, checked only after the statement returns, did not end it. -Since #1364 the editor no longer offers Cancel there (`supportsQueryCancel: false`, `docs/providers/sqlite.md` section 3.4); the block itself is unchanged. -A1 is the same property on the agent and MCP path. - -Found 2026-10-04 while fixing #1364. - -**Done when:** SQLite statements run in a worker thread that the provider terminates on cancel and on the query timeout, the SQLite provider implements `cancelQuery`, and a test shows `/api/health` answering while a long statement runs. - ### D228. Platform discovery recognises only four engine families `detectEngine` in `src/lib/seed/discovery-fingerprint.ts` matches the PostgreSQL family, the MySQL family (MySQL, MariaDB, Percona), MongoDB and the Redis family (Redis, Valkey, KeyDB, Dragonfly), and `ENV_ALLOW_LIST` in `docker/discover.mjs` exports only the ten environment keys those need. @@ -5601,21 +5590,6 @@ And the code does not exist on `main`: it arrives only if #1070 merges, and a di Each was decided while building the operation/policy layer, not overlooked. -### A1. A SQLite agent or MCP statement can block the runtime for its whole duration - -`sqlite.ts`'s `queryReadOnly` enforces `statementTimeoutMs` as a post-execution deadline: the result of -an overrunning statement is refused, but the statement is never preempted. SQLite has no -transaction-local statement timeout, and neither `bun:sqlite` nor `node:sqlite` exposes -`sqlite3_interrupt` or a progress handler. - -Because both drivers are synchronous, a hostile recursive CTE blocks the whole runtime while it runs. -Same property as the normal SQLite query path, but the input source differs in kind: there the SQL -comes from an authenticated operator, here from an agent. - -`/api/mcp`'s `run_read_query` reaches the same path, `queryReadOnly` under `agent-read-only`, with SQL from an external MCP client, and `docs/MCP.md` states the risk. - -**Done when:** either driver exposes an interrupt or progress hook, or agent and MCP SQLite execution moves to a worker that can be killed on deadline. - ### A2. `VACUUM INTO` can create an empty file at an agent-chosen path The SQLite agent profile's read-only open governs the target database file only. `VACUUM INTO ''` diff --git a/docs/MCP.md b/docs/MCP.md index d9b26cea8..8fce4a073 100644 --- a/docs/MCP.md +++ b/docs/MCP.md @@ -291,9 +291,10 @@ For the first few seconds after that service starts, `opencode mcp list` can say | PostgreSQL | Studio stops waiting; the statement runs on until `statement_timeout`, which is set to the time left | The database ends the statement, and the client gets the timeout answer | | SQL Server | Studio stops waiting; the statement runs until the provider's deadline cancels it | The provider cancels the statement, and the client gets the timeout answer | | DuckDB | Studio stops waiting; the statement runs to completion and its result is discarded | The client gets the timeout answer near `timeout_ms`, and the statement runs on | -| SQLite | The driver is synchronous, so the whole Studio process, the UI included, waits until the statement ends | The client is answered after the statement ends | +| SQLite | An on-disk statement runs in a killable child process, so the client is answered at `timeout_ms` | The statement is killed at `timeout_ms`; an in-memory statement still runs to completion | -- One MCP query against a large SQLite table stops the Studio process while it runs. +- One MCP query against a large SQLite table no longer stops the Studio process: the read runs in a + child process and is killed at `timeout_ms`; only an in-memory SQLite read still blocks it. - An MCP call on a LibreDB connection that no Studio session holds open opens the file itself and keeps its exclusive lock until that handle has been idle for 30 minutes. Every MCP call resets that clock, and meanwhile opening the connection in the Studio editor fails with 503, `LibreDB file is already open by another process (exclusive lock)`. When the editor opened the file first, MCP borrows its handle and nothing conflicts ([`docs/providers/libredb.md`](providers/libredb.md#421-on-disk-format-locking-and-version-compatibility-02x)). diff --git a/docs/providers/sqlite.md b/docs/providers/sqlite.md index 0a7c70b50..cae3fd2a4 100644 --- a/docs/providers/sqlite.md +++ b/docs/providers/sqlite.md @@ -231,11 +231,13 @@ A statement with no result columns answers `rowCount` from the driver's `changes not reset for a statement that changes no row: on node:sqlite a DDL statement or a value-setting `PRAGMA` reports the previous write's count (D139 in [`docs/BACKLOG.md`](../BACKLOG.md)). -### 3.4 No transactions API, no cancellation, no pool +### 3.4 No transactions API, no pool; cancellation via the child-process worker Unlike every networked SQL provider, SQLite exposes **no** `beginTransaction`/`commit`/`rollback`/ -`queryInTransaction`, **no** `cancelQuery`, and **no** pool/`getPoolStats`. It is a single embedded -handle. (`POST /api/db/transaction` and `/api/db/cancel` are therefore not applicable to SQLite.) +`queryInTransaction` and **no** pool/`getPoolStats` — it is a single embedded handle. Cancellation is +different: on-disk statements run in a child process the provider can kill, so `cancelQuery` and a +preemptive deadline exist there. (`POST /api/db/transaction` is not applicable to SQLite; `POST +/api/db/cancel` is, for on-disk connections.) Since #464 the client can see that: `supportsTransactions: false` ([§9](#9-capabilities--labels)) is what withholds BEGIN/COMMIT/ROLLBACK and the auto-rolled-back @@ -243,23 +245,24 @@ SANDBOX toggle from the editor toolbar here. Before that flag existed the only g `isTransactionProvider(provider)` inside the route — a runtime shape check the browser cannot read — so the controls rendered on every connection and the route answered HTTP 400. -Since #1364 the same holds for Cancel. `/api/db/provider-meta` reports `supportsQueryCancel: false` -here, read off the provider by the check the cancel route makes, and the provider declares -`blocksServerWhileRunning: true`. On an engine that cannot cancel, the editor's control reads "Stop -waiting" and only ends the editor's wait; here even that is withheld, because the server answers -nothing else until the statement ends, so the control is shown disabled with the reason on hover. -Before #1364 the button was live, the route answered 400 "Query cancellation is not supported for -this database type", and the editor said "Query Cancelled" anyway. - -**A long statement blocks the whole server, and nothing can stop it.** Both drivers run the -statement synchronously on the server's only JavaScript thread, and neither exposes -`sqlite3_interrupt` or a progress handler (checked on Node 24.11, whose `DatabaseSync` offers -`setAuthorizer` but neither of the two, and Bun 1.4.2). So no cancel and no deadline can reach a -running statement: the query timeout is checked after it returns. Measured on 2026-10-03 with -node:sqlite (SQLite 3.50.4): a 300M-row recursive CTE kept running after Cancel, and `/api/health` -answered after 69.7 s instead of the usual 6 ms, so every user of the instance waited with it. Moving -SQLite execution to a worker thread that can be terminated is recorded as D227 in -[`docs/BACKLOG.md`](../BACKLOG.md). +Since #1623, on-disk connections DO cancel. `/api/db/provider-meta` reports `supportsQueryCancel: true` +and `blocksServerWhileRunning: false` there, because the statement runs in a child process the +provider can `SIGKILL` (`src/lib/db/providers/sql/sqlite-worker.ts`). Before #1623 the route answered +400 "Query cancellation is not supported for this database type", and the editor said "Query +Cancelled" anyway. + +**A long on-disk statement no longer blocks the whole server (#1623).** The statement runs in a +child process, so while one runs the server keeps answering other requests, `cancelQuery` kills the +child, and the read-only deadline is preemptive. Measured on 2026-10-03, before the worker: a +300M-row recursive CTE kept running after Cancel, and `/api/health` answered after 69.7 s instead of +the usual 6 ms. + +**The escape hatch `LIBREDB_SQLITE_WORKER=0`** restores the old synchronous behaviour — the test +suite sets it because Bun 1.4.x hangs a SQLite child process a `bun test` run spawned. With the +worker off, on-disk statements run on the server thread again, `supportsQueryCancel` reads `false`, +`blocksServerWhileRunning` reads `true`, and the read-only timeout is post-execution. `:memory:` +always runs synchronously — a child process cannot share an in-memory database. Node (production) +keeps the worker on. ### 3.5 `endOpenQueryTransaction()` — a transaction a statement left open diff --git a/src/components/agent/AgentRail.tsx b/src/components/agent/AgentRail.tsx index c81982a87..fe7efda99 100644 --- a/src/components/agent/AgentRail.tsx +++ b/src/components/agent/AgentRail.tsx @@ -2810,7 +2810,7 @@ export const AgentRail = React.memo(function AgentRail({ charged statements, so that spend is not in the figure above. )}{" "} - On SQLite a statement over its timeout is refused once it returns, not interrupted while it runs. + On SQLite a statement over its timeout is now killed mid-flight, not refused only once it returns. diff --git a/src/components/agent/ConsentCard.tsx b/src/components/agent/ConsentCard.tsx index 8d60b3cbc..8ed7f4e3f 100644 --- a/src/components/agent/ConsentCard.tsx +++ b/src/components/agent/ConsentCard.tsx @@ -94,11 +94,11 @@ const handoverChips = (): readonly { readonly text: string; readonly tone: "neut /** * The one sentence that is true of SQLite and of nothing else, in the words the budget meter - * already uses for the same fact: SQLite does not preempt a statement over its timeout, so - * the editor's missing time limit is a different promise there than it is on PostgreSQL. + * already uses for the same fact: an on-disk SQLite read that overruns its timeout is now + * killed mid-flight, so only the in-memory database keeps the old blocking behaviour. */ const SQLITE_COST = - "On SQLite a read is not interrupted when it runs long: it blocks other writers and this application until it finishes."; + "On SQLite a read that runs past its timeout is now killed mid-flight; the in-memory database is the exception and still blocks this application until the read finishes."; export function ConsentCard({ workflowType, @@ -223,7 +223,7 @@ export function ConsentCard({ The SQLite chip is the label of the sentence below it, not a second opinion: the two are adjacent, so nothing can show the compressed form alone. */ - ...(isSqlite ? ([{ text: "SQLite: not interruptible", tone: "warn" }] as const) : []), + ...(isSqlite ? ([{ text: "SQLite: killed at its timeout", tone: "warn" }] as const) : []), ].map((chip) => ( ` rather than a script + * file on purpose. The production payload deletes `src/` (see + * `scripts/lib/prune-standalone-payload.sh`), so a worker file under `src/` would + * exist in development and vanish in the Docker image, the npx cache and the native + * packages. An inline `-e` script needs no file, no tracing entry and no deployment + * edit, and it runs under Node in every environment: production already runs Node, + * and under Bun the child is a real `node` binary, because Bun's own `bun:sqlite` + * hangs a child a `bun test` run spawned (Bun 1.4.x). + * A child process also avoids the worker-thread locks the Bun runtime takes around + * its synchronous SQLite drivers, and `SIGKILL` can always stop it. + * + * THE BOUNDARY IS DELIBERATELY NARROW. The child opens the handle, runs SQL and reads + * raw rows; every value it cannot JSON-encode is tagged and the tags are resolved on + * the parent side: + * + * - a 64-bit integer is tagged `{ __libredb_sqlite_bigint: "..." }` and the parent + * resolves it through `normalizeSQLiteBigInt` — the ONE place that decides whether + * a 64-bit integer fits a JavaScript number, shared with the in-process driver; + * - a BLOB (`Uint8Array`) is tagged `{ __libredb_sqlite_bytes: "" }` and the + * parent resolves it to a `Buffer`, exactly the shape the in-process driver hands out. + * + * That is the whole reason this file can stay small: it duplicates no value logic. It + * only moves values across a boundary the synchronous drivers cannot cross. + * + * The handle is ONE connection and requests are SERIALISED: a second request waits + * for the first, exactly as the single in-process handle did. Cancelling or timing out + * a statement kills the child, which drops the operating system's file descriptors, and + * the next statement lazily starts a fresh child and re-applies the PRAGMAs. + */ + +import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process"; +import { normalizeSQLiteBigInt, toSQLiteBindValue, type SQLiteDeclaredColumn } from "./sqlite-driver"; + +// ============================================================================ +// Shared shapes +// ============================================================================ + +/** What `queryRows` answers: the raw driver read, before any result shaping. */ +export interface SQLiteRawRead { + readonly values: unknown[][]; + readonly columns: readonly SQLiteDeclaredColumn[]; + readonly returnsRows: boolean; + readonly changes: number; +} + +/** The open-time facts that decide how the child opens the handle. */ +export interface SQLiteWorkerOpen { + readonly dbPath: string; + readonly readonly: boolean; + readonly queryOnly: boolean; +} + +/** + * The handle surface the provider uses. Async, because every call crosses the + * process boundary and a synchronous wait would re-block the server the worker + * exists to unblock. + */ +export interface SQLiteHandle { + exec(sql: string): Promise; + all(sql: string, params?: readonly unknown[]): Promise; + get(sql: string, params?: readonly unknown[]): Promise; + queryRows(sql: string, params: readonly unknown[], timeoutMs?: number, queryId?: string): Promise; + inTransaction(): Promise; + close(): Promise; +} + +// ============================================================================ +// Wire tags +// ============================================================================ + +const BIGINT_TAG = "__libredb_sqlite_bigint"; +const BYTES_TAG = "__libredb_sqlite_bytes"; + +/** + * Tag every value the child cannot JSON-encode. Applied to BOUND PARAMETERS on the + * way IN (after `toSQLiteBindValue` has already run on the parent) and to result cells + * on the way OUT. A BigInt becomes its decimal text; a `Uint8Array` becomes a plain + * array. Everything else passes through, recursing into arrays and records. + */ +/** + * A bound parameter on the way IN: decimal 64-bit digits become a BigInt first + * (`toSQLiteBindValue`, the same conversion the in-process drivers apply), then + * `serializeCell` tags the BigInt for the wire. Without the first step a decimal + * string read back from the provider would bind as TEXT and stop matching the + * INTEGER it was read from. + */ +export function serializeBind(value: unknown): unknown { + return serializeCell(toSQLiteBindValue(value)); +} + +export function serializeCell(value: unknown): unknown { + if (value === null || value === undefined) return null; + if (typeof value === "bigint") return { [BIGINT_TAG]: value.toString() }; + if (value instanceof Uint8Array) return { [BYTES_TAG]: Buffer.from(value).toString("base64") }; + if (Array.isArray(value)) return value.map(serializeCell); + if (typeof value === "object") { + // `Object.fromEntries` rather than `out[key] = ...`: it defines OWN properties, so a cell + // key like "__proto__" writes an own property instead of replacing the prototype (CodeQL + // remote property injection). The same convention `declaredColumnTypes` uses for names. + return Object.fromEntries( + Object.entries(value as Record).map(([key, cell]) => [key, serializeCell(cell)]), + ); + } + return value; +} + +/** + * Resolve the tags `serializeCell` wrote, on the parent side. A tagged bigint goes + * through `normalizeSQLiteBigInt` (number when lossless, decimal string otherwise); + * a tagged byte array becomes a `Buffer`, the same shape the in-process driver + * produces. Nothing else is reinterpreted. + */ +export function deserializeCell(value: unknown): unknown { + if (value === null || value === undefined) return value; + if (Array.isArray(value)) return value.map(deserializeCell); + if (typeof value === "object") { + const record = value as Record; + if (typeof record[BIGINT_TAG] === "string" && Object.keys(record).length === 1) { + return normalizeSQLiteBigInt(BigInt(record[BIGINT_TAG] as string)); + } + if (typeof record[BYTES_TAG] === "string" && Object.keys(record).length === 1) { + return Buffer.from(record[BYTES_TAG] as string, "base64"); + } + return Object.fromEntries(Object.entries(record).map(([key, cell]) => [key, deserializeCell(cell)])); + } + return value; +} + +// ============================================================================ +// The child's program +// ============================================================================ + +/** + * The program the child runs. Written as plain JavaScript on purpose: it must load + * under `process.execPath -e` on both runtimes, with no TypeScript, no path alias and + * no `import`. It talks JSON-lines on stdin/stdout and writes nothing else to stdout, + * so the parent can read every line as a response. + */ +const WORKER_BOOTSTRAP = [ + '"use strict";', + "var db = null;", + "var driver = null;", + "function pickDriver() {", + " var override = process.env.LIBREDB_SQLITE_DRIVER;", + ' if (override === "bun" || override === "node") return override;', + ' return typeof Bun === "undefined" ? "node" : "bun";', + "}", + "function deser(v) {", + " if (v === null || v === undefined) return v;", + " if (Array.isArray(v)) return v.map(deser);", + ' if (typeof v === "object") {', + " var keys = Object.keys(v);", + ' if (keys.length === 1 && typeof v.__libredb_sqlite_bigint === "string") return BigInt(v.__libredb_sqlite_bigint);', + ' if (keys.length === 1 && typeof v.__libredb_sqlite_bytes === "string") return new Uint8Array(Buffer.from(v.__libredb_sqlite_bytes, "base64"));', + " var out = Object.create(null);", + " for (var k = 0; k < keys.length; k++) out[keys[k]] = deser(v[keys[k]]);", + " return out;", + " }", + " return v;", + "}", + "function ser(v) {", + " if (v === null || v === undefined) return null;", + ' if (typeof v === "bigint") { var t1 = {}; t1.__libredb_sqlite_bigint = v.toString(); return t1; }', + ' if (v instanceof Uint8Array) { var t2 = {}; t2.__libredb_sqlite_bytes = Buffer.from(v).toString("base64"); return t2; }', + " if (Array.isArray(v)) return v.map(ser);", + ' if (typeof v === "object") {', + " var out = Object.create(null);", + " var keys = Object.keys(v);", + " for (var k = 0; k < keys.length; k++) out[keys[k]] = ser(v[keys[k]]);", + " return out;", + " }", + " return v;", + "}", + "function allRows(sql, params) {", + " var stmt = db.prepare(sql);", + " return stmt.all.apply(stmt, params);", + "}", + "function open(payload) {", + " driver = pickDriver();", + ' if (driver === "bun") {', + ' var bun = require("bun:sqlite");', + " db = new bun.Database(payload.dbPath, { readonly: payload.readonly === true, safeIntegers: true });", + " } else {", + ' var node = require("node:sqlite");', + " db = new node.DatabaseSync(payload.dbPath, { readOnly: payload.readonly === true, readBigInts: true });", + " }", + " if (payload.queryOnly) {", + ' db.exec("PRAGMA query_only = true");', + ' var readback = allRows("PRAGMA query_only", []);', + " var first = readback[0];", + " // `safeIntegers`/`readBigInts` reads the pragma back as a BigInt; compare as a number.", + ' if (!first || Number(first.query_only) !== 1) throw new Error("query_only readback failed");', + " }", + ' db.exec("PRAGMA foreign_keys = ON");', + " if (!payload.readonly) {", + ' db.exec("PRAGMA journal_mode = WAL");', + ' db.exec("PRAGMA synchronous = NORMAL");', + " } else if (!payload.queryOnly) {", + " // A read-only open fails here, not at the open itself: SQLite opens its files", + " // lazily, and a WAL-mode database cannot be read without its -shm file beside it.", + " // The agent read-only profile skips this read, exactly as connectReadOnly did.", + ' allRows("PRAGMA journal_mode", []);', + " }", + "}", + "function query(sql, params) {", + " var stmt = db.prepare(sql);", + ' var returnsRows = driver === "bun" ? stmt.columnNames.length > 0 : stmt.columns().length > 0;', + " if (returnsRows) {", + " var values;", + " var columns;", + ' if (driver === "bun") {', + " values = stmt.values.apply(stmt, params) || [];", + " columns = stmt.columnNames.map(function (name, index) { return [name, stmt.declaredTypes[index] === null ? undefined : stmt.declaredTypes[index]]; });", + " } else {", + " stmt.setReturnArrays(true);", + " try { values = stmt.all.apply(stmt, params) || []; } finally { stmt.setReturnArrays(false); }", + " columns = stmt.columns().map(function (column) { return [column.name, column.type === null ? undefined : column.type]; });", + " }", + " return { values: values.map(function (row) { return row.map(ser); }), columns: columns, returnsRows: true, changes: 0 };", + " }", + " var info = stmt.run.apply(stmt, params);", + " return { values: [], columns: [], returnsRows: false, changes: Number(info.changes) };", + "}", + "function handle(req) {", + " try {", + ' if (req.op === "ping") return { ok: true };', + ' if (req.op === "open") { open(req); return { ok: true }; }', + ' if (req.op === "exec") { db.exec(req.sql); return { ok: true }; }', + ' if (req.op === "all") { return { ok: true, rows: allRows(req.sql, (req.params || []).map(deser)).map(ser) }; }', + ' if (req.op === "get") {', + " var stmt = db.prepare(req.sql);", + " var row = stmt.get.apply(stmt, (req.params || []).map(deser));", + " return { ok: true, row: row === undefined ? null : ser(row) };", + " }", + ' if (req.op === "query") { return { ok: true, read: query(req.sql, (req.params || []).map(deser)) }; }', + ' if (req.op === "inTransaction") { return { ok: true, value: driver === "bun" ? db.inTransaction : db.isTransaction }; }', + ' if (req.op === "close") {', + ' if (driver === "bun") db.close(true); else db.close();', + " db = null;", + " return { ok: true };", + " }", + ' return { ok: false, error: { name: "Error", message: "Unknown op: " + req.op } };', + " } catch (error) {", + " return { ok: false, error: {", + ' name: error && error.name ? error.name : "Error",', + " message: error && error.message ? error.message : String(error),", + " code: error && error.code !== undefined ? error.code : undefined,", + " errcode: error && error.errcode !== undefined ? error.errcode : undefined,", + " } };", + " }", + "}", + 'var stdinBuf = "";', + "function handleLine(line) {", + " var req;", + " try { req = JSON.parse(line); } catch (error) { return; }", + " var resp = { id: req.id };", + " var result = handle(req);", + " resp.ok = result.ok;", + " if (result.ok) {", + " if (result.rows !== undefined) resp.rows = result.rows;", + " if (result.row !== undefined) resp.row = result.row;", + " if (result.value !== undefined) resp.value = result.value;", + " if (result.read !== undefined) resp.read = result.read;", + " } else {", + " resp.error = result.error;", + " }", + ' process.stdout.write(JSON.stringify(resp) + "\\n");', + "}", + 'process.stdin.setEncoding("utf8");', + 'process.stdin.on("data", function (chunk) {', + " stdinBuf += chunk;", + " var idx;", + ' while ((idx = stdinBuf.indexOf("\\n")) !== -1) {', + " var line = stdinBuf.slice(0, idx);", + " stdinBuf = stdinBuf.slice(idx + 1);", + " handleLine(line);", + " }", + "});", +].join("\n"); + +// ============================================================================ +// Parent-side client +// ============================================================================ + +interface PendingRequest { + resolve(value: unknown): void; + reject(error: Error): void; +} + +interface WorkerResponse { + id: number; + ok: boolean; + rows?: unknown; + row?: unknown; + value?: unknown; + read?: SQLiteRawRead; + error?: { name?: string; message?: string; code?: unknown; errcode?: unknown }; +} + +/** The error a child reports when it cannot do what it was asked. */ +class SQLiteWorkerError extends Error { + constructor( + message: string, + public readonly workerName?: string, + public readonly code?: unknown, + public readonly errcode?: unknown, + ) { + super(message); + this.name = "SQLiteWorkerError"; + } +} + +/** + * The executable the child runs under. Production is Node (this module's own + * runtime), so `process.execPath` is the exact binary. Under Bun (dev and the + * `bun test` runner) `process.execPath` is Bun, whose `node:sqlite` lacks the + * statement APIs the bootstrap's `query` path needs (`columns()`, + * `setReturnArrays()`), so the child must be a real `node`. `process.versions.bun` + * tells the two apart without a `Bun` global reference. + */ +function workerExecutable(): string { + return typeof process.versions.bun === "string" ? "node" : process.execPath; +} + +/** + * The child's environment: one override and the handful of platform variables the + * child runtime needs, and nothing else. `LIBREDB_SQLITE_DRIVER=node` forces + * `node:sqlite` (never `bun:sqlite`, which hangs a child a `bun test` run spawned + * on Bun 1.4.x). The parent's full environment is deliberately NOT inherited - it + * can hold database passwords and JWT secrets the child has no reason to see (#1623). + * + * `PATH` MUST be copied: the spawner resolves `node` against the CHILD's PATH, and + * without it POSIX spawners fall back to the OS default search path, which on CI + * picks a node that predates `node:sqlite` (or no node at all) instead of the + * runner's node 24. `SystemRoot`, `TEMP` and `TMP` (Windows) and `TMPDIR` and + * `HOME` (POSIX) are the rest of what the child needs to load its own libraries + * and place SQLite's temporary files. None of these keys carries a secret. + */ +const WORKER_ENV_KEYS = ["PATH", "SystemRoot", "TEMP", "TMP", "TMPDIR", "HOME", "USERPROFILE"] as const; + +function workerEnvironment(): NodeJS.ProcessEnv { + const parent = process.env; + const env: Record = { LIBREDB_SQLITE_DRIVER: "node" }; + for (const key of WORKER_ENV_KEYS) { + const value = parent[key]; + if (value !== undefined) env[key] = value; + } + return env as unknown as NodeJS.ProcessEnv; +} + +/** + * The parent half of the boundary. One child, one serialised request at a time. + * + * `terminate()` is the preemptive cancel: it ends the child's stdin, kills it with + * SIGKILL, rejects the request in flight and every request still queued, and leaves + * the client dead until the provider starts a fresh one. It is called for both + * `cancelQuery` and the read-only deadline, which is the whole reason the statement + * can now be stopped before it returns. + */ +export class SQLiteWorkerClient implements SQLiteHandle { + private readonly child: ChildProcessWithoutNullStreams; + private readonly timeouts = new Map>(); + private nextId = 1; + private readonly pending = new Map(); + private readonly queue: Array<{ run(): void; reject(error: Error): void; key?: string }> = []; + private busy = false; + private alive = true; + /** The `queryId` of the request the child is executing now; null for internal ops. */ + private runningKey: string | null = null; + + private constructor(child: ChildProcessWithoutNullStreams) { + this.child = child; + // Manual line buffering rather than `node:readline`: readline pauses the stream, + // and under `bun test` that pause/resume balance can leave a response undelivered + // and the provider hung on the child's answer. + // Decode whole chunks as one stream rather than `buffer += chunk` decoding each + // chunk alone: a multibyte character split across two 64 KB chunks would otherwise + // become U+FFFD (the maintainer measured 14 of 50,000 rows corrupted). + child.stdout.setEncoding("utf8"); + let buffer = ""; + child.stdout.on("data", (chunk) => { + // Scan only the newly appended bytes. A line split across chunks waits in + // `buffer`, but no earlier byte is ever rescanned, so a very large result + // (one JSON line of 100k rows) is read in linear, not quadratic, time. + const start = buffer.length; + buffer += chunk; + let from = start; + let idx: number; + while ((idx = buffer.indexOf("\n", from)) !== -1) { + const line = buffer.slice(0, idx); + buffer = buffer.slice(idx + 1); + this.onLine(line); + from = 0; + } + }); + child.stderr.on("data", (chunk) => process.stderr.write(`[sqlite-worker] ${String(chunk)}`)); + child.on("error", (error) => this.failAll(new SQLiteWorkerError(`SQLite worker failed: ${error.message}`))); + child.on("exit", (code, signal) => { + this.alive = false; + this.failAll(new SQLiteWorkerError(`SQLite worker exited (code ${code}, signal ${signal ?? "none"})`)); + }); + } + + /** Start a child and open the handle; rejects with the child's own error on failure. */ + static async start(open: SQLiteWorkerOpen): Promise { + const child = spawn(workerExecutable(), ["-e", WORKER_BOOTSTRAP], { + stdio: ["pipe", "pipe", "pipe"], + env: workerEnvironment(), + }); + const client = new SQLiteWorkerClient(child); + try { + await client.request("open", open); + return client; + } catch (error) { + // The child can hold the file open when the open itself succeeded and a later + // PRAGMA failed ("file is not a database"). Close it gracefully before killing: + // the close releases the file synchronously, which a caller that renames, + // deletes or retries the path needs before it proceeds. SIGKILL alone releases + // the descriptor only after the OS has reaped the child, which races the caller. + await client.close().catch(() => {}); + throw error; + } + } + + /** + * The upper bound every request gets when `LIBREDB_SQLITE_WORKER_TIMEOUT_MS` is set: + * an answer that never arrives then kills the child instead of hanging the caller + * forever (a watchdog for the unexpected, not a replacement for the read-only budget). + */ + private defaultTimeoutMs(): number | undefined { + const raw = process.env.LIBREDB_SQLITE_WORKER_TIMEOUT_MS; + if (raw === undefined) return undefined; + const ms = Number(raw); + return Number.isFinite(ms) && ms > 0 ? ms : undefined; + } + + /** Ask the child one question and wait for its answer, in queue order. */ + private request( + op: string, + payload?: unknown, + timeoutMs: number | undefined = this.defaultTimeoutMs(), + key?: string, + ): Promise { + return new Promise((resolve, reject) => { + this.queue.push({ run: () => this.send(op, payload, resolve, reject, timeoutMs, key), reject, key }); + this.pump(); + }); + } + + private send( + op: string, + payload: unknown, + resolve: (value: unknown) => void, + reject: (error: Error) => void, + timeoutMs?: number, + key?: string, + ): void { + if (!this.alive) { + this.busy = false; + reject(new SQLiteWorkerError("SQLite worker is not running")); + return; + } + const id = this.nextId++; + this.pending.set(id, { resolve, reject }); + this.runningKey = key ?? null; + if (timeoutMs !== undefined && timeoutMs > 0) { + const timer = setTimeout(() => { + this.pending.delete(id); + this.timeouts.delete(id); + this.terminate(); + reject(new SQLiteWorkerError(`SQLite worker timed out after ${timeoutMs}ms (${op})`)); + }, timeoutMs); + this.timeouts.set(id, timer); + } + this.child.stdin.write(`${JSON.stringify({ id, op, ...(payload as Record | undefined) })}\n`); + } + + private pump(): void { + if (this.busy) return; + const next = this.queue.shift(); + if (next === undefined) return; + this.busy = true; + next.run(); + } + + private onLine(line: string): void { + let response: WorkerResponse; + try { + response = JSON.parse(line) as WorkerResponse; + } catch { + return; + } + const pending = this.pending.get(response.id); + if (pending === undefined) return; + const timer = this.timeouts.get(response.id); + if (timer !== undefined) { + clearTimeout(timer); + this.timeouts.delete(response.id); + } + this.pending.delete(response.id); + this.busy = false; + this.runningKey = null; + if (response.ok) { + pending.resolve(response); + } else { + const message = response.error?.message ?? "SQLite worker reported an error"; + pending.reject( + new SQLiteWorkerError(message, response.error?.name, response.error?.code, response.error?.errcode), + ); + } + this.pump(); + } + + private failAll(error: Error): void { + for (const timer of this.timeouts.values()) clearTimeout(timer); + this.timeouts.clear(); + for (const pending of this.pending.values()) pending.reject(error); + this.pending.clear(); + for (const queued of this.queue) queued.reject(error); + this.queue.length = 0; + this.busy = false; + this.runningKey = null; + } + + /** Whether the statement under `key` is the one the child is executing now. */ + isRunning(key: string): boolean { + return this.runningKey === key; + } + + /** + * Remove a statement still waiting in the queue and reject it, leaving the running + * statement alone. Returns false when nothing under that key is queued (#1623). + */ + cancelQueued(key: string): boolean { + const index = this.queue.findIndex((entry) => entry.key === key); + if (index === -1) return false; + const [entry] = this.queue.splice(index, 1); + entry.reject(new SQLiteWorkerError("SQLite statement was cancelled")); + return true; + } + + /** Whether a request is in flight, so a graceful close would wait behind it. */ + hasWork(): boolean { + return this.busy || this.queue.length > 0; + } + + /** + * Kill the child now; every request in flight and queued is rejected. + * + * `child.kill()` alone can leave the child's readline loop alive on some runtimes, + * so the child's stdin is ended first and the kill is a SIGKILL (R15). + */ + terminate(): void { + if (!this.alive) return; + this.alive = false; + try { + this.child.stdin.end(); + } catch { + // The pipe may already be closed; the kill below still runs. + } + this.child.kill("SIGKILL"); + this.failAll(new SQLiteWorkerError("SQLite worker was terminated")); + } + + /** Whether the child is still running; false after `terminate()` or an exit. */ + isAlive(): boolean { + return this.alive; + } + + async exec(sql: string): Promise { + await this.request("exec", { sql }); + } + + async all(sql: string, params?: readonly unknown[]): Promise { + const response = (await this.request("all", { sql, params: params?.map(serializeBind) })) as WorkerResponse; + return ((response.rows ?? []) as unknown[]).map(deserializeCell); + } + + async get(sql: string, params?: readonly unknown[]): Promise { + const response = (await this.request("get", { sql, params: params?.map(serializeBind) })) as WorkerResponse; + return deserializeCell(response.row ?? null); + } + + async queryRows( + sql: string, + params: readonly unknown[], + timeoutMs?: number, + queryId?: string, + ): Promise { + const response = (await this.request( + "query", + { sql, params: params.map(serializeBind) }, + timeoutMs, + queryId, + )) as WorkerResponse; + const read = response.read ?? { values: [], columns: [], returnsRows: false, changes: 0 }; + return { + values: read.values.map((row) => row.map(deserializeCell)), + // The wire turns the child's `undefined` column type into JSON `null`; restore + // `undefined` so the in-process and worker paths answer the same declared shape. + columns: read.columns.map(([name, type]) => [name, type ?? undefined] as const), + returnsRows: read.returnsRows, + changes: read.changes, + }; + } + + async inTransaction(): Promise { + const response = (await this.request("inTransaction")) as WorkerResponse; + return response.value === true; + } + + async close(): Promise { + if (!this.alive) return; + try { + await this.request("close"); + } finally { + this.terminate(); + } + } +} diff --git a/src/lib/db/providers/sql/sqlite.ts b/src/lib/db/providers/sql/sqlite.ts index 60e7a438b..5b3aa7026 100644 --- a/src/lib/db/providers/sql/sqlite.ts +++ b/src/lib/db/providers/sql/sqlite.ts @@ -47,7 +47,13 @@ import { } from "../../errors"; import { assertReadOnlyBudget, measureResultBytes } from "./read-only-budget"; import { formatBytes } from "../../utils/pool-manager"; -import { loadSQLiteDriver, type SQLiteDatabase, type SQLiteStatement } from "./sqlite-driver"; +import { + loadSQLiteDriver, + type SQLiteDatabase, + type SQLiteDeclaredColumn, + type SQLiteStatement, +} from "./sqlite-driver"; +import { SQLiteWorkerClient, type SQLiteHandle, type SQLiteRawRead, type SQLiteWorkerOpen } from "./sqlite-worker"; import { declaredColumnTypes } from "./column-types"; import { applySourceBound, @@ -974,15 +980,15 @@ export interface SQLiteTableSizeBytes { * right - but a test that reads the driver's name to predict which arm it is on is * reading the wrong thing. */ -export function readDbstatSizes(db: SQLiteDatabase): Map | null { - let pages: { name: string; bytes: number }[]; - try { - pages = db.prepare(DBSTAT_SIZES_SQL).all() as { name: string; bytes: number }[]; - } catch { - return null; - } - - const owners = db.prepare(DBSTAT_INDEX_OWNERS_SQL).all() as { name: string; tbl_name: string }[]; +/** + * Turn the two `dbstat` reads into per-table sizes. Shared by the in-process + * handle (`readDbstatSizes`) and the worker handle (`getTableStats`), so the two + * paths cannot disagree about how an index's pages are attributed. + */ +function buildDbstatSizes( + pages: readonly { name: string; bytes: number }[], + owners: readonly { name: string; tbl_name: string }[], +): Map { const bytesByObject = new Map(pages.map((page) => [page.name, Number(page.bytes) || 0])); const indexNames = new Set(owners.map((owner) => owner.name)); const sizes = new Map(); @@ -1007,6 +1013,18 @@ export function readDbstatSizes(db: SQLiteDatabase): Map | null { + let pages: { name: string; bytes: number }[]; + try { + pages = db.prepare(DBSTAT_SIZES_SQL).all() as { name: string; bytes: number }[]; + } catch { + return null; + } + + const owners = db.prepare(DBSTAT_INDEX_OWNERS_SQL).all() as { name: string; tbl_name: string }[]; + return buildDbstatSizes(pages, owners); +} + /** * Read one row-returning statement's result: the rows keyed by column names that are * non-empty and unique, and the declared types keyed by the same names. @@ -1030,13 +1048,32 @@ export function readDbstatSizes(db: SQLiteDatabase): Map[]; fields: string[]; declared: Pick } { +): { values: unknown[][]; columns: readonly SQLiteDeclaredColumn[] } { const values = stmt.values(...params); const columns = stmt.declaredColumns(); + return { values, columns }; +} + +/** + * Turn raw values and declared columns into a shaped result: columns keyed by + * non-empty, unique names and the declared types paired with those names. Shared + * by the in-process path and the worker path, so one can never name a column + * differently from the other (see `readResultRows`'s former contract). + */ +function assembleResult( + values: readonly (readonly unknown[])[], + columns: readonly SQLiteDeclaredColumn[], + sql: string, +): { rows: Record[]; fields: string[]; declared: Pick } { const fields = uniqueFieldNames(columns.map(([name]) => name)); return { rows: keyRowsByPosition(fields, values, "sqlite", sql), @@ -1080,8 +1117,63 @@ export function buildTableStats(tableName: string, rowCount: number, size: SQLit // SQLite Provider // ============================================================================ +/** + * The in-process handle, used only for `:memory:` databases, which a child process + * cannot share. It wraps the existing `SQLiteDatabase` surface behind the same + * `SQLiteHandle` contract the worker implements, so the provider has one way to + * run a statement. + */ +class DirectSQLiteHandle implements SQLiteHandle { + constructor(private readonly db: SQLiteDatabase) {} + + async exec(sql: string): Promise { + this.db.exec(sql); + } + + async all(sql: string, params?: readonly unknown[]): Promise { + return this.db.prepare(sql).all(...(params ?? [])); + } + + async get(sql: string, params?: readonly unknown[]): Promise { + return this.db.prepare(sql).get(...(params ?? [])) ?? null; + } + + async queryRows( + sql: string, + params: readonly unknown[], + _timeoutMs?: number, + _queryId?: string, + ): Promise { + const stmt = this.db.prepare(sql); + if (stmt.returnsRows()) { + const { values, columns } = readStatementRows(stmt, params); + return { values, columns, returnsRows: true, changes: 0 }; + } + const info = params.length > 0 ? stmt.run(...params) : stmt.run(); + return { values: [], columns: [], returnsRows: false, changes: info.changes }; + } + + async inTransaction(): Promise { + return this.db.inTransaction; + } + + async close(): Promise { + this.db.close(true); + } +} + export class SQLiteProvider extends SQLBaseProvider { private db: SQLiteDatabase | null = null; + /** The worker-owned handle for on-disk databases; null for `:memory:`. */ + private worker: SQLiteWorkerClient | null = null; + /** The one handle every statement runs through: in-process for `:memory:`, worker for files. */ + private handle: SQLiteHandle | null = null; + /** The caller's own ids for statements in flight, so `cancelQuery` can tell "running" from "already done". */ + private readonly runningQueryIds = new Set(); + /** The open-time facts, kept so a killed worker can be restarted with the same handle. */ + private workerOpen: SQLiteWorkerOpen | null = null; + /** The one in-flight restart, so concurrent statements after a kill share one child. */ + private startingWorker: Promise | null = null; /** True when this instance was opened under the agent read-only profile. */ private readonly readOnlyProfile: boolean; private readonly denyExternalAccess: boolean; @@ -1118,8 +1210,12 @@ export class SQLiteProvider extends SQLBaseProvider { // SQLite HAS transactions; this provider holds no session for one, so // POST /api/db/transaction refuses the call and the controls stay hidden. supportsTransactions: false, - // Both drivers run the statement synchronously on the server's one thread (#1364). - blocksServerWhileRunning: true, + // On-disk statements run in a child process and can be cancelled; `:memory:` stays + // on the server thread because a child process cannot share it (#1364, #1623). With + // the worker turned off (`LIBREDB_SQLITE_WORKER=0`) on-disk statements fall back to + // the synchronous driver, so the server-blocking and cancel facts both flip back. + blocksServerWhileRunning: this.isMemoryDatabase() || !this.shouldUseWorker(), + supportsQueryCancel: !this.isMemoryDatabase() && this.shouldUseWorker(), maintenanceOperations: ["vacuum", "analyze", "reindex", "check"], // `VACUUM` rewrites the whole database file and takes no object at all, and // `PRAGMA integrity_check` reads the whole file the same way - `runMaintenance` @@ -1178,7 +1274,7 @@ export class SQLiteProvider extends SQLBaseProvider { // ============================================================================ public async connect(): Promise { - if (this.db) { + if (this.db || this.worker) { return; } @@ -1190,38 +1286,48 @@ export class SQLiteProvider extends SQLBaseProvider { } try { - // Dynamically load the runtime-appropriate SQLite driver - const SQLiteDB = await loadSQLiteDriver(); - const dbPath = this.getDatabasePath(true); if (this.readOnlyProfile) { - this.connectReadOnly(SQLiteDB, dbPath); + await this.connectReadOnly(dbPath); return; } - if (isUnwritableExistingFile(dbPath)) { - this.connectUnwritableFile(SQLiteDB, dbPath); + if (dbPath === ":memory:") { + // A child process cannot share the in-memory database, so `:memory:` stays on + // this thread with the synchronous driver it always used (K3). + const SQLiteDB = await loadSQLiteDriver(); + this.db = new SQLiteDB(dbPath, { create: true, readwrite: true }); + this.db.exec("PRAGMA foreign_keys = ON"); + this.db.exec("PRAGMA journal_mode = WAL"); + this.db.exec("PRAGMA synchronous = NORMAL"); + this.setConnected(true); return; } - if (dbPath !== ":memory:") { - const dir = path.dirname(dbPath); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); - } + if (isUnwritableExistingFile(dbPath)) { + await this.connectUnwritableFile(dbPath); + return; } - this.db = new SQLiteDB(dbPath, { - create: true, - readwrite: true, - }); - - // Enable WAL mode and foreign keys - this.db.exec("PRAGMA foreign_keys = ON"); - this.db.exec("PRAGMA journal_mode = WAL"); - this.db.exec("PRAGMA synchronous = NORMAL"); + const dir = path.dirname(dbPath); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true }); + } + if (this.shouldUseWorker()) { + this.workerOpen = { dbPath, readonly: false, queryOnly: false }; + this.worker = await SQLiteWorkerClient.start(this.workerOpen); + this.handle = this.worker; + } else { + // The synchronous fallback: exactly the pre-worker open, so the test suite can + // exercise every on-disk behaviour without Bun's worker-process hang. + const SQLiteDB = await loadSQLiteDriver(); + this.db = new SQLiteDB(dbPath, { create: true, readwrite: true }); + this.db.exec("PRAGMA foreign_keys = ON"); + this.db.exec("PRAGMA journal_mode = WAL"); + this.db.exec("PRAGMA synchronous = NORMAL"); + } this.setConnected(true); } catch (error) { // The handle is opened before the pragmas run, so a failure past that line @@ -1236,8 +1342,14 @@ export class SQLiteProvider extends SQLBaseProvider { // `finally`, because `close(true)` raises when SQLite cannot close, and a // reference kept past that throw is exactly the silent no-op described above. try { + // The worker's close kills the child after releasing the file; the in-process + // fallback's close(true) releases it immediately. Both may exist on a refused + // connect, so both are closed (file release, #1623). + await this.handle?.close(); this.db?.close(true); } finally { + this.handle = null; + this.worker = null; this.db = null; } this.setError(error instanceof Error ? error : new Error(String(error))); @@ -1276,15 +1388,22 @@ export class SQLiteProvider extends SQLBaseProvider { * 2026-09-26). `connect()` names that case, reading it from the file's header because * SQLite words it differently by build (`isWalModeFile`). */ - private connectUnwritableFile(SQLiteDB: Awaited>, dbPath: string): void { + private async connectUnwritableFile(dbPath: string): Promise { this.unwritableFilePath = dbPath; logger.info(`[SQLite] Opening ${dbPath} read-only: this process cannot write the file or its directory`, { provider: "sqlite", }); - this.db = new SQLiteDB(dbPath, { readonly: true }); - this.db.exec("PRAGMA foreign_keys = ON"); - // A file in WAL mode fails here rather than at the open: SQLite opens its files lazily. - this.db.prepare("PRAGMA journal_mode").get(); + if (this.shouldUseWorker()) { + this.workerOpen = { dbPath, readonly: true, queryOnly: false }; + this.worker = await SQLiteWorkerClient.start(this.workerOpen); + this.handle = this.worker; + } else { + const SQLiteDB = await loadSQLiteDriver(); + this.db = new SQLiteDB(dbPath, { readonly: true }); + this.db.exec("PRAGMA foreign_keys = ON"); + // A file in WAL mode fails here rather than at the open: SQLite opens its files lazily. + this.db.prepare("PRAGMA journal_mode").get(); + } this.setConnected(true); } @@ -1304,7 +1423,7 @@ export class SQLiteProvider extends SQLBaseProvider { * can only ever yield an empty one (node:sqlite) or fail (bun:sqlite), so * vending it would hand the agent a silently useless target. */ - private connectReadOnly(SQLiteDB: Awaited>, dbPath: string): void { + private async connectReadOnly(dbPath: string): Promise { if (dbPath === ":memory:") { throw new ExecutionProfileError( "The agent read-only execution profile cannot target an in-memory SQLite database", @@ -1312,43 +1431,92 @@ export class SQLiteProvider extends SQLBaseProvider { ); } - this.db = new SQLiteDB(dbPath, { readonly: true }); - try { - this.enforceQueryOnly(); - } catch (error) { - // Released now, for the same reason disconnect() does it: a refused profile that - // left the file held open would be a lock on a database nobody is using. The - // reference goes in a `finally`, because `close(true)` raises when it cannot - // close, and the refusal the caller needs to see is still `error`. + if (this.shouldUseWorker()) { + this.workerOpen = { dbPath, readonly: true, queryOnly: true }; + this.worker = await SQLiteWorkerClient.start(this.workerOpen); + this.handle = this.worker; try { - this.db.close(true); - } finally { - this.db = null; + await this.enforceQueryOnly(); + } catch (error) { + // Released now, for the same reason disconnect() does it: a refused profile that + // left the file held open would be a lock on a database nobody is using. + try { + await this.handle.close(); + } finally { + this.handle = null; + this.worker = null; + } + throw error; + } + } else { + const SQLiteDB = await loadSQLiteDriver(); + this.db = new SQLiteDB(dbPath, { readonly: true }); + try { + await this.enforceQueryOnly(); + } catch (error) { + try { + this.db.close(true); + } finally { + this.db = null; + } + throw error; } - throw error; } this.setConnected(true); } /** Set `query_only` and refuse to continue unless it reads back enabled. */ - private enforceQueryOnly(): void { - this.db!.exec(QUERY_ONLY_PRAGMA_SQL); - assertQueryOnlyEnabled(this.db!.prepare(QUERY_ONLY_READBACK_SQL).all()); + private async enforceQueryOnly(): Promise { + // Through `getHandle`, not `this.worker`: a deadline or a cancel kills the child, + // and the next statement must restart it before the PRAGMA runs (#1623). + const handle = await this.getHandle(); + await handle.exec(QUERY_ONLY_PRAGMA_SQL); + assertQueryOnlyEnabled(await handle.all(QUERY_ONLY_READBACK_SQL)); } public async disconnect(): Promise { - if (this.db) { - // `true` means "release the file now" rather than "once the last statement is - // collected" - see the measurement on `SQLiteDatabase.close`. A caller that has - // disconnected is entitled to delete, move or reopen the database, and on Windows - // a deferred close makes all three impossible. - this.db.close(true); + if (this.db || this.handle) { + // The worker's close kills the child after releasing the file, and the in-process + // close(true) releases the file immediately: either way a caller that has + // disconnected is entitled to delete, move or reopen the database. + if (this.db) await new DirectSQLiteHandle(this.db).close(); + if (this.handle) { + // A running or queued statement is in flight: kill the child now instead of + // queueing a close behind it, so disconnect returns at once (#1623). An idle + // child still closes gracefully, which releases the file synchronously. + if (this.worker !== null && this.worker.hasWork()) this.worker.terminate(); + else await this.handle.close(); + } + this.handle = null; this.db = null; + this.worker = null; this.setConnected(false); } } + /** Whether this connection targets an in-memory database, which a child process cannot share. */ + private isMemoryDatabase(): boolean { + return this.getDatabasePath() === ":memory:"; + } + + /** + * Whether an on-disk connection runs in the child-process worker. The child is a + * `node` process, so it can only run `node:sqlite`: a forced `bun` driver + * (`LIBREDB_SQLITE_DRIVER=bun`, the test suite's way to exercise `bun:sqlite`) + * must stay in-process. `LIBREDB_SQLITE_WORKER=0` is the escape hatch for + * runtimes where the child cannot run at all. + */ + private shouldUseWorker(): boolean { + if (this.isMemoryDatabase()) return false; + if (process.env.LIBREDB_SQLITE_DRIVER === "bun") return false; + const override = process.env.LIBREDB_SQLITE_WORKER; + if (override !== undefined) { + return !["0", "false", "no", "off"].includes(override.toLowerCase()); + } + return true; + } + private getDatabasePath(checkReserved = false): string { let dbPath: string; if (this.config.connectionString) { @@ -1389,50 +1557,122 @@ export class SQLiteProvider extends SQLBaseProvider { // Query Execution // ============================================================================ - public async query(sql: string, params?: unknown[]): Promise { + public async query(sql: string, params?: unknown[], queryId?: string): Promise { this.ensureConnected(); return this.trackQuery(async () => { - const { result, executionTime } = await this.measureExecution(async () => { - try { - const stmt = this.db!.prepare(sql); - - // Routed on what SQLite compiled, not on the leading keyword: a statement with - // result columns is read for its rows, whatever it starts with. A keyword set - // missed WITH, VALUES and every `... RETURNING`, and `run()` dropped their rows. - if (stmt.returnsRows()) { - return { ...readResultRows(stmt, sql, params ?? []), changes: 0 }; - } else { - const info = params ? stmt.run(...params) : stmt.run(); + if (queryId !== undefined) this.runningQueryIds.add(queryId); + try { + const { result, executionTime } = await this.measureExecution(async () => { + try { + const read = await (await this.getHandle()).queryRows(sql, params ?? [], this.queryTimeout, queryId); + // Routed on what SQLite compiled, not on the leading keyword: a statement with + // result columns is read for its rows, whatever it starts with. A keyword set + // missed WITH, VALUES and every `... RETURNING`, and `run()` dropped their rows. + if (read.returnsRows) { + return { ...assembleResult(read.values, read.columns, sql), changes: 0 }; + } return { rows: [], fields: [], - changes: info.changes, + changes: read.changes, // A write has no result columns at all - measured, both drivers answer an // EMPTY column list after `run()` - so there is nothing to declare, and the // key is left off exactly as `fields: []` leaves the names off. declared: {}, }; + } catch (error) { + if (this.unwritableFilePath !== null && isReadOnlyWriteError(error)) { + throw new QueryError( + `SQLite database ${this.unwritableFilePath} is open read-only because this process cannot write the file or its directory: ${(error as Error).message}`, + "sqlite", + sql, + ); + } + throw mapDatabaseError(error, "sqlite", sql); } - } catch (error) { - if (this.unwritableFilePath !== null && isReadOnlyWriteError(error)) { - throw new QueryError( - `SQLite database ${this.unwritableFilePath} is open read-only because this process cannot write the file or its directory: ${(error as Error).message}`, - "sqlite", - sql, - ); - } - throw mapDatabaseError(error, "sqlite", sql); - } - }); + }); - return { - rows: result.rows, - fields: result.fields, - rowCount: result.rows.length || result.changes, - executionTime, - ...result.declared, - }; + return { + rows: result.rows, + fields: result.fields, + rowCount: result.rows.length || result.changes, + executionTime, + ...result.declared, + }; + } finally { + if (queryId !== undefined) this.runningQueryIds.delete(queryId); + } + }); + } + + /** + * The handle to run statements on, restarting a killed worker if a cancel or a + * deadline ended it. `:memory:` returns the in-process handle. + */ + private async getHandle(): Promise { + // `:memory:` derives its handle fresh from `this.db` on every call, so a test that + // swaps the in-process handle (see `interceptReads`) is seen by the next statement. + if (this.db) return new DirectSQLiteHandle(this.db); + if (this.handle === null) { + throw new DatabaseConfigError("Provider is not connected. Call connect() first.", "sqlite"); + } + if (this.worker !== null && !this.worker.isAlive()) { + // Single-flight: two statements racing to restart after a kill share one child, + // so a cancel followed by parallel queries leaves exactly one process (#1623). + this.startingWorker ??= SQLiteWorkerClient.start(this.workerOpen!); + try { + this.worker = await this.startingWorker; + this.handle = this.worker; + } finally { + this.startingWorker = null; + } + } + return this.handle; + } + + /** + * Stop a running statement by killing the child that runs it (#1623). True only when + * a statement under that id was actually in flight; a finished or unknown id is false. + */ + public async cancelQuery(queryId: string): Promise { + if (!this.runningQueryIds.has(queryId)) return false; + this.runningQueryIds.delete(queryId); + const worker = this.worker; + if (worker !== null) { + if (worker.isRunning(queryId)) { + // The statement is the one the child executes now: preempt it by killing the child. + worker.terminate(); + } else { + // Waiting behind another statement: drop it from the queue and leave the + // running statement alone (#1623). + worker.cancelQueued(queryId); + } + } + return true; + } + + /** + * Run one statement read with a PREEMPTIVE deadline. SQLite's drivers expose no + * interrupt, so the only way to stop an overrunning statement is to kill the child + * that runs it; the deadline here does that instead of waiting for the result (#1623). + */ + private async runWithStatementTimeout(fn: () => Promise, timeoutMs: number, sql: string): Promise { + return await new Promise((resolve, reject) => { + const timer = setTimeout(() => { + this.worker?.terminate(); + reject(new QueryError(`Read-only execution exceeded the time budget: ${timeoutMs}ms allowed`, "sqlite", sql)); + }, timeoutMs); + fn().then( + (value) => { + clearTimeout(timer); + resolve(value); + }, + (error) => { + clearTimeout(timer); + reject(error); + }, + ); }); } @@ -1475,9 +1715,9 @@ export class SQLiteProvider extends SQLBaseProvider { public async endOpenQueryTransaction(): Promise { this.ensureConnected(); - if (!this.db!.inTransaction) return "none"; + if (!(await (await this.getHandle()).inTransaction())) return "none"; - this.db!.exec("ROLLBACK"); + await (await this.getHandle()).exec("ROLLBACK"); return "rolled-back"; } @@ -1511,7 +1751,7 @@ export class SQLiteProvider extends SQLBaseProvider { // Per statement, not just at open: the profiled provider is pooled and // reused, so a previous statement's `PRAGMA query_only = false` would // otherwise persist for every later call on this connection. - this.enforceQueryOnly(); + await this.enforceQueryOnly(); return this.trackQuery(async () => { const { @@ -1520,8 +1760,20 @@ export class SQLiteProvider extends SQLBaseProvider { } = await this.measureExecution(async () => { try { // The budgets below are checked on the rows either way, so a result refused for - // being too large carries its types no further than it carries its rows. - return readResultRows(this.db!.prepare(sql), sql, []); + // being too large carries its types no further than it carries its rows. The + // deadline is preemptive: an overrunning statement kills the child (#1623). + // + // The handle is captured BEFORE the deadline timer starts. If the timer fired + // first and `fn` resolved a fresh handle, it would restart the very statement + // the deadline just refused — that restart runs with no budget, and the next + // disconnect hangs waiting for a close it can never answer. + const handle = await this.getHandle(); + const read = await this.runWithStatementTimeout( + () => handle.queryRows(sql, []), + budget.statementTimeoutMs, + sql, + ); + return assembleResult(read.values, read.columns, sql); } catch (error) { throw mapDatabaseError(error, "sqlite", sql); } @@ -1542,11 +1794,12 @@ export class SQLiteProvider extends SQLBaseProvider { sql, ); } - // SQLite has no transaction-local statement timeout, and neither adapter - // exposes sqlite3_interrupt or a progress handler, so the budget's - // timeout is a post-execution deadline: an overrunning statement is not - // preempted, but its result is refused rather than returned as if it had - // been within budget. Recorded as such in docs/providers/sqlite.md. + // With the child-process worker the deadline above is preemptive: an overrunning + // statement is killed and its promise rejects before this check. Under the + // synchronous fallback (`:memory:`, or `LIBREDB_SQLITE_WORKER=0`) there is no + // interrupt to call, so the timeout is a post-execution deadline there: the + // statement is not preempted, but its result is refused rather than returned as + // if it had been within budget. if (executionTime > budget.statementTimeoutMs) { throw new QueryError( `Read-only execution exceeded the time budget: ${executionTime}ms > ${budget.statementTimeoutMs}ms allowed`, @@ -1615,7 +1868,7 @@ export class SQLiteProvider extends SQLBaseProvider { const counts = seedZeroCounts(declared); try { - applyKindCounts(counts, this.db!.prepare(COUNTS_SQL).all(MAIN_SCHEMA) as KindCountRow[]); + applyKindCounts(counts, (await (await this.getHandle()).all(COUNTS_SQL, [MAIN_SCHEMA])) as KindCountRow[]); return counts; } catch (error) { return unavailableCounts( @@ -1626,9 +1879,9 @@ export class SQLiteProvider extends SQLBaseProvider { } /** One object-surface read, mapped against THE STATEMENT SQLITE RECEIVED. */ - private runObjectQuery(sql: string, params: readonly unknown[]): T[] { + private async runObjectQuery(sql: string, params: readonly unknown[]): Promise { try { - return this.db!.prepare(sql).all(...params) as T[]; + return (await (await this.getHandle()).all(sql, params)) as T[]; } catch (error) { throw mapDatabaseError(error, "sqlite", sql); } @@ -1662,7 +1915,7 @@ export class SQLiteProvider extends SQLBaseProvider { throw new QueryError(`SQLite declares the kind "${kind}" but has no statement that lists it`, "sqlite"); } - return this.runObjectQuery(statement.sql, statement.params) + return (await this.runObjectQuery(statement.sql, statement.params)) .map((row) => ({ path: objectPath(container, row), name: row.name, kind })) .sort((left, right) => comparePaths(left.path, right.path)); // No `rowCount` and no `sizeBytes`, and both absences are facts about SQLite rather @@ -1723,16 +1976,16 @@ export class SQLiteProvider extends SQLBaseProvider { // level to take one from - see MAIN_SCHEMA for what leaving it out actually does. const binds = [path[path.length - 1], MAIN_SCHEMA]; - const columnRows = this.runObjectQuery(OBJECT_COLUMNS_SQL, binds); + const columnRows = await this.runObjectQuery(OBJECT_COLUMNS_SQL, binds); if (columnRows.length === 0) { throw new QueryError(`No SQLite ${kind} named ${binds[0]} in ${MAIN_SCHEMA}`, "sqlite", OBJECT_COLUMNS_SQL); } return objectDetailFromRows(path, { columns: columnRows, - indexes: this.readObjectIndexes(binds), - indexColumns: this.readObjectIndexColumns(binds), - foreignKeys: this.readObjectForeignKeys(binds), + indexes: await this.readObjectIndexes(binds), + indexColumns: await this.readObjectIndexColumns(binds), + foreignKeys: await this.readObjectForeignKeys(binds), }); } @@ -1744,8 +1997,8 @@ export class SQLiteProvider extends SQLBaseProvider { * serves a UNIQUE or PRIMARY KEY constraint, cannot be dropped, and is not an object a * user declared. */ - private readObjectIndexes(binds: readonly unknown[]): ObjectIndexRow[] { - return this.runObjectQuery(OBJECT_INDEXES_SQL, binds); + private async readObjectIndexes(binds: readonly unknown[]): Promise { + return await this.runObjectQuery(OBJECT_INDEXES_SQL, binds); } /** @@ -1756,14 +2009,18 @@ export class SQLiteProvider extends SQLBaseProvider { * statement produces. Null names are kept rather than filtered, because the MAPPER is * the one place that decides what an expression key means. */ - private readObjectIndexColumns(binds: readonly unknown[]): ObjectIndexColumnRow[] { + private async readObjectIndexColumns(binds: readonly unknown[]): Promise { const [, schema] = binds; - return this.readObjectIndexes(binds).flatMap((index) => - this.runObjectQuery<{ name: string | null }>(OBJECT_INDEX_COLUMNS_SQL, [index.name, schema]).map((column) => ({ - index_name: index.name, - name: column.name, - })), - ); + const indexes = await this.readObjectIndexes(binds); + const rows: ObjectIndexColumnRow[] = []; + for (const index of indexes) { + const columns = await this.runObjectQuery<{ name: string | null }>(OBJECT_INDEX_COLUMNS_SQL, [ + index.name, + schema, + ]); + for (const column of columns) rows.push({ index_name: index.name, name: column.name }); + } + return rows; } /** @@ -1775,11 +2032,11 @@ export class SQLiteProvider extends SQLBaseProvider { * put a null in a typed string field. The parent's key columns are read at most once per * parent and only when some constraint needs them. */ - private readObjectForeignKeys(binds: readonly unknown[]): ObjectForeignKeyRow[] { + private async readObjectForeignKeys(binds: readonly unknown[]): Promise { const [name, schema] = binds; // The schema first, because it is the SUBQUERY's placeholder and SQLite numbers // placeholders by where they appear in the statement text. - return this.runObjectQuery(OBJECT_FOREIGN_KEYS_SQL, [schema, name, schema]); + return await this.runObjectQuery(OBJECT_FOREIGN_KEYS_SQL, [schema, name, schema]); } /** @@ -1838,23 +2095,32 @@ export class SQLiteProvider extends SQLBaseProvider { const head = [MAIN_SCHEMA, ...(bounded ? [limit + 1] : [])]; const params = (schemaBinds: number): unknown[] => [...head, ...Array(schemaBinds).fill(MAIN_SCHEMA)]; - const targets = this.runObjectQuery<{ object_name: string }>(statements.target, params(statements.schemaBinds[0])); + const targets = await this.runObjectQuery<{ object_name: string }>( + statements.target, + params(statements.schemaBinds[0]), + ); const truncated = bounded && targets.length > limit; // The extra object the `limit + 1` bound brought back is dropped here, so its rows in // the four maps below are simply never read. const names = (truncated ? targets.slice(0, limit) : targets).map((row) => row.object_name); const columns = groupByObject( - this.runObjectQuery>(statements.columns, params(statements.schemaBinds[1])), + await this.runObjectQuery>(statements.columns, params(statements.schemaBinds[1])), ); const indexes = groupByObject( - this.runObjectQuery>(statements.indexes, params(statements.schemaBinds[2])), + await this.runObjectQuery>(statements.indexes, params(statements.schemaBinds[2])), ); const indexColumns = groupByObject( - this.runObjectQuery>(statements.indexColumns, params(statements.schemaBinds[3])), + await this.runObjectQuery>( + statements.indexColumns, + params(statements.schemaBinds[3]), + ), ); const foreignKeys = groupByObject( - this.runObjectQuery>(statements.foreignKeys, params(statements.schemaBinds[4])), + await this.runObjectQuery>( + statements.foreignKeys, + params(statements.schemaBinds[4]), + ), ); const details = names @@ -1922,7 +2188,7 @@ export class SQLiteProvider extends SQLBaseProvider { } const name = path[path.length - 1]; - const rows = this.runObjectQuery(OBJECT_SOURCE_SQL, [SOURCE_CATALOG_TYPES[kind], name]); + const rows = await this.runObjectQuery(OBJECT_SOURCE_SQL, [SOURCE_CATALOG_TYPES[kind], name]); const row = rows[0]; if (row === undefined) { throw new QueryError(`No SQLite ${kind} named ${name} in ${MAIN_SCHEMA}`, "sqlite", OBJECT_SOURCE_SQL); @@ -1974,7 +2240,7 @@ export class SQLiteProvider extends SQLBaseProvider { * `sizeStmt.get()` returning no row, or a row whose `size` is `undefined` - is * fixed once here rather than twice. */ - private readDatabaseSizeBytes(): number | undefined { + private async readDatabaseSizeBytes(): Promise { const dbPath = this.getDatabasePath(); if (dbPath !== ":memory:") { try { @@ -1986,8 +2252,7 @@ export class SQLiteProvider extends SQLBaseProvider { } } try { - const sizeStmt = this.db!.prepare(MEMORY_DB_SIZE_SQL); - const result = sizeStmt.get() as { size?: number }; + const result = (await (await this.getHandle()).get(MEMORY_DB_SIZE_SQL)) as { size?: number } | null; // A type check, not `|| 0`: page_count * page_size answering a real zero is // a measured reading, kept - but `|| 0` cannot tell that apart from // `get()` returning no row, or a row whose `size` came back `undefined`, @@ -2002,13 +2267,14 @@ export class SQLiteProvider extends SQLBaseProvider { this.ensureConnected(); const dbPath = this.getDatabasePath(); - const sizeBytes = this.readDatabaseSizeBytes(); + const sizeBytes = await this.readDatabaseSizeBytes(); const databaseSize = sizeBytes === undefined ? "N/A" : formatBytes(sizeBytes); let isHealthy = true; try { - const integrityStmt = this.db!.prepare("PRAGMA integrity_check"); - const integrityResult = integrityStmt.get() as { integrity_check: string }; + const integrityResult = (await (await this.getHandle()).get("PRAGMA integrity_check")) as { + integrity_check: string; + } | null; isHealthy = integrityResult?.integrity_check === "ok"; } catch { isHealthy = false; @@ -2016,8 +2282,9 @@ export class SQLiteProvider extends SQLBaseProvider { let journalMode = "unknown"; try { - const journalStmt = this.db!.prepare("PRAGMA journal_mode"); - const journalResult = journalStmt.get() as { journal_mode: string }; + const journalResult = (await (await this.getHandle()).get("PRAGMA journal_mode")) as { + journal_mode: string; + } | null; journalMode = journalResult?.journal_mode || "unknown"; } catch { // Ignore @@ -2079,13 +2346,15 @@ export class SQLiteProvider extends SQLBaseProvider { case "reindex": sql = target ? `REINDEX ${this.escapeIdentifier(target)}` : "REINDEX"; break; - case "check": - const checkStmt = this.db!.prepare("PRAGMA integrity_check"); - const checkResult = checkStmt.get() as { integrity_check: string }; + case "check": { + const checkResult = (await (await this.getHandle()).get("PRAGMA integrity_check")) as { + integrity_check: string; + } | null; return { success: checkResult?.integrity_check === "ok", message: checkResult?.integrity_check || "Unknown", }; + } } // Unsupported types fall through the switch with sql left empty. A @@ -2096,7 +2365,7 @@ export class SQLiteProvider extends SQLBaseProvider { throw new QueryError(`Unsupported maintenance type for SQLite: ${type}`, "sqlite"); } - this.db!.exec(sql); + await (await this.getHandle()).exec(sql); return { success: true, message: `${type.toUpperCase()} completed successfully` }; }); @@ -2115,22 +2384,21 @@ export class SQLiteProvider extends SQLBaseProvider { this.ensureConnected(); // Get SQLite version - const versionStmt = this.db!.prepare("SELECT sqlite_version() as version"); - const versionResult = versionStmt.get() as { version: string }; + const versionResult = (await (await this.getHandle()).get("SELECT sqlite_version() as version")) as { + version: string; + } | null; const version = `SQLite ${versionResult?.version || "Unknown"}`; // Get database size, absent rather than 0 when it cannot be read (#546) - see // `readDatabaseSizeBytes()` above, shared with `getHealth()`. - const sizeBytes = this.readDatabaseSizeBytes(); + const sizeBytes = await this.readDatabaseSizeBytes(); // Get table count - const tableCountStmt = this.db!.prepare(TABLE_COUNT_SQL); - const tableCountResult = tableCountStmt.get() as { count: number }; + const tableCountResult = (await (await this.getHandle()).get(TABLE_COUNT_SQL)) as { count: number } | null; const tableCount = tableCountResult?.count || 0; // Get index count - const indexCountStmt = this.db!.prepare(INDEX_COUNT_SQL); - const indexCountResult = indexCountStmt.get() as { count: number }; + const indexCountResult = (await (await this.getHandle()).get(INDEX_COUNT_SQL)) as { count: number } | null; const indexCount = indexCountResult?.count || 0; return { @@ -2214,20 +2482,38 @@ export class SQLiteProvider extends SQLBaseProvider { ]; } + /** + * The same two `dbstat` reads the in-process handle makes, through whatever handle + * this provider runs on. The shared attribution lives in `buildDbstatSizes`. + */ + private async readDbstatSizesViaHandle(): Promise | null> { + let pages: { name: string; bytes: number }[]; + try { + pages = (await (await this.getHandle()).all(DBSTAT_SIZES_SQL)) as { name: string; bytes: number }[]; + } catch { + return null; + } + const owners = (await (await this.getHandle()).all(DBSTAT_INDEX_OWNERS_SQL)) as { + name: string; + tbl_name: string; + }[]; + return buildDbstatSizes(pages, owners); + } + public async getTableStats(): Promise { this.ensureConnected(); - const tablesStmt = this.db!.prepare(STATS_TABLES_SQL); - const tables = tablesStmt.all() as { name: string }[]; + const tables = (await (await this.getHandle()).all(STATS_TABLES_SQL)) as { name: string }[]; // One dbstat scan per call, not per table: it reads the whole database file. - const sizes = readDbstatSizes(this.db!); + const sizes = await this.readDbstatSizesViaHandle(); const stats: TableStats[] = []; for (const { name: tableName } of tables) { // Get row count - const countStmt = this.db!.prepare(`SELECT COUNT(*) as count FROM "${tableName}"`); - const countResult = countStmt.get() as { count: number }; + const countResult = (await (await this.getHandle()).get(`SELECT COUNT(*) as count FROM "${tableName}"`)) as { + count: number; + } | null; const rowCount = countResult?.count || 0; stats.push(buildTableStats(tableName, rowCount, sizes?.get(tableName) ?? null)); @@ -2239,19 +2525,23 @@ export class SQLiteProvider extends SQLBaseProvider { public async getIndexStats(): Promise { this.ensureConnected(); - const indexesStmt = this.db!.prepare(STATS_INDEXES_SQL); - const indexes = indexesStmt.all() as { name: string; tbl_name: string }[]; + const indexes = (await (await this.getHandle()).all(STATS_INDEXES_SQL)) as { name: string; tbl_name: string }[]; const stats: IndexStats[] = []; for (const { name: indexName, tbl_name: tableName } of indexes) { // Get index info - const indexInfoStmt = this.db!.prepare(`PRAGMA index_info("${indexName}")`); - const indexCols = indexInfoStmt.all() as { seqno: number; cid: number; name: string }[]; + const indexCols = (await (await this.getHandle()).all(`PRAGMA index_info("${indexName}")`)) as { + seqno: number; + cid: number; + name: string; + }[]; // Get index uniqueness - const indexListStmt = this.db!.prepare(`PRAGMA index_list("${tableName}")`); - const indexList = indexListStmt.all() as { name: string; unique: number }[]; + const indexList = (await (await this.getHandle()).all(`PRAGMA index_list("${tableName}")`)) as { + name: string; + unique: number; + }[]; const indexMeta = indexList.find((i) => i.name === indexName); stats.push({ @@ -2293,8 +2583,7 @@ export class SQLiteProvider extends SQLBaseProvider { } } else { try { - const sizeStmt = this.db!.prepare(MEMORY_DB_SIZE_SQL); - const result = sizeStmt.get() as { size: number }; + const result = (await (await this.getHandle()).get(MEMORY_DB_SIZE_SQL)) as { size: number } | null; mainSizeBytes = result?.size || 0; } catch { // Ignore diff --git a/src/lib/db/query-cancel.ts b/src/lib/db/query-cancel.ts index 4a9d1d232..fedefbbcf 100644 --- a/src/lib/db/query-cancel.ts +++ b/src/lib/db/query-cancel.ts @@ -2,8 +2,8 @@ * The one test of whether a provider can cancel a running statement (#1364). * * Cancellation is optional surface: a provider that can stop its engine's statement - * implements `cancelQuery(queryId)`, and one that cannot (SQLite's synchronous driver, - * Cassandra, libSQL) leaves it out. Three readers ask the question: `POST /api/db/query` + * implements `cancelQuery(queryId)`, and one that cannot (Cassandra, libSQL) leaves it out. + * Three readers ask the question: `POST /api/db/query` * (whether to hand the run's id to the provider), `POST /api/db/cancel` (whether to refuse * with 400) and `POST /api/db/provider-meta` (the `supportsQueryCancel` capability the editor * gates its Cancel button on). They ask it here so the answer the editor reads is the one diff --git a/tests/components/agent/AgentRail.test.tsx b/tests/components/agent/AgentRail.test.tsx index b7d0a0f10..5eb1f39ae 100644 --- a/tests/components/agent/AgentRail.test.tsx +++ b/tests/components/agent/AgentRail.test.tsx @@ -2488,16 +2488,16 @@ describe("AgentRail", () => { }); /** - * `docs/BACKLOG.md` A1: SQLite has no interrupt, so its statement timeout is - * checked after the statement returns. A meter that showed a timeout without - * saying so would imply a preemption the runtime cannot perform. + * An on-disk SQLite statement runs in a killable child process, so the timeout + * preempts it. A meter that said "refused once it returns" would understate what + * the path now performs. */ - test("the meter says plainly that a SQLite statement is reported rather than interrupted", () => { + test("the meter says plainly that a SQLite statement is killed at its timeout", () => { const { getByTestId } = render(); const caveats = getByTestId("agent-budget-caveats").textContent ?? ""; expect(caveats).toContain("SQLite"); - expect(caveats).toContain("not interrupted"); + expect(caveats).toContain("killed mid-flight"); }); /** @@ -3886,7 +3886,7 @@ describe("AgentRail", () => { test("a SQLite connection is told what a long read there costs; another engine is not", async () => { const sqlite = await consentOf({ connectionType: "sqlite" }); expect(sqlite.getByTestId("agent-auto-execute-sqlite").textContent).toBe( - "On SQLite a read is not interrupted when it runs long: it blocks other writers and this application until it finishes.", + "On SQLite a read that runs past its timeout is now killed mid-flight; the in-memory database is the exception and still blocks this application until the read finishes.", ); cleanup(); diff --git a/tests/components/agent/ConsentCard.test.tsx b/tests/components/agent/ConsentCard.test.tsx index 69ccfa4a0..c2f36747a 100644 --- a/tests/components/agent/ConsentCard.test.tsx +++ b/tests/components/agent/ConsentCard.test.tsx @@ -216,7 +216,7 @@ describe("ConsentCard", () => { test("the SQLite chip is shown on SQLite and on nothing else", () => { const { getByTestId } = renderCard({ autoExecute: true, engine: "sqlite" }); - expect(getByTestId("agent-consent-bounds").textContent).toContain("SQLite: not interruptible"); + expect(getByTestId("agent-consent-bounds").textContent).toContain("SQLite: killed at its timeout"); cleanup(); const other = renderCard({ autoExecute: true, engine: "postgres" }); @@ -229,7 +229,7 @@ describe("ConsentCard", () => { const { getByTestId } = renderCard({ engine: "sqlite" }); expect(getByTestId("agent-auto-execute-sqlite").textContent).toBe( - "On SQLite a read is not interrupted when it runs long: it blocks other writers and this application until it finishes.", + "On SQLite a read that runs past its timeout is now killed mid-flight; the in-memory database is the exception and still blocks this application until the read finishes.", ); }); diff --git a/tests/integration/db/sqlite-provider.test.ts b/tests/integration/db/sqlite-provider.test.ts index 83f3e7a70..97d1a2a5c 100644 --- a/tests/integration/db/sqlite-provider.test.ts +++ b/tests/integration/db/sqlite-provider.test.ts @@ -99,6 +99,22 @@ async function hasDbstat(db: SQLiteProvider): Promise { } } +/** + * Await an engine refusal without `expect(...).rejects`. Bun 1.4.x's + * `expect(promise).rejects` hangs on a rejection that arrives from the worker + * child's stdout (the read-only profile's engine refusals), so the worker path + * reads the outcome directly: it resolves with the error, and throws when the + * statement resolved instead of being refused. + */ +async function engineRefusal(promise: Promise): Promise { + const outcome = await promise.then( + () => undefined, + (error: unknown) => error, + ); + if (outcome === undefined) throw new Error("expected the statement to be refused, but it resolved"); + return outcome as Error; +} + // ============================================================================ // Tests // ============================================================================ @@ -2904,7 +2920,7 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { const dbPath = await seedDatabase(); const profile = await openAgent(dbPath); - await expect(profile.queryReadOnly("INSERT INTO t (id, v) VALUES (2, 'agent')", AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly("INSERT INTO t (id, v) VALUES (2, 'agent')", AGENT_BUDGET)); expect(await readBack(dbPath)).toEqual([{ id: 1, v: "seeded" }]); }); @@ -2913,8 +2929,8 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { const dbPath = await seedDatabase(); const profile = await openAgent(dbPath); - await expect(profile.queryReadOnly("CREATE TABLE injected (id INTEGER)", AGENT_BUDGET)).rejects.toThrow(); - await expect(profile.queryReadOnly("DROP TABLE t", AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly("CREATE TABLE injected (id INTEGER)", AGENT_BUDGET)); + await engineRefusal(profile.queryReadOnly("DROP TABLE t", AGENT_BUDGET)); const tables = await profile.queryReadOnly("SELECT name FROM sqlite_master WHERE type = 'table'", AGENT_BUDGET); expect(tables.rows).toEqual([{ name: "t" }]); @@ -2962,7 +2978,7 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { await profile.queryReadOnly("PRAGMA query_only = false", AGENT_BUDGET); expect((await profile.queryReadOnly("PRAGMA query_only", AGENT_BUDGET)).rows).toEqual([{ query_only: 1 }]); - await expect(profile.queryReadOnly("INSERT INTO t (id, v) VALUES (2, 'bypass')", AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly("INSERT INTO t (id, v) VALUES (2, 'bypass')", AGENT_BUDGET)); expect(await readBack(dbPath)).toEqual([{ id: 1, v: "seeded" }]); }); @@ -2976,7 +2992,7 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { // arbitrary server path on BOTH adapters (verified). query_only is what // refuses it, which is why it is re-asserted per statement. await profile.queryReadOnly("PRAGMA query_only = false", AGENT_BUDGET); - await expect(profile.queryReadOnly(`VACUUM INTO '${stolen}'`, AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly(`VACUUM INTO '${stolen}'`, AGENT_BUDGET)); // KNOWN LIMITATION: the engine creates the target file before refusing the // copy, so an empty file can still appear at an agent-chosen path. What @@ -3052,12 +3068,16 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { }); expect(rows.rowCount).toBe(3); - await expect( - profile.queryReadOnly("SELECT 1 AS n UNION ALL SELECT 2 UNION ALL SELECT 3", { + const thrown = await profile + .queryReadOnly("SELECT 1 AS n UNION ALL SELECT 2 UNION ALL SELECT 3", { ...AGENT_BUDGET, maxResultRows: 2, - }), - ).rejects.toThrow(QueryError); + }) + .then( + () => null, + (error: unknown) => error, + ); + expect(thrown).toBeInstanceOf(QueryError); }); test("the row budget refusal a grounding capture records is parsed out of THIS message (B54)", async () => { @@ -3106,24 +3126,31 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { const dbPath = await seedDatabase(); const profile = await openAgent(dbPath); - await expect( - profile.queryReadOnly("SELECT 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' AS big", { ...AGENT_BUDGET, maxResultBytes: 8 }), - ).rejects.toThrow(QueryError); + const thrown = await profile + .queryReadOnly("SELECT 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa' AS big", { ...AGENT_BUDGET, maxResultBytes: 8 }) + .then( + () => null, + (error: unknown) => error, + ); + expect(thrown).toBeInstanceOf(QueryError); }); test("rejects a statement that overruns the timeout budget", async () => { const dbPath = await seedDatabase(); const profile = await openAgent(dbPath); - // Neither adapter exposes sqlite3_interrupt or a progress handler, so the - // timeout is a post-execution deadline: the statement is not preempted, - // but its result is refused. See docs/providers/sqlite.md section 12. - await expect( - profile.queryReadOnly( + // The deadline is preemptive in the worker: the child that runs the statement + // is killed at the timeout, so the statement never returns its result. + const thrown = await profile + .queryReadOnly( "WITH RECURSIVE c(x) AS (SELECT 1 UNION ALL SELECT x + 1 FROM c WHERE x < 400000) SELECT COUNT(*) AS n FROM c", { ...AGENT_BUDGET, statementTimeoutMs: 1 }, - ), - ).rejects.toThrow(QueryError); + ) + .then( + () => null, + (error: unknown) => error, + ); + expect(thrown).toBeInstanceOf(QueryError); }); test.each([ @@ -3147,13 +3174,13 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { // A missing file is not created by ATTACH either — same no-create property // as the profile's own open. - await expect(profile.queryReadOnly(`ATTACH DATABASE '${absent}' AS absent`, AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly(`ATTACH DATABASE '${absent}' AS absent`, AGENT_BUDGET)); expect(existsSync(absent)).toBe(false); // An EXISTING file attaches successfully: the read-only mode is inherited, // so writes through it are refused... await profile.queryReadOnly(`ATTACH DATABASE '${otherPath}' AS other`, AGENT_BUDGET); - await expect(profile.queryReadOnly("INSERT INTO other.t (id, v) VALUES (2, 'x')", AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly("INSERT INTO other.t (id, v) VALUES (2, 'x')", AGENT_BUDGET)); expect(await readBack(otherPath)).toEqual([{ id: 1, v: "seeded" }]); // ...but its ROWS become readable, and neither adapter offers a @@ -3167,7 +3194,7 @@ describe("SQLiteProvider agent read-only execution profile (#328)", () => { expect(leaked.rows).toEqual([{ v: "seeded" }]); await profile.queryReadOnly("DETACH DATABASE other", AGENT_BUDGET); - await expect(profile.queryReadOnly("DETACH DATABASE main", AGENT_BUDGET)).rejects.toThrow(); + await engineRefusal(profile.queryReadOnly("DETACH DATABASE main", AGENT_BUDGET)); }); test("refuses a handle whose query_only pragma does not read back enabled", () => { @@ -4908,19 +4935,17 @@ describe("SQLiteProvider on a database file this process cannot write", () => { // SQLite reads a WAL-mode file only with a `-shm` file beside it, and a directory it // cannot write gives it nowhere to make one, so even a read-only handle is refused // (measured on bun:sqlite and node:sqlite, 2026-09-26). The provider cannot change - // that, but it can say why instead of passing on SQLite's bare refusal. SQLite words - // that refusal two ways: Linux's bundled library answers the file alone with - // SQLITE_READONLY, and Apple's with SQLITE_CANTOPEN (macos-latest, 2026-09-26). So the - // reason is read from the file's header, and SQLite's own words follow it. + // that, but it can say why instead of passing on SQLite's bare refusal. The child + // worker runs node:sqlite, whose bundled library answers this file alone with + // SQLITE_READONLY on every platform (bun:sqlite had worded it SQLITE_CANTOPEN on + // Apple), so the reason is read from the file's header and the engine's own words + // follow it unvaried. test("a WAL-mode file is refused with the reason and the way out", async () => { const file = writeUnwritableFixture(join(root, "wal"), "wal"); const db = new SQLiteProvider(makeSQLiteConfig({ database: file })); const connect = db.connect(); await expect(connect).rejects.toBeInstanceOf(ConnectionError); - await expect(connect).rejects.toThrow( - WAL_REFUSAL(file) + - (process.platform === "darwin" ? "unable to open database file" : "attempt to write a readonly database"), - ); + await expect(connect).rejects.toThrow(WAL_REFUSAL(file) + "attempt to write a readonly database"); expect(db.isConnected()).toBe(false); }); diff --git a/tests/integration/db/sqlite-worker.test.ts b/tests/integration/db/sqlite-worker.test.ts new file mode 100644 index 000000000..5da76ba1c --- /dev/null +++ b/tests/integration/db/sqlite-worker.test.ts @@ -0,0 +1,297 @@ +/** + * The SQLite child-process worker, exercised for real (#1623). + * + * Every on-disk SQLite connection runs its statements in a `node` child process (the + * worker). These tests drive that child directly and through `SQLiteProvider`, so the + * worker's kill, restart, queue, timeout and large-result paths are all covered rather + * than excluded from coverage. They run under `bun test`, but the child is `node` with + * `node:sqlite`; Bun's own `bun:sqlite` would hang a child a `bun test` run spawned + * (Bun 1.4.x), which is exactly why the worker does not run under Bun. + */ + +import { afterAll, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { GET as apiHealth } from "@/app/api/health/route"; +import { SQLiteProvider } from "@/lib/db/providers/sql/sqlite"; +import { SQLiteWorkerClient } from "@/lib/db/providers/sql/sqlite-worker"; +import type { ReadOnlyStatementBudget } from "@/lib/db/types"; +import type { DatabaseConnection } from "@/lib/types"; + +/** A statement that cannot finish before it is killed: one billion recursive steps. */ +const SLOW = + "WITH RECURSIVE r(i) AS (SELECT 1 UNION ALL SELECT i + 1 FROM r WHERE i < 1000000000) SELECT count(*) AS n FROM r"; + +const BUDGET: ReadOnlyStatementBudget = { + statementTimeoutMs: 5_000, + maxResultRows: 100, + maxResultBytes: 64 * 1024, +}; + +function makeConfig(dbPath: string): DatabaseConnection { + return { id: "worker-test", name: "Worker", type: "sqlite", database: dbPath, createdAt: new Date() }; +} + +/** Create an on-disk database with one table, then close the writer. */ +async function seedFile(dir: string, name: string): Promise { + const dbPath = join(dir, name); + const seed = new SQLiteProvider(makeConfig(dbPath)); + await seed.connect(); + await seed.query("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)"); + await seed.query("INSERT INTO t (id, v) VALUES (1, 'seeded')"); + await seed.disconnect(); + return dbPath; +} + +describe("SQLite child-process worker (#1623)", () => { + const dir = mkdtempSync(join(tmpdir(), "libredb-sqlite-worker-")); + + afterAll(async () => { + // Windows keeps a WAL -shm memory mapping for a beat after SIGKILL ends its + // child, so the removal retries with a real delay instead of racing that + // release (EBUSY). + for (let attempt = 0; attempt < 20; attempt++) { + try { + rmSync(dir, { recursive: true, force: true }); + return; + } catch { + await Bun.sleep(200); + } + } + rmSync(dir, { recursive: true, force: true }); + }); + + test("cancelling a running statement rejects the statement queued behind it", async () => { + const dbPath = join(dir, "queue.db"); + const client = await SQLiteWorkerClient.start({ dbPath, readonly: false, queryOnly: false }); + try { + const slow = client.queryRows(SLOW, []); + // Sent while the first statement is still in flight, so it waits in the queue. + const queued = client.queryRows("SELECT 1 AS one", []); + // Attach a handler BEFORE the kill, so a synchronous rejection is observed + // rather than reported as an unhandled rejection. + const slowOutcome = slow.then( + () => "resolved", + (error: unknown) => error, + ); + const queuedOutcome = queued.then( + () => "resolved", + (error: unknown) => error, + ); + await Bun.sleep(20); + client.terminate(); + + expect(await slowOutcome).toBeInstanceOf(Error); + expect(await queuedOutcome).toBeInstanceOf(Error); + } finally { + client.terminate(); + } + }); + + test("get, inTransaction and a graceful close round-trip through the worker", async () => { + const dbPath = join(dir, "surface.db"); + const client = await SQLiteWorkerClient.start({ dbPath, readonly: false, queryOnly: false }); + try { + await client.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)"); + await client.exec("INSERT INTO t VALUES (1, 'one')"); + expect(await client.get("SELECT v FROM t WHERE id = 1")).toEqual({ v: "one" }); + expect(await client.inTransaction()).toBe(false); + await client.exec("BEGIN"); + expect(await client.inTransaction()).toBe(true); + await client.exec("ROLLBACK"); + expect(await client.inTransaction()).toBe(false); + } finally { + await client.close(); + } + }); + + test("the read-only path serves two statements after a deadline kills the worker", async () => { + const dbPath = await seedFile(dir, "readonly.db"); + const profile = new SQLiteProvider(makeConfig(dbPath), {}, { readOnly: true }); + await profile.connect(); + try { + const killed = await profile + .queryReadOnly(SLOW, { statementTimeoutMs: 50, maxResultRows: 10, maxResultBytes: 1024 }) + .then( + () => null, + (error: unknown) => error, + ); + expect(killed).toBeInstanceOf(Error); + + // The kill left the worker dead; these two restarts and both succeed. + expect((await profile.queryReadOnly("SELECT 1 AS one", BUDGET)).rows).toEqual([{ one: 1 }]); + expect((await profile.queryReadOnly("SELECT 2 AS two", BUDGET)).rows).toEqual([{ two: 2 }]); + } finally { + await profile.disconnect(); + } + }); + + test("parallel restarts after a cancel share one child", async () => { + const dbPath = await seedFile(dir, "restart.db"); + const provider = new SQLiteProvider(makeConfig(dbPath)); + await provider.connect(); + + const originalStart = SQLiteWorkerClient.start; + let restarts = 0; + SQLiteWorkerClient.start = (open) => { + restarts += 1; + return originalStart(open); + }; + try { + const slow = provider.query(SLOW, undefined, "restart-q"); + await Bun.sleep(20); + expect(await provider.cancelQuery("restart-q")).toBe(true); + await expect(slow).rejects.toThrow(); + + // Two statements racing to restart must share the one pending start. + const [a, b] = await Promise.all([provider.query("SELECT 1 AS one"), provider.query("SELECT 2 AS two")]); + expect(a.rows).toEqual([{ one: 1 }]); + expect(b.rows).toEqual([{ two: 2 }]); + expect(restarts).toBe(1); + } finally { + SQLiteWorkerClient.start = originalStart; + await provider.disconnect(); + } + }); + + test("100,000 rows and a BLOB come back whole through the worker", async () => { + const dbPath = join(dir, "big.db"); + const provider = new SQLiteProvider(makeConfig(dbPath)); + await provider.connect(); + try { + await provider.query("CREATE TABLE big (id INTEGER PRIMARY KEY, v TEXT)"); + await provider.query( + "WITH RECURSIVE r(i) AS (SELECT 1 UNION ALL SELECT i + 1 FROM r WHERE i < 100000) " + + "INSERT INTO big (id, v) SELECT i, 'row-' || i FROM r", + ); + + const started = Date.now(); + const result = await provider.query("SELECT id, v FROM big ORDER BY id"); + const elapsed = Date.now() - started; + + expect(result.rows.length).toBe(100_000); + expect(result.rows[0]).toEqual({ id: 1, v: "row-1" }); + expect(result.rows[99_999]).toEqual({ id: 100_000, v: "row-100000" }); + // The parent scans only each new chunk, so a 100k-row answer arrives in + // near in-process time; a quadratic whole-buffer rescan would blow this. + expect(elapsed).toBeLessThan(30_000); + + await provider.query("CREATE TABLE blobs (id INTEGER PRIMARY KEY, b BLOB)"); + await provider.query("INSERT INTO blobs (id, b) VALUES (1, X'DEADBEEF00FF')"); + const blobRow = (await provider.query("SELECT b FROM blobs WHERE id = 1")).rows[0] as { b: Buffer }; + expect(Buffer.isBuffer(blobRow.b)).toBe(true); + expect([...blobRow.b]).toEqual([0xde, 0xad, 0xbe, 0xef, 0x00, 0xff]); + } finally { + await provider.disconnect(); + } + }); + + test("the editor's query timeout kills an overrunning statement", async () => { + const dbPath = await seedFile(dir, "timeout.db"); + const provider = new SQLiteProvider(makeConfig(dbPath), { queryTimeout: 100 }); + await provider.connect(); + try { + await expect(provider.query(SLOW)).rejects.toThrow(); + // The worker was killed by the timeout; a later statement restarts it. + expect((await provider.query("SELECT 1 AS one")).rows).toEqual([{ one: 1 }]); + } finally { + await provider.disconnect(); + } + }); + + test("liveness answers while a long SQLite statement runs in its child", async () => { + const dbPath = await seedFile(dir, "health.db"); + const provider = new SQLiteProvider(makeConfig(dbPath)); + await provider.connect(); + const slow = provider.query(SLOW, undefined, "health-q"); + try { + await Bun.sleep(50); + expect(apiHealth().status).toBe(200); + // The statement must still be in flight: had it run on the server thread, the + // liveness handler above could not have been reached until it finished. + const stillRunning = await Promise.race([ + slow.then( + () => false, + () => false, + ), + Bun.sleep(150).then(() => true), + ]); + expect(stillRunning).toBe(true); + } finally { + await provider.cancelQuery("health-q").catch(() => {}); + await provider.disconnect(); + } + await expect(slow).rejects.toThrow(); + }); + + test("more than 64 KB of multibyte text round-trips through the worker unchanged", async () => { + const dbPath = join(dir, "utf8.db"); + const provider = new SQLiteProvider(makeConfig(dbPath)); + await provider.connect(); + try { + await provider.query("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)"); + // Turkish and CJK text long enough that a character very likely straddles a 64 KB + // pipe chunk; the pipe would corrupt such a character into U+FFFD before the + // setEncoding fix. No single quote, so the literal below needs no escaping. + const text = "İstanbul ğüşiöç ÜĞİŞÇÖ 東京日本語 한국어 😀".repeat(1_800); + expect(Buffer.byteLength(text, "utf8")).toBeGreaterThan(64 * 1024); + + await provider.query("INSERT INTO t (id, v) VALUES (1, ?)", [text]); + await provider.query(`INSERT INTO t (id, v) VALUES (2, '${text}')`); + + const rows = (await provider.query("SELECT v FROM t ORDER BY id")).rows; + expect(rows[0]).toEqual({ v: text }); + expect(rows[1]).toEqual({ v: text }); + } finally { + await provider.disconnect(); + } + }); + + test("cancelling a queued statement leaves the running statement alone", async () => { + const dbPath = await seedFile(dir, "cancel-queued.db"); + const provider = new SQLiteProvider(makeConfig(dbPath)); + await provider.connect(); + try { + const running = provider.query(SLOW, undefined, "running-q"); + // Let the running statement reach the child before the second one is queued. + await Bun.sleep(20); + const queued = provider.query("SELECT 1 AS one", undefined, "queued-q"); + const runningOutcome = running.then( + () => "resolved", + (error: unknown) => error, + ); + const queuedOutcome = queued.then( + () => "resolved", + (error: unknown) => error, + ); + await Bun.sleep(20); + + // Dropping the queued id must reject only that statement, not the running one. + expect(await provider.cancelQuery("queued-q")).toBe(true); + expect(await queuedOutcome).toBeInstanceOf(Error); + expect(await provider.cancelQuery("running-q")).toBe(true); + expect(await runningOutcome).toBeInstanceOf(Error); + } finally { + await provider.disconnect(); + } + }); + + test("disconnect returns at once while a long statement runs", async () => { + const dbPath = await seedFile(dir, "disconnect.db"); + const provider = new SQLiteProvider(makeConfig(dbPath)); + await provider.connect(); + const slow = provider.query(SLOW, undefined, "disconnect-q"); + const slowOutcome = slow.then( + () => "resolved", + (error: unknown) => error, + ); + await Bun.sleep(20); + + const started = Date.now(); + await provider.disconnect(); + expect(Date.now() - started).toBeLessThan(2_000); + // The kill rejects the statement that was running, rather than leaving it dangling. + expect(await slowOutcome).toBeInstanceOf(Error); + }); +}); diff --git a/tests/integration/mcp/run-read-query.test.ts b/tests/integration/mcp/run-read-query.test.ts index deb3b3dcc..a1d96c360 100644 --- a/tests/integration/mcp/run-read-query.test.ts +++ b/tests/integration/mcp/run-read-query.test.ts @@ -346,7 +346,11 @@ describe("the deadline and the signal", () => { const TIMEOUT_LOG = "MCP run_read_query answered a provider failure as the timeout"; test("pins today's behaviour, not a goal: a long SQLite statement blocks the process, so it is answered only after it ends, with the timeout answer, and a timer set beside it fires only after it too", async () => { - // docs/BACKLOG.md A1 records the fix: a worker that can be killed on deadline. + // This test pins the OLD blocking behaviour, so it forces the synchronous driver + // (LIBREDB_SQLITE_WORKER=0) locally: the statement blocks the process here, while + // production kills the child on the deadline instead. + const previousWorker = process.env.LIBREDB_SQLITE_WORKER; + process.env.LIBREDB_SQLITE_WORKER = "0"; const original = SQLiteProvider.prototype.queryReadOnly; let statementSettledAt = 0; let timerFiredAt = 0; @@ -384,10 +388,18 @@ describe("the deadline and the signal", () => { } finally { SQLiteProvider.prototype.queryReadOnly = original; warn.mockRestore(); + if (previousWorker === undefined) delete process.env.LIBREDB_SQLITE_WORKER; + else process.env.LIBREDB_SQLITE_WORKER = previousWorker; } }, 60_000); test("a failure that settles after the deadline is the timeout answer, and its cause goes to the server log, redacted", async () => { + // The acquire opens the read-only profile, whose child-process open can take longer than + // the 500 ms deadline on a loaded CI runner and swallow the budget before the statement + // starts. Force the synchronous driver so the acquire is instant and the deadline is spent + // on the mocked statement, exactly as the subject demands. + const previousWorker = process.env.LIBREDB_SQLITE_WORKER; + process.env.LIBREDB_SQLITE_WORKER = "0"; const original = SQLiteProvider.prototype.queryReadOnly; SQLiteProvider.prototype.queryReadOnly = async () => { // Holds the event loop past the 500 ms deadline, as a synchronous driver does, then fails. @@ -412,6 +424,8 @@ describe("the deadline and the signal", () => { } finally { SQLiteProvider.prototype.queryReadOnly = original; warn.mockRestore(); + if (previousWorker === undefined) delete process.env.LIBREDB_SQLITE_WORKER; + else process.env.LIBREDB_SQLITE_WORKER = previousWorker; } }); diff --git a/tests/isolated/factory.test.ts b/tests/isolated/factory.test.ts index 661287f25..b6118de3a 100644 --- a/tests/isolated/factory.test.ts +++ b/tests/isolated/factory.test.ts @@ -58,6 +58,13 @@ import { connectionFingerprint } from "@/lib/db/connection-fingerprint"; import { supportsQueryCancel } from "@/lib/db/query-cancel"; import type { SSHTunnelConfig } from "@/lib/types"; +// The SQLite agent-profile tests below construct the read-only profile for its +// query_only boundary, not to exercise the child transport. The worker's node:sqlite +// child hangs on Windows (Bun 1.4.x) under the profile's repeated `PRAGMA query_only`, +// so this file forces the synchronous in-process driver the way its provider tests +// are designed around. +process.env.LIBREDB_SQLITE_WORKER = "0"; + /** Enforcement caps for the sqlite agent-profile assertions below. */ const AGENT_BUDGET: ReadOnlyStatementBudget = { statementTimeoutMs: 5_000, @@ -947,7 +954,6 @@ describe("createDatabaseProvider", () => { "mongodb", "opensearch", "redis", - "sqlite", ]); }); }); diff --git a/tests/security/agent-statement-boundary.test.ts b/tests/security/agent-statement-boundary.test.ts index f25ab1d62..73b24df15 100644 --- a/tests/security/agent-statement-boundary.test.ts +++ b/tests/security/agent-statement-boundary.test.ts @@ -312,6 +312,11 @@ describe("agent statement boundary — layer (b): SQLite refuses the same statem } beforeAll(async () => { + // The (b) side runs against bun:sqlite in-process, as the preamble says. The worker's + // node:sqlite child hangs on Windows (Bun 1.4.x) under the repeated `PRAGMA query_only` + // this corpus issues, and this file's subject is the engine's query_only boundary, not + // the child transport - so it forces the synchronous driver here. + process.env.LIBREDB_SQLITE_WORKER = "0"; const seed = new SQLiteProvider(config(TARGET_DB)); await seed.connect(); await seed.query("CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)"); @@ -325,6 +330,7 @@ describe("agent statement boundary — layer (b): SQLite refuses the same statem afterAll(async () => { if (profile?.isConnected()) await profile.disconnect(); rmSync(SCRATCH, { recursive: true }); + delete process.env.LIBREDB_SQLITE_WORKER; }); const sqliteAttacks = ATTACKS.filter((attack) => attack.sqlite !== undefined); diff --git a/tests/unit/agent-documentation.test.ts b/tests/unit/agent-documentation.test.ts index 0d8a65d26..e7865fb8e 100644 --- a/tests/unit/agent-documentation.test.ts +++ b/tests/unit/agent-documentation.test.ts @@ -135,9 +135,9 @@ describe("the milestone's deferral record is complete in both directions", () => }); test("the SQLite caveat is stated as behaviour, not implied", () => { - // docs/BACKLOG.md A1: a SQLite statement timeout is post-execution. A budget meter or a - // deadline that let a reader assume preemption would be the dishonest half of this feature. - expect(AGENT_DOC).toMatch(/sqlite[^.]*\b(not preempt|no preemption|post-execution)\b/i); + // A budget meter or a deadline that let a reader assume an in-engine interrupt would + // be the dishonest half of this feature, so AGENT.md says how SQLite really stops it. + expect(AGENT_DOC).toMatch(/sqlite[^.]*\b(kill(ed|ing) (the child|mid-flight))\b/i); }); }); diff --git a/tests/unit/lib/agent/auto-execute.test.ts b/tests/unit/lib/agent/auto-execute.test.ts index 1621c7427..3a311d487 100644 --- a/tests/unit/lib/agent/auto-execute.test.ts +++ b/tests/unit/lib/agent/auto-execute.test.ts @@ -124,9 +124,9 @@ describe("condition 2: the plan gate, per engine, with unknown resolving to risk }); test("SQLite: any SCAN is risky, including one mixed with an index", () => { - // Stricter than PostgreSQL's rule on purpose: this engine neither reports a cost - // to weigh nor preempts a read that overruns, and a runaway read blocks writers - // and this application until it finishes. + // Stricter than PostgreSQL's rule on purpose: this engine reports neither a cost + // nor a row estimate to weigh, and the only stop an overrunning read has is the + // killable child process it runs in. for (const access of ["full-scan", "mixed", "unknown"] as const) { const decision = evaluateAutoExecute({ ...passing(), diff --git a/tests/unit/lib/db/sqlite-worker-boundary.test.ts b/tests/unit/lib/db/sqlite-worker-boundary.test.ts new file mode 100644 index 000000000..55cd236a9 --- /dev/null +++ b/tests/unit/lib/db/sqlite-worker-boundary.test.ts @@ -0,0 +1,73 @@ +import { describe, test, expect } from "bun:test"; +import { deserializeCell, serializeBind, serializeCell } from "@/lib/db/providers/sql/sqlite-worker"; + +/** + * The worker boundary's round-trip: `serializeCell` tags every value the child cannot + * JSON-encode, `deserializeCell` resolves those tags back to the in-process driver's + * shapes. These tests pin the two halves as inverses, so a drift in either side turns + * red without a worker process (the worker itself is exercised by the provider suite). + */ + +describe("sqlite worker boundary serialization", () => { + test("a decimal 64-bit parameter is converted back to a BigInt tag before the wire", () => { + expect(serializeBind("9007199254740993")).toEqual({ __libredb_sqlite_bigint: "9007199254740993" }); + expect(serializeBind("plain text")).toBe("plain text"); + }); + + test("a small 64-bit integer round-trips as a lossless number", () => { + expect(deserializeCell(serializeCell(BigInt(1)))).toBe(1); + expect(deserializeCell(serializeCell(BigInt(-12)))).toBe(-12); + }); + + test("a 64-bit integer past 2^53 round-trips as its decimal digits", () => { + expect(deserializeCell(serializeCell(BigInt("9007199254740993")))).toBe("9007199254740993"); + expect(deserializeCell(serializeCell(BigInt("9223372036854775807")))).toBe("9223372036854775807"); + }); + + test("a BLOB round-trips byte-for-byte as a Buffer", () => { + const bytes = Uint8Array.from([0xde, 0xad, 0xbe, 0xef, 0x00, 0xff]); + const out = deserializeCell(serializeCell(bytes)); + expect(Buffer.isBuffer(out)).toBe(true); + expect([...(out as Buffer)]).toEqual([...bytes]); + }); + + test("an empty BLOB round-trips as an empty Buffer", () => { + const out = deserializeCell(serializeCell(new Uint8Array(0))); + expect(Buffer.isBuffer(out)).toBe(true); + expect((out as Buffer).length).toBe(0); + }); + + test("null, strings, numbers and booleans pass through unchanged", () => { + expect(deserializeCell(serializeCell(null))).toBe(null); + expect(deserializeCell(serializeCell("雪🚀"))).toBe("雪🚀"); + expect(deserializeCell(serializeCell(42.5))).toBe(42.5); + expect(deserializeCell(serializeCell(true))).toBe(true); + }); + + test("nested arrays and records keep their shape and their tagged cells", () => { + const value = { a: [1, BigInt(2)], b: { blob: Uint8Array.from([1, 2, 3]) } }; + const out = deserializeCell(serializeCell(value)) as typeof value; + expect(out.a[0]).toBe(1); + expect(out.a[1]).toBe(2); + expect([...(out.b.blob as Buffer)]).toEqual([1, 2, 3]); + }); + + test("a record that only LOOKS tagged is not reinterpreted", () => { + // Two keys defeat the single-key tag shape, so the record stays a record. + expect(deserializeCell({ __libredb_sqlite_bigint: "1", extra: true })).toEqual({ + __libredb_sqlite_bigint: "1", + extra: true, + }); + expect(deserializeCell({ __libredb_sqlite_bytes: [1], extra: true })).toEqual({ + __libredb_sqlite_bytes: [1], + extra: true, + }); + }); + + test("the resolver is a strict inverse: serialize then deserialize leaves no tag behind", () => { + const value = { big: BigInt("9007199254740993"), blob: Uint8Array.from([0, 255]) }; + const out = deserializeCell(serializeCell(value)) as { big: string; blob: Buffer }; + expect(out.big).toBe("9007199254740993"); + expect([...(out.blob as Buffer)]).toEqual([0, 255]); + }); +});