diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index 3f1db9e2d9..0e71dcae31 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -192,7 +192,7 @@ response: let url = result.unwrap(); assert!(url.starts_with("goose://recipe?config=")); let encoded_part = url.strip_prefix("goose://recipe?config=").unwrap(); - assert!(encoded_part.len() > 0); + assert!(!encoded_part.is_empty()); } #[test] diff --git a/crates/goose-cli/src/logging.rs b/crates/goose-cli/src/logging.rs index 974c0a6c32..265a84f612 100644 --- a/crates/goose-cli/src/logging.rs +++ b/crates/goose-cli/src/logging.rs @@ -248,7 +248,7 @@ mod tests { ); // Create a proper temporary directory that will be automatically cleaned up - let _temp_dir = TempDir::with_prefix(&format!("goose_test_{}_", test_id)).unwrap(); + let _temp_dir = TempDir::with_prefix(format!("goose_test_{}_", test_id)).unwrap(); let test_dir = _temp_dir.path(); // Set up environment @@ -275,7 +275,7 @@ mod tests { if log_dir.exists() { for entry in std::fs::read_dir(&log_dir).unwrap() { let entry = entry.unwrap(); - if entry.path().extension().map_or(false, |ext| ext == "log") { + if entry.path().extension().is_some_and(|ext| ext == "log") { std::fs::remove_file(entry.path()).unwrap(); } } @@ -315,7 +315,7 @@ mod tests { let log_count = all_files .iter() - .filter(|e| e.path().extension().map_or(false, |ext| ext == "log")) + .filter(|e| e.path().extension().is_some_and(|ext| ext == "log")) .count(); println!("Found {} log files in directory", log_count); @@ -358,8 +358,8 @@ mod tests { .iter() .filter(|e| { let path = e.path(); - let matches = path.extension().map_or(false, |ext| ext == "log") - && path.file_name().map_or(false, |name| { + let matches = path.extension().is_some_and(|ext| ext == "log") + && path.file_name().is_some_and(|name| { let name = name.to_string_lossy(); if let Some(session) = session_name { name.ends_with(&format!("{}.log", session)) @@ -411,7 +411,7 @@ mod tests { }) .collect(); - log_files.sort_by(|a, b| a.file_name().cmp(&b.file_name())); + log_files.sort_by_key(|a| a.file_name()); assert_eq!(log_files.len(), 1, "Expected exactly one matching log file"); let log_file_name = log_files[0].file_name().to_string_lossy().into_owned(); diff --git a/crates/goose-cli/src/scenario_tests/mock_client.rs b/crates/goose-cli/src/scenario_tests/mock_client.rs index c649d0cc50..e529e09b13 100644 --- a/crates/goose-cli/src/scenario_tests/mock_client.rs +++ b/crates/goose-cli/src/scenario_tests/mock_client.rs @@ -14,9 +14,11 @@ use std::collections::HashMap; use tokio::sync::mpsc::{self, Receiver}; use tokio_util::sync::CancellationToken; +type Handler = Box Result, ErrorData> + Send + Sync>; + pub struct MockClient { tools: HashMap, - handlers: HashMap Result, ErrorData> + Send + Sync>>, + handlers: HashMap, } impl MockClient { diff --git a/crates/goose-cli/src/scenario_tests/scenario_runner.rs b/crates/goose-cli/src/scenario_tests/scenario_runner.rs index 130077c9fd..43dbb2599f 100644 --- a/crates/goose-cli/src/scenario_tests/scenario_runner.rs +++ b/crates/goose-cli/src/scenario_tests/scenario_runner.rs @@ -180,7 +180,7 @@ where let original_env = setup_environment(config)?; - let inner_provider = create(&factory_name, ModelConfig::new(&config.model_name)?)?; + let inner_provider = create(&factory_name, ModelConfig::new(config.model_name)?)?; let test_provider = Arc::new(TestProvider::new_recording(inner_provider, &file_path)); ( diff --git a/crates/goose-cli/src/session/completion.rs b/crates/goose-cli/src/session/completion.rs index f24ee6fe03..e473230252 100644 --- a/crates/goose-cli/src/session/completion.rs +++ b/crates/goose-cli/src/session/completion.rs @@ -443,18 +443,14 @@ mod tests { let mut cache = CompletionCache::new(); // Add some test prompts - let mut extension1_prompts = Vec::new(); - extension1_prompts.push("test_prompt1".to_string()); - extension1_prompts.push("test_prompt2".to_string()); - cache - .prompts - .insert("extension1".to_string(), extension1_prompts); + cache.prompts.insert( + "extension1".to_string(), + vec!["test_prompt1".to_string(), "test_prompt2".to_string()], + ); - let mut extension2_prompts = Vec::new(); - extension2_prompts.push("other_prompt".to_string()); cache .prompts - .insert("extension2".to_string(), extension2_prompts); + .insert("extension2".to_string(), vec!["other_prompt".to_string()]); // Add prompt info with arguments let test_prompt1_args = vec![ @@ -519,7 +515,7 @@ mod tests { let (pos, candidates) = completer.complete_slash_commands("/e").unwrap(); assert_eq!(pos, 0); // There might be multiple commands starting with "e" like "/exit" and "/extension" - assert!(candidates.len() >= 1); + assert!(!candidates.is_empty()); // Test multiple matches let (pos, candidates) = completer.complete_slash_commands("/").unwrap(); diff --git a/crates/goose-cli/src/session/task_execution_display/tests.rs b/crates/goose-cli/src/session/task_execution_display/tests.rs index 7968af7da7..c86b62258e 100644 --- a/crates/goose-cli/src/session/task_execution_display/tests.rs +++ b/crates/goose-cli/src/session/task_execution_display/tests.rs @@ -34,7 +34,7 @@ fn test_format_result_data_for_display() { assert_eq!(format_result_data_for_display(&json!(true)), "true"); assert_eq!(format_result_data_for_display(&json!(false)), "false"); assert_eq!(format_result_data_for_display(&json!(42)), "42"); - assert_eq!(format_result_data_for_display(&json!(3.14)), "3.14"); + assert_eq!(format_result_data_for_display(&json!(3.41)), "3.41"); assert_eq!(format_result_data_for_display(&json!(null)), "null"); let partial_obj = json!({ diff --git a/crates/goose-mcp/src/developer/analyze/tests/traversal_tests.rs b/crates/goose-mcp/src/developer/analyze/tests/traversal_tests.rs index 8cc860ae53..27956a8778 100644 --- a/crates/goose-mcp/src/developer/analyze/tests/traversal_tests.rs +++ b/crates/goose-mcp/src/developer/analyze/tests/traversal_tests.rs @@ -134,7 +134,7 @@ fn test_symlink_handling() { #[cfg(unix)] { use std::os::unix::fs::symlink; - let _ = symlink(&dir_path.join("target.rs"), dir_path.join("link.rs")); + let _ = symlink(dir_path.join("target.rs"), dir_path.join("link.rs")); let _ = symlink(&target_dir, dir_path.join("link_dir")); } diff --git a/crates/goose-mcp/src/developer/rmcp_developer.rs b/crates/goose-mcp/src/developer/rmcp_developer.rs index d8fd5d73e1..7169116e5c 100644 --- a/crates/goose-mcp/src/developer/rmcp_developer.rs +++ b/crates/goose-mcp/src/developer/rmcp_developer.rs @@ -2970,7 +2970,7 @@ mod tests { let temp_path = temp_dir.path(); // Set the current directory before creating the server - std::env::set_current_dir(&temp_path).unwrap(); + std::env::set_current_dir(temp_path).unwrap(); // Create some test files and directories fs::create_dir(temp_path.join("subdir1")).unwrap(); @@ -3029,7 +3029,7 @@ mod tests { let temp_path = temp_dir.path(); // Set the current directory before creating the server - std::env::set_current_dir(&temp_path).unwrap(); + std::env::set_current_dir(temp_path).unwrap(); // Create more than 50 files to test the limit for i in 0..60 { @@ -3085,7 +3085,7 @@ mod tests { let temp_path = temp_dir.path(); // Set the current directory before creating the server - std::env::set_current_dir(&temp_path).unwrap(); + std::env::set_current_dir(temp_path).unwrap(); let server = create_test_server(); @@ -3666,7 +3666,7 @@ Additional instructions here. // Cancel the command let cancel_params = CancelledNotificationParam { - request_id: request_id, + request_id, reason: Some("test cancellation".to_string()), }; @@ -3748,7 +3748,7 @@ Additional instructions here. // Cancel the command let cancel_params = CancelledNotificationParam { - request_id: request_id, + request_id, reason: Some("test cancellation".to_string()), }; diff --git a/crates/goose-server/src/routes/audio.rs b/crates/goose-server/src/routes/audio.rs index 8fa36a1945..6985d96d1c 100644 --- a/crates/goose-server/src/routes/audio.rs +++ b/crates/goose-server/src/routes/audio.rs @@ -391,7 +391,6 @@ pub fn routes(state: Arc) -> Router { mod tests { use super::*; use axum::{body::Body, http::Request}; - use serde_json::json; use tower::ServiceExt; #[tokio::test(flavor = "multi_thread")] @@ -424,7 +423,6 @@ mod tests { let state = AppState::new().await.unwrap(); let app = routes(state); - let large_data = "a".repeat(30 * 1024 * 1024); // 30MB let request = Request::builder() .uri("/audio/transcribe") .method("POST") diff --git a/crates/goose-server/src/routes/extension.rs b/crates/goose-server/src/routes/extension.rs index ecdaad4eab..fa274ddbba 100644 --- a/crates/goose-server/src/routes/extension.rs +++ b/crates/goose-server/src/routes/extension.rs @@ -631,25 +631,19 @@ mod tests { #[test] fn test_make_full() { assert_eq!( - make_full_cmd("uvx", &vec!["mcp_slack".to_string()]), + make_full_cmd("uvx", &["mcp_slack".to_string()]), "uvx mcp_slack" ); assert_eq!( - make_full_cmd("uvx", &vec!["mcp_slack ".to_string()]), + make_full_cmd("uvx", &["mcp_slack ".to_string()]), "uvx mcp_slack" ); assert_eq!( - make_full_cmd( - "uvx", - &vec!["mcp_slack".to_string(), "--verbose".to_string()] - ), + make_full_cmd("uvx", &["mcp_slack".to_string(), "--verbose".to_string()]), "uvx mcp_slack --verbose" ); assert_eq!( - make_full_cmd( - "uvx", - &vec!["mcp_slack".to_string(), " --verbose".to_string()] - ), + make_full_cmd("uvx", &["mcp_slack".to_string(), " --verbose".to_string()]), "uvx mcp_slack --verbose" ); } @@ -1093,14 +1087,14 @@ mod tests { // With bypass enabled, any command should be allowed regardless of allowlist assert!(is_command_allowed( "uvx", - &vec!["unauthorized_command".to_string()] + &["unauthorized_command".to_string()] )); // Test case insensitivity env::set_var("GOOSE_ALLOWLIST_BYPASS", "TRUE"); assert!(is_command_allowed( "uvx", - &vec!["unauthorized_command".to_string()] + &["unauthorized_command".to_string()] )); // Clean up diff --git a/crates/goose-server/src/routes/recipe_utils.rs b/crates/goose-server/src/routes/recipe_utils.rs index f55b193146..6caa7fc814 100644 --- a/crates/goose-server/src/routes/recipe_utils.rs +++ b/crates/goose-server/src/routes/recipe_utils.rs @@ -129,6 +129,6 @@ recipe: recipe_content let result = RecipeManifestMetadata::from_yaml_file(&file_path).unwrap(); assert_eq!(result.name, "Test Recipe"); - assert_eq!(result.is_global, true); + assert!(result.is_global); } } diff --git a/crates/goose-server/src/routes/reply.rs b/crates/goose-server/src/routes/reply.rs index e68aef9283..9f118a3b5a 100644 --- a/crates/goose-server/src/routes/reply.rs +++ b/crates/goose-server/src/routes/reply.rs @@ -557,58 +557,15 @@ pub fn routes(state: Arc) -> Router { #[cfg(test)] mod tests { use super::*; - use goose::conversation::message::Message; - use goose::{ - model::ModelConfig, - providers::{ - base::{Provider, ProviderUsage, Usage}, - errors::ProviderError, - }, - }; - - #[derive(Clone)] - struct MockProvider { - model_config: ModelConfig, - } - - #[async_trait::async_trait] - impl Provider for MockProvider { - fn metadata() -> goose::providers::base::ProviderMetadata { - goose::providers::base::ProviderMetadata::empty() - } - - async fn complete_with_model( - &self, - _model_config: &ModelConfig, - _system: &str, - _messages: &[Message], - _tools: &[rmcp::model::Tool], - ) -> anyhow::Result<(Message, ProviderUsage), ProviderError> { - Ok(( - Message::assistant().with_text("Mock response"), - ProviderUsage::new("mock".to_string(), Usage::default()), - )) - } - - fn get_model_config(&self) -> ModelConfig { - self.model_config.clone() - } - } mod integration_tests { use super::*; use axum::{body::Body, http::Request}; use goose::conversation::message::Message; - use serde_json::json; use tower::ServiceExt; #[tokio::test(flavor = "multi_thread")] async fn test_reply_endpoint() { - let mock_model_config = ModelConfig::new("test-model").unwrap(); - let mock_provider = MockProvider { - model_config: mock_model_config, - }; - let state = AppState::new().await.unwrap(); let app = routes(state); diff --git a/crates/goose/examples/databricks_oauth.rs b/crates/goose/examples/databricks_oauth.rs index fc31e3c7ef..8000a97d56 100644 --- a/crates/goose/examples/databricks_oauth.rs +++ b/crates/goose/examples/databricks_oauth.rs @@ -31,9 +31,9 @@ async fn main() -> Result<()> { let mut usage = Usage::default(); while let Some(Ok((msg, usage_part))) = stream.next().await { dbg!(msg); - usage_part.map(|u| { + if let Some(u) = usage_part { usage += u.usage; - }); + } } println!("\nToken Usage:"); println!("------------"); diff --git a/crates/goose/src/agents/recipe_tools/param_utils/tests.rs b/crates/goose/src/agents/recipe_tools/param_utils/tests.rs index 9be6ccfa0e..eaf99ede13 100644 --- a/crates/goose/src/agents/recipe_tools/param_utils/tests.rs +++ b/crates/goose/src/agents/recipe_tools/param_utils/tests.rs @@ -6,14 +6,13 @@ use serde_json::json; use crate::agents::recipe_tools::param_utils::prepare_command_params; fn setup_default_sub_recipe() -> SubRecipe { - let sub_recipe = SubRecipe { + SubRecipe { name: "test_sub_recipe".to_string(), path: "test_sub_recipe.yaml".to_string(), values: Some(HashMap::from([("key1".to_string(), "value1".to_string())])), sequential_when_repeated: true, description: Some("Test subrecipe".to_string()), - }; - sub_recipe + } } mod prepare_command_params_tests { diff --git a/crates/goose/src/agents/recipe_tools/sub_recipe_tools/tests.rs b/crates/goose/src/agents/recipe_tools/sub_recipe_tools/tests.rs index 1eb64b0842..e2938c8cce 100644 --- a/crates/goose/src/agents/recipe_tools/sub_recipe_tools/tests.rs +++ b/crates/goose/src/agents/recipe_tools/sub_recipe_tools/tests.rs @@ -1,51 +1,48 @@ -#[cfg(test)] -mod tests { - use std::collections::HashMap; +use std::collections::HashMap; - use crate::recipe::SubRecipe; - use serde_json::json; - use serde_json::Value; - use tempfile::TempDir; +use crate::recipe::SubRecipe; +use serde_json::json; +use serde_json::Value; +use tempfile::TempDir; - fn setup_default_sub_recipe() -> SubRecipe { - let sub_recipe = SubRecipe { - name: "test_sub_recipe".to_string(), - path: "test_sub_recipe.yaml".to_string(), - values: Some(HashMap::from([("key1".to_string(), "value1".to_string())])), - sequential_when_repeated: true, - description: Some("Test subrecipe".to_string()), - }; - sub_recipe +fn setup_default_sub_recipe() -> SubRecipe { + SubRecipe { + name: "test_sub_recipe".to_string(), + path: "test_sub_recipe.yaml".to_string(), + values: Some(HashMap::from([("key1".to_string(), "value1".to_string())])), + sequential_when_repeated: true, + description: Some("Test subrecipe".to_string()), + } +} + +mod get_input_schema { + use super::*; + use crate::agents::recipe_tools::sub_recipe_tools::get_input_schema; + + fn prepare_sub_recipe(sub_recipe_file_content: &str) -> (SubRecipe, TempDir) { + let mut sub_recipe = setup_default_sub_recipe(); + let temp_dir = tempfile::tempdir().unwrap(); + let temp_file = temp_dir.path().join(sub_recipe.path.clone()); + std::fs::write(&temp_file, sub_recipe_file_content).unwrap(); + sub_recipe.path = temp_file.to_string_lossy().to_string(); + (sub_recipe, temp_dir) } - mod get_input_schema { - use super::*; - use crate::agents::recipe_tools::sub_recipe_tools::get_input_schema; + fn verify_task_parameters(result: Value, expected_task_parameters_items: Value) { + let task_parameters = result + .get("properties") + .unwrap() + .as_object() + .unwrap() + .get("task_parameters") + .unwrap() + .as_object() + .unwrap(); + let task_parameters_items = task_parameters.get("items").unwrap(); + assert_eq!(&expected_task_parameters_items, task_parameters_items); + } - fn prepare_sub_recipe(sub_recipe_file_content: &str) -> (SubRecipe, TempDir) { - let mut sub_recipe = setup_default_sub_recipe(); - let temp_dir = tempfile::tempdir().unwrap(); - let temp_file = temp_dir.path().join(sub_recipe.path.clone()); - std::fs::write(&temp_file, sub_recipe_file_content).unwrap(); - sub_recipe.path = temp_file.to_string_lossy().to_string(); - (sub_recipe, temp_dir) - } - - fn verify_task_parameters(result: Value, expected_task_parameters_items: Value) { - let task_parameters = result - .get("properties") - .unwrap() - .as_object() - .unwrap() - .get("task_parameters") - .unwrap() - .as_object() - .unwrap(); - let task_parameters_items = task_parameters.get("items").unwrap(); - assert_eq!(&expected_task_parameters_items, task_parameters_items); - } - - const SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS: &str = r#"{ + const SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS: &str = r#"{ "version": "1.0.0", "title": "Test Recipe", "description": "A test recipe", @@ -66,67 +63,66 @@ mod tests { ] }"#; - #[test] - fn test_with_one_param_in_tool_input() { - let (mut sub_recipe, _temp_dir) = - prepare_sub_recipe(SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS); - sub_recipe.values = Some(HashMap::from([("key1".to_string(), "value1".to_string())])); + #[test] + fn test_with_one_param_in_tool_input() { + let (mut sub_recipe, _temp_dir) = + prepare_sub_recipe(SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS); + sub_recipe.values = Some(HashMap::from([("key1".to_string(), "value1".to_string())])); - let result = get_input_schema(&sub_recipe).unwrap(); + let result = get_input_schema(&sub_recipe).unwrap(); - verify_task_parameters( - result, - json!({ - "type": "object", - "properties": { - "key2": { "type": "number", "description": "An optional parameter" } - }, - "required": [] - }), - ); - } + verify_task_parameters( + result, + json!({ + "type": "object", + "properties": { + "key2": { "type": "number", "description": "An optional parameter" } + }, + "required": [] + }), + ); + } - #[test] - fn test_without_param_in_tool_input() { - let (mut sub_recipe, _temp_dir) = - prepare_sub_recipe(SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS); - sub_recipe.values = Some(HashMap::from([ - ("key1".to_string(), "value1".to_string()), - ("key2".to_string(), "value2".to_string()), - ])); + #[test] + fn test_without_param_in_tool_input() { + let (mut sub_recipe, _temp_dir) = + prepare_sub_recipe(SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS); + sub_recipe.values = Some(HashMap::from([ + ("key1".to_string(), "value1".to_string()), + ("key2".to_string(), "value2".to_string()), + ])); - let result = get_input_schema(&sub_recipe).unwrap(); + let result = get_input_schema(&sub_recipe).unwrap(); - assert_eq!( - None, - result - .get("properties") - .unwrap() - .as_object() - .unwrap() - .get("task_parameters") - ); - } + assert_eq!( + None, + result + .get("properties") + .unwrap() + .as_object() + .unwrap() + .get("task_parameters") + ); + } - #[test] - fn test_with_all_params_in_tool_input() { - let (mut sub_recipe, _temp_dir) = - prepare_sub_recipe(SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS); - sub_recipe.values = None; + #[test] + fn test_with_all_params_in_tool_input() { + let (mut sub_recipe, _temp_dir) = + prepare_sub_recipe(SUB_RECIPE_FILE_CONTENT_WITH_TWO_PARAMS); + sub_recipe.values = None; - let result = get_input_schema(&sub_recipe).unwrap(); + let result = get_input_schema(&sub_recipe).unwrap(); - verify_task_parameters( - result, - json!({ - "type": "object", - "properties": { - "key1": { "type": "string", "description": "A test parameter" }, - "key2": { "type": "number", "description": "An optional parameter" } - }, - "required": ["key1"] - }), - ); - } + verify_task_parameters( + result, + json!({ + "type": "object", + "properties": { + "key1": { "type": "string", "description": "A test parameter" }, + "key2": { "type": "number", "description": "An optional parameter" } + }, + "required": ["key1"] + }), + ); } } diff --git a/crates/goose/src/config/base.rs b/crates/goose/src/config/base.rs index 52e7532e9c..9dd93d8a89 100644 --- a/crates/goose/src/config/base.rs +++ b/crates/goose/src/config/base.rs @@ -1024,7 +1024,7 @@ mod tests { let key = format!("key{}", i); let value = format!("value{}", i); assert!( - final_values.get(&key).is_some(), + final_values.contains_key(&key), "Missing key {} in final values", key ); @@ -1075,7 +1075,7 @@ mod tests { // Should have recovered the data assert!( - recovered_values.len() >= 1, + !recovered_values.is_empty(), "Should have recovered at least one key" ); @@ -1169,7 +1169,7 @@ mod tests { // Should have recovered the data from backup assert!( - recovered_values.len() >= 1, + !recovered_values.is_empty(), "Should have recovered data from backup" ); @@ -1260,10 +1260,10 @@ mod tests { assert_eq!(value, Value::Number((-123).into())); // Test floats - let value = Config::parse_env_value("3.14")?; + let value = Config::parse_env_value("3.41")?; assert!(matches!(value, Value::Number(_))); if let Value::Number(n) = value { - assert_eq!(n.as_f64().unwrap(), 3.14); + assert_eq!(n.as_f64().unwrap(), 3.41); } let value = Config::parse_env_value("0.01")?; @@ -1421,7 +1421,7 @@ mod tests { // Test boolean environment variable std::env::set_var("ENABLED", "true"); let value: bool = config.get_param("enabled")?; - assert_eq!(value, true); + assert!(value); // Test JSON object environment variable std::env::set_var("CONFIG", "{\"debug\": true, \"level\": 5}"); @@ -1431,7 +1431,7 @@ mod tests { level: i32, } let value: TestConfig = config.get_param("config")?; - assert_eq!(value.debug, true); + assert!(value.debug); assert_eq!(value.level, 5); // Clean up diff --git a/crates/goose/src/config/signup_openrouter/tests.rs b/crates/goose/src/config/signup_openrouter/tests.rs index 3ea87b0e03..e07a9ff716 100644 --- a/crates/goose/src/config/signup_openrouter/tests.rs +++ b/crates/goose/src/config/signup_openrouter/tests.rs @@ -1,68 +1,65 @@ -#[cfg(test)] -mod tests { - use crate::config::signup_openrouter::PkceAuthFlow; - use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; - use sha2::{Digest, Sha256}; +use crate::config::signup_openrouter::PkceAuthFlow; +use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; +use sha2::{Digest, Sha256}; - #[test] - fn test_pkce_flow_creation() { - let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); +#[test] +fn test_pkce_flow_creation() { + let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); - // Verify code_verifier is 128 characters - assert_eq!(flow.code_verifier.len(), 128); + // Verify code_verifier is 128 characters + assert_eq!(flow.code_verifier.len(), 128); - // Verify code_challenge is base64url encoded (no padding) - assert!(!flow.code_challenge.contains('=')); - assert!(!flow.code_challenge.contains('+')); - assert!(!flow.code_challenge.contains('/')); + // Verify code_challenge is base64url encoded (no padding) + assert!(!flow.code_challenge.contains('=')); + assert!(!flow.code_challenge.contains('+')); + assert!(!flow.code_challenge.contains('/')); - // Verify auth URL is properly formatted - let auth_url = flow.get_auth_url(); - assert!(auth_url.starts_with("https://openrouter.ai/auth")); - assert!(auth_url.contains("callback_url=http%3A%2F%2Flocalhost%3A3000")); - assert!(auth_url.contains(&format!("code_challenge={}", flow.code_challenge))); - assert!(auth_url.contains("code_challenge_method=S256")); - } - - #[test] - fn test_different_flows_have_different_verifiers() { - let flow1 = PkceAuthFlow::new().expect("Failed to create PKCE flow 1"); - let flow2 = PkceAuthFlow::new().expect("Failed to create PKCE flow 2"); - - // Verify that different flows have different verifiers and challenges - assert_ne!(flow1.code_verifier, flow2.code_verifier); - assert_ne!(flow1.code_challenge, flow2.code_challenge); - } - - #[test] - fn test_code_verifier_is_alphanumeric() { - let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); - - // Verify all characters in code_verifier are alphanumeric - assert!(flow.code_verifier.chars().all(|c| c.is_alphanumeric())); - } - - #[test] - fn test_code_challenge_matches_verifier() { - let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); - - // Manually compute the expected challenge - let mut hasher = Sha256::new(); - hasher.update(&flow.code_verifier); - let hash = hasher.finalize(); - let expected_challenge = URL_SAFE_NO_PAD.encode(hash); - - // Verify the challenge matches - assert_eq!(flow.code_challenge, expected_challenge); - } - - #[test] - fn test_pkce_verifier_length_bounds() { - // PKCE spec requires verifier to be 43-128 characters - // Our implementation uses 128 characters - let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); - - assert!(flow.code_verifier.len() >= 43); - assert!(flow.code_verifier.len() <= 128); - } + // Verify auth URL is properly formatted + let auth_url = flow.get_auth_url(); + assert!(auth_url.starts_with("https://openrouter.ai/auth")); + assert!(auth_url.contains("callback_url=http%3A%2F%2Flocalhost%3A3000")); + assert!(auth_url.contains(&format!("code_challenge={}", flow.code_challenge))); + assert!(auth_url.contains("code_challenge_method=S256")); +} + +#[test] +fn test_different_flows_have_different_verifiers() { + let flow1 = PkceAuthFlow::new().expect("Failed to create PKCE flow 1"); + let flow2 = PkceAuthFlow::new().expect("Failed to create PKCE flow 2"); + + // Verify that different flows have different verifiers and challenges + assert_ne!(flow1.code_verifier, flow2.code_verifier); + assert_ne!(flow1.code_challenge, flow2.code_challenge); +} + +#[test] +fn test_code_verifier_is_alphanumeric() { + let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); + + // Verify all characters in code_verifier are alphanumeric + assert!(flow.code_verifier.chars().all(|c| c.is_alphanumeric())); +} + +#[test] +fn test_code_challenge_matches_verifier() { + let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); + + // Manually compute the expected challenge + let mut hasher = Sha256::new(); + hasher.update(&flow.code_verifier); + let hash = hasher.finalize(); + let expected_challenge = URL_SAFE_NO_PAD.encode(hash); + + // Verify the challenge matches + assert_eq!(flow.code_challenge, expected_challenge); +} + +#[test] +fn test_pkce_verifier_length_bounds() { + // PKCE spec requires verifier to be 43-128 characters + // Our implementation uses 128 characters + let flow = PkceAuthFlow::new().expect("Failed to create PKCE flow"); + + assert!(flow.code_verifier.len() >= 43); + assert!(flow.code_verifier.len() <= 128); } diff --git a/crates/goose/src/context_mgmt/auto_compact.rs b/crates/goose/src/context_mgmt/auto_compact.rs index a86f7d5a07..fa03a94140 100644 --- a/crates/goose/src/context_mgmt/auto_compact.rs +++ b/crates/goose/src/context_mgmt/auto_compact.rs @@ -558,11 +558,14 @@ mod tests { ]; // Create session metadata with specific token counts + #[allow(clippy::field_reassign_with_default)] let mut session_metadata = SessionMetadata::default(); - session_metadata.total_tokens = Some(8000); // High token count to trigger compaction - session_metadata.accumulated_total_tokens = Some(15000); // Even higher accumulated count - session_metadata.input_tokens = Some(5000); - session_metadata.output_tokens = Some(3000); + { + session_metadata.total_tokens = Some(8000); // High token count to trigger compaction + session_metadata.accumulated_total_tokens = Some(15000); // Even higher accumulated count + session_metadata.input_tokens = Some(5000); + session_metadata.output_tokens = Some(3000); + } // Test with session metadata - should use total_tokens for compaction (not accumulated) let result_with_metadata = check_compaction_needed( @@ -594,7 +597,10 @@ mod tests { // Test with metadata that has only accumulated tokens (no total_tokens) let mut session_metadata_no_total = SessionMetadata::default(); - session_metadata_no_total.accumulated_total_tokens = Some(7500); + #[allow(clippy::field_reassign_with_default)] + { + session_metadata_no_total.accumulated_total_tokens = Some(7500); + } let result_with_no_total = check_compaction_needed( &agent, @@ -650,7 +656,10 @@ mod tests { // Create session metadata with high token count to trigger compaction let mut session_metadata = SessionMetadata::default(); - session_metadata.total_tokens = Some(9000); // High enough to trigger compaction + #[allow(clippy::field_reassign_with_default)] + { + session_metadata.total_tokens = Some(9000); // High enough to trigger compaction + } // Test full compaction flow with session metadata let result = check_and_compact_messages( diff --git a/crates/goose/src/context_mgmt/truncate.rs b/crates/goose/src/context_mgmt/truncate.rs index c57e6d42f0..2bdae889b2 100644 --- a/crates/goose/src/context_mgmt/truncate.rs +++ b/crates/goose/src/context_mgmt/truncate.rs @@ -506,7 +506,7 @@ mod tests { let context_limit = 25; let result = truncate_messages( - &messages.messages(), + messages.messages(), &token_counts, context_limit, &OldestFirstTruncation, @@ -590,7 +590,7 @@ mod tests { let context_limit = 100; // Exactly matches total tokens let result = truncate_messages( - &messages.messages(), + messages.messages(), &token_counts, context_limit, &OldestFirstTruncation, @@ -605,7 +605,7 @@ mod tests { token_counts.push(1); let result = truncate_messages( - &messages.messages(), + messages.messages(), &token_counts, context_limit, &OldestFirstTruncation, @@ -698,7 +698,7 @@ mod tests { let (messages, token_counts) = result; // Verify the conversation still makes sense - assert!(messages.len() >= 1); + assert!(!messages.is_empty()); assert!(messages.last().unwrap().role == Role::User); assert!(token_counts.iter().sum::() <= context_limit); @@ -710,7 +710,7 @@ mod tests { // Test impossibly small context window let (messages, token_counts) = create_messages_with_counts(1, 10, false); let result = truncate_messages( - &messages.messages(), + messages.messages(), &token_counts, 5, // Impossibly small context &OldestFirstTruncation, diff --git a/crates/goose/src/conversation/mod.rs b/crates/goose/src/conversation/mod.rs index 4b7588acac..132e691e1a 100644 --- a/crates/goose/src/conversation/mod.rs +++ b/crates/goose/src/conversation/mod.rs @@ -379,7 +379,7 @@ mod tests { 0, "Fixed conversation should have no issues, but found: {:?}\n\n{}", issues_with_fixed, - debug_conversation_fix(&messages, &fixed.messages(), &issues) + debug_conversation_fix(&messages, fixed.messages(), &issues) ); (fixed.messages().clone(), issues) } diff --git a/crates/goose/src/permission/permission_judge.rs b/crates/goose/src/permission/permission_judge.rs index f804aa7f0a..b14cdbb871 100644 --- a/crates/goose/src/permission/permission_judge.rs +++ b/crates/goose/src/permission/permission_judge.rs @@ -337,7 +337,7 @@ mod tests { assert!(tool .description .as_ref() - .map_or(false, |desc| desc.contains("read-only operation"))); + .is_some_and(|desc| desc.contains("read-only operation"))); } #[test] diff --git a/crates/goose/src/providers/factory.rs b/crates/goose/src/providers/factory.rs index 22a02ae153..084b29fea3 100644 --- a/crates/goose/src/providers/factory.rs +++ b/crates/goose/src/providers/factory.rs @@ -30,11 +30,6 @@ use crate::model::ModelConfig; use anyhow::Result; use once_cell::sync::Lazy; -#[cfg(test)] -use super::errors::ProviderError; -#[cfg(test)] -use rmcp::model::Tool; - const DEFAULT_LEAD_TURNS: usize = 3; const DEFAULT_FAILURE_THRESHOLD: usize = 2; const DEFAULT_FALLBACK_TURNS: usize = 2; @@ -174,63 +169,8 @@ fn create_worker_model_config(default_model: &ModelConfig) -> Result ProviderMetadata { - ProviderMetadata::new( - "mock_test", - "Mock Test Provider", - "A mock provider for testing", - "mock-model", - vec!["mock-model"], - "", - vec![], - ) - } - - fn get_model_config(&self) -> ModelConfig { - self.model_config.clone() - } - - async fn complete_with_model( - &self, - _model_config: &ModelConfig, - _system: &str, - _messages: &[Message], - _tools: &[Tool], - ) -> Result<(Message, ProviderUsage), ProviderError> { - Ok(( - Message::new( - Role::Assistant, - Utc::now().timestamp(), - vec![MessageContent::Text( - RawTextContent { - text: format!( - "Response from {} with model {}", - self.name, self.model_config.model_name - ), - meta: None, - } - .no_annotation(), - )], - ), - ProviderUsage::new(self.model_config.model_name.clone(), Usage::default()), - )) - } - } - struct EnvVarGuard { vars: Vec<(String, Option)>, } @@ -355,17 +295,12 @@ mod tests { let default_model = ModelConfig::new_or_fail("gpt-3.5-turbo").with_context_limit(Some(16_000)); - let result = create_lead_worker_from_env("openai", &default_model, "gpt-4o"); + let _result = create_lead_worker_from_env("openai", &default_model, "gpt-4o"); _guard.set("GOOSE_WORKER_CONTEXT_LIMIT", "32000"); let _result = create_lead_worker_from_env("openai", &default_model, "gpt-4o"); _guard.set("GOOSE_CONTEXT_LIMIT", "64000"); let _result = create_lead_worker_from_env("openai", &default_model, "gpt-4o"); - - match result { - Ok(_) => {} - Err(_) => {} - } } } diff --git a/crates/goose/src/providers/utils.rs b/crates/goose/src/providers/utils.rs index 05046b724f..b03dcc2354 100644 --- a/crates/goose/src/providers/utils.rs +++ b/crates/goose/src/providers/utils.rs @@ -545,7 +545,7 @@ mod tests { 0x0D, 0x0A, 0x1A, 0x0A, // PNG header 0x00, 0x00, 0x00, 0x0D, // Rest of fake PNG data ]; - std::fs::write(&png_path, &png_data).unwrap(); + std::fs::write(&png_path, png_data).unwrap(); let png_path_str = png_path.to_str().unwrap(); // Create a fake PNG (wrong magic numbers) @@ -583,7 +583,7 @@ mod tests { 0x0D, 0x0A, 0x1A, 0x0A, // PNG header 0x00, 0x00, 0x00, 0x0D, // Rest of fake PNG data ]; - std::fs::write(&png_path, &png_data).unwrap(); + std::fs::write(&png_path, png_data).unwrap(); let png_path_str = png_path.to_str().unwrap(); // Create a fake PNG (wrong magic numbers) @@ -613,7 +613,7 @@ mod tests { let gif_path = temp_dir.path().join("test.gif"); // Minimal GIF89a header let gif_data = [0x47, 0x49, 0x46, 0x38, 0x39, 0x61]; - std::fs::write(&gif_path, &gif_data).unwrap(); + std::fs::write(&gif_path, gif_data).unwrap(); let gif_path_str = gif_path.to_str().unwrap(); // Test loading unsupported GIF format @@ -965,7 +965,7 @@ mod tests { // Make request to mock server let client = reqwest::Client::new(); let response = client - .get(&format!("{}/test", &mock_server.uri())) + .get(format!("{}/test", &mock_server.uri())) .send() .await .unwrap(); diff --git a/crates/goose/src/recipe/build_recipe/tests.rs b/crates/goose/src/recipe/build_recipe/tests.rs index 52c2834cf0..e842791866 100644 --- a/crates/goose/src/recipe/build_recipe/tests.rs +++ b/crates/goose/src/recipe/build_recipe/tests.rs @@ -1,97 +1,96 @@ -#[cfg(test)] -mod tests { - use crate::recipe::build_recipe::{ - build_recipe_from_template, resolve_sub_recipe_path, RecipeError, - }; - use crate::recipe::read_recipe_file_content::RecipeFile; - use crate::recipe::{RecipeParameterInputType, RecipeParameterRequirement}; - use tempfile::TempDir; +use crate::recipe::build_recipe::{ + build_recipe_from_template, resolve_sub_recipe_path, RecipeError, +}; +use crate::recipe::read_recipe_file_content::RecipeFile; +use crate::recipe::{RecipeParameterInputType, RecipeParameterRequirement}; +use tempfile::TempDir; - const NO_USER_PROMPT: Option Result> = None; +#[allow(clippy::type_complexity)] +const NO_USER_PROMPT: Option Result> = None; - fn setup_recipe_file(instructions_and_parameters: &str) -> (TempDir, RecipeFile) { - let recipe_content = format!( - r#"{{ +fn setup_recipe_file(instructions_and_parameters: &str) -> (TempDir, RecipeFile) { + let recipe_content = format!( + r#"{{ "version": "1.0.0", "title": "Test Recipe", "description": "A test recipe", {} }}"#, - instructions_and_parameters - ); - let temp_dir = tempfile::tempdir().unwrap(); - let recipe_path = temp_dir.path().join("test_recipe.json"); + instructions_and_parameters + ); + let temp_dir = tempfile::tempdir().unwrap(); + let recipe_path = temp_dir.path().join("test_recipe.json"); - std::fs::write(&recipe_path, recipe_content).unwrap(); + std::fs::write(&recipe_path, recipe_content).unwrap(); - let recipe_file = RecipeFile { - content: std::fs::read_to_string(&recipe_path).unwrap(), - parent_dir: temp_dir.path().to_path_buf(), - file_path: recipe_path, - }; + let recipe_file = RecipeFile { + content: std::fs::read_to_string(&recipe_path).unwrap(), + parent_dir: temp_dir.path().to_path_buf(), + file_path: recipe_path, + }; - (temp_dir, recipe_file) - } + (temp_dir, recipe_file) +} - fn setup_test_file(temp_dir: &TempDir, filename: &str, content: &str) -> std::path::PathBuf { - let file_path = temp_dir.path().join(filename); - std::fs::write(&file_path, content).unwrap(); - file_path - } +fn setup_test_file(temp_dir: &TempDir, filename: &str, content: &str) -> std::path::PathBuf { + let file_path = temp_dir.path().join(filename); + std::fs::write(&file_path, content).unwrap(); + file_path +} - fn setup_yaml_recipe_file(instructions_and_parameters: &str) -> (TempDir, RecipeFile) { - let recipe_content = format!( - r#"version: "1.0.0" +fn setup_yaml_recipe_file(instructions_and_parameters: &str) -> (TempDir, RecipeFile) { + let recipe_content = format!( + r#"version: "1.0.0" title: "Test Recipe" description: "A test recipe" {}"#, - instructions_and_parameters - ); - let temp_dir = tempfile::tempdir().unwrap(); - let recipe_path = temp_dir.path().join("test_recipe.yaml"); + instructions_and_parameters + ); + let temp_dir = tempfile::tempdir().unwrap(); + let recipe_path = temp_dir.path().join("test_recipe.yaml"); - std::fs::write(&recipe_path, recipe_content).unwrap(); + std::fs::write(&recipe_path, recipe_content).unwrap(); - let recipe_file = RecipeFile { - content: std::fs::read_to_string(&recipe_path).unwrap(), - parent_dir: temp_dir.path().to_path_buf(), - file_path: recipe_path, - }; + let recipe_file = RecipeFile { + content: std::fs::read_to_string(&recipe_path).unwrap(), + parent_dir: temp_dir.path().to_path_buf(), + file_path: recipe_path, + }; - (temp_dir, recipe_file) - } + (temp_dir, recipe_file) +} - fn setup_yaml_recipe_files( - parent_content: &str, - child_content: &str, - ) -> (TempDir, RecipeFile, RecipeFile) { - let temp_dir = tempfile::tempdir().unwrap(); - let temp_path = temp_dir.path(); +fn setup_yaml_recipe_files( + parent_content: &str, + child_content: &str, +) -> (TempDir, RecipeFile, RecipeFile) { + let temp_dir = tempfile::tempdir().unwrap(); + let temp_path = temp_dir.path(); - let parent_path = temp_path.join("parent.yaml"); - std::fs::write(&parent_path, parent_content).unwrap(); + let parent_path = temp_path.join("parent.yaml"); + std::fs::write(&parent_path, parent_content).unwrap(); - let child_path = temp_path.join("child.yaml"); - std::fs::write(&child_path, child_content).unwrap(); + let child_path = temp_path.join("child.yaml"); + std::fs::write(&child_path, child_content).unwrap(); - let parent_recipe_file = RecipeFile { - content: std::fs::read_to_string(&parent_path).unwrap(), - parent_dir: temp_path.to_path_buf(), - file_path: parent_path, - }; + let parent_recipe_file = RecipeFile { + content: std::fs::read_to_string(&parent_path).unwrap(), + parent_dir: temp_path.to_path_buf(), + file_path: parent_path, + }; - let child_recipe_file = RecipeFile { - content: std::fs::read_to_string(&child_path).unwrap(), - parent_dir: temp_path.to_path_buf(), - file_path: child_path, - }; + let child_recipe_file = RecipeFile { + content: std::fs::read_to_string(&child_path).unwrap(), + parent_dir: temp_path.to_path_buf(), + file_path: child_path, + }; - (temp_dir, parent_recipe_file, child_recipe_file) - } + (temp_dir, parent_recipe_file, child_recipe_file) +} - #[test] - fn test_build_recipe_from_template_success() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_success() { + let instructions_and_parameters = r#" "instructions": "Test instructions with {{ my_name }}", "parameters": [ { @@ -102,28 +101,28 @@ description: "A test recipe" } ]"#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let params = vec![("my_name".to_string(), "value".to_string())]; - let recipe = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT).unwrap(); + let params = vec![("my_name".to_string(), "value".to_string())]; + let recipe = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT).unwrap(); - assert_eq!(recipe.title, "Test Recipe"); - assert_eq!(recipe.description, "A test recipe"); - assert_eq!(recipe.instructions.unwrap(), "Test instructions with value"); - assert_eq!(recipe.parameters.as_ref().unwrap().len(), 1); - let param = &recipe.parameters.as_ref().unwrap()[0]; - assert_eq!(param.key, "my_name"); - assert!(matches!(param.input_type, RecipeParameterInputType::String)); - assert!(matches!( - param.requirement, - RecipeParameterRequirement::Required - )); - assert_eq!(param.description, "A test parameter"); - } + assert_eq!(recipe.title, "Test Recipe"); + assert_eq!(recipe.description, "A test recipe"); + assert_eq!(recipe.instructions.unwrap(), "Test instructions with value"); + assert_eq!(recipe.parameters.as_ref().unwrap().len(), 1); + let param = &recipe.parameters.as_ref().unwrap()[0]; + assert_eq!(param.key, "my_name"); + assert!(matches!(param.input_type, RecipeParameterInputType::String)); + assert!(matches!( + param.requirement, + RecipeParameterRequirement::Required + )); + assert_eq!(param.description, "A test parameter"); +} - #[test] - fn test_build_recipe_from_template_success_variable_in_prompt() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_success_variable_in_prompt() { + let instructions_and_parameters = r#" "instructions": "Test instructions", "prompt": "My prompt {{ my_name }}", "parameters": [ @@ -135,28 +134,28 @@ description: "A test recipe" } ]"#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let params = vec![("my_name".to_string(), "value".to_string())]; - let recipe = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT).unwrap(); + let params = vec![("my_name".to_string(), "value".to_string())]; + let recipe = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT).unwrap(); - assert_eq!(recipe.title, "Test Recipe"); - assert_eq!(recipe.description, "A test recipe"); - assert_eq!(recipe.instructions.unwrap(), "Test instructions"); - assert_eq!(recipe.prompt.unwrap(), "My prompt value"); - let param = &recipe.parameters.as_ref().unwrap()[0]; - assert_eq!(param.key, "my_name"); - assert!(matches!(param.input_type, RecipeParameterInputType::String)); - assert!(matches!( - param.requirement, - RecipeParameterRequirement::Required - )); - assert_eq!(param.description, "A test parameter"); - } + assert_eq!(recipe.title, "Test Recipe"); + assert_eq!(recipe.description, "A test recipe"); + assert_eq!(recipe.instructions.unwrap(), "Test instructions"); + assert_eq!(recipe.prompt.unwrap(), "My prompt value"); + let param = &recipe.parameters.as_ref().unwrap()[0]; + assert_eq!(param.key, "my_name"); + assert!(matches!(param.input_type, RecipeParameterInputType::String)); + assert!(matches!( + param.requirement, + RecipeParameterRequirement::Required + )); + assert_eq!(param.description, "A test parameter"); +} - #[test] - fn test_build_recipe_from_template_wrong_parameters_in_recipe_file() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_wrong_parameters_in_recipe_file() { + let instructions_and_parameters = r#" "instructions": "Test instructions with {{ expected_param1 }} {{ expected_param2 }}", "parameters": [ { @@ -166,29 +165,28 @@ description: "A test recipe" "description": "A test parameter" } ]"#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let build_recipe_result = - build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT); - assert!(build_recipe_result.is_err()); - let err = build_recipe_result.unwrap_err(); - println!("{}", err.to_string()); + let build_recipe_result = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT); + assert!(build_recipe_result.is_err()); + let err = build_recipe_result.unwrap_err(); + println!("{}", err); - match err { - RecipeError::TemplateRendering { source } => { - let err_str = source.to_string(); - assert!(err_str.contains("Unnecessary parameter definitions: wrong_param_key.")); - assert!(err_str.contains("Missing definitions for parameters in the recipe file:")); - assert!(err_str.contains("expected_param1")); - assert!(err_str.contains("expected_param2")); - } - _ => panic!("Expected TemplateRendering error"), + match err { + RecipeError::TemplateRendering { source } => { + let err_str = source.to_string(); + assert!(err_str.contains("Unnecessary parameter definitions: wrong_param_key.")); + assert!(err_str.contains("Missing definitions for parameters in the recipe file:")); + assert!(err_str.contains("expected_param1")); + assert!(err_str.contains("expected_param2")); } + _ => panic!("Expected TemplateRendering error"), } +} - #[test] - fn test_build_recipe_from_template_with_default_values_in_recipe_file() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_with_default_values_in_recipe_file() { + let instructions_and_parameters = r#" "instructions": "Test instructions with {{ param_with_default }} {{ param_without_default }}", "parameters": [ { @@ -205,23 +203,22 @@ description: "A test recipe" "description": "A test parameter" } ]"#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let params = vec![("param_without_default".to_string(), "value1".to_string())]; + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let params = vec![("param_without_default".to_string(), "value1".to_string())]; - let recipe = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT).unwrap(); + let recipe = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT).unwrap(); - assert_eq!(recipe.title, "Test Recipe"); - assert_eq!(recipe.description, "A test recipe"); - assert_eq!( - recipe.instructions.unwrap(), - "Test instructions with my_default_value value1" - ); - } + assert_eq!(recipe.title, "Test Recipe"); + assert_eq!(recipe.description, "A test recipe"); + assert_eq!( + recipe.instructions.unwrap(), + "Test instructions with my_default_value value1" + ); +} - #[test] - fn test_build_recipe_from_template_optional_parameters_with_empty_default_values_in_recipe_file( - ) { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_optional_parameters_with_empty_default_values_in_recipe_file() { + let instructions_and_parameters = r#" "instructions": "Test instructions with {{ optional_param }}", "parameters": [ { @@ -232,17 +229,17 @@ description: "A test recipe" "default": "" } ]"#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let recipe = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap(); - assert_eq!(recipe.title, "Test Recipe"); - assert_eq!(recipe.description, "A test recipe"); - assert_eq!(recipe.instructions.unwrap(), "Test instructions with "); - } + let recipe = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap(); + assert_eq!(recipe.title, "Test Recipe"); + assert_eq!(recipe.description, "A test recipe"); + assert_eq!(recipe.instructions.unwrap(), "Test instructions with "); +} - #[test] - fn test_build_recipe_from_template_optional_parameters_without_default_values_in_recipe_file() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_optional_parameters_without_default_values_in_recipe_file() { + let instructions_and_parameters = r#" "instructions": "Test instructions with {{ optional_param }}", "parameters": [ { @@ -252,24 +249,23 @@ description: "A test recipe" "description": "A test parameter" } ]"#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let build_recipe_result = - build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT); - assert!(build_recipe_result.is_err()); - let err = build_recipe_result.unwrap_err(); - println!("{}", err.to_string()); - match err { - RecipeError::TemplateRendering { source } => { - assert!(source.to_string().to_lowercase().contains("missing")); - } - _ => panic!("Expected TemplateRendering error"), + let build_recipe_result = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT); + assert!(build_recipe_result.is_err()); + let err = build_recipe_result.unwrap_err(); + println!("{}", err); + match err { + RecipeError::TemplateRendering { source } => { + assert!(source.to_string().to_lowercase().contains("missing")); } + _ => panic!("Expected TemplateRendering error"), } +} - #[test] - fn test_build_recipe_from_template_wrong_input_type_in_recipe_file() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_wrong_input_type_in_recipe_file() { + let instructions_and_parameters = r#" "instructions": "Test instructions with {{ param }}", "parameters": [ { @@ -279,37 +275,37 @@ description: "A test recipe" "description": "A test parameter" } ]"#; - let params = vec![("param".to_string(), "value".to_string())]; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let params = vec![("param".to_string(), "value".to_string())]; + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let build_recipe_result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); - assert!(build_recipe_result.is_err()); - let err = build_recipe_result.unwrap_err(); - match err { - RecipeError::TemplateRendering { source } => { - let err_msg = source.to_string(); - eprint!("Error: {}", err_msg); - assert!(err_msg.contains("unknown variant `some_invalid_type`")); - } - _ => panic!("Expected TemplateRendering error, got: {:?}", err), + let build_recipe_result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); + assert!(build_recipe_result.is_err()); + let err = build_recipe_result.unwrap_err(); + match err { + RecipeError::TemplateRendering { source } => { + let err_msg = source.to_string(); + eprint!("Error: {}", err_msg); + assert!(err_msg.contains("unknown variant `some_invalid_type`")); } + _ => panic!("Expected TemplateRendering error, got: {:?}", err), } +} - #[test] - fn test_build_recipe_from_template_success_without_parameters() { - let instructions_and_parameters = r#" +#[test] +fn test_build_recipe_from_template_success_without_parameters() { + let instructions_and_parameters = r#" "instructions": "Test instructions" "#; - let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); - let recipe = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap(); - assert_eq!(recipe.instructions.unwrap(), "Test instructions"); - assert!(recipe.parameters.is_none()); - } + let recipe = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap(); + assert_eq!(recipe.instructions.unwrap(), "Test instructions"); + assert!(recipe.parameters.is_none()); +} - #[test] - fn test_template_inheritance() { - let parent_content = r#" +#[test] +fn test_template_inheritance() { + let parent_content = r#" version: 1.0.0 title: Parent description: Parent recipe @@ -334,102 +330,102 @@ description: "A test recipe" description: whether the feature is enabled "#; - let child_content = r#" + let child_content = r#" {% extends "parent.yaml" -%} {% block prompt -%} What is the capital of Germany? {%- endblock %} "#; - let (_temp_dir, parent_recipe_file, child_recipe_file) = - setup_yaml_recipe_files(parent_content, child_content); + let (_temp_dir, parent_recipe_file, child_recipe_file) = + setup_yaml_recipe_files(parent_content, child_content); - let params = vec![ - ("date".to_string(), "today".to_string()), - ("is_enabled".to_string(), "true".to_string()), - ]; + let params = vec![ + ("date".to_string(), "today".to_string()), + ("is_enabled".to_string(), "true".to_string()), + ]; - let parent_recipe = - build_recipe_from_template(parent_recipe_file, params.clone(), NO_USER_PROMPT).unwrap(); - assert_eq!(parent_recipe.description, "Parent recipe"); - assert_eq!( + let parent_recipe = + build_recipe_from_template(parent_recipe_file, params.clone(), NO_USER_PROMPT).unwrap(); + assert_eq!(parent_recipe.description, "Parent recipe"); + assert_eq!( parent_recipe.prompt.unwrap(), "show me the news for day: today\nWhat is the capital of France?\n\n Feature is enabled.\n" ); - assert_eq!(parent_recipe.parameters.as_ref().unwrap().len(), 2); - assert_eq!(parent_recipe.parameters.as_ref().unwrap()[0].key, "date"); - assert_eq!( - parent_recipe.parameters.as_ref().unwrap()[1].key, - "is_enabled" - ); + assert_eq!(parent_recipe.parameters.as_ref().unwrap().len(), 2); + assert_eq!(parent_recipe.parameters.as_ref().unwrap()[0].key, "date"); + assert_eq!( + parent_recipe.parameters.as_ref().unwrap()[1].key, + "is_enabled" + ); - let child_recipe = - build_recipe_from_template(child_recipe_file, params, NO_USER_PROMPT).unwrap(); - assert_eq!(child_recipe.title, "Parent"); - assert_eq!(child_recipe.description, "Parent recipe"); - assert_eq!( + let child_recipe = + build_recipe_from_template(child_recipe_file, params, NO_USER_PROMPT).unwrap(); + assert_eq!(child_recipe.title, "Parent"); + assert_eq!(child_recipe.description, "Parent recipe"); + assert_eq!( child_recipe.prompt.unwrap().trim(), "show me the news for day: today\nWhat is the capital of Germany?\n\n Feature is enabled." ); - assert_eq!(child_recipe.parameters.as_ref().unwrap().len(), 2); - assert_eq!(child_recipe.parameters.as_ref().unwrap()[0].key, "date"); - assert_eq!( - child_recipe.parameters.as_ref().unwrap()[1].key, - "is_enabled" - ); + assert_eq!(child_recipe.parameters.as_ref().unwrap().len(), 2); + assert_eq!(child_recipe.parameters.as_ref().unwrap()[0].key, "date"); + assert_eq!( + child_recipe.parameters.as_ref().unwrap()[1].key, + "is_enabled" + ); +} + +mod sub_recipe_path_resolution { + use super::*; + + fn create_recipe_file( + temp_path: &std::path::Path, + recipe_folder: &str, + recipe_file_name: &str, + content: &str, + ) -> std::path::PathBuf { + let recipes_dir = temp_path.join(recipe_folder); + std::fs::create_dir_all(&recipes_dir).unwrap(); + let recipe_path = recipes_dir.join(recipe_file_name); + std::fs::write(&recipe_path, content).unwrap(); + recipe_path } - mod sub_recipe_path_resolution { - use super::*; + #[test] + fn test_resolve_sub_recipe_path_relative() { + let temp_dir = tempfile::tempdir().unwrap(); + let parent_dir = temp_dir.path(); - fn create_recipe_file( - temp_path: &std::path::Path, - recipe_folder: &str, - recipe_file_name: &str, - content: &str, - ) -> std::path::PathBuf { - let recipes_dir = temp_path.join(recipe_folder); - std::fs::create_dir_all(&recipes_dir).unwrap(); - let recipe_path = recipes_dir.join(recipe_file_name); - std::fs::write(&recipe_path, content).unwrap(); - recipe_path - } + let result = resolve_sub_recipe_path("./sub-recipes/child.yaml", parent_dir); + assert!(result.is_ok()); - #[test] - fn test_resolve_sub_recipe_path_relative() { - let temp_dir = tempfile::tempdir().unwrap(); - let parent_dir = temp_dir.path(); + let expected_path = parent_dir.join("./sub-recipes/child.yaml"); + assert_eq!(result.unwrap(), expected_path.to_str().unwrap()); + } - let result = resolve_sub_recipe_path("./sub-recipes/child.yaml", parent_dir); - assert!(result.is_ok()); + #[test] + fn test_resolve_sub_recipe_path_absolute() { + let temp_dir = tempfile::tempdir().unwrap(); + let parent_dir = temp_dir.path(); + let absolute_path = "/absolute/path/to/recipe.yaml"; - let expected_path = parent_dir.join("./sub-recipes/child.yaml"); - assert_eq!(result.unwrap(), expected_path.to_str().unwrap()); - } + let result = resolve_sub_recipe_path(absolute_path, parent_dir); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), absolute_path); + } - #[test] - fn test_resolve_sub_recipe_path_absolute() { - let temp_dir = tempfile::tempdir().unwrap(); - let parent_dir = temp_dir.path(); - let absolute_path = "/absolute/path/to/recipe.yaml"; - - let result = resolve_sub_recipe_path(absolute_path, parent_dir); - assert!(result.is_ok()); - assert_eq!(result.unwrap(), absolute_path); - } - - #[test] - fn test_build_recipe_with_relative_sub_recipe_path() { - let temp_dir = tempfile::tempdir().unwrap(); - let temp_path = temp_dir.path(); - let sub_recipe_content = r#" + #[test] + fn test_build_recipe_with_relative_sub_recipe_path() { + let temp_dir = tempfile::tempdir().unwrap(); + let temp_path = temp_dir.path(); + let sub_recipe_content = r#" version: 1.0.0 title: Child Recipe description: A child recipe instructions: Child instructions "#; - create_recipe_file(temp_path, "sub-recipes", "child.yaml", sub_recipe_content); - let main_recipe_content = r#"{ + create_recipe_file(temp_path, "sub-recipes", "child.yaml", sub_recipe_content); + let main_recipe_content = r#"{ "version": "1.0.0", "title": "Main Recipe", "description": "Main recipe with sub-recipe", @@ -441,92 +437,91 @@ instructions: Child instructions } ] }"#; - let main_recipe_path = - create_recipe_file(temp_path, "main", "main.json", main_recipe_content); + let main_recipe_path = + create_recipe_file(temp_path, "main", "main.json", main_recipe_content); - let recipe_file = RecipeFile { - content: main_recipe_content.to_string(), - parent_dir: temp_path.to_path_buf(), - file_path: main_recipe_path, - }; + let recipe_file = RecipeFile { + content: main_recipe_content.to_string(), + parent_dir: temp_path.to_path_buf(), + file_path: main_recipe_path, + }; - let recipe = - build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap(); + let recipe = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap(); - assert_eq!(recipe.title, "Main Recipe"); - assert!(recipe.sub_recipes.is_some()); + assert_eq!(recipe.title, "Main Recipe"); + assert!(recipe.sub_recipes.is_some()); - let sub_recipes = recipe.sub_recipes.unwrap(); - assert_eq!(sub_recipes.len(), 1); - assert_eq!(sub_recipes[0].name, "child"); + let sub_recipes = recipe.sub_recipes.unwrap(); + assert_eq!(sub_recipes.len(), 1); + assert_eq!(sub_recipes[0].name, "child"); - let expected_absolute_path = temp_path.join("./sub-recipes/child.yaml"); - assert_eq!( - sub_recipes[0].path, - expected_absolute_path.to_str().unwrap() - ); + let expected_absolute_path = temp_path.join("./sub-recipes/child.yaml"); + assert_eq!( + sub_recipes[0].path, + expected_absolute_path.to_str().unwrap() + ); + } +} + +mod file_parameter_tests { + use super::*; + + #[test] + fn test_build_recipe_file_parameter_valid_paths() { + let instructions_and_parameters = r#"instructions: "Test file content: {{ FILE_PARAM }}" +parameters: + - key: FILE_PARAM + input_type: file + requirement: required + description: A file parameter"#; + + let (temp_dir, recipe_file) = setup_yaml_recipe_file(instructions_and_parameters); + + let test_content = "Hello from file!\nThis is line 2\n Indented line 3"; + let test_file_path = setup_test_file(&temp_dir, "test_file.txt", test_content); + + let params = vec![( + "FILE_PARAM".to_string(), + test_file_path.to_string_lossy().to_string(), + )]; + let result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); + + assert!(result.is_ok()); + let recipe = result.unwrap(); + + let instructions = recipe.instructions.as_ref().unwrap(); + assert!(instructions.contains("Hello from file!")); + assert!(instructions.contains("Test file content:")); + } + + #[test] + fn test_build_recipe_file_parameter_nonexistent_file() { + let instructions_and_parameters = r#"instructions: "Test file content: {{ FILE_PARAM }}" +parameters: + - key: FILE_PARAM + input_type: file + requirement: required + description: A file parameter"#; + + let (_temp_dir, recipe_file) = setup_yaml_recipe_file(instructions_and_parameters); + + let params = vec![( + "FILE_PARAM".to_string(), + "/nonexistent/path/file.txt".to_string(), + )]; + let result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); + + assert!(result.is_err()); + if let Err(RecipeError::TemplateRendering { source }) = result { + assert!(source.to_string().contains("Failed to read parameter file")); + } else { + panic!("Expected TemplateRendering error"); } } - mod file_parameter_tests { - use super::*; - - #[test] - fn test_build_recipe_file_parameter_valid_paths() { - let instructions_and_parameters = r#"instructions: "Test file content: {{ FILE_PARAM }}" -parameters: - - key: FILE_PARAM - input_type: file - requirement: required - description: A file parameter"#; - - let (temp_dir, recipe_file) = setup_yaml_recipe_file(instructions_and_parameters); - - let test_content = "Hello from file!\nThis is line 2\n Indented line 3"; - let test_file_path = setup_test_file(&temp_dir, "test_file.txt", test_content); - - let params = vec![( - "FILE_PARAM".to_string(), - test_file_path.to_string_lossy().to_string(), - )]; - let result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); - - assert!(result.is_ok()); - let recipe = result.unwrap(); - - let instructions = recipe.instructions.as_ref().unwrap(); - assert!(instructions.contains("Hello from file!")); - assert!(instructions.contains("Test file content:")); - } - - #[test] - fn test_build_recipe_file_parameter_nonexistent_file() { - let instructions_and_parameters = r#"instructions: "Test file content: {{ FILE_PARAM }}" -parameters: - - key: FILE_PARAM - input_type: file - requirement: required - description: A file parameter"#; - - let (_temp_dir, recipe_file) = setup_yaml_recipe_file(instructions_and_parameters); - - let params = vec![( - "FILE_PARAM".to_string(), - "/nonexistent/path/file.txt".to_string(), - )]; - let result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); - - assert!(result.is_err()); - if let Err(RecipeError::TemplateRendering { source }) = result { - assert!(source.to_string().contains("Failed to read parameter file")); - } else { - panic!("Expected TemplateRendering error"); - } - } - - #[test] - fn test_build_recipe_file_parameter_with_default_rejected() { - let instructions_and_parameters = r#"instructions: "Test file content: {{ FILE_PARAM }}" + #[test] + fn test_build_recipe_file_parameter_with_default_rejected() { + let instructions_and_parameters = r#"instructions: "Test file content: {{ FILE_PARAM }}" parameters: - key: FILE_PARAM input_type: file @@ -534,19 +529,18 @@ parameters: description: A file parameter default: "/etc/passwd""#; - let (_temp_dir, recipe_file) = setup_yaml_recipe_file(instructions_and_parameters); + let (_temp_dir, recipe_file) = setup_yaml_recipe_file(instructions_and_parameters); - let params = vec![]; - let result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); + let params = vec![]; + let result = build_recipe_from_template(recipe_file, params, NO_USER_PROMPT); - assert!(result.is_err()); - if let Err(RecipeError::TemplateRendering { source }) = result { - assert!(source - .to_string() - .contains("File parameters cannot have default values")); - } else { - panic!("Expected TemplateRendering error for file parameter with default"); - } + assert!(result.is_err()); + if let Err(RecipeError::TemplateRendering { source }) = result { + assert!(source + .to_string() + .contains("File parameters cannot have default values")); + } else { + panic!("Expected TemplateRendering error for file parameter with default"); } } } diff --git a/crates/goose/src/scheduler.rs b/crates/goose/src/scheduler.rs index 48b81d3e36..d688c23110 100644 --- a/crates/goose/src/scheduler.rs +++ b/crates/goose/src/scheduler.rs @@ -1348,6 +1348,60 @@ async fn run_scheduled_job_internal( Ok(session_id_for_return) } +#[async_trait] +impl SchedulerTrait for Scheduler { + async fn add_scheduled_job(&self, job: ScheduledJob) -> Result<(), SchedulerError> { + self.add_scheduled_job(job).await + } + + async fn list_scheduled_jobs(&self) -> Result, SchedulerError> { + Ok(self.list_scheduled_jobs().await) + } + + async fn remove_scheduled_job(&self, id: &str) -> Result<(), SchedulerError> { + self.remove_scheduled_job(id).await + } + + async fn pause_schedule(&self, id: &str) -> Result<(), SchedulerError> { + self.pause_schedule(id).await + } + + async fn unpause_schedule(&self, id: &str) -> Result<(), SchedulerError> { + self.unpause_schedule(id).await + } + + async fn run_now(&self, id: &str) -> Result { + self.run_now(id).await + } + + async fn sessions( + &self, + sched_id: &str, + limit: usize, + ) -> Result, SchedulerError> { + self.sessions(sched_id, limit).await + } + + async fn update_schedule( + &self, + sched_id: &str, + new_cron: String, + ) -> Result<(), SchedulerError> { + self.update_schedule(sched_id, new_cron).await + } + + async fn kill_running_job(&self, sched_id: &str) -> Result<(), SchedulerError> { + self.kill_running_job(sched_id).await + } + + async fn get_running_job_info( + &self, + sched_id: &str, + ) -> Result)>, SchedulerError> { + self.get_running_job_info(sched_id).await + } +} + #[cfg(test)] mod tests { use super::*; @@ -1527,57 +1581,3 @@ mod tests { Ok(()) } } - -#[async_trait] -impl SchedulerTrait for Scheduler { - async fn add_scheduled_job(&self, job: ScheduledJob) -> Result<(), SchedulerError> { - self.add_scheduled_job(job).await - } - - async fn list_scheduled_jobs(&self) -> Result, SchedulerError> { - Ok(self.list_scheduled_jobs().await) - } - - async fn remove_scheduled_job(&self, id: &str) -> Result<(), SchedulerError> { - self.remove_scheduled_job(id).await - } - - async fn pause_schedule(&self, id: &str) -> Result<(), SchedulerError> { - self.pause_schedule(id).await - } - - async fn unpause_schedule(&self, id: &str) -> Result<(), SchedulerError> { - self.unpause_schedule(id).await - } - - async fn run_now(&self, id: &str) -> Result { - self.run_now(id).await - } - - async fn sessions( - &self, - sched_id: &str, - limit: usize, - ) -> Result, SchedulerError> { - self.sessions(sched_id, limit).await - } - - async fn update_schedule( - &self, - sched_id: &str, - new_cron: String, - ) -> Result<(), SchedulerError> { - self.update_schedule(sched_id, new_cron).await - } - - async fn kill_running_job(&self, sched_id: &str) -> Result<(), SchedulerError> { - self.kill_running_job(sched_id).await - } - - async fn get_running_job_info( - &self, - sched_id: &str, - ) -> Result)>, SchedulerError> { - self.get_running_job_info(sched_id).await - } -} diff --git a/crates/goose/src/security/security_inspector.rs b/crates/goose/src/security/security_inspector.rs index 6c54d6df3f..d742c78543 100644 --- a/crates/goose/src/security/security_inspector.rs +++ b/crates/goose/src/security/security_inspector.rs @@ -130,7 +130,7 @@ mod tests { if inspector.is_enabled() { // If security is enabled, should detect the dangerous command assert!( - results.len() >= 1, + !results.is_empty(), "Security inspector should detect dangerous command when enabled" ); if !results.is_empty() { diff --git a/crates/goose/src/session/info.rs b/crates/goose/src/session/info.rs index b5fc56bca1..9ebedb1aee 100644 --- a/crates/goose/src/session/info.rs +++ b/crates/goose/src/session/info.rs @@ -97,41 +97,3 @@ pub fn get_valid_sorted_sessions(sort_order: SortOrder) -> Result Result<()> { - let test_cases = vec![ + let test_cases = [ // Case 1: Unclosed quotes ( r#"{"role":"user","content":[{"type":"text","text":"Hello there}]"#, @@ -1665,7 +1665,10 @@ mod tests { let file_path = dir.path().join("metadata.jsonl"); let mut metadata = SessionMetadata::default(); - metadata.description = "Description with\nnewline and \"quotes\" and 🦆".to_string(); + #[allow(clippy::field_reassign_with_default)] + { + metadata.description = "Description with\nnewline and \"quotes\" and 🦆".to_string(); + } let messages = Conversation::new_unvalidated(vec![Message::user().with_text("test")]); @@ -1702,7 +1705,7 @@ mod tests { let mut lines: Vec = contents.lines().map(String::from).collect(); lines[0] = lines[0].replace( &get_home_dir().to_string_lossy().into_owned(), - &invalid_dir.to_string_lossy().into_owned(), + &invalid_dir.to_string_lossy(), ); fs::write(&file_path, lines.join("\n"))?; diff --git a/crates/goose/src/tool_inspection.rs b/crates/goose/src/tool_inspection.rs index a80c597e4c..b2ed584c2c 100644 --- a/crates/goose/src/tool_inspection.rs +++ b/crates/goose/src/tool_inspection.rs @@ -274,30 +274,6 @@ mod tests { use mcp_core::ToolCall; use serde_json::json; - struct MockInspector { - name: &'static str, - results: Vec, - } - - #[async_trait] - impl ToolInspector for MockInspector { - fn name(&self) -> &'static str { - self.name - } - - fn as_any(&self) -> &dyn std::any::Any { - self - } - - async fn inspect( - &self, - _tool_requests: &[ToolRequest], - _messages: &[Message], - ) -> Result> { - Ok(self.results.clone()) - } - } - #[test] fn test_apply_inspection_results() { let tool_request = ToolRequest { diff --git a/crates/goose/src/tracing/observation_layer.rs b/crates/goose/src/tracing/observation_layer.rs index 08b7c7f302..81ebe21a8e 100644 --- a/crates/goose/src/tracing/observation_layer.rs +++ b/crates/goose/src/tracing/observation_layer.rs @@ -344,9 +344,10 @@ mod tests { use tokio::sync::mpsc; use tracing::dispatcher; + type Events = Arc>>; struct TestFixture { original_subscriber: Option, - events: Option>>>, + events: Option, } impl TestFixture { diff --git a/crates/goose/tests/agent.rs b/crates/goose/tests/agent.rs index 99ae07ad44..2dc81680a0 100644 --- a/crates/goose/tests/agent.rs +++ b/crates/goose/tests/agent.rs @@ -20,6 +20,7 @@ use goose::providers::{ enum ProviderType { Azure, OpenAi, + #[allow(dead_code)] Anthropic, Bedrock, Databricks, @@ -540,9 +541,7 @@ mod schedule_tool_tests { mod final_output_tool_tests { use super::*; use futures::stream; - use goose::agents::final_output_tool::{ - FINAL_OUTPUT_CONTINUATION_MESSAGE, FINAL_OUTPUT_TOOL_NAME, - }; + use goose::agents::final_output_tool::FINAL_OUTPUT_TOOL_NAME; use goose::conversation::Conversation; use goose::providers::base::MessageStream; use goose::recipe::Response; @@ -1151,7 +1150,7 @@ mod max_turns_tests { } assert!( - responses.len() >= 1, + !responses.is_empty(), "Expected at least 1 response, got {}", responses.len() ); diff --git a/crates/goose/tests/dynamic_task_tools_tests.rs b/crates/goose/tests/dynamic_task_tools_tests.rs index 286f1b4564..082a368eab 100644 --- a/crates/goose/tests/dynamic_task_tools_tests.rs +++ b/crates/goose/tests/dynamic_task_tools_tests.rs @@ -431,7 +431,7 @@ mod tests { assert!(recipe.extensions.is_some()); let extensions = recipe.extensions.unwrap(); // At minimum we should get the full config (stdio), shortname may not resolve - assert!(extensions.len() >= 1 && extensions.len() <= 2); + assert!(!extensions.is_empty() && extensions.len() <= 2); // The last one should always be the stdio config we provided if let Some(last) = extensions.last() { match last { diff --git a/crates/goose/tests/execution_tests.rs b/crates/goose/tests/execution_tests.rs index 2ba8fbce81..bb918f5737 100644 --- a/crates/goose/tests/execution_tests.rs +++ b/crates/goose/tests/execution_tests.rs @@ -57,9 +57,7 @@ mod execution_tests { async fn test_session_limit() { let manager = AgentManager::new(Some(3)).await.unwrap(); - let sessions: Vec<_> = (0..3) - .map(|i| String::from(format!("session-{}", i))) - .collect(); + let sessions: Vec<_> = (0..3).map(|i| format!("session-{}", i)).collect(); for session in &sessions { manager diff --git a/crates/goose/tests/mcp_integration_test.rs b/crates/goose/tests/mcp_integration_test.rs index 820dbaa6ac..787d49c015 100644 --- a/crates/goose/tests/mcp_integration_test.rs +++ b/crates/goose/tests/mcp_integration_test.rs @@ -156,6 +156,7 @@ async fn test_replayed_session( let extension_manager = ExtensionManager::new(); + #[allow(clippy::redundant_closure_call)] let result = (async || -> Result<(), Box> { extension_manager.add_extension(extension_config).await?; diff --git a/crates/goose/tests/test_support.rs b/crates/goose/tests/test_support.rs index 6b02870898..feea692d6a 100644 --- a/crates/goose/tests/test_support.rs +++ b/crates/goose/tests/test_support.rs @@ -29,10 +29,17 @@ pub struct ConfigurableMockScheduler { running_jobs: Arc>>, call_log: Arc>>, behaviors: Arc>>, + #[allow(clippy::type_complexity)] sessions_data: Arc>>>, } #[allow(dead_code)] +impl Default for ConfigurableMockScheduler { + fn default() -> Self { + Self::new() + } +} + impl ConfigurableMockScheduler { pub fn new() -> Self { Self { @@ -44,36 +51,6 @@ impl ConfigurableMockScheduler { } } - pub async fn with_behavior(self, method: &str, behavior: MockBehavior) -> Self { - self.behaviors - .lock() - .await - .insert(method.to_string(), behavior); - self - } - - pub async fn with_existing_job(self, job: ScheduledJob) -> Self { - self.jobs.lock().await.insert(job.id.clone(), job); - self - } - - pub async fn with_running_job(self, job_id: &str) -> Self { - self.running_jobs.lock().await.insert(job_id.to_string()); - self - } - - pub async fn with_sessions_data( - self, - job_id: &str, - sessions: Vec<(String, SessionMetadata)>, - ) -> Self { - self.sessions_data - .lock() - .await - .insert(job_id.to_string(), sessions); - self - } - pub async fn get_calls(&self) -> Vec { self.call_log.lock().await.clone() } @@ -337,6 +314,12 @@ pub struct ScheduleToolTestBuilder { scheduler: Arc, } +impl Default for ScheduleToolTestBuilder { + fn default() -> Self { + Self::new() + } +} + impl ScheduleToolTestBuilder { pub fn new() -> Self { Self { diff --git a/crates/goose/tests/todo_session_integration.rs b/crates/goose/tests/todo_session_integration.rs index dcc0402cea..c63ec5f0d1 100644 --- a/crates/goose/tests/todo_session_integration.rs +++ b/crates/goose/tests/todo_session_integration.rs @@ -11,7 +11,6 @@ use goose::session::storage::SessionMetadata; use rmcp::model::Tool; use std::sync::Arc; use tempfile::TempDir; -use tokio; use uuid::Uuid; // Mock provider implementation for testing @@ -298,7 +297,7 @@ async fn test_todo_clear_removes_from_session() { .unwrap(); // Consume the stream - while let Some(_) = stream.next().await {} + while (stream.next().await).is_some() {} // With mock provider, the TODO won't actually be cleared via tool calls // but we can verify the structure is correct @@ -463,9 +462,12 @@ async fn test_todo_update_preserves_other_metadata() { // Set initial metadata with various fields let mut metadata = SessionMetadata::default(); - metadata.message_count = 5; - metadata.description = "Test session".to_string(); - metadata.total_tokens = Some(1000); + #[allow(clippy::field_reassign_with_default)] + { + metadata.message_count = 5; + metadata.description = "Test session".to_string(); + metadata.total_tokens = Some(1000); + } let todo_state = TodoState::new("Initial TODO".to_string()); todo_state .to_extension_data(&mut metadata.extension_data) diff --git a/scripts/clippy-lint.sh b/scripts/clippy-lint.sh index d44b669df4..afc9296deb 100755 --- a/scripts/clippy-lint.sh +++ b/scripts/clippy-lint.sh @@ -14,11 +14,11 @@ echo "🔍 Running all clippy checks..." # Run standard clippy with strict warnings echo " → Standard clippy rules (strict)" -cargo clippy --jobs 2 -- -D warnings +cargo clippy --all-targets --jobs 2 -- -D warnings # Run baseline rules check echo "" check_all_baseline_rules echo "" -echo "✅ All lint checks passed!" \ No newline at end of file +echo "✅ All lint checks passed!" diff --git a/ui/desktop/src/components/settings/extensions/deeplink.ts b/ui/desktop/src/components/settings/extensions/deeplink.ts index ffe1f6f74d..308dbf2d34 100644 --- a/ui/desktop/src/components/settings/extensions/deeplink.ts +++ b/ui/desktop/src/components/settings/extensions/deeplink.ts @@ -13,7 +13,16 @@ function getStdioConfig( timeout: number ) { // Validate that the command is one of the allowed commands - const allowedCommands = ['cu', 'docker', 'jbang', 'npx', 'uvx', 'goosed', 'npx.cmd', 'i-ching-mcp-server']; + const allowedCommands = [ + 'cu', + 'docker', + 'jbang', + 'npx', + 'uvx', + 'goosed', + 'npx.cmd', + 'i-ching-mcp-server', + ]; if (!allowedCommands.includes(cmd)) { toastService.handleError( 'Invalid Command',