From 88e378063da73df6347d899bb746c3a204c0416c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 02:56:52 +0800 Subject: [PATCH] fix(contracts): preserve safe defaults and client literal guidance --- docs/mcp-contracts.md | 15 ++- scripts/verify-gemini-client.mjs | 17 ++-- src/contracts/evm/evmToolContracts.ts | 2 +- src/contracts/inputSchemaPresentation.ts | 49 ++++++++++ src/contracts/managed/managedToolContracts.ts | 2 +- src/contracts/toolInputReferences.test.ts | 92 ++++++++++++++++++- src/contracts/wasm/wasmToolContracts.ts | 2 +- .../mcp/browserComparisonInput.test.ts | 20 +++- .../mcp/compactInputSchemaMcp.test.ts | 4 +- tests/boundary/mcp/evmInterfaceMcp.test.ts | 36 ++++++++ tests/boundary/mcp/toolSchemaValidity.test.ts | 24 ++++- tests/fixtures/mcpClientSchemaConstraints.ts | 12 ++- 12 files changed, 245 insertions(+), 30 deletions(-) diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index d128721151..24463a74ba 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -1118,7 +1118,9 @@ advertised. Every advertised example is a distinct valid canonical input. 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 -describes the accepted input groups. Which fields belong together is guidance +describes the accepted input groups. A property default is advertised only when +every input group accepts that property with the same default, so default-filling +clients cannot inject a scenario-only property into a passive comparison. Which fields belong together is guidance in that description rather than a `dependentRequired` or `dependencies` keyword, because Gemini function declarations reject both and one unknown keyword fails the client's whole tool catalog. Canonical runtime validation @@ -1135,9 +1137,11 @@ masks without a repeating regular expression or temporary byte arrays. The server rejects malformed or unconstrained patterns before provider dispatch; the advertised strings alone do not express those rules. -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 +Boolean and empty-string literal constraints in input schemas are relaxed only +in this advertisement. Boolean `const` becomes `type: boolean`; empty-string +`const` or an enum containing an empty string is widened without dropping the +accepted empty value. Defaults and descriptions remain intact. +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. @@ -1289,7 +1293,8 @@ and the identity dimension unknown. the complete `normalized_result` from `discover_webmcp_tools`, or null. - Scenario: `before_scenario` and `after_scenario` contain complete `normalized_result` objects from `capture_browser_scenario`. Optional - `normalization` defaults to `{ "rules": [] }`. + `normalization` defaults to `{ "rules": [] }` in the canonical scenario parser; + this scenario-only default is omitted from the merged advertised input. The advertised schema includes complete nested capture fields and rejects incomplete pairs and structurally malformed captures. Mixed comparison families, diff --git a/scripts/verify-gemini-client.mjs b/scripts/verify-gemini-client.mjs index 97b0995450..ad4d3c00ff 100644 --- a/scripts/verify-gemini-client.mjs +++ b/scripts/verify-gemini-client.mjs @@ -173,17 +173,18 @@ let schemasChecked = 0; let examplesChecked = 0; let artifactDigest; const schemaValidator = new Ajv2020({ strict: false, validateFormats: false }); -const booleanLiteralPath = (schema, path = "$", seen = new Set()) => { +const unsupportedLiteralPath = (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 === "const" && (typeof child === "boolean" || child === "")) + return childPath; if ( key === "enum" && Array.isArray(child) && - child.some((item) => typeof item === "boolean") + child.some((item) => typeof item === "boolean" || item === "") ) return childPath; const schemaChildrenAtKey = @@ -201,7 +202,7 @@ const booleanLiteralPath = (schema, path = "$", seen = new Set()) => { ? [[childPath, child]] : []; for (const [nestedPath, nested] of schemaChildrenAtKey) { - const found = booleanLiteralPath(nested, nestedPath, seen); + const found = unsupportedLiteralPath(nested, nestedPath, seen); if (found !== undefined) return found; } } @@ -270,11 +271,13 @@ const fixture = await createGeminiModelFixture({ schemaValidator.validateSchema(tool.parametersJsonSchema), `${tool.name}: invalid advertised JSON Schema: ${JSON.stringify(schemaValidator.errors)}`, ); - const booleanLiteral = booleanLiteralPath(tool.parametersJsonSchema); + const unsupportedLiteral = unsupportedLiteralPath( + tool.parametersJsonSchema, + ); assert.equal( - booleanLiteral, + unsupportedLiteral, undefined, - `${tool.name}: Gemini SDK declaration retains a boolean const/enum at ${booleanLiteral}`, + `${tool.name}: Gemini SDK declaration retains a boolean or empty-string const/enum at ${unsupportedLiteral}`, ); const name = tool.name.slice(prefix.length); const contract = toolContract(name); diff --git a/src/contracts/evm/evmToolContracts.ts b/src/contracts/evm/evmToolContracts.ts index 27be558186..824811b73a 100644 --- a/src/contracts/evm/evmToolContracts.ts +++ b/src/contracts/evm/evmToolContracts.ts @@ -13,7 +13,7 @@ import { evmInterfaceSummaryViewResultSchema } from "../../domain/analysisView/a /** Delivery selection leaves the existing provider input and original Evidence unchanged. */ const evmInterfaceInspectionInputSchema = inspectEvmInterfaceInputSchema.extend( { - detail: analysisDetailSchema.unwrap().optional(), + detail: analysisDetailSchema, }, ); diff --git a/src/contracts/inputSchemaPresentation.ts b/src/contracts/inputSchemaPresentation.ts index f6f66b4b61..dc40eefe02 100644 --- a/src/contracts/inputSchemaPresentation.ts +++ b/src/contracts/inputSchemaPresentation.ts @@ -246,6 +246,11 @@ const normalizeAdvertisedSchema = (value: unknown): SchemaNormalization => { if (node.type === undefined) node.type = "boolean"; changed = true; } + if (node.const === "") { + delete node.const; + if (node.type === undefined) node.type = "string"; + changed = true; + } if ( Array.isArray(node.enum) && node.enum.length > 0 && @@ -255,6 +260,18 @@ const normalizeAdvertisedSchema = (value: unknown): SchemaNormalization => { if (node.type === undefined) node.type = "boolean"; changed = true; } + if (Array.isArray(node.enum) && node.enum.includes("")) { + // Removing only the empty member would reject a canonical-valid input. + // Gemini transports turn const into enum and refuse empty enum members. + const values = node.enum; + delete node.enum; + if ( + node.type === undefined && + values.every((item) => typeof item === "string") + ) + node.type = "string"; + changed = true; + } const negated = Object.hasOwn(value, "not") ? normalizeAdvertisedSchema(value.not) @@ -380,6 +397,38 @@ const flattenRootObjectUnion = ( variants.set(name, known); } const required = sharedRequiredProperties(branches); + // A default lifted out of an input group can make a client send a field + // rejected by another strict group. Keep it only when every group owns the + // property and supplies the same default. + for (const [name, schemas] of variants) { + const defaults = branches.map((branch) => { + const property = isObject(branch.properties) + ? branch.properties[name] + : undefined; + return isObject(property) ? property.default : undefined; + }); + if ( + defaults.every( + (value) => value !== undefined && isDeepStrictEqual(value, defaults[0]), + ) + ) + continue; + const withoutDefaults: unknown[] = []; + for (const schema of schemas) { + let withoutDefault = schema; + if (isObject(schema) && Object.hasOwn(schema, "default")) { + const { default: _default, ...rest } = schema; + withoutDefault = rest; + } + if ( + !withoutDefaults.some((known) => + isDeepStrictEqual(known, withoutDefault), + ) + ) + withoutDefaults.push(withoutDefault); + } + variants.set(name, withoutDefaults); + } // A coarse property minimum rejects underspecified objects. Partner fields // and exact group membership remain enforced by the canonical runtime parser. const fewestRequired = Math.min( diff --git a/src/contracts/managed/managedToolContracts.ts b/src/contracts/managed/managedToolContracts.ts index 5fb4f70c70..627f2db50e 100644 --- a/src/contracts/managed/managedToolContracts.ts +++ b/src/contracts/managed/managedToolContracts.ts @@ -18,7 +18,7 @@ export const managedTargetInputSchema = z.strictObject({ /** Delivery selection belongs to artifact and member inspection, not native queries. */ export const managedArtifactInputSchema = managedTargetInputSchema.extend({ - detail: analysisDetailSchema.unwrap().optional(), + detail: analysisDetailSchema, }); /** Member inspection may select declaring types before members are decoded. */ diff --git a/src/contracts/toolInputReferences.test.ts b/src/contracts/toolInputReferences.test.ts index cb1f0765a1..c4c40b9185 100644 --- a/src/contracts/toolInputReferences.test.ts +++ b/src/contracts/toolInputReferences.test.ts @@ -419,7 +419,97 @@ describe("root input presentation", () => { }); }); -describe("advertised Boolean literals", () => { +describe("advertised client-compatible literals", () => { + it.each([{ const: "" }, { enum: ["", "named"] }])( + "widens unsupported empty-string literals without changing canonical validation: %j", + (literal) => { + const canonical = { + type: "object", + properties: { + value: { + type: "string", + ...literal, + default: "", + description: "The literal value.", + }, + }, + }; + const advertised = presentInputJsonSchema(canonical, () => undefined); + expect( + new Ajv2020({ strict: false }).compile(canonical)({ value: "" }), + ).toBe(true); + expect( + new Ajv2020({ strict: false }).compile(advertised)({ value: "" }), + ).toBe(true); + expect(advertised).toHaveProperty(["properties", "value"], { + type: "string", + default: "", + description: "The literal value.", + }); + }, + ); + + it("widens overlapping empty-string oneOf branches while preserving nonempty alternatives", () => { + const canonical = { + type: "object", + properties: { + value: { + oneOf: [ + { type: "string", const: "" }, + { type: "string", minLength: 1 }, + ], + }, + }, + }; + const advertised = presentInputJsonSchema(canonical, () => undefined); + const original = new Ajv2020({ strict: false }).compile(canonical); + const presented = new Ajv2020({ strict: false }).compile(advertised); + for (const value of ["", "named"]) { + expect(original({ value })).toBe(true); + expect(presented({ value })).toBe(true); + } + expect(advertised).toHaveProperty(["properties", "value", "anyOf"]); + expect(advertised).not.toHaveProperty(["properties", "value", "oneOf"]); + }); + + it("keeps only defaults shared by every root input group", () => { + const schema = z.union([ + z.strictObject({ + left: z.string(), + shared: z.boolean().default(false), + only_left: z.string().default("left"), + selection: z.string().default("left"), + }), + z.strictObject({ + right: z.string(), + shared: z.boolean().default(false), + selection: z.string().default("right"), + }), + ]); + const advertised = presentInputJsonSchema( + z.toJSONSchema(schema), + () => undefined, + ); + expect(advertised).toHaveProperty( + ["properties", "shared", "default"], + false, + ); + expect(advertised).not.toHaveProperty([ + "properties", + "only_left", + "default", + ]); + expect(advertised).toHaveProperty(["properties", "selection"], { + type: "string", + }); + const input = { right: "selected" }; + const validate = new Ajv2020({ strict: false, useDefaults: true }).compile( + advertised, + ); + expect(validate(input)).toBe(true); + expect(schema.safeParse(input).success).toBe(true); + expect(input).not.toHaveProperty("only_left"); + }); it("keeps canonical boolean-literal inputs accepted after advertisement", () => { const canonical = { type: "object", diff --git a/src/contracts/wasm/wasmToolContracts.ts b/src/contracts/wasm/wasmToolContracts.ts index 8743046fb1..3a62582982 100644 --- a/src/contracts/wasm/wasmToolContracts.ts +++ b/src/contracts/wasm/wasmToolContracts.ts @@ -14,7 +14,7 @@ import { wasmArtifactSummaryViewResultSchema } from "../../domain/analysisView/a /** Delivery selection does not change the provider input or original Evidence. */ export const wasmArtifactInspectionInputSchema = inspectWasmArtifactInputSchema.extend({ - detail: analysisDetailSchema.unwrap().optional(), + detail: analysisDetailSchema, }); /** Optional caller-supplied WABT capability with target-free CLI/MCP parity. */ export const INSPECT_WASM_ARTIFACT_CONTRACT = { diff --git a/tests/boundary/mcp/browserComparisonInput.test.ts b/tests/boundary/mcp/browserComparisonInput.test.ts index 265efd2096..eac705bb15 100644 --- a/tests/boundary/mcp/browserComparisonInput.test.ts +++ b/tests/boundary/mcp/browserComparisonInput.test.ts @@ -92,6 +92,14 @@ describe("browser comparison input boundary", () => { expect(ajv.validateSchema(advertised.inputSchema)).toBe(true); const validate = ajv.compile(advertised.inputSchema); const passive = await capturePassivePair(provider, browser); + const defaultFilledPassive = structuredClone(passive); + const fillDefaults = new Ajv2020({ + strict: false, + validateFormats: false, + useDefaults: true, + }).compile(advertised.inputSchema); + expect(fillDefaults(defaultFilledPassive)).toBe(true); + expect(defaultFilledPassive).not.toHaveProperty("normalization"); const scenario = contract.examples[0]?.input; if (scenario === undefined) throw new Error("Missing scenario example"); const withoutNormalization = Object.fromEntries( @@ -143,11 +151,13 @@ describe("browser comparison input boundary", () => { }, ]; for (const { input, advertisedValid, runtimeValid } of [ - ...[passive, scenario, withoutNormalization].map((input) => ({ - input, - advertisedValid: true, - runtimeValid: true, - })), + ...[passive, defaultFilledPassive, scenario, withoutNormalization].map( + (input) => ({ + input, + advertisedValid: true, + runtimeValid: true, + }), + ), ...invalid.map((input) => ({ input, advertisedValid: false, diff --git a/tests/boundary/mcp/compactInputSchemaMcp.test.ts b/tests/boundary/mcp/compactInputSchemaMcp.test.ts index 30a1221c59..4d5925158d 100644 --- a/tests/boundary/mcp/compactInputSchemaMcp.test.ts +++ b/tests/boundary/mcp/compactInputSchemaMcp.test.ts @@ -27,7 +27,7 @@ import { ToolResultDelivery } from "../../../src/server/toolResult.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; import { arrayItemSchemaError, - booleanLiteralSchemaError, + unsupportedInputLiteralSchemaError, } from "../../fixtures/mcpClientSchemaConstraints.js"; interface AdvertisedTool { @@ -284,7 +284,7 @@ it("keeps SDK-converted schemas valid and accepting canonical examples", async ( `${profile} profile: ${tool.name}`, ).toBeUndefined(); expect( - booleanLiteralSchemaError(tool.inputSchema, tool.name), + unsupportedInputLiteralSchemaError(tool.inputSchema, tool.name), ).toBeUndefined(); expect( ajv.validateSchema(tool.inputSchema), diff --git a/tests/boundary/mcp/evmInterfaceMcp.test.ts b/tests/boundary/mcp/evmInterfaceMcp.test.ts index 2bfed5c813..2869185d26 100644 --- a/tests/boundary/mcp/evmInterfaceMcp.test.ts +++ b/tests/boundary/mcp/evmInterfaceMcp.test.ts @@ -19,6 +19,8 @@ import { inspectSolidityMetadata } from "../../../src/evm/SolidityMetadata.js"; import { err, ok } from "../../../src/domain/result.js"; import { silentLogger } from "../../../src/logger.js"; import { createServer } from "../../../src/server/createServer.js"; +import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; +import { createToolResultDelivery } from "../../../src/server/toolResult.js"; import { registerEvmTools } from "../../../src/server/registerEvmTools.js"; import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; import { @@ -45,6 +47,40 @@ const decodedMetadataFixture = () => { return value; }; +it("preserves shared EVM detail guidance and defaults through full SDK discovery", async () => { + const inspect = vi.fn(async () => { + throw new Error("Discovery must not invoke the provider"); + }); + const server = new EvidenceMcpServer( + { name: "evm-detail", version: "1" }, + {}, + undefined, + createToolResultDelivery(undefined), + ); + registerEvmTools(server, new EvmInterfaceService({ inspect }), silentLogger); + const client = new Client({ name: "evm-detail", version: "1" }); + const [ct, st] = InMemoryTransport.createLinkedPair(); + onTestFinished(async () => { + await client.close(); + await server.close(); + }); + await server.connect(st); + await client.connect(ct); + const { inputSchema } = await advertisedMcpToolSchemas( + client, + "inspect_evm_interface", + ); + expect(inputSchema).toHaveProperty(["properties", "detail"], { + type: "string", + enum: ["complete", "summary"], + default: "complete", + description: expect.stringContaining( + "summary retains that complete Evidence", + ), + }); + expect(inspect).not.toHaveBeenCalled(); +}); + it("advertises exact valid schemas and records inline EVM interface evidence without a binary target", async () => { const value = decodedMetadataFixture(); const inspect = vi.fn(() => diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index b2b1553878..4afcaf4c17 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -11,7 +11,7 @@ import { processScenarioSchema } from "../../../src/domain/process/processScenar import { GENERATED_MCP_TOOL_CATALOG } from "../../fixtures/mcpToolCatalog.js"; import { arrayItemSchemaError, - booleanLiteralSchemaError, + unsupportedInputLiteralSchemaError, } from "../../fixtures/mcpClientSchemaConstraints.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; @@ -45,7 +45,7 @@ function schemaErrors(tools: readonly ToolSchemas[]): string[] { ); if (clientError !== undefined) return [clientError]; if (kind === "inputSchema") { - const literalError = booleanLiteralSchemaError( + const literalError = unsupportedInputLiteralSchemaError( schema, `${tool.name}.${kind}`, ); @@ -882,14 +882,30 @@ it("checks client array constraints in schema positions while preserving literal examples: [{ enum: [false] }], }; expect( - booleanLiteralSchemaError( + unsupportedInputLiteralSchemaError( { ...booleanMetadata, properties: {} }, "literal", ), ).toBeUndefined(); - expect(booleanLiteralSchemaError(booleanMetadata, "literal")).toBe( + expect(unsupportedInputLiteralSchemaError(booleanMetadata, "literal")).toBe( "literal.properties.default.const is boolean", ); + for (const [literal, reason] of [ + [{ const: "" }, "const is an empty string"], + [{ enum: ["named", ""] }, "enum contains an empty string"], + ] as const) { + expect( + unsupportedInputLiteralSchemaError( + { + type: "object", + properties: { value: literal }, + default: { const: "" }, + examples: [{ enum: [""] }], + }, + "literal", + ), + ).toBe(`literal.properties.value.${reason}`); + } }); it.each([false, true])( diff --git a/tests/fixtures/mcpClientSchemaConstraints.ts b/tests/fixtures/mcpClientSchemaConstraints.ts index 6a152a2b1b..c43c03d1f5 100644 --- a/tests/fixtures/mcpClientSchemaConstraints.ts +++ b/tests/fixtures/mcpClientSchemaConstraints.ts @@ -40,18 +40,21 @@ export const arrayItemSchemaError = ( return undefined; }; -/** Reject Gemini-incompatible boolean literals in advertised input positions. */ -export const booleanLiteralSchemaError = ( +/** Reject Gemini-incompatible boolean and empty-string input literals. */ +export const unsupportedInputLiteralSchemaError = ( schema: unknown, path: string, ): string | undefined => { if (!isRecord(schema)) return undefined; if (typeof schema.const === "boolean") return `${path}.const is boolean`; + if (schema.const === "") return `${path}.const is an empty string`; if ( Array.isArray(schema.enum) && schema.enum.some((value) => typeof value === "boolean") ) return `${path}.enum contains a boolean`; + if (Array.isArray(schema.enum) && schema.enum.includes("")) + return `${path}.enum contains an empty string`; for (const [key, value] of Object.entries(schema)) { const children = schemaMaps.has(key) && isRecord(value) @@ -66,7 +69,10 @@ export const booleanLiteralSchemaError = ( ? [[key, value] as const] : []; for (const [suffix, child] of children) { - const error = booleanLiteralSchemaError(child, `${path}.${suffix}`); + const error = unsupportedInputLiteralSchemaError( + child, + `${path}.${suffix}`, + ); if (error !== undefined) return error; } }