mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: replace custom mcp-types crate with equivalents from rmcp (#10349)
We started working with MCP in Codex before https://crates.io/crates/rmcp was mature, so we had our own crate for MCP types that was generated from the MCP schema: https://github.com/openai/codex/blob/8b95d3e082376f4cb23e92641705a22afb28a9da/codex-rs/mcp-types/README.md Now that `rmcp` is more mature, it makes more sense to use their MCP types in Rust, as they handle details (like the `_meta` field) that our custom version ignored. Though one advantage that our custom types had is that our generated types implemented `JsonSchema` and `ts_rs::TS`, whereas the types in `rmcp` do not. As such, part of the work of this PR is leveraging the adapters between `rmcp` types and the serializable types that are API for us (app server and MCP) introduced in #10356. Note this PR results in a number of changes to `codex-rs/app-server-protocol/schema`, which merit special attention during review. We must ensure that these changes are still backwards-compatible, which is possible because we have: ```diff - export type CallToolResult = { content: Array<ContentBlock>, isError?: boolean, structuredContent?: JsonValue, }; + export type CallToolResult = { content: Array<JsonValue>, structuredContent?: JsonValue, isError?: boolean, _meta?: JsonValue, }; ``` so `ContentBlock` has been replaced with the more general `JsonValue`. Note that `ContentBlock` was defined as: ```typescript export type ContentBlock = TextContent | ImageContent | AudioContent | ResourceLink | EmbeddedResource; ``` so the deletion of those individual variants should not be a cause of great concern. Similarly, we have the following change in `codex-rs/app-server-protocol/schema/typescript/Tool.ts`: ``` - export type Tool = { annotations?: ToolAnnotations, description?: string, inputSchema: ToolInputSchema, name: string, outputSchema?: ToolOutputSchema, title?: string, }; + export type Tool = { name: string, title?: string, description?: string, inputSchema: JsonValue, outputSchema?: JsonValue, annotations?: JsonValue, icons?: Array<JsonValue>, _meta?: JsonValue, }; ``` so: - `annotations?: ToolAnnotations` ➡️ `JsonValue` - `inputSchema: ToolInputSchema` ➡️ `JsonValue` - `outputSchema?: ToolOutputSchema` ➡️ `JsonValue` and two new fields: `icons?: Array<JsonValue>, _meta?: JsonValue` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/10349). * #10357 * __->__ #10349 * #10356
This commit is contained in:
@@ -1,9 +1,9 @@
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::mcp::RequestId;
|
||||
use crate::parse_command::ParsedCommand;
|
||||
use crate::protocol::FileChange;
|
||||
use mcp_types::RequestId;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
@@ -62,6 +62,7 @@ pub struct ExecApprovalRequestEvent {
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct ElicitationRequestEvent {
|
||||
pub server_name: String,
|
||||
#[ts(type = "string | number")]
|
||||
pub id: RequestId,
|
||||
pub message: String,
|
||||
// TODO: MCP servers can request we fill out a schema for the elicitation. We don't support
|
||||
|
||||
@@ -1,14 +1,14 @@
|
||||
//! Types used when representing Model Context Protocol (MCP) values inside the
|
||||
//! Codex protocol.
|
||||
//!
|
||||
//! We intentionally keep these types TS/JSON-schema friendly (via `ts-rs` and
|
||||
//! `schemars`) so they can be embedded in Codex's own protocol structures.
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use ts_rs::TS;
|
||||
|
||||
/// Types used when representing Model Context Protocol (MCP) values inside the
|
||||
/// Codex protocol.
|
||||
///
|
||||
/// We intentionally keep these types TS/JSON-schema friendly (via `ts-rs` and
|
||||
/// `schemars`) so they can be embedded in Codex's own protocol structures.
|
||||
|
||||
/// ID of a request, which can be either a string or an integer.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)]
|
||||
#[serde(untagged)]
|
||||
pub enum RequestId {
|
||||
@@ -26,6 +26,7 @@ impl std::fmt::Display for RequestId {
|
||||
}
|
||||
}
|
||||
|
||||
/// Definition for a tool the client can call.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct Tool {
|
||||
@@ -51,6 +52,7 @@ pub struct Tool {
|
||||
pub meta: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
/// A known resource that the server is capable of reading.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct Resource {
|
||||
@@ -80,6 +82,7 @@ pub struct Resource {
|
||||
pub meta: Option<serde_json::Value>,
|
||||
}
|
||||
|
||||
/// A template description for resources available on the server.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct ResourceTemplate {
|
||||
@@ -99,6 +102,7 @@ pub struct ResourceTemplate {
|
||||
pub mime_type: Option<String>,
|
||||
}
|
||||
|
||||
/// The server's response to a tool call.
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct CallToolResult {
|
||||
|
||||
+112
-37
@@ -2,8 +2,6 @@ use std::collections::HashMap;
|
||||
use std::path::Path;
|
||||
|
||||
use codex_utils_image::load_and_resize_to_fit;
|
||||
use mcp_types::CallToolResult;
|
||||
use mcp_types::ContentBlock;
|
||||
use serde::Deserialize;
|
||||
use serde::Deserializer;
|
||||
use serde::Serialize;
|
||||
@@ -24,6 +22,8 @@ use codex_git::GhostCommit;
|
||||
use codex_utils_image::error::ImageProcessingError;
|
||||
use schemars::JsonSchema;
|
||||
|
||||
use crate::mcp::CallToolResult;
|
||||
|
||||
/// Controls whether a command should use the session sandbox or bypass it.
|
||||
#[derive(
|
||||
Debug, Clone, Copy, Default, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema, TS,
|
||||
@@ -833,6 +833,7 @@ impl From<&CallToolResult> for FunctionCallOutputPayload {
|
||||
content,
|
||||
structured_content,
|
||||
is_error,
|
||||
meta: _,
|
||||
} = call_tool_result;
|
||||
|
||||
let is_success = is_error != &Some(true);
|
||||
@@ -869,7 +870,7 @@ impl From<&CallToolResult> for FunctionCallOutputPayload {
|
||||
}
|
||||
};
|
||||
|
||||
let content_items = convert_content_blocks_to_items(content);
|
||||
let content_items = convert_mcp_content_to_items(content);
|
||||
|
||||
FunctionCallOutputPayload {
|
||||
content: serialized_content,
|
||||
@@ -879,32 +880,45 @@ impl From<&CallToolResult> for FunctionCallOutputPayload {
|
||||
}
|
||||
}
|
||||
|
||||
fn convert_content_blocks_to_items(
|
||||
blocks: &[ContentBlock],
|
||||
fn convert_mcp_content_to_items(
|
||||
contents: &[serde_json::Value],
|
||||
) -> Option<Vec<FunctionCallOutputContentItem>> {
|
||||
#[derive(serde::Deserialize)]
|
||||
#[serde(tag = "type")]
|
||||
enum McpContent {
|
||||
#[serde(rename = "text")]
|
||||
Text { text: String },
|
||||
#[serde(rename = "image")]
|
||||
Image {
|
||||
data: String,
|
||||
#[serde(rename = "mimeType", alias = "mime_type")]
|
||||
mime_type: Option<String>,
|
||||
},
|
||||
#[serde(other)]
|
||||
Unknown,
|
||||
}
|
||||
|
||||
let mut saw_image = false;
|
||||
let mut items = Vec::with_capacity(blocks.len());
|
||||
tracing::warn!("Blocks: {:?}", blocks);
|
||||
for block in blocks {
|
||||
match block {
|
||||
ContentBlock::TextContent(text) => {
|
||||
items.push(FunctionCallOutputContentItem::InputText {
|
||||
text: text.text.clone(),
|
||||
});
|
||||
}
|
||||
ContentBlock::ImageContent(image) => {
|
||||
let mut items = Vec::with_capacity(contents.len());
|
||||
|
||||
for content in contents {
|
||||
let item = match serde_json::from_value::<McpContent>(content.clone()) {
|
||||
Ok(McpContent::Text { text }) => FunctionCallOutputContentItem::InputText { text },
|
||||
Ok(McpContent::Image { data, mime_type }) => {
|
||||
saw_image = true;
|
||||
// Just in case the content doesn't include a data URL, add it.
|
||||
let image_url = if image.data.starts_with("data:") {
|
||||
image.data.clone()
|
||||
let image_url = if data.starts_with("data:") {
|
||||
data
|
||||
} else {
|
||||
format!("data:{};base64,{}", image.mime_type, image.data)
|
||||
let mime_type = mime_type.unwrap_or_else(|| "application/octet-stream".into());
|
||||
format!("data:{mime_type};base64,{data}")
|
||||
};
|
||||
items.push(FunctionCallOutputContentItem::InputImage { image_url });
|
||||
FunctionCallOutputContentItem::InputImage { image_url }
|
||||
}
|
||||
// TODO: render audio, resource, and embedded resource content to the model.
|
||||
_ => return None,
|
||||
}
|
||||
Ok(McpContent::Unknown) | Err(_) => FunctionCallOutputContentItem::InputText {
|
||||
text: serde_json::to_string(content).unwrap_or_else(|_| "<content>".to_string()),
|
||||
},
|
||||
};
|
||||
items.push(item);
|
||||
}
|
||||
|
||||
if saw_image { Some(items) } else { None }
|
||||
@@ -936,12 +950,54 @@ mod tests {
|
||||
use crate::protocol::AskForApproval;
|
||||
use anyhow::Result;
|
||||
use codex_execpolicy::Policy;
|
||||
use mcp_types::ImageContent;
|
||||
use mcp_types::TextContent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn convert_mcp_content_to_items_preserves_data_urls() {
|
||||
let contents = vec![serde_json::json!({
|
||||
"type": "image",
|
||||
"data": "data:image/png;base64,Zm9v",
|
||||
"mimeType": "image/png",
|
||||
})];
|
||||
|
||||
let items = convert_mcp_content_to_items(&contents).expect("expected image items");
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,Zm9v".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn convert_mcp_content_to_items_builds_data_urls_when_missing_prefix() {
|
||||
let contents = vec![serde_json::json!({
|
||||
"type": "image",
|
||||
"data": "Zm9v",
|
||||
"mimeType": "image/png",
|
||||
})];
|
||||
|
||||
let items = convert_mcp_content_to_items(&contents).expect("expected image items");
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,Zm9v".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn convert_mcp_content_to_items_returns_none_without_images() {
|
||||
let contents = vec![serde_json::json!({
|
||||
"type": "text",
|
||||
"text": "hello",
|
||||
})];
|
||||
|
||||
assert_eq!(convert_mcp_content_to_items(&contents), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn converts_sandbox_mode_into_developer_instructions() {
|
||||
let workspace_write: DeveloperInstructions = SandboxMode::WorkspaceWrite.into();
|
||||
@@ -1124,20 +1180,12 @@ mod tests {
|
||||
fn serializes_image_outputs_as_array() -> Result<()> {
|
||||
let call_tool_result = CallToolResult {
|
||||
content: vec![
|
||||
ContentBlock::TextContent(TextContent {
|
||||
annotations: None,
|
||||
text: "caption".into(),
|
||||
r#type: "text".into(),
|
||||
}),
|
||||
ContentBlock::ImageContent(ImageContent {
|
||||
annotations: None,
|
||||
data: "BASE64".into(),
|
||||
mime_type: "image/png".into(),
|
||||
r#type: "image".into(),
|
||||
}),
|
||||
serde_json::json!({"type":"text","text":"caption"}),
|
||||
serde_json::json!({"type":"image","data":"BASE64","mimeType":"image/png"}),
|
||||
],
|
||||
is_error: None,
|
||||
structured_content: None,
|
||||
is_error: Some(false),
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let payload = FunctionCallOutputPayload::from(&call_tool_result);
|
||||
@@ -1169,6 +1217,33 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_existing_image_data_urls() -> Result<()> {
|
||||
let call_tool_result = CallToolResult {
|
||||
content: vec![serde_json::json!({
|
||||
"type": "image",
|
||||
"data": "data:image/png;base64,BASE64",
|
||||
"mimeType": "image/png"
|
||||
})],
|
||||
structured_content: None,
|
||||
is_error: Some(false),
|
||||
meta: None,
|
||||
};
|
||||
|
||||
let payload = FunctionCallOutputPayload::from(&call_tool_result);
|
||||
let Some(items) = payload.content_items else {
|
||||
panic!("expected content items");
|
||||
};
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![FunctionCallOutputContentItem::InputImage {
|
||||
image_url: "data:image/png;base64,BASE64".into(),
|
||||
}]
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserializes_array_payload_into_items() -> Result<()> {
|
||||
let json = r#"[
|
||||
|
||||
@@ -23,6 +23,11 @@ use crate::dynamic_tools::DynamicToolCallRequest;
|
||||
use crate::dynamic_tools::DynamicToolResponse;
|
||||
use crate::dynamic_tools::DynamicToolSpec;
|
||||
use crate::items::TurnItem;
|
||||
use crate::mcp::CallToolResult;
|
||||
use crate::mcp::RequestId;
|
||||
use crate::mcp::Resource as McpResource;
|
||||
use crate::mcp::ResourceTemplate as McpResourceTemplate;
|
||||
use crate::mcp::Tool as McpTool;
|
||||
use crate::message_history::HistoryEntry;
|
||||
use crate::models::BaseInstructions;
|
||||
use crate::models::ContentItem;
|
||||
@@ -35,11 +40,6 @@ use crate::plan_tool::UpdatePlanArgs;
|
||||
use crate::request_user_input::RequestUserInputResponse;
|
||||
use crate::user_input::UserInput;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use mcp_types::CallToolResult;
|
||||
use mcp_types::RequestId;
|
||||
use mcp_types::Resource as McpResource;
|
||||
use mcp_types::ResourceTemplate as McpResourceTemplate;
|
||||
use mcp_types::Tool as McpTool;
|
||||
use schemars::JsonSchema;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
|
||||
Reference in New Issue
Block a user