From d00db24b3747ee7190f26bb2a05c1d863fb8b8a2 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sat, 2 May 2026 11:54:10 -0700 Subject: [PATCH] Simplify hook metadata mapping --- codex-rs/core/src/session/tests.rs | 2 +- .../runtimes/shell/unix_escalation_tests.rs | 11 ++-- codex-rs/hooks/src/engine/discovery.rs | 61 ++++++++----------- 3 files changed, 31 insertions(+), 43 deletions(-) diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 8e850bc47a..9020e039b4 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1171,7 +1171,7 @@ async fn reload_user_config_layer_refreshes_hooks() -> anyhow::Result<()> { .await?; let codex_home = session.codex_home().await; std::fs::create_dir_all(&codex_home)?; - let config_toml_path = AbsolutePathBuf::try_from(codex_home.join(CONFIG_TOML_FILE))?; + let config_toml_path = codex_home.join(CONFIG_TOML_FILE); let user_config: codex_config::TomlValue = serde_json::from_value(serde_json::json!({ "hooks": { "SessionStart": [{ diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 8da6410a17..7c2aa5e8e5 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -371,13 +371,10 @@ async fn execve_permission_request_hook_short_circuits_prompt() -> anyhow::Resul .to_string(), ) .context("write hooks.json")?; - let config_toml_path = AbsolutePathBuf::try_from( - turn_context - .config - .codex_home - .join(codex_config::CONFIG_TOML_FILE), - ) - .context("build config.toml path")?; + let config_toml_path = turn_context + .config + .codex_home + .join(codex_config::CONFIG_TOML_FILE); let hook_list = codex_hooks::list_hooks(HooksConfig { feature_enabled: true, config_layer_stack: Some(turn_context.config.config_layer_stack.clone()), diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index c044869462..b09b005ae6 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -70,8 +70,7 @@ pub(crate) fn discover_handlers( ConfigLayerStackOrdering::LowestPrecedenceFirst, /*include_disabled*/ false, ) { - let hook_source = hook_source_for_config_layer_source(&layer.name); - let is_managed = is_managed_config_layer_source(&layer.name); + let (hook_source, is_managed) = hook_metadata_for_config_layer_source(&layer.name); let json_hooks = load_hooks_json(layer.config_folder().as_deref(), &mut warnings); let toml_hooks = load_toml_hooks_from_layer(layer, &mut warnings); @@ -557,30 +556,22 @@ fn hook_trusted_hash(is_managed: bool, state: Option<&HookStateToml>) -> Option< .flatten() } -fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource { +fn hook_metadata_for_config_layer_source(source: &ConfigLayerSource) -> (HookSource, bool) { match source { - ConfigLayerSource::System { .. } => HookSource::System, - ConfigLayerSource::User { .. } => HookSource::User, - ConfigLayerSource::Project { .. } => HookSource::Project, - ConfigLayerSource::Mdm { .. } => HookSource::Mdm, - ConfigLayerSource::SessionFlags => HookSource::SessionFlags, + ConfigLayerSource::System { .. } => (HookSource::System, true), + ConfigLayerSource::User { .. } => (HookSource::User, false), + ConfigLayerSource::Project { .. } => (HookSource::Project, false), + ConfigLayerSource::Mdm { .. } => (HookSource::Mdm, true), + ConfigLayerSource::SessionFlags => (HookSource::SessionFlags, false), ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } => { - HookSource::LegacyManagedConfigFile + (HookSource::LegacyManagedConfigFile, true) + } + ConfigLayerSource::LegacyManagedConfigTomlFromMdm => { + (HookSource::LegacyManagedConfigMdm, true) } - ConfigLayerSource::LegacyManagedConfigTomlFromMdm => HookSource::LegacyManagedConfigMdm, } } -fn is_managed_config_layer_source(source: &ConfigLayerSource) -> bool { - matches!( - source, - ConfigLayerSource::System { .. } - | ConfigLayerSource::Mdm { .. } - | ConfigLayerSource::LegacyManagedConfigTomlFromFile { .. } - | ConfigLayerSource::LegacyManagedConfigTomlFromMdm - ) -} - fn hook_source_for_requirement_source(source: Option<&RequirementSource>) -> HookSource { match source { Some(RequirementSource::MdmManagedPreferences { .. }) => HookSource::Mdm, @@ -818,50 +809,50 @@ mod tests { } #[test] - fn hook_source_for_config_layer_source_discards_source_details() { + fn hook_metadata_for_config_layer_source_discards_source_details() { let config_file = test_path_buf("/tmp/.codex/config.toml").abs(); let dot_codex_folder = test_path_buf("/tmp/worktree/.codex").abs(); assert_eq!( - super::hook_source_for_config_layer_source(&ConfigLayerSource::System { + super::hook_metadata_for_config_layer_source(&ConfigLayerSource::System { file: config_file.clone(), }), - HookSource::System, + (HookSource::System, true), ); assert_eq!( - super::hook_source_for_config_layer_source(&ConfigLayerSource::User { + super::hook_metadata_for_config_layer_source(&ConfigLayerSource::User { file: config_file.clone(), }), - HookSource::User, + (HookSource::User, false), ); assert_eq!( - super::hook_source_for_config_layer_source(&ConfigLayerSource::Project { + super::hook_metadata_for_config_layer_source(&ConfigLayerSource::Project { dot_codex_folder }), - HookSource::Project, + (HookSource::Project, false), ); assert_eq!( - super::hook_source_for_config_layer_source(&ConfigLayerSource::Mdm { + super::hook_metadata_for_config_layer_source(&ConfigLayerSource::Mdm { domain: "com.openai.codex".to_string(), key: "config".to_string(), }), - HookSource::Mdm, + (HookSource::Mdm, true), ); assert_eq!( - super::hook_source_for_config_layer_source(&ConfigLayerSource::SessionFlags), - HookSource::SessionFlags, + super::hook_metadata_for_config_layer_source(&ConfigLayerSource::SessionFlags), + (HookSource::SessionFlags, false), ); assert_eq!( - super::hook_source_for_config_layer_source( + super::hook_metadata_for_config_layer_source( &ConfigLayerSource::LegacyManagedConfigTomlFromFile { file: config_file }, ), - HookSource::LegacyManagedConfigFile, + (HookSource::LegacyManagedConfigFile, true), ); assert_eq!( - super::hook_source_for_config_layer_source( + super::hook_metadata_for_config_layer_source( &ConfigLayerSource::LegacyManagedConfigTomlFromMdm, ), - HookSource::LegacyManagedConfigMdm, + (HookSource::LegacyManagedConfigMdm, true), ); } }