agent: More subagent fixes (#50489)

- Skip agent location updates for subagent threads
- Hide edits summary for subagent thread
- Fix tool permission granularity selection from parent thread

Release Notes:

- N/A

---------

Co-authored-by: Bennet Bo Fenner <bennetbo@gmx.de>
This commit is contained in:
Ben BrandtandBennet Bo Fenner authored and GitHub committed 2026-03-02 14:42:01 +00:00
1 parent 8f329d1ee3
commit 5e9ee9ea4a
9 files changed
+178 -313

No files matched your search

+33 -24
View File
@@ -1713,6 +1713,7 @@ impl AcpThread {
pub fn resolve_locations(&mut self, id: acp::ToolCallId, cx: &mut Context<Self>) {
let project = self.project.clone();
let should_update_agent_location = self.parent_session_id.is_none();
let Some((_, tool_call)) = self.tool_call_mut(&id) else {
return;
};
@@ -1748,7 +1749,7 @@ impl AcpThread {
} else {
false
};
if !should_ignore {
if !should_ignore && should_update_agent_location {
project.set_agent_location(Some(location.into()), cx);
}
});
@@ -1979,8 +1980,10 @@ impl AcpThread {
.await?;
this.update(cx, |this, cx| {
this.project
.update(cx, |project, cx| project.set_agent_location(None, cx));
if this.parent_session_id.is_none() {
this.project
.update(cx, |project, cx| project.set_agent_location(None, cx));
}
let Ok(response) = response else {
// tx dropped, just return
return Ok(None);
@@ -2252,6 +2255,7 @@ impl AcpThread {
let limit = limit.unwrap_or(u32::MAX);
let project = self.project.clone();
let action_log = self.action_log.clone();
let should_update_agent_location = self.parent_session_id.is_none();
cx.spawn(async move |this, cx| {
let load = project.update(cx, |project, cx| {
let path = project
@@ -2302,15 +2306,17 @@ impl AcpThread {
let start = snapshot.anchor_before(start_position);
let end = snapshot.anchor_before(Point::new(line.saturating_add(limit), 0));
project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: start,
}),
cx,
);
});
if should_update_agent_location {
project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: start,
}),
cx,
);
});
}
Ok(snapshot.text_for_range(start..end).collect::<String>())
})
@@ -2324,6 +2330,7 @@ impl AcpThread {
) -> Task<Result<()>> {
let project = self.project.clone();
let action_log = self.action_log.clone();
let should_update_agent_location = self.parent_session_id.is_none();
cx.spawn(async move |this, cx| {
let load = project.update(cx, |project, cx| {
let path = project
@@ -2351,18 +2358,20 @@ impl AcpThread {
})
.await;
project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: edits
.last()
.map(|(range, _)| range.end)
.unwrap_or(Anchor::min_for_buffer(buffer.read(cx).remote_id())),
}),
cx,
);
});
if should_update_agent_location {
project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: edits
.last()
.map(|(range, _)| range.end)
.unwrap_or(Anchor::min_for_buffer(buffer.read(cx).remote_id())),
}),
cx,
);
});
}
let format_on_save = cx.update(|cx| {
action_log.update(cx, |action_log, cx| {
+48 -36
View File
@@ -84,6 +84,7 @@ pub struct EditAgent {
templates: Arc<Templates>,
edit_format: EditFormat,
thinking_allowed: bool,
update_agent_location: bool,
}
impl EditAgent {
@@ -94,6 +95,7 @@ impl EditAgent {
templates: Arc<Templates>,
edit_format: EditFormat,
allow_thinking: bool,
update_agent_location: bool,
) -> Self {
EditAgent {
model,
@@ -102,6 +104,7 @@ impl EditAgent {
templates,
edit_format,
thinking_allowed: allow_thinking,
update_agent_location,
}
}
@@ -170,15 +173,17 @@ impl EditAgent {
) -> Result<()> {
let buffer_id = cx.update(|cx| {
let buffer_id = buffer.read(cx).remote_id();
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: language::Anchor::min_for_buffer(buffer_id),
}),
cx,
)
});
if self.update_agent_location {
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: language::Anchor::min_for_buffer(buffer_id),
}),
cx,
)
});
}
buffer_id
});
@@ -190,15 +195,17 @@ impl EditAgent {
.ok()
};
let set_agent_location = |cx: &mut _| {
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: language::Anchor::max_for_buffer(buffer_id),
}),
cx,
)
})
if self.update_agent_location {
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: language::Anchor::max_for_buffer(buffer_id),
}),
cx,
)
})
}
};
let mut first_chunk = true;
while let Some(event) = parse_rx.next().await {
@@ -302,15 +309,17 @@ impl EditAgent {
if let Some(old_range) = old_range {
let old_range = snapshot.anchor_before(old_range.start)
..snapshot.anchor_before(old_range.end);
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: old_range.end,
}),
cx,
);
});
if self.update_agent_location {
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: old_range.end,
}),
cx,
);
});
}
output_events
.unbounded_send(EditAgentOutputEvent::ResolvingEditRange(old_range))
.ok();
@@ -383,15 +392,17 @@ impl EditAgent {
});
self.action_log
.update(cx, |log, cx| log.buffer_edited(buffer.clone(), cx));
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: max_edit_end,
}),
cx,
);
});
if self.update_agent_location {
self.project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: max_edit_end,
}),
cx,
);
});
}
(min_edit_start, max_edit_end)
});
output_events
@@ -1390,6 +1401,7 @@ mod tests {
Templates::new(),
EditFormat::XmlTags,
thinking_allowed,
true,
)
}
+1
View File
@@ -1469,6 +1469,7 @@ impl EditAgentTest {
Templates::new(),
edit_format,
true,
true,
),
project,
judge_model,
+6 -2
View File
@@ -253,7 +253,7 @@ impl AgentTool for EditFileTool {
error: "thread was dropped".to_string(),
})?;
let (project_path, abs_path, allow_thinking, authorize) =
let (project_path, abs_path, allow_thinking, update_agent_location, authorize) =
cx.update(|cx| {
let project_path = resolve_path(&input, project.clone(), cx).map_err(|err| {
EditFileToolOutput::Error {
@@ -271,8 +271,11 @@ impl AgentTool for EditFileTool {
.thread
.read_with(cx, |thread, _cx| thread.thinking_enabled())
.unwrap_or(true);
let update_agent_location = self.thread.read_with(cx, |thread, _cx| !thread.is_subagent()).unwrap_or_default();
let authorize = self.authorize(&input, &event_stream, cx);
Ok::<_, EditFileToolOutput>((project_path, abs_path, allow_thinking, authorize))
Ok::<_, EditFileToolOutput>((project_path, abs_path, allow_thinking, update_agent_location, authorize))
})?;
let result: anyhow::Result<EditFileToolOutput> = async {
@@ -293,6 +296,7 @@ impl AgentTool for EditFileTool {
self.templates.clone(),
edit_format,
allow_thinking,
update_agent_location,
);
let buffer = project
+13 -9
View File
@@ -212,7 +212,6 @@ impl AgentTool for ReadFileTool {
});
if is_image {
let image_entity: Entity<ImageItem> = cx
.update(|cx| {
self.project.update(cx, |project, cx| {
@@ -269,6 +268,9 @@ impl AgentTool for ReadFileTool {
.ok();
}
let update_agent_location = self.thread.read_with(cx, |thread, _cx| !thread.is_subagent()).unwrap_or_default();
let mut anchor = None;
// Check if specific line ranges are provided
@@ -328,15 +330,17 @@ impl AgentTool for ReadFileTool {
};
project.update(cx, |project, cx| {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: anchor.unwrap_or_else(|| {
text::Anchor::min_for_buffer(buffer.read(cx).remote_id())
if update_agent_location {
project.set_agent_location(
Some(AgentLocation {
buffer: buffer.downgrade(),
position: anchor.unwrap_or_else(|| {
text::Anchor::min_for_buffer(buffer.read(cx).remote_id())
}),
}),
}),
cx,
);
cx,
);
}
if let Ok(LanguageModelToolResultContent::Text(text)) = &result {
let text: &str = text;
let markdown = MarkdownCodeBlock {
@@ -220,9 +220,15 @@ impl StreamingEditFileTool {
}
fn set_agent_location(&self, buffer: WeakEntity<Buffer>, position: text::Anchor, cx: &mut App) {
self.project.update(cx, |project, cx| {
project.set_agent_location(Some(AgentLocation { buffer, position }), cx);
});
let should_update_agent_location = self
.thread
.read_with(cx, |thread, _cx| !thread.is_subagent())
.unwrap_or_default();
if should_update_agent_location {
self.project.update(cx, |project, cx| {
project.set_agent_location(Some(AgentLocation { buffer, position }), cx);
});
}
}
}
-12
View File
@@ -179,18 +179,6 @@ pub struct AuthorizeToolCall {
pub option_kind: String,
}
/// Action to select a permission granularity option from the dropdown.
/// This updates the selected granularity without triggering authorization.
#[derive(Clone, PartialEq, Deserialize, JsonSchema, Action)]
#[action(namespace = agent)]
#[serde(deny_unknown_fields)]
pub struct SelectPermissionGranularity {
/// The tool call ID for which to select the granularity.
pub tool_call_id: String,
/// The index of the selected granularity option.
pub index: usize,
}
/// Creates a new conversation thread, optionally based on an existing thread.
#[derive(Default, Clone, PartialEq, Deserialize, JsonSchema, Action)]
#[action(namespace = agent)]
+29 -179
View File
@@ -74,9 +74,9 @@ use crate::{
AgentDiffPane, AgentInitialContent, AgentPanel, AllowAlways, AllowOnce, AuthorizeToolCall,
ClearMessageQueue, CycleFavoriteModels, CycleModeSelector, CycleThinkingEffort,
EditFirstQueuedMessage, ExpandMessageEditor, Follow, KeepAll, NewThread, OpenAddContextMenu,
OpenAgentDiff, OpenHistory, RejectAll, RejectOnce, RemoveFirstQueuedMessage,
SelectPermissionGranularity, SendImmediately, SendNextQueuedMessage, ToggleFastMode,
ToggleProfileSelector, ToggleThinkingEffortMenu, ToggleThinkingMode, UndoLastReject,
OpenAgentDiff, OpenHistory, RejectAll, RejectOnce, RemoveFirstQueuedMessage, SendImmediately,
SendNextQueuedMessage, ToggleFastMode, ToggleProfileSelector, ToggleThinkingEffortMenu,
ToggleThinkingMode, UndoLastReject,
};
const STOPWATCH_THRESHOLD: Duration = Duration::from_secs(30);
@@ -155,6 +155,9 @@ pub(crate) struct Conversation {
threads: HashMap<acp::SessionId, Entity<AcpThread>>,
permission_requests: IndexMap<acp::SessionId, Vec<acp::ToolCallId>>,
subscriptions: Vec<Subscription>,
/// Tracks the selected granularity index for each tool call's permission dropdown.
/// The index corresponds to the position in the allow_options list.
selected_permission_granularity: HashMap<acp::SessionId, HashMap<acp::ToolCallId, usize>>,
}
impl Conversation {
@@ -196,6 +199,29 @@ impl Conversation {
.insert(thread.read(cx).session_id().clone(), thread);
}
pub fn selected_permission_granularity(
&self,
session_id: &acp::SessionId,
tool_call_id: &acp::ToolCallId,
) -> Option<usize> {
self.selected_permission_granularity
.get(session_id)
.and_then(|map| map.get(tool_call_id))
.copied()
}
pub fn set_selected_permission_granularity(
&mut self,
session_id: acp::SessionId,
tool_call_id: acp::ToolCallId,
granularity: usize,
) {
self.selected_permission_granularity
.entry(session_id)
.or_default()
.insert(tool_call_id, granularity);
}
pub fn pending_tool_call<'a>(
&'a self,
session_id: &acp::SessionId,
@@ -5580,182 +5606,6 @@ pub(crate) mod tests {
});
}
#[gpui::test]
async fn test_granularity_selection_updates_state(cx: &mut TestAppContext) {
init_test(cx);
let tool_call_id = acp::ToolCallId::new("granularity-test-1");
let tool_call =
acp::ToolCall::new(tool_call_id.clone(), "Run `cargo build`").kind(acp::ToolKind::Edit);
let permission_options =
ToolPermissionContext::new(TerminalTool::NAME, vec!["cargo build".to_string()])
.build_permission_options();
let connection =
StubAgentConnection::new().with_permission_requests(HashMap::from_iter([(
tool_call_id.clone(),
permission_options.clone(),
)]));
connection.set_next_prompt_updates(vec![acp::SessionUpdate::ToolCall(tool_call)]);
let (thread_view, cx) = setup_thread_view(StubAgentServer::new(connection), cx).await;
add_to_workspace(thread_view.clone(), cx);
cx.update(|_window, cx| {
AgentSettings::override_global(
AgentSettings {
notify_when_agent_waiting: NotifyWhenAgentWaiting::Never,
..AgentSettings::get_global(cx).clone()
},
cx,
);
});
let message_editor = message_editor(&thread_view, cx);
message_editor.update_in(cx, |editor, window, cx| {
editor.set_text("Build the project", window, cx);
});
active_thread(&thread_view, cx).update_in(cx, |view, window, cx| view.send(window, cx));
cx.run_until_parked();
// Verify default granularity is the last option (index 2 = "Only this time")
thread_view.read_with(cx, |thread_view, cx| {
let state = thread_view.active_thread().unwrap();
let selected = state
.read(cx)
.selected_permission_granularity
.get(&tool_call_id);
assert!(
selected.is_none(),
"Should have no selection initially (defaults to last)"
);
});
// Select the first option (index 0 = "Always for terminal")
thread_view.update_in(cx, |_, window, cx| {
window.dispatch_action(
crate::SelectPermissionGranularity {
tool_call_id: "granularity-test-1".to_string(),
index: 0,
}
.boxed_clone(),
cx,
);
});
cx.run_until_parked();
// Verify the selection was updated
thread_view.read_with(cx, |thread_view, cx| {
let state = thread_view.active_thread().unwrap();
let selected = state
.read(cx)
.selected_permission_granularity
.get(&tool_call_id);
assert_eq!(selected, Some(&0), "Should have selected index 0");
});
}
#[gpui::test]
async fn test_allow_button_uses_selected_granularity(cx: &mut TestAppContext) {
init_test(cx);
let tool_call_id = acp::ToolCallId::new("allow-granularity-test-1");
let tool_call =
acp::ToolCall::new(tool_call_id.clone(), "Run `npm install`").kind(acp::ToolKind::Edit);
let permission_options =
ToolPermissionContext::new(TerminalTool::NAME, vec!["npm install".to_string()])
.build_permission_options();
// Verify we have the expected options
let PermissionOptions::Dropdown(choices) = &permission_options else {
panic!("Expected dropdown permission options");
};
assert_eq!(choices.len(), 3);
assert!(
choices[0]
.allow
.option_id
.0
.contains("always_allow:terminal")
);
assert!(
choices[1]
.allow
.option_id
.0
.contains("always_allow_pattern:terminal")
);
assert_eq!(choices[2].allow.option_id.0.as_ref(), "allow");
let connection =
StubAgentConnection::new().with_permission_requests(HashMap::from_iter([(
tool_call_id.clone(),
permission_options.clone(),
)]));
connection.set_next_prompt_updates(vec![acp::SessionUpdate::ToolCall(tool_call)]);
let (thread_view, cx) = setup_thread_view(StubAgentServer::new(connection), cx).await;
add_to_workspace(thread_view.clone(), cx);
cx.update(|_window, cx| {
AgentSettings::override_global(
AgentSettings {
notify_when_agent_waiting: NotifyWhenAgentWaiting::Never,
..AgentSettings::get_global(cx).clone()
},
cx,
);
});
let message_editor = message_editor(&thread_view, cx);
message_editor.update_in(cx, |editor, window, cx| {
editor.set_text("Install dependencies", window, cx);
});
active_thread(&thread_view, cx).update_in(cx, |view, window, cx| view.send(window, cx));
cx.run_until_parked();
// Select the pattern option (index 1 = "Always for `npm` commands")
thread_view.update_in(cx, |_, window, cx| {
window.dispatch_action(
crate::SelectPermissionGranularity {
tool_call_id: "allow-granularity-test-1".to_string(),
index: 1,
}
.boxed_clone(),
cx,
);
});
cx.run_until_parked();
// Simulate clicking the Allow button by dispatching AllowOnce action
// which should use the selected granularity
active_thread(&thread_view, cx).update_in(cx, |view, window, cx| {
view.allow_once(&AllowOnce, window, cx)
});
cx.run_until_parked();
// Verify tool call was authorized
thread_view.read_with(cx, |thread_view, cx| {
let tool_call = thread_view.pending_tool_call(cx);
assert!(
tool_call.is_none(),
"Tool call should be authorized after Allow with pattern granularity"
);
});
}
#[gpui::test]
async fn test_deny_button_uses_selected_granularity(cx: &mut TestAppContext) {
init_test(cx);
@@ -235,10 +235,6 @@ pub struct ThreadView {
pub is_loading_contents: bool,
pub new_server_version_available: Option<SharedString>,
pub resumed_without_history: bool,
/// Tracks the selected granularity index for each tool call's permission dropdown.
/// The index corresponds to the position in the allow_options list.
/// Default is the last option (index pointing to "Only this time").
pub selected_permission_granularity: HashMap<agent_client_protocol::ToolCallId, usize>,
pub resume_thread_metadata: Option<AgentSessionInfo>,
pub _cancel_task: Option<Task<()>>,
pub skip_queue_processing_count: usize,
@@ -428,7 +424,6 @@ impl ThreadView {
discarded_partial_edits: HashSet::default(),
is_loading_contents: false,
new_server_version_available: None,
selected_permission_granularity: HashMap::default(),
_cancel_task: None,
skip_queue_processing_count: 0,
user_interrupted_generation: false,
@@ -1385,19 +1380,6 @@ impl ThreadView {
);
}
pub fn handle_select_permission_granularity(
&mut self,
action: &SelectPermissionGranularity,
_window: &mut Window,
cx: &mut Context<Self>,
) {
let tool_call_id = acp::ToolCallId::new(action.tool_call_id.clone());
self.selected_permission_granularity
.insert(tool_call_id, action.index);
cx.notify();
}
fn authorize_pending_with_granularity(
&mut self,
is_allow: bool,
@@ -1417,9 +1399,9 @@ impl ThreadView {
// Get selected index, defaulting to last option ("Only this time")
let selected_index = self
.selected_permission_granularity
.get(&tool_call_id)
.copied()
.conversation
.read(cx)
.selected_permission_granularity(&session_id, &tool_call_id)
.unwrap_or_else(|| choices.len().saturating_sub(1));
let selected_choice = choices.get(selected_index).or(choices.last())?;
@@ -1817,23 +1799,26 @@ impl ThreadView {
.when(!plan.is_empty() && !changed_buffers.is_empty(), |this| {
this.child(Divider::horizontal().color(DividerColor::Border))
})
.when(!changed_buffers.is_empty(), |this| {
this.child(self.render_edits_summary(
&changed_buffers,
edits_expanded,
pending_edits,
cx,
))
.when(edits_expanded, |parent| {
parent.child(self.render_edited_files(
action_log,
telemetry.clone(),
.when(
!changed_buffers.is_empty() && thread.parent_session_id().is_none(),
|this| {
this.child(self.render_edits_summary(
&changed_buffers,
edits_expanded,
pending_edits,
cx,
))
})
})
.when(edits_expanded, |parent| {
parent.child(self.render_edited_files(
action_log,
telemetry.clone(),
&changed_buffers,
pending_edits,
cx,
))
})
},
)
.when(!queue_is_empty, |this| {
this.when(!plan.is_empty() || !changed_buffers.is_empty(), |this| {
this.child(Divider::horizontal().color(DividerColor::Border))
@@ -5518,9 +5503,9 @@ impl ThreadView {
) -> Div {
// Get the selected granularity index, defaulting to the last option ("Only this time")
let selected_index = self
.selected_permission_granularity
.get(&tool_call_id)
.copied()
.conversation
.read(cx)
.selected_permission_granularity(&session_id, &tool_call_id)
.unwrap_or_else(|| choices.len().saturating_sub(1));
let selected_choice = choices.get(selected_index).or(choices.last());
@@ -5608,6 +5593,7 @@ impl ThreadView {
)
})
.on_click(cx.listener({
let session_id = session_id.clone();
let tool_call_id = tool_call_id.clone();
let option_id = deny_option_id;
let option_kind = deny_option_kind;
@@ -5628,6 +5614,7 @@ impl ThreadView {
choices,
dropdown_label,
entry_ix,
session_id,
tool_call_id,
selected_index,
is_first,
@@ -5640,6 +5627,7 @@ impl ThreadView {
choices: &[PermissionOptionChoice],
current_label: SharedString,
entry_ix: usize,
session_id: acp::SessionId,
tool_call_id: acp::ToolCallId,
selected_index: usize,
is_first: bool,
@@ -5653,6 +5641,8 @@ impl ThreadView {
let permission_dropdown_handle = self.permission_dropdown_handle.clone();
let conversation = self.conversation.clone();
PopoverMenu::new(("permission-granularity", entry_ix))
.with_handle(permission_dropdown_handle)
.trigger(
@@ -5673,6 +5663,8 @@ impl ThreadView {
}),
)
.menu(move |window, cx| {
let session_id = session_id.clone();
let conversation = conversation.clone();
let tool_call_id = tool_call_id.clone();
let options = menu_options.clone();
@@ -5680,23 +5672,23 @@ impl ThreadView {
for (index, display_name) in options.iter() {
let display_name = display_name.clone();
let index = *index;
let tool_call_id_for_entry = tool_call_id.clone();
let session_id = session_id.clone();
let conversation = conversation.clone();
let tool_call_id = tool_call_id.clone();
let is_selected = index == selected_index;
menu = menu.toggleable_entry(
display_name,
is_selected,
IconPosition::End,
None,
move |window, cx| {
window.dispatch_action(
SelectPermissionGranularity {
tool_call_id: tool_call_id_for_entry.0.to_string(),
move |_window, cx| {
conversation.update(cx, |conversation, _cx| {
conversation.set_selected_permission_granularity(
session_id.clone(),
tool_call_id.clone(),
index,
}
.boxed_clone(),
cx,
);
);
});
},
);
}
@@ -7520,7 +7512,6 @@ impl Render for ThreadView {
.on_action(cx.listener(Self::allow_once))
.on_action(cx.listener(Self::reject_once))
.on_action(cx.listener(Self::handle_authorize_tool_call))
.on_action(cx.listener(Self::handle_select_permission_granularity))
.on_action(cx.listener(Self::open_permission_dropdown))
.on_action(cx.listener(Self::open_add_context_menu))
.on_action(cx.listener(|this, _: &ToggleFastMode, _window, cx| {