fix(responses): restore DeepSeek streaming and routed code-mode tools - #1361
Conversation
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 with 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 is already Ready for Review. |
c4389b6 to
f63a3e7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
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. |
41099a1 to
b9b723c
Compare
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 19-21: The partialCustomToolInput flow must fail closed for
freeform argument shapes that do not begin with the exact expected wrapper,
rather than returning raw JSON text. Update the progressive delta emission
around partialCustomToolInput so unrecognized accumulated arguments produce no
delta while unwrapRoutedCustomToolArguments still supplies the authoritative
done input, and add a focused regression test in the existing custom-tool repair
tests covering {"input": "spaced"} and matching concatenated deltas to the final
input.
- Around line 174-190: The replay path in rewrite currently reclassifies
index-matched pending blocks without an item id, causing them to be buffered
again and dropped. Update the pending replay flow around takePendingArguments
and rewrite so an entry matched by outputIndex is stamped with the resolved
routed item id before replay, while preserving existing behavior for blocks that
already have an item id; add a focused regression test beside the buffering
tests covering an id-less function_call_arguments.delta followed by the routed
response.output_item.added and asserting the custom-tool input delta is emitted.
In `@tests/responses-custom-tool-repair.test.ts`:
- Around line 317-344: Extend the test around
createRoutedCustomToolRestoreBlockRewrite to include a Unicode escape split
across delta fragments, such as ending one fragment with an incomplete \uXXXX
sequence and completing it in the next. Assert that streamedInput decodes to the
expected character and remains equal to the done input, locking in the
incomplete-hex guard behavior.
🪄 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: f77113a1-54d2-4e9c-aba4-9ba59df096b4
📒 Files selected for processing (2)
src/server/responses-custom-tool-repair.tstests/responses-custom-tool-repair.test.ts
Tolerate whitespace in freeform argument wrappers, stamp resolved item ids onto index-matched pending replays, and lock split unicode escapes plus continuation call_id pairing with focused regressions.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
✅ Action performedReview finished.
|
|
|
|
Thanks @baileyh8 — merged to This was genuinely useful bug work. DeepSeek V4 Flash users were stuck with buffered/non-streaming Responses and late failures when upstream SSE omitted a terminal; your terminal-repair path restores progressive output while failing closed on incomplete lifecycles. The routed Appreciate the careful test coverage and the patience through the rebase/evidence pass. |
|
Thank you for the thoughtful review and for landing this. I really appreciate the recognition, and especially the extra care Wibias put into rebasing, validating the exact head, and tightening the remaining edge cases. Glad the fix is now in dev and useful to the project. |
3 participants
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.
Verification
Exact SHAs at verification time:
da36a30aaa7d986d9e929e820c57ad16e799ec0fdev:08e7e0fda77494cf25db3a0ad1127fe4800a15dadev(ancestry includes currentdev)Exact-head GitHub Actions:
da36a30a)da36a30a)0; CodeRabbit inline findings resolved on this headLocal gates on
da36a30a:bun run typecheck— passbun run privacy:scan— passtests/responses-custom-tool-repair.test.ts(includeshandleResponses continuation rewrites custom_tool_call_output and keeps call_id ordered+ forward-auth native passthrough)tests/responses-terminal-repair.test.ts(fail-closed incomplete / abort / single-terminal)tests/deepseek-inbound-wire.test.tstests/passthrough-abort.test.tstests/openai-responses-passthrough.test.tstests/codex-catalog.test.ts(169 pass / 0 fail separately)tests/responses-image-gen-repair.test.tstests/vision-fail-closed.test.tstests/deepseek-responses-item-id-repair.test.ts,tests/bridge-terminal-singleness.test.ts,tests/cancel-body-on-abort.test.ts(14 pass / 0 fail)tests/codex-catalog-sync-hardening.test.ts— still 16 fail on this head; reproduced on cleanorigin/dev@08e7e0fd(baseline, not introduced here)DeepSeek V4 Flash real streaming smoke (PR-head process)
Started this head with isolated
OPENCODEX_HOMEon127.0.0.1:19161and streamed/v1/responses:nvidia/deepseek-ai/deepseek-v4-flash-0731: progressiveresponse.output_text.deltaat 619ms before soleresponse.completedat 722ms; exactly one terminal; one[DONE]command-code/deepseek/deepseek-v4-flash: progressive deltas before soleresponse.completed; one[DONE]response.completedFail-closed malformed / truncated / timer-race / incomplete lifecycle behavior is covered by deterministic
tests/responses-terminal-repair.test.ts(not pretended to be a live malformed DeepSeek provider trace).Routed
execcontinuationExisting exact-head integration test already proves the requested boundary (no redundant test added):
handleResponses continuation rewrites custom_tool_call_output and keeps call_id orderedhandleResponses leaves custom tools native for forward-auth passthroughEvidence: first response restores client
custom_tool_callwith preservedcall_id; continuation rewrites matching output to upstreamfunction_call_outputfor the samecall_id; unrelatedcall_otherstayscustom_tool_call_output; second response precedes[DONE]; upstreamfunction_callshapes do not leak to the client; forward-auth remains native.Codex Desktop / app-server Browser locality smoke
ocx syncwrote live catalog with routed DeepSeek rows attool_mode: "code_mode_only"(e.g.nvidia/deepseek-ai-deepseek-v4-flash-0731)codex execagainst that routed model used the local Browser plugin path (mcp__node_repl__js+~/.codex/plugins/.../browser/.../browser-client.mjs), not an OpenCodex remote browser executorprovider=nvidia, status 200) onlyagent.browsers.list() == []); Codex reported unavailability rather than inventing a remote browser APIbrowser@openai-bundled,computer-use@openai-bundled, localcodex-computer-usepipe). Approval gates were left atask_for_approval=on-request(no dangerously-bypass)Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation