From fe8252d2ba621c66b8b4ce25587a3c68606442f8 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Wed, 16 Sep 2026 10:52:41 +0300 Subject: [PATCH] fix(mcp): keep an explicit projectPath project in sync (#1835) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A project opened through a tool call's `projectPath` (a repository other than the server's default — e.g. an indexed child of an un-indexed workspace) was opened read-only: no catch-up sync on open and no file watcher, so its answers went stale until someone ran `codegraph sync`. The engine now owns the lifecycle of every project the ToolHandler opens for an explicit path, mirroring the default project: - the project's writer lock is acquired (#1740 single-writer rule); if another live process holds it, that process keeps syncing and we only read; - a catch-up `sync()` runs on open and the first call against that project awaits it, time-boxed like the default gate (#905); - a file watcher runs for as long as the project stays cached. Bounds: the cache is keyed by the canonical (realpath) root, so a symlinked spelling shares one connection and one watcher; it is LRU with `MAX_CACHED_PROJECTS = 8` — opening a ninth closes the least recently used (watcher stopped, writer lock released, DB closed); `stop()` closes them all. Un-indexed paths still get the success-shaped guidance. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 2 + __tests__/mcp-projectpath-lifecycle.test.ts | 127 +++++++++++++++++ src/mcp/engine.ts | 124 ++++++++++++----- src/mcp/tools.ts | 143 +++++++++++++++++--- 4 files changed, 348 insertions(+), 48 deletions(-) create mode 100644 __tests__/mcp-projectpath-lifecycle.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cc33b5613..997bcc09ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,8 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- Projects queried through `projectPath` (a repository other than the server's default, such as an indexed child of an un-indexed workspace) now stay in sync like the default project does: the first call catches up edits made while no server was running, and a file watcher keeps the index current for as long as the project stays open in that session. Up to 8 such projects are kept open per server, the least recently used one is closed when a ninth is opened, and all are closed when the server stops. A project another live server already syncs is left to that server. (#1835) + - Rust calls on `self` now stay with the enclosing type instead of linking to an unrelated type’s same-named method. Thanks @L4XB. (#1861) - Turning telemetry off now resets its identity and stops running processes from recording, sending, or restoring unsent data. (#1869) diff --git a/__tests__/mcp-projectpath-lifecycle.test.ts b/__tests__/mcp-projectpath-lifecycle.test.ts new file mode 100644 index 0000000000..dd702aef65 --- /dev/null +++ b/__tests__/mcp-projectpath-lifecycle.test.ts @@ -0,0 +1,127 @@ +/** + * Explicit-`projectPath` project lifecycle (#1835). + * + * A server whose root has no index of its own (a workspace whose indexed + * children are gitignored) serves each child through `projectPath`. Before + * this fix those projects were opened read-only: no catch-up sync on open and + * no file watcher, so their answers went stale until someone ran + * `codegraph sync` by hand. Now the engine gives an explicit project the same + * lifecycle the default project gets — a catch-up sync the first call waits + * for, a watcher while it stays cached — bounded (LRU) and released on stop(). + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import CodeGraph from '../src/index'; +import { MCPEngine } from '../src/mcp/engine'; +import { __setLoadCodeGraphForTests } from '../src/mcp/tools'; + +const opened: CodeGraph[] = []; +/** CodeGraph that records every instance the ToolHandler opens. */ +class RecordingCodeGraph extends CodeGraph { + static openSync(projectRoot: string): CodeGraph { + const cg = CodeGraph.openSync(projectRoot); + opened.push(cg); + return cg; + } +} + +async function makeProject(dir: string, symbol: string): Promise { + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'src', 'sample.ts'), `export function ${symbol}() { return 1; }\n`); + const cg = await CodeGraph.init(dir, { config: { include: ['**/*.ts'], exclude: [] } }); + await cg.indexAll(); + cg.close(); +} + +async function waitFor(check: () => Promise, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (await check()) return true; + await new Promise((r) => setTimeout(r, 100)); + } + return check(); +} + +describe('MCP explicit projectPath lifecycle (#1835)', () => { + let workspace: string; + let serviceA: string; + let serviceB: string; + let engine: MCPEngine; + const prevDebounce = process.env.CODEGRAPH_WATCH_DEBOUNCE_MS; + + beforeEach(async () => { + workspace = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-1835-'))); + serviceA = path.join(workspace, 'service-a'); + serviceB = path.join(workspace, 'service-b'); + await makeProject(serviceA, 'alphaOriginal'); + await makeProject(serviceB, 'betaOriginal'); + process.env.CODEGRAPH_WATCH_DEBOUNCE_MS = '100'; + opened.length = 0; + __setLoadCodeGraphForTests(RecordingCodeGraph as unknown as typeof CodeGraph); + engine = new MCPEngine({ watch: true }); + // Two indexed children, none at the root: no default project (#1607). + await engine.ensureInitialized(workspace); + }); + + afterEach(() => { + engine.stop(); + __setLoadCodeGraphForTests(null); + if (prevDebounce === undefined) delete process.env.CODEGRAPH_WATCH_DEBOUNCE_MS; + else process.env.CODEGRAPH_WATCH_DEBOUNCE_MS = prevDebounce; + fs.rmSync(workspace, { recursive: true, force: true }); + }); + + async function search(projectPath: string, symbol: string): Promise { + const res = await engine.getToolHandler().execute('codegraph_search', { query: symbol, projectPath }); + expect(res.isError).toBeFalsy(); + return res.content.map((c) => (c.type === 'text' ? c.text : '')).join('\n'); + } + + it('catches up an edit made before the first call and watches later edits', async () => { + // Edited while no server owned the index — the catch-up path. + fs.writeFileSync(path.join(serviceA, 'src', 'sample.ts'), 'export function alphaRenamed() { return 1; }\n'); + const first = await search(serviceA, 'alphaRenamed'); + expect(first).toContain('alphaRenamed'); + expect(first).not.toContain('alphaOriginal'); + expect(opened).toHaveLength(1); + expect(opened[0].isWatching()).toBe(true); + await opened[0].waitUntilWatcherReady(5000); + + // Edited while the project stays cached — the watcher path. + fs.writeFileSync(path.join(serviceA, 'src', 'sample.ts'), 'export function alphaWatched() { return 1; }\n'); + const seen = await waitFor(async () => (await search(serviceA, 'alphaWatched')).includes('alphaWatched'), 10000); + expect(seen).toBe(true); + }); + + it('keeps one watched instance per canonical root and closes it on stop()', async () => { + const link = path.join(workspace, 'link-to-b'); + fs.symlinkSync(serviceB, link, 'dir'); + expect(await search(serviceB, 'betaOriginal')).toContain('betaOriginal'); + expect(await search(link, 'betaOriginal')).toContain('betaOriginal'); + expect(await search(path.join(serviceB, 'src'), 'betaOriginal')).toContain('betaOriginal'); + expect(opened).toHaveLength(1); + expect(opened[0].isWatching()).toBe(true); + expect(fs.existsSync(path.join(serviceB, '.codegraph', 'writer.pid'))).toBe(true); + + engine.stop(); + expect(opened[0].isWatching()).toBe(false); + expect(fs.existsSync(path.join(serviceB, '.codegraph', 'writer.pid'))).toBe(false); + expect(() => opened[0].getStats()).toThrow(); + }); + + it('does not take over a project another live process is already syncing', async () => { + // Simulate a foreign writer (another daemon) holding the lock. + fs.mkdirSync(path.join(serviceB, '.codegraph'), { recursive: true }); + const foreign = { pid: process.ppid, mode: 'daemon', startedAt: Date.now() }; + fs.writeFileSync(path.join(serviceB, '.codegraph', 'writer.pid'), JSON.stringify(foreign)); + expect(await search(serviceB, 'betaOriginal')).toContain('betaOriginal'); + expect(opened).toHaveLength(1); + expect(opened[0].isWatching()).toBe(false); + engine.stop(); + // Not ours — left in place. + expect(fs.readFileSync(path.join(serviceB, '.codegraph', 'writer.pid'), 'utf8')).toContain(String(process.ppid)); + fs.unlinkSync(path.join(serviceB, '.codegraph', 'writer.pid')); + }); +}); diff --git a/src/mcp/engine.ts b/src/mcp/engine.ts index b88f1b1215..21127de19c 100644 --- a/src/mcp/engine.ts +++ b/src/mcp/engine.ts @@ -76,6 +76,9 @@ export class MCPEngine { private watcherStarted = false; /** Set when this engine holds writer.pid (#1740). */ private writerLockRoot: string | null = null; + // Roots of explicit-`projectPath` projects whose writer.pid this engine holds + // (#1835) — released when the ToolHandler closes the project or on stop(). + private explicitWriterLocks: Set = new Set(); private opts: Required>; private closed = false; // Off-loop read-tool pool (daemon mode only). Created lazily once the default @@ -85,6 +88,10 @@ export class MCPEngine { constructor(opts: MCPEngineOptions = {}) { this.opts = { watch: opts.watch ?? true, queryPool: opts.queryPool ?? false }; this.toolHandler = new ToolHandler(null); + this.toolHandler.setProjectLifecycle({ + activate: (cg) => this.activateExplicitProject(cg), + release: (cg) => this.releaseExplicitProject(cg), + }); if (opts.writerLockRoot) { const writer = tryAcquireWriterLock(opts.writerLockRoot, 'fallback'); if (writer.kind === 'taken') { @@ -231,12 +238,97 @@ export class MCPEngine { this.queryPool = null; } this.toolHandler.closeAll(); + for (const root of this.explicitWriterLocks) releaseWriterLock(root); + this.explicitWriterLocks.clear(); if (this.cg) { try { this.cg.close(); } catch { /* ignore */ } this.cg = null; } } + /** + * Give a project opened for an explicit `projectPath` the default project's + * lifecycle (#1835): a file watcher while the ToolHandler keeps it cached and + * a catch-up sync now, whose promise the handler awaits before the first call + * against it. Only when this engine wins the project's writer lock — if another + * live process (its own daemon, say) holds it, that process already syncs the + * index and we must not contend for codegraph.lock (#1740). Never throws. + */ + private activateExplicitProject(cg: CodeGraph): Promise { + if (this.closed || !this.opts.watch) return Promise.resolve(); + const root = cg.getProjectRoot(); + const writer = tryAcquireWriterLock(root, 'fallback'); + if (writer.kind === 'taken') { + process.stderr.write( + `[CodeGraph MCP] Not syncing ${root} from this session — ${writerLockHeldMessage(writer.existing, writer.pidPath)}\n` + ); + return Promise.resolve(); + } + this.explicitWriterLocks.add(root); + + const disabledReason = watchDisabledReason(root); + if (disabledReason) { + process.stderr.write(`[CodeGraph MCP] File watcher disabled for ${root} — ${disabledReason}.\n`); + } else if (cg.watch(this.watchOptions())) { + process.stderr.write(`[CodeGraph MCP] File watcher active for ${root} (opened via projectPath)\n`); + } + + return cg + .sync() + .then((result) => { + const changed = result.filesAdded + result.filesModified + result.filesRemoved; + if (changed > 0) { + process.stderr.write(`[CodeGraph MCP] Caught up ${changed} file(s) changed in ${root}\n`); + } + }) + .catch((err) => { + const msg = err instanceof Error ? err.message : String(err); + process.stderr.write(`[CodeGraph MCP] Catch-up sync failed for ${root}: ${msg}\n`); + }); + } + + /** Drop the writer lock of an explicit project the ToolHandler is closing (#1835). */ + private releaseExplicitProject(cg: CodeGraph): void { + const root = cg.getProjectRoot(); + if (!this.explicitWriterLocks.delete(root)) return; + releaseWriterLock(root); + } + + /** Watch options shared by the default project and explicit projects. */ + private watchOptions(): Parameters[0] { + // Optional override for the debounce window via env var (issue #403). + // Useful for workspaces with bursty writes (formatter-on-save chains, + // large generated outputs) where the 2s default fires too often. Clamped + // to [100ms, 60s]; out-of-range / non-numeric values fall back to the + // FileWatcher default. We log the active value so it's discoverable. + const debounceMs = parseDebounceEnv(process.env.CODEGRAPH_WATCH_DEBOUNCE_MS); + if (debounceMs !== undefined) { + process.stderr.write(`[CodeGraph MCP] File watcher debounce: ${debounceMs}ms (CODEGRAPH_WATCH_DEBOUNCE_MS)\n`); + } + return { + debounceMs, + onSyncComplete: (result) => { + if (result.filesChanged > 0) { + process.stderr.write( + `[CodeGraph MCP] Auto-synced ${result.filesChanged} file(s) in ${result.durationMs}ms\n` + ); + } + }, + onSyncError: (err) => { + process.stderr.write(`[CodeGraph MCP] Auto-sync error: ${err.message}\n`); + }, + onDegraded: (reason) => { + // Live watching gave up permanently (watch-resource exhaustion or a + // write lock held past the retry budget). Say so loudly and ONCE — the + // graph will no longer auto-update, so a long-running MCP session must + // not keep assuming it's fresh. The reason already names the remedy + // (`codegraph sync` / git sync hooks). + process.stderr.write(`[CodeGraph MCP] File watcher degraded — ${reason}\n`); + }, + }; + } + + private async doInitialize(searchFrom: string): Promise { this.toolHandler.setDefaultProjectHint(searchFrom); @@ -326,37 +418,7 @@ export class MCPEngine { return; } - // Optional override for the debounce window via env var (issue #403). - // Useful for workspaces with bursty writes (formatter-on-save chains, - // large generated outputs) where the 2s default fires too often. Clamped - // to [100ms, 60s]; out-of-range / non-numeric values fall back to the - // FileWatcher default. We log the active value so it's discoverable. - const debounceMs = parseDebounceEnv(process.env.CODEGRAPH_WATCH_DEBOUNCE_MS); - if (debounceMs !== undefined) { - process.stderr.write(`[CodeGraph MCP] File watcher debounce: ${debounceMs}ms (CODEGRAPH_WATCH_DEBOUNCE_MS)\n`); - } - - const started = this.cg.watch({ - debounceMs, - onSyncComplete: (result) => { - if (result.filesChanged > 0) { - process.stderr.write( - `[CodeGraph MCP] Auto-synced ${result.filesChanged} file(s) in ${result.durationMs}ms\n` - ); - } - }, - onSyncError: (err) => { - process.stderr.write(`[CodeGraph MCP] Auto-sync error: ${err.message}\n`); - }, - onDegraded: (reason) => { - // Live watching gave up permanently (watch-resource exhaustion or a - // write lock held past the retry budget). Say so loudly and ONCE — the - // graph will no longer auto-update, so a long-running MCP session must - // not keep assuming it's fresh. The reason already names the remedy - // (`codegraph sync` / git sync hooks). - process.stderr.write(`[CodeGraph MCP] File watcher degraded — ${reason}\n`); - }, - }); + const started = this.cg.watch(this.watchOptions()); this.watcherStarted = true; if (started) { diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 42fdd22884..c1293b5077 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -37,6 +37,7 @@ import { extractQueryPaths, queryMightContainPaths } from '../search/query-paths import { existsSync, readFileSync, + realpathSync, statSync, } from 'fs'; import { createHash } from 'crypto'; @@ -1462,9 +1463,48 @@ const DEFAULT_MCP_TOOLS = new Set(['explore']); * Supports cross-project queries via the projectPath parameter. * Other projects are opened on-demand and cached for performance. */ +/** realpath when the path exists, the path itself otherwise — never throws. */ +function canonicalPath(p: string): string { + try { + return realpathSync(p); + } catch { + return p; + } +} + +/** + * How many explicit-`projectPath` projects a handler keeps open at once + * (#1835). Each cached project may hold a file watcher, a writer lock and a + * SQLite connection, so the cache is bounded LRU: opening one more than this + * closes the least recently used. Small on purpose — a session that queries + * many repositories still leaks nothing; it only pays a reopen + catch-up. + */ +export const MAX_CACHED_PROJECTS = 8; + +/** + * Engine-side lifecycle for a project the ToolHandler opened for an explicit + * `projectPath` (#1835). `activate` gives it the same treatment the default + * project gets — a file watcher while it stays open and a catch-up sync — and + * returns the catch-up promise, which the handler awaits (time-boxed) before + * the first call against that project. `release` runs when the handler closes + * the project (LRU eviction or shutdown) so the engine can drop its writer lock. + */ +export interface ProjectLifecycle { + activate(cg: CodeGraph): Promise; + release(cg: CodeGraph): void; +} + export class ToolHandler { - // Cache of opened CodeGraph instances for cross-project queries + // Cache of opened CodeGraph instances for cross-project queries, keyed by the + // CANONICAL (realpath) index root. Map insertion order doubles as LRU order: + // a hit re-inserts, and `MAX_CACHED_PROJECTS` bounds the size (#1835). private projectCache: Map = new Map(); + // Engine hook that watches + catches up an explicit project (null for the + // CLI and worker-thread handlers, which never own a watcher). + private projectLifecycle: ProjectLifecycle | null = null; + // Per explicit project: its catch-up sync, awaited once by the first call + // that targets it (same time-box as the default project's gate). + private projectGates: Map> = new Map(); // The directory the server last searched for a default project. Surfaced in // the "not initialized" error so users can see why detection missed. private defaultProjectHint: string | null = null; @@ -1509,6 +1549,14 @@ export class ToolHandler { this.queryPool = pool; } + /** + * Engine-only: own the lifecycle (watcher, catch-up, writer lock) of every + * project this handler opens for an explicit `projectPath` (#1835). + */ + setProjectLifecycle(lifecycle: ProjectLifecycle | null): void { + this.projectLifecycle = lifecycle; + } + /** * Update the default CodeGraph instance (e.g. after lazy initialization) */ @@ -1754,8 +1802,13 @@ export class ToolHandler { // (#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); + // Canonicalize for the identity checks below only: two spellings of one + // root (a symlinked checkout, `/tmp` vs `/private/tmp`) must share one + // connection and one watcher (#1835). The instance itself is still opened + // at the root as found, so nothing user-visible changes spelling. + const canonicalRoot = resolvedRoot ? canonicalPath(resolvedRoot) : null; - if (!resolvedRoot) { + if (!resolvedRoot || !canonicalRoot) { throw new NotIndexedError( `The project at ${projectPath} isn't indexed with codegraph (no .codegraph/ directory found ` + 'walking up from it), so codegraph cannot query it. Use your built-in tools (Read/Grep/Glob) ' + @@ -1771,21 +1824,71 @@ export class ToolHandler { // support) that surfaces as intermittent // "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) { + if (this.cg && canonicalPath(this.cg.getProjectRoot()) === canonicalRoot) { return this.freshen(this.cg); } - // Cache the open DB connection by RESOLVED ROOT only — never by the input + // Cache the open DB connection by CANONICAL 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 this.freshen(cached); + const cached = this.projectCache.get(canonicalRoot); + if (cached) { + // Refresh LRU position. + this.projectCache.delete(canonicalRoot); + this.projectCache.set(canonicalRoot, cached); + return this.freshen(cached); + } const cg = loadCodeGraph().openSync(resolvedRoot); - this.projectCache.set(resolvedRoot, cg); + this.projectCache.set(canonicalRoot, cg); + // Bounded: evict the least recently used explicit project (#1835). + while (this.projectCache.size > MAX_CACHED_PROJECTS) { + const [oldestKey, oldest] = this.projectCache.entries().next().value as [string, CodeGraph]; + this.projectCache.delete(oldestKey); + this.closeProject(oldest); + } + // Same lifecycle as the default project: watcher + catch-up sync, owned + // by the engine (#1835). The catch-up promise is the project's gate. + if (this.projectLifecycle) { + let gate: Promise; + try { + gate = this.projectLifecycle.activate(cg); + } catch (err) { + gate = Promise.reject(err); + } + gate = gate.catch(() => { /* logged by the engine; serve best-effort */ }); + this.projectGates.set(cg, gate); + } return cg; } + /** + * Before dispatching a call that names an explicit `projectPath`, wait + * (time-boxed, once) for that project's catch-up sync — the same guarantee the + * default project's gate gives the first call of a session (#1835). Resolution + * errors are left to the dispatch path, which already answers them with the + * success-shaped guidance. + */ + private async awaitProjectGate(projectPath: string): Promise { + let cg: CodeGraph; + try { + cg = this.getCodeGraph(projectPath); + } catch { + return; + } + const gate = this.projectGates.get(cg); + if (!gate) return; + this.projectGates.delete(cg); + await this.awaitCatchUpGate(gate); + } + + /** Close one explicit project: watcher, writer lock (via the engine), DB. */ + private closeProject(cg: CodeGraph): void { + this.projectGates.delete(cg); + try { this.projectLifecycle?.release(cg); } catch { /* best-effort */ } + try { cg.close(); } catch { /* best-effort */ } + } + /** * Heal a long-lived connection whose `.codegraph/` was removed and recreated * at the same path (a worktree recreated, or `rm -rf .codegraph` + re-init) @@ -1816,9 +1919,10 @@ export class ToolHandler { */ closeAll(): void { for (const cg of this.projectCache.values()) { - cg.close(); + this.closeProject(cg); } this.projectCache.clear(); + this.projectGates.clear(); this.worktreeMismatchCache.clear(); } @@ -2010,13 +2114,11 @@ export class ToolHandler { return result; // no default project — leave as is } - // Cross-project `projectPath` calls open a cached CodeGraph WITHOUT a - // watcher (watchers are only attached to the default session project). + // A cross-project `projectPath` call's cached CodeGraph only has a watcher + // when the engine owns its lifecycle (#1835) — the CLI's handler has none. // When the cross-project path happens to be the same project as the - // default cg, the cached instance is the wrong one — its pendingFiles is - // permanently empty. Detect the equal-path case and prefer the default - // cg so the staleness signal still fires when an agent passes the - // explicit projectPath form of its own project. + // default cg, prefer the default cg so the staleness signal still fires + // when an agent passes the explicit projectPath form of its own project. if (this.cg && cg !== this.cg) { try { const sameProject = @@ -2031,9 +2133,9 @@ export class ToolHandler { // stopped, getPendingFiles() is empty so the per-file banner below can't // fire — but the index is now FROZEN and silently drifting stale. Surface // one global notice instead, so the agent Reads for current content rather - // than trusting a response off a no-longer-updating index. (Cross-project - // calls open a watcher-less CodeGraph, so this is false there — correct: we - // only know degraded state for the default session project.) + // than trusting a response off a no-longer-updating index. (A cross-project + // instance without an engine-owned watcher reports false here — correct: we + // only know degraded state for a project we watch.) let degraded = false; try { degraded = cg.isWatcherDegraded?.() ?? false; @@ -2132,6 +2234,13 @@ export class ToolHandler { if (typeof pathCheck === 'object' && pathCheck !== undefined) { return pathCheck; } + // An explicit project gets the same first-call guarantee as the default + // (#1835): its post-open catch-up sync finishes (time-boxed) before we + // serve it. Resolved on the main thread so the watcher lives here even + // when dispatch is off-loaded to a worker. + if (typeof pathCheck === 'string') { + await this.awaitProjectGate(pathCheck); + } // The `path` and `pattern` properties used by codegraph_files are // also path-shaped — apply the same cap. if (args.path !== undefined) {