Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
let tab = tabManager.tabs.first(where: { $0.id == tabId }) {
tab.triggerNotificationFocusFlash(panelId: surfaceId, requiresSplit: false, shouldFocus: false)
}
notificationStore.markRead(forTabId: tabId, surfaceId: surfaceId)
}

func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {
Expand Down Expand Up @@ -7991,16 +7990,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
)
#endif

if let notificationId, let store = notificationStore {
markReadIfFocused(
notificationId: notificationId,
tabId: tabId,
surfaceId: surfaceId,
tabManager: context.tabManager,
notificationStore: store
)
}

#if DEBUG
recordMultiWindowNotificationFocusIfNeeded(
windowId: context.windowId,
Expand Down Expand Up @@ -8054,15 +8043,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
)
#endif

if let notificationId, let store = notificationStore {
markReadIfFocused(
notificationId: notificationId,
tabId: tabId,
surfaceId: surfaceId,
tabManager: tabManager,
notificationStore: store
)
}
#if DEBUG
if ProcessInfo.processInfo.environment["CMUX_UI_TEST_JUMP_UNREAD_SETUP"] == "1" {
writeJumpUnreadTestData(["jumpUnreadOpenInFallback": "1", "jumpUnreadOpenResult": "1"])
Expand Down Expand Up @@ -8122,22 +8102,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps])
}

private func markReadIfFocused(
notificationId: UUID,
tabId: UUID,
surfaceId: UUID?,
tabManager: TabManager,
notificationStore: TerminalNotificationStore
) {
DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) {
guard tabManager.selectedTabId == tabId else { return }
if let surfaceId {
guard tabManager.focusedSurfaceId(for: tabId) == surfaceId else { return }
}
notificationStore.markRead(id: notificationId)
}
}

#if DEBUG
private func recordMultiWindowNotificationOpenFailureIfNeeded(
tabId: UUID,
Expand Down
12 changes: 5 additions & 7 deletions Sources/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ class TabManager: ObservableObject {
self.focusSelectedTabPanel(previousTabId: previousTabId)
self.updateWindowTitleForSelectedTab()
if let selectedTabId = self.selectedTabId {
self.markFocusedPanelReadIfActive(tabId: selectedTabId)
self.flashFocusedPanelIfUnreadAndActive(tabId: selectedTabId)
}
#if DEBUG
let dtMs = self.debugWorkspaceSwitchStartTime > 0
Expand Down Expand Up @@ -671,7 +671,7 @@ class TabManager: ObservableObject {
guard let self else { return }
guard let tabId = notification.userInfo?[GhosttyNotificationKey.tabId] as? UUID else { return }
guard let surfaceId = notification.userInfo?[GhosttyNotificationKey.surfaceId] as? UUID else { return }
markPanelReadOnFocusIfActive(tabId: tabId, panelId: surfaceId)
flashPanelIfUnreadAndActive(tabId: tabId, panelId: surfaceId)
}
})

Expand Down Expand Up @@ -1596,16 +1596,16 @@ class TabManager: ObservableObject {
selectedTabId != pendingTabId
}

private func markFocusedPanelReadIfActive(tabId: UUID) {
private func flashFocusedPanelIfUnreadAndActive(tabId: UUID) {
let shouldSuppressFlash = suppressFocusFlash
suppressFocusFlash = false
guard !shouldSuppressFlash else { return }
guard AppFocusState.isAppActive() else { return }
guard let panelId = focusedPanelId(for: tabId) else { return }
markPanelReadOnFocusIfActive(tabId: tabId, panelId: panelId)
flashPanelIfUnreadAndActive(tabId: tabId, panelId: panelId)
}

private func markPanelReadOnFocusIfActive(tabId: UUID, panelId: UUID) {
private func flashPanelIfUnreadAndActive(tabId: UUID, panelId: UUID) {
guard selectedTabId == tabId else { return }
guard !suppressFocusFlash else { return }
guard AppFocusState.isAppActive() else { return }
Expand All @@ -1614,7 +1614,6 @@ class TabManager: ObservableObject {
if let tab = tabs.first(where: { $0.id == tabId }) {
tab.triggerNotificationFocusFlash(panelId: panelId, requiresSplit: false, shouldFocus: false)
}
notificationStore.markRead(forTabId: tabId, surfaceId: panelId)
}

private func enqueuePanelTitleUpdate(tabId: UUID, panelId: UUID, title: String) {
Expand Down Expand Up @@ -1740,7 +1739,6 @@ class TabManager: ObservableObject {
guard let notificationStore = AppDelegate.shared?.notificationStore else { return }
guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: targetPanelId) else { return }
tab.triggerNotificationFocusFlash(panelId: targetPanelId, requiresSplit: false, shouldFocus: true)
notificationStore.markRead(forTabId: tabId, surfaceId: targetPanelId)
}
}

Expand Down
43 changes: 24 additions & 19 deletions Sources/TerminalNotificationStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -833,16 +833,9 @@ final class TerminalNotificationStore: ObservableObject {
let isFocusedSurface = surfaceId == nil || focusedSurfaceId == surfaceId
let isFocusedPanel = isActiveTab && isFocusedSurface
let isAppFocused = AppFocusState.isAppFocused()
if isAppFocused && isFocusedPanel {
if !idsToClear.isEmpty {
notifications = updated
center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear)
center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear)
}
return
}
let suppressNativeDelivery = isAppFocused && isFocusedPanel

if WorkspaceAutoReorderSettings.isEnabled() {
if WorkspaceAutoReorderSettings.isEnabled() && !suppressNativeDelivery {
AppDelegate.shared?.tabManager?.moveTabToTop(tabId)
}

Expand All @@ -862,7 +855,11 @@ final class TerminalNotificationStore: ObservableObject {
center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear)
center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear)
}
scheduleUserNotification(notification)
if suppressNativeDelivery {
Self.runNotificationCustomCommand(notification)
} else {
scheduleUserNotification(notification)
}
}

