diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index a9a1d90e7..091a85e11 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -3504,10 +3504,11 @@ async fn reducer_ingests_plugin_install_failed_fact() { async fn reducer_ingests_plugin_install_failed_fact_without_detail() { let mut reducer = AnalyticsReducer::default(); let mut events = Vec::new(); - let mut plugin = PluginTelemetryMetadata::from_plugin_id( - &PluginId::parse("unknown@openai-curated-remote").expect("valid plugin id"), - ); - plugin.remote_plugin_id = Some("plugins~Plugin_00000000000000000000000000000000".to_string()); + let plugin = PluginTelemetryMetadata { + plugin_id: PluginId::parse("unknown@openai-curated-remote").expect("valid plugin id"), + remote_plugin_id: Some("plugins~Plugin_00000000000000000000000000000000".to_string()), + capability_summary: None, + }; reducer .ingest( diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index c8bd48c47..2972d05f0 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -337,7 +337,6 @@ use codex_core_plugins::PluginUninstallError as CorePluginUninstallError; use codex_core_plugins::PluginsManager; use codex_core_plugins::loader::load_plugin_apps; use codex_core_plugins::loader::load_plugin_mcp_servers; -use codex_core_plugins::loader::plugin_telemetry_metadata_from_root; use codex_core_plugins::manifest::PluginManifestInterface; use codex_core_plugins::marketplace::MarketplaceError; use codex_core_plugins::marketplace::MarketplacePluginSource; diff --git a/codex-rs/app-server/src/request_processors/config_processor.rs b/codex-rs/app-server/src/request_processors/config_processor.rs index 15c3b7c86..50bba5400 100644 --- a/codex-rs/app-server/src/request_processors/config_processor.rs +++ b/codex-rs/app-server/src/request_processors/config_processor.rs @@ -295,15 +295,14 @@ impl ConfigRequestProcessor { &self, pending_changes: std::collections::BTreeMap, ) { + let plugins_manager = self.thread_manager.plugins_manager(); for (plugin_id, enabled) in pending_changes { let Ok(plugin_id) = PluginId::parse(&plugin_id) else { continue; }; - let metadata = codex_core_plugins::loader::installed_plugin_telemetry_metadata( - self.config_manager.codex_home(), - &plugin_id, - ) - .await; + let metadata = plugins_manager + .telemetry_metadata_for_installed_plugin(&plugin_id) + .await; if enabled { self.analytics_events_client.track_plugin_enabled(metadata); } else { diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index e9bf04476..1e4716a88 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -23,7 +23,6 @@ use codex_mcp::McpOAuthLoginSupport; use codex_mcp::oauth_login_support; use codex_mcp::should_retry_without_scopes; use codex_plugin::PluginId; -use codex_plugin::PluginTelemetryMetadata; use codex_rmcp_client::perform_oauth_login_silent; #[derive(Clone)] @@ -1620,9 +1619,14 @@ impl PluginRequestProcessor { Some(self.effective_plugins_changed_callback()), ); - let mut plugin_metadata = - plugin_telemetry_metadata_from_root(&result.plugin_id, &result.installed_path).await; - plugin_metadata.remote_plugin_id = Some(remote_plugin_id.clone()); + let plugin_metadata = self + .thread_manager + .plugins_manager() + .telemetry_metadata_for_installed_plugin_with_remote_id( + &result.plugin_id, + &remote_plugin_id, + ) + .await; self.analytics_events_client .track_plugin_installed(plugin_metadata); @@ -1713,8 +1717,10 @@ impl PluginRequestProcessor { else { return; }; - let mut plugin = PluginTelemetryMetadata::from_plugin_id(&plugin_id); - plugin.remote_plugin_id = Some(remote_plugin_id.to_string()); + let plugin = self + .thread_manager + .plugins_manager() + .telemetry_metadata_for_plugin_id_with_remote_id(&plugin_id, remote_plugin_id); self.analytics_events_client .track_plugin_install_failed(plugin, error_type.to_string()); } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 54eb37168..2a8d44fc9 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -38,7 +38,6 @@ use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_plugin::PluginIdError; -use codex_plugin::PluginTelemetryMetadata; use codex_plugin::app_connector_ids_from_declarations; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; @@ -1164,13 +1163,11 @@ fn cleaned_app_category(category: Option) -> Option { .filter(|category| !category.is_empty()) } -pub async fn plugin_telemetry_metadata_from_root( +pub async fn plugin_capability_summary_from_root( plugin_id: &PluginId, plugin_root: &AbsolutePathBuf, -) -> PluginTelemetryMetadata { - let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { - return PluginTelemetryMetadata::from_plugin_id(plugin_id); - }; +) -> Option { + let manifest = load_plugin_manifest(plugin_root.as_path())?; let manifest_paths = &manifest.paths; let has_skills = !plugin_skill_roots(plugin_root, manifest_paths).is_empty(); @@ -1192,18 +1189,14 @@ pub async fn plugin_telemetry_metadata_from_root( .await; let app_connector_ids = app_connector_ids_from_declarations(&app_declarations); - PluginTelemetryMetadata { - plugin_id: plugin_id.clone(), - remote_plugin_id: None, - capability_summary: Some(PluginCapabilitySummary { - config_name: plugin_id.as_key(), - display_name: plugin_id.plugin_name.clone(), - description: None, - has_skills, - mcp_server_names, - app_connector_ids, - }), - } + Some(PluginCapabilitySummary { + config_name: plugin_id.as_key(), + display_name: plugin_id.plugin_name.clone(), + description: None, + has_skills, + mcp_server_names, + app_connector_ids, + }) } pub async fn load_plugin_mcp_servers( @@ -1279,24 +1272,6 @@ pub(crate) async fn load_plugin_mcp_servers_from_manifest( mcp_servers } -pub async fn installed_plugin_telemetry_metadata( - codex_home: &Path, - plugin_id: &PluginId, -) -> PluginTelemetryMetadata { - let store = match PluginStore::try_new(codex_home.to_path_buf()) { - Ok(store) => store, - Err(err) => { - warn!("failed to resolve plugin cache root: {err}"); - return PluginTelemetryMetadata::from_plugin_id(plugin_id); - } - }; - let Some(plugin_root) = store.active_plugin_root(plugin_id) else { - return PluginTelemetryMetadata::from_plugin_id(plugin_id); - }; - - plugin_telemetry_metadata_from_root(plugin_id, &plugin_root).await -} - async fn load_mcp_servers_from_file( plugin_root: &Path, mcp_config_path: &AbsolutePathBuf, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index b502106ea..19b48b2c0 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -6,7 +6,6 @@ use crate::is_openai_curated_marketplace_name; use crate::loader::PluginHookLoadOutcome; 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_from_manifest; use crate::loader::load_plugin_hooks; use crate::loader::load_plugin_hooks_from_layer_stack; @@ -15,7 +14,7 @@ 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::plugin_capability_summary_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; @@ -709,6 +708,66 @@ impl PluginsManager { remote_installed_plugins_to_config(plugins, &self.store) } + pub async fn telemetry_metadata_for_installed_plugin( + &self, + plugin_id: &PluginId, + ) -> PluginTelemetryMetadata { + let mut metadata = self.telemetry_metadata_for_plugin_id(plugin_id); + metadata.capability_summary = match self.store.active_plugin_root(plugin_id) { + Some(plugin_root) => plugin_capability_summary_from_root(plugin_id, &plugin_root).await, + None => None, + }; + metadata + } + + pub async fn telemetry_metadata_for_installed_plugin_with_remote_id( + &self, + plugin_id: &PluginId, + remote_plugin_id: &str, + ) -> PluginTelemetryMetadata { + let mut metadata = + self.telemetry_metadata_for_plugin_id_with_remote_id(plugin_id, remote_plugin_id); + metadata.capability_summary = match self.store.active_plugin_root(plugin_id) { + Some(plugin_root) => plugin_capability_summary_from_root(plugin_id, &plugin_root).await, + None => None, + }; + metadata + } + + pub fn telemetry_metadata_for_plugin_id( + &self, + plugin_id: &PluginId, + ) -> PluginTelemetryMetadata { + PluginTelemetryMetadata { + plugin_id: plugin_id.clone(), + remote_plugin_id: None, + capability_summary: None, + } + } + + pub fn telemetry_metadata_for_plugin_id_with_remote_id( + &self, + plugin_id: &PluginId, + remote_plugin_id: &str, + ) -> PluginTelemetryMetadata { + PluginTelemetryMetadata { + remote_plugin_id: Some(remote_plugin_id.to_string()), + ..self.telemetry_metadata_for_plugin_id(plugin_id) + } + } + + pub fn telemetry_metadata_for_capability_summary( + &self, + summary: &PluginCapabilitySummary, + ) -> Option { + let plugin_id = PluginId::parse(&summary.config_name).ok()?; + Some(PluginTelemetryMetadata { + plugin_id, + remote_plugin_id: None, + capability_summary: Some(summary.clone()), + }) + } + pub fn build_remote_installed_plugin_marketplaces_from_cache( &self, visible_marketplaces: &[&str], @@ -1281,7 +1340,7 @@ impl PluginsManager { }; if let Some(analytics_events_client) = analytics_events_client { analytics_events_client.track_plugin_install_failed( - PluginTelemetryMetadata::from_plugin_id(plugin_id), + self.telemetry_metadata_for_plugin_id(plugin_id), error_type.to_string(), ); } @@ -1351,7 +1410,7 @@ impl PluginsManager { }; 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) + self.telemetry_metadata_for_installed_plugin(&result.plugin_id) .await, ); } @@ -1392,7 +1451,10 @@ 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) + Some( + self.telemetry_metadata_for_installed_plugin(&plugin_id) + .await, + ) } else { None }; diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 4f448ff16..50a5fe3e7 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -845,6 +845,81 @@ remote_plugin = true assert_eq!(outcome, PluginLoadOutcome::default()); } +#[tokio::test] +async fn installed_plugin_telemetry_metadata_collects_capabilities() { + let codex_home = TempDir::new().unwrap(); + write_cached_plugin(codex_home.path(), "test", "sample"); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugin_id = PluginId::parse("sample@test").expect("plugin id should parse"); + + let metadata = manager + .telemetry_metadata_for_installed_plugin(&plugin_id) + .await; + + assert_eq!( + metadata, + PluginTelemetryMetadata { + plugin_id, + remote_plugin_id: None, + capability_summary: Some(PluginCapabilitySummary { + config_name: "sample@test".to_string(), + display_name: "sample".to_string(), + description: None, + has_skills: true, + mcp_server_names: Vec::new(), + app_connector_ids: Vec::new(), + }), + } + ); +} + +#[tokio::test] +async fn installed_plugin_telemetry_metadata_accepts_authoritative_remote_identity() { + let codex_home = TempDir::new().unwrap(); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let plugin_id = + PluginId::parse("linear@openai-curated-remote").expect("plugin id should parse"); + + let metadata = manager + .telemetry_metadata_for_installed_plugin_with_remote_id(&plugin_id, "plugins~Plugin_linear") + .await; + + assert_eq!( + metadata, + PluginTelemetryMetadata { + plugin_id, + remote_plugin_id: Some("plugins~Plugin_linear".to_string()), + capability_summary: None, + } + ); +} + +#[test] +fn capability_summary_telemetry_metadata_uses_local_identity() { + let codex_home = TempDir::new().unwrap(); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let summary = PluginCapabilitySummary { + config_name: "linear@openai-curated-remote".to_string(), + display_name: "Linear".to_string(), + description: Some("Track work".to_string()), + has_skills: true, + mcp_server_names: vec!["linear".to_string()], + app_connector_ids: vec![AppConnectorId("linear-app".to_string())], + }; + + let metadata = manager.telemetry_metadata_for_capability_summary(&summary); + + assert_eq!( + metadata, + Some(PluginTelemetryMetadata { + plugin_id: PluginId::parse("linear@openai-curated-remote") + .expect("plugin id should parse"), + remote_plugin_id: None, + capability_summary: Some(summary), + }) + ); +} + #[tokio::test] async fn remote_installed_cache_prefers_local_curated_conflicts_when_remote_plugin_disabled() { let codex_home = TempDir::new().unwrap(); @@ -1154,14 +1229,14 @@ async fn plugin_telemetry_metadata_uses_default_mcp_config_path() { }"#, ); - let metadata = plugin_telemetry_metadata_from_root( + let summary = plugin_capability_summary_from_root( &PluginId::parse("sample@test").expect("plugin id should parse"), &plugin_root.abs(), ) .await; assert_eq!( - metadata.capability_summary, + summary, Some(PluginCapabilitySummary { config_name: "sample@test".to_string(), display_name: "sample".to_string(), @@ -1174,7 +1249,7 @@ async fn plugin_telemetry_metadata_uses_default_mcp_config_path() { } #[tokio::test] -async fn plugin_telemetry_metadata_uses_manifest_mcp_server_objects() { +async fn plugin_capability_summary_uses_manifest_mcp_server_objects() { let codex_home = TempDir::new().unwrap(); let plugin_root = codex_home .path() @@ -1195,14 +1270,14 @@ async fn plugin_telemetry_metadata_uses_manifest_mcp_server_objects() { }"#, ); - let metadata = plugin_telemetry_metadata_from_root( + let summary = plugin_capability_summary_from_root( &PluginId::parse("counter-sample@test").expect("plugin id should parse"), &plugin_root.abs(), ) .await; assert_eq!( - metadata.capability_summary, + summary, Some(PluginCapabilitySummary { config_name: "counter-sample@test".to_string(), display_name: "counter-sample".to_string(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 9b9c7cc66..81f9f5235 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -656,13 +656,16 @@ async fn build_skills_and_plugins( sess.services .analytics_events_client .track_app_mentioned(tracking.clone(), mentioned_app_invocations); - for plugin in mentioned_plugins - .iter() - .filter_map(crate::plugins::PluginCapabilitySummary::telemetry_metadata) - { - sess.services - .analytics_events_client - .track_plugin_used(tracking.clone(), plugin); + for summary in &mentioned_plugins { + if let Some(plugin) = sess + .services + .plugins_manager + .telemetry_metadata_for_capability_summary(summary) + { + sess.services + .analytics_events_client + .track_plugin_used(tracking.clone(), plugin); + } } let mut injection_items: Vec = match injected_host_skill_prompts { diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 7daa78bc0..1a4eb23af 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -76,25 +76,3 @@ pub struct PluginTelemetryMetadata { pub remote_plugin_id: Option, pub capability_summary: Option, } - -impl PluginTelemetryMetadata { - pub fn from_plugin_id(plugin_id: &PluginId) -> Self { - Self { - plugin_id: plugin_id.clone(), - remote_plugin_id: None, - capability_summary: None, - } - } -} - -impl PluginCapabilitySummary { - pub fn telemetry_metadata(&self) -> Option { - PluginId::parse(&self.config_name) - .ok() - .map(|plugin_id| PluginTelemetryMetadata { - plugin_id, - remote_plugin_id: None, - capability_summary: Some(self.clone()), - }) - } -}