From 1cfe4e84ac530708f5e826f81abcaeb87abcc249 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Mon, 15 Dec 2025 12:54:38 -0500 Subject: [PATCH] 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) --- src/services/mcp/McpHub.ts | 3 + src/utils/__tests__/schema.spec.ts | 221 +++++++++++++++++++++++++++++ src/utils/schema.ts | 92 ++++++++++++ 3 files changed, 316 insertions(+) create mode 100644 src/utils/__tests__/schema.spec.ts create mode 100644 src/utils/schema.ts diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 3d54cb670e..8ad07487c1 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -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), alwaysAllow: alwaysAllowConfig.includes(tool.name), enabledForPrompt: !disabledToolsList.includes(tool.name), })) diff --git a/src/utils/__tests__/schema.spec.ts b/src/utils/__tests__/schema.spec.ts new file mode 100644 index 0000000000..c09e50f701 --- /dev/null +++ b/src/utils/__tests__/schema.spec.ts @@ -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 + 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 + 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[] + 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 + 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 + 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"]) + }) +}) diff --git a/src/utils/schema.ts b/src/utils/schema.ts new file mode 100644 index 0000000000..75e2227b32 --- /dev/null +++ b/src/utils/schema.ts @@ -0,0 +1,92 @@ +/** + * Utility functions for JSON Schema manipulation. + */ + +type JsonSchemaObject = Record + +/** + * 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 + const normalizedProperties: Record = {} + + 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 + const normalizedDefinitions: Record = {} + + for (const [key, value] of Object.entries(definitions)) { + normalizedDefinitions[key] = normalizeSchemaForStrictMode(value) ?? {} + } + + result.definitions = normalizedDefinitions + } + + if (result.$defs) { + const defs = result.$defs as Record + const normalizedDefs: Record = {} + + for (const [key, value] of Object.entries(defs)) { + normalizedDefs[key] = normalizeSchemaForStrictMode(value) ?? {} + } + + result.$defs = normalizedDefs + } + + return result +}