mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 20:32:25 +03:00
fix(sse): demote mid-conversation system roles to user in claude-to-openai translation (#12908)
* fix(sse): demote mid-conversation system roles to user in claude-to-openai translation Claude Code hook contexts (SessionStart ~25KB, PreToolUse) arrive as role:system messages in the middle of the messages array. HCP-Vision-Latest vLLM (via LiteLLM gateway) rejects any system not at index 0 with 400 "System message must be at the beginning." Demote every system at output index > 0 to user with content preserved byte-identical; the index-0 system (translator-made from the top-level system field, or client-placed first) stays untouched. Other upstreams are unaffected: 135/138 recent mid-system calls to GLM-5.3-Flash / DeepSeek-V4-Flash already returned 200. Supersedes the pass-through assertion of #6954 (its intent — systems never misattributed as assistant — still holds). * test(sse): harden mid-system demotion with env guard and array-path invariant note Quality-review Minor 1: document the array-return-path invariant in claudeToOpenAIRequest (convertClaudeMessage arrays are tool/user only, so no second system element can survive demotion while result.messages is empty). Minor 2: add the same OMNIROUTE_SYSTEM_INSTRUCTION_APPEND env guard to the #6954 test as the new mid-system test for consistency. APPROVED items, no behavior change. --------- Co-authored-by: Jihyun Son <jihyun.son@sk.com>
This commit is contained in:
@@ -171,10 +171,24 @@ export function claudeToOpenAIRequest(model, body, stream, credentials: unknown
|
||||
const msg = body.messages[i];
|
||||
const converted = convertClaudeMessage(msg, preserveCacheControl);
|
||||
if (converted) {
|
||||
// Handle array of messages (multiple tool results)
|
||||
// Claude Code hook contexts (SessionStart/PreToolUse) arrive as
|
||||
// role:"system" mid-array — strictly valid for the Anthropic Messages
|
||||
// API, but OpenAI-compatible upstreams reject a system turn after the
|
||||
// first message (HCP-Vision-Latest vLLM: 400 "System message must be
|
||||
// at the beginning."). Demote every system at index > 0 to "user",
|
||||
// keeping the content byte-identical; the index-0 system (the
|
||||
// translator-made one above, or one the client put first) stays.
|
||||
const demoteMidSystem = (out: JsonRecord) => {
|
||||
if (out.role === "system" && result.messages.length > 0) out.role = "user";
|
||||
};
|
||||
// Array return is tool/user elements only (never role:"system") — a
|
||||
// second system here would skip demotion while result.messages is
|
||||
// still empty and survive as a mid-array system.
|
||||
if (Array.isArray(converted)) {
|
||||
converted.forEach(demoteMidSystem);
|
||||
result.messages.push(...converted);
|
||||
} else {
|
||||
demoteMidSystem(converted);
|
||||
result.messages.push(converted);
|
||||
}
|
||||
}
|
||||
|
||||
158
tests/unit/claude-to-openai-mid-system-user-normalize.test.ts
Normal file
158
tests/unit/claude-to-openai-mid-system-user-normalize.test.ts
Normal file
@@ -0,0 +1,158 @@
|
||||
/**
|
||||
* Claude Code hook contexts (SessionStart hook ~25KB, PreToolUse hook) arrive
|
||||
* as role:"system" messages in the MIDDLE of the messages array — non-standard
|
||||
* for the Anthropic Messages API, but Claude Code emits them. The claude →
|
||||
* openai translation passed them through at their original positions, and
|
||||
* HCP-Vision-Latest's vLLM (fronted by a LiteLLM gateway) rejects any system
|
||||
* turn not at index 0 with:
|
||||
* 400 litellm.BadRequestError - Hosted_vlllException
|
||||
* "System message must be at the beginning."
|
||||
* Live repro (2026-09): the failing request translated to roles
|
||||
* system,user,system,assistant,tool,user,system
|
||||
* while 135/138 recent mid-system calls to other upstreams (GLM-5.3-Flash,
|
||||
* DeepSeek-V4-Flash) already returned 200 — so demoting mid-conversation
|
||||
* systems to user (content preserved byte-identical) satisfies the strict
|
||||
* vLLM gateway without changing what accepting upstreams already accept.
|
||||
* The index-0 system (translator-made from the top-level `system` field, or
|
||||
* client-supplied at position 0) stays untouched.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { claudeToOpenAIRequest } =
|
||||
await import("../../open-sse/translator/request/claude-to-openai.ts");
|
||||
|
||||
// The bilingual system-append feature rewrites/unshifts the index-0 system
|
||||
// message; blank it so role/content assertions are deterministic regardless
|
||||
// of the operator environment running the suite.
|
||||
delete process.env.OMNIROUTE_SYSTEM_INSTRUCTION_APPEND;
|
||||
|
||||
test("mid-conversation system roles are demoted to user with content preserved", () => {
|
||||
const body = {
|
||||
max_tokens: 64,
|
||||
messages: [
|
||||
{ role: "user", content: [{ type: "text", text: "plan the deploy" }] },
|
||||
{ role: "system", content: "SessionStart hook context" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "tool_use", id: "toolu-1", name: "Read", input: { file_path: "/x" } }],
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "toolu-1", content: "file contents" }],
|
||||
},
|
||||
{ role: "system", content: "PreToolUse hook context" },
|
||||
],
|
||||
};
|
||||
const result = claudeToOpenAIRequest("hcp-vision-latest", body, false, null);
|
||||
const roles = result.messages.map((m) => m.role);
|
||||
assert.deepEqual(roles, ["user", "user", "assistant", "tool", "user"]);
|
||||
assert.ok(!roles.includes("system"), "system may only appear at index 0");
|
||||
assert.equal(result.messages[1].content, "SessionStart hook context");
|
||||
assert.equal(result.messages[4].content, "PreToolUse hook context");
|
||||
});
|
||||
|
||||
test("index-0 system (translator-made from the top-level system field) is preserved", () => {
|
||||
const body = {
|
||||
system: "You are helpful.",
|
||||
max_tokens: 64,
|
||||
messages: [{ role: "user", content: [{ type: "text", text: "hello" }] }],
|
||||
};
|
||||
const result = claudeToOpenAIRequest("hcp-vision-latest", body, false, null);
|
||||
assert.equal(result.messages[0].role, "system");
|
||||
assert.equal(result.messages[0].content, "You are helpful.");
|
||||
});
|
||||
|
||||
test("no-op when there is no mid-conversation system (roles identical to pre-fix)", () => {
|
||||
const body = {
|
||||
system: "You are helpful.",
|
||||
messages: [
|
||||
{ role: "user", content: [{ type: "text", text: "hello" }] },
|
||||
{ role: "assistant", content: [{ type: "text", text: "hi" }] },
|
||||
{ role: "user", content: [{ type: "text", text: "bye" }] },
|
||||
],
|
||||
};
|
||||
const result = claudeToOpenAIRequest("gpt-4o", body, false, null);
|
||||
assert.deepEqual(
|
||||
result.messages.map((m) => m.role),
|
||||
["system", "user", "assistant", "user"]
|
||||
);
|
||||
|
||||
// Edge: empty messages array — nothing to demote, the translator-made
|
||||
// index-0 system is the only message left.
|
||||
const empty = claudeToOpenAIRequest("gpt-4o", { system: "sys", messages: [] }, false, null);
|
||||
assert.deepEqual(
|
||||
empty.messages.map((m) => m.role),
|
||||
["system"]
|
||||
);
|
||||
});
|
||||
|
||||
test("demotion preserves tool_result -> role:tool mapping and assistant content", () => {
|
||||
const body = {
|
||||
system: "You are helpful.",
|
||||
messages: [
|
||||
{ role: "user", content: [{ type: "text", text: "list files" }] },
|
||||
{ role: "system", content: "SessionStart hook context" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "listing now" },
|
||||
{ type: "tool_use", id: "toolu-2", name: "Bash", input: { command: "ls" } },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [
|
||||
{ type: "tool_result", tool_use_id: "toolu-2", content: "a.ts\nb.ts" },
|
||||
{ type: "text", text: "continue" },
|
||||
],
|
||||
},
|
||||
],
|
||||
};
|
||||
const result = claudeToOpenAIRequest("hcp-vision-latest", body, false, null);
|
||||
assert.deepEqual(
|
||||
result.messages.map((m) => m.role),
|
||||
["system", "user", "user", "assistant", "tool", "user"]
|
||||
);
|
||||
const tool = result.messages.find((m) => m.role === "tool");
|
||||
assert.equal(tool.tool_call_id, "toolu-2");
|
||||
assert.equal(tool.content, "a.ts\nb.ts");
|
||||
const assistant = result.messages.find((m) => m.role === "assistant");
|
||||
assert.equal(assistant.content, "listing now");
|
||||
assert.equal(assistant.tool_calls[0].function.name, "Bash");
|
||||
});
|
||||
|
||||
test("full reproduction: mid hook systems become user, index-0 system stays", () => {
|
||||
const body = {
|
||||
system: "You are Claude Code, Anthropic's official CLI for Claude.",
|
||||
max_tokens: 128,
|
||||
messages: [
|
||||
{ role: "user", content: [{ type: "text", text: "plan the deploy" }] },
|
||||
{ role: "system", content: "<session-start-hook>~25KB context</session-start-hook>" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "checking the tree" },
|
||||
{ type: "tool_use", id: "toolu-9", name: "Bash", input: { command: "ls" } },
|
||||
],
|
||||
},
|
||||
{
|
||||
role: "user",
|
||||
content: [{ type: "tool_result", tool_use_id: "toolu-9", content: "src\nopen-sse" }],
|
||||
},
|
||||
{ role: "user", content: [{ type: "text", text: "continue" }] },
|
||||
{ role: "system", content: "<pre-tool-use-hook>context</pre-tool-use-hook>" },
|
||||
],
|
||||
};
|
||||
const result = claudeToOpenAIRequest("hcp-vision-latest", body, false, null);
|
||||
assert.deepEqual(
|
||||
result.messages.map((m) => m.role),
|
||||
["system", "user", "user", "assistant", "tool", "user", "user"]
|
||||
);
|
||||
assert.equal(result.messages[0].role, "system");
|
||||
assert.equal(
|
||||
result.messages[2].content,
|
||||
"<session-start-hook>~25KB context</session-start-hook>"
|
||||
);
|
||||
assert.equal(result.messages[6].content, "<pre-tool-use-hook>context</pre-tool-use-hook>");
|
||||
});
|
||||
@@ -5,17 +5,31 @@
|
||||
* "assistant", so a Claude message with `role: "system"` (e.g. an injected
|
||||
* system reminder mid-conversation) was forwarded to OpenAI-format upstreams
|
||||
* as an assistant turn — polluting the conversation history.
|
||||
*
|
||||
* SUPERSEDED (mid-system demotion): mid-conversation system roles are now
|
||||
* demoted to "user" (content preserved) because OpenAI-compatible upstreams
|
||||
* reject a system turn after index 0 (HCP-Vision-Latest vLLM: 400 "System
|
||||
* message must be at the beginning."). The #6954 intent still holds — the
|
||||
* turns are NOT attributed to the assistant — so these tests assert the
|
||||
* demoted "user" shape. See
|
||||
* tests/unit/claude-to-openai-mid-system-user-normalize.test.ts for the
|
||||
* full demotion contract.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
// The bilingual system-append feature rewrites/unshifts the index-0 system
|
||||
// message; blank it so role/content assertions are deterministic regardless
|
||||
// of the operator environment running the suite.
|
||||
delete process.env.OMNIROUTE_SYSTEM_INSTRUCTION_APPEND;
|
||||
|
||||
const { claudeToOpenAIRequest } =
|
||||
await import("../../open-sse/translator/request/claude-to-openai.ts");
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 1. system message mid-conversation keeps role: "system"
|
||||
// 1. system message mid-conversation is demoted to "user" (never "assistant")
|
||||
// ---------------------------------------------------------------------------
|
||||
test("mid-conversation system message preserves role:system (not assistant)", () => {
|
||||
test("mid-conversation system message is demoted to user (not assistant)", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"gpt-4o",
|
||||
{
|
||||
@@ -30,13 +44,13 @@ test("mid-conversation system message preserves role:system (not assistant)", ()
|
||||
);
|
||||
|
||||
const roles = result.messages.map((m: { role: string }) => m.role);
|
||||
assert.deepEqual(roles, ["user", "assistant", "system", "user"]);
|
||||
assert.deepEqual(roles, ["user", "assistant", "user", "user"]);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. system message with array content keeps role: "system"
|
||||
// 2. system message with array content keeps its content when demoted to "user"
|
||||
// ---------------------------------------------------------------------------
|
||||
test("system message with array content preserves role:system", () => {
|
||||
test("system message with array content is demoted to user, content preserved", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"gpt-4o",
|
||||
{
|
||||
@@ -51,11 +65,12 @@ test("system message with array content preserves role:system", () => {
|
||||
false
|
||||
);
|
||||
|
||||
const sysMsg = result.messages.find((m: { role: string }) => m.role === "system");
|
||||
assert.ok(sysMsg, "expected a system message in output");
|
||||
// Array content with text blocks is flattened to a string for system role
|
||||
const demoted = result.messages[1];
|
||||
assert.ok(demoted, "expected a second message in output");
|
||||
assert.equal(demoted.role, "user");
|
||||
// Array content with text blocks is flattened to a string
|
||||
assert.equal(
|
||||
typeof sysMsg.content === "string" ? sysMsg.content : JSON.stringify(sysMsg.content),
|
||||
typeof demoted.content === "string" ? demoted.content : JSON.stringify(demoted.content),
|
||||
"System reminder text"
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user