feat(mcp): one registry-driven telemetry contract at all three dispatch chokepoints (#9525)

package.json

@@ -109,6 +109,7 @@
     "test:engine-parity": "vitest run test/contract/engine-parity.test.ts",
     "test:live-gate-parity": "vitest run test/contract/live-gate-parity.test.ts",
     "test:driver-parity": "vitest run test/contract/coding-agent-driver-parity.test.ts",
+    "validate:mcp": "vitest run test/contract/validate-mcp.test.ts",
     "test:changed": "vitest run --changed=origin/main",
     "pretest:workers": "npm run check-node-version",
     "test:workers": "vitest run --config vitest.workers.config.ts",
@@ -120,7 +121,7 @@
     "test:smoke:browser:install": "playwright install chromium",
     "test:smoke:browser": "node --experimental-strip-types scripts/smoke-ui-browser.ts",
     "pretest:ci": "npm run check-node-version",
-    "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
+    "test:ci": "git diff --check && npm run actionlint && npm run lint:composite-actions && npm run db:migrations:check && npm run db:schema-drift:check && npm run selfhost:env-reference:check && npm run miner:env-reference:check && npm run selfhost:validate-observability && npm run cf-typegen:check && npm run build --workspace @loopover/engine && npm run build --workspace @loopover/discovery-index && npm run build:mcp && npm run build:miner && npm run build --workspace @loopover/ui-kit && npm run typecheck && npm run test:coverage && npm run test:engine-parity && npm run test:live-gate-parity && npm run test:driver-parity && npm run validate:mcp && npm run test --workspace @loopover/engine && npm run test:workers && npm run test:mcp-pack && npm run test:miner-pack && npm run test:engine-pack && npm run test:ui-kit-pack && npm run test:miner-deployment-docs-audit && npm run rees:test && npm run ui:openapi:check && npm run ui:openapi:settings-parity && npm run ui:version-audit && npm run docs:drift-check && npm run coverage-boltons:check && npm run import-specifiers:check && npm run dead-source-files:check && npm run regate-sort-key:check && npm run command-redelivery-guards:check && npm run validate:no-hand-written-js && npm run replay-runner-manifest:check && npm run coco-dev-versions:check && npm run branding-drift:check && npm run manifest:drift-check && npm run engine-parity:drift-check && npm run engines-nvmrc:check && npm run release-manifest:sync:check && npm run command-reference:check && npm run ui:lint && npm run ui:typecheck && npm run ui:test && npm run ui:build",
     "test:release": "npm run test:ci && npm run changelog:check",
     "test:release:mcp": "npm run test:ci",
     "test:watch": "vitest",

packages/loopover-contract/src/index.ts

@@ -23,6 +23,7 @@ export {
 } from "./tool-definition.js";
 
 export * from "./enums.js";
+export * from "./telemetry.js";
 export { PREFLIGHT_LIMITS, PREDICT_GATE_MAX_CHANGED_PATHS, PREDICT_GATE_MAX_CHANGED_PATH_CHARS, WRITE_TOOL_LIMITS, SCENARIO_LIMITS } from "./limits.js";
 export { ownerRepoInput, ownerRepoPullInput, freshnessFields, toolErrorFields } from "./shared.js";
 export { TOOL_CONTRACTS, listToolDefinitions, getToolContract } from "./tools/index.js";

packages/loopover-contract/src/telemetry.ts

@@ -0,0 +1,265 @@
+// MCP dispatch telemetry: the one definition of WHAT is emitted (#9525).
+//
+// Three servers, one shape. Each runtime keeps its own thin sink -- the Worker and the self-host
+// Node process share `posthog-node`, the stdio CLI has its own double-gated client, the miner has
+// its own opt-in one -- but none of them decides what a telemetry event contains. That lives here,
+// in the zod-only leaf every surface already depends on, which is what makes a single allowlist
+// enforceable rather than aspirational. Before this, the three had three property lists.
+//
+// NOTHING IN THIS FILE PERFORMS I/O. It is pure data and pure functions, so the redaction and the
+// size caps are unit-testable without a network, and so this package stays the dependency-free leaf
+// that every other one can import.
+import { z } from "zod";
+import type { ToolCategory, ToolContract } from "./tool-definition.js";
+
+/** PostHog's own MCP-Analytics event family (#7737 upstream). */
+export const MCP_TOOL_CALL_EVENT = "$mcp_tool_call";
+
+/** LoopOver's own minimal usage event -- no arguments, no results, ever. */
+export const MCP_USAGE_EVENT = "usage_event";
+
+/**
+ * Why a call failed, as a CLOSED set.
+ *
+ * Developer-defined and deliberately small: telemetry breaks failures down by cause, and a
+ * caller-derived string in that position would be both a cardinality explosion and an injection of
+ * untrusted text into a dashboard. Anything that does not map to one of these is `unknown_error`.
+ */
+export const MCP_TELEMETRY_ERROR_CODES = [
+  "invalid_input",
+  "unauthorized",
+  "forbidden",
+  "not_found",
+  "not_configured",
+  "rate_limited",
+  "upstream_error",
+  "timeout",
+  "elicitation_declined",
+  "unknown_error",
+] as const;
+export type McpTelemetryErrorCode = (typeof MCP_TELEMETRY_ERROR_CODES)[number];
+
+/** Which server answered. The one dimension that is not derivable from the registry. */
+export const MCP_TELEMETRY_SURFACES = ["remote", "stdio", "miner"] as const;
+export type McpTelemetrySurface = (typeof MCP_TELEMETRY_SURFACES)[number];
+
+/**
+ * The COMPLETE set of property keys any MCP telemetry event may carry.
+ *
+ * Single-sourced so the meta-test can assert no payload key exists outside it. The check is worth
+ * having because the failure it prevents is silent: a property added at one sink ships data the
+ * other two never agreed to send, and nothing at the wire tells you.
+ */
+export const MCP_TELEMETRY_PROPERTY_KEYS = [
+  "tool",
+  "category",
+  "surface",
+  "ok",
+  "duration_ms",
+  "error_code",
+  "arguments",
+  "result",
+  "payloads_excluded",
+] as const;
+export type McpTelemetryPropertyKey = (typeof MCP_TELEMETRY_PROPERTY_KEYS)[number];
+
+/** What a dispatch chokepoint observes about one call. */
+export const McpToolCallTelemetry = z.object({
+  tool: z.string().min(1),
+  category: z.string().min(1),
+  surface: z.enum(MCP_TELEMETRY_SURFACES),
+  ok: z.boolean(),
+  durationMs: z.number().int().min(0),
+  errorCode: z.enum(MCP_TELEMETRY_ERROR_CODES).optional(),
+});
+export type McpToolCallTelemetry = z.infer<typeof McpToolCallTelemetry>;
+
+/**
+ * The minimal usage event: identity-free, payload-free, and the same on all three servers.
+ *
+ * `error_code` is omitted rather than sent as null on success, so a breakdown by error_code has no
+ * phantom bucket.
+ */
+export function buildUsageEventProperties(call: McpToolCallTelemetry): Record<string, unknown> {
+  return {
+    tool: call.tool,
+    category: call.category,
+    surface: call.surface,
+    ok: call.ok,
+    duration_ms: call.durationMs,
+    ...(call.errorCode ? { error_code: call.errorCode } : {}),
+  };
+}
+
+/**
+ * Whether a tool's arguments and results may ride the MCP-Analytics event.
+ *
+ * DEFAULT: NO, for every tool. That default is not caution for its own sake -- it is the standing
+ * guarantee LoopOver's telemetry has always made and that
+ * test/unit/mcp-local-telemetry-chokepoint.test.ts has asserted since #6238: the call's actual
+ * content never leaves the machine. Most of these tools take the user's own content AS their input.
+ * `loopover_lint_pr_text` takes the PR body. `loopover_check_slop_risk` takes the commit messages.
+ * `loopover_intake_idea` takes a freeform brief. Including arguments "with redaction" would have
+ * shipped all three, since none of them is secret-SHAPED -- it is simply the user's writing.
+ *
+ * This was not the first design. #9525 initially inverted it: include payloads except for
+ * admin/operator tools. The chokepoint test rejected it within one run by finding a real commit
+ * message on the wire, which is exactly the kind of promise a test should be enforcing rather than
+ * a comment.
+ *
+ * The mechanism stays because the MCP-Analytics event family is defined to carry these fields, and
+ * a future tool whose input is genuinely server-derived metadata can opt in by name here. Nothing
+ * does today, and adding one should be an argued change with the tool named in the diff.
+ */
+const TOOLS_WITH_PAYLOAD_TELEMETRY: ReadonlySet<string> = new Set();
+
+export function toolIncludesPayloads(contract: Pick<ToolContract, "name" | "category" | "auth">): boolean {
+  // The operator surfaces are excluded a second way, deliberately: were the allowlist above ever
+  // populated, an admin tool must still never qualify.
+  if (contract.category === "admin" || contract.auth === "mcp-admin" || contract.auth === "operator") return false;
+  return TOOLS_WITH_PAYLOAD_TELEMETRY.has(contract.name);
+}
+
+/** The inverse, kept because every call site reads better as "excluded". */
+export function toolExcludesPayloads(contract: Pick<ToolContract, "name" | "category" | "auth">): boolean {
+  return !toolIncludesPayloads(contract);
+}
+
+/** Property keys whose values are dropped wholesale, matched case-insensitively on the KEY. */
+const SECRET_KEY_PATTERN = /token|secret|password|passwd|dsn|credential|api[_-]?key|coldkey|hotkey|wallet|cookie|authorization|session/i;
+
+/**
+ * Value substrings that mark a string as secret-shaped regardless of its key.
+ *
+ * The token prefixes carry their own `\b`; the PEM header must NOT, because a word boundary before
+ * a leading hyphen never matches and the whole alternative would be dead. That is not hypothetical
+ * -- it was, until the forbidden-content test in test/unit/mcp-dispatch-telemetry.test.ts caught a
+ * full `-----BEGIN RSA PRIVATE KEY-----` passing through untouched.
+ */
+const SECRET_VALUE_PATTERN = /\b(?:gh[pousr]_[A-Za-z0-9]{16,}|sk-[A-Za-z0-9]{16,}|phc_[A-Za-z0-9]{16,})|-----BEGIN [A-Z ]*PRIVATE KEY-----/;
+
+export const REDACTED = "[redacted]";
+
+/** Default byte cap for an included arguments/result payload. Small on purpose: this is a telemetry
+ *  breadcrumb, not a copy of the traffic. */
+export const MCP_TELEMETRY_PAYLOAD_BYTE_CAP = 2048;
+
+/**
+ * Redact a value for telemetry: drop secret-shaped keys and values at every depth, then cap the
+ * serialized size.
+ *
+ * Recursive, unlike the miner's own flat scrubber, because a tool's arguments are arbitrarily
+ * nested by construction -- a flat pass over a `plannedChange.contributorLogin` would miss it.
+ *
+ * A secret-shaped KEY is dropped ENTIRELY, key and value both, rather than kept with a `[redacted]`
+ * placeholder. The placeholder form leaves the key NAME in the payload, and a property literally
+ * named `coldkey` or `githubToken` is itself something the repo's forbidden-content checks (rightly)
+ * treat as a finding -- there is no telemetry question that a key name answers, so nothing is lost
+ * by omitting it and a whole class of false-negative review is avoided.
+ */
+export function redactForTelemetry(value: unknown, depth = 0): unknown {
+  if (depth > 6) return REDACTED;
+  if (typeof value === "string") return SECRET_VALUE_PATTERN.test(value) ? REDACTED : value;
+  if (Array.isArray(value)) return value.map((entry) => redactForTelemetry(entry, depth + 1));
+  if (value !== null && typeof value === "object") {
+    const result: Record<string, unknown> = {};
+    for (const [key, entry] of Object.entries(value as Record<string, unknown>)) {
+      if (SECRET_KEY_PATTERN.test(key)) continue;
+      result[key] = redactForTelemetry(entry, depth + 1);
+    }
+    return result;
+  }
+  return value;
+}
+
+/** Redact, serialize, and cap. Returns undefined when there is nothing to send. */
+export function capturePayload(value: unknown, byteCap = MCP_TELEMETRY_PAYLOAD_BYTE_CAP): string | undefined {
+  if (value === undefined) return undefined;
+  let serialized: string;
+  try {
+    serialized = JSON.stringify(redactForTelemetry(value)) ?? "";
+  } catch {
+    // An unserializable payload (a BigInt, say) is not worth a telemetry failure. A CIRCULAR one
+    // never reaches here -- the depth cap above severs the cycle first -- which is the point of
+    // capping by depth rather than tracking seen references.
+    return undefined;
+  }
+  if (serialized.length === 0) return undefined;
+  return serialized.length <= byteCap ? serialized : `${serialized.slice(0, byteCap)}…[truncated]`;
+}
+
+/**
+ * PostHog's `$mcp_tool_call` event: the usage properties plus, for tools that permit it, redacted
+ * and capped arguments/results.
+ *
+ * When payloads are excluded the event says so explicitly (`payloads_excluded: true`) rather than
+ * silently omitting them -- an absent field and a deliberately withheld one are different facts, and
+ * only one of them is worth alerting on.
+ */
+export function buildMcpToolCallProperties(
+  call: McpToolCallTelemetry,
+  payloads: { arguments?: unknown; result?: unknown; excluded: boolean },
+): Record<string, unknown> {
+  const base = buildUsageEventProperties(call);
+  if (payloads.excluded) return { ...base, payloads_excluded: true };
+  const args = capturePayload(payloads.arguments);
+  const result = capturePayload(payloads.result);
+  return {
+    ...base,
+    payloads_excluded: false,
+    ...(args === undefined ? {} : { arguments: args }),
+    ...(result === undefined ? {} : { result }),
+  };
+}
+
+/**
+ * OTel span attributes for one tool call.
+ *
+ * Deliberately a STRICT SUBSET of the usage event -- no arguments, no results, not even the
+ * excluded-marker. A span is exported to a collector an operator may share more widely than their
+ * analytics project, so the safe default is that it carries only what a latency dashboard needs.
+ */
+export function buildMcpToolSpanAttributes(call: McpToolCallTelemetry): Record<string, unknown> {
+  return {
+    tool: call.tool,
+    category: call.category,
+    surface: call.surface,
+    ok: call.ok,
+    duration_ms: call.durationMs,
+    ...(call.errorCode ? { error_code: call.errorCode } : {}),
+  };
+}
+
+/** The span name for a tool call. */
+export function mcpToolSpanName(tool: string): string {
+  return `mcp.tool/${tool}`;
+}
+
+/**
+ * Map a thrown error or an error envelope onto the closed code set.
+ *
+ * Matches on shape and on the small set of messages the servers actually produce; everything else
+ * is `unknown_error` rather than a guess. Never reads a caller-supplied string into the code.
+ */
+export function resolveErrorCode(error: unknown): McpTelemetryErrorCode {
+  const envelope = error as { code?: unknown } | null | undefined;
+  if (envelope && typeof envelope.code === "string") {
+    const declared = MCP_TELEMETRY_ERROR_CODES.find((code) => code === envelope.code);
+    if (declared) return declared;
+  }
+  const message = error instanceof Error ? error.message : typeof error === "string" ? error : "";
+  if (/invalid input|invalid arguments|validation/i.test(message)) return "invalid_input";
+  if (/unauthor/i.test(message)) return "unauthorized";
+  if (/forbidden|access denied|not permitted/i.test(message)) return "forbidden";
+  if (/not found|no such/i.test(message)) return "not_found";
+  if (/not configured|unconfigured|missing .*(token|key)/i.test(message)) return "not_configured";
+  if (/rate limit|too many requests/i.test(message)) return "rate_limited";
+  if (/timed out|timeout/i.test(message)) return "timeout";
+  if (/declined|cancelled by user/i.test(message)) return "elicitation_declined";
+  if (/upstream|502|503|504/i.test(message)) return "upstream_error";
+  return "unknown_error";
+}
+
+/** The category a tool reports when the registry has no entry for it -- which the contract validator
+ *  (#9520) makes impossible, but telemetry must never throw on the path it instruments. */
+export const UNKNOWN_TOOL_CATEGORY: ToolCategory | "unknown" = "unknown";

packages/loopover-mcp/bin/loopover-mcp.ts

@@ -155,8 +155,13 @@ import { wrapStdioToolHandler } from "../lib/telemetry.js";
 // same way any external consumer's "@loopover/mcp/..." import would, landing on the one real
 // package.json/CHANGELOG.md either way -- no relative-depth arithmetic, so a future directory move
 // can never silently break this again the way the pre-dist/-migration relative path did.
-function resolveOwnPackageFile(specifier: string): URL {
-  return new URL(import.meta.resolve(specifier));
+// Returns a path, not a URL: node:fs accepts both, but the two `URL` types in play (the DOM's and
+// node:url's) are structurally incompatible, so handing a `URL` to readFileSync only typechecks in a
+// program whose lib does not pull in the DOM one. #9520's validator imports this module from a test,
+// which does -- so the boundary is narrowed to a string here rather than left to depend on which
+// program the file happens to be compiled in.
+function resolveOwnPackageFile(specifier: string): string {
+  return fileURLToPath(import.meta.resolve(specifier));
 }
 
 // Read name/version from this package's own package.json (always present in any install --