Merge pull request #1942 from rcourtman/maintainer/20260906T171513Z

Keep alert delivery status from reverting after newer checks

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

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

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<Record<string, Alert>>({ a1: makeAlert('a1') });
+    render(() => <OverviewTab {...defaultProps()} activeAlerts={alerts()} />);
+    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<string, Alert> = { a1: makeAlert('a1') };
     getDeliveryDiagnoses.mockRejectedValue(new Error('boom'));

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<Record<string, Alert>>({
+      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<Record<string, Alert>>({

frontend-modern/src/features/alerts/useAlertOverviewState.ts

@@ -68,17 +68,21 @@ export function useAlertOverviewState(props: UseAlertOverviewStateProps) {
     Record<string, AlertDeliveryDiagnosis>
   >({});
   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<string, AlertDeliveryDiagnosis> = {};
       for (const diagnosis of list) {
         next[diagnosis.alertIdentifier || diagnosis.alertId] = diagnosis;

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 <main class="p-4"><button onClick={()=>setAlerts({a1:alert('a1'),a2:alert('a2')})}>Add alert</button>
+<OverviewTab overrides={[]} activeAlerts={alerts()}
+updateAlert={()=>{}} showQuickTip={()=>false} dismissQuickTip={()=>{}} showAcknowledged={()=>true}
+setShowAcknowledged={()=>{}} alertsDisabled={()=>false}/></main>; }
+render(()=><Router><Route path="/qualification" component={Fixture}/></Router>,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(
+              '<div id="root"></div><script type="module" src="/dispatch-fixture.tsx"></script>',
+            );
+          } 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();
+}