diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 438bcb4c37..236ef471e6 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -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", diff --git a/codex-rs/core-plugins/src/executor_hooks.rs b/codex-rs/core-plugins/src/executor_hooks.rs new file mode 100644 index 0000000000..58d1ea6266 --- /dev/null +++ b/codex-rs/core-plugins/src/executor_hooks.rs @@ -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 { + 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 { + 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; diff --git a/codex-rs/core-plugins/src/executor_hooks_tests.rs b/codex-rs/core-plugins/src/executor_hooks_tests.rs new file mode 100644 index 0000000000..79682fac01 --- /dev/null +++ b/codex-rs/core-plugins/src/executor_hooks_tests.rs @@ -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::::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::::new() + ); +} diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 6aed23f44f..328a9a6c75 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -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; diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index af2a31e944..c4b9354981 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -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, - turn_context: &Arc, + step_context: &Arc, stop_hook_active: bool, last_assistant_message: Option, ) -> 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; diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 9561099eac..d3dd0b8098 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -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(), ) diff --git a/codex-rs/hooks/Cargo.toml b/codex-rs/hooks/Cargo.toml index 32149dc650..0cdf84418a 100644 --- a/codex-rs/hooks/Cargo.toml +++ b/codex-rs/hooks/Cargo.toml @@ -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 } diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index 6f9a6560c7..b68474f0cf 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -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, parse: fn(&ConfiguredHandler, HandlerRunResult, Option) -> ParsedHandler, ) { - 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 + 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(); diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs index 952048de8d..cb1ff333d4 100644 --- a/codex-rs/hooks/src/engine/command_runner_tests.rs +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -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 { diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 7fad77da2e..2031e7909c 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -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 { diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index a024f75939..cdcc0cae63 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -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( turn_id: Option, parse: fn(&ConfiguredHandler, HandlerRunResult, Option) -> ParsedHandler, ) -> Vec> { + 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( 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, ) -> 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 { diff --git a/codex-rs/hooks/src/engine/mcp_runner.rs b/codex-rs/hooks/src/engine/mcp_runner.rs index 9ca497ce77..ef33a1d871 100644 --- a/codex-rs/hooks/src/engine/mcp_runner.rs +++ b/codex-rs/hooks/src/engine/mcp_runner.rs @@ -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, hook_event: &Value, ) -> Result> { diff --git a/codex-rs/hooks/src/engine/mcp_runner_tests.rs b/codex-rs/hooks/src/engine/mcp_runner_tests.rs index 0f96c785bb..67c9353f4e 100644 --- a/codex-rs/hooks/src/engine/mcp_runner_tests.rs +++ b/codex-rs/hooks/src/engine/mcp_runner_tests.rs @@ -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 { diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 513dc10fac..b7c44303a0 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -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, 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 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) { + 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 } diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 9c2a5e4ed7..e3ed160811 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -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 { 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>>, output: String, + outputs_by_tool: HashMap, } impl HookMcpExecutor for StaticMcpExecutor { fn execute(&self, call: HookMcpCall) -> BoxFuture<'_, anyhow::Result> { 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>>, + 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>>, 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, + release: Arc, + } + + impl HookMcpExecutor for BlockingMcpExecutor { + fn execute(&self, _call: HookMcpCall) -> BoxFuture<'_, anyhow::Result> { + 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, diff --git a/codex-rs/hooks/src/events/common.rs b/codex-rs/hooks/src/events/common.rs index c67602086c..085f1390ce 100644 --- a/codex-rs/hooks/src/events/common.rs +++ b/codex-rs/hooks/src/events/common.rs @@ -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 { handlers .into_iter() + .filter(|handler| matches!(handler.source_path, HandlerSourcePath::Local(_))) .map(|handler| { let mut run = dispatcher::running_summary(&handler); run.status = HookRunStatus::Failed; diff --git a/codex-rs/hooks/src/events/compact.rs b/codex-rs/hooks/src/events/compact.rs index 2013d6c762..bf9d563484 100644 --- a/codex-rs/hooks/src/events/compact.rs +++ b/codex-rs/hooks/src/events/compact.rs @@ -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 { diff --git a/codex-rs/hooks/src/events/post_tool_use.rs b/codex-rs/hooks/src/events/post_tool_use.rs index 5f1dfa5850..f2a36e4338 100644 --- a/codex-rs/hooks/src/events/post_tool_use.rs +++ b/codex-rs/hooks/src/events/post_tool_use.rs @@ -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 { diff --git a/codex-rs/hooks/src/events/pre_tool_use.rs b/codex-rs/hooks/src/events/pre_tool_use.rs index 7233c014a9..137731357a 100644 --- a/codex-rs/hooks/src/events/pre_tool_use.rs +++ b/codex-rs/hooks/src/events/pre_tool_use.rs @@ -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 { diff --git a/codex-rs/hooks/src/events/session_end_tests.rs b/codex-rs/hooks/src/events/session_end_tests.rs index d2cf0183b7..2197bae5a2 100644 --- a/codex-rs/hooks/src/events/session_end_tests.rs +++ b/codex-rs/hooks/src/events/session_end_tests.rs @@ -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 { diff --git a/codex-rs/hooks/src/events/session_start.rs b/codex-rs/hooks/src/events/session_start.rs index 784ab1d074..07604c30a4 100644 --- a/codex-rs/hooks/src/events/session_start.rs +++ b/codex-rs/hooks/src/events/session_start.rs @@ -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 { diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index 5316867065..6bfee00efa 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -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 { diff --git a/codex-rs/hooks/src/events/user_prompt_submit.rs b/codex-rs/hooks/src/events/user_prompt_submit.rs index ebc63bbfa1..c70deba2f7 100644 --- a/codex-rs/hooks/src/events/user_prompt_submit.rs +++ b/codex-rs/hooks/src/events/user_prompt_submit.rs @@ -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 { diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 82d26ee901..5290e50f81 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -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) -> Self { + let mut hooks = self.clone(); + hooks.engine.set_executor_hooks(executor_hooks); + hooks + } + fn from_config( config: HooksConfig, mcp_executor: Arc, diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 424ec2a203..947d8ea3b7 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -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,