fix(config,providers): resolve api_key, model lookup, and fallback bugs (#2371, #2286, #2334)

- Accept legacy "api_key" (singular string) in ModelConfig JSON via custom
  UnmarshalJSON, fixing silent key drop that caused 401 auth errors
- Extend findMatches to fall back to Model field and bare model ID when
  ModelName doesn't match, fixing thinking_level lookup failures
- Classify HTTP 404 and "no endpoints found" as FailoverModelNotFound so
  the fallback chain correctly tries the next candidate instead of aborting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Sai Balusu 2026-04-05 23:39:32 -04:00
parent 90340ecf4b
commit c51acd47c7
5 changed files with 229 additions and 1 deletions

View file

@ -625,6 +625,45 @@ type ModelConfig struct {
isVirtual bool isVirtual bool
} }
// UnmarshalJSON implements custom JSON unmarshaling for ModelConfig to handle
// both "api_key" (singular string, legacy) and "api_keys" (array, current).
func (c *ModelConfig) UnmarshalJSON(data []byte) error {
// Use an alias to prevent infinite recursion.
type Alias ModelConfig
// Auxiliary struct captures the legacy singular "api_key" field.
aux := &struct {
*Alias
APIKeySingular json.RawMessage `json:"api_key,omitempty"`
}{
Alias: (*Alias)(c),
}
if err := json.Unmarshal(data, aux); err != nil {
return err
}
// If "api_key" was provided, merge it into APIKeys.
if len(aux.APIKeySingular) > 0 && string(aux.APIKeySingular) != "null" {
var single string
if err := json.Unmarshal(aux.APIKeySingular, &single); err == nil && strings.TrimSpace(single) != "" {
// Prepend the singular key; avoid duplicates.
found := false
for _, k := range c.APIKeys {
if k.String() == single {
found = true
break
}
}
if !found {
c.APIKeys = append(SecureStrings{NewSecureString(single)}, c.APIKeys...)
}
}
}
return nil
}
// APIKey returns the first API key from apiKeys // APIKey returns the first API key from apiKeys
func (c *ModelConfig) APIKey() string { func (c *ModelConfig) APIKey() string {
if len(c.APIKeys) > 0 { if len(c.APIKeys) > 0 {
@ -1207,14 +1246,40 @@ func (c *Config) GetModelConfig(modelName string) (*ModelConfig, error) {
return matches[idx], nil return matches[idx], nil
} }
// findMatches finds all ModelConfig entries with the given model_name. // findMatches finds all ModelConfig entries matching the given name.
// It matches on ModelName first; if no matches are found, it falls back to
// matching on the full Model field (e.g., "openai/gpt-4o") or the bare model
// ID after the protocol prefix (e.g., "gpt-4o").
func (c *Config) findMatches(modelName string) []*ModelConfig { func (c *Config) findMatches(modelName string) []*ModelConfig {
// Primary: match on ModelName (the user-facing alias).
var matches []*ModelConfig var matches []*ModelConfig
for i := range c.ModelList { for i := range c.ModelList {
if c.ModelList[i].ModelName == modelName { if c.ModelList[i].ModelName == modelName {
matches = append(matches, c.ModelList[i]) matches = append(matches, c.ModelList[i])
} }
} }
if len(matches) > 0 {
return matches
}
// Fallback: match on the Model field (protocol/model-id) or bare model ID.
for i := range c.ModelList {
mc := c.ModelList[i]
model := strings.TrimSpace(mc.Model)
if model == "" {
continue
}
if model == modelName {
matches = append(matches, mc)
continue
}
// Extract bare model ID: everything after the first "/".
if idx := strings.Index(model, "/"); idx >= 0 {
if model[idx+1:] == modelName {
matches = append(matches, mc)
}
}
}
return matches return matches
} }

View file

@ -30,6 +30,33 @@ func TestGetModelConfig_Found(t *testing.T) {
} }
} }
func TestGetModelConfig_ByModelID(t *testing.T) {
cfg := &Config{
Version: CurrentVersion,
ModelList: []*ModelConfig{
{ModelName: "my-alias", Model: "openai/gpt-4o", APIKeys: SimpleSecureStrings("key1"), ThinkingLevel: "high"},
},
}
// Lookup by full model ID should work when model_name doesn't match.
result, err := cfg.GetModelConfig("openai/gpt-4o")
if err != nil {
t.Fatalf("GetModelConfig(full model) error = %v", err)
}
if result.ThinkingLevel != "high" {
t.Errorf("ThinkingLevel = %q, want %q", result.ThinkingLevel, "high")
}
// Lookup by bare model ID (without protocol prefix).
result, err = cfg.GetModelConfig("gpt-4o")
if err != nil {
t.Fatalf("GetModelConfig(bare model ID) error = %v", err)
}
if result.ModelName != "my-alias" {
t.Errorf("ModelName = %q, want %q", result.ModelName, "my-alias")
}
}
func TestGetModelConfig_NotFound(t *testing.T) { func TestGetModelConfig_NotFound(t *testing.T) {
cfg := &Config{ cfg := &Config{
ModelList: []*ModelConfig{ ModelList: []*ModelConfig{
@ -331,3 +358,86 @@ func TestModelConfig_RequestTimeoutDefaultZeroValue(t *testing.T) {
t.Fatalf("RequestTimeout = %d, want 0", cfg.RequestTimeout) t.Fatalf("RequestTimeout = %d, want 0", cfg.RequestTimeout)
} }
} }
func TestModelConfig_APIKeySingular(t *testing.T) {
// Regression test: "api_key" (singular string) must be accepted
// and populate APIKeys, so the Authorization header is set.
jsonData := `{
"model_name": "openrouter-test",
"model": "openrouter/qwen/qwen3.6-plus:free",
"api_base": "https://openrouter.ai/api/v1",
"api_key": "sk-or-v1-test"
}`
var cfg ModelConfig
if err := json.Unmarshal([]byte(jsonData), &cfg); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}
if cfg.APIKey() != "sk-or-v1-test" {
t.Fatalf("APIKey() = %q, want %q", cfg.APIKey(), "sk-or-v1-test")
}
}
func TestModelConfig_APIKeysPlural(t *testing.T) {
// "api_keys" (plural array) must still work.
jsonData := `{
"model_name": "multi-key",
"model": "openai/gpt-4o",
"api_keys": ["key-a", "key-b"]
}`
var cfg ModelConfig
if err := json.Unmarshal([]byte(jsonData), &cfg); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}
if cfg.APIKey() != "key-a" {
t.Fatalf("APIKey() = %q, want %q", cfg.APIKey(), "key-a")
}
if len(cfg.APIKeys) != 2 {
t.Fatalf("len(APIKeys) = %d, want 2", len(cfg.APIKeys))
}
}
func TestModelConfig_APIKeyBothForms(t *testing.T) {
// When both "api_key" and "api_keys" are provided, the singular key
// should be prepended (if not already present in the array).
jsonData := `{
"model_name": "both",
"model": "openai/gpt-4o",
"api_key": "key-singular",
"api_keys": ["key-plural"]
}`
var cfg ModelConfig
if err := json.Unmarshal([]byte(jsonData), &cfg); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}
if cfg.APIKey() != "key-singular" {
t.Fatalf("APIKey() = %q, want %q", cfg.APIKey(), "key-singular")
}
if len(cfg.APIKeys) != 2 {
t.Fatalf("len(APIKeys) = %d, want 2", len(cfg.APIKeys))
}
}
func TestModelConfig_APIKeyDuplicate(t *testing.T) {
// If api_key is already in api_keys, don't duplicate it.
jsonData := `{
"model_name": "dup",
"model": "openai/gpt-4o",
"api_key": "same-key",
"api_keys": ["same-key"]
}`
var cfg ModelConfig
if err := json.Unmarshal([]byte(jsonData), &cfg); err != nil {
t.Fatalf("Unmarshal() error = %v", err)
}
if len(cfg.APIKeys) != 1 {
t.Fatalf("len(APIKeys) = %d, want 1 (deduped)", len(cfg.APIKeys))
}
}

