Fix macOS notification settings recovery

Open the current app's macOS notification settings entry and stop reporting test notification success until Electron confirms the native notification was shown.
This commit is contained in:
Neil
2026-05-20 22:39:18 -07:00
committed by GitHub
parent 80657c3e21
commit 8b77dffec2
6 changed files with 249 additions and 17 deletions
+1
View File
@@ -150,6 +150,7 @@ function prepareMacDevElectronApp() {
const appPath = path.join(distDir, appBundleName)
const markerPath = path.join(distDir, 'orca-dev-electron-app.json')
const bundleId = `com.stablyai.orca.dev.${sanitizeBundleIdPart(hash)}`
process.env.ORCA_DEV_MACOS_BUNDLE_ID = bundleId
const expectedMarker = JSON.stringify(
{ title, appBundleName, bundleId, sourceAppPath, electronVersion, bundleLayoutVersion },
null,
+106 -4
View File
@@ -10,33 +10,40 @@ const {
notificationShowMock,
notificationCloseMock,
notificationOnMock,
notificationOnceMock,
notificationCtorMock,
notificationIsSupportedMock,
getAllWindowsMock
getAllWindowsMock,
shellOpenExternalMock
} = vi.hoisted(() => {
const removeHandlerMock = vi.fn()
const handleMock = vi.fn()
const notificationShowMock = vi.fn()
const notificationCloseMock = vi.fn()
const notificationOnMock = vi.fn()
const notificationOnceMock = vi.fn()
const notificationCtorMock = vi.fn(function () {
return {
show: notificationShowMock,
close: notificationCloseMock,
on: notificationOnMock
on: notificationOnMock,
once: notificationOnceMock
}
})
const notificationIsSupportedMock = vi.fn(() => true)
const getAllWindowsMock = vi.fn(() => [])
const shellOpenExternalMock = vi.fn()
return {
removeHandlerMock,
handleMock,
notificationShowMock,
notificationCloseMock,
notificationOnMock,
notificationOnceMock,
notificationCtorMock,
notificationIsSupportedMock,
getAllWindowsMock
getAllWindowsMock,
shellOpenExternalMock
}
})
@@ -55,7 +62,7 @@ vi.mock('electron', () => ({
focus: vi.fn()
},
shell: {
openExternal: vi.fn()
openExternal: shellOpenExternalMock
}
}))
@@ -83,10 +90,12 @@ describe('registerNotificationHandlers', () => {
notificationShowMock.mockClear()
notificationCloseMock.mockClear()
notificationOnMock.mockClear()
notificationOnceMock.mockClear()
notificationIsSupportedMock.mockReset()
notificationIsSupportedMock.mockReturnValue(true)
getAllWindowsMock.mockReset()
getAllWindowsMock.mockReturnValue([])
shellOpenExternalMock.mockClear()
})
afterEach(() => {
@@ -101,6 +110,16 @@ describe('registerNotificationHandlers', () => {
return call[1] as (event: unknown, args: unknown) => unknown
}
function getOpenSystemSettingsHandler(): (event: unknown) => unknown {
const call = handleMock.mock.calls.find(
(c: unknown[]) => c[0] === 'notifications:openSystemSettings'
)
if (!call) {
throw new Error('notifications:openSystemSettings handler not registered')
}
return call[1] as (event: unknown) => unknown
}
function getLoadSoundHandler(): (event: unknown) => Promise<unknown> {
const call = handleMock.mock.calls.find((c: unknown[]) => c[0] === 'notifications:loadSound')
if (!call) {
@@ -127,6 +146,14 @@ describe('registerNotificationHandlers', () => {
return call[1] as () => void
}
function getNotificationOnceEventHandler(eventName: string): () => void {
const call = notificationOnceMock.mock.calls.find((c: unknown[]) => c[0] === eventName)
if (!call) {
throw new Error(`Notification ${eventName} once handler not registered`)
}
return call[1] as () => void
}
it('registers the IPC handler', () => {
registerNotificationHandlers({
getSettings: () => ({
@@ -143,6 +170,39 @@ describe('registerNotificationHandlers', () => {
expect(handleMock).toHaveBeenCalledWith('notifications:dispatch', expect.any(Function))
})
it('opens the current macOS app notification settings entry', () => {
const originalPlatform = process.platform
const originalBundleId = process.env.ORCA_DEV_MACOS_BUNDLE_ID
Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true })
process.env.ORCA_DEV_MACOS_BUNDLE_ID = 'com.stablyai.orca.dev.fb5a47066f08'
try {
registerNotificationHandlers({
getSettings: () => ({
notifications: {
enabled: true,
agentTaskComplete: true,
terminalBell: true,
suppressWhenFocused: true
}
})
} as never)
const handler = getOpenSystemSettingsHandler()
handler({})
expect(shellOpenExternalMock).toHaveBeenCalledWith(
'x-apple.systempreferences:com.apple.Notifications-Settings.extension?id=com.stablyai.orca.dev.fb5a47066f08'
)
} finally {
Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true })
if (originalBundleId === undefined) {
delete process.env.ORCA_DEV_MACOS_BUNDLE_ID
} else {
process.env.ORCA_DEV_MACOS_BUNDLE_ID = originalBundleId
}
}
})
it('suppresses notifications when disabled in settings', () => {
registerNotificationHandlers({
getSettings: () => ({
@@ -723,6 +783,48 @@ describe('registerNotificationHandlers', () => {
expect(notificationShowMock).toHaveBeenCalledTimes(2)
})
it('confirms explicit test notifications after the native show event', async () => {
registerNotificationHandlers({
getSettings: () => ({
notifications: {
enabled: true,
agentTaskComplete: true,
terminalBell: true,
suppressWhenFocused: false
}
})
} as never)
const handler = getDispatchHandler()
const result = handler({}, { source: 'test', requireDisplayConfirmation: true })
getNotificationOnceEventHandler('show')()
await expect(result).resolves.toEqual({ delivered: true })
expect(notificationShowMock).toHaveBeenCalledTimes(1)
})
it('reports not-displayed when explicit test notifications never show', async () => {
registerNotificationHandlers({
getSettings: () => ({
notifications: {
enabled: true,
agentTaskComplete: true,
terminalBell: true,
suppressWhenFocused: false
}
})
} as never)
const handler = getDispatchHandler()
const result = handler({}, { source: 'test', requireDisplayConfirmation: true })
await vi.advanceTimersByTimeAsync(2501)
await expect(result).resolves.toEqual({ delivered: false, reason: 'not-displayed' })
expect(notificationShowMock).toHaveBeenCalledTimes(1)
})
it('loads allowed custom sound files for preload playback', async () => {
const soundPath = join(tempDir, 'sound.ogg')
writeFileSync(soundPath, Buffer.from([1, 2, 3]))
+57 -8
View File
@@ -14,7 +14,11 @@ import { buildNotificationOptions } from './notification-options'
import { parsePaneKey } from '../../shared/stable-pane-id'
const NOTIFICATION_COOLDOWN_MS = 5000
const NOTIFICATION_DISPLAY_CONFIRMATION_TIMEOUT_MS = 2500
const MAX_NOTIFICATION_SOUND_BYTES = 10 * 1024 * 1024
const MACOS_PACKAGED_BUNDLE_ID = 'com.stablyai.orca'
const MACOS_NOTIFICATION_SETTINGS_URL =
'x-apple.systempreferences:com.apple.Notifications-Settings.extension'
const NOTIFICATION_SOUND_MIME_BY_EXTENSION: ReadonlyMap<string, string> = new Map([
['.ogg', 'audio/ogg'],
['.mp3', 'audio/mpeg'],
@@ -31,6 +35,40 @@ const NOTIFICATION_SOUND_MIME_BY_EXTENSION: ReadonlyMap<string, string> = new Ma
// strong reference until the notification is clicked or closed.
const activeNotifications = new Set<Notification>()
function getMacNotificationSettingsUrl(): string {
const bundleId = process.env.ORCA_DEV_MACOS_BUNDLE_ID ?? MACOS_PACKAGED_BUNDLE_ID
return `${MACOS_NOTIFICATION_SETTINGS_URL}?id=${encodeURIComponent(bundleId)}`
}
function openNotificationSystemSettings(): void {
if (process.platform === 'darwin') {
void shell.openExternal(getMacNotificationSettingsUrl())
} else if (process.platform === 'win32') {
void shell.openExternal('ms-settings:notifications')
}
}
function waitForNotificationDisplay(notification: Notification): Promise<boolean> {
return new Promise((resolve) => {
let settled = false
let timer: ReturnType<typeof setTimeout> | null = null
const settle = (displayed: boolean): void => {
if (settled) {
return
}
settled = true
if (timer) {
clearTimeout(timer)
}
resolve(displayed)
}
notification.once('show', () => settle(true))
notification.once('failed', () => settle(false))
timer = setTimeout(() => settle(false), NOTIFICATION_DISPLAY_CONFIRMATION_TIMEOUT_MS)
})
}
export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntimeService): void {
const recentNotifications = new Map<string, number>()
@@ -38,12 +76,7 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime
ipcMain.removeHandler('notifications:getPermissionStatus')
ipcMain.removeHandler('notifications:requestPermission')
ipcMain.handle('notifications:openSystemSettings', (): void => {
if (process.platform === 'darwin') {
// Deep-link into the macOS Notifications settings pane.
void shell.openExternal('x-apple.systempreferences:com.apple.Notifications-Settings')
} else if (process.platform === 'win32') {
void shell.openExternal('ms-settings:notifications')
}
openNotificationSystemSettings()
})
// Why: Electron's main-process `Notification` class exposes no synchronous
@@ -68,7 +101,10 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime
ipcMain.removeHandler('notifications:dispatch')
ipcMain.handle(
'notifications:dispatch',
(_event, args: NotificationDispatchRequest): NotificationDispatchResult => {
(
_event,
args: NotificationDispatchRequest
): NotificationDispatchResult | Promise<NotificationDispatchResult> => {
// Why: mobile push is independent of desktop notification guards.
// The user's phone should receive the notification even when the desktop
// window is focused (suppressWhenFocused), Electron notifications aren't
@@ -197,8 +233,21 @@ export function registerNotificationHandlers(store: Store, runtime?: OrcaRuntime
})
}
const displayConfirmation = args.requireDisplayConfirmation
? waitForNotificationDisplay(notification)
: null
notification.show()
if (displayConfirmation) {
return displayConfirmation.then((displayed) => {
if (!displayed) {
release()
return { delivered: false, reason: 'not-displayed' }
}
return { delivered: true }
})
}
return { delivered: true }
}
)
@@ -308,7 +357,7 @@ export function triggerStartupNotificationRegistration(store: Store): void {
// notifications…") but clicking it does nothing, which is confusing.
notification.on('click', () => {
cleanup()
void shell.openExternal('x-apple.systempreferences:com.apple.Notifications-Settings')
openNotificationSystemSettings()
})
notification.on('show', () => {
@@ -61,8 +61,51 @@ describe('NotificationsPane', () => {
// 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(notifications.dispatch).toHaveBeenCalledWith({
source: 'test',
requireDisplayConfirmation: true
})
expect(toastError).not.toHaveBeenCalled()
expect(toastSuccess).toHaveBeenCalledWith('Test notification sent')
})
it('opens macOS notification settings when the native test notification is not shown', async () => {
const notifications = {
getPermissionStatus: vi.fn(async () => ({
supported: true,
platform: 'darwin' 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(
'macOS did not show the notification',
expect.objectContaining({
action: expect.objectContaining({ label: 'Open Settings' })
})
)
const toastOptions = toastError.mock.calls[0]?.[1] as
| { action?: { onClick?: () => void } }
| undefined
toastOptions?.action?.onClick?.()
expect(notifications.openSystemSettings).toHaveBeenCalledTimes(1)
})
})
@@ -5,7 +5,7 @@ import { Button } from '../ui/button'
import { Label } from '../ui/label'
import { Separator } from '../ui/separator'
import { Slider } from '../ui/slider'
import { BellRing, Bot, FileAudio, Siren, Volume2, X } from 'lucide-react'
import { BellRing, Bot, ExternalLink, FileAudio, Siren, Volume2, X } from 'lucide-react'
import type { SettingsSearchEntry } from './settings-search'
import { basename } from '@/lib/path'
@@ -63,7 +63,10 @@ export async function sendNotificationSettingsTestNotification(
return
}
const result = await window.api.notifications.dispatch({ source: 'test' })
const result = await window.api.notifications.dispatch({
source: 'test',
requireDisplayConfirmation: true
})
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
@@ -82,6 +85,19 @@ export async function sendNotificationSettingsTestNotification(
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()
}
}
})
return
}
toast.error(
result.reason === 'disabled'
? 'Notifications are disabled'
@@ -125,6 +141,10 @@ export function NotificationsPane({
await sendNotificationSettingsTestNotification(notificationSettings, volumeDraft)
}
const handleOpenSystemSettings = async (): Promise<void> => {
await window.api.notifications.openSystemSettings()
}
const handleChooseSound = async (): Promise<void> => {
setIsPickingSound(true)
try {
@@ -267,7 +287,7 @@ export function NotificationsPane({
}
/>
<div className="px-1 pt-3">
<div className="flex flex-wrap items-center gap-2 px-1 pt-3">
<Button
variant="outline"
size="sm"
@@ -278,6 +298,15 @@ 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>
</div>
</div>
)
+9 -1
View File
@@ -1850,6 +1850,8 @@ 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. */
requireDisplayConfirmation?: boolean
worktreeId?: string
/** Stable `${tabId}:${leafId}` terminal pane key for click-to-focus routing. */
paneKey?: string
@@ -1870,7 +1872,13 @@ export type NotificationDispatchRequest = {
export type NotificationDispatchResult = {
delivered: boolean
/** Present when delivered is false. Tells the caller why delivery was skipped. */
reason?: 'disabled' | 'source-disabled' | 'suppressed-focus' | 'cooldown' | 'not-supported'
reason?:
| 'disabled'
| 'source-disabled'
| 'suppressed-focus'
| 'cooldown'
| 'not-supported'
| 'not-displayed'
}
export type NotificationSoundResult = {