diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 32d0bec7af..589467199e 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -42,6 +42,7 @@ const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; const CONFIG_TOML_FILE: &str = "config.toml"; +const CURATED_PLUGIN_CACHE_VERSION_SHA_PREFIX_LEN: usize = 8; #[derive(Clone, Copy, PartialEq, Eq)] enum NonCuratedCacheRefreshMode { @@ -144,6 +145,7 @@ pub fn refresh_curated_plugin_cache( plugin_version: &str, configured_curated_plugin_ids: &[PluginId], ) -> Result { + let cache_plugin_version = curated_plugin_cache_version(plugin_version); let store = PluginStore::try_new(codex_home.to_path_buf()).map_err(|err| err.to_string())?; let curated_marketplace_path = AbsolutePathBuf::try_from( codex_home @@ -181,7 +183,8 @@ pub fn refresh_curated_plugin_cache( let mut cache_refreshed = false; for plugin_id in configured_curated_plugin_ids { - if store.active_plugin_version(plugin_id).as_deref() == Some(plugin_version) { + if store.active_plugin_version(plugin_id).as_deref() == Some(cache_plugin_version.as_str()) + { continue; } @@ -195,7 +198,7 @@ pub fn refresh_curated_plugin_cache( }; store - .install_with_version(source_path, plugin_id.clone(), plugin_version.to_string()) + .install_with_version(source_path, plugin_id.clone(), cache_plugin_version.clone()) .map_err(|err| { format!( "failed to refresh curated plugin cache for {}: {err}", @@ -208,6 +211,14 @@ pub fn refresh_curated_plugin_cache( Ok(cache_refreshed) } +pub fn curated_plugin_cache_version(plugin_version: &str) -> String { + if is_full_git_sha(plugin_version) { + plugin_version[..CURATED_PLUGIN_CACHE_VERSION_SHA_PREFIX_LEN].to_string() + } else { + plugin_version.to_string() + } +} + pub fn refresh_non_curated_plugin_cache( codex_home: &Path, additional_roots: &[AbsolutePathBuf], @@ -328,6 +339,10 @@ fn configured_plugins_from_stack( configured_plugins_from_user_config_value(&user_layer.config) } +fn is_full_git_sha(value: &str) -> bool { + value.len() == 40 && value.chars().all(|ch| ch.is_ascii_hexdigit()) +} + fn configured_plugins_from_user_config_value( user_config: &toml::Value, ) -> HashMap { @@ -1079,6 +1094,23 @@ mod tests { ); } + #[test] + fn curated_plugin_cache_version_shortens_full_git_sha() { + assert_eq!( + curated_plugin_cache_version("0123456789abcdef0123456789abcdef01234567"), + "01234567" + ); + } + + #[test] + fn curated_plugin_cache_version_preserves_non_git_sha_versions() { + assert_eq!( + curated_plugin_cache_version("export-backup"), + "export-backup" + ); + assert_eq!(curated_plugin_cache_version("0123456"), "0123456"); + } + #[test] fn materialize_git_subdir_uses_sparse_checkout() { let codex_home = tempfile::tempdir().expect("create codex home"); diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 842616f94f..77265ece75 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -10,6 +10,7 @@ use codex_config::types::PluginConfig; use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core_plugins::installed_marketplaces::installed_marketplace_roots_from_layer_stack; use codex_core_plugins::loader::configured_curated_plugin_ids_from_codex_home; +use codex_core_plugins::loader::curated_plugin_cache_version; use codex_core_plugins::loader::installed_plugin_telemetry_metadata; use codex_core_plugins::loader::load_plugin_apps; use codex_core_plugins::loader::load_plugin_mcp_servers; @@ -567,13 +568,13 @@ impl PluginsManager { let auth_policy = resolved.policy.authentication; let plugin_version = if resolved.plugin_id.marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME { - Some( - read_curated_plugins_sha(self.codex_home.as_path()).ok_or_else(|| { + let curated_plugin_version = read_curated_plugins_sha(self.codex_home.as_path()) + .ok_or_else(|| { PluginStoreError::Invalid( "local curated marketplace sha is not available".to_string(), ) - })?, - ) + })?; + Some(curated_plugin_cache_version(&curated_plugin_version)) } else { None }; @@ -725,6 +726,7 @@ impl PluginsManager { "local curated marketplace sha is not available".to_string(), ) })?; + let cache_plugin_version = curated_plugin_cache_version(&curated_plugin_version); let mut local_plugins = Vec::<( String, PluginId, @@ -835,11 +837,7 @@ impl PluginsManager { } if remote_installed_plugin_names.contains(&plugin_name) { if !is_installed { - installs.push(( - source_path, - plugin_id.clone(), - curated_plugin_version.clone(), - )); + installs.push((source_path, plugin_id.clone(), cache_plugin_version.clone())); } if !is_installed { result.installed_plugin_ids.push(plugin_key.clone()); diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 8f8efdd713..c8bbba01b9 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -7,6 +7,7 @@ use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; use crate::plugins::LoadedPlugin; use crate::plugins::PluginLoadOutcome; +use crate::plugins::test_support::TEST_CURATED_PLUGIN_CACHE_VERSION; use crate::plugins::test_support::TEST_CURATED_PLUGIN_SHA; use crate::plugins::test_support::write_curated_plugin_sha_with as write_curated_plugin_sha; use crate::plugins::test_support::write_file; @@ -1022,6 +1023,42 @@ async fn install_plugin_updates_config_with_relative_path_and_plugin_key() { assert!(config.contains("enabled = true")); } +#[tokio::test] +async fn install_openai_curated_plugin_uses_short_sha_cache_version() { + let tmp = tempfile::tempdir().unwrap(); + let curated_root = curated_plugins_repo_path(tmp.path()); + write_openai_curated_marketplace(&curated_root, &["slack"]); + write_curated_plugin_sha(tmp.path(), TEST_CURATED_PLUGIN_SHA); + + let result = PluginsManager::new(tmp.path().to_path_buf()) + .install_plugin(PluginInstallRequest { + plugin_name: "slack".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + curated_root.join(".agents/plugins/marketplace.json"), + ) + .unwrap(), + }) + .await + .unwrap(); + + let installed_path = tmp.path().join(format!( + "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_CACHE_VERSION}" + )); + assert_eq!( + result, + PluginInstallOutcome { + plugin_id: PluginId::new( + "slack".to_string(), + OPENAI_CURATED_MARKETPLACE_NAME.to_string() + ) + .unwrap(), + plugin_version: TEST_CURATED_PLUGIN_CACHE_VERSION.to_string(), + installed_path: AbsolutePathBuf::try_from(installed_path).unwrap(), + auth_policy: MarketplacePluginAuthPolicy::OnInstall, + } + ); +} + #[tokio::test] async fn install_plugin_uses_manifest_version_for_non_curated_plugins() { let tmp = tempfile::tempdir().unwrap(); @@ -2660,7 +2697,7 @@ plugins = true ); assert_eq!( fs::read_to_string(tmp.path().join(format!( - "plugins/cache/openai-curated/gmail/{TEST_CURATED_PLUGIN_SHA}/marker.txt" + "plugins/cache/openai-curated/gmail/{TEST_CURATED_PLUGIN_CACHE_VERSION}/marker.txt" ))) .unwrap(), "first" @@ -2739,7 +2776,7 @@ plugins = true } #[test] -fn refresh_curated_plugin_cache_replaces_existing_local_version_with_sha() { +fn refresh_curated_plugin_cache_replaces_existing_local_version_with_short_sha_version() { let tmp = tempfile::tempdir().unwrap(); let curated_root = curated_plugins_repo_path(tmp.path()); write_openai_curated_marketplace(&curated_root, &["slack"]); @@ -2768,14 +2805,14 @@ fn refresh_curated_plugin_cache_replaces_existing_local_version_with_sha() { assert!( tmp.path() .join(format!( - "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_SHA}" + "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_CACHE_VERSION}" )) .is_dir() ); } #[test] -fn refresh_curated_plugin_cache_reinstalls_missing_configured_plugin_with_current_sha() { +fn refresh_curated_plugin_cache_reinstalls_missing_configured_plugin_with_current_short_version() { let tmp = tempfile::tempdir().unwrap(); let curated_root = curated_plugins_repo_path(tmp.path()); write_openai_curated_marketplace(&curated_root, &["slack"]); @@ -2794,7 +2831,7 @@ fn refresh_curated_plugin_cache_reinstalls_missing_configured_plugin_with_curren assert!( tmp.path() .join(format!( - "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_SHA}" + "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_CACHE_VERSION}" )) .is_dir() ); @@ -2849,7 +2886,7 @@ fn refresh_curated_plugin_cache_returns_false_when_configured_plugins_are_curren .unwrap(); write_plugin( &tmp.path().join("plugins/cache/openai-curated"), - &format!("slack/{TEST_CURATED_PLUGIN_SHA}"), + &format!("slack/{TEST_CURATED_PLUGIN_CACHE_VERSION}"), "slack", ); @@ -2859,6 +2896,42 @@ fn refresh_curated_plugin_cache_returns_false_when_configured_plugins_are_curren ); } +#[test] +fn refresh_curated_plugin_cache_migrates_full_sha_cache_version_to_short_version() { + let tmp = tempfile::tempdir().unwrap(); + let curated_root = curated_plugins_repo_path(tmp.path()); + write_openai_curated_marketplace(&curated_root, &["slack"]); + let plugin_id = PluginId::new( + "slack".to_string(), + OPENAI_CURATED_MARKETPLACE_NAME.to_string(), + ) + .unwrap(); + write_plugin( + &tmp.path().join("plugins/cache/openai-curated"), + &format!("slack/{TEST_CURATED_PLUGIN_SHA}"), + "slack", + ); + + assert!( + refresh_curated_plugin_cache(tmp.path(), TEST_CURATED_PLUGIN_SHA, &[plugin_id]) + .expect("cache refresh should migrate the full sha cache version") + ); + assert!( + !tmp.path() + .join(format!( + "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_SHA}" + )) + .exists() + ); + assert!( + tmp.path() + .join(format!( + "plugins/cache/openai-curated/slack/{TEST_CURATED_PLUGIN_CACHE_VERSION}" + )) + .is_dir() + ); +} + #[test] fn refresh_non_curated_plugin_cache_replaces_existing_local_version_with_manifest_version() { let tmp = tempfile::tempdir().unwrap(); diff --git a/codex-rs/core/src/plugins/startup_sync_tests.rs b/codex-rs/core/src/plugins/startup_sync_tests.rs index 8dc2f748f6..fb79d65ae3 100644 --- a/codex-rs/core/src/plugins/startup_sync_tests.rs +++ b/codex-rs/core/src/plugins/startup_sync_tests.rs @@ -1,7 +1,7 @@ use super::*; use crate::config::CONFIG_TOML_FILE; use crate::plugins::PluginsManager; -use crate::plugins::test_support::TEST_CURATED_PLUGIN_SHA; +use crate::plugins::test_support::TEST_CURATED_PLUGIN_CACHE_VERSION; use crate::plugins::test_support::write_curated_plugin_sha; use crate::plugins::test_support::write_file; use crate::plugins::test_support::write_openai_curated_marketplace; @@ -76,7 +76,7 @@ enabled = false assert!( tmp.path() .join(format!( - "plugins/cache/openai-curated/linear/{TEST_CURATED_PLUGIN_SHA}" + "plugins/cache/openai-curated/linear/{TEST_CURATED_PLUGIN_CACHE_VERSION}" )) .is_dir() ); diff --git a/codex-rs/core/src/plugins/test_support.rs b/codex-rs/core/src/plugins/test_support.rs index 8fbaebb803..8c12806140 100644 --- a/codex-rs/core/src/plugins/test_support.rs +++ b/codex-rs/core/src/plugins/test_support.rs @@ -6,6 +6,7 @@ use std::path::Path; use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; pub(crate) const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; +pub(crate) const TEST_CURATED_PLUGIN_CACHE_VERSION: &str = "01234567"; pub(crate) fn write_file(path: &Path, contents: &str) { fs::create_dir_all(path.parent().expect("file should have a parent")).unwrap();