Honor thread-level plugin exclusions across runtime capabilities (#44655)

## What changed

- Apply `disabled_plugin_ids` to plugin skills, recommendations, hooks, and MCP servers without modifying shared plugin state.
- Activate selection changes when the next task starts, keeping pending settings from changing the active runtime.
- Hide app tools contributed only by disabled plugins from model tool catalogs and search. Preserve connectors shared with enabled plugins and existing direct Apps RPC behavior.
- Include plugin identity in MCP approval keys and use that identity when persisting plugin tool approvals.

## Testing

Add regression coverage for disabling and re-enabling plugin skills and tools, deferred activation of settings, shared connector ownership, selected plugin servers, executor hooks, and plugin-specific approval keys and persistence.

GitOrigin-RevId: 1153ff6aa7ab6953d50e838da05203904ecd2956
This commit is contained in:
Matthew Zeng authored and copyberry committed 2026-09-10 21:51:34 +00:00
1 parent 5c94936b56
commit 8570091e14
27 files changed
+565 -171

No files matched your search

+12 -12
View File
@@ -36,7 +36,7 @@ use crate::elicitation::ElicitationRequestManager;
use crate::elicitation::ElicitationRequestRouter; use crate::elicitation::ElicitationRequestRouter;
use crate::event_stream::EventStreamConnectionSettings; use crate::event_stream::EventStreamConnectionSettings;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
use crate::mcp::ToolPluginProvenance; use crate::mcp::ToolPluginContext;
use crate::pagination::MAX_CODEX_APPS_TOOL_CATALOG_ITEMS; use crate::pagination::MAX_CODEX_APPS_TOOL_CATALOG_ITEMS;
use crate::pagination::MAX_MCP_CATALOG_ITEMS; use crate::pagination::MAX_MCP_CATALOG_ITEMS;
use crate::rmcp_client::AsyncManagedClient; use crate::rmcp_client::AsyncManagedClient;
@@ -170,14 +170,14 @@ struct McpServerView {
impl McpServerView { impl McpServerView {
async fn listed_tools( async fn listed_tools(
&self, &self,
tool_plugin_provenance: &ToolPluginProvenance, tool_plugin_context: &ToolPluginContext,
) -> Result<Vec<ToolInfo>, StartupOutcomeError> { ) -> Result<Vec<ToolInfo>, StartupOutcomeError> {
let tools = self.connection.client.listed_tools().await?; let tools = self.connection.client.listed_tools().await?;
let tools = filter_tools(tools, &self.tool_filter); let tools = filter_tools(tools, &self.tool_filter);
Ok(if self.connection.client.is_codex_apps_mcp_server { Ok(if self.connection.client.is_codex_apps_mcp_server {
prepare_codex_apps_tools_for_model(tools, tool_plugin_provenance) prepare_codex_apps_tools_for_model(tools, tool_plugin_context)
} else { } else {
prepare_regular_mcp_tools_for_model(tools, tool_plugin_provenance) prepare_regular_mcp_tools_for_model(tools, tool_plugin_context)
}) })
} }
} }
@@ -189,7 +189,7 @@ pub(crate) struct McpConnectionSet {
disabled_servers: Vec<String>, disabled_servers: Vec<String>,
required_servers: Vec<String>, required_servers: Vec<String>,
optional_startup_deadline: OnceLock<tokio::time::Instant>, optional_startup_deadline: OnceLock<tokio::time::Instant>,
tool_plugin_provenance: Arc<ToolPluginProvenance>, tool_plugin_context: Arc<ToolPluginContext>,
prefix_mcp_tool_names: bool, prefix_mcp_tool_names: bool,
non_prefixed_mcp_tool_servers: Vec<String>, non_prefixed_mcp_tool_servers: Vec<String>,
elicitation_requests: ElicitationRequestManager, elicitation_requests: ElicitationRequestManager,
@@ -234,7 +234,7 @@ impl McpConnectionSet {
let default_protocol_mode = config.protocol_mode; let default_protocol_mode = config.protocol_mode;
let host_owned_apps_protocol_mode = config.host_owned_apps_protocol_mode; let host_owned_apps_protocol_mode = config.host_owned_apps_protocol_mode;
let client_elicitation_capability = config.client_elicitation_capability.clone(); let client_elicitation_capability = config.client_elicitation_capability.clone();
let tool_plugin_provenance = crate::mcp::tool_plugin_provenance(&config); let tool_plugin_context = crate::mcp::tool_plugin_context(&config);
let auth = auth.as_ref(); let auth = auth.as_ref();
let mut servers = HashMap::new(); let mut servers = HashMap::new();
let mut event_stream_connection = None; let mut event_stream_connection = None;
@@ -272,7 +272,7 @@ impl McpConnectionSet {
elicitation_router, elicitation_router,
) )
}; };
let tool_plugin_provenance = Arc::new(tool_plugin_provenance); let tool_plugin_context = Arc::new(tool_plugin_context);
let startup_submit_id = submit_id; let startup_submit_id = submit_id;
let static_chatgpt_auth_provider = auth let static_chatgpt_auth_provider = auth
.filter(|auth| auth.uses_codex_backend()) .filter(|auth| auth.uses_codex_backend())
@@ -601,7 +601,7 @@ impl McpConnectionSet {
catalog_item_limit, catalog_item_limit,
); );
let defer_startup = allow_deferred_startup let defer_startup = allow_deferred_startup
&& !tool_plugin_provenance.is_selected_plugin_mcp_server(&server_name) && !tool_plugin_context.is_selected_plugin_mcp_server(&server_name)
&& async_managed_client && async_managed_client
.tool_catalog_cache_context .tool_catalog_cache_context
.as_ref() .as_ref()
@@ -755,7 +755,7 @@ impl McpConnectionSet {
disabled_servers, disabled_servers,
required_servers, required_servers,
optional_startup_deadline: OnceLock::new(), optional_startup_deadline: OnceLock::new(),
tool_plugin_provenance, tool_plugin_context,
prefix_mcp_tool_names, prefix_mcp_tool_names,
non_prefixed_mcp_tool_servers, non_prefixed_mcp_tool_servers,
elicitation_requests: elicitation_requests.clone(), elicitation_requests: elicitation_requests.clone(),
@@ -813,7 +813,7 @@ impl McpConnectionSet {
disabled_servers: Vec::new(), disabled_servers: Vec::new(),
required_servers: Vec::new(), required_servers: Vec::new(),
optional_startup_deadline: OnceLock::new(), optional_startup_deadline: OnceLock::new(),
tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), tool_plugin_context: Arc::new(ToolPluginContext::default()),
prefix_mcp_tool_names, prefix_mcp_tool_names,
non_prefixed_mcp_tool_servers: Vec::new(), non_prefixed_mcp_tool_servers: Vec::new(),
elicitation_requests: ElicitationRequestManager::default(), elicitation_requests: ElicitationRequestManager::default(),
@@ -915,12 +915,12 @@ impl McpConnectionSet {
} }
pub fn plugin_id_for_mcp_server_name(&self, server_name: &str) -> Option<&str> { pub fn plugin_id_for_mcp_server_name(&self, server_name: &str) -> Option<&str> {
self.tool_plugin_provenance self.tool_plugin_context
.plugin_id_for_mcp_server_name(server_name) .plugin_id_for_mcp_server_name(server_name)
} }
pub fn is_selected_plugin_mcp_server(&self, server_name: &str) -> bool { pub fn is_selected_plugin_mcp_server(&self, server_name: &str) -> bool {
self.tool_plugin_provenance self.tool_plugin_context
.is_selected_plugin_mcp_server(server_name) .is_selected_plugin_mcp_server(server_name)
} }
@@ -149,7 +149,7 @@ impl McpConnectionSet {
.startup_complete .startup_complete
.load(Ordering::Acquire); .load(Ordering::Acquire);
let server_tools = view let server_tools = view
.listed_tools(&self.tool_plugin_provenance) .listed_tools(&self.tool_plugin_context)
.instrument(trace_span!( .instrument(trace_span!(
"list_tools_for_server", "list_tools_for_server",
server_name = %server_name, server_name = %server_name,
@@ -304,11 +304,11 @@ impl McpConnectionSet {
}; };
let server_tools = filter_tools(server_tools, &view.tool_filter); let server_tools = filter_tools(server_tools, &view.tool_filter);
let server_tools = if server_name == CODEX_APPS_MCP_SERVER_NAME { let server_tools = if server_name == CODEX_APPS_MCP_SERVER_NAME {
prepare_codex_apps_tools_for_model(server_tools, &self.tool_plugin_provenance) prepare_codex_apps_tools_for_model(server_tools, &self.tool_plugin_context)
} else { } else {
crate::rmcp_client::prepare_regular_mcp_tools_for_model( crate::rmcp_client::prepare_regular_mcp_tools_for_model(
server_tools, server_tools,
&self.tool_plugin_provenance, &self.tool_plugin_context,
) )
}; };
let server_tools = server_tools let server_tools = server_tools
@@ -427,6 +427,9 @@ impl McpConnectionSet {
.filter(|tool| { .filter(|tool| {
server_has_permission server_has_permission
&& view.tool_filter.allows(&tool.tool.name) && view.tool_filter.allows(&tool.tool.name)
&& self
.tool_plugin_context
.allows_connector_id(tool.connector_id.as_deref())
&& tool_is_model_visible(tool) && tool_is_model_visible(tool)
}) })
.map(|tool| tool.tool.name.to_string()) .map(|tool| tool.tool.name.to_string())
@@ -452,7 +455,7 @@ impl McpConnectionSet {
let (_, tools) = self.refresh_codex_apps_tool_catalog().await?; let (_, tools) = self.refresh_codex_apps_tool_catalog().await?;
let tools = prepare_codex_apps_tools_for_model( let tools = prepare_codex_apps_tools_for_model(
filter_tools(tools, &view.tool_filter), filter_tools(tools, &view.tool_filter),
&self.tool_plugin_provenance, &self.tool_plugin_context,
) )
.into_iter() .into_iter()
.map(|tool| Self::with_server_metadata(tool, &view.metadata)); .map(|tool| Self::with_server_metadata(tool, &view.metadata));
@@ -108,7 +108,7 @@ impl McpConnectionSet {
disabled_servers: Vec::new(), disabled_servers: Vec::new(),
required_servers: Vec::new(), required_servers: Vec::new(),
optional_startup_deadline: OnceLock::new(), optional_startup_deadline: OnceLock::new(),
tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), tool_plugin_context: Arc::new(ToolPluginContext::default()),
prefix_mcp_tool_names, prefix_mcp_tool_names,
non_prefixed_mcp_tool_servers: Vec::new(), non_prefixed_mcp_tool_servers: Vec::new(),
elicitation_requests: ElicitationRequestManager::new( elicitation_requests: ElicitationRequestManager::new(
@@ -501,7 +501,7 @@ async fn prepared_call_timeout_includes_trusted_access_lookup() {
config config
.server_permission_profiles .server_permission_profiles
.insert("docs".to_string(), PermissionProfile::default()); .insert("docs".to_string(), PermissionProfile::default());
manager.tool_plugin_provenance = Arc::new(crate::tool_plugin_provenance(&config)); manager.tool_plugin_context = Arc::new(crate::tool_plugin_context(&config));
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
manager.trusted_access = Some(TrustedAccessContext::new( manager.trusted_access = Some(TrustedAccessContext::new(
auth.clone(), auth.clone(),
@@ -2816,7 +2816,7 @@ async fn capture_binding_skips_pending_optional_servers_after_configured_shared_
)); ));
plugin_config.mcp_server_catalog = catalog.build(); plugin_config.mcp_server_catalog = catalog.build();
plugin_config.optional_mcp_startup_grace = Duration::from_millis(250); plugin_config.optional_mcp_startup_grace = Duration::from_millis(250);
manager.tool_plugin_provenance = Arc::new(crate::tool_plugin_provenance(&plugin_config)); manager.tool_plugin_context = Arc::new(crate::tool_plugin_context(&plugin_config));
for server_name in ["pending-one", "pending-two", "pending-selected"] { for server_name in ["pending-one", "pending-two", "pending-selected"] {
manager.insert_test_client( manager.insert_test_client(
server_name.to_string(), server_name.to_string(),
@@ -2840,7 +2840,7 @@ async fn capture_binding_skips_pending_optional_servers_after_configured_shared_
&permission_profile, &permission_profile,
/*prefix_mcp_tool_names*/ true, /*prefix_mcp_tool_names*/ true,
); );
required_manager.tool_plugin_provenance = Arc::clone(&manager.tool_plugin_provenance); required_manager.tool_plugin_context = Arc::clone(&manager.tool_plugin_context);
required_manager.insert_test_client( required_manager.insert_test_client(
"pending-selected", "pending-selected",
manager.test_client("pending-selected").clone(), manager.test_client("pending-selected").clone(),
+2 -2
View File
@@ -48,7 +48,7 @@ pub use catalog::ResolvedMcpServer;
pub use mcp::CODEX_APPS_MCP_SERVER_NAME; pub use mcp::CODEX_APPS_MCP_SERVER_NAME;
pub use mcp::DEFAULT_OPTIONAL_MCP_STARTUP_GRACE; pub use mcp::DEFAULT_OPTIONAL_MCP_STARTUP_GRACE;
pub use mcp::McpConfig; pub use mcp::McpConfig;
pub use mcp::ToolPluginProvenance; pub use mcp::ToolPluginContext;
pub use server::EffectiveMcpServer; pub use server::EffectiveMcpServer;
pub use auth_elicitation::CodexAppsAuthElicitation; pub use auth_elicitation::CodexAppsAuthElicitation;
@@ -68,7 +68,7 @@ pub use mcp::effective_mcp_servers;
pub use mcp::effective_mcp_servers_from_configured; pub use mcp::effective_mcp_servers_from_configured;
pub use mcp::host_owned_codex_apps_enabled; pub use mcp::host_owned_codex_apps_enabled;
pub use mcp::hosted_plugin_runtime_mcp_server_config; pub use mcp::hosted_plugin_runtime_mcp_server_config;
pub use mcp::tool_plugin_provenance; pub use mcp::tool_plugin_context;
pub use plugin_config::PluginMcpConfigParseOutcome; pub use plugin_config::PluginMcpConfigParseOutcome;
pub use plugin_config::PluginMcpServerParseError; pub use plugin_config::PluginMcpServerParseError;
pub use plugin_config::parse_agent_plugin_mcp_config; pub use plugin_config::parse_agent_plugin_mcp_config;
+26 -19
View File
@@ -240,15 +240,21 @@ impl McpConfig {
} }
} }
/// Plugin attribution and selection data derived from the current MCP configuration.
#[derive(Debug, Clone, Default, PartialEq, Eq)] #[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct ToolPluginProvenance { pub struct ToolPluginContext {
plugin_display_names_by_connector_id: HashMap<String, Vec<String>>, plugin_display_names_by_connector_id: HashMap<String, Vec<String>>,
disabled_connector_ids: HashSet<String>,
plugin_display_names_by_mcp_server_name: HashMap<String, Vec<String>>, plugin_display_names_by_mcp_server_name: HashMap<String, Vec<String>>,
plugin_ids_by_mcp_server_name: HashMap<String, String>, plugin_ids_by_mcp_server_name: HashMap<String, String>,
selected_plugin_mcp_server_names: HashSet<String>, selected_plugin_mcp_server_names: HashSet<String>,
} }
impl ToolPluginProvenance { impl ToolPluginContext {
pub(crate) fn allows_connector_id(&self, connector_id: Option<&str>) -> bool {
connector_id.is_none_or(|id| !self.disabled_connector_ids.contains(id))
}
pub fn plugin_display_names_for_connector_id(&self, connector_id: &str) -> &[String] { pub fn plugin_display_names_for_connector_id(&self, connector_id: &str) -> &[String] {
self.plugin_display_names_by_connector_id self.plugin_display_names_by_connector_id
.get(connector_id) .get(connector_id)
@@ -274,9 +280,12 @@ impl ToolPluginProvenance {
} }
fn from_config(config: &McpConfig) -> Self { fn from_config(config: &McpConfig) -> Self {
let mut tool_plugin_provenance = Self::default(); let mut tool_plugin_context = Self {
disabled_connector_ids: config.connector_snapshot.disabled_connector_ids().clone(),
..Self::default()
};
for connector_id in config.connector_snapshot.connector_ids() { for connector_id in config.connector_snapshot.connector_ids() {
tool_plugin_provenance tool_plugin_context
.plugin_display_names_by_connector_id .plugin_display_names_by_connector_id
.insert( .insert(
connector_id.0.clone(), connector_id.0.clone(),
@@ -291,30 +300,28 @@ impl ToolPluginProvenance {
.mcp_server_catalog .mcp_server_catalog
.plugin_attributions_by_server_name() .plugin_attributions_by_server_name()
{ {
tool_plugin_provenance tool_plugin_context
.plugin_display_names_by_mcp_server_name .plugin_display_names_by_mcp_server_name
.insert( .insert(
server_name.clone(), server_name.clone(),
vec![attribution.display_name().to_string()], vec![attribution.display_name().to_string()],
); );
tool_plugin_provenance tool_plugin_context
.plugin_ids_by_mcp_server_name .plugin_ids_by_mcp_server_name
.insert(server_name, attribution.plugin_id().to_string()); .insert(server_name, attribution.plugin_id().to_string());
} }
tool_plugin_provenance tool_plugin_context.selected_plugin_mcp_server_names.extend(
.selected_plugin_mcp_server_names config
.extend( .mcp_server_catalog
config .selected_plugin_server_names()
.mcp_server_catalog .map(str::to_string),
.selected_plugin_server_names() );
.map(str::to_string),
);
for plugin_names in tool_plugin_provenance for plugin_names in tool_plugin_context
.plugin_display_names_by_connector_id .plugin_display_names_by_connector_id
.values_mut() .values_mut()
.chain( .chain(
tool_plugin_provenance tool_plugin_context
.plugin_display_names_by_mcp_server_name .plugin_display_names_by_mcp_server_name
.values_mut(), .values_mut(),
) )
@@ -322,7 +329,7 @@ impl ToolPluginProvenance {
plugin_names.sort_unstable(); plugin_names.sort_unstable();
plugin_names.dedup(); plugin_names.dedup();
} }
tool_plugin_provenance tool_plugin_context
} }
} }
@@ -409,8 +416,8 @@ pub fn effective_mcp_servers_from_configured(
servers servers
} }
pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance { pub fn tool_plugin_context(config: &McpConfig) -> ToolPluginContext {
ToolPluginProvenance::from_config(config) ToolPluginContext::from_config(config)
} }
pub async fn read_mcp_resource( pub async fn read_mcp_resource(
+7 -5
View File
@@ -253,7 +253,7 @@ fn mcp_prompt_auto_approval_rejects_auto_mode_in_default_permission_mode() {
} }
#[test] #[test]
fn tool_plugin_provenance_collects_app_and_mcp_sources() { fn tool_plugin_context_collects_app_and_mcp_sources() {
let mut config = test_mcp_config(PathBuf::new()); let mut config = test_mcp_config(PathBuf::new());
let mut catalog = ResolvedMcpCatalog::builder(); let mut catalog = ResolvedMcpCatalog::builder();
catalog.register(McpServerRegistration::from_plugin( catalog.register(McpServerRegistration::from_plugin(
@@ -289,11 +289,12 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() {
..PluginCapabilitySummary::default() ..PluginCapabilitySummary::default()
}, },
]); ]);
let provenance = tool_plugin_provenance(&config); let provenance = tool_plugin_context(&config);
assert_eq!( assert_eq!(
provenance, provenance,
ToolPluginProvenance { ToolPluginContext {
disabled_connector_ids: HashSet::new(),
plugin_display_names_by_connector_id: HashMap::from([ plugin_display_names_by_connector_id: HashMap::from([
( (
"connector_example".to_string(), "connector_example".to_string(),
@@ -351,11 +352,12 @@ fn selected_mcp_attribution_does_not_join_an_unrelated_local_summary() {
}, },
]); ]);
let provenance = tool_plugin_provenance(&config); let provenance = tool_plugin_context(&config);
assert_eq!( assert_eq!(
provenance, provenance,
ToolPluginProvenance { ToolPluginContext {
disabled_connector_ids: HashSet::new(),
plugin_display_names_by_connector_id: HashMap::new(), plugin_display_names_by_connector_id: HashMap::new(),
plugin_display_names_by_mcp_server_name: HashMap::from([( plugin_display_names_by_mcp_server_name: HashMap::from([(
"github".to_string(), "github".to_string(),
+11 -10
View File
@@ -29,7 +29,7 @@ use crate::codex_apps::prepare_openai_file_params_for_model;
use crate::elicitation::ElicitationRequestManager; use crate::elicitation::ElicitationRequestManager;
use crate::executor_environment_http_client::ExecutorEnvironmentHttpClient; use crate::executor_environment_http_client::ExecutorEnvironmentHttpClient;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
use crate::mcp::ToolPluginProvenance; use crate::mcp::ToolPluginContext;
use crate::openai_docs_source_attribution::maybe_with_openai_docs_source_attribution; use crate::openai_docs_source_attribution::maybe_with_openai_docs_source_attribution;
use crate::pagination::collect_paginated_with_limit; use crate::pagination::collect_paginated_with_limit;
use crate::runtime::McpRuntimeContext; use crate::runtime::McpRuntimeContext;
@@ -688,20 +688,21 @@ pub(crate) async fn list_tools_for_client_uncached(
Ok(tools) Ok(tools)
} }
/// Presents declared Codex Apps file parameters to the model as local-path inputs and adds plugin /// Filters disabled connectors, presents declared Codex Apps file parameters to the model as
/// names to each tool. Plugin membership is resolved by connector ID, falling back to the MCP /// local-path inputs, and adds plugin names to each tool. Plugin membership is resolved by
/// server when absent. /// connector ID, falling back to the MCP server when absent.
pub(crate) fn prepare_codex_apps_tools_for_model( pub(crate) fn prepare_codex_apps_tools_for_model(
mut tools: Vec<ToolInfo>, mut tools: Vec<ToolInfo>,
tool_plugin_provenance: &ToolPluginProvenance, tool_plugin_context: &ToolPluginContext,
) -> Vec<ToolInfo> { ) -> Vec<ToolInfo> {
tools.retain(|tool| tool_plugin_context.allows_connector_id(tool.connector_id.as_deref()));
for tool in &mut tools { for tool in &mut tools {
prepare_openai_file_params_for_model(tool); prepare_openai_file_params_for_model(tool);
let plugin_names = match tool.connector_id.as_deref() { let plugin_names = match tool.connector_id.as_deref() {
Some(connector_id) => { Some(connector_id) => {
tool_plugin_provenance.plugin_display_names_for_connector_id(connector_id) tool_plugin_context.plugin_display_names_for_connector_id(connector_id)
} }
None => tool_plugin_provenance None => tool_plugin_context
.plugin_display_names_for_mcp_server_name(tool.server_name.as_str()), .plugin_display_names_for_mcp_server_name(tool.server_name.as_str()),
}; };
add_plugin_provenance_to_tool(tool, plugin_names); add_plugin_provenance_to_tool(tool, plugin_names);
@@ -747,11 +748,11 @@ fn add_plugin_provenance_to_tool(tool: &mut ToolInfo, plugin_names: &[String]) {
/// Adds server-scoped plugin names to regular MCP tools without changing their input schemas. /// Adds server-scoped plugin names to regular MCP tools without changing their input schemas.
pub(crate) fn prepare_regular_mcp_tools_for_model( pub(crate) fn prepare_regular_mcp_tools_for_model(
mut tools: Vec<ToolInfo>, mut tools: Vec<ToolInfo>,
tool_plugin_provenance: &ToolPluginProvenance, tool_plugin_context: &ToolPluginContext,
) -> Vec<ToolInfo> { ) -> Vec<ToolInfo> {
for tool in &mut tools { for tool in &mut tools {
let plugin_names = tool_plugin_provenance let plugin_names =
.plugin_display_names_for_mcp_server_name(tool.server_name.as_str()); tool_plugin_context.plugin_display_names_for_mcp_server_name(tool.server_name.as_str());
add_plugin_provenance_to_tool(tool, plugin_names); add_plugin_provenance_to_tool(tool, plugin_names);
} }
tools tools
+33 -23
View File
@@ -59,28 +59,43 @@ impl PluginConnectorSource {
} }
} }
/// Immutable connector declarations and their plugin provenance. impl From<&PluginCapabilitySummary> for PluginConnectorSource {
fn from(summary: &PluginCapabilitySummary) -> Self {
Self::from_connector_ids(
summary.config_name.clone(),
summary.display_name.clone(),
summary.app_connector_ids.clone(),
)
}
}
/// Immutable connector selection and provenance after applying disabled plugins.
#[derive(Clone, Debug, Default, PartialEq, Eq)] #[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct ConnectorSnapshot { pub struct ConnectorSnapshot {
sources: Vec<PluginConnectorSource>,
connector_ids: Vec<AppConnectorId>, connector_ids: Vec<AppConnectorId>,
plugin_display_names_by_connector_id: HashMap<String, Vec<String>>, plugin_display_names_by_connector_id: HashMap<String, Vec<String>>,
disabled_connector_ids: HashSet<String>,
} }
impl ConnectorSnapshot { impl ConnectorSnapshot {
/// Builds a connector snapshot from package-scoped declarations. /// Builds the final selection from all plugin sources, preserving contribution order.
pub fn from_plugin_sources(sources: impl IntoIterator<Item = PluginConnectorSource>) -> Self { /// Pass host and selected sources together so any enabled shared owner keeps a connector.
let sources = sources pub fn from_plugin_sources(
.into_iter() sources: impl IntoIterator<Item = PluginConnectorSource>,
.filter(|source| !source.connector_ids().is_empty()) disabled_plugin_ids: &[String],
.collect::<Vec<_>>(); ) -> Self {
let mut connector_ids = Vec::new(); let mut connector_ids = Vec::new();
let mut seen_connector_ids = HashSet::new(); let mut seen_connector_ids = HashSet::new();
let mut plugin_display_names_by_connector_id: HashMap<String, Vec<String>> = HashMap::new(); let mut plugin_display_names_by_connector_id: HashMap<String, Vec<String>> = HashMap::new();
let mut disabled_connector_ids = HashSet::new();
for source in &sources { for source in sources {
if disabled_plugin_ids.contains(&source.plugin_id) {
disabled_connector_ids.extend(source.connector_ids.into_iter().map(|id| id.0));
continue;
}
for connector_id in source.connector_ids() { for connector_id in source.connector_ids() {
if seen_connector_ids.insert(connector_id.clone()) { if seen_connector_ids.insert(connector_id.0.clone()) {
connector_ids.push(connector_id.clone()); connector_ids.push(connector_id.clone());
} }
plugin_display_names_by_connector_id plugin_display_names_by_connector_id
@@ -93,23 +108,18 @@ impl ConnectorSnapshot {
plugin_names.sort_unstable(); plugin_names.sort_unstable();
plugin_names.dedup(); plugin_names.dedup();
} }
disabled_connector_ids.retain(|id| !seen_connector_ids.contains(id));
Self { Self {
sources,
connector_ids, connector_ids,
plugin_display_names_by_connector_id, plugin_display_names_by_connector_id,
disabled_connector_ids,
} }
} }
/// Adapts the current host plugin summaries to the connector-owned snapshot. /// Adapts the current host plugin summaries to the connector-owned snapshot.
pub fn from_plugin_capability_summaries(summaries: &[PluginCapabilitySummary]) -> Self { pub fn from_plugin_capability_summaries(summaries: &[PluginCapabilitySummary]) -> Self {
Self::from_plugin_sources(summaries.iter().map(|summary| { Self::from_plugin_sources(summaries.iter().map(PluginConnectorSource::from), &[])
PluginConnectorSource::from_connector_ids(
summary.config_name.clone(),
summary.display_name.clone(),
summary.app_connector_ids.clone(),
)
}))
} }
/// Returns the connector IDs in source contribution order. /// Returns the connector IDs in source contribution order.
@@ -117,6 +127,11 @@ impl ConnectorSnapshot {
&self.connector_ids &self.connector_ids
} }
/// Connector tools excluded because no enabled plugin still contributes them.
pub fn disabled_connector_ids(&self) -> &HashSet<String> {
&self.disabled_connector_ids
}
/// Returns the package display names associated with one connector. /// Returns the package display names associated with one connector.
pub fn plugin_display_names_for_connector_id(&self, connector_id: &str) -> &[String] { pub fn plugin_display_names_for_connector_id(&self, connector_id: &str) -> &[String] {
self.plugin_display_names_by_connector_id self.plugin_display_names_by_connector_id
@@ -124,11 +139,6 @@ impl ConnectorSnapshot {
.map(Vec::as_slice) .map(Vec::as_slice)
.unwrap_or_default() .unwrap_or_default()
} }
/// Combines two snapshots while preserving source order and provenance.
pub fn merged_with(&self, other: &Self) -> Self {
Self::from_plugin_sources(self.sources.iter().chain(&other.sources).cloned())
}
} }
#[cfg(test)] #[cfg(test)]
+42 -8
View File
@@ -6,19 +6,17 @@ use super::PluginConnectorSource;
#[test] #[test]
fn snapshot_merges_sources_in_order_and_dedupes_provenance() { fn snapshot_merges_sources_in_order_and_dedupes_provenance() {
let host_source = source("host", "Zulu", &["calendar", "calendar"]); let host = [
let host = ConnectorSnapshot::from_plugin_sources([
source("skills", "Skills only", &[]), source("skills", "Skills only", &[]),
host_source.clone(), source("host", "Zulu", &["calendar", "calendar"]),
]); ];
let selected = ConnectorSnapshot::from_plugin_sources([ let selected = [
source("selected-a", "Alpha", &["drive", "calendar"]), source("selected-a", "Alpha", &["drive", "calendar"]),
source("selected-b", "Alpha", &["calendar"]), source("selected-b", "Alpha", &["calendar"]),
]); ];
let merged = host.merged_with(&selected); let merged = ConnectorSnapshot::from_plugin_sources(host.into_iter().chain(selected), &[]);
assert_eq!(host.sources, vec![host_source]);
assert_eq!( assert_eq!(
merged.connector_ids(), merged.connector_ids(),
&[ &[
@@ -36,6 +34,42 @@ fn snapshot_merges_sources_in_order_and_dedupes_provenance() {
); );
} }
#[test]
fn disabled_plugins_preserve_shared_connectors() {
let sources = [
source("alpha", "Alpha", &["exclusive", "shared"]),
source("beta", "Beta", &["other", "shared"]),
];
let filtered = ConnectorSnapshot::from_plugin_sources(sources.clone(), &["alpha".to_string()]);
let expected = ConnectorSnapshot {
disabled_connector_ids: std::collections::HashSet::from(["exclusive".to_string()]),
..ConnectorSnapshot::from_plugin_sources(
[source("beta", "Beta", &["other", "shared"])],
&[],
)
};
assert_eq!(filtered, expected);
assert_eq!(
ConnectorSnapshot::from_plugin_sources(
sources.iter().rev().cloned(),
&["alpha".to_string()],
),
expected
);
assert_eq!(
ConnectorSnapshot::from_plugin_sources(sources, &["alpha".to_string(), "beta".to_string()],),
ConnectorSnapshot {
disabled_connector_ids: std::collections::HashSet::from([
"exclusive".to_string(),
"other".to_string(),
"shared".to_string(),
]),
..ConnectorSnapshot::default()
}
);
}
fn source(id: &str, display_name: &str, connector_ids: &[&str]) -> PluginConnectorSource { fn source(id: &str, display_name: &str, connector_ids: &[&str]) -> PluginConnectorSource {
PluginConnectorSource::from_connector_ids( PluginConnectorSource::from_connector_ids(
id, id,
+7 -7
View File
@@ -37,9 +37,9 @@ use codex_mcp::McpRuntimeContext;
use codex_mcp::McpRuntimeInput; use codex_mcp::McpRuntimeInput;
use codex_mcp::McpStartupPolicy; use codex_mcp::McpStartupPolicy;
use codex_mcp::ToolInfo; use codex_mcp::ToolInfo;
use codex_mcp::ToolPluginProvenance; use codex_mcp::ToolPluginContext;
use codex_mcp::effective_mcp_servers; use codex_mcp::effective_mcp_servers;
use codex_mcp::tool_plugin_provenance; use codex_mcp::tool_plugin_context;
use codex_protocol::mcp::ClientMcpExtensions; use codex_protocol::mcp::ClientMcpExtensions;
const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30); const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30);
@@ -224,10 +224,10 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
} }
let cache_key = accessible_connectors_cache_key(config, auth.as_ref()); 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(config).await;
let tool_plugin_provenance = tool_plugin_provenance(&mcp_config); let tool_plugin_context = tool_plugin_context(&mcp_config);
if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key) if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key)
{ {
let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_provenance); let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_context);
return Ok(AccessibleConnectorsStatus { return Ok(AccessibleConnectorsStatus {
connectors: cached_connectors, connectors: cached_connectors,
codex_apps_ready: true, codex_apps_ready: true,
@@ -334,7 +334,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager(
write_cached_accessible_connectors(cache_key, &accessible_connectors); write_cached_accessible_connectors(cache_key, &accessible_connectors);
} }
let accessible_connectors = let accessible_connectors =
with_app_plugin_sources(accessible_connectors, &tool_plugin_provenance); with_app_plugin_sources(accessible_connectors, &tool_plugin_context);
mcp_runtime.shutdown().await; mcp_runtime.shutdown().await;
Ok(AccessibleConnectorsStatus { Ok(AccessibleConnectorsStatus {
connectors: accessible_connectors, connectors: accessible_connectors,
@@ -501,10 +501,10 @@ fn accessible_connectors_for_app_list_from_mcp_tools(mcp_tools: &[ToolInfo]) ->
pub fn with_app_plugin_sources( pub fn with_app_plugin_sources(
mut connectors: Vec<AppInfo>, mut connectors: Vec<AppInfo>,
tool_plugin_provenance: &ToolPluginProvenance, tool_plugin_context: &ToolPluginContext,
) -> Vec<AppInfo> { ) -> Vec<AppInfo> {
for connector in &mut connectors { for connector in &mut connectors {
connector.plugin_display_names = tool_plugin_provenance connector.plugin_display_names = tool_plugin_context
.plugin_display_names_for_connector_id(connector.id.as_str()) .plugin_display_names_for_connector_id(connector.id.as_str())
.to_vec(); .to_vec();
} }
+8
View File
@@ -346,6 +346,14 @@ fn executor_hook_sources_for_step(step_context: &StepContext) -> Vec<ExecutorPlu
.enabled .enabled
}) })
}) })
.into_iter()
.filter(|source| {
!step_context
.turn
.disabled_plugin_ids
.contains(&source.plugin_id.as_key())
})
.collect()
}) })
.unwrap_or_default() .unwrap_or_default()
} }
+25 -11
View File
@@ -59,6 +59,7 @@ pub(crate) struct McpThreadIdentity<'a> {
pub(crate) session_source: &'a SessionSource, pub(crate) session_source: &'a SessionSource,
pub(crate) originator: &'a str, pub(crate) originator: &'a str,
pub(crate) environments: McpEnvironmentScope<'a>, pub(crate) environments: McpEnvironmentScope<'a>,
pub(crate) disabled_plugin_ids: &'a [String],
} }
enum OrderedMcpOverlay { enum OrderedMcpOverlay {
@@ -119,6 +120,7 @@ impl McpManager {
/*originator*/ /*originator*/
None, None,
McpEnvironmentScope::HostOnly, McpEnvironmentScope::HostOnly,
&[],
) )
.await .await
.config .config
@@ -146,6 +148,7 @@ impl McpManager {
.with_session_source(identity.session_source), .with_session_source(identity.session_source),
Some(identity.originator), Some(identity.originator),
identity.environments, identity.environments,
identity.disabled_plugin_ids,
) )
.await .await
} }
@@ -155,6 +158,7 @@ impl McpManager {
context: McpServerContributionContext<'_, Config>, context: McpServerContributionContext<'_, Config>,
originator: Option<&str>, originator: Option<&str>,
environment_scope: McpEnvironmentScope<'_>, environment_scope: McpEnvironmentScope<'_>,
disabled_plugin_ids: &[String],
) -> McpRuntimeProjection { ) -> McpRuntimeProjection {
let config = context.config(); let config = context.config();
let mut selected_plugin_available = false; let mut selected_plugin_available = false;
@@ -203,6 +207,8 @@ impl McpManager {
), ),
))); )));
} }
McpServerContribution::SelectedPlugin { ref plugin_id, .. }
if disabled_plugin_ids.contains(plugin_id) => {}
McpServerContribution::SelectedPlugin { McpServerContribution::SelectedPlugin {
name, name,
plugin_id, plugin_id,
@@ -228,11 +234,15 @@ impl McpManager {
plugin_display_name, plugin_display_name,
connector_ids, connector_ids,
} => { } => {
selected_plugin_available = true; if disabled_plugin_ids.contains(&plugin_id) {
selected_plugins.push(SelectedPluginIdentity { disabled_plugin_roots.push(selected_root_id);
selected_root_id, } else {
plugin_id: plugin_id.clone(), selected_plugin_available = true;
}); selected_plugins.push(SelectedPluginIdentity {
selected_root_id,
plugin_id: plugin_id.clone(),
});
}
if !connector_ids.is_empty() { if !connector_ids.is_empty() {
selected_plugin_connector_sources.push( selected_plugin_connector_sources.push(
PluginConnectorSource::from_connector_ids( PluginConnectorSource::from_connector_ids(
@@ -259,6 +269,15 @@ impl McpManager {
.plugins_manager .plugins_manager
.plugins_for_config(&config.plugins_config_input()) .plugins_for_config(&config.plugins_config_input())
.await; .await;
let host_plugin_connector_sources = loaded_plugins
.capability_summaries()
.iter()
.map(PluginConnectorSource::from);
let connector_snapshot = ConnectorSnapshot::from_plugin_sources(
host_plugin_connector_sources.chain(selected_plugin_connector_sources),
disabled_plugin_ids,
);
let loaded_plugins = loaded_plugins.without_plugins(disabled_plugin_ids);
let plugins_available = let plugins_available =
selected_plugin_available || !loaded_plugins.capability_summaries().is_empty(); selected_plugin_available || !loaded_plugins.capability_summaries().is_empty();
let mut mcp_config = config let mut mcp_config = config
@@ -333,12 +352,7 @@ impl McpManager {
); );
} }
mcp_config.mcp_server_catalog = catalog; mcp_config.mcp_server_catalog = catalog;
mcp_config.connector_snapshot = mcp_config.connector_snapshot = connector_snapshot;
mcp_config
.connector_snapshot
.merged_with(&ConnectorSnapshot::from_plugin_sources(
selected_plugin_connector_sources,
));
McpRuntimeProjection { McpRuntimeProjection {
config: mcp_config, config: mcp_config,
plugins_available, plugins_available,
+12 -15
View File
@@ -1374,6 +1374,7 @@ const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel";
#[derive(Clone, Debug, PartialEq, Eq, Serialize)] #[derive(Clone, Debug, PartialEq, Eq, Serialize)]
struct McpToolApprovalKey { struct McpToolApprovalKey {
server: String, server: String,
plugin_id: Option<String>,
connector_id: Option<String>, connector_id: Option<String>,
link_id: Option<String>, link_id: Option<String>,
tool_name: String, tool_name: String,
@@ -1642,6 +1643,7 @@ fn session_mcp_tool_approval_key(
Some(McpToolApprovalKey { Some(McpToolApprovalKey {
server: invocation.server.clone(), server: invocation.server.clone(),
plugin_id: metadata.and_then(|metadata| metadata.plugin_id.clone()),
connector_id, connector_id,
link_id: metadata.and_then(|metadata| metadata.link_id.clone()), link_id: metadata.and_then(|metadata| metadata.link_id.clone()),
tool_name: invocation.tool.clone(), tool_name: invocation.tool.clone(),
@@ -2154,7 +2156,13 @@ async fn maybe_persist_mcp_tool_approval(
}; };
persist_codex_app_tool_approval(&turn_context.config, &connector_id, &tool_name).await persist_codex_app_tool_approval(&turn_context.config, &connector_id, &tool_name).await
} else { } else {
persist_non_app_mcp_tool_approval(sess, &turn_context.config, &key.server, &tool_name).await persist_non_app_mcp_tool_approval(
&turn_context.config,
&key.server,
&tool_name,
key.plugin_id.as_deref(),
)
.await
}; };
if let Err(err) = persist_result { if let Err(err) = persist_result {
@@ -2207,33 +2215,22 @@ async fn persist_custom_mcp_tool_approval(
} }
async fn persist_non_app_mcp_tool_approval( async fn persist_non_app_mcp_tool_approval(
sess: &Session,
config: &Config, config: &Config,
server: &str, server: &str,
tool_name: &str, tool_name: &str,
plugin_id: Option<&str>,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
if let Some(config_edits_builder) = custom_mcp_tool_approval_config_builder(config, server)? { if let Some(config_edits_builder) = custom_mcp_tool_approval_config_builder(config, server)? {
return persist_custom_mcp_tool_approval_with(config_edits_builder, server, tool_name) return persist_custom_mcp_tool_approval_with(config_edits_builder, server, tool_name)
.await; .await;
} }
let plugin_config_name = sess if let Some(plugin_id) = plugin_id {
.services
.plugins_manager
.plugins_for_config(&config.plugins_config_input())
.await
.plugins()
.iter()
.filter(|plugin| plugin.is_active())
.find(|plugin| plugin.mcp_servers.contains_key(server))
.map(|plugin| plugin.config_name.clone());
if let Some(plugin_config_name) = plugin_config_name {
return ConfigEditsBuilder::for_config(config) return ConfigEditsBuilder::for_config(config)
.with_edits([ConfigEdit::SetPath { .with_edits([ConfigEdit::SetPath {
segments: vec![ segments: vec![
"plugins".to_string(), "plugins".to_string(),
plugin_config_name, plugin_id.to_string(),
"mcp_servers".to_string(), "mcp_servers".to_string(),
server.to_string(), server.to_string(),
"tools".to_string(), "tools".to_string(),
+25 -21
View File
@@ -910,6 +910,7 @@ fn custom_servers_support_session_and_persistent_approval() {
}; };
let expected = McpToolApprovalKey { let expected = McpToolApprovalKey {
server: "custom_server".to_string(), server: "custom_server".to_string(),
plugin_id: None,
connector_id: None, connector_id: None,
link_id: None, link_id: None,
tool_name: "run_action".to_string(), tool_name: "run_action".to_string(),
@@ -925,8 +926,24 @@ fn custom_servers_support_session_and_persistent_approval() {
/*metadata*/ None, /*metadata*/ None,
AppToolApproval::Auto AppToolApproval::Auto
), ),
Some(expected) Some(expected.clone())
); );
let mut metadata = approval_metadata(
/*connector_id*/ None, /*connector_name*/ None,
/*connector_description*/ None, /*tool_title*/ None,
/*tool_description*/ None,
);
for plugin_id in ["alpha@test", "beta@test"] {
metadata.plugin_id = Some(plugin_id.to_string());
let plugin_key = McpToolApprovalKey {
plugin_id: Some(plugin_id.to_string()),
..expected.clone()
};
assert_eq!(
session_mcp_tool_approval_key(&invocation, Some(&metadata), AppToolApproval::Auto),
Some(plugin_key)
);
}
} }
#[test] #[test]
@@ -946,6 +963,7 @@ fn codex_apps_connectors_support_persistent_approval() {
metadata.link_id = Some("link_a".to_string()); metadata.link_id = Some("link_a".to_string());
let expected = McpToolApprovalKey { let expected = McpToolApprovalKey {
server: CODEX_APPS_MCP_SERVER_NAME.to_string(), server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
plugin_id: None,
connector_id: Some("calendar".to_string()), connector_id: Some("calendar".to_string()),
link_id: Some("link_a".to_string()), link_id: Some("link_a".to_string()),
tool_name: "calendar/list_events".to_string(), tool_name: "calendar/list_events".to_string(),
@@ -2341,6 +2359,7 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config() {
std::fs::create_dir_all(&codex_home).expect("create codex home"); std::fs::create_dir_all(&codex_home).expect("create codex home");
let key = McpToolApprovalKey { let key = McpToolApprovalKey {
server: CODEX_APPS_MCP_SERVER_NAME.to_string(), server: CODEX_APPS_MCP_SERVER_NAME.to_string(),
plugin_id: None,
connector_id: Some("calendar".to_string()), connector_id: Some("calendar".to_string()),
link_id: None, link_id: None,
tool_name: "calendar/list_events".to_string(), tool_name: "calendar/list_events".to_string(),
@@ -2392,6 +2411,7 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_serve
turn_context.config = Arc::new(config); turn_context.config = Arc::new(config);
let key = McpToolApprovalKey { let key = McpToolApprovalKey {
server: "docs".to_string(), server: "docs".to_string(),
plugin_id: None,
connector_id: None, connector_id: None,
link_id: None, link_id: None,
tool_name: "search".to_string(), tool_name: "search".to_string(),
@@ -2426,29 +2446,12 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_serve
#[tokio::test] #[tokio::test]
async fn maybe_persist_mcp_tool_approval_writes_plugin_mcp_policy() { async fn maybe_persist_mcp_tool_approval_writes_plugin_mcp_policy() {
let (session, mut turn_context) = make_session_and_context().await; let (session, turn_context) = make_session_and_context().await;
let codex_home = session.codex_home().await; let codex_home = session.codex_home().await;
write_sample_plugin_mcp(codex_home.as_path()); std::fs::create_dir_all(&codex_home).expect("create codex home");
std::fs::write(
codex_home.join(CONFIG_TOML_FILE),
r#"
[features]
plugins = true
[plugins."sample@test"]
enabled = true
"#,
)
.expect("seed config");
let config = ConfigBuilder::default()
.codex_home(codex_home.to_path_buf())
.build()
.await
.expect("load config");
turn_context.config = Arc::new(config);
session.services.plugins_manager.clear_cache();
let key = McpToolApprovalKey { let key = McpToolApprovalKey {
server: "sample".to_string(), server: "sample".to_string(),
plugin_id: Some("sample@test".to_string()),
connector_id: None, connector_id: None,
link_id: None, link_id: None,
tool_name: "search".to_string(), tool_name: "search".to_string(),
@@ -2506,6 +2509,7 @@ async fn maybe_persist_mcp_tool_approval_writes_project_config_for_project_serve
turn_context.config = Arc::new(config); turn_context.config = Arc::new(config);
let key = McpToolApprovalKey { let key = McpToolApprovalKey {
server: "docs".to_string(), server: "docs".to_string(),
plugin_id: None,
connector_id: None, connector_id: None,
link_id: None, link_id: None,
tool_name: "search".to_string(), tool_name: "search".to_string(),
+9 -3
View File
@@ -98,11 +98,12 @@ impl Session {
config: &Config, config: &Config,
) -> (McpConfig, McpRuntimeContext) { ) -> (McpConfig, McpRuntimeContext) {
let originator = self.originator().await; let originator = self.originator().await;
let (session_source, host_fallback_cwd) = { let (session_source, host_fallback_cwd, disabled_plugin_ids) = {
let state = self.state.lock().await; let state = self.state.lock().await;
( (
state.session_configuration.session_source.clone(), state.session_configuration.session_source.clone(),
state.session_configuration.cwd().clone(), state.session_configuration.cwd().clone(),
state.active_disabled_plugin_ids.clone(),
) )
}; };
let environments = self.services.turn_environments.snapshot().await; let environments = self.services.turn_environments.snapshot().await;
@@ -128,6 +129,7 @@ impl Session {
McpThreadIdentity { McpThreadIdentity {
session_source: &session_source, session_source: &session_source,
originator: &originator, originator: &originator,
disabled_plugin_ids: &disabled_plugin_ids,
environments: McpEnvironmentScope::Live(&self.services.turn_environments), environments: McpEnvironmentScope::Live(&self.services.turn_environments),
}, },
&ready_selected_capability_roots, &ready_selected_capability_roots,
@@ -233,6 +235,7 @@ impl Session {
McpThreadIdentity { McpThreadIdentity {
session_source: &desired.session_source, session_source: &desired.session_source,
originator: &desired.originator, originator: &desired.originator,
disabled_plugin_ids: &desired.disabled_plugin_ids,
environments: McpEnvironmentScope::Live(&self.services.turn_environments), environments: McpEnvironmentScope::Live(&self.services.turn_environments),
}, },
&ready_selected_capability_roots, &ready_selected_capability_roots,
@@ -299,6 +302,7 @@ impl Session {
McpThreadIdentity { McpThreadIdentity {
session_source: &desired.session_source, session_source: &desired.session_source,
originator: &desired.originator, originator: &desired.originator,
disabled_plugin_ids: &desired.disabled_plugin_ids,
environments: McpEnvironmentScope::Live(&self.services.turn_environments), environments: McpEnvironmentScope::Live(&self.services.turn_environments),
}, },
&ready_selected_capability_roots, &ready_selected_capability_roots,
@@ -662,12 +666,13 @@ impl Session {
return; return;
}; };
let auth = self.services.auth_manager.auth().await; let auth = self.services.auth_manager.auth().await;
{ let disabled_plugin_ids = {
let mut state = self.state.lock().await; let mut state = self.state.lock().await;
let mut config = (*state.session_configuration.original_config_do_not_use).clone(); let mut config = (*state.session_configuration.original_config_do_not_use).clone();
config.mcp_servers = refresh_config.mcp_servers.clone(); config.mcp_servers = refresh_config.mcp_servers.clone();
state.session_configuration.original_config_do_not_use = Arc::new(config); state.session_configuration.original_config_do_not_use = Arc::new(config);
} state.active_disabled_plugin_ids.clone()
};
let ready_selected_capability_roots = self let ready_selected_capability_roots = self
.services .services
.mcp_runtime .mcp_runtime
@@ -690,6 +695,7 @@ impl Session {
McpThreadIdentity { McpThreadIdentity {
session_source: &turn_context.session_source, session_source: &turn_context.session_source,
originator: &turn_context.originator, originator: &turn_context.originator,
disabled_plugin_ids: &disabled_plugin_ids,
environments: McpEnvironmentScope::Live(&self.services.turn_environments), environments: McpEnvironmentScope::Live(&self.services.turn_environments),
}, },
&ready_selected_capability_roots, &ready_selected_capability_roots,
+8 -2
View File
@@ -28,6 +28,7 @@ pub(super) struct McpDesiredState {
pub(super) session_source: SessionSource, pub(super) session_source: SessionSource,
pub(super) environments: TurnEnvironmentSnapshot, pub(super) environments: TurnEnvironmentSnapshot,
pub(super) local_process_cwd: PathBuf, pub(super) local_process_cwd: PathBuf,
pub(super) disabled_plugin_ids: Vec<String>,
} }
impl Session { impl Session {
@@ -75,9 +76,12 @@ impl Session {
&self, &self,
auth: Option<CodexAuth>, auth: Option<CodexAuth>,
) -> McpDesiredState { ) -> McpDesiredState {
let session_configuration = { let (session_configuration, disabled_plugin_ids) = {
let state = self.state.lock().await; let state = self.state.lock().await;
state.session_configuration.clone() (
state.session_configuration.clone(),
state.active_disabled_plugin_ids.clone(),
)
}; };
let environments = self.services.turn_environments.snapshot().await; let environments = self.services.turn_environments.snapshot().await;
let cwd = environments let cwd = environments
@@ -98,6 +102,7 @@ impl Session {
session_source: session_configuration.session_source.clone(), session_source: session_configuration.session_source.clone(),
environments, environments,
local_process_cwd, local_process_cwd,
disabled_plugin_ids,
} }
} }
@@ -124,6 +129,7 @@ impl Session {
session_source: session_configuration.session_source.clone(), session_source: session_configuration.session_source.clone(),
environments: resolved_environments.clone(), environments: resolved_environments.clone(),
local_process_cwd, local_process_cwd,
disabled_plugin_ids: session_configuration.disabled_plugin_ids.clone(),
}; };
self.publish_mcp_runtime( self.publish_mcp_runtime(
&desired, &desired,
+12 -3
View File
@@ -235,6 +235,7 @@ mod mcp_prewarm;
mod mcp_refresh; mod mcp_refresh;
mod mcp_runtime; mod mcp_runtime;
pub(crate) mod multi_agents; pub(crate) mod multi_agents;
mod plugin_selection;
mod realtime_history; mod realtime_history;
mod retained_context; mod retained_context;
mod review; mod review;
@@ -2006,11 +2007,13 @@ impl Session {
} }
pub(crate) async fn refresh_hooks(&self, config: Arc<Config>) { pub(crate) async fn refresh_hooks(&self, config: Arc<Config>) {
let disabled_plugin_ids = self.state.lock().await.active_disabled_plugin_ids.clone();
let environments = self.services.turn_environments.snapshot().await; let environments = self.services.turn_environments.snapshot().await;
let hooks_config = build_hooks_config( let hooks_config = build_hooks_config(
config.as_ref(), config.as_ref(),
self.services.plugins_manager.as_ref(), self.services.plugins_manager.as_ref(),
environments.single_local_environment(), environments.single_local_environment(),
&disabled_plugin_ids,
) )
.await; .await;
@@ -2020,7 +2023,8 @@ impl Session {
if Arc::ptr_eq( if Arc::ptr_eq(
&state.session_configuration.original_config_do_not_use, &state.session_configuration.original_config_do_not_use,
&config, &config,
) { ) && state.active_disabled_plugin_ids == disabled_plugin_ids
{
let hooks = self.hooks().reconfigured(hooks_config); let hooks = self.hooks().reconfigured(hooks_config);
self.services.hooks.store(Arc::new(hooks)); self.services.hooks.store(Arc::new(hooks));
} }
@@ -4112,7 +4116,8 @@ impl Session {
.services .services
.plugins_manager .plugins_manager
.plugins_for_config(&turn_context.config.plugins_config_input()) .plugins_for_config(&turn_context.config.plugins_config_input())
.await; .await
.without_plugins(&turn_context.disabled_plugin_ids);
let recommended_plugin_candidates = if turn_context let recommended_plugin_candidates = if turn_context
.config .config
.features .features
@@ -4874,6 +4879,7 @@ async fn build_hooks_config(
config: &Config, config: &Config,
plugins_manager: &PluginsManager, plugins_manager: &PluginsManager,
environment: Option<&TurnEnvironment>, environment: Option<&TurnEnvironment>,
disabled_plugin_ids: &[String],
) -> HooksConfig { ) -> HooksConfig {
let (hook_shell_program, hook_shell_argv) = environment let (hook_shell_program, hook_shell_argv) = environment
.and_then(|environment| environment.shell.as_ref()) .and_then(|environment| environment.shell.as_ref())
@@ -4885,7 +4891,10 @@ async fn build_hooks_config(
}) })
.unwrap_or_default(); .unwrap_or_default();
let plugins_input = config.plugins_config_input(); let plugins_input = config.plugins_config_input();
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; let plugin_outcome = plugins_manager
.plugins_for_config(&plugins_input)
.await
.without_plugins(disabled_plugin_ids);
let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources(); let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources();
let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings(); let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings();
HooksConfig { HooksConfig {
@@ -0,0 +1,32 @@
//! Activates an admitted task's plugin selection without changing pending settings.
use std::sync::Arc;
use super::Session;
use super::turn_context::TurnContext;
impl Session {
pub(crate) async fn activate_plugin_selection(&self, turn_context: &TurnContext) {
let config = {
let mut state = self.state.lock().await;
if state.active_disabled_plugin_ids != turn_context.disabled_plugin_ids {
state.active_disabled_plugin_ids = turn_context.disabled_plugin_ids.clone();
self.mark_mcp_runtime_dirty();
}
Arc::clone(&state.session_configuration.original_config_do_not_use)
};
let plugin_outcome = self
.services
.plugins_manager
.plugins_for_config(&config.plugins_config_input())
.await
.without_plugins(&turn_context.disabled_plugin_ids);
// Cache changes from another process do not notify this session's hook runtime.
if !self.hooks().matches_plugin_hooks(
plugin_outcome.iter_effective_plugin_hook_sources(),
plugin_outcome.iter_effective_plugin_hook_warnings(),
) {
self.refresh_hooks(config).await;
}
}
}
+5
View File
@@ -996,6 +996,7 @@ impl Session {
let thread_extension_data_for_mcp = &thread_extension_data; let thread_extension_data_for_mcp = &thread_extension_data;
let mcp_originator = session_configuration.originator.clone(); let mcp_originator = session_configuration.originator.clone();
let mcp_session_source = session_configuration.session_source.clone(); let mcp_session_source = session_configuration.session_source.clone();
let mcp_disabled_plugin_ids = session_configuration.disabled_plugin_ids.clone();
let mcp_runtime_cwd = environment_selections let mcp_runtime_cwd = environment_selections
.first() .first()
.and_then(|environment| environment.cwd.to_abs_path().ok()) .and_then(|environment| environment.cwd.to_abs_path().ok())
@@ -1025,6 +1026,7 @@ impl Session {
McpThreadIdentity { McpThreadIdentity {
session_source: &mcp_session_source, session_source: &mcp_session_source,
originator: &mcp_originator, originator: &mcp_originator,
disabled_plugin_ids: &mcp_disabled_plugin_ids,
environments: McpEnvironmentScope::Initial(environment_selections), environments: McpEnvironmentScope::Initial(environment_selections),
}, },
/*ready_selected_capability_roots*/ &[], /*ready_selected_capability_roots*/ &[],
@@ -1340,6 +1342,7 @@ impl Session {
), ),
); );
state.base_instructions_provenance = base_instructions_provenance.clone(); state.base_instructions_provenance = base_instructions_provenance.clone();
state.active_disabled_plugin_ids = session_configuration.disabled_plugin_ids.clone();
let managed_network_requirements_configured = config let managed_network_requirements_configured = config
.config_layer_stack .config_layer_stack
.requirements_toml() .requirements_toml()
@@ -1419,6 +1422,7 @@ impl Session {
&config, &config,
plugins_manager.as_ref(), plugins_manager.as_ref(),
resolved_environments.single_local_environment(), resolved_environments.single_local_environment(),
&session_configuration.disabled_plugin_ids,
) )
.await; .await;
let (hooks, async_hook_results) = Hooks::new( let (hooks, async_hook_results) = Hooks::new(
@@ -1672,6 +1676,7 @@ impl Session {
McpThreadIdentity { McpThreadIdentity {
session_source: &session_configuration.session_source, session_source: &session_configuration.session_source,
originator: &session_configuration.originator, originator: &session_configuration.originator,
disabled_plugin_ids: &session_configuration.disabled_plugin_ids,
environments: McpEnvironmentScope::Live( environments: McpEnvironmentScope::Live(
&sess.services.turn_environments, &sess.services.turn_environments,
), ),
+4 -2
View File
@@ -878,7 +878,8 @@ async fn required_mcp_servers_for_input(
.services .services
.plugins_manager .plugins_manager
.plugins_for_config(&turn_context.config.plugins_config_input()) .plugins_for_config(&turn_context.config.plugins_config_input())
.await; .await
.without_plugins(&turn_context.disabled_plugin_ids);
let current_config = sess.services.mcp_runtime.current_config(); let current_config = sess.services.mcp_runtime.current_config();
let mentioned_plugins = let mentioned_plugins =
collect_explicit_plugin_mentions(user_input, loaded_plugins.capability_summaries()); collect_explicit_plugin_mentions(user_input, loaded_plugins.capability_summaries());
@@ -1661,7 +1662,8 @@ pub(crate) async fn prepare_tool_recommendations(
.plugins_manager .plugins_manager
.plugins_for_config(&turn_context.config.plugins_config_input()) .plugins_for_config(&turn_context.config.plugins_config_input())
.instrument(trace_span!("built_tools.load_plugins")) .instrument(trace_span!("built_tools.load_plugins"))
.await; .await
.without_plugins(&turn_context.disabled_plugin_ids);
let tool_suggest_is_enabled = tool_suggest_enabled(turn_context); let tool_suggest_is_enabled = tool_suggest_enabled(turn_context);
let auth = if tool_suggest_is_enabled { let auth = if tool_suggest_is_enabled {
sess.services.auth_manager.auth().await sess.services.auth_manager.auth().await
+2 -12
View File
@@ -1039,18 +1039,8 @@ impl Session {
.services .services
.plugins_manager .plugins_manager
.plugins_for_config(&plugins_input) .plugins_for_config(&plugins_input)
.await; .await
// Cache changes from another process do not notify this session's hook runtime. .without_plugins(&session_configuration.disabled_plugin_ids);
if !self.hooks().matches_plugin_hooks(
plugin_outcome.iter_effective_plugin_hook_sources(),
plugin_outcome.iter_effective_plugin_hook_warnings(),
) {
// Keep the refresh state out of the enclosing turn-construction future.
Box::pin(self.refresh_hooks(Arc::clone(
&session_configuration.original_config_do_not_use,
)))
.await;
}
let trusted_plugin_roots = TrustedPluginRoots::from_plugin_load_outcome( let trusted_plugin_roots = TrustedPluginRoots::from_plugin_load_outcome(
&plugin_outcome, &plugin_outcome,
per_turn_config.codex_home.as_path(), per_turn_config.codex_home.as_path(),
+3
View File
@@ -68,6 +68,8 @@ impl ReasoningEffortPin {
/// Persistent, session-scoped state previously stored directly on `Session`. /// Persistent, session-scoped state previously stored directly on `Session`.
pub(crate) struct SessionState { pub(crate) struct SessionState {
pub(crate) session_configuration: SessionConfiguration, pub(crate) session_configuration: SessionConfiguration,
/// Plugin selection of the last admitted task; settings updates take effect on the next task.
pub(crate) active_disabled_plugin_ids: Vec<String>,
/// Persisted origin of the session base instructions, when known. /// Persisted origin of the session base instructions, when known.
pub(crate) base_instructions_provenance: Option<BaseInstructionsProvenance>, pub(crate) base_instructions_provenance: Option<BaseInstructionsProvenance>,
pub(crate) history: ContextManager, pub(crate) history: ContextManager,
@@ -112,6 +114,7 @@ impl SessionState {
history: ContextManager, history: ContextManager,
) -> Self { ) -> Self {
Self { Self {
active_disabled_plugin_ids: Vec::new(),
session_configuration, session_configuration,
base_instructions_provenance: None, base_instructions_provenance: None,
history, history,
+1
View File
@@ -285,6 +285,7 @@ impl Session {
input: Vec<TurnInput>, input: Vec<TurnInput>,
task: T, task: T,
) { ) {
self.activate_plugin_selection(&turn_context).await;
// Inherited or recovered roots are applied before task start. Otherwise this // Inherited or recovered roots are applied before task start. Otherwise this
// task owns its turn, including background work. Later mail cannot change it. // task owns its turn, including background work. Later mail cannot change it.
turn_context turn_context
+54 -7
View File
@@ -1458,13 +1458,21 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
server.environment_id = environment_id.clone(); server.environment_id = environment_id.clone();
server.enabled = false; server.enabled = false;
let plugin_id = selected_root.id; let plugin_id = selected_root.id;
vec![codex_extension_api::McpServerContribution::SelectedPlugin { vec![
name: plugin_id.clone(), codex_extension_api::McpServerContribution::SelectedPluginPackage {
plugin_display_name: plugin_id.clone(), selected_root_id: plugin_id.clone(),
plugin_id, plugin_id: plugin_id.clone(),
selection_order: 0, plugin_display_name: plugin_id.clone(),
config: Box::new(server), connector_ids: vec![],
}] },
codex_extension_api::McpServerContribution::SelectedPlugin {
name: plugin_id.clone(),
plugin_display_name: plugin_id.clone(),
plugin_id,
selection_order: 0,
config: Box::new(server),
},
]
}) })
} }
} }
@@ -1477,6 +1485,10 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
.features .features
.enable(Feature::Apps) .enable(Feature::Apps)
.expect("test config should allow apps"); .expect("test config should allow apps");
config
.features
.enable(Feature::Plugins)
.expect("enable plugins");
std::fs::create_dir_all(&config.codex_home).expect("create codex home"); std::fs::create_dir_all(&config.codex_home).expect("create codex home");
let lifecycle_observed = Arc::new(std::sync::Mutex::new(Vec::new())); let lifecycle_observed = Arc::new(std::sync::Mutex::new(Vec::new()));
@@ -1550,6 +1562,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
McpThreadIdentity { McpThreadIdentity {
session_source: &SessionSource::Exec, session_source: &SessionSource::Exec,
originator: &first_originator, originator: &first_originator,
disabled_plugin_ids: &[],
environments: McpEnvironmentScope::Live(&first_session.services.turn_environments), environments: McpEnvironmentScope::Live(&first_session.services.turn_environments),
}, },
/*ready_selected_capability_roots*/ &[], /*ready_selected_capability_roots*/ &[],
@@ -1568,6 +1581,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
McpThreadIdentity { McpThreadIdentity {
session_source: &second_session_source, session_source: &second_session_source,
originator: &second_originator, originator: &second_originator,
disabled_plugin_ids: &[],
environments: McpEnvironmentScope::Live(&second_session.services.turn_environments), environments: McpEnvironmentScope::Live(&second_session.services.turn_environments),
}, },
/*ready_selected_capability_roots*/ &[], /*ready_selected_capability_roots*/ &[],
@@ -1628,6 +1642,39 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors()
.get("originator"), .get("originator"),
Some(&"codex_work_desktop".to_string()) Some(&"codex_work_desktop".to_string())
); );
for disabled_plugin_ids in [vec!["selected-a".to_string()], vec![]] {
let projection = first_session
.services
.mcp_manager
.runtime_config_for_step(
&config,
&first_session.services.mcp_thread_init,
&first_session.services.thread_extension_data,
McpThreadIdentity {
session_source: &SessionSource::Exec,
originator: &first_originator,
disabled_plugin_ids: &disabled_plugin_ids,
environments: McpEnvironmentScope::Live(
&first_session.services.turn_environments,
),
},
/*ready_selected_capability_roots*/ &[],
/*executor_capability_discovery*/ None,
)
.await;
assert_eq!(
projection.selected_plugins.disabled_plugin_roots,
disabled_plugin_ids
);
assert_eq!(
selected_servers(&projection.config).contains_key("selected-a"),
disabled_plugin_ids.is_empty()
);
assert_eq!(
projection.selected_plugins.plugins.len(),
usize::from(disabled_plugin_ids.is_empty())
);
}
} }
#[tokio::test] #[tokio::test]
@@ -61,6 +61,58 @@ use wiremock::matchers::path;
use super::rmcp_client::remote_aware_environment_id; use super::rmcp_client::remote_aware_environment_id;
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn thread_plugin_selection_disables_executor_hooks_without_disabling_their_server()
-> Result<()> {
skip_if_no_network!(Ok(()));
let fixture = executor_hook_fixture(
["disabled", "enabled", "disabled-again"]
.map(completed_turn_response)
.to_vec(),
)
.await?;
fixture.attach().await?;
for (disabled_plugin_ids, expected_calls) in [
(vec!["computer-use@openai-bundled".to_string()], 0),
(Vec::new(), 1),
(vec!["computer-use@openai-bundled".to_string()], 1),
] {
let enabled = disabled_plugin_ids.is_empty();
submit_thread_settings(
&fixture.test.codex,
ThreadSettingsOverrides {
disabled_plugin_ids: Some(disabled_plugin_ids),
..Default::default()
},
)
.await?;
fixture.test.submit_text_turn("finish this turn").await?;
if enabled {
fixture.wait_for_hook_call().await?;
}
assert_eq!(fixture.calls().await?.len(), expected_calls);
}
fixture
.test
.codex
.call_mcp_tool(
"node_repl",
"js",
Some(json!({"code": "1 + 1"})),
/*meta*/ None,
)
.await?;
assert_eq!(
fixture
.calls()
.await?
.last()
.context("standalone tool call")?["params"]["name"],
"js"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn executor_stop_hook_runs_after_attachment() -> Result<()> { async fn executor_stop_hook_runs_after_attachment() -> Result<()> {
skip_if_no_network!(Ok(())); skip_if_no_network!(Ok(()));
+148
View File
@@ -34,6 +34,7 @@ use codex_skills_extension::SkillsExtensionConfig;
use codex_skills_extension::install; use codex_skills_extension::install;
use core_test_support::apps_test_server::AppsTestServer; use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL;
use core_test_support::apps_test_server::recorded_apps_tool_calls;
use core_test_support::responses::ResponseMock; use core_test_support::responses::ResponseMock;
use core_test_support::responses::ResponsesRequest; use core_test_support::responses::ResponsesRequest;
use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_assistant_message;
@@ -51,6 +52,7 @@ use core_test_support::skip_if_no_network;
use core_test_support::skip_if_remote; use core_test_support::skip_if_remote;
use core_test_support::skip_if_target_windows; use core_test_support::skip_if_target_windows;
use core_test_support::stdio_server_bin; use core_test_support::stdio_server_bin;
use core_test_support::submit_thread_settings;
use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::TestCodex;
use core_test_support::test_codex::local_selections; use core_test_support::test_codex::local_selections;
use core_test_support::test_codex::test_codex; use core_test_support::test_codex::test_codex;
@@ -496,6 +498,152 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context(
Ok(()) Ok(())
} }
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn thread_disabled_plugins_filter_skills_and_tools_without_changing_shared_plugins()
-> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps = AppsTestServer::mount_with_connector_name(&server, "Google Calendar").await?;
let home = Arc::new(TempDir::new()?);
let skill_path = dunce::canonicalize(write_plugin_skill_plugin(home.as_ref()))?;
std::fs::write(
&skill_path,
"---\ndescription: inspect sample data\n---\nTHREAD_PLUGIN_SKILL_BODY\n",
)?;
write_plugin_mcp_plugin(home.as_ref(), &stdio_server_bin()?);
write_plugin_app_plugin_with_name(home.as_ref(), "sample_app");
let mut builder = test_codex()
.with_home(home)
.with_extensions(skills_extensions())
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
.with_config(move |config| {
config.features.enable(Feature::Apps).unwrap();
config.chatgpt_base_url = apps.chatgpt_base_url;
});
let test = builder.build_with_remote_and_local_env(&server).await?;
let manager = test.thread_manager.plugins_manager();
let plugins_input = test.config.plugins_config_input();
let shared_plugins = manager.plugins_for_config(&plugins_input).await;
let tool_calls = [
(
"sample",
"echo",
Some(serde_json::json!({"message": "plugin check"})),
),
(CODEX_APPS_MCP_SERVER_NAME, "calendar_list_events", None),
];
for (phase, enabled, injection_count) in [(0, true, 1), (1, false, 1), (2, true, 2)] {
if !enabled {
submit_thread_settings(
&test.codex,
ThreadSettingsOverrides {
disabled_plugin_ids: Some(vec![SAMPLE_PLUGIN_CONFIG_NAME.to_string()]),
..Default::default()
},
)
.await?;
// Saving pending settings must not change the admitted runtime.
for (server_name, tool, arguments) in &tool_calls {
test.codex
.call_mcp_tool(server_name, tool, arguments.clone(), /*meta*/ None)
.await?;
}
}
let app_call_id = format!("plugin-app-{phase}");
let mcp_call_id = format!("plugin-mcp-{phase}");
let mock = mount_sse_sequence(
&server,
vec![
sse(vec![
ev_response_created("search"),
ev_tool_search_call(
&app_call_id,
&serde_json::json!({"query": "create calendar event"}),
),
ev_tool_search_call(&mcp_call_id, &serde_json::json!({"query": "echo"})),
ev_completed("search"),
]),
sse(vec![ev_response_created("done"), ev_completed("done")]),
],
)
.await;
test.codex
.start_or_steer_turn(
TurnInputRequest::user_input(vec![
UserInput::Skill {
name: "sample:sample-search".into(),
path: skill_path.clone(),
},
UserInput::Mention {
name: "sample".into(),
path: format!("plugin://{SAMPLE_PLUGIN_CONFIG_NAME}"),
},
])
.with_thread_settings(ThreadSettingsOverrides {
disabled_plugin_ids: enabled.then(Vec::new),
..Default::default()
}),
)
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let requests = mock.requests();
// Previously injected instructions remain in history; disabled turns must add none.
for (role, marker) in [
("user", "THREAD_PLUGIN_SKILL_BODY"),
("developer", "Skills from this plugin"),
] {
assert_eq!(
requests[0]
.message_input_texts(role)
.iter()
.filter(|text| text.contains(marker))
.count(),
injection_count
);
}
for (call_id, namespace, tool) in [
(
&app_call_id,
SAMPLE_PLUGIN_APP_NAMESPACE,
SEARCH_CALENDAR_CREATE_TOOL,
),
(&mcp_call_id, SAMPLE_PLUGIN_MCP_NAMESPACE, "echo"),
] {
assert_eq!(
namespace_child_tool(&requests[1].tool_search_output(call_id), namespace, tool)
.is_some(),
enabled
);
}
let calls_before = recorded_apps_tool_calls(&server).await.len();
// Direct Apps RPC calls keep their existing behavior outside model tool filtering.
for (server_name, tool, arguments) in &tool_calls {
let result = test
.codex
.call_mcp_tool(server_name, tool, arguments.clone(), /*meta*/ None)
.await;
assert_eq!(
result.is_ok(),
enabled || *server_name == CODEX_APPS_MCP_SERVER_NAME,
"unexpected {server_name}/{tool} result: {result:?}"
);
}
assert_eq!(
recorded_apps_tool_calls(&server).await.len() - calls_before,
1
);
assert_eq!(
manager.plugins_for_config(&plugins_input).await,
shared_plugins
);
}
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn agent_plugin_skills_use_shared_catalog_and_direct_child_discovery() -> Result<()> { async fn agent_plugin_skills_use_shared_catalog_and_direct_child_discovery() -> Result<()> {
skip_if_no_network!(Ok(())); skip_if_no_network!(Ok(()));
+14 -1
View File
@@ -95,7 +95,7 @@ pub fn prompt_safe_plugin_description(description: Option<&str>) -> Option<Strin
/// Runtime view of loaded plugins and their derived capability summaries. /// Runtime view of loaded plugins and their derived capability summaries.
/// ///
/// Callers must apply any runtime capability policies before constructing this outcome. /// Runtime exclusions retain loaded metadata while removing derived capabilities.
#[derive(Debug, Clone, PartialEq)] #[derive(Debug, Clone, PartialEq)]
pub struct PluginLoadOutcome<M> { pub struct PluginLoadOutcome<M> {
plugins: Vec<LoadedPlugin<M>>, plugins: Vec<LoadedPlugin<M>>,
@@ -120,6 +120,19 @@ impl<M: Clone> PluginLoadOutcome<M> {
} }
} }
/// Marks matching canonical plugin IDs inactive while retaining their loaded metadata.
pub fn without_plugins(mut self, disabled_plugin_ids: &[String]) -> Self {
if disabled_plugin_ids.is_empty() {
return self;
}
for plugin in &mut self.plugins {
if disabled_plugin_ids.contains(&plugin.config_name) {
plugin.enabled = false;
}
}
Self::from_plugins(self.plugins)
}
pub fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot> { pub fn effective_plugin_skill_roots(&self) -> Vec<PluginSkillRoot> {
let mut skill_roots = Vec::new(); let mut skill_roots = Vec::new();
let mut seen_paths = HashSet::new(); let mut seen_paths = HashSet::new();