View file

@ -84,6 +84,17 @@ var (
substr("messages.1.content.1.tool_use.id"), substr("messages.1.content.1.tool_use.id"),
substr("invalid request format"), substr("invalid request format"),
} }
// modelNotFoundPatterns detects errors indicating the model is unavailable at
// the provider, which should trigger fallback to the next candidate.
modelNotFoundPatterns = []errorPattern{
substr("no endpoints found"),
substr("model not found"),
substr("does not exist"),
rxp(`model .* not available`),
substr("model_not_found"),
rxp(`\b404\b`),
}
contextOverflowPatterns = []errorPattern{ contextOverflowPatterns = []errorPattern{
rxp(`context[_ ]?length[_ ]?exceeded`), rxp(`context[_ ]?length[_ ]?exceeded`),
rxp(`context[_ ]?window[_ ]?exceeded`), rxp(`context[_ ]?window[_ ]?exceeded`),
@ -195,6 +206,8 @@ func classifyByStatus(status int) FailoverReason {
return FailoverRateLimit return FailoverRateLimit
case status == 400: case status == 400:
return FailoverFormat return FailoverFormat
case status == 404:
return FailoverModelNotFound
case transientStatusCodes[status]: case transientStatusCodes[status]:
return FailoverTimeout return FailoverTimeout
} }
@ -225,6 +238,9 @@ func classifyByMessage(msg string) FailoverReason {
if matchesAny(msg, contextOverflowPatterns) { if matchesAny(msg, contextOverflowPatterns) {
return FailoverContextOverflow return FailoverContextOverflow
} }
if matchesAny(msg, modelNotFoundPatterns) {
return FailoverModelNotFound
}
return "" return ""
} }

