fix(cli): Codex long-session turn-pin fallback + codex-settings key resolution (#13564 #13563)

changelog.d/fixes/13566-codex-settings-apikey.md

@@ -0,0 +1 @@
+- fix(api): resolve the codex-settings `apiKey` through the canonical key resolver instead of an inline 400 guard, so the dashboard Apply flow no longer fails with `baseUrl, apiKey and model are required` in cloud mode when no management key is selected (#13563)

changelog.d/fixes/13566-codex-turnpin-fallback.md

@@ -0,0 +1 @@
+- fix(sse): release the native Codex turn pin when the pinned model becomes model-scoped unusable, so a long-running Codex session falls back to the next healthy combo model instead of dying to a terminal `400 NATIVE_CODEX_PINNED_MODEL_UNAVAILABLE` (#13564)

open-sse/services/combo.ts

@@ -90,8 +90,8 @@ export {
 import {
   applyNativeCodexTurnPin,
   areAllPinnedTargetsModelScopedUnusable,
-  createPinnedModelUnavailableResponse,
   getNativeCodexTurnPin,
+  releaseNativeCodexTurnPin,
 } from "./combo/nativeCodexTurnPin.ts";
 import {
   pinIsDurablyUnhealthy,
@@ -837,37 +837,40 @@ async function handleComboChatInner({
   if (activeNativeTurnPin) {
     const pinnedTargets = applyNativeCodexTurnPin(orderedTargets, activeNativeTurnPin);
     if (pinnedTargets.length === 0) {
-      //#11371: quota-share ordering reserved a winner slot; release on
-      //early exit (idempotent).
-      targetResolution.quotaShareRelease?.();
+      // Pinned model no longer exists in the combo — release pin and fall through
+      // to full combo routing so the turn can continue with a healthy model.
+      releaseNativeCodexTurnPin(body as Record<string, unknown>, combo.name);
       log.warn(
         "COMBO",
-        `Native Codex turn cannot continue: pinned model ${activeNativeTurnPin.modelStr} unavailable (target not in combo); preserving turn pin and terminating turn`
+        `Native Codex turn pin released: pinned model ${activeNativeTurnPin.modelStr} no longer in combo; falling back to full combo routing`
       );
-      return createPinnedModelUnavailableResponse();
-    }
-    const allPinnedUnusable = await areAllPinnedTargetsModelScopedUnusable({
-      pinnedTargets,
-      resilienceSettings,
-      quotaCutoffResetWindowConfig,
-      comboName: combo.name,
-      body: body as Record<string, unknown>,
-      log,
-      isModelAvailable,
-    });
-    if (allPinnedUnusable) {
-      targetResolution.quotaShareRelease?.();
-      log.warn(
-        "COMBO",
-        `Native Codex turn cannot continue: pinned model ${activeNativeTurnPin.modelStr} is unavailable (model-scoped); preserving turn pin and terminating turn`
-      );
-      return createPinnedModelUnavailableResponse();
     } else {
-      orderedTargets = pinnedTargets;
-      log.info(
-        "COMBO",
-        `Native Codex turn pinned to ${activeNativeTurnPin.modelStr} on connection ${activeNativeTurnPin.connectionId.slice(0, 8)}`
-      );
+      const allPinnedUnusable = await areAllPinnedTargetsModelScopedUnusable({
+        pinnedTargets,
+        resilienceSettings,
+        quotaCutoffResetWindowConfig,
+        comboName: combo.name,
+        body: body as Record<string, unknown>,
+        log,
+        isModelAvailable,
+      });
+      if (allPinnedUnusable) {
+        // All pinned provider+model targets are model-scoped unusable — release
+        // the pin and fall through to full combo routing so the turn can try
+        // other models in the combo pool. This matches Claude Code's behavior
+        // where no turn pin allows natural multi-model fallback.
+        releaseNativeCodexTurnPin(body as Record<string, unknown>, combo.name);
+        log.warn(
+          "COMBO",
+          `Native Codex turn pin released: pinned model ${activeNativeTurnPin.modelStr} model-scoped unavailable; falling back to full combo routing`
+        );
+      } else {
+        orderedTargets = pinnedTargets;
+        log.info(
+          "COMBO",
+          `Native Codex turn pinned to ${activeNativeTurnPin.modelStr} on connection ${activeNativeTurnPin.connectionId.slice(0, 8)}`
+        );
+      }
     }
   }
 

open-sse/services/combo/nativeCodexTurnPin.ts

@@ -280,6 +280,11 @@ export async function areAllPinnedTargetsModelScopedUnusable(
   return true;
 }
 
+export function releaseNativeCodexTurnPin(body: Record<string, unknown>, comboName: string): void {
+  const key = nativeCodexTurnKey(body, comboName);
+  if (key) pins.delete(key);
+}
+
 export function clearNativeCodexTurnPinsForTests(): void {
   pins.clear();
 }

src/app/api/cli-tools/codex-settings/route.ts

@@ -13,7 +13,7 @@ import { createMultiBackup } from "@/shared/services/backupService";
 import { saveCliToolLastConfigured, deleteCliToolLastConfigured } from "@/lib/db/cliToolState";
 import { cliModelConfigSchema } from "@/shared/validation/schemas";
 import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
-import { getApiKeyById } from "@/lib/db/apiKeys";
+import { resolveApiKey } from "@/shared/services/apiKeyResolver";
 import { normalizeCodexBaseUrl } from "@/shared/utils/codexBaseUrl";
 import { migrateCodexFeatureFlags } from "@/shared/utils/codexConfig";
 
@@ -214,25 +214,9 @@ export async function POST(request: Request) {
       return NextResponse.json({ error: validation.error }, { status: 400 });
     }
     const { baseUrl, model, reasoningEffort, wireApi, modelMappings } = validation.data;
-    let { apiKey } = validation.data;
-    if (!apiKey) {
-      return NextResponse.json(
-        { error: "baseUrl, apiKey and model are required" },
-        { status: 400 }
-      );
-    }
-
-    // Resolve real key from DB by ID
-    if (keyId) {
-      try {
-        const keyRecord = await getApiKeyById(keyId);
-        if (keyRecord?.key) {
-          apiKey = keyRecord.key as string;
-        }
-      } catch {
-        // Non-critical: fall back to whatever value was in apiKey
-      }
-    }
+    // Canonical key resolution (#13563): by keyId -> submitted apiKey -> sk_omniroute.
+    // Matches cline/forge/openclaw/grok-build/jcode-settings.
+    const apiKey = await resolveApiKey(keyId, validation.data.apiKey);
 
     const codexDir = getCodexDir();
     const configPath = getCodexConfigPath();

tests/unit/codex-settings-api-key-resolution.test.ts

@@ -0,0 +1,115 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { SignJWT } from "jose";
+
+/**
+ * Regression test for #13563.
+ *
+ * Applying Codex settings from /dashboard/cli-code/codex always failed with
+ * `400 baseUrl, apiKey and model are required` whenever the dashboard sent an
+ * empty apiKey — which it does in cloud mode (CLOUD_URL set) with no management
+ * key selected. `baseUrl` and `model` are already Zod-gated (min(1)), so this
+ * response could only ever fire on an empty apiKey, yet the error text points
+ * at all three fields.
+ *
+ * The codex-settings route had diverged from the canonical API-key resolution
+ * used by cline/forge/openclaw/grok-build/jcode-settings: an inline
+ * `if (!apiKey) return 400` guard plus a hand-rolled `getApiKeyById` lookup,
+ * instead of the shared `resolveApiKey(keyId, apiKey)`. That helper resolves by
+ * keyId first, then falls back to the submitted apiKey, then to `sk_omniroute`.
+ *
+ * This test drives the real POST handler end-to-end (real DB-backed API key,
+ * real JWT auth cookie) and asserts the value written into auth.json.
+ */
+
+const TEST_ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "omr-codex-settings-key-"));
+const TEST_HOME = path.join(TEST_ROOT, "fake-home");
+fs.mkdirSync(TEST_HOME, { recursive: true });
+
+const originalHome = os.homedir;
+const originalDataDir = process.env.DATA_DIR;
+const originalJwtSecret = process.env.JWT_SECRET;
+const originalWriteFlag = process.env.CLI_ALLOW_CONFIG_WRITES;
+
+os.homedir = () => TEST_HOME;
+process.env.DATA_DIR = path.join(TEST_ROOT, "data");
+process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "codex-settings-key-api-secret";
+process.env.CLI_ALLOW_CONFIG_WRITES = "true";
+
+const core = await import("../../src/lib/db/core.ts");
+const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
+const route = await import("../../src/app/api/cli-tools/codex-settings/route.ts");
+
+const AUTH_PATH = path.join(TEST_HOME, ".codex", "auth.json");
+
+test.after(async () => {
+  core.resetDbInstance();
+  os.homedir = originalHome;
+  if (originalDataDir === undefined) delete process.env.DATA_DIR;
+  else process.env.DATA_DIR = originalDataDir;
+  if (originalJwtSecret === undefined) delete process.env.JWT_SECRET;
+  else process.env.JWT_SECRET = originalJwtSecret;
+  if (originalWriteFlag === undefined) delete process.env.CLI_ALLOW_CONFIG_WRITES;
+  else process.env.CLI_ALLOW_CONFIG_WRITES = originalWriteFlag;
+  fs.rmSync(TEST_ROOT, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+});
+
+const authCookie = async (): Promise<string> => {
+  process.env.JWT_SECRET = "codex-settings-key-api-jwt";
+  const token = await new SignJWT({ authenticated: true, sub: "codex-settings-key-api-test" })
+    .setProtectedHeader({ alg: "HS256" })
+    .setExpirationTime("1h")
+    .sign(new TextEncoder().encode(process.env.JWT_SECRET));
+  return `auth_token=${token}`;
+};
+
+const post = async (body: Record<string, unknown>) =>
+  route.POST(
+    new Request("http://localhost/api/cli-tools/codex-settings", {
+      method: "POST",
+      headers: {
+        cookie: await authCookie(),
+        "Content-Type": "application/json",
+      },
+      body: JSON.stringify({
+        baseUrl: "http://localhost:20128/api/v1",
+        model: "gpt-5.6-sol",
+        ...body,
+      }),
+    })
+  );
+
+const readWrittenApiKey = (): string | null => {
+  try {
+    return JSON.parse(fs.readFileSync(AUTH_PATH, "utf8")).OPENAI_API_KEY ?? null;
+  } catch {
+    return null;
+  }
+};
+
+test("#13563: POST codex-settings with empty apiKey resolves the real key from keyId instead of 400", async () => {
+  const created = await apiKeysDb.createApiKey("codex-settings-key-test", "codex-settings-machine");
+  assert.ok(created.key && created.key.length > 0, "createApiKey must return a real plaintext key");
+
+  const response = await post({ apiKey: "", keyId: created.id });
+
+  assert.equal(response.status, 200);
+  assert.equal(readWrittenApiKey(), created.key);
+});
+
+test("#13563: POST codex-settings with empty apiKey and no keyId writes sk_omniroute instead of 400", async () => {
+  const response = await post({ apiKey: "" });
+
+  assert.equal(response.status, 200);
+  assert.equal(readWrittenApiKey(), "sk_omniroute");
+});
+
+test("#13563: POST codex-settings with an explicit apiKey still writes it verbatim", async () => {
+  const response = await post({ apiKey: "sk-test-explicit" });
+
+  assert.equal(response.status, 200);
+  assert.equal(readWrittenApiKey(), "sk-test-explicit");
+});