openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-08 15:50:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove plugin hook load warnings from discovery

Browse Source
This commit is contained in:
Abhinav Vedmala
2026-04-26 13:32:55 -07:00
parent b2f2c98a7c
commit d2097f9802
8 changed files with 44 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

83
codex-rs/core-plugins/src/loader.rs
View File

@@ -483,7 +483,6 @@ async fn load_plugin(
mcp_servers: HashMap::new(),
apps: Vec::new(),
hook_sources: Vec::new(),
hook_load_warnings: Vec::new(),
error: None,
};
@@ -552,9 +551,7 @@ async fn load_plugin(
}
loaded_plugin.mcp_servers = mcp_servers;
loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await;
let hook_discovery = load_plugin_hooks(&plugin_root, &loaded_plugin_id, manifest_paths);
loaded_plugin.hook_sources = hook_discovery.sources;
loaded_plugin.hook_load_warnings = hook_discovery.warnings;
loaded_plugin.hook_sources = load_plugin_hooks(&plugin_root, &loaded_plugin_id, manifest_paths);
loaded_plugin
}
@@ -684,22 +681,19 @@ fn default_app_config_paths(plugin_root: &Path) -> Vec<AbsolutePathBuf> {
paths
}
#[derive(Debug, Default)]
pub struct PluginHookDiscovery {
pub sources: Vec<PluginHookSource>,
pub warnings: Vec<String>,
}
// Discover plugin-bundled hooks from manifest `hooks` entries when present
// (path, paths, inline object, or inline objects), otherwise from the default
// `hooks/hooks.json` file.
pub fn load_plugin_hooks(
plugin_root: &AbsolutePathBuf,
plugin_id: &PluginId,
manifest_paths: &PluginManifestPaths,
) -> PluginHookDiscovery {
let mut discovery = PluginHookDiscovery::default();
) -> Vec<PluginHookSource> {
let mut sources = Vec::new();
match &manifest_paths.hooks {
Some(PluginManifestHooks::Paths(paths)) => {
for path in paths {
append_plugin_hook_file(plugin_root, plugin_id, path, &mut discovery);
append_plugin_hook_file(plugin_root, plugin_id, path, &mut sources);
}
}
Some(PluginManifestHooks::Inline(hooks_files)) => {
@@ -710,7 +704,7 @@ pub fn load_plugin_hooks(
if hooks_file.hooks.is_empty() {
continue;
}
discovery.sources.push(PluginHookSource {
sources.push(PluginHookSource {
plugin_id: plugin_id.clone(),
plugin_root: plugin_root.clone(),
source_path: manifest_path.clone(),
@@ -722,36 +716,38 @@ pub fn load_plugin_hooks(
None => {
let default_path = plugin_root.join(DEFAULT_HOOKS_CONFIG_FILE);
if default_path.as_path().is_file() {
append_plugin_hook_file(plugin_root, plugin_id, &default_path, &mut discovery);
append_plugin_hook_file(plugin_root, plugin_id, &default_path, &mut sources);
}
}
}
discovery
sources
}
// Load one resolved plugin hook file and keep source metadata with its parsed
// hook events so runtime discovery can report plugin-originated hook runs.
fn append_plugin_hook_file(
plugin_root: &AbsolutePathBuf,
plugin_id: &PluginId,
path: &AbsolutePathBuf,
discovery: &mut PluginHookDiscovery,
sources: &mut Vec<PluginHookSource>,
) {
let contents = match fs::read_to_string(path.as_path()) {
Ok(contents) => contents,
Err(err) => {
discovery.warnings.push(format!(
"failed to read plugin hooks config {}: {err}",
path.display()
));
warn!(
path = %path.display(),
"failed to read plugin hooks config: {err}"
);
return;
}
};
let parsed = match serde_json::from_str::<HooksFile>(&contents) {
Ok(parsed) => parsed,
Err(err) => {
discovery.warnings.push(format!(
"failed to parse plugin hooks config {}: {err}",
path.display()
));
warn!(
path = %path.display(),
"failed to parse plugin hooks config: {err}"
);
return;
}
};
@@ -759,7 +755,7 @@ fn append_plugin_hook_file(
return;
}
discovery.sources.push(PluginHookSource {
sources.push(PluginHookSource {
plugin_id: plugin_id.clone(),
plugin_root: plugin_root.clone(),
source_path: path.clone(),
@@ -1241,19 +1237,15 @@ mod tests {
let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest");
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths);
let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths);
assert_eq!(discovery.warnings, Vec::<String>::new());
assert_eq!(discovery.sources.len(), 1);
assert_eq!(sources.len(), 1);
assert_eq!(
discovery.sources[0].plugin_id,
sources[0].plugin_id,
PluginId::parse("demo-plugin@test-marketplace").expect("plugin id")
);
assert_eq!(
discovery.sources[0].source_relative_path,
"hooks/hooks.json"
);
assert_eq!(discovery.sources[0].hooks.handler_count(), 1);
assert_eq!(sources[0].source_relative_path, "hooks/hooks.json");
assert_eq!(sources[0].hooks.handler_count(), 1);
}
#[test]
@@ -1313,20 +1305,17 @@ mod tests {
let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest");
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths);
let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths);
assert_eq!(discovery.warnings, Vec::<String>::new());
assert_eq!(
discovery
.sources
sources
.iter()
.map(|source| source.source_relative_path.as_str())
.collect::<Vec<_>>(),
vec!["hooks/one.json", "hooks/two.json"]
);
assert_eq!(
discovery
.sources
sources
.iter()
.map(|source| source.hooks.handler_count())
.collect::<Vec<_>>(),
@@ -1360,15 +1349,11 @@ mod tests {
let manifest = load_plugin_manifest(plugin_root.as_path()).expect("manifest");
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
let discovery = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths);
let sources = load_plugin_hooks(&plugin_root, &plugin_id, &manifest.paths);
assert_eq!(discovery.warnings, Vec::<String>::new());
assert_eq!(discovery.sources.len(), 1);
assert_eq!(
discovery.sources[0].source_relative_path,
"plugin.json#hooks[0]"
);
assert_eq!(discovery.sources[0].hooks.handler_count(), 1);
assert_eq!(sources.len(), 1);
assert_eq!(sources[0].source_relative_path, "plugin.json#hooks[0]");
assert_eq!(sources[0].hooks.handler_count(), 1);
}
#[test]

3
codex-rs/core/src/plugins/manager_tests.rs
View File

@@ -220,7 +220,6 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() {
)]),
apps: vec![AppConnectorId("connector_example".to_string())],
hook_sources: Vec::new(),
hook_load_warnings: Vec::new(),
error: None,
}]
);
@@ -722,7 +721,6 @@ async fn load_plugins_preserves_disabled_plugins_without_effective_contributions
mcp_servers: HashMap::new(),
apps: Vec::new(),
hook_sources: Vec::new(),
hook_load_warnings: Vec::new(),
error: None,
}]
);
@@ -841,7 +839,6 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() {
mcp_servers: HashMap::new(),
apps: Vec::new(),
hook_sources: Vec::new(),
hook_load_warnings: Vec::new(),
error: None,
};
let summary = |config_name: &str, display_name: &str| PluginCapabilitySummary {

10
codex-rs/core/src/session/session.rs
View File

@@ -691,21 +691,17 @@ impl Session {
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 plugin_hook_sources = if plugin_hooks_enabled {
let plugin_outcome = plugins_manager.plugins_for_config(&config).await;
(
plugin_outcome.effective_plugin_hook_sources(),
plugin_outcome.effective_plugin_hook_warnings(),
)
plugin_outcome.effective_plugin_hook_sources()
} else {
(Vec::new(), Vec::new())
Vec::new()
};
let hooks = Hooks::new(HooksConfig {
legacy_notify_argv: config.notify.clone(),
feature_enabled: config.features.enabled(Feature::CodexHooks),
config_layer_stack: Some(config.config_layer_stack.clone()),
plugin_hook_sources,
plugin_hook_load_warnings,
shell_program: Some(hook_shell_program),
shell_args: hook_shell_argv,
});

8
codex-rs/hooks/src/engine/discovery.rs
View File

@@ -38,11 +38,10 @@ struct HookHandlerSource<'a> {
pub(crate) fn discover_handlers(
config_layer_stack: Option<&ConfigLayerStack>,
plugin_hook_sources: Vec<PluginHookSource>,
plugin_hook_load_warnings: Vec<String>,
) -> DiscoveryResult {
let Some(config_layer_stack) = config_layer_stack else {
let mut handlers = Vec::new();
let mut warnings = plugin_hook_load_warnings;
let mut warnings = Vec::new();
let mut display_order = 0_i64;
append_plugin_hook_sources(
&mut handlers,
@@ -54,7 +53,7 @@ pub(crate) fn discover_handlers(
};
let mut handlers = Vec::new();
let mut warnings = plugin_hook_load_warnings;
let mut warnings = Vec::new();
let mut display_order = 0_i64;
append_managed_requirement_handlers(
@@ -169,7 +168,8 @@ fn append_plugin_hook_sources(
} = source;
let mut env = HashMap::new();
let plugin_root_value = plugin_root.display().to_string();
env.insert("AGENTS_PLUGIN_ROOT".to_string(), plugin_root_value.clone());
env.insert("PLUGIN_ROOT".to_string(), plugin_root_value.clone());
// For OOTB compat with existing plugins that use this env var.
env.insert("CLAUDE_PLUGIN_ROOT".to_string(), plugin_root_value);
append_hook_events(
handlers,

7
codex-rs/hooks/src/engine/mod.rs
View File

@@ -79,7 +79,6 @@ impl ClaudeHooksEngine {
enabled: bool,
config_layer_stack: Option<&ConfigLayerStack>,
plugin_hook_sources: Vec<PluginHookSource>,
plugin_hook_load_warnings: Vec<String>,
shell: CommandShell,
) -> Self {
if !enabled {
@@ -91,11 +90,7 @@ impl ClaudeHooksEngine {
}
let _ = schema_loader::generated_hook_schemas();
let discovered = discovery::discover_handlers(
config_layer_stack,
plugin_hook_sources,
plugin_hook_load_warnings,
);
let discovered = discovery::discover_handlers(config_layer_stack, plugin_hook_sources);
Self {
handlers: discovered.handlers,
warnings: discovered.warnings,

8
codex-rs/hooks/src/engine/mod_tests.rs
View File

@@ -109,7 +109,6 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
@@ -194,7 +193,6 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
@@ -303,7 +301,6 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
/*enabled*/ true,
Some(&config_layer_stack),
Vec::new(),
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
@@ -352,7 +349,7 @@ import os
from pathlib import Path
Path(r"{log_path}").write_text(json.dumps({{
"agents": os.environ.get("AGENTS_PLUGIN_ROOT"),
"plugin": os.environ.get("PLUGIN_ROOT"),
"claude": os.environ.get("CLAUDE_PLUGIN_ROOT"),
}}), encoding="utf-8")
"#,
@@ -383,7 +380,6 @@ Path(r"{log_path}").write_text(json.dumps({{
/*enabled*/ true,
None,
plugin_hook_sources,
Vec::new(),
CommandShell {
program: String::new(),
args: Vec::new(),
@@ -429,7 +425,7 @@ Path(r"{log_path}").write_text(json.dumps({{
assert_eq!(
logged,
serde_json::json!({
"agents": plugin_root.display().to_string(),
"plugin": plugin_root.display().to_string(),
"claude": plugin_root.display().to_string(),
})
);

2
codex-rs/hooks/src/registry.rs
View File

@@ -27,7 +27,6 @@ pub struct HooksConfig {
pub feature_enabled: bool,
pub config_layer_stack: Option<ConfigLayerStack>,
pub plugin_hook_sources: Vec<PluginHookSource>,
pub plugin_hook_load_warnings: Vec<String>,
pub shell_program: Option<String>,
pub shell_args: Vec<String>,
}
@@ -57,7 +56,6 @@ impl Hooks {
config.feature_enabled,
config.config_layer_stack.as_ref(),
config.plugin_hook_sources,
config.plugin_hook_load_warnings,
CommandShell {
program: config.shell_program.unwrap_or_default(),
args: config.shell_args,

9
codex-rs/plugin/src/load_outcome.rs
View File

@@ -23,7 +23,6 @@ pub struct LoadedPlugin<M> {
pub mcp_servers: HashMap<String, M>,
pub apps: Vec<AppConnectorId>,
pub hook_sources: Vec<PluginHookSource>,
pub hook_load_warnings: Vec<String>,
pub error: Option<String>,
}
@@ -151,14 +150,6 @@ impl<M: Clone> PluginLoadOutcome<M> {
.collect()
}
pub fn effective_plugin_hook_warnings(&self) -> Vec<String> {
self.plugins
.iter()
.filter(|plugin| plugin.is_active())
.flat_map(|plugin| plugin.hook_load_warnings.iter().cloned())
.collect()
}
pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] {
&self.capability_summaries
}