mirror of
https://github.com/openai/codex.git
synced 2026-06-02 19:31:59 +00:00
fix: Deduplicate installed local and remote curated plugins (#25681)
## Summary - Deduplicate installed `openai-curated` and `openai-curated-remote` plugin conflicts by feature flag. - Prefer remote when remote plugins are enabled; otherwise prefer local, while preserving one-sided installs. ## Testing - `just fmt` - `git diff --check` - Targeted `just test` was blocked locally because `cargo-nextest` is not installed.
This commit is contained in:
@@ -302,7 +302,6 @@ use codex_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use codex_core::windows_sandbox::WindowsSandboxSetupMode as CoreWindowsSandboxSetupMode;
|
||||
use codex_core::windows_sandbox::WindowsSandboxSetupRequest;
|
||||
use codex_core::windows_sandbox::sandbox_setup_is_complete;
|
||||
use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME;
|
||||
use codex_core_plugins::PluginInstallError as CorePluginInstallError;
|
||||
use codex_core_plugins::PluginInstallRequest;
|
||||
use codex_core_plugins::PluginLoadOutcome;
|
||||
|
||||
@@ -6,6 +6,8 @@ use codex_app_server_protocol::PluginInstallPolicy;
|
||||
use codex_app_server_protocol::PluginSharePrincipalRole;
|
||||
use codex_app_server_protocol::PluginShareTargetRole;
|
||||
use codex_config::types::McpServerConfig;
|
||||
use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME;
|
||||
use codex_core_plugins::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use codex_core_plugins::remote::RemotePluginScope;
|
||||
use codex_core_plugins::remote::is_valid_remote_plugin_id;
|
||||
use codex_core_plugins::remote::validate_remote_plugin_id;
|
||||
@@ -156,6 +158,52 @@ fn remote_installed_plugin_visible_scopes(config: &Config) -> Vec<RemotePluginSc
|
||||
scopes
|
||||
}
|
||||
|
||||
fn filter_openai_curated_installed_conflicts(
|
||||
marketplaces: &mut Vec<PluginMarketplaceEntry>,
|
||||
prefer_remote_curated_conflicts: bool,
|
||||
) {
|
||||
let local_installed_plugin_names = marketplaces
|
||||
.iter()
|
||||
.find(|marketplace| marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME)
|
||||
.map(|marketplace| installed_plugin_names(&marketplace.plugins))
|
||||
.unwrap_or_default();
|
||||
let remote_installed_plugin_names = marketplaces
|
||||
.iter()
|
||||
.find(|marketplace| marketplace.name == REMOTE_GLOBAL_MARKETPLACE_NAME)
|
||||
.map(|marketplace| installed_plugin_names(&marketplace.plugins))
|
||||
.unwrap_or_default();
|
||||
let conflicting_plugin_names = local_installed_plugin_names
|
||||
.intersection(&remote_installed_plugin_names)
|
||||
.cloned()
|
||||
.collect::<HashSet<_>>();
|
||||
if conflicting_plugin_names.is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
let marketplace_to_filter = if prefer_remote_curated_conflicts {
|
||||
OPENAI_CURATED_MARKETPLACE_NAME
|
||||
} else {
|
||||
REMOTE_GLOBAL_MARKETPLACE_NAME
|
||||
};
|
||||
for marketplace in marketplaces.iter_mut() {
|
||||
if marketplace.name != marketplace_to_filter {
|
||||
continue;
|
||||
}
|
||||
marketplace
|
||||
.plugins
|
||||
.retain(|plugin| !plugin.installed || !conflicting_plugin_names.contains(&plugin.name));
|
||||
}
|
||||
marketplaces.retain(|marketplace| !marketplace.plugins.is_empty());
|
||||
}
|
||||
|
||||
fn installed_plugin_names(plugins: &[PluginSummary]) -> HashSet<String> {
|
||||
plugins
|
||||
.iter()
|
||||
.filter(|plugin| plugin.installed)
|
||||
.map(|plugin| plugin.name.clone())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn remote_plugin_share_discoverability(
|
||||
discoverability: PluginShareDiscoverability,
|
||||
) -> codex_core_plugins::remote::RemotePluginShareDiscoverability {
|
||||
@@ -740,6 +788,10 @@ impl PluginRequestProcessor {
|
||||
)
|
||||
.await,
|
||||
);
|
||||
filter_openai_curated_installed_conflicts(
|
||||
&mut data,
|
||||
config.features.enabled(Feature::RemotePlugin),
|
||||
);
|
||||
|
||||
Ok(PluginInstalledResponse {
|
||||
marketplaces: data,
|
||||
|
||||
@@ -180,6 +180,107 @@ enabled = true
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_installed_prefers_remote_curated_conflicts_when_remote_plugin_enabled() -> Result<()>
|
||||
{
|
||||
let codex_home = TempDir::new()?;
|
||||
let server = MockServer::start().await;
|
||||
write_openai_curated_marketplace(codex_home.path(), &["linear", "calendar"])?;
|
||||
write_installed_plugin(&codex_home, "openai-curated", "linear")?;
|
||||
write_installed_plugin(&codex_home, "openai-curated", "calendar")?;
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
format!(
|
||||
r#"chatgpt_base_url = "{}/backend-api/"
|
||||
|
||||
[features]
|
||||
plugins = true
|
||||
remote_plugin = true
|
||||
plugin_sharing = false
|
||||
|
||||
[plugins."linear@openai-curated"]
|
||||
enabled = true
|
||||
|
||||
[plugins."calendar@openai-curated"]
|
||||
enabled = true
|
||||
"#,
|
||||
server.uri()
|
||||
),
|
||||
)?;
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("chatgpt-token")
|
||||
.account_id("account-123")
|
||||
.chatgpt_user_id("user-123")
|
||||
.chatgpt_account_id("account-123"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
let mut global_installed_body: serde_json::Value = serde_json::from_str(
|
||||
&remote_installed_plugin_body("", "1.2.3", /*enabled*/ true),
|
||||
)?;
|
||||
let mut remote_only = global_installed_body["plugins"][0].clone();
|
||||
remote_only["id"] = serde_json::json!("plugins~Plugin_11111111111111111111111111111111");
|
||||
remote_only["name"] = serde_json::json!("remote-only");
|
||||
remote_only["release"]["display_name"] = serde_json::json!("Remote Only");
|
||||
global_installed_body["plugins"]
|
||||
.as_array_mut()
|
||||
.expect("installed plugins should be an array")
|
||||
.push(remote_only);
|
||||
let global_installed_body = serde_json::to_string(&global_installed_body)?;
|
||||
mount_remote_installed_plugins(&server, "GLOBAL", &global_installed_body).await;
|
||||
mount_remote_installed_plugins(&server, "WORKSPACE", empty_remote_installed_plugins_body())
|
||||
.await;
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = mcp
|
||||
.send_plugin_installed_request(PluginInstalledParams {
|
||||
cwds: None,
|
||||
install_suggestion_plugin_names: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: PluginInstalledResponse = to_response(response)?;
|
||||
|
||||
let local_marketplace = response
|
||||
.marketplaces
|
||||
.iter()
|
||||
.find(|marketplace| marketplace.name == "openai-curated")
|
||||
.expect("expected openai-curated marketplace entry");
|
||||
assert_eq!(
|
||||
local_marketplace
|
||||
.plugins
|
||||
.iter()
|
||||
.map(|plugin| plugin.id.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["calendar@openai-curated".to_string()]
|
||||
);
|
||||
let remote_marketplace = response
|
||||
.marketplaces
|
||||
.iter()
|
||||
.find(|marketplace| marketplace.name == "openai-curated-remote")
|
||||
.expect("expected openai-curated-remote marketplace entry");
|
||||
assert_eq!(
|
||||
remote_marketplace
|
||||
.plugins
|
||||
.iter()
|
||||
.map(|plugin| plugin.id.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
"linear@openai-curated-remote".to_string(),
|
||||
"remote-only@openai-curated-remote".to_string(),
|
||||
]
|
||||
);
|
||||
assert_eq!(response.marketplace_load_errors, Vec::new());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_installed_ignores_local_cache_without_catalog() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
|
||||
@@ -5,6 +5,7 @@ use crate::manifest::load_plugin_manifest;
|
||||
use crate::marketplace::MarketplacePluginSource;
|
||||
use crate::marketplace::list_marketplaces;
|
||||
use crate::marketplace::load_marketplace;
|
||||
use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME;
|
||||
use crate::remote::RemoteInstalledPlugin;
|
||||
use crate::store::PluginStore;
|
||||
use crate::store::plugin_version_for_source;
|
||||
@@ -113,10 +114,15 @@ pub async fn load_plugins_from_layer_stack(
|
||||
extra_plugins: HashMap<String, PluginConfig>,
|
||||
store: &PluginStore,
|
||||
restriction_product: Option<Product>,
|
||||
prefer_remote_curated_conflicts: bool,
|
||||
) -> PluginLoadOutcome<McpServerConfig> {
|
||||
let skill_config_rules = skill_config_rules_from_stack(config_layer_stack);
|
||||
let mut configured_plugins = configured_plugins_from_stack(config_layer_stack);
|
||||
configured_plugins.extend(extra_plugins);
|
||||
let configured_plugins = merge_configured_plugins_with_remote_installed(
|
||||
configured_plugins_from_stack(config_layer_stack),
|
||||
extra_plugins,
|
||||
store,
|
||||
prefer_remote_curated_conflicts,
|
||||
);
|
||||
let mut configured_plugins: Vec<_> = configured_plugins.into_iter().collect();
|
||||
configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b));
|
||||
|
||||
@@ -149,6 +155,61 @@ pub async fn load_plugins_from_layer_stack(
|
||||
PluginLoadOutcome::from_plugins(plugins)
|
||||
}
|
||||
|
||||
fn merge_configured_plugins_with_remote_installed(
|
||||
mut configured_plugins: HashMap<String, PluginConfig>,
|
||||
extra_plugins: HashMap<String, PluginConfig>,
|
||||
store: &PluginStore,
|
||||
prefer_remote_curated_conflicts: bool,
|
||||
) -> HashMap<String, PluginConfig> {
|
||||
let local_curated_installed_plugin_keys = configured_plugins
|
||||
.keys()
|
||||
.filter_map(|plugin_key| {
|
||||
installed_plugin_name_for_marketplace(
|
||||
plugin_key,
|
||||
OPENAI_CURATED_MARKETPLACE_NAME,
|
||||
store,
|
||||
)
|
||||
.map(|plugin_name| (plugin_name, plugin_key.clone()))
|
||||
})
|
||||
.collect::<HashMap<_, _>>();
|
||||
|
||||
for (plugin_key, plugin_config) in extra_plugins {
|
||||
let remote_curated_plugin_name = installed_plugin_name_for_marketplace(
|
||||
&plugin_key,
|
||||
REMOTE_GLOBAL_MARKETPLACE_NAME,
|
||||
store,
|
||||
);
|
||||
let local_curated_plugin_key = remote_curated_plugin_name
|
||||
.as_ref()
|
||||
.and_then(|plugin_name| local_curated_installed_plugin_keys.get(plugin_name));
|
||||
|
||||
if let Some(local_curated_plugin_key) = local_curated_plugin_key {
|
||||
if prefer_remote_curated_conflicts {
|
||||
configured_plugins.remove(local_curated_plugin_key);
|
||||
} else {
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
configured_plugins.insert(plugin_key, plugin_config);
|
||||
}
|
||||
|
||||
configured_plugins
|
||||
}
|
||||
|
||||
fn installed_plugin_name_for_marketplace(
|
||||
plugin_key: &str,
|
||||
marketplace_name: &str,
|
||||
store: &PluginStore,
|
||||
) -> Option<String> {
|
||||
let plugin_id = PluginId::parse(plugin_key).ok()?;
|
||||
if plugin_id.marketplace_name != marketplace_name {
|
||||
return None;
|
||||
}
|
||||
store.active_plugin_root(&plugin_id)?;
|
||||
Some(plugin_id.plugin_name)
|
||||
}
|
||||
|
||||
pub fn remote_installed_plugins_to_config(
|
||||
plugins: &[RemoteInstalledPlugin],
|
||||
store: &PluginStore,
|
||||
|
||||
@@ -492,6 +492,7 @@ impl PluginsManager {
|
||||
self.remote_installed_plugin_configs(),
|
||||
&self.store,
|
||||
self.restriction_product,
|
||||
config.remote_plugin_enabled,
|
||||
)
|
||||
.await;
|
||||
log_plugin_load_errors(&outcome);
|
||||
@@ -537,6 +538,7 @@ impl PluginsManager {
|
||||
self.remote_installed_plugin_configs(),
|
||||
&self.store,
|
||||
self.restriction_product,
|
||||
config.remote_plugin_enabled,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
@@ -133,10 +133,14 @@ async fn load_config(codex_home: &Path, cwd: &Path) -> PluginsConfigInput {
|
||||
}
|
||||
|
||||
fn remote_installed_linear_plugin() -> RemoteInstalledPlugin {
|
||||
remote_installed_plugin("linear")
|
||||
}
|
||||
|
||||
fn remote_installed_plugin(name: &str) -> RemoteInstalledPlugin {
|
||||
RemoteInstalledPlugin {
|
||||
marketplace_name: "openai-curated-remote".to_string(),
|
||||
id: "plugins~Plugin_linear".to_string(),
|
||||
name: "linear".to_string(),
|
||||
id: format!("plugins~Plugin_{name}"),
|
||||
name: name.to_string(),
|
||||
enabled: true,
|
||||
install_policy: codex_app_server_protocol::PluginInstallPolicy::Available,
|
||||
auth_policy: codex_app_server_protocol::PluginAuthPolicy::OnUse,
|
||||
@@ -146,6 +150,18 @@ fn remote_installed_linear_plugin() -> RemoteInstalledPlugin {
|
||||
}
|
||||
}
|
||||
|
||||
fn write_cached_plugin(codex_home: &Path, marketplace_name: &str, plugin_name: &str) {
|
||||
write_plugin_with_version(
|
||||
&codex_home
|
||||
.join("plugins/cache")
|
||||
.join(marketplace_name)
|
||||
.join(plugin_name),
|
||||
"local",
|
||||
plugin_name,
|
||||
/*manifest_version*/ Some("local"),
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_plugins_loads_default_skills_and_mcp_servers() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
@@ -352,6 +368,92 @@ remote_plugin = true
|
||||
assert_eq!(outcome, PluginLoadOutcome::default());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_installed_cache_prefers_local_curated_conflicts_when_remote_plugin_disabled() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
remote_plugin = false
|
||||
|
||||
[plugins."linear@openai-curated"]
|
||||
enabled = true
|
||||
|
||||
[plugins."calendar@openai-curated"]
|
||||
enabled = true
|
||||
"#,
|
||||
);
|
||||
write_cached_plugin(codex_home.path(), "openai-curated", "linear");
|
||||
write_cached_plugin(codex_home.path(), "openai-curated", "calendar");
|
||||
write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear");
|
||||
write_cached_plugin(codex_home.path(), "openai-curated-remote", "remote-only");
|
||||
|
||||
let config = load_config(codex_home.path(), codex_home.path()).await;
|
||||
let manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
manager.write_remote_installed_plugins_cache(vec![
|
||||
remote_installed_plugin("linear"),
|
||||
remote_installed_plugin("remote-only"),
|
||||
]);
|
||||
|
||||
let outcome = manager.plugins_for_config(&config).await;
|
||||
assert_eq!(
|
||||
outcome
|
||||
.plugins()
|
||||
.iter()
|
||||
.map(|plugin| plugin.config_name.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
"calendar@openai-curated".to_string(),
|
||||
"linear@openai-curated".to_string(),
|
||||
"remote-only@openai-curated-remote".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn remote_installed_cache_prefers_remote_curated_conflicts_when_remote_plugin_enabled() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"[features]
|
||||
plugins = true
|
||||
remote_plugin = true
|
||||
|
||||
[plugins."linear@openai-curated"]
|
||||
enabled = true
|
||||
|
||||
[plugins."calendar@openai-curated"]
|
||||
enabled = true
|
||||
"#,
|
||||
);
|
||||
write_cached_plugin(codex_home.path(), "openai-curated", "linear");
|
||||
write_cached_plugin(codex_home.path(), "openai-curated", "calendar");
|
||||
write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear");
|
||||
write_cached_plugin(codex_home.path(), "openai-curated-remote", "remote-only");
|
||||
|
||||
let config = load_config(codex_home.path(), codex_home.path()).await;
|
||||
let manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
manager.write_remote_installed_plugins_cache(vec![
|
||||
remote_installed_plugin("linear"),
|
||||
remote_installed_plugin("remote-only"),
|
||||
]);
|
||||
|
||||
let outcome = manager.plugins_for_config(&config).await;
|
||||
assert_eq!(
|
||||
outcome
|
||||
.plugins()
|
||||
.iter()
|
||||
.map(|plugin| plugin.config_name.clone())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
"calendar@openai-curated".to_string(),
|
||||
"linear@openai-curated-remote".to_string(),
|
||||
"remote-only@openai-curated-remote".to_string(),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metadata() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
@@ -3713,6 +3815,7 @@ async fn load_plugins_ignores_project_config_files() {
|
||||
std::collections::HashMap::new(),
|
||||
&PluginStore::new(codex_home.path().to_path_buf()),
|
||||
Some(Product::Codex),
|
||||
/*prefer_remote_curated_conflicts*/ false,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user