Fix sidebar selections (#50900)

Release Notes:

- N/A

---------

Co-authored-by: Eric Holk <eric@zed.dev>

crates/agent_ui/src/agent_panel.rs

@@ -208,7 +208,7 @@ pub fn init(cx: &mut App) {
                 .register_action(|workspace, _: &OpenAgentDiff, window, cx| {
                     let thread = workspace
                         .panel::<AgentPanel>(cx)
-                        .and_then(|panel| panel.read(cx).active_thread_view().cloned())
+                        .and_then(|panel| panel.read(cx).active_connection_view().cloned())
                         .and_then(|thread_view| {
                             thread_view
                                 .read(cx)
@@ -570,6 +570,7 @@ pub struct AgentPanel {
     start_thread_in: StartThreadIn,
     worktree_creation_status: Option<WorktreeCreationStatus>,
     _thread_view_subscription: Option<Subscription>,
+    _active_thread_focus_subscription: Option<Subscription>,
     _worktree_creation_task: Option<Task<()>>,
     show_trust_workspace_message: bool,
     last_configuration_error_telemetry: Option<String>,
@@ -898,6 +899,7 @@ impl AgentPanel {
             start_thread_in: StartThreadIn::default(),
             worktree_creation_status: None,
             _thread_view_subscription: None,
+            _active_thread_focus_subscription: None,
             _worktree_creation_task: None,
             show_trust_workspace_message: false,
             last_configuration_error_telemetry: None,
@@ -988,7 +990,7 @@ impl AgentPanel {
             .unwrap_or(false)
     }
 
-    pub(crate) fn active_thread_view(&self) -> Option<&Entity<ConnectionView>> {
+    pub fn active_connection_view(&self) -> Option<&Entity<ConnectionView>> {
         match &self.active_view {
             ActiveView::AgentThread { server_view, .. } => Some(server_view),
             ActiveView::Uninitialized
@@ -1173,7 +1175,7 @@ impl AgentPanel {
     }
 
     fn expand_message_editor(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        let Some(thread_view) = self.active_thread_view() else {
+        let Some(thread_view) = self.active_connection_view() else {
             return;
         };
 
@@ -1432,7 +1434,7 @@ impl AgentPanel {
         cx: &mut Context<Self>,
     ) {
         if let Some(workspace) = self.workspace.upgrade()
-            && let Some(thread_view) = self.active_thread_view()
+            && let Some(thread_view) = self.active_connection_view()
             && let Some(active_thread) = thread_view.read(cx).active_thread().cloned()
         {
             active_thread.update(cx, |thread, cx| {
@@ -1763,6 +1765,12 @@ impl AgentPanel {
             ActiveView::AgentThread { server_view } => {
                 self._thread_view_subscription =
                     Self::subscribe_to_active_thread_view(server_view, window, cx);
+                let focus_handle = server_view.focus_handle(cx);
+                self._active_thread_focus_subscription =
+                    Some(cx.on_focus_in(&focus_handle, window, |_this, _window, cx| {
+                        cx.emit(AgentPanelEvent::ThreadFocused);
+                        cx.notify();
+                    }));
                 Some(
                     cx.observe_in(server_view, window, |this, server_view, window, cx| {
                         this._thread_view_subscription =
@@ -1775,6 +1783,7 @@ impl AgentPanel {
             }
             _ => {
                 self._thread_view_subscription = None;
+                self._active_thread_focus_subscription = None;
                 None
             }
         };
@@ -2035,6 +2044,7 @@ impl AgentPanel {
                 .map(|t| t.read(cx).id.clone())
                 == Some(session_id.clone())
             {
+                cx.emit(AgentPanelEvent::ActiveViewChanged);
                 return;
             }
         }
@@ -2642,6 +2652,7 @@ fn agent_panel_dock_position(cx: &App) -> DockPosition {
 
 pub enum AgentPanelEvent {
     ActiveViewChanged,
+    ThreadFocused,
     BackgroundThreadChanged,
 }
 
@@ -3523,7 +3534,7 @@ impl AgentPanel {
             });
 
         let is_thread_loading = self
-            .active_thread_view()
+            .active_connection_view()
             .map(|thread| thread.read(cx).is_loading())
             .unwrap_or(false);
 
@@ -4077,7 +4088,7 @@ impl Render for AgentPanel {
             .on_action(cx.listener(Self::reset_font_size))
             .on_action(cx.listener(Self::toggle_zoom))
             .on_action(cx.listener(|this, _: &ReauthenticateAgent, window, cx| {
-                if let Some(thread_view) = this.active_thread_view() {
+                if let Some(thread_view) = this.active_connection_view() {
                     thread_view.update(cx, |thread_view, cx| thread_view.reauthenticate(window, cx))
                 }
             }))
@@ -4263,7 +4274,7 @@ impl AgentPanelDelegate for ConcreteAssistantPanelDelegate {
             // Wait to create a new context until the workspace is no longer
             // being updated.
             cx.defer_in(window, move |panel, window, cx| {
-                if let Some(thread_view) = panel.active_thread_view() {
+                if let Some(thread_view) = panel.active_connection_view() {
                     thread_view.update(cx, |thread_view, cx| {
                         thread_view.insert_selections(window, cx);
                     });
@@ -4301,7 +4312,7 @@ impl AgentPanelDelegate for ConcreteAssistantPanelDelegate {
             // Wait to create a new context until the workspace is no longer
             // being updated.
             cx.defer_in(window, move |panel, window, cx| {
-                if let Some(thread_view) = panel.active_thread_view() {
+                if let Some(thread_view) = panel.active_connection_view() {
                     thread_view.update(cx, |thread_view, cx| {
                         thread_view.insert_terminal_text(text, window, cx);
                     });
@@ -4367,7 +4378,7 @@ impl AgentPanel {
     /// This is a test-only accessor that exposes the private `active_thread_view()`
     /// method for test assertions. Not compiled into production builds.
     pub fn active_thread_view_for_tests(&self) -> Option<&Entity<ConnectionView>> {
-        self.active_thread_view()
+        self.active_connection_view()
     }
 
     /// Sets the start_thread_in value directly, bypassing validation.
@@ -4552,7 +4563,7 @@ mod tests {
                 "workspace A agent type should be restored"
             );
             assert!(
-                panel.active_thread_view().is_some(),
+                panel.active_connection_view().is_some(),
                 "workspace A should have its active thread restored"
             );
         });
@@ -4572,7 +4583,7 @@ mod tests {
                 "workspace B agent type should be restored"
             );
             assert!(
-                panel.active_thread_view().is_none(),
+                panel.active_connection_view().is_none(),
                 "workspace B should have no active thread"
             );
         });
@@ -4709,7 +4720,7 @@ mod tests {
         send_message(&panel, &mut cx);
 
         let weak_view_a = panel.read_with(&cx, |panel, _cx| {
-            panel.active_thread_view().unwrap().downgrade()
+            panel.active_connection_view().unwrap().downgrade()
         });
 
         // Thread A should be idle (auto-completed via set_next_prompt_updates).

crates/agent_ui/src/connection_view.rs

@@ -399,7 +399,10 @@ impl ConnectionView {
 
 enum ServerState {
     Loading(Entity<LoadingView>),
-    LoadError(LoadError),
+    LoadError {
+        error: LoadError,
+        session_id: Option<acp::SessionId>,
+    },
     Connected(ConnectedServerState),
 }
 
@@ -430,6 +433,7 @@ impl AuthState {
 }
 
 struct LoadingView {
+    session_id: Option<acp::SessionId>,
     title: SharedString,
     _load_task: Task<()>,
     _update_title_task: Task<anyhow::Result<()>>,
@@ -572,12 +576,18 @@ impl ConnectionView {
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> ServerState {
+        let session_id = resume_thread
+            .as_ref()
+            .map(|thread| thread.session_id.clone());
         if project.read(cx).is_via_collab()
             && agent.clone().downcast::<NativeAgentServer>().is_none()
         {
-            return ServerState::LoadError(LoadError::Other(
-                "External agents are not yet supported in shared projects.".into(),
-            ));
+            return ServerState::LoadError {
+                error: LoadError::Other(
+                    "External agents are not yet supported in shared projects.".into(),
+                ),
+                session_id,
+            };
         }
         let mut worktrees = project.read(cx).visible_worktrees(cx).collect::<Vec<_>>();
         // Pick the first non-single-file worktree for the root directory if there are any,
@@ -633,17 +643,18 @@ impl ConnectionView {
         );
 
         let connect_task = agent.connect(delegate, cx);
+        let load_session_id = session_id.clone();
         let load_task = cx.spawn_in(window, async move |this, cx| {
             let connection = match connect_task.await {
                 Ok(connection) => connection,
                 Err(err) => {
                     this.update_in(cx, |this, window, cx| {
                         if err.downcast_ref::<LoadError>().is_some() {
-                            this.handle_load_error(err, window, cx);
+                            this.handle_load_error(load_session_id.clone(), err, window, cx);
                         } else if let Some(active) = this.active_thread() {
                             active.update(cx, |active, cx| active.handle_thread_error(err, cx));
                         } else {
-                            this.handle_load_error(err, window, cx);
+                            this.handle_load_error(load_session_id.clone(), err, window, cx);
                         }
                         cx.notify();
                     })
@@ -756,7 +767,7 @@ impl ConnectionView {
                         );
                     }
                     Err(err) => {
-                        this.handle_load_error(err, window, cx);
+                        this.handle_load_error(load_session_id.clone(), err, window, cx);
                     }
                 };
             })
@@ -792,6 +803,7 @@ impl ConnectionView {
             });
 
             LoadingView {
+                session_id,
                 title: "Loading…".into(),
                 _load_task: load_task,
                 _update_title_task: update_title_task,
@@ -1086,6 +1098,7 @@ impl ConnectionView {
 
     fn handle_load_error(
         &mut self,
+        session_id: Option<acp::SessionId>,
         err: anyhow::Error,
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -1106,7 +1119,13 @@ impl ConnectionView {
             LoadError::Other(format!("{:#}", err).into())
         };
         self.emit_load_error_telemetry(&load_error);
-        self.set_server_state(ServerState::LoadError(load_error), cx);
+        self.set_server_state(
+            ServerState::LoadError {
+                error: load_error,
+                session_id,
+            },
+            cx,
+        );
     }
 
     fn handle_agent_servers_updated(
@@ -1121,7 +1140,7 @@ impl ConnectionView {
         // This handles the case where a thread is restored before authentication completes.
         let should_retry = match &self.server_state {
             ServerState::Loading(_) => false,
-            ServerState::LoadError(_) => true,
+            ServerState::LoadError { .. } => true,
             ServerState::Connected(connected) => {
                 connected.auth_state.is_ok() && connected.has_thread_error(cx)
             }
@@ -1145,7 +1164,7 @@ impl ConnectionView {
         match &self.server_state {
             ServerState::Connected(_) => "New Thread".into(),
             ServerState::Loading(loading_view) => loading_view.read(cx).title.clone(),
-            ServerState::LoadError(error) => match error {
+            ServerState::LoadError { error, .. } => match error {
                 LoadError::Unsupported { .. } => format!("Upgrade {}", self.agent.name()).into(),
                 LoadError::FailedToInstall(_) => {
                     format!("Failed to Install {}", self.agent.name()).into()
@@ -1164,6 +1183,17 @@ impl ConnectionView {
         }
     }
 
+    // The parent ID is None if we haven't created a thread yet
+    pub fn parent_id(&self, cx: &App) -> Option<acp::SessionId> {
+        match &self.server_state {
+            ServerState::Connected(_) => self
+                .parent_thread(cx)
+                .map(|thread| thread.read(cx).id.clone()),
+            ServerState::Loading(loading) => loading.read(cx).session_id.clone(),
+            ServerState::LoadError { session_id, .. } => session_id.clone(),
+        }
+    }
+
     pub fn is_loading(&self) -> bool {
         matches!(self.server_state, ServerState::Loading { .. })
     }
@@ -1361,7 +1391,13 @@ impl ConnectionView {
                         self.focus_handle.focus(window, cx)
                     }
                 }
-                self.set_server_state(ServerState::LoadError(error.clone()), cx);
+                self.set_server_state(
+                    ServerState::LoadError {
+                        error: error.clone(),
+                        session_id: Some(thread_id),
+                    },
+                    cx,
+                );
             }
             AcpThreadEvent::TitleUpdated => {
                 let title = thread.read(cx).title();
@@ -2635,7 +2671,7 @@ impl Render for ConnectionView {
                     .flex_1()
                     // .child(self.render_recent_history(cx))
                     .into_any(),
-                ServerState::LoadError(e) => v_flex()
+                ServerState::LoadError { error: e, .. } => v_flex()
                     .flex_1()
                     .size_full()
                     .items_center()
@@ -3126,7 +3162,10 @@ pub(crate) mod tests {
                 "Tab title should show the agent name with an error prefix"
             );
             match &view.server_state {
-                ServerState::LoadError(LoadError::Other(msg)) => {
+                ServerState::LoadError {
+                    error: LoadError::Other(msg),
+                    ..
+                } => {
                     assert!(
                         msg.contains("Invalid gzip header"),
                         "Error callout should contain the underlying extraction error, got: {msg}"
@@ -3136,7 +3175,7 @@ pub(crate) mod tests {
                     "Expected LoadError::Other, got: {}",
                     match other {
                         ServerState::Loading(_) => "Loading (stuck!)",
-                        ServerState::LoadError(_) => "LoadError (wrong variant)",
+                        ServerState::LoadError { .. } => "LoadError (wrong variant)",
                         ServerState::Connected(_) => "Connected",
                     }
                 ),

crates/gpui/examples/active_state_bug.rs

@@ -0,0 +1,47 @@
+/// Click the button — the `.active()` background gets stuck on every other click.
+use gpui::*;
+use gpui_platform::application;
+
+struct Example;
+
+impl Render for Example {
+    fn render(&mut self, _window: &mut Window, _cx: &mut Context<Self>) -> impl IntoElement {
+        // Colors from Zed's default dark theme
+        let bg = hsla(215. / 360., 0.12, 0.15, 1.);
+        let text = hsla(221. / 360., 0.11, 0.86, 1.);
+        let hover = hsla(225. / 360., 0.118, 0.267, 1.);
+        let active = hsla(220. / 360., 0.118, 0.20, 1.);
+
+        div().bg(bg).size_full().p_1().child(
+            div()
+                .id("button")
+                .px_2()
+                .py_0p5()
+                .rounded_md()
+                .text_sm()
+                .text_color(text)
+                .hover(|s| s.bg(hover))
+                .active(|s| s.bg(active))
+                .on_click(|_, _, _| {})
+                .child("Click me"),
+        )
+    }
+}
+
+fn main() {
+    application().run(|cx: &mut App| {
+        cx.open_window(
+            WindowOptions {
+                window_bounds: Some(WindowBounds::Windowed(Bounds::centered(
+                    None,
+                    size(px(200.), px(60.)),
+                    cx,
+                ))),
+                ..Default::default()
+            },
+            |_, cx| cx.new(|_| Example),
+        )
+        .unwrap();
+        cx.activate(true);
+    });
+}

crates/gpui/src/elements/div.rs

@@ -2517,18 +2517,24 @@ impl Interactivity {
                 );
             }
 
+            // We unconditionally bind both the mouse up and mouse down active state handlers
+            // Because we might not get a chance to render a frame before the mouse up event arrives.
             let active_state = element_state
                 .clicked_state
                 .get_or_insert_with(Default::default)
                 .clone();
-            if active_state.borrow().is_clicked() {
+
+            {
+                let active_state = active_state.clone();
                 window.on_mouse_event(move |_: &MouseUpEvent, phase, window, _cx| {
-                    if phase == DispatchPhase::Capture {
+                    if phase == DispatchPhase::Capture && active_state.borrow().is_clicked() {
                         *active_state.borrow_mut() = ElementClickedState::default();
                         window.refresh();
                     }
                 });
-            } else {
+            }
+
+            {
                 let active_group_hitbox = self
                     .group_active_style
                     .as_ref()

crates/sidebar/Cargo.toml

@@ -21,6 +21,7 @@ agent-client-protocol.workspace = true
 agent_ui.workspace = true
 chrono.workspace = true
 editor.workspace = true
+feature_flags.workspace = true
 fs.workspace = true
 gpui.workspace = true
 menu.workspace = true

crates/workspace/src/multi_workspace.rs

@@ -35,6 +35,12 @@ actions!(
     ]
 );
 
+pub enum MultiWorkspaceEvent {
+    ActiveWorkspaceChanged,
+    WorkspaceAdded(Entity<Workspace>),
+    WorkspaceRemoved(EntityId),
+}
+
 pub enum SidebarEvent {
     Open,
     Close,
@@ -109,6 +115,8 @@ pub struct MultiWorkspace {
     _subscriptions: Vec<Subscription>,
 }
 
+impl EventEmitter<MultiWorkspaceEvent> for MultiWorkspace {}
+
 impl MultiWorkspace {
     pub fn new(workspace: Entity<Workspace>, window: &mut Window, cx: &mut Context<Self>) -> Self {
         let release_subscription = cx.on_release(|this: &mut MultiWorkspace, _cx| {
@@ -304,6 +312,7 @@ impl MultiWorkspace {
         if !self.multi_workspace_enabled(cx) {
             self.workspaces[0] = workspace;
             self.active_workspace_index = 0;
+            cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
             cx.notify();
             return;
         }
@@ -321,7 +330,11 @@ impl MultiWorkspace {
         cx: &mut Context<Self>,
     ) -> usize {
         let index = self.add_workspace(workspace, cx);
+        let changed = self.active_workspace_index != index;
         self.active_workspace_index = index;
+        if changed {
+            cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
+        }
         cx.notify();
         index
     }
@@ -338,7 +351,8 @@ impl MultiWorkspace {
                 });
             }
             Self::subscribe_to_workspace(&workspace, cx);
-            self.workspaces.push(workspace);
+            self.workspaces.push(workspace.clone());
+            cx.emit(MultiWorkspaceEvent::WorkspaceAdded(workspace));
             cx.notify();
             self.workspaces.len() - 1
         }
@@ -349,9 +363,13 @@ impl MultiWorkspace {
             index < self.workspaces.len(),
             "workspace index out of bounds"
         );
+        let changed = self.active_workspace_index != index;
         self.active_workspace_index = index;
         self.serialize(cx);
         self.focus_active_workspace(window, cx);
+        if changed {
+            cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
+        }
         cx.notify();
     }
 
@@ -406,7 +424,7 @@ impl MultiWorkspace {
         }
     }
 
-    fn focus_active_workspace(&self, window: &mut Window, cx: &mut App) {
+    pub fn focus_active_workspace(&self, window: &mut Window, cx: &mut App) {
         // If a dock panel is zoomed, focus it instead of the center pane.
         // Otherwise, focusing the center pane triggers dismiss_zoomed_items_to_reveal
         // which closes the zoomed dock.
@@ -633,6 +651,10 @@ impl MultiWorkspace {
 
         self.serialize(cx);
         self.focus_active_workspace(window, cx);
+        cx.emit(MultiWorkspaceEvent::WorkspaceRemoved(
+            removed_workspace.entity_id(),
+        ));
+        cx.emit(MultiWorkspaceEvent::ActiveWorkspaceChanged);
         cx.notify();
     }
 

crates/workspace/src/workspace.rs

@@ -27,9 +27,9 @@ mod workspace_settings;
 pub use crate::notifications::NotificationFrame;
 pub use dock::Panel;
 pub use multi_workspace::{
-    DraggedSidebar, FocusWorkspaceSidebar, MultiWorkspace, NewWorkspaceInWindow,
-    NextWorkspaceInWindow, PreviousWorkspaceInWindow, Sidebar, SidebarEvent, SidebarHandle,
-    ToggleWorkspaceSidebar,
+    DraggedSidebar, FocusWorkspaceSidebar, MultiWorkspace, MultiWorkspaceEvent,
+    NewWorkspaceInWindow, NextWorkspaceInWindow, PreviousWorkspaceInWindow, Sidebar, SidebarEvent,
+    SidebarHandle, ToggleWorkspaceSidebar,
 };
 pub use path_list::{PathList, SerializedPathList};
 pub use toast_layer::{ToastAction, ToastLayer, ToastView};
@@ -1230,6 +1230,7 @@ pub enum Event {
     ZoomChanged,
     ModalOpened,
     Activate,
+    PanelAdded(AnyView),
 }
 
 #[derive(Debug, Clone)]
@@ -2129,10 +2130,13 @@ impl Workspace {
 
         let dock_position = panel.position(window, cx);
         let dock = self.dock_at_position(dock_position);
+        let any_panel = panel.to_any();
 
         dock.update(cx, |dock, cx| {
             dock.add_panel(panel, self.weak_self.clone(), window, cx)
         });
+
+        cx.emit(Event::PanelAdded(any_panel));
     }
 
     pub fn remove_panel<T: Panel>(

crates/zed/src/zed.rs

@@ -371,15 +371,12 @@ pub fn initialize_workspace(
     })
     .detach();
 
-    cx.observe_new(|multi_workspace: &mut MultiWorkspace, window, cx| {
+    cx.observe_new(|_multi_workspace: &mut MultiWorkspace, window, cx| {
         let Some(window) = window else {
             return;
         };
-        let multi_workspace_handle = cx.entity();
-        let sidebar = cx.new(|cx| Sidebar::new(multi_workspace_handle.clone(), window, cx));
-        multi_workspace.register_sidebar(sidebar, window, cx);
 
-        let multi_workspace_handle = multi_workspace_handle.downgrade();
+        let multi_workspace_handle = cx.entity().downgrade();
         window.on_window_should_close(cx, move |window, cx| {
             multi_workspace_handle
                 .update(cx, |multi_workspace, cx| {
@@ -389,6 +386,20 @@ pub fn initialize_workspace(
                 })
                 .unwrap_or(true)
         });
+
+        let window_handle = window.window_handle();
+        let multi_workspace_handle = cx.entity();
+        cx.defer(move |cx| {
+            window_handle
+                .update(cx, |_, window, cx| {
+                    let sidebar =
+                        cx.new(|cx| Sidebar::new(multi_workspace_handle.clone(), window, cx));
+                    multi_workspace_handle.update(cx, |multi_workspace, cx| {
+                        multi_workspace.register_sidebar(sidebar, window, cx);
+                    });
+                })
+                .ok();
+        });
     })
     .detach();