diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index e573a7f107..4988376380 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -140,6 +140,7 @@ import { type AutoDenyDetail, AutoApprovalHandler, checkAutoApproval } from "../ import { MessageManager } from "../message-manager" import { validateAndFixToolResultIds } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" +import { orderToolResultsFirst } from "./orderToolResultsFirst" import { prepareApiConversationMessage } from "./apiConversationHistory" import { shouldAddUserMessageToHistory } from "./messageCounting" import { type TaskExecutionContext } from "./providerHandoff" @@ -5830,7 +5831,11 @@ export class Task extends EventEmitter implements TaskLike { const messagesSinceLastSummary = getMessagesSinceLastSummary(effectiveHistory) // For API only: merge consecutive user messages (excludes summary messages per // mergeConsecutiveApiMessages implementation) without mutating stored history. - const mergedForApi = mergeConsecutiveApiMessages(messagesSinceLastSummary, { roles: ["user"] }) + // Then lead each user message with its tool_result blocks, which also repairs history saved + // with images between results (#1774) so a stuck task can retry successfully. + const mergedForApi = orderToolResultsFirst( + mergeConsecutiveApiMessages(messagesSinceLastSummary, { roles: ["user"] }), + ) const messagesWithoutImages = maybeRemoveImageBlocks(mergedForApi, this.api) const cleanConversationHistory = this.buildCleanConversationHistory( messagesWithoutImages as ApiMessage[], diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index ad07230204..1dda1555a6 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2971,6 +2971,81 @@ describe("Cline", () => { expect(Object.keys(cleanConversationHistory[0]!)).toEqual(["role", "content"]) }) + it("sends tool_result blocks first when stored history has images between them (#1774)", async () => { + const cline = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + vi.spyOn(getTaskTestAccess(cline), "getSystemPrompt").mockResolvedValue("mock system prompt") + // Image-capable model so the image block is kept in the outgoing request. + vi.spyOn(cline.api, "getModel").mockReturnValue({ + id: "claude-3-sonnet", + info: { + supportsImages: true, + supportsPromptCache: false, + contextWindow: 200000, + maxTokens: 4096, + } as ModelInfo, + }) + const mockStream = { + async *[Symbol.asyncIterator]() { + yield { type: "text", text: "response" } + }, + } as AsyncGenerator + const createMessageSpy = vi.spyOn(cline.api, "createMessage").mockReturnValue(mockStream) + + // History as persisted by a turn that read an image alongside another tool call. + cline.apiConversationHistory = [ + { role: "user", content: [{ type: "text", text: "read the logo and list files" }], ts: 1 }, + { + role: "assistant", + content: [ + { type: "tool_use", id: "read", name: "read_file", input: { path: "logo.jpg" } }, + { type: "tool_use", id: "cmd", name: "execute_command", input: { command: "ls" } }, + ], + ts: 2, + }, + { + role: "user", + content: [ + { type: "tool_result", tool_use_id: "read", content: "File: logo.jpg" }, + { type: "image", source: { type: "base64", media_type: "image/jpeg", data: "abc" } }, + { type: "tool_result", tool_use_id: "cmd", content: "Exit code: 0" }, + { type: "text", text: "" }, + ], + ts: 3, + }, + ] + + const iterator = cline.attemptApiRequest(0) + await iterator.next() + + const [, sentHistory] = requireDefined(createMessageSpy.mock.calls[0]) + const lastUserContent = requireDefined(sentHistory.at(-1)).content as Array<{ type: string }> + // Both results lead; the image sits inside the read_file result; the environment details follow. + expect(lastUserContent).toEqual([ + { + type: "tool_result", + tool_use_id: "read", + content: [ + { type: "text", text: "File: logo.jpg" }, + { type: "image", source: { type: "base64", media_type: "image/jpeg", data: "abc" } }, + ], + }, + { type: "tool_result", tool_use_id: "cmd", content: "Exit code: 0" }, + { type: "text", text: "" }, + ]) + // The stored history keeps its original shape. + expect(requireDefined(cline.apiConversationHistory.at(-1)).content).toMatchObject([ + { type: "tool_result" }, + { type: "image" }, + { type: "tool_result" }, + { type: "text" }, + ]) + }) + it("should shape image blocks for API compatibility before request construction", async () => { const conversationHistory = [ { diff --git a/src/core/task/__tests__/orderToolResultsFirst.spec.ts b/src/core/task/__tests__/orderToolResultsFirst.spec.ts new file mode 100644 index 0000000000..6c6b93595a --- /dev/null +++ b/src/core/task/__tests__/orderToolResultsFirst.spec.ts @@ -0,0 +1,115 @@ +// npx vitest run core/task/__tests__/orderToolResultsFirst.spec.ts + +import type { Anthropic } from "@anthropic-ai/sdk" + +import type { ApiMessage } from "../../task-persistence" +import { mergeConsecutiveApiMessages } from "../mergeConsecutiveApiMessages" +import { orderToolResultsFirst } from "../orderToolResultsFirst" + +const toolResult = (id: string, content = `result ${id}`): Anthropic.Messages.ToolResultBlockParam => ({ + type: "tool_result", + tool_use_id: id, + content, +}) +const image = (data: string): Anthropic.Messages.ImageBlockParam => ({ + type: "image", + source: { type: "base64", media_type: "image/jpeg", data }, +}) +const text = (value: string): Anthropic.Messages.TextBlockParam => ({ type: "text", text: value }) +const nested = (id: string, content: Anthropic.Messages.ToolResultBlockParam["content"]) => ({ + type: "tool_result" as const, + tool_use_id: id, + content, +}) + +describe("orderToolResultsFirst", () => { + it("repairs the persisted #1774 shape so every tool_result leads the message", () => { + // [tool_result, image, tool_result, text] as saved when read_file on an image ran + // alongside another tool call in the same turn. + const stuck: ApiMessage = { + role: "user", + ts: 16, + content: [toolResult("read"), image("jpeg"), toolResult("cmd"), text("")], + } + + const [repaired] = orderToolResultsFirst([stuck]) + + // The image moves into the read_file result, the shape the Anthropic API expects. + expect(repaired.content).toEqual([ + nested("read", [text("result read"), image("jpeg")]), + toolResult("cmd"), + text(""), + ]) + expect(repaired.ts).toBe(16) + // Stored history is untouched. + expect(stuck.content).toEqual([ + toolResult("read"), + image("jpeg"), + toolResult("cmd"), + text(""), + ]) + }) + + it("keeps the order of results and of other blocks with several interleaved images", () => { + const [repaired] = orderToolResultsFirst([ + { + role: "user", + content: [toolResult("a"), toolResult("b"), image("1"), toolResult("c"), image("2"), text("t")], + }, + ]) + + expect(repaired.content).toEqual([ + toolResult("a"), + nested("b", [text("result b"), image("1")]), + nested("c", [text("result c"), image("2")]), + text("t"), + ]) + }) + + it("appends an image to a result whose content is already an array", () => { + const withArray = nested("a", [text("first"), text("second")]) + + const [repaired] = orderToolResultsFirst([{ role: "user", content: [withArray, image("1"), toolResult("b")] }]) + + expect(repaired.content).toEqual([nested("a", [text("first"), text("second"), image("1")]), toolResult("b")]) + // The original result block is not mutated. + expect(withArray.content).toEqual([text("first"), text("second")]) + }) + + it("keeps an image standalone when no result directly precedes it", () => { + const [repaired] = orderToolResultsFirst([ + { + role: "user", + content: [image("lead"), toolResult("a"), text("note"), image("after-text"), toolResult("b")], + }, + ]) + + expect(repaired.content).toEqual([ + toolResult("a"), + toolResult("b"), + image("lead"), + text("note"), + image("after-text"), + ]) + }) + + it("repairs interleaving introduced by merging consecutive user messages", () => { + const merged = mergeConsecutiveApiMessages([ + { role: "user", content: [toolResult("a"), text("note")] }, + { role: "user", content: [toolResult("b")] }, + ]) + + expect(orderToolResultsFirst(merged)[0].content).toEqual([toolResult("a"), toolResult("b"), text("note")]) + }) + + it("returns already-ordered, string, and assistant messages unchanged", () => { + const ordered: ApiMessage = { role: "user", content: [toolResult("a"), image("1"), text("t")] } + const plain: ApiMessage = { role: "user", content: "hello" } + const noResults: ApiMessage = { role: "user", content: [text("a"), image("1")] } + const assistant: ApiMessage = { role: "assistant", content: [text("a")] } + + const out = orderToolResultsFirst([ordered, plain, noResults, assistant]) + + out.forEach((message, index) => expect(message).toBe([ordered, plain, noResults, assistant][index])) + }) +}) diff --git a/src/core/task/orderToolResultsFirst.ts b/src/core/task/orderToolResultsFirst.ts new file mode 100644 index 0000000000..24a0059e65 --- /dev/null +++ b/src/core/task/orderToolResultsFirst.ts @@ -0,0 +1,56 @@ +import type { Anthropic } from "@anthropic-ai/sdk" + +import type { ApiMessage } from "../task-persistence" + +type Block = Anthropic.Messages.ContentBlockParam +type ToolResultBlock = Anthropic.Messages.ToolResultBlockParam + +function toContentArray(content: ToolResultBlock["content"]): Exclude { + if (typeof content === "string") return [{ type: "text", text: content }] + return content ?? [] +} + +/** + * Moves `tool_result` blocks to the front of each user message, keeping the relative order of + * the results and of the remaining blocks. An image that directly follows a result (as when + * `read_file` returns an image) is nested into that result's `content`, where Anthropic expects it. + * + * Anthropic requires the results for the previous turn's `tool_use` blocks to lead the next user + * message. History saved with image blocks between results (#1307, #1774), or produced by merging + * consecutive user messages, is otherwise rejected with a 400 on every retry. + * + * Used for *API request shaping only* (do not use for storage). Messages already in order are + * returned unchanged. + */ +export function orderToolResultsFirst(messages: ApiMessage[]): ApiMessage[] { + return messages.map((message) => { + if (message.role !== "user" || !Array.isArray(message.content)) { + return message + } + const firstOther = message.content.findIndex((block) => block.type !== "tool_result") + const hasLaterResult = + firstOther !== -1 && message.content.slice(firstOther).some((block) => block.type === "tool_result") + if (!hasLaterResult) { + return message + } + + const results: ToolResultBlock[] = [] + const rest: Block[] = [] + // Index of the result an immediately following image belongs to; reset by any other block. + let owner = -1 + for (const block of message.content) { + if (block.type === "tool_result") { + results.push(block) + owner = results.length - 1 + } else if (block.type === "image" && owner !== -1) { + const current = results[owner] + results[owner] = { ...current, content: [...toContentArray(current.content), block] } + } else { + owner = -1 + rest.push(block) + } + } + + return { ...message, content: [...results, ...rest] } + }) +}