Fix notification settings recovery copy

This commit is contained in:
Neil 2026-05-20 23:10:06 -07:00 committed by GitHub
parent 831abbc84f
commit 84614a4c7c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 234 additions and 20 deletions

View File

@ -203,6 +203,30 @@ describe('registerNotificationHandlers', () => {
}
})
it('opens Windows notification settings', () => {
const originalPlatform = process.platform
Object.defineProperty(process, 'platform', { value: 'win32', configurable: true })
try {
registerNotificationHandlers({
getSettings: () => ({
notifications: {
enabled: true,
agentTaskComplete: true,
terminalBell: true,
suppressWhenFocused: true
}
})
} as never)
const handler = getOpenSystemSettingsHandler()
handler({})
expect(shellOpenExternalMock).toHaveBeenCalledWith('ms-settings:notifications')
} finally {
Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true })
}
})
it('suppresses notifications when disabled in settings', () => {
registerNotificationHandlers({
getSettings: () => ({

View File

@ -2,14 +2,16 @@ 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(() => ({
const { toastError, toastMessage, toastSuccess } = vi.hoisted(() => ({
toastError: vi.fn(),
toastMessage: vi.fn(),
toastSuccess: vi.fn()
}))
vi.mock('sonner', () => ({
toast: {
error: toastError,
message: toastMessage,
success: toastSuccess
}
}))
@ -30,6 +32,7 @@ function createSettings(): GlobalSettings {
describe('NotificationsPane', () => {
beforeEach(() => {
toastError.mockClear()
toastMessage.mockClear()
toastSuccess.mockClear()
})
@ -66,6 +69,46 @@ describe('NotificationsPane', () => {
requireDisplayConfirmation: true
})
expect(toastError).not.toHaveBeenCalled()
expect(toastSuccess).not.toHaveBeenCalled()
expect(toastMessage).toHaveBeenCalledWith(
'Test notification requested',
expect.objectContaining({
description: 'If no macOS banner appeared, enable Allow notifications for Orca.',
action: expect.objectContaining({ label: 'Open Settings' })
})
)
const toastOptions = toastMessage.mock.calls[0]?.[1] as
| { action?: { onClick?: () => void } }
| undefined
toastOptions?.action?.onClick?.()
expect(notifications.openSystemSettings).toHaveBeenCalledTimes(1)
})
it('confirms delivered test notifications on platforms where show means displayed', async () => {
const notifications = {
getPermissionStatus: vi.fn(async () => ({
supported: true,
platform: 'win32' 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: 'granted' },
api: {
notifications,
shell: { pickAudio: vi.fn() }
}
})
await sendNotificationSettingsTestNotification(createSettings().notifications, 50)
expect(toastMessage).not.toHaveBeenCalled()
expect(toastError).not.toHaveBeenCalled()
expect(toastSuccess).toHaveBeenCalledWith('Test notification sent')
})
@ -108,4 +151,74 @@ describe('NotificationsPane', () => {
toastOptions?.action?.onClick?.()
expect(notifications.openSystemSettings).toHaveBeenCalledTimes(1)
})
it('uses Windows notification settings copy when the native test notification is not shown', async () => {
const notifications = {
getPermissionStatus: vi.fn(async () => ({
supported: true,
platform: 'win32' as NodeJS.Platform,
requested: true
})),
dispatch: vi.fn(async (_args: NotificationDispatchRequest) => ({
delivered: false,
reason: 'not-displayed' as const
})),
playSound: vi.fn(),
openSystemSettings: vi.fn(),
requestPermission: vi.fn()
}
vi.stubGlobal('window', {
Notification: { permission: 'granted' },
api: {
notifications,
shell: { pickAudio: vi.fn() }
}
})
await sendNotificationSettingsTestNotification(createSettings().notifications, 50)
expect(toastSuccess).not.toHaveBeenCalled()
expect(toastError).toHaveBeenCalledWith(
'Windows did not show the notification',
expect.objectContaining({
description: 'Enable notifications for Orca in Windows Settings.',
action: expect.objectContaining({ label: 'Open Settings' })
})
)
})
it('does not show an inert settings action on platforms without a settings shortcut', async () => {
const notifications = {
getPermissionStatus: vi.fn(async () => ({
supported: true,
platform: 'linux' as NodeJS.Platform,
requested: true
})),
dispatch: vi.fn(async (_args: NotificationDispatchRequest) => ({
delivered: false,
reason: 'not-displayed' as const
})),
playSound: vi.fn(),
openSystemSettings: vi.fn(),
requestPermission: vi.fn()
}
vi.stubGlobal('window', {
Notification: { permission: 'granted' },
api: {
notifications,
shell: { pickAudio: vi.fn() }
}
})
await sendNotificationSettingsTestNotification(createSettings().notifications, 50)
expect(toastSuccess).not.toHaveBeenCalled()
expect(toastError).toHaveBeenCalledWith(
'System did not show the notification',
expect.not.objectContaining({
action: expect.anything()
})
)
expect(notifications.openSystemSettings).not.toHaveBeenCalled()
})
})

View File

@ -1,6 +1,6 @@
import { type ReactNode, useEffect, useRef, useState } from 'react'
import { toast } from 'sonner'
import type { GlobalSettings } from '../../../../shared/types'
import type { GlobalSettings, NotificationPermissionStatusResult } from '../../../../shared/types'
import { Button } from '../ui/button'
import { Label } from '../ui/label'
import { Separator } from '../ui/separator'
@ -53,6 +53,34 @@ type NotificationsPaneProps = {
updateSettings: (updates: Partial<GlobalSettings>) => void
}
type SystemNotificationSettingsCopy = {
buttonLabel: string
failureTitle: string
failureDescription: string
}
function getSystemNotificationSettingsCopy(
platform: NodeJS.Platform
): SystemNotificationSettingsCopy | null {
if (platform === 'darwin') {
return {
buttonLabel: 'macOS Settings',
failureTitle: 'macOS did not show the notification',
failureDescription: 'Enable Allow notifications for Orca in System Settings.'
}
}
if (platform === 'win32') {
return {
buttonLabel: 'Windows Settings',
failureTitle: 'Windows did not show the notification',
failureDescription: 'Enable notifications for Orca in Windows Settings.'
}
}
return null
}
export async function sendNotificationSettingsTestNotification(
notificationSettings: GlobalSettings['notifications'],
volumeDraft: number
@ -81,20 +109,42 @@ export async function sendNotificationSettingsTestNotification(
toast.error('Custom notification sound could not be played')
return
}
const settingsCopy = getSystemNotificationSettingsCopy(permissionStatus.platform)
if (permissionStatus.platform === 'darwin' && settingsCopy) {
// Why: Electron's native 'show' event can fire even when macOS silently
// drops the banner because the per-app Allow notifications switch is off.
toast.message('Test notification requested', {
description: 'If no macOS banner appeared, enable Allow notifications for Orca.',
action: {
label: 'Open Settings',
onClick: () => {
void window.api.notifications.openSystemSettings()
}
}
})
return
}
toast.success('Test notification sent')
return
}
if (result.reason === 'not-displayed') {
toast.error('macOS did not show the notification', {
description: 'Enable Allow notifications for Orca in System Settings.',
action: {
label: 'Open Settings',
onClick: () => {
void window.api.notifications.openSystemSettings()
const settingsCopy = getSystemNotificationSettingsCopy(permissionStatus.platform)
if (settingsCopy) {
toast.error(settingsCopy.failureTitle, {
description: settingsCopy.failureDescription,
action: {
label: 'Open Settings',
onClick: () => {
void window.api.notifications.openSystemSettings()
}
}
}
})
})
} else {
toast.error('System did not show the notification', {
description: 'Check your desktop notification settings for Orca.'
})
}
return
}
@ -112,6 +162,8 @@ export function NotificationsPane({
const notificationSettings = settings.notifications
const notificationSettingsRef = useRef(notificationSettings)
const [isPickingSound, setIsPickingSound] = useState(false)
const [permissionStatus, setPermissionStatus] =
useState<NotificationPermissionStatusResult | null>(null)
const updateNotificationSettings = (updates: Partial<GlobalSettings['notifications']>): void => {
updateSettings({
@ -131,6 +183,25 @@ export function NotificationsPane({
setVolumeDraft(notificationSettings.customSoundVolume)
}, [notificationSettings])
useEffect(() => {
let cancelled = false
void window.api.notifications
.getPermissionStatus()
.then((status) => {
if (!cancelled) {
setPermissionStatus(status)
}
})
.catch(() => {
if (!cancelled) {
setPermissionStatus(null)
}
})
return () => {
cancelled = true
}
}, [])
const handleVolumeCommit = (value: number): void => {
if (notificationSettingsRef.current.customSoundVolume !== value) {
updateNotificationSettings({ customSoundVolume: value })
@ -158,6 +229,9 @@ export function NotificationsPane({
}
const selectedSoundPath = notificationSettings.customSoundPath
const systemSettingsCopy = permissionStatus
? getSystemNotificationSettingsCopy(permissionStatus.platform)
: null
return (
<div className="space-y-1">
@ -298,15 +372,18 @@ export function NotificationsPane({
<BellRing className="size-3.5" />
Send Test Notification
</Button>
<Button
variant="ghost"
size="sm"
onClick={() => void handleOpenSystemSettings()}
className="gap-2"
>
<ExternalLink className="size-3.5" />
macOS Settings
</Button>
{systemSettingsCopy ? (
<Button
variant="ghost"
size="sm"
disabled={!notificationSettings.enabled}
onClick={() => void handleOpenSystemSettings()}
className="gap-2"
>
<ExternalLink className="size-3.5" />
{systemSettingsCopy.buttonLabel}
</Button>
) : null}
</div>
</div>
)

View File

@ -1855,7 +1855,7 @@ export type NotificationEventSource = 'agent-task-complete' | 'terminal-bell' |
export type NotificationDispatchRequest = {
source: NotificationEventSource
/** Why: the Settings test button must not report success unless macOS actually shows it. */
/** Why: useful for fast native failures, but macOS can still drop notifications after 'show'. */
requireDisplayConfirmation?: boolean
worktreeId?: string
/** Stable `${tabId}:${leafId}` terminal pane key for click-to-focus routing. */