Fix focused notification sound playback

Sources/TerminalNotificationStore.swift

@@ -42,13 +42,22 @@ enum NotificationSoundSettings {
     )
     private static let pendingCustomSoundPreparationLock = NSLock()
     private static var pendingCustomSoundPreparationPaths: Set<String> = []
+    private static let activePlaybackSoundsLock = NSLock()
+    private static var activePlaybackSounds: [ObjectIdentifier: NSSound] = [:]
+    private static let activePlaybackSoundDelegate = ActivePlaybackSoundDelegate()
     private static let notificationSoundSupportedExtensions: Set<String> = [
         "aif",
         "aiff",
         "caf",
         "wav",
     ]
 
+    private final class ActivePlaybackSoundDelegate: NSObject, NSSoundDelegate {
+        func sound(_ sound: NSSound, didFinishPlaying finishedPlaying: Bool) {
+            NotificationSoundSettings.releaseActivePlaybackSound(sound)
+        }
+    }
+
     private struct CustomSoundSourceMetadata: Codable, Equatable {
         let sourcePath: String
         let sourceSize: UInt64
@@ -260,7 +269,16 @@ enum NotificationSoundSettings {
         playSoundFile(at: url)
     }
 
+    static func playSelectedSound(defaults: UserDefaults = .standard) {
+        let value = defaults.string(forKey: key) ?? defaultValue
+        playSound(value: value, defaults: defaults)
+    }
+
     static func previewSound(value: String, defaults: UserDefaults = .standard) {
+        playSound(value: value, defaults: defaults)
+    }
+
+    private static func playSound(value: String, defaults: UserDefaults) {
         switch value {
         case "default":
             NSSound.beep()
@@ -331,10 +349,26 @@ enum NotificationSoundSettings {
                 NSLog("Notification custom sound failed to load from path: \(url.path)")
                 return
             }
-            sound.play()
+            retainActivePlaybackSound(sound)
+            sound.delegate = activePlaybackSoundDelegate
+            if !sound.play() {
+                releaseActivePlaybackSound(sound)
+            }
         }
     }
 
+    private static func retainActivePlaybackSound(_ sound: NSSound) {
+        activePlaybackSoundsLock.lock()
+        activePlaybackSounds[ObjectIdentifier(sound)] = sound
+        activePlaybackSoundsLock.unlock()
+    }
+
+    private static func releaseActivePlaybackSound(_ sound: NSSound) {
+        activePlaybackSoundsLock.lock()
+        activePlaybackSounds.removeValue(forKey: ObjectIdentifier(sound))
+        activePlaybackSoundsLock.unlock()
+    }
+
     private static func cleanupStaleStagedSoundFiles(
         in directoryURL: URL,
         keeping fileName: String,
@@ -693,6 +727,11 @@ final class TerminalNotificationStore: ObservableObject {
         notification in
         store.scheduleUserNotification(notification)
     }
+    private var suppressedNotificationFeedbackHandler: (TerminalNotificationStore, TerminalNotification) -> Void = {
+        store,
+        notification in
+        store.playSuppressedNotificationFeedback(for: notification)
+    }
     private var indexes = NotificationIndexes()
 
     private init() {
@@ -877,7 +916,9 @@ final class TerminalNotificationStore: ObservableObject {
             center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear)
             center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear)
         }
-        if !shouldSuppressExternalDelivery {
+        if shouldSuppressExternalDelivery {
+            suppressedNotificationFeedbackHandler(self, notification)
+        } else {
             notificationDeliveryHandler(self, notification)
         }
     }
@@ -1005,15 +1046,19 @@ final class TerminalNotificationStore: ObservableObject {
         center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear)
     }
 
