From d7ac5e6cf46fcd0087ee8a2c4296c7bfc876ef26 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Thu, 11 Jun 2026 12:17:58 +0200 Subject: [PATCH] acp_thread: Preserve waiting tool call status on updates (#58537) Allows contents to update without dropping the permission (caused by claude subagents) Closes https://github.com/agentclientprotocol/claude-agent-acp/issues/708 Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - acp: Fix failed permissions requests for external agents --- crates/acp_thread/src/acp_thread.rs | 464 +++++++++++++++++++++- 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, 602 insertions(+), 41 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 83b2f304d31..963e41cad0c 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -507,7 +507,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) { @@ -590,6 +590,31 @@ impl ToolCall { Ok(()) } + fn update_status(&mut self, status: ToolCallStatus) { + 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, + } + } + + 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), @@ -698,7 +723,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, @@ -764,6 +789,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, @@ -792,6 +818,26 @@ impl From for ToolCallStatus { } } +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 { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!( @@ -2331,7 +2377,7 @@ impl AcpThread { &self.terminals, cx, )?; - call.status = status; + call.update_status(status); cx.emit(AcpThreadEvent::EntryUpdated(ix)); } else { @@ -2482,7 +2528,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, @@ -2517,24 +2569,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); @@ -4589,6 +4647,382 @@ mod tests { }); } + #[gpui::test] + async fn test_duplicate_tool_call_update_preserves_open_permission_request_until_authorized( + 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); + }); + + 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); + assert_eq!(outcome.option_kind, acp::PermissionOptionKind::AllowOnce); + } + RequestPermissionOutcome::Cancelled => { + 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] + 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_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_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); diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index dff6744cdfc..8634f11646c 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -2209,6 +2209,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 19c5475c170..2b212bb9e73 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -827,6 +827,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), @@ -1103,6 +1107,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")] @@ -4889,6 +4912,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(); } @@ -5057,6 +5093,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 @@ -5242,6 +5283,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 @@ -5298,11 +5343,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(()); } @@ -5491,6 +5534,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 @@ -5519,6 +5573,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| { @@ -5546,28 +5603,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)); } @@ -5716,6 +5769,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( @@ -6821,6 +6889,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 e80e2110a17..fc513d904a3 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 016058318bf..f817a99a500 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 af9c857f630..0f8d96db0e4 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.