From 39dabd0608ed9617d60f16ece05993089f24ff38 Mon Sep 17 00:00:00 2001 From: fernando Date: Fri, 9 Oct 2026 20:03:12 -0300 Subject: [PATCH] fix(mcp): advertise object roots for union input schemas (#1373) * fix(mcp): advertise object roots for union input schemas Anthropic tool input schemas reject anyOf, oneOf, and allOf at the root. Since 63bf9140 removed root flattening, compare_web_captures, inspect_web_page, inspect_electron_page, and project_managed_application_graph advertised root unions: Claude Desktop disabled compare_web_captures and Claude Code exposed the tools without parameters. Present a root union of plain-reference object branches as one strict object with the union of properties, shared required fields, a minProperties bound, and a description of the accepted input groups. The canonical Zod union still validates every call, so mixed input groups are rejected before the handler runs. Branches whose reference siblings cannot be merged keep their root alternatives. Co-Authored-By: Claude Opus 5.5 * test(browser): keep comparison input boundary within lint limits Express advertised and runtime expectations as one case table so the boundary test stays under the 150-line function limit. Co-Authored-By: Claude Opus 5.5 * fix(mcp): preserve input group dependencies in object schemas --------- Co-authored-by: Claude Opus 5.5 Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- docs/mcp-contracts.md | 25 ++-- src/contracts/inputSchemaPresentation.ts | 125 +++++++++++++++++- src/contracts/toolInputReferences.test.ts | 106 +++++++++------ .../mcp/browserComparisonInput.test.ts | 114 ++++++++++------ tests/boundary/mcp/toolSchemaValidity.test.ts | 13 +- 5 files changed, 282 insertions(+), 101 deletions(-) diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index f25ea8a57..66487bd01 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -25,12 +25,16 @@ health transition leaves that catalog unchanged and does not emit Advertised input and output schemas contain no reachable recursive references. Schemas share repeated definitions through schema-local references while -preserving complete fields and validation rules. Input properties retain their -descriptions and literal examples. The SDK advertises an object root for -canonical object unions and retains each branch's requirements and exclusions. -Tests validate the schemas after SDK conversion and compare advertised -validation with actual calls. Individual model APIs can impose additional -nesting limits; complete producer captures can exceed ten structural levels. +preserving complete nested fields and validation rules. Input properties retain +their descriptions and literal examples. Root object unions are presented as +one object without root `anyOf`, `oneOf`, or `allOf` for model API compatibility. +The projection merges properties, retains shared required fields and +group-specific field dependencies, and describes the accepted input groups. +Canonical runtime validation enforces the exact union, including exclusions +that the advertised projection does not express. Tests validate the schemas +after SDK conversion and check advertised validation and actual calls. +Individual model APIs can impose additional nesting limits; complete producer +captures can exceed ten structural levels. `compare_web_captures` accepts exactly one of two input shapes: @@ -42,10 +46,11 @@ nesting limits; complete producer captures can exceed ten structural levels. `normalization` defaults to `{ "rules": [] }`. The advertised schema includes complete nested capture fields and rejects -incomplete pairs, mixed comparison families, and structurally malformed -captures. Domain validation additionally checks relationships such as event -sequence references and retained counts; JSON Schema does not express those -cross-field invariants. +incomplete pairs and structurally malformed captures. Mixed comparison families +can pass advertised validation; the SDK rejects them against the canonical +input schema before invoking the handler. Domain validation additionally checks +relationships such as event sequence references and retained counts; JSON +Schema does not express those cross-field invariants. Call `binary_session` with `{}` and read `result.tool_availability` to choose a callable operation for the current target, provider, host, and negotiated client diff --git a/src/contracts/inputSchemaPresentation.ts b/src/contracts/inputSchemaPresentation.ts index 6eae1989c..8b1bd5b9e 100644 --- a/src/contracts/inputSchemaPresentation.ts +++ b/src/contracts/inputSchemaPresentation.ts @@ -1,3 +1,5 @@ +import { isDeepStrictEqual } from "node:util"; + const schemaMaps = new Set([ "$defs", "definitions", @@ -102,11 +104,128 @@ const describeSchema = ( ); }; -/** Describe canonical input fields without changing structural constraints. */ +const objectBranch = ( + root: Readonly>, + branch: unknown, +): Readonly> | undefined => { + // Reference siblings evaluate separately from the target in 2020-12, so a + // merged object could accept inputs the branch rejects. + const resolved = + isObject(branch) && typeof branch.$ref === "string" + ? Object.keys(branch).length === 1 + ? resolveReference(root, branch.$ref) + : undefined + : branch; + return isObject(resolved) && + resolved.type === "object" && + isObject(resolved.properties) + ? resolved + : undefined; +}; + +const requiredProperties = ( + branch: Readonly>, +): readonly string[] => + Array.isArray(branch.required) + ? branch.required.filter((name) => typeof name === "string") + : []; + +const sharedRequiredProperties = ( + branches: readonly Readonly>[], +): readonly string[] => { + const [first = [], ...others] = branches.map(requiredProperties); + return first.filter((name) => others.every((names) => names.includes(name))); +}; + +const inputGroup = (branch: Readonly>): string => { + const required = requiredProperties(branch); + const optional = Object.keys(branch.properties ?? {}) + .filter((name) => !required.includes(name)) + .map((name) => `[${name}]`); + return [...required, ...optional].join(" + "); +}; + +// Anthropic tool input schemas reject anyOf, oneOf, and allOf at the root, and +// clients then drop the tool or advertise it without parameters. Present a +// root union of strict object groups as one object whose properties are the +// union of the groups. The canonical Zod schema still enforces the exact group. +const flattenRootObjectUnion = ( + root: Record, +): Record => { + const { anyOf, oneOf, ...rest } = root; + const alternatives = Array.isArray(anyOf) ? anyOf : oneOf; + if ( + !Array.isArray(alternatives) || + alternatives.length === 0 || + root.properties !== undefined + ) + return root; + const branches = alternatives.map((branch) => objectBranch(root, branch)); + if (!branches.every((branch) => branch !== undefined)) return root; + const variants = new Map(); + for (const branch of branches) + for (const [name, schema] of Object.entries(branch.properties ?? {})) { + const known = variants.get(name) ?? []; + if (!known.some((seen) => isDeepStrictEqual(seen, schema))) + known.push(schema); + variants.set(name, known); + } + const required = sharedRequiredProperties(branches); + // A field unique to a group still needs that group's required partners. + // Counting properties alone would let optional fields pad an incomplete pair. + const dependencies = Object.fromEntries( + [...variants.keys()].flatMap((name) => { + const owners = branches.filter((branch) => + Object.hasOwn(branch.properties ?? {}, name), + ); + const partners = sharedRequiredProperties(owners).filter( + (partner) => partner !== name && !required.includes(partner), + ); + return partners.length > 0 ? [[name, partners]] : []; + }), + ); + const fewestRequired = Math.min( + ...branches.map((branch) => requiredProperties(branch).length), + ); + // Groups that differ only in property values add no caller guidance. + const groups = [...new Set(branches.map(inputGroup))]; + const description = [ + typeof rest.description === "string" ? rest.description : undefined, + groups.length > 1 + ? `Provide a complete input group: ${groups.join("; ")}.` + : undefined, + ].filter((text) => text !== undefined); + return { + ...rest, + type: "object", + ...(description.length > 0 ? { description: description.join(" ") } : {}), + properties: Object.fromEntries( + [...variants].map(([name, schemas]) => [ + name, + schemas.length === 1 ? schemas[0] : { anyOf: schemas }, + ]), + ), + ...(required.length > 0 ? { required } : {}), + ...(fewestRequired > 0 ? { minProperties: fewestRequired } : {}), + ...(Object.keys(dependencies).length > 0 + ? { + [root.$schema === "http://json-schema.org/draft-07/schema#" + ? "dependencies" + : "dependentRequired"]: dependencies, + } + : {}), + ...(branches.every((branch) => branch.additionalProperties === false) + ? { additionalProperties: false } + : {}), + }; +}; + +/** Describe canonical input fields and present the root as one object. */ export const presentInputJsonSchema = ( root: Record, describeProperty: (property: string) => string, ): Record => { - const described = describeSchema(root, root, describeProperty); - return isObject(described) ? described : root; + const flattened = flattenRootObjectUnion(root); + const described = describeSchema(flattened, flattened, describeProperty); + return isObject(described) ? described : flattened; }; diff --git a/src/contracts/toolInputReferences.test.ts b/src/contracts/toolInputReferences.test.ts index 5a2285f20..4441ba9f7 100644 --- a/src/contracts/toolInputReferences.test.ts +++ b/src/contracts/toolInputReferences.test.ts @@ -149,52 +149,78 @@ describe("root input presentation", () => { { anyOf: [branch, branch] }, (property) => `Parameter ${property}`, ); - const branches = z - .array(z.record(z.string(), z.unknown())) - .parse(presented.anyOf); - expect(branches).toHaveLength(2); - for (const presentedBranch of branches) { - expect(presentedBranch).toMatchObject(branch); - const properties = presentedBranch.properties; - if (typeof properties !== "object" || properties === null) - throw new Error("Missing branch properties"); - expect(Object.getPrototypeOf(properties)).toBe(Object.prototype); - expect(Object.hasOwn(properties, "__proto__")).toBe(true); - } + expect(presented).not.toHaveProperty("anyOf"); + expect(presented).toMatchObject({ + type: "object", + required: ["__proto__"], + additionalProperties: false, + }); + const properties = presented.properties; + if (typeof properties !== "object" || properties === null) + throw new Error("Missing root properties"); + expect(Object.getPrototypeOf(properties)).toBe(Object.prototype); + expect(Object.hasOwn(properties, "__proto__")).toBe(true); }); - it("preserves root alternatives, required pairs, and mixed-family exclusions", () => { - const schema = z.union([ - z.strictObject({ before: z.string(), after: z.string() }), - z.strictObject({ - before_scenario: z.number(), - after_scenario: z.number(), - }), - ]); - const canonical = z.toJSONSchema(schema, { io: "input" }); - const advertised = presentInputJsonSchema( - canonical, - (property) => property, - ); - const validate = new Ajv2020({ strict: false }).compile(advertised); - for (const input of [ - {}, - { before: "first" }, - { before: "first", after: "second" }, - { before_scenario: 1, after_scenario: 2 }, - { before: "first", after_scenario: 2 }, - { + it.each(["draft-2020-12", "draft-07"] as const)( + "advertises complete root groups while runtime keeps the exact union in %s", + (target) => { + const schema = z.union([ + z.strictObject({ before: z.string(), after: z.string() }), + z.strictObject({ + before_scenario: z.number(), + after_scenario: z.number(), + normalization: z.boolean().optional(), + }), + ]); + const canonical = z.toJSONSchema(schema, { io: "input", target }); + const advertised = presentInputJsonSchema( + canonical, + (property) => property, + ); + // Anthropic tool input schemas reject anyOf, oneOf, and allOf at the root. + for (const combinator of ["anyOf", "oneOf", "allOf"]) + expect(advertised).not.toHaveProperty(combinator); + expect(advertised.description).toBe( + "Provide a complete input group: before + after; before_scenario + after_scenario + [normalization].", + ); + const ajv = + target === "draft-2020-12" + ? new Ajv2020({ strict: false }) + : new Ajv({ strict: false }); + const validate = ajv.compile(advertised); + for (const input of [ + { before: "first", after: "second" }, + { before_scenario: 1, after_scenario: 2 }, + { before_scenario: 1, after_scenario: 2, normalization: true }, + ]) { + expect(schema.safeParse(input).success).toBe(true); + expect(validate(input)).toBe(true); + } + for (const input of [ + {}, + { before: "first" }, + { unexpected: true }, + { before_scenario: 1, after_scenario: "wrong" }, + { before_scenario: 1, normalization: true }, + { after_scenario: 2, normalization: true }, + ]) { + expect(schema.safeParse(input).success).toBe(false); + expect(validate(input)).toBe(false); + } + // Cross-group exclusion is enforced only by the canonical runtime parser. + const mixed = { before: "first", after: "second", before_scenario: 1, after_scenario: 2, - }, - { before_scenario: 1, after_scenario: "wrong" }, - ]) - expect(validate(input)).toBe(schema.safeParse(input).success); - }); + }; + expect(validate(mixed)).toBe(true); + expect(schema.safeParse(mixed).success).toBe(false); + }, + ); - it("retains reference siblings and does not manufacture root branch constraints", () => { + it("keeps a root branch whose reference siblings cannot be merged", () => { const branch = { $ref: "#/$defs/object", additionalProperties: false }; const root = { $defs: { @@ -205,7 +231,7 @@ describe("root input presentation", () => { const presented = presentInputJsonSchema(root, (property) => property); expect(presented.anyOf).toEqual([branch]); const ajv = new Ajv2020({ strict: false }); - for (const sample of [{}, { value: "value" }, null]) + for (const sample of [{}, { value: "value" }, { other: 1 }, null]) expect(ajv.compile(presented)(sample)).toBe(ajv.compile(root)(sample)); }); diff --git a/tests/boundary/mcp/browserComparisonInput.test.ts b/tests/boundary/mcp/browserComparisonInput.test.ts index 66104d292..9d6cdc5f9 100644 --- a/tests/boundary/mcp/browserComparisonInput.test.ts +++ b/tests/boundary/mcp/browserComparisonInput.test.ts @@ -23,6 +23,38 @@ const delivery = new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE); const contract = toolContract("compare_web_captures"); +async function capturePassivePair( + provider: CdpBrowserProvider, + browser: Awaited>, +) { + const produced = await provider.inspectPage( + inspectWebPageInputSchema.parse({ + cdp_endpoint: browser.endpoint, + target_id: "allowed-page", + allowed_origins: [browser.allowedOrigin, "https://private.example.test"], + observation_ms: 0, + include_json_body_shapes: true, + include_websocket_shapes: true, + }), + ); + if (!produced.ok) throw produced.error; + // Capture order must not turn identical frame identities into a DOM change. + produced.value.frames = [null, browser.allowedOrigin, ""].map( + (origin, index) => ({ + frame_id: String(index), + parent_frame_id: null, + url: "about:blank", + origin, + }), + ); + const reordered = structuredClone(produced.value); + reordered.frames.reverse(); + return { + before: { inspection: produced.value }, + after: { inspection: reordered }, + }; +} + describe("browser comparison input boundary", () => { it("advertises complete producer captures and agrees with SDK validation on both comparison families", async () => { const browser = await startFakeCdpBrowser({ sensitiveShapes: true }); @@ -51,40 +83,12 @@ describe("browser comparison input boundary", () => { ); if (advertised === undefined) throw new Error("Missing comparison tool"); expect(advertised.inputSchema.type).toBe("object"); - expect(advertised.inputSchema.anyOf).toHaveLength(2); + // Anthropic tool input schemas reject root combinators. + expect(advertised.inputSchema).not.toHaveProperty("anyOf"); const ajv = new Ajv2020({ strict: false, validateFormats: false }); expect(ajv.validateSchema(advertised.inputSchema)).toBe(true); const validate = ajv.compile(advertised.inputSchema); - const produced = await provider.inspectPage( - inspectWebPageInputSchema.parse({ - cdp_endpoint: browser.endpoint, - target_id: "allowed-page", - allowed_origins: [ - browser.allowedOrigin, - "https://private.example.test", - ], - observation_ms: 0, - include_json_body_shapes: true, - include_websocket_shapes: true, - }), - ); - if (!produced.ok) throw produced.error; - // The same frame URL can occur under different origins. Capture order - // must not turn identical frame identities into a DOM change. - produced.value.frames = [null, browser.allowedOrigin, ""].map( - (origin, index) => ({ - frame_id: String(index), - parent_frame_id: null, - url: "about:blank", - origin, - }), - ); - const reordered = structuredClone(produced.value); - reordered.frames.reverse(); - const passive = { - before: { inspection: produced.value }, - after: { inspection: reordered }, - }; + const passive = await capturePassivePair(provider, browser); const scenario = contract.examples[0]?.input; if (scenario === undefined) throw new Error("Missing scenario example"); const withoutNormalization = Object.fromEntries( @@ -99,12 +103,19 @@ describe("browser comparison input boundary", () => { { before: { inspection: {} }, after: passive.after }, { before: passive.before, - after: { inspection: produced.value, webmcp: {} }, + after: { ...passive.after, webmcp: {} }, }, { before_scenario: {}, after_scenario: {} }, { before_scenario: scenario.before_scenario }, - { ...passive, ...scenario }, { ...passive, normalization: { rules: [] } }, + { + before_scenario: scenario.before_scenario, + normalization: { rules: [] }, + }, + { + after_scenario: scenario.after_scenario, + normalization: { rules: [] }, + }, { ...passive, unexpected: true }, { ...scenario, @@ -119,26 +130,39 @@ describe("browser comparison input boundary", () => { }, }, ]; - const cases = [ + // The projection leaves group exclusion to the canonical runtime parser. + const mixedGroups = [{ ...passive, ...scenario }]; + for (const { input, advertisedValid, runtimeValid } of [ ...[passive, scenario, withoutNormalization].map((input) => ({ input, - expected: true, + advertisedValid: true, + runtimeValid: true, })), - ...invalid.map((input) => ({ input, expected: false })), - ]; - for (const { input, expected } of cases) { - expect(validate(input), JSON.stringify(validate.errors)).toBe(expected); + ...invalid.map((input) => ({ + input, + advertisedValid: false, + runtimeValid: false, + })), + ...mixedGroups.map((input) => ({ + input, + advertisedValid: true, + runtimeValid: false, + })), + ]) { + expect(validate(input), JSON.stringify(validate.errors)).toBe( + advertisedValid, + ); expect( browserCaptureComparisonInputSchema.safeParse(input).success, - ).toBe(expected); + ).toBe(runtimeValid); const beforeCalls = calls; const result = await client.callTool({ name: contract.name, arguments: input, }); - expect(calls - beforeCalls).toBe(expected ? 1 : 0); - expect(result.isError === true).toBe(!expected); - if (expected) + expect(calls - beforeCalls).toBe(runtimeValid ? 1 : 0); + expect(result.isError === true).toBe(!runtimeValid); + if (runtimeValid) expect(result.structuredContent).toMatchObject({ operation: contract.name, normalized_result: compareBrowserCaptures( @@ -147,6 +171,10 @@ describe("browser comparison input boundary", () => { }); } const parsedPassive = browserCaptureComparisonInputSchema.parse(passive); + expect(parsedPassive).toEqual({ + before: { ...passive.before, webmcp: null }, + after: { ...passive.after, webmcp: null }, + }); expect(compareBrowserCaptures(parsedPassive)).toMatchObject({ dimensions: { dom_structure: { status: "unchanged" } }, }); diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index 7a0fc43ad..1540964fd 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -396,15 +396,18 @@ describe("MCP root input schemas", () => { expect(ajv.compile(graphTool.inputSchema)({ unrelated: true })).toBe( false, ); + expect(graphTool.inputSchema.description).toContain( + "Provide a complete input group", + ); + expect(graphTool.inputSchema.description).not.toContain("exactly one"); for (const contract of TOOL_CONTRACTS) { const inputSchema = advertised.get(contract.name)!.inputSchema; expect(inputSchema.type, contract.name).toBe("object"); - expect( - inputSchema.properties !== undefined || - Array.isArray(inputSchema.anyOf), - contract.name, - ).toBe(true); + expect(inputSchema.properties, contract.name).toBeDefined(); + // Anthropic tool input schemas reject root combinators. + for (const combinator of ["anyOf", "oneOf", "allOf"]) + expect(inputSchema, contract.name).not.toHaveProperty(combinator); const validate = ajv.compile(inputSchema); for (const example of contract.examples) expect(