mirror of
https://github.com/morluto/rea.git
synced 2026-10-18 20:10:13 -05:00
fix(mcp): distinguish schema references from literal metadata
Share schema keyword roles and preserve reference bases for explicit local pointer targets and uncertain intermediate scopes. Retain literal values and canonical validation. Refs #4936.
This commit is contained in:
1 parent
1accac27c3
commit
ba98130049
9 files changed
+336
-40
No files matched your search
@@ -9,7 +9,7 @@ import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
} from "../dist/contracts/inputSchemaPresentation.js";
|
||||
} from "../dist/contracts/jsonSchemaKeywords.js";
|
||||
import {
|
||||
TOOL_CONTRACTS,
|
||||
toolContract,
|
||||
|
||||
@@ -2,7 +2,8 @@ import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
} from "./inputSchemaPresentation.js";
|
||||
schemaLiteralValues,
|
||||
} from "./jsonSchemaKeywords.js";
|
||||
|
||||
// Kimi expands schema-local references before sending tools to Moonshot.
|
||||
// Compact advertisements must fit after expansion, independently of client
|
||||
@@ -11,8 +12,6 @@ import {
|
||||
/** Annotation keys that carry guidance but no validation semantics. */
|
||||
const annotationKeys = new Set(["description", "examples", "default", "title"]);
|
||||
|
||||
/** Keys whose values are literal data, never nested schemas to rewrite. */
|
||||
const dataKeys = new Set(["const", "enum", "examples", "default"]);
|
||||
const definitionKeys = new Set(["$defs", "definitions"]);
|
||||
// An empty schema accepts every JSON value. Kimi otherwise infers string.
|
||||
const jsonValueTypes = [
|
||||
@@ -70,7 +69,7 @@ const rebuildSchemaPositions = (
|
||||
if (!isSchemaObject(node)) return node;
|
||||
const out: Record<string, unknown> = {};
|
||||
for (const [key, value] of Object.entries(node)) {
|
||||
if (dataKeys.has(key)) {
|
||||
if (schemaLiteralValues.has(key)) {
|
||||
out[key] = value;
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -1,27 +1,10 @@
|
||||
import { isDeepStrictEqual } from "node:util";
|
||||
|
||||
export const schemaMaps = new Set([
|
||||
"$defs",
|
||||
"definitions",
|
||||
"properties",
|
||||
"patternProperties",
|
||||
"dependentSchemas",
|
||||
"dependencies",
|
||||
]);
|
||||
export const schemaArrays = new Set(["allOf", "anyOf", "oneOf", "prefixItems"]);
|
||||
export const schemaChildren = new Set([
|
||||
"items",
|
||||
"additionalItems",
|
||||
"contains",
|
||||
"additionalProperties",
|
||||
"unevaluatedProperties",
|
||||
"unevaluatedItems",
|
||||
"propertyNames",
|
||||
"not",
|
||||
"if",
|
||||
"then",
|
||||
"else",
|
||||
]);
|
||||
import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
} from "./jsonSchemaKeywords.js";
|
||||
|
||||
const isObject = (value: unknown): value is Readonly<Record<string, unknown>> =>
|
||||
typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
export const schemaMaps = new Set([
|
||||
"$defs",
|
||||
"definitions",
|
||||
"properties",
|
||||
"patternProperties",
|
||||
"dependentSchemas",
|
||||
"dependencies",
|
||||
]);
|
||||
export const schemaArrays = new Set(["allOf", "anyOf", "oneOf", "prefixItems"]);
|
||||
export const schemaChildren = new Set([
|
||||
"items",
|
||||
"additionalItems",
|
||||
"contains",
|
||||
"additionalProperties",
|
||||
"unevaluatedProperties",
|
||||
"unevaluatedItems",
|
||||
"propertyNames",
|
||||
"not",
|
||||
"if",
|
||||
"then",
|
||||
"else",
|
||||
]);
|
||||
|
||||
/** Instance values, unless explicitly reached by a schema reference. */
|
||||
export const schemaLiteralValues = new Set([
|
||||
"const",
|
||||
"enum",
|
||||
"default",
|
||||
"examples",
|
||||
]);
|
||||
@@ -178,3 +178,152 @@ it.each(["draft-2020-12", "draft-07"] as const)(
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each(
|
||||
(["draft-2020-12", "draft-07"] as const).flatMap((target) =>
|
||||
(["const", "enum", "default", "examples"] as const).map((keyword) => ({
|
||||
target,
|
||||
keyword,
|
||||
})),
|
||||
),
|
||||
)(
|
||||
"keeps literal reference-looking data in $keyword from changing $target schema identity",
|
||||
({ target, keyword }) => {
|
||||
const data = {
|
||||
payload: {
|
||||
$ref: "child.json",
|
||||
$id: "literal-id",
|
||||
$dynamicRef: "other.json",
|
||||
},
|
||||
};
|
||||
const annotation =
|
||||
keyword === "enum" || keyword === "examples" ? [data] : data;
|
||||
const canonical = z
|
||||
.strictObject({ payload: z.record(z.string(), z.string()) })
|
||||
.meta({ [keyword]: annotation });
|
||||
const advertised = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target });
|
||||
expect(advertised.$id).toMatch(
|
||||
/^urn:rea:tool-output:sha256:[a-f0-9]{64}$/u,
|
||||
);
|
||||
expect(Reflect.get(advertised, keyword)).toEqual(annotation);
|
||||
const validate = new AjvJsonSchemaValidator().getValidator(
|
||||
JSON.parse(JSON.stringify(advertised)),
|
||||
);
|
||||
expect(validate(data).valid).toBe(true);
|
||||
expect(validate({ payload: 7 }).valid).toBe(false);
|
||||
expect(validate({ ...data, unexpected: true }).valid).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["draft-2020-12", "draft-07"] as const)(
|
||||
"preserves relative references reached through literal values in %s",
|
||||
(target) => {
|
||||
const canonical = z.looseObject({}).meta({
|
||||
$ref: "#/default/a~1b~0c%20value/0",
|
||||
default: { "a/b~c value": [{ $ref: "child.json" }] },
|
||||
});
|
||||
const projected = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target });
|
||||
expect(projected).not.toHaveProperty("$id");
|
||||
const ajv =
|
||||
target === "draft-2020-12"
|
||||
? new Ajv2020({ strict: false })
|
||||
: new Ajv({ strict: false });
|
||||
ajv.addSchema({
|
||||
$id: "child.json",
|
||||
type: "object",
|
||||
properties: { value: { type: "string", minLength: 3 } },
|
||||
required: ["value"],
|
||||
});
|
||||
const validate = ajv.compile(projected);
|
||||
expect(validate({ value: "abc" })).toBe(true);
|
||||
expect(validate({ value: "ab" })).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["const", "enum", "default", "examples", "$ref", "$id"])(
|
||||
"examines real schemas whose property name is %s",
|
||||
(name) => {
|
||||
const canonical = z.looseObject({}).meta({
|
||||
type: "object",
|
||||
properties: { [name]: { $ref: "child.json" } },
|
||||
});
|
||||
const projected = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target: "draft-2020-12" });
|
||||
expect(projected).not.toHaveProperty("$id");
|
||||
const ajv = new Ajv2020({ strict: false });
|
||||
ajv.addSchema({ $id: "child.json", type: "string", minLength: 3 });
|
||||
const validate = ajv.compile(projected);
|
||||
expect(validate({ [name]: "abc" })).toBe(true);
|
||||
expect(validate({ [name]: "ab" })).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it("resolves local pointers against the containing identified resource", () => {
|
||||
const canonical = z.looseObject({}).meta({
|
||||
type: "object",
|
||||
properties: {
|
||||
value: {
|
||||
$id: "https://example.test/nested.json",
|
||||
$ref: "#/default",
|
||||
default: { $ref: "child.json" },
|
||||
},
|
||||
},
|
||||
});
|
||||
const projected = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target: "draft-2020-12" });
|
||||
expect(projected).not.toHaveProperty("$id");
|
||||
expect(projected.properties).toEqual(canonical.meta()?.properties);
|
||||
});
|
||||
|
||||
it.each(["#/missing", "#/%XX", "#/default/bad~2key"])(
|
||||
"keeps the existing base when a local pointer cannot be resolved: %s",
|
||||
($ref) => {
|
||||
const canonical = z
|
||||
.looseObject({})
|
||||
.meta({ $ref, default: { "bad~2key": {} } });
|
||||
const projected = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target: "draft-2020-12" });
|
||||
expect(projected).not.toHaveProperty("$id");
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps reference discovery conservative for unknown schema vocabulary", () => {
|
||||
const canonical = z
|
||||
.looseObject({})
|
||||
.meta({ "x-schema": { $ref: "child.json" } });
|
||||
const projected = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target: "draft-2020-12" });
|
||||
expect(projected).not.toHaveProperty("$id");
|
||||
});
|
||||
|
||||
it.each(["relative/", "https://example.test/nested.json"])(
|
||||
"preserves the base when a pointer crosses an intermediate $id: %s",
|
||||
($id) => {
|
||||
const canonical = z.looseObject({}).meta({
|
||||
$ref: "#/default/inner/target",
|
||||
default: {
|
||||
inner: { $id, target: { $ref: "#/other" }, other: { type: "object" } },
|
||||
},
|
||||
other: {},
|
||||
});
|
||||
const projected = toolOutputSchemaWithMetadata({
|
||||
...toolContract("binary_session"),
|
||||
outputSchema: canonical,
|
||||
})["~standard"].jsonSchema.output({ target: "draft-2020-12" });
|
||||
expect(projected).not.toHaveProperty("$id");
|
||||
},
|
||||
);
|
||||
@@ -3,6 +3,12 @@ import { z } from "zod";
|
||||
import type { ToolContract } from "./toolContractTypes.js";
|
||||
import { advertisedInputExamples } from "./advertisedInputExamples.js";
|
||||
import { presentInputJsonSchema } from "./inputSchemaPresentation.js";
|
||||
import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
schemaLiteralValues,
|
||||
} from "./jsonSchemaKeywords.js";
|
||||
import { digestCanonicalValue } from "../domain/canonicalDigest.js";
|
||||
|
||||
const PROPERTY_DESCRIPTIONS: Readonly<Record<string, string>> = {
|
||||
@@ -151,30 +157,111 @@ export const transformAdvertisedInputJsonSchema = <Schema>(
|
||||
} as Schema;
|
||||
};
|
||||
|
||||
type SchemaPosition = "schema" | "map" | "unknown";
|
||||
|
||||
const hasRelativeSchemaReference = (
|
||||
schema: Record<string, unknown>,
|
||||
): boolean => {
|
||||
const pending: unknown[] = [schema];
|
||||
const visited = new WeakSet<object>();
|
||||
// Local pointers can make even annotation values executable schemas. Keep
|
||||
// their resource root and traversal role separate from ordinary instance data.
|
||||
const pending: { value: unknown; root: object; position: SchemaPosition }[] =
|
||||
[{ value: schema, root: schema, position: "schema" }];
|
||||
const visited = new WeakMap<
|
||||
object,
|
||||
Record<SchemaPosition, WeakSet<object>>
|
||||
>();
|
||||
while (pending.length > 0) {
|
||||
const current = pending.pop();
|
||||
if (current === null || typeof current !== "object" || visited.has(current))
|
||||
const entry = pending.pop()!;
|
||||
const current = entry.value;
|
||||
if (current === null || typeof current !== "object") continue;
|
||||
const { position } = entry;
|
||||
const root =
|
||||
position !== "map" &&
|
||||
isSchemaRecord(current) &&
|
||||
typeof current.$id === "string" &&
|
||||
URL.canParse(current.$id)
|
||||
? current
|
||||
: entry.root;
|
||||
let positions = visited.get(root);
|
||||
if (positions === undefined) {
|
||||
positions = {
|
||||
schema: new WeakSet(),
|
||||
map: new WeakSet(),
|
||||
unknown: new WeakSet(),
|
||||
};
|
||||
visited.set(root, positions);
|
||||
}
|
||||
if (positions[position].has(current)) continue;
|
||||
positions[position].add(current);
|
||||
if (Array.isArray(current)) {
|
||||
for (const value of current) pending.push({ value, root, position });
|
||||
continue;
|
||||
visited.add(current);
|
||||
}
|
||||
for (const [key, value] of Object.entries(current)) {
|
||||
if (
|
||||
position !== "map" &&
|
||||
["$id", "$ref", "$dynamicRef", "$recursiveRef"].includes(key) &&
|
||||
typeof value === "string" &&
|
||||
!value.startsWith("#") &&
|
||||
!URL.canParse(value)
|
||||
)
|
||||
return true;
|
||||
if (value !== null && typeof value === "object") pending.push(value);
|
||||
typeof value === "string"
|
||||
) {
|
||||
if (!value.startsWith("#") && !URL.canParse(value)) return true;
|
||||
if (key !== "$id" && value.startsWith("#")) {
|
||||
const target = localSchemaReference(root, value);
|
||||
// Unknown anchors are already examined at their schema positions.
|
||||
// Unresolvable pointers retain the existing reference base.
|
||||
if (target === undefined) return true;
|
||||
if (target.kind === "target")
|
||||
pending.push({ value: target.value, root, position: "schema" });
|
||||
}
|
||||
}
|
||||
if (position === "schema" && schemaLiteralValues.has(key)) continue;
|
||||
const childPosition =
|
||||
position === "map"
|
||||
? "schema"
|
||||
: position === "schema" && schemaMaps.has(key)
|
||||
? "map"
|
||||
: position === "schema" &&
|
||||
(schemaArrays.has(key) || schemaChildren.has(key))
|
||||
? "schema"
|
||||
: "unknown";
|
||||
if (value !== null && typeof value === "object")
|
||||
pending.push({ value, root, position: childPosition });
|
||||
}
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
/** Decode URI-fragment JSON Pointers without interpreting literal data as schemas. */
|
||||
const localSchemaReference = (
|
||||
root: object,
|
||||
reference: string,
|
||||
): { kind: "target"; value: unknown } | { kind: "anchor" } | undefined => {
|
||||
let fragment: string;
|
||||
try {
|
||||
fragment = decodeURIComponent(reference.slice(1));
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
if (fragment === "") return { kind: "target", value: root };
|
||||
if (!fragment.startsWith("/")) return { kind: "anchor" };
|
||||
let value: unknown = root;
|
||||
for (const token of fragment.slice(1).split("/")) {
|
||||
if (/~(?:[^01]|$)/u.test(token)) return undefined;
|
||||
const key = token.replaceAll("~1", "/").replaceAll("~0", "~");
|
||||
if (
|
||||
value === null ||
|
||||
typeof value !== "object" ||
|
||||
!Object.hasOwn(value, key)
|
||||
)
|
||||
return undefined;
|
||||
// A pointer can cross a resource boundary even inside literal data.
|
||||
// Preserve the original projection rather than guess the reference scope.
|
||||
if (value !== root && isSchemaRecord(value) && "$id" in value)
|
||||
return undefined;
|
||||
value = Reflect.get(value, key);
|
||||
}
|
||||
return { kind: "target", value };
|
||||
};
|
||||
|
||||
const withAdvertisedJsonSchema = <Schema extends z.ZodType>(
|
||||
canonical: Schema,
|
||||
io: "input" | "output",
|
||||
|
||||
@@ -16,7 +16,7 @@ import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
} from "../../../src/contracts/inputSchemaPresentation.js";
|
||||
} from "../../../src/contracts/jsonSchemaKeywords.js";
|
||||
import {
|
||||
COMPACT_INPUT_SCHEMA_BUDGET_BYTES,
|
||||
parseMcpInputSchemaProfile,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
|
||||
import { AjvJsonSchemaValidator } from "@modelcontextprotocol/client/validators/ajv";
|
||||
import { McpServer } from "@modelcontextprotocol/server";
|
||||
import { expect, it } from "vitest";
|
||||
import { z } from "zod";
|
||||
@@ -106,3 +107,50 @@ it("uses the new server's schema after reconnecting the client", async () => {
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it("advertises stable output identity without interpreting example data as references", async () => {
|
||||
const client = new Client({ name: "literal-output-test", version: "1" });
|
||||
const server = new McpServer({
|
||||
name: "literal-output-fixture",
|
||||
version: "1",
|
||||
});
|
||||
const value = { payload: { $ref: "child.json", $id: "example-only" } };
|
||||
const schema = advertised(
|
||||
z
|
||||
.strictObject({
|
||||
payload: z.strictObject({ $ref: z.string(), $id: z.string() }),
|
||||
})
|
||||
.meta({ examples: [value], default: value }),
|
||||
);
|
||||
server.registerTool(
|
||||
"literal_output",
|
||||
{
|
||||
inputSchema: z.strictObject({}),
|
||||
outputSchema: schema,
|
||||
},
|
||||
() => ({ content: [], structuredContent: value }),
|
||||
);
|
||||
try {
|
||||
await connect(server, client);
|
||||
const first = await client.listTools();
|
||||
const second = await client.listTools();
|
||||
const output = first.tools.find(
|
||||
(tool) => tool.name === "literal_output",
|
||||
)?.outputSchema;
|
||||
expect(output?.$id).toMatch(/^urn:rea:tool-output:sha256:[a-f0-9]{64}$/u);
|
||||
expect(second.tools[0]?.outputSchema).toEqual(output);
|
||||
expect(output?.examples).toEqual([value]);
|
||||
expect(output?.default).toEqual(value);
|
||||
expect(
|
||||
(await client.callTool({ name: "literal_output" })).structuredContent,
|
||||
).toEqual(value);
|
||||
expect(output).toBeDefined();
|
||||
const validate = new AjvJsonSchemaValidator().getValidator(
|
||||
z.record(z.string(), z.unknown()).parse(output),
|
||||
);
|
||||
expect(validate({ payload: 7 }).valid).toBe(false);
|
||||
expect(validate({ ...value, unexpected: true }).valid).toBe(false);
|
||||
} finally {
|
||||
await Promise.allSettled([client.close(), server.close()]);
|
||||
}
|
||||
});
|
||||
+1
-1
@@ -2,7 +2,7 @@ import {
|
||||
schemaArrays,
|
||||
schemaChildren,
|
||||
schemaMaps,
|
||||
} from "../../src/contracts/inputSchemaPresentation.js";
|
||||
} from "../../src/contracts/jsonSchemaKeywords.js";
|
||||
|
||||
const isRecord = (value: unknown): value is Record<string, unknown> =>
|
||||
typeof value === "object" && value !== null && !Array.isArray(value);
|
||||
|
||||
Reference in new issue
Block a user