Răsfoiți Sursa

fix: add missing tool_result blocks to prevent API errors (#10015)

Daniel 1 lună în urmă
părinte
comite
87317091ee

+ 305 - 4
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<Anthropic.ToolResultBlockParam | Anthropic.TextBlockParam>
+			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<Anthropic.ToolResultBlockParam | Anthropic.TextBlockParam>
+			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()
+		})
+	})
 })

+ 80 - 22
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,