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
This commit is contained in:
octo-patch 2026-04-22 23:38:30 +08:00
parent 9812540602
commit e89b85da97

View file

@ -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<String, Value>>) -> HashMap<String, String> {
if tool_params.is_none() {
return HashMap::new();
@ -12,15 +13,27 @@ pub fn filter_tool_params(tool_params: &Option<HashMap<String, Value>>) -> 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<String> = 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::<HashMap<String, String>>()
}
@ -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<String, JsonValue> = 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<JsonValue> = 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<std::collections::HashMap<String, serde_yaml::Value>> =
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");
}
}