+    private func resolvedNotificationTitle(for notification: TerminalNotification) -> String {
+        let appName = Bundle.main.object(forInfoDictionaryKey: "CFBundleDisplayName") as? String
+            ?? Bundle.main.object(forInfoDictionaryKey: "CFBundleName") as? String
+            ?? "cmux"
+        return notification.title.isEmpty ? appName : notification.title
+    }
+
     private func scheduleUserNotification(_ notification: TerminalNotification) {
         ensureAuthorization(origin: .notificationDelivery) { [weak self] authorized in
             guard let self, authorized else { return }
 
             let content = UNMutableNotificationContent()
-            let appName = Bundle.main.object(forInfoDictionaryKey: "CFBundleDisplayName") as? String
-                ?? Bundle.main.object(forInfoDictionaryKey: "CFBundleName") as? String
-                ?? "cmux"
-            content.title = notification.title.isEmpty ? appName : notification.title
+            content.title = self.resolvedNotificationTitle(for: notification)
             content.subtitle = notification.subtitle
             content.body = notification.body
             content.sound = NotificationSoundSettings.sound()
@@ -1046,6 +1091,15 @@ final class TerminalNotificationStore: ObservableObject {
         }
     }
 
+    private func playSuppressedNotificationFeedback(for notification: TerminalNotification) {
+        NotificationSoundSettings.playSelectedSound()
+        NotificationSoundSettings.runCustomCommand(
+            title: resolvedNotificationTitle(for: notification),
+            subtitle: notification.subtitle,
+            body: notification.body
+        )
+    }
+
     private func ensureAuthorization(
         origin: AuthorizationRequestOrigin,
         _ completion: @escaping (Bool) -> Void
@@ -1262,6 +1316,18 @@ final class TerminalNotificationStore: ObservableObject {
         }
     }
 
+    func configureSuppressedNotificationFeedbackHandlerForTesting(
+        _ handler: @escaping (TerminalNotificationStore, TerminalNotification) -> Void
+    ) {
+        suppressedNotificationFeedbackHandler = handler
+    }
+
+    func resetSuppressedNotificationFeedbackHandlerForTesting() {
+        suppressedNotificationFeedbackHandler = { store, notification in
+            store.playSuppressedNotificationFeedback(for: notification)
+        }
+    }
+
     func promptToEnableNotificationsForTesting() {
         promptToEnableNotifications()
     }

cmuxTests/NotificationAndMenuBarTests.swift

@@ -37,6 +37,8 @@ final class NotificationDockBadgeTests: XCTestCase {
     override func tearDown() {
         TerminalNotificationStore.shared.resetNotificationSettingsPromptHooksForTesting()
         TerminalNotificationStore.shared.replaceNotificationsForTesting([])
+        TerminalNotificationStore.shared.resetNotificationDeliveryHandlerForTesting()
+        TerminalNotificationStore.shared.resetSuppressedNotificationFeedbackHandlerForTesting()
         super.tearDown()
     }
 
@@ -399,6 +401,143 @@ final class NotificationDockBadgeTests: XCTestCase {
         }
     }
 
+    func testFocusedTerminalNotificationStillRunsLocalSoundFeedbackWhenExternalDeliveryIsSuppressed() throws {
+        guard let appDelegate = AppDelegate.shared else {
+            XCTFail("AppDelegate.shared must be set for this test")
+            return
+        }
+        let manager = TabManager()
+        let store = TerminalNotificationStore.shared
+
+        let originalTabManager = appDelegate.tabManager
+        let originalNotificationStore = appDelegate.notificationStore
+        let originalAppFocusOverride = AppFocusState.overrideIsFocused
+
+        var deliveredNotificationIDs: [UUID] = []
+        var localFeedbackNotificationIDs: [UUID] = []
+
+        store.replaceNotificationsForTesting([])
+        store.configureNotificationDeliveryHandlerForTesting { _, notification in
+            deliveredNotificationIDs.append(notification.id)
+        }
+        store.configureSuppressedNotificationFeedbackHandlerForTesting { _, notification in
+            localFeedbackNotificationIDs.append(notification.id)
+        }
+        appDelegate.tabManager = manager
+        appDelegate.notificationStore = store
+        AppFocusState.overrideIsFocused = true
+
+        defer {
+            store.replaceNotificationsForTesting([])
+            appDelegate.tabManager = originalTabManager
+            appDelegate.notificationStore = originalNotificationStore
+            AppFocusState.overrideIsFocused = originalAppFocusOverride
+        }
+
+        guard let workspace = manager.selectedWorkspace,
+              let terminalPanel = workspace.focusedTerminalPanel else {
+            XCTFail("Expected selected workspace with a focused terminal panel")
+            return
+        }
+
+        store.addNotification(
+            tabId: workspace.id,
+            surfaceId: terminalPanel.id,
+            title: "Unread",
+            subtitle: "",
+            body: ""
+        )
+
+        let createdNotificationID = try XCTUnwrap(store.notifications.first?.id)
+        XCTAssertTrue(store.hasUnreadNotification(forTabId: workspace.id, surfaceId: terminalPanel.id))
+        XCTAssertTrue(deliveredNotificationIDs.isEmpty)
+        XCTAssertEqual(localFeedbackNotificationIDs.count, 1)
+        XCTAssertEqual(localFeedbackNotificationIDs, [createdNotificationID])
+    }
+
+    func testFocusedTerminalSuppressedNotificationRunsCustomCommand() throws {
+        guard let appDelegate = AppDelegate.shared else {
+            XCTFail("AppDelegate.shared must be set for this test")
+            return
+        }
+        let manager = TabManager()
+        let store = TerminalNotificationStore.shared
+        let defaults = UserDefaults.standard
+        let commandOutputURL = FileManager.default.temporaryDirectory
+            .appendingPathComponent("cmux-notification-command-\(UUID().uuidString).txt", isDirectory: false)
+
+        let originalTabManager = appDelegate.tabManager
+        let originalNotificationStore = appDelegate.notificationStore
+        let originalAppFocusOverride = AppFocusState.overrideIsFocused
+        let hadSoundValue = defaults.object(forKey: NotificationSoundSettings.key) != nil
+        let originalSoundValue = defaults.object(forKey: NotificationSoundSettings.key)
+        let hadCommandValue = defaults.object(forKey: NotificationSoundSettings.customCommandKey) != nil
+        let originalCommandValue = defaults.object(forKey: NotificationSoundSettings.customCommandKey)
+
+        var deliveredNotificationIDs: [UUID] = []
+
+        store.replaceNotificationsForTesting([])
+        store.configureNotificationDeliveryHandlerForTesting { _, notification in
+            deliveredNotificationIDs.append(notification.id)
+        }
+        appDelegate.tabManager = manager
+        appDelegate.notificationStore = store
+        AppFocusState.overrideIsFocused = true
+        defaults.set("none", forKey: NotificationSoundSettings.key)
+        defaults.set(
+            "printf '%s\\n%s\\n%s' \"$CMUX_NOTIFICATION_TITLE\" \"$CMUX_NOTIFICATION_SUBTITLE\" \"$CMUX_NOTIFICATION_BODY\" > '\(commandOutputURL.path)'",
+            forKey: NotificationSoundSettings.customCommandKey
+        )
+
+        defer {
+            store.replaceNotificationsForTesting([])
+            appDelegate.tabManager = originalTabManager
+            appDelegate.notificationStore = originalNotificationStore
+            AppFocusState.overrideIsFocused = originalAppFocusOverride
+            if hadSoundValue {
+                defaults.set(originalSoundValue, forKey: NotificationSoundSettings.key)
+            } else {
+                defaults.removeObject(forKey: NotificationSoundSettings.key)
+            }
+            if hadCommandValue {
+                defaults.set(originalCommandValue, forKey: NotificationSoundSettings.customCommandKey)
+            } else {
+                defaults.removeObject(forKey: NotificationSoundSettings.customCommandKey)
+            }
+            try? FileManager.default.removeItem(at: commandOutputURL)
+        }
+
+        guard let workspace = manager.selectedWorkspace,
+              let terminalPanel = workspace.focusedTerminalPanel else {
+            XCTFail("Expected selected workspace with a focused terminal panel")
+            return
+        }
+
+        store.addNotification(
+            tabId: workspace.id,
+            surfaceId: terminalPanel.id,
+            title: "",
+            subtitle: "Focused subtitle",
+            body: "Focused body"
+        )
+
+        let commandFinished = XCTNSPredicateExpectation(
+            predicate: NSPredicate { _, _ in
+                FileManager.default.fileExists(atPath: commandOutputURL.path)
+            },
+            object: NSObject()
+        )
+        XCTAssertEqual(XCTWaiter().wait(for: [commandFinished], timeout: 2.0), .completed)
+        XCTAssertTrue(deliveredNotificationIDs.isEmpty)
+
+        let output = try String(contentsOf: commandOutputURL, encoding: .utf8)
+            .trimmingCharacters(in: .whitespacesAndNewlines)
+        let expectedTitle = Bundle.main.object(forInfoDictionaryKey: "CFBundleDisplayName") as? String
+            ?? Bundle.main.object(forInfoDictionaryKey: "CFBundleName") as? String
+            ?? "cmux"
+        XCTAssertEqual(output.components(separatedBy: "\n"), [expectedTitle, "Focused subtitle", "Focused body"])
+    }
+
     func testNotificationAuthorizationStateMappingCoversKnownUNAuthorizationStatuses() {
         XCTAssertEqual(TerminalNotificationStore.authorizationState(from: .notDetermined), .notDetermined)
         XCTAssertEqual(TerminalNotificationStore.authorizationState(from: .denied), .denied)