Judge filesystem alerts by their own threshold on config save (#2700)

Stacked on #2696, which is stacked on #2692. The unique commit is the last one, 539eadfab2. A config save re-judges every active alert (`reevaluateActiveAlertsLocked`). Filesystem usage alerts were re-judged against thresholds their evaluator never uses: - **Agent filesystems** (`agent:<host>/disk:<label>`) were judged by the host disk threshold. `CheckHost` judges them by the filesystem's own override, then the `DiskFillByType` threshold for an inferred hardware type, then the host threshold. - **Guest filesystems** (`<guestID>-disk-<key>`) were resolved by the filesystem's resource ID. That ID matches no `guest-disk:` override and names no guest, so the guest's own override was missed too. Example: a filesystem override with trigger 70 raises an alert at 80 under a default of 90. Saving any unrelated alert setting resolved the alert and sent a recovery notification, and the next poll raised it again as a new alert. `CheckHost` and `CheckGuest` now resolve a filesystem's threshold through `hostDiskUsageThresholdNoLock` / `guestDiskUsageThresholdNoLock`, and reevaluation calls the same helpers: - `resolveHostAlertThresholdsNoLock` applies the host helper for agent filesystem alerts. - The new `resolveGuestAlertThresholdsNoLock` resolves a guest filesystem alert's guest by the ID `parseGuestAlertIdentity` reads from its resource ID. Both helpers return override and per-type thresholds as owned copies, so #2696's rule that evaluation never normalizes the live config still holds. `TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold` (in `threshold_resolution_shared_test.go`) covers four cases: - an agent filesystem override - an NVMe fill threshold - a guest filesystem override - a guest-wide override on a filesystem alert With `host.go`, `guest.go` and `config_runtime.go` overlaid by their origin/main or #2696 copies, all four fail at the unchanged save. `go test ./internal/alerts/...` passes, and the new test plus #2696's `TestAlertEvaluationLeavesTheLiveConfigToTheLock` pass under `-race`. Known limits, unchanged by this PR: - Custom rules that filter on live metrics still see no readings during reevaluation. - `pulse-relaxed` is not applied during reevaluation. - The mock generator's dash-separated guest IDs don't parse as guest identities.
2026-10-23 20:25:56 +00:00 · 2026-10-07 21:10:14 +01:00
parent 77d2119792 539eadfab2
commit 1ac600ec2b
16 changed files with 961 additions and 222 deletions
@@ -3180,6 +3180,76 @@ ownership is `internal/alerts/alert_identity_migration_test.go`,
 `internal/monitoring/monitor_alert_override_migration_test.go`, and
 `frontend-modern/src/features/alerts/__tests__/alertsConfigurationModel.snapshot.test.ts`.

+### Alert config snapshots are owned copies
+
+`Manager.GetConfig()` returns `AlertConfig.Clone()`, a typed deep copy that
+shares no map, slice or pointer with the live config (custom rule filter
+values are copied as decoded JSON values). Callers encode, persist
+and edit that snapshot outside `m.mu`: the GET config handler, alert
+activation, the identity migration, backup evaluation and diagnostics. It used
+to be a shallow copy, so `SaveAlertConfig` normalizing it in place, or
+`UpdateConfig` re-normalizing the overrides of a re-applied snapshot, wrote the
+live maps while evaluation or another encoder read them, and Go aborted Pulse
+with a concurrent map write. `SaveAlertConfig` also clones before normalizing,
+because a config just handed to `UpdateConfig` is the live config. Persistence
+keeps an explicit `0` type-level delay as no delay on save and load, matching
+`NormalizeTimeThresholds`. It used to store and load it as the 5-second factory
+delay, which the noisy-gauge rule stretches to 300 seconds for memory and
+temperature, and the shared map write was what made the running manager agree.
+Pulse saves never wrote a `0`, so only a newly sent or hand-edited `0` changes
+meaning. `internal/alerts/config/clone_test.go`
+fills every field and fails when a new reference field is not copied.
+Regression ownership is `internal/alerts/resolved_lock_discipline_test.go` and
+`internal/config/persistence_alert_ownership_test.go`.
+
+### Alert evaluation owns the thresholds it normalizes
+
+Host, guest and Docker container evaluation copy what they read from
+`m.config` while holding `m.mu`, and only then fill in defaults.
+`CheckHost` and `CheckGuest` clone a per-disk
+override's `Disk` threshold before `ensureHysteresisThreshold` sets a missing
+`Clear` to the trigger minus 5. They used to normalize the live override's
+pointer after unlocking, so a `GetConfig()` clone under the read lock raced
+the write, and the saved override changed to whatever the last poll filled in.
+The live override now keeps the clear it was saved with, and the config API
+returns that value. Docker container evaluation resolves its thresholds from
+cloned `DockerDefaults` plus the override under the same lock, and reads the
+restart-loop and memory-limit settings under the lock. It used to point into
+`m.config.DockerDefaults` with no lock while `UpdateConfig` replaced
+`m.config`. The test-only `getThresholdForMetricFromConfig`, which normalized
+in place, is gone. `internal/alerts/alerts_test.go`
+`TestAlertEvaluationLeavesTheLiveConfigToTheLock` runs host, guest and Docker
+evaluation against `GetConfig()` and `UpdateConfig(GetConfig())` under
+`-race`.
+
+### Config-save reevaluation judges a filesystem alert by its own threshold
+
+`UpdateConfig` re-judges every active alert, and a filesystem usage alert's
+threshold resolves through the same per-filesystem helper its evaluator uses.
+`hostDiskUsageThresholdNoLock` resolves an agent filesystem
+(`agent:<host>/disk:<label>`): its own override, then the `DiskFillByType`
+threshold for a hardware type inferred from the device while the host's disk
+alerting is on, then the host's disk threshold. `guestDiskUsageThresholdNoLock`
+resolves a guest filesystem (`<guestID>-disk-<key>`): its `guest-disk:`
+override, then the guest's disk threshold. `CheckHost` and `CheckGuest` call
+them under the read lock, and `resolveHostAlertThresholdsNoLock` and
+`resolveGuestAlertThresholdsNoLock` call them for reevaluation. Both return an
+override or per-type threshold as an owned copy. A guest filesystem alert
+resolves its guest's threshold, override and identity-based custom rules by the
+canonical `instance:node:vmid` guest ID that `parseGuestAlertIdentity` reads
+from its resource ID. Custom rules that filter on live metrics still see no
+readings there, as for every reevaluated guest alert. Reevaluation used to
+judge agent filesystems by the host threshold, and guest filesystems without
+their `guest-disk:` override or the guest's own override, because the
+filesystem's resource ID names no guest. Any settings save resolved an alert a
+per-disk threshold of 70 raised at 80 under a default of 90, notifying
+recovery, and the next poll raised it again as a new alert.
+`internal/alerts/threshold_resolution_shared_test.go`
+`TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold` covers an agent
+filesystem override, an NVMe fill threshold, a guest filesystem override and a
+guest override: an unchanged save must keep the alert, and a save lifting the
+trigger above the reading must resolve it.
+
 ### Versioned alert-intent policy

 The alerts runtime owns one versioned alert-intent document and its durable
@@ -2756,6 +2756,101 @@ func TestCheckBackupsIgnoresVMIDs(t *testing.T) {
 	}
 }

