mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
refactor plugin data store seam
This commit is contained in:
@@ -0,0 +1,43 @@
|
||||
use crate::store::PluginStoreError;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::PathBuf;
|
||||
|
||||
const PLUGINS_DATA_DIR: &str = "plugins/data";
|
||||
|
||||
/// Resolves executor-readable writable roots for mutable plugin data.
|
||||
pub trait PluginDataStore: Send + Sync + 'static {
|
||||
/// Returns the writable local root exposed to hooks for one plugin.
|
||||
fn plugin_data_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf;
|
||||
}
|
||||
|
||||
/// Stores mutable plugin data under the local Codex Home layout.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct LocalPluginDataStore {
|
||||
root: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl LocalPluginDataStore {
|
||||
/// Creates the local mutable plugin data store for one Codex Home.
|
||||
pub fn from_codex_home(codex_home: PathBuf) -> Result<Self, PluginStoreError> {
|
||||
let root = AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_DATA_DIR))
|
||||
.map_err(|source| PluginStoreError::Io {
|
||||
context: "failed to resolve plugin data root",
|
||||
source,
|
||||
})?;
|
||||
Ok(Self { root })
|
||||
}
|
||||
}
|
||||
|
||||
impl PluginDataStore for LocalPluginDataStore {
|
||||
fn plugin_data_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf {
|
||||
self.root.join(format!(
|
||||
"{}-{}",
|
||||
plugin_id.plugin_name, plugin_id.marketplace_name
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "data_store_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,16 @@
|
||||
use super::*;
|
||||
use codex_plugin::PluginId;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn local_plugin_data_root_derives_path_from_key() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let store = LocalPluginDataStore::from_codex_home(tmp.path().to_path_buf()).unwrap();
|
||||
let plugin_id = PluginId::new("sample".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
store.plugin_data_root(&plugin_id).as_path(),
|
||||
tmp.path().join("plugins/data/sample-debug")
|
||||
);
|
||||
}
|
||||
@@ -1,3 +1,4 @@
|
||||
pub mod data_store;
|
||||
mod discoverable;
|
||||
pub mod installed_marketplaces;
|
||||
pub mod loader;
|
||||
@@ -24,6 +25,8 @@ pub const OPENAI_BUNDLED_MARKETPLACE_NAME: &str = "openai-bundled";
|
||||
pub type LoadedPlugin = codex_plugin::LoadedPlugin<codex_config::McpServerConfig>;
|
||||
pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome<codex_config::McpServerConfig>;
|
||||
|
||||
pub use data_store::LocalPluginDataStore;
|
||||
pub use data_store::PluginDataStore;
|
||||
pub use discoverable::ToolSuggestDiscoverablePlugin;
|
||||
pub use discoverable::ToolSuggestPluginDiscoveryInput;
|
||||
pub use manager::ConfiguredMarketplace;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::OPENAI_CURATED_MARKETPLACE_NAME;
|
||||
use crate::data_store::PluginDataStore;
|
||||
use crate::manifest::PluginManifestHooks;
|
||||
use crate::manifest::PluginManifestPaths;
|
||||
use crate::manifest::load_plugin_manifest;
|
||||
@@ -113,6 +114,7 @@ pub async fn load_plugins_from_layer_stack(
|
||||
config_layer_stack: &ConfigLayerStack,
|
||||
extra_plugins: HashMap<String, PluginConfig>,
|
||||
store: &PluginStore,
|
||||
data_store: &dyn PluginDataStore,
|
||||
restriction_product: Option<Product>,
|
||||
prefer_remote_curated_conflicts: bool,
|
||||
) -> PluginLoadOutcome<McpServerConfig> {
|
||||
@@ -133,6 +135,7 @@ pub async fn load_plugins_from_layer_stack(
|
||||
configured_name.clone(),
|
||||
&plugin,
|
||||
store,
|
||||
data_store,
|
||||
restriction_product,
|
||||
&skill_config_rules,
|
||||
)
|
||||
@@ -557,6 +560,7 @@ async fn load_plugin(
|
||||
config_name: String,
|
||||
plugin: &PluginConfig,
|
||||
store: &PluginStore,
|
||||
data_store: &dyn PluginDataStore,
|
||||
restriction_product: Option<Product>,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> LoadedPlugin<McpServerConfig> {
|
||||
@@ -659,7 +663,7 @@ async fn load_plugin(
|
||||
let (hook_sources, hook_load_warnings) = load_plugin_hooks(
|
||||
&plugin_root,
|
||||
&loaded_plugin_id,
|
||||
&store.plugin_data_root(&loaded_plugin_id),
|
||||
&data_store.plugin_data_root(&loaded_plugin_id),
|
||||
manifest_paths,
|
||||
);
|
||||
loaded_plugin.hook_sources = hook_sources;
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use super::PluginLoadOutcome;
|
||||
use super::startup_remote_sync::start_startup_remote_plugin_sync_once;
|
||||
use crate::OPENAI_CURATED_MARKETPLACE_NAME;
|
||||
use crate::data_store::LocalPluginDataStore;
|
||||
use crate::data_store::PluginDataStore;
|
||||
use crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack;
|
||||
use crate::loader::configured_curated_plugin_ids_from_codex_home;
|
||||
use crate::loader::curated_plugin_cache_version;
|
||||
@@ -398,6 +400,7 @@ impl From<RemotePluginFetchError> for PluginRemoteSyncError {
|
||||
pub struct PluginsManager {
|
||||
codex_home: PathBuf,
|
||||
store: PluginStore,
|
||||
data_store: Arc<dyn PluginDataStore>,
|
||||
featured_plugin_ids_cache: RwLock<Option<CachedFeaturedPluginIds>>,
|
||||
configured_marketplace_upgrade_state: RwLock<ConfiguredMarketplaceUpgradeState>,
|
||||
non_curated_cache_refresh_state: RwLock<NonCuratedCacheRefreshState>,
|
||||
@@ -420,9 +423,34 @@ impl PluginsManager {
|
||||
Self::new_with_restriction_product(codex_home, Some(Product::Codex))
|
||||
}
|
||||
|
||||
/// Creates a plugins manager with an alternative mutable plugin data backend.
|
||||
pub fn new_with_data_store(codex_home: PathBuf, data_store: Arc<dyn PluginDataStore>) -> Self {
|
||||
Self::new_with_restriction_product_and_data_store(
|
||||
codex_home,
|
||||
Some(Product::Codex),
|
||||
data_store,
|
||||
)
|
||||
}
|
||||
|
||||
pub fn new_with_restriction_product(
|
||||
codex_home: PathBuf,
|
||||
restriction_product: Option<Product>,
|
||||
) -> Self {
|
||||
let data_store = Arc::new(
|
||||
LocalPluginDataStore::from_codex_home(codex_home.clone())
|
||||
.unwrap_or_else(|err| panic!("plugin data root should be absolute: {err}")),
|
||||
);
|
||||
Self::new_with_restriction_product_and_data_store(
|
||||
codex_home,
|
||||
restriction_product,
|
||||
data_store,
|
||||
)
|
||||
}
|
||||
|
||||
fn new_with_restriction_product_and_data_store(
|
||||
codex_home: PathBuf,
|
||||
restriction_product: Option<Product>,
|
||||
data_store: Arc<dyn PluginDataStore>,
|
||||
) -> Self {
|
||||
// Product restrictions are enforced at marketplace admission time for a given CODEX_HOME:
|
||||
// listing, install, and curated refresh all consult this restriction context before new
|
||||
@@ -434,6 +462,7 @@ impl PluginsManager {
|
||||
Self {
|
||||
codex_home: codex_home.clone(),
|
||||
store: PluginStore::new(codex_home),
|
||||
data_store,
|
||||
featured_plugin_ids_cache: RwLock::new(None),
|
||||
configured_marketplace_upgrade_state: RwLock::new(
|
||||
ConfiguredMarketplaceUpgradeState::default(),
|
||||
@@ -491,6 +520,7 @@ impl PluginsManager {
|
||||
&config.config_layer_stack,
|
||||
self.remote_installed_plugin_configs(),
|
||||
&self.store,
|
||||
self.data_store.as_ref(),
|
||||
self.restriction_product,
|
||||
config.remote_plugin_enabled,
|
||||
)
|
||||
@@ -537,6 +567,7 @@ impl PluginsManager {
|
||||
config_layer_stack,
|
||||
self.remote_installed_plugin_configs(),
|
||||
&self.store,
|
||||
self.data_store.as_ref(),
|
||||
self.restriction_product,
|
||||
config.remote_plugin_enabled,
|
||||
)
|
||||
@@ -1456,7 +1487,7 @@ impl PluginsManager {
|
||||
),
|
||||
)
|
||||
.await;
|
||||
let plugin_data_root = self.store.plugin_data_root(&plugin_id);
|
||||
let plugin_data_root = self.data_store.plugin_data_root(&plugin_id);
|
||||
let (hook_sources, _hook_load_warnings) =
|
||||
load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths);
|
||||
let hooks = plugin_hook_declarations(&hook_sources)
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use super::*;
|
||||
use crate::LoadedPlugin;
|
||||
use crate::PluginLoadOutcome;
|
||||
use crate::data_store::LocalPluginDataStore;
|
||||
use crate::data_store::PluginDataStore;
|
||||
use crate::installed_marketplaces::marketplace_install_root;
|
||||
use crate::loader::load_plugins_from_layer_stack;
|
||||
use crate::loader::refresh_non_curated_plugin_cache;
|
||||
@@ -29,10 +31,12 @@ use codex_config::types::McpServerTransportConfig;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use tempfile::TempDir;
|
||||
use toml::Value;
|
||||
use wiremock::Mock;
|
||||
@@ -45,6 +49,17 @@ use wiremock::matchers::query_param;
|
||||
|
||||
const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct FakePluginDataStore {
|
||||
root: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl PluginDataStore for FakePluginDataStore {
|
||||
fn plugin_data_root(&self, _plugin_id: &PluginId) -> AbsolutePathBuf {
|
||||
self.root.clone()
|
||||
}
|
||||
}
|
||||
|
||||
fn write_plugin_with_version(
|
||||
root: &Path,
|
||||
dir_name: &str,
|
||||
@@ -3851,6 +3866,7 @@ async fn load_plugins_ignores_project_config_files() {
|
||||
&stack,
|
||||
std::collections::HashMap::new(),
|
||||
&PluginStore::new(codex_home.path().to_path_buf()),
|
||||
&LocalPluginDataStore::from_codex_home(codex_home.path().to_path_buf()).unwrap(),
|
||||
Some(Product::Codex),
|
||||
/*prefer_remote_curated_conflicts*/ false,
|
||||
)
|
||||
@@ -3858,3 +3874,62 @@ async fn load_plugins_ignores_project_config_files() {
|
||||
|
||||
assert_eq!(outcome, PluginLoadOutcome::default());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugins_for_layer_stack_uses_injected_plugin_data_store() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/sample/local");
|
||||
let config_path = AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE)).unwrap();
|
||||
let data_root =
|
||||
AbsolutePathBuf::try_from(codex_home.path().join("injected-plugin-data")).unwrap();
|
||||
|
||||
write_file(
|
||||
&plugin_root.join(".codex-plugin/plugin.json"),
|
||||
r#"{"name":"sample"}"#,
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join("hooks/hooks.json"),
|
||||
r#"{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"echo startup"}]}]}}"#,
|
||||
);
|
||||
let stack = ConfigLayerStack::new(
|
||||
vec![ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: config_path,
|
||||
profile: None,
|
||||
},
|
||||
toml::from_str(&plugin_config_toml(
|
||||
/*enabled*/ true, /*plugins_feature_enabled*/ true,
|
||||
))
|
||||
.expect("user config should parse"),
|
||||
)],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("config layer stack should build");
|
||||
let config = PluginsConfigInput::new(
|
||||
stack.clone(),
|
||||
/*plugins_enabled*/ true,
|
||||
/*remote_plugin_enabled*/ false,
|
||||
"https://chatgpt.com/backend-api/".to_string(),
|
||||
);
|
||||
let outcome = PluginsManager::new_with_data_store(
|
||||
codex_home.path().to_path_buf(),
|
||||
Arc::new(FakePluginDataStore {
|
||||
root: data_root.clone(),
|
||||
}),
|
||||
)
|
||||
.plugins_for_layer_stack(&stack, &config)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
outcome
|
||||
.effective_plugin_hook_sources()
|
||||
.into_iter()
|
||||
.map(|source| source.plugin_data_root)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![data_root]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -15,7 +15,6 @@ 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";
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct PluginInstallResult {
|
||||
@@ -27,7 +26,6 @@ pub struct PluginInstallResult {
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PluginStore {
|
||||
root: AbsolutePathBuf,
|
||||
data_root: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl PluginStore {
|
||||
@@ -39,11 +37,7 @@ impl PluginStore {
|
||||
pub fn try_new(codex_home: PathBuf) -> Result<Self, PluginStoreError> {
|
||||
let root = AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_CACHE_DIR))
|
||||
.map_err(|err| PluginStoreError::io("failed to resolve plugin cache root", err))?;
|
||||
let data_root =
|
||||
AbsolutePathBuf::from_absolute_path_checked(codex_home.join(PLUGINS_DATA_DIR))
|
||||
.map_err(|err| PluginStoreError::io("failed to resolve plugin data root", err))?;
|
||||
|
||||
Ok(Self { root, data_root })
|
||||
Ok(Self { root })
|
||||
}
|
||||
|
||||
pub fn root(&self) -> &AbsolutePathBuf {
|
||||
@@ -60,13 +54,6 @@ impl PluginStore {
|
||||
self.plugin_base_root(plugin_id).join(plugin_version)
|
||||
}
|
||||
|
||||
pub fn plugin_data_root(&self, plugin_id: &PluginId) -> AbsolutePathBuf {
|
||||
self.data_root.join(format!(
|
||||
"{}-{}",
|
||||
plugin_id.plugin_name, plugin_id.marketplace_name
|
||||
))
|
||||
}
|
||||
|
||||
pub fn active_plugin_version(&self, plugin_id: &PluginId) -> Option<String> {
|
||||
let mut discovered_versions = fs::read_dir(self.plugin_base_root(plugin_id).as_path())
|
||||
.ok()?
|
||||
|
||||
@@ -109,18 +109,6 @@ fn plugin_root_derives_path_from_key_and_version() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_data_root_derives_path_from_key() {
|
||||
let tmp = tempdir().unwrap();
|
||||
let store = PluginStore::new(tmp.path().to_path_buf());
|
||||
let plugin_id = PluginId::new("sample".to_string(), "debug".to_string()).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
store.plugin_data_root(&plugin_id).as_path(),
|
||||
tmp.path().join("plugins/data/sample-debug")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn install_with_version_uses_requested_cache_version() {
|
||||
let tmp = tempdir().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user