From 8078d50d154590ec9d495d78338ad8cf71431f1c Mon Sep 17 00:00:00 2001 From: Johannes Schickling Date: Sun, 23 Aug 2026 18:27:14 +0200 Subject: [PATCH] fix(codex-app-server): deflake darwin-parallel test races Two lib tests failed intermittently under full-suite parallel load on macOS while passing deterministically in isolation (#315): - process_group_cleanup_reaps_a_native_launcher_descendant: the shell redirection creates the pidfile before the pid bytes land, so reading immediately after is_file() observed an empty file and panicked with ParseIntError { kind: Empty }. Wait for parseable content within a bounded deadline instead of a single read. - subscribed_control_pump_delivers_a_typed_reference_to_the_real_fifo_head: asserted delivery-state == Accepted immediately after joining the pump, but the pump's final websocket frames race the fake server's exit under load, so it could exit before persisting Accepted. Poll for the outcome (bounded), bail early if the pump dies, then join and assert. No production changes. --- src/codex_app_server.rs | 56 ++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/codex_app_server.rs b/src/codex_app_server.rs index 231bd9b4..27cce8ad 100644 --- a/src/codex_app_server.rs +++ b/src/codex_app_server.rs @@ -3301,19 +3301,31 @@ mod tests { )); server.join().unwrap(); let _ = shutdown.shutdown(Shutdown::Both); + // The pump persists Accepted synchronously, but its final websocket + // frames race the fake server's exit under parallel-load + // descheduling. Await the outcome instead of requiring it to be + // settled the instant both threads have been joined. + let delivery_state_path = tmp.path().join("state/delivery-state.json"); + let deadline = Instant::now() + Duration::from_secs(10); + let accepted = loop { + if load_delivery_state(&delivery_state_path, "h.worker", "h.worker") + .unwrap() + .is_some_and(|state| state.phase == CodexDeliveryPhase::Accepted) + { + break true; + } + if pump.is_finished() { + break false; + } + assert!( + Instant::now() < deadline, + "delivery never reached Accepted before shutdown" + ); + std::thread::sleep(Duration::from_millis(20)); + }; pump.join().unwrap(); + assert!(accepted); assert!(delivery_config(tmp.path()).inbox.join(filename).is_file()); - assert_eq!( - load_delivery_state( - &tmp.path().join("state/delivery-state.json"), - "h.worker", - "h.worker", - ) - .unwrap() - .unwrap() - .phase, - CodexDeliveryPhase::Accepted - ); } #[test] @@ -4720,14 +4732,22 @@ mod tests { .stdout(Stdio::null()) .stderr(Stdio::null()); let mut launcher = spawn_process_group(&mut command).unwrap(); - let deadline = Instant::now() + Duration::from_secs(1); - while !descendant_pidfile.is_file() && Instant::now() < deadline { + let deadline = Instant::now() + Duration::from_secs(5); + // The shell redirection creates the pidfile before the pid bytes land, + // so an immediate read after `is_file()` can observe an empty file + // under load. Wait for parseable content instead. + let descendant = loop { + if let Ok(content) = std::fs::read_to_string(&descendant_pidfile) { + if let Ok(pid) = content.trim().parse::() { + break pid; + } + } + assert!( + Instant::now() < deadline, + "the launcher did not write its descendant pid" + ); std::thread::sleep(Duration::from_millis(10)); - } - let descendant = std::fs::read_to_string(&descendant_pidfile) - .expect("the launcher did not create its native descendant") - .parse::() - .unwrap(); + }; assert!( process_can_retain_cleanup_resources(descendant), "the native descendant was not alive before cleanup"