workspace: Always add projects to windows (#56187)

Anthony Eid created

Right now the recent project picker has different confirm behavior
depending on if a user had open their sidebar in their current Zed
session. If the sidebar had been open the recent project picker adds the
selected project to the multi workspace and makes it the active
workspace, without removing anything. If the sidebar hadn't been open
the recent project picker would replace the active workspace within the
multi workspace.

This caused confusion because the same UX flow had two different
outcomes depending on Zed's state that wasn't obvious to users. This PR
mitigates this by always adding the project to the window while AI
features are enabled. Future follow ups will include the ability to
disable the sidebar, but that's blocked on the agent panel not having a
way to view active threads currently.

This also caused issues in the parallel agents workflow because
replacing a workspace would drop the workspace, thus causing any
terminal processes or threads to be dropped as well.


Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #ISSUE

Release Notes:

- N/A or Added/Fixed/Improved ...

Change summary

crates/recent_projects/src/recent_projects.rs | 205 +++++++++++---------
crates/sidebar/src/sidebar_tests.rs           |  39 +--
crates/workspace/src/multi_workspace.rs       |  21 +
crates/workspace/src/multi_workspace_tests.rs |  53 ++++
4 files changed, 189 insertions(+), 129 deletions(-)

Detailed changes

crates/recent_projects/src/recent_projects.rs 🔗

@@ -78,8 +78,28 @@ struct OpenFolderEntry {
 #[derive(Clone, Debug)]
 enum ProjectPickerEntry {
     Header(SharedString),
-    OpenFolder { index: usize, positions: Vec<usize> },
+    /// A currently open folder from the active workspace's "Current Folders" section.
+    ///
+    /// `index` points into `RecentProjectsDelegate::open_folders`, and `positions` stores the
+    /// fuzzy-match highlight positions for rendering the folder name.
+    OpenFolder {
+        index: usize,
+        positions: Vec<usize>,
+    },
+    /// A project group from the current window's "This Window" section.
+    ///
+    /// These entries come from `RecentProjectsDelegate::window_project_groups`, not from the
+    /// recent-project database. Empty queries list every project group known to the current
+    /// window; non-empty queries list matching project groups. Confirming one activates or loads
+    /// that project group in the current window, while secondary confirm can move local project
+    /// groups to a new window when multiple groups are available.
     ProjectGroup(StringMatch),
+    /// A workspace from the recent-project database's "Recent Projects" section.
+    ///
+    /// The match's `candidate_id` indexes into `RecentProjectsDelegate::workspaces`. Confirming
+    /// one opens that recent workspace in either the current window or a new window, depending on
+    /// whether the picker was invoked for new-window behavior and whether this was a primary or
+    /// secondary confirm.
     RecentProject(StringMatch),
 }
 
@@ -1139,99 +1159,8 @@ impl PickerDelegate for RecentProjectsDelegate {
                 cx.emit(DismissEvent);
             }
             Some(ProjectPickerEntry::RecentProject(selected_match)) => {
-                let Some(workspace) = self.workspace.upgrade() else {
-                    return;
-                };
-                let Some(candidate_workspace) = self.workspaces.get(selected_match.candidate_id)
-                else {
-                    return;
-                };
-
-                let replace_current_window = self.create_new_window == secondary;
-                let candidate_workspace_id = candidate_workspace.workspace_id;
-                let candidate_workspace_location = candidate_workspace.location.clone();
-                let candidate_workspace_paths = candidate_workspace.paths.clone();
-
-                workspace.update(cx, |workspace, cx| {
-                    if workspace.database_id() == Some(candidate_workspace_id) {
-                        return;
-                    }
-                    match candidate_workspace_location {
-                        SerializedWorkspaceLocation::Local => {
-                            let paths = candidate_workspace_paths.paths().to_vec();
-                            if replace_current_window {
-                                if let Some(handle) =
-                                    window.window_handle().downcast::<MultiWorkspace>()
-                                {
-                                    cx.defer(move |cx| {
-                                        if let Some(task) = handle
-                                            .update(cx, |multi_workspace, window, cx| {
-                                                multi_workspace.open_project(
-                                                    paths,
-                                                    OpenMode::Activate,
-                                                    window,
-                                                    cx,
-                                                )
-                                            })
-                                            .log_err()
-                                        {
-                                            task.detach_and_log_err(cx);
-                                        }
-                                    });
-                                }
-                                return;
-                            } else {
-                                workspace
-                                    .open_workspace_for_paths(
-                                        OpenMode::NewWindow,
-                                        paths,
-                                        window,
-                                        cx,
-                                    )
-                                    .detach_and_prompt_err(
-                                        "Failed to open project",
-                                        window,
-                                        cx,
-                                        |_, _, _| None,
-                                    );
-                            }
-                        }
-                        SerializedWorkspaceLocation::Remote(mut connection) => {
-                            let app_state = workspace.app_state().clone();
-                            let replace_window = if replace_current_window {
-                                window.window_handle().downcast::<MultiWorkspace>()
-                            } else {
-                                None
-                            };
-                            let open_options = OpenOptions {
-                                requesting_window: replace_window,
-                                ..Default::default()
-                            };
-                            if let RemoteConnectionOptions::Ssh(connection) = &mut connection {
-                                RemoteSettings::get_global(cx)
-                                    .fill_connection_options_from_settings(connection);
-                            };
-                            let paths = candidate_workspace_paths.paths().to_vec();
-                            cx.spawn_in(window, async move |_, cx| {
-                                open_remote_project(
-                                    connection.clone(),
-                                    paths,
-                                    app_state,
-                                    open_options,
-                                    cx,
-                                )
-                                .await
-                            })
-                            .detach_and_prompt_err(
-                                "Failed to open project",
-                                window,
-                                cx,
-                                |_, _, _| None,
-                            );
-                        }
-                    }
-                });
-                cx.emit(DismissEvent);
+                let candidate_id = selected_match.candidate_id;
+                self.open_recent_projects(candidate_id, secondary, window, cx);
             }
             _ => {}
         }
@@ -2077,6 +2006,94 @@ fn open_local_project(
 }
 
 impl RecentProjectsDelegate {
+    fn open_recent_projects(
+        &mut self,
+        candidate_id: usize,
+        secondary: bool,
+        window: &mut Window,
+        cx: &mut Context<Picker<Self>>,
+    ) {
+        let Some(workspace) = self.workspace.upgrade() else {
+            return;
+        };
+        let Some(candidate_workspace) = self.workspaces.get(candidate_id) else {
+            return;
+        };
+
+        let replace_current_window = self.create_new_window == secondary;
+        let candidate_workspace_id = candidate_workspace.workspace_id;
+        let candidate_workspace_location = candidate_workspace.location.clone();
+        let candidate_workspace_paths = candidate_workspace.paths.clone();
+
+        workspace.update(cx, |workspace, cx| {
+            if workspace.database_id() == Some(candidate_workspace_id) {
+                return;
+            }
+            match candidate_workspace_location {
+                SerializedWorkspaceLocation::Local => {
+                    let paths = candidate_workspace_paths.paths().to_vec();
+                    if replace_current_window {
+                        if let Some(handle) = window.window_handle().downcast::<MultiWorkspace>() {
+                            cx.defer(move |cx| {
+                                if let Some(task) = handle
+                                    .update(cx, |multi_workspace, window, cx| {
+                                        multi_workspace.open_project(
+                                            paths,
+                                            OpenMode::Activate,
+                                            window,
+                                            cx,
+                                        )
+                                    })
+                                    .log_err()
+                                {
+                                    task.detach_and_log_err(cx);
+                                }
+                            });
+                        }
+                        return;
+                    } else {
+                        workspace
+                            .open_workspace_for_paths(OpenMode::NewWindow, paths, window, cx)
+                            .detach_and_prompt_err(
+                                "Failed to open project",
+                                window,
+                                cx,
+                                |_, _, _| None,
+                            );
+                    }
+                }
+                SerializedWorkspaceLocation::Remote(mut connection) => {
+                    let app_state = workspace.app_state().clone();
+                    let replace_window = if replace_current_window {
+                        window.window_handle().downcast::<MultiWorkspace>()
+                    } else {
+                        None
+                    };
+                    let open_options = OpenOptions {
+                        requesting_window: replace_window,
+                        ..Default::default()
+                    };
+                    if let RemoteConnectionOptions::Ssh(connection) = &mut connection {
+                        RemoteSettings::get_global(cx)
+                            .fill_connection_options_from_settings(connection);
+                    };
+                    let paths = candidate_workspace_paths.paths().to_vec();
+                    cx.spawn_in(window, async move |_, cx| {
+                        open_remote_project(connection.clone(), paths, app_state, open_options, cx)
+                            .await
+                    })
+                    .detach_and_prompt_err(
+                        "Failed to open project",
+                        window,
+                        cx,
+                        |_, _, _| None,
+                    );
+                }
+            }
+        });
+        cx.emit(DismissEvent);
+    }
+
     fn add_paths_to_project(
         &mut self,
         paths: Vec<PathBuf>,

crates/sidebar/src/sidebar_tests.rs 🔗

@@ -8198,14 +8198,13 @@ async fn add_test_project(
 }
 
 #[gpui::test]
-async fn test_transient_workspace_lifecycle(cx: &mut TestAppContext) {
+async fn test_workspace_lifecycle_retains_projects_when_sidebar_is_closed(cx: &mut TestAppContext) {
     let (fs, project_a) =
         init_multi_project_test(&["/project-a", "/project-b", "/project-c"], cx).await;
     let (multi_workspace, cx) =
         cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx));
     let _sidebar = setup_sidebar_closed(&multi_workspace, cx);
 
-    // Sidebar starts closed. Initial workspace A is transient.
     let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
     assert!(!multi_workspace.read_with(cx, |mw, _| mw.sidebar_open()));
     assert_eq!(
@@ -8214,25 +8213,25 @@ async fn test_transient_workspace_lifecycle(cx: &mut TestAppContext) {
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_a));
 
-    // Add B — replaces A as the transient workspace.
     let workspace_b = add_test_project("/project-b", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
-        1
+        2
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_b));
+    assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_a)));
 
