diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index ce1c9ed231f3718561dadb5c09989daf56b63c2e..85a88b8444d9c1d07564018c73c5265f25ca5b95 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -191,7 +191,7 @@ fn migrate_thread_remote_connections(cx: &mut App, migration_task: Task::default(); let mut remote_path_lists = HashMap::::default(); diff --git a/crates/agent_ui/src/threads_archive_view.rs b/crates/agent_ui/src/threads_archive_view.rs index 333a2d8e68c734a3bc440544901ccfd6b65e7043..dbe916d85f299bb0013c338084bc2ebda6b307f4 100644 --- a/crates/agent_ui/src/threads_archive_view.rs +++ b/crates/agent_ui/src/threads_archive_view.rs @@ -1079,7 +1079,7 @@ impl ProjectPickerModal { let db = WorkspaceDb::global(cx); cx.spawn_in(window, async move |this, cx| { let workspaces = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .log_err() .unwrap_or_default(); diff --git a/crates/recent_projects/src/recent_projects.rs b/crates/recent_projects/src/recent_projects.rs index 7eb924b7d12910d630e8f90e4ae64c10ea6d1300..8b7c16e4b598584e8d04890f79f254400e258110 100644 --- a/crates/recent_projects/src/recent_projects.rs +++ b/crates/recent_projects/src/recent_projects.rs @@ -96,7 +96,7 @@ pub async fn get_recent_projects( db: &WorkspaceDb, ) -> Vec { let workspaces = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .unwrap_or_default(); @@ -610,7 +610,7 @@ impl RecentProjects { cx.spawn_in(window, async move |this, cx| { let Some(fs) = fs else { return }; let workspaces = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .log_err() .unwrap_or_default(); @@ -2039,7 +2039,7 @@ impl RecentProjectsDelegate { db.delete_workspace_by_id(workspace_id).await.log_err(); let Some(fs) = fs else { return }; let workspaces = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .unwrap_or_default(); let workspaces = diff --git a/crates/recent_projects/src/sidebar_recent_projects.rs b/crates/recent_projects/src/sidebar_recent_projects.rs index 7c50e5ca3959874a26866d83e49aa799289ed69e..f19531c70705261c55bcb33bf4e7eb84dc2209c7 100644 --- a/crates/recent_projects/src/sidebar_recent_projects.rs +++ b/crates/recent_projects/src/sidebar_recent_projects.rs @@ -70,7 +70,7 @@ impl SidebarRecentProjects { cx.spawn_in(window, async move |this, cx| { let Some(fs) = fs else { return }; let workspaces = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .log_err() .unwrap_or_default(); diff --git a/crates/workspace/src/history_manager.rs b/crates/workspace/src/history_manager.rs index 9b03a3252d32793e12495817c2d9801d610d3ce4..8e60939a9c25bed03056102c2e1779e3e6c48d6a 100644 --- a/crates/workspace/src/history_manager.rs +++ b/crates/workspace/src/history_manager.rs @@ -44,7 +44,7 @@ impl HistoryManager { let db = WorkspaceDb::global(cx); cx.spawn(async move |cx| { let recent_folders = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .unwrap_or_default() .into_iter() diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index 6d79404adb0af3b51784f216ecb05ac70a543608..7248abe9b8dba7568174adfd828cede2fa16f250 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -66,6 +66,14 @@ fn parse_timestamp(text: &str) -> DateTime { .unwrap_or_else(|_| Utc::now()) } +fn contains_wsl_path(paths: &PathList) -> bool { + cfg!(windows) + && paths + .paths() + .iter() + .any(|path| util::paths::WslPath::from_path(path).is_some()) +} + #[derive(Copy, Clone, Debug, PartialEq)] pub(crate) struct SerializedAxis(pub(crate) gpui::Axis); impl sqlez::bindable::StaticColumnCount for SerializedAxis {} @@ -1740,26 +1748,30 @@ impl WorkspaceDb { WorkspaceId, PathList, Option, + Option, DateTime, )>, > { Ok(self .recent_workspaces_query()? .into_iter() - .map(|(id, paths, order, remote_connection_id, timestamp)| { - ( - id, - PathList::deserialize(&SerializedPathList { paths, order }), - remote_connection_id.map(RemoteConnectionId), - parse_timestamp(×tamp), - ) - }) + .map( + |(id, paths, order, remote_connection_id, session_id, timestamp)| { + ( + id, + PathList::deserialize(&SerializedPathList { paths, order }), + remote_connection_id.map(RemoteConnectionId), + session_id, + parse_timestamp(×tamp), + ) + }, + ) .collect()) } query! { - fn recent_workspaces_query() -> Result, String)>> { - SELECT workspace_id, paths, paths_order, remote_connection_id, timestamp + fn recent_workspaces_query() -> Result, Option, String)>> { + SELECT workspace_id, paths, paths_order, remote_connection_id, session_id, timestamp FROM workspaces WHERE paths IS NOT NULL OR @@ -1921,9 +1933,7 @@ impl WorkspaceDb { let mut any_dir = false; for path in paths { match fs.metadata(path).await.ok().flatten() { - None => { - return false; - } + None => return false, Some(meta) => { if meta.is_dir { any_dir = true; @@ -1934,9 +1944,10 @@ impl WorkspaceDb { any_dir } - // Returns the recent locations which are still valid on disk and deletes ones which no longer - // exist. - pub async fn recent_workspaces_on_disk( + // Returns the recent project workspaces suitable for showing in the recent-projects UI. + // Scratch workspaces (no paths) are filtered out - they aren't really "projects" and + // are restored separately by `last_session_workspace_locations`. + pub async fn recent_project_workspaces( &self, fs: &dyn Fs, ) -> Result< @@ -1947,11 +1958,9 @@ impl WorkspaceDb { DateTime, )>, > { - let mut result = Vec::new(); - let mut workspaces_to_delete = Vec::new(); let remote_connections = self.remote_connections()?; - let now = Utc::now(); - for (id, paths, remote_connection_id, timestamp) in self.recent_workspaces()? { + let mut result = Vec::new(); + for (id, paths, remote_connection_id, _session_id, timestamp) in self.recent_workspaces()? { if let Some(remote_connection_id) = remote_connection_id { if let Some(connection_options) = remote_connections.get(&remote_connection_id) { result.push(( @@ -1960,7 +1969,44 @@ impl WorkspaceDb { paths, timestamp, )); - } else { + } + continue; + } + + if paths.paths().is_empty() || contains_wsl_path(&paths) { + continue; + } + + if Self::all_paths_exist_with_a_directory(paths.paths(), fs).await { + result.push((id, SerializedWorkspaceLocation::Local, paths, timestamp)); + } + } + Ok(result) + } + + // Deletes workspace rows that can no longer be restored from. Remote workspaces whose + // connection was removed, and (on Windows) workspaces pointing at WSL paths, are cleaned + // up immediately. Local workspaces with no valid paths on disk are kept for seven days + // after going stale. Workspaces belonging to the current session or the last session are + // always preserved so that an in-progress restore can rehydrate them. + pub async fn garbage_collect_workspaces( + &self, + fs: &dyn Fs, + current_session_id: &str, + last_session_id: Option<&str>, + ) -> Result<()> { + let remote_connections = self.remote_connections()?; + let now = Utc::now(); + let mut workspaces_to_delete = Vec::new(); + for (id, paths, remote_connection_id, session_id, timestamp) in self.recent_workspaces()? { + if let Some(session_id) = session_id.as_deref() { + if session_id == current_session_id || Some(session_id) == last_session_id { + continue; + } + } + + if let Some(remote_connection_id) = remote_connection_id { + if !remote_connections.contains_key(&remote_connection_id) { workspaces_to_delete.push(id); } continue; @@ -1971,20 +2017,14 @@ impl WorkspaceDb { // will wait for the WSL VM and file server to boot up. This can // block for many seconds. Supported scenarios use remote // workspaces. - if cfg!(windows) { - let has_wsl_path = paths - .paths() - .iter() - .any(|path| util::paths::WslPath::from_path(path).is_some()); - if has_wsl_path { - workspaces_to_delete.push(id); - continue; - } + if contains_wsl_path(&paths) { + workspaces_to_delete.push(id); + continue; } - if Self::all_paths_exist_with_a_directory(paths.paths(), fs).await { - result.push((id, SerializedWorkspaceLocation::Local, paths, timestamp)); - } else if now - timestamp >= chrono::Duration::days(7) { + if !Self::all_paths_exist_with_a_directory(paths.paths(), fs).await + && now - timestamp >= chrono::Duration::days(7) + { workspaces_to_delete.push(id); } } @@ -1995,7 +2035,7 @@ impl WorkspaceDb { .map(|id| self.delete_workspace_by_id(id)), ) .await; - Ok(result) + Ok(()) } pub async fn last_workspace( @@ -2009,7 +2049,7 @@ impl WorkspaceDb { DateTime, )>, > { - Ok(self.recent_workspaces_on_disk(fs).await?.into_iter().next()) + Ok(self.recent_project_workspaces(fs).await?.into_iter().next()) } // Returns the locations of the workspaces that were still opened when the last @@ -2038,23 +2078,16 @@ impl WorkspaceDb { paths, window_id, }); - } else if paths.is_empty() { - // Empty workspace with items (drafts, files) - include for restoration + continue; + } + + if paths.is_empty() || Self::all_paths_exist_with_a_directory(paths.paths(), fs).await { workspaces.push(SessionWorkspace { workspace_id, location: SerializedWorkspaceLocation::Local, paths, window_id, }); - } else { - if Self::all_paths_exist_with_a_directory(paths.paths(), fs).await { - workspaces.push(SessionWorkspace { - workspace_id, - location: SerializedWorkspaceLocation::Local, - paths, - window_id, - }); - } } } @@ -2268,6 +2301,15 @@ impl WorkspaceDb { } } + #[cfg(test)] + query! { + pub(crate) async fn set_timestamp_for_tests(workspace_id: WorkspaceId, timestamp: String) -> Result<()> { + UPDATE workspaces + SET timestamp = ?2 + WHERE workspace_id = ?1 + } + } + query! { pub(crate) async fn set_window_open_status(workspace_id: WorkspaceId, bounds: SerializedWindowBounds, display: Uuid) -> Result<()> { UPDATE workspaces @@ -3580,6 +3622,228 @@ mod tests { ); } + fn pane_with_items(item_ids: &[ItemId]) -> SerializedPaneGroup { + SerializedPaneGroup::Pane(SerializedPane::new( + item_ids + .iter() + .map(|id| SerializedItem::new("Terminal", *id, true, false)) + .collect(), + true, + 0, + )) + } + + fn empty_pane_group() -> SerializedPaneGroup { + SerializedPaneGroup::Pane(SerializedPane::default()) + } + + fn workspace_with( + id: u64, + paths: &[&Path], + center_group: SerializedPaneGroup, + session_id: Option<&str>, + ) -> SerializedWorkspace { + SerializedWorkspace { + id: WorkspaceId(id as i64), + paths: PathList::new(paths), + location: SerializedWorkspaceLocation::Local, + center_group, + window_bounds: Default::default(), + display: Default::default(), + docks: Default::default(), + bookmarks: Default::default(), + breakpoints: Default::default(), + centered_layout: false, + session_id: session_id.map(|s| s.to_owned()), + window_id: Some(id), + user_toolchains: Default::default(), + } + } + + #[gpui::test] + async fn test_scratch_only_workspace_restores_from_last_session(cx: &mut gpui::TestAppContext) { + let fs = fs::FakeFs::new(cx.executor()); + let db = + WorkspaceDb::open_test_db("test_scratch_only_workspace_restores_from_last_session") + .await; + + db.save_workspace(workspace_with(1, &[], pane_with_items(&[100]), Some("s1"))) + .await; + + let sessions = db + .last_session_workspace_locations("s1", None, fs.as_ref()) + .await + .unwrap(); + assert_eq!(sessions.len(), 1); + assert_eq!(sessions[0].workspace_id, WorkspaceId(1)); + assert!(sessions[0].paths.is_empty()); + + let recents = db.recent_project_workspaces(fs.as_ref()).await.unwrap(); + assert!( + recents.iter().all(|(id, ..)| *id != WorkspaceId(1)), + "scratch-only workspace must not appear in the recent-projects UI" + ); + } + + #[gpui::test] + async fn test_gc_preserves_scratch_inside_window(cx: &mut gpui::TestAppContext) { + let fs = fs::FakeFs::new(cx.executor()); + let db = WorkspaceDb::open_test_db("test_gc_preserves_scratch_inside_window").await; + + db.save_workspace(workspace_with(1, &[], empty_pane_group(), None)) + .await; + + db.garbage_collect_workspaces(fs.as_ref(), "current", None) + .await + .unwrap(); + assert!( + db.workspace_for_id(WorkspaceId(1)).is_some(), + "fresh stale workspace must not be deleted before the 7-day window" + ); + } + + #[gpui::test] + async fn test_gc_deletes_stale_outside_window(cx: &mut gpui::TestAppContext) { + let fs = fs::FakeFs::new(cx.executor()); + let db = WorkspaceDb::open_test_db("test_gc_deletes_stale_outside_window").await; + + db.save_workspace(workspace_with(1, &[], empty_pane_group(), None)) + .await; + db.set_timestamp_for_tests(WorkspaceId(1), "2000-01-01 00:00:00".to_owned()) + .await + .unwrap(); + + db.garbage_collect_workspaces(fs.as_ref(), "current", None) + .await + .unwrap(); + assert!( + db.workspace_for_id(WorkspaceId(1)).is_none(), + "stale empty workspace older than the retention window must be deleted" + ); + } + + #[gpui::test] + async fn test_gc_preserves_directory_workspace_with_missing_path( + cx: &mut gpui::TestAppContext, + ) { + let fs = fs::FakeFs::new(cx.executor()); + let db = + WorkspaceDb::open_test_db("test_gc_preserves_directory_workspace_with_missing_path") + .await; + + let missing_dir = PathBuf::from("/missing-project-dir"); + db.save_workspace(workspace_with( + 1, + &[missing_dir.as_path()], + empty_pane_group(), + None, + )) + .await; + + db.garbage_collect_workspaces(fs.as_ref(), "current", None) + .await + .unwrap(); + assert!( + db.workspace_for_id(WorkspaceId(1)).is_some(), + "a stale workspace within the retention window must be kept" + ); + + db.set_timestamp_for_tests(WorkspaceId(1), "2000-01-01 00:00:00".to_owned()) + .await + .unwrap(); + db.garbage_collect_workspaces(fs.as_ref(), "current", None) + .await + .unwrap(); + assert!( + db.workspace_for_id(WorkspaceId(1)).is_none(), + "a stale workspace past the retention window must be deleted" + ); + } + + #[gpui::test] + async fn test_gc_preserves_current_and_last_sessions(cx: &mut gpui::TestAppContext) { + let fs = fs::FakeFs::new(cx.executor()); + let db = WorkspaceDb::open_test_db("test_gc_preserves_current_and_last_sessions").await; + + db.save_workspace(workspace_with(1, &[], empty_pane_group(), Some("current"))) + .await; + db.save_workspace(workspace_with(2, &[], empty_pane_group(), Some("last"))) + .await; + db.save_workspace(workspace_with(3, &[], empty_pane_group(), Some("stale"))) + .await; + + for id in [1, 2, 3] { + db.set_timestamp_for_tests(WorkspaceId(id), "2000-01-01 00:00:00".to_owned()) + .await + .unwrap(); + } + + db.garbage_collect_workspaces(fs.as_ref(), "current", Some("last")) + .await + .unwrap(); + + assert!( + db.workspace_for_id(WorkspaceId(1)).is_some(), + "GC must not delete workspaces belonging to the current session" + ); + assert!( + db.workspace_for_id(WorkspaceId(2)).is_some(), + "GC must not delete workspaces belonging to the last session" + ); + assert!( + db.workspace_for_id(WorkspaceId(3)).is_none(), + "GC should still delete stale workspaces from other sessions" + ); + } + + #[gpui::test] + async fn test_gc_deletes_empty_workspace_with_items(cx: &mut gpui::TestAppContext) { + let fs = fs::FakeFs::new(cx.executor()); + let db = WorkspaceDb::open_test_db("test_gc_deletes_empty_workspace_with_items").await; + + db.save_workspace(workspace_with(1, &[], pane_with_items(&[100]), None)) + .await; + db.set_timestamp_for_tests(WorkspaceId(1), "2000-01-01 00:00:00".to_owned()) + .await + .unwrap(); + + db.garbage_collect_workspaces(fs.as_ref(), "current", None) + .await + .unwrap(); + assert!( + db.workspace_for_id(WorkspaceId(1)).is_none(), + "a stale empty-path workspace must be deleted regardless of its items" + ); + } + + #[gpui::test] + async fn test_last_session_restores_workspace_with_missing_paths( + cx: &mut gpui::TestAppContext, + ) { + let fs = fs::FakeFs::new(cx.executor()); + let db = + WorkspaceDb::open_test_db("test_last_session_restores_workspace_with_missing_paths") + .await; + + let missing = PathBuf::from("/gone/file.rs"); + db.save_workspace(workspace_with( + 1, + &[missing.as_path()], + empty_pane_group(), + Some("s"), + )) + .await; + + let sessions = db + .last_session_workspace_locations("s", None, fs.as_ref()) + .await + .unwrap(); + assert!( + sessions.is_empty(), + "workspaces whose paths no longer exist on disk must not restore" + ); + } + #[gpui::test] async fn test_last_session_workspace_locations_remote(cx: &mut gpui::TestAppContext) { let fs = fs::FakeFs::new(cx.executor()); diff --git a/crates/workspace/src/welcome.rs b/crates/workspace/src/welcome.rs index a13ec56b2e07a81667fe096a8780393d93bf6f48..de189d89c9a21940dc848a535a83fff9aa33110f 100644 --- a/crates/workspace/src/welcome.rs +++ b/crates/workspace/src/welcome.rs @@ -271,7 +271,7 @@ impl WelcomePage { cx.spawn_in(window, async move |this: WeakEntity, cx| { let Some(fs) = fs else { return }; let workspaces = db - .recent_workspaces_on_disk(fs.as_ref()) + .recent_project_workspaces(fs.as_ref()) .await .log_err() .unwrap_or_default(); diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index f0bc7d557d5199d4b9599d09eebb4962e5eb6bbb..15c426b3b0db0707e7ba928d98ee2a28e0af00e8 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -21,7 +21,9 @@ use fs::{Fs, RealFs}; use futures::{StreamExt, channel::oneshot, future}; use git::GitHostingProviderRegistry; use git_ui::clone::clone_and_open; -use gpui::{App, AppContext, Application, AsyncApp, Focusable as _, QuitMode, UpdateGlobal as _}; +use gpui::{ + App, AppContext, Application, AsyncApp, Focusable as _, QuitMode, Task, UpdateGlobal as _, +}; use gpui_platform; use gpui_tokio::Tokio; @@ -850,26 +852,47 @@ fn main() { }) } - match open_rx + let (current_session_id, last_session_id) = { + let session = app_state.session.read(cx); + ( + session.id().to_owned(), + session.last_session_id().map(|id| id.to_owned()), + ) + }; + + let restore_task = match open_rx .try_recv() .ok() .and_then(|request| OpenRequest::parse(request, cx).log_err()) { Some(request) => { handle_open_request(request, app_state.clone(), cx); + Task::ready(()) } - None => { - cx.spawn({ - let app_state = app_state.clone(); - async move |cx| { - if let Err(e) = restore_or_create_workspace(app_state, cx).await { - fail_to_open_window_async(e, cx) - } + None => cx.spawn({ + let app_state = app_state.clone(); + async move |cx| { + if let Err(e) = restore_or_create_workspace(app_state, cx).await { + fail_to_open_window_async(e, cx) } - }) - .detach(); + } + }), + }; + + cx.spawn({ + let db = workspace::WorkspaceDb::global(cx); + let fs = app_state.fs.clone(); + async move |_cx| { + restore_task.await; + db.garbage_collect_workspaces( + fs.as_ref(), + ¤t_session_id, + last_session_id.as_deref(), + ) + .await } - } + }) + .detach_and_log_err(cx); let app_state = app_state.clone(); diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index a3d668c3f845c553685adb292927532ea732f760..e599f60820648655a0ebb6372e7cad40255ca602 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -3028,6 +3028,10 @@ mod tests { let window_is_edited = |window: WindowHandle, cx: &mut TestAppContext| { cx.update(|cx| window.read(cx).unwrap().workspace().read(cx).is_edited()) }; + let workspace_database_id = |window: WindowHandle, + cx: &mut TestAppContext| { + cx.update(|cx| window.read(cx).unwrap().workspace().read(cx).database_id()) + }; let editor = window .read_with(cx, |multi_workspace, cx| { @@ -3042,6 +3046,11 @@ mod tests { .unwrap(); assert!(!window_is_edited(window, cx)); + let initial_database_id = workspace_database_id(window, cx); + assert!( + initial_database_id.is_some(), + "a restored workspace must have a stable database id" + ); // Editing a buffer marks the window as edited. window @@ -3087,6 +3096,11 @@ mod tests { .unwrap() }); assert!(window_is_edited(window, cx)); + assert_eq!( + workspace_database_id(window, cx), + initial_database_id, + "the workspace must keep the same database id across a close/reopen cycle" + ); window .update(cx, |multi_workspace, _, cx| {