diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 82f92febee..e1a97216bd 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2504,6 +2504,7 @@ version = "0.0.0" dependencies = [ "anyhow", "chrono", + "codex-analytics", "codex-app-server-protocol", "codex-config", "codex-core-skills", diff --git a/codex-rs/app-server/src/codex_message_processor/plugins.rs b/codex-rs/app-server/src/codex_message_processor/plugins.rs index e64267b279..3b7e0b9fc0 100644 --- a/codex-rs/app-server/src/codex_message_processor/plugins.rs +++ b/codex-rs/app-server/src/codex_message_processor/plugins.rs @@ -2,10 +2,6 @@ use super::*; use crate::error_code::internal_error; use crate::error_code::invalid_request; use codex_app_server_protocol::PluginInstallPolicy; -use codex_core::config::edit::ConfigEditsBuilder; -use codex_core_plugins::loader::installed_plugin_telemetry_metadata; -use codex_core_plugins::loader::plugin_telemetry_metadata_from_root; -use codex_core_plugins::store::PluginStore; impl CodexMessageProcessor { pub(super) async fn plugin_list( @@ -483,17 +479,6 @@ impl CodexMessageProcessor { .install_plugin(request) .await .map_err(Self::plugin_install_error)?; - let installed_plugin_id = result.plugin_id.as_key(); - ConfigEditsBuilder::new(config.codex_home.as_path()) - .set_plugin_enabled(&installed_plugin_id, /*enabled*/ true) - .apply() - .await - .map_err(|err| { - internal_error(format!("failed to persist installed plugin config: {err}")) - })?; - self.analytics_events_client.track_plugin_installed( - plugin_telemetry_metadata_from_root(&result.plugin_id, &result.installed_path).await, - ); let config = match self.load_latest_config(config_cwd).await { Ok(config) => config, Err(err) => { @@ -518,7 +503,7 @@ impl CodexMessageProcessor { .plugin_apps_needing_auth_for_install( &config, auth.as_ref().is_some_and(CodexAuth::is_chatgpt_auth), - &installed_plugin_id, + &result.plugin_id.as_key(), &plugin_apps, ) .await; @@ -716,34 +701,12 @@ impl CodexMessageProcessor { if !plugin_id.is_empty() && is_valid_remote_plugin_id(&plugin_id) { return self.remote_plugin_uninstall_response(plugin_id).await; } - let parsed_plugin_id = codex_core::plugins::PluginId::parse(&plugin_id) - .map_err(|err| invalid_request(err.to_string()))?; - let plugin_telemetry = - match PluginStore::try_new(self.config.codex_home.as_path().to_path_buf()) { - Ok(store) if store.active_plugin_root(&parsed_plugin_id).is_some() => Some( - installed_plugin_telemetry_metadata( - self.config.codex_home.as_path(), - &parsed_plugin_id, - ) - .await, - ), - Ok(_) | Err(_) => None, - }; let plugins_manager = self.thread_manager.plugins_manager(); plugins_manager .uninstall_plugin(plugin_id.clone()) .await .map_err(Self::plugin_uninstall_error)?; - ConfigEditsBuilder::new(self.config.codex_home.as_path()) - .clear_plugin(&plugin_id) - .apply() - .await - .map_err(|err| internal_error(format!("failed to clear plugin config: {err}")))?; - if let Some(plugin_telemetry) = plugin_telemetry { - self.analytics_events_client - .track_plugin_uninstalled(plugin_telemetry); - } match self.load_latest_config(/*fallback_cwd*/ None).await { Ok(config) => self.on_effective_plugins_changed(config), Err(err) => { @@ -765,6 +728,9 @@ impl CodexMessageProcessor { CorePluginInstallError::Marketplace(err) => { Self::marketplace_error(err, "install plugin") } + CorePluginInstallError::Config(err) => { + internal_error(format!("failed to persist installed plugin config: {err}")) + } CorePluginInstallError::Remote(err) => { internal_error(format!("failed to enable remote plugin: {err}")) } @@ -783,6 +749,9 @@ impl CodexMessageProcessor { } match err { + CorePluginUninstallError::Config(err) => { + internal_error(format!("failed to clear plugin config: {err}")) + } CorePluginUninstallError::Remote(err) => { internal_error(format!("failed to uninstall remote plugin: {err}")) } diff --git a/codex-rs/app-server/src/config/external_agent_config.rs b/codex-rs/app-server/src/config/external_agent_config.rs index 8d41632429..96ba5b1520 100644 --- a/codex-rs/app-server/src/config/external_agent_config.rs +++ b/codex-rs/app-server/src/config/external_agent_config.rs @@ -1,7 +1,6 @@ use codex_config::types::PluginConfig; use codex_core::config::Config; use codex_core::config::ConfigBuilder; -use codex_core::config::edit::ConfigEditsBuilder; use codex_core::plugins::PluginId; use codex_core::plugins::PluginInstallRequest; use codex_core::plugins::PluginsManager; @@ -759,16 +758,9 @@ impl ExternalAgentConfigService { }) .await { - Ok(result) => match ConfigEditsBuilder::new(self.codex_home.as_path()) - .set_plugin_enabled(&result.plugin_id.as_key(), /*enabled*/ true) - .apply() - .await - { - Ok(()) => outcome.succeeded_plugin_ids.push(result.plugin_id.as_key()), - Err(_) => outcome - .failed_plugin_ids - .push(format!("{plugin_name}@{marketplace_name}")), - }, + Ok(_) => outcome + .succeeded_plugin_ids + .push(format!("{plugin_name}@{marketplace_name}")), Err(_) => outcome .failed_plugin_ids .push(format!("{plugin_name}@{marketplace_name}")), diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 466a27179a..c915e5a356 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -292,6 +292,9 @@ impl MessageProcessor { environment_manager, Some(analytics_events_client.clone()), )); + thread_manager + .plugins_manager() + .set_analytics_events_client(analytics_events_client.clone()); let codex_message_processor = CodexMessageProcessor::new(CodexMessageProcessorArgs { auth_manager: auth_manager.clone(), diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index 3d8b54363d..1ee1420341 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -19,8 +19,6 @@ use codex_config::types::AuthCredentialsStoreMode; use codex_core::config::set_project_trust_level; use codex_protocol::config_types::TrustLevel; use codex_utils_absolute_path::AbsolutePathBuf; -use flate2::Compression; -use flate2::write::GzEncoder; use pretty_assertions::assert_eq; use tempfile::TempDir; use tokio::time::timeout; @@ -35,8 +33,6 @@ use wiremock::matchers::query_param; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; const STARTUP_REMOTE_PLUGIN_SYNC_MARKER_FILE: &str = ".tmp/app-server-remote-plugin-sync-v1"; -const TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS: &str = - "CODEX_TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS"; const ALTERNATE_MARKETPLACE_RELATIVE_PATH: &str = ".claude-plugin/marketplace.json"; const ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH: &str = ".claude-plugin/plugin.json"; @@ -1208,102 +1204,6 @@ async fn app_server_startup_refreshes_remote_installed_cache_each_process() -> R Ok(()) } -#[tokio::test] -async fn app_server_startup_downloads_missing_remote_installed_plugin_bundle() -> Result<()> { - let codex_home = TempDir::new()?; - let server = MockServer::start().await; - write_remote_plugin_catalog_config( - codex_home.path(), - &format!("{}/backend-api/", 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 bundle_url = - mount_remote_plugin_bundle(&server, remote_plugin_bundle_tar_gz_bytes("linear")?).await; - let global_installed_body = format!( - r#"{{ - "plugins": [ - {{ - "id": "plugins~Plugin_linear", - "name": "linear", - "scope": "GLOBAL", - "installation_policy": "AVAILABLE", - "authentication_policy": "ON_USE", - "release": {{ - "version": "1.2.3", - "bundle_download_url": "{bundle_url}", - "display_name": "Linear", - "description": "Track work in Linear", - "app_ids": [], - "interface": {{}}, - "skills": [] - }}, - "enabled": true, - "disabled_skill_names": [] - }} - ], - "pagination": {{ - "limit": 50, - "next_page_token": null - }} -}}"# - ); - let empty_page_body = r#"{ - "plugins": [], - "pagination": { - "limit": 50, - "next_page_token": null - } -}"#; - for (scope, body) in [ - ("GLOBAL", global_installed_body.as_str()), - ("WORKSPACE", empty_page_body), - ] { - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", scope)) - .and(query_param("includeDownloadUrls", "true")) - .and(header("authorization", "Bearer chatgpt-token")) - .and(header("chatgpt-account-id", "account-123")) - .respond_with(ResponseTemplate::new(200).set_body_string(body)) - .mount(&server) - .await; - } - - let mut mcp = McpProcess::new_with_env_and_plugin_startup_tasks( - codex_home.path(), - &[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))], - ) - .await?; - timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; - - let installed_manifest = codex_home - .path() - .join("plugins/cache/chatgpt-global/linear/1.2.3/.codex-plugin/plugin.json"); - timeout(DEFAULT_TIMEOUT, async { - loop { - if installed_manifest.is_file() { - break; - } - tokio::time::sleep(Duration::from_millis(10)).await; - } - }) - .await?; - assert_eq!( - std::fs::read_to_string(installed_manifest)?, - r#"{"name":"linear"}"# - ); - - Ok(()) -} - #[tokio::test] async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() -> Result<()> { let codex_home = TempDir::new()?; @@ -1876,42 +1776,3 @@ fn write_openai_curated_marketplace( )?; Ok(()) } - -async fn mount_remote_plugin_bundle(server: &MockServer, body: Vec) -> String { - Mock::given(method("GET")) - .and(path("/bundles/linear.tar.gz")) - .respond_with( - ResponseTemplate::new(200) - .insert_header("content-type", "application/gzip") - .set_body_bytes(body), - ) - .mount(server) - .await; - format!("{}/bundles/linear.tar.gz", server.uri()) -} - -fn remote_plugin_bundle_tar_gz_bytes(plugin_name: &str) -> Result> { - let manifest = format!(r#"{{"name":"{plugin_name}"}}"#); - let skill = "# Plan Work\n\nTrack work in Linear.\n"; - let encoder = GzEncoder::new(Vec::new(), Compression::default()); - let mut tar = tar::Builder::new(encoder); - for (path, contents, mode) in [ - ( - ".codex-plugin/plugin.json", - manifest.as_bytes(), - /*mode*/ 0o644, - ), - ( - "skills/plan-work/SKILL.md", - skill.as_bytes(), - /*mode*/ 0o644, - ), - ] { - let mut header = tar::Header::new_gnu(); - header.set_size(contents.len() as u64); - header.set_mode(mode); - header.set_cksum(); - tar.append_data(&mut header, path, contents)?; - } - Ok(tar.into_inner()?.finish()?) -} diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index 0b2c5bb7f8..d467502b66 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -12,6 +12,7 @@ use toml_edit::Item as TomlItem; use toml_edit::Table as TomlTable; use toml_edit::value; +use codex_utils_path::resolve_symlink_write_paths; use codex_utils_path::write_atomically; use crate::AppToolApproval; @@ -112,14 +113,23 @@ impl ConfigEditsBuilder { fn apply_blocking(self) -> std::io::Result<()> { let config_path = self.codex_home.join(CONFIG_TOML_FILE); - let mut doc = read_or_create_document(&config_path)?; + let write_paths = resolve_symlink_write_paths(&config_path)?; + let mut doc = match write_paths.read_path.as_deref() { + Some(read_path) => read_or_create_document(read_path)?, + None => DocumentMut::new(), + }; + let mut mutated = false; if let Some(servers) = self.mcp_servers.as_ref() { replace_mcp_servers(&mut doc, servers); + mutated = true; } for edit in &self.plugin_edits { - apply_plugin_config_edit(&mut doc, edit); + mutated |= apply_plugin_config_edit(&mut doc, edit); } - write_atomically(&config_path, &doc.to_string()) + if !mutated { + return Ok(()); + } + write_atomically(&write_paths.write_path, &doc.to_string()) } } @@ -148,31 +158,56 @@ fn replace_mcp_servers(doc: &mut DocumentMut, servers: &BTreeMap bool { match edit { PluginConfigEdit::SetEnabled { plugin_id, enabled } => { - set_plugin_enabled(doc, plugin_id, *enabled); - } - PluginConfigEdit::Clear { plugin_id } => { - clear_plugin(doc, plugin_id); + set_plugin_enabled(doc, plugin_id, *enabled) } + PluginConfigEdit::Clear { plugin_id } => clear_plugin(doc, plugin_id), } } -fn set_plugin_enabled(doc: &mut DocumentMut, plugin_id: &str, enabled: bool) { +fn set_plugin_enabled(doc: &mut DocumentMut, plugin_id: &str, enabled: bool) -> bool { let root = doc.as_table_mut(); let plugins = ensure_table(root, "plugins", /*implicit*/ true); let plugin = ensure_table(plugins, plugin_id, /*implicit*/ false); - plugin["enabled"] = value(enabled); + let mut replacement = value(enabled); + if let Some(existing) = plugin.get("enabled") { + preserve_decor(existing, &mut replacement); + } + plugin["enabled"] = replacement; + true } -fn clear_plugin(doc: &mut DocumentMut, plugin_id: &str) { +fn clear_plugin(doc: &mut DocumentMut, plugin_id: &str) -> bool { let root = doc.as_table_mut(); - if !root.contains_key("plugins") { - return; + let Some(item) = root.get_mut("plugins") else { + return false; + }; + match item { + TomlItem::Table(plugins) => plugins.remove(plugin_id).is_some(), + TomlItem::Value(value) => { + let Some(inline) = value.as_inline_table().cloned() else { + return false; + }; + *item = TomlItem::Table(table_from_inline(&inline, /*implicit*/ true)); + if let TomlItem::Table(plugins) = item { + return plugins.remove(plugin_id).is_some(); + } + false + } + _ => false, + } +} + +fn preserve_decor(existing: &TomlItem, replacement: &mut TomlItem) { + if let (TomlItem::Value(existing_value), TomlItem::Value(replacement_value)) = + (existing, replacement) + { + replacement_value + .decor_mut() + .clone_from(existing_value.decor()); } - let plugins = ensure_table(root, "plugins", /*implicit*/ true); - plugins.remove(plugin_id); } fn ensure_table<'a>(parent: &'a mut TomlTable, key: &str, implicit: bool) -> &'a mut TomlTable { diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index 59304863ae..c37a38be77 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -2,6 +2,8 @@ use super::*; use crate::McpServerToolConfig; use pretty_assertions::assert_eq; use std::collections::HashMap; +#[cfg(unix)] +use std::os::unix::fs::symlink; use std::time::SystemTime; use std::time::UNIX_EPOCH; @@ -83,6 +85,52 @@ approval_mode = "approve" Ok(()) } +#[cfg(unix)] +#[tokio::test] +async fn plugin_edits_write_through_symlink_chain() -> anyhow::Result<()> { + let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + let codex_home = std::env::temp_dir().join(format!( + "codex-config-plugin-edit-symlink-test-{}-{unique_suffix}", + std::process::id() + )); + let target_home = std::env::temp_dir().join(format!( + "codex-config-plugin-edit-symlink-target-{}-{unique_suffix}", + std::process::id() + )); + std::fs::create_dir_all(&codex_home)?; + std::fs::create_dir_all(&target_home)?; + let target_path = target_home.join(CONFIG_TOML_FILE); + let link_path = codex_home.join("config-link.toml"); + let config_path = codex_home.join(CONFIG_TOML_FILE); + std::fs::write( + &target_path, + r#"[plugins."linear@openai-curated"] +enabled = false +"#, + )?; + symlink(&target_path, &link_path)?; + symlink("config-link.toml", &config_path)?; + + ConfigEditsBuilder::new(&codex_home) + .set_plugin_enabled("linear@openai-curated", /*enabled*/ true) + .apply() + .await?; + + let meta = std::fs::symlink_metadata(&config_path)?; + assert!(meta.file_type().is_symlink()); + assert_eq!( + std::fs::read_to_string(target_path)?, + r#"[plugins."linear@openai-curated"] +enabled = true +"# + ); + + std::fs::remove_dir_all(&codex_home)?; + std::fs::remove_dir_all(&target_home)?; + + Ok(()) +} + #[tokio::test] async fn plugin_edits_set_and_clear_enabled_entries() -> anyhow::Result<()> { let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); @@ -119,3 +167,83 @@ enabled = true Ok(()) } + +#[tokio::test] +async fn plugin_clear_leaves_malformed_plugins_scalar_unchanged() -> anyhow::Result<()> { + let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + let codex_home = std::env::temp_dir().join(format!( + "codex-config-plugin-clear-malformed-test-{}-{unique_suffix}", + std::process::id() + )); + std::fs::create_dir_all(&codex_home)?; + let config_path = codex_home.join(CONFIG_TOML_FILE); + let config = r#"plugins = "bad" +"#; + std::fs::write(&config_path, config)?; + + ConfigEditsBuilder::new(&codex_home) + .clear_plugin("linear@openai-curated") + .apply() + .await?; + + assert_eq!(std::fs::read_to_string(&config_path)?, config); + + std::fs::remove_dir_all(&codex_home)?; + + Ok(()) +} + +#[tokio::test] +async fn plugin_clear_missing_entry_does_not_create_config() -> anyhow::Result<()> { + let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + let codex_home = std::env::temp_dir().join(format!( + "codex-config-plugin-clear-missing-test-{}-{unique_suffix}", + std::process::id() + )); + std::fs::create_dir_all(&codex_home)?; + let config_path = codex_home.join(CONFIG_TOML_FILE); + + ConfigEditsBuilder::new(&codex_home) + .clear_plugin("linear@openai-curated") + .apply() + .await?; + + assert!(!config_path.exists()); + + std::fs::remove_dir_all(&codex_home)?; + + Ok(()) +} + +#[tokio::test] +async fn plugin_enabled_update_preserves_existing_value_decor() -> anyhow::Result<()> { + let unique_suffix = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos(); + let codex_home = std::env::temp_dir().join(format!( + "codex-config-plugin-edit-decor-test-{}-{unique_suffix}", + std::process::id() + )); + std::fs::create_dir_all(&codex_home)?; + let config_path = codex_home.join(CONFIG_TOML_FILE); + std::fs::write( + &config_path, + r#"[plugins."linear@openai-curated"] +enabled = false # keep +"#, + )?; + + ConfigEditsBuilder::new(&codex_home) + .set_plugin_enabled("linear@openai-curated", /*enabled*/ true) + .apply() + .await?; + + assert_eq!( + std::fs::read_to_string(&config_path)?, + r#"[plugins."linear@openai-curated"] +enabled = true # keep +"# + ); + + std::fs::remove_dir_all(&codex_home)?; + + Ok(()) +} diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 9555b40d42..7c8e8fb760 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -14,6 +14,7 @@ workspace = true [dependencies] anyhow = { workspace = true } +codex-analytics = { workspace = true } codex-app-server-protocol = { workspace = true } codex-config = { workspace = true } codex-core-skills = { workspace = true } diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index c2c49ec072..db68963eae 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -1,7 +1,7 @@ -pub mod discoverable; +mod discoverable; pub mod installed_marketplaces; pub mod loader; -pub mod manager; +mod manager; pub mod manifest; pub mod marketplace; pub mod marketplace_add; @@ -39,5 +39,22 @@ pub const TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST: &[&str] = &[ "computer-use@openai-bundled", ]; -pub type LoadedPlugin = codex_plugin::LoadedPlugin; -pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; +pub use discoverable::list_tool_suggest_discoverable_plugins; +pub use manager::ConfiguredMarketplace; +pub use manager::ConfiguredMarketplaceListOutcome; +pub use manager::ConfiguredMarketplacePlugin; +pub use manager::PluginDetail; +pub use manager::PluginDetailsUnavailableReason; +pub use manager::PluginInstallError; +pub use manager::PluginInstallOutcome; +pub use manager::PluginInstallRequest; +pub use manager::PluginReadOutcome; +pub use manager::PluginReadRequest; +pub use manager::PluginRemoteSyncError; +pub use manager::PluginUninstallError; +pub use manager::PluginsManager; +pub use manager::RemotePluginSyncResult; + +#[cfg(test)] +pub(crate) type LoadedPlugin = codex_plugin::LoadedPlugin; +pub(crate) type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 24cf0c9ba4..cd71c06fe6 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -3,12 +3,14 @@ use crate::PluginLoadOutcome; use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack; use crate::loader::configured_curated_plugin_ids_from_codex_home; use crate::loader::curated_plugin_cache_version; +use crate::loader::installed_plugin_telemetry_metadata; use crate::loader::load_plugin_apps; use crate::loader::load_plugin_mcp_servers; use crate::loader::load_plugin_skills; use crate::loader::load_plugins_from_layer_stack; use crate::loader::log_plugin_load_errors; use crate::loader::materialize_marketplace_plugin_source; +use crate::loader::plugin_telemetry_metadata_from_root; use crate::loader::refresh_curated_plugin_cache; use crate::loader::refresh_non_curated_plugin_cache; use crate::loader::refresh_non_curated_plugin_cache_force_reinstall; @@ -44,6 +46,7 @@ use crate::startup_sync::sync_openai_plugins_repo; use crate::store::PluginInstallResult as StorePluginInstallResult; use crate::store::PluginStore; use crate::store::PluginStoreError; +use codex_analytics::AnalyticsEventsClient; use codex_config::ConfigEditsBuilder; use codex_config::ConfigLayerStack; use codex_config::types::PluginConfig; @@ -285,6 +288,14 @@ pub enum PluginRemoteSyncError { #[error("duplicate remote plugin `{plugin_name}` in sync response")] DuplicateRemotePlugin { plugin_name: String }, + #[error( + "remote plugin `{plugin_name}` was not found in local marketplace `{marketplace_name}`" + )] + UnknownRemotePlugin { + plugin_name: String, + marketplace_name: String, + }, + #[error("{0}")] InvalidPluginId(#[from] PluginIdError), @@ -353,6 +364,7 @@ pub struct PluginsManager { remote_installed_plugins_cache_refresh_state: RwLock, remote_sync_lock: Semaphore, restriction_product: Option, + analytics_events_client: RwLock>, } #[derive(Clone)] @@ -394,9 +406,18 @@ impl PluginsManager { ), remote_sync_lock: Semaphore::new(/*permits*/ 1), restriction_product, + analytics_events_client: RwLock::new(None), } } + pub fn set_analytics_events_client(&self, analytics_events_client: AnalyticsEventsClient) { + let mut stored_client = match self.analytics_events_client.write() { + Ok(client_guard) => client_guard, + Err(err) => err.into_inner(), + }; + *stored_client = Some(analytics_events_client); + } + fn restriction_product_matches(&self, products: Option<&[Product]>) -> bool { match products { None => true, @@ -572,7 +593,7 @@ impl PluginsManager { } #[doc(hidden)] - pub fn write_remote_installed_plugins_cache( + pub(crate) fn write_remote_installed_plugins_cache( &self, plugins: Vec, ) -> bool { @@ -821,6 +842,23 @@ impl PluginsManager { .await .map_err(PluginInstallError::join)??; + ConfigEditsBuilder::new(&self.codex_home) + .set_plugin_enabled(&result.plugin_id.as_key(), /*enabled*/ true) + .apply() + .await + .map_err(anyhow::Error::from)?; + + let analytics_events_client = match self.analytics_events_client.read() { + Ok(client) => client.clone(), + Err(err) => err.into_inner().clone(), + }; + if let Some(analytics_events_client) = analytics_events_client { + analytics_events_client.track_plugin_installed( + plugin_telemetry_metadata_from_root(&result.plugin_id, &result.installed_path) + .await, + ); + } + Ok(PluginInstallOutcome { plugin_id: result.plugin_id, plugin_version: result.plugin_version, @@ -856,12 +894,33 @@ impl PluginsManager { } async fn uninstall_plugin_id(&self, plugin_id: PluginId) -> Result<(), PluginUninstallError> { + let plugin_telemetry = if self.store.active_plugin_root(&plugin_id).is_some() { + Some(installed_plugin_telemetry_metadata(self.codex_home.as_path(), &plugin_id).await) + } else { + None + }; let store = self.store.clone(); let plugin_id_for_store = plugin_id.clone(); tokio::task::spawn_blocking(move || store.uninstall(&plugin_id_for_store)) .await .map_err(PluginUninstallError::join)??; + ConfigEditsBuilder::new(&self.codex_home) + .clear_plugin(&plugin_id.as_key()) + .apply() + .await + .map_err(anyhow::Error::from)?; + + let analytics_events_client = match self.analytics_events_client.read() { + Ok(client) => client.clone(), + Err(err) => err.into_inner().clone(), + }; + if let Some(plugin_telemetry) = plugin_telemetry + && let Some(analytics_events_client) = analytics_events_client + { + analytics_events_client.track_plugin_uninstalled(plugin_telemetry); + } + Ok(()) } @@ -1189,7 +1248,7 @@ impl PluginsManager { }) } - pub async fn read_plugin_detail_for_marketplace_plugin( + pub(crate) async fn read_plugin_detail_for_marketplace_plugin( &self, config_layer_stack: &ConfigLayerStack, marketplace_name: &str, @@ -1813,6 +1872,9 @@ pub enum PluginInstallError { #[error("{0}")] Store(#[from] PluginStoreError), + #[error("{0}")] + Config(#[from] anyhow::Error), + #[error("failed to join plugin install task: {0}")] Join(#[from] tokio::task::JoinError), } @@ -1847,6 +1909,9 @@ pub enum PluginUninstallError { #[error("{0}")] Store(#[from] PluginStoreError), + #[error("{0}")] + Config(#[from] anyhow::Error), + #[error("failed to join plugin uninstall task: {0}")] Join(#[from] tokio::task::JoinError), } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 88d43664a2..aa40c98aa0 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -582,8 +582,6 @@ remote_plugin = true id: "plugins~Plugin_linear".to_string(), name: "linear".to_string(), enabled: true, - release_version: Some("local".to_string()), - bundle_download_url: Some("https://example.com/linear.tar.gz".to_string()), }]); let outcome = manager.plugins_for_test_config(&config).await; @@ -617,8 +615,6 @@ remote_plugin = true id: "plugins~Plugin_linear".to_string(), name: "linear".to_string(), enabled: true, - release_version: Some("local".to_string()), - bundle_download_url: Some("https://example.com/linear.tar.gz".to_string()), }]); let enabled_outcome = manager.plugins_for_test_config(&config).await; @@ -653,92 +649,12 @@ remote_plugin = true id: "plugins~Plugin_linear".to_string(), name: "linear".to_string(), enabled: true, - release_version: Some("local".to_string()), - bundle_download_url: Some("https://example.com/linear.tar.gz".to_string()), }]); let outcome = manager.plugins_for_test_config(&config).await; assert_eq!(outcome, PluginLoadOutcome::default()); } -#[tokio::test] -async fn remote_installed_cache_uses_available_local_cache_without_version_gate() { - let codex_home = TempDir::new().unwrap(); - let linear_root = codex_home - .path() - .join("plugins/cache/chatgpt-global/linear/1.0.0"); - write_plugin( - &codex_home.path().join("plugins/cache/chatgpt-global"), - "linear/1.0.0", - "linear", - ); - write_file( - &codex_home.path().join(CONFIG_TOML_FILE), - r#"[features] -plugins = true -remote_plugin = true -"#, - ); - - 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![RemoteInstalledPlugin { - marketplace_name: "chatgpt-global".to_string(), - id: "plugins~Plugin_linear".to_string(), - name: "linear".to_string(), - enabled: true, - release_version: Some("2.0.0".to_string()), - bundle_download_url: Some("https://example.com/linear.tar.gz".to_string()), - }]); - - let outcome = manager.plugins_for_test_config(&config).await; - assert_eq!( - outcome.effective_skill_roots(), - vec![AbsolutePathBuf::try_from(linear_root.join("skills")).unwrap()] - ); - assert_eq!(outcome.plugins().len(), 1); - assert_eq!(outcome.plugins()[0].config_name, "linear@chatgpt-global"); -} - -#[tokio::test] -async fn remote_installed_cache_uses_available_local_cache_without_release_metadata() { - let codex_home = TempDir::new().unwrap(); - let linear_root = codex_home - .path() - .join("plugins/cache/chatgpt-global/linear/local"); - write_plugin( - &codex_home.path().join("plugins/cache/chatgpt-global"), - "linear/local", - "linear", - ); - write_file( - &codex_home.path().join(CONFIG_TOML_FILE), - r#"[features] -plugins = true -remote_plugin = true -"#, - ); - - 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![RemoteInstalledPlugin { - marketplace_name: "chatgpt-global".to_string(), - id: "plugins~Plugin_linear".to_string(), - name: "linear".to_string(), - enabled: true, - release_version: None, - bundle_download_url: None, - }]); - - let outcome = manager.plugins_for_test_config(&config).await; - assert_eq!( - outcome.effective_skill_roots(), - vec![AbsolutePathBuf::try_from(linear_root.join("skills")).unwrap()] - ); - assert_eq!(outcome.plugins().len(), 1); - assert_eq!(outcome.plugins()[0].config_name, "linear@chatgpt-global"); -} - async fn wait_for_counter(counter: &AtomicUsize, expected: usize) { tokio::time::timeout(Duration::from_secs(5), async { loop { @@ -796,8 +712,6 @@ async fn remote_installed_plugins_cache_refresh_clears_stale_cache_when_auth_is_ id: "plugins~Plugin_linear".to_string(), name: "linear".to_string(), enabled: true, - release_version: Some("local".to_string()), - bundle_download_url: Some("https://example.com/linear.tar.gz".to_string()), }]); let notification_count = Arc::new(AtomicUsize::new(0)); let notification_count_for_callback = Arc::clone(¬ification_count); @@ -1611,7 +1525,7 @@ async fn load_plugins_rejects_invalid_plugin_keys() { } #[tokio::test] -async fn install_plugin_materializes_relative_path_plugin() { +async fn install_plugin_updates_config_with_relative_path_and_plugin_key() { let tmp = tempfile::tempdir().unwrap(); let repo_root = tmp.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).unwrap(); @@ -1658,6 +1572,9 @@ async fn install_plugin_materializes_relative_path_plugin() { auth_policy: MarketplacePluginAuthPolicy::OnUse, } ); + let config = fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(); + assert!(config.contains(r#"[plugins."sample-plugin@debug"]"#)); + assert!(config.contains("enabled = true")); } #[tokio::test] @@ -1858,7 +1775,7 @@ async fn install_plugin_supports_relative_git_subdir_marketplace_sources() { } #[tokio::test] -async fn uninstall_plugin_removes_cache() { +async fn uninstall_plugin_removes_cache_and_config_entry() { let tmp = tempfile::tempdir().unwrap(); write_plugin( &tmp.path().join("plugins/cache/debug"), @@ -1890,6 +1807,8 @@ enabled = true .join("plugins/cache/debug/sample-plugin") .exists() ); + let config = fs::read_to_string(tmp.path().join(CONFIG_TOML_FILE)).unwrap(); + assert!(!config.contains(r#"[plugins."sample-plugin@debug"]"#)); } #[tokio::test] diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index b4cb19783e..42ff572a81 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -50,8 +50,6 @@ pub struct RemoteInstalledPlugin { pub id: String, pub name: String, pub enabled: bool, - pub release_version: Option, - pub bundle_download_url: Option, } #[derive(Debug, Clone, PartialEq)] @@ -673,8 +671,6 @@ fn remote_installed_plugin_to_info( id: plugin.id.clone(), name: plugin.name.clone(), enabled: installed_plugin.enabled, - release_version: plugin.release.version.clone(), - bundle_download_url: plugin.release.bundle_download_url.clone(), } } @@ -831,7 +827,6 @@ async fn get_remote_plugin_installed_page( let client = build_reqwest_client(); let mut request = authenticated_request(client.get(&url), auth)?; request = request.query(&[("scope", scope.api_value())]); - request = request.query(&[("includeDownloadUrls", true)]); if let Some(page_token) = page_token { request = request.query(&[("pageToken", page_token)]); } diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index e7aaf7a46c..90851ee074 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -14,7 +14,7 @@ pub use codex_app_server_protocol::AppInfo; pub use codex_app_server_protocol::AppMetadata; use codex_connectors::AllConnectorsCacheKey; use codex_connectors::DirectoryListResponse; -use codex_core_plugins::discoverable::list_tool_suggest_discoverable_plugins; +use codex_core_plugins::list_tool_suggest_discoverable_plugins; use codex_exec_server::EnvironmentManager; use codex_exec_server::EnvironmentManagerArgs; use codex_exec_server::ExecServerRuntimePaths; diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 84e9306e64..44f1685a82 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -4,20 +4,22 @@ mod render; #[cfg(test)] pub(crate) mod test_support; -pub use codex_core_plugins::LoadedPlugin; -pub use codex_core_plugins::PluginLoadOutcome; -pub use codex_core_plugins::manager::ConfiguredMarketplace; -pub use codex_core_plugins::manager::ConfiguredMarketplaceListOutcome; -pub use codex_core_plugins::manager::ConfiguredMarketplacePlugin; -pub use codex_core_plugins::manager::PluginDetail; -pub use codex_core_plugins::manager::PluginDetailsUnavailableReason; -pub use codex_core_plugins::manager::PluginInstallError; -pub use codex_core_plugins::manager::PluginInstallOutcome; -pub use codex_core_plugins::manager::PluginInstallRequest; -pub use codex_core_plugins::manager::PluginReadOutcome; -pub use codex_core_plugins::manager::PluginReadRequest; -pub use codex_core_plugins::manager::PluginUninstallError; -pub use codex_core_plugins::manager::PluginsManager; +use codex_config::types::McpServerConfig; + +pub use codex_core_plugins::ConfiguredMarketplace; +pub use codex_core_plugins::ConfiguredMarketplaceListOutcome; +pub use codex_core_plugins::ConfiguredMarketplacePlugin; +pub use codex_core_plugins::PluginDetail; +pub use codex_core_plugins::PluginDetailsUnavailableReason; +pub use codex_core_plugins::PluginInstallError; +pub use codex_core_plugins::PluginInstallOutcome; +pub use codex_core_plugins::PluginInstallRequest; +pub use codex_core_plugins::PluginReadOutcome; +pub use codex_core_plugins::PluginReadRequest; +pub use codex_core_plugins::PluginRemoteSyncError; +pub use codex_core_plugins::PluginUninstallError; +pub use codex_core_plugins::PluginsManager; +pub use codex_core_plugins::RemotePluginSyncResult; pub use codex_core_plugins::marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError; pub use codex_core_plugins::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome; pub use codex_plugin::AppConnectorId; @@ -28,6 +30,9 @@ pub use codex_plugin::PluginIdError; pub use codex_plugin::PluginTelemetryMetadata; pub use codex_plugin::validate_plugin_segment; +pub type LoadedPlugin = codex_plugin::LoadedPlugin; +pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; + pub(crate) use injection::build_plugin_injections; pub(crate) use render::render_explicit_plugin_instructions;