From bbdf3030dec1e7894cbe58051076ea66d2c9208f Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 11 Jul 2026 17:08:53 +0000 Subject: [PATCH] Emit thread-idle lifecycle after guardian interrupts (#32460) ## What changed - Emit the thread-idle extension lifecycle when the guardian successfully aborts an active turn after repeated automatic review denials. - Keep user-initiated interrupt behavior unchanged. - Add a regression test that waits for the thread-idle callback after a guardian interrupt. GitOrigin-RevId: 6622c35361a3da9126c11bd6ca49a532c1bd6e27 --- codex-rs/core/src/guardian/review.rs | 7 +++- codex-rs/core/src/session/tests.rs | 59 ++++++++++++++-------------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/codex-rs/core/src/guardian/review.rs b/codex-rs/core/src/guardian/review.rs index e606c01d09..35dad0db87 100644 --- a/codex-rs/core/src/guardian/review.rs +++ b/codex-rs/core/src/guardian/review.rs @@ -254,9 +254,14 @@ async fn record_guardian_denial(session: &Arc, turn: &Arc, let session = Arc::clone(session); let turn_id = turn_id.to_string(); let _abort_task = runtime_handle.spawn(async move { - session + let aborted = session .abort_turn_if_active(&turn_id, TurnAbortReason::Interrupted) .await; + if aborted { + // Guardian aborts bypass normal task completion, so emit its idle lifecycle here. + // User interrupts deliberately do not take this path. + session.emit_thread_idle_lifecycle_if_idle().await; + } }); } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b94c2964ae..8d01d264db 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -9413,37 +9413,38 @@ impl SessionTask for GuardianDeniedApprovalTask { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn guardian_auto_review_interrupts_after_three_consecutive_denials() { - let (sess, tc, rx) = make_session_and_context_with_rx().await; - let input = vec![TurnInput::UserInput { - content: vec![UserInput::Text { - text: "trigger guardian denials".to_string(), - text_elements: Vec::new(), - }], - client_id: None, - }]; - sess.spawn_task(Arc::clone(&tc), input, GuardianDeniedApprovalTask) +async fn guardian_auto_review_emits_thread_idle_after_interrupt() { + struct ThreadIdleRecorder(async_channel::Sender<()>); + + impl codex_extension_api::ThreadLifecycleContributor for ThreadIdleRecorder { + fn on_thread_idle<'a>( + &'a self, + _input: codex_extension_api::ThreadIdleInput<'a>, + ) -> codex_extension_api::ExtensionFuture<'a, ()> { + Box::pin(async move { + self.0.send(()).await.expect("idle receiver open"); + }) + } + } + + let (mut session, turn_context) = make_session_and_context().await; + let (idle_tx, idle_rx) = async_channel::bounded(1); + let mut builder = codex_extension_api::ExtensionRegistryBuilder::::new(); + builder.thread_lifecycle_contributor(Arc::new(ThreadIdleRecorder(idle_tx))); + session.services.extensions = Arc::new(builder.build()); + + Arc::new(session) + .spawn_task( + Arc::new(turn_context), + Vec::new(), + GuardianDeniedApprovalTask, + ) .await; - let mut observed = Vec::new(); - let aborted = tokio::time::timeout(std::time::Duration::from_secs(5), async { - loop { - let event = rx.recv().await.expect("event"); - if let EventMsg::TurnAborted(event) = &event.msg { - let event = event.clone(); - observed.push(EventMsg::TurnAborted(event.clone())); - break event; - } - observed.push(event.msg); - } - }) - .await - .unwrap_or_else(|_| { - panic!( - "guardian denial circuit breaker should interrupt the turn; observed events: {observed:?}" - ) - }); - assert_eq!(aborted.reason, TurnAbortReason::Interrupted); + timeout(StdDuration::from_secs(5), idle_rx.recv()) + .await + .expect("guardian interrupt should emit thread idle lifecycle") + .expect("idle receiver open"); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)]