Keep Off alert defaults off after saving (#2684)

## Summary

Switching a metric Off in a **Global Defaults** row (Alerts >
Thresholds) did not stick. After Save and a reload the metric was back
On at its factory threshold and kept alerting. This affected every
section whose factory threshold is positive: Machines CPU, memory, disk
and disk temperature, Proxmox node temperature, PBS, Docker, Kubernetes,
TrueNAS, vSphere and storage.

The editors stage `-1` for Off, and the save payload passed it through
as `{trigger: -1}`. Backend config normalization reads a negative
default trigger as unset and restores the factory value. Guest and node
CPU, memory and disk escaped only because nothing normalizes them.

- `buildAlertsConfigurationPayload` now writes every Off default as
`{trigger: 0, clear: 0}`, the value the engine and normalization both
read as off. Per-resource overrides are serialized separately and keep
`-1`, which the override path stores as written. A `-1` already stored
on a guest or node default becomes `0` on the next save.
- The Global Defaults **Custom** badge now treats `-1` and `0` as the
same Off value. Without that, a fresh install showed Custom on
Kubernetes, TrueNAS Systems and vSphere, and VMs & Containers turned
Custom after any save, though nothing had changed. An unset factory
value stays distinct from Off: an unset TrueNAS disk temperature follows
Disk temperature by type.
- A row's custom-threshold marker still compares raw values. It drives
the row's revert control and override removal, so a stored `-1` override
must stay visible against an Off default. A test pins that.
- The alerts contract no longer claims Off is written as `-1`
everywhere.

## Verification

- New payload test: every section's Off default is written as `{0, 0}`
and reads back Off. It fails on `main` with `guestDefaults.cpu: expected
{ trigger: -1 }`.
- New backend test: a `{0, 0}` default stays off through `UpdateConfig`
in every section, and a host at 99% raises no CPU, memory or disk alert
when the agent defaults are off. Both fail when the defaults are written
as `-1`.
- Live on a private mock stack. On `main`, Proxmox CPU and Temp switched
Off saved as `{-1}` and `{80, 75}`, and after reload Temp read On. With
the fix:
- Machines CPU at 1% raised 8 alerts. Switching it Off stored `{0, 0}`
and resolved all 8.
  - Proxmox Temp Off resolved the node temperature alert.
  - Nothing re-fired over two minutes.
- Values stayed Off after a backend restart and a reload. Machines,
Proxmox, Kubernetes, TrueNAS and vSphere were each checked at desktop or
phone width.
- Custom badge on a fresh install: no Proxmox, Kubernetes, TrueNAS or
vSphere group reads Custom, and after an unrelated save only the group
actually changed does.
This commit is contained in:
rcourtman
2026-10-07 18:47:09 +01:00
committed by GitHub
8 changed files with 358 additions and 7 deletions
@@ -525,7 +525,7 @@ active-state and intent-policy linkage while persisting through the stable
A per-metric threshold is off whenever its trigger is `<= 0`. That boundary is
engine truth (`internal/alerts/canonical_metric.go`,
`internal/alerts/config_runtime.go`), not a display convention: `-1` is the
value Pulse writes, and `0` disables the metric just as completely because
value the editors stage, and `0` disables the metric just as completely because
earlier builds advertised `0` as the disable value and those overrides are still
on disk. Every threshold editor reads that same `<= 0` rule through
`frontend-modern/src/components/Alerts/alertResourceTableModel.ts` rather than
@@ -543,7 +543,28 @@ the metric's enabled default instead, and only clears the override when the
inherited default is already enabled. Editors must also not manufacture an off
state out of an empty input: a cleared box is mid-edit, and coercing it to `0`
disabled the metric in the engine while the row still showed On. Disabling is
the toggle's job, and the toggle writes the canonical `-1`.
the toggle's job, and the toggle stages the canonical `-1`.
The save path writes the backend's default contract, not the editors' sentinel.
Alert config normalization reads a negative trigger on a global default as
unset and restores the factory threshold, so `buildAlertsConfigurationPayload`
in `frontend-modern/src/features/alerts/alertsConfigurationModel.ts` writes
every Off global default as trigger `0`. Sending the staged `-1` through made
every Off default with a positive factory threshold come back On at that
threshold after save and reload; only guest and node CPU, memory and disk
escaped, because normalization leaves them alone. Overrides keep `-1`, which
the override path stores as written. Because `-1` and `0` are the same
setting, the Global Defaults "Custom" badge compares Off values as equal
rather than by raw number, so the factory `-1` I/O defaults do not read
Custom against a saved `0`. An unset factory value stays distinct from Off,
since unset can mean the default follows another setting. A row's
custom-threshold marker keeps comparing raw values: it drives the row's revert
control and override removal, so an explicit `-1` override must stay visible
against an Off default even though both alert the same way.
`frontend-modern/src/features/alerts/__tests__/alertsConfigurationModel.test.ts`
pins the payload for every section, and
`internal/alerts/config_defaults_off_test.go` pins that a zero default stays
off through `UpdateConfig` and raises no alert.
External availability-probe reporting loss owns one canonical
`external-probe-unavailable` alert per assigned agent, not one alert per target.
@@ -2650,8 +2671,10 @@ Metric enablement is an explicit On/Off interaction in row, global-default,
mobile, and bulk editors. Enabled numeric inputs accept positive trigger values
only and user-facing copy must not expose the persisted disable sentinel.
Internally, the canonical `<= 0` read rule remains intact for legacy data, while
new Off actions write `-1`, so existing configuration and the API contract
continue to round-trip without making `0` a second customer-facing disable path.
new Off actions stage `-1` (saved as trigger `0` on global defaults, which the
backend otherwise resets to factory), so existing configuration and the API
contract continue to round-trip without making `0` a second customer-facing
disable path.
Within the Proxmox tab, render-heavy ownership now further routes through
`frontend-modern/src/components/Alerts/ThresholdsTableProxmoxNodesSection.tsx`,
`frontend-modern/src/components/Alerts/ThresholdsTableProxmoxPBSSection.tsx`,
@@ -0,0 +1,55 @@
{
"version": 1,
"base_sha": "bab38a25b92684c978c4273200765152040624ed",
"verified_at": "2026-10-07T15:05:37Z",
"result": "passed",
"changed_paths": [
"frontend-modern/src/components/Alerts/alertResourceTableModel.ts",
"frontend-modern/src/features/alerts/alertsConfigurationModel.ts"
],
"content_sha256": {
"frontend-modern/src/components/Alerts/alertResourceTableModel.ts": "9d2cbc37fe80492a67b54b106a1c413ea1cbca66b1cd30e1c100d1c90a408eea",
"frontend-modern/src/features/alerts/alertsConfigurationModel.ts": "3070cf7df162eefd5653e5a6eb0a0722e2ffa04b33b12a81ebb2f38a4035c62f"
},
"routes": [
"/alerts/thresholds/proxmox",
"/alerts/thresholds/systems",
"/alerts/thresholds/kubernetes",
"/alerts/thresholds/truenas",
"/alerts/thresholds/vsphere",
"/alerts/thresholds/docker"
],
"viewports": [
{
"width": 1280,
"height": 900
},
{
"width": 1024,
"height": 768
},
{
"width": 390,
"height": 844
},
{
"width": 375,
"height": 812
}
],
"states": [
"Private mock stack (PULSE_MOCK_MODE: 3 PVE nodes, 3 agents, 1 Kubernetes cluster, TrueNAS, vSphere) on a backend built from bab38a25b9, Vite on this tree, alerts activated, fresh data dir per run, no injected data",
"Before (origin/main payload builder): Proxmox Global Defaults CPU and Temp switched Off and saved; the API stored nodeDefaults.cpu {-1, 0} and nodeDefaults.temperature {80, 75}; after reload Temp read On 80",
"Final content, 1280 wide: Machines CPU Off saved as agentDefaults.cpu {0, 0} and reads Off after reload with the Custom badge (factory is 80)",
"Final content, 390 wide: Kubernetes Clusters Memory Off saved as kubernetesDefaults.memory {0, 0} and reads Off after reload",
"Same payload change, earlier passes: Proxmox Temp Off saved {0, 0} and reads Off after reload (1280); TrueNAS Systems CPU and vSphere Hosts CPU Off saved {0, 0} and read Off (390); Machines CPU at 1% raised 8 agent CPU alerts, switching it Off resolved all 8, Proxmox Temp Off resolved the node temperature alert, nothing re-fired over two minutes, values stayed Off across a backend restart (1024 and 375)",
"Custom badge on a fresh install: no Proxmox, Kubernetes, TrueNAS or vSphere Global Defaults group reads Custom (on the payload-only build Kubernetes, TrueNAS Systems and vSphere Hosts/VMs did); after an unrelated save (Proxmox node CPU 81) only Virtualization Hosts reads Custom and VMs & Containers stays clean",
"Docker Containers still reads Custom on a fresh install because the backend's no-alerts.json fallback stores Docker CPU/memory/disk as {0, 0}; separate pre-existing defect, not touched here",
"No page errors in any Playwright run"
],
"interactions": [
"Playwright: Expand all, clicked a Global Defaults On/Off badge, Save Changes, read GET /api/alerts/config, reloaded and re-read badge states and the Custom badge (Machines CPU at 1280x900, Kubernetes Memory at 390x844)",
"Playwright: recorded Custom badges on Proxmox, Kubernetes, TrueNAS, vSphere and Docker on a fresh install, changed Proxmox node CPU to 81, saved, recorded them again",
"Browser pane and Playwright (earlier passes on the same payload change): Machines CPU 1% then Off with /api/alerts/active reads, backend restart, Proxmox Temp, TrueNAS and vSphere CPU Off at phone width"
]
}
@@ -116,6 +116,30 @@ describe('hasCustomAlertResourceGlobalDefaults', () => {
).toBe(true);
});
it('reads an Off default saved as 0 as the factory Off of -1', () => {
expect(
hasCustomAlertResourceGlobalDefaults(
{ cpu: 80, diskRead: 0, networkIn: 0 },
{ cpu: 80, diskRead: -1, networkIn: -1 },
),
).toBe(false);
expect(hasCustomAlertResourceGlobalDefaults({ diskRead: 100 }, { diskRead: -1 })).toBe(true);
});
it('keeps an Off default custom against an unset factory value', () => {
// An unset factory value follows another setting (TrueNAS disks follow Disk
// temperature by type), so Off is a choice that needs its reset control.
expect(
hasCustomAlertResourceGlobalDefaults({ temperature: 0 }, { temperature: undefined }),
).toBe(true);
expect(
hasCustomAlertResourceGlobalDefaults({ temperature: -1 }, { temperature: undefined }),
).toBe(true);
expect(
hasCustomAlertResourceGlobalDefaults({ temperature: undefined }, { temperature: undefined }),
).toBe(false);
});
it('returns false when a factory key is absent from globalDefaults (current is undefined)', () => {
expect(hasCustomAlertResourceGlobalDefaults({ memory: 90 }, { cpu: 80, memory: 90 })).toBe(
false,
@@ -101,7 +101,7 @@ export function hasCustomAlertResourceGlobalDefaults(
return Object.keys(factoryDefaults).some((key) => {
const current = globalDefaults[key];
const factory = factoryDefaults[key];
return current !== undefined && current !== factory;
return current !== undefined && !alertResourceThresholdsEquivalent(current, factory);
});
}
@@ -208,12 +208,26 @@ export const ALERT_RESOURCE_METRIC_OFF_VALUE = -1;
* The alert engine disables a metric whenever its trigger is `<= 0`
* (`internal/alerts/canonical_metric.go`), so every editor surface has to read
* `0` as Off too: older builds advertised `0` as the disable value and those
* overrides are still on disk. New writes always use the canonical `-1`.
* overrides are still on disk. Editors always stage the canonical `-1`; the
* save payload writes an Off global default as `0`, because the backend reads
* a negative default as unset.
*/
export function isAlertResourceMetricOff(value: number | undefined | null): boolean {
return typeof value === 'number' && Number.isFinite(value) && value <= 0;
}
/**
* Whether two thresholds alert the same way. Every value `<= 0` is Off, and a
* global default saves Off as `0` while the editors and factory defaults use
* `-1`, so the two compare equal.
*/
export function alertResourceThresholdsEquivalent(
left: number | undefined,
right: number | undefined,
): boolean {
return left === right || (isAlertResourceMetricOff(left) && isAlertResourceMetricOff(right));
}
/**
* Value an override editor must stage when a metric is switched back on.
* Clearing the override (`undefined`) only re-inherits the default, so it
@@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest';
import { ALERT_RESOURCE_METRIC_OFF_VALUE } from '@/components/Alerts/alertResourceTableModel';
import type { AlertConfig } from '@/types/alerts';
import {
@@ -224,4 +225,94 @@ describe('alertsConfigurationModel', () => {
expect(result.alertConfig).toBeUndefined();
expect(result.dockerValidationError).toBe(ALERT_DOCKER_GAP_VALIDATION_ERROR);
});
// The backend reads a negative default trigger as unset and restores the
// factory threshold, so every Global Defaults Off must reach it as 0.
it('writes an Off global default as trigger 0 in every section and reads it back Off', () => {
const thresholdMetrics = {
guestDefaults: ['cpu', 'memory', 'disk', 'diskRead', 'diskWrite', 'networkIn', 'networkOut'],
nodeDefaults: ['cpu', 'memory', 'disk', 'temperature'],
agentDefaults: ['cpu', 'memory', 'disk', 'diskTemperature'],
pbsDefaults: ['cpu', 'memory'],
kubernetesDefaults: [
'cpu',
'memory',
'disk',
'diskRead',
'diskWrite',
'networkIn',
'networkOut',
],
trueNASDefaults: [
'cpu',
'memory',
'disk',
'usage',
'temperature',
'diskRead',
'diskWrite',
'networkIn',
'networkOut',
],
trueNASDiskDefaults: ['temperature'],
vmwareDefaults: [
'cpu',
'memory',
'disk',
'usage',
'diskRead',
'diskWrite',
'networkIn',
'networkOut',
],
dockerDefaults: ['cpu', 'memory', 'disk'],
} as const;
const payloadSection = {
guestDefaults: 'guestDefaults',
nodeDefaults: 'nodeDefaults',
agentDefaults: 'agentDefaults',
pbsDefaults: 'pbsDefaults',
kubernetesDefaults: 'kubernetesDefaults',
trueNASDefaults: 'truenasDefaults',
trueNASDiskDefaults: 'truenasDiskDefaults',
vmwareDefaults: 'vmwareDefaults',
dockerDefaults: 'dockerDefaults',
} as const;
type Section = keyof typeof thresholdMetrics;
const snapshot = createDefaultAlertsConfigurationSnapshot();
for (const [section, metrics] of Object.entries(thresholdMetrics)) {
const defaults: Record<string, unknown> = snapshot[section as Section];
for (const metric of metrics) {
defaults[metric] = ALERT_RESOURCE_METRIC_OFF_VALUE;
}
}
snapshot.storageDefault = ALERT_RESOURCE_METRIC_OFF_VALUE;
const { alertConfig } = buildAlertsConfigurationPayload({
snapshot,
rawOverridesConfig: {},
alertsActivationState: null,
alertsActivationConfig: null,
});
expect(alertConfig).toBeTruthy();
const payload = alertConfig as unknown as Record<string, Record<string, unknown>>;
for (const [section, metrics] of Object.entries(thresholdMetrics)) {
const written = payload[payloadSection[section as Section]];
for (const metric of metrics) {
expect(written[metric], `${section}.${metric}`).toEqual({ trigger: 0, clear: 0 });
}
}
expect(alertConfig?.storageDefault).toEqual({ trigger: 0, clear: 0 });
const reloaded = readAlertsConfigurationSnapshot(alertConfig as AlertConfig);
for (const [section, metrics] of Object.entries(thresholdMetrics)) {
const defaults: Record<string, unknown> = reloaded[section as Section];
for (const metric of metrics) {
expect(defaults[metric], `${section}.${metric}`).toBe(0);
}
}
expect(reloaded.storageDefault).toBe(0);
});
});
@@ -174,8 +174,11 @@ const cloneBackupDefaults = (backupDefaults: BackupAlertConfig): BackupAlertConf
ignoreVMIDs: [...(backupDefaults.ignoreVMIDs ?? [])],
});
// Off is written as 0. The editors stage -1 for Off, but the backend reads a
// negative default trigger as unset and restores the factory threshold, so a
// saved -1 came back On.
const createHysteresisThreshold = (trigger: number | undefined, clearMargin = 5) => {
const normalized = typeof trigger === 'number' ? trigger : 0;
const normalized = typeof trigger === 'number' && trigger > 0 ? trigger : 0;
return {
trigger: normalized,
clear: Math.max(0, normalized - clearMargin),
@@ -48,6 +48,14 @@ describe('thresholdsResourceModel branch coverage 0723pm', () => {
expect(hasThresholdDiff(override, { cpu: 80, memory: 70 })).toBe(false);
});
it('keeps an explicit Off override visible against an Off default saved as 0', () => {
// The marker drives the row's revert control and override removal, so a
// stored -1 must not hide behind an Off default: switching the default
// back on would otherwise leave this resource silently off.
const override = makeOverride('a', { cpu: -1 });
expect(hasThresholdDiff(override, { cpu: 0 })).toBe(true);
});
it('returns true when one threshold diverges while a sibling matches', () => {
const override = makeOverride('a', { cpu: 80, memory: 90 });
expect(hasThresholdDiff(override, { cpu: 80, memory: 70 })).toBe(true);
+133
View File
@@ -0,0 +1,133 @@
package alerts
import (
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
)
// globalDefaultThresholds lists every pointer threshold the thresholds page
// edits in a Global Defaults row.
func globalDefaultThresholds(config *AlertConfig) map[string]**HysteresisThreshold {
return map[string]**HysteresisThreshold{
"guest.cpu": &config.GuestDefaults.CPU,
"guest.memory": &config.GuestDefaults.Memory,
"guest.disk": &config.GuestDefaults.Disk,
"guest.diskRead": &config.GuestDefaults.DiskRead,
"guest.diskWrite": &config.GuestDefaults.DiskWrite,
"guest.networkIn": &config.GuestDefaults.NetworkIn,
"guest.networkOut": &config.GuestDefaults.NetworkOut,
"node.cpu": &config.NodeDefaults.CPU,
"node.memory": &config.NodeDefaults.Memory,
"node.disk": &config.NodeDefaults.Disk,
"node.temperature": &config.NodeDefaults.Temperature,
"agent.cpu": &config.AgentDefaults.CPU,
"agent.memory": &config.AgentDefaults.Memory,
"agent.disk": &config.AgentDefaults.Disk,
"agent.diskTemperature": &config.AgentDefaults.DiskTemperature,
"pbs.cpu": &config.PBSDefaults.CPU,
"pbs.memory": &config.PBSDefaults.Memory,
"kubernetes.cpu": &config.KubernetesDefaults.CPU,
"kubernetes.memory": &config.KubernetesDefaults.Memory,
"kubernetes.disk": &config.KubernetesDefaults.Disk,
"kubernetes.diskRead": &config.KubernetesDefaults.DiskRead,
"kubernetes.diskWrite": &config.KubernetesDefaults.DiskWrite,
"kubernetes.networkIn": &config.KubernetesDefaults.NetworkIn,
"kubernetes.networkOut": &config.KubernetesDefaults.NetworkOut,
"truenas.cpu": &config.TrueNASDefaults.CPU,
"truenas.memory": &config.TrueNASDefaults.Memory,
"truenas.disk": &config.TrueNASDefaults.Disk,
"truenas.usage": &config.TrueNASDefaults.Usage,
"truenas.temperature": &config.TrueNASDefaults.Temperature,
"truenas.diskRead": &config.TrueNASDefaults.DiskRead,
"truenas.diskWrite": &config.TrueNASDefaults.DiskWrite,
"truenas.networkIn": &config.TrueNASDefaults.NetworkIn,
"truenas.networkOut": &config.TrueNASDefaults.NetworkOut,
"truenas.disk.temperature": &config.TrueNASDiskDefaults.Temperature,
"vmware.cpu": &config.VMwareDefaults.CPU,
"vmware.memory": &config.VMwareDefaults.Memory,
"vmware.disk": &config.VMwareDefaults.Disk,
"vmware.usage": &config.VMwareDefaults.Usage,
"vmware.diskRead": &config.VMwareDefaults.DiskRead,
"vmware.diskWrite": &config.VMwareDefaults.DiskWrite,
"vmware.networkIn": &config.VMwareDefaults.NetworkIn,
"vmware.networkOut": &config.VMwareDefaults.NetworkOut,
}
}
// The thresholds page saves a Global Defaults Off as trigger 0. Normalization
// reads a negative trigger as unset and restores the factory threshold, so 0
// is the value that has to survive it in every section.
func TestUpdateConfigKeepsZeroTriggerGlobalDefaultsOff(t *testing.T) {
m := newTestManager(t)
config := m.GetConfig()
for _, field := range globalDefaultThresholds(&config) {
*field = &HysteresisThreshold{Trigger: 0, Clear: 0}
}
config.DockerDefaults.CPU = HysteresisThreshold{Trigger: 0, Clear: 0}
config.DockerDefaults.Memory = HysteresisThreshold{Trigger: 0, Clear: 0}
config.DockerDefaults.Disk = HysteresisThreshold{Trigger: 0, Clear: 0}
config.StorageDefault = HysteresisThreshold{Trigger: 0, Clear: 0}
m.UpdateConfig(config)
got := m.GetConfig()
for name, field := range globalDefaultThresholds(&got) {
if *field == nil || **field != (HysteresisThreshold{Trigger: 0, Clear: 0}) {
t.Errorf("%s = %+v after UpdateConfig, want trigger 0 (off)", name, *field)
}
}
for name, threshold := range map[string]HysteresisThreshold{
"docker.cpu": got.DockerDefaults.CPU,
"docker.memory": got.DockerDefaults.Memory,
"docker.disk": got.DockerDefaults.Disk,
"storage": got.StorageDefault,
} {
if threshold != (HysteresisThreshold{Trigger: 0, Clear: 0}) {
t.Errorf("%s = %+v after UpdateConfig, want trigger 0 (off)", name, threshold)
}
}
}
func TestCheckHostRaisesNoMetricAlertForAgentDefaultsSavedOff(t *testing.T) {
m := newTestManager(t)
m.ClearActiveAlerts()
config := m.GetConfig()
config.AgentDefaults.CPU = &HysteresisThreshold{Trigger: 0, Clear: 0}
config.AgentDefaults.Memory = &HysteresisThreshold{Trigger: 0, Clear: 0}
config.AgentDefaults.Disk = &HysteresisThreshold{Trigger: 0, Clear: 0}
m.UpdateConfig(config)
// UpdateConfig restores the default delay; without one a breach fires on
// the first check, so a missing alert means the metric is off.
m.mu.Lock()
m.config.TimeThresholds = map[string]int{}
m.mu.Unlock()
host := models.Host{
ID: "host-off",
DisplayName: "Off Host",
Hostname: "host-off.example",
Platform: "linux",
CPUUsage: 99,
CPUCount: 8,
Memory: models.Memory{Usage: 99, Total: 16384, Used: 16220, Free: 164},
Disks: []models.Disk{{Mountpoint: "/", Usage: 99, Total: 100, Used: 99, Free: 1}},
Status: "online",
IntervalSeconds: 30,
LastSeen: time.Now(),
}
m.CheckHost(host)
resourceID := hostResourceID(host.ID)
diskResourceID, _ := hostDiskResourceID(host, host.Disks[0])
for _, alertID := range []string{
canonicalMetricStateID(resourceID, "cpu"),
canonicalMetricStateID(resourceID, "memory"),
canonicalMetricStateID(diskResourceID, "disk"),
} {
if _, exists := testLookupActiveAlert(t, m, alertID); exists {
t.Errorf("alert %q fired although its agent default was saved off", alertID)
}
}
}