2
0
Эх сурвалжийг харах

fix: filter orphaned tool_results when more results than tool_uses (#10027)

Daniel 3 долоо хоног өмнө
parent
commit
8a68b04c27

+ 90 - 3
src/core/task/__tests__/validateToolResultIds.spec.ts

@@ -358,7 +358,7 @@ describe("validateAndFixToolResultIds", () => {
 	})
 
 	describe("when there are more tool_results than tool_uses", () => {
-		it("should leave extra tool_results unchanged", () => {
+		it("should filter out orphaned tool_results with invalid IDs", () => {
 			const assistantMessage: Anthropic.MessageParam = {
 				role: "assistant",
 				content: [
@@ -391,9 +391,96 @@ describe("validateAndFixToolResultIds", () => {
 
 			expect(Array.isArray(result.content)).toBe(true)
 			const resultContent = result.content as Anthropic.ToolResultBlockParam[]
+			// Only one tool_result should remain - the first one gets fixed to tool-1
+			expect(resultContent.length).toBe(1)
 			expect(resultContent[0].tool_use_id).toBe("tool-1")
-			// Extra tool_result should remain unchanged
-			expect(resultContent[1].tool_use_id).toBe("extra-id")
+		})
+
+		it("should filter out duplicate tool_results when one already has a valid ID", () => {
+			// This is the exact scenario from the PostHog error:
+			// 2 tool_results (call_08230257, call_55577629), 1 tool_use (call_55577629)
+			const assistantMessage: Anthropic.MessageParam = {
+				role: "assistant",
+				content: [
+					{
+						type: "tool_use",
+						id: "call_55577629",
+						name: "read_file",
+						input: { path: "test.txt" },
+					},
+				],
+			}
+
+			const userMessage: Anthropic.MessageParam = {
+				role: "user",
+				content: [
+					{
+						type: "tool_result",
+						tool_use_id: "call_08230257", // Invalid ID
+						content: "Content from first result",
+					},
+					{
+						type: "tool_result",
+						tool_use_id: "call_55577629", // Valid ID
+						content: "Content from second result",
+					},
+				],
+			}
+
+			const result = validateAndFixToolResultIds(userMessage, [assistantMessage])
+
+			expect(Array.isArray(result.content)).toBe(true)
+			const resultContent = result.content as Anthropic.ToolResultBlockParam[]
+			// Should only keep one tool_result since there's only one tool_use
+			// The first invalid one gets fixed to the valid ID, then the second one
+			// (which already has that ID) becomes a duplicate and is filtered out
+			expect(resultContent.length).toBe(1)
+			expect(resultContent[0].tool_use_id).toBe("call_55577629")
+		})
+
+		it("should preserve text blocks while filtering orphaned 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: [
+					{
+						type: "tool_result",
+						tool_use_id: "wrong-1",
+						content: "Content 1",
+					},
+					{
+						type: "text",
+						text: "Some additional context",
+					},
+					{
+						type: "tool_result",
+						tool_use_id: "extra-id",
+						content: "Content 2",
+					},
+				],
+			}
+
+			const result = validateAndFixToolResultIds(userMessage, [assistantMessage])
+
+			expect(Array.isArray(result.content)).toBe(true)
+			const resultContent = result.content as Array<Anthropic.ToolResultBlockParam | Anthropic.TextBlockParam>
+			// Should have tool_result + text block, orphaned tool_result filtered out
+			expect(resultContent.length).toBe(2)
+			expect(resultContent[0].type).toBe("tool_result")
+			expect((resultContent[0] as Anthropic.ToolResultBlockParam).tool_use_id).toBe("tool-1")
+			expect(resultContent[1].type).toBe("text")
+			expect((resultContent[1] as Anthropic.TextBlockParam).text).toBe("Some additional context")
 		})
 	})
 

+ 51 - 40
src/core/task/validateToolResultIds.ts

@@ -139,35 +139,48 @@ export function validateAndFixToolResultIds(
 		)
 	}
 
-	// Start with corrected content - fix invalid IDs
-	let correctedContent = userMessage.content.map((block) => {
-		if (block.type !== "tool_result") {
-			return block
-		}
-
-		// If the ID is already valid, keep it
-		if (validToolUseIds.has(block.tool_use_id)) {
-			return block
-		}
-
-		// Find which tool_result index this block is by comparing references.
-		// This correctly handles duplicate tool_use_ids - we find the actual block's
-		// position among all tool_results, not the first block with a matching ID.
-		const toolResultIndex = toolResults.indexOf(block as Anthropic.ToolResultBlockParam)
-
-		// Try to match by position - only fix if there's a corresponding tool_use
-		if (toolResultIndex !== -1 && toolResultIndex < toolUseBlocks.length) {
-			const correctId = toolUseBlocks[toolResultIndex].id
-			return {
-				...block,
-				tool_use_id: correctId,
+	// 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
+	//
+	// Track which tool_use IDs have been used to prevent duplicates
+	const usedToolUseIds = new Set<string>()
+
+	const correctedContent = userMessage.content
+		.map((block) => {
+			if (block.type !== "tool_result") {
+				return block
 			}
-		}
 
-		// No corresponding tool_use for this tool_result - leave it unchanged
-		// This can happen when there are more tool_results than tool_uses
-		return block
-	})
+			// If the ID is already valid and not yet used, keep it
+			if (validToolUseIds.has(block.tool_use_id) && !usedToolUseIds.has(block.tool_use_id)) {
+				usedToolUseIds.add(block.tool_use_id)
+				return block
+			}
+
+			// Find which tool_result index this block is by comparing references.
+			// This correctly handles duplicate tool_use_ids - we find the actual block's
+			// position among all tool_results, not the first block with a matching ID.
+			const toolResultIndex = toolResults.indexOf(block as Anthropic.ToolResultBlockParam)
+
+			// Try to match by position - only fix if there's a corresponding tool_use
+			if (toolResultIndex !== -1 && toolResultIndex < toolUseBlocks.length) {
+				const correctId = toolUseBlocks[toolResultIndex].id
+				// Only use this ID if it hasn't been used yet
+				if (!usedToolUseIds.has(correctId)) {
+					usedToolUseIds.add(correctId)
+					return {
+						...block,
+						tool_use_id: correctId,
+					}
+				}
+			}
+
+			// No corresponding tool_use for this tool_result, or the ID is already used
+			// Filter out this orphaned tool_result by returning null
+			return null
+		})
+		.filter((block): block is NonNullable<typeof block> => block !== null)
 
 	// 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
@@ -179,21 +192,19 @@ export function validateAndFixToolResultIds(
 
 	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]
-	}
+	// Build final content: add missing tool_results at the beginning if any
+	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
+	const finalContent = missingToolResults.length > 0 ? [...missingToolResults, ...correctedContent] : correctedContent
 
 	return {
 		...userMessage,
-		content: correctedContent,
+		content: finalContent,
 	}
 }