From bfe2f84f4fc8f37517eaadfa3274eb0ff79b47ab Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Fri, 15 May 2026 01:37:57 +0300 Subject: [PATCH] feat: add system toast delivery --- .pi/docs/CONFIGURATION.md | 3 +- AGENTS.md | 2 +- src/app/api.rs | 12 +++++-- src/app/config_io.rs | 1 + src/app/input/settings.rs | 8 +++-- src/client/mod.rs | 46 ++++++++++++++++++++++--- src/config/model.rs | 11 ++++++ src/main.rs | 1 + src/platform/fallback.rs | 5 +++ src/platform/linux.rs | 64 ++++++++++++++++++++++++++++++++++ src/platform/macos.rs | 61 +++++++++++++++++++++++++++++++++ src/server/headless.rs | 72 +++++++++++++++++++++++++++++++++------ src/server/protocol.rs | 26 +++++++++----- src/ui/settings.rs | 1 + 14 files changed, 282 insertions(+), 31 deletions(-) diff --git a/.pi/docs/CONFIGURATION.md b/.pi/docs/CONFIGURATION.md index 0bee5925..25f17b5a 100644 --- a/.pi/docs/CONFIGURATION.md +++ b/.pi/docs/CONFIGURATION.md @@ -299,7 +299,8 @@ delivery = "off" available values: - `off` — disable popup notifications - `herdr` — show top-right in-app toasts -- `terminal` — ask the outer terminal to show a desktop notification +- `terminal` — ask the outer terminal to show a desktop notification. Some terminals suppress foreground notifications, including Ghostty on macOS. +- `system` — ask the OS notification service directly. macOS uses `osascript`; Linux requires `notify-send`. compatibility note: - older configs may still use `ui.toast.enabled = true|false` diff --git a/AGENTS.md b/AGENTS.md index 6c8f5f07..b73c77cf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,4 +101,4 @@ The app update check and the in-app **What's New** flow both depend on that exac Do not edit `website/latest.json` during normal feature, fix, or test work. It describes the latest published release binaries, not the current unreleased source tree. The release workflow updates it after release assets are published. -When changing the server/client wire protocol, ensure `src/server/protocol.rs::PROTOCOL_VERSION` is bumped relative to the latest released tag, and update all hardcoded protocol expectations and manual protocol fixtures in tests. Multiple unreleased wire changes in the same release cycle do not need repeated bumps; Herdr supports tagged releases, not arbitrary `master` client/server compatibility. Keep protocol test expectations intentionally explicit so compatibility changes are reviewed instead of silently following the constant. +When changing the server/client wire protocol, compare `src/server/protocol.rs::PROTOCOL_VERSION` against the latest released tag. Bump it only if the current source protocol is not already greater than the latest released protocol. Multiple unreleased wire changes in the same release cycle must share the same single protocol bump; Herdr supports tagged releases, not arbitrary `master` client/server compatibility. When a bump is required, update all hardcoded protocol expectations and manual protocol fixtures in tests. Keep protocol test expectations intentionally explicit so compatibility changes are reviewed instead of silently following the constant. diff --git a/src/app/api.rs b/src/app/api.rs index 46429ffe..1fb11118 100644 --- a/src/app/api.rs +++ b/src/app/api.rs @@ -84,11 +84,17 @@ impl App { if self.local_terminal_notifications && matches!( self.state.toast_config.delivery, - crate::config::ToastDelivery::Terminal + crate::config::ToastDelivery::Terminal | crate::config::ToastDelivery::System ) { + let notify = match self.state.toast_config.delivery { + crate::config::ToastDelivery::Terminal => crate::terminal_notify::show_notification, + crate::config::ToastDelivery::System => crate::platform::show_desktop_notification, + _ => unreachable!("toast delivery was checked above"), + }; + if let Some(version) = update_ready_version { - let _ = crate::terminal_notify::show_notification( + let _ = notify( &format!("v{version} available"), Some("detach, then run `herdr update`"), ); @@ -127,7 +133,7 @@ impl App { ToastKind::Finished => "finished", ToastKind::UpdateInstalled => "updated", }; - let _ = crate::terminal_notify::show_notification( + let _ = notify( &format!("{} {}", agent_label, event_text), Some(&crate::app::actions::notification_context( ws, diff --git a/src/app/config_io.rs b/src/app/config_io.rs index 0bf04831..36f44b9a 100644 --- a/src/app/config_io.rs +++ b/src/app/config_io.rs @@ -62,6 +62,7 @@ impl App { crate::config::ToastDelivery::Off => "\"off\"", crate::config::ToastDelivery::Herdr => "\"herdr\"", crate::config::ToastDelivery::Terminal => "\"terminal\"", + crate::config::ToastDelivery::System => "\"system\"", }; if self.update_config_file("toast setting", |content| { let content = diff --git a/src/app/input/settings.rs b/src/app/input/settings.rs index 9e89e515..79fdf5d8 100644 --- a/src/app/input/settings.rs +++ b/src/app/input/settings.rs @@ -51,6 +51,7 @@ fn toast_delivery_index(delivery: ToastDelivery) -> usize { ToastDelivery::Off => 0, ToastDelivery::Herdr => 1, ToastDelivery::Terminal => 2, + ToastDelivery::System => 3, } } @@ -58,7 +59,8 @@ fn toast_delivery_for_index(idx: usize) -> ToastDelivery { match idx { 0 => ToastDelivery::Off, 1 => ToastDelivery::Herdr, - _ => ToastDelivery::Terminal, + 2 => ToastDelivery::Terminal, + _ => ToastDelivery::System, } } @@ -151,7 +153,7 @@ pub(super) fn update_settings_state(state: &mut AppState, key: KeyEvent) -> Opti }, SettingsSection::Toast => match key.code { KeyCode::Up | KeyCode::Char('k') => state.settings.list.move_prev(), - KeyCode::Down | KeyCode::Char('j') => state.settings.list.move_next(3), + KeyCode::Down | KeyCode::Char('j') => state.settings.list.move_next(4), KeyCode::Enter | KeyCode::Char(' ') => { let delivery = toast_delivery_for_index(state.settings.list.selected); return Some(SettingsAction::SaveToastDelivery(delivery)); @@ -274,7 +276,7 @@ impl AppState { } SettingsSection::Toast => { let list_y = area.y + 3; - if row >= list_y && row < list_y + 6 { + if row >= list_y && row < list_y + 8 { Some(((row - list_y) / 2) as usize) } else { None diff --git a/src/client/mod.rs b/src/client/mod.rs index 00e1b138..50a54ef3 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -672,19 +672,21 @@ fn reload_local_sound_config(sound_config: &mut crate::config::SoundConfig) { } fn handle_notify(kind: NotifyKind, message: &str, sound_config: &crate::config::SoundConfig) { - handle_notify_with_terminal_notifier( + handle_notify_with_notifiers( kind, message, sound_config, crate::terminal_notify::show_notification, + crate::platform::show_desktop_notification, ); } -fn handle_notify_with_terminal_notifier( +fn handle_notify_with_notifiers( kind: NotifyKind, message: &str, sound_config: &crate::config::SoundConfig, mut show_terminal_notification: impl FnMut(&str, Option<&str>) -> io::Result, + mut show_system_notification: impl FnMut(&str, Option<&str>) -> io::Result, ) { match kind { NotifyKind::Sound => { @@ -700,12 +702,25 @@ fn handle_notify_with_terminal_notifier( } } NotifyKind::Toast => { - debug!(message = message, "received toast notification from server"); + debug!( + message = message, + "received terminal toast notification from server" + ); let (title, body) = crate::terminal_notify::split_message(message); if let Err(err) = show_terminal_notification(title, body) { warn!(err = %err, "failed to emit terminal notification"); } } + NotifyKind::SystemToast => { + debug!( + message = message, + "received system toast notification from server" + ); + let (title, body) = crate::terminal_notify::split_message(message); + if let Err(err) = show_system_notification(title, body) { + warn!(err = %err, "failed to emit system notification"); + } + } } } @@ -1066,7 +1081,7 @@ mod tests { let sound_config = crate::config::SoundConfig::default(); let mut emitted = None; - handle_notify_with_terminal_notifier( + handle_notify_with_notifiers( NotifyKind::Toast, "pi finished: workspace 1", &sound_config, @@ -1074,6 +1089,29 @@ mod tests { emitted = Some((title.to_string(), body.map(str::to_string))); Ok(true) }, + |_, _| Ok(false), + ); + + assert_eq!( + emitted, + Some(("pi finished".to_string(), Some("workspace 1".to_string()))) + ); + } + + #[test] + fn system_toast_notify_from_server_uses_system_notifier() { + let sound_config = crate::config::SoundConfig::default(); + let mut emitted = None; + + handle_notify_with_notifiers( + NotifyKind::SystemToast, + "pi finished: workspace 1", + &sound_config, + |_, _| Ok(false), + |title, body| { + emitted = Some((title.to_string(), body.map(str::to_string))); + Ok(true) + }, ); assert_eq!( diff --git a/src/config/model.rs b/src/config/model.rs index a576418b..a8485fb0 100644 --- a/src/config/model.rs +++ b/src/config/model.rs @@ -9,6 +9,7 @@ pub enum ToastDelivery { Off, Herdr, Terminal, + System, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, Default)] @@ -298,6 +299,16 @@ delivery = "terminal" assert_eq!(config.ui.toast.delivery, ToastDelivery::Terminal); } + #[test] + fn toast_config_parses_system_delivery() { + let toml = r#" +[ui.toast] +delivery = "system" +"#; + let config: Config = toml::from_str(toml).unwrap(); + assert_eq!(config.ui.toast.delivery, ToastDelivery::System); + } + #[test] fn toast_config_legacy_enabled_true_maps_to_herdr() { let toml = r#" diff --git a/src/main.rs b/src/main.rs index 8a43746d..92e6220d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -143,6 +143,7 @@ const DEFAULT_CONFIG: &str = r##"# herdr configuration # off = disable pop-up notifications # herdr = show top-right in-app toasts # terminal = ask the outer terminal to show a desktop notification +# system = ask the OS notification service directly # delivery = "off" # Play sounds when agents change state in background workspaces diff --git a/src/platform/fallback.rs b/src/platform/fallback.rs index 7addbc5d..bf3ac42b 100644 --- a/src/platform/fallback.rs +++ b/src/platform/fallback.rs @@ -34,3 +34,8 @@ pub fn process_exists(_pid: u32) -> bool { pub fn write_clipboard(_bytes: &[u8]) -> bool { false } + +/// Unsupported platform stub. +pub fn show_desktop_notification(_title: &str, _body: Option<&str>) -> std::io::Result { + Ok(false) +} diff --git a/src/platform/linux.rs b/src/platform/linux.rs index 13aa6ec8..6823cca8 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -155,6 +155,43 @@ pub fn write_clipboard(bytes: &[u8]) -> bool { false } +/// Show a native desktop notification through libnotify's command-line helper. +pub fn show_desktop_notification(title: &str, body: Option<&str>) -> std::io::Result { + show_desktop_notification_with_command(title, body, |program| Command::new(program)) +} + +fn show_desktop_notification_with_command( + title: &str, + body: Option<&str>, + mut command: impl FnMut(&str) -> Command, +) -> std::io::Result { + if std::env::var_os("DISPLAY").is_none() && std::env::var_os("WAYLAND_DISPLAY").is_none() { + return Ok(false); + } + + let mut cmd = command("notify-send"); + cmd.arg("--").arg(title); + if let Some(body) = body.filter(|body| !body.is_empty()) { + cmd.arg(body); + } + run_notification_command(cmd) +} + +fn run_notification_command(mut command: Command) -> std::io::Result { + let status = match command + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + { + Ok(status) => status, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(err) => return Err(err), + }; + + Ok(status.success()) +} + fn clipboard_commands() -> Vec { let mut commands = Vec::new(); @@ -248,4 +285,31 @@ mod tests { assert_eq!(commands[0].program, "xclip"); assert_eq!(commands[1].program, "xsel"); } + + #[test] + fn desktop_notification_separates_option_like_titles() { + let _guard = env_lock().lock().unwrap(); + unsafe { + std::env::remove_var("WAYLAND_DISPLAY"); + std::env::set_var("DISPLAY", ":0"); + } + + let path = + std::env::temp_dir().join(format!("herdr-notify-send-args-{}", std::process::id())); + let script = "printf '%s\\n' \"$@\" > \"$HERDR_NOTIFY_ARGS\""; + let shown = show_desktop_notification_with_command("-danger", Some("body"), |_| { + let mut cmd = Command::new("sh"); + cmd.arg("-c") + .arg(script) + .arg("notify-send") + .env("HERDR_NOTIFY_ARGS", &path); + cmd + }) + .expect("notification command should run"); + + assert!(shown); + let args = std::fs::read_to_string(&path).expect("args file"); + let _ = std::fs::remove_file(&path); + assert_eq!(args, "--\n-danger\nbody\n"); + } } diff --git a/src/platform/macos.rs b/src/platform/macos.rs index 987f5f54..3debc244 100644 --- a/src/platform/macos.rs +++ b/src/platform/macos.rs @@ -201,6 +201,43 @@ pub fn write_clipboard(bytes: &[u8]) -> bool { ) } +/// Show a native macOS notification through AppleScript. +pub fn show_desktop_notification(title: &str, body: Option<&str>) -> std::io::Result { + show_desktop_notification_with_command(title, body, |program| Command::new(program)) +} + +fn show_desktop_notification_with_command( + title: &str, + body: Option<&str>, + mut command: impl FnMut(&str) -> Command, +) -> std::io::Result { + let mut cmd = command("/usr/bin/osascript"); + cmd.arg("-e") + .arg("on run argv") + .arg("-e") + .arg("display notification (item 2 of argv) with title (item 1 of argv)") + .arg("-e") + .arg("end run") + .arg(title) + .arg(body.unwrap_or_default()); + run_notification_command(cmd) +} + +fn run_notification_command(mut command: Command) -> std::io::Result { + let status = match command + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + { + Ok(status) => status, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false), + Err(err) => return Err(err), + }; + + Ok(status.success()) +} + fn run_clipboard_command(command: &ClipboardCommand, bytes: &[u8]) -> bool { let mut child = match Command::new(command.program) .args(command.args) @@ -469,4 +506,28 @@ mod tests { assert_eq!(argv.join(" "), "node /Users/can/.local/bin/pi"); assert!(!argv.join(" ").contains("codex.system")); } + + #[test] + fn desktop_notification_uses_osascript_argv() { + let path = + std::env::temp_dir().join(format!("herdr-osascript-args-{}", std::process::id())); + let script = "printf '%s\\n' \"$@\" > \"$HERDR_NOTIFY_ARGS\""; + let shown = show_desktop_notification_with_command("title", Some("body"), |_| { + let mut cmd = Command::new("sh"); + cmd.arg("-c") + .arg(script) + .arg("osascript") + .env("HERDR_NOTIFY_ARGS", &path); + cmd + }) + .expect("notification command should run"); + + assert!(shown); + let args = std::fs::read_to_string(&path).expect("args file"); + let _ = std::fs::remove_file(&path); + assert_eq!( + args, + "-e\non run argv\n-e\ndisplay notification (item 2 of argv) with title (item 1 of argv)\n-e\nend run\ntitle\nbody\n" + ); + } } diff --git a/src/server/headless.rs b/src/server/headless.rs index 11c486cb..cc57c1f5 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -87,7 +87,15 @@ const SHUTDOWN_API_TIMEOUT: Duration = Duration::from_secs(5); const CLIENT_ACCEPT_POLL_INTERVAL: Duration = Duration::from_millis(250); fn should_forward_toast_to_clients(delivery: config::ToastDelivery) -> bool { - matches!(delivery, config::ToastDelivery::Terminal) + toast_notify_kind(delivery).is_some() +} + +fn toast_notify_kind(delivery: config::ToastDelivery) -> Option { + match delivery { + config::ToastDelivery::Terminal => Some(protocol::NotifyKind::Toast), + config::ToastDelivery::System => Some(protocol::NotifyKind::SystemToast), + config::ToastDelivery::Off | config::ToastDelivery::Herdr => None, + } } fn toast_event_text(kind: app::state::ToastKind) -> &'static str { @@ -847,7 +855,8 @@ impl HeadlessServer { if let Some(msg) = toast_msg { self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Toast, + kind: toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), message: msg, }); } @@ -930,7 +939,8 @@ impl HeadlessServer { if let Some(msg) = toast_msg { self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Toast, + kind: toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), message: msg, }); } @@ -962,7 +972,8 @@ impl HeadlessServer { if let Some(msg) = toast_msg { self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Toast, + kind: toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), message: msg, }); } @@ -991,7 +1002,7 @@ impl HeadlessServer { /// - Detect when a sound would be played and forward as /// `ServerMessage::Notify { kind: Sound }` to the foreground client. /// - Detect when a toast is set on AppState and forward as - /// `ServerMessage::Notify { kind: Toast }` to the foreground client. + /// `ServerMessage::Notify` to the foreground client for terminal/system delivery. fn drain_internal_events_with_forwarding(&mut self) -> bool { let mut changed = false; while let Ok(ev) = self.app.event_rx.try_recv() { @@ -1322,9 +1333,9 @@ impl HeadlessServer { let response = self.app.handle_api_request(msg.request); let _ = msg.respond_to.send(response); - // Forward new toast state only when terminal delivery is selected. + // Forward new toast state only when a client-local delivery mode is selected. // Herdr delivery renders the toast in-frame and must not ask clients to - // show a terminal/desktop notification. + // show a terminal or system notification. let toast_after = self.app.state.toast.clone(); let forwarded_toast_from_state = if should_forward_toast_to_clients(self.app.state.toast_config.delivery) @@ -1335,7 +1346,8 @@ impl HeadlessServer { let msg_text = format!("{}: {}", toast.title, toast.context); debug!(msg = %msg_text, "forwarding toast notification from API request"); self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Toast, + kind: toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), message: msg_text, }); true @@ -1406,7 +1418,8 @@ impl HeadlessServer { ) ); self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Toast, + kind: toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), message: msg_text, }); } @@ -2897,6 +2910,7 @@ mod tests { Some(client_tx), ), ); + server.foreground_client_id = Some(1); server.app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; let changed = server.handle_internal_event_with_forwarding(AppEvent::UpdateReady { @@ -2909,10 +2923,48 @@ mod tests { client_control_rx .recv_timeout(Duration::from_millis(50)) .is_err(), - "herdr delivery should render in-frame instead of forwarding a terminal notification" + "herdr delivery should render in-frame instead of forwarding a client-local notification" ); } + #[test] + fn system_toast_delivery_forwards_system_notify_kind() { + let mut server = test_headless_server(); + let (client_tx, client_control_rx, _client_rx) = test_client_writer(); + + server.clients.insert( + 1, + ClientConnection::new( + (80, 24), + crate::kitty_graphics::HostCellSize::default(), + crate::terminal_theme::TerminalTheme::default(), + None, + 1, + RenderEncoding::SemanticFrame, + Some(client_tx), + ), + ); + server.foreground_client_id = Some(1); + server.app.state.toast_config.delivery = crate::config::ToastDelivery::System; + + let changed = server.handle_internal_event_with_forwarding(AppEvent::UpdateReady { + version: "9.9.9".to_string(), + }); + + assert!(changed); + match read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("system toast message"), + ) { + ServerMessage::Notify { kind, message } => { + assert_eq!(kind, protocol::NotifyKind::SystemToast); + assert_eq!(message, "v9.9.9 available: detach, then run `herdr update`"); + } + other => panic!("expected system toast notify, got {other:?}"), + } + } + #[test] fn stale_api_agent_report_does_not_forward_done_sound() { let mut server = test_headless_server(); diff --git a/src/server/protocol.rs b/src/server/protocol.rs index 1a220767..e9b7975e 100644 --- a/src/server/protocol.rs +++ b/src/server/protocol.rs @@ -247,8 +247,10 @@ pub struct TerminalFrame { pub enum NotifyKind { /// Play a sound (bell/agent-done, etc.). Sound, - /// Display a toast message. + /// Display a toast message through the outer terminal. Toast, + /// Display a toast message through the host OS notification service. + SystemToast, } /// Messages sent from the server to the client over the client protocol socket. @@ -754,14 +756,20 @@ mod tests { #[test] fn server_notify_roundtrip() { - let msg = ServerMessage::Notify { - kind: NotifyKind::Sound, - message: "agent done".to_owned(), - }; - let encoded = bincode::serde::encode_to_vec(&msg, bincode::config::standard()).unwrap(); - let (decoded, _): (ServerMessage, _) = - bincode::serde::decode_from_slice(&encoded, bincode::config::standard()).unwrap(); - assert_eq!(msg, decoded); + for kind in [ + NotifyKind::Sound, + NotifyKind::Toast, + NotifyKind::SystemToast, + ] { + let msg = ServerMessage::Notify { + kind, + message: "agent done".to_owned(), + }; + let encoded = bincode::serde::encode_to_vec(&msg, bincode::config::standard()).unwrap(); + let (decoded, _): (ServerMessage, _) = + bincode::serde::decode_from_slice(&encoded, bincode::config::standard()).unwrap(); + assert_eq!(msg, decoded); + } } #[test] diff --git a/src/ui/settings.rs b/src/ui/settings.rs index ef80d85a..abc103d1 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -99,6 +99,7 @@ pub(super) fn render_settings_overlay(app: &AppState, frame: &mut Frame, area: R ("off", ToastDelivery::Off), ("inside herdr", ToastDelivery::Herdr), ("via terminal", ToastDelivery::Terminal), + ("via system", ToastDelivery::System), ], app.toast_delivery(), app.settings.list.selected,