mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
fix(mcp): preserve nested JSON values in compact array schemas
Give array unions an unrestricted item schema and validate converted tools/list schemas for both advertised profiles. Keep client constraints separate from JSON Schema validity. Refs: #3838, #3921, #3842 Co-authored-by: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Co-authored-by: 冰水泡枸杞 <xingzihai@users.noreply.github.com>
This commit is contained in:
1 parent
4fd28fc9e0
commit
b3389a5b08
4 files changed
+197
-120
No files matched your search
@@ -191,10 +191,19 @@ describe("compact advertised input schema", () => {
|
||||
const properties = schema.properties as Record<string, unknown>;
|
||||
expect(properties.normalized_result).toEqual({
|
||||
type: ["null", "boolean", "object", "array", "number", "string"],
|
||||
items: {},
|
||||
});
|
||||
expect(properties.literal).toEqual({ const: {} });
|
||||
const validate = new Ajv2020({ strict: false }).compile(schema);
|
||||
for (const normalized_result of [null, true, {}, [], 42, "result"])
|
||||
for (const normalized_result of [
|
||||
null,
|
||||
true,
|
||||
{},
|
||||
[],
|
||||
42,
|
||||
"result",
|
||||
[[null, true, 42, "nested"], { values: [[], [false]] }],
|
||||
])
|
||||
expect(validate({ normalized_result })).toBe(true);
|
||||
});
|
||||
|
||||
@@ -267,12 +276,20 @@ describe("compact input schema budget and reduction", () => {
|
||||
type: "array",
|
||||
items: {
|
||||
type: ["null", "boolean", "object", "array", "number", "string"],
|
||||
items: {},
|
||||
},
|
||||
},
|
||||
label: { type: ["string", "null"] },
|
||||
});
|
||||
const validate = new Ajv2020({ strict: false }).compile(schema);
|
||||
expect(validate({ capture: {}, observations: [], label: null })).toBe(true);
|
||||
expect(
|
||||
validate({
|
||||
capture: {},
|
||||
observations: [[[null, true, 42, "nested"], { values: [[], [false]] }]],
|
||||
label: null,
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(validate({ capture: {}, observations: [], label: "capture" })).toBe(
|
||||
true,
|
||||
);
|
||||
|
||||
@@ -23,6 +23,9 @@ const jsonValueTypes = [
|
||||
"number",
|
||||
"string",
|
||||
];
|
||||
// An empty item schema accepts every JSON value, including arbitrarily nested
|
||||
// arrays, while satisfying clients that require `items` for array types.
|
||||
const jsonValueSchema = () => ({ type: [...jsonValueTypes], items: {} });
|
||||
|
||||
export type AdvertisedInputSchemaPresentation = "inline" | "reduced";
|
||||
|
||||
@@ -157,7 +160,7 @@ const inlineWithinBudget = (
|
||||
yield "]";
|
||||
} else if (isSchemaObject(value)) {
|
||||
if (position === "schema" && Object.keys(value).length === 0) {
|
||||
yield JSON.stringify({ type: jsonValueTypes });
|
||||
yield JSON.stringify(jsonValueSchema());
|
||||
return;
|
||||
}
|
||||
if (position === "schema" && typeof value.$ref === "string") {
|
||||
@@ -293,9 +296,7 @@ const reduceToShallowProperties = (
|
||||
...(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 } }
|
||||
: {}),
|
||||
...(types?.includes("array") ? { items: jsonValueSchema() } : {}),
|
||||
...description,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -2,8 +2,12 @@ import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
|
||||
import { STDIO_DEFAULT_MAX_BUFFER_SIZE } from "@modelcontextprotocol/server";
|
||||
import { Ajv2020 } from "ajv/dist/2020.js";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { z } from "zod";
|
||||
|
||||
import { TOOL_CONTRACTS } from "../../../src/contracts/toolContracts.js";
|
||||
import {
|
||||
TOOL_CONTRACTS,
|
||||
toolContract,
|
||||
} from "../../../src/contracts/toolContracts.js";
|
||||
import { PROMPT_CONTRACTS } from "../../../src/contracts/promptContracts.js";
|
||||
import { advertisedInputJsonSchema } from "../../../src/contracts/toolSchemaMetadata.js";
|
||||
import { presentAdvertisedInputSchema } from "../../../src/contracts/compactInputSchema.js";
|
||||
@@ -178,48 +182,96 @@ describe("MCP compact input schema profile", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCP compact schema constraints", () => {
|
||||
it("advertises every tool input schema within the provider budget", 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([]);
|
||||
});
|
||||
it("advertises every tool input schema within the provider budget", 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([]);
|
||||
});
|
||||
|
||||
it("keeps output schemas and tool descriptions identical to the default profile", async () => {
|
||||
const compact = compactServer();
|
||||
const full = fullServer();
|
||||
registerCatalog(compact);
|
||||
registerCatalog(full);
|
||||
const [compactTools, fullTools] = await Promise.all([
|
||||
listToolsThroughTransport(compact),
|
||||
listToolsThroughTransport(full),
|
||||
]);
|
||||
const fullByName = new Map(fullTools.map((tool) => [tool.name, tool]));
|
||||
expect(compactTools.length).toBe(fullTools.length);
|
||||
for (const tool of compactTools) {
|
||||
const reference = fullByName.get(tool.name);
|
||||
if (reference === undefined) throw new Error(`Missing ${tool.name}`);
|
||||
expect(tool.outputSchema, tool.name).toEqual(reference.outputSchema);
|
||||
expect(tool.description, tool.name).toBe(reference.description);
|
||||
}
|
||||
});
|
||||
it("keeps output schemas and tool descriptions identical to the default profile", async () => {
|
||||
const compact = compactServer();
|
||||
const full = fullServer();
|
||||
registerCatalog(compact);
|
||||
registerCatalog(full);
|
||||
const [compactTools, fullTools] = await Promise.all([
|
||||
listToolsThroughTransport(compact),
|
||||
listToolsThroughTransport(full),
|
||||
]);
|
||||
const fullByName = new Map(fullTools.map((tool) => [tool.name, tool]));
|
||||
expect(compactTools.length).toBe(fullTools.length);
|
||||
for (const tool of compactTools) {
|
||||
const reference = fullByName.get(tool.name);
|
||||
if (reference === undefined) throw new Error(`Missing ${tool.name}`);
|
||||
expect(tool.outputSchema, tool.name).toEqual(reference.outputSchema);
|
||||
expect(tool.description, tool.name).toBe(reference.description);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps compact schemas valid and accepting canonical examples", async () => {
|
||||
const server = compactServer();
|
||||
it("detects missing client-required items in nested array alternatives", () => {
|
||||
const malformed = {
|
||||
type: "object",
|
||||
properties: {
|
||||
payload: {
|
||||
type: ["null", "array"],
|
||||
items: { type: ["string", "array"] },
|
||||
},
|
||||
},
|
||||
};
|
||||
expect(arrayItemSchemaError(malformed, "injected")).toBe(
|
||||
"injected.properties.payload.items: tool parameters array type must have items",
|
||||
);
|
||||
// `items: false` is valid JSON Schema, but the targeted Copilot
|
||||
// normalizer rejects it because it requires a truthy items schema.
|
||||
expect(arrayItemSchemaError({ type: "array", items: false }, "closed")).toBe(
|
||||
"closed: tool parameters array type must have items",
|
||||
);
|
||||
});
|
||||
|
||||
it("preserves arbitrary nested JSON values at the converted SDK boundary", async () => {
|
||||
const contract = {
|
||||
...toolContract("binary_session"),
|
||||
inputSchema: z.strictObject({ value: z.unknown() }),
|
||||
};
|
||||
const value = {
|
||||
deeply_nested: [[[[null, true, 7, "text"]]], { mixed: [[], [false]] }],
|
||||
};
|
||||
for (const profile of ["full", "compact"] as const) {
|
||||
const server = profile === "full" ? fullServer() : compactServer();
|
||||
server.registerTool(
|
||||
contract.name,
|
||||
toolRegistrationOptions(contract),
|
||||
async () => ({ content: [] }),
|
||||
);
|
||||
const tool = (await listToolsThroughTransport(server)).find(
|
||||
({ name }) => name === contract.name,
|
||||
);
|
||||
if (tool === undefined) throw new Error("Probe tool was not advertised");
|
||||
const validate = new Ajv2020({
|
||||
strict: false,
|
||||
validateFormats: false,
|
||||
}).compile(tool.inputSchema);
|
||||
expect(contract.inputSchema.safeParse({ value }).success).toBe(true);
|
||||
expect(validate({ value }), `${profile} profile`).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps SDK-converted schemas valid and accepting canonical examples", async () => {
|
||||
for (const profile of ["full", "compact"] as const) {
|
||||
const server = profile === "full" ? fullServer() : compactServer();
|
||||
registerCatalog(server);
|
||||
const tools = await listToolsThroughTransport(server);
|
||||
const ajv = new Ajv2020({ strict: false, validateFormats: false });
|
||||
@@ -227,95 +279,99 @@ 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(
|
||||
arrayItemSchemaError(tool.inputSchema, tool.name),
|
||||
`${profile} profile: ${tool.name}`,
|
||||
).toBeUndefined();
|
||||
expect(
|
||||
booleanLiteralSchemaError(tool.inputSchema, tool.name),
|
||||
).toBeUndefined();
|
||||
expect(
|
||||
ajv.validateSchema(tool.inputSchema),
|
||||
`${tool.name}: ${ajv.errorsText(ajv.errors)}`,
|
||||
`${profile} profile, ${tool.name}: ${ajv.errorsText(ajv.errors)}`,
|
||||
).toBe(true);
|
||||
const contract = contractsByName.get(tool.name);
|
||||
const validate = ajv.compile(tool.inputSchema);
|
||||
for (const example of contract?.examples ?? [])
|
||||
expect(validate(example.input), `${tool.name}: ${example.title}`).toBe(
|
||||
true,
|
||||
);
|
||||
expect(
|
||||
validate(example.input),
|
||||
`${profile} profile, ${tool.name}: ${example.title}`,
|
||||
).toBe(true);
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("omits Gemini-unsupported dependency keywords from both catalog profiles", async () => {
|
||||
const full = fullServer();
|
||||
const compact = compactServer();
|
||||
registerCatalog(full);
|
||||
registerCatalog(compact);
|
||||
const [fullTools, compactTools] = await Promise.all([
|
||||
listToolsThroughTransport(full),
|
||||
listToolsThroughTransport(compact),
|
||||
]);
|
||||
for (const tools of [fullTools, compactTools])
|
||||
expect(
|
||||
tools.flatMap(({ name, inputSchema }) =>
|
||||
unsupportedDependencyKeywords(inputSchema).map(
|
||||
(path) => `${name}${path.slice(1)}`,
|
||||
),
|
||||
it("omits Gemini-unsupported dependency keywords from both catalog profiles", async () => {
|
||||
const full = fullServer();
|
||||
const compact = compactServer();
|
||||
registerCatalog(full);
|
||||
registerCatalog(compact);
|
||||
const [fullTools, compactTools] = await Promise.all([
|
||||
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",
|
||||
);
|
||||
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.");
|
||||
});
|
||||
),
|
||||
).toEqual([]);
|
||||
const sqlite = fullTools.find(
|
||||
({ name }) => name === "inspect_sqlite_database",
|
||||
);
|
||||
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 () => {
|
||||
const server = compactServer();
|
||||
registerCatalog(server);
|
||||
const tools = await listToolsThroughTransport(server);
|
||||
for (const tool of tools) {
|
||||
// Kimi Code 2.1.1 expands references and defaults description-only
|
||||
// properties to string. Inline schemas with explicit shallow types
|
||||
// avoid both conversions for the structurally oversized tools.
|
||||
expect(JSON.stringify(tool.inputSchema), tool.name).not.toContain(
|
||||
'"$ref":',
|
||||
);
|
||||
}
|
||||
const comparison = tools.find(
|
||||
({ name }) => name === "compare_managed_members",
|
||||
it("retains parameter types and avoids client-side reference expansion", async () => {
|
||||
const server = compactServer();
|
||||
registerCatalog(server);
|
||||
const tools = await listToolsThroughTransport(server);
|
||||
for (const tool of tools) {
|
||||
// Kimi Code 2.1.1 expands references and defaults description-only
|
||||
// properties to string. Inline schemas with explicit shallow types
|
||||
// avoid both conversions for the structurally oversized tools.
|
||||
expect(JSON.stringify(tool.inputSchema), tool.name).not.toContain(
|
||||
'"$ref":',
|
||||
);
|
||||
expect(comparison?.inputSchema.properties).toMatchObject({
|
||||
left: { type: "object" },
|
||||
});
|
||||
const ledger = tools.find(
|
||||
({ name }) => name === "build_reconstruction_obligation_ledger",
|
||||
);
|
||||
expect(ledger?.inputSchema.properties).toMatchObject({
|
||||
evidence_bundle: { type: "object" },
|
||||
reviewed_obligations: { type: "array" },
|
||||
manifest: { type: "object" },
|
||||
});
|
||||
const coverage = tools.find(
|
||||
({ name }) => name === "evaluate_reconstruction_coverage",
|
||||
);
|
||||
expect(coverage?.inputSchema.properties).toMatchObject({
|
||||
coverage: { type: "object" },
|
||||
boundary_id: { type: "string" },
|
||||
});
|
||||
}
|
||||
const comparison = tools.find(
|
||||
({ name }) => name === "compare_managed_members",
|
||||
);
|
||||
expect(comparison?.inputSchema.properties).toMatchObject({
|
||||
left: { type: "object" },
|
||||
});
|
||||
const ledger = tools.find(
|
||||
({ name }) => name === "build_reconstruction_obligation_ledger",
|
||||
);
|
||||
expect(ledger?.inputSchema.properties).toMatchObject({
|
||||
evidence_bundle: { type: "object" },
|
||||
reviewed_obligations: { type: "array" },
|
||||
manifest: { type: "object" },
|
||||
});
|
||||
const coverage = tools.find(
|
||||
({ name }) => name === "evaluate_reconstruction_coverage",
|
||||
);
|
||||
expect(coverage?.inputSchema.properties).toMatchObject({
|
||||
coverage: { type: "object" },
|
||||
boundary_id: { type: "string" },
|
||||
});
|
||||
});
|
||||
|
||||
it("parses the profile selection the server startup reads", () => {
|
||||
expect(parseMcpInputSchemaProfile("compact")).toEqual({
|
||||
ok: true,
|
||||
value: "compact",
|
||||
});
|
||||
expect(parseMcpInputSchemaProfile("surprise")).not.toHaveProperty("value");
|
||||
it("parses the profile selection the server startup reads", () => {
|
||||
expect(parseMcpInputSchemaProfile("compact")).toEqual({
|
||||
ok: true,
|
||||
value: "compact",
|
||||
});
|
||||
expect(parseMcpInputSchemaProfile("surprise")).not.toHaveProperty("value");
|
||||
});
|
||||
|
||||
describe("MCP compact input schema canonical enforcement", () => {
|
||||
|
||||
+5
-2
@@ -7,13 +7,16 @@ import {
|
||||
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. */
|
||||
/** VS Code Copilot rejects falsy `items` for any type that admits arrays. */
|
||||
export const arrayItemSchemaError = (
|
||||
schema: unknown,
|
||||
path: string,
|
||||
): string | undefined => {
|
||||
if (!isRecord(schema)) return undefined;
|
||||
if (schema.type === "array" && !schema.items)
|
||||
const admitsArrays =
|
||||
schema.type === "array" ||
|
||||
(Array.isArray(schema.type) && schema.type.includes("array"));
|
||||
if (admitsArrays && !schema.items)
|
||||
return `${path}: tool parameters array type must have items`;
|
||||
for (const [key, value] of Object.entries(schema)) {
|
||||
const children =
|
||||
|
||||
Reference in new issue
Block a user