diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs index a7b90a8c0e..e8e6577a5a 100644 --- a/codex-rs/hooks/src/engine/command_runner_tests.rs +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -512,9 +512,9 @@ fn write_handler(temp: &TempDir, source: &str) -> ConfiguredHandler { async fn schedule(runtime: &CommandHookRuntime, handler: ConfiguredHandler, cwd: &Path) { let engine = ClaudeHooksEngine { - handlers: vec![handler], - warnings: Vec::new(), - required_load_errors: Vec::new(), + handlers: Arc::new(vec![handler]), + warnings: Arc::default(), + required_load_errors: Arc::default(), command_runtime: runtime.clone(), mcp_executor: mcp_executor(), }; diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index a007f3b1a4..5f4972d407 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -221,9 +221,10 @@ pub struct HookListEntry { #[derive(Clone)] pub(crate) struct ClaudeHooksEngine { - pub(crate) handlers: Vec, - warnings: Vec, - required_load_errors: Vec, + // Per-turn views share configuration; executor overlays copy handlers on mutation. + pub(crate) handlers: Arc>, + warnings: Arc<[String]>, + required_load_errors: Arc<[String]>, pub(crate) command_runtime: CommandHookRuntime, pub(crate) mcp_executor: Arc, } @@ -240,9 +241,9 @@ impl ClaudeHooksEngine { ) -> Self { if !enabled && plugin_hook_sources.is_empty() { return Self { - handlers: Vec::new(), - warnings: Vec::new(), - required_load_errors: Vec::new(), + handlers: Arc::default(), + warnings: Arc::default(), + required_load_errors: Arc::default(), command_runtime, mcp_executor, }; @@ -262,9 +263,9 @@ impl ClaudeHooksEngine { discovered.required_load_errors.clear(); } Self { - handlers: discovered.handlers, - warnings: discovered.warnings, - required_load_errors: discovered.required_load_errors, + handlers: Arc::new(discovered.handlers), + warnings: discovered.warnings.into(), + required_load_errors: discovered.required_load_errors.into(), command_runtime, mcp_executor, } @@ -275,15 +276,26 @@ impl ClaudeHooksEngine { } pub(crate) fn set_executor_hooks(&mut self, executor_hooks: Vec) { - self.handlers.retain(|handler| { + if executor_hooks.is_empty() + && !self.handlers.iter().any(|handler| { + matches!( + handler.source_path, + HandlerSourcePath::ExecutorScoped { .. } + ) + }) + { + return; + } + + let handlers = Arc::make_mut(&mut self.handlers); + handlers.retain(|handler| { !matches!( handler.source_path, HandlerSourcePath::ExecutorScoped { .. } ) }); - let mut display_order = self - .handlers + let mut display_order = handlers .iter() .map(|handler| handler.display_order) .max() @@ -320,7 +332,7 @@ impl ClaudeHooksEngine { if !seen_targets.insert(target) { continue; } - self.handlers.push(ConfiguredHandler { + handlers.push(ConfiguredHandler { builtin: true, event_name, matcher: None, diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index a185e45029..e7484cc4d6 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -115,13 +115,13 @@ fn permission_request_timeout_only_counts_synchronous_handlers() { ..synchronous_handler.clone() }; - engine.handlers = vec![synchronous_handler, asynchronous_handler.clone()]; + engine.handlers = Arc::new(vec![synchronous_handler, asynchronous_handler.clone()]); assert_eq!( engine.max_permission_request_timeout(), Duration::from_secs(5) ); - engine.handlers = vec![asynchronous_handler]; + engine.handlers = Arc::new(vec![asynchronous_handler]); assert_eq!(engine.max_permission_request_timeout(), Duration::ZERO); } @@ -1960,7 +1960,7 @@ fn builtin_cleanup_ignores_disablement_but_preserves_managed_only_policy() { }), mcp_executor(), ); - assert_eq!(feature_disabled.handlers, expected); + assert_eq!(feature_disabled.handlers.as_ref(), &expected); } } @@ -1996,7 +1996,10 @@ fn disabled_hooks_feature_keeps_builtin_cleanup_but_not_trusted_plugin_hooks() { }), mcp_executor(), ); - assert_eq!(engine.handlers, vec![discovered.handlers[0].clone()]); + assert_eq!( + engine.handlers.as_slice(), + &[discovered.handlers[0].clone()] + ); } #[tokio::test] @@ -2310,8 +2313,8 @@ fn executor_stop_hook_fixture() -> ( }; engine.set_executor_hooks(vec![source.clone()]); assert_eq!( - engine.handlers, - vec![ConfiguredHandler { + engine.handlers.as_slice(), + &[ConfiguredHandler { builtin: true, event_name: HookEventName::Stop, matcher: None, @@ -2400,7 +2403,7 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() { vec![expected_executor_call.clone()] ); - engine.handlers.push(ConfiguredHandler { + Arc::make_mut(&mut engine.handlers).push(ConfiguredHandler { builtin: false, event_name: HookEventName::Stop, matcher: None, @@ -2435,7 +2438,7 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() { ] ); - engine.handlers.push(ConfiguredHandler { + Arc::make_mut(&mut engine.handlers).push(ConfiguredHandler { builtin: false, event_name: HookEventName::Stop, matcher: None, @@ -2493,7 +2496,8 @@ async fn executor_stop_hooks_run_unless_regular_hooks_block_without_stopping() { #[test] fn executor_stop_hooks_register_each_target_environment_once() { let (mut engine, _, _, _, first_source) = executor_stop_hook_fixture(); - let mut expected_handlers = engine.handlers.clone(); + let previous = engine.clone(); + let mut expected_handlers = engine.handlers.as_ref().clone(); let mut duplicate_target = first_source.clone(); duplicate_target.plugin_id = PluginId::parse("chrome@openai-bundled").expect("valid plugin ID"); let mut second_source = first_source.clone(); @@ -2508,7 +2512,8 @@ fn executor_stop_hooks_register_each_target_environment_once() { expected_handlers.push(second_handler); engine.set_executor_hooks(vec![first_source, duplicate_target, second_source]); - assert_eq!(engine.handlers, expected_handlers); + assert_eq!(engine.handlers.as_ref(), &expected_handlers); + assert_eq!(previous.handlers.as_slice(), &expected_handlers[..1]); } #[tokio::test] @@ -2558,7 +2563,7 @@ fn executor_hooks_register_events_from_each_environment() { let (mut engine, _, _, _, mut first_source) = executor_stop_hook_fixture(); first_source.hooks.interrupt = std::mem::take(&mut first_source.hooks.stop); engine.set_executor_hooks(vec![first_source.clone()]); - let mut expected_handlers = engine.handlers.clone(); + let mut expected_handlers = engine.handlers.as_ref().clone(); let mut second_source = first_source.clone(); second_source.environment_id = "executor-b".to_string(); second_source.hooks.stop = std::mem::take(&mut second_source.hooks.interrupt); @@ -2574,7 +2579,7 @@ fn executor_hooks_register_events_from_each_environment() { engine.set_executor_hooks(vec![first_source, second_source]); - assert_eq!(engine.handlers, expected_handlers); + assert_eq!(engine.handlers.as_ref(), &expected_handlers); } #[tokio::test] @@ -2624,7 +2629,7 @@ async fn memory_consolidation_stop_preserves_policy_and_executor_cleanup() { tool: policy_call.tool.clone(), input: policy_call.input.clone(), }; - engine.handlers.push(handler); + Arc::make_mut(&mut engine.handlers).push(handler); assert_eq!( engine.preview_stop(&request).len(), usize::from(runs_policy) diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 2049ac6c95..b8703bf9c9 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -61,10 +61,10 @@ pub struct Hooks { // TODO: Once legacy `notify` is removed, capture this snapshot in `CommandHookRuntime::new` // and remove the environment plumbing from `Hooks` and `from_config`. environment: Arc>, - after_agent: Vec, + after_agent: Arc<[Hook]>, engine: ClaudeHooksEngine, - plugin_hook_sources: Vec, - plugin_hook_load_warnings: Vec, + plugin_hook_sources: Arc<[PluginHookSource]>, + plugin_hook_load_warnings: Arc<[String]>, } impl Hooks { @@ -144,8 +144,8 @@ impl Hooks { environment, after_agent, engine, - plugin_hook_sources: config.plugin_hook_sources, - plugin_hook_load_warnings: config.plugin_hook_load_warnings, + plugin_hook_sources: config.plugin_hook_sources.into(), + plugin_hook_load_warnings: config.plugin_hook_load_warnings.into(), } }