Skip to content

Add prompt-handler execution to codex-hooks #26267

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
abhinav-oai wants to merge 1 commit into from
Closed

Add prompt-handler execution to codex-hooks #26267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions codex-rs/core/src/session/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4070,6 +4070,7 @@ async fn build_hooks_for_config(
plugin_hook_load_warnings,
shell_program: hook_shell_program,
shell_args: hook_shell_argv,
prompt_hook_runner: None,
})
}

Expand Down
18 changes: 13 additions & 5 deletions codex-rs/hooks/src/engine/command_runner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use tracing::Span;

use super::CommandShell;
use super::ConfiguredHandler;
use super::ConfiguredHandlerKind;
use super::dispatcher::hook_event_name_label;
use super::dispatcher::hook_execution_mode_label;
use super::dispatcher::hook_handler_type_label;
Expand Down Expand Up @@ -42,7 +43,7 @@ pub(crate) struct CommandRunResult {
hook.source = hook_source_label(handler.source),
hook.display_order = handler.display_order,
hook.configured_order = configured_order,
hook.timeout_sec = handler.timeout_sec,
hook.timeout_sec = handler.timeout_sec(),
hook.command_outcome = tracing::field::Empty,
)
)]
Expand Down Expand Up @@ -98,7 +99,7 @@ pub(crate) async fn run_command(
);
}

