Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions crates/goose-cli/src/recipes/extract_from_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,13 @@ mod tests {
assert!(sub_recipes.is_some());
let sub_recipes = sub_recipes.unwrap();
assert!(sub_recipes.len() == 1);
assert_eq!(sub_recipes[0].path, "existing_sub_recipe.yaml".to_string());
let full_sub_recipe_path = recipe_path
.parent()
.unwrap()
.join("existing_sub_recipe.yaml")
.to_string_lossy()
.to_string();
assert_eq!(sub_recipes[0].path, full_sub_recipe_path);
assert_eq!(sub_recipes[0].name, "existing_sub_recipe".to_string());
assert!(sub_recipes[0].values.is_none());
assert!(response.is_some());
Expand All @@ -135,14 +141,13 @@ mod tests {

#[test]
fn test_extract_recipe_info_from_cli_with_additional_sub_recipes() {
let (_temp_dir, recipe_path) = create_recipe();
let (temp_dir, recipe_path) = create_recipe();

// Create actual sub-recipe files in the temp directory
std::fs::create_dir_all(_temp_dir.path().join("path/to")).unwrap();
std::fs::create_dir_all(_temp_dir.path().join("another")).unwrap();
std::fs::create_dir_all(temp_dir.path().join("path/to")).unwrap();
std::fs::create_dir_all(temp_dir.path().join("another")).unwrap();

let sub_recipe1_path = _temp_dir.path().join("path/to/sub_recipe1.yaml");
let sub_recipe2_path = _temp_dir.path().join("another/sub_recipe2.yaml");
let sub_recipe1_path = temp_dir.path().join("path/to/sub_recipe1.yaml");
let sub_recipe2_path = temp_dir.path().join("another/sub_recipe2.yaml");

std::fs::write(&sub_recipe1_path, "title: Sub Recipe 1").unwrap();
std::fs::write(&sub_recipe2_path, "title: Sub Recipe 2").unwrap();
Expand Down Expand Up @@ -176,7 +181,13 @@ mod tests {
assert!(sub_recipes.is_some());
let sub_recipes = sub_recipes.unwrap();
assert!(sub_recipes.len() == 3);
assert_eq!(sub_recipes[0].path, "existing_sub_recipe.yaml".to_string());
let full_sub_recipe_path = recipe_path
.parent()
.unwrap()
.join("existing_sub_recipe.yaml")
.to_string_lossy()
.to_string();
assert_eq!(sub_recipes[0].path, full_sub_recipe_path);
assert_eq!(sub_recipes[0].name, "existing_sub_recipe".to_string());
assert!(sub_recipes[0].values.is_none());
assert_eq!(
Expand Down
33 changes: 32 additions & 1 deletion crates/goose/src/recipe/build_recipe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::recipe::{
};
use anyhow::Result;
use std::collections::{HashMap, HashSet};
use std::path::Path;

#[derive(Debug, thiserror::Error)]
pub enum RecipeError {
Expand Down Expand Up @@ -66,6 +67,7 @@ pub fn build_recipe_from_template<F>(
where
F: Fn(&str, &str) -> Result<String, anyhow::Error>,
{
let recipe_parent_dir = recipe_file.parent_dir.clone();
let (rendered_content, missing_params) =
render_recipe_template(recipe_file, params.clone(), user_prompt_fn)
.map_err(|source| RecipeError::TemplateRendering { source })?;
Expand All @@ -76,8 +78,18 @@ where
});
}

let recipe = Recipe::from_content(&rendered_content)
let mut recipe = Recipe::from_content(&rendered_content)
.map_err(|source| RecipeError::RecipeParsing { source })?;

if let Some(ref mut sub_recipes) = recipe.sub_recipes {
for sub_recipe in sub_recipes {
if let Ok(resolved_path) = resolve_sub_recipe_path(&sub_recipe.path, &recipe_parent_dir)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure when this would fail, but if it did shouldn't we fail the entire operation rather than have a subrecipe that has an unresolved path?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire operation will fail when a subrecipe has an unresolved path as it throws RecipeParsing error

{
sub_recipe.path = resolved_path;
}
}
}

Ok(recipe)
}

Expand Down Expand Up @@ -185,5 +197,24 @@ where
Ok((param_map, missing_params))
}

fn resolve_sub_recipe_path(
sub_recipe_path: &str,
parent_recipe_dir: &Path,
) -> Result<String, RecipeError> {
let path = if Path::new(sub_recipe_path).is_absolute() {
sub_recipe_path.to_string()
} else {
parent_recipe_dir
.join(sub_recipe_path)
.to_str()
.ok_or_else(|| RecipeError::RecipeParsing {
source: anyhow::anyhow!("Invalid sub-recipe path: {}", sub_recipe_path),
})?
.to_string()
};

Ok(path)
}

#[cfg(test)]
mod tests;
94 changes: 92 additions & 2 deletions crates/goose/src/recipe/build_recipe/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#[cfg(test)]
mod tests {
use crate::recipe::build_recipe::{build_recipe_from_template, RecipeError};
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;
Expand Down Expand Up @@ -80,7 +82,6 @@ mod tests {
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");
Expand Down Expand Up @@ -349,4 +350,93 @@ mod tests {
"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
}

#[test]
fn test_resolve_sub_recipe_path_relative() {
let temp_dir = tempfile::tempdir().unwrap();
let parent_dir = temp_dir.path();

let result = resolve_sub_recipe_path("./sub-recipes/child.yaml", parent_dir);
assert!(result.is_ok());

let expected_path = parent_dir.join("./sub-recipes/child.yaml");
assert_eq!(result.unwrap(), expected_path.to_str().unwrap());
}

#[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#"
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#"{
"version": "1.0.0",
"title": "Main Recipe",
"description": "Main recipe with sub-recipe",
"instructions": "Main instructions",
"sub_recipes": [
{
"name": "child",
"path": "./sub-recipes/child.yaml"
}
]
}"#;
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 =
build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT).unwrap();

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 expected_absolute_path = temp_path.join("./sub-recipes/child.yaml");
assert_eq!(
sub_recipes[0].path,
expected_absolute_path.to_str().unwrap()
);
}
}
}
Loading