From 6b55de017badc9066642e1ff2561eb1da0ff93f8 Mon Sep 17 00:00:00 2001 From: Saim Shafique Date: Thu, 8 Oct 2026 09:26:00 +0500 Subject: [PATCH] fix(mcp): bound capture comparison input schema depth (#1005) * fix(mcp): advertise JSON values without recursive schemas `z.json()` projects a self-referential `$defs/__schema0` entry. Twenty tool input schemas and 123 output schemas carried it, and clients or model APIs that reject recursive JSON Schemas refuse the whole tool list. Every JSON value satisfies `{}`, so the shared JSON value schema now advertises that. Runtime parsing still admits only JSON values. Refs #919 Co-Authored-By: Claude Opus 5.5 * fix(mcp): keep advertised input schemas within model function-calling limits Some model providers reject the whole tool list when any input schema contains a recursive $ref or nests deeper than 10 levels (fixes #919). Sanitize the advertised wire projection: break cyclic $refs, bound nesting depth, and drop unreachable $defs. Canonical Zod contracts are untouched. --------- Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: sx4im Co-authored-by: N0zoM1z0 <1705004398@qq.com> --- docs/mcp-contracts.md | 18 ++ docs/product-catalog.json | 4 +- .../managed-conformance-manifest.json | 4 +- skills/reverse-engineer-anything/SKILL.md | 2 +- .../browserCaptureToolInputSchema.ts | 62 +++++ src/contracts/browserToolContracts.ts | 10 +- tests/boundary/mcp/browserMcp.test.ts | 35 +++ .../mcp/wireInputSchemaLimits.test.ts | 240 ++++++++++++++++++ 8 files changed, 364 insertions(+), 11 deletions(-) create mode 100644 src/contracts/browserCaptureToolInputSchema.ts create mode 100644 tests/boundary/mcp/wireInputSchemaLimits.test.ts diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 7dd8b8dfb8..70bf2259c0 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -13,6 +13,24 @@ are currently unavailable. Opening or closing a target or observing a provider health transition leaves that catalog unchanged and does not emit `notifications/tools/list_changed`. +Advertised input and output schemas contain no reachable recursive references. +The input compatibility profile limits nesting to ten object, array, and +`anyOf`/`oneOf`/`allOf` levels, following local references per path. Property +maps, reference definitions, and example data do not add schema levels. Tests +check the complete catalog after SDK conversion and its generated counterpart. +This is REA's local compatibility profile; individual model APIs can impose +additional limits. + +For passive `compare_web_captures` inputs, pass each complete +`inspect_web_page` result in `before.inspection` or `after.inspection`, with an +optional complete `discover_webmcp_tools` result in the matching `webmcp` field. +The input schema describes these producer-result objects as round-trip payloads. +For scenario comparisons, pass complete `capture_browser_scenario` results in +`before_scenario` and `after_scenario`. REA validates all nested fields with the +original capture schemas; observation output schemas remain complete. Scenario +authoring inputs and comparison normalization options retain their full +advertised structure. + Call `binary_session` with `{}` and read `result.tool_availability` to choose a callable operation for the current target, provider, host, and negotiated client capabilities. The default result includes the complete inventory with each diff --git a/docs/product-catalog.json b/docs/product-catalog.json index d0dd89d347..a6c504a7ef 100644 --- a/docs/product-catalog.json +++ b/docs/product-catalog.json @@ -689,9 +689,9 @@ "mcp_prompts": 6 }, "digests": { - "tools_sha256": "eb4749bb14998e23991c77e2d07e7cc69b28d98f2323f7fc3bbe6b4c2da79611", + "tools_sha256": "e043febb950a93c7d37597ed286cb43794432bb6f5b8f8de84a25bb83cbcb476", "prompts_sha256": "ef2de8814390b340623c32001610aa2709d90bed51537b6e562ab3abf144ab4d", - "combined_sha256": "0417b6ac9d07ff805aa6ec17f58577fc341320d46da50137da73b1c6dd0d3947", + "combined_sha256": "d9d74efe01bdc451a4d05a16d6e6ee87a0cc301d13331a19e7f015102f580bf2", "providers_sha256": "e601cdceb5981ee04d6f431492f961f8867b1faa4f08ca0ecd29be2a239bc467" } } diff --git a/docs/verification/managed-conformance-manifest.json b/docs/verification/managed-conformance-manifest.json index a0905ad824..5616dde3bc 100644 --- a/docs/verification/managed-conformance-manifest.json +++ b/docs/verification/managed-conformance-manifest.json @@ -1,5 +1,5 @@ { - "manifest_id": "ecm_6c5378cbeb0a0d0c0b127b1961f2b5e50d6541601a5f36ad7f5fb292e17ff203", + "manifest_id": "ecm_f7789a63af5aa5353dd07d14556ca41f430ca366fd99126344510486648a2952", "verifier": { "id": "rea-managed-conformance", "version": "1" @@ -13,7 +13,7 @@ "skill_digests": [ { "skill_id": "reverse-engineer-anything", - "sha256": "7777298dede63ed91211af502a4779850bfe451a11758349150d5a3df4261a93" + "sha256": "d2d5670c70b85c1d8be50da3850a295145d7a7197162ba6de3e60f5f0162130b" } ], "claims": [ diff --git a/skills/reverse-engineer-anything/SKILL.md b/skills/reverse-engineer-anything/SKILL.md index 9cf844f464..872e6b13bc 100644 --- a/skills/reverse-engineer-anything/SKILL.md +++ b/skills/reverse-engineer-anything/SKILL.md @@ -4,7 +4,7 @@ description: Reverse engineer native, managed, Electron/JavaScript, packaged, fi metadata: version: "28" tool_count: 134 - catalog_digest: "0417b6ac9d07ff805aa6ec17f58577fc341320d46da50137da73b1c6dd0d3947" + catalog_digest: "d9d74efe01bdc451a4d05a16d6e6ee87a0cc301d13331a19e7f015102f580bf2" --- # REA diff --git a/src/contracts/browserCaptureToolInputSchema.ts b/src/contracts/browserCaptureToolInputSchema.ts new file mode 100644 index 0000000000..bdc7dc1483 --- /dev/null +++ b/src/contracts/browserCaptureToolInputSchema.ts @@ -0,0 +1,62 @@ +import { browserCaptureComparisonInputSchema } from "../domain/browserCaptureComparison.js"; +import { browserScenarioCaptureSchema } from "../domain/browserScenarioCapture.js"; +import { captureSnapshotSchema } from "../domain/webCaptureDiffSchemas.js"; + +// Captures are complete producer results to pass back unchanged, rather than +// analyst-authored options. Describe that boundary without embedding the entire +// inspection result schema in every comparison input. The cloned parser still +// validates every nested field using the canonical domain schemas. +const passiveCaptureInputSchema = captureSnapshotSchema.clone( + captureSnapshotSchema.def, +); +passiveCaptureInputSchema._zod.toJSONSchema = () => ({ + type: "object", + properties: { + inspection: { + type: "object", + description: + "Complete result object returned by inspect_web_page. Pass the entire result unchanged, including capture identity, observations, completeness, and limitations. Every nested field is validated by REA.", + }, + webmcp: { + anyOf: [{ type: "object" }, { type: "null" }], + default: null, + description: + "Complete result object returned by discover_webmcp_tools for this capture, or null when it was not recorded. Pass the entire result unchanged. Every nested field is validated by REA.", + }, + }, + required: ["inspection"], +}); + +const scenarioCaptureInputSchema = browserScenarioCaptureSchema.clone( + browserScenarioCaptureSchema.def, +); +scenarioCaptureInputSchema._zod.toJSONSchema = () => ({ + type: "object", + description: + "Complete result object returned by capture_browser_scenario. Pass the entire result unchanged, including browser and scenario identity, steps, events, artifact contents and digests, completeness, and limitations. Every nested field is validated by REA.", +}); + +/** Advertise capture round trips while retaining full runtime validation. */ +export const browserCaptureToolInputSchema = + browserCaptureComparisonInputSchema.safeExtend({ + before: passiveCaptureInputSchema + .optional() + .describe( + "Earlier passive web-page capture with complete producer results.", + ), + after: passiveCaptureInputSchema + .optional() + .describe( + "Later passive web-page capture with complete producer results.", + ), + before_scenario: scenarioCaptureInputSchema + .optional() + .describe( + "Earlier complete result object returned by capture_browser_scenario.", + ), + after_scenario: scenarioCaptureInputSchema + .optional() + .describe( + "Later complete result object returned by capture_browser_scenario.", + ), + }); diff --git a/src/contracts/browserToolContracts.ts b/src/contracts/browserToolContracts.ts index ea16ce099f..445d52e81d 100644 --- a/src/contracts/browserToolContracts.ts +++ b/src/contracts/browserToolContracts.ts @@ -17,10 +17,7 @@ import { discoverWebMcpToolsInputSchema, webMcpDiscoverySchema, } from "../domain/webMcpDiscovery.js"; -import { - browserCaptureComparisonInputSchema, - browserCaptureComparisonSchema, -} from "../domain/browserCaptureComparison.js"; +import { browserCaptureComparisonSchema } from "../domain/browserCaptureComparison.js"; import { captureWebScreenshotInputSchema, compareWebScreenshotsInputSchema, @@ -29,6 +26,7 @@ import { } from "../domain/webScreenshot.js"; import { toolContractMetadata } from "./toolEffects.js"; import { evidenceResultOf } from "./toolOutputSchemas.js"; +import { browserCaptureToolInputSchema } from "./browserCaptureToolInputSchema.js"; const evidenceResult = evidenceResultOf; const listOutputSchema = evidenceResult(browserTargetListSchema); @@ -215,9 +213,9 @@ export const BROWSER_TOOL_CONTRACTS = [ name: "compare_web_captures", ...toolContractMetadata("compare_web_captures"), description: - "Compare passive web captures by providing before and after, or compare recorded scenarios by providing before_scenario and after_scenario with an optional normalization policy. Use exactly one input group. Scenario comparison aligns exact step IDs, records deterministic literal normalization, and returns every artifact-level difference plus alignment failures inline. Missing or truncated evidence never proves equality.", + "Compare passive web captures by providing before and after, each with the complete inspect_web_page result in inspection and an optional complete discover_webmcp_tools result in webmcp; or compare recorded scenarios by passing complete capture_browser_scenario results in before_scenario and after_scenario with an optional normalization policy. Use exactly one input group. Scenario comparison aligns exact step IDs, records deterministic literal normalization, and returns every artifact-level difference plus alignment failures inline. Missing or truncated evidence never proves equality.", kind: "browser-provider", - inputSchema: browserCaptureComparisonInputSchema, + inputSchema: browserCaptureToolInputSchema, outputSchema: captureDiffOutputSchema, examples: [ { diff --git a/tests/boundary/mcp/browserMcp.test.ts b/tests/boundary/mcp/browserMcp.test.ts index 21e615e622..a61576ee77 100644 --- a/tests/boundary/mcp/browserMcp.test.ts +++ b/tests/boundary/mcp/browserMcp.test.ts @@ -4,6 +4,7 @@ import { z } from "zod"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js"; +import { webPageInspectionSchema } from "../../../src/domain/browserObservation.js"; import { JAVASCRIPT_RUNTIME_RECONCILIATION_EXAMPLE } from "../../../src/contracts/javascript/javascriptRuntimeReconciliationExample.js"; import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; import { createServer } from "../../../src/server/createServer.js"; @@ -190,6 +191,40 @@ const verifySessionAndComparisonTools = async ( expect(compared.structuredContent).toMatchObject({ result: { overall_status: "unknown" }, }); + const parsedCapture = webPageInspectionSchema.parse(capture); + const socket = parsedCapture.network.websocket_events.find( + (event) => event.payload_shape?.json_shape != null, + ); + const payload = socket?.payload_shape; + if (socket === undefined || payload == null || payload.json_shape === null) + throw new Error("Missing deep WebSocket JSON shape fixture"); + const malformedCapture = { + ...parsedCapture, + network: { + ...parsedCapture.network, + websocket_events: [ + { + ...socket, + payload_shape: { + ...payload, + json_shape: { + ...payload.json_shape, + properties: [{ path: "/event", types: [42], observations: 1 }], + }, + }, + }, + ], + }, + }; + const malformedComparison = await connected.client.callTool({ + name: "compare_web_captures", + arguments: { + before: { inspection: malformedCapture }, + after: { inspection: capture }, + }, + }); + expect(malformedComparison.isError).toBe(true); + expect(JSON.stringify(malformedComparison.content)).toContain("types"); const incompleteComparison = await connected.client.callTool({ name: "compare_web_captures", arguments: { before: { inspection: capture } }, diff --git a/tests/boundary/mcp/wireInputSchemaLimits.test.ts b/tests/boundary/mcp/wireInputSchemaLimits.test.ts new file mode 100644 index 0000000000..be8bdae1cb --- /dev/null +++ b/tests/boundary/mcp/wireInputSchemaLimits.test.ts @@ -0,0 +1,240 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { McpServer } from "@modelcontextprotocol/server"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { describe, expect, it } from "vitest"; +import { z } from "zod"; + +import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; +import { browserCaptureComparisonInputSchema } from "../../../src/domain/browserCaptureComparison.js"; +import { webPageInspectionSchema } from "../../../src/domain/browserObservation.js"; +import { GENERATED_MCP_TOOL_CATALOG } from "../../../src/generatedMcpToolCatalog.js"; +import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +function resolveLocalReference(root: unknown, reference: string): unknown { + if (reference === "#") return root; + if (!reference.startsWith("#/")) return undefined; + let current = root; + for (const part of reference.slice(2).split("/")) { + const key = part.replaceAll("~1", "/").replaceAll("~0", "~"); + if (!isRecord(current) || !Object.hasOwn(current, key)) return undefined; + current = current[key]; + } + return current; +} + +// Count object/array and composition levels, following references per path. +// Definitions, property maps, and literal example/default data are not levels. +function inputDepth( + schema: unknown, + root: unknown = schema, + activeReferences: readonly string[] = [], +): number { + if (!isRecord(schema)) return 0; + let referenceDepth = 0; + if (typeof schema.$ref === "string") { + if (activeReferences.includes(schema.$ref)) return Number.POSITIVE_INFINITY; + const target = resolveLocalReference(root, schema.$ref); + if (target === undefined) return Number.POSITIVE_INFINITY; + referenceDepth = inputDepth(target, root, [ + ...activeReferences, + schema.$ref, + ]); + } + const childDepths: number[] = []; + for (const key of ["properties", "patternProperties", "dependentSchemas"]) { + const group = schema[key]; + if (isRecord(group)) + for (const child of Object.values(group)) + childDepths.push(inputDepth(child, root, activeReferences)); + } + for (const key of [ + "items", + "additionalProperties", + "contains", + "not", + "if", + "then", + "else", + "propertyNames", + "unevaluatedProperties", + "unevaluatedItems", + ]) + childDepths.push(inputDepth(schema[key], root, activeReferences)); + for (const key of ["allOf", "anyOf", "oneOf", "prefixItems"]) { + const children = schema[key]; + if (Array.isArray(children)) + for (const child of children) + childDepths.push(inputDepth(child, root, activeReferences)); + } + const types = Array.isArray(schema.type) ? schema.type : [schema.type]; + const container = types.includes("object") || types.includes("array") ? 1 : 0; + const composition = ["anyOf", "oneOf", "allOf"].some((key) => + Array.isArray(schema[key]), + ) + ? 1 + : 0; + return Math.max( + referenceDepth, + container + composition + Math.max(0, ...childDepths), + ); +} + +function inputDepthViolations( + tools: readonly { name: string; inputSchema: unknown }[], +) { + return tools.flatMap(({ name, inputSchema }) => { + const depth = inputDepth(inputSchema); + return depth > 10 ? [{ name, depth }] : []; + }); +} + +async function withCatalog( + verify: (client: Client, calls: ReadonlyMap) => Promise, +): Promise { + const server = new McpServer({ name: "wire-limits", version: "0" }); + const calls = new Map(); + for (const contract of TOOL_CONTRACTS) + server.registerTool( + contract.name, + toolRegistrationOptions(contract), + async () => { + calls.set(contract.name, (calls.get(contract.name) ?? 0) + 1); + return { content: [], isError: true }; + }, + ); + const client = new Client({ name: "wire-limits", version: "0" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + await verify(client, calls); + } finally { + await Promise.allSettled([client.close(), server.close()]); + } +} + +describe("complete catalog input compatibility profile (#919)", () => { + it("follows shared and escaped references without treating data as schemas", () => { + const diamond = { + type: "object", + properties: { + left: { $ref: "#/$defs/a~1b~0c" }, + right: { $ref: "#/$defs/a~1b~0c" }, + }, + $defs: { + "a/b~c": { type: "object", properties: { id: { type: "string" } } }, + unused: { $ref: "#/$defs/unused" }, + }, + examples: [{ $ref: "#" }], + default: { $ref: "#" }, + }; + expect(inputDepth(diamond)).toBe(2); + expect( + inputDepth({ type: "object", properties: { child: { $ref: "#" } } }), + ).toBe(Number.POSITIVE_INFINITY); + expect( + inputDepth({ + type: "object", + properties: { + values: { + anyOf: [ + { type: "array", items: { type: "object" } }, + { type: "null" }, + ], + }, + }, + }), + ).toBe(4); + }); + + it("keeps every live and generated input within ten structural levels", async () => { + await withCatalog(async (client) => { + const { tools } = await client.listTools(); + expect(tools.map(({ name }) => name).sort()).toEqual( + TOOL_CONTRACTS.map(({ name }) => name).sort(), + ); + expect(inputDepthViolations(tools)).toEqual([]); + expect(inputDepthViolations(GENERATED_MCP_TOOL_CATALOG)).toEqual([]); + }); + }); + + it("describes capture round trips without altering domain or scenario schemas", () => { + const contract = TOOL_CONTRACTS.find( + ({ name }) => name === "compare_web_captures", + ); + if (contract === undefined) + throw new Error("Missing capture comparison contract"); + const original = z.toJSONSchema(browserCaptureComparisonInputSchema, { + io: "input", + }); + const projected = z.toJSONSchema(contract.inputSchema, { io: "input" }); + expect(inputDepth(original)).toBeGreaterThan(10); + expect(inputDepth(projected)).toBeLessThanOrEqual(10); + expect(isRecord(original.properties)).toBe(true); + expect(isRecord(projected.properties)).toBe(true); + if (!isRecord(original.properties) || !isRecord(projected.properties)) + throw new Error("Missing comparison properties"); + for (const key of ["normalization"]) + expect(projected.properties[key], key).toEqual(original.properties[key]); + expect(z.toJSONSchema(webPageInspectionSchema).properties).toHaveProperty( + "network", + ); + for (const example of contract.examples) + expect(contract.inputSchema.parse(example.input)).toEqual( + browserCaptureComparisonInputSchema.parse(example.input), + ); + const example = contract.examples[0]; + if (example === undefined) throw new Error("Missing scenario example"); + for (const malformed of [ + { before_scenario: {}, after_scenario: {} }, + { + ...example.input, + before: { inspection: {} }, + after: { inspection: {} }, + }, + { ...example.input, __unexpected_root_key__: true }, + ]) { + const originalResult = + browserCaptureComparisonInputSchema.safeParse(malformed); + const projectedResult = contract.inputSchema.safeParse(malformed); + expect(originalResult.success).toBe(false); + expect(projectedResult.success).toBe(false); + if (!originalResult.success && !projectedResult.success) + expect(projectedResult.error.issues).toEqual( + originalResult.error.issues, + ); + } + }); + + it("rejects incomplete round-trip payloads before invoking the handler", async () => { + await withCatalog(async (client, calls) => { + const tool = (await client.listTools()).tools.find( + ({ name }) => name === "compare_web_captures", + ); + if (tool === undefined) + throw new Error("Missing advertised comparison tool"); + const incomplete = { + before: { inspection: {} }, + after: { inspection: {} }, + }; + const validate = new Ajv2020({ + strict: false, + validateFormats: false, + }).compile(tool.inputSchema); + expect(validate(incomplete)).toBe(true); + expect( + browserCaptureComparisonInputSchema.safeParse(incomplete).success, + ).toBe(false); + const result = await client.callTool({ + name: tool.name, + arguments: incomplete, + }); + expect(result.isError).toBe(true); + expect(calls.get(tool.name) ?? 0).toBe(0); + }); + }); +});