Przeglądaj źródła

perf(mcp): defer CodeGraph load off the serve --mcp startup path (cold-start ~811ms→~600ms)

The MCP handshake (initialize + tools/list) only needs static tool schemas — the
heavy CodeGraph chain (sqlite + query/graph/context) is needed only once a tool
actually opens a project. But engine.ts AND tools.ts imported CodeGraph as a
value, so loading ToolHandler (to answer tools/list) dragged the whole chain in
before the daemon could bind. In headless `claude -p` the agent fired its first
turn inside that window → "No such tool available" → grep/Read flounder spirals.

Fixes:
- engine.ts + tools.ts: `import type CodeGraph` (erased) + lazy `require('../index')`
  at the open sites; `findNearestCodeGraphRoot` now comes from the light
  `../directory`, not the heavy `../index`. CodeGraph loads in the already-
  backgrounded `ensureInitialized` (#172) — off the bind/handshake critical path,
  ready by the first tool call. require() is sync + cached (CommonJS build).
- index.ts: daemon-connect poll 100ms→25ms (same ~6s give-up budget, 240 tries)
  so the proxy attaches the instant the spawned daemon binds.

Cold-start to tools-registered: ~811ms → ~600ms (~25%), narrowing the race
window (full elimination would need the proxy to answer the handshake locally —
a larger daemon-architecture change, separate follow-up). The benchmark parser
already excludes any run that still races.

Validated: 30/30 MCP+daemon tests (incl. #411/#277 lifecycle), full suite 1090
pass (only pre-existing npm-shim network fails). Updated the adaptive-explore
test's skeleton-tag marker to the steer-to-explore wording from 5d7388c.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Colby McHenry 3 tygodni temu
rodzic
commit
91e28df0c7
4 zmienionych plików z 35 dodań i 10 usunięć
  1. 3 1
      __tests__/adaptive-explore-sizing.test.ts
  2. 13 3
      src/mcp/engine.ts
  3. 9 4
      src/mcp/index.ts
  4. 10 2
      src/mcp/tools.ts

+ 3 - 1
__tests__/adaptive-explore-sizing.test.ts

@@ -31,7 +31,9 @@ import * as os from 'os';
 import { ToolHandler } from '../src/mcp/tools';
 import CodeGraph from '../src/index';
 
-const SKELETON_MARK = '· skeleton (signatures only; Read for a full body)';
+// Stable marker — assert the `· skeleton` tag, not its exact trailing wording
+// (the steer-to-explore phrasing changed when the Read invitation was removed).
+const SKELETON_MARK = '· skeleton (signatures only';
 
 /** Return the `#### <path> ...` section for a file basename, header through the
  *  line before the next `###`/`####` header (or end of output). */

+ 13 - 3
src/mcp/engine.ts

@@ -10,10 +10,20 @@
  *   inotify watch set — that's the entire point of issue #411.
  */
 
-import CodeGraph, { findNearestCodeGraphRoot } from '../index';
+import type CodeGraph from '../index';
+import { findNearestCodeGraphRoot } from '../directory';
 import { watchDisabledReason } from '../sync';
 import { ToolHandler } from './tools';
 
+// Lazy-load the heavy CodeGraph chain (sqlite + query/graph/context layers) OFF
+// the MCP startup path. It's only needed once a tool actually opens a project —
+// not to answer initialize/tools-list — so deferring it lets `serve --mcp` (and
+// the daemon it spawns) bind + register tools in ~Node-startup time instead of
+// ~800ms, closing the "No such tool available" cold-start race that made headless
+// agents flounder. require() is sync + cached on the CommonJS build.
+const loadCodeGraph = (): typeof import('../index').default =>
+  (require('../index') as typeof import('../index')).default;
+
 export interface MCPEngineOptions {
   /**
    * Whether to start the file watcher when initializing. Daemon and direct
@@ -118,7 +128,7 @@ export class MCPEngine {
         try { this.cg.close(); } catch { /* ignore */ }
         this.cg = null;
       }
-      this.cg = CodeGraph.openSync(resolvedRoot);
+      this.cg = loadCodeGraph().openSync(resolvedRoot);
       this.projectPath = resolvedRoot;
       this.toolHandler.setDefaultCodeGraph(this.cg);
       this.startWatching();
@@ -154,7 +164,7 @@ export class MCPEngine {
 
     this.projectPath = resolvedRoot;
     try {
-      this.cg = await CodeGraph.open(resolvedRoot);
+      this.cg = await loadCodeGraph().open(resolvedRoot);
       this.toolHandler.setDefaultCodeGraph(this.cg);
       this.startWatching();
       this.catchUpSync();

+ 9 - 4
src/mcp/index.ts

@@ -37,8 +37,7 @@
 import * as fs from 'fs';
 import * as path from 'path';
 import { spawn, StdioOptions } from 'child_process';
-import { findNearestCodeGraphRoot } from '../index';
-import { getCodeGraphDir } from '../directory';
+import { findNearestCodeGraphRoot, getCodeGraphDir } from '../directory';
 import { StdioTransport } from './transport';
 import { MCPEngine } from './engine';
 import { MCPSession } from './session';
@@ -82,8 +81,14 @@ const TAKEOVER_RETRY_DELAY_MS = 100;
  * process startup. 60 × 100ms = 6s of headroom for a cold/slow box; on the
  * common path the socket appears within a few rounds.
  */
-const DAEMON_CONNECT_MAX_RETRIES = 60;
-const DAEMON_CONNECT_RETRY_DELAY_MS = 100;
+// Poll finely (25ms) so the proxy attaches the instant the freshly-spawned
+// daemon binds, instead of waiting up to a coarse 100ms after — shaves the
+// cold-start handshake (the window the headless agent races). Same ~6s total
+// give-up budget (240 × 25ms), just finer granularity; socket-connect probes
+// are cheap. Paired with deferring the CodeGraph load (engine.ts) off the bind
+// path, this narrows the "No such tool available" race window.
+const DAEMON_CONNECT_MAX_RETRIES = 240;
+const DAEMON_CONNECT_RETRY_DELAY_MS = 25;
 
 /**
  * Resolve the PPID watchdog poll interval from an env override. A value of

+ 10 - 2
src/mcp/tools.ts

@@ -4,7 +4,15 @@
  * Defines the tools exposed by the CodeGraph MCP server.
  */
 
-import CodeGraph, { findNearestCodeGraphRoot } from '../index';
+import type CodeGraph from '../index';
+import { findNearestCodeGraphRoot } from '../directory';
+// Lazy-load the heavy CodeGraph chain off the MCP startup path — see the same
+// helper in engine.ts. ToolHandler must load to answer tools/list (static
+// schemas), but it must NOT drag in sqlite/query layers before the daemon binds;
+// CodeGraph is pulled in only when a tool actually opens a project. require() is
+// sync + cached (CommonJS build).
+const loadCodeGraph = (): typeof import('../index').default =>
+  (require('../index') as typeof import('../index')).default;
 import {
   detectWorktreeIndexMismatch,
   worktreeMismatchWarning,
@@ -841,7 +849,7 @@ export class ToolHandler {
     }
 
     // Open and cache under both paths
-    const cg = CodeGraph.openSync(resolvedRoot);
+    const cg = loadCodeGraph().openSync(resolvedRoot);
     this.projectCache.set(resolvedRoot, cg);
     if (projectPath !== resolvedRoot) {
       this.projectCache.set(projectPath, cg);