diff --git a/scripts/lib/client-schema-patterns.mts b/scripts/lib/client-schema-patterns.mts index 2ac721492a..0afed6f172 100644 --- a/scripts/lib/client-schema-patterns.mts +++ b/scripts/lib/client-schema-patterns.mts @@ -1,48 +1,22 @@ -import { JsonSchemaReferences } from "../../src/contracts/jsonSchemaReferences.js"; -import { - schemaArrays, - schemaChildren, - schemaMaps, -} from "../../src/contracts/jsonSchemaKeywords.js"; - -const isSchemaObject = ( - value: unknown, -): value is Readonly> => - value !== null && typeof value === "object" && !Array.isArray(value); +import { clientSchemaPositions } from "./client-schema-positions.mjs"; /** Check forwarded regex constraints in native plain/Unicode/Unicode-sets modes. */ export const assertClientSchemaPatterns = ( schema: unknown, ): readonly string[] => { - if (!isSchemaObject(schema)) return []; const patterns: string[] = []; - const seen = new Set(); - const checkPattern = (pattern: string): void => { + for (const position of clientSchemaPositions(schema)) { + const pattern = + position.kind === "pattern-property" + ? position.pattern + : position.kind === "keyword" && + position.keyword === "pattern" && + typeof position.value === "string" + ? position.value + : undefined; + if (pattern === undefined) continue; for (const flags of ["", "u", "v"]) new RegExp(pattern, flags); patterns.push(pattern); - }; - const visit = (value: unknown): void => { - if (!isSchemaObject(value) || seen.has(value)) return; - seen.add(value); - for (const [key, child] of Object.entries(value)) { - if (key === "pattern" && typeof child === "string") checkPattern(child); - else if (schemaMaps.has(key) && isSchemaObject(child)) { - for (const [name, nested] of Object.entries(child)) { - if (key === "patternProperties") checkPattern(name); - visit(nested); - } - } else if ( - (schemaArrays.has(key) || schemaChildren.has(key)) && - Array.isArray(child) - ) { - for (const nested of child) visit(nested); - } else if (schemaChildren.has(key)) visit(child); - } - }; - visit(schema); - // Literal instance values stay opaque unless the reference owner resolves a - // local target into them. External/dynamic/ambiguous targets remain unknown. - for (const { target } of new JsonSchemaReferences(schema).references()) - if (target.kind === "target") visit(target.value); + } return patterns; }; diff --git a/scripts/lib/client-schema-positions.mts b/scripts/lib/client-schema-positions.mts new file mode 100644 index 0000000000..59c0bada03 --- /dev/null +++ b/scripts/lib/client-schema-positions.mts @@ -0,0 +1,92 @@ +import { JsonSchemaReferences } from "../../src/contracts/jsonSchemaReferences.js"; +import { + schemaArrays, + schemaChildren, + schemaMaps, +} from "../../src/contracts/jsonSchemaKeywords.js"; + +type SchemaObject = Readonly>; +export type ClientSchemaPosition = + | { + readonly kind: "schema"; + readonly schema: SchemaObject; + readonly path: string; + } + | { + readonly kind: "keyword"; + readonly keyword: string; + readonly value: unknown; + readonly path: string; + } + | { + readonly kind: "pattern-property"; + readonly pattern: string; + readonly path: string; + }; + +const isSchemaObject = (value: unknown): value is SchemaObject => + value !== null && typeof value === "object" && !Array.isArray(value); + +const referenceTargetPath = ( + root: SchemaObject, + tokens: readonly string[], + path: string, +): string => { + let value: unknown = root; + for (const token of tokens) { + path += Array.isArray(value) ? `[${token}]` : `.${token}`; + // The reference owner already established this exact pointer's target. + value = Reflect.get(value as object, token); + } + return path; +}; + +/** Ordered schema positions for native-client checks; instance literals remain opaque. */ +export function* clientSchemaPositions( + schema: unknown, +): IterableIterator { + if (!isSchemaObject(schema)) return; + const seen = new Set(); + const paths = new WeakMap(); + function* visit( + value: unknown, + path: string, + ): IterableIterator { + if (!isSchemaObject(value) || seen.has(value)) return; + seen.add(value); + paths.set(value, path); + yield { kind: "schema", schema: value, path }; + for (const [keyword, child] of Object.entries(value)) { + const childPath = `${path}.${keyword}`; + yield { kind: "keyword", keyword, value: child, path: childPath }; + if (schemaMaps.has(keyword) && isSchemaObject(child)) { + for (const [name, nested] of Object.entries(child)) { + const nestedPath = `${childPath}.${name}`; + if (keyword === "patternProperties") + yield { kind: "pattern-property", pattern: name, path: nestedPath }; + yield* visit(nested, nestedPath); + } + } else if ( + (schemaArrays.has(keyword) || schemaChildren.has(keyword)) && + Array.isArray(child) + ) { + for (const [index, nested] of child.entries()) + yield* visit(nested, `${childPath}[${index}]`); + } else if (schemaChildren.has(keyword)) yield* visit(child, childPath); + } + } + yield* visit(schema, "$"); + // Resolve after ordinary positions so an existing inline diagnostic keeps + // its encounter order. External/dynamic/ambiguous targets stay unknown. + for (const { scope, target } of new JsonSchemaReferences( + schema, + ).references()) { + if (target.kind !== "target") continue; + const scopePath = paths.get(scope.root); + if (scopePath !== undefined) + yield* visit( + target.value, + referenceTargetPath(scope.root, target.path, scopePath), + ); + } +} diff --git a/scripts/lib/gemini-schema-literals.mts b/scripts/lib/gemini-schema-literals.mts new file mode 100644 index 0000000000..1bcf481428 --- /dev/null +++ b/scripts/lib/gemini-schema-literals.mts @@ -0,0 +1,27 @@ +import { clientSchemaPositions } from "./client-schema-positions.mjs"; + +export interface GeminiUnsupportedLiteral { + readonly path: string; + readonly keyword: "const" | "enum"; + readonly reason: "boolean" | "empty-string"; +} + +/** Gemini's declaration policy, including schemas selected by resolved local references. */ +export const geminiUnsupportedLiteral = ( + schema: unknown, +): GeminiUnsupportedLiteral | undefined => { + for (const position of clientSchemaPositions(schema)) { + if (position.kind !== "keyword") continue; + const { keyword, value, path } = position; + if (keyword === "const") { + if (typeof value === "boolean") + return { path, keyword, reason: "boolean" }; + if (value === "") return { path, keyword, reason: "empty-string" }; + } else if (keyword === "enum" && Array.isArray(value)) { + if (value.some((item) => typeof item === "boolean")) + return { path, keyword, reason: "boolean" }; + if (value.includes("")) return { path, keyword, reason: "empty-string" }; + } + } + return undefined; +}; diff --git a/scripts/verify-gemini-client.mjs b/scripts/verify-gemini-client.mjs index 8ed5d94fab..9837ad53dd 100644 --- a/scripts/verify-gemini-client.mjs +++ b/scripts/verify-gemini-client.mjs @@ -5,11 +5,6 @@ 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/jsonSchemaKeywords.js"; import { TOOL_CONTRACTS, toolContract, @@ -27,6 +22,12 @@ import { } from "./verify/clients/result-discovery.mjs"; import { createVerifierRun, completeVerifierRun } from "./lib/verifier-run.mjs"; import { createGeminiModelFixture } from "./verify/clients/gemini-model-fixture.mjs"; +import { importVerifierSource } from "./lib/verifier-source-loader.mjs"; + +const { geminiUnsupportedLiteral } = await importVerifierSource( + new URL("./lib/gemini-schema-literals.mts", import.meta.url).href, + import.meta.url, +); const repo = fileURLToPath(new URL("..", import.meta.url)); const runtimeRoot = process.env.REA_VERIFY_RUNTIME_ROOT ?? repo; @@ -200,41 +201,6 @@ await withVerifierCleanup(async () => { strict: false, validateFormats: false, }); - 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" || child === "")) - return childPath; - if ( - key === "enum" && - Array.isArray(child) && - child.some((item) => typeof item === "boolean" || item === "") - ) - 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 = unsupportedLiteralPath(nested, nestedPath, seen); - if (found !== undefined) return found; - } - } - return undefined; - }; const decodeGeminiResult = (text) => { if ( text.startsWith("\n") && @@ -277,9 +243,9 @@ await withVerifierCleanup(async () => { schemaValidator.validateSchema(tool.parametersJsonSchema), `${tool.name}: invalid advertised JSON Schema: ${JSON.stringify(schemaValidator.errors)}`, ); - const unsupportedLiteral = unsupportedLiteralPath( + const unsupportedLiteral = geminiUnsupportedLiteral( tool.parametersJsonSchema, - ); + )?.path; assert.equal( unsupportedLiteral, undefined, diff --git a/tests/conformance/verifiers/clientSchemaItemConstraints.test.ts b/tests/conformance/verifiers/clientSchemaItemConstraints.test.ts new file mode 100644 index 0000000000..75a81cd695 --- /dev/null +++ b/tests/conformance/verifiers/clientSchemaItemConstraints.test.ts @@ -0,0 +1,57 @@ +import { expect, it } from "vitest"; +import { + arrayItemSchemaError, + strictGatewayItemSchemaErrors, +} from "../../fixtures/mcpClientSchemaConstraints.js"; + +it("checks client array requirements when a reference selects literal-held schemas", () => { + const literal = { examples: [{ type: "array" }] }; + expect(arrayItemSchemaError(literal, "tool")).toBeUndefined(); + expect( + arrayItemSchemaError({ ...literal, $ref: "#/examples/0" }, "tool"), + ).toBe("tool.examples[0]: tool parameters array type must have items"); + expect( + arrayItemSchemaError( + { + $ref: "#/examples/0", + examples: [{ type: "array", items: { type: "string" } }], + }, + "tool", + ), + ).toBeUndefined(); +}); + +it.each([ + { + selected: { type: "object", items: { type: "string" } }, + error: 'tool.examples[0].items is beside type "object"', + }, + { + selected: { + type: "array", + items: { anyOf: [{ type: "string" }, { type: "number" }] }, + }, + error: "tool.examples[0].items is an untyped union", + }, +])( + "checks referenced gateway item constraints: $error", + ({ selected, error }) => { + const literal = { examples: [selected] }; + expect(strictGatewayItemSchemaErrors(literal, "tool")).toEqual([]); + expect( + strictGatewayItemSchemaErrors( + { ...literal, $ref: "#/examples/0" }, + "tool", + ), + ).toEqual([error]); + expect( + strictGatewayItemSchemaErrors( + { + $ref: "#/examples/0", + examples: [{ type: "array", items: { type: "string" } }], + }, + "tool", + ), + ).toEqual([]); + }, +); diff --git a/tests/conformance/verifiers/clientSchemaPatterns.test.ts b/tests/conformance/verifiers/clientSchemaPatterns.test.ts index caec43e797..5b8027aa86 100644 --- a/tests/conformance/verifiers/clientSchemaPatterns.test.ts +++ b/tests/conformance/verifiers/clientSchemaPatterns.test.ts @@ -95,3 +95,13 @@ it("keeps native Unicode-mode failures and repeated pattern occurrences visible" }), ).toEqual(["^same", "^same"]); }); + +it("keeps pattern encounter order while visiting a shared schema object only once", () => { + const shared = { pattern: "nested$" }; + expect( + assertClientSchemaPatterns({ + patternProperties: { "^first": shared, "^second": shared }, + $ref: "#/patternProperties/^first", + }), + ).toEqual(["^first", "nested$", "^second"]); +}); diff --git a/tests/conformance/verifiers/geminiSchemaLiterals.test.ts b/tests/conformance/verifiers/geminiSchemaLiterals.test.ts new file mode 100644 index 0000000000..35e8a47bd4 --- /dev/null +++ b/tests/conformance/verifiers/geminiSchemaLiterals.test.ts @@ -0,0 +1,122 @@ +import { expect, it } from "vitest"; +import { Ajv2020 } from "ajv/dist/2020.js"; +import { geminiUnsupportedLiteral } from "../../../scripts/lib/gemini-schema-literals.mjs"; +import { clientSchemaPositions } from "../../../scripts/lib/client-schema-positions.mjs"; +import { unsupportedInputLiteralSchemaError } from "../../fixtures/mcpClientSchemaConstraints.js"; + +it("checks an explicitly referenced example as a schema while leaving unreferenced examples opaque", () => { + const literal = { examples: [{ const: false }] }; + expect(geminiUnsupportedLiteral(literal)).toBeUndefined(); + const schema = { ...literal, $ref: "#/examples/0" }; + const ajv = new Ajv2020({ strict: false, validateFormats: false }); + expect(ajv.validateSchema(schema)).toBe(true); + const validate = ajv.compile(schema); + expect(validate(false)).toBe(true); + expect(validate(true)).toBe(false); + expect(geminiUnsupportedLiteral(schema)).toEqual({ + path: "$.examples[0].const", + keyword: "const", + reason: "boolean", + }); + expect(unsupportedInputLiteralSchemaError(schema, "tool")).toBe( + "tool.examples[0].const is boolean", + ); +}); + +it("preserves the inline schema keyword encounter order used by the native Gemini verifier", () => { + const schema = { properties: { early: { enum: [""] } }, const: false }; + expect(geminiUnsupportedLiteral(schema)).toEqual({ + path: "$.properties.early.enum", + keyword: "enum", + reason: "empty-string", + }); + expect(unsupportedInputLiteralSchemaError(schema, "tool")).toBe( + "tool.properties.early.enum contains an empty string", + ); +}); + +it("resolves nested local references against their own resource while retaining root-relative diagnostic locations", () => { + const schema = { + $id: "https://schemas.example/root.json", + $defs: { + child: { + $id: "child.json", + $ref: "#/examples/0", + examples: [{ $ref: "#/default" }], + default: { enum: ["named", ""] }, + }, + }, + }; + expect(geminiUnsupportedLiteral(schema)).toEqual({ + path: "$.$defs.child.default.enum", + keyword: "enum", + reason: "empty-string", + }); + expect(unsupportedInputLiteralSchemaError(schema, "tool")).toBe( + "tool.$defs.child.default.enum contains an empty string", + ); +}); + +it("terminates reference cycles and reports the first newly reached constraint at its source path", () => { + const schema = { + $ref: "#/examples/0", + examples: [ + { $ref: "#/examples/1" }, + { $ref: "#/examples/0", properties: { chosen: { const: "" } } }, + ], + }; + expect(geminiUnsupportedLiteral(schema)).toEqual({ + path: "$.examples[1].properties.chosen.const", + keyword: "const", + reason: "empty-string", + }); + expect( + [...clientSchemaPositions(schema)].filter( + (position) => position.kind === "schema", + ), + ).toHaveLength(4); +}); + +it("uses decoded reference tokens and preserves the existing spelling of property names in diagnostic paths", () => { + const schema = { + examples: [{ properties: { "a/b~name": { const: false } } }], + $ref: "#/examples/0/properties/a~1b~0name", + }; + expect(geminiUnsupportedLiteral(schema)?.path).toBe( + "$.examples[0].properties.a/b~name.const", + ); +}); + +it("leaves external, dynamic, anchor and ambiguous resource references unknown", () => { + for (const reference of [ + { $ref: "https://schemas.example/external.json" }, + { $ref: "#anchor" }, + { $dynamicRef: "#/examples/0" }, + ]) + expect( + geminiUnsupportedLiteral({ ...reference, examples: [{ const: false }] }), + ).toBeUndefined(); + expect( + geminiUnsupportedLiteral({ + $id: "https://schemas.example/root.json", + $defs: { + first: { + $id: "child.json", + $ref: "#/examples/0", + examples: [{ const: false }], + }, + second: { $id: "child.json", examples: [{ const: false }] }, + }, + }), + ).toBeUndefined(); +}); + +it("keeps literal-policy diagnostic prose for const and enum failures", () => { + for (const [schema, message] of [ + [{ const: false }, "tool.const is boolean"], + [{ const: "" }, "tool.const is an empty string"], + [{ enum: [false, ""] }, "tool.enum contains a boolean"], + [{ enum: [""] }, "tool.enum contains an empty string"], + ] as const) + expect(unsupportedInputLiteralSchemaError(schema, "tool")).toBe(message); +}); diff --git a/tests/fixtures/mcpClientSchemaConstraints.ts b/tests/fixtures/mcpClientSchemaConstraints.ts index 42b496c93a..9c1fb262f1 100644 --- a/tests/fixtures/mcpClientSchemaConstraints.ts +++ b/tests/fixtures/mcpClientSchemaConstraints.ts @@ -1,8 +1,5 @@ -import { - schemaArrays, - schemaChildren, - schemaMaps, -} from "../../src/contracts/jsonSchemaKeywords.js"; +import { geminiUnsupportedLiteral } from "../../scripts/lib/gemini-schema-literals.mjs"; +import { clientSchemaPositions } from "../../scripts/lib/client-schema-positions.mjs"; const isRecord = (value: unknown): value is Record => typeof value === "object" && value !== null && !Array.isArray(value); @@ -12,30 +9,14 @@ export const arrayItemSchemaError = ( schema: unknown, path: string, ): string | undefined => { - if (!isRecord(schema)) return undefined; - const admitsArrays = - schema.type === "array" || - (Array.isArray(schema.type) && schema.type.includes("array")); - if (admitsArrays && !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; - } + for (const position of clientSchemaPositions(schema)) { + if (position.kind !== "schema") continue; + const value = position.schema; + const admitsArrays = + value.type === "array" || + (Array.isArray(value.type) && value.type.includes("array")); + if (admitsArrays && !value.items) + return `${path}${position.path.slice(1)}: tool parameters array type must have items`; } return undefined; }; @@ -45,38 +26,17 @@ 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) - ? 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 = unsupportedInputLiteralSchemaError( - child, - `${path}.${suffix}`, - ); - if (error !== undefined) return error; - } - } - return undefined; + const finding = geminiUnsupportedLiteral(schema); + if (finding === undefined) return undefined; + const reason = + finding.reason === "boolean" + ? finding.keyword === "const" + ? "is boolean" + : "contains a boolean" + : finding.keyword === "const" + ? "is an empty string" + : "contains an empty string"; + return `${path}${finding.path.slice(1)} ${reason}`; }; /** @@ -89,36 +49,23 @@ export const strictGatewayItemSchemaErrors = ( schema: unknown, path = "$", ): readonly string[] => { - if (!isRecord(schema)) return []; - const here: string[] = []; - if (schema.items !== undefined && schema.type !== "array") - here.push(`${path}.items is beside type ${JSON.stringify(schema.type)}`); - const items = schema.items; - if ( - schema.type === "array" && - isRecord(items) && - items.type === undefined && - (Array.isArray(items.oneOf) || Array.isArray(items.anyOf)) - ) - here.push(`${path}.items is an untyped union`); - return [ - ...here, - ...Object.entries(schema).flatMap(([key, value]) => { - if (schemaMaps.has(key) && isRecord(value)) - return Object.entries(value).flatMap(([name, child]) => - strictGatewayItemSchemaErrors(child, `${path}.${key}.${name}`), - ); - if (schemaArrays.has(key) && Array.isArray(value)) - return value.flatMap((child: unknown, index) => - strictGatewayItemSchemaErrors(child, `${path}.${key}[${index}]`), - ); - if (schemaChildren.has(key)) - return Array.isArray(value) - ? value.flatMap((child: unknown, index) => - strictGatewayItemSchemaErrors(child, `${path}.${key}[${index}]`), - ) - : strictGatewayItemSchemaErrors(value, `${path}.${key}`); - return []; - }), - ]; + const errors: string[] = []; + for (const position of clientSchemaPositions(schema)) { + if (position.kind !== "schema") continue; + const value = position.schema; + const selectedPath = `${path}${position.path.slice(1)}`; + if (value.items !== undefined && value.type !== "array") + errors.push( + `${selectedPath}.items is beside type ${JSON.stringify(value.type)}`, + ); + const items = value.items; + if ( + value.type === "array" && + isRecord(items) && + items.type === undefined && + (Array.isArray(items.oneOf) || Array.isArray(items.anyOf)) + ) + errors.push(`${selectedPath}.items is an untyped union`); + } + return errors; };