mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: Use short SHA versions for curated plugin cache entries (#19095)
Curated plugin cache entries now use an 8-character SHA prefix, instead of the full SHA, as the cache folder version number.
This commit is contained in:
committed by
GitHub
Unverified
parent
a9c111da54
commit
53be451673
@@ -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<bool, String> {
|
||||
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<String, PluginConfig> {
|
||||
@@ -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");
|
||||
|
||||
@@ -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());
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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()
|
||||
);
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user