Skip to content

Commit b309567

Browse files
authored
Allow hooks to error (#11615)
Allow hooks to return errors. We should do this before introducing more hook types, or we'll have to migrate them all.
1 parent 825a4af commit b309567

File tree

6 files changed

+251
-43
lines changed

6 files changed

+251
-43
lines changed

codex-rs/core/src/codex.rs

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use async_channel::Sender;
4444
use codex_hooks::HookEvent;
4545
use codex_hooks::HookEventAfterAgent;
4646
use codex_hooks::HookPayload;
47+
use codex_hooks::HookResult;
4748
use codex_hooks::Hooks;
4849
use codex_hooks::HooksConfig;
4950
use codex_network_proxy::NetworkProxy;
@@ -4533,7 +4534,8 @@ pub(crate) async fn run_turn(
45334534

45344535
if !needs_follow_up {
45354536
last_agent_message = sampling_request_last_agent_message;
4536-
sess.hooks()
4537+
let hook_outcomes = sess
4538+
.hooks()
45374539
.dispatch(HookPayload {
45384540
session_id: sess.conversation_id,
45394541
cwd: turn_context.cwd.clone(),
@@ -4548,6 +4550,47 @@ pub(crate) async fn run_turn(
45484550
},
45494551
})
45504552
.await;
4553+
4554+
let mut abort_message = None;
4555+
for hook_outcome in hook_outcomes {
4556+
let hook_name = hook_outcome.hook_name;
4557+
match hook_outcome.result {
4558+
HookResult::Success => {}
4559+
HookResult::FailedContinue(error) => {
4560+
warn!(
4561+
turn_id = %turn_context.sub_id,
4562+
hook_name = %hook_name,
4563+
error = %error,
4564+
"after_agent hook failed; continuing"
4565+
);
4566+
}
4567+
HookResult::FailedAbort(error) => {
4568+
let message = format!(
4569+
"after_agent hook '{hook_name}' failed and aborted turn completion: {error}"
4570+
);
4571+
warn!(
4572+
turn_id = %turn_context.sub_id,
4573+
hook_name = %hook_name,
4574+
error = %error,
4575+
"after_agent hook failed; aborting operation"
4576+
);
4577+
if abort_message.is_none() {
4578+
abort_message = Some(message);
4579+
}
4580+
}
4581+
}
4582+
}
4583+
if let Some(message) = abort_message {
4584+
sess.send_event(
4585+
&turn_context,
4586+
EventMsg::Error(ErrorEvent {
4587+
message,
4588+
codex_error_info: None,
4589+
}),
4590+
)
4591+
.await;
4592+
return None;
4593+
}
45514594
break;
45524595
}
45534596
continue;

codex-rs/core/src/tools/registry.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use async_trait::async_trait;
1515
use codex_hooks::HookEvent;
1616
use codex_hooks::HookEventAfterToolUse;
1717
use codex_hooks::HookPayload;
18+
use codex_hooks::HookResult;
1819
use codex_hooks::HookToolInput;
1920
use codex_hooks::HookToolInputLocalShell;
2021
use codex_hooks::HookToolKind;
@@ -172,7 +173,7 @@ impl ToolRegistry {
172173
Ok((preview, success)) => (preview.clone(), *success),
173174
Err(err) => (err.to_string(), false),
174175
};
175-
dispatch_after_tool_use_hook(AfterToolUseHookDispatch {
176+
let hook_abort_error = dispatch_after_tool_use_hook(AfterToolUseHookDispatch {
176177
invocation: &invocation,
177178
output_preview,
178179
success,
@@ -182,6 +183,10 @@ impl ToolRegistry {
182183
})
183184
.await;
184185

186+
if let Some(err) = hook_abort_error {
187+
return Err(err);
188+
}
189+
185190
match result {
186191
Ok(_) => {
187192
let mut guard = output_cell.lock().await;
@@ -339,12 +344,14 @@ struct AfterToolUseHookDispatch<'a> {
339344
mutating: bool,
340345
}
341346

342-
async fn dispatch_after_tool_use_hook(dispatch: AfterToolUseHookDispatch<'_>) {
347+
async fn dispatch_after_tool_use_hook(
348+
dispatch: AfterToolUseHookDispatch<'_>,
349+
) -> Option<FunctionCallError> {
343350
let AfterToolUseHookDispatch { invocation, .. } = dispatch;
344351
let session = invocation.session.as_ref();
345352
let turn = invocation.turn.as_ref();
346353
let tool_input = HookToolInput::from(&invocation.payload);
347-
session
354+
let hook_outcomes = session
348355
.hooks()
349356
.dispatch(HookPayload {
350357
session_id: session.conversation_id,
@@ -373,4 +380,34 @@ async fn dispatch_after_tool_use_hook(dispatch: AfterToolUseHookDispatch<'_>) {
373380
},
374381
})
375382
.await;
383+
384+
for hook_outcome in hook_outcomes {
385+
let hook_name = hook_outcome.hook_name;
386+
match hook_outcome.result {
387+
HookResult::Success => {}
388+
HookResult::FailedContinue(error) => {
389+
warn!(
390+
call_id = %invocation.call_id,
391+
tool_name = %invocation.tool_name,
392+
hook_name = %hook_name,
393+
error = %error,
394+
"after_tool_use hook failed; continuing"
395+
);
396+
}
397+
HookResult::FailedAbort(error) => {
398+
warn!(
399+
call_id = %invocation.call_id,
400+
tool_name = %invocation.tool_name,
401+
hook_name = %hook_name,
402+
error = %error,
403+
"after_tool_use hook failed; aborting operation"
404+
);
405+
return Some(FunctionCallError::Fatal(format!(
406+
"after_tool_use hook '{hook_name}' failed and aborted operation: {error}"
407+
)));
408+
}
409+
}
410+
}
411+
412+
None
376413
}

codex-rs/hooks/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ pub use types::Hook;
99
pub use types::HookEvent;
1010
pub use types::HookEventAfterAgent;
1111
pub use types::HookEventAfterToolUse;
12-
pub use types::HookOutcome;
1312
pub use types::HookPayload;
13+
pub use types::HookResponse;
14+
pub use types::HookResult;
1415
pub use types::HookToolInput;
1516
pub use types::HookToolInputLocalShell;
1617
pub use types::HookToolKind;

0 commit comments

Comments
 (0)