From 579bb3b0155300cd8223ce73dddc354a9309ddfe Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Fri, 15 May 2026 16:10:26 -0700 Subject: [PATCH] Simplify stop hook parsing --- codex-rs/hooks/src/events/stop.rs | 105 ++++++++++++------------------ 1 file changed, 43 insertions(+), 62 deletions(-) diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index a7cf33e196..6afd36584d 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -78,6 +78,12 @@ struct StopHandlerData { continuation_fragments: Vec, } +#[derive(Debug, Clone, Copy)] +enum StopHookKind { + Stop, + SubagentStop, +} + pub(crate) fn preview( handlers: &[ConfiguredHandler], request: &StopRequest, @@ -113,7 +119,7 @@ pub(crate) async fn run( }; } - let (input_json, parse_completed) = match request.target { + let input_json = match request.target { StopHookTarget::Stop => { let input = StopCommandInput { session_id: request.session_id.to_string(), @@ -129,16 +135,7 @@ pub(crate) async fn run( ), }; match serde_json::to_string(&input) { - Ok(input_json) => ( - input_json, - parse_completed - as fn( - &ConfiguredHandler, - CommandRunResult, - Option, - ) - -> dispatcher::ParsedHandler, - ), + Ok(input_json) => input_json, Err(error) => { return serialization_failure_outcome( common::serialization_failure_hook_events( @@ -172,16 +169,7 @@ pub(crate) async fn run( ), }; match serde_json::to_string(&input) { - Ok(input_json) => ( - input_json, - parse_subagent_stop_completed - as fn( - &ConfiguredHandler, - CommandRunResult, - Option, - ) - -> dispatcher::ParsedHandler, - ), + Ok(input_json) => input_json, Err(error) => { return serialization_failure_outcome( common::serialization_failure_hook_events( @@ -221,39 +209,6 @@ fn parse_completed( handler: &ConfiguredHandler, run_result: CommandRunResult, turn_id: Option, -) -> dispatcher::ParsedHandler { - parse_stop_completed( - handler, - run_result, - turn_id, - "Stop", - "stop", - output_parser::parse_stop, - ) -} - -fn parse_subagent_stop_completed( - handler: &ConfiguredHandler, - run_result: CommandRunResult, - turn_id: Option, -) -> dispatcher::ParsedHandler { - parse_stop_completed( - handler, - run_result, - turn_id, - "SubagentStop", - "subagent stop", - output_parser::parse_subagent_stop, - ) -} - -fn parse_stop_completed( - handler: &ConfiguredHandler, - run_result: CommandRunResult, - turn_id: Option, - hook_name: &str, - hook_label: &str, - parse_output: fn(&str) -> Option, ) -> dispatcher::ParsedHandler { let mut entries = Vec::new(); let mut status = HookRunStatus::Completed; @@ -262,6 +217,13 @@ fn parse_stop_completed( let mut should_block = false; let mut block_reason = None; let mut continuation_prompt = None; + let hook_kind = match handler.event_name { + HookEventName::Stop => StopHookKind::Stop, + HookEventName::SubagentStop => StopHookKind::SubagentStop, + event_name => { + panic!("expected stop hook event, got {event_name:?}"); + } + }; match run_result.error.as_deref() { Some(error) => { @@ -275,7 +237,12 @@ fn parse_stop_completed( Some(0) => { let trimmed_stdout = run_result.stdout.trim(); if trimmed_stdout.is_empty() { - } else if let Some(parsed) = parse_output(&run_result.stdout) { + } else if let Some(parsed) = match hook_kind { + StopHookKind::Stop => output_parser::parse_stop(&run_result.stdout), + StopHookKind::SubagentStop => { + output_parser::parse_subagent_stop(&run_result.stdout) + } + } { if let Some(system_message) = parsed.universal.system_message { entries.push(HookOutputEntry { kind: HookOutputEntryKind::Warning, @@ -315,9 +282,11 @@ fn parse_stop_completed( status = HookRunStatus::Failed; entries.push(HookOutputEntry { kind: HookOutputEntryKind::Error, - text: format!( - "{hook_name} hook returned decision:block without a non-empty reason" - ), + text: match hook_kind { + StopHookKind::Stop => "Stop hook returned decision:block without a non-empty reason", + StopHookKind::SubagentStop => "SubagentStop hook returned decision:block without a non-empty reason", + } + .to_string(), }); } } @@ -325,7 +294,13 @@ fn parse_stop_completed( status = HookRunStatus::Failed; entries.push(HookOutputEntry { kind: HookOutputEntryKind::Error, - text: format!("hook returned invalid {hook_label} hook JSON output"), + text: match hook_kind { + StopHookKind::Stop => "hook returned invalid stop hook JSON output", + StopHookKind::SubagentStop => { + "hook returned invalid subagent stop hook JSON output" + } + } + .to_string(), }); } } @@ -343,9 +318,15 @@ fn parse_stop_completed( status = HookRunStatus::Failed; entries.push(HookOutputEntry { kind: HookOutputEntryKind::Error, - text: format!( - "{hook_name} hook exited with code 2 but did not write a continuation prompt to stderr" - ), + text: match hook_kind { + StopHookKind::Stop => { + "Stop hook exited with code 2 but did not write a continuation prompt to stderr" + } + StopHookKind::SubagentStop => { + "SubagentStop hook exited with code 2 but did not write a continuation prompt to stderr" + } + } + .to_string(), }); } }