Detailed changes
@@ -1133,8 +1133,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()
{
@@ -2776,11 +2776,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,
@@ -822,12 +822,14 @@ impl MultiWorkspace {
// Now remove the key.
self.project_group_keys.retain(|k| k != 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,
);
@@ -860,10 +862,23 @@ 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>> {
self.workspaces
.iter()
.find(|ws| {
+ if excluding.contains(ws) {
+ return false;
+ }
let key = ws.read(cx).project_group_key(cx);
key.host().as_ref() == host
&& PathList::new(&ws.read(cx).root_paths(cx)) == *path_list
@@ -904,7 +919,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();
@@ -956,19 +971,26 @@ 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));
}
if let Some(transient) = self.active_workspace.transient_workspace() {
- if transient.read(cx).project_group_key(cx).path_list() == &path_list {
+ if transient.read(cx).project_group_key(cx).path_list() == &path_list
+ && !excluding.contains(transient)
+ {
return Task::ready(Ok(transient.clone()));
}
}
@@ -1503,7 +1525,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| {
@@ -2493,6 +2493,7 @@ pub fn delete_unloaded_items(
#[cfg(test)]
mod tests {
use super::*;
+ use crate::PathList;
use crate::{
multi_workspace::MultiWorkspace,
persistence::{
@@ -5014,4 +5015,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"
+ );
+ });
+ }
}