fix(responses): restore DeepSeek streaming and routed code-mode tools - #1361
fix(responses): restore DeepSeek streaming and routed code-mode tools#1361baileyh8 wants to merge 22 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds routed Codex local-tool support through ChangesRouted Codex tools
DeepSeek Responses streaming
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Codex
participant OpenAIResponses
participant ResponsesCore
participant UpstreamResponses
Codex->>OpenAIResponses: Send routed custom-tool request
OpenAIResponses->>ResponsesCore: Rewrite custom tools as function tools
ResponsesCore->>UpstreamResponses: Send sanitized Responses request
UpstreamResponses-->>ResponsesCore: Stream function-call events
ResponsesCore-->>Codex: Restore custom-tool events and terminal response
sequenceDiagram
participant DeepSeek
participant ResponsesCore
participant TerminalRepair
participant Codex
DeepSeek->>ResponsesCore: Return native Responses SSE
ResponsesCore->>TerminalRepair: Apply model-scoped repair policy
TerminalRepair-->>Codex: Forward progressive events
TerminalRepair-->>Codex: Emit response.completed or response.incomplete
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request has been marked Ready for Review. |
c4389b6 to
f63a3e7
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs-site/src/content/docs/guides/codex-integration.md`:
- Around line 198-201: Update the documentation paragraph to apply tool_mode:
"code_mode_only" only to routed catalog entries, matching the scope classified
by src/codex/catalog/sync.ts when model is defined. Avoid describing all
non-OpenAI rows as routed, and preserve the existing explanation of Codex
exec/MCP exposure and local tool execution. Ensure native account-qualified
entries such as team/gpt-5.5 remain excluded from this policy.
- Around line 196-214: Add the missing “Routed local tools” section to the
localized Codex integration pages for Japanese, Korean, Russian, and Simplified
Chinese, matching the English section’s current content and documenting the same
tool-routing behavior and restart guidance.
In `@src/providers/registry.ts`:
- Around line 2320-2322: Update the policy lookup logic around
modelResponsesTerminalRepair to floor policy.graceMs before validation, then
reject the normalized value when it is non-positive or non-finite. Return the
normalized positive integer so fractional values that floor to zero yield
undefined rather than { graceMs: 0 }.
In `@src/server/responses-custom-tool-repair.ts`:
- Around line 33-41: Update the escape-decoding branch in partialCustomToolInput
within src/server/responses-custom-tool-repair.ts (lines 33-41) to decode valid
JSON \b and \f escapes into backspace and form-feed characters, matching
JSON.parse behavior. Extend the fragmented escaped-control-character cases in
tests/responses-custom-tool-repair.test.ts (lines 90-168) to verify streamed
deltas equal the final done input; both sites require changes.
In `@tests/deepseek-inbound-wire.test.ts`:
- Around line 271-277: Update the test around ManualTerminalScheduler and the
terminal-repair flow to expose a pending() method that reports scheduled jobs,
then wait until pending() confirms one job is armed before calling
scheduler.advance(5_000). Replace the single Bun.sleep(0) assumption with this
readiness wait while preserving the existing terminal-close assertion.
In `@tests/responses-terminal-repair.test.ts`:
- Around line 395-414: Update the race test around
relayResponsesSseWithTerminalRepair to capture an armed timer callback before
delivering the real terminal, then execute that callback afterward and assert
only one terminal plus no pending timers. In the cancellation and abort tests,
consume each repaired stream through response.output_item.done so the lifecycle
is processed and a timer is armed, assert one pending timer, then perform
cancellation or abort and verify the timer is cleared and the stream behavior
remains correct.
🪄 Autofix
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 Plus
Run ID: 61eebdc9-078b-4cfa-8ddc-7e3daee6b311
📒 Files selected for processing (23)
docs-site/src/content/docs/guides/codex-integration.mddocs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/zh-cn/guides/providers.mddocs/superpowers/plans/2026-08-06-deepseek-responses-streaming-terminal-repair.mddocs/superpowers/plans/2026-08-09-routed-computer-use-browser.mddocs/superpowers/specs/2026-08-06-deepseek-responses-streaming-terminal-repair-design.mddocs/superpowers/specs/2026-08-09-routed-computer-use-browser-design.mdsrc/adapters/openai-responses.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/providers/registry.tssrc/responses/custom-tool-compat.tssrc/server/responses-custom-tool-repair.tssrc/server/responses-terminal-repair.tssrc/server/responses/core.tsstructure/04_transports-and-sidecars.mdtests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/deepseek-inbound-wire.test.tstests/openai-responses-passthrough.test.tstests/passthrough-abort.test.tstests/responses-custom-tool-repair.test.tstests/responses-terminal-repair.test.ts
c0d472e to
623d610
Compare
623d610 to
659c9e4
Compare
|
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/adapters/openai-responses.ts`:
- Around line 1249-1251: Use the same resolved authMode source for custom-tool
rewriting, custom-tool name collection, and registry transport checks. In
src/adapters/openai-responses.ts lines 1249-1251, replace the
adapterProvider-based decision with the shared route/provider authMode used by
core; update src/server/responses/core.ts lines 1904-1906 and 2267-2271 to use
adapterProvider, which is available from the flow beginning around line 1766, so
all passthrough and transport decisions remain consistent for custom key
providers.
In `@src/server/responses-custom-tool-repair.ts`:
- Around line 104-130: Update the response processing around itemNames and the
response.function_call_arguments.delta branch to buffer unmatched deltas by
output_index and/or function name until the routed item ID is known, then
associate and replay them under that ID. Do not relay buffered deltas as
function-call SSE. Keep releaseCall exclusively in the response.output_item.done
path after the call has been identified.
In `@src/server/responses-terminal-repair.ts`:
- Around line 292-303: Update the done-handling path around nextSseBlock(),
inspectPayload(), and emitSynthetic() to mark any residual unframed buffer
suffix as tainted, relay its bytes if required, but never inspect it as an SSE
payload or allow it to establish completion. Add a regression test covering a
final response.output_item.done frame without its terminating blank line,
asserting that the relay emits response.incomplete.
In `@src/server/responses/core.ts`:
- Around line 2311-2313: Reorder the block rewrites in the construction of
blockRewrites so createRoutedCustomToolRestoreBlockRewrite runs after the GitHub
Copilot and snapshot rewrites, ensuring those rewrites process upstream
function_call events before routed custom tools are converted. Preserve the
existing conditional behavior for routedCustomToolNames and translatorBudget.
In `@tests/responses-custom-tool-repair.test.ts`:
- Around line 199-257: Extend the handleResponses SSE test to assert clientSse
contains no function_call item type, not only no arguments event. Add a focused
non-streaming sibling test with the fetch stub returning application/json and an
output function_call, then assert the response exposes custom_tool_call with the
unwrapped input. Also cover the core authMode "forward" gate by verifying routed
custom tools are not restored or forwarded in that configuration.
🪄 Autofix
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 Plus
Run ID: ff96d133-218a-4256-b60c-ceafcf9de694
📒 Files selected for processing (27)
docs-site/src/content/docs/guides/codex-integration.mddocs-site/src/content/docs/guides/providers.mddocs-site/src/content/docs/ja/guides/codex-integration.mddocs-site/src/content/docs/ko/guides/codex-integration.mddocs-site/src/content/docs/ru/guides/codex-integration.mddocs-site/src/content/docs/zh-cn/guides/codex-integration.mddocs-site/src/content/docs/zh-cn/guides/providers.mddocs/superpowers/plans/2026-08-06-deepseek-responses-streaming-terminal-repair.mddocs/superpowers/plans/2026-08-09-routed-computer-use-browser.mddocs/superpowers/specs/2026-08-06-deepseek-responses-streaming-terminal-repair-design.mddocs/superpowers/specs/2026-08-09-routed-computer-use-browser-design.mdsrc/adapters/openai-responses.tssrc/codex/catalog/parsing.tssrc/codex/catalog/sync.tssrc/providers/registry.tssrc/responses/custom-tool-compat.tssrc/server/responses-custom-tool-repair.tssrc/server/responses-terminal-repair.tssrc/server/responses/core.tsstructure/04_transports-and-sidecars.mdtests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/deepseek-inbound-wire.test.tstests/openai-responses-passthrough.test.tstests/passthrough-abort.test.tstests/responses-custom-tool-repair.test.tstests/responses-terminal-repair.test.ts
|
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/responses-custom-tool-repair.ts (1)
94-105: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRetained bytes charged after a terminal event are never released, and no test covers that order.
releaseAlllatchesdisposedon the first terminal event, so every later release becomes a no-op whileretainPendingArgumentand the routed-delta branch keep charging theTranslatorBudget. The single root cause is thedisposedflag doing double duty as both a "already released" guard and a "disposed" marker.
src/server/responses-custom-tool-repair.ts#L94-L105: remove theif (disposed) return;guard and thedisposed = true;assignment fromreleaseAll, which is already idempotent. Setdisposedonly in therewrite.disposewrapper at Line 251, and gateretainPendingArgumenton it so no new bytes are charged after disposal.tests/responses-custom-tool-repair.test.ts#L171-L215: add a sibling test that sendsresponse.completed, then a lateresponse.function_call_arguments.delta, then callsrewrite.dispose?.(), and assertsbudget.snapshot().currentBytesis0.🤖 Prompt for 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. In `@src/server/responses-custom-tool-repair.ts` around lines 94 - 105, Separate disposal state from cleanup in releaseAll: in src/server/responses-custom-tool-repair.ts#L94-L105, remove its disposed guard and assignment, set disposed only in the rewrite.dispose wrapper at Line 251, and gate retainPendingArgument so it cannot charge bytes after disposal. Add the sibling regression test in tests/responses-custom-tool-repair.test.ts#L171-L215 covering response.completed, a late response.function_call_arguments.delta, and rewrite.dispose?.(), asserting budget.snapshot().currentBytes is 0.
🤖 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/server/responses-custom-tool-repair.ts`:
- Around line 117-134: Update takePendingArguments so a pending entry with an
itemId matches only the requested itemId; use outputIndex as a fallback only
when the pending entry has no itemId. Add a focused regression test beside the
existing buffering tests in responses-custom-tool-repair.test.ts that buffers
fc_a at output_index 0, adds fc_b at the same index, and verifies fc_a’s delta
is not replayed as fc_b input.
In `@tests/responses-custom-tool-repair.test.ts`:
- Around line 398-447: Update the forward-auth test around handleResponses to
capture the outbound request headers in the mocked globalThis.fetch, then assert
that the forwarded authorization header equals "Bearer caller-token". Keep the
existing outbound body and custom-tool assertions unchanged, and ensure the
header capture handles the fetch init headers.
- Around line 171-215: Add a sibling regression test near “buffers argument
events until a missing added event is identified by item done” that sends a
terminal event first, then an argument frame, and asserts the translator
budget’s currentBytes returns to zero. Exercise the same rewrite path and verify
post-terminal argument handling does not retain budget bytes, while preserving
existing terminal behavior.
---
Outside diff comments:
In `@src/server/responses-custom-tool-repair.ts`:
- Around line 94-105: Separate disposal state from cleanup in releaseAll: in
src/server/responses-custom-tool-repair.ts#L94-L105, remove its disposed guard
and assignment, set disposed only in the rewrite.dispose wrapper at Line 251,
and gate retainPendingArgument so it cannot charge bytes after disposal. Add the
sibling regression test in tests/responses-custom-tool-repair.test.ts#L171-L215
covering response.completed, a late response.function_call_arguments.delta, and
rewrite.dispose?.(), asserting budget.snapshot().currentBytes is 0.
🪄 Autofix
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 Plus
Run ID: ee36f5de-d3eb-45b3-b53c-b4f427d3142a
📒 Files selected for processing (4)
src/server/responses-custom-tool-repair.tssrc/server/responses-terminal-repair.tstests/responses-custom-tool-repair.test.tstests/responses-terminal-repair.test.ts
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
Summary
deepseek-v4-flashwhile safely repairing terminal-less upstream SSE streams.code_mode_onlyin the Codex catalog so routed sessions can reach supported local Computer Use and browser capabilities throughnode_repl.execcustom-tool calls across the passthrough boundary without changing native OpenAI model behavior.Why
OpenCodex previously forced
deepseek-v4-flashthrough non-streaming JSON because its upstream SSE can omit a terminal event. Codex users therefore saw no progressive output and could receive a late 502 when the response stalled.Separately, routed models were catalogued without the code-mode policy and their
execcustom-tool lifecycle was not translated across the Responses passthrough boundary. As a result, selecting a routed model disabled browser and Computer Use workflows even though Codex could safely expose them through its localnode_replruntime.Impact
DeepSeek Responses output now streams progressively, terminal events are synthesized only for structurally complete lifecycles, and unsafe or ambiguous streams fail closed as incomplete. Routed models can use supported local browser and Computer Use operations through code mode. Native OpenAI routes remain unchanged.
The existing Responses vision sidecar path and image-generation behavior were regression-tested as part of the end-to-end validation.
Verification
bun run typecheck— pass.bun run privacy:scan— pass.bun run buildindocs-site— pass, 221 pages built.git range-diffafter rebasing onto4f322803f— the original 20 commits remained patch-equivalent; review fixes9481b37b4and41099a15fare added on top, and GitHub Compare reports 0 commits behinddev.bun run teston the precedingdevbase — 10,643 pass / 7 skip / 8 fail; the same eightcodex-auth-apifailures reproduced on a clean upstreamdevworktree, and the subsequent upstream commits do not touch that subsystem.Checklist
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
New Features
Bug Fixes
Documentation