-    // Add C — replaces B as the transient workspace.
     let workspace_c = add_test_project("/project-c", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
-        1
+        3
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_c));
+    assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_b)));
 }
 
 #[gpui::test]
-async fn test_transient_workspace_retained(cx: &mut TestAppContext) {
+async fn test_workspaces_remain_retained_after_sidebar_closes(cx: &mut TestAppContext) {
     let (fs, project_a) = init_multi_project_test(
         &["/project-a", "/project-b", "/project-c", "/project-d"],
         cx,
@@ -8242,15 +8241,14 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) {
         cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx));
     let _sidebar = setup_sidebar(&multi_workspace, cx);
     assert!(multi_workspace.read_with(cx, |mw, _| mw.sidebar_open()));
+    let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
 
-    // Add B — retained since sidebar is open.
-    let workspace_a = add_test_project("/project-b", &fs, &multi_workspace, cx).await;
+    let workspace_b = add_test_project("/project-b", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
         2
     );
 
-    // Switch to A — B survives. (Switching from one internal workspace, to another)
     multi_workspace.update_in(cx, |mw, window, cx| {
         mw.activate(workspace_a, None, window, cx)
     });
@@ -8259,8 +8257,8 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) {
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
         2
     );
+    assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_b)));
 
-    // Close sidebar — both A and B remain retained.
     multi_workspace.update_in(cx, |mw, window, cx| mw.close_sidebar(window, cx));
     cx.run_until_parked();
     assert_eq!(
@@ -8268,7 +8266,6 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) {
         2
     );
 
