mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
codex: add interrupt reasons and shutdown-bounded hooks (#26259)
This commit is contained in:
@@ -230,7 +230,7 @@ Example with notification opt-out:
|
||||
- `externalAgentConfig/import` — apply selected external-agent migration items by passing explicit `migrationItems` with `cwd` (`null` for home) and any plugin/session `details` returned by detect. When a request includes migration items, the server emits `externalAgentConfig/import/completed` once after the full import finishes (immediately after the response when everything completed synchronously, or after background imports finish).
|
||||
- `config/value/write` — write a single config key/value to the user's config.toml on disk; dotted paths such as `desktop.someKey` use the same generic write surface.
|
||||
- `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk, with optional `reloadUserConfig: true` to hot-reload loaded threads, including multiple `desktop.*` edits.
|
||||
- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`, `allowedPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), computer use policy (`computerUse`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`.
|
||||
- `configRequirements/read` — fetch loaded requirements constraints from `requirements.toml` and/or MDM (or `null` if none are configured), including allow-lists (`allowedApprovalPolicies`, `allowedSandboxModes`, `allowedWebSearchModes`, `allowedPermissions`), lifecycle hook lockdown (`allowManagedHooksOnly`), computer use policy (`computerUse`), pinned feature values (`featureRequirements`), managed lifecycle hooks (`hooks`, keyed by canonical event names such as `PreToolUse`, `Stop`, and `Interrupt`), `enforceResidency`, and `network` constraints such as canonical domain/socket permissions plus `managedAllowedDomainsOnly` and `dangerFullAccessDenylistOnly`.
|
||||
|
||||
### Example: Start or resume a thread
|
||||
|
||||
@@ -1593,6 +1593,8 @@ 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.
|
||||
|
||||
The returned `eventName` covers the full discovered hook surface, including `Interrupt` entries alongside the existing tool, compaction, session-start, and stop events.
|
||||
|
||||
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.
|
||||
|
||||
```json
|
||||
@@ -1730,8 +1732,8 @@ approvals_reviewer = "user"
|
||||
```
|
||||
|
||||
Setting the app value to `"user"` routes its approval prompts to the user
|
||||
instead of Guardian; setting it to `"auto_review"` opts that app into Guardian
|
||||
review when allowed by configuration requirements.
|
||||
instead of `auto_review`; setting it to `"auto_review"` opts that app into
|
||||
`auto_review` review when allowed by configuration requirements.
|
||||
|
||||
Invoke an app by inserting `$<app-slug>` in the text input. The slug is derived from the app name and lowercased with non-alphanumeric characters replaced by `-` (for example, "Demo App" becomes `$demo-app`). Add a `mention` input item (recommended) so the server uses the exact `app://<connector-id>` path rather than guessing by name. Plugins use the same `mention` item shape, but with `plugin://<plugin-name>@<marketplace-name>` paths from `plugin/installed` or `plugin/list`.
|
||||
|
||||
|
||||
@@ -234,7 +234,7 @@ async fn record_guardian_denial(session: &Arc<Session>, turn: &Arc<TurnContext>,
|
||||
let turn_id = turn_id.to_string();
|
||||
let _abort_task = runtime_handle.spawn(async move {
|
||||
session
|
||||
.abort_turn_if_active(&turn_id, TurnAbortReason::Interrupted)
|
||||
.abort_turn_if_active_interrupted(&turn_id, codex_hooks::InterruptReason::AutoReview)
|
||||
.await;
|
||||
});
|
||||
}
|
||||
|
||||
@@ -5,6 +5,8 @@ use std::time::Duration;
|
||||
use codex_analytics::CompactionTrigger;
|
||||
use codex_analytics::HookRunFact;
|
||||
use codex_analytics::build_track_events_context;
|
||||
use codex_hooks::CommandExecutionPolicy;
|
||||
use codex_hooks::InterruptReason;
|
||||
use codex_hooks::InterruptRequest;
|
||||
use codex_hooks::PermissionRequestDecision;
|
||||
use codex_hooks::PermissionRequestOutcome;
|
||||
@@ -362,7 +364,11 @@ pub(crate) async fn run_turn_stop_hooks(
|
||||
outcome
|
||||
}
|
||||
|
||||
pub(crate) async fn run_turn_interrupt_hooks(sess: &Arc<Session>, turn_context: &Arc<TurnContext>) {
|
||||
pub(crate) async fn run_turn_interrupt_hooks(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
reason: InterruptReason,
|
||||
) {
|
||||
if matches!(&turn_context.session_source, SessionSource::SubAgent(_)) {
|
||||
return;
|
||||
}
|
||||
@@ -375,11 +381,16 @@ pub(crate) async fn run_turn_interrupt_hooks(sess: &Arc<Session>, turn_context:
|
||||
transcript_path: sess.hook_transcript_path().await,
|
||||
model: turn_context.model_info.slug.clone(),
|
||||
permission_mode: hook_permission_mode(turn_context),
|
||||
reason,
|
||||
};
|
||||
let hooks = sess.hooks();
|
||||
emit_hook_started_events(sess, turn_context, hooks.preview_interrupt(&request)).await;
|
||||
|
||||
let outcome = hooks.run_interrupt(request).await;
|
||||
let execution_policy = match reason {
|
||||
InterruptReason::Shutdown => CommandExecutionPolicy::shutdown_bounded(),
|
||||
InterruptReason::User | InterruptReason::AutoReview => CommandExecutionPolicy::Standard,
|
||||
};
|
||||
let outcome = hooks.run_interrupt(request, execution_policy).await;
|
||||
emit_hook_completed_events(sess, turn_context, outcome.hook_events).await;
|
||||
}
|
||||
|
||||
|
||||
@@ -624,7 +624,8 @@ pub async fn set_thread_memory_mode(sess: &Arc<Session>, sub_id: String, mode: T
|
||||
}
|
||||
|
||||
async fn shutdown_session_runtime(sess: &Arc<Session>) {
|
||||
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
sess.abort_all_tasks_interrupted(codex_hooks::InterruptReason::Shutdown)
|
||||
.await;
|
||||
let _ = sess.conversation.shutdown().await;
|
||||
sess.services
|
||||
.unified_exec_manager
|
||||
|
||||
@@ -3298,7 +3298,8 @@ impl Session {
|
||||
pub async fn interrupt_task(self: &Arc<Self>) {
|
||||
info!("interrupt received: abort current task, if any");
|
||||
let had_active_turn = self.active_turn.lock().await.is_some();
|
||||
self.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
self.abort_all_tasks_interrupted(codex_hooks::InterruptReason::User)
|
||||
.await;
|
||||
if !had_active_turn {
|
||||
self.cancel_mcp_startup().await;
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_hooks::InterruptReason;
|
||||
use futures::future::BoxFuture;
|
||||
use tokio::select;
|
||||
use tokio::sync::Notify;
|
||||
@@ -64,6 +65,35 @@ pub(crate) use user_shell::execute_user_shell_command;
|
||||
const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100;
|
||||
const TASK_COMPACT_METRIC: &str = "codex.task.compact";
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum TaskAbort {
|
||||
Standard(TurnAbortReason),
|
||||
Interrupted(InterruptReason),
|
||||
}
|
||||
|
||||
impl TaskAbort {
|
||||
fn from_turn_abort_reason(reason: TurnAbortReason) -> Self {
|
||||
match reason {
|
||||
TurnAbortReason::Interrupted => Self::Interrupted(InterruptReason::User),
|
||||
other => Self::Standard(other),
|
||||
}
|
||||
}
|
||||
|
||||
fn turn_abort_reason(self) -> TurnAbortReason {
|
||||
match self {
|
||||
Self::Standard(reason) => reason,
|
||||
Self::Interrupted(_) => TurnAbortReason::Interrupted,
|
||||
}
|
||||
}
|
||||
|
||||
fn interrupt_reason(self) -> Option<InterruptReason> {
|
||||
match self {
|
||||
Self::Interrupted(reason) => Some(reason),
|
||||
Self::Standard(_) => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum InterruptedTurnHistoryMarker {
|
||||
Disabled,
|
||||
@@ -485,6 +515,17 @@ impl Session {
|
||||
}
|
||||
|
||||
pub async fn abort_all_tasks(self: &Arc<Self>, reason: TurnAbortReason) {
|
||||
self.abort_all_tasks_with(TaskAbort::from_turn_abort_reason(reason))
|
||||
.await;
|
||||
}
|
||||
|
||||
pub async fn abort_all_tasks_interrupted(self: &Arc<Self>, origin: InterruptReason) {
|
||||
self.abort_all_tasks_with(TaskAbort::Interrupted(origin))
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn abort_all_tasks_with(self: &Arc<Self>, abort: TaskAbort) {
|
||||
let reason = abort.turn_abort_reason();
|
||||
let mut aborted_turn = false;
|
||||
let mut active_turn_to_clear = None;
|
||||
let mut turn_context = None;
|
||||
@@ -493,7 +534,7 @@ impl Session {
|
||||
aborted_turn = task.is_some();
|
||||
turn_context = task.as_ref().map(|task| Arc::clone(&task.turn_context));
|
||||
if let Some(task) = task {
|
||||
self.handle_task_abort(task, reason.clone()).await;
|
||||
self.handle_task_abort(task, abort).await;
|
||||
}
|
||||
if aborted_turn {
|
||||
active_turn_to_clear = Some(active_turn);
|
||||
@@ -518,7 +559,7 @@ impl Session {
|
||||
// in-flight approval wait can surface as a model-visible rejection before TurnAborted.
|
||||
self.input_queue.clear_pending(&active_turn).await;
|
||||
}
|
||||
if reason == TurnAbortReason::Interrupted && aborted_turn {
|
||||
if abort.interrupt_reason().is_some() && aborted_turn {
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
}
|
||||
}
|
||||
@@ -528,6 +569,21 @@ impl Session {
|
||||
turn_id: &str,
|
||||
reason: TurnAbortReason,
|
||||
) -> bool {
|
||||
self.abort_turn_if_active_with(turn_id, TaskAbort::from_turn_abort_reason(reason))
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn abort_turn_if_active_interrupted(
|
||||
self: &Arc<Self>,
|
||||
turn_id: &str,
|
||||
origin: InterruptReason,
|
||||
) -> bool {
|
||||
self.abort_turn_if_active_with(turn_id, TaskAbort::Interrupted(origin))
|
||||
.await
|
||||
}
|
||||
|
||||
async fn abort_turn_if_active_with(self: &Arc<Self>, turn_id: &str, abort: TaskAbort) -> bool {
|
||||
let reason = abort.turn_abort_reason();
|
||||
let active_turn = {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
if active
|
||||
@@ -547,7 +603,7 @@ impl Session {
|
||||
let task = active_turn.task.take();
|
||||
let turn_context = task.as_ref().map(|task| Arc::clone(&task.turn_context));
|
||||
if let Some(task) = task {
|
||||
self.handle_task_abort(task, reason.clone()).await;
|
||||
self.handle_task_abort(task, abort).await;
|
||||
}
|
||||
if let Some(turn_context) = turn_context.as_deref() {
|
||||
self.emit_turn_abort_lifecycle(reason.clone(), turn_context.extension_data.as_ref())
|
||||
@@ -565,7 +621,7 @@ impl Session {
|
||||
// in-flight approval wait can surface as a model-visible rejection before TurnAborted.
|
||||
self.input_queue.clear_pending(&active_turn).await;
|
||||
|
||||
if reason == TurnAbortReason::Interrupted {
|
||||
if abort.interrupt_reason().is_some() {
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
}
|
||||
|
||||
@@ -813,7 +869,8 @@ impl Session {
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn handle_task_abort(self: &Arc<Self>, task: RunningTask, reason: TurnAbortReason) {
|
||||
async fn handle_task_abort(self: &Arc<Self>, task: RunningTask, abort: TaskAbort) {
|
||||
let reason = abort.turn_abort_reason();
|
||||
let sub_id = task.turn_context.sub_id.clone();
|
||||
if task.cancellation_token.is_cancelled() {
|
||||
return;
|
||||
@@ -844,7 +901,7 @@ impl Session {
|
||||
.abort(session_ctx, Arc::clone(&task.turn_context))
|
||||
.await;
|
||||
|
||||
if reason == TurnAbortReason::Interrupted
|
||||
if abort.interrupt_reason().is_some()
|
||||
&& let Some(marker) = interrupted_turn_history_marker(
|
||||
InterruptedTurnHistoryMarker::from_config_and_version(
|
||||
task.turn_context.config.as_ref(),
|
||||
@@ -864,8 +921,8 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
if reason == TurnAbortReason::Interrupted {
|
||||
run_turn_interrupt_hooks(self, &task.turn_context).await;
|
||||
if let Some(interrupt_reason) = abort.interrupt_reason() {
|
||||
run_turn_interrupt_hooks(self, &task.turn_context, interrupt_reason).await;
|
||||
}
|
||||
|
||||
let (completed_at, duration_ms) = task
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
@@ -46,6 +47,14 @@ use tokio::time::sleep;
|
||||
use tokio::time::timeout;
|
||||
|
||||
fn write_interrupt_hook(home: &Path, system_message: Option<&str>) -> Result<()> {
|
||||
write_interrupt_hook_with_sleep(home, system_message, /*sleep_secs*/ 0)
|
||||
}
|
||||
|
||||
fn write_interrupt_hook_with_sleep(
|
||||
home: &Path,
|
||||
system_message: Option<&str>,
|
||||
sleep_secs: u64,
|
||||
) -> Result<()> {
|
||||
let script_path = home.join("interrupt_hook.py");
|
||||
let log_path = home.join("interrupt_hook_log.jsonl");
|
||||
let system_message_json =
|
||||
@@ -54,14 +63,19 @@ fn write_interrupt_hook(home: &Path, system_message: Option<&str>) -> Result<()>
|
||||
r#"import json
|
||||
from pathlib import Path
|
||||
import sys
|
||||
import time
|
||||
|
||||
log_path = Path(r"{log_path}")
|
||||
system_message = json.loads({system_message_json:?})
|
||||
sleep_secs = {sleep_secs}
|
||||
|
||||
payload = json.load(sys.stdin)
|
||||
with log_path.open("a", encoding="utf-8") as handle:
|
||||
handle.write(json.dumps(payload) + "\n")
|
||||
|
||||
if sleep_secs:
|
||||
time.sleep(sleep_secs)
|
||||
|
||||
if system_message is None:
|
||||
print("{{}}")
|
||||
else:
|
||||
@@ -69,6 +83,7 @@ else:
|
||||
"#,
|
||||
log_path = log_path.display(),
|
||||
system_message_json = system_message_json,
|
||||
sleep_secs = sleep_secs,
|
||||
);
|
||||
let hooks = serde_json::json!({
|
||||
"hooks": {
|
||||
@@ -250,6 +265,10 @@ async fn interrupt_hook_runs_before_turn_aborted_and_records_payload() -> Result
|
||||
.and_then(Value::as_str)
|
||||
.is_some_and(|path| !path.is_empty())
|
||||
);
|
||||
assert_eq!(
|
||||
payload.get("reason"),
|
||||
Some(&Value::String("user".to_string()))
|
||||
);
|
||||
assert!(payload.get("stop_hook_active").is_none());
|
||||
assert!(payload.get("last_assistant_message").is_none());
|
||||
|
||||
@@ -531,7 +550,113 @@ async fn guardian_interrupt_runs_interrupt_hook() -> Result<()> {
|
||||
.and_then(Value::as_str)
|
||||
.is_some_and(|path| !path.is_empty())
|
||||
);
|
||||
assert_eq!(
|
||||
payload.get("reason"),
|
||||
Some(&Value::String("auto_review".to_string()))
|
||||
);
|
||||
assert!(payload.get("last_assistant_message").is_none());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn shutdown_interrupt_runs_bounded_interrupt_hook() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let (gate_completed_tx, gate_completed_rx) = oneshot::channel();
|
||||
let chunks = vec![vec![
|
||||
StreamingSseChunk {
|
||||
gate: None,
|
||||
body: sse_event(ev_response_created("resp-shutdown-1")),
|
||||
},
|
||||
StreamingSseChunk {
|
||||
gate: None,
|
||||
body: sse_event(ev_message_item_added("msg-shutdown-1", "")),
|
||||
},
|
||||
StreamingSseChunk {
|
||||
gate: None,
|
||||
body: sse_event(ev_output_text_delta("shore ")),
|
||||
},
|
||||
StreamingSseChunk {
|
||||
gate: None,
|
||||
body: sse_event(ev_message_item_done("msg-shutdown-1", "shore break")),
|
||||
},
|
||||
StreamingSseChunk {
|
||||
gate: Some(gate_completed_rx),
|
||||
body: sse_event(ev_completed("resp-shutdown-1")),
|
||||
},
|
||||
]];
|
||||
let (server, _completions) = start_streaming_sse_server(chunks).await;
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.4")
|
||||
.with_pre_build_hook(|home| {
|
||||
if let Err(error) = write_interrupt_hook_with_sleep(
|
||||
home, /*system_message*/ None, /*sleep_secs*/ 9,
|
||||
) {
|
||||
panic!("failed to write interrupt hook fixture: {error}");
|
||||
}
|
||||
})
|
||||
.with_config(trust_discovered_hooks);
|
||||
let test = builder.build_with_streaming_server(&server).await?;
|
||||
|
||||
submit_text_turn(&test, "shutdown me").await?;
|
||||
|
||||
let _: TurnItem = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::ItemCompleted(ItemCompletedEvent {
|
||||
item: TurnItem::AgentMessage(item),
|
||||
..
|
||||
}) => Some(TurnItem::AgentMessage(item.clone())),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let codex = Arc::clone(&test.codex);
|
||||
let shutdown_task = tokio::spawn(async move { codex.shutdown_and_wait().await });
|
||||
|
||||
let started = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::HookStarted(started) if started.run.event_name == HookEventName::Interrupt => {
|
||||
Some(started.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
let completed = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::HookCompleted(completed)
|
||||
if completed.run.event_name == HookEventName::Interrupt =>
|
||||
{
|
||||
Some(completed.clone())
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
let aborted = wait_for_event_match(&test.codex, |event| match event {
|
||||
EventMsg::TurnAborted(aborted) => Some(aborted.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
let _ = gate_completed_tx.send(());
|
||||
shutdown_task.await??;
|
||||
|
||||
assert_eq!(started.run.event_name, HookEventName::Interrupt);
|
||||
assert_eq!(completed.run.event_name, HookEventName::Interrupt);
|
||||
assert_eq!(completed.run.status, HookRunStatus::Failed);
|
||||
assert_eq!(
|
||||
completed.run.entries,
|
||||
vec![HookOutputEntry {
|
||||
kind: HookOutputEntryKind::Error,
|
||||
text: "hook timed out after 8s".to_string(),
|
||||
}]
|
||||
);
|
||||
assert_eq!(aborted.reason, TurnAbortReason::Interrupted);
|
||||
|
||||
let hook_inputs = read_interrupt_hook_inputs(test.codex_home_path())?;
|
||||
assert_eq!(hook_inputs.len(), 1);
|
||||
assert_eq!(
|
||||
hook_inputs[0].get("reason"),
|
||||
Some(&Value::String("shutdown".to_string()))
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -30,6 +30,14 @@
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
"reason": {
|
||||
"enum": [
|
||||
"user",
|
||||
"shutdown",
|
||||
"auto_review"
|
||||
],
|
||||
"type": "string"
|
||||
},
|
||||
"session_id": {
|
||||
"type": "string"
|
||||
},
|
||||
@@ -46,6 +54,7 @@
|
||||
"hook_event_name",
|
||||
"model",
|
||||
"permission_mode",
|
||||
"reason",
|
||||
"session_id",
|
||||
"transcript_path",
|
||||
"turn_id"
|
||||
|
||||
@@ -10,6 +10,32 @@ use tokio::time::timeout;
|
||||
use super::CommandShell;
|
||||
use super::ConfiguredHandler;
|
||||
|
||||
const SHUTDOWN_BOUNDED_TIMEOUT_SEC: u64 = 8;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum CommandExecutionPolicy {
|
||||
Standard,
|
||||
ShutdownBounded { timeout_cap_sec: u64 },
|
||||
}
|
||||
|
||||
impl CommandExecutionPolicy {
|
||||
pub const fn shutdown_bounded() -> Self {
|
||||
Self::ShutdownBounded {
|
||||
timeout_cap_sec: SHUTDOWN_BOUNDED_TIMEOUT_SEC,
|
||||
}
|
||||
}
|
||||
|
||||
fn effective_timeout_sec(self, configured_timeout_sec: u64) -> u64 {
|
||||
match self {
|
||||
Self::Standard => configured_timeout_sec,
|
||||
Self::ShutdownBounded { timeout_cap_sec } => {
|
||||
configured_timeout_sec.min(timeout_cap_sec)
|
||||
}
|
||||
}
|
||||
.max(1)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct CommandRunResult {
|
||||
pub started_at: i64,
|
||||
@@ -26,6 +52,7 @@ pub(crate) async fn run_command(
|
||||
handler: &ConfiguredHandler,
|
||||
input_json: &str,
|
||||
cwd: &Path,
|
||||
execution_policy: CommandExecutionPolicy,
|
||||
) -> CommandRunResult {
|
||||
let started_at = chrono::Utc::now().timestamp();
|
||||
let started = Instant::now();
|
||||
@@ -68,7 +95,8 @@ pub(crate) async fn run_command(
|
||||
};
|
||||
}
|
||||
|
||||
let timeout_duration = Duration::from_secs(handler.timeout_sec);
|
||||
let effective_timeout_sec = execution_policy.effective_timeout_sec(handler.timeout_sec);
|
||||
let timeout_duration = Duration::from_secs(effective_timeout_sec);
|
||||
match timeout(timeout_duration, child.wait_with_output()).await {
|
||||
Ok(Ok(output)) => CommandRunResult {
|
||||
started_at,
|
||||
@@ -95,7 +123,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 {effective_timeout_sec}s")),
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -133,3 +161,99 @@ fn default_shell_command() -> Command {
|
||||
command
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
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;
|
||||
use tempfile::tempdir;
|
||||
|
||||
use super::CommandExecutionPolicy;
|
||||
use super::CommandShell;
|
||||
use super::ConfiguredHandler;
|
||||
use super::run_command;
|
||||
|
||||
#[test]
|
||||
fn shutdown_bounded_policy_caps_timeout() {
|
||||
assert_eq!(
|
||||
CommandExecutionPolicy::Standard.effective_timeout_sec(600),
|
||||
600
|
||||
);
|
||||
assert_eq!(
|
||||
CommandExecutionPolicy::shutdown_bounded().effective_timeout_sec(600),
|
||||
8
|
||||
);
|
||||
assert_eq!(
|
||||
CommandExecutionPolicy::ShutdownBounded { timeout_cap_sec: 3 }.effective_timeout_sec(2),
|
||||
2
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shutdown_bounded_timeout_uses_capped_timeout() {
|
||||
let temp_dir = tempdir().expect("create temp dir");
|
||||
let started = Instant::now();
|
||||
let result = run_command(
|
||||
&test_shell(),
|
||||
&handler(sleep_command(), 10),
|
||||
"{}",
|
||||
temp_dir.path(),
|
||||
CommandExecutionPolicy::ShutdownBounded { timeout_cap_sec: 1 },
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(started.elapsed() < Duration::from_millis(1500));
|
||||
assert_eq!(result.exit_code, None);
|
||||
assert_eq!(result.error.as_deref(), Some("hook timed out after 1s"));
|
||||
}
|
||||
|
||||
fn handler(command: String, timeout_sec: u64) -> ConfiguredHandler {
|
||||
ConfiguredHandler {
|
||||
event_name: HookEventName::Interrupt,
|
||||
matcher: None,
|
||||
command,
|
||||
timeout_sec,
|
||||
status_message: None,
|
||||
source_path: test_path_buf("/tmp/hooks.json").abs(),
|
||||
source: HookSource::User,
|
||||
display_order: 0,
|
||||
env: HashMap::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn test_shell() -> CommandShell {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
CommandShell {
|
||||
program: "powershell".to_string(),
|
||||
args: vec!["-NoProfile".to_string(), "-Command".to_string()],
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
CommandShell {
|
||||
program: "/bin/sh".to_string(),
|
||||
args: vec!["-lc".to_string()],
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn sleep_command() -> String {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
"Start-Sleep -Seconds 2".to_string()
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
{
|
||||
"sleep 2".to_string()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ use codex_protocol::protocol::HookScope;
|
||||
|
||||
use super::CommandShell;
|
||||
use super::ConfiguredHandler;
|
||||
use super::command_runner::CommandExecutionPolicy;
|
||||
use super::command_runner::CommandRunResult;
|
||||
use super::command_runner::run_command;
|
||||
use crate::events::common::matches_matcher;
|
||||
@@ -52,7 +53,8 @@ pub(crate) fn select_handlers_for_matcher_inputs(
|
||||
| HookEventName::SubagentStart
|
||||
| HookEventName::SubagentStop
|
||||
| HookEventName::PreCompact
|
||||
| HookEventName::PostCompact => {
|
||||
| HookEventName::PostCompact
|
||||
| HookEventName::Interrupt => {
|
||||
if matcher_inputs.is_empty() {
|
||||
matches_matcher(handler.matcher.as_deref(), /*input*/ None)
|
||||
} else {
|
||||
@@ -61,9 +63,7 @@ pub(crate) fn select_handlers_for_matcher_inputs(
|
||||
.any(|input| matches_matcher(handler.matcher.as_deref(), Some(input)))
|
||||
}
|
||||
}
|
||||
HookEventName::UserPromptSubmit | HookEventName::Stop | HookEventName::Interrupt => {
|
||||
true
|
||||
}
|
||||
HookEventName::UserPromptSubmit | HookEventName::Stop => true,
|
||||
})
|
||||
.cloned()
|
||||
.collect()
|
||||
@@ -95,13 +95,34 @@ pub(crate) async fn execute_handlers<T>(
|
||||
cwd: &Path,
|
||||
turn_id: Option<String>,
|
||||
parse: fn(&ConfiguredHandler, CommandRunResult, Option<String>) -> ParsedHandler<T>,
|
||||
) -> Vec<ParsedHandler<T>> {
|
||||
execute_handlers_with_policy(
|
||||
shell,
|
||||
handlers,
|
||||
input_json,
|
||||
cwd,
|
||||
turn_id,
|
||||
CommandExecutionPolicy::Standard,
|
||||
parse,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn execute_handlers_with_policy<T>(
|
||||
shell: &CommandShell,
|
||||
handlers: Vec<ConfiguredHandler>,
|
||||
input_json: String,
|
||||
cwd: &Path,
|
||||
turn_id: Option<String>,
|
||||
execution_policy: CommandExecutionPolicy,
|
||||
parse: fn(&ConfiguredHandler, CommandRunResult, Option<String>) -> ParsedHandler<T>,
|
||||
) -> Vec<ParsedHandler<T>> {
|
||||
let mut pending = FuturesUnordered::new();
|
||||
for (configured_order, handler) in handlers.into_iter().enumerate() {
|
||||
let input_json = input_json.clone();
|
||||
let turn_id = turn_id.clone();
|
||||
pending.push(async move {
|
||||
let result = run_command(shell, &handler, &input_json, cwd).await;
|
||||
let result = run_command(shell, &handler, &input_json, cwd, execution_policy).await;
|
||||
(configured_order, parse(&handler, result, turn_id))
|
||||
});
|
||||
}
|
||||
@@ -211,11 +232,11 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_handlers_ignores_interrupt_matchers() {
|
||||
fn select_handlers_filters_interrupt_matchers() {
|
||||
let handlers = vec![
|
||||
make_handler(
|
||||
HookEventName::Interrupt,
|
||||
Some("^interrupted$"),
|
||||
Some("^auto_review$"),
|
||||
"echo first",
|
||||
/*display_order*/ 0,
|
||||
),
|
||||
@@ -227,17 +248,36 @@ mod tests {
|
||||
),
|
||||
];
|
||||
|
||||
let selected = select_handlers(
|
||||
&handlers,
|
||||
HookEventName::Interrupt,
|
||||
/*matcher_input*/ None,
|
||||
);
|
||||
let selected = select_handlers(&handlers, HookEventName::Interrupt, Some("auto_review"));
|
||||
|
||||
assert_eq!(selected.len(), 2);
|
||||
assert_eq!(selected[0].display_order, 0);
|
||||
assert_eq!(selected[1].display_order, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_handlers_excludes_non_matching_interrupt_handlers() {
|
||||
let handlers = vec![
|
||||
make_handler(
|
||||
HookEventName::Interrupt,
|
||||
Some("^auto_review$"),
|
||||
"echo first",
|
||||
/*display_order*/ 0,
|
||||
),
|
||||
make_handler(
|
||||
HookEventName::Interrupt,
|
||||
Some("^shutdown$"),
|
||||
"echo second",
|
||||
/*display_order*/ 1,
|
||||
),
|
||||
];
|
||||
|
||||
let selected = select_handlers(&handlers, HookEventName::Interrupt, Some("auto_review"));
|
||||
|
||||
assert_eq!(selected.len(), 1);
|
||||
assert_eq!(selected[0].display_order, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_handlers_keeps_overlapping_session_start_matchers() {
|
||||
let handlers = vec![
|
||||
|
||||
@@ -272,8 +272,12 @@ impl ClaudeHooksEngine {
|
||||
crate::events::interrupt::preview(&self.handlers, request)
|
||||
}
|
||||
|
||||
pub(crate) async fn run_interrupt(&self, request: InterruptRequest) -> InterruptOutcome {
|
||||
crate::events::interrupt::run(&self.handlers, &self.shell, request).await
|
||||
pub(crate) async fn run_interrupt(
|
||||
&self,
|
||||
request: InterruptRequest,
|
||||
execution_policy: command_runner::CommandExecutionPolicy,
|
||||
) -> InterruptOutcome {
|
||||
crate::events::interrupt::run(&self.handlers, &self.shell, request, execution_policy).await
|
||||
}
|
||||
|
||||
async fn maybe_spill_texts(&self, session_id: ThreadId, texts: Vec<String>) -> Vec<String> {
|
||||
|
||||
@@ -114,8 +114,9 @@ pub(crate) fn matcher_pattern_for_event(
|
||||
| HookEventName::SubagentStart
|
||||
| HookEventName::SubagentStop
|
||||
| HookEventName::PreCompact
|
||||
| HookEventName::PostCompact => matcher,
|
||||
HookEventName::UserPromptSubmit | HookEventName::Stop | HookEventName::Interrupt => None,
|
||||
| HookEventName::PostCompact
|
||||
| HookEventName::Interrupt => matcher,
|
||||
HookEventName::UserPromptSubmit | HookEventName::Stop => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -262,10 +263,6 @@ mod tests {
|
||||
matcher_pattern_for_event(HookEventName::Stop, Some("^done$")),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
matcher_pattern_for_event(HookEventName::Interrupt, Some("^interrupted$")),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -290,5 +287,9 @@ mod tests {
|
||||
matcher_pattern_for_event(HookEventName::PostCompact, Some("manual|auto")),
|
||||
Some("manual|auto")
|
||||
);
|
||||
assert_eq!(
|
||||
matcher_pattern_for_event(HookEventName::Interrupt, Some("user|shutdown")),
|
||||
Some("user|shutdown")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,12 +12,30 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use super::common;
|
||||
use crate::engine::CommandShell;
|
||||
use crate::engine::ConfiguredHandler;
|
||||
use crate::engine::command_runner::CommandExecutionPolicy;
|
||||
use crate::engine::command_runner::CommandRunResult;
|
||||
use crate::engine::dispatcher;
|
||||
use crate::engine::output_parser;
|
||||
use crate::schema::InterruptCommandInput;
|
||||
use crate::schema::NullableString;
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum InterruptReason {
|
||||
User,
|
||||
Shutdown,
|
||||
AutoReview,
|
||||
}
|
||||
|
||||
impl InterruptReason {
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::User => "user",
|
||||
Self::Shutdown => "shutdown",
|
||||
Self::AutoReview => "auto_review",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct InterruptRequest {
|
||||
pub session_id: ThreadId,
|
||||
@@ -26,6 +44,7 @@ pub struct InterruptRequest {
|
||||
pub transcript_path: Option<PathBuf>,
|
||||
pub model: String,
|
||||
pub permission_mode: String,
|
||||
pub reason: InterruptReason,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
@@ -38,12 +57,12 @@ struct InterruptHandlerData;
|
||||
|
||||
pub(crate) fn preview(
|
||||
handlers: &[ConfiguredHandler],
|
||||
_request: &InterruptRequest,
|
||||
request: &InterruptRequest,
|
||||
) -> Vec<HookRunSummary> {
|
||||
dispatcher::select_handlers(
|
||||
handlers,
|
||||
HookEventName::Interrupt,
|
||||
/*matcher_input*/ None,
|
||||
Some(request.reason.as_str()),
|
||||
)
|
||||
.into_iter()
|
||||
.map(|handler| dispatcher::running_summary(&handler))
|
||||
@@ -54,11 +73,12 @@ pub(crate) async fn run(
|
||||
handlers: &[ConfiguredHandler],
|
||||
shell: &CommandShell,
|
||||
request: InterruptRequest,
|
||||
execution_policy: CommandExecutionPolicy,
|
||||
) -> InterruptOutcome {
|
||||
let matched = dispatcher::select_handlers(
|
||||
handlers,
|
||||
HookEventName::Interrupt,
|
||||
/*matcher_input*/ None,
|
||||
Some(request.reason.as_str()),
|
||||
);
|
||||
if matched.is_empty() {
|
||||
return InterruptOutcome::default();
|
||||
@@ -71,6 +91,7 @@ pub(crate) async fn run(
|
||||
transcript_path,
|
||||
model,
|
||||
permission_mode,
|
||||
reason,
|
||||
} = request;
|
||||
let input_json = match serde_json::to_string(&InterruptCommandInput {
|
||||
session_id: session_id.to_string(),
|
||||
@@ -80,6 +101,7 @@ pub(crate) async fn run(
|
||||
hook_event_name: "Interrupt".to_string(),
|
||||
model,
|
||||
permission_mode,
|
||||
reason: reason.as_str().to_string(),
|
||||
}) {
|
||||
Ok(input_json) => input_json,
|
||||
Err(error) => {
|
||||
@@ -93,12 +115,13 @@ pub(crate) async fn run(
|
||||
}
|
||||
};
|
||||
|
||||
let results = dispatcher::execute_handlers(
|
||||
let results = dispatcher::execute_handlers_with_policy(
|
||||
shell,
|
||||
matched,
|
||||
input_json,
|
||||
cwd.as_path(),
|
||||
Some(turn_id),
|
||||
execution_policy,
|
||||
parse_completed,
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -14,6 +14,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::command_runner::CommandExecutionPolicy;
|
||||
pub use events::common::SubagentHookContext;
|
||||
/// Hook event names as they appear in hooks JSON and config files.
|
||||
pub const HOOK_EVENT_NAMES: [&str; 11] = [
|
||||
@@ -35,7 +36,7 @@ pub const HOOK_EVENT_NAMES: [&str; 11] = [
|
||||
/// Other events can appear in hooks JSON, but Codex ignores their matcher
|
||||
/// fields because those events do not dispatch against a tool, compaction
|
||||
/// trigger, or session-start source.
|
||||
pub const HOOK_EVENT_NAMES_WITH_MATCHERS: [&str; 8] = [
|
||||
pub const HOOK_EVENT_NAMES_WITH_MATCHERS: [&str; 9] = [
|
||||
"PreToolUse",
|
||||
"PermissionRequest",
|
||||
"PostToolUse",
|
||||
@@ -44,6 +45,7 @@ pub const HOOK_EVENT_NAMES_WITH_MATCHERS: [&str; 8] = [
|
||||
"SessionStart",
|
||||
"SubagentStart",
|
||||
"SubagentStop",
|
||||
"Interrupt",
|
||||
];
|
||||
|
||||
pub use events::compact::PostCompactRequest;
|
||||
@@ -51,6 +53,7 @@ pub use events::compact::PreCompactOutcome;
|
||||
pub use events::compact::PreCompactRequest;
|
||||
pub use events::compact::StatelessHookOutcome;
|
||||
pub use events::interrupt::InterruptOutcome;
|
||||
pub use events::interrupt::InterruptReason;
|
||||
pub use events::interrupt::InterruptRequest;
|
||||
pub use events::permission_request::PermissionRequestDecision;
|
||||
pub use events::permission_request::PermissionRequestOutcome;
|
||||
|
||||
@@ -213,8 +213,12 @@ impl Hooks {
|
||||
self.engine.preview_interrupt(request)
|
||||
}
|
||||
|
||||
pub async fn run_interrupt(&self, request: InterruptRequest) -> InterruptOutcome {
|
||||
self.engine.run_interrupt(request).await
|
||||
pub async fn run_interrupt(
|
||||
&self,
|
||||
request: InterruptRequest,
|
||||
execution_policy: crate::CommandExecutionPolicy,
|
||||
) -> InterruptOutcome {
|
||||
self.engine.run_interrupt(request, execution_policy).await
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -621,6 +621,8 @@ pub(crate) struct InterruptCommandInput {
|
||||
pub model: String,
|
||||
#[schemars(schema_with = "permission_mode_schema")]
|
||||
pub permission_mode: String,
|
||||
#[schemars(schema_with = "interrupt_reason_schema")]
|
||||
pub reason: String,
|
||||
}
|
||||
|
||||
pub fn write_schema_fixtures(schema_root: &Path) -> anyhow::Result<()> {
|
||||
@@ -832,6 +834,10 @@ fn compaction_trigger_schema(_gen: &mut SchemaGenerator) -> Schema {
|
||||
string_enum_schema(&["manual", "auto"])
|
||||
}
|
||||
|
||||
fn interrupt_reason_schema(_gen: &mut SchemaGenerator) -> Schema {
|
||||
string_enum_schema(&["user", "shutdown", "auto_review"])
|
||||
}
|
||||
|
||||
fn string_const_schema(value: &str) -> Schema {
|
||||
let mut schema = SchemaObject {
|
||||
instance_type: Some(InstanceType::String.into()),
|
||||
@@ -1152,6 +1158,16 @@ mod tests {
|
||||
.get("last_assistant_message")
|
||||
.is_none()
|
||||
);
|
||||
assert_eq!(
|
||||
interrupt_input["properties"]["reason"]["enum"],
|
||||
serde_json::json!(["user", "shutdown", "auto_review"])
|
||||
);
|
||||
assert!(
|
||||
interrupt_input["required"]
|
||||
.as_array()
|
||||
.expect("interrupt input required fields")
|
||||
.contains(&Value::String("reason".to_string()))
|
||||
);
|
||||
|
||||
let interrupt_output: Value = serde_json::from_slice(
|
||||
&schema_json::<InterruptCommandOutputWire>()
|
||||
|
||||
Reference in New Issue
Block a user