diff --git a/config/scripts/run-electron-vite-dev.mjs b/config/scripts/run-electron-vite-dev.mjs index a8260987bbd..103f55b822a 100644 --- a/config/scripts/run-electron-vite-dev.mjs +++ b/config/scripts/run-electron-vite-dev.mjs @@ -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, diff --git a/src/main/ipc/notifications.test.ts b/src/main/ipc/notifications.test.ts index 8d9ae042ade..3f76973b41c 100644 --- a/src/main/ipc/notifications.test.ts +++ b/src/main/ipc/notifications.test.ts @@ -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 { 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])) diff --git a/src/main/ipc/notifications.ts b/src/main/ipc/notifications.ts index a4c7f6d78cf..5ab0e19a861 100644 --- a/src/main/ipc/notifications.ts +++ b/src/main/ipc/notifications.ts @@ -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 = new Map([ ['.ogg', 'audio/ogg'], ['.mp3', 'audio/mpeg'], @@ -31,6 +35,40 @@ const NOTIFICATION_SOUND_MIME_BY_EXTENSION: ReadonlyMap = new Ma // strong reference until the notification is clicked or closed. const activeNotifications = new Set() +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 { + return new Promise((resolve) => { + let settled = false + let timer: ReturnType | 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() @@ -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 => { // 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', () => { diff --git a/src/renderer/src/components/settings/NotificationsPane.test.tsx b/src/renderer/src/components/settings/NotificationsPane.test.tsx index 13601ad8a42..ce8431c3e1d 100644 --- a/src/renderer/src/components/settings/NotificationsPane.test.tsx +++ b/src/renderer/src/components/settings/NotificationsPane.test.tsx @@ -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) + }) }) diff --git a/src/renderer/src/components/settings/NotificationsPane.tsx b/src/renderer/src/components/settings/NotificationsPane.tsx index 7a694bbf0b7..430793abec3 100644 --- a/src/renderer/src/components/settings/NotificationsPane.tsx +++ b/src/renderer/src/components/settings/NotificationsPane.tsx @@ -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 => { + await window.api.notifications.openSystemSettings() + } + const handleChooseSound = async (): Promise => { setIsPickingSound(true) try { @@ -267,7 +287,7 @@ export function NotificationsPane({ } /> -
+
+
) diff --git a/src/shared/types.ts b/src/shared/types.ts index b3bf8443e7d..53a840c3627 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -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 = {