diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 186e96311b..5085b6d114 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -39,6 +39,7 @@ struct HookHandlerSource<'a> { path: &'a AbsolutePathBuf, key_source: String, source: HookSource, + is_managed: bool, hook_states: &'a HashMap, env: HashMap, plugin_id: Option, @@ -95,6 +96,7 @@ pub(crate) fn discover_handlers( path: &source_path, key_source: source_path.display().to_string(), source: hook_source, + is_managed: hook_source.is_managed(), hook_states: &hook_states, env: HashMap::new(), plugin_id: None, @@ -146,6 +148,7 @@ fn append_managed_requirement_handlers( path: &source_path, key_source: source_path.display().to_string(), source: hook_source_for_requirement_source(managed_hooks.source.as_ref()), + is_managed: true, hook_states, env: HashMap::new(), plugin_id: None, @@ -190,6 +193,7 @@ fn append_plugin_hook_sources( path: &source_path, key_source: format!("{plugin_id}:{source_relative_path}"), source: HookSource::Plugin, + is_managed: false, hook_states, env, plugin_id: Some(plugin_id), @@ -418,13 +422,13 @@ fn append_matcher_groups( handler_index ); let state = source.hook_states.get(&key); - let enabled = source.source.is_managed() - || state.and_then(|state| state.enabled) != Some(false); - let trusted_hash = (!source.source.is_managed()) + let enabled = + source.is_managed || state.and_then(|state| state.enabled) != Some(false); + let trusted_hash = (!source.is_managed) .then(|| state.and_then(|state| state.trusted_hash.clone())) .flatten(); let trust_status = - hook_trust_status(source.source.is_managed(), ¤t_hash, &trusted_hash); + hook_trust_status(source.is_managed, ¤t_hash, &trusted_hash); hook_entries.push(HookListEntry { key, event_name, @@ -438,7 +442,7 @@ fn append_matcher_groups( plugin_id: source.plugin_id.clone(), display_order: *display_order, enabled, - is_managed: source.source.is_managed(), + is_managed: source.is_managed, current_hash, trusted_hash, trust_status, @@ -610,6 +614,7 @@ mod tests { path, key_source: path.display().to_string(), source: hook_source(), + is_managed: hook_source().is_managed(), hook_states, env: std::collections::HashMap::new(), plugin_id: None, diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index e01cb8f820..fd9bc1b3c4 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -170,6 +170,69 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: assert!(log_contents.contains("\"hook_event_name\": \"PreToolUse\"")); } +#[test] +fn unknown_requirement_source_hooks_stay_managed() { + let temp = tempdir().expect("create temp dir"); + let managed_dir = + AbsolutePathBuf::try_from(temp.path().join("managed-hooks")).expect("absolute path"); + fs::create_dir_all(managed_dir.as_path()).expect("create managed hooks dir"); + let managed_hooks = managed_hooks_for_current_platform( + managed_dir, + HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "python3 /tmp/managed.py".to_string(), + timeout_sec: Some(10), + r#async: false, + status_message: Some("checking".to_string()), + }], + }], + ..Default::default() + }, + ); + let config_layer_stack = ConfigLayerStack::new( + Vec::new(), + ConfigRequirements { + managed_hooks: Some(ConstrainedWithSource::new( + Constrained::allow_any(managed_hooks.clone()), + Some(RequirementSource::Unknown), + )), + ..ConfigRequirements::default() + }, + ConfigRequirementsToml { + hooks: Some(managed_hooks), + ..ConfigRequirementsToml::default() + }, + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert_eq!(engine.handlers.len(), 1); + assert_eq!(engine.handlers[0].source, HookSource::Unknown); + let discovered = + super::discovery::discover_handlers(Some(&config_layer_stack), Vec::new(), Vec::new()); + assert_eq!(discovered.hook_entries.len(), 1); + assert_eq!(discovered.hook_entries[0].source, HookSource::Unknown); + assert_eq!(discovered.hook_entries[0].enabled, true); + assert_eq!(discovered.hook_entries[0].is_managed, true); + assert_eq!(discovered.hook_entries[0].trusted_hash, None); + assert_eq!( + discovered.hook_entries[0].trust_status, + HookTrustStatus::Managed + ); +} + #[test] fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { let temp = tempdir().expect("create temp dir");