diff --git a/crates/goose/src/agents/platform_extensions/summon.rs b/crates/goose/src/agents/platform_extensions/summon.rs index 1304cc676c16..fbc577b60c42 100644 --- a/crates/goose/src/agents/platform_extensions/summon.rs +++ b/crates/goose/src/agents/platform_extensions/summon.rs @@ -715,17 +715,10 @@ impl SummonClient { ) -> Result, String> { let sources = self.get_sources(session_id, working_dir).await; - if let Some(mut source) = sources.iter().find(|s| s.name == name).cloned() { - if source.source_type == SourceType::Subrecipe && source.content.is_empty() { - source.content = self.load_subrecipe_content(session_id, &source.name).await; - } - return Ok(Some(source)); - } - - Ok(None) + Ok(sources.iter().find(|s| s.name == name).cloned()) } - async fn load_subrecipe_content(&self, session_id: &str, name: &str) -> String { + async fn load_subrecipe_content(&self, session_id: &str, name: &str) -> Result { let session = match self .context .session_manager @@ -733,37 +726,38 @@ impl SummonClient { .await { Ok(s) => s, - Err(_) => return String::new(), + Err(_) => return Ok(String::new()), }; let sub_recipes = match session.recipe.as_ref().and_then(|r| r.sub_recipes.as_ref()) { Some(sr) => sr, - None => return String::new(), + None => return Ok(String::new()), }; let sr = match sub_recipes.iter().find(|sr| sr.name == name) { Some(sr) => sr, - None => return String::new(), + None => return Ok(String::new()), }; match load_local_recipe_file(&sr.path) { - Ok(recipe_file) => match Recipe::from_content(&recipe_file.content) { - Ok(recipe) => { - let mut content = recipe.instructions.unwrap_or_default(); - if let Some(params) = &recipe.parameters { - if !params.is_empty() { - content.push_str("\n\n"); - content.push_str(&Self::format_parameters(params)); - } - } - content - } - Err(_) => recipe_file.content, - }, - Err(_) => String::new(), + Ok(recipe_file) => Self::format_subrecipe_content(name, &recipe_file.content), + Err(_) => Ok(String::new()), } } + fn format_subrecipe_content(name: &str, raw_content: &str) -> Result { + let recipe = Recipe::from_content(raw_content) + .map_err(|_| format!("Subrecipe '{}' is not a valid recipe", name))?; + let mut content = recipe.instructions.unwrap_or_default(); + if let Some(params) = &recipe.parameters { + if !params.is_empty() { + content.push_str("\n\n"); + content.push_str(&Self::format_parameters(params)); + } + } + Ok(content) + } + fn discover_filesystem_sources(&self, working_dir: &Path) -> Vec { discover_filesystem_sources(working_dir) } @@ -1200,7 +1194,12 @@ impl SummonClient { let source = self.resolve_source(session_id, name, working_dir).await?; match source { - Some(source) => { + Some(mut source) => { + if source.source_type == SourceType::Subrecipe && source.content.is_empty() { + source.content = self + .load_subrecipe_content(session_id, &source.name) + .await?; + } let content = source.to_load_text(); let output = format!( @@ -2340,6 +2339,37 @@ You review code."#; assert!(text.contains("now available in your context")); } + #[test] + fn test_invalid_external_subrecipe_content_is_not_returned() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("invalid.yaml"); + fs::write(&path, "api_key: SUPERSECRET\n").unwrap(); + + let recipe_file = load_local_recipe_file(path.to_str().unwrap()).unwrap(); + let error = + SummonClient::format_subrecipe_content("invalid", &recipe_file.content).unwrap_err(); + + assert_eq!(error, "Subrecipe 'invalid' is not a valid recipe"); + assert!(!error.contains("SUPERSECRET")); + } + + #[test] + fn test_valid_external_subrecipe_content_still_loads() { + let temp_dir = TempDir::new().unwrap(); + let path = temp_dir.path().join("child.yaml"); + fs::write( + &path, + "title: Child\ndescription: External child\ninstructions: Run child steps", + ) + .unwrap(); + + let recipe_file = load_local_recipe_file(path.to_str().unwrap()).unwrap(); + let content = + SummonClient::format_subrecipe_content("child", &recipe_file.content).unwrap(); + + assert_eq!(content, "Run child steps"); + } + #[tokio::test] async fn test_load_agent_source() { let temp_dir = TempDir::new().unwrap();