mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Unify turn input submission and routing (#38275)
## What changed - Add `TurnInputRequest` and typed submission results for atomically starting a turn, steering the active turn, or declining input with a specific reason. - Expose `start_or_steer_turn`, `start_turn_if_idle`, and `steer_turn` on `CodexThread`, and migrate Core consumers to these APIs. - Make app-server `turn/start` steer an active regular turn and return that turn's ID. Reject incompatible output schemas and non-steerable turns without applying settings or enqueueing input. ## Testing - Cover concurrent start-or-steer submissions, accepted and rejected settings updates, output-schema compatibility, idle-start rejection, and app-server steering. GitOrigin-RevId: dd9b5528d76ec650c019e97af420bc13190ea86a
This commit is contained in:
@@ -1,13 +1,14 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
use std::sync::Weak;
|
||||
|
||||
use codex_core::CodexThread;
|
||||
use codex_core::NotSubmittedReason;
|
||||
use codex_core::StartIfIdleSubmission;
|
||||
use codex_core::ThreadManager;
|
||||
use codex_core::TurnInput;
|
||||
use codex_core::UserMessageAdmission;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_extension_api::ExtensionEventSink;
|
||||
use codex_extension_api::ExtensionFuture;
|
||||
use codex_extension_api::ThreadIdleCause;
|
||||
@@ -19,9 +20,7 @@ use codex_protocol::models::snapshot_local_user_input;
|
||||
use codex_protocol::protocol::AgentStatus;
|
||||
use codex_protocol::protocol::Event;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::Op;
|
||||
use codex_protocol::protocol::ThreadQueueChangedEvent;
|
||||
use codex_protocol::protocol::ThreadSettingsOverrides;
|
||||
use codex_protocol::protocol::W3cTraceContext;
|
||||
use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
@@ -49,8 +48,10 @@ pub enum QueueServiceError {
|
||||
InvalidPayload(#[from] serde_json::Error),
|
||||
#[error("local queued attachment is invalid: {0}")]
|
||||
InvalidAttachment(#[from] std::io::Error),
|
||||
#[error("queued user message could not be admitted: {0}")]
|
||||
Admission(#[from] CodexErr),
|
||||
#[error("Core failed to submit queued user message: {0}")]
|
||||
CoreSubmissionError(#[from] CodexErr),
|
||||
#[error("Core declined to start queued user message: {0:?}")]
|
||||
NotStarted(NotSubmittedReason),
|
||||
#[error("only user input can be added to the user-message queue")]
|
||||
InvalidInput,
|
||||
#[error(
|
||||
@@ -189,14 +190,13 @@ impl QueuedItemService {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Starts or steers the exact queued message and removes it after Core
|
||||
/// accepts the input for turn processing.
|
||||
/// Starts the exact queued message when idle and removes it after submission.
|
||||
pub async fn start(
|
||||
&self,
|
||||
thread: &CodexThread,
|
||||
queued_item_id: String,
|
||||
trace: Option<W3cTraceContext>,
|
||||
) -> Result<UserMessageAdmission, QueueServiceError> {
|
||||
) -> Result<StartIfIdleSubmission, QueueServiceError> {
|
||||
let thread_id = thread.session_configured().thread_id;
|
||||
let _dispatch_guard = self.dispatch_guard(thread_id).await;
|
||||
let item = self
|
||||
@@ -207,24 +207,18 @@ impl QueuedItemService {
|
||||
.ok_or_else(|| ThreadStoreError::InvalidRequest {
|
||||
message: format!("queued item not found: {queued_item_id}"),
|
||||
})?;
|
||||
let TurnInput::UserInput { content, client_id } = item.input else {
|
||||
let input @ TurnInput::UserInput { .. } = item.input else {
|
||||
return Err(QueueServiceError::InvalidInput);
|
||||
};
|
||||
let admission = thread
|
||||
.submit_user_input_and_wait_for_admission(
|
||||
Op::UserInput {
|
||||
items: content,
|
||||
final_output_json_schema: None,
|
||||
responsesapi_client_metadata: None,
|
||||
additional_context: BTreeMap::new(),
|
||||
thread_settings: ThreadSettingsOverrides::default(),
|
||||
},
|
||||
trace,
|
||||
client_id,
|
||||
)
|
||||
.await?;
|
||||
let submission = thread
|
||||
.start_turn_if_idle(TurnInputRequest::new(input).with_trace(trace))
|
||||
.await
|
||||
.map_err(QueueServiceError::CoreSubmissionError)?;
|
||||
if let StartIfIdleSubmission::NotSubmitted { reason } = submission {
|
||||
return Err(QueueServiceError::NotStarted(reason));
|
||||
}
|
||||
self.delete_locked(thread_id, queued_item_id).await?;
|
||||
Ok(admission)
|
||||
Ok(submission)
|
||||
}
|
||||
|
||||
async fn dispatch_if_idle(&self, thread_id: ThreadId) -> Result<(), QueueServiceError> {
|
||||
@@ -261,18 +255,33 @@ impl QueuedItemService {
|
||||
continue;
|
||||
}
|
||||
|
||||
if let Err(error) = thread.try_start_turn_if_idle(vec![input]).await {
|
||||
tracing::warn!(
|
||||
%thread_id,
|
||||
%queued_item_id,
|
||||
reason = ?error.reason(),
|
||||
"core could not start queued user input"
|
||||
);
|
||||
return Ok(());
|
||||
match thread
|
||||
.start_turn_if_idle(TurnInputRequest::new(input))
|
||||
.await
|
||||
{
|
||||
Ok(StartIfIdleSubmission::Started { .. }) => {
|
||||
self.delete_locked(thread_id, queued_item_id).await?;
|
||||
return Ok(());
|
||||
}
|
||||
Ok(StartIfIdleSubmission::NotSubmitted { reason }) => {
|
||||
tracing::warn!(
|
||||
%thread_id,
|
||||
%queued_item_id,
|
||||
?reason,
|
||||
"core could not start queued user input"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
Err(error) => {
|
||||
tracing::warn!(
|
||||
%thread_id,
|
||||
%queued_item_id,
|
||||
%error,
|
||||
"core could not start queued user input"
|
||||
);
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
self.delete_locked(thread_id, queued_item_id).await?;
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,8 +6,11 @@ use std::sync::Weak;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context;
|
||||
use codex_core::NotSubmittedReason;
|
||||
use codex_core::StartIfIdleSubmission;
|
||||
use codex_core::TurnInput;
|
||||
use codex_core::UserMessageAdmission;
|
||||
use codex_core::TurnInputRequest;
|
||||
use codex_core::TurnInputSubmission;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionEventSink;
|
||||
use codex_extension_api::ExtensionFuture;
|
||||
@@ -37,11 +40,14 @@ use codex_utils_absolute_path::test_support::PathExt;
|
||||
use core_test_support::hooks::trust_discovered_hooks;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::streaming_sse::StreamingSseChunk;
|
||||
use core_test_support::streaming_sse::start_streaming_sse_server;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event_match;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
const TINY_PNG_BYTES: &[u8] = &[
|
||||
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0,
|
||||
@@ -291,11 +297,11 @@ async fn starting_a_selected_item_preserves_the_remaining_queue() -> anyhow::Res
|
||||
.enqueue(thread_id, structured_user_input("second"))
|
||||
.await?;
|
||||
|
||||
let admission = service
|
||||
let submission = service
|
||||
.start(test.codex.as_ref(), second.id.clone(), /*trace*/ None)
|
||||
.await?;
|
||||
|
||||
assert!(matches!(admission, UserMessageAdmission::Started { .. }));
|
||||
assert!(matches!(submission, StartIfIdleSubmission::Started { .. }));
|
||||
assert_eq!(vec![first], service.list(thread_id).await?);
|
||||
wait_for_event_match(test.codex.as_ref(), |event| match event {
|
||||
EventMsg::TurnComplete(_) => Some(()),
|
||||
@@ -313,6 +319,65 @@ async fn starting_a_selected_item_preserves_the_remaining_queue() -> anyhow::Res
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn starting_a_selected_item_while_active_leaves_it_queued() -> anyhow::Result<()> {
|
||||
let (release_response, response_gate) = oneshot::channel();
|
||||
let (server, _completions) = start_streaming_sse_server(vec![vec![
|
||||
StreamingSseChunk {
|
||||
gate: None,
|
||||
body: responses::sse(vec![responses::ev_response_created("resp-1")]),
|
||||
},
|
||||
StreamingSseChunk {
|
||||
gate: Some(response_gate),
|
||||
body: responses::sse(vec![responses::ev_completed("resp-1")]),
|
||||
},
|
||||
]])
|
||||
.await;
|
||||
let test = test_codex().build_with_streaming_server(&server).await?;
|
||||
let thread_id = test.session_configured.thread_id;
|
||||
let service = QueuedItemService::new(
|
||||
loaded_thread_queue(&test)?,
|
||||
Weak::new(),
|
||||
Arc::new(NoopExtensionEventSink),
|
||||
);
|
||||
let queued = service
|
||||
.enqueue(thread_id, user_input("stay queued"))
|
||||
.await?;
|
||||
|
||||
let active_turn = test
|
||||
.codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "active turn".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]))
|
||||
.await?;
|
||||
assert!(matches!(active_turn, TurnInputSubmission::Started { .. }));
|
||||
tokio::time::timeout(
|
||||
Duration::from_secs(5),
|
||||
server.wait_for_request_count(/*count*/ 1),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let error = service
|
||||
.start(test.codex.as_ref(), queued.id.clone(), /*trace*/ None)
|
||||
.await
|
||||
.expect_err("active turn should reject explicit queue start");
|
||||
assert!(matches!(
|
||||
error,
|
||||
QueueServiceError::NotStarted(NotSubmittedReason::NotIdle)
|
||||
));
|
||||
assert_eq!(vec![queued], service.list(thread_id).await?);
|
||||
|
||||
release_response
|
||||
.send(())
|
||||
.expect("active response gate should remain open");
|
||||
wait_for_event_match(test.codex.as_ref(), |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_)).then_some(())
|
||||
})
|
||||
.await;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn stopped_turns_leave_queued_messages_in_place() -> anyhow::Result<()> {
|
||||
let server = start_mock_server().await;
|
||||
@@ -453,12 +518,13 @@ async fn rejected_queue_messages_are_consumed_without_retrying_or_blocking_follo
|
||||
assert!(queue.list(thread_id).await?.is_empty());
|
||||
|
||||
let rejected = staging.enqueue(thread_id, user_input("blocked")).await?;
|
||||
let admission = tokio::time::timeout(
|
||||
let submission = tokio::time::timeout(
|
||||
Duration::from_secs(10),
|
||||
queue.start(test.codex.as_ref(), rejected.id, /*trace*/ None),
|
||||
)
|
||||
.await??;
|
||||
assert!(matches!(admission, UserMessageAdmission::Started { .. }));
|
||||
.await?
|
||||
.expect("explicitly started input should be submitted");
|
||||
assert!(matches!(submission, StartIfIdleSubmission::Started { .. }));
|
||||
wait_for_event_match(test.codex.as_ref(), |event| {
|
||||
matches!(event, EventMsg::TurnComplete(_)).then_some(())
|
||||
})
|
||||
@@ -616,7 +682,7 @@ async fn non_user_input_cannot_enter_the_user_message_queue() -> anyhow::Result<
|
||||
let error = service
|
||||
.enqueue(
|
||||
ThreadId::new(),
|
||||
TurnInput::ResponseItem(ResponseItem::Other.into()),
|
||||
TurnInput::ResponseItem(ResponseItem::Other),
|
||||
)
|
||||
.await
|
||||
.expect_err("response item should not enter the user queue");
|
||||
|
||||
Reference in New Issue
Block a user