diff --git a/CHANGELOG.md b/CHANGELOG.md index 75f7d62cca..0d58d90f13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ _Development cycle in progress — entries are added as work merges into `releas ### 🔧 Bug Fixes - **fix(mitm):** `getMitmStatus()` in the build-time stub (Docker image) now returns a graceful `{ running: false }` status instead of throwing, so the Agent Bridge UI shows a clean "stopped" state rather than an error banner in containerised deployments. ([#3390](https://github.com/diegosouzapw/OmniRoute/issues/3390)) +- **fix(executor):** Mistral (and any provider in `PROVIDERS_REQUIRING_USER_LAST_MESSAGE`) no longer receives a trailing `assistant` message with plain text content — `stripTrailingAssistantForProvider` drops it on the upstream-send path, fixing the `400: Expected last role User or Tool … but got assistant` rejection. ([#3396](https://github.com/diegosouzapw/OmniRoute/issues/3396)) --- diff --git a/open-sse/executors/base.ts b/open-sse/executors/base.ts index 6324f7bb94..5521615f7c 100644 --- a/open-sse/executors/base.ts +++ b/open-sse/executors/base.ts @@ -34,6 +34,7 @@ import { fixToolPairs, fixToolAdjacency, stripTrailingAssistantOrphanToolUse, + stripTrailingAssistantForProvider, } from "../services/contextManager.ts"; import { randomUUID } from "node:crypto"; import { @@ -1076,7 +1077,10 @@ export class BaseExecutor { // tool_result isn't in the next message; re-run fixToolPairs to // drop any tool_result orphaned by that strip (discussion #2410). const adjacent = isClaude ? fixToolPairs(fixToolAdjacency(fixed)) : fixed; - tb.messages = stripTrailingAssistantOrphanToolUse(adjacent); + const stripped = stripTrailingAssistantOrphanToolUse(adjacent); + // Some providers (e.g. Mistral) require the last message to be user + // or tool and reject trailing assistant text messages with 400 (#3396). + tb.messages = stripTrailingAssistantForProvider(stripped, this.provider); } } let bodyString = JSON.stringify(transformedBody); diff --git a/open-sse/services/contextManager.ts b/open-sse/services/contextManager.ts index c31215c7d8..baad8b0204 100644 --- a/open-sse/services/contextManager.ts +++ b/open-sse/services/contextManager.ts @@ -555,3 +555,39 @@ export function stripTrailingAssistantOrphanToolUse( if (hasContent || hasToolCalls) result.push(newLast); return result; } + +/** + * Providers that strictly require the last message to be `user` or `tool`. + * A trailing `assistant` message with plain text content (no tool_use) is + * valid for Anthropic/OpenAI (signals "continue from here") but rejected by + * Mistral with: "Expected last role User or Tool … but got assistant" (#3396). + */ +const PROVIDERS_REQUIRING_USER_LAST_MESSAGE = new Set(["mistral"]); + +/** + * Strip a trailing `assistant` message that contains ONLY plain text (no + * `tool_use` / `tool_calls`) for providers that mandate user-last format. + * + * Call this AFTER `stripTrailingAssistantOrphanToolUse` on the upstream-send + * path so `tool_use` orphans are already removed before this check runs. + */ +export function stripTrailingAssistantForProvider( + messages: Record[], + provider: string +): Record[] { + if (!PROVIDERS_REQUIRING_USER_LAST_MESSAGE.has(provider)) return messages; + if (!Array.isArray(messages) || messages.length === 0) return messages; + + const last = messages[messages.length - 1]; + if (!last || last.role !== "assistant") return messages; + + // Only strip when the message has NO tool_use / tool_calls (those are + // handled by stripTrailingAssistantOrphanToolUse upstream of this call). + const hasToolUse = + Array.isArray(last.content) && + (last.content as Record[]).some((b) => b.type === "tool_use"); + const hasToolCalls = Array.isArray(last.tool_calls) && (last.tool_calls as unknown[]).length > 0; + if (hasToolUse || hasToolCalls) return messages; + + return messages.slice(0, messages.length - 1); +} diff --git a/tests/unit/mistral-trailing-assistant.test.ts b/tests/unit/mistral-trailing-assistant.test.ts new file mode 100644 index 0000000000..1ec5769d0d --- /dev/null +++ b/tests/unit/mistral-trailing-assistant.test.ts @@ -0,0 +1,84 @@ +/** + * Regression test for #3396: Mistral returns 400 when the last message is + * `role: "assistant"` with plain text content. + * + * `stripTrailingAssistantOrphanToolUse` only removed tool_use blocks — it left + * trailing text-only assistant messages intact. Mistral (and providers sharing + * the same constraint) reject such requests with: + * "400: Expected last role User or Tool (or Assistant with prefix True) + * for serving but got assistant" + * + * The fix adds `stripTrailingAssistantForProvider(messages, provider)` which + * also drops a trailing text-only assistant message for providers that require + * user-last format (e.g. "mistral"). + */ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { stripTrailingAssistantOrphanToolUse } from "../../open-sse/services/contextManager.ts"; +import { stripTrailingAssistantForProvider } from "../../open-sse/services/contextManager.ts"; + +const user = (content: string) => ({ role: "user", content }); +const assistant = (content: string) => ({ role: "assistant", content }); +const assistantWithToolUse = () => ({ + role: "assistant", + content: [{ type: "tool_use", id: "t1", name: "fn", input: {} }], +}); + +describe("stripTrailingAssistantForProvider (#3396)", () => { + it("strips trailing text-only assistant message for mistral", () => { + const msgs = [user("hi"), assistant("hello from model")]; + const result = stripTrailingAssistantForProvider(msgs, "mistral"); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].role, "user"); + }); + + it("strips trailing assistant with array-string content for mistral", () => { + const msgs = [ + user("hi"), + { role: "assistant", content: [{ type: "text", text: "response" }] }, + ]; + const result = stripTrailingAssistantForProvider(msgs, "mistral"); + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].role, "user"); + }); + + it("does NOT strip trailing tool_use assistant (stripTrailingOrphan handles that)", () => { + const msgs = [user("hi"), assistantWithToolUse()]; + const result = stripTrailingAssistantForProvider(msgs, "mistral"); + // tool_use trailing is already handled by stripTrailingAssistantOrphanToolUse; + // this function should not double-strip or corrupt tool-use state + assert.strictEqual(result.length, 2); + }); + + it("does NOT strip trailing text assistant for non-mistral providers", () => { + const msgs = [user("hi"), assistant("response")]; + const result = stripTrailingAssistantForProvider(msgs, "openai"); + assert.strictEqual(result.length, 2); + }); + + it("does NOT strip trailing text assistant for anthropic/claude", () => { + const msgs = [user("hi"), assistant("continue from here")]; + const result = stripTrailingAssistantForProvider(msgs, "claude"); + assert.strictEqual(result.length, 2); + }); + + it("returns messages unchanged when last message is user", () => { + const msgs = [assistant("a"), user("b")]; + const result = stripTrailingAssistantForProvider(msgs, "mistral"); + assert.strictEqual(result.length, 2); + assert.strictEqual(result[1].role, "user"); + }); + + it("returns empty array unchanged", () => { + const result = stripTrailingAssistantForProvider([], "mistral"); + assert.deepStrictEqual(result, []); + }); + + it("existing stripTrailingAssistantOrphanToolUse still works after change", () => { + const msgs = [user("hi"), assistantWithToolUse()]; + const result = stripTrailingAssistantOrphanToolUse(msgs); + // tool_use block removed; empty assistant → dropped + assert.strictEqual(result.length, 1); + assert.strictEqual(result[0].role, "user"); + }); +});