mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Gate automatic idle turns in Plan mode (#26147)
## Why Goal idle continuation is extension-triggered model-visible work, so it should follow one core-owned rule for when automatic work may start. In particular, it should not jump ahead of queued user/client work, start while another task is active, or inject a continuation turn while the thread is in Plan mode. Keeping this policy in `try_start_turn_if_idle` avoids passing `collaboration_mode` or review-specific state through `ThreadLifecycleContributor::on_thread_idle`. Active `/review` is covered by the same active-task gate because Review turns are not steerable. ## What Changed - Teach `Session::try_start_turn_if_idle` to reject automatic idle turns in Plan mode, both before reserving an idle turn and after building the turn context. - Document `CodexThread::try_start_turn_if_idle` as the extension-facing gate for automatic idle work, including Plan-mode and active Review-task behavior. - Add focused coverage for Plan-mode rejection and active Review-task rejection without queuing synthetic input. ## Testing - `just test -p codex-core try_start_turn_if_idle`
This commit is contained in:
@@ -74,6 +74,46 @@ pub struct ThreadConfigSnapshot {
|
||||
pub thread_source: Option<ThreadSource>,
|
||||
}
|
||||
|
||||
/// Explains why `CodexThread::try_start_turn_if_idle` rejected an automatic
|
||||
/// idle turn.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum TryStartTurnIfIdleRejectionReason {
|
||||
/// User/client-triggered mailbox work is already queued and must take
|
||||
/// priority over extension-initiated idle work.
|
||||
PendingTriggerTurn,
|
||||
/// The thread is in Plan mode, where automatic idle work must not start a
|
||||
/// new model turn.
|
||||
PlanMode,
|
||||
/// Another turn or task is active, or the idle reservation was lost before
|
||||
/// the automatic turn could start.
|
||||
Busy,
|
||||
}
|
||||
|
||||
/// Rejection returned when an extension asks to start automatic idle work but
|
||||
/// the thread is not eligible to run it.
|
||||
#[derive(Debug)]
|
||||
pub struct TryStartTurnIfIdleError {
|
||||
reason: TryStartTurnIfIdleRejectionReason,
|
||||
input: Vec<ResponseItem>,
|
||||
}
|
||||
|
||||
impl TryStartTurnIfIdleError {
|
||||
pub(crate) fn new(reason: TryStartTurnIfIdleRejectionReason, input: Vec<ResponseItem>) -> Self {
|
||||
Self { reason, input }
|
||||
}
|
||||
|
||||
/// Returns the stable reason the automatic idle turn was rejected.
|
||||
pub fn reason(&self) -> TryStartTurnIfIdleRejectionReason {
|
||||
self.reason
|
||||
}
|
||||
|
||||
/// Consumes the rejection and returns the original model-visible input
|
||||
/// unchanged, so callers can retry, drop, or log it explicitly.
|
||||
pub fn into_input(self) -> Vec<ResponseItem> {
|
||||
self.input
|
||||
}
|
||||
}
|
||||
|
||||
impl ThreadConfigSnapshot {
|
||||
pub fn sandbox_policy(&self) -> SandboxPolicy {
|
||||
codex_sandboxing::compatibility_sandbox_policy_for_permission_profile(
|
||||
@@ -276,11 +316,23 @@ impl CodexThread {
|
||||
self.codex.session.inject_if_running(items).await
|
||||
}
|
||||
|
||||
/// Starts a regular turn with model-visible items only if the thread is idle.
|
||||
/// Starts an automatic regular turn with model-visible items only when idle
|
||||
/// work is allowed for this thread.
|
||||
///
|
||||
/// This is the required entry point for extensions that want to launch
|
||||
/// model-visible work from `ThreadLifecycleContributor::on_thread_idle`.
|
||||
/// The call succeeds only if no user/client-triggered turn is queued, no
|
||||
/// task is currently active, and the thread is not in Plan mode. Active
|
||||
/// Review tasks are rejected by the active-task check because Review turns
|
||||
/// are not steerable.
|
||||
///
|
||||
/// On rejection, the returned error includes a stable reason and carries
|
||||
/// the original `items` unchanged so the caller can decide whether to drop
|
||||
/// them, retry later, or log why no automatic turn was started.
|
||||
pub async fn try_start_turn_if_idle(
|
||||
&self,
|
||||
items: Vec<ResponseItem>,
|
||||
) -> Result<(), Vec<ResponseItem>> {
|
||||
) -> Result<(), TryStartTurnIfIdleError> {
|
||||
self.codex.session.try_start_turn_if_idle(items).await
|
||||
}
|
||||
|
||||
|
||||
@@ -22,6 +22,8 @@ mod config_lock;
|
||||
pub use codex_thread::CodexThread;
|
||||
pub use codex_thread::CodexThreadSettingsOverrides;
|
||||
pub use codex_thread::ThreadConfigSnapshot;
|
||||
pub use codex_thread::TryStartTurnIfIdleError;
|
||||
pub use codex_thread::TryStartTurnIfIdleRejectionReason;
|
||||
pub use session::turn_context::TurnContext;
|
||||
mod agent;
|
||||
mod attestation;
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
use super::input_queue::TurnInput;
|
||||
use super::session::Session;
|
||||
use super::turn_context::TurnContext;
|
||||
use crate::codex_thread::TryStartTurnIfIdleError;
|
||||
use crate::codex_thread::TryStartTurnIfIdleRejectionReason;
|
||||
use crate::state::ActiveTurn;
|
||||
use crate::state::TurnState;
|
||||
use crate::tasks::RegularTask;
|
||||
use codex_protocol::config_types::ModeKind;
|
||||
use codex_protocol::models::ResponseItem;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -32,22 +35,40 @@ impl Session {
|
||||
}
|
||||
}
|
||||
|
||||
/// Starts a regular turn with the provided items only if the session is idle.
|
||||
/// Starts a regular turn with the provided items only if automatic idle work
|
||||
/// is allowed for the current session state.
|
||||
///
|
||||
/// This is the shared gate for extension-initiated idle work. It refuses to
|
||||
/// start a turn when user/client-triggered work is queued, any task is still
|
||||
/// active, or the session is currently in Plan mode. Active Review tasks are
|
||||
/// covered by the active-task check because Review turns are not steerable.
|
||||
pub(crate) async fn try_start_turn_if_idle(
|
||||
self: &Arc<Self>,
|
||||
input: Vec<ResponseItem>,
|
||||
) -> Result<(), Vec<ResponseItem>> {
|
||||
) -> Result<(), TryStartTurnIfIdleError> {
|
||||
if input.is_empty() {
|
||||
return Ok(());
|
||||
}
|
||||
if self.input_queue.has_trigger_turn_mailbox_items().await {
|
||||
return Err(input);
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::PendingTriggerTurn,
|
||||
input,
|
||||
));
|
||||
}
|
||||
if self.collaboration_mode().await.mode == ModeKind::Plan {
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::PlanMode,
|
||||
input,
|
||||
));
|
||||
}
|
||||
|
||||
let turn_state = {
|
||||
let mut active_turn = self.active_turn.lock().await;
|
||||
if active_turn.is_some() {
|
||||
return Err(input);
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::Busy,
|
||||
input,
|
||||
));
|
||||
}
|
||||
let active_turn = active_turn.get_or_insert_with(ActiveTurn::default);
|
||||
Arc::clone(&active_turn.turn_state)
|
||||
@@ -56,18 +77,32 @@ impl Session {
|
||||
if self.input_queue.has_trigger_turn_mailbox_items().await {
|
||||
self.clear_reserved_idle_turn(&turn_state).await;
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
return Err(input);
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::PendingTriggerTurn,
|
||||
input,
|
||||
));
|
||||
}
|
||||
|
||||
let turn_context = self
|
||||
.new_default_turn_with_sub_id(uuid::Uuid::new_v4().to_string())
|
||||
.await;
|
||||
if turn_context.collaboration_mode.mode == ModeKind::Plan {
|
||||
self.clear_reserved_idle_turn(&turn_state).await;
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::PlanMode,
|
||||
input,
|
||||
));
|
||||
}
|
||||
self.maybe_emit_unknown_model_warning_for_turn(turn_context.as_ref())
|
||||
.await;
|
||||
if self.input_queue.has_trigger_turn_mailbox_items().await {
|
||||
self.clear_reserved_idle_turn(&turn_state).await;
|
||||
self.maybe_start_turn_for_pending_work().await;
|
||||
return Err(input);
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::PendingTriggerTurn,
|
||||
input,
|
||||
));
|
||||
}
|
||||
let still_reserved = {
|
||||
let active_turn = self.active_turn.lock().await;
|
||||
@@ -77,7 +112,10 @@ impl Session {
|
||||
};
|
||||
if !still_reserved {
|
||||
self.clear_reserved_idle_turn(&turn_state).await;
|
||||
return Err(input);
|
||||
return Err(TryStartTurnIfIdleError::new(
|
||||
TryStartTurnIfIdleRejectionReason::Busy,
|
||||
input,
|
||||
));
|
||||
}
|
||||
|
||||
self.input_queue
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::turn_context::TurnEnvironment;
|
||||
use super::*;
|
||||
use crate::codex_thread::TryStartTurnIfIdleRejectionReason;
|
||||
use crate::config::ConfigBuilder;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::test_config;
|
||||
@@ -8770,7 +8771,90 @@ async fn try_start_turn_if_idle_rejects_active_turn_without_injecting() {
|
||||
.await
|
||||
.expect_err("active turn should reject idle-only input");
|
||||
|
||||
assert_eq!(vec![item], err);
|
||||
assert_eq!(TryStartTurnIfIdleRejectionReason::Busy, err.reason());
|
||||
assert_eq!(vec![item], err.into_input());
|
||||
assert_eq!(
|
||||
Vec::<TurnInput>::new(),
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await
|
||||
);
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn try_start_turn_if_idle_rejects_plan_mode_without_injecting() {
|
||||
let (sess, _tc, _rx) = make_session_and_context_with_rx().await;
|
||||
let mut collaboration_mode = sess.collaboration_mode().await;
|
||||
collaboration_mode.mode = ModeKind::Plan;
|
||||
{
|
||||
let mut state = sess.state.lock().await;
|
||||
state.session_configuration.collaboration_mode = collaboration_mode;
|
||||
}
|
||||
|
||||
let item = user_message("synthetic idle input");
|
||||
let err = sess
|
||||
.try_start_turn_if_idle(vec![item.clone()])
|
||||
.await
|
||||
.expect_err("plan mode should reject automatic idle input");
|
||||
|
||||
assert_eq!(TryStartTurnIfIdleRejectionReason::PlanMode, err.reason());
|
||||
assert_eq!(vec![item], err.into_input());
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert_eq!(
|
||||
Vec::<TurnInput>::new(),
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn try_start_turn_if_idle_rejects_pending_trigger_turn_without_injecting() {
|
||||
let (sess, _tc, _rx) = make_session_and_context_with_rx().await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
AgentPath::root(),
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"pending trigger".to_string(),
|
||||
/*trigger_turn*/ true,
|
||||
))
|
||||
.await;
|
||||
|
||||
let item = user_message("synthetic idle input");
|
||||
let err = sess
|
||||
.try_start_turn_if_idle(vec![item.clone()])
|
||||
.await
|
||||
.expect_err("pending trigger-turn mail should reject automatic idle input");
|
||||
|
||||
assert_eq!(
|
||||
TryStartTurnIfIdleRejectionReason::PendingTriggerTurn,
|
||||
err.reason()
|
||||
);
|
||||
assert_eq!(vec![item], err.into_input());
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert!(sess.input_queue.has_trigger_turn_mailbox_items().await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn try_start_turn_if_idle_rejects_active_review_turn_without_injecting() {
|
||||
let (sess, tc, _rx) = make_session_and_context_with_rx().await;
|
||||
sess.spawn_task(
|
||||
Arc::clone(&tc),
|
||||
Vec::new(),
|
||||
NeverEndingTask {
|
||||
kind: TaskKind::Review,
|
||||
listen_to_cancellation_token: true,
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let item = user_message("synthetic idle input");
|
||||
let err = sess
|
||||
.try_start_turn_if_idle(vec![item.clone()])
|
||||
.await
|
||||
.expect_err("active review turn should reject automatic idle input");
|
||||
|
||||
assert_eq!(TryStartTurnIfIdleRejectionReason::Busy, err.reason());
|
||||
assert_eq!(vec![item], err.into_input());
|
||||
assert_eq!(
|
||||
Vec::<TurnInput>::new(),
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await
|
||||
|
||||
@@ -307,8 +307,12 @@ impl GoalRuntimeHandle {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
if thread.try_start_turn_if_idle(vec![item]).await.is_err() {
|
||||
tracing::debug!("skipping goal continuation because the thread is no longer idle");
|
||||
if let Err(err) = thread.try_start_turn_if_idle(vec![item]).await {
|
||||
let reason = err.reason();
|
||||
tracing::debug!(
|
||||
?reason,
|
||||
"skipping goal continuation because automatic idle work was rejected"
|
||||
);
|
||||
}
|
||||
|
||||
let current_turn_is_goal_active = self
|
||||
|
||||
Reference in New Issue
Block a user