diff --git a/src/api/providers/__tests__/base-provider.spec.ts b/src/api/providers/__tests__/base-provider.spec.ts index da1f449c38..307f8f6b09 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("replaces 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).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"}}') + }) + + 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/__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 new file mode 100644 index 0000000000..e0cb202493 --- /dev/null +++ b/src/api/transform/strict-tool-schema.ts @@ -0,0 +1,113 @@ +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", + "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 !SUPPORTED_MIN_ITEMS.has(value) + } + if (key === "format") { + return !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. + * 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)) { + return schema + } + + const result: Record = {} + const removed: Record = {} + + for (const [key, originalValue] of Object.entries(schema)) { + if (isUnsupportedKeyword(key, originalValue)) { + removed[key] = originalValue + if (key === "additionalProperties") { + result[key] = false + } + 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..8e72bf7439 --- /dev/null +++ b/src/core/prompts/tools/native-tools/__tests__/strict-schema-subset.spec.ts @@ -0,0 +1,17 @@ +import type OpenAI from "openai" + +import { stripUnsupportedStrictKeywords } from "../../../../../api/transform/strict-tool-schema" +import { getNativeTools } from ".." + +type FunctionTool = OpenAI.Chat.ChatCompletionFunctionTool + +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(stripUnsupportedStrictKeywords(tool.function.parameters)).toEqual(tool.function.parameters) + }, + ) +}) 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?",