Skip to content

fix(responses): preserve caller User-Agent for provider compatibility - #4712

Closed
RailyW wants to merge 1 commit into
lidge-jun:devfrom
RailyW:fix/responses-client-user-agent
Closed

RailyW wants to merge 1 commit into
lidge-jun:devfrom
RailyW:fix/responses-client-user-agent

Conversation

@RailyW

@RailyW RailyW commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve the caller's User-Agent when forwarding Responses requests. Some Responses-compatible providers, including Volcengine Ark in the reported case, use this header to recognize Codex and select client-specific compatibility handling. Dropping it lets Bun's default identity reach the upstream, preventing that handling from being selected.

Closes #4699.

Changes

  • Retain user-agent in the shared forwarding allowlist so client identity survives auth materialization and internal HTTP/WebSocket bridges.
  • Use the incoming User-Agent as a fallback in both Responses API-key and forward modes.
  • Keep an explicitly configured provider User-Agent authoritative, regardless of header-name casing, without adding a duplicate header.
  • Apply the same precedence to the Responses-based search and vision executors that consume the shared allowlist.
  • Document the forwarding behavior and provider-header precedence in the provider reference and architecture documentation.

Compatibility

Requests without a caller User-Agent do not acquire a fabricated Codex identity. Explicit provider overrides, including empty values, remain authoritative. Credential selection, authorization precedence, and forwarding of other headers retain their existing behavior. No new configuration option is required.

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

  • New Features

    • Responses requests preserve the incoming User-Agent in API-key and forwarding modes.
    • Explicitly configured provider User-Agent headers take precedence, regardless of capitalization.
    • Responses search and vision requests follow the same identity-handling behavior.
    • Requests without an incoming or configured User-Agent retain the existing default behavior.
  • Documentation

    • Added guidance covering User-Agent forwarding, provider precedence, and related configuration behavior across Responses integrations and internal request paths.

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview 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: Advanced

Run ID: 1bd38f7e-6730-45e5-adfb-6a44fa42a6cd

📥 Commits

Reviewing files that changed from the base of the PR and between fa3431e and 2d5cedb.

📒 Files selected for processing (7)
  • structure/adapters/registry.md
  • structure/data-planes/inbound-compat.md
  • structure/gui-and-management-api.md
  • structure/runtime.md
  • structure/transports/byte-accounting.md
  • structure/transports/inventory.md
  • structure/transports/responses.md

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


📝 Walkthrough

Walkthrough

The Responses transport now preserves incoming User-Agent headers when no provider override exists. Provider headers take precedence case-insensitively. Search and vision forwarding use the same rule. Tests and documentation cover the contract.

Changes

Responses User-Agent forwarding

Layer / File(s) Summary
Adapter forwarding and precedence
src/adapters/openai-responses/passthrough.ts, tests/responses/responses-user-agent.test.ts
The adapter skips direct User-Agent copying during forward authentication. It applies the incoming value only when no provider header exists, using case-insensitive matching. Tests cover API-key, forward, custom forward, missing headers, empty overrides, and received upstream headers.
Search and vision forwarding
src/web-search/executor.ts, src/vision/describe.ts, tests/responses/responses-user-agent.test.ts
The search and vision paths preserve provider-configured User-Agent values regardless of casing. Sidecar tests verify caller and provider values over loopback connections.
Contract documentation and test registration
docs-site/src/content/docs/reference/configuration/providers.md, structure/**/*.md, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
The documentation defines User-Agent precedence, preservation, scope, and the absence of a fabricated identity. The new test is registered in the Responses layout mappings.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 2d5ce

