Fix focused OSC 777 notifications

[austinywang]

Summary\n- add a regression test for OSC 777 notifications emitted from the focused terminal\n- preserve Ghostty terminal escape notifications even when the originating pane is focused\n- keep generic app-generated focused notifications suppressed while using callback-context IDs for the surface path\n\n## Verification\n- ./scripts/reload.sh --tag issue-1103-osc-777-notifications\n- manual socket check: inactive OSC 777 stored unread, focused OSC 777 stored read, focused app-generated notification remains suppressed\n\nCloses #1103

---

Summary by cubic

Fixes OSC 777 notifications from the focused terminal pane so they still deliver and are stored as read. App-generated focused notifications remain suppressed. Closes #1103.

• Bug Fixes
  • Added TerminalNotificationSource and only suppress focused .app notifications; allow .terminalEscapeSequence while marking them read.
  • Used callbackTabId/callbackSurfaceId to correctly attribute notifications, and set source to .terminalEscapeSequence.
  • Added focused OSC 777 regression tests in both test suites.

^{Written for commit 76db700. Summary will update on new commits.}

Summary by CodeRabbit

• New Features

  • Enhanced notification source categorization to distinguish between app-generated and terminal-originated notifications.
  • Improved notification delivery when the application window is focused—notifications are now automatically marked as read to reduce clutter.

• Tests

  • Added test coverage for notification behavior when the application is focused.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Mar 9, 2026 8:48pm

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b0d3d06a-43e1-40e9-a7a2-faeec22aff92

📥 Commits

Reviewing files that changed from the base of the PR and between c447bee and 76db700.

📒 Files selected for processing (4)

• Sources/GhosttyTerminalView.swift
• Sources/TerminalNotificationStore.swift
• tests/test_notifications.py
• tests_v2/test_notifications.py

---

📝 Walkthrough

Walkthrough

The changes implement source-aware notification handling that categorizes notifications by origin (app or terminal escape sequence) and introduces focused-state detection to automatically mark notifications as read when the user is actively engaged with the originating pane, addressing issue #1103.

Changes

Cohort / File(s)	Summary
Core notification logic Sources/TerminalNotificationStore.swift, Sources/GhosttyTerminalView.swift	Introduces TerminalNotificationSource enum to distinguish between app-initiated and terminal escape sequence notifications. Adds source parameter to addNotification method with default value .app. Implements focused-state detection (isFocusedDeliveryTarget) to determine whether notifications should be marked as read and delivery behavior based on app and pane focus status.
Notification delivery tests tests/test_notifications.py, tests_v2/test_notifications.py	Adds new test function test_rxvt_notification_osc777_when_focused to validate that OSC 777 notifications are correctly marked as read when the application and originating pane are in focus, mirroring the existing unfocused test pattern.

Sequence Diagram

sequenceDiagram
    participant Terminal as Terminal Process
    participant ESC as Escape Sequence<br/>Handler
    participant Store as TerminalNotificationStore
    participant Delivery as Notification<br/>Delivery System
    participant UI as Application<br/>UI State
    
    rect rgba(100, 150, 200, 0.5)
    Note over Terminal,UI: OSC 777 Notification Received
    Terminal->>ESC: Send OSC 777 sequence
    ESC->>Store: addNotification(source: .terminalEscapeSequence)
    end
    
    rect rgba(150, 100, 200, 0.5)
    Note over Store,UI: Check Focus State
    Store->>UI: Check isAppFocused & isFocusedPanel
    UI-->>Store: Focus state (true/false)
    Store->>Store: Calculate isFocusedDeliveryTarget
    end
    
    rect rgba(200, 150, 100, 0.5)
    Note over Store,Delivery: Conditional Delivery
    alt App is Focused
        Store->>Delivery: Mark isRead=true, suppress pending
    else App is Not Focused
        Store->>Delivery: Mark isRead=false, queue for delivery
    end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fix notification unread persistence when workspaces regain focus #971: Modifies TerminalNotificationStore.addNotification behavior and read/delivery handling for focused panels with source-aware notification logic, directly related to focus-state notification suppression patterns.

Poem

🐰 Hop! The notifications now know where they came from—
App or terminal, focused or not,
When you're typing swift, we mark them as read,
No unread churn in your notification bed! 🎯✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the primary change: fixing OSC 777 notifications when the terminal is focused.
Description check	✅ Passed	The description covers the main changes, testing approach, and verification steps, though it lacks a demo video link and has incomplete checklist items.
Linked Issues check	✅ Passed	The PR successfully addresses issue #1103 by restoring OSC 777 notification support, ensuring focused notifications are properly handled and stored.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing OSC 777 notifications and include related regression tests; no out-of-scope modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch issue-1103-osc-777-notifications

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes a regression where OSC 777 / Ghostty terminal escape sequence notifications were silently dropped when the originating pane was focused. It introduces a TerminalNotificationSource enum (.app vs .terminalEscapeSequence) to distinguish terminal-generated notifications from internal app-generated ones: the former now bypass the focused-suppression early-return and are delivered to the OS notification center, stored as isRead: true to avoid unread indicator churn; the latter continue to be suppressed as before. The surface-level handler also switches to callbackTabId/callbackSurfaceId over the surfaceView properties for more reliable surface identification.

Key concerns found during review:

