From fd68265efd872b3840e774708567d1e695c1e895 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Mon, 28 Jul 2025 13:18:01 -0300 Subject: [PATCH] Fix integration tests for claude (#35212) Release Notes: - N/A --- crates/agent_servers/src/claude.rs | 12 +++- crates/agent_servers/src/claude/mcp_server.rs | 5 ++ crates/agent_servers/src/codex.rs | 2 +- crates/agent_servers/src/e2e_tests.rs | 56 ++++++++++++++++--- crates/agent_servers/src/gemini.rs | 2 +- crates/agent_servers/src/mcp_server.rs | 12 +++- 6 files changed, 73 insertions(+), 16 deletions(-) diff --git a/crates/agent_servers/src/claude.rs b/crates/agent_servers/src/claude.rs index 4b48dbf3c1..6565786204 100644 --- a/crates/agent_servers/src/claude.rs +++ b/crates/agent_servers/src/claude.rs @@ -438,7 +438,7 @@ impl ClaudeAgentSession { } } } - SdkMessage::System { .. } => {} + SdkMessage::System { .. } | SdkMessage::ControlResponse { .. } => {} } } @@ -642,6 +642,8 @@ enum SdkMessage { request_id: String, request: ControlRequest, }, + /// Response to a control request + ControlResponse { response: ControlResponse }, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -651,6 +653,12 @@ enum ControlRequest { Interrupt, } +#[derive(Debug, Clone, Serialize, Deserialize)] +struct ControlResponse { + request_id: String, + subtype: ResultErrorType, +} + #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] enum ResultErrorType { @@ -707,7 +715,7 @@ pub(crate) mod tests { use super::*; use serde_json::json; - crate::common_e2e_tests!(ClaudeCode); + crate::common_e2e_tests!(ClaudeCode, allow_option_id = "allow"); pub fn local_command() -> AgentServerCommand { AgentServerCommand { diff --git a/crates/agent_servers/src/claude/mcp_server.rs b/crates/agent_servers/src/claude/mcp_server.rs index a320a6d37f..cc303016f1 100644 --- a/crates/agent_servers/src/claude/mcp_server.rs +++ b/crates/agent_servers/src/claude/mcp_server.rs @@ -42,9 +42,13 @@ impl ClaudeZedMcpServer { } pub fn server_config(&self) -> Result { + #[cfg(not(test))] let zed_path = std::env::current_exe() .context("finding current executable path for use in mcp_server")?; + #[cfg(test)] + let zed_path = crate::e2e_tests::get_zed_path(); + Ok(McpServerConfig { command: zed_path, args: vec![ @@ -174,6 +178,7 @@ impl McpServerTool for PermissionTool { updated_input: input.input, } } else { + debug_assert_eq!(chosen_option, reject_option_id); PermissionToolResponse { behavior: PermissionToolBehavior::Deny, updated_input: input.input, diff --git a/crates/agent_servers/src/codex.rs b/crates/agent_servers/src/codex.rs index 3eb95a6841..b10ce9cf54 100644 --- a/crates/agent_servers/src/codex.rs +++ b/crates/agent_servers/src/codex.rs @@ -302,7 +302,7 @@ pub(crate) mod tests { use crate::AgentServerCommand; use std::path::Path; - crate::common_e2e_tests!(Codex); + crate::common_e2e_tests!(Codex, allow_option_id = "approve"); pub fn local_command() -> AgentServerCommand { let cli_path = Path::new(env!("CARGO_MANIFEST_DIR")) diff --git a/crates/agent_servers/src/e2e_tests.rs b/crates/agent_servers/src/e2e_tests.rs index 905f06a148..aca9001c79 100644 --- a/crates/agent_servers/src/e2e_tests.rs +++ b/crates/agent_servers/src/e2e_tests.rs @@ -1,4 +1,8 @@ -use std::{path::Path, sync::Arc, time::Duration}; +use std::{ + path::{Path, PathBuf}, + sync::Arc, + time::Duration, +}; use crate::{AgentServer, AgentServerSettings, AllAgentServersSettings}; use acp_thread::{AcpThread, AgentThreadEntry, ToolCall, ToolCallStatus}; @@ -79,21 +83,28 @@ pub async fn test_path_mentions(server: impl AgentServer + 'static, cx: &mut Tes .unwrap(); thread.read_with(cx, |thread, cx| { - assert_eq!(thread.entries().len(), 3); assert!(matches!( thread.entries()[0], AgentThreadEntry::UserMessage(_) )); - assert!(matches!(thread.entries()[1], AgentThreadEntry::ToolCall(_))); - let AgentThreadEntry::AssistantMessage(assistant_message) = &thread.entries()[2] else { - panic!("Expected AssistantMessage") - }; + let assistant_message = &thread + .entries() + .iter() + .rev() + .find_map(|entry| match entry { + AgentThreadEntry::AssistantMessage(msg) => Some(msg), + _ => None, + }) + .unwrap(); + assert!( assistant_message.to_markdown(cx).contains("Hello, world!"), "unexpected assistant message: {:?}", assistant_message.to_markdown(cx) ); }); + + drop(tempdir); } pub async fn test_tool_call(server: impl AgentServer + 'static, cx: &mut TestAppContext) { @@ -136,6 +147,7 @@ pub async fn test_tool_call(server: impl AgentServer + 'static, cx: &mut TestApp pub async fn test_tool_call_with_confirmation( server: impl AgentServer + 'static, + allow_option_id: acp::PermissionOptionId, cx: &mut TestAppContext, ) { let fs = init_test(cx).await; @@ -186,7 +198,7 @@ pub async fn test_tool_call_with_confirmation( thread.update(cx, |thread, cx| { thread.authorize_tool_call( tool_call_id, - acp::PermissionOptionId("0".into()), + allow_option_id, acp::PermissionOptionKind::AllowOnce, cx, ); @@ -294,7 +306,7 @@ pub async fn test_cancel(server: impl AgentServer + 'static, cx: &mut TestAppCon #[macro_export] macro_rules! common_e2e_tests { - ($server:expr) => { + ($server:expr, allow_option_id = $allow_option_id:expr) => { mod common_e2e { use super::*; @@ -319,7 +331,12 @@ macro_rules! common_e2e_tests { #[::gpui::test] #[cfg_attr(not(feature = "e2e"), ignore)] async fn tool_call_with_confirmation(cx: &mut ::gpui::TestAppContext) { - $crate::e2e_tests::test_tool_call_with_confirmation($server, cx).await; + $crate::e2e_tests::test_tool_call_with_confirmation( + $server, + ::agent_client_protocol::PermissionOptionId($allow_option_id.into()), + cx, + ) + .await; } #[::gpui::test] @@ -412,3 +429,24 @@ pub async fn run_until_first_tool_call( } } } + +pub fn get_zed_path() -> PathBuf { + let mut zed_path = std::env::current_exe().unwrap(); + + while zed_path + .file_name() + .map_or(true, |name| name.to_string_lossy() != "debug") + { + if !zed_path.pop() { + panic!("Could not find target directory"); + } + } + + zed_path.push("zed"); + + if !zed_path.exists() { + panic!("\n🚨 Run `cargo build` at least once before running e2e tests\n\n"); + } + + zed_path +} diff --git a/crates/agent_servers/src/gemini.rs b/crates/agent_servers/src/gemini.rs index 47b965cdad..8b9fed5777 100644 --- a/crates/agent_servers/src/gemini.rs +++ b/crates/agent_servers/src/gemini.rs @@ -188,7 +188,7 @@ pub(crate) mod tests { use crate::AgentServerCommand; use std::path::Path; - crate::common_e2e_tests!(Gemini); + crate::common_e2e_tests!(Gemini, allow_option_id = "0"); pub fn local_command() -> AgentServerCommand { let cli_path = Path::new(env!("CARGO_MANIFEST_DIR")) diff --git a/crates/agent_servers/src/mcp_server.rs b/crates/agent_servers/src/mcp_server.rs index 47575fa3ea..055b89dfe2 100644 --- a/crates/agent_servers/src/mcp_server.rs +++ b/crates/agent_servers/src/mcp_server.rs @@ -1,6 +1,6 @@ use acp_thread::AcpThread; use agent_client_protocol as acp; -use anyhow::{Context, Result}; +use anyhow::Result; use context_server::listener::{McpServerTool, ToolResponse}; use context_server::types::{ Implementation, InitializeParams, InitializeResponse, ProtocolVersion, ServerCapabilities, @@ -38,8 +38,14 @@ impl ZedMcpServer { } pub fn server_config(&self) -> Result { - let zed_path = std::env::current_exe() - .context("finding current executable path for use in mcp_server")?; + #[cfg(not(test))] + let zed_path = anyhow::Context::context( + std::env::current_exe(), + "finding current executable path for use in mcp_server", + )?; + + #[cfg(test)] + let zed_path = crate::e2e_tests::get_zed_path(); Ok(acp::McpServerConfig { command: zed_path,