Skip to content

fix(responses): strip Codex-private item metadata - #3066

Closed
yanzhibo-bytedance wants to merge 2 commits into
lidge-jun:devfrom
yanzhibo-bytedance:fix/strip-codex-private-metadata
Closed

fix(responses): strip Codex-private item metadata#3066
yanzhibo-bytedance wants to merge 2 commits into
lidge-jun:devfrom
yanzhibo-bytedance:fix/strip-codex-private-metadata

Conversation

@yanzhibo-bytedance

@yanzhibo-bytedance yanzhibo-bytedance commented Aug 31, 2026

Copy link
Copy Markdown

Summary

  • Strip Codex's ChatGPT-internal internal_chat_message_metadata_passthrough field from Responses input items before forwarding to noncanonical destinations.
  • Preserve the field for canonical ChatGPT forwarding and avoid mutating the caller-owned raw request body.
  • Cover custom gateway model IDs such as openai/gpt-5.6-sol, where Codex retains its built-in OpenAI provider identity before OpenCodex routes the request.
  • Addresses the proxy-side compatibility gap described in Unknown parameter: 'input[0].internal_chat_message_metadata_passthrough'. openai/codex#30161.

Verification

  • bun test tests/openai-responses-passthrough.test.ts — 115 pass, 0 fail.
  • bun run typecheck — passed.
  • bun run privacy:scan — passed.
  • bun run test — 16,533 pass, 14 skip; three unrelated 5-second load-sensitive timeouts under the local parallel run. Each timed-out assertion passed when rerun serially with a diagnostic timeout (10–30 seconds).

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (Not needed: no user-facing configuration or UI change.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

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
    • Removed internal metadata from requests sent through routed Responses destinations.
    • Preserved the original request body without modification.
    • Retained private metadata for the canonical ChatGPT route.

Remove internal ChatGPT item metadata before forwarding Responses requests to noncanonical destinations.
Exercise key and forwarded noncanonical routes while preserving canonical ChatGPT passthrough behavior.
@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: a75af31c-54fa-438c-b301-a05389f325bf

📥 Commits

Reviewing files that changed from the base of the PR and between 93b7ee8 and 2e15e4b.

📒 Files selected for processing (2)
  • src/adapters/openai-responses.ts
  • tests/openai-responses-passthrough.test.ts

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


📝 Walkthrough

Walkthrough

The Responses request builder now strips internal chat metadata for non-canonical destinations. Canonical ChatGPT forwarding preserves the metadata. Tests cover both behaviors and confirm that the raw request body remains unchanged.

Changes

Responses metadata filtering

Layer / File(s) Summary
Request filtering and routing
src/adapters/openai-responses.ts
Adds a copy-on-write helper that removes internal_chat_message_metadata_passthrough from input items. buildRequest applies it only for non-canonical destinations before client-loaded tool promotion.
Route behavior validation
tests/openai-responses-passthrough.test.ts
Verifies metadata removal for key-auth and forward-auth routes, preservation of the original rawBody, and metadata preservation for the canonical ChatGPT forward route.

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

Merge Risk: ⚪ Minimal · up to 2e15e

The PR removes private metadata from noncanonical forwarding while preserving canonical behavior and avoiding mutation of caller input. 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 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: removing Codex-private metadata from Responses requests.
  • 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.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 31, 2026
@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

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.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 60 / 80

이 PR은 Codex가 설정 프로바이더 이름을 openai로 유지한 채 OpenCodex 루프백으로 보낼 때, 입력 아이템에 붙는 ChatGPT 전용 필드 internal_chat_message_metadata_passthrough를 비정규(noncanonical) Responses 목적지로 넘기기 전에 떼어 내는 수정입니다. 지금 dev HEAD는 f83368dfd(#3057 entitlement tri-state)이고, HEAD의 src/adapters/openai-responses.ts에는 아직 이 strip이 없습니다. 게이트웨이·커스텀 Responses 업스트림은 화이트리스트가 빡세서 이 필드를 unknown_parameter로 400 내는 경우가 있고, openai/codex#30161 쪽 호환 구멍과 맞습니다. 이 브랜치는 stripInternalChatMessageMetadataPassthrough를 어댑터 쪽에 두고, !isCanonicalOpenAiForwardProvider(provider)일 때만 outBody에 적용합니다. 호출자가 넘긴 _rawBody는 바꾸지 않고, 직렬화할 복사본만 고칩니다. 테스트는 tests/openai-responses-passthrough.test.ts에 비정규 key/forward 두 경로에서 필드가 사라지고, 정규 ChatGPT forward에서는 남는 경우를 넣었습니다. draft이고 readiness 체크리스트는 아직 비어 있습니다.

같은 필드를 다루는 열린 PR이 이미 있습니다. #3038(L-Y-J, ready/non-draft)은 src/server/responses/internal-request-metadata.ts를 새로 만들고 core.ts/compact.ts에서 요청 body를 항상 in-place로 지웁니다. 서버 입구에서 한 번에 걷어 내서 combo·chat·anthropic replay·compact까지 같이 커버하려는 설계입니다. 반면 이 PR(#3066)은 어댑터의 noncanonical 경계에서만 지우고 정규 ChatGPT 포워딩은 필드를 남깁니다. Codex가 정규 백엔드에 그 메타를 기대한다면 #3038의 무조건 strip이 과도할 수 있고, 반대로 서버 입구에서 안 지우면 어댑터를 안 타는 경로(예: compact 전용, 다른 wire)에 구멍이 남을 수 있습니다. 둘 다 머지하면 이중 처리가 되고, 한쪽만 고르면 다른 쪽은 닫는 편이 맞습니다. types.ts/config.ts 스플릿과는 무관합니다.

라인 - src/adapters/openai-responses.ts의 strip은 message/function_call 등 input 배열 아이템의 해당 키만 지움. nested content 안의 동명 필드는 안 봄(현재 Codex wire는 아이템 최상위라서 보통 충분).

라인 - 정규 보존 테스트는 provider 픽스처(테스트 파일 기존 canonical forward)에 의존. 픽스처 baseUrl/auth가 HEAD에서 여전히 canonical로 판정되는지 한 번 더 확인할 것.

라인 - draft + readiness 네 칸 미체크. CI·최신 dev rebase·CodeRabbit 정리 전에는 머지 대상이 아님.

라인 - #3038과 목표 필드가 동일. 레이어만 다름(서버 전역 mutate vs 어댑터 noncanonical). 중복으로 보이면 하나를 landed-via로 정리해야 함.

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

너의 추천
draft를 ready로 올리기 전에 #3038과 한 줄로 합의하라. 정규 보존이 필요하면 #3038에 canonical 예외를 넣거나 #3066을 채택하고 #3038을 닫아라. 전역 strip이 충분하면 #3038을 먼저 랜딩하고 이 PR은 Landed via #3038 패턴으로 닫아라. 지금은 draft 유지, rebase onto 최신 dev(f83368dfd)만 해 두라.

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

@yanzhibo-bytedance
yanzhibo-bytedance marked this pull request as ready for review August 31, 2026 10:32
@github-actions
github-actions Bot marked this pull request as draft August 31, 2026 10:32
@yanzhibo-bytedance
yanzhibo-bytedance marked this pull request as ready for review August 31, 2026 10:32
@github-actions
github-actions Bot marked this pull request as draft August 31, 2026 10:33
@lidge-jun

Copy link
Copy Markdown
Owner

Superseded by #3107, which is your two commits rebased onto current dev with authorship intact.

This was chosen over #3038, which found the same defect. The deciding difference is layer: #3038 strips in core.ts/compact.ts unconditionally, including on the canonical ChatGPT forward where the field is not foreign, and its tests call the helper directly so they stay green with both production call sites deleted. Yours strips inside the existing isCanonicalOpenAiForwardProvider guard and its tests drive buildRequest.

Mutation, both directions: removing the strip fails the strip test; moving it outside the guard fails the ChatGPT preservation test. Two guards, independently load-bearing — which is exactly the property the other approach could not assert.

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

@lidge-jun

Copy link
Copy Markdown
Owner

Landed via #3107 at e9d198a

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 1, 2026
@lidge-jun lidge-jun closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants