diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 67a7c66c01..80606fb9ec 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -694,6 +694,20 @@ 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. +Boolean literal constraints in input schemas are relaxed only in this +advertisement. For example, a boolean `const` is presented as `type: boolean`; +an affected `not` is omitted, conditional predicates and their branches are +omitted together, pure boolean-literal `oneOf` branches collapse to a boolean, +and other affected `oneOf` branches are presented as `anyOf` to retain branch +guidance without making overlapping widened branches reject inputs. +This can make the advertised schema accept inputs the canonical contract +rejects. The MCP handler still validates every call with the canonical +contract. The Gemini CLI verifier checks the outgoing +`parametersJsonSchema` and canonical contract examples against that declaration +using a loopback model fixture; it does not establish live Google API or +OpenCode client behavior. Set `REA_VERIFY_GEMINI_SCHEMA_PROFILE=compact` to +verify the compact advertisement through the same client boundary. + ### Compact input schema profile Some function-calling providers reject a request while any single tool's diff --git a/scripts/verify-gemini-client.mjs b/scripts/verify-gemini-client.mjs index 8d1195f74c..a26613ea10 100644 --- a/scripts/verify-gemini-client.mjs +++ b/scripts/verify-gemini-client.mjs @@ -5,6 +5,11 @@ import { tmpdir } from "node:os"; import { join, resolve, sep } from "node:path"; import { fileURLToPath } from "node:url"; import Ajv2020 from "ajv/dist/2020.js"; +import { + schemaArrays, + schemaChildren, + schemaMaps, +} from "../dist/contracts/inputSchemaPresentation.js"; import { TOOL_CONTRACTS, toolContract, @@ -17,6 +22,8 @@ const repo = fileURLToPath(new URL("..", import.meta.url)); const runtimeRoot = process.env.REA_VERIFY_RUNTIME_ROOT ?? repo; const command = process.env.REA_VERIFY_GEMINI_COMMAND ?? "gemini"; const mode = process.argv[2] ?? "call"; +const schemaProfile = process.env.REA_VERIFY_GEMINI_SCHEMA_PROFILE ?? "full"; +assert(["full", "compact"].includes(schemaProfile)); // npm installs of Gemini CLI provide a JavaScript entry point; on Windows only a gemini.cmd shim reaches // PATH, which Node does not spawn without a shell, so a selected .js entry runs under this Node. It is // resolved once, because the preflight runs from the caller's directory and the client from the workspace. @@ -60,6 +67,7 @@ const environment = { GEMINI_CLI_NO_RELAUNCH: "1", GEMINI_CLI_TRUST_WORKSPACE: "true", REA_PROCESS_RUN_ID: verifier.run_id, + REA_MCP_INPUT_SCHEMA_PROFILE: schemaProfile, }; let clientVersion; try { @@ -161,8 +169,43 @@ let skillRequested = false; let finalSeen = false; let catalogSize = 0; let schemasChecked = 0; +let examplesChecked = 0; let artifactDigest; const schemaValidator = new Ajv2020({ strict: false, validateFormats: false }); +const booleanLiteralPath = (schema, path = "$", seen = new Set()) => { + if (!schema || typeof schema !== "object" || seen.has(schema)) + return undefined; + seen.add(schema); + for (const [key, child] of Object.entries(schema)) { + const childPath = `${path}.${key}`; + if (key === "const" && typeof child === "boolean") return childPath; + if ( + key === "enum" && + Array.isArray(child) && + child.some((item) => typeof item === "boolean") + ) + return childPath; + const schemaChildrenAtKey = + schemaMaps.has(key) && + child && + typeof child === "object" && + !Array.isArray(child) + ? Object.entries(child).map(([name, item]) => [ + `${childPath}.${name}`, + item, + ]) + : schemaArrays.has(key) && Array.isArray(child) + ? child.map((item, index) => [`${childPath}[${index}]`, item]) + : schemaChildren.has(key) + ? [[childPath, child]] + : []; + for (const [nestedPath, nested] of schemaChildrenAtKey) { + const found = booleanLiteralPath(nested, nestedPath, seen); + if (found !== undefined) return found; + } + } + return undefined; +}; const extractEvidence = (value) => { if (typeof value === "string") { if ( @@ -226,6 +269,22 @@ const fixture = await createGeminiModelFixture({ schemaValidator.validateSchema(tool.parametersJsonSchema), `${tool.name}: invalid advertised JSON Schema: ${JSON.stringify(schemaValidator.errors)}`, ); + const booleanLiteral = booleanLiteralPath(tool.parametersJsonSchema); + assert.equal( + booleanLiteral, + undefined, + `${tool.name}: Gemini SDK declaration retains a boolean const/enum at ${booleanLiteral}`, + ); + const name = tool.name.slice(prefix.length); + const contract = toolContract(name); + const validate = schemaValidator.compile(tool.parametersJsonSchema); + for (const example of contract.examples) { + assert( + validate(example.input), + `${tool.name}: Gemini SDK declaration rejects canonical example ${example.title}: ${JSON.stringify(schemaValidator.errors)}`, + ); + examplesChecked++; + } schemasChecked++; } } @@ -356,6 +415,9 @@ try { client: "Gemini CLI", clientVersion, mode, + schemaProfile, + schemasChecked, + examplesChecked, passed: failure === undefined, artifacts: lab, runtimeRoot, diff --git a/src/contracts/inputSchemaPresentation.ts b/src/contracts/inputSchemaPresentation.ts index 1a9754ace7..165de883e3 100644 --- a/src/contracts/inputSchemaPresentation.ts +++ b/src/contracts/inputSchemaPresentation.ts @@ -146,6 +146,195 @@ const inputGroup = (branch: Readonly>): string => { return [...required, ...optional].join(" + "); }; +type SchemaNormalization = { + readonly schema: unknown; + readonly changed: boolean; +}; + +const isBooleanLiteralSchema = (schema: unknown): boolean => + isObject(schema) && + (typeof schema.const === "boolean" || + (Array.isArray(schema.enum) && + schema.enum.length > 0 && + schema.enum.every((value) => typeof value === "boolean"))); + +const isBooleanSchema = (schema: unknown): boolean => + isObject(schema) && + (schema.type === "boolean" || isBooleanLiteralSchema(schema)); + +const preserveSharedDefault = ( + schema: Record, + branches: readonly unknown[], +): void => { + if (schema.default !== undefined) return; + const defaults = branches + .filter(isObject) + .map((branch) => branch.default) + .filter((value) => value !== undefined); + if ( + defaults.length > 0 && + defaults.every((value) => isDeepStrictEqual(value, defaults[0])) + ) + schema.default = defaults[0]; +}; + +const normalizeAdvertisedSchema = (value: unknown): SchemaNormalization => { + if (!isObject(value)) return { schema: value, changed: false }; + + const node: Record = { ...value }; + let changed = false; + const normalizeMap = (map: Readonly>) => + Object.fromEntries( + Object.entries(map).map(([name, child]) => { + const result = normalizeAdvertisedSchema(child); + changed ||= result.changed; + return [name, result.schema]; + }), + ); + const normalizeList = (list: readonly unknown[]) => + list.map((child) => { + const result = normalizeAdvertisedSchema(child); + changed ||= result.changed; + return result; + }); + + for (const [key, child] of Object.entries(value)) { + if (schemaMaps.has(key) && isObject(child)) { + node[key] = normalizeMap(child); + } else if ( + schemaArrays.has(key) && + key !== "anyOf" && + key !== "oneOf" && + Array.isArray(child) + ) { + node[key] = normalizeList(child).map((result) => result.schema); + } else if ( + schemaChildren.has(key) && + key !== "not" && + key !== "if" && + key !== "then" && + key !== "else" + ) { + const results = Array.isArray(child) + ? normalizeList(child) + : [normalizeAdvertisedSchema(child)]; + const nestedChanged = results.some((result) => result.changed); + if ( + key === "contains" && + nestedChanged && + value.maxContains !== undefined + ) { + delete node.contains; + delete node.minContains; + delete node.maxContains; + changed = true; + } else { + node[key] = Array.isArray(child) + ? results.map((result) => result.schema) + : results[0]?.schema; + changed ||= nestedChanged; + } + } + } + + if (typeof node.const === "boolean") { + delete node.const; + if (node.type === undefined) node.type = "boolean"; + changed = true; + } + if ( + Array.isArray(node.enum) && + node.enum.length > 0 && + node.enum.every((item) => typeof item === "boolean") + ) { + delete node.enum; + if (node.type === undefined) node.type = "boolean"; + changed = true; + } + + const negated = Object.hasOwn(value, "not") + ? normalizeAdvertisedSchema(value.not) + : undefined; + if (negated?.changed) { + // Widening a predicate under `not` narrows the containing schema. + changed = true; + delete node.not; + } else if (negated !== undefined) { + node.not = negated.schema; + } + + const conditional = Object.hasOwn(value, "if") + ? normalizeAdvertisedSchema(value.if) + : undefined; + const thenBranch = Object.hasOwn(value, "then") + ? normalizeAdvertisedSchema(value.then) + : undefined; + const elseBranch = Object.hasOwn(value, "else") + ? normalizeAdvertisedSchema(value.else) + : undefined; + if (conditional?.changed) { + // Changing the predicate can route canonical-valid inputs into a rejecting branch. + changed = true; + delete node.if; + delete node.then; + delete node.else; + } else { + if (conditional !== undefined) node.if = conditional.schema; + if (thenBranch !== undefined) { + node.then = thenBranch.schema; + changed ||= thenBranch.changed; + } + if (elseBranch !== undefined) { + node.else = elseBranch.schema; + changed ||= elseBranch.changed; + } + } + + for (const combinator of ["anyOf", "oneOf"] as const) { + const branches = value[combinator]; + if (!Array.isArray(branches)) continue; + const normalized = normalizeList(branches); + const schemas = normalized.map((result) => result.schema); + const branchesChanged = normalized.some((result) => result.changed); + if (combinator === "oneOf" && branchesChanged) { + const booleanLiteralsOnly = branches.every(isBooleanLiteralSchema); + if (booleanLiteralsOnly) { + node.type = "boolean"; + preserveSharedDefault(node, schemas); + delete node.oneOf; + changed = true; + continue; + } + // `anyOf` retains branch guidance without allowing widened branches to + // reject inputs that were accepted by exactly one canonical branch. + delete node.oneOf; + if (Array.isArray(node.anyOf)) { + const existingAllOf = Array.isArray(node.allOf) ? node.allOf : []; + node.allOf = [...existingAllOf, { anyOf: schemas }]; + } else { + node.anyOf = schemas; + } + changed = true; + continue; + } + if ( + branchesChanged && + schemas.length > 0 && + schemas.every(isBooleanSchema) + ) { + node.type = "boolean"; + preserveSharedDefault(node, schemas); + delete node[combinator]; + changed = true; + continue; + } + node[combinator] = schemas; + changed ||= branchesChanged; + } + + return { schema: node, changed }; +}; + // 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 @@ -215,7 +404,11 @@ export const presentInputJsonSchema = ( ): Record => { const flattened = flattenRootObjectUnion(root); const described = describeSchema(flattened, flattened, describeProperty); - if (!isObject(described)) return flattened; + if (!isObject(described)) + return normalizeAdvertisedSchema(flattened).schema as Record< + string, + unknown + >; const { dependencies: _dependencies, dependentRequired: _dependentRequired, @@ -237,12 +430,19 @@ export const presentInputJsonSchema = ( ] : [], ); - if (dependencyGuidance.length === 0) return presentation; + if (dependencyGuidance.length === 0) + return normalizeAdvertisedSchema(presentation).schema as Record< + string, + unknown + >; const description = [ typeof presentation.description === "string" ? presentation.description : undefined, ...dependencyGuidance, ].filter((text) => text !== undefined); - return { ...presentation, description: description.join(" ") }; + return normalizeAdvertisedSchema({ + ...presentation, + description: description.join(" "), + }).schema as Record; }; diff --git a/src/contracts/toolInputReferences.test.ts b/src/contracts/toolInputReferences.test.ts index 80775dad34..41cd19fdf4 100644 --- a/src/contracts/toolInputReferences.test.ts +++ b/src/contracts/toolInputReferences.test.ts @@ -379,3 +379,193 @@ describe("root input presentation", () => { }); }); }); + +describe("advertised Boolean literals", () => { + it("keeps canonical boolean-literal inputs accepted after advertisement", () => { + const canonical = { + type: "object", + properties: { + enabled: { type: "boolean", const: false }, + }, + required: ["enabled"], + additionalProperties: false, + }; + const advertised = presentInputJsonSchema(canonical, () => undefined); + const canonicalValidate = new Ajv2020({ strict: false }).compile(canonical); + const advertisedValidate = new Ajv2020({ strict: false }).compile( + advertised, + ); + + expect(canonicalValidate({ enabled: false })).toBe(true); + expect(canonicalValidate({ enabled: true })).toBe(false); + expect(advertisedValidate({ enabled: false })).toBe(true); + expect(advertisedValidate({ enabled: true })).toBe(true); + expect(advertised.properties).toEqual({ enabled: { type: "boolean" } }); + }); + + it("drops affected negation and conditional groups without rejecting canonical inputs", () => { + const negated = { + type: "object", + properties: { + flag: { type: "boolean", not: { const: false } }, + }, + }; + const negatedAdvertisement = presentInputJsonSchema( + negated, + () => undefined, + ); + const canonicalNegated = new Ajv2020({ strict: false }).compile(negated); + const advertisedNegated = new Ajv2020({ strict: false }).compile( + negatedAdvertisement, + ); + expect(canonicalNegated({ flag: true })).toBe(true); + expect(canonicalNegated({ flag: false })).toBe(false); + expect(advertisedNegated({ flag: true })).toBe(true); + + const conditional = { + type: "object", + properties: { + choice: { + if: { const: false }, + then: { type: "string" }, + else: { type: "boolean" }, + }, + }, + }; + const conditionalAdvertisement = presentInputJsonSchema( + conditional, + () => undefined, + ); + const canonicalConditional = new Ajv2020({ strict: false }).compile( + conditional, + ); + const advertisedConditional = new Ajv2020({ strict: false }).compile( + conditionalAdvertisement, + ); + expect(canonicalConditional({ choice: true })).toBe(true); + expect(advertisedConditional({ choice: true })).toBe(true); + expect(conditionalAdvertisement.properties).toEqual({ + choice: {}, + }); + }); +}); + +describe("advertised Boolean schema composition", () => { + it("widens an affected oneOf to anyOf when branches could overlap", () => { + const canonical = { + type: "object", + properties: { + flag: { + oneOf: [{ type: "boolean", const: false }, {}], + }, + }, + }; + const advertised = presentInputJsonSchema(canonical, () => undefined); + const canonicalValidate = new Ajv2020({ strict: false }).compile(canonical); + const advertisedValidate = new Ajv2020({ strict: false }).compile( + advertised, + ); + expect(canonicalValidate({ flag: true })).toBe(true); + expect(canonicalValidate({ flag: false })).toBe(false); + expect(advertisedValidate({ flag: true })).toBe(true); + expect(advertised).toHaveProperty(["properties", "flag", "anyOf"]); + }); + + it("retains nested branch guidance when widening oneOf to anyOf", () => { + const canonical = { + type: "object", + properties: { + selection: { + oneOf: [ + { + type: "object", + properties: { + source: { type: "string" }, + mode: { type: "boolean", const: false }, + }, + required: ["source", "mode"], + additionalProperties: false, + }, + { + type: "object", + properties: { + target: { type: "string" }, + mode: { type: "boolean", const: true }, + }, + required: ["target", "mode"], + additionalProperties: false, + }, + ], + }, + }, + }; + const advertised = presentInputJsonSchema(canonical, () => undefined); + const canonicalValidate = new Ajv2020({ strict: false }).compile(canonical); + const advertisedValidate = new Ajv2020({ strict: false }).compile( + advertised, + ); + const example = { selection: { source: "capture.json", mode: false } }; + expect(canonicalValidate(example)).toBe(true); + expect(advertisedValidate(example)).toBe(true); + expect(advertised).toMatchObject({ + properties: { + selection: { + anyOf: [ + { properties: { source: { type: "string" } } }, + { properties: { target: { type: "string" } } }, + ], + }, + }, + }); + }); + + it("removes a nested not when widening a boolean item predicate", () => { + const canonical = { + type: "object", + properties: { + values: { + type: "array", + not: { items: { type: "boolean", const: false } }, + }, + }, + }; + const advertised = presentInputJsonSchema(canonical, () => undefined); + const canonicalValidate = new Ajv2020({ strict: false }).compile(canonical); + const advertisedValidate = new Ajv2020({ strict: false }).compile( + advertised, + ); + expect(canonicalValidate({ values: [true] })).toBe(true); + expect(advertisedValidate({ values: [true] })).toBe(true); + expect(advertised).toHaveProperty(["properties", "values"], { + type: "array", + }); + }); + + it("normalizes schema keywords only and preserves literal payloads", () => { + const literal = { + const: false, + enum: [false], + properties: { nested: { const: true } }, + }; + const presented = presentInputJsonSchema( + { + type: "object", + properties: { + default: { type: "boolean", const: false }, + allowed: { enum: [false, true] }, + kind: { type: "string", const: "exact" }, + }, + default: literal, + examples: [literal], + }, + () => undefined, + ); + expect(presented.properties).toEqual({ + default: { type: "boolean" }, + allowed: { type: "boolean" }, + kind: { type: "string", const: "exact" }, + }); + expect(presented.default).toEqual(literal); + expect(presented.examples).toEqual([literal]); + }); +}); diff --git a/tests/boundary/mcp/compactInputSchemaMcp.test.ts b/tests/boundary/mcp/compactInputSchemaMcp.test.ts index ff6f18f225..51f3ae2d9c 100644 --- a/tests/boundary/mcp/compactInputSchemaMcp.test.ts +++ b/tests/boundary/mcp/compactInputSchemaMcp.test.ts @@ -21,7 +21,10 @@ import { processScenarioSchema } from "../../../src/domain/process/processScenar import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { ToolResultDelivery } from "../../../src/server/toolResult.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; -import { arrayItemSchemaError } from "../../fixtures/mcpClientSchemaConstraints.js"; +import { + arrayItemSchemaError, + booleanLiteralSchemaError, +} from "../../fixtures/mcpClientSchemaConstraints.js"; interface AdvertisedTool { readonly name: string; @@ -225,6 +228,9 @@ describe("MCP compact schema constraints", () => { ); for (const tool of tools) { expect(arrayItemSchemaError(tool.inputSchema, tool.name)).toBeUndefined(); + expect( + booleanLiteralSchemaError(tool.inputSchema, tool.name), + ).toBeUndefined(); expect( ajv.validateSchema(tool.inputSchema), `${tool.name}: ${ajv.errorsText(ajv.errors)}`, diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index 53008b94f6..b2b1553878 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -9,7 +9,10 @@ import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; import { emptyArraySchema } from "../../../src/domain/emptyArraySchema.js"; import { processScenarioSchema } from "../../../src/domain/process/processScenario.js"; import { GENERATED_MCP_TOOL_CATALOG } from "../../fixtures/mcpToolCatalog.js"; -import { arrayItemSchemaError } from "../../fixtures/mcpClientSchemaConstraints.js"; +import { + arrayItemSchemaError, + booleanLiteralSchemaError, +} from "../../fixtures/mcpClientSchemaConstraints.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; interface ToolSchemas { @@ -40,7 +43,15 @@ function schemaErrors(tools: readonly ToolSchemas[]): string[] { schema, `${tool.name}.${kind}`, ); - return clientError === undefined ? [] : [clientError]; + if (clientError !== undefined) return [clientError]; + if (kind === "inputSchema") { + const literalError = booleanLiteralSchemaError( + schema, + `${tool.name}.${kind}`, + ); + if (literalError !== undefined) return [literalError]; + } + return []; } catch (cause: unknown) { return [ `${tool.name}.${kind}: ${cause instanceof Error ? cause.message : String(cause)}`, @@ -466,6 +477,35 @@ describe("MCP JSON Schema validity", () => { expect(recursiveReferences(tools)).toEqual([]); expectKnownAuthorityHints(tools); const byName = new Map(tools.map((tool) => [tool.name, tool])); + for (const side of ["before", "after"]) + expect(byName.get("compare_web_captures")?.inputSchema).toHaveProperty( + [ + "properties", + side, + "properties", + "inspection", + "properties", + "scripts", + "properties", + "items", + "items", + "properties", + "source", + "anyOf", + ], + expect.arrayContaining([ + expect.objectContaining({ + properties: expect.objectContaining({ + reason: { type: "string" }, + }), + }), + expect.objectContaining({ + properties: expect.objectContaining({ + artifact: expect.objectContaining({ type: "object" }), + }), + }), + ]), + ); const catalogProjection = tools.map((tool) => ({ name: tool.name, title: tool.title, @@ -578,7 +618,11 @@ describe("MCP root input schemas", () => { (name === "inspect_web_page" && flag === false); const parsed = contract.inputSchema.safeParse(input); expect(parsed.success, `${name}: ${String(flag)}`).toBe(accepted); - expect(validate(input), `${name}: ${String(flag)}`).toBe(accepted); + // The advertisement relaxes Boolean literals; the canonical parser + // still rejects false when this operation requires script sources. + expect(validate(input), `${name}: ${String(flag)}`).toBe( + flag === undefined || typeof flag === "boolean", + ); if (parsed.success && flag === undefined) expect(parsed.data).toMatchObject({ include_script_sources: name === "analyze_web_bundle", @@ -831,6 +875,21 @@ it("checks client array constraints in schema positions while preserving literal ).toBe( "broken.properties.examples: tool parameters array type must have items", ); + const booleanMetadata = { + type: "object", + properties: { default: { type: "boolean", const: false } }, + default: { const: true }, + examples: [{ enum: [false] }], + }; + expect( + booleanLiteralSchemaError( + { ...booleanMetadata, properties: {} }, + "literal", + ), + ).toBeUndefined(); + expect(booleanLiteralSchemaError(booleanMetadata, "literal")).toBe( + "literal.properties.default.const is boolean", + ); }); it.each([false, true])( diff --git a/tests/fixtures/mcpClientSchemaConstraints.ts b/tests/fixtures/mcpClientSchemaConstraints.ts index 3c04c6527a..dcd2184415 100644 --- a/tests/fixtures/mcpClientSchemaConstraints.ts +++ b/tests/fixtures/mcpClientSchemaConstraints.ts @@ -36,3 +36,36 @@ export const arrayItemSchemaError = ( } return undefined; }; + +/** Reject Gemini-incompatible boolean literals in advertised input positions. */ +export const booleanLiteralSchemaError = ( + schema: unknown, + path: string, +): string | undefined => { + if (!isRecord(schema)) return undefined; + if (typeof schema.const === "boolean") return `${path}.const is boolean`; + if ( + Array.isArray(schema.enum) && + schema.enum.some((value) => typeof value === "boolean") + ) + return `${path}.enum contains a boolean`; + for (const [key, value] of Object.entries(schema)) { + const children = + schemaMaps.has(key) && isRecord(value) + ? Object.entries(value).map( + ([name, child]) => [`${key}.${name}`, child] as const, + ) + : schemaArrays.has(key) && Array.isArray(value) + ? value.map( + (child: unknown, index) => [`${key}[${index}]`, child] as const, + ) + : schemaChildren.has(key) + ? [[key, value] as const] + : []; + for (const [suffix, child] of children) { + const error = booleanLiteralSchemaError(child, `${path}.${suffix}`); + if (error !== undefined) return error; + } + } + return undefined; +};