Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
5d29cd9
fix: prevent findDbPath from escaping git worktree boundary
carlos-alm Mar 16, 2026
deac23e
fix: address review — real git ceiling test, debug log, robust non-gi…
carlos-alm Mar 16, 2026
34820e6
Merge remote-tracking branch 'origin/main' into fix/db-path-worktree-…
carlos-alm Mar 16, 2026
83efde5
test: strengthen findRepoRoot and ceiling tests per review feedback
carlos-alm Mar 16, 2026
ccb8bd5
Revert "test: strengthen findRepoRoot and ceiling tests per review fe…
carlos-alm Mar 16, 2026
73c51a4
Reapply "test: strengthen findRepoRoot and ceiling tests per review f…
carlos-alm Mar 16, 2026
e3c1d4a
fix: address Greptile review — flaky non-git test and misleading impo…
carlos-alm Mar 16, 2026
5b4a5cd
merge: resolve conflict with main in db test imports
carlos-alm Mar 16, 2026
20013d4
fix: resolve symlinks in findDbPath to fix ceiling check on macOS
carlos-alm Mar 16, 2026
7c877d7
style: format connection.js try/catch block
carlos-alm Mar 16, 2026
7e69570
Merge remote-tracking branch 'origin/main' into fix/db-path-worktree-…
carlos-alm Mar 16, 2026
0d0fdd9
test: harden findDbPath fallback test to mock execFileSync
carlos-alm Mar 16, 2026
2fa9dfc
fix: normalize findRepoRoot paths with realpathSync for cross-platfor…
carlos-alm Mar 16, 2026
651c164
fix: use stat-based path comparison for ceiling check on Windows
carlos-alm Mar 16, 2026
5dc707a
fix: remove test-only _resetRepoRootCache from public barrel export
carlos-alm Mar 16, 2026
5a9dd2b
Merge branch 'main' into fix/db-path-worktree-boundary
carlos-alm Mar 16, 2026
b87a12b
fix: tighten test assertions and key repo root cache on cwd
carlos-alm Mar 16, 2026
1301b9f
fix: normalize ceiling path in test to handle Windows 8.3 short names
carlos-alm Mar 16, 2026
25e940b
Merge branch 'main' into fix/db-path-worktree-boundary
carlos-alm Mar 16, 2026
5a77db1
fix: normalize ceiling path with realpathSync to handle Windows 8.3 s…
carlos-alm Mar 16, 2026
de87d18
Merge branch 'fix/db-path-worktree-boundary' of https://github.com/op…
carlos-alm Mar 16, 2026
620128b
fix: normalize Windows 8.3 short paths in ceiling boundary test
carlos-alm Mar 16, 2026
78b791c
fix: use existence-based assertions for ceiling test to handle Window…
carlos-alm Mar 16, 2026
8a41032
Merge remote-tracking branch 'origin/main' into fix/db-path-worktree-…
carlos-alm Mar 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 95 additions & 3 deletions src/db/connection.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,61 @@
import { execFileSync } from 'node:child_process';
import fs from 'node:fs';
import path from 'node:path';
import Database from 'better-sqlite3';
import { warn } from '../infrastructure/logger.js';
import { debug, warn } from '../infrastructure/logger.js';
import { DbError } from '../shared/errors.js';
import { Repository } from './repository/base.js';
import { SqliteRepository } from './repository/sqlite-repository.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 && _cachedRepoRootCwd === dir) {
return _cachedRepoRoot;
}
let root = null;
try {
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;
}
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) {
try {
process.kill(pid, 0);
Expand Down Expand Up @@ -46,6 +96,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 });
Expand All @@ -64,15 +130,41 @@ export function closeDb(db) {

export function findDbPath(customPath) {
if (customPath) return path.resolve(customPath);
let dir = process.cwd();
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 {
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;
if (ceiling && isSameDirectory(dir, ceiling)) {
debug(`findDbPath: stopped at git ceiling ${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');
Comment on lines +166 to +167
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undocumented behavior change for non-worktree users

When no existing DB is found during the walk-up, the default creation path has silently changed from process.cwd() to ceiling (the git repo root) for any user inside a git repository. Previously, running codegraph build from /repo/sub/ with no DB would create /repo/sub/.codegraph/graph.db. After this PR, it creates /repo/.codegraph/graph.db.

This is likely the desired long-term behavior, but it's a breaking change for users who intentionally scope their codegraph DB to a project subdirectory rather than the repo root. The PR description only mentions the worktree ceiling fix, not the changed default creation location.

Consider documenting this in the PR description or CHANGELOG, or preserving the previous fallback for non-worktree scenarios:

const base = ceiling || process.cwd();
return path.join(base, '.codegraph', 'graph.db');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on documenting this. Updated the PR description with a Behavior change section explaining that the default DB creation path is now the git repo root instead of process.cwd() when inside a git repo. The process.cwd() fallback is preserved when not in a git repo (line 109: const base = ceiling || process.cwd()).

}

/**
Expand Down
9 changes: 8 additions & 1 deletion src/db/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
// Barrel re-export — keeps all existing `import { ... } from '…/db/index.js'` working.
export { closeDb, findDbPath, openDb, openReadonlyOrFail, openRepo } from './connection.js';
export {
closeDb,
findDbPath,
findRepoRoot,
openDb,
openReadonlyOrFail,
openRepo,
} from './connection.js';
export { getBuildMeta, initSchema, MIGRATIONS, setBuildMeta } from './migrations.js';
export {
fanInJoinSQL,
Expand Down
163 changes: 160 additions & 3 deletions tests/unit/db.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,28 @@
* Unit tests for src/db.js — build_meta helpers included
*/

// 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';
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 } from '../../src/db/connection.js';
import {
closeDb,
findDbPath,
findRepoRoot,
getBuildMeta,
initSchema,
MIGRATIONS,
Expand Down Expand Up @@ -131,24 +145,30 @@ 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();
}
});

it('returns default path when no DB found', () => {
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 {
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();
}
});
});
Expand Down Expand Up @@ -194,6 +214,143 @@ 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', () => {
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('bypasses cache when called with explicit dir', () => {
_resetRepoRootCache();
execFileSyncSpy.mockClear();
const fromCwd = findRepoRoot();
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);
});
});

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/ (git init here — acts as ceiling)
// outerDir/worktree/sub/ (cwd inside worktree)
outerDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-ceiling-'));
worktreeRoot = path.join(outerDir, 'worktree');
fs.mkdirSync(path.join(outerDir, '.codegraph'), { recursive: true });
fs.writeFileSync(path.join(outerDir, '.codegraph', 'graph.db'), '');
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(() => {
fs.rmSync(outerDir, { recursive: true, force: true });
});

afterEach(() => {
_resetRepoRootCache();
});

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;
process.cwd = () => innerDir;
try {
_resetRepoRootCache();
Comment on lines +300 to +301
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test assertion will fail on macOS when /tmp is the temp dir

result is built from ceiling, which is the real (symlink-resolved) path returned by git rev-parse --show-toplevel. On macOS, if os.tmpdir() resolves through /tmp (which is a symlink to /private/tmp), git will return /private/tmp/cg-ceiling-xxx/worktree while worktreeRoot is still /tmp/cg-ceiling-xxx/worktree. The toBe assertion then compares:

  • actual: /private/tmp/cg-ceiling-xxx/worktree/.codegraph/graph.db
  • expected: /tmp/cg-ceiling-xxx/worktree/.codegraph/graph.db

These are not equal, so the test fails. The production code in connection.js is actually correct — realpathSync is applied to dir precisely to ensure ceiling (from git) and dir agree on symlinks. The test expectation just needs to match the same resolved form:

Suggested change
try {
_resetRepoRootCache();
expect(result).toBe(path.join(fs.realpathSync(worktreeRoot), '.codegraph', 'graph.db'));
expect(result).not.toContain(`${path.basename(outerDir)}${path.sep}.codegraph`);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 651c164. The test now uses \ for the expected ceiling instead of the test's , so the assertion always matches the path form that \ uses internally. This handles both the macOS symlink case (\ → ) and the Windows 8.3 short name case (\ → ). Additionally, the production code now uses a stat-based \ comparison (dev+ino fallback) instead of string equality for the ceiling check, which is robust against all path representation differences.

// 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(ceiling, '.codegraph', 'graph.db'));
expect(result).not.toContain(`${path.basename(outerDir)}${path.sep}.codegraph`);
} finally {
process.cwd = origCwd;
}
});

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();
// 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 });
}
});

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();
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
expect(result).toBe(path.join(emptyDir, '.codegraph', 'graph.db'));
} finally {
process.cwd = origCwd;
fs.rmSync(emptyDir, { recursive: true, force: true });
}
});
});

describe('openReadonlyOrFail', () => {
it('throws DbError when DB does not exist', () => {
expect.assertions(4);
Expand Down
Loading