From 2ae82ebb57d093848c6115cef77d85c4883f142f Mon Sep 17 00:00:00 2001 From: Max Stoiber Date: Mon, 6 Jul 2026 17:04:34 -0700 Subject: [PATCH] Stop exposing Codex context to direct MCP servers Direct user-configured and plugin MCP servers sit outside the managed connector trust boundary, but Codex currently attaches its raw turn, thread, workspace, plugin, and trace metadata to their tool calls. That leaks host implementation context to arbitrary servers and makes direct egress broader than its intended MCP contract. Introduce a source-derived direct_mcp_v1 profile and enforce it again at the final connection-manager boundary. Direct calls now omit host-generated context and strip reserved OpenAI/Codex metadata supplied through app-server calls while retaining ordinary MCP metadata and sandbox state only when the server negotiated that capability. Compatibility and extension registrations retain host-owned behavior. Hoopa still depends on threadId for its legacy per-thread routing and is registered as an ordinary config server, so temporarily reserve the exact logical name hoopa as host-owned. This is intentional migration debt rather than a trust signal; it can be removed once Hoopa moves to a trusted registration or no longer routes through request metadata. Move authoritative hosted thread metadata behind the profile boundary and remove tests and fixtures that asserted direct servers receive the now-private context. --- .../src/request_processors/mcp_processor.rs | 29 +---- .../app-server/tests/suite/v2/mcp_tool.rs | 13 +- codex-rs/codex-mcp/src/catalog.rs | 6 + codex-rs/codex-mcp/src/connection_manager.rs | 45 +++++++ .../codex-mcp/src/connection_manager_tests.rs | 1 + codex-rs/codex-mcp/src/egress.rs | 113 ++++++++++++++++++ codex-rs/codex-mcp/src/lib.rs | 1 + codex-rs/codex-mcp/src/mcp/mod.rs | 42 ++++++- codex-rs/codex-mcp/src/server.rs | 17 +++ codex-rs/core/src/codex_thread.rs | 15 ++- codex-rs/core/src/mcp_tool_call.rs | 24 +++- codex-rs/core/src/mcp_tool_call_tests.rs | 96 +-------------- codex-rs/core/src/session/mod.rs | 4 +- codex-rs/core/tests/suite/token_budget.rs | 90 -------------- 14 files changed, 256 insertions(+), 240 deletions(-) create mode 100644 codex-rs/codex-mcp/src/egress.rs diff --git a/codex-rs/app-server/src/request_processors/mcp_processor.rs b/codex-rs/app-server/src/request_processors/mcp_processor.rs index ac2125c2cfcd..5e26962699c9 100644 --- a/codex-rs/app-server/src/request_processors/mcp_processor.rs +++ b/codex-rs/app-server/src/request_processors/mcp_processor.rs @@ -1,7 +1,5 @@ use super::*; -const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId"; - #[derive(Clone)] pub(crate) struct McpRequestProcessor { auth_manager: Arc, @@ -460,12 +458,11 @@ impl McpRequestProcessor { let outgoing = Arc::clone(&self.outgoing); let thread_id = params.thread_id.clone(); let (_, thread) = self.load_thread(&thread_id).await?; - let meta = with_mcp_tool_call_thread_id_meta(params.meta, &thread_id); let request_id = request_id.clone(); tokio::spawn(async move { let result = thread - .call_mcp_tool(¶ms.server, ¶ms.tool, params.arguments, meta) + .call_mcp_tool(¶ms.server, ¶ms.tool, params.arguments, params.meta) .await .map(McpServerToolCallResponse::from) .map_err(|error| internal_error(format!("{error:#}"))); @@ -474,27 +471,3 @@ impl McpRequestProcessor { Ok(()) } } - -fn with_mcp_tool_call_thread_id_meta( - meta: Option, - thread_id: &str, -) -> Option { - match meta { - Some(serde_json::Value::Object(mut map)) => { - map.insert( - MCP_TOOL_THREAD_ID_META_KEY.to_string(), - serde_json::Value::String(thread_id.to_string()), - ); - Some(serde_json::Value::Object(map)) - } - None => { - let mut map = serde_json::Map::new(); - map.insert( - MCP_TOOL_THREAD_ID_META_KEY.to_string(), - serde_json::Value::String(thread_id.to_string()), - ); - Some(serde_json::Value::Object(map)) - } - other => other, - } -} diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 1f3289e43932..9314310ae44e 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -122,11 +122,9 @@ url = "{mcp_server_url}/mcp" ) .await??; let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; - let thread_id = thread.id.clone(); - let tool_call_request_id = mcp .send_mcp_server_tool_call_request(McpServerToolCallParams { - thread_id: thread_id.clone(), + thread_id: thread.id, server: TEST_SERVER_NAME.to_string(), tool: TEST_TOOL_NAME.to_string(), arguments: Some(json!({ @@ -154,7 +152,6 @@ url = "{mcp_server_url}/mcp" response.structured_content, Some(json!({ "echoed": "hello from app", - "threadId": thread_id, })) ); assert_eq!(response.is_error, Some(false)); @@ -736,13 +733,6 @@ impl ServerHandler for ToolAppsMcpServer { .and_then(|arguments| arguments.get("message")) .and_then(|value| value.as_str()) .unwrap_or_default(); - let thread_id = context - .meta - .0 - .get("threadId") - .and_then(|value| value.as_str()) - .unwrap_or_default(); - let mut meta = Meta::new(); meta.0.insert("calledBy".to_string(), json!("mcp-app")); @@ -810,7 +800,6 @@ impl ServerHandler for ToolAppsMcpServer { let mut result = CallToolResult::structured(json!({ "echoed": message, - "threadId": thread_id, })); result.content = vec![Content::text(format!("echo: {message}"))]; result.meta = Some(meta); diff --git a/codex-rs/codex-mcp/src/catalog.rs b/codex-rs/codex-mcp/src/catalog.rs index d60ea843df87..bd50b4da186a 100644 --- a/codex-rs/codex-mcp/src/catalog.rs +++ b/codex-rs/codex-mcp/src/catalog.rs @@ -364,6 +364,12 @@ impl ResolvedMcpCatalog { self.servers.get(name) } + pub(crate) fn servers(&self) -> impl Iterator { + self.servers + .iter() + .map(|(name, server)| (name.as_str(), server)) + } + pub fn configured_servers(&self) -> HashMap { self.servers .iter() diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 2143e3b57765..792a3b8080aa 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -17,6 +17,8 @@ use crate::McpAuthStatusEntry; use crate::codex_apps_cache::CodexAppsToolsCache; use crate::codex_apps_cache::CodexAppsToolsCacheKey; use crate::codex_apps_cache::CodexAppsToolsFetchSource; +use crate::egress::McpEgressProfile; +use crate::egress::sanitize_tool_call_meta; use crate::elicitation::ElicitationRequestManager; use crate::elicitation::ElicitationRequestRouter; use crate::elicitation::ElicitationReviewerHandle; @@ -436,6 +438,12 @@ impl McpConnectionManager { server_name == CODEX_APPS_MCP_SERVER_NAME && self.server_metadata.contains_key(server_name) } + pub fn server_uses_direct_mcp_v1(&self, server_name: &str) -> bool { + self.server_metadata + .get(server_name) + .is_some_and(|metadata| metadata.egress_profile == McpEgressProfile::DirectMcpV1) + } + pub fn set_approval_policy(&self, approval_policy: &Constrained) { if let Ok(mut policy) = self.elicitation_requests.approval_policy.lock() { *policy = approval_policy.value(); @@ -741,6 +749,33 @@ impl McpConnectionManager { tool: &str, arguments: Option, meta: Option, + ) -> Result { + self.call_tool_with_meta_policy( + server, tool, arguments, meta, /*allow_sandbox_state_meta*/ true, + ) + .await + } + + pub async fn call_tool_from_app( + &self, + server: &str, + tool: &str, + arguments: Option, + meta: Option, + ) -> Result { + self.call_tool_with_meta_policy( + server, tool, arguments, meta, /*allow_sandbox_state_meta*/ false, + ) + .await + } + + async fn call_tool_with_meta_policy( + &self, + server: &str, + tool: &str, + arguments: Option, + meta: Option, + allow_sandbox_state_meta: bool, ) -> Result { let client = self.client_by_name(server).await?; if !client.tool_filter.allows(tool) { @@ -748,6 +783,16 @@ impl McpConnectionManager { "tool '{tool}' is disabled for MCP server '{server}'" )); } + let egress_profile = self + .server_metadata + .get(server) + .map(|metadata| metadata.egress_profile) + .ok_or_else(|| anyhow!("unknown MCP server '{server}'"))?; + let meta = sanitize_tool_call_meta( + egress_profile, + meta, + allow_sandbox_state_meta && client.server_supports_sandbox_state_meta_capability, + ); let result: rmcp::model::CallToolResult = client .client diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 41fc19a594f5..5330b56356cd 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -1399,6 +1399,7 @@ async fn list_all_tools_adds_server_metadata_to_tools() { supports_parallel_tool_calls: true, default_tools_approval_mode: None, tool_approval_modes: HashMap::new(), + egress_profile: McpEgressProfile::DirectMcpV1, }, ); manager diff --git a/codex-rs/codex-mcp/src/egress.rs b/codex-rs/codex-mcp/src/egress.rs new file mode 100644 index 000000000000..5f01f4cdd4d0 --- /dev/null +++ b/codex-rs/codex-mcp/src/egress.rs @@ -0,0 +1,113 @@ +use crate::catalog::McpServerSource; +use crate::rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY; +use serde_json::Value; + +const LEGACY_HOOPA_MCP_SERVER_NAME: &str = "hoopa"; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum McpEgressProfile { + DirectMcpV1, + HostOwned, +} + +impl McpEgressProfile { + pub(crate) fn for_configured_name(server_name: &str) -> Self { + // TODO: Remove this exception after Hoopa uses a host-owned registration or stops using + // MCP request metadata for thread routing. + if server_name == LEGACY_HOOPA_MCP_SERVER_NAME { + Self::HostOwned + } else { + Self::DirectMcpV1 + } + } + + pub(crate) fn for_registration(server_name: &str, source: &McpServerSource) -> Self { + if Self::for_configured_name(server_name) == Self::HostOwned { + return Self::HostOwned; + } + + match source { + McpServerSource::Config + | McpServerSource::Plugin(_) + | McpServerSource::SelectedPlugin(_) => Self::DirectMcpV1, + McpServerSource::Compatibility { .. } | McpServerSource::Extension { .. } => { + Self::HostOwned + } + } + } +} + +pub(crate) fn sanitize_tool_call_meta( + profile: McpEgressProfile, + meta: Option, + allow_sandbox_state_meta: bool, +) -> Option { + if profile == McpEgressProfile::HostOwned { + return meta; + } + + let Some(Value::Object(mut meta)) = meta else { + return meta; + }; + meta.retain(|key, _| { + (allow_sandbox_state_meta && key == MCP_SANDBOX_STATE_META_CAPABILITY) + || !is_reserved_direct_mcp_meta_key(key) + }); + (!meta.is_empty()).then_some(Value::Object(meta)) +} + +fn is_reserved_direct_mcp_meta_key(key: &str) -> bool { + matches!( + key, + "installation_id" + | "installationId" + | "session_id" + | "sessionId" + | "thread_id" + | "threadId" + | "conversation_id" + | "conversationId" + | "turn_id" + | "turnId" + | "workspace_id" + | "workspaceId" + | "window_id" + | "windowId" + | "request_kind" + | "requestKind" + | "compaction" + | "turn_started_at_unix_ms" + | "turnStartedAtUnixMs" + | "forked_from_thread_id" + | "forkedFromThreadId" + | "parent_thread_id" + | "parentThreadId" + | "subagent_kind" + | "subagentKind" + | "thread_source" + | "threadSource" + | "sandbox" + | "workspaces" + | "plugin_id" + | "pluginId" + | "connector_id" + | "connector_name" + | "connector_display_name" + | "connector_description" + | "connectorDescription" + | "connected_account_email" + | "connectedAccountEmail" + | "link_id" + | "linkId" + | "template_id" + | "templateId" + | "resource_uri" + | "resourceUri" + ) || key.starts_with("openai/") + || key.starts_with("x-openai-") + || key.starts_with("_openai_") + || key.starts_with("codex/") + || key.starts_with("x-codex-") + || key.starts_with("_codex_") + || key.starts_with("codex_") +} diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 77128d79701a..b8f2ea461609 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -80,6 +80,7 @@ mod catalog; pub(crate) mod codex_apps; pub(crate) mod codex_apps_cache; pub(crate) mod connection_manager; +mod egress; pub(crate) mod elicitation; pub(crate) mod mcp; mod plugin_config; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 87ea25dd9f16..9830865c6615 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -47,6 +47,7 @@ use crate::ResolvedMcpCatalog; use crate::codex_apps_cache::CodexAppsToolsCache; use crate::codex_apps_cache::codex_apps_tools_cache_key; use crate::connection_manager::McpConnectionManager; +use crate::egress::McpEgressProfile; use crate::runtime::McpRuntimeContext; use crate::server::EffectiveMcpServer; @@ -250,7 +251,20 @@ pub fn effective_mcp_servers( config: &McpConfig, auth: Option<&CodexAuth>, ) -> HashMap { - effective_mcp_servers_from_configured(configured_mcp_servers(config), config, auth) + let configured_servers = config + .mcp_server_catalog + .servers() + .map(|(name, server)| { + ( + name.to_string(), + ( + server.config().clone(), + McpEgressProfile::for_registration(name, server.source()), + ), + ) + }) + .collect(); + effective_mcp_servers_with_profiles(configured_servers, config, auth) } /// Converts a materialized server map to its auth-gated runtime view. @@ -261,13 +275,32 @@ pub fn effective_mcp_servers_from_configured( configured_servers: HashMap, config: &McpConfig, auth: Option<&CodexAuth>, +) -> HashMap { + let configured_servers = configured_servers + .into_iter() + .map(|(name, server)| { + let profile = config + .mcp_server_catalog + .server(&name) + .map(|resolved| McpEgressProfile::for_registration(&name, resolved.source())) + .unwrap_or_else(|| McpEgressProfile::for_configured_name(&name)); + (name, (server, profile)) + }) + .collect(); + effective_mcp_servers_with_profiles(configured_servers, config, auth) +} + +fn effective_mcp_servers_with_profiles( + configured_servers: HashMap, + config: &McpConfig, + auth: Option<&CodexAuth>, ) -> HashMap { let chatgpt_origin = url::Url::parse(CHATGPT_CODEX_BASE_URL) .ok() .map(|url| url.origin()); let mut servers = configured_servers .into_iter() - .map(|(name, mut server)| { + .map(|(name, (mut server, egress_profile))| { match server.auth.clone() { McpServerAuth::ChatGpt => { let server_origin = match &server.transport { @@ -285,7 +318,10 @@ pub fn effective_mcp_servers_from_configured( } McpServerAuth::OAuth => {} } - (name, EffectiveMcpServer::configured(server)) + ( + name, + EffectiveMcpServer::configured_with_egress_profile(server, egress_profile), + ) }) .collect::>(); if !host_owned_codex_apps_enabled(config, auth) { diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index 39287b21361d..b08da25427ae 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -4,6 +4,8 @@ use codex_config::AppToolApproval; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use crate::egress::McpEgressProfile; + /// The runtime launch strategy for an effective MCP server. #[derive(Debug, Clone)] pub(crate) enum McpServerLaunch { @@ -14,12 +16,21 @@ pub(crate) enum McpServerLaunch { #[derive(Debug, Clone)] pub struct EffectiveMcpServer { launch: McpServerLaunch, + egress_profile: McpEgressProfile, } impl EffectiveMcpServer { pub fn configured(config: McpServerConfig) -> Self { + Self::configured_with_egress_profile(config, McpEgressProfile::DirectMcpV1) + } + + pub(crate) fn configured_with_egress_profile( + config: McpServerConfig, + egress_profile: McpEgressProfile, + ) -> Self { Self { launch: McpServerLaunch::Configured(Box::new(config)), + egress_profile, } } @@ -33,6 +44,10 @@ impl EffectiveMcpServer { } } + pub(crate) fn egress_profile(&self) -> McpEgressProfile { + self.egress_profile + } + pub fn enabled(&self) -> bool { match &self.launch { McpServerLaunch::Configured(config) => config.enabled, @@ -81,6 +96,7 @@ pub(crate) struct McpServerMetadata { pub supports_parallel_tool_calls: bool, pub default_tools_approval_mode: Option, pub tool_approval_modes: HashMap, + pub egress_profile: McpEgressProfile, } impl McpServerMetadata { @@ -111,6 +127,7 @@ impl From<&EffectiveMcpServer> for McpServerMetadata { .map(|approval_mode| (name.clone(), approval_mode)) }) .collect(), + egress_profile: server.egress_profile(), }, } } diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index c80268e2f9c7..de3a3ed3e664 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -651,10 +651,17 @@ impl CodexThread { arguments: Option, meta: Option, ) -> anyhow::Result { - self.current_mcp_runtime() - .await - .manager_arc() - .call_tool(server, tool, arguments, meta) + let manager = self.current_mcp_runtime().await.manager_arc(); + let meta = if manager.server_uses_direct_mcp_v1(server) { + meta + } else { + crate::mcp_tool_call::with_mcp_tool_call_thread_id_meta( + meta, + &self.session_configured.thread_id.to_string(), + ) + }; + manager + .call_tool_from_app(server, tool, arguments, meta) .await } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index d03f232d12b3..8af9fbfe172b 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -394,8 +394,11 @@ async fn handle_approved_mcp_tool_call( let result = async { let result = async { let rewritten_arguments = rewrite?; - let request_meta = - build_mcp_tool_call_request_meta(turn_context, &server, call_id, metadata); + let request_meta = if manager.server_uses_direct_mcp_v1(&server) { + None + } else { + build_host_mcp_tool_call_request_meta(turn_context, &server, call_id, metadata) + }; execute_mcp_tool_call( sess, step_context, @@ -574,7 +577,12 @@ async fn execute_mcp_tool_call( ) -> Result { let turn_context = step_context.turn.as_ref(); let manager = step_context.mcp.manager(); - let request_meta = with_mcp_tool_call_thread_id_meta(request_meta, &sess.thread_id.to_string()); + let include_host_metadata = !manager.server_uses_direct_mcp_v1(&invocation.server); + let request_meta = if include_host_metadata { + with_mcp_tool_call_thread_id_meta(request_meta, &sess.thread_id.to_string()) + } else { + request_meta + }; let request_meta = augment_mcp_tool_request_meta_with_sandbox_state( step_context, manager, @@ -587,7 +595,11 @@ async fn execute_mcp_tool_call( .services .rollout_thread_trace .start_mcp_call_trace(call_id); - let request_meta = mcp_call_trace.add_request_meta(request_meta); + let request_meta = if include_host_metadata { + mcp_call_trace.add_request_meta(request_meta) + } else { + request_meta + }; let result = manager .call_tool( &invocation.server, @@ -1080,7 +1092,7 @@ async fn custom_mcp_tool_approval_mode( .unwrap_or_default() } -fn build_mcp_tool_call_request_meta( +fn build_host_mcp_tool_call_request_meta( turn_context: &TurnContext, server: &str, call_id: &str, @@ -1124,7 +1136,7 @@ fn build_mcp_tool_call_request_meta( (!request_meta.is_empty()).then_some(serde_json::Value::Object(request_meta)) } -fn with_mcp_tool_call_thread_id_meta( +pub(crate) fn with_mcp_tool_call_thread_id_meta( meta: Option, thread_id: &str, ) -> Option { diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index f68d32ff4e69..7b2cef7066a2 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1065,75 +1065,6 @@ fn truncate_mcp_tool_result_for_event_bounds_large_error() { assert!(got.contains("truncated")); } -#[tokio::test] -async fn mcp_tool_call_request_meta_includes_turn_metadata_for_custom_server() { - let (_, turn_context) = make_session_and_context().await; - let expected_turn_metadata = turn_context - .turn_metadata_state - .current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context)) - .expect("turn metadata"); - - let meta = build_mcp_tool_call_request_meta( - &turn_context, - "custom_server", - "call-custom", - /*metadata*/ None, - ) - .expect("custom servers should receive turn metadata"); - let turn_metadata = meta - .get(crate::X_CODEX_TURN_METADATA_HEADER) - .expect("turn metadata should be present"); - - assert_eq!( - turn_metadata - .get("model") - .and_then(serde_json::Value::as_str), - Some(turn_context.model_info.slug.as_str()) - ); - assert_eq!( - turn_metadata - .get("reasoning_effort") - .and_then(serde_json::Value::as_str), - turn_context - .effective_reasoning_effort() - .map(|effort| effort.to_string()) - .as_deref() - ); - - assert_eq!( - meta, - serde_json::json!({ - crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata, - }) - ); -} - -#[tokio::test] -async fn mcp_tool_call_request_meta_includes_turn_started_at_unix_ms() { - let (_, turn_context) = make_session_and_context().await; - turn_context - .turn_metadata_state - .set_turn_started_at_unix_ms(/*turn_started_at_unix_ms*/ 1_700_000_000_123); - - let meta = build_mcp_tool_call_request_meta( - &turn_context, - "custom_server", - "call-custom", - /*metadata*/ None, - ) - .expect("custom servers should receive turn metadata"); - let turn_metadata = meta - .get(crate::X_CODEX_TURN_METADATA_HEADER) - .expect("turn metadata should be present"); - - assert_eq!( - turn_metadata - .get("turn_started_at_unix_ms") - .and_then(serde_json::Value::as_i64), - Some(1_700_000_000_123) - ); -} - #[tokio::test] async fn mcp_sandbox_cwd_uses_matching_server_environment_uri() -> anyhow::Result<()> { let (_, mut turn_context) = make_session_and_context().await; @@ -1169,29 +1100,6 @@ async fn mcp_sandbox_cwd_is_none_for_unselected_server_environment() -> anyhow:: Ok(()) } -#[tokio::test] -async fn plugin_mcp_tool_call_request_meta_includes_plugin_id() { - let (_, turn_context) = make_session_and_context().await; - let expected_turn_metadata = turn_context - .turn_metadata_state - .current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context)) - .expect("turn metadata"); - let mut metadata = approval_metadata( - /*connector_id*/ None, /*connector_name*/ None, - /*connector_description*/ None, /*tool_title*/ None, - /*tool_description*/ None, - ); - metadata.plugin_id = Some("sample@test".to_string()); - - assert_eq!( - build_mcp_tool_call_request_meta(&turn_context, "sample", "call-plugin", Some(&metadata),), - Some(serde_json::json!({ - crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata, - MCP_TOOL_PLUGIN_ID_META_KEY: "sample@test", - })) - ); -} - #[test] fn mcp_tool_call_item_metadata_only_trusts_codex_apps_identity() { let mut metadata = approval_metadata( @@ -1321,7 +1229,7 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps }; assert_eq!( - build_mcp_tool_call_request_meta( + build_host_mcp_tool_call_request_meta( &turn_context, CODEX_APPS_MCP_SERVER_NAME, "call_abc123xyz789", @@ -1348,7 +1256,7 @@ async fn codex_apps_tool_call_request_meta_includes_call_id_without_existing_cod .expect("turn metadata"); assert_eq!( - build_mcp_tool_call_request_meta( + build_host_mcp_tool_call_request_meta( &turn_context, CODEX_APPS_MCP_SERVER_NAME, "call_abc123xyz789", diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index fa240b890ba8..ee3c15282aaf 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -3364,9 +3364,7 @@ impl Session { "notes", "thread_hint", /*arguments*/ None, - Some(serde_json::json!({ - "threadId": self.thread_id().to_string(), - })), + /*meta*/ None, ) .await .ok() diff --git a/codex-rs/core/tests/suite/token_budget.rs b/codex-rs/core/tests/suite/token_budget.rs index e536d6b888b3..89ca25a53d20 100644 --- a/codex-rs/core/tests/suite/token_budget.rs +++ b/codex-rs/core/tests/suite/token_budget.rs @@ -1,6 +1,4 @@ use anyhow::Result; -use codex_config::types::McpServerConfig; -use codex_config::types::McpServerTransportConfig; use codex_core::config::TokenBudgetConfig; use codex_features::Feature; use codex_model_provider_info::built_in_model_providers; @@ -32,18 +30,14 @@ use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; -use core_test_support::stdio_server_bin; use core_test_support::test_codex::local; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; -use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; -use std::collections::HashMap; use std::path::Path; -use std::time::Duration; const CONFIGURED_CONTEXT_WINDOW: i64 = 128_000; @@ -259,90 +253,6 @@ async fn token_budget_guidance_follows_context_window() -> Result<()> { Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn token_budget_context_injects_plain_thread_hint_text() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let rmcp_test_server_bin = stdio_server_bin()?; - let test = test_codex() - .with_config(move |config| { - config.model_context_window = Some(CONFIGURED_CONTEXT_WINDOW); - config - .features - .enable(Feature::TokenBudget) - .expect("test config should allow token budget"); - let mut servers = config.mcp_servers.get().clone(); - servers.insert( - "notes".to_string(), - McpServerConfig { - auth: Default::default(), - transport: McpServerTransportConfig::Stdio { - command: rmcp_test_server_bin, - args: Vec::new(), - env: None, - env_vars: Vec::new(), - cwd: None, - }, - environment_id: "local".to_string(), - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: Some(Duration::from_secs(10)), - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth: None, - oauth_resource: None, - tools: HashMap::new(), - }, - ); - config - .mcp_servers - .set(servers) - .expect("test mcp servers should accept any configuration"); - }) - .build(&server) - .await?; - wait_for_mcp_server(&test.codex, "notes").await?; - let responses = mount_sse_sequence( - &server, - vec![sse(vec![ - ev_response_created("resp-1"), - ev_completed("resp-1"), - ])], - ) - .await; - - test.submit_turn("inject the history hint").await?; - - let request = responses.single_request(); - let thread_id = test.session_configured.thread_id; - let token_budgets = token_budget_contexts(&request); - assert_eq!(token_budgets.len(), 1); - let captures = assert_regex_match( - &format!( - r"^{CONTEXT_WINDOW_OPEN_TAG}\nThread id: {thread_id}\nFirst context window id: ([0-9a-f-]{{36}})\nCurrent context window id: ([0-9a-f-]{{36}})\nmanual history hint for thread {thread_id}\nunstructured notes/thread_hint fixture result\n{CONTEXT_WINDOW_CLOSE_TAG}$" - ), - &token_budgets[0], - ); - assert_eq!( - captures.get(1).expect("first window id capture").as_str(), - captures.get(2).expect("current window id capture").as_str() - ); - assert!( - !tool_names(&request) - .iter() - .any(|name| name == "mcp__notes__thread_hint"), - "thread_hint should be hidden from model tool exposure" - ); - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn token_budget_reminder_emits_after_crossing_compaction_threshold() -> Result<()> { skip_if_no_network!(Ok(()));