From ef1b92bf1eadadbe8f95dc5dab79d941d2e8c22e Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 04:20:31 +0800 Subject: [PATCH] test(mcp): check advertised schemas against a portable dialect in every profile (#10567) * test(mcp): check advertised schemas against a portable dialect in every profile * fix(mcp): validate advertised dialect by selected profile * fix(mcp): refuse zero-padded repeat bounds and record host-specific path patterns --- .github/workflows/ci.yml | 1 + docs/development/testing.md | 7 +- scripts/ci/scopes.mjs | 2 + scripts/lib/release-package-canary.mjs | 6 +- scripts/lib/verify-package-core.d.mts | 1 + scripts/lib/verify-package-core.mjs | 51 +- scripts/verify-package-mcp.mjs | 6 +- scripts/verify-recorded-crash.mjs | 10 +- scripts/verify-windows-package.mjs | 4 +- src/config/mcpInputSchemaProfile.ts | 10 +- src/contracts/advertisedDialect.ts | 549 ++++++++++++++++++ src/contracts/advertisedDialectBaseline.ts | 82 +++ .../mcp/absoluteLocalPathSchema.test.ts | 2 +- tests/boundary/mcp/advertisedDialect.test.ts | 206 +++++++ .../mcp/advertisedPatternsVMode.test.ts | 155 +---- .../mcp/compactInputSchemaMcp.test.ts | 231 +------- .../mcp/packageCatalogVerification.test.ts | 52 ++ 17 files changed, 986 insertions(+), 389 deletions(-) create mode 100644 src/contracts/advertisedDialect.ts create mode 100644 src/contracts/advertisedDialectBaseline.ts create mode 100644 tests/boundary/mcp/advertisedDialect.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bcb23b95e..4d9bceb234 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -935,6 +935,7 @@ jobs: src/contracts/sessionPathInputs.test.ts tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts tests/boundary/mcp/rawBinaryMcp.test.ts + tests/boundary/mcp/advertisedDialect.test.ts tests/boundary/filesystem/safeOutputTree.test.ts tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts diff --git a/docs/development/testing.md b/docs/development/testing.md index acce98b679..29f1dabeb7 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -289,7 +289,12 @@ operation name rather than array position. Check advertised schemas after SDK conversion for every supported schema profile. Meta-schema validity does not establish client acceptance or useful -rendered declarations. Keep canonical rejection checks separate from the +rendered declarations. `src/contracts/advertisedDialect.ts` owns the portable +advertised dialect: the keyword allowlist, size and depth limits, and the regex +grammar that compiles in V8, Python, RE2 and Rust. `advertisedDialect.test.ts` +lists every input, output and catalog profile combination and compares the +violation counts with `advertisedDialectBaseline.ts`, which may only shrink; +the packaged catalog check applies the same rules to the default profiles. Keep canonical rejection checks separate from the intentionally reduced compact advertisement, and probe actual client renderers when changing schema representations. diff --git a/scripts/ci/scopes.mjs b/scripts/ci/scopes.mjs index f00fff39e9..20da3e3149 100644 --- a/scripts/ci/scopes.mjs +++ b/scripts/ci/scopes.mjs @@ -265,6 +265,8 @@ const PATH_SCOPES = { "src/contracts/sessionPathInputs.test.ts", "tests/boundary/filesystem/directAnalysisSnapshotReplay.test.ts", "tests/boundary/mcp/rawBinaryMcp.test.ts", + "src/contracts/advertisedDialect*.ts", + "tests/boundary/mcp/advertisedDialect.test.ts", "tests/boundary/filesystem/safeOutputTree.test.ts", "tests/boundary/providers/native/artifactProvider.archiveSafety.test.ts", "tests/boundary/providers/native/artifactProvider.extractionCompleteness.test.ts", diff --git a/scripts/lib/release-package-canary.mjs b/scripts/lib/release-package-canary.mjs index 7783cc10a6..1ae3f66ec1 100644 --- a/scripts/lib/release-package-canary.mjs +++ b/scripts/lib/release-package-canary.mjs @@ -79,7 +79,11 @@ export async function verifyInstalledCanaryPackage( return withVerifierCleanup(async () => { try { await owner.open(client, transport); - const toolNames = await verifyCompleteToolCatalog(client); + const toolNames = await verifyCompleteToolCatalog( + client, + undefined, + environment, + ); return { mcpTools: toolNames.length, canonicalMcpTools: FULL_CATALOG_IDENTITY.counts.mcp_tools, diff --git a/scripts/lib/verify-package-core.d.mts b/scripts/lib/verify-package-core.d.mts index 88d04b9142..db565c8bc1 100644 --- a/scripts/lib/verify-package-core.d.mts +++ b/scripts/lib/verify-package-core.d.mts @@ -2,5 +2,6 @@ import type { Client } from "@modelcontextprotocol/client"; export function verifyCompleteToolCatalog( client: Pick, options?: Parameters[1], + environment?: Readonly, ): Promise; export function pathExists(path: string): Promise; diff --git a/scripts/lib/verify-package-core.mjs b/scripts/lib/verify-package-core.mjs index e6f06e64da..5487b28688 100644 --- a/scripts/lib/verify-package-core.mjs +++ b/scripts/lib/verify-package-core.mjs @@ -5,12 +5,26 @@ import { Ajv2020 } from "ajv/dist/2020.js"; * Verify canonical names, converted JSON Schemas, and session catalog metadata. * Only this operation needs built contracts; path checks remain source-only. */ -export const verifyCompleteToolCatalog = async (client, options) => { - const [{ requireMcpLifecycleResult, sortVerifierNames }, { TOOL_CONTRACTS }] = - await Promise.all([ - import("./mcp-verifier-results.mjs"), - import("../../dist/contracts/toolContracts.js"), - ]); +export const verifyCompleteToolCatalog = async ( + client, + options, + environment = {}, +) => { + const [ + { requireMcpLifecycleResult, sortVerifierNames }, + { TOOL_CONTRACTS }, + dialect, + { ADVERTISED_INPUT_DIALECT_BASELINE, ADVERTISED_OUTPUT_DIALECT_BASELINE }, + { mcpInputSchemaProfileSchema }, + { mcpOutputSchemaProfileSchema }, + ] = await Promise.all([ + import("./mcp-verifier-results.mjs"), + import("../../dist/contracts/toolContracts.js"), + import("../../dist/contracts/advertisedDialect.js"), + import("../../dist/contracts/advertisedDialectBaseline.js"), + import("../../dist/config/mcpInputSchemaProfile.js"), + import("../../dist/config/mcpOutputSchemaProfile.js"), + ]); const listed = await client.listTools(undefined, options); const canonical = sortVerifierNames(TOOL_CONTRACTS.map(({ name }) => name)); const observed = sortVerifierNames(listed.tools.map(({ name }) => name)); @@ -36,6 +50,31 @@ export const verifyCompleteToolCatalog = async (client, options) => { ); } } + // Compare the advertisement with the profiles actually passed to the server. + const inputProfile = mcpInputSchemaProfileSchema.parse( + environment.REA_MCP_INPUT_SCHEMA_PROFILE ?? "compact", + ); + const outputProfile = mcpOutputSchemaProfileSchema.parse( + environment.REA_MCP_OUTPUT_SCHEMA_PROFILE ?? "full", + ); + const regressions = [ + ...dialect.advertisedDialectRegressions( + dialect.countAdvertisedDialectViolations( + listed.tools.flatMap(dialect.advertisedInputDialectViolations), + ), + ADVERTISED_INPUT_DIALECT_BASELINE[inputProfile], + ), + ...dialect.advertisedDialectRegressions( + dialect.countAdvertisedDialectViolations( + listed.tools.flatMap(dialect.advertisedOutputPatternViolations), + ), + ADVERTISED_OUTPUT_DIALECT_BASELINE[outputProfile], + ), + ]; + if (regressions.length > 0) + throw new Error( + `packaged MCP advertises constructs outside the portable schema dialect: ${regressions.join("; ")}`, + ); const sessionDefinition = listed.tools.find( ({ name }) => name === "binary_session", ); diff --git a/scripts/verify-package-mcp.mjs b/scripts/verify-package-mcp.mjs index 6487f7b3f9..d77235befa 100644 --- a/scripts/verify-package-mcp.mjs +++ b/scripts/verify-package-mcp.mjs @@ -17,8 +17,8 @@ import * as prompts from "./verify-package-prompts.mjs"; import { verifyCompleteToolCatalog } from "./lib/verify-package-core.mjs"; import { withVerifierCleanup } from "./lib/verifier-cleanup.mjs"; -const verifyMcpToolsAndPrompts = async (client, mcpOptions) => { - await verifyCompleteToolCatalog(client, mcpOptions); +const verifyMcpToolsAndPrompts = async (client, mcpOptions, environment) => { + await verifyCompleteToolCatalog(client, mcpOptions, environment); await prompts.verifyPromptCatalog(client, mcpOptions, prompts.names); await prompts.verifyPromptCompletion(client, mcpOptions, []); }; @@ -283,7 +283,7 @@ export async function verifyPackageMcp({ try { await client.connect(transport); const mcpOptions = { timeout: 15_000 }; - await verifyMcpToolsAndPrompts(client, mcpOptions); + await verifyMcpToolsAndPrompts(client, mcpOptions, environment); await verifyMcpTargetFree(client, mcpOptions); await verifyMcpUnknownProvider(client, mcpOptions); await verifyMcpBinaryLifecycle(client, mcpOptions); diff --git a/scripts/verify-recorded-crash.mjs b/scripts/verify-recorded-crash.mjs index d4ce4aa629..b88b77ae32 100644 --- a/scripts/verify-recorded-crash.mjs +++ b/scripts/verify-recorded-crash.mjs @@ -97,9 +97,13 @@ try { advertised.filter((tool) => tool.name === "inspect_recorded_crash").length, 1, ); - const catalog = await verifyCompleteToolCatalog(basic.client, { - timeout: 90_000, - }); + const catalog = await verifyCompleteToolCatalog( + basic.client, + { + timeout: 90_000, + }, + environment, + ); for (const name of [ "inspect_binary_layout", "inspect_recorded_crash", diff --git a/scripts/verify-windows-package.mjs b/scripts/verify-windows-package.mjs index 05b95b324f..d1b5a3656d 100644 --- a/scripts/verify-windows-package.mjs +++ b/scripts/verify-windows-package.mjs @@ -109,7 +109,9 @@ try { let toolCount = 0; await withVerifierCleanup(async () => { await client.connect(server); - toolCount = (await verifyCompleteToolCatalog(client)).length; + toolCount = ( + await verifyCompleteToolCatalog(client, undefined, environment) + ).length; }, [ async () => { try { diff --git a/src/config/mcpInputSchemaProfile.ts b/src/config/mcpInputSchemaProfile.ts index 110e53a13f..9e156b1909 100644 --- a/src/config/mcpInputSchemaProfile.ts +++ b/src/config/mcpInputSchemaProfile.ts @@ -1,14 +1,12 @@ import { z } from "zod"; +import { ADVERTISED_INPUT_SCHEMA_MAX_BYTES } from "../contracts/advertisedDialect.js"; import { ConfigurationError } from "../domain/configurationErrors.js"; import { err, ok, type Result } from "../domain/result.js"; -// Moonshot's function-calling API rejects requests while any single tool -// parameters schema exceeds its literal serialized size cap. Community -// measurements bound the cap between ~13.7 KB (accepted) and ~15 KB -// (rejected); 13 KiB leaves margin under the lower bound. See -// https://github.com/morluto/rea/issues/1484 for the measured evidence. -export const COMPACT_INPUT_SCHEMA_BUDGET_BYTES = 13 * 1024; +/** The compact profile fits the portable dialect's per-tool size cap. */ +export const COMPACT_INPUT_SCHEMA_BUDGET_BYTES = + ADVERTISED_INPUT_SCHEMA_MAX_BYTES; export const mcpInputSchemaProfileSchema = z.enum(["full", "compact"]); diff --git a/src/contracts/advertisedDialect.ts b/src/contracts/advertisedDialect.ts new file mode 100644 index 0000000000..baf30a4aee --- /dev/null +++ b/src/contracts/advertisedDialect.ts @@ -0,0 +1,549 @@ +import { + COMPACT_SCHEMA_MAX_DEPTH, + schemaDepth, + serializedBytes, +} from "./compactJsonSchema.js"; +import { + schemaArrays, + schemaChildren, + schemaMaps, +} from "./jsonSchemaKeywords.js"; + +// The portable advertised dialect: the subset of JSON Schema that every +// client and model gateway REA has been seen to meet accepts, and the regex +// grammar that compiles in V8 (plain, `u`, `v`), Python `re`, Go `regexp` +// (RE2), Rust `regex` and `fancy-regex`. One construct outside it can make a +// client refuse the complete tools request, not only the affected tool. The +// server still validates every call against the canonical contract, so a +// projection into this dialect may widen an advertised constraint but never +// narrow it. Each rule names the rejection that forces it. + +/** One advertised construct outside the portable dialect. */ +export interface AdvertisedDialectViolation { + readonly tool: string; + readonly schema: "tool" | "input" | "output"; + /** JSON path inside the selected schema; `$` is its root. */ + readonly path: string; + readonly rule: AdvertisedDialectRule; + /** The specific keyword, type or regex construct, when the rule has several. */ + readonly detail?: string; +} + +export type AdvertisedDialectRule = + // Bedrock caps names at 64 and clients prefix `mcp__rea__` (10 characters). + | "tool-name" + // Claude Code truncates a tool description at 4,096 characters. + | "tool-description-length" + // Moonshot rejects a parameters schema above its literal size cap (#1484). + | "input-bytes" + // OpenCode and Muse Spark reject nesting deeper than ten levels (#2813, #919). + | "input-depth" + // Anthropic and OpenAI require a plain object root without a combinator (#1052). + | "root-shape" + // Each keyword outside the allowlist is rejected by at least one target. + | "keyword" + // Gemini, Moonshot and Ark reject type lists and untyped nodes (#7103, #3800). + | "type" + // VS Code and OpenAI reject arrays without `items`; Gemini rejects `items` + // beside a non-array type (#3576, #7103). + | "array-items" + // Gemini accepts only non-empty string enums (#4025, #5201). + | "enum" + // Root, empty or untyped unions, and unions beside properties (#3800). + | "any-of" + // VS Code truncates nested descriptions above 1,024 characters. + | "description-length" + // llama.cpp grammars fail at length and item bounds of 1,999 or more. + | "bound" + // Anthropic and Gemini property-name rules. + | "property-name" + // A pattern outside the portable regex grammar (#1329, #10348, #10389). + | "pattern" + // Codex turns `true` into a string schema. + | "boolean-schema"; + +const ALLOWED_KEYWORDS: ReadonlySet = new Set([ + "type", + "description", + "properties", + "required", + "additionalProperties", + "items", + "enum", + "anyOf", + "minItems", + "maxItems", + "minLength", + "maxLength", + "minimum", + "maximum", + "pattern", +]); +const PRIMITIVE_TYPES: ReadonlySet = new Set([ + "object", + "array", + "string", + "integer", + "number", + "boolean", +]); +const TOOL_NAME = /^[a-z][a-z0-9_]{0,53}$/u; +const PROPERTY_NAME = /^[A-Za-z_][A-Za-z0-9_]{0,63}$/u; +const TOOL_DESCRIPTION_LIMIT = 4096; +const DESCRIPTION_LIMIT = 1024; +const BOUND_LIMIT = 1999; +// Moonshot's function-calling API rejects requests while any single tool +// parameters schema exceeds its literal serialized size cap. Community +// measurements bound the cap between ~13.7 KB (accepted) and ~15 KB +// (rejected); 13 KiB leaves margin under the lower bound. See +// https://github.com/morluto/rea/issues/1484 for the measured evidence. +export const ADVERTISED_INPUT_SCHEMA_MAX_BYTES = 13 * 1024; + +// ---- Portable regex grammar ------------------------------------------------- +// +// pattern := "^" alt "$" (llama.cpp grammars require both anchors) +// atom := literal | escape | class | "(?:" alt ")" | "(" alt ")" +// quant := * + ? {n} {n,} {n,m} with n, m <= 1000, no leading zeros and no +// lazy or possessive suffix +// escape := "\" one of ^$\.*+?()[]{}| | \t \n \r \f \v | \x[0-7][0-9A-Fa-f] +// class := "[" "^"? item+ "]" where "-" is a range only between two +// endpoints, "-" elsewhere is escaped, and no punctuator repeats. +// Excluded: ".", \d \w \s \b and negations (portable syntax, different +// semantics), look-around, backreferences, named groups, \p, \u, \x{..}, \0, +// octal, inline flags, \A \z \Q..\E, non-ASCII literals. + +const ALNUM = /^[A-Za-z0-9]$/u; +const ESCAPABLE_OUTSIDE_CLASS = new Set("^$\\.*+?()[]{}|"); +const ESCAPABLE_IN_CLASS = new Set("^$\\.*+?()[]{}|-"); +const LITERAL_IN_CLASS = new Set("!#%&,:;<=>@`~'\" _.+*?$"); +const LITERAL_OUTSIDE_CLASS = new Set("_ !#%&,:;<=>@`~'\"-/"); +const CONTROL_ESCAPES = new Set("tnrfv"); +const ASCII_HEX_ESCAPE = /^[0-7][0-9A-Fa-f]$/u; +const REPEAT = /^\{(\d+)(?:,(\d*))?\}/u; + +const unescapedFinalDollar = (pattern: string): boolean => { + if (!pattern.endsWith("$")) return false; + let backslashes = 0; + for (let index = pattern.length - 2; pattern[index] === "\\"; index--) + backslashes++; + return backslashes % 2 === 0; +}; + +const escapeViolation = (escaped: string): string => { + if ("dDwWsS".includes(escaped)) return "shorthand-class"; + if ("bB".includes(escaped)) return "word-boundary"; + if (/^[0-9]$/u.test(escaped)) return "octal-or-backreference"; + if ("uU".includes(escaped)) return "unicode-escape"; + if ("pP".includes(escaped)) return "unicode-property"; + if (escaped === "k") return "named-backreference"; + if (escaped === "x") return "non-ascii-or-braced-hex-escape"; + return `escape:\\${escaped}`; +}; + +/** + * Name every construct in `pattern` outside the portable regex grammar. An + * empty result means it compiles with the same syntax in all seven engine + * modes the grammar was checked against. + */ +export const portablePatternViolations = ( + pattern: string, +): readonly string[] => { + const found = new Set(); + if (!pattern.startsWith("^") || !unescapedFinalDollar(pattern)) + found.add("unanchored"); + for (const character of pattern) { + const code = character.codePointAt(0)!; + if (code < 0x20 || code > 0x7e) found.add("non-ascii-literal"); + } + const at = (index: number): string => pattern[index] ?? ""; + let index = 0; + const groups = [1]; + let lastAtom = 1; + while (index < pattern.length) { + const character = at(index); + if (character === "\\") { + lastAtom = 1; + const escaped = at(index + 1); + if (escaped === "") { + found.add("trailing-escape"); + break; + } + if ( + ESCAPABLE_OUTSIDE_CLASS.has(escaped) || + CONTROL_ESCAPES.has(escaped) || + (escaped === "x" && + ASCII_HEX_ESCAPE.test(pattern.slice(index + 2, index + 4))) + ) { + index += escaped === "x" ? 4 : 2; + continue; + } + found.add( + escaped === "-" + ? "escaped-hyphen-outside-class" + : escapeViolation(escaped), + ); + index += 2; + continue; + } + if (character === "[") { + lastAtom = 1; + let cursor = index + 1; + if (at(cursor) === "^") cursor++; + if (at(cursor) === "]") found.add("empty-or-leading-bracket-class"); + let previous: "endpoint" | "range" | "punctuator" | "start" = "start"; + while (cursor < pattern.length && at(cursor) !== "]") { + const member = at(cursor); + if (member === "\\") { + const escaped = at(cursor + 1); + if ( + ESCAPABLE_IN_CLASS.has(escaped) || + CONTROL_ESCAPES.has(escaped) || + (escaped === "x" && + ASCII_HEX_ESCAPE.test(pattern.slice(cursor + 2, cursor + 4))) + ) { + cursor += escaped === "x" ? 4 : 2; + previous = "endpoint"; + continue; + } + found.add( + escaped === "" ? "trailing-escape" : escapeViolation(escaped), + ); + cursor += 2; + previous = "endpoint"; + continue; + } + if (member === "-") { + if ( + previous === "endpoint" && + cursor + 1 < pattern.length && + at(cursor + 1) !== "]" && + at(cursor + 1) !== "-" + ) { + cursor++; + previous = "range"; + continue; + } + found.add("unescaped-hyphen-in-class"); + cursor++; + previous = "punctuator"; + continue; + } + if (ALNUM.test(member)) { + cursor++; + previous = "endpoint"; + continue; + } + if (LITERAL_IN_CLASS.has(member)) { + if (at(cursor + 1) === member) + found.add("doubled-punctuator-in-class"); + cursor++; + previous = "punctuator"; + continue; + } + found.add( + "[({})|/^".includes(member) + ? `unescaped-syntax-in-class:${member}` + : "other-class-character", + ); + cursor++; + previous = "punctuator"; + } + if (cursor >= pattern.length) { + found.add("unterminated-class"); + break; + } + index = cursor + 1; + continue; + } + if (character === "(") { + groups.push(1); + if (pattern.startsWith("(?:", index)) { + index += 3; + continue; + } + if (/^\(\? 1 ? groups.pop()! : 1; + groups[groups.length - 1] = Math.max( + groups[groups.length - 1]!, + lastAtom, + ); + index++; + continue; + } + if ("*+?".includes(character)) { + if ("?+".includes(at(index + 1)) && at(index + 1) !== "") + found.add("lazy-or-possessive"); + index++; + continue; + } + if (character === "{") { + const repeat = REPEAT.exec(pattern.slice(index)); + if (repeat === null) { + found.add("literal-brace"); + index++; + continue; + } + // Go reads a bound with a leading zero, such as `{01}`, as literal text. + if ( + [repeat[1], repeat[2]].some( + (bound) => bound !== undefined && /^0\d/u.test(bound), + ) + ) + found.add("repeat-leading-zero"); + const lower = Number(repeat[1]); + const upper = + repeat[2] === undefined || repeat[2] === "" ? lower : Number(repeat[2]); + if (Math.max(lower, upper) > 1000) found.add("repeat-above-1000"); + const nested = lastAtom; + lastAtom *= Math.max(lower, upper); + if (nested > 1 && lastAtom > 1000) found.add("nested-repeat-above-1000"); + groups[groups.length - 1] = Math.max( + groups[groups.length - 1]!, + lastAtom, + ); + index += repeat[0].length; + if ("?+".includes(at(index)) && at(index) !== "") + found.add("lazy-or-possessive"); + continue; + } + if (character === ".") found.add("dot"); + else if ( + !"^$|)".includes(character) && + !ALNUM.test(character) && + !LITERAL_OUTSIDE_CLASS.has(character) + ) + found.add(`other-character:${character}`); + if (!"^$|".includes(character)) lastAtom = 1; + index++; + } + return [...found].sort(); +}; + +// ---- Schema walk ------------------------------------------------------------ + +type SchemaKind = "input" | "output"; + +const isRecord = (value: unknown): value is Readonly> => + typeof value === "object" && value !== null && !Array.isArray(value); + +/** Visit every schema position; instance literals stay opaque. */ +const visitSchemas = ( + schema: unknown, + path: string, + visit: (node: unknown, path: string, keyword: string | undefined) => void, + keyword?: string, +): void => { + visit(schema, path, keyword); + if (!isRecord(schema)) return; + for (const [key, value] of Object.entries(schema)) { + if (schemaMaps.has(key) && isRecord(value)) + for (const [name, child] of Object.entries(value)) + visitSchemas(child, `${path}.${key}.${name}`, visit, key); + else if (schemaArrays.has(key) && Array.isArray(value)) + value.forEach((child, index) => + visitSchemas(child, `${path}.${key}[${String(index)}]`, visit, key), + ); + else if (schemaChildren.has(key)) { + if (Array.isArray(value)) + value.forEach((child, index) => + visitSchemas(child, `${path}.${key}[${String(index)}]`, visit, key), + ); + else visitSchemas(value, `${path}.${key}`, visit, key); + } + } +}; + +/** Every `pattern` keyword at a schema position, with its path. */ +export const advertisedSchemaPatterns = ( + root: unknown, +): readonly { readonly path: string; readonly pattern: string }[] => { + const patterns: { path: string; pattern: string }[] = []; + visitSchemas(root, "$", (node, path) => { + if (isRecord(node) && typeof node.pattern === "string") + patterns.push({ path, pattern: node.pattern }); + }); + return patterns; +}; + +const patternViolations = ( + tool: string, + schema: SchemaKind, + root: unknown, +): AdvertisedDialectViolation[] => + advertisedSchemaPatterns(root).flatMap(({ path, pattern }) => + portablePatternViolations(pattern).map((detail) => ({ + tool, + schema, + path, + rule: "pattern" as const, + detail, + })), + ); + +const nodeViolations = ( + tool: string, + node: unknown, + path: string, + keyword: string | undefined, +): AdvertisedDialectViolation[] => { + const violations: AdvertisedDialectViolation[] = []; + const add = (rule: AdvertisedDialectRule, detail?: string) => + violations.push({ + tool, + schema: "input", + path, + rule, + ...(detail === undefined ? {} : { detail }), + }); + if (!isRecord(node)) { + // `additionalProperties: false` is the closed-object form every target accepts. + if ( + typeof node === "boolean" && + !(node === false && keyword === "additionalProperties") + ) + add("boolean-schema"); + return violations; + } + for (const key of Object.keys(node)) + if (!ALLOWED_KEYWORDS.has(key)) add("keyword", key); + const { type } = node; + if (Array.isArray(type)) add("type", "type-list"); + else if (type === undefined) add("type", "untyped"); + else if (typeof type !== "string" || !PRIMITIVE_TYPES.has(type)) + add("type", `type:${String(type)}`); + if (type === "array" && node.items === undefined) + add("array-items", "missing"); + if (node.items !== undefined && type !== "array") + add("array-items", "beside-non-array-type"); + if (Array.isArray(node.items)) add("array-items", "tuple"); + if (node.enum !== undefined) { + const values = node.enum; + if (!Array.isArray(values) || values.length === 0) add("enum", "empty"); + else if (values.some((value) => typeof value !== "string")) + add("enum", "non-string"); + else if (values.includes("")) add("enum", "empty-string"); + if (type !== "string") add("enum", "not-on-string-type"); + } + if (node.anyOf !== undefined) { + const branches = node.anyOf; + if (path === "$") add("any-of", "root"); + if (!Array.isArray(branches) || branches.length < 2) + add("any-of", "fewer-than-two-branches"); + else if ( + branches.some((branch) => !isRecord(branch) || branch.type === undefined) + ) + add("any-of", "untyped-branch"); + if (node.properties !== undefined) add("any-of", "beside-properties"); + } + if ( + typeof node.description === "string" && + node.description.length > DESCRIPTION_LIMIT + ) + add("description-length"); + for (const bound of ["minLength", "maxLength", "minItems", "maxItems"]) + if (typeof node[bound] === "number" && node[bound] >= BOUND_LIMIT) + add("bound", bound); + if (isRecord(node.properties)) + for (const name of Object.keys(node.properties)) + if (!PROPERTY_NAME.test(name)) add("property-name", name); + return violations; +}; + +/** Advertised tool fields the portable dialect constrains. */ +export interface AdvertisedToolDefinition { + readonly name: string; + readonly description?: string | undefined; + readonly inputSchema: unknown; + readonly outputSchema?: unknown; +} + +/** + * Every construct outside the portable dialect in one tool's name, + * description and input schema: the definition a client forwards to a model. + */ +export const advertisedInputDialectViolations = ( + tool: AdvertisedToolDefinition, +): readonly AdvertisedDialectViolation[] => { + const { name, inputSchema: input } = tool; + const violations: AdvertisedDialectViolation[] = []; + const whole = (schema: "tool" | "input", rule: AdvertisedDialectRule) => { + violations.push({ tool: name, schema, path: "$", rule }); + }; + if (!TOOL_NAME.test(name)) whole("tool", "tool-name"); + if ((tool.description ?? "").length > TOOL_DESCRIPTION_LIMIT) + whole("tool", "tool-description-length"); + if (serializedBytes(input) > ADVERTISED_INPUT_SCHEMA_MAX_BYTES) + whole("input", "input-bytes"); + if (schemaDepth(input) > COMPACT_SCHEMA_MAX_DEPTH) + whole("input", "input-depth"); + if ( + !isRecord(input) || + input.type !== "object" || + ["anyOf", "oneOf", "allOf"].some((key) => Object.hasOwn(input, key)) + ) + whole("input", "root-shape"); + visitSchemas(input, "$", (node, path, keyword) => + violations.push(...nodeViolations(name, node, path, keyword)), + ); + violations.push(...patternViolations(name, "input", input)); + return violations; +}; + +/** + * Patterns outside the portable grammar in one tool's output schema. No model + * reads output schemas, but SDK clients compile every one, and a pattern their + * engine refuses fails the call before it is sent. + */ +export const advertisedOutputPatternViolations = ( + tool: AdvertisedToolDefinition, +): readonly AdvertisedDialectViolation[] => + tool.outputSchema === undefined + ? [] + : patternViolations(tool.name, "output", tool.outputSchema); + +/** Stable count key: schema, rule and, when present, the specific construct. */ +export const advertisedDialectViolationKey = ( + violation: AdvertisedDialectViolation, +): string => + [violation.schema, violation.rule, violation.detail] + .filter((part) => part !== undefined) + .join(" "); + +/** Occurrences per violation key, in key order. */ +export const countAdvertisedDialectViolations = ( + violations: Iterable, +): Readonly> => { + const counts = new Map(); + for (const violation of violations) { + const key = advertisedDialectViolationKey(violation); + counts.set(key, (counts.get(key) ?? 0) + 1); + } + return Object.fromEntries( + [...counts].sort(([left], [right]) => (left < right ? -1 : 1)), + ); +}; + +/** + * Keys whose observed count exceeds the recorded allowance. A key absent from + * `allowed` is enforced at zero, so a new construct fails immediately. + */ +export const advertisedDialectRegressions = ( + observed: Readonly>, + allowed: Readonly>, +): readonly string[] => + Object.entries(observed).flatMap(([key, count]) => { + const limit = Object.hasOwn(allowed, key) ? allowed[key]! : 0; + return count > limit ? [`${key}: ${String(count)} > ${String(limit)}`] : []; + }); diff --git a/src/contracts/advertisedDialectBaseline.ts b/src/contracts/advertisedDialectBaseline.ts new file mode 100644 index 0000000000..ef103fc57a --- /dev/null +++ b/src/contracts/advertisedDialectBaseline.ts @@ -0,0 +1,82 @@ +// Advertised constructs outside the portable dialect that the presentation +// does not yet project away, counted per profile over every distinct tool +// definition the server can list (all catalog and output profiles). Counts may +// only shrink: the catalog test requires an exact match, so removing a +// construct lowers its count here, and any key not listed is enforced at zero. + +/** Input-side counts, keyed by `REA_MCP_INPUT_SCHEMA_PROFILE`. */ +export const ADVERTISED_INPUT_DIALECT_BASELINE: Readonly< + Record<"compact" | "full", Readonly>> +> = { + compact: { + "input bound maxItems": 2, + "input bound maxLength": 13, + "input keyword $schema": 256, + "input keyword allOf": 10, + "input keyword const": 81, + "input keyword default": 2, + "input keyword exclusiveMinimum": 61, + "input keyword format": 20, + "input keyword minProperties": 12, + "input keyword oneOf": 3, + "input keyword prefixItems": 2, + "input keyword propertyNames": 3, + "input keyword uniqueItems": 2, + "input pattern escape:\\/": 12, + "input pattern shorthand-class": 21, + "input pattern unanchored": 9, + "input type type-list": 11, + "input type type:null": 14, + "input type untyped": 40, + }, + full: { + "input any-of fewer-than-two-branches": 168, + "input any-of untyped-branch": 136, + "input boolean-schema": 1, + "input bound maxItems": 2, + "input bound maxLength": 30, + "input description-length": 1, + "input enum not-on-string-type": 1, + "input input-bytes": 23, + "input input-depth": 33, + "input keyword $schema": 256, + "input keyword allOf": 99, + "input keyword const": 1855, + "input keyword default": 803, + "input keyword else": 6, + "input keyword examples": 253, + "input keyword exclusiveMinimum": 335, + "input keyword format": 22, + "input keyword if": 7, + "input keyword minProperties": 12, + "input keyword not": 10, + "input keyword oneOf": 223, + "input keyword prefixItems": 255, + "input keyword propertyNames": 53, + "input keyword then": 7, + "input keyword uniqueItems": 5, + "input pattern dot": 5, + "input pattern escape:\\/": 105, + "input pattern non-ascii-or-braced-hex-escape": 1, + "input pattern shorthand-class": 30, + "input pattern unanchored": 98, + "input pattern unescaped-syntax-in-class:^": 1, + "input type type-list": 51, + "input type type:null": 372, + "input type untyped": 1454, + }, +}; + +/** Output-schema pattern counts, keyed by `REA_MCP_OUTPUT_SCHEMA_PROFILE`. */ +export const ADVERTISED_OUTPUT_DIALECT_BASELINE: Readonly< + Record<"full" | "compact" | "off", Readonly>> +> = { + full: { + "output pattern dot": 7, + "output pattern escape:\\/": 7, + "output pattern shorthand-class": 31, + "output pattern unanchored": 29, + }, + compact: {}, + off: {}, +}; diff --git a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts index 436124fd08..d10339bff1 100644 --- a/tests/boundary/mcp/absoluteLocalPathSchema.test.ts +++ b/tests/boundary/mcp/absoluteLocalPathSchema.test.ts @@ -354,7 +354,7 @@ it("rejects relative paths through in-memory MCP handlers", async () => { } }); -// advertisedPatternsVMode.test.ts proves no schema profile advertises the +// advertisedDialect.test.ts proves no schema profile advertises the // surrogate escape; this case proves the handlers still refuse such paths. it("rejects unpaired surrogates through in-memory MCP handlers", async () => { const firmwareCalls: string[] = []; diff --git a/tests/boundary/mcp/advertisedDialect.test.ts b/tests/boundary/mcp/advertisedDialect.test.ts new file mode 100644 index 0000000000..8b735905f0 --- /dev/null +++ b/tests/boundary/mcp/advertisedDialect.test.ts @@ -0,0 +1,206 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { describe, expect, it, onTestFinished } from "vitest"; + +import { + advertisedDialectRegressions, + advertisedInputDialectViolations, + advertisedOutputPatternViolations, + advertisedSchemaPatterns, + countAdvertisedDialectViolations, + portablePatternViolations, + type AdvertisedToolDefinition, +} from "../../../src/contracts/advertisedDialect.js"; +import { + ADVERTISED_INPUT_DIALECT_BASELINE, + ADVERTISED_OUTPUT_DIALECT_BASELINE, +} from "../../../src/contracts/advertisedDialectBaseline.js"; +import { mcpInputSchemaProfileSchema } from "../../../src/config/mcpInputSchemaProfile.js"; +import { mcpOutputSchemaProfileSchema } from "../../../src/config/mcpOutputSchemaProfile.js"; +import { mcpToolCatalogProfileSchema } from "../../../src/config/mcpToolCatalogProfile.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { silentLogger } from "../../../src/logger.js"; +import { createTestBinarySession } from "../../fixtures/binarySession.js"; + +describe("portable regex grammar", () => { + // Each refused construct failed to compile in at least one of V8 (plain, + // `u`, `v`), Python `re`, Go `regexp`, Rust `regex` or `fancy-regex`. + it.each([ + ["^[^\\0]*$", "octal-or-backreference"], + ["^[^\\uD800-\\uDFFF]*$", "unicode-escape"], + ["^[^\\x{0}]*$", "non-ascii-or-braced-hex-escape"], + ["^[A-Za-z0-9._-]+$", "unescaped-hyphen-in-class"], + ["^[^\\s[\\]]+$", "unescaped-syntax-in-class:["], + ["^(?!\\.\\.?$)[^/]+$", "look-around"], + ["^(a)\\1$", "octal-or-backreference"], + ["^(?a)$", "named-group"], + ["^\\p{L}+$", "unicode-property"], + ["^a{1,1001}$", "repeat-above-1000"], + ["^(?:a{32}){32}$", "nested-repeat-above-1000"], + ["^a{01}$", "repeat-leading-zero"], + ["^a{1,010}$", "repeat-leading-zero"], + ["^\\/[^\\x00]*$", "escape:\\/"], + ["^(?i)a$", "inline-flags-or-other-group"], + ["^\\-$", "escaped-hyphen-outside-class"], + ["^[\\:]$", "escape:\\:"], + ["^[..]$", "doubled-punctuator-in-class"], + ["^[^]$", "empty-or-leading-bracket-class"], + ["^a+?$", "lazy-or-possessive"], + ["^\\d+$", "shorthand-class"], + ["^a.b$", "dot"], + ["^\\/", "unanchored"], + ["^é$", "non-ascii-literal"], + ])("refuses %s", (pattern, construct) => { + expect(portablePatternViolations(pattern)).toContain(construct); + }); + + it.each([ + "^[^\\x00]*$", + "^[A-Za-z0-9._\\-]+$", + "^[a-z0-9_\\-]+$", + "^(?:[A-Za-z]|_)[A-Za-z0-9_]{0,63}$", + "^/[^\\x00]*$", + "^(?:a{32}|b{50}){20}$", + "^[!#%&,:;<=>@`~'\" _.+*?$]$", + "^a{2,1000}$", + "^(?:a|b)\\.c\\$$", + ])("accepts %s in every JavaScript mode", (pattern) => { + expect(portablePatternViolations(pattern)).toEqual([]); + for (const flags of ["", "u", "v"]) + expect(() => new RegExp(pattern, flags)).not.toThrow(); + }); +}); + +const listTools = async (environment: Readonly) => { + const session = createTestBinarySession(() => { + throw new Error("No deep provider may start for a schema projection"); + }); + const server = createServer( + { kind: "session", session }, + { environment, logger: silentLogger }, + ); + const client = new Client({ name: "advertised-dialect", version: "1" }); + onTestFinished(async () => { + await client.close(); + await server.close(); + await session.close(); + }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + await client.connect(clientTransport); + return (await client.listTools()).tools; +}; + +const distinct = ( + into: Map, + definition: AdvertisedToolDefinition, +) => into.set(JSON.stringify(definition), definition); + +it("advertises every profile within the portable dialect or its recorded baseline", async () => { + const inputs = new Map>(); + const outputs = new Map>(); + const uncompilable: string[] = []; + for (const input of mcpInputSchemaProfileSchema.options) + for (const output of mcpOutputSchemaProfileSchema.options) + for (const catalog of mcpToolCatalogProfileSchema.options) { + const tools = await listTools({ + ...process.env, + REA_MCP_INPUT_SCHEMA_PROFILE: input, + REA_MCP_OUTPUT_SCHEMA_PROFILE: output, + REA_MCP_TOOL_CATALOG_PROFILE: catalog, + }); + const combination = `${input}/${output}/${catalog}`; + // Every combination advertises local path patterns, so the grammar + // check is never vacuous. + expect( + tools.flatMap(({ inputSchema }) => + advertisedSchemaPatterns(inputSchema), + ).length, + combination, + ).toBeGreaterThan(0); + for (const tool of tools) { + if (!inputs.has(input)) inputs.set(input, new Map()); + if (!outputs.has(output)) outputs.set(output, new Map()); + distinct(inputs.get(input)!, { + name: tool.name, + description: tool.description, + inputSchema: tool.inputSchema, + }); + distinct(outputs.get(output)!, { + name: tool.name, + inputSchema: {}, + outputSchema: tool.outputSchema, + }); + // The grammar was checked against seven engines offline; this keeps + // V8's own modes, including baselined patterns, checked in CI. + for (const schema of [tool.inputSchema, tool.outputSchema]) + for (const { path, pattern } of advertisedSchemaPatterns(schema)) + for (const flags of ["", "u", "v"]) + try { + new RegExp(pattern, flags); + } catch (cause: unknown) { + uncompilable.push( + `${combination} ${tool.name} ${path} /${pattern}/${flags}: ${String(cause)}`, + ); + } + } + } + expect(uncompilable).toEqual([]); + + const observed = { + input: Object.fromEntries( + [...inputs].map(([profile, tools]) => [ + profile, + countAdvertisedDialectViolations( + [...tools.values()].flatMap(advertisedInputDialectViolations), + ), + ]), + ), + output: Object.fromEntries( + [...outputs].map(([profile, tools]) => [ + profile, + countAdvertisedDialectViolations( + [...tools.values()].flatMap(advertisedOutputPatternViolations), + ), + ]), + ), + }; + // New constructs fail first with a named key; a count that dropped below + // the baseline must be recorded so it cannot grow back. + expect({ + input: Object.fromEntries( + Object.entries(observed.input).map(([profile, counts]) => [ + profile, + advertisedDialectRegressions( + counts, + ADVERTISED_INPUT_DIALECT_BASELINE[ + profile as keyof typeof ADVERTISED_INPUT_DIALECT_BASELINE + ], + ), + ]), + ), + output: Object.fromEntries( + Object.entries(observed.output).map(([profile, counts]) => [ + profile, + advertisedDialectRegressions( + counts, + ADVERTISED_OUTPUT_DIALECT_BASELINE[ + profile as keyof typeof ADVERTISED_OUTPUT_DIALECT_BASELINE + ], + ), + ]), + ), + }).toEqual({ + input: { compact: [], full: [] }, + output: { full: [], compact: [], off: [] }, + }); + // Absolute local paths are advertised with the host's pattern + // (`localPath.ts`): POSIX hosts advertise `^\/`, which the grammar counts, + // and Windows hosts a drive-or-root pattern, which it does not. Exact counts + // are recorded on POSIX; Windows still enforces them as upper bounds above. + if (process.platform !== "win32") + expect(observed).toEqual({ + input: ADVERTISED_INPUT_DIALECT_BASELINE, + output: ADVERTISED_OUTPUT_DIALECT_BASELINE, + }); +}); diff --git a/tests/boundary/mcp/advertisedPatternsVMode.test.ts b/tests/boundary/mcp/advertisedPatternsVMode.test.ts index f33927d714..29ea3a5e36 100644 --- a/tests/boundary/mcp/advertisedPatternsVMode.test.ts +++ b/tests/boundary/mcp/advertisedPatternsVMode.test.ts @@ -3,6 +3,7 @@ import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ import { expect, it, onTestFinished } from "vitest"; import { createServer } from "../../../src/server/createServer.js"; +import { advertisedSchemaPatterns } from "../../../src/contracts/advertisedDialect.js"; import { BASE64_QUARTET_LENGTH_PATTERN, INVALID_BASE64_CHARACTER_PATTERN, @@ -11,11 +12,9 @@ import { } from "../../../src/domain/stringPatterns.js"; import { createTestBinarySession } from "../../fixtures/binarySession.js"; -interface AdvertisedPattern { - readonly tool: string; - readonly where: string; - readonly pattern: string; -} +// Pattern portability across every profile is owned by +// advertisedDialect.test.ts. This file keeps the composed canonical base64 +// constraint's advertised semantics. const collectCanonicalBase64Leaves = ( value: unknown, @@ -44,36 +43,11 @@ const collectCanonicalBase64Leaves = ( ); }; -const collectPatterns = ( - value: unknown, - where: string, - tool: string, - found: AdvertisedPattern[], -): void => { - if (value === null || typeof value !== "object") return; - if (Array.isArray(value)) { - value.forEach((entry, index) => - collectPatterns(entry, `${where}[${String(index)}]`, tool, found), - ); - return; - } - for (const [key, entry] of Object.entries(value)) { - if (key === "pattern" && typeof entry === "string") - found.push({ tool, where: `${where}/pattern`, pattern: entry }); - collectPatterns(entry, `${where}/${key}`, tool, found); - } -}; - -const listAdvertisedTools = async ( - environment?: Readonly, -) => { +const listAdvertisedTools = async () => { const session = createTestBinarySession(() => { throw new Error("No deep provider may start for a schema projection"); }); - const server = createServer( - { kind: "session", session }, - environment === undefined ? {} : { environment }, - ); + const server = createServer({ kind: "session", session }, {}); const client = new Client({ name: "advertised-patterns", version: "1" }); onTestFinished(async () => { await client.close(); @@ -87,27 +61,18 @@ const listAdvertisedTools = async ( return (await client.listTools()).tools; }; -// A `\u` or `\0` escape that is not itself escaped. RE2 has no `\u` escape, -// Rust refuses `\u` surrogate ranges, and DeepSeek rejects the short NUL escape -// even though V8 accepts all of them; any one refuses the complete tools -// request. Unlike the compile checks above, this guard covers every profile. -const NON_PORTABLE_ESCAPE = /(?:^|[^\\])(?:\\\\)*\\[0u]/u; - -const nonPortableEscapes = (patterns: readonly AdvertisedPattern[]) => - patterns.filter(({ pattern }) => NON_PORTABLE_ESCAPE.test(pattern)); - -it("advertises portable NUL escapes and patterns that compile in all JS modes", async () => { +it("advertises the composed canonical base64 constraint with exact SDK semantics", async () => { const advertised = await listAdvertisedTools(); - const patterns: AdvertisedPattern[] = []; + const patterns: string[] = []; const canonicalBase64Leaves: Record[] = []; - for (const tool of advertised) { - collectPatterns(tool.inputSchema, "inputSchema", tool.name, patterns); - collectCanonicalBase64Leaves(tool.inputSchema, canonicalBase64Leaves); - if (tool.outputSchema !== undefined) - collectPatterns(tool.outputSchema, "outputSchema", tool.name, patterns); - if (tool.outputSchema !== undefined) - collectCanonicalBase64Leaves(tool.outputSchema, canonicalBase64Leaves); - } + for (const tool of advertised) + for (const schema of [tool.inputSchema, tool.outputSchema]) { + if (schema === undefined) continue; + patterns.push( + ...advertisedSchemaPatterns(schema).map(({ pattern }) => pattern), + ); + collectCanonicalBase64Leaves(schema, canonicalBase64Leaves); + } for (const pattern of [ BASE64_QUARTET_LENGTH_PATTERN, @@ -116,41 +81,12 @@ it("advertises portable NUL escapes and patterns that compile in all JS modes", INVALID_CANONICAL_BASE64_PAD_BITS_PATTERN, ]) expect( - patterns.some(({ pattern: advertisedPattern }) => + patterns.some((advertisedPattern) => advertisedPattern.includes(pattern.source), ), pattern.source, ).toBe(true); - // Annex B and `u` mode accept an unescaped `-`, `/` or `[` inside a character - // class, so REA's own schema validation cannot observe the defect. `v` mode - // rejects them, and a client that compiles advertised patterns rejects the - // complete tools request rather than the affected call. - const uncompilable = patterns.flatMap((entry) => - ["", "u", "v"].flatMap((flags) => { - try { - new RegExp(entry.pattern, flags); - return []; - } catch (cause: unknown) { - return [ - `${entry.tool} ${entry.where} /${entry.pattern}/${flags}: ${cause instanceof Error ? cause.message : String(cause)}`, - ]; - } - }), - ); - expect(uncompilable).toEqual([]); - - // RE2 and Rust validators reject look-around and backreferences, and like - // the NUL escape below, one such pattern refuses the complete tools request. - expect( - patterns.filter(({ pattern }) => /\(\? pattern.includes("\\x00"))).toBe(true); - expect(canonicalBase64Leaves.length).toBeGreaterThan(0); const sdkValidator = new AjvJsonSchemaValidator(); const validators = canonicalBase64Leaves.map((schema) => @@ -166,60 +102,3 @@ it("advertises portable NUL escapes and patterns that compile in all JS modes", ), ); }); - -it("advertises patterns compatible with the full input profile", async () => { - const advertised = await listAdvertisedTools({ - ...process.env, - REA_MCP_INPUT_SCHEMA_PROFILE: "full", - }); - const patterns: AdvertisedPattern[] = []; - for (const tool of advertised) { - collectPatterns(tool.inputSchema, "inputSchema", tool.name, patterns); - if (tool.outputSchema !== undefined) - collectPatterns(tool.outputSchema, "outputSchema", tool.name, patterns); - } - - const uncompilable = patterns.flatMap((entry) => - ["", "u", "v"].flatMap((flags) => { - try { - new RegExp(entry.pattern, flags); - return []; - } catch (cause: unknown) { - return [ - `${entry.tool} ${entry.where} /${entry.pattern}/${flags}: ${cause instanceof Error ? cause.message : String(cause)}`, - ]; - } - }), - ); - expect(uncompilable).toEqual([]); - - expect( - patterns.filter(({ pattern }) => /\(\? - (["full", "compact", "off"] as const).map((output) => [input, output]), - ), -)( - "advertises portable escapes for %s input and %s output schemas", - async (input, output) => { - const advertised = await listAdvertisedTools({ - ...process.env, - REA_MCP_INPUT_SCHEMA_PROFILE: input, - REA_MCP_OUTPUT_SCHEMA_PROFILE: output, - }); - const patterns: AdvertisedPattern[] = []; - for (const tool of advertised) { - collectPatterns(tool.inputSchema, "inputSchema", tool.name, patterns); - if (tool.outputSchema !== undefined) - collectPatterns(tool.outputSchema, "outputSchema", tool.name, patterns); - } - // Every profile carries local path inputs, so the guard is not vacuous. - expect(patterns.some(({ pattern }) => pattern.includes("\\x00"))).toBe( - true, - ); - expect(nonPortableEscapes(patterns)).toEqual([]); - }, -); diff --git a/tests/boundary/mcp/compactInputSchemaMcp.test.ts b/tests/boundary/mcp/compactInputSchemaMcp.test.ts index 5a8b43dad3..625dd53343 100644 --- a/tests/boundary/mcp/compactInputSchemaMcp.test.ts +++ b/tests/boundary/mcp/compactInputSchemaMcp.test.ts @@ -11,11 +11,6 @@ import { PROMPT_CONTRACTS } from "../../../src/contracts/promptContracts.js"; import { advertisedInputJsonSchema } from "../../../src/contracts/toolSchemaMetadata.js"; import { presentAdvertisedInputSchema } from "../../../src/contracts/compactInputSchema.js"; import { createCatalogDigests } from "../../../src/catalogIdentity.js"; -import { - schemaArrays, - schemaChildren, - schemaMaps, -} from "../../../src/contracts/jsonSchemaKeywords.js"; import { COMPACT_INPUT_SCHEMA_BUDGET_BYTES, parseMcpInputSchemaProfile, @@ -41,160 +36,6 @@ interface AdvertisedTool { const serializedBytes = (value: unknown): number => Buffer.byteLength(JSON.stringify(value), "utf8"); -const schemaDepth = (value: unknown, depth = 0): number => { - if (Array.isArray(value)) - return Math.max( - depth, - ...value.map((child) => schemaDepth(child, depth + 1)), - ); - if (typeof value !== "object" || value === null) return depth; - return Math.max( - depth, - ...Object.values(value).map((child) => schemaDepth(child, depth + 1)), - ); -}; - -const unsupportedDependencyKeywords = ( - schema: unknown, - path = "$", -): readonly string[] => { - if (Array.isArray(schema)) - return schema.flatMap((child, index) => - unsupportedDependencyKeywords(child, `${path}[${index}]`), - ); - if (typeof schema !== "object" || schema === null) return []; - return Object.entries(schema).flatMap(([key, value]) => { - const childPath = `${path}.${key}`; - if (key === "dependencies" || key === "dependentRequired") - return [childPath]; - if (["const", "enum", "default", "examples"].includes(key)) return []; - if (schemaMaps.has(key) && typeof value === "object" && value !== null) - return Object.entries(value).flatMap(([name, child]) => - unsupportedDependencyKeywords(child, `${childPath}.${name}`), - ); - if (schemaArrays.has(key) && Array.isArray(value)) - return unsupportedDependencyKeywords(value, childPath); - if (schemaChildren.has(key)) - return unsupportedDependencyKeywords(value, childPath); - return []; - }); -}; - -// Moonshot rejects a $ref whose target is a oneOf/anyOf whose branches -// contain another $ref (morluto/rea#9581): "detected infinite recursion -// without termination condition". The rejection is about the reference -// shape, not the union itself, so this checks the pattern rather than -// banning references outright: a genuinely recursive schema still needs -// $ref, but never in this shape. -const resolveLocalPointer = (root: unknown, reference: string): unknown => { - if (!reference.startsWith("#/")) return undefined; - let current: unknown = root; - for (const token of reference - .slice(2) - .split("/") - .map((part) => part.replaceAll("~1", "/").replaceAll("~0", "~"))) { - if (typeof current !== "object" || current === null) return undefined; - current = (current as Record)[token]; - } - return current; -}; - -const collectSchemaReferences = ( - node: unknown, - path: string, - collected: { path: string; reference: string }[], -): void => { - if (Array.isArray(node)) { - node.forEach((child, index) => - collectSchemaReferences(child, `${path}[${index}]`, collected), - ); - return; - } - if (typeof node !== "object" || node === null) return; - for (const [key, value] of Object.entries(node)) { - if (key === "$ref" && typeof value === "string") { - collected.push({ path: `${path}.$ref`, reference: value }); - continue; - } - if (["const", "enum", "default", "examples"].includes(key)) continue; - if (schemaMaps.has(key) && typeof value === "object" && value !== null) { - for (const [name, child] of Object.entries(value)) - collectSchemaReferences(child, `${path}.${key}.${name}`, collected); - } else if (schemaArrays.has(key) && Array.isArray(value)) { - value.forEach((child, index) => - collectSchemaReferences(child, `${path}.${key}[${index}]`, collected), - ); - } else if (schemaChildren.has(key)) { - if (Array.isArray(value)) - value.forEach((child, index) => - collectSchemaReferences(child, `${path}.${key}[${index}]`, collected), - ); - else collectSchemaReferences(value, `${path}.${key}`, collected); - } - } -}; - -/** Paths of $refs Moonshot rejects: targets that are a oneOf/anyOf with a nested $ref. */ -const moonshotRejectedReferences = ( - schema: Record, -): readonly string[] => { - const collected: { path: string; reference: string }[] = []; - collectSchemaReferences(schema, "$", collected); - return collected - .filter(({ reference }) => { - const target = resolveLocalPointer(schema, reference); - if (typeof target !== "object" || target === null) return false; - for (const key of ["oneOf", "anyOf"]) { - const branches = (target as Record)[key]; - if (!Array.isArray(branches)) continue; - const nested: { path: string; reference: string }[] = []; - for (const branch of branches) - collectSchemaReferences(branch, "$", nested); - if (nested.length > 0) return true; - } - return false; - }) - .map(({ path }) => path); -}; - -it("distinguishes rejected references from allowed references and literal data", () => { - const union = { - oneOf: [ - { type: "object", properties: { a: { $ref: "#/$defs/s" } } }, - { type: "object", properties: { a: { $ref: "#/$defs/t" } } }, - ], - }; - const definitions = { s: { type: "string" }, t: { type: "number" } }; - const rejected = { - type: "object", - properties: { c: { $ref: "#/$defs/u" } }, - $defs: { ...definitions, u: union }, - }; - expect(moonshotRejectedReferences(rejected)).toEqual(["$.properties.c.$ref"]); - expect( - moonshotRejectedReferences({ - type: "object", - properties: { c: { $ref: "#/$defs/s" } }, - $defs: definitions, - }), - ).toEqual([]); - expect( - moonshotRejectedReferences({ - type: "object", - properties: { c: { $ref: "#/$defs/a~1b~0c" } }, - $defs: { ...definitions, "a/b~c": union }, - }), - ).toEqual(["$.properties.c.$ref"]); - expect( - moonshotRejectedReferences({ - const: rejected, - enum: [rejected], - default: rejected, - examples: [rejected], - }), - ).toEqual([]); -}); - const listToolsThroughTransport = async ( server: EvidenceMcpServer, ): Promise => { @@ -295,24 +136,13 @@ describe("MCP compact input schema profile", () => { }); }); -it("advertises every tool input schema within the provider budget", async () => { +it("advertises item schemas strict gateways accept", async () => { const server = compactServer(); registerCatalog(server); const tools = await listToolsThroughTransport(server); expect(tools.map(({ name }) => name).sort()).toEqual( TOOL_CONTRACTS.map(({ name }) => name).sort(), ); - const oversized = tools.filter( - ({ inputSchema }) => - serializedBytes(inputSchema) > COMPACT_INPUT_SCHEMA_BUDGET_BYTES, - ); - expect(oversized.map(({ name }) => name)).toEqual([]); - const tooDeep = tools.filter( - ({ inputSchema }) => schemaDepth(inputSchema) > 10, - ); - expect( - tooDeep.map(({ name, inputSchema }) => [name, schemaDepth(inputSchema)]), - ).toEqual([]); expect( tools.flatMap(({ name, inputSchema }) => strictGatewayItemSchemaErrors(inputSchema, name), @@ -532,7 +362,7 @@ it("keeps SDK-converted schemas valid and accepting canonical examples", async ( expect(checkedFullRestrictions).toBe(true); }); -it("omits Gemini-unsupported dependency keywords from both catalog profiles", async () => { +it("states dependency keywords as prose in both catalog profiles", async () => { const full = fullServer(); const compact = compactServer(); registerCatalog(full); @@ -541,14 +371,6 @@ it("omits Gemini-unsupported dependency keywords from both catalog profiles", as listToolsThroughTransport(full), listToolsThroughTransport(compact), ]); - for (const tools of [fullTools, compactTools]) - expect( - tools.flatMap(({ name, inputSchema }) => - unsupportedDependencyKeywords(inputSchema).map( - (path) => `${name}${path.slice(1)}`, - ), - ), - ).toEqual([]); const sqlite = fullTools.find( ({ name }) => name === "inspect_sqlite_database", ); @@ -597,55 +419,6 @@ it("retains parameter types and avoids client-side reference expansion", async ( }); }); -it("advertises Moonshot-accepted input schemas without the rejected reference pattern in both profiles", async () => { - // morluto/rea#9581: Kimi/Moonshot rejects a $ref whose target is a - // oneOf/anyOf with another $ref inside, which broke every Kimi request - // because clients send the whole tool list. The nine tools named in the - // issue must stay advertised with inlined schemas in both profiles. - const rejectedInIssue = [ - "capture_browser_scenario", - "capture_native_ui_scenario", - "observe_native_calls", - "inspect_analysis_view", - "compare_web_captures", - "trace_javascript_semantics", - "compare_process_captures", - "correlate_static_and_runtime", - "verify_reconstruction", - ]; - for (const profile of ["full", "compact"] as const) { - const server = profile === "full" ? fullServer() : compactServer(); - registerCatalog(server); - const tools = await listToolsThroughTransport(server); - const byName = new Map(tools.map((tool) => [tool.name, tool])); - for (const name of rejectedInIssue) { - const tool = byName.get(name); - if (tool === undefined) - throw new Error(`${profile} profile: ${name} was not advertised`); - expect( - tool.inputSchema, - `${profile} profile: ${name}`, - ).not.toHaveProperty("$defs"); - expect( - moonshotRejectedReferences(tool.inputSchema), - `${profile} profile: ${name}`, - ).toEqual([]); - } - expect( - tools.flatMap(({ name, inputSchema }) => - moonshotRejectedReferences(inputSchema).map( - (path) => `${profile} profile: ${name}${path.slice(1)}`, - ), - ), - ).toEqual([]); - expect( - tools - .filter(({ inputSchema }) => Object.hasOwn(inputSchema, "$defs")) - .map(({ name }) => `${profile} profile: ${name}`), - ).toEqual([]); - } -}); - it("parses the profile selection the server startup reads", () => { expect(parseMcpInputSchemaProfile("compact")).toEqual({ ok: true, diff --git a/tests/boundary/mcp/packageCatalogVerification.test.ts b/tests/boundary/mcp/packageCatalogVerification.test.ts index 73c47ba631..ec87d6f484 100644 --- a/tests/boundary/mcp/packageCatalogVerification.test.ts +++ b/tests/boundary/mcp/packageCatalogVerification.test.ts @@ -4,6 +4,26 @@ import { verifyCompleteToolCatalog } from "../../../scripts/lib/verify-package-c import { toolContract } from "../../../src/contracts/toolContracts.js"; import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +it("verifies the profiles selected for the packaged server", async () => { + const environment = { + REA_MCP_INPUT_SCHEMA_PROFILE: "full", + REA_MCP_OUTPUT_SCHEMA_PROFILE: "compact", + }; + const harness = await createApplicationMcpHarness({ + serverOptions: { environment }, + }); + try { + const names = await verifyCompleteToolCatalog( + harness.client, + undefined, + environment, + ); + expect(names).toContain("binary_session"); + } finally { + await harness.close(); + } +}); + it("verifies canonical names, converted schemas and session metadata together", async () => { const harness = await createApplicationMcpHarness(); try { @@ -159,3 +179,35 @@ it("verifies canonical names, converted schemas and session metadata together", await harness.close(); } }); + +it("refuses a packaged catalog with a pattern outside the portable dialect", async () => { + const harness = await createApplicationMcpHarness(); + try { + const listed = await harness.client.listTools(); + const [first, ...rest] = listed.tools; + if (first === undefined) throw new Error("Expected canonical tools"); + // RE2 and Rust refuse look-around, and one refused pattern makes some + // clients reject every tool. The check runs before any tool call. + for (const kind of ["inputSchema", "outputSchema"] as const) { + const lookAround = { + ...first, + [kind]: { + type: "object" as const, + properties: { name: { type: "string", pattern: "^(?!con$)[a-z]+$" } }, + }, + }; + await expect( + verifyCompleteToolCatalog({ + listTools: async () => ({ ...listed, tools: [lookAround, ...rest] }), + callTool: async () => { + throw new Error("The dialect check must refuse before any call"); + }, + }), + ).rejects.toThrow( + `${kind === "inputSchema" ? "input" : "output"} pattern look-around: 1 > 0`, + ); + } + } finally { + await harness.close(); + } +});