mirror of
https://github.com/openai/codex.git
synced 2026-11-06 20:20:34 +00:00
codex: preserve MCP file metadata for execution
This commit is contained in:
@@ -508,6 +508,16 @@ impl McpConnectionManager {
|
||||
normalize_tools_for_model_with_prefix(tools, self.prefix_mcp_tool_names)
|
||||
}
|
||||
|
||||
/// Returns an unshaped tool definition for execution-time metadata lookup.
|
||||
pub async fn tool_info_for_execution(&self, server: &str, tool_name: &str) -> Option<ToolInfo> {
|
||||
self.clients
|
||||
.get(server)?
|
||||
.listed_tools()
|
||||
.await?
|
||||
.into_iter()
|
||||
.find(|tool| tool.server_name == server && tool.tool.name == tool_name)
|
||||
}
|
||||
|
||||
/// Force-refresh codex apps tools by bypassing the in-process cache.
|
||||
///
|
||||
/// On success, the refreshed tools replace the cache contents and the
|
||||
|
||||
@@ -1479,10 +1479,7 @@ pub(crate) async fn lookup_mcp_tool_metadata(
|
||||
let plugin_id = manager
|
||||
.plugin_id_for_mcp_server_name(server)
|
||||
.map(str::to_string);
|
||||
let tools = manager.list_all_tools().await;
|
||||
let tool_info = tools
|
||||
.into_iter()
|
||||
.find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
|
||||
let tool_info = manager.tool_info_for_execution(server, tool_name).await?;
|
||||
let connector_description = if server == CODEX_APPS_MCP_SERVER_NAME {
|
||||
let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools(
|
||||
turn_context.config.as_ref(),
|
||||
|
||||
@@ -565,6 +565,116 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
|
||||
async fn stdio_server_adapts_inline_mcp_file_input() -> anyhow::Result<()> {
|
||||
let model_server = responses::start_mock_server().await;
|
||||
let files = tempdir()?;
|
||||
let upload_path = files.path().join("upload.txt");
|
||||
fs::write(&upload_path, b"upload me")?;
|
||||
let call_id = "file-call-1";
|
||||
let server_name = "rmcp_files";
|
||||
let namespace = format!("mcp__{server_name}");
|
||||
let arguments = serde_json::to_string(&json!({"file": upload_path}))?;
|
||||
let response_mock = responses::mount_sse_sequence(
|
||||
&model_server,
|
||||
vec![
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("resp-1"),
|
||||
responses::ev_function_call_with_namespace(
|
||||
call_id,
|
||||
&namespace,
|
||||
"file_round_trip",
|
||||
&arguments,
|
||||
),
|
||||
responses::ev_completed("resp-1"),
|
||||
]),
|
||||
responses::sse(vec![
|
||||
responses::ev_assistant_message("msg-1", "MCP file transfer completed."),
|
||||
responses::ev_completed("resp-2"),
|
||||
]),
|
||||
],
|
||||
)
|
||||
.await;
|
||||
|
||||
let rmcp_test_server_bin = stdio_server_bin()?;
|
||||
let fixture = test_codex()
|
||||
.with_config(move |config| {
|
||||
config
|
||||
.features
|
||||
.enable(Feature::McpFileTransfer)
|
||||
.expect("test feature should enable");
|
||||
insert_mcp_server(
|
||||
config,
|
||||
server_name,
|
||||
stdio_transport(
|
||||
rmcp_test_server_bin,
|
||||
Some(HashMap::from([(
|
||||
"MCP_TEST_FILE_INLINE".to_string(),
|
||||
"1".to_string(),
|
||||
)])),
|
||||
Vec::new(),
|
||||
),
|
||||
TestMcpServerOptions::default(),
|
||||
);
|
||||
})
|
||||
.build(&model_server)
|
||||
.await?;
|
||||
wait_for_mcp_server(&fixture.codex, server_name).await?;
|
||||
|
||||
fixture
|
||||
.codex
|
||||
.submit(auto_approved_user_turn(&fixture, "send the local file"))
|
||||
.await?;
|
||||
|
||||
let begin_event = wait_for_event(&fixture.codex, |event| {
|
||||
matches!(event, EventMsg::McpToolCallBegin(_))
|
||||
})
|
||||
.await;
|
||||
let EventMsg::McpToolCallBegin(begin) = begin_event else {
|
||||
unreachable!("event guard guarantees McpToolCallBegin");
|
||||
};
|
||||
assert_eq!(
|
||||
begin.invocation.arguments,
|
||||
Some(json!({"file": upload_path}))
|
||||
);
|
||||
|
||||
let end_event = wait_for_event(&fixture.codex, |event| {
|
||||
matches!(event, EventMsg::McpToolCallEnd(_))
|
||||
})
|
||||
.await;
|
||||
let EventMsg::McpToolCallEnd(end) = end_event else {
|
||||
unreachable!("event guard guarantees McpToolCallEnd");
|
||||
};
|
||||
let structured = end
|
||||
.result
|
||||
.as_ref()
|
||||
.expect("file tool should succeed")
|
||||
.structured_content
|
||||
.as_ref()
|
||||
.expect("file tool should return structured content");
|
||||
assert_eq!(structured, &json!({"received_inline": true}));
|
||||
|
||||
wait_for_event(&fixture.codex, |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
let requests = response_mock.requests();
|
||||
let output = requests[1].function_call_output(call_id);
|
||||
let output_text = output["output"].as_str().expect("function output text");
|
||||
assert!(!output_text.contains("data:"));
|
||||
assert!(!output_text.contains("mcp-file://"));
|
||||
let tool = requests[0]
|
||||
.tool_by_name(&namespace, "file_round_trip")
|
||||
.expect("file tool should be model visible");
|
||||
assert_eq!(tool["parameters"]["properties"]["file"]["type"], "string");
|
||||
assert!(
|
||||
tool["parameters"]["properties"]["file"]
|
||||
.get("x-mcp-file")
|
||||
.is_none()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shutdown_cancels_startup_prewarm_waiting_for_mcp_startup() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -44,6 +44,7 @@ const MEMO_URI: &str = "memo://codex/example-note";
|
||||
const MEMO_CONTENT: &str = "This is a sample MCP resource served by the rmcp test server.";
|
||||
const SANDBOX_STATE_META_CAPABILITY: &str = "codex/sandbox-state-meta";
|
||||
const SMALL_PNG_BASE64: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||
const FILE_INLINE_ENV: &str = "MCP_TEST_FILE_INLINE";
|
||||
|
||||
pub fn stdio() -> (tokio::io::Stdin, tokio::io::Stdout) {
|
||||
(tokio::io::stdin(), tokio::io::stdout())
|
||||
@@ -65,7 +66,7 @@ impl TestToolServer {
|
||||
);
|
||||
sandbox_meta_tool.annotations = Some(ToolAnnotations::new().read_only(true));
|
||||
|
||||
let tools = vec![
|
||||
let mut tools = vec![
|
||||
Self::echo_tool(),
|
||||
Self::echo_dash_tool(),
|
||||
Self::cwd_tool(),
|
||||
@@ -75,6 +76,9 @@ impl TestToolServer {
|
||||
Self::image_scenario_tool(),
|
||||
sandbox_meta_tool,
|
||||
];
|
||||
if std::env::var_os(FILE_INLINE_ENV).is_some() {
|
||||
tools.push(Self::file_round_trip_tool());
|
||||
}
|
||||
let resources = vec![Self::memo_resource()];
|
||||
let resource_templates = vec![Self::memo_template()];
|
||||
Self {
|
||||
@@ -166,6 +170,32 @@ impl TestToolServer {
|
||||
tool
|
||||
}
|
||||
|
||||
fn file_round_trip_tool() -> Tool {
|
||||
#[expect(clippy::expect_used)]
|
||||
let schema: JsonObject = serde_json::from_value(json!({
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"file": {
|
||||
"type": "string",
|
||||
"format": "uri",
|
||||
"x-mcp-file": {
|
||||
"accept": ["text/plain"],
|
||||
"maxSize": 1024,
|
||||
"transferModes": ["inline"]
|
||||
}
|
||||
}
|
||||
},
|
||||
"required": ["file"],
|
||||
"additionalProperties": false
|
||||
}))
|
||||
.expect("file round-trip tool schema should deserialize");
|
||||
Tool::new(
|
||||
Cow::Borrowed("file_round_trip"),
|
||||
Cow::Borrowed("Accept an uploaded file and return a downloadable file."),
|
||||
Arc::new(schema),
|
||||
)
|
||||
}
|
||||
|
||||
fn sync_tool() -> Tool {
|
||||
#[expect(clippy::expect_used)]
|
||||
let schema: JsonObject = serde_json::from_value(json!({
|
||||
@@ -395,6 +425,11 @@ struct ImageScenarioArgs {
|
||||
data_url: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize, Debug)]
|
||||
struct FileRoundTripArgs {
|
||||
file: String,
|
||||
}
|
||||
|
||||
impl ServerHandler for TestToolServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
let mut capabilities = ServerCapabilities::builder()
|
||||
@@ -481,6 +516,16 @@ impl ServerHandler for TestToolServer {
|
||||
context: rmcp::service::RequestContext<rmcp::service::RoleServer>,
|
||||
) -> Result<CallToolResult, McpError> {
|
||||
match request.name.as_ref() {
|
||||
"file_round_trip" => {
|
||||
let args = Self::parse_call_args::<FileRoundTripArgs>(&request, "file_round_trip")?;
|
||||
if args.file != "data:text/plain;base64,dXBsb2FkIG1l" {
|
||||
return Err(McpError::invalid_params(
|
||||
format!("expected inline file data, got {}", args.file),
|
||||
None,
|
||||
));
|
||||
}
|
||||
Ok(Self::structured_result(json!({"received_inline": true})))
|
||||
}
|
||||
"sandbox_meta" => Ok(Self::structured_result(serde_json::Value::Object(
|
||||
context.meta.0,
|
||||
))),
|
||||
|
||||
Reference in New Issue
Block a user