mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Accept explicit contribution context for MCP runtime configuration (#61283)
## What changed - Make `McpManager::runtime_config` accept `McpServerContributionContext` and honor its originator, selected environments, and disabled plugin IDs. - Add `McpServerContributionContext::for_host` to carry thread initialization data, originator, and session source without live extension state or discovery. - Pass explicit global contexts from threadless discovery and control paths. ## Testing Extend the thread manager test to cover host-only contribution identity, originator headers, and server restrictions for disabled plugins and pending environments. GitOrigin-RevId: 04ea2c8951187d9014867f0020223b6a7dd94d8d
This commit is contained in:
@@ -27,6 +27,7 @@ use crate::session::INITIAL_SUBMIT_ID;
|
||||
use codex_config::types::ApprovalsReviewer;
|
||||
use codex_config::types::ToolSuggestDiscoverableType;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_extension_api::McpServerContributionContext;
|
||||
use codex_features::Feature;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
@@ -213,7 +214,9 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
|
||||
});
|
||||
}
|
||||
let cache_key = accessible_connectors_cache_key(config, auth.as_ref());
|
||||
let mcp_config = mcp_manager.runtime_config(config).await;
|
||||
let mcp_config = mcp_manager
|
||||
.runtime_config(McpServerContributionContext::global(config))
|
||||
.await;
|
||||
let tool_plugin_context = tool_plugin_context(&mcp_config);
|
||||
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
|
||||
{
|
||||
|
||||
+17
-11
@@ -199,16 +199,18 @@ impl McpManager {
|
||||
}
|
||||
|
||||
/// Returns the MCP config after applying compatibility built-ins and
|
||||
/// runtime-only extension overlays.
|
||||
pub async fn runtime_config(&self, config: &Config) -> McpConfig {
|
||||
/// runtime-only extension overlays and restrictions from the supplied contribution context.
|
||||
pub async fn runtime_config(
|
||||
&self,
|
||||
context: McpServerContributionContext<'_, Config>,
|
||||
) -> McpConfig {
|
||||
self.runtime_config_with_context(
|
||||
McpServerContributionContext::global(config),
|
||||
// Threadless discovery and control-plane paths have no effective thread
|
||||
// originator; active-thread tool calls use runtime_config_for_step below.
|
||||
/*originator*/
|
||||
None,
|
||||
McpEnvironmentScope::HostOnly,
|
||||
&[],
|
||||
context,
|
||||
context.originator(),
|
||||
context
|
||||
.selected_environments()
|
||||
.map_or(McpEnvironmentScope::HostOnly, McpEnvironmentScope::Selected),
|
||||
context.disabled_plugin_ids(),
|
||||
)
|
||||
.await
|
||||
.config
|
||||
@@ -565,7 +567,9 @@ impl McpManager {
|
||||
|
||||
/// Returns configured and host-contributed servers before auth gating.
|
||||
pub async fn runtime_servers(&self, config: &Config) -> HashMap<String, McpServerConfig> {
|
||||
let mcp_config = self.runtime_config(config).await;
|
||||
let mcp_config = self
|
||||
.runtime_config(McpServerContributionContext::global(config))
|
||||
.await;
|
||||
configured_mcp_servers(&mcp_config)
|
||||
}
|
||||
|
||||
@@ -575,7 +579,9 @@ impl McpManager {
|
||||
config: &Config,
|
||||
auth: Option<&CodexAuth>,
|
||||
) -> HashMap<codex_mcp::McpServerKey, EffectiveMcpServer> {
|
||||
let mcp_config = self.runtime_config(config).await;
|
||||
let mcp_config = self
|
||||
.runtime_config(McpServerContributionContext::global(config))
|
||||
.await;
|
||||
effective_mcp_servers(&mcp_config, auth)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2131,7 +2131,7 @@ async fn spawn_internal_session_preserves_parent_lineage_without_forking_history
|
||||
async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() {
|
||||
struct InitialDataRecorder {
|
||||
lifecycle_observed: Arc<std::sync::Mutex<Vec<(String, String)>>>,
|
||||
mcp_observed: Arc<std::sync::Mutex<Vec<(String, SessionSource)>>>,
|
||||
mcp_observed: Arc<std::sync::Mutex<Vec<(String, SessionSource, bool)>>>,
|
||||
mcp_loaded: Arc<std::sync::atomic::AtomicUsize>,
|
||||
}
|
||||
|
||||
@@ -2177,11 +2177,13 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
) -> codex_extension_api::ExtensionFuture<'a, Vec<codex_extension_api::SelectedPlugin<'a>>>
|
||||
{
|
||||
Box::pin(async move {
|
||||
assert_eq!(
|
||||
context.selected_environments(),
|
||||
Some([].as_slice()),
|
||||
"thread MCP projection must preserve explicitly empty selections"
|
||||
);
|
||||
if context.thread_store().is_some() {
|
||||
assert_eq!(
|
||||
context.selected_environments(),
|
||||
Some([].as_slice()),
|
||||
"thread MCP projection must preserve explicitly empty selections"
|
||||
);
|
||||
}
|
||||
let thread_init = context
|
||||
.thread_init()
|
||||
.expect("initial MCP resolution should be thread-scoped");
|
||||
@@ -2198,6 +2200,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
.session_source()
|
||||
.expect("thread-scoped MCP resolution should identify its source")
|
||||
.clone(),
|
||||
context.thread_store().is_some(),
|
||||
));
|
||||
let mut server = codex_mcp::codex_apps_mcp_server_config(
|
||||
"https://selected.invalid",
|
||||
@@ -2208,7 +2211,8 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
&selected_root.location;
|
||||
let source_environment_id = environment_id.clone();
|
||||
server.environment_id = source_environment_id.clone();
|
||||
server.enabled = false;
|
||||
// Keep thread startup offline; host-only checks below never open connections.
|
||||
server.enabled = context.thread_store().is_none();
|
||||
let selected_root_id = selected_root.id;
|
||||
let plugin_id = format!("plugin-{selected_root_id}");
|
||||
let mcp_loaded = Arc::clone(&self.mcp_loaded);
|
||||
@@ -2315,26 +2319,16 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
.expect("start second thread");
|
||||
let first_session = &first_thread.thread.session;
|
||||
let first_originator = first_session.originator().await;
|
||||
let host_context = codex_extension_api::McpServerContributionContext::for_host(
|
||||
&config,
|
||||
Some(&first_session.services.mcp_thread_init),
|
||||
Some(&first_originator),
|
||||
Some(&SessionSource::Exec),
|
||||
);
|
||||
let first_resolved = first_session
|
||||
.services
|
||||
.mcp_manager
|
||||
.runtime_config_for_step(
|
||||
&config,
|
||||
&first_session.services.mcp_thread_init,
|
||||
&first_session.services.thread_extension_data,
|
||||
McpThreadIdentity {
|
||||
auth_changed: false,
|
||||
session_source: &SessionSource::Exec,
|
||||
originator: &first_originator,
|
||||
disabled_plugin_ids: &[],
|
||||
environments: McpEnvironmentScope::Selected(
|
||||
&first_session.services.turn_environments.selections(),
|
||||
),
|
||||
},
|
||||
/*ready_selected_capability_roots*/ &[],
|
||||
/*executor_capability_discovery*/ None,
|
||||
/*executor_capability_discovery_v2*/ None,
|
||||
)
|
||||
.runtime_config(host_context)
|
||||
.await;
|
||||
let second_session = &second_thread.thread.session;
|
||||
let second_originator = second_session.originator().await;
|
||||
@@ -2377,10 +2371,14 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner),
|
||||
vec![
|
||||
("selected-a".to_string(), SessionSource::Exec),
|
||||
("selected-b".to_string(), second_session_source.clone()),
|
||||
("selected-a".to_string(), SessionSource::Exec),
|
||||
("selected-b".to_string(), second_session_source),
|
||||
("selected-a".to_string(), SessionSource::Exec, true),
|
||||
(
|
||||
"selected-b".to_string(),
|
||||
second_session_source.clone(),
|
||||
true
|
||||
),
|
||||
("selected-a".to_string(), SessionSource::Exec, false),
|
||||
("selected-b".to_string(), second_session_source, true),
|
||||
]
|
||||
);
|
||||
let selected_servers = |config: &codex_mcp::McpConfig| {
|
||||
@@ -2391,7 +2389,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
.collect::<std::collections::BTreeMap<_, _>>()
|
||||
};
|
||||
assert_eq!(
|
||||
selected_servers(&first_resolved.config),
|
||||
selected_servers(&first_resolved),
|
||||
std::collections::BTreeMap::from([("selected-a".to_string(), "env-a".to_string())])
|
||||
);
|
||||
assert_eq!(
|
||||
@@ -2399,7 +2397,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
std::collections::BTreeMap::from([("selected-b".to_string(), "env-b".to_string())])
|
||||
);
|
||||
for (config, name, source_environment_id) in [
|
||||
(&first_resolved.config, "selected-a", "env-a"),
|
||||
(&first_resolved, "selected-a", "env-a"),
|
||||
(&second_resolved.config, "selected-b", "env-b"),
|
||||
] {
|
||||
let key = codex_mcp::McpServerKey::in_environment(source_environment_id, name);
|
||||
@@ -2418,7 +2416,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
));
|
||||
assert_eq!(Some(server), expected.build().server(&key));
|
||||
}
|
||||
let codex_apps_server = codex_mcp::configured_mcp_servers(&first_resolved.config)
|
||||
let codex_apps_server = codex_mcp::configured_mcp_servers(&first_resolved)
|
||||
.remove(codex_mcp::CODEX_APPS_MCP_SERVER_NAME)
|
||||
.expect("Codex Apps server should be configured");
|
||||
let codex_apps_headers = match codex_apps_server.transport {
|
||||
@@ -2433,6 +2431,38 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
|
||||
.get("originator"),
|
||||
Some(&"codex_work_desktop".to_string())
|
||||
);
|
||||
// A supplied plugin denial or unavailable environment must also restrict host projections.
|
||||
let disabled_plugins = ["plugin-selected-a".to_string()];
|
||||
let pending_environments = [codex_protocol::protocol::TurnEnvironmentSelection {
|
||||
environment_id: "env-a".to_string(),
|
||||
cwd: PathUri::parse("file:///plugins").expect("environment cwd"),
|
||||
workspace_roots: vec![],
|
||||
config: EnvironmentConfigState::Pending,
|
||||
selected_capability_roots: Default::default(),
|
||||
}];
|
||||
for (context, enabled) in [
|
||||
(host_context, true),
|
||||
(
|
||||
host_context.with_disabled_plugin_ids(&disabled_plugins),
|
||||
false,
|
||||
),
|
||||
(
|
||||
host_context.with_selected_environments(&pending_environments),
|
||||
false,
|
||||
),
|
||||
] {
|
||||
let resolved = first_session
|
||||
.services
|
||||
.mcp_manager
|
||||
.runtime_config(context)
|
||||
.await;
|
||||
assert_eq!(
|
||||
codex_mcp::configured_mcp_servers(&resolved)
|
||||
.get("selected-a")
|
||||
.is_some_and(|server| server.enabled),
|
||||
enabled,
|
||||
);
|
||||
}
|
||||
for (policy, disabled_plugin_ids, selected_enabled, expected_mcp_loads) in [
|
||||
("", vec!["plugin-selected-a".to_string()], false, 1),
|
||||
("", vec![], true, 1),
|
||||
|
||||
Reference in New Issue
Block a user