fix(adapters): convert Codex agent_message for every routed Responses destination - #3917
fix(adapters): convert Codex agent_message for every routed Responses destination#3917mashfromband wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueNo 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 (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe OpenAI Responses adapter now converts plaintext ChangesRouted agent message handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Plaintext sub-agent messages are converted for routed Responses destinations, preventing repeated schema-rejection failures while preserving forward-provider and fail-closed behavior. No actionable current-head merge risk remains. Sequence Diagram(s)sequenceDiagram
participant Codex
participant buildRequest
participant normalizeRoutedAgentMessages
participant RoutedResponsesDestination
Codex->>buildRequest: send request with agent_message history
buildRequest->>normalizeRoutedAgentMessages: normalize when authMode is not forward
normalizeRoutedAgentMessages->>buildRequest: return message/user items
buildRequest->>RoutedResponsesDestination: send normalized request
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files. (4 skipped: 4 unsupported.) ✨ 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
1/4 boxes ticked. This PR stays in draft until every box above is ticked. Hygiene✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 73 / 80이 PR는 Codex가 서브에이전트를 쓴 뒤 롤아웃에 남기는 비공개 입력 타입 증상 자체는 릴리즈 트레인 스냅샷이 말하는 콤보/라우팅 안정성 방향과 정면으로 맞고, 고치는 코드 양도 작다. 다만 PR이 아직 draft이고 본문이 로컬 풀 스위트 900초 타임아웃을 솔직히 적었다. 가벼운 hygiene/enforce-target만 초록인 상태라, Ready로 올린 뒤 크로스 플랫폼 CI가 한 바퀴 도는 게 머지 게이트다. 연결된 #3911은 템플릿 없이 자동으로 not_planned 닫힌 이슈라, 머지 전에 이슈를 다시 열거나 Fixes 링크를 정리할지가 남는다. 비슷한 표면의 열린 이슈 #3907(V2 child
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
… 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 lidge-jun#3911
21eaaf9 to
2430724
Compare
…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 Two additions on top of your change: an OAuth-destination regression, since the gate reads Verified on a fetched |
lidge-jun
added
the
landed-via-maintainer
2 participants
Fixes #3911.
Summary
A Codex thread that has used sub-agents cannot be routed. Codex writes every sub-agent
reply into the rollout as an
agent_messageitem, so it is replayed in theinputofevery later turn of that thread, and
agent_messageis private to the ChatGPT Codexbackend's schema. A routed Responses destination answers the whole request with
422 is a client error, so nothing fails over (
attemptshas one entry) and the statusreaches the client verbatim. In the report behind #3911 the same conversation failed 12
times in 15 minutes on
xai/grok-4.6picked by a combo route.The plaintext conversion for this already exists; it was scoped to the OpenCode Go
destination. Nothing about the rejection is destination-specific, so this widens the
conversion to every destination with
authModeother than"forward":forwarddestinations are unchanged. That is the only schema that understands theitem, and the existing forward tests still assert it arrives untouched.
path; the encrypted v2 task surface still owns them through
unreadable_encrypted_agent_taskand the opt-in recovery route. This PR does not touchthat policy.
isOpenCodeGoexisted only to scope this one call, so it is removed with it, and thehelper plus its test file move to destination-neutral names.
One behavioral consequence worth naming: opaque-blob recovery repairs an undecryptable
part into
[encrypted content omitted], which leaves the item entirely plaintext. On arouted retry that item is now converted too — which is what lets the retry be accepted at
all, since re-sending the repaired-but-still-private item would hit the same 422. The four
assertions in
tests/responses/responses-opaque-blob-recovery.test.tsthat pinned the oldshape are updated to the converted one (their fixture provider is
authMode: "key", i.e.routed); the
authMode: "forward"case in the same file is untouched and still passes.docs-sitesaid the conversion "is scoped to that destination ... other Responsesdestinations keep their input unchanged", so both reference pages are updated.
Verification
Rebased onto
devat8bc9e4e; the tested tree is21eaaf9.bun run typecheck— passed.bun run privacy:scan— passed (Privacy scan passed).git diff --check— clean.bun test tests/adapters/routed-agent-messages.test.ts tests/responses/responses-opaque-blob-recovery.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts— 82 pass, 0 fail, 917expect()calls. That is the moved/renamed suite, the file whose expectations this PR changes, and both layout guards.openai-responses.tscall-site line reproduces the old behavior in the newan arbitrary routed destination converts tootest.input[1]differing between requests:message→ 200,agent_message(plaintext) → 422,agent_messagewithencrypted_content→ 422. After this change the same routed request returns 200, and theauthMode: "forward"destination still receives the item unchanged.bun run test(full suite) reached this repository's own 900-second runner limit andexited 124 on my machine, which was not idle — the runner's own message names that cause.
The failures it did print before the limit were 5000 ms timeouts of server/API tests plus
one
EICACLSWindows ACL error, and each one I re-ran in isolation passed. I am notreporting the full local suite as a pass; the cross-platform CI on this PR is the signal
I am relying on for it.
Checklist
Security note: this sends strictly less to a routed destination than before in the
encrypted case (nothing changes — the item is still not converted) and the same bytes in
the plaintext case, re-labelled.
author/recipientwere already on the wire inside theitem and remain readable text. No decryption is attempted anywhere in this change.
Review readiness checklist
Summary by CodeRabbit
New Features
Documentation