mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Replace stored alerts with each payload instead of merging
The websocket store wrote every incoming active alert with setActiveAlerts(id, alert). Solid's path setter shallow-merges into an existing object, so a field the server stops sending kept its old value in the browser. After a backend restart, threshold alerts are restored without metricStatus until their next evaluation, but an open tab kept the pre-restart live reading and the node drawer and node table went on saying "78°C now" for a reading the server no longer reports. An unacknowledge from another session left ackTime and ackUser behind the same way, as did a dropped nodeDisplayName or metadata. Active and recently resolved alerts are now written with reconcile, so omitted keys are removed on the snapshot, keyed-delta and REST recovery commits alike. reconcile diffs nested values in place, so an unchanged alert's live status no longer re-notifies its readers on every frame, which the shallow merge did by swapping in a fresh object each time. The pending acknowledgement hold is unchanged: it still skips payloads that contradict an unconfirmed local acknowledge or unacknowledge. updateAlert now stores a copy of each local update. Because payloads reconcile the stored alert in place, a snooze's optimistic record, which shares nested objects with the record the caller keeps for rollback, would otherwise have that rollback record rewritten by the next payload.
This commit is contained in:
rcourtman
@@ -787,6 +787,18 @@ that fast path entirely: active-alert and resolved-alert stores keep their
|
||||
own commit path, resource `alerts` facets are not in the fast-path
|
||||
allow-list, and a row whose patch touches alert-relevant structure always
|
||||
takes the full canonical merge.
|
||||
That commit path replaces each stored alert with the server's payload for its
|
||||
id instead of merging into it. A field the server omits is removed from the
|
||||
store, whether the payload came from a socket snapshot, a keyed delta, or REST
|
||||
recovery: a threshold alert restored after a restart carries no `metricStatus`
|
||||
until its next evaluation, so the previous live reading must not keep reading
|
||||
as "now"; an unacknowledge from another session clears `ackTime` and
|
||||
`ackUser`; a dropped `nodeDisplayName` or `metadata` disappears. Resolved
|
||||
alerts follow the same rule. The pending acknowledgement hold still skips a
|
||||
payload that contradicts an unconfirmed local acknowledge or unacknowledge,
|
||||
and the confirming payload then replaces the optimistic copy. Because a
|
||||
replacement reconciles the stored alert in place, local optimistic updates are
|
||||
stored as copies, so it never rewrites a record a caller keeps for rollback.
|
||||
Operational evidence and lifecycle identity are typed through
|
||||
`internal/operationaltrust`. Evidence envelopes distinguish completeness,
|
||||
confidence, permissions, freshness, correlation, and bounded provider detail.
|
||||
|
||||
@@ -673,7 +673,13 @@ the store with reference-stable untouched items so the per-tick deep walk of
|
||||
the whole projection is gone. Active alerts travel the same keyed transport
|
||||
(`activeAlertsDelta`, replacing the ~37KB whole-array re-ship whenever any
|
||||
alert changed) while their application stays immediate per the alerts
|
||||
subsystem boundary. These projection baselines are socket-owned: an oversized
|
||||
subsystem boundary. Each committed alert is reconciled in place against its
|
||||
copy in the keyed active-alert store, so a reader of an unchanged alert there,
|
||||
or of an unchanged nested value such as its live `metricStatus` or `metadata`,
|
||||
is not notified although every commit carries freshly parsed objects; omitted
|
||||
fields are deleted rather than shallow-merged. The `state.activeAlerts` array
|
||||
is still replaced on every alert commit. These projection baselines are
|
||||
socket-owned: an oversized
|
||||
frame invalidates resources, connected infrastructure, and active alerts
|
||||
together, while REST recovery hydrates display state only. A later keyed delta
|
||||
without a socket baseline requests the throttled recovery path instead of
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
{
|
||||
"version": 1,
|
||||
"base_sha": "8424fd65057af75e1ea3c1fc9fe29528d7c0d68e",
|
||||
"verified_at": "2026-10-06T21:24:22Z",
|
||||
"result": "passed",
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/stores/websocket.ts"
|
||||
],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/stores/websocket.ts": "1be5c0f23d3dbaf85a32f7b8b3906c2cb25b20bb02412c31c200ffbad9c6c2d5"
|
||||
},
|
||||
"routes": [
|
||||
"/proxmox/overview",
|
||||
"/alerts/overview"
|
||||
],
|
||||
"viewports": [
|
||||
{
|
||||
"width": 1280,
|
||||
"height": 900
|
||||
},
|
||||
{
|
||||
"width": 375,
|
||||
"height": 812
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"Private mock stack (backend + Vite from this tree) with a local-only overlay pinning node standalone2's evaluated CPU temperature and letting mock mode restore persisted alerts across restarts",
|
||||
"standalone2 temperature alert latched below trigger (breach 95, live 78): node drawer Needs attention reads 'Temperature 78\u00b0C now, back under the 80\u00b0C alert level' and the node table temperature cell title carries the same summary",
|
||||
"Backend restarted with the reading missing: /api/alerts/active returns the restored alert without metricStatus; the open tab reconnects and the drawer and cell title fall back to 'Node temperature at 95.0\u00b0C' within seconds",
|
||||
"Reading returns: the next keyed delta adds metricStatus and the drawer reads '78\u00b0C now' again",
|
||||
"Control on the unchanged store, same run: after the restart the open tab kept 'Temperature 78\u00b0C now' in the drawer and cell title for the full two-minute window",
|
||||
"Alerts overview card snoozed ('Snoozed until ...', Resume shown), resumed, and acknowledged from the UI with no page errors"
|
||||
],
|
||||
"interactions": [
|
||||
"Expanded the standalone2 row details in the Proxmox nodes table (desktop) and after Show all nodes (narrow)",
|
||||
"Restarted the private backend while the tab stayed open, then changed the pinned reading back to 78",
|
||||
"Clicked Snooze and the first preset, then Resume, then Acknowledge on the alert card",
|
||||
"Acknowledged and unacknowledged through the API from another session with the tab open (card left and returned within 2s)"
|
||||
]
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createRoot } from 'solid-js';
|
||||
import { createComputed, createRoot } from 'solid-js';
|
||||
|
||||
// Mirrors MAX_INBOUND_WEBSOCKET_MESSAGE_BYTES in the store under test.
|
||||
const MAX_INBOUND_WEBSOCKET_MESSAGE_BYTES = 32 * 1024 * 1024;
|
||||
@@ -1291,6 +1291,325 @@ describe('websocket store unified resource contract', () => {
|
||||
}
|
||||
});
|
||||
|
||||
describe('active alert payloads replace the stored alert', () => {
|
||||
// A threshold alert held below its trigger, as the socket sends it: the
|
||||
// breach stays in value/message and the reading evaluated now rides in
|
||||
// metricStatus.
|
||||
const heldTemperatureAlert = (): Record<string, unknown> => ({
|
||||
id: 'pve2-temperature',
|
||||
type: 'temperature',
|
||||
level: 'warning',
|
||||
resourceId: 'node:pve2',
|
||||
resourceName: 'pve2',
|
||||
node: 'pve2',
|
||||
nodeDisplayName: 'PVE Two',
|
||||
instance: 'pve2',
|
||||
message: 'Node temperature at 95°C',
|
||||
value: 95,
|
||||
threshold: 80,
|
||||
startTime: '2026-10-06T10:00:00Z',
|
||||
acknowledged: false,
|
||||
metadata: { resourceType: 'Node', unit: '°C', probe: 'cpu.package' },
|
||||
metricStatus: {
|
||||
phase: 'latched',
|
||||
value: 78,
|
||||
observedAt: '2026-10-06T10:05:00Z',
|
||||
trigger: 80,
|
||||
recovery: 75,
|
||||
unit: '°C',
|
||||
},
|
||||
});
|
||||
|
||||
// The same alert as the backend restores it after a restart: no live
|
||||
// status until the next evaluation.
|
||||
const restoredTemperatureAlert = (): Record<string, unknown> => {
|
||||
const alert = heldTemperatureAlert();
|
||||
delete alert.metricStatus;
|
||||
return alert;
|
||||
};
|
||||
|
||||
const emitActiveAlerts = (lastUpdate: number, activeAlerts: unknown[]) =>
|
||||
emitMessage({ type: 'rawData', data: { lastUpdate, activeAlerts } });
|
||||
|
||||
const emitInitialAlerts = (activeAlerts: unknown[]) =>
|
||||
emitMessage({
|
||||
type: 'initialState',
|
||||
data: { resources: [], lastUpdate: 100, activeAlerts, recentlyResolved: [] },
|
||||
});
|
||||
|
||||
it('drops a live status, display name and metadata that a snapshot omits', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
emitInitialAlerts([heldTemperatureAlert()]);
|
||||
expect(store.activeAlerts['pve2-temperature']?.metricStatus?.value).toBe(78);
|
||||
|
||||
emitActiveAlerts(200, [restoredTemperatureAlert()]);
|
||||
const restored = store.activeAlerts['pve2-temperature'];
|
||||
expect(restored).not.toHaveProperty('metricStatus');
|
||||
expect(restored?.value).toBe(95);
|
||||
expect(store.state.activeAlerts[0]).not.toHaveProperty('metricStatus');
|
||||
|
||||
const renamed = restoredTemperatureAlert();
|
||||
delete renamed.nodeDisplayName;
|
||||
renamed.metadata = { resourceType: 'Node', unit: '°C' };
|
||||
emitActiveAlerts(300, [renamed]);
|
||||
expect(store.activeAlerts['pve2-temperature']).not.toHaveProperty('nodeDisplayName');
|
||||
expect(store.activeAlerts['pve2-temperature']?.metadata).toEqual({
|
||||
resourceType: 'Node',
|
||||
unit: '°C',
|
||||
});
|
||||
|
||||
delete renamed.metadata;
|
||||
emitActiveAlerts(400, [renamed]);
|
||||
expect(store.activeAlerts['pve2-temperature']).not.toHaveProperty('metadata');
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('drops a live status that a keyed delta deletes', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
emitInitialAlerts([heldTemperatureAlert()]);
|
||||
|
||||
emitMessage({
|
||||
type: 'rawData',
|
||||
data: {
|
||||
lastUpdate: 200,
|
||||
activeAlertsDelta: {
|
||||
upserts: [{ id: 'pve2-temperature', metricStatus: null, nodeDisplayName: null }],
|
||||
},
|
||||
},
|
||||
});
|
||||
const stored = store.activeAlerts['pve2-temperature'];
|
||||
expect(stored).not.toHaveProperty('metricStatus');
|
||||
expect(stored).not.toHaveProperty('nodeDisplayName');
|
||||
expect(stored?.message).toBe('Node temperature at 95°C');
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('drops a live status that REST recovery omits', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
emitInitialAlerts([heldTemperatureAlert()]);
|
||||
|
||||
apiFetchJSONMock.mockResolvedValueOnce([restoredTemperatureAlert()]);
|
||||
await expect(store.refreshActiveAlerts()).resolves.toBe(true);
|
||||
expect(apiFetchJSONMock).toHaveBeenCalledWith('/api/alerts/active');
|
||||
expect(store.activeAlerts['pve2-temperature']).not.toHaveProperty('metricStatus');
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('clears acknowledgement fields when the server reports an unacknowledge', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
emitInitialAlerts([
|
||||
{
|
||||
...restoredTemperatureAlert(),
|
||||
acknowledged: true,
|
||||
ackTime: '2026-10-06T10:06:00Z',
|
||||
ackUser: 'admin',
|
||||
},
|
||||
]);
|
||||
expect(store.activeAlerts['pve2-temperature']?.ackUser).toBe('admin');
|
||||
|
||||
// Unacknowledged from another session: the payload omits both fields.
|
||||
emitActiveAlerts(200, [restoredTemperatureAlert()]);
|
||||
const stored = store.activeAlerts['pve2-temperature'];
|
||||
expect(stored?.acknowledged).toBe(false);
|
||||
expect(stored).not.toHaveProperty('ackTime');
|
||||
expect(stored).not.toHaveProperty('ackUser');
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('holds a local acknowledge and unacknowledge until the server confirms each', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
const id = 'pve2-temperature';
|
||||
const acknowledged = {
|
||||
...restoredTemperatureAlert(),
|
||||
acknowledged: true,
|
||||
ackTime: '2026-10-06T10:06:01Z',
|
||||
ackUser: 'admin',
|
||||
};
|
||||
emitInitialAlerts([restoredTemperatureAlert()]);
|
||||
|
||||
store.updateAlert(id, {
|
||||
acknowledged: true,
|
||||
ackTime: '2026-10-06T10:06:00Z',
|
||||
ackUser: undefined,
|
||||
});
|
||||
emitActiveAlerts(200, [restoredTemperatureAlert()]);
|
||||
expect(store.activeAlerts[id]?.acknowledged).toBe(true);
|
||||
|
||||
emitActiveAlerts(300, [acknowledged]);
|
||||
expect(store.activeAlerts[id]).toMatchObject({
|
||||
acknowledged: true,
|
||||
ackTime: '2026-10-06T10:06:01Z',
|
||||
ackUser: 'admin',
|
||||
});
|
||||
|
||||
store.updateAlert(id, { acknowledged: false, ackTime: undefined, ackUser: undefined });
|
||||
emitActiveAlerts(400, [acknowledged]);
|
||||
expect(store.activeAlerts[id]?.acknowledged).toBe(false);
|
||||
expect(store.activeAlerts[id]).not.toHaveProperty('ackUser');
|
||||
|
||||
emitActiveAlerts(500, [restoredTemperatureAlert()]);
|
||||
expect(store.activeAlerts[id]?.acknowledged).toBe(false);
|
||||
expect(store.activeAlerts[id]).not.toHaveProperty('ackTime');
|
||||
expect(store.activeAlerts[id]).not.toHaveProperty('ackUser');
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the record a local update replaced intact for rollback', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
const id = 'pve2-temperature';
|
||||
const record = {
|
||||
id: 'alert:pve2-temperature',
|
||||
canonicalSpecId: 'temperature',
|
||||
subjectResourceId: 'node:pve2',
|
||||
state: 'open',
|
||||
severity: 'warning',
|
||||
firstObservedAt: '2026-10-06T10:00:00Z',
|
||||
lastObservedAt: '2026-10-06T10:05:00Z',
|
||||
stateChangedAt: '2026-10-06T10:00:00Z',
|
||||
acknowledgement: { at: '2026-10-06T10:01:00Z', by: 'admin' },
|
||||
evidenceIds: ['evidence-1'],
|
||||
causeKey: 'pve2-temperature',
|
||||
relatedResourceIds: [],
|
||||
};
|
||||
emitInitialAlerts([{ ...restoredTemperatureAlert(), operationalRecord: record }]);
|
||||
|
||||
// As a snooze does: the optimistic record reuses nested values of the
|
||||
// record it keeps for rollback.
|
||||
const previous = store.activeAlerts[id]?.operationalRecord;
|
||||
if (!previous) throw new Error('expected an operational record');
|
||||
store.updateAlert(id, {
|
||||
operationalRecord: {
|
||||
...previous,
|
||||
state: 'suppressed',
|
||||
suppression: {
|
||||
at: '2026-10-06T10:06:00Z',
|
||||
by: 'current-user',
|
||||
reason: 'user_snooze',
|
||||
expiresAt: '2026-10-06T11:06:00Z',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
emitActiveAlerts(200, [
|
||||
{
|
||||
...restoredTemperatureAlert(),
|
||||
operationalRecord: {
|
||||
...record,
|
||||
acknowledgement: { at: '2026-10-06T10:07:00Z', by: 'operator' },
|
||||
evidenceIds: ['evidence-1', 'evidence-2'],
|
||||
},
|
||||
},
|
||||
]);
|
||||
expect(store.activeAlerts[id]?.operationalRecord?.acknowledgement?.by).toBe('operator');
|
||||
expect(previous.acknowledgement).toEqual({ at: '2026-10-06T10:01:00Z', by: 'admin' });
|
||||
expect([...previous.evidenceIds]).toEqual(['evidence-1']);
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('leaves unchanged alerts and unchanged nested fields quiet', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
let disposeTracking = () => {};
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
const cpuAlert = (): Record<string, unknown> => ({
|
||||
...heldTemperatureAlert(),
|
||||
id: 'pve1-cpu',
|
||||
type: 'cpu',
|
||||
resourceId: 'node:pve1',
|
||||
metricStatus: {
|
||||
phase: 'breaching',
|
||||
value: 93,
|
||||
observedAt: '2026-10-06T10:05:00Z',
|
||||
trigger: 90,
|
||||
recovery: 85,
|
||||
},
|
||||
});
|
||||
emitInitialAlerts([heldTemperatureAlert(), cpuAlert()]);
|
||||
|
||||
let temperatureRuns = 0;
|
||||
let cpuRuns = 0;
|
||||
createRoot((d) => {
|
||||
disposeTracking = d;
|
||||
createComputed(() => {
|
||||
void store.activeAlerts['pve2-temperature']?.metricStatus?.value;
|
||||
temperatureRuns += 1;
|
||||
});
|
||||
createComputed(() => {
|
||||
void store.activeAlerts['pve1-cpu']?.metricStatus?.value;
|
||||
cpuRuns += 1;
|
||||
});
|
||||
});
|
||||
const cpuBefore = store.activeAlerts['pve1-cpu'];
|
||||
const cpuStatusBefore = cpuBefore?.metricStatus;
|
||||
|
||||
const cooler = heldTemperatureAlert();
|
||||
cooler.metricStatus = { ...(cooler.metricStatus as object), value: 77 };
|
||||
emitActiveAlerts(200, [cooler, cpuAlert()]);
|
||||
|
||||
expect(store.activeAlerts['pve2-temperature']?.metricStatus?.value).toBe(77);
|
||||
expect(temperatureRuns).toBe(2);
|
||||
expect(cpuRuns).toBe(1);
|
||||
expect(store.activeAlerts['pve1-cpu']).toBe(cpuBefore);
|
||||
expect(store.activeAlerts['pve1-cpu']?.metricStatus).toBe(cpuStatusBefore);
|
||||
} finally {
|
||||
disposeTracking();
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('drops fields a recently resolved alert payload omits', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
await waitForOpenTick();
|
||||
const resolved = {
|
||||
...restoredTemperatureAlert(),
|
||||
resolvedTime: '2026-10-06T10:10:00Z',
|
||||
};
|
||||
emitMessage({
|
||||
type: 'initialState',
|
||||
data: {
|
||||
resources: [],
|
||||
lastUpdate: 100,
|
||||
activeAlerts: [],
|
||||
recentlyResolved: [{ ...resolved, nodeDisplayName: 'PVE Two' }],
|
||||
},
|
||||
});
|
||||
expect(store.recentlyResolved['pve2-temperature']?.nodeDisplayName).toBe('PVE Two');
|
||||
|
||||
const renamed: Record<string, unknown> = { ...resolved };
|
||||
delete renamed.nodeDisplayName;
|
||||
emitMessage({ type: 'rawData', data: { lastUpdate: 200, recentlyResolved: [renamed] } });
|
||||
expect(store.recentlyResolved['pve2-temperature']).not.toHaveProperty('nodeDisplayName');
|
||||
} finally {
|
||||
dispose();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
it('drops scroll-queued deltas when a full snapshot supersedes their baseline', async () => {
|
||||
const { store, dispose } = await createStoreHarness();
|
||||
try {
|
||||
|
||||
@@ -422,7 +422,12 @@ export function createWebSocketStore(url: string) {
|
||||
clearPendingAck(id);
|
||||
}
|
||||
|
||||
setActiveAlerts(id, alert);
|
||||
// Each payload is the server's whole alert. A plain path set would
|
||||
// shallow-merge it, so a field the server now omits (metricStatus on an
|
||||
// alert restored after a restart, ackTime/ackUser after an unacknowledge)
|
||||
// would keep its old value. reconcile removes omitted keys and diffs
|
||||
// nested values in place, so unchanged fields stay quiet.
|
||||
setActiveAlerts(id, reconcile(alert));
|
||||
});
|
||||
|
||||
setState('activeAlerts', Object.values(alertsMap));
|
||||
@@ -1380,9 +1385,10 @@ export function createWebSocketStore(url: string) {
|
||||
}
|
||||
});
|
||||
|
||||
// Add new resolved alerts
|
||||
// Add new resolved alerts, replacing rather than merging (see
|
||||
// applyActiveAlerts)
|
||||
Object.entries(newResolvedAlerts).forEach(([id, alert]) => {
|
||||
setRecentlyResolved(id, alert);
|
||||
setRecentlyResolved(id, reconcile(alert));
|
||||
});
|
||||
|
||||
setState('recentlyResolved', Object.values(newResolvedAlerts));
|
||||
@@ -1973,7 +1979,13 @@ export function createWebSocketStore(url: string) {
|
||||
}, 15000);
|
||||
pendingAckTimeouts.set(alertIdentifier, pendingTimeout);
|
||||
}
|
||||
setActiveAlerts(alertIdentifier, { ...existingAlert, ...updates });
|
||||
// Incoming payloads reconcile stored alerts in place, so store a copy
|
||||
// of the update: an optimistic record can share nested objects with
|
||||
// the record its caller keeps for rollback.
|
||||
setActiveAlerts(alertIdentifier, {
|
||||
...existingAlert,
|
||||
...structuredClone(unwrap(updates)),
|
||||
});
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user