From b7f7a41e21cb116687a9d9b66a89f9d26da55968 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 13:16:05 -0500 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 05/10] 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 06/10] 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 ecba9ede6b386b681d1db7e85ca50e4d7a1a8434 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 14:34:47 -0500 Subject: [PATCH 07/10] feat: add API error telemetry to OpenRouter provider - Add new TelemetryEventName.API_ERROR to telemetry types - Log API errors to PostHog with sanitized error messages - Use sanitizeErrorMessage to redact sensitive info (URLs, paths, IPs, emails) - Add tests for telemetry capture in error scenarios --- packages/types/src/telemetry.ts | 2 + .../providers/__tests__/openrouter.spec.ts | 68 ++++++++++++++++++- src/api/providers/openrouter.ts | 30 ++++++++ 3 files changed, 97 insertions(+), 3 deletions(-) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index d5b39077404..6281873d2d4 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -71,6 +71,7 @@ export enum TelemetryEventName { SHELL_INTEGRATION_ERROR = "Shell Integration Error", CONSECUTIVE_MISTAKE_ERROR = "Consecutive Mistake Error", CODE_INDEX_ERROR = "Code Index Error", + API_ERROR = "API Error", TELEMETRY_SETTINGS_CHANGED = "Telemetry Settings Changed", MODEL_CACHE_EMPTY_RESPONSE = "Model Cache Empty Response", } @@ -197,6 +198,7 @@ export const rooCodeTelemetryEventSchema = z.discriminatedUnion("type", [ TelemetryEventName.SHELL_INTEGRATION_ERROR, TelemetryEventName.CONSECUTIVE_MISTAKE_ERROR, TelemetryEventName.CODE_INDEX_ERROR, + TelemetryEventName.API_ERROR, TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, TelemetryEventName.CONTEXT_CONDENSED, TelemetryEventName.SLIDING_WINDOW_TRUNCATION, diff --git a/src/api/providers/__tests__/openrouter.spec.ts b/src/api/providers/__tests__/openrouter.spec.ts index 157c5aacdd4..95e2761ce13 100644 --- a/src/api/providers/__tests__/openrouter.spec.ts +++ b/src/api/providers/__tests__/openrouter.spec.ts @@ -9,10 +9,27 @@ import OpenAI from "openai" import { OpenRouterHandler } from "../openrouter" import { ApiHandlerOptions } from "../../../shared/api" import { Package } from "../../../shared/package" +import { TelemetryEventName } from "@roo-code/types" +import { TelemetryService } from "@roo-code/telemetry" // Mock dependencies vitest.mock("openai") vitest.mock("delay", () => ({ default: vitest.fn(() => Promise.resolve()) })) + +// Mock TelemetryService +const mockCaptureEvent = vitest.fn() +vitest.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureEvent: (...args: unknown[]) => mockCaptureEvent(...args), + }, + }, +})) + +// Mock sanitizeErrorMessage - pass through the message unchanged for testing +vitest.mock("../../../services/code-index/shared/validation-helpers", () => ({ + sanitizeErrorMessage: (msg: string) => msg, +})) vitest.mock("../fetchers/modelCache", () => ({ getModels: vitest.fn().mockImplementation(() => { return Promise.resolve({ @@ -267,7 +284,7 @@ describe("OpenRouterHandler", () => { ) }) - it("handles API errors", async () => { + it("handles API errors and captures telemetry", async () => { const handler = new OpenRouterHandler(mockOptions) const mockStream = { async *[Symbol.asyncIterator]() { @@ -282,6 +299,34 @@ describe("OpenRouterHandler", () => { const generator = handler.createMessage("test", []) await expect(generator.next()).rejects.toThrow("OpenRouter API Error 500: API Error") + + // Verify telemetry was captured + expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + provider: "OpenRouter", + modelId: mockOptions.openRouterModelId, + operation: "createMessage", + errorCode: 500, + errorMessage: "API Error", + }) + }) + + it("captures telemetry when createMessage throws an exception", async () => { + const handler = new OpenRouterHandler(mockOptions) + const mockCreate = vitest.fn().mockRejectedValue(new Error("Connection failed")) + ;(OpenAI as any).prototype.chat = { + completions: { create: mockCreate }, + } as any + + const generator = handler.createMessage("test", []) + await expect(generator.next()).rejects.toThrow() + + // Verify telemetry was captured + expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + provider: "OpenRouter", + modelId: mockOptions.openRouterModelId, + operation: "createMessage", + errorMessage: "Connection failed", + }) }) it("yields tool_call_end events when finish_reason is tool_calls", async () => { @@ -384,7 +429,7 @@ describe("OpenRouterHandler", () => { ) }) - it("handles API errors", async () => { + it("handles API errors and captures telemetry", async () => { const handler = new OpenRouterHandler(mockOptions) const mockError = { error: { @@ -399,9 +444,18 @@ describe("OpenRouterHandler", () => { } as any await expect(handler.completePrompt("test prompt")).rejects.toThrow("OpenRouter API Error 500: API Error") + + // Verify telemetry was captured + expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + provider: "OpenRouter", + modelId: mockOptions.openRouterModelId, + operation: "completePrompt", + errorCode: 500, + errorMessage: "API Error", + }) }) - it("handles unexpected errors", async () => { + it("handles unexpected errors and captures telemetry", async () => { const handler = new OpenRouterHandler(mockOptions) const mockCreate = vitest.fn().mockRejectedValue(new Error("Unexpected error")) ;(OpenAI as any).prototype.chat = { @@ -409,6 +463,14 @@ describe("OpenRouterHandler", () => { } as any await expect(handler.completePrompt("test prompt")).rejects.toThrow("Unexpected error") + + // Verify telemetry was captured + expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + provider: "OpenRouter", + modelId: mockOptions.openRouterModelId, + operation: "completePrompt", + errorMessage: "Unexpected error", + }) }) }) }) diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index ef2c553b549..48304275641 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -7,7 +7,11 @@ import { OPENROUTER_DEFAULT_PROVIDER_NAME, OPEN_ROUTER_PROMPT_CACHING_MODELS, DEEP_SEEK_DEFAULT_TEMPERATURE, + TelemetryEventName, } from "@roo-code/types" +import { TelemetryService } from "@roo-code/telemetry" + +import { sanitizeErrorMessage } from "../../services/code-index/shared/validation-helpers" import { NativeToolCallParser } from "../../core/assistant-message/NativeToolCallParser" @@ -224,6 +228,12 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH try { stream = await this.client.chat.completions.create(completionParams, requestOptions) } catch (error) { + TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { + provider: this.providerName, + modelId, + operation: "createMessage", + errorMessage: sanitizeErrorMessage(error instanceof Error ? error.message : String(error)), + }) throw handleOpenAIError(error, this.providerName) } @@ -248,6 +258,13 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH if ("error" in chunk) { const error = chunk.error as { message?: string; code?: number } console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`) + TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { + provider: this.providerName, + modelId, + operation: "createMessage", + errorCode: error?.code, + errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), + }) throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } @@ -442,11 +459,24 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH try { response = await this.client.chat.completions.create(completionParams, requestOptions) } catch (error) { + TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { + provider: this.providerName, + modelId, + operation: "completePrompt", + errorMessage: sanitizeErrorMessage(error instanceof Error ? error.message : String(error)), + }) throw handleOpenAIError(error, this.providerName) } if ("error" in response) { const error = response.error as { message?: string; code?: number } + TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { + provider: this.providerName, + modelId, + operation: "completePrompt", + errorCode: error?.code, + errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), + }) throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } From 9209737774fb0bc79a5e728de2b39f9066bd3ff1 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 15:02:17 -0500 Subject: [PATCH 08/10] refactor: move expected error codes to types package - Add EXPECTED_API_ERROR_CODES and shouldReportApiErrorToTelemetry to types package - Filter out 429 rate limit errors from telemetry (expected behavior) - Add tests to verify 429 errors don't trigger telemetry --- packages/types/src/telemetry.ts | 19 +++++++++ .../providers/__tests__/openrouter.spec.ts | 42 +++++++++++++++++++ src/api/providers/openrouter.ts | 35 +++++++++------- 3 files changed, 82 insertions(+), 14 deletions(-) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 6281873d2d4..957c17292a6 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -269,3 +269,22 @@ export interface TelemetryClient { isTelemetryEnabled(): boolean shutdown(): Promise } + +/** + * Expected API error codes that should not be reported to telemetry. + * These are normal/expected errors that users can't do much about. + */ +export const EXPECTED_API_ERROR_CODES = new Set([ + 429, // Rate limit - expected when hitting API limits +]) + +/** + * Helper to check if an API error should be reported to telemetry. + * Filters out expected errors like rate limits. + * @param errorCode - The HTTP error code (if available) + * @returns true if the error should be reported, false if it should be filtered out + */ +export function shouldReportApiErrorToTelemetry(errorCode?: number): boolean { + if (errorCode === undefined) return true + return !EXPECTED_API_ERROR_CODES.has(errorCode) +} diff --git a/src/api/providers/__tests__/openrouter.spec.ts b/src/api/providers/__tests__/openrouter.spec.ts index 95e2761ce13..a6d1ca6afb5 100644 --- a/src/api/providers/__tests__/openrouter.spec.ts +++ b/src/api/providers/__tests__/openrouter.spec.ts @@ -329,6 +329,26 @@ describe("OpenRouterHandler", () => { }) }) + it("does NOT capture telemetry for 429 rate limit errors", async () => { + const handler = new OpenRouterHandler(mockOptions) + const mockStream = { + async *[Symbol.asyncIterator]() { + yield { error: { message: "Rate limit exceeded", code: 429 } } + }, + } + + const mockCreate = vitest.fn().mockResolvedValue(mockStream) + ;(OpenAI as any).prototype.chat = { + completions: { create: mockCreate }, + } as any + + const generator = handler.createMessage("test", []) + await expect(generator.next()).rejects.toThrow("OpenRouter API Error 429: Rate limit exceeded") + + // Verify telemetry was NOT captured for 429 errors + expect(mockCaptureEvent).not.toHaveBeenCalled() + }) + it("yields tool_call_end events when finish_reason is tool_calls", async () => { // Import NativeToolCallParser to set up state const { NativeToolCallParser } = await import("../../../core/assistant-message/NativeToolCallParser") @@ -472,5 +492,27 @@ describe("OpenRouterHandler", () => { errorMessage: "Unexpected error", }) }) + + it("does NOT capture telemetry for 429 rate limit errors", async () => { + const handler = new OpenRouterHandler(mockOptions) + const mockError = { + error: { + message: "Rate limit exceeded", + code: 429, + }, + } + + const mockCreate = vitest.fn().mockResolvedValue(mockError) + ;(OpenAI as any).prototype.chat = { + completions: { create: mockCreate }, + } as any + + await expect(handler.completePrompt("test prompt")).rejects.toThrow( + "OpenRouter API Error 429: Rate limit exceeded", + ) + + // Verify telemetry was NOT captured for 429 errors + expect(mockCaptureEvent).not.toHaveBeenCalled() + }) }) }) diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index 48304275641..119ec101a6f 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -8,6 +8,7 @@ import { OPEN_ROUTER_PROMPT_CACHING_MODELS, DEEP_SEEK_DEFAULT_TEMPERATURE, TelemetryEventName, + shouldReportApiErrorToTelemetry, } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" @@ -258,13 +259,16 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH if ("error" in chunk) { const error = chunk.error as { message?: string; code?: number } console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`) - TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { - provider: this.providerName, - modelId, - operation: "createMessage", - errorCode: error?.code, - errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), - }) + // Only report unexpected errors to telemetry (skip 429 rate limit errors) + if (shouldReportApiErrorToTelemetry(error?.code)) { + TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { + provider: this.providerName, + modelId, + operation: "createMessage", + errorCode: error?.code, + errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), + }) + } throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } @@ -470,13 +474,16 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH if ("error" in response) { const error = response.error as { message?: string; code?: number } - TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { - provider: this.providerName, - modelId, - operation: "completePrompt", - errorCode: error?.code, - errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), - }) + // Only report unexpected errors to telemetry (skip 429 rate limit errors) + if (shouldReportApiErrorToTelemetry(error?.code)) { + TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { + provider: this.providerName, + modelId, + operation: "completePrompt", + errorCode: error?.code, + errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), + }) + } throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } From 332a4e87e136a86f64a4db94979e0143165a8d04 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 9 Dec 2025 15:12:57 -0500 Subject: [PATCH 09/10] remove redundant comments --- packages/types/src/telemetry.ts | 19 ++++- .../providers/__tests__/openrouter.spec.ts | 28 +++----- src/api/providers/openrouter.ts | 70 +++++++++++-------- 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/packages/types/src/telemetry.ts b/packages/types/src/telemetry.ts index 957c17292a6..eb934a09263 100644 --- a/packages/types/src/telemetry.ts +++ b/packages/types/src/telemetry.ts @@ -71,7 +71,6 @@ export enum TelemetryEventName { SHELL_INTEGRATION_ERROR = "Shell Integration Error", CONSECUTIVE_MISTAKE_ERROR = "Consecutive Mistake Error", CODE_INDEX_ERROR = "Code Index Error", - API_ERROR = "API Error", TELEMETRY_SETTINGS_CHANGED = "Telemetry Settings Changed", MODEL_CACHE_EMPTY_RESPONSE = "Model Cache Empty Response", } @@ -198,7 +197,6 @@ export const rooCodeTelemetryEventSchema = z.discriminatedUnion("type", [ TelemetryEventName.SHELL_INTEGRATION_ERROR, TelemetryEventName.CONSECUTIVE_MISTAKE_ERROR, TelemetryEventName.CODE_INDEX_ERROR, - TelemetryEventName.API_ERROR, TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, TelemetryEventName.CONTEXT_CONDENSED, TelemetryEventName.SLIDING_WINDOW_TRUNCATION, @@ -288,3 +286,20 @@ export function shouldReportApiErrorToTelemetry(errorCode?: number): boolean { if (errorCode === undefined) return true return !EXPECTED_API_ERROR_CODES.has(errorCode) } + +/** + * Generic API provider error class for structured error tracking via PostHog. + * Can be reused by any API provider. + */ +export class ApiProviderError extends Error { + constructor( + message: string, + public readonly provider: string, + public readonly modelId: string, + public readonly operation: string, + public readonly errorCode?: number, + ) { + super(message) + this.name = "ApiProviderError" + } +} diff --git a/src/api/providers/__tests__/openrouter.spec.ts b/src/api/providers/__tests__/openrouter.spec.ts index a6d1ca6afb5..3347d96431e 100644 --- a/src/api/providers/__tests__/openrouter.spec.ts +++ b/src/api/providers/__tests__/openrouter.spec.ts @@ -9,27 +9,21 @@ import OpenAI from "openai" import { OpenRouterHandler } from "../openrouter" import { ApiHandlerOptions } from "../../../shared/api" import { Package } from "../../../shared/package" -import { TelemetryEventName } from "@roo-code/types" -import { TelemetryService } from "@roo-code/telemetry" +import { ApiProviderError } from "@roo-code/types" // Mock dependencies vitest.mock("openai") vitest.mock("delay", () => ({ default: vitest.fn(() => Promise.resolve()) })) // Mock TelemetryService -const mockCaptureEvent = vitest.fn() +const mockCaptureException = vitest.fn() vitest.mock("@roo-code/telemetry", () => ({ TelemetryService: { instance: { - captureEvent: (...args: unknown[]) => mockCaptureEvent(...args), + captureException: (...args: unknown[]) => mockCaptureException(...args), }, }, })) - -// Mock sanitizeErrorMessage - pass through the message unchanged for testing -vitest.mock("../../../services/code-index/shared/validation-helpers", () => ({ - sanitizeErrorMessage: (msg: string) => msg, -})) vitest.mock("../fetchers/modelCache", () => ({ getModels: vitest.fn().mockImplementation(() => { return Promise.resolve({ @@ -301,12 +295,11 @@ describe("OpenRouterHandler", () => { await expect(generator.next()).rejects.toThrow("OpenRouter API Error 500: API Error") // Verify telemetry was captured - expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), { provider: "OpenRouter", modelId: mockOptions.openRouterModelId, operation: "createMessage", errorCode: 500, - errorMessage: "API Error", }) }) @@ -321,11 +314,10 @@ describe("OpenRouterHandler", () => { await expect(generator.next()).rejects.toThrow() // Verify telemetry was captured - expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), { provider: "OpenRouter", modelId: mockOptions.openRouterModelId, operation: "createMessage", - errorMessage: "Connection failed", }) }) @@ -346,7 +338,7 @@ describe("OpenRouterHandler", () => { await expect(generator.next()).rejects.toThrow("OpenRouter API Error 429: Rate limit exceeded") // Verify telemetry was NOT captured for 429 errors - expect(mockCaptureEvent).not.toHaveBeenCalled() + expect(mockCaptureException).not.toHaveBeenCalled() }) it("yields tool_call_end events when finish_reason is tool_calls", async () => { @@ -466,12 +458,11 @@ describe("OpenRouterHandler", () => { await expect(handler.completePrompt("test prompt")).rejects.toThrow("OpenRouter API Error 500: API Error") // Verify telemetry was captured - expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), { provider: "OpenRouter", modelId: mockOptions.openRouterModelId, operation: "completePrompt", errorCode: 500, - errorMessage: "API Error", }) }) @@ -485,11 +476,10 @@ describe("OpenRouterHandler", () => { await expect(handler.completePrompt("test prompt")).rejects.toThrow("Unexpected error") // Verify telemetry was captured - expect(mockCaptureEvent).toHaveBeenCalledWith(TelemetryEventName.API_ERROR, { + expect(mockCaptureException).toHaveBeenCalledWith(expect.any(ApiProviderError), { provider: "OpenRouter", modelId: mockOptions.openRouterModelId, operation: "completePrompt", - errorMessage: "Unexpected error", }) }) @@ -512,7 +502,7 @@ describe("OpenRouterHandler", () => { ) // Verify telemetry was NOT captured for 429 errors - expect(mockCaptureEvent).not.toHaveBeenCalled() + expect(mockCaptureException).not.toHaveBeenCalled() }) }) }) diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index 119ec101a6f..ec9e04c4feb 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -7,13 +7,11 @@ import { OPENROUTER_DEFAULT_PROVIDER_NAME, OPEN_ROUTER_PROMPT_CACHING_MODELS, DEEP_SEEK_DEFAULT_TEMPERATURE, - TelemetryEventName, shouldReportApiErrorToTelemetry, + ApiProviderError, } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" -import { sanitizeErrorMessage } from "../../services/code-index/shared/validation-helpers" - import { NativeToolCallParser } from "../../core/assistant-message/NativeToolCallParser" import type { ApiHandlerOptions, ModelRecord } from "../../shared/api" @@ -229,12 +227,15 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH try { stream = await this.client.chat.completions.create(completionParams, requestOptions) } catch (error) { - TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { - provider: this.providerName, - modelId, - operation: "createMessage", - errorMessage: sanitizeErrorMessage(error instanceof Error ? error.message : String(error)), - }) + TelemetryService.instance.captureException( + new ApiProviderError( + error instanceof Error ? error.message : String(error), + this.providerName, + modelId, + "createMessage", + ), + { provider: this.providerName, modelId, operation: "createMessage" }, + ) throw handleOpenAIError(error, this.providerName) } @@ -259,15 +260,17 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH if ("error" in chunk) { const error = chunk.error as { message?: string; code?: number } console.error(`OpenRouter API Error: ${error?.code} - ${error?.message}`) - // Only report unexpected errors to telemetry (skip 429 rate limit errors) if (shouldReportApiErrorToTelemetry(error?.code)) { - TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { - provider: this.providerName, - modelId, - operation: "createMessage", - errorCode: error?.code, - errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), - }) + TelemetryService.instance.captureException( + new ApiProviderError( + error?.message ?? "Unknown error", + this.providerName, + modelId, + "createMessage", + error?.code, + ), + { provider: this.providerName, modelId, operation: "createMessage", errorCode: error?.code }, + ) } throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } @@ -463,26 +466,31 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH try { response = await this.client.chat.completions.create(completionParams, requestOptions) } catch (error) { - TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { - provider: this.providerName, - modelId, - operation: "completePrompt", - errorMessage: sanitizeErrorMessage(error instanceof Error ? error.message : String(error)), - }) + TelemetryService.instance.captureException( + new ApiProviderError( + error instanceof Error ? error.message : String(error), + this.providerName, + modelId, + "completePrompt", + ), + { provider: this.providerName, modelId, operation: "completePrompt" }, + ) throw handleOpenAIError(error, this.providerName) } if ("error" in response) { const error = response.error as { message?: string; code?: number } - // Only report unexpected errors to telemetry (skip 429 rate limit errors) if (shouldReportApiErrorToTelemetry(error?.code)) { - TelemetryService.instance.captureEvent(TelemetryEventName.API_ERROR, { - provider: this.providerName, - modelId, - operation: "completePrompt", - errorCode: error?.code, - errorMessage: sanitizeErrorMessage(error?.message ?? "Unknown error"), - }) + TelemetryService.instance.captureException( + new ApiProviderError( + error?.message ?? "Unknown error", + this.providerName, + modelId, + "completePrompt", + error?.code, + ), + { provider: this.providerName, modelId, operation: "completePrompt", errorCode: error?.code }, + ) } throw new Error(`OpenRouter API Error ${error?.code}: ${error?.message}`) } From ba3c0ce1a591d203e88e62d61628c2f032f6cda6 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 10 Dec 2025 00:23:45 +0000 Subject: [PATCH 10/10] revert: remove unrelated tool_result ID validation from #9952 --- src/core/task/Task.ts | 15 +- .../__tests__/validateToolResultIds.spec.ts | 519 ------------------ src/core/task/validateToolResultIds.ts | 135 ----- 3 files changed, 2 insertions(+), 667 deletions(-) delete mode 100644 src/core/task/__tests__/validateToolResultIds.spec.ts delete mode 100644 src/core/task/validateToolResultIds.ts diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 2c67bc1feb9..8ed9ab56405 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -129,7 +129,6 @@ 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 @@ -812,12 +811,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 messageWithTs = { ...validatedMessage, ts: Date.now() } + const messageWithTs = { ...message, ts: Date.now() } this.apiConversationHistory.push(messageWithTs) } @@ -856,12 +850,7 @@ export class Task extends EventEmitter implements TaskLike { content: this.userMessageContent, } - // 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 userMessageWithTs = { ...validatedMessage, ts: Date.now() } + const userMessageWithTs = { ...userMessage, 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 deleted file mode 100644 index ea97555719e..00000000000 --- a/src/core/task/__tests__/validateToolResultIds.spec.ts +++ /dev/null @@ -1,519 +0,0 @@ -import { Anthropic } from "@anthropic-ai/sdk" -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 = { - 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") - }) - }) - - 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 deleted file mode 100644 index ddfac24637c..00000000000 --- a/src/core/task/validateToolResultIds.ts +++ /dev/null @@ -1,135 +0,0 @@ -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. - * - * 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 - 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.) - // 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 - 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 - return block - }) - - return { - ...userMessage, - content: correctedContent, - } -}