From a424396a87464d8ae1a11065a01846b1c0fd2841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9D=B4=EB=8C=80=ED=9D=AC?= Date: Mon, 2 Feb 2026 12:50:33 +0900 Subject: [PATCH] Fixes thinking signature validation errors Addresses an issue where thinking signature validation fails due to model mapping and empty internal registry. - Implements a fallback mechanism in the router to use the global model registry when the internal registry is empty. This ensures that models registered via API keys are correctly resolved even without local provider configurations. - Modifies `GetModelGroup` to use registry-based grouping in addition to name pattern matching, covering cases where models are registered with API keys but lack provider names in their names. - Updates signature validation to compare model groups instead of exact model names. These changes resolve thinking signature validation errors and improve the accuracy of model resolution. --- internal/cache/signature_cache.go | 19 ++++ internal/cache/signature_cache_test.go | 81 +++++++++++++++++ internal/routing/router.go | 78 ++++++++++++++--- internal/routing/router_test.go | 87 +++++++++++++++++++ .../claude/antigravity_claude_request.go | 4 +- 5 files changed, 254 insertions(+), 15 deletions(-) diff --git a/internal/cache/signature_cache.go b/internal/cache/signature_cache.go index af5371bf..e15b0802 100644 --- a/internal/cache/signature_cache.go +++ b/internal/cache/signature_cache.go @@ -6,6 +6,8 @@ import ( "strings" "sync" "time" + + "github.com/router-for-me/CLIProxyAPI/v6/internal/registry" ) // SignatureEntry holds a cached thinking signature with timestamp @@ -184,6 +186,7 @@ func HasValidSignature(modelName, signature string) bool { } func GetModelGroup(modelName string) string { + // Fast path: check model name patterns first if strings.Contains(modelName, "gpt") { return "gpt" } else if strings.Contains(modelName, "claude") { @@ -191,5 +194,21 @@ func GetModelGroup(modelName string) string { } else if strings.Contains(modelName, "gemini") { return "gemini" } + + // Slow path: check registry for provider-based grouping + // This handles models registered via claude-api-key, gemini-api-key, etc. + // that don't have provider name in their model name (e.g., kimi-k2.5 via claude-api-key) + if providers := registry.GetGlobalRegistry().GetModelProviders(modelName); len(providers) > 0 { + provider := strings.ToLower(providers[0]) + switch provider { + case "claude": + return "claude" + case "gemini", "gemini-cli", "aistudio", "vertex", "antigravity": + return "gemini" + case "codex": + return "gpt" + } + } + return modelName } diff --git a/internal/cache/signature_cache_test.go b/internal/cache/signature_cache_test.go index 83408159..af4361f9 100644 --- a/internal/cache/signature_cache_test.go +++ b/internal/cache/signature_cache_test.go @@ -208,3 +208,84 @@ func TestCacheSignature_ExpirationLogic(t *testing.T) { // but the logic is verified by the implementation _ = time.Now() // Acknowledge we're not testing time passage } + +// === GetModelGroup Tests === +// These tests verify that GetModelGroup correctly identifies model groups +// both by name pattern (fast path) and by registry provider lookup (slow path). + +func TestGetModelGroup_ByNamePattern(t *testing.T) { + tests := []struct { + modelName string + expectedGroup string + }{ + {"gpt-4o", "gpt"}, + {"gpt-4-turbo", "gpt"}, + {"claude-sonnet-4-20250514", "claude"}, + {"claude-opus-4-5-thinking", "claude"}, + {"gemini-2.5-pro", "gemini"}, + {"gemini-3-pro-preview", "gemini"}, + } + + for _, tt := range tests { + t.Run(tt.modelName, func(t *testing.T) { + result := GetModelGroup(tt.modelName) + if result != tt.expectedGroup { + t.Errorf("GetModelGroup(%q) = %q, expected %q", tt.modelName, result, tt.expectedGroup) + } + }) + } +} + +func TestGetModelGroup_UnknownModel(t *testing.T) { + // For unknown models with no registry entry, should return the model name itself + result := GetModelGroup("unknown-model-xyz") + if result != "unknown-model-xyz" { + t.Errorf("GetModelGroup for unknown model should return model name, got %q", result) + } +} + +// TestGetModelGroup_RegistryFallback tests that models registered via +// provider-specific API keys (e.g., kimi-k2.5 via claude-api-key) are +// correctly grouped by their provider. +// This test requires a populated global registry. +func TestGetModelGroup_RegistryFallback(t *testing.T) { + // This test only makes sense when the global registry is populated + // In unit test context, skip if registry is empty + + // Example: kimi-k2.5 registered via claude-api-key should group as "claude" + // The model name doesn't contain "claude", so name pattern matching fails. + // The registry should be checked to find the provider. + + // Skip for now - this requires integration test setup + t.Skip("Requires populated global registry - run as integration test") +} + +// === Cross-Model Signature Validation Tests === +// These tests verify that signatures cached under one model name can be +// validated under mapped model names (same provider group). + +func TestCacheSignature_CrossModelValidation(t *testing.T) { + ClearSignatureCache("") + + // Original request uses "claude-opus-4-5-20251101" + originalModel := "claude-opus-4-5-20251101" + // Mapped model is "claude-opus-4-5-thinking" + mappedModel := "claude-opus-4-5-thinking" + + text := "Some thinking block content" + sig := "validSignature123456789012345678901234567890123456789012" + + // Cache signature under the original model + CacheSignature(originalModel, text, sig) + + // Both should return the same signature because they're in the same group + retrieved1 := GetCachedSignature(originalModel, text) + retrieved2 := GetCachedSignature(mappedModel, text) + + if retrieved1 != sig { + t.Errorf("Original model signature mismatch: got %q", retrieved1) + } + if retrieved2 != sig { + t.Errorf("Mapped model signature mismatch: got %q", retrieved2) + } +} diff --git a/internal/routing/router.go b/internal/routing/router.go index 30548ab1..543c7ecf 100644 --- a/internal/routing/router.go +++ b/internal/routing/router.go @@ -1,11 +1,14 @@ package routing import ( + "context" "sort" "strings" "github.com/router-for-me/CLIProxyAPI/v6/internal/config" + "github.com/router-for-me/CLIProxyAPI/v6/internal/registry" "github.com/router-for-me/CLIProxyAPI/v6/internal/thinking" + "github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/executor" ) // Router resolves models to provider candidates. @@ -115,25 +118,47 @@ func (r *Router) ResolveV2(req RoutingRequest) *RoutingDecision { } // findLocalCandidates finds local provider candidates for a model. +// If the internal registry is empty, it falls back to the global model registry. func (r *Router) findLocalCandidates(model string, suffixResult thinking.SuffixResult) []ProviderCandidate { var candidates []ProviderCandidate - for _, p := range r.registry.All() { - if !p.SupportsModel(model) { - continue - } + // Check internal registry first + registryProviders := r.registry.All() + if len(registryProviders) > 0 { + for _, p := range registryProviders { + if !p.SupportsModel(model) { + continue + } - // Apply thinking suffix if needed - actualModel := model - if suffixResult.HasSuffix && !thinking.ParseSuffix(model).HasSuffix { - actualModel = model + "(" + suffixResult.RawSuffix + ")" - } + // Apply thinking suffix if needed + actualModel := model + if suffixResult.HasSuffix && !thinking.ParseSuffix(model).HasSuffix { + actualModel = model + "(" + suffixResult.RawSuffix + ")" + } - if p.Available(actualModel) { - candidates = append(candidates, ProviderCandidate{ - Provider: p, - Model: actualModel, - }) + if p.Available(actualModel) { + candidates = append(candidates, ProviderCandidate{ + Provider: p, + Model: actualModel, + }) + } + } + } else { + // Fallback to global model registry (same logic as FallbackHandler) + // This ensures compatibility when the wrapper is initialized with an empty registry + providers := registry.GetGlobalRegistry().GetModelProviders(model) + if len(providers) > 0 { + actualModel := model + if suffixResult.HasSuffix && !thinking.ParseSuffix(model).HasSuffix { + actualModel = model + "(" + suffixResult.RawSuffix + ")" + } + // Create a synthetic provider candidate for each provider + for _, providerName := range providers { + candidates = append(candidates, ProviderCandidate{ + Provider: &globalRegistryProvider{name: providerName, model: actualModel}, + Model: actualModel, + }) + } } } @@ -145,6 +170,31 @@ func (r *Router) findLocalCandidates(model string, suffixResult thinking.SuffixR return candidates } +// globalRegistryProvider is a synthetic Provider implementation that wraps +// a provider name from the global model registry. It is used only for routing +// decisions when the internal registry is empty - actual execution goes through +// the normal handler path, not through this provider's Execute methods. +type globalRegistryProvider struct { + name string + model string +} + +func (p *globalRegistryProvider) Name() string { return p.name } +func (p *globalRegistryProvider) Type() ProviderType { return ProviderTypeOAuth } +func (p *globalRegistryProvider) Priority() int { return 0 } +func (p *globalRegistryProvider) SupportsModel(string) bool { return true } +func (p *globalRegistryProvider) Available(string) bool { return true } + +// Execute is not used for globalRegistryProvider - routing wrapper calls the handler directly. +func (p *globalRegistryProvider) Execute(ctx context.Context, model string, req executor.Request) (executor.Response, error) { + return executor.Response{}, nil +} + +// ExecuteStream is not used for globalRegistryProvider - routing wrapper calls the handler directly. +func (p *globalRegistryProvider) ExecuteStream(ctx context.Context, model string, req executor.Request) (<-chan executor.StreamChunk, error) { + return nil, nil +} + // buildLocalProviderDecision creates a decision for local provider routing. func (r *Router) buildLocalProviderDecision(requestedModel string, candidates []ProviderCandidate, thinkingSuffix string) *RoutingDecision { resolvedModel := requestedModel diff --git a/internal/routing/router_test.go b/internal/routing/router_test.go index ffa01ef9..c3674d01 100644 --- a/internal/routing/router_test.go +++ b/internal/routing/router_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/router-for-me/CLIProxyAPI/v6/internal/config" + globalRegistry "github.com/router-for-me/CLIProxyAPI/v6/internal/registry" "github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/executor" "github.com/stretchr/testify/assert" ) @@ -113,3 +114,89 @@ func TestRouter_Resolve_NoProviders(t *testing.T) { assert.Equal(t, "unknown-model", decision.ResolvedModel) assert.Empty(t, decision.Candidates) } + +// === Global Registry Fallback Tests (T-027) === +// These tests verify that when the internal registry is empty, +// the router falls back to the global model registry. +// This is the core fix for the thinking signature 400 error. + +func TestRouter_GlobalRegistryFallback_LocalProvider(t *testing.T) { + // This test requires registering a model in the global registry. + // We use a model that's already registered via api-key config in production. + // For isolated testing, we can skip if global registry is not populated. + + globalReg := globalRegistry.GetGlobalRegistry() + modelCount := globalReg.GetModelCount("claude-sonnet-4-20250514") + + if modelCount == 0 { + t.Skip("Global registry not populated - run with server context") + } + + // Empty internal registry + emptyRegistry := NewRegistry() + cfg := &config.Config{} + router := NewRouter(emptyRegistry, cfg) + + req := RoutingRequest{ + RequestedModel: "claude-sonnet-4-20250514", + PreferLocalProvider: true, + } + decision := router.ResolveV2(req) + + // Should find provider from global registry + assert.Equal(t, RouteTypeLocalProvider, decision.RouteType) + assert.Equal(t, "claude-sonnet-4-20250514", decision.ResolvedModel) + assert.False(t, decision.ShouldProxy) +} + +func TestRouter_GlobalRegistryFallback_ModelMapping(t *testing.T) { + // This test verifies that model mapping works with global registry fallback. + + globalReg := globalRegistry.GetGlobalRegistry() + modelCount := globalReg.GetModelCount("claude-opus-4-5-thinking") + + if modelCount == 0 { + t.Skip("Global registry not populated - run with server context") + } + + // Empty internal registry + emptyRegistry := NewRegistry() + cfg := &config.Config{ + AmpCode: config.AmpCode{ + ModelMappings: []config.AmpModelMapping{ + {From: "claude-opus-4-5-20251101", To: "claude-opus-4-5-thinking"}, + }, + }, + } + router := NewRouter(emptyRegistry, cfg) + + req := RoutingRequest{ + RequestedModel: "claude-opus-4-5-20251101", + PreferLocalProvider: true, + } + decision := router.ResolveV2(req) + + // Should find mapped model from global registry + assert.Equal(t, RouteTypeModelMapping, decision.RouteType) + assert.Equal(t, "claude-opus-4-5-thinking", decision.ResolvedModel) + assert.False(t, decision.ShouldProxy) +} + +func TestRouter_GlobalRegistryFallback_AmpCreditsWhenNotFound(t *testing.T) { + // Empty internal registry + emptyRegistry := NewRegistry() + cfg := &config.Config{} + router := NewRouter(emptyRegistry, cfg) + + // Use a model that definitely doesn't exist anywhere + req := RoutingRequest{ + RequestedModel: "nonexistent-model-12345", + PreferLocalProvider: true, + } + decision := router.ResolveV2(req) + + // Should fall back to AMP credits proxy + assert.Equal(t, RouteTypeAmpCredits, decision.RouteType) + assert.Equal(t, "nonexistent-model-12345", decision.ResolvedModel) + assert.True(t, decision.ShouldProxy) +} diff --git a/internal/translator/antigravity/claude/antigravity_claude_request.go b/internal/translator/antigravity/claude/antigravity_claude_request.go index 9bef7125..3a0c8d7b 100644 --- a/internal/translator/antigravity/claude/antigravity_claude_request.go +++ b/internal/translator/antigravity/claude/antigravity_claude_request.go @@ -115,7 +115,9 @@ func ConvertClaudeRequestToAntigravity(modelName string, inputRawJSON []byte, _ if signatureResult.Exists() && signatureResult.String() != "" { arrayClientSignatures := strings.SplitN(signatureResult.String(), "#", 2) if len(arrayClientSignatures) == 2 { - if modelName == arrayClientSignatures[0] { + // Compare using model group to handle model mapping + // e.g., claude-opus-4-5-thinking -> "claude" group should match "claude#signature" + if cache.GetModelGroup(modelName) == arrayClientSignatures[0] { clientSignature = arrayClientSignatures[1] } }