diff --git a/src/cli/commands/embed.ts b/src/cli/commands/embed.ts index 0dd94c101..7ad46a10c 100644 --- a/src/cli/commands/embed.ts +++ b/src/cli/commands/embed.ts @@ -1,5 +1,5 @@ import path from 'node:path'; -import { openReadonlyOrFail } from '../../db/index.js'; +import { openReadonlyOrFail, resolveBusyTimeoutMs } from '../../db/index.js'; import { getEmbeddingMeta } from '../../db/repository/embeddings.js'; import { buildEmbeddings, @@ -13,7 +13,7 @@ import type { CommandDefinition } from '../types.js'; function resolveStickyModel(dbPath: string | undefined): string | null { try { - const db = openReadonlyOrFail(dbPath); + const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath)); try { const storedName = getEmbeddingMeta(db, 'model'); if (!storedName) return null; diff --git a/src/cli/shared/open-graph.ts b/src/cli/shared/open-graph.ts index b2ef5dfd6..48f702deb 100644 --- a/src/cli/shared/open-graph.ts +++ b/src/cli/shared/open-graph.ts @@ -1,4 +1,4 @@ -import { openReadonlyOrFail } from '../../db/index.js'; +import { openReadonlyOrFail, resolveBusyTimeoutMs } from '../../db/index.js'; import type { BetterSqlite3Database } from '../../types.js'; /** @@ -8,6 +8,6 @@ export function openGraph(opts: { db?: string } = {}): { db: BetterSqlite3Database; close: () => void; } { - const db = openReadonlyOrFail(opts.db); + const db = openReadonlyOrFail(opts.db, resolveBusyTimeoutMs(opts.db)); return { db, close: () => db.close() }; } diff --git a/src/db/connection.ts b/src/db/connection.ts index 8933c0628..666bfaff8 100644 --- a/src/db/connection.ts +++ b/src/db/connection.ts @@ -6,7 +6,7 @@ import { DEFAULTS, loadConfig } from '../infrastructure/config.js'; import { debug, warn } from '../infrastructure/logger.js'; import { getNative, isNativeAvailable } from '../infrastructure/native.js'; import { DbError, toErrorMessage } from '../shared/errors.js'; -import type { BetterSqlite3Database, NativeDatabase } from '../types.js'; +import type { BetterSqlite3Database, CodegraphConfig, NativeDatabase } from '../types.js'; import { getDatabase } from './better-sqlite3.js'; import { Repository } from './repository/base.js'; import { NativeRepository } from './repository/native-repository.js'; @@ -386,6 +386,18 @@ interface ResolvedDbSettings { busyTimeoutMs: number; } +/** + * Derive the project rootDir from a possibly-custom DB path, for loadConfig(). + * Using findDbPath (not path.resolve(customDbPath)) ensures directory inputs like + * --db /path/to/repo are normalised to .codegraph/graph.db before we strip two levels. + * Convention: resolvedDbPath = /.codegraph/graph.db + * Shared by resolveDbSettings() and resolveBusyTimeoutMs() so rootDir derivation can't drift. + */ +function deriveRootDirFromDbPath(customDbPath: string | undefined): string | undefined { + const resolvedDbPath = customDbPath ? findDbPath(customDbPath) : undefined; + return resolvedDbPath ? path.dirname(path.dirname(resolvedDbPath)) : undefined; +} + /** * Resolve the effective engine for DB access (explicit opts.engine > config.build.engine > * 'auto') alongside config.db.busyTimeoutMs, in a single loadConfig() call. @@ -400,12 +412,7 @@ function resolveDbSettings( customDbPath: string | undefined, engineOpt: 'native' | 'wasm' | 'auto' | undefined, ): ResolvedDbSettings { - // Using findDbPath (not path.resolve(customDbPath)) ensures directory inputs like - // --db /path/to/repo are normalised to .codegraph/graph.db before we strip two levels. - // Convention: resolvedDbPath = /.codegraph/graph.db - const resolvedDbPath = customDbPath ? findDbPath(customDbPath) : undefined; - const rootDir = resolvedDbPath ? path.dirname(path.dirname(resolvedDbPath)) : undefined; - const config = loadConfig(rootDir); + const config = loadConfig(deriveRootDirFromDbPath(customDbPath)); // config.build.engine is already populated from CODEGRAPH_ENGINE env by applyEnvOverrides, // so this covers both the env-var path and the .codegraphrc.json config-file path. return { @@ -414,6 +421,31 @@ function resolveDbSettings( }; } +/** + * Resolve the full config for a given DB path, deriving rootDir the same way + * resolveDbSettings()/resolveBusyTimeoutMs() do. Exported so callers that need + * both the busy-timeout and other config values (e.g. withReadonlyDb()) can + * share a single loadConfig() call instead of resolving it twice. + * + * MUST be called before opening any DB handle, for the same reason as + * resolveDbSettings(): loadConfig can throw (e.g. ConfigError via + * resolveSecrets on a malformed llm.apiKeyCommand config), and an + * already-open handle at that point would never be closed. + */ +export function resolveDbConfig(customDbPath?: string): CodegraphConfig { + return loadConfig(deriveRootDirFromDbPath(customDbPath)); +} + +/** + * Resolve config.db.busyTimeoutMs alone, for the ad-hoc read-only query call + * sites (features/*, domain/analysis/*, domain/search/*) that call + * openReadonlyOrFail() directly and don't need engine selection. Shares + * rootDir derivation with resolveDbSettings() so the two can't drift. + */ +export function resolveBusyTimeoutMs(customDbPath?: string): number { + return resolveDbConfig(customDbPath).db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs; +} + /** Open a NativeRepository via rusqlite, throwing DbError if the DB file is missing. */ function openRepoNative(customDbPath?: string): { repo: Repository; close(): void } { const dbPath = findDbPath(customDbPath); diff --git a/src/db/index.ts b/src/db/index.ts index f08a37677..4abb201ea 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -15,6 +15,8 @@ export { openReadonlyWithNative, openRepo, releaseAdvisoryLock, + resolveBusyTimeoutMs, + resolveDbConfig, } from './connection.js'; export { getBuildMeta, initSchema, MIGRATIONS, setBuildMeta } from './migrations.js'; export { diff --git a/src/domain/analysis/context.ts b/src/domain/analysis/context.ts index d5ef6b0c8..acd87ffa8 100644 --- a/src/domain/analysis/context.ts +++ b/src/domain/analysis/context.ts @@ -438,12 +438,15 @@ export function contextData( config?: any; } = {}, ) { - return withReadonlyDb(customDbPath, (db) => { + return withReadonlyDb(customDbPath, (db, config) => { const depth = opts.depth || 0; const noSource = opts.noSource || false; const includeTests = opts.includeTests || false; - const { noTests, displayOpts } = resolveAnalysisOpts(opts); + const { noTests, displayOpts } = resolveAnalysisOpts({ + ...opts, + config: opts.config ?? config, + }); const dbPath = findDbPath(customDbPath); const repoRoot = path.resolve(path.dirname(dbPath), '..'); @@ -510,11 +513,14 @@ export function explainData( config?: any; } = {}, ) { - return withReadonlyDb(customDbPath, (db) => { + return withReadonlyDb(customDbPath, (db, config) => { const depth = opts.depth || 0; const kind = isFileLikeTarget(target) ? 'file' : 'function'; - const { noTests, displayOpts } = resolveAnalysisOpts(opts); + const { noTests, displayOpts } = resolveAnalysisOpts({ + ...opts, + config: opts.config ?? config, + }); const dbPath = findDbPath(customDbPath); const repoRoot = path.resolve(path.dirname(dbPath), '..'); diff --git a/src/domain/analysis/diff-impact.ts b/src/domain/analysis/diff-impact.ts index e16622cef..f5e116bba 100644 --- a/src/domain/analysis/diff-impact.ts +++ b/src/domain/analysis/diff-impact.ts @@ -6,7 +6,7 @@ import { cachedStmt } from '../../db/repository/cached-stmt.js'; import { evaluateBoundaries } from '../../features/boundaries.js'; import { coChangeForFiles } from '../../features/cochange.js'; import { ownersForFiles } from '../../features/owners.js'; -import { loadConfig } from '../../infrastructure/config.js'; +import { DEFAULTS, loadConfig } from '../../infrastructure/config.js'; import { debug } from '../../infrastructure/logger.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; import { paginateResult } from '../../shared/paginate.js'; @@ -269,10 +269,17 @@ export function diffImpactData( config?: any; } = {}, ) { - const db = openReadonlyOrFail(customDbPath); + // Resolve config before opening the DB so config.db.busyTimeoutMs can be + // threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s + // ordering in db/connection.ts — loadConfig can throw, and an already-open + // handle at that point would never be closed). + const config = opts.config || loadConfig(); + const db = openReadonlyOrFail( + customDbPath, + config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, + ); try { const noTests = opts.noTests || false; - const config = opts.config || loadConfig(); const maxDepth = opts.depth || config.analysis?.impactDepth || 3; const dbPath = findDbPath(customDbPath); diff --git a/src/domain/analysis/exports.ts b/src/domain/analysis/exports.ts index ff32eb212..a8ed5f9f3 100644 --- a/src/domain/analysis/exports.ts +++ b/src/domain/analysis/exports.ts @@ -32,8 +32,11 @@ export function exportsData( config?: any; } = {}, ) { - return withReadonlyDb(customDbPath, (db) => { - const { noTests, displayOpts } = resolveAnalysisOpts(opts); + return withReadonlyDb(customDbPath, (db, config) => { + const { noTests, displayOpts } = resolveAnalysisOpts({ + ...opts, + config: opts.config ?? config, + }); const dbFilePath = findDbPath(customDbPath); const repoRoot = path.resolve(path.dirname(dbFilePath), '..'); diff --git a/src/domain/analysis/module-map.ts b/src/domain/analysis/module-map.ts index d1194bd1c..1d46b658c 100644 --- a/src/domain/analysis/module-map.ts +++ b/src/domain/analysis/module-map.ts @@ -1,5 +1,10 @@ import path from 'node:path'; -import { openReadonlyOrFail, openReadonlyWithNative, testFilterSQL } from '../../db/index.js'; +import { + openReadonlyOrFail, + openReadonlyWithNative, + resolveBusyTimeoutMs, + testFilterSQL, +} from '../../db/index.js'; import { loadConfig } from '../../infrastructure/config.js'; import { debug } from '../../infrastructure/logger.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; @@ -307,7 +312,7 @@ function getComplexitySummary(db: BetterSqlite3Database, testFilter: string) { // --------------------------------------------------------------------------- export function moduleMapData(customDbPath: string, limit = 20, opts: { noTests?: boolean } = {}) { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; diff --git a/src/domain/analysis/query-helpers.ts b/src/domain/analysis/query-helpers.ts index 293db0a5b..6635cd953 100644 --- a/src/domain/analysis/query-helpers.ts +++ b/src/domain/analysis/query-helpers.ts @@ -1,19 +1,25 @@ -import { openReadonlyOrFail, openRepo, type Repository } from '../../db/index.js'; -import { loadConfig } from '../../infrastructure/config.js'; +import { openReadonlyOrFail, openRepo, type Repository, resolveDbConfig } from '../../db/index.js'; +import { DEFAULTS, loadConfig } from '../../infrastructure/config.js'; import type { BetterSqlite3Database, CodegraphConfig } from '../../types.js'; /** * Open a readonly DB connection, run `fn`, and close the DB on completion. * Eliminates the duplicated `openReadonlyOrFail` + `try/finally/db.close()` pattern * that appears in every analysis query function. + * + * Resolves the config once and passes it to `fn` so callers that also need + * `resolveAnalysisOpts` can reuse it as `opts.config` instead of triggering a + * second `loadConfig()` for the same rootDir (#1763 review). */ export function withReadonlyDb( customDbPath: string | undefined, - fn: (db: BetterSqlite3Database) => T, + fn: (db: BetterSqlite3Database, config: CodegraphConfig) => T, ): T { - const db = openReadonlyOrFail(customDbPath); + const config = resolveDbConfig(customDbPath); + const busyTimeoutMs = config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs; + const db = openReadonlyOrFail(customDbPath, busyTimeoutMs); try { - return fn(db); + return fn(db, config); } finally { db.close(); } diff --git a/src/domain/analysis/roles.ts b/src/domain/analysis/roles.ts index 15ca183d1..602af92b3 100644 --- a/src/domain/analysis/roles.ts +++ b/src/domain/analysis/roles.ts @@ -1,4 +1,4 @@ -import { openReadonlyOrFail } from '../../db/index.js'; +import { openReadonlyOrFail, resolveBusyTimeoutMs } from '../../db/index.js'; import { buildFileConditionSQL } from '../../db/query-builder.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; import { DEAD_ROLE_PREFIX } from '../../shared/kinds.js'; @@ -13,7 +13,7 @@ export interface DynamicCallCount { /** Return a count of flagged dynamic call sink edges, grouped by kind. */ export function dynamicCallsData(customDbPath: string): DynamicCallCount[] { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { return db .prepare( @@ -39,7 +39,7 @@ export function rolesData( offset?: number; } = {}, ) { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; const filterRole = opts.role || null; diff --git a/src/domain/analysis/symbol-lookup.ts b/src/domain/analysis/symbol-lookup.ts index 90a39934e..fc281e02f 100644 --- a/src/domain/analysis/symbol-lookup.ts +++ b/src/domain/analysis/symbol-lookup.ts @@ -13,6 +13,7 @@ import { listFunctionNodes, openReadonlyOrFail, Repository, + resolveBusyTimeoutMs, } from '../../db/index.js'; import { debug } from '../../infrastructure/logger.js'; import { isTestFile } from '../../infrastructure/test-filter.js'; @@ -95,7 +96,7 @@ export function queryNameData( customDbPath: string, opts: { noTests?: boolean; limit?: number; offset?: number } = {}, ) { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; let nodes = db.prepare(`SELECT * FROM nodes WHERE name LIKE ?`).all(`%${name}%`) as NodeRow[]; @@ -195,7 +196,7 @@ export function whereData( customDbPath: string, opts: { noTests?: boolean; file?: boolean; limit?: number; offset?: number } = {}, ) { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; const fileMode = opts.file || false; @@ -219,7 +220,7 @@ export function listFunctionsData( offset?: number; } = {}, ) { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; @@ -241,7 +242,7 @@ export function childrenData( customDbPath: string, opts: { noTests?: boolean; file?: string; kind?: string; limit?: number; offset?: number } = {}, ) { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; diff --git a/src/domain/search/search/hybrid.ts b/src/domain/search/search/hybrid.ts index bf6406c6c..0a2420b14 100644 --- a/src/domain/search/search/hybrid.ts +++ b/src/domain/search/search/hybrid.ts @@ -1,5 +1,5 @@ import { openReadonlyOrFail } from '../../../db/index.js'; -import { loadConfig } from '../../../infrastructure/config.js'; +import { DEFAULTS, loadConfig } from '../../../infrastructure/config.js'; import type { BetterSqlite3Database, CodegraphConfig } from '../../../types.js'; import { hasFtsIndex } from '../stores/fts5.js'; import { ftsSearchData } from './keyword.js'; @@ -184,7 +184,10 @@ export async function hybridSearchData( const k = opts.rrfK ?? searchCfg.rrfK ?? 60; const topK = (opts.limit ?? searchCfg.topK ?? 15) * 5; - const checkDb = openReadonlyOrFail(customDbPath) as BetterSqlite3Database; + const checkDb = openReadonlyOrFail( + customDbPath, + config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, + ) as BetterSqlite3Database; const ftsAvailable = hasFtsIndex(checkDb); checkDb.close(); if (!ftsAvailable) return null; diff --git a/src/domain/search/search/keyword.ts b/src/domain/search/search/keyword.ts index 66eda0acf..f9d5fe139 100644 --- a/src/domain/search/search/keyword.ts +++ b/src/domain/search/search/keyword.ts @@ -1,4 +1,4 @@ -import { openReadonlyOrFail } from '../../../db/index.js'; +import { openReadonlyOrFail, resolveBusyTimeoutMs } from '../../../db/index.js'; import { buildFileConditionSQL } from '../../../db/query-builder.js'; import type { BetterSqlite3Database } from '../../../types.js'; import { normalizeSymbol } from '../../queries.js'; @@ -41,7 +41,10 @@ export function ftsSearchData( ): FtsSearchResult | null { const limit = opts.limit || 15; - const db = openReadonlyOrFail(customDbPath) as BetterSqlite3Database; + const db = openReadonlyOrFail( + customDbPath, + resolveBusyTimeoutMs(customDbPath), + ) as BetterSqlite3Database; try { if (!hasFtsIndex(db)) { diff --git a/src/domain/search/search/prepare.ts b/src/domain/search/search/prepare.ts index 13cea6cb4..63bf90071 100644 --- a/src/domain/search/search/prepare.ts +++ b/src/domain/search/search/prepare.ts @@ -1,4 +1,4 @@ -import { openReadonlyOrFail } from '../../../db/index.js'; +import { openReadonlyOrFail, resolveBusyTimeoutMs } from '../../../db/index.js'; import { buildFileConditionSQL } from '../../../db/query-builder.js'; import { getEmbeddingCount, getEmbeddingMeta } from '../../../db/repository/embeddings.js'; import { info } from '../../../infrastructure/logger.js'; @@ -43,7 +43,10 @@ export function prepareSearch( customDbPath: string | undefined, opts: PrepareSearchOpts = {}, ): PreparedSearch | null { - const db = openReadonlyOrFail(customDbPath) as BetterSqlite3Database; + const db = openReadonlyOrFail( + customDbPath, + resolveBusyTimeoutMs(customDbPath), + ) as BetterSqlite3Database; try { const count = getEmbeddingCount(db); diff --git a/src/features/ast.ts b/src/features/ast.ts index d227b4bcc..f90f0fbcb 100644 --- a/src/features/ast.ts +++ b/src/features/ast.ts @@ -8,7 +8,7 @@ import { import { buildExtensionSet } from '../ast-analysis/shared.js'; import { walkWithVisitors } from '../ast-analysis/visitor.js'; import { createAstStoreVisitor } from '../ast-analysis/visitors/ast-store-visitor.js'; -import { bulkNodeIdsByFile, openReadonlyOrFail } from '../db/index.js'; +import { bulkNodeIdsByFile, openReadonlyOrFail, resolveBusyTimeoutMs } from '../db/index.js'; import { buildFileConditionSQL } from '../db/query-builder.js'; import { debug } from '../infrastructure/logger.js'; import { outputResult } from '../infrastructure/result-formatter.js'; @@ -304,7 +304,7 @@ export function astQueryData( limit: number; }; } { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); const { kind, file, noTests, limit, offset } = opts; let where = 'WHERE 1=1'; diff --git a/src/features/audit.ts b/src/features/audit.ts index f5dc5bddf..64a55b462 100644 --- a/src/features/audit.ts +++ b/src/features/audit.ts @@ -3,7 +3,7 @@ import { openReadonlyOrFail } from '../db/index.js'; import { normalizeFileFilter } from '../db/query-builder.js'; import { bfsTransitiveCallers } from '../domain/analysis/impact.js'; import { explainData } from '../domain/queries.js'; -import { loadConfig } from '../infrastructure/config.js'; +import { DEFAULTS, loadConfig } from '../infrastructure/config.js'; import { debug } from '../infrastructure/logger.js'; import { isTestFile } from '../infrastructure/test-filter.js'; import { toErrorMessage } from '../shared/errors.js'; @@ -170,7 +170,10 @@ export function auditData( } // 2. Open DB for enrichment - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail( + customDbPath, + config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, + ); const thresholds = resolveThresholds(customDbPath, opts.config); let functions: AuditFunctionEntry[]; diff --git a/src/features/cfg.ts b/src/features/cfg.ts index a1a036166..48f1a7c42 100644 --- a/src/features/cfg.ts +++ b/src/features/cfg.ts @@ -15,6 +15,7 @@ import { getFunctionNodeId, hasCfgTables, openReadonlyOrFail, + resolveBusyTimeoutMs, } from '../db/index.js'; import { debug, info } from '../infrastructure/logger.js'; import { paginateResult } from '../shared/paginate.js'; @@ -609,7 +610,7 @@ export function cfgData( customDbPath: string | undefined, opts: CfgOpts = {}, ): CfgDataResult { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; diff --git a/src/features/check.ts b/src/features/check.ts index ca12c29c6..a7f638f17 100644 --- a/src/features/check.ts +++ b/src/features/check.ts @@ -4,7 +4,7 @@ import path from 'node:path'; import { findDbPath, openReadonlyOrFail } from '../db/index.js'; import { bfsTransitiveCallers } from '../domain/analysis/impact.js'; import { findCycles } from '../domain/graph/cycles.js'; -import { loadConfig } from '../infrastructure/config.js'; +import { DEFAULTS, loadConfig } from '../infrastructure/config.js'; import { isTestFile } from '../infrastructure/test-filter.js'; import type { BetterSqlite3Database, CodegraphConfig } from '../types.js'; import { matchOwners, parseCodeowners } from './owners.js'; @@ -718,15 +718,23 @@ function makeEmptyCheck(): CheckResult { } export function checkData(customDbPath: string | undefined, opts: CheckOpts = {}): CheckResult { - const db = openReadonlyOrFail(customDbPath); + // Resolve repoRoot + config before opening the DB so config.db.busyTimeoutMs + // can be threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s + // ordering in db/connection.ts — loadConfig can throw, and an already-open + // handle at that point would never be closed). repoRoot only depends on + // findDbPath(), not on the DB actually existing, so this reorder is safe. + const dbPath = findDbPath(customDbPath); + const repoRoot = path.resolve(path.dirname(dbPath), '..'); + const config = opts.config || loadConfig(repoRoot); + const db = openReadonlyOrFail( + customDbPath, + config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, + ); try { - const dbPath = findDbPath(customDbPath); - const repoRoot = path.resolve(path.dirname(dbPath), '..'); const noTests = opts.noTests || false; const maxDepth = opts.depth || 3; - const config = opts.config || loadConfig(repoRoot); const flags = resolveCheckFlags(opts, config); const gitRoot = findGitRoot(repoRoot); diff --git a/src/features/cochange.ts b/src/features/cochange.ts index 80ee0194d..f6b990cff 100644 --- a/src/features/cochange.ts +++ b/src/features/cochange.ts @@ -8,7 +8,14 @@ import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; -import { closeDb, findDbPath, initSchema, openDb, openReadonlyOrFail } from '../db/index.js'; +import { + closeDb, + findDbPath, + initSchema, + openDb, + openReadonlyOrFail, + resolveBusyTimeoutMs, +} from '../db/index.js'; import { DEFAULTS } from '../infrastructure/config.js'; import { debug, warn } from '../infrastructure/logger.js'; import { isTestFile } from '../infrastructure/test-filter.js'; @@ -407,7 +414,7 @@ export function coChangeData( customDbPath?: string, opts: { limit?: number; minJaccard?: number; noTests?: boolean; offset?: number } = {}, ): Record { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); const limit = opts.limit || 20; const minJaccard = opts.minJaccard ?? DEFAULTS.coChange.minJaccard; const noTests = opts.noTests || false; @@ -479,7 +486,7 @@ export function coChangeTopData( customDbPath?: string, opts: { limit?: number; minJaccard?: number; noTests?: boolean; offset?: number } = {}, ): Record { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); const limit = opts.limit || 20; const minJaccard = opts.minJaccard ?? DEFAULTS.coChange.minJaccard; const noTests = opts.noTests || false; diff --git a/src/features/complexity-query.ts b/src/features/complexity-query.ts index d1ba86a3a..135721b11 100644 --- a/src/features/complexity-query.ts +++ b/src/features/complexity-query.ts @@ -285,6 +285,7 @@ function resolveComplexityQueryOptions(opts: ComplexityQueryOpts): { noTests: boolean; aboveThreshold: boolean; thresholds: any; + busyTimeoutMs: number; } { const config = opts.config || loadConfig(process.cwd()); return { @@ -292,6 +293,7 @@ function resolveComplexityQueryOptions(opts: ComplexityQueryOpts): { noTests: opts.noTests || false, aboveThreshold: opts.aboveThreshold || false, thresholds: config.manifesto?.rules || DEFAULTS.manifesto.rules, + busyTimeoutMs: config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, }; } @@ -320,10 +322,13 @@ export function complexityData( customDbPath?: string, opts: ComplexityQueryOpts = {}, ): Record { - const db = openReadonlyOrFail(customDbPath); + // Resolve config (and thus busyTimeoutMs) before opening the DB — mirrors + // resolveDbSettings()'s ordering in db/connection.ts, since loadConfig can + // throw and an already-open handle at that point would never be closed. + const { sort, noTests, aboveThreshold, thresholds, busyTimeoutMs } = + resolveComplexityQueryOptions(opts); + const db = openReadonlyOrFail(customDbPath, busyTimeoutMs); try { - const { sort, noTests, aboveThreshold, thresholds } = resolveComplexityQueryOptions(opts); - const { where, params } = buildComplexityWhere({ noTests, target: opts.target || null, diff --git a/src/features/dataflow.ts b/src/features/dataflow.ts index 4f231492b..931a8370d 100644 --- a/src/features/dataflow.ts +++ b/src/features/dataflow.ts @@ -18,7 +18,12 @@ import { } from '../ast-analysis/shared.js'; import { walkWithVisitors } from '../ast-analysis/visitor.js'; import { createDataflowVisitor } from '../ast-analysis/visitors/dataflow-visitor.js'; -import { hasDataflowTable, openReadonlyOrFail, openReadonlyWithNative } from '../db/index.js'; +import { + hasDataflowTable, + openReadonlyOrFail, + openReadonlyWithNative, + resolveBusyTimeoutMs, +} from '../db/index.js'; import { ALL_SYMBOL_KINDS, normalizeSymbol } from '../domain/queries.js'; import { debug, info } from '../infrastructure/logger.js'; import { isTestFile } from '../infrastructure/test-filter.js'; @@ -1531,7 +1536,7 @@ export function dataflowPathData( offset?: number; } = {}, ): Record { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const noTests = opts.noTests || false; const maxDepth = opts.maxDepth || 10; @@ -1632,7 +1637,7 @@ export function dataflowImpactData( offset?: number; } = {}, ): Record { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const maxDepth = opts.depth || 5; const noTests = opts.noTests || false; diff --git a/src/features/flow.ts b/src/features/flow.ts index e2a4f1f21..910787658 100644 --- a/src/features/flow.ts +++ b/src/features/flow.ts @@ -5,7 +5,7 @@ * framework entry points (routes, commands, events) through their call chains. */ -import { openReadonlyOrFail } from '../db/index.js'; +import { openReadonlyOrFail, resolveBusyTimeoutMs } from '../db/index.js'; import { CORE_SYMBOL_KINDS, findMatchingNodes } from '../domain/queries.js'; import { isTestFile } from '../infrastructure/test-filter.js'; import { toSymbolRef } from '../shared/normalize.js'; @@ -33,7 +33,7 @@ export function listEntryPointsData( dbPath?: string, opts: { noTests?: boolean; limit?: number; offset?: number } = {}, ): Record { - const db = openReadonlyOrFail(dbPath); + const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath)); try { const noTests = opts.noTests || false; @@ -255,7 +255,7 @@ export function flowData( offset?: number; } = {}, ): Record { - const db = openReadonlyOrFail(dbPath); + const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath)); try { const maxDepth = opts.depth || 10; const noTests = opts.noTests || false; diff --git a/src/features/manifesto.ts b/src/features/manifesto.ts index 68c22126c..bf6c97a2d 100644 --- a/src/features/manifesto.ts +++ b/src/features/manifesto.ts @@ -1,7 +1,7 @@ import { openReadonlyOrFail } from '../db/index.js'; import { buildFileConditionSQL } from '../db/query-builder.js'; import { findCycles } from '../domain/graph/cycles.js'; -import { loadConfig } from '../infrastructure/config.js'; +import { DEFAULTS, loadConfig } from '../infrastructure/config.js'; import { debug } from '../infrastructure/logger.js'; import { paginateResult } from '../shared/paginate.js'; import type { BetterSqlite3Database, CodegraphConfig, ThresholdRule } from '../types.js'; @@ -469,10 +469,17 @@ export function manifestoData( customDbPath?: string, opts: ManifestoOpts = {}, ): Record { - const db = openReadonlyOrFail(customDbPath); + // Resolve config before opening the DB so config.db.busyTimeoutMs can be + // threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s + // ordering in db/connection.ts — loadConfig can throw, and an already-open + // handle at that point would never be closed). + const config = opts.config || loadConfig(process.cwd()); + const db = openReadonlyOrFail( + customDbPath, + config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, + ); try { - const config = opts.config || loadConfig(process.cwd()); const rules = resolveRules( config.manifesto?.rules as unknown as Record>, ); diff --git a/src/features/owners.ts b/src/features/owners.ts index 93d1259f6..68e5084f5 100644 --- a/src/features/owners.ts +++ b/src/features/owners.ts @@ -1,6 +1,6 @@ import fs from 'node:fs'; import path from 'node:path'; -import { findDbPath, openReadonlyOrFail } from '../db/index.js'; +import { findDbPath, openReadonlyOrFail, resolveBusyTimeoutMs } from '../db/index.js'; import { normalizeFileFilter } from '../db/query-builder.js'; import { isTestFile } from '../infrastructure/test-filter.js'; @@ -320,7 +320,7 @@ function buildOwnersSummary( } export function ownersData(customDbPath?: string, opts: OwnersDataOpts = {}): OwnersDataResult { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const dbPath = findDbPath(customDbPath); const repoRoot = path.resolve(path.dirname(dbPath), '..'); diff --git a/src/features/structure-query.ts b/src/features/structure-query.ts index 033a00c99..80ea92481 100644 --- a/src/features/structure-query.ts +++ b/src/features/structure-query.ts @@ -7,8 +7,13 @@ * role classification). */ -import { openReadonlyOrFail, openReadonlyWithNative, testFilterSQL } from '../db/index.js'; -import { loadConfig } from '../infrastructure/config.js'; +import { + openReadonlyOrFail, + openReadonlyWithNative, + resolveBusyTimeoutMs, + testFilterSQL, +} from '../db/index.js'; +import { DEFAULTS, loadConfig } from '../infrastructure/config.js'; import { isTestFile } from '../infrastructure/test-filter.js'; import { normalizePath } from '../shared/constants.js'; import { paginateResult } from '../shared/paginate.js'; @@ -138,7 +143,7 @@ export function structureData( suppressed?: number; warning?: string; } { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { const rawDir = opts.directory || null; const filterDir = rawDir && normalizePath(rawDir) !== '.' ? rawDir : null; @@ -376,9 +381,16 @@ export function moduleBoundariesData( }[]; count: number; } { - const db = openReadonlyOrFail(customDbPath); + // Resolve config before opening the DB so config.db.busyTimeoutMs can be + // threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s + // ordering in db/connection.ts — loadConfig can throw, and an already-open + // handle at that point would never be closed). + const config = opts.config || loadConfig(); + const db = openReadonlyOrFail( + customDbPath, + config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs, + ); try { - const config = opts.config || loadConfig(); const threshold = opts.threshold ?? (config as unknown as { structure?: { cohesionThreshold?: number } }).structure diff --git a/src/shared/generators.ts b/src/shared/generators.ts index acfbfc91e..fe8839535 100644 --- a/src/shared/generators.ts +++ b/src/shared/generators.ts @@ -1,4 +1,4 @@ -import { iterateFunctionNodes, openReadonlyOrFail } from '../db/index.js'; +import { iterateFunctionNodes, openReadonlyOrFail, resolveBusyTimeoutMs } from '../db/index.js'; import { buildFileConditionSQL } from '../db/query-builder.js'; import { isTestFile } from '../infrastructure/test-filter.js'; import type { BetterSqlite3Database, NodeRow } from '../types.js'; @@ -26,7 +26,10 @@ export function* iterListFunctions( customDbPath?: string, opts: IterListOpts = {}, ): Generator { - const db = openReadonlyOrFail(customDbPath) as BetterSqlite3Database; + const db = openReadonlyOrFail( + customDbPath, + resolveBusyTimeoutMs(customDbPath), + ) as BetterSqlite3Database; try { const noTests = opts.noTests || false; @@ -68,7 +71,10 @@ interface IterRolesOpts { * Generator: stream role-classified symbols one-by-one. */ export function* iterRoles(customDbPath?: string, opts: IterRolesOpts = {}): Generator { - const db = openReadonlyOrFail(customDbPath) as BetterSqlite3Database; + const db = openReadonlyOrFail( + customDbPath, + resolveBusyTimeoutMs(customDbPath), + ) as BetterSqlite3Database; try { const noTests = opts.noTests || false; const conditions = ['role IS NOT NULL']; @@ -133,7 +139,10 @@ export function* iterWhere( customDbPath?: string, opts: IterWhereOpts = {}, ): Generator { - const db = openReadonlyOrFail(customDbPath) as BetterSqlite3Database; + const db = openReadonlyOrFail( + customDbPath, + resolveBusyTimeoutMs(customDbPath), + ) as BetterSqlite3Database; try { const noTests = opts.noTests || false; const placeholders = ALL_SYMBOL_KINDS.map(() => '?').join(', '); diff --git a/tests/unit/busy-timeout-query-sites.test.ts b/tests/unit/busy-timeout-query-sites.test.ts new file mode 100644 index 000000000..159ae6e1e --- /dev/null +++ b/tests/unit/busy-timeout-query-sites.test.ts @@ -0,0 +1,143 @@ +/** + * Regression tests for issue #1763: the ad-hoc read-only query call sites in + * features/*, domain/analysis/*, and domain/search/* must thread the + * user-configured `db.busyTimeoutMs` through to their `openReadonlyOrFail()` + * call, instead of silently falling back to the DEFAULTS.db.busyTimeoutMs + * hardcoded default. + * + * Covers the shared `resolveBusyTimeoutMs()` helper directly, plus a + * representative sample of call sites from each category identified while + * fixing the issue: + * - category (c), never loaded config before this fix: ownersData, cfgData + * - category (b), loaded config AFTER opening the db (now reordered): manifestoData + * - category (a), already loaded config before opening the db: hybridSearchData + * - the shared withReadonlyDb() wrapper (also covers exports/dependencies/context) + */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { closeDb, initSchema, openDb, resolveBusyTimeoutMs } from '../../src/db/index.js'; +import { withReadonlyDb } from '../../src/domain/analysis/query-helpers.js'; +import { hybridSearchData } from '../../src/domain/search/search/hybrid.js'; +import { cfgData } from '../../src/features/cfg.js'; +import { manifestoData } from '../../src/features/manifesto.js'; +import { ownersData } from '../../src/features/owners.js'; +import { DEFAULTS } from '../../src/infrastructure/config.js'; + +const CUSTOM_BUSY_TIMEOUT_MS = 42424; + +let tmpDir: string; +let dbPath: string; + +/** Capture every `busy_timeout = N` pragma issued against a real Database instance. */ +function captureBusyTimeoutPragmas(): { calls: string[]; restore: () => void } { + const original = Database.prototype.pragma; + const calls: string[] = []; + const spy = vi.spyOn(Database.prototype, 'pragma').mockImplementation(function ( + this: unknown, + sql: string, + ...rest: unknown[] + ) { + if (typeof sql === 'string' && sql.startsWith('busy_timeout')) calls.push(sql); + return original.apply(this, [sql, ...rest] as Parameters); + }); + return { calls, restore: () => spy.mockRestore() }; +} + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-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('resolveBusyTimeoutMs', () => { + it('reads db.busyTimeoutMs from the project config nearest the resolved DB path', () => { + expect(resolveBusyTimeoutMs(dbPath)).toBe(CUSTOM_BUSY_TIMEOUT_MS); + }); + + it('falls back to DEFAULTS.db.busyTimeoutMs when no project config sets it', () => { + const bareDir = fs.mkdtempSync(path.join(tmpDir, 'bare-')); + const bareDbPath = path.join(bareDir, '.codegraph', 'graph.db'); + const db = openDb(bareDbPath); + initSchema(db); + closeDb(db); + + expect(resolveBusyTimeoutMs(bareDbPath)).toBe(DEFAULTS.db.busyTimeoutMs); + expect(resolveBusyTimeoutMs(bareDbPath)).not.toBe(CUSTOM_BUSY_TIMEOUT_MS); + }); +}); + +describe('configured busyTimeoutMs reaches PRAGMA busy_timeout at ad-hoc read-only call sites', () => { + it('ownersData (never loaded config before this fix) applies the configured busy_timeout', () => { + const capture = captureBusyTimeoutPragmas(); + try { + ownersData(dbPath); + } finally { + capture.restore(); + } + expect(capture.calls).toContain(`busy_timeout = ${CUSTOM_BUSY_TIMEOUT_MS}`); + }); + + it('cfgData (never loaded config before this fix) applies the configured busy_timeout', () => { + const capture = captureBusyTimeoutPragmas(); + try { + cfgData('nonexistent-symbol', dbPath); + } finally { + capture.restore(); + } + expect(capture.calls).toContain(`busy_timeout = ${CUSTOM_BUSY_TIMEOUT_MS}`); + }); + + it('manifestoData (config load reordered to before the db opens) applies the configured busy_timeout', () => { + // manifestoData resolves its config via loadConfig(process.cwd()) — a + // pre-existing, cwd-based resolution this fix intentionally preserves + // (only the *timing* of the call moved, not what it resolves) — so the + // project config must be visible from process.cwd() here, not from dbPath. + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpDir); + const capture = captureBusyTimeoutPragmas(); + try { + manifestoData(dbPath); + } finally { + capture.restore(); + cwdSpy.mockRestore(); + } + expect(capture.calls).toContain(`busy_timeout = ${CUSTOM_BUSY_TIMEOUT_MS}`); + }); + + it('hybridSearchData (already loaded config before the db opens) applies the configured busy_timeout', async () => { + // Same pre-existing cwd-based config resolution as manifestoData (bare + // loadConfig() defaults to process.cwd()) — preserved, not introduced, by + // this fix. + const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpDir); + const capture = captureBusyTimeoutPragmas(); + try { + await hybridSearchData('nonexistent-query', dbPath); + } finally { + capture.restore(); + cwdSpy.mockRestore(); + } + expect(capture.calls).toContain(`busy_timeout = ${CUSTOM_BUSY_TIMEOUT_MS}`); + }); + + it('withReadonlyDb (shared helper backing exports/dependencies/context) applies the configured busy_timeout', () => { + const capture = captureBusyTimeoutPragmas(); + try { + withReadonlyDb(dbPath, (db) => db.prepare('SELECT 1').get()); + } finally { + capture.restore(); + } + expect(capture.calls).toContain(`busy_timeout = ${CUSTOM_BUSY_TIMEOUT_MS}`); + }); +});