+// Host and guest evaluation used to fill in the Clear of a disk override's
+// threshold after dropping m.mu, writing through the live config's pointer
+// while GetConfig cloned it under the read lock, and Docker evaluation read
+// m.config.DockerDefaults with no lock while UpdateConfig replaced m.config.
+// Evaluation now copies what it needs under m.mu. Run with -race.
+func TestAlertEvaluationLeavesTheLiveConfigToTheLock(t *testing.T) {
+	originalLogLevel := zerolog.GlobalLevel()
+	zerolog.SetGlobalLevel(zerolog.Disabled)
+	t.Cleanup(func() {
+		zerolog.SetGlobalLevel(originalLogLevel)
+	})
+
+	m := newTestManager(t)
+
+	host := models.Host{
+		ID:       "eval-lock",
+		Hostname: "eval-lock",
+		Status:   "online",
+		Disks:    []models.Disk{{Mountpoint: "/", Device: "/dev/sda1", Usage: 50, Total: 100, Used: 50, Free: 50}},
+	}
+	hostDiskOverrideID, _ := hostDiskResourceID(host, host.Disks[0])
+	vm := models.VM{
+		ID:       BuildGuestKey("pve1", "node1", 101),
+		VMID:     101,
+		Name:     "eval-lock",
+		Node:     "node1",
+		Instance: "pve1",
+		Status:   "running",
+		Disks:    []models.Disk{{Mountpoint: "/boot", Device: "/dev/vda1", Usage: 50, Total: 100, Used: 50, Free: 50}},
+	}
+	guestDiskOverrideID := "guest-disk:guest:pve1:101/disk:boot-dev-vda1"
+	dockerHost := models.DockerHost{
+		ID:          "eval-lock-docker",
+		Hostname:    "eval-lock-docker",
+		DisplayName: "eval-lock-docker",
+		Containers: []models.DockerContainer{{
+			ID:            "abcdef123456",
+			Name:          "web",
+			State:         "running",
+			CPUPercent:    10,
+			MemoryPercent: 10,
+			MemoryUsage:   100 << 20,
+			MemoryLimit:   1 << 30,
+		}},
+	}
+
+	// A disk override saved with only a trigger leaves Clear for evaluation
+	// to default.
+	withTriggerOnlyDiskOverrides := func(cfg AlertConfig) AlertConfig {
+		cfg.Enabled = true
+		cfg.ActivationState = ActivationActive
+		cfg.Overrides = map[string]ThresholdConfig{
+			hostDiskOverrideID:  {Disk: &HysteresisThreshold{Trigger: 90}},
+			guestDiskOverrideID: {Disk: &HysteresisThreshold{Trigger: 90}},
+		}
+		return cfg
+	}
+	m.UpdateConfig(withTriggerOnlyDiskOverrides(m.GetConfig()))
+
+	m.CheckHost(host)
+	m.CheckGuest(vm, "pve1")
+	live := m.GetConfig()
+	for _, id := range []string{hostDiskOverrideID, guestDiskOverrideID} {
+		if got := live.Overrides[id].Disk.Clear; got != 0 {
+			t.Fatalf("evaluating %s wrote clear %v into the live override", id, got)
+		}
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(3)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < 100; i++ {
+			// Resetting the overrides gives evaluation a Clear to fill again.
+			m.UpdateConfig(withTriggerOnlyDiskOverrides(m.GetConfig()))
+			m.UpdateConfig(m.GetConfig())
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		for i := 0; i < 200; i++ {
+			_ = m.GetConfig()
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		for i := 0; i < 200; i++ {
+			m.CheckHost(host)
+			m.CheckGuest(vm, "pve1")
+			m.CheckDockerHost(dockerHost)
+		}
+	}()
+	wg.Wait()
+}
+
 func TestCheckDockerHostIgnoresContainersByPrefix(t *testing.T) {
 	m := newTestManager(t)

@@ -0,0 +1,164 @@
+package config
+
+import (
+	"maps"
+	"slices"
+)
+
+// Clone returns a deep copy of the configuration that shares no maps, slices
+// or pointers with c, so the copy can be changed, normalized or encoded
+// without touching c. Nil and empty maps and slices keep their distinction.
+// Custom rule filter values are copied as the JSON values the API and
+// alerts.json decode them to; other Go values stored there stay shared.
+//
+// The alert manager hands out its live configuration only through Clone;
+// callers persist, encode and edit what it returns outside the manager lock.
+func (c AlertConfig) Clone() AlertConfig {
+	clone := c
+	clone.ActivationTime = clonePointer(c.ActivationTime)
+	clone.GuestDefaults = c.GuestDefaults.Clone()
+	clone.NodeDefaults = c.NodeDefaults.Clone()
+	clone.AgentDefaults = c.AgentDefaults.Clone()
+	clone.DiskFillByType = maps.Clone(c.DiskFillByType)
+	clone.DiskTempByType = maps.Clone(c.DiskTempByType)
+	clone.DockerIgnoredContainerPrefixes = slices.Clone(c.DockerIgnoredContainerPrefixes)
+	clone.IgnoredGuestPrefixes = slices.Clone(c.IgnoredGuestPrefixes)
+	clone.GuestTagWhitelist = slices.Clone(c.GuestTagWhitelist)
+	clone.GuestTagBlacklist = slices.Clone(c.GuestTagBlacklist)
+	clone.PBSDefaults = c.PBSDefaults.Clone()
+	clone.KubernetesDefaults = c.KubernetesDefaults.Clone()
+	clone.TrueNASDefaults = c.TrueNASDefaults.Clone()
+	clone.TrueNASDiskDefaults = c.TrueNASDiskDefaults.Clone()
+	clone.VMwareDefaults = c.VMwareDefaults.Clone()
+	clone.BackupDefaults = c.BackupDefaults.clone()
+	if c.Overrides != nil {
+		clone.Overrides = make(map[string]ThresholdConfig, len(c.Overrides))
+		for id, override := range c.Overrides {
+			clone.Overrides[id] = override.Clone()
+		}
+	}
+	if c.CustomRules != nil {
+		clone.CustomRules = make([]CustomAlertRule, len(c.CustomRules))
+		for i, rule := range c.CustomRules {
+			clone.CustomRules[i] = rule.clone()
+		}
+	}
+	clone.Schedule.QuietHours.Days = maps.Clone(c.Schedule.QuietHours.Days)
+	if c.Schedule.Escalation.Levels != nil {
+		clone.Schedule.Escalation.Levels = make([]EscalationLevel, len(c.Schedule.Escalation.Levels))
+		for i, level := range c.Schedule.Escalation.Levels {
+			level.DestinationIDs = slices.Clone(level.DestinationIDs)
+			clone.Schedule.Escalation.Levels[i] = level
+		}
+	}
+	clone.TimeThresholds = maps.Clone(c.TimeThresholds)
+	clone.MetricTimeThresholds = cloneNestedIntMap(c.MetricTimeThresholds)
+	clone.MetricEvaluationWindows = cloneNestedIntMap(c.MetricEvaluationWindows)
+	return clone
+}
+
+// Clone returns a deep copy of the thresholds that shares no pointers with t.
+func (t ThresholdConfig) Clone() ThresholdConfig {
+	clone := t
+	clone.CPU = clonePointer(t.CPU)
+	clone.Memory = clonePointer(t.Memory)
+	clone.Disk = clonePointer(t.Disk)
+	clone.DiskRead = clonePointer(t.DiskRead)
+	clone.DiskWrite = clonePointer(t.DiskWrite)
+	clone.NetworkIn = clonePointer(t.NetworkIn)
+	clone.NetworkOut = clonePointer(t.NetworkOut)
+	clone.Usage = clonePointer(t.Usage)
+	clone.Temperature = clonePointer(t.Temperature)
+	clone.DiskTemperature = clonePointer(t.DiskTemperature)
+	clone.SMARTHealthFailure = clonePointer(t.SMARTHealthFailure)
+	clone.SMARTReallocated = clonePointer(t.SMARTReallocated)
+	clone.SMARTPending = clonePointer(t.SMARTPending)
+	clone.SMARTUncorrectable = clonePointer(t.SMARTUncorrectable)
+	clone.SMARTMediaErrors = clonePointer(t.SMARTMediaErrors)
+	clone.SMARTCRCErrorDelta = clonePointer(t.SMARTCRCErrorDelta)
+	clone.SMARTLifeWarning = clonePointer(t.SMARTLifeWarning)
+	clone.SMARTLifeCritical = clonePointer(t.SMARTLifeCritical)
+	clone.SMARTSpareWarning = clonePointer(t.SMARTSpareWarning)
+	clone.SMARTSpareCritical = clonePointer(t.SMARTSpareCritical)
+	if t.Backup != nil {
+		backup := t.Backup.clone()
+		clone.Backup = &backup
+	}
+	clone.Snapshot = clonePointer(t.Snapshot)
+	clone.Note = clonePointer(t.Note)
+	return clone
+}
+
+func (b BackupAlertConfig) clone() BackupAlertConfig {
+	clone := b
+	clone.AlertOrphaned = clonePointer(b.AlertOrphaned)
+	clone.IgnoreVMIDs = slices.Clone(b.IgnoreVMIDs)
+	return clone
+}
+
+func (r CustomAlertRule) clone() CustomAlertRule {
+	clone := r
+	if r.FilterConditions.Filters != nil {
+		clone.FilterConditions.Filters = make([]FilterCondition, len(r.FilterConditions.Filters))
+		for i, filter := range r.FilterConditions.Filters {
+			filter.Value = cloneJSONValue(filter.Value)
+			clone.FilterConditions.Filters[i] = filter
+		}
+	}
+	clone.Thresholds = r.Thresholds.Clone()
+	if r.Notifications.Email != nil {
+		email := *r.Notifications.Email
+		email.Recipients = slices.Clone(email.Recipients)
+		clone.Notifications.Email = &email
+	}
+	clone.Notifications.Webhook = clonePointer(r.Notifications.Webhook)
+	return clone
+}
+
+// clonePointer copies the value p points at. Only use it for types that hold
+// no maps, slices or pointers of their own.
+func clonePointer[T any](p *T) *T {
+	if p == nil {
+		return nil
+	}
+	clone := *p
+	return &clone
+}
+
+func cloneNestedIntMap(input map[string]map[string]int) map[string]map[string]int {
+	if input == nil {
+		return nil
+	}
+	clone := make(map[string]map[string]int, len(input))
+	for key, inner := range input {
+		clone[key] = maps.Clone(inner)
+	}
+	return clone
+}
+
+// cloneJSONValue copies a filter value as encoding/json decodes one: arrays
+// and objects are copied, scalars are values already.
+func cloneJSONValue(value interface{}) interface{} {
+	switch typed := value.(type) {
+	case []interface{}:
+		if typed == nil {
+			return typed
+		}
+		clone := make([]interface{}, len(typed))
+		for i, item := range typed {
+			clone[i] = cloneJSONValue(item)
+		}
+		return clone
+	case map[string]interface{}:
+		if typed == nil {
+			return typed
+		}
+		clone := make(map[string]interface{}, len(typed))
+		for key, item := range typed {
+			clone[key] = cloneJSONValue(item)
+		}
+		return clone
+	default:
+		return value
+	}
+}
@@ -0,0 +1,133 @@
+package config
+
+import (
+	"reflect"
+	"testing"
+	"time"
+)
+
+var timeType = reflect.TypeOf(time.Time{})
+
+// Every field is filled, so a map, slice or pointer added to the config later
+// fails here until Clone copies it.
+func TestAlertConfigCloneSharesNothingWithTheOriginal(t *testing.T) {
+	var original AlertConfig
+	fillForClone(reflect.ValueOf(&original).Elem())
+
+	clone := original.Clone()
+
+	if !reflect.DeepEqual(clone, original) {
+		t.Fatalf("clone differs from the original:\n got %+v\nwant %+v", clone, original)
+	}
+	assertNothingShared(t, "AlertConfig", reflect.ValueOf(original), reflect.ValueOf(clone))
+}
+
+func TestAlertConfigCloneKeepsNilAndEmptyApart(t *testing.T) {
+	original := AlertConfig{
+		Overrides:   map[string]ThresholdConfig{},
+		CustomRules: []CustomAlertRule{{FilterConditions: FilterStack{Filters: []FilterCondition{{Value: []interface{}(nil)}}}}},
+	}
+
+	clone := original.Clone()
+
+	if !reflect.DeepEqual(clone, original) {
+		t.Fatalf("clone differs from the original:\n got %#v\nwant %#v", clone, original)
+	}
+	if clone.Overrides == nil {
+		t.Fatal("clone turned an empty overrides map into nil")
+	}
+	if clone.TimeThresholds != nil {
+		t.Fatal("clone turned a nil delay map into an empty one")
+	}
+}
+
+func fillForClone(value reflect.Value) {
+	switch value.Kind() {
+	case reflect.Bool:
+		value.SetBool(true)
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		value.SetInt(7)
+	case reflect.Float32, reflect.Float64:
+		value.SetFloat(7.5)
+	case reflect.String:
+		value.SetString("filled")
+	case reflect.Pointer:
+		value.Set(reflect.New(value.Type().Elem()))
+		fillForClone(value.Elem())
+	case reflect.Map:
+		value.Set(reflect.MakeMap(value.Type()))
+		key := reflect.New(value.Type().Key()).Elem()
+		fillForClone(key)
+		item := reflect.New(value.Type().Elem()).Elem()
+		fillForClone(item)
+		value.SetMapIndex(key, item)
+	case reflect.Slice:
+		value.Set(reflect.MakeSlice(value.Type(), 1, 1))
+		fillForClone(value.Index(0))
+	case reflect.Interface:
+		// Filter values arrive from encoding/json.
+		value.Set(reflect.ValueOf(map[string]interface{}{"filled": []interface{}{"filled", 7.5}}))
+	case reflect.Struct:
+		if value.Type() == timeType {
+			value.Set(reflect.ValueOf(time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC)))
+			return
+		}
+		for i := 0; i < value.NumField(); i++ {
+			if value.Field(i).CanSet() {
+				fillForClone(value.Field(i))
+			}
+		}
+	}
+}
+
+func assertNothingShared(t *testing.T, path string, original, clone reflect.Value) {
+	t.Helper()
+	switch original.Kind() {
+	case reflect.Pointer:
+		if original.IsNil() {
+			return
+		}
+		if original.Pointer() == clone.Pointer() {
+			t.Errorf("%s: clone shares the pointer", path)
+			return
+		}
+		assertNothingShared(t, path, original.Elem(), clone.Elem())
+	case reflect.Map:
+		if original.IsNil() {
+			return
+		}
+		if original.Pointer() == clone.Pointer() {
+			t.Errorf("%s: clone shares the map", path)
+			return
+		}
+		iter := original.MapRange()
+		for iter.Next() {
+			assertNothingShared(t, path+"[key]", iter.Value(), clone.MapIndex(iter.Key()))
+		}
+	case reflect.Slice:
+		if original.Len() == 0 {
+			return
+		}
+		if original.Pointer() == clone.Pointer() {
+			t.Errorf("%s: clone shares the slice", path)
+			return
+		}
+		for i := 0; i < original.Len(); i++ {
+			assertNothingShared(t, path+"[i]", original.Index(i), clone.Index(i))
+		}
+	case reflect.Interface:
+		if original.IsNil() {
+			return
+		}
+		assertNothingShared(t, path, original.Elem(), clone.Elem())
+	case reflect.Struct:
+		if original.Type() == timeType {
+			return
+		}
+		for i := 0; i < original.NumField(); i++ {
+			if original.Type().Field(i).IsExported() {
+				assertNothingShared(t, path+"."+original.Type().Field(i).Name, original.Field(i), clone.Field(i))
+			}
+		}
+	}
+}
@@ -545,7 +545,7 @@ func (m *Manager) reevaluateActiveAlertsLocked() {
 				continue
 			}

-			guestThresholds := m.getGuestThresholds(guestSnapshotFromAlert(alert, resourceID), resourceID)
+			guestThresholds := m.resolveGuestAlertThresholdsNoLock(alert, resourceID)
 			if guestThresholds.Disabled {
 				alertsToResolve = append(alertsToResolve, alertID)
 				continue
@@ -634,11 +634,15 @@ func (m *Manager) reevaluateActiveAlertsLocked() {
 	}
 }

-// GetConfig returns the current alert configuration.
+// GetConfig returns a copy of the alert configuration that the caller owns.
+// Callers encode, persist and edit it outside m.mu, and UpdateConfig
+// normalizes the maps it is handed in place, so a copy that shared the live
+// config's maps let a save or a re-apply write them while evaluation read
+// them, which Go aborts as a concurrent map write.
 func (m *Manager) GetConfig() AlertConfig {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
-	return m.config
+	return m.config.Clone()
 }

 func cloneThreshold(threshold *HysteresisThreshold) *HysteresisThreshold {
@@ -352,8 +352,19 @@ func (m *Manager) evaluateDockerContainer(host models.DockerHost, container mode
 	resourceType := "app-container"
 	containerTags := dockerLabelTags(container.Labels)

+	// Resolve the metric thresholds while the lock is held: the Docker defaults
+	// are fields of m.config, which UpdateConfig replaces, and the override
+	// still points into it. applyThresholdOverride clones what it keeps.
 	m.mu.RLock()
 	overrideConfig, hasOverride := m.lookupDockerContainerOverrideNoLock(host.ID, container.Name, resourceID)
+	thresholds := ThresholdConfig{
+		CPU:    cloneThreshold(&m.config.DockerDefaults.CPU),
+		Memory: cloneThreshold(&m.config.DockerDefaults.Memory),
+		Disk:   cloneThreshold(&m.config.DockerDefaults.Disk),
+	}
+	if hasOverride {
+		thresholds = m.applyThresholdOverride(thresholds, overrideConfig)
+	}
 	m.mu.RUnlock()
 	if hasOverride && overrideConfig.Disabled {
 		// Alerts disabled via override; clear any existing alerts and skip evaluation.
@@ -377,16 +388,6 @@ func (m *Manager) evaluateDockerContainer(host models.DockerHost, container mode
 	} else {
 		m.clearDockerContainerStateAlert(resourceID)

-		// Use Docker-specific defaults for containers
-		thresholds := ThresholdConfig{
-			CPU:    &m.config.DockerDefaults.CPU,
-			Memory: &m.config.DockerDefaults.Memory,
-			Disk:   &m.config.DockerDefaults.Disk,
-		}
-		if hasOverride {
-			thresholds = m.applyThresholdOverride(thresholds, overrideConfig)
-		}
-
 		if thresholds.CPU != nil {
 			cpuCapacityPercent := models.DockerContainerCPUCapacityPercent(container, host.CPUs)
 			cpuMetadata := map[string]interface{}{
@@ -998,6 +999,8 @@ func (m *Manager) checkDockerContainerRestartLoop(host models.DockerHost, contai
 	alertID := fmt.Sprintf("docker-container-restart-loop-%s", resourceID)
 	now := time.Now()

+	m.mu.Lock()
+
 	// Get config values with defaults
 	restartThreshold := m.config.DockerDefaults.RestartCount
 	if restartThreshold == 0 {
@@ -1008,8 +1011,6 @@ func (m *Manager) checkDockerContainerRestartLoop(host models.DockerHost, contai
 		timeWindow = 300 // Default: 5 minutes (300 seconds)
 	}

-	m.mu.Lock()
-
 	record, exists := m.dockerRestartTracking[resourceID]
 	if !exists {
 		record = &dockerRestartRecord{
@@ -1164,11 +1165,13 @@ func (m *Manager) checkDockerContainerMemoryLimit(host models.DockerHost, contai
 	alertID := fmt.Sprintf("docker-container-memory-limit-%s", resourceID)

 	// Get config values with defaults
+	m.mu.RLock()
 	warnThreshold := float64(m.config.DockerDefaults.MemoryWarnPct)
+	criticalThreshold := float64(m.config.DockerDefaults.MemoryCriticalPct)
+	m.mu.RUnlock()
 	if warnThreshold == 0 {
 		warnThreshold = 90.0 // Default: 90%
 	}
-	criticalThreshold := float64(m.config.DockerDefaults.MemoryCriticalPct)
 	if criticalThreshold == 0 {
 		criticalThreshold = 95.0 // Default: 95%
 	}
@@ -327,18 +327,12 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
 			seenDiskResources[perDiskResourceID] = struct{}{}
 			message := fmt.Sprintf("%s disk (%s) at %.1f%%", guestType, label, disk.Usage)

-			effectiveDiskThreshold := thresholds.Disk
 			m.mu.RLock()
-			diskOverride, hasDiskOverride := lookupGuestDiskOverride(m.config.Overrides, guest, guestID, keySource)
+			effectiveDiskThreshold, diskDisabled := m.guestDiskUsageThresholdNoLock(guest, guestID, keySource, thresholds.Disk)
 			m.mu.RUnlock()
-			if hasDiskOverride {
-				if diskOverride.Disabled {
-					m.clearAlert(canonicalMetricStateID(perDiskResourceID, "disk"))
-					continue
-				}
-				if diskOverride.Disk != nil {
-					effectiveDiskThreshold = ensureHysteresisThreshold(diskOverride.Disk)
-				}
+			if diskDisabled {
+				m.clearAlert(canonicalMetricStateID(perDiskResourceID, "disk"))
+				continue
 			}
 			if snapshot.DiskUnavailable {
 				if effectiveDiskThreshold == nil || effectiveDiskThreshold.Trigger <= 0 {
@@ -423,6 +417,45 @@ func guestDiskIdentity(disk models.Disk, idx int) (label, keySource, sanitizedKe
 	return label, keySource, sanitizedKey
 }

+// guestDiskUsageThresholdNoLock resolves the usage threshold one guest
+// filesystem is judged by: the filesystem's own override, else the guest's disk
+// threshold. disabled reports an override that switches the filesystem's
+// alerts off, and then the threshold is nil. CheckGuest and config-save
+// reevaluation both resolve through it. An override threshold is returned as
+// an owned copy, so normalizing it never writes into the live config. Callers
+// must hold m.mu.
+func (m *Manager) guestDiskUsageThresholdNoLock(guest any, guestID, diskKey string, guestDisk *HysteresisThreshold) (threshold *HysteresisThreshold, disabled bool) {
+	if override, ok := lookupGuestDiskOverride(m.config.Overrides, guest, guestID, diskKey); ok {
+		if override.Disabled {
+			return nil, true
+		}
+		if override.Disk != nil {
+			return ensureHysteresisThreshold(cloneThreshold(override.Disk)), false
+		}
+	}
+	return guestDisk, false
+}
+
+// resolveGuestAlertThresholdsNoLock resolves thresholds for a persisted guest
+// alert. A filesystem alert ("<guestID>-disk-<key>") resolves its guest's
+// thresholds by the guest ID, and its Disk by the filesystem's own override, as
+// CheckGuest does. Resolved by the filesystem's resource ID, which names no
+// guest, it missed both overrides, and a config save resolved an alert the next
+// poll raised again. Callers must hold m.mu.
+func (m *Manager) resolveGuestAlertThresholdsNoLock(alert *Alert, resourceID string) ThresholdConfig {
+	resourceID = strings.TrimSpace(resourceID)
+	if identity, ok := parseGuestAlertIdentity(resourceID); ok {
+		if diskKey, isFilesystem := strings.CutPrefix(identity.resourceSuffix, "-disk-"); isFilesystem {
+			guestID := strings.TrimSuffix(resourceID, identity.resourceSuffix)
+			guest := guestSnapshotFromAlert(alert, guestID)
+			thresholds := m.getGuestThresholds(guest, guestID)
+			thresholds.Disk, _ = m.guestDiskUsageThresholdNoLock(guest, guestID, diskKey, thresholds.Disk)
+			return thresholds
+		}
+	}
+	return m.getGuestThresholds(guestSnapshotFromAlert(alert, resourceID), resourceID)
+}
+
 func guestIORateMetrics(snapshot guestSnapshot) (diskRead, diskWrite, networkIn, networkOut *UnifiedResourceMetric) {
 	validity := snapshot.IORateValidity.EffectiveForRates(
 		snapshot.DiskRead,
@@ -172,9 +172,50 @@ func (m *Manager) resolveHostAlertThresholdsNoLock(alert *Alert, resourceID stri
 			thresholds.DiskTemperature = m.lowestHostDiskTemperatureThresholdNoLock(thresholds.DiskTemperature)
 		}
 	}
+	// A filesystem alert is judged by the threshold CheckHost evaluates that
+	// filesystem against, so a config save does not resolve an alert its
+	// override or per-type fill threshold still holds. A disabled filesystem
+	// resolves with no threshold, as CheckHost clears it.
+	if alert != nil && alert.Type == "disk" && isHostFilesystemResourceID(resourceID) {
+		thresholds.Disk, _ = m.hostDiskUsageThresholdNoLock(strings.TrimSpace(resourceID), metadataStringValue(alert.Metadata, "device"), thresholds.Disk)
+	}
 	return thresholds
 }

+// isHostFilesystemResourceID reports whether resourceID names one agent
+// filesystem ("agent:<host>/disk:<label>") rather than the host itself.
+func isHostFilesystemResourceID(resourceID string) bool {
+	_, child, ok := strings.Cut(stripHostResourcePrefix(resourceID), "/")
+	return ok && strings.HasPrefix(child, "disk:")
+}
+
+// hostDiskUsageThresholdNoLock resolves the usage threshold one agent
+// filesystem is judged by: the filesystem's own override, then the fill
+// threshold for its hardware type while the host's disk alerting is on, then
+// the host's disk threshold. disabled reports an override that switches the
+// filesystem's alerts off, and then the threshold is nil. CheckHost and
+// config-save reevaluation both resolve through it. An override or per-type
+// threshold is returned as an owned copy, so normalizing it never writes into
+// the live config. Callers must hold m.mu.
+func (m *Manager) hostDiskUsageThresholdNoLock(diskResourceID, device string, hostDisk *HysteresisThreshold) (threshold *HysteresisThreshold, disabled bool) {
+	if override, ok := m.config.Overrides[diskResourceID]; ok {
+		if override.Disabled {
+			return nil, true
+		}
+		if override.Disk != nil {
+			return ensureHysteresisThreshold(cloneThreshold(override.Disk)), false
+		}
+	}
+	if hostDisk != nil && hostDisk.Trigger > 0 {
+		if hwType := inferDiskHardwareType(device); hwType != "" {
+			if byType, ok := m.config.DiskFillByType[hwType]; ok {
+				return &byType, false
+			}
+		}
+	}
+	return hostDisk, false
+}
+
 func sanitizeHostComponent(value string) string {
 	value = strings.TrimSpace(strings.ToLower(value))
 	if value == "" {
@@ -450,39 +491,12 @@ func (m *Manager) CheckHost(host models.Host) {
 		diskResourceID, diskName := hostDiskResourceID(host, disk)
 		seenDisks[diskResourceID] = struct{}{}

-		// Check for disk-specific override
 		m.mu.RLock()
-		diskOverride, hasDiskOverride := m.config.Overrides[diskResourceID]
+		effectiveDiskThreshold, diskDisabled := m.hostDiskUsageThresholdNoLock(diskResourceID, disk.Device, thresholds.Disk)
 		m.mu.RUnlock()
-
-		// Determine the effective disk threshold
-		var effectiveDiskThreshold *HysteresisThreshold
-		if hasDiskOverride {
-			// If disk is disabled via override, skip alerting
-			if diskOverride.Disabled {
-				m.clearAlert(canonicalMetricStateID(diskResourceID, "disk"))
-				continue
-			}
-			// Use disk-specific threshold if set
-			if diskOverride.Disk != nil {
-				effectiveDiskThreshold = ensureHysteresisThreshold(diskOverride.Disk)
-			}
-		}
-		// Per-type override: consult DiskFillByType if hardware type is inferable
-		// from the device path and no disk-specific override applied above.
-		if effectiveDiskThreshold == nil && thresholds.Disk != nil && thresholds.Disk.Trigger > 0 {
-			if hwType := inferDiskHardwareType(disk.Device); hwType != "" {
-				m.mu.RLock()
-				if th, ok := m.config.DiskFillByType[hwType]; ok {
-					t := th
-					effectiveDiskThreshold = &t
-				}
-				m.mu.RUnlock()
-			}
-		}
-		// Fall back to host-level threshold
-		if effectiveDiskThreshold == nil {
-			effectiveDiskThreshold = thresholds.Disk
+		if diskDisabled {
+			m.clearAlert(canonicalMetricStateID(diskResourceID, "disk"))
+			continue
 		}

 		// Skip if no threshold configured (nil)
@@ -86,16 +86,6 @@ func getThresholdForMetric(config ThresholdConfig, metricType string) *Hysteresi
 	}
 }

-// getThresholdForMetricFromConfig returns the threshold for a specific metric type from a ThresholdConfig
-// ensuring hysteresis is properly set.
-func getThresholdForMetricFromConfig(config ThresholdConfig, metricType string) *HysteresisThreshold {
-	th := getThresholdForMetric(config, metricType)
-	if th == nil {
-		return nil
-	}
-	return ensureHysteresisThreshold(th)
-}
-
 // getTimeThreshold determines the delay to apply for a metric/resource combination.
 func (m *Manager) getTimeThreshold(resourceID string, resourceType, metricType string) int {
 	effective := m.resolveEffectiveIntentPolicyNoLock(resourceID, resourceType, MetricAlertIntentSignal(metricType))
@@ -1,6 +1,7 @@
 package alerts

 import (
+	"encoding/json"
 	"fmt"
 	"runtime"
 	"sync"
@@ -8,6 +9,7 @@ import (
 	"time"

 	alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs"
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
 	"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
 	"github.com/rs/zerolog"
 )
@@ -304,3 +306,80 @@ func waitForResolvedConcurrencyGroup(t *testing.T, wg *sync.WaitGroup) {
 		t.Fatal("resolved alert concurrency paths deadlocked")
 	}
 }
+
+// GetConfig snapshots are encoded, persisted and edited outside m.mu, and
+// UpdateConfig normalizes the maps it is handed in place. While a snapshot
+// shared the live config's maps, re-applying one rewrote the override map
+// that the GET /api/alerts/config handler was encoding from another snapshot,
+// and Go aborted the process with a concurrent map write. Run with -race.
+func TestConfigSnapshotsShareNothingWithTheLiveConfig(t *testing.T) {
+	originalLogLevel := zerolog.GlobalLevel()
+	zerolog.SetGlobalLevel(zerolog.Disabled)
+	t.Cleanup(func() {
+		zerolog.SetGlobalLevel(originalLogLevel)
+	})
+
+	m := NewManagerWithDataDir(t.TempDir())
+	t.Cleanup(m.Stop)
+
+	seed := m.GetConfig()
+	seed.Enabled = true
+	seed.ActivationState = ActivationActive
+	seed.Overrides = make(map[string]ThresholdConfig)
+	for i := 0; i < 64; i++ {
+		seed.Overrides[fmt.Sprintf("agent:snapshot-%d", i)] = ThresholdConfig{
+			CPU: &HysteresisThreshold{Trigger: 95, Clear: 90},
+		}
+	}
+	m.UpdateConfig(seed)
+
+	snapshot := m.GetConfig()
+	snapshot.Overrides["agent:snapshot-0"].CPU.Trigger = 1
+	delete(snapshot.Overrides, "agent:snapshot-1")
+	snapshot.TimeThresholds["guest"] = 99
+	snapshot.AgentDefaults.CPU.Trigger = 1
+
+	live := m.GetConfig()
+	if got := live.Overrides["agent:snapshot-0"].CPU.Trigger; got != 95 {
+		t.Fatalf("editing a snapshot changed the live override trigger to %v", got)
+	}
+	if _, ok := live.Overrides["agent:snapshot-1"]; !ok {
+		t.Fatal("deleting from a snapshot removed the live override")
+	}
+	if live.TimeThresholds["guest"] == 99 {
+		t.Fatal("editing a snapshot changed the live guest delay")
+	}
+	if live.AgentDefaults.CPU.Trigger == 1 {
+		t.Fatal("editing a snapshot changed the live agent CPU trigger")
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(3)
+	go func() {
+		defer wg.Done()
+		for i := 0; i < 200; i++ {
+			m.UpdateConfig(m.GetConfig())
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		for i := 0; i < 200; i++ {
+			if _, err := json.Marshal(m.GetConfig()); err != nil {
+				t.Errorf("encode snapshot: %v", err)
+				return
+			}
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		for i := 0; i < 200; i++ {
+			id := fmt.Sprintf("snapshot-%d", i%64)
+			m.CheckHost(models.Host{ID: id, Hostname: id, Status: "online", CPUUsage: 50})
+		}
+	}()
+	wg.Wait()
+
+	if got := len(m.GetConfig().Overrides); got != 64 {
+		t.Fatalf("live overrides = %d after re-applying snapshots, want 64", got)
+	}
+}
@@ -1350,6 +1350,123 @@ func TestGuestFilesystemEvidenceSuppressesDuplicateAggregateAlert(t *testing.T)
 	}
 }

+// A config save re-judges every active alert. A filesystem alert must be judged
+// by the threshold its evaluator used for that filesystem, not the host or guest
+// default: saving any unrelated setting used to resolve an alert a per-disk
+// threshold of 70 raised at 80 under a default of 90, notifying recovery, and
+// the next poll raised it again as a new alert.
+func TestConfigSaveJudgesFilesystemAlertsByTheirOwnThreshold(t *testing.T) {
+	perDisk := func() *HysteresisThreshold { return &HysteresisThreshold{Trigger: 70, Clear: 65} }
+	abovePerDisk := func() *HysteresisThreshold { return &HysteresisThreshold{Trigger: 85, Clear: 80} }
+
+	agentHost := func(device string) models.Host {
+		return models.Host{
+			ID: "host-fs", Hostname: "host-fs", Platform: "linux", Status: "online",
+			IntervalSeconds: 30, LastSeen: time.Now(),
+			Disks: []models.Disk{{Mountpoint: "/data", Device: device, Usage: 80, Total: 100, Used: 80, Free: 20}},
+		}
+	}
+	guestID := BuildGuestKey("pve1", "node1", 101)
+	guestVM := models.VM{
+		ID: guestID, VMID: 101, Name: "db", Node: "node1", Instance: "pve1", Status: "running",
+		Disks: []models.Disk{{Mountpoint: "/data", Device: "/dev/vdb1", Usage: 80, Total: 100, Used: 80, Free: 20}},
+	}
+	guestAlertID := canonicalMetricStateID(guestID+"-disk-data-dev-vdb1", "disk")
+
+	type diskCase struct {
+		name      string
+		alertID   string
+		configure func(cfg *AlertConfig)
+		evaluate  func(m *Manager)
+		// raise lifts the threshold that judges this filesystem above its reading.
+		raise func(cfg *AlertConfig)
+	}
+
+	agentDiskID, _ := hostDiskResourceID(agentHost("/dev/sdb1"), agentHost("/dev/sdb1").Disks[0])
+	guestDiskOverrideID := guestDiskOverrideKey(guestVM, guestID, "/data-/dev/vdb1")
+	guestOverrideID := guestOverridePrimaryKey(guestVM, guestID)
+
+	cases := []diskCase{
+		{
+			name:    "agent filesystem override",
+			alertID: canonicalMetricStateID(agentDiskID, "disk"),
+			configure: func(cfg *AlertConfig) {
+				cfg.Overrides = map[string]ThresholdConfig{agentDiskID: {Disk: perDisk()}}
+			},
+			evaluate: func(m *Manager) { m.CheckHost(agentHost("/dev/sdb1")) },
+			raise: func(cfg *AlertConfig) {
+				cfg.Overrides = map[string]ThresholdConfig{agentDiskID: {Disk: abovePerDisk()}}
+			},
+		},
+		{
+			name:    "agent disk fill by hardware type",
+			alertID: canonicalMetricStateID(agentDiskID, "disk"),
+			configure: func(cfg *AlertConfig) {
+				cfg.DiskFillByType["nvme"] = *perDisk()
+			},
+			evaluate: func(m *Manager) { m.CheckHost(agentHost("/dev/nvme0n1p1")) },
+			raise: func(cfg *AlertConfig) {
+				cfg.DiskFillByType["nvme"] = *abovePerDisk()
+			},
+		},
+		{
+			name:    "guest filesystem override",
+			alertID: guestAlertID,
+			configure: func(cfg *AlertConfig) {
+				cfg.Overrides = map[string]ThresholdConfig{guestDiskOverrideID: {Disk: perDisk()}}
+			},
+			evaluate: func(m *Manager) { m.CheckGuest(guestVM, "pve1") },
+			raise: func(cfg *AlertConfig) {
+				cfg.Overrides = map[string]ThresholdConfig{guestDiskOverrideID: {Disk: abovePerDisk()}}
+			},
+		},
+		{
+			name:    "guest override on a filesystem alert",
+			alertID: guestAlertID,
+			configure: func(cfg *AlertConfig) {
+				cfg.Overrides = map[string]ThresholdConfig{guestOverrideID: {Disk: perDisk()}}
+			},
+			evaluate: func(m *Manager) { m.CheckGuest(guestVM, "pve1") },
+			raise: func(cfg *AlertConfig) {
+				cfg.Overrides = map[string]ThresholdConfig{guestOverrideID: {Disk: abovePerDisk()}}
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			m := newTestManager(t)
+			cfg := m.GetConfig()
+			cfg.Enabled = true
+			// UpdateConfig fills missing delays with 5s; zero fires at once.
+			cfg.TimeThresholds = map[string]int{"agent": 0, "guest": 0}
+			cfg.AgentDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85}
+			cfg.GuestDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85}
+			tc.configure(&cfg)
+			m.UpdateConfig(cfg)
+
+			tc.evaluate(m)
+			startedAt := testRequireActiveAlert(t, m, tc.alertID).StartTime
+
+			m.UpdateConfig(m.GetConfig())
+			if !testHasActiveAlert(t, m, tc.alertID) {
+				t.Fatalf("an unchanged config save resolved %q, which its own threshold of 70 still holds at 80", tc.alertID)
+			}
+			tc.evaluate(m)
+			if got := testRequireActiveAlert(t, m, tc.alertID).StartTime; !got.Equal(startedAt) {
+				t.Fatalf("alert %q was raised again after the save: started %v, want %v", tc.alertID, got, startedAt)
+			}
+
+			cfg = m.GetConfig()
+			tc.raise(&cfg)
+			m.UpdateConfig(cfg)
+			if testHasActiveAlert(t, m, tc.alertID) {
+				t.Fatalf("a save lifting %q's own trigger above its reading kept the alert", tc.alertID)
+			}
+		})
+	}
+}
+
 // Regression: checkMetric stores canonical-identity alerts under the
 // canonical state key, so hysteresis resolution must not remove only the
 // unregistered legacy "<resourceID>-<metric>" ID.
@@ -2469,148 +2469,3 @@ func TestGetThresholdForMetric_EmptyConfig(t *testing.T) {
 		})
 	}
 }
-
-// TestGetThresholdForMetricFromConfig tests the getThresholdForMetricFromConfig function
-func TestGetThresholdForMetricFromConfig(t *testing.T) {
-	// t.Parallel()
-
-	tests := []struct {
-		name        string
-		config      ThresholdConfig
-		metricType  string
-		wantNil     bool
-		wantTrigger float64
-		wantClear   float64
-	}{
-		{
-			name:       "empty config returns nil",
-			config:     ThresholdConfig{},
-			metricType: "cpu",
-			wantNil:    true,
-		},
-		{
-			name: "cpu threshold returned",
-			config: ThresholdConfig{
-				CPU: &HysteresisThreshold{Trigger: 80, Clear: 70},
-			},
-			metricType:  "cpu",
-			wantTrigger: 80,
-			wantClear:   70,
-		},
-		{
-			name: "memory threshold returned",
-			config: ThresholdConfig{
-				Memory: &HysteresisThreshold{Trigger: 85, Clear: 75},
-			},
-			metricType:  "memory",
-			wantTrigger: 85,
-			wantClear:   75,
-		},
-		{
-			name: "disk threshold returned",
-			config: ThresholdConfig{
-				Disk: &HysteresisThreshold{Trigger: 90, Clear: 85},
-			},
-			metricType:  "disk",
-			wantTrigger: 90,
-			wantClear:   85,
-		},
-		{
-			name: "diskRead threshold returned",
-			config: ThresholdConfig{
-				DiskRead: &HysteresisThreshold{Trigger: 50, Clear: 40},
-			},
-			metricType:  "diskRead",
-			wantTrigger: 50,
-			wantClear:   40,
-		},
-		{
-			name: "diskWrite threshold returned",
-			config: ThresholdConfig{
-				DiskWrite: &HysteresisThreshold{Trigger: 55, Clear: 45},
-			},
-			metricType:  "diskWrite",
-			wantTrigger: 55,
-			wantClear:   45,
-		},
-		{
-			name: "networkIn threshold returned",
-			config: ThresholdConfig{
-				NetworkIn: &HysteresisThreshold{Trigger: 70, Clear: 60},
-			},
-			metricType:  "networkIn",
-			wantTrigger: 70,
-			wantClear:   60,
-		},
-		{
-			name: "networkOut threshold returned",
-			config: ThresholdConfig{
-				NetworkOut: &HysteresisThreshold{Trigger: 75, Clear: 65},
-			},
-			metricType:  "networkOut",
-			wantTrigger: 75,
-			wantClear:   65,
-		},
-		{
-			name: "temperature threshold returned",
-			config: ThresholdConfig{
-				Temperature: &HysteresisThreshold{Trigger: 65, Clear: 55},
-			},
-			metricType:  "temperature",
-			wantTrigger: 65,
-			wantClear:   55,
-		},
-		{
-			name: "usage threshold returned",
-			config: ThresholdConfig{
-				Usage: &HysteresisThreshold{Trigger: 88, Clear: 78},
-			},
-			metricType:  "usage",
-			wantTrigger: 88,
-			wantClear:   78,
-		},
-		{
-			name: "unknown metric returns nil",
-			config: ThresholdConfig{
-				CPU: &HysteresisThreshold{Trigger: 80, Clear: 70},
-			},
-			metricType: "unknown",
-			wantNil:    true,
-		},
-		{
-			name: "threshold with zero clear gets default hysteresis",
-			config: ThresholdConfig{
-				CPU: &HysteresisThreshold{Trigger: 80, Clear: 0},
-			},
-			metricType:  "cpu",
-			wantTrigger: 80,
-			wantClear:   75, // 80 - 5 default margin
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			// t.Parallel()
-
-			result := getThresholdForMetricFromConfig(tc.config, tc.metricType)
-
-			if tc.wantNil {
-				if result != nil {
-					t.Errorf("getThresholdForMetricFromConfig() = %v, want nil", result)
-				}
-				return
-			}
-
-			if result == nil {
-				t.Fatalf("getThresholdForMetricFromConfig() = nil, want non-nil")
-			}
-
-			if result.Trigger != tc.wantTrigger {
-				t.Errorf("Trigger = %v, want %v", result.Trigger, tc.wantTrigger)
-			}
-			if result.Clear != tc.wantClear {
-				t.Errorf("Clear = %v, want %v", result.Clear, tc.wantClear)
-			}
-		})
-	}
-}
@@ -848,8 +848,11 @@ func normalizeAlertDefaults(config *alerts.AlertConfig) {
 		config.TimeThresholds = make(map[string]int)
 	}
 	defaultDelay := 5
+	// Zero means no delay, as the alert manager reads it
+	// (alerts.Manager.UpdateConfig keeps it); only a missing or negative
+	// delay takes the default.
 	ensureDelay := func(key string) {
-		if delay, ok := config.TimeThresholds[key]; !ok || delay <= 0 {
+		if delay, ok := config.TimeThresholds[key]; !ok || delay < 0 {
 			config.TimeThresholds[key] = defaultDelay
 		}
 	}
@@ -858,7 +861,7 @@ func normalizeAlertDefaults(config *alerts.AlertConfig) {
 	ensureDelay("agent")
 	ensureDelay("storage")
 	ensureDelay("pbs")
-	if delay, ok := config.TimeThresholds["all"]; ok && delay <= 0 {
+	if delay, ok := config.TimeThresholds["all"]; ok && delay < 0 {
 		config.TimeThresholds["all"] = defaultDelay
 	}

@@ -928,6 +931,11 @@ func (c *ConfigPersistence) SaveAlertConfig(config alerts.AlertConfig) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()

+	// Normalize a copy. Callers hand in alerts.Manager.GetConfig() snapshots
+	// and configs they have just applied to the manager, so normalizing their
+	// maps and threshold pointers in place wrote into the running config.
+	config = config.Clone()
+
 	// Save-specific: normalize override clear values before shared defaults run
 	if config.MinimumDelta <= 0 {
 		margin := config.HysteresisMargin
@@ -0,0 +1,171 @@
+package config_test
+
+import (
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"sync"
+	"testing"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/alerts"
+	"github.com/rcourtman/pulse-go-rewrite/internal/config"
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
+	"github.com/rs/zerolog"
+)
+
+// SaveAlertConfig used to normalize the config it was handed in place.
+// Callers hand it alerts.Manager.GetConfig() snapshots, whose maps and
+// threshold pointers were the live manager config's own, so every save
+// wrote into the running config outside the manager lock. Evaluation reads
+// those maps under the lock and the GET handler encodes a snapshot outside
+// it, so a save racing either one could abort Pulse with a concurrent map
+// write. Run with -race: the rounds below take the same steps as alert
+// activation, the identity migration and GET /api/alerts/config, while
+// evaluation runs.
+func TestSaveAlertConfigLeavesTheLiveManagerConfigAlone(t *testing.T) {
+	originalLevel := zerolog.GlobalLevel()
+	zerolog.SetGlobalLevel(zerolog.Disabled)
+	t.Cleanup(func() { zerolog.SetGlobalLevel(originalLevel) })
+
+	manager := alerts.NewManagerWithDataDir(t.TempDir())
+	t.Cleanup(manager.Stop)
+
+	seed := manager.GetConfig()
+	seed.Enabled = true
+	seed.ActivationState = alerts.ActivationActive
+	// Agent CPU alerts off: persistence rewrites the clear value of a zero
+	// trigger, so a shared threshold pointer is written on every save.
+	seed.AgentDefaults.CPU = &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}
+	// No delay for guests: persistence used to replace an explicit zero with
+	// five seconds, writing the live delay map.
+	seed.TimeThresholds = map[string]int{"guest": 0}
+	seed.Overrides = make(map[string]alerts.ThresholdConfig)
+	for i := 0; i < 64; i++ {
+		seed.Overrides[fmt.Sprintf("agent:host-%d", i)] = alerts.ThresholdConfig{
+			CPU: &alerts.HysteresisThreshold{Trigger: 95, Clear: 90},
+		}
+	}
+	manager.UpdateConfig(seed)
+	want := manager.GetConfig()
+	wantJSON, err := json.Marshal(want)
+	if err != nil {
+		t.Fatalf("marshal seeded config: %v", err)
+	}
+
+	persistence := config.NewConfigPersistence(t.TempDir())
+	if err := persistence.EnsureConfigDir(); err != nil {
+		t.Fatalf("EnsureConfigDir: %v", err)
+	}
+
+	const rounds = 200
+	var wg sync.WaitGroup
+	run := func(step func(round int)) {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for round := 0; round < rounds; round++ {
+				step(round)
+			}
+		}()
+	}
+	// POST /api/alerts/activate: apply a snapshot, then persist it.
+	run(func(int) {
+		cfg := manager.GetConfig()
+		manager.UpdateConfig(cfg)
+		if err := persistence.SaveAlertConfig(cfg); err != nil {
+			t.Errorf("save after update: %v", err)
+		}
+	})
+	// The alert identity migration persists a snapshot.
+	run(func(int) {
+		if err := persistence.SaveAlertConfig(manager.GetConfig()); err != nil {
+			t.Errorf("save snapshot: %v", err)
+		}
+	})
+	// GET /api/alerts/config encodes a snapshot outside the manager lock.
+	run(func(int) {
+		if _, err := json.Marshal(manager.GetConfig()); err != nil {
+			t.Errorf("encode snapshot: %v", err)
+		}
+	})
+	run(func(round int) {
+		id := fmt.Sprintf("host-%d", round%64)
+		manager.CheckHost(models.Host{ID: id, Hostname: id, Status: "online", CPUUsage: 50})
+	})
+	wg.Wait()
+
+	got := manager.GetConfig()
+	if got.TimeThresholds["guest"] != 0 {
+		t.Fatalf("live guest delay = %d after saves, want the configured 0", got.TimeThresholds["guest"])
+	}
+	gotJSON, err := json.Marshal(got)
+	if err != nil {
+		t.Fatalf("marshal final config: %v", err)
+	}
+	if string(gotJSON) != string(wantJSON) {
+		t.Fatalf("saving changed the live config:\n got %s\nwant %s", gotJSON, wantJSON)
+	}
+
+	loaded, err := persistence.LoadAlertConfig()
+	if err != nil {
+		t.Fatalf("LoadAlertConfig: %v", err)
+	}
+	if delay, ok := loaded.TimeThresholds["guest"]; !ok || delay != 0 {
+		t.Fatalf("persisted guest delay = %d (present %v), want the configured 0", delay, ok)
+	}
+}
+
+func TestSaveAlertConfigDoesNotNormalizeTheCallersConfig(t *testing.T) {
+	persistence := config.NewConfigPersistence(t.TempDir())
+	if err := persistence.EnsureConfigDir(); err != nil {
+		t.Fatalf("EnsureConfigDir: %v", err)
+	}
+
+	cfg := alerts.AlertConfig{
+		AgentDefaults: alerts.ThresholdConfig{
+			CPU: &alerts.HysteresisThreshold{Trigger: 80, Clear: 0},
+		},
+		TimeThresholds: map[string]int{"guest": -1, "node": 0},
+		Overrides: map[string]alerts.ThresholdConfig{
+			"storage:local": {Usage: &alerts.HysteresisThreshold{Trigger: 90, Clear: 0}},
+		},
+		BackupDefaults: alerts.BackupAlertConfig{IgnoreVMIDs: []string{" 100 ", "100"}},
+	}
+	before, err := json.Marshal(cfg)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	cpu, override := cfg.AgentDefaults.CPU, cfg.Overrides["storage:local"].Usage
+
+	if err := persistence.SaveAlertConfig(cfg); err != nil {
+		t.Fatalf("SaveAlertConfig: %v", err)
+	}
+
+	after, err := json.Marshal(cfg)
+	if err != nil {
+		t.Fatalf("marshal: %v", err)
+	}
+	if string(after) != string(before) {
+		t.Fatalf("SaveAlertConfig changed its argument:\n got %s\nwant %s", after, before)
+	}
+	if cfg.AgentDefaults.CPU != cpu || cfg.Overrides["storage:local"].Usage != override {
+		t.Fatal("SaveAlertConfig replaced the caller's threshold pointers")
+	}
+
+	loaded, err := persistence.LoadAlertConfig()
+	if err != nil {
+		t.Fatalf("LoadAlertConfig: %v", err)
+	}
+	if got := loaded.TimeThresholds; got["guest"] != 5 || got["node"] != 0 {
+		t.Fatalf("persisted delays = %v, want guest 5 (unset) and node 0 (no delay)", got)
+	}
+	if got := *loaded.AgentDefaults.CPU; got != (alerts.HysteresisThreshold{Trigger: 80, Clear: 75}) {
+		t.Fatalf("persisted agent CPU = %+v, want clear derived below the trigger", got)
+	}
+	if got := *loaded.Overrides["storage:local"].Usage; got != (alerts.HysteresisThreshold{Trigger: 90, Clear: 85}) {
+		t.Fatalf("persisted override usage = %+v, want clear derived below the trigger", got)
+	}
+	if want := []string{"100"}; !reflect.DeepEqual(loaded.BackupDefaults.IgnoreVMIDs, want) {
+		t.Fatalf("persisted ignore list = %v, want %v", loaded.BackupDefaults.IgnoreVMIDs, want)
+	}
+}
@@ -213,7 +213,7 @@ func TestLoadAlertConfig_Normalization(t *testing.T) {
 			},
 		},
 		{
-			name: "TimeThresholds defaults",
+			name: "TimeThresholds zero means no delay",
 			input: map[string]interface{}{
 				"timeThreshold": 0,
 				"timeThresholds": map[string]interface{}{
@@ -222,8 +222,9 @@ func TestLoadAlertConfig_Normalization(t *testing.T) {
 				},
 			},
 			verify: func(t *testing.T, cfg *alerts.AlertConfig) {
-				assert.Equal(t, 5, cfg.TimeThresholds["guest"])
-				assert.Equal(t, 5, cfg.TimeThresholds["all"])
+				assert.Equal(t, 0, cfg.TimeThresholds["guest"])
+				assert.Equal(t, 0, cfg.TimeThresholds["all"])
+				assert.Equal(t, 5, cfg.TimeThresholds["node"])
 			},
 		},
 		{
@@ -290,11 +291,13 @@ func TestLoadAlertConfig_Normalization(t *testing.T) {
 				"timeThresholds": map[string]interface{}{
 					"guest": -1,
 					"pbs":   0,
+					"all":   -1,
 				},
 			},
 			verify: func(t *testing.T, cfg *alerts.AlertConfig) {
 				assert.Equal(t, 5, cfg.TimeThresholds["guest"])
-				assert.Equal(t, 5, cfg.TimeThresholds["pbs"])
+				assert.Equal(t, 0, cfg.TimeThresholds["pbs"])
+				assert.Equal(t, 5, cfg.TimeThresholds["all"])
 			},
 		},
 		{
@@ -514,7 +514,7 @@ func TestLoadAlertConfigAppliesDefaults(t *testing.T) {

 	raw := alerts.AlertConfig{
 		Enabled:                        false,
-		TimeThresholds:                 map[string]int{"guest": 0, "node": 0},
+		TimeThresholds:                 map[string]int{"guest": -1, "node": 0},
 		DockerIgnoredContainerPrefixes: []string{" Runner "},
 		SnapshotDefaults: alerts.SnapshotAlertConfig{
 			Enabled:         true,
@@ -549,10 +549,10 @@ func TestLoadAlertConfigAppliesDefaults(t *testing.T) {
 	}

 	if got := loaded.TimeThresholds["guest"]; got != 5 {
-		t.Fatalf("expected guest threshold default 5, got %d", got)
+		t.Fatalf("expected unset guest threshold default 5, got %d", got)
 	}
-	if got := loaded.TimeThresholds["node"]; got != 5 {
-		t.Fatalf("expected node threshold default 5, got %d", got)
+	if got := loaded.TimeThresholds["node"]; got != 0 {
+		t.Fatalf("expected node threshold 0 (no delay) to be kept, got %d", got)
 	}
 	if loaded.NodeDefaults.Temperature == nil {
 		t.Fatalf("expected node temperature defaults to be set")