Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -612,15 +612,10 @@ impl CatalogRequestProcessor {
.await;
let plugins_enabled =
config.features.enabled(Feature::Plugins) && workspace_codex_plugins_enabled;
let plugin_outcome = if plugins_enabled && config.features.enabled(Feature::PluginHooks)
{
let plugin_outcome = if plugins_enabled {
let plugins_input = config.plugins_config_input();
plugins_manager
.plugins_for_layer_stack(
&config.config_layer_stack,
&plugins_input,
/*plugin_hooks_feature_enabled*/ true,
)
.plugins_for_layer_stack(&config.config_layer_stack, &plugins_input)
.await
} else {
PluginLoadOutcome::default()
Expand Down
1 change: 0 additions & 1 deletion codex-rs/app-server/tests/suite/v2/hooks_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ fn write_plugin_hook_config(codex_home: &std::path::Path, hooks_json: &str) -> R
codex_home.join("config.toml"),
r#"[features]
plugins = true
plugin_hooks = true
hooks = true

[plugins."demo@test"]
Expand Down
1 change: 0 additions & 1 deletion codex-rs/app-server/tests/suite/v2/plugin_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,6 @@ description: Visible only for ChatGPT
codex_home.path().join("config.toml"),
r#"[features]
plugins = true
plugin_hooks = true

[[skills.config]]
name = "demo-plugin:thread-summarizer"
Expand Down
21 changes: 8 additions & 13 deletions codex-rs/core-plugins/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ pub async fn load_plugins_from_layer_stack(
extra_plugins: HashMap<String, PluginConfig>,
store: &PluginStore,
restriction_product: Option<Product>,
plugin_hooks_enabled: bool,
) -> PluginLoadOutcome<McpServerConfig> {
let skill_config_rules = skill_config_rules_from_stack(config_layer_stack);
let mut configured_plugins = configured_plugins_from_stack(config_layer_stack);
Expand All @@ -129,7 +128,6 @@ pub async fn load_plugins_from_layer_stack(
store,
restriction_product,
&skill_config_rules,
plugin_hooks_enabled,
)
.await;
for name in loaded_plugin.mcp_servers.keys() {
Expand Down Expand Up @@ -499,7 +497,6 @@ async fn load_plugin(
store: &PluginStore,
restriction_product: Option<Product>,
skill_config_rules: &SkillConfigRules,
plugin_hooks_enabled: bool,
) -> LoadedPlugin<McpServerConfig> {
let plugin_id = PluginId::parse(&config_name);
let active_plugin_root = plugin_id
Expand Down Expand Up @@ -597,16 +594,14 @@ async fn load_plugin(
}
loaded_plugin.mcp_servers = mcp_servers;
loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await;
if plugin_hooks_enabled {
let (hook_sources, hook_load_warnings) = load_plugin_hooks(
&plugin_root,
&loaded_plugin_id,
&store.plugin_data_root(&loaded_plugin_id),
manifest_paths,
);
loaded_plugin.hook_sources = hook_sources;
loaded_plugin.hook_load_warnings = hook_load_warnings;
}
let (hook_sources, hook_load_warnings) = load_plugin_hooks(
&plugin_root,
&loaded_plugin_id,
&store.plugin_data_root(&loaded_plugin_id),
manifest_paths,
);
loaded_plugin.hook_sources = hook_sources;
loaded_plugin.hook_load_warnings = hook_load_warnings;
loaded_plugin
}

Expand Down
56 changes: 15 additions & 41 deletions codex-rs/core-plugins/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ pub struct PluginsConfigInput {
pub config_layer_stack: ConfigLayerStack,
pub plugins_enabled: bool,
pub remote_plugin_enabled: bool,
pub plugin_hooks_enabled: bool,
pub chatgpt_base_url: String,
}

Expand All @@ -99,14 +98,12 @@ impl PluginsConfigInput {
config_layer_stack: ConfigLayerStack,
plugins_enabled: bool,
remote_plugin_enabled: bool,
plugin_hooks_enabled: bool,
chatgpt_base_url: String,
) -> Self {
Self {
config_layer_stack,
plugins_enabled,
remote_plugin_enabled,
plugin_hooks_enabled,
chatgpt_base_url,
}
}
Expand Down Expand Up @@ -415,7 +412,6 @@ pub struct PluginsManager {
#[derive(Clone)]
struct CachedPluginLoadOutcome {
config_version: String,
plugin_hooks_enabled: bool,
outcome: PluginLoadOutcome,
}

Expand Down Expand Up @@ -486,12 +482,8 @@ impl PluginsManager {
return PluginLoadOutcome::default();
}

let plugin_hooks_enabled = config.plugin_hooks_enabled;
let config_version = version_for_toml(&config.config_layer_stack.effective_config());
if !force_reload
&& let Some(outcome) =
self.cached_enabled_outcome(&config_version, plugin_hooks_enabled)
{
if !force_reload && let Some(outcome) = self.cached_enabled_outcome(&config_version) {
return outcome;
}

Expand All @@ -500,7 +492,6 @@ impl PluginsManager {
self.remote_installed_plugin_configs(),
&self.store,
self.restriction_product,
plugin_hooks_enabled,
)
.await;
log_plugin_load_errors(&outcome);
Expand All @@ -510,7 +501,6 @@ impl PluginsManager {
};
*cache = Some(CachedPluginLoadOutcome {
config_version,
plugin_hooks_enabled,
outcome: outcome.clone(),
});
outcome
Expand Down Expand Up @@ -538,7 +528,6 @@ impl PluginsManager {
&self,
config_layer_stack: &ConfigLayerStack,
config: &PluginsConfigInput,
plugin_hooks_feature_enabled: bool,
) -> PluginLoadOutcome {
if !config.plugins_enabled {
return PluginLoadOutcome::default();
Expand All @@ -548,7 +537,6 @@ impl PluginsManager {
self.remote_installed_plugin_configs(),
&self.store,
self.restriction_product,
plugin_hooks_feature_enabled,
)
.await
}
Expand All @@ -559,31 +547,21 @@ impl PluginsManager {
config_layer_stack: &ConfigLayerStack,
config: &PluginsConfigInput,
) -> Vec<PluginSkillRoot> {
self.plugins_for_layer_stack(config_layer_stack, config, config.plugin_hooks_enabled)
self.plugins_for_layer_stack(config_layer_stack, config)
.await
.effective_plugin_skill_roots()
}

fn cached_enabled_outcome(
&self,
config_version: &str,
plugin_hooks_enabled: bool,
) -> Option<PluginLoadOutcome> {
fn cached_enabled_outcome(&self, config_version: &str) -> Option<PluginLoadOutcome> {
match self.cached_enabled_outcome.read() {
Ok(cache) => cache
.as_ref()
.filter(|cached| {
cached.config_version == config_version
&& cached.plugin_hooks_enabled == plugin_hooks_enabled
})
.filter(|cached| cached.config_version == config_version)
.map(|cached| cached.outcome.clone()),
Err(err) => err
.into_inner()
.as_ref()
.filter(|cached| {
cached.config_version == config_version
&& cached.plugin_hooks_enabled == plugin_hooks_enabled
})
.filter(|cached| cached.config_version == config_version)
.map(|cached| cached.outcome.clone()),
}
}
Expand Down Expand Up @@ -1451,20 +1429,16 @@ impl PluginsManager {
),
)
.await;
let hooks = if config.plugin_hooks_enabled {
let plugin_data_root = self.store.plugin_data_root(&plugin_id);
let (hook_sources, _hook_load_warnings) =
load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths);
plugin_hook_declarations(&hook_sources)
.into_iter()
.map(|hook| PluginHookSummary {
key: hook.key,
event_name: hook.event_name,
})
.collect()
} else {
Vec::new()
};
let plugin_data_root = self.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)
.into_iter()
.map(|hook| PluginHookSummary {
key: hook.key,
event_name: hook.event_name,
})
.collect();
let apps = load_plugin_apps(source_path.as_path()).await;
let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path())
.await
Expand Down
73 changes: 0 additions & 73 deletions codex-rs/core-plugins/src/manager_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,29 +101,13 @@ fn run_git(repo: &Path, args: &[&str]) {
}

fn plugin_config_toml(enabled: bool, plugins_feature_enabled: bool) -> String {
plugin_config_toml_with_plugin_hooks(
enabled,
plugins_feature_enabled,
/*plugin_hooks_feature_enabled*/ false,
)
}

fn plugin_config_toml_with_plugin_hooks(
enabled: bool,
plugins_feature_enabled: bool,
plugin_hooks_feature_enabled: bool,
) -> String {
let mut root = toml::map::Map::new();

let mut features = toml::map::Map::new();
features.insert(
"plugins".to_string(),
Value::Boolean(plugins_feature_enabled),
);
features.insert(
"plugin_hooks".to_string(),
Value::Boolean(plugin_hooks_feature_enabled),
);
root.insert("features".to_string(), Value::Table(features));

let mut plugin = toml::map::Map::new();
Expand Down Expand Up @@ -1133,61 +1117,6 @@ async fn load_plugins_returns_empty_when_feature_disabled() {
assert_eq!(outcome, PluginLoadOutcome::default());
}

#[tokio::test]
async fn plugins_for_config_reloads_when_plugin_hooks_enablement_changes() {
let codex_home = TempDir::new().unwrap();
let plugin_root = codex_home
.path()
.join("plugins/cache")
.join("test/sample/local");

write_file(
&plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
);
write_file(
&plugin_root.join("hooks/hooks.json"),
r#"{
"hooks": {
"PreToolUse": [
{
"hooks": [{ "type": "command", "command": "echo plugin hook" }]
}
]
}
}"#,
);

let manager = PluginsManager::new(codex_home.path().to_path_buf());
write_file(
&codex_home.path().join(CONFIG_TOML_FILE),
&plugin_config_toml_with_plugin_hooks(
/*enabled*/ true, /*plugins_feature_enabled*/ true,
/*plugin_hooks_feature_enabled*/ false,
),
);
let config_without_plugin_hooks = load_config(codex_home.path(), codex_home.path()).await;
let without_plugin_hooks = manager
.plugins_for_config(&config_without_plugin_hooks)
.await;
assert!(
without_plugin_hooks
.effective_plugin_hook_sources()
.is_empty()
);

write_file(
&codex_home.path().join(CONFIG_TOML_FILE),
&plugin_config_toml_with_plugin_hooks(
/*enabled*/ true, /*plugins_feature_enabled*/ true,
/*plugin_hooks_feature_enabled*/ true,
),
);
let config_with_plugin_hooks = load_config(codex_home.path(), codex_home.path()).await;
let with_plugin_hooks = manager.plugins_for_config(&config_with_plugin_hooks).await;
assert_eq!(with_plugin_hooks.effective_plugin_hook_sources().len(), 1);
}

#[tokio::test]
async fn load_plugins_rejects_invalid_plugin_keys() {
let codex_home = TempDir::new().unwrap();
Expand Down Expand Up @@ -2032,7 +1961,6 @@ async fn read_plugin_for_config_installed_git_source_reads_from_cache_without_cl
&tmp.path().join(CONFIG_TOML_FILE),
r#"[features]
plugins = true
plugin_hooks = true

[plugins."toolkit@debug"]
enabled = true
Expand Down Expand Up @@ -3741,7 +3669,6 @@ async fn load_plugins_ignores_project_config_files() {
std::collections::HashMap::new(),
&PluginStore::new(codex_home.path().to_path_buf()),
Some(Product::Codex),
/*plugin_hooks_enabled*/ false,
)
.await;

Expand Down
5 changes: 0 additions & 5 deletions codex-rs/core-plugins/src/test_support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,6 @@ pub(crate) async fn load_plugins_config(codex_home: &Path, cwd: &Path) -> Plugin
"remote_plugin",
/*default_enabled*/ false,
),
feature_enabled(
&effective_config,
"plugin_hooks",
/*default_enabled*/ false,
),
"https://chatgpt.com/backend-api/".to_string(),
)
}
Expand Down
1 change: 0 additions & 1 deletion codex-rs/core/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1282,7 +1282,6 @@ impl Config {
self.config_layer_stack.clone(),
self.features.enabled(Feature::Plugins),
self.features.enabled(Feature::RemotePlugin),
self.features.enabled(Feature::PluginHooks),
self.chatgpt_base_url.clone(),
)
}
Expand Down
15 changes: 4 additions & 11 deletions codex-rs/core/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3319,17 +3319,10 @@ async fn build_hooks_for_config(
let mut hook_shell_argv = user_shell.derive_exec_args("", /*use_login_shell*/ false);
let hook_shell_program = hook_shell_argv.remove(0);
let _ = hook_shell_argv.pop();
let plugin_hooks_enabled = config.features.enabled(Feature::PluginHooks);
let (plugin_hook_sources, plugin_hook_load_warnings) = if plugin_hooks_enabled {
let plugins_input = config.plugins_config_input();
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
(
plugin_outcome.effective_plugin_hook_sources(),
plugin_outcome.effective_plugin_hook_warnings(),
)
} else {
(Vec::new(), Vec::new())
};
let plugins_input = config.plugins_config_input();
let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await;
let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources();
let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings();
Hooks::new(HooksConfig {
legacy_notify_argv: config.notify.clone(),
feature_enabled: config.features.enabled(Feature::CodexHooks),
Expand Down
4 changes: 0 additions & 4 deletions codex-rs/core/tests/suite/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2934,10 +2934,6 @@ print(json.dumps({{
.enable(Feature::Plugins)
.expect("test config should allow feature update");
trust_plugin_hooks(config, plugin_hook_sources);
config
.features
.enable(Feature::PluginHooks)
.expect("test config should allow feature update");
});
let test = builder.build(&server).await?;

Expand Down
Loading
Loading