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
44 changes: 18 additions & 26 deletions codex-rs/app-server/src/request_processors/mcp_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,17 +121,11 @@ impl McpRequestProcessor {
} = params;

let auth = self.auth_manager.auth().await;
let (config, mcp_config) = match thread_id.as_deref() {
let (mcp_config, runtime_context) = match thread_id.as_deref() {
Some(thread_id) => {
let (_, thread) = self.load_thread(thread_id).await?;
let thread_config = thread.config().await;
let config = self
.config_manager
.load_latest_config_for_thread(thread_config.as_ref())
.await
.map_err(|err| internal_error(format!("failed to reload config: {err}")))?;
let mcp_config = thread.runtime_mcp_config(&config).await;
(config, mcp_config)
let runtime = thread.current_mcp_runtime();
(runtime.config().clone(), runtime.runtime_context().clone())
}
None => {
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
Expand All @@ -140,7 +134,11 @@ impl McpRequestProcessor {
.mcp_manager()
.runtime_config(&config)
.await;
(config, mcp_config)
let runtime_context = McpRuntimeContext::new(
self.thread_manager.environment_manager(),
config.cwd.to_path_buf(),
);
(mcp_config, runtime_context)
}
};
let effective_servers = codex_mcp::effective_mcp_servers(&mcp_config, auth.as_ref());
Expand All @@ -167,10 +165,6 @@ impl McpRequestProcessor {
}
};

let runtime_context = McpRuntimeContext::new(
self.thread_manager.environment_manager(),
config.cwd.to_path_buf(),
);
let http_client = runtime_context
.resolve_http_client(&name, server)
.map_err(|err| {
Expand All @@ -189,16 +183,16 @@ impl McpRequestProcessor {
let handle = perform_oauth_login_return_url_with_http_client(
&name,
&url,
config.mcp_oauth_credentials_store_mode,
config.auth_keyring_backend_kind(),
mcp_config.mcp_oauth_credentials_store_mode,
mcp_config.auth_keyring_backend_kind,
http_headers,
env_http_headers,
&resolved_scopes.scopes,
server.oauth_client_id(),
server.oauth_resource.as_deref(),
timeout_secs,
config.mcp_oauth_callback_port,
config.mcp_oauth_callback_url.as_deref(),
mcp_config.mcp_oauth_callback_port,
mcp_config.mcp_oauth_callback_url.as_deref(),
http_client,
)
.await
Expand Down Expand Up @@ -250,18 +244,16 @@ impl McpRequestProcessor {
None => (self.load_latest_config(/*fallback_cwd*/ None).await?, None),
};
let mcp_manager = self.thread_manager.mcp_manager();
let codex_apps_tools_cache = mcp_manager.codex_apps_tools_cache();
let auth = self.auth_manager.auth().await;
let mcp_config = match thread {
Some(thread) => thread.runtime_mcp_config(&config).await,
None => mcp_manager.runtime_config(&config).await,
};
let codex_apps_tools_cache = mcp_manager.codex_apps_tools_cache();
let auth = self.auth_manager.auth().await;
let environment_manager = self.thread_manager.environment_manager();
// This status path has no turn-selected environment. Use config cwd
// as the local stdio fallback; named environment stdio MCPs must
// declare their own absolute cwd.
let runtime_context =
McpRuntimeContext::new(Arc::clone(&environment_manager), config.cwd.to_path_buf());
let runtime_context = McpRuntimeContext::new(
self.thread_manager.environment_manager(),
config.cwd.to_path_buf(),
);

tokio::spawn(async move {
Self::list_mcp_server_status_task(
Expand Down
24 changes: 24 additions & 0 deletions codex-rs/codex-mcp/src/catalog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,30 @@ impl ResolvedMcpCatalog {
.collect()
}

/// Replaces the resolved server set while preserving known server sources.
///
/// Names not present in the existing catalog are treated as config-owned.
pub fn with_materialized_servers(&self, servers: HashMap<String, McpServerConfig>) -> Self {
let mut builder = Self::builder();
for (name, config) in servers {
let source = self
.server(&name)
.map(|server| server.source.clone())
.unwrap_or(McpServerSource::Config);
let precedence = match &source {
McpServerSource::Plugin(_) => RegistrationPrecedence::Plugin(Reverse(0)),
McpServerSource::SelectedPlugin(_) => {
RegistrationPrecedence::SelectedPlugin(Reverse(0))
}
McpServerSource::Config => RegistrationPrecedence::Config,
McpServerSource::Compatibility { .. } => RegistrationPrecedence::Compatibility,
McpServerSource::Extension { .. } => RegistrationPrecedence::Extension(0),
};
builder.register(McpServerRegistration::new(name, source, config, precedence));
}
builder.build()
}

/// Returns package attribution for each winning plugin-owned server.
pub fn plugin_attributions_by_server_name(&self) -> HashMap<String, McpPluginAttribution> {
self.servers
Expand Down
11 changes: 11 additions & 0 deletions codex-rs/codex-mcp/src/catalog_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,17 @@ fn selected_plugins_override_discovered_plugins_but_not_config() {
}]
);

let refreshed = server("https://refreshed.example/mcp");
let catalog =
catalog.with_materialized_servers(HashMap::from([("docs".to_string(), refreshed.clone())]));
assert_eq!(
catalog.server("docs"),
Some(&super::ResolvedMcpServer {
source: selected_plugin_source("selected-alpha"),
config: refreshed,
})
);

let mut builder = catalog.to_builder();
let configured = server("https://config.example/mcp");
builder.register(McpServerRegistration::from_config(
Expand Down
62 changes: 44 additions & 18 deletions codex-rs/core/src/codex_delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use crate::guardian::spawn_approval_request_review;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION;
use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC;
use crate::mcp_tool_call::McpToolApprovalMetadata;
use crate::mcp_tool_call::build_guardian_mcp_tool_review_request;
use crate::mcp_tool_call::is_mcp_tool_approval_question_id;
use crate::mcp_tool_call::lookup_mcp_tool_metadata;
Expand All @@ -60,6 +61,12 @@ use codex_protocol::protocol::MultiAgentVersion;
#[cfg(test)]
use crate::session::completed_session_loop_termination;

#[derive(Clone)]
struct PendingMcpInvocation {
invocation: McpInvocation,
metadata: Option<McpToolApprovalMetadata>,
}

/// Start an interactive sub-Codex thread and return IO channels.
///
/// The returned `events_rx` yields non-approval events emitted by the sub-agent.
Expand Down Expand Up @@ -149,10 +156,10 @@ pub(crate) async fn run_codex_thread_interactive(
let parent_session_clone = Arc::clone(&parent_session);
let parent_ctx_clone = Arc::clone(&parent_ctx);
let codex_for_events = Arc::clone(&codex);
// Cache delegated MCP invocations so guardian can recover the full tool call
// context when the later legacy RequestUserInput approval event only carries
// a call_id plus approval question metadata.
let pending_mcp_invocations = Arc::new(Mutex::new(HashMap::<String, McpInvocation>::new()));
// Cache the child call's MCP metadata at begin time. The later legacy
// RequestUserInput approval event only carries a call_id and question metadata.
let pending_mcp_invocations =
Arc::new(Mutex::new(HashMap::<String, PendingMcpInvocation>::new()));
tokio::spawn(async move {
forward_events(
codex_for_events,
Expand Down Expand Up @@ -270,7 +277,7 @@ async fn forward_events(
tx_sub: Sender<Event>,
parent_session: Arc<Session>,
parent_ctx: Arc<TurnContext>,
pending_mcp_invocations: Arc<Mutex<HashMap<String, McpInvocation>>>,
pending_mcp_invocations: Arc<Mutex<HashMap<String, PendingMcpInvocation>>>,
cancel_token: CancellationToken,
) {
let cancelled = cancel_token.cancelled();
Expand Down Expand Up @@ -357,10 +364,34 @@ async fn forward_events(
id,
msg: EventMsg::McpToolCallBegin(event),
} => {
// Runtime refreshes are published before a request step is captured, so
// the child runtime at call begin is the one executing this invocation.
// Cache its metadata now; the later approval event has only a call ID.
let metadata = if let Some(turn_context) =
codex.session.turn_context_for_sub_id(&id).await
{
let mcp = codex.session.services.latest_mcp_runtime();
lookup_mcp_tool_metadata(
codex.session.as_ref(),
turn_context.as_ref(),
mcp.manager(),
&event.invocation.server,
&event.invocation.tool,
)
Comment thread
jif-oai marked this conversation as resolved.
.await
} else {
None
};
pending_mcp_invocations
.lock()
.await
.insert(event.call_id.clone(), event.invocation.clone());
.insert(
event.call_id.clone(),
PendingMcpInvocation {
invocation: event.invocation.clone(),
metadata,
},
);
if !forward_event_or_shutdown(
&codex,
&tx_sub,
Expand Down Expand Up @@ -648,7 +679,7 @@ async fn handle_request_user_input(
id: String,
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
pending_mcp_invocations: &Arc<Mutex<HashMap<String, McpInvocation>>>,
pending_mcp_invocations: &Arc<Mutex<HashMap<String, PendingMcpInvocation>>>,
event: RequestUserInputEvent,
cancel_token: &CancellationToken,
) {
Expand Down Expand Up @@ -686,12 +717,12 @@ async fn handle_request_user_input(
/// programmatically after running the guardian review.
///
/// The RequestUserInput event only carries `call_id` plus approval question
/// metadata, so this helper joins it back to the cached `McpToolCallBegin`
/// invocation in order to rebuild the full guardian review request.
/// metadata, so this helper joins it back to the child runtime metadata cached at
/// `McpToolCallBegin` in order to rebuild the full guardian review request.
async fn maybe_auto_review_mcp_request_user_input(
parent_session: &Arc<Session>,
parent_ctx: &Arc<TurnContext>,
pending_mcp_invocations: &Arc<Mutex<HashMap<String, McpInvocation>>>,
pending_mcp_invocations: &Arc<Mutex<HashMap<String, PendingMcpInvocation>>>,
event: &RequestUserInputEvent,
cancel_token: &CancellationToken,
) -> Option<RequestUserInputResponse> {
Expand All @@ -702,18 +733,13 @@ async fn maybe_auto_review_mcp_request_user_input(
.questions
.iter()
.find(|question| is_mcp_tool_approval_question_id(&question.id))?;
let invocation = pending_mcp_invocations
let pending = pending_mcp_invocations
.lock()
.await
.get(&event.call_id)
.cloned()?;
let metadata = lookup_mcp_tool_metadata(
parent_session.as_ref(),
parent_ctx.as_ref(),
&invocation.server,
&invocation.tool,
)
.await;
let invocation = pending.invocation;
let metadata = pending.metadata;
let approvals_reviewer =
mcp_approvals_reviewer(parent_ctx, &invocation.server, metadata.as_ref());
if !routes_approval_to_guardian_with_reviewer(parent_ctx, approvals_reviewer) {
Expand Down
22 changes: 14 additions & 8 deletions codex-rs/core/src/codex_delegate_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,13 @@ async fn delegated_mcp_guardian_abort_returns_synthetic_decline_answer() {

let pending_mcp_invocations = Arc::new(Mutex::new(HashMap::from([(
"call-1".to_string(),
McpInvocation {
server: "custom_server".to_string(),
tool: "dangerous_tool".to_string(),
arguments: None,
PendingMcpInvocation {
invocation: McpInvocation {
server: "custom_server".to_string(),
tool: "dangerous_tool".to_string(),
arguments: None,
},
metadata: None,
},
)])));
let cancel_token = CancellationToken::new();
Expand Down Expand Up @@ -460,10 +463,13 @@ async fn delegated_mcp_user_reviewer_returns_none_without_metadata() {
crate::session::tests::make_session_and_context_with_rx().await;
let pending_mcp_invocations = Arc::new(Mutex::new(HashMap::from([(
"call-1".to_string(),
McpInvocation {
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
tool: "dangerous_tool".to_string(),
arguments: None,
PendingMcpInvocation {
invocation: McpInvocation {
server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
tool: "dangerous_tool".to_string(),
arguments: None,
},
metadata: None,
},
)])));
let cancel_token = CancellationToken::new();
Expand Down
5 changes: 5 additions & 0 deletions codex-rs/core/src/codex_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,11 @@ impl CodexThread {
self.codex.session.runtime_mcp_config(config).await
}

/// Returns the exact MCP config, environment bindings, and manager most recently published.
pub fn current_mcp_runtime(&self) -> Arc<crate::session::McpRuntimeSnapshot> {
self.codex.session.services.latest_mcp_runtime()
}

pub fn multi_agent_version(&self) -> Option<MultiAgentVersion> {
self.codex.session.multi_agent_version()
}
Expand Down
11 changes: 2 additions & 9 deletions codex-rs/core/src/mcp_skill_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,15 +199,8 @@ pub(crate) async fn maybe_install_mcp_dependencies(
warn!("failed to refresh MCP dependencies for mentioned skills: {err}");
return;
}
let refresh_servers = sess.runtime_mcp_servers(&refresh_config).await;
sess.refresh_mcp_servers_now(
turn_context,
refresh_servers,
config.mcp_oauth_credentials_store_mode,
config.auth_keyring_backend_kind(),
elicitation_reviewer,
)
.await;
sess.refresh_mcp_servers_now(turn_context, &refresh_config, elicitation_reviewer)
.await;
}

async fn should_install_mcp_dependencies(
Expand Down
Loading
Loading