From 5d29cd9e60984b4c20a0e0164f35a75161717b52 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 02:15:37 -0600 Subject: [PATCH 01/17] fix: prevent findDbPath from escaping git worktree boundary findDbPath() walks up from cwd looking for .codegraph/graph.db. In a git worktree (e.g. .claude/worktrees/agent-xxx/), this crosses the worktree boundary and finds the main repo's database instead. Add findRepoRoot() using `git rev-parse --show-toplevel` (which returns the correct root for both repos and worktrees) and use it as a ceiling in findDbPath(). The walk-up now stops at the git boundary, so each worktree resolves to its own database. --- src/db/connection.js | 40 +++++++++++++- tests/unit/db.test.js | 121 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 155 insertions(+), 6 deletions(-) diff --git a/src/db/connection.js b/src/db/connection.js index beffdc41..1961c3a8 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -1,8 +1,43 @@ +import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import Database from 'better-sqlite3'; import { warn } from '../logger.js'; +let _cachedRepoRoot = undefined; // undefined = not computed, null = not a git repo + +/** + * Return the git worktree/repo root for the given directory (or cwd). + * Uses `git rev-parse --show-toplevel` which returns the correct root + * for both regular repos and git worktrees. + * Results are cached per-process when called without arguments. + * @param {string} [fromDir] - Directory to resolve from (defaults to cwd) + * @returns {string | null} Absolute path to repo root, or null if not in a git repo + */ +export function findRepoRoot(fromDir) { + const dir = fromDir || process.cwd(); + if (!fromDir && _cachedRepoRoot !== undefined) return _cachedRepoRoot; + let root = null; + try { + root = path.resolve( + execFileSync('git', ['rev-parse', '--show-toplevel'], { + cwd: dir, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(), + ); + } catch { + root = null; + } + if (!fromDir) _cachedRepoRoot = root; + return root; +} + +/** Reset the cached repo root (for testing). */ +export function _resetRepoRootCache() { + _cachedRepoRoot = undefined; +} + function isProcessAlive(pid) { try { process.kill(pid, 0); @@ -61,15 +96,18 @@ export function closeDb(db) { export function findDbPath(customPath) { if (customPath) return path.resolve(customPath); + const ceiling = findRepoRoot(); let dir = process.cwd(); while (true) { const candidate = path.join(dir, '.codegraph', 'graph.db'); if (fs.existsSync(candidate)) return candidate; + if (ceiling && path.resolve(dir) === ceiling) break; const parent = path.dirname(dir); if (parent === dir) break; dir = parent; } - return path.join(process.cwd(), '.codegraph', 'graph.db'); + const base = ceiling || process.cwd(); + return path.join(base, '.codegraph', 'graph.db'); } /** diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 10fcbcde..ed9e292d 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -8,15 +8,14 @@ import path from 'node:path'; import Database from 'better-sqlite3'; import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; import { + _resetRepoRootCache, closeDb, findDbPath, - getBuildMeta, - initSchema, - MIGRATIONS, + findRepoRoot, openDb, openReadonlyOrFail, - setBuildMeta, -} from '../../src/db.js'; +} from '../../src/db/connection.js'; +import { getBuildMeta, initSchema, MIGRATIONS, setBuildMeta } from '../../src/db.js'; let tmpDir; @@ -131,11 +130,13 @@ describe('findDbPath', () => { const origCwd = process.cwd; process.cwd = () => deepDir; try { + _resetRepoRootCache(); const result = findDbPath(); expect(result).toContain('.codegraph'); expect(result).toContain('graph.db'); } finally { process.cwd = origCwd; + _resetRepoRootCache(); } }); @@ -144,11 +145,13 @@ describe('findDbPath', () => { const origCwd = process.cwd; process.cwd = () => emptyDir; try { + _resetRepoRootCache(); const result = findDbPath(); expect(result).toContain('.codegraph'); expect(result).toContain('graph.db'); } finally { process.cwd = origCwd; + _resetRepoRootCache(); } }); }); @@ -194,6 +197,114 @@ describe('build_meta', () => { }); }); +describe('findRepoRoot', () => { + beforeEach(() => { + _resetRepoRootCache(); + }); + + afterEach(() => { + _resetRepoRootCache(); + }); + + it('returns normalized git toplevel for the current repo', () => { + _resetRepoRootCache(); + const root = findRepoRoot(); + expect(root).toBeTruthy(); + expect(path.isAbsolute(root)).toBe(true); + // Should contain a .git entry at the root + expect(fs.existsSync(path.join(root, '.git'))).toBe(true); + }); + + it('returns null when not in a git repo', () => { + const root = findRepoRoot(os.tmpdir()); + // os.tmpdir() is typically not a git repo; if it is, skip gracefully + if (root === null) { + expect(root).toBeNull(); + } + }); + + it('caches results when called without arguments', () => { + _resetRepoRootCache(); + const first = findRepoRoot(); + const second = findRepoRoot(); + expect(first).toBe(second); + }); + + it('does not use cache when called with explicit dir', () => { + _resetRepoRootCache(); + const fromCwd = findRepoRoot(); + // Calling with an explicit dir should still work (not use cwd cache) + const fromExplicit = findRepoRoot(process.cwd()); + expect(fromExplicit).toBe(fromCwd); + }); +}); + +describe('findDbPath with git ceiling', () => { + let outerDir; + let innerDir; + + beforeAll(() => { + // Simulate a worktree-inside-repo layout: + // outerDir/.codegraph/graph.db (parent repo DB — should NOT be found) + // outerDir/worktree/ (simulated worktree root) + // outerDir/worktree/sub/ (cwd inside worktree) + outerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ceiling-')); + innerDir = path.join(outerDir, 'worktree', 'sub'); + fs.mkdirSync(path.join(outerDir, '.codegraph'), { recursive: true }); + fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); + fs.mkdirSync(innerDir, { recursive: true }); + }); + + afterAll(() => { + fs.rmSync(outerDir, { recursive: true, force: true }); + }); + + afterEach(() => { + _resetRepoRootCache(); + }); + + it('stops walking at git ceiling and does not find parent DB', () => { + const origCwd = process.cwd; + // Mock cwd to be inside the inner "worktree" + process.cwd = () => innerDir; + // Mock findRepoRoot to return the worktree root (one level up from innerDir) + const worktreeRoot = path.join(outerDir, 'worktree'); + const origFindRepoRoot = findRepoRoot; + + // We can't easily mock findRepoRoot since findDbPath calls it internally, + // but we can test the behavior by creating a DB at the worktree level + fs.mkdirSync(path.join(worktreeRoot, '.codegraph'), { recursive: true }); + fs.writeFileSync(path.join(worktreeRoot, '.codegraph', 'graph.db'), ''); + + try { + _resetRepoRootCache(); + const result = findDbPath(); + // Should find the worktree DB, not the outer one + expect(result).toContain('worktree'); + expect(result).toContain('.codegraph'); + } finally { + process.cwd = origCwd; + // Clean up the worktree DB + fs.rmSync(path.join(worktreeRoot, '.codegraph'), { recursive: true, force: true }); + } + }); + + it('falls back gracefully when not in a git repo', () => { + const emptyDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-nogit-')); + const origCwd = process.cwd; + process.cwd = () => emptyDir; + _resetRepoRootCache(); + try { + const result = findDbPath(); + // Should return default path at cwd since there's no git ceiling + expect(result).toBe(path.join(emptyDir, '.codegraph', 'graph.db')); + } finally { + process.cwd = origCwd; + fs.rmSync(emptyDir, { recursive: true, force: true }); + } + }); +}); + describe('openReadonlyOrFail', () => { it('exits with error when DB does not exist', () => { const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { From deac23ef110f1d73ec4b94f06624d6ac8b556557 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 02:23:01 -0600 Subject: [PATCH 02/17] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20rea?= =?UTF-8?q?l=20git=20ceiling=20test,=20debug=20log,=20robust=20non-git=20t?= =?UTF-8?q?est?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Ceiling test now uses `git init` to create a real git repo boundary, and verifies the outer DB is NOT found (without the ceiling fix it would be). Added separate test for finding DB within the boundary. - Non-git test uses a fresh mkdtemp dir instead of os.tmpdir(). - Added debug log when ceiling stops the walk-up. - Removed unused `origFindRepoRoot` variable. --- src/db/connection.js | 7 +++++-- tests/unit/db.test.js | 45 ++++++++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/db/connection.js b/src/db/connection.js index 1961c3a8..b0e589ef 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -2,7 +2,7 @@ import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; import Database from 'better-sqlite3'; -import { warn } from '../logger.js'; +import { debug, warn } from '../logger.js'; let _cachedRepoRoot = undefined; // undefined = not computed, null = not a git repo @@ -101,7 +101,10 @@ export function findDbPath(customPath) { while (true) { const candidate = path.join(dir, '.codegraph', 'graph.db'); if (fs.existsSync(candidate)) return candidate; - if (ceiling && path.resolve(dir) === ceiling) break; + if (ceiling && path.resolve(dir) === ceiling) { + debug(`findDbPath: stopped at git ceiling ${ceiling}`); + break; + } const parent = path.dirname(dir); if (parent === dir) break; dir = parent; diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index ed9e292d..7941fecc 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -2,6 +2,7 @@ * Unit tests for src/db.js — build_meta helpers included */ +import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -216,10 +217,13 @@ describe('findRepoRoot', () => { }); it('returns null when not in a git repo', () => { - const root = findRepoRoot(os.tmpdir()); - // os.tmpdir() is typically not a git repo; if it is, skip gracefully - if (root === null) { + // Create a fresh temp dir that is guaranteed not to be inside a git repo + const noGitDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-nogit-root-')); + try { + const root = findRepoRoot(noGitDir); expect(root).toBeNull(); + } finally { + fs.rmSync(noGitDir, { recursive: true, force: true }); } }); @@ -241,18 +245,22 @@ describe('findRepoRoot', () => { describe('findDbPath with git ceiling', () => { let outerDir; + let worktreeRoot; let innerDir; beforeAll(() => { // Simulate a worktree-inside-repo layout: // outerDir/.codegraph/graph.db (parent repo DB — should NOT be found) - // outerDir/worktree/ (simulated worktree root) + // outerDir/worktree/ (git init here — acts as ceiling) // outerDir/worktree/sub/ (cwd inside worktree) outerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ceiling-')); - innerDir = path.join(outerDir, 'worktree', 'sub'); + worktreeRoot = path.join(outerDir, 'worktree'); + innerDir = path.join(worktreeRoot, 'sub'); fs.mkdirSync(path.join(outerDir, '.codegraph'), { recursive: true }); fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); fs.mkdirSync(innerDir, { recursive: true }); + // Initialize a real git repo at the worktree root so findRepoRoot returns it + execFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); }); afterAll(() => { @@ -263,28 +271,35 @@ describe('findDbPath with git ceiling', () => { _resetRepoRootCache(); }); - it('stops walking at git ceiling and does not find parent DB', () => { + it('stops at git ceiling and does not find parent DB', () => { + // No DB inside the worktree — the only DB is in outerDir (beyond the ceiling). + // Without the ceiling fix, findDbPath would walk up and find outerDir's DB. const origCwd = process.cwd; - // Mock cwd to be inside the inner "worktree" process.cwd = () => innerDir; - // Mock findRepoRoot to return the worktree root (one level up from innerDir) - const worktreeRoot = path.join(outerDir, 'worktree'); - const origFindRepoRoot = findRepoRoot; + try { + _resetRepoRootCache(); + const result = findDbPath(); + // Should return default path at the ceiling root, NOT the outer DB + expect(result).toBe(path.join(worktreeRoot, '.codegraph', 'graph.db')); + expect(result).not.toContain(path.basename(outerDir) + path.sep + '.codegraph'); + } finally { + process.cwd = origCwd; + } + }); - // We can't easily mock findRepoRoot since findDbPath calls it internally, - // but we can test the behavior by creating a DB at the worktree level + it('finds DB within the ceiling boundary', () => { + // Create a DB inside the worktree — should be found normally fs.mkdirSync(path.join(worktreeRoot, '.codegraph'), { recursive: true }); fs.writeFileSync(path.join(worktreeRoot, '.codegraph', 'graph.db'), ''); - + const origCwd = process.cwd; + process.cwd = () => innerDir; try { _resetRepoRootCache(); const result = findDbPath(); - // Should find the worktree DB, not the outer one expect(result).toContain('worktree'); expect(result).toContain('.codegraph'); } finally { process.cwd = origCwd; - // Clean up the worktree DB fs.rmSync(path.join(worktreeRoot, '.codegraph'), { recursive: true, force: true }); } }); From 83efde5f9cfb56ac391ad437969dbe9826578b3f Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 02:34:27 -0600 Subject: [PATCH 03/17] test: strengthen findRepoRoot and ceiling tests per review feedback - Mock execFileSync via vi.mock to verify caching calls exactly once and cache bypass calls twice (not just comparing return values) - Mock execFileSync to throw in "not in git repo" test so the null path is always exercised regardless of host environment - Rename "does not use cache" test to "bypasses cache" with spy assertion --- tests/unit/db.test.js | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 87165d94..a938c66f 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -2,12 +2,21 @@ * Unit tests for src/db.js — build_meta helpers included */ -import { execFileSync } from 'node:child_process'; +import { execFileSync as realExecFileSync } from 'node:child_process'; 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 } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; + +const execFileSyncSpy = vi.hoisted(() => vi.fn()); + +vi.mock('node:child_process', async (importOriginal) => { + const mod = await importOriginal(); + execFileSyncSpy.mockImplementation(mod.execFileSync); + return { ...mod, execFileSync: execFileSyncSpy }; +}); + import { _resetRepoRootCache, closeDb, @@ -217,29 +226,30 @@ describe('findRepoRoot', () => { }); it('returns null when not in a git repo', () => { - // Create a fresh temp dir that is guaranteed not to be inside a git repo - const noGitDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-nogit-root-')); - try { - const root = findRepoRoot(noGitDir); - expect(root).toBeNull(); - } finally { - fs.rmSync(noGitDir, { recursive: true, force: true }); - } + execFileSyncSpy.mockImplementationOnce(() => { + throw new Error('not a git repo'); + }); + const root = findRepoRoot(os.tmpdir()); + expect(root).toBeNull(); }); it('caches results when called without arguments', () => { _resetRepoRootCache(); + execFileSyncSpy.mockClear(); const first = findRepoRoot(); const second = findRepoRoot(); expect(first).toBe(second); + expect(execFileSyncSpy).toHaveBeenCalledTimes(1); }); - it('does not use cache when called with explicit dir', () => { + it('bypasses cache when called with explicit dir', () => { _resetRepoRootCache(); + execFileSyncSpy.mockClear(); const fromCwd = findRepoRoot(); - // Calling with an explicit dir should still work (not use cwd cache) const fromExplicit = findRepoRoot(process.cwd()); expect(fromExplicit).toBe(fromCwd); + // First call populates cache, second call with explicit dir must call again + expect(execFileSyncSpy).toHaveBeenCalledTimes(2); }); }); @@ -260,7 +270,7 @@ describe('findDbPath with git ceiling', () => { fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); fs.mkdirSync(innerDir, { recursive: true }); // Initialize a real git repo at the worktree root so findRepoRoot returns it - execFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); + realExecFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); }); afterAll(() => { From ccb8bd554a166a04adf3c51c7c2d20ff2b2824b2 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 02:34:46 -0600 Subject: [PATCH 04/17] Revert "test: strengthen findRepoRoot and ceiling tests per review feedback" This reverts commit 83efde5f9cfb56ac391ad437969dbe9826578b3f. --- tests/unit/db.test.js | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index a938c66f..87165d94 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -2,21 +2,12 @@ * Unit tests for src/db.js — build_meta helpers included */ -import { execFileSync as realExecFileSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; 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'; - -const execFileSyncSpy = vi.hoisted(() => vi.fn()); - -vi.mock('node:child_process', async (importOriginal) => { - const mod = await importOriginal(); - execFileSyncSpy.mockImplementation(mod.execFileSync); - return { ...mod, execFileSync: execFileSyncSpy }; -}); - +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { _resetRepoRootCache, closeDb, @@ -226,30 +217,29 @@ describe('findRepoRoot', () => { }); it('returns null when not in a git repo', () => { - execFileSyncSpy.mockImplementationOnce(() => { - throw new Error('not a git repo'); - }); - const root = findRepoRoot(os.tmpdir()); - expect(root).toBeNull(); + // Create a fresh temp dir that is guaranteed not to be inside a git repo + const noGitDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-nogit-root-')); + try { + const root = findRepoRoot(noGitDir); + expect(root).toBeNull(); + } finally { + fs.rmSync(noGitDir, { recursive: true, force: true }); + } }); it('caches results when called without arguments', () => { _resetRepoRootCache(); - execFileSyncSpy.mockClear(); const first = findRepoRoot(); const second = findRepoRoot(); expect(first).toBe(second); - expect(execFileSyncSpy).toHaveBeenCalledTimes(1); }); - it('bypasses cache when called with explicit dir', () => { + it('does not use cache when called with explicit dir', () => { _resetRepoRootCache(); - execFileSyncSpy.mockClear(); const fromCwd = findRepoRoot(); + // Calling with an explicit dir should still work (not use cwd cache) const fromExplicit = findRepoRoot(process.cwd()); expect(fromExplicit).toBe(fromCwd); - // First call populates cache, second call with explicit dir must call again - expect(execFileSyncSpy).toHaveBeenCalledTimes(2); }); }); @@ -270,7 +260,7 @@ describe('findDbPath with git ceiling', () => { fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); fs.mkdirSync(innerDir, { recursive: true }); // Initialize a real git repo at the worktree root so findRepoRoot returns it - realExecFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); + execFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); }); afterAll(() => { From 73c51a45b4a84e40d35e9ad65ff78b45f02630f8 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 02:35:10 -0600 Subject: [PATCH 05/17] Reapply "test: strengthen findRepoRoot and ceiling tests per review feedback" This reverts commit ccb8bd554a166a04adf3c51c7c2d20ff2b2824b2. --- tests/unit/db.test.js | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 87165d94..a938c66f 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -2,12 +2,21 @@ * Unit tests for src/db.js — build_meta helpers included */ -import { execFileSync } from 'node:child_process'; +import { execFileSync as realExecFileSync } from 'node:child_process'; 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 } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; + +const execFileSyncSpy = vi.hoisted(() => vi.fn()); + +vi.mock('node:child_process', async (importOriginal) => { + const mod = await importOriginal(); + execFileSyncSpy.mockImplementation(mod.execFileSync); + return { ...mod, execFileSync: execFileSyncSpy }; +}); + import { _resetRepoRootCache, closeDb, @@ -217,29 +226,30 @@ describe('findRepoRoot', () => { }); it('returns null when not in a git repo', () => { - // Create a fresh temp dir that is guaranteed not to be inside a git repo - const noGitDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-nogit-root-')); - try { - const root = findRepoRoot(noGitDir); - expect(root).toBeNull(); - } finally { - fs.rmSync(noGitDir, { recursive: true, force: true }); - } + execFileSyncSpy.mockImplementationOnce(() => { + throw new Error('not a git repo'); + }); + const root = findRepoRoot(os.tmpdir()); + expect(root).toBeNull(); }); it('caches results when called without arguments', () => { _resetRepoRootCache(); + execFileSyncSpy.mockClear(); const first = findRepoRoot(); const second = findRepoRoot(); expect(first).toBe(second); + expect(execFileSyncSpy).toHaveBeenCalledTimes(1); }); - it('does not use cache when called with explicit dir', () => { + it('bypasses cache when called with explicit dir', () => { _resetRepoRootCache(); + execFileSyncSpy.mockClear(); const fromCwd = findRepoRoot(); - // Calling with an explicit dir should still work (not use cwd cache) const fromExplicit = findRepoRoot(process.cwd()); expect(fromExplicit).toBe(fromCwd); + // First call populates cache, second call with explicit dir must call again + expect(execFileSyncSpy).toHaveBeenCalledTimes(2); }); }); @@ -260,7 +270,7 @@ describe('findDbPath with git ceiling', () => { fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); fs.mkdirSync(innerDir, { recursive: true }); // Initialize a real git repo at the worktree root so findRepoRoot returns it - execFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); + realExecFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); }); afterAll(() => { From e3c1d4a139d2da1327b9f92a9c3e80810d809aa2 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 03:01:26 -0600 Subject: [PATCH 06/17] =?UTF-8?q?fix:=20address=20Greptile=20review=20?= =?UTF-8?q?=E2=80=94=20flaky=20non-git=20test=20and=20misleading=20import?= =?UTF-8?q?=20name?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Mock execFileSync to throw in "falls back gracefully when not in a git repo" test, preventing flakiness when os.tmpdir() is inside a git repo - Rename realExecFileSync → execFileSyncForSetup with comment explaining it resolves to the spy due to vi.mock hoisting --- tests/unit/db.test.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index a938c66f..317e9fe9 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -2,7 +2,9 @@ * Unit tests for src/db.js — build_meta helpers included */ -import { execFileSync as realExecFileSync } from 'node:child_process'; +// Note: due to vi.mock hoisting, this resolves to the spy (which delegates +// to the real impl by default). Safe for setup calls before mockImplementationOnce. +import { execFileSync as execFileSyncForSetup } from 'node:child_process'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -270,7 +272,7 @@ describe('findDbPath with git ceiling', () => { fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); fs.mkdirSync(innerDir, { recursive: true }); // Initialize a real git repo at the worktree root so findRepoRoot returns it - realExecFileSync('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); + execFileSyncForSetup('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); }); afterAll(() => { @@ -319,6 +321,9 @@ describe('findDbPath with git ceiling', () => { const origCwd = process.cwd; process.cwd = () => emptyDir; _resetRepoRootCache(); + execFileSyncSpy.mockImplementationOnce(() => { + throw new Error('not a git repo'); + }); try { const result = findDbPath(); // Should return default path at cwd since there's no git ceiling From 20013d40a993965bb205b331f394bdc6ef17bb06 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 03:08:27 -0600 Subject: [PATCH 07/17] fix: resolve symlinks in findDbPath to fix ceiling check on macOS On macOS, os.tmpdir() returns /var/folders/... but git rev-parse returns /private/var/folders/... (resolved symlink). The ceiling comparison failed because the paths didn't match. Use fs.realpathSync on cwd to normalize symlinks before comparing against the ceiling. Impact: 1 functions changed, 1 affected --- src/db/connection.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/db/connection.js b/src/db/connection.js index f4bd1cf8..6167c38a 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -98,7 +98,9 @@ export function closeDb(db) { export function findDbPath(customPath) { if (customPath) return path.resolve(customPath); const ceiling = findRepoRoot(); - let dir = process.cwd(); + // Resolve symlinks (e.g. macOS /var → /private/var) so dir matches ceiling from git + let dir; + try { dir = fs.realpathSync(process.cwd()); } catch { dir = process.cwd(); } while (true) { const candidate = path.join(dir, '.codegraph', 'graph.db'); if (fs.existsSync(candidate)) return candidate; From 7c877d71ed2b0b1237aa7971b6c17a9f1ff64db5 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 03:08:47 -0600 Subject: [PATCH 08/17] style: format connection.js try/catch block Impact: 1 functions changed, 1 affected --- src/db/connection.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/db/connection.js b/src/db/connection.js index 6167c38a..dce39b2f 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -5,7 +5,7 @@ import Database from 'better-sqlite3'; import { DbError } from '../errors.js'; import { debug, warn } from '../logger.js'; -let _cachedRepoRoot = undefined; // undefined = not computed, null = not a git repo +let _cachedRepoRoot; // undefined = not computed, null = not a git repo /** * Return the git worktree/repo root for the given directory (or cwd). @@ -100,7 +100,11 @@ export function findDbPath(customPath) { const ceiling = findRepoRoot(); // Resolve symlinks (e.g. macOS /var → /private/var) so dir matches ceiling from git let dir; - try { dir = fs.realpathSync(process.cwd()); } catch { dir = process.cwd(); } + try { + dir = fs.realpathSync(process.cwd()); + } catch { + dir = process.cwd(); + } while (true) { const candidate = path.join(dir, '.codegraph', 'graph.db'); if (fs.existsSync(candidate)) return candidate; From 0d0fdd94d4441f54545e5d20f190a4f061325bb9 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 03:21:13 -0600 Subject: [PATCH 09/17] test: harden findDbPath fallback test to mock execFileSync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'returns default path when no DB found' test didn't control the git ceiling — if tmpDir was inside a git repo, findRepoRoot() would return a non-null ceiling and the default path would differ from emptyDir. Mock execFileSync to throw so the cwd fallback is always exercised. --- tests/unit/db.test.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index be6db766..14ff3042 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -159,11 +159,13 @@ describe('findDbPath', () => { const emptyDir = fs.mkdtempSync(path.join(tmpDir, 'empty-')); const origCwd = process.cwd; process.cwd = () => emptyDir; + _resetRepoRootCache(); + execFileSyncSpy.mockImplementationOnce(() => { + throw new Error('not a git repo'); + }); try { - _resetRepoRootCache(); const result = findDbPath(); - expect(result).toContain('.codegraph'); - expect(result).toContain('graph.db'); + expect(result).toBe(path.join(emptyDir, '.codegraph', 'graph.db')); } finally { process.cwd = origCwd; _resetRepoRootCache(); From 2fa9dfc4bad8511ceb4150f7e48159632f9617cd Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 03:28:36 -0600 Subject: [PATCH 10/17] fix: normalize findRepoRoot paths with realpathSync for cross-platform ceiling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On macOS, os.tmpdir() returns /var/... but git resolves symlinks to /private/var/..., and on Windows, 8.3 short names (RUNNER~1) differ from long names (runneradmin). findDbPath normalizes dir via fs.realpathSync but findRepoRoot only used path.resolve, causing the ceiling comparison to fail — the walk crossed the worktree boundary. Fix findRepoRoot to use fs.realpathSync on git output, and resolve test paths after directory creation so assertions match. Impact: 1 functions changed, 77 affected --- src/db/connection.js | 20 +++++++++++++------- tests/unit/db.test.js | 8 ++++++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/db/connection.js b/src/db/connection.js index 16cb6777..fcff8e20 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -20,13 +20,19 @@ export function findRepoRoot(fromDir) { if (!fromDir && _cachedRepoRoot !== undefined) return _cachedRepoRoot; let root = null; try { - root = path.resolve( - execFileSync('git', ['rev-parse', '--show-toplevel'], { - cwd: dir, - encoding: 'utf-8', - stdio: ['pipe', 'pipe', 'pipe'], - }).trim(), - ); + const raw = execFileSync('git', ['rev-parse', '--show-toplevel'], { + cwd: dir, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }).trim(); + // Use realpathSync to resolve symlinks (macOS /var → /private/var) and + // 8.3 short names (Windows RUNNER~1 → runneradmin) so the ceiling path + // matches the realpathSync'd dir in findDbPath. + try { + root = fs.realpathSync(raw); + } catch { + root = path.resolve(raw); + } } catch { root = null; } diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 14ff3042..33520a7e 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -272,12 +272,16 @@ describe('findDbPath with git ceiling', () => { // outerDir/worktree/sub/ (cwd inside worktree) outerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ceiling-')); worktreeRoot = path.join(outerDir, 'worktree'); - innerDir = path.join(worktreeRoot, 'sub'); fs.mkdirSync(path.join(outerDir, '.codegraph'), { recursive: true }); fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), ''); - fs.mkdirSync(innerDir, { recursive: true }); + fs.mkdirSync(path.join(worktreeRoot, 'sub'), { recursive: true }); // Initialize a real git repo at the worktree root so findRepoRoot returns it execFileSyncForSetup('git', ['init'], { cwd: worktreeRoot, stdio: 'pipe' }); + // Resolve symlinks (macOS /var → /private/var) and 8.3 short names + // (Windows RUNNER~1 → runneradmin) so test paths match findRepoRoot output. + outerDir = fs.realpathSync(outerDir); + worktreeRoot = fs.realpathSync(worktreeRoot); + innerDir = path.join(worktreeRoot, 'sub'); }); afterAll(() => { From 651c1642bf754e46b47a96e03b90c4b4ac8d540d Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 03:45:50 -0600 Subject: [PATCH 11/17] fix: use stat-based path comparison for ceiling check on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit git rev-parse resolves 8.3 short names (RUNNER~1 → runneradmin) but fs.realpathSync may not, causing the string comparison to fail and the walk to escape the git ceiling boundary. Replace string equality with isSameDirectory() that falls back to dev+ino comparison when paths differ textually but refer to the same directory. Also fix the test assertion to use findRepoRoot() for the expected ceiling path instead of the test's worktreeRoot, since git may resolve paths differently than realpathSync. Impact: 2 functions changed, 137 affected --- src/db/connection.js | 18 +++++++++++++++++- tests/unit/db.test.js | 6 +++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/db/connection.js b/src/db/connection.js index fcff8e20..ff57a720 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -85,6 +85,22 @@ function releaseAdvisoryLock(lockPath) { } } +/** + * Check if two paths refer to the same directory. + * Handles Windows 8.3 short names (RUNNER~1 vs runneradmin) and macOS + * symlinks (/tmp vs /private/tmp) where string comparison fails. + */ +function isSameDirectory(a, b) { + if (path.resolve(a) === path.resolve(b)) return true; + try { + const sa = fs.statSync(a); + const sb = fs.statSync(b); + return sa.dev === sb.dev && sa.ino === sb.ino; + } catch { + return false; + } +} + export function openDb(dbPath) { const dir = path.dirname(dbPath); if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true }); @@ -114,7 +130,7 @@ export function findDbPath(customPath) { while (true) { const candidate = path.join(dir, '.codegraph', 'graph.db'); if (fs.existsSync(candidate)) return candidate; - if (ceiling && path.resolve(dir) === ceiling) { + if (ceiling && isSameDirectory(dir, ceiling)) { debug(`findDbPath: stopped at git ceiling ${ceiling}`); break; } diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 33520a7e..c7bef2b3 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -299,9 +299,13 @@ describe('findDbPath with git ceiling', () => { process.cwd = () => innerDir; try { _resetRepoRootCache(); + // Use findRepoRoot() for the expected ceiling — git may resolve 8.3 short + // names (Windows RUNNER~1 → runneradmin) or symlinks (macOS /tmp → /private/tmp) + // differently than fs.realpathSync on the test's worktreeRoot. + const ceiling = findRepoRoot(); const result = findDbPath(); // Should return default path at the ceiling root, NOT the outer DB - expect(result).toBe(path.join(worktreeRoot, '.codegraph', 'graph.db')); + expect(result).toBe(path.join(ceiling, '.codegraph', 'graph.db')); expect(result).not.toContain(`${path.basename(outerDir)}${path.sep}.codegraph`); } finally { process.cwd = origCwd; From 5dc707a2dbe89f02e519e0dff813baaf49e5157d Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 04:06:14 -0600 Subject: [PATCH 12/17] fix: remove test-only _resetRepoRootCache from public barrel export --- src/db/index.js | 1 - tests/unit/db.test.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/db/index.js b/src/db/index.js index aa6fcdb8..882b5f89 100644 --- a/src/db/index.js +++ b/src/db/index.js @@ -1,6 +1,5 @@ // Barrel re-export — keeps all existing `import { ... } from '…/db/index.js'` working. export { - _resetRepoRootCache, closeDb, findDbPath, findRepoRoot, diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index c7bef2b3..638dfa97 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -19,8 +19,8 @@ vi.mock('node:child_process', async (importOriginal) => { return { ...mod, execFileSync: execFileSyncSpy }; }); +import { _resetRepoRootCache } from '../../src/db/connection.js'; import { - _resetRepoRootCache, closeDb, findDbPath, findRepoRoot, From b87a12b01f469937bb3f8edbf7975093c6f10a04 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 04:24:59 -0600 Subject: [PATCH 13/17] fix: tighten test assertions and key repo root cache on cwd Impact: 2 functions changed, 2 affected --- src/db/connection.js | 13 +++++++++++-- tests/unit/db.test.js | 4 ++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/db/connection.js b/src/db/connection.js index ff57a720..9bd13d1f 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -6,18 +6,23 @@ import { debug, warn } from '../infrastructure/logger.js'; import { DbError } from '../shared/errors.js'; let _cachedRepoRoot; // undefined = not computed, null = not a git repo +let _cachedRepoRootCwd; // cwd at the time the cache was populated /** * Return the git worktree/repo root for the given directory (or cwd). * Uses `git rev-parse --show-toplevel` which returns the correct root * for both regular repos and git worktrees. * Results are cached per-process when called without arguments. + * The cache is keyed on cwd so it invalidates if the working directory changes + * (e.g. MCP server serving multiple sessions). * @param {string} [fromDir] - Directory to resolve from (defaults to cwd) * @returns {string | null} Absolute path to repo root, or null if not in a git repo */ export function findRepoRoot(fromDir) { const dir = fromDir || process.cwd(); - if (!fromDir && _cachedRepoRoot !== undefined) return _cachedRepoRoot; + if (!fromDir && _cachedRepoRoot !== undefined && _cachedRepoRootCwd === dir) { + return _cachedRepoRoot; + } let root = null; try { const raw = execFileSync('git', ['rev-parse', '--show-toplevel'], { @@ -36,13 +41,17 @@ export function findRepoRoot(fromDir) { } catch { root = null; } - if (!fromDir) _cachedRepoRoot = root; + if (!fromDir) { + _cachedRepoRoot = root; + _cachedRepoRootCwd = dir; + } return root; } /** Reset the cached repo root (for testing). */ export function _resetRepoRootCache() { _cachedRepoRoot = undefined; + _cachedRepoRootCwd = undefined; } function isProcessAlive(pid) { diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 638dfa97..d733cc48 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -320,9 +320,9 @@ describe('findDbPath with git ceiling', () => { process.cwd = () => innerDir; try { _resetRepoRootCache(); + const ceiling = findRepoRoot(); const result = findDbPath(); - expect(result).toContain('worktree'); - expect(result).toContain('.codegraph'); + expect(result).toBe(path.join(ceiling, '.codegraph', 'graph.db')); } finally { process.cwd = origCwd; fs.rmSync(path.join(worktreeRoot, '.codegraph'), { recursive: true, force: true }); From 1301b9f7ceacfdab046bd996b6e9240df7c2d26f Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 04:33:33 -0600 Subject: [PATCH 14/17] fix: normalize ceiling path in test to handle Windows 8.3 short names --- tests/unit/db.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index d733cc48..cc4d9354 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -320,7 +320,7 @@ describe('findDbPath with git ceiling', () => { process.cwd = () => innerDir; try { _resetRepoRootCache(); - const ceiling = findRepoRoot(); + const ceiling = fs.realpathSync(findRepoRoot()); const result = findDbPath(); expect(result).toBe(path.join(ceiling, '.codegraph', 'graph.db')); } finally { From 5a77db18d4dfe9d6971eaed688c9351808d860a7 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 04:44:00 -0600 Subject: [PATCH 15/17] fix: normalize ceiling path with realpathSync to handle Windows 8.3 short names Impact: 1 functions changed, 1 affected --- src/db/connection.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/db/connection.js b/src/db/connection.js index 9bd13d1f..ca775f3f 100644 --- a/src/db/connection.js +++ b/src/db/connection.js @@ -128,7 +128,21 @@ export function closeDb(db) { export function findDbPath(customPath) { if (customPath) return path.resolve(customPath); - const ceiling = findRepoRoot(); + const rawCeiling = findRepoRoot(); + // Normalize ceiling with realpathSync to resolve 8.3 short names (Windows + // RUNNER~1 → runneradmin) and symlinks (macOS /var → /private/var). + // findRepoRoot already applies realpathSync internally, but the git output + // may still contain short names on some Windows CI environments. + let ceiling; + if (rawCeiling) { + try { + ceiling = fs.realpathSync(rawCeiling); + } catch { + ceiling = rawCeiling; + } + } else { + ceiling = null; + } // Resolve symlinks (e.g. macOS /var → /private/var) so dir matches ceiling from git let dir; try { From 620128b5c00c3c1c27779170b218dff7116a6fcf Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 04:54:23 -0600 Subject: [PATCH 16/17] fix: normalize Windows 8.3 short paths in ceiling boundary test On Windows CI, fs.realpathSync(process.cwd()) and git rev-parse --show-toplevel can resolve 8.3 short names (RUNNER~1) differently than long names (runneradmin). Apply realpathSync to both sides of the assertion so the comparison is path-equivalent. --- tests/unit/db.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index cc4d9354..524c61d9 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -322,7 +322,10 @@ describe('findDbPath with git ceiling', () => { _resetRepoRootCache(); const ceiling = fs.realpathSync(findRepoRoot()); const result = findDbPath(); - expect(result).toBe(path.join(ceiling, '.codegraph', 'graph.db')); + // Use realpathSync on both sides to normalize Windows 8.3 short names + // (e.g. RUNNER~1 vs runneradmin) that path.dirname walking may preserve + const expected = path.join(ceiling, '.codegraph', 'graph.db'); + expect(fs.realpathSync(result)).toBe(fs.realpathSync(expected)); } finally { process.cwd = origCwd; fs.rmSync(path.join(worktreeRoot, '.codegraph'), { recursive: true, force: true }); From 78b791c54cb852cc8c00045f85444a4acefeca7f Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Mon, 16 Mar 2026 05:03:31 -0600 Subject: [PATCH 17/17] fix: use existence-based assertions for ceiling test to handle Windows 8.3 names --- tests/unit/db.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit/db.test.js b/tests/unit/db.test.js index 524c61d9..a1be6c4b 100644 --- a/tests/unit/db.test.js +++ b/tests/unit/db.test.js @@ -320,12 +320,12 @@ describe('findDbPath with git ceiling', () => { process.cwd = () => innerDir; try { _resetRepoRootCache(); - const ceiling = fs.realpathSync(findRepoRoot()); const result = findDbPath(); - // Use realpathSync on both sides to normalize Windows 8.3 short names - // (e.g. RUNNER~1 vs runneradmin) that path.dirname walking may preserve - const expected = path.join(ceiling, '.codegraph', 'graph.db'); - expect(fs.realpathSync(result)).toBe(fs.realpathSync(expected)); + // Verify the DB was found (file exists) and is the worktree DB, not the outer one + expect(fs.existsSync(result)).toBe(true); + expect(result).toMatch(/\.codegraph[/\\]graph\.db$/); + // The outer DB is at outerDir/.codegraph — verify we didn't find that one + expect(result).not.toContain(`${path.basename(outerDir)}${path.sep}.codegraph`); } finally { process.cwd = origCwd; fs.rmSync(path.join(worktreeRoot, '.codegraph'), { recursive: true, force: true });