mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Rename plugin MCP and app extension APIs (#46564)
## What changed - Rename the extension providers to `PluginMcpProvider` and `PluginAppProvider`, and use app terminology for plugin app declarations. - Rename the MCP contributor to `PluginContributor` and its installation entry point to `install_plugins`; update the app-server call site and existing tests. - Move the corresponding modules and tests to plugin-based names, change the contributor ID to `plugin`, and update tracing spans and log messages to match. GitOrigin-RevId: 7106d2965675265f39e186e57cc334161bdb90d3
This commit is contained in:
committed by
copyberry
parent
3bb0a530d1
commit
b948cfa88d
@@ -95,7 +95,7 @@ pub(crate) fn thread_extensions(
|
||||
codex_guardian_v2::install(&mut builder, auth_manager.clone(), thread_manager);
|
||||
codex_memories_extension::install(&mut builder, codex_otel::global());
|
||||
codex_mcp_extension::install(&mut builder);
|
||||
codex_mcp_extension::install_executor_plugins(&mut builder, environment_manager);
|
||||
codex_mcp_extension::install_plugins(&mut builder, environment_manager);
|
||||
codex_web_search_extension::install(&mut builder, auth_manager.clone());
|
||||
codex_image_generation_extension::install(&mut builder, auth_manager, |config: &Config| {
|
||||
Some(config.codex_home.clone())
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
//! Executor-backed connector declaration loading.
|
||||
//! Plugin app declaration loading.
|
||||
|
||||
mod executor_plugin;
|
||||
mod plugin_app;
|
||||
|
||||
pub use executor_plugin::ExecutorPluginConnectorProvider;
|
||||
pub use executor_plugin::ExecutorPluginConnectorProviderError;
|
||||
pub use plugin_app::PluginAppProvider;
|
||||
pub use plugin_app::PluginAppProviderError;
|
||||
|
||||
@@ -7,13 +7,13 @@ use codex_utils_path_uri::PathUri;
|
||||
use std::io;
|
||||
use thiserror::Error;
|
||||
|
||||
/// Loads connector declarations from a resolved plugin through its owning executor.
|
||||
/// Loads app declarations from a resolved plugin.
|
||||
#[derive(Clone, Copy, Debug, Default)]
|
||||
pub struct ExecutorPluginConnectorProvider;
|
||||
pub struct PluginAppProvider;
|
||||
|
||||
/// Failure to load connector declarations from an executor plugin.
|
||||
/// Failure to load app declarations from a plugin.
|
||||
#[derive(Debug, Error)]
|
||||
pub enum ExecutorPluginConnectorProviderError {
|
||||
pub enum PluginAppProviderError {
|
||||
#[error("failed to read app config for selected plugin `{plugin_id}` at `{path}`: {source}")]
|
||||
ReadConfig {
|
||||
plugin_id: String,
|
||||
@@ -30,13 +30,13 @@ pub enum ExecutorPluginConnectorProviderError {
|
||||
},
|
||||
}
|
||||
|
||||
impl ExecutorPluginConnectorProvider {
|
||||
/// Returns the connector declarations contributed by `plugin`.
|
||||
#[tracing::instrument(name = "connectors.executor_plugin.declarations.load", skip_all)]
|
||||
impl PluginAppProvider {
|
||||
/// Returns the app declarations contributed by `plugin`.
|
||||
#[tracing::instrument(name = "connectors.plugin.declarations.load", skip_all)]
|
||||
pub async fn load(
|
||||
&self,
|
||||
plugin: &ResolvedExecutorPlugin,
|
||||
) -> Result<Vec<AppDeclaration>, ExecutorPluginConnectorProviderError> {
|
||||
) -> Result<Vec<AppDeclaration>, PluginAppProviderError> {
|
||||
let resolved_plugin = plugin.plugin();
|
||||
let plugin_id = resolved_plugin.selected_root_id();
|
||||
let Some(PluginResourceLocator::Environment {
|
||||
@@ -53,18 +53,16 @@ impl ExecutorPluginConnectorProvider {
|
||||
/*sandbox*/ None,
|
||||
)
|
||||
.await
|
||||
.map_err(|source| ExecutorPluginConnectorProviderError::ReadConfig {
|
||||
.map_err(|source| PluginAppProviderError::ReadConfig {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
path: config_path.clone(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
parse_plugin_app_config(&contents).map_err(|source| {
|
||||
ExecutorPluginConnectorProviderError::ParseConfig {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
path: config_path.clone(),
|
||||
source,
|
||||
}
|
||||
parse_plugin_app_config(&contents).map_err(|source| PluginAppProviderError::ParseConfig {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
path: config_path.clone(),
|
||||
source,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -10,7 +10,7 @@ use codex_mcp::hosted_plugin_runtime_mcp_server_config;
|
||||
#[cfg(test)]
|
||||
#[path = "event_stream_tests.rs"]
|
||||
mod event_stream_tests;
|
||||
mod executor_plugin;
|
||||
mod plugin;
|
||||
mod provider;
|
||||
mod stream_manager;
|
||||
|
||||
@@ -59,14 +59,14 @@ pub fn install(builder: &mut ExtensionRegistryBuilder<Config>) {
|
||||
builder.mcp_server_contributor(std::sync::Arc::new(HostedPluginRuntimeExtension));
|
||||
}
|
||||
|
||||
/// Installs discovery for MCP servers declared by thread-selected executor plugins.
|
||||
pub fn install_executor_plugins(
|
||||
/// Installs discovery for MCP servers and apps declared by thread-selected plugins.
|
||||
pub fn install_plugins(
|
||||
builder: &mut ExtensionRegistryBuilder<Config>,
|
||||
environment_manager: std::sync::Arc<codex_exec_server::EnvironmentManager>,
|
||||
) {
|
||||
builder.mcp_server_contributor(std::sync::Arc::new(
|
||||
executor_plugin::SelectedExecutorPluginMcpContributor::new(environment_manager),
|
||||
));
|
||||
builder.mcp_server_contributor(std::sync::Arc::new(plugin::PluginContributor::new(
|
||||
environment_manager,
|
||||
)));
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use codex_config::types::PluginMcpServerConfig;
|
||||
use codex_connectors_extension::ExecutorPluginConnectorProvider;
|
||||
use codex_connectors_extension::PluginAppProvider;
|
||||
use codex_core::config::Config;
|
||||
use codex_core_plugins::ExecutorPluginProvider;
|
||||
use codex_core_plugins::loader::apply_configured_plugin_mcp_server_policies;
|
||||
@@ -15,12 +15,12 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
|
||||
use self::provider::ExecutorPluginMcpProvider;
|
||||
use self::provider::PluginMcpProvider;
|
||||
|
||||
mod discovery;
|
||||
mod provider;
|
||||
|
||||
/// Frozen MCP and connector declarations for one selected package.
|
||||
/// Frozen MCP and app declarations for one selected package.
|
||||
///
|
||||
/// Each server config retains the stable logical environment ID. Reconnection may replace the
|
||||
/// concrete environment instance without changing that authority.
|
||||
@@ -33,7 +33,7 @@ struct SelectedPluginMetadata {
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub(crate) struct SelectedExecutorPluginMcpState {
|
||||
pub(crate) struct PluginContributorState {
|
||||
cache: Mutex<Vec<CachedSelectedRoot>>,
|
||||
}
|
||||
|
||||
@@ -42,18 +42,18 @@ struct CachedSelectedRoot {
|
||||
metadata: Option<SelectedPluginMetadata>,
|
||||
}
|
||||
|
||||
pub(crate) struct SelectedExecutorPluginMcpContributor {
|
||||
pub(crate) struct PluginContributor {
|
||||
plugin_provider: ExecutorPluginProvider,
|
||||
mcp_provider: ExecutorPluginMcpProvider,
|
||||
connector_provider: ExecutorPluginConnectorProvider,
|
||||
mcp_provider: PluginMcpProvider,
|
||||
app_provider: PluginAppProvider,
|
||||
}
|
||||
|
||||
impl SelectedExecutorPluginMcpContributor {
|
||||
impl PluginContributor {
|
||||
pub(crate) fn new(environment_manager: Arc<EnvironmentManager>) -> Self {
|
||||
Self {
|
||||
plugin_provider: ExecutorPluginProvider::new(Arc::clone(&environment_manager)),
|
||||
mcp_provider: ExecutorPluginMcpProvider,
|
||||
connector_provider: ExecutorPluginConnectorProvider,
|
||||
mcp_provider: PluginMcpProvider,
|
||||
app_provider: PluginAppProvider,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -62,10 +62,10 @@ impl SelectedExecutorPluginMcpContributor {
|
||||
/// Successful resolution, including a root that is not a plugin or declares no capabilities,
|
||||
/// is cached until the thread state is dropped. Environment availability never invalidates
|
||||
/// this cache; it only controls whether the cached metadata is projected into a model step.
|
||||
#[tracing::instrument(name = "mcp.executor_plugin.metadata.load", skip_all)]
|
||||
#[tracing::instrument(name = "mcp.plugin.metadata.load", skip_all)]
|
||||
async fn metadata_for_root(
|
||||
&self,
|
||||
state: &SelectedExecutorPluginMcpState,
|
||||
state: &PluginContributorState,
|
||||
selected_root: &SelectedCapabilityRoot,
|
||||
) -> Option<SelectedPluginMetadata> {
|
||||
if let Some(cached) = state
|
||||
@@ -84,34 +84,34 @@ impl SelectedExecutorPluginMcpContributor {
|
||||
tracing::warn!(
|
||||
selected_root = selected_root.id,
|
||||
error = %err,
|
||||
"failed to resolve selected executor plugin"
|
||||
"failed to resolve selected plugin"
|
||||
);
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let metadata = match plugin {
|
||||
Some(plugin) => {
|
||||
// MCP server declarations and app connector declarations are separate
|
||||
// MCP server and app declarations are separate
|
||||
// executor-owned files. Read them together so a remote environment only
|
||||
// pays for the slower read instead of both reads back-to-back.
|
||||
let (servers, connector_declarations) = tokio::join!(
|
||||
let (servers, app_declarations) = tokio::join!(
|
||||
self.mcp_provider.load(&plugin),
|
||||
self.connector_provider.load(&plugin)
|
||||
self.app_provider.load(&plugin)
|
||||
);
|
||||
let servers = servers.unwrap_or_else(|err| {
|
||||
tracing::warn!(
|
||||
selected_root = selected_root.id,
|
||||
error = %err,
|
||||
"failed to load selected executor plugin MCP servers"
|
||||
"failed to load selected plugin MCP servers"
|
||||
);
|
||||
Vec::new()
|
||||
});
|
||||
let connector_ids = connector_declarations
|
||||
let connector_ids = app_declarations
|
||||
.unwrap_or_else(|err| {
|
||||
tracing::warn!(
|
||||
selected_root = selected_root.id,
|
||||
error = %err,
|
||||
"failed to load selected executor plugin connectors"
|
||||
"failed to load selected plugin apps"
|
||||
);
|
||||
Vec::new()
|
||||
})
|
||||
@@ -142,9 +142,9 @@ impl SelectedExecutorPluginMcpContributor {
|
||||
}
|
||||
}
|
||||
|
||||
impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
||||
impl McpServerContributor<Config> for PluginContributor {
|
||||
fn id(&self) -> &'static str {
|
||||
"selected_executor_plugin_mcp"
|
||||
"plugin"
|
||||
}
|
||||
|
||||
fn contribute<'a>(
|
||||
@@ -189,7 +189,7 @@ impl McpServerContributor<Config> for SelectedExecutorPluginMcpContributor {
|
||||
));
|
||||
}
|
||||
} else {
|
||||
let state = thread_store.get_or_init(SelectedExecutorPluginMcpState::default);
|
||||
let state = thread_store.get_or_init(PluginContributorState::default);
|
||||
for (selection_order, selected_root) in selected_roots.iter().enumerate() {
|
||||
let Some(plugin) = self.metadata_for_root(&state, selected_root).await else {
|
||||
continue;
|
||||
@@ -14,13 +14,13 @@ use thiserror::Error;
|
||||
|
||||
const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json";
|
||||
|
||||
/// Loads MCP declarations from resolved plugins through their owning executor.
|
||||
/// Loads MCP declarations from resolved plugins.
|
||||
#[derive(Clone, Copy, Debug, Default)]
|
||||
pub(super) struct ExecutorPluginMcpProvider;
|
||||
pub(super) struct PluginMcpProvider;
|
||||
|
||||
/// Failure to load an executor plugin's MCP declarations.
|
||||
/// Failure to load a plugin's MCP declarations.
|
||||
#[derive(Debug, Error)]
|
||||
pub(super) enum ExecutorPluginMcpProviderError {
|
||||
pub(super) enum PluginMcpProviderError {
|
||||
#[error("failed to read MCP config for selected plugin `{plugin_id}` at `{path}`: {source}")]
|
||||
ReadConfig {
|
||||
plugin_id: String,
|
||||
@@ -47,13 +47,13 @@ pub(super) enum ExecutorPluginMcpProviderError {
|
||||
},
|
||||
}
|
||||
|
||||
impl ExecutorPluginMcpProvider {
|
||||
impl PluginMcpProvider {
|
||||
/// Returns MCP servers declared by `plugin`, bound to its environment.
|
||||
#[tracing::instrument(name = "mcp.executor_plugin.servers.load", skip_all)]
|
||||
#[tracing::instrument(name = "mcp.plugin.servers.load", skip_all)]
|
||||
pub(super) async fn load(
|
||||
&self,
|
||||
plugin: &ResolvedExecutorPlugin,
|
||||
) -> Result<Vec<(String, McpServerConfig)>, ExecutorPluginMcpProviderError> {
|
||||
) -> Result<Vec<(String, McpServerConfig)>, PluginMcpProviderError> {
|
||||
let ResolvedPluginLocation::Environment { root, .. } = plugin.plugin().location();
|
||||
|
||||
load_from_file_system(plugin.plugin(), root, plugin.file_system()).await
|
||||
@@ -64,7 +64,7 @@ async fn load_from_file_system(
|
||||
plugin: &ResolvedPlugin,
|
||||
plugin_root: &PathUri,
|
||||
file_system: &dyn ExecutorFileSystem,
|
||||
) -> Result<Vec<(String, McpServerConfig)>, ExecutorPluginMcpProviderError> {
|
||||
) -> Result<Vec<(String, McpServerConfig)>, PluginMcpProviderError> {
|
||||
let ResolvedPluginLocation::Environment { environment_id, .. } = plugin.location();
|
||||
let plugin_id = plugin.selected_root_id();
|
||||
let (contents, config_path) = match plugin.manifest().paths.mcp_servers.as_ref() {
|
||||
@@ -75,7 +75,7 @@ async fn load_from_file_system(
|
||||
file_system
|
||||
.read_file_text(path, ReadFileOptions::default(), /*sandbox*/ None)
|
||||
.await
|
||||
.map_err(|source| ExecutorPluginMcpProviderError::ReadConfig {
|
||||
.map_err(|source| PluginMcpProviderError::ReadConfig {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
path: path.clone(),
|
||||
source,
|
||||
@@ -90,7 +90,7 @@ async fn load_from_file_system(
|
||||
None => {
|
||||
let config_path = plugin_root
|
||||
.join(DEFAULT_MCP_CONFIG_FILE)
|
||||
.map_err(|source| ExecutorPluginMcpProviderError::InvalidConfigPath {
|
||||
.map_err(|source| PluginMcpProviderError::InvalidConfigPath {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
root: plugin_root.clone(),
|
||||
relative_path: DEFAULT_MCP_CONFIG_FILE,
|
||||
@@ -109,7 +109,7 @@ async fn load_from_file_system(
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
Err(source) => {
|
||||
return Err(ExecutorPluginMcpProviderError::ReadConfig {
|
||||
return Err(PluginMcpProviderError::ReadConfig {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
path: config_path.clone(),
|
||||
source,
|
||||
@@ -120,7 +120,7 @@ async fn load_from_file_system(
|
||||
}
|
||||
};
|
||||
let parsed = parse_executor_plugin_mcp_config(plugin_root, &contents, environment_id).map_err(
|
||||
|source| ExecutorPluginMcpProviderError::ParseConfig {
|
||||
|source| PluginMcpProviderError::ParseConfig {
|
||||
plugin_id: plugin_id.to_string(),
|
||||
path: config_path,
|
||||
source,
|
||||
@@ -132,7 +132,7 @@ async fn load_from_file_system(
|
||||
plugin = plugin_id,
|
||||
server = error.name,
|
||||
error = error.message,
|
||||
"ignoring invalid executor plugin MCP server"
|
||||
"ignoring invalid plugin MCP server"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use super::DEFAULT_MCP_CONFIG_FILE;
|
||||
use super::ExecutorPluginMcpProviderError;
|
||||
use super::PluginMcpProviderError;
|
||||
use super::load_from_file_system;
|
||||
use codex_config::McpServerConfig;
|
||||
use codex_config::McpServerTransportConfig;
|
||||
@@ -47,7 +47,7 @@ impl SyntheticExecutorFileSystem {
|
||||
fn unsupported<T>() -> FileSystemResult<T> {
|
||||
Err(io::Error::new(
|
||||
io::ErrorKind::Unsupported,
|
||||
"operation is not used by executor MCP provider tests",
|
||||
"operation is not used by plugin MCP provider tests",
|
||||
))
|
||||
}
|
||||
}
|
||||
@@ -176,7 +176,7 @@ async fn reads_declared_config_only_through_executor_file_system() {
|
||||
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
||||
let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
||||
.await
|
||||
.expect("load executor MCP config");
|
||||
.expect("load plugin MCP config");
|
||||
|
||||
assert_eq!(
|
||||
servers,
|
||||
@@ -263,7 +263,7 @@ async fn reads_manifest_object_config_without_executor_file_system_access() {
|
||||
let plugin_root_uri = PathUri::from_abs_path(&plugin_root);
|
||||
let servers = load_from_file_system(&plugin, &plugin_root_uri, &file_system)
|
||||
.await
|
||||
.expect("load manifest object executor MCP config");
|
||||
.expect("load manifest object plugin MCP config");
|
||||
|
||||
assert_eq!(
|
||||
servers,
|
||||
@@ -342,7 +342,7 @@ async fn malformed_declared_config_is_an_error() {
|
||||
.await
|
||||
.expect_err("malformed declared config should fail");
|
||||
|
||||
let ExecutorPluginMcpProviderError::ParseConfig {
|
||||
let PluginMcpProviderError::ParseConfig {
|
||||
plugin_id,
|
||||
path,
|
||||
source: _,
|
||||
@@ -380,7 +380,7 @@ async fn malformed_manifest_object_config_reports_actual_manifest_path() {
|
||||
.await
|
||||
.expect_err("malformed manifest object config should fail");
|
||||
|
||||
let ExecutorPluginMcpProviderError::ParseConfig {
|
||||
let PluginMcpProviderError::ParseConfig {
|
||||
plugin_id,
|
||||
path,
|
||||
source: _,
|
||||
@@ -158,8 +158,7 @@ async fn selected_plugin_package_is_contributed_without_servers_or_connectors()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn managed_plugins_requirement_disables_selected_executor_plugin_capabilities() -> TestResult
|
||||
{
|
||||
async fn managed_plugins_requirement_disables_selected_plugin_capabilities() -> TestResult {
|
||||
let codex_home = tempfile::tempdir()?;
|
||||
let plugin_root = tempfile::tempdir()?;
|
||||
std::fs::create_dir_all(plugin_root.path().join(".codex-plugin"))?;
|
||||
@@ -382,7 +381,7 @@ async fn raw_selected_plugin_contributions(
|
||||
) -> Result<Vec<McpServerContribution>, Box<dyn std::error::Error>> {
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
let environment_manager = Arc::new(EnvironmentManager::default_for_tests());
|
||||
codex_mcp_extension::install_executor_plugins(&mut builder, Arc::clone(&environment_manager));
|
||||
codex_mcp_extension::install_plugins(&mut builder, Arc::clone(&environment_manager));
|
||||
let registry = builder.build();
|
||||
let thread_init = ExtensionDataInit::new();
|
||||
let selected_capability_roots = vec![SelectedCapabilityRoot {
|
||||
Reference in New Issue
Block a user