diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 8ff488ee50..d0e581671b 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -1253,6 +1253,22 @@ Kimi-K3 backend, not the WorkBuddy MCP client. Set the profile on REA's registration in the actual MCP host; the host's name alone does not identify the downstream model or its schema constraints. +The compact profile also protects strict Anthropic-compatible gateways from +known schema-translation failures reported in [#3800](https://github.com/morluto/rea/issues/3800): +primitive `enum` properties without a sibling `type`, and `oneOf`/`allOf` +inside a property listed in `required`. Affected schemas use the same reduced +first-level projection described below, so the gateway receives a plain object +shape while REA continues to validate the complete canonical contract locally. +This is a compatibility projection, not a claim that the gateway reproduces +the full union semantics. + +For array items expressed as an untyped `oneOf`/`anyOf`, the compact profile +adds a type when every branch has the same type. Mixed-type item unions use the +reduced projection; selecting one branch's type would exclude valid calls. +This addresses the emitted shape reported through a Gemini-backed gateway in +[#5201](https://github.com/morluto/rea/issues/5201). Gateway conversion and +live provider acceptance have not been verified for this projection. + The compact profile renders the same tool set, output schemas, annotations, and canonical server-side validation. It changes only the advertised input JSON Schema: @@ -1284,6 +1300,11 @@ Boundary tests pin every advertised compact schema to the budget, keep them valid JSON Schema that still accepts every canonical example. Client verification also checks provider-facing sizes and canonical examples after conversion by the actual Kimi Code CLI; local captures do not establish Moonshot acceptance. +The SDK catalog test checks that full-profile schemas retain nested ADB extra, +managed selection, and WASM timeout and detail restrictions. Path-contract tests +check full-profile local-path rules. Compact MCP workflow tests verify canonical +rejection of invalid inputs before provider work, independent of how much detail +the selected advertisement retains. ### Output schemas diff --git a/docs/testing.md b/docs/testing.md index 36544c0cf6..8124da3f3d 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -475,6 +475,19 @@ pass that definition through the SDK's `toolDefinition` call option. This keeps the selected output-schema validation without eagerly compiling unrelated catalog schemas. Re-read definitions if the server advertises a changed list. +For exact nested input constraints, select the full profile explicitly or use +the full-SDK conformance suite that owns the field. Compact workflow tests +should assert canonical failure categories and provider-call controls where +rejection before provider work matters. Do not make today's omission of a +constraint a compact-profile invariant. + +When a file repeatedly validates the same MCP schemas, use +`createMcpSchemaValidation` for file-owned preparation and clear it in +`afterAll`. Its content keys include the complete schema, and the SDK provider +keeps dialect and format validation. Keep sessions and connections scoped to +each test; share immutable schema preparation through the fixture's validator +dependency. + Artifact verification uses `artifactMcpCall` for both lifecycle and analysis calls. A fresh SDK client has no held tool listing, so a bare `callTool` can skip output-schema validation. The helper discovers actual advertised diff --git a/src/contracts/compactInputSchema.test.ts b/src/contracts/compactInputSchema.test.ts index b2bc919feb..3a1f3b9541 100644 --- a/src/contracts/compactInputSchema.test.ts +++ b/src/contracts/compactInputSchema.test.ts @@ -224,6 +224,143 @@ describe("compact advertised input schema", () => { }); }); +describe("strict gateway compatibility", () => { + it("types homogeneous union items without discarding their branches", () => { + const original = { + type: "object", + properties: { + actions: { + type: "array", + items: { + oneOf: [ + { + type: "object", + properties: { kind: { const: "click" } }, + required: ["kind"], + }, + { + type: "object", + properties: { kind: { const: "type" } }, + required: ["kind"], + }, + ], + }, + }, + }, + }; + const { schema, presentation } = compactAdvertisedInputSchema( + original, + 13 * 1024, + ); + expect(presentation).toBe("inline"); + const actions = ( + schema.properties as Record> + ).actions; + if (actions === undefined) throw new Error("Missing actions property"); + expect(actions.items).toMatchObject({ + type: "object", + oneOf: original.properties.actions.items.oneOf, + }); + const canonical = new Ajv2020({ strict: false }).compile(original); + const advertised = new Ajv2020({ strict: false }).compile(schema); + for (const value of [ + [], + [{ kind: "click" }], + [{ kind: "type" }], + [{ kind: "other" }], + ]) + expect(advertised({ actions: value })).toBe( + canonical({ actions: value }), + ); + }); + + it("reduces mixed-type union items instead of selecting one branch type", () => { + const { schema, presentation } = compactAdvertisedInputSchema( + { + type: "object", + properties: { + values: { + type: "array", + items: { anyOf: [{ type: "string" }, { type: "object" }] }, + }, + }, + }, + 13 * 1024, + ); + expect(presentation).toBe("reduced"); + const values = ( + schema.properties as Record> + ).values; + if (values === undefined) throw new Error("Missing values property"); + expect(values.type).toBe("array"); + expect(values.items).toMatchObject({ + type: expect.arrayContaining(["string", "object"]), + }); + const validate = new Ajv2020({ strict: false }).compile(schema); + expect(validate({ values: ["text", {}] })).toBe(true); + }); + + it("reduces gateway-incompatible enums to explicitly typed properties", () => { + const { schema, presentation } = compactAdvertisedInputSchema( + { + type: "object", + properties: { mode: { enum: ["fast", "safe"] } }, + required: ["mode"], + }, + 13 * 1024, + ); + expect(presentation).toBe("reduced"); + expect(schema.properties).toEqual({ mode: { type: "string" } }); + }); + + it("reduces nested combinators instead of advertising them to strict gateways", () => { + const { schema, presentation } = compactAdvertisedInputSchema( + { + type: "object", + properties: { + query: { + type: "object", + properties: { + seed: { + oneOf: [ + { type: "object", properties: { kind: { type: "string" } } }, + ], + }, + }, + required: ["seed"], + }, + }, + required: ["query"], + }, + 13 * 1024, + ); + expect(presentation).toBe("reduced"); + expect(JSON.stringify(schema)).not.toContain("oneOf"); + expect(schema.properties).toEqual({ query: { type: "object" } }); + }); + + it("does not confuse keyword-named fields with schema keywords", () => { + const { schema, presentation } = compactAdvertisedInputSchema( + { + type: "object", + properties: { + enum: { type: "string" }, + oneOf: { type: "string" }, + default: { type: "boolean" }, + }, + required: ["enum", "oneOf"], + }, + 13 * 1024, + ); + expect(presentation).toBe("inline"); + expect(schema.properties).toMatchObject({ + enum: { type: "string" }, + oneOf: { type: "string" }, + default: { type: "boolean" }, + }); + }); +}); + describe("compact input schema budget and reduction", () => { it("budgets repeated-reference expansion before materializing it", () => { const definitions: Record = { diff --git a/src/contracts/compactInputSchema.ts b/src/contracts/compactInputSchema.ts index 1a11749feb..3df9af7bce 100644 --- a/src/contracts/compactInputSchema.ts +++ b/src/contracts/compactInputSchema.ts @@ -30,6 +30,17 @@ const jsonValueTypes = [ // arrays, while satisfying clients that require `items` for array types. const jsonValueSchema = () => ({ type: [...jsonValueTypes], items: {} }); +const enumValueTypes = (values: readonly unknown[]): readonly string[] => [ + ...new Set( + values.map((value) => { + if (value === null) return "null"; + if (Array.isArray(value)) return "array"; + if (typeof value === "object") return "object"; + return typeof value; + }), + ), +]; + export type AdvertisedInputSchemaPresentation = "inline" | "reduced"; /** Budget selected by the caller for compact MCP input-schema presentation. */ @@ -294,6 +305,11 @@ const detectedTypes = ( schema.type.every((type) => typeof type === "string") ) return schema.type; + if (Array.isArray(schema.enum) && schema.enum.length > 0) { + const enumTypes = enumValueTypes(schema.enum); + if (enumTypes.every((type) => jsonValueTypes.includes(type))) + return enumTypes; + } const alternatives = Array.isArray(schema.anyOf) ? schema.anyOf : Array.isArray(schema.oneOf) @@ -307,6 +323,89 @@ const detectedTypes = ( return [...new Set(types.flatMap((type) => type ?? []))]; }; +/** Give homogeneous array-item unions the type required by strict gateways. */ +const typeArrayItemUnions = ( + schema: Record, +): Record => { + const references = new JsonSchemaReferences(schema); + const visit = (node: unknown): unknown => { + const rebuilt = rebuildSchemaPositions(node, visit); + if (!isSchemaObject(rebuilt) || rebuilt.type !== "array") return rebuilt; + const items = rebuilt.items; + if ( + !isSchemaObject(items) || + items.type !== undefined || + (!Array.isArray(items.oneOf) && !Array.isArray(items.anyOf)) + ) + return rebuilt; + const types = detectedTypes(items, references, references.rootScope); + return types?.length === 1 + ? { ...rebuilt, items: { ...items, type: types[0] } } + : rebuilt; + }; + return visit(schema) as Record; +}; + +/** + * Strict gateways reject some nested combinators, primitive enums without a + * type, and array items whose union has no explicit type. Homogeneous item + * unions gain their shared type; remaining incompatible shapes use the shallow + * projection while canonical runtime validation stays intact. + */ +const hasGatewayIncompatibleShape = (schema: unknown): boolean => { + if (Array.isArray(schema)) return schema.some(hasGatewayIncompatibleShape); + if (!isSchemaObject(schema)) return false; + if (Array.isArray(schema.anyOf) && schema.anyOf.length === 0) return true; + if ( + schema.type === "array" && + isSchemaObject(schema.items) && + schema.items.type === undefined && + (Array.isArray(schema.items.oneOf) || Array.isArray(schema.items.anyOf)) + ) + return true; + if (Array.isArray(schema.required) && isSchemaObject(schema.properties)) { + for (const name of schema.required) { + const property = schema.properties[name]; + if (isSchemaObject(property) && containsGatewayCombinator(property)) + return true; + } + } + if ( + Array.isArray(schema.enum) && + schema.enum.length > 0 && + schema.type === undefined + ) { + const types = enumValueTypes(schema.enum); + if ( + types.length === 1 && + types.every((type) => + ["string", "number", "boolean", "null"].includes(type), + ) + ) + return true; + } + return schemaChildrenAndMaps(schema).some(hasGatewayIncompatibleShape); +}; + +const containsGatewayCombinator = (schema: unknown): boolean => { + if (Array.isArray(schema)) return schema.some(containsGatewayCombinator); + if (!isSchemaObject(schema)) return false; + if (schema.oneOf !== undefined || schema.allOf !== undefined) return true; + return schemaChildrenAndMaps(schema).some(containsGatewayCombinator); +}; + +/** Traverse only schema-valued positions; property names remain opaque data. */ +const schemaChildrenAndMaps = ( + schema: Readonly>, +): readonly unknown[] => + Object.entries(schema).flatMap(([key, value]) => { + if (schemaMaps.has(key) && isSchemaObject(value)) + return Object.values(value); + if (schemaArrays.has(key) && Array.isArray(value)) return value; + if (schemaChildren.has(key)) return Array.isArray(value) ? value : [value]; + return []; + }); + const REDUCED_ANNOTATION = "Advertised in reduced form by the compact input schema profile; the server still validates complete canonical inputs."; @@ -388,8 +487,15 @@ export const compactAdvertisedInputSchema = ( schema: Record, budgetBytes: number, ): CompactedInputSchema => { - const inline = inlineWithinBudget(stripAnnotations(schema), budgetBytes); - if (inline !== undefined && schemaDepth(inline) <= COMPACT_SCHEMA_MAX_DEPTH) + const expanded = inlineWithinBudget(stripAnnotations(schema), budgetBytes); + const inline = + expanded === undefined ? undefined : typeArrayItemUnions(expanded); + if ( + inline !== undefined && + serializedBytes(inline) <= budgetBytes && + schemaDepth(inline) <= COMPACT_SCHEMA_MAX_DEPTH && + !hasGatewayIncompatibleShape(inline) + ) return { schema: inline, presentation: "inline" }; const reduced = reduceToShallowProperties(schema); if (serializedBytes(reduced) <= budgetBytes) diff --git a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts index cbff37d2c4..fb23d2bec6 100644 --- a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts +++ b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts @@ -74,6 +74,8 @@ const standaloneAnalysisCases = ( ): readonly PathInputCase[] => [ { name: "inspect_go_binary", input: {} }, + { name: "inspect_recorded_crash", input: {} }, + { name: "inspect_binary_layout", input: {} }, { name: "inspect_rust_binary", input: {} }, { name: "inspect_evm_interface", input: { encoding: "hex" } }, { name: "inspect_sqlite_database", input: {} }, @@ -130,6 +132,24 @@ const inputCases = ( { path: absolute }, { path: relative }, ), + pathCase( + "import_evidence_bundle", + "path", + { path: absolute }, + { path: relative }, + ), + pathCase( + "compare_bundles", + "left_bundle_path", + { left_bundle_path: absolute, right_bundle_path: absolute }, + { left_bundle_path: relative, right_bundle_path: absolute }, + ), + pathCase( + "compare_bundles", + "right_bundle_path", + { left_bundle_path: absolute, right_bundle_path: absolute }, + { left_bundle_path: absolute, right_bundle_path: relative }, + ), pathCase( "binary_session", "expected_server_path", @@ -221,10 +241,36 @@ it("advertises full host-absolute path constraints after SDK schema conversion", const absolute = resolve("rea-absolute-input"); const relative = "relative-input"; - for (const { name, field, valid, invalid } of inputCases( - absolute, - relative, - )) { + for (const { name, field, valid, invalid } of [ + ...inputCases(absolute, relative), + { + name: "inspect_go_binary", + field: "path NUL", + valid: { path: absolute }, + invalid: { path: `${absolute}\0` }, + }, + { + name: "import_evidence_bundle", + field: "path NUL", + valid: { path: absolute }, + invalid: { path: `${absolute}\0` }, + }, + { + name: "compare_bundles", + field: "left_bundle_path empty", + valid: { left_bundle_path: absolute, right_bundle_path: absolute }, + invalid: { left_bundle_path: "", right_bundle_path: absolute }, + }, + { + name: "compare_bundles", + field: "left_bundle_path NUL", + valid: { left_bundle_path: absolute, right_bundle_path: absolute }, + invalid: { + left_bundle_path: `${absolute}\0`, + right_bundle_path: absolute, + }, + }, + ]) { const schema = listed.tools.find( (tool) => tool.name === name, )?.inputSchema; diff --git a/tests/boundary/mcp/adbStartMcp.test.ts b/tests/boundary/mcp/adbStartMcp.test.ts index 10670561d9..2f735ded6f 100644 --- a/tests/boundary/mcp/adbStartMcp.test.ts +++ b/tests/boundary/mcp/adbStartMcp.test.ts @@ -1,3 +1,4 @@ +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { Ajv2020 } from "ajv/dist/2020.js"; import { createHash } from "node:crypto"; @@ -142,10 +143,10 @@ const assertAdvertisedActivityExtras = async ( if (advertised === undefined) throw new Error("Missing ADB activity tool"); const ajv = new Ajv2020({ strict: false, validateFormats: false }); expect(ajv.validateSchema(advertised.inputSchema)).toBe(true); - const validateInput = ajv.compile(advertised.inputSchema); - expect(validateInput(input)).toBe(true); + const validateCompactInput = ajv.compile(advertised.inputSchema); + expect(validateCompactInput(input)).toBe(true); expect( - validateInput({ + validateCompactInput({ ...input, extras: [ { key: "bool", type: "boolean", value: "false" }, @@ -155,18 +156,7 @@ const assertAdvertisedActivityExtras = async ( ], }), ).toBe(true); - for (const [type, value] of [ - ["boolean", ""], - ["boolean", "yes"], - ["int", "1.5"], - ["long", ""], - ["float", "garbage"], - ]) - expect( - validateInput({ ...input, extras: [{ key: "invalid", type, value }] }), - `${type}: ${value}`, - ).toBe(false); - + // Canonical validation rejects malformed extras before starting ADB. const callsBeforeInvalidExtra = await calls(); const invalidExtra = await client.callTool({ name: "start_adb_activity", @@ -175,7 +165,10 @@ const assertAdvertisedActivityExtras = async ( extras: [{ key: "invalid", type: "boolean", value: "yes" }], }, }); - expect(invalidExtra.isError).toBe(true); + expect(parseMcpToolError(invalidExtra).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); expect(await calls()).toEqual(callsBeforeInvalidExtra); }; diff --git a/tests/boundary/mcp/apktoolMcp.test.ts b/tests/boundary/mcp/apktoolMcp.test.ts index 11da039440..751df14149 100644 --- a/tests/boundary/mcp/apktoolMcp.test.ts +++ b/tests/boundary/mcp/apktoolMcp.test.ts @@ -84,7 +84,10 @@ it("advertises exact valid Apktool schemas and records inline evidence through M }); const server = createServer( { kind: "session", session }, - { apktoolAnalysis: provider }, + { + apktoolAnalysis: provider, + environment: { ...process.env, REA_MCP_INPUT_SCHEMA_PROFILE: "full" }, + }, ); onTestFinished(() => server.close()); const [clientTransport, serverTransport] = diff --git a/tests/boundary/mcp/binaryDiagnosticsMcp.test.ts b/tests/boundary/mcp/binaryDiagnosticsMcp.test.ts index a083083f37..1e64477190 100644 --- a/tests/boundary/mcp/binaryDiagnosticsMcp.test.ts +++ b/tests/boundary/mcp/binaryDiagnosticsMcp.test.ts @@ -199,7 +199,7 @@ it.each([false, true])( }, ); -it("advertises exact valid schemas and records inline layout without a binary target", async () => { +it("records inline layout with compact schemas and canonical path checks", async () => { const value = binaryLayoutFixture(); let providerClosed = false; let providerCalls = 0; @@ -240,7 +240,6 @@ it("advertises exact valid schemas and records inline layout without a binary ta ); const { ajv, inputSchema } = schemas; expect(ajv.validate(inputSchema, { path: value.artifact.path })).toBe(true); - expect(ajv.validate(inputSchema, { path: "relative.elf" })).toBe(false); expect( ajv.validate(inputSchema, { path: value.artifact.path, @@ -279,14 +278,11 @@ it("advertises exact valid schemas and records inline layout without a binary ta parent_evidence_id: evidence.evidence_id, summary: { format: "elf", bytes: value.artifact.bytes }, }); - expect( - ( - await client.callTool({ - name: "inspect_binary_layout", - arguments: { path: "relative.elf" }, - }) - ).isError, - ).toBe(true); + const relative = await client.callTool({ + name: "inspect_binary_layout", + arguments: { path: "relative.elf" }, + }); + expect(parseMcpToolError(relative).error.code).toBe("invalid_request"); expect(providerCalls).toBe(1); }); @@ -391,9 +387,6 @@ it("records a portable export inventory through MCP without a binary target", as "inspect_native_exports", ); expect(schemas.ajv.validate(schemas.inputSchema, { path })).toBe(true); - expect( - schemas.ajv.validate(schemas.inputSchema, { path: "relative.so" }), - ).toBe(false); const response = await client.callTool({ name: "inspect_native_exports", arguments: { path }, @@ -426,4 +419,12 @@ it("records a portable export inventory through MCP without a binary target", as }), "observed", ); + const relative = await client.callTool({ + name: "inspect_native_exports", + arguments: { path: "relative.so" }, + }); + expect(parseMcpToolError(relative).error).toMatchObject({ + code: "invalid_request", + details: { issues: [{ path: ["path"], reason: "invalid_format" }] }, + }); }); diff --git a/tests/boundary/mcp/bundleComparisonMcp.test.ts b/tests/boundary/mcp/bundleComparisonMcp.test.ts index a7e890e99b..b80c9ccd9c 100644 --- a/tests/boundary/mcp/bundleComparisonMcp.test.ts +++ b/tests/boundary/mcp/bundleComparisonMcp.test.ts @@ -4,7 +4,6 @@ import { writeFile } from "node:fs/promises"; import { join } from "node:path"; import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; -import { Ajv2020 } from "ajv/dist/2020.js"; import { describe, expect, it, vi } from "vitest"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; @@ -162,7 +161,10 @@ it("requires absolute bundle paths without changing server-cwd semantics", async ] as const; for (const [leftPath, rightPath, rejectedField] of relativeCases) { const result = await compare(leftPath, rightPath); - expect(result.isError).toBe(true); + expect(parseMcpToolError(result).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); expect(result.content[0]).toMatchObject({ text: expect.stringContaining( `${rejectedField}: must be an absolute local filesystem path`, @@ -186,53 +188,17 @@ it("requires absolute bundle paths without changing server-cwd semantics", async ok: true, value: { status: "unchanged" }, }); - const listed = await connected.client.listTools(); - const comparisonTool = listed.tools.find( - ({ name }) => name === "compare_bundles", - ); - if (comparisonTool === undefined) - throw new Error("Expected the compare_bundles MCP contract"); - const validate = new Ajv2020({ - strict: false, - validateFormats: false, - }).compile(comparisonTool.inputSchema); - const absolutePaths = { - left_bundle_path: join(root, "left.json"), - right_bundle_path: join(root, "right.json"), - }; - expect(validate(absolutePaths)).toBe(true); - expect(validate({ ...absolutePaths, left_bundle_path: "left.json" })).toBe( - false, - ); - expect(validate({ ...absolutePaths, left_bundle_path: "" })).toBe(false); - expect( - validate({ - ...absolutePaths, - left_bundle_path: `${absolutePaths.left_bundle_path}\0suffix`, - }), - ).toBe(false); const relativeImport = await connected.client.callTool({ name: "import_evidence_bundle", arguments: { path: "left.json" }, }); - expect(relativeImport.isError).toBe(true); + expect(parseMcpToolError(relativeImport).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); expect(relativeImport.content[0]).toMatchObject({ text: expect.stringContaining("absolute local filesystem path"), }); - const importTool = listed.tools.find( - ({ name }) => name === "import_evidence_bundle", - ); - if (importTool === undefined) - throw new Error("Expected the import_evidence_bundle MCP contract"); - const validateImport = new Ajv2020({ - strict: false, - validateFormats: false, - }).compile(importTool.inputSchema); - expect(validateImport({ path: absolutePaths.left_bundle_path })).toBe(true); - expect(validateImport({ path: "left.json" })).toBe(false); - expect( - validateImport({ path: `${absolutePaths.left_bundle_path}\0` }), - ).toBe(false); const absolute = await compare( join(root, "left.json"), join(root, "right.json"), @@ -338,7 +304,6 @@ describe("bundle comparison MCP integration", () => { right_bundle_path: bundlePath, }, }); - expect(result.isError).toBe(true); expect(parseMcpToolError(result)).toMatchObject({ error: { code: "invalid_request", diff --git a/tests/boundary/mcp/compactInputSchemaMcp.test.ts b/tests/boundary/mcp/compactInputSchemaMcp.test.ts index a6e71d6a1c..86a1b15b87 100644 --- a/tests/boundary/mcp/compactInputSchemaMcp.test.ts +++ b/tests/boundary/mcp/compactInputSchemaMcp.test.ts @@ -79,6 +79,39 @@ const unsupportedDependencyKeywords = ( }); }; +const untypedUnionItems = (schema: unknown, path = "$"): readonly string[] => { + if (typeof schema !== "object" || schema === null || Array.isArray(schema)) + return []; + const node = schema as Record; + const items = node.items; + const here = + node.type === "array" && + typeof items === "object" && + items !== null && + !Array.isArray(items) && + (Array.isArray((items as Record).oneOf) || + Array.isArray((items as Record).anyOf)) && + (items as Record).type === undefined + ? [`${path}.items`] + : []; + return [ + ...here, + ...Object.entries(node).flatMap(([key, value]) => { + if (schemaMaps.has(key) && typeof value === "object" && value !== null) + return Object.entries(value).flatMap(([name, child]) => + untypedUnionItems(child, `${path}.${key}.${name}`), + ); + if (schemaArrays.has(key) && Array.isArray(value)) + return value.flatMap((child, index) => + untypedUnionItems(child, `${path}.${key}[${index}]`), + ); + if (schemaChildren.has(key)) + return untypedUnionItems(value, `${path}.${key}`); + return []; + }), + ]; +}; + const listToolsThroughTransport = async ( server: EvidenceMcpServer, ): Promise => { @@ -200,6 +233,11 @@ it("advertises every tool input schema within the provider budget", async () => expect( tooDeep.map(({ name, inputSchema }) => [name, schemaDepth(inputSchema)]), ).toEqual([]); + expect( + tools.flatMap(({ name, inputSchema }) => + untypedUnionItems(inputSchema).map((path) => `${name}${path.slice(1)}`), + ), + ).toEqual([]); }); it("keeps output schemas and tool descriptions identical to the default profile", async () => { @@ -270,6 +308,7 @@ it("preserves arbitrary nested JSON values at the converted SDK boundary", async }); it("keeps SDK-converted schemas valid and accepting canonical examples", async () => { + let checkedFullRestrictions = false; for (const profile of ["full", "compact"] as const) { const server = profile === "full" ? fullServer() : compactServer(); registerCatalog(server); @@ -298,7 +337,84 @@ it("keeps SDK-converted schemas valid and accepting canonical examples", async ( `${profile} profile, ${tool.name}: ${example.title}`, ).toBe(true); } + if (profile === "full") { + const adb = tools.find(({ name }) => name === "start_adb_activity"); + const managed = tools.find( + ({ name }) => name === "inspect_managed_members", + ); + const wasm = tools.find(({ name }) => name === "inspect_wasm_artifact"); + if (adb === undefined || managed === undefined || wasm === undefined) + throw new Error("Missing ADB, managed member, or WASM inspection tool"); + const validateAdb = ajv.compile(adb.inputSchema); + const activity = { + serial: "emulator-5554", + action: "android.intent.action.MAIN", + }; + expect( + validateAdb({ + ...activity, + extras: [ + { key: "bool", type: "boolean", value: "false" }, + { key: "int", type: "int", value: "-3" }, + { key: "long", type: "long", value: "9007199254740993" }, + { key: "float", type: "float", value: "-1.25" }, + ], + }), + ).toBe(true); + for (const [type, value] of [ + ["boolean", ""], + ["boolean", "yes"], + ["int", "1.5"], + ["long", ""], + ["float", "garbage"], + ]) + expect( + validateAdb({ + ...activity, + extras: [{ key: "invalid", type, value }], + }), + `${type}: ${value}`, + ).toBe(false); + const validateManaged = ajv.compile(managed.inputSchema); + expect( + validateManaged({ + path: "/artifacts/program.dll", + type_names: ["Fixture.Program", "Missing.Type"], + }), + ).toBe(true); + expect( + validateManaged({ + path: "/artifacts/program.dll", + type_names: ["Fixture.Program", "Fixture.Program"], + }), + ).toBe(false); + const validateWasm = ajv.compile(wasm.inputSchema); + expect( + validateWasm({ path: "/artifacts/program", detail: "summary" }), + ).toBe(true); + expect( + validateWasm({ path: "/artifacts/program", detail: "brief" }), + ).toBe(false); + for (const command_timeout_ms of [1, 42_000, 2_147_483_647]) + expect( + validateWasm({ path: "/artifacts/program", command_timeout_ms }), + ).toBe(true); + for (const command_timeout_ms of [ + 0, + -1, + 1.5, + 2_147_483_648, + "1000", + null, + ]) + expect( + validateWasm({ path: "/artifacts/program", command_timeout_ms }), + String(command_timeout_ms), + ).toBe(false); + checkedFullRestrictions = true; + } } + expect(checkedFullRestrictions).toBe(true); }); it("omits Gemini-unsupported dependency keywords from both catalog profiles", async () => { diff --git a/tests/boundary/mcp/flutterMcp.test.ts b/tests/boundary/mcp/flutterMcp.test.ts index f4e50531b7..bc9b6a078c 100644 --- a/tests/boundary/mcp/flutterMcp.test.ts +++ b/tests/boundary/mcp/flutterMcp.test.ts @@ -210,7 +210,10 @@ const fakeProvider = (): FlutterBuildAnalysisPort => ({ it("advertises and enforces the same Unicode selector grammar through MCP", async () => { const harness = await createApplicationMcpHarness({ - serverOptions: { flutterAnalysis: fakeProvider() }, + serverOptions: { + flutterAnalysis: fakeProvider(), + environment: { ...process.env, REA_MCP_INPUT_SCHEMA_PROFILE: "full" }, + }, }); onTestFinished(harness.close); const tool = (await harness.client.listTools()).tools.find( diff --git a/tests/boundary/mcp/goBinaryMcp.test.ts b/tests/boundary/mcp/goBinaryMcp.test.ts index 8d41bac603..76b814ecc0 100644 --- a/tests/boundary/mcp/goBinaryMcp.test.ts +++ b/tests/boundary/mcp/goBinaryMcp.test.ts @@ -190,7 +190,6 @@ goMcpTest( name: "inspect_go_binary", arguments: { path }, }); - expect(response.isError).toBe(true); expect(parseMcpToolError(response)).toMatchObject({ error: { code: fixture.code, @@ -236,15 +235,22 @@ goMcpTest( expect( ajv.validate(inputSchema, { path: "/artifacts/program", execute: true }), ).toBe(false); - expect(ajv.validate(inputSchema, { path: "/artifacts/\0program" })).toBe( - false, - ); expect(advertised.annotations).toMatchObject({ readOnlyHint: false, destructiveHint: false, idempotentHint: true, openWorldHint: false, }); + const nulPath = await client.callTool({ + name: "inspect_go_binary", + arguments: { path: "/artifacts/\0program" }, + }); + expect(parseMcpToolError(nulPath)).toMatchObject({ + error: { + code: "invalid_request", + details: { issues: [{ path: ["path"], reason: "invalid_format" }] }, + }, + }); const root = await createTestTempDirectory("rea-go-mcp-"); const path = join(root, "application"); const fixture = createGoBinaryFixture({ @@ -278,7 +284,6 @@ goMcpTest( name: "inspect_go_binary", arguments: { path: "relative.bin" }, }); - expect(invalid.isError).toBe(true); expect(parseMcpToolError(invalid)).toMatchObject({ error: { code: "invalid_request", diff --git a/tests/boundary/mcp/goBinaryViews.test.ts b/tests/boundary/mcp/goBinaryViews.test.ts index 0788c62dd6..9b1698ac65 100644 --- a/tests/boundary/mcp/goBinaryViews.test.ts +++ b/tests/boundary/mcp/goBinaryViews.test.ts @@ -1,8 +1,6 @@ import { readFile, unlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; -import { expect } from "vitest"; -import { Ajv2020 } from "ajv/dist/2020.js"; -import type { AnySchema } from "ajv"; +import { afterAll, expect } from "vitest"; import type { Client } from "@modelcontextprotocol/client"; import { createEvidence, @@ -22,9 +20,12 @@ import { } from "../../fixtures/go/image.js"; import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { createMcpSchemaValidation } from "../../fixtures/mcpSchemaValidation.js"; import { mcpTest } from "../../support/mcp/mcpFixture.js"; type Operation = "inspect_go_binary" | "inspect_analysis_view"; +const schemaValidation = createMcpSchemaValidation(); +afterAll(() => schemaValidation.clear()); const goViewTest = mcpTest.extend<{ goViews: { client: Client; @@ -53,9 +54,10 @@ const goViewTest = mcpTest.extend<{ try { const client = await mcp.connect( createServer({ kind: "session", session }), + undefined, + schemaValidation, ); const catalog = await client.listTools(); - const ajv = new Ajv2020({ strict: false }); const schemas = new Map( catalog.tools .filter( @@ -64,31 +66,33 @@ const goViewTest = mcpTest.extend<{ tool.name === "inspect_analysis_view", ) .map((tool) => { - const output = tool.outputSchema as AnySchema; - expect(tool.outputSchema?.$schema).toBe( + const output = tool.outputSchema; + if (output === undefined) + throw new Error(`Missing ${tool.name} output schema`); + expect(output.$schema).toBe( "https://json-schema.org/draft/2020-12/schema", ); - expect(ajv.validateSchema(output)).toBe(true); - return [ - tool.name, - { - input: ajv.compile(tool.inputSchema), - output: ajv.compile(output), - }, - ] as const; + expect(schemaValidation.assertions.validateSchema(output)).toBe( + true, + ); + expect( + schemaValidation.assertions.validateSchema(tool.inputSchema), + ).toBe(true); + return [tool.name, tool] as const; }), ); const call = async (name: Operation, input: Record) => { - const schema = schemas.get(name)!; - expect(schema.input(input), JSON.stringify(schema.input.errors)).toBe( - true, - ); - const reply = await client.callTool({ name, arguments: input }); - expect(reply.isError, JSON.stringify(reply)).not.toBe(true); + const tool = schemas.get(name); + if (tool === undefined) throw new Error(`Missing ${name} tool`); expect( - schema.output(reply.structuredContent), - JSON.stringify(schema.output.errors), + schemaValidation.getValidator(tool.inputSchema)(input).valid, + `${name} rejected its valid input`, ).toBe(true); + const reply = await client.callTool( + { name, arguments: input }, + { toolDefinition: tool }, + ); + expect(reply.isError, JSON.stringify(reply)).not.toBe(true); return parseEvidence(reply.structuredContent); }; await use({ client, root, path, bytes: fixture.bytes, call }); @@ -315,7 +319,10 @@ goViewTest( }, }, }); - expect(invalid.isError).toBe(true); + expect(parseMcpToolError(invalid).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); }, ); diff --git a/tests/boundary/mcp/managedMcp.test.ts b/tests/boundary/mcp/managedMcp.test.ts index 82352c3bd2..3d1c5ba545 100644 --- a/tests/boundary/mcp/managedMcp.test.ts +++ b/tests/boundary/mcp/managedMcp.test.ts @@ -608,9 +608,6 @@ const verifyManagedTypeSelection = async ( expect( validate({ path, type_names: ["Fixture.Program", "Missing.Type"] }), ).toBe(true); - expect( - validate({ path, type_names: ["Fixture.Program", "Fixture.Program"] }), - ).toBe(false); const selected = structured( await client.callTool({ name: "inspect_managed_members", @@ -632,7 +629,9 @@ const verifyManagedTypeSelection = async ( name: "inspect_managed_members", arguments: { path, type_names: ["Fixture.Program", "Fixture.Program"] }, }); - expect(repeatedSelection.isError).toBe(true); + expect(parseMcpToolError(repeatedSelection).error.code).toBe( + "invalid_request", + ); const reselectedOtherTarget = structured( await client.callTool({ name: "inspect_managed_artifact", diff --git a/tests/boundary/mcp/recordedCrashMcp.test.ts b/tests/boundary/mcp/recordedCrashMcp.test.ts index e7b0623e7f..f52a1d72b5 100644 --- a/tests/boundary/mcp/recordedCrashMcp.test.ts +++ b/tests/boundary/mcp/recordedCrashMcp.test.ts @@ -7,12 +7,13 @@ import { parseEvidence } from "../../../src/domain/evidence.js"; import { ok } from "../../../src/domain/result.js"; import { createServer } from "../../../src/server/createServer.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; import { recordedCrashFixture, RECORDED_CRASH_TEST_PROVIDER, } from "../../fixtures/binaryDiagnostics/recordedCrash.js"; -it("advertises exact valid schemas and records inline recorded-core evidence without a binary target", async () => { +it("records inline recorded-core evidence with compact schemas and canonical path checks", async () => { const value = recordedCrashFixture(); let providerClosed = false; let providerCalls = 0; @@ -58,7 +59,6 @@ it("advertises exact valid schemas and records inline recorded-core evidence wit expect(ajv.validateSchema(inputSchema)).toBe(true); expect(ajv.validateSchema(outputSchema)).toBe(true); expect(ajv.validate(inputSchema, { path: value.artifact.path })).toBe(true); - expect(ajv.validate(inputSchema, { path: "relative.elf" })).toBe(false); expect( ajv.validate(inputSchema, { path: value.artifact.path, @@ -82,13 +82,10 @@ it("advertises exact valid schemas and records inline recorded-core evidence wit expect(evidence.subject?.digest.sha256).toBe(value.artifact.sha256); expect(parsed.normalized_result).toEqual(value); expect(session.evidenceById(evidence.evidence_id)).toEqual(evidence); - expect( - ( - await client.callTool({ - name: "inspect_recorded_crash", - arguments: { path: "relative.elf" }, - }) - ).isError, - ).toBe(true); + const relative = await client.callTool({ + name: "inspect_recorded_crash", + arguments: { path: "relative.elf" }, + }); + expect(parseMcpToolError(relative).error.code).toBe("invalid_request"); expect(providerCalls).toBe(1); }); diff --git a/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts b/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts index d449b51d4a..e57f135996 100644 --- a/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts +++ b/tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts @@ -53,8 +53,16 @@ it.each(["full", "compact"] as const)( strict: false, validateFormats: false, }).compile(tool.inputSchema); - for (const value of ["literal", 42, true, null, {}, []]) { - const primitive = value === null || typeof value !== "object"; + const cases = [ + { value: "literal", expectedPrimitive: true }, + { value: 42, expectedPrimitive: true }, + { value: true, expectedPrimitive: true }, + { value: null, expectedPrimitive: true }, + { value: {}, expectedPrimitive: false }, + { value: [], expectedPrimitive: false }, + ] as const; + let malformedChecks = 0; + for (const { value, expectedPrimitive } of cases) { const arguments_ = { application, query: { @@ -62,15 +70,24 @@ it.each(["full", "compact"] as const)( direction: "backward-provenance", }, }; - if (primitive || profile === "full") - expect(validate(arguments_), JSON.stringify(value)).toBe(primitive); + if (expectedPrimitive || profile === "full") + expect(validate(arguments_), JSON.stringify(value)).toBe( + expectedPrimitive, + ); const result = await client.callTool({ name: tool.name, arguments: arguments_, }); - if (primitive) expect(result.isError).not.toBe(true); - else expect(result.isError).toBe(true); + if (expectedPrimitive) expect(result.isError).not.toBe(true); + else { + malformedChecks += 1; + expect(parseMcpToolError(result).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); + } } + expect(malformedChecks).toBe(2); }, ); diff --git a/tests/boundary/mcp/wasmArtifactMcp.test.ts b/tests/boundary/mcp/wasmArtifactMcp.test.ts index 9413177b72..0de931dbd2 100644 --- a/tests/boundary/mcp/wasmArtifactMcp.test.ts +++ b/tests/boundary/mcp/wasmArtifactMcp.test.ts @@ -37,9 +37,6 @@ const assertFirstSummaryDelivery = async ( } = await advertisedMcpToolSchemas(harness.client, "inspect_wasm_artifact"); const summaryInput = { path: value.artifact.path, detail: "summary" }; expect(wasmAjv.validate(inputSchema, summaryInput)).toBe(true); - expect( - wasmAjv.validate(inputSchema, { ...summaryInput, detail: "brief" }), - ).toBe(false); const directSummary = await harness.client.callTool({ name: "inspect_wasm_artifact", arguments: summaryInput, @@ -139,7 +136,6 @@ it.each(["bin", "hop"])( name: recoveryName ?? "", arguments: recoveryInput, }); - expect(inspection.isError).toBe(true); expect(parseMcpToolError(inspection)).toMatchObject({ error: { code: "capability_unavailable", @@ -170,7 +166,7 @@ it.each(["bin", "hop"])( }, ); -it("advertises exact valid schemas and records inline WASM interface evidence without a binary target", async () => { +it("records inline WASM evidence with compact inputs and canonical option checks", async () => { const value = wasmArtifactFixture(); value.tool_profile.timeout_ms = 42_000; const close = vi.fn(() => Promise.resolve()); @@ -181,6 +177,7 @@ it("advertises exact valid schemas and records inline WASM interface evidence wi return Promise.resolve(ok(wasmArtifactExecution(value))); }, }); + const inspect = vi.spyOn(service.provider, "inspect"); const session = createTestBinarySession(() => { throw new Error("deep provider must not start"); }); @@ -203,13 +200,6 @@ it("advertises exact valid schemas and records inline WASM interface evidence wi command_timeout_ms, }), ).toBe(true); - for (const command_timeout_ms of [0, -1, 1.5, 2_147_483_648, "1000", null]) - expect( - ajv.validate(inputSchema, { - path: value.artifact.path, - command_timeout_ms, - }), - ).toBe(false); expect( ajv.validate(inputSchema, { path: value.artifact.path, @@ -229,14 +219,27 @@ it("advertises exact valid schemas and records inline WASM interface evidence wi value, "observed", ); - expect( - ( - await client.callTool({ - name: "inspect_wasm_artifact", - arguments: { path: "relative.hex" }, - }) - ).isError, - ).toBe(true); + expect(inspect).toHaveBeenCalledTimes(1); + const invalidDetail = await client.callTool({ + name: "inspect_wasm_artifact", + arguments: { path: value.artifact.path, detail: "brief" }, + }); + expect(parseMcpToolError(invalidDetail).error.code).toBe("invalid_request"); + expect(inspect).toHaveBeenCalledTimes(1); + for (const command_timeout_ms of [0, -1, 1.5, 2_147_483_648, "1000", null]) { + const invalid = await client.callTool({ + name: "inspect_wasm_artifact", + arguments: { path: value.artifact.path, command_timeout_ms }, + }); + expect(parseMcpToolError(invalid).error.code).toBe("invalid_request"); + expect(inspect).toHaveBeenCalledTimes(1); + } + const relative = await client.callTool({ + name: "inspect_wasm_artifact", + arguments: { path: "relative.hex" }, + }); + expect(parseMcpToolError(relative).error.code).toBe("invalid_request"); + expect(inspect).toHaveBeenCalledTimes(1); await harness.close(); expect(close).toHaveBeenCalled(); }); diff --git a/tests/boundary/mcp/webNetworkCaptureMcp.test.ts b/tests/boundary/mcp/webNetworkCaptureMcp.test.ts index 5b5cf75902..f9281085b3 100644 --- a/tests/boundary/mcp/webNetworkCaptureMcp.test.ts +++ b/tests/boundary/mcp/webNetworkCaptureMcp.test.ts @@ -99,14 +99,20 @@ it("publishes valid SDK schemas and retains historical inline Evidence with the name: "inspect_web_network_capture", arguments: { ...args, capture_path: "relative.har" }, }); - expect(invalid.isError).toBe(true); + expect(parseMcpToolError(invalid).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); const invalidSensitive = await client.callTool({ name: "inspect_web_network_capture", arguments: { ...args, sensitive_values: ["secret"], mysecret: true }, }); - expect(invalidSensitive.isError).toBe(true); expect(parseMcpToolError(invalidSensitive)).toMatchObject({ - error: { code: "invalid_request", details: { issues: [{ path: [] }] } }, + error: { + code: "invalid_request", + category: "invalid_input", + details: { issues: [{ path: [] }] }, + }, }); expect(JSON.stringify(invalidSensitive)).not.toContain("secret"); expect(ajv.validate(inputSchema, { ...args, mysecret: true })).toBe(false); diff --git a/tests/boundary/mcp/webRuntimeMcp.test.ts b/tests/boundary/mcp/webRuntimeMcp.test.ts index ab1927c8e9..915214b2ce 100644 --- a/tests/boundary/mcp/webRuntimeMcp.test.ts +++ b/tests/boundary/mcp/webRuntimeMcp.test.ts @@ -84,7 +84,10 @@ it("advertises actual SDK schemas, instrumentation effects and session-owned inl name: "observe_web_execution", arguments: { ...input, observation_ms: 2_147_483_648 }, }); - expect(invalid.isError).toBe(true); + expect(parseMcpToolError(invalid).error).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + }); }); cliTest( diff --git a/tests/fixtures/mcpSchemaValidation.ts b/tests/fixtures/mcpSchemaValidation.ts index 67300764bb..3e81a0a8d6 100644 --- a/tests/fixtures/mcpSchemaValidation.ts +++ b/tests/fixtures/mcpSchemaValidation.ts @@ -13,7 +13,9 @@ export const createMcpSchemaValidation = () => { const meta = new Ajv2020({ strict: false, validateFormats: false }); let errors: unknown = null; return { - getValidator(schema: JsonSchemaType): JsonSchemaValidator { + getValidator( + schema: JsonSchemaType | Readonly>, + ): JsonSchemaValidator { const key = JSON.stringify(schema); let validator = sdkValidators.get(key); if (validator === undefined) { diff --git a/tests/support/mcp/mcpFixture.ts b/tests/support/mcp/mcpFixture.ts index 340918eec0..07b663d847 100644 --- a/tests/support/mcp/mcpFixture.ts +++ b/tests/support/mcp/mcpFixture.ts @@ -1,4 +1,8 @@ -import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { + Client, + InMemoryTransport, + type ClientOptions, +} from "@modelcontextprotocol/client"; import type { McpServer } from "@modelcontextprotocol/server"; import { test as base } from "vitest"; @@ -11,7 +15,11 @@ export interface McpClientIdentity { /** Registry for production MCP servers and SDK clients owned by one test. */ export interface TestMcp { /** Transfer server ownership on admission; a closed registry leaves it with the caller. */ - connect(server: McpServer, identity?: McpClientIdentity): Promise; + connect( + server: McpServer, + identity?: McpClientIdentity, + jsonSchemaValidator?: NonNullable, + ): Promise; close(): Promise; } @@ -67,9 +75,16 @@ export const createTestMcp = (): TestMcp => { }; return { - connect: (server, identity = DEFAULT_CLIENT_IDENTITY) => { + connect: ( + server, + identity = DEFAULT_CLIENT_IDENTITY, + jsonSchemaValidator, + ) => { if (closed) return Promise.reject(registryClosed()); - const client = new Client(identity); + const client = new Client( + identity, + jsonSchemaValidator === undefined ? undefined : { jsonSchemaValidator }, + ); const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); const connection: McpConnectionOwner = {