From cc88b3b8e5bb9f7d9f23ed6ae85a52fd7b5b9ed6 Mon Sep 17 00:00:00 2001 From: Can Celik Date: Wed, 2 Sep 2026 02:45:20 +0300 Subject: feat: add stable client endpoint compatibility (#3509) * feat: add stable client endpoint compatibility * fix: keep portable-pty patch checks composable * ci: run maintenance checks in pull requests * fix: reject endpoint errors from terminal clients * feat: keep compatible servers running across updates --- src/client/shell/actions.rs | 162 +++++++++++----- src/client/shell/composition.rs | 25 ++- src/client/shell/config.rs | 37 ++-- src/client/shell/copy_mode.rs | 4 +- src/client/shell/graphics.rs | 1 + src/client/shell/mouse.rs | 23 ++- src/client/shell/notifications.rs | 216 ++++++++++++++------- src/client/shell/settings.rs | 23 ++- src/client/shell/state.rs | 49 ++++- src/client/shell/surface_patch.rs | 2 + .../shell/tests/agents_worktrees_notifications.rs | 30 +++ src/client/shell/tests/keybindings_settings.rs | 147 +++++++++++++- src/client/shell/tests/startup_overlays.rs | 36 ++++ src/client/shell/worktrees.rs | 38 ++-- 14 files changed, 623 insertions(+), 170 deletions(-) (limited to 'src/client/shell') diff --git a/src/client/shell/actions.rs b/src/client/shell/actions.rs index efc9aae9..34c780da 100644 --- a/src/client/shell/actions.rs +++ b/src/client/shell/actions.rs @@ -227,11 +227,13 @@ impl ClientShellState { if action == crate::protocol::ClientShellCommandAction::Popup { self.popup_pending = true; self.popup_pending_deadline = None; - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::CommandInvoke(params), PendingEndpointKind::PopupCommand, outcome, - ); + ) { + self.popup_pending = false; + } } else { self.push_endpoint_method( crate::api::schema::Method::CommandInvoke(params), @@ -323,7 +325,7 @@ impl ClientShellState { self.word_selection_generation = self.word_selection_generation.saturating_add(1); let generation = self.word_selection_generation; self.pending_word_selection = Some(generation); - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::PaneSelectionRead( crate::api::schema::PaneSelectionReadParams { pane_id: hit.pane_id.clone(), @@ -345,7 +347,9 @@ impl ClientShellState { generation, }, outcome, - ); + ) { + self.pending_word_selection = None; + } } pub(super) fn push_endpoint_method( @@ -356,15 +360,65 @@ impl ClientShellState { self.push_endpoint_method_with_kind(method, PendingEndpointKind::Generic, outcome); } + fn push_endpoint_notice( + &mut self, + kind: ClientEndpointNoticeKind, + code: impl Into, + title: impl Into, + body: impl Into, + ) -> bool { + let key = ClientEndpointNoticeKey { + boot_id: self + .snapshot + .as_deref() + .map(|snapshot| snapshot.boot_id.clone()) + .unwrap_or_else(|| "disconnected".to_owned()), + kind, + code: code.into(), + }; + let body = body.into(); + if kind == ClientEndpointNoticeKind::Rejected { + if self + .visible_endpoint_notice + .as_ref() + .is_some_and(|notice| notice.key == key && notice.body == body) + { + return false; + } + } else if !self.endpoint_notice_seen.insert(key.clone()) { + return false; + } + self.visible_endpoint_notice = Some(ClientVisibleEndpointNotice { + key, + title: title.into(), + body, + deadline: std::time::Instant::now() + std::time::Duration::from_secs(8), + }); + true + } + pub(super) fn push_endpoint_method_with_kind( &mut self, method: crate::api::schema::Method, kind: PendingEndpointKind, outcome: &mut ClientShellInput, - ) { - let Some(snapshot) = self.snapshot.as_deref() else { - return; - }; + ) -> bool { + if self.snapshot.is_none() { + return false; + } + let method_name = crate::api::api_method_name(&method).to_owned(); + if !self.supports_endpoint_method(&method) { + outcome.repaint |= self.push_endpoint_notice( + ClientEndpointNoticeKind::Unsupported, + method_name.clone(), + "Action unavailable", + format!( + "This server does not support {method_name} yet. Update and restart it to enable this action." + ), + ); + return false; + } + let snapshot = self.snapshot.as_deref().expect("checked snapshot"); let confirmation_workspace_id = match &method { crate::api::schema::Method::TabClose(target) => snapshot .tabs @@ -385,6 +439,7 @@ impl ClientShellState { request_id.clone(), PendingEndpointRequest { boot_id: snapshot.boot_id.clone(), + method_name, confirmation_workspace_id, kind, }, @@ -396,14 +451,16 @@ impl ClientShellState { method, }), }); + true } pub(crate) fn receive_endpoint_error(&mut self, message: String) -> bool { - if self.endpoint_error.as_deref() == Some(message.as_str()) { - return false; - } - self.endpoint_error = Some(message); - true + self.push_endpoint_notice( + ClientEndpointNoticeKind::Rejected, + "paste_rejected", + "Paste rejected", + message, + ) } pub(crate) fn handle_endpoint_result( @@ -423,12 +480,49 @@ impl ClientShellState { { return (false, Vec::new()); } + if result.is_ok() { + let timeout_key = ClientEndpointNoticeKey { + boot_id: boot_id.to_owned(), + kind: ClientEndpointNoticeKind::Timeout, + code: pending.method_name.clone(), + }; + self.endpoint_notice_seen.remove(&timeout_key); + } + if let Err(error) = &result { + let code = error.code.as_deref().unwrap_or("invalid_response"); + if !matches!( + code, + "confirmation_required" | "stale_content" | "stale_target" + ) { + let (kind, notice_code, title, body) = match code { + "endpoint_timeout" => ( + ClientEndpointNoticeKind::Timeout, + pending.method_name.clone(), + "Server timed out", + format!("This server did not respond to {}.", pending.method_name), + ), + "server_unavailable" => ( + ClientEndpointNoticeKind::Unavailable, + "server".to_owned(), + "Server unavailable", + error.message.clone(), + ), + _ => ( + ClientEndpointNoticeKind::Rejected, + format!("{}:{code}", pending.method_name), + "Action rejected", + error.message.clone(), + ), + }; + self.push_endpoint_notice(kind, notice_code, title, body); + } + } match pending.kind { PendingEndpointKind::Generic => {} PendingEndpointKind::ProductAnnouncementDismiss { version, id } => { return match result { Ok(_) => (false, Vec::new()), - Err(error) => { + Err(_) => { let key = (version.clone(), id.clone()); if self.dismissed_product_announcement.as_ref() == Some(&key) { self.dismissed_product_announcement = None; @@ -447,7 +541,6 @@ impl ClientShellState { )); } } - self.endpoint_error = Some(error.message); (true, Vec::new()) } }; @@ -455,7 +548,7 @@ impl ClientShellState { PendingEndpointKind::ReleaseNotesDismiss => { return match result { Ok(_) => (false, Vec::new()), - Err(error) => { + Err(_) => { if self.overlay.is_none() { if let Some(notes) = self .snapshot @@ -467,7 +560,6 @@ impl ClientShellState { )); } } - self.endpoint_error = Some(error.message); (true, Vec::new()) } }; @@ -479,10 +571,9 @@ impl ClientShellState { Some(std::time::Instant::now() + std::time::Duration::from_secs(1)); (false, Vec::new()) } - Err(error) => { + Err(_) => { self.popup_pending = false; self.popup_pending_deadline = None; - self.endpoint_error = Some(error.message); (true, Vec::new()) } }; @@ -517,10 +608,7 @@ impl ClientShellState { Some("endpoint returned an unexpected selection result".to_owned()); (true, fallback()) } - Err(error) => { - self.endpoint_error = Some(error.message); - (true, fallback()) - } + Err(_) => (true, fallback()), }; } PendingEndpointKind::WordSelection { @@ -551,10 +639,7 @@ impl ClientShellState { ); return (true, Vec::new()); } - Err(error) => { - self.endpoint_error = Some(error.message); - return (true, Vec::new()); - } + Err(_) => return (true, Vec::new()), }; let Some((start_col, end_col)) = crate::app::actions::word_bounds_at_column(&row_text, col) @@ -640,10 +725,7 @@ impl ClientShellState { self.url_click_consumes_until_up = completed_before_release; (false, Vec::new()) } - Err(error) => { - self.endpoint_error = Some(error.message); - (true, replay_action(replay)) - } + Err(_) => (true, replay_action(replay)), }; } PendingEndpointKind::CopyMotion { @@ -673,10 +755,7 @@ impl ClientShellState { Some("endpoint returned an unexpected copy-motion result".to_owned()); (true, false) } - Err(error) => { - self.endpoint_error = Some(error.message); - (true, false) - } + Err(_) => (true, false), }; self.complete_copy_operation(session_generation, continue_queue, &mut outcome); return (repaint || outcome.repaint, outcome.actions); @@ -731,9 +810,8 @@ impl ClientShellState { Some("endpoint returned an unexpected copy-search result".to_owned()); (true, false) } - Err(error) => { + Err(_) => { self.cancel_deferred_copy_after_search(generation); - self.endpoint_error = Some(error.message); (true, false) } }; @@ -748,10 +826,7 @@ impl ClientShellState { Some("endpoint returned an unexpected config reload result".to_owned()); true } - Err(error) => { - self.endpoint_error = Some(error.message); - true - } + Err(_) => true, }; return (repaint, Vec::new()); } @@ -778,10 +853,7 @@ impl ClientShellState { } true } - Err(error) => { - self.endpoint_error = Some(error.message); - true - } + Err(_) => true, }; (repaint, Vec::new()) } diff --git a/src/client/shell/composition.rs b/src/client/shell/composition.rs index 6432f3e1..04ca995c 100644 --- a/src/client/shell/composition.rs +++ b/src/client/shell/composition.rs @@ -269,7 +269,10 @@ impl ClientShellState { restore_mode_bar(&mut frame, mode_bar, mode_bar_cells.as_deref()); self.hits.notification_toast = Rect::default(); let has_config_diagnostic = self.config_diagnostic.is_some(); - if has_config_diagnostic || self.visible_notification.is_some() { + if has_config_diagnostic + || self.visible_endpoint_notice.is_some() + || self.visible_notification.is_some() + { let cursor = frame.cursor.clone(); let mut composed = frame.to_ratatui_buffer()?; if let Some(diagnostic) = self.config_diagnostic.as_deref() { @@ -285,7 +288,25 @@ impl ClientShellState { &self.config.palette, ); } - if let Some(notification) = self.visible_notification.as_ref() { + if let Some(notice) = self.visible_endpoint_notice.as_ref() { + self.hits.notification_toast = if layout.mobile_header.is_empty() { + notifications::render_endpoint_notice( + &mut composed, + Rect::new(0, 0, cols, rows), + notice, + u16::from(has_config_diagnostic), + &self.config.palette, + ) + } else { + notifications::render_mobile_endpoint_notice_banner( + &mut composed, + Rect::new(0, 0, cols, rows), + notice, + has_config_diagnostic, + &self.config.palette, + ) + }; + } else if let Some(notification) = self.visible_notification.as_ref() { self.hits.notification_toast = if layout.mobile_header.is_empty() { notifications::render_visible_notification( &mut composed, diff --git a/src/client/shell/config.rs b/src/client/shell/config.rs index 0273cefb..1c2993b7 100644 --- a/src/client/shell/config.rs +++ b/src/client/shell/config.rs @@ -200,16 +200,8 @@ impl ClientShellConfig { }; config.keys.command = commands .iter() - .map(|command| crate::config::CommandKeybindConfig { - key: if command.binding_labels.len() == 1 { - crate::config::BindingConfig::One(command.binding_labels[0].clone()) - } else { - crate::config::BindingConfig::Many(command.binding_labels.clone()) - }, - // The client never executes this field; preserve the opaque endpoint ID - // through the shared config collision resolver. - command: command.command_id.clone(), - action_type: match command.action { + .filter_map(|command| { + let action_type = match command.action { crate::protocol::ClientShellCommandAction::Shell => { crate::config::CommandKeybindType::Shell } @@ -222,10 +214,22 @@ impl ClientShellConfig { crate::protocol::ClientShellCommandAction::PluginAction => { crate::config::CommandKeybindType::PluginAction } - }, - description: command.description.clone(), - width: None, - height: None, + crate::protocol::ClientShellCommandAction::Unknown => return None, + }; + Some(crate::config::CommandKeybindConfig { + key: if command.binding_labels.len() == 1 { + crate::config::BindingConfig::One(command.binding_labels[0].clone()) + } else { + crate::config::BindingConfig::Many(command.binding_labels.clone()) + }, + // The client never executes this field; preserve the opaque endpoint ID + // through the shared config collision resolver. + command: command.command_id.clone(), + action_type, + description: command.description.clone(), + width: None, + height: None, + }) }) .collect(); config @@ -236,6 +240,9 @@ impl ClientShellConfig { }; if self.keybinding_source == ClientShellKeybindingSource::Endpoint { for command in commands { + let Ok(action) = command.action.try_into() else { + continue; + }; keybinds .keybinds .custom_commands @@ -245,7 +252,7 @@ impl ClientShellConfig { )?, label: command.binding_label.clone(), command: command.command_id.clone(), - action: command.action.into(), + action, description: command.description.clone(), width: None, height: None, diff --git a/src/client/shell/copy_mode.rs b/src/client/shell/copy_mode.rs index c39a7507..32a17bf4 100644 --- a/src/client/shell/copy_mode.rs +++ b/src/client/shell/copy_mode.rs @@ -797,7 +797,9 @@ impl ClientShellState { } }; self.copy_operation_in_flight = true; - self.push_endpoint_method_with_kind(method, kind, outcome); + if !self.push_endpoint_method_with_kind(method, kind, outcome) { + self.copy_operation_in_flight = false; + } return; } } diff --git a/src/client/shell/graphics.rs b/src/client/shell/graphics.rs index 1fc23414..8a512d3a 100644 --- a/src/client/shell/graphics.rs +++ b/src/client/shell/graphics.rs @@ -39,6 +39,7 @@ impl ClientShellState { || self.mode != ClientShellMode::Terminal || self.endpoint_error.is_some() || self.config_diagnostic.is_some() + || self.visible_endpoint_notice.is_some() || self.visible_notification.is_some() || self.copy_feedback.is_some() || self diff --git a/src/client/shell/mouse.rs b/src/client/shell/mouse.rs index 437ec4ed..612319e2 100644 --- a/src/client/shell/mouse.rs +++ b/src/client/shell/mouse.rs @@ -78,14 +78,20 @@ impl ClientShellState { self.pane_scroll_targets .insert(pane_id.clone(), offset_from_bottom); self.pane_scroll_in_flight.insert(pane_id.clone(), serial); - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::PaneScroll(crate::api::schema::PaneScrollParams { pane_id: pane_id.clone(), offset_from_bottom: offset_from_bottom as u64, }), - PendingEndpointKind::PaneScroll { pane_id, serial }, + PendingEndpointKind::PaneScroll { + pane_id: pane_id.clone(), + serial, + }, outcome, - ); + ) { + self.pane_scroll_targets.remove(&pane_id); + self.pane_scroll_in_flight.remove(&pane_id); + } } pub(super) fn complete_pane_scroll( @@ -120,10 +126,9 @@ impl ClientShellState { Some("endpoint returned an unexpected pane-scroll result".to_owned()); true } - Err(error) => { + Err(_) => { self.pane_scroll_queued.remove(&pane_id); self.pane_scroll_targets.remove(&pane_id); - self.endpoint_error = Some(error.message); true } }; @@ -893,6 +898,14 @@ impl ClientShellState { return; } } + if self.visible_endpoint_notice.is_some() + && mouse.kind == MouseEventKind::Down(MouseButton::Left) + && super::contains(self.hits.notification_toast, point) + { + self.visible_endpoint_notice = None; + outcome.repaint = true; + return; + } if self.overlay.is_none() && self.mode == ClientShellMode::Terminal && self diff --git a/src/client/shell/notifications.rs b/src/client/shell/notifications.rs index 0310e57d..43fe566a 100644 --- a/src/client/shell/notifications.rs +++ b/src/client/shell/notifications.rs @@ -1,13 +1,16 @@ use super::*; use ratatui::{ + style::Color, text::{Line, Span}, widgets::{Block, Borders, Clear, Paragraph, Widget}, }; -pub(super) fn render_mobile_notification_banner( +fn render_mobile_notice_banner( buffer: &mut Buffer, area: Rect, - notification: &ClientVisibleNotification, + title: &str, + body: Option<&str>, + dot_color: Color, offset_for_warning: bool, palette: &Palette, ) -> Rect { @@ -23,6 +26,49 @@ pub(super) fn render_mobile_notification_banner( let background = palette.surface0; Clear.render(rect, buffer); buffer.set_style(rect, Style::default().bg(background)); + let mut x = rect.x; + for (text, style) in [ + (" ", Style::default().bg(background)), + ("●", Style::default().fg(dot_color).bg(background)), + (" ", Style::default().bg(background)), + ( + title, + Style::default() + .fg(palette.text) + .bg(background) + .add_modifier(Modifier::BOLD), + ), + ] { + x = super::render::put_segment(buffer, x, rect.y, rect.right(), text, style); + } + if let Some(body) = body.filter(|body| !body.is_empty()) { + x = super::render::put_segment( + buffer, + x, + rect.y, + rect.right(), + " · ", + Style::default().fg(palette.overlay0).bg(background), + ); + super::render::put_text( + buffer, + x, + rect.y, + rect.right().saturating_sub(x), + body, + Style::default().fg(palette.overlay0).bg(background), + ); + } + rect +} + +pub(super) fn render_mobile_notification_banner( + buffer: &mut Buffer, + area: Rect, + notification: &ClientVisibleNotification, + offset_for_warning: bool, + palette: &Palette, +) -> Rect { let event = ¬ification.event; let title = match event.kind { SemanticNotificationKind::NeedsAttention => event @@ -45,84 +91,55 @@ pub(super) fn render_mobile_notification_banner( palette.accent } }; - let mut x = rect.x; - x = super::render::put_segment( - buffer, - x, - rect.y, - rect.right(), - " ", - Style::default().bg(background), - ); - x = super::render::put_segment( - buffer, - x, - rect.y, - rect.right(), - "●", - Style::default().fg(dot_color).bg(background), - ); - x = super::render::put_segment( + render_mobile_notice_banner( buffer, - x, - rect.y, - rect.right(), - " ", - Style::default().bg(background), - ); - x = super::render::put_segment( - buffer, - x, - rect.y, - rect.right(), + area, &title, - Style::default() - .fg(palette.text) - .bg(background) - .add_modifier(Modifier::BOLD), - ); - if let Some(body) = event.body.as_deref().filter(|body| !body.is_empty()) { - x = super::render::put_segment( - buffer, - x, - rect.y, - rect.right(), - " · ", - Style::default().fg(palette.overlay0).bg(background), - ); - super::render::put_text( - buffer, - x, - rect.y, - rect.right().saturating_sub(x), - body, - Style::default().fg(palette.overlay0).bg(background), - ); - } - rect + event.body.as_deref(), + dot_color, + offset_for_warning, + palette, + ) } -pub(super) fn render_visible_notification( +pub(super) fn render_mobile_endpoint_notice_banner( buffer: &mut Buffer, area: Rect, - notification: &ClientVisibleNotification, - default_position: crate::config::ToastHerdrPosition, + notice: &ClientVisibleEndpointNotice, + offset_for_warning: bool, + palette: &Palette, +) -> Rect { + render_mobile_notice_banner( + buffer, + area, + ¬ice.title, + Some(¬ice.body), + palette.red, + offset_for_warning, + palette, + ) +} + +fn render_notification_card( + buffer: &mut Buffer, + area: Rect, + title: &str, + body: &str, + position: crate::config::ToastHerdrPosition, top_offset: u16, + dot_color: Color, palette: &Palette, ) -> Rect { if area.is_empty() { return Rect::default(); } - let event = ¬ification.event; - let body = event.body.as_deref().unwrap_or_default(); - let content_width = unicode_width::UnicodeWidthStr::width(event.title.as_str()) + let content_width = unicode_width::UnicodeWidthStr::width(title) .max(unicode_width::UnicodeWidthStr::width(body)) .saturating_add(6); let width = u16::try_from(content_width) .unwrap_or(u16::MAX) .min(area.width); let height: u16 = if body.is_empty() { 3 } else { 4 }.min(area.height); - let position = event.position.unwrap_or(default_position); let x = match position { crate::config::ToastHerdrPosition::TopLeft | crate::config::ToastHerdrPosition::BottomLeft => area.x, @@ -148,19 +165,10 @@ pub(super) fn render_visible_notification( let inner = block.inner(rect); block.render(rect, buffer); Paragraph::new(Line::from(vec![ - Span::styled( - "●", - Style::default().fg(match event.kind { - SemanticNotificationKind::NeedsAttention => palette.red, - SemanticNotificationKind::Finished => palette.blue, - SemanticNotificationKind::UpdateInstalled | SemanticNotificationKind::Custom => { - palette.accent - } - }), - ), + Span::styled("●", Style::default().fg(dot_color)), Span::raw(" "), Span::styled( - event.title.as_str(), + title, Style::default() .fg(palette.text) .add_modifier(Modifier::BOLD), @@ -185,6 +193,60 @@ pub(super) fn render_visible_notification( rect } +pub(super) fn render_visible_notification( + buffer: &mut Buffer, + area: Rect, + notification: &ClientVisibleNotification, + default_position: crate::config::ToastHerdrPosition, + top_offset: u16, + palette: &Palette, +) -> Rect { + let event = ¬ification.event; + let dot_color = match event.kind { + SemanticNotificationKind::NeedsAttention => palette.red, + SemanticNotificationKind::Finished => palette.blue, + SemanticNotificationKind::UpdateInstalled | SemanticNotificationKind::Custom => { + palette.accent + } + }; + render_notification_card( + buffer, + area, + &event.title, + event.body.as_deref().unwrap_or_default(), + event.position.unwrap_or(default_position), + top_offset, + dot_color, + palette, + ) +} + +pub(super) fn render_endpoint_notice( + buffer: &mut Buffer, + area: Rect, + notice: &ClientVisibleEndpointNotice, + top_offset: u16, + palette: &Palette, +) -> Rect { + render_notification_card( + buffer, + area, + ¬ice.title, + ¬ice.body, + crate::config::ToastHerdrPosition::TopRight, + top_offset, + match notice.key.kind { + ClientEndpointNoticeKind::Unsupported | ClientEndpointNoticeKind::Rejected => { + palette.red + } + ClientEndpointNoticeKind::Timeout | ClientEndpointNoticeKind::Unavailable => { + palette.yellow + } + }, + palette, + ) +} + impl ClientShellState { pub(super) fn focus_visible_notification(&mut self, outcome: &mut ClientShellInput) { let Some(notification) = self.visible_notification.take() else { @@ -246,6 +308,14 @@ impl ClientShellState { self.visible_notification = None; repaint = true; } + if self + .visible_endpoint_notice + .as_ref() + .is_some_and(|visible| now >= visible.deadline) + { + self.visible_endpoint_notice = None; + repaint = true; + } let pending = std::mem::take(&mut self.pending_notifications); let mut effects = Vec::new(); diff --git a/src/client/shell/settings.rs b/src/client/shell/settings.rs index 95a437fe..d90d371f 100644 --- a/src/client/shell/settings.rs +++ b/src/client/shell/settings.rs @@ -233,11 +233,15 @@ impl ClientShellState { settings.integration_messages.clear(); } } - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::IntegrationList(crate::api::schema::EmptyParams::default()), PendingEndpointKind::IntegrationList, outcome, - ); + ) { + if let Some(ClientShellOverlay::Settings(settings)) = self.overlay.as_mut() { + settings.loading_integrations = false; + } + } } fn install_recommended_integrations(&mut self, outcome: &mut ClientShellInput) { @@ -260,15 +264,22 @@ impl ClientShellState { settings.installing_integrations = true; settings.integration_messages.clear(); } - self.pending_integration_installs = targets.len(); + self.pending_integration_installs = 0; for target in targets { - self.push_endpoint_method_with_kind( + if self.push_endpoint_method_with_kind( crate::api::schema::Method::IntegrationInstall( crate::api::schema::IntegrationInstallParams { target }, ), PendingEndpointKind::IntegrationInstall, outcome, - ); + ) { + self.pending_integration_installs += 1; + } + } + if self.pending_integration_installs == 0 { + if let Some(ClientShellOverlay::Settings(settings)) = self.overlay.as_mut() { + settings.installing_integrations = false; + } } outcome.repaint = true; } @@ -296,7 +307,7 @@ impl ClientShellState { "endpoint returned an unexpected integration list result".into(), ); } - Err(error) => self.endpoint_error = Some(error.message), + Err(_) => {} } } (true, Vec::new()) diff --git a/src/client/shell/state.rs b/src/client/shell/state.rs index 81173c18..69e77dc5 100644 --- a/src/client/shell/state.rs +++ b/src/client/shell/state.rs @@ -636,10 +636,33 @@ pub(super) enum PendingEndpointKind { pub(super) struct PendingEndpointRequest { pub(super) boot_id: String, + pub(super) method_name: String, pub(super) confirmation_workspace_id: Option, pub(super) kind: PendingEndpointKind, } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub(super) enum ClientEndpointNoticeKind { + Unsupported, + Rejected, + Timeout, + Unavailable, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub(super) struct ClientEndpointNoticeKey { + pub(super) boot_id: String, + pub(super) kind: ClientEndpointNoticeKind, + pub(super) code: String, +} + +pub(super) struct ClientVisibleEndpointNotice { + pub(super) key: ClientEndpointNoticeKey, + pub(super) title: String, + pub(super) body: String, + pub(super) deadline: std::time::Instant, +} + pub(crate) struct ClientShellEndpointError { pub code: Option, pub message: String, @@ -836,10 +859,16 @@ pub(crate) struct ClientShellState { pub(super) popup_pending: bool, pub(super) popup_pending_deadline: Option, pub(super) next_request_id: u64, + /// Methods advertised by this endpoint. `None` is used only by local tests + /// and legacy construction paths; negotiated endpoint connections always + /// install an explicit set. + pub(super) endpoint_methods: Option>, pub(super) pending_requests: HashMap, pub(super) pending_integration_installs: usize, pub(super) pending_notifications: Vec, pub(super) visible_notification: Option, + pub(super) endpoint_notice_seen: HashSet, + pub(super) visible_endpoint_notice: Option, pub(super) outer_focused: Option, pub(super) ascii_input_source_active: bool, pub(super) pending_input_source_changes: Vec, @@ -971,10 +1000,13 @@ impl ClientShellState { popup_pending: false, popup_pending_deadline: None, next_request_id: 1, + endpoint_methods: None, pending_requests: HashMap::new(), pending_integration_installs: 0, pending_notifications: Vec::new(), visible_notification: None, + endpoint_notice_seen: HashSet::new(), + visible_endpoint_notice: None, outer_focused: None, ascii_input_source_active: false, pending_input_source_changes: Vec::new(), @@ -987,6 +1019,16 @@ impl ClientShellState { } } + pub(crate) fn set_endpoint_methods(&mut self, methods: Option>) { + self.endpoint_methods = methods.map(|methods| methods.into_iter().collect()); + } + + pub(super) fn supports_endpoint_method(&self, method: &crate::api::schema::Method) -> bool { + self.endpoint_methods + .as_ref() + .is_none_or(|methods| methods.contains(crate::api::api_method_name(method))) + } + fn focused_tab_count(&self) -> usize { let Some(snapshot) = self.snapshot.as_deref() else { return 0; @@ -1072,7 +1114,10 @@ impl ClientShellState { } } - pub(crate) fn set_snapshot(&mut self, snapshot: Box) { + pub(crate) fn set_snapshot(&mut self, mut snapshot: Box) { + snapshot + .commands + .retain(|command| command.action != crate::protocol::ClientShellCommandAction::Unknown); if self.snapshot.as_ref().is_some_and(|current| { current.boot_id == snapshot.boot_id && snapshot.revision < current.revision }) { @@ -1154,6 +1199,8 @@ impl ClientShellState { self.pending_integration_installs = 0; self.pending_notifications.clear(); self.visible_notification = None; + self.endpoint_notice_seen.clear(); + self.visible_endpoint_notice = None; self.endpoint_error = None; self.navigate_workspace_id = None; self.overlay = self diff --git a/src/client/shell/surface_patch.rs b/src/client/shell/surface_patch.rs index 10486f50..784fbcd8 100644 --- a/src/client/shell/surface_patch.rs +++ b/src/client/shell/surface_patch.rs @@ -66,6 +66,8 @@ fn fast_path_blocker( Some("client_surface_patch.fallback.endpoint_error") } else if state.config_diagnostic.is_some() { Some("client_surface_patch.fallback.config_diagnostic") + } else if state.visible_endpoint_notice.is_some() { + Some("client_surface_patch.fallback.endpoint_notice") } else if state.visible_notification.is_some() { Some("client_surface_patch.fallback.notification") } else if state.copy_feedback.is_some() { diff --git a/src/client/shell/tests/agents_worktrees_notifications.rs b/src/client/shell/tests/agents_worktrees_notifications.rs index 0ee5aa3f..055b594d 100644 --- a/src/client/shell/tests/agents_worktrees_notifications.rs +++ b/src/client/shell/tests/agents_worktrees_notifications.rs @@ -982,6 +982,36 @@ fn worktree_create_previews_the_endpoint_owned_checkout_path() { )); } +#[test] +fn unavailable_worktree_create_does_not_wedge_the_overlay() { + let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); + state.set_snapshot(Box::new(snapshot())); + state.set_pane_surface(surface()); + let mut prepare = ClientShellInput::default(); + state.record_binding( + crate::input::KeybindMatch::Action(crate::input::KeybindAction::NewWorktree), + &mut prepare, + ); + let [ClientShellAction::Endpoint { request, .. }] = &prepare.actions[..] else { + panic!("new worktree should prepare through worktree.list"); + }; + state.handle_endpoint_result("boot-1", &request.id, Ok(worktree_list_result(None))); + state.set_endpoint_methods(Some(vec!["worktree.list".into()])); + state.handle_input_bytes(b"feature/unavailable"); + + let submit = state.handle_input_bytes(b"\r"); + + assert!(submit.actions.is_empty()); + assert!(matches!( + &state.overlay, + Some(ClientShellOverlay::WorktreeCreate(create)) if !create.creating + )); + assert!(state + .visible_endpoint_notice + .as_ref() + .is_some_and(|notice| notice.key.code == "worktree.create")); +} + #[test] fn worktree_open_filters_and_clicks_a_stable_public_entry() { let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); diff --git a/src/client/shell/tests/keybindings_settings.rs b/src/client/shell/tests/keybindings_settings.rs index 5d2d8243..5b0fced7 100644 --- a/src/client/shell/tests/keybindings_settings.rs +++ b/src/client/shell/tests/keybindings_settings.rs @@ -436,6 +436,60 @@ fn plugin_command_carries_client_owned_selection_coordinates() { ); } +#[test] +fn unavailable_endpoint_method_is_disabled_without_disconnect() { + let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); + state.set_snapshot(Box::new(snapshot())); + state.set_pane_surface(surface()); + state.set_endpoint_methods(Some(vec!["pane.focus".into()])); + let mut outcome = ClientShellInput::default(); + + state.push_endpoint_method( + crate::api::schema::Method::WorkspaceFocus(crate::api::schema::WorkspaceTarget { + workspace_id: "missing".into(), + }), + &mut outcome, + ); + + assert!(outcome.actions.is_empty()); + assert!(outcome.repaint); + let notice = state + .visible_endpoint_notice + .as_ref() + .expect("unsupported action notice"); + assert_eq!(notice.key.kind, ClientEndpointNoticeKind::Unsupported); + assert_eq!(notice.key.code, "workspace.focus"); + assert!(notice.body.contains("This server")); + assert!(state.endpoint_error.is_none()); + + let mut repeated = ClientShellInput::default(); + state.push_endpoint_method( + crate::api::schema::Method::WorkspaceFocus(crate::api::schema::WorkspaceTarget { + workspace_id: "missing".into(), + }), + &mut repeated, + ); + assert!(repeated.actions.is_empty()); + assert!(!repeated.repaint); + + state.compose(106, 20).expect("endpoint notice frame"); + assert!(!state.hits.notification_toast.is_empty()); + + state.handle_input_bytes(b"x"); + assert!(state.visible_endpoint_notice.is_some()); + + let hit = state.hits.notification_toast; + let dismissed = + state.handle_raw_events(vec![RawInputEvent::Mouse(crossterm::event::MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: hit.x, + row: hit.y, + modifiers: KeyModifiers::empty(), + })]); + assert!(dismissed.repaint); + assert!(state.visible_endpoint_notice.is_none()); +} + #[test] fn generic_endpoint_failures_and_control_errors_are_visible() { let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); @@ -461,17 +515,98 @@ fn generic_endpoint_failures_and_control_errors_are_visible() { ); assert!(repaint); assert!(actions.is_empty()); + let notice = state + .visible_endpoint_notice + .as_ref() + .expect("rejected action notice"); + assert_eq!(notice.key.kind, ClientEndpointNoticeKind::Rejected); + assert_eq!(notice.key.code, "workspace.focus:not_found"); + assert_eq!(notice.body, "workspace no longer exists"); + assert!(state.endpoint_error.is_none()); + + assert!(state.receive_endpoint_error("Paste rejected: too large".into())); + let notice = state + .visible_endpoint_notice + .as_ref() + .expect("paste rejection notice"); + assert_eq!(notice.key.kind, ClientEndpointNoticeKind::Rejected); + assert_eq!(notice.key.code, "paste_rejected"); + assert_eq!(notice.body, "Paste rejected: too large"); + assert!(state.receive_endpoint_error("Paste rejected again".into())); assert_eq!( - state.endpoint_error.as_deref(), - Some("workspace no longer exists") + state + .visible_endpoint_notice + .as_ref() + .map(|notice| notice.body.as_str()), + Some("Paste rejected again") ); +} - assert!(state.receive_endpoint_error("Paste rejected: too large".into())); +#[test] +fn endpoint_timeout_is_a_deduplicated_server_notice() { + let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); + state.set_snapshot(Box::new(snapshot())); + let mut outcome = ClientShellInput::default(); + state.push_endpoint_method( + crate::api::schema::Method::WorkspaceFocus(crate::api::schema::WorkspaceTarget { + workspace_id: "work".into(), + }), + &mut outcome, + ); + let request_id = match &outcome.actions[..] { + [ClientShellAction::Endpoint { request, .. }] => request.id.clone(), + other => panic!("expected endpoint request, got {other:?}"), + }; + + let (repaint, actions) = state.handle_endpoint_result( + "boot-1", + &request_id, + Err(ClientShellEndpointError { + code: Some("endpoint_timeout".into()), + message: "transport timeout".into(), + }), + ); + + assert!(repaint); + assert!(actions.is_empty()); + let notice = state + .visible_endpoint_notice + .as_ref() + .expect("timeout notice"); + assert_eq!(notice.key.kind, ClientEndpointNoticeKind::Timeout); + assert_eq!(notice.key.code, "workspace.focus"); + assert!(notice.body.contains("workspace.focus")); + assert!(state.endpoint_error.is_none()); +} + +#[test] +fn endpoint_notice_dedupe_resets_for_a_new_server_boot() { + let mut state = ClientShellState::new(ClientShellConfig::from_config(&Config::default())); + state.set_snapshot(Box::new(snapshot())); + state.set_endpoint_methods(Some(Vec::new())); + let method = || { + crate::api::schema::Method::WorkspaceFocus(crate::api::schema::WorkspaceTarget { + workspace_id: "work".into(), + }) + }; + let mut first = ClientShellInput::default(); + state.push_endpoint_method(method(), &mut first); + assert!(first.repaint); + + let mut next_snapshot = snapshot(); + next_snapshot.boot_id = "boot-2".into(); + state.set_snapshot(Box::new(next_snapshot)); + let mut second = ClientShellInput::default(); + state.push_endpoint_method(method(), &mut second); + + assert!(second.repaint); assert_eq!( - state.endpoint_error.as_deref(), - Some("Paste rejected: too large") + state + .visible_endpoint_notice + .as_ref() + .map(|notice| notice.key.boot_id.as_str()), + Some("boot-2") ); - assert!(!state.receive_endpoint_error("Paste rejected: too large".into())); } #[test] diff --git a/src/client/shell/tests/startup_overlays.rs b/src/client/shell/tests/startup_overlays.rs index 40ba0bd6..2e2901e8 100644 --- a/src/client/shell/tests/startup_overlays.rs +++ b/src/client/shell/tests/startup_overlays.rs @@ -454,6 +454,41 @@ fn onboarding_completion_persists_and_opens_endpoint_integrations() { std::fs::remove_file(path).expect("remove onboarding config"); } +#[test] +fn unavailable_integration_list_does_not_wedge_settings() { + let mut config = + ClientShellConfig::from_config(&Config::default()).with_startup_onboarding(true); + config.local_config_path = std::env::temp_dir().join(format!( + "herdr-client-onboarding-unavailable-{}-{}.toml", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock") + .as_nanos() + )); + let mut state = ClientShellState::new(config); + state.set_snapshot(Box::new(snapshot())); + state.set_pane_surface(surface()); + state.set_endpoint_methods(Some(Vec::new())); + + let outcome = state.handle_input_bytes(b"\r"); + + assert!(outcome.actions.is_empty()); + assert!(matches!( + state.overlay, + Some(ClientShellOverlay::Settings(ClientSettingsOverlay { + section: ClientSettingsSection::Integrations, + loading_integrations: false, + .. + })) + )); + assert!(state + .visible_endpoint_notice + .as_ref() + .is_some_and(|notice| notice.key.code == "integration.list")); + let _ = std::fs::remove_file(&state.config.local_config_path); +} + #[test] fn startup_config_diagnostics_are_client_rendered_and_persist_until_replaced() { let config = ClientShellConfig::from_config(&Config::default()) @@ -558,6 +593,7 @@ fn endpoint_reload_result_does_not_override_snapshot_diagnostic_authority() { "reload-1".into(), PendingEndpointRequest { boot_id: "boot-1".into(), + method_name: "server.reload_config".into(), confirmation_workspace_id: None, kind: PendingEndpointKind::ReloadConfig, }, diff --git a/src/client/shell/worktrees.rs b/src/client/shell/worktrees.rs index 792cd248..bcadf1c8 100644 --- a/src/client/shell/worktrees.rs +++ b/src/client/shell/worktrees.rs @@ -276,7 +276,7 @@ impl ClientShellState { create.creating = true; create.error = None; let workspace_id = create.source_workspace_id.clone(); - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::WorktreeCreate(crate::api::schema::WorktreeCreateParams { workspace_id: Some(workspace_id), cwd: None, @@ -289,7 +289,11 @@ impl ClientShellState { }), PendingEndpointKind::WorktreeCreate, outcome, - ); + ) { + if let Some(ClientShellOverlay::WorktreeCreate(create)) = self.overlay.as_mut() { + create.creating = false; + } + } outcome.repaint = true; } @@ -328,7 +332,7 @@ impl ClientShellState { open.selected = index; open.opening = true; open.error = None; - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::WorktreeOpen(crate::api::schema::WorktreeOpenParams { workspace_id: Some(workspace_id), cwd: None, @@ -340,7 +344,11 @@ impl ClientShellState { }), PendingEndpointKind::WorktreeOpen, outcome, - ); + ) { + if let Some(ClientShellOverlay::WorktreeOpen(open)) = self.overlay.as_mut() { + open.opening = false; + } + } outcome.repaint = true; } @@ -355,7 +363,7 @@ impl ClientShellState { let forced = remove.force_confirmation; remove.removing = true; remove.error = None; - self.push_endpoint_method_with_kind( + if !self.push_endpoint_method_with_kind( crate::api::schema::Method::WorktreeRemove(crate::api::schema::WorktreeRemoveParams { workspace_id, force: forced, @@ -363,7 +371,11 @@ impl ClientShellState { }), PendingEndpointKind::WorktreeRemove { forced }, outcome, - ); + ) { + if let Some(ClientShellOverlay::WorktreeRemove(remove)) = self.overlay.as_mut() { + remove.removing = false; + } + } outcome.repaint = true; } @@ -511,11 +523,8 @@ impl ClientShellState { PendingEndpointKind::PrepareWorktreeCreate { .. } | PendingEndpointKind::PrepareWorktreeOpen { .. } | PendingEndpointKind::PrepareWorktreeRemove { .. }, - Err(error), - ) => { - self.endpoint_error = Some(error.message); - true - } + Err(_), + ) => true, (_, Ok(_)) => { self.endpoint_error = Some("endpoint returned an unexpected worktree result".to_owned()); @@ -535,11 +544,8 @@ impl ClientShellState { | PendingEndpointKind::PaneLinkActivate { .. } | PendingEndpointKind::CopyMotion { .. } | PendingEndpointKind::CopySearch { .. }, - Err(error), - ) => { - self.endpoint_error = Some(error.message); - true - } + Err(_), + ) => true, } } } -- cgit v1.2.3