diff --git a/src/core/task/__tests__/validateToolResultIds.spec.ts b/src/core/task/__tests__/validateToolResultIds.spec.ts index 67aaf238952..b457c3cd2e2 100644 --- a/src/core/task/__tests__/validateToolResultIds.spec.ts +++ b/src/core/task/__tests__/validateToolResultIds.spec.ts @@ -1,6 +1,10 @@ import { Anthropic } from "@anthropic-ai/sdk" import { TelemetryService } from "@roo-code/telemetry" -import { validateAndFixToolResultIds, ToolResultIdMismatchError } from "../validateToolResultIds" +import { + validateAndFixToolResultIds, + ToolResultIdMismatchError, + MissingToolResultError, +} from "../validateToolResultIds" // Mock TelemetryService vi.mock("@roo-code/telemetry", () => ({ @@ -394,7 +398,7 @@ describe("validateAndFixToolResultIds", () => { }) describe("when there are more tool_uses than tool_results", () => { - it("should fix the available tool_results", () => { + it("should fix the available tool_results and add missing ones", () => { const assistantMessage: Anthropic.MessageParam = { role: "assistant", content: [ @@ -426,15 +430,174 @@ describe("validateAndFixToolResultIds", () => { const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Anthropic.ToolResultBlockParam[] + // Should now have 2 tool_results: one fixed and one added for the missing tool_use + expect(resultContent.length).toBe(2) + // The missing tool_result is prepended + expect(resultContent[0].tool_use_id).toBe("tool-2") + expect(resultContent[0].content).toBe("Tool execution was interrupted before completion.") + // The original is fixed + expect(resultContent[1].tool_use_id).toBe("tool-1") + }) + }) + + describe("when tool_results are completely missing", () => { + it("should add missing tool_result for single tool_use", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-123", + name: "read_file", + input: { path: "test.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "text", + text: "Some user message without tool results", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Array + expect(resultContent.length).toBe(2) + // Missing tool_result should be prepended + expect(resultContent[0].type).toBe("tool_result") + expect((resultContent[0] as Anthropic.ToolResultBlockParam).tool_use_id).toBe("tool-123") + expect((resultContent[0] as Anthropic.ToolResultBlockParam).content).toBe( + "Tool execution was interrupted before completion.", + ) + // Original text block should be preserved + expect(resultContent[1].type).toBe("text") + }) + + it("should add missing tool_results for multiple tool_uses", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-1", + name: "read_file", + input: { path: "a.txt" }, + }, + { + type: "tool_use", + id: "tool-2", + name: "write_to_file", + input: { path: "b.txt", content: "test" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "text", + text: "User message", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Array + expect(resultContent.length).toBe(3) + // Both missing tool_results should be prepended + expect(resultContent[0].type).toBe("tool_result") + expect((resultContent[0] as Anthropic.ToolResultBlockParam).tool_use_id).toBe("tool-1") + expect(resultContent[1].type).toBe("tool_result") + expect((resultContent[1] as Anthropic.ToolResultBlockParam).tool_use_id).toBe("tool-2") + // Original text should be preserved + expect(resultContent[2].type).toBe("text") + }) + + it("should add only the missing tool_results when some exist", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-1", + name: "read_file", + input: { path: "a.txt" }, + }, + { + type: "tool_use", + id: "tool-2", + name: "write_to_file", + input: { path: "b.txt", content: "test" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "tool-1", + content: "Content for tool 1", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Anthropic.ToolResultBlockParam[] + expect(resultContent.length).toBe(2) + // Missing tool_result for tool-2 should be prepended + expect(resultContent[0].tool_use_id).toBe("tool-2") + expect(resultContent[0].content).toBe("Tool execution was interrupted before completion.") + // Existing tool_result should be preserved + expect(resultContent[1].tool_use_id).toBe("tool-1") + expect(resultContent[1].content).toBe("Content for tool 1") + }) + + it("should handle empty user content array by adding all missing tool_results", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-1", + name: "read_file", + input: { path: "test.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [], + } + + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) + expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] expect(resultContent.length).toBe(1) + expect(resultContent[0].type).toBe("tool_result") expect(resultContent[0].tool_use_id).toBe("tool-1") + expect(resultContent[0].content).toBe("Tool execution was interrupted before completion.") }) }) describe("telemetry", () => { - it("should call captureException when there is a mismatch", () => { + it("should call captureException for both missing and mismatch when there is a mismatch", () => { const assistantMessage: Anthropic.MessageParam = { role: "assistant", content: [ @@ -460,7 +623,17 @@ describe("validateAndFixToolResultIds", () => { validateAndFixToolResultIds(userMessage, [assistantMessage]) - expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(1) + // A mismatch also triggers missing detection since the wrong-id doesn't match any tool_use + expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( + expect.any(MissingToolResultError), + expect.objectContaining({ + missingToolUseIds: ["correct-id"], + existingToolResultIds: ["wrong-id"], + toolUseCount: 1, + toolResultCount: 1, + }), + ) expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( expect.any(ToolResultIdMismatchError), expect.objectContaining({ @@ -516,4 +689,132 @@ describe("validateAndFixToolResultIds", () => { expect(error.toolUseIds).toEqual(["use-1", "use-2"]) }) }) + + describe("MissingToolResultError", () => { + it("should create error with correct properties", () => { + const error = new MissingToolResultError( + "Missing tool results detected", + ["tool-1", "tool-2"], + ["existing-result-1"], + ) + + expect(error.name).toBe("MissingToolResultError") + expect(error.message).toBe("Missing tool results detected") + expect(error.missingToolUseIds).toEqual(["tool-1", "tool-2"]) + expect(error.existingToolResultIds).toEqual(["existing-result-1"]) + }) + }) + + describe("telemetry for missing tool_results", () => { + it("should call captureException when tool_results are missing", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-123", + name: "read_file", + input: { path: "test.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "text", + text: "No tool results here", + }, + ], + } + + validateAndFixToolResultIds(userMessage, [assistantMessage]) + + expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(1) + expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( + expect.any(MissingToolResultError), + expect.objectContaining({ + missingToolUseIds: ["tool-123"], + existingToolResultIds: [], + toolUseCount: 1, + toolResultCount: 0, + }), + ) + }) + + it("should call captureException twice when both mismatch and missing occur", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-1", + name: "read_file", + input: { path: "a.txt" }, + }, + { + type: "tool_use", + id: "tool-2", + name: "read_file", + input: { path: "b.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "wrong-id", // Wrong ID (mismatch) + content: "Content", + }, + // Missing tool_result for tool-2 + ], + } + + validateAndFixToolResultIds(userMessage, [assistantMessage]) + + // Should be called twice: once for missing, once for mismatch + expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(2) + expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( + expect.any(MissingToolResultError), + expect.any(Object), + ) + expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( + expect.any(ToolResultIdMismatchError), + expect.any(Object), + ) + }) + + it("should not call captureException for missing when all tool_results exist", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "tool-123", + name: "read_file", + input: { path: "test.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "tool-123", + content: "Content", + }, + ], + } + + validateAndFixToolResultIds(userMessage, [assistantMessage]) + + expect(TelemetryService.instance.captureException).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts index 159caca79d8..abedfc8c3a4 100644 --- a/src/core/task/validateToolResultIds.ts +++ b/src/core/task/validateToolResultIds.ts @@ -17,18 +17,35 @@ export class ToolResultIdMismatchError extends Error { } } +/** + * Custom error class for missing tool results. + * Used for structured error tracking via PostHog when tool_use blocks + * don't have corresponding tool_result blocks. + */ +export class MissingToolResultError extends Error { + constructor( + message: string, + public readonly missingToolUseIds: string[], + public readonly existingToolResultIds: string[], + ) { + super(message) + this.name = "MissingToolResultError" + } +} + /** * Validates and fixes tool_result IDs in a user message against the previous assistant message. * - * This is a centralized validation that catches all tool_use/tool_result ID mismatches + * This is a centralized validation that catches all tool_use/tool_result issues * before messages are added to the API conversation history. It handles scenarios like: * - Race conditions during streaming * - Message editing scenarios * - Resume/delegation scenarios + * - Missing tool_result blocks for tool_use calls * * @param userMessage - The user message being added to history * @param apiConversationHistory - The conversation history to find the previous assistant message from - * @returns The validated user message with corrected tool_use_ids + * @returns The validated user message with corrected tool_use_ids and any missing tool_results added */ export function validateAndFixToolResultIds( userMessage: Anthropic.MessageParam, @@ -39,16 +56,6 @@ export function validateAndFixToolResultIds( return userMessage } - // Find tool_result blocks in the user message - const toolResults = userMessage.content.filter( - (block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result", - ) - - // No tool results to validate - if (toolResults.length === 0) { - return userMessage - } - // Find the previous assistant message from conversation history const prevAssistantIdx = findLastIndex(apiConversationHistory, (msg) => msg.role === "assistant") if (prevAssistantIdx === -1) { @@ -65,28 +72,58 @@ export function validateAndFixToolResultIds( const toolUseBlocks = assistantContent.filter((block): block is Anthropic.ToolUseBlock => block.type === "tool_use") - // No tool_use blocks to match against + // No tool_use blocks to match against - no validation needed if (toolUseBlocks.length === 0) { return userMessage } + // Find tool_result blocks in the user message + const toolResults = userMessage.content.filter( + (block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result", + ) + // Build a set of valid tool_use IDs const validToolUseIds = new Set(toolUseBlocks.map((block) => block.id)) + // Build a set of existing tool_result IDs + const existingToolResultIds = new Set(toolResults.map((r) => r.tool_use_id)) + + // Check for missing tool_results (tool_use IDs that don't have corresponding tool_results) + const missingToolUseIds = toolUseBlocks + .filter((toolUse) => !existingToolResultIds.has(toolUse.id)) + .map((toolUse) => toolUse.id) + // Check if any tool_result has an invalid ID const hasInvalidIds = toolResults.some((result) => !validToolUseIds.has(result.tool_use_id)) - if (!hasInvalidIds) { - // All IDs are valid, no changes needed + // If no missing tool_results and no invalid IDs, no changes needed + if (missingToolUseIds.length === 0 && !hasInvalidIds) { return userMessage } - // We have mismatches - need to fix them + // We have issues - need to fix them const toolResultIdList = toolResults.map((r) => r.tool_use_id) const toolUseIdList = toolUseBlocks.map((b) => b.id) - // Report the mismatch to PostHog error tracking - if (TelemetryService.hasInstance()) { + // Report missing tool_results to PostHog error tracking + if (missingToolUseIds.length > 0 && TelemetryService.hasInstance()) { + TelemetryService.instance.captureException( + new MissingToolResultError( + `Detected missing tool_result blocks. Missing tool_use IDs: [${missingToolUseIds.join(", ")}], existing tool_result IDs: [${toolResultIdList.join(", ")}]`, + missingToolUseIds, + toolResultIdList, + ), + { + missingToolUseIds, + existingToolResultIds: toolResultIdList, + toolUseCount: toolUseBlocks.length, + toolResultCount: toolResults.length, + }, + ) + } + + // Report ID mismatches to PostHog error tracking + if (hasInvalidIds && TelemetryService.hasInstance()) { TelemetryService.instance.captureException( new ToolResultIdMismatchError( `Detected tool_result ID mismatch. tool_result IDs: [${toolResultIdList.join(", ")}], tool_use IDs: [${toolUseIdList.join(", ")}]`, @@ -102,10 +139,8 @@ export function validateAndFixToolResultIds( ) } - // Create a mapping of tool_result IDs to corrected IDs - // Strategy: Match by position (first tool_result -> first tool_use, etc.) - // This handles most cases where the mismatch is due to ID confusion - const correctedContent = userMessage.content.map((block) => { + // Start with corrected content - fix invalid IDs + let correctedContent = userMessage.content.map((block) => { if (block.type !== "tool_result") { return block } @@ -134,6 +169,29 @@ export function validateAndFixToolResultIds( return block }) + // Add missing tool_result blocks for any tool_use that doesn't have one + // After the ID correction above, recalculate which tool_use IDs are now covered + const coveredToolUseIds = new Set( + correctedContent + .filter((b): b is Anthropic.ToolResultBlockParam => b.type === "tool_result") + .map((r) => r.tool_use_id), + ) + + const stillMissingToolUseIds = toolUseBlocks.filter((toolUse) => !coveredToolUseIds.has(toolUse.id)) + + if (stillMissingToolUseIds.length > 0) { + // Add placeholder tool_result blocks for missing tool_use IDs + const missingToolResults: Anthropic.ToolResultBlockParam[] = stillMissingToolUseIds.map((toolUse) => ({ + type: "tool_result" as const, + tool_use_id: toolUse.id, + content: "Tool execution was interrupted before completion.", + })) + + // Insert missing tool_results at the beginning of the content array + // This ensures they come before any text blocks that may summarize the results + correctedContent = [...missingToolResults, ...correctedContent] + } + return { ...userMessage, content: correctedContent,