Skip to content

Commit 6babf15

Browse files
authored
fix: derive loadConfig() rootDir from --db path in read-only query functions (#2018)
Resolves conflicts against main: this branch was far behind (82 conflicting files across 3+ weeks of independent merges), of which only 9 files plus a new test file were actually owned by this PR's own commit — the remaining 73 were pure stack-noise from the branch predating those merges, bulk-resolved to main's content and verified byte-identical. The one substantive conflict: main had independently added an equivalent `resolveDbConfig()` helper (via a different PR, already wired into `withReadonlyDb()`) while this PR introduced the same helper under the name `resolveConfigForDbPath()`. Kept main's already-established name/signature (including its #1943 double-loadConfig optimization on `resolveBusyTimeoutMs`, which other already-merged files depend on) and renamed this PR's six call-site fixes to use it — preserving 100% of this PR's actual functional fix (issue #1881) with zero duplicate helpers. Addressed a Greptile review finding: `auditData()` was passing `opts.config` (possibly undefined) to `resolveThresholds()` instead of the already-resolved `config`, causing a redundant `resolveDbConfig()`/`loadConfig()` call. Fixed in b630cac; Greptile's re-review confirmed the fix and gave 5/5 confidence, "safe to merge." Pre-publish benchmark gate failed 3x on both the original push and the audit.ts fix push, always the same razor-thin "1-file rebuild" signature (75-78% vs 75% threshold) — consistent with the known flaky CI-runner-noise pattern seen throughout this session. Verified locally clean both times: `RUN_REGRESSION_GUARD=1 npm run test:regression-guard` → 25/25 passed. Merging via --admin.
1 parent 1620409 commit 6babf15

9 files changed

Lines changed: 263 additions & 41 deletions

File tree

src/db/connection.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ function resolveDbSettings(
431431
customDbPath: string | undefined,
432432
engineOpt: 'native' | 'wasm' | 'auto' | undefined,
433433
): ResolvedDbSettings {
434-
const config = loadConfig(deriveRootDirFromDbPath(customDbPath));
434+
const config = resolveDbConfig(customDbPath);
435435
// config.build.engine is already populated from CODEGRAPH_ENGINE env by applyEnvOverrides,
436436
// so this covers both the env-var path and the .codegraphrc.json config-file path.
437437
return {
@@ -444,7 +444,11 @@ function resolveDbSettings(
444444
* Resolve the full config for a given DB path, deriving rootDir the same way
445445
* resolveDbSettings()/resolveBusyTimeoutMs() do. Exported so callers that need
446446
* both the busy-timeout and other config values (e.g. withReadonlyDb()) can
447-
* share a single loadConfig() call instead of resolving it twice.
447+
* share a single loadConfig() call instead of resolving it twice. This is the
448+
* single entry point every read-only query function should use to call
449+
* loadConfig() when it has (or can resolve) a `--db` path — so `--db
450+
* /other/repo/.codegraph/graph.db` reads *that* repo's `.codegraphrc.json`
451+
* instead of the invoking directory's (issue #1881).
448452
*
449453
* MUST be called before opening any DB handle, for the same reason as
450454
* resolveDbSettings(): loadConfig can throw (e.g. ConfigError via

src/domain/analysis/diff-impact.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { execFileSync } from 'node:child_process';
22
import fs from 'node:fs';
33
import path from 'node:path';
4-
import { findDbPath, openReadonlyOrFail } from '../../db/index.js';
4+
import { findDbPath, openReadonlyOrFail, resolveDbConfig } from '../../db/index.js';
55
import { cachedStmt } from '../../db/repository/cached-stmt.js';
66
import { evaluateBoundaries } from '../../features/boundaries.js';
77
import { coChangeForFiles } from '../../features/cochange.js';
@@ -272,8 +272,10 @@ export function diffImpactData(
272272
// Resolve config before opening the DB so config.db.busyTimeoutMs can be
273273
// threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s
274274
// ordering in db/connection.ts — loadConfig can throw, and an already-open
275-
// handle at that point would never be closed).
276-
const config = opts.config || loadConfig();
275+
// handle at that point would never be closed). Derives rootDir from
276+
// customDbPath (not process.cwd()) so `--db /other/repo/...` reads that
277+
// repo's .codegraphrc.json (issue #1881).
278+
const config = opts.config || resolveDbConfig(customDbPath);
277279
const db = openReadonlyOrFail(
278280
customDbPath,
279281
config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs,

src/domain/search/search/hybrid.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { openReadonlyOrFail } from '../../../db/index.js';
2-
import { DEFAULTS, loadConfig } from '../../../infrastructure/config.js';
1+
import { openReadonlyOrFail, resolveDbConfig } from '../../../db/index.js';
2+
import { DEFAULTS } from '../../../infrastructure/config.js';
33
import type { BetterSqlite3Database, CodegraphConfig } from '../../../types.js';
44
import { hasFtsIndex } from '../stores/fts5.js';
55
import { ftsSearchData } from './keyword.js';
@@ -178,7 +178,9 @@ export async function hybridSearchData(
178178
customDbPath: string | undefined,
179179
opts: SemanticSearchOpts = {},
180180
): Promise<HybridSearchResult | null> {
181-
const config = opts.config || loadConfig();
181+
// Derive rootDir from customDbPath (not process.cwd()) so `--db /other/repo/...`
182+
// reads that repo's .codegraphrc.json (issue #1881).
183+
const config = opts.config || resolveDbConfig(customDbPath);
182184
const searchCfg = config.search || ({} as CodegraphConfig['search']);
183185
const limit = opts.limit ?? searchCfg.topK ?? 15;
184186
const k = opts.rrfK ?? searchCfg.rrfK ?? 60;

src/features/audit.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import path from 'node:path';
2-
import { openReadonlyOrFail } from '../db/index.js';
1+
import { openReadonlyOrFail, resolveDbConfig } from '../db/index.js';
32
import { normalizeFileFilter } from '../db/query-builder.js';
43
import { bfsTransitiveCallers } from '../domain/analysis/impact.js';
54
import { explainData } from '../domain/queries.js';
6-
import { DEFAULTS, loadConfig } from '../infrastructure/config.js';
5+
import { DEFAULTS } from '../infrastructure/config.js';
76
import { debug } from '../infrastructure/logger.js';
87
import { isTestFile } from '../infrastructure/test-filter.js';
98
import { toErrorMessage } from '../shared/errors.js';
@@ -35,13 +34,7 @@ function resolveThresholds(
3534
config: unknown,
3635
): Record<string, ThresholdEntry> {
3736
try {
38-
const cfg =
39-
config ||
40-
(() => {
41-
const dbDir = customDbPath ? path.dirname(customDbPath) : process.cwd();
42-
const repoRoot = path.resolve(dbDir, '..');
43-
return loadConfig(repoRoot);
44-
})();
37+
const cfg = config || resolveDbConfig(customDbPath);
4538
const userRules = (cfg as Record<string, unknown>).manifesto || {};
4639
const resolved: Record<string, ThresholdEntry> = {};
4740
for (const def of FUNCTION_RULES) {
@@ -144,7 +137,10 @@ export function auditData(
144137
opts: AuditDataOpts = {},
145138
): AuditResult {
146139
const noTests = opts.noTests || false;
147-
const config = opts.config || loadConfig();
140+
// Derive rootDir from customDbPath (not process.cwd()) so `--db /other/repo/...`
141+
// reads that repo's .codegraphrc.json (issue #1881) — matches resolveThresholds()
142+
// below, which already derived rootDir correctly.
143+
const config = opts.config || resolveDbConfig(customDbPath);
148144
const maxDepth =
149145
opts.depth ||
150146
(config as unknown as { analysis?: { auditDepth?: number } }).analysis?.auditDepth ||
@@ -174,7 +170,7 @@ export function auditData(
174170
customDbPath,
175171
config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs,
176172
);
177-
const thresholds = resolveThresholds(customDbPath, opts.config);
173+
const thresholds = resolveThresholds(customDbPath, config);
178174

179175
let functions: AuditFunctionEntry[];
180176
try {

src/features/complexity-query.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55
* pagination) from compute-time concerns (AST traversal, metric algorithms).
66
*/
77

8-
import { openReadonlyOrFail } from '../db/index.js';
8+
import { openReadonlyOrFail, resolveDbConfig } from '../db/index.js';
99
import { buildFileConditionSQL } from '../db/query-builder.js';
10-
import { DEFAULTS, loadConfig } from '../infrastructure/config.js';
10+
import { DEFAULTS } from '../infrastructure/config.js';
1111
import { debug } from '../infrastructure/logger.js';
1212
import { isTestFile } from '../infrastructure/test-filter.js';
1313
import { paginateResult } from '../shared/paginate.js';
@@ -281,14 +281,19 @@ interface ComplexityQueryOpts {
281281
}
282282

283283
/** Resolve query flags + effective manifesto thresholds from opts/config/DEFAULTS. */
284-
function resolveComplexityQueryOptions(opts: ComplexityQueryOpts): {
284+
function resolveComplexityQueryOptions(
285+
customDbPath: string | undefined,
286+
opts: ComplexityQueryOpts,
287+
): {
285288
sort: string;
286289
noTests: boolean;
287290
aboveThreshold: boolean;
288291
thresholds: any;
289292
busyTimeoutMs: number;
290293
} {
291-
const config = opts.config || loadConfig(process.cwd());
294+
// Derive rootDir from customDbPath (not process.cwd()) so `--db /other/repo/...`
295+
// reads that repo's .codegraphrc.json (issue #1881).
296+
const config = opts.config || resolveDbConfig(customDbPath);
292297
return {
293298
sort: opts.sort || 'cognitive',
294299
noTests: opts.noTests || false,
@@ -330,7 +335,7 @@ export function complexityData(
330335
// resolveDbSettings()'s ordering in db/connection.ts, since loadConfig can
331336
// throw and an already-open handle at that point would never be closed.
332337
const { sort, noTests, aboveThreshold, thresholds, busyTimeoutMs } =
333-
resolveComplexityQueryOptions(opts);
338+
resolveComplexityQueryOptions(customDbPath, opts);
334339
const db = openReadonlyOrFail(customDbPath, busyTimeoutMs);
335340
try {
336341
const { where, params } = buildComplexityWhere({

src/features/manifesto.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { openReadonlyOrFail } from '../db/index.js';
1+
import { openReadonlyOrFail, resolveDbConfig } from '../db/index.js';
22
import { buildFileConditionSQL } from '../db/query-builder.js';
33
import { findCycles } from '../domain/graph/cycles.js';
4-
import { DEFAULTS, loadConfig } from '../infrastructure/config.js';
4+
import { DEFAULTS } from '../infrastructure/config.js';
55
import { debug } from '../infrastructure/logger.js';
66
import { paginateResult } from '../shared/paginate.js';
77
import type { BetterSqlite3Database, CodegraphConfig, ThresholdRule } from '../types.js';
@@ -482,8 +482,10 @@ export function manifestoData(
482482
// Resolve config before opening the DB so config.db.busyTimeoutMs can be
483483
// threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s
484484
// ordering in db/connection.ts — loadConfig can throw, and an already-open
485-
// handle at that point would never be closed).
486-
const config = opts.config || loadConfig(process.cwd());
485+
// handle at that point would never be closed). Derives rootDir from
486+
// customDbPath (not process.cwd()) so `--db /other/repo/...` reads that
487+
// repo's .codegraphrc.json (issue #1881).
488+
const config = opts.config || resolveDbConfig(customDbPath);
487489
const db = openReadonlyOrFail(
488490
customDbPath,
489491
config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs,

src/features/structure-query.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ import {
1111
openReadonlyOrFail,
1212
openReadonlyWithNative,
1313
resolveBusyTimeoutMs,
14+
resolveDbConfig,
1415
testFilterSQL,
1516
} from '../db/index.js';
16-
import { DEFAULTS, loadConfig } from '../infrastructure/config.js';
17+
import { DEFAULTS } from '../infrastructure/config.js';
1718
import { isTestFile } from '../infrastructure/test-filter.js';
1819
import { normalizePath } from '../shared/constants.js';
1920
import { paginateResult } from '../shared/paginate.js';
@@ -384,8 +385,10 @@ export function moduleBoundariesData(
384385
// Resolve config before opening the DB so config.db.busyTimeoutMs can be
385386
// threaded through to openReadonlyOrFail() (mirrors resolveDbSettings()'s
386387
// ordering in db/connection.ts — loadConfig can throw, and an already-open
387-
// handle at that point would never be closed).
388-
const config = opts.config || loadConfig();
388+
// handle at that point would never be closed). Derives rootDir from
389+
// customDbPath (not process.cwd()) so `--db /other/repo/...` reads that
390+
// repo's .codegraphrc.json (issue #1881).
391+
const config = opts.config || resolveDbConfig(customDbPath);
389392
const db = openReadonlyOrFail(
390393
customDbPath,
391394
config.db?.busyTimeoutMs ?? DEFAULTS.db.busyTimeoutMs,

tests/unit/busy-timeout-query-sites.test.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,10 @@ describe('configured busyTimeoutMs reaches PRAGMA busy_timeout at ad-hoc read-on
101101
});
102102

103103
it('manifestoData (config load reordered to before the db opens) applies the configured busy_timeout', () => {
104-
// manifestoData resolves its config via loadConfig(process.cwd()) — a
105-
// pre-existing, cwd-based resolution this fix intentionally preserves
106-
// (only the *timing* of the call moved, not what it resolves) — so the
107-
// project config must be visible from process.cwd() here, not from dbPath.
108-
const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpDir);
104+
// manifestoData resolves its config from the resolved DB path's rootDir
105+
// (issue #1881 fix), not process.cwd() — so an unrelated cwd must not
106+
// affect resolution.
107+
const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(os.tmpdir());
109108
const capture = captureBusyTimeoutPragmas();
110109
try {
111110
manifestoData(dbPath);
@@ -117,10 +116,9 @@ describe('configured busyTimeoutMs reaches PRAGMA busy_timeout at ad-hoc read-on
117116
});
118117

119118
it('hybridSearchData (already loaded config before the db opens) applies the configured busy_timeout', async () => {
120-
// Same pre-existing cwd-based config resolution as manifestoData (bare
121-
// loadConfig() defaults to process.cwd()) — preserved, not introduced, by
122-
// this fix.
123-
const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tmpDir);
119+
// Same DB-path-derived config resolution as manifestoData (issue #1881
120+
// fix) — an unrelated cwd must not affect resolution.
121+
const cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(os.tmpdir());
124122
const capture = captureBusyTimeoutPragmas();
125123
try {
126124
await hybridSearchData('nonexistent-query', dbPath);

0 commit comments

Comments
 (0)