mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-25 17:04:19 +00:00
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
This commit is contained in:
parent
f56e782661
commit
d7ac5e6cf4
6 changed files with 602 additions and 41 deletions
|
|
@ -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<Item = &Entity<Diff>> {
|
||||
self.content.iter().filter_map(|content| match content {
|
||||
ToolCallContent::Diff(diff) => Some(diff),
|
||||
|
|
@ -698,7 +723,7 @@ pub enum SelectedPermissionParams {
|
|||
Terminal { patterns: Vec<String> },
|
||||
}
|
||||
|
||||
#[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<SelectedPermissionOutcome>,
|
||||
kind: AuthorizationKind,
|
||||
|
|
@ -792,6 +818,26 @@ impl From<acp::ToolCallStatus> for ToolCallStatus {
|
|||
}
|
||||
}
|
||||
|
||||
impl ToolCallStatus {
|
||||
fn as_acp_status(&self) -> Option<acp::ToolCallStatus> {
|
||||
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<Task<RequestPermissionOutcome>> {
|
||||
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);
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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<acp_thread::SelectedPermissionOutcome> {
|
||||
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<acp_thread::Diff>) {
|
||||
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<acp_thread::Diff> {
|
||||
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;
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue