History

agent: Extract unrelated cleanups from worktree-directory branch

- Remove subagent filter from set_title (lazy session lookup) The old code eagerly filtered out subagent threads in set_title. Now it always returns a NativeAgentSessionSetTitle that lazily looks up the session in run(), returning an error if not found. - Remove snake_case conversion for MCP tool name disambiguation Server IDs are now used as-is (to_string) instead of being converted to snake_case via heck. - Remove dead heck dependency from agent crate

Richard Feldman committed 8 months ago

Commit 4ff0d641 parent d60b2915 files · +20 −24Browse files

Unified · Split

5 files
  1. Cargo.lock +0 −1
  2. crates/agent/Cargo.toml +1 −1
  3. crates/agent/src/agent.rs +12 −15
  4. crates/agent/src/tests/mod.rs +3 −3
  5. crates/agent/src/thread.rs +4 −4
Cargo.lock+0−1
@@ -180,7 +180,6 @@ dependencies = [ · 179 unmodified lines
180 "gpui",180 "gpui",
181 "gpui_tokio",181 "gpui_tokio",
182 "handlebars 4.5.0",182 "handlebars 4.5.0",
183 "heck 0.5.0",
184 "html_to_markdown",183 "html_to_markdown",
185 "http_client",184 "http_client",
186 "indoc",185 "indoc",
crates/agent/Cargo.toml+1−1
@@ -38,7 +38,7 @@ futures.workspace = true · 37 unmodified lines
38git.workspace = true38git.workspace = true
39gpui.workspace = true39gpui.workspace = true
40handlebars = { workspace = true, features = ["rust-embed"] }40handlebars = { workspace = true, features = ["rust-embed"] }
41heck.workspace = true41
42html_to_markdown.workspace = true42html_to_markdown.workspace = true
43http_client.workspace = true43http_client.workspace = true
44indoc.workspace = true44indoc.workspace = true
crates/agent/src/agent.rs+12−15
@@ -1396,19 +1396,12 @@ impl acp_thread::AgentConnection for NativeAgentConnection { · 1395 unmodified lines
1396 fn set_title(1396 fn set_title(
1397 &self,1397 &self,
1398 session_id: &acp::SessionId,1398 session_id: &acp::SessionId,
1399 cx: &App,1399 _cx: &App,
1400 ) -> Option<Rc<dyn acp_thread::AgentSessionSetTitle>> {1400 ) -> Option<Rc<dyn acp_thread::AgentSessionSetTitle>> {
1401 self.0.read_with(cx, |agent, _cx| {1401 Some(Rc::new(NativeAgentSessionSetTitle {
1402 agent1402 connection: self.clone(),
1403 .sessions1403 session_id: session_id.clone(),
1404 .get(session_id)1404 }) as _)
1405 .filter(|s| !s.thread.read(cx).is_subagent())
1406 .map(|session| {
1407 Rc::new(NativeAgentSessionSetTitle {
1408 thread: session.thread.clone(),
1409 }) as _
1410 })
1411 })
1412 }1405 }
1413 1406
1414 fn session_list(&self, cx: &mut App) -> Option<Rc<dyn AgentSessionList>> {1407 fn session_list(&self, cx: &mut App) -> Option<Rc<dyn AgentSessionList>> {
@@ -1567,13 +1560,17 @@ impl acp_thread::AgentSessionRetry for NativeAgentSessionRetry { · 152 unmodified lines
1567}1560}
1568 1561
1569struct NativeAgentSessionSetTitle {1562struct NativeAgentSessionSetTitle {
1570 thread: Entity<Thread>,1563 connection: NativeAgentConnection,
1564 session_id: acp::SessionId,
1571}1565}
1572 1566
1573impl acp_thread::AgentSessionSetTitle for NativeAgentSessionSetTitle {1567impl acp_thread::AgentSessionSetTitle for NativeAgentSessionSetTitle {
1574 fn run(&self, title: SharedString, cx: &mut App) -> Task<Result<()>> {1568 fn run(&self, title: SharedString, cx: &mut App) -> Task<Result<()>> {
1575 self.thread1569 let Some(session) = self.connection.0.read(cx).sessions.get(&self.session_id) else {
1576 .update(cx, |thread, cx| thread.set_title(title, cx));1570 return Task::ready(Err(anyhow!("session not found")));
1571 };
1572 let thread = session.thread.clone();
1573 thread.update(cx, |thread, cx| thread.set_title(title, cx));
1577 Task::ready(Ok(()))1574 Task::ready(Ok(()))
1578 }1575 }
1579}1576}
crates/agent/src/tests/mod.rs+3−3
@@ -1828,11 +1828,11 @@ async fn test_mcp_tool_truncation(cx: &mut TestAppContext) { · 1827 unmodified lines
1828 cx,1828 cx,
1829 );1829 );
1830 1830
1831 // Server with spaces in name - tests snake_case conversion for API compatibility1831 // Server with spaces in name - tests disambiguation with server ID prefix
1832 let _server4_calls = setup_context_server(1832 let _server4_calls = setup_context_server(
1833 "Azure DevOps",1833 "Azure DevOps",
1834 vec![context_server::types::Tool {1834 vec![context_server::types::Tool {
1835 name: "echo".into(), // Also conflicts - will be disambiguated as azure_dev_ops_echo1835 name: "echo".into(), // Also conflicts - will be disambiguated as Azure DevOps_echo
1836 description: None,1836 description: None,
1837 input_schema: serde_json::to_value(EchoTool::input_schema(1837 input_schema: serde_json::to_value(EchoTool::input_schema(
1838 LanguageModelToolSchemaFormat::JsonSchema,1838 LanguageModelToolSchemaFormat::JsonSchema,
@@ -1855,7 +1855,7 @@ async fn test_mcp_tool_truncation(cx: &mut TestAppContext) { · 16 unmodified lines
1855 assert_eq!(1855 assert_eq!(
1856 tool_names_for_completion(&completion),1856 tool_names_for_completion(&completion),
1857 vec![1857 vec![
1858 "azure_dev_ops_echo",1858 "Azure DevOps_echo",
1859 "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",1859 "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
1860 "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",1860 "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
1861 "delay",1861 "delay",
crates/agent/src/thread.rs+4−4
@@ -32,7 +32,7 @@ use futures::{ · 31 unmodified lines
32use gpui::{32use gpui::{
33 App, AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Task, WeakEntity,33 App, AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Task, WeakEntity,
34};34};
35use heck::ToSnakeCase as _;35
36use language_model::{36use language_model::{
37 LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId,37 LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelId,
38 LanguageModelImage, LanguageModelProviderId, LanguageModelRegistry, LanguageModelRequest,38 LanguageModelImage, LanguageModelProviderId, LanguageModelRegistry, LanguageModelRequest,
@@ -2525,14 +2525,14 @@ impl Thread { · 2486 unmodified lines
2525 }2525 }
2526 2526
2527 // When there are duplicate tool names, disambiguate by prefixing them2527 // When there are duplicate tool names, disambiguate by prefixing them
2528 // with the server ID (converted to snake_case for API compatibility).2528 // with the server ID. In the rare case there isn't enough space for the
2529 // In the rare case there isn't enough space for the disambiguated tool2529 // disambiguated tool
2530 // name, keep only the last tool with this name.2530 // name, keep only the last tool with this name.
2531 for (server_id, tool_name, tool) in context_server_tools {2531 for (server_id, tool_name, tool) in context_server_tools {
2532 if duplicate_tool_names.contains(&tool_name) {2532 if duplicate_tool_names.contains(&tool_name) {
2533 let available = MAX_TOOL_NAME_LENGTH.saturating_sub(tool_name.len());2533 let available = MAX_TOOL_NAME_LENGTH.saturating_sub(tool_name.len());
2534 if available >= 2 {2534 if available >= 2 {
2535 let mut disambiguated = server_id.0.to_snake_case();2535 let mut disambiguated = server_id.0.to_string();
2536 disambiguated.truncate(available - 1);2536 disambiguated.truncate(available - 1);
2537 disambiguated.push('_');2537 disambiguated.push('_');
2538 disambiguated.push_str(&tool_name);2538 disambiguated.push_str(&tool_name);