From a36706aed6e7f582f731a4f33ef3b056dac25f36 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 23 Sep 2024 09:11:58 -0600 Subject: [PATCH] Fix up/down project_id confusion (#18099) Release Notes: - ssh remoting: Fix LSP queries run over collab --- crates/project/src/lsp_store.rs | 137 +++++++++++-------- crates/project/src/project.rs | 19 +-- crates/project/src/worktree_store.rs | 103 +++++++++----- crates/remote_server/src/headless_project.rs | 2 +- 4 files changed, 161 insertions(+), 100 deletions(-) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 95ca842360..4506fcc6fe 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -50,7 +50,7 @@ use parking_lot::{Mutex, RwLock}; use postage::watch; use rand::prelude::*; -use rpc::AnyProtoClient; +use rpc::{proto::SSH_PROJECT_ID, AnyProtoClient}; use serde::Serialize; use settings::{Settings, SettingsLocation, SettingsStore}; use sha2::{Digest, Sha256}; @@ -132,6 +132,7 @@ impl LocalLspStore { pub struct RemoteLspStore { upstream_client: AnyProtoClient, + upstream_project_id: u64, } impl RemoteLspStore {} @@ -164,8 +165,7 @@ impl LspStoreMode { pub struct LspStore { mode: LspStoreMode, - downstream_client: Option, - project_id: u64, + downstream_client: Option<(AnyProtoClient, u64)>, nonce: u128, buffer_store: Model, worktree_store: Model, @@ -302,14 +302,16 @@ impl LspStore { } } - pub fn upstream_client(&self) -> Option { + pub fn upstream_client(&self) -> Option<(AnyProtoClient, u64)> { match &self.mode { LspStoreMode::Ssh(SshLspStore { upstream_client, .. - }) - | LspStoreMode::Remote(RemoteLspStore { - upstream_client, .. - }) => Some(upstream_client.clone()), + }) => Some((upstream_client.clone(), SSH_PROJECT_ID)), + LspStoreMode::Remote(RemoteLspStore { + upstream_client, + upstream_project_id, + .. + }) => Some((upstream_client.clone(), *upstream_project_id)), LspStoreMode::Local(_) => None, } } @@ -374,7 +376,6 @@ impl LspStore { }), }), downstream_client: None, - project_id: 0, buffer_store, worktree_store, languages: languages.clone(), @@ -395,10 +396,11 @@ impl LspStore { &self, buffer: Model, client: AnyProtoClient, + upstream_project_id: u64, request: R, cx: &mut ModelContext<'_, LspStore>, ) -> Task::Response>> { - let message = request.to_proto(self.project_id, buffer.read(cx)); + let message = request.to_proto(upstream_project_id, buffer.read(cx)); cx.spawn(move |this, cx| async move { let response = client.request(message).await?; let this = this.upgrade().context("project dropped")?; @@ -413,7 +415,6 @@ impl LspStore { worktree_store: Model, languages: Arc, upstream_client: AnyProtoClient, - project_id: u64, cx: &mut ModelContext, ) -> Self { cx.subscribe(&buffer_store, Self::on_buffer_store_event) @@ -429,7 +430,6 @@ impl LspStore { current_lsp_settings: Default::default(), }), downstream_client: None, - project_id, buffer_store, worktree_store, languages: languages.clone(), @@ -461,9 +461,11 @@ impl LspStore { .detach(); Self { - mode: LspStoreMode::Remote(RemoteLspStore { upstream_client }), + mode: LspStoreMode::Remote(RemoteLspStore { + upstream_client, + upstream_project_id: project_id, + }), downstream_client: None, - project_id, buffer_store, worktree_store, languages: languages.clone(), @@ -768,13 +770,13 @@ impl LspStore { } pub(crate) fn send_diagnostic_summaries(&self, worktree: &mut Worktree) { - if let Some(client) = self.downstream_client.clone() { + if let Some((client, downstream_project_id)) = self.downstream_client.clone() { if let Some(summaries) = self.diagnostic_summaries.get(&worktree.id()) { for (path, summaries) in summaries { for (&server_id, summary) in summaries { client .send(proto::UpdateDiagnosticSummary { - project_id: self.project_id, + project_id: downstream_project_id, worktree_id: worktree.id().to_proto(), summary: Some(summary.to_proto(server_id, path)), }) @@ -798,8 +800,14 @@ impl LspStore { { let buffer = buffer_handle.read(cx); - if let Some(upstream_client) = self.upstream_client() { - return self.send_lsp_proto_request(buffer_handle, upstream_client, request, cx); + if let Some((upstream_client, upstream_project_id)) = self.upstream_client() { + return self.send_lsp_proto_request( + buffer_handle, + upstream_client, + upstream_project_id, + request, + cx, + ); } let language_server = match server { @@ -1077,9 +1085,9 @@ impl LspStore { push_to_history: bool, cx: &mut ModelContext, ) -> Task> { - if let Some(upstream_client) = self.upstream_client() { + if let Some((upstream_client, project_id)) = self.upstream_client() { let request = proto::ApplyCodeAction { - project_id: self.project_id, + project_id, buffer_id: buffer_handle.read(cx).remote_id().into(), action: Some(Self::serialize_code_action(&action)), }; @@ -1163,9 +1171,9 @@ impl LspStore { server_id: LanguageServerId, cx: &mut ModelContext, ) -> Task> { - if let Some(upstream_client) = self.upstream_client() { + if let Some((upstream_client, project_id)) = self.upstream_client() { let request = proto::ResolveInlayHint { - project_id: self.project_id, + project_id, buffer_id: buffer_handle.read(cx).remote_id().into(), language_server_id: server_id.0 as u64, hint: Some(InlayHints::project_to_proto_hint(hint.clone())), @@ -1274,9 +1282,9 @@ impl LspStore { trigger: String, cx: &mut ModelContext, ) -> Task>> { - if let Some(client) = self.upstream_client() { + if let Some((client, project_id)) = self.upstream_client() { let request = proto::OnTypeFormatting { - project_id: self.project_id, + project_id, buffer_id: buffer.read(cx).remote_id().into(), position: Some(serialize_anchor(&position)), trigger, @@ -1424,11 +1432,11 @@ impl LspStore { range: Range, cx: &mut ModelContext, ) -> Task> { - if let Some(upstream_client) = self.upstream_client() { + if let Some((upstream_client, project_id)) = self.upstream_client() { let request_task = upstream_client.request(proto::MultiLspQuery { buffer_id: buffer_handle.read(cx).remote_id().into(), version: serialize_version(&buffer_handle.read(cx).version()), - project_id: self.project_id, + project_id, strategy: Some(proto::multi_lsp_query::Strategy::All( proto::AllLanguageServers {}, )), @@ -1437,7 +1445,7 @@ impl LspStore { range: range.clone(), kinds: None, } - .to_proto(self.project_id, buffer_handle.read(cx)), + .to_proto(project_id, buffer_handle.read(cx)), )), }); let buffer = buffer_handle.clone(); @@ -1504,10 +1512,11 @@ impl LspStore { ) -> Task>> { let language_registry = self.languages.clone(); - if let Some(upstream_client) = self.upstream_client() { + if let Some((upstream_client, project_id)) = self.upstream_client() { let task = self.send_lsp_proto_request( buffer.clone(), upstream_client, + project_id, GetCompletions { position, context }, cx, ); @@ -1603,14 +1612,13 @@ impl LspStore { ) -> Task> { let client = self.upstream_client(); let language_registry = self.languages.clone(); - let project_id = self.project_id; let buffer_id = buffer.read(cx).remote_id(); let buffer_snapshot = buffer.read(cx).snapshot(); cx.spawn(move |this, cx| async move { let mut did_resolve = false; - if let Some(client) = client { + if let Some((client, project_id)) = client { for completion_index in completion_indices { let (server_id, completion) = { let completions_guard = completions.read(); @@ -1811,8 +1819,7 @@ impl LspStore { let buffer = buffer_handle.read(cx); let buffer_id = buffer.remote_id(); - if let Some(client) = self.upstream_client() { - let project_id = self.project_id; + if let Some((client, project_id)) = self.upstream_client() { cx.spawn(move |_, mut cx| async move { let response = client .request(proto::ApplyCompletionAdditionalEdits { @@ -1927,9 +1934,9 @@ impl LspStore { let buffer_id = buffer.remote_id().into(); let lsp_request = InlayHints { range }; - if let Some(client) = self.upstream_client() { + if let Some((client, project_id)) = self.upstream_client() { let request = proto::InlayHints { - project_id: self.project_id, + project_id, buffer_id, start: Some(serialize_anchor(&range_start)), end: Some(serialize_anchor(&range_end)), @@ -1977,16 +1984,16 @@ impl LspStore { ) -> Task> { let position = position.to_point_utf16(buffer.read(cx)); - if let Some(client) = self.upstream_client() { + if let Some((client, upstream_project_id)) = self.upstream_client() { let request_task = client.request(proto::MultiLspQuery { buffer_id: buffer.read(cx).remote_id().into(), version: serialize_version(&buffer.read(cx).version()), - project_id: self.project_id, + project_id: upstream_project_id, strategy: Some(proto::multi_lsp_query::Strategy::All( proto::AllLanguageServers {}, )), request: Some(proto::multi_lsp_query::Request::GetSignatureHelp( - GetSignatureHelp { position }.to_proto(self.project_id, buffer.read(cx)), + GetSignatureHelp { position }.to_proto(upstream_project_id, buffer.read(cx)), )), }); let buffer = buffer.clone(); @@ -2049,16 +2056,16 @@ impl LspStore { position: PointUtf16, cx: &mut ModelContext, ) -> Task> { - if let Some(client) = self.upstream_client() { + if let Some((client, upstream_project_id)) = self.upstream_client() { let request_task = client.request(proto::MultiLspQuery { buffer_id: buffer.read(cx).remote_id().into(), version: serialize_version(&buffer.read(cx).version()), - project_id: self.project_id, + project_id: upstream_project_id, strategy: Some(proto::multi_lsp_query::Strategy::All( proto::AllLanguageServers {}, )), request: Some(proto::multi_lsp_query::Request::GetHover( - GetHover { position }.to_proto(self.project_id, buffer.read(cx)), + GetHover { position }.to_proto(upstream_project_id, buffer.read(cx)), )), }); let buffer = buffer.clone(); @@ -2123,9 +2130,9 @@ impl LspStore { pub fn symbols(&self, query: &str, cx: &mut ModelContext) -> Task>> { let language_registry = self.languages.clone(); - if let Some(upstream_client) = self.upstream_client().as_ref() { + if let Some((upstream_client, project_id)) = self.upstream_client().as_ref() { let request = upstream_client.request(proto::GetProjectSymbols { - project_id: self.project_id, + project_id: *project_id, query: query.to_string(), }); cx.foreground_executor().spawn(async move { @@ -2598,8 +2605,7 @@ impl LspStore { downstream_client: AnyProtoClient, _: &mut ModelContext, ) { - self.project_id = project_id; - self.downstream_client = Some(downstream_client.clone()); + self.downstream_client = Some((downstream_client.clone(), project_id)); for (server_id, status) in &self.language_server_statuses { downstream_client @@ -2857,10 +2863,10 @@ impl LspStore { } if !old_summary.is_empty() || !new_summary.is_empty() { - if let Some(downstream_client) = &self.downstream_client { + if let Some((downstream_client, project_id)) = &self.downstream_client { downstream_client .send(proto::UpdateDiagnosticSummary { - project_id: self.project_id, + project_id: *project_id, worktree_id: worktree_id.to_proto(), summary: Some(proto::DiagnosticSummary { path: worktree_path.to_string_lossy().to_string(), @@ -2881,9 +2887,9 @@ impl LspStore { symbol: &Symbol, cx: &mut ModelContext, ) -> Task>> { - if let Some(client) = self.upstream_client() { + if let Some((client, project_id)) = self.upstream_client() { let request = client.request(proto::OpenBufferForSymbol { - project_id: self.project_id, + project_id, symbol: Some(Self::serialize_symbol(symbol)), }); cx.spawn(move |this, mut cx| async move { @@ -3184,6 +3190,17 @@ impl LspStore { envelope: TypedEnvelope, mut cx: AsyncAppContext, ) -> Result { + let response_from_ssh = this.update(&mut cx, |this, _| { + let ssh = this.as_ssh()?; + let mut payload = envelope.payload.clone(); + payload.project_id = SSH_PROJECT_ID; + + Some(ssh.upstream_client.request(payload)) + })?; + if let Some(response_from_ssh) = response_from_ssh { + return response_from_ssh.await; + } + let sender_id = envelope.original_sender_id().unwrap_or_default(); let buffer_id = BufferId::new(envelope.payload.buffer_id)?; let version = deserialize_version(&envelope.payload.version); @@ -4779,10 +4796,11 @@ impl LspStore { // TODO: We should use `adapter` here instead of reaching through the `CachedLspAdapter`. let lsp_adapter = adapter.adapter.clone(); - let project_id = self.project_id; + let Some((upstream_client, project_id)) = self.upstream_client() else { + return; + }; let worktree_id = worktree.read(cx).id().to_proto(); - let upstream_client = ssh.upstream_client.clone(); - let name = adapter.name(); + let name = adapter.name().to_string(); let Some(available_language) = self.languages.available_language_for_name(&language) else { log::error!("failed to find available language {language}"); @@ -5165,12 +5183,11 @@ impl LspStore { } }); - let project_id = self.project_id; for (worktree_id, summaries) in self.diagnostic_summaries.iter_mut() { summaries.retain(|path, summaries_by_server_id| { if summaries_by_server_id.remove(&server_id).is_some() { - if let Some(downstream_client) = self.downstream_client.clone() { - downstream_client + if let Some((client, project_id)) = self.downstream_client.clone() { + client .send(proto::UpdateDiagnosticSummary { project_id, worktree_id: worktree_id.to_proto(), @@ -5236,9 +5253,9 @@ impl LspStore { buffers: impl IntoIterator>, cx: &mut ModelContext, ) { - if let Some(client) = self.upstream_client() { + if let Some((client, project_id)) = self.upstream_client() { let request = client.request(proto::RestartLanguageServers { - project_id: self.project_id, + project_id, buffer_ids: buffers .into_iter() .map(|b| b.read(cx).remote_id().to_proto()) @@ -5694,9 +5711,9 @@ impl LspStore { async move { this.update(&mut cx, |this, cx| { cx.emit(LspStoreEvent::RefreshInlayHints); - this.downstream_client.as_ref().map(|client| { + this.downstream_client.as_ref().map(|(client, project_id)| { client.send(proto::RefreshInlayHints { - project_id: this.project_id, + project_id: *project_id, }) }) })? @@ -6073,9 +6090,9 @@ impl LspStore { cx.emit(LspStoreEvent::LanguageServerAdded(server_id)); - if let Some(downstream_client) = self.downstream_client.as_ref() { + if let Some((downstream_client, project_id)) = self.downstream_client.as_ref() { downstream_client.send(proto::StartLanguageServer { - project_id: self.project_id, + project_id: *project_id, server: Some(proto::LanguageServer { id: server_id.0 as u64, name: language_server.name().to_string(), diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 78584cbae0..0c54a16187 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -625,7 +625,7 @@ impl Project { let snippets = SnippetProvider::new(fs.clone(), BTreeSet::from_iter([global_snippets_dir]), cx); - let worktree_store = cx.new_model(|_| WorktreeStore::new(None, false, fs.clone())); + let worktree_store = cx.new_model(|_| WorktreeStore::local(false, fs.clone())); cx.subscribe(&worktree_store, Self::on_worktree_store_event) .detach(); @@ -722,7 +722,7 @@ impl Project { SnippetProvider::new(fs.clone(), BTreeSet::from_iter([global_snippets_dir]), cx); let worktree_store = - cx.new_model(|_| WorktreeStore::new(Some(ssh.clone().into()), false, fs.clone())); + cx.new_model(|_| WorktreeStore::remote(false, ssh.clone().into(), 0, None)); cx.subscribe(&worktree_store, Self::on_worktree_store_event) .detach(); @@ -744,7 +744,6 @@ impl Project { worktree_store.clone(), languages.clone(), ssh.clone().into(), - 0, cx, ) }); @@ -874,11 +873,15 @@ impl Project { let role = response.payload.role(); let worktree_store = cx.new_model(|_| { - let mut store = WorktreeStore::new(Some(client.clone().into()), true, fs.clone()); - if let Some(dev_server_project_id) = response.payload.dev_server_project_id { - store.set_dev_server_project_id(DevServerProjectId(dev_server_project_id)); - } - store + WorktreeStore::remote( + true, + client.clone().into(), + response.payload.project_id, + response + .payload + .dev_server_project_id + .map(DevServerProjectId), + ) })?; let buffer_store = cx.new_model(|cx| BufferStore::new(worktree_store.clone(), Some(remote_id), cx))?; diff --git a/crates/project/src/worktree_store.rs b/crates/project/src/worktree_store.rs index 5c3b2a00a9..9f25572fc7 100644 --- a/crates/project/src/worktree_store.rs +++ b/crates/project/src/worktree_store.rs @@ -36,19 +36,27 @@ struct MatchingEntry { respond: oneshot::Sender, } +enum WorktreeStoreState { + Local { + fs: Arc, + }, + Remote { + dev_server_project_id: Option, + upstream_client: AnyProtoClient, + upstream_project_id: u64, + }, +} + pub struct WorktreeStore { next_entry_id: Arc, - upstream_client: Option, - downstream_client: Option, - remote_id: u64, - dev_server_project_id: Option, + downstream_client: Option<(AnyProtoClient, u64)>, retain_worktrees: bool, worktrees: Vec, worktrees_reordered: bool, #[allow(clippy::type_complexity)] loading_worktrees: HashMap, Shared, Arc>>>>, - fs: Arc, + state: WorktreeStoreState, } pub enum WorktreeStoreEvent { @@ -69,27 +77,37 @@ impl WorktreeStore { client.add_model_request_handler(Self::handle_expand_project_entry); } - pub fn new( - upstream_client: Option, - retain_worktrees: bool, - fs: Arc, - ) -> Self { + pub fn local(retain_worktrees: bool, fs: Arc) -> Self { Self { next_entry_id: Default::default(), loading_worktrees: Default::default(), - dev_server_project_id: None, downstream_client: None, worktrees: Vec::new(), worktrees_reordered: false, retain_worktrees, - remote_id: 0, - upstream_client, - fs, + state: WorktreeStoreState::Local { fs }, } } - pub fn set_dev_server_project_id(&mut self, id: DevServerProjectId) { - self.dev_server_project_id = Some(id); + pub fn remote( + retain_worktrees: bool, + upstream_client: AnyProtoClient, + upstream_project_id: u64, + dev_server_project_id: Option, + ) -> Self { + Self { + next_entry_id: Default::default(), + loading_worktrees: Default::default(), + downstream_client: None, + worktrees: Vec::new(), + worktrees_reordered: false, + retain_worktrees, + state: WorktreeStoreState::Remote { + upstream_client, + upstream_project_id, + dev_server_project_id, + }, + } } /// Iterates through all worktrees, including ones that don't appear in the project panel @@ -159,14 +177,28 @@ impl WorktreeStore { ) -> Task>> { let path: Arc = abs_path.as_ref().into(); if !self.loading_worktrees.contains_key(&path) { - let task = if let Some(client) = self.upstream_client.clone() { - if let Some(dev_server_project_id) = self.dev_server_project_id { - self.create_dev_server_worktree(client, dev_server_project_id, abs_path, cx) - } else { - self.create_ssh_worktree(client, abs_path, visible, cx) + let task = match &self.state { + WorktreeStoreState::Remote { + upstream_client, + dev_server_project_id, + .. + } => { + if let Some(dev_server_project_id) = dev_server_project_id { + self.create_dev_server_worktree( + upstream_client.clone(), + *dev_server_project_id, + abs_path, + cx, + ) + } else if upstream_client.is_via_collab() { + Task::ready(Err(Arc::new(anyhow!("cannot create worktrees via collab")))) + } else { + self.create_ssh_worktree(upstream_client.clone(), abs_path, visible, cx) + } + } + WorktreeStoreState::Local { fs } => { + self.create_local_worktree(fs.clone(), abs_path, visible, cx) } - } else { - self.create_local_worktree(abs_path, visible, cx) }; self.loading_worktrees.insert(path.clone(), task.shared()); @@ -236,11 +268,11 @@ impl WorktreeStore { fn create_local_worktree( &mut self, + fs: Arc, abs_path: impl AsRef, visible: bool, cx: &mut ModelContext, ) -> Task, Arc>> { - let fs = self.fs.clone(); let next_entry_id = self.next_entry_id.clone(); let path: Arc = abs_path.as_ref().into(); @@ -374,6 +406,17 @@ impl WorktreeStore { self.worktrees_reordered = worktrees_reordered; } + fn upstream_client(&self) -> Option<(AnyProtoClient, u64)> { + match &self.state { + WorktreeStoreState::Remote { + upstream_client, + upstream_project_id, + .. + } => Some((upstream_client.clone(), *upstream_project_id)), + WorktreeStoreState::Local { .. } => None, + } + } + pub fn set_worktrees_from_proto( &mut self, worktrees: Vec, @@ -389,8 +432,8 @@ impl WorktreeStore { }) .collect::>(); - let client = self - .upstream_client + let (client, project_id) = self + .upstream_client() .clone() .ok_or_else(|| anyhow!("invalid project"))?; @@ -408,7 +451,7 @@ impl WorktreeStore { self.worktrees.push(handle); } else { self.add( - &Worktree::remote(self.remote_id, replica_id, worktree, client.clone(), cx), + &Worktree::remote(project_id, replica_id, worktree, client.clone(), cx), cx, ); } @@ -477,10 +520,9 @@ impl WorktreeStore { } pub fn send_project_updates(&mut self, cx: &mut ModelContext) { - let Some(downstream_client) = self.downstream_client.clone() else { + let Some((downstream_client, project_id)) = self.downstream_client.clone() else { return; }; - let project_id = self.remote_id; let update = proto::UpdateProject { project_id, @@ -549,8 +591,7 @@ impl WorktreeStore { cx: &mut ModelContext, ) { self.retain_worktrees = true; - self.remote_id = remote_id; - self.downstream_client = Some(downsteam_client); + self.downstream_client = Some((downsteam_client, remote_id)); // When shared, retain all worktrees for worktree_handle in self.worktrees.iter_mut() { diff --git a/crates/remote_server/src/headless_project.rs b/crates/remote_server/src/headless_project.rs index 9d5c26d6c7..0d644a64a6 100644 --- a/crates/remote_server/src/headless_project.rs +++ b/crates/remote_server/src/headless_project.rs @@ -45,7 +45,7 @@ impl HeadlessProject { let languages = Arc::new(LanguageRegistry::new(cx.background_executor().clone())); let worktree_store = cx.new_model(|cx| { - let mut store = WorktreeStore::new(None, true, fs.clone()); + let mut store = WorktreeStore::local(true, fs.clone()); store.shared(SSH_PROJECT_ID, session.clone().into(), cx); store });