codex: address PR review feedback (#26754)

codex-rs/tui/src/app/event_dispatch.rs

@@ -1720,13 +1720,16 @@ impl App {
                 parent_thread_id,
                 user_message,
             } => {
-                self.handle_start_side(app_server, parent_thread_id, user_message)
-                    .await;
+                self.handle_start_side(app_server, parent_thread_id, user_message);
             }
             AppEvent::SideThreadPrepared(request_id, result) => {
                 self.handle_side_thread_prepared(tui, app_server, request_id, result)
                     .await?;
             }
+            AppEvent::SideThreadCleanupFinished(thread_id, result) => {
+                self.handle_side_thread_cleanup_finished(thread_id, result)
+                    .await;
+            }
             AppEvent::OpenSkillsList => {
                 self.chat_widget.open_skills_list();
             }

codex-rs/tui/src/app/side.rs

@@ -550,7 +550,7 @@ impl App {
         Ok(())
     }
 
-    pub(super) async fn handle_start_side(
+    pub(super) fn handle_start_side(
         &mut self,
         app_server: &AppServerSession,
         parent_thread_id: ThreadId,
@@ -576,8 +576,6 @@ impl App {
             side_state: SideThreadState::new(parent_thread_id),
             user_message,
         });
-        self.refresh_in_memory_config_from_disk_best_effort("starting a side conversation")
-            .await;
         let request_handle = app_server.request_handle();
         let thread_params_mode = app_server.thread_params_mode();
         let remote_cwd_override = app_server.remote_cwd_override().map(Path::to_path_buf);
@@ -599,6 +597,41 @@ impl App {
         });
     }
 
+    fn cleanup_side_thread_in_background(
+        &mut self,
+        app_server: &AppServerSession,
+        thread_id: ThreadId,
+    ) {
+        // Hide the canceled thread while cleanup is pending, but retain enough local state to
+        // restore it in /agent if the server refuses to unsubscribe it.
+        self.agent_navigation.remove(thread_id);
+        let request_handle = app_server.request_handle();
+        let app_event_tx = self.app_event_tx.clone();
+        tokio::spawn(async move {
+            let result =
+                super::side_server::unsubscribe_side_thread(request_handle, thread_id).await;
+            app_event_tx.send(AppEvent::SideThreadCleanupFinished(thread_id, result));
+        });
+    }
+
+    pub(super) async fn handle_side_thread_cleanup_finished(
+        &mut self,
+        thread_id: ThreadId,
+        result: std::result::Result<(), String>,
+    ) {
+        self.discard_thread_local_state(thread_id).await;
+        if let Err(err) = result {
+            self.upsert_agent_picker_thread(
+                thread_id, /*agent_nickname*/ None, /*agent_role*/ None,
+                /*is_closed*/ false,
+            );
+            self.chat_widget.add_error_message(format!(
+                "Failed to close side conversation {thread_id}; it remains available in /agent: \
+                 {err}"
+            ));
+        }
+    }
+
     pub(super) async fn handle_side_thread_prepared(
         &mut self,
         tui: &mut tui::Tui,
@@ -617,19 +650,11 @@ impl App {
             .pending_side_start
             .take_if(|pending| pending.request_id == request_id)
         else {
-            if let Some(thread_id) = match &result {
+            if let Some(thread_id) = match result {
                 Ok(started) => Some(started.session.thread_id),
                 Err(err) => err.thread_id,
             } {
-                // ThreadStarted may have created local state before this preparation was canceled.
-                self.discard_thread_local_state(thread_id).await;
-                if result.is_ok() {
-                    let request_handle = app_server.request_handle();
-                    tokio::spawn(super::side_server::unsubscribe_side_thread(
-                        request_handle,
-                        thread_id,
-                    ));
-                }
+                self.cleanup_side_thread_in_background(app_server, thread_id);
             }
             return Ok(());
         };
@@ -677,7 +702,7 @@ impl App {
             }
             Err(err) => {
                 if let Some(thread_id) = err.thread_id {
-                    self.discard_thread_local_state(thread_id).await;
+                    self.cleanup_side_thread_in_background(app_server, thread_id);
                 }
                 self.restore_side_user_message(user_message.take());
                 self.chat_widget

codex-rs/tui/src/app/side_server.rs

@@ -46,8 +46,6 @@ pub(super) async fn prepare_side_thread(
         })
         .await;
     if let Err(err) = inject_result {
-        // The caller only receives fully prepared threads, so clean up this partial fork here.
-        unsubscribe_side_thread(request_handle, child_thread_id).await;
         return Err(SideThreadPrepareError {
             thread_id: Some(child_thread_id),
             message: format!(
@@ -61,19 +59,15 @@ pub(super) async fn prepare_side_thread(
 pub(super) async fn unsubscribe_side_thread(
     request_handle: AppServerRequestHandle,
     thread_id: ThreadId,
-) {
-    let unsubscribe_result = request_handle
+) -> std::result::Result<(), String> {
+    request_handle
         .request_typed::<ThreadUnsubscribeResponse>(ClientRequest::ThreadUnsubscribe {
             request_id: RequestId::String(format!("side-thread-unsubscribe-{}", Uuid::new_v4())),
             params: ThreadUnsubscribeParams {
                 thread_id: thread_id.to_string(),
             },
         })
-        .await;
-    if let Err(err) = unsubscribe_result {
-        tracing::warn!(
-            thread_id = %thread_id,
-            "failed to unsubscribe side thread during cleanup: {err}"
-        );
-    }
+        .await
+        .map(|_| ())
+        .map_err(|err| format!("thread/unsubscribe failed in TUI: {err}"))
 }

codex-rs/tui/src/app/tests.rs

@@ -3401,6 +3401,19 @@ async fn cancelling_pending_side_start_restores_message() {
     );
 }
 
+#[tokio::test]
+async fn failed_side_cleanup_restores_thread_to_agent_picker() {
+    let mut app = make_test_app().await;
+    let thread_id = ThreadId::new();
+    app.ensure_thread_channel(thread_id);
+
+    app.handle_side_thread_cleanup_finished(thread_id, Err("transport disconnected".to_string()))
+        .await;
+
+    assert!(!app.thread_event_channels.contains_key(&thread_id));
+    assert!(app.agent_navigation.get(&thread_id).is_some());
+}
+
 #[tokio::test]
 async fn side_discard_selection_keeps_current_side_thread() {
     let mut app = make_test_app().await;

codex-rs/tui/src/app_event.rs

@@ -155,6 +155,8 @@ pub(crate) enum AppEvent {
     },
     /// Finish preparing a transient side conversation off the TUI event loop.
     SideThreadPrepared(Uuid, Result<AppServerStartedThread, SideThreadPrepareError>),
+    /// Finish removing a canceled or failed side conversation off the TUI event loop.
+    SideThreadCleanupFinished(ThreadId, Result<(), String>),
 
     /// Submit an op to the specified thread, regardless of current focus.
     SubmitThreadOp {