From 3353b7e3ab03b30663a04f3578e7f7d527d29a08 Mon Sep 17 00:00:00 2001 From: Jesse Quijano Date: Thu, 12 Nov 2026 08:09:32 -0800 Subject: [PATCH 1/3] fix(providers): remove unsupported keywords from strict tool schemas Some gateways send strict OpenAI tool schemas to Anthropic strict tool use. Anthropic strict mode rejects some keywords that OpenAI strict mode accepts, for example maxItems. The gateway then rejects the full request with "For 'array' type, property 'maxItems' is not supported". - Add stripUnsupportedStrictKeywords(). It removes unsupported constraints from strict schemas and adds them to the description. - Call it only on the strict path in convertToolsForOpenAI(). MCP tools and non-strict tools keep their declared schemas. - Remove maxItems from ask_followup_question and minimum from edit_file. The descriptions state the limits. - Limit follow-up suggestions to four at runtime. - Add a test that keeps all native tool schemas in the portable subset. Fixes #869 --- .../providers/__tests__/base-provider.spec.ts | 233 +++++++++++++++++- src/api/providers/__tests__/openai.spec.ts | 49 ++++ src/api/providers/base-provider.ts | 3 +- src/api/transform/strict-tool-schema.ts | 109 ++++++++ .../__tests__/strict-schema-subset.spec.ts | 62 +++++ .../native-tools/ask_followup_question.ts | 3 +- .../prompts/tools/native-tools/edit_file.ts | 3 +- src/core/tools/AskFollowupQuestionTool.ts | 5 +- .../__tests__/askFollowupQuestionTool.spec.ts | 14 ++ 9 files changed, 473 insertions(+), 8 deletions(-) create mode 100644 src/api/transform/strict-tool-schema.ts create mode 100644 src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts diff --git a/src/api/providers/__tests__/base-provider.spec.ts b/src/api/providers/__tests__/base-provider.spec.ts index da1f449c38..755cc1660d 100644 --- a/src/api/providers/__tests__/base-provider.spec.ts +++ b/src/api/providers/__tests__/base-provider.spec.ts @@ -338,6 +338,227 @@ describe("BaseProvider", () => { expect(result?.[0].function.parameters.required).toEqual(["path"]) }) + it("removes unsupported constraints from the reported follow-up schema", () => { + const tools = [ + { + type: "function", + function: { + name: "ask_followup_question", + description: "Ask a question", + parameters: { + type: "object", + properties: { + follow_up: { + type: "array", + description: "Required list of 2-4 suggested responses", + items: { type: "object", properties: { text: { type: "string" } } }, + minItems: 1, + maxItems: 4, + }, + }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + const followUp = result?.[0].function.parameters.properties.follow_up + + expect(result?.[0].function.strict).toBe(true) + expect(followUp.minItems).toBe(1) + expect(followUp.maxItems).toBeUndefined() + expect(followUp.description).toBe('Required list of 2-4 suggested responses\n\nConstraints: {"maxItems":4}') + }) + + it("removes unsupported constraints recursively while keeping supported keywords", () => { + const tools = [ + { + type: "function", + function: { + name: "constraint_tool", + parameters: { + type: "object", + properties: { + count: { type: "number", minimum: 1, maximum: 9, multipleOf: 1 }, + name: { + type: "string", + minLength: 1, + maxLength: 5, + pattern: "^a", + format: "email", + }, + unsupportedFormat: { type: "string", format: "phone" }, + list: { + type: "array", + items: { type: "integer", exclusiveMinimum: 0 }, + minItems: 2, + uniqueItems: true, + contains: { type: "integer", minimum: 1 }, + }, + choice: { anyOf: [{ type: "string", maxLength: 3 }, { type: "null" }] }, + }, + $defs: { bounded: { type: "integer", maximum: 3 } }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + const parameters = result?.[0].function.parameters + + expect(parameters.properties.count).toEqual({ + type: "number", + description: 'Constraints: {"minimum":1,"maximum":9,"multipleOf":1}', + }) + expect(parameters.properties.name).toMatchObject({ type: "string", pattern: "^a", format: "email" }) + expect(parameters.properties.unsupportedFormat.format).toBeUndefined() + expect(parameters.properties.list.minItems).toBeUndefined() + expect(parameters.properties.list.uniqueItems).toBeUndefined() + expect(parameters.properties.list.contains).toBeUndefined() + expect(parameters.properties.list.items.exclusiveMinimum).toBeUndefined() + expect(parameters.properties.choice.anyOf[0].maxLength).toBeUndefined() + expect(parameters.$defs.bounded.maximum).toBeUndefined() + }) + + it.each([0, 1])("keeps supported minItems value %s", (minItems) => { + const tools = [ + { + type: "function", + function: { + name: "list_tool", + parameters: { + type: "object", + properties: { values: { type: "array", items: { type: "string" }, minItems } }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + expect(result?.[0].function.parameters.properties.values.minItems).toBe(minItems) + }) + + it("processes tuple items and nested schema keywords", () => { + const tools = [ + { + type: "function", + function: { + name: "nested_schema_tool", + parameters: { + type: "object", + properties: { + values: { + type: "array", + items: [ + { type: "string", maxLength: 2 }, + { type: "integer", minimum: 0 }, + ], + }, + }, + additionalProperties: { type: "string", minLength: 1 }, + not: { type: "number", maximum: 10 }, + if: { type: "number", minimum: 1 }, + then: { type: "string", maxLength: 5 }, + else: { type: "string", minLength: 2 }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + const parameters = result?.[0].function.parameters + + expect(parameters.properties.values.items[0].maxLength).toBeUndefined() + expect(parameters.properties.values.items[1].minimum).toBeUndefined() + expect(parameters.additionalProperties).toBe(false) + expect(parameters.not.maximum).toBeUndefined() + expect(parameters.if.minimum).toBeUndefined() + expect(parameters.then.maxLength).toBeUndefined() + expect(parameters.else.minLength).toBeUndefined() + }) + + it("removes additionalProperties values other than false in nested schemas", () => { + const tools = [ + { + type: "function", + function: { + name: "composed_object_tool", + parameters: { + type: "object", + properties: { + options: { + anyOf: [ + { type: "object", properties: {}, additionalProperties: true }, + { type: "object", properties: {}, additionalProperties: { type: "string" } }, + ], + }, + }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + const variants = result?.[0].function.parameters.properties.options.anyOf + + expect(variants[0].additionalProperties).toBeUndefined() + expect(variants[1].additionalProperties).toBeUndefined() + expect(variants[0].description).toBe('Constraints: {"additionalProperties":true}') + expect(variants[1].description).toBe('Constraints: {"additionalProperties":{"type":"string"}}') + }) + + it("keeps properties whose names match unsupported keywords", () => { + const tools = [ + { + type: "function", + function: { + name: "keyword_property_tool", + parameters: { + type: "object", + properties: { + maxItems: { type: "integer" }, + minimum: { type: "number" }, + }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + expect(result?.[0].function.parameters.properties).toHaveProperty("maxItems") + expect(result?.[0].function.parameters.properties).toHaveProperty("minimum") + }) + + it("normalizes nullable object properties before stripping constraints", () => { + const tools = [ + { + type: "function", + function: { + name: "nullable_object_tool", + parameters: { + type: "object", + properties: { + options: { + type: ["object", "null"], + properties: { limit: { type: "integer", minimum: 1 } }, + }, + }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + const options = result?.[0].function.parameters.properties.options + + expect(options.type).toBe("object") + expect(options.required).toEqual(["limit"]) + expect(options.additionalProperties).toBe(false) + expect(options.properties.limit.minimum).toBeUndefined() + }) + it("should not apply schema conversion to MCP tools in base-provider", () => { // Note: In base-provider, MCP tools are passed through unchanged // The openai-native provider has its own handling for MCP tools @@ -350,8 +571,9 @@ describe("BaseProvider", () => { parameters: { type: "object", properties: { - token: { type: "string" }, + token: { type: "string", maxLength: 20 }, }, + maxItems: 4, required: ["token"], }, }, @@ -362,6 +584,8 @@ describe("BaseProvider", () => { // MCP tools pass through original parameters in base-provider expect(result?.[0].function.parameters.additionalProperties).toBeUndefined() + expect(result?.[0].function.parameters.maxItems).toBe(4) + expect(result?.[0].function.parameters.properties.token.maxLength).toBe(20) }) it("should sanitize lone UTF-16 surrogates in name, description, and parameters (#461)", () => { @@ -453,9 +677,10 @@ describe("BaseProvider", () => { type: "object", properties: { path: { type: "string" }, - offset: { type: "integer" }, + offset: { type: "integer", minimum: 1 }, }, required: ["path"], + maxItems: 4, }, }, }, @@ -478,7 +703,7 @@ describe("BaseProvider", () => { parameters: { type: "object", properties: { - path: { type: ["string", "null"] }, + path: { type: ["string", "null"], maxLength: 20 }, }, required: ["path"], }, @@ -488,10 +713,12 @@ describe("BaseProvider", () => { const strictResult = provider.testConvertToolsForOpenAI(tools, true) expect(strictResult?.[0].function.parameters.properties.path.type).toBe("string") + expect(strictResult?.[0].function.parameters.properties.path.maxLength).toBeUndefined() // Caller-owned schema is untouched, so a later non-strict request // can still send the original nullable type. expect(tools[0].function.parameters.properties.path.type).toEqual(["string", "null"]) + expect(tools[0].function.parameters.properties.path.maxLength).toBe(20) const nonStrictResult = provider.testConvertToolsForOpenAI(tools, false) expect(nonStrictResult?.[0].function.parameters).toEqual(tools[0].function.parameters) diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index 7c45f22180..1e8daccb5b 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -1895,6 +1895,26 @@ describe("OpenAiHandler - strict tool schemas", () => { }, }, ] + const constrainedTools: OpenAI.Chat.ChatCompletionFunctionTool[] = [ + { + type: "function", + function: { + name: "ask_followup_question", + description: "Ask a question", + parameters: { + type: "object", + properties: { + follow_up: { + type: "array", + items: { type: "string" }, + minItems: 1, + maxItems: 4, + }, + }, + }, + }, + }, + ] const baseOptions = () => makeApiHandlerOptions({ openAiApiKey: "test-api-key", @@ -1919,6 +1939,22 @@ describe("OpenAiHandler - strict tool schemas", () => { expect(request.tools[0].function.parameters.additionalProperties).toBe(false) }) + it.each([undefined, true])("removes unsupported constraints when strict schemas are %s", async (strict) => { + const options = strict === undefined ? baseOptions() : { ...baseOptions(), openAiStrictToolSchemas: strict } + const handler = new OpenAiHandler(options) + const stream = handler.createMessage(systemPrompt, messages, { + taskId: "test-task", + tools: constrainedTools, + }) + await collectStream(stream) + + const request = mockCreate.mock.calls[0][0] + const followUp = request.tools[0].function.parameters.properties.follow_up + expect(request.tools[0].function.strict).toBe(true) + expect(followUp.minItems).toBe(1) + expect(followUp.maxItems).toBeUndefined() + }) + it("sends strict: false with the declared schema when openAiStrictToolSchemas is false", async () => { const handler = new OpenAiHandler({ ...baseOptions(), openAiStrictToolSchemas: false }) const stream = handler.createMessage(systemPrompt, messages, { taskId: "test-task", tools }) @@ -1930,6 +1966,19 @@ describe("OpenAiHandler - strict tool schemas", () => { expect(request.tools[0].function.parameters).toEqual(tools[0].function.parameters) }) + it("keeps unsupported constraints when strict schemas are disabled", async () => { + const handler = new OpenAiHandler({ ...baseOptions(), openAiStrictToolSchemas: false }) + const stream = handler.createMessage(systemPrompt, messages, { + taskId: "test-task", + tools: constrainedTools, + }) + await collectStream(stream) + + const request = mockCreate.mock.calls[0][0] + expect(request.tools[0].function.strict).toBe(false) + expect(request.tools[0].function.parameters.properties.follow_up.maxItems).toBe(4) + }) + it("propagates a gateway 400 that rejects strict tool schemas", async () => { const strictRejection = Object.assign( new Error("strict=true requires generated function arguments to satisfy the declared JSON Schema"), diff --git a/src/api/providers/base-provider.ts b/src/api/providers/base-provider.ts index 2829c28b86..9208687137 100644 --- a/src/api/providers/base-provider.ts +++ b/src/api/providers/base-provider.ts @@ -5,6 +5,7 @@ import { DEFAULT_OPEN_AI_STRICT_TOOL_SCHEMAS, type ModelInfo } from "@roo-code/t import type { ApiHandler, ApiHandlerCreateMessageMetadata, ModelCacheScope } from "../index" import { ApiStream } from "../transform/stream" import { sanitizeSurrogates, sanitizeSurrogatesDeep } from "../transform/sanitize-surrogates" +import { stripUnsupportedStrictKeywords } from "../transform/strict-tool-schema" import { countTokens } from "../../utils/countTokens" import { isMcpTool } from "../../utils/mcp-name" import { detachOnAbort } from "./utils/abort-signal" @@ -69,7 +70,7 @@ export abstract class BaseProvider implements ApiHandler { strict: useStrict, parameters: sanitizeSurrogatesDeep( useStrict - ? this.convertToolSchemaForOpenAI(tool.function.parameters) + ? stripUnsupportedStrictKeywords(this.convertToolSchemaForOpenAI(tool.function.parameters)) : tool.function.parameters, ), }, diff --git a/src/api/transform/strict-tool-schema.ts b/src/api/transform/strict-tool-schema.ts new file mode 100644 index 0000000000..932bc2e2ea --- /dev/null +++ b/src/api/transform/strict-tool-schema.ts @@ -0,0 +1,109 @@ +const UNSUPPORTED_STRICT_SCHEMA_KEYWORDS = new Set([ + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minLength", + "maxLength", + "maxItems", + "uniqueItems", + "contains", + "minContains", + "maxContains", + "minProperties", + "maxProperties", +]) + +const SUPPORTED_MIN_ITEMS = new Set([0, 1]) +const SUPPORTED_STRING_FORMATS = new Set([ + "date-time", + "time", + "date", + "duration", + "email", + "hostname", + "uri", + "ipv4", + "ipv6", + "uuid", +]) + +const SCHEMA_MAP_KEYWORDS = new Set(["properties", "patternProperties", "$defs", "definitions", "dependentSchemas"]) +const SCHEMA_LIST_KEYWORDS = new Set(["anyOf", "oneOf", "allOf", "prefixItems"]) +const SCHEMA_VALUE_KEYWORDS = new Set([ + "items", + "additionalItems", + "additionalProperties", + "unevaluatedItems", + "unevaluatedProperties", + "propertyNames", + "not", + "if", + "then", + "else", +]) + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value) +} + +function isUnsupportedKeyword(key: string, value: unknown): boolean { + if (UNSUPPORTED_STRICT_SCHEMA_KEYWORDS.has(key)) { + return true + } + if (key === "minItems") { + return typeof value !== "number" || !SUPPORTED_MIN_ITEMS.has(value) + } + if (key === "format") { + return typeof value !== "string" || !SUPPORTED_STRING_FORMATS.has(value) + } + if (key === "additionalProperties") { + return value !== false + } + return false +} + +/** + * Removes constraints that Anthropic and Bedrock reject for strict tool use. + * Removed constraints are retained in descriptions so models can still follow them. + */ +export function stripUnsupportedStrictKeywords(schema: unknown): unknown { + if (!isRecord(schema)) { + return schema + } + + const result: Record = {} + const removed: Record = {} + + for (const [key, originalValue] of Object.entries(schema)) { + if (isUnsupportedKeyword(key, originalValue)) { + removed[key] = originalValue + continue + } + + let value = originalValue + if (SCHEMA_MAP_KEYWORDS.has(key) && isRecord(value)) { + value = Object.fromEntries( + Object.entries(value).map(([name, subSchema]) => [name, stripUnsupportedStrictKeywords(subSchema)]), + ) + } else if (SCHEMA_LIST_KEYWORDS.has(key) && Array.isArray(value)) { + value = value.map(stripUnsupportedStrictKeywords) + } else if (SCHEMA_VALUE_KEYWORDS.has(key)) { + value = Array.isArray(value) + ? value.map(stripUnsupportedStrictKeywords) + : stripUnsupportedStrictKeywords(value) + } + result[key] = value + } + + if (Object.keys(removed).length > 0) { + const constraintNote = `Constraints: ${JSON.stringify(removed)}` + result.description = + typeof result.description === "string" && result.description.length > 0 + ? `${result.description}\n\n${constraintNote}` + : constraintNote + } + + return result +} diff --git a/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts b/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts new file mode 100644 index 0000000000..04b155971d --- /dev/null +++ b/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts @@ -0,0 +1,62 @@ +import type OpenAI from "openai" + +import { getNativeTools } from ".." + +type FunctionTool = OpenAI.Chat.ChatCompletionFunctionTool + +const UNSUPPORTED_KEYWORDS = new Set([ + "maxItems", + "minimum", + "maximum", + "exclusiveMinimum", + "exclusiveMaximum", + "multipleOf", + "minLength", + "maxLength", + "uniqueItems", + "contains", + "minContains", + "maxContains", + "minProperties", + "maxProperties", +]) + +function collectUnsupportedKeywords(schema: unknown, path = "$"): string[] { + if (typeof schema !== "object" || schema === null) { + return [] + } + + const issues: string[] = [] + for (const [key, value] of Object.entries(schema)) { + if ( + UNSUPPORTED_KEYWORDS.has(key) || + (key === "minItems" && value !== 0 && value !== 1) || + (key === "additionalProperties" && value !== false) + ) { + issues.push(`${path}.${key}`) + } + if (key === "properties" && typeof value === "object" && value !== null && !Array.isArray(value)) { + for (const [propertyName, propertySchema] of Object.entries(value)) { + issues.push(...collectUnsupportedKeywords(propertySchema, `${path}.properties.${propertyName}`)) + } + } else if (Array.isArray(value)) { + issues.push( + ...value.flatMap((entry, index) => collectUnsupportedKeywords(entry, `${path}.${key}[${index}]`)), + ) + } else if (typeof value === "object" && value !== null) { + issues.push(...collectUnsupportedKeywords(value, `${path}.${key}`)) + } + } + return issues +} + +describe("native tool strict schema subset", () => { + const tools = getNativeTools().filter((tool): tool is FunctionTool => tool.type === "function") + + it.each(tools.map((tool) => [tool.function.name, tool] as const))( + "keeps %s within the portable strict subset", + (_name, tool) => { + expect(collectUnsupportedKeywords(tool.function.parameters)).toEqual([]) + }, + ) +}) diff --git a/src/core/prompts/tools/native-tools/ask_followup_question.ts b/src/core/prompts/tools/native-tools/ask_followup_question.ts index b0591206ad..b271fff05d 100644 --- a/src/core/prompts/tools/native-tools/ask_followup_question.ts +++ b/src/core/prompts/tools/native-tools/ask_followup_question.ts @@ -1,5 +1,7 @@ import type OpenAI from "openai" +export const MAX_FOLLOW_UP_SUGGESTIONS = 4 + const ASK_FOLLOWUP_QUESTION_DESCRIPTION = `Ask the user a question to gather additional information needed to complete the task. Use when you need clarification or more details to proceed effectively. Parameters: @@ -52,7 +54,6 @@ export default { additionalProperties: false, }, minItems: 1, - maxItems: 4, }, }, required: ["question", "follow_up"], diff --git a/src/core/prompts/tools/native-tools/edit_file.ts b/src/core/prompts/tools/native-tools/edit_file.ts index 14a9851405..f781705b32 100644 --- a/src/core/prompts/tools/native-tools/edit_file.ts +++ b/src/core/prompts/tools/native-tools/edit_file.ts @@ -59,8 +59,7 @@ const edit_file = { expected_replacements: { type: "number", description: - "Number of replacements expected. Defaults to 1 if not specified. Use when you want to replace multiple occurrences of the same text.", - minimum: 1, + "Number of replacements expected. Must be 1 or greater. Defaults to 1 if not specified. Use when you want to replace multiple occurrences of the same text.", }, }, required: ["file_path", "old_string", "new_string"], diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 3baea7ed52..8581d71871 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -1,5 +1,6 @@ import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" +import { MAX_FOLLOW_UP_SUGGESTIONS } from "../prompts/tools/native-tools/ask_followup_question" import type { ToolUse } from "../../shared/tools" import { getSuggestionMode } from "@roo-code/types" @@ -66,7 +67,9 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { // Transform follow_up suggestions to the format expected by task.ask const follow_up_json = { question, - suggest: follow_up.map((s) => ({ answer: s.text, mode: getSuggestionMode(s.mode) })), + suggest: follow_up + .slice(0, MAX_FOLLOW_UP_SUGGESTIONS) + .map((s) => ({ answer: s.text, mode: getSuggestionMode(s.mode) })), } task.consecutiveMistakeCount = 0 diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 965cb23e19..e5b1257654 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -166,6 +166,20 @@ describe("AskFollowupQuestionTool", () => { expect(mockTask.ask).toHaveBeenCalledWith("followup", expectedJson, false) }) + it("limits follow-up suggestions to four", async () => { + const params = { + question: "Choose an option", + follow_up: ["A", "B", "C", "D", "E"].map((text) => ({ text })), + } + + await tool.execute(params, mockTask, mockCallbacks) + + const payload = JSON.parse(vi.mocked(mockTask.ask).mock.calls[0][1] ?? "") as { + suggest: Array<{ answer: string }> + } + expect(payload.suggest.map(({ answer }) => answer)).toEqual(["A", "B", "C", "D"]) + }) + it("should normalize malformed object mode values", async () => { const params = { question: "Switch mode?", From ee5d91948c824ba938fcfc1bd6f2bc8d9119c4e7 Mon Sep 17 00:00:00 2001 From: Jesse Quijano Date: Thu, 12 Nov 2026 14:32:38 -0800 Subject: [PATCH 2/3] fix(providers): set additionalProperties to false in strict schemas Strict mode requires additionalProperties: false on every object schema. stripUnsupportedStrictKeywords() removed a true or schema value, but it did not set false. convertToolSchemaForOpenAI() does not go into anyOf. Thus an object in anyOf had no additionalProperties value. - Set additionalProperties to false. Keep the removed value in the description. - Do not go into the additionalProperties value. The function always removes a schema value. - Add focused tests for each keyword list, each branch, and the description note. --- .../providers/__tests__/base-provider.spec.ts | 6 +- .../__tests__/strict-tool-schema.spec.ts | 207 ++++++++++++++++++ src/api/transform/strict-tool-schema.ts | 14 +- 3 files changed, 219 insertions(+), 8 deletions(-) create mode 100644 src/api/transform/__tests__/strict-tool-schema.spec.ts diff --git a/src/api/providers/__tests__/base-provider.spec.ts b/src/api/providers/__tests__/base-provider.spec.ts index 755cc1660d..307f8f6b09 100644 --- a/src/api/providers/__tests__/base-provider.spec.ts +++ b/src/api/providers/__tests__/base-provider.spec.ts @@ -478,7 +478,7 @@ describe("BaseProvider", () => { expect(parameters.else.minLength).toBeUndefined() }) - it("removes additionalProperties values other than false in nested schemas", () => { + it("replaces additionalProperties values other than false in nested schemas", () => { const tools = [ { type: "function", @@ -502,8 +502,8 @@ describe("BaseProvider", () => { const result = provider.testConvertToolsForOpenAI(tools) const variants = result?.[0].function.parameters.properties.options.anyOf - expect(variants[0].additionalProperties).toBeUndefined() - expect(variants[1].additionalProperties).toBeUndefined() + expect(variants[0].additionalProperties).toBe(false) + expect(variants[1].additionalProperties).toBe(false) expect(variants[0].description).toBe('Constraints: {"additionalProperties":true}') expect(variants[1].description).toBe('Constraints: {"additionalProperties":{"type":"string"}}') }) diff --git a/src/api/transform/__tests__/strict-tool-schema.spec.ts b/src/api/transform/__tests__/strict-tool-schema.spec.ts new file mode 100644 index 0000000000..8adc8ca0ed --- /dev/null +++ b/src/api/transform/__tests__/strict-tool-schema.spec.ts @@ -0,0 +1,207 @@ +// npx vitest run api/transform/__tests__/strict-tool-schema.spec.ts + +import { stripUnsupportedStrictKeywords } from "../strict-tool-schema" + +type Schema = Record + +function strip(schema: Schema): Schema { + return stripUnsupportedStrictKeywords(schema) as Schema +} + +function constraintNote(removed: Schema): string { + return `Constraints: ${JSON.stringify(removed)}` +} + +describe("stripUnsupportedStrictKeywords", () => { + it.each([ + ["minimum", 1], + ["maximum", 9], + ["exclusiveMinimum", 0], + ["exclusiveMaximum", 10], + ["multipleOf", 2], + ["minLength", 1], + ["maxLength", 5], + ["maxItems", 4], + ["uniqueItems", true], + ["contains", { type: "string" }], + ["minContains", 1], + ["maxContains", 3], + ["minProperties", 1], + ["maxProperties", 2], + ])("removes %s and records it in the description", (keyword, value) => { + expect(strip({ type: "string", [keyword]: value })).toEqual({ + type: "string", + description: constraintNote({ [keyword]: value }), + }) + }) + + it.each([0, 1])("keeps minItems %s", (minItems) => { + expect(strip({ type: "array", minItems })).toEqual({ type: "array", minItems }) + }) + + it.each([2, "1", null])("removes unsupported minItems %j", (minItems) => { + expect(strip({ type: "array", minItems })).toEqual({ + type: "array", + description: constraintNote({ minItems }), + }) + }) + + it.each(["date-time", "time", "date", "duration", "email", "hostname", "uri", "ipv4", "ipv6", "uuid"])( + "keeps supported format %s", + (format) => { + expect(strip({ type: "string", format })).toEqual({ type: "string", format }) + }, + ) + + it.each(["phone", 1, null])("removes unsupported format %j", (format) => { + expect(strip({ type: "string", format })).toEqual({ + type: "string", + description: constraintNote({ format }), + }) + }) + + it("keeps additionalProperties false", () => { + const schema = { type: "object", properties: {}, additionalProperties: false } + + expect(strip(schema)).toEqual(schema) + }) + + it.each([true, { type: "string" }])( + "replaces additionalProperties %j with false and records the original value", + (additionalProperties) => { + expect(strip({ type: "object", properties: {}, additionalProperties })).toEqual({ + type: "object", + properties: {}, + additionalProperties: false, + description: constraintNote({ additionalProperties }), + }) + }, + ) + + it("replaces additionalProperties with false in objects inside anyOf", () => { + const result = strip({ + anyOf: [ + { type: "object", properties: {}, additionalProperties: true }, + { type: "object", properties: {}, additionalProperties: { type: "string" } }, + ], + }) + + expect(result.anyOf).toEqual([ + { + type: "object", + properties: {}, + additionalProperties: false, + description: constraintNote({ additionalProperties: true }), + }, + { + type: "object", + properties: {}, + additionalProperties: false, + description: constraintNote({ additionalProperties: { type: "string" } }), + }, + ]) + }) + + it.each(["properties", "patternProperties", "$defs", "definitions", "dependentSchemas"])( + "strips each subschema in the %s map", + (keyword) => { + expect(strip({ [keyword]: { value: { type: "string", maxLength: 5 } } })).toEqual({ + [keyword]: { value: { type: "string", description: constraintNote({ maxLength: 5 }) } }, + }) + }, + ) + + it.each(["anyOf", "oneOf", "allOf", "prefixItems"])("strips each subschema in the %s list", (keyword) => { + expect(strip({ [keyword]: [{ type: "integer", minimum: 0 }, { type: "null" }] })).toEqual({ + [keyword]: [{ type: "integer", description: constraintNote({ minimum: 0 }) }, { type: "null" }], + }) + }) + + it.each([ + "items", + "additionalItems", + "unevaluatedItems", + "unevaluatedProperties", + "propertyNames", + "not", + "if", + "then", + "else", + ])("strips the %s subschema", (keyword) => { + expect(strip({ [keyword]: { type: "string", minLength: 1 } })).toEqual({ + [keyword]: { type: "string", description: constraintNote({ minLength: 1 }) }, + }) + }) + + it("strips each subschema in tuple-form items", () => { + expect(strip({ items: [{ type: "string", maxLength: 2 }, { type: "integer" }] })).toEqual({ + items: [{ type: "string", description: constraintNote({ maxLength: 2 }) }, { type: "integer" }], + }) + }) + + it.each([ + ["default", { minimum: 1 }], + ["const", { maxLength: 2 }], + ["examples", [{ maxItems: 3 }]], + ["enum", [{ minimum: 0 }]], + ["anyOf", { maxLength: 1 }], + ["properties", [{ maxLength: 1 }]], + ])("does not treat the %s value %j as a schema", (keyword, value) => { + const schema = { [keyword]: value } + + expect(strip(schema)).toEqual(schema) + }) + + it("keeps properties whose names match unsupported keywords", () => { + expect(strip({ properties: { maxItems: { type: "integer" }, minimum: { type: "number" } } })).toEqual({ + properties: { maxItems: { type: "integer" }, minimum: { type: "number" } }, + }) + }) + + it.each([ + ["a missing", undefined], + ["an empty", ""], + ["a non-string", 42], + ])("uses only the constraint note for %s description", (_label, description) => { + expect(strip({ type: "integer", description, minimum: 1 }).description).toBe(constraintNote({ minimum: 1 })) + }) + + it("appends the constraint note to an existing description", () => { + expect(strip({ type: "integer", description: "Count.", minimum: 1 }).description).toBe( + `Count.\n\n${constraintNote({ minimum: 1 })}`, + ) + }) + + it("records all removed constraints of one schema in one note", () => { + expect(strip({ type: "number", minimum: 1, maximum: 9 })).toEqual({ + type: "number", + description: constraintNote({ minimum: 1, maximum: 9 }), + }) + }) + + it("does not add a description when nothing is removed", () => { + expect(strip({ type: "string" })).toEqual({ type: "string" }) + }) + + it.each([ + ["null", null], + ["a string", "text"], + ["a boolean", true], + ["an array", [{ maxLength: 1 }]], + ])("returns %s unchanged", (_label, value) => { + expect(stripUnsupportedStrictKeywords(value)).toBe(value) + }) + + it("does not mutate the input schema", () => { + const schema = { + type: "object", + properties: { name: { type: "string", maxLength: 5 } }, + additionalProperties: true, + } + const snapshot = structuredClone(schema) + + strip(schema) + + expect(schema).toEqual(snapshot) + }) +}) diff --git a/src/api/transform/strict-tool-schema.ts b/src/api/transform/strict-tool-schema.ts index 932bc2e2ea..e0cb202493 100644 --- a/src/api/transform/strict-tool-schema.ts +++ b/src/api/transform/strict-tool-schema.ts @@ -15,8 +15,8 @@ const UNSUPPORTED_STRICT_SCHEMA_KEYWORDS = new Set([ "maxProperties", ]) -const SUPPORTED_MIN_ITEMS = new Set([0, 1]) -const SUPPORTED_STRING_FORMATS = new Set([ +const SUPPORTED_MIN_ITEMS = new Set([0, 1]) +const SUPPORTED_STRING_FORMATS = new Set([ "date-time", "time", "date", @@ -34,7 +34,6 @@ const SCHEMA_LIST_KEYWORDS = new Set(["anyOf", "oneOf", "allOf", "prefixItems"]) const SCHEMA_VALUE_KEYWORDS = new Set([ "items", "additionalItems", - "additionalProperties", "unevaluatedItems", "unevaluatedProperties", "propertyNames", @@ -53,10 +52,10 @@ function isUnsupportedKeyword(key: string, value: unknown): boolean { return true } if (key === "minItems") { - return typeof value !== "number" || !SUPPORTED_MIN_ITEMS.has(value) + return !SUPPORTED_MIN_ITEMS.has(value) } if (key === "format") { - return typeof value !== "string" || !SUPPORTED_STRING_FORMATS.has(value) + return !SUPPORTED_STRING_FORMATS.has(value) } if (key === "additionalProperties") { return value !== false @@ -67,6 +66,8 @@ function isUnsupportedKeyword(key: string, value: unknown): boolean { /** * Removes constraints that Anthropic and Bedrock reject for strict tool use. * Removed constraints are retained in descriptions so models can still follow them. + * An unsupported additionalProperties value becomes false, because strict mode + * requires additionalProperties: false on every object schema. */ export function stripUnsupportedStrictKeywords(schema: unknown): unknown { if (!isRecord(schema)) { @@ -79,6 +80,9 @@ export function stripUnsupportedStrictKeywords(schema: unknown): unknown { for (const [key, originalValue] of Object.entries(schema)) { if (isUnsupportedKeyword(key, originalValue)) { removed[key] = originalValue + if (key === "additionalProperties") { + result[key] = false + } continue } From 67b1ddb5a05f3d2dc431f4ff17c4401f0d1f2155 Mon Sep 17 00:00:00 2001 From: Jesse Quijano Date: Fri, 13 Nov 2026 19:57:33 -0800 Subject: [PATCH 3/3] test(tools): use strict schema sanitizer in native guard --- .../__tests__/strict-schema-subset.spec.ts | 49 +------------------ 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts b/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts index 04b155971d..8e72bf7439 100644 --- a/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts +++ b/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts @@ -1,62 +1,17 @@ import type OpenAI from "openai" +import { stripUnsupportedStrictKeywords } from "../../../../../api/transform/strict-tool-schema" import { getNativeTools } from ".." type FunctionTool = OpenAI.Chat.ChatCompletionFunctionTool -const UNSUPPORTED_KEYWORDS = new Set([ - "maxItems", - "minimum", - "maximum", - "exclusiveMinimum", - "exclusiveMaximum", - "multipleOf", - "minLength", - "maxLength", - "uniqueItems", - "contains", - "minContains", - "maxContains", - "minProperties", - "maxProperties", -]) - -function collectUnsupportedKeywords(schema: unknown, path = "$"): string[] { - if (typeof schema !== "object" || schema === null) { - return [] - } - - const issues: string[] = [] - for (const [key, value] of Object.entries(schema)) { - if ( - UNSUPPORTED_KEYWORDS.has(key) || - (key === "minItems" && value !== 0 && value !== 1) || - (key === "additionalProperties" && value !== false) - ) { - issues.push(`${path}.${key}`) - } - if (key === "properties" && typeof value === "object" && value !== null && !Array.isArray(value)) { - for (const [propertyName, propertySchema] of Object.entries(value)) { - issues.push(...collectUnsupportedKeywords(propertySchema, `${path}.properties.${propertyName}`)) - } - } else if (Array.isArray(value)) { - issues.push( - ...value.flatMap((entry, index) => collectUnsupportedKeywords(entry, `${path}.${key}[${index}]`)), - ) - } else if (typeof value === "object" && value !== null) { - issues.push(...collectUnsupportedKeywords(value, `${path}.${key}`)) - } - } - return issues -} - describe("native tool strict schema subset", () => { const tools = getNativeTools().filter((tool): tool is FunctionTool => tool.type === "function") it.each(tools.map((tool) => [tool.function.name, tool] as const))( "keeps %s within the portable strict subset", (_name, tool) => { - expect(collectUnsupportedKeywords(tool.function.parameters)).toEqual([]) + expect(stripUnsupportedStrictKeywords(tool.function.parameters)).toEqual(tool.function.parameters) }, ) })