From 0a360892121d8f9839a60977e3626436c09f8114 Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 21 May 2026 11:11:24 -0700 Subject: [PATCH 1/8] =?UTF-8?q?feat(web-ui):=20async=20notifications=20?= =?UTF-8?q?=E2=80=94=20browser=20+=20in-app=20center=20(#559)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - AppNotification type + useNotifications hook (localStorage, capped at 20) - NotificationProvider/Context wraps the app via root layout - NotificationCenter (bell + dropdown) mounts in AppSidebar footer - BatchExecutionMonitor dispatches batch.completed on terminal status transition and blocker.created on per-task BLOCKED transition - execution page requests browser Notification permission once - proof page dispatches gate.run.failed for each failed gate when a proof run completes with passed=false - Browser Notification fires only when tab is hidden AND permission is granted; in-app history always records the event Tests: 10 hook tests + 8 NotificationCenter tests, all green. Full suite: 863/863 passing. Build green. --- web-ui/__mocks__/@hugeicons/react.js | 2 + web-ui/__tests__/app/proof/page.test.tsx | 13 ++ .../components/layout/AppSidebar.test.tsx | 6 + .../layout/NotificationCenter.test.tsx | 150 +++++++++++++ .../components/proof/ProofPage.test.tsx | 11 + .../__tests__/hooks/useNotifications.test.ts | 202 ++++++++++++++++++ web-ui/src/app/execution/page.tsx | 9 + web-ui/src/app/layout.tsx | 7 +- web-ui/src/app/proof/page.tsx | 16 ++ .../execution/BatchExecutionMonitor.tsx | 47 ++++ web-ui/src/components/layout/AppSidebar.tsx | 4 +- .../components/layout/NotificationCenter.tsx | 142 ++++++++++++ web-ui/src/contexts/NotificationContext.tsx | 21 ++ web-ui/src/hooks/useNotifications.ts | 126 +++++++++++ web-ui/src/types/index.ts | 17 ++ 15 files changed, 770 insertions(+), 3 deletions(-) create mode 100644 web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx create mode 100644 web-ui/src/__tests__/hooks/useNotifications.test.ts create mode 100644 web-ui/src/components/layout/NotificationCenter.tsx create mode 100644 web-ui/src/contexts/NotificationContext.tsx create mode 100644 web-ui/src/hooks/useNotifications.ts diff --git a/web-ui/__mocks__/@hugeicons/react.js b/web-ui/__mocks__/@hugeicons/react.js index d7e97eec..34f1a5d0 100644 --- a/web-ui/__mocks__/@hugeicons/react.js +++ b/web-ui/__mocks__/@hugeicons/react.js @@ -75,4 +75,6 @@ module.exports = { MoneyBag02Icon: createIconMock('MoneyBag02Icon'), Analytics01Icon: createIconMock('Analytics01Icon'), ChartLineData01Icon: createIconMock('ChartLineData01Icon'), + // NotificationCenter + Notification02Icon: createIconMock('Notification02Icon'), }; diff --git a/web-ui/__tests__/app/proof/page.test.tsx b/web-ui/__tests__/app/proof/page.test.tsx index 054e40f1..522091fb 100644 --- a/web-ui/__tests__/app/proof/page.test.tsx +++ b/web-ui/__tests__/app/proof/page.test.tsx @@ -15,6 +15,19 @@ jest.mock('@/lib/workspace-storage', () => ({ jest.mock('swr', () => ({ __esModule: true, default: jest.fn() })); +// Stub NotificationContext — gate-failure dispatch is exercised in its own tests. +jest.mock('@/contexts/NotificationContext', () => ({ + useNotificationContext: () => ({ + notifications: [], + unreadCount: 0, + addNotification: jest.fn(), + markRead: jest.fn(), + markAllRead: jest.fn(), + clearAll: jest.fn(), + }), + NotificationProvider: ({ children }: { children: React.ReactNode }) => children, +})); + import useSWR from 'swr'; import { proofApi } from '@/lib/api'; diff --git a/web-ui/__tests__/components/layout/AppSidebar.test.tsx b/web-ui/__tests__/components/layout/AppSidebar.test.tsx index 205ff8c6..2fe98e42 100644 --- a/web-ui/__tests__/components/layout/AppSidebar.test.tsx +++ b/web-ui/__tests__/components/layout/AppSidebar.test.tsx @@ -31,6 +31,12 @@ jest.mock('@/components/proof', () => ({ open ?
CaptureGlitchModal
: null, })); +// Mock NotificationCenter — it requires NotificationProvider context, which +// this isolated sidebar test does not provide. The component has its own tests. +jest.mock('@/components/layout/NotificationCenter', () => ({ + NotificationCenter: () =>
, +})); + // Mock SWR (used for blocker + session badge counts) const mockSWRData: Record = {}; jest.mock('swr', () => ({ diff --git a/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx b/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx new file mode 100644 index 00000000..60dc56e2 --- /dev/null +++ b/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx @@ -0,0 +1,150 @@ +import { render, screen, fireEvent, within } from '@testing-library/react'; +import { NotificationCenter } from '@/components/layout/NotificationCenter'; +import { NotificationProvider } from '@/contexts/NotificationContext'; +import { useNotificationContext } from '@/contexts/NotificationContext'; +import { NOTIFICATIONS_STORAGE_KEY } from '@/hooks/useNotifications'; + +// Test harness — lets us add notifications from outside the component tree. +function Harness({ children }: { children: React.ReactNode }) { + return {children}; +} + +function Adder({ buttonId, payload }: { buttonId: string; payload: { type: 'batch.completed' | 'blocker.created' | 'gate.run.failed'; message: string } }) { + const { addNotification } = useNotificationContext(); + return ( + + ); +} + +beforeEach(() => { + localStorage.clear(); +}); + +describe('NotificationCenter', () => { + it('renders a bell button with no badge when there are no unread notifications', () => { + render( + + + + ); + const bell = screen.getByRole('button', { name: /notifications/i }); + expect(bell).toBeInTheDocument(); + expect(screen.queryByTestId('notification-badge')).not.toBeInTheDocument(); + }); + + it('shows an unread badge when there are unread notifications', () => { + render( + + + + + ); + fireEvent.click(screen.getByTestId('add-1')); + expect(screen.getByTestId('notification-badge')).toHaveTextContent('1'); + }); + + it('opens the dropdown and lists notifications when the bell is clicked', () => { + render( + + + + + + ); + + fireEvent.click(screen.getByTestId('add-1')); + fireEvent.click(screen.getByTestId('add-2')); + + expect(screen.queryByText(/Batch 1 done/)).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + + expect(screen.getByText(/Batch 1 done/)).toBeInTheDocument(); + expect(screen.getByText(/Blocked: task 7/)).toBeInTheDocument(); + }); + + it('shows empty state when there are no notifications', () => { + render( + + + + ); + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + expect(screen.getByText(/no notifications/i)).toBeInTheDocument(); + }); + + it('marks a single notification read via its X button', () => { + render( + + + + + ); + fireEvent.click(screen.getByTestId('add-1')); + expect(screen.getByTestId('notification-badge')).toHaveTextContent('1'); + + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + const item = screen.getByText(/msg-x/).closest('[data-testid="notification-item"]'); + expect(item).toBeTruthy(); + const markBtn = within(item as HTMLElement).getByRole('button', { name: /mark as read/i }); + fireEvent.click(markBtn); + + expect(screen.queryByTestId('notification-badge')).not.toBeInTheDocument(); + }); + + it('marks all notifications read', () => { + render( + + + + + + ); + fireEvent.click(screen.getByTestId('add-1')); + fireEvent.click(screen.getByTestId('add-2')); + expect(screen.getByTestId('notification-badge')).toHaveTextContent('2'); + + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + fireEvent.click(screen.getByRole('button', { name: /mark all read/i })); + + expect(screen.queryByTestId('notification-badge')).not.toBeInTheDocument(); + }); + + it('clears all notifications', () => { + render( + + + + + ); + fireEvent.click(screen.getByTestId('add-1')); + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + fireEvent.click(screen.getByRole('button', { name: /clear all/i })); + + expect(screen.queryByText(/^a$/)).not.toBeInTheDocument(); + expect(screen.getByText(/no notifications/i)).toBeInTheDocument(); + }); + + it('renders notifications from existing localStorage state on mount', () => { + const stored = [ + { + id: '1', + type: 'gate.run.failed', + message: 'gate failed: unit', + timestamp: new Date().toISOString(), + read: false, + }, + ]; + localStorage.setItem(NOTIFICATIONS_STORAGE_KEY, JSON.stringify(stored)); + + render( + + + + ); + expect(screen.getByTestId('notification-badge')).toHaveTextContent('1'); + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + expect(screen.getByText(/gate failed: unit/)).toBeInTheDocument(); + }); +}); diff --git a/web-ui/src/__tests__/components/proof/ProofPage.test.tsx b/web-ui/src/__tests__/components/proof/ProofPage.test.tsx index 92052d8d..7bd4621b 100644 --- a/web-ui/src/__tests__/components/proof/ProofPage.test.tsx +++ b/web-ui/src/__tests__/components/proof/ProofPage.test.tsx @@ -20,6 +20,17 @@ jest.mock('@/lib/api', () => ({ waiveRequirement: jest.fn(), }, })); +jest.mock('@/contexts/NotificationContext', () => ({ + useNotificationContext: () => ({ + notifications: [], + unreadCount: 0, + addNotification: jest.fn(), + markRead: jest.fn(), + markAllRead: jest.fn(), + clearAll: jest.fn(), + }), + NotificationProvider: ({ children }: { children: React.ReactNode }) => children, +})); jest.mock('@/components/proof', () => ({ ProofStatusBadge: ({ status }: { status: string }) => {status}, WaiveDialog: () => null, diff --git a/web-ui/src/__tests__/hooks/useNotifications.test.ts b/web-ui/src/__tests__/hooks/useNotifications.test.ts new file mode 100644 index 00000000..6eb81e23 --- /dev/null +++ b/web-ui/src/__tests__/hooks/useNotifications.test.ts @@ -0,0 +1,202 @@ +import { renderHook, act } from '@testing-library/react'; +import { useNotifications, NOTIFICATIONS_STORAGE_KEY, MAX_NOTIFICATIONS } from '@/hooks/useNotifications'; + +// Helper to mock Notification API + visibility +type PermissionState = NotificationPermission; + +let currentPermission: PermissionState = 'default'; + +function setNotificationPermission(value: PermissionState) { + currentPermission = value; +} + +function setVisibility(value: DocumentVisibilityState) { + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: () => value, + }); +} + +const NotificationCtor = jest.fn(); + +// Stub the Notification global with a function plus a `permission` getter. +function installNotificationStub() { + const stub = function (this: unknown, title: string, options?: NotificationOptions) { + NotificationCtor(title, options); + } as unknown as typeof Notification; + Object.defineProperty(stub, 'permission', { + configurable: true, + get: () => currentPermission, + }); + Object.defineProperty(stub, 'requestPermission', { + configurable: true, + value: jest.fn().mockResolvedValue('granted'), + }); + (global as unknown as { Notification: typeof Notification }).Notification = stub; +} + +beforeEach(() => { + localStorage.clear(); + NotificationCtor.mockClear(); + currentPermission = 'default'; + installNotificationStub(); + setVisibility('visible'); +}); + +describe('useNotifications', () => { + it('starts empty when no localStorage entry exists', () => { + const { result } = renderHook(() => useNotifications()); + expect(result.current.notifications).toEqual([]); + expect(result.current.unreadCount).toBe(0); + }); + + it('hydrates from localStorage on mount', () => { + const stored = [ + { + id: '1', + type: 'batch.completed', + message: 'Batch finished', + timestamp: new Date().toISOString(), + read: false, + }, + ]; + localStorage.setItem(NOTIFICATIONS_STORAGE_KEY, JSON.stringify(stored)); + const { result } = renderHook(() => useNotifications()); + expect(result.current.notifications).toHaveLength(1); + expect(result.current.notifications[0].id).toBe('1'); + expect(result.current.unreadCount).toBe(1); + }); + + it('prepends a new notification (newest first) and persists', () => { + const { result } = renderHook(() => useNotifications()); + + act(() => { + result.current.addNotification({ + type: 'batch.completed', + message: 'Batch 1 done', + }); + }); + + expect(result.current.notifications).toHaveLength(1); + expect(result.current.notifications[0].message).toBe('Batch 1 done'); + expect(result.current.notifications[0].read).toBe(false); + + act(() => { + result.current.addNotification({ + type: 'blocker.created', + message: 'Blocked', + }); + }); + + expect(result.current.notifications).toHaveLength(2); + expect(result.current.notifications[0].message).toBe('Blocked'); + expect(result.current.notifications[1].message).toBe('Batch 1 done'); + + const persisted = JSON.parse(localStorage.getItem(NOTIFICATIONS_STORAGE_KEY)!); + expect(persisted).toHaveLength(2); + }); + + it(`caps history at ${MAX_NOTIFICATIONS} entries`, () => { + const { result } = renderHook(() => useNotifications()); + act(() => { + for (let i = 0; i < MAX_NOTIFICATIONS + 5; i++) { + result.current.addNotification({ + type: 'batch.completed', + message: `msg ${i}`, + }); + } + }); + expect(result.current.notifications).toHaveLength(MAX_NOTIFICATIONS); + // Newest first → first element should be the last we added + expect(result.current.notifications[0].message).toBe(`msg ${MAX_NOTIFICATIONS + 4}`); + }); + + it('marks a single notification as read', () => { + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'a' }); + result.current.addNotification({ type: 'batch.completed', message: 'b' }); + }); + expect(result.current.unreadCount).toBe(2); + + const firstId = result.current.notifications[0].id; + act(() => { + result.current.markRead(firstId); + }); + expect(result.current.unreadCount).toBe(1); + expect(result.current.notifications.find((n) => n.id === firstId)?.read).toBe(true); + }); + + it('marks all notifications as read', () => { + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'a' }); + result.current.addNotification({ type: 'blocker.created', message: 'b' }); + }); + act(() => { + result.current.markAllRead(); + }); + expect(result.current.unreadCount).toBe(0); + }); + + it('clears all notifications and persists empty state', () => { + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'a' }); + }); + expect(result.current.notifications).toHaveLength(1); + + act(() => { + result.current.clearAll(); + }); + expect(result.current.notifications).toHaveLength(0); + const persisted = JSON.parse(localStorage.getItem(NOTIFICATIONS_STORAGE_KEY)!); + expect(persisted).toEqual([]); + }); + + it('fires a browser Notification when tab is hidden and permission is granted', () => { + setVisibility('hidden'); + setNotificationPermission('granted'); + + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ + type: 'batch.completed', + message: 'Batch 7 done', + }); + }); + + expect(NotificationCtor).toHaveBeenCalledTimes(1); + const [title, options] = NotificationCtor.mock.calls[0]; + expect(typeof title).toBe('string'); + expect(options.body).toBe('Batch 7 done'); + }); + + it('does NOT fire a browser Notification when tab is visible', () => { + setVisibility('visible'); + setNotificationPermission('granted'); + + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ + type: 'batch.completed', + message: 'Batch 8 done', + }); + }); + expect(NotificationCtor).not.toHaveBeenCalled(); + }); + + it('does NOT fire a browser Notification when permission is denied', () => { + setVisibility('hidden'); + setNotificationPermission('denied'); + + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ + type: 'batch.completed', + message: 'Batch 9 done', + }); + }); + expect(NotificationCtor).not.toHaveBeenCalled(); + }); +}); diff --git a/web-ui/src/app/execution/page.tsx b/web-ui/src/app/execution/page.tsx index ab6ffc07..6e06fea5 100644 --- a/web-ui/src/app/execution/page.tsx +++ b/web-ui/src/app/execution/page.tsx @@ -53,6 +53,15 @@ function ExecutionLandingContent() { setWorkspaceReady(true); }, []); + // Request browser notification permission once on first visit + useEffect(() => { + if (typeof window === 'undefined' || typeof Notification === 'undefined') return; + if (Notification.permission === 'default') { + // Fire-and-forget — browsers handle the UI; we honor whatever the user picks. + Notification.requestPermission().catch(() => {}); + } + }, []); + // If ?task= is present, redirect immediately useEffect(() => { if (taskIdParam) { diff --git a/web-ui/src/app/layout.tsx b/web-ui/src/app/layout.tsx index aeecae36..1d763791 100644 --- a/web-ui/src/app/layout.tsx +++ b/web-ui/src/app/layout.tsx @@ -2,6 +2,7 @@ import type { Metadata } from 'next'; import { Nunito_Sans } from 'next/font/google'; import { Toaster } from 'sonner'; import { AppLayout } from '@/components/layout'; +import { NotificationProvider } from '@/contexts/NotificationContext'; import './globals.css'; const nunitoSans = Nunito_Sans({ @@ -31,8 +32,10 @@ export default function RootLayout({ return ( - {children} - + + {children} + + ); diff --git a/web-ui/src/app/proof/page.tsx b/web-ui/src/app/proof/page.tsx index f5db6219..64ce2b39 100644 --- a/web-ui/src/app/proof/page.tsx +++ b/web-ui/src/app/proof/page.tsx @@ -15,6 +15,7 @@ import { Button } from '@/components/ui/button'; import { ProofStatusBadge, WaiveDialog, GateRunPanel, GateRunBanner, RunHistoryPanel, GateEvidencePanel, CaptureGlitchModal } from '@/components/proof'; import { proofApi } from '@/lib/api'; import { useProofRun } from '@/hooks/useProofRun'; +import { useNotificationContext } from '@/contexts/NotificationContext'; import { getSelectedWorkspacePath } from '@/lib/workspace-storage'; import type { ProofRequirement, ProofRequirementListResponse, ProofReqStatus, ProofSeverity, ProofRunDetail } from '@/types'; @@ -107,6 +108,21 @@ function ProofPageContent() { const [selectedRunId, setSelectedRunId] = useState(null); const { runState, gateEntries, passed, runMessage, errorMessage, startRun, retry } = useProofRun(); + const { addNotification } = useNotificationContext(); + + // Notify on each gate failure when a run completes with passed=false + useEffect(() => { + if (runState !== 'complete' || passed !== false) return; + for (const entry of gateEntries) { + if (entry.status === 'failed') { + addNotification({ + type: 'gate.run.failed', + message: `Gate run failed: ${entry.gate}`, + gateName: entry.gate, + }); + } + } + }, [runState, passed, gateEntries, addNotification]); // Sort state (default: status asc → open first, then severity) const [sortCol, setSortCol] = useState('status'); diff --git a/web-ui/src/components/execution/BatchExecutionMonitor.tsx b/web-ui/src/components/execution/BatchExecutionMonitor.tsx index cf560bd9..3de6794d 100644 --- a/web-ui/src/components/execution/BatchExecutionMonitor.tsx +++ b/web-ui/src/components/execution/BatchExecutionMonitor.tsx @@ -24,6 +24,7 @@ import { import { batchesApi, tasksApi } from '@/lib/api'; import { EventStream } from './EventStream'; import { useExecutionMonitor } from '@/hooks/useExecutionMonitor'; +import { useNotificationContext } from '@/contexts/NotificationContext'; import type { BatchResponse, Task } from '@/types'; // ── Status icon helper ──────────────────────────────────────────────── @@ -50,6 +51,7 @@ interface BatchExecutionMonitorProps { export function BatchExecutionMonitor({ batchId, workspacePath }: BatchExecutionMonitorProps) { const router = useRouter(); + const { addNotification } = useNotificationContext(); const [batch, setBatch] = useState(null); const [tasks, setTasks] = useState>({}); const [expandedTaskId, setExpandedTaskId] = useState(null); @@ -59,6 +61,10 @@ export function BatchExecutionMonitor({ batchId, workspacePath }: BatchExecution // Track which task IDs have already been fetched to avoid refetching const fetchedTaskIdsRef = useRef>(new Set()); + // Track previous batch + per-task statuses for transition-based notifications + const prevBatchStatusRef = useRef(null); + const prevTaskStatusesRef = useRef>({}); + // ── Fetch batch details + task names ──────────────────────────────── const fetchBatch = useCallback(async () => { try { @@ -98,6 +104,47 @@ export function BatchExecutionMonitor({ batchId, workspacePath }: BatchExecution }; }, [batch?.status, fetchBatch]); + // Fire notifications on batch terminal transition + per-task BLOCKED transitions + useEffect(() => { + if (!batch) return; + + const TERMINAL = ['COMPLETED', 'FAILED', 'CANCELLED']; + const prevBatchStatus = prevBatchStatusRef.current; + if ( + prevBatchStatus !== null && + !TERMINAL.includes(prevBatchStatus) && + TERMINAL.includes(batch.status) + ) { + const completedCount = batch.task_ids.filter( + (id) => batch.results[id] === 'COMPLETED' || batch.results[id] === 'DONE' + ).length; + addNotification({ + type: 'batch.completed', + message: `Batch ${batchId.slice(0, 8)} finished — ${completedCount}/${batch.task_ids.length} tasks done`, + batchId, + }); + } + prevBatchStatusRef.current = batch.status; + + // Per-task: notify on transition to BLOCKED + const prevTaskStatuses = prevTaskStatusesRef.current; + for (const taskId of batch.task_ids) { + const currentStatus = batch.results[taskId]; + const prevStatus = prevTaskStatuses[taskId]; + if (currentStatus === 'BLOCKED' && prevStatus && prevStatus !== 'BLOCKED') { + const title = tasks[taskId]?.title; + addNotification({ + type: 'blocker.created', + message: title + ? `Agent is blocked on "${title}" — your input needed` + : 'Agent is blocked — your input needed', + taskId, + }); + } + prevTaskStatuses[taskId] = currentStatus ?? 'READY'; + } + }, [batch, batchId, tasks, addNotification]); + // Auto-expand the first IN_PROGRESS task useEffect(() => { if (!batch || expandedTaskId) return; diff --git a/web-ui/src/components/layout/AppSidebar.tsx b/web-ui/src/components/layout/AppSidebar.tsx index 88c36e18..f1c1bbcd 100644 --- a/web-ui/src/components/layout/AppSidebar.tsx +++ b/web-ui/src/components/layout/AppSidebar.tsx @@ -20,6 +20,7 @@ import { import { getSelectedWorkspacePath } from '@/lib/workspace-storage'; import { blockersApi, sessionsApi } from '@/lib/api'; import { CaptureGlitchModal } from '@/components/proof'; +import { NotificationCenter } from './NotificationCenter'; import type { BlockerListResponse, SessionListResponse, ProofRequirement } from '@/types'; interface NavItem { @@ -146,8 +147,9 @@ export function AppSidebar() { })} - {/* Capture Glitch action */} + {/* Sidebar footer: notifications + capture glitch */}
+ + + {open && ( +
+
+ Notifications +
+ + +
+
+ +
+ {notifications.length === 0 ? ( +

No notifications yet

+ ) : ( +
    + {notifications.map((n) => ( + + ))} +
+ )} +
+
+ )} +
+ ); +} + +function NotificationRow({ + notification, + onMarkRead, +}: { + notification: AppNotification; + onMarkRead: (id: string) => void; +}) { + const Icon = TYPE_ICONS[notification.type]; + const colorClass = TYPE_COLOR[notification.type]; + return ( +
  • +
    • + ); +} diff --git a/web-ui/src/contexts/NotificationContext.tsx b/web-ui/src/contexts/NotificationContext.tsx new file mode 100644 index 00000000..246b572e --- /dev/null +++ b/web-ui/src/contexts/NotificationContext.tsx @@ -0,0 +1,21 @@ +'use client'; + +import { createContext, useContext, type ReactNode } from 'react'; +import { useNotifications, type UseNotificationsReturn } from '@/hooks/useNotifications'; + +const NotificationContext = createContext(null); + +export function NotificationProvider({ children }: { children: ReactNode }) { + const value = useNotifications(); + return ( + {children} + ); +} + +export function useNotificationContext(): UseNotificationsReturn { + const ctx = useContext(NotificationContext); + if (!ctx) { + throw new Error('useNotificationContext must be used inside '); + } + return ctx; +} diff --git a/web-ui/src/hooks/useNotifications.ts b/web-ui/src/hooks/useNotifications.ts new file mode 100644 index 00000000..c4a77719 --- /dev/null +++ b/web-ui/src/hooks/useNotifications.ts @@ -0,0 +1,126 @@ +'use client'; + +import { useCallback, useEffect, useState } from 'react'; +import type { AppNotification, AppNotificationType } from '@/types'; + +export const NOTIFICATIONS_STORAGE_KEY = 'codeframe_notifications'; +export const MAX_NOTIFICATIONS = 20; + +const TITLES: Record = { + 'batch.completed': 'Batch complete', + 'blocker.created': 'Blocker needs input', + 'gate.run.failed': 'Gate run failed', +}; + +export interface AddNotificationInput { + type: AppNotificationType; + message: string; + batchId?: string; + taskId?: string; + gateName?: string; +} + +export interface UseNotificationsReturn { + notifications: AppNotification[]; + unreadCount: number; + addNotification: (input: AddNotificationInput) => void; + markRead: (id: string) => void; + markAllRead: () => void; + clearAll: () => void; +} + +function readStored(): AppNotification[] { + if (typeof window === 'undefined') return []; + const raw = localStorage.getItem(NOTIFICATIONS_STORAGE_KEY); + if (!raw) return []; + try { + const parsed = JSON.parse(raw) as AppNotification[]; + return Array.isArray(parsed) ? parsed.slice(0, MAX_NOTIFICATIONS) : []; + } catch { + return []; + } +} + +function persist(notifications: AppNotification[]): void { + if (typeof window === 'undefined') return; + localStorage.setItem(NOTIFICATIONS_STORAGE_KEY, JSON.stringify(notifications)); +} + +function generateId(): string { + if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') { + return crypto.randomUUID(); + } + return `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 10)}`; +} + +function maybeFireBrowserNotification(n: AppNotification): void { + if (typeof window === 'undefined') return; + if (typeof Notification === 'undefined') return; + if (document.visibilityState === 'visible') return; + if (Notification.permission !== 'granted') return; + try { + new Notification(TITLES[n.type], { + body: n.message, + icon: '/favicon.ico', + tag: n.id, + }); + } catch { + // Some browsers throw if Notification is invoked from an insecure context; + // ignore — the in-app history still records the event. + } +} + +export function useNotifications(): UseNotificationsReturn { + const [notifications, setNotifications] = useState([]); + + // Hydrate on mount (avoid SSR mismatch by reading inside useEffect) + useEffect(() => { + setNotifications(readStored()); + }, []); + + const addNotification = useCallback((input: AddNotificationInput) => { + setNotifications((prev) => { + const entry: AppNotification = { + id: generateId(), + type: input.type, + message: input.message, + timestamp: new Date().toISOString(), + read: false, + batchId: input.batchId, + taskId: input.taskId, + gateName: input.gateName, + }; + const next = [entry, ...prev].slice(0, MAX_NOTIFICATIONS); + persist(next); + maybeFireBrowserNotification(entry); + return next; + }); + }, []); + + const markRead = useCallback((id: string) => { + setNotifications((prev) => { + const next = prev.map((n) => (n.id === id ? { ...n, read: true } : n)); + persist(next); + return next; + }); + }, []); + + const markAllRead = useCallback(() => { + setNotifications((prev) => { + const next = prev.map((n) => ({ ...n, read: true })); + persist(next); + return next; + }); + }, []); + + const clearAll = useCallback(() => { + setNotifications(() => { + persist([]); + return []; + }); + }, []); + + const unreadCount = notifications.reduce((acc, n) => acc + (n.read ? 0 : 1), 0); + + return { notifications, unreadCount, addNotification, markRead, markAllRead, clearAll }; +} diff --git a/web-ui/src/types/index.ts b/web-ui/src/types/index.ts index 3a3e3589..c7fe960e 100644 --- a/web-ui/src/types/index.ts +++ b/web-ui/src/types/index.ts @@ -660,3 +660,20 @@ export interface AgentCostsResponse { total_input_tokens: number; total_output_tokens: number; } + +// Async notifications (issue #559) +export type AppNotificationType = + | 'batch.completed' + | 'blocker.created' + | 'gate.run.failed'; + +export interface AppNotification { + id: string; + type: AppNotificationType; + message: string; + timestamp: string; + read: boolean; + batchId?: string; + taskId?: string; + gateName?: string; +} From 56e2b1533f0d0f6ac74acb5e4a41d3f989e9f59e Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 21 May 2026 11:23:27 -0700 Subject: [PATCH 2/8] fix(web-ui): cross-family review fixes for #559 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review findings: - Distinguish FAILED/CANCELLED batches from successful completion in both the in-app history (status-specific message, non-success icon) and the browser Notification body. Title relaxed to "Batch finished" so it is accurate for all terminal states. - Scope persisted notifications to the active workspace via a workspace-keyed localStorage entry; switch workspaces → see only that workspace's notifications. Hook subscribes to the existing `workspaceChanged` + `storage` events to reload on transitions. Added tests: - persists batchStatus on the stored notification - stores under workspace-scoped key when a workspace is selected - does not leak notifications across workspaces (switch A → B → A) - does not render success icon for a FAILED batch notification Tests: 867/867 passing (was 863). Build green. Note: P1 finding (notifications only fire while BatchExecutionMonitor is mounted) acknowledged as a Known Limitation — fixing it requires a global background poller architecture beyond this PR's scope. Will be documented in the PR body. --- .../layout/NotificationCenter.test.tsx | 26 ++++ .../__tests__/hooks/useNotifications.test.ts | 75 +++++++++++- .../execution/BatchExecutionMonitor.tsx | 11 +- .../components/layout/NotificationCenter.tsx | 24 ++-- web-ui/src/hooks/useNotifications.ts | 113 ++++++++++++------ web-ui/src/types/index.ts | 6 + 6 files changed, 200 insertions(+), 55 deletions(-) diff --git a/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx b/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx index 60dc56e2..92b662aa 100644 --- a/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx +++ b/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx @@ -126,6 +126,32 @@ describe('NotificationCenter', () => { expect(screen.getByText(/no notifications/i)).toBeInTheDocument(); }); + it('does not render a green checkmark for a FAILED batch notification', () => { + // Regression for codex review finding: FAILED/CANCELLED must not look like success. + const stored = [ + { + id: '1', + type: 'batch.completed', + batchStatus: 'FAILED', + message: 'Batch X failed — 2/5 tasks completed before failure', + timestamp: new Date().toISOString(), + read: false, + }, + ]; + localStorage.setItem(NOTIFICATIONS_STORAGE_KEY, JSON.stringify(stored)); + + render( + + + + ); + fireEvent.click(screen.getByRole('button', { name: /notifications/i })); + const item = screen.getByText(/Batch X failed/).closest('[data-testid="notification-item"]'); + expect(item).toBeTruthy(); + // The success icon must not be present in a failed-batch row + expect(within(item as HTMLElement).queryByTestId('icon-CheckmarkCircle01Icon')).toBeNull(); + }); + it('renders notifications from existing localStorage state on mount', () => { const stored = [ { diff --git a/web-ui/src/__tests__/hooks/useNotifications.test.ts b/web-ui/src/__tests__/hooks/useNotifications.test.ts index 6eb81e23..a6cf5faa 100644 --- a/web-ui/src/__tests__/hooks/useNotifications.test.ts +++ b/web-ui/src/__tests__/hooks/useNotifications.test.ts @@ -1,5 +1,11 @@ import { renderHook, act } from '@testing-library/react'; -import { useNotifications, NOTIFICATIONS_STORAGE_KEY, MAX_NOTIFICATIONS } from '@/hooks/useNotifications'; +import { + useNotifications, + NOTIFICATIONS_STORAGE_KEY, + NOTIFICATIONS_STORAGE_KEY_PREFIX, + MAX_NOTIFICATIONS, +} from '@/hooks/useNotifications'; +import { setSelectedWorkspacePath, clearSelectedWorkspacePath } from '@/lib/workspace-storage'; // Helper to mock Notification API + visibility type PermissionState = NotificationPermission; @@ -199,4 +205,71 @@ describe('useNotifications', () => { }); expect(NotificationCtor).not.toHaveBeenCalled(); }); + + it('persists batchStatus on the stored notification', () => { + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ + type: 'batch.completed', + batchStatus: 'FAILED', + message: 'Batch X failed — 2/5 tasks completed before failure', + }); + }); + expect(result.current.notifications[0].batchStatus).toBe('FAILED'); + const persisted = JSON.parse(localStorage.getItem(NOTIFICATIONS_STORAGE_KEY)!); + expect(persisted[0].batchStatus).toBe('FAILED'); + }); + + describe('workspace scoping', () => { + afterEach(() => { + clearSelectedWorkspacePath(); + }); + + it('stores notifications under a workspace-scoped key when a workspace is selected', () => { + setSelectedWorkspacePath('/workspace/A'); + const { result } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'workspace A msg' }); + }); + + const wsKey = `${NOTIFICATIONS_STORAGE_KEY_PREFIX}_${encodeURIComponent('/workspace/A')}`; + const persisted = JSON.parse(localStorage.getItem(wsKey)!); + expect(persisted).toHaveLength(1); + expect(persisted[0].message).toBe('workspace A msg'); + expect(persisted[0].workspacePath).toBe('/workspace/A'); + // The global key must remain untouched + expect(localStorage.getItem(NOTIFICATIONS_STORAGE_KEY)).toBeNull(); + }); + + it('does not leak notifications across workspaces', () => { + setSelectedWorkspacePath('/workspace/A'); + const { result, rerender } = renderHook(() => useNotifications()); + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'A1' }); + result.current.addNotification({ type: 'blocker.created', message: 'A2' }); + }); + expect(result.current.notifications).toHaveLength(2); + + // Switch workspaces + act(() => { + setSelectedWorkspacePath('/workspace/B'); + }); + rerender(); + expect(result.current.notifications).toHaveLength(0); + + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'B1' }); + }); + expect(result.current.notifications).toHaveLength(1); + expect(result.current.notifications[0].message).toBe('B1'); + + // Switch back — workspace A notifications should still be there + act(() => { + setSelectedWorkspacePath('/workspace/A'); + }); + rerender(); + expect(result.current.notifications).toHaveLength(2); + expect(result.current.notifications[0].message).toBe('A2'); + }); + }); }); diff --git a/web-ui/src/components/execution/BatchExecutionMonitor.tsx b/web-ui/src/components/execution/BatchExecutionMonitor.tsx index 3de6794d..d758de1f 100644 --- a/web-ui/src/components/execution/BatchExecutionMonitor.tsx +++ b/web-ui/src/components/execution/BatchExecutionMonitor.tsx @@ -118,9 +118,18 @@ export function BatchExecutionMonitor({ batchId, workspacePath }: BatchExecution const completedCount = batch.task_ids.filter( (id) => batch.results[id] === 'COMPLETED' || batch.results[id] === 'DONE' ).length; + const total = batch.task_ids.length; + const shortId = batchId.slice(0, 8); + const outcomeMessage = + batch.status === 'COMPLETED' + ? `Batch ${shortId} finished — ${completedCount}/${total} tasks done` + : batch.status === 'FAILED' + ? `Batch ${shortId} failed — ${completedCount}/${total} tasks completed before failure` + : `Batch ${shortId} cancelled — ${completedCount}/${total} tasks completed`; addNotification({ type: 'batch.completed', - message: `Batch ${batchId.slice(0, 8)} finished — ${completedCount}/${batch.task_ids.length} tasks done`, + batchStatus: batch.status as 'COMPLETED' | 'FAILED' | 'CANCELLED', + message: outcomeMessage, batchId, }); } diff --git a/web-ui/src/components/layout/NotificationCenter.tsx b/web-ui/src/components/layout/NotificationCenter.tsx index ab587786..13ac152d 100644 --- a/web-ui/src/components/layout/NotificationCenter.tsx +++ b/web-ui/src/components/layout/NotificationCenter.tsx @@ -9,19 +9,16 @@ import { } from '@hugeicons/react'; import { formatDistanceToNow } from 'date-fns'; import { useNotificationContext } from '@/contexts/NotificationContext'; -import type { AppNotification, AppNotificationType } from '@/types'; +import type { AppNotification } from '@/types'; -const TYPE_ICONS: Record = { - 'batch.completed': CheckmarkCircle01Icon, - 'blocker.created': Alert02Icon, - 'gate.run.failed': Alert02Icon, -}; - -const TYPE_COLOR: Record = { - 'batch.completed': 'text-green-600', - 'blocker.created': 'text-amber-600', - 'gate.run.failed': 'text-red-600', -}; +function iconForNotification(n: AppNotification) { + if (n.type === 'batch.completed' && n.batchStatus && n.batchStatus !== 'COMPLETED') { + return { Icon: Cancel01Icon, color: n.batchStatus === 'FAILED' ? 'text-red-600' : 'text-muted-foreground' }; + } + if (n.type === 'batch.completed') return { Icon: CheckmarkCircle01Icon, color: 'text-green-600' }; + if (n.type === 'blocker.created') return { Icon: Alert02Icon, color: 'text-amber-600' }; + return { Icon: Alert02Icon, color: 'text-red-600' }; +} function formatTimestamp(iso: string): string { try { @@ -115,8 +112,7 @@ function NotificationRow({ notification: AppNotification; onMarkRead: (id: string) => void; }) { - const Icon = TYPE_ICONS[notification.type]; - const colorClass = TYPE_COLOR[notification.type]; + const { Icon, color: colorClass } = iconForNotification(notification); return (
  • = { - 'batch.completed': 'Batch complete', + 'batch.completed': 'Batch finished', 'blocker.created': 'Blocker needs input', 'gate.run.failed': 'Gate run failed', }; +function storageKeyFor(workspacePath: string | null): string { + if (!workspacePath) return NOTIFICATIONS_GLOBAL_STORAGE_KEY; + return `${NOTIFICATIONS_STORAGE_KEY_PREFIX}_${encodeURIComponent(workspacePath)}`; +} + export interface AddNotificationInput { type: AppNotificationType; message: string; batchId?: string; + batchStatus?: AppNotificationBatchStatus; taskId?: string; gateName?: string; } @@ -29,9 +40,9 @@ export interface UseNotificationsReturn { clearAll: () => void; } -function readStored(): AppNotification[] { +function readStored(key: string): AppNotification[] { if (typeof window === 'undefined') return []; - const raw = localStorage.getItem(NOTIFICATIONS_STORAGE_KEY); + const raw = localStorage.getItem(key); if (!raw) return []; try { const parsed = JSON.parse(raw) as AppNotification[]; @@ -41,9 +52,9 @@ function readStored(): AppNotification[] { } } -function persist(notifications: AppNotification[]): void { +function persist(key: string, notifications: AppNotification[]): void { if (typeof window === 'undefined') return; - localStorage.setItem(NOTIFICATIONS_STORAGE_KEY, JSON.stringify(notifications)); + localStorage.setItem(key, JSON.stringify(notifications)); } function generateId(): string { @@ -71,54 +82,78 @@ function maybeFireBrowserNotification(n: AppNotification): void { } export function useNotifications(): UseNotificationsReturn { + const [workspacePath, setWorkspacePath] = useState(null); const [notifications, setNotifications] = useState([]); - // Hydrate on mount (avoid SSR mismatch by reading inside useEffect) + // Hydrate from localStorage when the workspace changes useEffect(() => { - setNotifications(readStored()); - }, []); - - const addNotification = useCallback((input: AddNotificationInput) => { - setNotifications((prev) => { - const entry: AppNotification = { - id: generateId(), - type: input.type, - message: input.message, - timestamp: new Date().toISOString(), - read: false, - batchId: input.batchId, - taskId: input.taskId, - gateName: input.gateName, - }; - const next = [entry, ...prev].slice(0, MAX_NOTIFICATIONS); - persist(next); - maybeFireBrowserNotification(entry); - return next; - }); + if (typeof window === 'undefined') return; + const path = getSelectedWorkspacePath(); + setWorkspacePath(path); + setNotifications(readStored(storageKeyFor(path))); + + const handleWorkspaceChange = () => { + const next = getSelectedWorkspacePath(); + setWorkspacePath(next); + setNotifications(readStored(storageKeyFor(next))); + }; + window.addEventListener('storage', handleWorkspaceChange); + window.addEventListener('workspaceChanged', handleWorkspaceChange); + return () => { + window.removeEventListener('storage', handleWorkspaceChange); + window.removeEventListener('workspaceChanged', handleWorkspaceChange); + }; }, []); - const markRead = useCallback((id: string) => { - setNotifications((prev) => { - const next = prev.map((n) => (n.id === id ? { ...n, read: true } : n)); - persist(next); - return next; - }); - }, []); + const addNotification = useCallback( + (input: AddNotificationInput) => { + setNotifications((prev) => { + const entry: AppNotification = { + id: generateId(), + type: input.type, + message: input.message, + timestamp: new Date().toISOString(), + read: false, + batchId: input.batchId, + batchStatus: input.batchStatus, + taskId: input.taskId, + gateName: input.gateName, + workspacePath: workspacePath ?? undefined, + }; + const next = [entry, ...prev].slice(0, MAX_NOTIFICATIONS); + persist(storageKeyFor(workspacePath), next); + maybeFireBrowserNotification(entry); + return next; + }); + }, + [workspacePath] + ); + + const markRead = useCallback( + (id: string) => { + setNotifications((prev) => { + const next = prev.map((n) => (n.id === id ? { ...n, read: true } : n)); + persist(storageKeyFor(workspacePath), next); + return next; + }); + }, + [workspacePath] + ); const markAllRead = useCallback(() => { setNotifications((prev) => { const next = prev.map((n) => ({ ...n, read: true })); - persist(next); + persist(storageKeyFor(workspacePath), next); return next; }); - }, []); + }, [workspacePath]); const clearAll = useCallback(() => { setNotifications(() => { - persist([]); + persist(storageKeyFor(workspacePath), []); return []; }); - }, []); + }, [workspacePath]); const unreadCount = notifications.reduce((acc, n) => acc + (n.read ? 0 : 1), 0); diff --git a/web-ui/src/types/index.ts b/web-ui/src/types/index.ts index c7fe960e..bf43542d 100644 --- a/web-ui/src/types/index.ts +++ b/web-ui/src/types/index.ts @@ -667,6 +667,8 @@ export type AppNotificationType = | 'blocker.created' | 'gate.run.failed'; +export type AppNotificationBatchStatus = 'COMPLETED' | 'FAILED' | 'CANCELLED'; + export interface AppNotification { id: string; type: AppNotificationType; @@ -674,6 +676,10 @@ export interface AppNotification { timestamp: string; read: boolean; batchId?: string; + /** Final batch status — set when type is 'batch.completed'. */ + batchStatus?: AppNotificationBatchStatus; taskId?: string; gateName?: string; + /** Workspace this notification belongs to. Populated automatically by the hook. */ + workspacePath?: string; } From ca191081446af134cd28e3c73359962cfcc3b1ee Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 21 May 2026 11:29:14 -0700 Subject: [PATCH 3/8] test(web-ui): add permission-request unit test for #559 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Outcome evidence for "Permission request shown once and respected" — the useEffect on the execution landing page must call Notification.requestPermission when permission is 'default' and not at all when 'granted' or 'denied'. --- .../app/execution/permission-request.test.tsx | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 web-ui/src/__tests__/app/execution/permission-request.test.tsx diff --git a/web-ui/src/__tests__/app/execution/permission-request.test.tsx b/web-ui/src/__tests__/app/execution/permission-request.test.tsx new file mode 100644 index 00000000..2247e06c --- /dev/null +++ b/web-ui/src/__tests__/app/execution/permission-request.test.tsx @@ -0,0 +1,54 @@ +/** + * Verifies the execution landing page requests Notification permission + * exactly once, only when permission is 'default', and not at all when + * already granted or denied. (#559 acceptance criterion: "Permission + * request shown once and respected".) + */ +import { render } from '@testing-library/react'; +import ExecutionLandingPage from '@/app/execution/page'; + +jest.mock('@/lib/workspace-storage', () => ({ + getSelectedWorkspacePath: jest.fn(() => null), +})); +jest.mock('next/navigation', () => ({ + useSearchParams: () => new URLSearchParams(), + useRouter: () => ({ replace: jest.fn(), push: jest.fn() }), +})); +jest.mock('@/components/execution/BatchExecutionMonitor', () => ({ + BatchExecutionMonitor: () => null, +})); + +let currentPermission: NotificationPermission = 'default'; +const requestPermissionMock = jest.fn().mockResolvedValue('granted'); + +function installNotificationStub() { + const stub = function () {} as unknown as typeof Notification; + Object.defineProperty(stub, 'permission', { configurable: true, get: () => currentPermission }); + Object.defineProperty(stub, 'requestPermission', { configurable: true, value: requestPermissionMock }); + (global as unknown as { Notification: typeof Notification }).Notification = stub; +} + +beforeEach(() => { + requestPermissionMock.mockClear(); + currentPermission = 'default'; + installNotificationStub(); +}); + +describe('ExecutionLandingPage permission request', () => { + it('calls Notification.requestPermission once when permission is default', () => { + render(); + expect(requestPermissionMock).toHaveBeenCalledTimes(1); + }); + + it('does NOT request permission when already granted', () => { + currentPermission = 'granted'; + render(); + expect(requestPermissionMock).not.toHaveBeenCalled(); + }); + + it('does NOT request permission when already denied', () => { + currentPermission = 'denied'; + render(); + expect(requestPermissionMock).not.toHaveBeenCalled(); + }); +}); From 653b42513f0b3d2c39812afe54b6a50ccc0e512d Mon Sep 17 00:00:00 2001 From: Test User Date: Thu, 21 May 2026 11:31:20 -0700 Subject: [PATCH 4/8] fix(web-ui): coderabbit findings on #559 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a11y: bell button now exposes `aria-expanded`, `aria-controls`, and `aria-haspopup="dialog"`; dropdown gets `id="notification-popover"` + `role="dialog"` so screen readers can associate the control with the popup. Added a test that asserts these attrs flip on open/close. - error handling: wrap `localStorage.setItem` in `persist()` with a try/catch so quota-exceeded or private-mode write failures degrade gracefully — the in-memory state still updates; we log via console.error but do not propagate. Added a test that stubs setItem to throw and verifies the hook continues to function. Skipped (with justification): - "Pre-prompt UI banner before requesting Notification permission" — the approved adapted plan explicitly chose the browser's native prompt over a custom banner. Browsers themselves do not re-prompt once the user has chosen, so a `notificationsAsked` flag is redundant. Tests: 872/872 passing (was 870). Build green. --- .../layout/NotificationCenter.test.tsx | 15 ++++++++++++ .../__tests__/hooks/useNotifications.test.ts | 24 +++++++++++++++++++ .../components/layout/NotificationCenter.tsx | 10 +++++++- web-ui/src/hooks/useNotifications.ts | 9 ++++++- 4 files changed, 56 insertions(+), 2 deletions(-) diff --git a/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx b/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx index 92b662aa..4d22bf4f 100644 --- a/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx +++ b/web-ui/src/__tests__/components/layout/NotificationCenter.test.tsx @@ -152,6 +152,21 @@ describe('NotificationCenter', () => { expect(within(item as HTMLElement).queryByTestId('icon-CheckmarkCircle01Icon')).toBeNull(); }); + it('exposes aria-expanded and aria-controls on the bell button', () => { + render( + + + + ); + const bell = screen.getByRole('button', { name: /notifications/i }); + expect(bell).toHaveAttribute('aria-expanded', 'false'); + expect(bell).toHaveAttribute('aria-controls', 'notification-popover'); + + fireEvent.click(bell); + expect(bell).toHaveAttribute('aria-expanded', 'true'); + expect(document.getElementById('notification-popover')).toBeInTheDocument(); + }); + it('renders notifications from existing localStorage state on mount', () => { const stored = [ { diff --git a/web-ui/src/__tests__/hooks/useNotifications.test.ts b/web-ui/src/__tests__/hooks/useNotifications.test.ts index a6cf5faa..abe4bf6b 100644 --- a/web-ui/src/__tests__/hooks/useNotifications.test.ts +++ b/web-ui/src/__tests__/hooks/useNotifications.test.ts @@ -206,6 +206,30 @@ describe('useNotifications', () => { expect(NotificationCtor).not.toHaveBeenCalled(); }); + it('does not crash when localStorage.setItem throws', () => { + // Simulate quota-exceeded / private-mode by stubbing setItem + const original = Storage.prototype.setItem; + const consoleErr = jest.spyOn(console, 'error').mockImplementation(() => {}); + Storage.prototype.setItem = jest.fn(() => { + throw new Error('QuotaExceededError'); + }); + + try { + const { result } = renderHook(() => useNotifications()); + expect(() => { + act(() => { + result.current.addNotification({ type: 'batch.completed', message: 'should not crash' }); + }); + }).not.toThrow(); + // The in-memory state still reflects the update even though persist failed + expect(result.current.notifications).toHaveLength(1); + expect(consoleErr).toHaveBeenCalled(); + } finally { + Storage.prototype.setItem = original; + consoleErr.mockRestore(); + } + }); + it('persists batchStatus on the stored notification', () => { const { result } = renderHook(() => useNotifications()); act(() => { diff --git a/web-ui/src/components/layout/NotificationCenter.tsx b/web-ui/src/components/layout/NotificationCenter.tsx index 13ac152d..10a05a84 100644 --- a/web-ui/src/components/layout/NotificationCenter.tsx +++ b/web-ui/src/components/layout/NotificationCenter.tsx @@ -49,6 +49,9 @@ export function NotificationCenter() { {open && ( -
    +