Make Claude auto tests mirror Claude Code traffic - #4228
Conversation
WalkthroughThe changes add Claude-specific request header and metadata fixtures applied during channel testing, introduce an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/channel-test.go (1)
830-842:⚠️ Potential issue | 🟠 MajorClear the running flag on early setup failure.
testAllChannelsRunningis set beforemodel.GetAllChannels, but the early return ongetChannelErrnever resets it. One transient query failure will make every later manual and scheduled run report “测试已在运行中” until the process restarts.Proposed fix
func testAllChannels(notify bool, allowAutoDisable bool) error { testAllChannelsLock.Lock() if testAllChannelsRunning { testAllChannelsLock.Unlock() return errors.New("测试已在运行中") } testAllChannelsRunning = true testAllChannelsLock.Unlock() channels, getChannelErr := model.GetAllChannels(0, 0, true, false) if getChannelErr != nil { + testAllChannelsLock.Lock() + testAllChannelsRunning = false + testAllChannelsLock.Unlock() return getChannelErr }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controller/channel-test.go` around lines 830 - 842, The function testAllChannels sets testAllChannelsRunning before calling model.GetAllChannels but returns early on error without clearing the flag; modify testAllChannels to ensure testAllChannelsRunning is always cleared on exit (and testAllChannelsLock unlocked) by introducing a deferred cleanup after acquiring the lock (e.g. defer that sets testAllChannelsRunning = false and releases any held lock) so that any early return from model.GetAllChannels or other setup failures resets the running flag and avoids permanently blocking subsequent runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controller/channel-test.go`:
- Around line 74-98: The current guard in applyClaudeCodeTestFixtures uses
info.RelayFormat to detect Claude but misses requests that start as OpenAI paths
and are later adapted to Claude; change the guard to detect the resolved
upstream being Claude instead (inspect the RelayInfo field that holds the
resolved upstream target — e.g., info.Upstream, info.UpstreamPath or
info.UpstreamURL — and test for the Claude upstream path/host used by your
adaptors) and only return early when that resolved upstream does not indicate
Claude; keep the rest of applyClaudeCodeTestFixtures (header injection, Metadata
set, and RuntimeHeadersOverride) intact.
---
Outside diff comments:
In `@controller/channel-test.go`:
- Around line 830-842: The function testAllChannels sets testAllChannelsRunning
before calling model.GetAllChannels but returns early on error without clearing
the flag; modify testAllChannels to ensure testAllChannelsRunning is always
cleared on exit (and testAllChannelsLock unlocked) by introducing a deferred
cleanup after acquiring the lock (e.g. defer that sets testAllChannelsRunning =
false and releases any held lock) so that any early return from
model.GetAllChannels or other setup failures resets the running flag and avoids
permanently blocking subsequent runs.
🪄 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: 999b6eac-4b01-42a3-b3eb-2e4db7642324
📒 Files selected for processing (4)
controller/channel-test.gocontroller/channel_test_fixture_test.gorelay/channel/claude/relay-claude.gorelay/channel/claude/relay_claude_test.go
| func applyClaudeCodeTestFixtures(c *gin.Context, info *relaycommon.RelayInfo, request dto.Request) { | ||
| if c == nil || c.Request == nil || info == nil || info.RelayFormat != types.RelayFormatClaude { | ||
| return | ||
| } | ||
|
|
||
| for key, value := range claudeCodeTestHeaders { | ||
| if strings.TrimSpace(c.Request.Header.Get(key)) == "" { | ||
| c.Request.Header.Set(key, value) | ||
| } | ||
| } | ||
|
|
||
| if req, ok := request.(*dto.GeneralOpenAIRequest); ok && len(req.Metadata) == 0 { | ||
| req.Metadata = json.RawMessage(`{"user_id":"channel-test"}`) | ||
| } | ||
|
|
||
| runtimeHeaders := map[string]interface{}{} | ||
| for key, value := range relaycommon.GetEffectiveHeaderOverride(info) { | ||
| runtimeHeaders[key] = value | ||
| } | ||
| for key, value := range claudeCodeTestHeaders { | ||
| runtimeHeaders[key] = value | ||
| } | ||
| info.RuntimeHeadersOverride = runtimeHeaders | ||
| info.UseRuntimeHeadersOverride = true | ||
| } |
There was a problem hiding this comment.
Don't key the Claude fixture off inbound relay format alone.
testChannel still builds /v1/chat/completions by default, so Claude-backed channels often arrive here as RelayFormatOpenAI. Those requests are later converted to Claude upstreams in adaptors like relay/channel/aws/adaptor.go:124-128 and relay/channel/vertex/adaptor.go:327-336, so this guard skips the new headers/metadata for a chunk of the Claude traffic the PR is trying to harden. Gating on the resolved upstream Claude path would make this effective for the default test flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@controller/channel-test.go` around lines 74 - 98, The current guard in
applyClaudeCodeTestFixtures uses info.RelayFormat to detect Claude but misses
requests that start as OpenAI paths and are later adapted to Claude; change the
guard to detect the resolved upstream being Claude instead (inspect the
RelayInfo field that holds the resolved upstream target — e.g., info.Upstream,
info.UpstreamPath or info.UpstreamURL — and test for the Claude upstream
path/host used by your adaptors) and only return early when that resolved
upstream does not indicate Claude; keep the rest of applyClaudeCodeTestFixtures
(header injection, Metadata set, and RuntimeHeadersOverride) intact.
|
newapi不会接受伪造请求特征操作的PR |
2 participants
Summary
metadata.user_idto Claude auto-test requests and preserve metadata in Claude conversionWhy
A large share of Claude channels were being auto-disabled even though manual Claude Code traffic could still succeed. The root cause was that auto-tests were not close enough to real Claude Code traffic, so Claude-only upstreams were returning false negatives. This change makes the synthetic traffic much closer to real Claude Code requests and removes the most dangerous part of the feedback loop: scheduled tests auto-banning channels.
Testing
Summary by CodeRabbit
Tests
Refactor