mirror of
https://github.com/openai/codex.git
synced 2026-10-19 16:17:49 +00:00
Honor managed plugin disablement for executor capabilities (#40640)
## Why Selected executor plugin roots could still expose capabilities when managed requirements disabled the `plugins` feature. ## What changed - Suppress MCP servers, skills, apps, and connectors from selected executor plugins when `plugins` is disabled, while preserving the selected-root identity used to filter those capabilities. - Apply the policy consistently to direct selected-root discovery and batched executor capability discovery. ## Testing - Add contributor-level and app-server coverage that verifies disabled plugin capabilities are absent and their MCP servers never start. GitOrigin-RevId: e30bd8a936c4b0e083908b89a65f4d02e01747de
This commit is contained in:
@@ -39,6 +39,7 @@ use pretty_assertions::assert_eq;
|
||||
use pretty_assertions::assert_ne;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
use test_case::test_case;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
use tokio::io::BufReader;
|
||||
use tokio::process::Child;
|
||||
@@ -63,6 +64,93 @@ const MCP_SERVER_NAME: &str = "executor_probe";
|
||||
const MCP_CALL_ID: &str = "selected-executor-mcp-call";
|
||||
const CONNECTOR_ID: &str = "calendar";
|
||||
|
||||
#[test_case(false; "direct selected root discovery")]
|
||||
#[test_case(true; "batched executor capability discovery")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn managed_plugins_requirement_disables_selected_executor_plugin_capabilities(
|
||||
executor_capability_discovery: bool,
|
||||
) -> Result<()> {
|
||||
let responses_server = responses::start_mock_server().await;
|
||||
let (apps_url, apps_server_handle) = start_apps_server_with_delays(
|
||||
vec![AppInfo {
|
||||
id: CONNECTOR_ID.to_string(),
|
||||
name: "Calendar".to_string(),
|
||||
description: None,
|
||||
logo_url: None,
|
||||
logo_url_dark: None,
|
||||
icon_assets: None,
|
||||
icon_dark_assets: None,
|
||||
distribution_channel: None,
|
||||
branding: None,
|
||||
app_metadata: None,
|
||||
labels: None,
|
||||
install_url: None,
|
||||
is_accessible: false,
|
||||
is_enabled: true,
|
||||
plugin_display_names: Vec::new(),
|
||||
}],
|
||||
vec![connector_tool(CONNECTOR_ID, "Calendar")?],
|
||||
Duration::ZERO,
|
||||
Duration::ZERO,
|
||||
)
|
||||
.await?;
|
||||
let fixture = selected_capability_fixture(&responses_server.uri(), &apps_url)?;
|
||||
let config_path = fixture.codex_home.path().join("config.toml");
|
||||
let config = std::fs::read_to_string(&config_path)?.replace(
|
||||
"executor_capability_discovery = true",
|
||||
&format!("executor_capability_discovery = {executor_capability_discovery}\nplugins = true"),
|
||||
);
|
||||
std::fs::write(config_path, config)?;
|
||||
std::fs::write(
|
||||
fixture.codex_home.path().join("requirements.toml"),
|
||||
"[features]\nplugins = false\n",
|
||||
)?;
|
||||
let response_mock = responses::mount_sse_once(
|
||||
&responses_server,
|
||||
responses::sse(vec![
|
||||
responses::ev_response_created("disabled-selected-plugin"),
|
||||
responses::ev_assistant_message("disabled-selected-plugin-message", "Done"),
|
||||
responses::ev_completed("disabled-selected-plugin"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let mut app_server = TestAppServer::builder()
|
||||
.with_codex_home(fixture.codex_home.path())
|
||||
.without_auto_env()
|
||||
.build()
|
||||
.await?;
|
||||
timeout(READ_TIMEOUT, app_server.initialize()).await??;
|
||||
let thread_id = start_thread(
|
||||
&mut app_server,
|
||||
fixture.selected_root,
|
||||
fixture.environment_cwd.clone(),
|
||||
)
|
||||
.await?;
|
||||
let mut exec_server =
|
||||
spawn_exec_server(fixture.codex_home.path(), &fixture.exec_server_url).await?;
|
||||
add_environment(&mut app_server, &fixture.exec_server_url).await?;
|
||||
|
||||
run_turn(
|
||||
&mut app_server,
|
||||
&thread_id,
|
||||
"Inspect the disabled selected plugin capabilities",
|
||||
fixture.environment_cwd,
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(
|
||||
!fixture.pid_file.exists(),
|
||||
"the disabled selected plugin MCP server must never start"
|
||||
);
|
||||
assert_selected_capabilities_absent(&response_mock.single_request());
|
||||
|
||||
exec_server.kill().await?;
|
||||
apps_server_handle.abort();
|
||||
let _ = apps_server_handle.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn selected_capability_stack_tracks_environment_availability_and_resume() -> Result<()> {
|
||||
let responses_server = responses::start_mock_server().await;
|
||||
|
||||
@@ -17,6 +17,7 @@ use codex_extension_api::McpServerContribution;
|
||||
use codex_extension_api::McpServerContributionContext;
|
||||
use codex_extension_api::SelectedPluginIdentity;
|
||||
use codex_extension_api::SelectedPluginSnapshot;
|
||||
use codex_features::Feature;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use codex_mcp::EffectiveMcpServer;
|
||||
@@ -160,6 +161,7 @@ impl McpManager {
|
||||
let mut selected_plugin_connector_sources = Vec::new();
|
||||
let mut selected_plugin_registrations = Vec::new();
|
||||
let mut selected_plugins = Vec::new();
|
||||
let mut disabled_plugin_roots = Vec::new();
|
||||
let mut overlays = Vec::new();
|
||||
// A contributor can emit multiple ordered actions, so order each action globally rather
|
||||
// than enumerating contributors.
|
||||
@@ -200,6 +202,11 @@ impl McpManager {
|
||||
*config,
|
||||
),
|
||||
),
|
||||
McpServerContribution::SelectedPluginPackage {
|
||||
selected_root_id, ..
|
||||
} if !config.features.enabled(Feature::Plugins) => {
|
||||
disabled_plugin_roots.push(selected_root_id);
|
||||
}
|
||||
McpServerContribution::SelectedPluginPackage {
|
||||
selected_root_id,
|
||||
plugin_id,
|
||||
@@ -322,6 +329,7 @@ impl McpManager {
|
||||
plugins_available,
|
||||
selected_plugins: SelectedPluginSnapshot {
|
||||
plugins: selected_plugins,
|
||||
disabled_plugin_roots,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -116,6 +116,8 @@ impl<'a, C> McpServerContributionContext<'a, C> {
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct SelectedPluginSnapshot {
|
||||
pub plugins: Vec<SelectedPluginIdentity>,
|
||||
/// Selected plugin roots suppressed by the effective plugin feature policy.
|
||||
pub disabled_plugin_roots: Vec<String>,
|
||||
}
|
||||
|
||||
/// The configured identity of a plugin resolved from one selected root.
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_extension_api::ExtensionFuture;
|
||||
use codex_extension_api::McpServerContribution;
|
||||
use codex_extension_api::McpServerContributionContext;
|
||||
use codex_extension_api::McpServerContributor;
|
||||
use codex_features::Feature;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
@@ -215,7 +216,11 @@ fn project_metadata(
|
||||
selected_root_id: &str,
|
||||
plugin: SelectedPluginMetadata,
|
||||
) -> Vec<McpServerContribution> {
|
||||
let mut servers = plugin.servers.iter().cloned().collect::<HashMap<_, _>>();
|
||||
let mut servers = if config.features.enabled(Feature::Plugins) {
|
||||
plugin.servers.iter().cloned().collect::<HashMap<_, _>>()
|
||||
} else {
|
||||
HashMap::new()
|
||||
};
|
||||
if let Some(plugin_policy) = plugin_policy {
|
||||
apply_configured_plugin_mcp_server_policies(plugin_policy, &mut servers);
|
||||
}
|
||||
|
||||
@@ -157,6 +157,61 @@ async fn selected_plugin_package_is_contributed_without_servers_or_connectors()
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_plugins_requirement_disables_selected_executor_plugin_capabilities() -> TestResult
|
||||
{
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
let plugin_root = tempfile::tempdir()?;
|
||||
std::fs::create_dir_all(plugin_root.path().join(".codex-plugin"))?;
|
||||
std::fs::write(
|
||||
plugin_root.path().join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"selected-root","interface":{"displayName":"Selected Root"}}"#,
|
||||
)?;
|
||||
std::fs::write(
|
||||
plugin_root.path().join(".mcp.json"),
|
||||
r#"{"mcpServers":{"probe":{"command":"probe-command"}}}"#,
|
||||
)?;
|
||||
let mut config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.cloud_config_bundle(
|
||||
CloudConfigBundleFixture::loader_with_enterprise_requirement(
|
||||
r#"
|
||||
[features]
|
||||
plugins = false
|
||||
"#,
|
||||
),
|
||||
)
|
||||
.build()
|
||||
.await?;
|
||||
assert!(!config.features.enabled(Feature::Plugins));
|
||||
|
||||
let direct = raw_selected_plugin_contributions(&config, plugin_root.path()).await?;
|
||||
assert!(
|
||||
matches!(
|
||||
direct.as_slice(),
|
||||
[McpServerContribution::SelectedPluginPackage { selected_root_id, .. }]
|
||||
if selected_root_id == "selected-root"
|
||||
),
|
||||
"managed Plugins disable should preserve only the direct selected-root identity"
|
||||
);
|
||||
|
||||
config
|
||||
.features
|
||||
.enable(Feature::ExecutorCapabilityDiscovery)
|
||||
.expect("test config should allow feature update");
|
||||
let discovered = raw_selected_plugin_contributions(&config, plugin_root.path()).await?;
|
||||
assert!(
|
||||
matches!(
|
||||
discovered.as_slice(),
|
||||
[McpServerContribution::SelectedPluginPackage { selected_root_id, .. }]
|
||||
if selected_root_id == "selected-root"
|
||||
),
|
||||
"managed Plugins disable should preserve only the discovered selected-root identity"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn high_level_discovery_matches_the_existing_plugin_provider() -> TestResult {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
|
||||
@@ -52,6 +52,9 @@ pub(crate) fn attribute_executor_plugins(
|
||||
catalog: &mut SkillCatalog,
|
||||
snapshot: &SelectedPluginSnapshot,
|
||||
) {
|
||||
catalog
|
||||
.entries
|
||||
.retain(|skill| !snapshot.disabled_plugin_roots.contains(&skill.authority.id));
|
||||
for skill in &mut catalog.entries {
|
||||
if let Some(plugin) = snapshot
|
||||
.plugins
|
||||
|
||||
Reference in New Issue
Block a user