diff --git a/Cargo.lock b/Cargo.lock index fcd7c96fa82..c90f3ef4f7e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -224,13 +224,14 @@ dependencies = [ [[package]] name = "agent-client-protocol" -version = "0.11.1" +version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2af62fb84df2af0f933d8f5fd78b843fa5eb0ec5a48fa1b528c41951d0bbe36c" +checksum = "1084cabbc2b00d353bad7e54750b0ef0f0bba9204c5884240c83a628704db86c" dependencies = [ "agent-client-protocol-derive", "agent-client-protocol-schema", - "anyhow", + "async-process", + "blocking", "futures 0.3.32", "futures-concurrency", "jsonrpcmsg", @@ -239,7 +240,7 @@ dependencies = [ "schemars 1.0.4", "serde", "serde_json", - "thiserror 2.0.17", + "shell-words", "tokio", "tokio-util", "tracing", @@ -248,20 +249,19 @@ dependencies = [ [[package]] name = "agent-client-protocol-derive" -version = "0.11.0" +version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce42c2d3c048c12897eef2e577dfff1e3355c632c9f1625cc953b9df48b44631" +checksum = "cabdc9d845d08ec7ed2d0c9de1ae4a1b198301407d55855261572761be90ec9f" dependencies = [ - "proc-macro2", "quote", "syn 2.0.117", ] [[package]] name = "agent-client-protocol-schema" -version = "0.12.0" +version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49bae57dad1c28a362fbdcf7bab0583316a02b45a70792109fced55780a3b63c" +checksum = "2984583e634f3f4d479b585aaa76de4a633255dcdf2be6489c6a8486f758af04" dependencies = [ "anyhow", "derive_more", @@ -2254,6 +2254,15 @@ dependencies = [ "utf8-chars", ] +[[package]] +name = "bs58" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bf88ba1141d185c399bee5288d850d63b8369520c1eafc32a0430b5b6c287bf4" +dependencies = [ + "tinyvec", +] + [[package]] name = "bstr" version = "1.12.1" @@ -16042,11 +16051,12 @@ dependencies = [ [[package]] name = "serde_with" -version = "3.18.0" +version = "3.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd5414fad8e6907dbdd5bc441a50ae8d6e26151a03b1de04d89a5576de61d01f" +checksum = "e72c1c2cb7b223fafb600a619537a871c2818583d619401b785e7c0b746ccde2" dependencies = [ "base64 0.22.1", + "bs58", "chrono", "hex", "indexmap 1.9.3", @@ -16061,9 +16071,9 @@ dependencies = [ [[package]] name = "serde_with_macros" -version = "3.18.0" +version = "3.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d3db8978e608f1fe7357e211969fd9abdcae80bac1ba7a3369bb7eb6b404eb65" +checksum = "b90c488738ecb4fb0262f41f43bc40efc5868d9fb744319ddf5f5317f417bfac" dependencies = [ "darling 0.23.0", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 07c94aa33ee..7303d4ebe0a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -500,7 +500,7 @@ ztracing_macro = { path = "crates/ztracing_macro" } # External crates # -agent-client-protocol = { version = "=0.11.1", features = ["unstable"] } +agent-client-protocol = { version = "=0.12.0", features = ["unstable"] } aho-corasick = "1.1" alacritty_terminal = { git = "https://github.com/zed-industries/alacritty", rev = "9d9640d4" } any_vec = "0.14" diff --git a/crates/acp_thread/src/connection.rs b/crates/acp_thread/src/connection.rs index 41cdd1250b3..b2bd00f61a3 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -310,7 +310,7 @@ pub trait AgentSessionList { cx: &mut App, ) -> Task>; - fn supports_delete(&self) -> bool { + fn supports_delete(&self, _cx: &App) -> bool { false } diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index a8b70c80b47..eda50ab5637 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -2428,7 +2428,7 @@ impl AgentSessionList for NativeAgentSessionList { Task::ready(Ok(AgentSessionListResponse::new(sessions))) } - fn supports_delete(&self) -> bool { + fn supports_delete(&self, _cx: &App) -> bool { true } diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 166a75ea250..f0a036ca4da 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -473,15 +473,17 @@ pub struct AcpSession { pub struct AcpSessionList { connection: ConnectionTo, + supports_delete: bool, updates_tx: async_channel::Sender, updates_rx: async_channel::Receiver, } impl AcpSessionList { - fn new(connection: ConnectionTo) -> Self { + fn new(connection: ConnectionTo, supports_delete: bool) -> Self { let (tx, rx) = async_channel::unbounded(); Self { connection, + supports_delete, updates_tx: tx, updates_rx: rx, } @@ -537,6 +539,29 @@ impl AgentSessionList for AcpSessionList { }) } + fn supports_delete(&self, cx: &App) -> bool { + self.supports_delete && cx.has_flag::() + } + + fn delete_session(&self, session_id: &acp::SessionId, cx: &mut App) -> Task> { + if !self.supports_delete(cx) { + return Task::ready(Err(anyhow::anyhow!("delete_session not supported"))); + } + + let conn = self.connection.clone(); + let updates_tx = self.updates_tx.clone(); + let session_id = session_id.clone(); + cx.foreground_executor().spawn(async move { + into_foreground_future(conn.send_request(acp::DeleteSessionRequest::new(session_id))) + .await + .map_err(map_acp_error)?; + updates_tx + .try_send(acp_thread::SessionListUpdate::Refresh) + .log_err(); + Ok(()) + }) + } + fn watch( &self, _cx: &mut App, @@ -927,6 +952,11 @@ impl AcpConnection { .unwrap_or_else(|| agent_id.0.clone()); let agent_version = agent_info .and_then(|info| (!info.version.is_empty()).then(|| SharedString::from(info.version))); + let agent_supports_delete = response + .agent_capabilities + .session_capabilities + .delete + .is_some(); let session_list = if response .agent_capabilities @@ -934,7 +964,10 @@ impl AcpConnection { .list .is_some() { - let list = Rc::new(AcpSessionList::new(connection.clone())); + let list = Rc::new(AcpSessionList::new( + connection.clone(), + agent_supports_delete, + )); *client_session_list.borrow_mut() = Some(list.clone()); Some(list) } else { @@ -2337,6 +2370,8 @@ pub mod test_support { mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; + use feature_flags::FeatureFlag as _; + use super::*; #[test] @@ -2484,6 +2519,128 @@ mod tests { ); } + #[gpui::test] + async fn session_delete_support_requires_beta_flag_and_capability( + cx: &mut gpui::TestAppContext, + ) { + let deleted_sessions = Arc::new(std::sync::Mutex::new(Vec::new())); + let connection = connect_session_delete_test_agent(deleted_sessions, cx).await; + let session_list = AcpSessionList::new(connection.clone(), true); + let missing_capability = AcpSessionList::new(connection, false); + + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + + assert_eq!( + session_list.supports_delete(cx), + cx.has_flag::() + ); + assert!(!missing_capability.supports_delete(cx)); + + cx.update_flags(false, vec![AcpBetaFeatureFlag::NAME.to_string()]); + assert!(session_list.supports_delete(cx)); + assert!(!missing_capability.supports_delete(cx)); + }); + } + + async fn connect_session_delete_test_agent( + deleted_sessions: Arc>>, + cx: &mut gpui::TestAppContext, + ) -> ConnectionTo { + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + + cx.background_spawn( + Agent + .builder() + .name("delete-test-agent") + .on_receive_request( + { + let deleted_sessions = deleted_sessions.clone(); + async move |request: acp::DeleteSessionRequest, responder, _cx| { + deleted_sessions + .lock() + .expect("deleted sessions lock should not be poisoned") + .push(request.session_id); + responder.respond(acp::DeleteSessionResponse::default()) + } + }, + agent_client_protocol::on_receive_request!(), + ) + .connect_to(agent_transport), + ) + .detach(); + + let (connection_tx, connection_rx) = futures::channel::oneshot::channel(); + cx.background_spawn(Client.builder().name("delete-test-client").connect_with( + client_transport, + move |connection: ConnectionTo| async move { + connection_tx.send(connection).ok(); + futures::future::pending::>().await + }, + )) + .detach(); + + connection_rx + .await + .expect("failed to receive ACP connection") + } + + #[gpui::test] + async fn session_list_delete_sends_session_delete_when_supported( + cx: &mut gpui::TestAppContext, + ) { + let deleted_sessions = Arc::new(std::sync::Mutex::new(Vec::new())); + let connection = connect_session_delete_test_agent(deleted_sessions.clone(), cx).await; + let session_list = AcpSessionList::new(connection, true); + let session_id = acp::SessionId::new("session-to-delete"); + + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + cx.update_flags(false, vec![AcpBetaFeatureFlag::NAME.to_string()]); + }); + cx.update(|cx| session_list.delete_session(&session_id, cx)) + .await + .expect("delete_session failed"); + + assert_eq!( + *deleted_sessions + .lock() + .expect("deleted sessions lock should not be poisoned"), + vec![session_id] + ); + } + + #[gpui::test] + async fn session_list_delete_does_not_send_when_unsupported(cx: &mut gpui::TestAppContext) { + let deleted_sessions = Arc::new(std::sync::Mutex::new(Vec::new())); + let connection = connect_session_delete_test_agent(deleted_sessions.clone(), cx).await; + let session_list = AcpSessionList::new(connection, false); + let session_id = acp::SessionId::new("session-to-delete"); + + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + cx.update_flags(false, vec![AcpBetaFeatureFlag::NAME.to_string()]); + }); + let error = cx + .update(|cx| session_list.delete_session(&session_id, cx)) + .await + .expect_err("delete_session should fail when unsupported"); + + assert!( + error.to_string().contains("delete_session not supported"), + "unexpected error: {error}" + ); + assert!( + deleted_sessions + .lock() + .expect("deleted sessions lock should not be poisoned") + .is_empty() + ); + } + #[cfg(not(windows))] #[gpui::test] async fn startup_returns_error_when_agent_exits_before_initialization( diff --git a/crates/agent_ui/src/threads_archive_view.rs b/crates/agent_ui/src/threads_archive_view.rs index f751c9ca1d6..d53a8d473e9 100644 --- a/crates/agent_ui/src/threads_archive_view.rs +++ b/crates/agent_ui/src/threads_archive_view.rs @@ -821,7 +821,11 @@ impl ThreadsArchiveView { let state = task.await?; let task = cx.update(|cx| { if let Some(session_id) = &session_id { - if let Some(list) = state.connection.session_list(cx) { + if let Some(list) = state + .connection + .session_list(cx) + .filter(|list| list.supports_delete(cx)) + { list.delete_session(session_id, cx) } else { Task::ready(Ok(()))