moving input item from MCP Protocol back to core Protocol (#1740)

- Currently we have duplicate input item. Let's have one source of truth
in the core.
- Used Requestid type
This commit is contained in:
aibrahim-oai
2025-07-30 13:43:08 -07:00
committed by GitHub
parent ea01a5ffe2
commit 2f5557056d

View File

@@ -1,6 +1,7 @@
use codex_core::config_types::SandboxMode; use codex_core::config_types::SandboxMode;
use codex_core::protocol::AskForApproval; use codex_core::protocol::AskForApproval;
use codex_core::protocol::EventMsg; use codex_core::protocol::EventMsg;
use codex_core::protocol::InputItem;
use serde::Deserialize; use serde::Deserialize;
use serde::Serialize; use serde::Serialize;
use strum_macros::Display; use strum_macros::Display;
@@ -21,7 +22,7 @@ pub struct MessageId(pub Uuid);
pub struct ToolCallRequest { pub struct ToolCallRequest {
#[serde(rename = "jsonrpc")] #[serde(rename = "jsonrpc")]
pub jsonrpc: &'static str, pub jsonrpc: &'static str,
pub id: u64, pub id: RequestId,
pub method: &'static str, pub method: &'static str,
pub params: ToolCallRequestParams, pub params: ToolCallRequestParams,
} }
@@ -38,7 +39,7 @@ pub enum ToolCallRequestParams {
impl ToolCallRequestParams { impl ToolCallRequestParams {
/// Wrap this request in a JSON-RPC request. /// Wrap this request in a JSON-RPC request.
#[allow(dead_code)] #[allow(dead_code)]
pub fn into_request(self, id: u64) -> ToolCallRequest { pub fn into_request(self, id: RequestId) -> ToolCallRequest {
ToolCallRequest { ToolCallRequest {
jsonrpc: "2.0", jsonrpc: "2.0",
id, id,
@@ -95,70 +96,13 @@ pub struct ConversationStreamArgs {
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ConversationSendMessageArgs { pub struct ConversationSendMessageArgs {
pub conversation_id: ConversationId, pub conversation_id: ConversationId,
pub content: Vec<MessageInputItem>, pub content: Vec<InputItem>,
#[serde(default, skip_serializing_if = "Option::is_none")] #[serde(default, skip_serializing_if = "Option::is_none")]
pub parent_message_id: Option<MessageId>, pub parent_message_id: Option<MessageId>,
#[serde(default, skip_serializing_if = "Option::is_none")] #[serde(default, skip_serializing_if = "Option::is_none")]
#[serde(flatten)] #[serde(flatten)]
pub conversation_overrides: Option<ConversationOverrides>, pub conversation_overrides: Option<ConversationOverrides>,
} }
/// Input items for a message.
/// Following OpenAI's Responses API: https://platform.openai.com/docs/api-reference/responses
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "camelCase")]
pub enum MessageInputItem {
Text {
text: String,
},
Image {
#[serde(flatten)]
source: ImageSource,
#[serde(default, skip_serializing_if = "Option::is_none")]
detail: Option<ImageDetail>,
},
File {
#[serde(flatten)]
source: FileSource,
},
}
/// Source of an image.
/// Following OpenAI's API: https://platform.openai.com/docs/guides/images-vision#giving-a-model-images-as-input
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum ImageSource {
ImageUrl { image_url: String },
FileId { file_id: String },
}
/// Source of a file.
/// Following OpenAI's Responses API: https://platform.openai.com/docs/guides/pdf-files?api-mode=responses#uploading-files
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(untagged)]
pub enum FileSource {
Url {
file_url: String,
},
Id {
file_id: String,
},
Base64 {
#[serde(default, skip_serializing_if = "Option::is_none")]
filename: Option<String>,
// Base64-encoded file contents.
file_data: String,
},
}
#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub enum ImageDetail {
Low,
High,
Auto,
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct ConversationsListArgs { pub struct ConversationsListArgs {
#[serde(default, skip_serializing_if = "Option::is_none")] #[serde(default, skip_serializing_if = "Option::is_none")]
@@ -303,6 +247,8 @@ pub enum ClientNotification {
#[allow(clippy::expect_used)] #[allow(clippy::expect_used)]
#[allow(clippy::unwrap_used)] #[allow(clippy::unwrap_used)]
mod tests { mod tests {
use std::path::PathBuf;
use super::*; use super::*;
use codex_core::protocol::McpInvocation; use codex_core::protocol::McpInvocation;
use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallBeginEvent;
@@ -331,7 +277,7 @@ mod tests {
base_instructions: None, base_instructions: None,
}); });
let observed = to_val(&req.into_request(2)); let observed = to_val(&req.into_request(mcp_types::RequestId::Integer(2)));
let expected = json!({ let expected = json!({
"jsonrpc": "2.0", "jsonrpc": "2.0",
"id": 2, "id": 2,
@@ -354,18 +300,12 @@ mod tests {
let req = ToolCallRequestParams::ConversationSendMessage(ConversationSendMessageArgs { let req = ToolCallRequestParams::ConversationSendMessage(ConversationSendMessageArgs {
conversation_id: ConversationId(uuid!("d0f6ecbe-84a2-41c1-b23d-b20473b25eab")), conversation_id: ConversationId(uuid!("d0f6ecbe-84a2-41c1-b23d-b20473b25eab")),
content: vec![ content: vec![
MessageInputItem::Text { text: "Hi".into() }, InputItem::Text { text: "Hi".into() },
MessageInputItem::Image { InputItem::Image {
source: ImageSource::ImageUrl { image_url: "https://example.com/cat.jpg".into(),
image_url: "https://example.com/cat.jpg".into(),
},
detail: Some(ImageDetail::High),
}, },
MessageInputItem::File { InputItem::LocalImage {
source: FileSource::Base64 { path: "notes.txt".into(),
filename: Some("notes.txt".into()),
file_data: "Zm9vYmFy".into(),
},
}, },
], ],
parent_message_id: Some(MessageId(uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8"))), parent_message_id: Some(MessageId(uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8"))),
@@ -380,7 +320,7 @@ mod tests {
}), }),
}); });
let observed = to_val(&req.into_request(2)); let observed = to_val(&req.into_request(mcp_types::RequestId::Integer(2)));
let expected = json!({ let expected = json!({
"jsonrpc": "2.0", "jsonrpc": "2.0",
"id": 2, "id": 2,
@@ -391,8 +331,8 @@ mod tests {
"conversation_id": "d0f6ecbe-84a2-41c1-b23d-b20473b25eab", "conversation_id": "d0f6ecbe-84a2-41c1-b23d-b20473b25eab",
"content": [ "content": [
{ "type": "text", "text": "Hi" }, { "type": "text", "text": "Hi" },
{ "type": "image", "image_url": "https://example.com/cat.jpg", "detail": "high" }, { "type": "image", "image_url": "https://example.com/cat.jpg" },
{ "type": "file", "filename": "notes.txt", "file_data": "Zm9vYmFy" } { "type": "local_image", "path": "notes.txt" }
], ],
"parent_message_id": "67e55044-10b1-426f-9247-bb680e5fe0c8", "parent_message_id": "67e55044-10b1-426f-9247-bb680e5fe0c8",
"model": "o4-mini", "model": "o4-mini",
@@ -414,7 +354,7 @@ mod tests {
cursor: Some("abc".into()), cursor: Some("abc".into()),
}); });
let observed = to_val(&req.into_request(2)); let observed = to_val(&req.into_request(RequestId::Integer(2)));
let expected = json!({ let expected = json!({
"jsonrpc": "2.0", "jsonrpc": "2.0",
"id": 2, "id": 2,
@@ -436,7 +376,7 @@ mod tests {
conversation_id: ConversationId(uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8")), conversation_id: ConversationId(uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8")),
}); });
let observed = to_val(&req.into_request(2)); let observed = to_val(&req.into_request(mcp_types::RequestId::Integer(2)));
let expected = json!({ let expected = json!({
"jsonrpc": "2.0", "jsonrpc": "2.0",
"id": 2, "id": 2,
@@ -454,48 +394,44 @@ mod tests {
// ----- Message inputs / sources ----- // ----- Message inputs / sources -----
#[test] #[test]
fn serialize_message_input_image_file_id_auto_detail() { fn serialize_message_input_image_url() {
let item = MessageInputItem::Image { let item = InputItem::Image {
source: ImageSource::FileId { image_url: "https://example.com/x.png".into(),
file_id: "file_123".into(),
},
detail: Some(ImageDetail::Auto),
}; };
let observed = to_val(&item); let observed = to_val(&item);
let expected = json!({ let expected = json!({
"type": "image", "type": "image",
"file_id": "file_123", "image_url": "https://example.com/x.png"
"detail": "auto"
}); });
assert_eq!(observed, expected); assert_eq!(observed, expected);
} }
#[test] #[test]
fn serialize_message_input_file_url_and_id_variants() { fn serialize_message_input_local_image_path() {
let url = MessageInputItem::File { let url = InputItem::LocalImage {
source: FileSource::Url { path: PathBuf::from("https://example.com/a.pdf"),
file_url: "https://example.com/a.pdf".into(),
},
}; };
let id = MessageInputItem::File { let id = InputItem::LocalImage {
source: FileSource::Id { path: PathBuf::from("file_456"),
file_id: "file_456".into(),
},
}; };
let observed_url = to_val(&url);
let expected_url = json!({"type":"local_image","path":"https://example.com/a.pdf"});
assert_eq!( assert_eq!(
to_val(&url), observed_url, expected_url,
json!({"type":"file","file_url":"https://example.com/a.pdf"}) "LocalImage with URL path should serialize as image_url"
);
let observed_id = to_val(&id);
let expected_id = json!({"type":"local_image","path":"file_456"});
assert_eq!(
observed_id, expected_id,
"LocalImage with file id should serialize as image_url"
); );
assert_eq!(to_val(&id), json!({"type":"file","file_id":"file_456"}));
} }
#[test] #[test]
fn serialize_message_input_image_url_without_detail() { fn serialize_message_input_image_url_without_detail() {
let item = MessageInputItem::Image { let item = InputItem::Image {
source: ImageSource::ImageUrl { image_url: "https://example.com/x.png".into(),
image_url: "https://example.com/x.png".into(),
},
detail: None,
}; };
let observed = to_val(&item); let observed = to_val(&item);
let expected = json!({ let expected = json!({