feat(telemetry): emit mcp_connect on the singular MCP connect path

Author: TheGreatAxios

src/mcp/client-telemetry.test.ts

@@ -25,7 +25,7 @@ await withMockedModule(
   }),
 );
 
-const { connectMCPServers } = await import("./client.js");
+const { connectMCPServers, connectMCPServer } = await import("./client.js");
 
 function recordingTelemetry(events: Record<string, unknown>[]): Telemetry {
   return {
@@ -62,11 +62,17 @@ describe("connectMCPServers telemetry", () => {
       expect(clients).toHaveLength(1);
       expect(warnings).toHaveLength(1);
       expect(events).toHaveLength(2);
-      expect(events.map((event) => event.transport)).toEqual([
+      // Order-insensitive: the batch fans out over Promise.all, and each
+      // singular connect captures its own event on completion, so arrival
+      // order follows settle order, not config order.
+      expect(events.map((event) => event.transport).sort()).toEqual([
         "stdio",
         "stdio",
       ]);
-      expect(events.map((event) => event.result)).toEqual(["ok", "fail"]);
+      expect(events.map((event) => event.result).sort()).toEqual([
+        "fail",
+        "ok",
+      ]);
     } finally {
       for (const client of clients) await client.close();
     }
@@ -92,3 +98,31 @@ describe("connectMCPServers telemetry", () => {
     }
   });
 });
+
+describe("connectMCPServer telemetry", () => {
+  beforeEach(() => {
+    connectCalls = 0;
+    failOnCall = -1;
+  });
+
+  test("singular ok + fail emits one mcp_connect each, exactly once per attempt", async () => {
+    const events: Record<string, unknown>[] = [];
+    const telemetry = recordingTelemetry(events);
+    const ok = await connectMCPServer(
+      { name: "a", type: "stdio", command: "test-server-a" },
+      { telemetry },
+    );
+    expect(ok.ok).toBe(true);
+    if (ok.ok) await ok.client.close();
+    failOnCall = 2;
+    const failed = await connectMCPServer(
+      { name: "b", type: "stdio", command: "test-server-b" },
+      { telemetry },
+    );
+    expect(failed.ok).toBe(false);
+    expect(connectCalls).toBe(2);
+    expect(events).toHaveLength(connectCalls);
+    expect(events.map((event) => event.transport)).toEqual(["stdio", "stdio"]);
+    expect(events.map((event) => event.result)).toEqual(["ok", "fail"]);
+  });
+});

src/mcp/client.ts

@@ -857,37 +857,40 @@ export async function connectMCPServer(
   config: ResolvedMCPServerConfig,
   options: MCPConnectOptions = {},
 ): Promise<MCPConnectResult> {
-  return isHttpServer(config)
+  const telemetry = options.telemetry ?? NOOP_TELEMETRY;
+  const result = await (isHttpServer(config)
     ? connectHttp(config, options)
-    : connectStdio(config, options);
+    : connectStdio(config, options));
+  // One enum-only event per outward connection outcome. Internal HTTP
+  // auth-recovery re-dials stay inside connectHttp, so this wrapper fires
+  // exactly once per connectMCPServer call. Server name, URL, and error
+  // text never leave the process.
+  captureMcpConnect(telemetry, {
+    transport: isHttpServer(config) ? "http" : "stdio",
+    result: classifyMcpConnectOutcome(result),
+  });
+  return result;
 }
 
 export async function connectMCPServers(
   configs: ResolvedMCPServerConfig[],
   onWarning: (message: string) => void,
   options: MCPConnectOptions = {},
 ): Promise<MCPClient[]> {
-  const telemetry = options.telemetry ?? NOOP_TELEMETRY;
+  // Telemetry flows through options into connectMCPServer, which captures
+  // one mcp_connect per config.
   const results = await Promise.all(
     configs.map((c) => connectMCPServer(c, options)),
   );
   const clients: MCPClient[] = [];
-  for (let index = 0; index < results.length; index += 1) {
-    const result = results[index];
-    if (result === undefined) continue;
+  for (const result of results) {
     if (result.ok) clients.push(result.client);
     else
       onWarning(
         `[mcp] Warning: failed to connect to MCP server "${result.serverName}": ${result.error}`,
       );
-    // One enum-only event per server: the transport that was dialed and the
-    // outcome enum. Server name, URL, and error text never leave the process.
-    const config = configs[index];
-    captureMcpConnect(telemetry, {
-      transport:
-        config !== undefined && isHttpServer(config) ? "http" : "stdio",
-      result: classifyMcpConnectOutcome(result),
-    });
+    // No emit here: connectMCPServer already captured one enum-only
+    // mcp_connect per config. Emitting again would double-count.
   }
   return clients;
 }