fix(responses): disable Lite transport for Spark - #3886
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe canonical OpenAI Responses adapter now removes the Responses Lite header when the wire model is ChangesSpark Responses Lite handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to Spark Responses requests now omit the Lite header so streams can complete normally, while other models retain their existing header behavior. The covered change has no remaining merge-blocking risk. 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. |
⏳ DRAFT
What to do
Review readiness checklist
2/4 boxes ticked. This PR stays in draft until every box above is ticked. Hygiene✅ Deterministic PR hygiene checks passed. |
@coderabbitai review |
|
✅ Action performedReview finished.
|
리뷰 · 우선순위 72 / 80이 PR은 Codex가 고치는 위치는 src/adapters/openai-responses.ts (finalBody.model === "gpt-5.3-codex-spark") - 비교가 정확한 문자열 일치입니다. 대소문자·공백·별칭 철자가 와이어에 다른 형태로 오면 억제가 안 됩니다. 지금 제품 철자가 고정이라면 괜찮지만, 카탈로그에 Spark 변형이 늘면 tests/codex-integration/codex-metadata-integrity.test.ts (새 테스트) - 호출자 Lite + 정적 Lite를 잘 커버합니다. 다만 별칭으로 #3885 닫기 약속 - 본문에 Closes #3885가 있어 머지 시 이슈가 같이 닫힙니다. 이슈 본문과 수정 범위가 일치합니다. 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
5963f59 to
a01d48a
Compare
Rebased onto the latest \dev\ (\522ce5f8c8) and refreshed the focused verification on head \�01d48a3eb: 171/171 focused transport tests passed, typecheck passed, privacy scan passed, and \git diff --check\ passed. @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- PR diff against merge base ---'
git diff --find-renames "$(git merge-base HEAD origin/dev)"...HEAD -- \
src/adapters/openai-responses.ts \
tests/codex-integration/codex-metadata-integrity.test.ts
echo '--- adapter outline ---'
ast-grep outline src/adapters/openai-responses.ts --items all
echo '--- test outline ---'
ast-grep outline tests/codex-integration/codex-metadata-integrity.test.ts --items all
echo '--- Lite-header definitions and usages ---'
rg -n -C 5 --glob '*.ts' \
'CODEX_RESPONSES_LITE_HEADER|x-openai-internal-codex-responses-lite|createResponsesPassthroughAdapter' \
src tests
echo '--- adapter source around changed lines ---'
git diff --unified=80 "$(git merge-base HEAD origin/dev)"...HEAD -- src/adapters/openai-responses.ts
echo '--- test source around changed lines ---'
git diff --unified=100 "$(git merge-base HEAD origin/dev)"...HEAD -- tests/codex-integration/codex-metadata-integrity.test.tsLength of output: 237097 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- repository refs and HEAD ---'
git status --short --branch
git show -s --format='HEAD %H%nparents %P%nsubject %s' HEAD
git branch -a --no-color
echo '--- current commit patch ---'
git show --format=fuller --find-renames --stat HEAD
git show --format= --find-renames HEAD -- \
src/adapters/openai-responses.ts \
tests/codex-integration/codex-metadata-integrity.test.ts
echo '--- adapter request construction ---'
sed -n '2298,2535p' src/adapters/openai-responses.ts
echo '--- metadata regression tests ---'
sed -n '1,245p' tests/codex-integration/codex-metadata-integrity.test.ts
echo '--- WebSocket request preparation ---'
sed -n '1,180p' src/server/responses/codex-ws-request.ts
echo '--- shared header behavior ---'
sed -n '1,220p' src/codex/forward-transport-headers.ts
echo '--- finalBody assignments and header mutations in adapter ---'
rg -n -C 4 --glob 'openai-responses.ts' \
'finalBody|headers\[|delete headers|new Headers\(headers\)|applyCodexRoutingHint' \
src/adaptersLength of output: 40822
The removal occurs before routing-header construction.
I did not run the test commands in this sandbox. The reported focused verification is consistent with the reviewed change.
|
The canonical backend starts a Spark SSE response with the Responses Lite header but closes it before a terminal event, which the adapter correctly surfaces as adapter_eof. The identical request completes without that header. Select the compatibility exception from the final wire model and remove both caller-provided and statically configured Lite headers only for gpt-5.3-codex-spark. Other canonical models retain the existing metadata path. Regression: cover Spark suppression and unaffected Sol forwarding at the adapter boundary.
a01d48a to
83c1d9b
Compare
Addressed the Grok review suggestion on the latest head: the regression now explicitly covers Focused verification remains green: 171/171 tests, 1209 assertions, plus typecheck. Test-only temporary directories and the stale Bun test lock were removed after the run. @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/dev)"
echo '--- HEAD ---'
git show -s --format='commit %H%nparents %P%nsubject %s' HEAD
echo '--- changed files ---'
git diff --name-status "$base"...HEAD
echo '--- pull request diff ---'
git diff --find-renames --unified=80 "$base"...HEAD -- \
src/adapters/openai-responses.ts \
tests/codex-integration/codex-metadata-integrity.test.ts
echo '--- adapter outline ---'
ast-grep outline src/adapters/openai-responses.ts --items all
echo '--- test outline ---'
ast-grep outline tests/codex-integration/codex-metadata-integrity.test.ts --items all
echo '--- adapter header and wire-model context ---'
rg -n -C 12 --glob 'openai-responses.ts' \
'finalBody\.model|CODEX_RESPONSES_LITE_HEADER|createResponsesPassthroughAdapter|applyCodexRoutingHint' \
src/adapters/openai-responses.ts
echo '--- metadata regression test context ---'
rg -n -C 18 --glob 'codex-metadata-integrity.test.ts' \
'gpt-5\.3-codex-spark|parsed\.modelId|CODEX_RESPONSES_LITE_HEADER|responses-lite' \
tests/codex-integration/codex-metadata-integrity.test.ts
echo '--- shared Lite-header behavior ---'
rg -n -C 8 --glob '*.ts' \
'CODEX_RESPONSES_LITE_HEADER|x-openai-internal-codex-responses-lite' \
src testsLength of output: 1615 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- available refs ---'
git branch --all --no-color
git show-ref --heads --remotes || true
echo '--- HEAD ---'
git show -s --format='commit %H%nparents %P%nsubject %s' HEAD
echo '--- current commit changed files ---'
git diff-tree --no-commit-id --name-status -r HEAD
echo '--- current commit patch ---'
git show --format=fuller --find-renames --unified=100 HEAD -- \
src/adapters/openai-responses.ts \
tests/codex-integration/codex-metadata-integrity.test.ts
echo '--- adapter outline ---'
ast-grep outline src/adapters/openai-responses.ts --items all
echo '--- test outline ---'
ast-grep outline tests/codex-integration/codex-metadata-integrity.test.ts --items all
echo '--- adapter header and wire-model context ---'
rg -n -C 14 --glob 'openai-responses.ts' \
'finalBody\.model|CODEX_RESPONSES_LITE_HEADER|createResponsesPassthroughAdapter|applyCodexRoutingHint' \
src/adapters/openai-responses.ts
echo '--- metadata regression test context ---'
rg -n -C 22 --glob 'codex-metadata-integrity.test.ts' \
'gpt-5\.3-codex-spark|parsed\.modelId|CODEX_RESPONSES_LITE_HEADER|responses-lite' \
tests/codex-integration/codex-metadata-integrity.test.tsLength of output: 51104
The test also verifies removal of the mixed-case static header without an incoming Lite header and preservation for I did not run the focused tests or typecheck in this sandbox. The reported verification is consistent with the reviewed change. ✅ Action performedReview finished.
|
…3942) * docs(devlog): plan the workstream-A Responses compatibility stack Roadmap for landing four Responses-compatibility changes on dev as one dependent branch chain whose tip carries all of them, so a single CI run certifies the set: PR #3906 (Muse Spark Free web_search strip), PR #3886 (Spark Responses Lite header), issue #3922 (Claude tool strict default, new work), and PR #3917 (routed agent_message conversion). Each phase doc carries exact path:line anchors and before/after diffs. Three rounds of independent audit corrected the Layer 2 HTTP/WebSocket coverage boundary, the Claude compatibility semantics, the Layer 3 test that an added strict field breaks, the converted authMode set, and the landing proof for each GitHub merge method. * fix(responses): strip web_search fields for Muse Spark Contributor Free tiers The -free tiers ride the same Zen Responses wire with the same gateway contract, so a Codex web_search carrying search_content_types / indexed_web_access 400s for them exactly like the paid tiers. (cherry picked from commit 11c498b) Co-authored-by: MohamadSabree8 <mohamadsabree8@users.noreply.github.com> * test(responses): cover nested and preview cases for Muse Spark Free tiers The carried fix covered a top-level web_search tool for the two Contributor Free ids. The sanitizer also walks input[].additional_tools.tools, and it must leave web_search_preview alone, so pin both for the free ids the way the paid ids are already pinned. Co-authored-by: MohamadSabree8 <mohamadsabree8@users.noreply.github.com> * fix(responses): disable Lite transport for Spark The canonical backend starts a Spark SSE response with the Responses Lite header but closes it before a terminal event, which the adapter correctly surfaces as adapter_eof. The identical request completes without that header. Select the compatibility exception from the final wire model and remove both caller-provided and statically configured Lite headers only for gpt-5.3-codex-spark. Other canonical models retain the existing metadata path. Regression: cover Spark suppression and unaffected Sol forwarding at the adapter boundary. (cherry picked from commit 83c1d9b) Co-authored-by: R <53855466+cb8010d6@users.noreply.github.com> * docs(devlog): note the layer-1 line drift in the layer-2 anchors Layer 1 inserts two lines above the canonical-forward block, so the phase doc now states both the pinned-base line numbers and where the same code sits on this branch. * fix(claude): carry the source strict intent into translated Responses tools Anthropic enables strict tool use by setting strict: true, while the Responses API reads an omitted strict as permission to normalize the schema into strict mode. Translating a Claude Code tool without the field therefore made every optional input_schema parameter behave as required upstream, so a tool call that omitted one failed even though the client never asked for strict mode. Emit the field from the source tool: an explicit true or false is preserved, an omitted one becomes an explicit false, and a non-boolean value cannot opt the tool into strict mode. The input_schema is forwarded unchanged, hosted web_search leaves the translator before this branch, and native Anthropic passthrough never reaches it. The existing exact expectation on the translated Read tool gains the field. The new regression asserts the three cases on the serialized outbound body built by a real Responses adapter, because parsed._rawBody is the translator's own object and reading it back would prove nothing about the wire. Closes #3922. * fix(adapters): convert Codex agent_message for every routed Responses destination `agent_message` is Codex's private multi-agent input item and exists only in the ChatGPT Codex backend's schema. Codex writes every sub-agent reply into the rollout, so it is replayed in the `input` of every later turn of that thread. A routed Responses destination answers the whole body with `422 unknown item type "agent_message"`, and 422 is a client error nothing fails over, so the thread stays broken until the history is dropped. The plaintext conversion already existed but was scoped to the OpenCode Go destination. Nothing about the rejection is destination-specific, so the conversion now applies to every destination with `authMode` other than "forward". Forward destinations keep the item unchanged, and genuine ciphertext and unknown part types keep their existing fail-closed path; the encrypted v2 task surface still owns those through `unreadable_encrypted_agent_task` and the opt-in recovery route. `isOpenCodeGo` existed only to scope this call and is removed with it. The helper and its tests move to destination-neutral names. Opaque-blob recovery repairs an undecryptable part into an omission marker, which leaves the item entirely plaintext; on a routed retry it is now converted too, which is what lets that retry be accepted at all. Fixes #3911 (cherry picked from commit 2430724) Co-authored-by: mashfromband <matsumoto.yukuhashi@gmail.com> * test(adapters): pin an OAuth destination and narrow the routed-422 wording The carried conversion is gated on authMode rather than on the destination URL, but its regression only exercised key and forward. The reported xAI/Grok failure is an OAuth pool destination, so pin one: a future narrowing of the gate back toward key-only would otherwise pass unnoticed. Also narrow the two reference pages. The conversion is justified by the destinations that actually reported the 422; authMode is an authentication setting, so it cannot establish what every custom upstream accepts. Co-authored-by: mashfromband <matsumoto.yukuhashi@gmail.com> * docs(devlog): drop a trailing blank line in the layer-1 phase doc --------- Co-authored-by: Codex <a@b.com> Co-authored-by: MohamadSabree8 <mohamadsabree8@users.noreply.github.com> Co-authored-by: R <53855466+cb8010d6@users.noreply.github.com> Co-authored-by: mashfromband <matsumoto.yukuhashi@gmail.com>
|
The work from this pull request has landed on Your commit was carried onto the stack with its original authorship intact: the carried commit kept you as its author and its Your delete-only form was carried unchanged. Worth recording as a known limit rather than a gap in your fix: on the WebSocket path, Verified on a fetched |
lidge-jun
added
the
landed-via-maintainer
2 participants
Summary
gpt-5.3-codex-sparkstream ending asresponse.incomplete/adapter_eofwhen Codex sendsx-openai-internal-codex-responses-lite: true.gpt-5.3-codex-spark. Both caller-provided and mixed-case static spellings are removed.The final body is authoritative because routing or aliases may make
parsed.modelIddiffer from the model sent upstream. Suppressing the header at that boundary also covers both the initial WebSocket preparation and its HTTP fallback without manufacturing a successful terminal after an upstream EOF.Closes #3885.
Verification
canonical adapter drops Lite only for the Spark wire modelfailed withExpected: null / Received: "true"before the source change.bun test tests/codex-integration/codex-metadata-integrity.test.ts tests/responses/ws-upstream.test.ts tests/responses/ws-upstream-reuse.test.ts— 171 pass / 0 fail after rebasing onto currentdev.bun test --rerun-each 3 tests/codex-integration/codex-metadata-integrity.test.ts— 54 pass / 0 fail.bun run typecheck— passed.bun run privacy:scan— passed.bun run test:changedselected the central adapter dependency set but reached the repository runner's 900-second suite ceiling (exit 124) without emitting an assertion failure. Full-suite completion remains deferred to CI; this result is not claimed as green.No GUI, configuration, or public API changes; docs and i18n are unaffected.
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
gpt-5.3-codex-sparknow correctly remove the internal Lite header, regardless of capitalization.gpt-5.3-codex-sparknow receive the same corrected handling.