feat: normalize MCP tool names and enhance OpenAI response handling

This commit is contained in:
leokun
2026-08-24 12:22:11 +08:00
parent 1a0cf89fe1
commit 4ddd3adb3f
9 changed files with 63114 additions and 19 deletions
+68 -4
View File
@@ -459,24 +459,36 @@ pub fn dynamic_mcp(
Error::Protocol(format!("MCP tool {} is missing input schema", wire.name))
})?),
};
let name = model_tool_name(&wire.name);
let definition = ToolDefinition {
name: wire.name.clone(),
name: name.clone(),
description: wire.description.clone(),
parameters,
};
if output
.insert(wire.name.clone(), (wire.clone(), definition))
.insert(name.clone(), (wire.clone(), definition))
.is_some()
{
return Err(Error::Protocol(format!(
"duplicate MCP tool definition: {}",
wire.name
"duplicate MCP tool name after normalization: {name}"
)));
}
}
Ok(output)
}
fn model_tool_name(name: &str) -> String {
name.chars()
.map(|character| {
if character.is_ascii_alphanumeric() || matches!(character, '_' | '-') {
character
} else {
'_'
}
})
.collect()
}
fn prost_value(value: &prost_types::Value) -> Value {
use prost_types::value::Kind;
match value.kind.as_ref() {
@@ -511,6 +523,58 @@ fn xml(value: &str) -> String {
mod tests {
use super::*;
fn direct_mcp_tool(name: &str) -> pb::McpToolDefinition {
pb::McpToolDefinition {
name: name.into(),
provider_identifier: "extension-GitKraken".into(),
tool_name: "git_status".into(),
description: "Get repository status".into(),
input_schema_json: Some(r#"{"type":"object"}"#.into()),
..Default::default()
}
}
#[test]
fn dynamic_mcp_normalizes_extension_identifier_for_model_tool_names() {
let original = "user-eamodio.gitlens-extension-GitKraken-git_status";
let request = pb::AgentRunRequest {
mcp_tools: Some(pb::McpTools {
mcp_tools: vec![direct_mcp_tool(original)],
}),
..Default::default()
};
let tools = dynamic_mcp(&request, &pb::RequestContext::default()).unwrap();
let normalized = "user-eamodio_gitlens-extension-GitKraken-git_status";
let (wire, definition) = tools.get(normalized).unwrap();
assert_eq!(definition.name, normalized);
assert_eq!(wire.name, original);
assert_eq!(wire.provider_identifier, "extension-GitKraken");
assert_eq!(wire.tool_name, "git_status");
assert!(normalized
.chars()
.all(|character| character.is_ascii_alphanumeric() || matches!(character, '_' | '-')));
}
#[test]
fn dynamic_mcp_rejects_names_that_collide_after_normalization() {
let request = pb::AgentRunRequest {
mcp_tools: Some(pb::McpTools {
mcp_tools: vec![
direct_mcp_tool("server.name-tool"),
direct_mcp_tool("server_name-tool"),
],
}),
..Default::default()
};
let error = dynamic_mcp(&request, &pb::RequestContext::default()).unwrap_err();
assert!(error
.to_string()
.contains("duplicate MCP tool name after normalization: server_name-tool"));
}
#[test]
fn meta_mcp_routes_projects_descriptor_routing_without_runtime_discovery() {
let context = pb::RequestContext {
+106 -15
View File
@@ -27,6 +27,12 @@ struct ResponseToolState {
ended: bool,
}
enum ResponseToolArguments<'a> {
None,
Delta(&'a str),
Snapshot(&'a str),
}
pub struct OpenAiResponsesProvider {
client: reqwest::Client,
config: ProviderConfig,
@@ -152,7 +158,7 @@ impl Provider for OpenAiResponsesProvider {
if item.get("type").and_then(Value::as_str) == Some("function_call") {
let index = required_u64(&value, "output_index")? as usize;
saw_tool = true;
for event in update_response_tool(index, item, None, false, &mut tools)? { yield event; }
for event in update_response_tool(index, item, ResponseToolArguments::None, false, &mut tools)? { yield event; }
}
}
"response.output_item.done" => {
@@ -172,8 +178,11 @@ impl Provider for OpenAiResponsesProvider {
Some("function_call") => {
let index = required_u64(&value, "output_index")? as usize;
saw_tool = true;
let final_arguments = item.get("arguments").and_then(Value::as_str);
for event in update_response_tool(index, item, final_arguments, true, &mut tools)? { yield event; }
let arguments = item
.get("arguments")
.and_then(Value::as_str)
.map_or(ResponseToolArguments::None, ResponseToolArguments::Snapshot);
for event in update_response_tool(index, item, arguments, true, &mut tools)? { yield event; }
}
_ => {}
}
@@ -182,13 +191,16 @@ impl Provider for OpenAiResponsesProvider {
let index = required_u64(&value, "output_index")? as usize;
if let Some(delta) = value.get("delta").and_then(Value::as_str) {
saw_tool = true;
for event in update_response_tool(index, &Value::Null, Some(delta), false, &mut tools)? { yield event; }
for event in update_response_tool(index, &Value::Null, ResponseToolArguments::Delta(delta), false, &mut tools)? { yield event; }
}
}
"response.function_call_arguments.done" => {
let index = required_u64(&value, "output_index")? as usize;
let final_arguments = value.get("arguments").and_then(Value::as_str);
for event in update_response_tool(index, &Value::Null, final_arguments, true, &mut tools)? { yield event; }
let arguments = value
.get("arguments")
.and_then(Value::as_str)
.map_or(ResponseToolArguments::None, ResponseToolArguments::Snapshot);
for event in update_response_tool(index, &Value::Null, arguments, true, &mut tools)? { yield event; }
}
"response.completed" => {
if let Some(usage) = value.pointer("/response/usage") { yield ModelEvent::Usage(responses_usage(usage)); }
@@ -208,8 +220,11 @@ impl Provider for OpenAiResponsesProvider {
}
Some("function_call") => {
saw_tool = true;
let final_arguments = item.get("arguments").and_then(Value::as_str);
for event in update_response_tool(index, item, final_arguments, true, &mut tools)? { yield event; }
let arguments = item
.get("arguments")
.and_then(Value::as_str)
.map_or(ResponseToolArguments::None, ResponseToolArguments::Snapshot);
for event in update_response_tool(index, item, arguments, true, &mut tools)? { yield event; }
}
_ => {}
}
@@ -303,7 +318,7 @@ fn reconcile_response_text(
fn update_response_tool(
index: usize,
item: &Value,
arguments: Option<&str>,
arguments: ResponseToolArguments<'_>,
done: bool,
tools: &mut std::collections::BTreeMap<usize, ResponseToolState>,
) -> Result<Vec<ModelEvent>> {
@@ -314,11 +329,17 @@ fn update_response_tool(
if let Some(name) = item.get("name").and_then(Value::as_str) {
tool.name.get_or_insert_with(|| name.into());
}
if let Some(arguments) = arguments {
if arguments.starts_with(&tool.arguments) {
tool.arguments.push_str(&arguments[tool.arguments.len()..]);
} else if !tool.arguments.ends_with(arguments) {
tool.arguments.push_str(arguments);
match arguments {
ResponseToolArguments::None => {}
ResponseToolArguments::Delta(delta) => tool.arguments.push_str(delta),
ResponseToolArguments::Snapshot(snapshot) if snapshot == tool.arguments => {}
ResponseToolArguments::Snapshot(snapshot) if snapshot.starts_with(&tool.arguments) => {
tool.arguments.push_str(&snapshot[tool.arguments.len()..]);
}
ResponseToolArguments::Snapshot(_) => {
return Err(Error::Provider(
"OpenAI Responses final tool arguments do not match streamed arguments".into(),
));
}
}
@@ -509,8 +530,11 @@ fn responses_usage(value: &Value) -> Usage {
#[cfg(test)]
mod tests {
use super::responses_input;
use std::collections::BTreeMap;
use super::{responses_input, update_response_tool, ResponseToolArguments, ResponseToolState};
use crate::model::{ContentPart, ProjectedContent, ProjectedMessage, Role, ToolResultContent};
use crate::provider::ModelEvent;
#[test]
fn read_image_stays_in_its_function_call_output() {
@@ -542,4 +566,71 @@ mod tests {
assert_eq!(input[0]["output"][1]["type"], "input_image");
assert_eq!(input[0]["output"][1]["detail"], "auto");
}
#[test]
fn tool_argument_deltas_are_ordered_bytes_and_final_snapshots_are_idempotent() {
let item = serde_json::json!({"call_id": "call-1", "name": "Shell"});
let mut tools = BTreeMap::<usize, ResponseToolState>::new();
let mut events = update_response_tool(
0,
&item,
ResponseToolArguments::Delta(r#"{"block_until_ms":300"#),
false,
&mut tools,
)
.unwrap();
events.extend(
update_response_tool(
0,
&item,
ResponseToolArguments::Delta("00"),
false,
&mut tools,
)
.unwrap(),
);
events.extend(
update_response_tool(
0,
&item,
ResponseToolArguments::Delta("}"),
false,
&mut tools,
)
.unwrap(),
);
events.extend(
update_response_tool(
0,
&item,
ResponseToolArguments::Snapshot(r#"{"block_until_ms":30000}"#),
true,
&mut tools,
)
.unwrap(),
);
let arguments = events
.iter()
.filter_map(|event| match event {
ModelEvent::ToolCallArgumentsDelta { delta, .. } => Some(delta.as_str()),
_ => None,
})
.collect::<String>();
assert_eq!(arguments, r#"{"block_until_ms":30000}"#);
assert_eq!(
serde_json::from_str::<serde_json::Value>(&arguments).unwrap()["block_until_ms"],
30000
);
assert!(update_response_tool(
0,
&item,
ResponseToolArguments::Snapshot(r#"{"block_until_ms":30000}"#),
true,
&mut tools,
)
.unwrap()
.is_empty());
}
}