diff --git a/src/preload/api/notification-sound-playback.test.ts b/src/preload/api/notification-sound-playback.test.ts new file mode 100644 index 00000000000..0507e2c639b --- /dev/null +++ b/src/preload/api/notification-sound-playback.test.ts @@ -0,0 +1,115 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type { notificationsApi } from './notifications-bridge' + +const SOUND_PATH = join(tmpdir(), 'notification.mp3') + +const { construct, invoke, play } = vi.hoisted(() => ({ + construct: vi.fn((audio: { currentTime: number; volume: number; pause: () => void }) => audio), + invoke: vi.fn(), + play: vi.fn(() => Promise.resolve()) +})) + +vi.mock('electron', () => ({ ipcRenderer: { invoke } })) + +async function loadNotificationsApi(): Promise { + vi.resetModules() + return (await import('./notifications-bridge')).notificationsApi +} + +describe('notificationsApi.playSound', () => { + beforeEach(() => { + construct.mockClear() + play.mockClear() + vi.stubGlobal( + 'Audio', + class extends EventTarget { + currentTime = 0 + volume = 1 + src = '' + pause = vi.fn() + play = play + + constructor() { + super() + construct(this) + } + } + ) + invoke.mockReset() + invoke.mockImplementation((channel: string) => { + if (channel === 'notifications:resolveSoundPath') { + return Promise.resolve({ ok: true, path: SOUND_PATH }) + } + if (channel === 'notifications:loadSound') { + return Promise.resolve({ + ok: true, + data: new Uint8Array([1]), + mimeType: 'audio/mpeg', + path: SOUND_PATH + }) + } + return Promise.resolve(undefined) + }) + }) + + afterEach(() => vi.unstubAllGlobals()) + + it('replays the cached sound for each notification instead of deduping mid-playback', async () => { + const notificationsApi = await loadNotificationsApi() + + await expect(notificationsApi.playSound()).resolves.toEqual({ played: true }) + const audio = construct.mock.calls[0]?.[0] + if (!audio) { + throw new Error('Audio was not constructed') + } + audio.currentTime = 0.75 + await expect(notificationsApi.playSound()).resolves.toEqual({ played: true }) + + expect(audio.currentTime).toBe(0) + expect(construct).toHaveBeenCalledOnce() + expect(play).toHaveBeenCalledTimes(2) + }) + + it('retries after a rejected play without leaving future notifications silent', async () => { + const notificationsApi = await loadNotificationsApi() + play.mockRejectedValueOnce(new Error('audio device unavailable')) + await expect(notificationsApi.playSound()).resolves.toEqual({ + played: false, + reason: 'playback-failed' + }) + await expect(notificationsApi.playSound()).resolves.toEqual({ played: true }) + expect(construct).toHaveBeenCalledOnce() + }) + + it('clamps volume and stays silent when no sound is configured', async () => { + const notificationsApi = await loadNotificationsApi() + await notificationsApi.playSound({ volume: 150 }) + const audio = construct.mock.calls[0]?.[0] + if (!audio) { + throw new Error('Audio was not constructed') + } + expect(audio.volume).toBe(1) + await notificationsApi.playSound({ volume: -10 }) + expect(audio.volume).toBe(0) + invoke.mockResolvedValueOnce({ ok: false, reason: 'missing-path' }) + await expect(notificationsApi.playSound()).resolves.toEqual({ + played: false, + reason: 'missing-path' + }) + expect(audio.pause).toHaveBeenCalledOnce() + expect(play).toHaveBeenCalledTimes(2) + }) + + it('shares one cached Audio across concurrent first playback', async () => { + const notificationsApi = await loadNotificationsApi() + + await expect( + Promise.all([notificationsApi.playSound(), notificationsApi.playSound()]) + ).resolves.toEqual([{ played: true }, { played: true }]) + + expect(construct).toHaveBeenCalledOnce() + expect(play).toHaveBeenCalledTimes(2) + }) +}) diff --git a/src/preload/api/notifications-bridge.ts b/src/preload/api/notifications-bridge.ts index 47f84802d3b..35f89ab9bfa 100644 --- a/src/preload/api/notifications-bridge.ts +++ b/src/preload/api/notifications-bridge.ts @@ -16,19 +16,8 @@ let cachedNotificationSound: { blobUrl: string audio: HTMLAudioElement } | null = null -let isNotificationSoundPlaying = false -// Why: audio.play() can reject before ended/error fires; cleanup prevents leaked listeners. -let cleanupNotificationSoundPlayback: (() => void) | null = null - -function clearNotificationSoundPlaybackState(): void { - cleanupNotificationSoundPlayback?.() - cleanupNotificationSoundPlayback = null - isNotificationSoundPlaying = false -} - function disposeCachedNotificationSound(): void { if (cachedNotificationSound) { - clearNotificationSoundPlaybackState() cachedNotificationSound.audio.pause() cachedNotificationSound.audio.src = '' URL.revokeObjectURL(cachedNotificationSound.blobUrl) @@ -53,11 +42,6 @@ export const notificationsApi = { volume?: number }): Promise => { try { - // Why: drop replays while still ringing; the test button passes force to always confirm. - if (!options?.force && isNotificationSoundPlaying) { - return { played: false, reason: 'deduped' } - } - const resolved = (await ipcRenderer.invoke( 'notifications:resolveSoundPath' )) as NotificationSoundPathResult @@ -77,13 +61,19 @@ export const notificationsApi = { disposeCachedNotificationSound() return { played: false, reason: sound.reason } } - const arrayBuffer = new ArrayBuffer(sound.data.byteLength) - new Uint8Array(arrayBuffer).set(sound.data) - const blob = new Blob([arrayBuffer], { type: sound.mimeType }) - disposeCachedNotificationSound() - const blobUrl = URL.createObjectURL(blob) - entry = { path: sound.path, blobUrl, audio: new Audio(blobUrl) } - cachedNotificationSound = entry + // Why: a concurrent playSound may have cached the same path while this load was in flight. + const latestEntry = cachedNotificationSound + if (latestEntry?.path === sound.path) { + entry = latestEntry + } else { + const arrayBuffer = new ArrayBuffer(sound.data.byteLength) + new Uint8Array(arrayBuffer).set(sound.data) + const blob = new Blob([arrayBuffer], { type: sound.mimeType }) + disposeCachedNotificationSound() + const blobUrl = URL.createObjectURL(blob) + entry = { path: sound.path, blobUrl, audio: new Audio(blobUrl) } + cachedNotificationSound = entry + } } const audio = entry.audio @@ -92,31 +82,13 @@ export const notificationsApi = { if (typeof options?.volume === 'number' && Number.isFinite(options.volume)) { audio.volume = Math.min(1, Math.max(0, options.volume / 100)) } - isNotificationSoundPlaying = true - cleanupNotificationSoundPlayback?.() - const release = (): void => { - cleanup() - if (cleanupNotificationSoundPlayback === cleanup) { - cleanupNotificationSoundPlayback = null - } - isNotificationSoundPlaying = false - } - const cleanup = (): void => { - audio.removeEventListener('ended', release) - audio.removeEventListener('error', release) - } - cleanupNotificationSoundPlayback = cleanup - audio.addEventListener('ended', release) - audio.addEventListener('error', release) try { await audio.play() } catch { - release() return { played: false, reason: 'playback-failed' } } return { played: true } } catch { - clearNotificationSoundPlaybackState() return { played: false, reason: 'playback-failed' } } } diff --git a/tests/e2e/notification-sound-replay.spec.ts b/tests/e2e/notification-sound-replay.spec.ts new file mode 100644 index 00000000000..c0f58ba6e7b --- /dev/null +++ b/tests/e2e/notification-sound-replay.spec.ts @@ -0,0 +1,153 @@ +import { writeFileSync } from 'node:fs' +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' + +// A long tone makes the second request unambiguously arrive during playback. +function notificationTone(): Buffer { + const rate = 48_000 + const samples = rate * 2 + const wav = Buffer.alloc(44 + samples * 2) + wav.write('RIFF', 0) + wav.writeUInt32LE(wav.length - 8, 4) + wav.write('WAVEfmt ', 8) + wav.writeUInt32LE(16, 16) + wav.writeUInt16LE(1, 20) + wav.writeUInt16LE(1, 22) + wav.writeUInt32LE(rate, 24) + wav.writeUInt32LE(rate * 2, 28) + wav.writeUInt16LE(2, 32) + wav.writeUInt16LE(16, 34) + wav.write('data', 36) + wav.writeUInt32LE(samples * 2, 40) + for (let i = 0; i < samples; i++) { + const seconds = i / rate + const envelope = Math.min(1, seconds * 100, (2 - seconds) * 100) + const frequency = seconds < 0.12 ? 880 : 440 + wav.writeInt16LE( + Math.round(Math.sin(2 * Math.PI * frequency * seconds) * envelope * 6000), + 44 + i * 2 + ) + } + return wav +} + +test('completion sound restarts during playback without overlapping players', async ({ + orcaPage +}, testInfo) => { + await waitForSessionReady(orcaPage) + const soundPath = testInfo.outputPath('notification-tone.wav') + writeFileSync(soundPath, notificationTone()) + await orcaPage.evaluate(async (soundPath) => { + const settings = await window.api.settings.get() + const next = await window.api.settings.set({ + notifications: { + ...settings.notifications, + customSoundId: 'custom', + customSoundPath: soundPath, + customSoundVolume: 100 + } + }) + window.__store?.setState({ settings: next }) + window.__store?.getState().openSettingsTarget({ pane: 'notifications', repoId: null }) + window.__store?.getState().openSettingsPage() + }, soundPath) + const cdp = await orcaPage.context().newCDPSession(orcaPage) + let isolatedContextId: number | undefined + cdp.on('Runtime.executionContextCreated', ({ context }) => { + if (context.name === 'Electron Isolated Context') { + isolatedContextId = context.id + } + }) + await cdp.send('Runtime.enable') + expect(isolatedContextId).toBeDefined() + // Intercept construction only; decoding, seeking and play remain Chromium's real Audio. + await cdp.send('Runtime.evaluate', { + contextId: isolatedContextId, + expression: `(() => { + const OriginalAudio = Audio; + const audios = []; + const context = new AudioContext(); + const destination = context.createMediaStreamDestination(); + const chunks = []; + const recorder = new MediaRecorder(destination.stream); + recorder.ondataavailable = event => chunks.push(event.data); + recorder.start(); + globalThis.Audio = function (...args) { + const audio = new OriginalAudio(...args); + context.createMediaElementSource(audio).connect(destination); + audios.push(audio); + return audio; + }; + globalThis.notificationAudioProbe = { audios, recorder, chunks, context }; + return context.resume(); + })()`, + awaitPromise: true + }) + const started = Date.now() + const first = await orcaPage.evaluate(() => window.api.notifications.playSound()) + expect(first).toEqual({ played: true }) + await orcaPage.waitForTimeout(600) + const beforeSeek = await cdp.send('Runtime.evaluate', { + contextId: isolatedContextId, + expression: 'notificationAudioProbe.audios[0].currentTime', + returnByValue: true + }) + expect(beforeSeek.result.value).toBeGreaterThan(0.3) + const second = await orcaPage.evaluate(() => window.api.notifications.playSound()) + const baseline = process.env.ORCA_NOTIFICATION_SOUND_BASELINE === '1' + expect(second).toEqual(baseline ? { played: false, reason: 'deduped' } : { played: true }) + const afterSeek = await cdp.send('Runtime.evaluate', { + contextId: isolatedContextId, + expression: 'notificationAudioProbe.audios[0].currentTime', + returnByValue: true + }) + if (!baseline) { + expect(afterSeek.result.value).toBeLessThan(0.2) + } + await orcaPage.waitForTimeout(2400) + expect(await orcaPage.evaluate(() => window.api.notifications.playSound())).toEqual({ + played: true + }) + await orcaPage.waitForTimeout(2200) + const count = await cdp.send('Runtime.evaluate', { + contextId: isolatedContextId, + expression: 'notificationAudioProbe.audios.length', + returnByValue: true + }) + expect(count.result.value).toBe(1) + const recording = await cdp.send('Runtime.evaluate', { + contextId: isolatedContextId, + awaitPromise: true, + returnByValue: true, + expression: `new Promise(resolve => { + const probe = notificationAudioProbe; + probe.recorder.onstop = async () => { + const bytes = new Uint8Array(await new Blob(probe.chunks).arrayBuffer()); + let binary = ''; for (const byte of bytes) binary += String.fromCharCode(byte); + resolve(btoa(binary)); + }; + probe.recorder.stop(); + })` + }) + if (typeof recording.result.value !== 'string') { + throw new Error('No audio recording') + } + writeFileSync( + testInfo.outputPath('decoded-audio.webm'), + Buffer.from(recording.result.value, 'base64') + ) + writeFileSync( + testInfo.outputPath('playback-results.json'), + JSON.stringify( + { baseline, first, second, elapsedMs: Date.now() - started, players: count.result.value }, + null, + 2 + ) + ) + await orcaPage.screenshot({ path: testInfo.outputPath('notification-settings.png') }) + await testInfo.attach('decoded notification audio', { + path: testInfo.outputPath('decoded-audio.webm'), + contentType: 'audio/webm' + }) + await cdp.detach() +})