feat(notifications): record a custom completion sound#2783
Conversation
Add a "Custom (record)" option to the notification sound picker so users can record their own sound (up to 5s) from the microphone in-app via MediaRecorder. The recording is stored as a data URL in the persisted settings store and played when the agent finishes a task or needs input. Adds the macOS microphone entitlement and NSMicrophoneUsageDescription so recording works in the packaged, sandboxed app. Generated-By: PostHog Code Task-Id: 1d32a904-f0b9-4439-9900-d24d12be04a8
|
React Doctor found 1 issue in 1 file · 1 warning. 1 warning
Reviewed by React Doctor for commit |
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/ui/src/features/notifications/notifications.test.ts:159-187
**New tests should be parameterized**
The two new `custom` sound tests check the same scenario (custom sound selected + `notifyPromptComplete` + `silent` flag outcome) differing only in whether `customCompletionSound` is set. Per the team's review standard, these should be expressed as a single `it.each` entry alongside the existing parameterised block. Keeping them as separate prose tests means a third case (e.g. `"custom"` with an empty string) would require another standalone block.
### Issue 2 of 3
packages/ui/src/features/settings/sections/GeneralSettings.tsx:417-419
**Volume slider appears active when no custom recording exists**
When `completionSound === "custom"` but no recording has been made yet, `completionSound !== "none"` is true so the volume slider is rendered — but no sound will actually play (the Test button is correctly hidden via `canTestSound`). This presents a functioning-looking control that has no audible effect. The slider condition should mirror `canTestSound` (or at least exclude the `custom`+no-recording state) to avoid the inconsistency.
### Issue 3 of 3
packages/ui/src/features/settings/sections/CustomSoundRecorder.tsx:56-60
**Silent failure when `FileReader` errors**
`onloadend` fires for both successful reads and read errors. When a read error occurs `reader.result` is `null`, so the branch `typeof reader.result === "string" ? reader.result : null` quietly calls `setCustomCompletionSound(null)` — effectively silently discarding what was just recorded with no feedback. Adding a `reader.onerror` handler (or checking `reader.error` inside `onloadend`) and showing a toast would give users visibility into a recording that failed to save.
Reviews (1): Last reviewed commit: "feat(notifications): let users record a ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("plays a recorded custom sound and silences the OS notification", () => { | ||
| const { service, notify, play } = makeService({ | ||
| hasFocus: false, | ||
| settings: { | ||
| completionSound: "custom", | ||
| customCompletionSound: "data:audio/webm;base64,AAAA", | ||
| }, | ||
| }); | ||
| service.notifyPromptComplete("My task", "end_turn", TASK_ID); | ||
| expect(play).toHaveBeenCalledWith( | ||
| "custom", | ||
| 80, | ||
| "data:audio/webm;base64,AAAA", | ||
| ); | ||
| expect(notify).toHaveBeenCalledWith( | ||
| expect.objectContaining({ silent: true }), | ||
| ); | ||
| }); | ||
|
|
||
| it("is not silent when custom is selected but nothing was recorded", () => { | ||
| const { service, notify } = makeService({ | ||
| hasFocus: false, | ||
| settings: { completionSound: "custom", customCompletionSound: null }, | ||
| }); | ||
| service.notifyPromptComplete("My task", "end_turn", TASK_ID); | ||
| expect(notify).toHaveBeenCalledWith( | ||
| expect.objectContaining({ silent: false }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
The two new custom sound tests check the same scenario (custom sound selected + notifyPromptComplete + silent flag outcome) differing only in whether customCompletionSound is set. Per the team's review standard, these should be expressed as a single it.each entry alongside the existing parameterised block. Keeping them as separate prose tests means a third case (e.g. "custom" with an empty string) would require another standalone block.
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/notifications/notifications.test.ts
Line: 159-187
Comment:
**New tests should be parameterized**
The two new `custom` sound tests check the same scenario (custom sound selected + `notifyPromptComplete` + `silent` flag outcome) differing only in whether `customCompletionSound` is set. Per the team's review standard, these should be expressed as a single `it.each` entry alongside the existing parameterised block. Keeping them as separate prose tests means a third case (e.g. `"custom"` with an empty string) would require another standalone block.
**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| {completionSound !== "none" && ( | ||
| <SettingRow label="Sound volume" noBorder> | ||
| <Flex align="center" gap="3"> |
There was a problem hiding this comment.
When completionSound === "custom" but no recording has been made yet, completionSound !== "none" is true so the volume slider is rendered — but no sound will actually play (the Test button is correctly hidden via canTestSound). This presents a functioning-looking control that has no audible effect. The slider condition should mirror canTestSound (or at least exclude the custom+no-recording state) to avoid the inconsistency.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/settings/sections/GeneralSettings.tsx
Line: 417-419
Comment:
**Volume slider appears active when no custom recording exists**
When `completionSound === "custom"` but no recording has been made yet, `completionSound !== "none"` is true so the volume slider is rendered — but no sound will actually play (the Test button is correctly hidden via `canTestSound`). This presents a functioning-looking control that has no audible effect. The slider condition should mirror `canTestSound` (or at least exclude the `custom`+no-recording state) to avoid the inconsistency.
How can I resolve this? If you propose a fix, please make it concise.| reader.onloadend = () => { | ||
| setCustomCompletionSound( | ||
| typeof reader.result === "string" ? reader.result : null, | ||
| ); | ||
| }; |
There was a problem hiding this comment.
FileReader errors
onloadend fires for both successful reads and read errors. When a read error occurs reader.result is null, so the branch typeof reader.result === "string" ? reader.result : null quietly calls setCustomCompletionSound(null) — effectively silently discarding what was just recorded with no feedback. Adding a reader.onerror handler (or checking reader.error inside onloadend) and showing a toast would give users visibility into a recording that failed to save.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/settings/sections/CustomSoundRecorder.tsx
Line: 56-60
Comment:
**Silent failure when `FileReader` errors**
`onloadend` fires for both successful reads and read errors. When a read error occurs `reader.result` is `null`, so the branch `typeof reader.result === "string" ? reader.result : null` quietly calls `setCustomCompletionSound(null)` — effectively silently discarding what was just recorded with no feedback. Adding a `reader.onerror` handler (or checking `reader.error` inside `onloadend`) and showing a toast would give users visibility into a recording that failed to save.
How can I resolve this? If you propose a fix, please make it concise.- Surface a toast when reading the recording fails instead of silently clearing it (split FileReader onloadend into onload/onerror). - Hide the volume slider when "custom" is selected but nothing is recorded yet, mirroring the Test button's canTestSound gating. - Parameterize the two custom-sound notification tests into a single it.each block. Generated-By: PostHog Code Task-Id: 1d32a904-f0b9-4439-9900-d24d12be04a8
1 participant
Problem
The notification sound that plays when the agent finishes a task or needs your input is limited to a fixed list of built-in sounds. Users wanted to add their own — ideally recording one on the fly right in the app.
Changes
Adds a Custom (record) option to the Sound effect picker in General settings. Selecting it reveals a small recorder that captures up to 5 seconds from the microphone via
MediaRecorder, stores it as a data URL in the persisted settings store, and plays it like any other completion sound (Test button, volume slider, and the OS-notification silencing all work with it).playCompletionSoundtakes an optional custom data URL and plays it when the sound is"custom".CustomSoundRecordercomponent: Record / Re-record / Stop / Clear, with mic-permission error handling.customCompletionSoundthrough the notification settings provider so the agent-completion/permission notifications use it.audio-inputentitlement andNSMicrophoneUsageDescriptionso recording works in the packaged, sandboxed app.How did you test this?
pnpm --filter @posthog/ui exec vitest run src/features/notifications/notifications.test.ts— 14 pass, including new cases for the custom sound (plays the recorded URL + silences the OS notification; not silenced when nothing is recorded yet).Automatic notifications
Created with PostHog Code from a Slack thread