diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 140987af27..2690c5f5ea 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -834,6 +834,22 @@ 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. +The store's `updateAlert` holds the only optimistic copy of an acknowledgement. +`useAlertAcknowledgementState` writes through it and keeps no override of its +own, so once the server confirms it, a later server change such as an +unacknowledge from another session reaches the open tab. When no hold is +pending and the stored alert already shows the local acknowledge or +unacknowledge, the store sets no hold and keeps the server's `ackTime` and +`ackUser`: the request's own broadcast can reach the socket before its HTTP +response, and a hold would then wait for a second confirmation and skip the +next real change. Every local acknowledgement write supersedes a REST recovery +in flight: the store discards that response, which the server may have built +before accepting the write, and fetches again once it settles. A hold that +gives up after 15 seconds re-syncs from `/api/alerts/active`, waiting for a +recovery already in flight and fetching again if that one applies nothing, +because the payloads the hold skipped may not be followed by another alert +change. `websocket-unified.test.ts`, `useAlertAcknowledgementState.test.tsx` +and `useAlertOverviewState.test.tsx` pin these paths. Operational evidence and lifecycle identity are typed through `internal/operationaltrust`. Evidence envelopes distinguish completeness, confidence, permissions, freshness, correlation, and bounded provider detail. diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 1a8d71ba70..bf1638080e 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -6465,6 +6465,11 @@ rebuilding it per surface. The overview shell must compose acknowledge/restore behavior rather than keeping duplicate API and notification logic inline in `useAlertOverviewState.ts` or a revived dashboard recent-alert panel. +That hook takes the shared alert store's required `updateAlert` and keeps no +acknowledgement override: the store holds the optimistic state until the server +confirms it, while a second copy in the hook outlived that confirmation and hid +a later unacknowledge from another session until reload. Snooze state reads the +same store alerts. The same feature-owner rule now applies to the alert scheduling surface: `frontend-modern/src/features/alerts/tabs/ScheduleTab.tsx` must remain the schedule render shell, while diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index 5ed121bd04..2e6f1aff5d 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -678,7 +678,12 @@ 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 +is still replaced on every alert commit. A pending acknowledgement hold that +gives up after 15 seconds re-syncs from `/api/alerts/active` (reusing a +recovery already in flight, and fetching again if that one applies nothing) +rather than waiting for an alert delta a quiet estate may never send. A local +acknowledgement write that overlaps a recovery discards its response and +fetches once more. 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 diff --git a/frontend-modern/browser-verification/278265400aa5afa5.json b/frontend-modern/browser-verification/278265400aa5afa5.json new file mode 100644 index 0000000000..346b72a358 --- /dev/null +++ b/frontend-modern/browser-verification/278265400aa5afa5.json @@ -0,0 +1,41 @@ +{ + "version": 1, + "base_sha": "8ad0cbbe260f032868b7fdd2132d8c71f804de2c", + "verified_at": "2026-10-07T10:19:14Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/alerts/useAlertAcknowledgementState.ts", + "frontend-modern/src/features/alerts/useAlertOverviewState.ts", + "frontend-modern/src/stores/websocket.ts" + ], + "content_sha256": { + "frontend-modern/src/features/alerts/useAlertAcknowledgementState.ts": "9fac405c3a458892dba9d90398b41ccf5165b5882b7d64ecf6b0864c93669698", + "frontend-modern/src/features/alerts/useAlertOverviewState.ts": "f291f66349571e09a70f2061b9503feae62a5c290b7e6a30f02389cdfdf3b60a", + "frontend-modern/src/stores/websocket.ts": "54fdd1c3048ca7bf81f2297c5e05e6c4c465936cbb15fd561e39e57a32ee2888" + }, + "routes": [ + "/alerts/overview" + ], + "viewports": [ + { + "width": 1280, + "height": 900 + }, + { + "width": 375, + "height": 812 + } + ], + "states": [ + "Private mock stack built from this tree (backend rebuilt at the base commit, Vite serving the edited sources): Alerts > Overview with 13 active mock alerts, one critical Docker service alert (ops-prometheus) and warnings, including the Acknowledged section and the Unacknowledge control on an acknowledged card.", + "Before the change (origin/main bea0cec8b9, same stack): after a local Acknowledge then a curl unacknowledge, /api/alerts/active reported acknowledged:false while the overview kept the card hidden and read '2 critical, Acknowledged 1' against a nav badge of 3 critical, until reload.", + "After the change at 1280x900 and 375x812: the same card returns to the active list as the first card within 2 seconds of the remote unacknowledge, the header counts agree with the nav badge, and no toast fires.", + "Socket-beats-response race (acknowledge response held 1.5s in the page so the broadcast lands first, then a server-side unacknowledge 200ms after it resolves): the card is back at once and no 'Server did not confirm' toast or REST re-fetch occurs over 18s. With only websocket.ts reverted to main, the card stayed hidden about 16s and then that toast fired.", + "Only console error on the final load is the existing 403 of the acknowledge request's first attempt, which the API client retries with a fresh CSRF token (same on main)." + ], + "interactions": [ + "Clicked Acknowledge on the critical card, then POST /api/alerts/unacknowledge for that alert from curl (another session); repeated at both viewports.", + "Restore direction: curl acknowledge of a warning alert, clicked Unacknowledge in the tab (card returns to active, 'Alert restored' toast), then curl acknowledge again; the card moved back to Acknowledged within 2 seconds.", + "Race run driven from the page with a fetch wrapper delaying the acknowledge response and AlertsAPI.unacknowledge sent after it, watching toasts and /api/alerts/active requests." + ] +} diff --git a/frontend-modern/src/features/alerts/__tests__/useAlertAcknowledgementState.test.tsx b/frontend-modern/src/features/alerts/__tests__/useAlertAcknowledgementState.test.tsx index 16ccacbe58..09331ea4ac 100644 --- a/frontend-modern/src/features/alerts/__tests__/useAlertAcknowledgementState.test.tsx +++ b/frontend-modern/src/features/alerts/__tests__/useAlertAcknowledgementState.test.tsx @@ -1,5 +1,6 @@ import { renderHook } from '@solidjs/testing-library'; -import { createSignal } from 'solid-js'; +import { createMemo } from 'solid-js'; +import { createStore, reconcile } from 'solid-js/store'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { AlertsAPI } from '@/api/alerts'; @@ -43,6 +44,37 @@ function makeAlert(id: string, acknowledged = false): Alert { } as Alert; } +function acknowledgedBy(id: string, ackUser: string): Alert { + return { ...makeAlert(id, true), ackTime: '2026-03-22T11:30:00Z', ackUser }; +} + +// The keyed alert store the way the websocket store keeps it: updateAlert +// merges a local change into the stored alert, and every server payload +// replaces the stored alert in place through reconcile. The page reads the +// alerts through a memo over the keyed store, as useAlertOverviewState does. +function renderWithAlertStore(initial: Alert[]) { + return renderHook(() => { + const [alertsById, setAlertsById] = createStore>( + Object.fromEntries(initial.map((alert) => [alert.id, alert])), + ); + const alerts = createMemo(() => Object.values(alertsById)); + const updateAlert = vi.fn((alertIdentifier: string, updates: Partial) => { + const existing = alertsById[alertIdentifier]; + if (existing) { + setAlertsById(alertIdentifier, { ...existing, ...updates }); + } + }); + const serverSends = (alert: Alert) => setAlertsById(alert.id, reconcile(alert)); + + return { + ...useAlertAcknowledgementState({ alerts, updateAlert, allowRestore: true }), + alerts, + updateAlert, + serverSends, + }; + }); +} + describe('useAlertAcknowledgementState', () => { beforeEach(() => { vi.useFakeTimers(); @@ -52,85 +84,99 @@ describe('useAlertAcknowledgementState', () => { vi.mocked(AlertsAPI.bulkAcknowledge).mockReset(); vi.mocked(notificationStore.success).mockReset(); vi.mocked(notificationStore.error).mockReset(); + vi.mocked(AlertsAPI.acknowledge).mockResolvedValue(undefined as never); + vi.mocked(AlertsAPI.unacknowledge).mockResolvedValue(undefined as never); }); afterEach(() => { vi.useRealTimers(); }); - it('owns shared alert acknowledge, restore, and bulk-ack runtime with optimistic state', async () => { - const [alerts] = createSignal([ + const unacknowledgedIds = (result: ReturnType['result']) => + result.unacknowledgedAlerts().map((alert) => alert.id); + + it('shows an unacknowledge from another session after a local acknowledge', async () => { + const { result } = renderWithAlertStore([makeAlert('alert-1'), makeAlert('alert-2')]); + expect(unacknowledgedIds(result)).toEqual(['alert-1', 'alert-2']); + + await result.handleAlertAcknowledgement(result.alerts()[0]); + expect(AlertsAPI.acknowledge).toHaveBeenCalledWith('alert-1'); + expect(notificationStore.success).toHaveBeenCalledWith('Alert acknowledged'); + expect(unacknowledgedIds(result)).toEqual(['alert-2']); + + result.serverSends(acknowledgedBy('alert-1', 'admin')); + expect(unacknowledgedIds(result)).toEqual(['alert-2']); + expect(result.alerts()[0]).toMatchObject({ acknowledged: true, ackUser: 'admin' }); + + // Unacknowledged from another session: the payload omits the ack fields. + result.serverSends(makeAlert('alert-1')); + expect(unacknowledgedIds(result)).toEqual(['alert-1', 'alert-2']); + expect(result.alerts()[0].acknowledged).toBe(false); + expect(result.alerts()[0]).not.toHaveProperty('ackUser'); + + // The card acknowledges again rather than restoring a stale local state. + vi.advanceTimersByTime(1500); + await result.handleAlertAcknowledgement(result.alerts()[0]); + expect(AlertsAPI.acknowledge).toHaveBeenCalledTimes(2); + expect(AlertsAPI.unacknowledge).not.toHaveBeenCalled(); + expect(unacknowledgedIds(result)).toEqual(['alert-2']); + }); + + it('shows an acknowledge from another session after a local restore', async () => { + const { result } = renderWithAlertStore([acknowledgedBy('alert-1', 'admin')]); + expect(unacknowledgedIds(result)).toEqual([]); + + await result.handleAlertAcknowledgement(result.alerts()[0]); + expect(AlertsAPI.unacknowledge).toHaveBeenCalledWith('alert-1'); + expect(notificationStore.success).toHaveBeenCalledWith('Alert restored'); + expect(unacknowledgedIds(result)).toEqual(['alert-1']); + + result.serverSends(makeAlert('alert-1')); + expect(unacknowledgedIds(result)).toEqual(['alert-1']); + + result.serverSends(acknowledgedBy('alert-1', 'operator')); + expect(unacknowledgedIds(result)).toEqual([]); + expect(result.alerts()[0]).toMatchObject({ acknowledged: true, ackUser: 'operator' }); + }); + + it('acknowledges, restores and bulk-acknowledges through the shared alert store', async () => { + const { result } = renderWithAlertStore([ makeAlert('alert-1'), makeAlert('alert-2', true), makeAlert('alert-3'), ]); - const updateAlert = vi.fn(); - - vi.mocked(AlertsAPI.acknowledge).mockResolvedValue(undefined as never); - vi.mocked(AlertsAPI.unacknowledge).mockResolvedValue(undefined as never); vi.mocked(AlertsAPI.bulkAcknowledge).mockResolvedValue({ results: [ - { alertIdentifier: 'alert-1', success: true }, + { alertIdentifier: 'alert-2', success: true }, { alertIdentifier: 'alert-3', success: false }, ], } as never); - const { result } = renderHook(() => - useAlertAcknowledgementState({ - alerts, - updateAlert, - allowRestore: true, - }), - ); + expect(unacknowledgedIds(result)).toEqual(['alert-1', 'alert-3']); - expect(result.unacknowledgedAlerts().map((alert) => alert.id)).toEqual(['alert-1', 'alert-3']); - - await result.handleAlertAcknowledgement(alerts()[0]); - - expect(AlertsAPI.acknowledge).toHaveBeenCalledWith('alert-1'); - expect(updateAlert).toHaveBeenCalledWith( + await result.handleAlertAcknowledgement(result.alerts()[0]); + expect(result.updateAlert).toHaveBeenCalledWith( 'alert-1', expect.objectContaining({ acknowledged: true }), ); - expect(notificationStore.success).toHaveBeenCalledWith('Alert acknowledged'); - expect(result.unacknowledgedAlerts().map((alert) => alert.id)).toEqual(['alert-3']); + expect(unacknowledgedIds(result)).toEqual(['alert-3']); vi.advanceTimersByTime(1500); expect(result.processingAlerts().has('alert-1')).toBe(false); - await result.handleAlertAcknowledgement(alerts()[1]); - + await result.handleAlertAcknowledgement(result.alerts()[1]); expect(AlertsAPI.unacknowledge).toHaveBeenCalledWith('alert-2'); - expect(updateAlert).toHaveBeenCalledWith( + expect(result.updateAlert).toHaveBeenCalledWith( 'alert-2', expect.objectContaining({ acknowledged: false }), ); expect(notificationStore.success).toHaveBeenCalledWith('Alert restored'); + expect(unacknowledgedIds(result)).toEqual(['alert-2', 'alert-3']); await result.handleBulkAcknowledge(); - expect(AlertsAPI.bulkAcknowledge).toHaveBeenCalledWith(['alert-2', 'alert-3']); + expect(notificationStore.success).toHaveBeenCalledWith('Acknowledged 1 alert.'); expect(notificationStore.error).toHaveBeenCalledWith('Failed to acknowledge 1 alert.'); - }); - - it('supports optimistic acknowledgement even without an upstream update callback', async () => { - const [alerts] = createSignal([makeAlert('alert-1'), makeAlert('alert-2')]); - vi.mocked(AlertsAPI.bulkAcknowledge).mockResolvedValue({ - results: [ - { alertIdentifier: 'alert-1', success: true }, - { alertIdentifier: 'alert-2', success: true }, - ], - } as never); - - const { result } = renderHook(() => - useAlertAcknowledgementState({ - alerts, - }), - ); - - await result.handleBulkAcknowledge(); - - expect(result.unacknowledgedAlerts()).toHaveLength(0); - expect(notificationStore.success).toHaveBeenCalledWith('Acknowledged 2 alerts.'); + expect(unacknowledgedIds(result)).toEqual(['alert-3']); }); }); diff --git a/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx b/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx index 8ee5e8e9fa..80e0cd2034 100644 --- a/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx +++ b/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx @@ -1,5 +1,6 @@ import { renderHook } from '@solidjs/testing-library'; import { createSignal } from 'solid-js'; +import { createStore, reconcile } from 'solid-js/store'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { AlertsAPI } from '@/api/alerts'; @@ -142,13 +143,19 @@ describe('useAlertOverviewState', () => { it('owns overview stats, filtering, and acknowledge flows outside the tab shell', async () => { const now = Date.now(); - const [activeAlerts] = createSignal>({ - warning: makeAlert('warning', new Date(now - 60_000).toISOString(), false), + const warning = () => makeAlert('warning', new Date(now - 60_000).toISOString(), false); + // Keyed like the websocket store: updateAlert merges into the stored + // alert and server payloads replace it in place. + const [alertsById, setAlertsById] = createStore>({ + warning: warning(), acknowledged: makeAlert('acknowledged', new Date(now - 2 * 60_000).toISOString(), true), old: makeAlert('old', new Date(now - 3 * 86_400_000).toISOString(), false), }); + const activeAlerts = () => alertsById; const [showAcknowledged] = createSignal(false); - const updateAlert = vi.fn(); + const updateAlert = vi.fn((alertIdentifier: string, updates: Partial) => { + setAlertsById(alertIdentifier, { ...alertsById[alertIdentifier], ...updates }); + }); vi.mocked(AlertsAPI.acknowledge).mockResolvedValue(undefined as any); vi.mocked(AlertsAPI.unacknowledge).mockResolvedValue(undefined as any); @@ -185,10 +192,22 @@ describe('useAlertOverviewState', () => { ); expect(notificationStore.success).toHaveBeenCalledWith('Alert acknowledged'); expect(result.processingAlerts().has('warning')).toBe(true); + expect(result.filteredAlerts().map((alert) => alert.id)).toEqual(['old']); + expect(result.alertStats()).toMatchObject({ active: 1, acknowledged: 2 }); + + // The server confirms, then another session unacknowledges: the card + // returns without a reload. + setAlertsById('warning', reconcile({ ...warning(), acknowledged: true, ackUser: 'admin' })); + expect(result.filteredAlerts().map((alert) => alert.id)).toEqual(['old']); + setAlertsById('warning', reconcile(warning())); + expect(result.filteredAlerts().map((alert) => alert.id)).toEqual(['warning', 'old']); + expect(result.alertStats()).toMatchObject({ active: 2, acknowledged: 1 }); vi.advanceTimersByTime(1500); expect(result.processingAlerts().has('warning')).toBe(false); + await result.handleAlertAcknowledgement(alertsById.warning); + vi.advanceTimersByTime(1500); await result.handleBulkAcknowledge(); expect(AlertsAPI.bulkAcknowledge).toHaveBeenCalledWith(['old']); diff --git a/frontend-modern/src/features/alerts/useAlertAcknowledgementState.ts b/frontend-modern/src/features/alerts/useAlertAcknowledgementState.ts index 24b80d9986..88c7d3e225 100644 --- a/frontend-modern/src/features/alerts/useAlertAcknowledgementState.ts +++ b/frontend-modern/src/features/alerts/useAlertAcknowledgementState.ts @@ -1,4 +1,4 @@ -import { createEffect, createMemo, createSignal, onCleanup } from 'solid-js'; +import { createMemo, createSignal, onCleanup } from 'solid-js'; import type { Accessor } from 'solid-js'; import { AlertsAPI } from '@/api/alerts'; @@ -18,18 +18,17 @@ import { getCanonicalAlertId } from './identity'; export interface UseAlertAcknowledgementStateProps { alerts: Accessor; - updateAlert?: (alertIdentifier: string, updates: Partial) => void; + // The shared alert store owns the optimistic acknowledgement: it holds back + // stale payloads until the server confirms, then lets later server changes + // (an unacknowledge from another session) through. Keep no second copy + // here; one would outlive that confirmation and hide those changes. + updateAlert: (alertIdentifier: string, updates: Partial) => void; allowRestore?: boolean; } -type AlertAcknowledgementOverride = Pick; - export function useAlertAcknowledgementState(props: UseAlertAcknowledgementStateProps) { const [processingAlerts, setProcessingAlerts] = createSignal>(new Set()); const [bulkAckProcessing, setBulkAckProcessing] = createSignal(false); - const [acknowledgementOverrides, setAcknowledgementOverrides] = createSignal< - Record - >({}); const processingReleaseTimers = new Map>(); const clearProcessingReleaseTimer = (alertIdentifier: string) => { @@ -46,46 +45,10 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState processingReleaseTimers.clear(); }); - createEffect(() => { - const alertsByIdentifier = new Map( - props.alerts().map((alert) => [getCanonicalAlertId(alert), alert] as const), - ); - - setAcknowledgementOverrides((previous) => { - let changed = false; - const next = { ...previous }; - - for (const [alertIdentifier, override] of Object.entries(previous)) { - const alert = alertsByIdentifier.get(alertIdentifier); - if (!alert || alert.acknowledged === override.acknowledged) { - delete next[alertIdentifier]; - changed = true; - } - } - - return changed ? next : previous; - }); - }); - - const effectiveAlerts = createMemo(() => - props.alerts().map((alert) => { - const override = acknowledgementOverrides()[getCanonicalAlertId(alert)]; - return override ? { ...alert, ...override } : alert; - }), - ); - const unacknowledgedAlerts = createMemo(() => - effectiveAlerts().filter((alert) => !alert.acknowledged), + props.alerts().filter((alert) => !alert.acknowledged), ); - const applyAlertUpdate = (alertIdentifier: string, updates: AlertAcknowledgementOverride) => { - setAcknowledgementOverrides((previous) => ({ - ...previous, - [alertIdentifier]: updates, - })); - props.updateAlert?.(alertIdentifier, updates); - }; - const releaseAlertProcessing = (alertIdentifier: string) => { clearProcessingReleaseTimer(alertIdentifier); const timer = setTimeout(() => { @@ -106,7 +69,7 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState } const currentAlert = - effectiveAlerts().find((entry) => getCanonicalAlertId(entry) === alertIdentifier) ?? alert; + props.alerts().find((entry) => getCanonicalAlertId(entry) === alertIdentifier) ?? alert; const wasAcknowledged = currentAlert.acknowledged; if (wasAcknowledged && !props.allowRestore) { return; @@ -117,7 +80,7 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState try { if (wasAcknowledged) { await AlertsAPI.unacknowledge(alertIdentifier); - applyAlertUpdate(alertIdentifier, { + props.updateAlert(alertIdentifier, { acknowledged: false, ackTime: undefined, ackUser: undefined, @@ -125,7 +88,7 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState notificationStore.success(getAlertOverviewRestoredNotification()); } else { await AlertsAPI.acknowledge(alertIdentifier); - applyAlertUpdate(alertIdentifier, { + props.updateAlert(alertIdentifier, { acknowledged: true, ackTime: new Date().toISOString(), ackUser: undefined, @@ -160,7 +123,7 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState const failures = result.results.filter((entry) => !entry.success); successes.forEach((entry) => { - applyAlertUpdate(entry.alertIdentifier, { + props.updateAlert(entry.alertIdentifier, { acknowledged: true, ackTime: acknowledgedAt, ackUser: undefined, @@ -187,7 +150,7 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState const handleGroupAcknowledge = async (alerts: Alert[]) => { const pending = alerts.filter((alert) => { const id = getCanonicalAlertId(alert); - return !effectiveAlerts().find((e) => getCanonicalAlertId(e) === id)?.acknowledged; + return !props.alerts().find((e) => getCanonicalAlertId(e) === id)?.acknowledged; }); if (pending.length === 0) return; @@ -201,7 +164,7 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState const successes = result.results.filter((entry) => entry.success); successes.forEach((entry) => { - applyAlertUpdate(entry.alertIdentifier, { + props.updateAlert(entry.alertIdentifier, { acknowledged: true, ackTime: acknowledgedAt, ackUser: undefined, @@ -220,7 +183,6 @@ export function useAlertAcknowledgementState(props: UseAlertAcknowledgementState }; return { - effectiveAlerts, unacknowledgedAlerts, processingAlerts, bulkAckProcessing, diff --git a/frontend-modern/src/features/alerts/useAlertOverviewState.ts b/frontend-modern/src/features/alerts/useAlertOverviewState.ts index 24c3fde4a6..583bb9163b 100644 --- a/frontend-modern/src/features/alerts/useAlertOverviewState.ts +++ b/frontend-modern/src/features/alerts/useAlertOverviewState.ts @@ -38,7 +38,6 @@ export function useAlertOverviewState(props: UseAlertOverviewStateProps) { const tickInterval = setInterval(() => setTick(Date.now()), 60_000); const activeAlerts = createMemo(() => Object.values(props.activeAlerts())); const { - effectiveAlerts: acknowledgedAlerts, unacknowledgedAlerts, processingAlerts, bulkAckProcessing, @@ -55,7 +54,7 @@ export function useAlertOverviewState(props: UseAlertOverviewStateProps) { processing: snoozeProcessingAlerts, handleSnooze, handleUnsnooze, - } = useAlertSnoozeState({ alerts: acknowledgedAlerts, updateAlert: props.updateAlert }); + } = useAlertSnoozeState({ alerts: activeAlerts, updateAlert: props.updateAlert }); onCleanup(() => { clearInterval(tickInterval); diff --git a/frontend-modern/src/stores/__tests__/websocket-unified.test.ts b/frontend-modern/src/stores/__tests__/websocket-unified.test.ts index 2b94a4ea8c..d8a246a34f 100644 --- a/frontend-modern/src/stores/__tests__/websocket-unified.test.ts +++ b/frontend-modern/src/stores/__tests__/websocket-unified.test.ts @@ -1474,6 +1474,244 @@ describe('websocket store unified resource contract', () => { } }); + it('lets a remote unacknowledge through when the acknowledge broadcast beat its response', async () => { + const { store, dispose } = await createStoreHarness(); + const { notificationStore } = await import('@/stores/notifications'); + const notifyError = vi.spyOn(notificationStore, 'error'); + try { + await waitForOpenTick(); + const id = 'pve2-temperature'; + emitInitialAlerts([restoredTemperatureAlert()]); + + // The acknowledge request's own broadcast lands before its response. + emitActiveAlerts(200, [ + { + ...restoredTemperatureAlert(), + acknowledged: true, + ackTime: '2026-10-06T10:06:01Z', + ackUser: 'admin', + }, + ]); + store.updateAlert(id, { + acknowledged: true, + ackTime: '2026-10-06T10:06:02Z', + ackUser: undefined, + }); + expect(store.activeAlerts[id]).toMatchObject({ + acknowledged: true, + ackTime: '2026-10-06T10:06:01Z', + ackUser: 'admin', + }); + + // Unacknowledged from another session before any further payload. + emitActiveAlerts(300, [restoredTemperatureAlert()]); + expect(store.activeAlerts[id]?.acknowledged).toBe(false); + + vi.advanceTimersByTime(15_000); + expect(notifyError).not.toHaveBeenCalled(); + expect(apiFetchJSONMock).not.toHaveBeenCalled(); + } finally { + notifyError.mockRestore(); + dispose(); + } + }); + + it('fetches the current alerts when the server never confirms a local acknowledge', async () => { + const { store, dispose } = await createStoreHarness(); + const { notificationStore } = await import('@/stores/notifications'); + const notifyError = vi.spyOn(notificationStore, 'error'); + try { + await waitForOpenTick(); + const id = 'pve2-temperature'; + emitInitialAlerts([restoredTemperatureAlert()]); + + store.updateAlert(id, { + acknowledged: true, + ackTime: '2026-10-06T10:06:00Z', + ackUser: undefined, + }); + // Unacknowledged elsewhere before this acknowledge was ever confirmed: + // indistinguishable from a stale payload, so the guard holds it back. + emitActiveAlerts(200, [restoredTemperatureAlert()]); + expect(store.activeAlerts[id]?.acknowledged).toBe(true); + + apiFetchJSONMock.mockResolvedValueOnce([restoredTemperatureAlert()]); + vi.advanceTimersByTime(15_000); + await flushMicrotasks(); + expect(notifyError).toHaveBeenCalledOnce(); + expect(apiFetchJSONMock).toHaveBeenCalledWith('/api/alerts/active'); + expect(store.activeAlerts[id]?.acknowledged).toBe(false); + expect(store.activeAlerts[id]).not.toHaveProperty('ackTime'); + } finally { + notifyError.mockRestore(); + dispose(); + } + }); + + const cpuAlert = (): Record => ({ + ...restoredTemperatureAlert(), + id: 'pve2-cpu', + type: 'cpu', + message: 'Node CPU at 95%', + }); + + const deferRecovery = () => { + let resolve: (alerts: unknown[]) => void = () => {}; + apiFetchJSONMock.mockReturnValueOnce( + new Promise((done) => { + resolve = done; + }), + ); + return (alerts: unknown[]) => resolve(alerts); + }; + + it('replaces a REST recovery in flight during a local acknowledge with a fresh one', async () => { + const { store, dispose } = await createStoreHarness(); + try { + await waitForOpenTick(); + const id = 'pve2-temperature'; + // Acknowledged by another session, then restored there: REST captures + // the restore before this tab's acknowledge reaches the server. + emitInitialAlerts([ + { + ...restoredTemperatureAlert(), + acknowledged: true, + ackTime: '2026-10-06T10:06:01Z', + ackUser: 'operator', + }, + cpuAlert(), + ]); + const resolveStale = deferRecovery(); + const stale = store.refreshActiveAlerts(); + store.updateAlert(id, { + acknowledged: true, + ackTime: '2026-10-06T10:06:02Z', + ackUser: undefined, + }); + + const resolveFresh = deferRecovery(); + resolveStale([restoredTemperatureAlert(), cpuAlert()]); + await expect(stale).resolves.toBe(false); + await flushMicrotasks(); + expect(store.activeAlerts[id]?.acknowledged).toBe(true); + expect(apiFetchJSONMock).toHaveBeenCalledTimes(2); + + // The fresh request reports what happened after the acknowledge, + // here the alert resolving and the CPU alert being acknowledged. + resolveFresh([{ ...cpuAlert(), acknowledged: true, ackUser: 'operator' }]); + await flushMicrotasks(); + expect(store.activeAlerts[id]).toBeUndefined(); + expect(store.activeAlerts['pve2-cpu']).toMatchObject({ + acknowledged: true, + ackUser: 'operator', + }); + } finally { + dispose(); + } + }); + + it('lets the give-up re-sync land when another alert is acknowledged meanwhile', async () => { + const { store, dispose } = await createStoreHarness(); + try { + await waitForOpenTick(); + const id = 'pve2-temperature'; + const acknowledgedCpu = { ...cpuAlert(), acknowledged: true, ackUser: 'admin' }; + emitInitialAlerts([restoredTemperatureAlert(), cpuAlert()]); + + store.updateAlert(id, { + acknowledged: true, + ackTime: '2026-10-06T10:06:00Z', + ackUser: undefined, + }); + // The CPU alert's acknowledge broadcast arrives before its response. + emitActiveAlerts(200, [restoredTemperatureAlert(), acknowledgedCpu]); + expect(store.activeAlerts[id]?.acknowledged).toBe(true); + + const resolveResync = deferRecovery(); + vi.advanceTimersByTime(15_000); + expect(apiFetchJSONMock).toHaveBeenCalledOnce(); + store.updateAlert('pve2-cpu', { + acknowledged: true, + ackTime: '2026-10-06T10:20:00Z', + ackUser: undefined, + }); + apiFetchJSONMock.mockResolvedValueOnce([restoredTemperatureAlert(), acknowledgedCpu]); + resolveResync([restoredTemperatureAlert(), acknowledgedCpu]); + await flushMicrotasks(12); + expect(apiFetchJSONMock).toHaveBeenCalledTimes(2); + expect(store.activeAlerts[id]?.acknowledged).toBe(false); + expect(store.activeAlerts['pve2-cpu']).toMatchObject({ + acknowledged: true, + ackUser: 'admin', + }); + } finally { + dispose(); + } + }); + + it('fetches again when the recovery in flight at the give-up is fenced out', async () => { + const { store, dispose } = await createStoreHarness(); + try { + await waitForOpenTick(); + const id = 'pve2-temperature'; + emitInitialAlerts([restoredTemperatureAlert()]); + store.updateAlert(id, { + acknowledged: true, + ackTime: '2026-10-06T10:06:00Z', + ackUser: undefined, + }); + + // A recovery starts after the acknowledge, then an unacknowledge from + // another session lands over the socket and fences it out. + const resolveFenced = deferRecovery(); + void store.refreshActiveAlerts(); + emitActiveAlerts(200, [restoredTemperatureAlert()]); + expect(store.activeAlerts[id]?.acknowledged).toBe(true); + + apiFetchJSONMock.mockResolvedValueOnce([restoredTemperatureAlert()]); + vi.advanceTimersByTime(15_000); + resolveFenced([{ ...restoredTemperatureAlert(), acknowledged: true, ackUser: 'admin' }]); + await flushMicrotasks(12); + expect(apiFetchJSONMock).toHaveBeenCalledTimes(2); + expect(store.activeAlerts[id]?.acknowledged).toBe(false); + } finally { + dispose(); + } + }); + + it('drops a recovery that predates a held acknowledge when the hold gives up', async () => { + const { store, dispose } = await createStoreHarness(); + try { + await waitForOpenTick(); + const id = 'pve2-temperature'; + emitInitialAlerts([restoredTemperatureAlert()]); + const resolveStale = deferRecovery(); + void store.refreshActiveAlerts(); + + store.updateAlert(id, { + acknowledged: true, + ackTime: '2026-10-06T10:06:00Z', + ackUser: undefined, + }); + // No confirming payload arrives before the hold gives up. + apiFetchJSONMock.mockResolvedValueOnce([ + { + ...restoredTemperatureAlert(), + acknowledged: true, + ackTime: '2026-10-06T10:06:01Z', + ackUser: 'admin', + }, + ]); + vi.advanceTimersByTime(15_000); + resolveStale([restoredTemperatureAlert()]); + await flushMicrotasks(12); + expect(apiFetchJSONMock).toHaveBeenCalledTimes(2); + expect(store.activeAlerts[id]).toMatchObject({ acknowledged: true, ackUser: 'admin' }); + } finally { + dispose(); + } + }); + it('keeps the record a local update replaced intact for rollback', async () => { const { store, dispose } = await createStoreHarness(); try { diff --git a/frontend-modern/src/stores/websocket.ts b/frontend-modern/src/stores/websocket.ts index 28023d5c01..b933f7e384 100644 --- a/frontend-modern/src/stores/websocket.ts +++ b/frontend-modern/src/stores/websocket.ts @@ -651,6 +651,31 @@ export function createWebSocketStore(url: string) { return request; }; + // A local acknowledgement is written once the server has accepted it, so a + // recovery in flight may hold the state from before it. Discard that + // response and fetch again once it settles. + const supersedeActiveAlertsRecovery = () => { + const inFlight = activeAlertsRecoveryInFlight; + if (!inFlight) return; + activeAlertsRevision += 1; + void inFlight.then(() => { + if (!isDisposed) void recoverActiveAlertsFromREST(true); + }); + }; + + // A recovery already in flight may predate the change that made this + // re-sync necessary; its revision fence then discards it, so fetch again. + const resyncActiveAlertsFromREST = () => { + const inFlight = activeAlertsRecoveryInFlight; + if (!inFlight) { + void recoverActiveAlertsFromREST(true); + return; + } + void inFlight.then((applied) => { + if (!applied && !isDisposed) void recoverActiveAlertsFromREST(true); + }); + }; + const scheduleColdActiveAlertsRecovery = (connectionId: number) => { if (hasActiveAlertsSnapshot || activeAlertsColdHydrationTimeout) return; activeAlertsColdHydrationTimeout = window.setTimeout(() => { @@ -1957,8 +1982,25 @@ export function createWebSocketStore(url: string) { updateAlert: (alertIdentifier: string, updates: Partial) => { const existingAlert = activeAlerts[alertIdentifier]; if (existingAlert) { - // Track this alert as having pending changes if acknowledgment is changing + let localUpdates = updates; if ('acknowledged' in updates) { + supersedeActiveAlertsRecovery(); + } + if ( + 'acknowledged' in updates && + !pendingAckChanges.has(alertIdentifier) && + Boolean(existingAlert.acknowledged) === Boolean(updates.acknowledged) + ) { + // The request's own broadcast can beat its HTTP response, so the + // server's state may already be stored. A pending entry would wait + // for a second confirmation and hold back the next real change (an + // unacknowledge from another session), so keep the server's fields. + localUpdates = { ...updates }; + delete localUpdates.acknowledged; + delete localUpdates.ackTime; + delete localUpdates.ackUser; + } else if ('acknowledged' in updates) { + // Track this alert as having pending changes if acknowledgment is changing const previousAckTime = existingAlert.ackTime; pendingAckChanges.set(alertIdentifier, { ack: !!updates.acknowledged, @@ -1975,6 +2017,10 @@ export function createWebSocketStore(url: string) { notificationStore.error( 'Server did not confirm the alert acknowledgment in time. Re-syncing from latest data.', ); + // The guard skipped the payloads that disagreed, and a quiet + // estate may send no further alert change, so fetch the + // server's current alerts rather than keep the local state. + resyncActiveAlertsFromREST(); } }, 15000); pendingAckTimeouts.set(alertIdentifier, pendingTimeout); @@ -1984,7 +2030,7 @@ export function createWebSocketStore(url: string) { // the record its caller keeps for rollback. setActiveAlerts(alertIdentifier, { ...existingAlert, - ...structuredClone(unwrap(updates)), + ...structuredClone(unwrap(localUpdates)), }); } },