refactor(openai-shim): extract stream lifecycle and response dispatch

[jatmn]

Summary

Extracts client response dispatch into openaiShim/clientDispatch.ts.

The module owns stream wrapper cancellation, response-URL routing, Anthropic/Gemini/OpenAI stream selection, non-streaming dispatch, withResponse, and invalid-content rejection. The façade supplies converters and request metadata.

Rebase

Rebased and conflict-resolved onto current main (10a9190bea37bf8574bdb1b040180bf3b84ba2b3), retaining the intentional de-monolithing. Concurrent extractions already in main were preserved; this PR remains focused on client dispatch.

Current head: aee66a93ba2843b2a4eb6dba9e638d36c963cab1.

Source-file budget

openaiShim.ts: +21 / -212, below 1,500 changed lines; tests excluded.

Test migration

clientDispatch.test.ts: 236 lines / 12 focused cases. The focused dispatch suite and retained façade suite cover routing, cancellation, non-streaming conversion, withResponse, and unexpected-content rejection.

Validation

• bun test ./src/services/api/openaiShim/clientDispatch.test.ts ./src/services/api/openaiShim.test.ts — 270 passed / 754 assertions
• bun run typecheck
• bun run build
• git diff --check

Prepared according to CONTRIBUTING.md and AGENTS.md.

Summary by CodeRabbit

• Improvements
  • Improved compatibility across different AI providers and transport types.
  • Enhanced streaming behavior, including more reliable abort/cancellation handling and early-exit cleanup.
  • Standardized conversion of both streaming and non-streaming provider responses into a consistent output format.
  • Preserved request identifiers and improved access to the original HTTP response payload.
  • Added clearer error messages when a provider returns an unexpected content type.
• Tests
  • Added a dedicated test suite for the dispatch/shim behavior (streaming, non-streaming, headers, and error handling).

[coderabbitai]

📝 Walkthrough

Walkthrough

The OpenAI shim now delegates request dispatch, stream cancellation, response conversion, error handling, and response metadata to a new clientDispatch module. Tests cover routing headers, cancellation, provider conversions, response attachment, and unexpected content types.

Changes

OpenAI shim dispatch

Layer / File(s)	Summary
Dispatch contracts and stream lifecycle src/services/api/openaiShim/clientDispatch.ts, src/services/api/openaiShim/clientDispatch.test.ts	Adds shared dependency and promise contracts, request URL header cloning, cancellable stream behavior, and tests for cancellation and header handling.
Request routing and shim integration src/services/api/openaiShim/clientDispatch.ts, src/services/api/openaiShim.ts	Centralizes provider request execution, transport-specific streaming and JSON conversion, content-type errors, .withResponse() metadata, and delegates OpenAIShimMessages.create(...) to the dispatcher.
Dispatch conversion and response validation src/services/api/openaiShim/clientDispatch.test.ts	Tests streaming converter selection, Anthropic passthrough, Gemini/OpenAI/Codex conversion, collector usage, response metadata, and unexpected content-type errors.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

• Gitlawb/openclaude#2007: Refactors related OpenAI shim response conversion paths used by this dispatcher.
• Gitlawb/openclaude#2008: Restructures related streaming conversion paths used by dispatch.
• Gitlawb/openclaude#1828: Implements related shim stream abort propagation semantics.

Suggested reviewers: kevincodex1, techbrewboss, vasanthdev2004

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Risk Surface Disclosed	⚠️ Warning	Provider-routing/outbound HTTP dispatch changed, but no review/PR note explicitly calls out that risk surface or says whether it’s blocking.	Add a review note or PR description section naming the provider-routing/outbound-network risk and explicitly marking it blocking or non-blocking.

✅ Passed checks (5 passed)

Check name	Status	Explanation
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.
No Hidden Policy Change	✅ Passed	PASS: The commit only extracts dispatch/cancellation logic and adds focused routing tests; no product, trust, telemetry, or permission policy changes were introduced.
Title check	✅ Passed	The title is concise and accurately reflects the extraction of OpenAI shim dispatch and stream lifecycle logic.
Description check	✅ Passed	The description covers the change, rebase context, file budget, test coverage, and validation, though it omits the template's Impact and Notes sections.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/services/api/openaiShim/clientDispatch.ts`:
- Around line 41-59: Update ClientDispatchDependencies in
src/services/api/openaiShim/clientDispatch.ts lines 41-59 to inject
codexStreamToAnthropic, collectCodexCompletedResponse, and
convertCodexResponseToAnthropicMessage, then use those dependencies in
createShimRequest instead of direct imports. In
src/services/api/openaiShim/clientDispatch.test.ts lines 131-150, extend the
dispatch table with a codex/responses streaming case and add a non-streaming
case covering the output or incomplete_details detection branch.
- Around line 201-220: Extract the nested stream-converter selection from the
OpenAIShimStream constructor into a small selectStreamConverter(request,
response) helper. Preserve the existing routing precedence and arguments for
codex/responses, messages, Gemini, and OpenAI/Ollama streams, then invoke the
helper from the constructor callback so the routing logic is centralized and
testable.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bc366074-fead-4584-9a6b-6fb16f7e0321

