From 092d49d3dbe1e6e43d416e73ee28aa70e71bec78 Mon Sep 17 00:00:00 2001 From: danusha2345 Date: Tue, 29 Sep 2026 13:12:40 +0300 Subject: [PATCH] fix(mcp): release an idle explicit project and its writer lock (#2087) A daemon that answered a `projectPath` query for another indexed project opened it in-process and, with no owner there, took its writer lock. The project cache only evicted over its LRU bound or on shutdown, so the lock stayed held for the daemon's whole life: the project's own daemon could not become its writer and `codegraph index` there was refused. Release a cached explicit project once it has gone unused for 10 minutes (CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS; 0 never releases), through the same trim path as LRU eviction, so an active call or a running catch-up still defers it. The next call reopens the project and catches it up (#1835). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/mcp-projectpath-lifecycle.test.ts | 55 +++++++++++++++++++ src/mcp/tools.ts | 58 ++++++++++++++++++++- 3 files changed, 112 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a85a4d0b8c..229936092e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - A Swift reference to a type now links to the type's own declaration, not to a file that extends it. An `extension View { … }` or `extension Text { … }` used to stand in for SwiftUI's type, so every view, every `Text("…")` and every `Color.red` in an app linked to whichever file happened to extend it. Those files topped the most-depended-on lists and their impact reached the whole app. A bare name like `@State`, `@Test` or `Result<…>` no longer links to some other type's nested `State` or `Result`, and a qualified name like `Build.Id` links to the `Id` it names. Methods declared in an extension of an SDK type still resolve on the types that conform to it, and a protocol's methods declared in any of its extensions now connect to each conforming type's own implementation. Re-index Swift projects after upgrading. - A Vapor route now links to the handler it names. `use: SearchController.show` used to link to whichever controller's `show` came first, so routes with a common handler name, like `show`, `index` or `get`, pointed at another endpoint's code in callers, impact and `codegraph_explore` answers. Nested types like `API.PackageController.get` and handlers declared in an extension of the controller now resolve too, and `use: self.index` resolves to the collection's own `index`. Re-index Vapor projects after upgrading. - A PHP call written without a receiver, such as `redirect($url)`, `view('books.show')`, `auth()` or `basename($path)`, is a function call, and no longer links to a same-named method, field or class elsewhere in the project. These wrong links showed up in callers, impact and `codegraph_explore` answers wherever a Laravel helper or PHP built-in shared its name with a project member. Re-index PHP projects after upgrading. +- A CodeGraph session that queried another project through `projectPath` no longer keeps that project locked for as long as it runs: after 10 minutes without a query it lets the project go, so the project's own session and `codegraph index` can take over again (tune with `CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS`, `0` keeps it open). (#2087) ## [1.6.1] - 2026-09-29 diff --git a/__tests__/mcp-projectpath-lifecycle.test.ts b/__tests__/mcp-projectpath-lifecycle.test.ts index 23a1a36a95..10ea19abc6 100644 --- a/__tests__/mcp-projectpath-lifecycle.test.ts +++ b/__tests__/mcp-projectpath-lifecycle.test.ts @@ -442,6 +442,61 @@ describe('MCP explicit projectPath lifecycle (#1835)', { timeout: 30_000 }, () = }, 60_000); + // A daemon that answered one projectPath query for another project must not + // keep that project's writer lock until it exits: the project's own daemon + // and `codegraph index` there would stay locked out (#2087). + it('releases an idle explicit project and its writer lock, and retakes it on the next call', async () => { + const prev = process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS; + process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS = '200'; + const lock = path.join(serviceB, '.codegraph/writer.pid'); + try { + expect(await search(serviceB, 'betaOriginal')).toContain('betaOriginal'); + expect(JSON.parse(fs.readFileSync(lock, 'utf8')).pid).toBe(process.pid); + expect(opened[0]!.isWatching()).toBe(true); + + expect(await waitFor(async () => !fs.existsSync(lock), 10000)).toBe(true); + expect(() => opened[0]!.getStats()).toThrow(); + + // Still synchronized while in use: the next call reopens and catches up. + fs.writeFileSync(path.join(serviceB, 'src/sample.ts'), 'export function afterIdleRelease() {}\n'); + expect(await search(serviceB, 'afterIdleRelease')).toContain('afterIdleRelease'); + expect(opened).toHaveLength(2); + expect(opened[1]!.isWatching()).toBe(true); + expect(JSON.parse(fs.readFileSync(lock, 'utf8')).pid).toBe(process.pid); + } finally { + if (prev === undefined) delete process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS; + else process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS = prev; + } + }); + + it('defers an idle release until an active catch-up finishes (#2087)', async () => { + let release!: () => void; + const held = new Promise((resolve) => { release = resolve; }); + onOpen = (cg) => { + const sync = cg.sync.bind(cg); + vi.spyOn(cg, 'sync').mockImplementation(async (...args) => { await held; return sync(...args); }); + }; + const prevIdle = process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS; + process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS = '50'; + process.env.CODEGRAPH_CATCHUP_GATE_TIMEOUT_MS = '10'; + const lock = path.join(serviceB, '.codegraph/writer.pid'); + try { + fs.writeFileSync(path.join(serviceB, 'src/sample.ts'), 'export function idleCatchUp() {}\n'); + await search(serviceB, 'betaOriginal'); + await new Promise((r) => setTimeout(r, 300)); + expect(() => opened[0]!.getStats()).not.toThrow(); + expect(fs.existsSync(lock)).toBe(true); + release(); + expect(await waitFor(async () => !fs.existsSync(lock), 10000)).toBe(true); + expect(names(serviceB)).toEqual(['idleCatchUp']); + expect(() => opened[0]!.getStats()).toThrow(); + } finally { + release(); + if (prevIdle === undefined) delete process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS; + else process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS = prevIdle; + } + }); + it('drains a tool operation before closing its cached graph', async () => { await search(serviceA, 'alphaOriginal'); const handler = engine.getToolHandler(); diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 7553ca5a65..d11fa40c5f 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -1867,6 +1867,23 @@ function canonicalPath(p: string): string { */ export const MAX_CACHED_PROJECTS = 8; +/** + * How long an explicit-`projectPath` project may go unused before the handler + * releases it (#2087). Releasing frees its SQLite handle, its watcher and the + * writer lock the engine may hold on that project — which otherwise stays held + * for the whole life of this daemon, locking the project's own daemon and + * `codegraph index` out. The next call reopens it and catches up. + * `CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS` overrides it; `0` never releases. + */ +const DEFAULT_PROJECT_IDLE_TIMEOUT_MS = 600_000; +function resolveProjectIdleTimeoutMs(): number { + const raw = process.env.CODEGRAPH_PROJECT_IDLE_TIMEOUT_MS; + if (raw === undefined || raw === '') return DEFAULT_PROJECT_IDLE_TIMEOUT_MS; + const n = Number(raw); + if (!Number.isFinite(n) || n < 0) return DEFAULT_PROJECT_IDLE_TIMEOUT_MS; + return Math.floor(n); +} + /** * Engine-side lifecycle for a project the ToolHandler opened for an explicit * `projectPath` (#1835). `activate` gives it the same treatment the default @@ -1892,6 +1909,10 @@ export class ToolHandler { // 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(); + // When each cached root was last handed to a call, and the one timer that + // releases the oldest once it has been idle long enough (#2087). + private projectUsedAt: Map = new Map(); + private idleReleaseTimer: NodeJS.Timeout | null = null; // 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; @@ -2260,6 +2281,7 @@ export class ToolHandler { // Refresh LRU position. this.projectCache.delete(canonicalRoot); this.projectCache.set(canonicalRoot, cached); + this.projectUsedAt.set(canonicalRoot, Date.now()); return this.freshen(cached); } @@ -2269,6 +2291,7 @@ export class ToolHandler { if (isSameIndexRoot(root, resolvedRoot)) { this.projectCache.delete(root); this.projectCache.set(root, open); + this.projectUsedAt.set(root, Date.now()); return this.freshen(open); } } @@ -2276,6 +2299,7 @@ export class ToolHandler { const open = () => loadCodeGraph().openSync(canonicalRoot); const cg = this.projectLifecycle?.open(canonicalRoot, open) ?? open(); this.projectCache.set(canonicalRoot, cg); + this.projectUsedAt.set(canonicalRoot, Date.now()); this.trimProjects(); return cg; } @@ -2295,13 +2319,21 @@ export class ToolHandler { await this.awaitCatchUpGate(gate); } - /** Never evict a graph while a tool call or its timed-out reconcile uses it. */ + /** + * Never evict a graph while a tool call or its timed-out reconcile uses it. + * Evicts over the LRU bound, on close, and once idle past the timeout + * (#2087). The cache is in last-use order, so idle entries lead it. + */ private trimProjects(): void { if (this.activeCalls > 0) return; + const idleMs = resolveProjectIdleTimeoutMs(); + const now = Date.now(); for (const [root, cg] of this.projectCache) { - if (!this.closing && this.projectCache.size <= MAX_CACHED_PROJECTS) break; + const idle = idleMs > 0 && now - (this.projectUsedAt.get(root) ?? now) >= idleMs; + if (!this.closing && this.projectCache.size <= MAX_CACHED_PROJECTS && !idle) break; if (this.projectGates.has(cg)) continue; this.projectCache.delete(root); + this.projectUsedAt.delete(root); if (this.projectLifecycle) { this.pendingCloses++; void Promise.resolve(this.projectLifecycle.release(cg)).finally(() => { @@ -2313,6 +2345,26 @@ export class ToolHandler { if (this.closing && this.projectCache.size === 0 && this.pendingCloses === 0) { for (const resolve of this.closeWaiters.splice(0)) resolve(); } + this.scheduleIdleRelease(idleMs); + } + + /** + * Arm one unref'd timer for the oldest project a trim could release. A + * project whose catch-up is still running is trimmed when that settles; the + * 1s floor keeps a project a trim must skip from re-arming in a tight loop. + */ + private scheduleIdleRelease(idleMs: number): void { + if (this.idleReleaseTimer || this.closing || idleMs <= 0) return; + for (const [root, cg] of this.projectCache) { + if (this.projectGates.has(cg)) continue; + const due = (this.projectUsedAt.get(root) ?? Date.now()) + idleMs - Date.now(); + this.idleReleaseTimer = setTimeout(() => { + this.idleReleaseTimer = null; + this.trimProjects(); + }, Math.min(Math.max(due, 1000), 0x7fffffff)); // setTimeout's 32-bit cap + this.idleReleaseTimer.unref(); + return; + } } /** @@ -2347,6 +2399,8 @@ export class ToolHandler { closeAll(): Promise { this.closing = true; this.worktreeMismatchCache.clear(); + if (this.idleReleaseTimer) clearTimeout(this.idleReleaseTimer); + this.idleReleaseTimer = null; this.trimProjects(); if (this.projectCache.size === 0 && this.activeCalls === 0 && this.pendingCloses === 0) return Promise.resolve(); return new Promise((resolve) => this.closeWaiters.push(resolve));