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

open-sse/utils/thinkCloseMarker.ts

@@ -2,50 +2,44 @@
  * `</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";
 
 /** 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;
 }

tests/unit/anthropic-stream-thinking-close-marker.test.ts

@@ -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();
 

tests/unit/claude-to-openai-think-close-5123.test.ts

@@ -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");
+});

tests/unit/glm-think-close-marker-leak.test.ts

@@ -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,16 +154,24 @@ 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({
       userAgent: "cursor-agent/0.5",
       thinkingMarkerHeader: "off",
     }),
     true,
-    "Header off must override UA"
+    "Header off suppresses"
   );
   assert.equal(
     THINKING_MARKER_HEADER,

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
   );
 });