charmbracelet
feat(notification): alert on turn completion and permission request
Implement delayed and cancellable cross-platform desktop notifications using gen2brain/beeep. There's not yet an application icon as was requested in https://github.com/charmbracelet/crush/pull/230#issuecomment-3084211904 but adding one once it exists, either before or after merge, should be simple based on beeep's docs.
I've tested and had it working on from-vanilla Arch and Debian 13, but I've not tested on any other system. I know all of the CoderAgent tests are failing, at least locally. How do I update those/resolve the issue?
https://github.com/user-attachments/assets/22e2392b-d7f1-4e43-aac0-9f4e0d30ddd8
I found a couple potential issues in internal/permission/permission.go and internal/permission/permission_test.go and fixed them in individual commits (b35c245804f02ffb71915c7cf03a0af75c466ce2, f2288be8d9ebb308cfcba7d7658b9d975c52f70f, and f4ac82e7b679089be16c6cfab6f4e0333065fd9d) so they can be discarded if non-issues or unwanted or separated into individual PRs or something. The actual feature is 4ebd6fb19cb74b258ef61335c472a3da76ce04c4 and docs are d35aa46671e29d4556465ca4ecd94f529e5588a0.
- [x] I have read
CONTRIBUTING.md. - [x] I have ~~created a discussion that was approved by a maintainer (for new features).~~ talked with a maintainer in Discord and been assigned #1224
Hmm I've been using this since opening the PR and it seems like the sub-agent's session also sends a notification when it's finished, instead of just the main agent. I'll look into differentiating between them so the user isn't mistakenly notified when the sub-agent's done but the main agent isn't.
I'm aware of the new conflicts, will get them resolved sometime in the next few days
@Amolith do you need an icon for this one? I'm looking forward to to getting this one in :)
Instead of scheduling notifications, giving the user time to interact, then cancelling the notif if they do, whether to send them is based on whether the window is focused and whether the terminal supports reporting focus:
- Not supported, no notif
- Supported and focused, no notif
- Supported but not focused, notif
The icon is omitted on macOS because of https://github.com/gen2brain/beeep/issues/67, but I'm going to look into doing them through the terminal, instead of the platform, using escape sequences as an enhancement on top of this, with this as the fallback if nothing better is supported. Should I plan to work that into this PR, or contribute it separately?
I don't know what's causing the govulncheck failure, but the Window test failure seems unrelated because I don't think any of my changes should affect the background shell stuff.
Ah this is not quite there. It incorrectly notifies for nested sessions (like when agentic fetch finishes), while it should only notify for the main session.
It incorrectly notifies for nested sessions
should be fixed and ready for testing
@Amolith quick question on this one: did we ascertain if we're able to get an icon in here on macOS and Windows? I remember testing this with you but can't remember where we ended up.
@meowgorithm we can if we use terminal escape sequences—@aymanbagabas mentioned OSC 99, OSC 9, and OSC 777 when I was chatting with him about it—rather than platform mechanisms like this PR currently uses. On Mac, the platform just doesn't allow icons; escape sequences are the only way. For Windows, I think you weren't getting any notifications at first because you were in WSL and it did work, including the icon, from outside WSL. I think escape sequences would work whether in or out.
I'm completely open to doing whatever you all think best. Personally, I think this PR would be good to start with. Then I, or whoever, can research which terminals support which escape sequences and build a hierarchy where we use the most featureful method available, falling back to the platform mechanism, this PR, if no escape sequences but focus/blur are supported.