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
6 changes: 3 additions & 3 deletions crates/goose-server/src/routes/recipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use axum::routing::get;
use axum::{extract::State, http::StatusCode, routing::post, Json, Router};
use goose::recipe::local_recipes;
use goose::recipe::validate_recipe::validate_recipe_template_from_content;
use goose::recipe::Recipe;
use goose::recipe::{strip_error_location, Recipe};
use goose::{recipe_deeplink, slash_commands};

use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -443,7 +443,7 @@ async fn save_recipe(
payload: Result<Json<Value>, JsonRejection>,
) -> Result<Json<SaveRecipeResponse>, ErrorResponse> {
let Json(raw_json) = payload.map_err(json_rejection_to_error_response)?;
let request = deserialize_save_recipe_request(raw_json)?;
let request: SaveRecipeRequest = deserialize_save_recipe_request(raw_json)?;
let has_security_warnings = request.recipe.check_for_security_warnings();
if has_security_warnings {
return Err(ErrorResponse {
Expand Down Expand Up @@ -492,7 +492,7 @@ fn deserialize_save_recipe_request(value: Value) -> Result<SaveRecipeRequest, Er
let result: Result<SaveRecipeRequest, _> = deserialize_with_path(&mut deserializer);
result.map_err(|err| {
let path = err.path().to_string();
let inner = err.into_inner();
let inner = strip_error_location(&err.into_inner().to_string());
let message = if path.is_empty() {
format!("Save recipe validation failed: {}", inner)
} else {
Expand Down
65 changes: 65 additions & 0 deletions crates/goose/src/agents/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,68 @@ pub struct SessionConfig {
#[serde(skip_serializing_if = "Option::is_none")]
pub retry_config: Option<RetryConfig>,
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_retry_config_validate_success() {
let config = RetryConfig {
max_retries: 3,
checks: vec![],
on_failure: None,
timeout_seconds: Some(60),
on_failure_timeout_seconds: Some(120),
};
assert!(config.validate().is_ok());
}

#[test]
fn test_retry_config_validate_max_retries_zero() {
let config = RetryConfig {
max_retries: 0,
checks: vec![],
on_failure: None,
timeout_seconds: None,
on_failure_timeout_seconds: None,
};
let result = config.validate();
assert!(result.is_err());
assert_eq!(result.unwrap_err(), "max_retries must be greater than 0");
}

#[test]
fn test_retry_config_validate_timeout_zero() {
let config = RetryConfig {
max_retries: 3,
checks: vec![],
on_failure: None,
timeout_seconds: Some(0),
on_failure_timeout_seconds: None,
};
let result = config.validate();
assert!(result.is_err());
assert_eq!(
result.unwrap_err(),
"timeout_seconds must be greater than 0 if specified"
);
}

#[test]
fn test_retry_config_validate_on_failure_timeout_zero() {
let config = RetryConfig {
max_retries: 3,
checks: vec![],
on_failure: None,
timeout_seconds: None,
on_failure_timeout_seconds: Some(0),
};
let result = config.validate();
assert!(result.is_err());
assert_eq!(
result.unwrap_err(),
"on_failure_timeout_seconds must be greater than 0 if specified"
);
}
}
26 changes: 26 additions & 0 deletions crates/goose/src/recipe/build_recipe/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,3 +638,29 @@ parameters:
}
}
}

#[test]
fn test_build_recipe_from_template_invalid_retry_config() {
let instructions_and_parameters = r#"
"instructions": "Test instructions",
"retry": {
"max_retries": 0,
"checks": []
}"#;
let (_temp_dir, recipe_content, recipe_dir) = setup_recipe_file(instructions_and_parameters);

let build_recipe_result =
build_recipe_from_template(recipe_content, &recipe_dir, Vec::new(), NO_USER_PROMPT);
assert!(build_recipe_result.is_err());
let err = build_recipe_result.unwrap_err();

match err {
RecipeError::TemplateRendering { source } => {
assert_eq!(
source.to_string(),
"Invalid retry configuration: max_retries must be greater than 0"
);
}
_ => panic!("Expected TemplateRendering error, got: {:?}", err),
}
}
73 changes: 61 additions & 12 deletions crates/goose/src/recipe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ fn default_version() -> String {
"1.0.0".to_string()
}

/// Strips location information (e.g., "at line X column Y") from error messages
/// to make them more user-friendly for UI display.
pub fn strip_error_location(error_msg: &str) -> String {
error_msg
.split(" at line")
.next()
.unwrap_or_default()
.to_string()
}

#[derive(Serialize, Deserialize, Debug, Clone, ToSchema)]
pub struct Recipe {
// Required fields
Expand Down Expand Up @@ -268,25 +278,16 @@ impl Recipe {
Ok(yaml_value) => {
if let Some(nested_recipe) = yaml_value.get("recipe") {
serde_yaml::from_value(nested_recipe.clone())
.map_err(|e| anyhow::anyhow!("Failed to parse nested recipe: {}", e))?
.map_err(|e| anyhow::anyhow!("{}", strip_error_location(&e.to_string())))?
} else {
serde_yaml::from_str(content)
.map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?
.map_err(|e| anyhow::anyhow!("{}", strip_error_location(&e.to_string())))?
}
}
Err(_) => serde_yaml::from_str(content)
.map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?,
.map_err(|e| anyhow::anyhow!("{}", strip_error_location(&e.to_string())))?,
};

if let Some(ref retry_config) = recipe.retry {
if let Err(validation_error) = retry_config.validate() {
return Err(anyhow::anyhow!(
"Invalid retry configuration: {}",
validation_error
));
}
}

Ok(recipe)
}
}
Expand Down Expand Up @@ -772,4 +773,52 @@ isGlobal: true"#;
panic!("Expected Stdio extension");
}
}

#[test]
fn test_format_serde_error_removes_location() {
let content = r#"{"version": "1.0.0"}"#;

let result = Recipe::from_content(content);
assert!(result.is_err());

let error_msg = result.unwrap_err().to_string();
assert_eq!(error_msg, "missing field `title`");
}

#[test]
fn test_format_serde_error_missing_title() {
let content = r#"{
"version": "1.0.0",
"description": "A test recipe",
"instructions": "Test instructions"
}"#;

let result = Recipe::from_content(content);
assert!(result.is_err());

let error_msg = result.unwrap_err().to_string();
assert_eq!(error_msg, "missing field `title`");
}

