refactor(chatCore): extrai recordKeyHealthStatus para leaf dedicado (#3501) - #4491
diegosouzapw wants to merge 1 commit into
Conversation
…3501) 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).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Code Review
This pull request refactors open-sse/handlers/chatCore.ts by extracting the recordKeyHealthStatus logic into a new dedicated module open-sse/handlers/chatCore/keyHealth.ts and introducing corresponding unit tests. The reviewer suggests mutating the in-memory creds.providerSpecificData object in-place before calling updateProviderConnection to ensure in-memory consistency and prevent stale health status reads during retries or subsequent executions within the same request lifecycle.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // 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)}` | ||
| ); | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation updates the database with the new apiKeyHealth but does not mutate the in-memory creds.providerSpecificData object in-place. This can lead to stale health status reads during retries or subsequent executions within the same request lifecycle if they share the same credentials reference. Mutating psd.apiKeyHealth in-place before calling updateProviderConnection ensures in-memory consistency.
// 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) {
if (psd) {
if (!psd.apiKeyHealth) psd.apiKeyHealth = {};
(psd.apiKeyHealth as Record<string, KeyHealth>)[currentKeyId] = updatedHealth;
}
updateProviderConnection(connId, {
providerSpecificData: {
...psd,
},
}).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") {
if (psd) {
if (!psd.apiKeyHealth) psd.apiKeyHealth = {};
(psd.apiKeyHealth as Record<string, KeyHealth>)[currentKeyId] = updatedHealth;
}
updateProviderConnection(connId, {
providerSpecificData: {
...psd,
},
}).catch((err: unknown) => {
log?.error?.(
"DB",
`Failed to persist apiKeyHealth: ${err instanceof Error ? err.message : String(err)}`
);
});
}
}Bumps [hono](https://github.com/honojs/hono) from 4.13.0 to 4.13.7. Release notes: <details><summary>Sourced from hono's releases.</summary> v4.13.7 — fix(hono-base): use raw value for x-forwarded-proto in getRequestProtocol (diegosouzapw#4491) v4.13.6 — fix(jsx): prevent XSS via JSX children attribute (diegosouzapw#4478) v4.13.5 — fix(deno): RPC type regression v4.13.4 — fix(router): RegExpRouter static match regression v4.13.3 — fix(client): form data serialization v4.13.2 — fix(jsx): hydration mismatch on self-closing tags v4.13.1 — fix(context): cookie helper set-cookie ordering v4.13.0 — feature: hono/jsx streaming SSR improvements Changelog (relevant to this repo): - **4.13.6** fixes an XSS in hono/jsx where JSX children attribute values were not properly escaped — security fix, primary motivation for this bump - **4.13.7** fixes x-forwarded-proto handling in getRequestProtocol — affects the getRequestProtocol() helper used by middleware Single-file dependency bump, no schema/permission impact. Fixes diegosouzapw#13148
Bumps [hono](https://github.com/honojs/hono) from 4.13.0 to 4.13.7. Release notes: <details><summary>Sourced from hono's releases.</summary> v4.13.7 — fix(hono-base): use raw value for x-forwarded-proto in getRequestProtocol (diegosouzapw#4491) v4.13.6 — fix(jsx): prevent XSS via JSX children attribute (diegosouzapw#4478) v4.13.5 — fix(deno): RPC type regression v4.13.4 — fix(router): RegExpRouter static match regression v4.13.3 — fix(client): form data serialization v4.13.2 — fix(jsx): hydration mismatch on self-closing tags v4.13.1 — fix(context): cookie helper set-cookie ordering v4.13.0 — feature: hono/jsx streaming SSR improvements Changelog (relevant to this repo): - **4.13.6** fixes an XSS in hono/jsx where JSX children attribute values were not properly escaped — security fix, primary motivation for this bump - **4.13.7** fixes x-forwarded-proto handling in getRequestProtocol — affects the getRequestProtocol() helper used by middleware Single-file dependency bump, no schema/permission impact. Fixes diegosouzapw#13148
O quê
Próximo incremento da decomposição do god-file
chatCore.ts(QG v2 Fase 9 T5, #3501). Extrai o closurerecordKeyHealthStatus(~56 ln) do topo dehandleChatCorepara o novo leafopen-sse/handlers/chatCore/keyHealth.ts.Como (preservação de comportamento)
401→recordKeyFailure+ persist sempre;2xx→recordKeySuccess+ persist só na recuperação dewarning/invalid; demais status apenas atualizam o set de extra-keys (trackConnectionExtraKeys).log, então os 2 call sites (dispatch a ~3063/3338) ficam inalterados.recordKeyFailure/recordKeySuccess/trackConnectionExtraKeys/KeyHealth) migram para o leaf.connectionHasExtraKeyspermanece no handler (ainda usado no site de rotação de chave);updateProviderConnectioné importado em ambos (usado em vários pontos do handler).Gates
chatCore.ts5110→5055 (shrink −55); baseline ratchetado.tests/unit/chatcore-key-health.test.ts— 6 casos de caracterização das transições in-memory doapiKeyRotator(warning na 1ª falha,invalidno threshold=2, recuperação em 2xx, escopo porselectedKeyId, no-op semconnectionIde em status fora de 401/2xx).chatcore-imports-cleanly✓ (sem imports órfãos) + 138/138 na suíte chatcore (excluído apenas ochatCore times out upstream execution, red flaky pré-existente comprovado no refactor(chatCore): extrai resolvers de service-tier do Codex para leaf puro (#3501) #4477).Extração de god-file sem mudança de comportamento em runtime.