The forwarding change covers caller identity and explicit provider overrides without an established production-impacting defect; it is suitable to merge under normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (7 skipped: 7… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #4699 requires caller User-Agent preservation, case-insensitive provider overrides, empty-value precedence, no fabricated identity, API-key and forward coverage, and wire-level coverage. `src/…
Out of Scope Changes check ✅ Passed The source changes remain connected to Issue #4699. They update Responses forwarding and the related search and vision executors that use the same header contract. The regression tests verify the requ…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving the caller's User-Agent in Responses requests for provider compatibility. It matches the implementation and stated objectives.
Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (7 skipped: 7 unsupported.)

  • 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 Sep 15, 2026
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

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

What to do

  • Tick all four boxes in the PR description once you're done (currently 2/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.

2/4 boxes ticked.

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

@RailyW
RailyW marked this pull request as ready for review September 15, 2026 09:43
@RailyW

RailyW commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@github-actions
github-actions Bot marked this pull request as draft September 15, 2026 09:44
@RailyW
RailyW force-pushed the fix/responses-client-user-agent branch from fa3431e to 2d5cedb Compare September 15, 2026 09:47
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

이 PR은 이슈 #4699를 고친다. Codex CLI가 OpenCodex를 거쳐 Responses 호환 게이트웨이(보고 사례는 Volcengine Ark)로 나갈 때, src/adapters/openai-responses/passthrough.tsbuildRequest가 업스트림 헤더를 새로 만들면서 호출자 User-Agent를 버린다. 그러면 Bun fetch가 기본값 Bun/<version>을 넣을 수 있고, UA로 Codex 호환 경로를 고르는 공급자는 일반 경로로 떨어져 요청이 실패할 수 있다. 지금 dev HEAD는 51d577c3f(#4700, 패키지 2.57.0)이고, tip의 passthrough.ts에는 아직 UA 폴백이 없다. key 모드는 Content-Type·Bearer·provider.headers만 넣고, forward 모드는 좁은 FORWARD_HEADERS만 복사한다. 그 목록에도 user-agent는 없다.

같은 버그를 고치는 열린 PR이 이미 있다. #4702(Ingwannu)는 draft가 아니고, 베이스가 현재 dev exact tip이며, 이전에 grok-bot이 74/80으로 리뷰했고 호스티드 CI 그린 후 머지를 추천했다. #4702는 FORWARD_HEADERS를 늘리지 않고 applyCallerUserAgentFallback 한 함수만 key/forward 조립 뒤에 호출한다. 이번 #4712는 반대로 FORWARD_HEADERSuser-agent를 넣은 뒤, passthrough forward 루프에서는 다시 건너뛰고(if (h === "user-agent") continue), 마지막에 호출자 UA 폴백을 따로 넣는다. 효과는 비슷하지만 allowlist에 넣었다가 본선에서 빼는 형태라 읽기 어렵다. 대신 search/vision 사이드카(src/web-search/executor.ts, src/vision/describe.ts)는 FORWARD_HEADERS를 그대로 순회하므로, 목록에 UA가 생기면 그쪽도 함께 살아난다. #4702만 머지하면 본선 Responses는 고쳐지고 search/vision은 여전히 빠질 수 있다. 그게 이 PR의 유일한 추가 가치다.

테스트는 tests/responses/responses-user-agent.test.ts를 새로 두고 key/forward·대소문자 override·빈 override·로컬 HTTP 수신기·search/vision까지 넓게 잡는다. 문서도 docs-site/.../providers.mdstructure/transports/responses.md뿐 아니라 overview·runtime·cursor·inventory 등 structure 여러 장에 같은 문장을 뿌렸다. 동작 설명은 맞지만 산포가 크다. 베이스는 dev, MERGEABLE, draft이며 readiness checklist는 0/4다. types/config 스플릿·고드파일 모놀리스 경로는 건드리지 않는다. 호스티드 CI는 enforce-target/hygiene/label은 pass, CodeRabbit은 이 웨이크 시점 pending이다.

라인 src/adapters/openai-responses/passthrough.ts FORWARD_HEADERS - user-agent를 목록에 추가한 뒤 forward 루프에서 다시 skip. #4702의 단일 fallback 함수보다 경로가 꼬인다.
라인 src/adapters/openai-responses/passthrough.ts caller UA 폴백 - callerUserAgent !== null이면 빈 문자열도 넣는다. #4702는 truthy일 때만 넣는다. 빈 UA 계약을 메인테이너가 골라야 한다.
경로 src/web-search/executor.ts / src/vision/describe.ts - FORWARD_HEADERS 확장 덕분에 사이드카도 UA를 실을 수 있다. 이건 #4702에 없는 범위다.
경로 structure/* 다수 - 같은 UA 문단을 여러 장에 복붙. 유지비 대비 이득이 작다.
경로 PR 상태 - draft + checklist 0/4. 같은 이슈를 닫는 #4702가 이미 ready다.

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

너의 추천
#4702를 우선 머지 후보로 두고, 이 PR(#4712)은 중복으로 닫는 쪽을 추천한다. search/vision까지 같이 살리려면 #4702에 FORWARD_HEADERS 확장(또는 사이드카 전용 fallback)만 짧게 이식하고, structure 산포 문서는 넣지 말자. 지금 상태에서 이 draft를 rebase해 경쟁시킬 필요는 없다. Closes #4699는 살아남는 한쪽 PR에만 남긴다.

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

@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@lidge-jun

Copy link
Copy Markdown
Owner

Closing as a duplicate of #4702, which is now merged into dev as 45cfb04e9757a5a257ab6290d9f24d2ea0bc7573.

Both PRs preserve the caller's User-Agent through the Responses passthrough, and both change src/adapters/openai-responses/passthrough.ts. The reason #4702 was taken is scope rather than quality: it is a 4-file diff confined to the passthrough and its test, and it already carried a green aggregate ci at its exact head. This PR is 18 files and additionally changes src/vision/describe.ts and src/web-search/executor.ts, which is a separate behavioural claim about how the vision describer and the web-search executor identify themselves upstream.

That second half is not rejected — it is unreviewed. If you want it, please open it as its own PR against dev now that the passthrough fix has landed. The diff should be small against the new dev, and it will get a review on its own merits instead of riding a User-Agent fix.

If any part of this branch is later carried by a maintainer rather than reopened by you, the carrying commit must include a Co-authored-by: RailyW trailer. Per AGENTS.md, naming a contributor in prose is not credit — the trailer is what GitHub reads for the contributor graph, and CREDITS.md exists because 27 earlier landings were done without it.

Thanks for the fix.

@lidge-jun lidge-jun closed this Sep 15, 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants