test(mcp): cover the telemetry sink, the span registry, and the miner chokepoint

codecov/patch flagged all three: the wrapper in dispatch-telemetry.ts had tests,
the I/O half had none. Both sides of every gate now, including the two that are
deliberately independent -- exception capture on with usage events off -- plus
the never-rejects guarantee on the deferred capture with no reachable host.

Also drops a redundant API-key guard inside captureEvents. Its only caller
already gates on the key, so the second check was unreachable by construction;
it now takes the resolved key instead of re-deriving one it cannot fail to
find.

Author: JSONbored

src/mcp/dispatch-telemetry-sink.ts

@@ -37,9 +37,13 @@ function trimmedOrUndefined(value: string | undefined): string | undefined {
 /** Deferred work the caller schedules via `waitUntil`, so a slow flush never delays the response. */
 export type DeferWork = (work: Promise<unknown>) => void;
 
-async function captureEvents(env: DispatchTelemetryEnv, properties: { usage: Record<string, unknown>; mcpToolCall: Record<string, unknown> }): Promise<void> {
-  const apiKey = trimmedOrUndefined(env.POSTHOG_API_KEY);
-  if (!apiKey) return;
+/** Only ever reached from behind `recordToolCall`'s gate, so it takes the resolved key rather than
+ *  re-deriving and re-checking it -- a second guard here would be unreachable by construction. */
+async function captureEvents(
+  env: DispatchTelemetryEnv,
+  apiKey: string,
+  properties: { usage: Record<string, unknown>; mcpToolCall: Record<string, unknown> },
+): Promise<void> {
   try {
     const { PostHog } = await import("posthog-node");
     const client = new PostHog(apiKey, {
@@ -76,8 +80,9 @@ export function createDispatchTelemetrySink(
 ): DispatchTelemetrySink {
   return {
     recordToolCall: (_call, properties) => {
-      if (!trimmedOrUndefined(env.POSTHOG_API_KEY)) return;
-      defer(captureEvents(env, properties));
+      const apiKey = trimmedOrUndefined(env.POSTHOG_API_KEY);
+      if (!apiKey) return;
+      defer(captureEvents(env, apiKey, properties));
     },
     captureException: (error, call) => {
       if (!isWorkerPostHogConfigured(env)) return;

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

@@ -0,0 +1,121 @@
+// The remote server's telemetry sink and span registry (#9525).
+//
+// The wrapper in dispatch-telemetry.ts is pure and covered separately; this covers the I/O half --
+// both sides of every gate, and the guarantee that a sink failure never reaches the tool caller.
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type { McpToolCallTelemetry } from "@loopover/contract";
+import { createDispatchTelemetrySink, type DispatchTelemetryEnv } from "../../src/mcp/dispatch-telemetry-sink";
+import {
+  getMcpDispatchSpanRunner,
+  resetMcpDispatchSpanRunnerForTest,
+  setMcpDispatchSpanRunner,
+} from "../../src/mcp/dispatch-span-registry";
+
+const call: McpToolCallTelemetry = { tool: "loopover_get_repo_context", category: "maintainer", surface: "remote", ok: true, durationMs: 4 };
+const properties = { usage: { tool: call.tool }, mcpToolCall: { tool: call.tool } };
+
+function env(overrides: Partial<DispatchTelemetryEnv> = {}): DispatchTelemetryEnv {
+  return overrides as DispatchTelemetryEnv;
+}
+
+afterEach(() => {
+  resetMcpDispatchSpanRunnerForTest();
+  vi.restoreAllMocks();
+});
+
+describe("MCP dispatch span registry (#9525)", () => {
+  it("is empty until a self-host boot fills it, and clears again", () => {
+    expect(getMcpDispatchSpanRunner()).toBeUndefined();
+    const runner = async <T>(_name: string, _attributes: Record<string, unknown>, fn: () => Promise<T>): Promise<T> => fn();
+    setMcpDispatchSpanRunner(runner);
+    expect(getMcpDispatchSpanRunner()).toBe(runner);
+    setMcpDispatchSpanRunner(null);
+    expect(getMcpDispatchSpanRunner()).toBeUndefined();
+  });
+});
+
+describe("MCP dispatch telemetry sink (#9525)", () => {
+  it("records nothing and defers nothing when POSTHOG_API_KEY is unset", () => {
+    const deferred: Promise<unknown>[] = [];
+    const sink = createDispatchTelemetrySink(env(), (work) => deferred.push(work));
+    sink.recordToolCall(call, properties);
+    expect(deferred).toEqual([]);
+  });
+
+  it("treats a blank POSTHOG_API_KEY as unset", () => {
+    const deferred: Promise<unknown>[] = [];
+    const sink = createDispatchTelemetrySink(env({ POSTHOG_API_KEY: "   " }), (work) => deferred.push(work));
+    sink.recordToolCall(call, properties);
+    expect(deferred).toEqual([]);
+  });
+
+  it("defers one capture when the key is set, and never rejects even with no reachable host", async () => {
+    const deferred: Promise<unknown>[] = [];
+    const sink = createDispatchTelemetrySink(env({ POSTHOG_API_KEY: "phc_test", POSTHOG_HOST: "http://127.0.0.1:1" }), (work) => deferred.push(work));
+    sink.recordToolCall(call, properties);
+    expect(deferred).toHaveLength(1);
+    // The never-throws guarantee: a PostHog init/capture/flush failure records nothing and resolves.
+    await expect(deferred[0]).resolves.toBeUndefined();
+  });
+
+  it("falls back to the US-cloud host when POSTHOG_HOST is unset", async () => {
+    const deferred: Promise<unknown>[] = [];
+    const sink = createDispatchTelemetrySink(env({ POSTHOG_API_KEY: "phc_test" }), (work) => deferred.push(work));
+    sink.recordToolCall(call, properties);
+    expect(deferred).toHaveLength(1);
+    // Reaches the real default host and fails there; the guarantee under test is that it resolves
+    // rather than rejecting into the tool caller.
+    await expect(deferred[0]).resolves.toBeUndefined();
+  }, 20_000);
+
+  it("captures nothing when the Worker exception key is unset", () => {
+    const deferred: Promise<unknown>[] = [];
+    const sink = createDispatchTelemetrySink(env({ POSTHOG_API_KEY: "phc_test" }), (work) => deferred.push(work));
+    sink.captureException(new Error("boom"), call);
+    expect(deferred).toEqual([]);
+  });
+
+  it("defers an exception capture when the Worker key IS set -- a separate gate from the usage one", async () => {
+    const deferred: Promise<unknown>[] = [];
+    const sink = createDispatchTelemetrySink(
+      env({ WORKER_POSTHOG_API_KEY: "phc_worker", WORKER_POSTHOG_HOST: "http://127.0.0.1:1" }),
+      (work) => deferred.push(work),
+    );
+    // No POSTHOG_API_KEY here: the two gates are deliberately independent (see the sink's header),
+    // so exception capture is on while usage events stay off.
+    sink.recordToolCall(call, properties);
+    sink.captureException(new Error("boom"), call);
+    expect(deferred).toHaveLength(1);
+    await expect(deferred[0]).resolves.toBeUndefined();
+  });
+
+  it("passes the call through untouched when no span runner is registered", async () => {
+    const sink = createDispatchTelemetrySink(env(), () => undefined);
+    await expect(sink.withSpan("mcp.tool/x", { tool: "x" }, async () => "through")).resolves.toBe("through");
+  });
+
+  it("uses the registry's runner when a self-host boot has filled it", async () => {
+    const seen: Array<{ name: string; attributes: Record<string, unknown> }> = [];
+    setMcpDispatchSpanRunner(async (name, attributes, fn) => {
+      seen.push({ name, attributes });
+      return fn();
+    });
+    const sink = createDispatchTelemetrySink(env(), () => undefined);
+    await expect(sink.withSpan("mcp.tool/x", { tool: "x" }, async () => "wrapped")).resolves.toBe("wrapped");
+    expect(seen).toEqual([{ name: "mcp.tool/x", attributes: { tool: "x" } }]);
+  });
+
+  it("prefers an explicitly injected runner over the registry", async () => {
+    setMcpDispatchSpanRunner(async () => {
+      throw new Error("registry runner should not have been used");
+    });
+    let injectedCalls = 0;
+    const injected = async <T>(_name: string, _attributes: Record<string, unknown>, fn: () => Promise<T>): Promise<T> => {
+      injectedCalls += 1;
+      return fn();
+    };
+    const sink = createDispatchTelemetrySink(env(), () => undefined, injected);
+    await expect(sink.withSpan("mcp.tool/x", {}, async () => "injected")).resolves.toBe("injected");
+    expect(injectedCalls).toBe(1);
+  });
+});

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

@@ -0,0 +1,31 @@
+// The miner MCP server's dispatch-telemetry chokepoint (#9525).
+//
+// Its sink is this package's own opt-in PostHog client, so both sides of that gate are driven here
+// via the module's exported reset helper rather than by mocking the SDK.
+import { afterEach, describe, expect, it } from "vitest";
+import { recordMinerDispatchTelemetry } from "../../packages/loopover-miner/lib/mcp-dispatch-telemetry";
+import { resetMinerPostHogForTesting } from "../../packages/loopover-miner/lib/posthog";
+
+afterEach(() => {
+  resetMinerPostHogForTesting();
+});
+
+describe("miner dispatch telemetry (#9525)", () => {
+  it("is a silent no-op when the miner's PostHog client is not initialized", () => {
+    expect(() =>
+      recordMinerDispatchTelemetry({ tool: "loopover_miner_ping", ok: true, durationMs: 3, args: {}, result: { status: "ok" } }),
+    ).not.toThrow();
+  });
+
+  it("never throws on the failure path, with or without an error value", () => {
+    expect(() => recordMinerDispatchTelemetry({ tool: "loopover_miner_ping", ok: false, durationMs: 3, error: new Error("boom") })).not.toThrow();
+    // `error` absent on a failed call: the exception-capture arm must be skipped, not passed undefined.
+    expect(() => recordMinerDispatchTelemetry({ tool: "loopover_miner_ping", ok: false, durationMs: 3 })).not.toThrow();
+  });
+
+  it("tolerates a tool with no contract entry rather than throwing on the path it instruments", () => {
+    // The contract validator (#9520) makes this unreachable in practice; telemetry still must not be
+    // the thing that breaks a tool call if it ever happens.
+    expect(() => recordMinerDispatchTelemetry({ tool: "loopover_not_in_the_registry", ok: true, durationMs: 1, args: { a: 1 } })).not.toThrow();
+  });
+});