fix(web-ui): deliver batch notifications when execution monitor is unmounted (#652)

CLAUDE.md

@@ -45,7 +45,7 @@ Issue **import + traceability** (#565) is **complete**: `POST /api/v2/integratio
 **Phase 5.4 is complete** — PRD stress-test web UI: trigger + streaming (#561). Backend: `GET /api/v2/prd/stress-test` SSE endpoint streams `goals_extracted`, `goal_analyzed`, `complete`, and `error` events from `core/prd_stress_test.py:stress_test_prd_stream()`, resolving the LLM provider via the standard chain and applying the standard rate limit. Frontend: `useStressTestStream` hook manages the SSE connection and event accumulation; `StressTestModal` renders the streaming progress and is opened via a "Stress Test" button on the `/prd` page (enabled only when a PRD exists). Results rendering + refinement (#562) is **complete**: the `complete` SSE event now carries structured, severity-tagged `ambiguities` (`Ambiguity.severity` is `"blocking"`/`"warning"`); `StressTestModal` shows a results view of `AmbiguityCard`s (question text, severity badge, answer textarea) with an "X of Y answered" progress indicator and a **[Refine PRD]** button (disabled until every blocking ambiguity is answered). Refine posts to `POST /api/v2/prd/stress-test/refine`, which folds the answers into a new PRD version via `resolve_ambiguities_into_prd` (offloaded with `asyncio.to_thread`) and `prd.create_new_version`, then `mutatePrd` reflects it in the editor.
 
 **Phase 5.3 is complete** — Async notifications cover both surfaces:
-- **Browser + in-app center (#559)**: `useNotifications` hook with workspace-scoped `localStorage` persistence and browser Notification dispatch (only when tab hidden + permission granted); `NotificationProvider` in root layout; `NotificationCenter` (bell icon + dropdown) mounts in sidebar footer. `BatchExecutionMonitor` dispatches `batch.completed` on terminal status transitions (distinguishing COMPLETED/FAILED/CANCELLED in both the in-app message and the success icon) and `blocker.created` on per-task BLOCKED transitions. `/execution` requests browser permission once on mount when permission is `'default'`. `/proof` dispatches `gate.run.failed` per failed gate when a proof run completes with `passed === false`. Known limitation: notifications only fire while `BatchExecutionMonitor` is mounted (cross-page background poller is out of scope; tracked for future work).
+- **Browser + in-app center (#559)**: `useNotifications` hook with workspace-scoped `localStorage` persistence and browser Notification dispatch (only when tab hidden + permission granted); `NotificationProvider` in root layout; `NotificationCenter` (bell icon + dropdown) mounts in sidebar footer. `/execution` requests browser permission once on mount when permission is `'default'`. `/proof` dispatches `gate.run.failed` per failed gate when a proof run completes with `passed === false`. **Background delivery (#652)**: a cross-page watcher (`useBatchNotificationWatcher`, mounted once in `NotificationProvider` in the root layout so it runs on every route) polls `GET /api/v2/batches` and is the single dispatcher of `batch.completed` (terminal transitions, distinguishing COMPLETED/FAILED/CANCELLED) and `blocker.created` (per-task BLOCKED transitions) — so these fire even when `BatchExecutionMonitor` is unmounted. The watcher baselines on its first poll (no spurious alerts for already-terminal/blocked batches), resets on workspace change, and guards against overlapping in-flight polls; `BatchExecutionMonitor` no longer dispatches them (avoids duplicates). Remaining limitation: `gate.run.failed` stays page-scoped to `/proof` (a proof run is a synchronous request/response the user actively watches, not a server-tracked background job).
 - **Outbound webhook (#560)**: Settings → Notifications tab takes a single URL + enabled toggle, persisted to `.codeframe/notifications_config.json` via `atomic_write_json`. `GET/PUT /api/v2/settings/notifications` and `POST /api/v2/settings/notifications/test` (test fires a sample payload and surfaces status code). `WebhookNotificationService.send_event` is the generic backend; dispatched fire-and-forget (5s timeout) from `core/conductor.py` on `BATCH_COMPLETED` only (not PARTIAL/FAILED/CANCELLED), `core/blockers.py:create()` after `BLOCKER_CREATED`, and `ui/routers/pr_v2.py:merge_pull_request` after successful merge. Failures are logged but never break the triggering operation.
 
 **Phase 5.2 is complete** — Costs page now ships per-task and per-agent breakdowns (#558) on top of the spend summary (#557). Backend: `GET /api/v2/costs/tasks?days=N&limit=M` (top-N tasks with titles, agent, tokens, cost) and `GET /api/v2/costs/by-agent?days=N` (per-agent rollup + total input/output tokens), both via `TokenRepository.get_top_tasks_by_cost` and `get_costs_by_agent`. Task board cards show an inline `MoneyBag02Icon` cost badge with token-breakdown tooltip when cost data exists. Fixed a v2 data-loss bug where `react_agent` int-cast UUID task IDs and stored NULL in `token_usage`.

web-ui/src/__tests__/hooks/useBatchNotificationWatcher.test.ts

@@ -0,0 +1,239 @@
+import { renderHook, act, waitFor } from '@testing-library/react';
+import { useBatchNotificationWatcher } from '@/hooks/useBatchNotificationWatcher';
+import { batchesApi, tasksApi } from '@/lib/api';
+import { getSelectedWorkspacePath } from '@/lib/workspace-storage';
+import type { BatchListResponse, BatchResponse, Task } from '@/types';
+
+jest.mock('@/lib/api');
+jest.mock('@/lib/workspace-storage');
+
+const mockList = batchesApi.list as jest.MockedFunction<typeof batchesApi.list>;
+const mockGetTask = tasksApi.getOne as jest.MockedFunction<typeof tasksApi.getOne>;
+const mockGetWorkspacePath = getSelectedWorkspacePath as jest.MockedFunction<
+  typeof getSelectedWorkspacePath
+>;
+
+function batch(overrides: Partial<BatchResponse> = {}): BatchResponse {
+  return {
+    id: 'batch-1234abcd',
+    workspace_id: 'ws-1',
+    task_ids: ['t1'],
+    status: 'RUNNING',
+    strategy: 'serial',
+    max_parallel: 1,
+    on_failure: 'continue',
+    started_at: null,
+    completed_at: null,
+    results: { t1: 'IN_PROGRESS' },
+    ...overrides,
+  };
+}
+
+function listResponse(batches: BatchResponse[]): BatchListResponse {
+  return { batches, total: batches.length, by_status: {} };
+}
+
+// Queue a sequence of list() responses, one per poll tick.
+function queueResponses(...responses: BatchListResponse[]) {
+  mockList.mockReset();
+  responses.forEach((r) => mockList.mockResolvedValueOnce(r));
+  // Any further polls repeat the last response.
+  if (responses.length > 0) {
+    mockList.mockResolvedValue(responses[responses.length - 1]);
+  }
+}
+
+const INTERVAL = 1000;
+
+beforeEach(() => {
+  jest.useFakeTimers();
+  jest.clearAllMocks();
+  mockGetWorkspacePath.mockReturnValue('/ws');
+  mockGetTask.mockResolvedValue({ id: 't1', title: 'Build login form' } as Task);
+});
+
+afterEach(() => {
+  jest.runOnlyPendingTimers();
+  jest.useRealTimers();
+});
+
+/** Run the immediate mount poll + flush its async work. */
+async function flushPoll() {
+  await act(async () => {
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+}
+
+/** Advance one polling interval and flush async work. */
+async function tick() {
+  await act(async () => {
+    jest.advanceTimersByTime(INTERVAL);
+    await Promise.resolve();
+    await Promise.resolve();
+  });
+}
+
+describe('useBatchNotificationWatcher', () => {
+  it('does not notify for batches already terminal on the first poll (baseline)', async () => {
+    const addNotification = jest.fn();
+    queueResponses(listResponse([batch({ status: 'COMPLETED', results: { t1: 'COMPLETED' } })]));
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll();
+
+    expect(addNotification).not.toHaveBeenCalled();
+  });
+
+  it('fires batch.completed when a running batch transitions to a terminal state', async () => {
+    const addNotification = jest.fn();
+    queueResponses(
+      listResponse([batch({ status: 'RUNNING', results: { t1: 'IN_PROGRESS' } })]),
+      listResponse([batch({ status: 'COMPLETED', results: { t1: 'COMPLETED' } })])
+    );
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll(); // baseline = RUNNING
+    expect(addNotification).not.toHaveBeenCalled();
+
+    await tick(); // now COMPLETED
+
+    expect(addNotification).toHaveBeenCalledTimes(1);
+    expect(addNotification).toHaveBeenCalledWith(
+      expect.objectContaining({
+        type: 'batch.completed',
+        batchStatus: 'COMPLETED',
+        batchId: 'batch-1234abcd',
+      })
+    );
+  });
+
+  it('fires batch.completed only once across repeated polls', async () => {
+    const addNotification = jest.fn();
+    queueResponses(
+      listResponse([batch({ status: 'RUNNING', results: { t1: 'IN_PROGRESS' } })]),
+      listResponse([batch({ status: 'FAILED', results: { t1: 'FAILED' } })])
+    );
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll();
+    await tick(); // FAILED
+    await tick(); // still FAILED — must not re-fire
+
+    expect(addNotification).toHaveBeenCalledTimes(1);
+    expect(addNotification).toHaveBeenCalledWith(
+      expect.objectContaining({ type: 'batch.completed', batchStatus: 'FAILED' })
+    );
+  });
+
+  it('fires blocker.created with the task title when a task transitions to BLOCKED', async () => {
+    const addNotification = jest.fn();
+    queueResponses(
+      listResponse([batch({ status: 'RUNNING', results: { t1: 'IN_PROGRESS' } })]),
+      listResponse([batch({ status: 'RUNNING', results: { t1: 'BLOCKED' } })])
+    );
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll();
+    await tick();
+
+    await waitFor(() =>
+      expect(addNotification).toHaveBeenCalledWith(
+        expect.objectContaining({
+          type: 'blocker.created',
+          taskId: 't1',
+          message: expect.stringContaining('Build login form'),
+        })
+      )
+    );
+  });
+
+  it('does nothing when no workspace is selected', async () => {
+    const addNotification = jest.fn();
+    mockGetWorkspacePath.mockReturnValue(null);
+    queueResponses(listResponse([batch()]));
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll();
+
+    expect(mockList).not.toHaveBeenCalled();
+    expect(addNotification).not.toHaveBeenCalled();
+  });
+
+  it('does not start an overlapping poll while one is still in flight', async () => {
+    const addNotification = jest.fn();
+    // First list() never resolves during the test window — simulates a slow poll.
+    let resolveSlow: (v: BatchListResponse) => void = () => {};
+    const slow = new Promise<BatchListResponse>((res) => {
+      resolveSlow = res;
+    });
+    mockList.mockReset();
+    mockList.mockReturnValueOnce(slow);
+    mockList.mockResolvedValue(listResponse([batch()]));
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll(); // immediate poll starts, awaiting `slow`
+    await tick(); // interval fires but must be skipped (in-flight)
+    await tick();
+
+    // Only the one still-pending call was made; no overlap.
+    expect(mockList).toHaveBeenCalledTimes(1);
+
+    // Let the slow poll finish; subsequent ticks resume normally.
+    await act(async () => {
+      resolveSlow(listResponse([batch()]));
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+    await tick();
+    expect(mockList.mock.calls.length).toBeGreaterThan(1);
+  });
+
+  it('does not dispatch stale notifications when the workspace changes mid-poll', async () => {
+    const addNotification = jest.fn();
+    // A slow poll for workspace /ws-a that resolves with a terminal transition.
+    let resolveSlow: (v: BatchListResponse) => void = () => {};
+    const slow = new Promise<BatchListResponse>((res) => {
+      resolveSlow = res;
+    });
+    mockGetWorkspacePath.mockReturnValue('/ws-a');
+    mockList.mockReset();
+    // First poll (baseline) sees RUNNING; second poll returns the slow promise.
+    mockList.mockResolvedValueOnce(
+      listResponse([batch({ status: 'RUNNING', results: { t1: 'IN_PROGRESS' } })])
+    );
+    mockList.mockReturnValueOnce(slow);
+    mockList.mockResolvedValue(listResponse([batch()]));
+
+    renderHook(() => useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL }));
+    await flushPoll(); // baseline RUNNING for /ws-a
+    await tick(); // second poll starts, awaiting `slow`
+
+    // Workspace switches away before the slow poll resolves.
+    mockGetWorkspacePath.mockReturnValue('/ws-b');
+    await act(async () => {
+      resolveSlow(listResponse([batch({ status: 'COMPLETED', results: { t1: 'COMPLETED' } })]));
+      await Promise.resolve();
+      await Promise.resolve();
+    });
+
+    // The terminal transition belongs to /ws-a, which is no longer active.
+    expect(addNotification).not.toHaveBeenCalled();
+  });
+
+  it('stops polling after unmount', async () => {
+    const addNotification = jest.fn();
+    queueResponses(listResponse([batch()]));
+
+    const { unmount } = renderHook(() =>
+      useBatchNotificationWatcher(addNotification, { intervalMs: INTERVAL })
+    );
+    await flushPoll();
+    const callsBefore = mockList.mock.calls.length;
+
+    unmount();
+    await tick();
+
+    expect(mockList.mock.calls.length).toBe(callsBefore);
+  });
+});

web-ui/src/components/execution/BatchExecutionMonitor.tsx

@@ -24,7 +24,6 @@
 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 ────────────────────────────────────────────────
@@ -51,7 +50,6 @@
 
 export function BatchExecutionMonitor({ batchId, workspacePath }: BatchExecutionMonitorProps) {
   const router = useRouter();
-  const { addNotification } = useNotificationContext();
   const [batch, setBatch] = useState<BatchResponse | null>(null);
   const [tasks, setTasks] = useState<Record<string, Task>>({});
   const [expandedTaskId, setExpandedTaskId] = useState<string | null>(null);
@@ -61,10 +59,6 @@
   // Track which task IDs have already been fetched to avoid refetching
   const fetchedTaskIdsRef = useRef<Set<string>>(new Set());
 
-  // Track previous batch + per-task statuses for transition-based notifications
-  const prevBatchStatusRef = useRef<string | null>(null);
-  const prevTaskStatusesRef = useRef<Record<string, string>>({});
-
   // ── Fetch batch details + task names ────────────────────────────────
   const fetchBatch = useCallback(async () => {
     try {
@@ -102,57 +96,12 @@
     return () => {
       if (pollRef.current) clearInterval(pollRef.current);
     };
   }, [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;
-      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',
-        batchStatus: batch.status as 'COMPLETED' | 'FAILED' | 'CANCELLED',
-        message: outcomeMessage,
-        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]);
+  // Note: batch.completed / blocker.created notifications are dispatched by the
+  // cross-page background watcher in NotificationProvider (issue #652), so they
+  // fire even when this monitor is unmounted. This component only renders the
+  // live view; it no longer dispatches notifications to avoid duplicates.
 
   // Auto-expand the first IN_PROGRESS task
   useEffect(() => {

web-ui/src/contexts/NotificationContext.tsx

@@ -2,13 +2,28 @@
 
 import { createContext, useContext, type ReactNode } from 'react';
 import { useNotifications, type UseNotificationsReturn } from '@/hooks/useNotifications';
+import { useBatchNotificationWatcher } from '@/hooks/useBatchNotificationWatcher';
 
 const NotificationContext = createContext<UseNotificationsReturn | null>(null);
 
+/**
+ * Cross-page background watcher. Mounted once here (inside the provider, so it
+ * runs on every route) it is the single dispatcher of batch.completed and
+ * blocker.created — making those notifications fire even when the execution
+ * page is unmounted. See issue #652.
+ */
+function BackgroundBatchWatcher({ addNotification }: { addNotification: UseNotificationsReturn['addNotification'] }) {
+  useBatchNotificationWatcher(addNotification);
+  return null;
+}
+
 export function NotificationProvider({ children }: { children: ReactNode }) {
   const value = useNotifications();
   return (
-    <NotificationContext.Provider value={value}>{children}</NotificationContext.Provider>
+    <NotificationContext.Provider value={value}>
+      <BackgroundBatchWatcher addNotification={value.addNotification} />
+      {children}
+    </NotificationContext.Provider>
   );
 }