From 9a44a7e499f18eaed5d06aabb5acf9184deb06b8 Mon Sep 17 00:00:00 2001 From: Andrei Eternal Date: Fri, 13 Mar 2026 15:51:19 -0700 Subject: [PATCH] [hooks] stop continuation & stop_hook_active mechanics (#14532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stop hooks now receive `stop_hook_active` and enable stop hooks to loop forever if they'd like to. In the initial hooks PR, we implemented a simpler mechanic that the stop-blocking could only happen once in a row - support stop hook adding a continuation prompt to add a further task - if multiple stop-blocks happen that have continuation prompts, they are concatenated example run: ``` › hey :) • Running SessionStart hook: lighting the observatory SessionStart hook (completed) warning: Hi, I'm a session start hook for wizard-tower (startup). hook context: Startup context: A wimboltine stonpet is an exotic cuisine from hyperspace • Aloha :) Happy to jam with you. What are we building today? • Running Stop hook: updating the guards Stop hook (blocked) warning: Wizard Tower Stop hook continuing conversation feedback: cook the stonpet • Aloha, here’s the hyperspace move for cooking a wimboltine stonpet: 1. Sear the stonpet in a hot pan with moon-oil until the edges shimmer. 2. Add star-lime, black salt, and a little fermented nebula paste. 3. Lower the heat and let it braise for 8 cosmic minutes with a splash of comet broth. 4. Finish with sky herbs and serve over warm asteroid rice. The vibe: crispy outside, tender center, deep interdimensional savor. • Running Stop hook: updating the guards Stop hook (completed) warning: Wizard Tower Stop hook saw a second pass and stayed calm to avoid a loop. ``` .codex/config.toml ``` [features] codex_hooks = true ``` .codex/hooks.json ``` { "hooks": { "SessionStart": [ { "matcher": "startup|resume", "hooks": [ { "type": "command", "command": "/usr/bin/python3 .codex/hooks/session_start_demo.py", "timeoutSec": 10, "statusMessage": "lighting the observatory" } ] } ], "Stop": [ { "hooks": [ { "type": "command", "command": "/usr/bin/python3 .codex/hooks/stop_demo_block.py", "timeoutSec": 10, "statusMessage": "updating the guards" } ] } ] } } ``` .codex/hooks/session_start_demo.py ``` #!/usr/bin/env python3 import json import sys from pathlib import Path def main() -> int: payload = json.load(sys.stdin) cwd = Path(payload.get("cwd", ".")).name or "wizard-tower" source = payload.get("source", "startup") source_label = "resume" if source == "resume" else "startup" source_prefix = ( "Resume context:" if source == "resume" else "Startup context:" ) output = { "systemMessage": ( f"Hi, I'm a session start hook for {cwd} ({source_label})." ), "hookSpecificOutput": { "hookEventName": "SessionStart", "additionalContext": ( f"{source_prefix} A wimboltine stonpet is an exotic cuisine from hyperspace" ), }, } print(json.dumps(output)) return 0 if __name__ == "__main__": raise SystemExit(main()) ``` .codex/hooks/stop_demo_block.py ``` #!/usr/bin/env python3 import json import sys def main() -> int: payload = json.load(sys.stdin) stop_hook_active = payload.get("stop_hook_active", False) last_assistant_message = payload.get("last_assistant_message") or "" char_count = len(last_assistant_message.strip()) if stop_hook_active: system_message = ( "Wizard Tower Stop hook saw a second pass and stayed calm to avoid a loop." ) print(json.dumps({"systemMessage": system_message})) else: system_message = ( f"Wizard Tower Stop hook continuing conversation" ) print(json.dumps({"systemMessage": system_message, "decision": "block", "reason": "cook the stonpet"})) return 0 if __name__ == "__main__": raise SystemExit(main()) ``` --- codex-rs/core/src/codex.rs | 25 +- codex-rs/core/tests/suite/hooks.rs | 271 ++++++++++++++++++ codex-rs/core/tests/suite/mod.rs | 2 + .../generated/stop.command.output.schema.json | 1 + codex-rs/hooks/src/engine/output_parser.rs | 18 +- codex-rs/hooks/src/events/stop.rs | 265 ++++++++++------- codex-rs/hooks/src/schema.rs | 2 + 7 files changed, 467 insertions(+), 117 deletions(-) create mode 100644 codex-rs/core/tests/suite/hooks.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 031e3640f..d5a9ad771 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -5652,7 +5652,6 @@ pub(crate) async fn run_turn( .await; let mut last_agent_message: Option = None; let mut stop_hook_active = false; - let mut pending_stop_hook_message: Option = None; // Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains // many turns, from the perspective of the user, it is a single turn. let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); @@ -5744,14 +5743,11 @@ pub(crate) async fn run_turn( } // Construct the input that we will send to the model. - let mut sampling_request_input: Vec = { + let sampling_request_input: Vec = { sess.clone_history() .await .for_prompt(&turn_context.model_info.input_modalities) }; - if let Some(stop_hook_message) = pending_stop_hook_message.take() { - sampling_request_input.push(DeveloperInstructions::new(stop_hook_message).into()); - } let sampling_request_input_messages = sampling_request_input .iter() @@ -5848,18 +5844,25 @@ pub(crate) async fn run_turn( .await; } if stop_outcome.should_block { - if stop_hook_active { + if let Some(continuation_prompt) = stop_outcome.continuation_prompt.clone() + { + let developer_message: ResponseItem = + DeveloperInstructions::new(continuation_prompt).into(); + sess.record_conversation_items( + &turn_context, + std::slice::from_ref(&developer_message), + ) + .await; + stop_hook_active = true; + continue; + } else { sess.send_event( &turn_context, EventMsg::Warning(WarningEvent { - message: "Stop hook blocked twice in the same turn; ignoring the second block to avoid an infinite loop.".to_string(), + message: "Stop hook requested continuation without a prompt; ignoring the block.".to_string(), }), ) .await; - } else { - stop_hook_active = true; - pending_stop_hook_message = stop_outcome.block_message_for_model; - continue; } } if stop_outcome.should_stop { diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs new file mode 100644 index 000000000..a6c28aed2 --- /dev/null +++ b/codex-rs/core/tests/suite/hooks.rs @@ -0,0 +1,271 @@ +use std::fs; +use std::path::Path; + +use anyhow::Context; +use anyhow::Result; +use codex_core::features::Feature; +use codex_protocol::models::ContentItem; +use codex_protocol::models::ResponseItem; +use codex_protocol::protocol::RolloutItem; +use codex_protocol::protocol::RolloutLine; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; + +const FIRST_CONTINUATION_PROMPT: &str = "Retry with exactly the phrase meow meow meow."; +const SECOND_CONTINUATION_PROMPT: &str = "Now tighten it to just: meow."; + +fn write_stop_hook(home: &Path, block_prompts: &[&str]) -> Result<()> { + let script_path = home.join("stop_hook.py"); + let log_path = home.join("stop_hook_log.jsonl"); + let prompts_json = + serde_json::to_string(block_prompts).context("serialize stop hook prompts for test")?; + let script = format!( + r#"import json +from pathlib import Path +import sys + +log_path = Path(r"{log_path}") +block_prompts = {prompts_json} + +payload = json.load(sys.stdin) +existing = [] +if log_path.exists(): + existing = [line for line in log_path.read_text(encoding="utf-8").splitlines() if line.strip()] + +with log_path.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload) + "\n") + +invocation_index = len(existing) +if invocation_index < len(block_prompts): + print(json.dumps({{"decision": "block", "reason": block_prompts[invocation_index]}})) +else: + print(json.dumps({{"systemMessage": f"stop hook pass {{invocation_index + 1}} complete"}})) +"#, + log_path = log_path.display(), + prompts_json = prompts_json, + ); + let hooks = serde_json::json!({ + "hooks": { + "Stop": [{ + "hooks": [{ + "type": "command", + "command": format!("python3 {}", script_path.display()), + "statusMessage": "running stop hook", + }] + }] + } + }); + + fs::write(&script_path, script).context("write stop hook script")?; + fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?; + Ok(()) +} + +fn rollout_developer_texts(text: &str) -> Result> { + let mut texts = Vec::new(); + for line in text.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() { + continue; + } + let rollout: RolloutLine = serde_json::from_str(trimmed).context("parse rollout line")?; + if let RolloutItem::ResponseItem(ResponseItem::Message { role, content, .. }) = rollout.item + && role == "developer" + { + for item in content { + if let ContentItem::InputText { text } = item { + texts.push(text); + } + } + } + } + Ok(texts) +} + +fn read_stop_hook_inputs(home: &Path) -> Result> { + fs::read_to_string(home.join("stop_hook_log.jsonl")) + .context("read stop hook log")? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str(line).context("parse stop hook log line")) + .collect() +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "draft one"), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-2", "draft two"), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_response_created("resp-3"), + ev_assistant_message("msg-3", "final draft"), + ev_completed("resp-3"), + ]), + ], + ) + .await; + + let mut builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = write_stop_hook( + home, + &[FIRST_CONTINUATION_PROMPT, SECOND_CONTINUATION_PROMPT], + ) { + panic!("failed to write stop hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let test = builder.build(&server).await?; + + test.submit_turn("hello from the sea").await?; + + let requests = responses.requests(); + assert_eq!(requests.len(), 3); + assert!( + requests[1] + .message_input_texts("developer") + .contains(&FIRST_CONTINUATION_PROMPT.to_string()), + "second request should include the first continuation prompt", + ); + assert!( + requests[2] + .message_input_texts("developer") + .contains(&FIRST_CONTINUATION_PROMPT.to_string()), + "third request should retain the first continuation prompt from history", + ); + assert!( + requests[2] + .message_input_texts("developer") + .contains(&SECOND_CONTINUATION_PROMPT.to_string()), + "third request should include the second continuation prompt", + ); + + let hook_inputs = read_stop_hook_inputs(test.codex_home_path())?; + assert_eq!(hook_inputs.len(), 3); + assert_eq!( + hook_inputs + .iter() + .map(|input| input["stop_hook_active"] + .as_bool() + .expect("stop_hook_active bool")) + .collect::>(), + vec![false, true, true], + ); + + let rollout_path = test.codex.rollout_path().expect("rollout path"); + let rollout_text = fs::read_to_string(&rollout_path)?; + let developer_texts = rollout_developer_texts(&rollout_text)?; + assert!( + developer_texts.contains(&FIRST_CONTINUATION_PROMPT.to_string()), + "rollout should persist the first continuation prompt", + ); + assert!( + developer_texts.contains(&SECOND_CONTINUATION_PROMPT.to_string()), + "rollout should persist the second continuation prompt", + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn resumed_thread_keeps_stop_continuation_prompt_in_history() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let initial_responses = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "initial draft"), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-2", "revised draft"), + ev_completed("resp-2"), + ]), + ], + ) + .await; + + let mut initial_builder = test_codex() + .with_pre_build_hook(|home| { + if let Err(error) = write_stop_hook(home, &[FIRST_CONTINUATION_PROMPT]) { + panic!("failed to write stop hook test fixture: {error}"); + } + }) + .with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let initial = initial_builder.build(&server).await?; + let home = initial.home.clone(); + let rollout_path = initial + .session_configured + .rollout_path + .clone() + .expect("rollout path"); + + initial.submit_turn("tell me something").await?; + + assert_eq!(initial_responses.requests().len(), 2); + + let resumed_response = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-3"), + ev_assistant_message("msg-3", "fresh turn after resume"), + ev_completed("resp-3"), + ]), + ) + .await; + + let mut resume_builder = test_codex().with_config(|config| { + config + .features + .enable(Feature::CodexHooks) + .expect("test config should allow feature update"); + }); + let resumed = resume_builder.resume(&server, home, rollout_path).await?; + + resumed.submit_turn("and now continue").await?; + + let resumed_request = resumed_response.single_request(); + assert!( + resumed_request + .message_input_texts("developer") + .contains(&FIRST_CONTINUATION_PROMPT.to_string()), + "resumed request should keep the persisted continuation prompt in history", + ); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 5ec63d952..3ef540365 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -77,6 +77,8 @@ mod exec_policy; mod fork_thread; mod grep_files; mod hierarchical_agents; +#[cfg(not(target_os = "windows"))] +mod hooks; mod image_rollout; mod items; mod js_repl; diff --git a/codex-rs/hooks/schema/generated/stop.command.output.schema.json b/codex-rs/hooks/schema/generated/stop.command.output.schema.json index f09f8763a..89559da46 100644 --- a/codex-rs/hooks/schema/generated/stop.command.output.schema.json +++ b/codex-rs/hooks/schema/generated/stop.command.output.schema.json @@ -24,6 +24,7 @@ }, "reason": { "default": null, + "description": "Claude requires `reason` when `decision` is `block`; we enforce that semantic rule during output parsing rather than in the JSON schema.", "type": "string" }, "stopReason": { diff --git a/codex-rs/hooks/src/engine/output_parser.rs b/codex-rs/hooks/src/engine/output_parser.rs index 6b7908f0e..dd4b3480e 100644 --- a/codex-rs/hooks/src/engine/output_parser.rs +++ b/codex-rs/hooks/src/engine/output_parser.rs @@ -17,6 +17,7 @@ pub(crate) struct StopOutput { pub universal: UniversalOutput, pub should_block: bool, pub reason: Option, + pub invalid_block_reason: Option, } use crate::schema::HookUniversalOutputWire; @@ -37,10 +38,21 @@ pub(crate) fn parse_session_start(stdout: &str) -> Option { pub(crate) fn parse_stop(stdout: &str) -> Option { let wire: StopCommandOutputWire = parse_json(stdout)?; + let should_block = matches!(wire.decision, Some(StopDecisionWire::Block)); + let invalid_block_reason = if should_block + && match wire.reason.as_deref() { + Some(reason) => reason.trim().is_empty(), + None => true, + } { + Some(invalid_block_message()) + } else { + None + }; Some(StopOutput { universal: UniversalOutput::from(wire.universal), - should_block: matches!(wire.decision, Some(StopDecisionWire::Block)), + should_block: should_block && invalid_block_reason.is_none(), reason: wire.reason, + invalid_block_reason, }) } @@ -69,3 +81,7 @@ where } serde_json::from_value(value).ok() } + +fn invalid_block_message() -> String { + "Stop hook returned decision:block without a non-empty reason".to_string() +} diff --git a/codex-rs/hooks/src/events/stop.rs b/codex-rs/hooks/src/events/stop.rs index 1ac4028ad..1d73fb7d7 100644 --- a/codex-rs/hooks/src/events/stop.rs +++ b/codex-rs/hooks/src/events/stop.rs @@ -34,16 +34,16 @@ pub struct StopOutcome { pub stop_reason: Option, pub should_block: bool, pub block_reason: Option, - pub block_message_for_model: Option, + pub continuation_prompt: Option, } -#[derive(Debug, PartialEq, Eq)] +#[derive(Debug, Default, PartialEq, Eq)] struct StopHandlerData { should_stop: bool, stop_reason: Option, should_block: bool, block_reason: Option, - block_message_for_model: Option, + continuation_prompt: Option, } pub(crate) fn preview( @@ -69,7 +69,7 @@ pub(crate) async fn run( stop_reason: None, should_block: false, block_reason: None, - block_message_for_model: None, + continuation_prompt: None, }; } @@ -102,34 +102,15 @@ pub(crate) async fn run( ) .await; - let should_stop = results.iter().any(|result| result.data.should_stop); - let stop_reason = results - .iter() - .find_map(|result| result.data.stop_reason.clone()); - - let should_block = !should_stop && results.iter().any(|result| result.data.should_block); - let block_reason = if should_block { - results - .iter() - .find_map(|result| result.data.block_reason.clone()) - } else { - None - }; - let block_message_for_model = if should_block { - results - .iter() - .find_map(|result| result.data.block_message_for_model.clone()) - } else { - None - }; + let aggregate = aggregate_results(results.iter().map(|result| &result.data)); StopOutcome { hook_events: results.into_iter().map(|result| result.completed).collect(), - should_stop, - stop_reason, - should_block, - block_reason, - block_message_for_model, + should_stop: aggregate.should_stop, + stop_reason: aggregate.stop_reason, + should_block: aggregate.should_block, + block_reason: aggregate.block_reason, + continuation_prompt: aggregate.continuation_prompt, } } @@ -144,7 +125,7 @@ fn parse_completed( let mut stop_reason = None; let mut should_block = false; let mut block_reason = None; - let mut block_message_for_model = None; + let mut continuation_prompt = None; match run_result.error.as_deref() { Some(error) => { @@ -176,12 +157,18 @@ fn parse_completed( text: stop_reason_text, }); } + } else if let Some(invalid_block_reason) = parsed.invalid_block_reason { + status = HookRunStatus::Failed; + entries.push(HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: invalid_block_reason, + }); } else if parsed.should_block { if let Some(reason) = parsed.reason.as_deref().and_then(trimmed_non_empty) { status = HookRunStatus::Blocked; should_block = true; block_reason = Some(reason.clone()); - block_message_for_model = Some(reason.clone()); + continuation_prompt = Some(reason.clone()); entries.push(HookOutputEntry { kind: HookOutputEntryKind::Feedback, text: reason, @@ -190,8 +177,9 @@ fn parse_completed( status = HookRunStatus::Failed; entries.push(HookOutputEntry { kind: HookOutputEntryKind::Error, - text: "hook returned decision \"block\" without a non-empty reason" - .to_string(), + text: + "Stop hook returned decision:block without a non-empty reason" + .to_string(), }); } } @@ -208,7 +196,7 @@ fn parse_completed( status = HookRunStatus::Blocked; should_block = true; block_reason = Some(reason.clone()); - block_message_for_model = Some(reason.clone()); + continuation_prompt = Some(reason.clone()); entries.push(HookOutputEntry { kind: HookOutputEntryKind::Feedback, text: reason, @@ -217,7 +205,9 @@ fn parse_completed( status = HookRunStatus::Failed; entries.push(HookOutputEntry { kind: HookOutputEntryKind::Error, - text: "hook exited with code 2 without stderr feedback".to_string(), + text: + "Stop hook exited with code 2 but did not write a continuation prompt to stderr" + .to_string(), }); } } @@ -250,11 +240,57 @@ fn parse_completed( stop_reason, should_block, block_reason, - block_message_for_model, + continuation_prompt, }, } } +fn aggregate_results<'a>( + results: impl IntoIterator, +) -> StopHandlerData { + let results = results.into_iter().collect::>(); + let should_stop = results.iter().any(|result| result.should_stop); + let stop_reason = results.iter().find_map(|result| result.stop_reason.clone()); + let should_block = !should_stop && results.iter().any(|result| result.should_block); + let block_reason = if should_block { + join_block_text(results.iter().copied(), |result| { + result.block_reason.as_deref() + }) + } else { + None + }; + let continuation_prompt = if should_block { + join_block_text(results.iter().copied(), |result| { + result.continuation_prompt.as_deref() + }) + } else { + None + }; + + StopHandlerData { + should_stop, + stop_reason, + should_block, + block_reason, + continuation_prompt, + } +} + +fn join_block_text<'a>( + results: impl IntoIterator, + select: impl Fn(&'a StopHandlerData) -> Option<&'a str>, +) -> Option { + let parts = results + .into_iter() + .filter_map(select) + .map(str::to_owned) + .collect::>(); + if parts.is_empty() { + return None; + } + Some(parts.join("\n\n")) +} + fn trimmed_non_empty(text: &str) -> Option { let trimmed = text.trim(); if !trimmed.is_empty() { @@ -292,7 +328,7 @@ fn serialization_failure_outcome( stop_reason: None, should_block: false, block_reason: None, - block_message_for_model: None, + continuation_prompt: None, } } @@ -307,10 +343,55 @@ mod tests { use pretty_assertions::assert_eq; use super::StopHandlerData; + use super::aggregate_results; use super::parse_completed; use crate::engine::ConfiguredHandler; use crate::engine::command_runner::CommandRunResult; + #[test] + fn block_decision_with_reason_sets_continuation_prompt() { + let parsed = parse_completed( + &handler(), + run_result( + Some(0), + r#"{"decision":"block","reason":"retry with tests"}"#, + "", + ), + Some("turn-1".to_string()), + ); + + assert_eq!( + parsed.data, + StopHandlerData { + should_stop: false, + stop_reason: None, + should_block: true, + block_reason: Some("retry with tests".to_string()), + continuation_prompt: Some("retry with tests".to_string()), + } + ); + assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); + } + + #[test] + fn block_decision_without_reason_is_invalid() { + let parsed = parse_completed( + &handler(), + run_result(Some(0), r#"{"decision":"block"}"#, ""), + Some("turn-1".to_string()), + ); + + assert_eq!(parsed.data, StopHandlerData::default()); + assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); + assert_eq!( + parsed.completed.run.entries, + vec![HookOutputEntry { + kind: HookOutputEntryKind::Error, + text: "Stop hook returned decision:block without a non-empty reason".to_string(), + }] + ); + } + #[test] fn continue_false_overrides_block_decision() { let parsed = parse_completed( @@ -330,7 +411,7 @@ mod tests { stop_reason: Some("done".to_string()), should_block: false, block_reason: None, - block_message_for_model: None, + continuation_prompt: None, } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Stopped); @@ -351,36 +432,25 @@ mod tests { stop_reason: None, should_block: true, block_reason: Some("retry with tests".to_string()), - block_message_for_model: Some("retry with tests".to_string()), + continuation_prompt: Some("retry with tests".to_string()), } ); assert_eq!(parsed.completed.run.status, HookRunStatus::Blocked); } #[test] - fn block_decision_without_reason_fails_instead_of_blocking() { - let parsed = parse_completed( - &handler(), - run_result(Some(0), r#"{"decision":"block"}"#, ""), - Some("turn-1".to_string()), - ); + fn exit_code_two_without_stderr_does_not_block() { + let parsed = parse_completed(&handler(), run_result(Some(2), "", " "), None); - assert_eq!( - parsed.data, - StopHandlerData { - should_stop: false, - stop_reason: None, - should_block: false, - block_reason: None, - block_message_for_model: None, - } - ); + assert_eq!(parsed.data, StopHandlerData::default()); assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); assert_eq!( parsed.completed.run.entries, vec![HookOutputEntry { kind: HookOutputEntryKind::Error, - text: "hook returned decision \"block\" without a non-empty reason".to_string(), + text: + "Stop hook exited with code 2 but did not write a continuation prompt to stderr" + .to_string(), }] ); } @@ -393,50 +463,13 @@ mod tests { Some("turn-1".to_string()), ); - assert_eq!( - parsed.data, - StopHandlerData { - should_stop: false, - stop_reason: None, - should_block: false, - block_reason: None, - block_message_for_model: None, - } - ); + assert_eq!(parsed.data, StopHandlerData::default()); assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); assert_eq!( parsed.completed.run.entries, vec![HookOutputEntry { kind: HookOutputEntryKind::Error, - text: "hook returned decision \"block\" without a non-empty reason".to_string(), - }] - ); - } - - #[test] - fn exit_code_two_without_stderr_feedback_fails_instead_of_blocking() { - let parsed = parse_completed( - &handler(), - run_result(Some(2), "ignored stdout", " "), - Some("turn-1".to_string()), - ); - - assert_eq!( - parsed.data, - StopHandlerData { - should_stop: false, - stop_reason: None, - should_block: false, - block_reason: None, - block_message_for_model: None, - } - ); - assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); - assert_eq!( - parsed.completed.run.entries, - vec![HookOutputEntry { - kind: HookOutputEntryKind::Error, - text: "hook exited with code 2 without stderr feedback".to_string(), + text: "Stop hook returned decision:block without a non-empty reason".to_string(), }] ); } @@ -449,16 +482,7 @@ mod tests { Some("turn-1".to_string()), ); - assert_eq!( - parsed.data, - StopHandlerData { - should_stop: false, - stop_reason: None, - should_block: false, - block_reason: None, - block_message_for_model: None, - } - ); + assert_eq!(parsed.data, StopHandlerData::default()); assert_eq!(parsed.completed.run.status, HookRunStatus::Failed); assert_eq!( parsed.completed.run.entries, @@ -469,6 +493,37 @@ mod tests { ); } + #[test] + fn aggregate_results_concatenates_blocking_reasons_in_declaration_order() { + let aggregate = aggregate_results([ + &StopHandlerData { + should_stop: false, + stop_reason: None, + should_block: true, + block_reason: Some("first".to_string()), + continuation_prompt: Some("first".to_string()), + }, + &StopHandlerData { + should_stop: false, + stop_reason: None, + should_block: true, + block_reason: Some("second".to_string()), + continuation_prompt: Some("second".to_string()), + }, + ]); + + assert_eq!( + aggregate, + StopHandlerData { + should_stop: false, + stop_reason: None, + should_block: true, + block_reason: Some("first\n\nsecond".to_string()), + continuation_prompt: Some("first\n\nsecond".to_string()), + } + ); + } + fn handler() -> ConfiguredHandler { ConfiguredHandler { event_name: HookEventName::Stop, diff --git a/codex-rs/hooks/src/schema.rs b/codex-rs/hooks/src/schema.rs index 43f6d9403..cb8503489 100644 --- a/codex-rs/hooks/src/schema.rs +++ b/codex-rs/hooks/src/schema.rs @@ -96,6 +96,8 @@ pub(crate) struct StopCommandOutputWire { pub universal: HookUniversalOutputWire, #[serde(default)] pub decision: Option, + /// Claude requires `reason` when `decision` is `block`; we enforce that + /// semantic rule during output parsing rather than in the JSON schema. #[serde(default)] pub reason: Option, }