fix(sse): suppress </think> by default on Chat Completions (#8245) (#8309)

Claude→OpenAI translation was emitting a literal </think> into
delta.content for ordinary Chat Completions clients. Reasoning already
ships as reasoning_content, so default to suppress and keep
x-omniroute-thinking-marker: on as the #4633 opt-in.
This commit is contained in:
Prudhvi Vuda
2026-07-24 08:35:51 -04:00
committed by GitHub
parent 1f7ec2c321
commit 14f4c67598
6 changed files with 173 additions and 96 deletions

View File

@@ -2,31 +2,28 @@
* `</think>` close-marker client policy.
*
* When OmniRoute translates a Claude-native streamed response to OpenAI Chat
* Completions shape (`claude-to-openai.ts`), it emits a single `</think>`
* close marker as `delta.content` so clients that scan content for the marker
* (Claude Code, Cursor) can split reasoning from the final answer — see #4633.
* Completions shape (`claude-to-openai.ts`), it historically emitted a single
* `</think>` close marker as `delta.content` so clients that scan content for
* the marker (Claude Code, Cursor) could split reasoning from the final answer
* — see #4633.
*
* Some OpenAI-compatible consumers do NOT parse that marker and render it
* verbatim, so a bare `</think>` leaks into the visible reply (#5245). OpenCode
* is one such client.
* Modern Chat Completions clients already receive reasoning on the separate
* `reasoning_content` field. Emitting the in-band marker by default now
* corrupts that split (literal `</think>` in visible content) for ordinary
* OpenAI clients — see #8245. OpenCode / Antigravity had the same leak earlier
* (#5245 / #1061).
*
* Policy is conservative and opt-OUT by allowlist: the marker stays ON by
* default (preserving #4633 for Claude Code / Cursor and any unrecognized
* client), and is suppressed ONLY for known clients that render it literally.
* Detection is by inbound `User-Agent`.
* Policy (#8245):
* - Default: SUPPRESS the marker on OpenAI Chat Completions.
* - Explicit opt-in: `x-omniroute-thinking-marker: on` force-keeps it for the
* shrinking set of content-scanning clients (#4633).
* - Explicit opt-out: `x-omniroute-thinking-marker: off` (same as default).
* - Responses API (`openai-responses`): always suppress (#7747) — wins over
* the header; there is no legitimate marker consumer on that path.
*
* Clients that DO render the marker verbatim but are not in the UA allowlist
* (e.g. Cursor's reasoning_content-native OpenAI path — #5312 / #5245) can opt
* in explicitly with the request header `x-omniroute-thinking-marker: off`,
* which suppresses the marker regardless of User-Agent. `on` forces it kept
* (overriding the UA allowlist). The default (header absent) is byte-identical
* to the UA-only policy, so #4633 / #5123 are never regressed.
*
* Responses API clients (`openai-responses`) are always suppressed: the
* Responses transformer maps `reasoning_content` to structured reasoning items
* natively, so no consumer on that path scans content for the marker — it can
* only leak verbatim into `response.output_text.delta` (observed with
* kimi-coding: a stray `</think>` at the start of the assistant text).
* The User-Agent allowlist below is retained for diagnostics / callers that
* still query `shouldSuppressThinkCloseMarker` directly; the resolved default
* no longer depends on UA.
*/
import { FORMATS } from "../translator/formats.ts";
@@ -34,18 +31,15 @@ import { FORMATS } from "../translator/formats.ts";
/** Header clients send to explicitly opt in/out of the `</think>` close marker. */
export const THINKING_MARKER_HEADER = "x-omniroute-thinking-marker";
// Lowercased User-Agent substrings of clients that render the textual
// `</think>` marker verbatim and therefore want it suppressed.
// - `opencode` (#5245): renders the marker as literal text.
// - `antigravity` (#1061): the Antigravity IDE client (UA
// `vscode/<v> (Antigravity/<v>)`) renders a bare `</think>` as the sole
// visible content on thinking-only turns, which trips its loop-detection.
// Lowercased User-Agent substrings of clients that historically rendered the
// textual `</think>` marker verbatim (#5245 / #1061). Kept for direct callers;
// resolveSuppressThinkClose no longer needs UA to decide the default (#8245).
const SUPPRESS_THINK_CLOSE_UA_MARKERS = ["opencode", "antigravity"];
/**
* Whether the streamed `</think>` close marker should be suppressed for the
* given inbound client. Returns false (emit the marker) for unknown clients and
* for Claude Code / Cursor, so #4633 is never regressed.
* given inbound client User-Agent. Prefer `resolveSuppressThinkClose` for
* request policy — UA alone is no longer the Chat Completions default (#8245).
*/
export function shouldSuppressThinkCloseMarker(userAgent: string | null | undefined): boolean {
if (!userAgent || typeof userAgent !== "string") return false;
@@ -56,7 +50,7 @@ export function shouldSuppressThinkCloseMarker(userAgent: string | null | undefi
/**
* Interpret the explicit `x-omniroute-thinking-marker` request header.
* Returns `true` (suppress the marker), `false` (force-keep the marker), or
* `null` when the header is absent/unrecognized (defer to the UA policy).
* `null` when the header is absent/unrecognized (defer to the default policy).
*/
export function thinkingMarkerHeaderSignal(
headerValue: string | null | undefined
@@ -70,10 +64,12 @@ export function thinkingMarkerHeaderSignal(
/**
* Resolve whether the streamed `</think>` close marker should be suppressed for
* this request. An explicit `x-omniroute-thinking-marker` header wins; absent
* that, the conservative User-Agent allowlist policy applies. With no header and
* an unrecognized UA the result is `false` (marker kept), so #4633 / #5123 stay
* byte-identical by default.
* this request.
*
* Precedence:
* 1. Responses API format → always suppress (#7747)
* 2. Explicit `x-omniroute-thinking-marker` header → honor on/off (#5312)
* 3. Default → suppress on Chat Completions (#8245)
*/
export function resolveSuppressThinkClose(opts: {
userAgent?: string | null;
@@ -82,10 +78,12 @@ export function resolveSuppressThinkClose(opts: {
}): boolean {
// The marker only exists for Chat Completions clients that scan content for
// it; Responses API clients receive reasoning as structured items instead.
// This wins over the UA allowlist AND the explicit header: there is no
// legitimate marker consumer in the Responses format.
// This wins over the explicit header: there is no legitimate marker consumer
// in the Responses format.
if (opts.clientResponseFormat === FORMATS.OPENAI_RESPONSES) return true;
const headerSignal = thinkingMarkerHeaderSignal(opts.thinkingMarkerHeader);
if (headerSignal !== null) return headerSignal;
return shouldSuppressThinkCloseMarker(opts.userAgent);
// #8245: suppress by default. Reasoning already ships as reasoning_content.
// Legacy content-scanning clients opt in with x-omniroute-thinking-marker: on.
return true;
}

View File

@@ -38,8 +38,8 @@ function newState() {
};
}
test("claudeToOpenAIResponse emits </think> close marker on message finish (deferred from content_block_stop)", () => {
const state = newState();
test("claudeToOpenAIResponse emits </think> on finish when suppressThinkClose=false (#4633 opt-in)", () => {
const state = { ...newState(), suppressThinkClose: false };
// Open thinking block.
claudeToOpenAIResponse(
@@ -101,6 +101,36 @@ test("claudeToOpenAIResponse emits </think> close marker on message finish (defe
assert.equal(state.pendingThinkClose, false, "pendingThinkClose must be cleared after flush");
});
test("claudeToOpenAIResponse suppresses </think> on finish when suppressThinkClose=true (#8245)", () => {
const state = { ...newState(), suppressThinkClose: true };
claudeToOpenAIResponse(
{ type: "content_block_start", index: 0, content_block: { type: "thinking", thinking: "" } },
state
);
claudeToOpenAIResponse(
{
type: "content_block_delta",
index: 0,
delta: { type: "thinking_delta", thinking: "Plan first." },
},
state
);
claudeToOpenAIResponse({ type: "content_block_stop", index: 0 }, state);
assert.equal(state.pendingThinkClose, true);
const finishChunks = claudeToOpenAIResponse(
{ type: "message_delta", delta: { stop_reason: "end_turn" }, usage: { output_tokens: 5 } },
state
);
const arr = Array.isArray(finishChunks) ? finishChunks : [];
const hasCloseMarker = arr.some(
(chunk) => chunk?.choices?.[0]?.delta?.content === "</think>"
);
assert.equal(hasCloseMarker, false, "marker must not leak under #8245 default policy");
assert.equal(state.pendingThinkClose, false, "pendingThinkClose must still be cleared");
});
test("claudeToOpenAIResponse does not emit </think> on stop of non-thinking blocks", () => {
const state = newState();

View File

@@ -106,10 +106,11 @@ test("thinking block followed by tool_use: </think> must NOT appear in any conte
});
// ─── Case (b): thinking block followed by text (pure-text response) ──────────
// This is the #4633 happy path. </think> MUST still be emitted so Claude Code /
// Cursor know when the thinking section ends.
test("thinking block followed by text: </think> IS still emitted (preserves #4633)", () => {
const state = newState();
// #4633 opt-in path: when suppressThinkClose is false (header `on`), </think>
// is still emitted for legacy content-scanning clients. Production default
// (#8245) sets suppressThinkClose=true via resolveSuppressThinkClose.
test("thinking block followed by text: </think> emitted when suppressThinkClose=false (#4633 opt-in)", () => {
const state = { ...newState(), suppressThinkClose: false };
const allResults: ReturnType<typeof claudeToOpenAIResponse>[] = [];
@@ -175,3 +176,50 @@ test("thinking block followed by text: </think> IS still emitted (preserves #463
`Expected a chunk with delta.content === "</think>" in a pure-text thinking response, but none found.\nAll chunks: ${JSON.stringify(chunks, null, 2)}`
);
});
// #8245 production default: suppressThinkClose=true → no literal marker in content.
test("thinking block followed by text: </think> suppressed when suppressThinkClose=true (#8245)", () => {
const state = { ...newState(), suppressThinkClose: true };
const allResults: ReturnType<typeof claudeToOpenAIResponse>[] = [];
allResults.push(
claudeToOpenAIResponse({ type: "message_start", message: { id: "msg_3", model: "claude-3-7-sonnet" } }, state)
);
allResults.push(
claudeToOpenAIResponse(
{ type: "content_block_start", index: 0, content_block: { type: "thinking", thinking: "" } },
state
)
);
allResults.push(
claudeToOpenAIResponse(
{
type: "content_block_delta",
index: 0,
delta: { type: "thinking_delta", thinking: "Plan..." },
},
state
)
);
allResults.push(claudeToOpenAIResponse({ type: "content_block_stop", index: 0 }, state));
allResults.push(
claudeToOpenAIResponse(
{ type: "content_block_start", index: 1, content_block: { type: "text", text: "" } },
state
)
);
allResults.push(
claudeToOpenAIResponse(
{ type: "content_block_delta", index: 1, delta: { type: "text_delta", text: "Hello!" } },
state
)
);
const chunks = collectChunks(allResults);
const hasThinkClose = chunks.some(
(chunk: any) => chunk?.choices?.[0]?.delta?.content === "</think>"
);
assert.equal(hasThinkClose, false, "marker must not leak into content under #8245 default");
const hasText = chunks.some((chunk: any) => chunk?.choices?.[0]?.delta?.content === "Hello!");
assert.ok(hasText, "assistant text must still be emitted");
});

View File

@@ -144,25 +144,7 @@ test("GLM translateSseResponse: suppressThinkClose=true suppresses </think> mark
);
});
test("GLM translateSseResponse: default (no flag) emits </think> marker for backward compat", async () => {
const { translateSseResponse } = await import("../../open-sse/executors/glm.ts");
const sseBody = buildClaudeSseStream();
const upstream = new Response(sseBody, {
headers: { "content-type": "text/event-stream" },
});
// Call without the 4th arg — default must be false (preserve #4633)
const translated = translateSseResponse(upstream, "glm", "glm-5.2");
const output = await collectStreamOutput(translated);
assert.ok(
output.includes("</think>"),
"marker must be emitted by default (backward compat with #4633)"
);
});
test("resolveSuppressThinkClose: OpenCode UA triggers suppression for GLM path", () => {
test("resolveSuppressThinkClose: Chat Completions default suppresses for GLM path (#8245)", () => {
// This is the resolution that executeTransport does before calling
// translateSseResponse. Verify the integration point.
assert.equal(
@@ -172,8 +154,16 @@ test("resolveSuppressThinkClose: OpenCode UA triggers suppression for GLM path",
);
assert.equal(
resolveSuppressThinkClose({ userAgent: "claude-code/1.0" }),
true,
"Claude Code UA suppresses by default (#8245); opt in with header on"
);
assert.equal(
resolveSuppressThinkClose({
userAgent: "claude-code/1.0",
thinkingMarkerHeader: "on",
}),
false,
"Claude Code UA must resolve to keep"
"Header on force-keeps the marker for legacy clients"
);
assert.equal(
resolveSuppressThinkClose({
@@ -181,7 +171,7 @@ test("resolveSuppressThinkClose: OpenCode UA triggers suppression for GLM path",
thinkingMarkerHeader: "off",
}),
true,
"Header off must override UA"
"Header off suppresses"
);
assert.equal(
THINKING_MARKER_HEADER,

View File

@@ -35,24 +35,32 @@ test("openai-responses suppression wins over an explicit keep header", () => {
);
});
test("openai chat format keeps the conservative default (marker on)", () => {
test("openai chat format suppresses the marker by default (#8245)", () => {
assert.equal(
resolveSuppressThinkClose({
userAgent: "OpenAI/JS 6.26.0",
thinkingMarkerHeader: null,
clientResponseFormat: FORMATS.OPENAI,
}),
false
true
);
});
test("absent client format preserves the UA/header policy", () => {
test("absent client format suppresses by default; header on keeps the marker (#8245)", () => {
assert.equal(
resolveSuppressThinkClose({ userAgent: "opencode/1.0", thinkingMarkerHeader: null }),
true
);
assert.equal(
resolveSuppressThinkClose({ userAgent: "unknown-client", thinkingMarkerHeader: null }),
true
);
assert.equal(
resolveSuppressThinkClose({
userAgent: "unknown-client",
thinkingMarkerHeader: "on",
clientResponseFormat: FORMATS.OPENAI,
}),
false
);
});

View File

@@ -10,11 +10,11 @@ const {
THINKING_MARKER_HEADER,
} = await import("../../open-sse/utils/thinkCloseMarker.ts");
// #5245: when translating a Claude-native stream to OpenAI shape,
// claude-to-openai.ts emits a textual `</think>` close marker (by design, for
// Claude Code / Cursor — #4633). Clients that render it verbatim (OpenCode)
// want it suppressed. `state.suppressThinkClose` gates the emission; default
// (unset/false) preserves the #4633 behaviour.
// #5245 / #8245: when translating a Claude-native stream to OpenAI shape,
// claude-to-openai.ts can emit a textual `</think>` close marker. Production
// policy (#8245) suppresses it by default via resolveSuppressThinkClose;
// `state.suppressThinkClose` gates emission in the translator. unset/false
// still emits at the translator layer (opt-in path for header `on`).
function newState(extra: Record<string, unknown> = {}) {
return { toolCalls: new Map(), toolNameMap: new Map(), ...extra } as Record<string, unknown>;
@@ -123,32 +123,31 @@ test("shouldSuppressThinkCloseMarker: suppresses for OpenCode, preserves CC/Curs
// ── translator gating ────────────────────────────────────────────────────────
test("claude-to-openai: default emits the </think> marker before the first text (#4633 preserved)", () => {
const contents = contentChunks(runThinkThenText(newState()));
assert.ok(contents.includes("</think>"), "marker must be emitted by default");
assert.ok(contents.includes("169"), "real text still emitted");
// marker comes before the real text
assert.ok(contents.indexOf("</think>") < contents.indexOf("169"));
});
test("claude-to-openai: suppressThinkClose drops the </think> marker but keeps the text (#5245)", () => {
test("claude-to-openai: suppressThinkClose (production default #8245) drops marker before text", () => {
const contents = contentChunks(runThinkThenText(newState({ suppressThinkClose: true })));
assert.ok(!contents.includes("</think>"), "marker must be suppressed");
assert.ok(contents.includes("169"), "real text still emitted");
});
// ── finish-flush path (L197-207, toolCalls.size === 0) ───────────────────────
test("claude-to-openai: finish-flush emits </think> by default for thinking-only response (#4633)", () => {
const contents = contentChunks(runThinkOnlyThenFinish(newState()));
assert.ok(contents.includes("</think>"), "marker must be emitted at finish by default");
test("claude-to-openai: suppressThinkClose=false (header on) emits marker before text (#4633 opt-in)", () => {
const contents = contentChunks(runThinkThenText(newState({ suppressThinkClose: false })));
assert.ok(contents.includes("</think>"), "marker must be emitted when opted in");
assert.ok(contents.includes("169"), "real text still emitted");
assert.ok(contents.indexOf("</think>") < contents.indexOf("169"));
});
test("claude-to-openai: finish-flush suppressed under suppressThinkClose (#5312)", () => {
// ── finish-flush path (L197-207, toolCalls.size === 0) ───────────────────────
test("claude-to-openai: finish-flush suppressed under suppressThinkClose (#8245 default)", () => {
const contents = contentChunks(runThinkOnlyThenFinish(newState({ suppressThinkClose: true })));
assert.ok(!contents.includes("</think>"), "marker must be suppressed at finish");
});
test("claude-to-openai: finish-flush emits </think> when suppressThinkClose=false (#4633 opt-in)", () => {
const contents = contentChunks(runThinkOnlyThenFinish(newState({ suppressThinkClose: false })));
assert.ok(contents.includes("</think>"), "marker must be emitted at finish when opted in");
});
// ── header signal resolution (x-omniroute-thinking-marker — #5312) ───────────
test("thinkingMarkerHeaderSignal: off → suppress, on → keep, absent/unknown → null", () => {
@@ -163,22 +162,26 @@ test("thinkingMarkerHeaderSignal: off → suppress, on → keep, absent/unknown
assert.equal(thinkingMarkerHeaderSignal(undefined), null);
});
test("resolveSuppressThinkClose: header opts in (Cursor) and overrides the UA allowlist", () => {
test("resolveSuppressThinkClose: default suppresses; header on opts into marker (#8245)", () => {
// header constant is the documented wire name
assert.equal(THINKING_MARKER_HEADER, "x-omniroute-thinking-marker");
// Cursor's UA is NOT in the allowlist → marker kept by default (orphan </think>, #5312)…
assert.equal(resolveSuppressThinkClose({ userAgent: "cursor-agent/0.5" }), false);
// …but `off` opts in to suppression regardless of UA.
// #8245: Chat Completions default suppresses for every UA (incl. Cursor / Claude Code).
assert.equal(resolveSuppressThinkClose({ userAgent: "cursor-agent/0.5" }), true);
assert.equal(resolveSuppressThinkClose({ userAgent: "claude-code/1.0" }), true);
assert.equal(resolveSuppressThinkClose({ userAgent: "opencode/1.0" }), true);
assert.equal(resolveSuppressThinkClose({ userAgent: "OpenAI/JS 6.26.0" }), true);
// Explicit `off` is also suppress.
assert.equal(
resolveSuppressThinkClose({ userAgent: "cursor-agent/0.5", thinkingMarkerHeader: "off" }),
true
);
// `on` force-keeps even for an allowlisted (OpenCode) UA.
// `on` force-keeps for legacy content-scanning clients (#4633 opt-in).
assert.equal(
resolveSuppressThinkClose({ userAgent: "opencode/1.0", thinkingMarkerHeader: "on" }),
false
);
// No header → defers to UA policy (OpenCode suppressed, unknown kept).
assert.equal(resolveSuppressThinkClose({ userAgent: "opencode/1.0" }), true);
assert.equal(resolveSuppressThinkClose({ userAgent: "claude-code/1.0" }), false);
assert.equal(
resolveSuppressThinkClose({ userAgent: "claude-code/1.0", thinkingMarkerHeader: "on" }),
false
);
});