From fa75e638246a785f76ec691d4daea4b008371521 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Wed, 24 Sep 2025 18:13:34 +0100 Subject: [PATCH] core: clear active turn on task abort to avoid stale turn state (#4173) --- codex-rs/core/src/codex.rs | 14 +++++++++----- codex-rs/core/src/lib.rs | 1 - codex-rs/core/src/state/mod.rs | 14 ++------------ .../core/src/{services/mod.rs => state/service.rs} | 11 +++-------- 4 files changed, 14 insertions(+), 26 deletions(-) rename codex-rs/core/src/{services/mod.rs => state/service.rs} (80%) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ff66d0609e..fdc7763959 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -116,9 +116,9 @@ use crate::rollout::RolloutRecorderParams; use crate::safety::SafetyCheck; use crate::safety::assess_command_safety; use crate::safety::assess_safety_for_untrusted_command; -use crate::services::SessionServices; use crate::shell; use crate::state::ActiveTurn; +use crate::state::SessionServices; use crate::turn_diff_tracker::TurnDiffTracker; use crate::unified_exec::UnifiedExecSessionManager; use crate::user_instructions::UserInstructions; @@ -1191,15 +1191,19 @@ impl AgentTask { // TOCTOU? if !self.handle.is_finished() { self.handle.abort(); + let sub_id = self.sub_id.clone(); + let is_review = self.kind == AgentTaskKind::Review; + let sess = self.sess; let event = Event { - id: self.sub_id.clone(), + id: sub_id.clone(), msg: EventMsg::TurnAborted(TurnAbortedEvent { reason }), }; - let sess = self.sess; tokio::spawn(async move { - if self.kind == AgentTaskKind::Review { - exit_review_mode(sess.clone(), self.sub_id, None).await; + if is_review { + exit_review_mode(sess.clone(), sub_id.clone(), None).await; } + // Ensure active turn state is cleared when a task is aborted. + sess.remove_task(&sub_id).await; sess.send_event(event).await; }); } diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 79203f1b37..e4f5709345 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -47,7 +47,6 @@ pub use model_provider_info::create_oss_provider_with_base_url; mod conversation_manager; mod event_mapping; pub mod review_format; -mod services; pub use codex_protocol::protocol::InitialHistory; pub use conversation_manager::ConversationManager; pub use conversation_manager::NewConversation; diff --git a/codex-rs/core/src/state/mod.rs b/codex-rs/core/src/state/mod.rs index 7b1306efdd..927f598146 100644 --- a/codex-rs/core/src/state/mod.rs +++ b/codex-rs/core/src/state/mod.rs @@ -1,18 +1,8 @@ -//! Session/turn state module. -//! -//! Encapsulates all mutable state for a Codex session and the currently active -//! turn. The goal is to present lock-safe, narrow APIs so other modules never -//! need to poke at raw mutexes or internal fields. -//! -//! Locking guidelines -//! - Lock ordering: `SessionState` → `ActiveTurn` → `TurnState`. -//! - Never hold a lock across an `.await`. Extract minimal data and drop the -//! guard before awaiting. -//! - Prefer helper methods on these types rather than exposing fields. - +mod service; mod session; mod turn; +pub(crate) use service::SessionServices; pub(crate) use session::SessionState; pub(crate) use turn::ActiveTurn; pub(crate) use turn::TurnState; diff --git a/codex-rs/core/src/services/mod.rs b/codex-rs/core/src/state/service.rs similarity index 80% rename from codex-rs/core/src/services/mod.rs rename to codex-rs/core/src/state/service.rs index bad425aa12..a67b9dda93 100644 --- a/codex-rs/core/src/services/mod.rs +++ b/codex-rs/core/src/state/service.rs @@ -1,16 +1,11 @@ -//! Group long‑lived helpers/managers for a session. - -use std::path::PathBuf; - -use tokio::sync::Mutex; - +use crate::RolloutRecorder; use crate::exec_command::ExecSessionManager; use crate::mcp_connection_manager::McpConnectionManager; -use crate::rollout::RolloutRecorder; use crate::unified_exec::UnifiedExecSessionManager; use crate::user_notification::UserNotifier; +use std::path::PathBuf; +use tokio::sync::Mutex; -/// Container for side‑effectful services and helpers used by `Session`. pub(crate) struct SessionServices { pub(crate) mcp_connection_manager: McpConnectionManager, pub(crate) session_manager: ExecSessionManager,