From 67eb5b43eb80440763ab35b04005c10c7180a2dd Mon Sep 17 00:00:00 2001 From: lior Date: Tue, 28 Jul 2026 23:20:01 +0300 Subject: [PATCH] fix(notifications): quiet lifecycle hook noise --- src/web/public/index.html | 2 +- src/web/public/notification-manager.js | 139 +++++++++++++++------- src/web/public/settings-ui.js | 7 +- test/notification-manager-noise.test.ts | 150 ++++++++++++++++++++++++ 4 files changed, 252 insertions(+), 46 deletions(-) create mode 100644 test/notification-manager-noise.test.ts diff --git a/src/web/public/index.html b/src/web/public/index.html index 224280db..3fa2501c 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -1848,7 +1848,7 @@

App Settings

Response complete
- + diff --git a/src/web/public/notification-manager.js b/src/web/public/notification-manager.js index 8c28e915..2d7f4d9f 100644 --- a/src/web/public/notification-manager.js +++ b/src/web/public/notification-manager.js @@ -9,7 +9,7 @@ * 5. Audio alerts (Web Audio API beep, user-opt-in) * * Features: - * - Per-event-type preferences (enabled, browser, audio, push) with v1→v4 migration + * - Per-event-type preferences (enabled, browser, audio, push) with v1→v5 migration * - Device-specific defaults (notifications disabled on mobile by default) * - 5s notification grouping window to batch rapid-fire events * - 100-notification cap with oldest eviction @@ -21,7 +21,7 @@ * @param {CodemanApp} app - Reference to the main app instance * * @dependency constants.js (STUCK_THRESHOLD_DEFAULT_MS, timing constants) - * @dependency mobile-handlers.js (MobileDetection.getDeviceType for device-specific defaults) + * @dependency mobile-handlers.js (MobileDetection stable handheld identity/device type) * @loadorder 4 of 15 — loaded after voice-input.js, before keyboard-accessory.js */ @@ -65,12 +65,19 @@ class NotificationManager { }); } - loadPreferences() { + _usesMobilePreferences() { + return ( + MobileDetection.isHandheldDevice?.() ?? + MobileDetection.getDeviceType() === 'mobile' + ); + } + + getDefaultPreferences() { const defaultEventTypes = { permission_prompt: { enabled: true, browser: true, audio: true, push: false }, elicitation_dialog: { enabled: true, browser: true, audio: true, push: false }, idle_prompt: { enabled: true, browser: true, audio: false, push: false }, - stop: { enabled: true, browser: false, audio: false, push: false }, + stop: { enabled: false, browser: false, audio: false, push: false }, session_error: { enabled: true, browser: true, audio: false, push: false }, respawn_cycle: { enabled: true, browser: false, audio: false, push: false }, token_milestone: { enabled: true, browser: false, audio: false, push: false }, @@ -80,8 +87,8 @@ class NotificationManager { }; // Device-specific defaults: mobile has notifications disabled by default - const isMobile = MobileDetection.getDeviceType() === 'mobile'; - const defaults = { + const isMobile = this._usesMobilePreferences(); + return { enabled: !isMobile, // Disabled on mobile by default browserNotifications: !isMobile, audioAlerts: false, @@ -92,51 +99,97 @@ class NotificationManager { muteInfo: false, // Per-event-type preferences eventTypes: defaultEventTypes, - _version: 4, + _version: 5, + }; + } + + /** + * Apply the complete v1→v5 migration to either local or server-hydrated + * preferences. Keeping one normalization path prevents fresh browsers from + * reviving retired drawer-only hook defaults. + */ + normalizePreferences(rawPreferences) { + const defaults = this.getDefaultPreferences(); + if ( + !rawPreferences || + typeof rawPreferences !== 'object' || + Array.isArray(rawPreferences) + ) { + return defaults; + } + + const prefs = { + ...rawPreferences, + eventTypes: + rawPreferences.eventTypes && + typeof rawPreferences.eventTypes === 'object' && + !Array.isArray(rawPreferences.eventTypes) + ? Object.fromEntries( + Object.entries(rawPreferences.eventTypes).map(([key, value]) => [ + key, + value && typeof value === 'object' ? { ...value } : value, + ]) + ) + : undefined, + }; + const version = Number.isInteger(prefs._version) ? prefs._version : 0; + + // Migrate: v1 had browserNotifications defaulting to false + if (version < 2) { + prefs.browserNotifications = true; + } + // Migrate: v2 -> v3 adds eventTypes + if (version < 3) { + prefs.eventTypes = { ...defaults.eventTypes }; + } + // Migrate: v3 -> v4 adds push field to all eventTypes + if (version < 4 && prefs.eventTypes) { + for (const key of Object.keys(prefs.eventTypes)) { + if (prefs.eventTypes[key] && prefs.eventTypes[key].push === undefined) { + prefs.eventTypes[key].push = false; + } + } + } + // Migrate: v4 -> v5 removes the drawer-only Response Complete default. + // Preserve users who opted into any external delivery channel. + if (version < 5) { + const stopPref = prefs.eventTypes?.stop; + if ( + stopPref?.enabled === true && + !stopPref.browser && + !stopPref.audio && + !stopPref.push + ) { + stopPref.enabled = false; + } + } + + return { + ...defaults, + ...prefs, + eventTypes: { ...defaults.eventTypes, ...prefs.eventTypes }, + _version: 5, }; + } + + loadPreferences() { try { const storageKey = this.getStorageKey(); const saved = localStorage.getItem(storageKey); if (saved) { - const prefs = JSON.parse(saved); - // Migrate: v1 had browserNotifications defaulting to false - if (!prefs._version || prefs._version < 2) { - prefs.browserNotifications = true; - prefs._version = 2; - } - // Migrate: v2 -> v3 adds eventTypes - if (prefs._version < 3) { - prefs.eventTypes = defaultEventTypes; - prefs._version = 3; - localStorage.setItem(storageKey, JSON.stringify(prefs)); - } - // Migrate: v3 -> v4 adds push field to all eventTypes - if (prefs._version < 4) { - if (prefs.eventTypes) { - for (const key of Object.keys(prefs.eventTypes)) { - if (prefs.eventTypes[key] && prefs.eventTypes[key].push === undefined) { - prefs.eventTypes[key].push = false; - } - } - } - prefs._version = 4; - localStorage.setItem(storageKey, JSON.stringify(prefs)); - } - // Merge with defaults to ensure all eventTypes exist - return { - ...defaults, - ...prefs, - eventTypes: { ...defaultEventTypes, ...prefs.eventTypes }, - }; + const normalized = this.normalizePreferences(JSON.parse(saved)); + localStorage.setItem(storageKey, JSON.stringify(normalized)); + return normalized; } } catch (_e) { /* ignore */ } - return defaults; + return this.getDefaultPreferences(); } // Get storage key for notification prefs (device-specific) getStorageKey() { - const isMobile = MobileDetection.getDeviceType() === 'mobile'; - return isMobile ? 'codeman-notification-prefs-mobile' : 'codeman-notification-prefs'; + return this._usesMobilePreferences() + ? 'codeman-notification-prefs-mobile' + : 'codeman-notification-prefs'; } savePreferences() { @@ -163,8 +216,10 @@ class NotificationManager { 'exit-gate': 'ralph_complete', 'subagent-spawn': 'subagent_spawn', 'subagent-complete': 'subagent_complete', - 'hook-teammate-idle': 'idle_prompt', - 'hook-task-completed': 'stop', + // Team lifecycle hooks are agent activity, not session-idle/stop alerts. + // Reuse the existing opt-in agent categories instead of making them noisy. + 'hook-teammate-idle': 'subagent_spawn', + 'hook-task-completed': 'subagent_complete', }; const eventTypeKey = categoryToEventType[category] || category; diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 9e0c0d0c..5470dda7 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -408,7 +408,7 @@ Object.assign(CodemanApp.prototype, { document.getElementById('eventIdleAudio').checked = idlePref.audio ?? false; // Response complete (stop) const stopPref = eventTypes.stop || {}; - document.getElementById('eventStopEnabled').checked = stopPref.enabled ?? true; + document.getElementById('eventStopEnabled').checked = stopPref.enabled ?? false; document.getElementById('eventStopBrowser').checked = stopPref.browser ?? false; document.getElementById('eventStopPush').checked = stopPref.push ?? false; document.getElementById('eventStopAudio').checked = stopPref.audio ?? false; @@ -1587,7 +1587,7 @@ Object.assign(CodemanApp.prototype, { audio: document.getElementById('eventSubagentAudio').checked, }, }, - _version: 4, + _version: 5, }; if (this.notificationManager) { this.notificationManager.preferences = notifPrefsToSave; @@ -2275,7 +2275,8 @@ Object.assign(CodemanApp.prototype, { if (notificationPreferences && this.notificationManager) { const localNotifPrefs = localStorage.getItem(this.notificationManager.getStorageKey()); if (!localNotifPrefs) { - this.notificationManager.preferences = notificationPreferences; + this.notificationManager.preferences = + this.notificationManager.normalizePreferences(notificationPreferences); this.notificationManager.savePreferences(); } } diff --git a/test/notification-manager-noise.test.ts b/test/notification-manager-noise.test.ts new file mode 100644 index 00000000..d5fbf896 --- /dev/null +++ b/test/notification-manager-noise.test.ts @@ -0,0 +1,150 @@ +import { readFileSync } from 'node:fs'; +import { JSDOM } from 'jsdom'; +import { afterEach, describe, expect, it } from 'vitest'; + +const SOURCE = readFileSync(new URL('../src/web/public/notification-manager.js', import.meta.url), 'utf8'); + +type EventPreference = { + enabled: boolean; + browser: boolean; + audio: boolean; + push: boolean; +}; + +type NotificationPreferences = { + enabled: boolean; + eventTypes: Record; + _version: number; +}; + +type Manager = { + preferences: NotificationPreferences; + notifications: unknown[]; + getStorageKey: () => string; + normalizePreferences: (preferences: Record) => NotificationPreferences; + notify: (notification: Record) => void; +}; + +const openWindows: JSDOM[] = []; + +function loadManager( + saved?: Record, + device: { deviceType?: string; handheld?: boolean } = {} +): { dom: JSDOM; manager: Manager } { + const dom = new JSDOM( + '
', + { + url: 'http://localhost/', + runScripts: 'outside-only', + } + ); + openWindows.push(dom); + const win = dom.window as unknown as Window & + typeof globalThis & { + MobileDetection: { + getDeviceType: () => string; + isHandheldDevice?: () => boolean; + }; + STUCK_THRESHOLD_DEFAULT_MS: number; + GROUPING_TIMEOUT_MS: number; + NOTIFICATION_LIST_CAP: number; + }; + win.MobileDetection = { + getDeviceType: () => device.deviceType ?? 'desktop', + ...(typeof device.handheld === 'boolean' ? { isHandheldDevice: () => device.handheld === true } : {}), + }; + win.STUCK_THRESHOLD_DEFAULT_MS = 600_000; + win.GROUPING_TIMEOUT_MS = 5_000; + win.NOTIFICATION_LIST_CAP = 100; + win.requestAnimationFrame = ((callback: FrameRequestCallback) => { + callback(0); + return 1; + }) as typeof requestAnimationFrame; + + if (saved) { + win.localStorage.setItem('codeman-notification-prefs', JSON.stringify(saved)); + } + + win.eval(` + ${SOURCE} + window.__testNotificationManager = NotificationManager; + `); + const NotificationManager = ( + win as unknown as { + __testNotificationManager: new (app: { sessions: Map }) => Manager; + } + ).__testNotificationManager; + const manager = new NotificationManager({ sessions: new Map() }) as Manager; + return { dom, manager }; +} + +afterEach(() => { + for (const dom of openWindows.splice(0)) dom.window.close(); +}); + +describe('notification noise defaults', () => { + it('keeps response-complete and team lifecycle drawer entries opt-in', () => { + const { manager } = loadManager(); + expect(manager.preferences.eventTypes.stop.enabled).toBe(false); + + for (const category of ['hook-stop', 'hook-teammate-idle', 'hook-task-completed']) { + manager.notify({ + urgency: 'info', + category, + sessionId: 'session-1', + sessionName: 'session', + title: category, + message: category, + }); + } + + expect(manager.notifications).toHaveLength(0); + }); + + it('migrates the old drawer-only Stop default but preserves explicit delivery', () => { + const quietV4 = { + enabled: true, + eventTypes: { + stop: { enabled: true, browser: false, audio: false, push: false }, + }, + _version: 4, + }; + const { manager: quietManager } = loadManager(quietV4); + expect(quietManager.preferences.eventTypes.stop.enabled).toBe(false); + expect(quietManager.preferences._version).toBe(5); + + const browserV4 = { + enabled: true, + eventTypes: { + stop: { enabled: true, browser: true, audio: false, push: false }, + }, + _version: 4, + }; + const { manager: browserManager } = loadManager(browserV4); + expect(browserManager.preferences.eventTypes.stop.enabled).toBe(true); + }); + + it('normalizes server-hydrated v4 preferences through the same quiet migration', () => { + const { manager } = loadManager(); + manager.preferences = manager.normalizePreferences({ + enabled: true, + eventTypes: { + stop: { enabled: true, browser: false, audio: false, push: false }, + }, + _version: 4, + }); + + expect(manager.preferences.eventTypes.stop.enabled).toBe(false); + expect(manager.preferences._version).toBe(5); + }); + + it('keeps mobile notification defaults and storage on an unfolded handheld', () => { + const { manager } = loadManager(undefined, { + deviceType: 'desktop', + handheld: true, + }); + + expect(manager.preferences.enabled).toBe(false); + expect(manager.getStorageKey()).toBe('codeman-notification-prefs-mobile'); + }); +});