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(