Fix plugin refactor review issues

This commit is contained in:
xli-oai
2026-04-30 01:48:25 -07:00
parent 818838178d
commit 65756a5ebd
8 changed files with 12 additions and 102 deletions
+1 -1
View File
@@ -2102,10 +2102,10 @@ dependencies = [
"codex-connectors",
"codex-core",
"codex-core-plugins",
"codex-features",
"codex-git-utils",
"codex-login",
"codex-model-provider",
"codex-plugin",
"codex-utils-cargo-bin",
"codex-utils-cli",
"pretty_assertions",
+1 -1
View File
@@ -14,10 +14,10 @@ codex-app-server-protocol = { workspace = true }
codex-connectors = { workspace = true }
codex-core = { workspace = true }
codex-core-plugins = { workspace = true }
codex-features = { workspace = true }
codex-git-utils = { workspace = true }
codex-login = { workspace = true }
codex-model-provider = { workspace = true }
codex-plugin = { workspace = true }
codex-utils-cli = { workspace = true }
serde = { workspace = true, features = ["derive"] }
tokio = { workspace = true, features = ["full"] }
+1 -1
View File
@@ -16,11 +16,11 @@ pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_o
pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status;
pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools;
pub use codex_core::connectors::with_app_enabled_state;
use codex_core_plugins::AppConnectorId;
use codex_core_plugins::PluginsManager;
use codex_login::AuthManager;
use codex_login::CodexAuth;
use codex_login::default_client::originator;
use codex_plugin::AppConnectorId;
const DIRECTORY_CONNECTORS_TIMEOUT: Duration = Duration::from_secs(60);
+9 -6
View File
@@ -380,7 +380,7 @@ pub struct PluginsManager {
#[derive(Clone)]
struct CachedPluginLoadOutcome {
config_version: String,
feature_flags: PluginFeatureFlags,
plugin_hooks_enabled: bool,
outcome: PluginLoadOutcome,
}
@@ -462,7 +462,8 @@ impl PluginsManager {
let config_version = version_for_toml(&config_layer_stack.effective_config());
if !force_reload
&& let Some(outcome) = self.cached_enabled_outcome(&config_version, feature_flags)
&& let Some(outcome) =
self.cached_enabled_outcome(&config_version, feature_flags.plugin_hooks_enabled)
{
return outcome;
}
@@ -482,7 +483,7 @@ impl PluginsManager {
};
*cache = Some(CachedPluginLoadOutcome {
config_version,
feature_flags,
plugin_hooks_enabled: feature_flags.plugin_hooks_enabled,
outcome: outcome.clone(),
});
outcome
@@ -538,20 +539,22 @@ impl PluginsManager {
fn cached_enabled_outcome(
&self,
config_version: &str,
feature_flags: PluginFeatureFlags,
plugin_hooks_enabled: bool,
) -> Option<PluginLoadOutcome> {
match self.cached_enabled_outcome.read() {
Ok(cache) => cache
.as_ref()
.filter(|cached| {
cached.config_version == config_version && cached.feature_flags == feature_flags
cached.config_version == config_version
&& cached.plugin_hooks_enabled == plugin_hooks_enabled
})
.map(|cached| cached.outcome.clone()),
Err(err) => err
.into_inner()
.as_ref()
.filter(|cached| {
cached.config_version == config_version && cached.feature_flags == feature_flags
cached.config_version == config_version
&& cached.plugin_hooks_enabled == plugin_hooks_enabled
})
.map(|cached| cached.outcome.clone()),
}
@@ -599,41 +599,6 @@ remote_plugin = true
assert_eq!(outcome.plugins()[0].config_name, "linear@chatgpt-global");
}
#[tokio::test]
async fn remote_installed_cache_reloads_when_remote_feature_enablement_changes() {
let codex_home = TempDir::new().unwrap();
let plugin_base = codex_home
.path()
.join("plugins/cache/chatgpt-global/linear");
write_plugin(&plugin_base, "local", "linear");
write_file(
&codex_home.path().join(CONFIG_TOML_FILE),
r#"[features]
plugins = true
remote_plugin = true
"#,
);
let config = load_config(codex_home.path(), codex_home.path()).await;
let manager = PluginsManager::new(codex_home.path().to_path_buf());
manager.write_remote_installed_plugins_cache(vec![RemoteInstalledPlugin {
marketplace_name: "chatgpt-global".to_string(),
id: "plugins~Plugin_linear".to_string(),
name: "linear".to_string(),
enabled: true,
}]);
let enabled_outcome = manager.plugins_for_test_config(&config).await;
assert_eq!(enabled_outcome.plugins().len(), 1);
let mut feature_flags = config.plugin_feature_flags();
feature_flags.remote_plugins_enabled = false;
let disabled_outcome =
PluginsManager::plugins_for_config(&manager, &config.config_layer_stack, feature_flags)
.await;
assert_eq!(disabled_outcome, PluginLoadOutcome::default());
}
#[tokio::test]
async fn remote_installed_cache_ignores_plugins_missing_local_cache() {
let codex_home = TempDir::new().unwrap();
-19
View File
@@ -1201,25 +1201,6 @@ impl ConfigEditsBuilder {
self
}
pub fn set_plugin_enabled(mut self, plugin_id: &str, enabled: bool) -> Self {
self.edits.push(ConfigEdit::SetPath {
segments: vec![
"plugins".to_string(),
plugin_id.to_string(),
"enabled".to_string(),
],
value: value(enabled),
});
self
}
pub fn clear_plugin(mut self, plugin_id: &str) -> Self {
self.edits.push(ConfigEdit::ClearPath {
segments: vec!["plugins".to_string(), plugin_id.to_string()],
});
self
}
/// Enable or disable a feature flag by key under the `[features]` table.
///
/// Disabling a default-false feature clears the root-scoped key instead of
-33
View File
@@ -1313,39 +1313,6 @@ model_reasoning_effort = "high"
assert_eq!(contents, initial_expected);
}
#[test]
fn blocking_builder_sets_and_clears_plugin_config() {
let tmp = tempdir().expect("tmpdir");
let codex_home = tmp.path();
ConfigEditsBuilder::new(codex_home)
.set_plugin_enabled("sample@test", /*enabled*/ true)
.set_plugin_enabled("other@test", /*enabled*/ false)
.apply_blocking()
.expect("persist plugin config");
ConfigEditsBuilder::new(codex_home)
.clear_plugin("sample@test")
.apply_blocking()
.expect("clear plugin config");
let raw = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config");
let config: TomlValue = toml::from_str(&raw).expect("parse config");
let plugins = config
.get("plugins")
.and_then(TomlValue::as_table)
.expect("plugins table should exist");
assert!(!plugins.contains_key("sample@test"));
assert_eq!(
plugins
.get("other@test")
.and_then(TomlValue::as_table)
.and_then(|plugin| plugin.get("enabled"))
.and_then(TomlValue::as_bool),
Some(false)
);
}
#[tokio::test]
async fn blocking_set_asynchronous_helpers_available() {
let tmp = tempdir().expect("tmpdir");
@@ -1,5 +1,4 @@
use super::*;
use crate::config::edit::ConfigEditsBuilder;
use crate::plugins::test_support::load_plugins_config;
use crate::plugins::test_support::write_curated_plugin_sha;
use crate::plugins::test_support::write_openai_curated_marketplace;
@@ -49,11 +48,6 @@ async fn verified_plugin_suggestion_completed_requires_installed_plugin() {
})
.await
.expect("plugin should install");
ConfigEditsBuilder::new(codex_home.path())
.set_plugin_enabled("sample@openai-curated", /*enabled*/ true)
.apply()
.await
.expect("plugin config should persist");
let refreshed_config = load_plugins_config(codex_home.path()).await;
assert!(verified_plugin_suggestion_completed(