Browse Source

fix: force additionalProperties false for strict mode compatibility (#10210)

Daniel 3 weeks ago
parent
commit
aabee0fb0b

+ 5 - 3
src/core/prompts/tools/native-tools/__tests__/mcp_server.spec.ts

@@ -155,11 +155,12 @@ describe("getMcpServerTools", () => {
 		const result = getMcpServerTools(mockHub as McpHub)
 
 		expect(result).toHaveLength(1)
+		// additionalProperties: false should only be on the root object type, not on primitive types
 		expect(getFunction(result[0]).parameters).toEqual({
 			type: "object",
 			properties: {
-				requiredField: { type: "string", additionalProperties: false },
-				optionalField: { type: "number", additionalProperties: false },
+				requiredField: { type: "string" },
+				optionalField: { type: "number" },
 			},
 			additionalProperties: false,
 			required: ["requiredField"],
@@ -183,10 +184,11 @@ describe("getMcpServerTools", () => {
 		const result = getMcpServerTools(mockHub as McpHub)
 
 		expect(result).toHaveLength(1)
+		// additionalProperties: false should only be on the root object type, not on primitive types
 		expect(getFunction(result[0]).parameters).toEqual({
 			type: "object",
 			properties: {
-				optionalField: { type: "string", additionalProperties: false },
+				optionalField: { type: "string" },
 			},
 			additionalProperties: false,
 		})

+ 19 - 20
src/utils/__tests__/json-schema.spec.ts

@@ -10,10 +10,10 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// additionalProperties should NOT be added to non-object types (string, null)
 		expect(result).toEqual({
 			anyOf: [{ type: "string" }, { type: "null" }],
 			description: "Optional field",
-			additionalProperties: false,
 		})
 	})
 
@@ -26,11 +26,11 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// additionalProperties should NOT be added to array or primitive types
 		expect(result).toEqual({
 			anyOf: [{ type: "array" }, { type: "null" }],
-			items: { type: "string", additionalProperties: false },
+			items: { type: "string" },
 			description: "Optional array",
-			additionalProperties: false,
 		})
 	})
 
@@ -42,10 +42,10 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// additionalProperties should NOT be added to string type
 		expect(result).toEqual({
 			type: "string",
 			description: "Required field",
-			additionalProperties: false,
 		})
 	})
 
@@ -64,14 +64,14 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// additionalProperties: false should ONLY be on the object type, not on primitives
 		expect(result).toEqual({
 			type: "object",
 			properties: {
-				name: { type: "string", additionalProperties: false },
+				name: { type: "string" },
 				optional: {
 					anyOf: [{ type: "string" }, { type: "null" }],
 					description: "Optional nested field",
-					additionalProperties: false,
 				},
 			},
 			required: ["name"],
@@ -96,21 +96,20 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// additionalProperties: false should ONLY be on object types
 		expect(result).toEqual({
 			type: "array",
 			items: {
 				type: "object",
 				properties: {
-					path: { type: "string", additionalProperties: false },
+					path: { type: "string" },
 					line_ranges: {
 						anyOf: [{ type: "array" }, { type: "null" }],
-						items: { type: "integer", additionalProperties: false },
-						additionalProperties: false,
+						items: { type: "integer" },
 					},
 				},
 				additionalProperties: false,
 			},
-			additionalProperties: false,
 		})
 	})
 
@@ -162,18 +161,18 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// additionalProperties: false should ONLY be on object types, not on null or primitive types
 		expect(result).toEqual({
 			anyOf: [
 				{
 					type: "object",
 					properties: {
-						optional: { anyOf: [{ type: "string" }, { type: "null" }], additionalProperties: false },
+						optional: { anyOf: [{ type: "string" }, { type: "null" }] },
 					},
 					additionalProperties: false,
 				},
-				{ type: "null", additionalProperties: false },
+				{ type: "null" },
 			],
-			additionalProperties: false,
 		})
 	})
 
