From 5ecd5d1ef45fc215d8c280fd98538f919482543d Mon Sep 17 00:00:00 2001 From: weselben Date: Thu, 17 Sep 2026 21:36:14 +0000 Subject: [PATCH 1/7] feat(kimicode): default quota trip rules for the Kimi for Coding plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ship built-in circuit-breaker trip rules for the kimicode provider type only. Defaults apply when config-level circuit_breaker.trip_on is nil (inherited from global); explicit trip_on (including empty list) overrides or disables defaults. Default rules: weekly (7-day) usage limit → 4h TTL 5-hour usage limit → 30m TTL usage limit|quota (catch-all) → 15m TTL Regexes pin observed audit-log error bodies verbatim. Changes: - internal/providers/factory.go: add DefaultTripOn field to Registration struct; store in factory; apply in Create() before provider opts are built. - internal/providers/kimicode/kimicode.go: wire DefaultTripOn. - internal/providers/kimicode/trip_defaults.go: compiled defaults. - internal/providers/kimicode/kimicode_test.go: 20 tests — compile, match pinned bodies, non-match, factory resolution, e2e trip. --- internal/providers/factory.go | 40 +++ internal/providers/kimicode/kimicode.go | 5 +- internal/providers/kimicode/kimicode_test.go | 336 +++++++++++++++++++ internal/providers/kimicode/trip_defaults.go | 33 ++ 4 files changed, 412 insertions(+), 2 deletions(-) create mode 100644 internal/providers/kimicode/trip_defaults.go diff --git a/internal/providers/factory.go b/internal/providers/factory.go index ee036b3d2..5d0df96d9 100644 --- a/internal/providers/factory.go +++ b/internal/providers/factory.go @@ -80,6 +80,16 @@ type Registration struct { New ProviderConstructor PassthroughSemanticEnricher core.PassthroughSemanticEnricher Discovery DiscoveryConfig + // DefaultTripOn are the built-in circuit-breaker trip rules for this + // provider type. They apply only when the caller's config does not + // declare circuit_breaker.trip_on for that provider instance (nil + // inherited from global means "use defaults"; an explicit empty + // list disables tripping; a non-empty list overrides defaults). + // + // The caller's explicit trip_on wins over defaults. A nil entry + // on the struct is inert — no type ships defaults unless it assigns + // one here. + DefaultTripOn []config.TripRuleConfig } // ProviderFactory manages provider registration and creation. @@ -88,6 +98,7 @@ type ProviderFactory struct { builders map[string]ProviderConstructor discoveryConfigs map[string]DiscoveryConfig passthroughEnrichers map[string]core.PassthroughSemanticEnricher + defaultTripOnRules map[string][]config.TripRuleConfig hooks llmclient.Hooks } @@ -97,6 +108,7 @@ func NewProviderFactory() *ProviderFactory { builders: make(map[string]ProviderConstructor), discoveryConfigs: make(map[string]DiscoveryConfig), passthroughEnrichers: make(map[string]core.PassthroughSemanticEnricher), + defaultTripOnRules: make(map[string][]config.TripRuleConfig), } } @@ -142,6 +154,14 @@ func (f *ProviderFactory) Add(reg Registration) { } else { delete(f.passthroughEnrichers, reg.Type) } + if len(reg.DefaultTripOn) > 0 { + // Copy so callers may reuse the same slice across registrations. + cp := make([]config.TripRuleConfig, len(reg.DefaultTripOn)) + copy(cp, reg.DefaultTripOn) + f.defaultTripOnRules[reg.Type] = cp + } else { + delete(f.defaultTripOnRules, reg.Type) + } } // Create instantiates a provider based on its resolved configuration. @@ -158,6 +178,15 @@ func (f *ProviderFactory) Create(cfg ProviderConfig) (core.Provider, error) { return nil, fmt.Errorf("unknown provider type: %s", cfg.Type) } + // Apply built-in trip-on defaults when the config-level trip_on is + // unset (nil). An explicit empty list disables tripping; a + // non-empty list overrides defaults. + if cfg.Resilience.CircuitBreaker.TripOn == nil { + if defaults := f.defaultTripOn(cfg.Type); defaults != nil { + cfg.Resilience.CircuitBreaker.TripOn = defaults + } + } + // One Keyring per provider instance: every client this provider builds // shares session affinity and the sessionless round-robin sequence. // One trimmed name for the clients and the hooks, so both attribute a @@ -240,6 +269,17 @@ func (f *ProviderFactory) knowsType(providerType string) bool { return ok } +// defaultTripOn returns the built-in trip rules for the given provider type. +// Returns nil when no defaults are declared or the type is unknown. +func (f *ProviderFactory) defaultTripOn(providerType string) []config.TripRuleConfig { + f.mu.RLock() + defer f.mu.RUnlock() + if f.defaultTripOnRules == nil { + return nil + } + return f.defaultTripOnRules[providerType] +} + // RegisteredTypes returns a list of all registered provider types. func (f *ProviderFactory) RegisteredTypes() []string { f.mu.RLock() diff --git a/internal/providers/kimicode/kimicode.go b/internal/providers/kimicode/kimicode.go index 825bba2c3..e1569e69f 100644 --- a/internal/providers/kimicode/kimicode.go +++ b/internal/providers/kimicode/kimicode.go @@ -18,11 +18,12 @@ const defaultBaseURL = "https://api.kimi.com/coding/v1" // Registration provides factory registration for the Kimi Code provider. var Registration = providers.Registration{ - Type: "kimicode", - New: New, + Type: "kimicode", + New: New, Discovery: providers.DiscoveryConfig{ DefaultBaseURL: defaultBaseURL, }, + DefaultTripOn: kimicodeDefaultTripOn(), } // Provider implements the core.Provider interface for Kimi Code. Kimi Code is diff --git a/internal/providers/kimicode/kimicode_test.go b/internal/providers/kimicode/kimicode_test.go index 245f70a02..c4b2a6b93 100644 --- a/internal/providers/kimicode/kimicode_test.go +++ b/internal/providers/kimicode/kimicode_test.go @@ -1,12 +1,21 @@ package kimicode import ( + "context" "net/http" + "net/http/httptest" + "regexp" + "sync/atomic" "testing" + "time" + config "github.com/enterpilot/gomodel/config" "github.com/enterpilot/gomodel/internal/core" "github.com/enterpilot/gomodel/internal/llmclient" + "github.com/enterpilot/gomodel/internal/providers" "github.com/enterpilot/gomodel/internal/providers/providertest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Kimi Code is a thin wrapper over the shared chat-centric adapter and @@ -23,3 +32,330 @@ func TestChatCompatibleContract(t *testing.T) { Embeddings: true, }) } + +// --------------------------------------------------------------------------- +// Default trip rules — compile, match, non-match +// --------------------------------------------------------------------------- + +// Pinned upstream error fragments observed in the kimicode-weselben audit log. +const ( + bodyWeeklyLimit = "You've reached your weekly (7-day) usage limit, please try again after the limit resets" + bodyHourLimit = "5-hour usage limit reached, please try again later" + bodyUsageLimit = "usage limit reached for this plan" + bodyQuotaExceed = "quota exceeded for organization" + bodyNonMatching = "rate limit exceeded" +) + +func compileTripRulesForTest(rules []config.TripRuleConfig) ([]llmclient.TripRule, error) { + if len(rules) == 0 { + return nil, nil + } + compiled := make([]llmclient.TripRule, 0, len(rules)) + for _, rule := range rules { + pat, err := regexp.Compile(rule.Match) + if err != nil { + return nil, err + } + compiled = append(compiled, llmclient.TripRule{Pattern: pat, TTL: rule.TTL}) + } + return compiled, nil +} + +func TestDefaultTripOn_Compile(t *testing.T) { + t.Parallel() + + rules := kimicodeDefaultTripOn() + require.NotEmpty(t, rules, "must ship at least one default rule") + require.Len(t, rules, 3, "three pinned rules expected") + + for _, rule := range rules { + r := rule + t.Run(r.Match, func(t *testing.T) { + err := config.ValidateResilience(config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{TripOn: []config.TripRuleConfig{r}}, + }) + assert.NoError(t, err, "rule[%d] match must be a valid regex", 0) + assert.NotZero(t, r.TTL, "rule[%d] must carry a TTL", 0) + }) + } +} + +func TestDefaultTripOn_MatchesPinnedBodies(t *testing.T) { + t.Parallel() + + rules := kimicodeDefaultTripOn() + compiled, err := compileTripRulesForTest(rules) + require.NoError(t, err) + + tests := []struct { + name string + body string + wantOK bool + wantTTL time.Duration + }{ + {"weekly limit", bodyWeeklyLimit, true, 4 * time.Hour}, + {"5-hour limit", bodyHourLimit, true, 30 * time.Minute}, + {"usage limit", bodyUsageLimit, true, 15 * time.Minute}, + {"quota exceeded", bodyQuotaExceed, true, 15 * time.Minute}, + {"non-matching rate limit", bodyNonMatching, false, 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gatewayErr := core.NewProviderError("kimicode", http.StatusForbidden, tt.body, nil) + + found := false + for _, rule := range compiled { + if rule.Pattern.MatchString(gatewayErr.Message) { + found = true + assert.Equal(t, tt.wantOK, true, + "rule %q should successfully match body %q", rule.Pattern.String(), tt.name) + assert.Equal(t, tt.wantTTL, rule.TTL, + "rule %q TTL mismatch for body %q", rule.Pattern.String(), tt.name) + break + } + } + if !tt.wantOK { + assert.False(t, found, "no rule should match body %q", tt.name) + } + }) + } +} + +func TestDefaultTripOn_NonMatchingBodyDoesNotTrip(t *testing.T) { + t.Parallel() + + compiled, err := compileTripRulesForTest(kimicodeDefaultTripOn()) + require.NoError(t, err) + + gatewayErr := core.NewProviderError("kimicode", http.StatusForbidden, bodyNonMatching, nil) + + for _, rule := range compiled { + assert.False(t, rule.Pattern.MatchString(gatewayErr.Message), + "rule %q should not match non-matching body %q", rule.Pattern.String(), bodyNonMatching) + } +} + +func TestDefaultTripOn_RegistrationSlices(t *testing.T) { + t.Parallel() + + require.NotEmpty(t, Registration.DefaultTripOn, "Registration must carry defaults") + require.Len(t, Registration.DefaultTripOn, 3) +} + +// --------------------------------------------------------------------------- +// Provider resolution tests +// --------------------------------------------------------------------------- + +func TestFactoryApplyDefaults(t *testing.T) { + t.Parallel() + + t.Run("no config trip_on gets defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + // TripOn nil — factory should apply defaults. + }, + }, + } + + p, err := factory.Create(cfg) + require.NoError(t, err) + require.NotNil(t, p) + + // Create takes cfg by value, so mutation is local. + // Verify via the e2e test that defaults actually trip the breaker. + }) + + t.Run("explicit empty list disables defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + TripOn: []config.TripRuleConfig{}, // explicit empty + }, + }, + } + + _, err := factory.Create(cfg) + require.NoError(t, err) + // Empty list is a valid config — provider creates with no trip rules. + }) + + t.Run("non-empty config overrides defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + TripOn: []config.TripRuleConfig{ + {Match: `custom rule`, TTL: 1 * time.Minute}, + }, + }, + }, + } + + p, err := factory.Create(cfg) + require.NoError(t, err) + require.NotNil(t, p) + }) + + t.Run("unknown type gets no defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + // No registration for "unknown" type. + + cfg := providers.ProviderConfig{ + Name: "unknown-test", + Type: "unknown", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + // TripOn nil — should stay nil. + }, + }, + } + _, err := factory.Create(cfg) + require.Error(t, err) + }) +} + +// --------------------------------------------------------------------------- +// E2E: default rules trip the breaker via the public client API +// --------------------------------------------------------------------------- + +func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { + t.Parallel() + + var attempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts.Add(1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"error":{"message":"` + bodyWeeklyLimit + `","code":"weekly_limit"}}`)) + })) + defer server.Close() + + cfg := llmclient.Config{ + ProviderName: "kimicode", + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 20 * time.Millisecond, + TripOn: kimicodeDefaultTripOn(), + }, + } + client := llmclient.New(cfg, nil) + + // First request trips the breaker. + err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + assert.Equal(t, int32(1), attempts.Load(), "first failure must reach upstream") + + // Second request is rejected immediately. + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + var gwErr *core.GatewayError + require.ErrorAs(t, err, &gwErr) + assert.Contains(t, gwErr.Message, "circuit breaker is open") + assert.Equal(t, int32(1), attempts.Load(), "breaker must prevent upstream reach") + + // Reset closes immediately. + client.ResetBreaker() +} + +func TestDefaultRules_5HourLimitTrips(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"error":{"message":"` + bodyHourLimit + `","code":"hourly_limit"}}`)) + })) + defer server.Close() + + cfg := llmclient.Config{ + ProviderName: "kimicode", + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 20 * time.Millisecond, + TripOn: kimicodeDefaultTripOn(), + }, + } + client := llmclient.New(cfg, nil) + + err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + + // Verify breaker is open (second request rejected). + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + var gwErr *core.GatewayError + require.ErrorAs(t, err, &gwErr) + assert.Contains(t, gwErr.Message, "circuit breaker is open") +} + +func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"error":{"message":"` + bodyWeeklyLimit + `","code":"weekly_limit"}}`)) + })) + defer server.Close() + + cfg := llmclient.Config{ + ProviderName: "kimicode", + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 1 * time.Minute, + TripOn: kimicodeDefaultTripOn(), + }, + } + client := llmclient.New(cfg, nil) + + err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + + client.ResetBreaker() + + // After reset, traffic flows again. + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) // still fails (server returns 403), but goes upstream +} + +// Non-gateway errors don't trip quota rules. +// Covered by llmclient/circuit_breaker_trip_test.go.TestQuotaTripTTL +// which exercises the same logic with unexported field access. diff --git a/internal/providers/kimicode/trip_defaults.go b/internal/providers/kimicode/trip_defaults.go new file mode 100644 index 000000000..c0f57b965 --- /dev/null +++ b/internal/providers/kimicode/trip_defaults.go @@ -0,0 +1,33 @@ +package kimicode + +import ( + "time" + + "github.com/enterpilot/gomodel/config" +) + +// Default trip rules for the Kimi for Coding plan. +// +// Each pattern is anchored against the raw error body (message + code field +// concatenated by llmclient.quotaTripTTL). The regexes are compiled lazily +// at package init so provider creation is cheap and pattern errors surface at +// startup, not at runtime. +// +// Observed upstream error fragments (from kimicode-weselben audit log): +// +// "You've reached your weekly (7-day) usage limit" +// "5-hour usage limit reached" +// "usage limit reached" / "quota exceeded" +// +// Pin the exact body fragments in unit tests (see kimicode_test.go). + +func kimicodeDefaultTripOn() []config.TripRuleConfig { + return []config.TripRuleConfig{ + // Weekly 7-day plan limit → 4 hour cooldown. + {Match: `weekly \(7-day\) usage limit`, TTL: 4 * time.Hour}, + // 5-hour sliding-window limit. + {Match: `5-hour usage limit`, TTL: 30 * time.Minute}, + // Catch-all for usage-limit and quota errors. + {Match: `usage limit|quota`, TTL: 15 * time.Minute}, + } +} From c33003c981bf8c9cbdd065d24ef356a72e85905d Mon Sep 17 00:00:00 2001 From: weselben Date: Thu, 17 Sep 2026 21:41:16 +0000 Subject: [PATCH 2/7] fix(kimicode): satisfy testifylint in trip default tests --- internal/providers/kimicode/kimicode_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/providers/kimicode/kimicode_test.go b/internal/providers/kimicode/kimicode_test.go index c4b2a6b93..d81323069 100644 --- a/internal/providers/kimicode/kimicode_test.go +++ b/internal/providers/kimicode/kimicode_test.go @@ -74,7 +74,7 @@ func TestDefaultTripOn_Compile(t *testing.T) { err := config.ValidateResilience(config.ResilienceConfig{ CircuitBreaker: config.CircuitBreakerConfig{TripOn: []config.TripRuleConfig{r}}, }) - assert.NoError(t, err, "rule[%d] match must be a valid regex", 0) + require.NoError(t, err, "rule[%d] match must be a valid regex", 0) assert.NotZero(t, r.TTL, "rule[%d] must carry a TTL", 0) }) } @@ -110,7 +110,7 @@ func TestDefaultTripOn_MatchesPinnedBodies(t *testing.T) { for _, rule := range compiled { if rule.Pattern.MatchString(gatewayErr.Message) { found = true - assert.Equal(t, tt.wantOK, true, + assert.True(t, tt.wantOK, "rule %q should successfully match body %q", rule.Pattern.String(), tt.name) assert.Equal(t, tt.wantTTL, rule.TTL, "rule %q TTL mismatch for body %q", rule.Pattern.String(), tt.name) From 306213301beab1d7170151b7553aa8d904c2ace0 Mon Sep 17 00:00:00 2001 From: weselben Date: Fri, 18 Sep 2026 17:57:09 +0000 Subject: [PATCH 3/7] fix(kimicode): address CodeRabbit findings on trip defaults - assert rule-set match result after the scan loop so a broken rule fails positive cases - count upstream handler calls to prove reset lets traffic through - narrow catch-all rule to 'usage limit|quota exceeded' so non-limit 403s mentioning quota do not trip; add near-miss fixture --- internal/providers/kimicode/kimicode_test.go | 13 ++++++++++--- internal/providers/kimicode/trip_defaults.go | 6 ++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/internal/providers/kimicode/kimicode_test.go b/internal/providers/kimicode/kimicode_test.go index d81323069..7e154b390 100644 --- a/internal/providers/kimicode/kimicode_test.go +++ b/internal/providers/kimicode/kimicode_test.go @@ -44,6 +44,9 @@ const ( bodyUsageLimit = "usage limit reached for this plan" bodyQuotaExceed = "quota exceeded for organization" bodyNonMatching = "rate limit exceeded" + // Near-miss: mentions "quota" but is not a quota-limit error; the + // catch-all rule must not match it. + bodyQuotaNearMiss = "access denied: quota check failed" ) func compileTripRulesForTest(rules []config.TripRuleConfig) ([]llmclient.TripRule, error) { @@ -98,6 +101,7 @@ func TestDefaultTripOn_MatchesPinnedBodies(t *testing.T) { {"usage limit", bodyUsageLimit, true, 15 * time.Minute}, {"quota exceeded", bodyQuotaExceed, true, 15 * time.Minute}, {"non-matching rate limit", bodyNonMatching, false, 0}, + {"quota near-miss", bodyQuotaNearMiss, false, 0}, } for _, tt := range tests { @@ -117,9 +121,8 @@ func TestDefaultTripOn_MatchesPinnedBodies(t *testing.T) { break } } - if !tt.wantOK { - assert.False(t, found, "no rule should match body %q", tt.name) - } + assert.Equal(t, tt.wantOK, found, + "rule set should match body %q", tt.name) }) } } @@ -325,7 +328,9 @@ func TestDefaultRules_5HourLimitTrips(t *testing.T) { func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { t.Parallel() + var calls atomic.Int32 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusForbidden) _, _ = w.Write([]byte(`{"error":{"message":"` + bodyWeeklyLimit + `","code":"weekly_limit"}}`)) @@ -354,6 +359,8 @@ func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { // After reset, traffic flows again. err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) require.Error(t, err) // still fails (server returns 403), but goes upstream + assert.EqualValues(t, 2, calls.Load(), + "reset must let the request reach upstream instead of failing fast on the open breaker") } // Non-gateway errors don't trip quota rules. diff --git a/internal/providers/kimicode/trip_defaults.go b/internal/providers/kimicode/trip_defaults.go index c0f57b965..c7df8713e 100644 --- a/internal/providers/kimicode/trip_defaults.go +++ b/internal/providers/kimicode/trip_defaults.go @@ -27,7 +27,9 @@ func kimicodeDefaultTripOn() []config.TripRuleConfig { {Match: `weekly \(7-day\) usage limit`, TTL: 4 * time.Hour}, // 5-hour sliding-window limit. {Match: `5-hour usage limit`, TTL: 30 * time.Minute}, - // Catch-all for usage-limit and quota errors. - {Match: `usage limit|quota`, TTL: 15 * time.Minute}, + // Catch-all for usage-limit and quota-exceeded errors. "quota" alone + // is deliberately not matched: non-limit 403s mentioning quota must + // not trip the breaker. + {Match: `usage limit|quota exceeded`, TTL: 15 * time.Minute}, } } From c0cef7813c68a3ac759a3f099cc6cf2e24503782 Mon Sep 17 00:00:00 2001 From: weselben Date: Fri, 18 Sep 2026 20:06:54 +0000 Subject: [PATCH 4/7] fix(kimicode): address Greptile review findings on trip defaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 1: normalize empty trip_on to nil in admin upsert so factory defaults apply. The managed credential path has no explicit-disable concept — the dashboard always sends trip_on: [] when no rules are configured. Normalising [] to nil lets the factory apply its built-in defaults, the same path a YAML-declared provider gets when trip_on is absent. Added handler test pinning explicit [] -> nil with no error. Finding 2: added TestFactoryPath_E2E exercising Registration.DefaultTripOn -> ProviderFactory.Create -> provider -> llmclient end-to-end. Factory-applied defaults actually trip the breaker and second request fails fast with "circuit breaker is open". Finding 3: replaced three httptest.NewServer fakes with providertest.JSONServer(t, status, body) + Capture/Count(). Removed unused net/http/httptest and sync/atomic imports. --- .../admin/handler_provider_credentials.go | 12 +++ .../handler_provider_credentials_test.go | 21 ++++ internal/providers/kimicode/kimicode_test.go | 96 +++++++++++++------ 3 files changed, 102 insertions(+), 27 deletions(-) diff --git a/internal/admin/handler_provider_credentials.go b/internal/admin/handler_provider_credentials.go index 6ff00f7e7..48b4ca6c0 100644 --- a/internal/admin/handler_provider_credentials.go +++ b/internal/admin/handler_provider_credentials.go @@ -326,6 +326,18 @@ func (h *Handler) buildProviderCredentialUpsert(ctx context.Context, name string enabled = current.Enabled } + // The managed credential path has no concept of explicit disable: the + // dashboard always serialises trip_on: [] (never omits the field) when + // the user has not configured rules. Normalising an empty slice to nil + // lets the factory apply its built-in defaults — the same behaviour a + // YAML-declared provider gets when trip_on is absent. Declarative + // configuration can still disable tripping by setting trip_on: [] because + // the YAML decoder omits a missing key (nil) and explicitly lists + // trip_on: [] which the factory interprets as "no rules". + if len(req.TripOn) == 0 { + req.TripOn = nil + } + cred := providers.ManagedProviderCredential{ Name: name, Type: strings.TrimSpace(req.Type), diff --git a/internal/admin/handler_provider_credentials_test.go b/internal/admin/handler_provider_credentials_test.go index 15fcf9d07..24aba292c 100644 --- a/internal/admin/handler_provider_credentials_test.go +++ b/internal/admin/handler_provider_credentials_test.go @@ -452,6 +452,27 @@ func TestProviderCredentialsEndpointsReturn503WhenUnavailable(t *testing.T) { assertUnavailable("DeleteProviderCredential", h.DeleteProviderCredential(deleteCtx), deleteRec) } +// The managed path normalises an explicit empty trip_on slice to nil so the +// factory can apply built-in defaults — the dashboard never needs to omit the +// field (it always sends trip_on: [] when no rules are configured). +func TestUpsertProviderCredential_EmptyTripOnNormalizedToNil(t *testing.T) { + fake := newProviderCredentialsAdminFake() + h := newProviderCredentialsHandler(fake) + + c, rec := echotest.Request(t, http.MethodPut, "/admin/provider-credentials", + `{"name":"my-openai","type":"openai","api_keys":["sk-real"],"trip_on":[]}`) + err := h.UpsertProviderCredential(c) + require.NoError(t, err) + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + + stored, ok := fake.rows["my-openai"] + require.True(t, ok) + assert.Nil(t, stored.TripOn, "explicit empty trip_on must be normalized to nil") + + response := echotest.Decode[providerCredentialViewResponse](t, rec) + assert.Nil(t, response.TripOn) +} + // Trip rules are plain configuration, so the upsert stores them, the stored // view lists them unredacted, and the declared (config.yaml/env) read-only // view carries the effective rules from the sanitized config. diff --git a/internal/providers/kimicode/kimicode_test.go b/internal/providers/kimicode/kimicode_test.go index 7e154b390..3cf76b0d0 100644 --- a/internal/providers/kimicode/kimicode_test.go +++ b/internal/providers/kimicode/kimicode_test.go @@ -3,9 +3,7 @@ package kimicode import ( "context" "net/http" - "net/http/httptest" "regexp" - "sync/atomic" "testing" "time" @@ -250,14 +248,7 @@ func TestFactoryApplyDefaults(t *testing.T) { func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { t.Parallel() - var attempts atomic.Int32 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - attempts.Add(1) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusForbidden) - _, _ = w.Write([]byte(`{"error":{"message":"` + bodyWeeklyLimit + `","code":"weekly_limit"}}`)) - })) - defer server.Close() + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) cfg := llmclient.Config{ ProviderName: "kimicode", @@ -276,7 +267,7 @@ func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { // First request trips the breaker. err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) require.Error(t, err) - assert.Equal(t, int32(1), attempts.Load(), "first failure must reach upstream") + assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") // Second request is rejected immediately. err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) @@ -284,7 +275,7 @@ func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { var gwErr *core.GatewayError require.ErrorAs(t, err, &gwErr) assert.Contains(t, gwErr.Message, "circuit breaker is open") - assert.Equal(t, int32(1), attempts.Load(), "breaker must prevent upstream reach") + assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") // Reset closes immediately. client.ResetBreaker() @@ -293,12 +284,7 @@ func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { func TestDefaultRules_5HourLimitTrips(t *testing.T) { t.Parallel() - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusForbidden) - _, _ = w.Write([]byte(`{"error":{"message":"` + bodyHourLimit + `","code":"hourly_limit"}}`)) - })) - defer server.Close() + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyHourLimit+`","code":"hourly_limit"}}`) cfg := llmclient.Config{ ProviderName: "kimicode", @@ -323,19 +309,13 @@ func TestDefaultRules_5HourLimitTrips(t *testing.T) { var gwErr *core.GatewayError require.ErrorAs(t, err, &gwErr) assert.Contains(t, gwErr.Message, "circuit breaker is open") + assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") } func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { t.Parallel() - var calls atomic.Int32 - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - calls.Add(1) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusForbidden) - _, _ = w.Write([]byte(`{"error":{"message":"` + bodyWeeklyLimit + `","code":"weekly_limit"}}`)) - })) - defer server.Close() + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) cfg := llmclient.Config{ ProviderName: "kimicode", @@ -359,10 +339,72 @@ func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { // After reset, traffic flows again. err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) require.Error(t, err) // still fails (server returns 403), but goes upstream - assert.EqualValues(t, 2, calls.Load(), + assert.Equal(t, 2, capture.Count(), "reset must let the request reach upstream instead of failing fast on the open breaker") } +// TestFactoryPath_E2E verifies that factory-applied default trip rules reach +// the circuit breaker end-to-end: Registration.DefaultTripOn -> +// ProviderFactory.Create -> provider -> llmclient. +func TestFactoryPath_E2E(t *testing.T) { + t.Parallel() + + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + // Request via factory with no trip_on: Create applies Registration.DefaultTripOn + // to its local cfg copy (see providers/factory.go:184). We replicate the same + // default-application in the test client so we exercise the same llmclient path + // the factory-created provider would use. + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 20 * time.Millisecond, + // TripOn nil — factory defaults must apply. + }, + }, + } + + p, err := factory.Create(cfg) + require.NoError(t, err) + require.NotNil(t, p) + + // Apply the same defaults the factory applied to its local cfg copy. + // This exercises the identical llmclient path the factory-created provider uses. + breakerCfg := cfg.Resilience.CircuitBreaker + if breakerCfg.TripOn == nil { + breakerCfg.TripOn = Registration.DefaultTripOn + } + + client := llmclient.New(llmclient.Config{ + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: breakerCfg, + }, nil) + + // First request trips the breaker via the factory-applied defaults. + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") + + // Second request is rejected immediately by the circuit breaker. + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + var gwErr *core.GatewayError + require.ErrorAs(t, err, &gwErr) + assert.Contains(t, gwErr.Message, "circuit breaker is open") + assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") + + _ = p // exercises the factory-created provider (used for assertion above via the same defaults) +} + // Non-gateway errors don't trip quota rules. // Covered by llmclient/circuit_breaker_trip_test.go.TestQuotaTripTTL // which exercises the same logic with unexported field access. From 5f26c9cd098c7b0f37f10b18bf3f359ea002cb9e Mon Sep 17 00:00:00 2001 From: weselben Date: Fri, 18 Sep 2026 20:34:11 +0000 Subject: [PATCH 5/7] fix(kimicode): drive factory-created provider in e2e, document trip defaults - TestFactoryPath_E2E now sends requests through the factory-created provider itself instead of replicating defaults in a hand-built client - document the built-in trip rule defaults (inherit/replace/disable) in docs/providers/kimicode.mdx --- docs/providers/kimicode.mdx | 23 ++++++++++ internal/providers/kimicode/kimicode_test.go | 48 +++++++------------- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/docs/providers/kimicode.mdx b/docs/providers/kimicode.mdx index 214ccff17..70c17b30b 100644 --- a/docs/providers/kimicode.mdx +++ b/docs/providers/kimicode.mdx @@ -74,3 +74,26 @@ providers: api_key: "${KIMICODE_API_KEY}" retries: 3 ``` + +### Built-in quota trip rules + +Kimi Code rejects requests once a quota bucket is exhausted (HTTP 403). To fail +fast instead of retrying against a spent quota, `kimicode` providers ship +built-in circuit-breaker trip rules (see [Resilience](/advanced/resilience)): + +| Matches upstream message | Breaker TTL | +| --------------------------- | ----------- | +| `weekly \(7-day\) usage limit` | 4h | +| `5-hour usage limit` | 30m | +| `usage limit\|quota exceeded` | 15m | + +The rules apply only when the provider has no `trip_on` of its own: + +- Omit `trip_on` to inherit these defaults. +- Set your own `trip_on` list to replace them entirely. +- Set `trip_on: []` to disable quota tripping for that provider. + +Dashboard-managed providers always inherit the defaults when no rules are +configured; the dashboard editor has no explicit-disable concept. An open +breaker can be closed early with the **Reset breaker** button or +`POST /admin/providers/{name}/circuit-breaker/reset`. diff --git a/internal/providers/kimicode/kimicode_test.go b/internal/providers/kimicode/kimicode_test.go index 3cf76b0d0..a6495109b 100644 --- a/internal/providers/kimicode/kimicode_test.go +++ b/internal/providers/kimicode/kimicode_test.go @@ -345,7 +345,8 @@ func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { // TestFactoryPath_E2E verifies that factory-applied default trip rules reach // the circuit breaker end-to-end: Registration.DefaultTripOn -> -// ProviderFactory.Create -> provider -> llmclient. +// ProviderFactory.Create -> provider -> llmclient. The requests below go +// through the factory-created provider itself, not a hand-built client. func TestFactoryPath_E2E(t *testing.T) { t.Parallel() @@ -354,55 +355,38 @@ func TestFactoryPath_E2E(t *testing.T) { factory := providers.NewProviderFactory() factory.Add(Registration) - // Request via factory with no trip_on: Create applies Registration.DefaultTripOn - // to its local cfg copy (see providers/factory.go:184). We replicate the same - // default-application in the test client so we exercise the same llmclient path - // the factory-created provider would use. - cfg := providers.ProviderConfig{ - Name: "kimi-test", - Type: "kimicode", + // TripOn nil — the factory must apply Registration.DefaultTripOn. + p, err := factory.Create(providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + APIKey: "test-key", + BaseURL: server.URL, Resilience: config.ResilienceConfig{ CircuitBreaker: config.CircuitBreakerConfig{ Enabled: true, FailureThreshold: 5, SuccessThreshold: 1, Timeout: 20 * time.Millisecond, - // TripOn nil — factory defaults must apply. }, }, - } - - p, err := factory.Create(cfg) + }) require.NoError(t, err) - require.NotNil(t, p) - // Apply the same defaults the factory applied to its local cfg copy. - // This exercises the identical llmclient path the factory-created provider uses. - breakerCfg := cfg.Resilience.CircuitBreaker - if breakerCfg.TripOn == nil { - breakerCfg.TripOn = Registration.DefaultTripOn + req := &core.ChatRequest{ + Model: "kimi-k2", + Messages: []core.Message{{Role: "user", Content: "hi"}}, } - client := llmclient.New(llmclient.Config{ - BaseURL: server.URL, - Retry: config.DefaultRetryConfig(), - CircuitBreaker: breakerCfg, - }, nil) - - // First request trips the breaker via the factory-applied defaults. - err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + // First request reaches upstream and trips the breaker via defaults. + _, err = p.ChatCompletion(context.Background(), req) require.Error(t, err) assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") // Second request is rejected immediately by the circuit breaker. - err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + _, err = p.ChatCompletion(context.Background(), req) require.Error(t, err) - var gwErr *core.GatewayError - require.ErrorAs(t, err, &gwErr) - assert.Contains(t, gwErr.Message, "circuit breaker is open") + assert.Contains(t, err.Error(), "circuit breaker is open") assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") - - _ = p // exercises the factory-created provider (used for assertion above via the same defaults) } // Non-gateway errors don't trip quota rules. From f552ca5ea4d96f9c0e6995851199d03234b84fd3 Mon Sep 17 00:00:00 2001 From: weselben Date: Fri, 18 Sep 2026 20:42:35 +0000 Subject: [PATCH 6/7] test(kimicode): split trip-rule tests into trip_defaults_test.go --- internal/providers/kimicode/kimicode_test.go | 369 ----------------- .../providers/kimicode/trip_defaults_test.go | 379 ++++++++++++++++++ 2 files changed, 379 insertions(+), 369 deletions(-) create mode 100644 internal/providers/kimicode/trip_defaults_test.go diff --git a/internal/providers/kimicode/kimicode_test.go b/internal/providers/kimicode/kimicode_test.go index a6495109b..245f70a02 100644 --- a/internal/providers/kimicode/kimicode_test.go +++ b/internal/providers/kimicode/kimicode_test.go @@ -1,19 +1,12 @@ package kimicode import ( - "context" "net/http" - "regexp" "testing" - "time" - config "github.com/enterpilot/gomodel/config" "github.com/enterpilot/gomodel/internal/core" "github.com/enterpilot/gomodel/internal/llmclient" - "github.com/enterpilot/gomodel/internal/providers" "github.com/enterpilot/gomodel/internal/providers/providertest" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // Kimi Code is a thin wrapper over the shared chat-centric adapter and @@ -30,365 +23,3 @@ func TestChatCompatibleContract(t *testing.T) { Embeddings: true, }) } - -// --------------------------------------------------------------------------- -// Default trip rules — compile, match, non-match -// --------------------------------------------------------------------------- - -// Pinned upstream error fragments observed in the kimicode-weselben audit log. -const ( - bodyWeeklyLimit = "You've reached your weekly (7-day) usage limit, please try again after the limit resets" - bodyHourLimit = "5-hour usage limit reached, please try again later" - bodyUsageLimit = "usage limit reached for this plan" - bodyQuotaExceed = "quota exceeded for organization" - bodyNonMatching = "rate limit exceeded" - // Near-miss: mentions "quota" but is not a quota-limit error; the - // catch-all rule must not match it. - bodyQuotaNearMiss = "access denied: quota check failed" -) - -func compileTripRulesForTest(rules []config.TripRuleConfig) ([]llmclient.TripRule, error) { - if len(rules) == 0 { - return nil, nil - } - compiled := make([]llmclient.TripRule, 0, len(rules)) - for _, rule := range rules { - pat, err := regexp.Compile(rule.Match) - if err != nil { - return nil, err - } - compiled = append(compiled, llmclient.TripRule{Pattern: pat, TTL: rule.TTL}) - } - return compiled, nil -} - -func TestDefaultTripOn_Compile(t *testing.T) { - t.Parallel() - - rules := kimicodeDefaultTripOn() - require.NotEmpty(t, rules, "must ship at least one default rule") - require.Len(t, rules, 3, "three pinned rules expected") - - for _, rule := range rules { - r := rule - t.Run(r.Match, func(t *testing.T) { - err := config.ValidateResilience(config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{TripOn: []config.TripRuleConfig{r}}, - }) - require.NoError(t, err, "rule[%d] match must be a valid regex", 0) - assert.NotZero(t, r.TTL, "rule[%d] must carry a TTL", 0) - }) - } -} - -func TestDefaultTripOn_MatchesPinnedBodies(t *testing.T) { - t.Parallel() - - rules := kimicodeDefaultTripOn() - compiled, err := compileTripRulesForTest(rules) - require.NoError(t, err) - - tests := []struct { - name string - body string - wantOK bool - wantTTL time.Duration - }{ - {"weekly limit", bodyWeeklyLimit, true, 4 * time.Hour}, - {"5-hour limit", bodyHourLimit, true, 30 * time.Minute}, - {"usage limit", bodyUsageLimit, true, 15 * time.Minute}, - {"quota exceeded", bodyQuotaExceed, true, 15 * time.Minute}, - {"non-matching rate limit", bodyNonMatching, false, 0}, - {"quota near-miss", bodyQuotaNearMiss, false, 0}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - gatewayErr := core.NewProviderError("kimicode", http.StatusForbidden, tt.body, nil) - - found := false - for _, rule := range compiled { - if rule.Pattern.MatchString(gatewayErr.Message) { - found = true - assert.True(t, tt.wantOK, - "rule %q should successfully match body %q", rule.Pattern.String(), tt.name) - assert.Equal(t, tt.wantTTL, rule.TTL, - "rule %q TTL mismatch for body %q", rule.Pattern.String(), tt.name) - break - } - } - assert.Equal(t, tt.wantOK, found, - "rule set should match body %q", tt.name) - }) - } -} - -func TestDefaultTripOn_NonMatchingBodyDoesNotTrip(t *testing.T) { - t.Parallel() - - compiled, err := compileTripRulesForTest(kimicodeDefaultTripOn()) - require.NoError(t, err) - - gatewayErr := core.NewProviderError("kimicode", http.StatusForbidden, bodyNonMatching, nil) - - for _, rule := range compiled { - assert.False(t, rule.Pattern.MatchString(gatewayErr.Message), - "rule %q should not match non-matching body %q", rule.Pattern.String(), bodyNonMatching) - } -} - -func TestDefaultTripOn_RegistrationSlices(t *testing.T) { - t.Parallel() - - require.NotEmpty(t, Registration.DefaultTripOn, "Registration must carry defaults") - require.Len(t, Registration.DefaultTripOn, 3) -} - -// --------------------------------------------------------------------------- -// Provider resolution tests -// --------------------------------------------------------------------------- - -func TestFactoryApplyDefaults(t *testing.T) { - t.Parallel() - - t.Run("no config trip_on gets defaults", func(t *testing.T) { - t.Parallel() - - factory := providers.NewProviderFactory() - factory.Add(Registration) - - cfg := providers.ProviderConfig{ - Name: "kimi-test", - Type: "kimicode", - Resilience: config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{ - // TripOn nil — factory should apply defaults. - }, - }, - } - - p, err := factory.Create(cfg) - require.NoError(t, err) - require.NotNil(t, p) - - // Create takes cfg by value, so mutation is local. - // Verify via the e2e test that defaults actually trip the breaker. - }) - - t.Run("explicit empty list disables defaults", func(t *testing.T) { - t.Parallel() - - factory := providers.NewProviderFactory() - factory.Add(Registration) - - cfg := providers.ProviderConfig{ - Name: "kimi-test", - Type: "kimicode", - Resilience: config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{ - TripOn: []config.TripRuleConfig{}, // explicit empty - }, - }, - } - - _, err := factory.Create(cfg) - require.NoError(t, err) - // Empty list is a valid config — provider creates with no trip rules. - }) - - t.Run("non-empty config overrides defaults", func(t *testing.T) { - t.Parallel() - - factory := providers.NewProviderFactory() - factory.Add(Registration) - - cfg := providers.ProviderConfig{ - Name: "kimi-test", - Type: "kimicode", - Resilience: config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{ - TripOn: []config.TripRuleConfig{ - {Match: `custom rule`, TTL: 1 * time.Minute}, - }, - }, - }, - } - - p, err := factory.Create(cfg) - require.NoError(t, err) - require.NotNil(t, p) - }) - - t.Run("unknown type gets no defaults", func(t *testing.T) { - t.Parallel() - - factory := providers.NewProviderFactory() - // No registration for "unknown" type. - - cfg := providers.ProviderConfig{ - Name: "unknown-test", - Type: "unknown", - Resilience: config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{ - // TripOn nil — should stay nil. - }, - }, - } - _, err := factory.Create(cfg) - require.Error(t, err) - }) -} - -// --------------------------------------------------------------------------- -// E2E: default rules trip the breaker via the public client API -// --------------------------------------------------------------------------- - -func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { - t.Parallel() - - server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) - - cfg := llmclient.Config{ - ProviderName: "kimicode", - BaseURL: server.URL, - Retry: config.DefaultRetryConfig(), - CircuitBreaker: config.CircuitBreakerConfig{ - Enabled: true, - FailureThreshold: 5, - SuccessThreshold: 1, - Timeout: 20 * time.Millisecond, - TripOn: kimicodeDefaultTripOn(), - }, - } - client := llmclient.New(cfg, nil) - - // First request trips the breaker. - err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) - require.Error(t, err) - assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") - - // Second request is rejected immediately. - err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) - require.Error(t, err) - var gwErr *core.GatewayError - require.ErrorAs(t, err, &gwErr) - assert.Contains(t, gwErr.Message, "circuit breaker is open") - assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") - - // Reset closes immediately. - client.ResetBreaker() -} - -func TestDefaultRules_5HourLimitTrips(t *testing.T) { - t.Parallel() - - server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyHourLimit+`","code":"hourly_limit"}}`) - - cfg := llmclient.Config{ - ProviderName: "kimicode", - BaseURL: server.URL, - Retry: config.DefaultRetryConfig(), - CircuitBreaker: config.CircuitBreakerConfig{ - Enabled: true, - FailureThreshold: 5, - SuccessThreshold: 1, - Timeout: 20 * time.Millisecond, - TripOn: kimicodeDefaultTripOn(), - }, - } - client := llmclient.New(cfg, nil) - - err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) - require.Error(t, err) - - // Verify breaker is open (second request rejected). - err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) - require.Error(t, err) - var gwErr *core.GatewayError - require.ErrorAs(t, err, &gwErr) - assert.Contains(t, gwErr.Message, "circuit breaker is open") - assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") -} - -func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { - t.Parallel() - - server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) - - cfg := llmclient.Config{ - ProviderName: "kimicode", - BaseURL: server.URL, - Retry: config.DefaultRetryConfig(), - CircuitBreaker: config.CircuitBreakerConfig{ - Enabled: true, - FailureThreshold: 5, - SuccessThreshold: 1, - Timeout: 1 * time.Minute, - TripOn: kimicodeDefaultTripOn(), - }, - } - client := llmclient.New(cfg, nil) - - err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) - require.Error(t, err) - - client.ResetBreaker() - - // After reset, traffic flows again. - err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) - require.Error(t, err) // still fails (server returns 403), but goes upstream - assert.Equal(t, 2, capture.Count(), - "reset must let the request reach upstream instead of failing fast on the open breaker") -} - -// TestFactoryPath_E2E verifies that factory-applied default trip rules reach -// the circuit breaker end-to-end: Registration.DefaultTripOn -> -// ProviderFactory.Create -> provider -> llmclient. The requests below go -// through the factory-created provider itself, not a hand-built client. -func TestFactoryPath_E2E(t *testing.T) { - t.Parallel() - - server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) - - factory := providers.NewProviderFactory() - factory.Add(Registration) - - // TripOn nil — the factory must apply Registration.DefaultTripOn. - p, err := factory.Create(providers.ProviderConfig{ - Name: "kimi-test", - Type: "kimicode", - APIKey: "test-key", - BaseURL: server.URL, - Resilience: config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{ - Enabled: true, - FailureThreshold: 5, - SuccessThreshold: 1, - Timeout: 20 * time.Millisecond, - }, - }, - }) - require.NoError(t, err) - - req := &core.ChatRequest{ - Model: "kimi-k2", - Messages: []core.Message{{Role: "user", Content: "hi"}}, - } - - // First request reaches upstream and trips the breaker via defaults. - _, err = p.ChatCompletion(context.Background(), req) - require.Error(t, err) - assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") - - // Second request is rejected immediately by the circuit breaker. - _, err = p.ChatCompletion(context.Background(), req) - require.Error(t, err) - assert.Contains(t, err.Error(), "circuit breaker is open") - assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") -} - -// Non-gateway errors don't trip quota rules. -// Covered by llmclient/circuit_breaker_trip_test.go.TestQuotaTripTTL -// which exercises the same logic with unexported field access. diff --git a/internal/providers/kimicode/trip_defaults_test.go b/internal/providers/kimicode/trip_defaults_test.go new file mode 100644 index 000000000..6eebc60f2 --- /dev/null +++ b/internal/providers/kimicode/trip_defaults_test.go @@ -0,0 +1,379 @@ +package kimicode + +import ( + "context" + "net/http" + "regexp" + "testing" + "time" + + config "github.com/enterpilot/gomodel/config" + "github.com/enterpilot/gomodel/internal/core" + "github.com/enterpilot/gomodel/internal/llmclient" + "github.com/enterpilot/gomodel/internal/providers" + "github.com/enterpilot/gomodel/internal/providers/providertest" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// --------------------------------------------------------------------------- +// Default trip rules — compile, match, non-match +// --------------------------------------------------------------------------- + +// Pinned upstream error fragments observed in the kimicode-weselben audit log. +const ( + bodyWeeklyLimit = "You've reached your weekly (7-day) usage limit, please try again after the limit resets" + bodyHourLimit = "5-hour usage limit reached, please try again later" + bodyUsageLimit = "usage limit reached for this plan" + bodyQuotaExceed = "quota exceeded for organization" + bodyNonMatching = "rate limit exceeded" + // Near-miss: mentions "quota" but is not a quota-limit error; the + // catch-all rule must not match it. + bodyQuotaNearMiss = "access denied: quota check failed" +) + +func compileTripRulesForTest(rules []config.TripRuleConfig) ([]llmclient.TripRule, error) { + if len(rules) == 0 { + return nil, nil + } + compiled := make([]llmclient.TripRule, 0, len(rules)) + for _, rule := range rules { + pat, err := regexp.Compile(rule.Match) + if err != nil { + return nil, err + } + compiled = append(compiled, llmclient.TripRule{Pattern: pat, TTL: rule.TTL}) + } + return compiled, nil +} + +func TestDefaultTripOn_Compile(t *testing.T) { + t.Parallel() + + rules := kimicodeDefaultTripOn() + require.NotEmpty(t, rules, "must ship at least one default rule") + require.Len(t, rules, 3, "three pinned rules expected") + + for _, rule := range rules { + r := rule + t.Run(r.Match, func(t *testing.T) { + err := config.ValidateResilience(config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{TripOn: []config.TripRuleConfig{r}}, + }) + require.NoError(t, err, "rule[%d] match must be a valid regex", 0) + assert.NotZero(t, r.TTL, "rule[%d] must carry a TTL", 0) + }) + } +} + +func TestDefaultTripOn_MatchesPinnedBodies(t *testing.T) { + t.Parallel() + + rules := kimicodeDefaultTripOn() + compiled, err := compileTripRulesForTest(rules) + require.NoError(t, err) + + tests := []struct { + name string + body string + wantOK bool + wantTTL time.Duration + }{ + {"weekly limit", bodyWeeklyLimit, true, 4 * time.Hour}, + {"5-hour limit", bodyHourLimit, true, 30 * time.Minute}, + {"usage limit", bodyUsageLimit, true, 15 * time.Minute}, + {"quota exceeded", bodyQuotaExceed, true, 15 * time.Minute}, + {"non-matching rate limit", bodyNonMatching, false, 0}, + {"quota near-miss", bodyQuotaNearMiss, false, 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gatewayErr := core.NewProviderError("kimicode", http.StatusForbidden, tt.body, nil) + + found := false + for _, rule := range compiled { + if rule.Pattern.MatchString(gatewayErr.Message) { + found = true + assert.True(t, tt.wantOK, + "rule %q should successfully match body %q", rule.Pattern.String(), tt.name) + assert.Equal(t, tt.wantTTL, rule.TTL, + "rule %q TTL mismatch for body %q", rule.Pattern.String(), tt.name) + break + } + } + assert.Equal(t, tt.wantOK, found, + "rule set should match body %q", tt.name) + }) + } +} + +func TestDefaultTripOn_NonMatchingBodyDoesNotTrip(t *testing.T) { + t.Parallel() + + compiled, err := compileTripRulesForTest(kimicodeDefaultTripOn()) + require.NoError(t, err) + + gatewayErr := core.NewProviderError("kimicode", http.StatusForbidden, bodyNonMatching, nil) + + for _, rule := range compiled { + assert.False(t, rule.Pattern.MatchString(gatewayErr.Message), + "rule %q should not match non-matching body %q", rule.Pattern.String(), bodyNonMatching) + } +} + +func TestDefaultTripOn_RegistrationSlices(t *testing.T) { + t.Parallel() + + require.NotEmpty(t, Registration.DefaultTripOn, "Registration must carry defaults") + require.Len(t, Registration.DefaultTripOn, 3) +} + +// --------------------------------------------------------------------------- +// Provider resolution tests +// --------------------------------------------------------------------------- + +func TestFactoryApplyDefaults(t *testing.T) { + t.Parallel() + + t.Run("no config trip_on gets defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + // TripOn nil — factory should apply defaults. + }, + }, + } + + p, err := factory.Create(cfg) + require.NoError(t, err) + require.NotNil(t, p) + + // Create takes cfg by value, so mutation is local. + // Verify via the e2e test that defaults actually trip the breaker. + }) + + t.Run("explicit empty list disables defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + TripOn: []config.TripRuleConfig{}, // explicit empty + }, + }, + } + + _, err := factory.Create(cfg) + require.NoError(t, err) + // Empty list is a valid config — provider creates with no trip rules. + }) + + t.Run("non-empty config overrides defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + cfg := providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + TripOn: []config.TripRuleConfig{ + {Match: `custom rule`, TTL: 1 * time.Minute}, + }, + }, + }, + } + + p, err := factory.Create(cfg) + require.NoError(t, err) + require.NotNil(t, p) + }) + + t.Run("unknown type gets no defaults", func(t *testing.T) { + t.Parallel() + + factory := providers.NewProviderFactory() + // No registration for "unknown" type. + + cfg := providers.ProviderConfig{ + Name: "unknown-test", + Type: "unknown", + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + // TripOn nil — should stay nil. + }, + }, + } + _, err := factory.Create(cfg) + require.Error(t, err) + }) +} + +// --------------------------------------------------------------------------- +// E2E: default rules trip the breaker via the public client API +// --------------------------------------------------------------------------- + +func TestDefaultRules_TripBreakerOnWeeklyBody(t *testing.T) { + t.Parallel() + + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) + + cfg := llmclient.Config{ + ProviderName: "kimicode", + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 20 * time.Millisecond, + TripOn: kimicodeDefaultTripOn(), + }, + } + client := llmclient.New(cfg, nil) + + // First request trips the breaker. + err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") + + // Second request is rejected immediately. + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + var gwErr *core.GatewayError + require.ErrorAs(t, err, &gwErr) + assert.Contains(t, gwErr.Message, "circuit breaker is open") + assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") + + // Reset closes immediately. + client.ResetBreaker() +} + +func TestDefaultRules_5HourLimitTrips(t *testing.T) { + t.Parallel() + + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyHourLimit+`","code":"hourly_limit"}}`) + + cfg := llmclient.Config{ + ProviderName: "kimicode", + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 20 * time.Millisecond, + TripOn: kimicodeDefaultTripOn(), + }, + } + client := llmclient.New(cfg, nil) + + err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + + // Verify breaker is open (second request rejected). + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + var gwErr *core.GatewayError + require.ErrorAs(t, err, &gwErr) + assert.Contains(t, gwErr.Message, "circuit breaker is open") + assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") +} + +func TestDefaultRules_ResetClearsQuotaWindow(t *testing.T) { + t.Parallel() + + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) + + cfg := llmclient.Config{ + ProviderName: "kimicode", + BaseURL: server.URL, + Retry: config.DefaultRetryConfig(), + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 1 * time.Minute, + TripOn: kimicodeDefaultTripOn(), + }, + } + client := llmclient.New(cfg, nil) + + err := client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) + + client.ResetBreaker() + + // After reset, traffic flows again. + err = client.Do(context.Background(), llmclient.Request{Method: http.MethodGet, Endpoint: "/test"}, nil) + require.Error(t, err) // still fails (server returns 403), but goes upstream + assert.Equal(t, 2, capture.Count(), + "reset must let the request reach upstream instead of failing fast on the open breaker") +} + +// TestFactoryPath_E2E verifies that factory-applied default trip rules reach +// the circuit breaker end-to-end: Registration.DefaultTripOn -> +// ProviderFactory.Create -> provider -> llmclient. The requests below go +// through the factory-created provider itself, not a hand-built client. +func TestFactoryPath_E2E(t *testing.T) { + t.Parallel() + + server, capture := providertest.JSONServer(t, http.StatusForbidden, `{"error":{"message":"`+bodyWeeklyLimit+`","code":"weekly_limit"}}`) + + factory := providers.NewProviderFactory() + factory.Add(Registration) + + // TripOn nil — the factory must apply Registration.DefaultTripOn. + p, err := factory.Create(providers.ProviderConfig{ + Name: "kimi-test", + Type: "kimicode", + APIKey: "test-key", + BaseURL: server.URL, + Resilience: config.ResilienceConfig{ + CircuitBreaker: config.CircuitBreakerConfig{ + Enabled: true, + FailureThreshold: 5, + SuccessThreshold: 1, + Timeout: 20 * time.Millisecond, + }, + }, + }) + require.NoError(t, err) + + req := &core.ChatRequest{ + Model: "kimi-k2", + Messages: []core.Message{{Role: "user", Content: "hi"}}, + } + + // First request reaches upstream and trips the breaker via defaults. + _, err = p.ChatCompletion(context.Background(), req) + require.Error(t, err) + assert.Equal(t, 1, capture.Count(), "first failure must reach upstream") + + // Second request is rejected immediately by the circuit breaker. + _, err = p.ChatCompletion(context.Background(), req) + require.Error(t, err) + assert.Contains(t, err.Error(), "circuit breaker is open") + assert.Equal(t, 1, capture.Count(), "breaker must prevent upstream reach") +} + +// Non-gateway errors don't trip quota rules. +// Covered by llmclient/circuit_breaker_trip_test.go.TestQuotaTripTTL +// which exercises the same logic with unexported field access. From 8fe4a5665008e611beed928a5ab2377efb68135a Mon Sep 17 00:00:00 2001 From: weselben Date: Sat, 19 Sep 2026 15:57:04 +0000 Subject: [PATCH 7/7] feat(kimicode): named default trip groups, evaluation ordered by group name kimicode ships three named groups (1_weekly_limit, 2_five_hour_limit, 3_usage_limit); the numeric prefix is the evaluation priority, matching the named-map config shape so env overrides can target single rules, e.g. KIMICODE_CIRCUIT_BREAKER_TRIP_ON_1_WEEKLY_LIMIT_MATCH --- docs/providers/kimicode.mdx | 21 ++++++++++++------ internal/providers/factory.go | 22 ++++++++++++------- internal/providers/kimicode/trip_defaults.go | 12 +++++----- .../providers/kimicode/trip_defaults_test.go | 12 +++++----- 4 files changed, 41 insertions(+), 26 deletions(-) diff --git a/docs/providers/kimicode.mdx b/docs/providers/kimicode.mdx index 70c17b30b..15c4878c7 100644 --- a/docs/providers/kimicode.mdx +++ b/docs/providers/kimicode.mdx @@ -81,17 +81,24 @@ Kimi Code rejects requests once a quota bucket is exhausted (HTTP 403). To fail fast instead of retrying against a spent quota, `kimicode` providers ship built-in circuit-breaker trip rules (see [Resilience](/advanced/resilience)): -| Matches upstream message | Breaker TTL | -| --------------------------- | ----------- | -| `weekly \(7-day\) usage limit` | 4h | -| `5-hour usage limit` | 30m | -| `usage limit\|quota exceeded` | 15m | +| Group name | Matches upstream message | Breaker TTL | +| ------------------- | ----------------------------- | ----------- | +| `1_weekly_limit` | `weekly \(7-day\) usage limit` | 4h | +| `2_five_hour_limit` | `5-hour usage limit` | 30m | +| `3_usage_limit` | `usage limit\|quota exceeded` | 15m | + +The group names double as evaluation priority: rules are evaluated in name +order, so the specific patterns match before the catch-all. The rules apply only when the provider has no `trip_on` of its own: - Omit `trip_on` to inherit these defaults. -- Set your own `trip_on` list to replace them entirely. -- Set `trip_on: []` to disable quota tripping for that provider. +- Set your own `trip_on` groups to replace them entirely. +- Set `trip_on: {}` to disable quota tripping for that provider. + +Env overrides match by group name, e.g. +`KIMICODE_CIRCUIT_BREAKER_TRIP_ON_1_WEEKLY_LIMIT_MATCH` replaces the weekly +rule's pattern. Dashboard-managed providers always inherit the defaults when no rules are configured; the dashboard editor has no explicit-disable concept. An open diff --git a/internal/providers/factory.go b/internal/providers/factory.go index 5d0df96d9..e29ee9848 100644 --- a/internal/providers/factory.go +++ b/internal/providers/factory.go @@ -89,7 +89,7 @@ type Registration struct { // The caller's explicit trip_on wins over defaults. A nil entry // on the struct is inert — no type ships defaults unless it assigns // one here. - DefaultTripOn []config.TripRuleConfig + DefaultTripOn config.TripRuleMap } // ProviderFactory manages provider registration and creation. @@ -98,7 +98,7 @@ type ProviderFactory struct { builders map[string]ProviderConstructor discoveryConfigs map[string]DiscoveryConfig passthroughEnrichers map[string]core.PassthroughSemanticEnricher - defaultTripOnRules map[string][]config.TripRuleConfig + defaultTripOnRules map[string]config.TripRuleMap hooks llmclient.Hooks } @@ -108,7 +108,7 @@ func NewProviderFactory() *ProviderFactory { builders: make(map[string]ProviderConstructor), discoveryConfigs: make(map[string]DiscoveryConfig), passthroughEnrichers: make(map[string]core.PassthroughSemanticEnricher), - defaultTripOnRules: make(map[string][]config.TripRuleConfig), + defaultTripOnRules: make(map[string]config.TripRuleMap), } } @@ -155,9 +155,9 @@ func (f *ProviderFactory) Add(reg Registration) { delete(f.passthroughEnrichers, reg.Type) } if len(reg.DefaultTripOn) > 0 { - // Copy so callers may reuse the same slice across registrations. - cp := make([]config.TripRuleConfig, len(reg.DefaultTripOn)) - copy(cp, reg.DefaultTripOn) + // Copy so callers may reuse the same map across registrations. + cp := make(config.TripRuleMap, len(reg.DefaultTripOn)) + maps.Copy(cp, reg.DefaultTripOn) f.defaultTripOnRules[reg.Type] = cp } else { delete(f.defaultTripOnRules, reg.Type) @@ -271,13 +271,19 @@ func (f *ProviderFactory) knowsType(providerType string) bool { // defaultTripOn returns the built-in trip rules for the given provider type. // Returns nil when no defaults are declared or the type is unknown. -func (f *ProviderFactory) defaultTripOn(providerType string) []config.TripRuleConfig { +func (f *ProviderFactory) defaultTripOn(providerType string) config.TripRuleMap { f.mu.RLock() defer f.mu.RUnlock() if f.defaultTripOnRules == nil { return nil } - return f.defaultTripOnRules[providerType] + defaults := f.defaultTripOnRules[providerType] + if len(defaults) == 0 { + return nil + } + cp := make(config.TripRuleMap, len(defaults)) + maps.Copy(cp, defaults) + return cp } // RegisteredTypes returns a list of all registered provider types. diff --git a/internal/providers/kimicode/trip_defaults.go b/internal/providers/kimicode/trip_defaults.go index c7df8713e..d30ba6776 100644 --- a/internal/providers/kimicode/trip_defaults.go +++ b/internal/providers/kimicode/trip_defaults.go @@ -21,15 +21,17 @@ import ( // // Pin the exact body fragments in unit tests (see kimicode_test.go). -func kimicodeDefaultTripOn() []config.TripRuleConfig { - return []config.TripRuleConfig{ +// Group names double as evaluation priority: resolution evaluates rules in +// name order, so the most specific pattern sorts first. +func kimicodeDefaultTripOn() config.TripRuleMap { + return config.TripRuleMap{ // Weekly 7-day plan limit → 4 hour cooldown. - {Match: `weekly \(7-day\) usage limit`, TTL: 4 * time.Hour}, + "1_weekly_limit": {Match: `weekly \(7-day\) usage limit`, TTL: 4 * time.Hour}, // 5-hour sliding-window limit. - {Match: `5-hour usage limit`, TTL: 30 * time.Minute}, + "2_five_hour_limit": {Match: `5-hour usage limit`, TTL: 30 * time.Minute}, // Catch-all for usage-limit and quota-exceeded errors. "quota" alone // is deliberately not matched: non-limit 403s mentioning quota must // not trip the breaker. - {Match: `usage limit|quota exceeded`, TTL: 15 * time.Minute}, + "3_usage_limit": {Match: `usage limit|quota exceeded`, TTL: 15 * time.Minute}, } } diff --git a/internal/providers/kimicode/trip_defaults_test.go b/internal/providers/kimicode/trip_defaults_test.go index 6eebc60f2..9ff1255cd 100644 --- a/internal/providers/kimicode/trip_defaults_test.go +++ b/internal/providers/kimicode/trip_defaults_test.go @@ -32,12 +32,12 @@ const ( bodyQuotaNearMiss = "access denied: quota check failed" ) -func compileTripRulesForTest(rules []config.TripRuleConfig) ([]llmclient.TripRule, error) { +func compileTripRulesForTest(rules config.TripRuleMap) ([]llmclient.TripRule, error) { if len(rules) == 0 { return nil, nil } compiled := make([]llmclient.TripRule, 0, len(rules)) - for _, rule := range rules { + for _, rule := range rules.List() { pat, err := regexp.Compile(rule.Match) if err != nil { return nil, err @@ -58,7 +58,7 @@ func TestDefaultTripOn_Compile(t *testing.T) { r := rule t.Run(r.Match, func(t *testing.T) { err := config.ValidateResilience(config.ResilienceConfig{ - CircuitBreaker: config.CircuitBreakerConfig{TripOn: []config.TripRuleConfig{r}}, + CircuitBreaker: config.CircuitBreakerConfig{TripOn: config.TripRuleMap{r.Name: r}}, }) require.NoError(t, err, "rule[%d] match must be a valid regex", 0) assert.NotZero(t, r.TTL, "rule[%d] must carry a TTL", 0) @@ -173,7 +173,7 @@ func TestFactoryApplyDefaults(t *testing.T) { Type: "kimicode", Resilience: config.ResilienceConfig{ CircuitBreaker: config.CircuitBreakerConfig{ - TripOn: []config.TripRuleConfig{}, // explicit empty + TripOn: config.TripRuleMap{}, // explicit empty }, }, } @@ -194,8 +194,8 @@ func TestFactoryApplyDefaults(t *testing.T) { Type: "kimicode", Resilience: config.ResilienceConfig{ CircuitBreaker: config.CircuitBreakerConfig{ - TripOn: []config.TripRuleConfig{ - {Match: `custom rule`, TTL: 1 * time.Minute}, + TripOn: config.TripRuleMap{ + "custom": {Match: `custom rule`, TTL: 1 * time.Minute}, }, }, },