Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions codex-rs/core/src/mcp_tool_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,12 @@ pub(crate) async fn handle_mcp_tool_call(
.unwrap_or_else(|| JsonValue::Object(serde_json::Map::new())),
};
}
let request_meta =
build_mcp_tool_call_request_meta(turn_context.as_ref(), &server, metadata.as_ref());
let request_meta = build_mcp_tool_call_request_meta(
turn_context.as_ref(),
&server,
&call_id,
metadata.as_ref(),
);
let connector_id = metadata
.as_ref()
.and_then(|metadata| metadata.connector_id.clone());
Expand Down Expand Up @@ -693,6 +697,7 @@ fn custom_mcp_tool_approval_mode(
fn build_mcp_tool_call_request_meta(
turn_context: &TurnContext,
server: &str,
call_id: &str,
metadata: Option<&McpToolApprovalMetadata>,
) -> Option<serde_json::Value> {
let mut request_meta = serde_json::Map::new();
Expand All @@ -704,10 +709,14 @@ fn build_mcp_tool_call_request_meta(
);
}

if server == CODEX_APPS_MCP_SERVER_NAME
&& let Some(codex_apps_meta) =
metadata.and_then(|metadata| metadata.codex_apps_meta.clone())
{
if server == CODEX_APPS_MCP_SERVER_NAME {
let mut codex_apps_meta = metadata
.and_then(|metadata| metadata.codex_apps_meta.clone())
.unwrap_or_default();
codex_apps_meta.insert(
"call_id".to_string(),
serde_json::Value::String(call_id.to_string()),
);
request_meta.insert(
MCP_TOOL_CODEX_APPS_META_KEY.to_string(),
serde_json::Value::Object(codex_apps_meta),
Expand Down
39 changes: 36 additions & 3 deletions codex-rs/core/src/mcp_tool_call_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,9 +668,13 @@ async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() {
)
.expect("turn metadata json");

let meta =
build_mcp_tool_call_request_meta(&turn_context, "custom_server", /*metadata*/ None)
.expect("custom servers should receive turn metadata");
let meta = build_mcp_tool_call_request_meta(
&turn_context,
"custom_server",
"call-custom",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is a test, but isn't a more representative call_id something like "call_abc123xyz789"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks for the callout

/*metadata*/ None,
)
.expect("custom servers should receive turn metadata");

assert_eq!(
meta,
Expand Down Expand Up @@ -715,11 +719,13 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
build_mcp_tool_call_request_meta(
&turn_context,
CODEX_APPS_MCP_SERVER_NAME,
"call_abc123xyz789",
Some(&metadata),
),
Some(serde_json::json!({
crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata,
MCP_TOOL_CODEX_APPS_META_KEY: {
"call_id": "call_abc123xyz789",
"resource_uri": "connector://calendar/tools/calendar_create_event",
"contains_mcp_source": true,
"connector_id": "calendar",
Expand All @@ -728,6 +734,33 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
);
}

#[tokio::test]
async fn codex_apps_tool_call_request_meta_includes_call_id_without_existing_codex_apps_meta() {
let (_, turn_context) = make_session_and_context().await;
let expected_turn_metadata = serde_json::from_str::<serde_json::Value>(
&turn_context
.turn_metadata_state
.current_header_value()
.expect("turn metadata header"),
)
.expect("turn metadata json");

assert_eq!(
build_mcp_tool_call_request_meta(
&turn_context,
CODEX_APPS_MCP_SERVER_NAME,
"call_abc123xyz789",
/*metadata*/ None,
),
Some(serde_json::json!({
crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata,
MCP_TOOL_CODEX_APPS_META_KEY: {
"call_id": "call_abc123xyz789",
},
}))
);
}

#[test]
fn mcp_tool_call_thread_id_meta_is_added_to_request_meta() {
assert_eq!(
Expand Down
1 change: 1 addition & 0 deletions codex-rs/core/tests/suite/openai_file_mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ async fn codex_apps_file_params_upload_local_paths_before_mcp_tool_call() -> Res
assert_eq!(
apps_tool_call.pointer("/params/_meta/_codex_apps"),
Some(&json!({
"call_id": "extract-call-1",
"resource_uri": DOCUMENT_EXTRACT_TEXT_RESOURCE_URI,
"contains_mcp_source": true,
"connector_id": "calendar",
Expand Down
2 changes: 2 additions & 0 deletions codex-rs/core/tests/suite/search_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
.structured_content,
Some(json!({
"_codex_apps": {
"call_id": "calendar-call-1",
"resource_uri": CALENDAR_CREATE_EVENT_RESOURCE_URI,
"contains_mcp_source": true,
"connector_id": "calendar",
Expand Down Expand Up @@ -586,6 +587,7 @@ async fn tool_search_returns_deferred_tools_without_follow_up_tool_injection() -
assert_eq!(
apps_tool_call.pointer("/params/_meta/_codex_apps"),
Some(&json!({
"call_id": "calendar-call-1",
"resource_uri": CALENDAR_CREATE_EVENT_RESOURCE_URI,
"contains_mcp_source": true,
"connector_id": "calendar",
Expand Down
Loading