From cd02d273044d92a64988d44957173c01f061d92f Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Mon, 6 Jul 2026 06:44:14 -0600 Subject: [PATCH 1/2] fix: thread configured busyTimeoutMs through remaining read-only query call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the config-driven db.busyTimeoutMs wiring (started in #1748/#1749 for openDb()/openReadonlyOrFail() and the call sites that already held a loaded config) to the ~30 ad-hoc read-only query call sites across features/*, domain/analysis/*, and domain/search/* that previously called openReadonlyOrFail() directly and silently fell back to DEFAULTS.db.busyTimeoutMs. Adds resolveBusyTimeoutMs() to src/db/connection.ts (exported via db/index.ts), sharing rootDir derivation with resolveDbSettings() via a new deriveRootDirFromDbPath() helper. For call sites that never loaded config in their path, this new call is threaded in directly. Fixing withReadonlyDb() in domain/analysis/query-helpers.ts centrally also covers its callers (exports.ts, dependencies.ts, context.ts) for free. For call sites that already loaded config but did so AFTER opening the DB (diffImpactData, checkData, complexityData, manifestoData, moduleBoundariesData), reordered config-load-before-db-open, mirroring the precedent already established by resolveDbSettings()/openReadonlyWithNative() (see the phase-15 gauntlet handle-leak fix). Verified this reorder is safe: loadConfig() only throws ConfigError for one narrow case (a non-string llm.apiKeyCommand), unrelated to db.busyTimeoutMs and independent of whether the DB exists — no existing test pins the previous error ordering for any of these call sites. Made every new config.db.busyTimeoutMs access optional-chained (config.db?.busyTimeoutMs) after this surfaced a real crash in tests/integration/complexity.test.ts, which mocks loadConfig() to return a partial object without a db key. Verified end-to-end against a real built graph with a custom .codegraphrc.json (db.busyTimeoutMs override) via the CLI (owners, check, structure --modules, map, audit, complexity, dataflow, search, co-change, ast, flow, cfg, roles, where, children), confirming the configured value reaches the PRAGMA busy_timeout call and the default still applies with no config override. Added tests/unit/busy-timeout-query-sites.test.ts covering resolveBusyTimeoutMs directly plus a representative sample of call sites from each category (ownersData, cfgData, manifestoData, hybridSearchData, withReadonlyDb), spying on Database.prototype.pragma to assert the configured value is actually applied. Filed two follow-ups discovered while completing this wiring pass, both intentionally out of scope here: - #1881: several functions (manifestoData, hybridSearchData, moduleBoundariesData, diffImpactData, complexityData, auditData) resolve their config via process.cwd() rather than the resolved --db path, unlike resolveDbSettings()/checkData() — a pre-existing inconsistency, not introduced by this change, that affects cross-repo --db usage for several config fields (not just busyTimeoutMs). - #1882: the Rust native DB layer (NativeDatabase::open_readonly/ open_read_write in crates/codegraph-core/src/db/connection.rs) still hardcodes PRAGMA busy_timeout = 5000. Investigated and confirmed this does NOT require giving Rust config-loading capability (the JS side already resolves the value) — it requires threading an already-resolved number across the napi FFI boundary plus a native rebuild across platforms, which is a heavier-weight change than this wiring pass. Refs #1763 (TS read-only call-site wiring is complete; the Rust portion of that issue's scope is deferred to #1882, so this does not close it). docs check acknowledged: internal config wiring only, no new config keys or CLI flags — db.busyTimeoutMs was already documented by the original phase-7 commit that introduced it. Impact: 36 functions changed, 131 affected --- src/cli/commands/embed.ts | 4 +- src/cli/shared/open-graph.ts | 4 +- src/db/connection.ts | 35 ++++- src/db/index.ts | 1 + src/domain/analysis/diff-impact.ts | 13 +- src/domain/analysis/module-map.ts | 9 +- src/domain/analysis/query-helpers.ts | 9 +- src/domain/analysis/roles.ts | 6 +- src/domain/analysis/symbol-lookup.ts | 9 +- src/domain/search/search/hybrid.ts | 7 +- src/domain/search/search/keyword.ts | 7 +- src/domain/search/search/prepare.ts | 7 +- src/features/ast.ts | 4 +- src/features/audit.ts | 7 +- src/features/cfg.ts | 3 +- src/features/check.ts | 18 ++- src/features/cochange.ts | 13 +- src/features/complexity-query.ts | 11 +- src/features/dataflow.ts | 11 +- src/features/flow.ts | 6 +- src/features/manifesto.ts | 13 +- src/features/owners.ts | 4 +- src/features/structure-query.ts | 22 ++- src/shared/generators.ts | 17 ++- tests/unit/busy-timeout-query-sites.test.ts | 143 ++++++++++++++++++++ 25 files changed, 317 insertions(+), 66 deletions(-) create mode 100644 tests/unit/busy-timeout-query-sites.test.ts 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..7efadbfe4 100644 --- a/src/db/connection.ts +++ b/src/db/connection.ts @@ -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,22 @@ function resolveDbSettings( }; } +/** + * 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. + * + * 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 resolveBusyTimeoutMs(customDbPath?: string): number { + const config = loadConfig(deriveRootDirFromDbPath(customDbPath)); + return config.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..517bd6493 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -15,6 +15,7 @@ export { openReadonlyWithNative, openRepo, releaseAdvisoryLock, + resolveBusyTimeoutMs, } from './connection.js'; export { getBuildMeta, initSchema, MIGRATIONS, setBuildMeta } from './migrations.js'; export { 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/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..20c0cd3b0 100644 --- a/src/domain/analysis/query-helpers.ts +++ b/src/domain/analysis/query-helpers.ts @@ -1,4 +1,9 @@ -import { openReadonlyOrFail, openRepo, type Repository } from '../../db/index.js'; +import { + openReadonlyOrFail, + openRepo, + type Repository, + resolveBusyTimeoutMs, +} from '../../db/index.js'; import { loadConfig } from '../../infrastructure/config.js'; import type { BetterSqlite3Database, CodegraphConfig } from '../../types.js'; @@ -11,7 +16,7 @@ export function withReadonlyDb( customDbPath: string | undefined, fn: (db: BetterSqlite3Database) => T, ): T { - const db = openReadonlyOrFail(customDbPath); + const db = openReadonlyOrFail(customDbPath, resolveBusyTimeoutMs(customDbPath)); try { return fn(db); } finally { 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}`); + }); +}); From 2afe183559504f94f8bea179c4f203d8f6844ee1 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Mon, 6 Jul 2026 19:15:18 -0600 Subject: [PATCH 2/2] fix: resolve config once in withReadonlyDb to avoid double loadConfig (Greptile) --- src/db/connection.ts | 23 ++++++++++++++++------- src/db/index.ts | 1 + src/domain/analysis/context.ts | 14 ++++++++++---- src/domain/analysis/exports.ts | 7 +++++-- src/domain/analysis/query-helpers.ts | 21 +++++++++++---------- 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/src/db/connection.ts b/src/db/connection.ts index 7efadbfe4..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'; @@ -422,19 +422,28 @@ function resolveDbSettings( } /** - * 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. + * 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 { - const config = loadConfig(deriveRootDirFromDbPath(customDbPath)); - return config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs; + return resolveDbConfig(customDbPath).db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs; } /** Open a NativeRepository via rusqlite, throwing DbError if the DB file is missing. */ diff --git a/src/db/index.ts b/src/db/index.ts index 517bd6493..4abb201ea 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -16,6 +16,7 @@ export { 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/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/query-helpers.ts b/src/domain/analysis/query-helpers.ts index 20c0cd3b0..6635cd953 100644 --- a/src/domain/analysis/query-helpers.ts +++ b/src/domain/analysis/query-helpers.ts @@ -1,24 +1,25 @@ -import { - openReadonlyOrFail, - openRepo, - type Repository, - resolveBusyTimeoutMs, -} 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, resolveBusyTimeoutMs(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(); }