mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
Add hooks for interrupted turns (#40511)
## What changed - Add an `Interrupt` hook event that runs for an active top-level turn before its interrupted abort event is emitted. - Flush the turn transcript before invoking the hook and provide the session, turn, transcript, working directory, model, and permission mode in its input. - Support command and MCP handlers, including asynchronous commands, with a one-second default timeout and a three-second maximum. - Expose the event through hook configuration, managed requirements, app-server notifications, generated schemas, analytics, and the TUI hook views. ## Testing - Cover handler discovery, timeout normalization, output parsing, protocol compatibility, TUI rendering, and interrupt execution ordering. GitOrigin-RevId: 163fa7c098d94ac2775f6d137f8e916f8ea9b6eb
This commit is contained in:
Andrei Eternal
committed by
copyberry
copyberry
parent
9b2ef38f54
commit
cbfd999db7
@@ -17,6 +17,7 @@ use codex_config::RequirementSource;
|
||||
use codex_config::TomlValue;
|
||||
use codex_config::version_for_toml;
|
||||
use codex_plugin::PluginHookSource;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
@@ -25,6 +26,7 @@ use super::ConfiguredHandler;
|
||||
use super::ConfiguredHandlerKind;
|
||||
use super::HookListEntry;
|
||||
use super::HookListEntryHandler;
|
||||
use super::dispatcher::hook_event_name_label;
|
||||
use crate::config_rules::hook_states_from_stack;
|
||||
use crate::events::common::matcher_pattern_for_event;
|
||||
use crate::events::common::validate_matcher_pattern;
|
||||
@@ -525,11 +527,11 @@ fn append_matcher_groups(
|
||||
source.path.as_path(),
|
||||
warnings,
|
||||
);
|
||||
let runs_async = r#async
|
||||
&& event_name != codex_protocol::protocol::HookEventName::SessionEnd;
|
||||
let runs_async = r#async && event_name != HookEventName::SessionEnd;
|
||||
if r#async && !runs_async {
|
||||
warnings.push(format!(
|
||||
"running async SessionEnd hook synchronously in {}",
|
||||
"running async {} hook synchronously in {}",
|
||||
hook_event_name_label(event_name),
|
||||
source.path.display()
|
||||
));
|
||||
}
|
||||
@@ -583,11 +585,12 @@ fn append_matcher_groups(
|
||||
timeout_sec,
|
||||
status_message,
|
||||
} => {
|
||||
if event_name == codex_protocol::protocol::HookEventName::SessionEnd {
|
||||
if event_name == HookEventName::SessionEnd {
|
||||
source.record_load_failure(
|
||||
format!(
|
||||
"skipping MCP tool hook in {}: SessionEnd MCP hooks are not supported",
|
||||
source.path.display()
|
||||
"skipping MCP tool hook in {}: {} MCP hooks are not supported",
|
||||
source.path.display(),
|
||||
hook_event_name_label(event_name),
|
||||
),
|
||||
warnings,
|
||||
);
|
||||
@@ -603,7 +606,12 @@ fn append_matcher_groups(
|
||||
);
|
||||
continue;
|
||||
}
|
||||
let timeout_sec = timeout_sec.unwrap_or(600).max(1);
|
||||
let timeout_sec = normalize_command_hook(
|
||||
event_name,
|
||||
timeout_sec,
|
||||
source.path.as_path(),
|
||||
warnings,
|
||||
);
|
||||
let config = HookHandlerConfig::McpTool {
|
||||
server: server.clone(),
|
||||
tool: tool.clone(),
|
||||
@@ -716,28 +724,30 @@ fn append_matcher_groups(
|
||||
}
|
||||
}
|
||||
|
||||
/// Normalizes command-hook timeouts. SessionEnd defaults to one second and is capped at three
|
||||
/// seconds; all other command hooks keep the standard ten-minute default.
|
||||
/// Normalizes hook timeouts. SessionEnd and Interrupt default to one second and are capped at three
|
||||
/// seconds; all other hooks keep the standard ten-minute default.
|
||||
fn normalize_command_hook(
|
||||
event_name: codex_protocol::protocol::HookEventName,
|
||||
event_name: HookEventName,
|
||||
timeout_sec: Option<u64>,
|
||||
source_path: &Path,
|
||||
warnings: &mut Vec<String>,
|
||||
) -> u64 {
|
||||
if event_name != codex_protocol::protocol::HookEventName::SessionEnd {
|
||||
return timeout_sec.unwrap_or(600).max(1);
|
||||
match event_name {
|
||||
HookEventName::SessionEnd | HookEventName::Interrupt => {
|
||||
let max_timeout_sec = SESSION_END_MAX_TIMEOUT_SEC;
|
||||
if timeout_sec.is_some_and(|timeout_sec| timeout_sec > max_timeout_sec) {
|
||||
warnings.push(format!(
|
||||
"clamping {} hook timeout to {max_timeout_sec}s in {}",
|
||||
hook_event_name_label(event_name),
|
||||
source_path.display()
|
||||
));
|
||||
}
|
||||
timeout_sec
|
||||
.unwrap_or(SESSION_END_DEFAULT_TIMEOUT_SEC)
|
||||
.clamp(1, max_timeout_sec)
|
||||
}
|
||||
_ => timeout_sec.unwrap_or(600).max(1),
|
||||
}
|
||||
|
||||
let max_timeout_sec = SESSION_END_MAX_TIMEOUT_SEC;
|
||||
if timeout_sec.is_some_and(|timeout_sec| timeout_sec > max_timeout_sec) {
|
||||
warnings.push(format!(
|
||||
"clamping SessionEnd hook timeout to {max_timeout_sec}s in {}",
|
||||
source_path.display()
|
||||
));
|
||||
}
|
||||
timeout_sec
|
||||
.unwrap_or(SESSION_END_DEFAULT_TIMEOUT_SEC)
|
||||
.clamp(1, max_timeout_sec)
|
||||
}
|
||||
|
||||
/// Hash a normalized, config-derived identity instead of source text so equivalent
|
||||
@@ -841,6 +851,7 @@ mod tests {
|
||||
use codex_config::HookEventsToml;
|
||||
use codex_config::RequirementSource;
|
||||
use codex_protocol::protocol::HookEventName;
|
||||
use codex_protocol::protocol::HookExecutionMode;
|
||||
use codex_protocol::protocol::HookSource;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_absolute_path::test_support::PathBufExt;
|
||||
@@ -852,6 +863,7 @@ mod tests {
|
||||
use super::HookListEntry;
|
||||
use super::HookListEntryHandler;
|
||||
use super::append_matcher_groups;
|
||||
use super::normalize_command_hook;
|
||||
use crate::output_spill::AdditionalContextLimit;
|
||||
use crate::output_spill::DEFAULT_HOOK_OUTPUT_TOKEN_LIMIT;
|
||||
use codex_config::HookHandlerConfig;
|
||||
@@ -1053,6 +1065,71 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interrupt_mcp_tool_hooks_are_supported_and_timeout_is_clamped() {
|
||||
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::Interrupt,
|
||||
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,
|
||||
},
|
||||
HookHandlerConfig::McpTool {
|
||||
server: "security".to_string(),
|
||||
tool: "report".to_string(),
|
||||
input: serde_json::Map::new(),
|
||||
timeout_sec: Some(600),
|
||||
status_message: None,
|
||||
},
|
||||
],
|
||||
}],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
handlers
|
||||
.iter()
|
||||
.map(|handler| handler.timeout_sec)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![1, 3]
|
||||
);
|
||||
assert_eq!(
|
||||
entries
|
||||
.iter()
|
||||
.map(|entry| entry.timeout_sec)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![1, 3]
|
||||
);
|
||||
assert!(
|
||||
entries
|
||||
.iter()
|
||||
.all(|entry| matches!(entry.handler, HookListEntryHandler::McpTool { .. }))
|
||||
);
|
||||
assert_eq!(
|
||||
warnings,
|
||||
vec![format!(
|
||||
"clamping Interrupt hook timeout to 3s in {}",
|
||||
source_path.display()
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
fn discover_command(
|
||||
event_name: HookEventName,
|
||||
additional_context_limit: Option<usize>,
|
||||
@@ -1308,6 +1385,75 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interrupt_normalizes_timeout_and_supports_async_execution() {
|
||||
let mut handlers = Vec::new();
|
||||
let mut hook_entries = Vec::new();
|
||||
let mut warnings = Vec::new();
|
||||
let mut display_order = 0;
|
||||
let source_path = source_path();
|
||||
let hook_states = std::collections::HashMap::new();
|
||||
|
||||
append_matcher_groups(
|
||||
&mut handlers,
|
||||
&mut hook_entries,
|
||||
&mut warnings,
|
||||
&mut display_order,
|
||||
&mut hook_handler_source(&source_path, &hook_states),
|
||||
HookEventName::Interrupt,
|
||||
vec![MatcherGroup {
|
||||
matcher: Some("ignored".to_string()),
|
||||
hooks: vec![HookHandlerConfig::Command {
|
||||
command: "echo interrupt".to_string(),
|
||||
command_windows: None,
|
||||
timeout_sec: Some(600),
|
||||
r#async: true,
|
||||
status_message: None,
|
||||
additional_context_limit: None,
|
||||
}],
|
||||
}],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
normalize_command_hook(
|
||||
HookEventName::Interrupt,
|
||||
/*timeout_sec*/ None,
|
||||
source_path.as_path(),
|
||||
&mut Vec::new(),
|
||||
),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
handlers
|
||||
.iter()
|
||||
.map(|handler| (
|
||||
handler.timeout_sec,
|
||||
handler.matcher.as_deref(),
|
||||
handler.execution_mode()
|
||||
))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![(3, None, HookExecutionMode::Async)]
|
||||
);
|
||||
assert_eq!(
|
||||
hook_entries
|
||||
.iter()
|
||||
.map(|entry| (entry.timeout_sec, entry.matcher.as_deref()))
|
||||
.collect::<Vec<_>>(),
|
||||
vec![(3, None)]
|
||||
);
|
||||
assert!(hook_entries.iter().all(|entry| matches!(
|
||||
entry.handler,
|
||||
HookListEntryHandler::Command { r#async: true, .. }
|
||||
)));
|
||||
assert_eq!(
|
||||
warnings,
|
||||
vec![format!(
|
||||
"clamping Interrupt hook timeout to 3s in {}",
|
||||
source_path.display()
|
||||
)]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bypass_hook_trust_allows_enabled_untrusted_handlers() {
|
||||
let mut handlers = Vec::new();
|
||||
|
||||
Reference in New Issue
Block a user