From 0d8dee94278af682eb8c3befec10cb67e7caba2c Mon Sep 17 00:00:00 2001 From: Abhinav Date: Thu, 11 Jun 2026 15:25:55 -0700 Subject: [PATCH] [codex] Avoid duplicate hooks.json discovery with profiles (#26418) ## Summary V2 profiles add both `config.toml` and `.config.toml` to the config stack. Because both user layers resolve hook discovery to the same Codex home, Codex loaded the same `hooks.json` twice. This duplicated hook rows and caused each matching command to run twice. Deduplicate JSON hook discovery by absolute config folder within each effective config stack. TOML hooks remain layer-specific, and multi-cwd `hooks/list` results remain independently resolved per cwd. ## Reproduction 1. Add `config.toml` and `work.config.toml` under `$CODEX_HOME`. 2. Add one command hook to `$CODEX_HOME/hooks.json`. 3. Run Codex with `--profile work`. 4. Trigger the hook. Before this change, one declaration creates two handlers. Afterward, it creates one. Fixes #25645 and addresses the single-cwd duplication in #25437. ## Validation - `cargo nextest run -p codex-hooks` - `just fix -p codex-hooks` - `just fmt` - `just argument-comment-lint -p codex-hooks` --- codex-rs/hooks/src/engine/discovery.rs | 9 ++- codex-rs/hooks/src/engine/mod_tests.rs | 91 ++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 92d17fbab..cab28b64a 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::collections::HashSet; use std::fs; use std::path::Path; @@ -69,6 +70,7 @@ pub(crate) fn discover_handlers( let mut hook_entries = Vec::new(); let mut warnings = plugin_hook_load_warnings; let mut display_order = 0_i64; + let mut visited_json_hook_folders = HashSet::new(); let hook_states = hook_states_from_stack(config_layer_stack); let policy = HookDiscoveryPolicy { allow_managed_hooks_only: config_layer_stack.is_some_and(|config_layer_stack| { @@ -111,7 +113,12 @@ pub(crate) fn discover_handlers( if !policy.allows(&policy_source) { continue; } - let json_hooks = load_hooks_json(layer.hooks_config_folder().as_deref(), &mut warnings); + let json_hooks = match layer.hooks_config_folder() { + Some(config_folder) if visited_json_hook_folders.insert(config_folder.clone()) => { + load_hooks_json(Some(config_folder.as_path()), &mut warnings) + } + _ => None, + }; let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings); if let (Some((json_source_path, json_events)), Some((toml_source_path, toml_events))) = diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 4b7697b34..d5b34b3f4 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -1134,6 +1134,97 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { assert_eq!(preview[1].source_path, config_path); } +#[test] +fn profile_user_layers_load_shared_hooks_json_once() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"); + let profile_path = AbsolutePathBuf::try_from(temp.path().join("work.config.toml")) + .expect("absolute profile path"); + let hooks_json_path = + AbsolutePathBuf::try_from(temp.path().join("hooks.json")).expect("absolute hooks path"); + fs::write( + hooks_json_path.as_path(), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "^Bash$", + "hooks": [ + { + "type": "command", + "command": "python3 /tmp/json-hook.py" + } + ] + } + ] + } + }"#, + ) + .expect("write hooks.json"); + let config_layer_stack = ConfigLayerStack::new( + vec![ + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: config_path, + profile: None, + }, + TomlValue::Table(Default::default()), + ), + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: profile_path, + profile: Some("work".to_string()), + }, + TomlValue::Table(Default::default()), + ), + ], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*bypass_hook_trust*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + assert!(engine.warnings().is_empty()); + assert_eq!(engine.handlers.len(), 1); + let preview = engine.preview_pre_tool_use(&PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + subagent: None, + cwd: cwd(), + transcript_path: None, + model: "gpt-test".to_string(), + permission_mode: "default".to_string(), + tool_name: "Bash".to_string(), + matcher_aliases: Vec::new(), + tool_use_id: "tool-1".to_string(), + tool_input: serde_json::json!({ "command": "echo hello" }), + }); + assert_eq!(preview.len(), 1); + assert_eq!(preview[0].source_path, hooks_json_path); + + let listed = crate::list_hooks(crate::HooksConfig { + feature_enabled: true, + bypass_hook_trust: true, + config_layer_stack: Some(config_layer_stack), + ..Default::default() + }); + assert!(listed.warnings.is_empty()); + assert_eq!(listed.hooks.len(), 1); + assert_eq!(listed.hooks[0].source_path, hooks_json_path); +} + #[tokio::test] async fn plugin_hook_sources_run_with_plugin_env_and_plugin_source() { let temp = tempdir().expect("create temp dir");