From ff50b47dceb151cb08d8ad46e0e35bce8e7abcde Mon Sep 17 00:00:00 2001 From: jameswt-oai Date: Tue, 23 Jun 2026 12:27:14 -0700 Subject: [PATCH] Separate local and remote plugin analytics IDs (#29495) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Plugin analytics overloaded `plugin_id`: most events used the Codex `@` identity, while remote install events used the backend plugin ID. That makes the same field change meaning across event types and complicates downstream identity resolution. This change makes the contract unambiguous: - `plugin_id`: the local Codex `@` identity, when resolved - `remote_plugin_id`: the backend plugin identity, when available For a remote install failure that happens before plugin details resolve, `plugin_id` is `null` and `remote_plugin_id` remains populated. ## What changed All six plugin analytics events use the same identity contract: - `codex_plugin_installed` - `codex_plugin_install_failed` - `codex_plugin_uninstalled` - `codex_plugin_enabled` - `codex_plugin_disabled` - `codex_plugin_used` Remote identity is resolved from the current installed-plugin snapshot first, with persisted install metadata as fallback. The telemetry metadata type keeps local identity optional for failures that occur before remote details are available. The app-server test client's manual analytics smokes now find remote mutation events through `remote_plugin_id` and validate that `plugin_id` remains local. ## Remote uninstall Resolve and capture telemetry metadata before removing the local plugin cache, then emit `codex_plugin_uninstalled` after the backend confirms success. The event is also emitted when backend uninstall succeeds but local cache cleanup reports `CacheRemove`. If a concurrent remote-cache refresh removes the local bundle before telemetry capture, the already-fetched remote plugin detail supplies fallback capability metadata. ## Validation - `just test -p codex-analytics` — 82 passed - `just test -p codex-core-plugins` — 271 passed - `just test -p codex-app-server-test-client` — 5 passed - `just test -p codex-plugin` — 3 passed - `just test -p codex-app-server plugin_install` — 37 passed - `just test -p codex-app-server plugin_uninstall` — 10 passed The production app-server install/uninstall flow was also exercised against `plugins~Plugin_f1b845ac33888191ac156169c58733c2` (`build-ios-apps@openai-curated-remote`), and the plugin's original uninstalled state was restored. --- .../analytics/src/analytics_client_tests.rs | 36 ++++-- codex-rs/analytics/src/client.rs | 11 +- codex-rs/analytics/src/events.rs | 12 +- codex-rs/app-server-test-client/README.md | 23 ++-- .../src/plugin_analytics_capture.rs | 35 +++--- .../src/plugin_analytics_capture_tests.rs | 14 ++- .../src/plugin_analytics_mutation_smoke.rs | 8 +- .../src/plugin_analytics_smoke.rs | 8 +- .../src/request_processors/plugins.rs | 56 ++++++++-- .../tests/suite/v2/plugin_install.rs | 18 +-- .../tests/suite/v2/plugin_uninstall.rs | 63 ++++++++++- codex-rs/core-plugins/src/manager.rs | 39 ++++++- codex-rs/core-plugins/src/manager_tests.rs | 105 +++++++++++++++++- codex-rs/core-plugins/src/remote.rs | 83 ++++++++++++-- codex-rs/plugin/src/lib.rs | 7 +- 15 files changed, 427 insertions(+), 91 deletions(-) diff --git a/codex-rs/analytics/src/analytics_client_tests.rs b/codex-rs/analytics/src/analytics_client_tests.rs index cef51d4f1..38d953961 100644 --- a/codex-rs/analytics/src/analytics_client_tests.rs +++ b/codex-rs/analytics/src/analytics_client_tests.rs @@ -3027,6 +3027,7 @@ fn plugin_used_event_serializes_expected_shape() { "event_type": "codex_plugin_used", "event_params": { "plugin_id": "sample@test", + "remote_plugin_id": null, "plugin_name": "sample", "marketplace_name": "test", "has_skills": true, @@ -3057,6 +3058,7 @@ fn plugin_management_event_serializes_expected_shape() { "event_type": "codex_plugin_installed", "event_params": { "plugin_id": "sample@test", + "remote_plugin_id": null, "plugin_name": "sample", "marketplace_name": "test", "has_skills": true, @@ -3086,6 +3088,7 @@ fn plugin_install_failed_event_serializes_expected_shape() { "event_type": "codex_plugin_install_failed", "event_params": { "plugin_id": "sample@test", + "remote_plugin_id": null, "plugin_name": "sample", "marketplace_name": "test", "has_skills": true, @@ -3099,7 +3102,7 @@ fn plugin_install_failed_event_serializes_expected_shape() { } #[test] -fn plugin_management_event_can_use_remote_plugin_id_override() { +fn plugin_management_event_keeps_plugin_id_local_when_remote_id_exists() { let mut plugin = sample_plugin_metadata(); plugin.remote_plugin_id = Some("plugins~Plugin_remote".to_string()); let event = TrackEventRequest::PluginInstalled(CodexPluginEventRequest { @@ -3110,11 +3113,21 @@ fn plugin_management_event_can_use_remote_plugin_id_override() { let payload = serde_json::to_value(&event).expect("serialize plugin installed event"); assert_eq!( - payload["event_params"]["plugin_id"], - "plugins~Plugin_remote" + payload, + json!({ + "event_type": "codex_plugin_installed", + "event_params": { + "plugin_id": "sample@test", + "remote_plugin_id": "plugins~Plugin_remote", + "plugin_name": "sample", + "marketplace_name": "test", + "has_skills": true, + "mcp_server_count": 2, + "connector_ids": ["calendar", "drive"], + "product_client_id": originator().value + } + }) ); - assert_eq!(payload["event_params"]["plugin_name"], "sample"); - assert_eq!(payload["event_params"]["marketplace_name"], "test"); } #[test] @@ -3454,6 +3467,7 @@ async fn reducer_ingests_plugin_state_changed_fact() { "event_type": "codex_plugin_disabled", "event_params": { "plugin_id": "sample@test", + "remote_plugin_id": null, "plugin_name": "sample", "marketplace_name": "test", "has_skills": true, @@ -3489,6 +3503,7 @@ async fn reducer_ingests_plugin_install_failed_fact() { "event_type": "codex_plugin_install_failed", "event_params": { "plugin_id": "sample@test", + "remote_plugin_id": null, "plugin_name": "sample", "marketplace_name": "test", "has_skills": true, @@ -3506,7 +3521,7 @@ async fn reducer_ingests_plugin_install_failed_fact_without_detail() { let mut reducer = AnalyticsReducer::default(); let mut events = Vec::new(); let plugin = PluginTelemetryMetadata { - plugin_id: PluginId::parse("unknown@openai-curated-remote").expect("valid plugin id"), + plugin_id: None, remote_plugin_id: Some("plugins~Plugin_00000000000000000000000000000000".to_string()), capability_summary: None, }; @@ -3529,9 +3544,10 @@ async fn reducer_ingests_plugin_install_failed_fact_without_detail() { json!([{ "event_type": "codex_plugin_install_failed", "event_params": { - "plugin_id": "plugins~Plugin_00000000000000000000000000000000", - "plugin_name": "unknown", - "marketplace_name": "openai-curated-remote", + "plugin_id": null, + "remote_plugin_id": "plugins~Plugin_00000000000000000000000000000000", + "plugin_name": null, + "marketplace_name": null, "has_skills": null, "mcp_server_count": null, "connector_ids": null, @@ -4570,7 +4586,7 @@ async fn turn_completed_without_started_notification_emits_null_started_at() { fn sample_plugin_metadata() -> PluginTelemetryMetadata { PluginTelemetryMetadata { - plugin_id: PluginId::parse("sample@test").expect("valid plugin id"), + plugin_id: Some(PluginId::parse("sample@test").expect("valid plugin id")), remote_plugin_id: None, capability_summary: Some(PluginCapabilitySummary { config_name: "sample@test".to_string(), diff --git a/codex-rs/analytics/src/client.rs b/codex-rs/analytics/src/client.rs index 24ff523c9..314f0028a 100644 --- a/codex-rs/analytics/src/client.rs +++ b/codex-rs/analytics/src/client.rs @@ -38,6 +38,7 @@ use codex_app_server_protocol::ServerResponse; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::default_client::create_client; +use codex_plugin::PluginId; use codex_plugin::PluginTelemetryMetadata; use codex_protocol::request_permissions::RequestPermissionsResponse; use std::collections::HashSet; @@ -173,7 +174,15 @@ impl AnalyticsEventsQueue { if emitted.len() >= ANALYTICS_EVENT_DEDUPE_MAX_KEYS { emitted.clear(); } - emitted.insert((tracking.turn_id.clone(), plugin.plugin_id.as_key())) + let Some(plugin_id) = plugin + .plugin_id + .as_ref() + .map(PluginId::as_key) + .or_else(|| plugin.remote_plugin_id.clone()) + else { + return true; + }; + emitted.insert((tracking.turn_id.clone(), plugin_id)) } } diff --git a/codex-rs/analytics/src/events.rs b/codex-rs/analytics/src/events.rs index 29d896156..2fe465c9b 100644 --- a/codex-rs/analytics/src/events.rs +++ b/codex-rs/analytics/src/events.rs @@ -26,6 +26,7 @@ use crate::now_unix_millis; use codex_app_server_protocol::CodexErrorInfo; use codex_app_server_protocol::CommandExecutionSource; use codex_login::default_client::originator; +use codex_plugin::PluginId; use codex_plugin::PluginTelemetryMetadata; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::models::AdditionalPermissionProfile; @@ -933,6 +934,7 @@ pub(crate) struct CodexTurnSteerEventRequest { #[derive(Serialize)] pub(crate) struct CodexPluginMetadata { pub(crate) plugin_id: Option, + pub(crate) remote_plugin_id: Option, pub(crate) plugin_name: Option, pub(crate) marketplace_name: Option, pub(crate) has_skills: Option, @@ -1040,11 +1042,13 @@ pub(crate) fn codex_plugin_metadata(plugin: PluginTelemetryMetadata) -> CodexPlu remote_plugin_id, capability_summary, } = plugin; - let event_plugin_id = remote_plugin_id.unwrap_or_else(|| plugin_id.as_key()); CodexPluginMetadata { - plugin_id: Some(event_plugin_id), - plugin_name: Some(plugin_id.plugin_name), - marketplace_name: Some(plugin_id.marketplace_name), + plugin_id: plugin_id.as_ref().map(PluginId::as_key), + remote_plugin_id, + plugin_name: plugin_id + .as_ref() + .map(|plugin_id| plugin_id.plugin_name.clone()), + marketplace_name: plugin_id.map(|plugin_id| plugin_id.marketplace_name), has_skills: capability_summary .as_ref() .map(|summary| summary.has_skills), diff --git a/codex-rs/app-server-test-client/README.md b/codex-rs/app-server-test-client/README.md index 44687672a..b1afc7787 100644 --- a/codex-rs/app-server-test-client/README.md +++ b/codex-rs/app-server-test-client/README.md @@ -46,12 +46,13 @@ cargo run -p codex-app-server-test-client -- \ Use `--capture-file /tmp/plugin-analytics.jsonl` to select the output path. The command validates one `codex_plugin_disabled`, `codex_plugin_enabled`, and -`codex_plugin_used` event with the expected local plugin identity and capability -metadata. The enabled and disabled events come from successful writes to the -temporary config; the command does not mutate the remote enabled state. It -prints the events and leaves the JSONL file in place for inspection. It does not -install or uninstall plugins and does not modify the profile's persistent -config. +`codex_plugin_used` event with the expected local and remote plugin identities +and capability metadata. Each event includes the local ID in `plugin_id` and the +backend ID in `remote_plugin_id`. The enabled and disabled events come from +successful writes to the temporary config; the command does not mutate the +remote enabled state. It prints the events and leaves the JSONL file in place +for inspection. It does not install or uninstall plugins and does not modify +the profile's persistent config. ### Testing remote install and uninstall analytics @@ -63,9 +64,11 @@ or CI. Choose a remote plugin that is available to the active account and is not currently installed. The command refuses to run when the plugin is already installed, installs it, validates `codex_plugin_installed`, uninstalls it, and -verifies that the original uninstalled state was restored. The current install -event uses the backend ID as `plugin_id`. Uninstall is part of cleanup but is -not yet an analytics assertion. +validates `codex_plugin_uninstalled`, and verifies that the original +uninstalled state was restored. + +The mutation events include the local Codex ID in `plugin_id` and the backend ID +in `remote_plugin_id`. `--remote-plugin-id` takes the backend ID, such as `plugins~Plugin_...`, not the local `@` ID. @@ -83,7 +86,7 @@ Analytics use the normal queue, reduction, batching, and serialization path, but the debug capture destination suppresses analytics network delivery. The command prints one of these final states: -- `PASS`: the install event validated and the plugin is uninstalled. +- `PASS`: the install and uninstall events validated and the plugin is uninstalled. - `FAIL-CLEAN`: validation failed, but the original uninstalled state was restored. - `FAIL-LOCAL-CACHE`: the backend is uninstalled, but local cleanup reported diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs b/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs index 4ffd3fd23..8a68a5746 100644 --- a/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs +++ b/codex-rs/app-server-test-client/src/plugin_analytics_capture.rs @@ -35,7 +35,7 @@ pub(super) fn read_events_for_remote_plugin( matching.extend( events .iter() - .filter(|event| event["event_params"]["plugin_id"] == remote_plugin_id) + .filter(|event| event["event_params"]["remote_plugin_id"] == remote_plugin_id) .cloned(), ); } @@ -44,6 +44,7 @@ pub(super) fn read_events_for_remote_plugin( pub(super) struct PluginEventIdentity<'a> { pub(super) plugin_id: &'a str, + pub(super) remote_plugin_id: &'a str, pub(super) plugin_name: &'a str, pub(super) marketplace_name: &'a str, } @@ -52,25 +53,29 @@ pub(super) fn validate_mutation_events( events: Vec, expected: PluginEventIdentity<'_>, ) -> Result> { - let event_type = "codex_plugin_installed"; - let matching = events - .iter() - .filter(|event| event["event_type"] == event_type) - .collect::>(); - let [event] = matching.as_slice() else { - bail!( - "expected exactly one `{event_type}` event for `{}`, found {}", - expected.plugin_id, - matching.len() - ); - }; - validate_event(event, &expected)?; - Ok(vec![(*event).clone()]) + let mut validated = Vec::new(); + for event_type in ["codex_plugin_installed", "codex_plugin_uninstalled"] { + let matching = events + .iter() + .filter(|event| event["event_type"] == event_type) + .collect::>(); + let [event] = matching.as_slice() else { + bail!( + "expected exactly one `{event_type}` event for `{}`, found {}", + expected.remote_plugin_id, + matching.len() + ); + }; + validate_event(event, &expected)?; + validated.push((*event).clone()); + } + Ok(validated) } fn validate_event(event: &Value, expected: &PluginEventIdentity<'_>) -> Result<()> { let params = &event["event_params"]; require_string(params, "plugin_id", expected.plugin_id)?; + require_string(params, "remote_plugin_id", expected.remote_plugin_id)?; require_string(params, "plugin_name", expected.plugin_name)?; require_string(params, "marketplace_name", expected.marketplace_name)?; for field in [ diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs b/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs index 45a9c865b..b85bd48b8 100644 --- a/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs +++ b/codex-rs/app-server-test-client/src/plugin_analytics_capture_tests.rs @@ -14,15 +14,17 @@ const REMOTE_PLUGIN_ID: &str = "plugins~Plugin_test"; fn reads_and_validates_remote_plugin_mutation_events() { let path = unique_capture_path("valid"); let installed = mutation_event("codex_plugin_installed"); + let uninstalled = mutation_event("codex_plugin_uninstalled"); let unrelated = json!({ "event_type": "codex_plugin_installed", "event_params": { - "plugin_id": "plugins~Plugin_other" + "plugin_id": "other@openai-curated-remote", + "remote_plugin_id": "plugins~Plugin_other" } }); let contents = [ json!({"events": [unrelated]}), - json!({"events": [installed]}), + json!({"events": [installed, uninstalled]}), ] .into_iter() .map(|payload| serde_json::to_string(&payload).expect("serialize capture payload")) @@ -35,7 +37,7 @@ fn reads_and_validates_remote_plugin_mutation_events() { let validated = validate_mutation_events(events, expected_identity()).expect("validate mutation events"); - assert_eq!(validated, vec![installed]); + assert_eq!(validated, vec![installed, uninstalled]); fs::remove_file(path).expect("remove capture file"); } @@ -62,7 +64,8 @@ fn mutation_event(event_type: &str) -> Value { json!({ "event_type": event_type, "event_params": { - "plugin_id": REMOTE_PLUGIN_ID, + "plugin_id": "sample@openai-curated-remote", + "remote_plugin_id": REMOTE_PLUGIN_ID, "plugin_name": "sample", "marketplace_name": "openai-curated-remote", "has_skills": true, @@ -75,7 +78,8 @@ fn mutation_event(event_type: &str) -> Value { fn expected_identity() -> PluginEventIdentity<'static> { PluginEventIdentity { - plugin_id: REMOTE_PLUGIN_ID, + plugin_id: "sample@openai-curated-remote", + remote_plugin_id: REMOTE_PLUGIN_ID, plugin_name: "sample", marketplace_name: "openai-curated-remote", } diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs b/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs index 2c6d50541..70aa13ab9 100644 --- a/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs +++ b/codex-rs/app-server-test-client/src/plugin_analytics_mutation_smoke.rs @@ -312,13 +312,19 @@ fn run_mutation_sequence( state_err } })?; + wait_for_remote_plugin_event( + capture_path, + &expected.remote_plugin_id, + "codex_plugin_uninstalled", + )?; let captured_events = read_events_for_remote_plugin(capture_path, &expected.remote_plugin_id)?; let events = validate_mutation_events( captured_events, PluginEventIdentity { - plugin_id: &expected.remote_plugin_id, + plugin_id: &expected.plugin_id, + remote_plugin_id: &expected.remote_plugin_id, plugin_name: &expected.plugin_name, marketplace_name: &expected.marketplace_name, }, diff --git a/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs b/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs index 701f6361a..65704eeb6 100644 --- a/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs +++ b/codex-rs/app-server-test-client/src/plugin_analytics_smoke.rs @@ -159,6 +159,7 @@ fn wait_for_plugin_usage( #[derive(Debug)] struct ExpectedPlugin { plugin_id: String, + remote_plugin_id: String, plugin_name: String, marketplace_name: String, } @@ -208,13 +209,15 @@ fn expected_plugin(response: &PluginInstalledResponse, plugin_id: &str) -> Resul plugin.availability ); } - plugin + let remote_plugin_id = plugin .remote_plugin_id .as_ref() - .with_context(|| format!("plugin `{plugin_id}` does not have a remote plugin id"))?; + .with_context(|| format!("plugin `{plugin_id}` does not have a remote plugin id"))? + .clone(); Ok(ExpectedPlugin { plugin_id: plugin.id.clone(), + remote_plugin_id, plugin_name: plugin.name.clone(), marketplace_name: marketplace.name.clone(), }) @@ -444,6 +447,7 @@ fn event_count(events: &[Value], event_type: &str) -> usize { fn validate_identity(event: &Value, expected: &ExpectedPlugin) -> Result<()> { let params = &event["event_params"]; require_string(params, "plugin_id", &expected.plugin_id)?; + require_string(params, "remote_plugin_id", &expected.remote_plugin_id)?; require_string(params, "plugin_name", &expected.plugin_name)?; require_string(params, "marketplace_name", &expected.marketplace_name) } diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index 76123b6aa..eb68feb54 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -23,6 +23,7 @@ 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)] @@ -1528,6 +1529,7 @@ impl PluginRequestProcessor { self.track_plugin_install_failed_for_remote_plugin( &remote_plugin_id, &remote_marketplace_name, + /*plugin_id*/ None, error_type, err.to_string(), ); @@ -1538,6 +1540,12 @@ impl PluginRequestProcessor { })?; let actual_remote_marketplace_name = remote_detail.marketplace_name.clone(); let remote_plugin_name = remote_detail.summary.name.clone(); + let resolved_plugin_id = PluginId::parse(&remote_detail.summary.id).map_err(|err| { + internal_error(format!( + "invalid resolved plugin id `{}`: {err}", + remote_detail.summary.id + )) + })?; if remote_detail.summary.availability == PluginAvailability::DisabledByAdmin { return Err(invalid_request(format!( "remote plugin {remote_plugin_id} is disabled by admin" @@ -1569,6 +1577,7 @@ impl PluginRequestProcessor { self.track_plugin_install_failed_for_remote_plugin( &remote_plugin_id, &actual_remote_marketplace_name, + Some(&resolved_plugin_id), error_type, err.to_string(), ); @@ -1585,6 +1594,7 @@ impl PluginRequestProcessor { self.track_plugin_install_failed_for_remote_plugin( &remote_plugin_id, &actual_remote_marketplace_name, + Some(&resolved_plugin_id), error_type, err.to_string(), ); @@ -1606,6 +1616,7 @@ impl PluginRequestProcessor { self.track_plugin_install_failed_for_remote_plugin( &remote_plugin_id, &actual_remote_marketplace_name, + Some(&result.plugin_id), error_type, err.to_string(), ); @@ -1702,6 +1713,7 @@ impl PluginRequestProcessor { &self, remote_plugin_id: &str, marketplace_name: &str, + plugin_id: Option<&PluginId>, error_type: &'static str, error_message: String, ) { @@ -1712,16 +1724,17 @@ impl PluginRequestProcessor { error = %error_message, "remote plugin install failed" ); - // The remote id is reported separately; this local name only satisfies - // PluginId validation before remote details are available. - let Ok(plugin_id) = PluginId::new("unknown".to_string(), marketplace_name.to_string()) - else { - return; + let plugin = if let Some(plugin_id) = plugin_id { + self.thread_manager + .plugins_manager() + .telemetry_metadata_for_plugin_id_with_remote_id(plugin_id, remote_plugin_id) + } else { + PluginTelemetryMetadata { + plugin_id: None, + remote_plugin_id: Some(remote_plugin_id.to_string()), + capability_summary: None, + } }; - 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()); } @@ -1980,11 +1993,31 @@ impl PluginRequestProcessor { let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), }; + let uninstall_target = codex_core_plugins::remote::resolve_remote_plugin_uninstall_target( + &remote_plugin_service_config, + auth.as_ref(), + &plugin_id, + ) + .await + .map_err(|err| { + remote_plugin_catalog_error_to_jsonrpc(err, "resolve remote plugin before uninstall") + })?; + let plugins_manager = self.thread_manager.plugins_manager(); + let mut plugin_telemetry = plugins_manager + .telemetry_metadata_for_installed_plugin_with_remote_id( + &uninstall_target.plugin_id, + &uninstall_target.remote_plugin_id, + ) + .await; + if plugin_telemetry.capability_summary.is_none() { + plugin_telemetry.capability_summary = + Some(uninstall_target.fallback_capability_summary.clone()); + } let uninstall_result = codex_core_plugins::remote::uninstall_remote_plugin( &remote_plugin_service_config, auth.as_ref(), config.codex_home.to_path_buf(), - &plugin_id, + uninstall_target, ) .await; @@ -1992,7 +2025,8 @@ impl PluginRequestProcessor { &uninstall_result, Ok(()) | Err(RemotePluginCatalogError::CacheRemove(_)) ) { - let plugins_manager = self.thread_manager.plugins_manager(); + self.analytics_events_client + .track_plugin_uninstalled(plugin_telemetry); if plugins_manager.clear_remote_installed_plugins_cache() { self.on_effective_plugins_changed(); } diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index aebc30dfe..cc76ef1eb 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -538,12 +538,10 @@ async fn plugin_install_tracks_analytics_when_remote_detail_fetch_fails() -> Res payload["events"][0]["event_type"], "codex_plugin_install_failed" ); - assert_eq!(event_params["plugin_id"], REMOTE_PLUGIN_ID); - assert_eq!(event_params["plugin_name"], "unknown"); - assert_eq!( - event_params["marketplace_name"], - "caller-marketplace-is-ignored" - ); + assert_eq!(event_params["plugin_id"], json!(null)); + assert_eq!(event_params["remote_plugin_id"], REMOTE_PLUGIN_ID); + assert_eq!(event_params["plugin_name"], json!(null)); + assert_eq!(event_params["marketplace_name"], json!(null)); assert_eq!( event_params["error_type"], "remote_catalog_unexpected_status" @@ -883,6 +881,7 @@ async fn plugin_install_tracks_analytics_event() -> Result<()> { "event_type": "codex_plugin_installed", "event_params": { "plugin_id": "sample-plugin@debug", + "remote_plugin_id": null, "plugin_name": "sample-plugin", "marketplace_name": "debug", "has_skills": false, @@ -946,6 +945,7 @@ async fn plugin_install_failure_tracks_analytics_event() -> Result<()> { "codex_plugin_install_failed" ); assert_eq!(event_params["plugin_id"], "sample-plugin@debug"); + assert_eq!(event_params["remote_plugin_id"], json!(null)); assert_eq!(event_params["plugin_name"], "sample-plugin"); assert_eq!(event_params["marketplace_name"], "debug"); assert_eq!(event_params["has_skills"], json!(null)); @@ -995,7 +995,8 @@ async fn plugin_install_tracks_remote_plugin_analytics_event() -> Result<()> { "events": [{ "event_type": "codex_plugin_installed", "event_params": { - "plugin_id": REMOTE_PLUGIN_ID, + "plugin_id": "linear@openai-curated-remote", + "remote_plugin_id": REMOTE_PLUGIN_ID, "plugin_name": "linear", "marketplace_name": "openai-curated-remote", "has_skills": true, @@ -1072,7 +1073,8 @@ async fn plugin_install_preserves_status_when_remote_bundle_error_body_is_too_la payload["events"][0]["event_type"], "codex_plugin_install_failed" ); - assert_eq!(event_params["plugin_id"], REMOTE_PLUGIN_ID); + assert_eq!(event_params["plugin_id"], "linear@openai-curated-remote"); + assert_eq!(event_params["remote_plugin_id"], REMOTE_PLUGIN_ID); assert_eq!(event_params["marketplace_name"], "openai-curated-remote"); assert_eq!(event_params["error_type"], "remote_bundle_download_status"); assert!( diff --git a/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs b/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs index 5d31d8390..6f9041a74 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_uninstall.rs @@ -139,6 +139,7 @@ async fn plugin_uninstall_tracks_analytics_event() -> Result<()> { "event_type": "codex_plugin_uninstalled", "event_params": { "plugin_id": "sample-plugin@debug", + "remote_plugin_id": null, "plugin_name": "sample-plugin", "marketplace_name": "debug", "has_skills": false, @@ -216,6 +217,11 @@ async fn plugin_uninstall_writes_remote_plugin_to_cloud_when_remote_plugin_enabl ) .mount(&server) .await; + Mock::given(method("POST")) + .and(path("/backend-api/codex/analytics-events/events")) + .respond_with(ResponseTemplate::new(200).set_body_string(r#"{"status":"ok"}"#)) + .mount(&server) + .await; let remote_plugin_cache_root = codex_home .path() @@ -225,6 +231,11 @@ async fn plugin_uninstall_writes_remote_plugin_to_cloud_when_remote_plugin_enabl remote_plugin_cache_root.join("1.0.0/.codex-plugin/plugin.json"), r#"{"name":"linear","version":"1.0.0"}"#, )?; + std::fs::create_dir_all(remote_plugin_cache_root.join("1.0.0/skills/plan-work"))?; + std::fs::write( + remote_plugin_cache_root.join("1.0.0/skills/plan-work/SKILL.md"), + "---\nname: plan-work\ndescription: Plan work\n---\n", + )?; let legacy_remote_plugin_cache_root = codex_home.path().join(format!( "plugins/cache/openai-curated-remote/{REMOTE_PLUGIN_ID}" )); @@ -233,6 +244,10 @@ async fn plugin_uninstall_writes_remote_plugin_to_cloud_when_remote_plugin_enabl let mut mcp = TestAppServer::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + // Simulate a background remote-cache refresh removing the local bundle + // before the uninstall request captures its telemetry metadata. + std::fs::remove_dir_all(remote_plugin_cache_root.join("1.0.0"))?; + let request_id = mcp .send_plugin_uninstall_request(PluginUninstallParams { plugin_id: REMOTE_PLUGIN_ID.to_string(), @@ -255,6 +270,25 @@ async fn plugin_uninstall_writes_remote_plugin_to_cloud_when_remote_plugin_enabl .await?; assert!(!remote_plugin_cache_root.exists()); assert!(!legacy_remote_plugin_cache_root.exists()); + let payload = wait_for_plugin_analytics_payload(&server).await?; + assert_eq!( + payload, + json!({ + "events": [{ + "event_type": "codex_plugin_uninstalled", + "event_params": { + "plugin_id": "linear@openai-curated-remote", + "remote_plugin_id": REMOTE_PLUGIN_ID, + "plugin_name": "linear", + "marketplace_name": "openai-curated-remote", + "has_skills": true, + "mcp_server_count": 0, + "connector_ids": [], + "product_client_id": DEFAULT_CLIENT_NAME, + } + }] + }) + ); Ok(()) } @@ -638,7 +672,11 @@ async fn mount_remote_plugin_detail_with_name( "interface": {{ "short_description": "Plan and track work" }}, - "skills": [] + "skills": [{{ + "name": "plan-work", + "description": "Plan work", + "interface": null + }}] }} }}"# ); @@ -652,6 +690,29 @@ async fn mount_remote_plugin_detail_with_name( .await; } +async fn wait_for_plugin_analytics_payload(server: &MockServer) -> Result { + timeout(DEFAULT_TIMEOUT, async { + loop { + let Some(requests) = server.received_requests().await else { + tokio::time::sleep(Duration::from_millis(25)).await; + continue; + }; + if let Some(request) = requests.iter().find(|request| { + request.method == "POST" + && request + .url + .path() + .ends_with("/codex/analytics-events/events") + }) { + return serde_json::from_slice(&request.body) + .map_err(|err| anyhow::anyhow!("invalid analytics payload: {err}")); + } + tokio::time::sleep(Duration::from_millis(25)).await; + } + }) + .await? +} + async fn wait_for_remote_plugin_request_count( server: &MockServer, method_name: &str, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 19b48b2c0..e60ce5034 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -708,6 +708,37 @@ impl PluginsManager { remote_installed_plugins_to_config(plugins, &self.store) } + fn remote_plugin_id_for(&self, plugin_id: &PluginId) -> Option { + let cached_remote_plugin_id = { + let cache = match self.remote_installed_plugins_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.as_ref().and_then(|plugins| { + plugins.iter().find_map(|plugin| { + (plugin.name == plugin_id.plugin_name + && plugin.marketplace_name == plugin_id.marketplace_name) + .then(|| plugin.id.clone()) + }) + }) + }; + if cached_remote_plugin_id.is_some() { + return cached_remote_plugin_id; + } + + match self.store.remote_plugin_id(plugin_id) { + Ok(remote_plugin_id) => remote_plugin_id, + Err(err) => { + tracing::warn!( + plugin_id = %plugin_id.as_key(), + error = %err, + "failed to read persisted remote plugin identity" + ); + None + } + } + } + pub async fn telemetry_metadata_for_installed_plugin( &self, plugin_id: &PluginId, @@ -739,8 +770,8 @@ impl PluginsManager { plugin_id: &PluginId, ) -> PluginTelemetryMetadata { PluginTelemetryMetadata { - plugin_id: plugin_id.clone(), - remote_plugin_id: None, + plugin_id: Some(plugin_id.clone()), + remote_plugin_id: self.remote_plugin_id_for(plugin_id), capability_summary: None, } } @@ -762,8 +793,8 @@ impl PluginsManager { ) -> Option { let plugin_id = PluginId::parse(&summary.config_name).ok()?; Some(PluginTelemetryMetadata { - plugin_id, - remote_plugin_id: None, + remote_plugin_id: self.remote_plugin_id_for(&plugin_id), + plugin_id: Some(plugin_id), capability_summary: Some(summary.clone()), }) } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index adf721fc6..3dfcc2aa8 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -859,7 +859,7 @@ async fn installed_plugin_telemetry_metadata_collects_capabilities() { assert_eq!( metadata, PluginTelemetryMetadata { - plugin_id, + plugin_id: Some(plugin_id), remote_plugin_id: None, capability_summary: Some(PluginCapabilitySummary { config_name: "sample@test".to_string(), @@ -873,6 +873,71 @@ async fn installed_plugin_telemetry_metadata_collects_capabilities() { ); } +#[tokio::test] +async fn installed_plugin_telemetry_metadata_resolves_persisted_remote_identity() { + let codex_home = TempDir::new().unwrap(); + write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear"); + let plugin_id = + PluginId::parse("linear@openai-curated-remote").expect("plugin id should parse"); + PluginStore::new(codex_home.path().to_path_buf()) + .write_remote_plugin_id(&plugin_id, "plugins~Plugin_linear") + .expect("persist remote plugin id"); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + + let metadata = manager + .telemetry_metadata_for_installed_plugin(&plugin_id) + .await; + + assert_eq!( + metadata, + PluginTelemetryMetadata { + plugin_id: Some(plugin_id), + remote_plugin_id: Some("plugins~Plugin_linear".to_string()), + capability_summary: Some(PluginCapabilitySummary { + config_name: "linear@openai-curated-remote".to_string(), + display_name: "linear".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_prefers_remote_snapshot_identity() { + let codex_home = TempDir::new().unwrap(); + write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear"); + let plugin_id = + PluginId::parse("linear@openai-curated-remote").expect("plugin id should parse"); + PluginStore::new(codex_home.path().to_path_buf()) + .write_remote_plugin_id(&plugin_id, "plugins~Plugin_stale") + .expect("persist remote plugin id"); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + manager.write_remote_installed_plugins_cache(vec![remote_installed_linear_plugin()]); + + let metadata = manager + .telemetry_metadata_for_installed_plugin(&plugin_id) + .await; + + assert_eq!( + metadata, + PluginTelemetryMetadata { + plugin_id: Some(plugin_id), + remote_plugin_id: Some("plugins~Plugin_linear".to_string()), + capability_summary: Some(PluginCapabilitySummary { + config_name: "linear@openai-curated-remote".to_string(), + display_name: "linear".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(); @@ -887,7 +952,7 @@ async fn installed_plugin_telemetry_metadata_accepts_authoritative_remote_identi assert_eq!( metadata, PluginTelemetryMetadata { - plugin_id, + plugin_id: Some(plugin_id), remote_plugin_id: Some("plugins~Plugin_linear".to_string()), capability_summary: None, } @@ -912,14 +977,46 @@ fn capability_summary_telemetry_metadata_uses_local_identity() { assert_eq!( metadata, Some(PluginTelemetryMetadata { - plugin_id: PluginId::parse("linear@openai-curated-remote") - .expect("plugin id should parse"), + plugin_id: Some( + PluginId::parse("linear@openai-curated-remote").expect("plugin id should parse"), + ), remote_plugin_id: None, capability_summary: Some(summary), }) ); } +#[test] +fn capability_summary_telemetry_metadata_resolves_persisted_remote_identity() { + let codex_home = TempDir::new().unwrap(); + write_cached_plugin(codex_home.path(), "openai-curated-remote", "linear"); + let plugin_id = + PluginId::parse("linear@openai-curated-remote").expect("plugin id should parse"); + PluginStore::new(codex_home.path().to_path_buf()) + .write_remote_plugin_id(&plugin_id, "plugins~Plugin_linear") + .expect("persist remote plugin id"); + 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: Some(plugin_id), + remote_plugin_id: Some("plugins~Plugin_linear".to_string()), + 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(); diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 05ce11095..506b943b1 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -12,8 +12,10 @@ use codex_login::CodexAuth; use codex_login::default_client::build_reqwest_client; use codex_plugin::AppConnectorId; use codex_plugin::AppDeclaration; +use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginId; use codex_plugin::app_connector_ids_from_declarations; +use codex_plugin::prompt_safe_plugin_description; use codex_utils_absolute_path::AbsolutePathBuf; use reqwest::RequestBuilder; use serde::Deserialize; @@ -120,6 +122,13 @@ pub struct RemotePluginServiceConfig { pub chatgpt_base_url: String, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemotePluginUninstallTarget { + pub plugin_id: PluginId, + pub remote_plugin_id: String, + pub fallback_capability_summary: PluginCapabilitySummary, +} + #[derive(Debug, Clone, PartialEq)] pub struct RemoteMarketplace { pub name: String, @@ -1304,40 +1313,90 @@ pub async fn install_remote_plugin( }) } +pub async fn resolve_remote_plugin_uninstall_target( + config: &RemotePluginServiceConfig, + auth: Option<&CodexAuth>, + remote_plugin_id: &str, +) -> Result { + let auth = ensure_chatgpt_auth(auth)?; + let plugin = fetch_plugin_detail( + config, + auth, + remote_plugin_id, + /*include_download_urls*/ false, + ) + .await?; + let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string(); + let plugin_id = PluginId::new(plugin.name.clone(), marketplace_name).map_err(|err| { + RemotePluginCatalogError::UnexpectedResponse(format!( + "invalid local plugin id for remote plugin `{}`: {err}", + plugin.id + )) + })?; + let app_declarations = plugin + .release + .app_manifest + .as_ref() + .map(plugin_app_declarations_from_value) + .unwrap_or_else(|| app_declarations_from_remote_app_ids(&plugin.release.app_ids)); + let mut mcp_server_names = plugin + .release + .mcp_servers + .iter() + .map(|server| server.key.clone()) + .collect::>(); + mcp_server_names.sort_unstable(); + mcp_server_names.dedup(); + let fallback_capability_summary = PluginCapabilitySummary { + config_name: plugin_id.as_key(), + display_name: plugin.release.display_name, + description: prompt_safe_plugin_description(Some(&plugin.release.description)), + has_skills: !plugin.release.skills.is_empty(), + mcp_server_names, + app_connector_ids: app_connector_ids_from_declarations(&app_declarations), + }; + Ok(RemotePluginUninstallTarget { + plugin_id, + remote_plugin_id: plugin.id, + fallback_capability_summary, + }) +} + pub async fn uninstall_remote_plugin( config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, codex_home: PathBuf, - plugin_id: &str, + target: RemotePluginUninstallTarget, ) -> Result<(), RemotePluginCatalogError> { let auth = ensure_chatgpt_auth(auth)?; - let plugin = fetch_plugin_detail( - config, auth, plugin_id, /*include_download_urls*/ false, - ) - .await?; - let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string(); - let plugin_name = plugin.name; + let RemotePluginUninstallTarget { + plugin_id, + remote_plugin_id, + fallback_capability_summary: _, + } = target; + let marketplace_name = plugin_id.marketplace_name.clone(); + let plugin_name = plugin_id.plugin_name.clone(); let base_url = config.chatgpt_base_url.trim_end_matches('/'); - let url = format!("{base_url}/ps/plugins/{plugin_id}/uninstall"); + let url = format!("{base_url}/ps/plugins/{remote_plugin_id}/uninstall"); let client = build_reqwest_client(); let request = authenticated_request(client.post(&url), auth)?; let response: RemotePluginMutationResponse = send_and_decode(request, &url).await?; - if response.id != plugin_id { + if response.id != remote_plugin_id { return Err(RemotePluginCatalogError::UnexpectedPluginId { - expected: plugin_id.to_string(), + expected: remote_plugin_id, actual: response.id, }); } if response.enabled { return Err(RemotePluginCatalogError::UnexpectedEnabledState { - plugin_id: plugin_id.to_string(), + plugin_id: response.id, expected_enabled: false, actual_enabled: response.enabled, }); } - let legacy_plugin_id = plugin_id.to_string(); + let legacy_plugin_id = response.id; tokio::task::spawn_blocking(move || { remove_remote_plugin_cache(codex_home, marketplace_name, plugin_name, legacy_plugin_id) }) diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 1a4eb23af..8a00d8c5e 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -70,9 +70,10 @@ pub struct PluginHookSource { #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginTelemetryMetadata { - pub plugin_id: PluginId, - /// Optional backend identifier for remote plugins, used when analytics - /// should report the remote id instead of the local plugin cache id. + /// Local plugin identifier used by Codex configuration and the plugin cache, + /// when it has been resolved. + pub plugin_id: Option, + /// Optional backend identifier for remote plugins. pub remote_plugin_id: Option, pub capability_summary: Option, }