From dfaf0e83537a03229defb212e63a5060976b8a48 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 24 Apr 2026 17:37:08 +0000 Subject: [PATCH] fix: add pre-send validation for tool_use/tool_result pairing (#11777) Adds validateMessageHistoryBeforeSend() as a safety net that runs on the final message array right before the API call. This catches any tool_use blocks that are missing corresponding tool_result blocks - a mismatch that causes Anthropic API rejection. The validation: - Iterates through the final messages looking for assistant messages with tool_use blocks - Checks the following user message has matching tool_result blocks - Injects placeholder tool_results for any missing pairings - Inserts synthetic user messages when none follow an assistant message - Reports mismatches to telemetry via MissingToolResultError This addresses cases where post-processing steps (getEffectiveApiHistory, mergeConsecutiveApiMessages, buildCleanConversationHistory) may introduce mismatches after the existing insert-time validation. --- src/core/task/Task.ts | 10 +- .../__tests__/validateToolResultIds.spec.ts | 223 ++++++++++++++++++ src/core/task/validateToolResultIds.ts | 115 +++++++++ 3 files changed, 346 insertions(+), 2 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 005bb0f292b..20beb3a788e 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -130,7 +130,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" +import { validateAndFixToolResultIds, validateMessageHistoryBeforeSend } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes @@ -4274,10 +4274,16 @@ export class Task extends EventEmitter implements TaskLike { // Reset the flag after using it this.skipPrevResponseIdOnce = false + // Final safety-net: ensure every tool_use has a matching tool_result before sending. + // This catches mismatches introduced by post-processing (condensing, merging, cleaning). + const validatedHistory = validateMessageHistoryBeforeSend( + cleanConversationHistory as unknown as Anthropic.Messages.MessageParam[], + ) + // The provider accepts reasoning items alongside standard messages; cast to the expected parameter type. const stream = this.api.createMessage( systemPrompt, - cleanConversationHistory as unknown as Anthropic.Messages.MessageParam[], + validatedHistory as unknown as Anthropic.Messages.MessageParam[], metadata, ) const iterator = stream[Symbol.asyncIterator]() diff --git a/src/core/task/__tests__/validateToolResultIds.spec.ts b/src/core/task/__tests__/validateToolResultIds.spec.ts index 0926e899aad..3cb9cd1323a 100644 --- a/src/core/task/__tests__/validateToolResultIds.spec.ts +++ b/src/core/task/__tests__/validateToolResultIds.spec.ts @@ -2,6 +2,7 @@ import { Anthropic } from "@anthropic-ai/sdk" import { TelemetryService } from "@roo-code/telemetry" import { validateAndFixToolResultIds, + validateMessageHistoryBeforeSend, ToolResultIdMismatchError, MissingToolResultError, } from "../validateToolResultIds" @@ -995,3 +996,225 @@ describe("validateAndFixToolResultIds", () => { }) }) }) + +describe("validateMessageHistoryBeforeSend", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("should return the same array reference when all tool_use blocks have matching tool_results", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + }, + { + role: "assistant", + content: [ + { type: "tool_use", id: "tool_1", name: "read_file", input: { path: "foo.ts" } }, + { type: "tool_use", id: "tool_2", name: "read_file", input: { path: "bar.ts" } }, + ], + }, + { + role: "user", + content: [ + { type: "tool_result", tool_use_id: "tool_1", content: "file contents 1" }, + { type: "tool_result", tool_use_id: "tool_2", content: "file contents 2" }, + ], + }, + ] + + const result = validateMessageHistoryBeforeSend(messages) + expect(result).toBe(messages) // Same reference = no modification + expect(TelemetryService.instance.captureException).not.toHaveBeenCalled() + }) + + it("should inject placeholder tool_results for missing tool_use IDs", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + }, + { + role: "assistant", + content: [ + { type: "tool_use", id: "tool_1", name: "read_file", input: { path: "foo.ts" } }, + { type: "tool_use", id: "tool_2", name: "read_file", input: { path: "bar.ts" } }, + { type: "tool_use", id: "tool_3", name: "read_file", input: { path: "baz.ts" } }, + { type: "tool_use", id: "tool_4", name: "read_file", input: { path: "qux.ts" } }, + ], + }, + { + role: "user", + content: [ + { type: "tool_result", tool_use_id: "tool_1", content: "result 1" }, + // tool_2, tool_3, tool_4 are missing + ], + }, + ] + + const result = validateMessageHistoryBeforeSend(messages) + + // Should have the same number of messages + expect(result.length).toBe(3) + + // The patched user message should contain placeholders for tool_2, tool_3, tool_4 + const patchedUser = result[2] + expect(patchedUser.role).toBe("user") + const content = patchedUser.content as Anthropic.Messages.ContentBlockParam[] + + // 3 placeholders + 1 existing tool_result = 4 + expect(content.length).toBe(4) + + const toolResults = content.filter((b): b is Anthropic.ToolResultBlockParam => b.type === "tool_result") + expect(toolResults.length).toBe(4) + + const toolResultIds = toolResults.map((r) => r.tool_use_id) + expect(toolResultIds).toContain("tool_1") + expect(toolResultIds).toContain("tool_2") + expect(toolResultIds).toContain("tool_3") + expect(toolResultIds).toContain("tool_4") + + // Should report to telemetry + expect(TelemetryService.instance.captureException).toHaveBeenCalledTimes(1) + const capturedError = (TelemetryService.instance.captureException as ReturnType).mock.calls[0][0] + expect(capturedError).toBeInstanceOf(MissingToolResultError) + expect(capturedError.missingToolUseIds).toEqual(["tool_2", "tool_3", "tool_4"]) + }) + + it("should insert a synthetic user message when no following user message exists", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + }, + { + role: "assistant", + content: [{ type: "tool_use", id: "tool_1", name: "read_file", input: { path: "foo.ts" } }], + }, + // No following user message + ] + + const result = validateMessageHistoryBeforeSend(messages) + + // Should now have 3 messages (synthetic user message added) + expect(result.length).toBe(3) + expect(result[2].role).toBe("user") + + const content = result[2].content as Anthropic.ToolResultBlockParam[] + expect(content.length).toBe(1) + expect(content[0].type).toBe("tool_result") + expect(content[0].tool_use_id).toBe("tool_1") + expect(content[0].content).toBe("Tool execution was interrupted before completion.") + }) + + it("should handle multiple assistant messages with tool_use blocks", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + }, + { + role: "assistant", + content: [{ type: "tool_use", id: "tool_1", name: "read_file", input: { path: "foo.ts" } }], + }, + { + role: "user", + content: [{ type: "tool_result", tool_use_id: "tool_1", content: "result 1" }], + }, + { + role: "assistant", + content: [ + { type: "tool_use", id: "tool_2", name: "read_file", input: { path: "bar.ts" } }, + { type: "tool_use", id: "tool_3", name: "read_file", input: { path: "baz.ts" } }, + ], + }, + { + role: "user", + content: [ + // Only tool_2 has a result, tool_3 is missing + { type: "tool_result", tool_use_id: "tool_2", content: "result 2" }, + ], + }, + ] + + const result = validateMessageHistoryBeforeSend(messages) + + expect(result.length).toBe(5) + + // First pair should be untouched + const firstUserContent = result[2].content as Anthropic.Messages.ContentBlockParam[] + expect(firstUserContent.length).toBe(1) + + // Second pair should have placeholder for tool_3 + const secondUserContent = result[4].content as Anthropic.Messages.ContentBlockParam[] + const toolResults = secondUserContent.filter( + (b): b is Anthropic.ToolResultBlockParam => b.type === "tool_result", + ) + expect(toolResults.length).toBe(2) + expect(toolResults.map((r) => r.tool_use_id).sort()).toEqual(["tool_2", "tool_3"]) + }) + + it("should not modify messages without tool_use blocks", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + }, + { + role: "assistant", + content: [{ type: "text", text: "Hi there!" }], + }, + { + role: "user", + content: [{ type: "text", text: "Thanks" }], + }, + ] + + const result = validateMessageHistoryBeforeSend(messages) + expect(result).toBe(messages) // Same reference + }) + + it("should handle assistant messages with string content", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: "Hello", + }, + { + role: "assistant", + content: "Hi there!", + }, + ] + + const result = validateMessageHistoryBeforeSend(messages) + expect(result).toBe(messages) + }) + + it("should handle the next message being an assistant (not user)", () => { + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text", text: "Hello" }], + }, + { + role: "assistant", + content: [{ type: "tool_use", id: "tool_1", name: "read_file", input: { path: "foo.ts" } }], + }, + { + role: "assistant", + content: [{ type: "text", text: "Continuing..." }], + }, + ] + + const result = validateMessageHistoryBeforeSend(messages) + + // Should insert a synthetic user message between the two assistant messages + expect(result.length).toBe(4) + expect(result[2].role).toBe("user") + const syntheticContent = result[2].content as Anthropic.ToolResultBlockParam[] + expect(syntheticContent[0].tool_use_id).toBe("tool_1") + // The original second assistant message should still be there + expect(result[3].role).toBe("assistant") + }) +}) diff --git a/src/core/task/validateToolResultIds.ts b/src/core/task/validateToolResultIds.ts index a966d429ed5..9d2752d9e78 100644 --- a/src/core/task/validateToolResultIds.ts +++ b/src/core/task/validateToolResultIds.ts @@ -232,3 +232,118 @@ export function validateAndFixToolResultIds( content: finalContent, } } + +/** + * Pre-send validation that ensures every tool_use block in the final message + * array has a corresponding tool_result block in the immediately following + * user message. This acts as a last-resort safety net right before the API + * call, catching any mismatches introduced by post-processing steps like + * getEffectiveApiHistory(), mergeConsecutiveApiMessages(), or + * buildCleanConversationHistory(). + * + * For any missing tool_result, a placeholder is injected so the API request + * remains valid. Mismatches are reported to telemetry. + * + * @param messages - The final message array about to be sent to the API + * @returns A new array with any missing tool_result placeholders injected + */ +export function validateMessageHistoryBeforeSend( + messages: Anthropic.Messages.MessageParam[], +): Anthropic.Messages.MessageParam[] { + // Work on a shallow copy so we don't mutate the caller's array. + const result: Anthropic.Messages.MessageParam[] = [] + let modified = false + + for (let i = 0; i < messages.length; i++) { + const current = messages[i] + + // We only care about assistant messages that contain tool_use blocks. + if (current.role !== "assistant" || !Array.isArray(current.content)) { + result.push(current) + continue + } + + const toolUseBlocks = (current.content as Anthropic.Messages.ContentBlockParam[]).filter( + (block): block is Anthropic.ToolUseBlock => block.type === "tool_use", + ) + + if (toolUseBlocks.length === 0) { + result.push(current) + continue + } + + result.push(current) + + // Collect tool_use IDs that need matching tool_results. + const toolUseIds = new Set(toolUseBlocks.map((b) => b.id)) + + // Look at the next message - it should be a user message with tool_results. + const next = messages[i + 1] + + // Gather existing tool_result IDs from the next message (if it's a user message). + const existingToolResultIds = new Set() + let nextContent: Anthropic.Messages.ContentBlockParam[] = [] + + if (next && next.role === "user" && Array.isArray(next.content)) { + nextContent = next.content as Anthropic.Messages.ContentBlockParam[] + for (const block of nextContent) { + if (block.type === "tool_result") { + existingToolResultIds.add((block as Anthropic.ToolResultBlockParam).tool_use_id) + } + } + } + + // Determine which tool_use IDs are missing a tool_result. + const missingIds = [...toolUseIds].filter((id) => !existingToolResultIds.has(id)) + + if (missingIds.length === 0) { + continue // All good for this pair. + } + + // Report to telemetry. + if (TelemetryService.hasInstance()) { + TelemetryService.instance.captureException( + new MissingToolResultError( + `Pre-send validation: missing tool_result blocks for tool_use IDs: [${missingIds.join(", ")}]`, + missingIds, + [...existingToolResultIds], + ), + { + missingToolUseIds: missingIds, + existingToolResultIds: [...existingToolResultIds], + toolUseCount: toolUseBlocks.length, + existingToolResultCount: existingToolResultIds.size, + messageIndex: i, + }, + ) + } + + modified = true + + // Build placeholder tool_result blocks for the missing IDs. + const placeholders: Anthropic.ToolResultBlockParam[] = missingIds.map((id) => ({ + type: "tool_result" as const, + tool_use_id: id, + content: "Tool execution was interrupted before completion.", + })) + + if (next && next.role === "user") { + // Inject placeholders into the existing user message. + const patchedNext: Anthropic.Messages.MessageParam = { + ...next, + content: [...placeholders, ...nextContent], + } + result.push(patchedNext) + i++ // Skip the next message since we already pushed the patched version. + } else { + // No following user message at all - insert a synthetic one. + result.push({ + role: "user" as const, + content: placeholders, + }) + // Don't skip - the next message (if any) still needs to be processed. + } + } + + return modified ? result : messages +}