Skip to content

Commit e1a96b6

Browse files
benbrandtbenbrandt
authored
acp: Tool name prep (#36726)
Prep work for deduping tool names Release Notes: - N/A
1 parent ca139b7 commit e1a96b6

21 files changed

+126
-123
lines changed

‎crates/agent2/src/agent.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -857,7 +857,6 @@ impl acp_thread::AgentConnection for NativeAgentConnection {
857857
cx.spawn(async move |cx| {
858858
log::debug!("Starting thread creation in async context");
859859

860-
let action_log = cx.new(|_cx| ActionLog::new(project.clone()))?;
861860
// Create Thread
862861
let thread = agent.update(
863862
cx,
@@ -878,7 +877,6 @@ impl acp_thread::AgentConnection for NativeAgentConnection {
878877
project.clone(),
879878
agent.project_context.clone(),
880879
agent.context_server_registry.clone(),
881-
action_log.clone(),
882880
agent.templates.clone(),
883881
default_model,
884882
cx,

‎crates/agent2/src/tests/mod.rs

Lines changed: 26 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use super::*;
22
use acp_thread::{AgentConnection, AgentModelGroupName, AgentModelList, UserMessageId};
3-
use action_log::ActionLog;
43
use agent_client_protocol::{self as acp};
54
use agent_settings::AgentProfileId;
65
use anyhow::Result;
@@ -224,7 +223,7 @@ async fn test_prompt_caching(cx: &mut TestAppContext) {
224223

225224
let tool_use = LanguageModelToolUse {
226225
id: "tool_1".into(),
227-
name: EchoTool.name().into(),
226+
name: EchoTool::name().into(),
228227
raw_input: json!({"text": "test"}).to_string(),
229228
input: json!({"text": "test"}),
230229
is_input_complete: true,
@@ -237,7 +236,7 @@ async fn test_prompt_caching(cx: &mut TestAppContext) {
237236
let completion = fake_model.pending_completions().pop().unwrap();
238237
let tool_result = LanguageModelToolResult {
239238
tool_use_id: "tool_1".into(),
240-
tool_name: EchoTool.name().into(),
239+
tool_name: EchoTool::name().into(),
241240
is_error: false,
242241
content: "test".into(),
243242
output: Some("test".into()),
@@ -307,7 +306,7 @@ async fn test_basic_tool_calls(cx: &mut TestAppContext) {
307306
// Test a tool calls that's likely to complete *after* streaming stops.
308307
let events = thread
309308
.update(cx, |thread, cx| {
310-
thread.remove_tool(&AgentTool::name(&EchoTool));
309+
thread.remove_tool(&EchoTool::name());
311310
thread.add_tool(DelayTool);
312311
thread.send(
313312
UserMessageId::new(),
@@ -411,7 +410,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
411410
fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse(
412411
LanguageModelToolUse {
413412
id: "tool_id_1".into(),
414-
name: ToolRequiringPermission.name().into(),
413+
name: ToolRequiringPermission::name().into(),
415414
raw_input: "{}".into(),
416415
input: json!({}),
417416
is_input_complete: true,
@@ -420,7 +419,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
420419
fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse(
421420
LanguageModelToolUse {
422421
id: "tool_id_2".into(),
423-
name: ToolRequiringPermission.name().into(),
422+
name: ToolRequiringPermission::name().into(),
424423
raw_input: "{}".into(),
425424
input: json!({}),
426425
is_input_complete: true,
@@ -451,14 +450,14 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
451450
vec![
452451
language_model::MessageContent::ToolResult(LanguageModelToolResult {
453452
tool_use_id: tool_call_auth_1.tool_call.id.0.to_string().into(),
454-
tool_name: ToolRequiringPermission.name().into(),
453+
tool_name: ToolRequiringPermission::name().into(),
455454
is_error: false,
456455
content: "Allowed".into(),
457456
output: Some("Allowed".into())
458457
}),
459458
language_model::MessageContent::ToolResult(LanguageModelToolResult {
460459
tool_use_id: tool_call_auth_2.tool_call.id.0.to_string().into(),
461-
tool_name: ToolRequiringPermission.name().into(),
460+
tool_name: ToolRequiringPermission::name().into(),
462461
is_error: true,
463462
content: "Permission to run tool denied by user".into(),
464463
output: None
@@ -470,7 +469,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
470469
fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse(
471470
LanguageModelToolUse {
472471
id: "tool_id_3".into(),
473-
name: ToolRequiringPermission.name().into(),
472+
name: ToolRequiringPermission::name().into(),
474473
raw_input: "{}".into(),
475474
input: json!({}),
476475
is_input_complete: true,
@@ -492,7 +491,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
492491
vec![language_model::MessageContent::ToolResult(
493492
LanguageModelToolResult {
494493
tool_use_id: tool_call_auth_3.tool_call.id.0.to_string().into(),
495-
tool_name: ToolRequiringPermission.name().into(),
494+
tool_name: ToolRequiringPermission::name().into(),
496495
is_error: false,
497496
content: "Allowed".into(),
498497
output: Some("Allowed".into())
@@ -504,7 +503,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
504503
fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse(
505504
LanguageModelToolUse {
506505
id: "tool_id_4".into(),
507-
name: ToolRequiringPermission.name().into(),
506+
name: ToolRequiringPermission::name().into(),
508507
raw_input: "{}".into(),
509508
input: json!({}),
510509
is_input_complete: true,
@@ -519,7 +518,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) {
519518
vec![language_model::MessageContent::ToolResult(
520519
LanguageModelToolResult {
521520
tool_use_id: "tool_id_4".into(),
522-
tool_name: ToolRequiringPermission.name().into(),
521+
tool_name: ToolRequiringPermission::name().into(),
523522
is_error: false,
524523
content: "Allowed".into(),
525524
output: Some("Allowed".into())
@@ -571,7 +570,7 @@ async fn test_resume_after_tool_use_limit(cx: &mut TestAppContext) {
571570
cx.run_until_parked();
572571
let tool_use = LanguageModelToolUse {
573572
id: "tool_id_1".into(),
574-
name: EchoTool.name().into(),
573+
name: EchoTool::name().into(),
575574
raw_input: "{}".into(),
576575
input: serde_json::to_value(&EchoToolInput { text: "def".into() }).unwrap(),
577576
is_input_complete: true,
@@ -584,7 +583,7 @@ async fn test_resume_after_tool_use_limit(cx: &mut TestAppContext) {
584583
let completion = fake_model.pending_completions().pop().unwrap();
585584
let tool_result = LanguageModelToolResult {
586585
tool_use_id: "tool_id_1".into(),
587-
tool_name: EchoTool.name().into(),
586+
tool_name: EchoTool::name().into(),
588587
is_error: false,
589588
content: "def".into(),
590589
output: Some("def".into()),
@@ -690,14 +689,14 @@ async fn test_send_after_tool_use_limit(cx: &mut TestAppContext) {
690689

691690
let tool_use = LanguageModelToolUse {
692691
id: "tool_id_1".into(),
693-
name: EchoTool.name().into(),
692+
name: EchoTool::name().into(),
694693
raw_input: "{}".into(),
695694
input: serde_json::to_value(&EchoToolInput { text: "def".into() }).unwrap(),
696695
is_input_complete: true,
697696
};
698697
let tool_result = LanguageModelToolResult {
699698
tool_use_id: "tool_id_1".into(),
700-
tool_name: EchoTool.name().into(),
699+
tool_name: EchoTool::name().into(),
701700
is_error: false,
702701
content: "def".into(),
703702
output: Some("def".into()),
@@ -874,14 +873,14 @@ async fn test_profiles(cx: &mut TestAppContext) {
874873
"test-1": {
875874
"name": "Test Profile 1",
876875
"tools": {
877-
EchoTool.name(): true,
878-
DelayTool.name(): true,
876+
EchoTool::name(): true,
877+
DelayTool::name(): true,
879878
}
880879
},
881880
"test-2": {
882881
"name": "Test Profile 2",
883882
"tools": {
884-
InfiniteTool.name(): true,
883+
InfiniteTool::name(): true,
885884
}
886885
}
887886
}
@@ -910,7 +909,7 @@ async fn test_profiles(cx: &mut TestAppContext) {
910909
.iter()
911910
.map(|tool| tool.name.clone())
912911
.collect();
913-
assert_eq!(tool_names, vec![DelayTool.name(), EchoTool.name()]);
912+
assert_eq!(tool_names, vec![DelayTool::name(), EchoTool::name()]);
914913
fake_model.end_last_completion_stream();
915914

916915
// Switch to test-2 profile, and verify that it has only the infinite tool.
@@ -929,7 +928,7 @@ async fn test_profiles(cx: &mut TestAppContext) {
929928
.iter()
930929
.map(|tool| tool.name.clone())
931930
.collect();
932-
assert_eq!(tool_names, vec![InfiniteTool.name()]);
931+
assert_eq!(tool_names, vec![InfiniteTool::name()]);
933932
}
934933

935934
#[gpui::test]
@@ -1552,7 +1551,7 @@ async fn test_tool_updates_to_completion(cx: &mut TestAppContext) {
15521551
fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse(
15531552
LanguageModelToolUse {
15541553
id: "1".into(),
1555-
name: ThinkingTool.name().into(),
1554+
name: ThinkingTool::name().into(),
15561555
raw_input: input.to_string(),
15571556
input,
15581557
is_input_complete: false,
@@ -1840,11 +1839,11 @@ async fn setup(cx: &mut TestAppContext, model: TestModel) -> ThreadTest {
18401839
"test-profile": {
18411840
"name": "Test Profile",
18421841
"tools": {
1843-
EchoTool.name(): true,
1844-
DelayTool.name(): true,
1845-
WordListTool.name(): true,
1846-
ToolRequiringPermission.name(): true,
1847-
InfiniteTool.name(): true,
1842+
EchoTool::name(): true,
1843+
DelayTool::name(): true,
1844+
WordListTool::name(): true,
1845+
ToolRequiringPermission::name(): true,
1846+
InfiniteTool::name(): true,
18481847
}
18491848
}
18501849
}
@@ -1903,13 +1902,11 @@ async fn setup(cx: &mut TestAppContext, model: TestModel) -> ThreadTest {
19031902
let project_context = cx.new(|_cx| ProjectContext::default());
19041903
let context_server_registry =
19051904
cx.new(|cx| ContextServerRegistry::new(project.read(cx).context_server_store(), cx));
1906-
let action_log = cx.new(|_| ActionLog::new(project.clone()));
19071905
let thread = cx.new(|cx| {
19081906
Thread::new(
19091907
project,
19101908
project_context.clone(),
19111909
context_server_registry,
1912-
action_log,
19131910
templates,
19141911
Some(model.clone()),
19151912
cx,

‎crates/agent2/src/tests/test_tools.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,11 @@ impl AgentTool for EchoTool {
1616
type Input = EchoToolInput;
1717
type Output = String;
1818

19-
fn name(&self) -> SharedString {
20-
"echo".into()
19+
fn name() -> &'static str {
20+
"echo"
2121
}
2222

23-
fn kind(&self) -> acp::ToolKind {
23+
fn kind() -> acp::ToolKind {
2424
acp::ToolKind::Other
2525
}
2626

@@ -51,8 +51,8 @@ impl AgentTool for DelayTool {
5151
type Input = DelayToolInput;
5252
type Output = String;
5353

54-
fn name(&self) -> SharedString {
55-
"delay".into()
54+
fn name() -> &'static str {
55+
"delay"
5656
}
5757

5858
fn initial_title(&self, input: Result<Self::Input, serde_json::Value>) -> SharedString {
@@ -63,7 +63,7 @@ impl AgentTool for DelayTool {
6363
}
6464
}
6565

66-
fn kind(&self) -> acp::ToolKind {
66+
fn kind() -> acp::ToolKind {
6767
acp::ToolKind::Other
6868
}
6969

@@ -92,11 +92,11 @@ impl AgentTool for ToolRequiringPermission {
9292
type Input = ToolRequiringPermissionInput;
9393
type Output = String;
9494

95-
fn name(&self) -> SharedString {
96-
"tool_requiring_permission".into()
95+
fn name() -> &'static str {
96+
"tool_requiring_permission"
9797
}
9898

99-
fn kind(&self) -> acp::ToolKind {
99+
fn kind() -> acp::ToolKind {
100100
acp::ToolKind::Other
101101
}
102102

@@ -127,11 +127,11 @@ impl AgentTool for InfiniteTool {
127127
type Input = InfiniteToolInput;
128128
type Output = String;
129129

130-
fn name(&self) -> SharedString {
131-
"infinite".into()
130+
fn name() -> &'static str {
131+
"infinite"
132132
}
133133

134-
fn kind(&self) -> acp::ToolKind {
134+
fn kind() -> acp::ToolKind {
135135
acp::ToolKind::Other
136136
}
137137

@@ -178,11 +178,11 @@ impl AgentTool for WordListTool {
178178
type Input = WordListInput;
179179
type Output = String;
180180

181-
fn name(&self) -> SharedString {
182-
"word_list".into()
181+
fn name() -> &'static str {
182+
"word_list"
183183
}
184184

185-
fn kind(&self) -> acp::ToolKind {
185+
fn kind() -> acp::ToolKind {
186186
acp::ToolKind::Other
187187
}
188188

‎crates/agent2/src/thread.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -544,12 +544,12 @@ impl Thread {
544544
project: Entity<Project>,
545545
project_context: Entity<ProjectContext>,
546546
context_server_registry: Entity<ContextServerRegistry>,
547-
action_log: Entity<ActionLog>,
548547
templates: Arc<Templates>,
549548
model: Option<Arc<dyn LanguageModel>>,
550549
cx: &mut Context<Self>,
551550
) -> Self {
552551
let profile_id = AgentSettings::get_global(cx).default_profile.clone();
552+
let action_log = cx.new(|_cx| ActionLog::new(project.clone()));
553553
Self {
554554
id: acp::SessionId(uuid::Uuid::new_v4().to_string().into()),
555555
prompt_id: PromptId::new(),
@@ -959,11 +959,11 @@ impl Thread {
959959
));
960960
self.add_tool(TerminalTool::new(self.project.clone(), cx));
961961
self.add_tool(ThinkingTool);
962-
self.add_tool(WebSearchTool); // TODO: Enable this only if it's a zed model.
962+
self.add_tool(WebSearchTool);
963963
}
964964

965-
pub fn add_tool(&mut self, tool: impl AgentTool) {
966-
self.tools.insert(tool.name(), tool.erase());
965+
pub fn add_tool<T: AgentTool>(&mut self, tool: T) {
966+
self.tools.insert(T::name().into(), tool.erase());
967967
}
968968

969969
pub fn remove_tool(&mut self, name: &str) -> bool {
@@ -1989,7 +1989,7 @@ where
19891989
type Input: for<'de> Deserialize<'de> + Serialize + JsonSchema;
19901990
type Output: for<'de> Deserialize<'de> + Serialize + Into<LanguageModelToolResultContent>;
19911991

1992-
fn name(&self) -> SharedString;
1992+
fn name() -> &'static str;
19931993

19941994
fn description(&self) -> SharedString {
19951995
let schema = schemars::schema_for!(Self::Input);
@@ -2001,7 +2001,7 @@ where
20012001
)
20022002
}
20032003

2004-
fn kind(&self) -> acp::ToolKind;
2004+
fn kind() -> acp::ToolKind;
20052005

20062006
/// The initial tool title to display. Can be updated during the tool run.
20072007
fn initial_title(&self, input: Result<Self::Input, serde_json::Value>) -> SharedString;
@@ -2077,15 +2077,15 @@ where
20772077
T: AgentTool,
20782078
{
20792079
fn name(&self) -> SharedString {
2080-
self.0.name()
2080+
T::name().into()
20812081
}
20822082

20832083
fn description(&self) -> SharedString {
20842084
self.0.description()
20852085
}
20862086

20872087
fn kind(&self) -> agent_client_protocol::ToolKind {
2088-
self.0.kind()
2088+
T::kind()
20892089
}
20902090

20912091
fn initial_title(&self, input: serde_json::Value) -> SharedString {

0 commit comments

Comments
 (0)