Restore Codex cache affinity for Claude Code gpt-5.4 requests - #4059
Closed
FlamesCN wants to merge 2 commits into
Closed
Restore Codex cache affinity for Claude Code gpt-5.4 requests#4059FlamesCN wants to merge 2 commits into
FlamesCN wants to merge 2 commits into
Conversation
Claude Code reached Codex through the /v1/messages compatibility path, but new-api dropped the session-derived cache key before the final Responses request. This preserved direct Responses caching while leaving Claude Code turns cold. The fix carries the Claude session id into prompt_cache_key, forces the equivalent session_id header on the compatibility path, and keeps Codex-only incompatible metadata out of the upstream request. Constraint: Codex only accepts /v1/responses and rejects metadata on the final upstream request Rejected: Rely on pass_headers alone | did not produce cache hits on the Claude compatibility path Rejected: Only sync prompt_cache_key without session_id header | direct probes still missed cache reads Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep Claude -> Responses compatibility preserving both prompt_cache_key and session_id together for Codex-backed models Tested: go test ./service/... ./relay/... Tested: manual production verification with Claude Code gpt-5.4 second-turn cache hit (cache_tokens=9472) Not-tested: upstream/main latest integration after cherry-pick
Contributor
WalkthroughThis PR extends the relay system to support prompt cache key propagation and context-based parameter overrides. Changes include: clearing metadata in Codex responses, adding runtime header override helpers for session IDs, extending channel affinity to extract values from request headers and nested JSON paths, propagating prompt cache keys through conversion pipelines, enabling context-targeted sync operations, and updating frontend UI components for the new features. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Collaborator
|
我个人不建议硬编码加入处理逻辑这个操作，现有的参数覆盖已经支持字段同步操作，缺少的只是这个key（转换后被抛弃）；这个key的获取方式也不是固定的，claude code曾修改过key（metadata）的结构，如果要硬编码就不太灵活，而且如果再次修改结构就需要更改代码和版本，如果anthropic/openai更激进一点，配置项格式直接不一样或者直接修改客户端的请求参数位置，那可能功能都会挂掉或者泄露特征，到时候连修改和关闭的地方都没有。 另外这个操作也不是所有人都需要的，缓存命中未必需要同一个cache key。 有一个方案是把这个metadata作为内置参数，然后把请求格式作为条件参数，选择性设置字段，作为模版追加到渠道亲和性针对codex和claude code预设的参数覆盖上，仅供参考。 |
Collaborator
|
😨另外我看了一下你改的是chat到responses这一段，没有claude相关部分，你在配置了 chat转responses那个功能然后发现的这个问题吗？ |
Claude Code cache reuse on the /v1/messages -> responses compatibility path should be driven by channel-affinity rules and override templates, not by hardcoded metadata parsing. This change adds request-header and nested JSON key extraction, exposes the resolved affinity key to override context, and lets templates sync that key into prompt_cache_key and session_id from the console. Constraint: Operators need to adapt to client header/metadata shape changes without shipping a new binary Rejected: Keep parsing metadata.user_id.session_id in convert.go | brittle to client request format changes Rejected: Add a separate Claude-specific settings surface | duplicates existing channel-affinity and param-override controls Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep Claude cache-affinity behavior driven by key_sources plus override templates; avoid reintroducing client-specific parsing in convert paths Tested: go test ./... Tested: bun run build Tested: production smoke test on 38.76.144.165 for /v1/messages -> gpt-5.4 with metadata-only and header-only session keys; second request hit cache in both cases Not-tested: GitHub PR creation via gh CLI (local auth token invalid)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
service/convert.go (1)
38-60:⚠️ Potential issue | 🟠 MajorReplace direct
json.Marshalcalls withcommon.Marshalwrappers.Lines 40 and 55 use
json.Marshaldirectly, violating the repository's JSON handling standard. All JSON marshal/unmarshal operations must use wrapper functions fromcommon/json.go.Proposed patch
- effortBytes, _ := json.Marshal(effort) + effortBytes, _ := common.Marshal(effort) openAIRequest.Verbosity = effortBytes @@ - reasoningJSON, err := json.Marshal(reasoning) + reasoningJSON, err := common.Marshal(reasoning) if err != nil { return nil, fmt.Errorf("failed to marshal reasoning: %w", err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/convert.go` around lines 38 - 60, Replace direct json.Marshal calls with the repository's wrapper from common/json.go: call common.Marshal when serializing effort (from claudeRequest.GetEfforts()) and when marshalling the reasoning struct (instead of json.Marshal for reasoning). For the effort path replace effortBytes, _ := json.Marshal(effort) with a checked common.Marshal call and handle the error (returning a wrapped fmt.Errorf) before assigning openAIRequest.Verbosity; for the thinking block replace reasoningJSON, err := json.Marshal(reasoning) with common.Marshal and keep the existing error handling (returning fmt.Errorf) if marshal fails. Ensure you reference claudeRequest.GetEfforts, openAIRequest.Verbosity, reasoning (openrouter.RequestReasoning) and openAIRequest.Reasoning when making these changes.service/channel_affinity.go (1)
539-564:⚠️ Potential issue | 🟡 Minor
key_nested_pathstill drops out of the final admin log.Line 560 only adds this field in
appendChannelAffinityTemplateAdminInfo().MarkChannelAffinityUsed()still rebuildsginKeyChannelAffinityLogInfowithoutkey_nested_path, so successful requests won't surface the new metadata unless that map is updated too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/channel_affinity.go` around lines 539 - 564, MarkChannelAffinityUsed() rebuilds the ginKeyChannelAffinityLogInfo map but omits "key_nested_path", so add the same "key_nested_path": meta.KeySourceNestedPath entry to the map constructed in MarkChannelAffinityUsed() (the same map whose keys include "reason", "rule_name", "using_group", "model", "request_path", "key_source", "key_key", "key_path", "key_hint", "key_fp", and "override_template") so that the value from appendChannelAffinityTemplateAdminInfo()/meta.KeySourceNestedPath is preserved in the final admin log when you call c.Set(ginKeyChannelAffinityLogInfo,...).
🧹 Nitpick comments (2)
web/src/components/table/channels/modals/ParamOverrideEditorModal.jsx (1)
276-276: Consider wrapping this new option label int()for i18n consistency.
'上下文字段'is hardcoded without theuseTranslation()+t('中文key')pattern. While this mirrors the existing approach throughout the file, the coding guidelines require all user-facing labels to support locale switching viat(). If the component is refactored for i18n compliance, include this label.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/ParamOverrideEditorModal.jsx` at line 276, The new option label in ParamOverrideEditorModal.jsx ('上下文字段' in the array entry { label: '上下文字段', value: 'context' }) is hardcoded; update the component to use the i18n helper by importing/using useTranslation() and replace the literal with t('your.translation.key') (e.g., { label: t('paramOverride.contextLabel'), value: 'context' }); add the translation key to the locale files accordingly so the label is translatable.web/src/pages/Setting/Operation/SettingsChannelAffinity.jsx (1)
532-537: Exposenested_pathconsistently forrequest_headerin the visual editor.
service/channel_affinity.gonow honorsNestedPathforrequest_headersources too, but Line 1368 disables this field unless the type isgjson, and Lines 533-537 / 1300-1301 still render and describe it as gjson-only. JSON mode can preserve that config, but operators still can't create or adjust header-wrapped JSON extraction from the visual flow.Also applies to: 1300-1301, 1360-1378
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Operation/SettingsChannelAffinity.jsx` around lines 532 - 537, The UI hides and labels nested_path as gjson-only but the backend (service/channel_affinity.go:NestedPath) also supports nested_path for request_header; update the rendering and controls to expose nested_path when s.type === 'gjson' OR s.type === 'request_header'. Specifically, change the detail computation (variable detail) and any conditional render/disable logic that currently tests s.type === 'gjson' (and the related controls at the blocks referenced around lines 1300-1301 and 1360-1378) so they treat 'request_header' like 'gjson' for showing, editing, and describing nested_path, and ensure labels/instructions reflect both types rather than gjson-only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@service/channel_affinity.go`:
- Around line 325-333: The lookup uses the untrimmed header name so a header
like " X-Claude-Code-Session-Id " passes the emptiness check but fails to match;
in the case handling "request_header" (inside the switch), compute and reuse a
trimmedKey (e.g., trimmed := strings.TrimSpace(src.Key)) for both the emptiness
check and the Header.Get call so that Header.Get(trimmed) is used before passing
the value to extractNestedChannelAffinityValue or returning it; update
references to src.Key in this block to the trimmed variable.
In `@service/convert.go`:
- Line 21: The code currently assigns claudeRequest.Metadata directly to the
converted request's Metadata (Metadata: claudeRequest.Metadata), which can leak
or break behavior on upstreams that don't expect full Claude metadata; change
the conversion in service/convert.go so that you either (a) only forward a
vetted subset of keys (e.g., build a new map of whitelisted affinity keys and
copy those into request.Metadata) or (b) gate forwarding behind a channel
capability check (call the channel's SupportsMetadata/CapableOfMetadata flag
before copying). Locate the conversion that references claudeRequest.Metadata
and replace the direct passthrough with a whitelist filter or capability check
(and add tests covering relay/channel/volcengine/adaptor.go behavior to ensure
non-compatible channels do not receive raw metadata).
---
Outside diff comments:
In `@service/channel_affinity.go`:
- Around line 539-564: MarkChannelAffinityUsed() rebuilds the
ginKeyChannelAffinityLogInfo map but omits "key_nested_path", so add the same
"key_nested_path": meta.KeySourceNestedPath entry to the map constructed in
MarkChannelAffinityUsed() (the same map whose keys include "reason",
"rule_name", "using_group", "model", "request_path", "key_source", "key_key",
"key_path", "key_hint", "key_fp", and "override_template") so that the value
from appendChannelAffinityTemplateAdminInfo()/meta.KeySourceNestedPath is
preserved in the final admin log when you call
c.Set(ginKeyChannelAffinityLogInfo,...).
In `@service/convert.go`:
- Around line 38-60: Replace direct json.Marshal calls with the repository's
wrapper from common/json.go: call common.Marshal when serializing effort (from
claudeRequest.GetEfforts()) and when marshalling the reasoning struct (instead
of json.Marshal for reasoning). For the effort path replace effortBytes, _ :=
json.Marshal(effort) with a checked common.Marshal call and handle the error
(returning a wrapped fmt.Errorf) before assigning openAIRequest.Verbosity; for
the thinking block replace reasoningJSON, err := json.Marshal(reasoning) with
common.Marshal and keep the existing error handling (returning fmt.Errorf) if
marshal fails. Ensure you reference claudeRequest.GetEfforts,
openAIRequest.Verbosity, reasoning (openrouter.RequestReasoning) and
openAIRequest.Reasoning when making these changes.
---
Nitpick comments:
In `@web/src/components/table/channels/modals/ParamOverrideEditorModal.jsx`:
- Line 276: The new option label in ParamOverrideEditorModal.jsx ('上下文字段' in the
array entry { label: '上下文字段', value: 'context' }) is hardcoded; update the
component to use the i18n helper by importing/using useTranslation() and replace
the literal with t('your.translation.key') (e.g., { label:
t('paramOverride.contextLabel'), value: 'context' }); add the translation key to
the locale files accordingly so the label is translatable.
In `@web/src/pages/Setting/Operation/SettingsChannelAffinity.jsx`:
- Around line 532-537: The UI hides and labels nested_path as gjson-only but the
backend (service/channel_affinity.go:NestedPath) also supports nested_path for
request_header; update the rendering and controls to expose nested_path when
s.type === 'gjson' OR s.type === 'request_header'. Specifically, change the
detail computation (variable detail) and any conditional render/disable logic
that currently tests s.type === 'gjson' (and the related controls at the blocks
referenced around lines 1300-1301 and 1360-1378) so they treat 'request_header'
like 'gjson' for showing, editing, and describing nested_path, and ensure
labels/instructions reflect both types rather than gjson-only.
🪄 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: 1a8ad3dd-52c2-4b99-a975-4bc9387ead5e
📒 Files selected for processing (11)
constant/context_key.gorelay/common/override.gorelay/common/override_test.gorelay/common/relay_info.goservice/channel_affinity.goservice/channel_affinity_template_test.goservice/convert.gosetting/operation_setting/channel_affinity_setting.goweb/src/components/table/channels/modals/ParamOverrideEditorModal.jsxweb/src/constants/channel-affinity-template.constants.jsweb/src/pages/Setting/Operation/SettingsChannelAffinity.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- service/channel_affinity_template_test.go
Comment on lines
+325
to
+333
| case "request_header": | ||
| if c == nil || c.Request == nil || strings.TrimSpace(src.Key) == "" { | ||
| return "" | ||
| } | ||
| value := strings.TrimSpace(c.Request.Header.Get(src.Key)) | ||
| if nestedPath != "" { | ||
| return extractNestedChannelAffinityValue(value, nestedPath) | ||
| } | ||
| return value |
Contributor
There was a problem hiding this comment.
Use the trimmed header name for the lookup.
Line 326 trims src.Key only for the emptiness check, but Line 329 still calls Header.Get(src.Key). A rule saved as " X-Claude-Code-Session-Id " passes validation and then silently never matches.
Suggested fix
case "request_header":
- if c == nil || c.Request == nil || strings.TrimSpace(src.Key) == "" {
+ headerName := strings.TrimSpace(src.Key)
+ if c == nil || c.Request == nil || headerName == "" {
return ""
}
- value := strings.TrimSpace(c.Request.Header.Get(src.Key))
+ value := strings.TrimSpace(c.Request.Header.Get(headerName))
if nestedPath != "" {
return extractNestedChannelAffinityValue(value, nestedPath)
}
return value📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case "request_header": | |
| if c == nil || c.Request == nil || strings.TrimSpace(src.Key) == "" { | |
| return "" | |
| } | |
| value := strings.TrimSpace(c.Request.Header.Get(src.Key)) | |
| if nestedPath != "" { | |
| return extractNestedChannelAffinityValue(value, nestedPath) | |
| } | |
| return value | |
| case "request_header": | |
| headerName := strings.TrimSpace(src.Key) | |
| if c == nil || c.Request == nil || headerName == "" { | |
| return "" | |
| } | |
| value := strings.TrimSpace(c.Request.Header.Get(headerName)) | |
| if nestedPath != "" { | |
| return extractNestedChannelAffinityValue(value, nestedPath) | |
| } | |
| return value |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/channel_affinity.go` around lines 325 - 333, The lookup uses the
untrimmed header name so a header like " X-Claude-Code-Session-Id " passes the
emptiness check but fails to match; in the case handling "request_header"
(inside the switch), compute and reuse a trimmedKey (e.g., trimmed :=
strings.TrimSpace(src.Key)) for both the emptiness check and the Header.Get call
so that Header.Get(trimmed) is used before passing the value to
extractNestedChannelAffinityValue or returning it; update references to src.Key
in this block to the trimmed variable.
| openAIRequest := dto.GeneralOpenAIRequest{ | ||
| Model: claudeRequest.Model, | ||
| Temperature: claudeRequest.Temperature, | ||
| Metadata: claudeRequest.Metadata, |
Contributor
There was a problem hiding this comment.
Scope metadata passthrough to compatible upstreams.
Line 21 now forwards raw Claude metadata to all converted OpenAI requests. That can break or alter behavior on non-Codex channels that parse request.Metadata (e.g., relay/channel/volcengine/adaptor.go Line 88-91), and may leak client metadata to upstreams that don’t need it. Please gate this by channel capability or whitelist only required affinity keys before forwarding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@service/convert.go` at line 21, The code currently assigns
claudeRequest.Metadata directly to the converted request's Metadata (Metadata:
claudeRequest.Metadata), which can leak or break behavior on upstreams that
don't expect full Claude metadata; change the conversion in service/convert.go
so that you either (a) only forward a vetted subset of keys (e.g., build a new
map of whitelisted affinity keys and copy those into request.Metadata) or (b)
gate forwarding behind a channel capability check (call the channel's
SupportsMetadata/CapableOfMetadata flag before copying). Locate the conversion
that references claudeRequest.Metadata and replace the direct passthrough with a
whitelist filter or capability check (and add tests covering
relay/channel/volcengine/adaptor.go behavior to ensure non-compatible channels
do not receive raw metadata).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 变更描述 / Description
修复 Claude Code 通过
/v1/messages兼容路径访问 Codex 时丢失缓存亲和的问题。这个提交把 Claude session id 同步到prompt_cache_key和上游session_id头，同时避免把 Codex 不接受的 metadata 带到最终/v1/responses请求里。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
go test ./relay ./service/... ./relay/channel/codex ./relay/channel/claude ./relay/channel/openaibun run build补充：当前
upstream/main上执行go test ./service/... ./relay/...仍会在relay/helper的TestStreamScannerHandler_StreamStatus_PreInitialized失败，这不是本 PR 触达的包。Summary by CodeRabbit
New Features
Bug Fixes
Tests