acp: Prevent registry loading from hanging indefinitely (#56108)

Helps #51567
Refs #54531

Summary:

- Add total timeouts for ACP Registry JSON fetches and icon fetches,
including response body reads.
- Download registry icons concurrently and keep icon failures non-fatal,
so a blocked icon CDN does not delay registry availability by one
timeout per agent.
- Surface the stored registry fetch error in the ACP Registry empty
state and add a retry action.

This addresses cases where the registry request, or one of the icon
requests, never finishes. It does not make blocked networks succeed, but
it prevents the UI from sitting on `Loading registry...` indefinitely
and gives the user something actionable instead.

Test plan:

- `git diff --check HEAD~1..HEAD`
- `cargo fmt --check --package project --package agent_ui`
- `cargo check -p project`
- `cargo check -p agent_ui`
- `cargo test -p project --features test-support registry_refresh_`

Release Notes:

- Fixed the ACP Registry getting stuck on loading when registry or icon
requests hang.

---------

Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>

crates/agent_ui/src/agent_registry_ui.rs

@@ -292,10 +292,12 @@ impl AgentRegistryPage {
     fn render_empty_state(&self, cx: &mut Context<Self>) -> impl IntoElement {
         let has_search = self.search_query(cx).is_some();
         let registry_store = self.registry_store.read(cx);
+        let is_fetching = registry_store.is_fetching();
+        let fetch_error = registry_store.fetch_error();
 
-        let message = if registry_store.is_fetching() {
+        let message = if is_fetching {
             "Loading registry..."
-        } else if registry_store.fetch_error().is_some() {
+        } else if fetch_error.is_some() {
             "Failed to load the agent registry. Please check your connection and try again."
         } else {
             match self.filter {
@@ -325,15 +327,42 @@ impl AgentRegistryPage {
 
         h_flex()
             .py_4()
+            .min_w_0()
+            .w_full()
             .gap_1p5()
-            .when(registry_store.fetch_error().is_some(), |this| {
+            .items_start()
+            .when(fetch_error.is_some(), |this| {
                 this.child(
                     Icon::new(IconName::Warning)
                         .size(IconSize::Small)
                         .color(Color::Warning),
                 )
             })
-            .child(Label::new(message))
+            .child(
+                v_flex()
+                    .min_w_0()
+                    .flex_1()
+                    .gap_1()
+                    .child(Label::new(message))
+                    .when_some(fetch_error.clone(), |this, fetch_error| {
+                        this.child(
+                            Label::new(fetch_error)
+                                .size(LabelSize::Small)
+                                .color(Color::Muted),
+                        )
+                    }),
+            )
+            .when_some(fetch_error, |this, _| {
+                let registry_store = self.registry_store.clone();
+                this.child(
+                    Button::new("retry-agent-registry", "Retry")
+                        .style(ButtonStyle::Outlined)
+                        .size(ButtonSize::Compact)
+                        .on_click(move |_, _, cx| {
+                            registry_store.update(cx, |store, cx| store.refresh(cx));
+                        }),
+                )
+            })
     }
 
     fn render_agents(

crates/project/src/agent_registry_store.rs

@@ -2,19 +2,27 @@ use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
-use anyhow::{Context as _, Result, bail};
+use anyhow::{Context as _, Result, anyhow, bail};
 use collections::HashMap;
 use fs::Fs;
-use futures::AsyncReadExt;
-use gpui::{App, AppContext as _, Context, Entity, Global, SharedString, Task, TaskExt};
-use http_client::{AsyncBody, HttpClient};
+use futures::{AsyncReadExt, future::join_all};
+use gpui::{
+    App, AppContext as _, BackgroundExecutor, Context, Entity, FutureExt as _, Global,
+    SharedString, Task, TaskExt,
+};
+use http_client::{AsyncBody, HttpClient, StatusCode};
 use serde::Deserialize;
 use settings::Settings as _;
+use util::ResultExt;
 
 use crate::{AgentId, DisableAiSettings};
 
 const REGISTRY_URL: &str = "https://cdn.agentclientprotocol.com/registry/v1/latest/registry.json";
 const REFRESH_THROTTLE_DURATION: Duration = Duration::from_secs(60 * 60);
+// Bound the full request lifecycle, including response body reads; the shared
+// HTTP client only has a connect timeout.
+const REGISTRY_FETCH_TIMEOUT: Duration = Duration::from_secs(30);
+const REGISTRY_ICON_FETCH_TIMEOUT: Duration = Duration::from_secs(10);
 
 #[derive(Clone, Debug)]
 pub struct RegistryAgentMetadata {
@@ -209,12 +217,20 @@ impl AgentRegistryStore {
 
         let fs = self.fs.clone();
         let http_client = self.http_client.clone();
+        let executor = cx.background_executor().clone();
 
         self.pending_refresh = Some(cx.spawn(async move |this, cx| {
-            let result = match fetch_registry_index(http_client.clone()).await {
+            let result = match fetch_registry_index(http_client.clone(), &executor).await {
                 Ok(data) => {
-                    build_registry_agents(fs.clone(), http_client, data.index, data.raw_body, true)
-                        .await
+                    build_registry_agents(
+                        fs.clone(),
+                        http_client,
+                        data.index,
+                        data.raw_body,
+                        true,
+                        &executor,
+                    )
+                    .await
                 }
                 Err(error) => {
                     log::error!("AgentRegistryStore::refresh: fetch failed: {error:#}");
@@ -231,7 +247,7 @@ impl AgentRegistryStore {
                         this.fetch_error = None;
                     }
                     Err(error) => {
-                        this.fetch_error = Some(SharedString::from(error.to_string()));
+                        this.fetch_error = Some(SharedString::from(format!("{error:#}")));
                     }
                 }
                 cx.notify();
@@ -295,7 +311,9 @@ impl AgentRegistryStore {
             let index: RegistryIndex =
                 serde_json::from_slice(&bytes).context("parsing cached registry")?;
 
-            let agents = build_registry_agents(fs, http_client, index, bytes, false).await?;
+            let executor = cx.background_executor().clone();
+            let agents =
+                build_registry_agents(fs, http_client, index, bytes, false, &executor).await?;
 
             this.update(cx, |this, cx| {
                 this.agents = agents;
@@ -313,24 +331,20 @@ struct RegistryFetchResult {
     raw_body: Vec<u8>,
 }
 
-async fn fetch_registry_index(http_client: Arc<dyn HttpClient>) -> Result<RegistryFetchResult> {
-    let mut response = http_client
-        .get(REGISTRY_URL, AsyncBody::default(), true)
-        .await
-        .context("requesting ACP registry")?;
+async fn fetch_registry_index(
+    http_client: Arc<dyn HttpClient>,
+    executor: &BackgroundExecutor,
+) -> Result<RegistryFetchResult> {
+    let (status, body) =
+        fetch_url_body(http_client, REGISTRY_URL, REGISTRY_FETCH_TIMEOUT, executor)
+            .await
+            .context("fetching ACP registry")?;
 
-    let mut body = Vec::new();
-    response
-        .body_mut()
-        .read_to_end(&mut body)
-        .await
-        .context("reading ACP registry response")?;
-
-    if response.status().is_client_error() {
+    if status.is_client_error() {
         let text = String::from_utf8_lossy(body.as_slice());
         bail!(
             "registry status error {}, response: {text:?}",
-            response.status().as_u16()
+            status.as_u16()
         );
     }
 
@@ -347,6 +361,7 @@ async fn build_registry_agents(
     index: RegistryIndex,
     raw_body: Vec<u8>,
     update_cache: bool,
+    executor: &BackgroundExecutor,
 ) -> Result<Vec<RegistryAgent>> {
     let cache_dir = registry_cache_dir();
     fs.create_dir(&cache_dir).await?;
@@ -362,18 +377,18 @@ async fn build_registry_agents(
     }
 
     let current_platform = current_platform_key();
+    let icon_paths = resolve_icon_paths(
+        &index.agents,
+        &icons_dir,
+        update_cache,
+        fs.clone(),
+        http_client.clone(),
+        executor,
+    )
+    .await;
 
     let mut agents = Vec::new();
-    for entry in index.agents {
-        let icon_path = resolve_icon_path(
-            &entry,
-            &icons_dir,
-            update_cache,
-            fs.clone(),
-            http_client.clone(),
-        )
-        .await?;
-
+    for (entry, icon_path) in index.agents.into_iter().zip(icon_paths) {
         let metadata = RegistryAgentMetadata {
             id: AgentId::new(entry.id),
             name: entry.name.into(),
@@ -440,12 +455,34 @@ async fn build_registry_agents(
     Ok(agents)
 }
 
+async fn resolve_icon_paths(
+    entries: &[RegistryEntry],
+    icons_dir: &Path,
+    update_cache: bool,
+    fs: Arc<dyn Fs>,
+    http_client: Arc<dyn HttpClient>,
+    executor: &BackgroundExecutor,
+) -> Vec<Option<SharedString>> {
+    join_all(entries.iter().map(|entry| {
+        let fs = fs.clone();
+        let http_client = http_client.clone();
+        async move {
+            resolve_icon_path(entry, icons_dir, update_cache, fs, http_client, executor)
+                .await
+                .log_err()
+                .flatten()
+        }
+    }))
+    .await
+}
+
 async fn resolve_icon_path(
     entry: &RegistryEntry,
     icons_dir: &Path,
     update_cache: bool,
     fs: Arc<dyn Fs>,
     http_client: Arc<dyn HttpClient>,
+    executor: &BackgroundExecutor,
 ) -> Result<Option<SharedString>> {
     let icon_url = resolve_icon_url(entry);
     let Some(icon_url) = icon_url else {
@@ -454,7 +491,8 @@ async fn resolve_icon_path(
 
     let icon_path = icons_dir.join(format!("{}.svg", entry.id));
     if update_cache && !fs.is_file(&icon_path).await {
-        if let Err(error) = download_icon(fs.clone(), http_client, &icon_url, entry).await {
+        if let Err(error) = download_icon(fs.clone(), http_client, &icon_url, entry, executor).await
+        {
             log::warn!(
                 "Failed to download ACP registry icon for {}: {error:#}",
                 entry.id
@@ -476,25 +514,16 @@ async fn download_icon(
     http_client: Arc<dyn HttpClient>,
     icon_url: &str,
     entry: &RegistryEntry,
+    executor: &BackgroundExecutor,
 ) -> Result<()> {
-    let mut response = http_client
-        .get(icon_url, AsyncBody::default(), true)
-        .await
-        .with_context(|| format!("requesting icon for {}", entry.id))?;
+    let (status, body) =
+        fetch_url_body(http_client, icon_url, REGISTRY_ICON_FETCH_TIMEOUT, executor)
+            .await
+            .with_context(|| format!("fetching icon for {}", entry.id))?;
 
-    let mut body = Vec::new();
-    response
-        .body_mut()
-        .read_to_end(&mut body)
-        .await
-        .with_context(|| format!("reading icon for {}", entry.id))?;
-
-    if response.status().is_client_error() {
+    if status.is_client_error() {
         let text = String::from_utf8_lossy(body.as_slice());
-        bail!(
-            "icon status error {}, response: {text:?}",
-            response.status().as_u16()
-        );
+        bail!("icon status error {}, response: {text:?}", status.as_u16());
     }
 
     let icon_path = registry_cache_dir()
@@ -504,6 +533,38 @@ async fn download_icon(
     Ok(())
 }
 
+async fn fetch_url_body(
+    http_client: Arc<dyn HttpClient>,
+    url: &str,
+    timeout: Duration,
+    executor: &BackgroundExecutor,
+) -> Result<(StatusCode, Vec<u8>)> {
+    async {
+        let mut response = http_client
+            .get(url, AsyncBody::default(), true)
+            .await
+            .with_context(|| format!("requesting {url}"))?;
+
+        let status = response.status();
+        let mut body = Vec::new();
+        response
+            .body_mut()
+            .read_to_end(&mut body)
+            .await
+            .with_context(|| format!("reading response from {url}"))?;
+
+        Ok((status, body))
+    }
+    .with_timeout(timeout, executor)
+    .await
+    .map_err(|_| {
+        anyhow!(
+            "timed out after {}s while fetching {url}",
+            timeout.as_secs()
+        )
+    })?
+}
+
 fn resolve_icon_url(entry: &RegistryEntry) -> Option<String> {
     let icon = entry.icon.as_ref()?;
     if icon.starts_with("https://") || icon.starts_with("http://") {

crates/project/tests/integration/agent_registry_store.rs

@@ -0,0 +1,115 @@
+use std::{future, sync::Arc, time::Duration};
+
+use fs::FakeFs;
+use gpui::TestAppContext;
+use http_client::{AsyncBody, FakeHttpClient, HttpClient, Response};
+use project::AgentRegistryStore;
+use serde_json::json;
+
+use crate::init_test;
+
+#[gpui::test]
+async fn registry_refresh_times_out_when_fetch_never_completes(cx: &mut TestAppContext) {
+    init_test(cx);
+
+    let fs = FakeFs::new(cx.executor());
+    let http_client =
+        FakeHttpClient::create(|_| future::pending::<anyhow::Result<Response<AsyncBody>>>())
+            as Arc<dyn HttpClient>;
+
+    let registry_store =
+        cx.update(|cx| AgentRegistryStore::init_global(cx, fs.clone(), http_client));
+    cx.run_until_parked();
+
+    cx.executor().advance_clock(Duration::from_secs(31));
+    cx.run_until_parked();
+
+    registry_store.update(cx, |store, _| {
+        assert!(!store.is_fetching());
+        assert!(
+            store
+                .fetch_error()
+                .is_some_and(|error| error.contains("timed out after 30s")),
+            "expected registry fetch timeout error, got {:?}",
+            store.fetch_error()
+        );
+    });
+}
+
+#[gpui::test]
+async fn registry_refresh_does_not_block_sequentially_on_hung_icon_downloads(
+    cx: &mut TestAppContext,
+) {
+    init_test(cx);
+
+    let fs = FakeFs::new(cx.executor());
+    let http_client = FakeHttpClient::create(|request| async move {
+        if request.uri().to_string().contains("registry.json") {
+            Ok(Response::builder()
+                .status(200)
+                .body(AsyncBody::from(
+                    serde_json::to_string(&json!({
+                        "version": "1",
+                        "agents": [
+                            {
+                                "id": "slow-icon-a",
+                                "name": "Slow Icon A",
+                                "version": "1.0.0",
+                                "description": "An agent with a slow icon.",
+                                "icon": "https://example.test/slow-icon-a.svg",
+                                "distribution": {
+                                    "npx": {
+                                        "package": "slow-icon-a"
+                                    }
+                                }
+                            },
+                            {
+                                "id": "slow-icon-b",
+                                "name": "Slow Icon B",
+                                "version": "1.0.0",
+                                "description": "Another agent with a slow icon.",
+                                "icon": "https://example.test/slow-icon-b.svg",
+                                "distribution": {
+                                    "npx": {
+                                        "package": "slow-icon-b"
+                                    }
+                                }
+                            },
+                            {
+                                "id": "slow-icon-c",
+                                "name": "Slow Icon C",
+                                "version": "1.0.0",
+                                "description": "A third agent with a slow icon.",
+                                "icon": "https://example.test/slow-icon-c.svg",
+                                "distribution": {
+                                    "npx": {
+                                        "package": "slow-icon-c"
+                                    }
+                                }
+                            }
+                        ]
+                    }))
+                    .unwrap(),
+                ))
+                .unwrap())
+        } else {
+            future::pending::<anyhow::Result<Response<AsyncBody>>>().await
+        }
+    }) as Arc<dyn HttpClient>;
+
+    let registry_store =
+        cx.update(|cx| AgentRegistryStore::init_global(cx, fs.clone(), http_client));
+    cx.run_until_parked();
+
+    cx.executor().advance_clock(Duration::from_secs(11));
+    cx.run_until_parked();
+
+    registry_store.update(cx, |store, _| {
+        assert!(!store.is_fetching());
+        assert_eq!(store.agents().len(), 3);
+        assert_eq!(store.agents()[0].id().as_ref(), "slow-icon-a");
+        assert_eq!(store.agents()[1].id().as_ref(), "slow-icon-b");
+        assert_eq!(store.agents()[2].id().as_ref(), "slow-icon-c");
+        assert_eq!(store.fetch_error(), None);
+    });
+}

crates/project/tests/integration/project_tests.rs

@@ -1,5 +1,6 @@
 #![allow(clippy::format_collect)]
 
+mod agent_registry_store;
 mod bookmark_store;
 mod color_extractor;
 mod context_server_store;