scriptoriumadmin/Roo-Code
1
0
Fork 0
mirror of https://github.com/RooVetGit/Roo-Code.git synced 2026-09-07 08:26:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix(EXT-665): append tool_result to existing user message instead of creating new one

Browse source 
True root cause: When reopenParentFromDelegation created a NEW user message
with only the new_task tool_result, validateAndFixToolResultIds would see
the assistant message expecting BOTH tool_results (e.g., update_todo_list + new_task)
but the new message only had new_task, causing a placeholder to be generated
for update_todo_list.

Fix: Find the existing user message that was created by flushPendingToolResultsToHistory
(which already has other tool_results like update_todo_list) and APPEND the
new_task tool_result to it instead of creating a new message.

This ensures all tool_results for a turn are in the same user message,
matching what the Anthropic API expects.
This commit is contained in:
Roo Code 2026-01-28 05:17:35 +00:00
parent 8083b7d935
commit 7748b1ce30
2 changed files with 170 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

107
src/__tests__/history-resume-delegation.spec.ts
View file

@ -205,6 +205,7 @@ describe("History resume delegation - parent metadata transitions", () => {
it("reopenParentFromDelegation injects tool_result when new_task tool_use exists in API history", async () => {
const provider = {
contextProxy: { globalStorageUri: { fsPath: "/storage" } },
log: vi.fn(),
getTaskWithId: vi.fn().mockResolvedValue({
historyItem: {
id: "p-tool",
@ -704,4 +705,110 @@ describe("History resume delegation - parent metadata transitions", () => {
expect((lastMsg.content[0] as any).tool_use_id).toBe(toolUseId)
expect((lastMsg.content[0] as any).content).toContain("Subtask c-new completed")
})
it("reopenParentFromDelegation appends to existing user message when multiple tools in one turn (EXT-665 root cause)", async () => {
// This tests the actual root cause of EXT-665:
// When assistant calls multiple tools (e.g., update_todo_list + new_task) in one turn,
// flushPendingToolResultsToHistory saves a user message with update_todo_list tool_result.
// When child completes, we must APPEND new_task tool_result to that existing message,
// NOT create a new message (which would cause validateAndFixToolResultIds to generate
// a placeholder for update_todo_list since the new message only has new_task).
const logSpy = vi.fn()
const provider = {
contextProxy: { globalStorageUri: { fsPath: "/storage" } },
log: logSpy,
getTaskWithId: vi.fn().mockResolvedValue({
historyItem: {
id: "p-multi",
status: "delegated",
awaitingChildId: "c-multi",
childIds: [],
ts: 100,
task: "Parent with multiple tools in one turn",
tokensIn: 0,
tokensOut: 0,
totalCost: 0,
},
}),
emit: vi.fn(),
getCurrentTask: vi.fn(() => ({ taskId: "c-multi" })),
removeClineFromStack: vi.fn().mockResolvedValue(undefined),
createTaskWithHistoryItem: vi.fn().mockResolvedValue({
taskId: "p-multi",
resumeAfterDelegation: vi.fn().mockResolvedValue(undefined),
overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
}),
updateTaskHistory: vi.fn().mockResolvedValue([]),
} as unknown as ClineProvider
// Simulate the EXT-665 scenario:
// - Assistant calls update_todo_list (id=A) AND new_task (id=B) in one turn
// - flushPendingToolResultsToHistory saved tool_result for update_todo_list
const updateTodoListToolId = "toolu_update123"
const newTaskToolId = "toolu_newtask456"
const existingUiMessages = [{ type: "ask", ask: "tool", text: "tools", ts: 50 }]
const existingApiMessages = [
{ role: "user", content: [{ type: "text", text: "Do the work" }], ts: 40 },
{
role: "assistant",
content: [
{
type: "tool_use",
name: "update_todo_list",
id: updateTodoListToolId,
input: { todos: "[ ] Task 1" },
},
{
type: "tool_use",
name: "new_task",
id: newTaskToolId,
input: { mode: "code", message: "Do subtask" },
},
],
ts: 50,
},
// This user message was saved by flushPendingToolResultsToHistory during delegation
// It has tool_result for update_todo_list, but NOT for new_task yet
{
role: "user",
content: [
{
type: "tool_result",
tool_use_id: updateTodoListToolId,
content: "Delegating to subtask...",
},
],
ts: 60,
},
]
vi.mocked(readTaskMessages).mockResolvedValue(existingUiMessages as any)
vi.mocked(readApiMessages).mockResolvedValue(existingApiMessages as any)
await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, {
parentTaskId: "p-multi",
childTaskId: "c-multi",
completionResultSummary: "Subtask completed",
})
// Verify that we logged APPEND (not CREATE)
expect(logSpy).toHaveBeenCalledWith(
expect.stringContaining("Appended new_task tool_result to existing user message"),
)
// Verify API history still has exactly 3 messages (no new message created)
const apiCall = vi.mocked(saveApiMessages).mock.calls[0][0]
expect(apiCall.messages).toHaveLength(3)
// Verify the last user message now has BOTH tool_results
const lastUserMsg = apiCall.messages[2]
expect(lastUserMsg.role).toBe("user")
expect(lastUserMsg.content).toHaveLength(2) // update_todo_list + new_task
// Verify both tool_results are present
const toolResults = (lastUserMsg.content as any[]).filter((b: any) => b.type === "tool_result")
expect(toolResults).toHaveLength(2)
expect(toolResults.map((t: any) => t.tool_use_id).sort()).toEqual([updateTodoListToolId, newTaskToolId].sort())
})
})

84
src/core/webview/ClineProvider.ts
View file

@ -3347,7 +3347,9 @@ export class ClineProvider
// Check ALL user messages for an existing tool_result with this tool_use_id
// (not just the last message, to prevent duplicate tool_results - EXT-665)
let alreadyHasToolResult = false
for (const msg of parentApiMessages) {
let existingUserMsgIndex = -1
for (let i = 0; i < parentApiMessages.length; i++) {
const msg = parentApiMessages[i]
if (msg.role === "user" && Array.isArray(msg.content)) {
for (const block of msg.content) {
if (block.type === "tool_result" && block.tool_use_id === toolUseId) {
@ -3359,31 +3361,71 @@ export class ClineProvider
}
}
if (alreadyHasToolResult) break
// Track the last user message index for potential appending
existingUserMsgIndex = i
}
}
// Only create a NEW user message with the tool_result if none exists
// If no tool_result exists for new_task, we need to add one.
// IMPORTANT: If there's already a user message with tool_results AFTER the assistant
// message (from flushPendingToolResultsToHistory), append to it instead of creating a new one.
// Creating a new user message causes validateAndFixToolResultIds to generate
// placeholder tool_results for OTHER tool_uses that were already satisfied in
// the previous user message (EXT-665 root cause).
if (!alreadyHasToolResult) {
parentApiMessages.push({
role: "user",
content: [
{
type: "tool_result" as const,
tool_use_id: toolUseId,
content: `Subtask ${childTaskId} completed.\n\nResult:\n${completionResultSummary}`,
},
],
ts,
})
}
const newToolResult: Anthropic.ToolResultBlockParam = {
type: "tool_result" as const,
tool_use_id: toolUseId,
content: `Subtask ${childTaskId} completed.\n\nResult:\n${completionResultSummary}`,
}
// Validate the newly injected tool_result against the preceding assistant message.
// This ensures the tool_result's tool_use_id matches a tool_use in the immediately
// preceding assistant message (Anthropic API requirement).
const lastMessage = parentApiMessages[parentApiMessages.length - 1]
if (lastMessage?.role === "user") {
const validatedMessage = validateAndFixToolResultIds(lastMessage, parentApiMessages.slice(0, -1))
parentApiMessages[parentApiMessages.length - 1] = validatedMessage
// Find the assistant message with the new_task tool_use
let assistantMsgIdx = -1
for (let i = parentApiMessages.length - 1; i >= 0; i--) {
const msg = parentApiMessages[i]
if (msg.role === "assistant" && Array.isArray(msg.content)) {
for (const block of msg.content) {
if (block.type === "tool_use" && block.name === "new_task" && block.id === toolUseId) {
assistantMsgIdx = i
break
}
}
if (assistantMsgIdx !== -1) break
}
}
// Find a user message with tool_results that comes AFTER the assistant message
// This would be from flushPendingToolResultsToHistory during delegation
let targetUserMsgIdx = -1
if (assistantMsgIdx !== -1) {
for (let i = assistantMsgIdx + 1; i < parentApiMessages.length; i++) {
const msg = parentApiMessages[i]
if (msg.role === "user" && Array.isArray(msg.content)) {
// Check if this user message has any tool_results
const hasToolResults = msg.content.some((block: any) => block.type === "tool_result")
if (hasToolResults) {
targetUserMsgIdx = i
break
}
}
}
}
if (targetUserMsgIdx !== -1 && Array.isArray(parentApiMessages[targetUserMsgIdx].content)) {
// Append to existing user message that has tool_results
;(parentApiMessages[targetUserMsgIdx].content as Anthropic.ContentBlockParam[]).push(newToolResult)
this.log(
`[reopenParentFromDelegation] Appended new_task tool_result to existing user message at index ${targetUserMsgIdx}`,
)
} else {
// Create new user message if no suitable existing message found
parentApiMessages.push({
role: "user",
content: [newToolResult],
ts,
})
this.log(`[reopenParentFromDelegation] Created new user message for new_task tool_result`)
}
}
} else {
// If there is no corresponding tool_use in the parent API history, we cannot emit a