fix(web): retain current delivery evidence across overlapping reads

Mount and Retry reads can finish out of order, erasing current attempts and held-event evidence or hiding an unavailable result. Assign refresh ownership and ignore abandoned completions after disposal. Ordinary regression tests and six scripted Chromium cases protect ordering and loading state; this does not qualify installed notification delivery.

Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot]
2026-09-06 10:19:46 +01:00
parent 9761e5e914
commit c27c8abc77
8 changed files with 301 additions and 44 deletions
@@ -15,6 +15,17 @@
## Purpose
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
result. Attempt loading reflects only the latest attempt request; held events
remain independently asynchronous. Scope disposal abandons both reads.
Verification: ordinary overlap, held-event and disposal cases in
`useNotificationDeliveryLog.test.tsx`, registered mount/Retry coverage in
`useAlertDestinationsTabState.test.tsx`, and positive rendered-content assertions
in `scripts/check-delivery-log-ordering.mjs`. Scripted browser evidence is not
proof of backend delivery or receipt by an independent recipient.
The shared delivery-health card wraps action groups according to available
space, retaining readable explanation width when Review, Retry, Dismiss and
Refresh appear together. Its heading uses the opaque semantic foreground,
@@ -4774,6 +4774,17 @@ requests only the two held-notification event types over the same seven-day
window as its delivery-attempt log, with an explicit bounded limit. That event
request runs independently of `GET /api/notifications/delivery-log`; failure
of either API must not recast the other response as empty or unavailable.
Overlapping Destinations mount, configuration Retry and queue-action refreshes
use latest-request ownership: only the most recently started refresh may
replace delivery attempts, unavailable/loading flags or held-event rows.
An older success cannot hide a newer unavailable result, and an older failure
cannot erase newer evidence. Held-event completion remains independent of the
attempt-log loading flag. Disposing the owning Solid scope ignores pending
completions and prevents further reads through the abandoned loader.
Regression coverage lives in the delivery-log and destinations-tab state tests;
`scripts/check-delivery-log-ordering.mjs` additionally asserts positive
rendered current-attempt content in Chromium at desktop and mobile widths
using scripted APIs, not installed delivery or recipient evidence.
`GET /api/alerts/incidents` with both `alertIdentifier` and `started_at` is an
occurrence-qualified timeline read. The incident/resource-history projection
@@ -20,6 +20,18 @@
## Purpose
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
attempt-log spinner. Cleanup prevents abandoned requests from updating state
and makes subsequent calls through the disposed loader inert; it does not
cancel transport requests. Latest-request failure remains unavailable rather
than being concealed by an older successful response.
Verification combines the hook's ordinary race/disposal tests, registered
mount/Retry integration, and `scripts/check-delivery-log-ordering.mjs` Chromium
content assertions at desktop and narrow widths. This is component-level
presentation proof with scripted APIs, not full-tab or installed qualification.
The shared delivery-health card wraps action groups according to available
space, retaining readable explanation width when Review, Retry, Dismiss and
Refresh appear together. Its heading uses the opaque semantic foreground,
+9 -33
View File
@@ -1,57 +1,33 @@
{
"version": 1,
"base_sha": "61607333cc9e2fe1c1712b94ce458ac45abdabd1",
"verified_at": "2026-09-06T03:17:28.710286Z",
"base_sha": "9761e5e91457a2d007542ff61074db9ddcc427e7",
"verified_at": "2026-09-06T09:19:36.187652Z",
"result": "passed",
"changed_paths": [
"frontend-modern/src/features/alerts/AlertDeliveryHealthCard.tsx",
"frontend-modern/src/features/alerts/OverviewTab.tsx",
"frontend-modern/src/features/alerts/useNotificationDeliveryHealth.ts"
"frontend-modern/src/features/alerts/useNotificationDeliveryLog.ts"
],
"content_sha256": {
"frontend-modern/src/features/alerts/AlertDeliveryHealthCard.tsx": "dad60838804e575c0314398b304bda6d2fbd842cb2f693b3f33d168a4bce5dc1",
"frontend-modern/src/features/alerts/OverviewTab.tsx": "8f7fdc04bd0546f86152c8bb392ff3e3fb755f1f0a1c28d1ee85da3e9fe23582",
"frontend-modern/src/features/alerts/useNotificationDeliveryHealth.ts": "1eadf6df30b4f1130f868b8f139561ea60fdb87e1823deeeaaf2f8fbc699a00c"
"frontend-modern/src/features/alerts/useNotificationDeliveryLog.ts": "be919497714d749fcbe9a48a325aa82282b7afa8d6866d84eb1cc09ab52c14a6"
},
"routes": [
"/qualification (actual delivery-health and Overview components in isolated Solid Router fixtures, scripted API promises)",
"/patrol",
"/alerts"
"/qualification (real Destinations state hook and delivery-log card, scripted APIs; not full DestinationsTab)"
],
"viewports": [
{
"width": 1440,
"height": 900
},
{
"width": 900,
"height": 900
},
{
"width": 390,
"height": 900
},
{
"width": 1440,
"height": 1000
},
{
"width": 900,
"height": 1000
},
{
"width": 390,
"height": 1000
}
],
"states": [
"Delivery ordering: 18 scripted cases at three widths. Overview refresh: 12 scripted cases across light/dark themes and three widths. Unavailable health, retained Retry/Dismiss actions, pending disabled Refresh and healthy recovery. These fixtures do not qualify installed backend delivery or recipient outcomes.",
"Final card pixels inspected at desktop, intermediate and narrow widths in both themes. Heading is readable and actions wrap within the card. This is a focused card check, not a full dark-theme shell audit.",
"Ordinary and alert-mirrored Patrol findings preserve unknown cause and failed-read evidence in the investigation transcript and linked Assistant. Scripted responses qualify presentation and context preservation, not diagnostic competence."
"Four cases preserve current attempt and held-event rows after stale success/failure; two cases retain disabled refresh while latest read is pending and show unavailable after latest failure. Held-event pending does not block attempt refresh."
],
"interactions": [
"Retry and Dismiss followed by failed health read, manual Refresh while pending, healthy recovery and overlapping old/new response ordering. Geometry assertions check heading overflow and control bounds.",
"Selected issue, evidence expansion, keyboard Review and Explain, nested transcript expansion/collapse, Assistant streaming, preserved draft, reload, error/retry, menu Escape and outside dismissal at 1440/900/390 widths.",
"Private receipts: /Volumes/Development/pulse/tmp/patrol-merge-browser/final-health-ordering and final-overview-refresh, and tmp/patrol-assistant-journey/result.json. No infrastructure mutations or autonomous provider calls."
"Registered mount read, fixture Configuration Retry invoking real handleRetry, scripted out-of-order attempt and held-event completion. Six Chromium cases passed via pulse-heavy-run -- node scripts/check-delivery-log-ordering.mjs.",
"Inspected 390-error.png and 1440-pending-unavailable.png: current attempt and held rows visible on narrow layout; unavailable warning visible at desktop. Synthetic component fixture only, no backend or independent-recipient qualification.",
"Evidence: queue/staging/20260906T091009Z-web-product/evidence/browser.log and screenshots under /var/lib/pulse-maintainer."
]
}
@@ -115,6 +115,53 @@ describe('useAlertDestinationsTabState', () => {
expect(result.deliveryNeedsAttention()).toBe(true);
});
it('preserves Retry log evidence when the registered mount read fails late', async () => {
let rejectMount!: (error: Error) => void;
const current = {
entries: [],
windowDays: 30,
completedRetentionDays: 7,
deadLetterRetentionDays: 30,
};
vi.mocked(NotificationsAPI.getWebhooks).mockResolvedValue([]);
vi.mocked(NotificationsAPI.getHealth).mockResolvedValue({
queue: { status: 'healthy' },
} as never);
vi.mocked(NotificationsAPI.getDeliveryLog)
.mockReturnValueOnce(
new Promise((_resolve, reject) => {
rejectMount = reject;
}),
)
.mockResolvedValueOnce(current);
const [appriseConfig, setAppriseConfig] = createSignal(buildAppriseConfig());
const onRetryLoad = vi.fn();
const { result, cleanup } = renderHook(() =>
useAlertDestinationsTabState({
appriseConfig,
setAppriseConfig,
configLoadError: () => 'configuration unavailable',
emailConfig: () => buildEmailConfig(),
isLoadingDestinations: () => false,
isRetrying: () => false,
onRetryLoad,
}),
);
try {
await waitFor(() => expect(NotificationsAPI.getDeliveryLog).toHaveBeenCalledTimes(1));
result.handleRetry();
await waitFor(() => expect(result.deliveryLog()).toEqual(current));
expect(onRetryLoad).toHaveBeenCalledTimes(1);
rejectMount(new Error('stale mount failure'));
await Promise.resolve();
expect(result.deliveryLog()).toEqual(current);
expect(result.deliveryLogUnavailable()).toBe(false);
expect(result.refreshingDeliveryLog()).toBe(false);
} finally {
cleanup();
}
});
beforeEach(() => {
vi.mocked(AlertsAPI.getEvents).mockReset();
vi.mocked(AlertsAPI.getEvents).mockResolvedValue([]);
@@ -113,10 +113,7 @@ describe('useNotificationDeliveryLog', () => {
}
}));
// Known defect: mount and queue-action refreshes can overlap. Keep the
// desired invariant executable until the governed runtime repair lands;
// Vitest fails these tests if the invariant starts passing unexpectedly.
it.fails('keeps the newest successful read when an older read fails', () =>
it('keeps the newest successful read when an older read fails', () =>
createRoot(async (dispose) => {
const older = deferred<NotificationDeliveryLog>();
vi.mocked(NotificationsAPI.getDeliveryLog)
@@ -135,7 +132,7 @@ describe('useNotificationDeliveryLog', () => {
}
}));
it.fails('does not replace a newer unavailable result with an older success', () =>
it('does not replace a newer unavailable result with an older success', () =>
createRoot(async (dispose) => {
const older = deferred<NotificationDeliveryLog>();
vi.mocked(NotificationsAPI.getDeliveryLog)
@@ -154,7 +151,7 @@ describe('useNotificationDeliveryLog', () => {
}
}));
it.fails('keeps refreshing true while the newest read remains pending', () =>
it('keeps refreshing true while the newest read remains pending', () =>
createRoot(async (dispose) => {
const older = deferred<NotificationDeliveryLog>();
const newer = deferred<NotificationDeliveryLog>();
@@ -175,4 +172,69 @@ describe('useNotificationDeliveryLog', () => {
}
}));
it.each(['success', 'failure'])('ignores an older held-event %s after a newer read', (outcome) =>
createRoot(async (dispose) => {
const older = deferred<Awaited<ReturnType<typeof AlertsAPI.getEvents>>>();
const current = [
{
id: 2,
type: 'notification_deferred',
alertId: 'current',
occurredAt: '2026-09-06T08:00:00Z',
},
];
vi.mocked(AlertsAPI.getEvents)
.mockReturnValueOnce(older.promise)
.mockResolvedValueOnce(current);
vi.mocked(NotificationsAPI.getDeliveryLog).mockResolvedValue(emptyLog);
const state = useNotificationDeliveryLog();
try {
await state.loadDeliveryLog();
await state.loadDeliveryLog();
if (outcome === 'success') older.resolve([]);
else older.reject(new Error('stale held read'));
await Promise.resolve();
expect(state.heldEvents()).toEqual(current);
} finally {
dispose();
}
}),
);
it.each(['success', 'failure'])(
'ignores pending %s after disposal and starts no new reads',
(outcome) =>
createRoot(async (dispose) => {
const log = deferred<NotificationDeliveryLog>();
const held = deferred<Awaited<ReturnType<typeof AlertsAPI.getEvents>>>();
vi.mocked(NotificationsAPI.getDeliveryLog).mockReturnValue(log.promise);
vi.mocked(AlertsAPI.getEvents).mockReturnValue(held.promise);
const state = useNotificationDeliveryLog();
const pending = state.loadDeliveryLog();
dispose();
if (outcome === 'success') {
log.resolve(emptyLog);
held.resolve([
{
id: 1,
type: 'notification_suppressed',
alertId: 'late',
occurredAt: '2026-09-06T08:00:00Z',
},
]);
} else {
log.reject(new Error('late log'));
held.reject(new Error('late held'));
}
await pending;
await state.loadDeliveryLog();
expect(state.deliveryLog()).toBeNull();
expect(state.deliveryLogUnavailable()).toBe(false);
expect(state.heldEvents()).toEqual([]);
// Disposal freezes state, including loading; no abandoned completion owns it.
expect(state.refreshingDeliveryLog()).toBe(true);
expect(NotificationsAPI.getDeliveryLog).toHaveBeenCalledTimes(1);
expect(AlertsAPI.getEvents).toHaveBeenCalledTimes(1);
}),
);
});
@@ -1,4 +1,4 @@
import { createSignal } from 'solid-js';
import { createSignal, onCleanup } from 'solid-js';
import { AlertsAPI } from '@/api/alerts';
import { NotificationsAPI, type NotificationDeliveryLog } from '@/api/notifications';
@@ -26,7 +26,15 @@ export function useNotificationDeliveryLog() {
const [refreshingDeliveryLog, setRefreshingDeliveryLog] = createSignal(false);
const [heldEvents, setHeldEvents] = createSignal<AlertEvent[]>([]);
const loadHeldEvents = async () => {
// Both reads belong to the same refresh, but held events never block the log.
let latestRequest = 0;
let disposed = false;
onCleanup(() => {
disposed = true;
});
const ownsRequest = (request: number) => !disposed && request === latestRequest;
const loadHeldEvents = async (request: number) => {
try {
const since = new Date(
Date.now() - HELD_EVENT_WINDOW_DAYS * 24 * 60 * 60 * 1000,
@@ -36,31 +44,36 @@ export function useNotificationDeliveryLog() {
since,
limit: HELD_EVENT_LIMIT,
});
setHeldEvents(events);
if (ownsRequest(request)) setHeldEvents(events);
} catch (error) {
if (!ownsRequest(request)) return;
logger.error('Failed to load held alert notification events', error);
setHeldEvents([]);
}
};
const loadDeliveryLog = async () => {
if (disposed) return;
const request = ++latestRequest;
setRefreshingDeliveryLog(true);
// Held events refresh independently: they must never delay or fail the
// primary delivery-attempt log.
void loadHeldEvents();
void loadHeldEvents(request);
try {
// Request the server's bounded maximum. A degraded queue can retain more
// than the default page of 50 failures, and the evidence view should not
// hide them behind unrelated successful attempts when space is available.
const log = await NotificationsAPI.getDeliveryLog(DELIVERY_LOG_LIMIT);
if (!ownsRequest(request)) return;
setDeliveryLog(log);
setDeliveryLogUnavailable(false);
} catch (error) {
if (!ownsRequest(request)) return;
logger.error('Failed to load notification delivery log', error);
setDeliveryLog(null);
setDeliveryLogUnavailable(true);
} finally {
setRefreshingDeliveryLog(false);
if (ownsRequest(request)) setRefreshingDeliveryLog(false);
}
};
+125
View File
@@ -0,0 +1,125 @@
// 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, expect } from "@playwright/test";
import { fileURLToPath } from "node:url";
import { resolve } from "node:path";
import { mkdirSync } from "node:fs";
import assert from "node:assert/strict";
const root = fileURLToPath(new URL('../frontend-modern', import.meta.url));
const evidence = process.env.PULSE_BROWSER_EVIDENCE_DIR || resolve(root, '../tmp/delivery-log-ordering');
mkdirSync(evidence, { recursive: true });
process.chdir(root);
const fixture = `
import { render } from 'solid-js/web';
import { NotificationsAPI } from '/src/api/notifications';
import { AlertsAPI } from '/src/api/alerts';
import { useAlertDestinationsTabState } from '/src/features/alerts/useAlertDestinationsTabState';
import { AlertDeliveryLogCard } from '/src/features/alerts/AlertDeliveryLogCard';
import '/src/index.css';
const pending = [];
const held = [];
NotificationsAPI.getHealth = async () => ({queue:{status:'healthy'}});
NotificationsAPI.getDeliveryLog = () => new Promise((resolve, reject) => pending.push({resolve, reject}));
AlertsAPI.getEvents = () => new Promise((resolve, reject) => held.push({resolve, reject}));
window.finishHeld = (i, status) => status === 'error' ? held[i].reject(new Error('scripted held failure')) : held[i].resolve([{id:i+1,type:'notification_deferred',alertId:status,occurredAt:'2026-09-06T08:01:00Z'}]);
NotificationsAPI.retryTerminalFailures = NotificationsAPI.dismissTerminalFailures = async () => ({affected: 1});
window.confirm = () => true;
window.finish = (i, status) => status === 'error' ? pending[i].reject(new Error('scripted offline')) : pending[i].resolve({entries:[{notificationId:status,type:'email',outcome:'sent',alertIds:[status],alertCount:1,attempts:1,success:true,timestamp:'2026-09-06T08:00:00Z'}],windowDays:30,completedRetentionDays:7,deadLetterRetentionDays:30});
window.count = () => pending.length;
function Fixture() {
const s = useAlertDestinationsTabState({emailConfig:()=>({}), appriseConfig:()=>({}), setAppriseConfig:()=>{}, configLoadError:()=> 'scripted config unavailable', isRetrying:()=>false, isLoadingDestinations:()=>false, onRetryLoad:()=>{}, webhooks:()=>[]});
return <main><h1>Delivery log ordering fixture</h1><button onClick={s.handleRetry}>Configuration Retry</button><output>{String(s.refreshingDeliveryLog())}</output><AlertDeliveryLogCard log={s.deliveryLog()} unavailable={s.deliveryLogUnavailable()} refreshing={s.refreshingDeliveryLog()} onRefresh={s.loadDeliveryLog} webhooks={[]} heldEvents={s.heldEvents()}/></main>;
}
render(() => <Fixture/>, document.getElementById('root'));
`;
const server = await createServer({
root,
configFile: false,
optimizeDeps: {
noDiscovery: true,
entries: [],
esbuildOptions: { target: "esnext" },
},
esbuild: { target: "esnext" },
plugins: [
solid(),
{
name: "ordering-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="/ordering-fixture.tsx"></script>',
);
} else next();
});
},
resolveId(id) {
if (id === "/ordering-fixture.tsx") return id;
},
load(id) {
if (id === "/ordering-fixture.tsx") return fixture;
},
},
],
resolve: { alias: { "@": resolve(root, "src") } },
server: { host: "127.0.0.1", port: 5197, strictPort: true },
});
let browser;
try {
await server.listen();
browser = await chromium.launch({ headless: true });
const results = [];
for (const width of [1440,390]) {
for (const old of ['old-attempt','error']) {
const page = await browser.newPage({viewport:{width,height:900}});
await page.goto('http://127.0.0.1:5197/qualification');
await page.waitForFunction(() => window.count?.() === 1);
await page.getByRole('button',{name:'Configuration Retry',exact:true}).click();
await page.waitForFunction(() => window.count() === 2);
await page.evaluate(() => window.finish(1,'current-attempt'));
await page.getByText('current-attempt',{exact:true}).waitFor();
// Held reads must not keep the attempt refresh control disabled.
const refresh = page.getByRole('button', {name:'Refresh delivery status', exact:true});
await expect(refresh).toBeEnabled();
await page.evaluate(() => window.finishHeld(1, 'current-held'));
await page.getByText('current-held', {exact:true}).waitFor();
const before = await page.locator('main').innerText();
await page.evaluate(s => { window.finish(0,s); window.finishHeld(0,s); },old);
await page.evaluate(() => new Promise(r => requestAnimationFrame(() => requestAnimationFrame(r))));
const after = await page.locator('main').innerText();
assert.equal(after, before, 'old completion must preserve current evidence');
assert.match(after, /current-attempt/);
assert.match(after, /current-held/);
results.push({width,old,before,after,currentEvidencePreserved:before===after});
await page.screenshot({path:evidence+'/'+width+'-'+old+'.png'});
await page.close();
}
}
for (const width of [1440, 390]) {
const page = await browser.newPage({viewport:{width,height:900}});
await page.goto('http://127.0.0.1:5197/qualification');
await page.waitForFunction(() => window.count?.() === 1);
await page.getByRole('button',{name:'Configuration Retry',exact:true}).click();
await page.waitForFunction(() => window.count() === 2);
await page.evaluate(() => window.finish(0, 'old-attempt'));
const refresh = page.getByRole('button',{name:'Refresh delivery status',exact:true});
await expect(refresh).toBeDisabled();
await expect(page.locator('output')).toHaveText('true');
await expect(page.getByText('old-attempt',{exact:true})).toHaveCount(0);
await page.evaluate(() => window.finish(1, 'error'));
await expect(page.getByRole('alert')).toBeVisible();
await expect(refresh).toBeEnabled();
await page.screenshot({path:evidence+'/'+width+'-pending-unavailable.png'});
results.push({width,scenario:'old completion leaves newest pending; newest failure is unavailable',passed:true});
await page.close();
}
console.log(JSON.stringify({scope:'real Chromium, real caller/hook/card, scripted APIs; positive ordering assertions, not installed qualification',results},null,2));
} finally {
await browser?.close();
await server.close();
}