-
Notifications
You must be signed in to change notification settings - Fork 2.8k
fix: subrecipe relative path with summon #7295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,7 +10,9 @@ use crate::state::AppState; | |||||||||||||||||||||||||||
| use anyhow::Result; | ||||||||||||||||||||||||||||
| use axum::http::StatusCode; | ||||||||||||||||||||||||||||
| use goose::agents::Agent; | ||||||||||||||||||||||||||||
| use goose::recipe::build_recipe::{build_recipe_from_template, RecipeError}; | ||||||||||||||||||||||||||||
| use goose::recipe::build_recipe::{ | ||||||||||||||||||||||||||||
| build_recipe_from_template, resolve_sub_recipe_path, RecipeError, | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
| use goose::recipe::local_recipes::{get_recipe_library_dir, list_local_recipes}; | ||||||||||||||||||||||||||||
| use goose::recipe::validate_recipe::validate_recipe_template_from_content; | ||||||||||||||||||||||||||||
| use goose::recipe::Recipe; | ||||||||||||||||||||||||||||
|
|
@@ -44,13 +46,23 @@ pub fn short_id_from_path(path: &str) -> String { | |||||||||||||||||||||||||||
| pub fn get_all_recipes_manifests() -> Result<Vec<RecipeManifest>> { | ||||||||||||||||||||||||||||
| let recipes_with_path = list_local_recipes()?; | ||||||||||||||||||||||||||||
| let mut recipe_manifests_with_path = Vec::new(); | ||||||||||||||||||||||||||||
| for (file_path, recipe) in recipes_with_path { | ||||||||||||||||||||||||||||
| for (file_path, mut recipe) in recipes_with_path { | ||||||||||||||||||||||||||||
| let Ok(last_modified) = fs::metadata(file_path.clone()) | ||||||||||||||||||||||||||||
| .map(|m| chrono::DateTime::<chrono::Utc>::from(m.modified().unwrap()).to_rfc3339()) | ||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||
| continue; | ||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if let Some(recipe_dir) = file_path.parent() { | ||||||||||||||||||||||||||||
| if let Some(ref mut sub_recipes) = recipe.sub_recipes { | ||||||||||||||||||||||||||||
| for sr in sub_recipes.iter_mut() { | ||||||||||||||||||||||||||||
| if let Ok(resolved) = resolve_sub_recipe_path(&sr.path, recipe_dir) { | ||||||||||||||||||||||||||||
| sr.path = resolved; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
Comment on lines
+56
to
+64
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| let manifest_with_path = RecipeManifest { | ||||||||||||||||||||||||||||
| id: short_id_from_path(file_path.to_string_lossy().as_ref()), | ||||||||||||||||||||||||||||
| recipe, | ||||||||||||||||||||||||||||
|
|
@@ -126,10 +138,22 @@ pub async fn get_recipe_file_path_by_id( | |||||||||||||||||||||||||||
| pub async fn load_recipe_by_id(state: &AppState, id: &str) -> Result<Recipe, ErrorResponse> { | ||||||||||||||||||||||||||||
| let path = get_recipe_file_path_by_id(state, id).await?; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Recipe::from_file_path(&path).map_err(|err| ErrorResponse { | ||||||||||||||||||||||||||||
| let mut recipe = Recipe::from_file_path(&path).map_err(|err| ErrorResponse { | ||||||||||||||||||||||||||||
| message: format!("Failed to load recipe: {}", err), | ||||||||||||||||||||||||||||
| status: StatusCode::INTERNAL_SERVER_ERROR, | ||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||
| })?; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if let Some(recipe_dir) = path.parent() { | ||||||||||||||||||||||||||||
| if let Some(ref mut sub_recipes) = recipe.sub_recipes { | ||||||||||||||||||||||||||||
| for sr in sub_recipes.iter_mut() { | ||||||||||||||||||||||||||||
| if let Ok(resolved) = resolve_sub_recipe_path(&sr.path, recipe_dir) { | ||||||||||||||||||||||||||||
| sr.path = resolved; | ||||||||||||||||||||||||||||
|
Comment on lines
+149
to
+150
|
||||||||||||||||||||||||||||
| if let Ok(resolved) = resolve_sub_recipe_path(&sr.path, recipe_dir) { | |
| sr.path = resolved; | |
| match resolve_sub_recipe_path(&sr.path, recipe_dir) { | |
| Ok(resolved) => { | |
| sr.path = resolved; | |
| } | |
| Err(err) => { | |
| eprintln!( | |
| "Warning: failed to resolve sub-recipe path '{}' relative to '{}': {err}", | |
| sr.path.display(), | |
| recipe_dir.display() | |
| ); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent error handling: when
resolve_sub_recipe_pathfails, the error is silently ignored. This could hide issues like missing sub-recipe files. Consider logging a warning when path resolution fails so issues are visible during debugging.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably ok in this case