diff --git a/docs/client-verification.md b/docs/client-verification.md index 575cc61d24..1eed82281d 100644 --- a/docs/client-verification.md +++ b/docs/client-verification.md @@ -6,6 +6,28 @@ and provider lanes. Each record below states its host, client version, model fixture, and remaining coverage; a native-client run with a loopback model does not establish live model-service acceptance. +## Published Pi code-mode renderer limit + +A separate probe of the published `@earendil-works/pi-codemode` 1.1.0 and 1.2.0 +declaration renderers rendered the full `compare_web_captures` input as +`args: unknown`: its generated TypeScript type exceeds Pi's 16,000-character +default. REA keeps the full advertisement's fields by inlining references when +the shared form would exceed Pi's 32-reference expansion limit; retaining that +shared form would leave later fields unknown instead. The existing +`REA_MCP_INPUT_SCHEMA_PROFILE=compact` profile lets callers stay below +its character limit by advertising reduced nested validation structure, while +REA continues to validate calls against the complete canonical input schema. +Compact applies to the server's advertised inputs and is not an automatic +full-profile fallback; see the [compact input schema profile](mcp-contracts.md#compact-input-schema-profile) +for details of its reduced forms. + +Custom renderer hosts can raise `inputMaxChars` through `renderToolSignature` or +`renderToolSample`; `renderDeclarations` does not expose that override. The +probe exercised the published renderer code, not a native Pi host or model, and +does not establish broader Pi compatibility. See the pinned upstream +[`declarations.ts`](https://github.com/earendil-works/pi/blob/b45e0ebcc5ae2fc42a6f2f23104d10e656e6bedb/packages/codemode/src/declarations.ts) +and [renderer usage notes](https://github.com/earendil-works/pi/blob/b45e0ebcc5ae2fc42a6f2f23104d10e656e6bedb/packages/codemode/README.md#declarations-for-the-model). + ## End-to-end, integration and golden evidence The optional `verify:qwen-client`, `verify:pi-client`, and `verify:hermes-client` diff --git a/src/contracts/compactInputSchema.test.ts b/src/contracts/compactInputSchema.test.ts index 212d3dbb42..8953679efa 100644 --- a/src/contracts/compactInputSchema.test.ts +++ b/src/contracts/compactInputSchema.test.ts @@ -263,7 +263,12 @@ describe("compact input schema budget and reduction", () => { expect(presentation).toBe("reduced"); expect(schema.properties).toEqual({ capture: { type: "object", description: "Complete capture." }, - observations: { type: "array" }, + observations: { + type: "array", + items: { + type: ["null", "boolean", "object", "array", "number", "string"], + }, + }, label: { type: ["string", "null"] }, }); const validate = new Ajv2020({ strict: false }).compile(schema); diff --git a/src/contracts/compactInputSchema.ts b/src/contracts/compactInputSchema.ts index 49585bae3c..cf297000bb 100644 --- a/src/contracts/compactInputSchema.ts +++ b/src/contracts/compactInputSchema.ts @@ -289,7 +289,15 @@ const reduceToShallowProperties = ( : {}; return [ name, - { ...(type === undefined ? {} : { type }), ...description }, + { + ...(type === undefined ? {} : { type }), + // Reduced inputs intentionally omit nested validation, but clients + // still require an item schema to accept an array declaration. + ...(types?.includes("array") + ? { items: { type: jsonValueTypes } } + : {}), + ...description, + }, ]; }), ); diff --git a/src/contracts/tupleSchemaParity.test.ts b/src/contracts/tupleSchemaParity.test.ts index 3445804e43..2589d5643d 100644 --- a/src/contracts/tupleSchemaParity.test.ts +++ b/src/contracts/tupleSchemaParity.test.ts @@ -8,6 +8,7 @@ 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"; +import { compactAdvertisedInputSchema } from "./compactInputSchema.js"; const directions = [ [], @@ -162,3 +163,59 @@ describe.each(["draft-2020-12", "draft-07"] as const)( }); }, ); + +describe("fixed tuple element guidance in advertised projections", () => { + it.each(["draft-2020-12", "draft-07"] as const)( + "keeps literal item guidance in full and compact %s projections", + (target) => { + const full = z.toJSONSchema(relationshipCoverageSchema, { + io: "output", + target, + }); + for (const schema of [ + full, + compactAdvertisedInputSchema(full, 64 * 1024).schema, + ]) { + const tupleSchemas: Record[] = []; + const visit = (value: unknown): void => { + if (Array.isArray(value)) { + value.forEach(visit); + } else if (typeof value === "object" && value !== null) { + const candidate = value as Record; + if ( + Array.isArray(candidate.prefixItems) || + (Array.isArray(candidate.items) && "additionalItems" in candidate) + ) + tupleSchemas.push(candidate); + Object.values(candidate).forEach(visit); + } + }; + visit(schema); + + expect(tupleSchemas).toHaveLength(6); + for (const tuple of tupleSchemas) { + expect(tuple).toMatchObject({ + type: "array", + minItems: expect.any(Number), + maxItems: expect.any(Number), + }); + const positional = z + .array(z.object({ const: z.string() })) + .parse(tuple.prefixItems ?? tuple.items); + const rest = tuple.prefixItems ? tuple.items : tuple.additionalItems; + const ajv = + target === "draft-2020-12" + ? new Ajv2020({ strict: false }) + : new Ajv({ strict: false }); + const validateItem = ajv.compile( + z.record(z.string(), z.unknown()).parse(rest), + ); + for (const value of ["callers", "callees", "incoming", null]) + expect(validateItem(value), JSON.stringify(tuple)).toBe( + positional.some((position) => position.const === value), + ); + } + } + }, + ); +}); diff --git a/src/domain/fixedTupleSchema.ts b/src/domain/fixedTupleSchema.ts index 656c1017cb..7e4b5e138b 100644 --- a/src/domain/fixedTupleSchema.ts +++ b/src/domain/fixedTupleSchema.ts @@ -5,10 +5,10 @@ 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.ZodTuple> => + // Keep the positional schemas and lengths explicit. The union rest schema + // gives clients that only understand `items` useful element guidance; the + // exact maximum length means no rest element can be accepted. z - .tuple(items, z.never()) + .tuple(items, z.union(items)) .check(z.minLength(items.length), z.maxLength(items.length)); diff --git a/src/domain/sqlite/sqliteDatabase.ts b/src/domain/sqlite/sqliteDatabase.ts index 60f3c6d3fa..cb126098e7 100644 --- a/src/domain/sqlite/sqliteDatabase.ts +++ b/src/domain/sqlite/sqliteDatabase.ts @@ -44,7 +44,7 @@ export const inspectSqliteDatabaseInputSchema = z .enum(["inventory", "selected_table", "database"]) .optional() .describe( - "Default: compact inventory without a table, or detailed selected-table schema with a table. Database explicitly expands all stored declarations and supported columns.", + "Default: compact inventory without a table, or detailed selected-table schema with a table. selected_table requires table. Database explicitly expands all stored declarations and supported columns.", ), row_limit: z .number() @@ -55,6 +55,7 @@ export const inspectSqliteDatabaseInputSchema = z "Selected-table sample size, default 20; subject to byte and worker resource limits", ), }) + .describe("When schema_scope is selected_table, also provide table.") .superRefine((input, context) => { if (input.row_limit !== undefined && input.table === undefined) context.addIssue({ diff --git a/tests/boundary/mcp/compactInputSchemaMcp.test.ts b/tests/boundary/mcp/compactInputSchemaMcp.test.ts index 6e473c4711..ff6f18f225 100644 --- a/tests/boundary/mcp/compactInputSchemaMcp.test.ts +++ b/tests/boundary/mcp/compactInputSchemaMcp.test.ts @@ -21,6 +21,7 @@ import { processScenarioSchema } from "../../../src/domain/process/processScenar import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; import { ToolResultDelivery } from "../../../src/server/toolResult.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; +import { arrayItemSchemaError } from "../../fixtures/mcpClientSchemaConstraints.js"; interface AdvertisedTool { readonly name: string; @@ -223,6 +224,7 @@ describe("MCP compact schema constraints", () => { TOOL_CONTRACTS.map((contract) => [contract.name, contract]), ); for (const tool of tools) { + expect(arrayItemSchemaError(tool.inputSchema, tool.name)).toBeUndefined(); expect( ajv.validateSchema(tool.inputSchema), `${tool.name}: ${ajv.errorsText(ajv.errors)}`, @@ -259,6 +261,11 @@ describe("MCP compact schema constraints", () => { expect(sqlite?.inputSchema.description).toContain( "When row_limit is provided, also provide table.", ); + for (const tools of [fullTools, compactTools]) + expect( + tools.find(({ name }) => name === "inspect_sqlite_database") + ?.inputSchema.description, + ).toContain("When schema_scope is selected_table, also provide table."); }); it("retains parameter types and avoids client-side reference expansion", async () => { diff --git a/tests/boundary/mcp/toolInputAdmission.test.ts b/tests/boundary/mcp/toolInputAdmission.test.ts new file mode 100644 index 0000000000..8284ede7c7 --- /dev/null +++ b/tests/boundary/mcp/toolInputAdmission.test.ts @@ -0,0 +1,94 @@ +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import type { CallToolResult } from "@modelcontextprotocol/server"; +import { STDIO_DEFAULT_MAX_BUFFER_SIZE } from "@modelcontextprotocol/server"; +import { describe, expect, it } from "vitest"; + +import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js"; +import { COMPACT_INPUT_SCHEMA_BUDGET_BYTES } from "../../../src/config/mcpInputSchemaProfile.js"; +import { EvidenceMcpServer } from "../../../src/server/EvidenceMcpServer.js"; +import { ToolResultDelivery } from "../../../src/server/toolResult.js"; +import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; + +const createServer = (profile: "full" | "compact"): EvidenceMcpServer => + new EvidenceMcpServer( + { name: "tool-input-admission-test", version: "0" }, + { capabilities: {} }, + undefined, + new ToolResultDelivery(STDIO_DEFAULT_MAX_BUFFER_SIZE), + profile === "compact" + ? { budgetBytes: COMPACT_INPUT_SCHEMA_BUDGET_BYTES } + : undefined, + ); + +// These tests cover the registered SDK boundary with deterministic sentinel +// handlers. They establish canonical rejection before dispatch; they do not +// exercise providers, targets, or the shape of SDK-generated error text. +describe("catalog-wide MCP input admission", () => { + it.each(["full", "compact"] as const)( + "rejects unknown root keys before dispatch with the %s advertised profile", + async (profile) => { + const server = createServer(profile); + const dispatches = new Map(); + for (const contract of TOOL_CONTRACTS) { + server.registerTool( + contract.name, + toolRegistrationOptions(contract), + async (): Promise => { + dispatches.set( + contract.name, + (dispatches.get(contract.name) ?? 0) + 1, + ); + return { + content: [{ type: "text", text: "sentinel handler reached" }], + isError: true, + }; + }, + ); + } + + const client = new Client({ + name: "tool-input-admission-test", + version: "0", + }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await client.connect(clientTransport); + + expect(TOOL_CONTRACTS.length).toBeGreaterThan(0); + for (const contract of TOOL_CONTRACTS) { + const input = { + ...(contract.examples[0]?.input ?? {}), + __rea_unknown_admission_probe__: true, + }; + expect(contract.inputSchema.safeParse(input).success).toBe(false); + + const result = await client.callTool({ + name: contract.name, + arguments: input, + }); + expect(result.isError, contract.name).toBe(true); + expect(dispatches.has(contract.name), contract.name).toBe(false); + } + + const firstWithExample = TOOL_CONTRACTS.find( + (contract) => contract.examples[0] !== undefined, + ); + const liveExample = firstWithExample?.examples[0]; + if (firstWithExample === undefined || liveExample === undefined) + throw new Error( + "No canonical example is available for the liveness check", + ); + const live = await client.callTool({ + name: firstWithExample.name, + arguments: liveExample.input, + }); + expect(live.isError).toBe(true); + expect(dispatches.get(firstWithExample.name)).toBe(1); + } finally { + await Promise.allSettled([client.close(), server.close()]); + } + }, + ); +}); diff --git a/tests/boundary/mcp/toolSchemaValidity.test.ts b/tests/boundary/mcp/toolSchemaValidity.test.ts index ad038e17a3..63c8d7abf8 100644 --- a/tests/boundary/mcp/toolSchemaValidity.test.ts +++ b/tests/boundary/mcp/toolSchemaValidity.test.ts @@ -5,15 +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"; import { GENERATED_MCP_TOOL_CATALOG } from "../../fixtures/mcpToolCatalog.js"; +import { arrayItemSchemaError } from "../../fixtures/mcpClientSchemaConstraints.js"; import { toolRegistrationOptions } from "../../../src/server/toolRegistrationOptions.js"; interface ToolSchemas { @@ -23,37 +19,6 @@ 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. diff --git a/tests/fixtures/mcpClientSchemaConstraints.ts b/tests/fixtures/mcpClientSchemaConstraints.ts new file mode 100644 index 0000000000..3c04c6527a --- /dev/null +++ b/tests/fixtures/mcpClientSchemaConstraints.ts @@ -0,0 +1,38 @@ +import { + schemaArrays, + schemaChildren, + schemaMaps, +} from "../../src/contracts/inputSchemaPresentation.js"; + +const isRecord = (value: unknown): value is Record => + typeof value === "object" && value !== null && !Array.isArray(value); + +/** VS Code Copilot requires truthy items even for Draft 2020-12 tuples. */ +export const arrayItemSchemaError = ( + schema: unknown, + path: string, +): string | undefined => { + if (!isRecord(schema)) return undefined; + 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; +};