From c3951af24fff4942fdd941d814f1d8c84e25b9e3 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 18 May 2026 20:27:54 +0200 Subject: [PATCH] acp: Support additional session directories (#57051) Still behind a feature flag for now for testing with various agents. 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: - N/A --- Cargo.lock | 8 +- Cargo.toml | 2 +- crates/acp_thread/src/connection.rs | 19 + crates/agent_servers/src/acp.rs | 489 +++++++++++++++++- .../src/conversation_view/thread_view.rs | 9 + 5 files changed, 496 insertions(+), 31 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cfe21109573..3631fa41c4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -224,9 +224,9 @@ dependencies = [ [[package]] name = "agent-client-protocol" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1084cabbc2b00d353bad7e54750b0ef0f0bba9204c5884240c83a628704db86c" +checksum = "4361ba6627e51de955b10f3c77fb9eb959c85191a236c1c2c84e32f4ff240faf" dependencies = [ "agent-client-protocol-derive", "agent-client-protocol-schema", @@ -259,9 +259,9 @@ dependencies = [ [[package]] name = "agent-client-protocol-schema" -version = "0.13.1" +version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2984583e634f3f4d479b585aaa76de4a633255dcdf2be6489c6a8486f758af04" +checksum = "b957d8391ac3933e2a940446171c508d2b8ffc386d8fa7d0b9c936a2575b463e" dependencies = [ "anyhow", "derive_more", diff --git a/Cargo.toml b/Cargo.toml index 7303d4ebe0a..e6d883245ab 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -500,7 +500,7 @@ ztracing_macro = { path = "crates/ztracing_macro" } # External crates # -agent-client-protocol = { version = "=0.12.0", features = ["unstable"] } +agent-client-protocol = { version = "=0.12.1", 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 87c8ccf65c1..f58d8a581b8 100644 --- a/crates/acp_thread/src/connection.rs +++ b/crates/acp_thread/src/connection.rs @@ -115,6 +115,11 @@ pub trait AgentConnection { self.supports_load_session() || self.supports_resume_session() } + /// Whether this agent supports additional session directories. + fn supports_session_additional_directories(&self, _cx: &App) -> bool { + false + } + fn auth_methods(&self) -> &[acp::AuthMethod]; fn terminal_auth_task( @@ -702,6 +707,7 @@ mod test_support { permission_requests: HashMap, next_prompt_updates: Arc>>, supports_load_session: bool, + supports_session_additional_directories: bool, agent_id: AgentId, telemetry_id: SharedString, } @@ -724,6 +730,7 @@ mod test_support { permission_requests: HashMap::default(), sessions: Arc::default(), supports_load_session: false, + supports_session_additional_directories: false, agent_id: AgentId::new("stub"), telemetry_id: "stub".into(), } @@ -746,6 +753,14 @@ mod test_support { self } + pub fn with_supports_session_additional_directories( + mut self, + supports_session_additional_directories: bool, + ) -> Self { + self.supports_session_additional_directories = supports_session_additional_directories; + self + } + pub fn with_agent_id(mut self, agent_id: AgentId) -> Self { self.agent_id = agent_id; self @@ -863,6 +878,10 @@ mod test_support { self.supports_load_session } + fn supports_session_additional_directories(&self, _cx: &App) -> bool { + self.supports_session_additional_directories + } + fn load_session( self: Rc, session_id: acp::SessionId, diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index b3328790a5d..ff5519b7240 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -9,7 +9,7 @@ use agent_client_protocol::{ }; use anyhow::anyhow; use async_channel; -use collections::HashMap; +use collections::{HashMap, HashSet}; use feature_flags::{AcpBetaFeatureFlag, FeatureFlagAppExt as _}; use futures::channel::mpsc; use futures::future::Shared; @@ -509,6 +509,7 @@ impl AgentSessionList for AcpSessionList { cx: &mut App, ) -> Task> { let conn = self.connection.clone(); + let include_additional_directories = cx.has_flag::(); cx.foreground_executor().spawn(async move { let acp_request = acp::ListSessionsRequest::new() .cwd(request.cwd) @@ -522,7 +523,14 @@ impl AgentSessionList for AcpSessionList { .into_iter() .map(|s| AgentSessionInfo { session_id: s.session_id, - work_dirs: Some(PathList::new(&[s.cwd])), + work_dirs: Some(work_dirs_from_session_info( + s.cwd, + if include_additional_directories { + s.additional_directories + } else { + vec![] + }, + )), title: s.title.map(Into::into), updated_at: s.updated_at.and_then(|date_str| { chrono::DateTime::parse_from_rfc3339(&date_str) @@ -1053,6 +1061,15 @@ impl AcpConnection { } } + fn session_directories_from_work_dirs( + &self, + work_dirs: &PathList, + cx: &App, + ) -> Result { + let supports_additional_directories = self.supports_session_additional_directories(cx); + session_directories_from_work_dirs(work_dirs, supports_additional_directories) + } + fn open_or_create_session( self: Rc, session_id: acp::SessionId, @@ -1062,7 +1079,7 @@ impl AcpConnection { rpc_call: impl FnOnce( ConnectionTo, acp::SessionId, - PathBuf, + SessionDirectories, ) -> futures::future::LocalBoxFuture<'static, Result> + 'static, @@ -1089,9 +1106,9 @@ impl AcpConnection { } } - // TODO: remove this once ACP supports multiple working directories - let Some(cwd) = work_dirs.ordered_paths().next().cloned() else { - return Task::ready(Err(anyhow!("Working directory cannot be empty"))); + let directories = match self.session_directories_from_work_dirs(&work_dirs, cx) { + Ok(directories) => directories, + Err(error) => return Task::ready(Err(error)), }; let shared_task = cx @@ -1133,7 +1150,9 @@ impl AcpConnection { ); let response = - match rpc_call(this.connection.clone(), session_id.clone(), cwd).await { + match rpc_call(this.connection.clone(), session_id.clone(), directories) + .await + { Ok(response) => response, Err(err) => { this.sessions.borrow_mut().remove(&session_id); @@ -1288,6 +1307,77 @@ impl AcpConnection { } } +#[derive(Clone, Debug, PartialEq, Eq)] +struct SessionDirectories { + cwd: PathBuf, + additional_directories: Vec, +} + +impl SessionDirectories { + fn into_new_session_request(self, mcp_servers: Vec) -> acp::NewSessionRequest { + acp::NewSessionRequest::new(self.cwd) + .additional_directories(self.additional_directories) + .mcp_servers(mcp_servers) + } + + fn into_load_session_request( + self, + session_id: acp::SessionId, + mcp_servers: Vec, + ) -> acp::LoadSessionRequest { + acp::LoadSessionRequest::new(session_id, self.cwd) + .additional_directories(self.additional_directories) + .mcp_servers(mcp_servers) + } + + fn into_resume_session_request( + self, + session_id: acp::SessionId, + mcp_servers: Vec, + ) -> acp::ResumeSessionRequest { + acp::ResumeSessionRequest::new(session_id, self.cwd) + .additional_directories(self.additional_directories) + .mcp_servers(mcp_servers) + } +} + +fn session_directories_from_work_dirs( + work_dirs: &PathList, + supports_additional_directories: bool, +) -> Result { + let mut ordered_paths = work_dirs.ordered_paths(); + let cwd = ordered_paths + .next() + .cloned() + .ok_or_else(|| anyhow!("Working directory cannot be empty"))?; + let additional_directories = if supports_additional_directories { + ordered_paths.cloned().collect() + } else { + Vec::new() + }; + + Ok(SessionDirectories { + cwd, + additional_directories, + }) +} + +fn work_dirs_from_session_info(cwd: PathBuf, additional_directories: Vec) -> PathList { + let mut seen_paths = HashSet::default(); + let mut paths = Vec::with_capacity(1 + additional_directories.len()); + + seen_paths.insert(cwd.clone()); + paths.push(cwd); + + for path in additional_directories { + if seen_paths.insert(path.clone()) { + paths.push(path); + } + } + + PathList::new(&paths) +} + fn emit_load_error_to_all_sessions( sessions: &Rc>>, error: LoadError, @@ -1385,17 +1475,18 @@ impl AgentConnection for AcpConnection { work_dirs: PathList, cx: &mut App, ) -> Task>> { - // TODO: remove this once ACP supports multiple working directories - let Some(cwd) = work_dirs.ordered_paths().next().cloned() else { - return Task::ready(Err(anyhow!("Working directory cannot be empty"))); + let directories = match self.session_directories_from_work_dirs(&work_dirs, cx) { + Ok(directories) => directories, + Err(error) => return Task::ready(Err(error)), }; let name = self.id.0.clone(); let mcp_servers = mcp_servers_for_project(&project, cx); cx.spawn(async move |cx| { let response = into_foreground_future( - self.connection - .send_request(acp::NewSessionRequest::new(cwd.clone()).mcp_servers(mcp_servers)), + self.connection.send_request( + directories.into_new_session_request(mcp_servers), + ), ) .await .map_err(map_acp_error)?; @@ -1550,6 +1641,15 @@ impl AgentConnection for AcpConnection { .is_some() } + fn supports_session_additional_directories(&self, cx: &App) -> bool { + cx.has_flag::() + && self + .agent_capabilities + .session_capabilities + .additional_directories + .is_some() + } + fn load_session( self: Rc, session_id: acp::SessionId, @@ -1570,14 +1670,11 @@ impl AgentConnection for AcpConnection { project, work_dirs, title, - move |connection, session_id, cwd| { + move |connection, session_id, directories| { Box::pin(async move { - let response = into_foreground_future( - connection.send_request( - acp::LoadSessionRequest::new(session_id.clone(), cwd) - .mcp_servers(mcp_servers), - ), - ) + let response = into_foreground_future(connection.send_request( + directories.into_load_session_request(session_id.clone(), mcp_servers), + )) .await .map_err(map_acp_error)?; Ok(SessionConfigResponse { @@ -1616,14 +1713,11 @@ impl AgentConnection for AcpConnection { project, work_dirs, title, - move |connection, session_id, cwd| { + move |connection, session_id, directories| { Box::pin(async move { - let response = into_foreground_future( - connection.send_request( - acp::ResumeSessionRequest::new(session_id.clone(), cwd) - .mcp_servers(mcp_servers), - ), - ) + let response = into_foreground_future(connection.send_request( + directories.into_resume_session_request(session_id.clone(), mcp_servers), + )) .await .map_err(map_acp_error)?; Ok(SessionConfigResponse { @@ -2107,6 +2201,10 @@ pub mod test_support { self.inner.supports_resume_session() } + fn supports_session_additional_directories(&self, cx: &App) -> bool { + self.inner.supports_session_additional_directories(cx) + } + fn resume_session( self: Rc, session_id: acp::SessionId, @@ -2557,6 +2655,345 @@ mod tests { ); } + #[test] + fn session_directories_use_ordered_paths_when_supported() { + let work_dirs = PathList::new(&[ + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ]); + + let directories = + session_directories_from_work_dirs(&work_dirs, true).expect("work dirs should convert"); + + assert_eq!( + directories, + SessionDirectories { + cwd: std::path::PathBuf::from("/workspace-b"), + additional_directories: vec![ + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c") + ], + } + ); + + let session_id = acp::SessionId::new("session-1"); + let new_session_request = directories.clone().into_new_session_request(Vec::new()); + let load_session_request = directories + .clone() + .into_load_session_request(session_id.clone(), Vec::new()); + let resume_session_request = + directories.into_resume_session_request(session_id, Vec::new()); + + assert_eq!( + new_session_request.cwd, + std::path::PathBuf::from("/workspace-b") + ); + assert_eq!( + new_session_request.additional_directories, + vec![ + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c") + ] + ); + assert_eq!( + load_session_request.additional_directories, + new_session_request.additional_directories + ); + assert_eq!( + resume_session_request.additional_directories, + new_session_request.additional_directories + ); + } + + #[test] + fn session_directories_drop_additional_paths_when_unsupported() { + let work_dirs = PathList::new(&[ + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + ]); + + let directories = session_directories_from_work_dirs(&work_dirs, false) + .expect("work dirs should convert"); + + assert_eq!( + directories, + SessionDirectories { + cwd: std::path::PathBuf::from("/workspace-b"), + additional_directories: Vec::new(), + } + ); + } + + #[test] + fn session_info_work_dirs_preserve_cwd_then_additional_directories() { + let work_dirs = work_dirs_from_session_info( + std::path::PathBuf::from("/workspace-b"), + vec![ + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ], + ); + + assert_eq!( + work_dirs.ordered_paths().cloned().collect::>(), + vec![ + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ] + ); + } + + #[test] + fn session_info_work_dirs_deduplicate_cwd_and_additional_directories() { + let work_dirs = work_dirs_from_session_info( + std::path::PathBuf::from("/workspace-b"), + vec![ + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ], + ); + + assert_eq!( + work_dirs.ordered_paths().cloned().collect::>(), + vec![ + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ] + ); + } + + #[gpui::test] + async fn session_list_includes_additional_directories_in_work_dirs_when_beta_enabled( + cx: &mut gpui::TestAppContext, + ) { + cx.update(|cx| set_acp_beta_override(cx, "on")); + let connection = connect_session_list_test_agent( + vec![ + acp::SessionInfo::new("session-1", "/workspace-b").additional_directories(vec![ + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ]), + ], + cx, + ) + .await; + let session_list = AcpSessionList::new(connection, false); + + let response = cx + .update(|cx| session_list.list_sessions(AgentSessionListRequest::default(), cx)) + .await + .expect("session list should load"); + let session = response + .sessions + .first() + .expect("session list should include the returned session"); + let work_dirs = session + .work_dirs + .as_ref() + .expect("session should include work dirs"); + + assert_eq!( + work_dirs.ordered_paths().cloned().collect::>(), + vec![ + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ] + ); + } + + #[gpui::test] + async fn session_list_excludes_additional_directories_in_work_dirs_when_beta_disabled( + cx: &mut gpui::TestAppContext, + ) { + cx.update(|cx| set_acp_beta_override(cx, "off")); + + let connection = connect_session_list_test_agent( + vec![ + acp::SessionInfo::new("session-1", "/workspace-b").additional_directories(vec![ + std::path::PathBuf::from("/workspace-a"), + std::path::PathBuf::from("/workspace-c"), + ]), + ], + cx, + ) + .await; + let session_list = AcpSessionList::new(connection, false); + + let response = cx + .update(|cx| session_list.list_sessions(AgentSessionListRequest::default(), cx)) + .await + .expect("session list should load"); + let session = response + .sessions + .first() + .expect("session list should include the returned session"); + let work_dirs = session + .work_dirs + .as_ref() + .expect("session should include work dirs"); + + assert_eq!( + work_dirs.ordered_paths().cloned().collect::>(), + vec![std::path::PathBuf::from("/workspace-b")] + ); + } + + fn set_acp_beta_override(cx: &mut App, value: &str) { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + settings::SettingsStore::update_global(cx, |store, _| { + store.register_setting::(); + }); + feature_flags::FeatureFlagStore::init(cx); + + let value = value.to_string(); + settings::SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings(cx, |content| { + content + .feature_flags + .get_or_insert_default() + .insert(AcpBetaFeatureFlag::NAME.to_string(), value); + }); + }); + } + + async fn connect_session_list_test_agent( + sessions: Vec, + cx: &mut gpui::TestAppContext, + ) -> ConnectionTo { + let (client_transport, agent_transport) = agent_client_protocol::Channel::duplex(); + let sessions = Arc::new(sessions); + + cx.background_spawn( + Agent + .builder() + .name("list-test-agent") + .on_receive_request( + { + let sessions = sessions.clone(); + async move |_request: acp::ListSessionsRequest, responder, _cx| { + responder.respond(acp::ListSessionsResponse::new((*sessions).clone())) + } + }, + 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("list-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 additional_directories_support_requires_beta_flag_and_agent_capability( + cx: &mut gpui::TestAppContext, + ) { + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + settings::SettingsStore::update_global(cx, |store, _| { + store.register_setting::(); + }); + feature_flags::FeatureFlagStore::init(cx); + }); + + let fs = fs::FakeFs::new(cx.executor()); + fs.insert_tree("/", serde_json::json!({ "a": {}, "b": {} })) + .await; + let project = project::Project::test(fs, [std::path::Path::new("/a")], cx).await; + let mut harness = test_support::connect_fake_acp_connection(project, cx).await; + cx.update(|cx| { + settings::SettingsStore::update_global(cx, |store, _| { + store.register_setting::(); + }); + feature_flags::FeatureFlagStore::init(cx); + }); + + let work_dirs = PathList::new(&[ + std::path::PathBuf::from("/workspace-b"), + std::path::PathBuf::from("/workspace-a"), + ]); + + let missing_capability = cx + .update(|cx| { + harness + .connection + .session_directories_from_work_dirs(&work_dirs, cx) + }) + .expect("work dirs should convert"); + assert!(missing_capability.additional_directories.is_empty()); + + Rc::get_mut(&mut harness.connection) + .expect("test harness should own the only ACP connection handle") + .agent_capabilities + .session_capabilities + .additional_directories = Some(acp::SessionAdditionalDirectoriesCapabilities::new()); + + cx.update(|cx| { + settings::SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings(cx, |content| { + content + .feature_flags + .get_or_insert_default() + .insert("acp-beta".to_string(), "off".to_string()); + }); + }); + }); + let disabled = cx + .update(|cx| { + harness + .connection + .session_directories_from_work_dirs(&work_dirs, cx) + }) + .expect("work dirs should convert"); + assert!(disabled.additional_directories.is_empty()); + + cx.update(|cx| { + settings::SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings(cx, |content| { + content + .feature_flags + .get_or_insert_default() + .insert("acp-beta".to_string(), "on".to_string()); + }); + }); + }); + let enabled = cx + .update(|cx| { + harness + .connection + .session_directories_from_work_dirs(&work_dirs, cx) + }) + .expect("work dirs should convert"); + assert_eq!( + enabled, + SessionDirectories { + cwd: std::path::PathBuf::from("/workspace-b"), + additional_directories: vec![std::path::PathBuf::from("/workspace-a")], + } + ); + } + #[gpui::test] async fn session_delete_support_requires_beta_flag_and_capability( cx: &mut gpui::TestAppContext, diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 3bd2fb6326b..9d78baf826c 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -8826,6 +8826,15 @@ impl ThreadView { return None; } + if self + .thread + .read(cx) + .connection() + .supports_session_additional_directories(cx) + { + return None; + } + let project = self.project.upgrade()?; let worktree_count = project.read(cx).visible_worktrees(cx).count(); if worktree_count <= 1 {