Fix ghost project when selecting remote project group (#59272)
# Objective Fixes #54882: When selecting a project group that has a remote host (SSH, WSL, Docker) from the window project picker, or when closing/removing a workspace with a remote neighbor group, the code was unconditionally calling find_or_create_local_workspace. This created a local project with the remote server's paths, producing a 'ghost' project where language servers and file watchers fail trying to access paths that don't exist locally. ## Solution Three locations fixed: 1. RecentProjectsDelegate::confirm (recent_projects.rs) - window project picker selecting a ProjectGroup entry. Routes to find_or_create_workspace with connect_with_modal for remote hosts. 2. MultiWorkspace::close_workspace (multi_workspace.rs) - closing a workspace whose neighboring group is remote. Falls through to the empty workspace fallback instead of creating a ghost project. 3. MultiWorkspace::remove_project_group (multi_workspace.rs) - removing a project group whose neighbor is remote. Same fallback. All three detect remoteness via key.host().is_some() and skip find_or_create_local_workspace when set. Includes regression tests for all three paths using mock remote connections. ## Testing - Not yet tested with a custom build - Unit tests added ## 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 adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase --- Release Notes: - Fixed ghost project appearing in the window project picker and sidebar when switching between local and remote projects. --------- Co-authored-by: Lukas Wirth <me@lukaswirth.dev>
This commit is contained in:
1 parent
cf973e35dd
commit
1635b8254c
3 files changed
+253
-4
No files matched your search
@@ -18,7 +18,7 @@ use fs::Fs;
|
||||
mod wsl_picker;
|
||||
|
||||
use remote::RemoteConnectionOptions;
|
||||
pub use remote_connection::{RemoteConnectionModal, connect};
|
||||
pub use remote_connection::{RemoteConnectionModal, connect, connect_with_modal};
|
||||
pub use remote_connections::{navigate_to_positions, open_remote_project};
|
||||
|
||||
use disconnected_overlay::DisconnectedOverlay;
|
||||
@@ -1192,11 +1192,22 @@ impl PickerDelegate for RecentProjectsDelegate {
|
||||
.log_err();
|
||||
} else {
|
||||
let path_list = key.path_list().clone();
|
||||
let host = key.host();
|
||||
if let Some(task) = handle
|
||||
.update(cx, |multi_workspace, window, cx| {
|
||||
multi_workspace.find_or_create_local_workspace(
|
||||
let modal_workspace = multi_workspace.workspace().clone();
|
||||
multi_workspace.find_or_create_workspace(
|
||||
path_list,
|
||||
host,
|
||||
Some(key.clone()),
|
||||
move |options, window, cx| {
|
||||
connect_with_modal(
|
||||
&modal_workspace,
|
||||
options,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
},
|
||||
&[],
|
||||
None,
|
||||
OpenMode::Activate,
|
||||
@@ -3023,4 +3034,113 @@ mod tests {
|
||||
state
|
||||
})
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_remote_project_group_confirm_does_not_create_local_workspace(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
// Regression test: confirming a ProjectGroup entry with a remote host
|
||||
// should call find_or_create_workspace with the host, not
|
||||
// find_or_create_local_workspace.
|
||||
let app_state = init_test(cx);
|
||||
app_state
|
||||
.fs
|
||||
.as_fake()
|
||||
.insert_tree("/local", json!({}))
|
||||
.await;
|
||||
|
||||
cx.update(|cx| {
|
||||
open_paths(
|
||||
&[PathBuf::from("/local")],
|
||||
app_state,
|
||||
workspace::OpenOptions::default(),
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
let mw = cx.update(|cx| cx.windows()[0].downcast::<MultiWorkspace>().unwrap());
|
||||
let remote_key = remote_project_group(1);
|
||||
|
||||
// Get workspace info via WindowHandle::read_with (returns Result)
|
||||
let (workspace, groups, fh) = mw
|
||||
.read_with(cx, |mw, _cx| {
|
||||
let ws = mw.workspace().clone();
|
||||
(
|
||||
ws.clone(),
|
||||
mw.project_group_keys(),
|
||||
ws.read(_cx).focus_handle(_cx),
|
||||
)
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
let mut augmented_groups = groups.clone();
|
||||
augmented_groups.push(remote_key.clone());
|
||||
|
||||
// Create the popover (same as the title bar does)
|
||||
let popover: Entity<RecentProjects> = cx.update(|cx| {
|
||||
let window = cx.windows()[0];
|
||||
window
|
||||
.update(cx, |_, window, cx| {
|
||||
RecentProjects::popover(
|
||||
workspace.downgrade(),
|
||||
augmented_groups,
|
||||
Some(false),
|
||||
fh,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
// Get the picker from the popover
|
||||
let picker: Entity<Picker<RecentProjectsDelegate>> = cx.update(|cx| {
|
||||
let window = cx.windows()[0];
|
||||
window
|
||||
.update(cx, |_, _window, cx| popover.read(cx).picker.clone())
|
||||
.unwrap()
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
// Find the remote project group entry index via Entity::read_with (no unwrap)
|
||||
let filtered = picker.read_with(cx, |p, _| p.delegate.filtered_entries.clone());
|
||||
let remote_idx = filtered
|
||||
.iter()
|
||||
.position(|entry| {
|
||||
matches!(entry, ProjectPickerEntry::ProjectGroup(m) if m.candidate_id == groups.len())
|
||||
})
|
||||
.expect("remote project group entry should exist");
|
||||
|
||||
// Select and confirm the remote entry via Entity::update
|
||||
let _ = cx.update(|cx| {
|
||||
let window = cx.windows()[0];
|
||||
window.update(cx, |_, window, cx| {
|
||||
picker.update(cx, |picker, cx| {
|
||||
picker.delegate.set_selected_index(remote_idx, window, cx);
|
||||
picker.delegate.confirm(false, window, cx);
|
||||
});
|
||||
})
|
||||
});
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
// Verify no local workspace was created for the remote paths
|
||||
let has_local = mw
|
||||
.read_with(cx, |mw, cx| {
|
||||
mw.workspace_for_paths(remote_key.path_list(), None, cx)
|
||||
.is_some()
|
||||
})
|
||||
.unwrap();
|
||||
assert!(
|
||||
!has_local,
|
||||
"remote project group confirm should not create a local workspace"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -990,7 +990,9 @@ impl MultiWorkspace {
|
||||
.map(|group| group.key.clone())
|
||||
});
|
||||
|
||||
if let Some(neighboring_group_key) = neighboring_group_key {
|
||||
if let Some(neighboring_group_key) = neighboring_group_key
|
||||
&& neighboring_group_key.host().is_none()
|
||||
{
|
||||
return this.find_or_create_local_workspace(
|
||||
neighboring_group_key.path_list().clone(),
|
||||
Some(neighboring_group_key),
|
||||
@@ -1052,7 +1054,9 @@ impl MultiWorkspace {
|
||||
self.remove(
|
||||
workspaces,
|
||||
move |this, window, cx| {
|
||||
if let Some(neighbor_key) = neighbor_key {
|
||||
if let Some(neighbor_key) = neighbor_key
|
||||
&& neighbor_key.host().is_none()
|
||||
{
|
||||
return this.find_or_create_local_workspace(
|
||||
neighbor_key.path_list().clone(),
|
||||
Some(neighbor_key.clone()),
|
||||
|
||||
@@ -942,3 +942,128 @@ async fn test_open_project_closes_empty_workspace_but_not_non_empty_ones(cx: &mu
|
||||
.unwrap();
|
||||
assert!(workspace_a.read_with(cx, |workspace, _cx| workspace.session_id().is_some()),);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_close_workspace_with_remote_neighbor_does_not_create_local_workspace(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
// Regression test: closing a workspace whose neighboring group is
|
||||
// remote with no existing workspace should not create a local
|
||||
// workspace with the remote paths.
|
||||
init_test(cx);
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree("/root_a", json!({ "file.txt": "" })).await;
|
||||
let project_a = Project::test(fs, ["/root_a".as_ref()], cx).await;
|
||||
|
||||
let (multi_workspace, cx) =
|
||||
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a, window, cx));
|
||||
|
||||
multi_workspace.update(cx, |mw, cx| {
|
||||
mw.open_sidebar(cx);
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
// Add a mock-remote group with no workspace as the second group.
|
||||
let remote_key = ProjectGroupKey::new(
|
||||
Some(RemoteConnectionOptions::Mock(
|
||||
remote::MockConnectionOptions { id: 1 },
|
||||
)),
|
||||
PathList::new(&[PathBuf::from("/remote/project")]),
|
||||
);
|
||||
multi_workspace.update(cx, |mw, _cx| {
|
||||
mw.test_add_project_group(ProjectGroup {
|
||||
key: remote_key.clone(),
|
||||
workspaces: Vec::new(),
|
||||
expanded: true,
|
||||
});
|
||||
});
|
||||
|
||||
let workspace_a = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone());
|
||||
|
||||
// Close workspace A. The neighbor is the remote group with no workspace.
|
||||
// The fix should skip find_or_create_local_workspace and fall through
|
||||
// to creating an empty workspace instead.
|
||||
multi_workspace
|
||||
.update_in(cx, |mw, window, cx| {
|
||||
mw.close_workspace(&workspace_a, window, cx)
|
||||
})
|
||||
.await
|
||||
.expect("close_workspace should succeed");
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
multi_workspace.update(cx, |mw, cx| {
|
||||
// The active workspace should NOT be a local workspace with the
|
||||
// remote paths. It should be an empty workspace (no worktrees).
|
||||
let workspaces: Vec<_> = mw.workspaces().cloned().collect();
|
||||
for ws in &workspaces {
|
||||
let key = ws.read(cx).project_group_key(cx);
|
||||
assert!(
|
||||
key.host().is_some()
|
||||
|| key.path_list().paths() != [PathBuf::from("/remote/project")],
|
||||
"remote neighbor should not have created a local workspace"
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_remove_project_group_with_remote_neighbor_does_not_create_local_workspace(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
// Regression test: removing a project group whose neighboring group is
|
||||
// remote with no workspace should not create a local workspace with
|
||||
// the remote paths.
|
||||
init_test(cx);
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree("/root_a", json!({ "file.txt": "" })).await;
|
||||
let project_a = Project::test(fs, ["/root_a".as_ref()], 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);
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
let key_a = project_a.read_with(cx, |p, cx| p.project_group_key(cx));
|
||||
|
||||
// Add a mock-remote group with no workspace.
|
||||
let remote_key = ProjectGroupKey::new(
|
||||
Some(RemoteConnectionOptions::Mock(
|
||||
remote::MockConnectionOptions { id: 1 },
|
||||
)),
|
||||
PathList::new(&[PathBuf::from("/remote/project")]),
|
||||
);
|
||||
multi_workspace.update(cx, |mw, _cx| {
|
||||
mw.test_add_project_group(ProjectGroup {
|
||||
key: remote_key.clone(),
|
||||
workspaces: Vec::new(),
|
||||
expanded: true,
|
||||
});
|
||||
});
|
||||
|
||||
// Remove the local group A. The neighbor is the remote group with no
|
||||
// workspace. The fix should skip find_or_create_local_workspace and
|
||||
// fall through to creating an empty workspace.
|
||||
multi_workspace
|
||||
.update_in(cx, |mw, window, cx| {
|
||||
mw.remove_project_group(&key_a, window, cx)
|
||||
})
|
||||
.await
|
||||
.expect("remove_project_group should succeed");
|
||||
|
||||
cx.run_until_parked();
|
||||
|
||||
multi_workspace.update(cx, |mw, cx| {
|
||||
let workspaces: Vec<_> = mw.workspaces().cloned().collect();
|
||||
for ws in &workspaces {
|
||||
let key = ws.read(cx).project_group_key(cx);
|
||||
assert!(
|
||||
key.host().is_some() || key.path_list().paths() != [PathBuf::from("/remote/project")],
|
||||
"remote neighbor should not have created a local workspace after remove_project_group"
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
Reference in new issue
Block a user