From 10aa74dfedc3ae776f50d1fa1eb83b64ae82d3d6 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 15 Oct 2026 01:12:46 +0800 Subject: [PATCH] fix(mcp): preserve tuple constraints in advertised schemas Declare fixed tuple tails and lengths at the canonical boundary so client array validation accepts every tool. Preserve nonempty candidate constraints in managed and application comparisons. Add catalog-wide client checks and parser/schema parity coverage for both dialects and projection directions. Fixes #3576 --- docs/tool-design.md | 6 + src/contracts/tupleSchemaParity.test.ts | 164 ++++++++++++++++++ src/domain/captures/packetCapture.ts | 3 +- src/domain/fixedTupleSchema.ts | 14 ++ src/domain/hopperValues.ts | 9 +- ...riptApplicationVersionComparisonSchemas.ts | 4 +- .../managedNativeVerificationSchemas.ts | 4 +- src/domain/relationshipCoverage.ts | 10 +- tests/boundary/mcp/toolSchemaValidity.test.ts | 62 ++++++- 9 files changed, 262 insertions(+), 14 deletions(-) create mode 100644 src/contracts/tupleSchemaParity.test.ts create mode 100644 src/domain/fixedTupleSchema.ts diff --git a/docs/tool-design.md b/docs/tool-design.md index 6117ff6d03..7231301838 100644 --- a/docs/tool-design.md +++ b/docs/tool-design.md @@ -87,6 +87,12 @@ malformed, unavailable, cancellation, and partial outcomes through affected consumers. Real-engine claims require the matching provider lane; see [testing.md](testing.md). +For fixed tuples in public contracts, use `src/domain/fixedTupleSchema.ts`: +Zod's tuple projection omits implicit length bounds and Draft 2020-12 `items`, +which some clients require even alongside `prefixItems`. Tuples with a rest +schema also need explicit minimum lengths. Check parser/schema parity for empty, +short, overlong, and malformed values in both input and output projections. + When discoverability changes, evaluate representative broad, direct, and negative tasks on the intended host. Record host/model/catalog, selected tools, arguments, errors, retries, and task outcomes. Schema validity and keyword heuristics do not diff --git a/src/contracts/tupleSchemaParity.test.ts b/src/contracts/tupleSchemaParity.test.ts new file mode 100644 index 0000000000..3445804e43 --- /dev/null +++ b/src/contracts/tupleSchemaParity.test.ts @@ -0,0 +1,164 @@ +import { Ajv } from "ajv"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { describe, expect, it } from "vitest"; +import { z } from "zod"; + +import { packetFieldsSchema } from "../domain/captures/packetCapture.js"; +import { swiftStringLiteralSchema } from "../domain/hopperValues.js"; +import { applicationVersionComparisonResultSchema } from "../domain/javascript/javascriptApplicationVersionComparisonSchemas.js"; +import { pinvokeVerificationSchema } from "../domain/managed/managedNativeVerificationSchemas.js"; +import { relationshipCoverageSchema } from "../domain/relationshipCoverage.js"; + +const directions = [ + [], + ["callers"], + ["callees"], + ["callers", "callees"], + ["callees", "callers"], + ["callers", "callers"], + ["callees", "callees"], + ["callers", "callees", "callers"], + ["incoming"], + [null], + null, +]; +const literal = { + representation: "small", + source_address: "0x1000", + address: null, + value: "hello", + registers: ["x0", "x1"], + words: ["0x6f6c6c6568", "0xe500000000000000"], +}; +const ambiguousItem = { + item_id: `javc_item_${"a".repeat(64)}`, + node_kind: "javascript-module", + status: "unknown", + left_node_id: `jag_node_${"b".repeat(64)}`, + right_node_id: null, + match: { + status: "ambiguous", + basis: "none", + confidence: "unknown", + candidate_left_node_ids: [], + candidate_right_node_ids: [`jag_node_${"c".repeat(64)}`], + }, + dimensions: [], + evidence_links: [`ev_${"d".repeat(64)}`, `ev_${"e".repeat(64)}`], + limitations: [], +}; +const verifiedCandidates = pinvokeVerificationSchema.options.find( + (option) => option.shape.status.value === "verified", +)?.shape.candidates; +if (verifiedCandidates === undefined) + throw new Error("Missing verified native candidate contract"); +const nativeSymbol = { + evidence_id: `ev_${"f".repeat(64)}`, + operation: "analyze_function", + name: "NativeEntry", + address: "0x1000", + module_name: null, + module_path: null, + source: "function-dossier", +}; + +const cases: readonly { + readonly name: string; + readonly schema: z.ZodType; + readonly samples: readonly { + readonly value: unknown; + readonly valid: boolean; + }[]; +}[] = [ + ...(["partial", "unknown"] as const).map((status) => ({ + name: `${status} relationship directions`, + schema: relationshipCoverageSchema, + samples: directions.map((limited_directions) => ({ + value: { + status, + ...(status === "partial" + ? { limitations: ["Read limit reached."] } + : { reason: "Coverage unavailable." }), + limited_directions, + }, + valid: + Array.isArray(limited_directions) && + ['["callers"]', '["callees"]', '["callers","callees"]'].includes( + JSON.stringify(limited_directions), + ), + })), + })), + { + name: "nonempty managed native candidates", + schema: verifiedCandidates, + samples: [ + { value: [nativeSymbol], valid: true }, + { value: [nativeSymbol, nativeSymbol], valid: true }, + { value: [], valid: false }, + { value: [null], valid: false }, + ], + }, + { + name: "Swift register and word pairs", + schema: swiftStringLiteralSchema, + samples: [ + { value: literal, valid: true }, + ...["registers", "words"].flatMap((field) => + [[], ["x0"], ["x0", "x1", "x2"], ["x0", null], null].map((value) => ({ + value: { ...literal, [field]: value }, + valid: false, + })), + ), + ], + }, + { + name: "packet frame numbers", + schema: packetFieldsSchema, + samples: [ + { value: { "frame.number": ["1"], "ip.addr": ["a", "b"] }, valid: true }, + ...[[], ["1", "2"], ["0"], [1], null].map((value) => ({ + value: { "frame.number": value }, + valid: false, + })), + ], + }, + { + name: "nonempty ambiguous application candidates", + schema: applicationVersionComparisonResultSchema.shape.items, + samples: [ + { value: [ambiguousItem], valid: true }, + { + value: [ + { + ...ambiguousItem, + match: { ...ambiguousItem.match, candidate_right_node_ids: [] }, + }, + ], + valid: false, + }, + ], + }, +]; + +describe.each(["draft-2020-12", "draft-07"] as const)( + "tuple JSON Schema parity in %s", + (target) => { + describe.each(["input", "output"] as const)("%s projection", (io) => { + it.each(cases)("preserves $name", ({ schema, samples }) => { + const projected = z.toJSONSchema(schema, { io, target }); + const ajv = + target === "draft-2020-12" + ? new Ajv2020({ strict: false, validateFormats: false }) + : new Ajv({ strict: false, validateFormats: false }); + expect(ajv.validateSchema(projected)).toBe(true); + const validate = ajv.compile(projected); + for (const { value, valid } of samples) { + expect(schema.safeParse(value).success, JSON.stringify(value)).toBe( + valid, + ); + expect(validate(value), JSON.stringify(value)).toBe(valid); + } + }); + }); + }, +); diff --git a/src/domain/captures/packetCapture.ts b/src/domain/captures/packetCapture.ts index deb439cadf..3f7c0af792 100644 --- a/src/domain/captures/packetCapture.ts +++ b/src/domain/captures/packetCapture.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { fixedTupleSchema } from "../fixedTupleSchema.js"; import { localPathStringSchema } from "../localPath.js"; const argument = z @@ -95,7 +96,7 @@ export type InspectPacketCaptureInput = z.infer< /** Values remain strings; repeated occurrences remain ordered arrays. */ export const packetFieldsSchema = z .object({ - "frame.number": z.tuple([z.string().regex(/^[1-9][0-9]*$/)]), + "frame.number": fixedTupleSchema([z.string().regex(/^[1-9][0-9]*$/)]), }) .catchall(z.array(z.string())); export const packetCaptureSchema = z diff --git a/src/domain/fixedTupleSchema.ts b/src/domain/fixedTupleSchema.ts new file mode 100644 index 0000000000..656c1017cb --- /dev/null +++ b/src/domain/fixedTupleSchema.ts @@ -0,0 +1,14 @@ +import { z } from "zod"; + +/** Fixed tuples with explicit item and length constraints in JSON Schema. */ +export const fixedTupleSchema = < + const Items extends readonly [z.ZodType, ...z.ZodType[]], +>( + items: Items, +): z.ZodTuple => + // Zod's tuple projection omits implicit arity and, without a rest schema, + // `items` in Draft 2020-12. Spell out both at the canonical boundary so + // consumers agree with the parser and clients accept the declaration. + z + .tuple(items, z.never()) + .check(z.minLength(items.length), z.maxLength(items.length)); diff --git a/src/domain/hopperValues.ts b/src/domain/hopperValues.ts index e346ac59f4..0f6c42ef46 100644 --- a/src/domain/hopperValues.ts +++ b/src/domain/hopperValues.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { fixedTupleSchema } from "./fixedTupleSchema.js"; import { addressCoordinate } from "./native/addressCoordinate.js"; import type { JsonValue } from "./jsonValue.js"; @@ -286,10 +287,10 @@ const swiftStringLiteralFields = { value: z .string() .describe("Decoded UTF-8 text, including the canonical empty string."), - registers: z.tuple([z.string(), z.string()]), - words: z - .tuple([z.string(), z.string()]) - .describe("Observed constant register values, in register order."), + registers: fixedTupleSchema([z.string(), z.string()]), + words: fixedTupleSchema([z.string(), z.string()]).describe( + "Observed constant register values, in register order.", + ), }; /** A Swift String literal decoded from the two words of its ABI value, not a data reference. */ diff --git a/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts b/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts index d9d54449ed..bd2c1a1678 100644 --- a/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts +++ b/src/domain/javascript/javascriptApplicationVersionComparisonSchemas.ts @@ -70,7 +70,9 @@ export const compareApplicationVersionsInputSchema = z }) .superRefine(validateApplicationVersionIdentities); -const candidateNodesSchema = z.tuple([nodeIdSchema]).rest(nodeIdSchema); +const candidateNodesSchema = z + .tuple([nodeIdSchema], nodeIdSchema) + .check(z.minLength(1)); const exactMatchSchema = z.strictObject({ status: z.literal("matched"), basis: z.enum([ diff --git a/src/domain/managed/managedNativeVerificationSchemas.ts b/src/domain/managed/managedNativeVerificationSchemas.ts index c83488a50d..320842a75c 100644 --- a/src/domain/managed/managedNativeVerificationSchemas.ts +++ b/src/domain/managed/managedNativeVerificationSchemas.ts @@ -48,8 +48,8 @@ const pinvokeVerificationContextShape = { }; const observedCandidatesSchema = z - .tuple([nativeSymbolSchema]) - .rest(nativeSymbolSchema); + .tuple([nativeSymbolSchema], nativeSymbolSchema) + .check(z.minLength(1)); export const pinvokeVerificationSchema = z.union([ z.strictObject({ diff --git a/src/domain/relationshipCoverage.ts b/src/domain/relationshipCoverage.ts index f241206a07..ec69dc5cc8 100644 --- a/src/domain/relationshipCoverage.ts +++ b/src/domain/relationshipCoverage.ts @@ -1,16 +1,16 @@ import { z } from "zod"; +import { fixedTupleSchema } from "./fixedTupleSchema.js"; + const diagnostics = z.array(z.string().min(1)).min(1).exactOptional(); const completeCoverage = z.strictObject({ status: z.literal("complete"), diagnostics, }); const limitedDirections = z.union([ - z.tuple([z.literal("callers")]).check(z.minLength(1), z.maxLength(1)), - z.tuple([z.literal("callees")]).check(z.minLength(1), z.maxLength(1)), - z - .tuple([z.literal("callers"), z.literal("callees")]) - .check(z.minLength(2), z.maxLength(2)), + fixedTupleSchema([z.literal("callers")]), + fixedTupleSchema([z.literal("callees")]), + fixedTupleSchema([z.literal("callers"), z.literal("callees")]), ]); const partialCoverage = z.strictObject({ status: z.literal("partial"), diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index cc4293c3b9..ad038e17a3 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -5,6 +5,11 @@ import { describe, expect, it } from "vitest"; import { z } from "zod"; import { advertisedInputExamples } from "../../../src/contracts/advertisedInputExamples.js"; +import { + schemaArrays, + schemaChildren, + schemaMaps, +} from "../../../src/contracts/inputSchemaPresentation.js"; import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; import { emptyArraySchema } from "../../../src/domain/emptyArraySchema.js"; import { processScenarioSchema } from "../../../src/domain/process/processScenario.js"; @@ -18,6 +23,37 @@ interface ToolSchemas { readonly annotations?: Record | undefined; } +function arrayItemSchemaError( + schema: unknown, + path: string, +): string | undefined { + if (!isRecord(schema)) return undefined; + // VS Code Copilot's toolSchemaNormalizer requires truthy `items`, even + // for Draft 2020-12 tuples with prefixItems (morluto/rea#3576). + if (schema.type === "array" && !schema.items) + return `${path}: tool parameters array type must have items`; + 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) || schemaChildren.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 = arrayItemSchemaError(child, `${path}.${suffix}`); + if (error !== undefined) return error; + } + } + return undefined; +} + function schemaErrors(tools: readonly ToolSchemas[]): string[] { // Reuse Ajv's meta-schema validators, then clear each root's reference // namespace, including nested IDs, before validating the next schema. @@ -35,7 +71,11 @@ function schemaErrors(tools: readonly ToolSchemas[]): string[] { if (!ajv.validateSchema(schema)) return [`${tool.name}.${kind}: ${ajv.errorsText(ajv.errors)}`]; ajv.compile(schema); - return []; + const clientError = arrayItemSchemaError( + schema, + `${tool.name}.${kind}`, + ); + return clientError === undefined ? [] : [clientError]; } catch (cause: unknown) { return [ `${tool.name}.${kind}: ${cause instanceof Error ? cause.message : String(cause)}`, @@ -766,6 +806,26 @@ it("validates each tool independently when output schema IDs repeat", () => { expect(errors[0]).toContain("missing"); }); +it("checks client array constraints in schema positions while preserving literal metadata", () => { + const schema = { + type: "object", + properties: { examples: { type: "array", items: { type: "string" } } }, + examples: [{ type: "array" }], + default: { type: "array" }, + const: { type: "array" }, + enum: [{ type: "array" }], + }; + expect(arrayItemSchemaError(schema, "literal")).toBeUndefined(); + expect( + arrayItemSchemaError( + { ...schema, properties: { examples: { type: "array" } } }, + "broken", + ), + ).toBe( + "broken.properties.examples: tool parameters array type must have items", + ); +}); + it.each([false, true])( "rejects references to another tool's nested IDs (earlier compilation fails: %s)", (invalidPrevious) => {