fix(routing): normalize thinking:disabled for combo-substituted models that reject it (#3554) (#3563)

Integrated into release/v3.8.20

CHANGELOG.md

@@ -10,7 +10,9 @@
 
 ## [3.8.20] — Unreleased
 
-_Development cycle in progress._
+### 🔧 Bug Fixes
+
+- **fix(routing):** combo model substitution no longer forwards a client `thinking:{type:"disabled"}` to a target model that rejects it — when a combo/route swaps the upstream model (e.g. `claude-opus-4-8` → `claude-fable-5`), OmniRoute now strips the now-invalid `thinking.type:"disabled"` for models flagged `rejectsThinkingDisabled` (Fable 5 defaults to adaptive and rejects it), preventing the upstream 400 that silently broke Claude Code's internal title/name-generation calls. Models that accept `disabled` (opus/sonnet) are untouched. ([#3554](https://github.com/diegosouzapw/OmniRoute/issues/3554))
 
 ---
 

open-sse/handlers/chatCore.ts

@@ -34,6 +34,7 @@ import {
 import { resolveModelAlias } from "../services/modelDeprecation.ts";
 import { getUnsupportedParams } from "../config/providerRegistry.ts";
 import { supportsMaxTokens } from "@/lib/modelCapabilities.ts";
+import { normalizeThinkingForModel } from "@/shared/constants/modelSpecs.ts";
 import {
   buildErrorBody,
   createErrorResult,
@@ -3553,6 +3554,14 @@ export async function handleChatCore({
   }
   translatedBody.model = finalModelToUpstream;
 
+  // #3554: a combo/route may substitute the upstream model AFTER the client chose its
+  // `thinking` value. Claude Code sends `thinking:{type:"disabled"}` for internal calls,
+  // which claude-fable-5 (adaptive-only) rejects with a 400. Drop the now-invalid value
+  // when the resolved target model rejects it; models that accept `disabled` are untouched.
+  if (typeof finalModelToUpstream === "string") {
+    translatedBody = normalizeThinkingForModel(translatedBody, finalModelToUpstream);
+  }
+
   const previousResponseIdPolicy = applyResponsesPreviousResponseIdPolicy(translatedBody, {
     mode: settings.responsesPreviousResponseIdMode,
     sourceFormat,

src/shared/constants/modelSpecs.ts

@@ -15,6 +15,10 @@ export interface ModelSpec {
   supportsThinking?: boolean;
   supportsTools?: boolean;
   supportsVision?: boolean;
+  // Model defaults to adaptive thinking and REJECTS an explicit `thinking.type:"disabled"`
+  // (upstream returns 400). Used to normalize the request when a combo/route substitutes
+  // this model after the client already chose `disabled`. See issue #3554.
+  rejectsThinkingDisabled?: boolean;
 }
 
 const BEDROCK_CLAUDE_ALIASES = (...modelIds: string[]) => [
@@ -221,6 +225,8 @@ export const MODEL_SPECS: Record<string, ModelSpec> = {
     supportsThinking: true,
     supportsTools: true,
     supportsVision: true,
+    // Fable 5 defaults to adaptive thinking and rejects `thinking.type:"disabled"` (#3554).
+    rejectsThinkingDisabled: true,
     aliases: BEDROCK_CLAUDE_ALIASES("claude-fable-5"),
   },
 
@@ -427,6 +433,34 @@ export function getModelSpec(modelId: string): ModelSpec | undefined {
   return undefined;
 }
 
+/**
+ * Normalize a request's `thinking` field against the (possibly combo-substituted) target model.
+ *
+ * A combo/route can swap the upstream model AFTER the client already chose its `thinking`
+ * value. Claude Code sends `thinking:{type:"disabled"}` for internal title/name-generation
+ * calls — valid for opus/sonnet, but claude-fable-5 defaults to adaptive thinking and rejects
+ * `type:"disabled"` with an upstream 400. When the resolved target model is flagged
+ * `rejectsThinkingDisabled`, drop the now-invalid `thinking` so the model uses its adaptive
+ * default instead of hard-failing. Models that accept `disabled` are left untouched, and any
+ * non-`disabled` thinking (enabled/adaptive) is always preserved. See issue #3554.
+ */
+export function normalizeThinkingForModel<T extends Record<string, unknown>>(
+  body: T,
+  modelId: string
+): T {
+  const thinking = body?.thinking as Record<string, unknown> | undefined;
+  if (
+    thinking &&
+    typeof thinking === "object" &&
+    thinking.type === "disabled" &&
+    getModelSpec(modelId)?.rejectsThinkingDisabled
+  ) {
+    const { thinking: _omitted, ...rest } = body as Record<string, unknown>;
+    return rest as T;
+  }
+  return body;
+}
+
 export function capMaxOutputTokens(modelId: string, requested?: number): number {
   const spec = getModelSpec(modelId);
   const cap = spec?.maxOutputTokens ?? MODEL_SPECS.__default__.maxOutputTokens;

tests/unit/combo-thinking-disabled-fable5-3554.test.ts

@@ -0,0 +1,61 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import {
+  normalizeThinkingForModel,
+  getModelSpec,
+} from "../../src/shared/constants/modelSpecs.ts";
+
+// Regression for #3554: a combo can substitute the upstream model AFTER the client
+// already chose its `thinking` value. Claude Code sends `thinking:{type:"disabled"}` for
+// internal title/name-generation calls. That value is valid for claude-opus-4-8 and
+// claude-sonnet-4-6, but claude-fable-5 defaults to adaptive thinking and REJECTS
+// `thinking.type:"disabled"` with an upstream 400. When the substituted target rejects
+// `disabled`, OmniRoute must strip the now-invalid value instead of forwarding it.
+
+test("#3554 claude-fable-5 is flagged as rejecting thinking.type:disabled", () => {
+  assert.equal(getModelSpec("claude-fable-5")?.rejectsThinkingDisabled, true);
+});
+
+test("#3554 models that accept disabled are NOT flagged (opus-4-8, sonnet-4-6)", () => {
+  assert.notEqual(getModelSpec("claude-opus-4-8")?.rejectsThinkingDisabled, true);
+  assert.notEqual(getModelSpec("claude-sonnet-4-6")?.rejectsThinkingDisabled, true);
+});
+
+test("#3554 normalizeThinkingForModel strips thinking.type:disabled for fable-5", () => {
+  const body = { model: "claude-opus-4-8", thinking: { type: "disabled" }, max_tokens: 64000 };
+  const out = normalizeThinkingForModel(body, "claude-fable-5");
+  assert.equal("thinking" in out, false, "thinking must be stripped for fable-5");
+  assert.equal(out.max_tokens, 64000, "other fields untouched");
+  assert.equal(out.model, "claude-opus-4-8", "model field untouched by this helper");
+});
+
+test("#3554 normalizeThinkingForModel preserves disabled for opus-4-8 and sonnet-4-6", () => {
+  for (const m of ["claude-opus-4-8", "claude-sonnet-4-6"]) {
+    const out = normalizeThinkingForModel({ model: m, thinking: { type: "disabled" } }, m);
+    assert.deepEqual(out.thinking, { type: "disabled" }, `disabled preserved for ${m}`);
+  }
+});
+
+test("#3554 normalizeThinkingForModel preserves enabled/adaptive thinking for fable-5", () => {
+  const enabled = normalizeThinkingForModel(
+    { thinking: { type: "enabled", budget_tokens: 4000 } },
+    "claude-fable-5"
+  );
+  assert.deepEqual(enabled.thinking, { type: "enabled", budget_tokens: 4000 });
+  const adaptive = normalizeThinkingForModel({ thinking: { type: "adaptive" } }, "claude-fable-5");
+  assert.deepEqual(adaptive.thinking, { type: "adaptive" });
+});
+
+test("#3554 normalizeThinkingForModel is a no-op when there is no thinking field", () => {
+  const body = { model: "claude-fable-5", messages: [] };
+  const out = normalizeThinkingForModel(body, "claude-fable-5");
+  assert.deepEqual(out, body);
+});
+
+test("#3554 normalizeThinkingForModel tolerates unknown models (no spec → preserve)", () => {
+  const out = normalizeThinkingForModel(
+    { thinking: { type: "disabled" } },
+    "some-unknown-model-xyz"
+  );
+  assert.deepEqual(out.thinking, { type: "disabled" });
+});