Преглед на файлове

fix: add explicit deduplication for duplicate tool_result blocks (#10466)

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>
roomote[bot] преди 3 седмици
родител
ревизия
f2276bebc4
променени са 4 файла, в които са добавени 131 реда и са изтрити 12 реда
  1. 4 0
      src/core/task/Task.ts
  2. 90 0
      src/core/task/__tests__/validateToolResultIds.spec.ts
  3. 34 12
      src/core/task/validateToolResultIds.ts
  4. 3 0
      src/core/tools/ExecuteCommandTool.ts

+ 4 - 0
src/core/task/Task.ts

@@ -1354,6 +1354,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		this.handleWebviewAskResponse("noButtonClicked", text, images)
 	}
 
+	public supersedePendingAsk(): void {
+		this.lastMessageTs = Date.now()
+	}
+
 	/**
 	 * Updates the API configuration but preserves the locked tool protocol.
 	 * The task's tool protocol is locked at creation time and should NOT change

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

@@ -482,6 +482,96 @@ describe("validateAndFixToolResultIds", () => {
 			expect(resultContent[1].type).toBe("text")
 			expect((resultContent[1] as Anthropic.TextBlockParam).text).toBe("Some additional context")
 		})
+
+		// Verifies fix for GitHub #10465: Terminal fallback race condition can generate
+		// duplicate tool_results with the same valid tool_use_id, causing API protocol violations.
+		it("should filter out duplicate tool_results with identical valid tool_use_ids (terminal fallback scenario)", () => {
+			const assistantMessage: Anthropic.MessageParam = {
+				role: "assistant",
+				content: [
+					{
+						type: "tool_use",
+						id: "tooluse_QZ-pU8v2QKO8L8fHoJRI2g",
+						name: "execute_command",
+						input: { command: "ps aux | grep test", cwd: "/path/to/project" },
+					},
+				],
+			}
+
+			// Two tool_results with the SAME valid tool_use_id from terminal fallback race condition
+			const userMessage: Anthropic.MessageParam = {
+				role: "user",
+				content: [
+					{
+						type: "tool_result",
+						tool_use_id: "tooluse_QZ-pU8v2QKO8L8fHoJRI2g", // First result from command execution
+						content: "No test processes found",
+					},
+					{
+						type: "tool_result",
+						tool_use_id: "tooluse_QZ-pU8v2QKO8L8fHoJRI2g", // Duplicate from user approval during fallback
+						content: '{"status":"approved","message":"The user approved this operation"}',
+					},
+				],
+			}
+
+			const result = validateAndFixToolResultIds(userMessage, [assistantMessage])
+
+			expect(Array.isArray(result.content)).toBe(true)
+			const resultContent = result.content as Anthropic.ToolResultBlockParam[]
+
+			// Only ONE tool_result should remain to prevent API protocol violation
+			expect(resultContent.length).toBe(1)
+			expect(resultContent[0].tool_use_id).toBe("tooluse_QZ-pU8v2QKO8L8fHoJRI2g")
+			expect(resultContent[0].content).toBe("No test processes found")
+		})
+
+		it("should preserve text blocks while deduplicating tool_results with same valid ID", () => {
+			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: "First result",
+					},
+					{
+						type: "text",
+						text: "Environment details here",
+					},
+					{
+						type: "tool_result",
+						tool_use_id: "tool-123", // Duplicate with same valid ID
+						content: "Duplicate result from fallback",
+					},
+				],
+			}
+
+			const result = validateAndFixToolResultIds(userMessage, [assistantMessage])
+
+			expect(Array.isArray(result.content)).toBe(true)
+			const resultContent = result.content as Array<Anthropic.ToolResultBlockParam | Anthropic.TextBlockParam>
+
+			// Should have: 1 tool_result + 1 text block (duplicate 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-123")
+			expect((resultContent[0] as Anthropic.ToolResultBlockParam).content).toBe("First result")
+			expect(resultContent[1].type).toBe("text")
+			expect((resultContent[1] as Anthropic.TextBlockParam).text).toBe("Environment details here")
+		})
 	})
 
 	describe("when there are more tool_uses than tool_results", () => {

+ 34 - 12
src/core/task/validateToolResultIds.ts

@@ -78,7 +78,31 @@ export function validateAndFixToolResultIds(
 	}
 
 	// Find tool_result blocks in the user message
-	const toolResults = userMessage.content.filter(
+	let toolResults = userMessage.content.filter(
+		(block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result",
+	)
+
+	// Deduplicate tool_result blocks to prevent API protocol violations (GitHub #10465)
+	// Terminal fallback race conditions can generate duplicate tool_results with the same tool_use_id.
+	// Filter out duplicates before validation since Set-based checks below would miss them.
+	const seenToolResultIds = new Set<string>()
+	const deduplicatedContent = userMessage.content.filter((block) => {
+		if (block.type !== "tool_result") {
+			return true
+		}
+		if (seenToolResultIds.has(block.tool_use_id)) {
+			return false // Duplicate - filter out
+		}
+		seenToolResultIds.add(block.tool_use_id)
+		return true
+	})
+
+	userMessage = {
+		...userMessage,
+		content: deduplicatedContent,
+	}
+
+	toolResults = deduplicatedContent.filter(
 		(block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result",
 	)
 
@@ -139,15 +163,12 @@ 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
-	//
-	// Track which tool_use IDs have been used to prevent duplicates
+	// Match tool_results to tool_uses by position and fix incorrect IDs
 	const usedToolUseIds = new Set<string>()
+	const contentArray = userMessage.content as Anthropic.Messages.ContentBlockParam[]
 
-	const correctedContent = userMessage.content
-		.map((block) => {
+	const correctedContent = contentArray
+		.map((block: Anthropic.Messages.ContentBlockParam) => {
 			if (block.type !== "tool_result") {
 				return block
 			}
@@ -177,17 +198,18 @@ export function validateAndFixToolResultIds(
 			}
 
 			// 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
 	const coveredToolUseIds = new Set(
 		correctedContent
-			.filter((b): b is Anthropic.ToolResultBlockParam => b.type === "tool_result")
-			.map((r) => r.tool_use_id),
+			.filter(
+				(b: Anthropic.Messages.ContentBlockParam): b is Anthropic.ToolResultBlockParam =>
+					b.type === "tool_result",
+			)
+			.map((r: Anthropic.ToolResultBlockParam) => r.tool_use_id),
 	)
 
 	const stillMissingToolUseIds = toolUseBlocks.filter((toolUse) => !coveredToolUseIds.has(toolUse.id))

+ 3 - 0
src/core/tools/ExecuteCommandTool.ts

@@ -116,6 +116,9 @@ export class ExecuteCommandTool extends BaseTool<"execute_command"> {
 				provider?.postMessageToWebview({ type: "commandExecutionStatus", text: JSON.stringify(status) })
 				await task.say("shell_integration_warning")
 
+				// Invalidate pending ask from first execution to prevent race condition
+				task.supersedePendingAsk()
+
 				if (error instanceof ShellIntegrationError) {
 					const [rejected, result] = await executeCommandInTerminal(task, {
 						...options,