fix(notifications): quiet lifecycle hook noise

2026-09-30 12:39:42 +02:00 · 2026-07-28 23:20:01 +03:00
parent 77bcbc9b94
commit 67eb5b43eb
4 changed files with 252 additions and 46 deletions
@@ -1848,7 +1848,7 @@
              <input type="checkbox" id="eventIdleAudio">

              <div class="event-label">Response complete</div>
-              <input type="checkbox" id="eventStopEnabled" checked>
+              <input type="checkbox" id="eventStopEnabled">
              <input type="checkbox" id="eventStopBrowser">
              <input type="checkbox" id="eventStopPush">
              <input type="checkbox" id="eventStopAudio">
@@ -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;

@@ -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();
          }
        }
@@ -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<string, EventPreference>;
+  _version: number;
+};
+
+type Manager = {
+  preferences: NotificationPreferences;
+  notifications: unknown[];
+  getStorageKey: () => string;
+  normalizePreferences: (preferences: Record<string, unknown>) => NotificationPreferences;
+  notify: (notification: Record<string, unknown>) => void;
+};
+
+const openWindows: JSDOM[] = [];
+
+function loadManager(
+  saved?: Record<string, unknown>,
+  device: { deviceType?: string; handheld?: boolean } = {}
+): { dom: JSDOM; manager: Manager } {
+  const dom = new JSDOM(
+    '<!doctype html><body><span id="notifBadge"></span><div id="notifList"></div><div id="notifEmpty"></div></body>',
+    {
+      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<unknown, unknown> }) => 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');
+  });
+});