diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 89b502af3f..ca89196d43 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -3392,6 +3392,7 @@ impl App { thread_id, destination, prompt, + expected_name, fallback_title, effort, result, @@ -3401,6 +3402,7 @@ impl App { thread_id, destination, prompt, + expected_name, fallback_title, effort, result, diff --git a/codex-rs/tui/src/app/tests/side_chat_policy_tests.rs b/codex-rs/tui/src/app/tests/side_chat_policy_tests.rs index 5a83488083..b96d871e86 100644 --- a/codex-rs/tui/src/app/tests/side_chat_policy_tests.rs +++ b/codex-rs/tui/src/app/tests/side_chat_policy_tests.rs @@ -4,11 +4,13 @@ use super::*; use crate::app::session_lifecycle::ThreadAttachPresentation; +use crate::sidebar_chooser::side_chat_label; use codex_app_server_client::AppServerEvent; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use core_test_support::responses; +use pretty_assertions::assert_eq; /// Captures one real model request and waits until its turn can be inherited by a fork. pub(super) async fn ask( @@ -104,3 +106,96 @@ async fn temporary_side_chat_sends_policy_on_its_first_request() -> Result<()> { server.shutdown().await?; Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn saved_side_chat_title_uses_only_its_first_direct_input() -> Result<()> { + let question = "Why is CI failing on Windows?"; + let pixel = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="; + let image = crate::chatwidget::UserMessage { + remote_image_urls: vec![pixel.into()], + .."".into() + }; + let mut labels = Vec::new(); + for message in [question.into(), image] { + let (mut app, mut events, _ops) = make_test_app_with_channels().await; + let model = responses::start_mock_server().await; + app_test_support::MockResponsesConfig::new(&model.uri()) + .enable_feature(Feature::TuiWorkspace) + .write(&app.config.codex_home)?; + app.config = app + .rebuild_config_for_cwd(app.config.cwd.to_path_buf()) + .await?; + let mut server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let started = server.start_thread(&app.config).await?; + let parent = started.session.thread_id; + ask(&mut server, &model, parent, "Prepare a release").await?; + server + .thread_set_name(parent, "Release plan".into()) + .await?; + app.enqueue_primary_thread_session(started.session, started.turns) + .await?; + enable_side_retention(&mut app)?; + let mut tui = crate::tui::test_support::make_test_tui()?; + tui.set_owned_screen(/*owned*/ true)?; + let title_prompt = crate::app::thread_title::thread_title_prompt(question); + responses::mount_response_handler(&model, move |request| { + let answer = if request.message_input_texts("user").contains(&title_prompt) { + r#"{"title":"Windows CI failure"}"# + } else { + "Answer" + }; + responses::sse_response(responses::sse(vec![ + responses::ev_assistant_message("reply", answer), + responses::ev_completed("done"), + ])) + }) + .await; + Box::pin(app.handle_start_side( + &mut tui, + &mut server, + parent, + Some(message), + crate::app_event::SideStartMode::New, + )) + .await?; + + time::timeout(Duration::from_secs(15), async { + let mut child_finished = false; + while !child_finished || !app.pending_thread_titles.is_empty() { + tokio::select! { + Some(event) = events.recv() => { + boxed_future!(app.handle_event(&mut tui, &mut server, event)).await?; + } + Some(event) = server.next_event() => { + if let AppServerEvent::ServerNotification(n) = &event.event + && let ServerNotification::TurnCompleted(turn) = n.as_ref() + && app.side_thread_for_parent(parent) + .is_some_and(|id| id.to_string() == turn.thread_id) + { + child_finished = true; + } + app.handle_backend_event(&server, event).await; + } + } + } + Ok::<_, color_eyre::Report>(()) + }) + .await??; + let saved = server + .list_saved_side_chats(parent, app.config.codex_home.as_path()) + .await?; + let [child] = saved.as_slice() else { + panic!("expected one saved side chat") + }; + assert!(!server.side_chat_fork_names.contains_key(&child.id)); + labels.push(side_chat_label(child).unwrap().to_owned()); + let parent = server.thread_read(parent, /*include_turns*/ false).await?; + assert_eq!(parent.name.as_deref(), Some("Release plan")); + server.shutdown().await?; + } + insta::assert_snapshot!(labels.join("\n"), @r" + Windows CI failure + [Image] + "); + Ok(()) +} diff --git a/codex-rs/tui/src/app/tests/turn_submission.rs b/codex-rs/tui/src/app/tests/turn_submission.rs index 8195cd0246..6900b5541d 100644 --- a/codex-rs/tui/src/app/tests/turn_submission.rs +++ b/codex-rs/tui/src/app/tests/turn_submission.rs @@ -175,6 +175,7 @@ async fn turn_start_failure_is_shown_without_exiting() -> Result<()> { let mut tui = crate::tui::test_support::make_test_tui()?; let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; let thread_id = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000")?; + app_server.side_chat_fork_names.insert(thread_id, None); app.active_thread_id = Some(thread_id); app.chat_widget .handle_thread_session(test_thread_session(thread_id, app.config.cwd.to_path_buf())); @@ -194,6 +195,7 @@ async fn turn_start_failure_is_shown_without_exiting() -> Result<()> { .handle_event(&mut tui, &mut app_server, AppEvent::CodexOp(op)) .await?; + assert!(app_server.side_chat_fork_names.contains_key(&thread_id)); assert!(matches!(control, AppRunControl::Continue)); let error_cell = std::iter::from_fn(|| app_event_rx.try_recv().ok()) .find_map(|event| match event { diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 47b631e091..98aa67ca5b 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -2229,18 +2229,7 @@ impl App { && let ServerNotification::ItemCompleted(notification) = notification.as_ref() && let ThreadItemPayload::UserMessage { content, .. } = ¬ification.item.payload { - Some( - content - .iter() - .filter_map(|item| match item { - codex_app_server_protocol::UserInput::Text { text, .. } => { - Some(crate::user_prompt::extract_prompt_request_with_offset(text).0) - } - _ => None, - }) - .collect::>() - .join(" "), - ) + super::thread_title::user_message_text(content) } else { None }; @@ -2248,7 +2237,6 @@ impl App { self.handle_thread_event_now_recovering_file_changes(event) .await; if let Some(user_message) = automatic_title_user_message - && !user_message.trim().is_empty() && let Some(thread_id) = self.active_thread_id { self.generate_thread_title( @@ -2256,6 +2244,7 @@ impl App { thread_id, ThreadTitleDestination::Automatic, &user_message, + /*expected_name*/ None, ); } if !had_active_modal diff --git a/codex-rs/tui/src/app/thread_title.rs b/codex-rs/tui/src/app/thread_title.rs index d33362705c..dd703f4b02 100644 --- a/codex-rs/tui/src/app/thread_title.rs +++ b/codex-rs/tui/src/app/thread_title.rs @@ -1,7 +1,7 @@ //! Structured-output schema and normalization for generated TUI thread titles. //! //! Automatic titles use the first message until generation completes and as a fallback; -//! the originating thread's saved name takes precedence over an automatic result. Manual +//! a saved side chat may also replace the name it inherited at fork. Manual //! renames cancel pending requests; canceled events cannot affect newer requests. use super::App; @@ -70,6 +70,7 @@ impl App { thread_id: ThreadId, destination: ThreadTitleDestination, source_text: &str, + expected_name: Option, ) { if app_server.thread_params_mode().settings_owner() == ThreadSettingsOwner::Server { if let ThreadTitleDestination::RenameSuggestion { request_id } = destination { @@ -87,7 +88,9 @@ impl App { let (prompt, fallback_title) = match destination { ThreadTitleDestination::Automatic => { let fallback = fallback_thread_title(source_text); - if let Some(title) = &fallback { + if !self.side_threads.contains_key(&thread_id) + && let Some(title) = &fallback + { self.chat_widget .display_thread_name(thread_id, title.clone()); } @@ -135,6 +138,7 @@ impl App { thread_id, destination, prompt, + expected_name, fallback_title, effort, result, @@ -150,6 +154,7 @@ impl App { thread_id: ThreadId, destination: ThreadTitleDestination, prompt: String, + expected_name: Option, fallback_title: Option, effort: Option, result: Result, @@ -165,7 +170,7 @@ impl App { return; } let save_name = (destination == ThreadTitleDestination::Automatic) - .then(|| app_server.automatic_thread_name_writer(thread_id)); + .then(|| app_server.automatic_thread_name_writer(thread_id, expected_name)); let temporary_thread_id = match result .and_then(|id| ThreadId::from_string(&id).map_err(|error| error.to_string())) { @@ -278,6 +283,7 @@ impl App { thread_id, ThreadTitleDestination::RenameSuggestion { request_id }, &conversation, + /*expected_name*/ None, ); } } @@ -354,6 +360,20 @@ pub(super) fn thread_title_prompt(user_message: &str) -> String { format!("{prefix}{user_message}") } +pub(super) fn user_message_text(content: &[UserInput]) -> Option { + let text = content + .iter() + .filter_map(|item| match item { + UserInput::Text { text, .. } => { + Some(crate::user_prompt::extract_prompt_request_with_offset(text).0) + } + _ => None, + }) + .collect::>() + .join(" "); + (!text.trim().is_empty()).then_some(text) +} + /// Format recent substantive messages chronologically without trusting their markup. pub(super) fn recent_conversation_messages<'a, I>(items: I) -> Option where diff --git a/codex-rs/tui/src/app/thread_title_tests.rs b/codex-rs/tui/src/app/thread_title_tests.rs index f1788ca4c4..6755717850 100644 --- a/codex-rs/tui/src/app/thread_title_tests.rs +++ b/codex-rs/tui/src/app/thread_title_tests.rs @@ -109,6 +109,7 @@ async fn cloud_conversations_skip_tui_generated_titles() { thread_id, ThreadTitleDestination::Automatic, "Fix the login timeout", + /*expected_name*/ None, ); assert!(app.pending_thread_titles.is_empty()); @@ -118,10 +119,12 @@ async fn cloud_conversations_skip_tui_generated_titles() { #[tokio::test] async fn automatic_thread_title_respects_origin_metadata_after_switching() -> color_eyre::Result<()> { - for (switch_threads, manual_name) in [ - (true, None), - (true, Some("Manual title")), - (false, Some("Manual title")), + for (switch_threads, fork_name, manual_name) in [ + (true, None, None), + (true, Some("Release plan"), None), + (true, Some("Release plan"), Some("Manual title")), + (true, None, Some("Manual title")), + (false, None, Some("Manual title")), ] { let mut app = make_test_app().await; let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; @@ -141,7 +144,13 @@ async fn automatic_thread_title_respects_origin_metadata_after_switching() -> co ); let mut tui = crate::tui::test_support::make_test_tui()?; - let automatic = app_server.automatic_thread_name_writer(thread_id); + if let Some(name) = fork_name { + app_server + .thread_set_name(thread_id, name.to_owned()) + .await?; + } + let automatic = + app_server.automatic_thread_name_writer(thread_id, fork_name.map(str::to_owned)); if switch_threads { let second = app_server.start_thread(&app.config).await?; let second_id = second.session.thread_id; @@ -532,7 +541,7 @@ async fn canceled_thread_title_ignores_late_start_and_completion() -> color_eyre app.pending_thread_titles .insert((thread_id, destination), cancellation.clone()); - let writer = app_server.automatic_thread_name_writer(thread_id); + let writer = app_server.automatic_thread_name_writer(thread_id, /*expected_name*/ None); let token = cancellation.clone(); let mut automatic = std::pin::pin!(writer.save("Stale title".into(), &token)); assert!(automatic.as_mut().now_or_never().is_none()); @@ -558,6 +567,7 @@ async fn canceled_thread_title_ignores_late_start_and_completion() -> color_eyre thread_id, destination, "prompt".to_string(), + /*expected_name*/ None, /*fallback_title*/ None, /*effort*/ None, Ok(temporary_thread_id.to_string()), @@ -603,7 +613,7 @@ async fn manual_rename_waits_for_an_in_flight_automatic_write() -> color_eyre::R codex_app_server_client::AppServerClient::Cloud(client), crate::app_server_session::ThreadParamsMode::Remote, ); - let writer = session.automatic_thread_name_writer(thread_id); + let writer = session.automatic_thread_name_writer(thread_id, /*expected_name*/ None); let token = CancellationToken::new(); let mut automatic = std::pin::pin!(writer.save("Generated".into(), &token)); assert!(automatic.as_mut().now_or_never().is_none()); @@ -915,6 +925,7 @@ async fn failed_thread_title_requests_are_removed() -> color_eyre::Result<()> { thread_id, ThreadTitleDestination::Automatic, "prompt".to_string(), + /*expected_name*/ None, /*fallback_title*/ None, /*effort*/ None, Err("startup failed".to_string()), @@ -925,6 +936,7 @@ async fn failed_thread_title_requests_are_removed() -> color_eyre::Result<()> { thread_id, suggestion, "prompt".to_string(), + /*expected_name*/ None, /*fallback_title*/ None, /*effort*/ None, Ok("invalid-thread-id".to_string()), diff --git a/codex-rs/tui/src/app/user_turn_submission.rs b/codex-rs/tui/src/app/user_turn_submission.rs index 174c4ec1c0..90c0a70448 100644 --- a/codex-rs/tui/src/app/user_turn_submission.rs +++ b/codex-rs/tui/src/app/user_turn_submission.rs @@ -17,6 +17,7 @@ pub(super) async fn try_submit_user_turn( ) -> Result { let AppCommand::UserTurn { turn_trigger, + items, cloud_submission, client_user_message_id, .. @@ -94,6 +95,20 @@ pub(super) async fn try_submit_user_turn( match result { Ok((admission, selection)) => { + if *turn_trigger == "user" + && !items.is_empty() + && matches!(admission, UserMessageAdmission::StartResponse(_)) + && let Some(fork_name) = app_server.side_chat_fork_names.remove(&thread_id) + && let Some(text) = super::thread_title::user_message_text(items) + { + app.generate_thread_title( + app_server, + thread_id, + ThreadTitleDestination::Automatic, + &text, + fork_name, + ); + } if let Some(selection) = selection { if selection.model.is_some() { app.chat_widget.cloud_picker_selection = selection.model; diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index aae7d5f5bb..9ccf41bf75 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -441,6 +441,7 @@ pub(crate) enum AppEvent { thread_id: ThreadId, destination: ThreadTitleDestination, prompt: String, + expected_name: Option, fallback_title: Option, effort: Option, result: Result, diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index d2676ef21b..272a4d286c 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -396,6 +396,8 @@ pub(crate) struct AppServerSession { request_id_namespace: Option, next_request_id: i64, automatic_name_writes: HashMap>, + /// Newly forked saved side chats and the names their first direct turn may replace. + pub(crate) side_chat_fork_names: HashMap>, /// The newest docked replay refresh on this connection. pub(crate) pending_side_skills_refresh: Option, history_pagination: HashMap, @@ -430,6 +432,7 @@ pub(crate) struct AutomaticThreadName { session: AppServerSession, thread_id: ThreadId, gate: Arc, + expected_name: Option, } impl AutomaticThreadName { @@ -440,7 +443,7 @@ impl AutomaticThreadName { let Some(Ok(thread)) = cancellation.run_until_cancelled(read).await else { return; }; - if thread.name.is_some() { + if thread.name != self.expected_name { return; } let Ok(_permit) = self.gate.acquire().await else { @@ -618,6 +621,7 @@ impl AppServerSession { request_id_namespace: None, next_request_id: 1, automatic_name_writes: HashMap::new(), + side_chat_fork_names: HashMap::new(), pending_side_skills_refresh: None, attachment_subscribed: None, history_pagination: HashMap::new(), @@ -1373,6 +1377,11 @@ impl AppServerSession { } pub(crate) fn observe_sidebar_fork(&mut self, fork: &SidebarFork) { + if fork.storage == SideChatStorage::Attached && fork.boundary.is_ok() { + let child = &fork.thread.session; + self.side_chat_fork_names + .insert(child.thread_id, child.thread_name.clone()); + } if fork.legacy { self.history_support = ThreadHistorySupport::LegacyOnly; } @@ -1736,6 +1745,7 @@ impl AppServerSession { }) .await .wrap_err("failed to delete session")?; + self.side_chat_fork_names.remove(&thread_id); Ok(()) } @@ -1953,6 +1963,7 @@ impl AppServerSession { pub(crate) fn automatic_thread_name_writer( &mut self, thread_id: ThreadId, + expected_name: Option, ) -> AutomaticThreadName { self.automatic_name_writes .retain(|_, gate| Arc::strong_count(gate) > 1); @@ -1965,6 +1976,7 @@ impl AppServerSession { session, thread_id, gate: Arc::clone(gate), + expected_name, } } @@ -1990,6 +2002,7 @@ impl AppServerSession { }) .await .wrap_err("thread/name/set failed in TUI")?; + self.side_chat_fork_names.remove(&thread_id); if let Some(gate) = gate.as_ref() { gate.close(); self.automatic_name_writes.remove(&thread_id);