Skip to content
Open
Show file tree
Hide file tree
Changes from 11 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
94 changes: 90 additions & 4 deletions crates/goose/src/recipe/build_recipe/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use crate::recipe::read_recipe_file_content::read_parameter_file_content;
use crate::recipe::template_recipe::render_recipe_content_with_params;
use crate::recipe::template_recipe::{
render_recipe_content_with_params, render_recipe_content_with_structured_params,
};
use crate::recipe::validate_recipe::validate_recipe_template_from_content;
use crate::recipe::{
Recipe, RecipeParameter, RecipeParameterInputType, RecipeParameterRequirement,
BUILT_IN_RECIPE_DIR_PARAM,
};
use anyhow::Result;
use serde_json::Value;
use std::collections::HashMap;
use std::path::Path;

Expand All @@ -32,18 +35,101 @@ where
validate_recipe_template_from_content(&recipe_content, Some(recipe_dir_str.clone()))?
.parameters;

let (params_for_template, missing_params) =
apply_values_to_parameters(&params, recipe_parameters, &recipe_dir_str, user_prompt_fn)?;
let has_structured_params = recipe_parameters.as_ref().is_some_and(|params| {
params.iter().any(|p| {
matches!(
p.input_type,
RecipeParameterInputType::Object | RecipeParameterInputType::Array
)
})
});

let (params_for_template, missing_params) = apply_values_to_parameters(
&params,
recipe_parameters.clone(),
&recipe_dir_str,
user_prompt_fn,
)?;

let rendered_content = if missing_params.is_empty() {
render_recipe_content_with_params(&recipe_content, &params_for_template)?
if has_structured_params {
let structured_map =
to_structured_params(&params_for_template, recipe_parameters.as_deref())?;
render_recipe_content_with_structured_params(&recipe_content, &structured_map)?
} else {
render_recipe_content_with_params(&recipe_content, &params_for_template)?
}
} else {
String::new()
};

Ok((rendered_content, missing_params))
}

fn to_structured_params(
string_map: &HashMap<String, String>,
recipe_parameters: Option<&[RecipeParameter]>,
) -> Result<HashMap<String, Value>> {
let structured_keys: HashMap<&str, &RecipeParameterInputType> = recipe_parameters
.unwrap_or_default()
.iter()
.filter(|p| {
matches!(
p.input_type,
RecipeParameterInputType::Object | RecipeParameterInputType::Array
)
})
.map(|p| (p.key.as_str(), &p.input_type))
.collect();

string_map
.iter()
.map(|(k, v)| {
let value = if let Some(input_type) = structured_keys.get(k.as_str()) {
let parsed: Value = serde_json::from_str(v).map_err(|e| {
anyhow::anyhow!(
"Parameter '{}' has input_type {} but value is not valid JSON: {}",
k,
input_type,
e
)
Comment on lines +89 to +95
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Validate structured defaults before render-time JSON parsing

Object/array values are first parsed in to_structured_params, which runs only in the rendering path. As a result, an optional structured parameter with an invalid default (for example, non-JSON text) passes recipe validation and only fails when the default is injected and parsed during build, turning a schema error into a late runtime error.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 3e95363 -- validate_optional_parameters now checks that object/array parameter defaults are valid JSON matching their input_type (object must be {}, array must be []). Invalid defaults are rejected during recipe validation instead of surfacing as late runtime errors.

})?;
match input_type {
RecipeParameterInputType::Object if !parsed.is_object() => {
anyhow::bail!(
"Parameter '{}' has input_type object but received {}",
k,
json_type_name(&parsed)
)
}
RecipeParameterInputType::Array if !parsed.is_array() => {
anyhow::bail!(
"Parameter '{}' has input_type array but received {}",
k,
json_type_name(&parsed)
)
}
_ => parsed,
}
} else {
Value::String(v.clone())
};
Ok((k.clone(), value))
})
.collect()
}

fn json_type_name(v: &Value) -> &'static str {
match v {
Value::Null => "null",
Value::Bool(_) => "a boolean",
Value::Number(_) => "a number",
Value::String(_) => "a string",
Value::Array(_) => "an array",
Value::Object(_) => "an object",
}
}

pub fn build_recipe_from_template<F>(
recipe_content: String,
recipe_dir: &Path,
Expand Down
191 changes: 191 additions & 0 deletions crates/goose/src/recipe/build_recipe/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -664,3 +664,194 @@ fn test_build_recipe_from_template_invalid_retry_config() {
_ => panic!("Expected Invalid error, got: {:?}", err),
}
}

