acp: Fix agent servers sometimes not being registered when Zed starts (#38330)

Cole Miller , Michael , and Agus created 1 month ago

In local projects, initialize the list of agents in the agent server
store immediately. Previously we were initializing the list only after a
delay, in an attempt to avoid sending the `ExternalAgentsUpdated`
message to the downstream client (if any) before its handlers were
initialized. But we already have a separate codepath for that situation,
in the `AgentServerStore::shared`, and we can insert the delay in that
place instead.

Release Notes:

- acp: Fixed a bug where starting an external agent thread soon after
Zed starts up would show a "not registered" error.

---------

Co-authored-by: Michael <[e-mail]>
Co-authored-by: Agus <[e-mail]>

Change summary

crates/edit_prediction_context/src/tree_sitter_index.rs | 14 --
crates/project/src/agent_server_store.rs                | 43 ++++------
crates/project/src/git_store/conflict_set.rs            |  6 
crates/remote_server/src/headless_project.rs            |  2 
4 files changed, 26 insertions(+), 39 deletions(-)

Detailed changes

crates/edit_prediction_context/src/tree_sitter_index.rs

@@ -506,7 +506,6 @@ mod tests {
     use super::*;
     use std::{path::Path, sync::Arc};
 
-    use futures::channel::oneshot;
     use gpui::TestAppContext;
     use indoc::indoc;
     use language::{Language, LanguageConfig, LanguageId, LanguageMatcher, tree_sitter_rust};
@@ -655,17 +654,10 @@ mod tests {
             expect_file_decl("a.rs", &decls[1], &project, cx);
         });
 
-        // Drop the buffer and wait for release
-        let (release_tx, release_rx) = oneshot::channel();
-        cx.update(|cx| {
-            cx.observe_release(&buffer, |_, _| {
-                release_tx.send(()).ok();
-            })
-            .detach();
+        // Need to trigger flush_effects so that the observe_release handler will run.
+        cx.update(|_cx| {
+            drop(buffer);
         });
-        drop(buffer);
-        cx.run_until_parked();
-        release_rx.await.ok();
         cx.run_until_parked();
 
         index.read_with(cx, |index, cx| {

crates/project/src/agent_server_store.rs

@@ -234,7 +234,7 @@ impl AgentServerStore {
         let subscription = cx.observe_global::<SettingsStore>(|this, cx| {
             this.agent_servers_settings_changed(cx);
         });
-        let this = Self {
+        let mut this = Self {
             state: AgentServerStoreState::Local {
                 node_runtime,
                 fs,
@@ -245,14 +245,7 @@ impl AgentServerStore {
             },
             external_agents: Default::default(),
         };
-        cx.spawn(async move |this, cx| {
-            cx.background_executor().timer(Duration::from_secs(1)).await;
-            this.update(cx, |this, cx| {
-                this.agent_servers_settings_changed(cx);
-            })
-            .ok();
-        })
-        .detach();
+        this.agent_servers_settings_changed(cx);
         this
     }
 
@@ -305,22 +298,29 @@ impl AgentServerStore {
         }
     }
 
-    pub fn shared(&mut self, project_id: u64, client: AnyProtoClient) {
+    pub fn shared(&mut self, project_id: u64, client: AnyProtoClient, cx: &mut Context<Self>) {
         match &mut self.state {
             AgentServerStoreState::Local {
                 downstream_client, ..
             } => {
-                client
-                    .send(proto::ExternalAgentsUpdated {
-                        project_id,
-                        names: self
-                            .external_agents
+                *downstream_client = Some((project_id, client.clone()));
+                // Send the current list of external agents downstream, but only after a delay,
+                // to avoid having the message arrive before the downstream project's agent server store
+                // sets up its handlers.
+                cx.spawn(async move |this, cx| {
+                    cx.background_executor().timer(Duration::from_secs(1)).await;
+                    let names = this.update(cx, |this, _| {
+                        this.external_agents
                             .keys()
                             .map(|name| name.to_string())
-                            .collect(),
-                    })
-                    .log_err();
-                *downstream_client = Some((project_id, client));
+                            .collect()
+                    })?;
+                    client
+                        .send(proto::ExternalAgentsUpdated { project_id, names })
+                        .log_err();
+                    anyhow::Ok(())
+                })
+                .detach();
             }
             AgentServerStoreState::Remote { .. } => {
                 debug_panic!(
@@ -721,11 +721,6 @@ struct RemoteExternalAgentServer {
     new_version_available_tx: Option<watch::Sender<Option<String>>>,
 }
 
-// new method: status_updated
-// does nothing in the all-local case
-// for RemoteExternalAgentServer, sends on the stored tx
-// etc.
-
 impl ExternalAgentServer for RemoteExternalAgentServer {
     fn get_command(
         &mut self,

crates/project/src/git_store/conflict_set.rs

@@ -257,7 +257,7 @@ impl EventEmitter<ConflictSetUpdate> for ConflictSet {}
 mod tests {
     use std::{path::Path, sync::mpsc};
 
-    use crate::{Project, project_settings::ProjectSettings};
+    use crate::Project;
 
     use super::*;
     use fs::FakeFs;
@@ -484,7 +484,7 @@ mod tests {
         cx.update(|cx| {
             settings::init(cx);
             WorktreeSettings::register(cx);
-            ProjectSettings::register(cx);
+            Project::init_settings(cx);
             AllLanguageSettings::register(cx);
         });
         let initial_text = "
@@ -585,7 +585,7 @@ mod tests {
         cx.update(|cx| {
             settings::init(cx);
             WorktreeSettings::register(cx);
-            ProjectSettings::register(cx);
+            Project::init_settings(cx);
             AllLanguageSettings::register(cx);
         });
 

crates/remote_server/src/headless_project.rs

@@ -197,7 +197,7 @@ impl HeadlessProject {
         let agent_server_store = cx.new(|cx| {
             let mut agent_server_store =
                 AgentServerStore::local(node_runtime.clone(), fs.clone(), environment, cx);
-            agent_server_store.shared(REMOTE_SERVER_PROJECT_ID, session.clone());
+            agent_server_store.shared(REMOTE_SERVER_PROJECT_ID, session.clone(), cx);
             agent_server_store
         });