mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
fix(alerts): ignore superseded delivery diagnosis responses
Overlapping bulk diagnosis refreshes could resolve out of order and replace a current notifications-disabled warning with older dispatch evidence. Version each request, including empty alert sets, so only the newest response updates card diagnoses. Pin overlap and empty-set invalidation with component and registered hook regressions. All 47 focused tests and TypeScript pass. Isolated Chromium verifies rendered ordering at three widths; update both subsystem contracts and bind browser proof to the runtime bytes. No backend delivery or recipient receipt is claimed. Change-source: pulse-maintainer
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
{
|
||||
"version": 1,
|
||||
"base_sha": "4fd68c72b26f033f5b1b0fcb7d7b2b0d67e85ec8",
|
||||
"verified_at": "2026-09-06T14:12:06.315693Z",
|
||||
"base_sha": "5000e0409b5795f732f32882d7c6929e7eff16b1",
|
||||
"verified_at": "2026-09-06T16:16:58.486143Z",
|
||||
"result": "passed",
|
||||
"changed_paths": [
|
||||
"frontend-modern/src/features/alerts/deliveryDiagnosisPresentation.ts"
|
||||
"frontend-modern/src/features/alerts/useAlertOverviewState.ts"
|
||||
],
|
||||
"content_sha256": {
|
||||
"frontend-modern/src/features/alerts/deliveryDiagnosisPresentation.ts": "bc3eadf8f790517b377430a42eb67e8fdfcadb53bb13b87cd971d6a9ab91d607"
|
||||
"frontend-modern/src/features/alerts/useAlertOverviewState.ts": "64d0b891e7ad228e8590da859dc25e825b6164c8cf76a01983a219d6cd079b23"
|
||||
},
|
||||
"routes": [
|
||||
"/qualification (isolated real OverviewTab, not installed /alerts)"
|
||||
@@ -27,9 +27,9 @@
|
||||
}
|
||||
],
|
||||
"states": [
|
||||
"Scripted ready and cooldown diagnoses with lastNotified render Dispatch requested; cooldown says next eligible. Missing timestamp remains Notification pending. No Notified label."
|
||||
"First diagnosis request pending; active set expanded; newer notifications-disabled response visible; older dispatch response completes without replacing current status."
|
||||
],
|
||||
"interactions": [
|
||||
"Loaded real Overview in Chromium via scripts/check-alert-dispatch-copy.mjs; asserted status text ranges fit each viewport and no page errors. Inspected desktop and phone screenshots. No delivery actions invoked; no backend receipt claimed. Screenshots retained in lane outcome evidence."
|
||||
"Ran pulse-heavy-run -- node scripts/check-alert-diagnosis-ordering.mjs in Chromium. Clicked Add alert fixture control, completed older response after current warning rendered, asserted warning retained, dispatch absent, new alert present, status text fits viewport and no page errors. Inspected desktop and phone screenshots. Synthetic API only; no delivery action or recipient receipt."
|
||||
]
|
||||
}
|
||||
|
||||
@@ -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'));
|
||||
|
||||
@@ -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,31 @@ 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('owns overview stats, filtering, and acknowledge flows outside the tab shell', async () => {
|
||||
const now = Date.now();
|
||||
const [activeAlerts] = createSignal<Record<string, Alert>>({
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
Reference in New Issue
Block a user