fix(tool): remove unnecessary schema validation (#375)
This commit is contained in:
parent
05e9a04d1c
commit
1555c0fa33
4 changed files with 45 additions and 61 deletions
|
|
@ -6,7 +6,6 @@ use schemars::JsonSchema;
|
|||
use crate::{
|
||||
handler::server::tool::{
|
||||
CallToolHandler, DynCallToolHandler, ToolCallContext, schema_for_type,
|
||||
validate_against_schema,
|
||||
},
|
||||
model::{CallToolResult, Tool, ToolAnnotations},
|
||||
};
|
||||
|
|
@ -246,19 +245,6 @@ where
|
|||
|
||||
let result = (item.call)(context).await?;
|
||||
|
||||
// Validate structured content against output schema if present
|
||||
if let Some(ref output_schema) = item.attr.output_schema {
|
||||
// When output_schema is defined, structured_content is required
|
||||
if result.structured_content.is_none() {
|
||||
return Err(crate::ErrorData::invalid_params(
|
||||
"Tool with output_schema must return structured_content",
|
||||
None,
|
||||
));
|
||||
}
|
||||
// Validate the structured content against the schema
|
||||
validate_against_schema(result.structured_content.as_ref().unwrap(), output_schema)?;
|
||||
}
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -67,43 +67,6 @@ pub fn schema_for_type<T: JsonSchema>() -> JsonObject {
|
|||
}
|
||||
}
|
||||
|
||||
/// Validate that a JSON value conforms to basic type constraints from a schema.
|
||||
///
|
||||
/// Note: This is a basic validation that only checks type compatibility.
|
||||
/// For full JSON Schema validation, a dedicated validation library would be needed.
|
||||
pub fn validate_against_schema(
|
||||
value: &serde_json::Value,
|
||||
schema: &JsonObject,
|
||||
) -> Result<(), crate::ErrorData> {
|
||||
// Basic type validation
|
||||
if let Some(schema_type) = schema.get("type").and_then(|t| t.as_str()) {
|
||||
let value_type = get_json_value_type(value);
|
||||
|
||||
if schema_type != value_type {
|
||||
return Err(crate::ErrorData::invalid_params(
|
||||
format!(
|
||||
"Value type does not match schema. Expected '{}', got '{}'",
|
||||
schema_type, value_type
|
||||
),
|
||||
None,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn get_json_value_type(value: &serde_json::Value) -> &'static str {
|
||||
match value {
|
||||
serde_json::Value::Null => "null",
|
||||
serde_json::Value::Bool(_) => "boolean",
|
||||
serde_json::Value::Number(_) => "number",
|
||||
serde_json::Value::String(_) => "string",
|
||||
serde_json::Value::Array(_) => "array",
|
||||
serde_json::Value::Object(_) => "object",
|
||||
}
|
||||
}
|
||||
|
||||
/// Call [`schema_for_type`] with a cache
|
||||
pub fn cached_schema_for_type<T: JsonSchema + std::any::Any>() -> Arc<JsonObject> {
|
||||
thread_local! {
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ pub use extension::*;
|
|||
pub use meta::*;
|
||||
pub use prompt::*;
|
||||
pub use resource::*;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde::{Deserialize, Serialize, de::DeserializeOwned};
|
||||
use serde_json::Value;
|
||||
pub use tool::*;
|
||||
|
||||
|
|
@ -1260,12 +1260,32 @@ impl CallToolResult {
|
|||
}
|
||||
}
|
||||
|
||||
/// Validate that content or structured content is provided
|
||||
pub fn validate(&self) -> Result<(), &'static str> {
|
||||
match (&self.content, &self.structured_content) {
|
||||
(None, None) => Err("either content or structured_content must be provided"),
|
||||
_ => Ok(()),
|
||||
/// Convert the `structured_content` part of response into a certain type.
|
||||
///
|
||||
/// # About json schema validation
|
||||
/// Since rust is a strong type language, we don't need to do json schema validation here.
|
||||
///
|
||||
/// But if you do have to validate the response data, you can use [`jsonschema`](https://crates.io/crates/jsonschema) crate.
|
||||
pub fn into_typed<T>(self) -> Result<T, serde_json::Error>
|
||||
where
|
||||
T: DeserializeOwned,
|
||||
{
|
||||
let raw_text = match (self.structured_content, &self.content) {
|
||||
(Some(value), _) => return serde_json::from_value(value),
|
||||
(None, Some(contents)) => {
|
||||
if let Some(text) = contents.first().and_then(|c| c.as_text()) {
|
||||
let text = &text.text;
|
||||
Some(text)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
(None, None) => None,
|
||||
};
|
||||
if let Some(text) = raw_text {
|
||||
return serde_json::from_str(text);
|
||||
}
|
||||
serde_json::from_value(serde_json::Value::Null)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1294,7 +1314,11 @@ impl<'de> Deserialize<'de> for CallToolResult {
|
|||
};
|
||||
|
||||
// Validate mutual exclusivity
|
||||
result.validate().map_err(serde::de::Error::custom)?;
|
||||
if result.content.is_none() && result.structured_content.is_none() {
|
||||
return Err(serde::de::Error::custom(
|
||||
"CallToolResult must have either content or structured_content",
|
||||
));
|
||||
}
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -170,13 +170,24 @@ async fn test_structured_error_in_call_result() {
|
|||
|
||||
#[tokio::test]
|
||||
async fn test_mutual_exclusivity_validation() {
|
||||
#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
|
||||
pub struct Response {
|
||||
message: String,
|
||||
}
|
||||
let response = Response {
|
||||
message: "Hello".into(),
|
||||
};
|
||||
// Test that content and structured_content can both be passed separately
|
||||
let content_result = CallToolResult::success(vec![Content::text("Hello")]);
|
||||
let content_result = CallToolResult::success(vec![Content::json(response.clone()).unwrap()]);
|
||||
let structured_result = CallToolResult::structured(json!({"message": "Hello"}));
|
||||
|
||||
// Verify the validation
|
||||
assert!(content_result.validate().is_ok());
|
||||
assert!(structured_result.validate().is_ok());
|
||||
content_result
|
||||
.into_typed::<Response>()
|
||||
.expect("Failed to extract content");
|
||||
structured_result
|
||||
.into_typed::<Response>()
|
||||
.expect("Failed to extract content");
|
||||
|
||||
// Try to create a result with both fields
|
||||
let json_with_both = json!({
|
||||
|
|
|
|||
Loading…
Reference in a new issue