feat: implement native OS notifications and sound alerts - #8278
feat: implement native OS notifications and sound alerts#8278IamCoder18 wants to merge 22 commits into
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Fix these issues in Kilo Cloud Files Reviewed (2 files — incremental since e53c8d2)
Reviewed by claude-sonnet-4.6 · 2,941,995 tokens |
78b1762 to
bcf2535
Compare
| // osascript -e receives the string directly; escape backslashes, quotes, and control characters for AppleScript | ||
| const esc = (s: string) => | ||
| s.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\n/g, "\\n").replace(/\r/g, "\\r").replace(/\t/g, "\\t") | ||
| await exec("osascript", ["-e", `display notification "${esc(body)}" with title "${esc(title)}"`]).catch(() => { |
There was a problem hiding this comment.
There must be a better way than osascript on macOS
There was a problem hiding this comment.
Can you check how we did it in kilocode-legacy?
There was a problem hiding this comment.
I looked into alternatives. The UserNotifications framework would require a native Node.js addon, which adds complexity and maintenance burden. Terminal-notifier is another option but requires an external dependency.
I have not found a cleaner built-in approach yet. I will keep looking, but osascript seems to be the most straightforward option for now without adding dependencies.
There was a problem hiding this comment.
I checked the kilocode-legacy repository but I couldn't find OS notification implementation there. I'll check further later today.
There was a problem hiding this comment.
I found the related code. It appears to be terminal-notifier first, then osascript. I will update my code to prioritize terminal-notifier. Thank you for the feedback!
There was a problem hiding this comment.
I don't think we should do this. This will get messy.
| await exec("powershell", [ | ||
| "-NonInteractive", | ||
| "-Command", | ||
| `$t = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String("${encodedTitle}")); ` + | ||
| `$b = [System.Text.Encoding]::UTF8.GetString([System.Convert]::FromBase64String("${encodedBody}")); ` + | ||
| `[Windows.UI.Notifications.ToastNotificationManager, Windows.UI.Notifications, ContentType = WindowsRuntime] > $null; ` + | ||
| `$template = [Windows.UI.Notifications.ToastNotificationManager]::GetTemplateContent([Windows.UI.Notifications.ToastTemplateType]::ToastText02); ` + | ||
| `$xml = [Windows.Data.Xml.Dom.XmlDocument]::new(); $xml.LoadXml($template.GetXml()); ` + | ||
| `$textNodes = $xml.GetElementsByTagName("text"); ` + | ||
| `$textNodes.Item(0).AppendChild($xml.CreateTextNode($t)) > $null; ` + | ||
| `$textNodes.Item(1).AppendChild($xml.CreateTextNode($b)) > $null; ` + | ||
| `$toast = [Windows.UI.Notifications.ToastNotification]::new($xml); ` + | ||
| `[Windows.UI.Notifications.ToastNotificationManager]::CreateToastNotifier("Kilo Code").Show($toast)`, | ||
| ]).catch(() => { |
There was a problem hiding this comment.
This looks really hacky, and no catch handling
There was a problem hiding this comment.
I agree — the Windows code was complex and the catch blocks were empty. I have added logging to all catch handlers so failures are now visible:
- notify.ts: Added console.log("[Kilo New] notification failed:", err) to all 3 platform branches
- sound.ts: Added similar logging to all fallback chains
| @@ -0,0 +1,48 @@ | |||
| import * as os from "os" | |||
There was a problem hiding this comment.
Do we need both, aac and wav? Why 2 different formats?
There was a problem hiding this comment.
We originally got the AAC audio files from the Tauri desktop app. However, the command-line tools we use to play audio (afplay on macOS, aplay/paplay on Linux, and PowerShell System.Media.SoundPlayer on Windows) do not support AAC format natively.
To ensure cross-platform compatibility, we converted all sounds to compressed WAV files. The AAC files are not bundled in the extension — only the WAV files are included. The AAC files in the repo are kept for reference but are not part of the final package.
There was a problem hiding this comment.
@IamCoder18 why for reference? Can we not just remove them if they are not used?
| `$path = '${safePath}'; $s = New-Object System.Media.SoundPlayer($path); $s.PlaySync(); $s.Dispose()`, | ||
| ]) | ||
| } catch { | ||
| // No audio |
There was a problem hiding this comment.
we should log in all catch cases
There was a problem hiding this comment.
Fixed! Added logging to all catch blocks in both notify.ts and sound.ts. Failures are now logged with [Kilo New] prefix to make them easy to find in the Output panel.
| // osascript -e receives the string directly; escape backslashes, quotes, and control characters for AppleScript | ||
| const esc = (s: string) => | ||
| s.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\n/g, "\\n").replace(/\r/g, "\\r").replace(/\t/g, "\\t") | ||
| await exec("osascript", ["-e", `display notification "${esc(body)}" with title "${esc(title)}"`]).catch(() => { |
There was a problem hiding this comment.
Can you check how we did it in kilocode-legacy?
ae520d4 to
6108896
Compare
|
@marius-kilocode I resolve all the issues you brought up, including primarily using |
|
@IamCoder18 thanks I have another look |
|
Ok I tested it:
Other than that it's pretty cool! Thanks for the PR! |
271a9e3 to
4d2056e
Compare
|
@marius-kilocode Thanks for testing! I've added a "Test notification" button next to each sound setting so you can preview the sound. Regarding the ESC sound, I'm not sure what you mean by this, could you clarify? |
85cb3b2 to
1743654
Compare
|
Hello, dear developers. |
|
Is there any reason as to why this PR hasn't been merged into the main branch? Beside resolving the merge conflicts, what else is needed for this PR to proceed? I think this is very important work by @IamCoder18 that needs to reach the main branch ASAP. |
- Add window focus tracking to detect when VS Code is backgrounded - Implement `notifyIfNotFocused` to trigger OS-level alerts - Send notifications for agent task completion (busy -> idle transition) - Send notifications for permission requests, questions, and session errors - Add configuration checks and cleanup for focus listeners
- Add sound.ts utility for playing audio files via platform-specific commands - Integrate sound playback into notifyIfNotFocused() for agent, permissions, and error notifications - Convert AAC audio files to WAV for broader compatibility
- Fix sound file path to correctly resolve from dist/ to kilo-vscode/audio-wav/ - Add error handling for notify.ts on all platforms - Add path validation to prevent PowerShell injection - Add all 45+ sound options to NotificationsTab dropdown - Re-add test notification command
- Initialize lastFocusState as undefined instead of accessing vscode.window.state.focused at class init - Add runtime check for onDidChangeWindowState existence to support test environment - Only skip notifications when explicitly focused (not undefined) to allow notifications on initial load
The path whitelist regex rejected legal extension install paths containing
apostrophes (e.g. O'Connor) or non-ASCII characters (e.g. Müller, José, 用户),
causing notification sounds to silently fail for those users. Use Unicode
property escapes (\p{L}\p{N}\p{M}) to accept all scripts while still blocking
shell metacharacters.
The regex was too restrictive and rejected legal paths containing parentheses, brackets, braces, tildes, at signs, commas, plus signs, and equals signs. Add these characters to the whitelist while still blocking all shell metacharacters (; | & $ ` > < # ! * ? " %).
The whitelist approach rejected valid path characters (!, #, $, %, &, etc.) that are safe in both execution contexts: execFile on Unix (no shell) and single-quoted PowerShell strings on Windows (only ' is special). Replace with a blocklist that only rejects control characters (0x00-0x1F, 0x7F) which should never appear in file paths and could break command-line parsing.
KiloProvider is instantiated for the sidebar and for tab panels, each subscribing to the same SSE event stream from KiloConnectionService. When the same session is open in multiple panels, every provider triggered notifyIfNotFocused() for the same event, causing duplicate sounds and OS toasts. Add a process-wide Set in KiloConnectionService (the singleton) to dedupe by 'sessionID:eventType' key. The first provider to call shouldNotify() gets true and marks the key; subsequent callers get false. Keys expire after 5s to allow re-notification for subsequent events.
…uppressing distinct events
- Add 'system' sound option that uses OS-native sounds on macOS/Linux/Windows - Change default sounds from 'default' to 'system' - Add test notification buttons in settings UI - Add cooldowns to prevent notification flooding
…ows silent audio - Key cooldown map by sessionID so different sessions have independent cooldowns - Append silent audio element to Windows toast XML to prevent default sound - Include timestamp in shouldNotify key for events without requestID
Add translations for new keys settings.notifications.sound.system and settings.notifications.testSound which were added to English but missing in all other locales.
Add OS-level system notifications with sound playback for agent events when VS Code is not focused. Includes 45+ configurable notification sounds and per-event enable/disable settings.
e53c8d2 to
45cd7c7
Compare
|
Hey @marius-kilocode and @lambertjosh (original issue author), I've resolved the failing CI and the merge conflicts and manually tested again to make sure everything work. I hope you can take another look at this! |
|
Hey! Sorry for the ping again. There is a lot of community interest in this feature (at least 17 different people across hearts, replies to the linked issues, and comments in this thread), and I was hoping you could take another look at this. |
|
This PR now closes 4 issues and has been requested by at least 19 different people. Please take a look at this when you have the time. |
|
Thanks for the contribution! I'm supportive of this change to improve our notifications and follow OS standards. |
|
Make Notifications Great Again! |
marius-kilocode
left a comment
marius-kilocode
left a comment
There was a problem hiding this comment.
I think this PR is trying to solve too much at once. Can we split it?
For a first PR, I would prefer focusing only on sound notifications:
- Add the sound playback utility.
- Wire the existing sound settings to actual playback.
- Add the test/preview sound button.
- Play sounds for the narrow set of existing state transitions we agree on, probably permission/question needed, error, and successful completion.
- Remove the OS notification implementation for now (
notify.ts,osascript, PowerShell toast,notify-send). - Remove unused
.aacfiles unless there is a documented conversion/source-asset workflow. - Keep the ESLint caps unchanged by extracting any new logic out of
KiloProvider.
Then we can do a second PR for the broader notification system:
- Add a central extension-side notification manager/service.
- Have sidebar tabs, Agent Manager, and other surfaces publish or expose agent/session state to that manager.
- Deduplicate globally across webviews.
- Decide delivery based on visibility/focus.
- Add OS-level notification delivery only behind that centralized system, ideally through a maintained/bundled notifier helper rather than ad hoc platform shell commands.
The current PR mixes sound playback, OS notifications, settings UI, event-state tracking, dedupe, packaging assets, and provider-level notification decisions. Splitting it would make the sound fix easier to review and ship while keeping the larger notification architecture deliberate.
| @@ -0,0 +1,48 @@ | |||
| import * as os from "os" | |||
There was a problem hiding this comment.
@IamCoder18 why for reference? Can we not just remove them if they are not used?
| } else if (status === "idle" && this.busySessions.has(sid) && this.trackedSessionIds.has(sid)) { | ||
| this.busySessions.delete(sid) | ||
| if (this.connectionService.shouldNotify(sid, "session.status:idle")) | ||
| this.notifyIfNotFocused("agent", "Agent task completed", undefined, sid) |
There was a problem hiding this comment.
So what happens now on failure or cancel? Isn't the user in those cases also getting notified?
| // osascript -e receives the string directly; escape backslashes, quotes, and control characters for AppleScript | ||
| const esc = (s: string) => | ||
| s.replace(/\\/g, "\\\\").replace(/"/g, '\\"').replace(/\n/g, "\\n").replace(/\r/g, "\\r").replace(/\t/g, "\\t") | ||
| await exec("osascript", ["-e", `display notification "${esc(body)}" with title "${esc(title)}"`]).catch(() => { |
There was a problem hiding this comment.
I don't think we should do this. This will get messy.
|
I've been working on the first split-up PR. It should be ready tomorrow. |
|
Hi @marius-kilocode, I split up the PR and focused only on sound notifications in the pull request here: #10545. It includes
All 12 CI checks have passed successfully and the code review found no issues. I have also attached a few short demo videos inside #10545 showing the UI test button, audio on core agent state transitions (Questions, Permissions, and Completion), the error sound triggers, and our 700ms cooldown handling multiple sounds in quick succession on both Linux and Windows. When you have a moment, I would appreciate it if you could take a look at the new PR. Thank you for the guidance on structuring this feature properly! |
|
Hi @marius-kilocode, |
Hey @Firues! Thank you so much for your support. My PR actually didn't get merged because it was implemented upstream (in OpenCode), and the VS Code sound notifications are now being tracked in a PR by Marius: #11098 Once merged, I will work on the OS notifications side of it. |
|
Hey @IamCoder18, thank you for all the work here and for splitting this out into #10545. Since this was implemented upstream in OpenCode and is now being tracked through #11098, I think we can close this PR and the related one to keep the queue manageable. I’ll close them unless you have any concerns. |
|
Agreed. |
6 participants
Context
This PR implements OS-level system notifications with optional sound playback for the Kilo Code VS Code extension. When an agent task completes (or needs attention via permissions/questions/errors), a native OS notification is shown (and optionally a sound played) if the user is not focused on VS Code.
Why this matters: Users often run long agent tasks and switch to other applications. They need a way to know when the agent finishes without actively watching VS Code.
Closes #7048
Closes #8127
Closes #7877
Closes #10321
Implementation
1. New Notification Utility (
src/util/notify.ts)Created a platform-specific notification sender that sends OS-level notifications:
osascriptto show Notification Center notificationsnotify-send(silently fails if unavailable)2. New Sound Utility (
src/util/sound.ts)Created a sound playback utility for playing notification sounds:
afplayorplay(sox)aplay,paplay, orplay(sox)System.Media.SoundPlayerAll 45+ sound options are available in settings with sensible defaults per notification type (alert for agent, bip-bop for permissions, nope for errors).
3. Focus Tracking in
KiloProvider.tsbusySessionsSet to track which sessions are currently runninglastFocusStateto track VS Code window focus viaonDidChangeWindowStatelastFocusStateisfalse(user in another app)4. Event Handlers
Modified
handleEvent()to trigger notifications and sounds on:session.statusbusy→idlenotifications.agentpermission.askednotifications.permissionsquestion.askednotifications.permissionssession.errornotifications.errors5. Settings Integration
Uses existing settings from
package.jsonthat were previously unused:kilo-code.new.notifications.agent(default: true)kilo-code.new.notifications.permissions(default: true)kilo-code.new.notifications.errors(default: true)And adds new sound settings:
kilo-code.new.sounds.agent(default: "default" → alert-01)kilo-code.new.sounds.permissions(default: "default" → bip-bop-01)kilo-code.new.sounds.errors(default: "default" → nope-01)These have UI in the Notifications settings tab with all 45+ sound options.
Screenshots
I test notifications, audio, and that settings are applied correctly.
2026-04-03.08-08-54.mp4
How to Test
Get in Touch
My discord is
@iamcoder18and I am in the Kilo Code discord server.