fix(thinking): header opt-out of </think> marker for reasoning_content clients (#5312 RC-C, #5245) (#5367)

Integrated into release/v3.8.41 — header opt-out of </think> marker (#5312 RC-C, #5245).
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-29 12:19:16 -03:00
committed by GitHub
parent 6a5971663a
commit 734061f736
4 changed files with 150 additions and 4 deletions

View File

@@ -10,6 +10,7 @@ _In development — bullets added per PR; finalized at release._
### 🔧 Bug Fixes
- **thinking (claude):** let reasoning_content-native clients (e.g. Cursor) opt out of the `</think>` close-marker via the request header `x-omniroute-thinking-marker: off` so it no longer leaks an orphan `</think>` into visible `content` (RC-C of #5312, shared with #5245). The suppression machinery already existed (UA allowlist, #5348); this adds explicit per-request opt-in (also `on`/`keep`). Header absent ⇒ byte-identical default (Claude Code/#4633 clients still get the marker). Regression guard: `tests/unit/think-close-marker-suppress-5245.test.ts`. ([#5312](https://github.com/diegosouzapw/OmniRoute/issues/5312), [#5245](https://github.com/diegosouzapw/OmniRoute/issues/5245))
- **cors:** browser/Electron clients (e.g. Wayland AI) can now use OmniRoute as an OpenAI-compatible provider out-of-the-box. The token-authenticated API surface (`/v1/*`, `/v1beta/*`) now returns a permissive `Access-Control-Allow-Origin` (echoes the request `Origin`, `*` when absent) by default — matching 9router and the OpenAI-compatible ecosystem — so a renderer `fetch` can read the response instead of failing CORS-blocked as "site not found" / empty catalog (while `curl`, which sends no preflight, worked). This is **safe**: those routes auth via `Authorization`/`x-api-key` headers browsers never auto-attach (no credentialed-session/CSRF exposure), and `Access-Control-Allow-Credentials` is never paired with the echo/wildcard. Cookie-authed **MANAGEMENT/dashboard routes stay exactly fail-closed**; `CORS_ALLOW_ALL`/`CORS_ALLOWED_ORIGINS` still take precedence. Regression guards: `tests/unit/cors/origins.test.ts`, `tests/unit/authz/pipeline.test.ts`. (Bug 2 of [#5242](https://github.com/diegosouzapw/OmniRoute/issues/5242) — thanks @jonlwheat2-gif)
- **grok-web:** forward the Cloudflare clearance cookies and stop mislabeling IP-reputation blocks as a bad cookie. "Check cookie" returned `Invalid SSO cookie` even with a valid, complete browser session — but the cookie parser was never the problem (it robustly extracts `sso`/`sso-rw` from a full DevTools header). Two real gaps fixed: **(1)** `buildGrokCookieHeader` now forwards `cf_clearance` and `__cf_bm` when pasted (it dropped them before; AIClient2API forwards them too) — strictly additive, a bare `sso` blob still yields exactly `sso=…`; **(2)** when the user supplied a `cf_clearance`, a 401 / invalid-credentials-403 from grok.com is now surfaced as an IP-reputation/anti-bot block (cf_clearance is IP+TLS+UA-pinned and can't be replayed from a different machine) instead of the misleading "Invalid SSO cookie — re-paste". A bare cookie with no clearance still gets the re-paste hint. Regression guards in `web-cookie-auth.test.ts` + `provider-validation-specialty.test.ts`. ([#5350](https://github.com/diegosouzapw/OmniRoute/issues/5350) — thanks @SeaXen)
- **cli (serve):** opt-in native **HTTPS/TLS** for `omniroute serve` — so strict-CSP Electron apps and browsers can reach OmniRoute over `https://` instead of plain `http://localhost`. Provide `--tls-cert <path> --tls-key <path>` (or `OMNIROUTE_TLS_CERT`/`OMNIROUTE_TLS_KEY`) and the standalone server terminates TLS on the same listener (no extra port/proxy); WebSocket upgrade (live dashboard + `/v1` streaming) works over `wss://` unchanged since `https.Server extends http.Server`. With no TLS flags the HTTP path is byte-identical to before; only one of cert/key, or an unreadable path, logs a warning and stays HTTP (never half-enables, never crashes). Auto-generated self-signed certs for localhost are a follow-up; for now provide an explicit cert/key (or front OmniRoute with a TLS terminator). Regression guard: `tests/unit/tls-options.test.ts`. (Bug 1C of [#5242](https://github.com/diegosouzapw/OmniRoute/issues/5242) — thanks @jonlwheat2-gif)

View File

@@ -73,7 +73,10 @@ import {
withBodyTimeout,
} from "../utils/stream.ts";
import { ensureStreamReadiness } from "../utils/streamReadiness.ts";
import { shouldSuppressThinkCloseMarker } from "../utils/thinkCloseMarker.ts";
import {
resolveSuppressThinkClose,
THINKING_MARKER_HEADER,
} from "../utils/thinkCloseMarker.ts";
import { resolveStreamReadinessTimeout } from "../utils/streamReadinessPolicy.ts";
import { createStreamController } from "../utils/streamHandler.ts";
import * as streamFailure from "../utils/streamFailureFinalization.ts";
@@ -788,6 +791,15 @@ export async function handleChatCore({
.filter(Boolean)
.join(" ");
// Explicit per-request opt-in/out for the `</think>` close marker
// (#5312 / #5245): `x-omniroute-thinking-marker: off` suppresses it for
// reasoning_content-native clients (e.g. Cursor's OpenAI path) that the UA
// allowlist does not cover; absent the header, the UA policy applies.
const thinkingMarkerHeader = getHeaderValueCaseInsensitive(
clientRawRequest?.headers ?? null,
THINKING_MARKER_HEADER
);
const explicitStreamAlias = resolveExplicitStreamAlias(body);
// Remove non-standard non-stream aliases before provider translation/execution.
@@ -4191,8 +4203,13 @@ export async function handleChatCore({
handleStreamFailure,
copilotCompatibleReasoning,
// Suppress the `</think>` close marker for clients that render it verbatim
// (e.g. OpenCode); preserved for Claude Code / Cursor and unknown clients (#5245).
shouldSuppressThinkCloseMarker(streamUserAgent)
// (e.g. OpenCode by UA; any client via `x-omniroute-thinking-marker: off`);
// preserved for Claude Code / Cursor and unknown clients by default (#5245 /
// #5312). The header wins over the UA allowlist.
resolveSuppressThinkClose({
userAgent: streamUserAgent,
thinkingMarkerHeader,
})
);
} else {
log?.debug?.("STREAM", `Standard passthrough mode`);

View File

@@ -14,8 +14,18 @@
* 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`.
*
* 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.
*/
/** 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.
const SUPPRESS_THINK_CLOSE_UA_MARKERS = ["opencode"];
@@ -30,3 +40,34 @@ export function shouldSuppressThinkCloseMarker(userAgent: string | null | undefi
const ua = userAgent.toLowerCase();
return SUPPRESS_THINK_CLOSE_UA_MARKERS.some((marker) => ua.includes(marker));
}
/**
* 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).
*/
export function thinkingMarkerHeaderSignal(
headerValue: string | null | undefined
): boolean | null {
if (typeof headerValue !== "string") return null;
const value = headerValue.trim().toLowerCase();
if (value === "off" || value === "false" || value === "0" || value === "suppress") return true;
if (value === "on" || value === "true" || value === "1" || value === "keep") return false;
return null;
}
/**
* 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.
*/
export function resolveSuppressThinkClose(opts: {
userAgent?: string | null;
thinkingMarkerHeader?: string | null;
}): boolean {
const headerSignal = thinkingMarkerHeaderSignal(opts.thinkingMarkerHeader);
if (headerSignal !== null) return headerSignal;
return shouldSuppressThinkCloseMarker(opts.userAgent);
}

View File

@@ -3,7 +3,12 @@ import assert from "node:assert/strict";
const { claudeToOpenAIResponse } =
await import("../../open-sse/translator/response/claude-to-openai.ts");
const { shouldSuppressThinkCloseMarker } = await import("../../open-sse/utils/thinkCloseMarker.ts");
const {
shouldSuppressThinkCloseMarker,
thinkingMarkerHeaderSignal,
resolveSuppressThinkClose,
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
@@ -55,6 +60,42 @@ function runThinkThenText(state: Record<string, unknown>) {
return out;
}
// Drive a thinking-only stream that flushes the deferred </think> at the
// message_delta finish path (no text_delta ever arrives; toolCalls stays empty).
// Exercises the L197-207 finish-flush branch in claude-to-openai.ts.
function runThinkOnlyThenFinish(state: Record<string, unknown>) {
const out: unknown[] = [];
const push = (r: unknown) => {
if (Array.isArray(r)) out.push(...r);
else if (r) out.push(r);
};
push(claudeToOpenAIResponse({ type: "message_start", message: { id: "m1" } }, state));
push(
claudeToOpenAIResponse(
{ type: "content_block_start", index: 0, content_block: { type: "thinking" } },
state
)
);
push(
claudeToOpenAIResponse(
{
type: "content_block_delta",
index: 0,
delta: { type: "thinking_delta", thinking: "plan" },
},
state
)
);
push(claudeToOpenAIResponse({ type: "content_block_stop", index: 0 }, state));
push(
claudeToOpenAIResponse(
{ type: "message_delta", delta: { stop_reason: "end_turn" }, usage: { output_tokens: 5 } },
state
)
);
return out;
}
function contentChunks(chunks: unknown[]): string[] {
return chunks
.map(
@@ -92,3 +133,49 @@ test("claude-to-openai: suppressThinkClose drops the </think> marker but keeps t
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: finish-flush suppressed under suppressThinkClose (#5312)", () => {
const contents = contentChunks(runThinkOnlyThenFinish(newState({ suppressThinkClose: true })));
assert.ok(!contents.includes("</think>"), "marker must be suppressed at finish");
});
// ── header signal resolution (x-omniroute-thinking-marker — #5312) ───────────
test("thinkingMarkerHeaderSignal: off → suppress, on → keep, absent/unknown → null", () => {
assert.equal(thinkingMarkerHeaderSignal("off"), true);
assert.equal(thinkingMarkerHeaderSignal("OFF"), true);
assert.equal(thinkingMarkerHeaderSignal(" off "), true);
assert.equal(thinkingMarkerHeaderSignal("on"), false);
assert.equal(thinkingMarkerHeaderSignal("keep"), false);
assert.equal(thinkingMarkerHeaderSignal(""), null);
assert.equal(thinkingMarkerHeaderSignal("weird"), null);
assert.equal(thinkingMarkerHeaderSignal(null), null);
assert.equal(thinkingMarkerHeaderSignal(undefined), null);
});
test("resolveSuppressThinkClose: header opts in (Cursor) and overrides the UA allowlist", () => {
// 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.
assert.equal(
resolveSuppressThinkClose({ userAgent: "cursor-agent/0.5", thinkingMarkerHeader: "off" }),
true
);
// `on` force-keeps even for an allowlisted (OpenCode) UA.
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);
});