fix(relay): normalize bifrost errors, remap credential 404, fix analytics

- Normalize non-OK Bifrost upstream responses through parseUpstreamError +
  buildErrorBody/sanitizeErrorMessage so plain-text/HTML errors are wrapped
  into valid OpenAI JSON instead of leaking raw bodies (invalid character 'd').
- handleNoCredentials() now takes an isCombo flag: single-model requests
  remap the combo 404 to 401/503 so direct clients no longer see a misleading
  'No active credentials' 404. Combo routing keeps 404 for fall-through.
- recordUsage() now records any non-2xx upstream status as 'error' rather
  than 'success' (was status < 500), fixing 4xx analytics misclassification.

Adds/updates tests covering all three scenarios.

src/app/api/v1/relay/chat/completions/route.ts

@@ -10,7 +10,11 @@ import { CORS_HEADERS, handleCorsOptions } from "@/shared/utils/cors";
 import { handleChat } from "@/sse/handlers/chat";
 import { createInjectionGuard } from "@/middleware/promptInjectionGuard";
 import { getRelayTokenByHash, checkRateLimit, recordRelayUsage } from "@/lib/db/relayProxies";
-import { buildErrorBody } from "@omniroute/open-sse/utils/error";
+import {
+  buildErrorBody,
+  parseUpstreamError,
+  sanitizeErrorMessage,
+} from "@omniroute/open-sse/utils/error";
 import {
   checkIpRateLimit,
   extractToken,
@@ -108,6 +112,32 @@ async function forwardToBifrost(
       headers.set("Content-Type", upstream.headers.get("Content-Type") ?? "application/json");
     }
 
+    // Issue #1: Bifrost (or the upstream behind it) may return plain text or HTML
+    // on a non-OK status (e.g. 502 from a sidecar, "invalid character 'd'" style
+    // proxy errors). Forwarding `upstream.body` raw leaks non-JSON into a client
+    // that expects OpenAI-shaped JSON, producing client-side parse failures.
+    // Normalize any non-OK response through parseUpstreamError + buildErrorBody so
+    // the client always receives a valid JSON error. (Hard rule #12.)
+    if (!upstream.ok) {
+      const parsed = await parseUpstreamError(upstream, null);
+      const errorBody = buildErrorBody(
+        parsed.statusCode,
+        sanitizeErrorMessage(parsed.message),
+        parsed.responseBody
+      );
+      const errorHeaders = new Headers(headers);
+      errorHeaders.set("Content-Type", "application/json");
+      if (parsed.retryAfterMs && parsed.retryAfterMs > 0) {
+        errorHeaders.set("Retry-After", String(Math.ceil(parsed.retryAfterMs / 1000)));
+      }
+      clearTimeout(tid);
+      recordUsage(token.id, request, startTime, clientIp, userAgent, "error", parsed.statusCode);
+      return new Response(JSON.stringify(errorBody), {
+        status: parsed.statusCode,
+        headers: errorHeaders,
+      });
+    }
+
     if (wantsStream && upstream.body) {
       const stream = finalizeReadableStream(upstream.body, (error) => {
         clearTimeout(tid);
@@ -144,7 +174,7 @@ async function forwardToBifrost(
       startTime,
       clientIp,
       userAgent,
-      upstream.status < 500 ? "success" : "error",
+      upstream.status >= 200 && upstream.status < 300 ? "success" : "error",
       upstream.status
     );
 

src/sse/handlers/chat.ts

@@ -1673,7 +1673,8 @@ async function handleSingleModelChat(
           model,
           lastError,
           lastStatus,
-          candidateAliases
+          candidateAliases,
+          isCombo
         );
         const lastFailedConnectionId =
           excludedConnectionIds.size > 0
@@ -1818,7 +1819,8 @@ async function handleSingleModelChat(
             comboStrategy,
             isCombo,
             comboStepId: runtimeOptions.comboStepId ?? null,
-            comboExecutionKey: runtimeOptions.comboExecutionKey ?? runtimeOptions.comboStepId ?? null,
+            comboExecutionKey:
+              runtimeOptions.comboExecutionKey ?? runtimeOptions.comboStepId ?? null,
             extendedContext,
             modelApiFormat: apiFormat,
             modelTargetFormat: targetFormat,

src/sse/handlers/chatHelpers.ts

@@ -630,7 +630,8 @@ export function handleNoCredentials(
   model: string,
   lastError: string | null,
   lastStatus: number | null,
-  candidateAliases?: readonly string[]
+  candidateAliases?: readonly string[],
+  isCombo: boolean = false
 ) {
   if (credentials?.allRateLimited) {
     const errorMsg = lastError || credentials.lastError || "Unavailable";
@@ -705,7 +706,7 @@ export function handleNoCredentials(
     log.warn("AUTH", `No active credentials for provider: ${provider}`);
     // #FIX: surface the candidate aliases (from resolveModelOrError) so the
     // operator can pick a working provider/model prefix instead of guessing.
-    // Without this, "No active credentials for provider: kiro" leaves the
+    // Without this, "No active credentials for provider: byNara" leaves the
     // user staring at a wall — most bugs in this area are actually "wrong
     // provider was picked", not "the provider is broken".
     const hint =
@@ -715,6 +716,26 @@ export function handleNoCredentials(
             .map((a) => `${a}/${model}`)
             .join(", ")}.`
         : "";
+
+    // Issue #2: for single-model (non-combo) requests, a 404 leaks a misleading
+    // "No active credentials" status to a direct API client (e.g. OpenCode) that
+    // then mis-files it as "resource not found" instead of an auth/credential
+    // failure. The 404 is only meaningful as a combo fall-through signal, so
+    // remap it to an explicit error status for single-model traffic: a 401 when
+    // the provider exists but has no usable credentials, else 503 when the
+    // provider itself is unknown/unreachable. Combo routing keeps the 404 so it
+    // can still skip past a disabled-credentials leg.
+    if (!isCombo) {
+      const singleModelStatus =
+        provider && String(provider).trim().length > 0
+          ? HTTP_STATUS.UNAUTHORIZED
+          : HTTP_STATUS.SERVICE_UNAVAILABLE;
+      return errorResponse(
+        singleModelStatus,
+        `No active credentials for provider: ${provider}.${hint}`
+      );
+    }
+
     return errorResponse(
       HTTP_STATUS.NOT_FOUND,
       `No active credentials for provider: ${provider}.${hint}`

tests/unit/api/v1/relay-completions-errors.test.ts

@@ -0,0 +1,200 @@
+import { test } from "node:test";
+import assert from "node:assert/strict";
+import { createHash } from "node:crypto";
+import {
+  checkIpRateLimit,
+  getClientIp,
+  sanitizeForensicHeader,
+} from "../../../../src/app/api/v1/relay/chat/completions/relaySecurity.ts";
+import { getDbInstance } from "../../../../src/lib/db/core.ts";
+import { getRelayLogs } from "../../../../src/lib/db/relayProxies.ts";
+
+// ─── Relay completions route: Bifrost upstream error normalization ──────────
+//
+// T-issues: (1) a plain-text/HTML non-OK Bifrost response must be normalized
+// into a valid OpenAI JSON error instead of leaking raw text (which produces
+// client-side "invalid character 'd'" parse failures); (3) upstream 4xx must be
+// recorded as analytics "error", never "success".
+
+const ORIGINAL_BIFROST_BASE_URL = process.env.BIFROST_BASE_URL;
+const ORIGINAL_BIFROST_API_KEY = process.env.BIFROST_API_KEY;
+const ORIGINAL_BIFROST_OMNI_KEY = process.env.OMNIROUTE_BIFROST_KEY;
+const ORIGINAL_BIFROST_TIMEOUT = process.env.BIFROST_TIMEOUT_MS;
+const ORIGINAL_BIFROST_STREAMING = process.env.BIFROST_STREAMING_ENABLED;
+const ORIGINAL_RELAY_BACKEND = process.env.OMNIROUTE_RELAY_BACKEND;
+const ORIGINAL_FETCH = globalThis.fetch;
+
+function seedRelayToken(rawToken: string) {
+  const id = `rl_test_${Date.now()}_${Math.random().toString(16).slice(2)}`;
+  const now = Math.floor(Date.now() / 1000);
+  getDbInstance()
+    .prepare(
+      `
+      INSERT INTO relay_tokens (id, name, token_hash, token_prefix, description, combo_id,
+        allowed_models, max_tokens_per_request, max_requests_per_minute, max_requests_per_day,
+        max_cost_per_day, enabled, created_at, updated_at, expires_at, metadata)
+      VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 1, ?, ?, ?, ?)
+    `
+    )
+    .run(
+      id,
+      "relay-completions-err",
+      createHash("sha256").update(rawToken).digest("hex"),
+      "rl_test",
+      "",
+      null,
+      JSON.stringify(["*"]),
+      128000,
+      60,
+      10000,
+      0,
+      now,
+      now,
+      null,
+      "{}"
+    );
+  return { id, rawToken };
+}
+
+function restoreEnv() {
+  if (ORIGINAL_BIFROST_BASE_URL === undefined) delete process.env.BIFROST_BASE_URL;
+  else process.env.BIFROST_BASE_URL = ORIGINAL_BIFROST_BASE_URL;
+  if (ORIGINAL_BIFROST_API_KEY === undefined) delete process.env.BIFROST_API_KEY;
+  else process.env.BIFROST_API_KEY = ORIGINAL_BIFROST_API_KEY;
+  if (ORIGINAL_BIFROST_OMNI_KEY === undefined) delete process.env.OMNIROUTE_BIFROST_KEY;
+  else process.env.OMNIROUTE_BIFROST_KEY = ORIGINAL_BIFROST_OMNI_KEY;
+  if (ORIGINAL_BIFROST_TIMEOUT === undefined) delete process.env.BIFROST_TIMEOUT_MS;
+  else process.env.BIFROST_TIMEOUT_MS = ORIGINAL_BIFROST_TIMEOUT;
+  if (ORIGINAL_BIFROST_STREAMING === undefined) delete process.env.BIFROST_STREAMING_ENABLED;
+  else process.env.BIFROST_STREAMING_ENABLED = ORIGINAL_BIFROST_STREAMING;
+  if (ORIGINAL_RELAY_BACKEND === undefined) delete process.env.OMNIROUTE_RELAY_BACKEND;
+  else process.env.OMNIROUTE_RELAY_BACKEND = ORIGINAL_RELAY_BACKEND;
+  globalThis.fetch = ORIGINAL_FETCH;
+}
+
+function setupBifrostEnv() {
+  process.env.OMNIROUTE_RELAY_BACKEND = "bifrost";
+  process.env.BIFROST_BASE_URL = "http://bifrost.test.local:8080";
+  process.env.BIFROST_TIMEOUT_MS = "5000";
+  delete process.env.BIFROST_API_KEY;
+  delete process.env.OMNIROUTE_BIFROST_KEY;
+  delete process.env.BIFROST_STREAMING_ENABLED;
+}
+
+test("relay route: normalizes plain-text Bifrost 404 into JSON error (Issue #1)", async () => {
+  setupBifrostEnv();
+  const relayToken = seedRelayToken(`relay_err_${Date.now()}`);
+
+  // Bifrost sidecar returns a raw HTML/plain-text non-OK response — the exact
+  // "invalid character 'd'" scenario behind client JSON parse failures.
+  globalThis.fetch = async () => {
+    return new Response("<html><body>404 page not found</body></html>", {
+      status: 404,
+      headers: { "content-type": "text/html" },
+    });
+  };
+
+  const { POST } = await import(
+    `../../../../src/app/api/v1/relay/chat/completions/route.ts?case=${Date.now()}-${Math.random()}`
+  );
+
+  const req = new Request("http://localhost/api/v1/relay/chat/completions", {
+    method: "POST",
+    headers: {
+      authorization: `Bearer ${relayToken.rawToken}`,
+      "content-type": "application/json",
+      "x-request-id": "relay-err-404",
+    },
+    body: JSON.stringify({ model: "gpt-4", messages: [{ role: "user", content: "hi" }] }),
+  });
+
+  const res = await POST(req);
+  // Status preserved from upstream (404), but body is valid JSON, not HTML.
+  assert.equal(res.status, 404);
+  assert.equal(res.headers.get("content-type"), "application/json");
+  // The critical fix: the client receives parseable JSON, NOT a raw HTML body
+  // (which previously caused "invalid character 'd'" JSON.parse failures).
+  const raw = await res.text();
+  assert.doesNotMatch(String(raw), /^</, "response body must be JSON, not raw HTML");
+  const body = JSON.parse(raw);
+  assert.ok(body?.error?.message, "must contain an error message");
+  assert.match(String(body?.error?.message), /page not found/);
+
+  restoreEnv();
+});
+
+test("relay route: normalizes HTML 502 from Bifrost into JSON error (Issue #1)", async () => {
+  setupBifrostEnv();
+  const relayToken = seedRelayToken(`relay_err_${Date.now()}`);
+
+  globalThis.fetch = async () => {
+    return new Response(
+      "<!doctype html><title>502 Bad Gateway</title><pre>invalid character 'd'</pre>",
+      { status: 502, headers: { "content-type": "text/html" } }
+    );
+  };
+
+  const { POST } = await import(
+    `../../../../src/app/api/v1/relay/chat/completions/route.ts?case=${Date.now()}-${Math.random()}`
+  );
+
+  const req = new Request("http://localhost/api/v1/relay/chat/completions", {
+    method: "POST",
+    headers: {
+      authorization: `Bearer ${relayToken.rawToken}`,
+      "content-type": "application/json",
+      "x-request-id": "relay-err-502",
+    },
+    body: JSON.stringify({ model: "gpt-4", messages: [{ role: "user", content: "hi" }] }),
+  });
+
+  const res = await POST(req);
+  assert.equal(res.status, 502);
+  assert.equal(res.headers.get("content-type"), "application/json");
+  const body = await res.json();
+  assert.ok(body?.error?.message);
+
+  // Upstream 4xx/5xx must be recorded as analytics "error" (Issue #3).
+  const logs = getRelayLogs(relayToken.id, 10);
+  assert.equal(logs.length, 1);
+  assert.equal(logs[0].status, "error");
+  assert.equal(logs[0].status_code, 502);
+
+  restoreEnv();
+});
+
+test("relay route: upstream 401 recorded as analytics error not success (Issue #3)", async () => {
+  setupBifrostEnv();
+  const relayToken = seedRelayToken(`relay_err_${Date.now()}`);
+
+  globalThis.fetch = async () => {
+    return new Response(JSON.stringify({ error: { message: "unauthorized" } }), {
+      status: 401,
+      headers: { "content-type": "application/json" },
+    });
+  };
+
+  const { POST } = await import(
+    `../../../../src/app/api/v1/relay/chat/completions/route.ts?case=${Date.now()}-${Math.random()}`
+  );
+
+  const req = new Request("http://localhost/api/v1/relay/chat/completions", {
+    method: "POST",
+    headers: {
+      authorization: `Bearer ${relayToken.rawToken}`,
+      "content-type": "application/json",
+      "x-request-id": "relay-err-401",
+    },
+    body: JSON.stringify({ model: "gpt-4", messages: [{ role: "user", content: "hi" }] }),
+  });
+
+  const res = await POST(req);
+  assert.equal(res.status, 401);
+
+  const logs = getRelayLogs(relayToken.id, 10);
+  assert.equal(logs.length, 1);
+  assert.equal(logs[0].status, "error");
+  assert.equal(logs[0].status_code, 401);
+
+  restoreEnv();
+});

tests/unit/chat-helpers.test.ts

@@ -308,7 +308,18 @@ test("handleNoCredentials reports missing provider credentials and exhausted acc
   // open-sse/services/accountFallback.ts:1593-1599) so the next combo target is
   // tried. We surface "no active credentials" as 404 so combo can skip past a
   // disabled-credentials provider instead of failing the whole request.
-  const missing = handleNoCredentials(null, null, "openai", "gpt-4o-mini", null, null);
+  // In combo routing the no-credentials branch must stay 404 NOT_FOUND so the
+  // combo target loop can fall through to the next target. Pass isCombo=true.
+  const missing = handleNoCredentials(
+    null,
+    null,
+    "openai",
+    "gpt-4o-mini",
+    null,
+    null,
+    undefined,
+    true
+  );
   const exhausted = handleNoCredentials(
     null,
     "conn_123",
@@ -327,6 +338,65 @@ test("handleNoCredentials reports missing provider credentials and exhausted acc
   assert.match(exhaustedJson.error.message, /Primary account failed/);
 });
 
+test("handleNoCredentials remaps leaked 404 to 401/503 for single-model requests", async () => {
+  // Issue #2: a direct (non-combo) API client must not receive a misleading 404
+  // "No active credentials" error — remap to an explicit auth/credential status.
+  const forKnownProvider = handleNoCredentials(
+    null,
+    null,
+    "byNara",
+    "claude-sonnet-4.6",
+    null,
+    null,
+    undefined,
+    /* isCombo */ false
+  );
+  assert.equal(forKnownProvider.status, 401);
+  const knownJson = (await forKnownProvider.json()) as { error?: { message?: string } };
+  assert.match(knownJson.error?.message ?? "", /No active credentials for provider: byNara/);
+
+  const forUnknownProvider = handleNoCredentials(
+    null,
+    null,
+    "",
+    "gpt-4o-mini",
+    null,
+    null,
+    undefined,
+    /* isCombo */ false
+  );
+  assert.equal(forUnknownProvider.status, 503);
+});
+
+test("handleNoCredentials still leaks 404 (combo fall-through) only when combo", async () => {
+  // Regression guard: the 404 is intentionally preserved for combo routing so it
+  // can skip a disabled-credentials leg. Explicitly assert isCombo=true keeps 404
+  // and isCombo=false does not. (Issue #2)
+  const combo = handleNoCredentials(
+    null,
+    null,
+    "kiro",
+    "claude-opus-5",
+    null,
+    null,
+    undefined,
+    true
+  );
+  assert.equal(combo.status, 404);
+
+  const single = handleNoCredentials(
+    null,
+    null,
+    "byNara",
+    "claude-opus-5",
+    null,
+    null,
+    undefined,
+    false
+  );
+  assert.notEqual(single.status, 404);
+});
+
 test("handleNoCredentials returns Retry-After when every account is rate limited", async () => {
   const retryAfter = new Date(Date.now() + 45_000).toISOString();
   const response = handleNoCredentials(
@@ -506,7 +576,7 @@ test("executeChatWithBreaker preserves account TLS scope when a proxy bypasses t
           ],
           usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 },
         }),
-        { headers: { "content-type": "application/json" } },
+        { headers: { "content-type": "application/json" } }
       );
     },
   });

tests/unit/fix-error-message-candidates.test.ts

@@ -17,7 +17,8 @@ test("handleNoCredentials includes candidate aliases hint when supplied", async
     /* model */ "claude-opus-5",
     /* lastError */ null,
     /* lastStatus */ null,
-    /* candidateAliases */ ["anthropic", "claude", "agentrouter"]
+    /* candidateAliases */ ["anthropic", "claude", "agentrouter"],
+    /* isCombo */ true
   );
 
   assert.equal(res.status, 404);
@@ -42,8 +43,10 @@ test("handleNoCredentials omits hint when no candidates supplied", async () => {
     "kiro",
     "claude-opus-5",
     null,
-    null
+    null,
     /* no candidateAliases */
+    undefined,
+    /* isCombo */ true
   );
 
   assert.equal(res.status, 404);
@@ -65,14 +68,18 @@ test("handleNoCredentials trims candidate list to top 3", async () => {
     "claude-opus-5",
     null,
     null,
-    ["anthropic", "claude", "agentrouter", "github", "vertex-partner"]
+    ["anthropic", "claude", "agentrouter", "github", "vertex-partner"],
+    /* isCombo */ true
   );
 
   const body = (await res.json()) as { error?: { message?: string } };
   const message = body?.error?.message ?? "";
   // Top-3 (anthropic, claude, agentrouter) — github and vertex-partner are
   // dropped to keep the hint actionable.
-  assert.match(message, /Try one of: anthropic\/claude-opus-5, claude\/claude-opus-5, agentrouter\/claude-opus-5/);
+  assert.match(
+    message,
+    /Try one of: anthropic\/claude-opus-5, claude\/claude-opus-5, agentrouter\/claude-opus-5/
+  );
   assert.doesNotMatch(message, /github\/claude-opus-5/);
   assert.doesNotMatch(message, /vertex-partner\/claude-opus-5/);
-});
+});