diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index a47e21132..acb62df48 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -15,6 +15,15 @@ ## Purpose +Overview delivery diagnoses use latest-started refresh ownership. Older bulk +responses cannot overwrite newer card notification status, and an empty active +alert set invalidates outstanding reads. Disposal also prevents updates. Failed +refreshes retain the existing snapshot; this ordering repair does not add a +freshness indicator or establish recipient receipt. Verify response overlap in +`OverviewTab.deliverystatus.test.tsx`, empty-set invalidation in +`useAlertOverviewState.test.tsx`, and rendered ordering at three widths using +`scripts/check-alert-diagnosis-ordering.mjs`. + Delivery-attempt and held-event reads in Destinations use latest-started refresh ownership. A delayed mount response must not overwrite evidence from configuration Retry or a queue-action refresh, including a newer unavailable diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 19d0e4991..7af5c7136 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -20,6 +20,15 @@ ## Purpose +Overview delivery diagnoses use latest-started refresh ownership. Older bulk +responses cannot overwrite newer card notification status, and an empty active +alert set invalidates outstanding reads. Disposal also prevents updates. Failed +refreshes retain the existing snapshot; this ordering repair does not add a +freshness indicator or establish recipient receipt. Verify response overlap in +`OverviewTab.deliverystatus.test.tsx`, empty-set invalidation in +`useAlertOverviewState.test.tsx`, and rendered ordering at three widths using +`scripts/check-alert-diagnosis-ordering.mjs`. + The Destinations delivery-log state primitive assigns a generation to each refresh and rejects stale completions before updating rows, unavailable state or loading state. Held-event reads share that generation without blocking the diff --git a/frontend-modern/src/features/alerts/__tests__/OverviewTab.deliverystatus.test.tsx b/frontend-modern/src/features/alerts/__tests__/OverviewTab.deliverystatus.test.tsx index 631ae1590..5dcaaaeb7 100644 --- a/frontend-modern/src/features/alerts/__tests__/OverviewTab.deliverystatus.test.tsx +++ b/frontend-modern/src/features/alerts/__tests__/OverviewTab.deliverystatus.test.tsx @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createSignal } from 'solid-js'; import { cleanup, render, screen, waitFor } from '@solidjs/testing-library'; import { DEFAULT_LOCALE, setActiveLocale } from '@/i18n'; import type { Alert, AlertDeliveryDiagnosis } from '@/types/api'; @@ -124,6 +125,27 @@ describe('OverviewTab delivery status line', () => { if (state.reason === 'cooldown') expect(screen.getByText(/next eligible/)).toBeTruthy(); }); + it('ignores an older diagnosis response after the active alert set changes', async () => { + let finishOlder!: (value: AlertDeliveryDiagnosis[]) => void; + getDeliveryDiagnoses.mockReturnValueOnce( + new Promise((resolve) => { + finishOlder = resolve; + }), + ); + getDeliveryDiagnoses.mockResolvedValueOnce([ + makeDiagnosis('a1', { status: 'suppressed', reason: 'notifications_disabled' }), + ]); + const [alerts, setAlerts] = createSignal>({ a1: makeAlert('a1') }); + render(() => ); + await waitFor(() => expect(getDeliveryDiagnoses).toHaveBeenCalledTimes(1)); + setAlerts({ a1: makeAlert('a1'), a2: makeAlert('a2') }); + await waitFor(() => expect(screen.getByText('Notifications are turned off')).toBeTruthy()); + finishOlder([makeDiagnosis('a1', { lastNotified: '2026-08-26T10:15:00Z' })]); + await Promise.resolve(); + expect(screen.queryByText(/^Dispatch requested /)).toBeNull(); + expect(screen.getByText('Notifications are turned off')).toBeTruthy(); + }); + it('renders no delivery line when the diagnosis fetch fails', async () => { const activeAlerts: Record = { a1: makeAlert('a1') }; getDeliveryDiagnoses.mockRejectedValue(new Error('boom')); diff --git a/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx b/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx index edc4f4569..8ee5e8e9f 100644 --- a/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx +++ b/frontend-modern/src/features/alerts/__tests__/useAlertOverviewState.test.tsx @@ -4,12 +4,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { AlertsAPI } from '@/api/alerts'; import { notificationStore } from '@/stores/notifications'; -import type { Alert } from '@/types/api'; +import type { Alert, AlertDeliveryDiagnosis } from '@/types/api'; import { useAlertOverviewState } from '../useAlertOverviewState'; vi.mock('@/api/alerts', () => ({ AlertsAPI: { + getDeliveryDiagnoses: vi.fn(), acknowledge: vi.fn(), bulkAcknowledge: vi.fn(), unacknowledge: vi.fn(), @@ -47,6 +48,7 @@ describe('useAlertOverviewState', () => { beforeEach(() => { vi.useFakeTimers(); vi.setSystemTime(new Date('2026-03-22T12:00:00Z')); + vi.mocked(AlertsAPI.getDeliveryDiagnoses).mockReset().mockResolvedValue([]); vi.mocked(AlertsAPI.acknowledge).mockReset(); vi.mocked(AlertsAPI.unacknowledge).mockReset(); vi.mocked(AlertsAPI.bulkAcknowledge).mockReset(); @@ -58,6 +60,86 @@ describe('useAlertOverviewState', () => { vi.useRealTimers(); }); + it('invalidates pending diagnosis reads when the active set becomes empty', async () => { + let finish!: (value: AlertDeliveryDiagnosis[]) => void; + vi.mocked(AlertsAPI.getDeliveryDiagnoses).mockReturnValueOnce( + new Promise((resolve) => { + finish = resolve; + }), + ); + const [activeAlerts, setActiveAlerts] = createSignal>({ + a1: makeAlert('a1', new Date().toISOString()), + }); + const { result } = renderHook(() => + useAlertOverviewState({ + activeAlerts, + overrides: () => [], + showAcknowledged: () => true, + updateAlert: vi.fn(), + }), + ); + expect(AlertsAPI.getDeliveryDiagnoses).toHaveBeenCalledOnce(); + setActiveAlerts({}); + finish([{ alertIdentifier: 'a1', reason: 'ready' } as AlertDeliveryDiagnosis]); + await Promise.resolve(); + expect(result.deliveryDiagnoses()).toEqual({}); + }); + + it('does not accept an older success when a newer periodic refresh fails', async () => { + let finishOlder!: (value: AlertDeliveryDiagnosis[]) => void; + const retained = { + alertIdentifier: 'a1', + reason: 'notifications_disabled', + } as AlertDeliveryDiagnosis; + vi.mocked(AlertsAPI.getDeliveryDiagnoses) + .mockResolvedValueOnce([retained]) + .mockReturnValueOnce( + new Promise((resolve) => { + finishOlder = resolve; + }), + ) + .mockRejectedValueOnce(new Error('refresh unavailable')); + const { result } = renderHook(() => + useAlertOverviewState({ + activeAlerts: () => ({ a1: makeAlert('a1', new Date().toISOString()) }), + overrides: () => [], + showAcknowledged: () => true, + updateAlert: vi.fn(), + }), + ); + await Promise.resolve(); + expect(result.deliveryDiagnoses()).toEqual({ a1: retained }); + await vi.advanceTimersByTimeAsync(60_000); + await vi.advanceTimersByTimeAsync(60_000); + expect(AlertsAPI.getDeliveryDiagnoses).toHaveBeenCalledTimes(3); + finishOlder([{ alertIdentifier: 'a1', reason: 'ready' } as AlertDeliveryDiagnosis]); + await Promise.resolve(); + expect(result.deliveryDiagnoses()).toEqual({ a1: retained }); + }); + + it('ignores pending responses and stops periodic reads after disposal', async () => { + let finish!: (value: AlertDeliveryDiagnosis[]) => void; + vi.mocked(AlertsAPI.getDeliveryDiagnoses).mockReturnValueOnce( + new Promise((resolve) => { + finish = resolve; + }), + ); + const { result, cleanup } = renderHook(() => + useAlertOverviewState({ + activeAlerts: () => ({ a1: makeAlert('a1', new Date().toISOString()) }), + overrides: () => [], + showAcknowledged: () => true, + updateAlert: vi.fn(), + }), + ); + cleanup(); + finish([{ alertIdentifier: 'a1', reason: 'ready' } as AlertDeliveryDiagnosis]); + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(120_000); + expect(result.deliveryDiagnoses()).toEqual({}); + expect(AlertsAPI.getDeliveryDiagnoses).toHaveBeenCalledOnce(); + }); + it('owns overview stats, filtering, and acknowledge flows outside the tab shell', async () => { const now = Date.now(); const [activeAlerts] = createSignal>({ diff --git a/frontend-modern/src/features/alerts/useAlertOverviewState.ts b/frontend-modern/src/features/alerts/useAlertOverviewState.ts index 650a92063..377bb479e 100644 --- a/frontend-modern/src/features/alerts/useAlertOverviewState.ts +++ b/frontend-modern/src/features/alerts/useAlertOverviewState.ts @@ -68,17 +68,21 @@ export function useAlertOverviewState(props: UseAlertOverviewStateProps) { Record >({}); let diagnosisStateDisposed = false; + let diagnosisRequestVersion = 0; onCleanup(() => { diagnosisStateDisposed = true; }); const refreshDeliveryDiagnoses = async () => { + // A slower previous refresh must not replace a newer notification state. + // Increment even for an empty alert set to invalidate outstanding requests. + const requestVersion = ++diagnosisRequestVersion; if (activeAlerts().length === 0) { setDeliveryDiagnoses({}); return; } try { const list = await AlertsAPI.getDeliveryDiagnoses(); - if (diagnosisStateDisposed) return; + if (diagnosisStateDisposed || requestVersion !== diagnosisRequestVersion) return; const next: Record = {}; for (const diagnosis of list) { next[diagnosis.alertIdentifier || diagnosis.alertId] = diagnosis; diff --git a/scripts/check-alert-diagnosis-ordering.mjs b/scripts/check-alert-diagnosis-ordering.mjs new file mode 100644 index 000000000..45b4e30f4 --- /dev/null +++ b/scripts/check-alert-diagnosis-ordering.mjs @@ -0,0 +1,146 @@ +// Isolated real-browser component qualification; no installed backend or delivery claim. +import { createServer } from "../frontend-modern/node_modules/vite/dist/node/index.js"; +import solid from "../frontend-modern/node_modules/vite-plugin-solid/dist/esm/index.mjs"; +import { chromium } from "@playwright/test"; +import { resolve } from "node:path"; +import { mkdirSync } from "node:fs"; +import assert from "node:assert/strict"; +const root = resolve("frontend-modern"); +process.chdir(root); +const fixture = ` +import { createSignal } from 'solid-js'; +import { render } from 'solid-js/web'; +import { Router, Route } from '@solidjs/router'; +import { AlertsAPI } from '/src/api/alerts'; +import { NotificationsAPI } from '/src/api/notifications'; +import { OverviewTab } from '/src/features/alerts/OverviewTab'; +import '/src/index.css'; +let finishOlder; +let requests = 0; +AlertsAPI.getDeliveryDiagnoses = () => { + requests++; + if (requests === 1) return new Promise(resolve => { finishOlder = resolve; }); + return Promise.resolve([{alertIdentifier:'a1', alertId:'a1', status:'suppressed', + reason:'notifications_disabled', message:'Notifications disabled by current configuration'}]); +}; +window.finishOlder = () => finishOlder([{alertIdentifier:'a1', alertId:'a1', + status:'would_send',reason:'ready',lastNotified:'2026-08-26T10:15:00Z'}]); +window.requestCount = () => requests; +AlertsAPI.getEvents = async () => []; +NotificationsAPI.getHealth = async () => ({queue:{status:'healthy'}}); +const alert = id => ({id,resourceId:id,resourceName:'VM '+id,type:'cpu',level:'warning', +message:'High CPU on '+id,startTime:new Date().toISOString(),acknowledged:false,node:'node1'}); +function Fixture() { +const [alerts, setAlerts] = createSignal({a1:alert('a1')}); +return
+{}} showQuickTip={()=>false} dismissQuickTip={()=>{}} showAcknowledged={()=>true} +setShowAcknowledged={()=>{}} alertsDisabled={()=>false}/>
; } +render(()=>,document.getElementById('root')); +`; +const server = await createServer({ + root, + configFile: false, + optimizeDeps: { + noDiscovery: true, + entries: [], + esbuildOptions: { target: "esnext" }, + }, + esbuild: { target: "esnext" }, + plugins: [ + solid(), + { + name: "dispatch-fixture", + configureServer(s) { + s.middlewares.use((req, res, next) => { + if (req.url === "/qualification") { + res.setHeader("Content-Type", "text/html"); + res.end( + '
', + ); + } else next(); + }); + }, + resolveId(id) { + if (id === "/dispatch-fixture.tsx") return id; + }, + load(id) { + if (id === "/dispatch-fixture.tsx") return fixture; + }, + }, + ], + resolve: { alias: { "@": resolve(root, "src") } }, + server: { host: "127.0.0.1", port: 5199, strictPort: true }, +}); +let browser; +try { + await server.listen(); + browser = await chromium.launch({ headless: true }); + mkdirSync("/tmp/pulse-alert-diagnosis-ordering", { recursive: true }); + for (const width of [1440, 900, 390]) { + const page = await browser.newPage({ viewport: { width, height: 1000 } }); + const errors = []; + page.on("pageerror", (e) => { + errors.push(e.message); + console.error(e.message); + }); + page.on("console", (m) => { + if (m.type() === "error") console.error(m.text()); + }); + await page.route("http://127.0.0.1:5199/api/**", (route) => + route.fulfill({ json: [] }), + ); + await page.goto("http://127.0.0.1:5199/qualification"); + await page.waitForFunction(() => window.requestCount?.() === 1); + await page.getByRole("button", { name: "Add alert", exact: true }).click(); + await page + .getByText("Notifications are turned off", { exact: true }) + .waitFor(); + await page.evaluate(async () => { + window.finishOlder(); + await Promise.resolve(); + }); + assert.equal( + await page + .getByText("Notifications are turned off", { exact: true }) + .count(), + 1, + ); + assert.equal(await page.getByText(/^Dispatch requested /).count(), 0); + assert.equal( + await page.getByText("High CPU on a2", { exact: true }).count(), + 1, + ); + const label = page.getByText("Notifications are turned off", { + exact: true, + }); + assert.equal( + await label.evaluate((el) => { + const range = document.createRange(); + range.selectNodeContents(el); + return [...range.getClientRects()].every( + (b) => b.left >= 0 && b.right <= innerWidth, + ); + }), + true, + "current status must fit viewport", + ); + assert.deepEqual(errors, []); + await page.screenshot({ + path: "/tmp/pulse-alert-diagnosis-ordering/" + width + ".png", + fullPage: true, + }); + await page.close(); + } + console.log( + JSON.stringify({ + result: "passed", + viewports: [1440, 900, 390], + scope: + "Real Overview and Chromium; scripted diagnoses, not installed delivery or receipt", + }), + ); +} finally { + await browser?.close(); + await server.close(); +}