From dbb057b192ca4b7e2fd3d1905a74063f4b6e8862 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Thu, 16 Apr 2026 22:05:57 +0200 Subject: [PATCH] agent_ui: Do not save thread metadata for collab projects (#54094) Make sure that we never write thread metadata to the DB for a thread running on a collab project. When joining a collab project and starting a native thread we would set all your threads that had no paths associated with to the path of the collab project (in `move_thread_paths`). 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 is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --- crates/agent_ui/src/agent_panel.rs | 4 + crates/agent_ui/src/thread_metadata_store.rs | 142 ++++++++++++++++++- crates/project/src/project.rs | 12 ++ crates/sidebar/src/sidebar.rs | 7 + crates/sidebar/src/sidebar_tests.rs | 71 ++++++++++ 5 files changed, 235 insertions(+), 1 deletion(-) diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index ccdefc2b762749272b2ad0731e8d99c47096be23..38527d8cd3c6680894e417ad188bb1ca754ae85b 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -2120,6 +2120,10 @@ impl AgentPanel { }); } + if self.project.read(cx).is_via_collab() { + return; + } + // Update metadata store so threads' path lists stay in sync with // the project's current worktrees. Without this, threads saved // before a worktree was added would have stale paths and not diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 768951c8f4b8dae4904963d45d0f49d22b230167..0b99694159cd1c51bb7776336d6d7b2f6e03341d 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -1138,7 +1138,7 @@ impl ThreadMetadataStore { }; let thread_ref = thread.read(cx); - if thread_ref.is_draft_thread() { + if thread_ref.is_draft_thread() || thread_ref.project().read(cx).is_via_collab() { return; } @@ -3747,4 +3747,144 @@ mod tests { ); }); } + + #[gpui::test] + async fn test_collab_guest_threads_not_saved_to_metadata_store(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [Path::new("/project-a")], cx).await; + + let (panel, mut vcx) = setup_panel_with_project(project.clone(), cx); + crate::test_support::open_thread_with_connection( + &panel, + StubAgentConnection::new(), + &mut vcx, + ); + let thread = panel.read_with(&vcx, |panel, cx| panel.active_agent_thread(cx).unwrap()); + let thread_id = crate::test_support::active_thread_id(&panel, &vcx); + thread.update_in(&mut vcx, |thread, _window, cx| { + thread.push_user_content_block(None, "hello".into(), cx); + thread.set_title("Thread".into(), cx).detach(); + }); + vcx.run_until_parked(); + + // Confirm the thread is in the store while the project is local. + cx.update(|cx| { + let store = ThreadMetadataStore::global(cx); + assert!( + store.read(cx).entry(thread_id).is_some(), + "thread must be in the store while the project is local" + ); + }); + + cx.update(|cx| { + let store = ThreadMetadataStore::global(cx); + store.update(cx, |store, cx| { + store.delete(thread_id, cx); + }); + }); + project.update(cx, |project, _cx| { + project.mark_as_collab_for_testing(); + }); + + thread.update_in(&mut vcx, |thread, _window, cx| { + thread.push_user_content_block(None, "more content".into(), cx); + }); + vcx.run_until_parked(); + + cx.update(|cx| { + let store = ThreadMetadataStore::global(cx); + assert!( + store.read(cx).entry(thread_id).is_none(), + "threads must not be persisted while the project is a collab guest session" + ); + }); + } + + // When a worktree is added to a collab project, update_thread_work_dirs + // fires with the new worktree paths. Without an is_via_collab() guard it + // overwrites the stored paths of any retained or active local threads with + // the new (expanded) path set, corrupting metadata that belonged to the + // guest's own local project. + #[gpui::test] + async fn test_collab_guest_retained_thread_paths_not_overwritten_on_worktree_change( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/project-a", serde_json::json!({})).await; + fs.insert_tree("/project-b", serde_json::json!({})).await; + let project = Project::test(fs, [Path::new("/project-a")], cx).await; + + let (panel, mut vcx) = setup_panel_with_project(project.clone(), cx); + + // Open thread A and give it content so its metadata is saved with /project-a. + crate::test_support::open_thread_with_connection( + &panel, + StubAgentConnection::new(), + &mut vcx, + ); + let thread_a_id = crate::test_support::active_thread_id(&panel, &vcx); + let thread_a = panel.read_with(&vcx, |panel, cx| panel.active_agent_thread(cx).unwrap()); + thread_a.update_in(&mut vcx, |thread, _window, cx| { + thread.push_user_content_block(None, "hello".into(), cx); + thread.set_title("Thread A".into(), cx).detach(); + }); + vcx.run_until_parked(); + + cx.update(|cx| { + let store = ThreadMetadataStore::global(cx); + let entry = store.read(cx).entry(thread_a_id).unwrap(); + assert_eq!( + entry.folder_paths().paths(), + &[std::path::PathBuf::from("/project-a")], + "thread A must be saved with /project-a before collab" + ); + }); + + // Open thread B, making thread A a retained thread in the panel. + crate::test_support::open_thread_with_connection( + &panel, + StubAgentConnection::new(), + &mut vcx, + ); + vcx.run_until_parked(); + + // Transition the project into collab mode (simulates joining as a guest). + project.update(cx, |project, _cx| { + project.mark_as_collab_for_testing(); + }); + + // Add a second worktree. For a real collab guest this would be one of + // the host's worktrees arriving via the collab protocol, but here we + // use a local path because the test infrastructure cannot easily produce + // a remote worktree with a fully-scanned root entry. + // + // This fires WorktreeAdded → update_thread_work_dirs. Without an + // is_via_collab() guard that call overwrites the stored paths of + // retained thread A from {/project-a} to {/project-a, /project-b}, + // polluting its metadata with a path it never belonged to. + project + .update(cx, |project, cx| { + project.find_or_create_worktree(Path::new("/project-b"), true, cx) + }) + .await + .unwrap(); + vcx.run_until_parked(); + + cx.update(|cx| { + let store = ThreadMetadataStore::global(cx); + let entry = store + .read(cx) + .entry(thread_a_id) + .expect("thread A must still exist in the store"); + assert_eq!( + entry.folder_paths().paths(), + &[std::path::PathBuf::from("/project-a")], + "retained thread A's stored path must not be updated while the project is via collab" + ); + }); + } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 83e069a0f80363234ce3e149f44a61aa786b0138..bb128388ae3bd7f29d3bd39260a56eaf38adbebc 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2075,6 +2075,18 @@ impl Project { project } + /// Transitions a local test project into the `Collab` client state so that + /// `is_via_collab()` returns `true`. Use only in tests. + #[cfg(any(test, feature = "test-support"))] + pub fn mark_as_collab_for_testing(&mut self) { + self.client_state = ProjectClientState::Collab { + sharing_has_stopped: false, + capability: Capability::ReadWrite, + remote_id: 0, + replica_id: clock::ReplicaId::new(1), + }; + } + #[cfg(any(test, feature = "test-support"))] pub fn add_test_remote_worktree( &mut self, diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index ba29102e569029013404357a699bba33483bd9f5..c8bfa93368ac2607393d2bad1283058866371efd 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -510,6 +510,9 @@ impl Sidebar { cx: &mut Context, ) { let project = workspace.read(cx).project().clone(); + if project.read(cx).is_via_collab() { + return; + } cx.subscribe_in( &project, @@ -576,6 +579,10 @@ impl Sidebar { old_paths: &WorktreePaths, cx: &mut Context, ) { + if project.read(cx).is_via_collab() { + return; + } + let new_paths = project.read(cx).worktree_paths(cx); let old_folder_paths = old_paths.folder_path_list().clone(); diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 5cb836ebedb5ff2de3d177116555a1b53b586981..d687ae5066f74997967559ec5126bdfbf90ddb3d 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -10297,3 +10297,74 @@ fn test_worktree_info_missing_branch_returns_none() { assert_eq!(infos[0].branch_name, None); assert_eq!(infos[0].name, SharedString::from("myapp")); } + +#[gpui::test] +async fn test_collab_guest_move_thread_paths_is_noop(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/project-a", serde_json::json!({ "src": {} })) + .await; + fs.insert_tree("/project-b", serde_json::json!({ "src": {} })) + .await; + cx.update(|cx| ::set_global(fs.clone(), cx)); + let project = project::Project::test(fs, ["/project-a".as_ref()], cx).await; + + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + + // Set up the sidebar while the project is local. This registers the + // WorktreePathsChanged subscription for the project. + let _sidebar = setup_sidebar(&multi_workspace, cx); + + let session_id = acp::SessionId::new(Arc::from("test-thread")); + save_named_thread_metadata("test-thread", "My Thread", &project, cx).await; + + let thread_id = cx.update(|_window, cx| { + ThreadMetadataStore::global(cx) + .read(cx) + .entry_by_session(&session_id) + .map(|e| e.thread_id) + .expect("thread must be in the store") + }); + + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx); + let entry = store.read(cx).entry(thread_id).unwrap(); + assert_eq!( + entry.folder_paths().paths(), + &[PathBuf::from("/project-a")], + "thread must be saved with /project-a before collab" + ); + }); + + // Transition the project into collab mode. The sidebar's subscription is + // still active from when the project was local. + project.update(cx, |project, _cx| { + project.mark_as_collab_for_testing(); + }); + + // Adding a worktree fires WorktreePathsChanged with old_paths = {/project-a}. + // The sidebar's subscription is still active, so move_thread_paths is called. + // Without the is_via_collab() guard inside move_thread_paths, this would + // update the stored thread paths from {/project-a} to {/project-a, /project-b}. + project + .update(cx, |project, cx| { + project.find_or_create_worktree("/project-b", true, cx) + }) + .await + .expect("should add worktree"); + cx.run_until_parked(); + + cx.update(|_window, cx| { + let store = ThreadMetadataStore::global(cx); + let entry = store + .read(cx) + .entry(thread_id) + .expect("thread must still exist"); + assert_eq!( + entry.folder_paths().paths(), + &[PathBuf::from("/project-a")], + "thread path must not change when project is via collab" + ); + }); +}