mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: add additionalProperties: false to nested MCP tool schemas (#10109)
This commit is contained in:
parent
596783d365
commit
93bc8c4f1d
6 changed files with 298 additions and 26 deletions
13
pnpm-lock.yaml
generated
13
pnpm-lock.yaml
generated
|
|
@ -853,7 +853,7 @@ importers:
|
|||
specifier: ^2.8.0
|
||||
version: 2.8.0
|
||||
zod:
|
||||
specifier: ^3.25.61
|
||||
specifier: 3.25.61
|
||||
version: 3.25.61
|
||||
devDependencies:
|
||||
'@roo-code/build':
|
||||
|
|
@ -7842,6 +7842,7 @@ packages:
|
|||
next@15.2.6:
|
||||
resolution: {integrity: sha512-DIKFctUpZoCq5ok2ztVU+PqhWsbiqM9xNP7rHL2cAp29NQcmDp7Y6JnBBhHRbFt4bCsCZigj6uh+/Gwh2158Wg==}
|
||||
engines: {node: ^18.18.0 || ^19.8.0 || >= 20.0.0}
|
||||
deprecated: This version has a security vulnerability. Please upgrade to a patched version. See https://nextjs.org/blog/security-update-2025-12-11 for more details.
|
||||
hasBin: true
|
||||
peerDependencies:
|
||||
'@opentelemetry/api': ^1.1.0
|
||||
|
|
@ -14131,7 +14132,7 @@ snapshots:
|
|||
sirv: 3.0.1
|
||||
tinyglobby: 0.2.14
|
||||
tinyrainbow: 2.0.0
|
||||
vitest: 3.2.4(@types/debug@4.1.12)(@types/node@20.17.50)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0)
|
||||
vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.2.1)(@vitest/ui@3.2.4)(jiti@2.4.2)(jsdom@26.1.0)(lightningcss@1.30.1)(tsx@4.19.4)(yaml@2.8.0)
|
||||
|
||||
'@vitest/utils@3.2.4':
|
||||
dependencies:
|
||||
|
|
@ -17263,8 +17264,8 @@ snapshots:
|
|||
smol-toml: 1.3.4
|
||||
strip-json-comments: 5.0.2
|
||||
typescript: 5.8.3
|
||||
zod: 3.25.61
|
||||
zod-validation-error: 3.4.1(zod@3.25.61)
|
||||
zod: 3.25.76
|
||||
zod-validation-error: 3.4.1(zod@3.25.76)
|
||||
|
||||
knuth-shuffle-seeded@1.0.6:
|
||||
dependencies:
|
||||
|
|
@ -21215,9 +21216,9 @@ snapshots:
|
|||
typescript: 5.8.3
|
||||
zod: 3.25.61
|
||||
|
||||
zod-validation-error@3.4.1(zod@3.25.61):
|
||||
zod-validation-error@3.4.1(zod@3.25.76):
|
||||
dependencies:
|
||||
zod: 3.25.61
|
||||
zod: 3.25.76
|
||||
|
||||
zod@3.23.8: {}
|
||||
|
||||
|
|
|
|||
|
|
@ -158,8 +158,8 @@ describe("getMcpServerTools", () => {
|
|||
expect(getFunction(result[0]).parameters).toEqual({
|
||||
type: "object",
|
||||
properties: {
|
||||
requiredField: { type: "string" },
|
||||
optionalField: { type: "number" },
|
||||
requiredField: { type: "string", additionalProperties: false },
|
||||
optionalField: { type: "number", additionalProperties: false },
|
||||
},
|
||||
additionalProperties: false,
|
||||
required: ["requiredField"],
|
||||
|
|
@ -186,7 +186,7 @@ describe("getMcpServerTools", () => {
|
|||
expect(getFunction(result[0]).parameters).toEqual({
|
||||
type: "object",
|
||||
properties: {
|
||||
optionalField: { type: "string" },
|
||||
optionalField: { type: "string", additionalProperties: false },
|
||||
},
|
||||
additionalProperties: false,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import type OpenAI from "openai"
|
||||
import { McpHub } from "../../../../services/mcp/McpHub"
|
||||
import { buildMcpToolName } from "../../../../utils/mcp-name"
|
||||
import { ToolInputSchema, type JsonSchema } from "../../../../utils/json-schema"
|
||||
|
||||
/**
|
||||
* Dynamically generates native tool definitions for all enabled tools across connected MCP servers.
|
||||
|
|
@ -40,22 +41,16 @@ export function getMcpServerTools(mcpHub?: McpHub): OpenAI.Chat.ChatCompletionTo
|
|||
}
|
||||
seenToolNames.add(toolName)
|
||||
|
||||
const originalSchema = tool.inputSchema as Record<string, any> | undefined
|
||||
const toolInputProps = originalSchema?.properties ?? {}
|
||||
const toolInputRequired = (originalSchema?.required ?? []) as string[]
|
||||
const originalSchema = tool.inputSchema as Record<string, unknown> | undefined
|
||||
|
||||
// Build parameters directly from the tool's input schema.
|
||||
// The server_name and tool_name are encoded in the function name itself
|
||||
// (e.g., mcp_serverName_toolName), so they don't need to be in the arguments.
|
||||
const parameters: OpenAI.FunctionParameters = {
|
||||
type: "object",
|
||||
properties: toolInputProps,
|
||||
additionalProperties: false,
|
||||
}
|
||||
|
||||
// Only add required if there are required fields
|
||||
if (toolInputRequired.length > 0) {
|
||||
parameters.required = toolInputRequired
|
||||
// Parse with ToolInputSchema to ensure additionalProperties: false is set recursively
|
||||
let parameters: JsonSchema
|
||||
if (originalSchema) {
|
||||
const result = ToolInputSchema.safeParse(originalSchema)
|
||||
parameters = result.success ? result.data : (originalSchema as JsonSchema)
|
||||
} else {
|
||||
// No schema provided - create a minimal valid schema
|
||||
parameters = ToolInputSchema.parse({ type: "object" })
|
||||
}
|
||||
|
||||
const toolDefinition: OpenAI.Chat.ChatCompletionTool = {
|
||||
|
|
@ -63,7 +58,7 @@ export function getMcpServerTools(mcpHub?: McpHub): OpenAI.Chat.ChatCompletionTo
|
|||
function: {
|
||||
name: toolName,
|
||||
description: tool.description,
|
||||
parameters: parameters,
|
||||
parameters: parameters as OpenAI.FunctionParameters,
|
||||
},
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -507,7 +507,7 @@
|
|||
"web-tree-sitter": "^0.25.6",
|
||||
"workerpool": "^9.2.0",
|
||||
"yaml": "^2.8.0",
|
||||
"zod": "^3.25.61"
|
||||
"zod": "3.25.61"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@roo-code/build": "workspace:^",
|
||||
|
|
|
|||
213
src/utils/__tests__/json-schema.spec.ts
Normal file
213
src/utils/__tests__/json-schema.spec.ts
Normal file
|
|
@ -0,0 +1,213 @@
|
|||
import { ToolInputSchema } from "../json-schema"
|
||||
|
||||
describe("ToolInputSchema", () => {
|
||||
it("should validate and default additionalProperties to false", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string" },
|
||||
},
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect(result.type).toBe("object")
|
||||
expect(result.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should recursively apply defaults to nested schemas", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
user: {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string" },
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect(result.additionalProperties).toBe(false)
|
||||
expect((result.properties as any).user.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should apply defaults to object schemas in array items", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
items: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: {
|
||||
id: { type: "number" },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect(result.additionalProperties).toBe(false)
|
||||
expect((result.properties as any).items.items.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should throw on invalid schema", () => {
|
||||
const invalidSchema = { type: "invalid-type" }
|
||||
|
||||
expect(() => ToolInputSchema.parse(invalidSchema)).toThrow()
|
||||
})
|
||||
|
||||
it("should use safeParse for error handling", () => {
|
||||
const invalidSchema = { type: "invalid-type" }
|
||||
|
||||
const result = ToolInputSchema.safeParse(invalidSchema)
|
||||
|
||||
expect(result.success).toBe(false)
|
||||
})
|
||||
|
||||
it("should apply defaults in anyOf schemas", () => {
|
||||
const schema = {
|
||||
anyOf: [{ type: "object", properties: { a: { type: "string" } } }, { type: "string" }],
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect((result.anyOf as any)[0].additionalProperties).toBe(false)
|
||||
expect((result.anyOf as any)[1].additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should apply defaults in oneOf schemas", () => {
|
||||
const schema = {
|
||||
oneOf: [{ type: "object", properties: { a: { type: "string" } } }, { type: "number" }],
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect((result.oneOf as any)[0].additionalProperties).toBe(false)
|
||||
expect((result.oneOf as any)[1].additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should apply defaults in allOf schemas", () => {
|
||||
const schema = {
|
||||
allOf: [
|
||||
{ type: "object", properties: { a: { type: "string" } } },
|
||||
{ type: "object", properties: { b: { type: "number" } } },
|
||||
],
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect((result.allOf as any)[0].additionalProperties).toBe(false)
|
||||
expect((result.allOf as any)[1].additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should apply defaults to tuple-style array items", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
tuple: {
|
||||
type: "array",
|
||||
items: [
|
||||
{ type: "object", properties: { a: { type: "string" } } },
|
||||
{ type: "object", properties: { b: { type: "number" } } },
|
||||
],
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
const tupleItems = (result.properties as any).tuple.items
|
||||
expect(tupleItems[0].additionalProperties).toBe(false)
|
||||
expect(tupleItems[1].additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should preserve explicit additionalProperties: false", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string" },
|
||||
},
|
||||
additionalProperties: false,
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect(result.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should handle deeply nested complex schemas", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
level1: {
|
||||
type: "object",
|
||||
properties: {
|
||||
level2: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: {
|
||||
level3: {
|
||||
type: "object",
|
||||
properties: {
|
||||
value: { type: "string" },
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
expect(result.additionalProperties).toBe(false)
|
||||
expect((result.properties as any).level1.additionalProperties).toBe(false)
|
||||
expect((result.properties as any).level1.properties.level2.items.additionalProperties).toBe(false)
|
||||
expect((result.properties as any).level1.properties.level2.items.properties.level3.additionalProperties).toBe(
|
||||
false,
|
||||
)
|
||||
})
|
||||
|
||||
it("should handle the real-world MCP memory create_entities schema", () => {
|
||||
// This is based on the actual schema that caused the OpenAI error
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
entities: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string", description: "The name of the entity" },
|
||||
entityType: { type: "string", description: "The type of the entity" },
|
||||
observations: {
|
||||
type: "array",
|
||||
items: { type: "string" },
|
||||
description: "An array of observation contents",
|
||||
},
|
||||
},
|
||||
required: ["name", "entityType", "observations"],
|
||||
},
|
||||
description: "An array of entities to create",
|
||||
},
|
||||
},
|
||||
required: ["entities"],
|
||||
}
|
||||
|
||||
const result = ToolInputSchema.parse(schema)
|
||||
|
||||
// Top-level object should have additionalProperties: false
|
||||
expect(result.additionalProperties).toBe(false)
|
||||
// Items in the entities array should have additionalProperties: false
|
||||
expect((result.properties as any).entities.items.additionalProperties).toBe(false)
|
||||
})
|
||||
})
|
||||
63
src/utils/json-schema.ts
Normal file
63
src/utils/json-schema.ts
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
import type { z as z4 } from "zod/v4"
|
||||
import { z } from "zod"
|
||||
|
||||
/**
|
||||
* Re-export Zod v4's JSONSchema type for convenience
|
||||
*/
|
||||
export type JsonSchema = z4.core.JSONSchema.JSONSchema
|
||||
|
||||
/**
|
||||
* Zod schema for JSON Schema primitive types
|
||||
*/
|
||||
const JsonSchemaPrimitiveTypeSchema = z.enum(["string", "number", "integer", "boolean", "null"])
|
||||
|
||||
/**
|
||||
* Zod schema for JSON Schema enum values
|
||||
*/
|
||||
const JsonSchemaEnumValueSchema = z.union([z.string(), z.number(), z.boolean(), z.null()])
|
||||
|
||||
/**
|
||||
* Zod schema that validates tool input JSON Schema and sets `additionalProperties: false` by default.
|
||||
* Uses recursive parsing so the default applies to all nested schemas automatically.
|
||||
*
|
||||
* This is required by some API providers (e.g., OpenAI) for strict function calling.
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* // Validates and applies defaults in one pass - throws on invalid
|
||||
* const validatedSchema = ToolInputSchema.parse(schema)
|
||||
*
|
||||
* // Or use safeParse for error handling
|
||||
* const result = ToolInputSchema.safeParse(schema)
|
||||
* if (result.success) {
|
||||
* // result.data has additionalProperties: false by default
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
export const ToolInputSchema: z.ZodType<JsonSchema> = z.lazy(() =>
|
||||
z
|
||||
.object({
|
||||
type: z.union([JsonSchemaPrimitiveTypeSchema, z.literal("object"), z.literal("array")]).optional(),
|
||||
properties: z.record(z.string(), ToolInputSchema).optional(),
|
||||
items: z.union([ToolInputSchema, z.array(ToolInputSchema)]).optional(),
|
||||
required: z.array(z.string()).optional(),
|
||||
additionalProperties: z.union([z.boolean(), ToolInputSchema]).default(false),
|
||||
description: z.string().optional(),
|
||||
default: z.unknown().optional(),
|
||||
enum: z.array(JsonSchemaEnumValueSchema).optional(),
|
||||
const: JsonSchemaEnumValueSchema.optional(),
|
||||
anyOf: z.array(ToolInputSchema).optional(),
|
||||
oneOf: z.array(ToolInputSchema).optional(),
|
||||
allOf: z.array(ToolInputSchema).optional(),
|
||||
$ref: z.string().optional(),
|
||||
minimum: z.number().optional(),
|
||||
maximum: z.number().optional(),
|
||||
minLength: z.number().optional(),
|
||||
maxLength: z.number().optional(),
|
||||
pattern: z.string().optional(),
|
||||
minItems: z.number().optional(),
|
||||
maxItems: z.number().optional(),
|
||||
uniqueItems: z.boolean().optional(),
|
||||
})
|
||||
.passthrough(),
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue