Merge pull request #2133 from DragonFSKY/fix/2061-stale-modelstates

fix(auth): prevent stale runtime state inheritance from disabled auth entries
2026-03-28 20:50:57 +08:00
parent 10b824fcac 74b862d8b8
commit 73c831747b
4 changed files with 250 additions and 6 deletions
--- a/sdk/cliproxy/auth/conductor.go
+++ b/sdk/cliproxy/auth/conductor.go
@@ -923,8 +923,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()
--- 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)
+	}
+}
--- 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)
--- a/sdk/cliproxy/service_stale_state_test.go
+++ b/sdk/cliproxy/service_stale_state_test.go
@@ -0,0 +1,85 @@
+package cliproxy
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/router-for-me/CLIProxyAPI/v6/internal/registry"
+	coreauth "github.com/router-for-me/CLIProxyAPI/v6/sdk/cliproxy/auth"
+	"github.com/router-for-me/CLIProxyAPI/v6/sdk/config"
+)
+
+func TestServiceApplyCoreAuthAddOrUpdate_DeleteReAddDoesNotInheritStaleRuntimeState(t *testing.T) {
+	service := &Service{
+		cfg:         &config.Config{},
+		coreManager: coreauth.NewManager(nil, nil, nil),
+	}
+
+	authID := "service-stale-state-auth"
+	modelID := "stale-model"
+	lastRefreshedAt := time.Date(2026, time.March, 1, 8, 0, 0, 0, time.UTC)
+	nextRefreshAfter := lastRefreshedAt.Add(30 * time.Minute)
+
+	t.Cleanup(func() {
+		GlobalModelRegistry().UnregisterClient(authID)
+	})
+
+	service.applyCoreAuthAddOrUpdate(context.Background(), &coreauth.Auth{
+		ID:               authID,
+		Provider:         "claude",
+		Status:           coreauth.StatusActive,
+		LastRefreshedAt:  lastRefreshedAt,
+		NextRefreshAfter: nextRefreshAfter,
+		ModelStates: map[string]*coreauth.ModelState{
+			modelID: {
+				Quota: coreauth.QuotaState{BackoffLevel: 7},
+			},
+		},
+	})
+
+	service.applyCoreAuthRemoval(context.Background(), authID)
+
+	disabled, ok := service.coreManager.GetByID(authID)
+	if !ok || disabled == nil {
+		t.Fatalf("expected disabled auth after removal")
+	}
+	if !disabled.Disabled || disabled.Status != coreauth.StatusDisabled {
+		t.Fatalf("expected disabled auth after removal, got disabled=%v status=%v", disabled.Disabled, disabled.Status)
+	}
+	if disabled.LastRefreshedAt.IsZero() {
+		t.Fatalf("expected disabled auth to still carry prior LastRefreshedAt for regression setup")
+	}
+	if disabled.NextRefreshAfter.IsZero() {
+		t.Fatalf("expected disabled auth to still carry prior NextRefreshAfter for regression setup")
+	}
+	if len(disabled.ModelStates) == 0 {
+		t.Fatalf("expected disabled auth to still carry prior ModelStates for regression setup")
+	}
+
+	service.applyCoreAuthAddOrUpdate(context.Background(), &coreauth.Auth{
+		ID:       authID,
+		Provider: "claude",
+		Status:   coreauth.StatusActive,
+	})
+
+	updated, ok := service.coreManager.GetByID(authID)
+	if !ok || updated == nil {
+		t.Fatalf("expected re-added auth to be present")
+	}
+	if updated.Disabled {
+		t.Fatalf("expected re-added auth to be active")
+	}
+	if !updated.LastRefreshedAt.IsZero() {
+		t.Fatalf("expected LastRefreshedAt to reset on delete -> re-add, got %v", updated.LastRefreshedAt)
+	}
+	if !updated.NextRefreshAfter.IsZero() {
+		t.Fatalf("expected NextRefreshAfter to reset on delete -> re-add, got %v", updated.NextRefreshAfter)
+	}
+	if len(updated.ModelStates) != 0 {
+		t.Fatalf("expected ModelStates to reset on delete -> re-add, got %d entries", len(updated.ModelStates))
+	}
+	if models := registry.GetGlobalRegistry().GetModelsForClient(authID); len(models) == 0 {
+		t.Fatalf("expected re-added auth to re-register models in global registry")
+	}
+}