From 35ce0efd7f864ae8f08e204166b9adea06b6d5f6 Mon Sep 17 00:00:00 2001 From: Caleul Raposo Caixeta <50340360+Caleul@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:41:04 +0000 Subject: [PATCH] Fix macOS workspace session restore when closing window via X button (#60477) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/zed-industries/zed/issues/48468 ## Solution This change addresses a macOS shutdown ordering issue where closing the last window with the native close button could clear session state before persistence tasks complete, causing next launch to open no project/untitled. - [crates/workspace/src/workspace.rs](/Users/caleul/Documents/Work/Projects/zed/crates/workspace/src/workspace.rs) - Make `save_last_workspace` dynamic on macOS based on whether closing the last window will quit the app (`on_last_window_closed == quit_app`). - Keeps `save_last_workspace` true only when the window close path is equivalent to quit, so last-session data remains valid only in the intended scenario. - [crates/workspace/src/multi_workspace.rs](/Users/caleul/Documents/Work/Projects/zed/crates/workspace/src/multi_workspace.rs) - In `close_window`, before removing the window, explicitly flush: - workspace serialization tasks - pending workspace removal tasks - multi-workspace serialization task - This prevents losing in-memory state on fast close/quit paths. - [crates/session/src/session.rs](/Users/caleul/Documents/Work/Projects/zed/crates/session/src/session.rs) - Guard `app_will_quit` to only persist window stack when non-empty, preventing empty-stack state from being written during close path that destroys the window stack first. ## Testing - Did you test these changes? - Manual repro/fix confirmed in local testing flow by the previous author. - Please confirm after merge with: 1) open 2–3 projects in workspace sidebar 2) set `on_last_window_closed: "quit_app"` and `restore_on_startup: "last_session"` 3) close with red X 4) relaunch app and verify sessions restore all active workspaces - Are there any parts that need more testing? - Multi-window mixed close scenarios and one-window with dirty non-serializable items. - Regression for non-quit close mode (`on_last_window_closed: "close") to ensure behavior remains unchanged. - How can others test this? - Follow the 4-step repro path above on macOS. - Check logs for `timed out waiting on app_will_quit` in `~/Library/Logs/Zed/Zed.log` and confirm no restore regression. - Platforms tested: - Verified for macOS (target path of bug). - Not validated on Linux/Windows in this patch path. ## 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 (no UI/UX diff here) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes - Closing last macOS window via the red close button now correctly preserves and restores last-session workspace state when configured to quit on last-window close. --------- Co-authored-by: Kirill Bulatov --- crates/session/src/session.rs | 5 +- crates/workspace/src/item.rs | 11 ++ crates/workspace/src/multi_workspace.rs | 49 ++++++-- crates/workspace/src/persistence.rs | 131 +++++++++++++++++++++ crates/workspace/src/workspace.rs | 118 +++++++++++++++---- crates/workspace/src/workspace_settings.rs | 12 ++ crates/zed/src/zed.rs | 8 +- 7 files changed, 296 insertions(+), 38 deletions(-) diff --git a/crates/session/src/session.rs b/crates/session/src/session.rs index 04109ce1b2..2c9d340913 100644 --- a/crates/session/src/session.rs +++ b/crates/session/src/session.rs @@ -79,6 +79,7 @@ impl AppSession { let mut current_window_stack = Vec::new(); loop { if let Some(windows) = cx.update(|cx| window_stack(cx)) + && !windows.is_empty() && windows != current_window_stack { store_window_stack(db.clone(), &windows).await; @@ -103,7 +104,9 @@ impl AppSession { } fn app_will_quit(&mut self, cx: &mut Context) -> Task<()> { - if let Some(window_stack) = window_stack(cx) { + if let Some(window_stack) = window_stack(cx) + && !window_stack.is_empty() + { let db = KeyValueStore::global(cx); cx.background_spawn(async move { store_window_stack(db, &window_stack).await }) } else { diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index ddadbf6094..088e12a6dc 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -1457,6 +1457,7 @@ pub mod test { pub save_as_count: usize, pub reload_count: usize, pub is_dirty: bool, + pub save_error: Option, pub buffer_kind: ItemBufferKind, pub has_conflict: bool, pub has_deleted_file: bool, @@ -1549,6 +1550,7 @@ pub mod test { save_as_count: 0, reload_count: 0, is_dirty: false, + save_error: None, has_conflict: false, has_deleted_file: false, capability: Capability::ReadWrite, @@ -1589,6 +1591,11 @@ pub mod test { self } + pub fn with_save_error(mut self, message: impl Into) -> Self { + self.save_error = Some(message.into()); + self + } + pub fn with_conflict(mut self, has_conflict: bool) -> Self { self.has_conflict = has_conflict; self @@ -1743,6 +1750,7 @@ pub mod test { save_as_count: self.save_as_count, reload_count: self.reload_count, is_dirty: self.is_dirty, + save_error: self.save_error.clone(), buffer_kind: self.buffer_kind, has_conflict: self.has_conflict, has_deleted_file: self.has_deleted_file, @@ -1798,6 +1806,9 @@ pub mod test { _window: &mut Window, cx: &mut Context, ) -> Task> { + if let Some(error) = &self.save_error { + return Task::ready(Err(anyhow::anyhow!("{error}"))); + } self.save_count += 1; self.is_dirty = false; for item in &self.project_items { diff --git a/crates/workspace/src/multi_workspace.rs b/crates/workspace/src/multi_workspace.rs index 1fba90b372..283fc9d09a 100644 --- a/crates/workspace/src/multi_workspace.rs +++ b/crates/workspace/src/multi_workspace.rs @@ -564,17 +564,36 @@ impl MultiWorkspace { multi_workspace.workspaces().cloned().collect::>() })?; + let mut prepared = anyhow::Ok(true); for workspace in workspaces { - let should_continue = workspace - .update_in(cx, |workspace, window, cx| { - workspace.prepare_to_close(CloseIntent::CloseWindow, window, cx) - })? - .await?; - if !should_continue { - return anyhow::Ok(()); + prepared = match workspace.update_in(cx, |workspace, window, cx| { + workspace.prepare_to_close(CloseIntent::CloseWindow, window, cx) + }) { + Ok(task) => task.await, + Err(error) => Err(error), + }; + if !matches!(prepared, Ok(true)) { + break; } } + if !matches!(prepared, Ok(true)) { + this.update(cx, |multi_workspace, cx| { + for workspace in multi_workspace.workspaces() { + workspace.update(cx, |workspace, _| { + workspace.removing = false; + }); + } + })?; + prepared?; + return anyhow::Ok(()); + } + + let flush_tasks = this.update_in(cx, |multi_workspace, window, cx| { + multi_workspace.flush_pending_serialization(window, cx) + })?; + futures::future::join_all(flush_tasks).await; + cx.update(|window, _cx| { window.remove_window(); })?; @@ -1483,6 +1502,22 @@ impl MultiWorkspace { self._serialize_task.take().unwrap_or(Task::ready(())) } + pub fn flush_pending_serialization( + &mut self, + window: &mut Window, + cx: &mut Context, + ) -> Vec> { + let mut tasks = Vec::new(); + for workspace in self.workspaces() { + tasks.push(workspace.update(cx, |workspace, cx| { + workspace.flush_serialization(window, cx) + })); + } + tasks.append(&mut self.take_pending_removal_tasks()); + tasks.push(self.flush_serialization()); + tasks + } + fn app_will_quit(&mut self, _cx: &mut Context) -> impl Future + use<> { let mut tasks: Vec> = Vec::new(); if let Some(task) = self._serialize_task.take() { diff --git a/crates/workspace/src/persistence.rs b/crates/workspace/src/persistence.rs index d352a92c23..eeda4cef67 100644 --- a/crates/workspace/src/persistence.rs +++ b/crates/workspace/src/persistence.rs @@ -5118,6 +5118,137 @@ mod tests { ); } + #[gpui::test] + async fn test_close_window_quit_app_preserves_all_sidebar_workspaces( + cx: &mut gpui::TestAppContext, + ) { + crate::tests::init_test(cx); + + cx.update_global::(|store, cx| { + store.update_user_settings(cx, |settings| { + settings.workspace.on_last_window_closed = + Some(settings::OnLastWindowClosed::QuitApp); + }); + }); + + let fs = fs::FakeFs::new(cx.executor()); + let dir1 = unique_test_dir(&fs, "close-quit-a").await; + let dir2 = unique_test_dir(&fs, "close-quit-b").await; + let project1 = Project::test(fs.clone(), [dir1.as_path()], cx).await; + let project2 = Project::test(fs.clone(), [dir2.as_path()], cx).await; + + let db = cx.update(|cx| WorkspaceDb::global(cx)); + let ws1_id = db.next_id().await.unwrap(); + let ws2_id = db.next_id().await.unwrap(); + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project1.clone(), window, cx)); + + multi_workspace.update(cx, |mw, cx| { + mw.open_sidebar(cx); + }); + + let session_id = format!("close-quit-session-{}", Uuid::new_v4()); + + let workspace1 = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + workspace1.update(cx, |ws, _| { + ws.set_database_id(ws1_id); + ws.session_id = Some(session_id.clone()); + }); + + let workspace2 = multi_workspace.update_in(cx, |mw, window, cx| { + let workspace = cx.new(|cx| crate::Workspace::test_new(project2.clone(), window, cx)); + workspace.update(cx, |ws, _| { + ws.set_database_id(ws2_id); + ws.session_id = Some(session_id.clone()); + }); + mw.add(workspace.clone(), window, cx); + workspace + }); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace2.clone(), None, window, cx); + }); + cx.run_until_parked(); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.close_window(&crate::CloseWindow, window, cx); + }); + cx.run_until_parked(); + + let mut restored_ids = db + .last_session_workspace_locations(&session_id, None, fs.as_ref()) + .await + .unwrap() + .into_iter() + .map(|workspace| workspace.workspace_id) + .collect::>(); + restored_ids.sort_by_key(|id| id.0); + assert_eq!(restored_ids, vec![ws1_id, ws2_id]); + } + + #[cfg(target_os = "macos")] + #[gpui::test] + async fn test_close_window_platform_default_still_removes_from_session( + cx: &mut gpui::TestAppContext, + ) { + crate::tests::init_test(cx); + + let fs = fs::FakeFs::new(cx.executor()); + let dir1 = unique_test_dir(&fs, "close-default-a").await; + let dir2 = unique_test_dir(&fs, "close-default-b").await; + let project1 = Project::test(fs.clone(), [dir1.as_path()], cx).await; + let project2 = Project::test(fs.clone(), [dir2.as_path()], cx).await; + + let db = cx.update(|cx| WorkspaceDb::global(cx)); + let ws1_id = db.next_id().await.unwrap(); + let ws2_id = db.next_id().await.unwrap(); + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project1.clone(), window, cx)); + + multi_workspace.update(cx, |mw, cx| { + mw.open_sidebar(cx); + }); + + let session_id = format!("close-default-session-{}", Uuid::new_v4()); + + let workspace1 = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + workspace1.update(cx, |ws, _| { + ws.set_database_id(ws1_id); + ws.session_id = Some(session_id.clone()); + }); + + let workspace2 = multi_workspace.update_in(cx, |mw, window, cx| { + let workspace = cx.new(|cx| crate::Workspace::test_new(project2.clone(), window, cx)); + workspace.update(cx, |ws, _| { + ws.set_database_id(ws2_id); + ws.session_id = Some(session_id.clone()); + }); + mw.add(workspace.clone(), window, cx); + workspace + }); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.activate(workspace2.clone(), None, window, cx); + }); + cx.run_until_parked(); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.close_window(&crate::CloseWindow, window, cx); + }); + cx.run_until_parked(); + + let restored_ids = db + .last_session_workspace_locations(&session_id, None, fs.as_ref()) + .await + .unwrap() + .into_iter() + .map(|workspace| workspace.workspace_id) + .collect::>(); + assert_eq!(restored_ids, Vec::new()); + } + #[gpui::test] async fn test_create_workspace_bounds_observer_uses_fresh_id(cx: &mut gpui::TestAppContext) { crate::tests::init_test(cx); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index b5647de453..82ee3f6e7d 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -157,7 +157,7 @@ use uuid::Uuid; pub use workspace_settings::{ AccessibleMode, AutosaveSetting, BottomDockLayout, EncodingDisplayOptions, FocusFollowsMouse, RestoreOnStartupBehavior, StatusBarSettings, TabBarSettings, WorkspaceSettings, - observe_accessible_mode, + closing_last_window_quits_app, observe_accessible_mode, }; use zed_actions::{Spawn, feedback::FileBugReport, theme::ToggleMode}; @@ -3391,29 +3391,27 @@ impl Workspace { .count() })?; - #[cfg(target_os = "macos")] - let save_last_workspace = false; - - // On Linux and Windows, closing the last window should restore the last workspace. - #[cfg(not(target_os = "macos"))] - let save_last_workspace = { - let remaining_workspaces = cx.update(|_window, cx| { + let (remaining_workspaces, closing_last_window_quits) = cx.update(|window, cx| { + let current_window = window.window_handle(); + let remaining_workspaces = cx.windows() - .iter() + .into_iter() + .filter(|window| *window != current_window) .filter_map(|window| window.downcast::()) .filter_map(|multi_workspace| { - multi_workspace - .update(cx, |multi_workspace, _, cx| { - multi_workspace.workspace().read(cx).removing - }) - .ok() + multi_workspace.read(cx).ok().map(|multi_workspace| { + multi_workspace.workspace().read(cx).removing + }) }) .filter(|removing| !removing) - .count() - })?; + .count(); - close_intent != CloseIntent::ReplaceWindow && remaining_workspaces == 0 - }; + (remaining_workspaces, closing_last_window_quits_app(cx)) + })?; + + let save_last_workspace = close_intent != CloseIntent::ReplaceWindow + && remaining_workspaces == 0 + && closing_last_window_quits; if let Some(active_call) = active_call && workspace_count == 1 @@ -7299,14 +7297,16 @@ impl Workspace { let db = WorkspaceDb::global(cx); let kvp = db::kvp::KeyValueStore::global(cx); window.spawn(cx, async move |_| { - db.set_window_open_status( + let open_status_write = db.set_window_open_status( database_id, window_bounds, display.unwrap_or_default(), - ) - .await - .log_err(); - db.set_session_id(database_id, None).await.log_err(); + ); + let session_id_write = db.set_session_id(database_id, None); + let (open_status, session_id) = + futures::join!(open_status_write, session_id_write); + open_status.log_err(); + session_id.log_err(); persistence::write_default_dock_state(&kvp, docks) .await .log_err(); @@ -12337,6 +12337,78 @@ mod tests { assert!(task.await.unwrap()); } + #[cfg(target_os = "macos")] + #[gpui::test] + async fn test_cancelled_close_window_resets_removing(cx: &mut TestAppContext) { + init_test(cx); + + cx.update(|cx| { + register_serializable_item::(cx); + }); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, None, cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + + let item = cx.new(|cx| { + TestItem::new(cx) + .with_dirty(true) + .with_serialize(|| Some(Task::ready(Ok(())))) + }); + workspace.update_in(cx, |w, window, cx| { + w.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx); + }); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.close_window(&CloseWindow, window, cx); + }); + cx.executor().run_until_parked(); + + assert!(cx.has_pending_prompt()); + cx.simulate_prompt_answer("Cancel"); + cx.executor().run_until_parked(); + + assert!(!workspace.read_with(cx, |workspace, _| workspace.removing)); + let remaining_windows = cx.update(|_window, cx| cx.windows().len()); + assert_eq!(remaining_windows, 1); + } + + #[gpui::test] + async fn test_failed_close_window_resets_removing(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, None, cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + + let item = cx.new(|cx| { + TestItem::new(cx) + .with_dirty(true) + .with_save_error("simulated save failure") + .with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) + }); + workspace.update_in(cx, |w, window, cx| { + w.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx); + }); + + multi_workspace.update_in(cx, |mw, window, cx| { + mw.close_window(&CloseWindow, window, cx); + }); + cx.executor().run_until_parked(); + + assert!(cx.has_pending_prompt()); + cx.simulate_prompt_answer("Save"); + cx.executor().run_until_parked(); + + assert!(!workspace.read_with(cx, |workspace, _| workspace.removing)); + let remaining_windows = cx.update(|_window, cx| cx.windows().len()); + assert_eq!(remaining_windows, 1); + } + #[gpui::test] async fn test_quit_without_worktrees_hot_exits(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/workspace/src/workspace_settings.rs b/crates/workspace/src/workspace_settings.rs index 698e9ffef4..1011579c6e 100644 --- a/crates/workspace/src/workspace_settings.rs +++ b/crates/workspace/src/workspace_settings.rs @@ -44,6 +44,18 @@ pub struct WorkspaceSettings { pub focus_follows_mouse: FocusFollowsMouse, } +#[cfg(target_os = "macos")] +pub fn closing_last_window_quits_app(cx: &App) -> bool { + WorkspaceSettings::get_global(cx) + .on_last_window_closed + .is_quit_app() +} + +#[cfg(not(target_os = "macos"))] +pub fn closing_last_window_quits_app(_cx: &App) -> bool { + true +} + #[derive(Copy, Clone, Deserialize)] pub struct FocusFollowsMouse { pub enabled: bool, diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 1223bb1cc6..f4afcc6930 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -1870,13 +1870,7 @@ fn quit(_: &Quit, cx: &mut App) { for window in &workspace_windows { window .update(cx, |multi_workspace, window, cx| { - for workspace in multi_workspace.workspaces() { - flush_tasks.push(workspace.update(cx, |workspace, cx| { - workspace.flush_serialization(window, cx) - })); - } - flush_tasks.append(&mut multi_workspace.take_pending_removal_tasks()); - flush_tasks.push(multi_workspace.flush_serialization()); + flush_tasks.extend(multi_workspace.flush_pending_serialization(window, cx)); }) .log_err(); }