From 83e3d63ba1b928d9f73d52480473694f84aa5a18 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 11 Nov 2026 01:35:10 +0000 Subject: [PATCH] Finalize aborted turns and ignore unrelated late aborts (#60700) ## Why An aborted turn could remain active, and a late abort for an unknown turn could interrupt a different turn with an explicit ID. ## What changed Finalize the active turn after applying an abort. Only fall back to the active turn when the abort lacks a turn ID or the active turn has a generated ID. ## Testing Extend regression coverage to verify that a late abort preserves a review turn's ID and completed status, and that repeating `turn/interrupt` returns `no active turn to interrupt`. GitOrigin-RevId: 73072dda7879bd50b1013f4a0bbf1b68546f2619 --- .../src/protocol/thread_history.rs | 22 +++++++++++++++++-- .../tests/suite/v2/turn_interrupt.rs | 17 ++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index 05c4b52d74..70e4cad1de 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -1461,6 +1461,7 @@ impl ThreadHistoryBuilder { if let Some(turn) = self.current_turn.as_mut().filter(|turn| turn.id == turn_id) { let changed_turn = apply_abort(turn); self.record_changed_turn(changed_turn); + self.finish_current_turn(); return; } @@ -1471,10 +1472,13 @@ impl ThreadHistoryBuilder { } } - // If the event has no ID (or refers to an unknown turn), fall back to the active turn. - if let Some(turn) = self.current_turn.as_mut() { + // Fall back only when the abort or the active turn lacks an explicit ID. + if let Some(turn) = self.current_turn.as_mut() + && (payload.turn_id.is_none() || turn.has_generated_id) + { let changed_turn = apply_abort(turn); self.record_changed_turn(changed_turn); + self.finish_current_turn(); } } @@ -1592,6 +1596,7 @@ impl ThreadHistoryBuilder { } fn new_turn(&mut self, id: Option) -> PendingTurn { + let has_generated_id = id.is_none(); let id = id.unwrap_or_else(|| { if self.next_rollout_index == 0 { Uuid::now_v7().to_string() @@ -1601,6 +1606,7 @@ impl ThreadHistoryBuilder { }); PendingTurn { id, + has_generated_id, root_turn_id: None, items: Vec::new(), item_index: TurnItemIndex::default(), @@ -1930,6 +1936,7 @@ impl TurnItemIndex { struct PendingTurn { id: String, + has_generated_id: bool, root_turn_id: Option, items: Vec, item_index: TurnItemIndex, @@ -2360,6 +2367,16 @@ mod tests { started_at_ms: Some(0), completed_at_ms: 0, }), + // A late abort for a discarded turn must preserve the review turn's explicit ID. + EventMsg::TurnAborted(TurnAbortedEvent { + root_turn_id: None, + turn_id: Some("discarded-turn".into()), + started_at: None, + reason: TurnAbortReason::Replaced, + error: None, + completed_at: None, + duration_ms: None, + }), EventMsg::ItemCompleted(ItemCompletedEvent { content_token_count: None, thread_id, @@ -2387,6 +2404,7 @@ mod tests { let turns = builder.finish(); assert_eq!(turns[0].id, "turn-1"); + assert_eq!(turns[0].status, TurnStatus::Completed); assert_eq!( turns[0].item_payloads().collect::>(), [ diff --git a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs index bdc1740778..35459f29e6 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_interrupt.rs @@ -146,6 +146,23 @@ async fn turn_interrupt_aborts_running_turn() -> Result<()> { assert_eq!(completed.thread_id, thread_id); assert_eq!(completed.turn.status, TurnStatus::Interrupted); + let repeat_id = mcp + .send_turn_interrupt_request(TurnInterruptParams { thread_id, turn_id }) + .await?; + let repeat_err = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(repeat_id)), + ) + .await??; + assert_eq!( + repeat_err.error, + JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: "no active turn to interrupt".to_string(), + data: None, + } + ); + Ok(()) }