mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Preserve complete MCP namespace descriptions (#36882)
## What changed - Keep complete MCP namespace descriptions in tool-search source metadata. - Raise the namespace tool-spec description limit from 1,000 bytes to 512 KiB, truncating at a UTF-8 character boundary only when the new limit is exceeded. ## Testing - Cover descriptions beyond the former limit and multibyte truncation at 512 KiB. - Update SSE and stdio MCP tests to verify that complete server instructions are preserved without hiding tools. GitOrigin-RevId: 000bfcafb3df348065ae451685bfbf978a0e3248
This commit is contained in:
@@ -31,7 +31,7 @@ use serde_json::Value;
|
||||
|
||||
const LEGACY_MCP_TOOL_NAME_PREFIX: &str = "mcp__";
|
||||
const MCP_TOOL_NAME_DELIMITER: &str = "__";
|
||||
const MAX_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 1_000;
|
||||
const MAX_MCP_NAMESPACE_DESCRIPTION_BYTES: usize = 512 * 1024;
|
||||
|
||||
pub struct McpHandler {
|
||||
tool_info: ToolInfo,
|
||||
@@ -106,7 +106,6 @@ impl ToolExecutor<ToolInvocation> for McpHandler {
|
||||
.as_deref()
|
||||
.map(str::trim)
|
||||
.filter(|description| !description.is_empty())
|
||||
.map(bounded_mcp_namespace_description)
|
||||
.map(str::to_string),
|
||||
});
|
||||
|
||||
@@ -258,15 +257,12 @@ fn create_tool_spec(tool_info: &ToolInfo) -> Result<ToolSpec, serde_json::Error>
|
||||
|
||||
Ok(ToolSpec::Namespace(ResponsesApiNamespace {
|
||||
name: tool_info.callable_namespace.clone(),
|
||||
description: bounded_mcp_namespace_description(&description).to_string(),
|
||||
description: take_bytes_at_char_boundary(&description, MAX_MCP_NAMESPACE_DESCRIPTION_BYTES)
|
||||
.to_string(),
|
||||
tools: vec![ResponsesApiNamespaceTool::Function(tool)],
|
||||
}))
|
||||
}
|
||||
|
||||
fn bounded_mcp_namespace_description(description: &str) -> &str {
|
||||
take_bytes_at_char_boundary(description, MAX_MCP_NAMESPACE_DESCRIPTION_BYTES)
|
||||
}
|
||||
|
||||
fn mcp_hook_tool_input(raw_arguments: &str) -> Value {
|
||||
if raw_arguments.trim().is_empty() {
|
||||
return Value::Object(Map::new());
|
||||
|
||||
@@ -43,9 +43,8 @@ fn search_info_uses_connector_name_for_output_namespace_description() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_namespace_descriptions_are_bounded_without_mutating_metadata() {
|
||||
let expected_description = "é".repeat(499);
|
||||
let full_description = format!("{expected_description}🦀keep the complete app metadata");
|
||||
fn mcp_namespace_descriptions_preserve_complete_metadata() {
|
||||
let full_description = format!("{}🦀keep the complete app metadata", "é".repeat(499));
|
||||
let mut info = tool_info();
|
||||
info.namespace_description = Some(full_description.clone());
|
||||
let handler = McpHandler::new(info).expect("MCP tool spec should build");
|
||||
@@ -55,17 +54,39 @@ fn mcp_namespace_descriptions_are_bounded_without_mutating_metadata() {
|
||||
search_info.source_info,
|
||||
Some(ToolSearchSourceInfo {
|
||||
name: "Calendar".to_string(),
|
||||
description: Some(expected_description.clone()),
|
||||
description: Some(full_description.clone()),
|
||||
})
|
||||
);
|
||||
let LoadableToolSpec::Namespace(namespace) = search_info.entry.output else {
|
||||
panic!("expected namespace search output");
|
||||
};
|
||||
assert_eq!(namespace.description, full_description);
|
||||
assert_eq!(
|
||||
handler.tool_info.namespace_description,
|
||||
Some(full_description)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_namespace_descriptions_are_bounded_at_512_kib() {
|
||||
let expected_description = "é".repeat(MAX_MCP_NAMESPACE_DESCRIPTION_BYTES / 2 - 1);
|
||||
let full_description = format!("{expected_description}🦀overflow");
|
||||
let mut info = tool_info();
|
||||
info.namespace_description = Some(full_description.clone());
|
||||
let handler = McpHandler::new(info).expect("MCP tool spec should build");
|
||||
let search_info = handler.search_info().expect("MCP search info");
|
||||
|
||||
assert_eq!(
|
||||
search_info.source_info,
|
||||
Some(ToolSearchSourceInfo {
|
||||
name: "Calendar".to_string(),
|
||||
description: Some(full_description),
|
||||
})
|
||||
);
|
||||
let LoadableToolSpec::Namespace(namespace) = search_info.entry.output else {
|
||||
panic!("expected namespace search output");
|
||||
};
|
||||
assert_eq!(namespace.description, expected_description);
|
||||
assert_eq!(
|
||||
handler.tool_info.namespace_description,
|
||||
Some(full_description)
|
||||
);
|
||||
}
|
||||
|
||||
fn tool_info() -> ToolInfo {
|
||||
|
||||
@@ -545,7 +545,7 @@ fn assert_cwd_tool_output(structured: &Value, expected_cwd: &Path) {
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn mcp_namespace_instructions_are_bounded_without_hiding_tools() -> anyhow::Result<()> {
|
||||
async fn mcp_namespace_instructions_are_preserved_without_hiding_tools() -> anyhow::Result<()> {
|
||||
skip_if_wine_exec!(
|
||||
Ok(()),
|
||||
"requires a Windows test_stdio_server in the Wine-exec environment"
|
||||
@@ -553,8 +553,8 @@ async fn mcp_namespace_instructions_are_bounded_without_hiding_tools() -> anyhow
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = responses::start_mock_server().await;
|
||||
let expected_description = "é".repeat(499);
|
||||
let instructions = format!("{expected_description}🦀keep the valid MCP server");
|
||||
let expected_description = format!("{}🦀keep the valid MCP server", "é".repeat(499));
|
||||
let instructions = expected_description.clone();
|
||||
let response = mount_sse_once(
|
||||
&server,
|
||||
responses::sse(vec![
|
||||
@@ -614,7 +614,7 @@ async fn mcp_namespace_instructions_are_bounded_without_hiding_tools() -> anyhow
|
||||
);
|
||||
assert!(
|
||||
responses::namespace_child_tool(&body, "mcp__bounded", "echo").is_some(),
|
||||
"bounding the namespace must not hide a valid MCP tool"
|
||||
"preserving the namespace must not hide a valid MCP tool"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
@@ -672,8 +672,8 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
|
||||
.await;
|
||||
|
||||
let expected_env_value = "propagated-env";
|
||||
let expected_description = "é".repeat(499);
|
||||
let instructions = format!("{expected_description}🦀keep the complete MCP metadata");
|
||||
let expected_description = format!("{}🦀keep the complete MCP metadata", "é".repeat(499));
|
||||
let instructions = expected_description.clone();
|
||||
let rmcp_test_server_bin = remote_aware_stdio_server_bin()?;
|
||||
|
||||
let fixture = test_codex()
|
||||
@@ -769,7 +769,7 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
|
||||
"the complete tool search description must remain bounded"
|
||||
);
|
||||
assert!(search_description.contains(&format!("- rmcp: {expected_description}")));
|
||||
assert!(!search_description.contains("🦀keep the complete MCP metadata"));
|
||||
assert!(search_description.contains("🦀keep the complete MCP metadata"));
|
||||
|
||||
let search_output = call_mock
|
||||
.single_request()
|
||||
|
||||
Reference in New Issue
Block a user