Fix MCP output validation and error reporting

README.md

@@ -16,6 +16,7 @@ OpenSEO is an SEO tool for _the people_. If tools like Semrush or Ahrefs are too
 <img width="1385" height="794" alt="Image" src="https://github.com/user-attachments/assets/fd208249-44ea-4849-bb4b-5fc896aeab73" />
 
 ## Table of Contents
+
 - [Main SEO Workflows](#main-seo-workflows)
 - [OpenSEO MCP](#openseo-mcp)
 - [OpenSEO Agent Skills](#openseo-agent-skills)
@@ -32,6 +33,7 @@ OpenSEO is an SEO tool for _the people_. If tools like Semrush or Ahrefs are too
 - [SEO API Cost Reference](#seo-api-cost-reference)
 
 ## Hosted Version
+
 If you not interested in self hosting, or just want to support the project, we also have a hosted version:
 
 [openseo.so](https://openseo.so)

src/server/mcp/instrumentation.test.ts

@@ -0,0 +1,88 @@
+import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { z } from "zod";
+import type { ToolExtra } from "@/server/mcp/context";
+import { AppError } from "@/server/lib/errors";
+
+const mocks = vi.hoisted(() => ({
+  captureServerError: vi.fn(),
+}));
+
+// waitUntil runs the capture promise inline so assertions see the call.
+vi.mock("cloudflare:workers", () => ({
+  waitUntil: (promise: Promise<unknown>) => void promise,
+}));
+
+vi.mock("@/server/lib/posthog", () => ({
+  captureServerError: mocks.captureServerError,
+}));
+
+const toolExtra: ToolExtra = {
+  signal: new AbortController().signal,
+  requestId: 1,
+  sendNotification: vi.fn(),
+  sendRequest: vi.fn(),
+};
+
+const outputSchema = { items: z.array(z.object({}).passthrough()) };
+
+function okResult(structuredContent: Record<string, unknown>): CallToolResult {
+  return { content: [{ type: "text", text: "ok" }], structuredContent };
+}
+
+describe("instrumentMcpToolHandler", () => {
+  beforeEach(() => {
+    mocks.captureServerError.mockReset();
+  });
+
+  it("passes a valid result through without reporting", async () => {
+    const { instrumentMcpToolHandler } = await import("./instrumentation");
+    const wrapped = instrumentMcpToolHandler("demo", outputSchema, async () =>
+      okResult({ items: [{ domain: "example.com" }] }),
+    );
+
+    const result = await wrapped({}, toolExtra);
+
+    expect(result.structuredContent).toEqual({
+      items: [{ domain: "example.com" }],
+    });
+    expect(mocks.captureServerError).not.toHaveBeenCalled();
+  });
+
+  it("reports an output schema mismatch the SDK would silently reject", async () => {
+    const { instrumentMcpToolHandler } = await import("./instrumentation");
+    const wrapped = instrumentMcpToolHandler("demo", outputSchema, async () =>
+      okResult({ items: "not-an-array" }),
+    );
+
+    await wrapped({}, toolExtra);
+
+    expect(mocks.captureServerError).toHaveBeenCalledTimes(1);
+    expect(mocks.captureServerError.mock.calls[0][1]).toMatchObject({
+      errorCode: "MCP_OUTPUT_VALIDATION",
+      tool: "demo",
+    });
+  });
+
+  it("reports and rethrows a reportable handler error", async () => {
+    const { instrumentMcpToolHandler } = await import("./instrumentation");
+    const boom = new Error("upstream exploded");
+    const wrapped = instrumentMcpToolHandler("demo", outputSchema, async () => {
+      throw boom;
+    });
+
+    await expect(wrapped({}, toolExtra)).rejects.toThrow("upstream exploded");
+    expect(mocks.captureServerError).toHaveBeenCalledTimes(1);
+    expect(mocks.captureServerError.mock.calls[0][0]).toBe(boom);
+  });
+
+  it("rethrows expected errors without reporting them", async () => {
+    const { instrumentMcpToolHandler } = await import("./instrumentation");
+    const wrapped = instrumentMcpToolHandler("demo", outputSchema, async () => {
+      throw new AppError("NOT_FOUND");
+    });
+
+    await expect(wrapped({}, toolExtra)).rejects.toThrow("NOT_FOUND");
+    expect(mocks.captureServerError).not.toHaveBeenCalled();
+  });
+});

src/server/mcp/instrumentation.ts

@@ -0,0 +1,85 @@
+import { waitUntil } from "cloudflare:workers";
+import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
+import {
+  getParseErrorMessage,
+  normalizeObjectSchema,
+  safeParseAsync,
+  type AnySchema,
+  type ZodRawShapeCompat,
+} from "@modelcontextprotocol/sdk/server/zod-compat.js";
+import { asAppError } from "@/server/lib/errors";
+import { captureServerError } from "@/server/lib/posthog";
+import { shouldCaptureAppErrorCode } from "@/shared/error-codes";
+import type { ToolExtra } from "@/server/mcp/context";
+
+type ToolHandler<TArgs> = (
+  args: TArgs,
+  extra: ToolExtra,
+) => CallToolResult | Promise<CallToolResult>;
+
+/**
+ * Wraps an MCP tool handler so failures reach PostHog. Unlike TanStack server
+ * functions (covered by errorHandlingMiddleware), the MCP route has no error
+ * middleware, so tool failures are otherwise invisible in error reporting.
+ *
+ * This captures two classes of failure:
+ *  - Exceptions thrown by the handler (DataForSEO outages, auth failures, …),
+ *    gated by shouldCaptureAppErrorCode to keep expected errors out of PostHog.
+ *  - Output-schema validation failures. The SDK validates structuredContent
+ *    against the output schema *after* the handler returns and converts a
+ *    failure into a -32602 JSON-RPC error it never rethrows, so we re-run the
+ *    same validation (via the SDK's own helpers) to surface the mismatch
+ *    instead of shipping it silently.
+ */
+export function instrumentMcpToolHandler<TArgs>(
+  toolName: string,
+  outputSchema: AnySchema | ZodRawShapeCompat | undefined,
+  handler: ToolHandler<TArgs>,
+): (args: TArgs, extra: ToolExtra) => Promise<CallToolResult> {
+  const normalizedOutputSchema = normalizeObjectSchema(outputSchema);
+
+  return async (args, extra) => {
+    try {
+      const result = await handler(args, extra);
+      if (
+        normalizedOutputSchema &&
+        !result.isError &&
+        result.structuredContent
+      ) {
+        const validation = await safeParseAsync(
+          normalizedOutputSchema,
+          result.structuredContent,
+        );
+        if (!validation.success) {
+          // getParseErrorMessage reports type-level mismatches (expected vs
+          // received *types*), so it carries no row data. Keep it that way:
+          // output schemas must not gain value-echoing refinements (enums on
+          // user data, etc.) that would surface response values in PostHog.
+          waitUntil(
+            captureServerError(
+              new Error(`MCP output validation failed for ${toolName}`),
+              {
+                errorCode: "MCP_OUTPUT_VALIDATION",
+                tool: toolName,
+                issues: getParseErrorMessage(validation.error).slice(0, 500),
+              },
+            ),
+          );
+        }
+      }
+      return result;
+    } catch (error) {
+      const appError = asAppError(error);
+      if (shouldCaptureAppErrorCode(appError?.code)) {
+        console.error(`mcp.tool error (${toolName}):`, error);
+        waitUntil(
+          captureServerError(error, {
+            errorCode: appError?.code ?? "INTERNAL_ERROR",
+            tool: toolName,
+          }),
+        );
+      }
+      throw error;
+    }
+  };
+}

src/server/mcp/output-schemas.ts

@@ -11,7 +11,13 @@ const mcpMetaOutputSchema = z
   })
   .passthrough();
 
-export const looseObjectOutputSchema = z.record(z.string(), z.unknown());
+// Tools that pass DataForSEO rows straight through to structuredContent hand the
+// MCP SDK typed class instances (e.g. DataforseoLabsSerpCompetitorsLiveItem), not
+// plain objects. Zod 4's z.record() requires a plain-object prototype and rejects
+// class instances ("expected record, received <ClassName>"), which the SDK surfaces
+// as a -32602 output validation error. A loose object schema accepts any object
+// shape, so it validates both plain rows and typed instances.
+export const looseObjectOutputSchema = z.object({}).passthrough();
 
 export const optionalMetaOutputSchema = {
   meta: mcpMetaOutputSchema.optional(),

src/server/mcp/server.ts

@@ -1,4 +1,5 @@
 import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
+import { instrumentMcpToolHandler } from "@/server/mcp/instrumentation";
 import { getBacklinksOverviewTool } from "@/server/mcp/tools/get-backlinks-overview";
 import { getDomainKeywordSuggestionsTool } from "@/server/mcp/tools/get-domain-keyword-suggestions";
 import { getDomainOverviewTool } from "@/server/mcp/tools/get-domain-overview";
@@ -22,91 +23,172 @@ import {
 } from "@/server/mcp/tools/search-console-tools";
 import { whoamiTool } from "@/server/mcp/tools/whoami";
 
+// Each handler is wrapped with instrumentMcpToolHandler so failures reach
+// PostHog — the MCP route has no error middleware of its own. Tools are
+// registered one explicit call at a time (not via a loop/helper) so each one's
+// input/output schema types stay concrete, which the SDK's registerTool
+// generics require to type the handler callback.
 export function registerOpenSeoMcpTools(server: McpServer) {
-  server.registerTool(whoamiTool.name, whoamiTool.config, whoamiTool.handler);
+  server.registerTool(
+    whoamiTool.name,
+    whoamiTool.config,
+    instrumentMcpToolHandler(
+      whoamiTool.name,
+      whoamiTool.config.outputSchema,
+      whoamiTool.handler,
+    ),
+  );
   server.registerTool(
     listProjectsTool.name,
     listProjectsTool.config,
-    listProjectsTool.handler,
+    instrumentMcpToolHandler(
+      listProjectsTool.name,
+      listProjectsTool.config.outputSchema,
+      listProjectsTool.handler,
+    ),
   );
   server.registerTool(
     listSavedKeywordsTool.name,
     listSavedKeywordsTool.config,
-    listSavedKeywordsTool.handler,
+    instrumentMcpToolHandler(
+      listSavedKeywordsTool.name,
+      listSavedKeywordsTool.config.outputSchema,
+      listSavedKeywordsTool.handler,
+    ),
   );
   server.registerTool(
     researchKeywordsTool.name,
     researchKeywordsTool.config,
-    researchKeywordsTool.handler,
+    instrumentMcpToolHandler(
+      researchKeywordsTool.name,
+      researchKeywordsTool.config.outputSchema,
+      researchKeywordsTool.handler,
+    ),
   );
   server.registerTool(
     saveKeywordsTool.name,
     saveKeywordsTool.config,
-    saveKeywordsTool.handler,
+    instrumentMcpToolHandler(
+      saveKeywordsTool.name,
+      saveKeywordsTool.config.outputSchema,
+      saveKeywordsTool.handler,
+    ),
   );
   server.registerTool(
     getDomainOverviewTool.name,
     getDomainOverviewTool.config,
-    getDomainOverviewTool.handler,
+    instrumentMcpToolHandler(
+      getDomainOverviewTool.name,
+      getDomainOverviewTool.config.outputSchema,
+      getDomainOverviewTool.handler,
+    ),
   );
   server.registerTool(
     getDomainKeywordSuggestionsTool.name,
     getDomainKeywordSuggestionsTool.config,
-    getDomainKeywordSuggestionsTool.handler,
+    instrumentMcpToolHandler(
+      getDomainKeywordSuggestionsTool.name,
+      getDomainKeywordSuggestionsTool.config.outputSchema,
+      getDomainKeywordSuggestionsTool.handler,
+    ),
   );
   server.registerTool(
     getBacklinksOverviewTool.name,
     getBacklinksOverviewTool.config,
-    getBacklinksOverviewTool.handler,
+    instrumentMcpToolHandler(
+      getBacklinksOverviewTool.name,
+      getBacklinksOverviewTool.config.outputSchema,
+      getBacklinksOverviewTool.handler,
+    ),
   );
   server.registerTool(
     getSerpResultsTool.name,
     getSerpResultsTool.config,
-    getSerpResultsTool.handler,
+    instrumentMcpToolHandler(
+      getSerpResultsTool.name,
+      getSerpResultsTool.config.outputSchema,
+      getSerpResultsTool.handler,
+    ),
   );
   server.registerTool(
     getRankTrackerTool.name,
     getRankTrackerTool.config,
-    getRankTrackerTool.handler,
+    instrumentMcpToolHandler(
+      getRankTrackerTool.name,
+      getRankTrackerTool.config.outputSchema,
+      getRankTrackerTool.handler,
+    ),
   );
   server.registerTool(
     getRankedKeywordsTool.name,
     getRankedKeywordsTool.config,
-    getRankedKeywordsTool.handler,
+    instrumentMcpToolHandler(
+      getRankedKeywordsTool.name,
+      getRankedKeywordsTool.config.outputSchema,
+      getRankedKeywordsTool.handler,
+    ),
   );
   server.registerTool(
     findSerpCompetitorsTool.name,
     findSerpCompetitorsTool.config,
-    findSerpCompetitorsTool.handler,
+    instrumentMcpToolHandler(
+      findSerpCompetitorsTool.name,
+      findSerpCompetitorsTool.config.outputSchema,
+      findSerpCompetitorsTool.handler,
+    ),
   );
   server.registerTool(
     searchLocalBusinessesTool.name,
     searchLocalBusinessesTool.config,
-    searchLocalBusinessesTool.handler,
+    instrumentMcpToolHandler(
+      searchLocalBusinessesTool.name,
+      searchLocalBusinessesTool.config.outputSchema,
+      searchLocalBusinessesTool.handler,
+    ),
   );
   server.registerTool(
     getLocalSerpResultsTool.name,
     getLocalSerpResultsTool.config,
-    getLocalSerpResultsTool.handler,
+    instrumentMcpToolHandler(
+      getLocalSerpResultsTool.name,
+      getLocalSerpResultsTool.config.outputSchema,
+      getLocalSerpResultsTool.handler,
+    ),
   );
   server.registerTool(
     getGoogleBusinessQuestionsTool.name,
     getGoogleBusinessQuestionsTool.config,
-    getGoogleBusinessQuestionsTool.handler,
+    instrumentMcpToolHandler(
+      getGoogleBusinessQuestionsTool.name,
+      getGoogleBusinessQuestionsTool.config.outputSchema,
+      getGoogleBusinessQuestionsTool.handler,
+    ),
   );
   server.registerTool(
     getKeywordMetricsTool.name,
     getKeywordMetricsTool.config,
-    getKeywordMetricsTool.handler,
+    instrumentMcpToolHandler(
+      getKeywordMetricsTool.name,
+      getKeywordMetricsTool.config.outputSchema,
+      getKeywordMetricsTool.handler,
+    ),
   );
   server.registerTool(
     getSearchConsolePerformanceTool.name,
     getSearchConsolePerformanceTool.config,
-    getSearchConsolePerformanceTool.handler,
+    instrumentMcpToolHandler(
+      getSearchConsolePerformanceTool.name,
+      getSearchConsolePerformanceTool.config.outputSchema,
+      getSearchConsolePerformanceTool.handler,
+    ),
   );
   server.registerTool(
     inspectUrlsTool.name,
     inspectUrlsTool.config,
-    inspectUrlsTool.handler,
+    instrumentMcpToolHandler(
+      inspectUrlsTool.name,
+      inspectUrlsTool.config.outputSchema,
+      inspectUrlsTool.handler,
+    ),
   );
 }

src/server/mcp/tools/output-schema-validation.test.ts

@@ -0,0 +1,53 @@
+import {
+  normalizeObjectSchema,
+  safeParseAsync,
+} from "@modelcontextprotocol/sdk/server/zod-compat.js";
+import { describe, expect, it, vi } from "vitest";
+
+vi.mock("cloudflare:workers", () => ({
+  env: {},
+}));
+
+// A class instance reproduces what the DataForSEO SDK hands the tools: an
+// object whose prototype is not Object.prototype (e.g.
+// DataforseoLabsSerpCompetitorsLiveItem). Zod 4's z.record() rejects those
+// ("expected record, received <ClassName>"), so a record-based output schema
+// makes the MCP server fail these passthrough tools with a -32602 output
+// validation error even though the API call succeeded.
+class ProviderRow {
+  constructor(
+    public domain: string,
+    public rank_absolute: number,
+  ) {}
+}
+
+describe("DataForSEO research tool output schemas", () => {
+  // Every tool that streams provider rows straight to structuredContent.
+  it.each([
+    ["find_serp_competitors", "competitors"],
+    ["get_local_serp_results", "results"],
+    ["search_local_businesses", "businesses"],
+    ["get_google_business_questions", "questions"],
+    ["get_ranked_keywords", "keywords"],
+  ])(
+    "%s accepts typed (non-plain-object) provider rows",
+    async (toolName, field) => {
+      const tools = await import("./dataforseo-research-tools");
+      const tool = Object.values(tools).find((t) => t.name === toolName);
+      if (!tool) throw new Error(`tool ${toolName} not found`);
+
+      const schema = normalizeObjectSchema(tool.config.outputSchema);
+      if (!schema) throw new Error("output schema did not normalize");
+
+      // Mirror the MCP server: validate structuredContent against the tool's
+      // own output schema. Extra keys (e.g. get_ranked_keywords' totalCount)
+      // are allowed by the passthrough schemas, so one payload covers all.
+      const result = await safeParseAsync(schema, {
+        [field]: [new ProviderRow("example.com", 1)],
+        totalCount: 1,
+      });
+
+      expect(result.success).toBe(true);
+    },
+  );
+});