📥 Commits

Reviewing files that changed from the base of the PR and between 10a9190 and aee66a9.

📒 Files selected for processing (3)

• src/services/api/openaiShim.ts
• src/services/api/openaiShim/clientDispatch.test.ts
• src/services/api/openaiShim/clientDispatch.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: smoke-and-tests (24.11.x)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

**/*.{ts,tsx}: Add or update tests when a TypeScript or TSX change affects behavior.
Run the relevant TypeScript validation checks for changed code, including bun run typecheck and, when applicable, bun run typecheck:type-tests.

Files:

• src/services/api/openaiShim/clientDispatch.test.ts
• src/services/api/openaiShim.ts
• src/services/api/openaiShim/clientDispatch.ts

**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Preserve existing repository patterns unless intentionally refactoring them.
Keep changes small, readable, and focused; avoid broad rewrites or unrelated cleanup.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
Review AI-generated changes for correctness, style consistency, unnecessary noise, and adherence to project architecture before submitting them.
Provider changes must follow the documented integration patterns in docs/integrations/overview.md and the focused guides under docs/integrations/how-to/.
When changing provider behavior, avoid breaking third-party providers and test the exact provider/model path changed when possible.
Provider pull requests must explicitly identify affected providers, limitations, and follow-up work.
Run the narrowest meaningful validation command for the touched area, and ensure relevant CI checks pass before merging.
Use bun install to install dependencies and the repository's Bun scripts for building, testing, smoke testing, and development.
Dependency changes require a concrete project benefit such as a bug fix, security issue, or approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, or dependency stack, or introduce a new runtime, without prior maintainer agreement.
Keep each pull request focused on one issue or clearly scoped improvement and avoid bundling unrelated fixes, features, or refactors.

Files:

• src/services/api/openaiShim/clientDispatch.test.ts
• src/services/api/openaiShim.ts
• src/services/api/openaiShim/clientDispatch.ts

---

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

• src/services/api/openaiShim/clientDispatch.test.ts
• src/services/api/openaiShim.ts
• src/services/api/openaiShim/clientDispatch.ts

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}

⚙️ CodeRabbit configuration file

{src/services/api/**,src/integrations/**,src/utils/model/**,src/utils/provider*.ts,src/commands/provider/**}: Review provider routing, model selection, env precedence, auth/token handling, OpenAI-compatible shims, retries, proxy behavior, and outbound HTTP behavior with high scrutiny. Block on silent default changes, hidden fallback expansion, credential reuse mistakes, hardcoded provider assumptions, or new network reach that is not intentional and documented.

Files:

• src/services/api/openaiShim/clientDispatch.test.ts
• src/services/api/openaiShim.ts
• src/services/api/openaiShim/clientDispatch.ts

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

• src/services/api/openaiShim/clientDispatch.test.ts

🔇 Additional comments (11)

src/services/api/openaiShim/clientDispatch.ts (5)

1-14: LGTM!

---

61-79: LGTM!

---

80-160: Cancellation lifecycle is correctly synchronized.

Combined-signal creation, pre-iteration abort wiring, and the early-return/error abort path in the finally block all check out — traced through consumer-break, parent-abort, and pre-iteration-abort scenarios and each converges on the expected controller state. Well covered by the accompanying tests.

---

223-287: LGTM on the non-streaming dispatch/conversion logic — see the DI/coverage comment above regarding the codex branch (Lines 223-229, 234-251) specifically.

---

289-301: LGTM!

src/services/api/openaiShim.ts (3)

961-965: LGTM!

---

1010-1010: LGTM!

---

1041-1062: Clean delegation to createShimRequest; dependency wiring matches the extracted contract.

requestProcessEnv, providerOverride, reasoningEffort, and all required ClientDispatchDependencies fields are supplied correctly, and _doRequest/_convertNonStreamingResponse/_convertGeminiToAnthropicResponse are bound appropriately.

src/services/api/openaiShim/clientDispatch.test.ts (3)

1-61: LGTM!

---

63-129: LGTM! Good targeted coverage of headersWithRequestUrl and the OpenAIShimStream cancellation lifecycle (parent-abort, pre-iteration cancel, early-return abort).

---

152-236: LGTM! withResponse, Anthropic Messages passthrough, Gemini/OpenAI non-streaming conversion, and the unexpected-content-type rejection path are all well covered.

[jatmn]

@kevincodex1 LGTM

[relda88]

Logic looks correct. One suggestion: the early return on line prevents the cleanup function from running — worth adding a finally block.

[kevincodex1]

hi thanks @relda88 for review can you comment it directly in the file change so @jatmn knows the right direction?

[jatmn]

Thanks — the iterator already uses finally to clean the combined signal, and the early return only returns the existing generator after pre-iteration cleanup has been cleared.