sidebar: Serialize after adding/removing workspaces (#49372)

Before this PR we wouldn't always serialize workspaces when a
mutliworkspace adds/removes a workspace. This PR fixes this by adding a
test and calling serialization in remove

Before you mark this PR as ready for review, make sure that you have:
- [x] Added a solid test coverage and/or screenshots from doing manual
testing
- [x] Done a self-review taking into account security and performance
aspects
- [ ] Aligned any UI changes with the [UI
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)

Release Notes:

- N/A

crates/collab/tests/integration/editor_tests.rs

@@ -107,7 +107,7 @@ async fn test_host_disconnect(
                 cx,
             )
         });
-        MultiWorkspace::new(workspace, cx)
+        MultiWorkspace::new(workspace, window, cx)
     });
     let cx_b = &mut VisualTestContext::from_window(*window_b, cx_b);
     let workspace_b = window_b

crates/collab/tests/integration/git_tests.rs

@@ -67,7 +67,7 @@ async fn test_project_diff(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext)
                 cx,
             )
         });
-        MultiWorkspace::new(workspace, cx)
+        MultiWorkspace::new(workspace, window, cx)
     });
     let cx_b = &mut VisualTestContext::from_window(*window_b, cx_b);
     let workspace_b = window_b

crates/collab/tests/integration/test_server.rs

@@ -886,7 +886,7 @@ impl TestClient {
         let window = cx.add_window(|window, cx| {
             window.activate_window();
             let workspace = cx.new(|cx| Workspace::new(None, project, app_state, window, cx));
-            MultiWorkspace::new(workspace, cx)
+            MultiWorkspace::new(workspace, window, cx)
         });
         let cx = VisualTestContext::from_window(*window, cx).into_mut();
         cx.run_until_parked();
@@ -905,7 +905,7 @@ impl TestClient {
         let window = cx.add_window(|window, cx| {
             window.activate_window();
             let workspace = cx.new(|cx| Workspace::new(None, project, app_state, window, cx));
-            MultiWorkspace::new(workspace, cx)
+            MultiWorkspace::new(workspace, window, cx)
         });
         let cx = VisualTestContext::from_window(*window, cx).into_mut();
         let workspace = window

crates/git_ui/src/worktree_picker.rs

@@ -518,7 +518,7 @@ async fn open_remote_worktree(
                 workspace.centered_layout = workspace_position.centered_layout;
                 workspace
             });
-            cx.new(|cx| MultiWorkspace::new(workspace, cx))
+            cx.new(|cx| MultiWorkspace::new(workspace, window, cx))
         })?
     };
 

crates/recent_projects/src/remote_connections.rs

@@ -233,7 +233,7 @@ pub async fn open_remote_project(
                 workspace.centered_layout = workspace_position.centered_layout;
                 workspace
             });
-            cx.new(|cx| MultiWorkspace::new(workspace, cx))
+            cx.new(|cx| MultiWorkspace::new(workspace, window, cx))
         })?;
         let workspace = window.update(cx, |multi_workspace, _, _cx| {
             multi_workspace.workspace().clone()

crates/recent_projects/src/remote_servers.rs

@@ -495,7 +495,7 @@ impl ProjectPicker {
                                 telemetry::event!("SSH Project Created");
                                 Workspace::new(None, project.clone(), app_state.clone(), window, cx)
                             });
-                            cx.new(|cx| MultiWorkspace::new(workspace, cx))
+                            cx.new(|cx| MultiWorkspace::new(workspace, window, cx))
                         })
                         .log_err()?;
 

crates/settings_ui/src/settings_ui.rs

@@ -4800,7 +4800,7 @@ pub mod test {
                     cx,
                 )
             });
-            MultiWorkspace::new(workspace, cx)
+            MultiWorkspace::new(workspace, window, cx)
         });
 
         let (_multi_workspace2, cx) = cx.add_window_view(|window, cx| {
@@ -4813,7 +4813,7 @@ pub mod test {
                     cx,
                 )
             });
-            MultiWorkspace::new(workspace, cx)
+            MultiWorkspace::new(workspace, window, cx)
         });
 
         let workspace2_handle = cx.window_handle().downcast::<MultiWorkspace>().unwrap();
@@ -4945,7 +4945,7 @@ pub mod test {
                     cx,
                 )
             });
-            MultiWorkspace::new(workspace, cx)
+            MultiWorkspace::new(workspace, window, cx)
         });
 
         let workspace1_handle = cx.window_handle().downcast::<MultiWorkspace>().unwrap();
@@ -4995,7 +4995,7 @@ pub mod test {
                     cx,
                 )
             });
