mirror of
https://github.com/openai/codex.git
synced 2026-06-01 19:02:59 +00:00
Discover hooks bundled with plugins (#19705)
## Why Plugins can bundle lifecycle hooks, but Codex previously only discovered hooks from user, project, and managed config layers. This adds the plugin discovery and runtime plumbing needed for plugin-bundled hooks while keeping execution behind the `plugin_hooks` feature flag. ## What - Discovers plugin hook sources from each plugin's default `hooks/hooks.json`. - Supports `plugin.json` manifest `hooks` entries as either relative paths or inline hook objects. - Plumbs discovered plugin hook sources through plugin loading into the hook runtime when `plugin_hooks` is enabled. - Marks plugin-originated hook runs as `HookSource::Plugin`. - Injects `PLUGIN_ROOT` and `CLAUDE_PLUGIN_ROOT` into plugin hook command environments. - Updates generated schemas and hook source metadata for the plugin hook source. ## Stack 1. This PR - openai/codex#19705 2. openai/codex#19778 3. openai/codex#19840 4. openai/codex#19882 ## Reviewer Notes - Core logic is in `codex-rs/core-plugins/src/loader.rs` and `codex-rs/hooks/src/engine/discovery.rs` - Moved existing / adding new tests to `codex-rs/core-plugins/src/loader_tests.rs` hence the large diff there - Otherwise mostly plumbing and minor schema updates ### Core Changes The `codex-rs/core` changes are limited to wiring plugin hook support into existing core flows: - `core/src/session/session.rs` conditionally pulls effective plugin hook sources and plugin hook load warnings from `PluginsManager` when `plugin_hooks` is enabled, then passes them into `HooksConfig`. - `core/src/hook_runtime.rs` adds the `plugin` metric tag for `HookSource::Plugin`. - `core/config.schema.json` picks up the new `plugin_hooks` feature flag, and `core/src/plugins/manager_tests.rs` updates fixtures for the added plugin hook fields. --------- Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -108,12 +108,12 @@ fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command {
|
||||
};
|
||||
if shell.program.is_empty() {
|
||||
command.arg(&handler.command);
|
||||
command
|
||||
} else {
|
||||
command.args(&shell.args);
|
||||
command.arg(&handler.command);
|
||||
command
|
||||
}
|
||||
command.envs(&handler.env);
|
||||
command
|
||||
}
|
||||
|
||||
fn default_shell_command() -> Command {
|
||||
|
||||
@@ -12,8 +12,10 @@ use codex_config::HooksFile;
|
||||
use codex_config::ManagedHooksRequirementsToml;
|
||||
use codex_config::MatcherGroup;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashMap;
|
||||
|
||||
use super::ConfiguredHandler;
|
||||
use crate::events::common::matcher_pattern_for_event;
|
||||
@@ -25,23 +27,34 @@ pub(crate) struct DiscoveryResult {
|
||||
pub warnings: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
#[derive(Clone)]
|
||||
struct HookHandlerSource<'a> {
|
||||
path: &'a AbsolutePathBuf,
|
||||
is_managed: bool,
|
||||
source: HookSource,
|
||||
env: HashMap<String, String>,
|
||||
}
|
||||
|
||||
pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) -> DiscoveryResult {
|
||||
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 {
|
||||
return DiscoveryResult {
|
||||
handlers: Vec::new(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let mut handlers = Vec::new();
|
||||
let mut warnings = plugin_hook_load_warnings;
|
||||
let mut display_order = 0_i64;
|
||||
append_plugin_hook_sources(
|
||||
&mut handlers,
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
plugin_hook_sources,
|
||||
);
|
||||
return DiscoveryResult { handlers, warnings };
|
||||
};
|
||||
|
||||
let mut handlers = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut warnings = plugin_hook_load_warnings;
|
||||
let mut display_order = 0_i64;
|
||||
|
||||
append_managed_requirement_handlers(
|
||||
@@ -80,6 +93,7 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) -
|
||||
path: &source_path,
|
||||
is_managed: false,
|
||||
source: hook_source,
|
||||
env: HashMap::new(),
|
||||
},
|
||||
hook_events,
|
||||
);
|
||||
@@ -94,12 +108,20 @@ pub(crate) fn discover_handlers(config_layer_stack: Option<&ConfigLayerStack>) -
|
||||
path: &source_path,
|
||||
is_managed: false,
|
||||
source: hook_source,
|
||||
env: HashMap::new(),
|
||||
},
|
||||
hook_events,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
append_plugin_hook_sources(
|
||||
&mut handlers,
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
plugin_hook_sources,
|
||||
);
|
||||
|
||||
DiscoveryResult { handlers, warnings }
|
||||
}
|
||||
|
||||
@@ -125,11 +147,51 @@ fn append_managed_requirement_handlers(
|
||||
path: &source_path,
|
||||
is_managed: true,
|
||||
source: hook_source_for_requirement_source(managed_hooks.source.as_ref()),
|
||||
env: HashMap::new(),
|
||||
},
|
||||
managed_hooks.get().hooks.clone(),
|
||||
);
|
||||
}
|
||||
|
||||
fn append_plugin_hook_sources(
|
||||
handlers: &mut Vec<ConfiguredHandler>,
|
||||
warnings: &mut Vec<String>,
|
||||
display_order: &mut i64,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
) {
|
||||
// TODO(abhinav): check enabled/trusted state here before plugin hooks become runnable.
|
||||
for source in plugin_hook_sources {
|
||||
let PluginHookSource {
|
||||
plugin_root,
|
||||
plugin_data_root,
|
||||
source_path,
|
||||
hooks,
|
||||
..
|
||||
} = source;
|
||||
let mut env = HashMap::new();
|
||||
let plugin_root_value = plugin_root.display().to_string();
|
||||
let plugin_data_root_value = plugin_data_root.display().to_string();
|
||||
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);
|
||||
env.insert("PLUGIN_DATA".to_string(), plugin_data_root_value.clone());
|
||||
// For OOTB compat with existing plugins that use this env var.
|
||||
env.insert("CLAUDE_PLUGIN_DATA".to_string(), plugin_data_root_value);
|
||||
append_hook_events(
|
||||
handlers,
|
||||
warnings,
|
||||
display_order,
|
||||
HookHandlerSource {
|
||||
path: &source_path,
|
||||
is_managed: false,
|
||||
source: HookSource::Plugin,
|
||||
env,
|
||||
},
|
||||
hooks,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn managed_hooks_source_path(
|
||||
managed_hooks: &ManagedHooksRequirementsToml,
|
||||
requirement_source: Option<&RequirementSource>,
|
||||
@@ -278,7 +340,7 @@ fn append_hook_events(
|
||||
handlers,
|
||||
warnings,
|
||||
display_order,
|
||||
source,
|
||||
source.clone(),
|
||||
event_name,
|
||||
groups,
|
||||
);
|
||||
@@ -298,7 +360,7 @@ fn append_matcher_groups(
|
||||
handlers,
|
||||
warnings,
|
||||
display_order,
|
||||
source,
|
||||
source.clone(),
|
||||
event_name,
|
||||
matcher_pattern_for_event(event_name, group.matcher.as_deref()),
|
||||
group.hooks,
|
||||
@@ -347,6 +409,9 @@ fn append_group_handlers(
|
||||
));
|
||||
continue;
|
||||
}
|
||||
let command = source.env.iter().fold(command, |command, (key, value)| {
|
||||
command.replace(&format!("${{{key}}}"), value)
|
||||
});
|
||||
let timeout_sec = timeout_sec.unwrap_or(600).max(1);
|
||||
handlers.push(ConfiguredHandler {
|
||||
event_name,
|
||||
@@ -358,6 +423,7 @@ fn append_group_handlers(
|
||||
source_path: source.path.clone(),
|
||||
source: source.source,
|
||||
display_order: *display_order,
|
||||
env: source.env.clone(),
|
||||
});
|
||||
*display_order += 1;
|
||||
}
|
||||
@@ -431,6 +497,7 @@ mod tests {
|
||||
path,
|
||||
is_managed: false,
|
||||
source: hook_source(),
|
||||
env: std::collections::HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -475,6 +542,7 @@ mod tests {
|
||||
source_path: source_path.clone(),
|
||||
source: hook_source(),
|
||||
display_order: 0,
|
||||
env: std::collections::HashMap::new(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
@@ -508,6 +576,7 @@ mod tests {
|
||||
source_path: source_path.clone(),
|
||||
source: hook_source(),
|
||||
display_order: 0,
|
||||
env: std::collections::HashMap::new(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -164,6 +164,7 @@ mod tests {
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: HookSource::User,
|
||||
display_order,
|
||||
env: std::collections::HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,7 +4,10 @@ pub(crate) mod dispatcher;
|
||||
pub(crate) mod output_parser;
|
||||
pub(crate) mod schema_loader;
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_protocol::protocol::HookRunSummary;
|
||||
use codex_protocol::protocol::HookSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -39,6 +42,7 @@ pub(crate) struct ConfiguredHandler {
|
||||
pub source_path: AbsolutePathBuf,
|
||||
pub source: HookSource,
|
||||
pub display_order: i64,
|
||||
pub env: HashMap<String, String>,
|
||||
}
|
||||
|
||||
impl ConfiguredHandler {
|
||||
@@ -74,6 +78,8 @@ impl ClaudeHooksEngine {
|
||||
pub(crate) fn new(
|
||||
enabled: bool,
|
||||
config_layer_stack: Option<&ConfigLayerStack>,
|
||||
plugin_hook_sources: Vec<PluginHookSource>,
|
||||
plugin_hook_load_warnings: Vec<String>,
|
||||
shell: CommandShell,
|
||||
) -> Self {
|
||||
if !enabled {
|
||||
@@ -85,7 +91,11 @@ impl ClaudeHooksEngine {
|
||||
}
|
||||
|
||||
let _ = schema_loader::generated_hook_schemas();
|
||||
let discovered = discovery::discover_handlers(config_layer_stack);
|
||||
let discovered = discovery::discover_handlers(
|
||||
config_layer_stack,
|
||||
plugin_hook_sources,
|
||||
plugin_hook_load_warnings,
|
||||
);
|
||||
Self {
|
||||
handlers: discovered.handlers,
|
||||
warnings: discovered.warnings,
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
|
||||
@@ -15,7 +16,10 @@ use codex_config::ManagedHooksRequirementsToml;
|
||||
use codex_config::MatcherGroup;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::TomlValue;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::HookSource;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
@@ -105,6 +109,8 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle:
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
@@ -188,6 +194,8 @@ fn requirements_managed_hooks_warn_when_managed_dir_is_missing() {
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
@@ -295,6 +303,8 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
Some(&config_layer_stack),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
@@ -325,3 +335,192 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() {
|
||||
assert_eq!(preview[0].source_path, hooks_json_path);
|
||||
assert_eq!(preview[1].source_path, config_path);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_hook_sources_run_with_plugin_env_and_plugin_source() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
let plugin_root =
|
||||
AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root");
|
||||
let plugin_data_root =
|
||||
AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root");
|
||||
fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir");
|
||||
let source_path = plugin_root.join("hooks/hooks.json");
|
||||
let log_path = plugin_root.join("env.json");
|
||||
let script_path = plugin_root.join("hooks/write_env.py");
|
||||
fs::write(
|
||||
script_path.as_path(),
|
||||
format!(
|
||||
r#"import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
|
||||
Path(r"{log_path}").write_text(json.dumps({{
|
||||
"plugin": os.environ.get("PLUGIN_ROOT"),
|
||||
"claude": os.environ.get("CLAUDE_PLUGIN_ROOT"),
|
||||
}}), encoding="utf-8")
|
||||
"#,
|
||||
log_path = log_path.display(),
|
||||
),
|
||||
)
|
||||
.expect("write hook script");
|
||||
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
|
||||
let plugin_hook_sources = vec![PluginHookSource {
|
||||
plugin_id,
|
||||
plugin_root: plugin_root.clone(),
|
||||
plugin_data_root: plugin_data_root.clone(),
|
||||
source_path: source_path.clone(),
|
||||
source_relative_path: "hooks/hooks.json".to_string(),
|
||||
hooks: HookEventsToml {
|
||||
pre_tool_use: vec![MatcherGroup {
|
||||
matcher: Some("Bash".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: format!("python3 {}", script_path.display()),
|
||||
timeout_sec: Some(5),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
}];
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*config_layer_stack*/ None,
|
||||
plugin_hook_sources,
|
||||
Vec::new(),
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
},
|
||||
);
|
||||
|
||||
let preview = engine.preview_pre_tool_use(&PreToolUseRequest {
|
||||
session_id: ThreadId::new(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
cwd: cwd(),
|
||||
transcript_path: None,
|
||||
model: "gpt-test".to_string(),
|
||||
permission_mode: "default".to_string(),
|
||||
tool_name: "Bash".to_string(),
|
||||
matcher_aliases: Vec::new(),
|
||||
tool_use_id: "tool-1".to_string(),
|
||||
tool_input: serde_json::json!({ "command": "echo hello" }),
|
||||
});
|
||||
assert_eq!(preview.len(), 1);
|
||||
assert_eq!(preview[0].source, HookSource::Plugin);
|
||||
assert_eq!(preview[0].source_path, source_path);
|
||||
|
||||
let outcome = engine
|
||||
.run_pre_tool_use(PreToolUseRequest {
|
||||
session_id: ThreadId::new(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
cwd: cwd(),
|
||||
transcript_path: None,
|
||||
model: "gpt-test".to_string(),
|
||||
permission_mode: "default".to_string(),
|
||||
tool_name: "Bash".to_string(),
|
||||
matcher_aliases: Vec::new(),
|
||||
tool_use_id: "tool-1".to_string(),
|
||||
tool_input: serde_json::json!({ "command": "echo hello" }),
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(outcome.hook_events.len(), 1);
|
||||
assert_eq!(outcome.hook_events[0].run.source, HookSource::Plugin);
|
||||
let logged: serde_json::Value =
|
||||
serde_json::from_str(&fs::read_to_string(log_path.as_path()).expect("read env log"))
|
||||
.expect("parse env log");
|
||||
assert_eq!(
|
||||
logged,
|
||||
serde_json::json!({
|
||||
"plugin": plugin_root.display().to_string(),
|
||||
"claude": plugin_root.display().to_string(),
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_hook_sources_expand_plugin_placeholders() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
let plugin_root =
|
||||
AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root");
|
||||
let plugin_data_root =
|
||||
AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root");
|
||||
let source_path = plugin_root.join("hooks/hooks.json");
|
||||
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
|
||||
let plugin_hook_sources = vec![PluginHookSource {
|
||||
plugin_id,
|
||||
plugin_root: plugin_root.clone(),
|
||||
plugin_data_root: plugin_data_root.clone(),
|
||||
source_path,
|
||||
source_relative_path: "hooks/hooks.json".to_string(),
|
||||
hooks: HookEventsToml {
|
||||
pre_tool_use: vec![MatcherGroup {
|
||||
matcher: Some("Bash".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "run ${PLUGIN_ROOT} ${CLAUDE_PLUGIN_ROOT} ${PLUGIN_DATA} ${CLAUDE_PLUGIN_DATA}"
|
||||
.to_string(),
|
||||
timeout_sec: Some(5),
|
||||
r#async: false,
|
||||
status_message: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
}];
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*config_layer_stack*/ None,
|
||||
plugin_hook_sources,
|
||||
Vec::new(),
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
engine.handlers[0].command,
|
||||
format!(
|
||||
"run {} {} {} {}",
|
||||
plugin_root.display(),
|
||||
plugin_root.display(),
|
||||
plugin_data_root.display(),
|
||||
plugin_data_root.display()
|
||||
)
|
||||
);
|
||||
assert_eq!(
|
||||
engine.handlers[0].env,
|
||||
HashMap::from([
|
||||
("PLUGIN_ROOT".to_string(), plugin_root.display().to_string()),
|
||||
(
|
||||
"CLAUDE_PLUGIN_ROOT".to_string(),
|
||||
plugin_root.display().to_string()
|
||||
),
|
||||
(
|
||||
"PLUGIN_DATA".to_string(),
|
||||
plugin_data_root.display().to_string()
|
||||
),
|
||||
(
|
||||
"CLAUDE_PLUGIN_DATA".to_string(),
|
||||
plugin_data_root.display().to_string()
|
||||
),
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_hook_load_warnings_are_startup_warnings() {
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*config_layer_stack*/ None,
|
||||
Vec::new(),
|
||||
vec!["failed plugin hook".to_string()],
|
||||
CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
},
|
||||
);
|
||||
|
||||
assert_eq!(engine.warnings(), &["failed plugin hook".to_string()]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user