mirror of
https://github.com/openai/codex.git
synced 2026-11-15 14:52:36 +00:00
Preserve acceptance order in retained thread context (#42770)
## Why Queued prompts and `request_user_input` answers can be persisted in a different order from when the host accepted them. Using persistence order can therefore reorder retained instructions during replay or apply the wrong rollback boundary. ## What changed - Reserve an acceptance sequence for user prompts and elicitation responses when retained thread context is enabled. - Persist that sequence with user-message metadata and verified answers, and use it when ordering, bounding, replaying, and rolling back retained context. - Keep events without acceptance metadata compatible with legacy recording order. ## Testing - Cover delayed queued-input recording, rejected prompts, checkpoint replay, compaction, rollback migration, and legacy retained-context behavior. GitOrigin-RevId: 42d24465cc2033bc51c0c2da79d3c0884a8ad98a
This commit is contained in:
1 parent
88f87d907a
commit
99d66aa1c5
24 files changed
+489
-152
No files matched your search
@@ -23,6 +23,8 @@ pub enum TurnInput {
|
||||
UserInput {
|
||||
content: Vec<UserInput>,
|
||||
client_id: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
acceptance_order: Option<u64>,
|
||||
},
|
||||
FunctionCallOutput(ResponseItem),
|
||||
// Preserve the existing serialized format while carrying injection API metadata
|
||||
@@ -487,6 +489,7 @@ mod tests {
|
||||
.extend_pending_input_and_accept_mailbox_delivery_for_turn_state(
|
||||
&turn_state,
|
||||
vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "steer".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -519,6 +522,7 @@ mod tests {
|
||||
.extend_pending_input_and_accept_mailbox_delivery_for_turn_state(
|
||||
&turn_state,
|
||||
vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "already pending".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
|
||||
@@ -293,6 +293,7 @@ use crate::network_policy_decision::execpolicy_network_rule_amendment;
|
||||
use crate::rollout::map_session_init_error;
|
||||
use crate::session_startup_prewarm::SessionStartupPrewarmHandle;
|
||||
use crate::shell;
|
||||
use crate::state::AcceptedUserInputResponse;
|
||||
use crate::state::AutoCompactWindowIds;
|
||||
use crate::state::AutoCompactWindowSnapshot;
|
||||
use crate::state::PendingRequestPermissions;
|
||||
@@ -319,6 +320,7 @@ use codex_core_plugins::PluginCommandAttribution;
|
||||
use codex_core_plugins::PluginsManager;
|
||||
use codex_core_plugins::RecommendedPluginCandidatesInput;
|
||||
use codex_git_utils::get_git_repo_root;
|
||||
use codex_history::CodexHarnessMetadata;
|
||||
use codex_history::CompactedItem;
|
||||
use codex_history::InitialHistory;
|
||||
use codex_history::ResponseItemEnvelope;
|
||||
@@ -2947,12 +2949,12 @@ impl Session {
|
||||
clippy::await_holding_invalid_type,
|
||||
reason = "active turn checks and turn state updates must remain atomic"
|
||||
)]
|
||||
pub async fn request_user_input(
|
||||
pub(crate) async fn request_user_input(
|
||||
&self,
|
||||
turn_context: &TurnContext,
|
||||
call_id: String,
|
||||
args: RequestUserInputArgs,
|
||||
) -> Option<RequestUserInputResponse> {
|
||||
) -> Option<AcceptedUserInputResponse> {
|
||||
let _elicitation = self.services.elicitations.register();
|
||||
let sub_id = turn_context.sub_id.clone();
|
||||
let (tx_response, rx_response) = oneshot::channel();
|
||||
@@ -2998,15 +3000,23 @@ impl Session {
|
||||
let mut active = self.active_turn.lock().await;
|
||||
match active.as_mut() {
|
||||
Some(at) => {
|
||||
let mut ts = at.turn_state.lock().await;
|
||||
ts.remove_pending_user_input(sub_id)
|
||||
let sender = at.turn_state.lock().await.remove_pending_user_input(sub_id);
|
||||
match sender {
|
||||
Some(sender) => Some((sender, self.reserve_user_input_order().await)),
|
||||
None => None,
|
||||
}
|
||||
}
|
||||
None => None,
|
||||
}
|
||||
};
|
||||
match entry {
|
||||
Some(tx_response) => {
|
||||
tx_response.send(response).ok();
|
||||
Some((tx_response, acceptance_order)) => {
|
||||
tx_response
|
||||
.send(AcceptedUserInputResponse {
|
||||
response,
|
||||
acceptance_order,
|
||||
})
|
||||
.ok();
|
||||
}
|
||||
None => {
|
||||
warn!("No pending user input found for sub_id: {sub_id}");
|
||||
@@ -4539,14 +4549,24 @@ impl Session {
|
||||
turn_context: &TurnContext,
|
||||
input: &[UserInput],
|
||||
client_id: Option<String>,
|
||||
acceptance_order: Option<u64>,
|
||||
persist_context: PersistContext,
|
||||
) {
|
||||
// Persist the user message to history, but emit the turn item from `UserInput` so
|
||||
// UI-only `text_elements` are preserved. `ResponseItem::Message` does not carry
|
||||
// those spans, and `record_response_item_and_emit_turn_item` would drop them.
|
||||
let response_item = self.response_item_from_user_input(input.to_vec());
|
||||
self.record_conversation_items(turn_context, std::slice::from_ref(&response_item))
|
||||
.await;
|
||||
self.record_annotated_conversation_items(
|
||||
turn_context,
|
||||
vec![ResponseItemEnvelope {
|
||||
item: response_item,
|
||||
metadata: acceptance_order.map(|order| CodexHarnessMetadata {
|
||||
user_input_order: Some(order),
|
||||
..Default::default()
|
||||
}),
|
||||
}],
|
||||
)
|
||||
.await;
|
||||
let mut user_message_item = UserMessageItem::new(input);
|
||||
user_message_item.client_id = client_id;
|
||||
let turn_item = TurnItem::UserMessage(user_message_item);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
//! Records host-verified facts at the same persistence boundary as context checkpoints.
|
||||
//! Orders retained inputs and records host-verified facts at the context checkpoint boundary.
|
||||
|
||||
use codex_features::Feature;
|
||||
use codex_history::RetainedContextEvent;
|
||||
use codex_history::RolloutItem;
|
||||
|
||||
@@ -7,6 +8,14 @@ use super::Session;
|
||||
use super::thread_settings;
|
||||
|
||||
impl Session {
|
||||
/// Legacy mode neither reserves a sequence nor takes the session-state lock.
|
||||
pub(crate) async fn reserve_user_input_order(&self) -> Option<u64> {
|
||||
if !self.enabled(Feature::GuardianThreadContext) {
|
||||
return None;
|
||||
}
|
||||
Some(self.state.lock().await.history.reserve_input_order())
|
||||
}
|
||||
|
||||
pub(crate) async fn record_retained_context(&self, mut event: RetainedContextEvent) {
|
||||
event.bound();
|
||||
// Share the checkpoint persistence lock so a fact cannot land on the wrong side
|
||||
|
||||
@@ -183,6 +183,7 @@ pub(super) async fn spawn_review_thread(
|
||||
|
||||
// Seed the child task with the review prompt as the initial user message.
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: review_prompt,
|
||||
// Review prompt is synthesized; no UI element ranges to preserve.
|
||||
|
||||
@@ -1478,15 +1478,17 @@ async fn reconstruct_history_preserves_legacy_compaction_count_with_session_meta
|
||||
async fn reconstruct_history_legacy_compaction_without_replacement_history_does_not_inject_current_initial_context()
|
||||
{
|
||||
let (session, turn_context) = make_session_and_context().await;
|
||||
let answer =
|
||||
codex_history::RetainedContextEvent::VerifiedAnswer(codex_history::VerifiedAnswer {
|
||||
let answer = codex_history::RetainedContextEvent::VerifiedAnswer {
|
||||
answer: codex_history::VerifiedAnswer {
|
||||
turn_id: "legacy-turn".to_owned(),
|
||||
call_id: "ask-1".to_owned(),
|
||||
questions: vec![codex_history::VerifiedQuestionAnswer {
|
||||
question: "Upload?".to_owned(),
|
||||
answer: "Only privately.".to_owned(),
|
||||
}],
|
||||
});
|
||||
},
|
||||
acceptance_order: None,
|
||||
};
|
||||
let mut retained = codex_history::RetainedContext::default();
|
||||
retained.mark_user_messages_incomplete();
|
||||
retained.record(&answer);
|
||||
|
||||
@@ -8154,6 +8154,7 @@ async fn spawn_task_turn_span_inherits_dispatch_trace_context() {
|
||||
sess.spawn_task(
|
||||
Arc::clone(&tc),
|
||||
vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -9698,6 +9699,7 @@ async fn spawn_task_does_not_update_previous_turn_settings_for_non_run_turn_task
|
||||
sess.set_previous_turn_settings(/*previous_turn_settings*/ None)
|
||||
.await;
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -11468,6 +11470,7 @@ async fn guardian_auto_review_emits_thread_idle_after_interrupt() {
|
||||
async fn guardian_helper_review_interrupts_after_three_consecutive_denials() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "keep turn active for helper reviews".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -11535,6 +11538,7 @@ async fn turn_complete_flushes_terminal_event_after_delivery() {
|
||||
.await;
|
||||
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "complete normally".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -11562,6 +11566,7 @@ async fn turn_aborted_flushes_terminal_event_after_delivery() {
|
||||
.await;
|
||||
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "interrupt me".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -11604,6 +11609,7 @@ async fn turn_aborted_flushes_terminal_event_after_delivery() {
|
||||
async fn abort_regular_task_emits_marker_before_turn_aborted() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -11645,6 +11651,7 @@ async fn abort_regular_task_emits_marker_before_turn_aborted() {
|
||||
async fn abort_gracefully_emits_marker_before_turn_aborted() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -11706,6 +11713,7 @@ async fn submit_steer_only(
|
||||
async fn task_finish_emits_turn_item_lifecycle_for_leftover_pending_user_input() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -12143,6 +12151,7 @@ async fn steered_input_reopens_mailbox_delivery_for_current_turn() {
|
||||
(sess.input_queue.get_pending_input(&sess.active_turn).await).0,
|
||||
vec![
|
||||
TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -12199,6 +12208,7 @@ async fn stale_defer_mailbox_delivery_does_not_override_steered_input() {
|
||||
(sess.input_queue.get_pending_input(&sess.active_turn).await).0,
|
||||
vec![
|
||||
TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "follow up".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -12270,6 +12280,7 @@ async fn tool_calls_reopen_mailbox_delivery_for_current_turn() {
|
||||
async fn abort_review_task_emits_exited_then_aborted_and_records_history() {
|
||||
let (sess, tc, rx) = make_session_and_context_with_rx().await;
|
||||
let input = vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "start review".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
|
||||
@@ -313,7 +313,7 @@ async fn start_or_steer(
|
||||
}
|
||||
let mut task_input = merge_additional_context_input(session, additional_context).await;
|
||||
if has_explicit_input {
|
||||
task_input.push(pending_turn_input(input));
|
||||
task_input.push(pending_turn_input(session, input).await);
|
||||
}
|
||||
session
|
||||
.spawn_task(turn_context, task_input, RegularTask::new())
|
||||
@@ -424,7 +424,7 @@ async fn start_if_idle(
|
||||
if let SubmittedTurnInput::UserInput { content, .. } = &input {
|
||||
turn_context.session_telemetry.user_prompt(content);
|
||||
}
|
||||
task_input.push(pending_turn_input(input));
|
||||
task_input.push(pending_turn_input(session, input).await);
|
||||
}
|
||||
TurnStartKind::Automatic => {
|
||||
// Empty automatic user input resumes sampling without a new message.
|
||||
@@ -433,7 +433,7 @@ async fn start_if_idle(
|
||||
.input_queue
|
||||
.extend_pending_input_for_turn_state(
|
||||
turn_state.as_ref(),
|
||||
vec![pending_turn_input(input)],
|
||||
vec![pending_turn_input(session, input).await],
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -621,9 +621,10 @@ impl Session {
|
||||
TurnInput::UserInput {
|
||||
content: std::mem::take(content),
|
||||
client_id: client_id.clone(),
|
||||
acceptance_order: self.reserve_user_input_order().await,
|
||||
}
|
||||
}
|
||||
input => pending_turn_input(input.clone()),
|
||||
input => pending_turn_input(self, input.clone()).await,
|
||||
};
|
||||
pending_input.push(input);
|
||||
if active_task
|
||||
@@ -663,11 +664,13 @@ async fn merge_additional_context_input(
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn pending_turn_input(input: SubmittedTurnInput) -> TurnInput {
|
||||
async fn pending_turn_input(session: &Session, input: SubmittedTurnInput) -> TurnInput {
|
||||
match input {
|
||||
SubmittedTurnInput::UserInput { content, client_id } => {
|
||||
TurnInput::UserInput { content, client_id }
|
||||
}
|
||||
SubmittedTurnInput::UserInput { content, client_id } => TurnInput::UserInput {
|
||||
content,
|
||||
client_id,
|
||||
acceptance_order: session.reserve_user_input_order().await,
|
||||
},
|
||||
SubmittedTurnInput::ResponseItem(mut item)
|
||||
if matches!(
|
||||
&item,
|
||||
|
||||
@@ -862,6 +862,7 @@ async fn steer_only_enforces_expected_turn_id() {
|
||||
.spawn_task(
|
||||
Arc::clone(&turn_context),
|
||||
vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
@@ -951,6 +952,7 @@ async fn rejects_non_regular_turns() {
|
||||
.spawn_task(
|
||||
Arc::clone(&turn_context),
|
||||
vec![TurnInput::UserInput {
|
||||
acceptance_order: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
|
||||
Reference in new issue
Block a user