From c675cf5e722ece075cde8cebb3b538b7838a196b Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 17:04:39 +0800 Subject: [PATCH 01/10] refactor(config): Implement reconciliation for providers and clients This commit introduces a reconciliation mechanism for handling configuration updates, significantly improving efficiency and resource management. Previously, reloading the configuration would tear down and recreate all access providers from scratch, regardless of whether their individual configurations had changed. This was inefficient and could disrupt services. The new `sdkaccess.ReconcileProviders` function now compares the old and new configurations to intelligently manage the provider lifecycle: - Unchanged providers are kept. - New providers are created. - Providers removed from the config are closed and discarded. - Providers with updated configurations are gracefully closed and recreated. To support this, a `Close()` method has been added to the `Provider` interface. A similar reconciliation logic has been applied to the client registration state in `state.RegisterClient`. This ensures that model registrations are accurately tracked when a client's configuration is updated, correctly handling added, removed, and unchanged models. Enhanced logging provides visibility into these operations. --- internal/api/server.go | 61 +++++--- internal/registry/model_registry.go | 234 +++++++++++++++++++++++----- internal/watcher/watcher.go | 79 ++++++++-- sdk/access/reconcile.go | 230 +++++++++++++++++++++++++++ sdk/cliproxy/service.go | 15 +- 5 files changed, 549 insertions(+), 70 deletions(-) create mode 100644 sdk/access/reconcile.go diff --git a/internal/api/server.go b/internal/api/server.go index 70bd487e..2a35b642 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -194,7 +194,7 @@ func NewServer(cfg *config.Config, authManager *auth.Manager, accessManager *sdk loggerToggle: toggle, configFilePath: configFilePath, } - s.applyAccessConfig(cfg) + s.applyAccessConfig(nil, cfg) // Initialize management handler s.mgmt = managementHandlers.NewHandler(cfg, configFilePath, authManager) if optionState.localPassword != "" { @@ -547,16 +547,23 @@ func corsMiddleware() gin.HandlerFunc { } } -func (s *Server) applyAccessConfig(cfg *config.Config) { - if s == nil || s.accessManager == nil { +func (s *Server) applyAccessConfig(oldCfg, newCfg *config.Config) { + if s == nil || s.accessManager == nil || newCfg == nil { return } - providers, err := sdkaccess.BuildProviders(cfg) + existing := s.accessManager.Providers() + providers, added, updated, removed, err := sdkaccess.ReconcileProviders(oldCfg, newCfg, existing) if err != nil { - log.Errorf("failed to update request auth providers: %v", err) + log.Errorf("failed to reconcile request auth providers: %v", err) return } s.accessManager.SetProviders(providers) + if len(added)+len(updated)+len(removed) > 0 { + log.Infof("server request auth providers reconciled (added=%d updated=%d removed=%d)", len(added), len(updated), len(removed)) + log.Debugf("server request auth provider details - added=%v updated=%v removed=%v", added, updated, removed) + } else { + log.Debug("server request auth providers unchanged after config update") + } } // UpdateClients updates the server's client list and configuration. @@ -566,44 +573,60 @@ func (s *Server) applyAccessConfig(cfg *config.Config) { // - clients: The new slice of AI service clients // - cfg: The new application configuration func (s *Server) UpdateClients(cfg *config.Config) { + oldCfg := s.cfg + // Update request logger enabled state if it has changed - if s.requestLogger != nil && s.cfg.RequestLog != cfg.RequestLog { + previousRequestLog := false + if oldCfg != nil { + previousRequestLog = oldCfg.RequestLog + } + if s.requestLogger != nil && (oldCfg == nil || previousRequestLog != cfg.RequestLog) { if s.loggerToggle != nil { s.loggerToggle(cfg.RequestLog) } else if toggler, ok := s.requestLogger.(interface{ SetEnabled(bool) }); ok { toggler.SetEnabled(cfg.RequestLog) } - log.Debugf("request logging updated from %t to %t", s.cfg.RequestLog, cfg.RequestLog) - } - - if s.cfg.LoggingToFile != cfg.LoggingToFile { - if err := logging.ConfigureLogOutput(cfg.LoggingToFile); err != nil { - log.Errorf("failed to reconfigure log output: %v", err) + if oldCfg != nil { + log.Debugf("request logging updated from %t to %t", previousRequestLog, cfg.RequestLog) } else { - log.Debugf("logging_to_file updated from %t to %t", s.cfg.LoggingToFile, cfg.LoggingToFile) + log.Debugf("request logging toggled to %t", cfg.RequestLog) } } - if s.cfg == nil || s.cfg.UsageStatisticsEnabled != cfg.UsageStatisticsEnabled { + if oldCfg != nil && oldCfg.LoggingToFile != cfg.LoggingToFile { + if err := logging.ConfigureLogOutput(cfg.LoggingToFile); err != nil { + log.Errorf("failed to reconfigure log output: %v", err) + } else { + log.Debugf("logging_to_file updated from %t to %t", oldCfg.LoggingToFile, cfg.LoggingToFile) + } + } + + if oldCfg == nil || oldCfg.UsageStatisticsEnabled != cfg.UsageStatisticsEnabled { usage.SetStatisticsEnabled(cfg.UsageStatisticsEnabled) - if s.cfg != nil { - log.Debugf("usage_statistics_enabled updated from %t to %t", s.cfg.UsageStatisticsEnabled, cfg.UsageStatisticsEnabled) + if oldCfg != nil { + log.Debugf("usage_statistics_enabled updated from %t to %t", oldCfg.UsageStatisticsEnabled, cfg.UsageStatisticsEnabled) + } else { + log.Debugf("usage_statistics_enabled toggled to %t", cfg.UsageStatisticsEnabled) } } // Update log level dynamically when debug flag changes - if s.cfg.Debug != cfg.Debug { + if oldCfg == nil || oldCfg.Debug != cfg.Debug { util.SetLogLevel(cfg) - log.Debugf("debug mode updated from %t to %t", s.cfg.Debug, cfg.Debug) + if oldCfg != nil { + log.Debugf("debug mode updated from %t to %t", oldCfg.Debug, cfg.Debug) + } else { + log.Debugf("debug mode toggled to %t", cfg.Debug) + } } + s.applyAccessConfig(oldCfg, cfg) s.cfg = cfg s.handlers.UpdateClients(cfg) if s.mgmt != nil { s.mgmt.SetConfig(cfg) s.mgmt.SetAuthManager(s.handlers.AuthManager) } - s.applyAccessConfig(cfg) // Count client sources from configuration and auth directory authFiles := util.CountAuthFiles(cfg.AuthDir) diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index 2987aa5d..0a941916 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -101,58 +101,220 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ r.mutex.Lock() defer r.mutex.Unlock() - // Remove any existing registration for this client - r.unregisterClientInternal(clientID) - provider := strings.ToLower(clientProvider) + seen := make(map[string]struct{}) modelIDs := make([]string, 0, len(models)) + newModels := make(map[string]*ModelInfo, len(models)) + for _, model := range models { + if model == nil || model.ID == "" { + continue + } + if _, exists := seen[model.ID]; exists { + continue + } + seen[model.ID] = struct{}{} + modelIDs = append(modelIDs, model.ID) + newModels[model.ID] = model + } + + if len(modelIDs) == 0 { + // No models supplied; unregister existing client state if present. + r.unregisterClientInternal(clientID) + delete(r.clientModels, clientID) + delete(r.clientProviders, clientID) + misc.LogCredentialSeparator() + return + } + now := time.Now() - for _, model := range models { - modelIDs = append(modelIDs, model.ID) - - if existing, exists := r.models[model.ID]; exists { - // Model already exists, increment count - existing.Count++ - existing.LastUpdated = now - if existing.SuspendedClients == nil { - existing.SuspendedClients = make(map[string]string) - } - if provider != "" { - if existing.Providers == nil { - existing.Providers = make(map[string]int) - } - existing.Providers[provider]++ - } - log.Debugf("Incremented count for model %s, now %d clients", model.ID, existing.Count) + oldModels, hadExisting := r.clientModels[clientID] + oldProvider, hadProvider := r.clientProviders[clientID] + providerChanged := hadProvider && oldProvider != provider + if !hadExisting { + // Pure addition path. + for _, modelID := range modelIDs { + model := newModels[modelID] + r.addModelRegistration(modelID, provider, model, now) + } + r.clientModels[clientID] = modelIDs + if provider != "" { + r.clientProviders[clientID] = provider } else { - // New model, create registration - registration := &ModelRegistration{ - Info: model, - Count: 1, - LastUpdated: now, - QuotaExceededClients: make(map[string]*time.Time), - SuspendedClients: make(map[string]string), - } - if provider != "" { - registration.Providers = map[string]int{provider: 1} - } - r.models[model.ID] = registration - log.Debugf("Registered new model %s from provider %s", model.ID, clientProvider) + delete(r.clientProviders, clientID) + } + log.Debugf("Registered client %s from provider %s with %d models", clientID, clientProvider, len(modelIDs)) + misc.LogCredentialSeparator() + return + } + + oldSet := make(map[string]struct{}, len(oldModels)) + for _, id := range oldModels { + oldSet[id] = struct{}{} + } + + added := make([]string, 0) + removed := make([]string, 0) + for _, id := range modelIDs { + if _, exists := oldSet[id]; !exists { + added = append(added, id) + } + } + for _, id := range oldModels { + if _, exists := newModels[id]; !exists { + removed = append(removed, id) } } - r.clientModels[clientID] = modelIDs + // Handle provider change for overlapping models before modifications. + if providerChanged && oldProvider != "" { + for _, id := range modelIDs { + if reg, ok := r.models[id]; ok && reg.Providers != nil { + if count, okProv := reg.Providers[oldProvider]; okProv { + if count <= 1 { + delete(reg.Providers, oldProvider) + } else { + reg.Providers[oldProvider] = count - 1 + } + } + } + } + } + + // Apply removals first to keep counters accurate. + for _, id := range removed { + r.removeModelRegistration(clientID, id, oldProvider, now) + } + + // Apply additions. + for _, id := range added { + model := newModels[id] + r.addModelRegistration(id, provider, model, now) + } + + // Update metadata for models that remain associated with the client. + addedSet := make(map[string]struct{}, len(added)) + for _, id := range added { + addedSet[id] = struct{}{} + } + for _, id := range modelIDs { + model := newModels[id] + if reg, ok := r.models[id]; ok { + reg.Info = cloneModelInfo(model) + reg.LastUpdated = now + if providerChanged { + if _, newlyAdded := addedSet[id]; newlyAdded { + continue + } + if reg.Providers == nil { + reg.Providers = make(map[string]int) + } + reg.Providers[provider]++ + } + } + } + + // Update client bookkeeping. + if len(modelIDs) > 0 { + r.clientModels[clientID] = modelIDs + } if provider != "" { r.clientProviders[clientID] = provider } else { delete(r.clientProviders, clientID) } - log.Debugf("Registered client %s from provider %s with %d models", clientID, clientProvider, len(models)) - // Separator at the end of the registration block (acts as boundary to next group) + + if len(added) == 0 && len(removed) == 0 && !providerChanged { + // Only metadata (e.g., display name) changed. + misc.LogCredentialSeparator() + return + } + + log.Debugf("Reconciled client %s (provider %s) models: +%d, -%d", clientID, provider, len(added), len(removed)) misc.LogCredentialSeparator() } +func (r *ModelRegistry) addModelRegistration(modelID, provider string, model *ModelInfo, now time.Time) { + if model == nil || modelID == "" { + return + } + if existing, exists := r.models[modelID]; exists { + existing.Count++ + existing.LastUpdated = now + existing.Info = cloneModelInfo(model) + if existing.SuspendedClients == nil { + existing.SuspendedClients = make(map[string]string) + } + if provider != "" { + if existing.Providers == nil { + existing.Providers = make(map[string]int) + } + existing.Providers[provider]++ + } + log.Debugf("Incremented count for model %s, now %d clients", modelID, existing.Count) + return + } + + registration := &ModelRegistration{ + Info: cloneModelInfo(model), + Count: 1, + LastUpdated: now, + QuotaExceededClients: make(map[string]*time.Time), + SuspendedClients: make(map[string]string), + } + if provider != "" { + registration.Providers = map[string]int{provider: 1} + } + r.models[modelID] = registration + log.Debugf("Registered new model %s from provider %s", modelID, provider) +} + +func (r *ModelRegistry) removeModelRegistration(clientID, modelID, provider string, now time.Time) { + registration, exists := r.models[modelID] + if !exists { + return + } + registration.Count-- + registration.LastUpdated = now + if registration.QuotaExceededClients != nil { + delete(registration.QuotaExceededClients, clientID) + } + if registration.SuspendedClients != nil { + delete(registration.SuspendedClients, clientID) + } + if registration.Count < 0 { + registration.Count = 0 + } + if provider != "" && registration.Providers != nil { + if count, ok := registration.Providers[provider]; ok { + if count <= 1 { + delete(registration.Providers, provider) + } else { + registration.Providers[provider] = count - 1 + } + } + } + log.Debugf("Decremented count for model %s, now %d clients", modelID, registration.Count) + if registration.Count <= 0 { + delete(r.models, modelID) + log.Debugf("Removed model %s as no clients remain", modelID) + } +} + +func cloneModelInfo(model *ModelInfo) *ModelInfo { + if model == nil { + return nil + } + copy := *model + if len(model.SupportedGenerationMethods) > 0 { + copy.SupportedGenerationMethods = append([]string(nil), model.SupportedGenerationMethods...) + } + if len(model.SupportedParameters) > 0 { + copy.SupportedParameters = append([]string(nil), model.SupportedParameters...) + } + return © +} + // UnregisterClient removes a client and decrements counts for its models // Parameters: // - clientID: Unique identifier for the client to remove diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index b6a8b0ae..cf8bd2bf 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -52,6 +52,39 @@ type Watcher struct { dispatchCancel context.CancelFunc } +type stableIDGenerator struct { + counters map[string]int +} + +func newStableIDGenerator() *stableIDGenerator { + return &stableIDGenerator{counters: make(map[string]int)} +} + +func (g *stableIDGenerator) next(kind string, parts ...string) (string, string) { + if g == nil { + return kind + ":000000000000", "000000000000" + } + hasher := sha256.New() + hasher.Write([]byte(kind)) + for _, part := range parts { + trimmed := strings.TrimSpace(part) + hasher.Write([]byte{0}) + hasher.Write([]byte(trimmed)) + } + digest := hex.EncodeToString(hasher.Sum(nil)) + if len(digest) < 12 { + digest = fmt.Sprintf("%012s", digest) + } + short := digest[:12] + key := kind + ":" + short + index := g.counters[key] + g.counters[key] = index + 1 + if index > 0 { + short = fmt.Sprintf("%s-%d", short, index) + } + return fmt.Sprintf("%s:%s", kind, short), short +} + // AuthUpdateAction represents the type of change detected in auth sources. type AuthUpdateAction string @@ -640,6 +673,7 @@ func (w *Watcher) removeClient(path string) { func (w *Watcher) SnapshotCoreAuths() []*coreauth.Auth { out := make([]*coreauth.Auth, 0, 32) now := time.Now() + idGen := newStableIDGenerator() // Also synthesize auth entries for OpenAI-compatibility providers directly from config w.clientsMutex.RLock() cfg := w.config @@ -647,14 +681,18 @@ func (w *Watcher) SnapshotCoreAuths() []*coreauth.Auth { if cfg != nil { // Gemini official API keys -> synthesize auths for i := range cfg.GlAPIKey { - k := cfg.GlAPIKey[i] + k := strings.TrimSpace(cfg.GlAPIKey[i]) + if k == "" { + continue + } + id, token := idGen.next("gemini:apikey", k) a := &coreauth.Auth{ - ID: fmt.Sprintf("gemini:apikey:%d", i), + ID: id, Provider: "gemini", Label: "gemini-apikey", Status: coreauth.StatusActive, Attributes: map[string]string{ - "source": fmt.Sprintf("config:gemini#%d", i), + "source": fmt.Sprintf("config:gemini[%s]", token), "api_key": k, }, CreatedAt: now, @@ -665,15 +703,20 @@ func (w *Watcher) SnapshotCoreAuths() []*coreauth.Auth { // Claude API keys -> synthesize auths for i := range cfg.ClaudeKey { ck := cfg.ClaudeKey[i] + key := strings.TrimSpace(ck.APIKey) + if key == "" { + continue + } + id, token := idGen.next("claude:apikey", key, ck.BaseURL) attrs := map[string]string{ - "source": fmt.Sprintf("config:claude#%d", i), - "api_key": ck.APIKey, + "source": fmt.Sprintf("config:claude[%s]", token), + "api_key": key, } if ck.BaseURL != "" { attrs["base_url"] = ck.BaseURL } a := &coreauth.Auth{ - ID: fmt.Sprintf("claude:apikey:%d", i), + ID: id, Provider: "claude", Label: "claude-apikey", Status: coreauth.StatusActive, @@ -686,15 +729,20 @@ func (w *Watcher) SnapshotCoreAuths() []*coreauth.Auth { // Codex API keys -> synthesize auths for i := range cfg.CodexKey { ck := cfg.CodexKey[i] + key := strings.TrimSpace(ck.APIKey) + if key == "" { + continue + } + id, token := idGen.next("codex:apikey", key, ck.BaseURL) attrs := map[string]string{ - "source": fmt.Sprintf("config:codex#%d", i), - "api_key": ck.APIKey, + "source": fmt.Sprintf("config:codex[%s]", token), + "api_key": key, } if ck.BaseURL != "" { attrs["base_url"] = ck.BaseURL } a := &coreauth.Auth{ - ID: fmt.Sprintf("codex:apikey:%d", i), + ID: id, Provider: "codex", Label: "codex-apikey", Status: coreauth.StatusActive, @@ -710,11 +758,16 @@ func (w *Watcher) SnapshotCoreAuths() []*coreauth.Auth { if providerName == "" { providerName = "openai-compatibility" } - base := compat.BaseURL + base := strings.TrimSpace(compat.BaseURL) for j := range compat.APIKeys { - key := compat.APIKeys[j] + key := strings.TrimSpace(compat.APIKeys[j]) + if key == "" { + continue + } + idKind := fmt.Sprintf("openai-compatibility:%s", providerName) + id, token := idGen.next(idKind, key, base) attrs := map[string]string{ - "source": fmt.Sprintf("config:%s#%d", compat.Name, j), + "source": fmt.Sprintf("config:%s[%s]", providerName, token), "base_url": base, "api_key": key, "compat_name": compat.Name, @@ -724,7 +777,7 @@ func (w *Watcher) SnapshotCoreAuths() []*coreauth.Auth { attrs["models_hash"] = hash } a := &coreauth.Auth{ - ID: fmt.Sprintf("openai-compatibility:%s:%d", compat.Name, j), + ID: id, Provider: providerName, Label: compat.Name, Status: coreauth.StatusActive, diff --git a/sdk/access/reconcile.go b/sdk/access/reconcile.go new file mode 100644 index 00000000..32e4072a --- /dev/null +++ b/sdk/access/reconcile.go @@ -0,0 +1,230 @@ +package access + +import ( + "reflect" + "sort" + "strings" + + "github.com/router-for-me/CLIProxyAPI/v6/internal/config" +) + +// ReconcileProviders builds the desired provider list by reusing existing providers when possible +// and creating or removing providers only when their configuration changed. It returns the final +// ordered provider slice along with the identifiers of providers that were added, updated, or +// removed compared to the previous configuration. +func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (result []Provider, added, updated, removed []string, err error) { + if newCfg == nil { + return nil, nil, nil, nil, nil + } + + existingMap := make(map[string]Provider, len(existing)) + for _, provider := range existing { + if provider == nil { + continue + } + existingMap[provider.Identifier()] = provider + } + + oldCfgMap := accessProviderMap(oldCfg) + newEntries := collectProviderEntries(newCfg) + + result = make([]Provider, 0, len(newEntries)) + finalIDs := make(map[string]struct{}, len(newEntries)) + + for _, providerCfg := range newEntries { + key := providerIdentifier(providerCfg) + if key == "" { + continue + } + + if oldCfgProvider, ok := oldCfgMap[key]; ok && providerConfigEqual(oldCfgProvider, providerCfg) { + if existingProvider, ok := existingMap[key]; ok { + result = append(result, existingProvider) + finalIDs[key] = struct{}{} + continue + } + } + + provider, buildErr := buildProvider(providerCfg, newCfg) + if buildErr != nil { + return nil, nil, nil, nil, buildErr + } + if _, ok := oldCfgMap[key]; ok { + if _, existed := existingMap[key]; existed { + updated = append(updated, key) + } else { + added = append(added, key) + } + } else { + added = append(added, key) + } + result = append(result, provider) + finalIDs[key] = struct{}{} + } + + if len(result) == 0 && len(newCfg.APIKeys) > 0 { + config.SyncInlineAPIKeys(newCfg, newCfg.APIKeys) + if providerCfg := newCfg.ConfigAPIKeyProvider(); providerCfg != nil { + key := providerIdentifier(providerCfg) + if key != "" { + if oldCfgProvider, ok := oldCfgMap[key]; ok && providerConfigEqual(oldCfgProvider, providerCfg) { + if existingProvider, ok := existingMap[key]; ok { + result = append(result, existingProvider) + } else { + provider, buildErr := buildProvider(providerCfg, newCfg) + if buildErr != nil { + return nil, nil, nil, nil, buildErr + } + if _, existed := existingMap[key]; existed { + updated = append(updated, key) + } else { + added = append(added, key) + } + result = append(result, provider) + } + } else { + provider, buildErr := buildProvider(providerCfg, newCfg) + if buildErr != nil { + return nil, nil, nil, nil, buildErr + } + if _, ok := oldCfgMap[key]; ok { + if _, existed := existingMap[key]; existed { + updated = append(updated, key) + } else { + added = append(added, key) + } + } else { + added = append(added, key) + } + result = append(result, provider) + } + finalIDs[key] = struct{}{} + } + } + } + + removedSet := make(map[string]struct{}) + for id := range existingMap { + if _, ok := finalIDs[id]; !ok { + removedSet[id] = struct{}{} + } + } + + removed = make([]string, 0, len(removedSet)) + for id := range removedSet { + removed = append(removed, id) + } + + sort.Strings(added) + sort.Strings(updated) + sort.Strings(removed) + + return result, added, updated, removed, nil +} + +func accessProviderMap(cfg *config.Config) map[string]*config.AccessProvider { + result := make(map[string]*config.AccessProvider) + if cfg == nil { + return result + } + for i := range cfg.Access.Providers { + providerCfg := &cfg.Access.Providers[i] + if providerCfg.Type == "" { + continue + } + key := providerIdentifier(providerCfg) + if key == "" { + continue + } + result[key] = providerCfg + } + if len(result) == 0 && len(cfg.APIKeys) > 0 { + if provider := cfg.ConfigAPIKeyProvider(); provider != nil { + if key := providerIdentifier(provider); key != "" { + result[key] = provider + } + } + } + return result +} + +func collectProviderEntries(cfg *config.Config) []*config.AccessProvider { + entries := make([]*config.AccessProvider, 0, len(cfg.Access.Providers)) + if cfg == nil { + return entries + } + for i := range cfg.Access.Providers { + providerCfg := &cfg.Access.Providers[i] + if providerCfg.Type == "" { + continue + } + if key := providerIdentifier(providerCfg); key != "" { + entries = append(entries, providerCfg) + } + } + return entries +} + +func providerIdentifier(provider *config.AccessProvider) string { + if provider == nil { + return "" + } + if name := strings.TrimSpace(provider.Name); name != "" { + return name + } + typ := strings.TrimSpace(provider.Type) + if typ == "" { + return "" + } + if strings.EqualFold(typ, config.AccessProviderTypeConfigAPIKey) { + return config.DefaultAccessProviderName + } + return typ +} + +func providerConfigEqual(a, b *config.AccessProvider) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + if !strings.EqualFold(strings.TrimSpace(a.Type), strings.TrimSpace(b.Type)) { + return false + } + if strings.TrimSpace(a.SDK) != strings.TrimSpace(b.SDK) { + return false + } + if !stringSetEqual(a.APIKeys, b.APIKeys) { + return false + } + if len(a.Config) != len(b.Config) { + return false + } + if len(a.Config) > 0 && !reflect.DeepEqual(a.Config, b.Config) { + return false + } + return true +} + +func stringSetEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + if len(a) == 0 { + return true + } + seen := make(map[string]int, len(a)) + for _, val := range a { + seen[val]++ + } + for _, val := range b { + count := seen[val] + if count == 0 { + return false + } + if count == 1 { + delete(seen, val) + } else { + seen[val] = count - 1 + } + } + return len(seen) == 0 +} diff --git a/sdk/cliproxy/service.go b/sdk/cliproxy/service.go index dba2cfda..7c96abe7 100644 --- a/sdk/cliproxy/service.go +++ b/sdk/cliproxy/service.go @@ -110,12 +110,23 @@ func (s *Service) refreshAccessProviders(cfg *config.Config) { if s == nil || s.accessManager == nil || cfg == nil { return } - providers, err := sdkaccess.BuildProviders(cfg) + s.cfgMu.RLock() + oldCfg := s.cfg + s.cfgMu.RUnlock() + + existing := s.accessManager.Providers() + providers, added, updated, removed, err := sdkaccess.ReconcileProviders(oldCfg, cfg, existing) if err != nil { - log.Errorf("failed to rebuild request auth providers: %v", err) + log.Errorf("failed to reconcile request auth providers: %v", err) return } s.accessManager.SetProviders(providers) + if len(added)+len(updated)+len(removed) > 0 { + log.Infof("request auth providers reconciled (added=%d updated=%d removed=%d)", len(added), len(updated), len(removed)) + log.Debugf("provider changes details - added=%v updated=%v removed=%v", added, updated, removed) + } else { + log.Debug("request auth providers unchanged after config reload") + } } func (s *Service) ensureAuthUpdateQueue(ctx context.Context) { From 63af4c551d19e088bc7a05ea8f8b0f3e93ebe41e Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 17:17:43 +0800 Subject: [PATCH 02/10] fix(registry): Fix provider change logic for new models When a client changed its provider and registered a new model in the same `RegisterClient` call, the logic would incorrectly attempt to decrement the provider count for the new model from the old provider. This was because the loop iterated over all new model IDs without checking if they were part of the client's previous registration. This commit adds a check to ensure that a model existed in the client's old model set before attempting to decrement the old provider's usage count. This prevents incorrect state updates in the registry during provider transitions that also introduce new models. --- internal/registry/model_registry.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index 0a941916..a2de4831 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -169,6 +169,9 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ // Handle provider change for overlapping models before modifications. if providerChanged && oldProvider != "" { for _, id := range modelIDs { + if _, existed := oldSet[id]; !existed { + continue + } if reg, ok := r.models[id]; ok && reg.Providers != nil { if count, okProv := reg.Providers[oldProvider]; okProv { if count <= 1 { From 2717ba3e508170d17cd724774f977c40a22d5332 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 17:48:12 +0800 Subject: [PATCH 03/10] fix(registry): Avoid provider update when new provider is empty When a client re-registered and changed its provider from a non-empty value to an empty string, the logic would still trigger a provider update for the client's models. An empty provider string should not cause an update. This commit fixes this behavior by adding a check to ensure the new provider is a non-empty string before updating the model's provider information. Additionally, the logic for detecting a provider change has been simplified by removing an unnecessary variable. --- internal/registry/model_registry.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index a2de4831..cc679941 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -129,8 +129,8 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ now := time.Now() oldModels, hadExisting := r.clientModels[clientID] - oldProvider, hadProvider := r.clientProviders[clientID] - providerChanged := hadProvider && oldProvider != provider + oldProvider, _ := r.clientProviders[clientID] + providerChanged := oldProvider != provider if !hadExisting { // Pure addition path. for _, modelID := range modelIDs { @@ -205,7 +205,7 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ if reg, ok := r.models[id]; ok { reg.Info = cloneModelInfo(model) reg.LastUpdated = now - if providerChanged { + if providerChanged && provider != "" { if _, newlyAdded := addedSet[id]; newlyAdded { continue } From a887a337a59aca1e891595d36dc66052eab8c892 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 18:52:58 +0800 Subject: [PATCH 04/10] fix(registry): Handle duplicate model IDs in client registration The previous model registration logic used a set-like map to track the models associated with a client. This caused issues when a client registered multiple instances of the same model ID, as they were all treated as a single registration. This commit refactors the registration logic to use count maps for both the old and new model lists. This allows the system to accurately track the number of instances for each model ID provided by a client. The changes ensure that: - When a client updates its model list, the exact number of added or removed instances for each model ID is correctly calculated. - Provider counts are accurately incremented or decremented based on the number of model instances being added, removed, or having their provider changed. - The registry correctly handles scenarios where a client reduces the number of duplicate model registrations (e.g., from `[A, A]` to `[A]`), properly deregistering the surplus instance. --- internal/registry/model_registry.go | 67 +++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index cc679941..dba10d9d 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -105,10 +105,12 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ seen := make(map[string]struct{}) modelIDs := make([]string, 0, len(models)) newModels := make(map[string]*ModelInfo, len(models)) + newCounts := make(map[string]int, len(models)) for _, model := range models { if model == nil || model.ID == "" { continue } + newCounts[model.ID]++ if _, exists := seen[model.ID]; exists { continue } @@ -148,36 +150,45 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ return } - oldSet := make(map[string]struct{}, len(oldModels)) + oldCounts := make(map[string]int, len(oldModels)) for _, id := range oldModels { - oldSet[id] = struct{}{} + oldCounts[id]++ } added := make([]string, 0) - removed := make([]string, 0) for _, id := range modelIDs { - if _, exists := oldSet[id]; !exists { + if oldCounts[id] == 0 { added = append(added, id) } } - for _, id := range oldModels { - if _, exists := newModels[id]; !exists { + + removed := make([]string, 0) + for id := range oldCounts { + if newCounts[id] == 0 { removed = append(removed, id) } } // Handle provider change for overlapping models before modifications. if providerChanged && oldProvider != "" { - for _, id := range modelIDs { - if _, existed := oldSet[id]; !existed { + for id, newCount := range newCounts { + if newCount == 0 { continue } + oldCount := oldCounts[id] + if oldCount == 0 { + continue + } + toRemove := newCount + if oldCount < toRemove { + toRemove = oldCount + } if reg, ok := r.models[id]; ok && reg.Providers != nil { if count, okProv := reg.Providers[oldProvider]; okProv { - if count <= 1 { + if count <= toRemove { delete(reg.Providers, oldProvider) } else { - reg.Providers[oldProvider] = count - 1 + reg.Providers[oldProvider] = count - toRemove } } } @@ -186,13 +197,34 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ // Apply removals first to keep counters accurate. for _, id := range removed { - r.removeModelRegistration(clientID, id, oldProvider, now) + oldCount := oldCounts[id] + for i := 0; i < oldCount; i++ { + r.removeModelRegistration(clientID, id, oldProvider, now) + } + } + + for id, oldCount := range oldCounts { + newCount := newCounts[id] + if newCount == 0 || oldCount <= newCount { + continue + } + overage := oldCount - newCount + for i := 0; i < overage; i++ { + r.removeModelRegistration(clientID, id, oldProvider, now) + } } // Apply additions. - for _, id := range added { + for id, newCount := range newCounts { + oldCount := oldCounts[id] + if newCount <= oldCount { + continue + } model := newModels[id] - r.addModelRegistration(id, provider, model, now) + diff := newCount - oldCount + for i := 0; i < diff; i++ { + r.addModelRegistration(id, provider, model, now) + } } // Update metadata for models that remain associated with the client. @@ -209,10 +241,17 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ if _, newlyAdded := addedSet[id]; newlyAdded { continue } + overlapCount := newCounts[id] + if oldCount := oldCounts[id]; oldCount < overlapCount { + overlapCount = oldCount + } + if overlapCount <= 0 { + continue + } if reg.Providers == nil { reg.Providers = make(map[string]int) } - reg.Providers[provider]++ + reg.Providers[provider] += overlapCount } } } From e1de04230f53b23d44cd6563731990f9dcb6fd2c Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 19:19:24 +0800 Subject: [PATCH 05/10] fix(registry): Reset client status on model re-registration When a client re-registers with the model registry, its previous status for a given model (e.g., quota exceeded or suspended) was not being cleared. This could lead to a situation where a client is permanently unable to use a model even after re-registering. This change ensures that when a client re-registers an existing model, its ID is removed from the model's `QuotaExceededClients` and `SuspendedClients` lists. This effectively resets the client's status for that model, allowing for a fresh start upon reconnection. --- internal/registry/model_registry.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index dba10d9d..b52e48b2 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -237,6 +237,12 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ if reg, ok := r.models[id]; ok { reg.Info = cloneModelInfo(model) reg.LastUpdated = now + if reg.QuotaExceededClients != nil { + delete(reg.QuotaExceededClients, clientID) + } + if reg.SuspendedClients != nil { + delete(reg.SuspendedClients, clientID) + } if providerChanged && provider != "" { if _, newlyAdded := addedSet[id]; newlyAdded { continue From 0f55e550cf18988f45f6a997701e2a476980598a Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 19:38:44 +0800 Subject: [PATCH 06/10] refactor(registry): Preserve duplicate models in client registration The `RegisterClient` function previously deduplicated the list of models provided by a client. This could lead to an inaccurate representation of the client's state if it intentionally registered the same model ID multiple times. This change refactors the registration logic to store the raw, unfiltered list of models, preserving their original order and count. A new `rawModelIDs` slice tracks the complete list for storage in `clientModels`, while the logic for processing changes continues to use a unique set of model IDs for efficiency. This ensures the registry's state accurately reflects what the client provides. --- internal/registry/model_registry.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index b52e48b2..44b6991c 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -102,24 +102,24 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ defer r.mutex.Unlock() provider := strings.ToLower(clientProvider) - seen := make(map[string]struct{}) - modelIDs := make([]string, 0, len(models)) + uniqueModelIDs := make([]string, 0, len(models)) + rawModelIDs := make([]string, 0, len(models)) newModels := make(map[string]*ModelInfo, len(models)) newCounts := make(map[string]int, len(models)) for _, model := range models { if model == nil || model.ID == "" { continue } + rawModelIDs = append(rawModelIDs, model.ID) newCounts[model.ID]++ - if _, exists := seen[model.ID]; exists { + if _, exists := newModels[model.ID]; exists { continue } - seen[model.ID] = struct{}{} - modelIDs = append(modelIDs, model.ID) newModels[model.ID] = model + uniqueModelIDs = append(uniqueModelIDs, model.ID) } - if len(modelIDs) == 0 { + if len(uniqueModelIDs) == 0 { // No models supplied; unregister existing client state if present. r.unregisterClientInternal(clientID) delete(r.clientModels, clientID) @@ -135,17 +135,17 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ providerChanged := oldProvider != provider if !hadExisting { // Pure addition path. - for _, modelID := range modelIDs { + for _, modelID := range rawModelIDs { model := newModels[modelID] r.addModelRegistration(modelID, provider, model, now) } - r.clientModels[clientID] = modelIDs + r.clientModels[clientID] = append([]string(nil), rawModelIDs...) if provider != "" { r.clientProviders[clientID] = provider } else { delete(r.clientProviders, clientID) } - log.Debugf("Registered client %s from provider %s with %d models", clientID, clientProvider, len(modelIDs)) + log.Debugf("Registered client %s from provider %s with %d models", clientID, clientProvider, len(rawModelIDs)) misc.LogCredentialSeparator() return } @@ -156,7 +156,7 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ } added := make([]string, 0) - for _, id := range modelIDs { + for _, id := range uniqueModelIDs { if oldCounts[id] == 0 { added = append(added, id) } @@ -232,7 +232,7 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ for _, id := range added { addedSet[id] = struct{}{} } - for _, id := range modelIDs { + for _, id := range uniqueModelIDs { model := newModels[id] if reg, ok := r.models[id]; ok { reg.Info = cloneModelInfo(model) @@ -263,8 +263,8 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ } // Update client bookkeeping. - if len(modelIDs) > 0 { - r.clientModels[clientID] = modelIDs + if len(rawModelIDs) > 0 { + r.clientModels[clientID] = append([]string(nil), rawModelIDs...) } if provider != "" { r.clientProviders[clientID] = provider From b6d5ce2d4dcb253696e9a95aa3f32f05cba82755 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 20:05:43 +0800 Subject: [PATCH 07/10] fix(access): Force rebuild of aliased provider configurations The provider reconciliation logic did not correctly handle aliased provider configurations (e.g., using YAML anchors). When a provider config was aliased, the check for configuration equality would pass, causing the system to reuse the existing provider instance without rebuilding it, even if the underlying configuration had changed. This change introduces a check to detect if the old and new provider configurations point to the same object in memory. If they are aliased, the provider is now always rebuilt to ensure it reflects the latest configuration. The optimization to reuse an existing provider based on deep equality is now only applied to non-aliased providers. --- sdk/access/reconcile.go | 43 +++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/sdk/access/reconcile.go b/sdk/access/reconcile.go index 32e4072a..d4eda6c8 100644 --- a/sdk/access/reconcile.go +++ b/sdk/access/reconcile.go @@ -37,11 +37,14 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res continue } - if oldCfgProvider, ok := oldCfgMap[key]; ok && providerConfigEqual(oldCfgProvider, providerCfg) { - if existingProvider, ok := existingMap[key]; ok { - result = append(result, existingProvider) - finalIDs[key] = struct{}{} - continue + if oldCfgProvider, ok := oldCfgMap[key]; ok { + isAliased := oldCfgProvider == providerCfg + if !isAliased && providerConfigEqual(oldCfgProvider, providerCfg) { + if existingProvider, okExisting := existingMap[key]; okExisting { + result = append(result, existingProvider) + finalIDs[key] = struct{}{} + continue + } } } @@ -67,9 +70,23 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res if providerCfg := newCfg.ConfigAPIKeyProvider(); providerCfg != nil { key := providerIdentifier(providerCfg) if key != "" { - if oldCfgProvider, ok := oldCfgMap[key]; ok && providerConfigEqual(oldCfgProvider, providerCfg) { - if existingProvider, ok := existingMap[key]; ok { - result = append(result, existingProvider) + if oldCfgProvider, ok := oldCfgMap[key]; ok { + isAliased := oldCfgProvider == providerCfg + if !isAliased && providerConfigEqual(oldCfgProvider, providerCfg) { + if existingProvider, okExisting := existingMap[key]; okExisting { + result = append(result, existingProvider) + } else { + provider, buildErr := buildProvider(providerCfg, newCfg) + if buildErr != nil { + return nil, nil, nil, nil, buildErr + } + if _, existed := existingMap[key]; existed { + updated = append(updated, key) + } else { + added = append(added, key) + } + result = append(result, provider) + } } else { provider, buildErr := buildProvider(providerCfg, newCfg) if buildErr != nil { @@ -87,15 +104,7 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res if buildErr != nil { return nil, nil, nil, nil, buildErr } - if _, ok := oldCfgMap[key]; ok { - if _, existed := existingMap[key]; existed { - updated = append(updated, key) - } else { - added = append(added, key) - } - } else { - added = append(added, key) - } + added = append(added, key) result = append(result, provider) } finalIDs[key] = struct{}{} From 08856a97fb27026f3b6fcf1253a01d070543a17c Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 20:48:20 +0800 Subject: [PATCH 08/10] fix(access): Exclude inline provider from reconciliation changes The `ReconcileProviders` function was incorrectly including the default inline provider (`access.teleport.dev`) in the lists of added, updated, and removed providers. The inline provider is a special case managed directly by the access controller and does not correspond to a separate, reloadable resource. Including it in the change lists could lead to errors when attempting to perform lifecycle operations on it. This commit modifies the reconciliation logic to explicitly ignore the inline provider when calculating changes. This ensures that only external, reloadable providers are reported as changed, preventing incorrect lifecycle management. --- sdk/access/reconcile.go | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/sdk/access/reconcile.go b/sdk/access/reconcile.go index d4eda6c8..ca4d7679 100644 --- a/sdk/access/reconcile.go +++ b/sdk/access/reconcile.go @@ -31,6 +31,16 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res result = make([]Provider, 0, len(newEntries)) finalIDs := make(map[string]struct{}, len(newEntries)) + isInlineProvider := func(id string) bool { + return strings.EqualFold(id, config.DefaultAccessProviderName) + } + appendChange := func(list *[]string, id string) { + if isInlineProvider(id) { + return + } + *list = append(*list, id) + } + for _, providerCfg := range newEntries { key := providerIdentifier(providerCfg) if key == "" { @@ -54,12 +64,12 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res } if _, ok := oldCfgMap[key]; ok { if _, existed := existingMap[key]; existed { - updated = append(updated, key) + appendChange(&updated, key) } else { - added = append(added, key) + appendChange(&added, key) } } else { - added = append(added, key) + appendChange(&added, key) } result = append(result, provider) finalIDs[key] = struct{}{} @@ -81,9 +91,9 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res return nil, nil, nil, nil, buildErr } if _, existed := existingMap[key]; existed { - updated = append(updated, key) + appendChange(&updated, key) } else { - added = append(added, key) + appendChange(&added, key) } result = append(result, provider) } @@ -93,9 +103,9 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res return nil, nil, nil, nil, buildErr } if _, existed := existingMap[key]; existed { - updated = append(updated, key) + appendChange(&updated, key) } else { - added = append(added, key) + appendChange(&added, key) } result = append(result, provider) } @@ -104,7 +114,7 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res if buildErr != nil { return nil, nil, nil, nil, buildErr } - added = append(added, key) + appendChange(&added, key) result = append(result, provider) } finalIDs[key] = struct{}{} @@ -115,6 +125,9 @@ func ReconcileProviders(oldCfg, newCfg *config.Config, existing []Provider) (res removedSet := make(map[string]struct{}) for id := range existingMap { if _, ok := finalIDs[id]; !ok { + if isInlineProvider(id) { + continue + } removedSet[id] = struct{}{} } } From cd0b1be46c0e32633b12e24736095c2c8b8f52ee Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 21:42:42 +0800 Subject: [PATCH 09/10] fix(log): Reduce noise on metadata updates and provider sync --- internal/api/server.go | 6 +++--- internal/registry/model_registry.go | 3 +-- sdk/cliproxy/service.go | 6 +++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index 2a35b642..28bda337 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -559,10 +559,10 @@ func (s *Server) applyAccessConfig(oldCfg, newCfg *config.Config) { } s.accessManager.SetProviders(providers) if len(added)+len(updated)+len(removed) > 0 { - log.Infof("server request auth providers reconciled (added=%d updated=%d removed=%d)", len(added), len(updated), len(removed)) - log.Debugf("server request auth provider details - added=%v updated=%v removed=%v", added, updated, removed) + log.Debugf("auth providers reconciled (added=%d updated=%d removed=%d)", len(added), len(updated), len(removed)) + log.Debugf("auth provider changes details - added=%v updated=%v removed=%v", added, updated, removed) } else { - log.Debug("server request auth providers unchanged after config update") + log.Debug("auth providers unchanged after config update") } } diff --git a/internal/registry/model_registry.go b/internal/registry/model_registry.go index 44b6991c..29607a5b 100644 --- a/internal/registry/model_registry.go +++ b/internal/registry/model_registry.go @@ -273,8 +273,7 @@ func (r *ModelRegistry) RegisterClient(clientID, clientProvider string, models [ } if len(added) == 0 && len(removed) == 0 && !providerChanged { - // Only metadata (e.g., display name) changed. - misc.LogCredentialSeparator() + // Only metadata (e.g., display name) changed; skip separator when no log output. return } diff --git a/sdk/cliproxy/service.go b/sdk/cliproxy/service.go index 7c96abe7..725bc4e6 100644 --- a/sdk/cliproxy/service.go +++ b/sdk/cliproxy/service.go @@ -122,10 +122,10 @@ func (s *Service) refreshAccessProviders(cfg *config.Config) { } s.accessManager.SetProviders(providers) if len(added)+len(updated)+len(removed) > 0 { - log.Infof("request auth providers reconciled (added=%d updated=%d removed=%d)", len(added), len(updated), len(removed)) - log.Debugf("provider changes details - added=%v updated=%v removed=%v", added, updated, removed) + log.Debugf("auth providers reconciled (added=%d updated=%d removed=%d)", len(added), len(updated), len(removed)) + log.Debugf("auth provider changes details - added=%v updated=%v removed=%v", added, updated, removed) } else { - log.Debug("request auth providers unchanged after config reload") + log.Debug("auth providers unchanged after config reload") } } From b0f2ad7cfe2e568c27c535cd0f72c3249c796858 Mon Sep 17 00:00:00 2001 From: hkfires <10558748+hkfires@users.noreply.github.com> Date: Fri, 26 Sep 2025 22:04:32 +0800 Subject: [PATCH 10/10] fix(cliproxy): Clear stale compatibility model registrations Previously, if an OpenAI compatibility configuration was removed from the config file or its model list was emptied, the associated models for that auth entry were not unregistered from the global model registry. This resulted in stale registrations persisting. This change ensures that when an auth entry is identified as being for a compatibility provider, its models are explicitly unregistered if: - The corresponding configuration is found but has an empty model list. - The corresponding configuration is no longer found in the config file. --- sdk/cliproxy/service.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/sdk/cliproxy/service.go b/sdk/cliproxy/service.go index 725bc4e6..904eff4c 100644 --- a/sdk/cliproxy/service.go +++ b/sdk/cliproxy/service.go @@ -508,22 +508,35 @@ func (s *Service) registerModelsForAuth(a *coreauth.Auth) { if s.cfg != nil { providerKey := provider compatName := strings.TrimSpace(a.Provider) + isCompatAuth := false if strings.EqualFold(providerKey, "openai-compatibility") { + isCompatAuth = true if a.Attributes != nil { if v := strings.TrimSpace(a.Attributes["compat_name"]); v != "" { compatName = v } if v := strings.TrimSpace(a.Attributes["provider_key"]); v != "" { providerKey = strings.ToLower(v) + isCompatAuth = true } } if providerKey == "openai-compatibility" && compatName != "" { providerKey = strings.ToLower(compatName) } + } else if a.Attributes != nil { + if v := strings.TrimSpace(a.Attributes["compat_name"]); v != "" { + compatName = v + isCompatAuth = true + } + if v := strings.TrimSpace(a.Attributes["provider_key"]); v != "" { + providerKey = strings.ToLower(v) + isCompatAuth = true + } } for i := range s.cfg.OpenAICompatibility { compat := &s.cfg.OpenAICompatibility[i] if strings.EqualFold(compat.Name, compatName) { + isCompatAuth = true // Convert compatibility models to registry models ms := make([]*ModelInfo, 0, len(compat.Models)) for j := range compat.Models { @@ -543,10 +556,18 @@ func (s *Service) registerModelsForAuth(a *coreauth.Auth) { providerKey = "openai-compatibility" } GlobalModelRegistry().RegisterClient(a.ID, providerKey, ms) + } else { + // Ensure stale registrations are cleared when model list becomes empty. + GlobalModelRegistry().UnregisterClient(a.ID) } return } } + if isCompatAuth { + // No matching provider found or models removed entirely; drop any prior registration. + GlobalModelRegistry().UnregisterClient(a.ID) + return + } } } if len(models) > 0 {