• moveTabToTop now fires for active-pane notifications: Bypassing the early return also bypasses the guard that previously prevented tab auto-reorder from triggering when the user is already on the relevant tab. When WorkspaceAutoReorderSettings is enabled this could cause unexpected tab-strip reordering.
• markRead(id:) no longer removes pre-read notifications from the OS Notification Center: Notifications born with isRead: true will be skipped entirely by markRead, so even if a user clicks the OS banner it will not be dismissed from Notification Center by cmux.
• The new regression test (test_rxvt_notification_osc777_when_focused) is correctly mirrored in both tests/ and tests_v2/, and accurately validates the new "delivered but stored as read" semantics.

Confidence Score: 3/5

• The core logic change is sound but two side-effects introduced by bypassing the early-return path need attention before merging.
• The primary fix (preserving focused OSC 777 notifications via a source enum) is well-reasoned and has good test coverage. However, bypassing the early-return unintentionally enables moveTabToTop for the already-active tab, and the isRead: true at creation time makes markRead(id:) a no-op — leaving OS notification banners undismissed after user interaction. Both are real behavioral regressions in adjacent functionality.
• Sources/TerminalNotificationStore.swift — specifically the moveTabToTop guard and the markRead(id:) dismissal path for pre-read notifications.

Important Files Changed

Filename	Overview
Sources/TerminalNotificationStore.swift	Introduces TerminalNotificationSource enum and changes focused-notification suppression logic: .app notifications are still suppressed when focused, while .terminalEscapeSequence notifications proceed to delivery (stored as read). The early-return bypass also means moveTabToTop now fires for focused escape-sequence notifications — a side effect that wasn't present before.
Sources/GhosttyTerminalView.swift	Both GHOSTTY_ACTION_DESKTOP_NOTIFICATION handler sites (app-level and surface-level) now pass source: .terminalEscapeSequence. The surface-level handler also switches to callbackTabId ?? surfaceView.tabId and callbackSurfaceId ?? surfaceView.terminalSurface?.id for more reliable surface identification.
tests/test_notifications.py	Adds test_rxvt_notification_osc777_when_focused, which correctly verifies that a focused OSC 777 notification is stored (count=1) with the right title/body and is_read: true. Test leaves app focused (set_app_focus(True) is never reset), though all subsequent tests set their own focus state explicitly so there's no cascading failure.
tests_v2/test_notifications.py	Identical changes to tests/test_notifications.py — same new test and same test registration. Both test suites are kept in sync.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[OSC 777 / GHOSTTY_ACTION_DESKTOP_NOTIFICATION] --> B{target == SURFACE?}
    B -- No, app-level --> C[tabId = selectedTabId\nsurfaceId = focusedSurfaceId]
    B -- Yes, surface-level --> D[tabId = callbackTabId ?? surfaceView.tabId\nsurfaceId = callbackSurfaceId ?? surfaceView.terminalSurface?.id]
    C --> E[addNotification\nsource: .terminalEscapeSequence]
    D --> E
    E --> F{isFocusedDeliveryTarget\nAND source == .app?}
    F -- Yes --> G[Clear old notifications\nReturn early — suppressed]
    F -- No --> H[moveTabToTop if autoReorder]
    H --> I[Create notification\nisRead = isFocusedDeliveryTarget]
    I --> J[scheduleUserNotification]
    J --> K{isFocusedDeliveryTarget?}
    K -- Yes, focused OSC 777 --> L[Stored as read\nOS banner delivered]
    K -- No, unfocused --> M[Stored as unread\nOS banner delivered]

Loading

Comments Outside Diff (2)

1. Sources/TerminalNotificationStore.swift, line 861-863 (link)

   Unintended moveTabToTop side-effect for focused pane

   Before this PR, every focused notification hit the early return (line ~858) before reaching the WorkspaceAutoReorderSettings block, so moveTabToTop was never called for a notification generated from the active pane. Now that .terminalEscapeSequence notifications bypass that early return even when isFocusedDeliveryTarget is true, moveTabToTop(tabId) will fire for the tab the user is currently working in.

   Depending on the semantics of moveTabToTop (e.g. it may trigger a UI reorder or animation), this could cause unexpected churn in the tab strip when using OSC 777 from the focused terminal while auto-reorder is enabled. Consider guarding the reorder call with !isFocusedDeliveryTarget:

2. Sources/TerminalNotificationStore.swift, line 884-891 (link)

   markRead(id:) silently skips OS notification dismissal for pre-read notifications

   Focused OSC 777 notifications are now stored with isRead: true at creation time. If the user later clicks the system notification banner, markRead(id:) will hit the guard on line 887 (guard !updated[index].isRead else { return }) and exit without ever calling center.removeDeliveredNotificationsOffMain(withIdentifiers: [id.uuidString]). The OS notification banner will therefore persist in Notification Center even after user interaction.

   If dismissing the OS notification on click is desired for these "pre-read" notifications, you could skip the isRead state mutation but still call the removal:

   func markRead(id: UUID) {
       var updated = notifications
       guard let index = updated.firstIndex(where: { $0.id == id }) else { return }
       // Always remove from OS Notification Center, even if already marked read.
       center.removeDeliveredNotificationsOffMain(withIdentifiers: [id.uuidString])
       guard !updated[index].isRead else { return }
       updated[index].isRead = true
       notifications = updated
   }

_{Last reviewed commit: 76db700}

[cubic-dev-ai]

No issues found across 4 files