From 47a784fb3abf271023ea7126c78f59337b3f93d3 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 4 Jun 2026 14:14:50 +0200 Subject: [PATCH 1/6] acp_thread: reserve waiting tool call status on updates Allows contents to update without dropping the permission (caused by claude subagents) Closes https://github.com/agentclientprotocol/claude-agent-acp/issues/708 --- crates/acp_thread/src/acp_thread.rs | 148 +++++++++++++++++++++++++++- 1 file changed, 145 insertions(+), 3 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index e3fcf7ac4fa625..36adde0f194a42 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -734,6 +734,19 @@ impl From for ToolCallStatus { } } +fn should_preserve_waiting_status( + current_status: &ToolCallStatus, + incoming_status: &ToolCallStatus, +) -> bool { + matches!( + (current_status, incoming_status), + ( + ToolCallStatus::WaitingForConfirmation { .. }, + ToolCallStatus::Pending | ToolCallStatus::InProgress + ) + ) +} + impl Display for ToolCallStatus { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!( @@ -2124,8 +2137,18 @@ impl AcpThread { }; match update { - ToolCallUpdate::UpdateFields(update) => { + ToolCallUpdate::UpdateFields(mut update) => { let location_updated = update.fields.locations.is_some(); + let preserve_waiting_status = update + .fields + .status + .as_ref() + .map(|status| ToolCallStatus::from(*status)) + .is_some_and(|status| should_preserve_waiting_status(&call.status, &status)); + if preserve_waiting_status { + update.fields.status = None; + } + call.update_fields( update.fields, update.meta, @@ -2198,15 +2221,23 @@ impl AcpThread { unreachable!() }; + let preserve_waiting_status = should_preserve_waiting_status(&call.status, &status); + let mut fields = update.fields; + if preserve_waiting_status { + fields.status = None; + } + call.update_fields( - update.fields, + fields, update.meta, language_registry, path_style, &self.terminals, cx, )?; - call.status = status; + if !preserve_waiting_status { + call.status = status; + } cx.emit(AcpThreadEvent::EntryUpdated(ix)); } else { @@ -4333,6 +4364,117 @@ mod tests { }); } + #[gpui::test] + async fn test_duplicate_tool_call_update_preserves_open_permission_request( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .unwrap(); + + let tool_call_id = acp::ToolCallId::new("toolu_01duplicate"); + let allow_option_id = acp::PermissionOptionId::new("allow"); + let permission_task = thread + .update(cx, |thread, cx| { + thread.request_tool_call_authorization( + acp::ToolCall::new(tool_call_id.clone(), "Original title") + .kind(acp::ToolKind::Execute) + .status(acp::ToolCallStatus::Pending) + .content(vec!["original content".into()]) + .into(), + PermissionOptions::Flat(vec![acp::PermissionOption::new( + allow_option_id.clone(), + "Allow", + acp::PermissionOptionKind::AllowOnce, + )]), + AuthorizationKind::PermissionGrant, + cx, + ) + }) + .unwrap(); + + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCall( + acp::ToolCall::new(tool_call_id.clone(), "Updated title") + .kind(acp::ToolKind::Execute) + .status(acp::ToolCallStatus::Pending) + .content(vec!["updated content".into()]), + ), + cx, + ) + }) + .unwrap(); + + thread.read_with(cx, |thread, cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert_eq!(tool_call.label.read(cx).source(), "Updated title"); + assert!(matches!( + tool_call.status, + ToolCallStatus::WaitingForConfirmation { .. } + )); + assert_eq!(tool_call.content.len(), 1); + assert_eq!(tool_call.content[0].to_markdown(cx), "updated content"); + }); + + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( + tool_call_id.clone(), + acp::ToolCallUpdateFields::new() + .status(acp::ToolCallStatus::InProgress) + .title("Updated again") + .content(vec!["updated again".into()]), + )), + cx, + ) + }) + .unwrap(); + + thread.read_with(cx, |thread, cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert_eq!(tool_call.label.read(cx).source(), "Updated again"); + assert!(matches!( + tool_call.status, + ToolCallStatus::WaitingForConfirmation { .. } + )); + assert_eq!(tool_call.content.len(), 1); + assert_eq!(tool_call.content[0].to_markdown(cx), "updated again"); + }); + + let selected_outcome = SelectedPermissionOutcome::new( + allow_option_id.clone(), + acp::PermissionOptionKind::AllowOnce, + ); + thread.update(cx, |thread, cx| { + thread.authorize_tool_call(tool_call_id.clone(), selected_outcome, cx); + }); + + match permission_task.await { + RequestPermissionOutcome::Selected(outcome) => { + assert_eq!(outcome.option_id, allow_option_id); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + RequestPermissionOutcome::Cancelled => { + panic!("permission request should remain open after duplicate tool call update") + } + } + } + #[gpui::test] async fn test_no_pending_edits_if_tool_calls_are_completed(cx: &mut TestAppContext) { init_test(cx); From 4791ebc9a78ea08b1e070f009f509aa4c32ceb73 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 9 Jun 2026 11:30:38 +0200 Subject: [PATCH 2/6] Extend duplicate tool call authorization test --- crates/acp_thread/src/acp_thread.rs | 34 ++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 36adde0f194a42..f281f7a17be3b2 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -4365,7 +4365,7 @@ mod tests { } #[gpui::test] - async fn test_duplicate_tool_call_update_preserves_open_permission_request( + async fn test_duplicate_tool_call_update_preserves_open_permission_request_until_authorized( cx: &mut TestAppContext, ) { init_test(cx); @@ -4464,6 +4464,13 @@ mod tests { thread.authorize_tool_call(tool_call_id.clone(), selected_outcome, cx); }); + thread.read_with(cx, |thread, _cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert!(matches!(tool_call.status, ToolCallStatus::InProgress)); + }); + match permission_task.await { RequestPermissionOutcome::Selected(outcome) => { assert_eq!(outcome.option_id, allow_option_id); @@ -4473,6 +4480,31 @@ mod tests { panic!("permission request should remain open after duplicate tool call update") } } + + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( + tool_call_id.clone(), + acp::ToolCallUpdateFields::new() + .status(acp::ToolCallStatus::Completed) + .title("Completed") + .content(vec!["done".into()]), + )), + cx, + ) + }) + .unwrap(); + + thread.read_with(cx, |thread, cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert_eq!(tool_call.label.read(cx).source(), "Completed"); + assert!(matches!(tool_call.status, ToolCallStatus::Completed)); + assert_eq!(tool_call.content.len(), 1); + assert_eq!(tool_call.content[0].to_markdown(cx), "done"); + }); } #[gpui::test] From 3ba9d60e9a3a92b811da9c61a77e90f501f8112a Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 9 Jun 2026 12:33:29 +0200 Subject: [PATCH 3/6] Preserve tool authorization outcomes on auto-resolve --- crates/acp_thread/src/acp_thread.rs | 200 +++++++++++++++++----- crates/agent/src/agent.rs | 8 + crates/agent/src/thread.rs | 142 +++++++++++++-- crates/agent/src/tools/edit_file_tool.rs | 8 +- crates/agent/src/tools/edit_session.rs | 14 +- crates/agent/src/tools/write_file_tool.rs | 7 +- 6 files changed, 308 insertions(+), 71 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index f281f7a17be3b2..a54c159562aff3 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -449,7 +449,7 @@ impl ToolCall { } if let Some(status) = status { - self.status = status.into(); + self.update_acp_status(status); } if let Some(subagent_session_info) = subagent_session_info_from_meta(&meta) { @@ -532,6 +532,26 @@ impl ToolCall { Ok(()) } + fn update_status(&mut self, status: ToolCallStatus) { + match status.as_acp_status() { + Some(status) => self.update_acp_status(status), + None => self.status = status, + } + } + + fn update_acp_status(&mut self, status: acp::ToolCallStatus) { + if let ToolCallStatus::WaitingForConfirmation { current_status, .. } = &mut self.status + && matches!( + status, + acp::ToolCallStatus::Pending | acp::ToolCallStatus::InProgress + ) + { + *current_status = status; + } else { + self.status = status.into(); + } + } + pub fn diffs(&self) -> impl Iterator> { self.content.iter().filter_map(|content| match content { ToolCallContent::Diff(diff) => Some(diff), @@ -640,7 +660,7 @@ pub enum SelectedPermissionParams { Terminal { patterns: Vec }, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct SelectedPermissionOutcome { pub option_id: acp::PermissionOptionId, pub option_kind: acp::PermissionOptionKind, @@ -706,6 +726,7 @@ pub enum ToolCallStatus { Pending, /// The tool call is waiting for confirmation from the user. WaitingForConfirmation { + current_status: acp::ToolCallStatus, options: PermissionOptions, respond_tx: oneshot::Sender, kind: AuthorizationKind, @@ -734,17 +755,24 @@ impl From for ToolCallStatus { } } -fn should_preserve_waiting_status( - current_status: &ToolCallStatus, - incoming_status: &ToolCallStatus, -) -> bool { - matches!( - (current_status, incoming_status), - ( - ToolCallStatus::WaitingForConfirmation { .. }, - ToolCallStatus::Pending | ToolCallStatus::InProgress - ) - ) +impl ToolCallStatus { + fn as_acp_status(&self) -> Option { + match self { + ToolCallStatus::Pending => Some(acp::ToolCallStatus::Pending), + ToolCallStatus::WaitingForConfirmation { current_status, .. } => Some(*current_status), + ToolCallStatus::InProgress => Some(acp::ToolCallStatus::InProgress), + ToolCallStatus::Completed => Some(acp::ToolCallStatus::Completed), + ToolCallStatus::Failed => Some(acp::ToolCallStatus::Failed), + ToolCallStatus::Rejected | ToolCallStatus::Canceled => None, + } + } + + fn status_after_permission_grant(status: acp::ToolCallStatus) -> ToolCallStatus { + match ToolCallStatus::from(status) { + ToolCallStatus::Pending => ToolCallStatus::InProgress, + status => status, + } + } } impl Display for ToolCallStatus { @@ -2137,17 +2165,8 @@ impl AcpThread { }; match update { - ToolCallUpdate::UpdateFields(mut update) => { + ToolCallUpdate::UpdateFields(update) => { let location_updated = update.fields.locations.is_some(); - let preserve_waiting_status = update - .fields - .status - .as_ref() - .map(|status| ToolCallStatus::from(*status)) - .is_some_and(|status| should_preserve_waiting_status(&call.status, &status)); - if preserve_waiting_status { - update.fields.status = None; - } call.update_fields( update.fields, @@ -2221,23 +2240,15 @@ impl AcpThread { unreachable!() }; - let preserve_waiting_status = should_preserve_waiting_status(&call.status, &status); - let mut fields = update.fields; - if preserve_waiting_status { - fields.status = None; - } - call.update_fields( - fields, + update.fields, update.meta, language_registry, path_style, &self.terminals, cx, )?; - if !preserve_waiting_status { - call.status = status; - } + call.update_status(status); cx.emit(AcpThreadEvent::EntryUpdated(ix)); } else { @@ -2388,7 +2399,13 @@ impl AcpThread { ) -> Result> { let (tx, rx) = oneshot::channel(); + let current_status = self + .tool_call(&tool_call.tool_call_id) + .and_then(|(_, tool_call)| tool_call.status.as_acp_status()) + .or(tool_call.fields.status) + .unwrap_or(acp::ToolCallStatus::Pending); let status = ToolCallStatus::WaitingForConfirmation { + current_status, options, respond_tx: tx, kind, @@ -2423,24 +2440,30 @@ impl AcpThread { return; }; - let is_action_choice = matches!( - call.status, - ToolCallStatus::WaitingForConfirmation { - kind: AuthorizationKind::ActionChoice, - .. - } - ); let new_status = - if is_action_choice { - ToolCallStatus::InProgress - } else { - match outcome.option_kind { + match &call.status { + ToolCallStatus::WaitingForConfirmation { + kind: AuthorizationKind::ActionChoice, + .. + } => ToolCallStatus::InProgress, + ToolCallStatus::WaitingForConfirmation { current_status, .. } => { + match outcome.option_kind { + acp::PermissionOptionKind::RejectOnce + | acp::PermissionOptionKind::RejectAlways => ToolCallStatus::Rejected, + acp::PermissionOptionKind::AllowOnce + | acp::PermissionOptionKind::AllowAlways => { + ToolCallStatus::status_after_permission_grant(*current_status) + } + _ => ToolCallStatus::status_after_permission_grant(*current_status), + } + } + _ => match outcome.option_kind { acp::PermissionOptionKind::RejectOnce | acp::PermissionOptionKind::RejectAlways => ToolCallStatus::Rejected, acp::PermissionOptionKind::AllowOnce | acp::PermissionOptionKind::AllowAlways => ToolCallStatus::InProgress, _ => ToolCallStatus::InProgress, - } + }, }; let curr_status = mem::replace(&mut call.status, new_status); @@ -4507,6 +4530,93 @@ mod tests { }); } + #[gpui::test] + async fn test_permission_request_tracks_agent_status_until_resolved(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .unwrap(); + + let tool_call_id = acp::ToolCallId::new("toolu_01auto_resolve"); + let permission_task = thread + .update(cx, |thread, cx| { + thread.request_tool_call_authorization( + acp::ToolCall::new(tool_call_id.clone(), "Original title") + .kind(acp::ToolKind::Execute) + .status(acp::ToolCallStatus::Pending) + .into(), + PermissionOptions::Flat(vec![acp::PermissionOption::new( + acp::PermissionOptionId::new("allow"), + "Allow", + acp::PermissionOptionKind::AllowOnce, + )]), + AuthorizationKind::PermissionGrant, + cx, + ) + }) + .unwrap(); + + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( + tool_call_id.clone(), + acp::ToolCallUpdateFields::new().status(acp::ToolCallStatus::InProgress), + )), + cx, + ) + }) + .unwrap(); + + thread.read_with(cx, |thread, _cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert!(matches!( + tool_call.status, + ToolCallStatus::WaitingForConfirmation { + current_status: acp::ToolCallStatus::InProgress, + .. + } + )); + }); + + thread.update(cx, |thread, cx| { + thread.authorize_tool_call( + tool_call_id.clone(), + SelectedPermissionOutcome::new( + acp::PermissionOptionId::new("allow"), + acp::PermissionOptionKind::AllowOnce, + ), + cx, + ); + }); + + thread.read_with(cx, |thread, _cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert!(matches!(tool_call.status, ToolCallStatus::InProgress)); + }); + + match permission_task.await { + RequestPermissionOutcome::Selected(outcome) => { + assert_eq!(outcome.option_id, acp::PermissionOptionId::new("allow")); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + RequestPermissionOutcome::Cancelled => { + panic!("resolved permission request should select an outcome") + } + } + } + #[gpui::test] async fn test_no_pending_edits_if_tool_calls_are_completed(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index 6b4b81e7426f77..4e45c44d0e6cb4 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -2081,6 +2081,14 @@ impl NativeAgentConnection { }) .detach(); } + ThreadEvent::ToolCallAuthorizationResolved { + tool_call_id, + outcome, + } => { + acp_thread.update(cx, |thread, cx| { + thread.authorize_tool_call(tool_call_id, outcome, cx); + })?; + } ThreadEvent::ToolCall(tool_call) => { acp_thread.update(cx, |thread, cx| { thread.upsert_tool_call(tool_call, cx) diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index b84065479820b8..f71a71925c208b 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -834,6 +834,10 @@ pub enum ThreadEvent { ToolCall(acp::ToolCall), ToolCallUpdate(acp_thread::ToolCallUpdate), ToolCallAuthorization(ToolCallAuthorization), + ToolCallAuthorizationResolved { + tool_call_id: acp::ToolCallId, + outcome: acp_thread::SelectedPermissionOutcome, + }, SubagentSpawned(acp::SessionId), Retry(acp_thread::RetryStatus), ContextCompaction(acp_thread::ContextCompaction), @@ -1110,6 +1114,25 @@ pub struct ToolCallAuthorization { pub kind: acp_thread::AuthorizationKind, } +fn auto_resolve_permission_outcome( + options: &acp_thread::PermissionOptions, + is_allow: bool, +) -> Result { + let kind = if is_allow { + acp::PermissionOptionKind::AllowOnce + } else { + acp::PermissionOptionKind::RejectOnce + }; + let option = options + .first_option_of_kind(kind) + .ok_or_else(|| anyhow!("permission prompt has no auto-resolution option"))?; + + Ok(acp_thread::SelectedPermissionOutcome::new( + option.option_id.clone(), + option.kind, + )) +} + #[derive(Debug, thiserror::Error)] enum CompletionError { #[error("max tokens")] @@ -4480,6 +4503,19 @@ impl ThreadEventStream { .ok(); } + fn resolve_tool_call_authorization( + &self, + tool_use_id: &LanguageModelToolUseId, + outcome: acp_thread::SelectedPermissionOutcome, + ) { + self.0 + .unbounded_send(Ok(ThreadEvent::ToolCallAuthorizationResolved { + tool_call_id: acp::ToolCallId::new(tool_use_id.to_string()), + outcome, + })) + .ok(); + } + fn send_retry(&self, status: acp_thread::RetryStatus) { self.0.unbounded_send(Ok(ThreadEvent::Retry(status))).ok(); } @@ -4623,6 +4659,11 @@ impl ToolCallEventStream { .update_tool_call_fields(&self.tool_use_id, fields, meta); } + pub fn resolve_authorization(&self, outcome: acp_thread::SelectedPermissionOutcome) { + self.stream + .resolve_tool_call_authorization(&self.tool_use_id, outcome); + } + pub fn update_diff(&self, diff: Entity) { self.stream .0 @@ -4808,6 +4849,10 @@ impl ToolCallEventStream { let stream = self.stream.clone(); let tool_use_id = self.tool_use_id.clone(); let sandbox_grants = self.sandbox_grants.clone(); + let auto_allow_outcome = match auto_resolve_permission_outcome(&options, true) { + Ok(outcome) => outcome, + Err(error) => return Task::ready(Err(error)), + }; cx.spawn(async move |cx| { let (response_tx, mut response_rx) = oneshot::channel(); if let Err(error) = stream @@ -4864,11 +4909,9 @@ impl ToolCallEventStream { cx, )) { drop(response_rx); - stream.update_tool_call_fields( + stream.resolve_tool_call_authorization( &tool_use_id, - acp::ToolCallUpdateFields::new() - .status(acp::ToolCallStatus::InProgress), - None, + auto_allow_outcome.clone(), ); return Ok(()); } @@ -5057,6 +5100,17 @@ impl ToolCallEventStream { let fs = self.fs.clone(); let stream = self.stream.clone(); let tool_use_id = self.tool_use_id.clone(); + let auto_resolution_outcomes = if check_settings.is_some() { + match ( + auto_resolve_permission_outcome(&options, true), + auto_resolve_permission_outcome(&options, false), + ) { + (Ok(allow), Ok(deny)) => Some((allow, deny)), + (Err(error), _) | (_, Err(error)) => return Task::ready(Err(error)), + } + } else { + None + }; cx.spawn(async move |cx| { let (response_tx, mut response_rx) = oneshot::channel(); if let Err(error) = stream @@ -5085,6 +5139,9 @@ impl ToolCallEventStream { return Self::persist_permission_outcome(&outcome, fs, cx); }; + let Some((auto_allow_outcome, auto_deny_outcome)) = auto_resolution_outcomes else { + return Err(anyhow!("missing auto-resolution outcomes")); + }; let (mut settings_tx, mut settings_rx) = watch::channel(()); let _settings_subscription = cx.update(|cx| { @@ -5112,28 +5169,24 @@ impl ToolCallEventStream { } _ = settings_changed.fuse() => { // On auto-resolve, we dismiss the prompt UI by - // replacing the tool call's `WaitingForConfirmation` - // status with `InProgress` (or `Failed`). Dropping - // `response_rx` closes the `oneshot` held by the - // UI, so any late click by the user is a no-op. + // resolving the tool call's `WaitingForConfirmation` + // status with an internal selected outcome. Dropping + // `response_rx` prevents the synthetic response from + // being delivered back into this loop. match cx.update(|cx| check_settings(cx)) { ToolPermissionDecision::Allow => { drop(response_rx); - stream.update_tool_call_fields( + stream.resolve_tool_call_authorization( &tool_use_id, - acp::ToolCallUpdateFields::new() - .status(acp::ToolCallStatus::InProgress), - None, + auto_allow_outcome.clone(), ); return Ok(()); } ToolPermissionDecision::Deny(reason) => { drop(response_rx); - stream.update_tool_call_fields( + stream.resolve_tool_call_authorization( &tool_use_id, - acp::ToolCallUpdateFields::new() - .status(acp::ToolCallStatus::Failed), - None, + auto_deny_outcome.clone(), ); return Err(anyhow!(reason)); } @@ -5282,6 +5335,21 @@ impl ToolCallEventStreamReceiver { } } + pub async fn expect_authorization_resolved( + &mut self, + ) -> (acp::ToolCallId, acp_thread::SelectedPermissionOutcome) { + let event = self.0.next().await; + if let Some(Ok(ThreadEvent::ToolCallAuthorizationResolved { + tool_call_id, + outcome, + })) = event + { + (tool_call_id, outcome) + } else { + panic!("Expected authorization resolved but got: {:?}", event); + } + } + pub async fn expect_diff(&mut self) -> Entity { let event = self.0.next().await; if let Some(Ok(ThreadEvent::ToolCallUpdate(acp_thread::ToolCallUpdate::UpdateDiff( @@ -6076,6 +6144,48 @@ mod tests { ); } + #[test] + fn test_auto_resolve_permission_outcome_uses_once_only_options() { + let options = acp_thread::PermissionOptions::Dropdown(vec![ + acp_thread::PermissionOptionChoice { + allow: acp::PermissionOption::new( + acp::PermissionOptionId::new("always_allow:test_tool"), + "Always allow", + acp::PermissionOptionKind::AllowAlways, + ), + deny: acp::PermissionOption::new( + acp::PermissionOptionId::new("always_deny:test_tool"), + "Always deny", + acp::PermissionOptionKind::RejectAlways, + ), + sub_patterns: vec![], + }, + acp_thread::PermissionOptionChoice { + allow: acp::PermissionOption::new( + acp::PermissionOptionId::new("allow"), + "Allow once", + acp::PermissionOptionKind::AllowOnce, + ), + deny: acp::PermissionOption::new( + acp::PermissionOptionId::new("deny"), + "Deny once", + acp::PermissionOptionKind::RejectOnce, + ), + sub_patterns: vec![], + }, + ]); + + let allow = auto_resolve_permission_outcome(&options, true) + .expect("allow auto-resolve should use once-only option"); + assert_eq!(allow.option_id, acp::PermissionOptionId::new("allow")); + assert_eq!(allow.option_kind, acp::PermissionOptionKind::AllowOnce); + + let deny = auto_resolve_permission_outcome(&options, false) + .expect("deny auto-resolve should use once-only option"); + assert_eq!(deny.option_id, acp::PermissionOptionId::new("deny")); + assert_eq!(deny.option_kind, acp::PermissionOptionKind::RejectOnce); + } + #[gpui::test] async fn test_replay_tool_call_replays_image_content(cx: &mut TestAppContext) { let (thread, _event_stream) = setup_thread_for_test(cx).await; diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index e80e2110a17f25..fc513d904a3445 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -2456,10 +2456,10 @@ mod tests { .unwrap(); // The prompt's response channel should drop without a click; the - // tool dismisses the prompt by transitioning the tool call status - // to `InProgress`. - let dismiss = stream_rx.expect_update_fields().await; - assert_eq!(dismiss.status, Some(acp::ToolCallStatus::InProgress)); + // tool dismisses the prompt by resolving the pending authorization. + let (_, outcome) = stream_rx.expect_authorization_resolved().await; + assert_eq!(outcome.option_id, acp::PermissionOptionId::new("save")); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); drop(auth); let EditFileToolOutput::Success { new_text, .. } = task.await.unwrap() else { diff --git a/crates/agent/src/tools/edit_session.rs b/crates/agent/src/tools/edit_session.rs index 016058318bfcac..f817a99a500732 100644 --- a/crates/agent/src/tools/edit_session.rs +++ b/crates/agent/src/tools/edit_session.rs @@ -1058,9 +1058,17 @@ async fn resolve_dirty_buffer( }; let Some(decision) = decision else { - event_stream.update_fields( - acp::ToolCallUpdateFields::new().status(acp::ToolCallStatus::InProgress), - ); + let outcome = match mode { + EditSessionMode::Edit => acp_thread::SelectedPermissionOutcome::new( + acp::PermissionOptionId::new("save"), + acp::PermissionOptionKind::AllowOnce, + ), + EditSessionMode::Write => acp_thread::SelectedPermissionOutcome::new( + acp::PermissionOptionId::new("keep"), + acp::PermissionOptionKind::RejectOnce, + ), + }; + event_stream.resolve_authorization(outcome); return match mode { EditSessionMode::Edit => Ok(()), EditSessionMode::Write => Err( diff --git a/crates/agent/src/tools/write_file_tool.rs b/crates/agent/src/tools/write_file_tool.rs index af9c857f63011f..0f8d96db0e4004 100644 --- a/crates/agent/src/tools/write_file_tool.rs +++ b/crates/agent/src/tools/write_file_tool.rs @@ -1345,9 +1345,10 @@ mod tests { .await .unwrap(); - // The prompt is dismissed by transitioning to InProgress. - let dismiss = stream_rx.expect_update_fields().await; - assert_eq!(dismiss.status, Some(acp::ToolCallStatus::InProgress)); + // The prompt is dismissed by resolving the pending authorization. + let (_, outcome) = stream_rx.expect_authorization_resolved().await; + assert_eq!(outcome.option_id, acp::PermissionOptionId::new("keep")); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::RejectOnce); drop(auth); // The overwrite is cancelled with an error. From 90c603b97e87a26274632851314b2816db369dd0 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 9 Jun 2026 12:47:03 +0200 Subject: [PATCH 4/6] Preserve waiting status on tool call updates --- crates/acp_thread/src/acp_thread.rs | 95 ++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 3 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index a54c159562aff3..676bd98937ba5f 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -533,9 +533,14 @@ impl ToolCall { } fn update_status(&mut self, status: ToolCallStatus) { - match status.as_acp_status() { - Some(status) => self.update_acp_status(status), - None => self.status = status, + match status { + ToolCallStatus::Pending => self.update_acp_status(acp::ToolCallStatus::Pending), + ToolCallStatus::InProgress => self.update_acp_status(acp::ToolCallStatus::InProgress), + ToolCallStatus::Completed => self.update_acp_status(acp::ToolCallStatus::Completed), + ToolCallStatus::Failed => self.update_acp_status(acp::ToolCallStatus::Failed), + status @ (ToolCallStatus::WaitingForConfirmation { .. } + | ToolCallStatus::Rejected + | ToolCallStatus::Canceled) => self.status = status, } } @@ -4617,6 +4622,90 @@ mod tests { } } + #[gpui::test] + async fn test_permission_request_sets_waiting_status_on_existing_tool_call( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .unwrap(); + + let tool_call_id = acp::ToolCallId::new("toolu_01existing_permission"); + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCall( + acp::ToolCall::new(tool_call_id.clone(), "Running title") + .kind(acp::ToolKind::Execute) + .status(acp::ToolCallStatus::InProgress), + ), + cx, + ) + }) + .unwrap(); + + let permission_task = thread + .update(cx, |thread, cx| { + thread.request_tool_call_authorization( + acp::ToolCall::new(tool_call_id.clone(), "Needs permission") + .kind(acp::ToolKind::Execute) + .status(acp::ToolCallStatus::Pending) + .into(), + PermissionOptions::Flat(vec![acp::PermissionOption::new( + acp::PermissionOptionId::new("allow"), + "Allow", + acp::PermissionOptionKind::AllowOnce, + )]), + AuthorizationKind::PermissionGrant, + cx, + ) + }) + .unwrap(); + + thread.read_with(cx, |thread, cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert_eq!(tool_call.label.read(cx).source(), "Needs permission"); + assert!(matches!( + tool_call.status, + ToolCallStatus::WaitingForConfirmation { + current_status: acp::ToolCallStatus::InProgress, + .. + } + )); + }); + + thread.update(cx, |thread, cx| { + thread.authorize_tool_call( + tool_call_id.clone(), + SelectedPermissionOutcome::new( + acp::PermissionOptionId::new("allow"), + acp::PermissionOptionKind::AllowOnce, + ), + cx, + ); + }); + + match permission_task.await { + RequestPermissionOutcome::Selected(outcome) => { + assert_eq!(outcome.option_id, acp::PermissionOptionId::new("allow")); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + RequestPermissionOutcome::Cancelled => { + panic!("permission request should resolve after authorization") + } + } + } + #[gpui::test] async fn test_no_pending_edits_if_tool_calls_are_completed(cx: &mut TestAppContext) { init_test(cx); From 5cc20ff37249bd2a9dfec0d45c03d58148536b21 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 9 Jun 2026 13:04:02 +0200 Subject: [PATCH 5/6] cleanup --- crates/acp_thread/src/acp_thread.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 676bd98937ba5f..0fa33787a287cd 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -545,12 +545,7 @@ impl ToolCall { } fn update_acp_status(&mut self, status: acp::ToolCallStatus) { - if let ToolCallStatus::WaitingForConfirmation { current_status, .. } = &mut self.status - && matches!( - status, - acp::ToolCallStatus::Pending | acp::ToolCallStatus::InProgress - ) - { + if let ToolCallStatus::WaitingForConfirmation { current_status, .. } = &mut self.status { *current_status = status; } else { self.status = status.into(); @@ -2172,7 +2167,6 @@ impl AcpThread { match update { ToolCallUpdate::UpdateFields(update) => { let location_updated = update.fields.locations.is_some(); - call.update_fields( update.fields, update.meta, From f5d060f79086ce884f52f4ae95fe116b95b95bd1 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 9 Jun 2026 13:06:07 +0200 Subject: [PATCH 6/6] cleanup 2 --- crates/acp_thread/src/acp_thread.rs | 69 ++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 0fa33787a287cd..658849b0ab7ad8 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -545,7 +545,12 @@ impl ToolCall { } fn update_acp_status(&mut self, status: acp::ToolCallStatus) { - if let ToolCallStatus::WaitingForConfirmation { current_status, .. } = &mut self.status { + if let ToolCallStatus::WaitingForConfirmation { current_status, .. } = &mut self.status + && matches!( + status, + acp::ToolCallStatus::Pending | acp::ToolCallStatus::InProgress + ) + { *current_status = status; } else { self.status = status.into(); @@ -4700,6 +4705,68 @@ mod tests { } } + #[gpui::test] + async fn test_terminal_tool_call_update_closes_open_permission_request( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let connection = Rc::new(FakeAgentConnection::new()); + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .unwrap(); + + let tool_call_id = acp::ToolCallId::new("toolu_01completed_while_waiting"); + let permission_task = thread + .update(cx, |thread, cx| { + thread.request_tool_call_authorization( + acp::ToolCall::new(tool_call_id.clone(), "Needs permission") + .kind(acp::ToolKind::Execute) + .status(acp::ToolCallStatus::Pending) + .into(), + PermissionOptions::Flat(vec![acp::PermissionOption::new( + acp::PermissionOptionId::new("allow"), + "Allow", + acp::PermissionOptionKind::AllowOnce, + )]), + AuthorizationKind::PermissionGrant, + cx, + ) + }) + .unwrap(); + + thread + .update(cx, |thread, cx| { + thread.handle_session_update( + acp::SessionUpdate::ToolCallUpdate(acp::ToolCallUpdate::new( + tool_call_id.clone(), + acp::ToolCallUpdateFields::new().status(acp::ToolCallStatus::Completed), + )), + cx, + ) + }) + .unwrap(); + + thread.read_with(cx, |thread, _cx| { + let (_, tool_call) = thread + .tool_call(&tool_call_id) + .expect("tool call should exist"); + assert!(matches!(tool_call.status, ToolCallStatus::Completed)); + }); + + match permission_task.await { + RequestPermissionOutcome::Cancelled => {} + RequestPermissionOutcome::Selected(_) => { + panic!("terminal tool call update should close pending permission request") + } + } + } + #[gpui::test] async fn test_no_pending_edits_if_tool_calls_are_completed(cx: &mut TestAppContext) { init_test(cx);