From 028780cebccc135d67a9ff97fbb0b11a150608de Mon Sep 17 00:00:00 2001 From: Can Celik Date: Fri, 18 Sep 2026 21:50:26 +0300 Subject: [PATCH] fix: reveal selected agent when cycling the sidebar (#4355) * fix: reveal selected agent when cycling the sidebar * fix: reveal agent after endpoint activation --- src/client/shell/endpoint_agents.rs | 35 ++++++ src/client/shell/endpoint_navigation.rs | 18 ++- src/client/shell/endpoints.rs | 7 ++ src/client/shell/state.rs | 2 + src/client/shell/tests/endpoints.rs | 148 ++++++++++++++++++++++++ 5 files changed, 208 insertions(+), 2 deletions(-) diff --git a/src/client/shell/endpoint_agents.rs b/src/client/shell/endpoint_agents.rs index 3cc106b3..b56b41fa 100644 --- a/src/client/shell/endpoint_agents.rs +++ b/src/client/shell/endpoint_agents.rs @@ -87,6 +87,41 @@ pub(super) fn render_expanded( ); } +impl ClientShellState { + pub(super) fn reveal_endpoint_agent( + &mut self, + endpoint_id: &ClientEndpointId, + pane_id: &str, + body_height: u16, + ) { + if body_height == 0 { + return; + } + let rows = agent_rows(&self.endpoints, &self.active_endpoint_id, &self.config); + let Some(target) = rows + .iter() + .position(|row| &row.endpoint_id == endpoint_id && row.agent.pane_id == pane_id) + else { + return; + }; + let heights = rows + .iter() + .map(|row| row.agent.rows.len().max(1).min(u16::MAX as usize) as u16) + .collect::>(); + let mut gaps = vec![self.config.agents.row_gap; rows.len()]; + if let Some(last) = gaps.last_mut() { + *last = 0; + } + self.agent_scroll = super::scroll::list_scroll_start_to_reveal( + &heights, + &gaps, + body_height, + self.agent_scroll, + target, + ); + } +} + struct EndpointAgentRow { endpoint_id: ClientEndpointId, machine_label: String, diff --git a/src/client/shell/endpoint_navigation.rs b/src/client/shell/endpoint_navigation.rs index 6cb8c2c4..c15fd434 100644 --- a/src/client/shell/endpoint_navigation.rs +++ b/src/client/shell/endpoint_navigation.rs @@ -203,11 +203,23 @@ impl ClientShellState { _ => unreachable!("endpoint agent navigation"), }; let target = &agents[next]; - self.focus_or_activate( + if self.focus_or_activate( target.endpoint_id.clone(), ClientEndpointFocusTarget::Pane(target.pane_id.clone()), outcome, - ); + ) { + if target.endpoint_id == self.active_endpoint_id { + self.reveal_endpoint_agent( + &target.endpoint_id, + &target.pane_id, + self.hits.agent_body.height, + ); + } else { + self.pending_agent_reveal = + Some((target.endpoint_id.clone(), target.pane_id.clone())); + } + outcome.repaint = true; + } return true; } false @@ -218,6 +230,7 @@ impl ClientShellState { endpoint_id: ClientEndpointId, outcome: &mut ClientShellInput, ) -> bool { + self.pending_agent_reveal = None; let online = self.endpoint_is_online(&endpoint_id); if !online && !endpoint_id.is_local() { let label = self.endpoint_label(&endpoint_id).to_owned(); @@ -242,6 +255,7 @@ impl ClientShellState { target: ClientEndpointFocusTarget, outcome: &mut ClientShellInput, ) -> bool { + self.pending_agent_reveal = None; let online = self.endpoint_is_online(&endpoint_id); if !online && !endpoint_id.is_local() { let label = self.endpoint_label(&endpoint_id).to_owned(); diff --git a/src/client/shell/endpoints.rs b/src/client/shell/endpoints.rs index c27358e5..3d1d419b 100644 --- a/src/client/shell/endpoints.rs +++ b/src/client/shell/endpoints.rs @@ -195,6 +195,10 @@ impl ClientShellState { } pub(crate) fn activate_endpoint_projection(&mut self, endpoint_id: &ClientEndpointId) -> bool { + let pending_agent_reveal = self + .pending_agent_reveal + .take_if(|(target_endpoint, _)| target_endpoint == endpoint_id); + let agent_body_height = self.hits.agent_body.height; let Some(endpoint) = self .endpoints .iter() @@ -221,6 +225,9 @@ impl ClientShellState { // The aggregate agent list belongs to the client, not one endpoint. self.agent_scroll = agent_scroll; } + if let Some((_, pane_id)) = pending_agent_reveal { + self.reveal_endpoint_agent(endpoint_id, &pane_id, agent_body_height); + } true } diff --git a/src/client/shell/state.rs b/src/client/shell/state.rs index 901c5a28..b975b2f6 100644 --- a/src/client/shell/state.rs +++ b/src/client/shell/state.rs @@ -861,6 +861,7 @@ pub(crate) struct ClientShellState { pub(super) remote_collapsed_groups: HashMap>, pub(super) workspace_scroll: usize, pub(super) agent_scroll: usize, + pub(super) pending_agent_reveal: Option<(ClientEndpointId, String)>, pub(super) tab_scroll: usize, pub(super) mobile_switcher_scroll: usize, pub(super) reveal_focused_workspace: bool, @@ -1023,6 +1024,7 @@ impl ClientShellState { remote_collapsed_groups, workspace_scroll: 0, agent_scroll: 0, + pending_agent_reveal: None, tab_scroll: 0, mobile_switcher_scroll: 0, reveal_focused_workspace: true, diff --git a/src/client/shell/tests/endpoints.rs b/src/client/shell/tests/endpoints.rs index 846806f1..e4914971 100644 --- a/src/client/shell/tests/endpoints.rs +++ b/src/client/shell/tests/endpoints.rs @@ -109,6 +109,154 @@ fn state_with_scrollable_agents() -> (ClientShellState, ClientEndpointId) { (state, remote) } +#[test] +fn agent_navigation_reveals_offscreen_targets() { + use crate::input::KeybindAction; + + for action in [ + KeybindAction::NextAgent, + KeybindAction::PreviousAgent, + KeybindAction::FocusAgent(0), + ] { + let (mut state, remote) = state_with_scrollable_agents(); + let (endpoint_id, pane_id) = match action { + KeybindAction::NextAgent => (ClientEndpointId::Local, "pane_2"), + KeybindAction::PreviousAgent => (remote, "pane_8"), + _ => (ClientEndpointId::Local, "pane_1"), + }; + state.agent_scroll = if action == KeybindAction::PreviousAgent { + 0 + } else { + state.hits.agent_max_scroll + }; + state.compose(100, 28).unwrap(); + assert!(!state + .hits + .endpoint_agents + .iter() + .any(|(_, endpoint, pane)| { endpoint == &endpoint_id && pane == pane_id })); + + let mut outcome = ClientShellInput::default(); + assert!(state.handle_endpoint_navigation(action, &mut outcome)); + assert!(outcome.repaint, "agent navigation must request a frame"); + if endpoint_id != state.active_endpoint_id { + assert!(state.activate_endpoint_projection(&endpoint_id)); + } + state.compose(100, 28).unwrap(); + assert!( + state + .hits + .endpoint_agents + .iter() + .any(|(_, endpoint, pane)| { endpoint == &endpoint_id && pane == pane_id }), + "{action:?} must reveal the selected agent" + ); + } +} + +#[test] +fn agent_navigation_reveals_target_using_destination_sort() { + use crate::api::schema::{ + AgentViewBuiltinSortField, AgentViewSort, AgentViewSortField, AgentViewSortOrder, + }; + + let (mut state, remote) = state_with_scrollable_agents(); + for (endpoint_id, base) in [(ClientEndpointId::Local, 0), (remote.clone(), 8)] { + let mut projection = state + .endpoints + .iter() + .find(|endpoint| endpoint.endpoint_id == endpoint_id) + .unwrap() + .snapshot + .clone() + .unwrap(); + for (index, agent) in projection.agents.iter_mut().enumerate() { + agent.state_change_seq = base + index as u64; + } + if endpoint_id == remote { + projection.agent_view_label = Some("recent".into()); + } + state.set_endpoint_snapshot(&endpoint_id, projection); + } + state.set_test_endpoint_agent_view(&ClientEndpointId::Local, None); + let mut view = current_workspace_view(); + view.label = Some("recent".into()); + view.filter = None; + view.sort = vec![AgentViewSort { + field: AgentViewSortField::Builtin(AgentViewBuiltinSortField::StateChangeSeq), + order: AgentViewSortOrder::Desc, + }]; + state.set_test_endpoint_agent_view(&remote, Some(view)); + state.compose(100, 28).unwrap(); + + let mut outcome = ClientShellInput::default(); + assert!(state + .handle_endpoint_navigation(crate::input::KeybindAction::FocusAgent(15), &mut outcome,)); + assert!(matches!( + outcome.actions.as_slice(), + [ClientShellAction::ActivateEndpoint { + endpoint_id, + target: Some(ClientEndpointFocusTarget::Pane(pane_id)), + }] if endpoint_id == &remote && pane_id == "pane_8" + )); + // A superseded handoff restores its source before activating the new target. + assert!(state.activate_endpoint_projection(&ClientEndpointId::Local)); + state.compose(100, 28).unwrap(); + assert!(state.activate_endpoint_projection(&remote)); + state.compose(100, 28).unwrap(); + assert!(state + .hits + .endpoint_agents + .iter() + .any(|(_, endpoint, pane)| { endpoint == &remote && pane == "pane_8" })); +} + +#[test] +fn agent_navigation_reveal_is_cancelled_by_another_selection() { + for select_pane in [false, true] { + let (mut state, remote) = state_with_scrollable_agents(); + let scroll = state.agent_scroll; + let mut outcome = ClientShellInput::default(); + assert!(state + .handle_endpoint_navigation(crate::input::KeybindAction::PreviousAgent, &mut outcome,)); + assert_eq!(state.agent_scroll, scroll); + if select_pane { + assert!(state.focus_or_activate( + remote.clone(), + ClientEndpointFocusTarget::Pane("pane_1".into()), + &mut outcome, + )); + } else { + assert!(state.activate_endpoint(remote.clone(), &mut outcome)); + } + assert!(state.activate_endpoint_projection(&remote)); + state.compose(100, 28).unwrap(); + assert_eq!(state.agent_scroll, scroll); + } +} + +#[test] +fn agent_navigation_keeps_scroll_when_target_is_visible() { + let (mut state, _) = state_with_scrollable_agents(); + let (_, endpoint_id, pane_id) = state.hits.endpoint_agents[1].clone(); + let targets = super::super::aggregate_navigation::online_agent_targets( + &state.endpoints, + &state.active_endpoint_id, + state.config.agent_panel_sort, + ); + let index = targets + .iter() + .position(|target| target.endpoint_id == endpoint_id && target.pane_id == pane_id) + .unwrap(); + let scroll = state.agent_scroll; + assert!(state.handle_endpoint_navigation( + crate::input::KeybindAction::FocusAgent(index), + &mut ClientShellInput::default(), + )); + state.compose(100, 28).unwrap(); + assert_eq!(state.agent_scroll, scroll); +} + #[test] fn switching_machines_preserves_aggregate_agent_scroll_and_visible_rows() { let (mut state, remote) = state_with_scrollable_agents();