fix: reveal selected agent when cycling the sidebar (#4355)

* fix: reveal selected agent when cycling the sidebar

* fix: reveal agent after endpoint activation

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::<Vec<_>>();
+        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<ClientEndpointId, HashSet<String>>,
     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
@@ -110,6 +110,154 @@ fn state_with_scrollable_agents() -> (ClientShellState, ClientEndpointId) {
 }
 
 #[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();
     for endpoint_id in [remote.clone(), ClientEndpointId::Local, remote] {