fix(backend): only touch NNTP pool when provider list changes

internal/backend/config.go

@@ -139,40 +139,40 @@ func (a *App) SaveConfig(configData *config.ConfigData) error {
 	return a.applyConfigChanges(configData)
 }
 
-// poolConfigChanged checks if pool-related configuration has changed
+// poolConfigChanged checks whether the provider list has changed in a way that
+// requires touching the NNTP pool. Only provider-affecting fields are
+// considered — non-provider settings (Posting, Watcher, Queue, etc.) must not
+// trigger pool updates.
+//
+// Note: ConnectionPool.MinConnections and ConnectionPool.HealthCheckInterval
+// are deliberately excluded. They are nntppool.NewClient construction options
+// and cannot be applied to a running client by AddProvider/RemoveProvider, so
+// changing them requires an application restart to take effect.
+//
+// Server reordering with no other changes is treated as no change (providers
+// are matched by their canonical key, not by index).
 func (a *App) poolConfigChanged(newConfig *config.ConfigData) bool {
 	if a.config == nil {
 		return true // No existing config, so pool needs to be created
 	}
 
-	oldConfig := a.config
+	oldByKey := make(map[string]config.ServerConfig, len(a.config.Servers))
+	for _, s := range a.config.Servers {
+		oldByKey[pool.ProviderKey(s)] = s
+	}
 
-	// Compare servers
-	if len(oldConfig.Servers) != len(newConfig.Servers) {
+	if len(newConfig.Servers) != len(oldByKey) {
 		return true
 	}
 
-	for i, newServer := range newConfig.Servers {
-		oldServer := oldConfig.Servers[i]
-		if newServer.Host != oldServer.Host ||
-			newServer.Port != oldServer.Port ||
-			newServer.Username != oldServer.Username ||
-			newServer.Password != oldServer.Password ||
-			newServer.SSL != oldServer.SSL ||
-			newServer.MaxConnections != oldServer.MaxConnections ||
-			newServer.MaxConnectionIdleTimeInSeconds != oldServer.MaxConnectionIdleTimeInSeconds ||
-			newServer.MaxConnectionTTLInSeconds != oldServer.MaxConnectionTTLInSeconds ||
-			newServer.InsecureSSL != oldServer.InsecureSSL ||
-			((newServer.Enabled == nil) != (oldServer.Enabled == nil)) ||
-			(newServer.Enabled != nil && oldServer.Enabled != nil && *newServer.Enabled != *oldServer.Enabled) {
+	for _, newSrv := range newConfig.Servers {
+		oldSrv, ok := oldByKey[pool.ProviderKey(newSrv)]
+		if !ok {
+			return true
+		}
+		if pool.ServerConfigChanged(oldSrv, newSrv) {
 			return true
 		}
-	}
-
-	// Compare connection pool settings
-	if oldConfig.ConnectionPool.MinConnections != newConfig.ConnectionPool.MinConnections ||
-		oldConfig.ConnectionPool.HealthCheckInterval != newConfig.ConnectionPool.HealthCheckInterval {
-		return true
 	}
 
 	return false
@@ -276,18 +276,21 @@ func (a *App) applyConfigChanges(configData *config.ConfigData) error {
 		}
 	}
 
-	// Update or create the connection pool manager
+	// Update or create the connection pool manager. The pool is only touched
+	// when the provider list actually changed (poolCfgChanged), or when no
+	// pool exists yet but servers are now configured.
 	poolManagerCreated := false
-	if a.poolManager != nil && poolCfgChanged {
+	switch {
+	case a.poolManager != nil && poolCfgChanged:
 		if err := a.poolManager.UpdateConfig(a.config); err != nil {
 			slog.Error("Failed to update connection pool manager with new config", "error", err)
 		} else {
 			slog.Info("Connection pool manager updated with new configuration")
 		}
-	} else if a.poolManager != nil {
+	case a.poolManager != nil:
 		slog.Info("Pool configuration unchanged, skipping pool update")
-	} else if a.poolManager == nil && len(a.config.Servers) > 0 {
-		// Create pool manager if it doesn't exist but we now have servers configured
+	case len(a.config.Servers) > 0:
+		// First-time creation: poolManager is nil and we now have servers.
 		slog.Info("Creating connection pool manager for newly configured servers")
 		poolManager, err := pool.New(a.config)
 		if err != nil {

internal/pool/manager.go

@@ -130,17 +130,31 @@ func (m *Manager) UpdateConfig(newCfg *config.ConfigData) error {
 
 	slog.Info("Updating NNTP connection pools with new configuration")
 
-	// If pools don't exist yet, create them from scratch
+	// If pools don't exist yet, build them in place while holding the lock so
+	// concurrent Close calls can't race with us. Close any leftover pools
+	// before overwriting to avoid leaks (defensive — should not normally
+	// happen when m.uploadPool == nil).
 	if m.uploadPool == nil {
-		m.mu.Unlock()
-		// Temporarily unlock since New doesn't need the lock and we reassign below
-		mgr, err := New(newCfg)
-		m.mu.Lock()
+		uploadPool, err := newCfg.GetUploadPool()
 		if err != nil {
-			return fmt.Errorf("failed to create pools: %w", err)
+			return fmt.Errorf("failed to create upload pool: %w", err)
 		}
-		m.uploadPool = mgr.uploadPool
-		m.verifyPool = mgr.verifyPool
+		verifyPool, err := newCfg.GetVerifyPool()
+		if err != nil {
+			slog.Warn("Failed to create dedicated verify pool, will use upload pool", "error", err)
+			verifyPool = uploadPool
+		}
+
+		if m.verifyPool != nil && m.verifyPool != m.uploadPool {
+			_ = m.verifyPool.Close()
+		}
+		// m.uploadPool is nil here by branch condition, but guard for safety.
+		if m.uploadPool != nil {
+			_ = m.uploadPool.Close()
+		}
+
+		m.uploadPool = uploadPool
+		m.verifyPool = verifyPool
 		m.config = newCfg
 		return nil
 	}
@@ -235,6 +249,17 @@ func (m *Manager) diffProviders(pool NNTPClient, oldServers, newServers []config
 	return nil
 }
 
+// ServerConfigChanged returns true if any provider-relevant fields differ between two server configs.
+// This is the canonical predicate for "did this provider's pool-affecting state change".
+func ServerConfigChanged(a, b config.ServerConfig) bool {
+	return serverConfigChanged(a, b)
+}
+
+// ProviderKey returns the canonical provider key for a server config.
+func ProviderKey(s config.ServerConfig) string {
+	return providerKey(s)
+}
+
 // serverConfigChanged returns true if any relevant fields differ between two server configs.
 func serverConfigChanged(a, b config.ServerConfig) bool {
 	if a.Host != b.Host || a.Port != b.Port {