Repository navigation
refactor(combo): god-file split pilot — types + validateQuality + predicates (QG v2 Fase 9 T5 D1-D3) - #4162
Conversation
|
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 the large combo.ts file by extracting types, predicates, and validation logic into dedicated files under open-sse/services/combo/ (types.ts, comboPredicates.ts, and validateQuality.ts) while preserving backward compatibility via re-exports. The review feedback identifies two potential runtime TypeError vulnerabilities: one during SSE data parsing in validateQuality.ts if the parsed JSON is not an object, and another in comboPredicates.ts if isProviderCircuitOpenResult is invoked with a nullish result argument.
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.
| let parsed: Record<string, unknown>; | ||
| try { | ||
| parsed = JSON.parse(data); | ||
| } catch { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
When parsing the SSE data payload, JSON.parse(data) can return null or primitive values (such as booleans or numbers) if the payload is malformed or contains unexpected content. Accessing properties like parsed.type or passing parsed directly to isKnownNonClaudeStreamPayload without a non-null object check can lead to runtime TypeError exceptions. Adding a defensive check to ensure parsed is a non-null object prevents potential crashes during stream peeking.
| let parsed: Record<string, unknown>; | |
| try { | |
| parsed = JSON.parse(data); | |
| } catch { | |
| continue; | |
| } | |
| let parsed: Record<string, unknown>; | |
| try { | |
| const val = JSON.parse(data); | |
| if (!val || typeof val !== "object") continue; | |
| parsed = val as Record<string, unknown>; | |
| } catch { | |
| continue; | |
| } |
| export function isProviderCircuitOpenResult( | ||
| result: { headers?: Headers | null; status?: number }, | ||
| errorText: string | ||
| ): boolean { | ||
| const breakerHeader = result.headers?.get?.("x-omniroute-provider-breaker"); |
There was a problem hiding this comment.
The isProviderCircuitOpenResult function accepts a result object, but does not check if result itself is null or undefined before accessing result.headers. To prevent potential runtime TypeError exceptions if the function is called with a nullish value (e.g., in test environments or edge cases), use optional chaining on result as well.
| export function isProviderCircuitOpenResult( | |
| result: { headers?: Headers | null; status?: number }, | |
| errorText: string | |
| ): boolean { | |
| const breakerHeader = result.headers?.get?.("x-omniroute-provider-breaker"); | |
| export function isProviderCircuitOpenResult( | |
| result: { headers?: Headers | null; status?: number } | null | undefined, | |
| errorText: string | |
| ): boolean { | |
| const breakerHeader = result?.headers?.get?.("x-omniroute-provider-breaker"); |
…dicates (QG v2 Fase 9 T5 D1-D3) (cheaper-inference#4162) Integrated into release/v3.8.29
Contexto (QG v2 Fase 9 — Onda 3 Task 5, PILOTO)
Primeiro de ~15 PRs do split dos god-files. Prova o padrão extract + re-export byte-idêntico em
open-sse/services/combo.ts(5297 LOC). 3 extrações de risco BAIXO, 1 commit cada. Plano:docs/superpowers/plans/2026-06-18-godfiles-split.md(local).O que mudou
combo/types.tscombo/validateQuality.tsvalidateResponseQuality+toRetryAfterDisplayValuecombo/comboPredicates.tscombo.ts: 5297 → 4739 LOC. Cada símbolo originalmente público segue exportado via re-export barrel (export { ... } from "./combo/..."); internos importados de volta onde combo.ts os usa. Nenhum consumidor externo muda.Validação (auditada pelo controlador, trust-but-verify)
combo.ts= só remoções + re-exports/imports (zero lógica editada);validateResponseQualitye os 11 predicados diffados função-a-função vs origem = IDÊNTICOS.getTargetProviderinterno importado de volta).ERR_FS_CP_EINVALlocal é artefato do node_modules symlinkado da worktree, na cópia de native-assets pós-compile — não afeta CI).combo.ts(5298→4740;+1= trailing-newline do gate). Drift pré-existente de 22 arquivos NÃO tocado (escopo separado).🤖 Generated with Claude Code