diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index ea6ab7f9632c..6152d7b2ecc1 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2099,6 +2099,7 @@ "HookHandlerType": { "enum": [ "command", + "mcpTool", "prompt", "agent" ], 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 b29284db0bde..96c2d04b7f59 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 @@ -11871,12 +11871,89 @@ "HookHandlerType": { "enum": [ "command", + "mcpTool", "prompt", "agent" ], "type": "string" }, "HookMetadata": { + "oneOf": [ + { + "properties": { + "async": { + "default": false, + "type": "boolean" + }, + "command": { + "type": "string" + }, + "handlerType": { + "enum": [ + "command" + ], + "type": "string" + } + }, + "required": [ + "command", + "handlerType" + ], + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "mcpTool" + ], + "type": "string" + }, + "server": { + "type": "string" + }, + "tool": { + "type": "string" + } + }, + "required": [ + "handlerType", + "server", + "tool" + ], + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "prompt" + ], + "type": "string" + } + }, + "required": [ + "handlerType" + ], + "title": "PromptHookMetadata", + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "agent" + ], + "type": "string" + } + }, + "required": [ + "handlerType" + ], + "title": "AgentHookMetadata", + "type": "object" + } + ], "properties": { "additionalContextLimit": { "description": "Configured `additionalContext` spill threshold. `null` uses 2,500 tokens; `0` disables spilling.", @@ -11887,12 +11964,6 @@ "null" ] }, - "command": { - "type": [ - "string", - "null" - ] - }, "currentHash": { "type": "string" }, @@ -11906,17 +11977,6 @@ "eventName": { "$ref": "#/definitions/v2/HookEventName" }, - "executionMode": { - "allOf": [ - { - "$ref": "#/definitions/v2/HookExecutionMode" - } - ], - "default": "sync" - }, - "handlerType": { - "$ref": "#/definitions/v2/HookHandlerType" - }, "isManaged": { "type": "boolean" }, @@ -11961,7 +12021,6 @@ "displayOrder", "enabled", "eventName", - "handlerType", "isManaged", "key", "source", 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 7e5d4ca22de0..3ec9520207e2 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 @@ -8100,12 +8100,89 @@ "HookHandlerType": { "enum": [ "command", + "mcpTool", "prompt", "agent" ], "type": "string" }, "HookMetadata": { + "oneOf": [ + { + "properties": { + "async": { + "default": false, + "type": "boolean" + }, + "command": { + "type": "string" + }, + "handlerType": { + "enum": [ + "command" + ], + "type": "string" + } + }, + "required": [ + "command", + "handlerType" + ], + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "mcpTool" + ], + "type": "string" + }, + "server": { + "type": "string" + }, + "tool": { + "type": "string" + } + }, + "required": [ + "handlerType", + "server", + "tool" + ], + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "prompt" + ], + "type": "string" + } + }, + "required": [ + "handlerType" + ], + "title": "PromptHookMetadata", + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "agent" + ], + "type": "string" + } + }, + "required": [ + "handlerType" + ], + "title": "AgentHookMetadata", + "type": "object" + } + ], "properties": { "additionalContextLimit": { "description": "Configured `additionalContext` spill threshold. `null` uses 2,500 tokens; `0` disables spilling.", @@ -8116,12 +8193,6 @@ "null" ] }, - "command": { - "type": [ - "string", - "null" - ] - }, "currentHash": { "type": "string" }, @@ -8135,17 +8206,6 @@ "eventName": { "$ref": "#/definitions/HookEventName" }, - "executionMode": { - "allOf": [ - { - "$ref": "#/definitions/HookExecutionMode" - } - ], - "default": "sync" - }, - "handlerType": { - "$ref": "#/definitions/HookHandlerType" - }, "isManaged": { "type": "boolean" }, @@ -8190,7 +8250,6 @@ "displayOrder", "enabled", "eventName", - "handlerType", "isManaged", "key", "source", diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json index 11d6f2845884..638e4886a79d 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookCompletedNotification.json @@ -31,6 +31,7 @@ "HookHandlerType": { "enum": [ "command", + "mcpTool", "prompt", "agent" ], diff --git a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json index 8d6d82aa0476..a6d1955b60e0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HookStartedNotification.json @@ -31,6 +31,7 @@ "HookHandlerType": { "enum": [ "command", + "mcpTool", "prompt", "agent" ], diff --git a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json index 2ebaa4bc09ff..9ccf201a19b4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/HooksListResponse.json @@ -36,22 +36,83 @@ ], "type": "string" }, - "HookExecutionMode": { - "enum": [ - "sync", - "async" - ], - "type": "string" - }, - "HookHandlerType": { - "enum": [ - "command", - "prompt", - "agent" - ], - "type": "string" - }, "HookMetadata": { + "oneOf": [ + { + "properties": { + "async": { + "default": false, + "type": "boolean" + }, + "command": { + "type": "string" + }, + "handlerType": { + "enum": [ + "command" + ], + "type": "string" + } + }, + "required": [ + "command", + "handlerType" + ], + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "mcpTool" + ], + "type": "string" + }, + "server": { + "type": "string" + }, + "tool": { + "type": "string" + } + }, + "required": [ + "handlerType", + "server", + "tool" + ], + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "prompt" + ], + "type": "string" + } + }, + "required": [ + "handlerType" + ], + "title": "PromptHookMetadata", + "type": "object" + }, + { + "properties": { + "handlerType": { + "enum": [ + "agent" + ], + "type": "string" + } + }, + "required": [ + "handlerType" + ], + "title": "AgentHookMetadata", + "type": "object" + } + ], "properties": { "additionalContextLimit": { "description": "Configured `additionalContext` spill threshold. `null` uses 2,500 tokens; `0` disables spilling.", @@ -62,12 +123,6 @@ "null" ] }, - "command": { - "type": [ - "string", - "null" - ] - }, "currentHash": { "type": "string" }, @@ -81,17 +136,6 @@ "eventName": { "$ref": "#/definitions/HookEventName" }, - "executionMode": { - "allOf": [ - { - "$ref": "#/definitions/HookExecutionMode" - } - ], - "default": "sync" - }, - "handlerType": { - "$ref": "#/definitions/HookHandlerType" - }, "isManaged": { "type": "boolean" }, @@ -136,7 +180,6 @@ "displayOrder", "enabled", "eventName", - "handlerType", "isManaged", "key", "source", diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst index 14d34815a23b..7a6dc5793c4b 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-experimental.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst index d9f5acc201af..54e169c33ddd 100644 Binary files a/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst and b/codex-rs/app-server-protocol/schema/precomputed/app-server-exports-stable.json.zst differ diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookHandlerType.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookHandlerType.ts index dc3f087bff96..7dd671e94289 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookHandlerType.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookHandlerType.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 HookHandlerType = "command" | "prompt" | "agent"; +export type HookHandlerType = "command" | "mcpTool" | "prompt" | "agent"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts b/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts index a831f81be119..04358a9142c8 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/HookMetadata.ts @@ -3,14 +3,12 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; import type { HookEventName } from "./HookEventName"; -import type { HookExecutionMode } from "./HookExecutionMode"; -import type { HookHandlerType } from "./HookHandlerType"; import type { HookSource } from "./HookSource"; import type { HookTrustStatus } from "./HookTrustStatus"; -export type HookMetadata = { key: string, eventName: HookEventName, handlerType: HookHandlerType, executionMode: HookExecutionMode, matcher: string | null, command: string | null, timeoutSec: bigint, statusMessage: string | null, +export type HookMetadata = { key: string, eventName: HookEventName, matcher: string | null, timeoutSec: bigint, statusMessage: string | null, /** * Configured `additionalContext` spill threshold. * `null` uses 2,500 tokens; `0` disables spilling. */ -additionalContextLimit: number | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustStatus: HookTrustStatus, }; +additionalContextLimit: number | null, sourcePath: AbsolutePathBuf, source: HookSource, pluginId: string | null, displayOrder: bigint, enabled: boolean, isManaged: boolean, currentHash: string, trustStatus: HookTrustStatus, } & ({ "handlerType": "command", command: string, async: boolean, } | { "handlerType": "mcpTool", server: string, tool: string, } | { "handlerType": "prompt", } | { "handlerType": "agent", }); diff --git a/codex-rs/app-server-protocol/src/protocol/v2/hook.rs b/codex-rs/app-server-protocol/src/protocol/v2/hook.rs index 5d08afee3d90..41e34a3f8f11 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/hook.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/hook.rs @@ -23,7 +23,7 @@ v2_enum_from_core!( v2_enum_from_core!( pub enum HookHandlerType from CoreHookHandlerType { - Command, Prompt, Agent + Command, McpTool, Prompt, Agent } ); diff --git a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs index 8c2e11df2ad8..de2bdc3a4e39 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/plugin.rs @@ -1,7 +1,5 @@ use super::AppSummary; use super::HookEventName; -use super::HookExecutionMode; -use super::HookHandlerType; use super::HookSource; use super::HookTrustStatus; use crate::JsonSchema; @@ -518,17 +516,32 @@ pub struct HooksListEntry { pub errors: Vec, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(tag = "handlerType", rename_all = "camelCase")] +#[ts(tag = "handlerType", export_to = "v2/")] +pub enum HookHandlerMetadata { + Command { + command: String, + #[serde(default)] + r#async: bool, + }, + McpTool { + server: String, + tool: String, + }, + Prompt {}, + Agent {}, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(export_to = "v2/")] pub struct HookMetadata { pub key: String, pub event_name: HookEventName, - pub handler_type: HookHandlerType, - #[serde(default)] - pub execution_mode: HookExecutionMode, + #[serde(flatten)] + pub handler: HookHandlerMetadata, pub matcher: Option, - pub command: Option, pub timeout_sec: u64, pub status_message: Option, /// Configured `additionalContext` spill threshold. diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 6743ed25bd14..2b52e0cab6a5 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3396,6 +3396,45 @@ fn user_input_into_core_preserves_media_fields() { ); } +#[test] +fn hook_handler_metadata_only_exposes_async_for_commands() { + assert_eq!( + serde_json::to_value(HookHandlerMetadata::Command { + command: "echo hello".to_string(), + r#async: true, + }) + .unwrap(), + json!({ + "handlerType": "command", + "command": "echo hello", + "async": true, + }), + ); + assert_eq!( + serde_json::from_value::(json!({ + "handlerType": "command", + "command": "echo hello", + })) + .unwrap(), + HookHandlerMetadata::Command { + command: "echo hello".to_string(), + r#async: false, + }, + ); + assert_eq!( + serde_json::to_value(HookHandlerMetadata::McpTool { + server: "security".to_string(), + tool: "scan".to_string(), + }) + .unwrap(), + json!({ + "handlerType": "mcpTool", + "server": "security", + "tool": "scan", + }), + ); +} + #[test] fn skills_list_params_serialization_uses_force_reload() { assert_eq!( diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index d9a534a22ffb..a9cbbe3d3277 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -1961,10 +1961,12 @@ For linked Git worktrees, project hook declarations come from the matching `.cod Hooks are returned even when disabled so clients can render and re-enable them. User-controlled state lives under `hooks.state`. Managed hooks are non-configurable, and user entries for managed hook keys are ignored during loading. -`executionMode` reports how a command hook runs. `sync` hooks participate in the current operation, while `async` hooks run in the background and deliver informational output through the existing steer-based injection path. Output is injected immediately into an active turn or persisted without starting a new turn when the session is idle. +A command hook's `async` field reports its effective execution behavior. Hooks with `async: false` participate in the current operation, while hooks with `async: true` run in the background and deliver informational output through the existing steer-based injection path. Output is injected immediately into an active turn or persisted without starting a new turn when the session is idle. MCP tool hooks do not have an `async` field and always run synchronously. Lifecycle notifications continue to report `executionMode` on hook run summaries. For unmanaged hooks, `currentHash` and `trustStatus` describe whether the current definition is first-seen, approved, or changed since approval. Only trusted unmanaged hooks become runnable. Hook keys combine the source identity with a trailing event/group/handler selector that is currently positional. +MCP tool hooks appear with `handlerType: "mcpTool"`. Their `server` and `tool` fields identify the configured MCP target. Command hooks instead include a `command` field. + ```json { "method": "hooks/list", @@ -1985,7 +1987,7 @@ For unmanaged hooks, `currentHash` and `trustStatus` describe whether the curren "key": "/Users/me/.codex/config.toml:pre_tool_use:0:0", "eventName": "pre_tool_use", "handlerType": "command", - "executionMode": "sync", + "async": false, "isManaged": false, "matcher": "Bash", "command": "python3 /Users/me/hook.py", diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 47801f7a7293..0929e2629eff 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -90,6 +90,7 @@ use codex_app_server_protocol::GetWorkspaceMessagesResponse; use codex_app_server_protocol::GitDiffToRemoteParams; use codex_app_server_protocol::GitDiffToRemoteResponse; use codex_app_server_protocol::GitInfo as ApiGitInfo; +use codex_app_server_protocol::HookHandlerMetadata; use codex_app_server_protocol::HookMetadata; use codex_app_server_protocol::HooksListParams; use codex_app_server_protocol::HooksListResponse; diff --git a/codex-rs/app-server/src/request_processors/catalog_processor.rs b/codex-rs/app-server/src/request_processors/catalog_processor.rs index d56df03f5e48..1598ec48e98d 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -1,5 +1,6 @@ use super::*; use codex_core::config::permission_profile_catalog; +use codex_hooks::HookListEntryHandler; use futures::StreamExt; #[derive(Clone)] @@ -66,24 +67,36 @@ fn skills_to_info( fn hooks_to_info(hooks: &[codex_hooks::HookListEntry]) -> Vec { hooks .iter() - .map(|hook| HookMetadata { - key: hook.key.clone(), - event_name: hook.event_name.into(), - handler_type: hook.handler_type.into(), - execution_mode: hook.execution_mode.into(), - matcher: hook.matcher.clone(), - command: hook.command.clone(), - timeout_sec: hook.timeout_sec, - status_message: hook.status_message.clone(), - additional_context_limit: hook.additional_context_limit, - source_path: hook.source_path.clone(), - source: hook.source.into(), - plugin_id: hook.plugin_id.clone(), - display_order: hook.display_order, - enabled: hook.enabled, - is_managed: hook.is_managed, - current_hash: hook.current_hash.clone(), - trust_status: hook.trust_status.into(), + .map(|hook| { + let handler = match &hook.handler { + HookListEntryHandler::Command { command, r#async } => { + HookHandlerMetadata::Command { + command: command.clone(), + r#async: *r#async, + } + } + HookListEntryHandler::McpTool { server, tool } => HookHandlerMetadata::McpTool { + server: server.clone(), + tool: tool.clone(), + }, + }; + HookMetadata { + key: hook.key.clone(), + event_name: hook.event_name.into(), + handler, + matcher: hook.matcher.clone(), + timeout_sec: hook.timeout_sec, + status_message: hook.status_message.clone(), + additional_context_limit: hook.additional_context_limit, + source_path: hook.source_path.clone(), + source: hook.source.into(), + plugin_id: hook.plugin_id.clone(), + display_order: hook.display_order, + enabled: hook.enabled, + is_managed: hook.is_managed, + current_hash: hook.current_hash.clone(), + trust_status: hook.trust_status.into(), + } }) .collect() } diff --git a/codex-rs/app-server/tests/suite/v2/hooks_list.rs b/codex-rs/app-server/tests/suite/v2/hooks_list.rs index aac0d44ce395..634b79716db7 100644 --- a/codex-rs/app-server/tests/suite/v2/hooks_list.rs +++ b/codex-rs/app-server/tests/suite/v2/hooks_list.rs @@ -8,8 +8,7 @@ use app_test_support::create_mock_responses_server_sequence_unchecked; use codex_app_server_protocol::ConfigBatchWriteParams; use codex_app_server_protocol::ConfigEdit; use codex_app_server_protocol::HookEventName; -use codex_app_server_protocol::HookExecutionMode; -use codex_app_server_protocol::HookHandlerType; +use codex_app_server_protocol::HookHandlerMetadata; use codex_app_server_protocol::HookMetadata; use codex_app_server_protocol::HookSource; use codex_app_server_protocol::HookTrustStatus; @@ -51,7 +50,7 @@ fn command_hook_hash( matcher: Option<&str>, command: &str, timeout_sec: u64, - execution_mode: HookExecutionMode, + r#async: bool, status_message: Option<&str>, additional_context_limit: Option, ) -> String { @@ -63,7 +62,7 @@ fn command_hook_hash( command: command.to_string(), command_windows: None, timeout_sec: Some(timeout_sec), - r#async: execution_mode == HookExecutionMode::Async, + r#async, status_message: status_message.map(ToOwned::to_owned), additional_context_limit, }], @@ -213,10 +212,11 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> { hooks: vec![HookMetadata { key: format!("{}:pre_tool_use:0:0", config_path.as_path().display()), event_name: HookEventName::PreToolUse, - handler_type: HookHandlerType::Command, - execution_mode: HookExecutionMode::Async, + handler: HookHandlerMetadata::Command { + command: "python3 /tmp/listed-hook.py".to_string(), + r#async: true, + }, matcher: Some("Bash".to_string()), - command: Some("python3 /tmp/listed-hook.py".to_string()), timeout_sec: 5, status_message: Some("running listed hook".to_string()), additional_context_limit: Some(4_096), @@ -231,7 +231,7 @@ async fn hooks_list_shows_discovered_hook() -> Result<()> { Some("Bash"), "python3 /tmp/listed-hook.py", /*timeout_sec*/ 5, - HookExecutionMode::Async, + /*async*/ true, Some("running listed hook"), /*additional_context_limit*/ Some(4_096), ), @@ -294,10 +294,11 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> { hooks: vec![HookMetadata { key: "demo@test:hooks/hooks.json:pre_tool_use:0:0".to_string(), event_name: HookEventName::PreToolUse, - handler_type: HookHandlerType::Command, - execution_mode: HookExecutionMode::Sync, + handler: HookHandlerMetadata::Command { + command: "echo plugin hook".to_string(), + r#async: false, + }, matcher: Some("Bash".to_string()), - command: Some("echo plugin hook".to_string()), timeout_sec: 7, status_message: Some("running plugin hook".to_string()), additional_context_limit: None, @@ -312,7 +313,7 @@ async fn hooks_list_shows_discovered_plugin_hook() -> Result<()> { Some("Bash"), "echo plugin hook", /*timeout_sec*/ 7, - HookExecutionMode::Sync, + /*async*/ false, Some("running plugin hook"), /*additional_context_limit*/ None, ), @@ -512,8 +513,11 @@ async fn plugin_upgrade_refreshes_hook_runtime_for_loaded_session() -> Result<() .find(|hook| hook.event_name == HookEventName::UserPromptSubmit) .expect("plugin should register a user-prompt hook"); assert_eq!( - hook.command, - Some(format!("python3 {}", expected_hook_path.display())) + hook.handler, + HookHandlerMetadata::Command { + command: format!("python3 {}", expected_hook_path.display()), + r#async: false, + } ); assert!(!hook.enabled); @@ -691,8 +695,11 @@ source = "{}" .join("hooks/log_version.py") .canonicalize()?; assert_eq!( - data[0].hooks[0].command, - Some(format!("python3 {}", expected_hook_path.display())) + data[0].hooks[0].handler, + HookHandlerMetadata::Command { + command: format!("python3 {}", expected_hook_path.display()), + r#async: false, + } ); let turn_id = mcp @@ -717,6 +724,100 @@ source = "{}" Ok(()) } +#[tokio::test] +async fn hooks_list_shows_discovered_plugin_mcp_tool_hook() -> Result<()> { + let codex_home = TempDir::new()?; + let cwd = TempDir::new()?; + write_plugin_hook_config( + codex_home.path(), + r#"{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "mcp_tool", + "server": "security", + "tool": "inspect", + "input": {"path": "${tool_input.path}"}, + "timeout": 9, + "statusMessage": "checking security policy" + } + ] + } + ] + } +}"#, + )?; + + let mut mcp = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_TIMEOUT) + .await?; + + let request_id = mcp + .send_hooks_list_request(HooksListParams { + cwds: vec![cwd.path().to_path_buf()], + }) + .await?; + let HooksListResponse { data } = + timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??; + let source_path = AbsolutePathBuf::from_absolute_path(std::fs::canonicalize( + codex_home + .path() + .join("plugins/cache/test/demo/local/hooks/hooks.json"), + )?)?; + let identity = NormalizedHookIdentity { + event_name: "pre_tool_use", + group: codex_config::MatcherGroup { + matcher: Some("Bash".to_string()), + hooks: vec![codex_config::HookHandlerConfig::McpTool { + server: "security".to_string(), + tool: "inspect".to_string(), + input: serde_json::from_value(serde_json::json!({ + "path": "${tool_input.path}", + }))?, + timeout_sec: Some(9), + status_message: Some("checking security policy".to_string()), + }], + }, + }; + let identity = codex_config::TomlValue::try_from(identity)?; + + assert_eq!( + data, + vec![HooksListEntry { + cwd: cwd.path().to_path_buf(), + hooks: vec![HookMetadata { + key: "demo@test:hooks/hooks.json:pre_tool_use:0:0".to_string(), + event_name: HookEventName::PreToolUse, + handler: HookHandlerMetadata::McpTool { + server: "security".to_string(), + tool: "inspect".to_string(), + }, + matcher: Some("Bash".to_string()), + timeout_sec: 9, + status_message: Some("checking security policy".to_string()), + additional_context_limit: None, + source_path, + source: HookSource::Plugin, + plugin_id: Some("demo@test".to_string()), + display_order: 0, + enabled: true, + is_managed: false, + current_hash: codex_config::version_for_toml(&identity), + trust_status: HookTrustStatus::Untrusted, + }], + warnings: Vec::new(), + errors: Vec::new(), + }] + ); + + Ok(()) +} + #[tokio::test] async fn hooks_list_warms_plugin_capabilities_for_thread_start() -> Result<()> { let codex_home = TempDir::new()?; @@ -884,10 +985,11 @@ timeout = 5 project_config_path.as_path().display() ), event_name: HookEventName::PreToolUse, - handler_type: HookHandlerType::Command, - execution_mode: HookExecutionMode::Sync, + handler: HookHandlerMetadata::Command { + command: "echo project hook".to_string(), + r#async: false, + }, matcher: Some("Bash".to_string()), - command: Some("echo project hook".to_string()), timeout_sec: 5, status_message: None, additional_context_limit: None, @@ -902,7 +1004,7 @@ timeout = 5 Some("Bash"), "echo project hook", /*timeout_sec*/ 5, - HookExecutionMode::Sync, + /*async*/ false, /*status_message*/ None, /*additional_context_limit*/ None, ), @@ -951,8 +1053,20 @@ async fn hooks_list_uses_root_repo_hooks_for_linked_worktrees() -> Result<()> { let repo_config_path = AbsolutePathBuf::from_absolute_path(repo_root.join(".codex/config.toml"))?; - assert_eq!(repo_hook.command.as_deref(), Some("echo root hook")); - assert_eq!(worktree_hook.command.as_deref(), Some("echo root hook")); + assert_eq!( + repo_hook.handler, + HookHandlerMetadata::Command { + command: "echo root hook".to_string(), + r#async: false, + } + ); + assert_eq!( + worktree_hook.handler, + HookHandlerMetadata::Command { + command: "echo root hook".to_string(), + r#async: false, + } + ); assert_eq!(repo_hook.key, worktree_hook.key); assert_eq!(repo_hook.source_path, repo_config_path); assert_eq!(worktree_hook.source_path, repo_config_path); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 262b4e0138fe..902a3ec35984 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -4172,6 +4172,7 @@ async fn build_hooks_config( plugin_hook_load_warnings, shell_program: hook_shell_program, shell_args: hook_shell_argv, + mcp_executor: None, } } diff --git a/codex-rs/hooks/src/engine/command_runner.rs b/codex-rs/hooks/src/engine/command_runner.rs index 662de697e6b5..fbd083742935 100644 --- a/codex-rs/hooks/src/engine/command_runner.rs +++ b/codex-rs/hooks/src/engine/command_runner.rs @@ -121,6 +121,7 @@ impl CommandHookRuntime { ConfiguredHandlerKind::Command { command, env, .. } => { run_command(&runtime, &handler, command, env, &input_json, &cwd).await } + ConfiguredHandlerKind::McpTool { .. } => return, }; let mut hook_result = parse(&handler, result, turn_id).completed; let mut entries = Vec::new(); diff --git a/codex-rs/hooks/src/engine/command_runner_tests.rs b/codex-rs/hooks/src/engine/command_runner_tests.rs index d63d533bc397..fc4ae899025c 100644 --- a/codex-rs/hooks/src/engine/command_runner_tests.rs +++ b/codex-rs/hooks/src/engine/command_runner_tests.rs @@ -156,6 +156,7 @@ async fn schedule(runtime: &CommandHookRuntime, handler: ConfiguredHandler, cwd: warnings: Vec::new(), required_load_errors: Vec::new(), command_runtime: runtime.clone(), + mcp_executor: None, }; engine .run_user_prompt_submit(UserPromptSubmitRequest { diff --git a/codex-rs/hooks/src/engine/discovery.rs b/codex-rs/hooks/src/engine/discovery.rs index 158eac120509..2c730a8a0988 100644 --- a/codex-rs/hooks/src/engine/discovery.rs +++ b/codex-rs/hooks/src/engine/discovery.rs @@ -24,6 +24,7 @@ use serde::Serialize; use super::ConfiguredHandler; use super::ConfiguredHandlerKind; use super::HookListEntry; +use super::HookListEntryHandler; use crate::config_rules::hook_states_from_stack; use crate::events::common::matcher_pattern_for_event; use crate::events::common::validate_matcher_pattern; @@ -31,8 +32,6 @@ use crate::events::session_end::SESSION_END_DEFAULT_TIMEOUT_SEC; use crate::events::session_end::SESSION_END_MAX_TIMEOUT_SEC; use crate::output_spill::AdditionalContextLimit; use crate::output_spill::DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT; -use codex_protocol::protocol::HookExecutionMode; -use codex_protocol::protocol::HookHandlerType; use codex_protocol::protocol::HookSource; use codex_protocol::protocol::HookTrustStatus; @@ -577,15 +576,63 @@ fn append_matcher_groups( additional_context_limit, } } - HookHandlerConfig::McpTool { .. } => { - source.record_load_failure( - format!( - "skipping MCP tool hook in {}: MCP tool hooks are not supported yet", - source.path.display() - ), - warnings, - ); - continue; + HookHandlerConfig::McpTool { + server, + tool, + input, + timeout_sec, + status_message, + } => { + if event_name == codex_protocol::protocol::HookEventName::SessionEnd { + source.record_load_failure( + format!( + "skipping MCP tool hook in {}: SessionEnd MCP hooks are not supported", + source.path.display() + ), + warnings, + ); + continue; + } + if server.trim().is_empty() || tool.trim().is_empty() { + source.record_load_failure( + format!( + "skipping MCP tool hook in {}: server and tool must not be empty", + source.path.display() + ), + warnings, + ); + continue; + } + if matches!(&source.requirement, HookRequirement::Required(_)) { + source.record_load_failure( + format!( + "skipping MCP tool hook in {}: MCP tool hooks are not supported yet", + source.path.display() + ), + warnings, + ); + continue; + } + + let timeout_sec = timeout_sec.unwrap_or(600).max(1); + let config = HookHandlerConfig::McpTool { + server: server.clone(), + tool: tool.clone(), + input: input.clone(), + timeout_sec: Some(timeout_sec), + status_message: status_message.clone(), + }; + NormalizedHandler { + config, + kind: ConfiguredHandlerKind::McpTool { + server, + tool, + input, + }, + timeout_sec, + status_message, + additional_context_limit: None, + } } HookHandlerConfig::Prompt {} => { source.record_load_failure( @@ -622,20 +669,26 @@ fn append_matcher_groups( let enabled = hook_enabled(source.is_managed, state); let trusted_hash = hook_trusted_hash(source.is_managed, state); let trust_status = hook_trust_status(source.is_managed, ¤t_hash, trusted_hash); - let ConfiguredHandlerKind::Command { command, .. } = &kind; - let execution_mode = - if matches!(kind, ConfiguredHandlerKind::Command { r#async: true, .. }) { - HookExecutionMode::Async - } else { - HookExecutionMode::Sync - }; + let handler = match &kind { + ConfiguredHandlerKind::Command { + command, r#async, .. + } => HookListEntryHandler::Command { + command: command.clone(), + r#async: *r#async, + }, + ConfiguredHandlerKind::McpTool { server, tool, .. } => { + HookListEntryHandler::McpTool { + server: server.clone(), + tool: tool.clone(), + } + } + }; hook_entries.push(HookListEntry { key, event_name, - handler_type: HookHandlerType::Command, + handler, matcher: matcher.map(ToOwned::to_owned), - command: Some(command.clone()), timeout_sec, status_message: status_message.clone(), additional_context_limit, @@ -647,7 +700,6 @@ fn append_matcher_groups( is_managed: source.is_managed, current_hash, trust_status, - execution_mode, }); if enabled && (source.bypass_hook_trust @@ -809,6 +861,7 @@ mod tests { use super::ConfiguredHandler; use super::ConfiguredHandlerKind; use super::HookListEntry; + use super::HookListEntryHandler; use super::append_matcher_groups; use crate::output_spill::AdditionalContextLimit; use crate::output_spill::DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT; @@ -926,6 +979,91 @@ mod tests { } } + #[test] + fn mcp_tool_hooks_preserve_argument_templates_and_list_their_target() { + let source_path = source_path(); + let hook_states = std::collections::HashMap::new(); + let input = serde_json::from_value(serde_json::json!({ + "file_path": "${tool_input.file_path}", + "optional": "${tool_input.optional}", + })) + .expect("MCP hook input should be an object"); + let mut handlers = Vec::new(); + let mut entries = Vec::new(); + let mut warnings = Vec::new(); + let mut display_order = 0; + + append_matcher_groups( + &mut handlers, + &mut entries, + &mut warnings, + &mut display_order, + &mut hook_handler_source(&source_path, &hook_states), + HookEventName::PostToolUse, + vec![MatcherGroup { + matcher: Some("Write|Edit".to_string()), + hooks: vec![HookHandlerConfig::McpTool { + server: "security".to_string(), + tool: "scan".to_string(), + input, + timeout_sec: Some(30), + status_message: Some("Scanning file".to_string()), + }], + }], + ); + + assert!(warnings.is_empty()); + assert_eq!(handlers.len(), 1); + assert_eq!(entries.len(), 1); + assert_eq!( + entries[0].handler, + HookListEntryHandler::McpTool { + server: "security".to_string(), + tool: "scan".to_string(), + } + ); + assert!(entries[0].current_hash.starts_with("sha256:")); + } + + #[test] + fn session_end_mcp_tool_hooks_are_warned_and_skipped() { + let source_path = source_path(); + let hook_states = std::collections::HashMap::new(); + let mut handlers = Vec::new(); + let mut entries = Vec::new(); + let mut warnings = Vec::new(); + let mut display_order = 0; + + append_matcher_groups( + &mut handlers, + &mut entries, + &mut warnings, + &mut display_order, + &mut hook_handler_source(&source_path, &hook_states), + HookEventName::SessionEnd, + vec![MatcherGroup { + matcher: None, + hooks: vec![HookHandlerConfig::McpTool { + server: "security".to_string(), + tool: "scan".to_string(), + input: serde_json::Map::new(), + timeout_sec: None, + status_message: None, + }], + }], + ); + + assert!(handlers.is_empty()); + assert!(entries.is_empty()); + assert_eq!( + warnings, + vec![format!( + "skipping MCP tool hook in {}: SessionEnd MCP hooks are not supported", + source_path.display() + )] + ); + } + fn discover_command( event_name: HookEventName, additional_context_limit: Option, @@ -1136,6 +1274,11 @@ mod tests { .collect::>(), vec![1, 3] ); + assert!( + handlers + .iter() + .all(ConfiguredHandler::can_apply_control_effects) + ); assert_eq!( handlers .iter() @@ -1150,6 +1293,10 @@ mod tests { .collect::>(), vec![1, 3] ); + assert!(hook_entries.iter().all(|entry| matches!( + entry.handler, + HookListEntryHandler::Command { r#async: false, .. } + ))); assert_eq!( hook_entries .iter() diff --git a/codex-rs/hooks/src/engine/dispatcher.rs b/codex-rs/hooks/src/engine/dispatcher.rs index 0858c409e437..8361de803ad1 100644 --- a/codex-rs/hooks/src/engine/dispatcher.rs +++ b/codex-rs/hooks/src/engine/dispatcher.rs @@ -16,6 +16,7 @@ use super::ConfiguredHandler; use super::ConfiguredHandlerKind; use super::HandlerRunResult; use super::command_runner::run_command; +use super::mcp_runner::run_mcp_tool; use crate::events::common::matches_matcher; #[derive(Debug)] @@ -123,14 +124,25 @@ pub(crate) async fn execute_handlers( ) .await } + ConfiguredHandlerKind::McpTool { + server, + tool, + input, + } => { + let executor = engine.mcp_executor.as_deref()?; + run_mcp_tool(executor, &handler, server, tool, input, &input_json).await + } }; - (configured_order, parse(&handler, result, turn_id)) + Some((configured_order, parse(&handler, result, turn_id))) }); } let mut completed = Vec::new(); let mut completion_order = 0; - while let Some((configured_order, mut parsed)) = pending.next().await { + while let Some(result) = pending.next().await { + let Some((configured_order, mut parsed)) = result else { + continue; + }; parsed.completion_order = completion_order; completion_order += 1; completed.push((configured_order, parsed)); @@ -205,6 +217,7 @@ pub(crate) fn hook_execution_mode_label(mode: HookExecutionMode) -> &'static str pub(crate) fn hook_handler_type_label(handler_type: HookHandlerType) -> &'static str { match handler_type { HookHandlerType::Command => "command", + HookHandlerType::McpTool => "mcp_tool", HookHandlerType::Prompt => "prompt", HookHandlerType::Agent => "agent", } diff --git a/codex-rs/hooks/src/engine/mcp_runner.rs b/codex-rs/hooks/src/engine/mcp_runner.rs new file mode 100644 index 000000000000..9ca497ce77d4 --- /dev/null +++ b/codex-rs/hooks/src/engine/mcp_runner.rs @@ -0,0 +1,140 @@ +use std::time::Duration; +use std::time::Instant; + +use anyhow::Context; +use anyhow::Result; +use regex::Regex; +use serde_json::Map; +use serde_json::Value; + +use super::ConfiguredHandler; +use super::HandlerRunResult; +use crate::mcp::HookMcpCall; +use crate::mcp::HookMcpExecutor; + +/// Expands an MCP argument template against this hook event and invokes the configured tool. +#[tracing::instrument( + name = "codex.hooks.mcp_tool", + level = "trace", + skip_all, + fields( + hook.server = server, + hook.tool = tool, + hook.timeout_sec = handler.timeout_sec, + ) +)] +pub(crate) async fn run_mcp_tool( + executor: &dyn HookMcpExecutor, + handler: &ConfiguredHandler, + server: &str, + tool: &str, + argument_template: &Map, + hook_event_json: &str, +) -> HandlerRunResult { + let started_at = chrono::Utc::now().timestamp(); + let started = Instant::now(); + let result = async { + let hook_event: Value = + serde_json::from_str(hook_event_json).context("failed to parse hook event input")?; + let input = expand_mcp_argument_template(argument_template, &hook_event)?; + executor + .execute(HookMcpCall { + server: server.to_string(), + tool: tool.to_string(), + input, + timeout: Duration::from_secs(handler.timeout_sec), + }) + .await + } + .await; + + let (exit_code, stdout, error) = match result { + Ok(output) => (Some(0), output, None), + Err(error) => (None, String::new(), Some(error.to_string())), + }; + HandlerRunResult { + started_at, + completed_at: chrono::Utc::now().timestamp(), + duration_ms: started.elapsed().as_millis().try_into().unwrap_or(i64::MAX), + exit_code, + stdout, + stderr: String::new(), + error, + } +} + +/// Recursively substitutes `${field.nested}` placeholders using values from a hook event. +/// +/// A complete placeholder preserves its JSON type; a placeholder embedded in surrounding +/// text is rendered as a string. Missing fields fail the hook instead of passing unresolved +/// arguments to the server. For example, the template `{"count":"${tool_input.count}"}` +/// becomes `{"count":3}` when the event contains `{"tool_input":{"count":3}}`. +fn expand_mcp_argument_template( + argument_template: &Map, + hook_event: &Value, +) -> Result> { + argument_template + .iter() + .map(|(key, value)| Ok((key.clone(), resolve_value(value, hook_event)?))) + .collect() +} + +fn resolve_value(value: &Value, hook_event: &Value) -> Result { + match value { + Value::Object(input) => Ok(Value::Object(expand_mcp_argument_template( + input, hook_event, + )?)), + Value::Array(items) => items + .iter() + .map(|item| resolve_value(item, hook_event)) + .collect::>>() + .map(Value::Array), + Value::String(text) => resolve_string(text, hook_event), + _ => Ok(value.clone()), + } +} + +fn resolve_string(text: &str, hook_event: &Value) -> Result { + let pattern = Regex::new(r"\$\{([^{}]+)\}")?; + let captures = pattern.captures_iter(text).collect::>(); + if captures.is_empty() { + return Ok(Value::String(text.to_string())); + } + + if let [capture] = captures.as_slice() + && let Some(placeholder) = capture.get(0) + && placeholder.start() == 0 + && placeholder.end() == text.len() + { + return resolve_path(hook_event, &capture[1]).cloned(); + } + + let mut resolved = String::new(); + let mut previous_end = 0; + for capture in captures { + let Some(placeholder) = capture.get(0) else { + continue; + }; + resolved.push_str(&text[previous_end..placeholder.start()]); + let value = resolve_path(hook_event, &capture[1])?; + match value { + Value::String(value) => resolved.push_str(value), + _ => resolved.push_str(&serde_json::to_string(value)?), + } + previous_end = placeholder.end(); + } + resolved.push_str(&text[previous_end..]); + Ok(Value::String(resolved)) +} + +fn resolve_path<'a>(hook_event: &'a Value, path: &str) -> Result<&'a Value> { + path.split('.').try_fold(hook_event, |value, field| { + value + .get(field) + .with_context(|| format!("hook input placeholder `${{{path}}}` was not found")) + }) +} + +#[cfg(test)] +#[path = "mcp_runner_tests.rs"] +mod tests; diff --git a/codex-rs/hooks/src/engine/mcp_runner_tests.rs b/codex-rs/hooks/src/engine/mcp_runner_tests.rs new file mode 100644 index 000000000000..0f96c785bbfd --- /dev/null +++ b/codex-rs/hooks/src/engine/mcp_runner_tests.rs @@ -0,0 +1,137 @@ +use std::sync::Arc; +use std::sync::Mutex; +use std::time::Duration; + +use futures::FutureExt; +use futures::future::BoxFuture; +use pretty_assertions::assert_eq; +use serde_json::Map; +use serde_json::Value; +use serde_json::json; + +use super::expand_mcp_argument_template; +use super::run_mcp_tool; +use crate::engine::ConfiguredHandler; +use crate::engine::ConfiguredHandlerKind; +use crate::mcp::HookMcpCall; +use crate::mcp::HookMcpExecutor; +use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookSource; +use codex_utils_absolute_path::test_support::PathBufExt; +use codex_utils_absolute_path::test_support::test_path_buf; + +struct RecordingExecutor { + calls: Arc>>, + output: String, +} + +impl HookMcpExecutor for RecordingExecutor { + fn execute(&self, call: HookMcpCall) -> BoxFuture<'_, anyhow::Result> { + async move { + self.calls.lock().expect("lock calls").push(call); + Ok(self.output.clone()) + } + .boxed() + } +} + +#[test] +fn placeholders_preserve_json_types_and_expand_nested_inputs() { + let event_input = json!({ + "tool_input": { + "file_path": "/tmp/example.rs", + "count": 3, + "optional": null, + "metadata": { "language": "rust" }, + }, + }); + let input = serde_json::from_value::>(json!({ + "path": "${tool_input.file_path}", + "message": "scan ${tool_input.file_path}", + "count": "${tool_input.count}", + "optional": "${tool_input.optional}", + "nested": ["${tool_input.metadata}", { "literal": true }], + })) + .expect("object input"); + + assert_eq!( + Value::Object( + expand_mcp_argument_template(&input, &event_input).expect("expand MCP arguments") + ), + json!({ + "path": "/tmp/example.rs", + "message": "scan /tmp/example.rs", + "count": 3, + "optional": null, + "nested": [{ "language": "rust" }, { "literal": true }], + }) + ); +} + +#[test] +fn missing_placeholder_fails_without_passing_unresolved_input() { + let input = serde_json::from_value::>(json!({ + "path": "${tool_input.missing}", + })) + .expect("object input"); + + let error = expand_mcp_argument_template(&input, &json!({ "tool_input": {} })) + .expect_err("missing placeholder should fail"); + + assert_eq!( + error.to_string(), + "hook input placeholder `${tool_input.missing}` was not found" + ); +} + +#[tokio::test] +async fn mcp_tool_results_use_command_hook_output_contract() { + let configured_input = serde_json::from_value::>(json!({ + "file_path": "${tool_input.file_path}", + })) + .expect("object input"); + let handler = ConfiguredHandler { + event_name: HookEventName::PostToolUse, + matcher: None, + timeout_sec: 30, + status_message: None, + additional_context_limit: Default::default(), + source_path: test_path_buf("/tmp/hooks.json").abs(), + source: HookSource::User, + display_order: 0, + kind: ConfiguredHandlerKind::McpTool { + server: "security".to_string(), + tool: "scan".to_string(), + input: configured_input.clone(), + }, + }; + let calls = Arc::new(Mutex::new(Vec::new())); + let executor = RecordingExecutor { + calls: Arc::clone(&calls), + output: r#"{"decision":"block","reason":"unsafe file"}"#.to_string(), + }; + + let result = run_mcp_tool( + &executor, + &handler, + "security", + "scan", + &configured_input, + r#"{"tool_input":{"file_path":"/tmp/example.rs"}}"#, + ) + .await; + + assert_eq!(result.exit_code, Some(0)); + assert_eq!(result.stdout, executor.output); + assert_eq!(result.error, None); + assert_eq!( + *calls.lock().expect("lock calls"), + vec![HookMcpCall { + server: "security".to_string(), + tool: "scan".to_string(), + input: serde_json::from_value(json!({ "file_path": "/tmp/example.rs" })) + .expect("object input"), + timeout: Duration::from_secs(30), + }] + ); +} diff --git a/codex-rs/hooks/src/engine/mod.rs b/codex-rs/hooks/src/engine/mod.rs index 421e25537c8a..c9f44ece9e9a 100644 --- a/codex-rs/hooks/src/engine/mod.rs +++ b/codex-rs/hooks/src/engine/mod.rs @@ -1,6 +1,7 @@ pub(crate) mod command_runner; pub(crate) mod discovery; pub(crate) mod dispatcher; +pub(crate) mod mcp_runner; pub(crate) mod output_parser; pub(crate) mod schema_loader; @@ -22,6 +23,7 @@ use crate::events::stop::StopOutcome; use crate::events::stop::StopRequest; use crate::events::user_prompt_submit::UserPromptSubmitOutcome; use crate::events::user_prompt_submit::UserPromptSubmitRequest; +use crate::mcp::HookMcpExecutor; use crate::output_spill::AdditionalContextLimit; use codex_config::ConfigLayerStack; use codex_plugin::PluginHookSource; @@ -33,6 +35,7 @@ use codex_protocol::protocol::HookSource; use codex_protocol::protocol::HookTrustStatus; use codex_utils_absolute_path::AbsolutePathBuf; use std::collections::HashMap; +use std::sync::Arc; use std::time::Duration; use command_runner::CommandHookRuntime; @@ -63,6 +66,11 @@ pub(crate) enum ConfiguredHandlerKind { env: HashMap, r#async: bool, }, + McpTool { + server: String, + tool: String, + input: serde_json::Map, + }, } #[derive(Debug)] @@ -80,7 +88,8 @@ impl ConfiguredHandler { pub(crate) fn execution_mode(&self) -> HookExecutionMode { match self.kind { ConfiguredHandlerKind::Command { r#async: true, .. } => HookExecutionMode::Async, - ConfiguredHandlerKind::Command { r#async: false, .. } => HookExecutionMode::Sync, + ConfiguredHandlerKind::Command { r#async: false, .. } + | ConfiguredHandlerKind::McpTool { .. } => HookExecutionMode::Sync, } } @@ -117,17 +126,23 @@ impl ConfiguredHandler { fn handler_type(&self) -> HookHandlerType { match &self.kind { ConfiguredHandlerKind::Command { .. } => HookHandlerType::Command, + ConfiguredHandlerKind::McpTool { .. } => HookHandlerType::McpTool, } } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum HookListEntryHandler { + Command { command: String, r#async: bool }, + McpTool { server: String, tool: String }, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct HookListEntry { pub key: String, pub event_name: HookEventName, - pub handler_type: HookHandlerType, + pub handler: HookListEntryHandler, pub matcher: Option, - pub command: Option, pub timeout_sec: u64, pub status_message: Option, pub additional_context_limit: Option, @@ -139,7 +154,6 @@ pub struct HookListEntry { pub is_managed: bool, pub current_hash: String, pub trust_status: HookTrustStatus, - pub execution_mode: codex_protocol::protocol::HookExecutionMode, } #[derive(Clone)] @@ -148,6 +162,7 @@ pub(crate) struct ClaudeHooksEngine { warnings: Vec, required_load_errors: Vec, pub(crate) command_runtime: CommandHookRuntime, + mcp_executor: Option>, } impl ClaudeHooksEngine { @@ -158,6 +173,7 @@ impl ClaudeHooksEngine { plugin_hook_sources: Vec, plugin_hook_load_warnings: Vec, command_runtime: CommandHookRuntime, + mcp_executor: Option>, ) -> Self { if !enabled { return Self { @@ -165,22 +181,36 @@ impl ClaudeHooksEngine { warnings: Vec::new(), required_load_errors: Vec::new(), command_runtime, + mcp_executor, }; } let _ = schema_loader::generated_hook_schemas(); - let discovered = discovery::discover_handlers( + let mut discovered = discovery::discover_handlers( config_layer_stack, plugin_hook_sources, plugin_hook_load_warnings, bypass_hook_trust, ); - + if mcp_executor.is_none() { + discovered.handlers.retain(|handler| { + if matches!(&handler.kind, ConfiguredHandlerKind::McpTool { .. }) { + discovered.warnings.push(format!( + "skipping MCP tool hook in {}: MCP invocation is not available yet", + handler.source_path.display() + )); + false + } else { + true + } + }); + } Self { handlers: discovered.handlers, warnings: discovered.warnings, required_load_errors: discovered.required_load_errors, command_runtime, + mcp_executor, } } diff --git a/codex-rs/hooks/src/engine/mod_tests.rs b/codex-rs/hooks/src/engine/mod_tests.rs index 8f6194a19f46..baf486d9140a 100644 --- a/codex-rs/hooks/src/engine/mod_tests.rs +++ b/codex-rs/hooks/src/engine/mod_tests.rs @@ -1,6 +1,8 @@ use std::collections::HashMap; use std::fs; use std::path::Path; +use std::sync::Arc; +use std::sync::Mutex; use std::time::Duration; use codex_config::AbsolutePathBuf; @@ -22,11 +24,14 @@ use codex_plugin::PluginHookSource; use codex_plugin::PluginId; use codex_protocol::ThreadId; use codex_protocol::protocol::HookEventName; +use codex_protocol::protocol::HookHandlerType; use codex_protocol::protocol::HookOutputEntry; use codex_protocol::protocol::HookOutputEntryKind; use codex_protocol::protocol::HookRunStatus; use codex_protocol::protocol::HookSource; use codex_protocol::protocol::HookTrustStatus; +use futures::FutureExt; +use futures::future::BoxFuture; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -35,7 +40,10 @@ use super::CommandHookRuntime; use super::CommandShell; use super::ConfiguredHandler; use super::ConfiguredHandlerKind; +use super::HookListEntryHandler; use crate::events::pre_tool_use::PreToolUseRequest; +use crate::mcp::HookMcpCall; +use crate::mcp::HookMcpExecutor; fn cwd() -> AbsolutePathBuf { AbsolutePathBuf::current_dir().expect("current dir") @@ -58,6 +66,7 @@ fn permission_request_timeout_only_counts_synchronous_handlers() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); let command = "echo synchronous permission hook"; let synchronous_handler = ConfiguredHandler { @@ -354,6 +363,84 @@ fn required_managed_hooks_reject_unsupported_handler_types() { ); } +#[test] +fn required_managed_mcp_hooks_reject_empty_targets() { + let temp = tempdir().expect("create temp dir"); + let events = HookEventsToml { + pre_tool_use: vec![MatcherGroup { + matcher: Some("^Bash$".to_string()), + hooks: vec![HookHandlerConfig::McpTool { + server: "policy".to_string(), + tool: " ".to_string(), + input: Default::default(), + timeout_sec: None, + status_message: None, + }], + }], + ..Default::default() + }; + let config_layer_stack = required_hooks_stack( + managed_hooks_for_current_platform(temp.path(), events), + RequirementSource::LegacyManagedConfigTomlFromMdm, + ); + + let error = crate::Hooks::new( + crate::HooksConfig { + feature_enabled: true, + config_layer_stack: Some(config_layer_stack), + ..Default::default() + }, + ThreadId::new(), + ) + .err() + .expect("invalid required managed MCP hook should reject startup"); + + assert!( + error + .to_string() + .contains("server and tool must not be empty") + ); +} + +#[test] +fn required_managed_session_end_mcp_hooks_reject_startup() { + let temp = tempdir().expect("create temp dir"); + let events = HookEventsToml { + session_end: vec![MatcherGroup { + matcher: None, + hooks: vec![HookHandlerConfig::McpTool { + server: "policy".to_string(), + tool: "check".to_string(), + input: Default::default(), + timeout_sec: None, + status_message: None, + }], + }], + ..Default::default() + }; + let config_layer_stack = required_hooks_stack( + managed_hooks_for_current_platform(temp.path(), events), + RequirementSource::LegacyManagedConfigTomlFromMdm, + ); + + let error = crate::Hooks::new( + crate::HooksConfig { + feature_enabled: true, + config_layer_stack: Some(config_layer_stack), + ..Default::default() + }, + ThreadId::new(), + ) + .err() + .expect("required managed SessionEnd MCP hook should reject startup"); + + assert!( + error + .to_string() + .contains("SessionEnd MCP hooks are not supported") + ); +} + #[test] fn required_managed_hooks_with_unknown_source_still_reject_discovery_failures() { let temp = tempdir().expect("create temp dir"); @@ -493,6 +580,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().is_empty()); @@ -510,6 +598,7 @@ with Path(r"{log_path}").open("a", encoding="utf-8") as handle: plugin_hook_load_warnings: Vec::new(), shell_program: None, shell_args: Vec::new(), + mcp_executor: None, }); assert!(listed.hooks[0].is_managed); let cwd = cwd(); @@ -600,6 +689,7 @@ async fn requirements_managed_hooks_execute_windows_command_override() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); let outcome = engine @@ -680,6 +770,7 @@ fn unknown_requirement_source_hooks_stay_managed() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert_eq!(engine.handlers.len(), 1); @@ -763,6 +854,7 @@ fn user_disablement_filters_non_managed_hooks_but_not_managed_hooks() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert_eq!(engine.handlers.len(), 1); @@ -829,6 +921,7 @@ fn user_disablement_does_not_filter_managed_layer_hooks() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert_eq!(engine.handlers.len(), 1); @@ -991,6 +1084,7 @@ fn requirements_managed_hooks_load_when_managed_dir_is_missing() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().is_empty()); @@ -1054,6 +1148,7 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().is_empty()); @@ -1067,8 +1162,11 @@ fn allow_managed_hooks_only_false_keeps_unmanaged_hooks() { assert_eq!(discovered.hook_entries.len(), 1); assert!(!discovered.hook_entries[0].is_managed); assert_eq!( - discovered.hook_entries[0].command.as_deref(), - Some("python3 /tmp/user-hook.py") + discovered.hook_entries[0].handler, + HookListEntryHandler::Command { + command: "python3 /tmp/user-hook.py".to_string(), + r#async: false, + } ); } @@ -1108,6 +1206,7 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().is_empty()); @@ -1121,8 +1220,11 @@ fn allow_managed_hooks_only_in_config_toml_does_not_enable_policy() { assert_eq!(discovered.hook_entries.len(), 1); assert!(!discovered.hook_entries[0].is_managed); assert_eq!( - discovered.hook_entries[0].command.as_deref(), - Some("python3 /tmp/user-hook.py") + discovered.hook_entries[0].handler, + HookListEntryHandler::Command { + command: "python3 /tmp/user-hook.py".to_string(), + r#async: false, + } ); } @@ -1178,6 +1280,7 @@ fn allow_managed_hooks_only_skips_unmanaged_json_and_toml_hooks() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.handlers.is_empty()); @@ -1217,6 +1320,7 @@ fn allow_managed_hooks_only_skips_unmanaged_plugin_hooks() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.handlers.is_empty()); @@ -1289,6 +1393,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().is_empty()); @@ -1298,6 +1403,7 @@ fn allow_managed_hooks_only_keeps_managed_requirement_and_config_layer_hooks() { .iter() .map(|handler| match &handler.kind { ConfiguredHandlerKind::Command { command, .. } => Some(command.as_str()), + ConfiguredHandlerKind::McpTool { .. } => None, }) .collect::>(), vec![ @@ -1401,6 +1507,7 @@ fn discovers_hooks_from_json_and_toml_in_the_same_layer() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().iter().any(|warning| { @@ -1496,6 +1603,7 @@ fn profile_user_layers_load_shared_hooks_json_once() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.warnings().is_empty()); @@ -1570,6 +1678,7 @@ fn malformed_hooks_json_is_reported_as_startup_warning() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert!(engine.handlers.is_empty()); @@ -1642,6 +1751,7 @@ print(json.dumps({ program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); let preview = engine.preview_pre_tool_use(&PreToolUseRequest { @@ -1669,6 +1779,7 @@ print(json.dumps({ plugin_hook_load_warnings: Vec::new(), shell_program: None, shell_args: Vec::new(), + mcp_executor: None, }); assert_eq!( listed.hooks[0].plugin_id.as_deref(), @@ -1762,6 +1873,7 @@ fn plugin_hook_sources_expand_plugin_placeholders() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert_eq!( @@ -1806,7 +1918,143 @@ fn plugin_hook_load_warnings_are_startup_warnings() { program: String::new(), args: Vec::new(), }), + /*mcp_executor*/ None, ); assert_eq!(engine.warnings(), &["failed plugin hook".to_string()]); } + +struct StaticMcpExecutor { + calls: Arc>>, + output: String, +} + +impl HookMcpExecutor for StaticMcpExecutor { + fn execute(&self, call: HookMcpCall) -> BoxFuture<'_, anyhow::Result> { + async move { + self.calls.lock().expect("lock MCP calls").push(call); + Ok(self.output.clone()) + } + .boxed() + } +} + +#[tokio::test] +async fn mcp_tool_hooks_expand_event_input_and_apply_pre_tool_decisions() { + let temp = tempdir().expect("create temp dir"); + let config_path = + AbsolutePathBuf::try_from(temp.path().join("config.toml")).expect("absolute config path"); + fs::write( + temp.path().join("hooks.json"), + serde_json::json!({ + "hooks": { + "PreToolUse": [{ + "matcher": "Bash", + "hooks": [{ + "type": "mcp_tool", + "server": "security", + "tool": "scan", + "input": { "command": "${tool_input.command}" }, + "timeout": 20, + }], + }], + }, + }) + .to_string(), + ) + .expect("write MCP hooks.json"); + let config_layer_stack = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::User { + file: config_path, + profile: None, + }, + TomlValue::Table(Default::default()), + )], + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("config layer stack"); + + let unavailable = ClaudeHooksEngine::new( + /*enabled*/ true, + /*bypass_hook_trust*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + command_runtime(CommandShell { + program: String::new(), + args: Vec::new(), + }), + /*mcp_executor*/ None, + ); + assert!(unavailable.handlers.is_empty()); + assert_eq!(unavailable.warnings().len(), 1); + assert!(unavailable.warnings()[0].contains("MCP invocation is not available")); + + let calls = Arc::new(Mutex::new(Vec::new())); + let executor = StaticMcpExecutor { + calls: Arc::clone(&calls), + output: serde_json::json!({ + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": "blocked by MCP scanner", + }, + }) + .to_string(), + }; + let engine = ClaudeHooksEngine::new( + /*enabled*/ true, + /*bypass_hook_trust*/ true, + Some(&config_layer_stack), + Vec::new(), + Vec::new(), + command_runtime(CommandShell { + program: String::new(), + args: Vec::new(), + }), + Some(Arc::new(executor)), + ); + let outcome = engine + .run_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": "rm important.txt" }), + }) + .await; + + assert!(outcome.should_block); + assert_eq!( + outcome.block_reason.as_deref(), + Some("blocked by MCP scanner") + ); + assert_eq!( + outcome.hook_events[0].run.handler_type, + HookHandlerType::McpTool + ); + assert_eq!( + outcome.hook_events[0].run.execution_mode, + codex_protocol::protocol::HookExecutionMode::Sync + ); + assert_eq!( + *calls.lock().expect("lock MCP calls"), + vec![HookMcpCall { + server: "security".to_string(), + tool: "scan".to_string(), + input: serde_json::from_value(serde_json::json!({ + "command": "rm important.txt", + })) + .expect("object input"), + timeout: Duration::from_secs(20), + }] + ); +} diff --git a/codex-rs/hooks/src/lib.rs b/codex-rs/hooks/src/lib.rs index 2bc0731d30de..7536bda0b413 100644 --- a/codex-rs/hooks/src/lib.rs +++ b/codex-rs/hooks/src/lib.rs @@ -3,6 +3,7 @@ mod declarations; mod engine; pub(crate) mod events; mod legacy_notify; +mod mcp; mod output_spill; mod registry; mod schema; @@ -14,6 +15,7 @@ pub use config_rules::hook_states_from_stack; pub use declarations::PluginHookDeclaration; pub use declarations::plugin_hook_declarations; pub use engine::HookListEntry; +pub use engine::HookListEntryHandler; pub use events::common::SubagentHookContext; /// Hook event names as they appear in hooks JSON and config files. pub const HOOK_EVENT_NAMES: [&str; 11] = [ @@ -71,6 +73,8 @@ pub use events::user_prompt_submit::UserPromptSubmitOutcome; pub use events::user_prompt_submit::UserPromptSubmitRequest; pub use legacy_notify::legacy_notify_json; pub use legacy_notify::notify_hook; +pub use mcp::HookMcpCall; +pub use mcp::HookMcpExecutor; pub use registry::HookListOutcome; pub use registry::Hooks; pub use registry::HooksConfig; diff --git a/codex-rs/hooks/src/mcp.rs b/codex-rs/hooks/src/mcp.rs new file mode 100644 index 000000000000..5999db2954af --- /dev/null +++ b/codex-rs/hooks/src/mcp.rs @@ -0,0 +1,22 @@ +use std::time::Duration; + +use futures::future::BoxFuture; +use serde_json::Map; +use serde_json::Value; + +/// One MCP tool call requested by a configured hook handler. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HookMcpCall { + pub server: String, + pub tool: String, + pub input: Map, + pub timeout: Duration, +} + +/// Executes already-connected MCP tools on behalf of hooks without coupling this crate to core. +/// +/// Implementations own server readiness, policy enforcement, timeout handling, and elicitation. +pub trait HookMcpExecutor: Send + Sync { + /// Returns text that is interpreted using ordinary command-hook output semantics. + fn execute(&self, call: HookMcpCall) -> BoxFuture<'_, anyhow::Result>; +} diff --git a/codex-rs/hooks/src/registry.rs b/codex-rs/hooks/src/registry.rs index 8f3680c1b2e9..f45a645e3d0f 100644 --- a/codex-rs/hooks/src/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -20,6 +20,7 @@ use crate::events::stop::StopOutcome; use crate::events::stop::StopRequest; use crate::events::user_prompt_submit::UserPromptSubmitOutcome; use crate::events::user_prompt_submit::UserPromptSubmitRequest; +use crate::mcp::HookMcpExecutor; use crate::types::Hook; use crate::types::HookEvent; use crate::types::HookPayload; @@ -29,6 +30,7 @@ use codex_config::ConfigLayerStack; use codex_plugin::PluginHookSource; use codex_protocol::ThreadId; use codex_protocol::shell_environment::scrub_non_inheritable_env_vars; +use std::sync::Arc; use std::time::Duration; use tokio::process::Command; @@ -42,6 +44,7 @@ pub struct HooksConfig { pub plugin_hook_load_warnings: Vec, pub shell_program: Option, pub shell_args: Vec, + pub mcp_executor: Option>, } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -105,6 +108,7 @@ impl Hooks { config.plugin_hook_sources, config.plugin_hook_load_warnings, command_runtime, + config.mcp_executor, ); Self { after_agent, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index c8b5992b7a5a..2ae44aea087d 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1514,6 +1514,7 @@ pub enum HookEventName { #[serde(rename_all = "snake_case")] pub enum HookHandlerType { Command, + McpTool, Prompt, Agent, } diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index 359b64d2541f..2a765613bfd7 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -1,5 +1,5 @@ use codex_app_server_protocol::HookEventName; -use codex_app_server_protocol::HookExecutionMode; +use codex_app_server_protocol::HookHandlerMetadata; use codex_app_server_protocol::HookMetadata; use codex_app_server_protocol::HookSource; use codex_app_server_protocol::HookTrustStatus; @@ -487,17 +487,34 @@ impl HooksBrowserView { width, /*max_lines*/ None, )); - lines.extend(detail_wrapped_lines( - "Command", - hook.command.as_deref().unwrap_or("-"), - width, - Some(MAX_COMMAND_DETAIL_LINES), - )); - let execution_mode = match hook.execution_mode { - HookExecutionMode::Sync => "Sync", - HookExecutionMode::Async => "Async", - }; - lines.push(detail_line("Mode", execution_mode)); + match &hook.handler { + HookHandlerMetadata::Command { command, r#async } => { + lines.extend(detail_wrapped_lines( + "Command", + command, + width, + Some(MAX_COMMAND_DETAIL_LINES), + )); + lines.push(detail_line("Mode", if *r#async { "Async" } else { "Sync" })); + } + HookHandlerMetadata::McpTool { server, tool } => { + lines.extend(detail_wrapped_lines( + "MCP Server", + server, + width, + /*max_lines*/ None, + )); + lines.extend(detail_wrapped_lines( + "MCP Tool", tool, width, /*max_lines*/ None, + )); + } + HookHandlerMetadata::Prompt {} => { + lines.push(detail_line("Handler", "Prompt")); + } + HookHandlerMetadata::Agent {} => { + lines.push(detail_line("Handler", "Agent")); + } + } lines.push(detail_line("Timeout", &format!("{}s", hook.timeout_sec))); if let Some(limit) = hook.additional_context_limit { let value = if limit == 0 { @@ -839,13 +856,18 @@ fn detail_wrapped_lines( width: usize, max_lines: Option, ) -> Vec> { - let prefix = format!("{label:<10}"); + let label_width = label.width().saturating_add(1).max(10); + let prefix = format!("{label:(); + let mut configured_hook = hook( + "plugin:security-scanner", + HookEventName::PreToolUse, + HookSource::Plugin, + Some("security-tools@openai-curated"), + "", + /*enabled*/ true, + /*is_managed*/ false, + /*display_order*/ 0, + ); + configured_hook.handler = HookHandlerMetadata::McpTool { + server: "scanner".to_string(), + tool: "scan_file".to_string(), + }; + let mut view = HooksBrowserView::new( + vec![configured_hook], + Vec::new(), + Vec::new(), + AppEventSender::new(tx_raw), + ); + + view.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert_snapshot!( + "hooks_browser_mcp_tool_handler", + render_lines(&view, /*width*/ 112) + ); + } + #[test] fn renders_handler_additional_context_limit() { let (tx_raw, _rx) = unbounded_channel::(); @@ -1150,7 +1204,10 @@ mod tests { /*display_order*/ 0, ); untrusted_hook.trust_status = HookTrustStatus::Untrusted; - untrusted_hook.execution_mode = HookExecutionMode::Async; + let HookHandlerMetadata::Command { r#async, .. } = &mut untrusted_hook.handler else { + panic!("expected command hook"); + }; + *r#async = true; let mut view = HooksBrowserView::new( vec![untrusted_hook], Vec::new(), diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap new file mode 100644 index 000000000000..e3ba7d099c1d --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__hooks_browser_view__tests__hooks_browser_mcp_tool_handler.snap @@ -0,0 +1,19 @@ +--- +source: tui/src/bottom_pane/hooks_browser_view.rs +expression: "render_lines(&view, 112)" +--- + + PreToolUse hooks + Turn hooks on or off. Your changes are saved automatically. + + [x] Hook 1 + + Event PreToolUse + Matcher Bash + Source Plugin - security-tools@openai-curated + MCP Server scanner + MCP Tool scan_file + Timeout 30s + Trust Trusted + + Press space or enter to toggle; esc to go back diff --git a/codex-rs/tui/src/startup_hooks_review.rs b/codex-rs/tui/src/startup_hooks_review.rs index 4bf9daea5b31..4a8f6433380e 100644 --- a/codex-rs/tui/src/startup_hooks_review.rs +++ b/codex-rs/tui/src/startup_hooks_review.rs @@ -306,8 +306,7 @@ mod tests { use crate::test_support::PathBufExt; use crate::test_support::test_path_buf; use codex_app_server_protocol::HookEventName; - use codex_app_server_protocol::HookExecutionMode; - use codex_app_server_protocol::HookHandlerType; + use codex_app_server_protocol::HookHandlerMetadata; use codex_app_server_protocol::HookMetadata; use codex_app_server_protocol::HookSource; use codex_app_server_protocol::HookTrustStatus; @@ -321,11 +320,12 @@ mod tests { HookMetadata { key: key.to_string(), event_name: HookEventName::PreToolUse, - handler_type: HookHandlerType::Command, - execution_mode: HookExecutionMode::Sync, + handler: HookHandlerMetadata::Command { + command: "/tmp/hook.sh".to_string(), + r#async: false, + }, is_managed: false, matcher: Some("Bash".to_string()), - command: Some("/tmp/hook.sh".to_string()), timeout_sec: 30, status_message: None, additional_context_limit: None,