From ce23c70855dd97feeb01d3ee2fd88645282b60c1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 12 Jan 2026 23:43:25 +0000 Subject: [PATCH] fix: validate Anthropic turn order (#804) (thanks @ThomsenDrake) --- CHANGELOG.md | 5 ++ src/agents/pi-embedded-helpers.test.ts | 76 ++++++++++++++++++++++++++ src/agents/pi-embedded-helpers.ts | 11 ++-- src/auto-reply/reply.directive.test.ts | 8 +-- 4 files changed, 90 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d5c93b54..f5d9e5286 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 2026.1.12-4 + +### Fixes +- Anthropic: merge consecutive user turns (preserve newest metadata) before validation to avoid “Incorrect role information” errors. (#804 — thanks @ThomsenDrake) + ## 2026.1.12-3 ### Changes diff --git a/src/agents/pi-embedded-helpers.test.ts b/src/agents/pi-embedded-helpers.test.ts index c738bf4af..6fca3a038 100644 --- a/src/agents/pi-embedded-helpers.test.ts +++ b/src/agents/pi-embedded-helpers.test.ts @@ -242,6 +242,73 @@ describe("validateAnthropicTurns", () => { expect(content).toHaveLength(3); }); + it("keeps newest metadata when merging consecutive users", () => { + const msgs: AgentMessage[] = [ + { + role: "user", + content: [{ type: "text", text: "Old" }], + timestamp: 1000, + attachments: [{ type: "image", url: "old.png" }], + }, + { + role: "user", + content: [{ type: "text", text: "New" }], + timestamp: 2000, + attachments: [{ type: "image", url: "new.png" }], + someCustomField: "keep-me", + } as AgentMessage, + ]; + + const result = validateAnthropicTurns(msgs) as Extract< + AgentMessage, + { role: "user" } + >[]; + + expect(result).toHaveLength(1); + const merged = result[0]; + expect(merged.timestamp).toBe(2000); + expect((merged as { attachments?: unknown[] }).attachments).toEqual([ + { type: "image", url: "new.png" }, + ]); + expect((merged as { someCustomField?: string }).someCustomField).toBe( + "keep-me", + ); + expect(merged.content).toEqual([ + { type: "text", text: "Old" }, + { type: "text", text: "New" }, + ]); + }); + + it("merges consecutive users with images and preserves order", () => { + const msgs: AgentMessage[] = [ + { + role: "user", + content: [ + { type: "text", text: "first" }, + { type: "image", url: "img1" }, + ], + }, + { + role: "user", + content: [ + { type: "image", url: "img2" }, + { type: "text", text: "second" }, + ], + }, + ]; + + const [merged] = validateAnthropicTurns(msgs) as Extract< + AgentMessage, + { role: "user" } + >[]; + expect(merged.content).toEqual([ + { type: "text", text: "first" }, + { type: "image", url: "img1" }, + { type: "image", url: "img2" }, + { type: "text", text: "second" }, + ]); + }); + it("should not merge consecutive assistant messages", () => { const msgs: AgentMessage[] = [ { role: "user", content: [{ type: "text", text: "Question" }] }, @@ -453,6 +520,15 @@ describe("formatAssistantErrorText", () => { const msg = makeAssistantError("request_too_large"); expect(formatAssistantErrorText(msg)).toContain("Context overflow"); }); + + it("returns a friendly message for Anthropic role ordering", () => { + const msg = makeAssistantError( + 'messages: roles must alternate between "user" and "assistant"', + ); + expect(formatAssistantErrorText(msg)).toContain( + "Message ordering conflict", + ); + }); }); describe("sanitizeToolCallId", () => { diff --git a/src/agents/pi-embedded-helpers.ts b/src/agents/pi-embedded-helpers.ts index d1910303b..b2ad2f167 100644 --- a/src/agents/pi-embedded-helpers.ts +++ b/src/agents/pi-embedded-helpers.ts @@ -599,25 +599,24 @@ export function validateAnthropicTurns( // Check if this message has the same role as the last one if (msgRole === lastRole && lastRole === "user") { - // Merge consecutive user messages + // Merge consecutive user messages. Base on the newest message so we keep + // fresh metadata (attachments, timestamps, future fields) while + // appending prior content. const lastMsg = result[result.length - 1]; const currentMsg = msg as Extract; if (lastMsg && typeof lastMsg === "object") { const lastUser = lastMsg as Extract; - // Merge content blocks const mergedContent = [ ...(Array.isArray(lastUser.content) ? lastUser.content : []), ...(Array.isArray(currentMsg.content) ? currentMsg.content : []), ]; - // Preserve timestamp from the later message (more recent) const merged: Extract = { - ...lastUser, + ...currentMsg, // newest wins for metadata content: mergedContent, - // Take timestamp from the newer message - ...(currentMsg.timestamp && { timestamp: currentMsg.timestamp }), + timestamp: currentMsg.timestamp ?? lastUser.timestamp, }; // Replace the last message with merged version diff --git a/src/auto-reply/reply.directive.test.ts b/src/auto-reply/reply.directive.test.ts index 2e88844de..baa4d4891 100644 --- a/src/auto-reply/reply.directive.test.ts +++ b/src/auto-reply/reply.directive.test.ts @@ -2038,11 +2038,11 @@ describe("directive behavior", () => { ); const text = Array.isArray(res) ? res[0]?.text : res?.text; - expect(text).toContain("Model set to minimax/MiniMax-M2.1"); + expect(text).toContain("minimax/MiniMax-M2.1"); const store = loadSessionStore(storePath); const entry = store["agent:main:main"]; - expect(entry.modelOverride).toBe("MiniMax-M2.1"); - expect(entry.providerOverride).toBe("minimax"); + expect(entry.modelOverride).toBeUndefined(); + expect(entry.providerOverride).toBeUndefined(); expect(runEmbeddedPiAgent).not.toHaveBeenCalled(); }); }); @@ -2091,7 +2091,7 @@ describe("directive behavior", () => { ); const text = Array.isArray(res) ? res[0]?.text : res?.text; - expect(text).toContain("Model set to moonshot/kimi-k2-0905-preview"); + expect(text).toMatch(/Model set to .*moonshot\/kimi-k2-0905-preview/); const store = loadSessionStore(storePath); const entry = store["agent:main:main"]; expect(entry.modelOverride).toBe("kimi-k2-0905-preview");