diff --git a/crates/coding-agent-loop/src/profiles/anthropic.rs b/crates/coding-agent-loop/src/profiles/anthropic.rs index 558a4e376..784fde0c2 100644 --- a/crates/coding-agent-loop/src/profiles/anthropic.rs +++ b/crates/coding-agent-loop/src/profiles/anthropic.rs @@ -161,15 +161,6 @@ mod tests { use super::*; use crate::test_support::MockExecutionEnvironment; - fn linux_env() -> MockExecutionEnvironment { - MockExecutionEnvironment { - working_dir: "/home/test", - platform_str: "linux", - os_version_str: "Linux 6.1.0".into(), - ..Default::default() - } - } - #[test] fn anthropic_profile_identity() { let profile = AnthropicProfile::new("claude-sonnet-4-20250514"); @@ -189,7 +180,7 @@ mod tests { #[test] fn anthropic_system_prompt_contains_env_context() { let profile = AnthropicProfile::new("claude-sonnet-4-20250514"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("You are Claude, an AI coding assistant made by Anthropic")); assert!(prompt.contains("")); @@ -222,7 +213,7 @@ mod tests { #[test] fn anthropic_system_prompt_includes_project_docs() { let profile = AnthropicProfile::new("claude-sonnet-4-20250514"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let docs = vec!["# Project README".into(), "# CONTRIBUTING guide".into()]; let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &docs, None); assert!(prompt.contains("# Project README")); @@ -232,7 +223,7 @@ mod tests { #[test] fn anthropic_system_prompt_includes_env_context() { let profile = AnthropicProfile::new("claude-opus-4-6"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let ctx = EnvContext { git_branch: Some("feature-branch".into()), is_git_repo: true, @@ -253,7 +244,7 @@ mod tests { #[test] fn anthropic_system_prompt_includes_user_instructions() { let profile = AnthropicProfile::new("claude-opus-4-6"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let ctx = EnvContext::default(); let prompt = profile.build_system_prompt(&env, &ctx, &[], Some("Always write tests first")); assert!(prompt.contains("Always write tests first")); diff --git a/crates/coding-agent-loop/src/profiles/gemini.rs b/crates/coding-agent-loop/src/profiles/gemini.rs index bd84c4274..ca0272a59 100644 --- a/crates/coding-agent-loop/src/profiles/gemini.rs +++ b/crates/coding-agent-loop/src/profiles/gemini.rs @@ -212,15 +212,6 @@ mod tests { use crate::test_support::MockExecutionEnvironment; use std::sync::Arc; - fn linux_env() -> MockExecutionEnvironment { - MockExecutionEnvironment { - working_dir: "/home/test", - platform_str: "linux", - os_version_str: "Linux 6.1.0".into(), - ..Default::default() - } - } - #[test] fn gemini_profile_identity() { let profile = GeminiProfile::new("gemini-2.0-flash"); @@ -240,7 +231,7 @@ mod tests { #[test] fn gemini_system_prompt_contains_identity() { let profile = GeminiProfile::new("gemini-2.0-flash"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("You are Gemini CLI")); assert!(prompt.contains("solving bugs")); @@ -252,7 +243,7 @@ mod tests { #[test] fn gemini_system_prompt_contains_tool_guidance() { let profile = GeminiProfile::new("gemini-2.0-flash"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("read_file")); assert!(prompt.contains("read_many_files")); @@ -270,7 +261,7 @@ mod tests { #[test] fn gemini_system_prompt_contains_project_docs_convention() { let profile = GeminiProfile::new("gemini-2.0-flash"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("GEMINI.md")); assert!(prompt.contains("AGENTS.md")); @@ -279,7 +270,7 @@ mod tests { #[test] fn gemini_system_prompt_contains_coding_best_practices() { let profile = GeminiProfile::new("gemini-2.0-flash"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("clean, maintainable code")); assert!(prompt.contains("Handle errors appropriately")); @@ -289,7 +280,7 @@ mod tests { #[test] fn gemini_system_prompt_contains_env_context() { let profile = GeminiProfile::new("gemini-2.0-flash"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("")); assert!(prompt.contains("linux")); diff --git a/crates/coding-agent-loop/src/profiles/mod.rs b/crates/coding-agent-loop/src/profiles/mod.rs index b2be81067..2db4e3b08 100644 --- a/crates/coding-agent-loop/src/profiles/mod.rs +++ b/crates/coding-agent-loop/src/profiles/mod.rs @@ -93,18 +93,9 @@ mod tests { use super::*; use crate::test_support::MockExecutionEnvironment; - fn linux_env() -> MockExecutionEnvironment { - MockExecutionEnvironment { - working_dir: "/home/test", - platform_str: "linux", - os_version_str: "Linux 6.1.0".into(), - ..Default::default() - } - } - #[test] fn env_context_block_contains_platform() { - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let block = build_env_context_block(&env); assert!(block.contains("")); assert!(block.contains("")); @@ -115,7 +106,7 @@ mod tests { #[test] fn env_context_block_with_extra_context() { - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let ctx = EnvContext { git_branch: Some("main".into()), is_git_repo: true, diff --git a/crates/coding-agent-loop/src/profiles/openai.rs b/crates/coding-agent-loop/src/profiles/openai.rs index 2fe1e9e05..00992e9ac 100644 --- a/crates/coding-agent-loop/src/profiles/openai.rs +++ b/crates/coding-agent-loop/src/profiles/openai.rs @@ -416,104 +416,8 @@ fn make_apply_patch_tool() -> RegisteredTool { #[cfg(test)] mod tests { use super::*; - use crate::execution_env::*; - use crate::test_support::MockExecutionEnvironment; - use async_trait::async_trait; + use crate::test_support::{MockExecutionEnvironment, MutableMockExecutionEnvironment}; use std::collections::HashMap; - use std::sync::Mutex; - - fn linux_env() -> MockExecutionEnvironment { - MockExecutionEnvironment { - working_dir: "/home/test", - platform_str: "linux", - os_version_str: "Linux 6.1.0".into(), - ..Default::default() - } - } - - /// A specialized mock with Mutex-protected files for apply_patch tests that - /// need mutable write/delete operations. - struct MockFileEnv { - files: Mutex>, - } - - impl MockFileEnv { - fn new(files: HashMap) -> Self { - Self { - files: Mutex::new(files), - } - } - } - - #[async_trait] - impl ExecutionEnvironment for MockFileEnv { - async fn read_file(&self, path: &str, _: Option, _: Option) -> Result { - self.files - .lock() - .unwrap() - .get(path) - .cloned() - .ok_or_else(|| format!("File not found: {path}")) - } - async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { - self.files - .lock() - .unwrap() - .insert(path.to_string(), content.to_string()); - Ok(()) - } - async fn delete_file(&self, path: &str) -> Result<(), String> { - self.files.lock().unwrap().remove(path); - Ok(()) - } - async fn file_exists(&self, path: &str) -> Result { - Ok(self.files.lock().unwrap().contains_key(path)) - } - async fn list_directory(&self, _: &str, _: Option) -> Result, String> { - Ok(vec![]) - } - async fn exec_command( - &self, - _: &str, - _: u64, - _: Option<&str>, - _: Option<&std::collections::HashMap>, - ) -> Result { - Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 0, - timed_out: false, - duration_ms: 0, - }) - } - async fn grep( - &self, - _: &str, - _: &str, - _: &GrepOptions, - ) -> Result, String> { - Ok(vec![]) - } - async fn glob(&self, _: &str, _: Option<&str>) -> Result, String> { - Ok(vec![]) - } - async fn initialize(&self) -> Result<(), String> { - Ok(()) - } - async fn cleanup(&self) -> Result<(), String> { - Ok(()) - } - fn working_directory(&self) -> &str { - "/tmp" - } - fn platform(&self) -> &str { - "linux" - } - fn os_version(&self) -> String { - "Linux 6.1.0".into() - } - } #[test] fn openai_profile_identity() { @@ -534,7 +438,7 @@ mod tests { #[test] fn openai_system_prompt_contains_env_context() { let profile = OpenAiProfile::new("o3-mini"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("You are a coding agent powered by OpenAI")); assert!(prompt.contains("")); @@ -546,7 +450,7 @@ mod tests { #[test] fn openai_system_prompt_contains_tool_guidance() { let profile = OpenAiProfile::new("o3-mini"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("read_file")); assert!(prompt.contains("apply_patch")); @@ -560,7 +464,7 @@ mod tests { #[test] fn openai_system_prompt_contains_coding_best_practices() { let profile = OpenAiProfile::new("o3-mini"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None); assert!(prompt.contains("clean, maintainable code")); assert!(prompt.contains("existing code conventions")); @@ -569,7 +473,7 @@ mod tests { #[test] fn openai_system_prompt_includes_project_docs() { let profile = OpenAiProfile::new("o3-mini"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let docs = vec!["# Project README".into(), "# CONTRIBUTING guide".into()]; let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &docs, None); assert!(prompt.contains("# Project README")); @@ -579,7 +483,7 @@ mod tests { #[test] fn openai_system_prompt_includes_user_instructions() { let profile = OpenAiProfile::new("o3-mini"); - let env = linux_env(); + let env = MockExecutionEnvironment::linux(); let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], Some("Always write tests first")); assert!(prompt.contains("Always write tests first")); assert!(prompt.contains("# User Instructions")); @@ -737,7 +641,7 @@ mod tests { #[tokio::test] async fn apply_patch_add_file() { - let env = MockFileEnv::new(HashMap::new()); + let env = MutableMockExecutionEnvironment::new(HashMap::new()); let ops = vec![PatchOperation::Add { path: "src/new.rs".into(), content: "fn new() {}".into(), @@ -757,7 +661,7 @@ mod tests { "src/lib.rs".to_string(), "fn hello() {\n println!(\"old\");\n}".to_string(), ); - let env = MockFileEnv::new(files); + let env = MutableMockExecutionEnvironment::new(files); let ops = vec![PatchOperation::Update { path: "src/lib.rs".into(), diff --git a/crates/coding-agent-loop/src/provider_profile.rs b/crates/coding-agent-loop/src/provider_profile.rs index 99c7f5d5f..bdd48cc91 100644 --- a/crates/coding-agent-loop/src/provider_profile.rs +++ b/crates/coding-agent-loop/src/provider_profile.rs @@ -78,101 +78,37 @@ pub trait ProviderProfile: Send + Sync { #[cfg(test)] mod tests { use super::*; - use crate::execution_env::ExecutionEnvironment; - use crate::test_support::MockExecutionEnvironment; - - /// A specialized profile for provider_profile tests that uses distinct id/model - /// and a custom build_system_prompt (unlike the shared TestProfile). - struct ProviderTestProfile { - registry: ToolRegistry, - } - - impl ProviderTestProfile { - fn new() -> Self { - Self { - registry: ToolRegistry::new(), - } - } - } - - impl ProviderProfile for ProviderTestProfile { - fn id(&self) -> String { - "test-provider".into() - } - fn model(&self) -> String { - "test-model".into() - } - fn tool_registry(&self) -> &ToolRegistry { - &self.registry - } - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.registry - } - fn build_system_prompt( - &self, - env: &dyn ExecutionEnvironment, - _env_context: &EnvContext, - project_docs: &[String], - user_instructions: Option<&str>, - ) -> String { - let base = format!( - "You are working on {}. Docs: {}", - env.platform(), - project_docs.len() - ); - match user_instructions { - Some(instructions) => format!("{base}\n\n{instructions}"), - None => base, - } - } - fn capabilities(&self) -> ProfileCapabilities { - ProfileCapabilities { - supports_reasoning: true, - supports_streaming: true, - supports_parallel_tool_calls: false, - context_window_size: 200_000, - } - } - fn knowledge_cutoff(&self) -> &str { - "May 2025" - } - } + use crate::test_support::{MockExecutionEnvironment, TestProfile}; #[test] fn profile_id_and_model() { - let profile = ProviderTestProfile::new(); - assert_eq!(profile.id(), "test-provider"); - assert_eq!(profile.model(), "test-model"); + let profile = TestProfile::new(); + assert_eq!(profile.id(), "mock"); + assert_eq!(profile.model(), "mock-model"); } #[test] fn profile_capabilities() { - let profile = ProviderTestProfile::new(); - assert!(profile.supports_reasoning()); - assert!(profile.supports_streaming()); + let profile = TestProfile::new(); + assert!(!profile.supports_reasoning()); + assert!(!profile.supports_streaming()); assert!(!profile.supports_parallel_tool_calls()); assert_eq!(profile.context_window_size(), 200_000); } #[test] fn profile_build_system_prompt() { - let profile = ProviderTestProfile::new(); - let env = MockExecutionEnvironment { - working_dir: "/home/test", - platform_str: "linux", - os_version_str: "Linux 6.1.0".into(), - ..Default::default() - }; + let profile = TestProfile::new(); + let env = MockExecutionEnvironment::linux(); let ctx = EnvContext::default(); let docs = vec!["README.md contents".into()]; let prompt = profile.build_system_prompt(&env, &ctx, &docs, None); - assert!(prompt.contains("linux")); - assert!(prompt.contains("1")); + assert!(prompt.contains("test assistant")); } #[test] fn profile_build_system_prompt_with_user_instructions() { - let profile = ProviderTestProfile::new(); + let profile = TestProfile::new(); let env = MockExecutionEnvironment::default(); let ctx = EnvContext::default(); let prompt = profile.build_system_prompt(&env, &ctx, &[], Some("Always use TDD")); @@ -181,13 +117,13 @@ mod tests { #[test] fn profile_provider_options_none() { - let profile = ProviderTestProfile::new(); + let profile = TestProfile::new(); assert!(profile.provider_options().is_none()); } #[test] fn profile_tools_empty_registry() { - let profile = ProviderTestProfile::new(); + let profile = TestProfile::new(); assert!(profile.tools().is_empty()); } } diff --git a/crates/coding-agent-loop/src/session.rs b/crates/coding-agent-loop/src/session.rs index 980ec72e2..2a20402da 100644 --- a/crates/coding-agent-loop/src/session.rs +++ b/crates/coding-agent-loop/src/session.rs @@ -678,10 +678,9 @@ mod tests { use super::*; use crate::test_support::*; use crate::tool_registry::{RegisteredTool, ToolRegistry}; - use async_trait::async_trait; use unified_llm::error::ProviderErrorDetail; - use unified_llm::provider::{ProviderAdapter, StreamEventStream}; - use unified_llm::types::{Response, ToolDefinition}; + use unified_llm::provider::ProviderAdapter; + use unified_llm::types::ToolDefinition; // --- Tests --- @@ -1142,7 +1141,7 @@ mod tests { let provider = Arc::new(MockLlmProvider::new(responses)); let client = make_client(provider).await; - let profile = Arc::new(ParallelTestProfile::with_tools(registry)); + let profile = Arc::new(TestProfile::parallel(registry)); let env = Arc::new(MockExecutionEnvironment::default()); let mut session = Session::new(client, profile, env, SessionConfig::default()); let mut rx = session.subscribe(); @@ -1193,7 +1192,7 @@ mod tests { let provider = Arc::new(MockLlmProvider::new(responses)); let client = make_client(provider).await; let registry = ToolRegistry::new(); - let profile = Arc::new(ParallelTestProfile::with_tools_and_context_window( + let profile = Arc::new(TestProfile::parallel_with_context_window( registry, 100, )); let env = Arc::new(MockExecutionEnvironment::default()); @@ -1220,44 +1219,11 @@ mod tests { assert!(found_warning); } - // --- Capturing LLM Provider (captures reasoning_effort from request) --- - - struct CapturingLlmProvider { - captured_effort: Arc>>>, - } - - impl CapturingLlmProvider { - fn new(captured_effort: Arc>>>) -> Self { - Self { captured_effort } - } - } - - #[async_trait] - impl ProviderAdapter for CapturingLlmProvider { - fn name(&self) -> &str { - "mock" - } - - async fn complete(&self, request: &Request) -> Result { - *self.captured_effort.lock().unwrap() = Some(request.reasoning_effort.clone()); - Ok(text_response("captured")) - } - - async fn stream( - &self, - _request: &Request, - ) -> Result { - Err(SdkError::Configuration { - message: "streaming not supported in mock".into(), - }) - } - } - #[tokio::test] async fn set_reasoning_effort_mid_session() { - let captured_effort: Arc>>> = Arc::new(Mutex::new(None)); - let provider = Arc::new(CapturingLlmProvider::new(captured_effort.clone())); - let client = make_client(provider).await; + let provider = Arc::new(CapturingLlmProvider::new()); + let provider_ref = provider.clone(); + let client = make_client(provider as Arc).await; let profile = Arc::new(TestProfile::new()); let env = Arc::new(MockExecutionEnvironment::default()); let mut session = Session::new(client, profile, env, SessionConfig::default()); @@ -1266,8 +1232,9 @@ mod tests { session.set_reasoning_effort(Some("high".to_string())); session.process_input("test").await.unwrap(); - let effort = captured_effort.lock().unwrap().clone(); - assert_eq!(effort, Some(Some("high".to_string()))); + let captured = provider_ref.captured_request.lock().unwrap(); + let request = captured.as_ref().expect("request should have been captured"); + assert_eq!(request.reasoning_effort, Some("high".to_string())); } #[tokio::test] @@ -1278,7 +1245,7 @@ mod tests { let client = make_client(provider).await; let registry = ToolRegistry::new(); // Large context window so short input stays well under 80% - let profile = Arc::new(ParallelTestProfile::with_tools_and_context_window( + let profile = Arc::new(TestProfile::parallel_with_context_window( registry, 200_000, )); let env = Arc::new(MockExecutionEnvironment::default()); @@ -1404,37 +1371,9 @@ mod tests { #[tokio::test] async fn user_instructions_in_system_prompt() { - // Use a capturing provider that records the system prompt - let captured_messages: Arc>>> = Arc::new(Mutex::new(None)); - let captured_messages_clone = captured_messages.clone(); - - struct CapturingProvider { - captured: Arc>>>, - } - - #[async_trait] - impl ProviderAdapter for CapturingProvider { - fn name(&self) -> &str { - "mock" - } - async fn complete(&self, request: &Request) -> Result { - *self.captured.lock().unwrap() = Some(request.messages.clone()); - Ok(text_response("ok")) - } - async fn stream( - &self, - _request: &Request, - ) -> Result { - Err(SdkError::Configuration { - message: "not supported".into(), - }) - } - } - - let provider = Arc::new(CapturingProvider { - captured: captured_messages_clone, - }); - let client = make_client(provider).await; + let provider = Arc::new(CapturingLlmProvider::new()); + let provider_ref = provider.clone(); + let client = make_client(provider as Arc).await; let profile = Arc::new(TestProfile::new()); let env = Arc::new(MockExecutionEnvironment::default()); let config = SessionConfig { @@ -1444,11 +1383,14 @@ mod tests { let mut session = Session::new(client, profile, env, config); session.process_input("test").await.unwrap(); - // The test profile doesn't include user_instructions in prompt (it's stubbed), - // but we verify the config is wired through by checking the session accepted it - assert_eq!( - session.config.user_instructions, - Some("Always use TDD".into()) + // Verify user instructions are included in the system prompt + let captured = provider_ref.captured_request.lock().unwrap(); + let request = captured.as_ref().expect("request should have been captured"); + let system_msg = &request.messages[0]; + let system_text = system_msg.text(); + assert!( + system_text.contains("Always use TDD"), + "System prompt should contain user instructions" ); } } diff --git a/crates/coding-agent-loop/src/test_support.rs b/crates/coding-agent-loop/src/test_support.rs index 95678f22e..ab0fa0b96 100644 --- a/crates/coding-agent-loop/src/test_support.rs +++ b/crates/coding-agent-loop/src/test_support.rs @@ -7,7 +7,7 @@ use crate::tool_registry::ToolRegistry; use async_trait::async_trait; use std::collections::HashMap; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use unified_llm::client::Client; use unified_llm::error::SdkError; use unified_llm::provider::{ProviderAdapter, StreamEventStream}; @@ -23,6 +23,23 @@ pub(crate) struct MockExecutionEnvironment { pub working_dir: &'static str, pub platform_str: &'static str, pub os_version_str: String, + /// When true, read_file applies offset/limit by splitting on lines. + pub apply_read_offset_limit: bool, + /// Captures (path, content) pairs from write_file calls. + pub written_files: Mutex>, + /// Captures the timeout_ms argument from exec_command calls. + pub captured_timeout: Mutex>, +} + +impl MockExecutionEnvironment { + pub fn linux() -> Self { + Self { + working_dir: "/home/test", + platform_str: "linux", + os_version_str: "Linux 6.1.0".into(), + ..Default::default() + } + } } impl Default for MockExecutionEnvironment { @@ -41,6 +58,9 @@ impl Default for MockExecutionEnvironment { working_dir: "/tmp/test", platform_str: "darwin", os_version_str: "Darwin 24.0.0".into(), + apply_read_offset_limit: false, + written_files: Mutex::new(Vec::new()), + captured_timeout: Mutex::new(None), } } } @@ -50,16 +70,31 @@ impl ExecutionEnvironment for MockExecutionEnvironment { async fn read_file( &self, path: &str, - _offset: Option, - _limit: Option, + offset: Option, + limit: Option, ) -> Result { - self.files + let content = self + .files .get(path) .cloned() - .ok_or_else(|| format!("File not found: {path}")) + .ok_or_else(|| format!("File not found: {path}"))?; + + if self.apply_read_offset_limit { + let lines: Vec<&str> = content.lines().collect(); + let start = offset.unwrap_or(1).saturating_sub(1); + let count = limit.unwrap_or(2000); + let selected: Vec<&str> = lines.into_iter().skip(start).take(count).collect(); + Ok(selected.join("\n")) + } else { + Ok(content) + } } - async fn write_file(&self, _path: &str, _content: &str) -> Result<(), String> { + async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { + self.written_files + .lock() + .expect("written_files lock poisoned") + .push((path.to_string(), content.to_string())); Ok(()) } @@ -82,10 +117,14 @@ impl ExecutionEnvironment for MockExecutionEnvironment { async fn exec_command( &self, _command: &str, - _timeout_ms: u64, + timeout_ms: u64, _working_dir: Option<&str>, _env_vars: Option<&std::collections::HashMap>, ) -> Result { + *self + .captured_timeout + .lock() + .expect("captured_timeout lock poisoned") = Some(timeout_ms); Ok(self.exec_result.clone()) } @@ -123,21 +162,159 @@ impl ExecutionEnvironment for MockExecutionEnvironment { } } +// --- MutableMockExecutionEnvironment --- + +/// A mock execution environment with Mutex-protected files for tests that need +/// write operations to be visible to subsequent reads (e.g., apply_patch tests). +pub(crate) struct MutableMockExecutionEnvironment { + pub files: Mutex>, +} + +impl MutableMockExecutionEnvironment { + pub fn new(files: HashMap) -> Self { + Self { + files: Mutex::new(files), + } + } +} + +#[async_trait] +impl ExecutionEnvironment for MutableMockExecutionEnvironment { + async fn read_file( + &self, + path: &str, + _offset: Option, + _limit: Option, + ) -> Result { + self.files + .lock() + .expect("files lock poisoned") + .get(path) + .cloned() + .ok_or_else(|| format!("File not found: {path}")) + } + + async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { + self.files + .lock() + .expect("files lock poisoned") + .insert(path.to_string(), content.to_string()); + Ok(()) + } + + async fn delete_file(&self, path: &str) -> Result<(), String> { + self.files + .lock() + .expect("files lock poisoned") + .remove(path); + Ok(()) + } + + async fn file_exists(&self, path: &str) -> Result { + Ok(self + .files + .lock() + .expect("files lock poisoned") + .contains_key(path)) + } + + async fn list_directory( + &self, + _path: &str, + _depth: Option, + ) -> Result, String> { + Ok(vec![]) + } + + async fn exec_command( + &self, + _command: &str, + _timeout_ms: u64, + _working_dir: Option<&str>, + _env_vars: Option<&std::collections::HashMap>, + ) -> Result { + Ok(ExecResult { + stdout: String::new(), + stderr: String::new(), + exit_code: 0, + timed_out: false, + duration_ms: 0, + }) + } + + async fn grep( + &self, + _pattern: &str, + _path: &str, + _options: &GrepOptions, + ) -> Result, String> { + Ok(vec![]) + } + + async fn glob(&self, _pattern: &str, _path: Option<&str>) -> Result, String> { + Ok(vec![]) + } + + async fn initialize(&self) -> Result<(), String> { + Ok(()) + } + + async fn cleanup(&self) -> Result<(), String> { + Ok(()) + } + + fn working_directory(&self) -> &str { + "/tmp" + } + + fn platform(&self) -> &str { + "linux" + } + + fn os_version(&self) -> String { + "Linux 6.1.0".into() + } +} + // --- TestProfile --- pub(crate) struct TestProfile { pub registry: ToolRegistry, + pub parallel_tool_calls: bool, + pub context_window: usize, } impl TestProfile { pub fn new() -> Self { Self { registry: ToolRegistry::new(), + parallel_tool_calls: false, + context_window: 200_000, } } pub fn with_tools(registry: ToolRegistry) -> Self { - Self { registry } + Self { + registry, + parallel_tool_calls: false, + context_window: 200_000, + } + } + + pub fn parallel(registry: ToolRegistry) -> Self { + Self { + registry, + parallel_tool_calls: true, + context_window: 200_000, + } + } + + pub fn parallel_with_context_window(registry: ToolRegistry, context_window: usize) -> Self { + Self { + registry, + parallel_tool_calls: true, + context_window, + } } } @@ -163,17 +340,20 @@ impl ProviderProfile for TestProfile { _env: &dyn ExecutionEnvironment, _env_context: &EnvContext, _project_docs: &[String], - _user_instructions: Option<&str>, + user_instructions: Option<&str>, ) -> String { - "You are a test assistant.".into() + match user_instructions { + Some(instructions) => format!("You are a test assistant.\n\n# User Instructions\n{instructions}"), + None => "You are a test assistant.".into(), + } } fn capabilities(&self) -> ProfileCapabilities { ProfileCapabilities { supports_reasoning: false, supports_streaming: false, - supports_parallel_tool_calls: false, - context_window_size: 200_000, + supports_parallel_tool_calls: self.parallel_tool_calls, + context_window_size: self.context_window, } } @@ -355,70 +535,6 @@ pub(crate) fn make_error_tool() -> crate::tool_registry::RegisteredTool { } } -// --- ParallelTestProfile --- - -pub(crate) struct ParallelTestProfile { - pub registry: ToolRegistry, - pub context_window: usize, -} - -impl ParallelTestProfile { - pub fn with_tools(registry: ToolRegistry) -> Self { - Self { - registry, - context_window: 200_000, - } - } - - pub fn with_tools_and_context_window(registry: ToolRegistry, context_window: usize) -> Self { - Self { - registry, - context_window, - } - } -} - -impl ProviderProfile for ParallelTestProfile { - fn id(&self) -> String { - "mock".into() - } - - fn model(&self) -> String { - "mock-model".into() - } - - fn tool_registry(&self) -> &ToolRegistry { - &self.registry - } - - fn tool_registry_mut(&mut self) -> &mut ToolRegistry { - &mut self.registry - } - - fn build_system_prompt( - &self, - _env: &dyn ExecutionEnvironment, - _env_context: &EnvContext, - _project_docs: &[String], - _user_instructions: Option<&str>, - ) -> String { - "You are a test assistant.".into() - } - - fn capabilities(&self) -> ProfileCapabilities { - ProfileCapabilities { - supports_reasoning: false, - supports_streaming: false, - supports_parallel_tool_calls: true, - context_window_size: self.context_window, - } - } - - fn knowledge_cutoff(&self) -> &str { - "May 2025" - } -} - // --- MockErrorProvider --- pub(crate) struct MockErrorProvider { @@ -442,6 +558,42 @@ impl ProviderAdapter for MockErrorProvider { } } +// --- CapturingLlmProvider --- + +/// A mock LLM provider that captures the full Request for test assertions. +pub(crate) struct CapturingLlmProvider { + pub captured_request: Mutex>, +} + +impl CapturingLlmProvider { + pub fn new() -> Self { + Self { + captured_request: Mutex::new(None), + } + } +} + +#[async_trait] +impl ProviderAdapter for CapturingLlmProvider { + fn name(&self) -> &str { + "mock" + } + + async fn complete(&self, request: &Request) -> Result { + *self + .captured_request + .lock() + .expect("captured_request lock poisoned") = Some(request.clone()); + Ok(text_response("captured")) + } + + async fn stream(&self, _request: &Request) -> Result { + Err(SdkError::Configuration { + message: "streaming not supported in mock".into(), + }) + } +} + pub(crate) fn multi_tool_call_response( calls: Vec<(&str, &str, serde_json::Value)>, ) -> Response { diff --git a/crates/coding-agent-loop/src/tools.rs b/crates/coding-agent-loop/src/tools.rs index 405a7c65c..d8ea7a744 100644 --- a/crates/coding-agent-loop/src/tools.rs +++ b/crates/coding-agent-loop/src/tools.rs @@ -412,243 +412,17 @@ mod tests { use super::*; use crate::execution_env::*; use crate::test_support::MockExecutionEnvironment; - use async_trait::async_trait; - use std::sync::Mutex; - - /// A specialized mock that applies offset/limit to file content (for read_file tool tests). - struct ReadFileEnv { - content: String, - } - - #[async_trait] - impl ExecutionEnvironment for ReadFileEnv { - async fn read_file(&self, _path: &str, offset: Option, limit: Option) -> Result { - let lines: Vec<&str> = self.content.lines().collect(); - let start = offset.unwrap_or(1).saturating_sub(1); - let count = limit.unwrap_or(2000); - let selected: Vec<&str> = lines.into_iter().skip(start).take(count).collect(); - Ok(selected.join("\n")) - } - async fn write_file(&self, _: &str, _: &str) -> Result<(), String> { - Ok(()) - } - async fn delete_file(&self, _: &str) -> Result<(), String> { - Ok(()) - } - async fn file_exists(&self, _: &str) -> Result { - Ok(false) - } - async fn list_directory(&self, _: &str, _depth: Option) -> Result, String> { - Ok(vec![]) - } - async fn exec_command(&self, _: &str, _: u64, _: Option<&str>, _: Option<&std::collections::HashMap>) -> Result { - Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 0, - timed_out: false, - duration_ms: 0, - }) - } - async fn grep(&self, _: &str, _: &str, _: &GrepOptions) -> Result, String> { - Ok(vec![]) - } - async fn glob(&self, _: &str, _path: Option<&str>) -> Result, String> { - Ok(vec![]) - } - async fn initialize(&self) -> Result<(), String> { - Ok(()) - } - async fn cleanup(&self) -> Result<(), String> { - Ok(()) - } - fn working_directory(&self) -> &str { - "/tmp" - } - fn platform(&self) -> &str { - "darwin" - } - fn os_version(&self) -> String { - String::new() - } - } - - struct WriteFileEnv { - written: Mutex>, - } - - #[async_trait] - impl ExecutionEnvironment for WriteFileEnv { - async fn read_file(&self, _path: &str, _offset: Option, _limit: Option) -> Result { - Ok(String::new()) - } - async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { - *self.written.lock().unwrap() = Some((path.into(), content.into())); - Ok(()) - } - async fn delete_file(&self, _: &str) -> Result<(), String> { - Ok(()) - } - async fn file_exists(&self, _: &str) -> Result { - Ok(false) - } - async fn list_directory(&self, _: &str, _depth: Option) -> Result, String> { - Ok(vec![]) - } - async fn exec_command(&self, _: &str, _: u64, _: Option<&str>, _: Option<&std::collections::HashMap>) -> Result { - Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 0, - timed_out: false, - duration_ms: 0, - }) - } - async fn grep(&self, _: &str, _: &str, _: &GrepOptions) -> Result, String> { - Ok(vec![]) - } - async fn glob(&self, _: &str, _path: Option<&str>) -> Result, String> { - Ok(vec![]) - } - async fn initialize(&self) -> Result<(), String> { - Ok(()) - } - async fn cleanup(&self) -> Result<(), String> { - Ok(()) - } - fn working_directory(&self) -> &str { - "/tmp" - } - fn platform(&self) -> &str { - "darwin" - } - fn os_version(&self) -> String { - String::new() - } - } - - struct EditFileEnv { - content: String, - written: Mutex>, - } - - #[async_trait] - impl ExecutionEnvironment for EditFileEnv { - async fn read_file(&self, _path: &str, _offset: Option, _limit: Option) -> Result { - Ok(self.content.clone()) - } - async fn write_file(&self, _path: &str, content: &str) -> Result<(), String> { - *self.written.lock().unwrap() = Some(content.into()); - Ok(()) - } - async fn delete_file(&self, _: &str) -> Result<(), String> { - Ok(()) - } - async fn file_exists(&self, _: &str) -> Result { - Ok(false) - } - async fn list_directory(&self, _: &str, _depth: Option) -> Result, String> { - Ok(vec![]) - } - async fn exec_command(&self, _: &str, _: u64, _: Option<&str>, _: Option<&std::collections::HashMap>) -> Result { - Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 0, - timed_out: false, - duration_ms: 0, - }) - } - async fn grep(&self, _: &str, _: &str, _: &GrepOptions) -> Result, String> { - Ok(vec![]) - } - async fn glob(&self, _: &str, _path: Option<&str>) -> Result, String> { - Ok(vec![]) - } - async fn initialize(&self) -> Result<(), String> { - Ok(()) - } - async fn cleanup(&self) -> Result<(), String> { - Ok(()) - } - fn working_directory(&self) -> &str { - "/tmp" - } - fn platform(&self) -> &str { - "darwin" - } - fn os_version(&self) -> String { - String::new() - } - } - - - struct ShellCapturingEnv { - captured_timeout: Mutex>, - } - - #[async_trait] - impl ExecutionEnvironment for ShellCapturingEnv { - async fn read_file(&self, _: &str, _offset: Option, _limit: Option) -> Result { - Ok(String::new()) - } - async fn write_file(&self, _: &str, _: &str) -> Result<(), String> { - Ok(()) - } - async fn delete_file(&self, _: &str) -> Result<(), String> { - Ok(()) - } - async fn file_exists(&self, _: &str) -> Result { - Ok(false) - } - async fn list_directory(&self, _: &str, _depth: Option) -> Result, String> { - Ok(vec![]) - } - async fn exec_command( - &self, - _: &str, - timeout_ms: u64, - _: Option<&str>, - _: Option<&std::collections::HashMap>, - ) -> Result { - *self.captured_timeout.lock().unwrap() = Some(timeout_ms); - Ok(ExecResult { - stdout: String::new(), - stderr: String::new(), - exit_code: 0, - timed_out: false, - duration_ms: 0, - }) - } - async fn grep(&self, _: &str, _: &str, _: &GrepOptions) -> Result, String> { - Ok(vec![]) - } - async fn glob(&self, _: &str, _path: Option<&str>) -> Result, String> { - Ok(vec![]) - } - async fn initialize(&self) -> Result<(), String> { - Ok(()) - } - async fn cleanup(&self) -> Result<(), String> { - Ok(()) - } - fn working_directory(&self) -> &str { - "/tmp" - } - fn platform(&self) -> &str { - "darwin" - } - fn os_version(&self) -> String { - String::new() - } - } - + use std::collections::HashMap; #[tokio::test] async fn read_file_returns_content() { let tool = make_read_file_tool(); - let env: Arc = Arc::new(ReadFileEnv { - content: " 1 | hello\n 2 | world".into(), + let mut files = HashMap::new(); + files.insert("/test.txt".into(), " 1 | hello\n 2 | world".into()); + let env: Arc = Arc::new(MockExecutionEnvironment { + files, + apply_read_offset_limit: true, + ..Default::default() }); let result = (tool.executor)(serde_json::json!({"file_path": "/test.txt"}), env).await; assert_eq!(result.unwrap(), " 1 | hello\n 2 | world"); @@ -657,8 +431,15 @@ mod tests { #[tokio::test] async fn read_file_with_offset_and_limit() { let tool = make_read_file_tool(); - let env: Arc = Arc::new(ReadFileEnv { - content: " 1 | line1\n 2 | line2\n 3 | line3\n 4 | line4".into(), + let mut files = HashMap::new(); + files.insert( + "/test.txt".into(), + " 1 | line1\n 2 | line2\n 3 | line3\n 4 | line4".into(), + ); + let env: Arc = Arc::new(MockExecutionEnvironment { + files, + apply_read_offset_limit: true, + ..Default::default() }); let result = (tool.executor)( serde_json::json!({"file_path": "/test.txt", "offset": 2, "limit": 2}), @@ -671,9 +452,7 @@ mod tests { #[tokio::test] async fn write_file_calls_env() { let tool = make_write_file_tool(); - let env = Arc::new(WriteFileEnv { - written: Mutex::new(None), - }); + let env = Arc::new(MockExecutionEnvironment::default()); let env_clone: Arc = env.clone(); let result = (tool.executor)( serde_json::json!({"file_path": "/out.txt", "content": "hello"}), @@ -681,18 +460,20 @@ mod tests { ) .await; assert_eq!(result.unwrap(), "Successfully wrote to /out.txt"); - let written = env.written.lock().unwrap(); - let (path, content) = written.as_ref().unwrap(); - assert_eq!(path, "/out.txt"); - assert_eq!(content, "hello"); + let written = env.written_files.lock().unwrap(); + assert_eq!(written.len(), 1); + assert_eq!(written[0].0, "/out.txt"); + assert_eq!(written[0].1, "hello"); } #[tokio::test] async fn edit_file_replaces_match() { let tool = make_edit_file_tool(); - let env = Arc::new(EditFileEnv { - content: " 1 | hello world".into(), - written: Mutex::new(None), + let mut files = HashMap::new(); + files.insert("/f.txt".into(), " 1 | hello world".into()); + let env = Arc::new(MockExecutionEnvironment { + files, + ..Default::default() }); let env_clone: Arc = env.clone(); let result = (tool.executor)( @@ -705,16 +486,19 @@ mod tests { ) .await; assert_eq!(result.unwrap(), "Successfully edited /f.txt"); - let written = env.written.lock().unwrap(); - assert_eq!(written.as_ref().unwrap(), "goodbye world"); + let written = env.written_files.lock().unwrap(); + assert_eq!(written.len(), 1); + assert_eq!(written[0].1, "goodbye world"); } #[tokio::test] async fn edit_file_not_found_error() { let tool = make_edit_file_tool(); - let env: Arc = Arc::new(EditFileEnv { - content: " 1 | hello world".into(), - written: Mutex::new(None), + let mut files = HashMap::new(); + files.insert("/f.txt".into(), " 1 | hello world".into()); + let env: Arc = Arc::new(MockExecutionEnvironment { + files, + ..Default::default() }); let result = (tool.executor)( serde_json::json!({ @@ -731,9 +515,11 @@ mod tests { #[tokio::test] async fn edit_file_not_unique_error() { let tool = make_edit_file_tool(); - let env: Arc = Arc::new(EditFileEnv { - content: " 1 | aa bb aa".into(), - written: Mutex::new(None), + let mut files = HashMap::new(); + files.insert("/f.txt".into(), " 1 | aa bb aa".into()); + let env: Arc = Arc::new(MockExecutionEnvironment { + files, + ..Default::default() }); let result = (tool.executor)( serde_json::json!({ @@ -752,9 +538,11 @@ mod tests { #[tokio::test] async fn edit_file_replace_all() { let tool = make_edit_file_tool(); - let env = Arc::new(EditFileEnv { - content: " 1 | aa bb aa".into(), - written: Mutex::new(None), + let mut files = HashMap::new(); + files.insert("/f.txt".into(), " 1 | aa bb aa".into()); + let env = Arc::new(MockExecutionEnvironment { + files, + ..Default::default() }); let env_clone: Arc = env.clone(); let result = (tool.executor)( @@ -768,8 +556,9 @@ mod tests { ) .await; assert_eq!(result.unwrap(), "Successfully edited /f.txt"); - let written = env.written.lock().unwrap(); - assert_eq!(written.as_ref().unwrap(), "cc bb cc"); + let written = env.written_files.lock().unwrap(); + assert_eq!(written.len(), 1); + assert_eq!(written[0].1, "cc bb cc"); } #[tokio::test] @@ -794,9 +583,7 @@ mod tests { #[tokio::test] async fn shell_with_timeout() { let tool = make_shell_tool(); - let env = Arc::new(ShellCapturingEnv { - captured_timeout: Mutex::new(None), - }); + let env = Arc::new(MockExecutionEnvironment::default()); let env_clone: Arc = env.clone(); let _result = (tool.executor)( serde_json::json!({"command": "sleep 1", "timeout_ms": 5000}),