diff --git a/open-sse/utils/thinkCloseMarker.ts b/open-sse/utils/thinkCloseMarker.ts index 06a6a46774..db609fe8df 100644 --- a/open-sse/utils/thinkCloseMarker.ts +++ b/open-sse/utils/thinkCloseMarker.ts @@ -2,31 +2,28 @@ * `` 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 `` - * 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 + * `` 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 `` 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 `` 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 `` 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 `` close marker. */ export const THINKING_MARKER_HEADER = "x-omniroute-thinking-marker"; -// Lowercased User-Agent substrings of clients that render the textual -// `` marker verbatim and therefore want it suppressed. -// - `opencode` (#5245): renders the marker as literal text. -// - `antigravity` (#1061): the Antigravity IDE client (UA -// `vscode/ (Antigravity/)`) renders a bare `` 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 `` 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 `` 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 `` 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; } diff --git a/tests/unit/anthropic-stream-thinking-close-marker.test.ts b/tests/unit/anthropic-stream-thinking-close-marker.test.ts index 028b2c2a78..62ce2cbd2b 100644 --- a/tests/unit/anthropic-stream-thinking-close-marker.test.ts +++ b/tests/unit/anthropic-stream-thinking-close-marker.test.ts @@ -38,8 +38,8 @@ function newState() { }; } -test("claudeToOpenAIResponse emits close marker on message finish (deferred from content_block_stop)", () => { - const state = newState(); +test("claudeToOpenAIResponse emits on finish when suppressThinkClose=false (#4633 opt-in)", () => { + const state = { ...newState(), suppressThinkClose: false }; // Open thinking block. claudeToOpenAIResponse( @@ -101,6 +101,36 @@ test("claudeToOpenAIResponse emits close marker on message finish (defe assert.equal(state.pendingThinkClose, false, "pendingThinkClose must be cleared after flush"); }); +test("claudeToOpenAIResponse suppresses 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 === "" + ); + 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 on stop of non-thinking blocks", () => { const state = newState(); diff --git a/tests/unit/claude-to-openai-think-close-5123.test.ts b/tests/unit/claude-to-openai-think-close-5123.test.ts index 50d5a4990d..545bb5a70b 100644 --- a/tests/unit/claude-to-openai-think-close-5123.test.ts +++ b/tests/unit/claude-to-openai-think-close-5123.test.ts @@ -106,10 +106,11 @@ test("thinking block followed by tool_use: must NOT appear in any conte }); // ─── Case (b): thinking block followed by text (pure-text response) ────────── -// This is the #4633 happy path. MUST still be emitted so Claude Code / -// Cursor know when the thinking section ends. -test("thinking block followed by text: IS still emitted (preserves #4633)", () => { - const state = newState(); +// #4633 opt-in path: when suppressThinkClose is false (header `on`), +// is still emitted for legacy content-scanning clients. Production default +// (#8245) sets suppressThinkClose=true via resolveSuppressThinkClose. +test("thinking block followed by text: emitted when suppressThinkClose=false (#4633 opt-in)", () => { + const state = { ...newState(), suppressThinkClose: false }; const allResults: ReturnType[] = []; @@ -175,3 +176,50 @@ test("thinking block followed by text: IS still emitted (preserves #463 `Expected a chunk with delta.content === "" 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: suppressed when suppressThinkClose=true (#8245)", () => { + const state = { ...newState(), suppressThinkClose: true }; + const allResults: ReturnType[] = []; + + 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 === "" + ); + 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"); +}); diff --git a/tests/unit/glm-think-close-marker-leak.test.ts b/tests/unit/glm-think-close-marker-leak.test.ts index 7fcb9b862d..37df29c741 100644 --- a/tests/unit/glm-think-close-marker-leak.test.ts +++ b/tests/unit/glm-think-close-marker-leak.test.ts @@ -144,25 +144,7 @@ test("GLM translateSseResponse: suppressThinkClose=true suppresses mark ); }); -test("GLM translateSseResponse: default (no flag) emits 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(""), - "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, diff --git a/tests/unit/think-close-marker-responses-format.test.ts b/tests/unit/think-close-marker-responses-format.test.ts index 002045ae86..9f6a2b3314 100644 --- a/tests/unit/think-close-marker-responses-format.test.ts +++ b/tests/unit/think-close-marker-responses-format.test.ts @@ -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 ); }); diff --git a/tests/unit/think-close-marker-suppress-5245.test.ts b/tests/unit/think-close-marker-suppress-5245.test.ts index a93439837d..42e27faea5 100644 --- a/tests/unit/think-close-marker-suppress-5245.test.ts +++ b/tests/unit/think-close-marker-suppress-5245.test.ts @@ -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 `` 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 `` 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 = {}) { return { toolCalls: new Map(), toolNameMap: new Map(), ...extra } as Record; @@ -123,32 +123,31 @@ test("shouldSuppressThinkCloseMarker: suppresses for OpenCode, preserves CC/Curs // ── translator gating ──────────────────────────────────────────────────────── -test("claude-to-openai: default emits the marker before the first text (#4633 preserved)", () => { - const contents = contentChunks(runThinkThenText(newState())); - assert.ok(contents.includes(""), "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("") < contents.indexOf("169")); -}); - -test("claude-to-openai: suppressThinkClose drops the 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(""), "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 by default for thinking-only response (#4633)", () => { - const contents = contentChunks(runThinkOnlyThenFinish(newState())); - assert.ok(contents.includes(""), "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(""), "marker must be emitted when opted in"); + assert.ok(contents.includes("169"), "real text still emitted"); + assert.ok(contents.indexOf("") < 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(""), "marker must be suppressed at finish"); }); +test("claude-to-openai: finish-flush emits when suppressThinkClose=false (#4633 opt-in)", () => { + const contents = contentChunks(runThinkOnlyThenFinish(newState({ suppressThinkClose: false }))); + assert.ok(contents.includes(""), "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 , #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 + ); });