From c3dbb6392ef5c66ecfa598deace2f08f793b91cd Mon Sep 17 00:00:00 2001 From: nicholasclark-openai Date: Wed, 25 Mar 2026 12:44:40 -0700 Subject: [PATCH] Move MCP span assertion into unit test Keep the narrow MCP span PR out of the integration test suite by asserting the span fields in mcp_tool_call_tests instead. Co-authored-by: Codex --- codex-rs/core/src/mcp_tool_call_tests.rs | 55 ++++++++++++++++++++++++ codex-rs/core/tests/suite/rmcp_client.rs | 30 +------------ 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 9d7d1c3a41..edd698c134 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -18,6 +18,10 @@ use serde::Deserialize; use std::collections::HashMap; use std::sync::Arc; use tempfile::tempdir; +use tracing::Instrument; +use tracing::Level; +use tracing_subscriber::fmt::format::FmtSpan; +use tracing_test::internal::MockWriter; fn annotations( read_only: Option, @@ -119,6 +123,57 @@ fn approval_question_text_prepends_safety_reason() { ); } +#[tokio::test] +async fn mcp_tool_call_span_records_expected_fields() { + let buffer: &'static std::sync::Mutex> = + Box::leak(Box::new(std::sync::Mutex::new(Vec::new()))); + let subscriber = tracing_subscriber::fmt() + .with_level(true) + .with_ansi(false) + .with_max_level(Level::TRACE) + .with_span_events(FmtSpan::FULL) + .with_writer(MockWriter::new(buffer)) + .finish(); + let _guard = tracing::subscriber::set_default(subscriber); + + let (session, turn_context) = make_session_and_context().await; + + async {} + .instrument(mcp_tool_call_span( + &session, + &turn_context, + McpToolCallSpanFields { + server_name: "rmcp", + tool_name: "echo", + call_id: "call-123", + server_origin: Some("https://example.com:8443/mcp"), + connector_id: Some("calendar"), + connector_name: Some("Calendar"), + }, + )) + .await; + + let logs = String::from_utf8(buffer.lock().expect("buffer lock").clone()).expect("utf8 logs"); + assert!( + logs.contains("mcp.tools.call{otel.kind=\"client\"") + && logs.contains("rpc.system=\"jsonrpc\"") + && logs.contains("rpc.method=\"tools/call\"") + && logs.contains("mcp.server.name=\"rmcp\"") + && logs.contains("mcp.server.origin=\"https://example.com:8443/mcp\"") + && logs.contains("mcp.transport=\"streamable_http\"") + && logs.contains("mcp.connector.id=\"calendar\"") + && logs.contains("mcp.connector.name=\"Calendar\"") + && logs.contains("tool.name=\"echo\"") + && logs.contains("tool.call_id=\"call-123\"") + && logs.contains("server.address=\"example.com\"") + && logs.contains("server.port=8443") + && logs.contains("conversation.id=") + && logs.contains("session.id=") + && logs.contains("turn.id="), + "missing MCP tool span fields\nlogs:\n{logs}" + ); +} + #[tokio::test] async fn approval_elicitation_request_uses_message_override_and_preserves_tool_params_keys() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index ec99d9c01b..6cbf9521ba 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -48,28 +48,14 @@ use tokio::process::Child; use tokio::process::Command; use tokio::time::Instant; use tokio::time::sleep; -use tracing::Level; -use tracing_subscriber::fmt::format::FmtSpan; -use tracing_test::internal::MockWriter; static OPENAI_PNG: &str = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAD0AAAA9CAYAAAAeYmHpAAAE6klEQVR4Aeyau44UVxCGx1fZsmRLlm3Zoe0XcGQ5cUiCCIgJeS9CHgAhMkISQnIuGQgJEkBcxLW+nqnZ6uqqc+nuWRC7q/P3qetf9e+MtOwyX25O4Nep6JPyop++0qev9HrfgZ+F6r2DuB/vHOrt/UIkqdDHYvujOW6fO7h/CNEI+a5jc+pBR8uy0jVFsziYu5HtfSUk+Io34q921hLNctFSX0gwww+S8wce8K1LfCU+cYW4888aov8NxqvQILUPPReLOrm6zyLxa4i+6VZuFbJo8d1MOHZm+7VUtB/aIvhPWc/3SWg49JcwFLlHxuXKjtyloo+YNhuW3VS+WPBuUEMvCFKjEDVgFBQHXrnazpqiSxNZCkQ1kYiozsbm9Oz7l4i2Il7vGccGNWAc3XosDrZe/9P3ZnMmzHNEQw4smf8RQ87XEAMsC7Az0Au+dgXerfH4+sHvEc0SYGic8WBBUGqFH2gN7yDrazy7m2pbRTeRmU3+MjZmr1h6LJgPbGy23SI6GlYT0brQ71IY8Us4PNQCm+zepSbaD2BY9xCaAsD9IIj/IzFmKMSdHHonwdZATbTnYREf6/VZGER98N9yCWIvXQwXDoDdhZJoT8jwLnJXDB9w4Sb3e6nK5ndzlkTLnP3JBu4LKkbrYrU69gCVceV0JvpyuW1xlsUVngzhwMetn/XamtTORF9IO5YnWNiyeF9zCAfqR3fUW+vZZKLtgP+ts8BmQRBREAdRDhH3o8QuRh/YucNFz2BEjxbRN6LGzphfKmvP6v6QhqIQyZ8XNJ0W0X83MR1PEcJBNO2KC2Z1TW/v244scp9FwRViZxIOBF0Lctk7ZVSavdLvRlV1hz/ysUi9sr8CIcB3nvWBwA93ykTz18eAYxQ6N/K2DkPA1lv3iXCwmDUT7YkjIby9siXueIJj9H+pzSqJ9oIuJWTUgSSt4WO7o/9GGg0viR4VinNRUDoIj34xoCd6pxD3aK3zfdbnx5v1J3ZNNEJsE0sBG7N27ReDrJc4sFxz7dI/ZAbOmmiKvHBitQXpAdR6+F7v+/ol/tOouUV01EeMZQF2BoQDn6dP4XNr+j9GZEtEK1/L8pFw7bd3a53tsTa7WD+054jOFmPg1XBKPQgnqFfmFcy32ZRvjmiIIQTYFvyDxQ8nH8WIwwGwlyDjDznnilYyFr6njrlZwsKkBpO59A7OwgdzPEWRm+G+oeb7IfyNuzjEEVLrOVxJsxvxwF8kmCM6I2QYmJunz4u4TrADpfl7mlbRTWQ7VmrBzh3+C9f6Grc3YoGN9dg/SXFthpRsT6vobfXRs2VBlgBHXVMLHjDNbIZv1sZ9+X3hB09cXdH1JKViyG0+W9bWZDa/r2f9zAFR71sTzGpMSWz2iI4YssWjWo3REy1MDGjdwe5e0dFSiAC1JakBvu4/CUS8Eh6dqHdU0Or0ioY3W5ClSqDXAy7/6SRfgw8vt4I+tbvvNtFT2kVDhY5+IGb1rCqYaXNF08vSALsXCPmt0kQNqJT1p5eI1mkIV/BxCY1z85lOzeFbPBQHURkkPTlwTYK9gTVE25l84IbFFN+YJDHjdpn0gq6mrHht0dkcjbM4UL9283O5p77GN+SPW/QwVB4IUYg7Or+Kp7naR6qktP98LNF2UxWo9yObPIT9KYg+hK4i56no4rfnM0qeyFf6AwAAAP//trwR3wAAAAZJREFUAwBZ0sR75itw5gAAAABJRU5ErkJggg=="; -#[tokio::test(flavor = "current_thread")] +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] #[serial(mcp_test_value)] async fn stdio_server_round_trip() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); - let buffer: &'static std::sync::Mutex> = - Box::leak(Box::new(std::sync::Mutex::new(Vec::new()))); - let subscriber = tracing_subscriber::fmt() - .with_level(true) - .with_ansi(false) - .with_max_level(Level::TRACE) - .with_span_events(FmtSpan::FULL) - .with_writer(MockWriter::new(buffer)) - .finish(); - let _guard = tracing::subscriber::set_default(subscriber); - let server = responses::start_mock_server().await; let call_id = "call-123"; @@ -203,20 +189,6 @@ async fn stdio_server_round_trip() -> anyhow::Result<()> { wait_for_event(&fixture.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - let logs = String::from_utf8(buffer.lock().unwrap().clone()).unwrap(); - assert!( - logs.contains("turn{otel.name=\"session_task.turn\"") - && logs.contains("mcp.tools.call{otel.kind=\"client\"") - && logs.contains("rpc.system=\"jsonrpc\"") - && logs.contains("rpc.method=\"tools/call\"") - && logs.contains("mcp.server.name=\"rmcp\"") - && logs.contains("mcp.transport=\"stdio\"") - && logs.contains("tool.name=\"echo\"") - && logs.contains("tool.call_id=\"call-123\"") - && logs.contains("turn.id="), - "missing MCP tracing spans nested under session_task.turn\nlogs:\n{logs}" - ); - server.verify().await; Ok(())