From 477dd280f254042c2a46bc187d9b90a123baaa22 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Sun, 21 Jun 2026 08:04:24 -0300 Subject: [PATCH] refactor(chatCore): extrai recordKeyHealthStatus para leaf dedicado (#3501) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move o closure `recordKeyHealthStatus` (~56 ln) do topo de handleChatCore para o novo leaf open-sse/handlers/chatCore/keyHealth.ts, byte-idêntico: 401 → recordKeyFailure + persist sempre; 2xx → recordKeySuccess + persist só na recuperação de warning/invalid; demais status apenas atualizam o set de extra-keys. O handler mantém um closure fino de binding que repassa `log`, então os 2 call sites ficam inalterados. Imports que ficaram órfãos (recordKeyFailure / recordKeySuccess / trackConnectionExtraKeys / KeyHealth) migram para o leaf; connectionHasExtraKeys permanece (ainda usado no site de rotação) e updateProviderConnection é importado em ambos (usado em vários pontos do handler). chatCore.ts 5110->5055 (shrink -55); baseline file-size ratchetado. complexity 1905=1905 (neutro). Coberto por tests/unit/chatcore-key-health.test.ts (6 casos — transições in-memory do apiKeyRotator: warning/invalid no threshold, recuperação 2xx, escopo por selectedKeyId, no-op sem connectionId e em status fora de 401/2xx). --- open-sse/handlers/chatCore.ts | 65 ++----------------- open-sse/handlers/chatCore/keyHealth.ts | 84 +++++++++++++++++++++++++ tests/unit/chatcore-key-health.test.ts | 70 +++++++++++++++++++++ 3 files changed, 159 insertions(+), 60 deletions(-) create mode 100644 open-sse/handlers/chatCore/keyHealth.ts create mode 100644 tests/unit/chatcore-key-health.test.ts diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index dc869e87f3..c03a133de6 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -120,13 +120,8 @@ import { } from "../services/errorClassifier.ts"; import { updateProviderConnection, getProviderConnectionById } from "@/lib/db/providers"; import { wasRefreshTokenRotated } from "@omniroute/open-sse/services/refreshSerializer.ts"; -import { - recordKeyFailure, - recordKeySuccess, - trackConnectionExtraKeys, - connectionHasExtraKeys, - type KeyHealth, -} from "../services/apiKeyRotator.ts"; +import { connectionHasExtraKeys } from "../services/apiKeyRotator.ts"; +import { recordKeyHealthStatus as recordKeyHealthStatusFor } from "./chatCore/keyHealth.ts"; import { getCallLogPipelineCaptureStreamChunks, @@ -765,62 +760,12 @@ export async function handleChatCore({ }).catch(() => {}); }; + // Key-health updater extracted to chatCore/keyHealth.ts (#3501); bind the per-request log once + // and delegate so the existing call sites stay byte-identical. const recordKeyHealthStatus = ( status: number, creds: Record | null | undefined - ): void => { - const connId = creds?.connectionId as string | undefined; - if (!connId) return; - - const psd = creds.providerSpecificData as Record | undefined; - const extraKeys = (psd?.extraApiKeys as string[] | undefined) ?? []; - const health = psd?.apiKeyHealth as Record | undefined; - const currentKeyId = (psd?.selectedKeyId as string | undefined) ?? "primary"; - - trackConnectionExtraKeys(connId, extraKeys); - - if (status === 401) { - const updatedHealth = recordKeyFailure(connId, currentKeyId); - log?.warn?.( - "AUTH", - `401 on connection ${connId.slice(0, 8)} - key marked as failed (failure #${updatedHealth.failures})` - ); - - // Persist health status to DB on every failure (not just invalid transitions) - // This ensures in-memory state survives process restarts - const prevStatus = health?.[currentKeyId]?.status; - const prevFailures = health?.[currentKeyId]?.failures ?? 0; - if (updatedHealth.status !== prevStatus || updatedHealth.failures !== prevFailures) { - updateProviderConnection(connId, { - providerSpecificData: { - ...psd, - apiKeyHealth: { ...health, [currentKeyId]: updatedHealth }, - }, - }).catch((err: unknown) => { - log?.error?.( - "DB", - `Failed to persist apiKeyHealth: ${err instanceof Error ? err.message : String(err)}` - ); - }); - } - } else if (status >= 200 && status < 300) { - const updatedHealth = recordKeySuccess(connId, currentKeyId); - const prevStatus = health?.[currentKeyId]?.status; - if (prevStatus === "warning" || prevStatus === "invalid") { - updateProviderConnection(connId, { - providerSpecificData: { - ...psd, - apiKeyHealth: { ...health, [currentKeyId]: updatedHealth }, - }, - }).catch((err: unknown) => { - log?.error?.( - "DB", - `Failed to persist apiKeyHealth: ${err instanceof Error ? err.message : String(err)}` - ); - }); - } - } - }; + ): void => recordKeyHealthStatusFor(status, creds, log); const persistCodexQuotaState = async (headers: Record | null, status = 0) => { if (provider !== "codex" || !connectionId || !headers) return; diff --git a/open-sse/handlers/chatCore/keyHealth.ts b/open-sse/handlers/chatCore/keyHealth.ts new file mode 100644 index 0000000000..91e7c63618 --- /dev/null +++ b/open-sse/handlers/chatCore/keyHealth.ts @@ -0,0 +1,84 @@ +/** + * chatCore per-request API-key health updater (Quality Gate v2 / Fase 9 — chatCore god-file + * decomposition, #3501). + * + * Byte-identical extraction of the `recordKeyHealthStatus` closure that lived at the top of + * handleChatCore. Translates an upstream HTTP status into the in-memory key-health state + * (apiKeyRotator) for the connection's currently-selected key, and persists the change to the + * provider connection so it survives process restarts: + * - 401 → record a failure (warning, then invalid at the threshold), always persisted. + * - 2xx → record a success, persisted only when recovering from a warning/invalid state. + * Any other status only refreshes the tracked extra-key set. The handler binds its `log` once and + * delegates here, keeping the existing call sites unchanged. + */ + +import { + recordKeyFailure, + recordKeySuccess, + trackConnectionExtraKeys, + type KeyHealth, +} from "../../services/apiKeyRotator.ts"; +import { updateProviderConnection } from "@/lib/db/providers"; + +type KeyHealthLog = { + warn?: (tag: string, message: string) => void; + error?: (tag: string, message: string) => void; +} | null; + +export function recordKeyHealthStatus( + status: number, + creds: Record | null | undefined, + log?: KeyHealthLog +): void { + const connId = creds?.connectionId as string | undefined; + if (!connId) return; + + const psd = creds.providerSpecificData as Record | undefined; + const extraKeys = (psd?.extraApiKeys as string[] | undefined) ?? []; + const health = psd?.apiKeyHealth as Record | undefined; + const currentKeyId = (psd?.selectedKeyId as string | undefined) ?? "primary"; + + trackConnectionExtraKeys(connId, extraKeys); + + if (status === 401) { + const updatedHealth = recordKeyFailure(connId, currentKeyId); + log?.warn?.( + "AUTH", + `401 on connection ${connId.slice(0, 8)} - key marked as failed (failure #${updatedHealth.failures})` + ); + + // Persist health status to DB on every failure (not just invalid transitions) + // This ensures in-memory state survives process restarts + const prevStatus = health?.[currentKeyId]?.status; + const prevFailures = health?.[currentKeyId]?.failures ?? 0; + if (updatedHealth.status !== prevStatus || updatedHealth.failures !== prevFailures) { + updateProviderConnection(connId, { + providerSpecificData: { + ...psd, + apiKeyHealth: { ...health, [currentKeyId]: updatedHealth }, + }, + }).catch((err: unknown) => { + log?.error?.( + "DB", + `Failed to persist apiKeyHealth: ${err instanceof Error ? err.message : String(err)}` + ); + }); + } + } else if (status >= 200 && status < 300) { + const updatedHealth = recordKeySuccess(connId, currentKeyId); + const prevStatus = health?.[currentKeyId]?.status; + if (prevStatus === "warning" || prevStatus === "invalid") { + updateProviderConnection(connId, { + providerSpecificData: { + ...psd, + apiKeyHealth: { ...health, [currentKeyId]: updatedHealth }, + }, + }).catch((err: unknown) => { + log?.error?.( + "DB", + `Failed to persist apiKeyHealth: ${err instanceof Error ? err.message : String(err)}` + ); + }); + } + } +} diff --git a/tests/unit/chatcore-key-health.test.ts b/tests/unit/chatcore-key-health.test.ts new file mode 100644 index 0000000000..28e3e2c8c8 --- /dev/null +++ b/tests/unit/chatcore-key-health.test.ts @@ -0,0 +1,70 @@ +// tests/unit/chatcore-key-health.test.ts +// Characterization of recordKeyHealthStatus — the per-request API-key health updater extracted +// from handleChatCore (chatCore god-file decomposition, #3501). Locks the observable in-memory +// transitions driven through apiKeyRotator: 401 → failure (warning, then invalid at the threshold), +// 2xx → success/recovery, selectedKeyId scoping, and the no-op paths (missing connectionId, and +// non-401/non-2xx statuses). The DB persistence side effect (updateProviderConnection) is moved +// byte-identically and is fire-and-forget; these tests assert the synchronous health mutations. +import { test, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { recordKeyHealthStatus } from "../../open-sse/handlers/chatCore/keyHealth.ts"; +import { getAllKeyHealth, removeConnectionHealth } from "../../open-sse/services/apiKeyRotator.ts"; + +const noopLog = { warn: () => {}, error: () => {} }; +const touched: string[] = []; + +function creds(connectionId: string, psd: Record = {}) { + touched.push(connectionId); + return { connectionId, providerSpecificData: psd }; +} + +afterEach(() => { + for (const c of touched.splice(0)) removeConnectionHealth(c); +}); + +test("missing connectionId is a no-op (no health entry created)", () => { + const before = Object.keys(getAllKeyHealth()).length; + const r = recordKeyHealthStatus(200, { providerSpecificData: {} }, noopLog); + assert.equal(r, undefined); + assert.equal(Object.keys(getAllKeyHealth()).length, before); +}); + +test("401 marks the selected key as failed → warning after the first failure", () => { + const conn = "kh-401-warning"; + recordKeyHealthStatus(401, creds(conn), noopLog); + const h = getAllKeyHealth()[`${conn}:primary`]; + assert.equal(h?.failures, 1); + assert.equal(h?.status, "warning"); +}); + +test("401 reaches invalid at the failure threshold (2 consecutive)", () => { + const conn = "kh-401-invalid"; + recordKeyHealthStatus(401, creds(conn), noopLog); + recordKeyHealthStatus(401, creds(conn), noopLog); + const h = getAllKeyHealth()[`${conn}:primary`]; + assert.equal(h?.failures, 2); + assert.equal(h?.status, "invalid"); +}); + +test("2xx after a failure resets the key to active with 0 failures", () => { + const conn = "kh-2xx-recover"; + recordKeyHealthStatus(401, creds(conn), noopLog); + recordKeyHealthStatus(204, creds(conn), noopLog); + const h = getAllKeyHealth()[`${conn}:primary`]; + assert.equal(h?.failures, 0); + assert.equal(h?.status, "active"); +}); + +test("honors selectedKeyId — scopes the update to the active extra key, not primary", () => { + const conn = "kh-selected-key"; + recordKeyHealthStatus(401, creds(conn, { selectedKeyId: "extra_1" }), noopLog); + const all = getAllKeyHealth(); + assert.equal(all[`${conn}:extra_1`]?.status, "warning"); + assert.equal(all[`${conn}:primary`], undefined); +}); + +test("non-401 / non-2xx status does not touch key health", () => { + const conn = "kh-5xx-noop"; + recordKeyHealthStatus(500, creds(conn), noopLog); + assert.equal(getAllKeyHealth()[`${conn}:primary`], undefined); +});