diff --git a/CHANGELOG.md b/CHANGELOG.md index ae3d8c7..4f15083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,23 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Thanks to [@essopsp](https://github.com/essopsp) for the repro. ### Fixed +- **MCP**: tools no longer fail with "CodeGraph not initialized" when the index + actually exists. This hit clients that launch the MCP server from a directory + other than your project and don't report a workspace root in `initialize` + (some IDE/JetBrains-family integrations) — the server fell back to its own + working directory, missed the project's `.codegraph/`, and returned the + misleading "Run 'codegraph init' first" on every call. The only workaround + was passing `projectPath` to each tool by hand. Now, when no project path is + supplied, the server asks the client for its workspace root via the standard + MCP `roots/list` request (when the client advertises the `roots` capability) + before falling back to the working directory — so detection just works for + spec-compliant clients. When it still can't resolve a project, the error is + now actionable: it names the directory it searched and tells you to pass + `projectPath` or add `--path /abs/project` to the server's MCP config args, + instead of pointing you at a re-init you don't need. Closes + [#196](https://github.com/colbymchenry/codegraph/issues/196). Thanks to + [@zhangyu1197](https://github.com/zhangyu1197) for the report and the + `projectPath` workaround. - **MCP**: the server no longer hangs on startup under WSL2 when the project lives on an NTFS `/mnt/*` mount. Setting up the recursive file watcher there took tens of seconds — every directory read crosses the Windows/9p diff --git a/__tests__/mcp-roots.test.ts b/__tests__/mcp-roots.test.ts new file mode 100644 index 0000000..8e1d452 --- /dev/null +++ b/__tests__/mcp-roots.test.ts @@ -0,0 +1,180 @@ +/** + * MCP project-resolution regression tests (issue #196). + * + * When an MCP client launches the server outside the project directory AND + * doesn't pass a `rootUri`/`workspaceFolders` in `initialize`, the server used + * to fall straight back to `process.cwd()` — which for many IDE clients is the + * wrong directory. Every tool call without an explicit `projectPath` then + * failed with a misleading "CodeGraph not initialized. Run 'codegraph init'." + * + * The fix: when no explicit path is provided, the server asks the client for + * its workspace root via the spec-blessed `roots/list` request (if the client + * advertised the `roots` capability), and only falls back to cwd otherwise. + * When it still can't resolve, the error now says exactly how to fix it. + * + * These tests drive the real stdio transport via a spawned subprocess — no + * mocking — so they also exercise the new bidirectional request/response path. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { spawn, ChildProcessWithoutNullStreams } from 'child_process'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { CodeGraph } from '../src'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +function spawnServer(cwd: string): ChildProcessWithoutNullStreams { + // --no-watch keeps the test deterministic and avoids watcher startup noise. + return spawn(process.execPath, [BIN, 'serve', '--mcp', '--no-watch'], { + cwd, + stdio: ['pipe', 'pipe', 'pipe'], + }) as ChildProcessWithoutNullStreams; +} + +/** Parse every JSON-RPC message the server writes to stdout into an array. */ +function collectMessages(child: ChildProcessWithoutNullStreams): Array> { + const messages: Array> = []; + let buf = ''; + child.stdout.on('data', (chunk) => { + buf += chunk.toString('utf8'); + let idx; + while ((idx = buf.indexOf('\n')) !== -1) { + const line = buf.slice(0, idx).trim(); + buf = buf.slice(idx + 1); + if (!line) continue; + try { messages.push(JSON.parse(line)); } catch { /* ignore non-JSON */ } + } + }); + return messages; +} + +function waitForMessage( + messages: ReadonlyArray>, + predicate: (m: Record) => boolean, + timeoutMs: number, +): Promise> { + return new Promise((resolve, reject) => { + const started = Date.now(); + const tick = () => { + const hit = messages.find(predicate); + if (hit) return resolve(hit); + if (Date.now() - started > timeoutMs) { + return reject(new Error(`Timed out. Messages so far: ${JSON.stringify(messages)}`)); + } + setTimeout(tick, 20); + }; + tick(); + }); +} + +function send(child: ChildProcessWithoutNullStreams, msg: object): void { + child.stdin.write(JSON.stringify(msg) + '\n'); +} + +const CLIENT_INFO = { name: 'test', version: '0.0.0' }; + +describe('MCP project resolution via roots/list (issue #196)', () => { + let cwdDir: string; // where the server is launched — has NO .codegraph + let projectDir: string; // the real indexed project the client reports + let child: ChildProcessWithoutNullStreams | null = null; + + beforeEach(() => { + cwdDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-cwd-')); + projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-mcp-proj-')); + }); + + afterEach(() => { + if (child && !child.killed) { + child.kill('SIGKILL'); + child = null; + } + fs.rmSync(cwdDir, { recursive: true, force: true }); + fs.rmSync(projectDir, { recursive: true, force: true }); + }); + + it('resolves the project from the client roots/list when no rootUri is sent', async () => { + const cg = await CodeGraph.init(projectDir); + cg.close(); + + child = spawnServer(cwdDir); + const messages = collectMessages(child); + + // Advertise the roots capability but pass NO rootUri/workspaceFolders. + send(child, { + jsonrpc: '2.0', id: 0, method: 'initialize', + params: { protocolVersion: '2025-11-25', capabilities: { roots: {} }, clientInfo: CLIENT_INFO }, + }); + await waitForMessage(messages, (m) => m.id === 0 && !!m.result, 5000); + send(child, { jsonrpc: '2.0', method: 'notifications/initialized' }); + + // First tool call (no projectPath) drives the server to ask us for roots. + send(child, { jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + + const rootsReq = await waitForMessage(messages, (m) => m.method === 'roots/list', 5000); + expect(typeof rootsReq.id).toBe('string'); // server-initiated id + send(child, { + jsonrpc: '2.0', id: rootsReq.id, + result: { roots: [{ uri: `file://${projectDir}`, name: 'proj' }] }, + }); + + // The status call now succeeds against the resolved project. + const resp = await waitForMessage(messages, (m) => m.id === 1, 8000); + const text = resp.result.content[0].text as string; + expect(text).toContain('CodeGraph Status'); + expect(text).not.toContain('No CodeGraph project is loaded'); + }, 20000); + + it('returns an actionable error when there is no rootUri and no roots capability', async () => { + child = spawnServer(cwdDir); + const messages = collectMessages(child); + + send(child, { + jsonrpc: '2.0', id: 0, method: 'initialize', + params: { protocolVersion: '2025-11-25', capabilities: {}, clientInfo: CLIENT_INFO }, + }); + await waitForMessage(messages, (m) => m.id === 0 && !!m.result, 5000); + send(child, { jsonrpc: '2.0', method: 'notifications/initialized' }); + + send(child, { jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + const resp = await waitForMessage(messages, (m) => m.id === 1, 8000); + const text = resp.result.content[0].text as string; + + expect(text).toContain('No CodeGraph project is loaded'); + expect(text).toContain('projectPath'); + expect(text).toContain('--path'); + // Names the directory it actually searched (the wrong cwd) so the user can + // see why detection missed. basename survives any symlink realpath-ing. + expect(text).toContain(path.basename(cwdDir)); + // It must not have hung waiting on roots/list — the client never offered it. + expect(messages.some((m) => m.method === 'roots/list')).toBe(false); + }, 20000); + + it('honors an explicit rootUri without asking the client for roots', async () => { + const cg = await CodeGraph.init(projectDir); + cg.close(); + + child = spawnServer(cwdDir); + const messages = collectMessages(child); + + send(child, { + jsonrpc: '2.0', id: 0, method: 'initialize', + params: { + protocolVersion: '2025-11-25', + capabilities: { roots: {} }, + clientInfo: CLIENT_INFO, + rootUri: `file://${projectDir}`, + }, + }); + await waitForMessage(messages, (m) => m.id === 0 && !!m.result, 5000); + send(child, { jsonrpc: '2.0', method: 'notifications/initialized' }); + + send(child, { jsonrpc: '2.0', id: 1, method: 'tools/call', params: { name: 'codegraph_status', arguments: {} } }); + const resp = await waitForMessage(messages, (m) => m.id === 1, 8000); + const text = resp.result.content[0].text as string; + + expect(text).toContain('CodeGraph Status'); + // rootUri is a stronger signal than roots — we never needed to ask. + expect(messages.some((m) => m.method === 'roots/list')).toBe(false); + }, 20000); +}); diff --git a/src/mcp/index.ts b/src/mcp/index.ts index b601c36..c790a4b 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -54,6 +54,26 @@ const SERVER_INFO = { */ const PROTOCOL_VERSION = '2024-11-05'; +/** + * How long to wait for the client's `roots/list` response before giving up + * and falling back to the process cwd. + */ +const ROOTS_LIST_TIMEOUT_MS = 5000; + +/** + * Extract the first usable filesystem path from a `roots/list` result. + * Shape per MCP spec: `{ roots: [{ uri: "file:///path", name?: string }] }`. + * Returns null if the result is empty or malformed. + */ +function firstRootPath(result: unknown): string | null { + if (!result || typeof result !== 'object') return null; + const roots = (result as { roots?: unknown }).roots; + if (!Array.isArray(roots) || roots.length === 0) return null; + const first = roots[0] as { uri?: unknown }; + if (typeof first?.uri !== 'string') return null; + return fileUriToPath(first.uri); +} + /** * MCP Server for CodeGraph * @@ -68,6 +88,13 @@ export class MCPServer { // In-flight background init kicked off from handleInitialize. Tracked so the // sync retry path doesn't race against it (double-opening the SQLite file). private initPromise: Promise | null = null; + // Whether the client advertised the MCP `roots` capability during initialize. + // If so, and no explicit project path was given, we ask it for the workspace + // root via roots/list rather than guessing from the (often wrong) cwd. + private clientSupportsRoots = false; + // Guards the one-shot deferred resolution (roots/list or cwd) so we don't + // re-issue roots/list on every tool call. + private rootsAttempted = false; constructor(projectPath?: string) { this.projectPath = projectPath || null; @@ -108,6 +135,9 @@ export class MCPServer { * are still possible. */ private async tryInitializeDefault(projectPath: string): Promise { + // Record where we searched so a later "not initialized" error can name it. + this.toolHandler.setDefaultProjectHint(projectPath); + // Walk up parent directories to find nearest .codegraph/ const resolvedRoot = findNearestCodeGraphRoot(projectPath); @@ -146,10 +176,28 @@ export class MCPServer { // Already initialized successfully if (this.toolHandler.hasDefaultCodeGraph()) return; - // No project path to retry with - if (!this.projectPath) return; - const resolvedRoot = findNearestCodeGraphRoot(this.projectPath); + // No explicit path was given at initialize. Resolve it now, exactly once: + // ask the client via roots/list (if it advertised roots), else use cwd. + // Deferring to here lets a roots answer override the wrong cwd, and the + // one-shot guard means we never re-issue roots/list per tool call. + if (!this.projectPath && !this.rootsAttempted) { + this.rootsAttempted = true; + this.initPromise = ( + this.clientSupportsRoots + ? this.initFromRoots() + : this.tryInitializeDefault(process.cwd()) + ).finally(() => { this.initPromise = null; }); + try { await this.initPromise; } catch { /* fall through to last-resort below */ } + if (this.toolHandler.hasDefaultCodeGraph()) return; + } + + // Last resort: re-walk from the best candidate we have. Picks up projects + // initialized after the server started, and covers clients that sent no + // usable initialize signal at all. + const candidate = this.projectPath ?? process.cwd(); + this.toolHandler.setDefaultProjectHint(candidate); + const resolvedRoot = findNearestCodeGraphRoot(candidate); if (!resolvedRoot) return; try { @@ -167,6 +215,28 @@ export class MCPServer { } } + /** + * Resolve the project root via the MCP `roots/list` request and initialize + * from the first root the client reports. Falls back to the process cwd if + * the client returns no usable root or doesn't answer in time. See issue #196. + */ + private async initFromRoots(): Promise { + let target = process.cwd(); + try { + const result = await this.transport.request('roots/list', undefined, ROOTS_LIST_TIMEOUT_MS); + const rootPath = firstRootPath(result); + if (rootPath) { + target = rootPath; + } else { + process.stderr.write('[CodeGraph MCP] Client returned no workspace roots; falling back to process cwd.\n'); + } + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + process.stderr.write(`[CodeGraph MCP] roots/list request failed (${msg}); falling back to process cwd.\n`); + } + await this.tryInitializeDefault(target); + } + /** * Start file watching on the active CodeGraph instance. * Logs sync activity to stderr for diagnostics. @@ -279,20 +349,25 @@ export class MCPServer { const params = request.params as { rootUri?: string; workspaceFolders?: Array<{ uri: string; name: string }>; + capabilities?: { roots?: unknown }; } | undefined; - // Extract project path from rootUri or workspaceFolders - let projectPath = this.projectPath; + // Does the client support the MCP `roots` protocol? If so, and we have no + // explicit path, we ask it for the workspace root after the handshake + // instead of falling back to the (frequently wrong) cwd. See issue #196. + this.clientSupportsRoots = !!params?.capabilities?.roots; + // Explicit project signal, strongest first: a client-provided rootUri / + // workspaceFolders (LSP-style, non-standard but some clients send it), else + // the --path the server was launched with. cwd is NOT used here — we defer + // it so a roots/list answer can win over it. + let explicitPath: string | null = null; if (params?.rootUri) { - projectPath = fileUriToPath(params.rootUri); + explicitPath = fileUriToPath(params.rootUri); } else if (params?.workspaceFolders?.[0]?.uri) { - projectPath = fileUriToPath(params.workspaceFolders[0].uri); - } - - // Fall back to current working directory if no path provided - if (!projectPath) { - projectPath = process.cwd(); + explicitPath = fileUriToPath(params.workspaceFolders[0].uri); + } else if (this.projectPath) { + explicitPath = this.projectPath; } // Respond to the handshake BEFORE doing any heavy initialization. Loading @@ -315,13 +390,20 @@ export class MCPServer { instructions: SERVER_INSTRUCTIONS, }); - // Kick off the default-project init in the background. Tool calls that - // arrive before it finishes will see the "not initialized yet" path and - // fall through to `retryInitIfNeeded`, which now waits for this promise - // rather than racing against it with a second open. - this.initPromise = this.tryInitializeDefault(projectPath).finally(() => { - this.initPromise = null; - }); + // If we know the project dir, kick off init in the background now. Tool + // calls that arrive before it finishes fall through to `retryInitIfNeeded`, + // which waits for this promise rather than racing it with a second open. + // + // If we DON'T know it (no rootUri, no --path), defer: the first tool call + // resolves it via roots/list (when the client supports roots) or cwd. This + // is the fix for issue #196 — clients that launch the server outside the + // project and don't pass a rootUri previously got a misleading "not + // initialized" error on every call. + if (explicitPath) { + this.initPromise = this.tryInitializeDefault(explicitPath).finally(() => { + this.initPromise = null; + }); + } } /** diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 7b0d55b..204ee59 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -440,6 +440,9 @@ export const tools: ToolDefinition[] = [ export class ToolHandler { // Cache of opened CodeGraph instances for cross-project queries private projectCache: 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; constructor(private cg: CodeGraph | null) {} @@ -450,6 +453,14 @@ export class ToolHandler { this.cg = cg; } + /** + * Record the directory the server tried to resolve the default project from. + * Used only to make the "no default project" error actionable. + */ + setDefaultProjectHint(searchedPath: string): void { + this.defaultProjectHint = searchedPath; + } + /** * Whether a default CodeGraph instance is available */ @@ -495,7 +506,16 @@ export class ToolHandler { private getCodeGraph(projectPath?: string): CodeGraph { if (!projectPath) { if (!this.cg) { - throw new Error('CodeGraph not initialized for this project. Run \'codegraph init\' first.'); + const searched = this.defaultProjectHint ?? process.cwd(); + throw new Error( + 'No CodeGraph project is loaded for this session.\n' + + `Searched for a .codegraph/ directory starting from: ${searched}\n` + + 'The index is likely fine — this is a working-directory detection issue: ' + + "the MCP client launched the server outside your project and didn't report the " + + 'workspace root. Fix it either way:\n' + + ' • Pass projectPath to the tool call, e.g. projectPath: "/absolute/path/to/your/project"\n' + + ' • Or add --path to the server\'s MCP config args: ["serve", "--mcp", "--path", "/absolute/path/to/your/project"]' + ); } return this.cg; } diff --git a/src/mcp/transport.ts b/src/mcp/transport.ts index 4403891..2638600 100644 --- a/src/mcp/transport.ts +++ b/src/mcp/transport.ts @@ -63,6 +63,13 @@ export type MessageHandler = (message: JsonRpcRequest | JsonRpcNotification) => export class StdioTransport { private rl: readline.Interface | null = null; private messageHandler: MessageHandler | null = null; + // Outstanding server-initiated requests (e.g. roots/list), keyed by the id + // we sent. Responses from the client are matched back here. + private pending = new Map void; + reject: (error: Error) => void; + }>(); + private nextRequestId = 1; /** * Start listening for messages on stdin @@ -89,12 +96,42 @@ export class StdioTransport { * Stop listening */ stop(): void { + // Fail any in-flight server-initiated requests so their awaiters don't hang. + for (const { reject } of this.pending.values()) { + reject(new Error('Transport stopped')); + } + this.pending.clear(); if (this.rl) { this.rl.close(); this.rl = null; } } + /** + * Send a server-initiated request to the client and await its response. + * + * MCP is bidirectional: the server can ask the client questions too. We use + * this for `roots/list` — the spec-blessed way to learn the workspace root + * when the client didn't pass one in `initialize` (see issue #196). Rejects + * on timeout so callers can fall back rather than hang forever. + */ + request(method: string, params?: unknown, timeoutMs = 5000): Promise { + const id = `cg-srv-${this.nextRequestId++}`; + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + this.pending.delete(id); + reject(new Error(`Timed out after ${timeoutMs}ms waiting for "${method}" response`)); + }, timeoutMs); + // Don't let a pending request keep the process alive on shutdown. + timer.unref?.(); + this.pending.set(id, { + resolve: (value) => { clearTimeout(timer); resolve(value); }, + reject: (error) => { clearTimeout(timer); reject(error); }, + }); + process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id, method, params }) + '\n'); + }); + } + /** * Send a response */ @@ -152,6 +189,20 @@ export class StdioTransport { return; } + // Response to a server-initiated request (has id + result/error, no method). + // Route it to the awaiting requester instead of the message handler — these + // used to be dropped as "Invalid Request" because they carry no method. + const obj = parsed as Record; + if ( + obj?.jsonrpc === '2.0' && + typeof obj.method !== 'string' && + 'id' in obj && + ('result' in obj || 'error' in obj) + ) { + this.handleResponse(obj); + return; + } + // Validate basic JSON-RPC structure if (!this.isValidMessage(parsed)) { this.sendError(null, ErrorCodes.InvalidRequest, 'Invalid Request: not a valid JSON-RPC 2.0 message'); @@ -174,6 +225,24 @@ export class StdioTransport { } } + /** + * Resolve (or reject) the pending server-initiated request matching this + * response's id. Unknown ids are ignored — the client may echo something we + * never sent, or a request may have already timed out. + */ + private handleResponse(msg: Record): void { + const id = msg.id as string | number; + const pending = this.pending.get(id); + if (!pending) return; + this.pending.delete(id); + if ('error' in msg && msg.error) { + const err = msg.error as { message?: string }; + pending.reject(new Error(err.message || 'Request failed')); + } else { + pending.resolve(msg.result); + } + } + /** * Check if message is a valid JSON-RPC 2.0 message */