let timeout_duration = Duration::from_secs(handler.timeout_sec);
let timeout_duration = Duration::from_secs(handler.timeout_sec());
match timeout(timeout_duration, child.wait_with_output()).await {
Ok(Ok(output)) => finish_command_run(
started_at,
Expand Down Expand Up @@ -129,7 +130,7 @@ pub(crate) async fn run_command(
exit_code: None,
stdout: String::new(),
stderr: String::new(),
error: Some(format!("hook timed out after {}s", handler.timeout_sec)),
error: Some(format!("hook timed out after {}s", handler.timeout_sec())),
outcome: "timeout",
},
),
Expand Down Expand Up @@ -162,16 +163,23 @@ fn finish_command_run(
}

fn build_command(shell: &CommandShell, handler: &ConfiguredHandler) -> Command {
let ConfiguredHandlerKind::Command {
command: command_text,
..
} = &handler.kind
else {
panic!("prompt handler cannot run as a command hook");
};
let mut command = if shell.program.is_empty() {
default_shell_command()
} else {
Command::new(&shell.program)
};
if shell.program.is_empty() {
command.arg(&handler.command);
command.arg(command_text);
} else {
command.args(&shell.args);
command.arg(&handler.command);
command.arg(command_text);
}
command.envs(&handler.env);
command
Expand Down
33 changes: 20 additions & 13 deletions codex-rs/hooks/src/engine/discovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@ use serde::Deserialize;
use serde::Serialize;

use super::ConfiguredHandler;
use super::ConfiguredHandlerKind;
use super::HookListEntry;
use crate::config_rules::hook_states_from_stack;
use crate::events::common::matcher_pattern_for_event;
use crate::events::common::validate_matcher_pattern;
use codex_protocol::protocol::HookEventName;
use codex_protocol::protocol::HookHandlerType;
use codex_protocol::protocol::HookSource;
use codex_protocol::protocol::HookTrustStatus;
Expand Down Expand Up @@ -444,7 +446,7 @@ fn append_matcher_groups(
warnings: &mut Vec<String>,
display_order: &mut i64,
source: &HookHandlerSource<'_>,
event_name: codex_protocol::protocol::HookEventName,
event_name: HookEventName,
groups: Vec<MatcherGroup>,
) {
for (group_index, group) in groups.into_iter().enumerate() {
Expand Down Expand Up @@ -494,12 +496,10 @@ fn append_matcher_groups(
r#async,
status_message: status_message.clone(),
};
let current_hash =
command_hook_hash(event_name, matcher, &group, normalized_handler);
let current_hash = hook_hash(event_name, matcher, &group, normalized_handler);
let command = source.env.iter().fold(command, |command, (key, value)| {
command.replace(&format!("${{{key}}}"), value)
});
// TODO(abhinav): replace this positional suffix with a durable hook id.
let key =
crate::hook_key(&source.key_source, event_name, group_index, handler_index);
let state = source.hook_states.get(&key);
Expand Down Expand Up @@ -534,8 +534,10 @@ fn append_matcher_groups(
handlers.push(ConfiguredHandler {
event_name,
matcher: matcher.map(ToOwned::to_owned),
command,
timeout_sec,
kind: ConfiguredHandlerKind::Command {
command,
timeout_sec,
},
status_message,
source_path: source.path.clone(),
source: source.source,
Expand Down Expand Up @@ -567,8 +569,8 @@ struct NormalizedHookIdentity {
group: MatcherGroup,
}

fn command_hook_hash(
event_name: codex_protocol::protocol::HookEventName,
fn hook_hash(
event_name: HookEventName,
matcher: Option<&str>,
group: &MatcherGroup,
normalized_handler: HookHandlerConfig,
Expand Down Expand Up @@ -665,6 +667,7 @@ mod tests {
use pretty_assertions::assert_eq;

use super::ConfiguredHandler;
use super::ConfiguredHandlerKind;
use super::append_matcher_groups;
use codex_config::HookHandlerConfig;
use codex_config::HookStateToml;
Expand Down Expand Up @@ -785,8 +788,10 @@ mod tests {
vec![ConfiguredHandler {
event_name: HookEventName::UserPromptSubmit,
matcher: None,
command: "echo hello".to_string(),
timeout_sec: 600,
kind: ConfiguredHandlerKind::Command {
command: "echo hello".to_string(),
timeout_sec: 600,
},
status_message: None,
source_path: source_path.clone(),
source: hook_source(),
Expand Down Expand Up @@ -820,8 +825,10 @@ mod tests {
vec![ConfiguredHandler {
event_name: HookEventName::PreToolUse,
matcher: Some("^Bash$".to_string()),
command: "echo hello".to_string(),
timeout_sec: 600,
kind: ConfiguredHandlerKind::Command {
command: "echo hello".to_string(),
timeout_sec: 600,
},
status_message: None,
source_path: source_path.clone(),
source: hook_source(),
Expand Down Expand Up @@ -1007,7 +1014,7 @@ mod tests {
assert_eq!(warnings, Vec::<String>::new());
assert_eq!(handlers.len(), 1);
assert_eq!(
handlers[0].command,
handlers[0].command().expect("command handler"),
if cfg!(windows) {
"echo windows"
} else {
Expand Down
49 changes: 38 additions & 11 deletions codex-rs/hooks/src/engine/dispatcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ use codex_protocol::protocol::HookScope;

use super::CommandShell;
use super::ConfiguredHandler;
use super::ConfiguredHandlerKind;
use super::command_runner::CommandRunResult;
use super::command_runner::run_command;
use super::prompt_runner::PromptHookRunner;
use super::prompt_runner::run_prompt;
use crate::events::common::matches_matcher;

#[derive(Debug)]
Expand Down Expand Up @@ -71,7 +74,7 @@ pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary {
HookRunSummary {
id: handler.run_id(),
event_name: handler.event_name,
handler_type: HookHandlerType::Command,
handler_type: handler.handler_type(),
execution_mode: HookExecutionMode::Sync,
scope: scope_for_event(handler.event_name),
source_path: handler.source_path.clone(),
Expand All @@ -86,20 +89,41 @@ pub(crate) fn running_summary(handler: &ConfiguredHandler) -> HookRunSummary {
}
}

pub(crate) struct HandlerExecutionContext<'a> {
pub shell: &'a CommandShell,
pub prompt_runner: Option<&'a PromptHookRunner>,
pub cwd: &'a Path,
pub default_model: String,
pub turn_id: Option<String>,
}

pub(crate) async fn execute_handlers<T>(
shell: &CommandShell,
handlers: Vec<ConfiguredHandler>,
input_json: String,
cwd: &Path,
turn_id: Option<String>,
context: HandlerExecutionContext<'_>,
parse: fn(&ConfiguredHandler, CommandRunResult, Option<String>) -> ParsedHandler<T>,
) -> Vec<ParsedHandler<T>> {
let HandlerExecutionContext {
shell,
prompt_runner,
cwd,
default_model,
turn_id,
} = context;
let mut pending = FuturesUnordered::new();
for (configured_order, handler) in handlers.into_iter().enumerate() {
let input_json = input_json.clone();
let default_model = default_model.clone();
let turn_id = turn_id.clone();
pending.push(async move {
let result = run_command(shell, &handler, configured_order, &input_json, cwd).await;
let result = match &handler.kind {
ConfiguredHandlerKind::Command { .. } => {
run_command(shell, &handler, configured_order, &input_json, cwd).await
}
ConfiguredHandlerKind::Prompt { .. } => {
run_prompt(prompt_runner, &handler, &input_json, default_model).await
}
};
(configured_order, parse(&handler, result, turn_id))
});
}
Expand All @@ -124,7 +148,7 @@ pub(crate) fn completed_summary(
HookRunSummary {
id: handler.run_id(),
event_name: handler.event_name,
handler_type: HookHandlerType::Command,
handler_type: handler.handler_type(),
execution_mode: HookExecutionMode::Sync,
scope: scope_for_event(handler.event_name),
source_path: handler.source_path.clone(),
Expand Down Expand Up @@ -217,6 +241,7 @@ mod tests {
use pretty_assertions::assert_eq;

use super::ConfiguredHandler;
use super::ConfiguredHandlerKind;
use super::select_handlers;
use super::select_handlers_for_matcher_inputs;

Expand All @@ -229,8 +254,10 @@ mod tests {
ConfiguredHandler {
event_name,
matcher: matcher.map(str::to_owned),
command: command.to_string(),
timeout_sec: 5,
kind: ConfiguredHandlerKind::Command {
command: command.to_string(),
timeout_sec: 5,
},
status_message: None,
source_path: test_path_buf("/tmp/hooks.json").abs(),
source: HookSource::User,
Expand Down Expand Up @@ -496,8 +523,8 @@ mod tests {
let selected = select_handlers(&handlers, HookEventName::Stop, /*matcher_input*/ None);

assert_eq!(selected.len(), 3);
assert_eq!(selected[0].command, "first");
assert_eq!(selected[1].command, "second");
assert_eq!(selected[2].command, "third");
assert_eq!(selected[0].command().expect("command handler"), "first");
assert_eq!(selected[1].command().expect("command handler"), "second");
assert_eq!(selected[2].command().expect("command handler"), "third");
}
}
Loading
Loading