#[test]
fn test_format_serde_error_invalid_type() {
let content = r#"{
"version": "1.0.0",
"title": "Test",
"description": "Test",
"instructions": "Test",
"settings": {
"temperature": "not_a_number"
}
}"#;

let result = Recipe::from_content(content);
assert!(result.is_err());

let error_msg = result.unwrap_err().to_string();
assert_eq!(
error_msg,
"settings.temperature: invalid type: string \"not_a_number\", expected f32"
);
}
}
2 changes: 1 addition & 1 deletion crates/goose/src/recipe/template_recipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use anyhow::Result;
use minijinja::{Environment, UndefinedBehavior};
use regex::Regex;

const CURRENT_TEMPLATE_NAME: &str = "current_template";
const CURRENT_TEMPLATE_NAME: &str = "recipe_template";
const OPEN_BRACE: &str = "{{";
const CLOSE_BRACE: &str = "}}";

Expand Down
13 changes: 13 additions & 0 deletions crates/goose/src/recipe/validate_recipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub fn validate_recipe_template_from_content(
let (recipe, _) = parse_recipe_content(recipe_content, recipe_dir)?;

validate_prompt_or_instructions(&recipe)?;
validate_retry_config(&recipe)?;
if let Some(response) = &recipe.response {
if let Some(json_schema) = &response.json_schema {
validate_json_schema(json_schema)?;
Expand All @@ -53,6 +54,18 @@ pub fn validate_recipe_template_from_content(
Ok(recipe)
}

fn validate_retry_config(recipe: &Recipe) -> Result<()> {
if let Some(ref retry_config) = recipe.retry {
if let Err(validation_error) = retry_config.validate() {
return Err(anyhow::anyhow!(
"Invalid retry configuration: {}",
validation_error
));
}
}
Ok(())
}

fn validate_prompt_or_instructions(recipe: &Recipe) -> Result<()> {
let has_instructions = recipe
.instructions
Expand Down
Loading