fix: sync codex responses passthrough fields - #6018
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds Codex request and SSE response-header passthrough support, expands OpenAI Responses and compaction DTO parity, preserves Codex metadata through conversion, and maps cache-write token usage details across relay handlers. ChangesCodex and Responses integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Codex
participant ChannelAffinity
participant ParamOverride
participant UpstreamResponses
participant StreamScanner
participant Client
Codex->>ChannelAffinity: send request and session headers
ChannelAffinity->>ParamOverride: apply pass_headers and pass_sse_headers
ParamOverride->>UpstreamResponses: forward configured request headers
UpstreamResponses-->>StreamScanner: return SSE response and headers
StreamScanner->>Client: stream events and selected SSE headers
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
relay/common/override.go (1)
1305-1389: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider a deny-list for sensitive headers before allowing
pass_response_headerswildcards.
shouldCopyResponseHeaderPassThrough(inrelay/helper/stream_scanner.go) allows a trailing*to match any header name, so a template like"*"or"set-*"would let an admin forward every upstream header — including things likeSet-Cookieor internal auth headers — straight to the end client. A small deny-list (e.g.set-cookie,authorization,www-authenticate) enforced inparseHeaderPassThroughNamesorGetResponseHeaderPassThroughwould guard against accidental credential/session leakage from a broad admin misconfiguration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/common/override.go` around lines 1305 - 1389, Add a deny-list guard for sensitive response headers before accepting wildcard pass-through patterns. The issue is in `parseHeaderPassThroughNames`/`GetResponseHeaderPassThrough`, where broad values like `*` or `set-*` can later be matched by `shouldCopyResponseHeaderPassThrough` and leak headers such as `Set-Cookie` or `Authorization`. Update the parsing/validation path to reject or filter reserved sensitive names and wildcard patterns that would match them, so only safe headers are allowed through.relay/common/override_test.go (1)
1394-1423: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse testify
require/assertinstead oft.Fatalfandreflect.DeepEqual.Per the project's test guidelines, new Go backend tests must use
requirefor fatal assertions andassertfor non-fatal value checks. This new test usest.Fatalfandreflect.DeepEqualinstead.♻️ Proposed refactor to use testify
func TestApplyParamOverridePassResponseHeadersNoopsRequestBody(t *testing.T) { info := &RelayInfo{ RequestHeaders: map[string]string{}, ChannelMeta: &ChannelMeta{ ParamOverride: map[string]interface{}{ "operations": []interface{}{ map[string]interface{}{ "mode": "pass_response_headers", "value": []interface{}{"OpenAI-Model", "X-Codex-*"}, }, }, }, }, } input := []byte(`{"model":"gpt-5"}`) out, err := ApplyParamOverrideWithRelayInfo(input, info) - if err != nil { - t.Fatalf("ApplyParamOverrideWithRelayInfo returned error: %v", err) - } - if string(out) != string(input) { - t.Fatalf("pass_response_headers should not modify request body, got %s", string(out)) - } + require.NoError(t, err) + require.Equal(t, string(input), string(out), "pass_response_headers should not modify request body") got := GetResponseHeaderPassThrough(info) want := []string{"openai-model", "x-codex-*"} - if !reflect.DeepEqual(got, want) { - t.Fatalf("unexpected response header passthrough names, got %#v want %#v", got, want) - } + require.Equal(t, want, got, "unexpected response header passthrough names") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/common/override_test.go` around lines 1394 - 1423, The new test in TestApplyParamOverridePassResponseHeadersNoopsRequestBody should follow the project’s testify conventions instead of using t.Fatalf and reflect.DeepEqual. Update the fatal setup/error checks in ApplyParamOverrideWithRelayInfo to use require, and replace the value comparison for GetResponseHeaderPassThrough with assert. Keep the test behavior the same while using testify helpers consistently throughout the test.Source: Coding guidelines
web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx (1)
323-336: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider sharing Codex header constants and template builder across default-web features.
CODEX_CLI_RESPONSE_HEADER_PASSTHROUGH_HEADERS(lines 330-336) and the inlineCODEX_CLI_HEADER_PASSTHROUGH_TEMPLATE(lines 360-372) duplicate the same data and logic asweb/default/src/features/system-settings/general/channel-affinity/constants.ts(lines 53-59, 89-103). If either allowlist changes, both copies must be updated in sync. Consider extracting these to a shared module and importing from both feature directories.This follows the pre-existing duplication pattern for
CODEX_CLI_HEADER_PASSTHROUGH_HEADERS, so it's not a regression — but it's worth addressing to prevent drift.Also applies to: 360-372
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx` around lines 323 - 336, The Codex response header allowlist and header template logic are duplicated in param-override-editor-dialog and the channel-affinity constants, so they can drift out of sync. Extract the shared header constants and template builder into a common module, then import and reuse them from both the dialog code and the system-settings channel-affinity code, using the existing symbols CODEX_CLI_RESPONSE_HEADER_PASSTHROUGH_HEADERS and CODEX_CLI_HEADER_PASSTHROUGH_TEMPLATE as the migration targets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@relay/common/override.go`:
- Around line 443-464: GetResponseHeaderPassThrough is currently collecting all
pass_response_headers entries by Mode without honoring Conditions, which can
leak headers from rules that should not apply. Update the header collection path
to use the same condition evaluation as applyOperations/checkConditions, ideally
by threading the evaluated context through GetResponseHeaderPassThrough or
reusing the filtered result from ApplyParamOverride so only condition-passing
names are returned.
In
`@web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx`:
- Line 117: The new `pass_response_headers` placeholder in
`getModeValuePlaceholder()` is still hardcoded and not language-reactive. Add a
locale key for this placeholder and wrap the returned value in `t()` so the
`placeholder` prop in `ParamOverrideEditorDialog` updates with the active
language, consistent with `OPERATION_MODE_OPTIONS`, `MODE_DESCRIPTIONS`, and
`getModeValueLabel()`.
---
Nitpick comments:
In `@relay/common/override_test.go`:
- Around line 1394-1423: The new test in
TestApplyParamOverridePassResponseHeadersNoopsRequestBody should follow the
project’s testify conventions instead of using t.Fatalf and reflect.DeepEqual.
Update the fatal setup/error checks in ApplyParamOverrideWithRelayInfo to use
require, and replace the value comparison for GetResponseHeaderPassThrough with
assert. Keep the test behavior the same while using testify helpers consistently
throughout the test.
In `@relay/common/override.go`:
- Around line 1305-1389: Add a deny-list guard for sensitive response headers
before accepting wildcard pass-through patterns. The issue is in
`parseHeaderPassThroughNames`/`GetResponseHeaderPassThrough`, where broad values
like `*` or `set-*` can later be matched by
`shouldCopyResponseHeaderPassThrough` and leak headers such as `Set-Cookie` or
`Authorization`. Update the parsing/validation path to reject or filter reserved
sensitive names and wildcard patterns that would match them, so only safe
headers are allowed through.
In
`@web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx`:
- Around line 323-336: The Codex response header allowlist and header template
logic are duplicated in param-override-editor-dialog and the channel-affinity
constants, so they can drift out of sync. Extract the shared header constants
and template builder into a common module, then import and reuse them from both
the dialog code and the system-settings channel-affinity code, using the
existing symbols CODEX_CLI_RESPONSE_HEADER_PASSTHROUGH_HEADERS and
CODEX_CLI_HEADER_PASSTHROUGH_TEMPLATE as the migration targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f69cd023-c4e1-4ea0-bafe-2b4bba08bc1a
📒 Files selected for processing (20)
relay/channel/codex/adaptor.gorelay/channel/gemini/relay_responses.gorelay/channel/openai/adaptor.gorelay/channel/openai/chat_via_responses.gorelay/channel/openai/relay_responses.gorelay/channel/openai/relay_responses_compact.gorelay/channel/openai/responses_via_chat.gorelay/common/override.gorelay/common/override_test.gorelay/helper/stream_scanner.gorelay/helper/stream_scanner_test.goservice/channel_affinity_template_test.gosetting/operation_setting/channel_affinity_setting.goweb/classic/src/components/table/channels/modals/ParamOverrideEditorModal.jsxweb/classic/src/components/table/usage-logs/modals/ParamOverrideModal.jsxweb/classic/src/constants/channel-affinity-template.constants.jsweb/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsxweb/default/src/features/system-settings/general/channel-affinity/constants.tsweb/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsxweb/default/src/features/usage-logs/lib/format.ts
✅ Files skipped from review due to trivial changes (1)
- web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- service/channel_affinity_template_test.go
| { label: 'Return Custom Error', value: 'return_error' }, | ||
| { label: 'Prune Object Items', value: 'prune_objects' }, | ||
| { label: 'Pass Through Headers', value: 'pass_headers' }, | ||
| { label: 'Pass Through Response Headers', value: 'pass_response_headers' }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how OPERATION_MODE_OPTIONS labels and MODE_DESCRIPTIONS are rendered
# in the JSX to determine if t() is applied at the call site.
rg -n "OPERATION_MODE_OPTIONS\|MODE_DESCRIPTIONS\|getModeValueLabel\|getModeValuePlaceholder" \
web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx \
| head -40Repository: QuantumNous/new-api
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx"
echo "== outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== relevant snippets =="
nl -ba "$FILE" | sed -n '1,260p'
echo
nl -ba "$FILE" | sed -n '260,420p'
echo
nl -ba "$FILE" | sed -n '420,760p'Repository: QuantumNous/new-api
Length of output: 4029
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx"
echo "== imports and option/description definitions =="
sed -n '1,260p' "$FILE" | cat -n
echo
echo "== helper usages and JSX =="
rg -n -C 4 "getModeValueLabel|getModeValuePlaceholder|MODE_DESCRIPTIONS|OPERATION_MODE_LABEL_MAP|useTranslation|t\(" "$FILE"Repository: QuantumNous/new-api
Length of output: 50375
Wrap the new pass_response_headers placeholder in t().
OPERATION_MODE_OPTIONS, MODE_DESCRIPTIONS, and getModeValueLabel() already go through t() at render time, but getModeValuePlaceholder('pass_response_headers') is still passed straight to placeholder, so this new user-facing string remains hardcoded. Add a locale key and translate it here to keep the field language-reactive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx`
at line 117, The new `pass_response_headers` placeholder in
`getModeValuePlaceholder()` is still hardcoded and not language-reactive. Add a
locale key for this placeholder and wrap the returned value in `t()` so the
`placeholder` prop in `ParamOverrideEditorDialog` updates with the active
language, consistent with `OPERATION_MODE_OPTIONS`, `MODE_DESCRIPTIONS`, and
`getModeValueLabel()`.
Source: Coding guidelines
6c48462 to
53a729e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
relay/helper/stream_scanner_test.go (1)
103-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInitialize the affinity fixture explicitly.
Lines 109-112 rely on ambient channel-affinity configuration and ignore selection success. Create scoped rule/cache state and assert the affinity lookup succeeds before applying the template; this keeps the parallel test deterministic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@relay/helper/stream_scanner_test.go` around lines 103 - 117, Make TestStreamScannerHandler_CopiesCodexResponseHeaders initialize its channel-affinity rule and cache state explicitly within the test scope, rather than relying on ambient configuration. Assert that GetPreferredChannelByAffinity succeeds and returns the expected channel before calling ApplyChannelAffinityOverrideTemplate, preserving deterministic behavior under t.Parallel.Source: Coding guidelines
web/classic/src/components/table/channels/modals/ParamOverrideEditorModal.jsx (1)
1081-1088: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStatic-analysis "missing key" hint at 2166-2179 is a false positive.
The mapped list root (the outer
div) already carrieskey={operation.id}; the flagged nested<Tag>doesn't need its own key. The i18n-wrapping refactor itself (operationModeOptions, translated tag labels/descriptions) looks correct and consistent.Also applies to: 2034-2034, 2173-2179, 2255-2255, 2289-2289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/classic/src/components/table/channels/modals/ParamOverrideEditorModal.jsx` around lines 1081 - 1088, The static-analysis missing-key warnings are false positives: preserve the existing outer list key, such as operation.id, and do not add redundant keys to nested Tag elements. Keep the operationModeOptions useMemo translation and related translated labels/descriptions unchanged.Source: Linters/SAST tools
web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx (1)
298-358: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCodex header allowlists are duplicated across two web/default files.
CODEX_CLI_HEADER_PASSTHROUGH_HEADERSandCODEX_CLI_SSE_HEADER_PASSTHROUGH_HEADERSare declared locally here and separately inweb/default/src/features/system-settings/general/channel-affinity/constants.ts(which also has abuildCodexPassHeadersTemplate()helper this file doesn't reuse). They currently match, but this PR triples the duplicated surface by adding a second list (SSE headers) in the same pattern that caused the original bug (#6016: header allowlist drift). Consider importing the shared lists/helper fromconstants.ts(or a new shared module) instead of maintaining a third copy.Also applies to: 383-395
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx` around lines 298 - 358, Remove the locally duplicated CODEX_CLI_HEADER_PASSTHROUGH_HEADERS and CODEX_CLI_SSE_HEADER_PASSTHROUGH_HEADERS declarations, and import the shared allowlists (and buildCodexPassHeadersTemplate() where applicable) from constants.ts or a dedicated shared module. Update all consumers in this dialog to use the shared definitions so header changes cannot drift between files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@relay/helper/stream_scanner_test.go`:
- Around line 103-117: Make TestStreamScannerHandler_CopiesCodexResponseHeaders
initialize its channel-affinity rule and cache state explicitly within the test
scope, rather than relying on ambient configuration. Assert that
GetPreferredChannelByAffinity succeeds and returns the expected channel before
calling ApplyChannelAffinityOverrideTemplate, preserving deterministic behavior
under t.Parallel.
In
`@web/classic/src/components/table/channels/modals/ParamOverrideEditorModal.jsx`:
- Around line 1081-1088: The static-analysis missing-key warnings are false
positives: preserve the existing outer list key, such as operation.id, and do
not add redundant keys to nested Tag elements. Keep the operationModeOptions
useMemo translation and related translated labels/descriptions unchanged.
In
`@web/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsx`:
- Around line 298-358: Remove the locally duplicated
CODEX_CLI_HEADER_PASSTHROUGH_HEADERS and
CODEX_CLI_SSE_HEADER_PASSTHROUGH_HEADERS declarations, and import the shared
allowlists (and buildCodexPassHeadersTemplate() where applicable) from
constants.ts or a dedicated shared module. Update all consumers in this dialog
to use the shared definitions so header changes cannot drift between files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8078da07-18c4-4bfa-b422-3487d43a6325
📒 Files selected for processing (36)
dto/openai_request.godto/openai_request_zero_value_test.godto/openai_response.godto/openai_responses_compaction_request.gorelay/channel/api_request_test.gorelay/channel/claude/relay-claude.gorelay/channel/openai/relay_responses.gorelay/channel/openai/relay_responses_compact.gorelay/chat_completions_via_responses_test.gorelay/common/override.gorelay/common/override_test.gorelay/common/relay_info.gorelay/helper/stream_scanner.gorelay/helper/stream_scanner_test.gorelay/responses_handler.goservice/channel_affinity_template_test.goservice/relayconvert/responses_request_to_chat.goservice/relayconvert/responses_request_to_chat_test.goservice/relayconvert/responses_to_chat.gosetting/operation_setting/channel_affinity_setting.gosetting/ratio_setting/cache_ratio.goweb/classic/src/components/table/channels/modals/ParamOverrideEditorModal.jsxweb/classic/src/components/table/usage-logs/modals/ParamOverrideModal.jsxweb/classic/src/constants/channel-affinity-template.constants.jsweb/default/src/features/channels/components/dialogs/param-override-editor-dialog.tsxweb/default/src/features/system-settings/general/channel-affinity/constants.tsweb/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsxweb/default/src/features/usage-logs/lib/format.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh-TW.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/i18n/static-keys.ts
✅ Files skipped from review due to trivial changes (5)
- web/default/src/features/system-settings/general/channel-affinity/rule-editor-dialog.tsx
- web/default/src/i18n/locales/ja.json
- web/default/src/i18n/locales/ru.json
- web/default/src/i18n/locales/zh.json
- web/default/src/i18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (5)
- relay/chat_completions_via_responses_test.go
- relay/channel/api_request_test.go
- setting/operation_setting/channel_affinity_setting.go
- dto/openai_request_zero_value_test.go
- service/channel_affinity_template_test.go
|
请问这个pr是 fix了curl方式请求codex 5.6-luna “not fond model”的issue吗 |
不是，仅仅是按照codex的代码去补充一下使用codex时需要默认透传的请求头和sse响应头。 至于找不到模型不属于该处理的问题，本项目不处理不通过codex cli使用codex渠道出现的错误，不对codex渠道使用提供技术支持。 |
a326773 to
d1e79fb
Compare
* upstream/main: (56 commits) fix: list only channel models in unset price models tab fix: harden unset price models tab batch copy, feedback, and memo equality feat: add unset price models tab to model pricing settings (QuantumNous#6124) feat: enhance model search functionality with status and sync filters fix: bound uncached remainder by prompt-max(cached,write) and forward compact prompt_cache_key feat: bill OpenAI cache_write_tokens at cache-creation price with zero clamp feat: enhance text protocol conversion and advanced custom routing (QuantumNous#5825) fix: adjust margin for StatusBadge component in logs columns feat: enhance stale instance handling and update theme colors feat: update theme colors feat(timing): add timing metrics display for stream logs and enhance localization revert: restore StatusBadge horizontal padding revert: undo t0ng7u UI design-system refactor ✨ feat(web): polish themed data views and add task log details feat(image): enhance image stream handling with client disconnect logic and billing adjustments fix(billing): improve quota handling and error reporting for pre-consume operations fix(billing): reject saturated pre-consume quota 🐛 fix: Fontsource asset resolution across workspace layouts fix: sync codex field (QuantumNous#6018) chore(deps): bump golang.org/x/crypto from 0.51.0 to 0.52.0 (QuantumNous#6096) ...
3 participants
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
同步codex客户端传递的字段（request header和sse response header）
compact的字段
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
pass_sse_headersparameter override mode across the UI, templates, validation, and audit labeling.