fix: fail over zero-output incomplete combo streams

[RHODIZSECURITY]

Problem

Combo stream preflight currently treats a zero-output response.incomplete terminal as an accepted target. That means an upstream can return HTTP 200, terminate with a transport-level incomplete such as adapter_eof, and prevent the combo from advancing to a healthy backup target.

This was reproducible on the Claude-compatible streaming path: the selected target returned an HTTP 200 SSE stream that ended as response.incomplete with incomplete_details.reason = "adapter_eof"; the combo had remaining targets but did not fail over.

Fix

Treat only zero-output transport/infrastructure incompletes as retryable combo failures:

• adapter_eof
• missing_terminal_event
• upstream_stall_timeout

The existing output-commit boundary remains authoritative: once visible output or a tool-side effect has committed the target, the request is never replayed on another target.

Semantic incompletes such as max_output_tokens and content_filter remain accepted and are not retried, because another provider cannot safely or correctly reinterpret those outcomes.

Regression coverage

Added focused unit coverage for:

• zero-output transport incompletes -> retryable failure
• semantic incomplete -> no replay
• transport incomplete after output commit -> no replay

Added server-level combo failover coverage where target A ends with adapter_eof before output and target B completes successfully. The request/usage receipts verify A as a 502 attempt followed by B as the 200 winner.

Validation

• bun test tests/combo-stream-preflight.test.ts tests/server-combo-failover-e2e.test.ts -> 88 pass, 0 fail
• bun run typecheck -> PASS
• bun run privacy:scan -> PASS
• GUI lint/doctor if-changed -> correctly skipped (no gui/ changes)
• bun run test -> exit 0
  • main parallel phase: 17,224 pass, 16 platform skips, 0 fail across 1,024 files
  • all subsequent serialized sub-gates also exited 0
• git diff --check -> PASS

Base: current dev at the time of implementation (d23eab43aa49).

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

• All CI tests are green on my local testing.

• I pushed my PR to the latest dev commit.

• I resolved all correct Codex and CodeRabbit findings.

• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes
  • Improved streaming recovery when a response ends prematurely before producing output.
  • Automatically retries eligible transport interruptions, including adapter disconnects, missing terminal events, and upstream timeouts.
  • Preserves meaningful partial responses and semantic limits without unnecessary retries.
  • Enables failover to the next available stream target when the initial attempt produces no output.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[github-actions]

⏳ DRAFT

• review readiness checklist open (0/4 boxes ticked).

What to do

• Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

• ⬜ All CI tests are green on my local testing.
• ⬜ I pushed my PR to the latest dev commit.
• ⬜ I resolved all correct Codex and CodeRabbit findings.
• ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 7aeffdc6-7c09-4389-8777-dde82f842cde

📥 Commits

Reviewing files that changed from the base of the PR and between d23eab4 and 43d2338.

📒 Files selected for processing (3)

• src/server/responses/combo-stream-preflight.ts
• tests/combo-stream-preflight.test.ts
• tests/server-combo-failover-e2e.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

Zero-output stream failover

Layer / File(s)	Summary
Classify and replay retryable terminals src/server/responses/combo-stream-preflight.ts, tests/combo-stream-preflight.test.ts	The preflight logic classifies adapter_eof, missing_terminal_event, and upstream_stall_timeout as retryable for zero-output response.incomplete events. It preserves semantic and post-output incompletes. Tests cover status, error payloads, token usage, and replay behavior.
Validate backup target failover tests/server-combo-failover-e2e.test.ts	The end-to-end test verifies that an adapter EOF without output produces a 502 attempt for target a, then a 200 response from target b, with no firstOutputMs on the failed attempt.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 43d23

The PR retries only zero-output transport failures against configured backup targets while preserving no-replay behavior after output or tool effects; no actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant UpstreamStream
  participant ComboStreamPreflight
  participant ComboFailover
  participant BackupStream
  UpstreamStream->>ComboStreamPreflight: Emit zero-output response.incomplete
  ComboStreamPreflight->>ComboStreamPreflight: Classify transport reason
  ComboStreamPreflight->>ComboFailover: Return retryable 502
  ComboFailover->>BackupStream: Start target b
  BackupStream-->>ComboFailover: Return successful stream

Loading

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: failover for zero-output incomplete combo streams.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lidge-jun]

리뷰 · 우선순위 49 / 80

이 PR은 초안(draft)이고, 체크리스트 네 칸이 아직 비어 있습니다. 베이스는 지금 dev HEAD d23eab43a라고 본문에 적혀 있습니다. 바꾸는 파일은 src/server/responses/combo-stream-preflight.ts와 테스트 두 개뿐입니다. 범위가 좁고, types.ts / config.ts 분할과 겹치지 않습니다.

지금 dev의 콤보 스트림 프리플라이트는 response.failed이면서 아직 출력(또는 툴 부작용)이 없는 경우만 다른 타깃으로 다시 보냅니다. response.incomplete로 끝나는 스트림은, 출력이 없어도 “받은 것으로 인정”합니다. 그래서 업스트림이 HTTP 200 SSE를 주다가 incomplete_details.reason = adapter_eof처럼 전송/인프라 이유로 끊으면, 콤보에 백업 타깃이 남아 있어도 넘어가지 않습니다. 본문이 말한 Claude 호환 스트리밍 경로의 재현과 맞습니다.

