From a72433d5606984e71dc37a83a65a6f91af5c84b4 Mon Sep 17 00:00:00 2001 From: jameswt-oai Date: Mon, 22 Jun 2026 14:28:39 -0700 Subject: [PATCH] [codex-core-plugins] Remote Plugin ID Persisted to File (#27669) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## This PR Remote plugin analytics cannot rely only on the in-memory installed-plugin snapshot because that snapshot is refreshed asynchronously after startup. This PR persists the authoritative backend identity alongside each cached remote plugin bundle so later consumers can resolve it without a network request. ### Behavior - Store Codex-owned remote installation metadata in an atomic `.codex-remote-plugin-install.json` sidecar under the plugin cache root. - Use a versioned, snake_case schema: ```json { "schema_version": 1, "remote_plugin_id": "plugins~Plugin_..." } ``` - Write the metadata during remote bundle installation. - Backfill it when bundle sync finds an already-current cached bundle. - Clear it when a generic/local install replaces the cache. - Let existing uninstall and stale-cache removal delete it with the plugin cache root. - Reject unsupported schema versions rather than silently misreading future formats. This PR does not change analytics serialization or event behavior. ### Review surface The implementation is limited to four `codex-core-plugins` files: - `store.rs`: owns the versioned sidecar read/write/remove lifecycle. - `remote_bundle.rs`: persists the backend ID after a remote bundle install. - `remote/remote_installed_plugin_sync.rs`: backfills metadata for an already-current cached bundle. - Tests cover the storage lifecycle and both remote write paths. ## Testing / Validation ### Automated - `just test -p codex-core-plugins` (268 tests passed) - `just fix -p codex-core-plugins` passes with one pre-existing `large_enum_variant` warning in `manifest.rs`. - Coverage verifies the exact filename and JSON schema, identity replacement, local reinstall clearing, uninstall cleanup, remote bundle installation, unsupported schema rejection, and installed-plugin sync backfill. ### Live manual validation Validated the production app-server RPC path with an isolated temporary `CODEX_HOME` and the PR-built Codex binary. The app-server communicated over stdio and did not bind a port. Test plugin: `plugins~Plugin_b80dd84519148191a409cde181c9b3d6` (`build-macos-apps@openai-curated-remote`). 1. Confirmed `plugin/read` initially reported the plugin uninstalled. 2. Installed it through `plugin/install` and confirmed version `0.1.4` was cached. 3. Verified `$CODEX_HOME/plugins/cache/openai-curated-remote/build-macos-apps/.codex-remote-plugin-install.json` was created beside the `0.1.4/` bundle directory with mode `0600` and the expected contents: ```json { "schema_version": 1, "remote_plugin_id": "plugins~Plugin_b80dd84519148191a409cde181c9b3d6" } ``` 4. Deleted only the sidecar, restarted the app-server, and confirmed installed-plugin startup sync recreated it with the same contents. 5. Uninstalled through `plugin/uninstall`, confirmed `plugin/read` returned `installed: false`, and verified the local plugin cache root was removed. 6. Restored the account's original uninstalled state and removed the isolated home and copied credentials. ## Split Overview ```text main ├── #27093 Debug analytics capture merged │ └── #27099 Non-mutating plugin smoke merged │ └── #27100 Remote install/uninstall smoke merged └── #27102 Plugin telemetry metadata refactor merged └── #27669 Persist remote plugin identity ← this PR Next: └── Final PR: add explicit local and remote IDs to plugin analytics ``` This PR is based directly on `main`; prerequisite [#27102](https://github.com/openai/codex/pull/27102) has merged. The original combined [#26281](https://github.com/openai/codex/pull/26281) remains the aggregate reference until the final replacement PR is published. --- .../remote/remote_installed_plugin_sync.rs | 116 ++++++++++++++++ codex-rs/core-plugins/src/remote_bundle.rs | 46 ++++++- codex-rs/core-plugins/src/store.rs | 127 ++++++++++++++++++ codex-rs/core-plugins/src/store_tests.rs | 106 +++++++++++++++ 4 files changed, 393 insertions(+), 2 deletions(-) diff --git a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs index 8041d444b..2a23b066e 100644 --- a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs +++ b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs @@ -212,6 +212,16 @@ pub async fn sync_remote_installed_plugin_bundles_once( .map(str::trim) .filter(|version| !version.is_empty()); if store.active_plugin_version(&plugin_id).as_deref() == release_version { + if let Err(err) = store.write_remote_plugin_id(&plugin_id, &plugin.id) { + warn!( + remote_plugin_id = %plugin.id, + plugin = %plugin.name, + marketplace = %marketplace_name, + error = %err, + "failed to persist identity for cached remote installed plugin" + ); + failed_remote_plugin_ids.insert(plugin.id); + } continue; } @@ -439,6 +449,13 @@ fn clear_remote_installed_plugin_bundle_sync_in_flight(key: &RemoteInstalledPlug mod tests { use super::*; use pretty_assertions::assert_eq; + use serde_json::json; + use wiremock::Mock; + use wiremock::MockServer; + use wiremock::ResponseTemplate; + use wiremock::matchers::method; + use wiremock::matchers::path; + use wiremock::matchers::query_param; #[test] fn remote_installed_plugin_sync_in_flight_dedupes_by_cache_root() { @@ -461,6 +478,105 @@ mod tests { clear_remote_installed_plugin_bundle_sync_in_flight(&key); } + #[tokio::test] + async fn sync_backfills_remote_plugin_install_metadata_for_current_bundle() { + let server = MockServer::start().await; + let codex_home = tempfile::tempdir().expect("create codex home"); + let cached_manifest = codex_home + .path() + .join(PLUGINS_CACHE_DIR) + .join(REMOTE_GLOBAL_MARKETPLACE_NAME) + .join("linear") + .join("1.2.3") + .join(".codex-plugin") + .join("plugin.json"); + std::fs::create_dir_all(cached_manifest.parent().expect("manifest parent")) + .expect("create cached plugin manifest parent"); + std::fs::write(&cached_manifest, r#"{"name":"linear","version":"1.2.3"}"#) + .expect("write cached plugin manifest"); + let remote_plugin_id = "plugins~Plugin_linear"; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "GLOBAL")) + .and(query_param("includeDownloadUrls", "true")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [{ + "id": remote_plugin_id, + "name": "linear", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "ENABLED", + "release": { + "version": "1.2.3", + "display_name": "Linear", + "description": "Track work", + "interface": {}, + }, + "enabled": true, + }], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "USER")) + .and(query_param("includeDownloadUrls", "true")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "WORKSPACE")) + .and(query_param("includeDownloadUrls", "true")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + let config = RemotePluginServiceConfig { + chatgpt_base_url: format!("{}/backend-api", server.uri()), + }; + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + let outcome = sync_remote_installed_plugin_bundles_once( + codex_home.path().to_path_buf(), + &config, + Some(&auth), + ) + .await + .expect("sync current remote plugin bundle"); + + assert_eq!(outcome, RemoteInstalledPluginBundleSyncOutcome::default()); + let plugin_id = PluginId::new( + "linear".to_string(), + REMOTE_GLOBAL_MARKETPLACE_NAME.to_string(), + ) + .expect("valid plugin id"); + let metadata_path = PluginStore::new(codex_home.path().to_path_buf()) + .plugin_base_root(&plugin_id) + .join(".codex-remote-plugin-install.json"); + assert_eq!( + serde_json::from_str::( + &std::fs::read_to_string(metadata_path.as_path()) + .expect("read remote plugin install metadata") + ) + .expect("parse remote plugin install metadata"), + json!({ + "schema_version": 1, + "remote_plugin_id": remote_plugin_id, + }) + ); + } + #[test] fn stale_remote_plugin_cleanup_skips_cache_mutations_in_progress() { let codex_home = tempfile::tempdir().expect("create codex home"); diff --git a/codex-rs/core-plugins/src/remote_bundle.rs b/codex-rs/core-plugins/src/remote_bundle.rs index 49e6bd413..66592dc36 100644 --- a/codex-rs/core-plugins/src/remote_bundle.rs +++ b/codex-rs/core-plugins/src/remote_bundle.rs @@ -34,6 +34,7 @@ const TEST_ALLOW_LOOPBACK_HTTP_REMOTE_PLUGIN_BUNDLES_ENV: &str = pub struct ValidatedRemotePluginBundle { pub plugin_id: PluginId, pub plugin_version: String, + remote_plugin_id: String, app_manifest: Option, bundle_download_url: String, } @@ -190,6 +191,7 @@ pub fn validate_remote_plugin_bundle( Ok(ValidatedRemotePluginBundle { plugin_id, plugin_version, + remote_plugin_id: remote_plugin_id.to_string(), app_manifest, bundle_download_url, }) @@ -403,9 +405,12 @@ fn install_remote_plugin_bundle( })?; let store = PluginStore::try_new(codex_home)?; - store + let remote_plugin_id = bundle.remote_plugin_id; + let result = store .install_with_version(plugin_root, bundle.plugin_id, bundle.plugin_version) - .map_err(RemotePluginBundleInstallError::from) + .map_err(RemotePluginBundleInstallError::from)?; + store.write_remote_plugin_id(&result.plugin_id, &remote_plugin_id)?; + Ok(result) } fn extract_remote_plugin_bundle_to_path( @@ -745,6 +750,43 @@ mod tests { ); } + #[test] + fn install_persists_remote_plugin_install_metadata() { + let codex_home = tempdir().expect("tempdir"); + let bundle = valid_remote_plugin_bundle(); + + let result = install_remote_plugin_bundle( + codex_home.path().to_path_buf(), + bundle, + tar_gz_bytes(&[( + ".codex-plugin/plugin.json", + br#"{"name":"linear","version":"1.2.3"}"#, + /*mode*/ 0o644, + )]), + ) + .expect("install bundle"); + let store = PluginStore::new(codex_home.path().to_path_buf()); + + assert_eq!( + store.remote_plugin_id(&result.plugin_id).unwrap(), + Some(REMOTE_PLUGIN_ID.to_string()) + ); + let metadata_path = store + .plugin_base_root(&result.plugin_id) + .join(".codex-remote-plugin-install.json"); + assert_eq!( + serde_json::from_str::( + &std::fs::read_to_string(metadata_path.as_path()) + .expect("read remote plugin install metadata") + ) + .expect("parse remote plugin install metadata"), + serde_json::json!({ + "schema_version": 1, + "remote_plugin_id": REMOTE_PLUGIN_ID, + }) + ); + } + #[test] fn install_preserves_non_global_bundle_manifest_metadata() { let codex_home = tempdir().expect("tempdir"); diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index f0c737e28..5d78fefa9 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -7,16 +7,26 @@ use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_plugins::find_plugin_manifest_path; use semver::Version; use serde::Deserialize; +use serde::Serialize; use serde_json::Value as JsonValue; use std::cmp::Ordering; use std::fs; use std::io; +use std::io::Write; use std::path::Path; use std::path::PathBuf; pub const DEFAULT_PLUGIN_VERSION: &str = "local"; pub const PLUGINS_CACHE_DIR: &str = "plugins/cache"; pub const PLUGINS_DATA_DIR: &str = "plugins/data"; +const REMOTE_PLUGIN_INSTALL_METADATA_FILE: &str = ".codex-remote-plugin-install.json"; +const REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION: u8 = 1; + +#[derive(Debug, Deserialize, Serialize)] +struct RemotePluginInstallMetadata { + schema_version: u8, + remote_plugin_id: String, +} #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallResult { @@ -106,6 +116,102 @@ impl PluginStore { self.active_plugin_version(plugin_id).is_some() } + pub fn remote_plugin_id( + &self, + plugin_id: &PluginId, + ) -> Result, PluginStoreError> { + if !self.is_installed(plugin_id) { + return Ok(None); + } + let path = self.remote_plugin_install_metadata_path(plugin_id); + let contents = match fs::read_to_string(path.as_path()) { + Ok(contents) => contents, + Err(err) if err.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(PluginStoreError::io( + "failed to read remote plugin install metadata", + err, + )); + } + }; + let metadata: RemotePluginInstallMetadata = + serde_json::from_str(&contents).map_err(|err| { + PluginStoreError::Invalid(format!( + "failed to parse remote plugin install metadata: {err}" + )) + })?; + if metadata.schema_version != REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION { + return Err(PluginStoreError::Invalid(format!( + "unsupported remote plugin install metadata schema version: {}", + metadata.schema_version + ))); + } + let remote_plugin_id = metadata.remote_plugin_id.trim(); + if remote_plugin_id.is_empty() { + return Err(PluginStoreError::Invalid( + "invalid remote plugin install metadata: remote plugin id must not be blank" + .to_string(), + )); + } + Ok(Some(remote_plugin_id.to_string())) + } + + pub fn write_remote_plugin_id( + &self, + plugin_id: &PluginId, + remote_plugin_id: &str, + ) -> Result<(), PluginStoreError> { + if !self.is_installed(plugin_id) { + return Err(PluginStoreError::Invalid(format!( + "cannot write remote identity for uninstalled plugin `{}`", + plugin_id.as_key() + ))); + } + let remote_plugin_id = remote_plugin_id.trim(); + if remote_plugin_id.is_empty() { + return Err(PluginStoreError::Invalid( + "invalid remote plugin install metadata: remote plugin id must not be blank" + .to_string(), + )); + } + let path = self.remote_plugin_install_metadata_path(plugin_id); + let parent = path.as_path().parent().ok_or_else(|| { + PluginStoreError::Invalid(format!( + "remote plugin install metadata path has no parent: {}", + path.display() + )) + })?; + let mut contents = serde_json::to_vec_pretty(&RemotePluginInstallMetadata { + schema_version: REMOTE_PLUGIN_INSTALL_METADATA_SCHEMA_VERSION, + remote_plugin_id: remote_plugin_id.to_string(), + }) + .map_err(|err| { + PluginStoreError::Invalid(format!( + "failed to serialize remote plugin install metadata: {err}" + )) + })?; + contents.push(b'\n'); + let mut temporary = tempfile::NamedTempFile::new_in(parent).map_err(|err| { + PluginStoreError::io( + "failed to create temporary remote plugin install metadata", + err, + ) + })?; + temporary.write_all(&contents).map_err(|err| { + PluginStoreError::io("failed to write remote plugin install metadata", err) + })?; + temporary.as_file_mut().flush().map_err(|err| { + PluginStoreError::io("failed to flush remote plugin install metadata", err) + })?; + temporary.persist(path.as_path()).map_err(|err| { + PluginStoreError::io( + "failed to persist remote plugin install metadata", + err.error, + ) + })?; + Ok(()) + } + pub fn install( &self, source_path: AbsolutePathBuf, @@ -197,6 +303,7 @@ impl PluginStore { &plugin_version, manifest, )?; + self.remove_remote_plugin_install_metadata(&plugin_id)?; Ok(PluginInstallResult { plugin_id, @@ -208,6 +315,26 @@ impl PluginStore { pub fn uninstall(&self, plugin_id: &PluginId) -> Result<(), PluginStoreError> { remove_existing_target(self.plugin_base_root(plugin_id).as_path()) } + + fn remote_plugin_install_metadata_path(&self, plugin_id: &PluginId) -> AbsolutePathBuf { + self.plugin_base_root(plugin_id) + .join(REMOTE_PLUGIN_INSTALL_METADATA_FILE) + } + + fn remove_remote_plugin_install_metadata( + &self, + plugin_id: &PluginId, + ) -> Result<(), PluginStoreError> { + let path = self.remote_plugin_install_metadata_path(plugin_id); + match fs::remove_file(path.as_path()) { + Ok(()) => Ok(()), + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()), + Err(err) => Err(PluginStoreError::io( + "failed to remove remote plugin install metadata", + err, + )), + } + } } #[derive(Debug, thiserror::Error)] diff --git a/codex-rs/core-plugins/src/store_tests.rs b/codex-rs/core-plugins/src/store_tests.rs index aa9b17cc2..237fb4e88 100644 --- a/codex-rs/core-plugins/src/store_tests.rs +++ b/codex-rs/core-plugins/src/store_tests.rs @@ -1,6 +1,7 @@ use super::*; use codex_plugin::PluginId; use pretty_assertions::assert_eq; +use serde_json::json; use tempfile::tempdir; fn write_plugin_with_version( @@ -191,6 +192,111 @@ fn install_with_version_uses_requested_cache_version() { assert!(installed_path.join(".codex-plugin/plugin.json").is_file()); } +#[test] +fn remote_plugin_install_metadata_follows_installed_cache_lifecycle() { + let tmp = tempdir().unwrap(); + write_plugin(tmp.path(), "sample-plugin", "sample-plugin"); + let plugin_id = PluginId::new( + "sample-plugin".to_string(), + "openai-curated-remote".to_string(), + ) + .unwrap(); + let store = PluginStore::new(tmp.path().to_path_buf()); + let source = AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap(); + + store + .install(source.clone(), plugin_id.clone()) + .expect("install plugin"); + assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None); + + store + .write_remote_plugin_id(&plugin_id, "plugins~Plugin_sample") + .expect("write remote identity"); + let metadata_path = store.remote_plugin_install_metadata_path(&plugin_id); + assert_eq!( + metadata_path.as_path().file_name(), + Some(std::ffi::OsStr::new(".codex-remote-plugin-install.json")) + ); + assert_eq!( + serde_json::from_str::( + &fs::read_to_string(metadata_path.as_path()).expect("read install metadata") + ) + .expect("parse install metadata"), + json!({ + "schema_version": 1, + "remote_plugin_id": "plugins~Plugin_sample", + }) + ); + assert_eq!( + store.remote_plugin_id(&plugin_id).unwrap(), + Some("plugins~Plugin_sample".to_string()) + ); + store + .write_remote_plugin_id(&plugin_id, "plugins~Plugin_updated") + .expect("replace remote identity"); + assert_eq!( + store.remote_plugin_id(&plugin_id).unwrap(), + Some("plugins~Plugin_updated".to_string()) + ); + assert_eq!( + serde_json::from_str::( + &fs::read_to_string(metadata_path.as_path()).expect("read updated install metadata") + ) + .expect("parse updated install metadata"), + json!({ + "schema_version": 1, + "remote_plugin_id": "plugins~Plugin_updated", + }) + ); + + store + .install(source, plugin_id.clone()) + .expect("replace with local install"); + assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None); + assert!(!metadata_path.as_path().exists()); + + store + .write_remote_plugin_id(&plugin_id, "plugins~Plugin_sample") + .expect("restore remote identity"); + store.uninstall(&plugin_id).expect("uninstall plugin"); + assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None); + assert!(!metadata_path.as_path().exists()); +} + +#[test] +fn remote_plugin_install_metadata_rejects_unsupported_schema_version() { + let tmp = tempdir().unwrap(); + write_plugin(tmp.path(), "sample-plugin", "sample-plugin"); + let plugin_id = PluginId::new( + "sample-plugin".to_string(), + "openai-curated-remote".to_string(), + ) + .unwrap(); + let store = PluginStore::new(tmp.path().to_path_buf()); + store + .install( + AbsolutePathBuf::try_from(tmp.path().join("sample-plugin")).unwrap(), + plugin_id.clone(), + ) + .expect("install plugin"); + fs::write( + store + .remote_plugin_install_metadata_path(&plugin_id) + .as_path(), + r#"{"schema_version":2,"remote_plugin_id":"plugins~Plugin_sample"}"#, + ) + .expect("write unsupported install metadata"); + + let err = store + .remote_plugin_id(&plugin_id) + .expect_err("unsupported schema version should fail"); + + assert_eq!( + err.to_string(), + "unsupported remote plugin install metadata schema version: 2" + ); +} + #[test] fn install_prefers_on_disk_manifest_version_over_fallback() { let tmp = tempdir().unwrap();