Fix critical P0 infrastructure concurrency issues

This commit addresses 3 critical P0 race conditions and resource leaks in core infrastructure:

**P0-1: Discovery Service Goroutine Leak** (service.go:468, 488)
- **Problem**: ForceRefresh() and SetSubnet() spawned unbounded goroutines without checking if scan already in progress
- **Impact**: Rapid API calls create goroutine explosion, resource exhaustion
- **Fix**:
  - ForceRefresh: Check isScanning before spawning goroutine (lines 470-476)
  - SetSubnet: Check isScanning, defer scan if already running (lines 491-504)
  - Both now log when skipping to aid debugging

**P0-2: Config Persistence Unlock/Relock Race** (persistence.go:1177-1206)
- **Problem**: LoadNodesConfig() unlocked RLock, called SaveNodesConfig (acquires Lock), then relocked
- **Impact**: Another goroutine could modify config between unlock/relock, causing migrated data loss
- **Fix**:
  - Copy instance slices while holding RLock to ensure consistency (lines 1189-1194)
  - Release lock, save copies, then return without relocking (lines 1196-1205)
  - Prevents TOCTOU vulnerability where migrations could be overwritten

**P0-3: Config Watcher Channel Close Race** (watcher.go:19-178)
- **Problem**: Stop() used select-check-close pattern vulnerable to concurrent calls
- **Impact**: Multiple Stop() calls panic on double-close
- **Fix**:
  - Added sync.Once field stopOnce to ConfigWatcher struct (line 26)
  - Changed Stop() to use stopOnce.Do() ensuring single execution (lines 175-178)
  - Removed racy select-based guard

All fixes maintain backwards compatibility and add defensive logging for operational visibility.

internal/config/persistence.go

@@ -1174,23 +1174,37 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) {
 		}
 	}
 
-	// If any migrations were applied, save the updated configuration
-	if migrationApplied {
-		log.Info().Msg("Migrations applied, saving updated configuration")
-		// Need to unlock before saving to avoid deadlock
-		c.mu.RUnlock()
-		if err := c.SaveNodesConfig(config.PVEInstances, config.PBSInstances, config.PMGInstances); err != nil {
-			log.Error().Err(err).Msg("Failed to save configuration after migration")
-		}
-		c.mu.RLock()
-	}
-
 	log.Info().Str("file", c.nodesFile).
 		Int("pve", len(config.PVEInstances)).
 		Int("pbs", len(config.PBSInstances)).
 		Int("pmg", len(config.PMGInstances)).
 		Bool("encrypted", c.crypto != nil).
 		Msg("Nodes configuration loaded")
+
+	// If any migrations were applied, save the updated configuration after releasing the read lock
+	// This prevents unlock/relock race condition where another goroutine could modify config
+	// between unlock and relock, causing migrated data to be lost
+	if migrationApplied {
+		// Make copies while still holding read lock to ensure consistency
+		pveCopy := make([]PVEInstance, len(config.PVEInstances))
+		copy(pveCopy, config.PVEInstances)
+		pbsCopy := make([]PBSInstance, len(config.PBSInstances))
+		copy(pbsCopy, config.PBSInstances)
+		pmgCopy := make([]PMGInstance, len(config.PMGInstances))
+		copy(pmgCopy, config.PMGInstances)
+
+		// Release read lock before saving (SaveNodesConfig acquires write lock)
+		c.mu.RUnlock()
+
+		log.Info().Msg("Migrations applied, saving updated configuration")
+		if err := c.SaveNodesConfig(pveCopy, pbsCopy, pmgCopy); err != nil {
+			log.Error().Err(err).Msg("Failed to save configuration after migration")
+		}
+
+		// Don't reacquire lock - we're returning
+		return &config, nil
+	}
+
 	return &config, nil
 }
 

internal/config/watcher.go

@@ -23,6 +23,7 @@ type ConfigWatcher struct {
 	apiTokensPath        string
 	watcher              *fsnotify.Watcher
 	stopChan             chan struct{}
+	stopOnce             sync.Once // Ensures Stop() can only close channel once
 	lastModTime          time.Time
 	mockLastModTime      time.Time
 	apiTokensLastModTime time.Time
@@ -170,14 +171,11 @@ func (cw *ConfigWatcher) Start() error {
 
 // Stop stops the config watcher
 func (cw *ConfigWatcher) Stop() {
-	select {
-	case <-cw.stopChan:
-		// Already stopped
-		return
-	default:
+	// Use sync.Once to prevent double-close panic
+	cw.stopOnce.Do(func() {
 		close(cw.stopChan)
-	}
-	cw.watcher.Close()
+		cw.watcher.Close()
+	})
 }
 
 // ReloadConfig manually triggers a config reload (e.g., from SIGHUP)

internal/discovery/service.go

@@ -466,6 +466,15 @@ func (s *Service) IsScanning() bool {
 
 // ForceRefresh triggers an immediate scan
 func (s *Service) ForceRefresh() {
+	// Check if scan is already in progress to prevent goroutine leak
+	s.mu.RLock()
+	if s.isScanning {
+		s.mu.RUnlock()
+		log.Debug().Msg("Scan already in progress, skipping ForceRefresh")
+		return
+	}
+	s.mu.RUnlock()
+
 	go s.performScan()
 }
 
@@ -480,12 +489,19 @@ func (s *Service) SetInterval(interval time.Duration) {
 // SetSubnet updates the subnet to scan
 func (s *Service) SetSubnet(subnet string) {
 	s.mu.Lock()
-	defer s.mu.Unlock()
 	s.subnet = subnet
+	// Check if scan is already in progress to prevent goroutine leak
+	alreadyScanning := s.isScanning
+	s.mu.Unlock()
+
 	log.Info().Str("subnet", subnet).Msg("Updated discovery subnet")
 
-	// Trigger immediate rescan with new subnet
-	go s.performScan()
+	// Trigger immediate rescan with new subnet if not already scanning
+	if !alreadyScanning {
+		go s.performScan()
+	} else {
+		log.Debug().Msg("Scan already in progress, new subnet will be used in next scan")
+	}
 }
 
 // GetStatus returns the current service status