Skip to content

Commit 7bfaac5

Browse files
daniel-lxstechstrom
daniel-lxs
authored and
techstrom
committed
fix: force additionalProperties false for strict mode compatibility (RooCodeInc#10210)
1 parent b646f4a commit 7bfaac5

3 files changed

Lines changed: 42 additions & 25 deletions

File tree

‎src/core/prompts/tools/native-tools/__tests__/mcp_server.spec.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,11 +155,12 @@ describe("getMcpServerTools", () => {
155155
const result = getMcpServerTools(mockHub as McpHub)
156156

157157
expect(result).toHaveLength(1)
158+
// additionalProperties: false should only be on the root object type, not on primitive types
158159
expect(getFunction(result[0]).parameters).toEqual({
159160
type: "object",
160161
properties: {
161-
requiredField: { type: "string", additionalProperties: false },
162-
optionalField: { type: "number", additionalProperties: false },
162+
requiredField: { type: "string" },
163+
optionalField: { type: "number" },
163164
},
164165
additionalProperties: false,
165166
required: ["requiredField"],
@@ -183,10 +184,11 @@ describe("getMcpServerTools", () => {
183184
const result = getMcpServerTools(mockHub as McpHub)
184185

185186
expect(result).toHaveLength(1)
187+
// additionalProperties: false should only be on the root object type, not on primitive types
186188
expect(getFunction(result[0]).parameters).toEqual({
187189
type: "object",
188190
properties: {
189-
optionalField: { type: "string", additionalProperties: false },
191+
optionalField: { type: "string" },
190192
},
191193
additionalProperties: false,
192194
})

‎src/utils/__tests__/json-schema.spec.ts‎

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@ describe("normalizeToolSchema", () => {
1010

1111
const result = normalizeToolSchema(input)
1212

13+
// additionalProperties should NOT be added to non-object types (string, null)
1314
expect(result).toEqual({
1415
anyOf: [{ type: "string" }, { type: "null" }],
1516
description: "Optional field",
16-
additionalProperties: false,
1717
})
1818
})
1919

@@ -26,11 +26,11 @@ describe("normalizeToolSchema", () => {
2626

2727
const result = normalizeToolSchema(input)
2828

29+
// additionalProperties should NOT be added to array or primitive types
2930
expect(result).toEqual({
3031
anyOf: [{ type: "array" }, { type: "null" }],
31-
items: { type: "string", additionalProperties: false },
32+
items: { type: "string" },
3233
description: "Optional array",
33-
additionalProperties: false,
3434
})
3535
})
3636

@@ -42,10 +42,10 @@ describe("normalizeToolSchema", () => {
4242

4343
const result = normalizeToolSchema(input)
4444

45+
// additionalProperties should NOT be added to string type
4546
expect(result).toEqual({
4647
type: "string",
4748
description: "Required field",
48-
additionalProperties: false,
4949
})
5050
})
5151

@@ -64,14 +64,14 @@ describe("normalizeToolSchema", () => {
6464

6565
const result = normalizeToolSchema(input)
6666

67+
// additionalProperties: false should ONLY be on the object type, not on primitives
6768
expect(result).toEqual({
6869
type: "object",
6970
properties: {
70-
name: { type: "string", additionalProperties: false },
71+
name: { type: "string" },
7172
optional: {
7273
anyOf: [{ type: "string" }, { type: "null" }],
7374
description: "Optional nested field",
74-
additionalProperties: false,
7575
},
7676
},
7777
required: ["name"],
@@ -96,21 +96,20 @@ describe("normalizeToolSchema", () => {
9696

9797
const result = normalizeToolSchema(input)
9898

99+
// additionalProperties: false should ONLY be on object types
99100
expect(result).toEqual({
100101
type: "array",
101102
items: {
102103
type: "object",
103104
properties: {
104-
path: { type: "string", additionalProperties: false },
105+
path: { type: "string" },
105106
line_ranges: {
106107
anyOf: [{ type: "array" }, { type: "null" }],
107-
items: { type: "integer", additionalProperties: false },
108-
additionalProperties: false,
108+
items: { type: "integer" },
109109
},
110110
},
111111
additionalProperties: false,
112112
},
113-
additionalProperties: false,
114113
})
115114
})
116115

@@ -162,18 +161,18 @@ describe("normalizeToolSchema", () => {
162161

163162
const result = normalizeToolSchema(input)
164163

164+
// additionalProperties: false should ONLY be on object types, not on null or primitive types
165165
expect(result).toEqual({
166166
anyOf: [
167167
{
168168
type: "object",
169169
properties: {
170-
optional: { anyOf: [{ type: "string" }, { type: "null" }], additionalProperties: false },
170+
optional: { anyOf: [{ type: "string" }, { type: "null" }] },
171171
},
172172
additionalProperties: false,
173173
},
174-
{ type: "null", additionalProperties: false },
174+
{ type: "null" },
175175
],
176-
additionalProperties: false,
177176
})
178177
})
179178

@@ -183,7 +182,9 @@ describe("normalizeToolSchema", () => {
183182
expect(normalizeToolSchema(123 as any)).toBe(123)
184183
})
185184

186-
it("should transform additionalProperties when it is a schema object", () => {
185+
it("should force additionalProperties to false for object types even when set to a schema", () => {
186+
// For strict mode compatibility, we MUST force additionalProperties: false
187+
// even when the original schema allowed arbitrary properties
187188
const input = {
188189
type: "object",
189190
additionalProperties: {
@@ -193,13 +194,11 @@ describe("normalizeToolSchema", () => {
193194

194195
const result = normalizeToolSchema(input)
195196

197+
// The original additionalProperties schema is replaced with false for strict mode
196198
expect(result).toEqual({
197199
type: "object",
198200
properties: {},
199-
additionalProperties: {
200-
anyOf: [{ type: "string" }, { type: "null" }],
201-
additionalProperties: false,
202-
},
201+
additionalProperties: false,
203202
})
204203
})
205204

@@ -276,11 +275,11 @@ describe("normalizeToolSchema", () => {
276275

277276
const result = normalizeToolSchema(input)
278277

278+
// additionalProperties should NOT be added to string types
279279
expect(result).toEqual({
280280
type: "string",
281281
format: "date-time",
282282
description: "Timestamp",
283-
additionalProperties: false,
284283
})
285284
})
286285

@@ -335,10 +334,10 @@ describe("normalizeToolSchema", () => {
335334

336335
const result = normalizeToolSchema(input)
337336

337+
// additionalProperties should NOT be added to string types
338338
expect(result).toEqual({
339339
type: "string",
340340
description: "URL field",
341-
additionalProperties: false,
342341
})
343342
expect(result.format).toBeUndefined()
344343
})

‎src/utils/json-schema.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,8 @@ const NormalizedToolSchemaInternal: z.ZodType<Record<string, unknown>, z.ZodType
110110
properties: z.record(z.string(), NormalizedToolSchemaInternal).optional(),
111111
items: z.union([NormalizedToolSchemaInternal, z.array(NormalizedToolSchemaInternal)]).optional(),
112112
required: z.array(z.string()).optional(),
113-
additionalProperties: z.union([z.boolean(), NormalizedToolSchemaInternal]).default(false),
113+
// Don't set default here - we'll handle it conditionally in the transform
114+
additionalProperties: z.union([z.boolean(), NormalizedToolSchemaInternal]).optional(),
114115
description: z.string().optional(),
115116
default: z.unknown().optional(),
116117
enum: z.array(JsonSchemaEnumValueSchema).optional(),
@@ -132,9 +133,13 @@ const NormalizedToolSchemaInternal: z.ZodType<Record<string, unknown>, z.ZodType
132133
})
133134
.passthrough()
134135
.transform((schema) => {
135-
const { type, required, properties, format, ...rest } = schema
136+
const { type, required, properties, additionalProperties, format, ...rest } = schema
136137
const result: Record<string, unknown> = { ...rest }
137138

139+
// Determine if this schema represents an object type
140+
const isObjectType =
141+
type === "object" || (Array.isArray(type) && type.includes("object")) || properties !== undefined
142+
138143
// If type is an array, convert to anyOf format (JSON Schema 2020-12)
139144
if (Array.isArray(type)) {
140145
result.anyOf = type.map((t) => ({ type: t }))
@@ -164,6 +169,17 @@ const NormalizedToolSchemaInternal: z.ZodType<Record<string, unknown>, z.ZodType
164169
result.properties = {}
165170
}
166171

172+
// Only add additionalProperties for object-type schemas
173+
// Adding it to primitive types (string, number, etc.) is invalid JSON Schema
174+
if (isObjectType) {
175+
// For strict mode compatibility, we MUST set additionalProperties to false
176+
// Even if the original schema had {} (any) or true, we force false because
177+
// OpenAI/OpenRouter strict mode rejects schemas with additionalProperties != false
178+
// The original schema intent (allowing arbitrary properties) is incompatible with strict mode
179+
result.additionalProperties = false
180+
}
181+
// For non-object types, don't include additionalProperties at all
182+
167183
return result
168184
}),
169185
)

0 commit comments

Comments
 (0)