From c2a3b5a2899e63765f253b4fc77856188701c521 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sun, 26 Apr 2026 20:34:16 -0700 Subject: [PATCH] Add hook selector toggle semantics --- .../schema/json/ClientRequest.json | 10 +- .../codex_app_server_protocol.schemas.json | 10 +- .../codex_app_server_protocol.v2.schemas.json | 10 +- .../json/v2/HooksConfigWriteParams.json | 10 +- .../schema/typescript/v2/HookConfigSource.ts | 2 +- .../typescript/v2/HooksConfigWriteParams.ts | 2 +- .../app-server-protocol/src/protocol/v2.rs | 6 +- codex-rs/app-server/README.md | 21 +- .../app-server/src/codex_message_processor.rs | 56 +++- .../app-server/tests/suite/v2/config_rpc.rs | 94 ++++++ codex-rs/config/src/hook_config.rs | 6 +- codex-rs/config/src/hooks_tests.rs | 27 ++ codex-rs/core/config.schema.json | 8 +- codex-rs/core/src/config/edit.rs | 116 +++++-- codex-rs/core/src/config/edit_tests.rs | 39 ++- codex-rs/hooks/src/engine/config_rules.rs | 117 ++++++- codex-rs/hooks/src/engine/discovery.rs | 45 +-- codex-rs/hooks/src/engine/inventory.rs | 203 ++++++++++-- codex-rs/hooks/src/engine/mod_tests.rs | 294 ++++++++++++++++++ codex-rs/hooks/src/lib.rs | 1 + 20 files changed, 972 insertions(+), 105 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 3a078e258b..64cfac9391 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -1449,7 +1449,9 @@ }, "HookConfigSource": { "enum": [ - "plugin" + "plugin", + "user", + "project" ], "type": "string" }, @@ -1469,6 +1471,12 @@ }, "source": { "$ref": "#/definitions/HookConfigSource" + }, + "sourcePath": { + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index e5ed9e4f7a..11373e93e4 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -9617,7 +9617,9 @@ }, "HookConfigSource": { "enum": [ - "plugin" + "plugin", + "user", + "project" ], "type": "string" }, @@ -9922,6 +9924,12 @@ }, "source": { "$ref": "#/definitions/v2/HookConfigSource" + }, + "sourcePath": { + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 500dffb7ad..d61263d34c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -6247,7 +6247,9 @@ }, "HookConfigSource": { "enum": [ - "plugin" + "plugin", + "user", + "project" ], "type": "string" }, @@ -6552,6 +6554,12 @@ }, "source": { "$ref": "#/definitions/HookConfigSource" + }, + "sourcePath": { + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/HooksConfigWriteParams.json b/codex-rs/app-server-protocol/schema/json/v2/HooksConfigWriteParams.json index 336dda66ff..17f3587a54 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HooksConfigWriteParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HooksConfigWriteParams.json @@ -3,7 +3,9 @@ "definitions": { "HookConfigSource": { "enum": [ - "plugin" + "plugin", + "user", + "project" ], "type": "string" } @@ -23,6 +25,12 @@ }, "source": { "$ref": "#/definitions/HookConfigSource" + }, + "sourcePath": { + "type": [ + "string", + "null" + ] } }, "required": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookConfigSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookConfigSource.ts index 32e48df25a..ff688a18da 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookConfigSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookConfigSource.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type HookConfigSource = "plugin"; +export type HookConfigSource = "plugin" | "user" | "project"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HooksConfigWriteParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HooksConfigWriteParams.ts index a4ce18b035..8485c4263f 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HooksConfigWriteParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HooksConfigWriteParams.ts @@ -3,4 +3,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { HookConfigSource } from "./HookConfigSource"; -export type HooksConfigWriteParams = { source: HookConfigSource, pluginId?: string | null, key: string, enabled: boolean, }; +export type HooksConfigWriteParams = { source: HookConfigSource, pluginId?: string | null, sourcePath?: string | null, key: string, enabled: boolean, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 96ea627dd4..9b841810fc 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -149,7 +149,7 @@ macro_rules! v2_enum_from_core { }; } -#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub enum NonSteerableTurnKind { @@ -4669,6 +4669,8 @@ pub struct HookMetadata { #[ts(export_to = "v2/")] pub enum HookConfigSource { Plugin, + User, + Project, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -4678,6 +4680,8 @@ pub struct HooksConfigWriteParams { pub source: HookConfigSource, #[ts(optional = nullable)] pub plugin_id: Option, + #[ts(optional = nullable)] + pub source_path: Option, pub key: String, pub enabled: bool, } diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 8d36e035cb..9c61e9121e 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -206,8 +206,8 @@ Example with notification opt-out: - `device/key/public` — return a device key's SPKI DER public key as base64 plus its `algorithm` and `protectionClass`. - `device/key/sign` — sign one of the accepted structured payload variants with a controller-local device key. The only accepted payload today is `remoteControlClientConnection`, which binds a server-issued `/client` websocket challenge to the enrolled controller device without signing the bearer token itself; this is intentionally not an arbitrary-byte signing API. - `skills/config/write` — write user-level skill config by name or absolute path. -- `hooks/list` — list plugin-bundled hooks for one or more `cwd` values, including stable hook keys and effective per-hook enabled state from user/session config. -- `hooks/config/write` — write user-level plugin hook enablement config by `pluginId` and stable hook `key`. +- `hooks/list` — list discovered hooks for one or more `cwd` values, including source metadata, stable hook keys, and effective per-hook enabled state from user/session config. +- `hooks/config/write` — write user-level hook enablement config for plugin hooks by `pluginId` and for user/project hooks by `sourcePath`. - `plugin/install` — install a plugin from a discovered marketplace entry, rejecting marketplace entries marked unavailable for install, install MCPs if any, and return the effective plugin auth policy plus any apps that still need auth (**under development; do not call from production clients yet**). - `plugin/uninstall` — uninstall a plugin by id by removing its cached files and clearing its user-level config entry (**under development; do not call from production clients yet**). - `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; returns an `authorization_url` and later emits `mcpServer/oauthLogin/completed` once the browser flow finishes. @@ -1452,7 +1452,7 @@ To enable or disable a skill by name: } ``` -To list plugin-bundled hooks: +To list discovered hooks: ```json { @@ -1479,6 +1479,21 @@ To enable or disable a plugin-bundled hook: } ``` +To enable or disable a user or project hook, use the hook's `sourcePath` and `key` returned by `hooks/list`: + +```json +{ + "method": "hooks/config/write", + "id": 30, + "params": { + "source": "project", + "sourcePath": "/Users/alice/project/.codex/hooks.json", + "key": "PreToolUse:0:0", + "enabled": false + } +} +``` + ## Apps Use `app/list` to fetch available apps (connectors). Each entry includes metadata like the app `id`, display `name`, `installUrl`, `branding`, `appMetadata`, `labels`, whether it is currently accessible, and whether it is enabled in config. diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 84c0e1c811..910571966a 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -256,6 +256,7 @@ use codex_core::config::NetworkProxyAuditMetadata; use codex_core::config::ThreadStoreConfig; use codex_core::config::edit::ConfigEdit; use codex_core::config::edit::ConfigEditsBuilder; +use codex_core::config::edit::HookConfigEditSelector; use codex_core::config_loader::CloudRequirementsLoadError; use codex_core::config_loader::CloudRequirementsLoadErrorCode; use codex_core::config_loader::project_trust_key; @@ -6972,13 +6973,11 @@ impl CodexMessageProcessor { } else { Vec::new() }; - let hooks = codex_hooks::list_plugin_hooks( - Some(&config.config_layer_stack), - &plugin_hook_sources, - ) - .into_iter() - .map(hook_to_info) - .collect(); + let hooks = + codex_hooks::list_hooks(Some(&config.config_layer_stack), &plugin_hook_sources) + .into_iter() + .map(hook_to_info) + .collect(); data.push(HooksListEntry { cwd, hooks, @@ -7178,6 +7177,7 @@ impl CodexMessageProcessor { let HooksConfigWriteParams { source, plugin_id, + source_path, key, enabled, } = params; @@ -7201,11 +7201,41 @@ impl CodexMessageProcessor { return; } ConfigEdit::SetHookConfig { - plugin_id, - key, + selector: HookConfigEditSelector::Plugin { plugin_id, key }, enabled, } } + ApiHookConfigSource::User | ApiHookConfigSource::Project => { + let Some(source_path) = + source_path.filter(|source_path| !source_path.as_os_str().is_empty()) + else { + self.send_invalid_request_error( + request_id, + format!( + "hooks/config/write requires sourcePath when source is {}", + hook_config_source_label(source) + ), + ) + .await; + return; + }; + if key.trim().is_empty() { + self.send_invalid_request_error( + request_id, + "hooks/config/write requires a non-empty key".to_string(), + ) + .await; + return; + } + let selector = match source { + ApiHookConfigSource::User => HookConfigEditSelector::User { source_path, key }, + ApiHookConfigSource::Project => { + HookConfigEditSelector::Project { source_path, key } + } + ApiHookConfigSource::Plugin => unreachable!("plugin handled above"), + }; + ConfigEdit::SetHookConfig { selector, enabled } + } }; let result = ConfigEditsBuilder::new(&self.config.codex_home) .with_edits([edit]) @@ -9564,6 +9594,14 @@ fn hook_to_info(hook: HookInventoryEntry) -> HookMetadata { } } +fn hook_config_source_label(source: ApiHookConfigSource) -> &'static str { + match source { + ApiHookConfigSource::Plugin => "plugin", + ApiHookConfigSource::User => "user", + ApiHookConfigSource::Project => "project", + } +} + fn local_plugin_interface_to_info(interface: PluginManifestInterface) -> PluginInterface { PluginInterface { display_name: interface.display_name, diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index b5f795740c..c12a70edec 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -14,6 +14,11 @@ use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::ConfigWriteResponse; +use codex_app_server_protocol::HookConfigSource; +use codex_app_server_protocol::HookEventName; +use codex_app_server_protocol::HookSource; +use codex_app_server_protocol::HooksConfigWriteResponse; +use codex_app_server_protocol::HooksListResponse; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::MergeStrategy; @@ -44,6 +49,95 @@ fn write_config(codex_home: &TempDir, contents: &str) -> Result<()> { )?) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn hooks_list_returns_user_config_hooks() -> Result<()> { + let codex_home = TempDir::new()?; + write_config( + &codex_home, + r#" +[hooks] + +[[hooks.SessionStart]] +matcher = "startup" + +[[hooks.SessionStart.hooks]] +type = "command" +command = "echo user" +"#, + )?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_raw_request( + "hooks/list", + Some(json!({ + "cwds": [codex_home.path()], + })), + ) + .await?; + let resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: HooksListResponse = to_response(resp)?; + + assert_eq!(response.data.len(), 1); + assert_eq!(response.data[0].errors, Vec::new()); + assert_eq!(response.data[0].hooks.len(), 1); + let hook = &response.data[0].hooks[0]; + assert_eq!(hook.source, HookSource::User); + assert_eq!(hook.event_name, HookEventName::SessionStart); + assert_eq!(hook.key, "SessionStart:0:0"); + assert_eq!(hook.enabled, true); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn hooks_config_write_persists_project_selector() -> Result<()> { + let codex_home = TempDir::new()?; + let project_hook_path = codex_home.path().join("repo/.codex/hooks.json"); + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_raw_request( + "hooks/config/write", + Some(json!({ + "source": HookConfigSource::Project, + "sourcePath": project_hook_path.clone(), + "key": "PreToolUse:0:0", + "enabled": false, + })), + ) + .await?; + let resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let response: HooksConfigWriteResponse = to_response(resp)?; + assert_eq!(response.effective_enabled, false); + + let contents = std::fs::read_to_string(codex_home.path().join("config.toml"))?; + let expected = format!( + r#"[[hooks.config]] +source = "project" +source_path = "{}" +key = "PreToolUse:0:0" +enabled = false +"#, + project_hook_path.display(), + ); + assert_eq!(contents, expected); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn config_read_returns_effective_and_layers() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/config/src/hook_config.rs b/codex-rs/config/src/hook_config.rs index 4d9c43ec39..2edd3660fa 100644 --- a/codex-rs/config/src/hook_config.rs +++ b/codex-rs/config/src/hook_config.rs @@ -85,10 +85,12 @@ impl HookEventsToml { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum HookConfigSource { Plugin, + User, + Project, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] @@ -96,6 +98,8 @@ pub struct HookConfig { pub source: HookConfigSource, #[serde(default)] pub plugin_id: Option, + #[serde(default)] + pub source_path: Option, pub key: String, pub enabled: bool, } diff --git a/codex-rs/config/src/hooks_tests.rs b/codex-rs/config/src/hooks_tests.rs index 0299583e78..86040f7795 100644 --- a/codex-rs/config/src/hooks_tests.rs +++ b/codex-rs/config/src/hooks_tests.rs @@ -1,4 +1,5 @@ use pretty_assertions::assert_eq; +use std::path::PathBuf; use super::HookConfig; use super::HookConfigSource; @@ -101,12 +102,38 @@ enabled = false vec![HookConfig { source: HookConfigSource::Plugin, plugin_id: Some("openai-curated/superpowers".to_string()), + source_path: None, key: "hooks/hooks.json:SessionStart:0:0".to_string(), enabled: false, }] ); } +#[test] +fn hook_events_deserialize_project_config_overrides() { + let parsed: HookEventsToml = toml::from_str( + r#" +[[config]] +source = "project" +source_path = "/repo/.codex/hooks.json" +key = "PreToolUse:0:0" +enabled = false +"#, + ) + .expect("hook config TOML should deserialize"); + + assert_eq!( + parsed.config, + vec![HookConfig { + source: HookConfigSource::Project, + plugin_id: None, + source_path: Some(PathBuf::from("/repo/.codex/hooks.json")), + key: "PreToolUse:0:0".to_string(), + enabled: false, + }] + ); +} + #[test] fn managed_hooks_requirements_flatten_hook_events() { let parsed: ManagedHooksRequirementsToml = toml::from_str( diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index ce3c1340c3..3490d3770a 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -873,6 +873,10 @@ }, "source": { "$ref": "#/definitions/HookConfigSource" + }, + "source_path": { + "default": null, + "type": "string" } }, "required": [ @@ -884,7 +888,9 @@ }, "HookConfigSource": { "enum": [ - "plugin" + "plugin", + "user", + "project" ], "type": "string" }, diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index bc5ed85585..ef1a5732ce 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -61,10 +61,9 @@ pub enum ConfigEdit { SetSkillConfig { path: PathBuf, enabled: bool }, /// Set or clear a skill config entry under `[[skills.config]]` by name. SetSkillConfigByName { name: String, enabled: bool }, - /// Set or clear a plugin hook config entry under `[[hooks.config]]`. + /// Set or clear a hook config entry under `[[hooks.config]]`. SetHookConfig { - plugin_id: String, - key: String, + selector: HookConfigEditSelector, enabled: bool, }, /// Set trust_level under `[projects.""]`, @@ -86,8 +85,10 @@ enum SkillConfigSelector { } #[derive(Clone, Debug, PartialEq, Eq)] -enum HookConfigSelector { +pub enum HookConfigEditSelector { Plugin { plugin_id: String, key: String }, + User { source_path: PathBuf, key: String }, + Project { source_path: PathBuf, key: String }, } /// Produces a config edit that sets `[tui].theme = ""`. @@ -530,17 +531,9 @@ impl ConfigDocument { ConfigEdit::SetSkillConfigByName { name, enabled } => { Ok(self.set_skill_config(SkillConfigSelector::Name(name.clone()), *enabled)) } - ConfigEdit::SetHookConfig { - plugin_id, - key, - enabled, - } => Ok(self.set_hook_config( - HookConfigSelector::Plugin { - plugin_id: plugin_id.clone(), - key: key.clone(), - }, - *enabled, - )), + ConfigEdit::SetHookConfig { selector, enabled } => { + Ok(self.set_hook_config(selector.clone(), *enabled)) + } ConfigEdit::SetPath { segments, value } => Ok(self.insert(segments, value.clone())), ConfigEdit::ClearPath { segments } => Ok(self.clear_owned(segments)), ConfigEdit::SetProjectTrustLevel { path, level } => { @@ -741,21 +734,52 @@ impl ConfigDocument { mutated } - fn set_hook_config(&mut self, selector: HookConfigSelector, enabled: bool) -> bool { + fn set_hook_config(&mut self, selector: HookConfigEditSelector, enabled: bool) -> bool { let selector = match selector { - HookConfigSelector::Plugin { plugin_id, key } => HookConfigSelector::Plugin { + HookConfigEditSelector::Plugin { plugin_id, key } => HookConfigEditSelector::Plugin { plugin_id: plugin_id.trim().to_string(), key: key.trim().to_string(), }, + HookConfigEditSelector::User { source_path, key } => HookConfigEditSelector::User { + source_path, + key: key.trim().to_string(), + }, + HookConfigEditSelector::Project { source_path, key } => { + HookConfigEditSelector::Project { + source_path, + key: key.trim().to_string(), + } + } }; - if matches!( - &selector, - HookConfigSelector::Plugin { plugin_id, key } - if plugin_id.is_empty() || key.is_empty() - ) { + if match &selector { + HookConfigEditSelector::Plugin { plugin_id, key } => { + plugin_id.is_empty() || key.is_empty() + } + HookConfigEditSelector::User { source_path, key } + | HookConfigEditSelector::Project { source_path, key } => { + source_path.as_os_str().is_empty() || key.is_empty() + } + } { return false; } + let selector = match selector { + HookConfigEditSelector::Plugin { plugin_id, key } => HookConfigEditSelector::Plugin { + plugin_id: plugin_id.trim().to_string(), + key: key.trim().to_string(), + }, + HookConfigEditSelector::User { source_path, key } => HookConfigEditSelector::User { + source_path: normalize_hook_config_source_path(&source_path), + key, + }, + HookConfigEditSelector::Project { source_path, key } => { + HookConfigEditSelector::Project { + source_path: normalize_hook_config_source_path(&source_path), + key, + } + } + }; + let mut remove_hooks_table = false; let mut mutated = false; @@ -1003,7 +1027,7 @@ fn write_skill_config_selector(table: &mut TomlTable, selector: &SkillConfigSele } } -fn hook_config_selector_from_table(table: &TomlTable) -> Option { +fn hook_config_selector_from_table(table: &TomlTable) -> Option { let source = table .get("source") .and_then(|item| item.as_str()) @@ -1018,26 +1042,62 @@ fn hook_config_selector_from_table(table: &TomlTable) -> Option Some(HookConfigSelector::Plugin { - plugin_id: plugin_id.to_string(), + match (source, plugin_id, source_path, key) { + (Some("plugin"), Some(plugin_id), None, Some(key)) => { + Some(HookConfigEditSelector::Plugin { + plugin_id: plugin_id.to_string(), + key: key.to_string(), + }) + } + (Some("user"), None, Some(source_path), Some(key)) => Some(HookConfigEditSelector::User { + source_path, key: key.to_string(), }), + (Some("project"), None, Some(source_path), Some(key)) => { + Some(HookConfigEditSelector::Project { + source_path, + key: key.to_string(), + }) + } _ => None, } } -fn write_hook_config_selector(table: &mut TomlTable, selector: &HookConfigSelector) { +fn write_hook_config_selector(table: &mut TomlTable, selector: &HookConfigEditSelector) { match selector { - HookConfigSelector::Plugin { plugin_id, key } => { + HookConfigEditSelector::Plugin { plugin_id, key } => { table["source"] = value("plugin"); table["plugin_id"] = value(plugin_id.clone()); + table.remove("source_path"); + table["key"] = value(key.clone()); + } + HookConfigEditSelector::User { source_path, key } => { + table["source"] = value("user"); + table.remove("plugin_id"); + table["source_path"] = value(source_path.to_string_lossy().to_string()); + table["key"] = value(key.clone()); + } + HookConfigEditSelector::Project { source_path, key } => { + table["source"] = value("project"); + table.remove("plugin_id"); + table["source_path"] = value(source_path.to_string_lossy().to_string()); table["key"] = value(key.clone()); } } } +fn normalize_hook_config_source_path(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_path_buf()) +} + /// Persist edits using a blocking strategy. pub fn apply_blocking( codex_home: &Path, diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index 827790e48e..eab5ffffae 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -6,6 +6,7 @@ use codex_protocol::openai_models::ReasoningEffort; use pretty_assertions::assert_eq; #[cfg(unix)] use std::os::unix::fs::symlink; +use std::path::PathBuf; use tempfile::tempdir; use toml::Value as TomlValue; @@ -140,8 +141,10 @@ fn set_hook_config_writes_disabled_plugin_entry() { ConfigEditsBuilder::new(codex_home) .with_edits([ConfigEdit::SetHookConfig { - plugin_id: "demo-plugin@test-marketplace".to_string(), - key: "hooks/hooks.json:PreToolUse:0:0".to_string(), + selector: HookConfigEditSelector::Plugin { + plugin_id: "demo-plugin@test-marketplace".to_string(), + key: "hooks/hooks.json:PreToolUse:0:0".to_string(), + }, enabled: false, }]) .apply_blocking() @@ -174,8 +177,10 @@ enabled = false ConfigEditsBuilder::new(codex_home) .with_edits([ConfigEdit::SetHookConfig { - plugin_id: "demo-plugin@test-marketplace".to_string(), - key: "hooks/hooks.json:PreToolUse:0:0".to_string(), + selector: HookConfigEditSelector::Plugin { + plugin_id: "demo-plugin@test-marketplace".to_string(), + key: "hooks/hooks.json:PreToolUse:0:0".to_string(), + }, enabled: true, }]) .apply_blocking() @@ -185,6 +190,32 @@ enabled = false assert_eq!(contents, ""); } +#[test] +fn set_hook_config_writes_disabled_project_entry() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + + ConfigEditsBuilder::new(codex_home) + .with_edits([ConfigEdit::SetHookConfig { + selector: HookConfigEditSelector::Project { + source_path: PathBuf::from("/repo/.codex/hooks.json"), + key: "PreToolUse:0:0".to_string(), + }, + enabled: false, + }]) + .apply_blocking() + .expect("persist"); + + let contents = std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"[[hooks.config]] +source = "project" +source_path = "/repo/.codex/hooks.json" +key = "PreToolUse:0:0" +enabled = false +"#; + assert_eq!(contents, expected); +} + #[test] fn blocking_set_model_preserves_inline_table_contents() { let tmp = tempdir().expect("tmpdir"); diff --git a/codex-rs/hooks/src/engine/config_rules.rs b/codex-rs/hooks/src/engine/config_rules.rs index d586a6ea49..64866061d2 100644 --- a/codex-rs/hooks/src/engine/config_rules.rs +++ b/codex-rs/hooks/src/engine/config_rules.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::path::Path; use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; @@ -7,10 +8,13 @@ use codex_config::HookConfig; use codex_config::HookConfigSource; use codex_config::HookEventsToml; use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookSource; +use codex_utils_absolute_path::AbsolutePathBuf; #[derive(Default)] pub(crate) struct HookConfigRules { plugin: HashMap<(String, String), bool>, + path: HashMap<(HookConfigSource, String, String), bool>, } impl HookConfigRules { @@ -53,10 +57,40 @@ impl HookConfigRules { self.plugin .get(&(plugin_id.to_string(), key.to_string())) .copied() - .unwrap_or({ - // TODO(abhinav): Default-enabled plugin hooks are temporary until hook trust is added. - true - }) + .unwrap_or_else(default_hook_enabled) + } + + pub(crate) fn enabled_for_hook( + &self, + source: HookSource, + plugin_id: Option<&str>, + source_path: &AbsolutePathBuf, + key: &str, + ) -> bool { + match source { + HookSource::Plugin => plugin_id + .map(|plugin_id| self.enabled_for_plugin_hook(plugin_id, key)) + .unwrap_or_else(default_hook_enabled), + HookSource::User | HookSource::Project => { + let Some(source) = hook_config_source_for_hook_source(source) else { + return default_hook_enabled(); + }; + self.path + .get(&( + source, + normalize_source_path(source_path.as_path()), + key.to_string(), + )) + .copied() + .unwrap_or_else(default_hook_enabled) + } + HookSource::System + | HookSource::Mdm + | HookSource::SessionFlags + | HookSource::LegacyManagedConfigFile + | HookSource::LegacyManagedConfigMdm + | HookSource::Unknown => default_hook_enabled(), + } } fn append(&mut self, entry: HookConfig, warnings: &mut Vec) { @@ -81,10 +115,72 @@ impl HookConfigRules { } self.plugin.insert((plugin_id, entry.key), entry.enabled); } + HookConfigSource::User | HookConfigSource::Project => { + let Some(source_path) = entry.source_path else { + warnings.push(format!( + "ignoring {} hooks.config entry without a source_path selector", + hook_config_source_label(entry.source) + )); + return; + }; + if source_path.as_os_str().is_empty() { + warnings.push(format!( + "ignoring {} hooks.config entry with empty source_path", + hook_config_source_label(entry.source) + )); + return; + } + if entry.key.trim().is_empty() { + warnings.push("ignoring hooks.config entry with empty key".to_string()); + return; + } + self.path.insert( + ( + entry.source, + normalize_source_path(&source_path), + entry.key.trim().to_string(), + ), + entry.enabled, + ); + } } } } +fn default_hook_enabled() -> bool { + // TODO(abhinav): Default-enabled hooks are temporary until hook trust is added. + true +} + +fn hook_config_source_for_hook_source(source: HookSource) -> Option { + match source { + HookSource::User => Some(HookConfigSource::User), + HookSource::Project => Some(HookConfigSource::Project), + HookSource::System + | HookSource::Mdm + | HookSource::SessionFlags + | HookSource::Plugin + | HookSource::LegacyManagedConfigFile + | HookSource::LegacyManagedConfigMdm + | HookSource::Unknown => None, + } +} + +fn hook_config_source_label(source: HookConfigSource) -> &'static str { + match source { + HookConfigSource::Plugin => "plugin", + HookConfigSource::User => "user", + HookConfigSource::Project => "project", + } +} + +fn normalize_source_path(path: &Path) -> String { + path.canonicalize() + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .to_string() +} + pub(crate) fn hook_config_key( source_relative_path: &str, event_name: HookEventName, @@ -100,6 +196,19 @@ pub(crate) fn hook_config_key( ) } +pub(crate) fn local_hook_config_key( + event_name: HookEventName, + group_index: usize, + handler_index: usize, +) -> String { + format!( + "{}:{}:{}", + hook_event_name_config_label(event_name), + group_index, + handler_index + ) +} + fn hook_event_name_config_label(event_name: HookEventName) -> &'static str { match event_name { HookEventName::PreToolUse => "PreToolUse", diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 89be9769df..11b42219de 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -20,6 +20,7 @@ use std::collections::HashMap; use super::ConfiguredHandler; use super::config_rules::HookConfigRules; use super::config_rules::hook_config_key; +use super::config_rules::local_hook_config_key; use crate::events::common::matcher_pattern_for_event; use crate::events::common::validate_matcher_pattern; use codex_protocol::protocol::HookSource; @@ -217,7 +218,7 @@ fn append_plugin_hook_sources( } } -fn managed_hooks_source_path( +pub(crate) fn managed_hooks_source_path( managed_hooks: &ManagedHooksRequirementsToml, requirement_source: Option<&RequirementSource>, warnings: &mut Vec, @@ -262,7 +263,7 @@ fn managed_hooks_source_path( } } -fn load_hooks_json( +pub(crate) fn load_hooks_json( config_folder: Option<&Path>, warnings: &mut Vec, ) -> Option<(AbsolutePathBuf, HookEventsToml)> { @@ -305,7 +306,7 @@ fn load_hooks_json( (!parsed.hooks.is_empty()).then_some((source_path, parsed.hooks)) } -fn load_toml_hooks_from_layer( +pub(crate) fn load_toml_hooks_from_layer( layer: &ConfigLayerEntry, warnings: &mut Vec, ) -> Option<(AbsolutePathBuf, HookEventsToml)> { @@ -420,19 +421,27 @@ fn append_group_handlers( } for (handler_index, handler) in group_handlers.into_iter().enumerate() { - if let Some(plugin) = &source.plugin { - let key = hook_config_key( - &plugin.source_relative_path, - event_name, - group_index, - handler_index, - ); - if !context - .hook_config_rules - .enabled_for_plugin_hook(&plugin.plugin_id, &key) - { - continue; - } + let (key, plugin_id) = if let Some(plugin) = &source.plugin { + ( + hook_config_key( + &plugin.source_relative_path, + event_name, + group_index, + handler_index, + ), + Some(plugin.plugin_id.as_str()), + ) + } else { + ( + local_hook_config_key(event_name, group_index, handler_index), + None, + ) + }; + if !context + .hook_config_rules + .enabled_for_hook(source.source, plugin_id, source.path, &key) + { + continue; } match handler { @@ -483,7 +492,7 @@ fn append_group_handlers( } } -fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource { +pub(crate) fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource { match source { ConfigLayerSource::System { .. } => HookSource::System, ConfigLayerSource::User { .. } => HookSource::User, @@ -497,7 +506,7 @@ fn hook_source_for_config_layer_source(source: &ConfigLayerSource) -> HookSource } } -fn hook_source_for_requirement_source(source: Option<&RequirementSource>) -> HookSource { +pub(crate) fn hook_source_for_requirement_source(source: Option<&RequirementSource>) -> HookSource { match source { Some(RequirementSource::MdmManagedPreferences { .. }) => HookSource::Mdm, Some(RequirementSource::SystemRequirementsToml { .. }) => HookSource::System, diff --git a/codex-rs/hooks/src/engine/inventory.rs b/codex-rs/hooks/src/engine/inventory.rs index 5e508ebef2..1c4568c80f 100644 --- a/codex-rs/hooks/src/engine/inventory.rs +++ b/codex-rs/hooks/src/engine/inventory.rs @@ -1,5 +1,8 @@ use codex_config::ConfigLayerStack; +use codex_config::ConfigLayerStackOrdering; +use codex_config::HookEventsToml; use codex_config::HookHandlerConfig; +use codex_config::MatcherGroup; use codex_plugin::PluginHookSource; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookHandlerType; @@ -8,6 +11,12 @@ use codex_utils_absolute_path::AbsolutePathBuf; use super::config_rules::HookConfigRules; use super::config_rules::hook_config_key; +use super::config_rules::local_hook_config_key; +use super::discovery::hook_source_for_config_layer_source; +use super::discovery::hook_source_for_requirement_source; +use super::discovery::load_hooks_json; +use super::discovery::load_toml_hooks_from_layer; +use super::discovery::managed_hooks_source_path; #[derive(Debug, Clone, PartialEq, Eq)] pub struct HookInventoryEntry { @@ -25,6 +34,101 @@ pub struct HookInventoryEntry { pub enabled: bool, } +#[derive(Clone)] +struct InventoryHookSource { + source: HookSource, + plugin_id: Option, + source_path: AbsolutePathBuf, + source_relative_path: Option, +} + +pub fn list_hooks( + config_layer_stack: Option<&ConfigLayerStack>, + plugin_hook_sources: &[PluginHookSource], +) -> Vec { + let mut warnings = Vec::new(); + let hook_config_rules = config_layer_stack + .map(|config_layer_stack| HookConfigRules::from_stack(config_layer_stack, &mut warnings)) + .unwrap_or_default(); + let mut entries = Vec::new(); + + if let Some(config_layer_stack) = config_layer_stack { + if let Some(managed_hooks) = config_layer_stack.requirements().managed_hooks.as_ref() + && let Some(source_path) = managed_hooks_source_path( + managed_hooks.get(), + managed_hooks.source.as_ref(), + &mut warnings, + ) + { + append_hook_events( + &mut entries, + InventoryHookSource { + source: hook_source_for_requirement_source(managed_hooks.source.as_ref()), + plugin_id: None, + source_path, + source_relative_path: None, + }, + managed_hooks.get().hooks.clone(), + &hook_config_rules, + ); + } + + for layer in config_layer_stack.get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ false, + ) { + let hook_source = hook_source_for_config_layer_source(&layer.name); + if let Some((source_path, hook_events)) = + load_hooks_json(layer.config_folder().as_deref(), &mut warnings) + { + append_hook_events( + &mut entries, + InventoryHookSource { + source: hook_source, + plugin_id: None, + source_path, + source_relative_path: None, + }, + hook_events, + &hook_config_rules, + ); + } + if let Some((source_path, hook_events)) = + load_toml_hooks_from_layer(layer, &mut warnings) + { + append_hook_events( + &mut entries, + InventoryHookSource { + source: hook_source, + plugin_id: None, + source_path, + source_relative_path: None, + }, + hook_events, + &hook_config_rules, + ); + } + } + } + + for source in plugin_hook_sources { + let plugin_id = source.plugin_id.as_key(); + append_hook_events( + &mut entries, + InventoryHookSource { + source: HookSource::Plugin, + plugin_id: Some(plugin_id), + source_path: source.source_path.clone(), + source_relative_path: Some(source.source_relative_path.clone()), + }, + source.hooks.clone(), + &hook_config_rules, + ); + } + + entries +} + pub fn list_plugin_hooks( config_layer_stack: Option<&ConfigLayerStack>, plugin_hook_sources: &[PluginHookSource], @@ -36,41 +140,80 @@ pub fn list_plugin_hooks( let mut entries = Vec::new(); for source in plugin_hook_sources { - let plugin_id = source.plugin_id.as_key(); - for (event_name, groups) in source.hooks.clone().into_matcher_groups() { - for (group_index, group) in groups.into_iter().enumerate() { - for (handler_index, handler) in group.hooks.into_iter().enumerate() { - let key = hook_config_key( - &source.source_relative_path, - event_name, - group_index, - handler_index, - ); - let enabled = hook_config_rules.enabled_for_plugin_hook(&plugin_id, &key); - let (handler_type, command, timeout_sec, status_message) = - hook_inventory_handler_fields(handler); - entries.push(HookInventoryEntry { - source: HookSource::Plugin, - plugin_id: Some(plugin_id.clone()), - key, - event_name, - matcher: group.matcher.clone(), - handler_type, - command, - timeout_sec, - status_message, - source_path: source.source_path.clone(), - source_relative_path: Some(source.source_relative_path.clone()), - enabled, - }); - } - } - } + append_hook_events( + &mut entries, + InventoryHookSource { + source: HookSource::Plugin, + plugin_id: Some(source.plugin_id.as_key()), + source_path: source.source_path.clone(), + source_relative_path: Some(source.source_relative_path.clone()), + }, + source.hooks.clone(), + &hook_config_rules, + ); } entries } +fn append_hook_events( + entries: &mut Vec, + source: InventoryHookSource, + hook_events: HookEventsToml, + hook_config_rules: &HookConfigRules, +) { + for (event_name, groups) in hook_events.into_matcher_groups() { + append_matcher_groups( + entries, + source.clone(), + event_name, + groups, + hook_config_rules, + ); + } +} + +fn append_matcher_groups( + entries: &mut Vec, + source: InventoryHookSource, + event_name: HookEventName, + groups: Vec, + hook_config_rules: &HookConfigRules, +) { + for (group_index, group) in groups.into_iter().enumerate() { + for (handler_index, handler) in group.hooks.into_iter().enumerate() { + let key = match (source.source, source.source_relative_path.as_deref()) { + (HookSource::Plugin, Some(source_relative_path)) => { + hook_config_key(source_relative_path, event_name, group_index, handler_index) + } + _ => local_hook_config_key(event_name, group_index, handler_index), + }; + let enabled = hook_config_rules.enabled_for_hook( + source.source, + source.plugin_id.as_deref(), + &source.source_path, + &key, + ); + let (handler_type, command, timeout_sec, status_message) = + hook_inventory_handler_fields(handler); + entries.push(HookInventoryEntry { + source: source.source, + plugin_id: source.plugin_id.clone(), + key, + event_name, + matcher: group.matcher.clone(), + handler_type, + command, + timeout_sec, + status_message, + source_path: source.source_path.clone(), + source_relative_path: source.source_relative_path.clone(), + enabled, + }); + } + } +} + fn hook_inventory_handler_fields( handler: HookHandlerConfig, ) -> (HookHandlerType, Option, Option, Option) { diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index c3c6f7d979..ec8a3d4e89 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -18,6 +18,7 @@ use codex_config::TomlValue; use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_protocol::ThreadId; +use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::HookSource; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -30,6 +31,70 @@ fn cwd() -> AbsolutePathBuf { AbsolutePathBuf::current_dir().expect("current dir") } +fn config_with_command_hook(event_name: &str, matcher: &str, command: &str) -> TomlValue { + let mut root = toml_table(); + let TomlValue::Table(root_table) = &mut root else { + unreachable!("root should be a table"); + }; + let mut hooks = toml_table(); + let TomlValue::Table(hooks_table) = &mut hooks else { + unreachable!("hooks should be a table"); + }; + let mut group = toml_table(); + let TomlValue::Table(group_table) = &mut group else { + unreachable!("group should be a table"); + }; + group_table.insert( + "matcher".to_string(), + TomlValue::String(matcher.to_string()), + ); + let mut handler = toml_table(); + let TomlValue::Table(handler_table) = &mut handler else { + unreachable!("handler should be a table"); + }; + handler_table.insert("type".to_string(), TomlValue::String("command".to_string())); + handler_table.insert( + "command".to_string(), + TomlValue::String(command.to_string()), + ); + group_table.insert("hooks".to_string(), TomlValue::Array(vec![handler])); + hooks_table.insert(event_name.to_string(), TomlValue::Array(vec![group])); + root_table.insert("hooks".to_string(), hooks); + root +} + +fn config_with_project_hook_override(source_path: &AbsolutePathBuf, key: &str) -> TomlValue { + let mut root = toml_table(); + let TomlValue::Table(root_table) = &mut root else { + unreachable!("root should be a table"); + }; + let mut hooks = toml_table(); + let TomlValue::Table(hooks_table) = &mut hooks else { + unreachable!("hooks should be a table"); + }; + let mut config_entry = toml_table(); + let TomlValue::Table(config_entry_table) = &mut config_entry else { + unreachable!("hooks config entry should be a table"); + }; + config_entry_table.insert( + "source".to_string(), + TomlValue::String("project".to_string()), + ); + config_entry_table.insert( + "source_path".to_string(), + TomlValue::String(source_path.display().to_string()), + ); + config_entry_table.insert("key".to_string(), TomlValue::String(key.to_string())); + config_entry_table.insert("enabled".to_string(), TomlValue::Boolean(false)); + hooks_table.insert("config".to_string(), TomlValue::Array(vec![config_entry])); + root_table.insert("hooks".to_string(), hooks); + root +} + +fn toml_table() -> TomlValue { + TomlValue::Table(Default::default()) +} + fn managed_hooks_for_current_platform( managed_dir: impl AsRef, hooks: HookEventsToml, @@ -520,3 +585,232 @@ fn plugin_hook_sources_can_be_disabled_by_user_config() { assert_eq!(preview, Vec::new()); } + +#[test] +fn hook_inventory_lists_config_and_plugin_hooks() { + let temp = tempdir().expect("create temp dir"); + let user_config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("user config path"); + let project_dot_codex = + AbsolutePathBuf::try_from(temp.path().join("repo/.codex")).expect("project config dir"); + fs::create_dir_all(project_dot_codex.as_path()).expect("create project .codex"); + let project_hooks_path = project_dot_codex.join("hooks.json"); + fs::write( + project_hooks_path.as_path(), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo project" } + ] + } + ] + } +}"#, + ) + .expect("write project hooks"); + + let user_config = config_with_command_hook("SessionStart", "startup", "echo user"); + let session_config = config_with_command_hook("Stop", "stop", "echo session"); + let managed_dir = + AbsolutePathBuf::try_from(temp.path().join("managed-hooks")).expect("managed hooks dir"); + fs::create_dir_all(managed_dir.as_path()).expect("create managed hooks dir"); + let managed_hooks = managed_hooks_for_current_platform( + managed_dir.clone(), + HookEventsToml { + post_tool_use: vec![MatcherGroup { + matcher: Some("Bash".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "echo managed".to_string(), + timeout_sec: None, + r#async: false, + status_message: None, + }], + }], + ..Default::default() + }, + ); + let plugin_root = + AbsolutePathBuf::try_from(temp.path().join("demo-plugin")).expect("plugin root"); + let plugin_hook_sources = vec![PluginHookSource { + plugin_id: PluginId::parse("demo-plugin@test-marketplace").expect("plugin id"), + plugin_root: plugin_root.clone(), + source_path: plugin_root.join("hooks/hooks.json"), + source_relative_path: "hooks/hooks.json".to_string(), + hooks: HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("Read".to_string()), + hooks: vec![HookHandlerConfig::Command { + command: "echo plugin".to_string(), + timeout_sec: None, + r#async: false, + status_message: None, + }], + }], + ..Default::default() + }, + }]; + let config_layer_stack = ConfigLayerStack::new( + vec![ + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: user_config_path.clone(), + }, + user_config, + ), + ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: project_dot_codex, + }, + TomlValue::Table(Default::default()), + ), + ConfigLayerEntry::new(ConfigLayerSource::SessionFlags, session_config), + ], + ConfigRequirements { + managed_hooks: Some(ConstrainedWithSource::new( + Constrained::allow_any(managed_hooks), + Some(RequirementSource::SystemRequirementsToml { + file: managed_dir.join("requirements.toml"), + }), + )), + ..ConfigRequirements::default() + }, + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let entries = super::inventory::list_hooks(Some(&config_layer_stack), &plugin_hook_sources); + let session_source_path = if cfg!(windows) { + AbsolutePathBuf::resolve_path_against_base("/config.toml", r"C:\") + } else { + AbsolutePathBuf::resolve_path_against_base("/config.toml", "/") + }; + let summary: Vec<_> = entries + .into_iter() + .map(|entry| { + ( + entry.source, + entry.event_name, + entry.key, + entry.source_path, + entry.enabled, + ) + }) + .collect(); + + assert_eq!( + summary, + vec![ + ( + HookSource::System, + HookEventName::PostToolUse, + "PostToolUse:0:0".to_string(), + managed_dir, + true, + ), + ( + HookSource::User, + HookEventName::SessionStart, + "SessionStart:0:0".to_string(), + user_config_path, + true, + ), + ( + HookSource::Project, + HookEventName::PreToolUse, + "PreToolUse:0:0".to_string(), + project_hooks_path, + true, + ), + ( + HookSource::SessionFlags, + HookEventName::Stop, + "Stop:0:0".to_string(), + session_source_path, + true, + ), + ( + HookSource::Plugin, + HookEventName::PreToolUse, + "hooks/hooks.json:PreToolUse:0:0".to_string(), + plugin_root.join("hooks/hooks.json"), + true, + ), + ] + ); +} + +#[test] +fn project_hook_sources_can_be_disabled_by_user_config() { + let temp = tempdir().expect("create temp dir"); + let user_config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("user config path"); + let project_dot_codex = + AbsolutePathBuf::try_from(temp.path().join("repo/.codex")).expect("project config dir"); + fs::create_dir_all(project_dot_codex.as_path()).expect("create project .codex"); + let project_hooks_path = project_dot_codex.join("hooks.json"); + fs::write( + project_hooks_path.as_path(), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { "type": "command", "command": "echo project" } + ] + } + ] + } +}"#, + ) + .expect("write project hooks"); + + let user_config = config_with_project_hook_override(&project_hooks_path, "PreToolUse:0:0"); + let config_layer_stack = ConfigLayerStack::new( + vec![ + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: user_config_path, + }, + user_config, + ), + ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: project_dot_codex, + }, + TomlValue::Table(Default::default()), + ), + ], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + Some(&config_layer_stack), + Vec::new(), + CommandShell { + program: String::new(), + args: Vec::new(), + }, + ); + + let preview = engine.preview_pre_tool_use(&PreToolUseRequest { + session_id: ThreadId::new(), + turn_id: "turn-1".to_string(), + 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, Vec::new()); +} diff --git a/codex-rs/hooks/src/lib.rs b/codex-rs/hooks/src/lib.rs index 8d9702d20b..a554080f09 100644 --- a/codex-rs/hooks/src/lib.rs +++ b/codex-rs/hooks/src/lib.rs @@ -6,6 +6,7 @@ mod schema; mod types; pub use engine::inventory::HookInventoryEntry; +pub use engine::inventory::list_hooks; pub use engine::inventory::list_plugin_hooks; pub use events::permission_request::PermissionRequestDecision; pub use events::permission_request::PermissionRequestOutcome;