Skip to content

Commit 68949dc

Browse files
authored
fix(embed): resolve default DB path from positional dir, not cwd (#1869)
fix(embed): resolve default DB path from positional dir, not cwd (#1869) Fixes embed's default DB path resolution to prefer the positional <dir> argument over process.cwd(), matching build's behavior. Also fixes two Greptile-flagged issues: rootDirHint priority in findDbPath's no-DB-found fallback, and resolveBusyTimeoutMs's config lookup path. While resolving CI on this PR, found and fixed a genuine Windows-only test bug: the #1869 regression test opened two `Database(readonly)` handles for verification queries and never closed them. POSIX tolerates unlinking a file with an open handle, masking the leak on macOS/Linux, but Windows does not — afterAll's fs.rmSync failed with EBUSY: resource busy or locked, unlink '...\graph.db'. Fixed to close handles per the file's own established pattern; Windows Node 22 tests pass clean now. The Pre-publish benchmark gate failed once after that fix (the routine flaky "1-file rebuild" timing gate seen on prior PRs) and passed clean on rerun with no code changes.
1 parent 90447ea commit 68949dc

6 files changed

Lines changed: 249 additions & 16 deletions

File tree

src/cli/commands/embed.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ import {
1111
import { info, warn } from '../../infrastructure/logger.js';
1212
import type { CommandDefinition } from '../types.js';
1313

14-
function resolveStickyModel(dbPath: string | undefined): string | null {
14+
function resolveStickyModel(dbPath: string | undefined, rootDir: string): string | null {
1515
try {
16-
const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath));
16+
const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath ?? rootDir), rootDir);
1717
try {
1818
const storedName = getEmbeddingMeta(db, 'model');
1919
if (!storedName) return null;
@@ -47,7 +47,7 @@ export const command: CommandDefinition = {
4747
`Embedding strategy: ${EMBEDDING_STRATEGIES.join(', ')}. "structured" uses graph context (callers/callees), "source" embeds raw code`,
4848
'structured',
4949
],
50-
['-d, --db <path>', 'Path to graph.db'],
50+
['-d, --db <path>', 'Path to graph.db (default: <dir>/.codegraph/graph.db)'],
5151
],
5252
validate([_dir], opts, ctx) {
5353
if (!(EMBEDDING_STRATEGIES as readonly string[]).includes(opts.strategy)) {
@@ -85,7 +85,7 @@ export const command: CommandDefinition = {
8585
// before execute() runs — but keeps this branch type-safe.
8686
model = DEFAULT_MODEL;
8787
} else {
88-
const sticky = resolveStickyModel(dbPath);
88+
const sticky = resolveStickyModel(dbPath, root);
8989
if (sticky) {
9090
info(
9191
`Reusing previously-stored embedding model "${sticky}". Pass --model to switch, or set embeddings.model in your config.`,

src/db/connection.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -303,13 +303,16 @@ function resolveDbSearchCeiling(rawCeiling: string | null): string | null {
303303
}
304304
}
305305

306-
/** Resolve symlinks in cwd (e.g. macOS /var → /private/var) so dir matches ceiling from git. */
307-
function resolveDbSearchStartDir(): string {
306+
/**
307+
* Resolve symlinks in the search start dir (e.g. macOS /var → /private/var) so
308+
* it matches the ceiling from git. Defaults to cwd when no `dir` hint is given.
309+
*/
310+
function resolveDbSearchStartDir(dir: string = process.cwd()): string {
308311
try {
309-
return fs.realpathSync(process.cwd());
312+
return fs.realpathSync(dir);
310313
} catch (e) {
311-
debug(`realpathSync failed for cwd: ${toErrorMessage(e)}`);
312-
return process.cwd();
314+
debug(`realpathSync failed for "${dir}": ${toErrorMessage(e)}`);
315+
return dir;
313316
}
314317
}
315318

@@ -342,24 +345,40 @@ function walkUpForDbPath(startDir: string, ceiling: string | null): string | nul
342345
}
343346
}
344347

345-
export function findDbPath(customPath?: string): string {
348+
/**
349+
* Locate `.codegraph/graph.db` for the current command.
350+
*
351+
* When `customPath` is set (e.g. `--db`), it wins outright.
352+
* Otherwise the search starts at `rootDirHint` when the caller has one
353+
* (e.g. the positional `dir` argument on commands like `embed [dir]`) —
354+
* mirroring `build [dir]`'s convention of resolving the DB relative to
355+
* that directory — and falls back to `process.cwd()` when there is no hint,
356+
* walking upward toward the enclosing git repo root either way.
357+
*/
358+
export function findDbPath(customPath?: string, rootDirHint?: string): string {
346359
if (customPath) {
347360
return resolveCustomDbPath(customPath);
348361
}
349-
const ceiling = resolveDbSearchCeiling(findRepoRoot());
350-
const startDir = resolveDbSearchStartDir();
362+
const ceiling = resolveDbSearchCeiling(findRepoRoot(rootDirHint));
363+
const startDir = resolveDbSearchStartDir(rootDirHint);
351364
const found = walkUpForDbPath(startDir, ceiling);
352365
if (found) return found;
353-
const base = ceiling || process.cwd();
366+
const base = rootDirHint || ceiling || process.cwd();
354367
return path.join(base, '.codegraph', 'graph.db');
355368
}
356369

357-
/** Open a database in readonly mode, with a user-friendly error if the DB doesn't exist. */
370+
/**
371+
* Open a database in readonly mode, with a user-friendly error if the DB doesn't exist.
372+
* `rootDirHint` is forwarded to `findDbPath()` for callers that know the project
373+
* root (e.g. a command's positional `dir` argument) but weren't passed an explicit
374+
* `--db`; it has no effect when `customPath` is set.
375+
*/
358376
export function openReadonlyOrFail(
359377
customPath?: string,
360378
busyTimeoutMs: number = DEFAULTS.db.busyTimeoutMs,
379+
rootDirHint?: string,
361380
): BetterSqlite3Database {
362-
const dbPath = findDbPath(customPath);
381+
const dbPath = findDbPath(customPath, rootDirHint);
363382
if (!fs.existsSync(dbPath)) {
364383
throw new DbError(
365384
`No codegraph database found at ${dbPath}.\nRun "codegraph build" first to analyze your codebase.`,

src/domain/search/generator.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,10 @@ export async function buildEmbeddings(
226226
options: BuildEmbeddingsOptions = {},
227227
): Promise<void> {
228228
const strategy = options.strategy || 'structured';
229-
const dbPath = customDbPath || findDbPath(undefined);
229+
// Search from rootDir (mirrors build's <rootDir>/.codegraph/graph.db convention),
230+
// not process.cwd() — otherwise embed silently attaches to whatever unrelated
231+
// .codegraph/graph.db happens to be found walking up from the current directory.
232+
const dbPath = customDbPath || findDbPath(undefined, rootDir);
230233

231234
if (!fs.existsSync(dbPath)) {
232235
throw new DbError(

tests/search/embedding-strategy.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,79 @@ describe('embed resolves source files from DB root, not cwd (#983)', () => {
459459
});
460460
});
461461

462+
describe('buildEmbeddings resolves default DB from rootDir, not cwd (#1869)', () => {
463+
let targetDir: string, unrelatedCwdDir: string, targetDbPath: string, unrelatedDbPath: string;
464+
let originalCwd: string;
465+
466+
beforeAll(() => {
467+
// The project embed is asked to operate on (positional `dir`, no --db).
468+
targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-embed1869-target-'));
469+
// An unrelated directory that happens to have its own graph.db — simulates
470+
// running `embed <dir>` from inside a different, larger repo checkout.
471+
unrelatedCwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-embed1869-cwd-'));
472+
473+
fs.writeFileSync(path.join(targetDir, 'c.js'), 'export function gamma() { return 3; }\n');
474+
475+
const targetDbDir = path.join(targetDir, '.codegraph');
476+
fs.mkdirSync(targetDbDir, { recursive: true });
477+
targetDbPath = path.join(targetDbDir, 'graph.db');
478+
const targetDb = new Database(targetDbPath);
479+
targetDb.pragma('journal_mode = WAL');
480+
initSchema(targetDb);
481+
insertNode(targetDb, 'gamma', 'function', 'c.js', 1, 1);
482+
targetDb
483+
.prepare('INSERT OR REPLACE INTO build_meta (key, value) VALUES (?, ?)')
484+
.run('root_dir', path.resolve(targetDir));
485+
targetDb.close();
486+
487+
// A DB at the unrelated cwd, which must never be touched by this run.
488+
const unrelatedDbDir = path.join(unrelatedCwdDir, '.codegraph');
489+
fs.mkdirSync(unrelatedDbDir, { recursive: true });
490+
unrelatedDbPath = path.join(unrelatedDbDir, 'graph.db');
491+
const unrelatedDb = new Database(unrelatedDbPath);
492+
unrelatedDb.pragma('journal_mode = WAL');
493+
initSchema(unrelatedDb);
494+
unrelatedDb.close();
495+
496+
originalCwd = process.cwd();
497+
});
498+
499+
afterAll(() => {
500+
try {
501+
process.chdir(originalCwd);
502+
} catch {
503+
/* ignore */
504+
}
505+
if (targetDir) fs.rmSync(targetDir, { recursive: true, force: true });
506+
if (unrelatedCwdDir) fs.rmSync(unrelatedCwdDir, { recursive: true, force: true });
507+
});
508+
509+
test('writes to <dir>/.codegraph/graph.db, not the cwd DB, when --db is omitted', async () => {
510+
EMBEDDED_TEXTS.length = 0;
511+
process.chdir(unrelatedCwdDir);
512+
513+
// Simulates `codegraph embed <targetDir>` with no --db flag: buildEmbeddings
514+
// must default the DB path relative to targetDir, exactly like `build <dir>`.
515+
await buildEmbeddings(targetDir, 'minilm', undefined);
516+
517+
expect(EMBEDDED_TEXTS.length).toBe(1);
518+
519+
const targetDb = new Database(targetDbPath, { readonly: true });
520+
const targetCount = targetDb.prepare('SELECT COUNT(*) as c FROM embeddings').get().c;
521+
targetDb.close();
522+
expect(targetCount).toBe(1);
523+
524+
// The unrelated cwd DB was never opened for writing — it doesn't even
525+
// have an `embeddings` table, since buildEmbeddings creates it lazily.
526+
const unrelatedDb = new Database(unrelatedDbPath, { readonly: true });
527+
const unrelatedTables = unrelatedDb
528+
.prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'embeddings'")
529+
.all();
530+
unrelatedDb.close();
531+
expect(unrelatedTables).toHaveLength(0);
532+
});
533+
});
534+
462535
describe('context window overflow detection', () => {
463536
let bigDir: string, bigDbPath: string;
464537

tests/unit/db.test.ts

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,90 @@ describe('findDbPath with git ceiling', () => {
576576
});
577577
});
578578

579+
describe('findDbPath with rootDirHint (#1869)', () => {
580+
let targetDir: string, unrelatedCwdDir: string;
581+
582+
beforeAll(() => {
583+
// A project with its own graph.db — the intended target (e.g. `embed <dir>`'s
584+
// positional argument).
585+
targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-roothint-target-'));
586+
fs.mkdirSync(path.join(targetDir, '.codegraph'), { recursive: true });
587+
fs.writeFileSync(path.join(targetDir, '.codegraph', 'graph.db'), '');
588+
// An unrelated directory with its OWN graph.db, standing in for whatever
589+
// cwd the command happens to be invoked from.
590+
unrelatedCwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-roothint-cwd-'));
591+
fs.mkdirSync(path.join(unrelatedCwdDir, '.codegraph'), { recursive: true });
592+
fs.writeFileSync(path.join(unrelatedCwdDir, '.codegraph', 'graph.db'), '');
593+
// Resolve symlinks (macOS /tmp → /private/tmp) so expected paths match
594+
// findDbPath's internal realpathSync'd start dir.
595+
targetDir = fs.realpathSync(targetDir);
596+
unrelatedCwdDir = fs.realpathSync(unrelatedCwdDir);
597+
});
598+
599+
afterAll(() => {
600+
fs.rmSync(targetDir, { recursive: true, force: true });
601+
fs.rmSync(unrelatedCwdDir, { recursive: true, force: true });
602+
});
603+
604+
afterEach(() => {
605+
_resetRepoRootCache();
606+
});
607+
608+
it('searches from rootDirHint instead of cwd when no customPath is given', () => {
609+
const origCwd = process.cwd;
610+
process.cwd = () => unrelatedCwdDir;
611+
try {
612+
_resetRepoRootCache();
613+
const result = findDbPath(undefined, targetDir);
614+
expect(result).toBe(path.join(targetDir, '.codegraph', 'graph.db'));
615+
} finally {
616+
process.cwd = origCwd;
617+
}
618+
});
619+
620+
it('still lets an explicit customPath win over rootDirHint', () => {
621+
const explicit = path.join(tmpDir, 'explicit-override.db');
622+
const result = findDbPath(explicit, targetDir);
623+
expect(result).toBe(path.resolve(explicit));
624+
});
625+
626+
it('falls back to cwd when rootDirHint is not provided (unchanged default behavior)', () => {
627+
const origCwd = process.cwd;
628+
process.cwd = () => unrelatedCwdDir;
629+
try {
630+
_resetRepoRootCache();
631+
const result = findDbPath();
632+
expect(result).toBe(path.join(unrelatedCwdDir, '.codegraph', 'graph.db'));
633+
} finally {
634+
process.cwd = origCwd;
635+
}
636+
});
637+
638+
it('prefers rootDirHint over the git ceiling for the no-DB-found default path (Greptile review)', () => {
639+
// rootDirHint is a subdirectory *inside* a git repo, with no existing
640+
// .codegraph/graph.db anywhere between it and the repo root — so
641+
// walkUpForDbPath finds nothing and the fallback default-path branch
642+
// runs. Before the fix, `ceiling || rootDirHint || cwd` let the git
643+
// ceiling win whenever one existed, producing <git-root>/.codegraph/graph.db
644+
// instead of build's own <rootDirHint>/.codegraph/graph.db convention.
645+
const repoRoot = fs.mkdtempSync(path.join(tmpDir, 'cg-hint-priority-'));
646+
execFileSyncForSetup('git', ['init'], { cwd: repoRoot, stdio: 'pipe' });
647+
const subDir = path.join(repoRoot, 'packages', 'sub');
648+
fs.mkdirSync(subDir, { recursive: true });
649+
const resolvedRepoRoot = fs.realpathSync(repoRoot);
650+
const resolvedSubDir = fs.realpathSync(subDir);
651+
652+
_resetRepoRootCache();
653+
try {
654+
const result = findDbPath(undefined, resolvedSubDir);
655+
expect(result).toBe(path.join(resolvedSubDir, '.codegraph', 'graph.db'));
656+
expect(result).not.toBe(path.join(resolvedRepoRoot, '.codegraph', 'graph.db'));
657+
} finally {
658+
_resetRepoRootCache();
659+
}
660+
});
661+
});
662+
579663
describe('openReadonlyOrFail', () => {
580664
it('throws DbError when DB does not exist', () => {
581665
expect.assertions(4);
@@ -616,4 +700,37 @@ describe('openReadonlyOrFail', () => {
616700
expect(timeout).toBe(5000);
617701
readDb.close();
618702
});
703+
704+
it('resolves against rootDirHint instead of cwd when no explicit path is given (#1869)', () => {
705+
const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-readonly-roothint-target-'));
706+
const unrelatedCwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-readonly-roothint-cwd-'));
707+
try {
708+
const dbDir = path.join(targetDir, '.codegraph');
709+
fs.mkdirSync(dbDir, { recursive: true });
710+
const db = openDb(path.join(dbDir, 'graph.db'));
711+
initSchema(db);
712+
closeDb(db);
713+
// A DB at the unrelated cwd must be ignored in favor of rootDirHint.
714+
fs.mkdirSync(path.join(unrelatedCwdDir, '.codegraph'), { recursive: true });
715+
fs.writeFileSync(path.join(unrelatedCwdDir, '.codegraph', 'graph.db'), '');
716+
717+
const origCwd = process.cwd;
718+
process.cwd = () => unrelatedCwdDir;
719+
try {
720+
_resetRepoRootCache();
721+
const readDb = openReadonlyOrFail(undefined, undefined, targetDir);
722+
const tables = readDb
723+
.prepare("SELECT name FROM sqlite_master WHERE type='table'")
724+
.all()
725+
.map((r) => r.name);
726+
expect(tables).toContain('nodes');
727+
readDb.close();
728+
} finally {
729+
process.cwd = origCwd;
730+
}
731+
} finally {
732+
fs.rmSync(targetDir, { recursive: true, force: true });
733+
fs.rmSync(unrelatedCwdDir, { recursive: true, force: true });
734+
}
735+
});
619736
});

tests/unit/embed-command.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import path from 'node:path';
12
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
23

34
vi.mock('../../src/domain/search/index.js', async (importOriginal) => {
@@ -8,11 +9,13 @@ vi.mock('../../src/db/index.js', () => ({
89
openReadonlyOrFail: vi.fn(() => {
910
throw new Error('no db in this test');
1011
}),
12+
resolveBusyTimeoutMs: vi.fn(() => 5000),
1113
}));
1214
vi.mock('../../src/db/repository/embeddings.js', () => ({ getEmbeddingMeta: vi.fn() }));
1315

1416
const { command } = await import('../../src/cli/commands/embed.js');
1517
const { buildEmbeddings } = await import('../../src/domain/search/index.js');
18+
const { openReadonlyOrFail } = await import('../../src/db/index.js');
1619

1720
function fakeCtx(embeddings: Record<string, unknown>, llm: Record<string, unknown> = {}) {
1821
return {
@@ -81,6 +84,7 @@ describe('embed command validate()', () => {
8184
describe('embed command execute()', () => {
8285
beforeEach(() => {
8386
vi.mocked(buildEmbeddings).mockClear();
87+
vi.mocked(openReadonlyOrFail).mockClear();
8488
});
8589

8690
afterEach(() => {
@@ -115,4 +119,21 @@ describe('embed command execute()', () => {
115119
const [, , , options] = vi.mocked(buildEmbeddings).mock.calls[0]!;
116120
expect(options.remote).toBeUndefined();
117121
});
122+
123+
it('resolves the sticky-model DB lookup and buildEmbeddings against the positional dir, not cwd (#1869)', async () => {
124+
const ctx = fakeCtx({});
125+
const targetDir = path.join('some', 'other', 'project');
126+
127+
await command.execute!([targetDir], { strategy: 'structured' } as never, ctx);
128+
129+
// resolveStickyModel() must open the DB relative to the resolved dir, not
130+
// whatever the process's cwd happens to be.
131+
expect(openReadonlyOrFail).toHaveBeenCalledTimes(1);
132+
const [, , rootDirHint] = vi.mocked(openReadonlyOrFail).mock.calls[0]!;
133+
expect(rootDirHint).toBe(path.resolve(targetDir));
134+
135+
expect(buildEmbeddings).toHaveBeenCalledTimes(1);
136+
const [rootArg] = vi.mocked(buildEmbeddings).mock.calls[0]!;
137+
expect(rootArg).toBe(path.resolve(targetDir));
138+
});
118139
});

0 commit comments

Comments
 (0)