From e89b85da97f050b4349c8721da8454736a34cc4a Mon Sep 17 00:00:00 2001 From: octo-patch Date: Wed, 22 Apr 2026 23:38:30 +0800 Subject: [PATCH] fix: handle list parameters in prompt_target function calls (fixes #429) List/sequence parameters were silently dropped by filter_tool_params because the filter only passed through string, number, and bool values. - filter_tool_params now includes Value::Sequence, converting items to a comma-separated string for URL path and GET query string usage - compute_request_path_body POST path now overrides comma-separated strings with proper JSON arrays so target services receive the correct type - Add tests covering filter_tool_params with list values and POST body with list parameters --- crates/prompt_gateway/src/tools.rs | 113 +++++++++++++++++++++++++---- 1 file changed, 97 insertions(+), 16 deletions(-) diff --git a/crates/prompt_gateway/src/tools.rs b/crates/prompt_gateway/src/tools.rs index c909a2dd..ee472441 100644 --- a/crates/prompt_gateway/src/tools.rs +++ b/crates/prompt_gateway/src/tools.rs @@ -1,9 +1,10 @@ use common::configuration::{HttpMethod, Parameter}; use std::collections::HashMap; +use serde_json::Value as JsonValue; use serde_yaml::Value; -// only add params that are of string, number and bool type +// only add params that are of string, number, bool, or sequence type pub fn filter_tool_params(tool_params: &Option>) -> HashMap { if tool_params.is_none() { return HashMap::new(); @@ -12,15 +13,27 @@ pub fn filter_tool_params(tool_params: &Option>) -> HashM .as_ref() .unwrap() .iter() - .filter(|(_, value)| value.is_number() || value.is_string() || value.is_bool()) - .map(|(key, value)| match value { - Value::Number(n) => (key.clone(), n.to_string()), - Value::String(s) => (key.clone(), s.clone()), - Value::Bool(b) => (key.clone(), b.to_string()), - Value::Null => todo!(), - Value::Sequence(_) => todo!(), - Value::Mapping(_) => todo!(), - Value::Tagged(_) => todo!(), + .filter(|(_, value)| { + value.is_number() || value.is_string() || value.is_bool() || value.is_sequence() + }) + .filter_map(|(key, value)| match value { + Value::Number(n) => Some((key.clone(), n.to_string())), + Value::String(s) => Some((key.clone(), s.clone())), + Value::Bool(b) => Some((key.clone(), b.to_string())), + Value::Sequence(seq) => { + // Convert sequence to comma-separated string for URL params / GET requests + let items: Vec = seq + .iter() + .filter_map(|v| match v { + Value::String(s) => Some(s.clone()), + Value::Number(n) => Some(n.to_string()), + Value::Bool(b) => Some(b.to_string()), + _ => None, + }) + .collect(); + Some((key.clone(), items.join(","))) + } + _ => None, }) .collect::>() } @@ -41,16 +54,37 @@ pub fn compute_request_path_body( let (path, body) = match http_method { HttpMethod::Get => (format!("{}?{}", path_with_params, query_string), None), HttpMethod::Post => { - let mut additional_params = additional_params; + // Build the POST body as a JSON object, preserving list params as JSON arrays + let mut body_params: HashMap = additional_params + .into_iter() + .map(|(k, v)| (k, JsonValue::String(v))) + .collect(); + + // Override list (sequence) params with proper JSON arrays, replacing the + // comma-separated string that filter_tool_params produced for path handling + if let Some(params) = tool_params { + for (key, value) in params.iter() { + if let Value::Sequence(seq) = value { + let json_arr: Vec = seq + .iter() + .filter_map(|v| serde_json::to_value(v).ok()) + .collect(); + body_params.insert(key.clone(), JsonValue::Array(json_arr)); + } + } + } + if !query_string.is_empty() { - query_string.split("&").for_each(|param| { - let mut parts = param.split("="); + query_string.split('&').for_each(|param| { + let mut parts = param.split('='); let key = parts.next().unwrap(); - let value = parts.next().unwrap(); - additional_params.insert(key.to_string(), value.to_string()); + let value = parts.next().unwrap_or(""); + body_params + .entry(key.to_string()) + .or_insert_with(|| JsonValue::String(value.to_string())); }); } - let body = serde_json::to_string(&additional_params).unwrap(); + let body = serde_json::to_string(&body_params).unwrap(); (path_with_params, Some(body)) } }; @@ -159,4 +193,51 @@ mod test { ); assert_eq!(body, None); } + + #[test] + fn test_filter_tool_params_list() { + use super::filter_tool_params; + let tool_params = serde_yaml::from_str( + r#" + device_ids: + - device1 + - device2 + - device3 + name: test + "#, + ) + .unwrap(); + let params = filter_tool_params(&tool_params); + assert_eq!(params.get("device_ids").unwrap(), "device1,device2,device3"); + assert_eq!(params.get("name").unwrap(), "test"); + } + + #[test] + fn test_compute_request_path_body_list_post() { + let endpoint_path = "/agent/device_reboot"; + let tool_params: Option> = + serde_yaml::from_str( + r#" + device_ids: + - device1 + - device2 + "#, + ) + .unwrap(); + let prompt_target_params = vec![]; + let http_method = HttpMethod::Post; + let (path, body) = super::compute_request_path_body( + endpoint_path, + &tool_params, + &prompt_target_params, + &http_method, + ) + .unwrap(); + assert_eq!(path, "/agent/device_reboot"); + let body_str = body.unwrap(); + let body_json: serde_json::Value = serde_json::from_str(&body_str).unwrap(); + assert!(body_json["device_ids"].is_array()); + assert_eq!(body_json["device_ids"][0], "device1"); + assert_eq!(body_json["device_ids"][1], "device2"); + } }