Fix macOS notification settings test dispatch

Remove the stale renderer Web Notification permission gate from the Settings test notification path and cover native dispatch behavior with a regression test.
This commit is contained in:
Neil 2026-05-20 21:57:28 -07:00 committed by GitHub
parent 20099eb82e
commit 4e9bfad20a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 100 additions and 57 deletions

View File

@ -0,0 +1,68 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { GlobalSettings, NotificationDispatchRequest } from '../../../../shared/types'
import { sendNotificationSettingsTestNotification } from './NotificationsPane'
const { toastError, toastSuccess } = vi.hoisted(() => ({
toastError: vi.fn(),
toastSuccess: vi.fn()
}))
vi.mock('sonner', () => ({
toast: {
error: toastError,
success: toastSuccess
}
}))
function createSettings(): GlobalSettings {
return {
notifications: {
enabled: true,
agentTaskComplete: true,
terminalBell: true,
suppressWhenFocused: true,
customSoundPath: null,
customSoundVolume: 50
}
} as GlobalSettings
}
describe('NotificationsPane', () => {
beforeEach(() => {
toastError.mockClear()
toastSuccess.mockClear()
})
afterEach(() => {
vi.unstubAllGlobals()
})
it('uses native main-process delivery even when renderer permission is stale denied on macOS', async () => {
const notifications = {
getPermissionStatus: vi.fn(async () => ({
supported: true,
platform: 'darwin' as NodeJS.Platform,
requested: true
})),
dispatch: vi.fn(async (_args: NotificationDispatchRequest) => ({ delivered: true })),
playSound: vi.fn(),
openSystemSettings: vi.fn(),
requestPermission: vi.fn()
}
vi.stubGlobal('window', {
Notification: { permission: 'denied' },
api: {
notifications,
shell: { pickAudio: vi.fn() }
}
})
await sendNotificationSettingsTestNotification(createSettings().notifications, 50)
// Why: this UI sends via Electron's main-process Notification module;
// renderer Web Notification.permission can stay stale after macOS Settings changes.
expect(notifications.dispatch).toHaveBeenCalledWith({ source: 'test' })
expect(toastError).not.toHaveBeenCalled()
expect(toastSuccess).toHaveBeenCalledWith('Test notification sent')
})
})

View File

@ -53,23 +53,40 @@ type NotificationsPaneProps = {
updateSettings: (updates: Partial<GlobalSettings>) => void
}
function getRendererNotificationPermission(): NotificationPermission | null {
if (typeof window.Notification === 'undefined') {
return null
export async function sendNotificationSettingsTestNotification(
notificationSettings: GlobalSettings['notifications'],
volumeDraft: number
): Promise<void> {
const permissionStatus = await window.api.notifications.getPermissionStatus()
if (!permissionStatus.supported) {
toast.error('Notifications are not supported on this system')
return
}
return window.Notification.permission
}
function showNotificationPermissionDeniedToast(): void {
toast.error('Notifications are blocked in macOS', {
description: 'Enable notifications for this Orca app in System Settings.',
action: {
label: 'Open Settings',
onClick: () => {
void window.api.notifications.openSystemSettings()
}
const result = await window.api.notifications.dispatch({ source: 'test' })
if (result.delivered) {
// Why: the Test button must always play through, even if the user clicks
// it twice in quick succession — the in-flight dedupe is for incidental
// bursts of real notifications, not for an explicit user action.
const soundResult = notificationSettings.customSoundPath
? await window.api.notifications.playSound({
force: true,
volume: volumeDraft
})
: null
if (notificationSettings.customSoundPath && soundResult && !soundResult.played) {
toast.error('Custom notification sound could not be played')
return
}
})
toast.success('Test notification sent')
return
}
toast.error(
result.reason === 'disabled'
? 'Notifications are disabled'
: 'Test notification was not delivered'
)
}
export function NotificationsPane({
@ -105,49 +122,7 @@ export function NotificationsPane({
}
const handleSendTestNotification = async (): Promise<void> => {
// Why: Electron main cannot reliably read macOS notification authorization,
// but the renderer exposes it. Without this check, dev builds can report
// "sent" while macOS silently drops the notification.
if (getRendererNotificationPermission() === 'denied') {
showNotificationPermissionDeniedToast()
return
}
const permissionStatus = await window.api.notifications.getPermissionStatus()
if (!permissionStatus.supported) {
toast.error('Notifications are not supported on this system')
return
}
const result = await window.api.notifications.dispatch({ source: 'test' })
if (result.delivered) {
// Why: the Test button must always play through, even if the user clicks
// it twice in quick succession — the in-flight dedupe is for incidental
// bursts of real notifications, not for an explicit user action.
const soundResult = notificationSettings.customSoundPath
? await window.api.notifications.playSound({
force: true,
volume: volumeDraft
})
: null
if (notificationSettings.customSoundPath && soundResult && !soundResult.played) {
toast.error('Custom notification sound could not be played')
return
}
toast.success('Test notification sent')
return
}
if (getRendererNotificationPermission() === 'denied') {
showNotificationPermissionDeniedToast()
return
}
toast.error(
result.reason === 'disabled'
? 'Notifications are disabled'
: 'Test notification was not delivered'
)
await sendNotificationSettingsTestNotification(notificationSettings, volumeDraft)
}
const handleChooseSound = async (): Promise<void> => {