From 72ced3ef3b4b08a49bfb0458215af6c8e31cdec1 Mon Sep 17 00:00:00 2001 From: James Watterson-Tenvooren Date: Thu, 11 Jun 2026 14:38:46 -0700 Subject: [PATCH] Version remote plugin install metadata --- .../remote/remote_installed_plugin_sync.rs | 95 +++++++++++++++++++ codex-rs/core-plugins/src/remote_bundle.rs | 16 +++- codex-rs/core-plugins/src/store.rs | 83 ++++++++++------ codex-rs/core-plugins/src/store_tests.rs | 64 ++++++++++++- 4 files changed, 226 insertions(+), 32 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 f88d46273f..ebcae3f4f1 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 @@ -435,6 +435,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() { @@ -457,6 +464,94 @@ 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", "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 2a452e7203..26c2f3bb79 100644 --- a/codex-rs/core-plugins/src/remote_bundle.rs +++ b/codex-rs/core-plugins/src/remote_bundle.rs @@ -727,7 +727,7 @@ mod tests { } #[test] - fn install_persists_remote_plugin_identity() { + fn install_persists_remote_plugin_install_metadata() { let codex_home = tempdir().expect("tempdir"); let bundle = valid_remote_plugin_bundle(); @@ -747,6 +747,20 @@ mod tests { 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] diff --git a/codex-rs/core-plugins/src/store.rs b/codex-rs/core-plugins/src/store.rs index 771dc3c121..424dbb2596 100644 --- a/codex-rs/core-plugins/src/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -18,11 +18,12 @@ 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"; +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)] -#[serde(rename_all = "camelCase")] -struct RemotePluginIdentity { +struct RemotePluginInstallMetadata { + schema_version: u8, remote_plugin_id: String, } @@ -115,24 +116,34 @@ impl PluginStore { if !self.is_installed(plugin_id) { return Ok(None); } - let path = self.remote_plugin_identity_path(plugin_id); + 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 identity", + "failed to read remote plugin install metadata", 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(); + 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 identity: remote plugin id must not be blank".to_string(), + "invalid remote plugin install metadata: remote plugin id must not be blank" + .to_string(), )); } Ok(Some(remote_plugin_id.to_string())) @@ -152,35 +163,44 @@ impl PluginStore { 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(), + "invalid remote plugin install metadata: remote plugin id must not be blank" + .to_string(), )); } - let path = self.remote_plugin_identity_path(plugin_id); + let path = self.remote_plugin_install_metadata_path(plugin_id); let parent = path.as_path().parent().ok_or_else(|| { PluginStoreError::Invalid(format!( - "remote plugin identity path has no parent: {}", + "remote plugin install metadata path has no parent: {}", path.display() )) })?; - let mut contents = serde_json::to_vec(&RemotePluginIdentity { + 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 identity: {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 identity", 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 - .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) + PluginStoreError::io( + "failed to persist remote plugin install metadata", + err.error, + ) })?; Ok(()) } @@ -221,7 +241,7 @@ impl PluginStore { self.plugin_base_root(&plugin_id).as_path(), &plugin_version, )?; - self.remove_remote_plugin_identity(&plugin_id)?; + self.remove_remote_plugin_install_metadata(&plugin_id)?; Ok(PluginInstallResult { plugin_id, @@ -234,18 +254,21 @@ impl PluginStore { remove_existing_target(self.plugin_base_root(plugin_id).as_path()) } - fn remote_plugin_identity_path(&self, plugin_id: &PluginId) -> AbsolutePathBuf { + fn remote_plugin_install_metadata_path(&self, plugin_id: &PluginId) -> AbsolutePathBuf { self.plugin_base_root(plugin_id) - .join(REMOTE_PLUGIN_IDENTITY_FILE) + .join(REMOTE_PLUGIN_INSTALL_METADATA_FILE) } - fn remove_remote_plugin_identity(&self, plugin_id: &PluginId) -> Result<(), PluginStoreError> { - let path = self.remote_plugin_identity_path(plugin_id); + 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 identity", + "failed to remove remote plugin install metadata", err, )), } diff --git a/codex-rs/core-plugins/src/store_tests.rs b/codex-rs/core-plugins/src/store_tests.rs index c3c53f92cb..65af6043c9 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( @@ -152,7 +153,7 @@ fn install_with_version_uses_requested_cache_version() { } #[test] -fn remote_plugin_identity_follows_installed_cache_lifecycle() { +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( @@ -171,6 +172,21 @@ fn remote_plugin_identity_follows_installed_cache_lifecycle() { 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()) @@ -182,17 +198,63 @@ fn remote_plugin_identity_follows_installed_cache_lifecycle() { 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]