Просмотр исходного кода

fix(mcp): re-resolve a path's index every call so worktree state isn't pinned (#926) (#939)

A long-lived MCP server (the shared daemon) cached both the project handle
(projectCache) and the worktree-mismatch verdict (worktreeMismatchCache) for
its whole lifetime — cleared only on shutdown — keyed off the input path and
never re-checked. So a worktree path first resolved BEFORE it had its own
.codegraph/ — when the walk-up reached the main checkout — stayed pinned to
the main checkout: every query kept hitting the wrong index and every result
carried a false "this index belongs to a different git working tree" warning,
until the server restarted. The CLI was correct (fresh process per run);
re-indexing didn't help.

- getCodeGraph: re-resolve findNearestCodeGraphRoot on every call (cheap stat
  walk, no git, no reopen) and cache the open DB by RESOLVED ROOT only. Drops
  the input-path short-circuit that pinned the first resolution, and the
  double-keying (which also double-closed each instance in closeAll()).
- worktreeMismatchFor: key the verdict on (startPath, indexRoot) so a changed
  index root recomputes instead of serving the stale "borrowed the parent's
  index" verdict.

Adds a regression test that fails pre-fix (the stale warning survives the
index-root flip); the existing "no further git spawn" caching test still
passes. The query-staleness half (a second project opened through the handler)
isn't unit-testable under vitest, so it was validated with a dist A/B.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Colby Mchenry 1 день назад
Родитель
Сommit
62d5cdde2c
3 измененных файлов с 112 добавлено и 26 удалено
  1. 1 0
      CHANGELOG.md
  2. 75 0
      __tests__/worktree-detection.test.ts
  3. 36 26
      src/mcp/tools.ts

+ 1 - 0
CHANGELOG.md

@@ -38,6 +38,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 - `codegraph index` now rebuilds the full graph from scratch, so it produces the same result as a fresh `codegraph init` instead of reporting "0 nodes, 0 edges" and looking like it wiped your index. Previously, re-running `index` on an unchanged project skipped every file (their contents hadn't changed) and showed an empty-looking summary; it now clears and re-indexes for an honest, complete rebuild every time. Use `codegraph sync` for fast incremental updates between full rebuilds. Thanks @Arc-univer. (#874)
 - The file watcher that auto-syncs the graph now fails cleanly when live watching can no longer be trusted, instead of looking healthy while the index quietly goes stale. If the operating system runs out of file-watch resources, or another process holds the write lock far longer than a normal save, CodeGraph now disables auto-sync once — with a single clear message telling you to run `codegraph sync` (or rely on the git sync hooks) to refresh — rather than retrying forever or repeating the same error on a loop. And while auto-sync is disabled, CodeGraph's tool responses (and `codegraph status`) now say so plainly, so your AI agent knows to read files directly instead of trusting a frozen index. This mostly matters for long-running MCP/daemon sessions, which could otherwise keep serving stale results while appearing to work. Thanks @thismilktea. (#876)
 - On Linux, hitting the kernel's inotify watch limit on a large project no longer silently leaves half the tree unwatched. CodeGraph now tells you once — naming the exact setting to raise (`fs.inotify.max_user_watches`, e.g. `sudo sysctl fs.inotify.max_user_watches=1048576`) — and keeps live-watching the directories it could register while `codegraph sync` (or the git sync hooks) covers the rest. (#876)
+- A long-running MCP server now notices when a git worktree gains its own index. Before, if the server (or shared daemon) first saw a worktree before you ran `codegraph init` in it — so the lookup walked up to the main checkout's index — it pinned that decision for its whole life: even after the worktree had its own `.codegraph/`, every query kept hitting the main checkout's index and every result carried a false "this index belongs to a different git working tree" warning, until you restarted the server. The CLI got it right but the MCP server didn't, and re-indexing didn't help. The server now re-checks which index a path belongs to on each call, so the worktree's own index is picked up — and the stale warning drops — without a restart. (#926)
 
 
 ## [1.0.1] - 2026-06-13

+ 75 - 0
__tests__/worktree-detection.test.ts

@@ -187,3 +187,78 @@ describe('worktree mismatch surfaces on hot read tools (issue #155)', () => {
     }
   });
 });
