mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
Fix stale turn steering fallback in tui_app_server (#15714)
This PR adds code to recover from a narrow app-server timing race where a follow-up can be sent after the previous turn has already ended but before the TUI has observed that completion. Instead of surfacing turn/steer failed: no active turn to steer, the client now treats that as a stale active-turn cache and falls back to starting a fresh turn, matching the intended submit behavior more closely. This is similar to the strategy employed by other app server clients (notably, the IDE extension and desktop app). This race exists because the current app-server API makes the client choose between two separate RPCs, turn/steer and turn/start, based on its local view of whether a turn is still active. That view is replicated from asynchronous notifications, so it can be stale for a brief window. The server may already have ended the turn while the client still believes it is in progress. Since the choice is made client-side rather than atomically on the server, tui_app_server can occasionally send turn/steer for a turn that no longer exists.
This commit is contained in:
@@ -621,6 +621,10 @@ impl ThreadEventStore {
|
||||
fn active_turn_id(&self) -> Option<&str> {
|
||||
self.active_turn_id.as_deref()
|
||||
}
|
||||
|
||||
fn clear_active_turn_id(&mut self) {
|
||||
self.active_turn_id = None;
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -986,6 +990,13 @@ fn active_turn_not_steerable_turn_error(error: &TypedRequestError) -> Option<App
|
||||
.then_some(turn_error)
|
||||
}
|
||||
|
||||
fn active_turn_missing_steer_error(error: &TypedRequestError) -> bool {
|
||||
let TypedRequestError::Server { source, .. } = error else {
|
||||
return false;
|
||||
};
|
||||
source.message == "no active turn to steer"
|
||||
}
|
||||
|
||||
impl App {
|
||||
pub fn chatwidget_init_for_forked_or_resumed_thread(
|
||||
&self,
|
||||
@@ -2022,23 +2033,32 @@ impl App {
|
||||
personality,
|
||||
submission_type,
|
||||
} => {
|
||||
let mut should_start_turn = true;
|
||||
if let Some(turn_id) = self.active_turn_id_for_thread(thread_id).await {
|
||||
match app_server
|
||||
.turn_steer(thread_id, turn_id, items.to_vec())
|
||||
.await
|
||||
{
|
||||
Ok(_) => {}
|
||||
Ok(_) => return Ok(true),
|
||||
Err(error) => {
|
||||
if let Some(turn_error) = active_turn_not_steerable_turn_error(&error) {
|
||||
if !self.chat_widget.enqueue_rejected_steer() {
|
||||
self.chat_widget.add_error_message(turn_error.message);
|
||||
}
|
||||
return Ok(true);
|
||||
} else if active_turn_missing_steer_error(&error) {
|
||||
if let Some(channel) = self.thread_event_channels.get(&thread_id) {
|
||||
let mut store = channel.store.lock().await;
|
||||
store.clear_active_turn_id();
|
||||
}
|
||||
should_start_turn = true;
|
||||
} else {
|
||||
return Err(error.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
}
|
||||
if should_start_turn {
|
||||
app_server
|
||||
.turn_start(
|
||||
thread_id,
|
||||
@@ -8045,6 +8065,17 @@ guardian_approval = true
|
||||
assert_eq!(refreshed_store.active_turn_id(), Some("turn-2"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_event_store_clear_active_turn_id_resets_cached_turn() {
|
||||
let mut store = ThreadEventStore::new(8);
|
||||
let thread_id = ThreadId::new();
|
||||
store.push_notification(turn_started_notification(thread_id, "turn-1"));
|
||||
|
||||
store.clear_active_turn_id();
|
||||
|
||||
assert_eq!(store.active_turn_id(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_event_store_rebase_preserves_resolved_request_state() {
|
||||
let thread_id = ThreadId::new();
|
||||
@@ -8275,6 +8306,21 @@ guardian_approval = true
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn active_turn_missing_steer_error_detects_stale_turn_race() {
|
||||
let error = TypedRequestError::Server {
|
||||
method: "turn/steer".to_string(),
|
||||
source: JSONRPCErrorError {
|
||||
code: -32602,
|
||||
message: "no active turn to steer".to_string(),
|
||||
data: None,
|
||||
},
|
||||
};
|
||||
|
||||
assert!(active_turn_missing_steer_error(&error));
|
||||
assert_eq!(active_turn_not_steerable_turn_error(&error), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn select_model_availability_nux_uses_existing_model_order_as_priority() {
|
||||
let mut presets = all_model_presets();
|
||||
|
||||
Reference in New Issue
Block a user