Skip to content

Commit 4761346

Browse files
committed
fix: reopen DB when its file is replaced at the same path (#925)
After a project dir is removed and recreated at the same path (a `git worktree remove`+`add`, or a fresh `codegraph init`), a long-lived MCP server kept reading the old, now-unlinked inode while init/sync wrote to the new one — serving a stale snapshot for the life of the daemon, with `codegraph sync` unable to refresh it. DatabaseConnection records its DB file's (dev, ino) at open and exposes isFileReplaced(); CodeGraph surfaces it as isDbReplaced(). Detection runs at the per-tool-call chokepoint, ToolHandler.getCodeGraph: - cross-project (projectPath) cache hits evict + close a replaced entry so the next query reopens against the new inode; - the default project is reopened via a hook the MCPEngine registers (it owns the default instance's watcher + catch-up), driven from the default-serving path rather than the cold init methods, which a loaded daemon never re-enters. A missing file (mid-recreate) is ignored so a transient gap never churns the connection.
1 parent 4aa2752 commit 4761346

5 files changed

Lines changed: 275 additions & 8 deletions

File tree

__tests__/mcp-db-reopen.test.ts

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/**
2+
* Reopen a DB connection when its file is replaced under us (same path, new
3+
* inode). Regression for issue #925.
4+
*
5+
* Scenario: the project dir is removed and recreated at the same path (a
6+
* `git worktree remove` + `git worktree add`, or a fresh `codegraph init`).
7+
* A long-lived SQLite handle keeps reading the old, now-unlinked inode while
8+
* `init`/`sync` write to the new inode at the same path — so the MCP server
9+
* served a stale snapshot for the life of the daemon. Four layers are covered:
10+
* 1. `DatabaseConnection.isFileReplaced()` — the inode-identity primitive.
11+
* 2. `CodeGraph.isDbReplaced()` — a live instance reports its DB was swapped.
12+
* 3. `ToolHandler.liveCachedGraph()` — a cross-project (`projectPath`) cache
13+
* hit on a replaced DB is evicted + closed, so the caller reopens against
14+
* the new inode (the existing open path) instead of serving the stale one.
15+
* 4. `ToolHandler.getCodeGraph()` default path — a swapped default project is
16+
* reopened via the engine reload hook (the steady-state tool-call path,
17+
* which the cold init methods never re-enter — issue #925's headline case).
18+
*/
19+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
20+
import * as fs from 'fs';
21+
import * as path from 'path';
22+
import * as os from 'os';
23+
import CodeGraph from '../src/index';
24+
import { DatabaseConnection } from '../src/db';
25+
import { ToolHandler } from '../src/mcp/tools';
26+
27+
const rmDb = (dbPath: string) => {
28+
for (const p of [dbPath, `${dbPath}-wal`, `${dbPath}-shm`]) fs.rmSync(p, { force: true });
29+
};
30+
31+
describe('DatabaseConnection.isFileReplaced (issue #925)', () => {
32+
let dir: string;
33+
beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-dbid-')); });
34+
afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); });
35+
36+
it('false while the same file; true after the file at the path is swapped; false while missing', () => {
37+
const dbPath = path.join(dir, 'graph.db');
38+
const conn = DatabaseConnection.initialize(dbPath); // records inode A
39+
expect(conn.isFileReplaced()).toBe(false);
40+
41+
// Replace the file at the same path with a brand-new inode (the recreate).
42+
rmDb(dbPath);
43+
DatabaseConnection.initialize(dbPath).close(); // creates inode B at the same path
44+
expect(conn.isFileReplaced()).toBe(true);
45+
46+
// Mid-recreate gap (file absent) must NOT count — don't churn on a transient.
47+
rmDb(dbPath);
48+
expect(conn.isFileReplaced()).toBe(false);
49+
50+
conn.close();
51+
});
52+
});
53+
54+
describe('Reopen on replaced project DB (issue #925)', () => {
55+
let dir: string;
56+
57+
const buildProject = async (fnName: string) => {
58+
fs.mkdirSync(path.join(dir, 'src'), { recursive: true });
59+
fs.writeFileSync(path.join(dir, 'src', 'probe.ts'), `export function ${fnName}() { return 1; }\n`);
60+
const cg = CodeGraph.initSync(dir, { config: { include: ['**/*.ts'], exclude: [] } });
61+
await cg.indexAll();
62+
cg.close();
63+
};
64+
const replaceProjectDb = async (fnName: string) => {
65+
fs.rmSync(path.join(dir, '.codegraph'), { recursive: true, force: true });
66+
await buildProject(fnName);
67+
};
68+
69+
beforeEach(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-db-reopen-')); });
70+
afterEach(() => { fs.rmSync(dir, { recursive: true, force: true }); });
71+
72+
it('CodeGraph.isDbReplaced flips after the DB file is recreated at the same path', async () => {
73+
await buildProject('probeAlpha');
74+
const cg = CodeGraph.openSync(dir);
75+
try {
76+
expect(cg.isDbReplaced()).toBe(false);
77+
await replaceProjectDb('probeBeta'); // new .codegraph/codegraph.db = new inode
78+
expect(cg.isDbReplaced()).toBe(true);
79+
} finally {
80+
cg.close();
81+
}
82+
});
83+
84+
it('ToolHandler.liveCachedGraph evicts + closes a cached project whose DB file was replaced', async () => {
85+
await buildProject('probeAlpha');
86+
const cg = CodeGraph.openSync(dir);
87+
const closeSpy = vi.spyOn(cg, 'close');
88+
const handler = new ToolHandler(null);
89+
// Seed the cross-project cache the way getCodeGraph would (private field).
90+
(handler as unknown as { projectCache: Map<string, CodeGraph> }).projectCache.set(dir, cg);
91+
const live = (k: string) => (handler as unknown as { liveCachedGraph(k: string): CodeGraph | null }).liveCachedGraph(k);
92+
const cached = () => (handler as unknown as { projectCache: Map<string, CodeGraph> }).projectCache;
93+
94+
// Fresh → returned as-is, not evicted/closed.
95+
expect(live(dir)).toBe(cg);
96+
expect(closeSpy).not.toHaveBeenCalled();
97+
98+
// Replace the DB file at the same path → next lookup evicts + closes it,
99+
// returning null so the caller falls through to reopen against the new DB.
100+
await replaceProjectDb('probeBeta');
101+
expect(live(dir)).toBeNull();
102+
expect(cached().has(dir)).toBe(false);
103+
expect(closeSpy).toHaveBeenCalledTimes(1);
104+
});
105+
106+
it('getCodeGraph reopens the DEFAULT project via the reload hook when its DB file was replaced', async () => {
107+
await buildProject('probeAlpha');
108+
const stale = CodeGraph.openSync(dir);
109+
const handler = new ToolHandler(stale);
110+
// Mirror the engine's reload hook: open the fresh DB and install it as default.
111+
let hookCalls = 0;
112+
handler.setDefaultReloadHook(() => {
113+
hookCalls++;
114+
handler.setDefaultCodeGraph(CodeGraph.openSync(dir));
115+
});
116+
const getDefault = () =>
117+
(handler as unknown as { getCodeGraph(p?: string): CodeGraph }).getCodeGraph();
118+
119+
// Fresh default → served as-is on the steady-state (no-projectPath) path; hook not fired.
120+
expect(getDefault()).toBe(stale);
121+
expect(hookCalls).toBe(0);
122+
123+
// Replace the DB at the same path → the default-serving path detects it and
124+
// fires the hook, which installs a fresh default; the call returns the new one.
125+
await replaceProjectDb('probeBeta');
126+
const reopened = getDefault();
127+
expect(hookCalls).toBe(1);
128+
expect(reopened).not.toBe(stale);
129+
expect(reopened.isDbReplaced()).toBe(false); // fresh handle on the new inode
130+
131+
stale.close();
132+
reopened.close();
133+
});
134+
});

src/db/index.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,18 +37,38 @@ function configureConnection(db: SqliteDatabase): void {
3737
db.pragma('mmap_size = 268435456'); // 256 MB memory-mapped I/O
3838
}
3939

40+
/**
41+
* Identity of a DB file as `dev:ino`, or null if it does not exist. Lets a
42+
* long-lived connection notice when the file at its fixed path was swapped for
43+
* a different inode — e.g. the project dir was removed and recreated at the
44+
* same path (`git worktree remove`+`add`, or a fresh `codegraph init`), which
45+
* leaves the open handle pinned to the old, now-unlinked inode. See #925.
46+
*/
47+
function dbFileIdentity(dbPath: string): string | null {
48+
try {
49+
const st = fs.statSync(dbPath);
50+
return `${st.dev}:${st.ino}`;
51+
} catch {
52+
return null;
53+
}
54+
}
55+
4056
/**
4157
* Database connection wrapper with lifecycle management
4258
*/
4359
export class DatabaseConnection {
4460
private db: SqliteDatabase;
4561
private dbPath: string;
4662
private backend: SqliteBackend;
63+
// Identity (`dev:ino`) of the DB file when this connection opened it, so a
64+
// replaced file at the same path (#925) is detectable via isFileReplaced().
65+
private dbFileId: string | null;
4766

4867
private constructor(db: SqliteDatabase, dbPath: string, backend: SqliteBackend) {
4968
this.db = db;
5069
this.dbPath = dbPath;
5170
this.backend = backend;
71+
this.dbFileId = dbFileIdentity(dbPath);
5272
}
5373

5474
/**
@@ -128,6 +148,18 @@ export class DatabaseConnection {
128148
return this.dbPath;
129149
}
130150

151+
/**
152+
* Whether the DB file at this connection's path has been replaced by a
153+
* different inode since we opened it — i.e. the open handle is now pinned to
154+
* a deleted file while reads/writes at the path land on a new one. A missing
155+
* file (mid-recreate) reports false, so a transient gap doesn't churn the
156+
* connection; only a concrete swap to a new inode counts. See #925.
157+
*/
158+
isFileReplaced(): boolean {
159+
const current = dbFileIdentity(this.dbPath);
160+
return current !== null && current !== this.dbFileId;
161+
}
162+
131163
/**
132164
* The journal mode actually in effect (e.g. 'wal', 'delete').
133165
*

src/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,17 @@ export class CodeGraph {
317317
this.db.close();
318318
}
319319

320+
/**
321+
* Whether the underlying DB file was replaced on disk since this instance
322+
* opened it (same path, new inode — e.g. a `git worktree remove`+`add` or a
323+
* fresh `codegraph init` at the same path). Callers that hold a long-lived
324+
* instance use this to reopen instead of serving the stale, now-unlinked
325+
* snapshot. See #925.
326+
*/
327+
isDbReplaced(): boolean {
328+
return this.db.isFileReplaced();
329+
}
330+
320331
/**
321332
* Get the project root directory
322333
*/

src/mcp/engine.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ export class MCPEngine {
5555
constructor(opts: MCPEngineOptions = {}) {
5656
this.opts = { watch: opts.watch ?? true };
5757
this.toolHandler = new ToolHandler(null);
58+
// Reopen the default project's stale handle from the per-tool-call chokepoint
59+
// (ToolHandler.getCodeGraph), not just the cold init paths — a loaded daemon
60+
// never re-enters those, so without this the default-project swap (#925) would
61+
// never be detected at runtime.
62+
this.toolHandler.setDefaultReloadHook(() => this.reloadDefaultIfDbReplaced());
5863
}
5964

6065
/**
@@ -138,6 +143,44 @@ export class MCPEngine {
138143
}
139144
}
140145

146+
/**
147+
* Reopen the default project's CodeGraph if its on-disk DB file was replaced
148+
* (same path, different inode) since we opened it — e.g. the project dir was
149+
* removed and recreated at the same path (`git worktree remove` + `add`, or a
150+
* fresh `codegraph init`). The long-lived SQLite handle would otherwise keep
151+
* reading the old, now-unlinked inode while every `init`/`sync` writes to the
152+
* new one, serving a stale snapshot for the life of the daemon. See #925.
153+
*
154+
* Invoked via the ToolHandler default-reload hook from `getCodeGraph`, i.e. on
155+
* the per-tool-call path that serves the default project — not the cold init
156+
* methods, which a loaded daemon never re-enters. Fully synchronous and run
157+
* between requests, so the close+reopen never races an in-flight query. Opens
158+
* the replacement before closing the stale handle, so a failed reopen leaves
159+
* the (still-functional, if stale) current connection in place rather than
160+
* tearing it down to nothing.
161+
*/
162+
private reloadDefaultIfDbReplaced(): void {
163+
if (this.closed || !this.cg || !this.projectPath || !this.cg.isDbReplaced()) return;
164+
165+
const root = this.projectPath;
166+
let next: CodeGraph;
167+
try {
168+
next = loadCodeGraph().openSync(root);
169+
} catch (err) {
170+
const msg = err instanceof Error ? err.message : String(err);
171+
process.stderr.write(`[CodeGraph MCP] DB for ${root} was replaced but reopening failed: ${msg}\n`);
172+
return;
173+
}
174+
process.stderr.write(`[CodeGraph MCP] Database file for ${root} was replaced (inode changed); reopened.\n`);
175+
const old = this.cg;
176+
this.cg = next;
177+
this.watcherStarted = false;
178+
this.toolHandler.setDefaultCodeGraph(next);
179+
try { old?.close(); } catch { /* ignore */ }
180+
this.startWatching();
181+
this.catchUpSync();
182+
}
183+
141184
/**
142185
* Close everything. Used on graceful daemon shutdown (SIGTERM/idle timeout)
143186
* and on direct-mode stop. Idempotent.

src/mcp/tools.ts

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,11 @@ export class ToolHandler {
681681
// populated by the watcher, not by catch-up. Cleared on first await so
682682
// subsequent calls don't pay any cost.
683683
private catchUpGate: Promise<void> | null = null;
684+
// Hook the MCP engine registers so getCodeGraph can reopen the default
685+
// project when its DB file was replaced on disk (#925). The engine owns the
686+
// default instance's watcher/catch-up, so the reopen is delegated to it.
687+
// Null when the handler has no engine (e.g. unit tests).
688+
private onDefaultStale: (() => void) | null = null;
684689

685690
constructor(private cg: CodeGraph | null) {}
686691

@@ -702,6 +707,14 @@ export class ToolHandler {
702707
this.catchUpGate = p;
703708
}
704709

710+
/**
711+
* Register the engine's hook to reopen the default project if its DB file was
712+
* replaced on disk. Called from getCodeGraph on the default-serving path. See #925.
713+
*/
714+
setDefaultReloadHook(fn: (() => void) | null): void {
715+
this.onDefaultStale = fn;
716+
}
717+
705718
/**
706719
* Record the directory the server tried to resolve the default project from.
707720
* Used only to make the "no default project" error actionable.
@@ -806,6 +819,38 @@ export class ToolHandler {
806819
}
807820
}
808821

822+
/**
823+
* If the default project's DB file was replaced on disk (#925), ask the engine
824+
* (via its registered hook) to reopen it before we serve `this.cg`; the engine
825+
* owns the default instance's watcher/catch-up, so the reopen is delegated.
826+
* After the hook runs, `this.cg` points at the fresh instance. No-op without a
827+
* registered hook or an open default.
828+
*/
829+
private refreshDefaultIfReplaced(): void {
830+
if (this.onDefaultStale && this.cg?.isDbReplaced()) this.onDefaultStale();
831+
}
832+
833+
/**
834+
* Return the cached CodeGraph for `key` if its DB file is still the one it
835+
* opened. If the file was replaced on disk (same path, new inode — a
836+
* `git worktree remove`+`add`, or a fresh `codegraph init`), evict and close
837+
* the stale instance and return null so the caller reopens against the new
838+
* inode instead of serving the now-unlinked snapshot for the daemon's life.
839+
* The default instance is never cached here, so closing a cached one is safe.
840+
* See #925.
841+
*/
842+
private liveCachedGraph(key: string): CodeGraph | null {
843+
const cg = this.projectCache.get(key);
844+
if (!cg) return null;
845+
if (!cg.isDbReplaced()) return cg;
846+
// Drop every key that points at this replaced instance, then close it once.
847+
for (const [k, v] of this.projectCache) {
848+
if (v === cg) this.projectCache.delete(k);
849+
}
850+
try { cg.close(); } catch { /* ignore */ }
851+
return null;
852+
}
853+
809854
/**
810855
* Get CodeGraph instance for a project
811856
*
@@ -817,6 +862,7 @@ export class ToolHandler {
817862
*/
818863
private getCodeGraph(projectPath?: string): CodeGraph {
819864
if (!projectPath) {
865+
this.refreshDefaultIfReplaced();
820866
if (!this.cg) {
821867
const searched = this.defaultProjectHint ?? process.cwd();
822868
throw new NotIndexedError(
@@ -834,10 +880,10 @@ export class ToolHandler {
834880
return this.cg;
835881
}
836882

837-
// Check cache first (using original path as key)
838-
if (this.projectCache.has(projectPath)) {
839-
return this.projectCache.get(projectPath)!;
840-
}
883+
// Check cache first (using original path as key). Reopen instead of serving
884+
// a cached instance whose DB file was replaced on disk under us (#925).
885+
const cached = this.liveCachedGraph(projectPath);
886+
if (cached) return cached;
841887

842888
// Reject sensitive system directories before opening. Only validate a
843889
// path that actually exists — a nested or not-yet-created sub-path of a
@@ -871,15 +917,16 @@ export class ToolHandler {
871917
// not cached under projectPath — the server owns and closes the default
872918
// instance, so routing it through projectCache.closeAll() would double-close it.
873919
if (this.cg && this.cg.getProjectRoot() === resolvedRoot) {
920+
this.refreshDefaultIfReplaced();
874921
return this.cg;
875922
}
876923

877924
// Check if we already have this resolved root cached (different path, same project)
878-
if (this.projectCache.has(resolvedRoot)) {
879-
const cg = this.projectCache.get(resolvedRoot)!;
925+
const hit = this.liveCachedGraph(resolvedRoot);
926+
if (hit) {
880927
// Cache under original path too for faster future lookups
881-
this.projectCache.set(projectPath, cg);
882-
return cg;
928+
this.projectCache.set(projectPath, hit);
929+
return hit;
883930
}
884931

885932
// Open and cache under both paths

0 commit comments

Comments
 (0)