이 PR이 하는 일은 짧습니다. RETRYABLE_ZERO_OUTPUT_INCOMPLETE_REASONS에 adapter_eof, missing_terminal_event, upstream_stall_timeout 세 개를 넣고, retryableZeroOutputTerminal()이 response.failed이거나 위 이유의 response.incomplete일 때 참이 됩니다. 프리플라이트는 terminalStatus === failed || incomplete이고 출력이 아직 없으면, 예전처럼 failedTerminalResponse로 502 실패 봉투를 만들어 콤보가 다음 타깃으로 넘어가게 합니다. max_output_tokens / content_filter 같은 의미 있는 incomplete는 그대로 수락합니다. 출력이 한 번이라도 커밋된 뒤에는 전송 incomplete여도 다시 보내지 않습니다. 출력 커밋 경계는 기존과 같습니다.

테스트는 단위에서 세 갈래(재시도 가능 incomplete → 502, 의미 incomplete → 수락, 출력 후 incomplete → 수락)를 잠갔고, tests/server-combo-failover-e2e.test.ts에 타깃 A가 출력 없이 adapter_eof로 끝나고 B가 이기는 경로를 넣었습니다. 본문 검증(88 집중 / 전체 스위트 통과)도 적혀 있습니다. 초안이라도 변경 자체는 읽기 쉽고, 중복 전송을 막는 경계를 깨지 않으려는 의도가 분명합니다.

라인 retryableZeroOutputTerminal / failedTerminalResponse - incomplete 이벤트에는 보통 response.error가 없습니다. 그래서 실패 봉투의 error.message는 logCtx.upstreamError에 의존합니다. 단위 테스트가 이유별 문구(Upstream stream ended unexpectedly... 등)를 기대하므로, 인스펙터가 incomplete reason을 logCtx에 넣는 기존 경로가 있어야 합니다. 그 연결이 깨지면 콤보 분류는 되어도 메시지·영수증이 빈약해집니다. 리뷰/CI에서 incomplete → upstreamError 매핑이 실제로 채워지는지 한 번 더 확인하세요.
심볼 RETRYABLE_ZERO_OUTPUT_INCOMPLETE_REASONS - 세 이유만 허용 목록으로 둔 선택은 안전합니다. 다만 업스트림/어댑터가 새 전송 이유를 추가하면 다시 수락 쪽으로 떨어집니다. 주석에 “허용 목록이며 새 전송 이유는 여기만 추가”라고 한 줄 적어 두면 후속자가 덜 헷갈립니다.
경로 초안 체크리스트 - CI·최신 dev·Codex/CodeRabbit·ready 네 칸이 비어 있습니다. 내용이 좋아도 레디 표시 전에는 머지 대기열에 넣지 않는 편이 맞습니다.
심볼 chatTruncatedZeroOutputStream (e2e) - Chat 프레임만 끊는 헬퍼가 실제 어댑터의 adapter_eof incomplete 변환과 같은 경로를 타는지 확인이 필요합니다. 단위 테스트가 Responses incomplete를 직접 넣는 것과, e2e가 Chat 절단 → 어댑터 incomplete로 가는 것이 한 줄로 연결돼 있어야 “실경로 회귀” 주장이 성립합니다.
중복/분할 - 비슷한 콤보 failover PR이 열려 있지 않고, 설정 스키마도 안 건드립니다. 닫고 다시 쌓을 대상이 아닙니다.

메인테이너의 판단이 필요한 지점

• 초안을 ready로 올린 뒤 바로 독립 버그픽스로 넣을지, 비슷한 responses 정리 PR과 묶을지.
• 허용 목록에 없는 새 전송 incomplete를 기본 수락(현재)으로 둘지, 나중에 “알 수 없는 incomplete + zero output”도 재시도할지. 지금은 허용 목록이 더 안전합니다.
• incomplete를 502 실패 봉투로 바꿀 때 클라이언트/영수증에 남는 메시지가 충분한지, 아니면 reason 코드를 명시 필드로 남길지.
• types/config 분할과 무관합니다. 이 PR을 분할 때문에 닫을 이유는 없습니다.

너의 추천
방향은 맞습니다. 출력 없는 전송 incomplete만 콤보 실패로 바꾸고, 의미 incomplete와 출력 커밋 뒤 incomplete는 그대로 두는 경계가 분명합니다. 다음 단계: (1) 체크리스트를 채우고 ready로 전환하고, (2) incomplete reason → logCtx.upstreamError 매핑이 e2e/단위에서 실제로 채워지는지 확인한 뒤, (3) 허용 목록 주석 한 줄을 보태세요. 그다음 dev에 독립 버그픽스로 넣으면 됩니다. 초안 상태에서는 머지하지 마세요.

이 댓글은 grok-bot이 작성했습니다

[lidge-jun]

Landed via maintainer as #3236 → 1c8278b4d on dev, your commit cherry-picked and credited as-is. Thanks — the reason allowlist matches exactly the three transport faults the proxy mints itself (bridge.ts, relay.ts, responses-terminal-repair.ts), and the e2e with receipts is the kind of coverage this path needed.

Closing this PR as superseded by the carry.