mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: normalize MCP tool schemas for OpenAI strict mode
OpenAI's strict mode function calling requires additionalProperties: false on all object types in the schema, including nested objects inside arrays. This fix normalizes schemas at the source when MCP tools are fetched in McpHub.fetchToolsList(), rather than doing post-processing during conversion to OpenAI format. This approach: - Normalizes schemas once when fetched, not on every API call - Benefits all consumers of MCP tools, not just OpenAI format conversion - Is cleaner and more maintainable Added normalizeSchemaForStrictMode utility that handles: - Nested object properties - Array items (single schema and tuple validation) - Schema combinators (anyOf, allOf, oneOf) - Conditional schemas (if/then/else) - Schema definitions (definitions and $defs)
This commit is contained in:
parent
1d4fc52485
commit
1cfe4e84ac
3 changed files with 316 additions and 0 deletions
|
|
@ -34,6 +34,7 @@ import { arePathsEqual, getWorkspacePath } from "../../utils/path"
|
|||
import { injectVariables } from "../../utils/config"
|
||||
import { safeWriteJson } from "../../utils/safeWriteJson"
|
||||
import { sanitizeMcpName } from "../../utils/mcp-name"
|
||||
import { normalizeSchemaForStrictMode } from "../../utils/schema"
|
||||
|
||||
// Discriminated union for connection states
|
||||
export type ConnectedMcpConnection = {
|
||||
|
|
@ -976,8 +977,10 @@ export class McpHub {
|
|||
}
|
||||
|
||||
// Mark tools as always allowed and enabled for prompt based on settings
|
||||
// Also normalize inputSchema to ensure OpenAI strict mode compatibility
|
||||
const tools = (response?.tools || []).map((tool) => ({
|
||||
...tool,
|
||||
inputSchema: normalizeSchemaForStrictMode(tool.inputSchema as Record<string, unknown>),
|
||||
alwaysAllow: alwaysAllowConfig.includes(tool.name),
|
||||
enabledForPrompt: !disabledToolsList.includes(tool.name),
|
||||
}))
|
||||
|
|
|
|||
221
src/utils/__tests__/schema.spec.ts
Normal file
221
src/utils/__tests__/schema.spec.ts
Normal file
|
|
@ -0,0 +1,221 @@
|
|||
import { normalizeSchemaForStrictMode } from "../schema"
|
||||
|
||||
describe("normalizeSchemaForStrictMode", () => {
|
||||
it("should return undefined for undefined input", () => {
|
||||
expect(normalizeSchemaForStrictMode(undefined)).toBeUndefined()
|
||||
})
|
||||
|
||||
it("should return non-object values unchanged", () => {
|
||||
expect(normalizeSchemaForStrictMode(null as any)).toBeNull()
|
||||
expect(normalizeSchemaForStrictMode("string" as any)).toBe("string")
|
||||
})
|
||||
|
||||
it("should add additionalProperties: false to object types with properties", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string" },
|
||||
},
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect(result?.additionalProperties).toBe(false)
|
||||
expect(result?.properties).toEqual({ name: { type: "string" } })
|
||||
})
|
||||
|
||||
it("should not add additionalProperties to object types without properties", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect(result?.additionalProperties).toBeUndefined()
|
||||
})
|
||||
|
||||
it("should recursively process nested object properties", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
nested: {
|
||||
type: "object",
|
||||
properties: {
|
||||
value: { type: "string" },
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect(result?.additionalProperties).toBe(false)
|
||||
const nestedSchema = result?.properties as Record<string, any>
|
||||
expect(nestedSchema.nested.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should process object types inside array items", () => {
|
||||
const schema = {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string" },
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
const itemsSchema = result?.items as Record<string, any>
|
||||
expect(itemsSchema.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should process array items when items is an array (tuple validation)", () => {
|
||||
const schema = {
|
||||
type: "array",
|
||||
items: [
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
first: { type: "string" },
|
||||
},
|
||||
},
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
second: { type: "number" },
|
||||
},
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
const itemsArray = result?.items as Record<string, any>[]
|
||||
expect(itemsArray[0].additionalProperties).toBe(false)
|
||||
expect(itemsArray[1].additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should process anyOf, allOf, and oneOf combinators", () => {
|
||||
const schema = {
|
||||
anyOf: [{ type: "object", properties: { a: { type: "string" } } }],
|
||||
allOf: [{ type: "object", properties: { b: { type: "string" } } }],
|
||||
oneOf: [{ type: "object", properties: { c: { type: "string" } } }],
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect((result?.anyOf as any[])[0].additionalProperties).toBe(false)
|
||||
expect((result?.allOf as any[])[0].additionalProperties).toBe(false)
|
||||
expect((result?.oneOf as any[])[0].additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should process conditional schemas (if/then/else)", () => {
|
||||
const schema = {
|
||||
if: { type: "object", properties: { condition: { type: "boolean" } } },
|
||||
then: { type: "object", properties: { thenValue: { type: "string" } } },
|
||||
else: { type: "object", properties: { elseValue: { type: "string" } } },
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect((result?.if as any).additionalProperties).toBe(false)
|
||||
expect((result?.then as any).additionalProperties).toBe(false)
|
||||
expect((result?.else as any).additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should process definitions and $defs", () => {
|
||||
const schema = {
|
||||
definitions: {
|
||||
Entity: { type: "object", properties: { id: { type: "string" } } },
|
||||
},
|
||||
$defs: {
|
||||
Item: { type: "object", properties: { name: { type: "string" } } },
|
||||
},
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect((result?.definitions as any).Entity.additionalProperties).toBe(false)
|
||||
expect((result?.$defs as any).Item.additionalProperties).toBe(false)
|
||||
})
|
||||
|
||||
it("should handle complex nested schema like MCP memory create_entities", () => {
|
||||
// This is a schema similar to what caused the original error
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
entities: {
|
||||
type: "array",
|
||||
items: {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string" },
|
||||
entityType: { type: "string" },
|
||||
observations: {
|
||||
type: "array",
|
||||
items: { type: "string" },
|
||||
},
|
||||
},
|
||||
required: ["name", "entityType", "observations"],
|
||||
},
|
||||
},
|
||||
},
|
||||
required: ["entities"],
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
// Top level should have additionalProperties: false
|
||||
expect(result?.additionalProperties).toBe(false)
|
||||
|
||||
// The object inside the array items should also have additionalProperties: false
|
||||
const propertiesSchema = result?.properties as Record<string, any>
|
||||
expect(propertiesSchema.entities.items.additionalProperties).toBe(false)
|
||||
|
||||
// Required arrays should be preserved
|
||||
expect(result?.required).toEqual(["entities"])
|
||||
expect(propertiesSchema.entities.items.required).toEqual(["name", "entityType", "observations"])
|
||||
})
|
||||
|
||||
it("should not mutate the original schema", () => {
|
||||
const original = {
|
||||
type: "object",
|
||||
properties: {
|
||||
nested: {
|
||||
type: "object",
|
||||
properties: {
|
||||
value: { type: "string" },
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
const originalJson = JSON.stringify(original)
|
||||
|
||||
normalizeSchemaForStrictMode(original)
|
||||
|
||||
expect(JSON.stringify(original)).toBe(originalJson)
|
||||
})
|
||||
|
||||
it("should preserve non-object properties", () => {
|
||||
const schema = {
|
||||
type: "object",
|
||||
properties: {
|
||||
name: { type: "string", description: "The name", minLength: 1 },
|
||||
age: { type: "integer", minimum: 0, maximum: 150 },
|
||||
},
|
||||
required: ["name"],
|
||||
}
|
||||
|
||||
const result = normalizeSchemaForStrictMode(schema)
|
||||
|
||||
expect(result?.additionalProperties).toBe(false)
|
||||
const props = result?.properties as Record<string, any>
|
||||
expect(props.name.description).toBe("The name")
|
||||
expect(props.name.minLength).toBe(1)
|
||||
expect(props.age.minimum).toBe(0)
|
||||
expect(props.age.maximum).toBe(150)
|
||||
expect(result?.required).toEqual(["name"])
|
||||
})
|
||||
})
|
||||
92
src/utils/schema.ts
Normal file
92
src/utils/schema.ts
Normal file
|
|
@ -0,0 +1,92 @@
|
|||
/**
|
||||
* Utility functions for JSON Schema manipulation.
|
||||
*/
|
||||
|
||||
type JsonSchemaObject = Record<string, unknown>
|
||||
|
||||
/**
|
||||
* Recursively adds `additionalProperties: false` to all object types in a JSON Schema.
|
||||
*
|
||||
* OpenAI's strict mode function calling requires `additionalProperties: false` on all
|
||||
* object types in the schema, including nested objects inside arrays, combinators, etc.
|
||||
*
|
||||
* @param schema - The JSON Schema object to normalize
|
||||
* @returns A new schema with `additionalProperties: false` added to all object types
|
||||
*/
|
||||
export function normalizeSchemaForStrictMode(schema: JsonSchemaObject | undefined): JsonSchemaObject | undefined {
|
||||
if (!schema || typeof schema !== "object") {
|
||||
return schema
|
||||
}
|
||||
|
||||
const result: JsonSchemaObject = { ...schema }
|
||||
|
||||
// Add additionalProperties: false to object types with properties
|
||||
if (result.type === "object" && result.properties) {
|
||||
result.additionalProperties = false
|
||||
|
||||
// Recursively process each property
|
||||
const properties = result.properties as Record<string, JsonSchemaObject>
|
||||
const normalizedProperties: Record<string, JsonSchemaObject> = {}
|
||||
|
||||
for (const [key, value] of Object.entries(properties)) {
|
||||
normalizedProperties[key] = normalizeSchemaForStrictMode(value) ?? {}
|
||||
}
|
||||
|
||||
result.properties = normalizedProperties
|
||||
}
|
||||
|
||||
// Handle array items
|
||||
if (result.items) {
|
||||
if (Array.isArray(result.items)) {
|
||||
result.items = result.items.map((item) => normalizeSchemaForStrictMode(item as JsonSchemaObject) ?? {})
|
||||
} else {
|
||||
result.items = normalizeSchemaForStrictMode(result.items as JsonSchemaObject)
|
||||
}
|
||||
}
|
||||
|
||||
// Handle combinators (anyOf, allOf, oneOf)
|
||||
for (const combinator of ["anyOf", "allOf", "oneOf"] as const) {
|
||||
const combinatorValue = result[combinator]
|
||||
if (Array.isArray(combinatorValue)) {
|
||||
result[combinator] = combinatorValue.map(
|
||||
(subSchema) => normalizeSchemaForStrictMode(subSchema as JsonSchemaObject) ?? {},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Handle conditional schemas (if/then/else)
|
||||
if (result.if) {
|
||||
result.if = normalizeSchemaForStrictMode(result.if as JsonSchemaObject)
|
||||
}
|
||||
if (result.then) {
|
||||
result.then = normalizeSchemaForStrictMode(result.then as JsonSchemaObject)
|
||||
}
|
||||
if (result.else) {
|
||||
result.else = normalizeSchemaForStrictMode(result.else as JsonSchemaObject)
|
||||
}
|
||||
|
||||
// Handle definitions/$defs
|
||||
if (result.definitions) {
|
||||
const definitions = result.definitions as Record<string, JsonSchemaObject>
|
||||
const normalizedDefinitions: Record<string, JsonSchemaObject> = {}
|
||||
|
||||
for (const [key, value] of Object.entries(definitions)) {
|
||||
normalizedDefinitions[key] = normalizeSchemaForStrictMode(value) ?? {}
|
||||
}
|
||||
|
||||
result.definitions = normalizedDefinitions
|
||||
}
|
||||
|
||||
if (result.$defs) {
|
||||
const defs = result.$defs as Record<string, JsonSchemaObject>
|
||||
const normalizedDefs: Record<string, JsonSchemaObject> = {}
|
||||
|
||||
for (const [key, value] of Object.entries(defs)) {
|
||||
normalizedDefs[key] = normalizeSchemaForStrictMode(value) ?? {}
|
||||
}
|
||||
|
||||
result.$defs = normalizedDefs
|
||||
}
|
||||
|
||||
return result
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue