diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2bb451b58..b29c2e940 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1669,6 +1669,7 @@ dependencies = [ "codex-execpolicy", "codex-file-search", "codex-git", + "codex-hooks", "codex-keyring-store", "codex-network-proxy", "codex-otel", @@ -1912,6 +1913,21 @@ dependencies = [ "walkdir", ] +[[package]] +name = "codex-hooks" +version = "0.0.0" +dependencies = [ + "anyhow", + "chrono", + "codex-protocol", + "futures", + "pretty_assertions", + "serde", + "serde_json", + "tempfile", + "tokio", +] + [[package]] name = "codex-keyring-store" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 02a09e562..eebc652d4 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -17,6 +17,7 @@ members = [ "cli", "common", "core", + "hooks", "secrets", "exec", "exec-server", @@ -82,6 +83,7 @@ codex-cli = { path = "cli"} codex-client = { path = "codex-client" } codex-common = { path = "common" } codex-core = { path = "core" } +codex-hooks = { path = "hooks" } codex-secrets = { path = "secrets" } codex-exec = { path = "exec" } codex-execpolicy = { path = "execpolicy" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 8b9f76a68..4142a72d7 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -36,6 +36,7 @@ codex-client = { workspace = true } codex-execpolicy = { workspace = true } codex-file-search = { workspace = true } codex-git = { workspace = true } +codex-hooks = { workspace = true } codex-keyring-store = { workspace = true } codex-network-proxy = { workspace = true } codex-otel = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 490cf2b76..38fec2e12 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -27,9 +27,6 @@ use crate::features::FEATURES; use crate::features::Feature; use crate::features::Features; use crate::features::maybe_push_unstable_features_warning; -use crate::hooks::HookEvent; -use crate::hooks::HookEventAfterAgent; -use crate::hooks::Hooks; use crate::models_manager::manager::ModelsManager; use crate::parse_command::parse_command; use crate::parse_turn_item; @@ -45,6 +42,10 @@ use crate::turn_metadata::resolve_turn_metadata_header_with_timeout; use crate::util::error_or_panic; use async_channel::Receiver; use async_channel::Sender; +use codex_hooks::HookEvent; +use codex_hooks::HookEventAfterAgent; +use codex_hooks::HookPayload; +use codex_hooks::Hooks; use codex_network_proxy::NetworkProxy; use codex_protocol::ThreadId; use codex_protocol::approvals::ExecPolicyAmendment; @@ -1102,7 +1103,7 @@ impl Session { Arc::clone(&config), Arc::clone(&auth_manager), ), - hooks: Hooks::new(config.as_ref()), + hooks: Hooks::new(config.notify.clone()), rollout: Mutex::new(rollout_recorder), user_shell: Arc::new(default_shell), shell_snapshot_tx, @@ -4056,7 +4057,7 @@ pub(crate) async fn run_turn( if !needs_follow_up { last_agent_message = sampling_request_last_agent_message; sess.hooks() - .dispatch(crate::hooks::HookPayload { + .dispatch(HookPayload { session_id: sess.conversation_id, cwd: turn_context.cwd.clone(), triggered_at: chrono::Utc::now(), @@ -6217,7 +6218,7 @@ mod tests { Arc::clone(&config), Arc::clone(&auth_manager), ), - hooks: Hooks::new(&config), + hooks: Hooks::new(config.notify.clone()), rollout: Mutex::new(None), user_shell: Arc::new(default_user_shell()), shell_snapshot_tx: watch::channel(None).0, @@ -6352,7 +6353,7 @@ mod tests { Arc::clone(&config), Arc::clone(&auth_manager), ), - hooks: Hooks::new(&config), + hooks: Hooks::new(config.notify.clone()), rollout: Mutex::new(None), user_shell: Arc::new(default_user_shell()), shell_snapshot_tx: watch::channel(None).0, diff --git a/codex-rs/core/src/hooks/mod.rs b/codex-rs/core/src/hooks/mod.rs deleted file mode 100644 index 2c0612825..000000000 --- a/codex-rs/core/src/hooks/mod.rs +++ /dev/null @@ -1,8 +0,0 @@ -mod registry; -mod types; -mod user_notification; - -pub(crate) use registry::Hooks; -pub(crate) use types::HookEvent; -pub(crate) use types::HookEventAfterAgent; -pub(crate) use types::HookPayload; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 883c89682..7378fe3e6 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -37,7 +37,6 @@ pub mod features; mod file_watcher; mod flags; pub mod git_info; -pub mod hooks; pub mod instructions; pub mod landlock; pub mod mcp; diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 04beef77b..3aec0e535 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -8,13 +8,13 @@ use crate::client::ModelClient; use crate::config::StartedNetworkProxy; use crate::exec_policy::ExecPolicyManager; use crate::file_watcher::FileWatcher; -use crate::hooks::Hooks; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; use crate::skills::SkillsManager; use crate::state_db::StateDbHandle; use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; +use codex_hooks::Hooks; use codex_otel::OtelManager; use tokio::sync::Mutex; use tokio::sync::RwLock; diff --git a/codex-rs/hooks/BUILD.bazel b/codex-rs/hooks/BUILD.bazel new file mode 100644 index 000000000..f13a065f2 --- /dev/null +++ b/codex-rs/hooks/BUILD.bazel @@ -0,0 +1,6 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "hooks", + crate_name = "codex_hooks", +) diff --git a/codex-rs/hooks/Cargo.toml b/codex-rs/hooks/Cargo.toml new file mode 100644 index 000000000..a49b1166f --- /dev/null +++ b/codex-rs/hooks/Cargo.toml @@ -0,0 +1,27 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-hooks" +version.workspace = true + +[lib] +doctest = false +name = "codex_hooks" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +chrono = { workspace = true, features = ["serde"] } +codex-protocol = { workspace = true } +futures = { workspace = true, features = ["alloc"] } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["process"] } + +[dev-dependencies] +anyhow = { workspace = true } +pretty_assertions = { workspace = true } +tempfile = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread", "time"] } diff --git a/codex-rs/hooks/src/lib.rs b/codex-rs/hooks/src/lib.rs new file mode 100644 index 000000000..c3d4383ba --- /dev/null +++ b/codex-rs/hooks/src/lib.rs @@ -0,0 +1,13 @@ +mod registry; +mod types; +mod user_notification; + +pub use registry::Hooks; +pub use registry::command_from_argv; +pub use types::Hook; +pub use types::HookEvent; +pub use types::HookEventAfterAgent; +pub use types::HookOutcome; +pub use types::HookPayload; +pub use user_notification::legacy_notify_json; +pub use user_notification::notify_hook; diff --git a/codex-rs/core/src/hooks/registry.rs b/codex-rs/hooks/src/registry.rs similarity index 82% rename from codex-rs/core/src/hooks/registry.rs rename to codex-rs/hooks/src/registry.rs index 6bccee85c..0b91eea4e 100644 --- a/codex-rs/core/src/hooks/registry.rs +++ b/codex-rs/hooks/src/registry.rs @@ -1,33 +1,24 @@ use tokio::process::Command; -use super::types::Hook; -use super::types::HookEvent; -use super::types::HookOutcome; -use super::types::HookPayload; -use super::user_notification::notify_hook; -use crate::config::Config; +use crate::types::Hook; +use crate::types::HookEvent; +use crate::types::HookOutcome; +use crate::types::HookPayload; #[derive(Default, Clone)] -pub(crate) struct Hooks { +pub struct Hooks { after_agent: Vec, } -fn get_notify_hook(config: &Config) -> Option { - config - .notify - .as_ref() - .filter(|argv| !argv.is_empty() && !argv[0].is_empty()) - .map(|argv| notify_hook(argv.clone())) -} - // Hooks are arbitrary, user-specified functions that are deterministically // executed after specific events in the Codex lifecycle. impl Hooks { - // new creates a new Hooks instance from config. - // For legacy compatibility, if config.notify is set, it will be added to - // the after_agent hooks. - pub(crate) fn new(config: &Config) -> Self { - let after_agent = get_notify_hook(config).into_iter().collect(); + pub fn new(notify: Option>) -> Self { + let after_agent = notify + .filter(|argv| !argv.is_empty() && !argv[0].is_empty()) + .map(crate::notify_hook) + .into_iter() + .collect(); Self { after_agent } } @@ -37,7 +28,7 @@ impl Hooks { } } - pub(crate) async fn dispatch(&self, hook_payload: HookPayload) { + pub async fn dispatch(&self, hook_payload: HookPayload) { // TODO(gt): support interrupting program execution by returning a result here. for hook in self.hooks_for_event(&hook_payload.hook_event) { let outcome = hook.execute(&hook_payload).await; @@ -48,7 +39,7 @@ impl Hooks { } } -pub(super) fn command_from_argv(argv: &[String]) -> Option { +pub fn command_from_argv(argv: &[String]) -> Option { let (program, args) = argv.split_first()?; if program.is_empty() { return None; @@ -77,16 +68,8 @@ mod tests { use tempfile::tempdir; use tokio::time::timeout; - use crate::config::test_config; - - use super::super::types::Hook; - use super::super::types::HookEvent; - use super::super::types::HookEventAfterAgent; - use super::super::types::HookOutcome; - use super::super::types::HookPayload; - use super::Hooks; - use super::command_from_argv; - use super::get_notify_hook; + use super::*; + use crate::types::HookEventAfterAgent; const CWD: &str = "/tmp"; const INPUT_MESSAGE: &str = "hello"; @@ -154,17 +137,20 @@ mod tests { } #[test] - fn get_notify_hook_requires_program_name() { - let mut config = test_config(); - - config.notify = Some(vec![]); - assert!(get_notify_hook(&config).is_none()); - - config.notify = Some(vec!["".to_string()]); - assert!(get_notify_hook(&config).is_none()); - - config.notify = Some(vec!["notify-send".to_string()]); - assert!(get_notify_hook(&config).is_some()); + fn hooks_new_requires_program_name() { + assert!(Hooks::new(None).after_agent.is_empty()); + assert!(Hooks::new(Some(vec![])).after_agent.is_empty()); + assert!( + Hooks::new(Some(vec!["".to_string()])) + .after_agent + .is_empty() + ); + assert_eq!( + Hooks::new(Some(vec!["notify-send".to_string()])) + .after_agent + .len(), + 1 + ); } #[tokio::test] @@ -270,11 +256,8 @@ mod tests { let script_path_arg = script_path_arg.clone(); Box::pin(async move { let json = to_string(payload).expect("serialize hook payload"); - let powershell = crate::powershell::try_find_powershell_executable_blocking() - .map(|path| path.to_string_lossy().into_owned()) - .unwrap_or_else(|| "powershell.exe".to_string()); let mut command = command_from_argv(&[ - powershell, + "powershell.exe".to_string(), "-NoLogo".to_string(), "-NoProfile".to_string(), "-ExecutionPolicy".to_string(), diff --git a/codex-rs/core/src/hooks/types.rs b/codex-rs/hooks/src/types.rs similarity index 84% rename from codex-rs/core/src/hooks/types.rs rename to codex-rs/hooks/src/types.rs index 3b22d031b..8cb39b5ea 100644 --- a/codex-rs/core/src/hooks/types.rs +++ b/codex-rs/hooks/src/types.rs @@ -9,12 +9,11 @@ use futures::future::BoxFuture; use serde::Serialize; use serde::Serializer; -pub(crate) type HookFn = - Arc Fn(&'a HookPayload) -> BoxFuture<'a, HookOutcome> + Send + Sync>; +pub type HookFn = Arc Fn(&'a HookPayload) -> BoxFuture<'a, HookOutcome> + Send + Sync>; #[derive(Clone)] -pub(crate) struct Hook { - pub(crate) func: HookFn, +pub struct Hook { + pub func: HookFn, } impl Default for Hook { @@ -26,24 +25,24 @@ impl Default for Hook { } impl Hook { - pub(super) async fn execute(&self, payload: &HookPayload) -> HookOutcome { + pub async fn execute(&self, payload: &HookPayload) -> HookOutcome { (self.func)(payload).await } } #[derive(Debug, Serialize, Clone)] #[serde(rename_all = "snake_case")] -pub(crate) struct HookPayload { - pub(crate) session_id: ThreadId, - pub(crate) cwd: PathBuf, +pub struct HookPayload { + pub session_id: ThreadId, + pub cwd: PathBuf, #[serde(serialize_with = "serialize_triggered_at")] - pub(crate) triggered_at: DateTime, - pub(crate) hook_event: HookEvent, + pub triggered_at: DateTime, + pub hook_event: HookEvent, } #[derive(Debug, Clone, Serialize)] #[serde(rename_all = "snake_case")] -pub(crate) struct HookEventAfterAgent { +pub struct HookEventAfterAgent { pub thread_id: ThreadId, pub turn_id: String, pub input_messages: Vec, @@ -59,7 +58,7 @@ where #[derive(Debug, Clone, Serialize)] #[serde(tag = "event_type", rename_all = "snake_case")] -pub(crate) enum HookEvent { +pub enum HookEvent { AfterAgent { #[serde(flatten)] event: HookEventAfterAgent, @@ -67,7 +66,7 @@ pub(crate) enum HookEvent { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum HookOutcome { +pub enum HookOutcome { Continue, #[allow(dead_code)] Stop, diff --git a/codex-rs/core/src/hooks/user_notification.rs b/codex-rs/hooks/src/user_notification.rs similarity index 91% rename from codex-rs/core/src/hooks/user_notification.rs rename to codex-rs/hooks/src/user_notification.rs index de1317f9c..1e33404b5 100644 --- a/codex-rs/core/src/hooks/user_notification.rs +++ b/codex-rs/hooks/src/user_notification.rs @@ -1,14 +1,14 @@ +use std::path::Path; +use std::process::Stdio; use std::sync::Arc; use serde::Serialize; -use std::path::Path; -use std::process::Stdio; -use super::registry::command_from_argv; -use super::types::Hook; -use super::types::HookEvent; -use super::types::HookOutcome; -use super::types::HookPayload; +use crate::Hook; +use crate::HookEvent; +use crate::HookOutcome; +use crate::HookPayload; +use crate::command_from_argv; /// Legacy notify payload appended as the final argv argument for backward compatibility. #[derive(Debug, Clone, PartialEq, Serialize)] @@ -28,10 +28,7 @@ enum UserNotification { }, } -pub(super) fn legacy_notify_json( - hook_event: &HookEvent, - cwd: &Path, -) -> Result { +pub fn legacy_notify_json(hook_event: &HookEvent, cwd: &Path) -> Result { serde_json::to_string(&match hook_event { HookEvent::AfterAgent { event } => UserNotification::AgentTurnComplete { thread_id: event.thread_id.to_string(), @@ -43,7 +40,7 @@ pub(super) fn legacy_notify_json( }) } -pub(super) fn notify_hook(argv: Vec) -> Hook { +pub fn notify_hook(argv: Vec) -> Hook { let argv = Arc::new(argv); Hook { func: Arc::new(move |payload: &HookPayload| { @@ -72,15 +69,14 @@ pub(super) fn notify_hook(argv: Vec) -> Hook { #[cfg(test)] mod tests { - use std::path::Path; - - use super::*; use anyhow::Result; use codex_protocol::ThreadId; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; + use super::*; + fn expected_notification_json() -> Value { json!({ "type": "agent-turn-complete", @@ -112,7 +108,7 @@ mod tests { #[test] fn legacy_notify_json_matches_historical_wire_shape() -> Result<()> { let hook_event = HookEvent::AfterAgent { - event: super::super::types::HookEventAfterAgent { + event: crate::HookEventAfterAgent { thread_id: ThreadId::from_string("b5f6c1c2-1111-2222-3333-444455556666") .expect("valid thread id"), turn_id: "12345".to_string(),