From cf8200f9ad92a915bd0de4273c3509025cf8c4f6 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 7 Jan 2026 16:06:17 +0200 Subject: [PATCH] Skip worktree trust queries for remote collab project clients (#46256) Release Notes: - Skip worktree trust queries for remote collab project clients --- .../collab/src/tests/channel_buffer_tests.rs | 6 +- crates/collab/src/tests/editor_tests.rs | 115 ++++++++++++++++++ crates/collab/src/tests/test_server.rs | 27 +++- crates/project/src/project.rs | 13 +- crates/project/src/worktree_store.rs | 26 +++- crates/workspace/src/workspace.rs | 23 +--- crates/worktree/src/worktree.rs | 2 +- 7 files changed, 176 insertions(+), 36 deletions(-) diff --git a/crates/collab/src/tests/channel_buffer_tests.rs b/crates/collab/src/tests/channel_buffer_tests.rs index 62c61d3cf0b22e7adad5ada7ec46598fbadf673c..b0220bc8d2ca85f3dcae5fe4e2e15a2abcdb43fd 100644 --- a/crates/collab/src/tests/channel_buffer_tests.rs +++ b/crates/collab/src/tests/channel_buffer_tests.rs @@ -153,9 +153,9 @@ async fn test_channel_notes_participant_indices( .fs() .insert_tree("/root", json!({"file.txt": "123"})) .await; - let (project_a, worktree_id_a) = client_a.build_local_project("/root", cx_a).await; - let project_b = client_b.build_empty_local_project(cx_b); - let project_c = client_c.build_empty_local_project(cx_c); + let (project_a, worktree_id_a) = client_a.build_local_project_with_trust("/root", cx_a).await; + let project_b = client_b.build_empty_local_project(false, cx_b); + let project_c = client_c.build_empty_local_project(false, cx_c); let (workspace_a, mut cx_a) = client_a.build_workspace(&project_a, cx_a); let (workspace_b, mut cx_b) = client_b.build_workspace(&project_b, cx_b); diff --git a/crates/collab/src/tests/editor_tests.rs b/crates/collab/src/tests/editor_tests.rs index 32ade22ea1a5f9603e39dab40f2934029e4a2359..360a608f478c40052c53417eca6d6d60e917e883 100644 --- a/crates/collab/src/tests/editor_tests.rs +++ b/crates/collab/src/tests/editor_tests.rs @@ -1,5 +1,6 @@ use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer}; use call::ActiveCall; +use collections::{HashMap, HashSet}; use editor::{ DocumentColorsRenderMode, Editor, FETCH_COLORS_DEBOUNCE_TIMEOUT, MultiBufferOffset, RowInfo, SelectionEffects, @@ -26,6 +27,7 @@ use pretty_assertions::assert_eq; use project::{ ProgressToken, ProjectPath, SERVER_PROGRESS_THROTTLE_TIMEOUT, lsp_store::lsp_ext_command::{ExpandedMacro, LspExtExpandMacro}, + trusted_worktrees::{PathTrust, TrustedWorktrees}, }; use recent_projects::disconnected_overlay::DisconnectedOverlay; use rpc::RECEIVE_TIMEOUT; @@ -4639,6 +4641,119 @@ fn extract_color_inlays(editor: &Editor, cx: &App) -> Vec { .collect() } +#[gpui::test] +async fn test_remote_project_worktree_trust(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { + let has_restricted_worktrees = |project: &gpui::Entity, + cx: &mut VisualTestContext| { + cx.update(|_, cx| { + let worktree_store = project.read(cx).worktree_store(); + TrustedWorktrees::try_get_global(cx) + .unwrap() + .read(cx) + .has_restricted_worktrees(&worktree_store, cx) + }) + }; + + cx_a.update(|cx| { + project::trusted_worktrees::init(HashMap::default(), None, None, cx); + }); + cx_b.update(|cx| { + project::trusted_worktrees::init(HashMap::default(), None, None, cx); + }); + + let mut server = TestServer::start(cx_a.executor()).await; + let client_a = server.create_client(cx_a, "user_a").await; + let client_b = server.create_client(cx_b, "user_b").await; + server + .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)]) + .await; + + client_a + .fs() + .insert_tree( + path!("/a"), + json!({ + "file.txt": "contents", + }), + ) + .await; + + let (project_a, worktree_id) = client_a + .build_local_project_with_trust(path!("/a"), cx_a) + .await; + let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a); + let active_call_a = cx_a.read(ActiveCall::global); + let project_id = active_call_a + .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx)) + .await + .unwrap(); + + let project_b = client_b.join_remote_project(project_id, cx_b).await; + let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b); + + let _editor_a = workspace_a + .update_in(cx_a, |workspace, window, cx| { + workspace.open_path( + (worktree_id, rel_path("src/main.rs")), + None, + true, + window, + cx, + ) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + + let _editor_b = workspace_b + .update_in(cx_b, |workspace, window, cx| { + workspace.open_path( + (worktree_id, rel_path("src/main.rs")), + None, + true, + window, + cx, + ) + }) + .await + .unwrap() + .downcast::() + .unwrap(); + + cx_a.run_until_parked(); + cx_b.run_until_parked(); + + assert!( + has_restricted_worktrees(&project_a, cx_a), + "local client should have restricted worktrees after opening it" + ); + assert!( + !has_restricted_worktrees(&project_b, cx_b), + "remote client joined a project should have no restricted worktrees" + ); + + cx_a.update(|_, cx| { + if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) { + trusted_worktrees.update(cx, |trusted_worktrees, cx| { + trusted_worktrees.trust( + &project_a.read(cx).worktree_store(), + HashSet::from_iter([PathTrust::Worktree(worktree_id)]), + cx, + ); + }); + } + }); + assert!( + !has_restricted_worktrees(&project_a, cx_a), + "local client should have no worktrees after trusting those" + ); + assert!( + !has_restricted_worktrees(&project_b, cx_b), + "remote client should still be trusted" + ); +} + fn blame_entry(sha: &str, range: Range) -> git::blame::BlameEntry { git::blame::BlameEntry { sha: sha.parse().unwrap(), diff --git a/crates/collab/src/tests/test_server.rs b/crates/collab/src/tests/test_server.rs index 3abbd1a014b556db02e70b42c239729100f17eb8..0f5c86fdfd332b92a303988dcf125a34f4867d8c 100644 --- a/crates/collab/src/tests/test_server.rs +++ b/crates/collab/src/tests/test_server.rs @@ -745,7 +745,24 @@ impl TestClient { root_path: impl AsRef, cx: &mut TestAppContext, ) -> (Entity, WorktreeId) { - let project = self.build_empty_local_project(cx); + let project = self.build_empty_local_project(false, cx); + let (worktree, _) = project + .update(cx, |p, cx| p.find_or_create_worktree(root_path, true, cx)) + .await + .unwrap(); + worktree + .read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete()) + .await; + cx.run_until_parked(); + (project, worktree.read_with(cx, |tree, _| tree.id())) + } + + pub async fn build_local_project_with_trust( + &self, + root_path: impl AsRef, + cx: &mut TestAppContext, + ) -> (Entity, WorktreeId) { + let project = self.build_empty_local_project(true, cx); let (worktree, _) = project .update(cx, |p, cx| p.find_or_create_worktree(root_path, true, cx)) .await @@ -832,7 +849,11 @@ impl TestClient { self.active_workspace(cx) } - pub fn build_empty_local_project(&self, cx: &mut TestAppContext) -> Entity { + pub fn build_empty_local_project( + &self, + init_worktree_trust: bool, + cx: &mut TestAppContext, + ) -> Entity { cx.update(|cx| { Project::local( self.client().clone(), @@ -841,7 +862,7 @@ impl TestClient { self.app_state.languages.clone(), self.app_state.fs.clone(), None, - false, + init_worktree_trust, cx, ) }) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index bf6e505983bc47b3591a486a61d4f9b6ed489d8f..818909f90f030857bfbbdd9e754f6cad6aec56f7 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4854,11 +4854,6 @@ impl Project { ) -> Result<()> { this.update(&mut cx, |project, cx| { let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) { - trusted_worktrees.update(cx, |trusted_worktrees, cx| { - trusted_worktrees.can_trust(&project.worktree_store, worktree_id, cx) - }); - } if let Some(worktree) = project.worktree_for_id(worktree_id, cx) { worktree.update(cx, |worktree, _| { let worktree = worktree.as_remote_mut().unwrap(); @@ -4891,6 +4886,10 @@ impl Project { envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result { + if this.read_with(&cx, |project, _| project.is_via_collab())? { + return Ok(proto::Ack {}); + } + let trusted_worktrees = cx .update(|cx| TrustedWorktrees::try_get_global(cx))? .context("missing trusted worktrees")?; @@ -4914,6 +4913,10 @@ impl Project { envelope: TypedEnvelope, mut cx: AsyncApp, ) -> Result { + if this.read_with(&cx, |project, _| project.is_via_collab())? { + return Ok(proto::Ack {}); + } + let trusted_worktrees = cx .update(|cx| TrustedWorktrees::try_get_global(cx))? .context("missing trusted worktrees")?; diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index a26740159d0a046a13b2927df0b0555fd0a048a3..7b3ddb657cb10ae407538a9dde34504b36b8d4ea 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -25,7 +25,7 @@ use worktree::{ WorktreeId, }; -use crate::ProjectPath; +use crate::{ProjectPath, trusted_worktrees::TrustedWorktrees}; enum WorktreeStoreState { Local { @@ -469,6 +469,7 @@ impl WorktreeStore { cx: &mut Context, ) -> Task>> { let abs_path: Arc = SanitizedPath::new_arc(&abs_path); + let is_via_collab = matches!(&self.state, WorktreeStoreState::Remote { upstream_client, .. } if upstream_client.is_via_collab()); if !self.loading_worktrees.contains_key(&abs_path) { let task = match &self.state { WorktreeStoreState::Remote { @@ -497,7 +498,28 @@ impl WorktreeStore { this.update(cx, |this, _| this.loading_worktrees.remove(&abs_path)) .ok(); match result { - Ok(worktree) => Ok(worktree), + Ok(worktree) => { + if !is_via_collab { + if let Some((trusted_worktrees, worktree_store)) = this + .update(cx, |_, cx| { + TrustedWorktrees::try_get_global(cx).zip(Some(cx.entity())) + }) + .ok() + .flatten() + { + trusted_worktrees + .update(cx, |trusted_worktrees, cx| { + trusted_worktrees.can_trust( + &worktree_store, + worktree.read(cx).id(), + cx, + ); + }) + .ok(); + } + } + Ok(worktree) + } Err(err) => Err((*err).cloned()), } }) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index ab5e9eb7e3dcc171780606de74d322719d4d2c3e..e82ab432b42ae637030532a243de72d5df2558d9 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -1283,32 +1283,11 @@ impl Workspace { this.collaborator_left(*peer_id, window, cx); } - project::Event::WorktreeUpdatedEntries(worktree_id, _) => { - if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) { - trusted_worktrees.update(cx, |trusted_worktrees, cx| { - trusted_worktrees.can_trust( - &this.project().read(cx).worktree_store(), - *worktree_id, - cx, - ); - }); - } - } - project::Event::WorktreeRemoved(_) => { this.update_worktree_data(window, cx); } - project::Event::WorktreeAdded(worktree_id) => { - if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) { - trusted_worktrees.update(cx, |trusted_worktrees, cx| { - trusted_worktrees.can_trust( - &this.project().read(cx).worktree_store(), - *worktree_id, - cx, - ); - }); - } + project::Event::WorktreeUpdatedEntries(..) | project::Event::WorktreeAdded(..) => { this.update_worktree_data(window, cx); } diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 9c2a448f3aad351470399fd11107028966b6543c..bc94fe69064eb6e6238c223454c580f20168c0bc 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -354,7 +354,7 @@ struct UpdateObservationState { _maintain_remote_snapshot: Task>, } -#[derive(Clone)] +#[derive(Debug, Clone)] pub enum Event { UpdatedEntries(UpdatedEntriesSet), UpdatedGitRepositories(UpdatedGitRepositoriesSet),