Keep Off alert defaults off after saving

Switching a Global Defaults metric Off on the thresholds page staged -1,
and the save payload passed it through as trigger -1. Alert config
normalization reads a negative default trigger as unset and restores the
factory threshold, so every Off default with a positive factory value came
back On after save and reload: agent CPU, memory, disk and disk
temperature, Proxmox node temperature, PBS, Docker, Kubernetes, TrueNAS,
vSphere and storage. Guest and node CPU, memory and disk escaped only
because nothing normalizes them.

The payload builder now writes every Off default as trigger 0, which the
engine and normalization both read as off. Overrides 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.

With -1 and 0 both live Off values, the Global Defaults Custom badge
compares Off values as equal, so the factory -1 I/O defaults no longer
read Custom against a saved 0 and tell users they changed something they
had not. A row's custom marker keeps comparing raw values: it drives the
row's revert control and override removal, so a stored -1 override must
stay visible against an Off default.

Contract-Neutral: Global Defaults Off now persists; the Off wire value and Custom badge rule are recorded in the alerts contract, and frontend-primitives gains no rule
This commit is contained in:
rcourtman
2026-10-07 16:05:38 +01:00
parent bab38a25b9
commit 564ea41a6e
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.
@@ -2597,8 +2618,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)
}
}
}