Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
65 changes: 61 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,72 @@ 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: std::collections::HashSet<&str> = recipe_parameters
.unwrap_or_default()
.iter()
.filter(|p| {
matches!(
p.input_type,
RecipeParameterInputType::Object | RecipeParameterInputType::Array
)
})
.map(|p| p.key.as_str())
.collect();

string_map
.iter()
.map(|(k, v)| {
let value = if structured_keys.contains(k.as_str()) {
serde_json::from_str(v).map_err(|e| {
anyhow::anyhow!(
"Parameter '{}' has input_type object/array but value is not valid JSON: {}",
k,
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.

})?
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 parameter shape by input_type

to_structured_params currently treats object and array parameters identically and only checks that the value is valid JSON. That allows mismatched payloads (e.g., input_type: array receiving {...} or input_type: object receiving [...]) to pass parsing and fail later during template rendering with confusing MiniJinja errors (item.name/field access on the wrong type). Since input_type semantics are now exposed in schema/docs, this should reject mismatches at conversion time by checking Value::is_object() vs Value::is_array() per parameter key and returning a targeted validation 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 call. Fixed in 83db531 -- to_structured_params now validates that input_type: object receives a JSON object and input_type: array receives a JSON array, returning a targeted error like Parameter 'signal' has input_type object but received an array instead of letting mismatches surface as confusing MiniJinja template errors. Two new tests cover both mismatch directions.

} else {
Value::String(v.clone())
};
Ok((k.clone(), value))
})
.collect()
}

pub fn build_recipe_from_template<F>(
recipe_content: String,
recipe_dir: &Path,
Expand Down
126 changes: 126 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,129 @@ 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),
}
}
134 changes: 134 additions & 0 deletions crates/goose/src/recipe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ pub enum RecipeParameterInputType {
/// Cannot have default values to prevent importing sensitive user files.
File,
Select,
/// Structured object parameter passed as JSON.
/// Enables dot-notation access in templates: `{{ param.field }}`
Object,
/// Array parameter passed as a JSON array.
/// Enables iteration in templates: `{% for item in param %}`
Array,
Comment on lines +186 to +189
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Wire object/array input types into recipe rendering

Adding Object and Array as valid input_type values here lets these recipes parse and validate, but the active build path still renders with HashMap<String, String> (apply_values_to_parameters and render_recipe_content_with_params in crates/goose/src/recipe/build_recipe/mod.rs), so structured values are never materialized as JSON objects/arrays. In practice, templates using {{ signal.namespace }} or {% for item in findings %} will fail at render time even though the recipe schema now accepts those input types.

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.

Thanks for calling this out. The latest push (2de4daa) addresses this: render_recipe_template now detects input_type: object/array and routes through render_recipe_content_with_structured_params, parsing the JSON string values into serde_json::Value before MiniJinja rendering. All existing callers benefit without API changes. Four integration tests through build_recipe_from_template cover this end-to-end (object, array, mixed, and invalid-JSON error cases).

}

impl fmt::Display for RecipeParameterInputType {
Expand Down Expand Up @@ -873,4 +879,132 @@ isGlobal: true"#;
"settings.temperature: invalid type: string \"not_a_number\", expected f32"
);
}

#[test]
fn test_from_content_json_with_object_parameter() {
let content = r#"{
"version": "1.0.0",
"title": "Object Param Recipe",
"description": "Recipe with object parameter",
"instructions": "Analyze {{ signal.name }} in {{ signal.namespace }}",
"parameters": [
{
"key": "signal",
"input_type": "object",
"requirement": "required",
"description": "Signal data with name and namespace"
}
]
}"#;

let recipe = Recipe::from_content(content).unwrap();
assert!(recipe.parameters.is_some());
let params = recipe.parameters.unwrap();
assert_eq!(params.len(), 1);
assert_eq!(params[0].key, "signal");
assert!(matches!(
params[0].input_type,
RecipeParameterInputType::Object
));
assert!(matches!(
params[0].requirement,
RecipeParameterRequirement::Required
));
}

#[test]
fn test_from_content_json_with_array_parameter() {
let content = r#"{
"version": "1.0.0",
"title": "Array Param Recipe",
"description": "Recipe with array parameter",
"instructions": "Process findings",
"parameters": [
{
"key": "findings",
"input_type": "array",
"requirement": "required",
"description": "List of diagnostic findings"
}
]
}"#;

let recipe = Recipe::from_content(content).unwrap();
assert!(recipe.parameters.is_some());
let params = recipe.parameters.unwrap();
assert_eq!(params.len(), 1);
assert_eq!(params[0].key, "findings");
assert!(matches!(
params[0].input_type,
RecipeParameterInputType::Array
));
}

#[test]
fn test_from_content_yaml_with_object_and_array_parameters() {
let content = r#"version: 1.0.0
title: Mixed Structured Params
description: Recipe with object and array parameters
instructions: Analyze signal and findings
parameters:
- key: signal
input_type: object
requirement: required
description: Signal data
- key: findings
input_type: array
requirement: optional
description: Diagnostic findings
- key: cluster_name
input_type: string
requirement: required
description: Target cluster"#;

let recipe = Recipe::from_content(content).unwrap();
assert!(recipe.parameters.is_some());
let params = recipe.parameters.unwrap();
assert_eq!(params.len(), 3);

assert_eq!(params[0].key, "signal");
assert!(matches!(
params[0].input_type,
RecipeParameterInputType::Object
));
assert!(matches!(
params[0].requirement,
RecipeParameterRequirement::Required
));

assert_eq!(params[1].key, "findings");
assert!(matches!(
params[1].input_type,
RecipeParameterInputType::Array
));
assert!(matches!(
params[1].requirement,
RecipeParameterRequirement::Optional
));

assert_eq!(params[2].key, "cluster_name");
assert!(matches!(
params[2].input_type,
RecipeParameterInputType::String
));
}

#[test]
fn test_object_array_input_type_serde_roundtrip() {
let object_type = RecipeParameterInputType::Object;
let array_type = RecipeParameterInputType::Array;

let object_json = serde_json::to_string(&object_type).unwrap();
let array_json = serde_json::to_string(&array_type).unwrap();
assert_eq!(object_json, "\"object\"");
assert_eq!(array_json, "\"array\"");

let deser_object: RecipeParameterInputType = serde_json::from_str(&object_json).unwrap();
let deser_array: RecipeParameterInputType = serde_json::from_str(&array_json).unwrap();
assert!(matches!(deser_object, RecipeParameterInputType::Object));
assert!(matches!(deser_array, RecipeParameterInputType::Array));
}
}
Loading
Loading