diff --git a/clients/web/src/test/core/jsonUtils.test.ts b/clients/web/src/test/core/jsonUtils.test.ts index e71b1e30..6c46a7e6 100644 --- a/clients/web/src/test/core/jsonUtils.test.ts +++ b/clients/web/src/test/core/jsonUtils.test.ts @@ -245,6 +245,42 @@ describe("JSON Utils", () => { }); }); + it("keeps a converted argument named __proto__ (#2123)", () => { + const protoNamed: Tool = { + name: "proto-named", + inputSchema: { + type: "object", + properties: Object.fromEntries([["__proto__", { type: "number" }]]), + }, + }; + const converted = convertToolParameters(protoNamed, { + ["__proto__"]: "3", + }); + expect(Object.hasOwn(converted, "__proto__")).toBe(true); + }); + + it("reads an array type as a set when branches agree (#2123)", () => { + const setTyped: Tool = { + name: "set-typed", + inputSchema: { + type: "object", + anyOf: [ + { + type: "object", + properties: { v: { type: ["number", "null"] }, a: {} }, + }, + { + type: "object", + properties: { v: { type: ["null", "number"] }, b: {} }, + }, + ], + }, + }; + // The same declaration written in the other order — not a disagreement, + // so the coercion survives. + expect(convertToolParameters(setTyped, { v: "2" })).toEqual({ v: 2 }); + }); + it("leaves an ambiguously typed argument uncoerced (#2123)", () => { const ambiguous: Tool = { name: "ambiguous", @@ -293,8 +329,9 @@ describe("JSON Utils", () => { ], }, }; - // Both spell the type the same way, so it is not ambiguous. - expect(convertToolParameters(arrayTyped, { v: "2" })).toEqual({ v: "2" }); + // Both spell the type the same way, so it is not ambiguous — and the + // nullable declaration collapses to `number`, which is what coerces. + expect(convertToolParameters(arrayTyped, { v: "2" })).toEqual({ v: 2 }); }); it("ignores a malformed branch declaration when matching constants (#2123)", () => { diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts index 24592e24..48b4a9bb 100644 --- a/clients/web/src/test/core/nullableUnion.test.ts +++ b/clients/web/src/test/core/nullableUnion.test.ts @@ -774,6 +774,16 @@ describe("admitsNull", () => { expect(admitsNull({ const: null, anyOf: [false] })).toBe(false); }); + it("treats core identifier keywords as asserting nothing", () => { + // `$defs` declares subschemas for reference; it constrains nothing on its + // own, so this branch is unconstrained and admits null. + expect(admitsNull({ const: null, anyOf: [{ $defs: {} }] })).toBe(true); + // `$ref` is different — it applies whatever it points at. + expect(admitsNull({ const: null, anyOf: [{ $ref: "#/$defs/T" }] })).toBe( + false, + ); + }); + it("does not override a sibling that rejects null", () => { // `const` is conjunctive with its siblings, not an override: both of // these reject every value, so claiming nullability would let the diff --git a/core/json/jsonUtils.ts b/core/json/jsonUtils.ts index 147a6180..040917aa 100644 --- a/core/json/jsonUtils.ts +++ b/core/json/jsonUtils.ts @@ -1,4 +1,5 @@ import type { Tool } from "@modelcontextprotocol/client"; +import { normalizeNullableUnion } from "./nullableUnion.js"; import { narrowBySuppliedNames, resolveRootUnion, @@ -220,8 +221,11 @@ function coercionProperties( function typeNameOf(schema: unknown): string { if (typeof schema !== "object" || schema === null) return ""; const { type } = schema as { type?: unknown }; + // Sorted: JSON Schema reads an array `type` as a SET, so `["number","null"]` + // and `["null","number"]` are the same declaration and must not read as a + // disagreement that drops the property from the coercion map. return Array.isArray(type) - ? type.join(",") + ? [...type].map(String).sort().join(",") : typeof type === "string" ? type : ""; @@ -262,7 +266,15 @@ export function convertToolParameters( const { base, branches } = resolveRootUnion(tool.inputSchema ?? {}); const properties = coercionProperties(base, branches, params); for (const [key, value] of Object.entries(params)) { - const paramSchema = properties[key] as ParameterSchema | undefined; + const declared = properties[key]; + // Collapsed first: a nullable declaration (`type: ["number","null"]`, or an + // `anyOf` with a null branch) states its real type on the surviving branch, + // and `convertParameterValue` dispatches on a single `type` string — so + // without this a nullable number is sent as the string it was typed as. + const paramSchema = + typeof declared === "object" && declared !== null + ? (normalizeNullableUnion(declared) as ParameterSchema) + : (declared as ParameterSchema | undefined); // A `const` the supplied text names is sent as the schema's own typed // value, not as the text: a branch pinned to `const: 2` is selected by @@ -270,13 +282,21 @@ export function convertToolParameters( // rejects. Only an exact match is substituted — anything else is the // user's input and is left alone. const pinned = (paramSchema as { const?: unknown } | undefined)?.const; - if (pinned !== undefined && String(pinned) === value) { - result[key] = pinned as JsonValue; - } else if (paramSchema) { - result[key] = convertParameterValue(value, paramSchema); - } else { - result[key] = value; - } + const converted = + pinned !== undefined && String(pinned) === value + ? (pinned as JsonValue) + : paramSchema + ? convertParameterValue(value, paramSchema) + : value; + // `defineProperty`, not assignment: `__proto__` is a legal argument name — + // a discriminator can carry it — and assigning it would invoke the legacy + // prototype setter instead of putting it in the call. + Object.defineProperty(result, key, { + value: converted, + writable: true, + enumerable: true, + configurable: true, + }); } return result; diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts index fd6761dc..9232abdd 100644 --- a/core/json/nullableUnion.ts +++ b/core/json/nullableUnion.ts @@ -440,6 +440,15 @@ const ANNOTATION_ONLY_KEYWORDS = new Set([ "readOnly", "writeOnly", "$comment", + // Core keywords that identify or declare rather than assert. `$ref` is + // deliberately absent: it applies whatever it points at, which is exactly an + // assertion this module cannot see. + "$defs", + "$id", + "$schema", + "$anchor", + "$dynamicAnchor", + "$vocabulary", ]); /** Whether a schema states no assertion at all. */