mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Run allowlisted executor plugin stop hooks (#40009)
## What changed - Discover inline hooks from executor-provided plugin manifests and accept only the bundled Computer Use `Stop` hook for `node_repl.turn_ended`. - Add the accepted hook to the current step's hook engine with executor-scoped provenance. - Run executor-scoped hooks in the background after regular stop-hook decisions, without delaying turn completion, applying control effects, or exposing them in hook summaries. Skip them when regular hooks only block continuation. ## Testing - Added coverage for manifest filtering and option preservation. - Added coverage for background execution, interaction with regular stop hooks, and non-blocking turn completion. GitOrigin-RevId: c0f75f83a23ba8a83bb3a5d5997c4882aacc32d6
This commit is contained in:
committed by
copyberry
parent
e77c2a90af
commit
8b61c50ebe
Generated
+1
@@ -3380,6 +3380,7 @@ dependencies = [
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-output-truncation",
|
||||
"codex-utils-path-uri",
|
||||
"codex-utils-pty",
|
||||
"futures",
|
||||
"pretty_assertions",
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
use codex_config::HookEventsToml;
|
||||
use codex_config::HookHandlerConfig;
|
||||
use codex_config::MatcherGroup;
|
||||
use codex_exec_server::ExecutorCapabilityDiscoverySnapshot;
|
||||
use codex_plugin::ExecutorPluginHookSource;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_plugin::manifest::PluginManifestHooks;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
|
||||
use crate::manifest::parse_plugin_manifest_uri;
|
||||
|
||||
struct AllowlistedExecutorPluginHook {
|
||||
plugin_id: &'static str,
|
||||
event: HookEventName,
|
||||
server: &'static str,
|
||||
tool: &'static str,
|
||||
}
|
||||
|
||||
// Executor plugin manifests are unsigned, so temporarily hardcode the expected
|
||||
// bundled cleanup hook identity and MCP target until plugin signing lands.
|
||||
const ALLOWLISTED_EXECUTOR_PLUGIN_HOOKS: &[AllowlistedExecutorPluginHook] =
|
||||
&[AllowlistedExecutorPluginHook {
|
||||
plugin_id: "computer-use@openai-bundled",
|
||||
event: HookEventName::Stop,
|
||||
server: "node_repl",
|
||||
tool: "turn_ended",
|
||||
}];
|
||||
|
||||
/// Returns accepted inline hook sources from executor-discovered plugin manifests.
|
||||
///
|
||||
/// Executor scoped hooks are best-effort because executor capabilities can become available
|
||||
/// after earlier lifecycle events have passed.
|
||||
///
|
||||
/// Note: Executor manifests are not signed yet, so temporarily we only admit the known Computer Use cleanup hook.
|
||||
pub fn executor_plugin_hook_sources(
|
||||
snapshot: &ExecutorCapabilityDiscoverySnapshot,
|
||||
) -> Vec<ExecutorPluginHookSource> {
|
||||
let mut sources = Vec::new();
|
||||
|
||||
for entry in snapshot.roots() {
|
||||
let Ok(plugin_id) = PluginId::parse(&entry.selected_root.id) else {
|
||||
continue;
|
||||
};
|
||||
let CapabilityRootLocation::Environment {
|
||||
environment_id,
|
||||
path: plugin_root,
|
||||
} = &entry.selected_root.location;
|
||||
let Ok(discovery) = &entry.result else {
|
||||
continue;
|
||||
};
|
||||
let Some(plugin) = &discovery.plugin else {
|
||||
continue;
|
||||
};
|
||||
let Ok(manifest) = parse_plugin_manifest_uri(
|
||||
plugin_root,
|
||||
&plugin.manifest.path,
|
||||
&plugin.manifest.contents,
|
||||
) else {
|
||||
continue;
|
||||
};
|
||||
// Only inline hooks are supported for now, so skip any other source types.
|
||||
let Some(PluginManifestHooks::Inline(hook_files)) = manifest.paths.hooks else {
|
||||
continue;
|
||||
};
|
||||
|
||||
for (hook_index, hook_file) in hook_files.into_iter().enumerate() {
|
||||
let manifest_relative_path = plugin
|
||||
.manifest
|
||||
.path
|
||||
.relative_path_from(plugin_root)
|
||||
.unwrap_or_else(|| plugin.manifest.path.to_string());
|
||||
|
||||
sources.push(ExecutorPluginHookSource {
|
||||
plugin_id: plugin_id.clone(),
|
||||
environment_id: environment_id.clone(),
|
||||
plugin_root: plugin_root.clone(),
|
||||
manifest_path: plugin.manifest.path.clone(),
|
||||
source_relative_path: format!("{manifest_relative_path}#hooks[{hook_index}]"),
|
||||
hooks: hook_file.hooks,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// FIXME: Remove this temporary filter once executor plugin hooks can be trusted.
|
||||
sources.into_iter().filter_map(allowlisted_source).collect()
|
||||
}
|
||||
|
||||
fn allowlisted_source(mut source: ExecutorPluginHookSource) -> Option<ExecutorPluginHookSource> {
|
||||
let allowlisted_hook = ALLOWLISTED_EXECUTOR_PLUGIN_HOOKS.iter().find(|hook| {
|
||||
hook.plugin_id == source.plugin_id.as_key() && hook.event == HookEventName::Stop
|
||||
})?;
|
||||
let handler = source
|
||||
.hooks
|
||||
.stop
|
||||
.into_iter()
|
||||
.filter(|group| group.matcher.is_none())
|
||||
.flat_map(|group| group.hooks)
|
||||
.find(|handler| {
|
||||
matches!(
|
||||
handler,
|
||||
HookHandlerConfig::McpTool { server, tool, .. }
|
||||
if server == allowlisted_hook.server && tool == allowlisted_hook.tool
|
||||
)
|
||||
})?;
|
||||
|
||||
source.hooks = HookEventsToml {
|
||||
stop: vec![MatcherGroup {
|
||||
matcher: None,
|
||||
hooks: vec![handler],
|
||||
}],
|
||||
..Default::default()
|
||||
};
|
||||
Some(source)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "executor_hooks_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,238 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_exec_server::CapabilityRootDiscovery;
|
||||
use codex_exec_server::CapabilityTextFile;
|
||||
use codex_exec_server::DiscoveredPluginFiles;
|
||||
use codex_exec_server::ExecutorCapabilityDiscoverySnapshot;
|
||||
use codex_plugin::ExecutorPluginHookSource;
|
||||
use codex_protocol::capabilities::CapabilityRootLocation;
|
||||
use codex_protocol::capabilities::SelectedCapabilityRoot;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn snapshot_for_manifest(
|
||||
plugin_id: &str,
|
||||
environment_id: &str,
|
||||
manifest_path: &str,
|
||||
manifest: serde_json::Value,
|
||||
) -> ExecutorCapabilityDiscoverySnapshot {
|
||||
let plugin_root = PathUri::parse("file:///plugins/computer-use").expect("plugin root");
|
||||
let manifest_path = PathUri::parse(manifest_path).expect("manifest path");
|
||||
let selected_root = SelectedCapabilityRoot {
|
||||
id: plugin_id.to_string(),
|
||||
location: CapabilityRootLocation::Environment {
|
||||
environment_id: environment_id.to_string(),
|
||||
path: plugin_root.clone(),
|
||||
},
|
||||
};
|
||||
let discovery = CapabilityRootDiscovery {
|
||||
id: plugin_id.to_string(),
|
||||
path: plugin_root,
|
||||
plugin: Some(DiscoveredPluginFiles {
|
||||
manifest: CapabilityTextFile {
|
||||
path: manifest_path,
|
||||
contents: manifest.to_string(),
|
||||
},
|
||||
mcp_config: None,
|
||||
apps_config: None,
|
||||
}),
|
||||
skills: Vec::new(),
|
||||
namespace_manifests: Vec::new(),
|
||||
warnings: Vec::new(),
|
||||
error: None,
|
||||
};
|
||||
ExecutorCapabilityDiscoverySnapshot::new(
|
||||
&[selected_root],
|
||||
vec![Ok(Arc::new(discovery))],
|
||||
HashMap::new(),
|
||||
)
|
||||
}
|
||||
|
||||
fn cleanup_hook_manifest() -> serde_json::Value {
|
||||
json!({
|
||||
"name": "computer-use",
|
||||
"hooks": {
|
||||
"hooks": {
|
||||
"Stop": [{
|
||||
"hooks": [{
|
||||
"type": "mcp_tool",
|
||||
"server": "node_repl",
|
||||
"tool": "turn_ended",
|
||||
"input": {
|
||||
"hook_event_name": "${hook_event_name}",
|
||||
"session_id": "${session_id}",
|
||||
"turn_id": "${turn_id}"
|
||||
}
|
||||
}]
|
||||
}]
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn expected_source(index: usize) -> ExecutorPluginHookSource {
|
||||
ExecutorPluginHookSource {
|
||||
plugin_id: PluginId::parse("computer-use@openai-bundled").expect("plugin id"),
|
||||
environment_id: "executor-a".to_string(),
|
||||
plugin_root: PathUri::parse("file:///plugins/computer-use").expect("plugin root"),
|
||||
manifest_path: PathUri::parse("file:///plugins/computer-use/.codex-plugin/plugin.json")
|
||||
.expect("manifest path"),
|
||||
source_relative_path: format!(".codex-plugin/plugin.json#hooks[{index}]"),
|
||||
hooks: HookEventsToml {
|
||||
stop: vec![MatcherGroup {
|
||||
matcher: None,
|
||||
hooks: vec![HookHandlerConfig::McpTool {
|
||||
server: "node_repl".to_string(),
|
||||
tool: "turn_ended".to_string(),
|
||||
input: serde_json::from_value(json!({
|
||||
"hook_event_name": "${hook_event_name}",
|
||||
"session_id": "${session_id}",
|
||||
"turn_id": "${turn_id}",
|
||||
}))
|
||||
.expect("executor hook input"),
|
||||
timeout_sec: None,
|
||||
status_message: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discovers_allowlisted_executor_plugin_hook_sources() {
|
||||
let mut manifest = cleanup_hook_manifest();
|
||||
manifest["hooks"]["hooks"]["Stop"][0]["hooks"]
|
||||
.as_array_mut()
|
||||
.expect("stop hooks")
|
||||
.push(json!({
|
||||
"type": "command",
|
||||
"command": "echo ignored"
|
||||
}));
|
||||
manifest["hooks"]["hooks"]["UserPromptSubmit"] = json!([{
|
||||
"hooks": [{
|
||||
"type": "mcp_tool",
|
||||
"server": "other",
|
||||
"tool": "ignored"
|
||||
}]
|
||||
}]);
|
||||
let second_hook_file = cleanup_hook_manifest()["hooks"].clone();
|
||||
let first_hook_file = manifest["hooks"].take();
|
||||
manifest["hooks"] = json!([first_hook_file, second_hook_file]);
|
||||
let snapshot = snapshot_for_manifest(
|
||||
"computer-use@openai-bundled",
|
||||
"executor-a",
|
||||
"file:///plugins/computer-use/.codex-plugin/plugin.json",
|
||||
manifest,
|
||||
);
|
||||
|
||||
let sources = executor_plugin_hook_sources(&snapshot);
|
||||
|
||||
assert_eq!(
|
||||
sources,
|
||||
vec![expected_source(/*index*/ 0), expected_source(/*index*/ 1)]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_allowlisted_executor_plugin_hook_options() {
|
||||
let mut manifest = cleanup_hook_manifest();
|
||||
let handler = &mut manifest["hooks"]["hooks"]["Stop"][0]["hooks"][0];
|
||||
handler["input"] = json!({ "untrusted": "manifest-provided input" });
|
||||
handler["timeout"] = json!(30);
|
||||
handler["statusMessage"] = json!("Cleaning up Computer Use");
|
||||
let snapshot = snapshot_for_manifest(
|
||||
"computer-use@openai-bundled",
|
||||
"executor-a",
|
||||
"file:///plugins/computer-use/.codex-plugin/plugin.json",
|
||||
manifest,
|
||||
);
|
||||
|
||||
let mut expected = expected_source(/*index*/ 0);
|
||||
expected.hooks.stop[0].hooks[0] = HookHandlerConfig::McpTool {
|
||||
server: "node_repl".to_string(),
|
||||
tool: "turn_ended".to_string(),
|
||||
input: serde_json::from_value(json!({ "untrusted": "manifest-provided input" }))
|
||||
.expect("manifest-provided executor hook input"),
|
||||
timeout_sec: Some(30),
|
||||
status_message: Some("Cleaning up Computer Use".to_string()),
|
||||
};
|
||||
|
||||
assert_eq!(executor_plugin_hook_sources(&snapshot), vec![expected]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_unallowlisted_executor_plugin_hooks() {
|
||||
let mut wrong_event = cleanup_hook_manifest();
|
||||
let hook_events = wrong_event["hooks"]["hooks"]
|
||||
.as_object_mut()
|
||||
.expect("hook events");
|
||||
let stop_groups = hook_events.remove("Stop").expect("stop groups");
|
||||
hook_events.insert("SessionStart".to_string(), stop_groups);
|
||||
let mut wrong_handler = cleanup_hook_manifest();
|
||||
wrong_handler["hooks"]["hooks"]["Stop"][0]["hooks"][0] = json!({
|
||||
"type": "command",
|
||||
"command": "echo cleanup"
|
||||
});
|
||||
let mut wrong_server = cleanup_hook_manifest();
|
||||
wrong_server["hooks"]["hooks"]["Stop"][0]["hooks"][0]["server"] = json!("other");
|
||||
let mut wrong_tool = cleanup_hook_manifest();
|
||||
wrong_tool["hooks"]["hooks"]["Stop"][0]["hooks"][0]["tool"] = json!("other");
|
||||
let mut wrong_matcher = cleanup_hook_manifest();
|
||||
wrong_matcher["hooks"]["hooks"]["Stop"][0]["matcher"] = json!("unexpected");
|
||||
for (name, plugin_id, manifest) in [
|
||||
(
|
||||
"wrong plugin id",
|
||||
"computer-use@other",
|
||||
cleanup_hook_manifest(),
|
||||
),
|
||||
("wrong event", "computer-use@openai-bundled", wrong_event),
|
||||
(
|
||||
"wrong handler",
|
||||
"computer-use@openai-bundled",
|
||||
wrong_handler,
|
||||
),
|
||||
("wrong server", "computer-use@openai-bundled", wrong_server),
|
||||
("wrong tool", "computer-use@openai-bundled", wrong_tool),
|
||||
(
|
||||
"wrong matcher",
|
||||
"computer-use@openai-bundled",
|
||||
wrong_matcher,
|
||||
),
|
||||
] {
|
||||
let snapshot = snapshot_for_manifest(
|
||||
plugin_id,
|
||||
"executor-a",
|
||||
"file:///plugins/computer-use/.codex-plugin/plugin.json",
|
||||
manifest,
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
executor_plugin_hook_sources(&snapshot),
|
||||
Vec::<ExecutorPluginHookSource>::new(),
|
||||
"{name}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_file_backed_executor_plugin_hooks() {
|
||||
let file_backed = snapshot_for_manifest(
|
||||
"computer-use@openai-bundled",
|
||||
"executor-a",
|
||||
"file:///plugins/computer-use/.codex-plugin/plugin.json",
|
||||
json!({
|
||||
"name": "computer-use",
|
||||
"hooks": "./hooks/hooks.json"
|
||||
}),
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
executor_plugin_hook_sources(&file_backed),
|
||||
Vec::<ExecutorPluginHookSource>::new()
|
||||
);
|
||||
}
|
||||
@@ -3,6 +3,7 @@ mod artifact_operation;
|
||||
mod command_migration;
|
||||
mod discoverable;
|
||||
mod error_subtype;
|
||||
mod executor_hooks;
|
||||
mod git_policy;
|
||||
mod http_client_selector;
|
||||
pub mod installed_marketplaces;
|
||||
@@ -61,6 +62,7 @@ pub use command_migration::import_commands_with_profile;
|
||||
pub use command_migration::missing_command_names_with_profile;
|
||||
pub use discoverable::ToolSuggestDiscoverablePlugin;
|
||||
pub use discoverable::ToolSuggestPluginDiscoveryInput;
|
||||
pub use executor_hooks::executor_plugin_hook_sources;
|
||||
pub use loader::PluginHookLoadOutcome;
|
||||
pub use manager::ConfiguredMarketplace;
|
||||
pub use manager::ConfiguredMarketplaceListOutcome;
|
||||
|
||||
@@ -5,6 +5,7 @@ use std::time::Duration;
|
||||
use codex_analytics::CompactionTrigger;
|
||||
use codex_analytics::HookRunFact;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_core_plugins::executor_plugin_hook_sources;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PermissionRequestOutcome;
|
||||
use codex_hooks::PermissionRequestRequest;
|
||||
@@ -52,6 +53,7 @@ use crate::context::HookAdditionalContext;
|
||||
use crate::event_mapping::parse_turn_item;
|
||||
use crate::session::TurnInput;
|
||||
use crate::session::session::Session;
|
||||
use crate::session::step_context::StepContext;
|
||||
use crate::session::turn_context::TurnContext;
|
||||
use crate::tools::hook_names::HookToolName;
|
||||
use crate::tools::sandboxing::PermissionRequestPayload;
|
||||
@@ -305,10 +307,11 @@ pub(crate) async fn run_post_tool_use_hooks(
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub(crate) async fn run_turn_stop_hooks(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
step_context: &Arc<StepContext>,
|
||||
stop_hook_active: bool,
|
||||
last_assistant_message: Option<String>,
|
||||
) -> StopOutcome {
|
||||
let turn_context = &step_context.turn;
|
||||
// Resolve the stop hook kind from the session source before building the
|
||||
// request. Root turns run Stop; thread-spawned child turns run SubagentStop.
|
||||
let (target, transcript_path) = match &turn_context.session_source {
|
||||
@@ -365,7 +368,12 @@ pub(crate) async fn run_turn_stop_hooks(
|
||||
last_assistant_message,
|
||||
target,
|
||||
};
|
||||
let hooks = sess.hooks();
|
||||
let executor_hook_sources = step_context
|
||||
.executor_capability_discovery
|
||||
.as_deref()
|
||||
.map(executor_plugin_hook_sources)
|
||||
.unwrap_or_default();
|
||||
let hooks = sess.hooks().with_executor_hooks(executor_hook_sources);
|
||||
emit_hook_started_events(sess, turn_context, hooks.preview_stop(&request)).await;
|
||||
|
||||
let mut outcome = hooks.run_stop(request).await;
|
||||
|
||||
@@ -501,7 +501,7 @@ pub(crate) async fn run_turn(
|
||||
last_agent_message = sampling_request_last_agent_message;
|
||||
let stop_outcome = run_turn_stop_hooks(
|
||||
&sess,
|
||||
&turn_context,
|
||||
&step_context,
|
||||
stop_hook_active,
|
||||
last_agent_message.clone(),
|
||||
)
|
||||
|
||||
@@ -21,6 +21,7 @@ codex-plugin = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
codex-utils-output-truncation = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
futures = { workspace = true, features = ["alloc"] }
|
||||
regex = { workspace = true }
|
||||
schemars = { workspace = true }
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::HashMap;
|
||||
#[cfg(not(windows))]
|
||||
use std::ffi::OsStr;
|
||||
use std::ffi::OsString;
|
||||
use std::future::Future;
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::process::Stdio;
|
||||
@@ -111,19 +112,13 @@ impl CommandHookRuntime {
|
||||
turn_id: Option<String>,
|
||||
parse: fn(&ConfiguredHandler, HandlerRunResult, Option<String>) -> ParsedHandler<T>,
|
||||
) {
|
||||
let mut state = self.lock_state();
|
||||
if self.result_sender.is_closed() || state.concurrency_limit.is_closed() {
|
||||
if self.result_sender.is_closed() {
|
||||
return;
|
||||
}
|
||||
|
||||
while state.tasks.try_join_next().is_some() {}
|
||||
let result_sender = self.result_sender.clone();
|
||||
let concurrency_limit = Arc::clone(&state.concurrency_limit);
|
||||
let runtime = self.clone();
|
||||
state.tasks.spawn(async move {
|
||||
let Ok(_permit) = concurrency_limit.acquire_owned().await else {
|
||||
return;
|
||||
};
|
||||
self.schedule_async_task(async move {
|
||||
let result = match &handler.kind {
|
||||
ConfiguredHandlerKind::Command { command, env, .. } => {
|
||||
run_command(&runtime, &handler, command, env, &input_json, &cwd).await
|
||||
@@ -165,6 +160,22 @@ impl CommandHookRuntime {
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn schedule_async_task(&self, task: impl Future<Output = ()> + Send + 'static) {
|
||||
let mut state = self.lock_state();
|
||||
if state.concurrency_limit.is_closed() {
|
||||
return;
|
||||
}
|
||||
|
||||
while state.tasks.try_join_next().is_some() {}
|
||||
let concurrency_limit = Arc::clone(&state.concurrency_limit);
|
||||
state.tasks.spawn(async move {
|
||||
let Ok(_permit) = concurrency_limit.acquire_owned().await else {
|
||||
return;
|
||||
};
|
||||
task.await;
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) async fn shutdown(&self) {
|
||||
let mut tasks = {
|
||||
let mut state = self.lock_state();
|
||||
|
||||
@@ -58,7 +58,7 @@ async fn cmd_shell_runs_quoted_hook_command_path() {
|
||||
timeout_sec: 10,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path,
|
||||
source_path: source_path.into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
@@ -107,7 +107,7 @@ async fn fast_exiting_hook_preserves_stdout_when_stdin_is_not_consumed() {
|
||||
timeout_sec: 10,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path,
|
||||
source_path: source_path.into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
@@ -145,7 +145,7 @@ async fn command_hook_does_not_expose_configured_noise_auth_token() {
|
||||
timeout_sec: 10,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path,
|
||||
source_path: source_path.into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
@@ -304,7 +304,8 @@ fn write_handler(temp: &TempDir, source: &str) -> ConfiguredHandler {
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: AbsolutePathBuf::try_from(temp.path().join("hooks.json"))
|
||||
.expect("absolute test hook path"),
|
||||
.expect("absolute test hook path")
|
||||
.into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -705,7 +705,7 @@ fn append_matcher_groups(
|
||||
additional_context_limit: AdditionalContextLimit::from_config(
|
||||
additional_context_limit,
|
||||
),
|
||||
source_path: source.path.clone(),
|
||||
source_path: source.path.clone().into(),
|
||||
source: source.source,
|
||||
display_order: *display_order,
|
||||
kind,
|
||||
@@ -1166,7 +1166,7 @@ mod tests {
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: source_path.clone(),
|
||||
source_path: source_path.clone().into(),
|
||||
source: hook_source(),
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
@@ -1205,7 +1205,7 @@ mod tests {
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: source_path.clone(),
|
||||
source_path: source_path.clone().into(),
|
||||
source: hook_source(),
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -15,6 +15,7 @@ use super::ClaudeHooksEngine;
|
||||
use super::ConfiguredHandler;
|
||||
use super::ConfiguredHandlerKind;
|
||||
use super::HandlerRunResult;
|
||||
use super::HandlerSourcePath;
|
||||
use super::command_runner::run_command;
|
||||
use super::mcp_runner::run_mcp_tool;
|
||||
use crate::events::common::matches_matcher;
|
||||
@@ -71,13 +72,16 @@ pub(crate) fn select_handlers_for_matcher_inputs(
|
||||
}
|
||||
|
||||
pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary {
|
||||
let HandlerSourcePath::Local(source_path) = &handler.source_path else {
|
||||
unreachable!("executor-scoped hooks do not produce public hook summaries");
|
||||
};
|
||||
HookRunSummary {
|
||||
id: handler.run_id(),
|
||||
event_name: handler.event_name,
|
||||
handler_type: handler.handler_type(),
|
||||
execution_mode: handler.execution_mode(),
|
||||
scope: scope_for_event(handler.event_name),
|
||||
source_path: handler.source_path.clone(),
|
||||
source_path: source_path.clone(),
|
||||
source: handler.source,
|
||||
display_order: handler.display_order,
|
||||
status: HookRunStatus::Running,
|
||||
@@ -97,8 +101,16 @@ pub(crate) async fn execute_handlers<T: 'static>(
|
||||
turn_id: Option<String>,
|
||||
parse: fn(&ConfiguredHandler, HandlerRunResult, Option<String>) -> ParsedHandler<T>,
|
||||
) -> Vec<ParsedHandler<T>> {
|
||||
let mut executor_handlers = Vec::new();
|
||||
let mut pending = FuturesUnordered::new();
|
||||
for (configured_order, handler) in handlers.into_iter().enumerate() {
|
||||
if matches!(
|
||||
handler.source_path,
|
||||
HandlerSourcePath::ExecutorScoped { .. }
|
||||
) {
|
||||
executor_handlers.push(handler);
|
||||
continue;
|
||||
}
|
||||
if handler.execution_mode() == HookExecutionMode::Async {
|
||||
engine.command_runtime.schedule_async_hook(
|
||||
handler,
|
||||
@@ -112,62 +124,97 @@ pub(crate) async fn execute_handlers<T: 'static>(
|
||||
let input_json = input_json.clone();
|
||||
let turn_id = turn_id.clone();
|
||||
pending.push(async move {
|
||||
let result = match &handler.kind {
|
||||
ConfiguredHandlerKind::Command { command, env, .. } => {
|
||||
run_command(
|
||||
&engine.command_runtime,
|
||||
&handler,
|
||||
command,
|
||||
env,
|
||||
&input_json,
|
||||
cwd,
|
||||
)
|
||||
.await
|
||||
}
|
||||
ConfiguredHandlerKind::McpTool {
|
||||
server,
|
||||
tool,
|
||||
input,
|
||||
} => {
|
||||
run_mcp_tool(
|
||||
engine.mcp_executor.as_ref(),
|
||||
&handler,
|
||||
server,
|
||||
tool,
|
||||
input,
|
||||
&input_json,
|
||||
)
|
||||
.await
|
||||
}
|
||||
};
|
||||
let result = execute_handler(engine, &handler, &input_json, cwd).await;
|
||||
(configured_order, parse(&handler, result, turn_id))
|
||||
});
|
||||
}
|
||||
|
||||
let mut completed = Vec::new();
|
||||
let mut completion_order = 0;
|
||||
let mut should_stop = false;
|
||||
let mut should_block = false;
|
||||
while let Some((configured_order, mut parsed)) = pending.next().await {
|
||||
should_stop |= parsed.completed.run.status == HookRunStatus::Stopped;
|
||||
should_block |= parsed.completed.run.status == HookRunStatus::Blocked;
|
||||
parsed.completion_order = completion_order;
|
||||
completion_order += 1;
|
||||
completed.push((configured_order, parsed));
|
||||
}
|
||||
completed.sort_by_key(|(configured_order, _)| *configured_order);
|
||||
|
||||
if should_stop || !should_block {
|
||||
for handler in executor_handlers {
|
||||
let task_engine = engine.clone();
|
||||
let input_json = input_json.clone();
|
||||
let cwd = cwd.to_path_buf();
|
||||
engine.command_runtime.schedule_async_task(async move {
|
||||
let result = execute_handler(&task_engine, &handler, &input_json, &cwd).await;
|
||||
if let Some(error) = result.error {
|
||||
tracing::warn!(
|
||||
source_path = %handler.source_path,
|
||||
%error,
|
||||
"executor-scoped hook failed"
|
||||
);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
completed.into_iter().map(|(_, parsed)| parsed).collect()
|
||||
}
|
||||
|
||||
async fn execute_handler(
|
||||
engine: &ClaudeHooksEngine,
|
||||
handler: &ConfiguredHandler,
|
||||
input_json: &str,
|
||||
cwd: &Path,
|
||||
) -> HandlerRunResult {
|
||||
match &handler.kind {
|
||||
ConfiguredHandlerKind::Command { command, env, .. } => {
|
||||
run_command(
|
||||
&engine.command_runtime,
|
||||
handler,
|
||||
command,
|
||||
env,
|
||||
input_json,
|
||||
cwd,
|
||||
)
|
||||
.await
|
||||
}
|
||||
ConfiguredHandlerKind::McpTool {
|
||||
server,
|
||||
tool,
|
||||
input,
|
||||
} => {
|
||||
run_mcp_tool(
|
||||
engine.mcp_executor.as_ref(),
|
||||
handler,
|
||||
server,
|
||||
tool,
|
||||
input,
|
||||
input_json,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn completed_summary(
|
||||
handler: &ConfiguredHandler,
|
||||
run_result: &HandlerRunResult,
|
||||
status: HookRunStatus,
|
||||
entries: Vec<codex_protocol::protocol::HookOutputEntry>,
|
||||
) -> HookRunSummary {
|
||||
let HandlerSourcePath::Local(source_path) = &handler.source_path else {
|
||||
unreachable!("executor-scoped hooks do not produce public hook summaries");
|
||||
};
|
||||
HookRunSummary {
|
||||
id: handler.run_id(),
|
||||
event_name: handler.event_name,
|
||||
handler_type: handler.handler_type(),
|
||||
execution_mode: handler.execution_mode(),
|
||||
scope: scope_for_event(handler.event_name),
|
||||
source_path: handler.source_path.clone(),
|
||||
source_path: source_path.clone(),
|
||||
source: handler.source,
|
||||
display_order: handler.display_order,
|
||||
status,
|
||||
@@ -279,7 +326,7 @@ mod tests {
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: HookSource::User,
|
||||
display_order,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -69,7 +69,7 @@ pub(crate) async fn run_mcp_tool(
|
||||
/// text is rendered as a string. Missing fields fail the hook instead of passing unresolved
|
||||
/// arguments to the server. For example, the template `{"count":"${tool_input.count}"}`
|
||||
/// becomes `{"count":3}` when the event contains `{"tool_input":{"count":3}}`.
|
||||
fn expand_mcp_argument_template(
|
||||
pub(crate) fn expand_mcp_argument_template(
|
||||
argument_template: &Map<String, Value>,
|
||||
hook_event: &Value,
|
||||
) -> Result<Map<String, Value>> {
|
||||
|
||||
@@ -96,7 +96,7 @@ async fn mcp_tool_results_use_command_hook_output_contract() {
|
||||
timeout_sec: 30,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::McpTool {
|
||||
|
||||
@@ -26,7 +26,10 @@ use crate::events::user_prompt_submit::UserPromptSubmitRequest;
|
||||
use crate::mcp::HookMcpExecutor;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_config::HookHandlerConfig;
|
||||
use codex_plugin::ExecutorPluginHookSource;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_protocol::protocol::HookExecutionMode;
|
||||
use codex_protocol::protocol::HookHandlerType;
|
||||
@@ -34,6 +37,7 @@ use codex_protocol::protocol::HookRunSummary;
|
||||
use codex_protocol::protocol::HookSource;
|
||||
use codex_protocol::protocol::HookTrustStatus;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
@@ -53,12 +57,53 @@ pub(crate) struct ConfiguredHandler {
|
||||
pub timeout_sec: u64,
|
||||
pub status_message: Option<String>,
|
||||
pub additional_context_limit: AdditionalContextLimit,
|
||||
pub source_path: AbsolutePathBuf,
|
||||
pub source_path: HandlerSourcePath,
|
||||
pub source: HookSource,
|
||||
pub display_order: i64,
|
||||
pub kind: ConfiguredHandlerKind,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum HandlerSourcePath {
|
||||
Local(AbsolutePathBuf),
|
||||
/// Executor-scoped handlers are currently excluded from user-visible hook reporting
|
||||
/// (events, summary, telemetry) because the only supported 1p hook is
|
||||
/// coupled to executor-scoped execution. Their handlers are always executed async.
|
||||
///
|
||||
/// TODO: With CCA, all hooks will be executor-scoped, so user visibility
|
||||
/// (participation in lifecycle events and summaries) and execution behavior
|
||||
/// (non-blocking) will need to be determined independently.
|
||||
ExecutorScoped {
|
||||
plugin_id: PluginId,
|
||||
environment_id: String,
|
||||
manifest_path: PathUri,
|
||||
source_relative_path: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl From<AbsolutePathBuf> for HandlerSourcePath {
|
||||
fn from(path: AbsolutePathBuf) -> Self {
|
||||
Self::Local(path)
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for HandlerSourcePath {
|
||||
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
Self::Local(path) => write!(formatter, "{}", path.display()),
|
||||
Self::ExecutorScoped {
|
||||
environment_id,
|
||||
manifest_path,
|
||||
source_relative_path,
|
||||
..
|
||||
} => write!(
|
||||
formatter,
|
||||
"{environment_id}:{manifest_path}:{source_relative_path}"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum ConfiguredHandlerKind {
|
||||
Command {
|
||||
@@ -86,6 +131,10 @@ pub(crate) struct HandlerRunResult {
|
||||
|
||||
impl ConfiguredHandler {
|
||||
pub(crate) fn execution_mode(&self) -> HookExecutionMode {
|
||||
if matches!(self.source_path, HandlerSourcePath::ExecutorScoped { .. }) {
|
||||
return HookExecutionMode::Async;
|
||||
}
|
||||
|
||||
match self.kind {
|
||||
ConfiguredHandlerKind::Command { r#async: true, .. } => HookExecutionMode::Async,
|
||||
ConfiguredHandlerKind::Command { r#async: false, .. }
|
||||
@@ -103,7 +152,7 @@ impl ConfiguredHandler {
|
||||
"{}:{}:{}",
|
||||
self.event_name_label(),
|
||||
self.display_order,
|
||||
self.source_path.display()
|
||||
self.source_path
|
||||
)
|
||||
}
|
||||
|
||||
@@ -205,6 +254,72 @@ impl ClaudeHooksEngine {
|
||||
&self.warnings
|
||||
}
|
||||
|
||||
pub(crate) fn set_executor_hooks(&mut self, executor_hooks: Vec<ExecutorPluginHookSource>) {
|
||||
self.handlers.retain(|handler| {
|
||||
!matches!(
|
||||
handler.source_path,
|
||||
HandlerSourcePath::ExecutorScoped { .. }
|
||||
)
|
||||
});
|
||||
|
||||
// FIXME: Remove this restriction once executor hooks support multiple environments.
|
||||
if executor_hooks.len() > 1 {
|
||||
tracing::warn!(
|
||||
executor_hook_count = executor_hooks.len(),
|
||||
"multiple executor hooks found; only executing the first"
|
||||
);
|
||||
}
|
||||
let Some(source) = executor_hooks.into_iter().next() else {
|
||||
return;
|
||||
};
|
||||
let Some(handler) = source
|
||||
.hooks
|
||||
.stop
|
||||
.into_iter()
|
||||
.flat_map(|group| group.hooks)
|
||||
.next()
|
||||
else {
|
||||
unreachable!("allowlisted executor hook source must contain a Stop handler");
|
||||
};
|
||||
let HookHandlerConfig::McpTool {
|
||||
server,
|
||||
tool,
|
||||
input,
|
||||
timeout_sec,
|
||||
status_message,
|
||||
} = handler
|
||||
else {
|
||||
unreachable!("allowlisted executor Stop handler must be an MCP tool");
|
||||
};
|
||||
|
||||
let display_order = self
|
||||
.handlers
|
||||
.iter()
|
||||
.map(|handler| handler.display_order)
|
||||
.max()
|
||||
.map_or(0, |display_order| display_order.saturating_add(1));
|
||||
self.handlers.push(ConfiguredHandler {
|
||||
event_name: HookEventName::Stop,
|
||||
matcher: None,
|
||||
timeout_sec: timeout_sec.unwrap_or(5).max(1),
|
||||
status_message,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: HandlerSourcePath::ExecutorScoped {
|
||||
plugin_id: source.plugin_id,
|
||||
environment_id: source.environment_id,
|
||||
manifest_path: source.manifest_path,
|
||||
source_relative_path: source.source_relative_path,
|
||||
},
|
||||
source: HookSource::Plugin,
|
||||
display_order,
|
||||
kind: ConfiguredHandlerKind::McpTool {
|
||||
server,
|
||||
tool,
|
||||
input,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
pub(crate) fn required_load_errors(&self) -> &[String] {
|
||||
&self.required_load_errors
|
||||
}
|
||||
|
||||
@@ -20,10 +20,12 @@ use codex_config::MatcherGroup;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_config::Sourced;
|
||||
use codex_config::TomlValue;
|
||||
use codex_plugin::ExecutorPluginHookSource;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_protocol::protocol::HookExecutionMode;
|
||||
use codex_protocol::protocol::HookHandlerType;
|
||||
use codex_protocol::protocol::HookOutputEntry;
|
||||
use codex_protocol::protocol::HookOutputEntryKind;
|
||||
@@ -34,14 +36,18 @@ use futures::FutureExt;
|
||||
use futures::future::BoxFuture;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
use tokio::sync::Notify;
|
||||
|
||||
use super::ClaudeHooksEngine;
|
||||
use super::CommandHookRuntime;
|
||||
use super::CommandShell;
|
||||
use super::ConfiguredHandler;
|
||||
use super::ConfiguredHandlerKind;
|
||||
use super::HandlerSourcePath;
|
||||
use super::HookListEntryHandler;
|
||||
use crate::events::pre_tool_use::PreToolUseRequest;
|
||||
use crate::events::stop::StopHookTarget;
|
||||
use crate::events::stop::StopRequest;
|
||||
use crate::mcp::HookMcpCall;
|
||||
use crate::mcp::HookMcpExecutor;
|
||||
|
||||
@@ -63,6 +69,7 @@ pub(crate) fn mcp_executor() -> Arc<dyn HookMcpExecutor> {
|
||||
Arc::new(StaticMcpExecutor {
|
||||
calls: Arc::new(Mutex::new(Vec::new())),
|
||||
output: String::new(),
|
||||
outputs_by_tool: HashMap::new(),
|
||||
})
|
||||
}
|
||||
|
||||
@@ -87,7 +94,7 @@ fn permission_request_timeout_only_counts_synchronous_handlers() {
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: cwd().join("hooks.json"),
|
||||
source_path: cwd().join("hooks.json").into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::Command {
|
||||
@@ -1128,7 +1135,9 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() {
|
||||
);
|
||||
assert_eq!(
|
||||
engine.handlers[0].source_path,
|
||||
AbsolutePathBuf::try_from(missing_dir).expect("absolute missing dir")
|
||||
AbsolutePathBuf::try_from(missing_dir)
|
||||
.expect("absolute missing dir")
|
||||
.into()
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1941,18 +1950,277 @@ fn plugin_hook_load_warnings_are_startup_warnings() {
|
||||
struct StaticMcpExecutor {
|
||||
calls: Arc<Mutex<Vec<HookMcpCall>>>,
|
||||
output: String,
|
||||
outputs_by_tool: HashMap<String, String>,
|
||||
}
|
||||
|
||||
impl HookMcpExecutor for StaticMcpExecutor {
|
||||
fn execute(&self, call: HookMcpCall) -> BoxFuture<'_, anyhow::Result<String>> {
|
||||
async move {
|
||||
let output = self
|
||||
.outputs_by_tool
|
||||
.get(&call.tool)
|
||||
.unwrap_or(&self.output)
|
||||
.clone();
|
||||
self.calls.lock().expect("lock MCP calls").push(call);
|
||||
Ok(self.output.clone())
|
||||
Ok(output)
|
||||
}
|
||||
.boxed()
|
||||
}
|
||||
}
|
||||
|
||||
fn executor_stop_hook_fixture() -> (
|
||||
ClaudeHooksEngine,
|
||||
Arc<Mutex<Vec<HookMcpCall>>>,
|
||||
StopRequest,
|
||||
HookMcpCall,
|
||||
ExecutorPluginHookSource,
|
||||
) {
|
||||
let calls = Arc::new(Mutex::new(Vec::new()));
|
||||
let executor = Arc::new(StaticMcpExecutor {
|
||||
calls: Arc::clone(&calls),
|
||||
output: r#"{"decision":"block","reason":"keep going"}"#.to_string(),
|
||||
outputs_by_tool: HashMap::from([(
|
||||
"terminate".to_string(),
|
||||
r#"{"continue":false,"stopReason":"done"}"#.to_string(),
|
||||
)]),
|
||||
});
|
||||
let mut engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
/*bypass_hook_trust*/ false,
|
||||
/*config_layer_stack*/ None,
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
command_runtime(CommandShell {
|
||||
program: String::new(),
|
||||
args: Vec::new(),
|
||||
}),
|
||||
executor,
|
||||
);
|
||||
let source = ExecutorPluginHookSource {
|
||||
plugin_id: PluginId::parse("computer-use@openai-bundled").expect("valid plugin ID"),
|
||||
environment_id: "executor-a".to_string(),
|
||||
plugin_root: "file:///plugins/computer-use"
|
||||
.parse()
|
||||
.expect("valid plugin root URI"),
|
||||
manifest_path: "file:///plugins/computer-use/.codex-plugin/plugin.json"
|
||||
.parse()
|
||||
.expect("valid plugin manifest URI"),
|
||||
source_relative_path: ".codex-plugin/plugin.json#hooks[0]".to_string(),
|
||||
hooks: HookEventsToml {
|
||||
stop: vec![MatcherGroup {
|
||||
matcher: None,
|
||||
hooks: vec![HookHandlerConfig::McpTool {
|
||||
server: "node_repl".to_string(),
|
||||
tool: "turn_ended".to_string(),
|
||||
input: serde_json::from_value(serde_json::json!({
|
||||
"turn_id": "${turn_id}",
|
||||
}))
|
||||
.expect("executor hook input"),
|
||||
timeout_sec: None,
|
||||
status_message: None,
|
||||
}],
|
||||
}],
|
||||
..Default::default()
|
||||
},
|
||||
};
|
||||
engine.set_executor_hooks(vec![source.clone()]);
|
||||
assert_eq!(
|
||||
engine.handlers,
|
||||
vec![ConfiguredHandler {
|
||||
event_name: HookEventName::Stop,
|
||||
matcher: None,
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: HandlerSourcePath::ExecutorScoped {
|
||||
plugin_id: PluginId::parse("computer-use@openai-bundled").expect("valid plugin ID"),
|
||||
environment_id: "executor-a".to_string(),
|
||||
manifest_path: "file:///plugins/computer-use/.codex-plugin/plugin.json"
|
||||
.parse()
|
||||
.expect("valid plugin manifest URI"),
|
||||
source_relative_path: ".codex-plugin/plugin.json#hooks[0]".to_string(),
|
||||
},
|
||||
source: HookSource::Plugin,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::McpTool {
|
||||
server: "node_repl".to_string(),
|
||||
tool: "turn_ended".to_string(),
|
||||
input: serde_json::from_value(serde_json::json!({
|
||||
"turn_id": "${turn_id}",
|
||||
}))
|
||||
.expect("executor hook input"),
|
||||
},
|
||||
}]
|
||||
);
|
||||
assert_eq!(
|
||||
engine.handlers[0].execution_mode(),
|
||||
HookExecutionMode::Async
|
||||
);
|
||||
assert!(!engine.handlers[0].can_apply_control_effects());
|
||||
let request = StopRequest {
|
||||
session_id: ThreadId::new(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
cwd: cwd(),
|
||||
transcript_path: None,
|
||||
model: "test-model".to_string(),
|
||||
permission_mode: "default".to_string(),
|
||||
stop_hook_active: false,
|
||||
last_assistant_message: None,
|
||||
target: StopHookTarget::Stop,
|
||||
};
|
||||
|
||||
let expected_executor_call = HookMcpCall {
|
||||
server: "node_repl".to_string(),
|
||||
tool: "turn_ended".to_string(),
|
||||
input: serde_json::from_value(serde_json::json!({ "turn_id": "turn-1" }))
|
||||
.expect("expanded executor hook input"),
|
||||
timeout: Duration::from_secs(5),
|
||||
};
|
||||
|
||||
(engine, calls, request, expected_executor_call, source)
|
||||
}
|
||||
|
||||
async fn wait_for_mcp_calls(calls: &Arc<Mutex<Vec<HookMcpCall>>>, count: usize) {
|
||||
tokio::time::timeout(Duration::from_secs(1), async {
|
||||
while calls.lock().expect("lock MCP calls").len() < count {
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
})
|
||||
.await
|
||||
.expect("executor hook should complete in the background");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() {
|
||||
let (mut engine, calls, request, expected_executor_call, _source) =
|
||||
executor_stop_hook_fixture();
|
||||
|
||||
assert_eq!(engine.preview_stop(&request), Vec::new());
|
||||
let outcome = engine.run_stop(request.clone()).await;
|
||||
wait_for_mcp_calls(&calls, /*count*/ 1).await;
|
||||
assert!(!outcome.should_block);
|
||||
assert!(outcome.hook_events.is_empty());
|
||||
assert_eq!(
|
||||
*calls.lock().expect("lock MCP calls"),
|
||||
vec![expected_executor_call.clone()]
|
||||
);
|
||||
|
||||
engine.handlers.push(ConfiguredHandler {
|
||||
event_name: HookEventName::Stop,
|
||||
matcher: None,
|
||||
timeout_sec: 30,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: cwd().join("hooks.json").into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: ConfiguredHandlerKind::McpTool {
|
||||
server: "security".to_string(),
|
||||
tool: "check".to_string(),
|
||||
input: Default::default(),
|
||||
},
|
||||
});
|
||||
|
||||
let outcome = engine.run_stop(request.clone()).await;
|
||||
assert!(outcome.should_block);
|
||||
assert_eq!(outcome.hook_events.len(), 1);
|
||||
assert_eq!(
|
||||
*calls.lock().expect("lock MCP calls"),
|
||||
vec![
|
||||
expected_executor_call.clone(),
|
||||
HookMcpCall {
|
||||
server: "security".to_string(),
|
||||
tool: "check".to_string(),
|
||||
input: Default::default(),
|
||||
timeout: Duration::from_secs(30),
|
||||
},
|
||||
]
|
||||
);
|
||||
|
||||
engine.handlers.push(ConfiguredHandler {
|
||||
event_name: HookEventName::Stop,
|
||||
matcher: None,
|
||||
timeout_sec: 30,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: cwd().join("hooks.json").into(),
|
||||
source: HookSource::User,
|
||||
display_order: 1,
|
||||
kind: ConfiguredHandlerKind::McpTool {
|
||||
server: "security".to_string(),
|
||||
tool: "terminate".to_string(),
|
||||
input: Default::default(),
|
||||
},
|
||||
});
|
||||
|
||||
let outcome = engine.run_stop(request).await;
|
||||
wait_for_mcp_calls(&calls, /*count*/ 5).await;
|
||||
assert!(outcome.should_stop);
|
||||
assert!(!outcome.should_block);
|
||||
assert_eq!(outcome.hook_events.len(), 2);
|
||||
assert_eq!(
|
||||
*calls.lock().expect("lock MCP calls"),
|
||||
vec![
|
||||
expected_executor_call.clone(),
|
||||
HookMcpCall {
|
||||
server: "security".to_string(),
|
||||
tool: "check".to_string(),
|
||||
input: Default::default(),
|
||||
timeout: Duration::from_secs(30),
|
||||
},
|
||||
HookMcpCall {
|
||||
server: "security".to_string(),
|
||||
tool: "check".to_string(),
|
||||
input: Default::default(),
|
||||
timeout: Duration::from_secs(30),
|
||||
},
|
||||
HookMcpCall {
|
||||
server: "security".to_string(),
|
||||
tool: "terminate".to_string(),
|
||||
input: Default::default(),
|
||||
timeout: Duration::from_secs(30),
|
||||
},
|
||||
expected_executor_call,
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn executor_stop_hooks_do_not_delay_stop_completion() {
|
||||
struct BlockingMcpExecutor {
|
||||
started: Arc<Notify>,
|
||||
release: Arc<Notify>,
|
||||
}
|
||||
|
||||
impl HookMcpExecutor for BlockingMcpExecutor {
|
||||
fn execute(&self, _call: HookMcpCall) -> BoxFuture<'_, anyhow::Result<String>> {
|
||||
async move {
|
||||
self.started.notify_one();
|
||||
self.release.notified().await;
|
||||
Ok(String::new())
|
||||
}
|
||||
.boxed()
|
||||
}
|
||||
}
|
||||
|
||||
let (mut engine, _, request, _, _) = executor_stop_hook_fixture();
|
||||
let started = Arc::new(Notify::new());
|
||||
let release = Arc::new(Notify::new());
|
||||
engine.mcp_executor = Arc::new(BlockingMcpExecutor {
|
||||
started: Arc::clone(&started),
|
||||
release: Arc::clone(&release),
|
||||
});
|
||||
|
||||
let outcome = tokio::time::timeout(Duration::from_millis(100), engine.run_stop(request))
|
||||
.await
|
||||
.expect("executor hook must not delay Stop completion");
|
||||
assert!(!outcome.should_block);
|
||||
tokio::time::timeout(Duration::from_secs(1), started.notified())
|
||||
.await
|
||||
.expect("executor hook should start in the background");
|
||||
release.notify_one();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_tool_hooks_expand_event_input_and_apply_pre_tool_decisions() {
|
||||
let temp = tempdir().expect("create temp dir");
|
||||
@@ -2014,6 +2282,7 @@ async fn mcp_tool_hooks_expand_event_input_and_apply_pre_tool_decisions() {
|
||||
},
|
||||
})
|
||||
.to_string(),
|
||||
outputs_by_tool: HashMap::new(),
|
||||
};
|
||||
let engine = ClaudeHooksEngine::new(
|
||||
/*enabled*/ true,
|
||||
|
||||
@@ -6,6 +6,7 @@ use codex_protocol::protocol::HookRunStatus;
|
||||
use codex_protocol::protocol::HookRunSummary;
|
||||
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::HandlerSourcePath;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::output_spill::AdditionalContext;
|
||||
|
||||
@@ -65,6 +66,7 @@ pub(crate) fn serialization_failure_hook_events(
|
||||
) -> Vec<HookCompletedEvent> {
|
||||
handlers
|
||||
.into_iter()
|
||||
.filter(|handler| matches!(handler.source_path, HandlerSourcePath::Local(_)))
|
||||
.map(|handler| {
|
||||
let mut run = dispatcher::running_summary(&handler);
|
||||
run.status = HookRunStatus::Failed;
|
||||
|
||||
@@ -528,7 +528,7 @@ mod tests {
|
||||
timeout_sec: 5,
|
||||
status_message: Some("running compact hook".to_string()),
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -594,7 +594,7 @@ mod tests {
|
||||
timeout_sec: 5,
|
||||
status_message: Some("running post tool use hook".to_string()),
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -778,7 +778,7 @@ mod tests {
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -65,7 +65,7 @@ fn handler(matcher: Option<&str>) -> ConfiguredHandler {
|
||||
timeout_sec: 2,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -545,7 +545,7 @@ mod tests {
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -14,6 +14,7 @@ use super::common;
|
||||
use crate::engine::ClaudeHooksEngine;
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::HandlerRunResult;
|
||||
use crate::engine::HandlerSourcePath;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::schema::NullableString;
|
||||
@@ -88,6 +89,7 @@ pub(crate) fn preview(
|
||||
request.target.matcher_input(),
|
||||
)
|
||||
.into_iter()
|
||||
.filter(|handler| matches!(handler.source_path, HandlerSourcePath::Local(_)))
|
||||
.map(|handler| dispatcher::running_summary(&handler))
|
||||
.collect()
|
||||
}
|
||||
@@ -645,7 +647,7 @@ mod tests {
|
||||
timeout_sec: 600,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -466,7 +466,7 @@ mod tests {
|
||||
timeout_sec: 5,
|
||||
status_message: None,
|
||||
additional_context_limit: Default::default(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs().into(),
|
||||
source: codex_protocol::protocol::HookSource::User,
|
||||
display_order: 0,
|
||||
kind: crate::engine::ConfiguredHandlerKind::Command {
|
||||
|
||||
@@ -27,6 +27,7 @@ use crate::types::HookPayload;
|
||||
use crate::types::HookResponse;
|
||||
use async_channel::Receiver;
|
||||
use codex_config::ConfigLayerStack;
|
||||
use codex_plugin::ExecutorPluginHookSource;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::shell_environment::scrub_non_inheritable_env_vars;
|
||||
@@ -95,6 +96,12 @@ impl Hooks {
|
||||
)
|
||||
}
|
||||
|
||||
pub fn with_executor_hooks(&self, executor_hooks: Vec<ExecutorPluginHookSource>) -> Self {
|
||||
let mut hooks = self.clone();
|
||||
hooks.engine.set_executor_hooks(executor_hooks);
|
||||
hooks
|
||||
}
|
||||
|
||||
fn from_config(
|
||||
config: HooksConfig,
|
||||
mcp_executor: Arc<dyn HookMcpExecutor>,
|
||||
|
||||
@@ -11,6 +11,7 @@ mod provider;
|
||||
|
||||
use codex_config::HookEventsToml;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
pub use load_outcome::LoadedPlugin;
|
||||
pub use load_outcome::PluginLoadOutcome;
|
||||
pub use load_outcome::prompt_safe_plugin_description;
|
||||
@@ -67,6 +68,17 @@ pub struct PluginHookSource {
|
||||
pub hooks: HookEventsToml,
|
||||
}
|
||||
|
||||
/// Inline plugin hooks whose paths and MCP target belong to an executor environment.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ExecutorPluginHookSource {
|
||||
pub plugin_id: PluginId,
|
||||
pub environment_id: String,
|
||||
pub plugin_root: PathUri,
|
||||
pub manifest_path: PathUri,
|
||||
pub source_relative_path: String,
|
||||
pub hooks: HookEventsToml,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct PluginTelemetryMetadata {
|
||||
/// Local plugin identifier used by Codex configuration and the plugin cache,
|
||||
|
||||
Reference in New Issue
Block a user