Skip to content

Commit 0317315

Browse files
authored
fix: adds empty description check for tool schema (#1474)
1 parent fda6aea commit 0317315

File tree

1 file changed

+22
-9
lines changed

1 file changed

+22
-9
lines changed

crates/q_chat/src/tool_manager.rs

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,17 @@ pub struct PromptBundle {
452452
pub prompt_get: PromptGet,
453453
}
454454

455+
/// Categorizes different types of tool name validation failures:
456+
/// - `TooLong`: The tool name exceeds the maximum allowed length
457+
/// - `IllegalChar`: The tool name contains characters that are not allowed
458+
/// - `EmptyDescription`: The tool description is empty or missing
459+
#[allow(dead_code)]
460+
enum OutOfSpecName {
461+
TooLong(String),
462+
IllegalChar(String),
463+
EmptyDescription(String),
464+
}
465+
455466
/// Manages the lifecycle and interactions with tools from various sources, including MCP servers.
456467
/// This struct is responsible for initializing tools, handling tool requests, and maintaining
457468
/// a cache of available prompts from connected servers.
@@ -519,7 +530,7 @@ impl ToolManager {
519530
// Each mcp server might have multiple tools.
520531
// To avoid naming conflicts we are going to namespace it.
521532
// This would also help us locate which mcp server to call the tool from.
522-
let mut out_of_spec_tool_names = Vec::<String>::new();
533+
let mut out_of_spec_tool_names = Vec::<OutOfSpecName>::new();
523534
let mut hasher = DefaultHasher::new();
524535
let number_of_tools = specs.len();
525536
// Sanitize tool names to ensure they comply with the naming requirements:
@@ -543,7 +554,10 @@ impl ToolManager {
543554
};
544555
let full_name = format!("{}{}{}", server_name, NAMESPACE_DELIMITER, sn);
545556
if full_name.len() > 64 {
546-
out_of_spec_tool_names.push(spec.name);
557+
out_of_spec_tool_names.push(OutOfSpecName::TooLong(spec.name));
558+
continue;
559+
} else if spec.description.is_empty() {
560+
out_of_spec_tool_names.push(OutOfSpecName::EmptyDescription(spec.name));
547561
continue;
548562
}
549563
if sn != spec.name {
@@ -565,16 +579,15 @@ impl ToolManager {
565579
// - There is not a whole lot we can do with this data
566580
if let Some(tx_clone) = &tx_clone {
567581
let send_result = if !out_of_spec_tool_names.is_empty() {
568-
let allowed_len = 64 - server_name.len();
569582
let msg = out_of_spec_tool_names.iter().fold(
570-
String::from("The following tool names are out of spec. They will be excluded from the list of available tools:\n"),
583+
String::from("The following tools are out of spec. They will be excluded from the list of available tools:\n"),
571584
|mut acc, name| {
572-
let msg = if name.len() > allowed_len {
573-
"exceeded max length of 64 when combined with server name"
574-
} else {
575-
"must be compliant with ^[a-zA-Z][a-zA-Z0-9_]*$"
585+
let (tool_name, msg) = match name {
586+
OutOfSpecName::TooLong(tool_name) => (tool_name.as_str(), "tool name exceeds max length of 64 when combined with server name"),
587+
OutOfSpecName::IllegalChar(tool_name) => (tool_name.as_str(), "tool name must be compliant with ^[a-zA-Z][a-zA-Z0-9_]*$"),
588+
OutOfSpecName::EmptyDescription(tool_name) => (tool_name.as_str(), "tool schema contains empty description"),
576589
};
577-
acc.push_str(format!(" - {} ({})\n", name, msg).as_str());
590+
acc.push_str(format!(" - {} ({})\n", tool_name, msg).as_str());
578591
acc
579592
}
580593
);

0 commit comments

Comments
 (0)