From 9a454a7d2cad2a0132c36b49dbb8eacabf0080f1 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 20 Oct 2025 20:49:05 +0200 Subject: [PATCH] acp: Fix following for agents that only provide locations (#40710) We were dropping the entities once we created the buffers, so the weak entities could never be upgraded. This treats new locations we see the same as we would for a read/write call and stores the entity so that we can follow like we normally would. Release Notes: - acp: Fix following not working with certain tool calls. --- crates/acp_thread/src/acp_thread.rs | 72 +++++++++++++++++++---------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index f041c9120a52e4b9c23acf121110f00ee157641e..ed2e01f0b37197d6878dee699fba43ed3410066f 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -328,7 +328,7 @@ impl ToolCall { location: acp::ToolCallLocation, project: WeakEntity, cx: &mut AsyncApp, - ) -> Option { + ) -> Option { let buffer = project .update(cx, |project, cx| { project @@ -350,17 +350,14 @@ impl ToolCall { }) .ok()?; - Some(AgentLocation { - buffer: buffer.downgrade(), - position, - }) + Some(ResolvedLocation { buffer, position }) } fn resolve_locations( &self, project: Entity, cx: &mut App, - ) -> Task>> { + ) -> Task>> { let locations = self.locations.clone(); project.update(cx, |_, cx| { cx.spawn(async move |project, cx| { @@ -374,6 +371,23 @@ impl ToolCall { } } +// Separate so we can hold a strong reference to the buffer +// for saving on the thread +#[derive(Clone, Debug, PartialEq, Eq)] +struct ResolvedLocation { + buffer: Entity, + position: Anchor, +} + +impl From<&ResolvedLocation> for AgentLocation { + fn from(value: &ResolvedLocation) -> Self { + Self { + buffer: value.buffer.downgrade(), + position: value.position, + } + } +} + #[derive(Debug)] pub enum ToolCallStatus { /// The tool call hasn't started running yet, but we start showing it to @@ -1393,35 +1407,43 @@ impl AcpThread { let task = tool_call.resolve_locations(project, cx); cx.spawn(async move |this, cx| { let resolved_locations = task.await; + this.update(cx, |this, cx| { let project = this.project.clone(); + + for location in resolved_locations.iter().flatten() { + this.shared_buffers + .insert(location.buffer.clone(), location.buffer.read(cx).snapshot()); + } let Some((ix, tool_call)) = this.tool_call_mut(&id) else { return; }; + if let Some(Some(location)) = resolved_locations.last() { project.update(cx, |project, cx| { - if let Some(agent_location) = project.agent_location() { - let should_ignore = agent_location.buffer == location.buffer - && location - .buffer - .update(cx, |buffer, _| { - let snapshot = buffer.snapshot(); - let old_position = - agent_location.position.to_point(&snapshot); - let new_position = location.position.to_point(&snapshot); - // ignore this so that when we get updates from the edit tool - // the position doesn't reset to the startof line - old_position.row == new_position.row - && old_position.column > new_position.column - }) - .ok() - .unwrap_or_default(); - if !should_ignore { - project.set_agent_location(Some(location.clone()), cx); - } + let should_ignore = if let Some(agent_location) = project.agent_location() { + let snapshot = location.buffer.read(cx).snapshot(); + let old_position = agent_location.position.to_point(&snapshot); + let new_position = location.position.to_point(&snapshot); + agent_location.buffer == location.buffer + // ignore this so that when we get updates from the edit tool + // the position doesn't reset to the startof line + && (old_position.row == new_position.row + && old_position.column > new_position.column) + } else { + false + }; + if !should_ignore { + project.set_agent_location(Some(location.into()), cx); } }); } + + let resolved_locations = resolved_locations + .iter() + .map(|l| l.as_ref().map(|l| AgentLocation::from(l))) + .collect::>(); + if tool_call.resolved_locations != resolved_locations { tool_call.resolved_locations = resolved_locations; cx.emit(AcpThreadEvent::EntryUpdated(ix));