From ad666f96975bed20ce71287ccd5623310473873e Mon Sep 17 00:00:00 2001 From: James Watterson-Tenvooren Date: Thu, 11 Jun 2026 13:50:02 -0700 Subject: [PATCH] Persist remote plugin identities with cached bundles --- .../remote/remote_installed_plugin_sync.rs | 10 ++ codex-rs/core-plugins/src/remote_bundle.rs | 32 +++++- codex-rs/core-plugins/src/store.rs | 104 ++++++++++++++++++ codex-rs/core-plugins/src/store_tests.rs | 44 ++++++++ 4 files changed, 188 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 fe3a597b0f..f88d46273f 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 @@ -199,6 +199,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; } diff --git a/codex-rs/core-plugins/src/remote_bundle.rs b/codex-rs/core-plugins/src/remote_bundle.rs index cb60d60b77..2a452e7203 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, }) @@ -379,9 +381,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( @@ -721,6 +726,29 @@ mod tests { ); } + #[test] + fn install_persists_remote_plugin_identity() { + 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()) + ); + } + #[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 19c3b0116f..771dc3c121 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -6,16 +6,25 @@ 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_IDENTITY_FILE: &str = ".remote-plugin.json"; + +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +struct RemotePluginIdentity { + remote_plugin_id: String, +} #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallResult { @@ -99,6 +108,83 @@ 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_identity_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 identity", + err, + )); + } + }; + let identity: RemotePluginIdentity = serde_json::from_str(&contents).map_err(|err| { + PluginStoreError::Invalid(format!("failed to parse remote plugin identity: {err}")) + })?; + let remote_plugin_id = identity.remote_plugin_id.trim(); + if remote_plugin_id.is_empty() { + return Err(PluginStoreError::Invalid( + "invalid remote plugin identity: 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 identity: remote plugin id must not be blank".to_string(), + )); + } + let path = self.remote_plugin_identity_path(plugin_id); + let parent = path.as_path().parent().ok_or_else(|| { + PluginStoreError::Invalid(format!( + "remote plugin identity path has no parent: {}", + path.display() + )) + })?; + let mut contents = serde_json::to_vec(&RemotePluginIdentity { + remote_plugin_id: remote_plugin_id.to_string(), + }) + .map_err(|err| { + PluginStoreError::Invalid(format!("failed to serialize remote plugin identity: {err}")) + })?; + contents.push(b'\n'); + let mut temporary = tempfile::NamedTempFile::new_in(parent).map_err(|err| { + PluginStoreError::io("failed to create temporary remote plugin identity", err) + })?; + temporary + .write_all(&contents) + .map_err(|err| PluginStoreError::io("failed to write remote plugin identity", err))?; + temporary + .as_file_mut() + .flush() + .map_err(|err| PluginStoreError::io("failed to flush remote plugin identity", err))?; + temporary.persist(path.as_path()).map_err(|err| { + PluginStoreError::io("failed to persist remote plugin identity", err.error) + })?; + Ok(()) + } + pub fn install( &self, source_path: AbsolutePathBuf, @@ -135,6 +221,7 @@ impl PluginStore { self.plugin_base_root(&plugin_id).as_path(), &plugin_version, )?; + self.remove_remote_plugin_identity(&plugin_id)?; Ok(PluginInstallResult { plugin_id, @@ -146,6 +233,23 @@ 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_identity_path(&self, plugin_id: &PluginId) -> AbsolutePathBuf { + self.plugin_base_root(plugin_id) + .join(REMOTE_PLUGIN_IDENTITY_FILE) + } + + fn remove_remote_plugin_identity(&self, plugin_id: &PluginId) -> Result<(), PluginStoreError> { + let path = self.remote_plugin_identity_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 identity", + 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 200055fe6c..c3c53f92cb 100644 --- a/codex-rs/core-plugins/src/store_tests.rs +++ b/codex-rs/core-plugins/src/store_tests.rs @@ -151,6 +151,50 @@ fn install_with_version_uses_requested_cache_version() { assert!(installed_path.join(".codex-plugin/plugin.json").is_file()); } +#[test] +fn remote_plugin_identity_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"); + 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()) + ); + + store + .install(source, plugin_id.clone()) + .expect("replace with local install"); + assert_eq!(store.remote_plugin_id(&plugin_id).unwrap(), None); + + 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); +} + #[test] fn install_uses_manifest_version_when_present() { let tmp = tempdir().unwrap();