mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Move hook request plumbing into hook runtime (#23388)
## Why `run_turn` was still hand-building hook payloads and lifecycle events for a couple of hook paths. Most hook call sites already delegate request construction and event emission to `hook_runtime`, which keeps turn orchestration focused on model-flow decisions rather than hook plumbing. This also keeps the legacy `after_agent` message extraction next to the legacy hook dispatch instead of leaving response-item walking in `run_turn`. ## What changed - Added `run_stop_hooks` in `hook_runtime` to build `StopRequest`, emit preview start events, run the hook, and emit completion events. - Added `run_legacy_after_agent_hook` in `hook_runtime` to build and dispatch the legacy `AfterAgent` hook payload, including extracting input messages from response items. - Updated `run_turn` to call the hook runtime helpers and keep only the resulting continuation/block/stop decisions inline. - Removed the repeated pending session-start hook check from the run loop. ## Validation - `cargo test -p codex-core hook_runtime`
This commit is contained in:
committed by
GitHub
Unverified
parent
ef24ef127f
commit
9289b7cea8
@@ -13,6 +13,8 @@ use codex_hooks::PostToolUseRequest;
|
||||
use codex_hooks::PreToolUseOutcome;
|
||||
use codex_hooks::PreToolUseRequest;
|
||||
use codex_hooks::SessionStartOutcome;
|
||||
use codex_hooks::StopOutcome;
|
||||
use codex_hooks::StopRequest;
|
||||
use codex_hooks::UserPromptSubmitOutcome;
|
||||
use codex_hooks::UserPromptSubmitRequest;
|
||||
use codex_otel::HOOK_RUN_DURATION_METRIC;
|
||||
@@ -362,6 +364,97 @@ pub(crate) async fn run_user_prompt_submit_hooks(
|
||||
.await
|
||||
}
|
||||
|
||||
pub(crate) async fn run_stop_hooks(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
stop_hook_active: bool,
|
||||
last_assistant_message: Option<String>,
|
||||
) -> StopOutcome {
|
||||
let request = StopRequest {
|
||||
session_id: sess.session_id().into(),
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
transcript_path: sess.hook_transcript_path().await,
|
||||
model: turn_context.model_info.slug.clone(),
|
||||
permission_mode: hook_permission_mode(turn_context),
|
||||
stop_hook_active,
|
||||
last_assistant_message,
|
||||
};
|
||||
let hooks = sess.hooks();
|
||||
emit_hook_started_events(sess, turn_context, hooks.preview_stop(&request)).await;
|
||||
let mut outcome = hooks.run_stop(request).await;
|
||||
emit_hook_completed_events(sess, turn_context, std::mem::take(&mut outcome.hook_events)).await;
|
||||
outcome
|
||||
}
|
||||
|
||||
pub(crate) async fn run_legacy_after_agent_hook(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
input: &[ResponseItem],
|
||||
last_assistant_message: Option<String>,
|
||||
) -> bool {
|
||||
let mut abort_message = None;
|
||||
let input_messages = input
|
||||
.iter()
|
||||
.filter_map(|item| match parse_turn_item(item) {
|
||||
Some(TurnItem::UserMessage(user_message)) => Some(user_message.message()),
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
let hooks = sess.hooks();
|
||||
for hook_outcome in hooks
|
||||
.dispatch(codex_hooks::HookPayload {
|
||||
session_id: sess.session_id().into(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
client: turn_context.app_server_client_name.clone(),
|
||||
triggered_at: chrono::Utc::now(),
|
||||
hook_event: codex_hooks::HookEvent::AfterAgent {
|
||||
event: codex_hooks::HookEventAfterAgent {
|
||||
thread_id: sess.conversation_id,
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
input_messages,
|
||||
last_assistant_message,
|
||||
},
|
||||
},
|
||||
})
|
||||
.await
|
||||
{
|
||||
let hook_name = hook_outcome.hook_name;
|
||||
let (error, should_abort) = match hook_outcome.result {
|
||||
codex_hooks::HookResult::Success => continue,
|
||||
codex_hooks::HookResult::FailedContinue(error) => (error, false),
|
||||
codex_hooks::HookResult::FailedAbort(error) => (error, true),
|
||||
};
|
||||
let action = if should_abort {
|
||||
"aborting operation"
|
||||
} else {
|
||||
"continuing"
|
||||
};
|
||||
tracing::warn!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
hook_name = %hook_name,
|
||||
error = %error,
|
||||
"after_agent hook failed; {action}"
|
||||
);
|
||||
if should_abort && abort_message.is_none() {
|
||||
abort_message = Some(format!(
|
||||
"after_agent hook '{hook_name}' failed and aborted turn completion: {error}"
|
||||
));
|
||||
}
|
||||
}
|
||||
let Some(message) = abort_message else {
|
||||
return false;
|
||||
};
|
||||
let event = EventMsg::Error(codex_protocol::protocol::ErrorEvent {
|
||||
message,
|
||||
codex_error_info: None,
|
||||
});
|
||||
sess.send_event(turn_context, event).await;
|
||||
true
|
||||
}
|
||||
|
||||
pub(crate) async fn inspect_pending_input(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
|
||||
@@ -19,11 +19,12 @@ use crate::context::ContextualUserFragment;
|
||||
use crate::feedback_tags;
|
||||
use crate::goals::GoalRuntimeEvent;
|
||||
use crate::hook_runtime::PendingInputHookDisposition;
|
||||
use crate::hook_runtime::emit_hook_completed_events;
|
||||
use crate::hook_runtime::inspect_pending_input;
|
||||
use crate::hook_runtime::record_additional_contexts;
|
||||
use crate::hook_runtime::record_pending_input;
|
||||
use crate::hook_runtime::run_legacy_after_agent_hook;
|
||||
use crate::hook_runtime::run_pending_session_start_hooks;
|
||||
use crate::hook_runtime::run_stop_hooks;
|
||||
use crate::hook_runtime::run_user_prompt_submit_hooks;
|
||||
use crate::injection::ToolMentionKind;
|
||||
use crate::injection::app_id_from_path;
|
||||
@@ -35,7 +36,6 @@ use crate::mentions::build_skill_name_counts;
|
||||
use crate::mentions::collect_explicit_app_ids;
|
||||
use crate::mentions::collect_explicit_plugin_mentions;
|
||||
use crate::mentions::collect_tool_mentions_from_messages;
|
||||
use crate::parse_turn_item;
|
||||
use crate::plugins::build_plugin_injections;
|
||||
use crate::session::PreviousTurnSettings;
|
||||
use crate::session::session::Session;
|
||||
@@ -71,10 +71,6 @@ use codex_async_utils::OrCancelExt;
|
||||
use codex_features::Feature;
|
||||
use codex_git_utils::get_git_repo_root;
|
||||
use codex_git_utils::get_git_repo_root_with_fs;
|
||||
use codex_hooks::HookEvent;
|
||||
use codex_hooks::HookEventAfterAgent;
|
||||
use codex_hooks::HookPayload;
|
||||
use codex_hooks::HookResult;
|
||||
use codex_protocol::config_types::AutoCompactTokenLimitScope;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::config_types::ServiceTier;
|
||||
@@ -91,7 +87,6 @@ use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use codex_protocol::protocol::AgentMessageContentDeltaEvent;
|
||||
use codex_protocol::protocol::AgentReasoningSectionBreakEvent;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::CodexErrorInfo;
|
||||
use codex_protocol::protocol::ErrorEvent;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
@@ -247,10 +242,6 @@ pub(crate) async fn run_turn(
|
||||
let mut can_drain_pending_input = input.is_empty();
|
||||
|
||||
loop {
|
||||
if run_pending_session_start_hooks(&sess, &turn_context).await {
|
||||
break;
|
||||
}
|
||||
|
||||
// Note that pending_input would be something like a message the user
|
||||
// submitted through the UI while the model was running. Though the UI
|
||||
// may support this, the model might not.
|
||||
@@ -310,14 +301,6 @@ pub(crate) async fn run_turn(
|
||||
.for_prompt(&turn_context.model_info.input_modalities)
|
||||
};
|
||||
|
||||
let sampling_request_input_messages = sampling_request_input
|
||||
.iter()
|
||||
.filter_map(|item| match parse_turn_item(item) {
|
||||
Some(TurnItem::UserMessage(user_message)) => Some(user_message),
|
||||
_ => None,
|
||||
})
|
||||
.map(|user_message| user_message.message())
|
||||
.collect::<Vec<String>>();
|
||||
let turn_metadata_header = turn_context.turn_metadata_state.current_header_value();
|
||||
match run_sampling_request(
|
||||
Arc::clone(&sess),
|
||||
@@ -326,7 +309,7 @@ pub(crate) async fn run_turn(
|
||||
Arc::clone(&turn_diff_tracker),
|
||||
&mut client_session,
|
||||
turn_metadata_header.as_deref(),
|
||||
sampling_request_input,
|
||||
sampling_request_input.clone(),
|
||||
cancellation_token.child_token(),
|
||||
)
|
||||
.await
|
||||
@@ -401,39 +384,13 @@ pub(crate) async fn run_turn(
|
||||
|
||||
if !needs_follow_up {
|
||||
last_agent_message = sampling_request_last_agent_message;
|
||||
let stop_hook_permission_mode = match turn_context.approval_policy.value() {
|
||||
AskForApproval::Never => "bypassPermissions",
|
||||
AskForApproval::UnlessTrusted
|
||||
| AskForApproval::OnFailure
|
||||
| AskForApproval::OnRequest
|
||||
| AskForApproval::Granular(_) => "default",
|
||||
}
|
||||
.to_string();
|
||||
let stop_request = codex_hooks::StopRequest {
|
||||
session_id: sess.session_id().into(),
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
transcript_path: sess.hook_transcript_path().await,
|
||||
model: turn_context.model_info.slug.clone(),
|
||||
permission_mode: stop_hook_permission_mode,
|
||||
let stop_outcome = run_stop_hooks(
|
||||
&sess,
|
||||
&turn_context,
|
||||
stop_hook_active,
|
||||
last_assistant_message: last_agent_message.clone(),
|
||||
};
|
||||
let hooks = sess.hooks();
|
||||
for run in hooks.preview_stop(&stop_request) {
|
||||
sess.send_event(
|
||||
&turn_context,
|
||||
EventMsg::HookStarted(codex_protocol::protocol::HookStartedEvent {
|
||||
turn_id: Some(turn_context.sub_id.clone()),
|
||||
run,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let stop_outcome = hooks.run_stop(stop_request).await;
|
||||
emit_hook_completed_events(&sess, &turn_context, stop_outcome.hook_events)
|
||||
.await;
|
||||
last_agent_message.clone(),
|
||||
)
|
||||
.await;
|
||||
if stop_outcome.should_block {
|
||||
if let Some(hook_prompt_message) =
|
||||
build_hook_prompt_message(&stop_outcome.continuation_fragments)
|
||||
@@ -458,63 +415,14 @@ pub(crate) async fn run_turn(
|
||||
if stop_outcome.should_stop {
|
||||
break;
|
||||
}
|
||||
let hook_outcomes = sess
|
||||
.hooks()
|
||||
.dispatch(HookPayload {
|
||||
session_id: sess.session_id().into(),
|
||||
#[allow(deprecated)]
|
||||
cwd: turn_context.cwd.clone(),
|
||||
client: turn_context.app_server_client_name.clone(),
|
||||
triggered_at: chrono::Utc::now(),
|
||||
hook_event: HookEvent::AfterAgent {
|
||||
event: HookEventAfterAgent {
|
||||
thread_id: sess.conversation_id,
|
||||
turn_id: turn_context.sub_id.clone(),
|
||||
input_messages: sampling_request_input_messages,
|
||||
last_assistant_message: last_agent_message.clone(),
|
||||
},
|
||||
},
|
||||
})
|
||||
.await;
|
||||
|
||||
let mut abort_message = None;
|
||||
for hook_outcome in hook_outcomes {
|
||||
let hook_name = hook_outcome.hook_name;
|
||||
match hook_outcome.result {
|
||||
HookResult::Success => {}
|
||||
HookResult::FailedContinue(error) => {
|
||||
warn!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
hook_name = %hook_name,
|
||||
error = %error,
|
||||
"after_agent hook failed; continuing"
|
||||
);
|
||||
}
|
||||
HookResult::FailedAbort(error) => {
|
||||
let message = format!(
|
||||
"after_agent hook '{hook_name}' failed and aborted turn completion: {error}"
|
||||
);
|
||||
warn!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
hook_name = %hook_name,
|
||||
error = %error,
|
||||
"after_agent hook failed; aborting operation"
|
||||
);
|
||||
if abort_message.is_none() {
|
||||
abort_message = Some(message);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(message) = abort_message {
|
||||
sess.send_event(
|
||||
&turn_context,
|
||||
EventMsg::Error(ErrorEvent {
|
||||
message,
|
||||
codex_error_info: None,
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
if run_legacy_after_agent_hook(
|
||||
&sess,
|
||||
&turn_context,
|
||||
&sampling_request_input,
|
||||
last_agent_message.clone(),
|
||||
)
|
||||
.await
|
||||
{
|
||||
return None;
|
||||
}
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user