-    // Add C — added as new transient workspace. (switching from retained, to transient)
     let workspace_c = add_test_project("/project-c", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
@@ -8276,52 +8273,50 @@ async fn test_transient_workspace_retained(cx: &mut TestAppContext) {
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_c));
 
-    // Add D — replaces C as the transient workspace (Have retained and transient workspaces, transient workspace is dropped)
     let workspace_d = add_test_project("/project-d", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
-        3
+        4
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_d));
+    assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_c)));
 }
 
 #[gpui::test]
-async fn test_transient_workspace_promotion(cx: &mut TestAppContext) {
+async fn test_sidebar_opening_keeps_existing_retained_workspaces(cx: &mut TestAppContext) {
     let (fs, project_a) =
         init_multi_project_test(&["/project-a", "/project-b", "/project-c"], cx).await;
     let (multi_workspace, cx) =
         cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx));
     setup_sidebar_closed(&multi_workspace, cx);
 
-    // Add B — replaces A as the transient workspace (A is discarded).
+    let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
     let workspace_b = add_test_project("/project-b", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
-        1
+        2
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_b));
+    assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_a)));
 
-    // Open sidebar — promotes the transient B to retained.
     multi_workspace.update_in(cx, |mw, window, cx| {
         mw.toggle_sidebar(window, cx);
     });
     cx.run_until_parked();
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
-        1
+        2
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspaces().any(|w| w == &workspace_b)));
 
