Bump to 0.194.2 for @bennetbo

Cherry-picked agent: Fix bug that prevented MCP servers to appear in the
settings view (#33857)

Closes https://github.com/zed-industries/zed/issues/33827

After #33644 was merged, we would not start MCP servers coming from
extensions correctly anymore. The optimization uncovered a bug in the
implementation of `ContextServerDescriptorRegistry`, because we never
called `cx.notify()` when adding/removing context servers.
`ContextServerStore` listens for these events, and before #33644 this
was just working because of aace condition.

Release Notes:

- agent: Fixed bug that prevented MCP servers to appear in the settings
view.

Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>

Co-authored-by: Danilo Leal <67129314+danilo-leal@users.noreply.github.com>
Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>

Cargo.lock

@@ -19942,7 +19942,7 @@ dependencies = [
 
 [[package]]
 name = "zed"
-version = "0.194.0"
+version = "0.194.2"
 dependencies = [
  "activity_indicator",
  "agent",

crates/agent/src/agent_profile.rs

@@ -1,7 +1,7 @@
 use std::sync::Arc;
 
 use agent_settings::{AgentProfileId, AgentProfileSettings, AgentSettings};
-use assistant_tool::{Tool, ToolSource, ToolWorkingSet};
+use assistant_tool::{Tool, ToolSource, ToolWorkingSet, UniqueToolName};
 use collections::IndexMap;
 use convert_case::{Case, Casing};
 use fs::Fs;
@@ -72,7 +72,7 @@ impl AgentProfile {
         &self.id
     }
 
-    pub fn enabled_tools(&self, cx: &App) -> Vec<Arc<dyn Tool>> {
+    pub fn enabled_tools(&self, cx: &App) -> Vec<(UniqueToolName, Arc<dyn Tool>)> {
         let Some(settings) = AgentSettings::get_global(cx).profiles.get(&self.id) else {
             return Vec::new();
         };
@@ -81,7 +81,7 @@ impl AgentProfile {
             .read(cx)
             .tools(cx)
             .into_iter()
-            .filter(|tool| Self::is_enabled(settings, tool.source(), tool.name()))
+            .filter(|(_, tool)| Self::is_enabled(settings, tool.source(), tool.name()))
             .collect()
     }
 
@@ -137,7 +137,7 @@ mod tests {
         let mut enabled_tools = cx
             .read(|cx| profile.enabled_tools(cx))
             .into_iter()
-            .map(|tool| tool.name())
+            .map(|(_, tool)| tool.name())
             .collect::<Vec<_>>();
         enabled_tools.sort();
 
@@ -174,7 +174,7 @@ mod tests {
         let mut enabled_tools = cx
             .read(|cx| profile.enabled_tools(cx))
             .into_iter()
-            .map(|tool| tool.name())
+            .map(|(_, tool)| tool.name())
             .collect::<Vec<_>>();
         enabled_tools.sort();
 
@@ -207,7 +207,7 @@ mod tests {
         let mut enabled_tools = cx
             .read(|cx| profile.enabled_tools(cx))
             .into_iter()
-            .map(|tool| tool.name())
+            .map(|(_, tool)| tool.name())
             .collect::<Vec<_>>();
         enabled_tools.sort();
 
@@ -267,10 +267,10 @@ mod tests {
     }
 
     fn default_tool_set(cx: &mut TestAppContext) -> Entity<ToolWorkingSet> {
-        cx.new(|_| {
+        cx.new(|cx| {
             let mut tool_set = ToolWorkingSet::default();
-            tool_set.insert(Arc::new(FakeTool::new("enabled_mcp_tool", "mcp")));
-            tool_set.insert(Arc::new(FakeTool::new("disabled_mcp_tool", "mcp")));
+            tool_set.insert(Arc::new(FakeTool::new("enabled_mcp_tool", "mcp")), cx);
+            tool_set.insert(Arc::new(FakeTool::new("disabled_mcp_tool", "mcp")), cx);
             tool_set
         })
     }

crates/agent/src/thread.rs

@@ -13,7 +13,7 @@ use anyhow::{Result, anyhow};
 use assistant_tool::{ActionLog, AnyToolCard, Tool, ToolWorkingSet};
 use chrono::{DateTime, Utc};
 use client::{ModelRequestUsage, RequestUsage};
-use collections::{HashMap, HashSet};
+use collections::HashMap;
 use feature_flags::{self, FeatureFlagAppExt};
 use futures::{FutureExt, StreamExt as _, future::Shared};
 use git::repository::DiffType;
@@ -960,13 +960,14 @@ impl Thread {
         model: Arc<dyn LanguageModel>,
     ) -> Vec<LanguageModelRequestTool> {
         if model.supports_tools() {
-            resolve_tool_name_conflicts(self.profile.enabled_tools(cx).as_slice())
+            self.profile
+                .enabled_tools(cx)
                 .into_iter()
                 .filter_map(|(name, tool)| {
                     // Skip tools that cannot be supported
                     let input_schema = tool.input_schema(model.tool_input_format()).ok()?;
                     Some(LanguageModelRequestTool {
-                        name,
+                        name: name.into(),
                         description: tool.description(),
                         input_schema,
                     })
@@ -2386,7 +2387,7 @@ impl Thread {
 
         let tool_list = available_tools
             .iter()
-            .map(|tool| format!("- {}: {}", tool.name(), tool.description()))
+            .map(|(name, tool)| format!("- {}: {}", name, tool.description()))
             .collect::<Vec<_>>()
             .join("\n");
 
@@ -2606,7 +2607,7 @@ impl Thread {
             .profile
             .enabled_tools(cx)
             .iter()
-            .map(|tool| tool.name())
+            .map(|(name, _)| name.clone().into())
             .collect();
 
         self.message_feedback.insert(message_id, feedback);
@@ -3144,85 +3145,6 @@ struct PendingCompletion {
     _task: Task<()>,
 }
 
-/// Resolves tool name conflicts by ensuring all tool names are unique.
-///
-/// When multiple tools have the same name, this function applies the following rules:
-/// 1. Native tools always keep their original name
-/// 2. Context server tools get prefixed with their server ID and an underscore
-/// 3. All tool names are truncated to MAX_TOOL_NAME_LENGTH (64 characters)
-/// 4. If conflicts still exist after prefixing, the conflicting tools are filtered out
-///
-/// Note: This function assumes that built-in tools occur before MCP tools in the tools list.
-fn resolve_tool_name_conflicts(tools: &[Arc<dyn Tool>]) -> Vec<(String, Arc<dyn Tool>)> {
-    fn resolve_tool_name(tool: &Arc<dyn Tool>) -> String {
-        let mut tool_name = tool.name();
-        tool_name.truncate(MAX_TOOL_NAME_LENGTH);
-        tool_name
-    }
-
-    const MAX_TOOL_NAME_LENGTH: usize = 64;
-
-    let mut duplicated_tool_names = HashSet::default();
-    let mut seen_tool_names = HashSet::default();
-    for tool in tools {
-        let tool_name = resolve_tool_name(tool);
-        if seen_tool_names.contains(&tool_name) {
-            debug_assert!(
-                tool.source() != assistant_tool::ToolSource::Native,
-                "There are two built-in tools with the same name: {}",
-                tool_name
-            );
-            duplicated_tool_names.insert(tool_name);
-        } else {
-            seen_tool_names.insert(tool_name);
-        }
-    }
-
-    if duplicated_tool_names.is_empty() {
-        return tools
-            .into_iter()
-            .map(|tool| (resolve_tool_name(tool), tool.clone()))
-            .collect();
-    }
-
-    tools
-        .into_iter()
-        .filter_map(|tool| {
-            let mut tool_name = resolve_tool_name(tool);
-            if !duplicated_tool_names.contains(&tool_name) {
-                return Some((tool_name, tool.clone()));
-            }
-            match tool.source() {
-                assistant_tool::ToolSource::Native => {
-                    // Built-in tools always keep their original name
-                    Some((tool_name, tool.clone()))
-                }
-                assistant_tool::ToolSource::ContextServer { id } => {
-                    // Context server tools are prefixed with the context server ID, and truncated if necessary
-                    tool_name.insert(0, '_');
-                    if tool_name.len() + id.len() > MAX_TOOL_NAME_LENGTH {
-                        let len = MAX_TOOL_NAME_LENGTH - tool_name.len();
-                        let mut id = id.to_string();
-                        id.truncate(len);
-                        tool_name.insert_str(0, &id);
-                    } else {
-                        tool_name.insert_str(0, &id);
-                    }
-
-                    tool_name.truncate(MAX_TOOL_NAME_LENGTH);
-
-                    if seen_tool_names.contains(&tool_name) {
-                        log::error!("Cannot resolve tool name conflict for tool {}", tool.name());
-                        None
-                    } else {
-                        Some((tool_name, tool.clone()))
-                    }
-                }
-            }
-        })
-        .collect()
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -3238,7 +3160,6 @@ mod tests {
     use futures::future::BoxFuture;
     use futures::stream::BoxStream;
     use gpui::TestAppContext;
-    use icons::IconName;
     use language_model::fake_provider::{FakeLanguageModel, FakeLanguageModelProvider};
     use language_model::{
         LanguageModelCompletionError, LanguageModelName, LanguageModelProviderId,
@@ -3883,148 +3804,6 @@ fn main() {{
         });
     }
 
-    #[gpui::test]
-    fn test_resolve_tool_name_conflicts() {
-        use assistant_tool::{Tool, ToolSource};
-
-        assert_resolve_tool_name_conflicts(
-            vec![
-                TestTool::new("tool1", ToolSource::Native),
-                TestTool::new("tool2", ToolSource::Native),
-                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-1".into() }),
-            ],
-            vec!["tool1", "tool2", "tool3"],
-        );
-
-        assert_resolve_tool_name_conflicts(
-            vec![
-                TestTool::new("tool1", ToolSource::Native),
-                TestTool::new("tool2", ToolSource::Native),
-                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-1".into() }),
-                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-2".into() }),
-            ],
-            vec!["tool1", "tool2", "mcp-1_tool3", "mcp-2_tool3"],
-        );
-
-        assert_resolve_tool_name_conflicts(
-            vec![
-                TestTool::new("tool1", ToolSource::Native),
-                TestTool::new("tool2", ToolSource::Native),
-                TestTool::new("tool3", ToolSource::Native),
-                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-1".into() }),
-                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-2".into() }),
-            ],
-            vec!["tool1", "tool2", "tool3", "mcp-1_tool3", "mcp-2_tool3"],
-        );
-
-        // Test that tool with very long name is always truncated
-        assert_resolve_tool_name_conflicts(
-            vec![TestTool::new(
-                "tool-with-more-then-64-characters-blah-blah-blah-blah-blah-blah-blah-blah",
-                ToolSource::Native,
-            )],
-            vec!["tool-with-more-then-64-characters-blah-blah-blah-blah-blah-blah-"],
-        );
-
-        // Test deduplication of tools with very long names, in this case the mcp server name should be truncated
-        assert_resolve_tool_name_conflicts(
-            vec![
-                TestTool::new("tool-with-very-very-very-long-name", ToolSource::Native),
-                TestTool::new(
-                    "tool-with-very-very-very-long-name",
-                    ToolSource::ContextServer {
-                        id: "mcp-with-very-very-very-long-name".into(),
-                    },
-                ),
-            ],
-            vec![
-                "tool-with-very-very-very-long-name",
-                "mcp-with-very-very-very-long-_tool-with-very-very-very-long-name",
-            ],
-        );
-
-        fn assert_resolve_tool_name_conflicts(
-            tools: Vec<TestTool>,
-            expected: Vec<impl Into<String>>,
-        ) {
-            let tools: Vec<Arc<dyn Tool>> = tools
-                .into_iter()
-                .map(|t| Arc::new(t) as Arc<dyn Tool>)
-                .collect();
-            let tools = resolve_tool_name_conflicts(&tools);
-            assert_eq!(tools.len(), expected.len());
-            for (i, expected_name) in expected.into_iter().enumerate() {
-                let expected_name = expected_name.into();
-                let actual_name = &tools[i].0;
-                assert_eq!(
-                    actual_name, &expected_name,
-                    "Expected '{}' got '{}' at index {}",
-                    expected_name, actual_name, i
-                );
-            }
-        }
-
-        struct TestTool {
-            name: String,
-            source: ToolSource,
-        }
-
-        impl TestTool {
-            fn new(name: impl Into<String>, source: ToolSource) -> Self {
-                Self {
-                    name: name.into(),
-                    source,
-                }
-            }
-        }
-
-        impl Tool for TestTool {
-            fn name(&self) -> String {
-                self.name.clone()
-            }
-
-            fn icon(&self) -> IconName {
-                IconName::Ai
-            }
-
-            fn may_perform_edits(&self) -> bool {
-                false
-            }
-
-            fn needs_confirmation(&self, _input: &serde_json::Value, _cx: &App) -> bool {
-                true
-            }
-
-            fn source(&self) -> ToolSource {
-                self.source.clone()
-            }
-
-            fn description(&self) -> String {
-                "Test tool".to_string()
-            }
-
-            fn ui_text(&self, _input: &serde_json::Value) -> String {
-                "Test tool".to_string()
-            }
-
-            fn run(
-                self: Arc<Self>,
-                _input: serde_json::Value,
-                _request: Arc<LanguageModelRequest>,
-                _project: Entity<Project>,
-                _action_log: Entity<ActionLog>,
-                _model: Arc<dyn LanguageModel>,
-                _window: Option<AnyWindowHandle>,
-                _cx: &mut App,
-            ) -> assistant_tool::ToolResult {
-                assistant_tool::ToolResult {
-                    output: Task::ready(Err(anyhow::anyhow!("No content"))),
-                    card: None,
-                }
-            }
-        }
-    }
-
     // Helper to create a model that returns errors
     enum TestError {
         Overloaded,

crates/agent/src/thread_store.rs

@@ -6,7 +6,7 @@ use crate::{
 };
 use agent_settings::{AgentProfileId, CompletionMode};
 use anyhow::{Context as _, Result, anyhow};
-use assistant_tool::{ToolId, ToolWorkingSet};
+use assistant_tool::{Tool, ToolId, ToolWorkingSet};
 use chrono::{DateTime, Utc};
 use collections::HashMap;
 use context_server::ContextServerId;
@@ -537,8 +537,8 @@ impl ThreadStore {
                     }
                     ContextServerStatus::Stopped | ContextServerStatus::Error(_) => {
                         if let Some(tool_ids) = self.context_server_tool_ids.remove(server_id) {
-                            tool_working_set.update(cx, |tool_working_set, _| {
-                                tool_working_set.remove(&tool_ids);
+                            tool_working_set.update(cx, |tool_working_set, cx| {
+                                tool_working_set.remove(&tool_ids, cx);
                             });
                         }
                     }
@@ -569,19 +569,17 @@ impl ThreadStore {
                     .log_err()
                 {
                     let tool_ids = tool_working_set
-                        .update(cx, |tool_working_set, _| {
-                            response
-                                .tools
-                                .into_iter()
-                                .map(|tool| {
-                                    log::info!("registering context server tool: {:?}", tool.name);
-                                    tool_working_set.insert(Arc::new(ContextServerTool::new(
+                        .update(cx, |tool_working_set, cx| {
+                            tool_working_set.extend(
+                                response.tools.into_iter().map(|tool| {
+                                    Arc::new(ContextServerTool::new(
                                         context_server_store.clone(),
                                         server.id(),
                                         tool,
-                                    )))
-                                })
-                                .collect::<Vec<_>>()
+                                    )) as Arc<dyn Tool>
+                                }),
+                                cx,
+                            )
                         })
                         .log_err();
 

crates/agent_ui/src/tool_compatibility.rs

@@ -42,8 +42,8 @@ impl IncompatibleToolsState {
                     .profile()
                     .enabled_tools(cx)
                     .iter()
-                    .filter(|tool| tool.input_schema(model.tool_input_format()).is_err())
-                    .cloned()
+                    .filter(|(_, tool)| tool.input_schema(model.tool_input_format()).is_err())
+                    .map(|(_, tool)| tool.clone())
                     .collect()
             })
     }

crates/assistant_tool/Cargo.toml

@@ -22,6 +22,7 @@ gpui.workspace = true
 icons.workspace = true
 language.workspace = true
 language_model.workspace = true
+log.workspace = true
 parking_lot.workspace = true
 project.workspace = true
 regex.workspace = true

crates/assistant_tool/src/tool_working_set.rs

@@ -1,18 +1,52 @@
-use std::sync::Arc;
-
-use collections::{HashMap, IndexMap};
-use gpui::App;
+use std::{borrow::Borrow, sync::Arc};
 
 use crate::{Tool, ToolRegistry, ToolSource};
+use collections::{HashMap, HashSet, IndexMap};
+use gpui::{App, SharedString};
+use util::debug_panic;
 
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Default)]
 pub struct ToolId(usize);
 
+/// A unique identifier for a tool within a working set.
+#[derive(Clone, PartialEq, Eq, Hash, Default)]
+pub struct UniqueToolName(SharedString);
+
+impl Borrow<str> for UniqueToolName {
+    fn borrow(&self) -> &str {
+        &self.0
+    }
+}
+
+impl From<String> for UniqueToolName {
+    fn from(value: String) -> Self {
+        UniqueToolName(SharedString::new(value))
+    }
+}
+
+impl Into<String> for UniqueToolName {
+    fn into(self) -> String {
+        self.0.into()
+    }
+}
+
+impl std::fmt::Debug for UniqueToolName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
+impl std::fmt::Display for UniqueToolName {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.0.as_ref())
+    }
+}
+
 /// A working set of tools for use in one instance of the Assistant Panel.
 #[derive(Default)]
 pub struct ToolWorkingSet {
     context_server_tools_by_id: HashMap<ToolId, Arc<dyn Tool>>,
-    context_server_tools_by_name: HashMap<String, Arc<dyn Tool>>,
+    context_server_tools_by_name: HashMap<UniqueToolName, Arc<dyn Tool>>,
     next_tool_id: ToolId,
 }
 
@@ -24,16 +58,20 @@ impl ToolWorkingSet {
             .or_else(|| ToolRegistry::global(cx).tool(name))
     }
 
-    pub fn tools(&self, cx: &App) -> Vec<Arc<dyn Tool>> {
-        let mut tools = ToolRegistry::global(cx).tools();
-        tools.extend(self.context_server_tools_by_id.values().cloned());
+    pub fn tools(&self, cx: &App) -> Vec<(UniqueToolName, Arc<dyn Tool>)> {
+        let mut tools = ToolRegistry::global(cx)
+            .tools()
+            .into_iter()
+            .map(|tool| (UniqueToolName(tool.name().into()), tool))
+            .collect::<Vec<_>>();
+        tools.extend(self.context_server_tools_by_name.clone());
         tools
     }
 
     pub fn tools_by_source(&self, cx: &App) -> IndexMap<ToolSource, Vec<Arc<dyn Tool>>> {
         let mut tools_by_source = IndexMap::default();
 
-        for tool in self.tools(cx) {
+        for (_, tool) in self.tools(cx) {
             tools_by_source
                 .entry(tool.source())
                 .or_insert_with(Vec::new)
@@ -49,27 +87,324 @@ impl ToolWorkingSet {
         tools_by_source
     }
 
-    pub fn insert(&mut self, tool: Arc<dyn Tool>) -> ToolId {
+    pub fn insert(&mut self, tool: Arc<dyn Tool>, cx: &App) -> ToolId {
+        let tool_id = self.register_tool(tool);
+        self.tools_changed(cx);
+        tool_id
+    }
+
+    pub fn extend(&mut self, tools: impl Iterator<Item = Arc<dyn Tool>>, cx: &App) -> Vec<ToolId> {
+        let ids = tools.map(|tool| self.register_tool(tool)).collect();
+        self.tools_changed(cx);
+        ids
+    }
+
+    pub fn remove(&mut self, tool_ids_to_remove: &[ToolId], cx: &App) {
+        self.context_server_tools_by_id
+            .retain(|id, _| !tool_ids_to_remove.contains(id));
+        self.tools_changed(cx);
+    }
+
+    fn register_tool(&mut self, tool: Arc<dyn Tool>) -> ToolId {
         let tool_id = self.next_tool_id;
         self.next_tool_id.0 += 1;
         self.context_server_tools_by_id
             .insert(tool_id, tool.clone());
-        self.tools_changed();
         tool_id
     }
 
-    pub fn remove(&mut self, tool_ids_to_remove: &[ToolId]) {
-        self.context_server_tools_by_id
-            .retain(|id, _| !tool_ids_to_remove.contains(id));
-        self.tools_changed();
-    }
-
-    fn tools_changed(&mut self) {
-        self.context_server_tools_by_name.clear();
-        self.context_server_tools_by_name.extend(
-            self.context_server_tools_by_id
+    fn tools_changed(&mut self, cx: &App) {
+        self.context_server_tools_by_name = resolve_context_server_tool_name_conflicts(
+            &self
+                .context_server_tools_by_id
                 .values()
-                .map(|tool| (tool.name(), tool.clone())),
+                .cloned()
+                .collect::<Vec<_>>(),
+            &ToolRegistry::global(cx).tools(),
         );
     }
 }
+
+fn resolve_context_server_tool_name_conflicts(
+    context_server_tools: &[Arc<dyn Tool>],
+    native_tools: &[Arc<dyn Tool>],
+) -> HashMap<UniqueToolName, Arc<dyn Tool>> {
+    fn resolve_tool_name(tool: &Arc<dyn Tool>) -> String {
+        let mut tool_name = tool.name();
+        tool_name.truncate(MAX_TOOL_NAME_LENGTH);
+        tool_name
+    }
+
+    const MAX_TOOL_NAME_LENGTH: usize = 64;
+
+    let mut duplicated_tool_names = HashSet::default();
+    let mut seen_tool_names = HashSet::default();
+    seen_tool_names.extend(native_tools.iter().map(|tool| tool.name()));
+    for tool in context_server_tools {
+        let tool_name = resolve_tool_name(tool);
+        if seen_tool_names.contains(&tool_name) {
+            debug_assert!(
+                tool.source() != ToolSource::Native,
+                "Expected MCP tool but got a native tool: {}",
+                tool_name
+            );
+            duplicated_tool_names.insert(tool_name);
+        } else {
+            seen_tool_names.insert(tool_name);
+        }
+    }
+
+    if duplicated_tool_names.is_empty() {
+        return context_server_tools
+            .into_iter()
+            .map(|tool| (resolve_tool_name(tool).into(), tool.clone()))
+            .collect();
+    }
+
+    context_server_tools
+        .into_iter()
+        .filter_map(|tool| {
+            let mut tool_name = resolve_tool_name(tool);
+            if !duplicated_tool_names.contains(&tool_name) {
+                return Some((tool_name.into(), tool.clone()));
+            }
+            match tool.source() {
+                ToolSource::Native => {
+                    debug_panic!("Expected MCP tool but got a native tool: {}", tool_name);
+                    // Built-in tools always keep their original name
+                    Some((tool_name.into(), tool.clone()))
+                }
+                ToolSource::ContextServer { id } => {
+                    // Context server tools are prefixed with the context server ID, and truncated if necessary
+                    tool_name.insert(0, '_');
+                    if tool_name.len() + id.len() > MAX_TOOL_NAME_LENGTH {
+                        let len = MAX_TOOL_NAME_LENGTH - tool_name.len();
+                        let mut id = id.to_string();
+                        id.truncate(len);
+                        tool_name.insert_str(0, &id);
+                    } else {
+                        tool_name.insert_str(0, &id);
+                    }
+
+                    tool_name.truncate(MAX_TOOL_NAME_LENGTH);
+
+                    if seen_tool_names.contains(&tool_name) {
+                        log::error!("Cannot resolve tool name conflict for tool {}", tool.name());
+                        None
+                    } else {
+                        Some((tool_name.into(), tool.clone()))
+                    }
+                }
+            }
+        })
+        .collect()
+}
+#[cfg(test)]
+mod tests {
+    use gpui::{AnyWindowHandle, Entity, Task, TestAppContext};
+    use language_model::{LanguageModel, LanguageModelRequest};
+    use project::Project;
+
+    use crate::{ActionLog, ToolResult};
+
+    use super::*;
+
+    #[gpui::test]
+    fn test_unique_tool_names(cx: &mut TestAppContext) {
+        fn assert_tool(
+            tool_working_set: &ToolWorkingSet,
+            unique_name: &str,
+            expected_name: &str,
+            expected_source: ToolSource,
+            cx: &App,
+        ) {
+            let tool = tool_working_set.tool(unique_name, cx).unwrap();
+            assert_eq!(tool.name(), expected_name);
+            assert_eq!(tool.source(), expected_source);
+        }
+
+        let tool_registry = cx.update(ToolRegistry::default_global);
+        tool_registry.register_tool(TestTool::new("tool1", ToolSource::Native));
+        tool_registry.register_tool(TestTool::new("tool2", ToolSource::Native));
+
+        let mut tool_working_set = ToolWorkingSet::default();
+        cx.update(|cx| {
+            tool_working_set.extend(
+                vec![
+                    Arc::new(TestTool::new(
+                        "tool2",
+                        ToolSource::ContextServer { id: "mcp-1".into() },
+                    )) as Arc<dyn Tool>,
+                    Arc::new(TestTool::new(
+                        "tool2",
+                        ToolSource::ContextServer { id: "mcp-2".into() },
+                    )) as Arc<dyn Tool>,
+                ]
+                .into_iter(),
+                cx,
+            );
+        });
+
+        cx.update(|cx| {
+            assert_tool(&tool_working_set, "tool1", "tool1", ToolSource::Native, cx);
+            assert_tool(&tool_working_set, "tool2", "tool2", ToolSource::Native, cx);
+            assert_tool(
+                &tool_working_set,
+                "mcp-1_tool2",
+                "tool2",
+                ToolSource::ContextServer { id: "mcp-1".into() },
+                cx,
+            );
+            assert_tool(
+                &tool_working_set,
+                "mcp-2_tool2",
+                "tool2",
+                ToolSource::ContextServer { id: "mcp-2".into() },
+                cx,
+            );
+        })
+    }
+
+    #[gpui::test]
+    fn test_resolve_context_server_tool_name_conflicts() {
+        assert_resolve_context_server_tool_name_conflicts(
+            vec![
+                TestTool::new("tool1", ToolSource::Native),
+                TestTool::new("tool2", ToolSource::Native),
+            ],
+            vec![TestTool::new(
+                "tool3",
+                ToolSource::ContextServer { id: "mcp-1".into() },
+            )],
+            vec!["tool3"],
+        );
+
+        assert_resolve_context_server_tool_name_conflicts(
+            vec![
+                TestTool::new("tool1", ToolSource::Native),
+                TestTool::new("tool2", ToolSource::Native),
+            ],
+            vec![
+                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-1".into() }),
+                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-2".into() }),
+            ],
+            vec!["mcp-1_tool3", "mcp-2_tool3"],
+        );
+
+        assert_resolve_context_server_tool_name_conflicts(
+            vec![
+                TestTool::new("tool1", ToolSource::Native),
+                TestTool::new("tool2", ToolSource::Native),
+                TestTool::new("tool3", ToolSource::Native),
+            ],
+            vec![
+                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-1".into() }),
+                TestTool::new("tool3", ToolSource::ContextServer { id: "mcp-2".into() }),
+            ],
+            vec!["mcp-1_tool3", "mcp-2_tool3"],
+        );
+
+        // Test deduplication of tools with very long names, in this case the mcp server name should be truncated
+        assert_resolve_context_server_tool_name_conflicts(
+            vec![TestTool::new(
+                "tool-with-very-very-very-long-name",
+                ToolSource::Native,
+            )],
+            vec![TestTool::new(
+                "tool-with-very-very-very-long-name",
+                ToolSource::ContextServer {
+                    id: "mcp-with-very-very-very-long-name".into(),
+                },
+            )],
+            vec!["mcp-with-very-very-very-long-_tool-with-very-very-very-long-name"],
+        );
+
+        fn assert_resolve_context_server_tool_name_conflicts(
+            builtin_tools: Vec<TestTool>,
+            context_server_tools: Vec<TestTool>,
+            expected: Vec<&'static str>,
+        ) {
+            let context_server_tools: Vec<Arc<dyn Tool>> = context_server_tools
+                .into_iter()
+                .map(|t| Arc::new(t) as Arc<dyn Tool>)
+                .collect();
+            let builtin_tools: Vec<Arc<dyn Tool>> = builtin_tools
+                .into_iter()
+                .map(|t| Arc::new(t) as Arc<dyn Tool>)
+                .collect();
+            let tools =
+                resolve_context_server_tool_name_conflicts(&context_server_tools, &builtin_tools);
+            assert_eq!(tools.len(), expected.len());
+            for (i, (name, _)) in tools.into_iter().enumerate() {
+                assert_eq!(
+                    name.0.as_ref(),
+                    expected[i],
+                    "Expected '{}' got '{}' at index {}",
+                    expected[i],
+                    name,
+                    i
+                );
+            }
+        }
+    }
+
+    struct TestTool {
+        name: String,
+        source: ToolSource,
+    }
+
+    impl TestTool {
+        fn new(name: impl Into<String>, source: ToolSource) -> Self {
+            Self {
+                name: name.into(),
+                source,
+            }
+        }
+    }
+
+    impl Tool for TestTool {
+        fn name(&self) -> String {
+            self.name.clone()
+        }
+
+        fn icon(&self) -> icons::IconName {
+            icons::IconName::Ai
+        }
+
+        fn may_perform_edits(&self) -> bool {
+            false
+        }
+
+        fn needs_confirmation(&self, _input: &serde_json::Value, _cx: &App) -> bool {
+            true
+        }
+
+        fn source(&self) -> ToolSource {
+            self.source.clone()
+        }
+
+        fn description(&self) -> String {
+            "Test tool".to_string()
+        }
+
+        fn ui_text(&self, _input: &serde_json::Value) -> String {
+            "Test tool".to_string()
+        }
+
+        fn run(
+            self: Arc<Self>,
+            _input: serde_json::Value,
+            _request: Arc<LanguageModelRequest>,
+            _project: Entity<Project>,
+            _action_log: Entity<ActionLog>,
+            _model: Arc<dyn LanguageModel>,
+            _window: Option<AnyWindowHandle>,
+            _cx: &mut App,
+        ) -> ToolResult {
+            ToolResult {
+                output: Task::ready(Err(anyhow::anyhow!("No content"))),
+                card: None,
+            }
+        }
+    }
+}

crates/language/src/language_settings.rs

@@ -321,7 +321,7 @@ inventory::submit! {
             let language_settings_content_ref = generator
                 .subschema_for::<LanguageSettingsContent>()
                 .to_value();
-            let schema = json_schema!({
+            replace_subschema::<LanguageToSettingsMap>(generator, || json_schema!({
                 "type": "object",
                 "properties": params
                     .language_names
@@ -333,8 +333,7 @@ inventory::submit! {
                         )
                     })
                     .collect::<serde_json::Map<_, _>>()
-            });
-            replace_subschema::<LanguageToSettingsMap>(generator, schema)
+            }))
         }
     }
 }

crates/project/src/context_server_store.rs

@@ -812,9 +812,9 @@ mod tests {
         .await;
 
         let executor = cx.executor();
-        let registry = cx.new(|_| {
+        let registry = cx.new(|cx| {
             let mut registry = ContextServerDescriptorRegistry::new();
-            registry.register_context_server_descriptor(SERVER_1_ID.into(), fake_descriptor_1);
+            registry.register_context_server_descriptor(SERVER_1_ID.into(), fake_descriptor_1, cx);
             registry
         });
         let store = cx.new(|cx| {

crates/project/src/context_server_store/extension.rs

@@ -103,19 +103,20 @@ struct ContextServerDescriptorRegistryProxy {
 impl ExtensionContextServerProxy for ContextServerDescriptorRegistryProxy {
     fn register_context_server(&self, extension: Arc<dyn Extension>, id: Arc<str>, cx: &mut App) {
         self.context_server_factory_registry
-            .update(cx, |registry, _| {
+            .update(cx, |registry, cx| {
                 registry.register_context_server_descriptor(
                     id.clone(),
                     Arc::new(ContextServerDescriptor { id, extension })
                         as Arc<dyn registry::ContextServerDescriptor>,
+                    cx,
                 )
             });
     }
 
     fn unregister_context_server(&self, server_id: Arc<str>, cx: &mut App) {
         self.context_server_factory_registry
-            .update(cx, |registry, _| {
-                registry.unregister_context_server_descriptor_by_id(&server_id)
+            .update(cx, |registry, cx| {
+                registry.unregister_context_server_descriptor_by_id(&server_id, cx)
             });
     }
 }

crates/project/src/context_server_store/registry.rs

@@ -4,7 +4,7 @@ use anyhow::Result;
 use collections::HashMap;
 use context_server::ContextServerCommand;
 use extension::ContextServerConfiguration;
-use gpui::{App, AppContext as _, AsyncApp, Entity, Global, Task};
+use gpui::{App, AppContext as _, AsyncApp, Context, Entity, Global, Task};
 
 use crate::worktree_store::WorktreeStore;
 
@@ -66,12 +66,19 @@ impl ContextServerDescriptorRegistry {
         &mut self,
         id: Arc<str>,
         descriptor: Arc<dyn ContextServerDescriptor>,
+        cx: &mut Context<Self>,
     ) {
         self.context_servers.insert(id, descriptor);
+        cx.notify();
     }
 
     /// Unregisters the [`ContextServerDescriptor`] for the server with the given ID.
-    pub fn unregister_context_server_descriptor_by_id(&mut self, server_id: &str) {
+    pub fn unregister_context_server_descriptor_by_id(
+        &mut self,
+        server_id: &str,
+        cx: &mut Context<Self>,
+    ) {
         self.context_servers.remove(server_id);
+        cx.notify();
     }
 }

crates/settings/src/settings_json.rs

@@ -23,35 +23,26 @@ inventory::collect!(ParameterizedJsonSchema);
 
 const DEFS_PATH: &str = "#/$defs/";
 
-/// Replaces the JSON schema definition for some type, and returns a reference to it.
+/// Replaces the JSON schema definition for some type if it is in use (in the definitions list), and
+/// returns a reference to it.
+///
+/// This asserts that JsonSchema::schema_name() + "2" does not exist because this indicates that
+/// there are multiple types that use this name, and unfortunately schemars APIs do not support
+/// resolving this ambiguity - see https://github.com/GREsau/schemars/issues/449
+///
+/// This takes a closure for `schema` because some settings types are not available on the remote
+/// server, and so will crash when attempting to access e.g. GlobalThemeRegistry.
 pub fn replace_subschema<T: JsonSchema>(
     generator: &mut schemars::SchemaGenerator,
-    schema: schemars::Schema,
+    schema: impl Fn() -> schemars::Schema,
 ) -> schemars::Schema {
-    // The key in definitions may not match T::schema_name() if multiple types have the same name.
-    // This is a workaround for there being no straightforward way to get the key used for a type -
-    // see https://github.com/GREsau/schemars/issues/449
-    let ref_schema = generator.subschema_for::<T>();
-    if let Some(serde_json::Value::String(definition_pointer)) = ref_schema.get("$ref") {
-        if let Some(definition_name) = definition_pointer.strip_prefix(DEFS_PATH) {
-            generator
-                .definitions_mut()
-                .insert(definition_name.to_string(), schema.to_value());
-            return ref_schema;
-        } else {
-            log::error!(
-                "bug: expected `$ref` field to start with {DEFS_PATH}, \
-                got {definition_pointer}"
-            );
-        }
-    } else {
-        log::error!("bug: expected `$ref` field in result of `subschema_for`");
-    }
     // fallback on just using the schema name, which could collide.
     let schema_name = T::schema_name();
-    generator
-        .definitions_mut()
-        .insert(schema_name.to_string(), schema.to_value());
+    let definitions = generator.definitions_mut();
+    assert!(!definitions.contains_key(&format!("{schema_name}2")));
+    if definitions.contains_key(schema_name.as_ref()) {
+        definitions.insert(schema_name.to_string(), schema().to_value());
+    }
     Schema::new_ref(format!("{DEFS_PATH}{schema_name}"))
 }
 

crates/theme/src/settings.rs

@@ -978,11 +978,10 @@ pub struct ThemeName(pub Arc<str>);
 inventory::submit! {
     ParameterizedJsonSchema {
         add_and_get_ref: |generator, _params, cx| {
-            let schema = json_schema!({
+            replace_subschema::<ThemeName>(generator, || json_schema!({
                 "type": "string",
                 "enum": ThemeRegistry::global(cx).list_names(),
-            });
-            replace_subschema::<ThemeName>(generator, schema)
+            }))
         }
     }
 }
@@ -996,15 +995,14 @@ pub struct IconThemeName(pub Arc<str>);
 inventory::submit! {
     ParameterizedJsonSchema {
         add_and_get_ref: |generator, _params, cx| {
-            let schema = json_schema!({
+            replace_subschema::<IconThemeName>(generator, || json_schema!({
                 "type": "string",
                 "enum": ThemeRegistry::global(cx)
                     .list_icon_themes()
                     .into_iter()
                     .map(|icon_theme| icon_theme.name)
                     .collect::<Vec<SharedString>>(),
-            });
-            replace_subschema::<IconThemeName>(generator, schema)
+            }))
         }
     }
 }
@@ -1018,11 +1016,12 @@ pub struct FontFamilyName(pub Arc<str>);
 inventory::submit! {
     ParameterizedJsonSchema {
         add_and_get_ref: |generator, params, _cx| {
-            let schema = json_schema!({
-                "type": "string",
-                "enum": params.font_names,
-            });
-            replace_subschema::<FontFamilyName>(generator, schema)
+            replace_subschema::<FontFamilyName>(generator, || {
+                json_schema!({
+                    "type": "string",
+                    "enum": params.font_names,
+                })
+            })
         }
     }
 }

crates/zed/Cargo.toml

@@ -2,7 +2,7 @@
 description = "The fast, collaborative code editor."
 edition.workspace = true
 name = "zed"
-version = "0.194.0"
+version = "0.194.2"
 publish.workspace = true
 license = "GPL-3.0-or-later"
 authors = ["Zed Team <hi@zed.dev>"]

crates/zed/RELEASE_CHANNEL

@@ -1 +1 @@
-dev
+preview