mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Move pending input into input queue (#22728)
## Why Pending model input was split across `Session`, `TurnState`, and the agent mailbox. That made it easy for new paths to manage queued user input or mailbox delivery outside the intended ownership boundary. This PR consolidates the model-facing input lifecycle behind the session input queue so turn-local pending input, next-turn queued items, and mailbox delivery coordination are owned in one place. ## What Changed - Added `session/input_queue.rs` to own pending input queues and mailbox delivery coordination. - Removed the standalone `agent/mailbox.rs` channel wrapper and store mailbox items directly in the input queue. - Moved pending-input mutations off `TurnState`; `TurnState` now exposes the queue-owned storage directly for now. - Routed abort cleanup, mailbox delivery phase changes, next-turn queued items, and active-turn pending input through `InputQueue`. - Boxed stack-heavy agent resume/fork startup futures that the refactor pushed over the default test stack. - Updated session, task, goal, stream-event, and multi-agent call sites and tests to use the new queue ownership. ## Verification - `cargo test -p codex-core --lib agent::control::tests` - `cargo test -p codex-core --lib agent::control::tests::resume_closed_child_reopens_open_descendants -- --exact` - `cargo test -p codex-core --lib agent::control::tests::spawn_agent_fork_last_n_turns_keeps_only_recent_turns -- --exact` - `cargo test -p codex-core --lib agent::control::tests::resume_thread_subagent_restores_stored_nickname_and_role -- --exact` - `cargo test -p codex-core` was also run; it completed with 1814 passed, 4 ignored, and one timeout in `agent::control::tests::resume_thread_subagent_restores_stored_nickname_and_role`, which passed when rerun in isolation.
This commit is contained in:
committed by
GitHub
Unverified
parent
a66e0e9c4b
commit
afa0101ae2
@@ -4394,7 +4394,6 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
/*goal_tools_supported*/ true,
|
||||
);
|
||||
|
||||
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
|
||||
let session = Session {
|
||||
conversation_id: thread_id,
|
||||
installation_id: "11111111-1111-4111-8111-111111111111".to_string(),
|
||||
@@ -4407,9 +4406,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
|
||||
pending_mcp_server_refresh_config: Mutex::new(None),
|
||||
conversation: Arc::new(RealtimeConversationManager::new()),
|
||||
active_turn: Mutex::new(None),
|
||||
mailbox,
|
||||
mailbox_rx: Mutex::new(mailbox_rx),
|
||||
idle_pending_input: Mutex::new(Vec::new()),
|
||||
input_queue: super::input_queue::InputQueue::new(),
|
||||
goal_runtime: crate::goals::GoalRuntimeState::new(),
|
||||
guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(),
|
||||
services,
|
||||
@@ -6252,7 +6249,6 @@ where
|
||||
/*goal_tools_supported*/ true,
|
||||
));
|
||||
|
||||
let (mailbox, mailbox_rx) = crate::agent::Mailbox::new();
|
||||
let session = Arc::new(Session {
|
||||
conversation_id: thread_id,
|
||||
installation_id: "11111111-1111-4111-8111-111111111111".to_string(),
|
||||
@@ -6265,9 +6261,7 @@ where
|
||||
pending_mcp_server_refresh_config: Mutex::new(None),
|
||||
conversation: Arc::new(RealtimeConversationManager::new()),
|
||||
active_turn: Mutex::new(None),
|
||||
mailbox,
|
||||
mailbox_rx: Mutex::new(mailbox_rx),
|
||||
idle_pending_input: Mutex::new(Vec::new()),
|
||||
input_queue: super::input_queue::InputQueue::new(),
|
||||
goal_runtime: crate::goals::GoalRuntimeState::new(),
|
||||
guardian_review_session: crate::guardian::GuardianReviewSessionManager::default(),
|
||||
services,
|
||||
@@ -8098,7 +8092,7 @@ async fn steer_input_returns_active_turn_id() {
|
||||
.expect("steering with matching expected turn id should succeed");
|
||||
|
||||
assert_eq!(turn_id, tc.sub_id);
|
||||
assert!(sess.has_pending_input().await);
|
||||
assert!(sess.input_queue.has_pending_input(&sess.active_turn).await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8144,7 +8138,7 @@ async fn prepend_pending_input_keeps_older_tail_ahead_of_newer_input() {
|
||||
.await
|
||||
.expect("inject initial pending input into active turn");
|
||||
|
||||
let drained = sess.get_pending_input().await;
|
||||
let drained = sess.input_queue.get_pending_input(&sess.active_turn).await;
|
||||
assert_eq!(drained, vec![blocked, later.clone()]);
|
||||
|
||||
sess.inject_response_items(vec![newer.clone()])
|
||||
@@ -8153,11 +8147,15 @@ async fn prepend_pending_input_keeps_older_tail_ahead_of_newer_input() {
|
||||
|
||||
let mut drained_iter = drained.into_iter();
|
||||
let _blocked = drained_iter.next().expect("blocked prompt should exist");
|
||||
sess.prepend_pending_input(drained_iter.collect())
|
||||
sess.input_queue
|
||||
.prepend_pending_input(&sess.active_turn, drained_iter.collect())
|
||||
.await
|
||||
.expect("requeue later pending input at the front of the queue");
|
||||
|
||||
assert_eq!(sess.get_pending_input().await, vec![later, newer]);
|
||||
assert_eq!(
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![later, newer]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8171,7 +8169,8 @@ async fn queued_response_items_for_next_turn_move_into_next_active_turn() {
|
||||
phase: None,
|
||||
};
|
||||
|
||||
sess.queue_response_items_for_next_turn(vec![queued_item.clone()])
|
||||
sess.input_queue
|
||||
.queue_response_items_for_next_turn(vec![queued_item.clone()])
|
||||
.await;
|
||||
|
||||
sess.spawn_task(
|
||||
@@ -8184,7 +8183,10 @@ async fn queued_response_items_for_next_turn_move_into_next_active_turn() {
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(sess.get_pending_input().await, vec![queued_item]);
|
||||
assert_eq!(
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![queued_item]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8198,13 +8200,18 @@ async fn idle_interrupt_does_not_wake_queued_next_turn_items() {
|
||||
phase: None,
|
||||
};
|
||||
|
||||
sess.queue_response_items_for_next_turn(vec![queued_item])
|
||||
sess.input_queue
|
||||
.queue_response_items_for_next_turn(vec![queued_item])
|
||||
.await;
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Interrupted).await;
|
||||
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert!(sess.has_queued_response_items_for_next_turn().await);
|
||||
assert!(
|
||||
sess.input_queue
|
||||
.has_queued_response_items_for_next_turn()
|
||||
.await
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8222,16 +8229,17 @@ async fn abort_empty_active_turn_preserves_pending_input() {
|
||||
let active_turn = active.get_or_insert_with(ActiveTurn::default);
|
||||
Arc::clone(&active_turn.turn_state)
|
||||
};
|
||||
turn_state
|
||||
.lock()
|
||||
.await
|
||||
.push_pending_input(pending_item.clone());
|
||||
sess.input_queue
|
||||
.extend_pending_input_for_turn_state(turn_state.as_ref(), vec![pending_item.clone()])
|
||||
.await;
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
|
||||
assert!(sess.active_turn.lock().await.is_none());
|
||||
assert_eq!(
|
||||
turn_state.lock().await.take_pending_input(),
|
||||
sess.input_queue
|
||||
.take_pending_input_for_turn_state(turn_state.as_ref())
|
||||
.await,
|
||||
vec![pending_item]
|
||||
);
|
||||
}
|
||||
@@ -8593,7 +8601,7 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh
|
||||
})
|
||||
.await?;
|
||||
|
||||
let pending_input = sess.get_pending_input().await;
|
||||
let pending_input = sess.input_queue.get_pending_input(&sess.active_turn).await;
|
||||
let [ResponseInputItem::Message { role, content, .. }] = pending_input.as_slice() else {
|
||||
panic!("expected one budget-limit steering message, got {pending_input:#?}");
|
||||
};
|
||||
@@ -8814,7 +8822,7 @@ async fn external_objective_change_steers_active_turn() -> anyhow::Result<()> {
|
||||
})
|
||||
.await?;
|
||||
|
||||
let pending_input = sess.get_pending_input().await;
|
||||
let pending_input = sess.input_queue.get_pending_input(&sess.active_turn).await;
|
||||
assert!(
|
||||
pending_input.iter().any(|item| {
|
||||
matches!(
|
||||
@@ -9021,19 +9029,26 @@ async fn queue_only_mailbox_mail_waits_for_next_turn_after_answer_boundary() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
!sess.has_pending_input().await,
|
||||
!sess.input_queue.has_pending_input(&sess.active_turn).await,
|
||||
"queue-only mailbox mail should stay buffered once the current turn emitted its answer"
|
||||
);
|
||||
assert_eq!(sess.get_pending_input().await, Vec::new());
|
||||
assert_eq!(
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
Vec::new()
|
||||
);
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![communication.to_response_input_item()],
|
||||
);
|
||||
}
|
||||
@@ -9051,23 +9066,27 @@ async fn trigger_turn_mailbox_mail_waits_for_next_turn_after_answer_boundary() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
AgentPath::try_from("/root/worker").expect("worker path should parse"),
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"late trigger update".to_string(),
|
||||
/*trigger_turn*/ true,
|
||||
));
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(InterAgentCommunication::new(
|
||||
AgentPath::try_from("/root/worker").expect("worker path should parse"),
|
||||
AgentPath::root(),
|
||||
Vec::new(),
|
||||
"late trigger update".to_string(),
|
||||
/*trigger_turn*/ true,
|
||||
))
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
!sess.has_pending_input().await,
|
||||
!sess.input_queue.has_pending_input(&sess.active_turn).await,
|
||||
"trigger-turn mailbox mail should not extend the current turn after its answer boundary"
|
||||
);
|
||||
|
||||
sess.abort_all_tasks(TurnAbortReason::Replaced).await;
|
||||
|
||||
assert!(sess.has_trigger_turn_mailbox_items().await);
|
||||
assert!(sess.input_queue.has_trigger_turn_mailbox_items().await);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -9090,8 +9109,12 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
sess.steer_input(
|
||||
vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9104,7 +9127,7 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
|
||||
.expect("steered input should be accepted");
|
||||
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![
|
||||
ResponseInputItem::from(vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9135,8 +9158,12 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
sess.steer_input(
|
||||
vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9148,10 +9175,12 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
|
||||
.await
|
||||
.expect("steered input should be accepted");
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![
|
||||
ResponseInputItem::from(vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
@@ -9182,8 +9211,12 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
|
||||
)
|
||||
.await;
|
||||
|
||||
sess.defer_mailbox_delivery_to_next_turn(&tc.sub_id).await;
|
||||
sess.enqueue_mailbox_communication(communication.clone());
|
||||
sess.input_queue
|
||||
.defer_mailbox_delivery_to_next_turn(&sess.active_turn, &tc.sub_id)
|
||||
.await;
|
||||
sess.input_queue
|
||||
.enqueue_mailbox_communication(communication.clone())
|
||||
.await;
|
||||
|
||||
let item = ResponseItem::FunctionCall {
|
||||
id: None,
|
||||
@@ -9207,7 +9240,7 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
|
||||
assert!(output.needs_follow_up);
|
||||
assert!(output.tool_future.is_some());
|
||||
assert_eq!(
|
||||
sess.get_pending_input().await,
|
||||
sess.input_queue.get_pending_input(&sess.active_turn).await,
|
||||
vec![communication.to_response_input_item()],
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user