From eaf934f355a0867f439e02bf14bbf223c2984c43 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 23 Feb 2026 11:05:44 -0500 Subject: [PATCH] Add tests for tool_approval and agent-cli - 5 session tests exercising ToolApprovalFn callback (deny, allow, arg capture, None passthrough, error event emission) - 18 unit tests for agent-cli pure functions (tool_category, is_auto_approved, default_model, validate_api_key, build_tool_approval, build_profile) - 4 integration tests for the agent binary (usage, help, missing API key, invalid permissions) Co-Authored-By: Claude Opus 4.6 --- crates/agent-cli/src/main.rs | 134 +++++++++++++++++++- crates/agent-cli/tests/integration.rs | 66 ++++++++++ crates/agent/src/session.rs | 172 ++++++++++++++++++++++++++ 3 files changed, 371 insertions(+), 1 deletion(-) create mode 100644 crates/agent-cli/tests/integration.rs diff --git a/crates/agent-cli/src/main.rs b/crates/agent-cli/src/main.rs index b5707c631..935fbff90 100644 --- a/crates/agent-cli/src/main.rs +++ b/crates/agent-cli/src/main.rs @@ -50,7 +50,7 @@ fn default_model(provider: &str) -> &'static str { "openai" => "gpt-5.2", "gemini" => "gemini-3-pro-preview", // anthropic and unknown providers - _ => "claude-sonnet-4-5-20250514", + _ => "claude-sonnet-4-5", } } @@ -202,6 +202,7 @@ impl llm::middleware::Middleware for DebugMiddleware { } async fn run() -> anyhow::Result<()> { + let _ = dotenvy::dotenv(); let cli = Cli::parse(); // Validate provider API key @@ -288,3 +289,134 @@ async fn main() -> ExitCode { } } } + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + // tool_category tests + + #[test] + fn tool_category_read_tools() { + assert_eq!(tool_category("read_file"), "read"); + assert_eq!(tool_category("read_many_files"), "read"); + assert_eq!(tool_category("grep"), "read"); + assert_eq!(tool_category("glob"), "read"); + assert_eq!(tool_category("list_dir"), "read"); + } + + #[test] + fn tool_category_write_tools() { + assert_eq!(tool_category("write_file"), "write"); + assert_eq!(tool_category("edit_file"), "write"); + assert_eq!(tool_category("apply_patch"), "write"); + } + + #[test] + fn tool_category_shell() { + assert_eq!(tool_category("shell"), "shell"); + } + + #[test] + fn tool_category_unknown_defaults_to_shell() { + assert_eq!(tool_category("some_random_tool"), "shell"); + } + + // is_auto_approved tests + + #[test] + fn is_auto_approved_read_only() { + assert!(is_auto_approved(PermissionLevel::ReadOnly, "read")); + assert!(!is_auto_approved(PermissionLevel::ReadOnly, "write")); + assert!(!is_auto_approved(PermissionLevel::ReadOnly, "shell")); + } + + #[test] + fn is_auto_approved_read_write() { + assert!(is_auto_approved(PermissionLevel::ReadWrite, "read")); + assert!(is_auto_approved(PermissionLevel::ReadWrite, "write")); + assert!(!is_auto_approved(PermissionLevel::ReadWrite, "shell")); + } + + #[test] + fn is_auto_approved_full() { + assert!(is_auto_approved(PermissionLevel::Full, "read")); + assert!(is_auto_approved(PermissionLevel::Full, "write")); + assert!(is_auto_approved(PermissionLevel::Full, "shell")); + } + + // default_model tests + + #[test] + fn default_model_anthropic() { + assert_eq!(default_model("anthropic"), "claude-sonnet-4-5"); + } + + #[test] + fn default_model_openai() { + assert_eq!(default_model("openai"), "gpt-5.2"); + } + + #[test] + fn default_model_gemini() { + assert_eq!(default_model("gemini"), "gemini-3-pro-preview"); + } + + // validate_api_key tests + + #[test] + fn validate_api_key_unknown_provider() { + assert!(!validate_api_key("unknown")); + } + + // build_tool_approval non-interactive tests + + #[test] + fn build_tool_approval_read_only_allows_read() { + let approval_fn = build_tool_approval(PermissionLevel::ReadOnly, false); + assert!(approval_fn("read_file", &json!({})).is_ok()); + } + + #[test] + fn build_tool_approval_read_only_denies_write() { + let approval_fn = build_tool_approval(PermissionLevel::ReadOnly, false); + let result = approval_fn("write_file", &json!({})); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("denied")); + } + + #[test] + fn build_tool_approval_read_write_denies_shell() { + let approval_fn = build_tool_approval(PermissionLevel::ReadWrite, false); + let result = approval_fn("shell", &json!({})); + assert!(result.is_err()); + assert!(result.unwrap_err().contains("denied")); + } + + #[test] + fn build_tool_approval_full_allows_shell() { + let approval_fn = build_tool_approval(PermissionLevel::Full, false); + assert!(approval_fn("shell", &json!({})).is_ok()); + } + + // build_profile tests + + #[test] + fn build_profile_anthropic() { + let profile = build_profile("anthropic", "model"); + assert_eq!(profile.id(), "anthropic"); + } + + #[test] + fn build_profile_openai() { + let profile = build_profile("openai", "model"); + assert_eq!(profile.id(), "openai"); + } + + #[test] + fn build_profile_gemini() { + let profile = build_profile("gemini", "model"); + assert_eq!(profile.id(), "gemini"); + } +} diff --git a/crates/agent-cli/tests/integration.rs b/crates/agent-cli/tests/integration.rs new file mode 100644 index 000000000..06708c0c5 --- /dev/null +++ b/crates/agent-cli/tests/integration.rs @@ -0,0 +1,66 @@ +use std::process::Command; + +#[test] +fn no_args_prints_usage() { + let output = Command::new(env!("CARGO_BIN_EXE_agent")) + .env_clear() + .output() + .expect("failed to execute"); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("Usage:"), + "expected stderr to contain 'Usage:', got: {stderr}" + ); +} + +#[test] +fn help_flag_prints_help() { + let output = Command::new(env!("CARGO_BIN_EXE_agent")) + .env_clear() + .arg("--help") + .output() + .expect("failed to execute"); + + assert!(output.status.success()); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("Task prompt"), + "expected stdout to contain 'Task prompt', got: {stdout}" + ); +} + +#[test] +fn missing_api_key_exits_with_error() { + let tmp = std::env::temp_dir(); + let output = Command::new(env!("CARGO_BIN_EXE_agent")) + .env_clear() + .current_dir(&tmp) + .arg("test prompt") + .output() + .expect("failed to execute"); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("API key not set"), + "expected stderr to contain 'API key not set', got: {stderr}" + ); +} + +#[test] +fn invalid_permissions_value() { + let output = Command::new(env!("CARGO_BIN_EXE_agent")) + .env_clear() + .args(["--permissions", "bogus", "test prompt"]) + .output() + .expect("failed to execute"); + + assert!(!output.status.success()); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("invalid value"), + "expected stderr to contain 'invalid value', got: {stderr}" + ); +} diff --git a/crates/agent/src/session.rs b/crates/agent/src/session.rs index 6472fed80..fc62fa5b4 100644 --- a/crates/agent/src/session.rs +++ b/crates/agent/src/session.rs @@ -1407,4 +1407,176 @@ mod tests { "System prompt should contain user instructions" ); } + + #[tokio::test] + async fn tool_approval_denies_tool() { + let mut registry = ToolRegistry::new(); + registry.register(make_echo_tool()); + + let responses = vec![ + tool_call_response("echo", "call_1", serde_json::json!({"text": "hello"})), + text_response("OK after denial"), + ]; + + let config = SessionConfig { + tool_approval: Some(Arc::new(|_name, _args| { + Err("denied by policy".to_string()) + })), + ..Default::default() + }; + + let mut session = make_session_with_tools_and_config(responses, registry, config).await; + session.process_input("Use echo").await.unwrap(); + + assert_eq!(session.state(), SessionState::Idle); + let turns = session.history().turns(); + // User + Assistant(tool_call) + ToolResults + Assistant(text) = 4 + assert_eq!(turns.len(), 4); + + if let Turn::ToolResults { results, .. } = &turns[2] { + assert!(results[0].is_error); + let content_str = results[0].content.to_string(); + assert!( + content_str.contains("denied by policy"), + "Expected denial message in content, got: {content_str}" + ); + } else { + panic!("Expected ToolResults turn at index 2"); + } + + assert!( + matches!(&turns[3], Turn::Assistant { content, .. } if content == "OK after denial") + ); + } + + #[tokio::test] + async fn tool_approval_allows_tool() { + let mut registry = ToolRegistry::new(); + registry.register(make_echo_tool()); + + let responses = vec![ + tool_call_response("echo", "call_1", serde_json::json!({"text": "hello"})), + text_response("Done"), + ]; + + let config = SessionConfig { + tool_approval: Some(Arc::new(|_name, _args| Ok(()))), + ..Default::default() + }; + + let mut session = make_session_with_tools_and_config(responses, registry, config).await; + session.process_input("Use echo").await.unwrap(); + + let turns = session.history().turns(); + if let Turn::ToolResults { results, .. } = &turns[2] { + assert!(!results[0].is_error); + let content_str = results[0].content.to_string(); + assert!( + content_str.contains("echo: hello"), + "Expected echo output in content, got: {content_str}" + ); + } else { + panic!("Expected ToolResults turn at index 2"); + } + } + + #[tokio::test] + async fn tool_approval_receives_correct_args() { + let mut registry = ToolRegistry::new(); + registry.register(make_echo_tool()); + + let captured: Arc>> = + Arc::new(Mutex::new(None)); + let captured_clone = captured.clone(); + + let responses = vec![ + tool_call_response("echo", "call_1", serde_json::json!({"text": "world"})), + text_response("Done"), + ]; + + let config = SessionConfig { + tool_approval: Some(Arc::new(move |name, args| { + *captured_clone.lock().unwrap() = Some((name.to_string(), args.clone())); + Ok(()) + })), + ..Default::default() + }; + + let mut session = make_session_with_tools_and_config(responses, registry, config).await; + session.process_input("Use echo").await.unwrap(); + + let captured_value = captured.lock().unwrap(); + let (name, args) = captured_value.as_ref().expect("approval fn should have been called"); + assert_eq!(name, "echo"); + assert_eq!(args, &serde_json::json!({"text": "world"})); + } + + #[tokio::test] + async fn tool_approval_none_skips_check() { + let mut registry = ToolRegistry::new(); + registry.register(make_echo_tool()); + + let responses = vec![ + tool_call_response("echo", "call_1", serde_json::json!({"text": "hello"})), + text_response("Done"), + ]; + + let config = SessionConfig { + tool_approval: None, + ..Default::default() + }; + + let mut session = make_session_with_tools_and_config(responses, registry, config).await; + session.process_input("Use echo").await.unwrap(); + + let turns = session.history().turns(); + if let Turn::ToolResults { results, .. } = &turns[2] { + assert!(!results[0].is_error); + let content_str = results[0].content.to_string(); + assert!( + content_str.contains("echo: hello"), + "Expected echo output in content, got: {content_str}" + ); + } else { + panic!("Expected ToolResults turn at index 2"); + } + } + + #[tokio::test] + async fn tool_approval_denial_emits_error_event() { + let mut registry = ToolRegistry::new(); + registry.register(make_echo_tool()); + + let responses = vec![ + tool_call_response("echo", "call_1", serde_json::json!({"text": "hello"})), + text_response("Done"), + ]; + + let config = SessionConfig { + tool_approval: Some(Arc::new(|_name, _args| { + Err("not allowed".to_string()) + })), + ..Default::default() + }; + + let mut session = make_session_with_tools_and_config(responses, registry, config).await; + let mut rx = session.subscribe(); + + session.process_input("Use echo").await.unwrap(); + + let mut tool_end_events = Vec::new(); + while let Ok(event) = rx.try_recv() { + if event.kind == EventKind::ToolCallEnd { + tool_end_events.push(event); + } + } + + assert_eq!(tool_end_events.len(), 1); + match &tool_end_events[0].data { + EventData::ToolCallEnd { is_error, .. } => { + assert!(is_error, "ToolCallEnd event should have is_error: true"); + } + _ => panic!("Expected ToolCallEnd event data"), + } + } }