fix(responses): preserve caller User-Agent - #4702
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe Responses adapter now preserves the caller’s ChangesResponses User-Agent Preservation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The adapter change preserves the intended User-Agent fallback behavior without identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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 2 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
리뷰 · 우선순위 74 / 80이 PR은 이슈 #4699를 고친다. Codex CLI가 OpenCodex를 거쳐 Responses 호환 게이트웨이(보고 사례는 Volcengine Ark/화산방주)로 나갈 때, 어댑터가 업스트림 헤더를 새로 만들면서호출자 고치는 방법은 좁다. 테스트는 라인 68-74 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Audited every register entry against upstream v2.56.0 (e4a8539) and the upstream dev line, then acted on what the audit found. Four changes: Restore the regression coverage the v2.56.0 merge deleted. The merge resolved tests/codex-integration/codex-metadata-integrity.test.ts by taking the fork side wholesale, which silently discarded twelve upstream tests (upstream had 21, the merged file had 12) while the file still ran green. They covered live code: applyCodexRoutingHint, the mixed-case CODEX_RESPONSES_LITE_HEADER override, stripBracketedModelSuffix alias directions, and the WS/HTTP Lite fallback boundaries. Restored verbatim, and the sync procedure now requires comparing conflicted test files against the upstream version. Rewrite F-002 onto upstream's implementation. Upstream 45cfb04 (lidge-jun#4702) already implements this contract as a single non-credential fallback. The local version instead added "user-agent" to FORWARD_HEADERS and looped that allowlist in the API-key branch, which had two real defects: a configured provider User-Agent was joined with the caller's rather than staying authoritative (`user-agent: operator-agent/1, codex_cli_rs/...` on the wire), and key-mode upstreams received the whole 17-entry forward set including x-oai-attestation and x-codex-turn-metadata. Adopt the upstream helper and remove the entry, so F-002 is absorbed and stops re-conflicting. Drop the duplicated F-003 surface. requireOpencodeManagementToken() was a refactor of behavior upstream already had inline, with sentinels that tested the helper rather than the CLI contract; the two modality tests are covered more strongly upstream. The sentinel now drives cmdOpencode and asserts both the admin-token fetch and the loud failure without one. Rewrite the register itself. Every entry now records a verified upstream disposition (fork-only / absorbed / partially overlapping) with its evidence commit. F-004 loses two paths that carry no fork delta and gains the output-ceiling versus wire-default distinction. F-005 records that its load-bearing case is a pre-v2.35.0 ownership record, measured by disabling the canonicalizer. F-006 is recorded as partially overlapping rather than fork-only, since upstream strips the same marker over the same three tools behind an opt-in flag. F-007 registers the custom-model aria-label. Verified: typecheck, structure:check, privacy:scan, lint:gui, the docs build, and 515 focused tests across the changed areas. Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
2 participants
Summary
User-Agentas a narrow fallback in both key-auth and canonical-forward Responses routesUser-Agentauthoritative, including mixed-case header namesWhy
Some Responses-compatible providers, including Ark-backed compatibility paths, use the Codex client fingerprint to select Codex-specific behavior. The adapter currently rebuilds its upstream headers and drops the caller
User-Agent, so those providers can choose a generic path even though the request came from Codex.Validation
bun test tests/codex-integration/codex-metadata-integrity.test.ts— 24 passedbun run typecheck— passedbun run structure:check— passedbun run privacy:scan— passedcd docs-site && bun run build— passed, 441 pagesgit diff --check— passedAll tests used isolated temporary
HOME,OPENCODEX_HOME, andCODEX_HOMEvalues. No local runtime configuration was read or written.No Go counterpart exists for this TypeScript Responses adapter change on the currently published integration branches.
Closes #4699
Summary by CodeRabbit
User-Agentwhen no provider value is configured.User-Agentvalues take precedence, regardless of header capitalization.User-Agentis added when the caller does not provide one; other caller headers remain unaffected.User-Agenthandling.