From df9808c0e42cde6be2c2bafe9508640c3661d20d Mon Sep 17 00:00:00 2001 From: Rafael Dias Zendron Date: Thu, 16 Jul 2026 14:12:10 -0300 Subject: [PATCH] fix(6954,6953): preserve system role + strip empty-signature thinking blocks (#6982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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> --- .../translator/request/claude-to-openai.ts | 14 +- .../translator/request/openai-to-claude.ts | 19 +- .../claude-to-openai-system-role-6954.test.ts | 103 +++++++++ ...-claude-strip-empty-signature-6953.test.ts | 202 ++++++++++++++++++ 4 files changed, 333 insertions(+), 5 deletions(-) create mode 100644 tests/unit/claude-to-openai-system-role-6954.test.ts create mode 100644 tests/unit/openai-to-claude-strip-empty-signature-6953.test.ts diff --git a/open-sse/translator/request/claude-to-openai.ts b/open-sse/translator/request/claude-to-openai.ts index 9b8b70f3c7..ab50607e75 100644 --- a/open-sse/translator/request/claude-to-openai.ts +++ b/open-sse/translator/request/claude-to-openai.ts @@ -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; diff --git a/open-sse/translator/request/openai-to-claude.ts b/open-sse/translator/request/openai-to-claude.ts index ff5dc4b6d5..1cfa99ddff 100644 --- a/open-sse/translator/request/openai-to-claude.ts +++ b/open-sse/translator/request/openai-to-claude.ts @@ -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, diff --git a/tests/unit/claude-to-openai-system-role-6954.test.ts b/tests/unit/claude-to-openai-system-role-6954.test.ts new file mode 100644 index 0000000000..d9c48ac085 --- /dev/null +++ b/tests/unit/claude-to-openai-system-role-6954.test.ts @@ -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"); +}); diff --git a/tests/unit/openai-to-claude-strip-empty-signature-6953.test.ts b/tests/unit/openai-to-claude-strip-empty-signature-6953.test.ts new file mode 100644 index 0000000000..f129a64f70 --- /dev/null +++ b/tests/unit/openai-to-claude-strip-empty-signature-6953.test.ts @@ -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"); +});