From 5c817a9b429b0bf657fb397c5befa3945e8618db Mon Sep 17 00:00:00 2001 From: DragonFSKY Date: Sat, 14 Mar 2026 23:46:23 +0800 Subject: [PATCH] fix(auth): prevent stale ModelStates inheritance from disabled auth entries When an auth file is deleted and re-created with the same path/ID, the new auth could inherit stale ModelStates (cooldown/backoff) from the previously disabled entry, preventing it from being routed. Gate runtime state inheritance (ModelStates, LastRefreshedAt, NextRefreshAfter) on both existing and incoming auth being non-disabled in Manager.Update and Service.applyCoreAuthAddOrUpdate. Closes #2061 --- sdk/cliproxy/auth/conductor.go | 6 +- sdk/cliproxy/auth/conductor_update_test.go | 155 +++++++++++++++++++++ sdk/cliproxy/service.go | 10 +- 3 files changed, 165 insertions(+), 6 deletions(-) diff --git a/sdk/cliproxy/auth/conductor.go b/sdk/cliproxy/auth/conductor.go index b29e04db..aab23305 100644 --- a/sdk/cliproxy/auth/conductor.go +++ b/sdk/cliproxy/auth/conductor.go @@ -832,8 +832,10 @@ func (m *Manager) Update(ctx context.Context, auth *Auth) (*Auth, error) { auth.Index = existing.Index auth.indexAssigned = existing.indexAssigned } - if len(auth.ModelStates) == 0 && len(existing.ModelStates) > 0 { - auth.ModelStates = existing.ModelStates + if !existing.Disabled && existing.Status != StatusDisabled && !auth.Disabled && auth.Status != StatusDisabled { + if len(auth.ModelStates) == 0 && len(existing.ModelStates) > 0 { + auth.ModelStates = existing.ModelStates + } } } auth.EnsureIndex() diff --git a/sdk/cliproxy/auth/conductor_update_test.go b/sdk/cliproxy/auth/conductor_update_test.go index f058f517..7dd44ff8 100644 --- a/sdk/cliproxy/auth/conductor_update_test.go +++ b/sdk/cliproxy/auth/conductor_update_test.go @@ -47,3 +47,158 @@ func TestManager_Update_PreservesModelStates(t *testing.T) { t.Fatalf("expected BackoffLevel to be %d, got %d", backoffLevel, state.Quota.BackoffLevel) } } + +func TestManager_Update_DisabledExistingDoesNotInheritModelStates(t *testing.T) { + m := NewManager(nil, nil, nil) + + // Register a disabled auth with existing ModelStates. + if _, err := m.Register(context.Background(), &Auth{ + ID: "auth-disabled", + Provider: "claude", + Disabled: true, + Status: StatusDisabled, + ModelStates: map[string]*ModelState{ + "stale-model": { + Quota: QuotaState{BackoffLevel: 5}, + }, + }, + }); err != nil { + t.Fatalf("register auth: %v", err) + } + + // Update with empty ModelStates — should NOT inherit stale states. + if _, err := m.Update(context.Background(), &Auth{ + ID: "auth-disabled", + Provider: "claude", + Disabled: true, + Status: StatusDisabled, + }); err != nil { + t.Fatalf("update auth: %v", err) + } + + updated, ok := m.GetByID("auth-disabled") + if !ok || updated == nil { + t.Fatalf("expected auth to be present") + } + if len(updated.ModelStates) != 0 { + t.Fatalf("expected disabled auth NOT to inherit ModelStates, got %d entries", len(updated.ModelStates)) + } +} + +func TestManager_Update_ActiveToDisabledDoesNotInheritModelStates(t *testing.T) { + m := NewManager(nil, nil, nil) + + // Register an active auth with ModelStates (simulates existing live auth). + if _, err := m.Register(context.Background(), &Auth{ + ID: "auth-a2d", + Provider: "claude", + Status: StatusActive, + ModelStates: map[string]*ModelState{ + "stale-model": { + Quota: QuotaState{BackoffLevel: 9}, + }, + }, + }); err != nil { + t.Fatalf("register auth: %v", err) + } + + // File watcher deletes config → synthesizes Disabled=true auth → Update. + // Even though existing is active, incoming auth is disabled → skip inheritance. + if _, err := m.Update(context.Background(), &Auth{ + ID: "auth-a2d", + Provider: "claude", + Disabled: true, + Status: StatusDisabled, + }); err != nil { + t.Fatalf("update auth: %v", err) + } + + updated, ok := m.GetByID("auth-a2d") + if !ok || updated == nil { + t.Fatalf("expected auth to be present") + } + if len(updated.ModelStates) != 0 { + t.Fatalf("expected active→disabled transition NOT to inherit ModelStates, got %d entries", len(updated.ModelStates)) + } +} + +func TestManager_Update_DisabledToActiveDoesNotInheritStaleModelStates(t *testing.T) { + m := NewManager(nil, nil, nil) + + // Register a disabled auth with stale ModelStates. + if _, err := m.Register(context.Background(), &Auth{ + ID: "auth-d2a", + Provider: "claude", + Disabled: true, + Status: StatusDisabled, + ModelStates: map[string]*ModelState{ + "stale-model": { + Quota: QuotaState{BackoffLevel: 4}, + }, + }, + }); err != nil { + t.Fatalf("register auth: %v", err) + } + + // Re-enable: incoming auth is active, existing is disabled → skip inheritance. + if _, err := m.Update(context.Background(), &Auth{ + ID: "auth-d2a", + Provider: "claude", + Status: StatusActive, + }); err != nil { + t.Fatalf("update auth: %v", err) + } + + updated, ok := m.GetByID("auth-d2a") + if !ok || updated == nil { + t.Fatalf("expected auth to be present") + } + if len(updated.ModelStates) != 0 { + t.Fatalf("expected disabled→active transition NOT to inherit stale ModelStates, got %d entries", len(updated.ModelStates)) + } +} + +func TestManager_Update_ActiveInheritsModelStates(t *testing.T) { + m := NewManager(nil, nil, nil) + + model := "active-model" + backoffLevel := 3 + + // Register an active auth with ModelStates. + if _, err := m.Register(context.Background(), &Auth{ + ID: "auth-active", + Provider: "claude", + Status: StatusActive, + ModelStates: map[string]*ModelState{ + model: { + Quota: QuotaState{BackoffLevel: backoffLevel}, + }, + }, + }); err != nil { + t.Fatalf("register auth: %v", err) + } + + // Update with empty ModelStates — both sides active → SHOULD inherit. + if _, err := m.Update(context.Background(), &Auth{ + ID: "auth-active", + Provider: "claude", + Status: StatusActive, + }); err != nil { + t.Fatalf("update auth: %v", err) + } + + updated, ok := m.GetByID("auth-active") + if !ok || updated == nil { + t.Fatalf("expected auth to be present") + } + if len(updated.ModelStates) == 0 { + t.Fatalf("expected active auth to inherit ModelStates") + } + state := updated.ModelStates[model] + if state == nil { + t.Fatalf("expected model state to be present") + } + if state.Quota.BackoffLevel != backoffLevel { + t.Fatalf("expected BackoffLevel to be %d, got %d", backoffLevel, state.Quota.BackoffLevel) + } +} diff --git a/sdk/cliproxy/service.go b/sdk/cliproxy/service.go index abe1deed..e5c7e76c 100644 --- a/sdk/cliproxy/service.go +++ b/sdk/cliproxy/service.go @@ -286,10 +286,12 @@ func (s *Service) applyCoreAuthAddOrUpdate(ctx context.Context, auth *coreauth.A var err error if existing, ok := s.coreManager.GetByID(auth.ID); ok { auth.CreatedAt = existing.CreatedAt - auth.LastRefreshedAt = existing.LastRefreshedAt - auth.NextRefreshAfter = existing.NextRefreshAfter - if len(auth.ModelStates) == 0 && len(existing.ModelStates) > 0 { - auth.ModelStates = existing.ModelStates + if !existing.Disabled && existing.Status != coreauth.StatusDisabled && !auth.Disabled && auth.Status != coreauth.StatusDisabled { + auth.LastRefreshedAt = existing.LastRefreshedAt + auth.NextRefreshAfter = existing.NextRefreshAfter + if len(auth.ModelStates) == 0 && len(existing.ModelStates) > 0 { + auth.ModelStates = existing.ModelStates + } } op = "update" _, err = s.coreManager.Update(ctx, auth)