mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
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.
This commit is contained in:
@@ -1,7 +1,5 @@
|
||||
use super::*;
|
||||
|
||||
const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId";
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct McpRequestProcessor {
|
||||
auth_manager: Arc<AuthManager>,
|
||||
@@ -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<serde_json::Value>,
|
||||
thread_id: &str,
|
||||
) -> Option<serde_json::Value> {
|
||||
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,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -364,6 +364,12 @@ impl ResolvedMcpCatalog {
|
||||
self.servers.get(name)
|
||||
}
|
||||
|
||||
pub(crate) fn servers(&self) -> impl Iterator<Item = (&str, &ResolvedMcpServer)> {
|
||||
self.servers
|
||||
.iter()
|
||||
.map(|(name, server)| (name.as_str(), server))
|
||||
}
|
||||
|
||||
pub fn configured_servers(&self) -> HashMap<String, McpServerConfig> {
|
||||
self.servers
|
||||
.iter()
|
||||
|
||||
@@ -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<AskForApproval>) {
|
||||
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<serde_json::Value>,
|
||||
meta: Option<serde_json::Value>,
|
||||
) -> Result<CallToolResult> {
|
||||
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<serde_json::Value>,
|
||||
meta: Option<serde_json::Value>,
|
||||
) -> Result<CallToolResult> {
|
||||
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<serde_json::Value>,
|
||||
meta: Option<serde_json::Value>,
|
||||
allow_sandbox_state_meta: bool,
|
||||
) -> Result<CallToolResult> {
|
||||
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
|
||||
|
||||
@@ -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
|
||||
|
||||
113
codex-rs/codex-mcp/src/egress.rs
Normal file
113
codex-rs/codex-mcp/src/egress.rs
Normal file
@@ -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<Value>,
|
||||
allow_sandbox_state_meta: bool,
|
||||
) -> Option<Value> {
|
||||
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_")
|
||||
}
|
||||
@@ -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;
|
||||
|
||||
@@ -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<String, EffectiveMcpServer> {
|
||||
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<String, McpServerConfig>,
|
||||
config: &McpConfig,
|
||||
auth: Option<&CodexAuth>,
|
||||
) -> HashMap<String, EffectiveMcpServer> {
|
||||
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<String, (McpServerConfig, McpEgressProfile)>,
|
||||
config: &McpConfig,
|
||||
auth: Option<&CodexAuth>,
|
||||
) -> HashMap<String, EffectiveMcpServer> {
|
||||
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::<HashMap<_, _>>();
|
||||
if !host_owned_codex_apps_enabled(config, auth) {
|
||||
|
||||
@@ -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<AppToolApproval>,
|
||||
pub tool_approval_modes: HashMap<String, AppToolApproval>,
|
||||
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(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -651,10 +651,17 @@ impl CodexThread {
|
||||
arguments: Option<serde_json::Value>,
|
||||
meta: Option<serde_json::Value>,
|
||||
) -> anyhow::Result<CallToolResult> {
|
||||
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
|
||||
}
|
||||
|
||||
|
||||
@@ -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<CallToolResult, String> {
|
||||
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<serde_json::Value>,
|
||||
thread_id: &str,
|
||||
) -> Option<serde_json::Value> {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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(()));
|
||||
|
||||
Reference in New Issue
Block a user