Preserve disabled backup collection scope

Contract-Neutral: The release-note line records a persistence bug fix; it does not change release promotion or artifact dispatch contracts.

docs/releases/RELEASE_NOTES_v6.4.0-rc.1.md

@@ -46,6 +46,8 @@ build path.
 ## Fixed
 
 - Fixed polling intervals are honored when adaptive scheduling is disabled.
+- Disabled Proxmox backup collection now remains disabled after configuration
+  reloads instead of reverting to the default collection scope.
 - Re-enrollment clears host-removal blocks from every owning store so a valid
   returning agent is not held in a partially removed state.
 - Connection alerts can no longer bypass the configured offline-alert policy.

internal/config/persistence.go

@@ -1477,6 +1477,18 @@ type NodesConfig struct {
 	PMGInstances []PMGInstance `json:"pmgInstances"`
 }
 
+func nodeConfigFieldPresent(instances []map[string]json.RawMessage, index int, field string) bool {
+	if index < 0 || index >= len(instances) {
+		return false
+	}
+	for key := range instances[index] {
+		if strings.EqualFold(key, field) {
+			return true
+		}
+	}
+	return false
+}
+
 // SystemSettings represents system configuration settings
 type SystemSettings struct {
 	PVEPollingInterval           int                  `json:"pvePollingInterval"` // PVE polling interval in seconds
@@ -1827,6 +1839,18 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) {
 	if err := json.Unmarshal(data, &config); err != nil {
 		return nil, fmt.Errorf("parse nodes config: %w", err)
 	}
+	// Boolean collector settings predate an explicit schema version. Keep a
+	// parallel view of the saved object keys so a legacy missing field can
+	// still receive its default without treating an explicit false as missing.
+	// Decoding straight into bool loses that distinction and used to re-enable
+	// collectors every time the configuration was loaded.
+	var persistedFields struct {
+		PVEInstances []map[string]json.RawMessage `json:"pveInstances"`
+		PBSInstances []map[string]json.RawMessage `json:"pbsInstances"`
+	}
+	if err := json.Unmarshal(data, &persistedFields); err != nil {
+		return nil, fmt.Errorf("parse nodes config field presence: %w", err)
+	}
 
 	if config.PVEInstances == nil {
 		config.PVEInstances = []PVEInstance{}
@@ -1875,12 +1899,12 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) {
 			}
 		}
 
-		// Migration: Ensure MonitorBackups is enabled for PVE instances
-		// This fixes issue #1139 where PVE backups weren't showing
-		if !config.PVEInstances[i].MonitorBackups {
+		// Legacy records omitted MonitorBackups before backup collection became
+		// the default. Default only an absent field; false is an operator choice.
+		if !nodeConfigFieldPresent(persistedFields.PVEInstances, i, "MonitorBackups") {
 			log.Info().
 				Str("instance", config.PVEInstances[i].Name).
-				Msg("Enabling MonitorBackups for PVE instance (was disabled)")
+				Msg("Enabling MonitorBackups for legacy PVE instance (field was absent)")
 			config.PVEInstances[i].MonitorBackups = true
 			migrationApplied = true
 		}
@@ -1916,12 +1940,12 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) {
 			migrationApplied = true
 		}
 