-            MultiWorkspace::new(workspace, cx)
+            MultiWorkspace::new(workspace, window, cx)
         });
 
         cx.run_until_parked();

crates/workspace/src/multi_workspace.rs

@@ -2,8 +2,8 @@ use anyhow::Result;
 use feature_flags::{AgentV2FeatureFlag, FeatureFlagAppExt};
 use gpui::{
     AnyView, App, Context, DragMoveEvent, Entity, EntityId, EventEmitter, FocusHandle, Focusable,
-    ManagedView, MouseButton, Pixels, Render, Subscription, Task, Tiling, Window, actions,
-    deferred, px,
+    ManagedView, MouseButton, Pixels, Render, Subscription, Task, Tiling, Window, WindowId,
+    actions, deferred, px,
 };
 use project::Project;
 use std::path::PathBuf;
@@ -93,6 +93,7 @@ impl<T: Sidebar> SidebarHandle for Entity<T> {
 }
 
 pub struct MultiWorkspace {
+    window_id: WindowId,
     workspaces: Vec<Entity<Workspace>>,
     active_workspace_index: usize,
     sidebar: Option<Box<dyn SidebarHandle>>,
@@ -101,8 +102,9 @@ pub struct MultiWorkspace {
 }
 
 impl MultiWorkspace {
-    pub fn new(workspace: Entity<Workspace>, _cx: &mut Context<Self>) -> Self {
+    pub fn new(workspace: Entity<Workspace>, window: &mut Window, _cx: &mut Context<Self>) -> Self {
         Self {
+            window_id: window.window_handle().window_id(),
             workspaces: vec![workspace],
             active_workspace_index: 0,
             sidebar: None,
@@ -154,7 +156,7 @@ impl MultiWorkspace {
         if self.sidebar_open {
             self.close_sidebar(window, cx);
         } else {
-            self.open_sidebar(window, cx);
+            self.open_sidebar(cx);
             if let Some(sidebar) = &self.sidebar {
                 sidebar.focus(window, cx);
             }
@@ -180,21 +182,21 @@ impl MultiWorkspace {
                 sidebar.focus(window, cx);
             }
         } else {
-            self.open_sidebar(window, cx);
+            self.open_sidebar(cx);
             if let Some(sidebar) = &self.sidebar {
                 sidebar.focus(window, cx);
             }
         }
     }
 
-    pub fn open_sidebar(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+    pub fn open_sidebar(&mut self, cx: &mut Context<Self>) {
         self.sidebar_open = true;
         for workspace in &self.workspaces {
             workspace.update(cx, |workspace, cx| {
                 workspace.set_workspace_sidebar_open(true, cx);
             });
         }
-        self.serialize(window, cx);
+        self.serialize(cx);
         cx.notify();
     }
 
@@ -208,7 +210,7 @@ impl MultiWorkspace {
         let pane = self.workspace().read(cx).active_pane().clone();
         let pane_focus = pane.read(cx).focus_handle(cx);
         window.focus(&pane_focus, cx);
-        self.serialize(window, cx);
+        self.serialize(cx);
         cx.notify();
     }
 
@@ -239,6 +241,7 @@ impl MultiWorkspace {
         let index = self.add_workspace(workspace, cx);
         if self.active_workspace_index != index {
             self.active_workspace_index = index;
+            self.serialize(cx);
             cx.notify();
         }
     }
@@ -266,7 +269,7 @@ impl MultiWorkspace {
             "workspace index out of bounds"
         );
         self.active_workspace_index = index;
-        self.serialize(window, cx);
+        self.serialize(cx);
         self.focus_active_workspace(window, cx);
         cx.notify();
     }
@@ -289,8 +292,8 @@ impl MultiWorkspace {
         }
     }
 
-    fn serialize(&self, window: &mut Window, cx: &mut App) {
-        let window_id = window.window_handle().window_id();
+    fn serialize(&self, cx: &mut App) {
+        let window_id = self.window_id;
         let state = crate::persistence::model::MultiWorkspaceState {
             active_workspace_id: self.workspace().read(cx).database_id(),
             sidebar_open: self.sidebar_open,
@@ -404,7 +407,7 @@ impl MultiWorkspace {
     #[cfg(any(test, feature = "test-support"))]
     pub fn test_new(project: Entity<Project>, window: &mut Window, cx: &mut Context<Self>) -> Self {
         let workspace = cx.new(|cx| Workspace::test_new(project, window, cx));
-        Self::new(workspace, cx)
+        Self::new(workspace, window, cx)
     }
 
     #[cfg(any(test, feature = "test-support"))]
@@ -453,6 +456,7 @@ impl MultiWorkspace {
         }
 
         self.focus_active_workspace(window, cx);
+        self.serialize(cx);
         cx.notify();
     }
 

crates/workspace/src/persistence.rs

@@ -2355,6 +2355,72 @@ mod tests {
     use serde_json::json;
     use std::{thread, time::Duration};
 
+    #[gpui::test]
+    async fn test_multi_workspace_serializes_on_add_and_remove(cx: &mut gpui::TestAppContext) {
+        use crate::multi_workspace::MultiWorkspace;
+        use crate::persistence::read_multi_workspace_state;
+        use feature_flags::FeatureFlagAppExt;
+        use gpui::AppContext as _;
+        use project::Project;
+
+        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 project1 = Project::test(fs.clone(), [], cx).await;
+        let project2 = Project::test(fs.clone(), [], cx).await;
+
+        let (multi_workspace, cx) =
+            cx.add_window_view(|window, cx| MultiWorkspace::test_new(project1.clone(), window, cx));
+
+        multi_workspace.update_in(cx, |mw, _, cx| {
+            mw.set_random_database_id(cx);
+        });
+
+        let window_id =
+            multi_workspace.update_in(cx, |_, window, _cx| window.window_handle().window_id());
+
+        // --- Add a second workspace ---
+        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, _cx| ws.set_random_database_id());
+            mw.activate(workspace.clone(), cx);
+            workspace
+        });
+
+        // Run background tasks so serialize has a chance to flush.
+        cx.run_until_parked();
+
+        // Read back the persisted state and check that the active workspace ID was written.
+        let state_after_add = read_multi_workspace_state(window_id);
+        let active_workspace2_db_id = workspace2.read_with(cx, |ws, _| ws.database_id());
+        assert_eq!(
+            state_after_add.active_workspace_id, active_workspace2_db_id,
+            "After adding a second workspace, the serialized active_workspace_id should match \
+             the newly activated workspace's database id"
+        );
+
+        // --- Remove the second workspace (index 1) ---
+        multi_workspace.update_in(cx, |mw, window, cx| {
+            mw.remove_workspace(1, window, cx);
+        });
+
+        cx.run_until_parked();
+
+        let state_after_remove = read_multi_workspace_state(window_id);
+        let remaining_db_id =
+            multi_workspace.read_with(cx, |mw, cx| mw.workspace().read(cx).database_id());
+        assert_eq!(
+            state_after_remove.active_workspace_id, remaining_db_id,
+            "After removing a workspace, the serialized active_workspace_id should match \
+             the remaining active workspace's database id"
+        );
+    }
+
     #[gpui::test]
     async fn test_breakpoints() {
         zlog::init_test();

crates/workspace/src/workspace.rs

@@ -1876,7 +1876,7 @@ impl Workspace {
 
                                 workspace
                             });
-                            cx.new(|cx| MultiWorkspace::new(workspace, cx))
+                            cx.new(|cx| MultiWorkspace::new(workspace, window, cx))
                         }
                     })?;
                     let workspace =
@@ -7972,8 +7972,8 @@ pub async fn restore_multiworkspace(
 
     if state.sidebar_open {
         window_handle
-            .update(cx, |multi_workspace, window, cx| {
-                multi_workspace.open_sidebar(window, cx);
+            .update(cx, |multi_workspace, _, cx| {
+                multi_workspace.open_sidebar(cx);
             })
             .ok();
     }
@@ -8536,7 +8536,7 @@ pub fn open_workspace_by_id(
                         workspace.centered_layout = centered_layout;
                         workspace
                     });
-                    cx.new(|cx| MultiWorkspace::new(workspace, cx))
+                    cx.new(|cx| MultiWorkspace::new(workspace, window, cx))
                 }
             })?;
 
@@ -9053,7 +9053,7 @@ pub fn join_in_room_project(
                     let workspace = cx.new(|cx| {
                         Workspace::new(Default::default(), project, app_state.clone(), window, cx)
                     });
-                    cx.new(|cx| MultiWorkspace::new(workspace, cx))
+                    cx.new(|cx| MultiWorkspace::new(workspace, window, cx))
                 })
             })?
         };

crates/zed/src/visual_test_runner.rs

@@ -2574,7 +2574,7 @@ fn run_multi_workspace_sidebar_visual_tests(
                         Workspace::new(None, project2.clone(), app_state.clone(), window, cx)
                     });
                     cx.new(|cx| {
-                        let mut multi_workspace = MultiWorkspace::new(workspace1, cx);
+                        let mut multi_workspace = MultiWorkspace::new(workspace1, window, cx);
                         multi_workspace.activate(workspace2, cx);
                         multi_workspace
                     })