From c4a574c823c10f438572509fc2abbf8770daf7f2 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 14 Mar 2026 20:18:55 -0600 Subject: [PATCH] codex: address PR review feedback (#14710) --- codex-rs/tui/src/app.rs | 20 ++++++++++- codex-rs/tui/src/app/app_server_adapter.rs | 34 +++++++++++++++++-- .../tui/src/onboarding/onboarding_screen.rs | 4 ++- 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 0786c0309c..9bc52576d5 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -705,7 +705,7 @@ pub(crate) struct App { pending_patch_approval_request_ids: HashMap, pending_elicitation_request_ids: HashMap<(String, RequestId), RequestId>, pending_permissions_request_ids: HashMap, - pending_user_input_request_ids: HashMap, + pending_user_input_request_ids: HashMap>, pending_dynamic_tool_request_ids: HashMap, } @@ -7600,6 +7600,24 @@ smart_approvals = true .expect("shutdown should complete"); } + #[tokio::test] + async fn pending_user_input_request_ids_preserve_fifo_per_turn() { + let mut app = make_test_app().await; + + app.note_pending_user_input_request_id("turn-1", RequestId::Integer(1)); + app.note_pending_user_input_request_id("turn-1", RequestId::Integer(2)); + + assert_eq!( + app.pop_pending_user_input_request_id("turn-1"), + Some(RequestId::Integer(1)) + ); + assert_eq!( + app.pop_pending_user_input_request_id("turn-1"), + Some(RequestId::Integer(2)) + ); + assert_eq!(app.pop_pending_user_input_request_id("turn-1"), None); + } + #[tokio::test] async fn clear_only_ui_reset_preserves_chat_session_state() { let mut app = make_test_app().await; diff --git a/codex-rs/tui/src/app/app_server_adapter.rs b/codex-rs/tui/src/app/app_server_adapter.rs index 89e8547b20..9abf4bf9ac 100644 --- a/codex-rs/tui/src/app/app_server_adapter.rs +++ b/codex-rs/tui/src/app/app_server_adapter.rs @@ -577,7 +577,7 @@ impl App { .await?; } Op::UserInputAnswer { id, response } => { - let Some(request_id) = self.pending_user_input_request_ids.remove(&id) else { + let Some(request_id) = self.pop_pending_user_input_request_id(&id) else { return Err(format!( "Missing app-server request for user input turn {id}." )); @@ -775,8 +775,7 @@ impl App { .insert(params.item_id.clone(), request_id.clone()); } ServerRequest::ToolRequestUserInput { request_id, params } => { - self.pending_user_input_request_ids - .insert(params.turn_id.clone(), request_id.clone()); + self.note_pending_user_input_request_id(¶ms.turn_id, request_id.clone()); } ServerRequest::DynamicToolCall { request_id, params } => { self.pending_dynamic_tool_request_ids @@ -791,6 +790,35 @@ impl App { } } + pub(super) fn note_pending_user_input_request_id( + &mut self, + turn_id: &str, + request_id: RequestId, + ) { + self.pending_user_input_request_ids + .entry(turn_id.to_string()) + .or_default() + .push_back(request_id); + } + + pub(super) fn pop_pending_user_input_request_id(&mut self, turn_id: &str) -> Option { + let mut remove_turn_id = false; + let request_id = self + .pending_user_input_request_ids + .get_mut(turn_id) + .and_then(|request_ids| { + let request_id = request_ids.pop_front(); + if request_ids.is_empty() { + remove_turn_id = true; + } + request_id + }); + if remove_turn_id { + self.pending_user_input_request_ids.remove(turn_id); + } + request_id + } + async fn route_thread_update(&mut self, update: ThreadUpdate) { let Some(thread_id_text) = update.thread_id() else { self.handle_thread_update_now(update); diff --git a/codex-rs/tui/src/onboarding/onboarding_screen.rs b/codex-rs/tui/src/onboarding/onboarding_screen.rs index ca6c09282f..81360ee957 100644 --- a/codex-rs/tui/src/onboarding/onboarding_screen.rs +++ b/codex-rs/tui/src/onboarding/onboarding_screen.rs @@ -460,7 +460,9 @@ pub(crate) async fn run_onboarding_app( &mut onboarding_screen, ).await; } else { - break; + return Err(eyre!( + "onboarding app server event stream closed before onboarding completed" + )); } } }