Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
runIssueRagRetrieval,
validateIssueRagInput,
} from "./issue-rag";
import { recordMcpToolCall } from "./telemetry";
import {
authenticatePrivateToken,
extractBearerToken,
Expand Down Expand Up @@ -1631,6 +1632,9 @@ export async function handleMcpRequest(c: AppContext): Promise<Response> {
const server = new LoopoverMcp(c.env, identity).createServer();
try {
const response = await createMcpHandler(server, { route: "/mcp", enableJsonResponse: true })(c.req.raw, c.env, getExecutionContext(c));
if (typeof usageMetadata.toolName === "string") {
recordMcpToolTelemetry(c.env, usageMetadata.toolName, response.status < 400, Date.now() - startedAt);
}
await recordProductUsageEvent(c.env, {
surface: "mcp",
role: "miner",
Expand All @@ -1646,6 +1650,9 @@ export async function handleMcpRequest(c: AppContext): Promise<Response> {
}).catch(() => undefined);
return response;
} catch (error) {
if (typeof usageMetadata.toolName === "string") {
recordMcpToolTelemetry(c.env, usageMetadata.toolName, false, Date.now() - startedAt);
}
await recordProductUsageEvent(c.env, {
surface: "mcp",
role: "miner",
Expand All @@ -1663,6 +1670,18 @@ export async function handleMcpRequest(c: AppContext): Promise<Response> {
}
}

// Single chokepoint for the #6228 PostHog tool-call telemetry (#6237): every `tools/call` request that
// reaches handleMcpRequest routes through here exactly once, whether it succeeds or throws. Pure
// observability -- never lets a telemetry failure reach the caller, matching recordMcpToolCall's own
// no-op guarantee (#6235) with a second, defensive layer at the actual call site.
function recordMcpToolTelemetry(env: Env, tool: string, ok: boolean, durationMs: number): void {
try {
recordMcpToolCall(env, { tool, callerType: "remote", ok, durationMs });
} catch {
// Telemetry must never affect the tool response (#6237).
}
}

async function describeMcpUsageRequest(request: Request, telemetryMetadata: Record<string, unknown> | undefined): Promise<Record<string, unknown>> {
const body = await request.clone().json().catch(() => null);
if (!body || typeof body !== "object") return { transport: "http", method: request.method, ...telemetryMetadata };
Expand Down
75 changes: 75 additions & 0 deletions test/unit/mcp-server-telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { createTestEnv } from "../helpers/d1";
describe("MCP server telemetry", () => {
afterEach(() => {
vi.doUnmock("agents/mcp");
vi.doUnmock("../../src/mcp/telemetry");
vi.resetModules();
});

Expand Down Expand Up @@ -147,4 +148,78 @@ describe("MCP server telemetry", () => {
}),
]);
});

it("records exactly one recordMcpToolCall, tagged callerType remote, on a successful tool invocation (#6237)", async () => {
vi.resetModules();
vi.doMock("agents/mcp", () => ({
createMcpHandler: () => () => Response.json({ ok: true }),
}));
const recordMcpToolCall = vi.fn();
vi.doMock("../../src/mcp/telemetry", () => ({ recordMcpToolCall }));
const { handleMcpRequest } = await import("../../src/mcp/server");
const env = createTestEnv({ PRODUCT_USAGE_HASH_SALT: "mcp-remote-telemetry-salt" });
const request = new Request("https://api.test/mcp", {
method: "POST",
headers: {
authorization: `Bearer ${env.LOOPOVER_MCP_TOKEN}`,
"content-type": "application/json",
},
body: JSON.stringify({ jsonrpc: "2.0", id: "tool-call", method: "tools/call", params: { name: "loopover_local_status" } }),
});

await expect(
handleMcpRequest({
env,
executionCtx: { waitUntil() {}, passThroughOnException() {} },
req: {
method: "POST",
raw: request,
header: (name: string) => request.headers.get(name) ?? undefined,
},
json: (body: unknown, status?: number) => Response.json(body, status === undefined ? undefined : { status }),
} as never),
).resolves.toMatchObject({ status: 200 });

expect(recordMcpToolCall).toHaveBeenCalledTimes(1);
expect(recordMcpToolCall).toHaveBeenCalledWith(
env,
expect.objectContaining({ tool: "loopover_local_status", callerType: "remote", ok: true, durationMs: expect.any(Number) }),
);
});

it("does not let a throwing recordMcpToolCall affect the tool response (#6237)", async () => {
vi.resetModules();
vi.doMock("agents/mcp", () => ({
createMcpHandler: () => () => Response.json({ ok: true, result: "unchanged" }),
}));
vi.doMock("../../src/mcp/telemetry", () => ({
recordMcpToolCall: () => {
throw new Error("posthog_unreachable");
},
}));
const { handleMcpRequest } = await import("../../src/mcp/server");
const env = createTestEnv({ PRODUCT_USAGE_HASH_SALT: "mcp-remote-telemetry-throws-salt" });
const request = new Request("https://api.test/mcp", {
method: "POST",
headers: {
authorization: `Bearer ${env.LOOPOVER_MCP_TOKEN}`,
"content-type": "application/json",
},
body: JSON.stringify({ jsonrpc: "2.0", id: "tool-call", method: "tools/call", params: { name: "loopover_local_status" } }),
});

const response = await handleMcpRequest({
env,
executionCtx: { waitUntil() {}, passThroughOnException() {} },
req: {
method: "POST",
raw: request,
header: (name: string) => request.headers.get(name) ?? undefined,
},
json: (body: unknown, status?: number) => Response.json(body, status === undefined ? undefined : { status }),
} as never);

expect(response.status).toBe(200);
await expect(response.clone().json()).resolves.toEqual({ ok: true, result: "unchanged" });
});
});