mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
fix: filter orphan tool_results that reference already-paired tool_uses (GitHub #10494)
This fixes a race condition where deleted queued message text persists and creates duplicate tool_result blocks in subsequent user messages for the same tool_use_id that was already paired in a previous message. Changes: - Add cross-message orphan filtering in validateAndFixToolResultIds() - Scan conversation history to find tool_use_ids already paired - Filter out orphaned tool_results referencing already-paired IDs - Exclude already-paired IDs from missing tool_result calculations - Add comprehensive tests for cross-message scenarios
This commit is contained in:
parent
503f40241d
commit
30a999d804
2 changed files with 202 additions and 2 deletions
|
|
@ -994,4 +994,159 @@ describe("validateAndFixToolResultIds", () => {
|
|||
expect(TelemetryService.instance.captureException).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe("cross-message orphan filtering (GitHub #10494)", () => {
|
||||
it("should filter out tool_result in second user message that references already-paired tool_use", () => {
|
||||
const assistantMessage: Anthropic.MessageParam = {
|
||||
role: "assistant",
|
||||
content: [
|
||||
{
|
||||
type: "tool_use",
|
||||
id: "tooluse_PbLZjpT1QZSfWtIIWnMF4Q",
|
||||
name: "execute_command",
|
||||
input: { command: "npm test" },
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
const existingUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "tool_result",
|
||||
tool_use_id: "tooluse_PbLZjpT1QZSfWtIIWnMF4Q",
|
||||
content: "✅ 21 passed",
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
const orphanUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [
|
||||
{
|
||||
type: "tool_result",
|
||||
tool_use_id: "tooluse_PbLZjpT1QZSfWtIIWnMF4Q",
|
||||
content: "deleted message text that should not appear",
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
const conversationHistory = [assistantMessage, existingUserMessage]
|
||||
const result = validateAndFixToolResultIds(orphanUserMessage, conversationHistory)
|
||||
|
||||
expect(Array.isArray(result.content)).toBe(true)
|
||||
const resultContent = result.content as Anthropic.ToolResultBlockParam[]
|
||||
expect(resultContent.filter((b) => b.type === "tool_result")).toHaveLength(0)
|
||||
})
|
||||
|
||||
it("should keep tool_results in separate messages if they reference different tool_uses", () => {
|
||||
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 existingUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "tool-1", content: "Content A" }],
|
||||
}
|
||||
|
||||
const secondUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "tool-2", content: "Content B" }],
|
||||
}
|
||||
|
||||
const conversationHistory = [assistantMessage, existingUserMessage]
|
||||
const result = validateAndFixToolResultIds(secondUserMessage, conversationHistory)
|
||||
|
||||
expect(Array.isArray(result.content)).toBe(true)
|
||||
const resultContent = result.content as Anthropic.ToolResultBlockParam[]
|
||||
expect(resultContent.filter((b) => b.type === "tool_result")).toHaveLength(1)
|
||||
expect(resultContent[0].tool_use_id).toBe("tool-2")
|
||||
})
|
||||
|
||||
it("should preserve non-tool_result content when filtering orphaned tool_results", () => {
|
||||
const assistantMessage: Anthropic.MessageParam = {
|
||||
role: "assistant",
|
||||
content: [{ type: "tool_use", id: "tool-123", name: "execute_command", input: { command: "npm test" } }],
|
||||
}
|
||||
|
||||
const existingUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "tool-123", content: "Command output" }],
|
||||
}
|
||||
|
||||
const orphanUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "tool_result", tool_use_id: "tool-123", content: "Orphan result" },
|
||||
{ type: "text", text: "Environment details here" },
|
||||
],
|
||||
}
|
||||
|
||||
const conversationHistory = [assistantMessage, existingUserMessage]
|
||||
const result = validateAndFixToolResultIds(orphanUserMessage, conversationHistory)
|
||||
|
||||
expect(Array.isArray(result.content)).toBe(true)
|
||||
const resultContent = result.content as Array<Anthropic.ToolResultBlockParam | Anthropic.TextBlockParam>
|
||||
expect(resultContent.filter((b) => b.type === "tool_result")).toHaveLength(0)
|
||||
expect(resultContent.filter((b) => b.type === "text")).toHaveLength(1)
|
||||
expect((resultContent.find((b) => b.type === "text") as Anthropic.TextBlockParam).text).toBe("Environment details here")
|
||||
})
|
||||
|
||||
it("should handle multiple tool_uses with mixed valid and orphaned 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 existingUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "tool-1", content: "Content A" }],
|
||||
}
|
||||
|
||||
const mixedUserMessage: Anthropic.MessageParam = {
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "tool_result", tool_use_id: "tool-1", content: "Orphan content for tool-1" },
|
||||
{ type: "tool_result", tool_use_id: "tool-2", content: "Valid content for tool-2" },
|
||||
],
|
||||
}
|
||||
|
||||
const conversationHistory = [assistantMessage, existingUserMessage]
|
||||
const result = validateAndFixToolResultIds(mixedUserMessage, conversationHistory)
|
||||
|
||||
expect(Array.isArray(result.content)).toBe(true)
|
||||
const resultContent = result.content as Anthropic.ToolResultBlockParam[]
|
||||
const toolResults = resultContent.filter((b) => b.type === "tool_result")
|
||||
expect(toolResults).toHaveLength(1)
|
||||
expect(toolResults[0].tool_use_id).toBe("tool-2")
|
||||
expect(toolResults[0].content).toBe("Valid content for tool-2")
|
||||
})
|
||||
|
||||
it("should not filter when there are no previous user messages after the assistant message", () => {
|
||||
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 conversationHistory = [assistantMessage]
|
||||
const result = validateAndFixToolResultIds(userMessage, conversationHistory)
|
||||
|
||||
expect(Array.isArray(result.content)).toBe(true)
|
||||
const resultContent = result.content as Anthropic.ToolResultBlockParam[]
|
||||
expect(resultContent.filter((b) => b.type === "tool_result")).toHaveLength(1)
|
||||
expect(resultContent[0].tool_use_id).toBe("tool-123")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ export class MissingToolResultError extends Error {
|
|||
* - Message editing scenarios
|
||||
* - Resume/delegation scenarios
|
||||
* - Missing tool_result blocks for tool_use calls
|
||||
* - Cross-message orphan tool_results from race conditions (GitHub #10494)
|
||||
*
|
||||
* @param userMessage - The user message being added to history
|
||||
* @param apiConversationHistory - The conversation history to find the previous assistant message from
|
||||
|
|
@ -109,12 +110,56 @@ export function validateAndFixToolResultIds(
|
|||
// Build a set of valid tool_use IDs
|
||||
const validToolUseIds = new Set(toolUseBlocks.map((block) => block.id))
|
||||
|
||||
// === Cross-message orphan filtering (GitHub #10494) ===
|
||||
// Scan forward through conversation history to find which tool_use_ids have
|
||||
// already been paired with tool_results in previous user messages.
|
||||
// This prevents orphaned tool_results from duplicate responses (e.g., queued
|
||||
// message deletion race condition, retry logic, etc.) from being submitted.
|
||||
const alreadyPairedToolUseIds = new Set<string>()
|
||||
for (let i = prevAssistantIdx + 1; i < apiConversationHistory.length; i++) {
|
||||
const msg = apiConversationHistory[i]
|
||||
if (msg.role === "user" && Array.isArray(msg.content)) {
|
||||
const previousToolResults = msg.content.filter(
|
||||
(block): block is Anthropic.ToolResultBlockParam => block.type === "tool_result",
|
||||
)
|
||||
previousToolResults.forEach((tr) => alreadyPairedToolUseIds.add(tr.tool_use_id))
|
||||
}
|
||||
}
|
||||
|
||||
// Filter out tool_results that reference tool_uses which are already paired
|
||||
// (i.e., they are orphans - the tool_use is no longer available for pairing)
|
||||
if (alreadyPairedToolUseIds.size > 0) {
|
||||
const contentArray = userMessage.content as Anthropic.Messages.ContentBlockParam[]
|
||||
const filteredContent = contentArray.filter((block: Anthropic.Messages.ContentBlockParam) => {
|
||||
if (block.type !== "tool_result") {
|
||||
return true // Keep all non-tool_result blocks
|
||||
}
|
||||
// Filter out tool_results that reference already-paired tool_uses
|
||||
if (alreadyPairedToolUseIds.has(block.tool_use_id)) {
|
||||
return false // This is an orphan - tool_use already paired in previous message
|
||||
}
|
||||
return true
|
||||
})
|
||||
|
||||
userMessage = {
|
||||
...userMessage,
|
||||
content: filteredContent,
|
||||
}
|
||||
|
||||
// Re-extract toolResults from filtered content for subsequent processing
|
||||
toolResults = filteredContent.filter(
|
||||
(block: Anthropic.Messages.ContentBlockParam): block is Anthropic.ToolResultBlockParam =>
|
||||
block.type === "tool_result",
|
||||
)
|
||||
}
|
||||
// === End cross-message orphan filtering ===
|
||||
|
||||
// Build a set of existing tool_result IDs
|
||||
const existingToolResultIds = new Set(toolResults.map((r) => r.tool_use_id))
|
||||
|
||||
// Check for missing tool_results (tool_use IDs that don't have corresponding tool_results)
|
||||
const missingToolUseIds = toolUseBlocks
|
||||
.filter((toolUse) => !existingToolResultIds.has(toolUse.id))
|
||||
.filter((toolUse) => !existingToolResultIds.has(toolUse.id) && !alreadyPairedToolUseIds.has(toolUse.id))
|
||||
.map((toolUse) => toolUse.id)
|
||||
|
||||
// Check if any tool_result has an invalid ID
|
||||
|
|
@ -212,7 +257,7 @@ export function validateAndFixToolResultIds(
|
|||
.map((r: Anthropic.ToolResultBlockParam) => r.tool_use_id),
|
||||
)
|
||||
|
||||
const stillMissingToolUseIds = toolUseBlocks.filter((toolUse) => !coveredToolUseIds.has(toolUse.id))
|
||||
const stillMissingToolUseIds = toolUseBlocks.filter((toolUse) => !coveredToolUseIds.has(toolUse.id) && !alreadyPairedToolUseIds.has(toolUse.id))
|
||||
|
||||
// Build final content: add missing tool_results at the beginning if any
|
||||
const missingToolResults: Anthropic.ToolResultBlockParam[] = stillMissingToolUseIds.map((toolUse) => ({
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue