mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
This commit is contained in:
committed by
GitHub
parent
f688d1150f
commit
e0615a8194
@@ -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))
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<string, unknown>[],
|
||||
provider: string
|
||||
): Record<string, unknown>[] {
|
||||
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<string, unknown>[]).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);
|
||||
}
|
||||
|
||||
84
tests/unit/mistral-trailing-assistant.test.ts
Normal file
84
tests/unit/mistral-trailing-assistant.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user