From 97c4989046db07321a822fa70ba562d6592ab133 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Tue, 16 Jun 2026 11:35:22 -0700 Subject: [PATCH] Simplify StopFailure hook tests --- codex-rs/core/tests/suite/hooks.rs | 214 ------------------ codex-rs/core/tests/suite/mod.rs | 2 + .../core/tests/suite/stop_failure_hooks.rs | 183 +++++++++++++++ 3 files changed, 185 insertions(+), 214 deletions(-) create mode 100644 codex-rs/core/tests/suite/stop_failure_hooks.rs diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 5eb3e4a1ea..f0ad29fd68 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -37,7 +37,6 @@ 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::sse_failed; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::skip_if_windows; @@ -60,15 +59,6 @@ const BLOCKED_PROMPT_CONTEXT: &str = "Remember the blocked lighthouse note."; const PERMISSION_REQUEST_HOOK_MATCHER: &str = "^Bash$"; const PERMISSION_REQUEST_ALLOW_REASON: &str = "should not be used for allow"; -fn bundled_default_model_slug() -> String { - codex_core::test_support::all_model_presets() - .iter() - .find(|preset| preset.is_default) - .expect("bundled models should include a default") - .model - .clone() -} - fn restrictive_workspace_write_profile() -> PermissionProfile { PermissionProfile::workspace_write_with( &[], @@ -178,55 +168,6 @@ else: Ok(()) } -fn write_stop_failure_hook(home: &Path, matcher: &str, output: Value) -> Result<()> { - let script_path = home.join("stop_failure_hook.py"); - let log_path = home.join("stop_failure_hook_log.jsonl"); - let output_json = serde_json::to_string(&output).context("serialize StopFailure output")?; - let script = format!( - r#"import json -from pathlib import Path -import sys - -log_path = Path(r"{log_path}") -output = json.loads({output_json:?}) -payload = json.load(sys.stdin) - -with log_path.open("a", encoding="utf-8") as handle: - handle.write(json.dumps(payload) + "\n") - -print(json.dumps(output)) -"#, - log_path = log_path.display(), - output_json = output_json, - ); - let hooks = serde_json::json!({ - "hooks": { - "StopFailure": [{ - "matcher": matcher, - "hooks": [{ - "type": "command", - "command": format!("python3 {}", script_path.display()), - "statusMessage": "choosing a recovery", - }] - }] - } - }); - - fs::write(&script_path, script).context("write StopFailure hook script")?; - fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?; - Ok(()) -} - -fn read_stop_failure_hook_inputs(home: &Path) -> Result> { - let path = home.join("stop_failure_hook_log.jsonl"); - let contents = fs::read_to_string(path).context("read StopFailure hook log")?; - contents - .lines() - .filter(|line| !line.trim().is_empty()) - .map(|line| serde_json::from_str(line).context("parse StopFailure hook input")) - .collect() -} - fn write_parallel_stop_hooks(home: &Path, prompts: &[&str]) -> Result<()> { let hook_entries = prompts .iter() @@ -1124,161 +1065,6 @@ fn request_message_input_texts(body: &[u8], role: &str) -> Vec { .collect() } -#[tokio::test] -async fn stop_failure_hook_can_retry_with_an_explicit_model() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let responses = mount_sse_sequence( - &server, - vec![ - sse_failed("resp-1", "server_is_overloaded", "capacity"), - sse(vec![ - ev_response_created("resp-2"), - ev_assistant_message("msg-2", "recovered"), - ev_completed("resp-2"), - ]), - ], - ) - .await; - let recovery_output = serde_json::json!({ - "hookSpecificOutput": { - "hookEventName": "StopFailure", - "recovery": { - "action": "retry", - "model": { "selector": "id", "id": "gpt-5.4" }, - "reason": "The configured fallback is available." - } - } - }); - - let mut builder = test_codex() - .with_model("gpt-5.2") - .with_pre_build_hook(move |home| { - write_stop_failure_hook(home, "overloaded", recovery_output) - .expect("failed to write StopFailure hook fixture"); - }) - .with_config(trust_discovered_hooks); - let test = builder.build(&server).await?; - - test.submit_turn("please recover").await?; - - let requests = responses.requests(); - assert_eq!(requests.len(), 2); - assert_eq!(requests[0].body_json()["model"], "gpt-5.2"); - assert_eq!(requests[1].body_json()["model"], "gpt-5.4"); - assert!( - requests[1].body_contains_text(""), - "the recovery request should use the ordinary model-switch context" - ); - - let hook_inputs = read_stop_failure_hook_inputs(test.codex_home_path())?; - assert_eq!(hook_inputs.len(), 1); - let input = &hook_inputs[0]; - assert_eq!(input["hook_event_name"], "StopFailure"); - assert_eq!(input["error"], "overloaded"); - assert_eq!(input["model"], "gpt-5.2"); - assert_eq!(input["last_assistant_message"], Value::Null); - assert!( - input["error_details"] - .as_str() - .is_some_and(|details| details.contains("capacity")) - ); - - Ok(()) -} - -#[tokio::test] -async fn stop_failure_hook_can_retry_with_the_catalog_default() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let responses = mount_sse_sequence( - &server, - vec![ - sse_failed("resp-1", "server_is_overloaded", "capacity"), - sse(vec![ - ev_response_created("resp-2"), - ev_assistant_message("msg-2", "recovered"), - ev_completed("resp-2"), - ]), - ], - ) - .await; - let recovery_output = serde_json::json!({ - "hookSpecificOutput": { - "hookEventName": "StopFailure", - "recovery": { - "action": "retry", - "model": { "selector": "catalog_default" } - } - } - }); - - let mut builder = test_codex() - .with_model("gpt-5.2") - .with_pre_build_hook(move |home| { - write_stop_failure_hook(home, "overloaded", recovery_output) - .expect("failed to write StopFailure hook fixture"); - }) - .with_config(trust_discovered_hooks); - let test = builder.build(&server).await?; - - test.submit_turn("use the default fallback").await?; - - let requests = responses.requests(); - assert_eq!(requests.len(), 2); - assert_eq!( - requests[1].body_json()["model"], - bundled_default_model_slug() - ); - - Ok(()) -} - -#[tokio::test] -async fn stop_failure_hook_runs_at_most_once_per_turn() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let responses = mount_sse_sequence( - &server, - vec![ - sse_failed("resp-1", "server_is_overloaded", "first failure"), - sse_failed("resp-2", "server_is_overloaded", "recovery failure"), - ], - ) - .await; - let recovery_output = serde_json::json!({ - "hookSpecificOutput": { - "hookEventName": "StopFailure", - "recovery": { - "action": "retry", - "model": { "selector": "id", "id": "gpt-5.4" } - } - } - }); - - let mut builder = test_codex() - .with_model("gpt-5.2") - .with_pre_build_hook(move |home| { - write_stop_failure_hook(home, "overloaded", recovery_output) - .expect("failed to write StopFailure hook fixture"); - }) - .with_config(trust_discovered_hooks); - let test = builder.build(&server).await?; - - test.submit_turn("only recover once").await?; - - assert_eq!(responses.requests().len(), 2); - assert_eq!( - read_stop_failure_hook_inputs(test.codex_home_path())?.len(), - 1 - ); - - Ok(()) -} - #[tokio::test] async fn stop_hook_can_block_multiple_times_in_same_turn() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 87dc0ffce3..98002444be 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -109,6 +109,8 @@ mod skill_approval; mod skills; mod spawn_agent_description; mod sqlite_state; +#[cfg(not(target_os = "windows"))] +mod stop_failure_hooks; mod stream_error_allows_next_turn; mod stream_no_completed; mod subagent_notifications; diff --git a/codex-rs/core/tests/suite/stop_failure_hooks.rs b/codex-rs/core/tests/suite/stop_failure_hooks.rs new file mode 100644 index 0000000000..9875d401c2 --- /dev/null +++ b/codex-rs/core/tests/suite/stop_failure_hooks.rs @@ -0,0 +1,183 @@ +use std::fs; +use std::path::Path; + +use anyhow::Context; +use anyhow::Result; +use core_test_support::hooks::trust_discovered_hooks; +use core_test_support::responses::ResponsesRequest; +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_sequence; +use core_test_support::responses::sse; +use core_test_support::responses::sse_failed; +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; +use serde_json::Value; + +const INITIAL_MODEL: &str = "gpt-5.2"; +const FALLBACK_MODEL: &str = "gpt-5.4"; + +struct StopFailureRun { + requests: Vec, + hook_inputs: Vec, +} + +fn write_stop_failure_hook(home: &Path, recovery: Value) -> Result<()> { + let script_path = home.join("stop_failure_hook.py"); + let log_path = home.join("stop_failure_hook_log.jsonl"); + let output_json = serde_json::json!({ + "hookSpecificOutput": { + "hookEventName": "StopFailure", + "recovery": recovery, + } + }) + .to_string(); + let script = format!( + r#"import json +from pathlib import Path +import sys + +log_path = Path(r"{log_path}") +payload = json.load(sys.stdin) + +with log_path.open("a", encoding="utf-8") as handle: + handle.write(json.dumps(payload) + "\n") + +print({output_json:?}) +"#, + log_path = log_path.display(), + ); + let hooks = serde_json::json!({ + "hooks": { + "StopFailure": [{ + "matcher": "overloaded", + "hooks": [{ + "type": "command", + "command": format!("python3 {}", script_path.display()), + }] + }] + } + }); + + fs::write(&script_path, script).context("write StopFailure hook script")?; + fs::write(home.join("hooks.json"), hooks.to_string()).context("write hooks.json")?; + Ok(()) +} + +fn read_hook_inputs(home: &Path) -> Result> { + fs::read_to_string(home.join("stop_failure_hook_log.jsonl"))? + .lines() + .filter(|line| !line.trim().is_empty()) + .map(|line| serde_json::from_str(line).context("parse StopFailure hook input")) + .collect() +} + +fn successful_response() -> String { + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-2", "recovered"), + ev_completed("resp-2"), + ]) +} + +async fn run_scenario(recovery: Value, second_response: String) -> Result { + let server = start_mock_server().await; + let responses = mount_sse_sequence( + &server, + vec![ + sse_failed("resp-1", "server_is_overloaded", "capacity"), + second_response, + ], + ) + .await; + let mut builder = test_codex() + .with_model(INITIAL_MODEL) + .with_pre_build_hook(move |home| { + write_stop_failure_hook(home, recovery) + .expect("failed to write StopFailure hook fixture"); + }) + .with_config(trust_discovered_hooks); + let test = builder.build(&server).await?; + + test.submit_turn("recover from overload").await?; + + Ok(StopFailureRun { + requests: responses.requests(), + hook_inputs: read_hook_inputs(test.codex_home_path())?, + }) +} + +#[tokio::test] +async fn retries_with_an_explicit_model() -> Result<()> { + skip_if_no_network!(Ok(())); + + let run = run_scenario( + serde_json::json!({ + "action": "retry", + "model": { "selector": "id", "id": FALLBACK_MODEL }, + }), + successful_response(), + ) + .await?; + + assert_eq!(run.requests.len(), 2); + assert_eq!(run.requests[0].body_json()["model"], INITIAL_MODEL); + assert_eq!(run.requests[1].body_json()["model"], FALLBACK_MODEL); + assert!(run.requests[1].body_contains_text("")); + assert_eq!(run.hook_inputs.len(), 1); + let input = &run.hook_inputs[0]; + assert_eq!(input["hook_event_name"], "StopFailure"); + assert_eq!(input["error"], "overloaded"); + assert_eq!(input["model"], INITIAL_MODEL); + assert_eq!(input["last_assistant_message"], Value::Null); + assert!( + input["error_details"] + .as_str() + .is_some_and(|details| details.contains("capacity")) + ); + Ok(()) +} + +#[tokio::test] +async fn retries_with_the_catalog_default() -> Result<()> { + skip_if_no_network!(Ok(())); + + let run = run_scenario( + serde_json::json!({ + "action": "retry", + "model": { "selector": "catalog_default" }, + }), + successful_response(), + ) + .await?; + + let default_model = codex_core::test_support::all_model_presets() + .iter() + .find(|preset| preset.is_default) + .expect("bundled models should include a default") + .model + .clone(); + assert_eq!(run.requests[1].body_json()["model"], default_model); + Ok(()) +} + +#[tokio::test] +async fn runs_at_most_once_per_turn() -> Result<()> { + skip_if_no_network!(Ok(())); + + let run = run_scenario( + serde_json::json!({ + "action": "retry", + "model": { "selector": "id", "id": FALLBACK_MODEL }, + }), + sse_failed("resp-2", "server_is_overloaded", "recovery failure"), + ) + .await?; + + assert_eq!(run.requests.len(), 2); + assert_eq!(run.hook_inputs.len(), 1); + Ok(()) +}