mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Preserve terminal turn errors in replayed history (#35524)
## Why When rebuilding a thread from rollout events, errors embedded in turn completion events were ignored. Failed retries could therefore be restored as completed turns, causing warnings such as model-overload errors to disappear from the TUI transcript. ## What changed - Mark turns with terminal completion errors as failed and retain their message and structured error information. - Apply late completion errors to their matching historical turn without interrupting the active turn. ## Testing Added history-builder coverage for current and late completion errors, plus a TUI replay snapshot covering repeated overload failures. GitOrigin-RevId: 5c474baa0fd5c73368b444d79fb850cbd4c8023a
This commit is contained in:
@@ -1245,8 +1245,16 @@ impl ThreadHistoryBuilder {
|
||||
}
|
||||
|
||||
fn handle_turn_complete(&mut self, payload: &TurnCompleteEvent) {
|
||||
let mark_completed = |turn: &mut PendingTurn| {
|
||||
if matches!(turn.status, TurnStatus::Completed | TurnStatus::InProgress) {
|
||||
let terminal_error = payload.error.as_ref().map(|error| V2TurnError {
|
||||
message: error.message.clone(),
|
||||
codex_error_info: error.codex_error_info.clone().map(Into::into),
|
||||
additional_details: None,
|
||||
});
|
||||
let apply_completion = |turn: &mut PendingTurn| {
|
||||
if let Some(error) = terminal_error.as_ref() {
|
||||
turn.status = TurnStatus::Failed;
|
||||
turn.error = Some(error.clone());
|
||||
} else if matches!(turn.status, TurnStatus::Completed | TurnStatus::InProgress) {
|
||||
turn.status = TurnStatus::Completed;
|
||||
}
|
||||
turn.completed_at = payload.completed_at;
|
||||
@@ -1260,7 +1268,7 @@ impl ThreadHistoryBuilder {
|
||||
.as_mut()
|
||||
.filter(|turn| turn.id == payload.turn_id)
|
||||
{
|
||||
let changed_turn = mark_completed(current_turn);
|
||||
let changed_turn = apply_completion(current_turn);
|
||||
self.record_changed_turn(changed_turn);
|
||||
self.finish_current_turn();
|
||||
return;
|
||||
@@ -1271,7 +1279,10 @@ impl ThreadHistoryBuilder {
|
||||
.iter_mut()
|
||||
.find(|turn| turn.id == payload.turn_id)
|
||||
{
|
||||
if matches!(turn.status, TurnStatus::Completed | TurnStatus::InProgress) {
|
||||
if let Some(error) = terminal_error.as_ref() {
|
||||
turn.status = TurnStatus::Failed;
|
||||
turn.error = Some(error.clone());
|
||||
} else if matches!(turn.status, TurnStatus::Completed | TurnStatus::InProgress) {
|
||||
turn.status = TurnStatus::Completed;
|
||||
}
|
||||
turn.completed_at = payload.completed_at;
|
||||
@@ -1283,7 +1294,7 @@ impl ThreadHistoryBuilder {
|
||||
|
||||
// If the completion event cannot be matched, apply it to the active turn.
|
||||
if let Some(current_turn) = self.current_turn.as_mut() {
|
||||
let changed_turn = mark_completed(current_turn);
|
||||
let changed_turn = apply_completion(current_turn);
|
||||
self.record_changed_turn(changed_turn);
|
||||
self.finish_current_turn();
|
||||
}
|
||||
@@ -3671,6 +3682,106 @@ mod tests {
|
||||
assert_eq!(turns[1].items.len(), 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn late_turn_complete_with_embedded_error_preserves_active_turn() {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
EventMsg::UserMessage(UserMessageEvent {
|
||||
client_id: None,
|
||||
message: "first".into(),
|
||||
images: None,
|
||||
text_elements: Vec::new(),
|
||||
local_images: Vec::new(),
|
||||
..Default::default()
|
||||
}),
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-b".into(),
|
||||
trace_id: None,
|
||||
started_at: Some(30),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
EventMsg::UserMessage(UserMessageEvent {
|
||||
client_id: None,
|
||||
message: "second".into(),
|
||||
images: None,
|
||||
text_elements: Vec::new(),
|
||||
local_images: Vec::new(),
|
||||
..Default::default()
|
||||
}),
|
||||
EventMsg::TurnComplete(TurnCompleteEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
started_at: Some(10),
|
||||
last_agent_message: None,
|
||||
error: Some(ErrorEvent {
|
||||
message: "Selected model is at capacity. Please try a different model.".into(),
|
||||
codex_error_info: Some(CodexErrorInfo::ServerOverloaded),
|
||||
}),
|
||||
completed_at: Some(20),
|
||||
duration_ms: Some(10_000),
|
||||
time_to_first_token_ms: None,
|
||||
}),
|
||||
];
|
||||
|
||||
let items = events
|
||||
.into_iter()
|
||||
.map(RolloutItem::EventMsg)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(
|
||||
build_turns_from_rollout_items(&items),
|
||||
vec![
|
||||
Turn {
|
||||
id: "turn-a".into(),
|
||||
items_view: TurnItemsView::Full,
|
||||
items: vec![ThreadItem::UserMessage {
|
||||
id: "item-1".into(),
|
||||
client_id: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "first".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
}],
|
||||
status: TurnStatus::Failed,
|
||||
error: Some(TurnError {
|
||||
message: "Selected model is at capacity. Please try a different model."
|
||||
.into(),
|
||||
codex_error_info: Some(
|
||||
crate::protocol::v2::CodexErrorInfo::ServerOverloaded,
|
||||
),
|
||||
additional_details: None,
|
||||
}),
|
||||
started_at: Some(10),
|
||||
completed_at: Some(20),
|
||||
duration_ms: Some(10_000),
|
||||
},
|
||||
Turn {
|
||||
id: "turn-b".into(),
|
||||
items_view: TurnItemsView::Full,
|
||||
items: vec![ThreadItem::UserMessage {
|
||||
id: "item-2".into(),
|
||||
client_id: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "second".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
}],
|
||||
status: TurnStatus::InProgress,
|
||||
error: None,
|
||||
started_at: Some(30),
|
||||
completed_at: None,
|
||||
duration_ms: None,
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn late_turn_aborted_does_not_interrupt_active_turn() {
|
||||
let events = vec![
|
||||
@@ -4121,6 +4232,69 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_complete_with_embedded_error_marks_turn_failed() {
|
||||
let events = vec![
|
||||
EventMsg::TurnStarted(TurnStartedEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
trace_id: None,
|
||||
started_at: Some(10),
|
||||
model_context_window: None,
|
||||
collaboration_mode_kind: Default::default(),
|
||||
}),
|
||||
EventMsg::UserMessage(UserMessageEvent {
|
||||
client_id: None,
|
||||
message: "retry me".into(),
|
||||
images: None,
|
||||
text_elements: Vec::new(),
|
||||
local_images: Vec::new(),
|
||||
..Default::default()
|
||||
}),
|
||||
EventMsg::TurnComplete(TurnCompleteEvent {
|
||||
turn_id: "turn-a".into(),
|
||||
started_at: Some(10),
|
||||
last_agent_message: None,
|
||||
error: Some(ErrorEvent {
|
||||
message: "Selected model is at capacity. Please try a different model.".into(),
|
||||
codex_error_info: Some(CodexErrorInfo::ServerOverloaded),
|
||||
}),
|
||||
completed_at: Some(20),
|
||||
duration_ms: Some(10_000),
|
||||
time_to_first_token_ms: None,
|
||||
}),
|
||||
];
|
||||
|
||||
let items = events
|
||||
.into_iter()
|
||||
.map(RolloutItem::EventMsg)
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert_eq!(
|
||||
build_turns_from_rollout_items(&items),
|
||||
vec![Turn {
|
||||
id: "turn-a".into(),
|
||||
items_view: TurnItemsView::Full,
|
||||
items: vec![ThreadItem::UserMessage {
|
||||
id: "item-1".into(),
|
||||
client_id: None,
|
||||
content: vec![UserInput::Text {
|
||||
text: "retry me".into(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
}],
|
||||
status: TurnStatus::Failed,
|
||||
error: Some(TurnError {
|
||||
message: "Selected model is at capacity. Please try a different model.".into(),
|
||||
codex_error_info: Some(crate::protocol::v2::CodexErrorInfo::ServerOverloaded),
|
||||
additional_details: None,
|
||||
}),
|
||||
started_at: Some(10),
|
||||
completed_at: Some(20),
|
||||
duration_ms: Some(10_000),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rebuilds_hook_prompt_items_from_rollout_response_items() {
|
||||
let hook_prompt = build_hook_prompt_message(&[
|
||||
|
||||
@@ -75,6 +75,54 @@ async fn resumed_initial_messages_render_history() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_failed_turns_preserve_overload_warnings_between_retries() {
|
||||
let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let prompt = "The workspace also looks super confusing with its separator.";
|
||||
let error_message = "Selected model is at capacity. Please try a different model.";
|
||||
let failed_turn = |turn_id: &str, item_id: &str| AppServerTurn {
|
||||
items: vec![AppServerThreadItem::UserMessage {
|
||||
id: item_id.to_string(),
|
||||
client_id: None,
|
||||
content: vec![AppServerUserInput::Text {
|
||||
text: prompt.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
}],
|
||||
..app_server_turn(
|
||||
turn_id,
|
||||
AppServerTurnStatus::Failed,
|
||||
/*duration_ms*/ None,
|
||||
/*error*/
|
||||
Some(AppServerTurnError {
|
||||
message: error_message.to_string(),
|
||||
codex_error_info: Some(CodexErrorInfo::ServerOverloaded),
|
||||
additional_details: None,
|
||||
}),
|
||||
)
|
||||
};
|
||||
|
||||
chat.replay_thread_turns(
|
||||
vec![
|
||||
failed_turn("turn-1", "user-1"),
|
||||
failed_turn("turn-2", "user-2"),
|
||||
],
|
||||
ReplayKind::ResumeInitialMessages,
|
||||
);
|
||||
|
||||
let rendered = drain_insert_history(&mut rx)
|
||||
.into_iter()
|
||||
.map(|lines| lines_to_single_string(&lines))
|
||||
.collect::<String>();
|
||||
|
||||
assert_eq!(rendered.matches(prompt).count(), 2);
|
||||
assert_eq!(rendered.matches(error_message).count(), 2);
|
||||
insta::assert_snapshot!(
|
||||
"replayed_failed_turns_preserve_overload_warnings_between_retries",
|
||||
rendered
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn restored_conversation_ultra_remains_selected_after_switching_to_plan() {
|
||||
let (mut chat, _rx, _ops) = make_chatwidget_manual(Some("gpt-5.4")).await;
|
||||
|
||||
+15
@@ -0,0 +1,15 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/history_replay.rs
|
||||
expression: rendered
|
||||
---
|
||||
|
||||
› The workspace also looks super confusing with its separator.
|
||||
|
||||
|
||||
⚠ Selected model is at capacity. Please try a different model.
|
||||
|
||||
|
||||
› The workspace also looks super confusing with its separator.
|
||||
|
||||
|
||||
⚠ Selected model is at capacity. Please try a different model.
|
||||
Reference in New Issue
Block a user