Skip to content

fix(responses): strip OpenAI-internal input metadata before routing upstream - #3038

Closed
L-Y-J wants to merge 1 commit into
lidge-jun:devfrom
L-Y-J:fix/strip-openai-internal-request-metadata
Closed

fix(responses): strip OpenAI-internal input metadata before routing upstream#3038
L-Y-J wants to merge 1 commit into
lidge-jun:devfrom
L-Y-J:fix/strip-openai-internal-request-metadata

Conversation

@L-Y-J

@L-Y-J L-Y-J commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

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:

internal_chat_message_metadata_passthrough: { content_item_kinds: [...] }

Codex intentionally keeps this field for providers whose name is openai (codex-rs core/src/client.rs strips it only for !is_openai() providers). Because opencodex occupies the built-in OpenAI provider slot via openai_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:

Provider error 400: {
  "error": {
    "message": "Unknown parameter: 'input[0].internal_chat_message_metadata_passthrough.content_item_kinds'.",
    "type": "invalid_request_error",
    "code": "unknown_parameter"
  }
}

Confirmed on combo/joy-openai and gpt-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:

  • New src/server/responses/internal-request-metadata.tsstripOpenAiInternalRequestMetadata(body): flat loop over body.input[] deleting internal_chat_message_metadata_passthrough, matching codex-rs's own normalization for non-OpenAI providers.
  • src/server/responses/core.ts2 call sites:
    1. After readJsonRequestBody(...) (covers direct POST, combo children, and the WebSocket bridge).
    2. After expandPreviousResponseInput(...) (covers the previous-response replay path).
  • src/server/responses/compact.ts1 call site after its own readJsonRequestBody(req) (compact bodies are re-serialized via JSON.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

  • Unit tests: bun test tests/openai-internal-request-metadata.test.ts — 6/6 pass:
    • strips from message items, preserving every other field
    • strips from every item in a mixed request (message/reasoning/function_call/function_call_output)
    • coexists with extra_content.gemini.thought_signature and other metadata
    • no-op when the field is absent
    • tolerates null / non-array / malformed input without throwing
  • Type check: bun x tsc --noEmit — clean (exit 0).
  • End-to-end: sent a direct POST to a local 2.36.0 proxy carrying the failing field — before the patch it reproduced the exact upstream 400; after the patch the same request returns status=completed with 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:

  • 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

  • Bug Fixes
    • Prevented internal request metadata from being sent to upstream providers that reject unknown fields.
    • Improved compatibility with strict Responses API endpoints, avoiding related 400 errors.
    • Preserved all other request content while safely handling malformed or incomplete input.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added bug Something isn't working review-ready labels Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

  • ✅ 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.

4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2ba1afde-e50a-4675-8a0b-cc618f9620e5

📥 Commits

Reviewing files that changed from the base of the PR and between 4180067 and 65eedf9.

📒 Files selected for processing (4)
  • src/server/responses/compact.ts
  • src/server/responses/core.ts
  • src/server/responses/internal-request-metadata.ts
  • tests/openai-internal-request-metadata.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The Responses API now removes internal_chat_message_metadata_passthrough from request input items before validation and routing. Sanitization runs for compact requests, normal requests, and expanded previous-response input. Tests cover supported item types and malformed inputs.

Changes

Request metadata sanitization

Layer / File(s) Summary
Sanitization helper and endpoint wiring
src/server/responses/internal-request-metadata.ts, src/server/responses/compact.ts, src/server/responses/core.ts
stripOpenAiInternalRequestMetadata removes the internal field from object items in body.input arrays. The compact path sanitizes after JSON parsing at line 491. The core path sanitizes before dispatch at line 2613 and after previous-response expansion at line 2634.
Sanitization behavior tests
tests/openai-internal-request-metadata.test.ts
Tests verify removal across message, reasoning, function-call, and function-call-output items. They also verify preservation of sibling fields and safe handling of absent, empty, and malformed inputs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 65eed

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: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: stripping OpenAI-internal input metadata before routing Responses API requests upstream.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 75 / 80

이 PR은 Codex Desktop 0.151.0-alpha 이후부터 생기는 전 턴 실패를 막는 경계 정리다. 지금 dev HEAD는 4180067b4 (#3036 docs-only wp2 마감)이고, 제품 쪽에서는 #3035로 Plus 계정의 gpt-5.6를 되돌린 직후다. 그 위에 Codex 클라이언트가 Responses input[] 항목에 OpenAI 백엔드 전용 필드 internal_chat_message_metadata_passthrough (안에 content_item_kinds)를 붙이기 시작했다. codex-rs core/src/client.rs는 제공자 이름이 openai가 아닐 때만 이 필드를 지운다. opencodex는 openai_base_url로 그 openai 슬롯을 차지하기 때문에 필드가 프록시까지 그대로 들어온다. 프록시가 라우팅한 엄격한 업스트림(본문 기준 combo/joy-openai, gpt-5.6-sol)은 모르는 파라미터라며 400 unknown_parameter를 돌려준다. 이어하기만이 아니라 매 턴이 깨진다.

고침은 한 헬퍼로 그 규칙을 프록시 입구에서 한 번 더 적용하는 것이다. 새 파일 src/server/responses/internal-request-metadata.tsstripOpenAiInternalRequestMetadata(body)body.input이 배열일 때만 평평하게 돌면서 항목 객체의 그 키를 delete한다. null/비배열/깨진 항목은 조용히 넘어가서 요청 파싱을 추가 실패로 바꾸지 않는다. 호출은 세 곳이다. src/server/responses/core.tshandleResponsesInner에서 readJsonRequestBody 직후(직접 POST, 콤보 분기 전, 자식 재진입, WS→HTTP와 같은 본문 읽기 경로), 그리고 expandPreviousResponseInput 직후(이전 응답 재생이 저장해 둔 항목을 앞에 붙일 때 필드가 다시 살아날 수 있어서). src/server/responses/compact.ts에서도 본문 읽은 뒤 한 번 더 지운다. 컴팩트는 나중에 JSON.stringify({ ...compactBody, model })로 다시 직렬화하므로, 여기서 안 지우면 컴팩트 업스트림으로 그대로 새어 나간다.

콤보 경로는 조금 다르게 닫힌다. 부모 쪽에서 첫 strip이 끝난 뒤 handleComboResponses로 들어가고, 그 안에서 expandPreviousResponseInput(대략 core.ts 2124 근처)가 저장 항목을 붙일 수 있다. 그 확장 직후에는 이 PR이 strip을 한 번 더 넣지 않는다. 대신 자식 요청이 JSON.stringify(childBody)로 다시 handleResponsesInner에 들어가며 첫 strip을 한 번 더 탄다. 지금 구조에서는 업스트림까지 필드가 가지 않는다. 다만 “확장 직후 명시 strip”이 아니라 “자식 재진입에 맡긴다”는 계약이라, 나중에 확장된 body를 재진입 없이 밖으로 보내는 경로가 생기면 구멍이 된다.

테스트 tests/openai-internal-request-metadata.test.ts 6개는 헬퍼만 본다. 메시지 항목에서 필드만 제거되고 나머지 유지, 섞인 타입(message/reasoning/function_call/function_call_output) 전부 제거, extra_content.gemini.thought_signature 같은 다른 메타는 보존, 필드 없을 때 no-op, 깨진 input에서 throw 없음, 빈 배열. 호출 지점 통합 테스트는 없다. 본문은 로컬 e2e(패치 전 400 → 후 completed)와 Desktop 0.151.0-alpha.7.2 세션 복구를 적었다. encrypted_function_args는 codex-rs가 non-openai에서 같이 지우는 다른 필드인데, 이 PR은 의도적으로 건드리지 않았다. 지금 라우팅 업스트림이 거절하지 않거나 안 본다는 전제다. types.ts/config.ts 분할 캠페인과는 무관해서 닫을 대상이 아니고, 미리보기 배포도 계획에 없다. bug + review-ready 라벨이 이미 붙어 있고 체크리스트 4/4다. CI는 enforce-target/hygiene/label은 통과, resolve-pr과 CodeRabbit은 이 시각에 아직 pending이다.

src/server/responses/internal-request-metadata.ts - 헬퍼는 input 배열의 최상위 항목만 지운다. 중첩 content 안의 동명 필드는 범위 밖이다(현 Codex 부착 위치와 맞음).
src/server/responses/core.ts handleResponsesInner 첫 strip - 콤보 분기보다 앞에 두어 부모·자식·직접 POST를 같이 덮는다. 위치는 맞다.
src/server/responses/core.ts expandPreviousResponseInput 직후 둘째 strip - 저장 상태(state.items)에 예전 필드가 남아 있으면 재생 때 다시 붙는다. 둘째 strip이 그걸 막는다.
src/server/responses/core.ts handleComboResponses 확장(약 2124) - 확장 직후 명시 strip이 없고 자식 재진입 strip에 의존한다. 지금은 안전하지만 계약이 한 단계 간접적이다.
src/server/responses/compact.ts - 컴팩트 본문 재직렬화 전에 지운다. 이 경로를 빠뜨리면 컴팩트만 400이 난다.
경로 encrypted_function_args - 의도적 미처리. 나중에 엄격 업스트림이 거절하면 같은 헬퍼에 키를 추가하면 된다.
tests/openai-internal-request-metadata.test.ts - 헬퍼 단위만 있다. core/compact 호출 지점을 한 줄이라도 고정하면 회귀가 더 빨리 보인다.

메인테이너의 판단이 필요한 지점

  • resolve-pr / CodeRabbit이 초록이 될 때까지 기다릴지, 로컬·본문 e2e가 충분하니 CI만 보고 바로 머지할지
  • handleComboResponses 확장 직후에도 strip을 한 줄 더 넣을지(자식 재진입에만 맡기지 않기)
  • encrypted_function_args 를 지금 같이 지울지, 거절 사례가 나온 뒤에 헬퍼에 추가할지
  • 이 증상을 추적하는 이슈가 있으면 이 PR로 닫을지(검색상 전용 이슈는 바로 안 보임)

너의 추천

CI(resolve-pr)만 초록 확인되면 머지하세요. Desktop 0.151 사용자에게 매 턴 400이 나는 실사용 차단이라 우선순위가 높다. 헬퍼·세 호출점·단위 테스트 방향은 dev 응답 경로와 맞다. 여유 있으면 콤보 확장 직후 strip 한 줄을 같은 PR이나 바로 이어서 넣고, encrypted_function_args는 거절 사례가 나오기 전에는 그대로 두세요. 라벨은 바꾸지 않습니다. types.ts/config.ts 분할로 닫을 대상이 아니고 미리보기 배포도 없습니다.

이 댓글은 grok-bot이 작성했습니다

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

x3M3x pushed a commit to x3M3x/opencodex that referenced this pull request Aug 31, 2026
…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.
@lidge-jun

Copy link
Copy Markdown
Owner

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 core.ts and compact.ts after readJsonRequestBody, unconditionally, so it also removes the field on the canonical ChatGPT forward path — where it is not foreign. isCanonicalOpenAiForwardProvider (src/providers/openai-tiers.ts:34-37) marks exactly that destination, and #3107 strips inside the adapter's existing if (!isCanonicalOpenAiForwardProvider(provider)) block instead.

Test coverage. tests/openai-internal-request-metadata.test.ts imports stripOpenAiInternalRequestMetadata and calls it directly; it never drives handleResponses, compact, or an adapter buildRequest. Deleting both production call sites leaves that file green, so it cannot fail if the fix stops running.

For contrast, #3107's tests call the production buildRequest, and both directions are mutation-verified: removing the strip fails noncanonical Responses destinations strip Codex-private item metadata, and moving it outside the guard fails canonical ChatGPT forward preserves Codex-private item metadata.

Triaged in the 2026-08-31 non-priority-70 bug round.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants