mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Extract hooks into dedicated crate (#11311)
Summary - move `core/src/hooks` implementation into a new `codex-hooks` crate with its own manifest - update `codex-rs` workspace and `codex-core` crate to depend on the extracted `hooks` crate and wire up the shared APIs - ensure references, modules, and lockfile reflect the new crate layout Testing - Not run (not requested)
This commit is contained in:
committed by
GitHub
Unverified
parent
1d5eba0090
commit
d735df1f50
Generated
+16
@@ -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"
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
load("//:defs.bzl", "codex_rust_crate")
|
||||
|
||||
codex_rust_crate(
|
||||
name = "hooks",
|
||||
crate_name = "codex_hooks",
|
||||
)
|
||||
@@ -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"] }
|
||||
@@ -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;
|
||||
@@ -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<Hook>,
|
||||
}
|
||||
|
||||
fn get_notify_hook(config: &Config) -> Option<Hook> {
|
||||
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<Vec<String>>) -> 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<Command> {
|
||||
pub fn command_from_argv(argv: &[String]) -> Option<Command> {
|
||||
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(),
|
||||
@@ -9,12 +9,11 @@ use futures::future::BoxFuture;
|
||||
use serde::Serialize;
|
||||
use serde::Serializer;
|
||||
|
||||
pub(crate) type HookFn =
|
||||
Arc<dyn for<'a> Fn(&'a HookPayload) -> BoxFuture<'a, HookOutcome> + Send + Sync>;
|
||||
pub type HookFn = Arc<dyn for<'a> 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<Utc>,
|
||||
pub(crate) hook_event: HookEvent,
|
||||
pub triggered_at: DateTime<Utc>,
|
||||
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<String>,
|
||||
@@ -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,
|
||||
+12
-16
@@ -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<String, serde_json::Error> {
|
||||
pub fn legacy_notify_json(hook_event: &HookEvent, cwd: &Path) -> Result<String, serde_json::Error> {
|
||||
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<String>) -> Hook {
|
||||
pub fn notify_hook(argv: Vec<String>) -> Hook {
|
||||
let argv = Arc::new(argv);
|
||||
Hook {
|
||||
func: Arc::new(move |payload: &HookPayload| {
|
||||
@@ -72,15 +69,14 @@ pub(super) fn notify_hook(argv: Vec<String>) -> 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(),
|
||||
Reference in New Issue
Block a user