diff --git a/src/cli/commands/embed.ts b/src/cli/commands/embed.ts index 7ad46a10c..c4b00e2e7 100644 --- a/src/cli/commands/embed.ts +++ b/src/cli/commands/embed.ts @@ -11,9 +11,9 @@ import { import { info, warn } from '../../infrastructure/logger.js'; import type { CommandDefinition } from '../types.js'; -function resolveStickyModel(dbPath: string | undefined): string | null { +function resolveStickyModel(dbPath: string | undefined, rootDir: string): string | null { try { - const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath)); + const db = openReadonlyOrFail(dbPath, resolveBusyTimeoutMs(dbPath ?? rootDir), rootDir); try { const storedName = getEmbeddingMeta(db, 'model'); if (!storedName) return null; @@ -47,7 +47,7 @@ export const command: CommandDefinition = { `Embedding strategy: ${EMBEDDING_STRATEGIES.join(', ')}. "structured" uses graph context (callers/callees), "source" embeds raw code`, 'structured', ], - ['-d, --db ', 'Path to graph.db'], + ['-d, --db ', 'Path to graph.db (default: /.codegraph/graph.db)'], ], validate([_dir], opts, ctx) { if (!(EMBEDDING_STRATEGIES as readonly string[]).includes(opts.strategy)) { @@ -85,7 +85,7 @@ export const command: CommandDefinition = { // before execute() runs — but keeps this branch type-safe. model = DEFAULT_MODEL; } else { - const sticky = resolveStickyModel(dbPath); + const sticky = resolveStickyModel(dbPath, root); if (sticky) { info( `Reusing previously-stored embedding model "${sticky}". Pass --model to switch, or set embeddings.model in your config.`, diff --git a/src/db/connection.ts b/src/db/connection.ts index 7c85980ce..c8321c6ef 100644 --- a/src/db/connection.ts +++ b/src/db/connection.ts @@ -303,13 +303,16 @@ function resolveDbSearchCeiling(rawCeiling: string | null): string | null { } } -/** Resolve symlinks in cwd (e.g. macOS /var → /private/var) so dir matches ceiling from git. */ -function resolveDbSearchStartDir(): string { +/** + * Resolve symlinks in the search start dir (e.g. macOS /var → /private/var) so + * it matches the ceiling from git. Defaults to cwd when no `dir` hint is given. + */ +function resolveDbSearchStartDir(dir: string = process.cwd()): string { try { - return fs.realpathSync(process.cwd()); + return fs.realpathSync(dir); } catch (e) { - debug(`realpathSync failed for cwd: ${toErrorMessage(e)}`); - return process.cwd(); + debug(`realpathSync failed for "${dir}": ${toErrorMessage(e)}`); + return dir; } } @@ -342,24 +345,40 @@ function walkUpForDbPath(startDir: string, ceiling: string | null): string | nul } } -export function findDbPath(customPath?: string): string { +/** + * Locate `.codegraph/graph.db` for the current command. + * + * When `customPath` is set (e.g. `--db`), it wins outright. + * Otherwise the search starts at `rootDirHint` when the caller has one + * (e.g. the positional `dir` argument on commands like `embed [dir]`) — + * mirroring `build [dir]`'s convention of resolving the DB relative to + * that directory — and falls back to `process.cwd()` when there is no hint, + * walking upward toward the enclosing git repo root either way. + */ +export function findDbPath(customPath?: string, rootDirHint?: string): string { if (customPath) { return resolveCustomDbPath(customPath); } - const ceiling = resolveDbSearchCeiling(findRepoRoot()); - const startDir = resolveDbSearchStartDir(); + const ceiling = resolveDbSearchCeiling(findRepoRoot(rootDirHint)); + const startDir = resolveDbSearchStartDir(rootDirHint); const found = walkUpForDbPath(startDir, ceiling); if (found) return found; - const base = ceiling || process.cwd(); + const base = rootDirHint || ceiling || process.cwd(); return path.join(base, '.codegraph', 'graph.db'); } -/** Open a database in readonly mode, with a user-friendly error if the DB doesn't exist. */ +/** + * Open a database in readonly mode, with a user-friendly error if the DB doesn't exist. + * `rootDirHint` is forwarded to `findDbPath()` for callers that know the project + * root (e.g. a command's positional `dir` argument) but weren't passed an explicit + * `--db`; it has no effect when `customPath` is set. + */ export function openReadonlyOrFail( customPath?: string, busyTimeoutMs: number = DEFAULTS.db.busyTimeoutMs, + rootDirHint?: string, ): BetterSqlite3Database { - const dbPath = findDbPath(customPath); + const dbPath = findDbPath(customPath, rootDirHint); if (!fs.existsSync(dbPath)) { throw new DbError( `No codegraph database found at ${dbPath}.\nRun "codegraph build" first to analyze your codebase.`, diff --git a/src/domain/search/generator.ts b/src/domain/search/generator.ts index 61f5e037d..44d04a0a5 100644 --- a/src/domain/search/generator.ts +++ b/src/domain/search/generator.ts @@ -226,7 +226,10 @@ export async function buildEmbeddings( options: BuildEmbeddingsOptions = {}, ): Promise { const strategy = options.strategy || 'structured'; - const dbPath = customDbPath || findDbPath(undefined); + // Search from rootDir (mirrors build's /.codegraph/graph.db convention), + // not process.cwd() — otherwise embed silently attaches to whatever unrelated + // .codegraph/graph.db happens to be found walking up from the current directory. + const dbPath = customDbPath || findDbPath(undefined, rootDir); if (!fs.existsSync(dbPath)) { throw new DbError( diff --git a/tests/search/embedding-strategy.test.ts b/tests/search/embedding-strategy.test.ts index f7a79f934..21dd723ef 100644 --- a/tests/search/embedding-strategy.test.ts +++ b/tests/search/embedding-strategy.test.ts @@ -459,6 +459,79 @@ describe('embed resolves source files from DB root, not cwd (#983)', () => { }); }); +describe('buildEmbeddings resolves default DB from rootDir, not cwd (#1869)', () => { + let targetDir: string, unrelatedCwdDir: string, targetDbPath: string, unrelatedDbPath: string; + let originalCwd: string; + + beforeAll(() => { + // The project embed is asked to operate on (positional `dir`, no --db). + targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-embed1869-target-')); + // An unrelated directory that happens to have its own graph.db — simulates + // running `embed ` from inside a different, larger repo checkout. + unrelatedCwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-embed1869-cwd-')); + + fs.writeFileSync(path.join(targetDir, 'c.js'), 'export function gamma() { return 3; }\n'); + + const targetDbDir = path.join(targetDir, '.codegraph'); + fs.mkdirSync(targetDbDir, { recursive: true }); + targetDbPath = path.join(targetDbDir, 'graph.db'); + const targetDb = new Database(targetDbPath); + targetDb.pragma('journal_mode = WAL'); + initSchema(targetDb); + insertNode(targetDb, 'gamma', 'function', 'c.js', 1, 1); + targetDb + .prepare('INSERT OR REPLACE INTO build_meta (key, value) VALUES (?, ?)') + .run('root_dir', path.resolve(targetDir)); + targetDb.close(); + + // A DB at the unrelated cwd, which must never be touched by this run. + const unrelatedDbDir = path.join(unrelatedCwdDir, '.codegraph'); + fs.mkdirSync(unrelatedDbDir, { recursive: true }); + unrelatedDbPath = path.join(unrelatedDbDir, 'graph.db'); + const unrelatedDb = new Database(unrelatedDbPath); + unrelatedDb.pragma('journal_mode = WAL'); + initSchema(unrelatedDb); + unrelatedDb.close(); + + originalCwd = process.cwd(); + }); + + afterAll(() => { + try { + process.chdir(originalCwd); + } catch { + /* ignore */ + } + if (targetDir) fs.rmSync(targetDir, { recursive: true, force: true }); + if (unrelatedCwdDir) fs.rmSync(unrelatedCwdDir, { recursive: true, force: true }); + }); + + test('writes to /.codegraph/graph.db, not the cwd DB, when --db is omitted', async () => { + EMBEDDED_TEXTS.length = 0; + process.chdir(unrelatedCwdDir); + + // Simulates `codegraph embed ` with no --db flag: buildEmbeddings + // must default the DB path relative to targetDir, exactly like `build `. + await buildEmbeddings(targetDir, 'minilm', undefined); + + expect(EMBEDDED_TEXTS.length).toBe(1); + + const targetDb = new Database(targetDbPath, { readonly: true }); + const targetCount = targetDb.prepare('SELECT COUNT(*) as c FROM embeddings').get().c; + targetDb.close(); + expect(targetCount).toBe(1); + + // The unrelated cwd DB was never opened for writing — it doesn't even + // have an `embeddings` table, since buildEmbeddings creates it lazily. + const unrelatedDb = new Database(unrelatedDbPath, { readonly: true }); + const unrelatedTables = unrelatedDb + .prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'embeddings'") + .all(); + unrelatedDb.close(); + expect(unrelatedTables).toHaveLength(0); + }); +}); + describe('context window overflow detection', () => { let bigDir: string, bigDbPath: string; diff --git a/tests/unit/db.test.ts b/tests/unit/db.test.ts index abd5895f7..ce9b4b11c 100644 --- a/tests/unit/db.test.ts +++ b/tests/unit/db.test.ts @@ -576,6 +576,90 @@ describe('findDbPath with git ceiling', () => { }); }); +describe('findDbPath with rootDirHint (#1869)', () => { + let targetDir: string, unrelatedCwdDir: string; + + beforeAll(() => { + // A project with its own graph.db — the intended target (e.g. `embed `'s + // positional argument). + targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-roothint-target-')); + fs.mkdirSync(path.join(targetDir, '.codegraph'), { recursive: true }); + fs.writeFileSync(path.join(targetDir, '.codegraph', 'graph.db'), ''); + // An unrelated directory with its OWN graph.db, standing in for whatever + // cwd the command happens to be invoked from. + unrelatedCwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-roothint-cwd-')); + fs.mkdirSync(path.join(unrelatedCwdDir, '.codegraph'), { recursive: true }); + fs.writeFileSync(path.join(unrelatedCwdDir, '.codegraph', 'graph.db'), ''); + // Resolve symlinks (macOS /tmp → /private/tmp) so expected paths match + // findDbPath's internal realpathSync'd start dir. + targetDir = fs.realpathSync(targetDir); + unrelatedCwdDir = fs.realpathSync(unrelatedCwdDir); + }); + + afterAll(() => { + fs.rmSync(targetDir, { recursive: true, force: true }); + fs.rmSync(unrelatedCwdDir, { recursive: true, force: true }); + }); + + afterEach(() => { + _resetRepoRootCache(); + }); + + it('searches from rootDirHint instead of cwd when no customPath is given', () => { + const origCwd = process.cwd; + process.cwd = () => unrelatedCwdDir; + try { + _resetRepoRootCache(); + const result = findDbPath(undefined, targetDir); + expect(result).toBe(path.join(targetDir, '.codegraph', 'graph.db')); + } finally { + process.cwd = origCwd; + } + }); + + it('still lets an explicit customPath win over rootDirHint', () => { + const explicit = path.join(tmpDir, 'explicit-override.db'); + const result = findDbPath(explicit, targetDir); + expect(result).toBe(path.resolve(explicit)); + }); + + it('falls back to cwd when rootDirHint is not provided (unchanged default behavior)', () => { + const origCwd = process.cwd; + process.cwd = () => unrelatedCwdDir; + try { + _resetRepoRootCache(); + const result = findDbPath(); + expect(result).toBe(path.join(unrelatedCwdDir, '.codegraph', 'graph.db')); + } finally { + process.cwd = origCwd; + } + }); + + it('prefers rootDirHint over the git ceiling for the no-DB-found default path (Greptile review)', () => { + // rootDirHint is a subdirectory *inside* a git repo, with no existing + // .codegraph/graph.db anywhere between it and the repo root — so + // walkUpForDbPath finds nothing and the fallback default-path branch + // runs. Before the fix, `ceiling || rootDirHint || cwd` let the git + // ceiling win whenever one existed, producing /.codegraph/graph.db + // instead of build's own /.codegraph/graph.db convention. + const repoRoot = fs.mkdtempSync(path.join(tmpDir, 'cg-hint-priority-')); + execFileSyncForSetup('git', ['init'], { cwd: repoRoot, stdio: 'pipe' }); + const subDir = path.join(repoRoot, 'packages', 'sub'); + fs.mkdirSync(subDir, { recursive: true }); + const resolvedRepoRoot = fs.realpathSync(repoRoot); + const resolvedSubDir = fs.realpathSync(subDir); + + _resetRepoRootCache(); + try { + const result = findDbPath(undefined, resolvedSubDir); + expect(result).toBe(path.join(resolvedSubDir, '.codegraph', 'graph.db')); + expect(result).not.toBe(path.join(resolvedRepoRoot, '.codegraph', 'graph.db')); + } finally { + _resetRepoRootCache(); + } + }); +}); + describe('openReadonlyOrFail', () => { it('throws DbError when DB does not exist', () => { expect.assertions(4); @@ -616,4 +700,37 @@ describe('openReadonlyOrFail', () => { expect(timeout).toBe(5000); readDb.close(); }); + + it('resolves against rootDirHint instead of cwd when no explicit path is given (#1869)', () => { + const targetDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-readonly-roothint-target-')); + const unrelatedCwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-readonly-roothint-cwd-')); + try { + const dbDir = path.join(targetDir, '.codegraph'); + fs.mkdirSync(dbDir, { recursive: true }); + const db = openDb(path.join(dbDir, 'graph.db')); + initSchema(db); + closeDb(db); + // A DB at the unrelated cwd must be ignored in favor of rootDirHint. + fs.mkdirSync(path.join(unrelatedCwdDir, '.codegraph'), { recursive: true }); + fs.writeFileSync(path.join(unrelatedCwdDir, '.codegraph', 'graph.db'), ''); + + const origCwd = process.cwd; + process.cwd = () => unrelatedCwdDir; + try { + _resetRepoRootCache(); + const readDb = openReadonlyOrFail(undefined, undefined, targetDir); + const tables = readDb + .prepare("SELECT name FROM sqlite_master WHERE type='table'") + .all() + .map((r) => r.name); + expect(tables).toContain('nodes'); + readDb.close(); + } finally { + process.cwd = origCwd; + } + } finally { + fs.rmSync(targetDir, { recursive: true, force: true }); + fs.rmSync(unrelatedCwdDir, { recursive: true, force: true }); + } + }); }); diff --git a/tests/unit/embed-command.test.ts b/tests/unit/embed-command.test.ts index 8c85595aa..82476a327 100644 --- a/tests/unit/embed-command.test.ts +++ b/tests/unit/embed-command.test.ts @@ -1,3 +1,4 @@ +import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('../../src/domain/search/index.js', async (importOriginal) => { @@ -8,11 +9,13 @@ vi.mock('../../src/db/index.js', () => ({ openReadonlyOrFail: vi.fn(() => { throw new Error('no db in this test'); }), + resolveBusyTimeoutMs: vi.fn(() => 5000), })); vi.mock('../../src/db/repository/embeddings.js', () => ({ getEmbeddingMeta: vi.fn() })); const { command } = await import('../../src/cli/commands/embed.js'); const { buildEmbeddings } = await import('../../src/domain/search/index.js'); +const { openReadonlyOrFail } = await import('../../src/db/index.js'); function fakeCtx(embeddings: Record, llm: Record = {}) { return { @@ -81,6 +84,7 @@ describe('embed command validate()', () => { describe('embed command execute()', () => { beforeEach(() => { vi.mocked(buildEmbeddings).mockClear(); + vi.mocked(openReadonlyOrFail).mockClear(); }); afterEach(() => { @@ -115,4 +119,21 @@ describe('embed command execute()', () => { const [, , , options] = vi.mocked(buildEmbeddings).mock.calls[0]!; expect(options.remote).toBeUndefined(); }); + + it('resolves the sticky-model DB lookup and buildEmbeddings against the positional dir, not cwd (#1869)', async () => { + const ctx = fakeCtx({}); + const targetDir = path.join('some', 'other', 'project'); + + await command.execute!([targetDir], { strategy: 'structured' } as never, ctx); + + // resolveStickyModel() must open the DB relative to the resolved dir, not + // whatever the process's cwd happens to be. + expect(openReadonlyOrFail).toHaveBeenCalledTimes(1); + const [, , rootDirHint] = vi.mocked(openReadonlyOrFail).mock.calls[0]!; + expect(rootDirHint).toBe(path.resolve(targetDir)); + + expect(buildEmbeddings).toHaveBeenCalledTimes(1); + const [rootArg] = vi.mocked(buildEmbeddings).mock.calls[0]!; + expect(rootArg).toBe(path.resolve(targetDir)); + }); });