mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
fix(contracts): preserve array guidance across MCP schema profiles
Keep item schemas when compact inputs reduce nested structure, expose literal tuple elements to items-first renderers, and preserve SQLite table-selection guidance across compaction. Verify catalog-wide client constraints and canonical rejection before dispatch. Document the stock Pi renderer size limit and caller-selected recovery.
This commit is contained in:
1 parent
a732c859dd
commit
204b9f3ac1
10 files changed
+241
-44
No files matched your search
@@ -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`
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
];
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -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<string, unknown>[] = [];
|
||||
const visit = (value: unknown): void => {
|
||||
if (Array.isArray(value)) {
|
||||
value.forEach(visit);
|
||||
} else if (typeof value === "object" && value !== null) {
|
||||
const candidate = value as Record<string, unknown>;
|
||||
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),
|
||||
);
|
||||
}
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -5,10 +5,10 @@ export const fixedTupleSchema = <
|
||||
const Items extends readonly [z.ZodType, ...z.ZodType[]],
|
||||
>(
|
||||
items: Items,
|
||||
): z.ZodTuple<Items, z.ZodNever> =>
|
||||
// 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<Items, z.ZodUnion<Items>> =>
|
||||
// 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));
|
||||
@@ -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({
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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<string, number>();
|
||||
for (const contract of TOOL_CONTRACTS) {
|
||||
server.registerTool(
|
||||
contract.name,
|
||||
toolRegistrationOptions(contract),
|
||||
async (): Promise<CallToolResult> => {
|
||||
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()]);
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -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<string, unknown> | 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.
|
||||
|
||||
+38
@@ -0,0 +1,38 @@
|
||||
import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
} from "../../src/contracts/inputSchemaPresentation.js";
|
||||
|
||||
const isRecord = (value: unknown): value is Record<string, unknown> =>
|
||||
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;
|
||||
};
|
||||
Reference in new issue
Block a user