Resolve sub recipe path relative to the parent recipe path (#3642)

crates/goose-cli/src/recipes/extract_from_cli.rs

@@ -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());
@@ -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();
@@ -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!(

crates/goose/src/recipe/build_recipe/mod.rs

@@ -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 {
@@ -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 })?;
@@ -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)
+            {
+                sub_recipe.path = resolved_path;
+            }
+        }
+    }
+
     Ok(recipe)
 }
 
@@ -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;

crates/goose/src/recipe/build_recipe/tests.rs

@@ -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;
@@ -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");
@@ -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()
+            );
+        }
+    }
 }