fix(sse): combo failover for OpenAI streams truncated without finish_reason - #7568
Conversation
…reason (#7285) validateResponseQuality() only recognized Claude SSE lifecycle events (message_start/content_block_*/message_stop/message_delta.stop_reason). An OpenAI-shape stream (choices[].delta) that emits some bytes (e.g. a role-only delta) and then closes without ever carrying finish_reason (and without a data: [DONE] sentinel) fell through to the generic replay branch and was forwarded to the client as a success instead of triggering combo failover. Adds OpenAI-shape lifecycle tracking (hasChoicePayload/hasTerminalMarker) parallel to the existing Claude tracking: when an OpenAI-shape chunk was seen but the stream ends without finish_reason or [DONE], and no recognized content was found, mark the response invalid so combo failover retries a sibling target. Healthy OpenAI streams (finish_reason present, or real content found) are unaffected — they exit the peek loop before reaching this check, preserving the #3399/#3685 pass-through contract. Regression test: tests/unit/combo-streaming-openai-no-finish-reason-7285.test.ts
There was a problem hiding this comment.
Code Review
This pull request fixes issue #7285 by introducing tracking for OpenAI-shape SSE streams to detect when they are truncated without a finish_reason or [DONE] sentinel, allowing combo failover to trigger correctly. The changes include new lifecycle flags, helper functions, and comprehensive unit tests. The review feedback suggests adding defensive checks in the new helper functions to prevent potential runtime TypeError crashes when handling malformed or null payloads.
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.
| export function isOpenAIChoicesPayload(parsed: Record<string, unknown>): boolean { | ||
| return Array.isArray(parsed.choices); | ||
| } | ||
|
|
||
| export function hasOpenAIFinishReason(parsed: Record<string, unknown>): boolean { | ||
| if (!Array.isArray(parsed.choices)) return false; | ||
| return parsed.choices.some((choice) => isRecord(choice) && choice.finish_reason != null); | ||
| } |
There was a problem hiding this comment.
To prevent potential runtime TypeError crashes when parsing malformed or unexpected SSE payloads (such as data: null which parses to null), we should defensively check that parsed is a non-null object before accessing its properties.
| export function isOpenAIChoicesPayload(parsed: Record<string, unknown>): boolean { | |
| return Array.isArray(parsed.choices); | |
| } | |
| export function hasOpenAIFinishReason(parsed: Record<string, unknown>): boolean { | |
| if (!Array.isArray(parsed.choices)) return false; | |
| return parsed.choices.some((choice) => isRecord(choice) && choice.finish_reason != null); | |
| } | |
| export function isOpenAIChoicesPayload(parsed: Record<string, unknown>): boolean { | |
| return !!parsed && Array.isArray(parsed.choices); | |
| } | |
| export function hasOpenAIFinishReason(parsed: Record<string, unknown>): boolean { | |
| if (!parsed || !Array.isArray(parsed.choices)) return false; | |
| return parsed.choices.some((choice) => isRecord(choice) && choice.finish_reason != null); | |
| } |
…reason (diegosouzapw#7285) (diegosouzapw#7568) validateResponseQuality() only recognized Claude SSE lifecycle events (message_start/content_block_*/message_stop/message_delta.stop_reason). An OpenAI-shape stream (choices[].delta) that emits some bytes (e.g. a role-only delta) and then closes without ever carrying finish_reason (and without a data: [DONE] sentinel) fell through to the generic replay branch and was forwarded to the client as a success instead of triggering combo failover. Adds OpenAI-shape lifecycle tracking (hasChoicePayload/hasTerminalMarker) parallel to the existing Claude tracking: when an OpenAI-shape chunk was seen but the stream ends without finish_reason or [DONE], and no recognized content was found, mark the response invalid so combo failover retries a sibling target. Healthy OpenAI streams (finish_reason present, or real content found) are unaffected — they exit the peek loop before reaching this check, preserving the diegosouzapw#3399/diegosouzapw#3685 pass-through contract. Regression test: tests/unit/combo-streaming-openai-no-finish-reason-7285.test.ts
…reason (diegosouzapw#7285) (diegosouzapw#7568) validateResponseQuality() only recognized Claude SSE lifecycle events (message_start/content_block_*/message_stop/message_delta.stop_reason). An OpenAI-shape stream (choices[].delta) that emits some bytes (e.g. a role-only delta) and then closes without ever carrying finish_reason (and without a data: [DONE] sentinel) fell through to the generic replay branch and was forwarded to the client as a success instead of triggering combo failover. Adds OpenAI-shape lifecycle tracking (hasChoicePayload/hasTerminalMarker) parallel to the existing Claude tracking: when an OpenAI-shape chunk was seen but the stream ends without finish_reason or [DONE], and no recognized content was found, mark the response invalid so combo failover retries a sibling target. Healthy OpenAI streams (finish_reason present, or real content found) are unaffected — they exit the peek loop before reaching this check, preserving the diegosouzapw#3399/diegosouzapw#3685 pass-through contract. Regression test: tests/unit/combo-streaming-openai-no-finish-reason-7285.test.ts
Closes #7285
Root cause
validateResponseQuality()(open-sse/services/combo/validateQuality.ts::parseAccumulatedSse()) only recognizes Claude SSE lifecycle events (message_start,content_block_*,message_stop,message_delta.delta.stop_reason).hasOpenAICompatibleStreamValue()(open-sse/utils/streamHelpers.ts) only inspectsdelta.content/reasoning_content/reasoning_text/tool_calls— it never looks atchoice.finish_reason.Result: an OpenAI-shape upstream that returns HTTP 200, emits a role-only delta (or other non-content bytes), and then closes the connection without ever sending a chunk carrying
finish_reason(and withoutdata: [DONE]) falls through every existing failover branch and lands on the generic "replay buffered bytes" path —{valid: true, clonedResponse}. The truncated stream is forwarded to the client as a success instead of triggering combo failover to a sibling target.auto/*:freesuffixes amplify exposure because they narrow the candidate pool to free-tier providers, the population most likely to truncate mid-stream.Fix
Added OpenAI-shape lifecycle tracking (
hasChoicePayload/hasTerminalMarker) parallel to the existing Claude tracking invalidateQuality.ts:streamHelpers.ts:isOpenAIChoicesPayload()andhasOpenAIFinishReason().parseAccumulatedSse()now markshasChoicePayloadon anychoices[]chunk, andhasTerminalMarkerwhen a choice carriesfinish_reason != nullor thedata: [DONE]sentinel appears.{valid: false}) so combo failover retries a sibling target.Healthy OpenAI streams are unaffected: a stream carrying real content (or
finish_reason) exits the bounded peek loop early via the existingfoundContentbranch, before ever reaching the new check — preserving the #3399/#3685 pass-through contract (a slow-but-healthy stream must not be misclassified as truncated).STREAM_RECOVERY_ENABLEDstays untouched (defaultfalse) — this fix lives entirely invalidateResponseQuality(), independent ofstreamRecovery.ts's retry path.TDD proof (Hard Rule #18)
New regression test:
tests/unit/combo-streaming-openai-no-finish-reason-7285.test.tsRED (before fix):
GREEN (after fix):
Gates run (all green)
node --import tsx/esm --test tests/unit/combo-streaming-openai-no-finish-reason-7285.test.ts— 2/2 passtests/unit/combo-quality-validator-reasoning.test.ts(12/12)tests/unit/combo-round-robin-streaming-lock-3811.test.ts(1/1)tests/unit/combo-stream-readiness-fallback.test.ts(11/11)tests/unit/combo-streaming-empty-content-failover.test.ts(6/6, Streaming combo returns "[Proxy Error] The upstream API returned an empty response" instead of failing over #3685 contract)tests/unit/empty-response-hardening.test.ts(12/12)tests/unit/masked-200-exhaustion-fallback-6427.test.ts(3/3)tests/unit/streamHelpers.test.ts(20/20)tests/unit/streaming-empty-content-block-1382.test.ts(2/2)tests/unit/validate-response-quality.test.ts(8/8)tests/unit/gemini-cli-ansi-sanitization.test.ts(5/5)tests/unit/correctness/sse-parser.property.test.ts(4/4)node scripts/check/check-file-size.mjs— OK (no growth on frozen files, new test file untouched by cap)node scripts/check/check-complexity.mjs— OK (2054 ≤ baseline 2056)node scripts/check/check-cognitive-complexity.mjs— OK (889 ≤ baseline 890)npm run typecheck:core— clean, exit 0npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>— clean, exit 0npm run typecheck:noimplicit:coresurfaces pre-existing implicit-anyerrors in unrelated files (open-sse/services/combo.ts,open-sse/utils/usageTracking.ts,src/shared/services/cliRuntime.ts) — none in the files this PR touches; not introduced by this change.Scope
Diff strictly scoped to the issue:
open-sse/services/combo/validateQuality.ts,open-sse/utils/streamHelpers.ts, the new regression test, and a changelog fragment. No drive-by refactors.