@@ -183,7 +182,9 @@ describe("normalizeToolSchema", () => {
 		expect(normalizeToolSchema(123 as any)).toBe(123)
 	})
 
-	it("should transform additionalProperties when it is a schema object", () => {
+	it("should force additionalProperties to false for object types even when set to a schema", () => {
+		// For strict mode compatibility, we MUST force additionalProperties: false
+		// even when the original schema allowed arbitrary properties
 		const input = {
 			type: "object",
 			additionalProperties: {
@@ -193,13 +194,11 @@ describe("normalizeToolSchema", () => {
 
 		const result = normalizeToolSchema(input)
 
+		// The original additionalProperties schema is replaced with false for strict mode
 		expect(result).toEqual({
 			type: "object",
 			properties: {},
-			additionalProperties: {
-				anyOf: [{ type: "string" }, { type: "null" }],
-				additionalProperties: false,
-			},
+			additionalProperties: false,
 		})
 	})
 
@@ -276,11 +275,11 @@ describe("normalizeToolSchema", () => {
 
 			const result = normalizeToolSchema(input)
 
+			// additionalProperties should NOT be added to string types
 			expect(result).toEqual({
 				type: "string",
 				format: "date-time",
 				description: "Timestamp",
-				additionalProperties: false,
 			})
 		})
 
@@ -335,10 +334,10 @@ describe("normalizeToolSchema", () => {
 
 			const result = normalizeToolSchema(input)
 
+			// additionalProperties should NOT be added to string types
 			expect(result).toEqual({
 				type: "string",
 				description: "URL field",
-				additionalProperties: false,
 			})
 			expect(result.format).toBeUndefined()
 		})

+ 18 - 2
src/utils/json-schema.ts

@@ -110,7 +110,8 @@ const NormalizedToolSchemaInternal: z.ZodType<Record<string, unknown>, z.ZodType
 				properties: z.record(z.string(), NormalizedToolSchemaInternal).optional(),
 				items: z.union([NormalizedToolSchemaInternal, z.array(NormalizedToolSchemaInternal)]).optional(),
 				required: z.array(z.string()).optional(),
-				additionalProperties: z.union([z.boolean(), NormalizedToolSchemaInternal]).default(false),
+				// Don't set default here - we'll handle it conditionally in the transform
+				additionalProperties: z.union([z.boolean(), NormalizedToolSchemaInternal]).optional(),
 				description: z.string().optional(),
 				default: z.unknown().optional(),
 				enum: z.array(JsonSchemaEnumValueSchema).optional(),
@@ -132,9 +133,13 @@ const NormalizedToolSchemaInternal: z.ZodType<Record<string, unknown>, z.ZodType
 			})
 			.passthrough()
 			.transform((schema) => {
-				const { type, required, properties, format, ...rest } = schema
+				const { type, required, properties, additionalProperties, format, ...rest } = schema
 				const result: Record<string, unknown> = { ...rest }
 
+				// Determine if this schema represents an object type
+				const isObjectType =
+					type === "object" || (Array.isArray(type) && type.includes("object")) || properties !== undefined
+
 				// If type is an array, convert to anyOf format (JSON Schema 2020-12)
 				if (Array.isArray(type)) {
 					result.anyOf = type.map((t) => ({ type: t }))
@@ -164,6 +169,17 @@ const NormalizedToolSchemaInternal: z.ZodType<Record<string, unknown>, z.ZodType
 					result.properties = {}
 				}
 
+				// Only add additionalProperties for object-type schemas
+				// Adding it to primitive types (string, number, etc.) is invalid JSON Schema
+				if (isObjectType) {
+					// For strict mode compatibility, we MUST set additionalProperties to false
+					// Even if the original schema had {} (any) or true, we force false because
+					// OpenAI/OpenRouter strict mode rejects schemas with additionalProperties != false
+					// The original schema intent (allowing arbitrary properties) is incompatible with strict mode
+					result.additionalProperties = false
+				}
+				// For non-object types, don't include additionalProperties at all
+
 				return result
 			}),
 )