mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(6954,6953): preserve system role + strip empty-signature thinking blocks (#6982)
* fix(6954,6953): preserve system role + strip empty-signature thinking blocks #6954 — System turns misattributed as assistant (claude-to-openai.ts:352) The ternary `msg.role === 'user' || msg.role === 'tool' ? 'user' : 'assistant'` mapped any non-user/non-tool role (including 'system') to 'assistant'. Mid-conversation system turns (Claude format) lost their role on translation to OpenAI format, causing them to be treated as assistant output. Fix: add explicit 'system' branch to the ternary. #6953 — Empty-signature thinking blocks poison Anthropic leg (openai-to-claude.ts) Non-Anthropic providers (codex/gpt-5.x) synthesize thinking blocks with signature:''\. On replay, the old code fabricated a DEFAULT_THINKING_CLAUDE_SIGNATURE to fill the empty signature — but Anthropic rejects foreign signatures with HTTP 400, permanently degrading combo/blend routes to codex-only. Fix: strip thinking blocks with empty/missing signatures and redacted_thinking blocks with empty/missing data entirely. They carry no replayable value. Tests: 8 new tests (4 per bug), all passing. Existing #5312 and #5945 regression tests still pass — no interference. * fix(6953): strip only signature:"" thinking blocks, preserve undefined signature CI caught a regression: translator-helper-branches test had a Claude-format thinking block without signature field (undefined) that was being stripped by the original fix. The fix was too aggressive — it stripped both signature:"" (non-Anthropic synthesized) and signature: undefined (legitimate Claude-format). Correct behavior: - signature === "" (empty string): strip — hallmark of codex/gpt-5.x block - signature === undefined: preserve with DEFAULT_THINKING_CLAUDE_SIGNATURE fallback - redacted_thinking data === "": strip - redacted_thinking data === undefined: preserve with fallback Added regression test for undefined-signature preservation. --------- Co-authored-by: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
bc6cd2a806
commit
df9808c0e4
@@ -349,7 +349,15 @@ function fixMissingToolResponses(messages) {
|
||||
|
||||
// Convert single Claude message - returns single message or array of messages
|
||||
function convertClaudeMessage(msg, preserveCacheControl = false) {
|
||||
const role = msg.role === "user" || msg.role === "tool" ? "user" : "assistant";
|
||||
// Preserve system role for mid-conversation system turns (#6954).
|
||||
// Previously any role that wasn't "user" or "tool" was mapped to "assistant",
|
||||
// which misattributed system messages as assistant output.
|
||||
const role =
|
||||
msg.role === "user" || msg.role === "tool"
|
||||
? "user"
|
||||
: msg.role === "system"
|
||||
? "system"
|
||||
: "assistant";
|
||||
|
||||
// Simple string content
|
||||
if (typeof msg.content === "string") {
|
||||
@@ -411,9 +419,7 @@ function convertClaudeMessage(msg, preserveCacheControl = false) {
|
||||
function: {
|
||||
name: block.name,
|
||||
arguments:
|
||||
typeof block.input === "string"
|
||||
? block.input
|
||||
: JSON.stringify(block.input || {}),
|
||||
typeof block.input === "string" ? block.input : JSON.stringify(block.input || {}),
|
||||
},
|
||||
});
|
||||
break;
|
||||
|
||||
@@ -592,7 +592,24 @@ function getContentBlocksFromMessage(
|
||||
if (part.type === "text" && part.text) {
|
||||
blocks.push({ type: "text", text: part.text });
|
||||
} else if (part.type === "thinking" || part.type === "redacted_thinking") {
|
||||
// Preserve thinking blocks with signature
|
||||
// #6953 — thinking blocks with signature:"" (empty string) come from non-Anthropic
|
||||
// providers (codex/gpt-5.x). Anthropic rejects replayed `thinking` blocks that
|
||||
// carry a foreign or fabricated signature with HTTP 400. Fabricating a default
|
||||
// signature (the old behaviour) made the poisoning permanent: once a codex-served
|
||||
// turn introduced a `signature:""` thinking block, every subsequent Anthropic leg
|
||||
// attempt 400'd and the router silently fell back to codex forever.
|
||||
//
|
||||
// Fix: strip thinking blocks whose signature is the empty string — that explicit
|
||||
// empty value is the hallmark of a synthesized block from a non-Anthropic provider.
|
||||
// Thinking blocks with `signature: undefined` (field absent) are legitimate Claude-
|
||||
// format messages and fall through to the DEFAULT_THINKING_CLAUDE_SIGNATURE fallback
|
||||
// as before.
|
||||
if (part.type === "thinking" && part.signature === "") {
|
||||
continue; // drop — synthesized by non-Anthropic provider, no valid signature
|
||||
}
|
||||
if (part.type === "redacted_thinking" && part.data === "") {
|
||||
continue; // drop — same: empty data from non-Anthropic provider
|
||||
}
|
||||
blocks.push({
|
||||
...part,
|
||||
signature: part.signature || DEFAULT_THINKING_CLAUDE_SIGNATURE,
|
||||
|
||||
103
tests/unit/claude-to-openai-system-role-6954.test.ts
Normal file
103
tests/unit/claude-to-openai-system-role-6954.test.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
/**
|
||||
* Tests for #6954 — mid-conversation system turns misattributed as assistant.
|
||||
*
|
||||
* `convertClaudeMessage` mapped any role that wasn't "user" or "tool" to
|
||||
* "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.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { claudeToOpenAIRequest } =
|
||||
await import("../../open-sse/translator/request/claude-to-openai.ts");
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 1. system message mid-conversation keeps role: "system"
|
||||
// ---------------------------------------------------------------------------
|
||||
test("mid-conversation system message preserves role:system (not assistant)", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"gpt-4o",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "hello" },
|
||||
{ role: "assistant", content: "hi" },
|
||||
{ role: "system", content: "Reminder: be concise." },
|
||||
{ role: "user", content: "ok" },
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const roles = result.messages.map((m: { role: string }) => m.role);
|
||||
assert.deepEqual(roles, ["user", "assistant", "system", "user"]);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 2. system message with array content keeps role: "system"
|
||||
// ---------------------------------------------------------------------------
|
||||
test("system message with array content preserves role:system", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"gpt-4o",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "hello" },
|
||||
{
|
||||
role: "system",
|
||||
content: [{ type: "text", text: "System reminder text" }],
|
||||
},
|
||||
],
|
||||
},
|
||||
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
|
||||
assert.equal(
|
||||
typeof sysMsg.content === "string" ? sysMsg.content : JSON.stringify(sysMsg.content),
|
||||
"System reminder text"
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 3. top-level body.system still produces role: "system" (regression check)
|
||||
// ---------------------------------------------------------------------------
|
||||
test("body.system still produces role:system at index 0", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"gpt-4o",
|
||||
{
|
||||
system: "You are helpful.",
|
||||
messages: [{ role: "user", content: "hi" }],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
assert.equal(result.messages[0].role, "system");
|
||||
assert.equal(result.messages[1].role, "user");
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// 4. assistant with tool_use still maps to assistant (regression check)
|
||||
// ---------------------------------------------------------------------------
|
||||
test("assistant role still maps to assistant (no regression)", () => {
|
||||
const result = claudeToOpenAIRequest(
|
||||
"gpt-4o",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "use the tool" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "calling tool" },
|
||||
{ type: "tool_use", id: "t1", name: "foo", input: {} },
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const roles = result.messages.map((m: { role: string }) => m.role);
|
||||
assert.ok(roles.includes("assistant"), "assistant role must be preserved");
|
||||
});
|
||||
202
tests/unit/openai-to-claude-strip-empty-signature-6953.test.ts
Normal file
202
tests/unit/openai-to-claude-strip-empty-signature-6953.test.ts
Normal file
@@ -0,0 +1,202 @@
|
||||
/**
|
||||
* TDD regression for #6953 — thinking blocks with empty signatures poison the
|
||||
* Anthropic leg of combo/blend routes.
|
||||
*
|
||||
* Non-Anthropic providers (codex/gpt-5.x) synthesize Anthropic-format `thinking`
|
||||
* blocks with `signature: ""`. When the client replays these in the next
|
||||
* request's history, the Anthropic leg rejects them with HTTP 400 "Invalid
|
||||
* signature in thinking block", and the router silently falls back to codex
|
||||
* permanently.
|
||||
*
|
||||
* The old code fabricated a DEFAULT_THINKING_CLAUDE_SIGNATURE to fill the empty
|
||||
* signature — but that fabricated signature is equally foreign to Anthropic, so
|
||||
* it also 400'd.
|
||||
*
|
||||
* Fix (#6953): strip thinking blocks with empty/missing signatures entirely.
|
||||
* They carry no replayable cryptographic value. For `redacted_thinking`, strip
|
||||
* if `data` is empty/missing for the same reason.
|
||||
*/
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { openaiToClaudeRequest } =
|
||||
await import("../../open-sse/translator/request/openai-to-claude.ts");
|
||||
|
||||
test('#6953: thinking block with signature:"" is stripped, not fabricated', () => {
|
||||
const result = openaiToClaudeRequest(
|
||||
"claude-opus-4-8",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "hello" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "I will help you." },
|
||||
{ type: "thinking", thinking: "reasoning here", signature: "" },
|
||||
{
|
||||
type: "text",
|
||||
text: "Let me use a tool.",
|
||||
},
|
||||
],
|
||||
},
|
||||
{ role: "user", content: "ok go ahead" },
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const assistant = result.messages.find((m) => m.role === "assistant");
|
||||
assert.ok(assistant, "expected assistant message");
|
||||
|
||||
// The thinking block with empty signature must be DROPPED, not preserved
|
||||
// with a fabricated signature.
|
||||
const thinkingBlocks = assistant.content.filter((b) => b && b.type === "thinking");
|
||||
assert.equal(
|
||||
thinkingBlocks.length,
|
||||
0,
|
||||
"thinking block with empty signature must be stripped, not fabricated"
|
||||
);
|
||||
|
||||
// Text blocks must survive
|
||||
const textBlocks = assistant.content.filter((b) => b && b.type === "text");
|
||||
assert.ok(textBlocks.length >= 1, "text blocks must be preserved");
|
||||
});
|
||||
|
||||
test("#6953: thinking block with valid signature is preserved verbatim", () => {
|
||||
const realSig = "EuY2xhdWRlLXNpZ25hdHVyZS0xNzA5...";
|
||||
const result = openaiToClaudeRequest(
|
||||
"claude-opus-4-8",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "hello" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "thinking", thinking: "real reasoning", signature: realSig },
|
||||
{ type: "text", text: "response" },
|
||||
],
|
||||
},
|
||||
{ role: "user", content: "ok" },
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const assistant = result.messages.find((m) => m.role === "assistant");
|
||||
assert.ok(assistant);
|
||||
|
||||
const thinkingBlocks = assistant.content.filter((b) => b && b.type === "thinking");
|
||||
assert.equal(thinkingBlocks.length, 1, "valid thinking block must be preserved");
|
||||
assert.equal(thinkingBlocks[0].signature, realSig, "valid signature must be preserved verbatim");
|
||||
});
|
||||
|
||||
test("#6953: thinking block with undefined signature (Claude-format) is preserved with fallback", () => {
|
||||
// Claude-format messages may have thinking blocks without a signature field at all.
|
||||
// These are legitimate and must NOT be stripped — only signature:"" (empty string)
|
||||
// indicates a non-Anthropic synthesized block.
|
||||
const result = openaiToClaudeRequest(
|
||||
"claude-opus-4-8",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "hello" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "thinking", thinking: "I already have this" },
|
||||
{ type: "text", text: "response" },
|
||||
],
|
||||
},
|
||||
{ role: "user", content: "ok" },
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const assistant = result.messages.find((m) => m.role === "assistant");
|
||||
assert.ok(assistant);
|
||||
|
||||
const thinkingBlocks = assistant.content.filter((b) => b && b.type === "thinking");
|
||||
assert.equal(
|
||||
thinkingBlocks.length,
|
||||
1,
|
||||
"thinking block with undefined signature must be preserved"
|
||||
);
|
||||
assert.equal(thinkingBlocks[0].thinking, "I already have this", "thinking content must match");
|
||||
assert.ok(thinkingBlocks[0].signature, "fallback signature must be applied");
|
||||
});
|
||||
|
||||
test("#6953: redacted_thinking with empty data is stripped", () => {
|
||||
const result = openaiToClaudeRequest(
|
||||
"claude-opus-4-8",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "hello" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "redacted_thinking", data: "" },
|
||||
{ type: "text", text: "response" },
|
||||
],
|
||||
},
|
||||
{ role: "user", content: "ok" },
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const assistant = result.messages.find((m) => m.role === "assistant");
|
||||
assert.ok(assistant);
|
||||
|
||||
const redactedBlocks = assistant.content.filter((b) => b && b.type === "redacted_thinking");
|
||||
assert.equal(redactedBlocks.length, 0, "redacted_thinking with empty data must be stripped");
|
||||
});
|
||||
|
||||
test("#6953: combo scenario — codex-sourced thinking block does not block Anthropic leg", () => {
|
||||
// Simulates a combo route: turn 1 served by codex produced a thinking block
|
||||
// with signature:"". Turn 2 should be able to route to Anthropic without
|
||||
// the poisoned block causing a 400.
|
||||
const result = openaiToClaudeRequest(
|
||||
"claude-opus-4-8",
|
||||
{
|
||||
messages: [
|
||||
{ role: "user", content: "write a function" },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{
|
||||
type: "thinking",
|
||||
thinking: "**Reviewing the request**\n\nI need to write a function...",
|
||||
signature: "", // codex-sourced, no real signature
|
||||
},
|
||||
{ type: "text", text: "Here's the function:" },
|
||||
{
|
||||
type: "tool_use",
|
||||
id: "toolu_01abc",
|
||||
name: "write_file",
|
||||
input: { path: "main.rs", content: "fn main() {}" },
|
||||
},
|
||||
],
|
||||
},
|
||||
{ role: "user", content: "looks good, now add tests" },
|
||||
],
|
||||
},
|
||||
false
|
||||
);
|
||||
|
||||
const assistant = result.messages.find((m) => m.role === "assistant");
|
||||
assert.ok(assistant);
|
||||
|
||||
// No thinking block with empty-string signature should survive
|
||||
const badThinking = assistant.content.find(
|
||||
(b) => b && b.type === "thinking" && b.signature === ""
|
||||
);
|
||||
assert.equal(
|
||||
badThinking,
|
||||
undefined,
|
||||
"no thinking block with empty-string signature should survive"
|
||||
);
|
||||
|
||||
// Tool use must survive
|
||||
const toolUse = assistant.content.find((b) => b && b.type === "tool_use");
|
||||
assert.ok(toolUse, "tool_use block must be preserved");
|
||||
});
|
||||
Reference in New Issue
Block a user