diff --git a/crates/codegraph-core/src/db/connection.rs b/crates/codegraph-core/src/db/connection.rs index a41237b8..5297796e 100644 --- a/crates/codegraph-core/src/db/connection.rs +++ b/crates/codegraph-core/src/db/connection.rs @@ -16,6 +16,11 @@ use crate::db::repository::edges::{self, EdgeRow}; use crate::domain::graph::builder::stages::insert_nodes::{self, FileHashEntry, InsertNodesBatch}; use crate::graph::classifiers::roles::{self, RoleSummary}; +/// Fallback `PRAGMA busy_timeout` (ms) used when the caller doesn't pass a +/// resolved value. Mirrors `DEFAULTS.db.busyTimeoutMs` in +/// `src/infrastructure/config.ts` — keep both in sync. +const DEFAULT_BUSY_TIMEOUT_MS: u32 = 5000; + // ── Migration DDL (mirrored from src/db/migrations.ts) ────────────────── struct Migration { @@ -483,8 +488,12 @@ pub struct NativeDatabase { impl NativeDatabase { /// Open a read-write connection to the database at `db_path`. /// Creates the file and parent directories if they don't exist. + /// + /// `busy_timeout_ms` mirrors `config.db.busyTimeoutMs` on the TS side + /// (`DEFAULTS.db.busyTimeoutMs` in `src/infrastructure/config.ts`); + /// defaults to `DEFAULT_BUSY_TIMEOUT_MS` when omitted. #[napi(factory)] - pub fn open_read_write(db_path: String) -> napi::Result { + pub fn open_read_write(db_path: String, busy_timeout_ms: Option) -> napi::Result { let flags = OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_CREATE | OpenFlags::SQLITE_OPEN_NO_MUTEX; @@ -498,12 +507,13 @@ impl NativeDatabase { // are not cache-coherent on Windows, leading to SQLITE_CORRUPT (#715). // Read-only connections keep mmap since they don't share a WAL file // with a concurrent writer from a different library. - conn.execute_batch( + let busy_timeout_ms = busy_timeout_ms.unwrap_or(DEFAULT_BUSY_TIMEOUT_MS); + conn.execute_batch(&format!( "PRAGMA journal_mode = WAL; \ PRAGMA synchronous = NORMAL; \ - PRAGMA busy_timeout = 5000; \ + PRAGMA busy_timeout = {busy_timeout_ms}; \ PRAGMA temp_store = MEMORY;", - ) + )) .map_err(|e| napi::Error::from_reason(format!("Failed to set pragmas: {e}")))?; Ok(Self { conn: SendWrapper::new(Some(conn)), @@ -512,17 +522,22 @@ impl NativeDatabase { } /// Open a read-only connection to the database at `db_path`. + /// + /// `busy_timeout_ms` mirrors `config.db.busyTimeoutMs` on the TS side + /// (`DEFAULTS.db.busyTimeoutMs` in `src/infrastructure/config.ts`); + /// defaults to `DEFAULT_BUSY_TIMEOUT_MS` when omitted. #[napi(factory)] - pub fn open_readonly(db_path: String) -> napi::Result { + pub fn open_readonly(db_path: String, busy_timeout_ms: Option) -> napi::Result { let flags = OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_NO_MUTEX; let conn = Connection::open_with_flags(&db_path, flags) .map_err(|e| napi::Error::from_reason(format!("Failed to open DB readonly: {e}")))?; conn.set_prepared_statement_cache_capacity(64); - conn.execute_batch( - "PRAGMA busy_timeout = 5000; \ + let busy_timeout_ms = busy_timeout_ms.unwrap_or(DEFAULT_BUSY_TIMEOUT_MS); + conn.execute_batch(&format!( + "PRAGMA busy_timeout = {busy_timeout_ms}; \ PRAGMA mmap_size = 268435456; \ PRAGMA temp_store = MEMORY;", - ) + )) .map_err(|e| napi::Error::from_reason(format!("Failed to set pragmas: {e}")))?; Ok(Self { conn: SendWrapper::new(Some(conn)), diff --git a/src/db/connection.ts b/src/db/connection.ts index 285a84db..475a6cde 100644 --- a/src/db/connection.ts +++ b/src/db/connection.ts @@ -474,7 +474,10 @@ export function resolveBusyTimeoutMs(customDbPath?: string): number { } /** Open a NativeRepository via rusqlite, throwing DbError if the DB file is missing. */ -function openRepoNative(customDbPath?: string): { repo: Repository; close(): void } { +function openRepoNative( + customDbPath: string | undefined, + busyTimeoutMs: number, +): { repo: Repository; close(): void } { const dbPath = findDbPath(customDbPath); if (!fs.existsSync(dbPath)) { throw new DbError( @@ -483,7 +486,7 @@ function openRepoNative(customDbPath?: string): { repo: Repository; close(): voi ); } const native = getNative(); - const ndb = native.NativeDatabase.openReadonly(dbPath); + const ndb = native.NativeDatabase.openReadonly(dbPath, busyTimeoutMs); try { warnOnVersionMismatch(() => ndb.getBuildMeta('codegraph_version')); const repo = new NativeRepository(ndb, dbPath); @@ -529,10 +532,11 @@ function wrapInjectedRepo(repo: Repository): { repo: Repository; close(): void } function tryOpenRepoNative( customDbPath: string | undefined, engine: 'native' | 'wasm' | 'auto', + busyTimeoutMs: number, ): { repo: Repository; close(): void } | undefined { if (engine === 'wasm' || !isNativeAvailable()) return undefined; try { - return openRepoNative(customDbPath); + return openRepoNative(customDbPath, busyTimeoutMs); } catch (e) { // Re-throw user-visible errors (e.g. DB not found) — only silently // fall back for native-engine failures (e.g. incompatible native binary). @@ -582,7 +586,7 @@ export function openRepo( // This ensures --engine wasm and benchmark workers bypass the native path. const { engine, busyTimeoutMs } = resolveDbSettings(customDbPath, opts.engine); - const native = tryOpenRepoNative(customDbPath, engine); + const native = tryOpenRepoNative(customDbPath, engine, busyTimeoutMs); if (native) return native; return openRepoSqliteFallback(customDbPath, busyTimeoutMs); @@ -622,7 +626,7 @@ export function openReadonlyWithNative( try { const dbPath = findDbPath(customPath); const native = getNative(); - nativeDb = native.NativeDatabase.openReadonly(dbPath); + nativeDb = native.NativeDatabase.openReadonly(dbPath, busyTimeoutMs); } catch (e) { const msg = toErrorMessage(e); if (isBusyOrLockedError(msg)) { diff --git a/src/domain/graph/builder/stages/native-db-lifecycle.ts b/src/domain/graph/builder/stages/native-db-lifecycle.ts index 7395c0d6..b21d66ee 100644 --- a/src/domain/graph/builder/stages/native-db-lifecycle.ts +++ b/src/domain/graph/builder/stages/native-db-lifecycle.ts @@ -44,7 +44,7 @@ export function reopenNativeDb(ctx: PipelineContext, label: string): void { const native = loadNative(); if (!native?.NativeDatabase) return; try { - ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath); + ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath, ctx.config.db.busyTimeoutMs); } catch (e) { debug(`reopen nativeDb for ${label} failed: ${toErrorMessage(e)}`); ctx.nativeDb = undefined; diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index 236ecbed..94f60c70 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -579,7 +579,7 @@ async function runPostNativeAnalysis( const native = loadNative(); if (native?.NativeDatabase) { try { - ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath); + ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath, ctx.config.db.busyTimeoutMs); if (ctx.engineOpts) ctx.engineOpts.nativeDb = ctx.nativeDb; } catch { ctx.nativeDb = undefined; @@ -1955,7 +1955,7 @@ function openNativeDatabase(ctx: PipelineContext): void { // is kept and transferred to the NativeDbProxy below, not released here. ctx.db.close(); acquireAdvisoryLock(ctx.dbPath); - ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath); + ctx.nativeDb = native.NativeDatabase.openReadWrite(ctx.dbPath, ctx.config.db.busyTimeoutMs); ctx.nativeDb.initSchema(); // Replace ctx.db with a NativeDbProxy so post-native JS fallback // (structure, analysis) can use it without reopening better-sqlite3. diff --git a/src/features/branch-compare.ts b/src/features/branch-compare.ts index 11bc2574..394842bc 100644 --- a/src/features/branch-compare.ts +++ b/src/features/branch-compare.ts @@ -3,6 +3,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { getDatabase } from '../db/better-sqlite3.js'; +import { resolveBusyTimeoutMs } from '../db/index.js'; import { buildGraph } from '../domain/graph/builder.js'; import { kindIcon } from '../domain/queries.js'; import { debug } from '../infrastructure/logger.js'; @@ -120,7 +121,7 @@ function openNativeDbForFanMetrics(dbPath: string): NativeDatabase | undefined { if (!isNativeAvailable()) return undefined; try { const native = getNative(); - return native.NativeDatabase.openReadonly(dbPath); + return native.NativeDatabase.openReadonly(dbPath, resolveBusyTimeoutMs(dbPath)); } catch (e) { debug(`loadSymbolsFromDb: native path failed: ${toErrorMessage(e)}`); return undefined; diff --git a/src/types.ts b/src/types.ts index 3946f779..06f498bd 100644 --- a/src/types.ts +++ b/src/types.ts @@ -2390,8 +2390,8 @@ export interface NativeAddon { extractDataflowAnalysisBatch?(filePaths: string[]): (DataflowResult | null)[]; ParseTreeCache: new () => NativeParseTreeCache; NativeDatabase: { - openReadWrite(dbPath: string): NativeDatabase; - openReadonly(dbPath: string): NativeDatabase; + openReadWrite(dbPath: string, busyTimeoutMs?: number): NativeDatabase; + openReadonly(dbPath: string, busyTimeoutMs?: number): NativeDatabase; }; } diff --git a/tests/unit/native-db-busy-timeout-threading.test.ts b/tests/unit/native-db-busy-timeout-threading.test.ts new file mode 100644 index 00000000..253d6c4f --- /dev/null +++ b/tests/unit/native-db-busy-timeout-threading.test.ts @@ -0,0 +1,121 @@ +/** + * Regression tests for issue #1882: the JS call sites that open a + * `NativeDatabase` must pass the resolved `config.db.busyTimeoutMs` through + * as the factory's `busyTimeoutMs` argument, instead of silently relying on + * the Rust-side hardcoded default. + * + * Mocks `infrastructure/native.js` (the same pattern as + * `tests/unit/openRepo-busy.test.ts`) so the assertions are about *what + * argument each call site passes*, independent of whether a native addon is + * actually built for the current platform. + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; + +const CUSTOM_BUSY_TIMEOUT_MS = 424242; + +const openReadonlyCalls: Array<[string, number | undefined]> = []; +const openReadWriteCalls: Array<[string, number | undefined]> = []; + +function makeFakeNativeDb() { + return { + getBuildMeta: () => null, + close: () => {}, + initSchema: () => {}, + exec: () => {}, + }; +} + +vi.mock('../../src/infrastructure/native.js', () => ({ + isNativeAvailable: () => true, + getNative: () => ({ + NativeDatabase: { + openReadonly: (dbPath: string, busyTimeoutMs?: number) => { + openReadonlyCalls.push([dbPath, busyTimeoutMs]); + return makeFakeNativeDb(); + }, + openReadWrite: (dbPath: string, busyTimeoutMs?: number) => { + openReadWriteCalls.push([dbPath, busyTimeoutMs]); + return makeFakeNativeDb(); + }, + }, + }), + loadNative: () => ({ + NativeDatabase: { + openReadonly: (dbPath: string, busyTimeoutMs?: number) => { + openReadonlyCalls.push([dbPath, busyTimeoutMs]); + return makeFakeNativeDb(); + }, + openReadWrite: (dbPath: string, busyTimeoutMs?: number) => { + openReadWriteCalls.push([dbPath, busyTimeoutMs]); + return makeFakeNativeDb(); + }, + }, + }), +})); + +import { + closeDb, + initSchema, + openDb, + openReadonlyWithNative, + openRepo, +} from '../../src/db/index.js'; +import { PipelineContext } from '../../src/domain/graph/builder/context.js'; +import { reopenNativeDb } from '../../src/domain/graph/builder/stages/native-db-lifecycle.js'; + +let tmpDir: string; +let dbPath: string; + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-native-busy-threading-')); + dbPath = path.join(tmpDir, '.codegraph', 'graph.db'); + const db = openDb(dbPath); + initSchema(db); + closeDb(db); + fs.writeFileSync( + path.join(tmpDir, '.codegraphrc.json'), + JSON.stringify({ db: { busyTimeoutMs: CUSTOM_BUSY_TIMEOUT_MS } }), + ); +}); + +afterAll(() => { + if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +describe('openRepo threads busyTimeoutMs into NativeDatabase.openReadonly', () => { + it('passes the configured busyTimeoutMs to the native factory', () => { + openReadonlyCalls.length = 0; + const { close } = openRepo(dbPath); + close(); + expect(openReadonlyCalls).toHaveLength(1); + expect(openReadonlyCalls[0]?.[1]).toBe(CUSTOM_BUSY_TIMEOUT_MS); + }); +}); + +describe('openReadonlyWithNative threads busyTimeoutMs into NativeDatabase.openReadonly', () => { + it('passes the configured busyTimeoutMs to the native factory', () => { + openReadonlyCalls.length = 0; + const { close } = openReadonlyWithNative(dbPath); + close(); + expect(openReadonlyCalls).toHaveLength(1); + expect(openReadonlyCalls[0]?.[1]).toBe(CUSTOM_BUSY_TIMEOUT_MS); + }); +}); + +describe('reopenNativeDb (build pipeline) threads ctx.config.db.busyTimeoutMs into NativeDatabase.openReadWrite', () => { + it('passes ctx.config.db.busyTimeoutMs to the native factory', () => { + openReadWriteCalls.length = 0; + const ctx = new PipelineContext(); + ctx.dbPath = dbPath; + ctx.opts = { engine: 'native' }; + ctx.config = { db: { busyTimeoutMs: CUSTOM_BUSY_TIMEOUT_MS } } as PipelineContext['config']; + + reopenNativeDb(ctx, 'test'); + + expect(openReadWriteCalls).toHaveLength(1); + expect(openReadWriteCalls[0]?.[1]).toBe(CUSTOM_BUSY_TIMEOUT_MS); + }); +}); diff --git a/tests/unit/native-db-busy-timeout.test.ts b/tests/unit/native-db-busy-timeout.test.ts new file mode 100644 index 00000000..6cde1ccf --- /dev/null +++ b/tests/unit/native-db-busy-timeout.test.ts @@ -0,0 +1,74 @@ +/** + * Regression tests for issue #1882: `config.db.busyTimeoutMs` must reach the + * Rust native DB layer (`NativeDatabase::open_readonly` / `open_read_write`), + * not just the TS-side better-sqlite3 pragma threaded by #1763. + * + * Verifies the applied `busy_timeout` via `queryGet('PRAGMA busy_timeout', [])` + * rather than the `pragma()` helper — `pragma()` only supports TEXT-affinity + * results (see #2019) and throws for INTEGER-returning pragmas like + * `busy_timeout`. + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { DEFAULTS } from '../../src/infrastructure/config.js'; +import { getNative, isNativeAvailable } from '../../src/infrastructure/native.js'; +import type { NativeDatabase } from '../../src/types.js'; + +const hasNativeDb = + isNativeAvailable() && typeof getNative().NativeDatabase?.openReadWrite === 'function'; + +/** Read the effective `busy_timeout` pragma value via queryGet (avoids the pragma() TEXT-only bug, #2019). */ +function readBusyTimeout(ndb: NativeDatabase): number { + const row = ndb.queryGet('PRAGMA busy_timeout', []) as { timeout: number } | null; + return row?.timeout as number; +} + +describe.skipIf(!hasNativeDb)('NativeDatabase busy_timeout_ms threading (Rust layer)', () => { + let tmpDir: string; + let dbPath: string; + let nativeDb: NativeDatabase | undefined; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-native-busy-')); + dbPath = path.join(tmpDir, 'test.db'); + }); + + afterEach(() => { + try { + nativeDb?.close(); + } catch { + /* already closed */ + } + nativeDb = undefined; + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + it('openReadWrite applies a configured busyTimeoutMs', () => { + const NativeDB = getNative().NativeDatabase; + nativeDb = NativeDB.openReadWrite(dbPath, 424242); + expect(readBusyTimeout(nativeDb)).toBe(424242); + }); + + it('openReadWrite defaults to DEFAULTS.db.busyTimeoutMs when omitted', () => { + const NativeDB = getNative().NativeDatabase; + nativeDb = NativeDB.openReadWrite(dbPath); + expect(readBusyTimeout(nativeDb)).toBe(DEFAULTS.db.busyTimeoutMs); + }); + + it('openReadonly applies a configured busyTimeoutMs', () => { + const NativeDB = getNative().NativeDatabase; + // openReadonly requires the file to already exist. + NativeDB.openReadWrite(dbPath).close(); + nativeDb = NativeDB.openReadonly(dbPath, 99999); + expect(readBusyTimeout(nativeDb)).toBe(99999); + }); + + it('openReadonly defaults to DEFAULTS.db.busyTimeoutMs when omitted', () => { + const NativeDB = getNative().NativeDatabase; + NativeDB.openReadWrite(dbPath).close(); + nativeDb = NativeDB.openReadonly(dbPath); + expect(readBusyTimeout(nativeDb)).toBe(DEFAULTS.db.busyTimeoutMs); + }); +});