func markRead(id: UUID) {
Expand Down Expand Up @@ -993,10 +990,7 @@ final class TerminalNotificationStore: ObservableObject {
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.notificationDisplayTitle(notification)
content.subtitle = notification.subtitle
content.body = notification.body
content.sound = NotificationSoundSettings.sound()
Expand All @@ -1019,16 +1013,27 @@ final class TerminalNotificationStore: ObservableObject {
if let error {
NSLog("Failed to schedule notification: \(error)")
} else {
NotificationSoundSettings.runCustomCommand(
title: content.title,
subtitle: content.subtitle,
body: content.body
)
Self.runNotificationCustomCommand(notification)
}
}
}
}

nonisolated private static func notificationDisplayTitle(_ 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
}

nonisolated private static func runNotificationCustomCommand(_ notification: TerminalNotification) {
NotificationSoundSettings.runCustomCommand(
title: notificationDisplayTitle(notification),
subtitle: notification.subtitle,
body: notification.body
)
}

private func ensureAuthorization(
origin: AuthorizationRequestOrigin,
_ completion: @escaping (Bool) -> Void
Expand Down
151 changes: 151 additions & 0 deletions cmuxTests/CmuxWebViewKeyEquivalentTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6658,6 +6658,8 @@ final class NotificationDockBadgeTests: XCTestCase {
}

override func tearDown() {
AppFocusState.overrideIsFocused = nil
AppDelegate.shared = nil
TerminalNotificationStore.shared.resetNotificationSettingsPromptHooksForTesting()
TerminalNotificationStore.shared.replaceNotificationsForTesting([])
super.tearDown()
Expand Down Expand Up @@ -7161,6 +7163,155 @@ final class NotificationDockBadgeTests: XCTestCase {
XCTAssertEqual(store.latestNotification(forTabId: tabB)?.id, notificationBUnread.id)
}

func testFocusedTabNotificationIsStoredWhenNativeDeliveryIsSuppressed() {
let store = TerminalNotificationStore.shared
store.replaceNotificationsForTesting([])

let appDelegate = AppDelegate()
let tabManager = TabManager()
appDelegate.tabManager = tabManager
AppDelegate.shared = appDelegate
AppFocusState.overrideIsFocused = true

guard let tabId = tabManager.selectedTabId else {
XCTFail("Expected selected tab for notification test")
return
}

store.addNotification(
tabId: tabId,
surfaceId: nil,
title: "Needs input",
subtitle: "",
body: "agent requires user action"
)

XCTAssertEqual(store.unreadCount(forTabId: tabId), 1)
guard let latest = store.latestNotification(forTabId: tabId) else {
XCTFail("Expected notification to be stored for focused tab")
return
}
XCTAssertEqual(latest.tabId, tabId)
XCTAssertEqual(latest.title, "Needs input")
XCTAssertEqual(latest.body, "agent requires user action")
XCTAssertFalse(latest.isRead)
}

func testApplicationDidBecomeActiveDoesNotMarkFocusedNotificationRead() {
let store = TerminalNotificationStore.shared
let appDelegate = AppDelegate()
let tabManager = TabManager()
appDelegate.tabManager = tabManager
appDelegate.notificationStore = store
AppDelegate.shared = appDelegate
AppFocusState.overrideIsFocused = true

guard let tabId = tabManager.selectedTabId,
let surfaceId = tabManager.focusedSurfaceId(for: tabId) else {
XCTFail("Expected selected tab and focused surface for activation test")
return
}

let notification = TerminalNotification(
id: UUID(),
tabId: tabId,
surfaceId: surfaceId,
title: "Unread",
subtitle: "",
body: "should persist across app activation",
createdAt: Date(),
Comment thread
austinywang marked this conversation as resolved.
isRead: false
)
store.replaceNotificationsForTesting([notification])

appDelegate.applicationDidBecomeActive(
Notification(name: NSApplication.didBecomeActiveNotification)
)

XCTAssertTrue(store.hasUnreadNotification(forTabId: tabId, surfaceId: surfaceId))
XCTAssertFalse(store.notifications[0].isRead)
}

func testSelectingWorkspaceDoesNotMarkFocusedNotificationRead() {
let store = TerminalNotificationStore.shared
let appDelegate = AppDelegate()
let tabManager = TabManager()
appDelegate.tabManager = tabManager
appDelegate.notificationStore = store
AppDelegate.shared = appDelegate
AppFocusState.overrideIsFocused = true

guard let originalTabId = tabManager.selectedTabId,
let originalSurfaceId = tabManager.focusedSurfaceId(for: originalTabId) else {
XCTFail("Expected selected tab and focused surface for workspace selection test")
return
}
guard let originalWorkspace = tabManager.tabs.first(where: { $0.id == originalTabId }) else {
XCTFail("Expected original workspace for workspace selection test")
return
}

let notification = TerminalNotification(
id: UUID(),
tabId: originalTabId,
surfaceId: originalSurfaceId,
title: "Unread",
subtitle: "",
body: "should persist across workspace selection",
createdAt: Date(),
isRead: false
)
store.replaceNotificationsForTesting([notification])

_ = tabManager.addWorkspace(select: true)
tabManager.selectWorkspace(originalWorkspace)

let drained = expectation(description: "workspace selection side effects drained")
DispatchQueue.main.async { drained.fulfill() }
wait(for: [drained], timeout: 1.0)

XCTAssertEqual(tabManager.selectedTabId, originalTabId)
XCTAssertTrue(store.hasUnreadNotification(forTabId: originalTabId, surfaceId: originalSurfaceId))
XCTAssertFalse(store.notifications[0].isRead)
}

func testNotificationFocusNavigationDoesNotMarkNotificationRead() {
let store = TerminalNotificationStore.shared
let appDelegate = AppDelegate()
let tabManager = TabManager()
appDelegate.tabManager = tabManager
appDelegate.notificationStore = store
AppDelegate.shared = appDelegate
AppFocusState.overrideIsFocused = true

guard let tabId = tabManager.selectedTabId,
let surfaceId = tabManager.focusedSurfaceId(for: tabId) else {
XCTFail("Expected selected tab and focused surface for notification focus test")
return
}

let notification = TerminalNotification(
id: UUID(),
tabId: tabId,
surfaceId: surfaceId,
title: "Unread",
subtitle: "",
body: "should persist after notification focus",
createdAt: Date(),
isRead: false
)
store.replaceNotificationsForTesting([notification])

tabManager.focusTabFromNotification(tabId, surfaceId: surfaceId)

let drained = expectation(description: "notification focus drained")
DispatchQueue.main.async { drained.fulfill() }

@cubic-dev-ai cubic-dev-ai Bot Mar 6, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This async drain can finish before notification-focus delayed side effects run, so the test may assert too early and miss regressions.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cmuxTests/CmuxWebViewKeyEquivalentTests.swift, line 7308:

<comment>This async drain can finish before notification-focus delayed side effects run, so the test may assert too early and miss regressions.</comment>

<file context>
@@ -7304,7 +7305,7 @@ final class NotificationDockBadgeTests: XCTestCase {
 
         let drained = expectation(description: "notification focus drained")
-        DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { drained.fulfill() }
+        DispatchQueue.main.async { drained.fulfill() }
         wait(for: [drained], timeout: 1.0)
 
</file context>
Fix with Cubic

wait(for: [drained], timeout: 1.0)

XCTAssertTrue(store.hasUnreadNotification(forTabId: tabId, surfaceId: surfaceId))
XCTAssertFalse(store.notifications[0].isRead)
}

func testNotificationIndexesUpdateAfterReadAndClearMutations() {
let tab = UUID()
let surfaceUnread = UUID()
Expand Down
12 changes: 8 additions & 4 deletions tests/test_focus_notification_dismiss.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env python3
"""
E2E: focusing a panel clears its notification and triggers a flash.
E2E: focusing a panel preserves its notification and triggers a flash.

Note: This uses the socket focus command (no assistive access needed).
"""
Expand Down Expand Up @@ -74,8 +74,12 @@ def main() -> int:
client.send("x")
time.sleep(0.2)

if not wait_for_notification(client, surface_id, is_read=True, timeout=2.0):
print("FAIL: Notification did not become read after focus")
if wait_for_notification(client, surface_id, is_read=True, timeout=2.0):
print("FAIL: Notification became read after focus")
return 1
items = client.list_notifications()
if not any(item["surface_id"] == surface_id and not item["is_read"] for item in items):
print("FAIL: Notification did not remain present and unread after focus")
return 1

final_flash = client.flash_count(term_b)
Expand All @@ -93,7 +97,7 @@ def main() -> int:
except Exception:
pass

print("PASS: Focus clears notification and flashes panel")
print("PASS: Focus preserves notification and flashes panel")
return 0
except (cmuxError, RuntimeError) as exc:
print(f"FAIL: {exc}")
Expand Down
Loading