fix(mcp): attach mcp_tool/error_code to the remote sink's exception capture

The remote MCP telemetry sink's captureException call named these two
grouping properties in a comment but never actually sent them, so a
PostHog exception breakdown by tool/cause silently dropped the surface
with the most traffic while the stdio and miner sinks kept reporting.
Extend capturePostHogWorkerError with an optional, scrubbed extra
properties argument and pass mcp_tool/error_code through it, matching
the other two sinks property-for-property. The HTTP middleware path
keeps its existing request_path/request_method shape unchanged.

Author: bitfathers94

src/api/worker-posthog.ts

@@ -99,16 +99,29 @@ export interface WorkerErrorRequestContext {
 /** Capture one exception from the hosted Worker path. Never throws -- a PostHog init/capture/flush failure
  *  degrades to recording nothing, matching every other capture* function in this codebase's identical
  *  best-effort guarantee. Resolves once the event has actually been flushed (or given up on), so the caller
- *  should schedule this via ctx.waitUntil rather than await it inline on the request's hot path. */
-export async function capturePostHogWorkerError(env: WorkerPostHogEnv, error: unknown, context: WorkerErrorRequestContext): Promise<void> {
+ *  should schedule this via ctx.waitUntil rather than await it inline on the request's hot path.
+ *
+ *  `extraProperties` lets a non-HTTP caller (the MCP dispatch sink, #10037) attach its own grouping
+ *  properties (e.g. `mcp_tool`/`error_code`) alongside the fixed `environment`/`request_path`/
+ *  `request_method` shape -- scrubbed the same way those are, then merged in. Omitting it leaves
+ *  `createWorkerPostHogErrorMiddleware`'s HTTP callers unaffected. */
+export async function capturePostHogWorkerError(
+  env: WorkerPostHogEnv,
+  error: unknown,
+  context: WorkerErrorRequestContext,
+  extraProperties?: Record<string, unknown>,
+): Promise<void> {
   try {
     const client = await buildClient(env);
     if (!client) return;
     const err = error instanceof Error ? error : new Error(String(error));
+    const scrubbedExtra = extraProperties ? { ...extraProperties } : undefined;
+    if (scrubbedExtra) scrubRecord(scrubbedExtra, 0);
     client.captureException(err, WORKER_ERROR_DISTINCT_ID, {
       environment: trimmedOrUndefined(env.WORKER_POSTHOG_ENVIRONMENT) ?? "production",
       request_path: scrubString(context.path),
       request_method: context.method,
+      ...scrubbedExtra,
     });
     await client.flush();
   } catch {

src/mcp/dispatch-telemetry-sink.ts

@@ -102,7 +102,10 @@ export function createDispatchTelemetrySink(
       if (!isWorkerPostHogConfigured(env)) return;
       // `mcp_tool` + `error_code` are the grouping properties: an exception dashboard broken down by
       // tool and cause is the thing an operator can act on, unlike a stack-only view.
-      defer(capturePostHogWorkerError(env, error, { path: `mcp.tool/${call.tool}`, method: call.errorCode ?? "unknown_error" }));
+      const errorCode = call.errorCode ?? "unknown_error";
+      defer(
+        capturePostHogWorkerError(env, error, { path: `mcp.tool/${call.tool}`, method: errorCode }, { mcp_tool: call.tool, error_code: errorCode }),
+      );
     },
     // The registry is consulted per call rather than captured at construction so a self-host boot
     // that fills the slot after the first request still traces.

test/unit/mcp-dispatch-telemetry-sink.test.ts

@@ -138,6 +138,60 @@ describe("MCP dispatch telemetry sink (#9525)", () => {
   });
 });
 
+// The exception properties tests below mock posthog-node so they can read the properties the sink
+// actually hands to captureException, instead of only observing whether the deferred promise resolves.
+describe("MCP dispatch telemetry sink exception properties (#10037)", () => {
+  afterEach(() => {
+    vi.doUnmock("posthog-node");
+    vi.resetModules();
+  });
+
+  it("attaches mcp_tool and error_code to the captured exception, matching the stdio/miner sinks", async () => {
+    vi.resetModules();
+    const captureException = vi.fn();
+    const flush = vi.fn().mockResolvedValue(undefined);
+    vi.doMock("posthog-node", () => ({
+      PostHog: vi.fn(function (this: { captureException: typeof captureException; flush: typeof flush }) {
+        this.captureException = captureException;
+        this.flush = flush;
+      }),
+    }));
+    const { createDispatchTelemetrySink: freshCreateDispatchTelemetrySink } = await import("../../src/mcp/dispatch-telemetry-sink");
+    const deferred: Promise<unknown>[] = [];
+    const sink = freshCreateDispatchTelemetrySink(env({ WORKER_POSTHOG_API_KEY: "phc_worker" }), (work) => deferred.push(work));
+    const forbiddenCall: McpToolCallTelemetry = { ...call, ok: false, errorCode: "forbidden" };
+
+    sink.captureException(new Error("boom"), forbiddenCall);
+    expect(deferred).toHaveLength(1);
+    await deferred[0];
+
+    const properties = captureException.mock.calls.at(-1)?.[2] as Record<string, unknown>;
+    expect(properties).toMatchObject({ mcp_tool: forbiddenCall.tool, error_code: "forbidden" });
+  });
+
+  it("defaults error_code to unknown_error when the call carries none", async () => {
+    vi.resetModules();
+    const captureException = vi.fn();
+    const flush = vi.fn().mockResolvedValue(undefined);
+    vi.doMock("posthog-node", () => ({
+      PostHog: vi.fn(function (this: { captureException: typeof captureException; flush: typeof flush }) {
+        this.captureException = captureException;
+        this.flush = flush;
+      }),
+    }));
+    const { createDispatchTelemetrySink: freshCreateDispatchTelemetrySink } = await import("../../src/mcp/dispatch-telemetry-sink");
+    const deferred: Promise<unknown>[] = [];
+    const sink = freshCreateDispatchTelemetrySink(env({ WORKER_POSTHOG_API_KEY: "phc_worker" }), (work) => deferred.push(work));
+    const noCodeCall: McpToolCallTelemetry = { ...call, ok: false };
+
+    sink.captureException(new Error("boom"), noCodeCall);
+    await deferred[0];
+
+    const properties = captureException.mock.calls.at(-1)?.[2] as Record<string, unknown>;
+    expect(properties).toMatchObject({ mcp_tool: noCodeCall.tool, error_code: "unknown_error" });
+  });
+});
+
 describe("LoopoverMcp telemetry-sink injection (#9525)", () => {
   it("routes a real tool call through the injected sink", async () => {
     const { Client } = await import("@modelcontextprotocol/sdk/client/index.js");

test/unit/worker-posthog.test.ts

@@ -149,6 +149,30 @@ describe("capturePostHogWorkerError", () => {
     expect(flushed).toBe(true);
   });
 
+  it("merges extraProperties into the captured exception alongside the fixed request shape (#10037)", async () => {
+    await capturePostHogWorkerError({ WORKER_POSTHOG_API_KEY: "phc_test" } as WorkerPostHogEnv, new Error("boom"), { path: "mcp.tool/loopover_x", method: "forbidden" }, {
+      mcp_tool: "loopover_x",
+      error_code: "forbidden",
+    });
+    const properties = mocks.captureException.mock.calls.at(-1)?.[2] as Record<string, unknown>;
+    expect(properties).toMatchObject({
+      environment: "production",
+      request_path: "mcp.tool/loopover_x",
+      request_method: "forbidden",
+      mcp_tool: "loopover_x",
+      error_code: "forbidden",
+    });
+  });
+
+  it("scrubs a secret-shaped extraProperties value the same way request_path is scrubbed (#10037)", async () => {
+    await capturePostHogWorkerError({ WORKER_POSTHOG_API_KEY: "phc_test" } as WorkerPostHogEnv, new Error("boom"), { path: "/x", method: "GET" }, {
+      leaked_token: `${"github" + "_pat_"}${"a".repeat(24)}`,
+    });
+    const properties = mocks.captureException.mock.calls.at(-1)?.[2] as Record<string, unknown>;
+    expect(properties.leaked_token).not.toContain("github_pat_");
+    expect(properties.leaked_token).toContain("[redacted]");
+  });
+
   it("never throws when the PostHog client construction fails", async () => {
     mocks.PostHog.mockImplementationOnce(() => {
       throw new Error("client construction failed");
@@ -223,6 +247,16 @@ describe("createWorkerPostHogErrorMiddleware", () => {
     expect((properties as Record<string, unknown>).request_method).toBe("GET");
   });
 
+  it("still carries only request_path/request_method, never mcp_tool, when no extraProperties argument is passed (#10037)", async () => {
+    const { app, executionCtx, getWaited } = buildTestApp();
+    const res = await app.fetch(new Request("https://loopover.test/boom"), { WORKER_POSTHOG_API_KEY: "phc_test" } as WorkerPostHogEnv, executionCtx);
+    expect(res.status).toBe(500);
+    await getWaited();
+    const properties = mocks.captureException.mock.calls.at(-1)?.[2] as Record<string, unknown>;
+    expect(properties).toMatchObject({ request_path: "/boom", request_method: "GET" });
+    expect(properties).not.toHaveProperty("mcp_tool");
+  });
+
   it("falls back to a no-op executionCtx when c.executionCtx throws (self-host calling the same Worker fetch handler outside a real isolate)", async () => {
     const app = new Hono<{ Bindings: WorkerPostHogEnv }>();
     app.use(createWorkerPostHogErrorMiddleware());