From 15aa240f7dc0f0af300c81b1b67730c617f39bc8 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 13 May 2025 07:54:25 +1000 Subject: [PATCH] feat: specify parameters configuration in recipe file (#2444) --- crates/goose-cli/src/cli.rs | 2 +- crates/goose-cli/src/commands/recipe.rs | 2 +- crates/goose-cli/src/lib.rs | 1 - crates/goose-cli/src/recipe.rs | 125 ------- crates/goose-cli/src/recipes/mod.rs | 1 + crates/goose-cli/src/recipes/recipe.rs | 475 ++++++++++++++++++++++++ crates/goose/src/recipe/mod.rs | 41 ++ 7 files changed, 519 insertions(+), 128 deletions(-) delete mode 100644 crates/goose-cli/src/recipe.rs create mode 100644 crates/goose-cli/src/recipes/recipe.rs diff --git a/crates/goose-cli/src/cli.rs b/crates/goose-cli/src/cli.rs index b0833654..8cce4d8f 100644 --- a/crates/goose-cli/src/cli.rs +++ b/crates/goose-cli/src/cli.rs @@ -10,7 +10,7 @@ use crate::commands::mcp::run_server; use crate::commands::recipe::{handle_deeplink, handle_validate}; use crate::commands::session::{handle_session_list, handle_session_remove}; use crate::logging::setup_logging; -use crate::recipe::load_recipe; +use crate::recipes::recipe::load_recipe; use crate::session; use crate::session::{build_session, SessionBuilderConfig}; use goose_bench::bench_config::BenchRunConfig; diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index f2bdf52e..57736b76 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -2,7 +2,7 @@ use anyhow::Result; use base64::Engine; use console::style; -use crate::recipe::load_recipe; +use crate::recipes::recipe::load_recipe; /// Validates a recipe file /// diff --git a/crates/goose-cli/src/lib.rs b/crates/goose-cli/src/lib.rs index 2cf95d06..148f2300 100644 --- a/crates/goose-cli/src/lib.rs +++ b/crates/goose-cli/src/lib.rs @@ -3,7 +3,6 @@ use once_cell::sync::Lazy; pub mod cli; pub mod commands; pub mod logging; -pub mod recipe; pub mod recipes; pub mod session; pub mod signal; diff --git a/crates/goose-cli/src/recipe.rs b/crates/goose-cli/src/recipe.rs deleted file mode 100644 index 44a1462e..00000000 --- a/crates/goose-cli/src/recipe.rs +++ /dev/null @@ -1,125 +0,0 @@ -use anyhow::Result; -use console::style; - -use goose::recipe::Recipe; -use minijinja::UndefinedBehavior; -use serde_json::Value as JsonValue; -use serde_yaml::Value as YamlValue; -use std::collections::HashMap; - -use crate::recipes::search_recipe::retrieve_recipe_file; - -/// Loads and validates a recipe from a YAML or JSON file -/// -/// # Arguments -/// -/// * `path` - Path to the recipe file (YAML or JSON) -/// * `log` - whether to log information about the recipe or not -/// * `params` - optional parameters to render the recipe with -/// -/// # Returns -/// -/// The parsed recipe struct if successful -/// -/// # Errors -/// -/// Returns an error if: -/// - The file doesn't exist -/// - The file can't be read -/// - The YAML/JSON is invalid -/// - The required fields are missing -pub fn load_recipe( - recipe_name: &str, - log: bool, - params: Option>, -) -> Result { - let content = retrieve_recipe_file(recipe_name)?; - - // Check if any parameters were provided - let rendered_content = match params { - None => content, - Some(params) => render_content_with_params(&content, ¶ms)?, - }; - - let recipe: Recipe; - if serde_json::from_str::(&rendered_content).is_ok() { - recipe = serde_json::from_str(&rendered_content)? - } else if serde_yaml::from_str::(&rendered_content).is_ok() { - recipe = serde_yaml::from_str(&rendered_content)? - } else { - return Err(anyhow::anyhow!( - "Unsupported file format for recipe file. Expected .yaml or .json" - )); - } - - if log { - // Display information about the loaded recipe - println!( - "{} {}", - style("Loading recipe:").green().bold(), - style(&recipe.title).green() - ); - println!("{} {}", style("Description:").dim(), &recipe.description); - - println!(); // Add a blank line for spacing - } - - Ok(recipe) -} - -fn render_content_with_params(content: &str, params: &[(String, String)]) -> Result { - // Turn params into HashMap - let param_map: HashMap = params.iter().cloned().collect(); - - // Create a minijinja environment and context - let mut env = minijinja::Environment::new(); - env.set_undefined_behavior(UndefinedBehavior::Strict); - let template = env.template_from_str(content) - .map_err(|_| anyhow::anyhow!("Failed to render recipe, please check if the recipe has proper syntax for variables: eg: {{ variable_name }}"))?; - - // Render the template with the parameters - template.render(param_map).map_err(|_| { - anyhow::anyhow!( - "Failed to render the recipe - please check if all required parameters are provided" - ) - }) -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_render_content_with_params() { - // Test basic parameter substitution - let content = "Hello {{ name }}!"; - let params = vec![("name".to_string(), "World".to_string())]; - let result = render_content_with_params(content, ¶ms).unwrap(); - assert_eq!(result, "Hello World!"); - - // Test multiple parameters - let content = "{{ greeting }} {{ name }}!"; - let params = vec![ - ("greeting".to_string(), "Hi".to_string()), - ("name".to_string(), "Alice".to_string()), - ]; - let result = render_content_with_params(content, ¶ms).unwrap(); - assert_eq!(result, "Hi Alice!"); - - // Test missing parameter results in error - let content = "Hello {{ missing }}!"; - let params = vec![]; - let err = render_content_with_params(content, ¶ms).unwrap_err(); - assert!(err - .to_string() - .contains("please check if all required parameters")); - - // Test invalid template syntax results in error - let content = "Hello {{ unclosed"; - let params = vec![]; - let err = render_content_with_params(content, ¶ms).unwrap_err(); - assert!(err - .to_string() - .contains("please check if the recipe has proper syntax")); - } -} diff --git a/crates/goose-cli/src/recipes/mod.rs b/crates/goose-cli/src/recipes/mod.rs index dba435f1..99056549 100644 --- a/crates/goose-cli/src/recipes/mod.rs +++ b/crates/goose-cli/src/recipes/mod.rs @@ -1,2 +1,3 @@ pub mod github_recipe; +pub mod recipe; pub mod search_recipe; diff --git a/crates/goose-cli/src/recipes/recipe.rs b/crates/goose-cli/src/recipes/recipe.rs new file mode 100644 index 00000000..e18b347e --- /dev/null +++ b/crates/goose-cli/src/recipes/recipe.rs @@ -0,0 +1,475 @@ +use anyhow::Result; +use console::style; + +use crate::recipes::search_recipe::retrieve_recipe_file; +use goose::recipe::{Recipe, RecipeParameter, RecipeParameterRequirement}; +use minijinja::{Environment, Error, Template, UndefinedBehavior}; +use serde_json::Value as JsonValue; +use serde_yaml::Value as YamlValue; +use std::collections::{HashMap, HashSet}; + +/// Loads and validates a recipe from a YAML or JSON file +/// +/// # Arguments +/// +/// * `path` - Path to the recipe file (YAML or JSON) +/// * `log` - whether to log information about the recipe or not +/// * `params` - optional parameters to render the recipe with +/// +/// # Returns +/// +/// The parsed recipe struct if successful +/// +/// # Errors +/// +/// Returns an error if: +/// - The file doesn't exist +/// - The file can't be read +/// - The YAML/JSON is invalid +/// - The required fields are missing +pub fn load_recipe( + recipe_name: &str, + log: bool, + params: Option>, +) -> Result { + let recipe_file_content = retrieve_recipe_file(recipe_name)?; + + let recipe_parameters = validate_recipe_file_parameters(&recipe_file_content)?; + + let (rendered_content, params_for_template) = if let Some(user_params) = params { + let params_for_template = apply_values_to_parameters(&user_params, recipe_parameters)?; + ( + render_content_with_params(&recipe_file_content, ¶ms_for_template)?, + Some(params_for_template), + ) + } else { + (recipe_file_content, None) + }; + + let recipe = parse_recipe_content(&rendered_content)?; + if log { + // Display information about the loaded recipe + println!( + "{} {}", + style("Loading recipe:").green().bold(), + style(&recipe.title).green() + ); + println!("{} {}", style("Description:").dim(), &recipe.description); + + if let Some(params) = params_for_template { + if !params.is_empty() { + println!("{}", style("Parameters:").dim()); + for (key, value) in params { + println!("{}: {}", key, value); + } + } + } + + println!(); // Add a blank line for spacing + } + + Ok(recipe) +} + +fn validate_recipe_file_parameters(recipe_file_content: &str) -> Result> { + let recipe_from_recipe_file: Recipe = parse_recipe_content(recipe_file_content)?; + validate_optional_parameters(&recipe_from_recipe_file)?; + validate_parameters_in_template(recipe_from_recipe_file, recipe_file_content) +} + +fn validate_parameters_in_template( + recipe: Recipe, + recipe_file_content: &str, +) -> Result> { + let template_variables = extract_template_variables(recipe_file_content)?; + + let param_keys: HashSet = recipe + .parameters + .as_ref() + .unwrap_or(&vec![]) + .iter() + .map(|p| p.key.clone()) + .collect(); + + let missing_keys = template_variables + .difference(¶m_keys) + .collect::>(); + + let extra_keys = param_keys + .difference(&template_variables) + .collect::>(); + + if missing_keys.is_empty() && extra_keys.is_empty() { + return Ok(recipe.parameters.unwrap_or_default()); + } + + let mut message = String::new(); + + if !missing_keys.is_empty() { + message.push_str(&format!( + "Missing definitions for parameters in the recipe file: {}.", + missing_keys + .iter() + .map(|s| s.to_string()) + .collect::>() + .join(", ") + )); + } + + if !extra_keys.is_empty() { + message.push_str(&format!( + "\nUnnecessary parameter definitions: {}.", + extra_keys + .iter() + .map(|s| s.to_string()) + .collect::>() + .join(", ") + )); + } + Err(anyhow::anyhow!("{}", message.trim_end())) +} + +fn validate_optional_parameters(recipe: &Recipe) -> Result<()> { + let optional_params_without_default_values: Vec = recipe + .parameters + .as_ref() + .unwrap_or(&vec![]) + .iter() + .filter(|p| { + matches!(p.requirement, RecipeParameterRequirement::Optional) && p.default.is_none() + }) + .map(|p| p.key.clone()) + .collect(); + + if optional_params_without_default_values.is_empty() { + Ok(()) + } else { + Err(anyhow::anyhow!("Optional parameters missing default values in the recipe: {}. Please provide defaults.", optional_params_without_default_values.join(", "))) + } +} + +fn parse_recipe_content(content: &str) -> Result { + if serde_json::from_str::(content).is_ok() { + Ok(serde_json::from_str(content)?) + } else if serde_yaml::from_str::(content).is_ok() { + Ok(serde_yaml::from_str(content)?) + } else { + Err(anyhow::anyhow!( + "Unsupported file format for recipe file. Expected .yaml or .json" + )) + } +} + +fn extract_template_variables(template_str: &str) -> Result> { + let mut env = Environment::new(); + env.set_undefined_behavior(UndefinedBehavior::Strict); + + let template = env + .template_from_str(template_str) + .map_err(|e: Error| anyhow::anyhow!("Invalid template syntax: {}", e.to_string()))?; + + Ok(template.undeclared_variables(true)) +} + +fn apply_values_to_parameters( + user_params: &[(String, String)], + recipe_parameters: Vec, +) -> Result> { + let mut param_map: HashMap = user_params.iter().cloned().collect(); + let mut missing_params: Vec = Vec::new(); + for param in recipe_parameters { + if !param_map.contains_key(¶m.key) { + match (¶m.default, ¶m.requirement) { + (Some(default), _) => param_map.insert(param.key.clone(), default.clone()), + (None, RecipeParameterRequirement::UserPrompt) => { + let input_value = cliclack::input(format!( + "Please enter {} ({})", + param.key, param.description + )) + .interact()?; + param_map.insert(param.key.clone(), input_value) + } + _ => { + missing_params.push(param.key.clone()); + None + } + }; + } + } + match missing_params.is_empty() { + true => Ok(param_map), + false => { + let formatted = missing_params + .iter() + .map(|key| format!("--params {}=your_value", key)) + .collect::>() + .join(" "); + + Err(anyhow::anyhow!( + "Please provide the following parameters in the command line: {}", + formatted + )) + } + } +} + +fn render_content_with_params(content: &str, params: &HashMap) -> Result { + // Create a minijinja environment and context + let mut env = minijinja::Environment::new(); + env.set_undefined_behavior(UndefinedBehavior::Strict); + let template: Template<'_, '_> = env.template_from_str(content) + .map_err(|e: Error| anyhow::anyhow!("Failed to render recipe {}, please check if the recipe has proper syntax for variables: eg: {{ variable_name }}", e.to_string()))?; + + // Render the template with the parameters + template.render(params).map_err(|e: Error| { + anyhow::anyhow!( + "Failed to render the recipe {} - please check if all required parameters are provided", + e.to_string() + ) + }) +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use goose::recipe::{RecipeParameterInputType, RecipeParameterRequirement}; + use tempfile::TempDir; + + use super::*; + + fn setup_recipe_file(instructions_and_parameters: &str) -> (TempDir, PathBuf) { + let recipe_content = format!( + r#"{{ + "version": "1.0.0", + "title": "Test Recipe", + "description": "A test recipe", + {} + }}"#, + instructions_and_parameters + ); + // Create a temporary file + let temp_dir = tempfile::tempdir().unwrap(); + let recipe_path: std::path::PathBuf = temp_dir.path().join("test_recipe.json"); + std::fs::write(&recipe_path, recipe_content).unwrap(); + (temp_dir, recipe_path) + } + + #[test] + fn test_render_content_with_params() { + // Test basic parameter substitution + let content = "Hello {{ name }}!"; + let mut params = HashMap::new(); + params.insert("name".to_string(), "World".to_string()); + let result = render_content_with_params(content, ¶ms).unwrap(); + assert_eq!(result, "Hello World!"); + + // Test multiple parameters + let content = "{{ greeting }} {{ name }}!"; + let mut params = HashMap::new(); + params.insert("greeting".to_string(), "Hi".to_string()); + params.insert("name".to_string(), "Alice".to_string()); + let result = render_content_with_params(content, ¶ms).unwrap(); + assert_eq!(result, "Hi Alice!"); + + // Test missing parameter results in error + let content = "Hello {{ missing }}!"; + let params = HashMap::new(); + let err = render_content_with_params(content, ¶ms).unwrap_err(); + assert!(err + .to_string() + .contains("please check if all required parameters")); + + // Test invalid template syntax results in error + let content = "Hello {{ unclosed"; + let params = HashMap::new(); + let err = render_content_with_params(content, ¶ms).unwrap_err(); + assert!(err + .to_string() + .contains("please check if the recipe has proper syntax")); + } + + #[test] + fn test_load_recipe_success() { + let instructions_and_parameters = r#" + "instructions": "Test instructions with {{ my_name }}", + "parameters": [ + { + "key": "my_name", + "input_type": "string", + "requirement": "required", + "description": "A test parameter" + } + ]"#; + + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + + let params = vec![("my_name".to_string(), "value".to_string())]; + let recipe = load_recipe(recipe_path.to_str().unwrap(), false, Some(params)).unwrap(); + + assert_eq!(recipe.title, "Test Recipe"); + assert_eq!(recipe.description, "A test recipe"); + assert_eq!(recipe.instructions.unwrap(), "Test instructions with value"); + // Verify parameters match recipe definition + 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_load_recipe_success_variable_in_prompt() { + let instructions_and_parameters = r#" + "instructions": "Test instructions", + "prompt": "My prompt {{ my_name }}", + "parameters": [ + { + "key": "my_name", + "input_type": "string", + "requirement": "required", + "description": "A test parameter" + } + ]"#; + + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + + let params = vec![("my_name".to_string(), "value".to_string())]; + let recipe = load_recipe(recipe_path.to_str().unwrap(), false, Some(params)).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"); + } + + #[test] + fn test_load_recipe_wrong_parameters_in_recipe_file() { + let instructions_and_parameters = r#" + "instructions": "Test instructions with {{ expected_param1 }} {{ expected_param2 }}", + "parameters": [ + { + "key": "wrong_param_key", + "input_type": "string", + "requirement": "required", + "description": "A test parameter" + } + ]"#; + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + + let load_recipe_result = load_recipe(recipe_path.to_str().unwrap(), false, None); + assert!(load_recipe_result.is_err()); + let err = load_recipe_result.unwrap_err(); + println!("{}", err.to_string()); + assert!(err + .to_string() + .contains("Unnecessary parameter definitions: wrong_param_key.")); + assert!(err + .to_string() + .contains("Missing definitions for parameters in the recipe file:")); + assert!(err.to_string().contains("expected_param1")); + assert!(err.to_string().contains("expected_param2")); + } + + #[test] + fn test_load_recipe_with_default_values_in_recipe_file() { + let instructions_and_parameters = r#" + "instructions": "Test instructions with {{ param_with_default }} {{ param_without_default }}", + "parameters": [ + { + "key": "param_with_default", + "input_type": "string", + "requirement": "optional", + "default": "my_default_value", + "description": "A test parameter" + }, + { + "key": "param_without_default", + "input_type": "string", + "requirement": "required", + "description": "A test parameter" + } + ]"#; + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + let params = vec![("param_without_default".to_string(), "value1".to_string())]; + + let recipe = load_recipe(recipe_path.to_str().unwrap(), false, Some(params)).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" + ); + } + + #[test] + fn test_load_recipe_optional_parameters_without_default_values_in_recipe_file() { + let instructions_and_parameters = r#" + "instructions": "Test instructions with {{ optional_param }}", + "parameters": [ + { + "key": "optional_param", + "input_type": "string", + "requirement": "optional", + "description": "A test parameter" + } + ]"#; + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + + let load_recipe_result = load_recipe(recipe_path.to_str().unwrap(), false, None); + assert!(load_recipe_result.is_err()); + let err = load_recipe_result.unwrap_err(); + println!("{}", err.to_string()); + assert!(err.to_string().contains( + "Optional parameters missing default values in the recipe: optional_param. Please provide defaults." + )); + } + + #[test] + fn test_load_recipe_wrong_input_type_in_recipe_file() { + let instructions_and_parameters = r#" + "instructions": "Test instructions with {{ param }}", + "parameters": [ + { + "key": "param", + "input_type": "some_invalid_type", + "requirement": "required", + "description": "A test parameter" + } + ]"#; + let params = vec![("param".to_string(), "value".to_string())]; + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + + let load_recipe_result = load_recipe(recipe_path.to_str().unwrap(), false, Some(params)); + assert!(load_recipe_result.is_err()); + let err = load_recipe_result.unwrap_err(); + assert!(err.to_string().contains("unknown variant `some_invalid_type`, expected one of `string`, `number`, `date`, `file`")); + } + + #[test] + fn test_load_recipe_success_without_parameters() { + let instructions_and_parameters = r#" + "instructions": "Test instructions" + "#; + let (_temp_dir, recipe_path) = setup_recipe_file(instructions_and_parameters); + + let load_recipe_result = load_recipe(recipe_path.to_str().unwrap(), false, None); + assert!(load_recipe_result.is_ok()); + let recipe = load_recipe_result.unwrap(); + assert_eq!(recipe.instructions.unwrap(), "Test instructions"); + assert!(recipe.parameters.is_none()); + } +} diff --git a/crates/goose/src/recipe/mod.rs b/crates/goose/src/recipe/mod.rs index 51c25ef1..3eb97880 100644 --- a/crates/goose/src/recipe/mod.rs +++ b/crates/goose/src/recipe/mod.rs @@ -22,6 +22,7 @@ fn default_version() -> String { /// * `context` - Supplementary context information for the Recipe /// * `activities` - Activity labels that appear when loading the Recipe /// * `author` - Information about the Recipe's creator and metadata +/// * `parameters` - Additional parameters for the Recipe /// /// # Example /// @@ -47,6 +48,7 @@ fn default_version() -> String { /// context: None, /// activities: None, /// author: None, +/// parameters: None, /// }; /// ``` #[derive(Serialize, Deserialize, Debug)] @@ -78,6 +80,9 @@ pub struct Recipe { #[serde(skip_serializing_if = "Option::is_none")] pub author: Option, // any additional author information + + #[serde(skip_serializing_if = "Option::is_none")] + pub parameters: Option>, // any additional parameters for the recipe } #[derive(Serialize, Deserialize, Debug)] @@ -89,6 +94,33 @@ pub struct Author { pub metadata: Option, // any additional metadata for the author } +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "snake_case")] +pub enum RecipeParameterRequirement { + Required, + Optional, + UserPrompt, +} + +#[derive(Serialize, Deserialize, Debug)] +#[serde(rename_all = "snake_case")] +pub enum RecipeParameterInputType { + String, + Number, + Date, + File, +} + +#[derive(Serialize, Deserialize, Debug)] +pub struct RecipeParameter { + pub key: String, + pub input_type: RecipeParameterInputType, + pub requirement: RecipeParameterRequirement, + pub description: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub default: Option, +} + /// Builder for creating Recipe instances pub struct RecipeBuilder { // Required fields with default values @@ -103,6 +135,7 @@ pub struct RecipeBuilder { context: Option>, activities: Option>, author: Option, + parameters: Option>, } impl Recipe { @@ -131,6 +164,7 @@ impl Recipe { context: None, activities: None, author: None, + parameters: None, } } } @@ -189,6 +223,12 @@ impl RecipeBuilder { self } + /// Sets the parameters for the Recipe + pub fn parameters(mut self, parameters: Vec) -> Self { + self.parameters = Some(parameters); + self + } + /// Builds the Recipe instance /// /// Returns an error if any required fields are missing @@ -210,6 +250,7 @@ impl RecipeBuilder { context: self.context, activities: self.activities, author: self.author, + parameters: self.parameters, }) } }