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) <noreply@anthropic.com>
This commit is contained in:
1 parent
7d5d1e5b30
commit
9ebb489644
3 files changed
+460
-1
No files matched your search
@@ -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<typeof setTimeout>;
|
||||
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<string> = 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, { title: (count: number) => 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<Notification> = new Set();
|
||||
|
||||
/** Active batch state per batch key */
|
||||
const activeBatches: Map<string, BatchState> = 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,
|
||||
};
|
||||
}
|
||||
+8
-1
@@ -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";
|
||||
|
||||
|
||||
@@ -0,0 +1,267 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
|
||||
// Track all constructed notification instances
|
||||
const constructedNotifications: Array<{
|
||||
options: Record<string, unknown>;
|
||||
clickHandler?: () => void;
|
||||
closeHandler?: () => void;
|
||||
}> = [];
|
||||
|
||||
vi.mock("electron", () => {
|
||||
return {
|
||||
Notification: class MockNotification {
|
||||
static isSupported = () => true;
|
||||
options: Record<string, unknown>;
|
||||
_clickHandler?: () => void;
|
||||
_closeHandler?: () => void;
|
||||
|
||||
constructor(options: Record<string, unknown>) {
|
||||
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<typeof createNotificationService>;
|
||||
let mockGetMainWindow: ReturnType<typeof vi.fn>;
|
||||
let mockEmitEvent: ReturnType<typeof vi.fn>;
|
||||
let mockLogger: { info: ReturnType<typeof vi.fn>; error: ReturnType<typeof vi.fn> };
|
||||
|
||||
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 });
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user