mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
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.
This commit is contained in:
parent
ad25634905
commit
dfaf0e8353
3 changed files with 346 additions and 2 deletions
|
|
@ -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<TaskEvents> 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]()
|
||||
|
|
|
|||
|
|
@ -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<typeof vi.fn>).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")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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<string>()
|
||||
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
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue