From 8258717fbaabe682875498de06edf7bf6ceae3cf Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 1 Oct 2026 20:56:09 -0700 Subject: [PATCH] feat(telemetry): report auth success and MCP connection outcomes --- docs/TELEMETRY.md | 19 +- src/mcp/client-telemetry.test.ts | 380 ++++++++++++++++++++++++++ src/mcp/client.ts | 39 ++- src/telemetry/classify.ts | 70 +++++ src/telemetry/index.ts | 15 + src/telemetry/product-events.test.ts | 44 +++ src/telemetry/product-events.ts | 44 ++- src/tui/onboarding.ts | 3 + src/tui/provider-setup-submit.test.ts | 127 +++++++++ src/tui/provider/connect.ts | 2 + src/tui/provider/submit.ts | 9 + 11 files changed, 744 insertions(+), 8 deletions(-) create mode 100644 src/mcp/client-telemetry.test.ts diff --git a/docs/TELEMETRY.md b/docs/TELEMETRY.md index 7acf69ab7..fae9806d1 100644 --- a/docs/TELEMETRY.md +++ b/docs/TELEMETRY.md @@ -27,6 +27,9 @@ Each event carries a small set of properties: | `summarizer_failure` | The compaction summary call fails after its retry budget is spent | `provider`, `model`, `error_kind`, `duration_ms` | | `crash` | A fatal error reaches the process-level handler | `kind`, `error_class` | | `auth_failure` | A provider rejects the stored credentials | `auth_provider` | +| `auth_success` | Provider setup completes (OAuth login or validated API key) | `auth_provider` | +| `mcp_connect` | An MCP server connection attempt settles | `transport`, `result` | +| `mcp_oauth` | An MCP browser-OAuth callback wait settles | `result` | | `survey sent` | User submits intentional feedback via `/feedback` | `$survey_id`, `$survey_response`, `$survey_questions`, `turn_trace_id` | `compaction` is deliberately silent on the runs where the compactor decides @@ -106,13 +109,21 @@ on `crash` and nowhere else, so the column means one thing everywhere it is recorded. `auth_provider` is a separate property for that reason: it names which -provider's sign-in was rejected (`codex`, `xai`, `anthropic`, `other`), -chosen from a fixed first-party set in `src/tui/chrome-state.ts`. No -part of the provider's rejection message is sent. +provider's sign-in was rejected or completed (`codex`, `xai`, `anthropic`, +`other`), chosen from a fixed first-party set (`auth_failure` classifies in +`src/tui/chrome-state.ts`, `auth_success` in `src/telemetry/classify.ts`). +No part of the provider's message is sent. The MCP events follow the same +shape: `mcp_connect` carries `transport` (the connect path that actually ran, +`http` or `stdio`) and `result` (the settled outcome, `ok`, `auth`, `timeout`, +or `fail`), while `mcp_oauth` carries only `result` (`completed`, +`cancelled`, or `timeout`). Server names, URLs, commands, and error text never +leave the process. The mapping is `src/telemetry/classify.ts`, and the tests that feed each emission site a deliberately identifying name and assert it reaches no part of -the payload are in `src/telemetry/product-events.test.ts`. +the payload are in `src/telemetry/product-events.test.ts` for the TUI-side +events and `src/mcp/client-telemetry.test.ts` for the MCP connect and OAuth +events. ## AI observability events diff --git a/src/mcp/client-telemetry.test.ts b/src/mcp/client-telemetry.test.ts new file mode 100644 index 000000000..d2a295c9b --- /dev/null +++ b/src/mcp/client-telemetry.test.ts @@ -0,0 +1,380 @@ +// mcp_connect / mcp_oauth emission sites: every connect attempt reports one +// enum-only outcome, and every browser-OAuth callback wait reports one +// enum-only outcome. The leak assertions serialize the whole PostHog batch +// body (not just the intended property): a server name, URL, command path, or +// provider denial smuggled under another key must fail the test. + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + mockMcpCallbackServerModule, + mockMcpClientModule, + mockMcpOAuthProviderModule, + mockMcpTransportModule, + type MockAuthProvider, +} from "../../testkit/mcp-sdk-mock.js"; +import { + classifyMcpConnectResult, + classifyMcpOAuthResult, + classifyMcpTransport, +} from "../telemetry/classify.js"; +import { + createTelemetry, + NOOP_TELEMETRY, + type Telemetry, +} from "../telemetry/index.js"; +import { setTelemetry } from "../telemetry/singleton.js"; +import type { Settings } from "../config/settings.js"; + +interface BatchBody { + batch: { event: string; properties: Record }[]; +} + +function harness(): { + telemetry: Telemetry; + wire: () => Promise; + events: () => Promise; +} { + const bodies: BatchBody[] = []; + const fetchFn = ((_url: string, init: RequestInit) => { + bodies.push(JSON.parse(init.body as string) as BatchBody); + return Promise.resolve(new Response("1", { status: 200 })); + }) as unknown as typeof fetch; + const settings: Settings = { + providers: {}, + telemetry: { installationId: "install-id" }, + }; + const telemetry = createTelemetry({ + settings, + env: {}, + fetchFn, + apiKey: "test-key", + }); + const wire = async (): Promise => { + await telemetry.flush(); + return JSON.stringify(bodies); + }; + return { + telemetry, + wire, + events: async () => { + await telemetry.flush(); + return bodies.flatMap((body) => body.batch); + }, + }; +} + +// Identifying fixtures: an employer-named server, its internal URL, and its +// local command path. None of these strings may reach the wire. +const SERVER_NAME = "acme-internal-hr"; +const SERVER_URL = "https://hr.acme-internal.example.com/mcp"; +const SERVER_COMMAND = "/opt/acme-internal/bin/hr-server"; + +const mock = { + liveProvider: undefined as MockAuthProvider | undefined, + lastRequestSignal: undefined as AbortSignal | undefined, + connectFailuresLeft: 0, + failNextConnect: undefined as Error | undefined, + waitForCodeCalls: 0, + finishAuthCalls: 0, + callbackGate: undefined as Promise | undefined, +}; + +await mockMcpClientModule(mock, { + connect: async (provider) => { + if (mock.failNextConnect !== undefined) { + const err = mock.failNextConnect; + mock.failNextConnect = undefined; + throw err; + } + if (mock.connectFailuresLeft > 0) { + mock.connectFailuresLeft -= 1; + await provider?.redirectToAuthorization?.( + new URL("https://auth.test/authorize"), + ); + const { UnauthorizedError } = + await import("@modelcontextprotocol/sdk/client/auth.js"); + throw new UnauthorizedError("authorization required"); + } + }, + listTools: async () => ({ tools: [] }), + callTool: async () => ({ content: [] }), + close: async () => undefined, +}); + +await mockMcpTransportModule(mock, { + finishAuth: async () => { + mock.finishAuthCalls += 1; + }, +}); + +await mockMcpCallbackServerModule({ + // Mirrors the real loopback server: an abort rejects the pending wait, a + // rejected gate surfaces the provider's denial, otherwise the code arrives. + waitForCode: async (signal) => { + mock.waitForCodeCalls += 1; + if (mock.callbackGate !== undefined) { + await new Promise((resolve, reject) => { + if (signal?.aborted === true) { + reject(signal.reason); + return; + } + const onAbort = (): void => { + signal?.removeEventListener("abort", onAbort); + reject(signal?.reason); + }; + signal?.addEventListener("abort", onAbort, { once: true }); + void Promise.resolve(mock.callbackGate).then( + () => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, + (err: unknown) => { + signal?.removeEventListener("abort", onAbort); + reject(err); + }, + ); + }); + } + signal?.throwIfAborted(); + return "code-from-browser"; + }, + close: () => undefined, +}); + +await mockMcpOAuthProviderModule( + async (options: { + serverName: string; + onAuthURL: (serverName: string, authorizationUrl: string) => void; + }) => { + const provider = { + resetAuthorization: async () => undefined, + tokens: () => undefined, + refreshToken: async () => { + throw new Error("no refresh token"); + }, + saveCodeVerifier: async () => undefined, + codeVerifier: () => undefined, + redirectToAuthorization: (url: URL) => { + options.onAuthURL(options.serverName, url.toString()); + }, + }; + return provider; + }, +); + +const { + BROWSER_AUTH_WAIT_MS, + connectMCPServer, + resetBrowserAuthState, + setBrowserAuthWaitMs, +} = await import("./client.js"); + +beforeEach(() => { + mock.liveProvider = undefined; + mock.lastRequestSignal = undefined; + mock.connectFailuresLeft = 0; + mock.failNextConnect = undefined; + mock.waitForCodeCalls = 0; + mock.finishAuthCalls = 0; + mock.callbackGate = undefined; + resetBrowserAuthState(); +}); + +afterEach(() => { + resetBrowserAuthState(); + setBrowserAuthWaitMs(BROWSER_AUTH_WAIT_MS); + setTelemetry(NOOP_TELEMETRY); +}); + +describe("classifyMcpTransport", () => { + test("reports the connect-path predicate, never the server identity", () => { + expect(classifyMcpTransport({ type: "http" })).toBe("http"); + expect(classifyMcpTransport({ url: SERVER_URL })).toBe("http"); + // An http-typed entry wins even when a command is also set — the same + // predicate connectMCPServer dispatches on. + expect( + classifyMcpTransport({ type: "http", url: SERVER_URL, command: "x" }), + ).toBe("http"); + expect(classifyMcpTransport({ type: "stdio" })).toBe("stdio"); + expect(classifyMcpTransport({ command: SERVER_COMMAND })).toBe("stdio"); + expect(classifyMcpTransport({})).toBe("stdio"); + }); +}); + +describe("classifyMcpConnectResult", () => { + test("ok wins; an offered-but-unfinished browser auth reports auth", () => { + expect(classifyMcpConnectResult({ ok: true })).toBe("ok"); + expect(classifyMcpConnectResult({ ok: false, authPending: true })).toBe( + "auth", + ); + }); + + test("timeout-shaped failures report timeout; everything else is fail", () => { + expect( + classifyMcpConnectResult({ + ok: false, + error: "connect timed out after 5000ms", + }), + ).toBe("timeout"); + expect(classifyMcpConnectResult({ ok: false, error: "spawn ENOENT" })).toBe( + "fail", + ); + expect(classifyMcpConnectResult({ ok: false })).toBe("fail"); + }); +}); + +describe("classifyMcpOAuthResult", () => { + test("only a timed-out wait reports timeout", () => { + expect(classifyMcpOAuthResult(new Error("wait timed out"))).toBe("timeout"); + // Abandoned waits, closed servers, and provider denials (whose text is + // provider-authored) are all the operator not completing the flow. + expect(classifyMcpOAuthResult(new Error("aborted"))).toBe("cancelled"); + expect( + classifyMcpOAuthResult(new Error("Authorization failed: access_denied")), + ).toBe("cancelled"); + expect(classifyMcpOAuthResult("not an error")).toBe("cancelled"); + }); +}); + +describe("mcp_connect emission", () => { + test("a failed stdio spawn reports stdio/fail with no identity on the wire", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + // The OS refuses the spawn, exactly like a missing local binary in + // production; the error text carries the command path and must stay local. + mock.failNextConnect = new Error(`spawn ${SERVER_COMMAND} ENOENT`); + + const result = await connectMCPServer({ + name: SERVER_NAME, + command: SERVER_COMMAND, + }); + expect(result.ok).toBe(false); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["mcp_connect"]); + expect(captured[0]?.properties.transport).toBe("stdio"); + expect(captured[0]?.properties.result).toBe("fail"); + const body = await wire(); + expect(body).not.toContain("acme"); + expect(body).not.toContain(SERVER_COMMAND); + }); + + test("a clean http connect reports http/ok with no URL on the wire", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + + const result = await connectMCPServer({ + name: SERVER_NAME, + type: "http", + url: SERVER_URL, + oauth: false, + }); + expect(result.ok).toBe(true); + if (result.ok) await result.client.close(); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["mcp_connect"]); + expect(captured[0]?.properties.transport).toBe("http"); + expect(captured[0]?.properties.result).toBe("ok"); + const body = await wire(); + expect(body).not.toContain("acme"); + expect(body).not.toContain("hr.acme-internal.example.com"); + }); +}); + +describe("mcp_oauth emission", () => { + test("a completed browser flow reports completed, then the connect reports ok", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + mock.connectFailuresLeft = 1; + + const result = await connectMCPServer( + { name: SERVER_NAME, type: "http", url: SERVER_URL }, + { onAuthURL: () => undefined }, + ); + expect(result.ok).toBe(true); + if (result.ok) await result.client.close(); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["mcp_oauth", "mcp_connect"]); + expect(captured[0]?.properties.result).toBe("completed"); + expect(captured[1]?.properties.transport).toBe("http"); + expect(captured[1]?.properties.result).toBe("ok"); + expect(mock.finishAuthCalls).toBe(1); + expect(await wire()).not.toContain("acme"); + }); + + test("an expired callback wait reports timeout and the connect reports auth", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + setBrowserAuthWaitMs(20); + mock.connectFailuresLeft = Number.POSITIVE_INFINITY; + // The operator never completes the browser flow; only the deadline ends it. + mock.callbackGate = new Promise(() => undefined); + + const result = await connectMCPServer( + { name: SERVER_NAME, type: "http", url: SERVER_URL }, + { onAuthURL: () => undefined }, + ); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.authPending).toBe(true); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["mcp_oauth", "mcp_connect"]); + expect(captured[0]?.properties.result).toBe("timeout"); + expect(captured[1]?.properties.transport).toBe("http"); + expect(captured[1]?.properties.result).toBe("auth"); + expect(await wire()).not.toContain("acme"); + }); + + test("an aborted connect reports cancelled with no denial text on the wire", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + mock.connectFailuresLeft = Number.POSITIVE_INFINITY; + mock.callbackGate = new Promise(() => undefined); + const controller = new AbortController(); + + const connecting = connectMCPServer( + { name: SERVER_NAME, type: "http", url: SERVER_URL }, + { onAuthURL: () => undefined, signal: controller.signal }, + ); + while (mock.waitForCodeCalls === 0) await Promise.resolve(); + controller.abort(new Error("operator went away")); + + const result = await connecting; + expect(result.ok).toBe(false); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["mcp_oauth", "mcp_connect"]); + expect(captured[0]?.properties.result).toBe("cancelled"); + const body = await wire(); + expect(body).not.toContain("acme"); + expect(body).not.toContain("operator went away"); + }); + + test("a provider denial reports cancelled and never ships the denial text", async () => { + const { telemetry, wire, events } = harness(); + setTelemetry(telemetry); + mock.connectFailuresLeft = Number.POSITIVE_INFINITY; + // The provider refuses with its own error text, as a real denial does. + mock.callbackGate = Promise.reject( + new Error("Authorization failed: access_denied"), + ); + // Swallow the unhandled rejection surfacing through the gate handle: the + // wait path observes it via the mocked waitForCode await. + mock.callbackGate.catch(() => undefined); + + const result = await connectMCPServer( + { name: SERVER_NAME, type: "http", url: SERVER_URL }, + { onAuthURL: () => undefined }, + ); + expect(result.ok).toBe(false); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual(["mcp_oauth", "mcp_connect"]); + expect(captured[0]?.properties.result).toBe("cancelled"); + const body = await wire(); + expect(body).not.toContain("acme"); + expect(body).not.toContain("access_denied"); + }); +}); diff --git a/src/mcp/client.ts b/src/mcp/client.ts index 21337f238..944efc629 100644 --- a/src/mcp/client.ts +++ b/src/mcp/client.ts @@ -14,6 +14,16 @@ import type { ResolvedMCPServerConfig } from "./exa.js"; import type { McpToolAnnotations } from "./tool-permissions.js"; import { buildStdioMcpProcessEnv } from "./stdio-env.js"; import { isHttpServer } from "./is-http-server.js"; +import { + classifyMcpConnectResult, + classifyMcpOAuthResult, + classifyMcpTransport, +} from "../telemetry/classify.js"; +import { + captureMcpConnect, + captureMcpOAuth, +} from "../telemetry/product-events.js"; +import { liveTelemetry } from "../telemetry/singleton.js"; import { MCP_CLIENT_NAME } from "../branding.js"; export interface MCPTool { @@ -230,8 +240,22 @@ async function waitForBrowserAuthCode( AbortSignal.timeout(browserAuthWaitMs), ]); try { - return await context.callback.waitForCode(deadline); + const code = await context.callback.waitForCode(deadline); + captureMcpOAuth(liveTelemetry, { result: "completed" }); + return code; } catch (err) { + // waitForCode only rejects with "aborted" for both expiries, so the error + // text cannot tell a lapsed wait from an abandoned one — the signals can: + // a deadline abort with the lifecycle still alive means the wait expired. + // Anything else (lifecycle abort, close(), provider denial) is the + // operator not completing the flow. The denial text itself is + // provider-authored and never leaves the process. + captureMcpOAuth(liveTelemetry, { + result: + !lifecycle.aborted && deadline.aborted + ? "timeout" + : classifyMcpOAuthResult(err), + }); if (lifecycle.aborted) throw err; if (deadline.aborted) throw browserAuthWaitError(context.serverName); throw err; @@ -825,9 +849,18 @@ export async function connectMCPServer( config: ResolvedMCPServerConfig, options: MCPConnectOptions = {}, ): Promise { - return isHttpServer(config) + const result = await (isHttpServer(config) ? connectHttp(config, options) - : connectStdio(config, options); + : connectStdio(config, options)); + // The single funnel for every connect path (startup, late add, retry, + // backoff redial, web-search): one settled attempt reports one enum-only + // outcome. Transport is the connect-path predicate that actually ran, never + // the server name, URL, or command; the error text is never attached. + captureMcpConnect(liveTelemetry, { + transport: classifyMcpTransport(config), + result: classifyMcpConnectResult(result), + }); + return result; } export async function connectMCPServers( diff --git a/src/telemetry/classify.ts b/src/telemetry/classify.ts index b223f0c87..7f41a9746 100644 --- a/src/telemetry/classify.ts +++ b/src/telemetry/classify.ts @@ -11,6 +11,7 @@ // that something unrecognised was used, never what it was called. import { DIRECTOR_IDS } from "../agent/directors/types.js"; +import { isHttpServer } from "../mcp/is-http-server.js"; import { isMcpToolName } from "../mcp/tool-name.js"; const CUSTOM = "custom"; @@ -146,3 +147,72 @@ export function classifyErrorClass(error: unknown): string { ? error.constructor.name : CUSTOM; } + +// First-party provider enum shared by auth_failure and auth_success. The +// success path classifies structured setup data (the OAuth kind, or the +// picked preset's protocol flag) — never the settings catalog name, which is +// operator-authored free text and always buckets to "other". +export type AuthProvider = "codex" | "xai" | "anthropic" | "other"; + +export function classifyAuthProvider(value: string): AuthProvider { + switch (value) { + case "codex": + return "codex"; + case "xai": + return "xai"; + case "anthropic": + return "anthropic"; + default: + return "other"; + } +} + +// MCP connect-path transport. Delegates to the same predicate the trust +// prompt and connectMCPServer use, so the reported transport is the one that +// was actually opened — never the server name, URL, or command. Callers pass +// the full server config; only the routing fields are read. +export type McpTransport = "http" | "stdio"; + +export function classifyMcpTransport(config: { + type?: "stdio" | "http"; + url?: string; + command?: string; +}): McpTransport { + return isHttpServer(config) ? "http" : "stdio"; +} + +// Settled outcome of one MCP server connection attempt. The error text is +// provider- or OS-authored (paths, URLs, profiles), so only its shape is +// reported: offered-but-unfinished browser auth, an aborted/timed-out dial, +// or a plain failure. +export type McpConnectResult = "ok" | "auth" | "timeout" | "fail"; + +const CONNECT_TIMEOUT_MESSAGE = /\btimeout\b|\btimed out\b|\babort/i; + +export function classifyMcpConnectResult(outcome: { + ok: boolean; + authPending?: boolean; + error?: string; +}): McpConnectResult { + if (outcome.ok) return "ok"; + if (outcome.authPending === true) return "auth"; + if ( + outcome.error !== undefined && + CONNECT_TIMEOUT_MESSAGE.test(outcome.error) + ) + return "timeout"; + return "fail"; +} + +// Outcome of one MCP browser-OAuth callback wait, for the case where no code +// arrived. A wait that expired is a timeout; an abandoned wait or a provider +// denial (access_denied and friends) is the operator not completing the flow. +export type McpOAuthResult = "completed" | "cancelled" | "timeout"; + +const OAUTH_TIMEOUT_MESSAGE = /\btimed out\b/i; + +export function classifyMcpOAuthResult(err: unknown): "cancelled" | "timeout" { + if (err instanceof Error && OAUTH_TIMEOUT_MESSAGE.test(err.message)) + return "timeout"; + return "cancelled"; +} diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 90cbb2eb6..381cf46d0 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -63,6 +63,9 @@ export type TelemetryEvent = | "summarizer_failure" | "crash" | "auth_failure" + | "auth_success" + | "mcp_connect" + | "mcp_oauth" // PostHog Surveys event name (space included). Intentional operator feedback // from /feedback — can ship when ambient product telemetry is off; still // blocked by env kill switches. See captureIntentional. @@ -199,6 +202,18 @@ const EVENT_PROPERTY_ALLOWLIST: Record = { // Which provider rejected the credentials, not why — the rejection detail is // provider-authored text and error_class means a JS constructor name. auth_failure: ["auth_provider"], + // Which provider accepted the credentials during provider setup (OAuth login + // or validated API key), as the same first-party enum auth_failure reports — + // never the settings catalog name, which is operator-authored free text. + auth_success: ["auth_provider"], + // The outcome of one MCP server connection attempt: transport is the + // connect-path predicate (http vs stdio), never the server name or URL, and + // result is the settled outcome, never the error text. + mcp_connect: ["transport", "result"], + // The outcome of one MCP browser-OAuth callback wait: completed when the + // authorization code arrived, cancelled when the operator abandoned or + // denied it, timed out when the wait expired. Carries no server identity. + mcp_oauth: ["result"], // Intentional /feedback survey response (PostHog custom survey capture shape). // Free text is only sent because the operator typed it for that purpose. // turn_trace_id links to the last $ai_generation in this session when known. diff --git a/src/telemetry/product-events.test.ts b/src/telemetry/product-events.test.ts index bd163981c..85a520aec 100644 --- a/src/telemetry/product-events.test.ts +++ b/src/telemetry/product-events.test.ts @@ -26,6 +26,7 @@ import { unlimitedAdmissionQueue } from "../subagent/admission.js"; import { createSubAgentSessionStore } from "../subagent/session-store.js"; import { classifyAgentName, + classifyAuthProvider, classifyErrorClass, classifyPermissionKind, classifySkillName, @@ -34,6 +35,7 @@ import { import { createTelemetry, NOOP_TELEMETRY, type Telemetry } from "./index.js"; import { buildSubagentEndProperties, + captureAuthSuccess, captureSkillUsed, captureSlashCommand, createPluginLoadReporter, @@ -660,6 +662,48 @@ test("auth_failure names the provider and never ships the rejection message", as expect(body).not.toContain("error_class"); }); +// --------------------------------------------------------------------------- +// auth_success — the settings catalog name is operator free text, so only the +// classified enum leaves the process +// --------------------------------------------------------------------------- + +test("auth_success reports the classified provider and never the settings name", async () => { + const { telemetry, wire, events } = harness(); + + captureAuthSuccess(telemetry, classifyAuthProvider("codex")); + captureAuthSuccess(telemetry, classifyAuthProvider("xai")); + captureAuthSuccess(telemetry, classifyAuthProvider("anthropic")); + // An employer-named settings entry funnels to "other" instead of shipping + // the name: the input type is the closed enum, so reaching this branch + // means a value nobody classified crossed the boundary. + captureAuthSuccess( + telemetry, + classifyAuthProvider("acmecorp-eng" as "other"), + ); + + const captured = await events(); + expect(captured.map((e) => e.event)).toEqual([ + "auth_success", + "auth_success", + "auth_success", + "auth_success", + ]); + expect(captured.map((e) => e.properties.auth_provider)).toEqual([ + "codex", + "xai", + "anthropic", + "other", + ]); + expect(await wire()).not.toContain("acmecorp"); +}); + +test("classifyAuthProvider passes the closed enum through", () => { + expect(classifyAuthProvider("codex")).toBe("codex"); + expect(classifyAuthProvider("xai")).toBe("xai"); + expect(classifyAuthProvider("anthropic")).toBe("anthropic"); + expect(classifyAuthProvider("other")).toBe("other"); +}); + // --------------------------------------------------------------------------- // compaction — must not fire when the compactor did nothing // --------------------------------------------------------------------------- diff --git a/src/telemetry/product-events.ts b/src/telemetry/product-events.ts index 1eff10728..1e22173a4 100644 --- a/src/telemetry/product-events.ts +++ b/src/telemetry/product-events.ts @@ -6,7 +6,14 @@ import type { SubAgentTerminalReason, } from "../subagent/types.js"; import type { Telemetry } from "./index.js"; -import { classifyCommandName, classifySkillName } from "./classify.js"; +import { + type AuthProvider, + type McpConnectResult, + type McpOAuthResult, + type McpTransport, + classifyCommandName, + classifySkillName, +} from "./classify.js"; /** Emit slash_command with a classified first-party (or `custom`) name. */ export function captureSlashCommand( @@ -101,3 +108,38 @@ export function createPluginLoadReporter(): PluginLoadReporter { loadedPluginIdentities.add(identity); }; } + +/** Emit auth_success with the classified first-party provider. */ +export function captureAuthSuccess( + telemetry: Telemetry, + provider: AuthProvider, +): void { + if (!telemetry.enabled) return; + telemetry.capture("auth_success", { auth_provider: provider }); +} + +export interface CaptureMcpConnectArgs { + transport: McpTransport; + result: McpConnectResult; +} + +/** Emit mcp_connect with the classified transport and settled outcome. */ +export function captureMcpConnect( + telemetry: Telemetry, + args: CaptureMcpConnectArgs, +): void { + if (!telemetry.enabled) return; + telemetry.capture("mcp_connect", { + transport: args.transport, + result: args.result, + }); +} + +/** Emit mcp_oauth with the classified browser-callback outcome. */ +export function captureMcpOAuth( + telemetry: Telemetry, + args: { result: McpOAuthResult }, +): void { + if (!telemetry.enabled) return; + telemetry.capture("mcp_oauth", { result: args.result }); +} diff --git a/src/tui/onboarding.ts b/src/tui/onboarding.ts index 0a86ed49d..d5d32d0fc 100644 --- a/src/tui/onboarding.ts +++ b/src/tui/onboarding.ts @@ -11,6 +11,7 @@ import { activateHeldTelemetry, telemetryFirstRunPending, } from "../telemetry/first-run.js"; +import { getTelemetry } from "../telemetry/singleton.js"; import { runProviderSetup } from "./provider/setup.js"; import { runWelcome } from "./welcome.js"; @@ -52,6 +53,8 @@ export async function runOnboarding( settingsPath, existing, resolveLocalSettingsPath(config.cwd, settingsPath), + undefined, + getTelemetry(), ), }); diff --git a/src/tui/provider-setup-submit.test.ts b/src/tui/provider-setup-submit.test.ts index d27691ba4..37c53ee20 100644 --- a/src/tui/provider-setup-submit.test.ts +++ b/src/tui/provider-setup-submit.test.ts @@ -5,6 +5,7 @@ import { join } from "node:path"; import type { OAuthScopeCheckResult } from "../auth/oauth-scope-check.js"; import { COMMAND_NAME } from "../branding.js"; +import type { Telemetry, TelemetryEvent } from "../telemetry/index.js"; import { withMockedModule } from "../../testkit/mock-module.js"; // The oauth branch probes real provider scope over the network; stub the @@ -643,4 +644,130 @@ describe("buildProviderSubmitHandler", () => { }); }); }); + + describe("auth_success telemetry", () => { + function recordingTelemetry(): { + telemetry: Telemetry; + events: { + event: TelemetryEvent; + properties: Record | undefined; + }[]; + } { + const events: { + event: TelemetryEvent; + properties: Record | undefined; + }[] = []; + const telemetry: Telemetry = { + enabled: true, + installationId: "test-installation", + capture: (event, properties) => { + events.push({ event, properties }); + }, + captureIntentional: () => false, + flush: async () => undefined, + discard: () => undefined, + }; + return { telemetry, events }; + } + + test("OAuth success reports the OAuth kind, not the settings name", async () => { + await withTempDir(async (dir) => { + const { telemetry, events } = recordingTelemetry(); + const submit = buildProviderSubmitHandler( + join(dir, "settings.json"), + null, + localSettingsPath(dir), + undefined, + telemetry, + ); + + await submit( + { + name: "", + baseURL: "https://chatgpt.com/backend-api", + apiKey: "", + model: "gpt-5", + oauthProfile: "work", + }, + noopSetPhase, + { skipValidation: true, oauth: stagedCodexOAuth() }, + ); + + expect(events).toEqual([ + { event: "auth_success", properties: { auth_provider: "codex" } }, + ]); + }); + }); + + test("API-key success reports anthropic only for the Anthropic preset", async () => { + await withTempDir(async (dir) => { + const { telemetry, events } = recordingTelemetry(); + const submit = buildProviderSubmitHandler( + join(dir, "settings.json"), + null, + localSettingsPath(dir), + undefined, + telemetry, + ); + const values: ProviderFormValues = { + name: "acmecorp-proxy", + baseURL: "https://api.acmecorp.example/v1", + apiKey: "sk-test-fake", + model: "gpt-5", + oauthProfile: "", + }; + + await submit(values, noopSetPhase, { + skipValidation: true, + preset: { + id: "openai", + models: ["gpt-5"], + anthropic: false, + opencodeGo: false, + }, + }); + + expect(events).toEqual([ + { event: "auth_success", properties: { auth_provider: "other" } }, + ]); + }); + }); + + test("failed validation emits no auth_success", async () => { + await withTempDir(async (dir) => { + const { telemetry, events } = recordingTelemetry(); + const submit = buildProviderSubmitHandler( + join(dir, "settings.json"), + null, + localSettingsPath(dir), + undefined, + telemetry, + ); + + await expect( + submit( + { + name: "openai", + baseURL: "https://api.openai.com/v1", + apiKey: "", + model: "gpt-5", + oauthProfile: "", + }, + noopSetPhase, + { + skipValidation: false, + preset: { + id: "openai", + models: ["gpt-5"], + anthropic: false, + opencodeGo: false, + }, + }, + ), + ).rejects.toThrow(/api key/i); + + expect(events).toEqual([]); + }); + }); + }); }); diff --git a/src/tui/provider/connect.ts b/src/tui/provider/connect.ts index c047b00d9..dd02c5400 100644 --- a/src/tui/provider/connect.ts +++ b/src/tui/provider/connect.ts @@ -6,6 +6,7 @@ */ import type { Settings } from "../../config/settings.js"; +import { getTelemetry } from "../../telemetry/singleton.js"; import { runProviderSetup } from "./setup.js"; import type { ProviderSetupConfig } from "./types.js"; import { @@ -47,6 +48,7 @@ export async function connectProviderInline( input.existing, input.localSettingsPath, input.persistSettings, + getTelemetry(), ); const submitted = await runProviderSetup({ diff --git a/src/tui/provider/submit.ts b/src/tui/provider/submit.ts index 4d10fcdb0..c4354bce2 100644 --- a/src/tui/provider/submit.ts +++ b/src/tui/provider/submit.ts @@ -17,6 +17,9 @@ import { isBlockingOAuthScopeCheckResult, } from "../../auth/oauth-scope-check.js"; import type { ProviderSetupSubmit } from "./types.js"; +import { NOOP_TELEMETRY, type Telemetry } from "../../telemetry/index.js"; +import { classifyAuthProvider } from "../../telemetry/classify.js"; +import { captureAuthSuccess } from "../../telemetry/product-events.js"; /** * Persist the project-local provider/model selection after a successful @@ -59,6 +62,7 @@ export function buildProviderSubmitHandler( await saveGlobalSettings(settingsPath, next); return next; }, + telemetry: Telemetry = NOOP_TELEMETRY, ): ProviderSetupSubmit { return async (values, setPhase, { skipValidation, preset, oauth }) => { const { name, baseURL, apiKey, model } = values; @@ -108,6 +112,7 @@ export function buildProviderSubmitHandler( oauth.providerName, selectedModel, ); + captureAuthSuccess(telemetry, classifyAuthProvider(oauth.kind)); return; } @@ -175,5 +180,9 @@ export function buildProviderSubmitHandler( providerName, selectedModel, ); + captureAuthSuccess( + telemetry, + classifyAuthProvider(preset?.anthropic === true ? "anthropic" : "other"), + ); }; }