From b7f7a41e21cb116687a9d9b66a89f9d26da55968 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 13:16:05 -0500 Subject: [PATCH 1/8] fix: validate and fix tool_result IDs before API requests Fixes ApiError: Expected toolResult blocks for mismatched IDs ## Problem Tool_result blocks sometimes had incorrect tool_use_id values, causing the Anthropic API to reject requests with errors like: "Expected toolResult blocks at messages.6.content for the following Ids: toolu_ABC, but found: toolu_XYZ" ## Solution Added centralized validation in addToApiConversationHistory() that: - Validates tool_result IDs against tool_use IDs from previous assistant message - Fixes mismatches using position-based matching - Logs warnings for debugging when corrections are made ## Changes - Added src/core/task/validateToolResultIds.ts with validation logic - Added src/core/task/__tests__/validateToolResultIds.spec.ts (12 tests) - Modified src/core/task/Task.ts to call validation - Simplified src/core/assistant-message/presentAssistantMessage.ts by removing redundant source-level validation --- .../presentAssistantMessage.ts | 3 +- src/core/task/Task.ts | 19 +- .../__tests__/validateToolResultIds.spec.ts | 420 ++++++++++++++++++ src/core/task/validateToolResultIds.ts | 111 +++++ 4 files changed, 549 insertions(+), 4 deletions(-) create mode 100644 src/core/task/__tests__/validateToolResultIds.spec.ts create mode 100644 src/core/task/validateToolResultIds.ts diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index bcbf0a35f77..cf2c5a9df6e 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -42,8 +42,6 @@ import { Task } from "../task/Task" import { codebaseSearchTool } from "../tools/CodebaseSearchTool" import { experiments, EXPERIMENT_IDS } from "../../shared/experiments" import { applyDiffTool as applyDiffToolClass } from "../tools/ApplyDiffTool" -import { isNativeProtocol } from "@roo-code/types" -import { resolveToolProtocol } from "../../utils/resolveToolProtocol" /** * Processes and presents assistant message content to the user interface. @@ -525,6 +523,7 @@ export async function presentAssistantMessage(cline: Task) { } // Add tool_result with text content only + // Note: tool_use_id validation happens centrally in addToApiConversationHistory cline.userMessageContent.push({ type: "tool_result", tool_use_id: toolCallId, diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 488562e8f9a..c6304d9388f 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -129,6 +129,7 @@ import { getMessagesSinceLastSummary, summarizeConversation, getEffectiveApiHist import { MessageQueueService } from "../message-queue/MessageQueueService" import { AutoApprovalHandler, checkAutoApproval } from "../auto-approval" import { MessageManager } from "../message-manager" +import { validateAndFixToolResultIds } from "./validateToolResultIds" const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes const DEFAULT_USAGE_COLLECTION_TIMEOUT_MS = 5000 // 5 seconds @@ -811,7 +812,14 @@ export class Task extends EventEmitter implements TaskLike { this.apiConversationHistory.push(messageWithTs) } else { - const messageWithTs = { ...message, ts: Date.now() } + // For user messages, validate and fix tool_result IDs against the previous assistant message + // This is a centralized catch-all that handles all tool_use/tool_result ID mismatches + const previousAssistantMessage = this.apiConversationHistory + .slice() + .reverse() + .find((msg) => msg.role === "assistant") + const validatedMessage = validateAndFixToolResultIds(message, previousAssistantMessage) + const messageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(messageWithTs) } @@ -849,7 +857,14 @@ export class Task extends EventEmitter implements TaskLike { role: "user", content: this.userMessageContent, } - const userMessageWithTs = { ...userMessage, ts: Date.now() } + + // Validate and fix tool_result IDs against the previous assistant message + const previousAssistantMessage = this.apiConversationHistory + .slice() + .reverse() + .find((msg) => msg.role === "assistant") + const validatedMessage = validateAndFixToolResultIds(userMessage, previousAssistantMessage) + const userMessageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(userMessageWithTs as ApiMessage) await this.saveApiConversationHistory() diff --git a/src/core/task/__tests__/validateToolResultIds.spec.ts b/src/core/task/__tests__/validateToolResultIds.spec.ts new file mode 100644 index 00000000000..931711b8284 --- /dev/null +++ b/src/core/task/__tests__/validateToolResultIds.spec.ts @@ -0,0 +1,420 @@ +import { Anthropic } from "@anthropic-ai/sdk" +import { validateAndFixToolResultIds } from "../validateToolResultIds" + +describe("validateAndFixToolResultIds", () => { + describe("when there is no previous assistant message", () => { + it("should return the user message unchanged", () => { + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "tool-123", + content: "Result", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, undefined) + + expect(result).toEqual(userMessage) + }) + }) + + describe("when tool_result IDs match tool_use IDs", () => { + it("should return the user message unchanged for single tool", () => { + 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: "File content", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(result).toEqual(userMessage) + }) + + it("should return the user message unchanged for multiple tools", () => { + 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: "tool-1", + content: "Content A", + }, + { + type: "tool_result", + tool_use_id: "tool-2", + content: "Content B", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(result).toEqual(userMessage) + }) + }) + + describe("when tool_result IDs do not match tool_use IDs", () => { + it("should fix single mismatched tool_use_id by position", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "correct-id-123", + name: "read_file", + input: { path: "test.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "wrong-id-456", + content: "File content", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Anthropic.ToolResultBlockParam[] + expect(resultContent[0].tool_use_id).toBe("correct-id-123") + expect(resultContent[0].content).toBe("File content") + }) + + it("should fix multiple mismatched tool_use_ids by position", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "correct-1", + name: "read_file", + input: { path: "a.txt" }, + }, + { + type: "tool_use", + id: "correct-2", + name: "read_file", + input: { path: "b.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "wrong-1", + content: "Content A", + }, + { + type: "tool_result", + tool_use_id: "wrong-2", + content: "Content B", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Anthropic.ToolResultBlockParam[] + expect(resultContent[0].tool_use_id).toBe("correct-1") + expect(resultContent[1].tool_use_id).toBe("correct-2") + }) + + it("should partially fix when some IDs match and some don't", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "id-1", + name: "read_file", + input: { path: "a.txt" }, + }, + { + type: "tool_use", + id: "id-2", + name: "read_file", + input: { path: "b.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "id-1", // Correct + content: "Content A", + }, + { + type: "tool_result", + tool_use_id: "wrong-id", // Wrong + content: "Content B", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Anthropic.ToolResultBlockParam[] + expect(resultContent[0].tool_use_id).toBe("id-1") + expect(resultContent[1].tool_use_id).toBe("id-2") + }) + }) + + describe("when user message has non-tool_result content", () => { + it("should preserve text blocks alongside tool_result blocks", () => { + 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: "wrong-id", + content: "File content", + }, + { + type: "text", + text: "Additional context", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Array + expect(resultContent[0].type).toBe("tool_result") + expect((resultContent[0] as Anthropic.ToolResultBlockParam).tool_use_id).toBe("tool-123") + expect(resultContent[1].type).toBe("text") + expect((resultContent[1] as Anthropic.TextBlockParam).text).toBe("Additional context") + }) + }) + + describe("when assistant message has non-tool_use content", () => { + it("should only consider tool_use blocks for matching", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "text", + text: "Let me read that file for you.", + }, + { + 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: "wrong-id", + content: "File content", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(Array.isArray(result.content)).toBe(true) + const resultContent = result.content as Anthropic.ToolResultBlockParam[] + expect(resultContent[0].tool_use_id).toBe("tool-123") + }) + }) + + describe("when user message content is a string", () => { + it("should return the message unchanged", () => { + 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: "Just a plain text message", + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(result).toEqual(userMessage) + }) + }) + + describe("when assistant message content is a string", () => { + it("should return the user message unchanged", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: "Just some text, no tool use", + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "tool-123", + content: "Result", + }, + ], + } + + const result = validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(result).toEqual(userMessage) + }) + }) + + describe("when there are more tool_results than tool_uses", () => { + it("should leave extra tool_results unchanged", () => { + 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: "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 Anthropic.ToolResultBlockParam[] + expect(resultContent[0].tool_use_id).toBe("tool-1") + // Extra tool_result should remain unchanged + expect(resultContent[1].tool_use_id).toBe("extra-id") + }) + }) + + describe("when there are more tool_uses than tool_results", () => { + it("should fix the available tool_results", () => { + 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-1", + content: "Content 1", + }, + ], + } + + 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].tool_use_id).toBe("tool-1") + }) + }) +}) diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts new file mode 100644 index 00000000000..66eafb21e41 --- /dev/null +++ b/src/core/task/validateToolResultIds.ts @@ -0,0 +1,111 @@ +import { Anthropic } from "@anthropic-ai/sdk" + +/** + * 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 + * before messages are added to the API conversation history. It handles scenarios like: + * - Race conditions during streaming + * - Message editing scenarios + * - Resume/delegation scenarios + * + * @param userMessage - The user message being added to history + * @param previousAssistantMessage - The previous assistant message containing tool_use blocks + * @returns The validated user message with corrected tool_use_ids + */ +export function validateAndFixToolResultIds( + userMessage: Anthropic.MessageParam, + previousAssistantMessage: Anthropic.MessageParam | undefined, +): Anthropic.MessageParam { + // Only process user messages with array content + if (userMessage.role !== "user" || !Array.isArray(userMessage.content)) { + 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 + } + + // No previous assistant message to validate against + if (!previousAssistantMessage || previousAssistantMessage.role !== "assistant") { + return userMessage + } + + // Get tool_use blocks from the assistant message + const assistantContent = previousAssistantMessage.content + if (!Array.isArray(assistantContent)) { + return userMessage + } + + const toolUseBlocks = assistantContent.filter((block): block is Anthropic.ToolUseBlock => block.type === "tool_use") + + // No tool_use blocks to match against + if (toolUseBlocks.length === 0) { + return userMessage + } + + // Build a set of valid tool_use IDs + const validToolUseIds = new Set(toolUseBlocks.map((block) => block.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 + return userMessage + } + + // We have mismatches - need to fix them + console.warn( + `[validateAndFixToolResultIds] Detected tool_result ID mismatch. ` + + `tool_result IDs: [${toolResults.map((r) => r.tool_use_id).join(", ")}], ` + + `tool_use IDs: [${toolUseBlocks.map((b) => b.id).join(", ")}]`, + ) + + // 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) => { + 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 is (among all tool_results) + const toolResultIndex = toolResults.findIndex((r) => r.tool_use_id === block.tool_use_id) + + // 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 + console.warn( + `[validateAndFixToolResultIds] Correcting tool_use_id: "${block.tool_use_id}" -> "${correctId}"`, + ) + return { + ...block, + tool_use_id: correctId, + } + } + + // No corresponding tool_use for this tool_result - leave it unchanged + // This can happen when there are more tool_results than tool_uses + console.warn( + `[validateAndFixToolResultIds] No matching tool_use for tool_result with id "${block.tool_use_id}" - leaving unchanged`, + ) + return block + }) + + return { + ...userMessage, + content: correctedContent, + } +} From 3a087a7ab98a065fc67bc0b587e320db7f9ec5a4 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 14:18:13 -0500 Subject: [PATCH 2/8] chore: remove unnecessary comment --- src/core/assistant-message/presentAssistantMessage.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index cf2c5a9df6e..84f00806da4 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -523,7 +523,6 @@ export async function presentAssistantMessage(cline: Task) { } // Add tool_result with text content only - // Note: tool_use_id validation happens centrally in addToApiConversationHistory cline.userMessageContent.push({ type: "tool_result", tool_use_id: toolCallId, From 2e21a546e2812949262c31e4773869bc706546a4 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 16:58:12 -0500 Subject: [PATCH 3/8] feat: add captureException method for telemetry error tracking --- packages/cloud/src/TelemetryClient.ts | 4 + packages/telemetry/src/BaseTelemetryClient.ts | 2 + .../telemetry/src/PostHogTelemetryClient.ts | 16 +++ packages/telemetry/src/TelemetryService.ts | 13 +++ packages/types/src/telemetry.ts | 1 + .../__tests__/validateToolResultIds.spec.ts | 101 +++++++++++++++++- src/core/task/validateToolResultIds.ts | 46 ++++++-- 7 files changed, 171 insertions(+), 12 deletions(-) diff --git a/packages/cloud/src/TelemetryClient.ts b/packages/cloud/src/TelemetryClient.ts index 1213530a95b..f4b05feed53 100644 --- a/packages/cloud/src/TelemetryClient.ts +++ b/packages/cloud/src/TelemetryClient.ts @@ -69,6 +69,10 @@ abstract class BaseTelemetryClient implements TelemetryClient { public abstract capture(event: TelemetryEvent): Promise + public captureException(_error: Error, _additionalProperties?: Record): void { + // No-op - exception capture is only supported by PostHog + } + public setProvider(provider: TelemetryPropertiesProvider): void { this.providerRef = new WeakRef(provider) } diff --git a/packages/telemetry/src/BaseTelemetryClient.ts b/packages/telemetry/src/BaseTelemetryClient.ts index 2eb308b414b..5f59e1643b7 100644 --- a/packages/telemetry/src/BaseTelemetryClient.ts +++ b/packages/telemetry/src/BaseTelemetryClient.ts @@ -59,6 +59,8 @@ export abstract class BaseTelemetryClient implements TelemetryClient { public abstract capture(event: TelemetryEvent): Promise + public abstract captureException(error: Error, additionalProperties?: Record): void + public setProvider(provider: TelemetryPropertiesProvider): void { this.providerRef = new WeakRef(provider) } diff --git a/packages/telemetry/src/PostHogTelemetryClient.ts b/packages/telemetry/src/PostHogTelemetryClient.ts index d7f632f1379..cea945f1580 100644 --- a/packages/telemetry/src/PostHogTelemetryClient.ts +++ b/packages/telemetry/src/PostHogTelemetryClient.ts @@ -61,6 +61,22 @@ export class PostHogTelemetryClient extends BaseTelemetryClient { }) } + public override captureException(error: Error, additionalProperties?: Record): void { + if (!this.isTelemetryEnabled()) { + if (this.debug) { + console.info(`[PostHogTelemetryClient#captureException] Skipping exception: ${error.message}`) + } + + return + } + + if (this.debug) { + console.info(`[PostHogTelemetryClient#captureException] ${error.message}`) + } + + this.client.captureException(error, this.distinctId, additionalProperties) + } + /** * Updates the telemetry state based on user preferences and VSCode settings. * Only enables telemetry if both VSCode global telemetry is enabled and diff --git a/packages/telemetry/src/TelemetryService.ts b/packages/telemetry/src/TelemetryService.ts index 8f4fbe09741..ff94b524f84 100644 --- a/packages/telemetry/src/TelemetryService.ts +++ b/packages/telemetry/src/TelemetryService.ts @@ -65,6 +65,19 @@ export class TelemetryService { this.clients.forEach((client) => client.capture({ event: eventName, properties })) } + /** + * Captures an exception using PostHog's error tracking + * @param error The error to capture + * @param additionalProperties Additional properties to include with the exception + */ + public captureException(error: Error, additionalProperties?: Record): void { + if (!this.isReady) { + return + } + + this.clients.forEach((client) => client.captureException(error, additionalProperties)) + } + public captureTaskCreated(taskId: string): void { this.captureEvent(TelemetryEventName.TASK_CREATED, { taskId }) } diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 233edfe499e..d5b39077404 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -262,6 +262,7 @@ export interface TelemetryClient { setProvider(provider: TelemetryPropertiesProvider): void capture(options: TelemetryEvent): Promise + captureException(error: Error, additionalProperties?: Record): void updateTelemetryState(isOptedIn: boolean): void isTelemetryEnabled(): boolean shutdown(): Promise diff --git a/src/core/task/__tests__/validateToolResultIds.spec.ts b/src/core/task/__tests__/validateToolResultIds.spec.ts index 931711b8284..ea97555719e 100644 --- a/src/core/task/__tests__/validateToolResultIds.spec.ts +++ b/src/core/task/__tests__/validateToolResultIds.spec.ts @@ -1,7 +1,22 @@ import { Anthropic } from "@anthropic-ai/sdk" -import { validateAndFixToolResultIds } from "../validateToolResultIds" +import { TelemetryService } from "@roo-code/telemetry" +import { validateAndFixToolResultIds, ToolResultIdMismatchError } from "../validateToolResultIds" + +// Mock TelemetryService +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + hasInstance: vi.fn(() => true), + instance: { + captureException: vi.fn(), + }, + }, +})) describe("validateAndFixToolResultIds", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + describe("when there is no previous assistant message", () => { it("should return the user message unchanged", () => { const userMessage: Anthropic.MessageParam = { @@ -417,4 +432,88 @@ describe("validateAndFixToolResultIds", () => { expect(resultContent[0].tool_use_id).toBe("tool-1") }) }) + + describe("telemetry", () => { + it("should call captureException when there is a mismatch", () => { + const assistantMessage: Anthropic.MessageParam = { + role: "assistant", + content: [ + { + type: "tool_use", + id: "correct-id", + name: "read_file", + input: { path: "test.txt" }, + }, + ], + } + + const userMessage: Anthropic.MessageParam = { + role: "user", + content: [ + { + type: "tool_result", + tool_use_id: "wrong-id", + content: "Content", + }, + ], + } + + validateAndFixToolResultIds(userMessage, assistantMessage) + + expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(1) + expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( + expect.any(ToolResultIdMismatchError), + expect.objectContaining({ + toolResultIds: ["wrong-id"], + toolUseIds: ["correct-id"], + toolResultCount: 1, + toolUseCount: 1, + }), + ) + }) + + it("should not call captureException when IDs match", () => { + 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() + }) + }) + + describe("ToolResultIdMismatchError", () => { + it("should create error with correct properties", () => { + const error = new ToolResultIdMismatchError( + "Mismatch detected", + ["result-1", "result-2"], + ["use-1", "use-2"], + ) + + expect(error.name).toBe("ToolResultIdMismatchError") + expect(error.message).toBe("Mismatch detected") + expect(error.toolResultIds).toEqual(["result-1", "result-2"]) + expect(error.toolUseIds).toEqual(["use-1", "use-2"]) + }) + }) }) diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts index 66eafb21e41..ddfac24637c 100644 --- a/src/core/task/validateToolResultIds.ts +++ b/src/core/task/validateToolResultIds.ts @@ -1,4 +1,20 @@ import { Anthropic } from "@anthropic-ai/sdk" +import { TelemetryService } from "@roo-code/telemetry" + +/** + * Custom error class for tool result ID mismatches. + * Used for structured error tracking via PostHog. + */ +export class ToolResultIdMismatchError extends Error { + constructor( + message: string, + public readonly toolResultIds: string[], + public readonly toolUseIds: string[], + ) { + super(message) + this.name = "ToolResultIdMismatchError" + } +} /** * Validates and fixes tool_result IDs in a user message against the previous assistant message. @@ -62,11 +78,25 @@ export function validateAndFixToolResultIds( } // We have mismatches - need to fix them - console.warn( - `[validateAndFixToolResultIds] Detected tool_result ID mismatch. ` + - `tool_result IDs: [${toolResults.map((r) => r.tool_use_id).join(", ")}], ` + - `tool_use IDs: [${toolUseBlocks.map((b) => b.id).join(", ")}]`, - ) + 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()) { + TelemetryService.instance.captureException( + new ToolResultIdMismatchError( + `Detected tool_result ID mismatch. tool_result IDs: [${toolResultIdList.join(", ")}], tool_use IDs: [${toolUseIdList.join(", ")}]`, + toolResultIdList, + toolUseIdList, + ), + { + toolResultIds: toolResultIdList, + toolUseIds: toolUseIdList, + toolResultCount: toolResults.length, + toolUseCount: toolUseBlocks.length, + }, + ) + } // Create a mapping of tool_result IDs to corrected IDs // Strategy: Match by position (first tool_result -> first tool_use, etc.) @@ -87,9 +117,6 @@ export function validateAndFixToolResultIds( // 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 - console.warn( - `[validateAndFixToolResultIds] Correcting tool_use_id: "${block.tool_use_id}" -> "${correctId}"`, - ) return { ...block, tool_use_id: correctId, @@ -98,9 +125,6 @@ export function validateAndFixToolResultIds( // No corresponding tool_use for this tool_result - leave it unchanged // This can happen when there are more tool_results than tool_uses - console.warn( - `[validateAndFixToolResultIds] No matching tool_use for tool_result with id "${block.tool_use_id}" - leaving unchanged`, - ) return block }) From a3e1da20cc4246be1b86466a786f247450e2e75d Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 17:01:18 -0500 Subject: [PATCH 4/8] perf: optimize backwards walk for finding previous assistant message - Replace .slice().reverse().find() with direct backwards loop - Avoids creating two intermediate arrays for large conversation histories - Applied to both addToApiConversationHistory and flushPendingToolResultsToHistory --- src/core/task/Task.ts | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index c6304d9388f..07990d14235 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -813,11 +813,13 @@ export class Task extends EventEmitter implements TaskLike { this.apiConversationHistory.push(messageWithTs) } else { // For user messages, validate and fix tool_result IDs against the previous assistant message - // This is a centralized catch-all that handles all tool_use/tool_result ID mismatches - const previousAssistantMessage = this.apiConversationHistory - .slice() - .reverse() - .find((msg) => msg.role === "assistant") + let previousAssistantMessage: Anthropic.MessageParam | undefined + for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { + if (this.apiConversationHistory[i].role === "assistant") { + previousAssistantMessage = this.apiConversationHistory[i] + break + } + } const validatedMessage = validateAndFixToolResultIds(message, previousAssistantMessage) const messageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(messageWithTs) @@ -859,10 +861,14 @@ export class Task extends EventEmitter implements TaskLike { } // Validate and fix tool_result IDs against the previous assistant message - const previousAssistantMessage = this.apiConversationHistory - .slice() - .reverse() - .find((msg) => msg.role === "assistant") + // Walk backwards to find the previous assistant message without creating intermediate arrays + let previousAssistantMessage: Anthropic.MessageParam | undefined + for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { + if (this.apiConversationHistory[i].role === "assistant") { + previousAssistantMessage = this.apiConversationHistory[i] + break + } + } const validatedMessage = validateAndFixToolResultIds(userMessage, previousAssistantMessage) const userMessageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(userMessageWithTs as ApiMessage) From 8386e836ef10bfff7f383c9248f052ecf1cbcffa Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 17:09:34 -0500 Subject: [PATCH 5/8] fix: remove unnecessary comment in tool_result ID validation logic --- src/core/task/Task.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 07990d14235..3730ac0a3f5 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -861,7 +861,6 @@ export class Task extends EventEmitter implements TaskLike { } // Validate and fix tool_result IDs against the previous assistant message - // Walk backwards to find the previous assistant message without creating intermediate arrays let previousAssistantMessage: Anthropic.MessageParam | undefined for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { if (this.apiConversationHistory[i].role === "assistant") { From ddf88d8b42887444217d6cb9a063ed4eb315581f Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 17:16:23 -0500 Subject: [PATCH 6/8] refactor: use findLastIndex utility for consistency - Replace manual backwards loop with findLastIndex in both addToApiConversationHistory and flushPendingToolResultsToHistory - Improves clarity and consistency with existing code patterns --- src/core/task/Task.ts | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 3730ac0a3f5..2c67bc1feb9 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -813,13 +813,9 @@ export class Task extends EventEmitter implements TaskLike { this.apiConversationHistory.push(messageWithTs) } else { // For user messages, validate and fix tool_result IDs against the previous assistant message - let previousAssistantMessage: Anthropic.MessageParam | undefined - for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { - if (this.apiConversationHistory[i].role === "assistant") { - previousAssistantMessage = this.apiConversationHistory[i] - break - } - } + const prevAssistantIdx = findLastIndex(this.apiConversationHistory, (msg) => msg.role === "assistant") + const previousAssistantMessage = + prevAssistantIdx !== -1 ? this.apiConversationHistory[prevAssistantIdx] : undefined const validatedMessage = validateAndFixToolResultIds(message, previousAssistantMessage) const messageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(messageWithTs) @@ -861,13 +857,9 @@ export class Task extends EventEmitter implements TaskLike { } // Validate and fix tool_result IDs against the previous assistant message - let previousAssistantMessage: Anthropic.MessageParam | undefined - for (let i = this.apiConversationHistory.length - 1; i >= 0; i--) { - if (this.apiConversationHistory[i].role === "assistant") { - previousAssistantMessage = this.apiConversationHistory[i] - break - } - } + const prevAssistantIdx = findLastIndex(this.apiConversationHistory, (msg) => msg.role === "assistant") + const previousAssistantMessage = + prevAssistantIdx !== -1 ? this.apiConversationHistory[prevAssistantIdx] : undefined const validatedMessage = validateAndFixToolResultIds(userMessage, previousAssistantMessage) const userMessageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(userMessageWithTs as ApiMessage) From f4bb76a85151af19017fa4eee631cb2ffcd41db8 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 10 Dec 2025 01:11:57 +0000 Subject: [PATCH 7/8] refactor: DRY up validateAndFixToolResultIds by passing apiConversationHistory - Changed validateAndFixToolResultIds to accept apiConversationHistory array - Moved findLastIndex logic inside the validation function - Simplified Task.ts callers to just pass this.apiConversationHistory - Updated tests to pass arrays instead of single messages --- src/core/task/Task.ts | 10 ++----- .../__tests__/validateToolResultIds.spec.ts | 28 +++++++++---------- src/core/task/validateToolResultIds.ts | 12 +++++--- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 2c67bc1feb9..35f9011a86d 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -813,10 +813,7 @@ export class Task extends EventEmitter implements TaskLike { this.apiConversationHistory.push(messageWithTs) } else { // For user messages, validate and fix tool_result IDs against the previous assistant message - const prevAssistantIdx = findLastIndex(this.apiConversationHistory, (msg) => msg.role === "assistant") - const previousAssistantMessage = - prevAssistantIdx !== -1 ? this.apiConversationHistory[prevAssistantIdx] : undefined - const validatedMessage = validateAndFixToolResultIds(message, previousAssistantMessage) + const validatedMessage = validateAndFixToolResultIds(message, this.apiConversationHistory) const messageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(messageWithTs) } @@ -857,10 +854,7 @@ export class Task extends EventEmitter implements TaskLike { } // Validate and fix tool_result IDs against the previous assistant message - const prevAssistantIdx = findLastIndex(this.apiConversationHistory, (msg) => msg.role === "assistant") - const previousAssistantMessage = - prevAssistantIdx !== -1 ? this.apiConversationHistory[prevAssistantIdx] : undefined - const validatedMessage = validateAndFixToolResultIds(userMessage, previousAssistantMessage) + const validatedMessage = validateAndFixToolResultIds(userMessage, this.apiConversationHistory) const userMessageWithTs = { ...validatedMessage, ts: Date.now() } this.apiConversationHistory.push(userMessageWithTs as ApiMessage) diff --git a/src/core/task/__tests__/validateToolResultIds.spec.ts b/src/core/task/__tests__/validateToolResultIds.spec.ts index ea97555719e..67aaf238952 100644 --- a/src/core/task/__tests__/validateToolResultIds.spec.ts +++ b/src/core/task/__tests__/validateToolResultIds.spec.ts @@ -30,7 +30,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, undefined) + const result = validateAndFixToolResultIds(userMessage, []) expect(result).toEqual(userMessage) }) @@ -61,7 +61,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(result).toEqual(userMessage) }) @@ -101,7 +101,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(result).toEqual(userMessage) }) @@ -132,7 +132,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] @@ -175,7 +175,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] @@ -218,7 +218,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] @@ -256,7 +256,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Array @@ -296,7 +296,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] @@ -323,7 +323,7 @@ describe("validateAndFixToolResultIds", () => { content: "Just a plain text message", } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(result).toEqual(userMessage) }) @@ -347,7 +347,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(result).toEqual(userMessage) }) @@ -383,7 +383,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] @@ -424,7 +424,7 @@ describe("validateAndFixToolResultIds", () => { ], } - const result = validateAndFixToolResultIds(userMessage, assistantMessage) + const result = validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(Array.isArray(result.content)).toBe(true) const resultContent = result.content as Anthropic.ToolResultBlockParam[] @@ -458,7 +458,7 @@ describe("validateAndFixToolResultIds", () => { ], } - validateAndFixToolResultIds(userMessage, assistantMessage) + validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(1) expect(TelemetryService.instance.captureException).toHaveBeenCalledWith( @@ -496,7 +496,7 @@ describe("validateAndFixToolResultIds", () => { ], } - validateAndFixToolResultIds(userMessage, assistantMessage) + validateAndFixToolResultIds(userMessage, [assistantMessage]) expect(TelemetryService.instance.captureException).not.toHaveBeenCalled() }) diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts index ddfac24637c..9d3fcd732d5 100644 --- a/src/core/task/validateToolResultIds.ts +++ b/src/core/task/validateToolResultIds.ts @@ -1,5 +1,6 @@ import { Anthropic } from "@anthropic-ai/sdk" import { TelemetryService } from "@roo-code/telemetry" +import { findLastIndex } from "../../shared/array" /** * Custom error class for tool result ID mismatches. @@ -26,12 +27,12 @@ export class ToolResultIdMismatchError extends Error { * - Resume/delegation scenarios * * @param userMessage - The user message being added to history - * @param previousAssistantMessage - The previous assistant message containing tool_use blocks + * @param apiConversationHistory - The conversation history to find the previous assistant message from * @returns The validated user message with corrected tool_use_ids */ export function validateAndFixToolResultIds( userMessage: Anthropic.MessageParam, - previousAssistantMessage: Anthropic.MessageParam | undefined, + apiConversationHistory: Anthropic.MessageParam[], ): Anthropic.MessageParam { // Only process user messages with array content if (userMessage.role !== "user" || !Array.isArray(userMessage.content)) { @@ -48,11 +49,14 @@ export function validateAndFixToolResultIds( return userMessage } - // No previous assistant message to validate against - if (!previousAssistantMessage || previousAssistantMessage.role !== "assistant") { + // Find the previous assistant message from conversation history + const prevAssistantIdx = findLastIndex(apiConversationHistory, (msg) => msg.role === "assistant") + if (prevAssistantIdx === -1) { return userMessage } + const previousAssistantMessage = apiConversationHistory[prevAssistantIdx] + // Get tool_use blocks from the assistant message const assistantContent = previousAssistantMessage.content if (!Array.isArray(assistantContent)) { From 11fdfd87963946dcd24a78ba6a0ca3b04874f0b3 Mon Sep 17 00:00:00 2001 From: Hannes Rudolph Date: Tue, 9 Dec 2025 18:17:03 -0700 Subject: [PATCH 8/8] fix: use indexOf to correctly handle duplicate tool_use_ids When multiple tool_result blocks share the same invalid tool_use_id, findIndex always returned the first occurrence index, causing all duplicates to map to position 0. Using indexOf(block) finds the block by reference, correctly mapping each to its actual position. --- src/core/task/validateToolResultIds.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts index 9d3fcd732d5..159caca79d8 100644 --- a/src/core/task/validateToolResultIds.ts +++ b/src/core/task/validateToolResultIds.ts @@ -115,8 +115,10 @@ export function validateAndFixToolResultIds( return block } - // Find which tool_result index this is (among all tool_results) - const toolResultIndex = toolResults.findIndex((r) => r.tool_use_id === block.tool_use_id) + // 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) {