Skip to content

fix(notifications): mute system notification for the viewed surface#562

Merged
sbertix merged 2 commits into
supabitapp:mainfrom
mykhaliuk:fix/notification-focus-suppression
Jul 2, 2026
Merged

fix(notifications): mute system notification for the viewed surface#562
sbertix merged 2 commits into
supabitapp:mainfrom
mykhaliuk:fix/notification-focus-suppression

Conversation

@mykhaliuk

Copy link
Copy Markdown
Contributor

Fixes #558

What

  • WorktreeTerminalState.appendNotification now derives isViewed: the worktree is selected, the surface is the focused one of the selected tab, and the window is key and visible (lastWindowIsKey / lastWindowIsVisible from syncFocus; isSelected() guards against stale window flags after switching worktrees). The same composite replaces the previous isRead derivation, which was missing the window-key check.
  • The flag rides along onNotificationReceivedTerminalClient.Event.notificationReceived(isViewed:), and AppFeature skips the systemNotificationClient.send / notificationSoundClient.play effects when the surface is viewed.
  • As discussed in the issue, the behavior is exposed as a Mute notifications for active surface toggle under Settings → Notifications (GlobalSettings.muteNotificationsForActiveSurface, default true), applied symmetrically to the banner and the fallback sound. The toggle disables itself when both delivery channels are off, driven by SettingsFeature.State.hasActiveNotificationChannel.

Untouched on purpose

  • The sidebar worktreeNotificationReceived action still fires for every notification.
  • SystemNotificationClient.willPresent keeps forcing banners for the active app: a notification from a non-selected worktree must still show while Supacode is frontmost.

Tests

  • AppFeatureSystemNotificationTests: viewed + mute-on skips the banner and the sound; viewed + mute-off fires both; existing cases updated for the new event arity.
  • SettingsFeatureTests.hasActiveNotificationChannelReflectsDeliveryToggles: all four delivery-toggle combinations.
  • AgentBusyStateTests: callback closures updated.

Derive isViewed (worktree selected, surface focused, window key and
visible) in appendNotification, reuse it for the inbox isRead flag, and
forward it through TerminalClient.Event.notificationReceived so
AppFeature skips the banner and fallback sound for a surface the user
is already watching. Gated by a new "Mute notifications for active
surface" toggle (default on) under Settings > Notifications.

Fixes supabitapp#558
@mykhaliuk mykhaliuk force-pushed the fix/notification-focus-suppression branch from 8d9b255 to 80e7e74 Compare July 2, 2026 12:57
Gate isViewedSurface on the tab's zoom-aware visible leaves so a focused
pane hidden behind a split zoom no longer suppresses its notification.

Cover the isViewedSurface composite directly (selected, focused, window
key/visible, zoom-hidden, unsynced) and pin that a viewed surface skips
both delivery channels. Sync window focus in the pre-existing focused-and-
selected inbox test so it reflects the full viewed condition.

@sbertix sbertix left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks 🙇‍♂️

@sbertix sbertix enabled auto-merge (squash) July 2, 2026 15:01
@sbertix sbertix merged commit f15420c into supabitapp:main Jul 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System notification fires even when the notifying surface is focused and the app is active

2 participants