View file

@ -288,6 +288,7 @@ func TestFailoverError_IsRetriable(t *testing.T) {
{FailoverBilling, true}, {FailoverBilling, true},
{FailoverTimeout, true}, {FailoverTimeout, true},
{FailoverOverloaded, true}, {FailoverOverloaded, true},
{FailoverModelNotFound, true},
{FailoverFormat, false}, {FailoverFormat, false},
{FailoverContextOverflow, false}, {FailoverContextOverflow, false},
{FailoverUnknown, true}, {FailoverUnknown, true},
@ -398,3 +399,38 @@ func TestIsContextWindowError(t *testing.T) {
t.Error("expected false for nil error") t.Error("expected false for nil error")
} }
} }
func TestClassifyError_ModelNotFound(t *testing.T) {
tests := []struct {
name string
err error
}{
{
"openrouter 404 no endpoints",
fmt.Errorf("API request failed:\n Status: 404\n Body: {\"error\":{\"message\":\"No endpoints found for qwen3.6-plus-preview:free.\",\"code\":404}}"),
},
{
"model not found",
errors.New("model not found: gpt-99"),
},
{
"does not exist",
errors.New("The model `gpt-99` does not exist"),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := ClassifyError(tt.err, "openrouter", "test-model")
if result == nil {
t.Fatal("expected non-nil classification for model-not-found error")
}
if result.Reason != FailoverModelNotFound {
t.Errorf("reason = %q, want %q", result.Reason, FailoverModelNotFound)
}
if !result.IsRetriable() {
t.Error("model_not_found should be retriable (trigger fallback)")
}
})
}
}

View file

@ -78,6 +78,7 @@ const (
FailoverFormat FailoverReason = "format" FailoverFormat FailoverReason = "format"
FailoverContextOverflow FailoverReason = "context_overflow" FailoverContextOverflow FailoverReason = "context_overflow"
FailoverOverloaded FailoverReason = "overloaded" FailoverOverloaded FailoverReason = "overloaded"
FailoverModelNotFound FailoverReason = "model_not_found"
FailoverUnknown FailoverReason = "unknown" FailoverUnknown FailoverReason = "unknown"
) )