Ver código fonte

fix(mcp): auto-detect project via roots/list when no rootUri (#196) (#214)

MCP tools failed with "CodeGraph not initialized" when a client launched
the server outside the project and sent no rootUri/workspaceFolders — the
server fell back to its own cwd, missed the project's .codegraph/, and
returned a misleading "run codegraph init" error on every call. The only
workaround was passing projectPath by hand to each tool.

When no explicit path is given, the server now asks the client for its
workspace root via the standard MCP roots/list request (gated on the
client advertising the roots capability) before falling back to cwd. This
required teaching the stdio transport to send server->client requests and
match their responses by id (previously responses were dropped as invalid).

When a project still can't be resolved, the error now names the directory
it searched and tells the user to pass projectPath or add --path to the
MCP config, instead of pointing at a re-init they don't need.

Reported-by: @zhangyu1197
Closes #196

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Colby Mchenry 1 mês atrás
pai
commit
1cd162a66d
5 arquivos alterados com 388 adições e 20 exclusões
  1. 17 0
      CHANGELOG.md
  2. 180 0
      __tests__/mcp-roots.test.ts
  3. 101 19
      src/mcp/index.ts
  4. 21 1
      src/mcp/tools.ts
  5. 69 0
      src/mcp/transport.ts

+ 17 - 0
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

+ 180 - 0
__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<Record<string, any>> {
+  const messages: Array<Record<string, any>> = [];
+  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<Record<string, any>>,
+  predicate: (m: Record<string, any>) => boolean,
+  timeoutMs: number,
+): Promise<Record<string, any>> {
+  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);
+});

+ 101 - 19
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<void> | 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<void> {
+    // 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<void> {
+    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;
+      });
+    }
   }
 
   /**

+ 21 - 1
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<string, CodeGraph> = 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;
     }

+ 69 - 0
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<string | number, {
+    resolve: (value: unknown) => 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<unknown> {
+    const id = `cg-srv-${this.nextRequestId++}`;
+    return new Promise<unknown>((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<string, unknown>;
+    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<string, unknown>): 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
    */