mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
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
This commit is contained in:
@@ -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<String>) -> 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<String>,
|
||||
items: Vec<ThreadItemPayload>,
|
||||
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::<Vec<_>>(),
|
||||
[
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user