feat(telemetry): add auth_success, mcp_connect, and mcp_oauth events

src/agent/tools.ts

@@ -1264,6 +1264,9 @@ export async function createAgentToolset(
       try {
         result = await connectMCPClient(config, {
           stderr: "ignore",
+          ...(args.telemetry !== undefined
+            ? { telemetry: args.telemetry }
+            : {}),
           ...(callbacks.interactiveAuth
             ? {
                 onAuthURL: (name: string, url: string) => {

src/mcp/client-auth-reauth-cap.test.ts

@@ -7,6 +7,7 @@ import {
   test,
 } from "bun:test";
 import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js";
+import type { Telemetry } from "../telemetry/index.js";
 import { withMockedModule } from "../../tests/helpers/mock-module.js";
 
 let finishAuthCalls = 0;
@@ -263,7 +264,7 @@ const config = {
   url: "https://mcp.linear.app/mcp",
 };
 
-async function connectWithAuthPrompt(): Promise<{
+async function connectWithAuthPrompt(telemetry?: Telemetry): Promise<{
   ok: boolean;
   error?: string;
   authPending?: boolean;
@@ -273,6 +274,7 @@ async function connectWithAuthPrompt(): Promise<{
       authURLCount += 1;
       authEvents.push("authURL");
     },
+    ...(telemetry !== undefined ? { telemetry } : {}),
   });
   return result.ok
     ? { ok: true }
@@ -864,6 +866,46 @@ describe("HTTP MCP re-auth loop prevention", () => {
     expect(providerCreates).toBeGreaterThan(MAX_BROWSER_AUTH_ATTEMPTS);
   });
 
+  test("a capped browser-auth denial emits one mcp_oauth timeout", async () => {
+    const oauthResults: unknown[] = [];
+    const telemetry: Telemetry = {
+      enabled: true,
+      installationId: "test",
+      capture: (event, properties) => {
+        if (event === "mcp_oauth")
+          oauthResults.push(
+            (properties as Record<string, unknown> | undefined)?.result,
+          );
+      },
+      captureIntentional: () => false,
+      flush: async () => undefined,
+      discard: () => undefined,
+    };
+    const connectWithTelemetry = () => connectWithAuthPrompt(telemetry);
+    connectFailuresLeft = Number.POSITIVE_INFINITY;
+
+    for (let episode = 0; episode < MAX_BROWSER_AUTH_ATTEMPTS; episode += 1) {
+      const result = await connectWithTelemetry();
+      expect(result.ok).toBe(false);
+      expect(result.error).toContain("finishAuth exploded");
+    }
+    expect(authURLCount).toBe(MAX_BROWSER_AUTH_ATTEMPTS);
+    // Each failed episode emits exactly one timeout through recovery.
+    expect(oauthResults).toEqual(
+      Array.from({ length: MAX_BROWSER_AUTH_ATTEMPTS }, () => "timeout"),
+    );
+
+    const capped = await connectWithTelemetry();
+    expect(capped.ok).toBe(false);
+    expect(capped.error).toContain("retrying paused");
+    expect(authURLCount).toBe(MAX_BROWSER_AUTH_ATTEMPTS);
+    // The cap denial exits before a browser flow exists, but it still maps
+    // to exactly one terminal timeout rather than staying silent.
+    expect(oauthResults).toEqual(
+      Array.from({ length: MAX_BROWSER_AUTH_ATTEMPTS + 1 }, () => "timeout"),
+    );
+  });
+
   test("successful interactive auth clears the cap so a later failure can prompt", async () => {
     finishAuthError = undefined;
     connectFailuresLeft = 1;

src/mcp/client-telemetry.test.ts

@@ -0,0 +1,128 @@
+import { beforeEach, describe, expect, test } from "bun:test";
+import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js";
+import { withMockedModule } from "../../tests/helpers/mock-module.js";
+
+let connectCalls = 0;
+let failOnCall = -1;
+const connectError = new Error("spawn ENOENT");
+
+await withMockedModule(
+  import.meta.resolve("@modelcontextprotocol/sdk/client/index.js"),
+  (real: typeof import("@modelcontextprotocol/sdk/client/index.js")) => ({
+    ...real,
+    Client: class {
+      async connect(): Promise<void> {
+        connectCalls += 1;
+        if (connectCalls === failOnCall) throw connectError;
+      }
+      async listTools(): Promise<{ tools: [] }> {
+        return { tools: [] };
+      }
+      async close(): Promise<void> {
+        return undefined;
+      }
+    },
+  }),
+);
+
+const { connectMCPServers, connectMCPServer } = await import("./client.js");
+
+function recordingTelemetry(events: Record<string, unknown>[]): Telemetry {
+  return {
+    enabled: true,
+    installationId: "test",
+    capture: (event, properties) => {
+      if (event === "mcp_connect") events.push({ ...(properties ?? {}) });
+    },
+    captureIntentional: () => false,
+    flush: async () => undefined,
+    discard: () => undefined,
+  };
+}
+
+describe("connectMCPServers telemetry", () => {
+  beforeEach(() => {
+    connectCalls = 0;
+    failOnCall = -1;
+  });
+
+  test("a mixed multi-config run emits exactly one mcp_connect per config", async () => {
+    failOnCall = 2;
+    const events: Record<string, unknown>[] = [];
+    const warnings: string[] = [];
+    const clients = await connectMCPServers(
+      [
+        { name: "a", type: "stdio", command: "test-server-a" },
+        { name: "b", type: "stdio", command: "test-server-b" },
+      ],
+      (message) => warnings.push(message),
+      { telemetry: recordingTelemetry(events) },
+    );
+    try {
+      expect(clients).toHaveLength(1);
+      expect(warnings).toHaveLength(1);
+      expect(events).toHaveLength(2);
+      // 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).sort()).toEqual([
+        "fail",
+        "ok",
+      ]);
+    } finally {
+      for (const client of clients) await client.close();
+    }
+  });
+
+  test("omitted telemetry stays silent on the dropping default", async () => {
+    // The omitted path routes capture calls into NOOP_TELEMETRY, whose
+    // capture drops everything — the gate lives in the Telemetry handle,
+    // not at the call site. This keeper pins that contract: the default
+    // handle is disabled, and connecting without telemetry resolves
+    // normally with nothing to flush or send.
+    expect(NOOP_TELEMETRY.enabled).toBe(false);
+    const warnings: string[] = [];
+    const clients = await connectMCPServers(
+      [{ name: "a", type: "stdio", command: "test-server-a" }],
+      (message) => warnings.push(message),
+    );
+    try {
+      expect(clients).toHaveLength(1);
+      expect(warnings).toHaveLength(0);
+    } finally {
+      for (const client of clients) await client.close();
+    }
+  });
+});
+
+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

@@ -15,6 +15,14 @@ import type { McpToolAnnotations } from "./tool-permissions.js";
 import { buildStdioMcpProcessEnv } from "./stdio-env.js";
 import { isHttpServer } from "./is-http-server.js";
 import { MCP_CLIENT_NAME } from "../branding.js";
+import {
+  captureMcpConnect,
+  captureMcpOauth,
+  classifyMcpConnectOutcome,
+  classifyMcpOauthOutcome,
+  NOOP_TELEMETRY,
+  type Telemetry,
+} from "../telemetry/index.js";
 
 export interface MCPTool {
   name: string;
@@ -94,6 +102,9 @@ export interface MCPConnectOptions {
   // learns the transport died under a live client. Never fired for
   // intentional teardown — close() disarms it before closing the transport.
   onDisconnect?: () => void;
+  // Optional telemetry handle for enum-only connect/oauth outcome events. No
+  // globals: callers that care pass their instance; otherwise nothing emits.
+  telemetry?: Telemetry;
 }
 
 export function unwrapToolContent(content: unknown): string {
@@ -137,6 +148,7 @@ interface HTTPAuthContext {
   interactive: boolean;
   serverName: string;
   onAuthorized?: (serverName: string) => void;
+  telemetry: Telemetry;
 }
 
 interface BrowserAuthFlow {
@@ -353,6 +365,13 @@ function gateRedirectToAuthorization(context: HTTPAuthContext): void {
       return promptEmitted;
     } catch (err) {
       unfreezePkce(coordinator);
+      // The cap throws before a browser flow exists, so this denial never
+      // reaches the instrumented recovery path below — emit here so the
+      // terminal denial maps to timeout like every other non-abort end.
+      captureMcpOauth(
+        context.telemetry,
+        classifyMcpOauthOutcome(err, coordinator.lifecycle.signal.aborted),
+      );
       throw err;
     }
   };
@@ -441,16 +460,28 @@ async function driveRecovery(
   }
 
   const browserFlow = coordinator.browserFlow;
-  await browserFlow.promptEmitted;
-  const code = await waitForBrowserAuthCode(context);
-  await new StreamableHTTPClientTransport(
-    context.url,
-    streamableHTTPTransportOptions(
-      context.authProvider,
-      coordinator.lifecycle.signal,
-    ),
-  ).finishAuth(code);
-  await coordinator.probe();
+  // One enum-only outcome per browser attempt: prompt, wait, token exchange,
+  // and probe either complete together or map the failure to exactly one of
+  // cancelled (explicit abort) / timeout (expiry, cap, denial, teardown).
+  try {
+    await browserFlow.promptEmitted;
+    const code = await waitForBrowserAuthCode(context);
+    await new StreamableHTTPClientTransport(
+      context.url,
+      streamableHTTPTransportOptions(
+        context.authProvider,
+        coordinator.lifecycle.signal,
+      ),
+    ).finishAuth(code);
+    await coordinator.probe();
+  } catch (error) {
+    captureMcpOauth(
+      context.telemetry,
+      classifyMcpOauthOutcome(error, coordinator.lifecycle.signal.aborted),
+    );
+    throw error;
+  }
+  captureMcpOauth(context.telemetry, "completed");
 }
 
 function completeVerifiedRecovery(
@@ -763,6 +794,7 @@ async function connectHttp(
         ...(options.onAuthorized !== undefined
           ? { onAuthorized: options.onAuthorized }
           : {}),
+        telemetry: options.telemetry ?? NOOP_TELEMETRY,
       };
       gateRedirectToAuthorization(authContext);
     }
@@ -825,16 +857,28 @@ 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[]> {
+  // Telemetry flows through options into connectMCPServer, which captures
+  // one mcp_connect per config.
   const results = await Promise.all(
     configs.map((c) => connectMCPServer(c, options)),
   );
@@ -845,6 +889,8 @@ export async function connectMCPServers(
       onWarning(
         `[mcp] Warning: failed to connect to MCP server "${result.serverName}": ${result.error}`,
       );
+    // No emit here: connectMCPServer already captured one enum-only
+    // mcp_connect per config. Emitting again would double-count.
   }
   return clients;
 }