Detailed changes
@@ -1135,8 +1135,12 @@ impl PickerDelegate for RecentProjectsDelegate {
cx.defer(move |cx| {
if let Some(task) = handle
.update(cx, |multi_workspace, window, cx| {
- multi_workspace
- .find_or_create_local_workspace(path_list, window, cx)
+ multi_workspace.find_or_create_local_workspace(
+ path_list,
+ &[],
+ window,
+ cx,
+ )
})
.log_err()
{
@@ -3193,11 +3193,12 @@ impl Sidebar {
.clone()
});
+ let excluded = [workspace_to_remove.clone()];
let remove_task = multi_workspace.update(cx, |mw, cx| {
mw.remove(
[workspace_to_remove],
move |this, window, cx| {
- this.find_or_create_local_workspace(fallback_paths, window, cx)
+ this.find_or_create_local_workspace(fallback_paths, &excluded, window, cx)
},
window,
cx,
@@ -987,12 +987,14 @@ impl MultiWorkspace {
// Now remove the group.
self.project_groups.retain(|group| group.key != *group_key);
+ let excluded_workspaces = workspaces.clone();
self.remove(
workspaces,
move |this, window, cx| {
if let Some(neighbor_key) = neighbor_key {
return this.find_or_create_local_workspace(
neighbor_key.path_list().clone(),
+ &excluded_workspaces,
window,
cx,
);
@@ -1069,8 +1071,21 @@ impl MultiWorkspace {
path_list: &PathList,
host: Option<&RemoteConnectionOptions>,
cx: &App,
+ ) -> Option<Entity<Workspace>> {
+ self.workspace_for_paths_excluding(path_list, host, &[], cx)
+ }
+
+ fn workspace_for_paths_excluding(
+ &self,
+ path_list: &PathList,
+ host: Option<&RemoteConnectionOptions>,
+ excluding: &[Entity<Workspace>],
+ cx: &App,
) -> Option<Entity<Workspace>> {
for workspace in self.workspaces() {
+ if excluding.contains(workspace) {
+ continue;
+ }
let root_paths = PathList::new(&workspace.read(cx).root_paths(cx));
let key = workspace.read(cx).project_group_key(cx);
let host_matches = key.host().as_ref() == host;
@@ -1116,7 +1131,7 @@ impl MultiWorkspace {
}
let Some(connection_options) = host else {
- return self.find_or_create_local_workspace(paths, window, cx);
+ return self.find_or_create_local_workspace(paths, &[], window, cx);
};
let app_state = self.workspace().read(cx).app_state().clone();
@@ -1168,13 +1183,18 @@ impl MultiWorkspace {
/// or creates a new one (deserializing its saved state from the database).
/// Never searches other windows or matches workspaces with a superset of
/// the requested paths.
+ ///
+ /// `excluding` lists workspaces that should be skipped during the search
+ /// (e.g. workspaces that are about to be removed).
pub fn find_or_create_local_workspace(
&mut self,
path_list: PathList,
+ excluding: &[Entity<Workspace>],
window: &mut Window,
cx: &mut Context<Self>,
) -> Task<Result<Entity<Workspace>>> {
- if let Some(workspace) = self.workspace_for_paths(&path_list, None, cx) {
+ if let Some(workspace) = self.workspace_for_paths_excluding(&path_list, None, excluding, cx)
+ {
self.activate(workspace.clone(), window, cx);
return Task::ready(Ok(workspace));
}
@@ -1733,7 +1753,7 @@ impl MultiWorkspace {
cx: &mut Context<Self>,
) -> Task<Result<Entity<Workspace>>> {
if self.multi_workspace_enabled(cx) {
- self.find_or_create_local_workspace(PathList::new(&paths), window, cx)
+ self.find_or_create_local_workspace(PathList::new(&paths), &[], window, cx)
} else {
let workspace = self.workspace().clone();
cx.spawn_in(window, async move |_this, cx| {
@@ -433,6 +433,7 @@ async fn test_find_or_create_local_workspace_reuses_active_workspace_when_sideba
.update_in(cx, |mw, window, cx| {
mw.find_or_create_local_workspace(
PathList::new(&[PathBuf::from("/root_a")]),
+ &[],
window,
cx,
)
@@ -491,6 +492,7 @@ async fn test_find_or_create_local_workspace_reuses_active_workspace_after_sideb
.update_in(cx, |mw, window, cx| {
mw.find_or_create_local_workspace(
PathList::new(&[PathBuf::from("/root_a")]),
+ &[],
window,
cx,
)
@@ -2495,6 +2495,7 @@ pub fn delete_unloaded_items(
#[cfg(test)]
mod tests {
use super::*;
+ use crate::PathList;
use crate::ProjectGroupKey;
use crate::{
multi_workspace::MultiWorkspace,
@@ -4989,4 +4990,79 @@ mod tests {
"After removing the only remaining group, should have an empty workspace"
);
}
+
+ /// Regression test for a crash where `find_or_create_local_workspace`
+ /// returned a workspace that was about to be removed, hitting an assert
+ /// in `MultiWorkspace::remove`.
+ ///
+ /// The scenario: two workspaces share the same root paths (e.g. due to
+ /// a provisional key mismatch). When the first is removed and the
+ /// fallback searches for the same paths, `workspace_for_paths` must
+ /// skip the doomed workspace so the assert in `remove` is satisfied.
+ #[gpui::test]
+ async fn test_remove_fallback_skips_excluded_workspaces(cx: &mut gpui::TestAppContext) {
+ crate::tests::init_test(cx);
+ cx.update(|cx| {
+ cx.set_staff(true);
+ cx.update_flags(true, vec!["agent-v2".to_string()]);
+ });
+
+ let fs = fs::FakeFs::new(cx.executor());
+ let dir = unique_test_dir(&fs, "shared").await;
+
+ // Two projects that open the same directory — this creates two
+ // workspaces whose root_paths are identical.
+ let project_a = Project::test(fs.clone(), [dir.as_path()], cx).await;
+ let project_b = Project::test(fs.clone(), [dir.as_path()], cx).await;
+
+ let (multi_workspace, cx) = cx
+ .add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx));
+
+ multi_workspace.update(cx, |mw, cx| mw.open_sidebar(cx));
+
+ let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| {
+ mw.test_add_workspace(project_b.clone(), window, cx)
+ });
+ cx.run_until_parked();
+
+ // workspace_a is first in the workspaces vec.
+ let workspace_a =
+ multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().cloned().unwrap());
+ assert_ne!(workspace_a, workspace_b);
+
+ // Activate workspace_a so removing it triggers the fallback path.
+ multi_workspace.update_in(cx, |mw, window, cx| {
+ mw.activate(workspace_a.clone(), window, cx);
+ });
+ cx.run_until_parked();
+
+ // Remove workspace_a. The fallback searches for the same paths.
+ // Without the `excluding` parameter, `workspace_for_paths` would
+ // return workspace_a (first match) and the assert in `remove`
+ // would fire. With the fix, workspace_a is skipped and
+ // workspace_b is found instead.
+ let path_list = PathList::new(std::slice::from_ref(&dir));
+ let excluded = vec![workspace_a.clone()];
+ multi_workspace.update_in(cx, |mw, window, cx| {
+ mw.remove(
+ vec![workspace_a.clone()],
+ move |this, window, cx| {
+ this.find_or_create_local_workspace(path_list, &excluded, window, cx)
+ },
+ window,
+ cx,
+ )
+ .detach_and_log_err(cx);
+ });
+ cx.run_until_parked();
+
+ // workspace_b should now be active — workspace_a was removed.
+ multi_workspace.read_with(cx, |mw, _cx| {
+ assert_eq!(
+ mw.workspace(),
+ &workspace_b,
+ "fallback should have found workspace_b, not the excluded workspace_a"
+ );
+ });
+ }
}