diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index b86b59aa5d..107717b98c 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -194,6 +194,7 @@ use tokio::sync::mpsc::error::TryRecvError; use tokio::sync::mpsc::error::TrySendError; use tokio::sync::mpsc::unbounded_channel; use tokio::task::JoinHandle; +use tokio_util::sync::CancellationToken; use toml::Value as TomlValue; use uuid::Uuid; mod agent_message_consolidation; @@ -625,7 +626,7 @@ pub(crate) struct App { realtime_replay_order: VecDeque, temporary_structured_requests: HashMap>, /// Track title generation across thread switches and deduplicate automatic requests. - pending_thread_titles: HashSet<(ThreadId, ThreadTitleDestination)>, + pending_thread_titles: HashMap<(ThreadId, ThreadTitleDestination), CancellationToken>, thread_event_listener_tasks: HashMap>, agent_navigation: AgentNavigationState, agents_overview: agents_overview::AgentsOverviewState, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 5ff2b60dc0..e48049de83 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -2465,7 +2465,10 @@ impl App { } AppEvent::RenameAgentsOverviewThread { thread_id, name } => { match app_server.thread_set_name(thread_id, name.clone()).await { - Ok(()) => self.chat_widget.expect_manual_thread_name(thread_id, name), + Ok(()) => { + self.chat_widget.expect_manual_thread_name(thread_id, name); + self.cancel_thread_title_generation(thread_id); + } Err(error) => { if let Ok(mut state) = self.agents_overview.view_state.lock() { state.input = name; @@ -2483,6 +2486,7 @@ impl App { .await; } AppEvent::ThreadTitleStarted { + cancellation, thread_id, destination, prompt, @@ -2496,9 +2500,11 @@ impl App { prompt, effort, result, + cancellation, ); } AppEvent::GeneratedThreadTitle { + cancellation, thread_id, temporary_thread_id, destination, @@ -2507,6 +2513,9 @@ impl App { self.temporary_structured_requests .remove(&temporary_thread_id); + if cancellation.is_cancelled() { + return Ok(AppRunControl::Continue); + } self.finish_thread_title_generation(thread_id, destination); match destination { ThreadTitleDestination::Automatic => { diff --git a/codex-rs/tui/src/app/recap.rs b/codex-rs/tui/src/app/recap.rs index 2a8c820b6e..4f76d144bd 100644 --- a/codex-rs/tui/src/app/recap.rs +++ b/codex-rs/tui/src/app/recap.rs @@ -3,6 +3,7 @@ use std::time::Duration; use std::time::Instant; +use tokio_util::sync::CancellationToken; use super::App; use crate::app_event::AppEvent; @@ -333,6 +334,7 @@ impl App { recap_output_schema(), /*effort*/ None, receiver, + CancellationToken::new(), ) .await .map_err(|error| error.to_string()); diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index 24ce138de4..632d486a79 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -311,7 +311,9 @@ impl App { self.pending_plugin_enabled_writes.clear(); self.pending_hook_enabled_writes.clear(); self.temporary_structured_requests.clear(); - self.pending_thread_titles.clear(); + for (_, cancellation) in self.pending_thread_titles.drain() { + cancellation.cancel(); + } self.sync_thread_title_progress(); self.agents_overview.dispatched_requests.clear(); self.agents_overview.request_id = None; diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__thread_title__tests__manual_rename_cancels_thread_title.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__thread_title__tests__manual_rename_cancels_thread_title.snap new file mode 100644 index 0000000000..a37e03f7fd --- /dev/null +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__thread_title__tests__manual_rename_cancels_thread_title.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/app/thread_title_tests.rs +assertion_line: 363 +expression: normalize_snapshot_paths(popup) +--- +› Ask Codex to do anything + + gpt-5.2 default · /tmp/project · Keep this title diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 038925294c..ae79ecc642 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -785,7 +785,7 @@ See the Codex keymap documentation for supported actions and examples." pending_realtime_transcript_replay: HashMap::new(), realtime_replay_order: VecDeque::new(), temporary_structured_requests: HashMap::new(), - pending_thread_titles: HashSet::new(), + pending_thread_titles: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), agents_overview: Default::default(), diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 2748a624b7..62fbdbe72b 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -69,7 +69,7 @@ pub(super) async fn make_test_app() -> App { pending_realtime_transcript_replay: HashMap::new(), realtime_replay_order: VecDeque::new(), temporary_structured_requests: HashMap::new(), - pending_thread_titles: HashSet::new(), + pending_thread_titles: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), agents_overview: Default::default(), diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index d939bdc1bf..7e78acd8c7 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -5914,7 +5914,7 @@ async fn make_test_app() -> App { pending_realtime_transcript_replay: HashMap::new(), realtime_replay_order: VecDeque::new(), temporary_structured_requests: HashMap::new(), - pending_thread_titles: HashSet::new(), + pending_thread_titles: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), pending_server_profiles: HashMap::new(), @@ -6013,7 +6013,7 @@ pub(super) async fn make_test_app_with_channels() -> ( pending_realtime_transcript_replay: HashMap::new(), realtime_replay_order: VecDeque::new(), temporary_structured_requests: HashMap::new(), - pending_thread_titles: HashSet::new(), + pending_thread_titles: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), pending_server_profiles: HashMap::new(), diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 90615d49c2..1e15836342 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -908,6 +908,7 @@ impl App { let name = name.to_string(); app_server.thread_set_name(thread_id, name.clone()).await?; self.chat_widget.expect_manual_thread_name(thread_id, name); + self.cancel_thread_title_generation(thread_id); Ok(true) } AppCommand::Review { target } => { diff --git a/codex-rs/tui/src/app/thread_title.rs b/codex-rs/tui/src/app/thread_title.rs index 1a699e8fb4..d2da668448 100644 --- a/codex-rs/tui/src/app/thread_title.rs +++ b/codex-rs/tui/src/app/thread_title.rs @@ -1,7 +1,8 @@ //! Structured-output schema and normalization for generated TUI thread titles. //! //! Automatic titles are persisted only after generation, including for inactive threads; -//! the originating thread's saved name takes precedence over an automatic result. +//! the originating thread's saved name takes precedence over an automatic result. Manual +//! renames cancel pending requests; canceled events cannot affect newer requests. use super::App; use super::thread_events::ThreadBufferedEvent; @@ -11,6 +12,7 @@ use crate::app_server_session::AppServerSession; use crate::temporary_structured_request::TemporaryStructuredThreadOptions; use crate::temporary_structured_request::run_temporary_structured_turn; use crate::temporary_structured_request::start_temporary_thread; +use crate::temporary_structured_request::unsubscribe_temporary_thread; use codex_app_server_protocol::ServerNotification; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::UserInput; @@ -21,6 +23,7 @@ use serde::Deserialize; use serde_json::Value; use serde_json::json; use tokio::sync::mpsc; +use tokio_util::sync::CancellationToken; pub(super) const THREAD_TITLE_MAX_CHARS: usize = 36; const THREAD_TITLE_MODEL: &str = "gpt-5.6-luna"; @@ -37,7 +40,7 @@ impl App { pub(super) fn sync_thread_title_progress(&mut self) { let pending = self.chat_widget.thread_id().is_some_and(|thread_id| { self.pending_thread_titles - .iter() + .keys() .any(|(id, _)| *id == thread_id) }); self.chat_widget @@ -53,6 +56,19 @@ impl App { self.sync_thread_title_progress(); } + /// A saved manual name supersedes any pending title generation for that thread. + pub(super) fn cancel_thread_title_generation(&mut self, thread_id: ThreadId) { + self.pending_thread_titles.retain(|(id, _), cancellation| { + if *id == thread_id { + cancellation.cancel(); + false + } else { + true + } + }); + self.sync_thread_title_progress(); + } + /// Start a hidden title-generation thread without blocking the UI loop. pub(super) fn generate_thread_title( &mut self, @@ -61,9 +77,12 @@ impl App { destination: ThreadTitleDestination, prompt: String, ) { - if !self.pending_thread_titles.insert((thread_id, destination)) { + let std::collections::hash_map::Entry::Vacant(entry) = + self.pending_thread_titles.entry((thread_id, destination)) + else { return; - } + }; + let cancellation = entry.insert(CancellationToken::new()).clone(); self.sync_thread_title_progress(); let request_handle = app_server.request_handle(); let model = if self.chat_widget.config_ref().model_provider_id == "openai" @@ -99,6 +118,7 @@ impl App { .map_err(|error| error.to_string()); event_sender.send(AppEvent::ThreadTitleStarted { + cancellation, thread_id, destination, prompt, @@ -109,6 +129,7 @@ impl App { } /// Register a started hidden thread and generate its structured title. + #[allow(clippy::too_many_arguments)] pub(super) fn on_thread_title_started( &mut self, app_server: &AppServerSession, @@ -117,7 +138,17 @@ impl App { prompt: String, effort: Option, result: Result, + cancellation: CancellationToken, ) { + if cancellation.is_cancelled() { + if let Ok(temporary_thread_id) = result { + let request_handle = app_server.request_handle(); + tokio::spawn(async move { + unsubscribe_temporary_thread(&request_handle, temporary_thread_id).await; + }); + } + return; + } let temporary_thread_id_text = match result { Ok(thread_id) => thread_id, Err(error) => { @@ -155,11 +186,13 @@ impl App { thread_title_output_schema(), effort, receiver, + cancellation.clone(), ) .await .map_err(|error| error.to_string()); event_sender.send(AppEvent::GeneratedThreadTitle { + cancellation, thread_id, temporary_thread_id, destination, diff --git a/codex-rs/tui/src/app/thread_title_tests.rs b/codex-rs/tui/src/app/thread_title_tests.rs index c5ee38daa5..437643a9d5 100644 --- a/codex-rs/tui/src/app/thread_title_tests.rs +++ b/codex-rs/tui/src/app/thread_title_tests.rs @@ -8,10 +8,12 @@ use super::thread_title_prompt; use crate::app::session_lifecycle::ThreadAttachPresentation; use crate::app::test_support::make_test_app; use crate::app::thread_events::ThreadBufferedEvent; +use crate::app_command::AppCommand; use crate::app_event::AppEvent; use crate::app_event::ThreadTitleDestination; use crate::app_event_sender::AppEventSender; use crate::app_server_session::ResumeModelSettings; +use crate::chatwidget::tests::helpers::normalize_snapshot_paths; use crate::chatwidget::tests::helpers::render_bottom_popup; use crate::test_support::PathBufExt; use codex_app_server_client::AppServerEvent; @@ -30,6 +32,7 @@ use crossterm::event::KeyModifiers; use pretty_assertions::assert_eq; use tempfile::tempdir; use tokio::sync::mpsc::unbounded_channel; +use tokio_util::sync::CancellationToken; const EXPECTED_THREAD_TITLE_INSTRUCTIONS: &str = concat!( "Generate a concise, single-line task title of at most 36 characters ", @@ -105,8 +108,10 @@ async fn automatic_thread_title_respects_origin_metadata_after_switching() -> co .await?; app.enqueue_primary_thread_session(started.session, started.turns) .await?; - app.pending_thread_titles - .insert((thread_id, ThreadTitleDestination::Automatic)); + app.pending_thread_titles.insert( + (thread_id, ThreadTitleDestination::Automatic), + CancellationToken::new(), + ); let mut tui = crate::tui::test_support::make_test_tui()?; if switch_threads { @@ -131,6 +136,7 @@ async fn automatic_thread_title_respects_origin_metadata_after_switching() -> co &mut tui, &mut app_server, AppEvent::GeneratedThreadTitle { + cancellation: CancellationToken::new(), thread_id, temporary_thread_id: codex_protocol::ThreadId::new(), destination: ThreadTitleDestination::Automatic, @@ -168,25 +174,48 @@ async fn automatic_thread_title_respects_origin_metadata_after_switching() -> co #[tokio::test] async fn slash_rename_generates_editable_title_through_embedded_app_server() -> color_eyre::Result<()> { - check_thread_title_generation(/*automatic*/ false).await + check_thread_title_generation(TitleScenario::Suggestion).await } #[tokio::test] async fn automatic_thread_title_generates_without_a_provisional_name() -> color_eyre::Result<()> { - check_thread_title_generation(/*automatic*/ true).await + check_thread_title_generation(TitleScenario::Automatic).await } -async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<()> { +#[tokio::test] +async fn manual_rename_cancels_running_thread_title() -> color_eyre::Result<()> { + check_thread_title_generation(TitleScenario::ManualRename).await +} + +#[tokio::test] +async fn overview_rename_cancels_running_thread_title() -> color_eyre::Result<()> { + check_thread_title_generation(TitleScenario::OverviewRename).await +} + +#[derive(Clone, Copy)] +enum TitleScenario { + Automatic, + Suggestion, + ManualRename, + OverviewRename, +} + +async fn check_thread_title_generation(scenario: TitleScenario) -> color_eyre::Result<()> { + let automatic = !matches!(scenario, TitleScenario::Suggestion); + let cancel = matches!( + scenario, + TitleScenario::ManualRename | TitleScenario::OverviewRename + ); let server = wiremock::MockServer::start().await; - let response = responses::mount_sse_once( - &server, - responses::sse(vec![ - responses::ev_response_created("title-response"), - responses::ev_assistant_message("title-message", r#"{"title":"Fix login timeout"}"#), - responses::ev_completed("title-response"), - ]), - ) - .await; + let mut template = responses::sse_response(responses::sse(vec![ + responses::ev_response_created("title-response"), + responses::ev_assistant_message("title-message", r#"{"title":"Fix login timeout"}"#), + responses::ev_completed("title-response"), + ])); + if cancel { + template = template.set_delay(std::time::Duration::from_secs(/*secs*/ 60)); + } + let response = responses::mount_response_once(&server, template).await; let codex_home = tempdir()?; let provider_id = "thread-title-test"; std::fs::write( @@ -206,6 +235,7 @@ async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<() let mut app = make_test_app().await; let (event_tx, mut event_rx) = unbounded_channel(); + app.local_settings.tui.animations = false; app.app_event_tx = AppEventSender::new(event_tx); app.config.codex_home = codex_home.path().to_path_buf().abs(); app.config.sqlite = @@ -265,7 +295,7 @@ async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<() .await?; } assert_eq!(app.chat_widget.thread_name(), None); - assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); assert_eq!( app_server .thread_read(thread_id, /*include_turns*/ false) @@ -283,13 +313,23 @@ async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<() enum TitleDriveEvent { Ui(Box), Server(AppServerEvent), + RequestStarted, } + let mut renamed = false; + let mut generated = false; + let mut interrupted = false; + let mut temporary_thread_id = None; loop { let event = tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 10), async { tokio::select! { event = event_rx.recv() => event.map(|event| TitleDriveEvent::Ui(Box::new(event))), event = app_server.next_event() => event.map(TitleDriveEvent::Server), + () = async { + while response.requests().is_empty() { + tokio::time::sleep(std::time::Duration::from_millis(/*millis*/ 10)).await; + } + }, if cancel && !renamed => Some(TitleDriveEvent::RequestStarted), } }) .await? @@ -297,29 +337,68 @@ async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<() match event { TitleDriveEvent::Ui(event) => { - let generated = matches!(event.as_ref(), AppEvent::GeneratedThreadTitle { .. }); - app.handle_event(&mut tui, &mut app_server, *event).await?; - if generated { - break; + if let AppEvent::ThreadTitleStarted { result, .. } = event.as_ref() { + temporary_thread_id = + Some(result.as_ref().expect("hidden title thread").clone()); } + generated |= matches!(event.as_ref(), AppEvent::GeneratedThreadTitle { .. }); + app.handle_event(&mut tui, &mut app_server, *event).await?; } TitleDriveEvent::Server(event) => { + if let AppServerEvent::ServerNotification(notification) = &event + && let ServerNotification::TurnCompleted(completed) = notification.as_ref() + && Some(&completed.thread_id) == temporary_thread_id.as_ref() + { + interrupted = completed.turn.status == TurnStatus::Interrupted; + } app.handle_app_server_event(&app_server, event).await; } + TitleDriveEvent::RequestStarted => { + let name = "Keep this title".to_string(); + let event = match scenario { + TitleScenario::ManualRename => { + AppEvent::CodexOp(AppCommand::set_thread_name(name)) + } + TitleScenario::OverviewRename => { + AppEvent::RenameAgentsOverviewThread { thread_id, name } + } + TitleScenario::Automatic | TitleScenario::Suggestion => unreachable!(), + }; + app.handle_event(&mut tui, &mut app_server, event).await?; + assert!(app.pending_thread_titles.is_empty()); + let popup = render_bottom_popup(&app.chat_widget, /*width*/ 120); + assert!(popup.contains("Keep this title")); + assert!(!popup.contains('⠋')); + if matches!(scenario, TitleScenario::ManualRename) { + insta::assert_snapshot!( + "manual_rename_cancels_thread_title", + normalize_snapshot_paths(popup) + ); + } + renamed = true; + } + } + if generated && (!cancel || interrupted) { + break; } } if automatic { + let expected_name = if cancel { + "Keep this title" + } else { + "Fix login timeout" + }; assert_eq!( app.chat_widget.thread_name(), - Some("Fix login timeout".to_string()) + Some(expected_name.to_string()) ); assert_eq!( app_server .thread_read(thread_id, /*include_turns*/ false) .await? .name, - Some("Fix login timeout".to_string()) + Some(expected_name.to_string()) ); assert!(app.pending_thread_titles.is_empty()); } else { @@ -336,7 +415,7 @@ async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<() } assert!(app.temporary_structured_requests.is_empty()); assert!(app.pending_thread_titles.is_empty()); - assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); let request = response.single_request(); assert!( @@ -350,6 +429,74 @@ async fn check_thread_title_generation(automatic: bool) -> color_eyre::Result<() Ok(()) } +#[tokio::test] +async fn canceled_thread_title_ignores_late_start_and_completion() -> color_eyre::Result<()> { + let mut app = make_test_app().await; + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let started = app_server.start_thread(&app.config).await?; + let thread_id = started.session.thread_id; + app.enqueue_primary_thread_session(started.session, started.turns) + .await?; + let hidden = app_server.start_thread(&app.config).await?; + let temporary_thread_id = hidden.session.thread_id; + let cancellation = CancellationToken::new(); + let destination = ThreadTitleDestination::Automatic; + app.pending_thread_titles + .insert((thread_id, destination), cancellation.clone()); + app.sync_thread_title_progress(); + + app.try_submit_active_thread_op_via_app_server( + &mut app_server, + thread_id, + &AppCommand::set_thread_name("Keep this title".to_string()), + ) + .await?; + assert!(cancellation.is_cancelled()); + assert!(app.pending_thread_titles.is_empty()); + + // A canceled producer must not clear a replacement request or launch its hidden turn. + let replacement = CancellationToken::new(); + app.pending_thread_titles + .insert((thread_id, destination), replacement.clone()); + app.on_thread_title_started( + &app_server, + thread_id, + destination, + "prompt".to_string(), + /*effort*/ None, + Ok(temporary_thread_id.to_string()), + cancellation.clone(), + ); + assert!(app.temporary_structured_requests.is_empty()); + let mut tui = crate::tui::test_support::make_test_tui()?; + app.handle_event( + &mut tui, + &mut app_server, + AppEvent::GeneratedThreadTitle { + cancellation, + thread_id, + temporary_thread_id, + destination, + result: Ok(r#"{"title":"Late automatic title"}"#.to_string()), + }, + ) + .await?; + assert!( + app.pending_thread_titles + .contains_key(&(thread_id, destination)) + ); + assert!(!replacement.is_cancelled()); + assert_eq!( + app_server + .thread_read(thread_id, /*include_turns*/ false) + .await? + .name, + Some("Keep this title".to_string()), + ); + app_server.shutdown().await?; + Ok(()) +} + #[test] fn recent_conversation_messages_escape_markup_and_ignore_commentary() { let items = vec![ @@ -600,6 +747,8 @@ fn title_agent_message(id: &str, text: &str, phase: Option) -> Thr async fn thread_title_progress_clears_failed_requests_and_follows_thread_switches() -> color_eyre::Result<()> { let mut app = make_test_app().await; + app.local_settings.tui.animations = false; + app.chat_widget.local_settings.tui.animations = false; let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; let started = app_server.start_thread(&app.config).await?; let thread_id = started.session.thread_id; @@ -609,11 +758,14 @@ async fn thread_title_progress_clears_failed_requests_and_follows_thread_switche request_id: uuid::Uuid::new_v4(), }; app.pending_thread_titles.extend([ - (thread_id, ThreadTitleDestination::Automatic), - (thread_id, suggestion), + ( + (thread_id, ThreadTitleDestination::Automatic), + CancellationToken::new(), + ), + ((thread_id, suggestion), CancellationToken::new()), ]); app.sync_thread_title_progress(); - assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); app.on_thread_title_started( &app_server, @@ -622,8 +774,9 @@ async fn thread_title_progress_clears_failed_requests_and_follows_thread_switche "prompt".to_string(), /*effort*/ None, Err("startup failed".to_string()), + CancellationToken::new(), ); - assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); app.on_thread_title_started( &app_server, thread_id, @@ -631,13 +784,16 @@ async fn thread_title_progress_clears_failed_requests_and_follows_thread_switche "prompt".to_string(), /*effort*/ None, Ok("invalid-thread-id".to_string()), + CancellationToken::new(), ); assert!(app.pending_thread_titles.is_empty()); - assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); let mut tui = crate::tui::test_support::make_test_tui()?; - app.pending_thread_titles - .insert((thread_id, ThreadTitleDestination::Automatic)); + app.pending_thread_titles.insert( + (thread_id, ThreadTitleDestination::Automatic), + CancellationToken::new(), + ); let second = app_server.start_thread(&app.config).await?; let second_id = second.session.thread_id; app.ensure_thread_channel(second_id) @@ -651,18 +807,19 @@ async fn thread_title_progress_clears_failed_requests_and_follows_thread_switche &mut tui, ratatui::layout::Size::new(/*width*/ 120, /*height*/ 30), )?; - assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); app.select_agent_thread(&mut tui, &mut app_server, thread_id) .await?; app.render_chat_widget_frame( &mut tui, ratatui::layout::Size::new(/*width*/ 120, /*height*/ 30), )?; - assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); app.handle_event( &mut tui, &mut app_server, AppEvent::GeneratedThreadTitle { + cancellation: CancellationToken::new(), thread_id, temporary_thread_id: codex_protocol::ThreadId::new(), destination: ThreadTitleDestination::Automatic, @@ -671,7 +828,7 @@ async fn thread_title_progress_clears_failed_requests_and_follows_thread_switche ) .await?; assert!(app.pending_thread_titles.is_empty()); - assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains("renaming...")); + assert!(!render_bottom_popup(&app.chat_widget, /*width*/ 120).contains('⠋')); app_server.shutdown().await?; Ok(()) } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 00023b84f5..61dfd920eb 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -11,6 +11,7 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::AtomicBool; +use tokio_util::sync::CancellationToken; use crate::inline_visualization::InlineVisualizationContext; use codex_app_server_protocol::AddCreditsNudgeCreditType; @@ -303,6 +304,7 @@ pub(crate) enum AppEvent { }, /// Register a hidden title-generation thread started in the background. ThreadTitleStarted { + cancellation: CancellationToken, thread_id: ThreadId, destination: ThreadTitleDestination, prompt: String, @@ -311,6 +313,7 @@ pub(crate) enum AppEvent { }, /// Route a hidden title request to its automatic rename or editable prompt. GeneratedThreadTitle { + cancellation: CancellationToken, thread_id: ThreadId, temporary_thread_id: ThreadId, destination: ThreadTitleDestination, diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__default_footer_generating_thread_title.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__default_footer_generating_thread_title.snap index b4a338a29c..46b2c060d6 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__default_footer_generating_thread_title.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__default_footer_generating_thread_title.snap @@ -4,4 +4,4 @@ expression: "render_bottom_popup(&chat, 100)" --- › Ask Codex to do anything - gpt-5.6-sol default · /tmp/project · renaming... ⠋ + gpt-5.6-sol default · /tmp/project · ⠋ diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__terminal_title__thread_title_generation_status_surfaces.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__terminal_title__thread_title_generation_status_surfaces.snap index e1b2adba20..569f645c21 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__terminal_title__thread_title_generation_status_surfaces.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__terminal_title__thread_title_generation_status_surfaces.snap @@ -8,9 +8,9 @@ title: | 550e8400-e29b-41d4-a716-446655440000 | 550e8400-e29b-41d4-a716-44665.. default title: project name=None, pending=true -status: renaming... ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ -title: renaming... ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ | 550e8400-e29b-41d4-a716-44665... ⠋ -default title: renaming... ⠋ | project +status: ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ +title: ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ | 550e8400-e29b-41d4-a716-44665... ⠋ +default title: ⠋ | project name=Some("Fix login timeout"), pending=true status: Fix login timeout ⠋ | Fix login timeout ⠋ | 550e8400-e29b-41d4-a716-446655440000 ⠋ diff --git a/codex-rs/tui/src/chatwidget/tests/terminal_title.rs b/codex-rs/tui/src/chatwidget/tests/terminal_title.rs index d310031d48..90a4c4c6bc 100644 --- a/codex-rs/tui/src/chatwidget/tests/terminal_title.rs +++ b/codex-rs/tui/src/chatwidget/tests/terminal_title.rs @@ -312,14 +312,14 @@ async fn thread_title_progress_animates_when_main_turn_is_idle() { chat.terminal_title_animation_origin = now; assert_eq!( chat.terminal_title_value_for_item(TerminalTitleItem::ThreadName, now), - Some("renaming... ⠋".to_string()) + Some("⠋".to_string()) ); assert_eq!( chat.terminal_title_value_for_item( TerminalTitleItem::ThreadName, now + Duration::from_millis(/*millis*/ 100) ), - Some("renaming... ⠙".to_string()) + Some("⠙".to_string()) ); chat.refresh_thread_title_progress_for_time_tick(); assert!(draw_rx.try_recv().is_ok()); @@ -335,7 +335,7 @@ async fn thread_title_progress_animates_when_main_turn_is_idle() { chat.local_settings.tui.animations = false; chat.refresh_status_surfaces(); assert!(chat.terminal_title_next_refresh.is_none()); - assert_eq!(chat.last_terminal_title, Some("renaming... ⠋".to_string())); + assert_eq!(chat.last_terminal_title, Some("⠋".to_string())); chat.local_settings.tui.animations = true; chat.set_thread_title_generation_pending(/*pending*/ false); assert_eq!(chat.last_terminal_title, None); diff --git a/codex-rs/tui/src/chatwidget/thread_title_status.rs b/codex-rs/tui/src/chatwidget/thread_title_status.rs index e1c061c464..d6c8a3528d 100644 --- a/codex-rs/tui/src/chatwidget/thread_title_status.rs +++ b/codex-rs/tui/src/chatwidget/thread_title_status.rs @@ -42,8 +42,10 @@ impl ChatWidget { } else { TERMINAL_TITLE_SPINNER_FRAMES[0] }; - let value = value.unwrap_or_else(|| "renaming...".to_string()); - Some(format!("{value} {spinner}")) + Some(match value { + Some(value) => format!("{value} {spinner}"), + None => spinner.to_string(), + }) } pub(crate) fn refresh_thread_title_progress_for_time_tick(&mut self) { diff --git a/codex-rs/tui/src/temporary_structured_request.rs b/codex-rs/tui/src/temporary_structured_request.rs index f421426321..9a1a20678b 100644 --- a/codex-rs/tui/src/temporary_structured_request.rs +++ b/codex-rs/tui/src/temporary_structured_request.rs @@ -16,6 +16,8 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::ThreadUnsubscribeParams; use codex_app_server_protocol::ThreadUnsubscribeResponse; +use codex_app_server_protocol::TurnInterruptParams; +use codex_app_server_protocol::TurnInterruptResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStatus; @@ -25,6 +27,7 @@ use color_eyre::eyre::eyre; use serde_json::Value; use std::time::Duration; use tokio::sync::mpsc::UnboundedReceiver; +use tokio_util::sync::CancellationToken; use uuid::Uuid; const STRUCTURED_TURN_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 30); @@ -263,7 +266,7 @@ pub(crate) async fn unsubscribe_temporary_thread( } } -/// Run a bounded structured turn and make a bounded temporary-thread cleanup attempt. +/// Run a bounded structured turn, interrupt it on cancellation, and detach the temporary thread. pub(crate) async fn run_temporary_structured_turn( request_handle: AppServerRequestHandle, thread_id: String, @@ -271,8 +274,15 @@ pub(crate) async fn run_temporary_structured_turn( output_schema: Value, effort: Option, notifications: UnboundedReceiver, + cancellation: CancellationToken, ) -> color_eyre::Result { + let mut turn_id = None; let result = tokio::time::timeout(STRUCTURED_TURN_TIMEOUT, async { + if cancellation.is_cancelled() { + return Err(eyre!("temporary structured turn cancelled")); + } + // Wait for turn/start to return its ID before interrupting, even if cancellation + // arrives during startup. Dropping that request could leave an orphaned turn. let turn = start_structured_turn( &request_handle, thread_id.clone(), @@ -282,11 +292,40 @@ pub(crate) async fn run_temporary_structured_turn( ) .await?; - collect_structured_response(notifications, &turn.turn.id).await + turn_id = Some(turn.turn.id.clone()); + tokio::select! { + biased; + _ = cancellation.cancelled() => Err(eyre!("temporary structured turn cancelled")), + result = collect_structured_response(notifications, &turn.turn.id) => result, + } }) .await .unwrap_or_else(|_| Err(eyre!("temporary structured turn timed out"))); + // Give interruption its own deadline so cancellation near the response timeout + // still gets a chance to stop the hidden turn. + if cancellation.is_cancelled() + && let Some(turn_id) = turn_id + { + let interrupt = + request_handle.request_typed::(ClientRequest::TurnInterrupt { + request_id: RequestId::String(format!( + "temporary-structured-interrupt-{}", + Uuid::new_v4() + )), + params: TurnInterruptParams { + thread_id: thread_id.clone(), + turn_id, + }, + }); + match tokio::time::timeout(STRUCTURED_TURN_TIMEOUT, interrupt).await { + Ok(Ok(_)) => {} + Ok(Err(error)) => { + tracing::debug!(%error, "failed to interrupt temporary structured turn") + } + Err(_) => tracing::debug!("temporary structured turn interrupt timed out"), + } + } unsubscribe_temporary_thread(&request_handle, thread_id).await; result