-    // Close sidebar — the retained B remains.
     multi_workspace.update_in(cx, |mw, window, cx| {
         mw.toggle_sidebar(window, cx);
     });
 
-    // Add C — added as new transient workspace.
     let workspace_c = add_test_project("/project-c", &fs, &multi_workspace, cx).await;
     assert_eq!(
         multi_workspace.read_with(cx, |mw, _| mw.workspaces().count()),
-        2
+        3
     );
     assert!(multi_workspace.read_with(cx, |mw, _| mw.workspace() == &workspace_c));
 }

crates/workspace/src/multi_workspace.rs 🔗

@@ -324,12 +324,15 @@ impl MultiWorkspace {
         });
         let quit_subscription = cx.on_app_quit(Self::app_will_quit);
         let settings_subscription = cx.observe_global_in::<settings::SettingsStore>(window, {
-            let mut previous_disable_ai = DisableAiSettings::get_global(cx).disable_ai;
+            let mut previous_multi_workspace_enabled = !DisableAiSettings::get_global(cx)
+                .disable_ai
+                && AgentSettings::get_global(cx).enabled;
             move |this, window, cx| {
-                if DisableAiSettings::get_global(cx).disable_ai != previous_disable_ai {
+                let multi_workspace_enabled = this.multi_workspace_enabled(cx);
+                if previous_multi_workspace_enabled && !multi_workspace_enabled {
                     this.collapse_to_single_workspace(window, cx);
-                    previous_disable_ai = DisableAiSettings::get_global(cx).disable_ai;
                 }
+                previous_multi_workspace_enabled = multi_workspace_enabled;
             }
         });
         Self::subscribe_to_workspace(&workspace, window, cx);
@@ -1428,11 +1431,17 @@ impl MultiWorkspace {
         let old_active_workspace = self.active_workspace.clone();
         let old_active_was_retained = self.active_workspace_is_retained();
         let workspace_was_retained = self.is_workspace_retained(&workspace);
+        let should_retain_workspaces = self.multi_workspace_enabled(cx);
+
+        if should_retain_workspaces && !old_active_was_retained {
+            let key = old_active_workspace.read(cx).project_group_key(cx);
+            self.retain_workspace(old_active_workspace.clone(), key, cx);
+        }
 
         if !workspace_was_retained {
             self.register_workspace(&workspace, window, cx);
 
-            if self.sidebar_open {
+            if should_retain_workspaces {
                 let key = workspace.read(cx).project_group_key(cx);
                 self.retain_workspace(workspace.clone(), key, cx);
             }
@@ -1445,7 +1454,7 @@ impl MultiWorkspace {
             group.last_active_workspace = Some(self.active_workspace.downgrade());
         }
 
-        if !self.sidebar_open && !old_active_was_retained {
+        if !should_retain_workspaces && !old_active_was_retained {
             self.detach_workspace(&old_active_workspace, cx);
         }
 
@@ -1471,7 +1480,7 @@ impl MultiWorkspace {
     }
 
     /// Collapses to a single workspace, discarding all groups.
-    /// Used when multi-workspace is disabled (e.g. disable_ai).
+    /// Used when multi-workspace is disabled by settings.
     fn collapse_to_single_workspace(&mut self, window: &mut Window, cx: &mut Context<Self>) {
         if self.sidebar_open {
             self.close_sidebar(window, cx);

crates/workspace/src/multi_workspace_tests.rs 🔗

@@ -2,12 +2,13 @@ use std::path::PathBuf;
 
 use super::*;
 use crate::item::test::TestItem;
+use agent_settings::AgentSettings;
 use client::proto;
 use fs::{FakeFs, Fs};
 use gpui::{TestAppContext, VisualTestContext};
 use project::DisableAiSettings;
 use serde_json::json;
-use settings::SettingsStore;
+use settings::{Settings, SettingsStore};
 use util::path;
 
 fn init_test(cx: &mut TestAppContext) {
@@ -90,6 +91,43 @@ async fn test_sidebar_disabled_when_disable_ai_is_enabled(cx: &mut TestAppContex
     });
 }
 
+#[gpui::test]
+async fn test_multi_workspace_collapses_when_agent_is_disabled(cx: &mut TestAppContext) {
+    init_test(cx);
+    let fs = FakeFs::new(cx.executor());
+    fs.insert_tree("/root_a", json!({ "file.txt": "" })).await;
+    fs.insert_tree("/root_b", json!({ "file.txt": "" })).await;
+    let project_a = Project::test(fs.clone(), ["/root_a".as_ref()], cx).await;
+    let project_b = Project::test(fs, ["/root_b".as_ref()], cx).await;
+
+    let (multi_workspace, cx) =
+        cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx));
+
+    multi_workspace.update_in(cx, |multi_workspace, window, cx| {
+        multi_workspace.test_add_workspace(project_b, window, cx);
+    });
+    cx.run_until_parked();
+
+    multi_workspace.read_with(cx, |multi_workspace, cx| {
+        assert!(multi_workspace.multi_workspace_enabled(cx));
+        assert_eq!(multi_workspace.workspaces().count(), 2);
+    });
+
+    cx.update(|_window, cx| {
+        let mut settings = AgentSettings::get_global(cx).clone();
+        settings.enabled = false;
+        AgentSettings::override_global(settings, cx);
+    });
+    cx.run_until_parked();
+
+    multi_workspace.read_with(cx, |multi_workspace, cx| {
+        assert!(!multi_workspace.multi_workspace_enabled(cx));
+        assert!(!multi_workspace.sidebar_open());
+        assert_eq!(multi_workspace.workspaces().count(), 1);
+        assert!(multi_workspace.project_group_keys().is_empty());
+    });
+}
+
 #[gpui::test]
 async fn test_project_group_keys_initial(cx: &mut TestAppContext) {
     init_test(cx);
@@ -618,7 +656,7 @@ async fn test_close_workspace_prefers_already_loaded_neighboring_workspace(
 }
 
 #[gpui::test]
-async fn test_switching_projects_with_sidebar_closed_detaches_old_active_workspace(
+async fn test_switching_projects_with_sidebar_closed_retains_old_active_workspace(
     cx: &mut TestAppContext,
 ) {
     init_test(cx);
@@ -648,7 +686,7 @@ async fn test_switching_projects_with_sidebar_closed_detaches_old_active_workspa
     });
     cx.run_until_parked();
 
-    multi_workspace.read_with(cx, |mw, _cx| {
+    multi_workspace.read_with(cx, |mw, cx| {
         assert_eq!(
             mw.workspace().entity_id(),
             workspace_b.entity_id(),
@@ -656,14 +694,15 @@ async fn test_switching_projects_with_sidebar_closed_detaches_old_active_workspa
         );
         assert_eq!(
             mw.workspaces().count(),
-                        1,
-                        "only the new active workspace should remain open after switching with the sidebar closed"
+            2,
+            "the previous active workspace should remain open after switching with the sidebar closed"
         );
+        assert_eq!(mw.project_groups(cx).len(), 2);
     });
 
     assert!(
-        workspace_a.read_with(cx, |workspace, _cx| workspace.session_id().is_none()),
-        "the previous active workspace should be detached when switching away with the sidebar closed"
+        workspace_a.read_with(cx, |workspace, _cx| workspace.session_id().is_some()),
+        "the previous active workspace should remain attached when switching away with the sidebar closed"
     );
 }