diff --git a/src/main/ipc/notifications.test.ts b/src/main/ipc/notifications.test.ts index 56580c4b8a2..fbe6c508348 100644 --- a/src/main/ipc/notifications.test.ts +++ b/src/main/ipc/notifications.test.ts @@ -9,8 +9,7 @@ const { notificationOnMock, notificationCtorMock, notificationIsSupportedMock, - getAllWindowsMock, - getNotificationSettingsMock + getAllWindowsMock } = vi.hoisted(() => { const removeHandlerMock = vi.fn() const handleMock = vi.fn() @@ -26,7 +25,6 @@ const { }) const notificationIsSupportedMock = vi.fn(() => true) const getAllWindowsMock = vi.fn(() => []) - const getNotificationSettingsMock = vi.fn(() => ({ authorizationStatus: 'authorized' })) return { removeHandlerMock, handleMock, @@ -35,8 +33,7 @@ const { notificationOnMock, notificationCtorMock, notificationIsSupportedMock, - getAllWindowsMock, - getNotificationSettingsMock + getAllWindowsMock } }) @@ -54,9 +51,6 @@ vi.mock('electron', () => ({ app: { focus: vi.fn() }, - systemPreferences: { - getNotificationSettings: getNotificationSettingsMock - }, shell: { openExternal: vi.fn() } @@ -64,8 +58,7 @@ vi.mock('electron', () => ({ import { registerNotificationHandlers, - triggerStartupNotificationRegistration, - getPermissionStatus + triggerStartupNotificationRegistration } from './notifications' describe('registerNotificationHandlers', () => { @@ -82,8 +75,6 @@ describe('registerNotificationHandlers', () => { notificationIsSupportedMock.mockReturnValue(true) getAllWindowsMock.mockReset() getAllWindowsMock.mockReturnValue([]) - getNotificationSettingsMock.mockReset() - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'authorized' }) }) function getDispatchHandler(): (event: unknown, args: unknown) => unknown { @@ -130,36 +121,6 @@ describe('registerNotificationHandlers', () => { expect(notificationCtorMock).not.toHaveBeenCalled() }) - it('returns system-denied when macOS permission is denied', () => { - // Why: getPermissionStatus only consults systemPreferences.getNotificationSettings - // on macOS. On Linux CI the non-darwin branch would check Notification.isSupported() - // instead, so we must force the darwin path to exercise the 'denied' gate. - const originalPlatform = process.platform - Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) - - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'denied' }) - - registerNotificationHandlers({ - getSettings: () => ({ - notifications: { - enabled: true, - agentTaskComplete: true, - terminalBell: true, - suppressWhenFocused: true - } - }) - } as never) - - const handler = getDispatchHandler() - expect(handler({}, { source: 'test' })).toEqual({ - delivered: false, - reason: 'system-denied' - }) - expect(notificationCtorMock).not.toHaveBeenCalled() - - Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }) - }) - it('suppresses active-worktree notifications while Orca is focused', () => { getAllWindowsMock.mockReturnValue([ { @@ -272,11 +233,9 @@ describe('registerNotificationHandlers', () => { const handler = getDispatchHandler() - // Agent fires first — should deliver expect(handler({}, { source: 'agent-task-complete', worktreeId: 'repo::wt1' })).toEqual({ delivered: true }) - // Bell fires immediately after for the same worktree — should be suppressed expect(handler({}, { source: 'terminal-bell', worktreeId: 'repo::wt1' })).toEqual({ delivered: false, reason: 'cooldown' @@ -295,7 +254,6 @@ describe('triggerStartupNotificationRegistration', () => { notificationOnMock.mockClear() notificationIsSupportedMock.mockReset() notificationIsSupportedMock.mockReturnValue(true) - getNotificationSettingsMock.mockReset() Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) }) @@ -303,9 +261,7 @@ describe('triggerStartupNotificationRegistration', () => { Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }) }) - it('shows welcome notification when permission is not-determined and not yet requested', () => { - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'not determined' }) - + it('shows welcome notification when not yet requested', () => { const store = { getUI: () => ({ notificationPermissionRequested: undefined }), updateUI: vi.fn() @@ -321,23 +277,7 @@ describe('triggerStartupNotificationRegistration', () => { expect(notificationShowMock).toHaveBeenCalledTimes(1) }) - it('does not fire when permission is already authorized', () => { - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'authorized' }) - - const store = { - getUI: () => ({ notificationPermissionRequested: undefined }), - updateUI: vi.fn() - } - - triggerStartupNotificationRegistration(store as never) - - expect(notificationCtorMock).not.toHaveBeenCalled() - expect(store.updateUI).not.toHaveBeenCalled() - }) - it('does not fire when notificationPermissionRequested flag is set', () => { - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'not determined' }) - const store = { getUI: () => ({ notificationPermissionRequested: true }), updateUI: vi.fn() @@ -350,8 +290,6 @@ describe('triggerStartupNotificationRegistration', () => { it('does nothing on non-darwin platforms', () => { Object.defineProperty(process, 'platform', { value: 'linux', configurable: true }) - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'not determined' }) - const store = { getUI: () => ({ notificationPermissionRequested: undefined }), updateUI: vi.fn() @@ -362,44 +300,3 @@ describe('triggerStartupNotificationRegistration', () => { expect(notificationCtorMock).not.toHaveBeenCalled() }) }) - -describe('getPermissionStatus', () => { - const originalPlatform = process.platform - - beforeEach(() => { - getNotificationSettingsMock.mockReset() - notificationIsSupportedMock.mockReset() - Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) - }) - - afterEach(() => { - Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }) - }) - - it('returns authorized when macOS reports authorized', () => { - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'authorized' }) - expect(getPermissionStatus()).toBe('authorized') - }) - - it('returns denied when macOS reports denied', () => { - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'denied' }) - expect(getPermissionStatus()).toBe('denied') - }) - - it('returns not-determined when macOS reports not determined', () => { - getNotificationSettingsMock.mockReturnValue({ authorizationStatus: 'not determined' }) - expect(getPermissionStatus()).toBe('not-determined') - }) - - it('returns authorized on non-darwin when Notification is supported', () => { - Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }) - notificationIsSupportedMock.mockReturnValue(true) - expect(getPermissionStatus()).toBe('authorized') - }) - - it('returns denied on non-darwin when Notification is not supported', () => { - Object.defineProperty(process, 'platform', { value: 'linux', configurable: true }) - notificationIsSupportedMock.mockReturnValue(false) - expect(getPermissionStatus()).toBe('denied') - }) -}) diff --git a/src/main/ipc/notifications.ts b/src/main/ipc/notifications.ts index 5df29a3fc5d..7175230f74b 100644 --- a/src/main/ipc/notifications.ts +++ b/src/main/ipc/notifications.ts @@ -1,45 +1,12 @@ -import { app, BrowserWindow, Notification, ipcMain, systemPreferences, shell } from 'electron' +import { app, BrowserWindow, Notification, ipcMain, shell } from 'electron' import type { Store } from '../persistence' import type { NotificationDispatchRequest, NotificationDispatchResult } from '../../shared/types' const NOTIFICATION_COOLDOWN_MS = 5000 -export type NotificationPermissionStatus = 'authorized' | 'denied' | 'not-determined' | 'unknown' - -export function getPermissionStatus(): NotificationPermissionStatus { - if (process.platform !== 'darwin') { - // Windows/Linux don't have a per-app notification permission gate. - return Notification.isSupported() ? 'authorized' : 'denied' - } - // Why: getNotificationSettings() is macOS-only and absent from Electron's - // cross-platform type definitions, so we need the cast. - const getSettings = (systemPreferences as unknown as Record) - .getNotificationSettings as (() => { authorizationStatus: string }) | undefined - if (!getSettings) { - return 'unknown' - } - const settings = getSettings() - switch (settings.authorizationStatus) { - case 'authorized': - case 'provisional': - return 'authorized' - case 'denied': - return 'denied' - case 'not determined': - return 'not-determined' - default: - return 'unknown' - } -} - export function registerNotificationHandlers(store: Store): void { const recentNotifications = new Map() - ipcMain.removeHandler('notifications:getPermissionStatus') - ipcMain.handle('notifications:getPermissionStatus', (): NotificationPermissionStatus => { - return getPermissionStatus() - }) - ipcMain.removeHandler('notifications:openSystemSettings') ipcMain.handle('notifications:openSystemSettings', (): void => { if (process.platform === 'darwin') { @@ -63,16 +30,6 @@ export function registerNotificationHandlers(store: Store): void { return { delivered: false, reason: 'disabled' } } - // Why: even when in-app notifications are enabled, macOS can independently - // block them at the system level. Checking here prevents a silent no-op - // and lets the renderer show actionable feedback. We only block on 'denied' - // — 'not-determined' is allowed through because posting the notification is - // what triggers the macOS permission dialog for the first time. - const permissionStatus = getPermissionStatus() - if (permissionStatus === 'denied') { - return { delivered: false, reason: 'system-denied' } - } - if ( (args.source === 'agent-task-complete' && !settings.agentTaskComplete) || (args.source === 'terminal-bell' && !settings.terminalBell) @@ -158,10 +115,6 @@ export function triggerStartupNotificationRegistration(store: Store): void { if (process.platform !== 'darwin' || !Notification.isSupported()) { return } - const status = getPermissionStatus() - if (status !== 'not-determined') { - return - } // Why: only fire once per install — not on every launch where status stays // not-determined (e.g. if the user dismisses the macOS dialog without choosing). const ui = store.getUI() diff --git a/src/preload/index.d.ts b/src/preload/index.d.ts index 1497dcaa5e8..ab5f0e4c496 100644 --- a/src/preload/index.d.ts +++ b/src/preload/index.d.ts @@ -116,11 +116,8 @@ type CliApi = { remove: () => Promise } -type NotificationPermissionStatus = 'authorized' | 'denied' | 'not-determined' | 'unknown' - type NotificationsApi = { dispatch: (args: NotificationDispatchRequest) => Promise - getPermissionStatus: () => Promise openSystemSettings: () => Promise } diff --git a/src/preload/index.ts b/src/preload/index.ts index 03fd8891940..8fa232bf24a 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -283,8 +283,6 @@ const api = { notifications: { dispatch: (args: Record): Promise => ipcRenderer.invoke('notifications:dispatch', args), - getPermissionStatus: (): Promise => - ipcRenderer.invoke('notifications:getPermissionStatus'), openSystemSettings: (): Promise => ipcRenderer.invoke('notifications:openSystemSettings') }, diff --git a/src/renderer/src/components/settings/NotificationsPane.tsx b/src/renderer/src/components/settings/NotificationsPane.tsx index e6d8ec9b227..ea6809a0bd5 100644 --- a/src/renderer/src/components/settings/NotificationsPane.tsx +++ b/src/renderer/src/components/settings/NotificationsPane.tsx @@ -1,10 +1,10 @@ -import { type ReactNode, useCallback, useEffect, useState } from 'react' +import { type ReactNode } from 'react' import { toast } from 'sonner' import type { GlobalSettings } from '../../../../shared/types' import { Button } from '../ui/button' import { Label } from '../ui/label' import { Separator } from '../ui/separator' -import { BellRing, Bot, Siren, TriangleAlert } from 'lucide-react' +import { BellRing, Bot, Siren } from 'lucide-react' import type { SettingsSearchEntry } from './settings-search' export const NOTIFICATIONS_PANE_SEARCH_ENTRIES: SettingsSearchEntry[] = [ @@ -45,25 +45,6 @@ export function NotificationsPane({ updateSettings }: NotificationsPaneProps): React.JSX.Element { const notificationSettings = settings.notifications - const [permissionBlocked, setPermissionBlocked] = useState(false) - - const recheckPermission = useCallback(() => { - void window.api.notifications.getPermissionStatus().then((status) => { - // Why: only flag 'denied', not 'not-determined'. 'not-determined' means - // macOS hasn't prompted the user yet — the startup registration or the - // next notification dispatch will trigger that dialog. Showing "blocked" - // for a permission that was never asked would be misleading. - setPermissionBlocked(status === 'denied') - }) - }, []) - - useEffect(() => { - if (!notificationSettings.enabled) { - setPermissionBlocked(false) - return - } - recheckPermission() - }, [notificationSettings.enabled, recheckPermission]) const updateNotificationSettings = (updates: Partial): void => { updateSettings({ @@ -76,40 +57,13 @@ export function NotificationsPane({ const handleSendTestNotification = async (): Promise => { const result = await window.api.notifications.dispatch({ source: 'test' }) - if (!result.delivered && result.reason === 'system-denied') { - // Why: don't auto-open System Settings — the yellow banner above already - // provides the "Open Notification Settings" button. Just tell the user - // what happened so the silent failure isn't confusing. - toast.error('Notification blocked by macOS', { - description: 'Allow notifications for Orca in System Settings → Notifications.' - }) + if (result.delivered) { + toast.success('Test notification sent') } - // Why: the macOS permission dialog may have appeared (for 'not-determined' - // status), so re-check to update the banner accordingly. - recheckPermission() } return (
- {notificationSettings.enabled && permissionBlocked && ( -
- -
-

- Notifications are blocked by your system settings. -

- -
-
- )} -