-		// Migration: Ensure MonitorBackups is enabled for PBS instances
-		// This fixes issue #411 where PBS backups weren't showing
-		if !config.PBSInstances[i].MonitorBackups {
+		// As above, preserve explicit collection-scope choices while retaining
+		// the default for records written before these fields existed.
+		if !nodeConfigFieldPresent(persistedFields.PBSInstances, i, "MonitorBackups") {
 			log.Info().
 				Str("instance", config.PBSInstances[i].Name).
-				Msg("Enabling MonitorBackups for PBS instance (was disabled)")
+				Msg("Enabling MonitorBackups for legacy PBS instance (field was absent)")
 			config.PBSInstances[i].MonitorBackups = true
 			migrationApplied = true
 		}
@@ -1930,10 +1954,10 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) {
 		// Without this, PBS datastores aren't polled, which means backup data
 		// from the direct PBS connection is never fetched — and PVE-side backup
 		// polling skips PBS storages when a direct connection exists.
-		if !config.PBSInstances[i].MonitorDatastores {
+		if !nodeConfigFieldPresent(persistedFields.PBSInstances, i, "MonitorDatastores") {
 			log.Info().
 				Str("instance", config.PBSInstances[i].Name).
-				Msg("Enabling MonitorDatastores for PBS instance (was disabled)")
+				Msg("Enabling MonitorDatastores for legacy PBS instance (field was absent)")
 			config.PBSInstances[i].MonitorDatastores = true
 			migrationApplied = true
 		}

internal/config/persistence_test.go

@@ -2383,21 +2383,21 @@ func TestLoadSystemSettingsFileNotExist(t *testing.T) {
 	}
 }
 
-func TestLoadNodesConfig_PVEMonitorBackupsMigration(t *testing.T) {
+func TestLoadNodesConfig_PreservesExplicitlyDisabledPVEBackups(t *testing.T) {
 	tempDir := t.TempDir()
 	cp := config.NewConfigPersistence(tempDir)
 	if err := cp.EnsureConfigDir(); err != nil {
 		t.Fatalf("EnsureConfigDir: %v", err)
 	}
 
-	// Save PVE instance with MonitorBackups=false (simulating old config or missing field)
+	// Save through the canonical writer so the false field is explicitly present.
 	pveInstances := []config.PVEInstance{
 		{
 			Name:           "pve-no-backups",
 			Host:           "https://pve.local:8006",
 			User:           "root@pam",
 			Password:       "secret",
-			MonitorBackups: false, // This should be migrated to true
+			MonitorBackups: false,
 		},
 	}
 
@@ -2415,28 +2415,55 @@ func TestLoadNodesConfig_PVEMonitorBackupsMigration(t *testing.T) {
 	}
 
 	pve := loaded.PVEInstances[0]
-	// MonitorBackups should be migrated to true
-	if !pve.MonitorBackups {
-		t.Errorf("expected MonitorBackups to be migrated to true, got false")
+	if pve.MonitorBackups {
+		t.Error("expected explicit MonitorBackups=false to survive reload")
 	}
 }
 
-func TestLoadNodesConfig_PBSMonitorDatastoresMigration(t *testing.T) {
+func TestLoadNodesConfig_DefaultsMissingLegacyCollectionFields(t *testing.T) {
 	tempDir := t.TempDir()
 	cp := config.NewConfigPersistence(tempDir)
 	if err := cp.EnsureConfigDir(); err != nil {
 		t.Fatalf("EnsureConfigDir: %v", err)
 	}
 
-	// Save PBS instance with MonitorDatastores=false (simulating old config)
+	legacy := []byte(`{
+		"pveInstances": [{"Name":"legacy-pve","Host":"https://pve.local:8006"}],
+		"pbsInstances": [{"Name":"legacy-pbs","Host":"https://pbs.local:8007"}],
+		"pmgInstances": []
+	}`)
+	if err := os.WriteFile(filepath.Join(tempDir, "nodes.enc"), legacy, 0600); err != nil {
+		t.Fatalf("write legacy nodes config: %v", err)
+	}
+
+	loaded, err := cp.LoadNodesConfig()
+	if err != nil {
+		t.Fatalf("LoadNodesConfig: %v", err)
+	}
+	if len(loaded.PVEInstances) != 1 || !loaded.PVEInstances[0].MonitorBackups {
+		t.Fatalf("legacy PVE MonitorBackups was not defaulted: %+v", loaded.PVEInstances)
+	}
+	if len(loaded.PBSInstances) != 1 || !loaded.PBSInstances[0].MonitorBackups || !loaded.PBSInstances[0].MonitorDatastores {
+		t.Fatalf("legacy PBS collection fields were not defaulted: %+v", loaded.PBSInstances)
+	}
+}
+
+func TestLoadNodesConfig_PreservesExplicitlyDisabledPBSCollection(t *testing.T) {
+	tempDir := t.TempDir()
+	cp := config.NewConfigPersistence(tempDir)
+	if err := cp.EnsureConfigDir(); err != nil {
+		t.Fatalf("EnsureConfigDir: %v", err)
+	}
+
+	// Both false fields are explicit in the canonical saved representation.
 	pbsInstances := []config.PBSInstance{
 		{
 			Name:              "pbs-no-datastores",
 			Host:              "https://pbs.local:8007",
 			User:              "admin@pbs",
 			Password:          "secret",
-			MonitorBackups:    true,
-			MonitorDatastores: false, // This should be migrated to true
+			MonitorBackups:    false,
+			MonitorDatastores: false,
 		},
 	}
 
@@ -2454,7 +2481,7 @@ func TestLoadNodesConfig_PBSMonitorDatastoresMigration(t *testing.T) {
 	}
 
 	pbs := loaded.PBSInstances[0]
-	if !pbs.MonitorDatastores {
-		t.Errorf("expected MonitorDatastores to be migrated to true, got false")
+	if pbs.MonitorBackups || pbs.MonitorDatastores {
+		t.Fatalf("expected explicit PBS collection flags to remain false: %+v", pbs)
 	}
 }