fix(responses): strip OpenAI-internal input metadata before routing upstream - #3038
fix(responses): strip OpenAI-internal input metadata before routing upstream#3038L-Y-J wants to merge 1 commit into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Responses API now removes ChangesRequest metadata sanitization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change removes one backend-only metadata field before Responses requests are routed upstream while preserving unrelated input data. No actionable merge-blocking risk remains after normal checks and review. 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 |
리뷰 · 우선순위 75 / 80이 PR은 Codex Desktop 고침은 한 헬퍼로 그 규칙을 프록시 입구에서 한 번 더 적용하는 것이다. 새 파일 콤보 경로는 조금 다르게 닫힌다. 부모 쪽에서 첫 strip이 끝난 뒤 테스트
메인테이너의 판단이 필요한 지점
너의 추천 CI(resolve-pr)만 초록 확인되면 머지하세요. Desktop 0.151 사용자에게 매 턴 400이 나는 실사용 차단이라 우선순위가 높다. 헬퍼·세 호출점·단위 테스트 방향은 이 댓글은 grok-bot이 작성했습니다 |
Ingwannu
left a comment
There was a problem hiding this comment.
Requesting changes on exact head 65eedf9. The reported strict-upstream 400 is real, but the current helper is called unconditionally immediately after request decoding in both core and compact. That strips internal_chat_message_metadata_passthrough from true native ChatGPT/OpenAI forward requests as well as routed providers, even though Codex deliberately preserves the field for the OpenAI backend. Mirror the non-OpenAI rule after the final route and auth destination are known, not at the shared ingress boundary. Preserve the field for the canonical OpenAI forward lane, strip it for routed and custom upstreams, and apply the same route-aware rule after previous-response expansion and to each combo child before its non-native send. Add integration regressions proving native preservation plus routed removal in direct, compact, replay, and combo paths; helper-only tests cannot catch the current over-stripping. Keep encrypted_function_args out of scope until there is a rejection case, as proposed.
…oadmap (lidge-jun#3087) Rescans every open issue and bug-labelled PR against a written-down four-axis rubric, and plans the six targets that score >= 70 as one PABCD cycle each. Six enter the train: lidge-jun#3071 (73), lidge-jun#3032 (75), lidge-jun#3026 (75), lidge-jun#3029 (72), lidge-jun#3008 (71), lidge-jun#3019 (70). Sixteen below-bar items are recorded with components so the next scan does not re-litigate them, and lidge-jun#3068 is suppressed as a duplicate of lidge-jun#3071. The scan corrected several assumptions the titles suggested. lidge-jun#1527 and lidge-jun#3070 are already fixed on dev; lidge-jun#3059 asserts an unmount path the tree cannot produce; PRs lidge-jun#3040, lidge-jun#3041 and lidge-jun#3067 each found a real defect and proposed a worse remedy; PRs lidge-jun#3063 and lidge-jun#3038 claim regressions that pass against unfixed source. Eleven adversarial review rounds, all findings verified in-tree before amendment. Findings per round: 9, 5, 4, 4, 3, 2, 3, 3, 1, 0. Round 11 passed. Round 1 found nine holes in the plan; after that the defects were in the fixes, which is what 002-011 mostly record.
|
Closing in favor of #3107, which lands the same fix at a narrower layer. Thank you for the report — the defect is real and #3107 exists because you found it. Two things kept this from being the vehicle. Layer. This strips in Test coverage. For contrast, #3107's tests call the production Triaged in the 2026-08-31 non-priority-70 bug round. |
Problem
Starting with Codex 0.151.0-alpha (shipped to Codex Desktop users on 2026-08-30), the client attaches an OpenAI-backend-only field to Responses input items:
Codex intentionally keeps this field for providers whose name is
openai(codex-rs core/src/client.rsstrips it only for!is_openai()providers). Because opencodex occupies the built-in OpenAI provider slot viaopenai_base_url, the field arrives at opencodex and is forwarded verbatim to routed upstreams. Strict-upstream providers reject the unknown parameter and the turn fails:Confirmed on
combo/joy-openaiandgpt-5.6-sol; every turn (not just resumed sessions) fails with 400.Fix
Mirror the
!is_openai()stripping rule at the proxy boundary by adding a single helper and calling it at every point where a Responses request body is read or rewritten:src/server/responses/internal-request-metadata.ts—stripOpenAiInternalRequestMetadata(body): flat loop overbody.input[]deletinginternal_chat_message_metadata_passthrough, matchingcodex-rs's own normalization for non-OpenAI providers.src/server/responses/core.ts— 2 call sites:readJsonRequestBody(...)(covers direct POST, combo children, and the WebSocket bridge).expandPreviousResponseInput(...)(covers the previous-response replay path).src/server/responses/compact.ts— 1 call site after its ownreadJsonRequestBody(req)(compact bodies are re-serialized viaJSON.stringify({ ...compactBody, model }), which previously leaked the field).Only this one field is stripped.
encrypted_function_args(also cleared by codex-rs for non-OpenAI) is left untouched because routed upstreams either accept it or never see it; if a future provider starts rejecting it, the same helper is the right place to add the deletion.Verification
bun test tests/openai-internal-request-metadata.test.ts— 6/6 pass:extra_content.gemini.thought_signatureand other metadatanull/ non-array / malformedinputwithout throwingbun x tsc --noEmit— clean (exit 0).status=completedwith a valid completion. Confirmed in Codex Desktop 0.151.0-alpha.7.2 that the previously failing session (combo/joy-openai) now completes turns end-to-end.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