diff --git a/apps/cli/install.sh b/apps/cli/install.sh index 431c821d7b..ca82ecfdd8 100755 --- a/apps/cli/install.sh +++ b/apps/cli/install.sh @@ -1,6 +1,6 @@ #!/bin/sh # Roo Code CLI Installer -# Usage: curl -fsSL https://raw.githubusercontent.com/RooVetGit/Roo-Code/main/apps/cli/install.sh | sh +# Usage: curl -fsSL https://raw.githubusercontent.com/RooCodeInc/Roo-Code/main/apps/cli/install.sh | sh # # Environment variables: # ROO_INSTALL_DIR - Installation directory (default: ~/.roo/cli) diff --git a/apps/cli/scripts/release.sh b/apps/cli/scripts/release.sh index e4242446c2..43d5298956 100755 --- a/apps/cli/scripts/release.sh +++ b/apps/cli/scripts/release.sh @@ -331,7 +331,7 @@ print_summary() { echo "" printf "${GREEN}${BOLD}✓ Release v$VERSION created successfully!${NC}\n" echo "" - echo " Release URL: https://github.com/RooVetGit/Roo-Code/releases/tag/$TAG" + echo " Release URL: https://github.com/RooCodeInc/Roo-Code/releases/tag/$TAG" echo "" echo " Install with:" echo " curl -fsSL https://raw.githubusercontent.com/RooCodeInc/Roo-Code/main/apps/cli/install.sh | sh" diff --git a/src/api/providers/__tests__/base-provider.spec.ts b/src/api/providers/__tests__/base-provider.spec.ts new file mode 100644 index 0000000000..ced452f5a5 --- /dev/null +++ b/src/api/providers/__tests__/base-provider.spec.ts @@ -0,0 +1,283 @@ +import { Anthropic } from "@anthropic-ai/sdk" + +import type { ModelInfo } from "@roo-code/types" + +import { BaseProvider } from "../base-provider" +import type { ApiStream } from "../../transform/stream" + +// Create a concrete implementation for testing +class TestProvider extends BaseProvider { + createMessage(_systemPrompt: string, _messages: Anthropic.Messages.MessageParam[]): ApiStream { + throw new Error("Not implemented") + } + + getModel(): { id: string; info: ModelInfo } { + return { + id: "test-model", + info: { + maxTokens: 4096, + contextWindow: 128000, + supportsPromptCache: false, + }, + } + } + + // Expose protected method for testing + public testConvertToolSchemaForOpenAI(schema: any): any { + return this.convertToolSchemaForOpenAI(schema) + } + + // Expose protected method for testing + public testConvertToolsForOpenAI(tools: any[] | undefined): any[] | undefined { + return this.convertToolsForOpenAI(tools) + } +} + +describe("BaseProvider", () => { + let provider: TestProvider + + beforeEach(() => { + provider = new TestProvider() + }) + + describe("convertToolSchemaForOpenAI", () => { + it("should add additionalProperties: false to object schemas", () => { + const schema = { + type: "object", + properties: { + name: { type: "string" }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.additionalProperties).toBe(false) + }) + + it("should add required array with all properties for strict mode", () => { + const schema = { + type: "object", + properties: { + name: { type: "string" }, + age: { type: "number" }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.required).toEqual(["name", "age"]) + }) + + it("should recursively add additionalProperties: false to nested objects", () => { + const schema = { + type: "object", + properties: { + user: { + type: "object", + properties: { + name: { type: "string" }, + }, + }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.additionalProperties).toBe(false) + expect(result.properties.user.additionalProperties).toBe(false) + }) + + it("should recursively add additionalProperties: false to array item objects", () => { + const schema = { + type: "object", + properties: { + users: { + type: "array", + items: { + type: "object", + properties: { + name: { type: "string" }, + }, + }, + }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.additionalProperties).toBe(false) + expect(result.properties.users.items.additionalProperties).toBe(false) + }) + + it("should handle deeply nested objects", () => { + const schema = { + type: "object", + properties: { + level1: { + type: "object", + properties: { + level2: { + type: "object", + properties: { + level3: { + type: "object", + properties: { + value: { type: "string" }, + }, + }, + }, + }, + }, + }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.additionalProperties).toBe(false) + expect(result.properties.level1.additionalProperties).toBe(false) + expect(result.properties.level1.properties.level2.additionalProperties).toBe(false) + expect(result.properties.level1.properties.level2.properties.level3.additionalProperties).toBe(false) + }) + + it("should convert nullable types to non-nullable", () => { + const schema = { + type: "object", + properties: { + name: { type: ["string", "null"] }, + }, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.properties.name.type).toBe("string") + }) + + it("should return non-object schemas unchanged", () => { + const schema = { type: "string" } + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result).toEqual(schema) + }) + + it("should return null/undefined unchanged", () => { + expect(provider.testConvertToolSchemaForOpenAI(null)).toBeNull() + expect(provider.testConvertToolSchemaForOpenAI(undefined)).toBeUndefined() + }) + + it("should handle empty properties object", () => { + const schema = { + type: "object", + properties: {}, + } + + const result = provider.testConvertToolSchemaForOpenAI(schema) + + expect(result.additionalProperties).toBe(false) + expect(result.required).toEqual([]) + }) + }) + + describe("convertToolsForOpenAI", () => { + it("should return undefined for undefined input", () => { + const result = provider.testConvertToolsForOpenAI(undefined) + expect(result).toBeUndefined() + }) + + it("should set strict: true for non-MCP tools", () => { + const tools = [ + { + type: "function", + function: { + name: "read_file", + description: "Read a file", + parameters: { type: "object", properties: {} }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + expect(result?.[0].function.strict).toBe(true) + }) + + it("should set strict: false for MCP tools (mcp-- prefix)", () => { + const tools = [ + { + type: "function", + function: { + name: "mcp--github--get_me", + description: "Get current user", + parameters: { type: "object", properties: {} }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + expect(result?.[0].function.strict).toBe(false) + }) + + it("should apply schema conversion to non-MCP tools", () => { + const tools = [ + { + type: "function", + function: { + name: "read_file", + description: "Read a file", + parameters: { + type: "object", + properties: { + path: { type: "string" }, + }, + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + expect(result?.[0].function.parameters.additionalProperties).toBe(false) + expect(result?.[0].function.parameters.required).toEqual(["path"]) + }) + + 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 + const tools = [ + { + type: "function", + function: { + name: "mcp--github--get_me", + description: "Get current user", + parameters: { + type: "object", + properties: { + token: { type: "string" }, + }, + required: ["token"], + }, + }, + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + // MCP tools pass through original parameters in base-provider + expect(result?.[0].function.parameters.additionalProperties).toBeUndefined() + }) + + it("should preserve non-function tools unchanged", () => { + const tools = [ + { + type: "other_type", + data: "some data", + }, + ] + + const result = provider.testConvertToolsForOpenAI(tools) + + expect(result?.[0]).toEqual(tools[0]) + }) + }) +}) diff --git a/src/api/providers/__tests__/openai-native-tools.spec.ts b/src/api/providers/__tests__/openai-native-tools.spec.ts index 1a3b93b9c2..7be4814b63 100644 --- a/src/api/providers/__tests__/openai-native-tools.spec.ts +++ b/src/api/providers/__tests__/openai-native-tools.spec.ts @@ -1,6 +1,8 @@ import OpenAI from "openai" import { OpenAiHandler } from "../openai" +import { OpenAiNativeHandler } from "../openai-native" +import type { ApiHandlerOptions } from "../../../shared/api" describe("OpenAiHandler native tools", () => { it("includes tools in request when custom model info lacks supportsNativeTools (regression test)", async () => { @@ -75,3 +77,220 @@ describe("OpenAiHandler native tools", () => { ) }) }) + +describe("OpenAiNativeHandler MCP tool schema handling", () => { + it("should add additionalProperties: false to MCP tools while keeping strict: false", async () => { + let capturedRequestBody: any + + const handler = new OpenAiNativeHandler({ + openAiNativeApiKey: "test-key", + apiModelId: "gpt-4o", + } as ApiHandlerOptions) + + // Mock the responses API call + const mockClient = { + responses: { + create: vi.fn().mockImplementation((body: any) => { + capturedRequestBody = body + return { + [Symbol.asyncIterator]: async function* () { + yield { + type: "response.done", + response: { + output: [{ type: "message", content: [{ type: "output_text", text: "test" }] }], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + } + }, + } + }), + }, + } + ;(handler as any).client = mockClient + + const mcpTools: OpenAI.Chat.ChatCompletionTool[] = [ + { + type: "function", + function: { + name: "mcp--github--get_me", + description: "Get current GitHub user", + parameters: { + type: "object", + properties: { + token: { type: "string", description: "API token" }, + }, + required: ["token"], + }, + }, + }, + ] + + const stream = handler.createMessage("system prompt", [], { + taskId: "test-task-id", + tools: mcpTools, + toolProtocol: "native" as const, + }) + + // Consume the stream + for await (const _ of stream) { + // Just consume + } + + // Verify the request body + expect(capturedRequestBody.tools).toBeDefined() + expect(capturedRequestBody.tools.length).toBe(1) + + const tool = capturedRequestBody.tools[0] + expect(tool.name).toBe("mcp--github--get_me") + expect(tool.strict).toBe(false) // MCP tools should have strict: false + expect(tool.parameters.additionalProperties).toBe(false) // Should have additionalProperties: false + expect(tool.parameters.required).toEqual(["token"]) // Should preserve original required array + }) + + it("should add additionalProperties: false and required array to non-MCP tools with strict: true", async () => { + let capturedRequestBody: any + + const handler = new OpenAiNativeHandler({ + openAiNativeApiKey: "test-key", + apiModelId: "gpt-4o", + } as ApiHandlerOptions) + + // Mock the responses API call + const mockClient = { + responses: { + create: vi.fn().mockImplementation((body: any) => { + capturedRequestBody = body + return { + [Symbol.asyncIterator]: async function* () { + yield { + type: "response.done", + response: { + output: [{ type: "message", content: [{ type: "output_text", text: "test" }] }], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + } + }, + } + }), + }, + } + ;(handler as any).client = mockClient + + const regularTools: OpenAI.Chat.ChatCompletionTool[] = [ + { + type: "function", + function: { + name: "read_file", + description: "Read a file from the filesystem", + parameters: { + type: "object", + properties: { + path: { type: "string", description: "File path" }, + encoding: { type: "string", description: "File encoding" }, + }, + }, + }, + }, + ] + + const stream = handler.createMessage("system prompt", [], { + taskId: "test-task-id", + tools: regularTools, + toolProtocol: "native" as const, + }) + + // Consume the stream + for await (const _ of stream) { + // Just consume + } + + // Verify the request body + expect(capturedRequestBody.tools).toBeDefined() + expect(capturedRequestBody.tools.length).toBe(1) + + const tool = capturedRequestBody.tools[0] + expect(tool.name).toBe("read_file") + expect(tool.strict).toBe(true) // Non-MCP tools should have strict: true + expect(tool.parameters.additionalProperties).toBe(false) // Should have additionalProperties: false + expect(tool.parameters.required).toEqual(["path", "encoding"]) // Should have all properties as required + }) + + it("should recursively add additionalProperties: false to nested objects in MCP tools", async () => { + let capturedRequestBody: any + + const handler = new OpenAiNativeHandler({ + openAiNativeApiKey: "test-key", + apiModelId: "gpt-4o", + } as ApiHandlerOptions) + + // Mock the responses API call + const mockClient = { + responses: { + create: vi.fn().mockImplementation((body: any) => { + capturedRequestBody = body + return { + [Symbol.asyncIterator]: async function* () { + yield { + type: "response.done", + response: { + output: [{ type: "message", content: [{ type: "output_text", text: "test" }] }], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + } + }, + } + }), + }, + } + ;(handler as any).client = mockClient + + const mcpToolsWithNestedObjects: OpenAI.Chat.ChatCompletionTool[] = [ + { + type: "function", + function: { + name: "mcp--linear--create_issue", + description: "Create a Linear issue", + parameters: { + type: "object", + properties: { + title: { type: "string" }, + metadata: { + type: "object", + properties: { + priority: { type: "number" }, + labels: { + type: "array", + items: { + type: "object", + properties: { + name: { type: "string" }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + ] + + const stream = handler.createMessage("system prompt", [], { + taskId: "test-task-id", + tools: mcpToolsWithNestedObjects, + toolProtocol: "native" as const, + }) + + // Consume the stream + for await (const _ of stream) { + // Just consume + } + + // Verify the request body + const tool = capturedRequestBody.tools[0] + expect(tool.strict).toBe(false) // MCP tool should have strict: false + expect(tool.parameters.additionalProperties).toBe(false) // Root level + expect(tool.parameters.properties.metadata.additionalProperties).toBe(false) // Nested object + expect(tool.parameters.properties.metadata.properties.labels.items.additionalProperties).toBe(false) // Array items + }) +}) diff --git a/src/api/providers/base-provider.ts b/src/api/providers/base-provider.ts index 64d99b3f0c..a6adeeadbd 100644 --- a/src/api/providers/base-provider.ts +++ b/src/api/providers/base-provider.ts @@ -55,6 +55,7 @@ export abstract class BaseProvider implements ApiHandler { * Converts tool schemas to be compatible with OpenAI's strict mode by: * - Ensuring all properties are in the required array (strict mode requirement) * - Converting nullable types (["type", "null"]) to non-nullable ("type") + * - Adding additionalProperties: false to all object schemas (required by OpenAI Responses API) * - Recursively processing nested objects and arrays * * This matches the behavior of ensureAllRequired in openai-native.ts @@ -66,6 +67,12 @@ export abstract class BaseProvider implements ApiHandler { const result = { ...schema } + // OpenAI Responses API requires additionalProperties: false on all object schemas + // Only add if not already set to false (to avoid unnecessary mutations) + if (result.additionalProperties !== false) { + result.additionalProperties = false + } + if (result.properties) { const allKeys = Object.keys(result.properties) // OpenAI strict mode requires ALL properties to be in required array diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index 8f9cc2297f..58a62497f7 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -196,6 +196,12 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio const result = { ...schema } + // OpenAI Responses API requires additionalProperties: false on all object schemas + // Only add if not already set to false (to avoid unnecessary mutations) + if (result.additionalProperties !== false) { + result.additionalProperties = false + } + if (result.properties) { const allKeys = Object.keys(result.properties) result.required = allKeys @@ -219,6 +225,42 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio return result } + // Adds additionalProperties: false to all object schemas recursively + // without modifying required array. Used for MCP tools with strict: false + // to comply with OpenAI Responses API requirements. + const ensureAdditionalPropertiesFalse = (schema: any): any => { + if (!schema || typeof schema !== "object" || schema.type !== "object") { + return schema + } + + const result = { ...schema } + + // OpenAI Responses API requires additionalProperties: false on all object schemas + // Only add if not already set to false (to avoid unnecessary mutations) + if (result.additionalProperties !== false) { + result.additionalProperties = false + } + + if (result.properties) { + // Recursively process nested objects + const newProps = { ...result.properties } + for (const key of Object.keys(result.properties)) { + const prop = newProps[key] + if (prop && prop.type === "object") { + newProps[key] = ensureAdditionalPropertiesFalse(prop) + } else if (prop && prop.type === "array" && prop.items?.type === "object") { + newProps[key] = { + ...prop, + items: ensureAdditionalPropertiesFalse(prop.items), + } + } + } + result.properties = newProps + } + + return result + } + // Build a request body for the OpenAI Responses API. // Ensure we explicitly pass max_output_tokens based on Roo's reserved model response calculation // so requests do not default to very large limits (e.g., 120k). @@ -295,12 +337,15 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio .map((tool) => { // MCP tools use the 'mcp--' prefix - disable strict mode for them // to preserve optional parameters from the MCP server schema + // But we still need to add additionalProperties: false for OpenAI Responses API const isMcp = isMcpTool(tool.function.name) return { type: "function", name: tool.function.name, description: tool.function.description, - parameters: isMcp ? tool.function.parameters : ensureAllRequired(tool.function.parameters), + parameters: isMcp + ? ensureAdditionalPropertiesFalse(tool.function.parameters) + : ensureAllRequired(tool.function.parameters), strict: !isMcp, } }), diff --git a/src/core/condense/__tests__/index.spec.ts b/src/core/condense/__tests__/index.spec.ts index 1309afb221..ef5af01243 100644 --- a/src/core/condense/__tests__/index.spec.ts +++ b/src/core/condense/__tests__/index.spec.ts @@ -334,6 +334,254 @@ describe("getKeepMessagesWithToolBlocks", () => { expect(result.toolUseBlocksToPreserve).toHaveLength(1) expect(result.reasoningBlocksToPreserve).toHaveLength(0) }) + + it("should preserve tool_use when tool_result is in 2nd kept message and tool_use is 2 messages before boundary", () => { + const toolUseBlock = { + type: "tool_use" as const, + id: "toolu_second_kept", + name: "read_file", + input: { path: "test.txt" }, + } + const toolResultBlock = { + type: "tool_result" as const, + tool_use_id: "toolu_second_kept", + content: "file contents", + } + + const messages: ApiMessage[] = [ + { role: "user", content: "Hello", ts: 1 }, + { role: "assistant", content: "Let me help", ts: 2 }, + { + role: "assistant", + content: [{ type: "text" as const, text: "Reading file..." }, toolUseBlock], + ts: 3, + }, + { role: "user", content: "Some other message", ts: 4 }, + { role: "assistant", content: "First kept message", ts: 5 }, + { + role: "user", + content: [toolResultBlock, { type: "text" as const, text: "Continue" }], + ts: 6, + }, + { role: "assistant", content: "Third kept message", ts: 7 }, + ] + + const result = getKeepMessagesWithToolBlocks(messages, 3) + + // keepMessages should be the last 3 messages (ts: 5, 6, 7) + expect(result.keepMessages).toHaveLength(3) + expect(result.keepMessages[0].ts).toBe(5) + expect(result.keepMessages[1].ts).toBe(6) + expect(result.keepMessages[2].ts).toBe(7) + + // Should preserve the tool_use block from message at ts:3 (2 messages before boundary) + expect(result.toolUseBlocksToPreserve).toHaveLength(1) + expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock) + }) + + it("should preserve tool_use when tool_result is in 3rd kept message and tool_use is at boundary edge", () => { + const toolUseBlock = { + type: "tool_use" as const, + id: "toolu_third_kept", + name: "search", + input: { query: "test" }, + } + const toolResultBlock = { + type: "tool_result" as const, + tool_use_id: "toolu_third_kept", + content: "search results", + } + + const messages: ApiMessage[] = [ + { role: "user", content: "Start", ts: 1 }, + { + role: "assistant", + content: [{ type: "text" as const, text: "Searching..." }, toolUseBlock], + ts: 2, + }, + { role: "user", content: "First kept message", ts: 3 }, + { role: "assistant", content: "Second kept message", ts: 4 }, + { + role: "user", + content: [toolResultBlock, { type: "text" as const, text: "Done" }], + ts: 5, + }, + ] + + const result = getKeepMessagesWithToolBlocks(messages, 3) + + // keepMessages should be the last 3 messages (ts: 3, 4, 5) + expect(result.keepMessages).toHaveLength(3) + expect(result.keepMessages[0].ts).toBe(3) + expect(result.keepMessages[1].ts).toBe(4) + expect(result.keepMessages[2].ts).toBe(5) + + // Should preserve the tool_use block from message at ts:2 (at the search boundary edge) + expect(result.toolUseBlocksToPreserve).toHaveLength(1) + expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock) + }) + + it("should preserve multiple tool_uses when tool_results are in different kept messages", () => { + const toolUseBlock1 = { + type: "tool_use" as const, + id: "toolu_multi_1", + name: "read_file", + input: { path: "file1.txt" }, + } + const toolUseBlock2 = { + type: "tool_use" as const, + id: "toolu_multi_2", + name: "read_file", + input: { path: "file2.txt" }, + } + const toolResultBlock1 = { + type: "tool_result" as const, + tool_use_id: "toolu_multi_1", + content: "contents 1", + } + const toolResultBlock2 = { + type: "tool_result" as const, + tool_use_id: "toolu_multi_2", + content: "contents 2", + } + + const messages: ApiMessage[] = [ + { role: "user", content: "Start", ts: 1 }, + { + role: "assistant", + content: [{ type: "text" as const, text: "Reading file 1..." }, toolUseBlock1], + ts: 2, + }, + { role: "user", content: "Some message", ts: 3 }, + { + role: "assistant", + content: [{ type: "text" as const, text: "Reading file 2..." }, toolUseBlock2], + ts: 4, + }, + { + role: "user", + content: [toolResultBlock1, { type: "text" as const, text: "First result" }], + ts: 5, + }, + { + role: "user", + content: [toolResultBlock2, { type: "text" as const, text: "Second result" }], + ts: 6, + }, + { role: "assistant", content: "Got both files", ts: 7 }, + ] + + const result = getKeepMessagesWithToolBlocks(messages, 3) + + // keepMessages should be the last 3 messages (ts: 5, 6, 7) + expect(result.keepMessages).toHaveLength(3) + + // Should preserve both tool_use blocks + expect(result.toolUseBlocksToPreserve).toHaveLength(2) + expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock1) + expect(result.toolUseBlocksToPreserve).toContainEqual(toolUseBlock2) + }) + + it("should not crash when tool_result references tool_use beyond search boundary", () => { + const toolResultBlock = { + type: "tool_result" as const, + tool_use_id: "toolu_beyond_boundary", + content: "result", + } + + // Tool_use is at ts:1, but with N_MESSAGES_TO_KEEP=3, we only search back 3 messages + // from startIndex-1. StartIndex is 7 (messages.length=10, keepCount=3, startIndex=7). + // So we search from index 6 down to index 4 (7-1 down to 7-3). + // The tool_use at index 0 (ts:1) is beyond the search boundary. + const messages: ApiMessage[] = [ + { + role: "assistant", + content: [ + { type: "text" as const, text: "Way back..." }, + { + type: "tool_use" as const, + id: "toolu_beyond_boundary", + name: "old_tool", + input: {}, + }, + ], + ts: 1, + }, + { role: "user", content: "Message 2", ts: 2 }, + { role: "assistant", content: "Message 3", ts: 3 }, + { role: "user", content: "Message 4", ts: 4 }, + { role: "assistant", content: "Message 5", ts: 5 }, + { role: "user", content: "Message 6", ts: 6 }, + { role: "assistant", content: "Message 7", ts: 7 }, + { + role: "user", + content: [toolResultBlock], + ts: 8, + }, + { role: "assistant", content: "Message 9", ts: 9 }, + { role: "user", content: "Message 10", ts: 10 }, + ] + + // Should not crash + const result = getKeepMessagesWithToolBlocks(messages, 3) + + // keepMessages should be the last 3 messages + expect(result.keepMessages).toHaveLength(3) + expect(result.keepMessages[0].ts).toBe(8) + expect(result.keepMessages[1].ts).toBe(9) + expect(result.keepMessages[2].ts).toBe(10) + + // Should not preserve the tool_use since it's beyond the search boundary + expect(result.toolUseBlocksToPreserve).toHaveLength(0) + }) + + it("should not duplicate tool_use blocks when same tool_result ID appears multiple times", () => { + const toolUseBlock = { + type: "tool_use" as const, + id: "toolu_duplicate", + name: "read_file", + input: { path: "test.txt" }, + } + const toolResultBlock1 = { + type: "tool_result" as const, + tool_use_id: "toolu_duplicate", + content: "result 1", + } + const toolResultBlock2 = { + type: "tool_result" as const, + tool_use_id: "toolu_duplicate", + content: "result 2", + } + + const messages: ApiMessage[] = [ + { role: "user", content: "Start", ts: 1 }, + { + role: "assistant", + content: [{ type: "text" as const, text: "Using tool..." }, toolUseBlock], + ts: 2, + }, + { + role: "user", + content: [toolResultBlock1], + ts: 3, + }, + { role: "assistant", content: "Processing", ts: 4 }, + { + role: "user", + content: [toolResultBlock2], // Same tool_use_id as first result + ts: 5, + }, + ] + + const result = getKeepMessagesWithToolBlocks(messages, 3) + + // keepMessages should be the last 3 messages (ts: 3, 4, 5) + expect(result.keepMessages).toHaveLength(3) + + // Should only preserve the tool_use block once, not twice + expect(result.toolUseBlocksToPreserve).toHaveLength(1) + expect(result.toolUseBlocksToPreserve[0]).toEqual(toolUseBlock) + }) }) describe("getMessagesSinceLastSummary", () => { diff --git a/src/core/condense/index.ts b/src/core/condense/index.ts index 3238eca707..79bc31ef9f 100644 --- a/src/core/condense/index.ts +++ b/src/core/condense/index.ts @@ -7,6 +7,7 @@ import { t } from "../../i18n" import { ApiHandler } from "../../api" import { ApiMessage } from "../task-persistence/apiMessages" import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning" +import { findLast } from "../../shared/array" /** * Checks if a message contains tool_result blocks. @@ -30,6 +31,28 @@ function getToolUseBlocks(message: ApiMessage): Anthropic.Messages.ToolUseBlock[ return message.content.filter((block) => block.type === "tool_use") as Anthropic.Messages.ToolUseBlock[] } +/** + * Gets the tool_result blocks from a message. + */ +function getToolResultBlocks(message: ApiMessage): Anthropic.ToolResultBlockParam[] { + if (message.role !== "user" || typeof message.content === "string") { + return [] + } + return message.content.filter((block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result") +} + +/** + * Finds a tool_use block by ID in a message. + */ +function findToolUseBlockById(message: ApiMessage, toolUseId: string): Anthropic.Messages.ToolUseBlock | undefined { + if (message.role !== "assistant" || typeof message.content === "string") { + return undefined + } + return message.content.find( + (block): block is Anthropic.Messages.ToolUseBlock => block.type === "tool_use" && block.id === toolUseId, + ) +} + /** * Gets reasoning blocks from a message's content array. * Task stores reasoning as {type: "reasoning", text: "..."} blocks, @@ -57,11 +80,11 @@ export type KeepMessagesResult = { /** * Extracts tool_use blocks that need to be preserved to match tool_result blocks in keepMessages. - * When the first kept message is a user message with tool_result blocks, - * we need to find the corresponding tool_use blocks from the preceding assistant message. + * Checks ALL kept messages for tool_result blocks and searches backwards through the condensed + * region (bounded by N_MESSAGES_TO_KEEP) to find the matching tool_use blocks by ID. * These tool_use blocks will be appended to the summary message to maintain proper pairing. * - * Also extracts reasoning blocks from the preceding assistant message, which are required + * Also extracts reasoning blocks from messages containing preserved tool_uses, which are required * by DeepSeek and Z.ai for interleaved thinking mode. Without these, the API returns a 400 error * "Missing reasoning_content field in the assistant message". * See: https://api-docs.deepseek.com/guides/thinking_mode#tool-calls @@ -78,28 +101,53 @@ export function getKeepMessagesWithToolBlocks(messages: ApiMessage[], keepCount: const startIndex = messages.length - keepCount const keepMessages = messages.slice(startIndex) - // Check if the first kept message is a user message with tool_result blocks - if (keepMessages.length > 0 && hasToolResultBlocks(keepMessages[0])) { - // Look for the preceding assistant message with tool_use blocks - const precedingIndex = startIndex - 1 - if (precedingIndex >= 0) { - const precedingMessage = messages[precedingIndex] - const toolUseBlocks = getToolUseBlocks(precedingMessage) - if (toolUseBlocks.length > 0) { - // Also extract reasoning blocks for DeepSeek/Z.ai interleaved thinking - // Task stores reasoning as {type: "reasoning", text: "..."} content blocks - const reasoningBlocks = getReasoningBlocks(precedingMessage) - // Return the tool_use blocks and reasoning blocks to be merged into the summary message - return { - keepMessages, - toolUseBlocksToPreserve: toolUseBlocks, - reasoningBlocksToPreserve: reasoningBlocks, - } + const toolUseBlocksToPreserve: Anthropic.Messages.ToolUseBlock[] = [] + const reasoningBlocksToPreserve: Anthropic.Messages.ContentBlockParam[] = [] + const preservedToolUseIds = new Set() + + // Check ALL kept messages for tool_result blocks + for (const keepMsg of keepMessages) { + if (!hasToolResultBlocks(keepMsg)) { + continue + } + + const toolResults = getToolResultBlocks(keepMsg) + + for (const toolResult of toolResults) { + const toolUseId = toolResult.tool_use_id + + // Skip if we've already found this tool_use + if (preservedToolUseIds.has(toolUseId)) { + continue + } + + // Search backwards through the condensed region (bounded) + const searchStart = startIndex - 1 + const searchEnd = Math.max(0, startIndex - N_MESSAGES_TO_KEEP) + const messagesToSearch = messages.slice(searchEnd, searchStart + 1) + + // Find the message containing this tool_use + const messageWithToolUse = findLast(messagesToSearch, (msg) => { + return findToolUseBlockById(msg, toolUseId) !== undefined + }) + + if (messageWithToolUse) { + const toolUse = findToolUseBlockById(messageWithToolUse, toolUseId)! + toolUseBlocksToPreserve.push(toolUse) + preservedToolUseIds.add(toolUseId) + + // Also preserve reasoning blocks from that message + const reasoning = getReasoningBlocks(messageWithToolUse) + reasoningBlocksToPreserve.push(...reasoning) } } } - return { keepMessages, toolUseBlocksToPreserve: [], reasoningBlocksToPreserve: [] } + return { + keepMessages, + toolUseBlocksToPreserve, + reasoningBlocksToPreserve, + } } export const N_MESSAGES_TO_KEEP = 3