+
+/**
+ * A long-lived MCP server (the shared daemon) cached its worktree-mismatch
+ * verdict keyed only by the start path, and that cache was cleared only on
+ * shutdown. So once the server decided "this worktree borrows the main
+ * checkout's index" — true while the worktree had no `.codegraph/` of its own —
+ * the verdict was pinned for the daemon's whole life. After the worktree got
+ * its own index (the resolved index root flipped from the main checkout to the
+ * worktree itself), the CLI saw the worktree's index but the MCP server kept
+ * emitting the stale false warning until a restart (issue #926).
+ *
+ * The verdict depends on BOTH the start path and the resolved index root, so it
+ * must be cached under both — a changed index root has to invalidate it. This
+ * drives the real `ToolHandler` worktree-notice path across exactly that change
+ * (the resolved index root flips when the server's default project is re-opened
+ * onto the worktree's own index), with no mocking.
+ */
+describe('worktree mismatch verdict re-resolves when the index root changes (issue #926)', () => {
+  let mainRepo: string;
+  let worktree: string;
+  let mainCg: CodeGraph;
+  let worktreeCg: CodeGraph;
+  let handler: ToolHandler;
+
+  beforeEach(async () => {
+    mainRepo = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-wt-926-'));
+    git(mainRepo, 'init', '-q');
+    git(mainRepo, 'config', 'user.email', 'test@example.com');
+    git(mainRepo, 'config', 'user.name', 'Test');
+    git(mainRepo, 'config', 'commit.gpgsign', 'false');
+    fs.mkdirSync(path.join(mainRepo, 'src'));
+    fs.writeFileSync(path.join(mainRepo, 'src', 'a.ts'), 'export function mainOnly() { return 1; }\n');
+    git(mainRepo, 'add', '.');
+    git(mainRepo, 'commit', '-q', '-m', 'init');
+
+    // The long-lived server's default project starts as the MAIN checkout.
+    mainCg = CodeGraph.initSync(mainRepo);
+    await mainCg.indexAll();
+
+    // Nested worktree that later gains its own index.
+    worktree = path.join(mainRepo, 'wt');
+    git(mainRepo, 'worktree', 'add', '-q', '-b', 'feature', worktree);
+    worktreeCg = CodeGraph.initSync(worktree);
+    await worktreeCg.indexAll();
+
+    handler = new ToolHandler(mainCg);
+  });
+
+  afterEach(() => {
+    try { mainCg.destroy(); } catch { /* best effort */ }
+    try { worktreeCg.destroy(); } catch { /* best effort */ }
+    try { git(mainRepo, 'worktree', 'remove', '--force', worktree); } catch { /* best effort */ }
+    fs.rmSync(mainRepo, { recursive: true, force: true });
+  });
+
+  it('drops the stale "borrowed the main index" warning once the index root flips to the worktree', async () => {
+    // The server runs from inside the worktree, default project = main checkout.
+    handler.setDefaultProjectHint(worktree);
+
+    // Phase 1: the index genuinely belongs to a different working tree (the main
+    // checkout) → warn, and cache that verdict.
+    const before = await handler.execute('codegraph_status', {});
+    expect(before.content[0].text).toContain('different git working tree');
+    expect(before.content[0].text).toContain(real(mainRepo));
+
+    // Phase 2: the worktree's own index is now the server's default project
+    // (engine re-open → setDefaultCodeGraph). The resolved index root for the
+    // SAME start path flipped to the worktree itself, so the verdict must be
+    // recomputed to "no mismatch" — not served stale from before.
+    handler.setDefaultCodeGraph(worktreeCg);
+
+    const after = await handler.execute('codegraph_status', {});
+    expect(after.content[0].text).not.toContain('different git working tree');
+  });
+});

+ 36 - 26
src/mcp/tools.ts

@@ -823,11 +823,6 @@ export class ToolHandler {
       return this.cg;
     }
 
-    // Check cache first (using original path as key)
-    if (this.projectCache.has(projectPath)) {
-      return this.projectCache.get(projectPath)!;
-    }
-
     // Reject sensitive system directories before opening. Only validate a
     // path that actually exists — a nested or not-yet-created sub-path of a
     // real project must still be allowed to resolve UP to its .codegraph/
@@ -840,7 +835,16 @@ export class ToolHandler {
       }
     }
 
