From 9ebb4896449b174c705315a72d735c559614e8c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 22 Mar 2026 16:30:49 -0400 Subject: [PATCH] feat(04-01): create notification service with batching, urgency, and GC protection - Add createNotificationService with 10s batch window, urgency mapping, click-to-navigate - Extend DesktopEvent union with NotificationClickEvent type - Add 10 unit tests covering batching, urgency, NEVER_BATCH, GC protection, Linux urgency - Platform-specific: silent flag, Windows timeoutType, Linux urgency level Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/notification-service.ts | 185 ++++++++++++++++ src/shared/types.ts | 9 +- tests/unit/notification-service.test.ts | 267 ++++++++++++++++++++++++ 3 files changed, 460 insertions(+), 1 deletion(-) create mode 100644 src/main/notification-service.ts create mode 100644 tests/unit/notification-service.test.ts diff --git a/src/main/notification-service.ts b/src/main/notification-service.ts new file mode 100644 index 00000000..3fd0308e --- /dev/null +++ b/src/main/notification-service.ts @@ -0,0 +1,185 @@ +import { Notification, app } from "electron"; +import type { BrowserWindow } from "electron"; +import type { DesktopEvent, SectionId } from "@shared/types"; + +export interface NotificationOptions { + title: string; + body: string; + urgency: "critical" | "informational"; + navigation?: { + section: SectionId; + entityId?: string; + }; + /** Batch key -- notifications with same key within window are batched */ + batchKey?: string; +} + +interface NotificationServiceDependencies { + getMainWindow: () => BrowserWindow | null; + emitEvent: (event: DesktopEvent) => void; + logger: { info: (...args: unknown[]) => void; error: (...args: unknown[]) => void }; +} + +interface BatchState { + count: number; + timer: ReturnType; + section: SectionId; +} + +const BATCH_WINDOW_MS = 10_000; + +/** + * Batch keys that should NEVER be batched -- always fire individually. + * These represent events the user must see each instance of. + */ +const NEVER_BATCH: ReadonlySet = new Set([ + "pending_approval", + "crash_recovery", + "update_available", + "manual_intervention", + "budget_exhaustion", +]); + +/** + * Batch summary copy per batch key. + * Used when the batch timer expires and accumulated events need a summary notification. + */ +const BATCH_SUMMARY_COPY: Record string; body: string }> = { + failed_runs: { + title: (count) => `${count} tasks failed`, + body: "Click to view failed runs", + }, + agent_hired: { + title: (count) => `${count} agents hired`, + body: "Click to view organization", + }, + budget_alerts: { + title: (count) => `${count} budget alerts`, + body: "Click to view cost details", + }, + task_completed: { + title: (count) => `${count} tasks completed`, + body: "Click to view task list", + }, + agent_paused: { + title: (count) => `${count} agents paused`, + body: "Click to view paused agents", + }, +}; + +export function createNotificationService(deps: NotificationServiceDependencies) { + /** + * Prevent GC on Windows -- Notification objects must be retained + * while visible so click handlers fire. Removed on "close" event. + */ + const activeNotifications: Set = new Set(); + + /** Active batch state per batch key */ + const activeBatches: Map = new Map(); + + function notify(options: NotificationOptions) { + if (!Notification.isSupported()) return; + + const key = options.batchKey; + + // If no batch key, or the key is in the NEVER_BATCH list, fire immediately + if (!key || NEVER_BATCH.has(key)) { + fireNotification(options); + return; + } + + const existing = activeBatches.get(key); + if (!existing) { + // First event in batch -- fire immediately, start batch window + fireNotification(options); + activeBatches.set(key, { + count: 0, + timer: setTimeout(() => flushBatch(key), BATCH_WINDOW_MS), + section: options.navigation?.section ?? "overview", + }); + return; + } + + // Accumulate -- do not fire individual notification + existing.count++; + } + + function flushBatch(key: string) { + const batch = activeBatches.get(key); + activeBatches.delete(key); + + if (batch && batch.count > 0) { + const copy = BATCH_SUMMARY_COPY[key]; + const title = copy ? copy.title(batch.count) : `${batch.count} more events`; + const body = copy ? copy.body : "Click to view"; + + fireNotification({ + title, + body, + urgency: "informational", + navigation: { section: batch.section }, + // No entityId -- batched summary navigates to section overview + }); + } + } + + function fireNotification(options: NotificationOptions) { + if (!Notification.isSupported()) return; + + const notificationOptions: Electron.NotificationConstructorOptions = { + title: options.title, + body: options.body, + silent: options.urgency === "informational", + }; + + // Platform-specific urgency handling + if (process.platform === "win32" && options.urgency === "critical") { + notificationOptions.timeoutType = "never"; + } + if (process.platform === "linux") { + notificationOptions.urgency = options.urgency === "critical" ? "critical" : "normal"; + } + + const notification = new Notification(notificationOptions); + + // Click handler: restore window and navigate (D-02 window restoration sequence) + notification.on("click", () => { + app.focus({ steal: true }); + const win = deps.getMainWindow(); + if (win && !win.isDestroyed()) { + if (win.isMinimized()) win.restore(); + win.show(); + win.focus(); + } + if (options.navigation) { + deps.emitEvent({ + type: "notification-click", + section: options.navigation.section, + entityId: options.navigation.entityId, + }); + } + }); + + // GC protection: hold reference until notification closes + notification.on("close", () => { + activeNotifications.delete(notification); + }); + + notification.show(); + activeNotifications.add(notification); + } + + function destroy() { + // Clear all batch timers + for (const batch of activeBatches.values()) { + clearTimeout(batch.timer); + } + activeBatches.clear(); + activeNotifications.clear(); + } + + return { + notify, + destroy, + }; +} diff --git a/src/shared/types.ts b/src/shared/types.ts index 931c8cd1..04c00ad4 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -376,6 +376,12 @@ export interface NewMessageEvent { message: AgentMessageRecord; } +export interface NotificationClickEvent { + type: "notification-click"; + section: SectionId; + entityId?: string; +} + export type DesktopEvent = | LogChunkEvent | DomainChangedEvent @@ -384,7 +390,8 @@ export type DesktopEvent = | HeartbeatStatusEvent | AgentMessageEvent | AutomationFiredEvent - | NewMessageEvent; + | NewMessageEvent + | NotificationClickEvent; export type InboxItemType = "pending_approval" | "failed_run" | "budget_warning" | "blocked_task" | "review_needed" | "agent_message"; diff --git a/tests/unit/notification-service.test.ts b/tests/unit/notification-service.test.ts new file mode 100644 index 00000000..41bdffbd --- /dev/null +++ b/tests/unit/notification-service.test.ts @@ -0,0 +1,267 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; + +// Track all constructed notification instances +const constructedNotifications: Array<{ + options: Record; + clickHandler?: () => void; + closeHandler?: () => void; +}> = []; + +vi.mock("electron", () => { + return { + Notification: class MockNotification { + static isSupported = () => true; + options: Record; + _clickHandler?: () => void; + _closeHandler?: () => void; + + constructor(options: Record) { + this.options = options; + } + + show() { + constructedNotifications.push({ + options: this.options, + clickHandler: this._clickHandler, + closeHandler: this._closeHandler, + }); + } + + on(event: string, handler: () => void) { + if (event === "click") this._clickHandler = handler; + if (event === "close") this._closeHandler = handler; + // Update the last entry if already shown, or store for later + const last = constructedNotifications[constructedNotifications.length - 1]; + if (last && event === "click") last.clickHandler = handler; + if (last && event === "close") last.closeHandler = handler; + } + }, + app: { focus: vi.fn() }, + }; +}); + +// Import after mocking +const { createNotificationService } = await import("@main/notification-service"); + +describe("notification-service", () => { + let service: ReturnType; + let mockGetMainWindow: ReturnType; + let mockEmitEvent: ReturnType; + let mockLogger: { info: ReturnType; error: ReturnType }; + + beforeEach(() => { + vi.useFakeTimers(); + constructedNotifications.length = 0; + + mockGetMainWindow = vi.fn(() => null); + mockEmitEvent = vi.fn(); + mockLogger = { + info: vi.fn(), + error: vi.fn(), + }; + + service = createNotificationService({ + getMainWindow: mockGetMainWindow, + emitEvent: mockEmitEvent, + logger: mockLogger, + }); + }); + + afterEach(() => { + service.destroy(); + vi.useRealTimers(); + }); + + it("Test 1: fireNotification with urgency 'critical' creates Notification with silent: false", () => { + service.notify({ + title: "Budget Exhausted", + body: "Agent paused", + urgency: "critical", + }); + + expect(constructedNotifications).toHaveLength(1); + expect(constructedNotifications[0]!.options.silent).toBe(false); + }); + + it("Test 2: fireNotification with urgency 'informational' creates Notification with silent: true", () => { + service.notify({ + title: "Agent Hired", + body: "Bob joined", + urgency: "informational", + }); + + expect(constructedNotifications).toHaveLength(1); + expect(constructedNotifications[0]!.options.silent).toBe(true); + }); + + it("Test 3: notify without batchKey fires immediately (no batching)", () => { + service.notify({ + title: "Update Available", + body: "Version 2.0 is ready", + urgency: "informational", + }); + + expect(constructedNotifications).toHaveLength(1); + }); + + it("Test 4: notify with batchKey fires first event immediately, starts batch timer", () => { + service.notify({ + title: "Task Failed", + body: "Task A failed", + urgency: "informational", + navigation: { section: "tasks" }, + batchKey: "failed_runs", + }); + + expect(constructedNotifications).toHaveLength(1); + expect(constructedNotifications[0]!.options.title).toBe("Task Failed"); + }); + + it("Test 5: notify with same batchKey within 10s increments count, does NOT fire second notification", () => { + service.notify({ + title: "Task Failed", + body: "Task A failed", + urgency: "informational", + navigation: { section: "tasks" }, + batchKey: "failed_runs", + }); + + service.notify({ + title: "Task Failed", + body: "Task B failed", + urgency: "informational", + navigation: { section: "tasks" }, + batchKey: "failed_runs", + }); + + // Only the first should have fired + expect(constructedNotifications).toHaveLength(1); + }); + + it("Test 6: After 10s batch timer expires, fires summary notification with count in title", () => { + service.notify({ + title: "Task Failed", + body: "Task A failed", + urgency: "informational", + navigation: { section: "tasks" }, + batchKey: "failed_runs", + }); + + // Add 2 more batched notifications + service.notify({ + title: "Task Failed", + body: "Task B failed", + urgency: "informational", + navigation: { section: "tasks" }, + batchKey: "failed_runs", + }); + service.notify({ + title: "Task Failed", + body: "Task C failed", + urgency: "informational", + navigation: { section: "tasks" }, + batchKey: "failed_runs", + }); + + expect(constructedNotifications).toHaveLength(1); // Only first + + // Advance timer by 10 seconds + vi.advanceTimersByTime(10_000); + + // Summary notification should have fired + expect(constructedNotifications).toHaveLength(2); + const summary = constructedNotifications[1]!; + expect(summary.options.title).toContain("2"); + expect(summary.options.title).toContain("tasks failed"); + expect(summary.options.body).toBe("Click to view failed runs"); + }); + + it("Test 7: Batched summary notification has no entityId (navigates to section overview)", () => { + service.notify({ + title: "Agent Hired", + body: "Alice joined", + urgency: "informational", + navigation: { section: "orgchart", entityId: "agent-1" }, + batchKey: "agent_hired", + }); + + service.notify({ + title: "Agent Hired", + body: "Bob joined", + urgency: "informational", + navigation: { section: "orgchart", entityId: "agent-2" }, + batchKey: "agent_hired", + }); + + vi.advanceTimersByTime(10_000); + + // Summary notification fires -- simulate click + expect(constructedNotifications).toHaveLength(2); + const summaryClickHandler = constructedNotifications[1]!.clickHandler; + expect(summaryClickHandler).toBeInstanceOf(Function); + summaryClickHandler!(); + + // emitEvent should have been called with section but no entityId + const emitCall = mockEmitEvent.mock.calls.find( + ([event]: [{ type: string }]) => event.type === "notification-click", + ); + expect(emitCall).toBeDefined(); + expect(emitCall![0].section).toBe("orgchart"); + expect(emitCall![0].entityId).toBeUndefined(); + }); + + it("Test 8: Events in NEVER_BATCH list always fire individually regardless of batchKey", () => { + // pending_approval is in NEVER_BATCH + service.notify({ + title: "Approval Required", + body: "First approval", + urgency: "critical", + navigation: { section: "approvals" }, + batchKey: "pending_approval", + }); + + service.notify({ + title: "Approval Required", + body: "Second approval", + urgency: "critical", + navigation: { section: "approvals" }, + batchKey: "pending_approval", + }); + + // Both should fire individually + expect(constructedNotifications).toHaveLength(2); + }); + + it("Test 9: Active Notification objects are held in a Set and removed on 'close' event", () => { + service.notify({ + title: "Test", + body: "Test body", + urgency: "informational", + }); + + expect(constructedNotifications).toHaveLength(1); + + // The close handler should exist and be callable without error + const closeHandler = constructedNotifications[0]!.closeHandler; + expect(closeHandler).toBeInstanceOf(Function); + + // Calling close handler should not throw (notification removed from internal Set) + closeHandler!(); + }); + + it("Test 10: On Linux, urgency 'critical' sets urgency option to 'critical'", () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + + service.notify({ + title: "Critical Alert", + body: "Something critical", + urgency: "critical", + }); + + expect(constructedNotifications).toHaveLength(1); + expect(constructedNotifications[0]!.options.urgency).toBe("critical"); + + Object.defineProperty(process, "platform", { value: originalPlatform, configurable: true }); + }); +});