Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/agent_ui/src/agent_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
142 changes: 141 additions & 1 deletion crates/agent_ui/src/thread_metadata_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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"
);
});
}
}
12 changes: 12 additions & 0 deletions crates/project/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions crates/sidebar/src/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,9 @@ impl Sidebar {
cx: &mut Context<Self>,
) {
let project = workspace.read(cx).project().clone();
if project.read(cx).is_via_collab() {
return;
}

cx.subscribe_in(
&project,
Expand Down Expand Up @@ -576,6 +579,10 @@ impl Sidebar {
old_paths: &WorktreePaths,
cx: &mut Context<Self>,
) {
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();

Expand Down
71 changes: 71 additions & 0 deletions crates/sidebar/src/sidebar_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| <dyn fs::Fs>::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"
);
});
}
Loading