#[test]
fn test_build_recipe_with_object_parameter() {
let yaml = r#"instructions: "Signal: {{ signal.name }} in {{ signal.namespace }}"
parameters:
- key: signal
input_type: object
requirement: required
description: "Signal data""#;

let (_temp_dir, recipe_file) = setup_yaml_recipe_file(yaml);
let params = vec![(
"signal".to_string(),
r#"{"name": "OOMKilled", "namespace": "production"}"#.to_string(),
)];
let recipe = build_recipe_from_template(
recipe_file.content,
&recipe_file.parent_dir,
params,
NO_USER_PROMPT,
)
.unwrap();

assert_eq!(
recipe.instructions.unwrap(),
"Signal: OOMKilled in production"
);
let param = &recipe.parameters.as_ref().unwrap()[0];
assert!(matches!(param.input_type, RecipeParameterInputType::Object));
}

#[test]
fn test_build_recipe_with_array_parameter() {
let yaml = r#"instructions: |
{% for item in findings %}{{ item.name }}: {{ item.output }}
{% endfor %}
parameters:
- key: findings
input_type: array
requirement: required
description: "Diagnostic findings""#;

let (_temp_dir, recipe_file) = setup_yaml_recipe_file(yaml);
let params = vec![(
"findings".to_string(),
r#"[{"name": "check-1", "output": "passed"}, {"name": "check-2", "output": "failed"}]"#
.to_string(),
)];
let recipe = build_recipe_from_template(
recipe_file.content,
&recipe_file.parent_dir,
params,
NO_USER_PROMPT,
)
.unwrap();

let instructions = recipe.instructions.unwrap();
assert!(instructions.contains("check-1: passed"));
assert!(instructions.contains("check-2: failed"));
}

#[test]
fn test_build_recipe_with_mixed_string_and_object_params() {
let yaml = r#"instructions: "Alert {{ alert_name }} for {{ signal.resource_kind }}/{{ signal.resource_name }}"
parameters:
- key: alert_name
input_type: string
requirement: required
description: "Alert name"
- key: signal
input_type: object
requirement: required
description: "Signal data""#;

let (_temp_dir, recipe_file) = setup_yaml_recipe_file(yaml);
let params = vec![
("alert_name".to_string(), "HighMemory".to_string()),
(
"signal".to_string(),
r#"{"resource_kind": "Pod", "resource_name": "api-server-abc"}"#.to_string(),
),
];
let recipe = build_recipe_from_template(
recipe_file.content,
&recipe_file.parent_dir,
params,
NO_USER_PROMPT,
)
.unwrap();

assert_eq!(
recipe.instructions.unwrap(),
"Alert HighMemory for Pod/api-server-abc"
);
}

#[test]
fn test_build_recipe_with_object_param_invalid_json() {
let yaml = r#"instructions: "Signal: {{ signal.name }}"
parameters:
- key: signal
input_type: object
requirement: required
description: "Signal data""#;

let (_temp_dir, recipe_file) = setup_yaml_recipe_file(yaml);
let params = vec![("signal".to_string(), "not-valid-json".to_string())];
let result = build_recipe_from_template(
recipe_file.content,
&recipe_file.parent_dir,
params,
NO_USER_PROMPT,
);

assert!(result.is_err());
match result.unwrap_err() {
RecipeError::Invalid { source } => {
assert!(
source.to_string().contains("not valid JSON"),
"Expected JSON parse error, got: {}",
source
);
}
other => panic!("Expected Invalid error, got: {:?}", other),
}
}

#[test]
fn test_build_recipe_with_object_param_receives_array() {
let yaml = r#"instructions: "Signal: {{ signal.name }}"
parameters:
- key: signal
input_type: object
requirement: required
description: "Signal data""#;

let (_temp_dir, recipe_file) = setup_yaml_recipe_file(yaml);
let params = vec![("signal".to_string(), r#"[1, 2, 3]"#.to_string())];
let result = build_recipe_from_template(
recipe_file.content,
&recipe_file.parent_dir,
params,
NO_USER_PROMPT,
);

assert!(result.is_err());
match result.unwrap_err() {
RecipeError::Invalid { source } => {
let msg = source.to_string();
assert!(
msg.contains("input_type object") && msg.contains("an array"),
"Expected type mismatch error, got: {}",
msg
);
}
other => panic!("Expected Invalid error, got: {:?}", other),
}
}

#[test]
fn test_build_recipe_with_array_param_receives_object() {
let yaml = r#"instructions: |
{% for item in findings %}{{ item }}{% endfor %}
parameters:
- key: findings
input_type: array
requirement: required
description: "Findings list""#;

let (_temp_dir, recipe_file) = setup_yaml_recipe_file(yaml);
let params = vec![("findings".to_string(), r#"{"key": "value"}"#.to_string())];
let result = build_recipe_from_template(
recipe_file.content,
&recipe_file.parent_dir,
params,
NO_USER_PROMPT,
);

assert!(result.is_err());
match result.unwrap_err() {
RecipeError::Invalid { source } => {
let msg = source.to_string();
assert!(
msg.contains("input_type array") && msg.contains("an object"),
"Expected type mismatch error, got: {}",
msg
);
}
other => panic!("Expected Invalid error, got: {:?}", other),
}
}
Loading
Loading