-    // Walk up parent directories to find nearest .codegraph/
+    // Always RE-RESOLVE the nearest .codegraph/ from the input path. The walk
+    // is cheap (a few existsSync up the tree) and is the only thing that
+    // notices a path whose index root CHANGED since it was first seen — most
+    // importantly a git worktree that gained its own .codegraph/ after the
+    // (long-lived) server first resolved it up to the parent checkout. We used
+    // to short-circuit on a `projectCache[projectPath]` entry before resolving,
+    // which pinned that first resolution for the server's whole lifetime, so a
+    // worktree kept being served the parent checkout's index until restart
+    // (#926). The DB connection itself is still cached (by resolved root,
+    // below), so re-resolving costs only the stat walk, never a reopen.
     const resolvedRoot = findNearestCodeGraphRoot(projectPath);
 
     if (!resolvedRoot) {
@@ -856,27 +860,20 @@ export class ToolHandler {
     // default instance rather than opening a SECOND connection to the same DB.
     // A duplicate connection serializes reads against the watcher's auto-sync
     // writes; on the wasm backend (no WAL) that surfaces as intermittent
-    // "database is locked" on concurrent tool calls. See issue #238. Deliberately
-    // not cached under projectPath — the server owns and closes the default
-    // instance, so routing it through projectCache.closeAll() would double-close it.
+    // "database is locked" on concurrent tool calls. See issue #238. The
+    // default instance is owned/closed by the server, so it's never cached.
     if (this.cg && this.cg.getProjectRoot() === resolvedRoot) {
       return this.cg;
     }
 
-    // Check if we already have this resolved root cached (different path, same project)
-    if (this.projectCache.has(resolvedRoot)) {
-      const cg = this.projectCache.get(resolvedRoot)!;
-      // Cache under original path too for faster future lookups
-      this.projectCache.set(projectPath, cg);
-      return cg;
-    }
+    // Cache the open DB connection by RESOLVED ROOT only — never by the input
+    // path. One key per instance means closeAll() closes each exactly once, and
+    // a changed resolution maps to a different entry instead of a stale hit.
+    const cached = this.projectCache.get(resolvedRoot);
+    if (cached) return cached;
 
-    // Open and cache under both paths
     const cg = loadCodeGraph().openSync(resolvedRoot);
     this.projectCache.set(resolvedRoot, cg);
-    if (projectPath !== resolvedRoot) {
-      this.projectCache.set(projectPath, cg);
-    }
     return cg;
   }
 
@@ -947,17 +944,30 @@ export class ToolHandler {
    */
   private worktreeMismatchFor(projectPath?: string): WorktreeIndexMismatch | null {
     const startPath = projectPath ?? this.defaultProjectHint ?? process.cwd();
-    const cached = this.worktreeMismatchCache.get(startPath);
-    if (cached !== undefined) return cached;
 
-    let mismatch: WorktreeIndexMismatch | null = null;
+    // The verdict depends on BOTH the start path AND the index root it resolves
+    // to, so the cache must be keyed on the pair. Resolve the index root first
+    // (cheap — getCodeGraph re-walks to the nearest .codegraph/, no git), then
+    // key on `(startPath, indexRoot)`. The moment that root changes — most
+    // importantly when a git worktree gains its own index and the walk-up stops
+    // there instead of at the parent checkout — the key changes and the verdict
+    // is recomputed, instead of serving the stale "borrowed the parent's index"
+    // warning for the server's whole lifetime. Keying on startPath alone pinned
+    // that first verdict until restart (#926).
+    let indexRoot: string;
     try {
-      mismatch = detectWorktreeIndexMismatch(startPath, this.getCodeGraph(projectPath).getProjectRoot());
+      indexRoot = this.getCodeGraph(projectPath).getProjectRoot();
     } catch {
       // No resolvable project (or any other resolution error) → nothing to warn.
-      mismatch = null;
+      return null;
     }
-    this.worktreeMismatchCache.set(startPath, mismatch);
+
+    const cacheKey = `${startPath}\u0000${indexRoot}`;
+    const cached = this.worktreeMismatchCache.get(cacheKey);
+    if (cached !== undefined) return cached;
+
+    const mismatch = detectWorktreeIndexMismatch(startPath, indexRoot);
+    this.worktreeMismatchCache.set(cacheKey, mismatch);
     return mismatch;
   }