fix(mcp): read the initialize handshake before the transport consumes the body

src/mcp/server.ts

@@ -665,7 +665,12 @@ export async function handleMcpRequest(c: AppContext): Promise<Response> {
   if (!identity) return c.json({ error: "unauthorized" }, 401);
 
   const telemetry = buildMcpClientTelemetry(c.req.raw.headers, { defaultClientName: "mcp" })!;
-  const usageMetadata = await describeMcpUsageRequest(c.req.raw, telemetry.metadata);
+  // ONE clone-and-parse of the JSON-RPC body, here, BEFORE createMcpHandler below consumes it (#10190).
+  // A second `request.clone()` after that point throws `TypeError: unusable` -- the Fetch spec forbids
+  // cloning a request whose body is already disturbed -- which is why the post-response handshake read this
+  // replaces turned every `initialize` into an unhandled 500.
+  const envelope = await readMcpRequestEnvelope(c.req.raw);
+  const usageMetadata = describeMcpUsageRequest(envelope, c.req.raw.method, telemetry.metadata);
   const startedAt = Date.now();
   const executionCtx = getExecutionContext(c);
   // #9525: the dispatch chokepoint's sink is built per request so its deferred work rides this
@@ -693,7 +698,7 @@ export async function handleMcpRequest(c: AppContext): Promise<Response> {
     // a rejected handshake never inflates the session/client counts.
     if (response.status < 400) {
       if (usageMetadata.rpcMethod === "initialize") {
-        recordMcpInitialize(c.env, defer, await readInitializeHandshake(c.req.raw), analyticsContext);
+        recordMcpInitialize(c.env, defer, readInitializeHandshake(envelope), analyticsContext);
       } else if (usageMetadata.rpcMethod === "tools/list") {
         // Names come from this server's own registration chokepoint, not the cross-server contract
         // registry, so the event reports what was actually advertised to THIS client.
@@ -765,20 +770,35 @@ function trimmedHeader(value: string | null): string | undefined {
  * client. Returns an empty handshake for a malformed or absent body: the fields are optional by
  * contract, and a session that connected is still worth counting.
  */
-async function readInitializeHandshake(request: Request): Promise<McpInitializeTelemetry> {
-  const body = await request.clone().json().catch(() => null);
-  if (!body || typeof body !== "object") return {};
-  const clientInfo = (body as { params?: { clientInfo?: { name?: unknown; version?: unknown } } }).params?.clientInfo;
+function readInitializeHandshake(envelope: McpRequestEnvelope | null): McpInitializeTelemetry {
+  const clientInfo = envelope?.params?.clientInfo;
   return {
     clientName: typeof clientInfo?.name === "string" ? clientInfo.name : undefined,
     clientVersion: typeof clientInfo?.version === "string" ? clientInfo.version : undefined,
   };
 }
 
-async function describeMcpUsageRequest(request: Request, telemetryMetadata: Record<string, unknown> | undefined): Promise<Record<string, unknown>> {
+/** The JSON-RPC envelope fields the telemetry paths read. Deliberately structural and permissive: this is an
+ *  unvalidated client body, and every consumer below re-checks the type of the field it uses. */
+type McpRequestEnvelope = {
+  method?: unknown;
+  params?: { name?: unknown; clientInfo?: { name?: unknown; version?: unknown } };
+};
+
+/** Clone-and-parse the request body exactly once, at the top of {@link handleMcpRequest} (#10190). Returns
+ *  null for an absent or malformed body -- the telemetry fields are all optional by contract, and a request
+ *  that is still worth counting must never be failed over its own instrumentation. */
+async function readMcpRequestEnvelope(request: Request): Promise<McpRequestEnvelope | null> {
   const body = await request.clone().json().catch(() => null);
-  if (!body || typeof body !== "object") return { transport: "http", method: request.method, ...telemetryMetadata };
-  const envelope = body as { method?: unknown; params?: { name?: unknown } };
+  return body && typeof body === "object" ? (body as McpRequestEnvelope) : null;
+}
+
+function describeMcpUsageRequest(
+  envelope: McpRequestEnvelope | null,
+  method: string,
+  telemetryMetadata: Record<string, unknown> | undefined,
+): Record<string, unknown> {
+  if (!envelope) return { transport: "http", method, ...telemetryMetadata };
   const rpcMethod = typeof envelope.method === "string" ? envelope.method : undefined;
   const toolName = envelope.params && typeof envelope.params.name === "string" ? envelope.params.name : undefined;
   return {

test/unit/mcp-server-telemetry.test.ts

@@ -273,4 +273,51 @@ describe("MCP server telemetry", () => {
     expect(response.status).toBe(200);
     await expect(response.clone().json()).resolves.toEqual({ ok: true, result: "unchanged" });
   });
+
+  // REGRESSION (#10190): the #10175 handshake read called `c.req.raw.clone()` AFTER createMcpHandler had
+  // consumed the body. The Fetch spec forbids cloning a request whose body is already disturbed, so it threw
+  // `TypeError: unusable`, handleMcpRequest's catch rethrew, and a correct 2xx MCP response was replaced by an
+  // unhandled 500 -- on every `initialize`, i.e. the first call of every MCP session. The telemetry key gate
+  // did not save it: the handshake read is an ARGUMENT to recordMcpInitialize, evaluated before its no-op check.
+  const runInitialize = async (params: unknown): Promise<Response> => {
+    vi.resetModules();
+    // A handler that CONSUMES the request body, exactly as the real MCP transport does -- the whole point of
+    // the regression. A handler that ignored the body would pass even with the bug reintroduced.
+    vi.doMock("agents/mcp", () => ({
+      createMcpHandler: () => async (request: Request) => {
+        await request.text();
+        return Response.json({ jsonrpc: "2.0", id: "init", result: { protocolVersion: "2024-11-05" } });
+      },
+    }));
+    const { handleMcpRequest } = await import("../../src/mcp/server");
+    const env = createTestEnv({ PRODUCT_USAGE_HASH_SALT: "mcp-initialize-clone-salt" });
+    const request = new Request("https://api.test/mcp", {
+      method: "POST",
+      headers: { authorization: `Bearer ${env.LOOPOVER_MCP_TOKEN}`, "content-type": "application/json" },
+      body: JSON.stringify({ jsonrpc: "2.0", id: "init", method: "initialize", ...(params === undefined ? {} : { params }) }),
+    });
+    return handleMcpRequest({
+      env,
+      executionCtx: { waitUntil() {}, passThroughOnException() {} },
+      req: { method: "POST", raw: request, header: (name: string) => request.headers.get(name) ?? undefined },
+      json: (body: unknown, status?: number) => Response.json(body, status === undefined ? undefined : { status }),
+    } as never);
+  };
+
+  it("REGRESSION (#10190): an initialize whose body the handler consumed still returns the handler's response, not a 500", async () => {
+    const response = await runInitialize({
+      protocolVersion: "2024-11-05",
+      capabilities: {},
+      clientInfo: { name: "claude-code", version: "2.1.0" },
+    });
+    expect(response.status).toBe(200);
+    await expect(response.clone().json()).resolves.toMatchObject({ result: { protocolVersion: "2024-11-05" } });
+  });
+
+  it("REGRESSION (#10190): an initialize with no params, and one whose clientInfo fields are not strings, still succeed", async () => {
+    // The handshake fields are optional by contract -- a client that omits them must not be failed over
+    // LoopOver's own instrumentation.
+    await expect(runInitialize(undefined)).resolves.toMatchObject({ status: 200 });
+    await expect(runInitialize({ clientInfo: { name: 42, version: null } })).resolves.toMatchObject({ status: 200 });
+  });
 });