From 4001fd2ec8ff05a982da18744b8cbde9df3ab530 Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Sat, 6 Jun 2026 17:41:44 +0300 Subject: [PATCH] feat: improve toast ergonomics refs #486 --- docs/next/CHANGELOG.md | 1 + .../src/content/docs/cli-reference.mdx | 10 +- .../src/content/docs/configuration.mdx | 14 +- .../website/src/content/docs/socket-api.mdx | 23 + src/api/mod.rs | 1 + src/api/schema.rs | 78 ++ src/api/server.rs | 1 + src/app/actions.rs | 405 +++++++- src/app/api.rs | 301 +++++- src/app/input/mouse.rs | 2 + src/app/input/navigate.rs | 4 + src/app/mod.rs | 185 ++++ src/app/runtime.rs | 16 + src/app/state.rs | 26 + src/cli.rs | 2 + src/cli/notification.rs | 207 ++++ src/client/mod.rs | 55 +- src/config.rs | 5 +- src/config/model.rs | 123 ++- src/main.rs | 15 +- src/protocol/wire.rs | 7 +- src/server/headless.rs | 958 +++++++++++++++--- src/ui.rs | 164 ++- src/ui/status.rs | 123 ++- tests/api_ping.rs | 2 +- tests/cli_wrapper.rs | 12 +- tests/client_mode.rs | 36 +- tests/cross_area.rs | 16 +- tests/detach_reattach.rs | 44 +- tests/multi_client.rs | 2 +- tests/server_headless.rs | 14 +- 31 files changed, 2596 insertions(+), 256 deletions(-) create mode 100644 src/cli/notification.rs diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index cac5e68e..2752a0d3 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -12,6 +12,7 @@ - Added `herdr integration install droid` for Factory Droid hooks that report session ids through Herdr's socket API. When native agent session restore is enabled, Herdr can resume Droid panes with `droid --resume `. - Added directional pane swap with `prefix+shift+h/j/k/l`, a pane context-menu swap action, pane layout/neighbor/edge/focus/resize socket APIs, matching CLI commands, and optional `pane split --ratio` support. - Added `herdr pane zoom` and the `pane.zoom` socket API to toggle, set, or clear tab-local pane zoom from scripts and integrations. +- Added toast ergonomics controls for delayed agent notifications, in-app toast placement, copied-to-clipboard feedback, and the `notification.show` socket API with `herdr notification show` and optional `none`, `done`, or `request` sounds. (#486) ## [0.6.8] - 2026-06-04 diff --git a/docs/next/website/src/content/docs/cli-reference.mdx b/docs/next/website/src/content/docs/cli-reference.mdx index be21972e..61ca1f28 100644 --- a/docs/next/website/src/content/docs/cli-reference.mdx +++ b/docs/next/website/src/content/docs/cli-reference.mdx @@ -1,6 +1,6 @@ --- title: CLI reference -description: Herdr commands for sessions, workspaces, tabs, panes, agents, waits, integrations, and status. +description: Herdr commands for sessions, workspaces, tabs, panes, notifications, agents, waits, integrations, and status. --- Herdr’s CLI talks to the running server over the same local socket API used by integrations and agents. @@ -43,6 +43,14 @@ herdr server reload-config `herdr server` runs the headless server explicitly. Use it for supervised or service-style setups. `reload-config` applies reloadable settings without restarting panes. +## Notifications + +```bash +herdr notification show [--body TEXT] [--position top-left|top-right|bottom-left|bottom-right] [--sound none|done|request] +``` + +`notification show` uses the configured `[ui.toast]` delivery. `--position` only affects in-app Herdr toasts. `--sound` defaults to `none`; `done` and `request` play the existing finished and needs-attention sounds only when the notification is shown. + ## Sessions ```bash diff --git a/docs/next/website/src/content/docs/configuration.mdx b/docs/next/website/src/content/docs/configuration.mdx index 1cabdd51..97a6a36a 100644 --- a/docs/next/website/src/content/docs/configuration.mdx +++ b/docs/next/website/src/content/docs/configuration.mdx @@ -303,11 +303,19 @@ Herdr can show popup notifications when agents finish or need input. ```toml [ui.toast] delivery = "off" +delay_seconds = 1 + +[ui.toast.herdr] +position = "bottom-right" + +[ui.toast.clipboard] +enabled = true +position = "bottom-center" ``` `delivery = "off"` disables popup notifications. This is the default. -`delivery = "herdr"` shows a top-right toast inside the Herdr UI. Click the toast, or bind `keys.open_notification_target`, to focus the target workspace, tab, and pane. +`delivery = "herdr"` shows a toast inside the Herdr UI. Click the toast, or bind `keys.open_notification_target`, to focus the target workspace, tab, and pane. Set `ui.toast.herdr.position` to `top-left`, `top-right`, `bottom-left`, or `bottom-right`; desktop positions are relative to the full Herdr frame. `delivery = "terminal"` asks the outer terminal to show a desktop notification. Herdr sends terminal notification escape sequences for Ghostty, iTerm2, Kitty, and WezTerm. This is useful over SSH because the local terminal owns the notification. @@ -315,6 +323,10 @@ delivery = "off" Popup notifications are for background attention. Herdr suppresses popups for the active tab. +`delay_seconds` waits before sending finished or needs-input agent notifications. Herdr notifies only if the pane is still in the same state when the delay expires. Set it to `0` for instant notifications. Valid values are `0` through `3600`. + +Clipboard feedback is configured separately because it confirms a foreground copy action and is never sent through terminal or system delivery. Set `ui.toast.clipboard.enabled = false` to hide the copied-to-clipboard popup. Clipboard positions are `top-left`, `top-center`, `top-right`, `bottom-left`, `bottom-center`, and `bottom-right`. + ## Sound Sound notifications are enabled by default and are played by the local Herdr client. diff --git a/docs/next/website/src/content/docs/socket-api.mdx b/docs/next/website/src/content/docs/socket-api.mdx index 645d2845..7b820539 100644 --- a/docs/next/website/src/content/docs/socket-api.mdx +++ b/docs/next/website/src/content/docs/socket-api.mdx @@ -81,6 +81,7 @@ Raw socket method names use dot notation: | Area | Methods | | --- | --- | | Server | `ping`, `server.stop`, `server.reload_config` | +| Notification | `notification.show` | | Workspace | `workspace.create`, `workspace.list`, `workspace.get`, `workspace.focus`, `workspace.rename`, `workspace.close` | | Worktree | `worktree.list`, `worktree.create`, `worktree.open`, `worktree.remove` | | Tab | `tab.create`, `tab.list`, `tab.get`, `tab.focus`, `tab.rename`, `tab.close` | @@ -137,6 +138,28 @@ Omitting `pane_id` targets the server's active focused pane. The response is true when either zoom state or focus changed. Reason values are `single_pane`, `already_zoomed`, and `already_unzoomed`. +The CLI wrapper for `notification.show` is: + +```bash +herdr notification show "build failed" --body "api workspace" --position top-left --sound request +``` + +Show a user notification through the configured toast delivery: + +```json +{"id":"req_notify","method":"notification.show","params":{"title":"build failed","body":"api workspace","position":"top-left","sound":"request"}} +``` + +`title` is required and must contain visible text after control characters and repeated whitespace are removed. `body` is optional. Herdr collapses newlines, tabs, carriage returns, and repeated whitespace into spaces, then trims notification text to 80 characters for `title` and 240 characters for `body`. An empty sanitized `title` returns `invalid_params`. `position` is optional and applies only when `ui.toast.delivery = "herdr"`; desktop positions are relative to the full Herdr frame, and omitted positions use `ui.toast.herdr.position`. Terminal, system, and off delivery ignore `position`. `sound` is optional and can be `none`, `done`, or `request`; it defaults to `none` and plays only when the notification is shown. + +The response reports whether anything was shown: + +```json +{"id":"req_notify","result":{"type":"notification_show","shown":true,"reason":"shown"}} +``` + +Possible reasons are `shown`, `disabled`, `rate_limited`, `no_foreground_client`, and `busy`. `disabled` means `ui.toast.delivery = "off"`. `busy` means an existing in-app toast was not replaced. Terminal and system delivery are best-effort through the current foreground attached Herdr client. + Worktree methods manage Git checkouts as Herdr workspaces. `worktree.create` creates a checkout and returns the new `workspace`, `tab`, `root_pane`, and `worktree` records. `worktree.open` opens an existing checkout or returns the already-open workspace. `worktree.remove` runs `git worktree remove` against a linked child workspace and never deletes the branch. Create a worktree from a source workspace: diff --git a/src/api/mod.rs b/src/api/mod.rs index 7d2d5bc1..c168313b 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -22,6 +22,7 @@ pub(crate) fn request_changes_ui(request: &Request) -> bool { matches!( &request.method, Method::ServerReloadConfig(_) + | Method::NotificationShow(_) | Method::WorkspaceCreate(_) | Method::WorkspaceFocus(_) | Method::WorkspaceRename(_) diff --git a/src/api/schema.rs b/src/api/schema.rs index c0715758..ce1ca9bd 100644 --- a/src/api/schema.rs +++ b/src/api/schema.rs @@ -24,6 +24,8 @@ pub enum Method { ServerLiveHandoff(ServerLiveHandoffParams), #[serde(rename = "server.reload_config")] ServerReloadConfig(EmptyParams), + #[serde(rename = "notification.show")] + NotificationShow(NotificationShowParams), #[serde(rename = "workspace.create")] WorkspaceCreate(WorkspaceCreateParams), #[serde(rename = "workspace.list")] @@ -130,6 +132,50 @@ pub struct EmptyParams {} #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] pub struct PingParams {} +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct NotificationShowParams { + pub title: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub body: Option<String>, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub position: Option<crate::config::ToastHerdrPosition>, + #[serde(default, skip_serializing_if = "NotificationShowSound::is_none")] + pub sound: NotificationShowSound, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +#[serde(rename_all = "snake_case")] +pub enum NotificationShowSound { + #[default] + None, + Done, + Request, +} + +impl NotificationShowSound { + pub fn is_none(&self) -> bool { + matches!(self, Self::None) + } + + pub fn to_sound(self) -> Option<crate::sound::Sound> { + match self { + Self::None => None, + Self::Done => Some(crate::sound::Sound::Done), + Self::Request => Some(crate::sound::Sound::Request), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum NotificationShowReason { + Shown, + Disabled, + RateLimited, + NoForegroundClient, + Busy, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct WorkspaceTarget { pub workspace_id: String, @@ -627,6 +673,10 @@ pub enum ResponseResult { matched_line: Option<String>, read: PaneReadResult, }, + NotificationShow { + shown: bool, + reason: NotificationShowReason, + }, IntegrationInstall { target: IntegrationTarget, details: IntegrationInstallResult, @@ -1063,6 +1113,34 @@ mod tests { assert_eq!(restored, request); } + #[test] + fn notification_show_request_parses() { + let json = r#"{"id":"req_1","method":"notification.show","params":{"title":"build failed","body":"api workspace","position":"top-left","sound":"request"}}"#; + let request: Request = serde_json::from_str(json).unwrap(); + let Method::NotificationShow(params) = request.method else { + panic!("wrong method parsed"); + }; + assert_eq!(params.title, "build failed"); + assert_eq!(params.body.as_deref(), Some("api workspace")); + assert_eq!( + params.position, + Some(crate::config::ToastHerdrPosition::TopLeft) + ); + assert_eq!(params.sound, NotificationShowSound::Request); + } + + #[test] + fn notification_show_sound_defaults_to_none() { + let json = + r#"{"id":"req_1","method":"notification.show","params":{"title":"build failed"}}"#; + let request: Request = serde_json::from_str(json).unwrap(); + let Method::NotificationShow(params) = request.method else { + panic!("wrong method parsed"); + }; + + assert_eq!(params.sound, NotificationShowSound::None); + } + #[test] fn unknown_method_is_rejected() { let json = r#"{"id":"req_1","method":"nope","params":{}}"#; diff --git a/src/api/server.rs b/src/api/server.rs index 7e4dc3b1..e2c4b4a1 100644 --- a/src/api/server.rs +++ b/src/api/server.rs @@ -271,6 +271,7 @@ fn api_method_name(method: &Method) -> &'static str { Method::ServerStop(_) => "server.stop", Method::ServerLiveHandoff(_) => "server.live_handoff", Method::ServerReloadConfig(_) => "server.reload_config", + Method::NotificationShow(_) => "notification.show", Method::WorkspaceCreate(_) => "workspace.create", Method::WorkspaceList(_) => "workspace.list", Method::WorkspaceGet(_) => "workspace.get", diff --git a/src/app/actions.rs b/src/app/actions.rs index 17daa6ff..06ffe51b 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -12,8 +12,9 @@ use crate::workspace::WorkspaceGitStatus; use unicode_width::UnicodeWidthChar; use super::state::{ - text_matches_query, AppState, Mode, NavigatorRow, NavigatorStateFilter, NavigatorTarget, - PaneFocusTarget, ToastKind, ToastNotification, ToastTarget, ViewLayout, + text_matches_query, AgentNotificationDelivery, AppState, Mode, NavigatorRow, + NavigatorStateFilter, NavigatorTarget, PaneFocusTarget, PendingAgentNotification, ToastKind, + ToastNotification, ToastTarget, ViewLayout, }; fn is_background_completion_transition(prev_state: AgentState, new_state: AgentState) -> bool { @@ -71,6 +72,27 @@ fn toast_agent_label(agent_label: &str) -> &str { agent_label } +fn toast_event_text(kind: ToastKind) -> &'static str { + match kind { + ToastKind::NeedsAttention => "needs attention", + ToastKind::Finished => "finished", + ToastKind::UpdateInstalled => "updated", + } +} + +fn sound_for_toast_kind( + kind: ToastKind, + suppress_active_tab_notifications: bool, +) -> Option<crate::sound::Sound> { + match kind { + ToastKind::NeedsAttention => Some(crate::sound::Sound::Request), + ToastKind::Finished if !suppress_active_tab_notifications => { + Some(crate::sound::Sound::Done) + } + ToastKind::Finished | ToastKind::UpdateInstalled => None, + } +} + pub fn notification_context( ws: &crate::workspace::Workspace, workspace_label: &str, @@ -2174,6 +2196,7 @@ impl AppState { kind: ToastKind::UpdateInstalled, title: format!("v{version} available"), context: crate::update::update_install_instruction(&install_command), + position: None, target: None, }); } @@ -2384,12 +2407,149 @@ impl AppState { } let seen = pane.seen; - if self.local_sound_playback && self.sound.allows(change.known_agent) { - if let Some(sound) = notification_sound_for_state_change( - suppress_active_tab_notifications, - change.previous_state, + if let Some(delivery) = self.record_or_deliver_agent_notification(ws_idx, pane_id, change) { + self.apply_agent_notification_delivery(&delivery); + } + + Some(seen) + } + + fn record_or_deliver_agent_notification( + &mut self, + ws_idx: usize, + pane_id: PaneId, + change: &EffectiveStateChange, + ) -> Option<AgentNotificationDelivery> { + self.pending_agent_notifications.remove(&pane_id); + + let is_active_tab = self.pane_is_in_active_tab(ws_idx, pane_id); + let suppress_active_tab_notifications = + active_tab_suppresses_notifications(is_active_tab, self.outer_terminal_focus); + + let client_notification_kind = notification_toast_for_state_change( + suppress_active_tab_notifications, + change.previous_state, + change.state, + ); + let sound = notification_sound_for_state_change( + suppress_active_tab_notifications, + change.previous_state, + change.state, + ); + if client_notification_kind.is_none() && sound.is_none() { + return None; + } + + let agent_label = change.agent_label.clone()?; + let kind = client_notification_kind.unwrap_or(match sound { + Some(crate::sound::Sound::Request) => ToastKind::NeedsAttention, + Some(crate::sound::Sound::Done) | None => ToastKind::Finished, + }); + let workspace_id = self.workspaces[ws_idx].id.clone(); + + if self.toast_config.delay_seconds == 0 { + return self.agent_notification_delivery( + ws_idx, + pane_id, + workspace_id, + agent_label, + change.known_agent, + kind, change.state, - ) { + ); + } + + self.pending_agent_notifications.insert( + pane_id, + PendingAgentNotification { + pane_id, + workspace_id, + agent_label, + known_agent: change.known_agent, + kind, + state: change.state, + deadline: { + let now = std::time::Instant::now(); + let delay_seconds = self + .toast_config + .delay_seconds + .min(crate::config::MAX_TOAST_DELAY_SECONDS); + now.checked_add(std::time::Duration::from_secs(delay_seconds)) + .unwrap_or(now) + }, + }, + ); + None + } + + fn agent_notification_delivery( + &self, + ws_idx: usize, + pane_id: PaneId, + workspace_id: String, + agent_label: String, + known_agent: Option<Agent>, + kind: ToastKind, + expected_state: AgentState, + ) -> Option<AgentNotificationDelivery> { + let terminal_state = self + .workspaces + .get(ws_idx)? + .pane_state(pane_id) + .and_then(|pane| self.terminals.get(&pane.attached_terminal_id))?; + if terminal_state.state != expected_state { + return None; + } + if terminal_state.effective_agent_label() != Some(agent_label.as_str()) { + return None; + } + + let is_active_tab = self.pane_is_in_active_tab(ws_idx, pane_id); + let suppress_active_tab_notifications = + active_tab_suppresses_notifications(is_active_tab, self.outer_terminal_focus); + let sound = sound_for_toast_kind(kind, suppress_active_tab_notifications) + .filter(|_| self.sound.allows(known_agent)); + let build_toast = || { + let workspace_label = self.workspaces[ws_idx].display_name(); + let context = + notification_context(&self.workspaces[ws_idx], &workspace_label, ws_idx, pane_id); + ToastNotification { + kind, + title: format!( + "{} {}", + toast_agent_label(&agent_label), + toast_event_text(kind) + ), + context, + position: None, + target: Some(ToastTarget { + workspace_id: workspace_id.clone(), + pane_id, + }), + } + }; + let toast = (!is_active_tab).then(&build_toast); + let client_notification = (!suppress_active_tab_notifications).then(build_toast); + + if toast.is_none() && client_notification.is_none() && sound.is_none() { + return None; + } + + Some(AgentNotificationDelivery { + pane_id, + workspace_id, + agent_label, + known_agent, + kind, + toast, + client_notification, + sound, + }) + } + + fn apply_agent_notification_delivery(&mut self, delivery: &AgentNotificationDelivery) { + if self.local_sound_playback { + if let Some(sound) = delivery.sound { crate::sound::play(sound, &self.sound); } } @@ -2398,42 +2558,61 @@ impl AppState { self.toast_config.delivery, crate::config::ToastDelivery::Herdr ) { - if let (Some(agent_label), Some(kind)) = ( - change.agent_label.as_deref(), - notification_toast_for_state_change( - is_active_tab, - change.previous_state, - change.state, - ), - ) { - let event_text = match kind { - ToastKind::NeedsAttention => "needs attention", - ToastKind::Finished => "finished", - ToastKind::UpdateInstalled => "updated", - }; - let workspace_label = self.workspaces[ws_idx].display_name(); - let context = notification_context( - &self.workspaces[ws_idx], - &workspace_label, - ws_idx, - pane_id, - ); - self.toast = Some(ToastNotification { - kind, - title: format!("{} {}", toast_agent_label(agent_label), event_text), - context, - target: Some(ToastTarget { - workspace_id: self.workspaces[ws_idx].id.clone(), - pane_id, - }), - }); + if let Some(toast) = delivery.toast.clone() { + self.toast = Some(toast); } } + } - Some(seen) + pub fn next_pending_agent_notification_deadline(&self) -> Option<std::time::Instant> { + self.pending_agent_notifications + .values() + .map(|pending| pending.deadline) + .min() + } + + pub fn drain_due_agent_notifications( + &mut self, + now: std::time::Instant, + ) -> Vec<AgentNotificationDelivery> { + let due_panes: Vec<PaneId> = self + .pending_agent_notifications + .iter() + .filter_map(|(&pane_id, pending)| (pending.deadline <= now).then_some(pane_id)) + .collect(); + let mut deliveries = Vec::new(); + + for pane_id in due_panes { + let Some(pending) = self.pending_agent_notifications.remove(&pane_id) else { + continue; + }; + let Some(ws_idx) = self + .workspaces + .iter() + .position(|ws| ws.id == pending.workspace_id) + else { + continue; + }; + let Some(delivery) = self.agent_notification_delivery( + ws_idx, + pending.pane_id, + pending.workspace_id, + pending.agent_label, + pending.known_agent, + pending.kind, + pending.state, + ) else { + continue; + }; + self.apply_agent_notification_delivery(&delivery); + deliveries.push(delivery); + } + + deliveries } fn handle_pane_died(&mut self, pane_id: PaneId) { + self.pending_agent_notifications.remove(&pane_id); let ws_idx = self .workspaces .iter() @@ -2500,6 +2679,7 @@ mod tests { fn app_with_workspaces(names: &[&str]) -> AppState { let mut state = AppState::test_new(); + state.toast_config.delay_seconds = 0; for name in names { let ws = Workspace::test_new(name); state.workspaces.push(ws); @@ -3784,6 +3964,157 @@ mod tests { assert_eq!(toast.context, "background · 2"); } + #[test] + fn delayed_background_waiting_schedules_before_toast() { + let mut state = app_with_workspaces(&["active", "background"]); + state.active = Some(0); + state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + state.toast_config.delay_seconds = 1; + let bg_pane_id = *state.workspaces[1].panes.keys().next().unwrap(); + + state.handle_app_event(AppEvent::StateChanged { + pane_id: bg_pane_id, + agent: Some(Agent::Pi), + state: AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + + assert!(state.toast.is_none()); + assert!(state.pending_agent_notifications.contains_key(&bg_pane_id)); + + let deadline = state.next_pending_agent_notification_deadline().unwrap(); + let deliveries = state.drain_due_agent_notifications(deadline); + assert_eq!(deliveries.len(), 1); + + let toast = state.toast.as_ref().unwrap(); + assert_eq!(toast.kind, ToastKind::NeedsAttention); + assert_eq!(toast.title, "pi needs attention"); + assert_eq!(toast.context, "background · 2"); + assert!(state.pending_agent_notifications.is_empty()); + } + + #[test] + fn delayed_background_waiting_cancels_when_agent_resumes_working() { + let mut state = app_with_workspaces(&["active", "background"]); + state.active = Some(0); + state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + state.toast_config.delay_seconds = 1; + let bg_pane_id = *state.workspaces[1].panes.keys().next().unwrap(); + + state.handle_app_event(AppEvent::StateChanged { + pane_id: bg_pane_id, + agent: Some(Agent::Pi), + state: AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + let deadline = state.next_pending_agent_notification_deadline().unwrap(); + + state.handle_app_event(AppEvent::StateChanged { + pane_id: bg_pane_id, + agent: Some(Agent::Pi), + state: AgentState::Working, + visible_blocker: false, + visible_idle: false, + visible_working: true, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + + assert!(state.pending_agent_notifications.is_empty()); + assert!(state.drain_due_agent_notifications(deadline).is_empty()); + assert!(state.toast.is_none()); + } + + #[test] + fn delayed_background_waiting_is_suppressed_if_pane_becomes_active() { + let mut state = app_with_workspaces(&["active", "background"]); + state.active = Some(0); + state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + state.toast_config.delay_seconds = 1; + let bg_pane_id = *state.workspaces[1].panes.keys().next().unwrap(); + + state.handle_app_event(AppEvent::StateChanged { + pane_id: bg_pane_id, + agent: Some(Agent::Pi), + state: AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + let deadline = state.next_pending_agent_notification_deadline().unwrap(); + state.active = Some(1); + + assert!(state.drain_due_agent_notifications(deadline).is_empty()); + assert!(state.toast.is_none()); + } + + #[test] + fn delayed_active_tab_unfocused_keeps_client_notification_available() { + let mut state = app_with_workspaces(&["active"]); + state.active = Some(0); + state.outer_terminal_focus = Some(false); + state.toast_config.delivery = crate::config::ToastDelivery::System; + state.toast_config.delay_seconds = 1; + let pane_id = *state.workspaces[0].panes.keys().next().unwrap(); + + state.handle_app_event(AppEvent::StateChanged { + pane_id, + agent: Some(Agent::Pi), + state: AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + + let deadline = state.next_pending_agent_notification_deadline().unwrap(); + let deliveries = state.drain_due_agent_notifications(deadline); + + assert_eq!(deliveries.len(), 1); + assert!(deliveries[0].toast.is_none()); + assert!(deliveries[0].client_notification.is_some()); + assert!(state.toast.is_none()); + } + + #[test] + fn delayed_background_waiting_is_cleared_when_pane_dies() { + let mut state = app_with_workspaces(&["active", "background"]); + state.active = Some(0); + state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + state.toast_config.delay_seconds = 1; + let bg_pane_id = *state.workspaces[1].panes.keys().next().unwrap(); + + state.handle_app_event(AppEvent::StateChanged { + pane_id: bg_pane_id, + agent: Some(Agent::Pi), + state: AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + let deadline = state.next_pending_agent_notification_deadline().unwrap(); + state.handle_app_event(AppEvent::PaneDied { + pane_id: bg_pane_id, + }); + + assert!(state.pending_agent_notifications.is_empty()); + assert!(state.drain_due_agent_notifications(deadline).is_empty()); + assert!(state.toast.is_none()); + } + #[test] fn hook_reported_unknown_agent_sets_toast_title_from_label() { let mut state = app_with_workspaces(&["active", "background"]); diff --git a/src/app/api.rs b/src/app/api.rs index 592ee7da..4bb89a77 100644 --- a/src/app/api.rs +++ b/src/app/api.rs @@ -12,6 +12,8 @@ mod worktrees; use super::{api_helpers::pane_agent_status, App, Mode, OverlayPaneState, ToastKind}; use crate::events::AppEvent; +const API_NOTIFICATION_RATE_LIMIT: Duration = Duration::from_secs(1); + #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum RuntimeExitAction { RespawnShell, @@ -154,7 +156,7 @@ impl App { if let Some((version, install_command)) = update_ready { let instruction = crate::update::update_install_instruction(&install_command); let _ = notify(&format!("v{version} available"), Some(&instruction)); - } else { + } else if self.state.toast_config.delay_seconds == 0 { for update in &pane_updates { let is_active_tab = self .state @@ -257,6 +259,11 @@ impl App { } pub(crate) fn show_clipboard_feedback(&mut self) { + if !self.state.toast_config.clipboard.enabled { + self.state.copy_feedback = None; + self.copy_feedback_deadline = None; + return; + } self.state.copy_feedback = Some(crate::app::state::CopyFeedback { message: "copied to clipboard".to_string(), }); @@ -396,7 +403,7 @@ impl App { } } - pub(super) fn sync_toast_deadline( + pub(crate) fn sync_toast_deadline( &mut self, previous_toast: Option<crate::app::state::ToastNotification>, ) { @@ -412,6 +419,72 @@ impl App { } } + pub(crate) fn emit_delayed_client_local_agent_notifications( + &self, + deliveries: &[crate::app::state::AgentNotificationDelivery], + ) { + if !self.local_terminal_notifications + || !matches!( + self.state.toast_config.delivery, + crate::config::ToastDelivery::Terminal | crate::config::ToastDelivery::System + ) + { + return; + } + + 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"), + }; + + for delivery in deliveries { + let Some(toast) = &delivery.client_notification else { + continue; + }; + let _ = notify(&toast.title, Some(&toast.context)); + } + } + + pub(crate) fn refresh_agent_notification_delivery_contexts( + &mut self, + deliveries: &mut [crate::app::state::AgentNotificationDelivery], + ) { + for delivery in deliveries { + let Some(ws_idx) = self + .state + .workspaces + .iter() + .position(|ws| ws.id == delivery.workspace_id) + else { + continue; + }; + let ws = &self.state.workspaces[ws_idx]; + let workspace_label = + ws.display_name_from(&self.state.terminals, &self.terminal_runtimes); + let context = crate::app::actions::notification_context( + ws, + &workspace_label, + ws_idx, + delivery.pane_id, + ); + if let Some(toast) = delivery.toast.as_mut() { + toast.context = context.clone(); + } + if let Some(toast) = delivery.client_notification.as_mut() { + toast.context = context.clone(); + } + if let Some(toast) = self.state.toast.as_mut() { + if toast.target.as_ref().is_some_and(|target| { + target.workspace_id == delivery.workspace_id + && target.pane_id == delivery.pane_id + }) { + toast.context = context; + } + } + } + } + pub(super) fn emit_event(&self, event: crate::api::schema::EventEnvelope) { self.event_hub.push(event); } @@ -519,6 +592,9 @@ impl App { }, } } + Method::NotificationShow(params) => { + return self.handle_notification_show(request.id, params); + } Method::WorkspaceList(_) => return self.handle_workspace_list(request.id), Method::WorkspaceGet(target) => return self.handle_workspace_get(request.id, target), Method::WorkspaceCreate(params) => { @@ -606,6 +682,128 @@ impl App { serde_json::to_string(&response).unwrap() } + + fn handle_notification_show( + &mut self, + id: String, + params: crate::api::schema::NotificationShowParams, + ) -> String { + use crate::api::schema::{NotificationShowReason, ResponseResult}; + + let requested_sound = params.sound; + let Some(title) = sanitized_notification_text(¶ms.title, 80) else { + return responses::encode_error(id, "invalid_params", "notification title is empty"); + }; + let body = params + .body + .as_deref() + .and_then(|body| sanitized_notification_text(body, 240)); + + let reason = match self.state.toast_config.delivery { + crate::config::ToastDelivery::Off => NotificationShowReason::Disabled, + crate::config::ToastDelivery::Herdr => { + if self.state.toast.is_some() { + NotificationShowReason::Busy + } else if self.api_notification_rate_limited(Instant::now()) { + NotificationShowReason::RateLimited + } else { + let previous_toast = self.state.toast.clone(); + self.mark_api_notification_shown(Instant::now()); + self.state.toast = Some(crate::app::state::ToastNotification { + kind: ToastKind::UpdateInstalled, + title, + context: body.unwrap_or_default(), + position: params.position, + target: None, + }); + self.sync_toast_deadline(previous_toast); + self.emit_api_notification_sound(requested_sound); + NotificationShowReason::Shown + } + } + crate::config::ToastDelivery::Terminal | crate::config::ToastDelivery::System => { + if self.api_notification_rate_limited(Instant::now()) { + NotificationShowReason::RateLimited + } else { + 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!("notification delivery was checked above"), + }; + match notify(&title, body.as_deref()) { + Ok(true) => { + self.mark_api_notification_shown(Instant::now()); + self.emit_api_notification_sound(requested_sound); + NotificationShowReason::Shown + } + Ok(false) | Err(_) => NotificationShowReason::NoForegroundClient, + } + } + } + }; + + responses::encode_success( + id, + ResponseResult::NotificationShow { + shown: matches!(reason, NotificationShowReason::Shown), + reason, + }, + ) + } + + fn emit_api_notification_sound(&self, sound: crate::api::schema::NotificationShowSound) { + if !self.state.local_sound_playback || !self.state.sound.allows(None) { + return; + } + if let Some(sound) = sound.to_sound() { + crate::sound::play(sound, &self.state.sound); + } + } + + pub(crate) fn api_notification_rate_limited(&self, now: Instant) -> bool { + self.last_api_notification_at + .is_some_and(|last| now.duration_since(last) < API_NOTIFICATION_RATE_LIMIT) + } + + pub(crate) fn mark_api_notification_shown(&mut self, now: Instant) { + self.last_api_notification_at = Some(now); + } +} + +fn sanitized_notification_text(value: &str, max_chars: usize) -> Option<String> { + let mut sanitized = String::new(); + let mut previous_space = false; + for ch in value.chars() { + let replacement = if ch == '\n' || ch == '\r' || ch == '\t' { + Some(' ') + } else if ch.is_control() { + None + } else { + Some(ch) + }; + let Some(ch) = replacement else { + continue; + }; + if ch.is_whitespace() { + if previous_space { + continue; + } + previous_space = true; + sanitized.push(' '); + } else { + previous_space = false; + sanitized.push(ch); + } + if sanitized.chars().count() >= max_chars { + break; + } + } + let sanitized = sanitized.trim().to_string(); + (!sanitized.is_empty()).then_some(sanitized) } #[cfg(test)] @@ -660,6 +858,7 @@ mod tests { app.state.selected = 0; app.state.mode = Mode::Terminal; app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + app.state.toast_config.delay_seconds = 0; let (events, _) = tokio::sync::mpsc::channel(4); let runtime = crate::terminal::TerminalRuntime::spawn( @@ -713,6 +912,103 @@ mod tests { let _ = std::fs::remove_dir_all(temp_root); } + #[tokio::test] + async fn delayed_herdr_toast_context_uses_live_root_runtime_cwd_label() { + let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); + let mut app = App::new( + &crate::config::Config::default(), + true, + None, + api_rx, + crate::api::EventHub::default(), + ); + + let mut workspace = crate::workspace::Workspace::test_new("stale"); + workspace.custom_name = None; + let root = workspace.tabs[0].root_pane; + let terminal_id = workspace.terminal_id(root).cloned().unwrap(); + let temp_root = std::env::temp_dir().join(format!( + "herdr-delayed-toast-context-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let stale_cwd = temp_root.join("__herdr_original__"); + let live_cwd = temp_root.join("__herdr_projects__"); + std::fs::create_dir_all(&stale_cwd).unwrap(); + std::fs::create_dir_all(&live_cwd).unwrap(); + init_repo(&stale_cwd); + init_repo(&live_cwd); + + workspace.identity_cwd = stale_cwd.clone(); + app.state.workspaces = vec![workspace]; + app.state.ensure_test_terminals(); + app.state.terminals.get_mut(&terminal_id).unwrap().cwd = stale_cwd; + app.state.active = None; + app.state.selected = 0; + app.state.mode = Mode::Terminal; + app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + app.state.toast_config.delay_seconds = 1; + + let (events, _) = tokio::sync::mpsc::channel(4); + let runtime = crate::terminal::TerminalRuntime::spawn( + root, + 24, + 80, + live_cwd.clone(), + 0, + crate::terminal_theme::TerminalTheme::default(), + crate::pane::PaneShellConfig::new("/bin/sh", crate::config::ShellModeConfig::NonLogin), + events, + std::sync::Arc::new(tokio::sync::Notify::new()), + std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + ) + .unwrap(); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(2); + while runtime.cwd() != Some(live_cwd.clone()) && std::time::Instant::now() < deadline { + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + app.terminal_runtimes.insert(terminal_id, runtime); + + app.handle_internal_event(AppEvent::StateChanged { + pane_id: root, + agent: Some(Agent::Codex), + state: AgentState::Working, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + app.handle_internal_event(AppEvent::StateChanged { + pane_id: root, + agent: Some(Agent::Codex), + state: AgentState::Idle, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: std::time::Instant::now(), + }); + + let notification_deadline = app + .state + .next_pending_agent_notification_deadline() + .expect("pending notification deadline"); + assert!(app.handle_scheduled_tasks(notification_deadline, false)); + assert_eq!( + app.state.toast.as_ref().map(|toast| toast.context.as_str()), + Some("__herdr_projects__ · 1") + ); + + for (_, runtime) in app.terminal_runtimes.drain() { + runtime.shutdown(); + } + let _ = std::fs::remove_dir_all(temp_root); + } + #[tokio::test] async fn pane_died_respawns_shell_and_clears_restored_agent_session() { let (_api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); @@ -802,6 +1098,7 @@ mod tests { kind: ToastKind::Finished, title: "codex finished".into(), context: "__herdr_original__ · 1".into(), + position: None, target: Some(crate::app::state::ToastTarget { workspace_id, pane_id: root, diff --git a/src/app/input/mouse.rs b/src/app/input/mouse.rs index 9272c279..ae10c0fa 100644 --- a/src/app/input/mouse.rs +++ b/src/app/input/mouse.rs @@ -2244,6 +2244,7 @@ mod tests { app.state.active = Some(0); app.state.selected = 0; app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + app.state.toast_config.delay_seconds = 0; let target_terminal_id = app.state.workspaces[1] .panes .get(&target_pane) @@ -2305,6 +2306,7 @@ mod tests { kind: crate::app::state::ToastKind::Finished, title: "pi finished".into(), context: "background · 2".into(), + position: None, target: Some(crate::app::state::ToastTarget { workspace_id, pane_id: target_pane, diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index 7f803825..8dacc5c9 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -148,6 +148,7 @@ impl App { kind: crate::app::state::ToastKind::NeedsAttention, title: "custom command failed".to_string(), context: err.to_string(), + position: None, target: None, }); self.sync_toast_deadline(previous_toast); @@ -229,6 +230,7 @@ impl App { kind: crate::app::state::ToastKind::NeedsAttention, title: "edit scrollback failed".to_string(), context: err.to_string(), + position: None, target: None, }); self.sync_toast_deadline(previous_toast); @@ -271,6 +273,7 @@ impl App { kind: crate::app::state::ToastKind::Finished, title: "opened scrollback".to_string(), context: format!("focused pane {public_pane_id}"), + position: None, target: None, }); } @@ -1295,6 +1298,7 @@ mod tests { kind: crate::app::state::ToastKind::NeedsAttention, title: "pi needs attention".into(), context: "two".into(), + position: None, target: Some(crate::app::state::ToastTarget { workspace_id: target_workspace_id, pane_id: target_pane, diff --git a/src/app/mod.rs b/src/app/mod.rs index a9408736..6a387c9a 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -97,6 +97,7 @@ pub struct App { pub(crate) config_diagnostic_deadline: Option<Instant>, pub(crate) toast_deadline: Option<Instant>, pub(crate) copy_feedback_deadline: Option<Instant>, + pub(crate) last_api_notification_at: Option<Instant>, pub(crate) last_git_remote_status_refresh: Instant, pub(crate) git_refresh_in_flight: bool, pub(crate) git_refresh_due_after_in_flight: bool, @@ -479,6 +480,7 @@ impl App { update_dismissed: false, config_diagnostic, toast: None, + pending_agent_notifications: std::collections::HashMap::new(), copy_feedback: None, outer_terminal_focus: None, prefix_code, @@ -568,6 +570,7 @@ impl App { config_diagnostic_deadline: None, toast_deadline: None, copy_feedback_deadline: None, + last_api_notification_at: None, state, terminal_runtimes: restored_terminal_runtimes, event_tx, @@ -1263,6 +1266,7 @@ impl App { kind: crate::app::state::ToastKind::UpdateInstalled, title: "reloaded config".to_string(), context: "using config.toml".to_string(), + position: None, target: None, }); } @@ -1274,6 +1278,7 @@ impl App { kind: crate::app::state::ToastKind::UpdateInstalled, title: "reloaded config".to_string(), context: "with warnings".to_string(), + position: None, target: None, }); } @@ -1729,6 +1734,19 @@ mod tests { assert!(app.copy_feedback_deadline.is_some()); } + #[test] + fn clipboard_feedback_can_be_disabled() { + let mut app = test_app(); + app.state.toast_config.clipboard.enabled = false; + + app.handle_internal_event(AppEvent::ClipboardWrite { + content: b"copied".to_vec(), + }); + + assert!(app.state.copy_feedback.is_none()); + assert!(app.copy_feedback_deadline.is_none()); + } + #[test] fn clipboard_feedback_does_not_replace_notification_toast() { let mut app = test_app(); @@ -1736,6 +1754,7 @@ mod tests { kind: crate::app::state::ToastKind::NeedsAttention, title: "pi needs attention".to_string(), context: "background · 2".to_string(), + position: None, target: None, }); let original_toast = app.state.toast.clone(); @@ -1754,6 +1773,172 @@ mod tests { ); } + #[test] + fn notification_show_api_creates_herdr_toast_with_position() { + let mut app = test_app(); + app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + + let response = + app.handle_api_request_after_internal_events_drained(crate::api::schema::Request { + id: "notify".into(), + method: crate::api::schema::Method::NotificationShow( + crate::api::schema::NotificationShowParams { + title: "build failed".into(), + body: Some("api workspace".into()), + position: Some(crate::config::ToastHerdrPosition::TopLeft), + sound: crate::api::schema::NotificationShowSound::None, + }, + ), + }); + + let parsed: crate::api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + crate::api::schema::ResponseResult::NotificationShow { + shown: true, + reason: crate::api::schema::NotificationShowReason::Shown, + } + ); + let toast = app.state.toast.as_ref().expect("api toast"); + assert_eq!(toast.title, "build failed"); + assert_eq!(toast.context, "api workspace"); + assert_eq!( + toast.position, + Some(crate::config::ToastHerdrPosition::TopLeft) + ); + assert!(app.toast_deadline.is_some()); + } + + #[test] + fn notification_show_api_herdr_toast_expires() { + let mut app = test_app(); + app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + + let response = + app.handle_api_request_after_internal_events_drained(crate::api::schema::Request { + id: "notify".into(), + method: crate::api::schema::Method::NotificationShow( + crate::api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: crate::api::schema::NotificationShowSound::None, + }, + ), + }); + + let parsed: crate::api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + crate::api::schema::ResponseResult::NotificationShow { + shown: true, + reason: crate::api::schema::NotificationShowReason::Shown, + } + ); + let deadline = app.toast_deadline.expect("api toast deadline"); + assert!(app.handle_scheduled_tasks(deadline, false)); + assert!(app.state.toast.is_none()); + assert!(app.toast_deadline.is_none()); + } + + #[test] + fn notification_show_api_respects_off_delivery() { + let mut app = test_app(); + app.state.toast_config.delivery = crate::config::ToastDelivery::Off; + + let response = + app.handle_api_request_after_internal_events_drained(crate::api::schema::Request { + id: "notify".into(), + method: crate::api::schema::Method::NotificationShow( + crate::api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: crate::api::schema::NotificationShowSound::None, + }, + ), + }); + + let parsed: crate::api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + crate::api::schema::ResponseResult::NotificationShow { + shown: false, + reason: crate::api::schema::NotificationShowReason::Disabled, + } + ); + assert!(app.state.toast.is_none()); + } + + #[test] + fn notification_show_api_does_not_replace_existing_toast() { + let mut app = test_app(); + app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + app.state.toast = Some(crate::app::state::ToastNotification { + kind: crate::app::state::ToastKind::NeedsAttention, + title: "pi needs attention".to_string(), + context: "background · 2".to_string(), + position: None, + target: None, + }); + + let response = + app.handle_api_request_after_internal_events_drained(crate::api::schema::Request { + id: "notify".into(), + method: crate::api::schema::Method::NotificationShow( + crate::api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: crate::api::schema::NotificationShowSound::None, + }, + ), + }); + + let parsed: crate::api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + crate::api::schema::ResponseResult::NotificationShow { + shown: false, + reason: crate::api::schema::NotificationShowReason::Busy, + } + ); + assert_eq!( + app.state.toast.as_ref().map(|toast| toast.title.as_str()), + Some("pi needs attention") + ); + } + + #[test] + fn notification_show_api_is_rate_limited() { + let mut app = test_app(); + app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + app.mark_api_notification_shown(Instant::now()); + + let response = + app.handle_api_request_after_internal_events_drained(crate::api::schema::Request { + id: "notify".into(), + method: crate::api::schema::Method::NotificationShow( + crate::api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: crate::api::schema::NotificationShowSound::None, + }, + ), + }); + + let parsed: crate::api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + crate::api::schema::ResponseResult::NotificationShow { + shown: false, + reason: crate::api::schema::NotificationShowReason::RateLimited, + } + ); + assert!(app.state.toast.is_none()); + } + #[test] fn internal_event_drain_limits_work_per_tick() { let mut app = test_app(); diff --git a/src/app/runtime.rs b/src/app/runtime.rs index fe971bf5..ca780360 100644 --- a/src/app/runtime.rs +++ b/src/app/runtime.rs @@ -204,6 +204,21 @@ impl App { changed = true; } + if self + .state + .next_pending_agent_notification_deadline() + .is_some_and(|deadline| now >= deadline) + { + let previous_toast = self.state.toast.clone(); + let mut deliveries = self.state.drain_due_agent_notifications(now); + if !deliveries.is_empty() { + self.refresh_agent_notification_delivery_contexts(&mut deliveries); + self.emit_delayed_client_local_agent_notifications(&deliveries); + self.sync_toast_deadline(previous_toast); + changed = true; + } + } + if self .copy_feedback_deadline .is_some_and(|deadline| now >= deadline) @@ -509,6 +524,7 @@ impl App { include_resize_poll.then_some(self.next_resize_poll), self.config_diagnostic_deadline, self.toast_deadline, + self.state.next_pending_agent_notification_deadline(), self.copy_feedback_deadline, self.next_animation_tick, include_git_refresh diff --git a/src/app/state.rs b/src/app/state.rs index e06fb3f5..494bee0f 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -1176,9 +1176,33 @@ pub struct ToastNotification { pub kind: ToastKind, pub title: String, pub context: String, + pub position: Option<crate::config::ToastHerdrPosition>, pub target: Option<ToastTarget>, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PendingAgentNotification { + pub pane_id: PaneId, + pub workspace_id: String, + pub agent_label: String, + pub known_agent: Option<crate::detect::Agent>, + pub kind: ToastKind, + pub state: AgentState, + pub deadline: std::time::Instant, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AgentNotificationDelivery { + pub pane_id: PaneId, + pub workspace_id: String, + pub agent_label: String, + pub known_agent: Option<crate::detect::Agent>, + pub kind: ToastKind, + pub toast: Option<ToastNotification>, + pub client_notification: Option<ToastNotification>, + pub sound: Option<crate::sound::Sound>, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct CopyFeedback { pub message: String, @@ -1288,6 +1312,7 @@ pub struct AppState { pub update_dismissed: bool, pub config_diagnostic: Option<String>, pub toast: Option<ToastNotification>, + pub pending_agent_notifications: std::collections::HashMap<PaneId, PendingAgentNotification>, pub copy_feedback: Option<CopyFeedback>, /// Last reported focus state for the outer terminal hosting herdr. /// None means unsupported or not yet reported, which preserves active-pane suppression. @@ -1615,6 +1640,7 @@ impl AppState { update_dismissed: false, config_diagnostic: None, toast: None, + pending_agent_notifications: std::collections::HashMap::new(), copy_feedback: None, outer_terminal_focus: None, prefix_code: KeyCode::Char('b'), diff --git a/src/cli.rs b/src/cli.rs index f756b96f..3650ebdb 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -10,6 +10,7 @@ use crate::api::schema::{ mod agent; mod integration; +mod notification; mod pane; mod server; mod status; @@ -40,6 +41,7 @@ pub fn maybe_run(args: &[String]) -> std::io::Result<CommandOutcome> { "workspace" => workspace::run_workspace_command(&args[2..])?, "worktree" => worktree::run_worktree_command(&args[2..])?, "tab" => tab::run_tab_command(&args[2..])?, + "notification" => notification::run_notification_command(&args[2..])?, "agent" => agent::run_agent_command(&args[2..])?, "terminal" => run_terminal_command(&args[2..])?, "pane" => pane::run_pane_command(&args[2..])?, diff --git a/src/cli/notification.rs b/src/cli/notification.rs new file mode 100644 index 00000000..ff56d4f2 --- /dev/null +++ b/src/cli/notification.rs @@ -0,0 +1,207 @@ +use crate::api::schema::{Method, NotificationShowParams, NotificationShowSound, Request}; +use crate::config::ToastHerdrPosition; + +pub(super) fn run_notification_command(args: &[String]) -> std::io::Result<i32> { + let Some(subcommand) = args.first().map(|arg| arg.as_str()) else { + print_notification_help(); + return Ok(2); + }; + + match subcommand { + "show" => notification_show(&args[1..]), + "help" | "--help" | "-h" => { + print_notification_help(); + Ok(0) + } + _ => { + print_notification_help(); + Ok(2) + } + } +} + +fn notification_show(args: &[String]) -> std::io::Result<i32> { + let params = match parse_notification_show_args(args) { + Ok(params) => params, + Err(NotificationShowArgError::Usage) => { + eprintln!( + "usage: herdr notification show <title> [--body TEXT] [--position top-left|top-right|bottom-left|bottom-right] [--sound none|done|request]" + ); + return Ok(2); + } + Err(NotificationShowArgError::Message(message)) => { + eprintln!("{message}"); + return Ok(2); + } + }; + + super::print_response(&super::send_request(&Request { + id: "cli:notification:show".into(), + method: Method::NotificationShow(params), + })?) +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum NotificationShowArgError { + Usage, + Message(String), +} + +fn parse_notification_show_args( + args: &[String], +) -> Result<NotificationShowParams, NotificationShowArgError> { + let Some(title) = args.first().cloned() else { + return Err(NotificationShowArgError::Usage); + }; + if matches!(title.as_str(), "help" | "--help" | "-h") { + return Err(NotificationShowArgError::Usage); + } + + let mut body = None; + let mut position = None; + let mut sound = NotificationShowSound::None; + let mut index = 1; + while index < args.len() { + match args[index].as_str() { + "--body" => { + let Some(value) = args.get(index + 1) else { + return Err(NotificationShowArgError::Message( + "missing value for --body".into(), + )); + }; + body = Some(value.clone()); + index += 2; + } + "--position" => { + let Some(value) = args.get(index + 1) else { + return Err(NotificationShowArgError::Message( + "missing value for --position".into(), + )); + }; + position = Some(parse_toast_position(value)?); + index += 2; + } + "--sound" => { + let Some(value) = args.get(index + 1) else { + return Err(NotificationShowArgError::Message( + "missing value for --sound".into(), + )); + }; + sound = parse_notification_sound(value)?; + index += 2; + } + other => { + return Err(NotificationShowArgError::Message(format!( + "unknown option: {other}" + ))); + } + } + } + + Ok(NotificationShowParams { + title, + body, + position, + sound, + }) +} + +fn parse_toast_position(value: &str) -> Result<ToastHerdrPosition, NotificationShowArgError> { + match value { + "top-left" => Ok(ToastHerdrPosition::TopLeft), + "top-right" => Ok(ToastHerdrPosition::TopRight), + "bottom-left" => Ok(ToastHerdrPosition::BottomLeft), + "bottom-right" => Ok(ToastHerdrPosition::BottomRight), + _ => Err(NotificationShowArgError::Message(format!( + "invalid position: {value} (expected top-left, top-right, bottom-left, or bottom-right)" + ))), + } +} + +fn parse_notification_sound( + value: &str, +) -> Result<NotificationShowSound, NotificationShowArgError> { + match value { + "none" => Ok(NotificationShowSound::None), + "done" => Ok(NotificationShowSound::Done), + "request" => Ok(NotificationShowSound::Request), + _ => Err(NotificationShowArgError::Message(format!( + "invalid sound: {value} (expected none, done, or request)" + ))), + } +} + +fn print_notification_help() { + eprintln!("herdr notification commands:"); + eprintln!( + " herdr notification show <title> [--body TEXT] [--position top-left|top-right|bottom-left|bottom-right] [--sound none|done|request]" + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn args(values: &[&str]) -> Vec<String> { + values.iter().map(|value| (*value).to_string()).collect() + } + + #[test] + fn notification_show_args_parse_title_body_and_position() { + let params = parse_notification_show_args(&args(&[ + "build failed", + "--body", + "api workspace", + "--position", + "top-right", + "--sound", + "request", + ])) + .unwrap(); + + assert_eq!( + params, + NotificationShowParams { + title: "build failed".into(), + body: Some("api workspace".into()), + position: Some(ToastHerdrPosition::TopRight), + sound: NotificationShowSound::Request, + } + ); + } + + #[test] + fn notification_show_args_reject_invalid_position() { + let error = + parse_notification_show_args(&args(&["build failed", "--position", "top-center"])) + .unwrap_err(); + + assert_eq!( + error, + NotificationShowArgError::Message( + "invalid position: top-center (expected top-left, top-right, bottom-left, or bottom-right)" + .into() + ) + ); + } + + #[test] + fn notification_show_args_default_sound_is_none() { + let params = parse_notification_show_args(&args(&["build failed"])).unwrap(); + + assert_eq!(params.sound, NotificationShowSound::None); + } + + #[test] + fn notification_show_args_reject_invalid_sound() { + let error = + parse_notification_show_args(&args(&["build failed", "--sound", "loud"])).unwrap_err(); + + assert_eq!( + error, + NotificationShowArgError::Message( + "invalid sound: loud (expected none, done, or request)".into() + ) + ); + } +} diff --git a/src/client/mod.rs b/src/client/mod.rs index 0a1b9507..564b5e27 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -862,8 +862,12 @@ async fn run_client_loop( ServerMessage::ServerShutdown { reason } => { return Err(ClientError::ServerShutdown { reason }); } - ServerMessage::Notify { kind, message } => { - handle_notify(kind, &message, &state.sound_config); + ServerMessage::Notify { + kind, + message, + body, + } => { + handle_notify(kind, &message, body.as_deref(), &state.sound_config); } ServerMessage::Clipboard { data } => { forward_clipboard(&data); @@ -993,10 +997,16 @@ fn reload_local_client_config( } } -fn handle_notify(kind: NotifyKind, message: &str, sound_config: &crate::config::SoundConfig) { +fn handle_notify( + kind: NotifyKind, + message: &str, + body: Option<&str>, + sound_config: &crate::config::SoundConfig, +) { handle_notify_with_notifiers( kind, message, + body, sound_config, crate::terminal_notify::show_notification, crate::platform::show_desktop_notification, @@ -1006,6 +1016,7 @@ fn handle_notify(kind: NotifyKind, message: &str, sound_config: &crate::config:: fn handle_notify_with_notifiers( kind: NotifyKind, message: &str, + body: Option<&str>, sound_config: &crate::config::SoundConfig, mut show_terminal_notification: impl FnMut(&str, Option<&str>) -> io::Result<bool>, mut show_system_notification: impl FnMut(&str, Option<&str>) -> io::Result<bool>, @@ -1028,8 +1039,7 @@ fn handle_notify_with_notifiers( 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) { + if let Err(err) = show_terminal_notification(message, body) { warn!(err = %err, "failed to emit terminal notification"); } } @@ -1038,8 +1048,7 @@ fn handle_notify_with_notifiers( 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) { + if let Err(err) = show_system_notification(message, body) { warn!(err = %err, "failed to emit system notification"); } } @@ -1684,7 +1693,8 @@ mod tests { handle_notify_with_notifiers( NotifyKind::Toast, - "pi finished: workspace 1", + "pi finished", + Some("workspace 1"), &sound_config, |title, body| { emitted = Some((title.to_string(), body.map(str::to_string))); @@ -1706,7 +1716,8 @@ mod tests { handle_notify_with_notifiers( NotifyKind::SystemToast, - "pi finished: workspace 1", + "pi finished", + Some("workspace 1"), &sound_config, |_, _| Ok(false), |title, body| { @@ -1721,6 +1732,32 @@ mod tests { ); } + #[test] + fn system_toast_notify_preserves_colon_in_title() { + let sound_config = crate::config::SoundConfig::default(); + let mut emitted = None; + + handle_notify_with_notifiers( + NotifyKind::SystemToast, + "build: failed", + Some("api workspace"), + &sound_config, + |_, _| Ok(false), + |title, body| { + emitted = Some((title.to_string(), body.map(str::to_string))); + Ok(true) + }, + ); + + assert_eq!( + emitted, + Some(( + "build: failed".to_string(), + Some("api workspace".to_string()) + )) + ); + } + #[test] fn decode_clipboard_payload_decodes_base64() { assert_eq!(decode_clipboard_payload("dGVzdA=="), Some(b"test".to_vec())); diff --git a/src/config.rs b/src/config.rs index 4fc275c1..a01be048 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,8 +19,9 @@ pub use self::{ }, model::{ validated_sidebar_bounds, AgentPanelScopeConfig, Config, ConfigReloadReport, - ConfigReloadStatus, KeysConfig, NewTerminalCwdConfig, ShellModeConfig, ToastConfig, - ToastDelivery, UpdateChannelConfig, + ConfigReloadStatus, KeysConfig, NewTerminalCwdConfig, ShellModeConfig, + ToastClipboardPosition, ToastConfig, ToastDelivery, ToastHerdrPosition, + UpdateChannelConfig, MAX_TOAST_DELAY_SECONDS, }, sound::SoundConfig, theme::{parse_color, CustomThemeColors, ThemeConfig}, diff --git a/src/config/model.rs b/src/config/model.rs index 3b8b2cb7..c36150f6 100644 --- a/src/config/model.rs +++ b/src/config/model.rs @@ -8,6 +8,8 @@ use super::{ DEFAULT_MOUSE_SCROLL_LINES, DEFAULT_SCROLLBACK_LIMIT_BYTES, }; +pub const MAX_TOAST_DELAY_SECONDS: u64 = 3600; + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, Default)] #[serde(rename_all = "lowercase")] pub enum UpdateChannelConfig { @@ -49,6 +51,28 @@ pub enum ToastDelivery { System, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, Default)] +#[serde(rename_all = "kebab-case")] +pub enum ToastHerdrPosition { + TopLeft, + TopRight, + BottomLeft, + #[default] + BottomRight, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, Default)] +#[serde(rename_all = "kebab-case")] +pub enum ToastClipboardPosition { + TopLeft, + TopCenter, + TopRight, + BottomLeft, + #[default] + BottomCenter, + BottomRight, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, Default)] #[serde(rename_all = "lowercase")] pub enum AgentPanelScopeConfig { @@ -122,6 +146,22 @@ fn parse_right_click_passthrough_modifier(value: &str) -> Option<Option<KeyModif #[derive(Debug, Clone)] pub struct ToastConfig { pub delivery: ToastDelivery, + pub delay_seconds: u64, + pub herdr: HerdrToastConfig, + pub clipboard: ClipboardToastConfig, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +#[serde(default)] +pub struct HerdrToastConfig { + pub position: ToastHerdrPosition, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +#[serde(default)] +pub struct ClipboardToastConfig { + pub enabled: bool, + pub position: ToastClipboardPosition, } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -600,6 +640,26 @@ impl Default for ToastConfig { fn default() -> Self { Self { delivery: ToastDelivery::Off, + delay_seconds: 1, + herdr: HerdrToastConfig::default(), + clipboard: ClipboardToastConfig::default(), + } + } +} + +impl Default for HerdrToastConfig { + fn default() -> Self { + Self { + position: ToastHerdrPosition::BottomRight, + } + } +} + +impl Default for ClipboardToastConfig { + fn default() -> Self { + Self { + enabled: true, + position: ToastClipboardPosition::BottomCenter, } } } @@ -614,6 +674,9 @@ impl<'de> Deserialize<'de> for ToastConfig { struct RawToastConfig { delivery: Option<ToastDelivery>, enabled: Option<bool>, + delay_seconds: Option<u64>, + herdr: HerdrToastConfig, + clipboard: ClipboardToastConfig, } let raw = RawToastConfig::deserialize(deserializer)?; @@ -622,7 +685,19 @@ impl<'de> Deserialize<'de> for ToastConfig { Some(false) | None => ToastDelivery::Off, }; let delivery = raw.delivery.unwrap_or(legacy_delivery); - Ok(Self { delivery }) + let default = Self::default(); + let delay_seconds = raw.delay_seconds.unwrap_or(default.delay_seconds); + if delay_seconds > MAX_TOAST_DELAY_SECONDS { + return Err(de::Error::custom(format!( + "ui.toast.delay_seconds must be between 0 and {MAX_TOAST_DELAY_SECONDS}" + ))); + } + Ok(Self { + delivery, + delay_seconds, + herdr: raw.herdr, + clipboard: raw.clipboard, + }) } } @@ -984,9 +1059,40 @@ mouse_scroll_lines = 0 let toml = r#" [ui.toast] delivery = "terminal" +delay_seconds = 2 + +[ui.toast.herdr] +position = "top-left" + +[ui.toast.clipboard] +enabled = false +position = "top-center" "#; let config: Config = toml::from_str(toml).unwrap(); assert_eq!(config.ui.toast.delivery, ToastDelivery::Terminal); + assert_eq!(config.ui.toast.delay_seconds, 2); + assert_eq!(config.ui.toast.herdr.position, ToastHerdrPosition::TopLeft); + assert!(!config.ui.toast.clipboard.enabled); + assert_eq!( + config.ui.toast.clipboard.position, + ToastClipboardPosition::TopCenter + ); + } + + #[test] + fn toast_config_defaults_preserve_existing_behavior_with_delay() { + let config = Config::default(); + assert_eq!(config.ui.toast.delivery, ToastDelivery::Off); + assert_eq!(config.ui.toast.delay_seconds, 1); + assert_eq!( + config.ui.toast.herdr.position, + ToastHerdrPosition::BottomRight + ); + assert!(config.ui.toast.clipboard.enabled); + assert_eq!( + config.ui.toast.clipboard.position, + ToastClipboardPosition::BottomCenter + ); } #[test] @@ -1030,6 +1136,21 @@ delivery = "terminal" assert_eq!(config.ui.toast.delivery, ToastDelivery::Terminal); } + #[test] + fn toast_config_rejects_unbounded_delay() { + let toml = format!( + r#" +[ui.toast] +delay_seconds = {} +"#, + MAX_TOAST_DELAY_SECONDS + 1 + ); + + let error = toml::from_str::<Config>(&toml).unwrap_err().to_string(); + + assert!(error.contains("ui.toast.delay_seconds must be between 0 and 3600")); + } + #[test] fn missing_onboarding_shows_setup() { let config = Config::default(); diff --git a/src/main.rs b/src/main.rs index f88880d7..053ecf26 100644 --- a/src/main.rs +++ b/src/main.rs @@ -233,10 +233,18 @@ const DEFAULT_CONFIG: &str = r##"# herdr configuration # Background notification popup delivery [ui.toast] # off = disable pop-up notifications -# herdr = show top-right in-app toasts +# herdr = show in-app toasts # terminal = ask the outer terminal to show a desktop notification # system = ask the OS notification service directly # delivery = "off" +# delay_seconds = 1 + +[ui.toast.herdr] +# position = "bottom-right" + +[ui.toast.clipboard] +# enabled = true +# position = "bottom-center" # Play sounds when agents change state in background workspaces [ui.sound] @@ -424,6 +432,7 @@ fn main() -> io::Result<()> { println!(" herdr workspace <subcommand> ..."); println!(" herdr worktree <subcommand> ..."); println!(" herdr tab <subcommand> ..."); + println!(" herdr notification <subcommand> ..."); println!(" herdr agent <subcommand> ..."); println!(" herdr pane <subcommand> ..."); println!(" herdr wait <subcommand> ..."); @@ -467,6 +476,10 @@ fn main() -> io::Result<()> { "Git worktree helpers over the socket API", ), ("herdr tab <subcommand>", "Tab helpers over the socket API"), + ( + "herdr notification <subcommand>", + "Notification helpers over the socket API", + ), ( "herdr agent <subcommand>", "Agent/terminal helpers over the socket API", diff --git a/src/protocol/wire.rs b/src/protocol/wire.rs index a64a06d5..14a9e3b3 100644 --- a/src/protocol/wire.rs +++ b/src/protocol/wire.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; // --------------------------------------------------------------------------- /// Current protocol version. Bumped when wire format changes incompatibly. -pub const PROTOCOL_VERSION: u32 = 12; +pub const PROTOCOL_VERSION: u32 = 13; /// Maximum allowed frame payload size (2 MB). Frames larger than this are /// rejected to prevent denial-of-service via oversized length prefixes. @@ -371,8 +371,10 @@ pub enum ServerMessage { Notify { /// What kind of notification. kind: NotifyKind, - /// Human-readable message. + /// Human-readable title or sound label. message: String, + /// Optional human-readable notification body. + body: Option<String>, }, /// OSC 52 clipboard data forwarded from a PTY through the server. @@ -889,6 +891,7 @@ mod tests { let msg = ServerMessage::Notify { kind, message: "agent done".to_owned(), + body: None, }; let encoded = bincode::serde::encode_to_vec(&msg, bincode::config::standard()).unwrap(); let (decoded, _): (ServerMessage, _) = diff --git a/src/server/headless.rs b/src/server/headless.rs index c1fc7352..c78df8a1 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -63,6 +63,27 @@ use crate::server::client_transport::ClientWriter; #[cfg(test)] use std::fs; +fn sound_notify_message(sound: crate::sound::Sound) -> &'static str { + match sound { + crate::sound::Sound::Done => "agent done", + crate::sound::Sound::Request => "agent attention", + } +} + +fn notification_show_response_shown(response: &str) -> bool { + let Ok(response) = serde_json::from_str::<api::schema::SuccessResponse>(response) else { + return false; + }; + matches!( + response.result, + api::schema::ResponseResult::NotificationShow { shown: true, .. } + ) +} + +fn non_empty_body(value: &str) -> Option<String> { + (!value.is_empty()).then(|| value.to_owned()) +} + // --------------------------------------------------------------------------- // Loop event enum for the headless server event loop // --------------------------------------------------------------------------- @@ -1297,6 +1318,142 @@ impl HeadlessServer { .unwrap_or(crate::detect::AgentState::Unknown) } + fn forward_agent_notification_delivery( + &mut self, + delivery: &crate::app::state::AgentNotificationDelivery, + ) { + if let Some(sound) = delivery.sound { + self.send_notify_to_foreground_client( + protocol::NotifyKind::Sound, + sound_notify_message(sound), + None, + ); + } + + if should_forward_toast_to_clients(self.app.state.toast_config.delivery) { + if let Some(toast) = &delivery.client_notification { + self.send_notify_to_foreground_client( + toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), + &toast.title, + non_empty_body(&toast.context), + ); + } + } + } + + fn send_notify_to_foreground_client( + &mut self, + kind: protocol::NotifyKind, + message: impl Into<String>, + body: Option<String>, + ) -> bool { + self.send_to_foreground_client(ServerMessage::Notify { + kind, + message: message.into(), + body, + }) + } + + fn send_flat_toast_to_foreground_client( + &mut self, + kind: protocol::NotifyKind, + message: impl AsRef<str>, + ) -> bool { + let (title, body) = crate::terminal_notify::split_message(message.as_ref()); + self.send_notify_to_foreground_client(kind, title, body.map(str::to_string)) + } + + fn handle_notification_show_api( + &mut self, + id: String, + params: api::schema::NotificationShowParams, + ) -> String { + use api::schema::{NotificationShowReason, ResponseResult}; + + let Some(title) = sanitize_notification_text(¶ms.title, 80) else { + return serde_json::to_string(&api::schema::ErrorResponse { + id, + error: api::schema::ErrorBody { + code: "invalid_params".into(), + message: "notification title is empty".into(), + }, + }) + .unwrap_or_else(|_| "{}".to_string()); + }; + + match self.app.state.toast_config.delivery { + config::ToastDelivery::Off => { + return serde_json::to_string(&api::schema::SuccessResponse { + id, + result: ResponseResult::NotificationShow { + shown: false, + reason: NotificationShowReason::Disabled, + }, + }) + .unwrap_or_else(|_| "{}".to_string()); + } + config::ToastDelivery::Herdr => { + let sound = params.sound; + let response = self.app.handle_api_request_after_internal_events_drained( + api::schema::Request { + id, + method: api::schema::Method::NotificationShow(params), + }, + ); + if notification_show_response_shown(&response) { + self.forward_api_notification_sound(sound); + } + return response; + } + config::ToastDelivery::Terminal | config::ToastDelivery::System => {} + } + + let body = params + .body + .as_deref() + .and_then(|body| sanitize_notification_text(body, 240)); + if self.app.api_notification_rate_limited(Instant::now()) { + return serde_json::to_string(&api::schema::SuccessResponse { + id, + result: ResponseResult::NotificationShow { + shown: false, + reason: NotificationShowReason::RateLimited, + }, + }) + .unwrap_or_else(|_| "{}".to_string()); + } + let kind = toast_notify_kind(self.app.state.toast_config.delivery) + .expect("terminal/system delivery has notify kind"); + let shown = self.send_notify_to_foreground_client(kind, title, body); + if shown { + self.app.mark_api_notification_shown(Instant::now()); + self.forward_api_notification_sound(params.sound); + } + let reason = if shown { + NotificationShowReason::Shown + } else { + NotificationShowReason::NoForegroundClient + }; + + serde_json::to_string(&api::schema::SuccessResponse { + id, + result: ResponseResult::NotificationShow { shown, reason }, + }) + .unwrap_or_else(|_| "{}".to_string()) + } + + fn forward_api_notification_sound(&mut self, sound: api::schema::NotificationShowSound) { + let Some(sound) = sound.to_sound() else { + return; + }; + self.send_notify_to_foreground_client( + protocol::NotifyKind::Sound, + sound_notify_message(sound), + None, + ); + } + /// Handles a single internal event with forwarding logic for clipboard, /// sound, and toast notifications to connected clients. /// @@ -1350,51 +1507,51 @@ impl HeadlessServer { let next_state = self.pane_effective_state(pane_id_val); - if self.app.state.sound.allows(agent_val) { + if self.app.state.toast_config.delay_seconds == 0 + && self.app.state.sound.allows(agent_val) + { if let Some(sound) = crate::app::actions::notification_sound_for_state_change( suppress_active_tab_notifications, prev_state, next_state, ) { - let msg = match sound { - crate::sound::Sound::Done => "agent done", - crate::sound::Sound::Request => "agent attention", - }; - self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Sound, - message: msg.to_owned(), - }); + self.send_notify_to_foreground_client( + protocol::NotifyKind::Sound, + sound_notify_message(sound), + None, + ); } } - let toast_msg = - if should_forward_toast_to_clients(self.app.state.toast_config.delivery) { - if self.app.state.toast.is_some() && self.app.state.toast != toast_before { - self.app - .state - .toast - .as_ref() - .map(|toast| format!("{}: {}", toast.title, toast.context)) - } else { - toast_message_from_state_change( - &self.app.state, - &self.app.terminal_runtimes, - pane_id_val, - suppress_active_tab_notifications, - prev_state, - next_state, - ) - } + let toast_msg = if self.app.state.toast_config.delay_seconds == 0 + && should_forward_toast_to_clients(self.app.state.toast_config.delivery) + { + if self.app.state.toast.is_some() && self.app.state.toast != toast_before { + self.app + .state + .toast + .as_ref() + .map(|toast| format!("{}: {}", toast.title, toast.context)) } else { - None - }; + toast_message_from_state_change( + &self.app.state, + &self.app.terminal_runtimes, + pane_id_val, + suppress_active_tab_notifications, + prev_state, + next_state, + ) + } + } else { + None + }; if let Some(msg) = toast_msg { - self.send_to_foreground_client(ServerMessage::Notify { - kind: toast_notify_kind(self.app.state.toast_config.delivery) + self.send_flat_toast_to_foreground_client( + toast_notify_kind(self.app.state.toast_config.delivery) .expect("toast forwarding requires a client notification kind"), - message: msg, - }); + msg, + ); } true @@ -1435,51 +1592,51 @@ impl HeadlessServer { let next_state = self.pane_effective_state(pane_id_val); - if self.app.state.sound.allows(agent_val) { + if self.app.state.toast_config.delay_seconds == 0 + && self.app.state.sound.allows(agent_val) + { if let Some(sound) = crate::app::actions::notification_sound_for_state_change( suppress_active_tab_notifications, prev_state, next_state, ) { - let msg = match sound { - crate::sound::Sound::Done => "agent done", - crate::sound::Sound::Request => "agent attention", - }; - self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Sound, - message: msg.to_owned(), - }); + self.send_notify_to_foreground_client( + protocol::NotifyKind::Sound, + sound_notify_message(sound), + None, + ); } } - let toast_msg = - if should_forward_toast_to_clients(self.app.state.toast_config.delivery) { - if self.app.state.toast.is_some() && self.app.state.toast != toast_before { - self.app - .state - .toast - .as_ref() - .map(|toast| format!("{}: {}", toast.title, toast.context)) - } else { - toast_message_from_state_change( - &self.app.state, - &self.app.terminal_runtimes, - pane_id_val, - suppress_active_tab_notifications, - prev_state, - next_state, - ) - } + let toast_msg = if self.app.state.toast_config.delay_seconds == 0 + && should_forward_toast_to_clients(self.app.state.toast_config.delivery) + { + if self.app.state.toast.is_some() && self.app.state.toast != toast_before { + self.app + .state + .toast + .as_ref() + .map(|toast| format!("{}: {}", toast.title, toast.context)) } else { - None - }; + toast_message_from_state_change( + &self.app.state, + &self.app.terminal_runtimes, + pane_id_val, + suppress_active_tab_notifications, + prev_state, + next_state, + ) + } + } else { + None + }; if let Some(msg) = toast_msg { - self.send_to_foreground_client(ServerMessage::Notify { - kind: toast_notify_kind(self.app.state.toast_config.delivery) + self.send_flat_toast_to_foreground_client( + toast_notify_kind(self.app.state.toast_config.delivery) .expect("toast forwarding requires a client notification kind"), - message: msg, - }); + msg, + ); } true @@ -1513,11 +1670,11 @@ impl HeadlessServer { }; if let Some(msg) = toast_msg { - self.send_to_foreground_client(ServerMessage::Notify { - kind: toast_notify_kind(self.app.state.toast_config.delivery) + self.send_flat_toast_to_foreground_client( + toast_notify_kind(self.app.state.toast_config.delivery) .expect("toast forwarding requires a client notification kind"), - message: msg, - }); + msg, + ); } true @@ -2153,6 +2310,13 @@ impl HeadlessServer { return true; } + if let api::schema::Method::NotificationShow(params) = &msg.request.method { + let response = + self.handle_notification_show_api(msg.request.id.clone(), params.clone()); + let _ = msg.respond_to.send(response); + return true; + } + let mut changed = api::request_changes_ui(&msg.request); let skip_default_workspace = matches!( &msg.request.method, @@ -2218,26 +2382,26 @@ impl HeadlessServer { // Herdr delivery renders the toast in-frame and must not ask clients to // 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) - && toast_after.is_some() - && toast_after != toast_before - { - if let Some(toast) = &toast_after { - 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: toast_notify_kind(self.app.state.toast_config.delivery) - .expect("toast forwarding requires a client notification kind"), - message: msg_text, - }); - true - } else { - false - } + let forwarded_toast_from_state = if should_forward_toast_to_clients( + self.app.state.toast_config.delivery, + ) && toast_after.is_some() + && toast_after != toast_before + { + if let Some(toast) = &toast_after { + debug!(title = %toast.title, body = %toast.context, "forwarding toast notification from API request"); + self.send_notify_to_foreground_client( + toast_notify_kind(self.app.state.toast_config.delivery) + .expect("toast forwarding requires a client notification kind"), + &toast.title, + non_empty_body(&toast.context), + ); + true } else { false - }; + } + } else { + false + }; // Forward notifications for effective pane state changes that occurred // during the API request. Hook authority is already folded into @@ -2284,6 +2448,7 @@ impl HeadlessServer { ); if !forwarded_toast_from_state + && self.app.state.toast_config.delay_seconds == 0 && should_forward_toast_to_clients(self.app.state.toast_config.delivery) { if let Some(kind) = crate::app::actions::notification_toast_for_state_change( @@ -2307,43 +2472,37 @@ impl HeadlessServer { &self.app.state.terminals, &self.app.terminal_runtimes, ); - let msg_text = format!( - "{} {}: {}", - agent_label, - event_text, - crate::app::actions::notification_context( - &self.app.state.workspaces[*ws_idx], - &workspace_label, - *ws_idx, - *pane_id, - ) + let context = crate::app::actions::notification_context( + &self.app.state.workspaces[*ws_idx], + &workspace_label, + *ws_idx, + *pane_id, ); - self.send_to_foreground_client(ServerMessage::Notify { - kind: toast_notify_kind(self.app.state.toast_config.delivery) + self.send_notify_to_foreground_client( + toast_notify_kind(self.app.state.toast_config.delivery) .expect("toast forwarding requires a client notification kind"), - message: msg_text, - }); + format!("{agent_label} {event_text}"), + non_empty_body(&context), + ); } } } // Forward sound notification when server-side sound policy allows it. // Clients still decide locally whether they can execute the side effect. - if self.app.state.sound.allows(agent) { + if self.app.state.toast_config.delay_seconds == 0 && self.app.state.sound.allows(agent) + { if let Some(sound) = crate::app::actions::notification_sound_for_state_change( suppress_active_tab_notifications, *prev_state, new_state, ) { - let msg_text = match sound { - crate::sound::Sound::Done => "agent done", - crate::sound::Sound::Request => "agent attention", - }; debug!(sound = ?sound, "forwarding sound notification from API request"); - self.send_to_foreground_client(ServerMessage::Notify { - kind: protocol::NotifyKind::Sound, - message: msg_text.to_owned(), - }); + self.send_notify_to_foreground_client( + protocol::NotifyKind::Sound, + sound_notify_message(sound), + None, + ); } } } @@ -2919,6 +3078,25 @@ impl HeadlessServer { changed = true; } + if self + .app + .state + .next_pending_agent_notification_deadline() + .is_some_and(|deadline| now >= deadline) + { + let previous_toast = self.app.state.toast.clone(); + let mut deliveries = self.app.state.drain_due_agent_notifications(now); + if !deliveries.is_empty() { + self.app + .refresh_agent_notification_delivery_contexts(&mut deliveries); + self.app.sync_toast_deadline(previous_toast); + for delivery in &deliveries { + self.forward_agent_notification_delivery(delivery); + } + changed = true; + } + } + if self .app .copy_feedback_deadline @@ -3111,6 +3289,38 @@ async fn sleep_until_or_pending(deadline: Option<Instant>) { } } +fn sanitize_notification_text(value: &str, max_chars: usize) -> Option<String> { + let mut sanitized = String::new(); + let mut previous_space = false; + for ch in value.chars() { + let replacement = if ch == '\n' || ch == '\r' || ch == '\t' { + Some(' ') + } else if ch.is_control() { + None + } else { + Some(ch) + }; + let Some(ch) = replacement else { + continue; + }; + if ch.is_whitespace() { + if previous_space { + continue; + } + previous_space = true; + sanitized.push(' '); + } else { + previous_space = false; + sanitized.push(ch); + } + if sanitized.chars().count() >= max_chars { + break; + } + } + let sanitized = sanitized.trim().to_string(); + (!sanitized.is_empty()).then_some(sanitized) +} + fn server_config_diagnostic_summaries(diagnostics: &[String]) -> (Option<String>, Option<String>) { let without_keybindings = diagnostics .iter() @@ -5678,6 +5888,7 @@ next_tab = "" kind: crate::app::state::ToastKind::NeedsAttention, title: "pi needs attention".to_owned(), context: "background · 2".to_owned(), + position: None, target: None, }); server.render_and_stream(); @@ -6182,7 +6393,8 @@ next_tab = "" assert!(server.send_to_foreground_client(ServerMessage::Notify { kind: protocol::NotifyKind::Toast, - message: "pi finished: workspace 1".to_string(), + message: "pi finished".to_string(), + body: Some("workspace 1".to_string()), })); match read_server_message( @@ -6190,9 +6402,14 @@ next_tab = "" .recv_timeout(Duration::from_millis(100)) .expect("foreground toast message"), ) { - ServerMessage::Notify { kind, message } => { + ServerMessage::Notify { + kind, + message, + body, + } => { assert_eq!(kind, protocol::NotifyKind::Toast); - assert_eq!(message, "pi finished: workspace 1"); + assert_eq!(message, "pi finished"); + assert_eq!(body.as_deref(), Some("workspace 1")); } other => panic!("expected toast notify, got {other:?}"), } @@ -6270,17 +6487,524 @@ next_tab = "" .recv_timeout(Duration::from_millis(100)) .expect("system toast message"), ) { - ServerMessage::Notify { kind, message } => { + ServerMessage::Notify { + kind, + message, + body, + } => { assert_eq!(kind, protocol::NotifyKind::SystemToast); + assert_eq!(message, "v9.9.9 available"); assert_eq!( - message, - "v9.9.9 available: detach, run `herdr update`, then follow its restart guidance" + body.as_deref(), + Some("detach, run `herdr update`, then follow its restart guidance") ); } other => panic!("expected system toast notify, got {other:?}"), } } + #[test] + fn notification_show_api_forwards_system_notification_to_foreground_client() { + 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 (respond_to, response_rx) = std::sync::mpsc::channel(); + let changed = server.handle_api_request_with_shutdown_check(api::ApiRequestMessage { + request: api::schema::Request { + id: "notify".into(), + method: api::schema::Method::NotificationShow( + api::schema::NotificationShowParams { + title: "build failed".into(), + body: Some("api workspace".into()), + position: Some(crate::config::ToastHerdrPosition::TopLeft), + sound: api::schema::NotificationShowSound::Request, + }, + ), + }, + respond_to, + }); + + assert!(changed); + let response = response_rx + .recv_timeout(Duration::from_millis(100)) + .unwrap(); + let parsed: api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + api::schema::ResponseResult::NotificationShow { + shown: true, + reason: api::schema::NotificationShowReason::Shown, + } + ); + let first = read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("api notification message"), + ); + let second = read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("api sound message"), + ); + + match first { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::SystemToast); + assert_eq!(message, "build failed"); + assert_eq!(body.as_deref(), Some("api workspace")); + } + other => panic!("expected api notification, got {other:?}"), + } + match second { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::Sound); + assert_eq!(message, "agent attention"); + assert!(body.is_none()); + } + other => panic!("expected api sound, got {other:?}"), + } + } + + #[test] + fn notification_show_api_preserves_colon_in_forwarded_title() { + 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 (respond_to, response_rx) = std::sync::mpsc::channel(); + let changed = server.handle_api_request_with_shutdown_check(api::ApiRequestMessage { + request: api::schema::Request { + id: "notify".into(), + method: api::schema::Method::NotificationShow( + api::schema::NotificationShowParams { + title: "build: failed".into(), + body: Some("api workspace".into()), + position: None, + sound: api::schema::NotificationShowSound::None, + }, + ), + }, + respond_to, + }); + + assert!(changed); + let response = response_rx + .recv_timeout(Duration::from_millis(100)) + .unwrap(); + let parsed: api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + api::schema::ResponseResult::NotificationShow { + shown: true, + reason: api::schema::NotificationShowReason::Shown, + } + ); + match read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("api notification message"), + ) { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::SystemToast); + assert_eq!(message, "build: failed"); + assert_eq!(body.as_deref(), Some("api workspace")); + } + other => panic!("expected api notification, got {other:?}"), + } + } + + #[test] + fn notification_show_api_validates_empty_title_before_disabled_delivery() { + let mut server = test_headless_server(); + server.app.state.toast_config.delivery = crate::config::ToastDelivery::Off; + + let (respond_to, response_rx) = std::sync::mpsc::channel(); + let changed = server.handle_api_request_with_shutdown_check(api::ApiRequestMessage { + request: api::schema::Request { + id: "notify".into(), + method: api::schema::Method::NotificationShow( + api::schema::NotificationShowParams { + title: "\n\t".into(), + body: None, + position: None, + sound: api::schema::NotificationShowSound::None, + }, + ), + }, + respond_to, + }); + + assert!(changed); + let response = response_rx + .recv_timeout(Duration::from_millis(100)) + .unwrap(); + let parsed: api::schema::ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(parsed.error.code, "invalid_params"); + assert_eq!(parsed.error.message, "notification title is empty"); + } + + #[test] + fn notification_show_api_reports_no_foreground_client() { + let mut server = test_headless_server(); + server.foreground_client_id = None; + server.app.state.toast_config.delivery = crate::config::ToastDelivery::System; + + let (respond_to, response_rx) = std::sync::mpsc::channel(); + let changed = server.handle_api_request_with_shutdown_check(api::ApiRequestMessage { + request: api::schema::Request { + id: "notify".into(), + method: api::schema::Method::NotificationShow( + api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: api::schema::NotificationShowSound::Request, + }, + ), + }, + respond_to, + }); + + assert!(changed); + let response = response_rx + .recv_timeout(Duration::from_millis(100)) + .unwrap(); + let parsed: api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + api::schema::ResponseResult::NotificationShow { + shown: false, + reason: api::schema::NotificationShowReason::NoForegroundClient, + } + ); + } + + #[test] + fn notification_show_api_herdr_toast_expires_headless() { + let mut server = test_headless_server(); + server.app.state.toast_config.delivery = crate::config::ToastDelivery::Herdr; + + let (respond_to, response_rx) = std::sync::mpsc::channel(); + assert!( + server.handle_api_request_with_shutdown_check(api::ApiRequestMessage { + request: api::schema::Request { + id: "notify".into(), + method: api::schema::Method::NotificationShow( + api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: api::schema::NotificationShowSound::None, + }, + ), + }, + respond_to, + }) + ); + + let response = response_rx + .recv_timeout(Duration::from_millis(100)) + .unwrap(); + let parsed: api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + api::schema::ResponseResult::NotificationShow { + shown: true, + reason: api::schema::NotificationShowReason::Shown, + } + ); + let deadline = server.app.toast_deadline.expect("api toast deadline"); + assert!(server.handle_scheduled_tasks_headless(deadline, false)); + assert!(server.app.state.toast.is_none()); + assert!(server.app.toast_deadline.is_none()); + } + + #[test] + fn notification_show_api_forwards_sound_for_herdr_delivery() { + 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::Herdr; + + let (respond_to, response_rx) = std::sync::mpsc::channel(); + assert!( + server.handle_api_request_with_shutdown_check(api::ApiRequestMessage { + request: api::schema::Request { + id: "notify".into(), + method: api::schema::Method::NotificationShow( + api::schema::NotificationShowParams { + title: "build failed".into(), + body: None, + position: None, + sound: api::schema::NotificationShowSound::Done, + }, + ), + }, + respond_to, + }) + ); + + let response = response_rx + .recv_timeout(Duration::from_millis(100)) + .unwrap(); + let parsed: api::schema::SuccessResponse = serde_json::from_str(&response).unwrap(); + assert_eq!( + parsed.result, + api::schema::ResponseResult::NotificationShow { + shown: true, + reason: api::schema::NotificationShowReason::Shown, + } + ); + match read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("api sound message"), + ) { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::Sound); + assert_eq!(message, "agent done"); + assert!(body.is_none()); + } + other => panic!("expected api sound, got {other:?}"), + } + } + + #[test] + fn delayed_agent_notification_forwards_after_deadline() { + let mut server = test_headless_server(); + let background = crate::workspace::Workspace::test_new("background"); + let pane_id = background.tabs[0].root_pane; + let foreground = crate::workspace::Workspace::test_new("foreground"); + server.app.state.workspaces = vec![background, foreground]; + server.app.state.ensure_test_terminals(); + server.app.state.active = Some(1); + server.app.state.selected = 1; + server.app.state.mode = crate::app::Mode::Terminal; + server.app.state.toast_config.delivery = crate::config::ToastDelivery::System; + server.app.state.toast_config.delay_seconds = 1; + + 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.sync_foreground_client_state(); + + let changed = server.handle_internal_event_with_forwarding(AppEvent::StateChanged { + pane_id, + agent: Some(crate::detect::Agent::Pi), + state: crate::detect::AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: Instant::now(), + }); + + assert!(changed); + assert!(server.app.state.toast.is_none()); + assert!( + client_control_rx + .recv_timeout(Duration::from_millis(50)) + .is_err(), + "delayed transition should not notify immediately" + ); + + let deadline = server + .app + .state + .next_pending_agent_notification_deadline() + .expect("pending notification deadline"); + assert!(server.handle_scheduled_tasks_headless(deadline, false)); + + let first = read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("delayed sound message"), + ); + let second = read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("delayed toast message"), + ); + + assert!(matches!( + first, + ServerMessage::Notify { + kind: protocol::NotifyKind::Sound, + .. + } + )); + match second { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::SystemToast); + assert_eq!(message, "pi needs attention"); + assert_eq!(body.as_deref(), Some("background · 1")); + } + other => panic!("expected delayed system toast, got {other:?}"), + } + assert!(server.app.state.pending_agent_notifications.is_empty()); + } + + #[test] + fn delayed_active_tab_unfocused_agent_notification_forwards_after_deadline() { + let mut server = test_headless_server(); + let workspace = crate::workspace::Workspace::test_new("active"); + let pane_id = workspace.tabs[0].root_pane; + server.app.state.workspaces = vec![workspace]; + server.app.state.ensure_test_terminals(); + server.app.state.active = Some(0); + server.app.state.selected = 0; + server.app.state.mode = crate::app::Mode::Terminal; + server.app.state.toast_config.delivery = crate::config::ToastDelivery::System; + server.app.state.toast_config.delay_seconds = 1; + + 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(), + Some(false), + 1, + RenderEncoding::SemanticFrame, + Some(client_tx), + ), + ); + server.foreground_client_id = Some(1); + server.sync_foreground_client_state(); + + assert!( + server.handle_internal_event_with_forwarding(AppEvent::StateChanged { + pane_id, + agent: Some(crate::detect::Agent::Pi), + state: crate::detect::AgentState::Blocked, + visible_blocker: false, + visible_idle: false, + visible_working: false, + process_exited: false, + observed_at: Instant::now(), + }) + ); + assert!(server.app.state.toast.is_none()); + assert!( + client_control_rx + .recv_timeout(Duration::from_millis(50)) + .is_err(), + "delayed transition should not notify immediately" + ); + + let deadline = server + .app + .state + .next_pending_agent_notification_deadline() + .expect("pending notification deadline"); + assert!(server.handle_scheduled_tasks_headless(deadline, false)); + + let first = read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("delayed sound message"), + ); + let second = read_server_message( + client_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("delayed toast message"), + ); + + assert!(matches!( + first, + ServerMessage::Notify { + kind: protocol::NotifyKind::Sound, + .. + } + )); + match second { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::SystemToast); + assert_eq!(message, "pi needs attention"); + assert_eq!(body.as_deref(), Some("active · 1")); + } + other => panic!("expected delayed system toast, got {other:?}"), + } + } + #[test] fn stale_api_agent_report_does_not_forward_done_sound() { let mut server = test_headless_server(); diff --git a/src/ui.rs b/src/ui.rs index bbdc0b04..bdae81c9 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -51,7 +51,7 @@ pub(crate) use self::scrollbar::{ use self::settings::render_settings_overlay; use self::sidebar::{render_sidebar, render_sidebar_collapsed}; use self::status::{ - render_config_diagnostic, render_copy_feedback, render_toast_notification, + copy_feedback_rect, render_config_diagnostic, render_copy_feedback, render_toast_notification, toast_notification_rect, }; use self::tabs::render_tab_bar; @@ -254,7 +254,14 @@ fn compute_view_internal( let toast_hit_area = app .toast .as_ref() - .map(|toast| toast_notification_rect(terminal_area, toast, app.config_diagnostic.is_some())) + .map(|toast| { + toast_notification_rect( + area, + toast, + app.config_diagnostic.is_some(), + toast.position.unwrap_or(app.toast_config.herdr.position), + ) + }) .unwrap_or_default(); app.view = crate::app::ViewState { @@ -412,6 +419,7 @@ fn render_notifications(app: &AppState, frame: &mut Frame, terminal_area: Rect) render_config_diagnostic(frame, terminal_area, message, &app.palette); } let mut copy_feedback_offset = u16::from(has_config_diagnostic); + let mut toast_rect = None; if let Some(toast) = &app.toast { if app.view.layout == ViewLayout::Mobile { render_mobile_toast_banner( @@ -424,18 +432,25 @@ fn render_notifications(app: &AppState, frame: &mut Frame, terminal_area: Rect) } else { render_toast_notification( frame, - terminal_area, + frame.area(), toast, has_config_diagnostic, + toast.position.unwrap_or(app.toast_config.herdr.position), &app.palette, ); + toast_rect = Some(toast_notification_rect( + frame.area(), + toast, + has_config_diagnostic, + toast.position.unwrap_or(app.toast_config.herdr.position), + )); + } + if app.view.layout == ViewLayout::Mobile { + toast_rect = Some(mobile_toast_banner_rect( + frame.area(), + has_config_diagnostic, + )); } - copy_feedback_offset = - copy_feedback_offset.saturating_add(if app.view.layout == ViewLayout::Mobile { - 1 - } else { - toast_notification_rect(terminal_area, toast, has_config_diagnostic).height - }); } if let Some(feedback) = &app.copy_feedback { let area = if app.view.layout == ViewLayout::Mobile { @@ -443,10 +458,48 @@ fn render_notifications(app: &AppState, frame: &mut Frame, terminal_area: Rect) } else { terminal_area }; - render_copy_feedback(frame, area, feedback, copy_feedback_offset, &app.palette); + if let Some(toast_rect) = toast_rect { + copy_feedback_offset = copy_feedback_offset_for_toast( + area, + feedback, + copy_feedback_offset, + app.toast_config.clipboard.position, + toast_rect, + ); + } + render_copy_feedback( + frame, + area, + feedback, + copy_feedback_offset, + app.toast_config.clipboard.position, + &app.palette, + ); } } +fn copy_feedback_offset_for_toast( + area: Rect, + feedback: &crate::app::state::CopyFeedback, + base_offset: u16, + position: crate::config::ToastClipboardPosition, + toast_rect: Rect, +) -> u16 { + let feedback_rect = copy_feedback_rect(area, feedback, base_offset, position); + if rects_overlap(feedback_rect, toast_rect) { + base_offset.saturating_add(toast_rect.height) + } else { + base_offset + } +} + +fn rects_overlap(a: Rect, b: Rect) -> bool { + a.x < b.x.saturating_add(b.width) + && b.x < a.x.saturating_add(a.width) + && a.y < b.y.saturating_add(b.height) + && b.y < a.y.saturating_add(a.height) +} + fn dim_background(frame: &mut Frame, area: Rect) { let buf = frame.buffer_mut(); for y in area.y..area.y + area.height { @@ -480,6 +533,50 @@ mod tests { use ratatui::style::Color; use ratatui::{backend::TestBackend, Terminal}; + #[test] + fn copy_feedback_offset_only_increases_when_toast_rect_overlaps() { + let area = Rect::new(0, 0, 80, 24); + let feedback = crate::app::state::CopyFeedback { + message: "copied to clipboard".into(), + }; + let toast = crate::app::state::ToastNotification { + kind: crate::app::state::ToastKind::Finished, + title: "pi finished".into(), + context: "workspace · 1".into(), + position: None, + target: None, + }; + + let bottom_right_toast = toast_notification_rect( + area, + &toast, + false, + crate::config::ToastHerdrPosition::BottomRight, + ); + assert_eq!( + copy_feedback_offset_for_toast( + area, + &feedback, + 0, + crate::config::ToastClipboardPosition::TopCenter, + bottom_right_toast, + ), + 0 + ); + + let bottom_center_toast = Rect::new(28, 21, 24, 3); + assert_eq!( + copy_feedback_offset_for_toast( + area, + &feedback, + 0, + crate::config::ToastClipboardPosition::BottomCenter, + bottom_center_toast, + ), + bottom_center_toast.height + ); + } + #[tokio::test] async fn focused_pane_cursor_wins_during_terminal_render() { let mut app = crate::app::state::AppState::test_new(); @@ -541,6 +638,53 @@ mod tests { ); } + #[test] + fn desktop_toast_hit_area_uses_full_frame_not_terminal_area() { + let mut app = crate::app::state::AppState::test_new(); + app.workspaces = vec![Workspace::test_new("one")]; + app.active = Some(0); + app.selected = 0; + app.mode = Mode::Terminal; + app.toast_config.herdr.position = crate::config::ToastHerdrPosition::TopLeft; + app.toast = Some(crate::app::state::ToastNotification { + kind: crate::app::state::ToastKind::Finished, + title: "pi finished".into(), + context: "one".into(), + position: None, + target: None, + }); + + compute_view(&mut app, Rect::new(0, 0, 100, 20)); + + assert_eq!(app.view.layout, ViewLayout::Desktop); + assert!(app.view.terminal_area.x > 0); + assert_eq!(app.view.toast_hit_area.x, 0); + assert_eq!(app.view.toast_hit_area.y, 0); + } + + #[test] + fn desktop_toast_hit_area_still_offsets_for_config_diagnostic() { + let mut app = crate::app::state::AppState::test_new(); + app.workspaces = vec![Workspace::test_new("one")]; + app.active = Some(0); + app.selected = 0; + app.mode = Mode::Terminal; + app.config_diagnostic = Some("config warning".into()); + app.toast_config.herdr.position = crate::config::ToastHerdrPosition::TopLeft; + app.toast = Some(crate::app::state::ToastNotification { + kind: crate::app::state::ToastKind::Finished, + title: "pi finished".into(), + context: "one".into(), + position: None, + target: None, + }); + + compute_view(&mut app, Rect::new(0, 0, 100, 20)); + + assert_eq!(app.view.toast_hit_area.x, 0); + assert_eq!(app.view.toast_hit_area.y, 1); + } + #[test] fn configured_mobile_width_threshold_controls_layout_switch() { let mut app = crate::app::state::AppState::test_new(); diff --git a/src/ui/status.rs b/src/ui/status.rs index 8ec4f392..a3aa78fa 100644 --- a/src/ui/status.rs +++ b/src/ui/status.rs @@ -9,10 +9,16 @@ use ratatui::{ use super::widgets::panel_contrast_fg; use crate::{ app::state::{CopyFeedback, Palette, ToastKind, ToastNotification}, + config::{ToastClipboardPosition, ToastHerdrPosition}, detect::AgentState, }; -pub(crate) fn copy_feedback_rect(area: Rect, feedback: &CopyFeedback, offset_rows: u16) -> Rect { +pub(crate) fn copy_feedback_rect( + area: Rect, + feedback: &CopyFeedback, + offset_rows: u16, + position: ToastClipboardPosition, +) -> Rect { if area.width == 0 || area.height == 0 { return Rect::default(); } @@ -20,8 +26,25 @@ pub(crate) fn copy_feedback_rect(area: Rect, feedback: &CopyFeedback, offset_row let content_width = feedback.message.len() as u16 + 4; let width = content_width.min(area.width); let height = 3u16.min(area.height); - let x = area.x + area.width.saturating_sub(width) / 2; - let y = area.y + area.height.saturating_sub(height + offset_rows); + let x = match position { + ToastClipboardPosition::TopLeft | ToastClipboardPosition::BottomLeft => area.x, + ToastClipboardPosition::TopCenter | ToastClipboardPosition::BottomCenter => { + area.x + area.width.saturating_sub(width) / 2 + } + ToastClipboardPosition::TopRight | ToastClipboardPosition::BottomRight => { + area.x + area.width.saturating_sub(width) + } + }; + let y = match position { + ToastClipboardPosition::TopLeft + | ToastClipboardPosition::TopCenter + | ToastClipboardPosition::TopRight => area.y + offset_rows.min(area.height), + ToastClipboardPosition::BottomLeft + | ToastClipboardPosition::BottomCenter + | ToastClipboardPosition::BottomRight => { + area.y + area.height.saturating_sub(height + offset_rows) + } + }; Rect::new(x, y, width, height) } @@ -29,16 +52,27 @@ pub(crate) fn toast_notification_rect( area: Rect, toast: &ToastNotification, offset_for_warning: bool, + position: ToastHerdrPosition, ) -> Rect { let content_width = (toast.title.len().max(toast.context.len()) as u16) + 4; let width = content_width.saturating_add(2).min(area.width); let content_height = if toast.context.is_empty() { 1 } else { 2 }; let height = (content_height + 2).min(area.height); - let x = area.x + area.width.saturating_sub(width); - let y = area.y - + area - .height - .saturating_sub(height + if offset_for_warning { 1 } else { 0 }); + let x = match position { + ToastHerdrPosition::TopLeft | ToastHerdrPosition::BottomLeft => area.x, + ToastHerdrPosition::TopRight | ToastHerdrPosition::BottomRight => { + area.x + area.width.saturating_sub(width) + } + }; + let warning_offset = u16::from(offset_for_warning); + let y = match position { + ToastHerdrPosition::TopLeft | ToastHerdrPosition::TopRight => { + area.y + warning_offset.min(area.height) + } + ToastHerdrPosition::BottomLeft | ToastHerdrPosition::BottomRight => { + area.y + area.height.saturating_sub(height + warning_offset) + } + }; Rect::new(x, y, width, height) } @@ -47,6 +81,7 @@ pub(super) fn render_toast_notification( area: Rect, toast: &ToastNotification, offset_for_warning: bool, + position: ToastHerdrPosition, p: &Palette, ) { let dot_color = match toast.kind { @@ -54,7 +89,7 @@ pub(super) fn render_toast_notification( ToastKind::Finished => p.blue, ToastKind::UpdateInstalled => p.accent, }; - let toast_area = toast_notification_rect(area, toast, offset_for_warning); + let toast_area = toast_notification_rect(area, toast, offset_for_warning, position); frame.render_widget(Clear, toast_area); let block = Block::default() @@ -95,9 +130,10 @@ pub(super) fn render_copy_feedback( area: Rect, feedback: &CopyFeedback, offset_rows: u16, + position: ToastClipboardPosition, p: &Palette, ) { - let feedback_area = copy_feedback_rect(area, feedback, offset_rows); + let feedback_area = copy_feedback_rect(area, feedback, offset_rows, position); if feedback_area.is_empty() { return; } @@ -198,3 +234,70 @@ pub(super) fn state_label_color(state: AgentState, seen: bool, p: &Palette) -> C (AgentState::Unknown, _) => p.overlay0, } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{ToastClipboardPosition, ToastHerdrPosition}; + + fn toast() -> ToastNotification { + ToastNotification { + kind: ToastKind::Finished, + title: "done".to_string(), + context: "workspace".to_string(), + position: None, + target: None, + } + } + + fn feedback() -> CopyFeedback { + CopyFeedback { + message: "copied to clipboard".to_string(), + } + } + + #[test] + fn toast_rect_uses_configured_corner() { + let area = Rect::new(10, 20, 100, 40); + let toast = toast(); + + let top_left = toast_notification_rect(area, &toast, false, ToastHerdrPosition::TopLeft); + assert_eq!(top_left.x, area.x); + assert_eq!(top_left.y, area.y); + + let top_right = toast_notification_rect(area, &toast, false, ToastHerdrPosition::TopRight); + assert_eq!(top_right.x + top_right.width, area.x + area.width); + assert_eq!(top_right.y, area.y); + + let bottom_left = + toast_notification_rect(area, &toast, false, ToastHerdrPosition::BottomLeft); + assert_eq!(bottom_left.x, area.x); + assert_eq!(bottom_left.y + bottom_left.height, area.y + area.height); + + let bottom_right = + toast_notification_rect(area, &toast, false, ToastHerdrPosition::BottomRight); + assert_eq!(bottom_right.x + bottom_right.width, area.x + area.width); + assert_eq!(bottom_right.y + bottom_right.height, area.y + area.height); + } + + #[test] + fn copy_feedback_rect_uses_configured_position() { + let area = Rect::new(10, 20, 100, 40); + let feedback = feedback(); + + let top_center = copy_feedback_rect(area, &feedback, 0, ToastClipboardPosition::TopCenter); + assert_eq!(top_center.y, area.y); + assert_eq!( + top_center.x, + area.x + area.width.saturating_sub(top_center.width) / 2 + ); + + let bottom_center = + copy_feedback_rect(area, &feedback, 0, ToastClipboardPosition::BottomCenter); + assert_eq!(bottom_center.y + bottom_center.height, area.y + area.height); + assert_eq!( + bottom_center.x, + area.x + area.width.saturating_sub(bottom_center.width) / 2 + ); + } +} diff --git a/tests/api_ping.rs b/tests/api_ping.rs index b3fc4632..32759104 100644 --- a/tests/api_ping.rs +++ b/tests/api_ping.rs @@ -273,7 +273,7 @@ fn ping_over_socket_returns_version() { assert_eq!(value["result"]["version"], env!("CARGO_PKG_VERSION")); // Intentionally hardcoded so wire protocol bumps require updating this test. // Changing this value means old clients/servers are no longer compatible. - assert_eq!(value["result"]["protocol"], 12); + assert_eq!(value["result"]["protocol"], 13); cleanup_spawned_herdr(child, base); } diff --git a/tests/cli_wrapper.rs b/tests/cli_wrapper.rs index bbe053a7..5f5749cb 100644 --- a/tests/cli_wrapper.rs +++ b/tests/cli_wrapper.rs @@ -1398,7 +1398,7 @@ fn status_commands_report_client_and_server_versions() { "stdout: {full_stdout}" ); assert!( - full_stdout.contains(" protocol: 12"), + full_stdout.contains(" protocol: 13"), "stdout: {full_stdout}" ); assert!(full_stdout.contains("server:\n"), "stdout: {full_stdout}"); @@ -1431,7 +1431,7 @@ fn status_commands_report_client_and_server_versions() { "stdout: {server_stdout}" ); assert!( - server_stdout.contains("protocol: 12"), + server_stdout.contains("protocol: 13"), "stdout: {server_stdout}" ); @@ -1443,7 +1443,7 @@ fn status_commands_report_client_and_server_versions() { "stdout: {client_stdout}" ); assert!( - client_stdout.contains("protocol: 12"), + client_stdout.contains("protocol: 13"), "stdout: {client_stdout}" ); assert!( @@ -1453,7 +1453,7 @@ fn status_commands_report_client_and_server_versions() { let full_json = run_cli_json(&socket_path, &["status", "--json"]); assert_eq!(full_json["client"]["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(full_json["client"]["protocol"], 12); + assert_eq!(full_json["client"]["protocol"], 13); assert_eq!(full_json["server"]["status"], "running"); assert_eq!(full_json["server"]["running"], true); assert_eq!(full_json["server"]["compatible"], true); @@ -1467,12 +1467,12 @@ fn status_commands_report_client_and_server_versions() { let server_json = run_cli_json(&socket_path, &["status", "server", "--json"]); assert_eq!(server_json["status"], "running"); assert_eq!(server_json["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(server_json["protocol"], 12); + assert_eq!(server_json["protocol"], 13); assert_eq!(server_json["compatible"], true); let client_json = run_cli_json(&socket_path, &["status", "client", "--json"]); assert_eq!(client_json["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(client_json["protocol"], 12); + assert_eq!(client_json["protocol"], 13); assert!(client_json["binary"] .as_str() .is_some_and(|path| !path.is_empty())); diff --git a/tests/client_mode.rs b/tests/client_mode.rs index 13dca5ef..99c6e8ad 100644 --- a/tests/client_mode.rs +++ b/tests/client_mode.rs @@ -274,8 +274,8 @@ fn client_connects_and_receives_frame() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12, "server should report protocol version 12"); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13, "server should report protocol version 13"); assert!( error.is_none(), "handshake should not have error: {:?}", @@ -342,8 +342,8 @@ fn client_sees_headless_startup_config_diagnostic() { let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); stream @@ -391,8 +391,8 @@ fn client_input_forwarded_to_pane() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Send an Input message containing "echo hello\n". @@ -445,8 +445,8 @@ fn client_resize_sends_message() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain the initial frame(s). @@ -504,8 +504,8 @@ fn server_shutdown_sends_message_to_client() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Send SIGINT so the server takes the graceful shutdown path and @@ -736,8 +736,8 @@ fn client_receives_frame_after_pane_output() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); read_next_frame_payload(&mut stream, Duration::from_secs(10)) @@ -783,8 +783,8 @@ fn navigate_mode_keybind_dispatch_in_server() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frames. @@ -901,8 +901,8 @@ fn graceful_shutdown_sends_server_shutdown_to_client() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frame(s). @@ -1000,8 +1000,8 @@ fn client_receives_notify_on_agent_state_change() { // Connect as a client and perform handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frame(s). diff --git a/tests/cross_area.rs b/tests/cross_area.rs index 38c22192..fcac6f81 100644 --- a/tests/cross_area.rs +++ b/tests/cross_area.rs @@ -687,7 +687,7 @@ fn cross_area_detach_and_reattach_preserves_state() { // Local attach (client A). let mut client_a = UnixStream::connect(&client_socket).expect("client A should connect"); - client_handshake(&mut client_a, 12, 100, 30); + client_handshake(&mut client_a, 13, 100, 30); assert!(wait_for_frame(&mut client_a, Duration::from_secs(2))); // Use herdr: create a workspace and write output into its pane. @@ -724,7 +724,7 @@ fn cross_area_detach_and_reattach_preserves_state() { // Reattach from another terminal/session (client B). let mut client_b = UnixStream::connect(&client_socket).expect("client B should connect"); - client_handshake(&mut client_b, 12, 80, 24); + client_handshake(&mut client_b, 13, 80, 24); assert!( wait_for_frame(&mut client_b, Duration::from_secs(5)), "reattached client should receive frame" @@ -780,7 +780,7 @@ fn cross_area_agent_process_survives_detach_and_reattach() { wait_for_socket(&client_socket, Duration::from_secs(10)); let mut client_a = UnixStream::connect(&client_socket).expect("client A should connect"); - client_handshake(&mut client_a, 12, 100, 30); + client_handshake(&mut client_a, 13, 100, 30); assert!(wait_for_frame(&mut client_a, Duration::from_secs(2))); let created = workspace_create(&api_socket, "agent-persist"); @@ -833,7 +833,7 @@ fn cross_area_agent_process_survives_detach_and_reattach() { // Reattach and ensure client-side state reflects the persisted working status. let mut client_b = UnixStream::connect(&client_socket).expect("client B should connect"); - client_handshake(&mut client_b, 12, 80, 24); + client_handshake(&mut client_b, 13, 80, 24); let saw_working_on_client = wait_for_frame_matching(&mut client_b, Duration::from_secs(5), |frame| { frame_contains_text(frame, "working") @@ -878,7 +878,7 @@ fn cross_area_client_and_api_workspace_views_are_consistent() { wait_for_socket(&client_socket, Duration::from_secs(10)); let mut client = UnixStream::connect(&client_socket).expect("client should connect"); - client_handshake(&mut client, 12, 100, 30); + client_handshake(&mut client, 13, 100, 30); assert!(wait_for_frame(&mut client, Duration::from_secs(2))); drain_server_messages(&mut client, Duration::from_millis(300)); @@ -941,9 +941,9 @@ fn cross_area_two_clients_shared_view_and_single_detach_stability() { wait_for_socket(&client_socket, Duration::from_secs(10)); let mut client_a = UnixStream::connect(&client_socket).expect("client A should connect"); - client_handshake(&mut client_a, 12, 110, 30); + client_handshake(&mut client_a, 13, 110, 30); let mut client_b = UnixStream::connect(&client_socket).expect("client B should connect"); - client_handshake(&mut client_b, 12, 100, 30); + client_handshake(&mut client_b, 13, 100, 30); assert!(wait_for_frame(&mut client_a, Duration::from_secs(2))); assert!(wait_for_frame(&mut client_b, Duration::from_secs(2))); @@ -1112,7 +1112,7 @@ fn cross_area_server_kill_then_restart_and_reconnect() { let mut reconnect_client = UnixStream::connect(&client_socket).expect("new client should connect after restart"); - client_handshake(&mut reconnect_client, 12, 80, 24); + client_handshake(&mut reconnect_client, 13, 80, 24); assert!( wait_for_frame(&mut reconnect_client, Duration::from_secs(5)), "new client should receive frame after restart" diff --git a/tests/detach_reattach.rs b/tests/detach_reattach.rs index 18b0e6fc..d8313914 100644 --- a/tests/detach_reattach.rs +++ b/tests/detach_reattach.rs @@ -276,8 +276,8 @@ fn navigate_q_detaches_client_and_server_persists() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frames. @@ -338,8 +338,8 @@ fn explicit_detach_message_causes_clean_disconnect() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frames. @@ -397,8 +397,8 @@ fn reattach_after_detach_shows_current_state() { // --- Client A --- let mut stream_a = UnixStream::connect(&client_socket).expect("client A should connect"); let (version, error) = - client_handshake(&mut stream_a, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream_a, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frames. @@ -436,8 +436,8 @@ fn reattach_after_detach_shows_current_state() { // --- Client B (reattach) --- let mut stream_b = UnixStream::connect(&client_socket).expect("client B should connect"); let (version, error) = - client_handshake(&mut stream_b, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream_b, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!( error.is_none(), "reattach handshake should succeed: {:?}", @@ -516,8 +516,8 @@ fn processes_survive_during_and_after_detach() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frames. @@ -555,8 +555,8 @@ fn processes_survive_during_and_after_detach() { // Reattach — verify we can connect and receive a frame. let mut stream_b = UnixStream::connect(&client_socket).expect("should reattach"); let (version, error) = - client_handshake(&mut stream_b, 12, 80, 24).expect("reattach handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream_b, 13, 80, 24).expect("reattach handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Verify the reattached client receives a frame. @@ -604,8 +604,8 @@ fn server_persists_after_client_connection_drop() { // Connect and handshake. let mut stream = UnixStream::connect(&client_socket).expect("should connect"); let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Drain initial frames. @@ -631,8 +631,8 @@ fn server_persists_after_client_connection_drop() { // Reattach — verify we can connect and handshake again. let mut stream_b = UnixStream::connect(&client_socket).expect("should reattach"); let (version, error) = - client_handshake(&mut stream_b, 12, 80, 24).expect("reattach handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream_b, 13, 80, 24).expect("reattach handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "reattach should succeed: {:?}", error); cleanup_spawned_herdr(spawned, base); @@ -653,8 +653,8 @@ fn detached_output_preserves_last_attached_pty_size() { let mut stream = UnixStream::connect(&client_socket).expect("client should connect"); let (version, error) = - client_handshake(&mut stream, 12, 120, 40).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream, 13, 120, 40).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); drain_messages(&mut stream); @@ -722,8 +722,8 @@ fn output_accumulated_while_detached_visible_on_reattach() { // Connect and handshake client A. let mut stream_a = UnixStream::connect(&client_socket).expect("client A should connect"); let (version, error) = - client_handshake(&mut stream_a, 12, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream_a, 13, 80, 24).expect("handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Detach client A immediately. @@ -780,8 +780,8 @@ fn output_accumulated_while_detached_visible_on_reattach() { // --- Client B (reattach) --- let mut stream_b = UnixStream::connect(&client_socket).expect("client B should connect"); let (version, error) = - client_handshake(&mut stream_b, 12, 80, 24).expect("reattach handshake should succeed"); - assert_eq!(version, 12); + client_handshake(&mut stream_b, 13, 80, 24).expect("reattach handshake should succeed"); + assert_eq!(version, 13); assert!(error.is_none(), "{:?}", error); // Client B should receive a frame with the current state. diff --git a/tests/multi_client.rs b/tests/multi_client.rs index 57e689c2..aedb01dc 100644 --- a/tests/multi_client.rs +++ b/tests/multi_client.rs @@ -566,7 +566,7 @@ fn client_handshake( fn connect_raw_client(client_socket: &Path, cols: u16, rows: u16) -> UnixStream { let mut stream = UnixStream::connect(client_socket).expect("should connect to client socket"); - client_handshake(&mut stream, 12, cols, rows).expect("handshake should succeed"); + client_handshake(&mut stream, 13, cols, rows).expect("handshake should succeed"); stream } diff --git a/tests/server_headless.rs b/tests/server_headless.rs index 08d4f5a5..84a0c29e 100644 --- a/tests/server_headless.rs +++ b/tests/server_headless.rs @@ -595,9 +595,9 @@ fn client_handshake_succeeds() { // Send Hello with the current protocol version, 80 cols, 24 rows. let (version, error) = - client_handshake(&mut stream, 12, 80, 24).expect("handshake should succeed"); + client_handshake(&mut stream, 13, 80, 24).expect("handshake should succeed"); - assert_eq!(version, 12, "server should report protocol version 12"); + assert_eq!(version, 13, "server should report protocol version 13"); assert!( error.is_none(), "handshake should not have an error: {:?}", @@ -626,7 +626,7 @@ fn client_handshake_rejects_incompatible_version() { let (version, error) = client_handshake(&mut stream, 0, 80, 24) .expect("should read Welcome response even on rejection"); - assert_eq!(version, 12, "server should report its version 12"); + assert_eq!(version, 13, "server should report its version 13"); assert!( error.is_some(), "version 0 should be rejected with an error" @@ -651,10 +651,10 @@ fn client_handshake_clamps_small_terminal_size() { // Send Hello with 0x0 terminal size — should be clamped. let mut stream = UnixStream::connect(&client_socket).expect("should connect to client socket"); - let (version, error) = client_handshake(&mut stream, 12, 0, 0) + let (version, error) = client_handshake(&mut stream, 13, 0, 0) .expect("handshake with 0x0 should succeed (server clamps)"); - assert_eq!(version, 12); + assert_eq!(version, 13); assert!( error.is_none(), "0x0 size should be accepted (clamped): {:?}", @@ -714,9 +714,9 @@ fn no_hello_client_closed_within_five_seconds() { // Verify the server is still healthy — a proper client can still connect. let mut good_stream = UnixStream::connect(&client_socket).expect("should connect after no-hello client"); - let (version, error) = client_handshake(&mut good_stream, 12, 80, 24) + let (version, error) = client_handshake(&mut good_stream, 13, 80, 24) .expect("proper handshake should still work after no-hello client"); - assert_eq!(version, 12); + assert_eq!(version, 13); assert!(error.is_none()); // API should still work.