From a1286d12a2a80fccd8e270c12ec32f48431b5722 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 29 Jul 2026 22:20:03 +0000 Subject: [PATCH] Allow naming forked chats from the TUI (#36036) ## What changed - Accept an optional thread name with `/fork`, trimming surrounding whitespace. - Apply the name to the newly forked thread and update the active session metadata. - Keep the successful fork active and show an error when naming it fails. ## Testing - Cover unnamed and named `/fork` dispatch, persisted names, and naming failures. GitOrigin-RevId: 1799576f82ef0856f992a0378f041e4bab89621b --- codex-rs/tui/src/app/event_dispatch.rs | 23 ++++- codex-rs/tui/src/app/tests.rs | 9 +- .../src/app/tests/advanced_reasoning_tests.rs | 8 +- .../app/tests/session_lifecycle_requests.rs | 89 ++++++++++++++++--- codex-rs/tui/src/app_event.rs | 6 +- codex-rs/tui/src/chatwidget/slash_dispatch.rs | 8 +- .../src/chatwidget/tests/slash_commands.rs | 21 ++++- codex-rs/tui/src/slash_command.rs | 1 + 8 files changed, 143 insertions(+), 22 deletions(-) diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index f820122163..626b2001d9 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -162,7 +162,7 @@ impl App { AppEvent::DeleteCurrentThread => { return Ok(self.delete_current_thread(app_server).await); } - AppEvent::ForkCurrentSession => { + AppEvent::ForkCurrentSession { name } => { self.session_telemetry.counter( "codex.thread.fork", /*inc*/ 1, @@ -184,7 +184,23 @@ impl App { fork_config.model_reasoning_effort = self.chat_widget.current_reasoning_effort(); match app_server.fork_thread(fork_config, thread_id).await { - Ok(forked) => { + Ok(mut forked) => { + let name_error = if let Some(name) = name { + match app_server + .thread_set_name(forked.session.thread_id, name.clone()) + .await + { + Ok(()) => { + forked.session.thread_name = Some(name); + None + } + Err(err) => { + Some(format!("Failed to name the forked session: {err}")) + } + } + } else { + None + }; self.shutdown_current_thread(app_server).await; match self .replace_chat_widget_with_app_server_thread( @@ -196,6 +212,9 @@ impl App { .await { Ok(()) => { + if let Some(err) = name_error { + self.chat_widget.add_error_message(err); + } if let Some(summary) = summary { let mut lines: Vec> = Vec::new(); if let Some(usage_line) = summary.usage_line { diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 96fc311973..4bd7a3655f 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -6292,9 +6292,12 @@ async fn in_app_resume_uses_configured_or_explicit_cwd() -> Result<()> { )); assert_eq!(app.chat_widget.thread_id(), Some(thread_id)); - let control = - Box::pin(app.handle_event(&mut tui, &mut app_server, AppEvent::ForkCurrentSession)) - .await?; + let control = Box::pin(app.handle_event( + &mut tui, + &mut app_server, + AppEvent::ForkCurrentSession { name: None }, + )) + .await?; assert!(matches!(control, AppRunControl::Continue)); assert!(!crate::session_resume::cwds_differ( diff --git a/codex-rs/tui/src/app/tests/advanced_reasoning_tests.rs b/codex-rs/tui/src/app/tests/advanced_reasoning_tests.rs index 14e757990a..72f1a7fda3 100644 --- a/codex-rs/tui/src/app/tests/advanced_reasoning_tests.rs +++ b/codex-rs/tui/src/app/tests/advanced_reasoning_tests.rs @@ -28,8 +28,12 @@ async fn fork_current_session_preserves_conversation_ultra() -> Result<()> { let mut tui = crate::tui::test_support::make_test_tui()?; let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; - let control = - Box::pin(app.handle_event(&mut tui, &mut app_server, AppEvent::ForkCurrentSession)).await?; + let control = Box::pin(app.handle_event( + &mut tui, + &mut app_server, + AppEvent::ForkCurrentSession { name: None }, + )) + .await?; assert!(matches!(control, AppRunControl::Continue)); assert_ne!(app.chat_widget.thread_id(), Some(source_thread_id)); diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index e980727a54..b3aa550b09 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -5,6 +5,7 @@ use app_test_support::rollout_path; use codex_app_server_protocol::ClientNotification; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::JSONRPCError; +use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::SortDirection; @@ -40,6 +41,7 @@ fn take_backfill_counts(requests: &Arc>>) -> (usize, usize) { async fn start_recording_app_server( config: &Config, mut blocked_thread_list: Option<(ThreadId, oneshot::Sender<()>, oneshot::Receiver<()>)>, + failed_thread_name: Option<&'static str>, ) -> Result<( AppServerSession, Arc>>, @@ -105,15 +107,31 @@ async fn start_recording_app_server( let _ = started.send(()); let _ = release.await; } - let response = match embedded.request(request).await? { - Ok(result) => JSONRPCMessage::Response(JSONRPCResponse { + let force_failure = matches!( + &request, + ClientRequest::ThreadSetName { params, .. } + if failed_thread_name == Some(params.name.as_str()) + ); + let response = if force_failure { + JSONRPCMessage::Error(JSONRPCError { id: request_id, - result, - }), - Err(error) => JSONRPCMessage::Error(JSONRPCError { - id: request_id, - error, - }), + error: JSONRPCErrorError { + code: -32603, + message: "forced thread/name/set failure".to_string(), + data: None, + }, + }) + } else { + match embedded.request(request).await? { + Ok(result) => JSONRPCMessage::Response(JSONRPCResponse { + id: request_id, + result, + }), + Err(error) => JSONRPCMessage::Error(JSONRPCError { + id: request_id, + error, + }), + } }; websocket .send(Message::Text(serde_json::to_string(&response)?.into())) @@ -166,8 +184,12 @@ fn fresh_session_applies_requested_name() -> Result<()> { let codex_home = tempdir()?; app.config.codex_home = codex_home.path().to_path_buf().abs(); app.config.sqlite = SqliteConfig::new_for_testing(codex_home.path().abs()); - let (mut app_server, requests, proxy) = - start_recording_app_server(&app.config, /*blocked_thread_list*/ None).await?; + let (mut app_server, requests, proxy) = start_recording_app_server( + &app.config, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, + ) + .await?; let mut tui = crate::tui::test_support::make_test_tui()?; app.start_fresh_session_with_summary_hint( @@ -267,6 +289,7 @@ fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> { let (mut app_server, requests, proxy) = start_recording_app_server( &app.config, Some((root_thread_id, started_tx, release_rx)), + Some("Failed Fork"), ) .await?; let root = app_server @@ -291,15 +314,59 @@ fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> { let control = Box::pin(app.handle_event( &mut tui, &mut app_server, - AppEvent::ForkCurrentSession, + AppEvent::ForkCurrentSession { + name: Some("Add User Fork".to_string()), + }, )) .await?; assert!(matches!(control, AppRunControl::Continue)); assert_ne!(app.chat_widget.thread_id(), Some(root_thread_id)); + let named_fork_id = app + .chat_widget + .thread_id() + .expect("named fork should have a thread id"); + assert_eq!( + app.chat_widget.thread_name(), + Some("Add User Fork".to_string()) + ); // Forking may read the source metadata once when the response includes its parent // id. It must not scan or backfill loaded threads for the newly created fork. assert!(matches!(take_backfill_counts(&requests), (0, 0) | (0, 1))); + let named_fork = app_server + .thread_read(named_fork_id, /*include_turns*/ false) + .await?; + assert_eq!(named_fork.name.as_deref(), Some("Add User Fork")); + take_backfill_counts(&requests); + + let control = Box::pin(app.handle_event( + &mut tui, + &mut app_server, + AppEvent::ForkCurrentSession { + name: Some("Failed Fork".to_string()), + }, + )) + .await?; + + assert!(matches!(control, AppRunControl::Continue)); + assert_ne!(app.chat_widget.thread_id(), Some(named_fork_id)); + let name_error = std::iter::from_fn(|| app_event_rx.try_recv().ok()) + .find_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => { + let rendered = + lines_to_single_string(&cell.display_lines(/*width*/ 80)); + rendered + .contains("Failed to name the forked session") + .then_some(rendered) + } + _ => None, + }) + .expect("fork naming error history cell"); + insta::assert_snapshot!( + name_error, + @"■ Failed to name the forked session: thread/name/set failed in TUI" + ); + assert!(matches!(take_backfill_counts(&requests), (0, 0) | (0, 1))); app.start_fresh_session_with_summary_hint( &mut tui, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 989eff7716..a570ee5808 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -287,8 +287,10 @@ pub(crate) enum AppEvent { /// Permanently delete the current active main thread and exit after it succeeds. DeleteCurrentThread, - /// Fork the current session into a new thread. - ForkCurrentSession, + /// Fork the current session into a new thread, optionally assigning it a name. + ForkCurrentSession { + name: Option, + }, /// Branch before a selected prompt and reopen it in the new thread's composer. ForkSessionForPromptEdit { diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 6d14460be1..e0d19d8b89 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -242,7 +242,8 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::OpenResumePicker); } SlashCommand::Fork => { - self.app_event_tx.send(AppEvent::ForkCurrentSession); + self.app_event_tx + .send(AppEvent::ForkCurrentSession { name: None }); } SlashCommand::App => { let Some(thread_id) = self.thread_id else { @@ -744,6 +745,11 @@ impl ChatWidget { name: Some(trimmed.to_string()), }); } + SlashCommand::Fork if !trimmed.is_empty() => { + self.app_event_tx.send(AppEvent::ForkCurrentSession { + name: Some(trimmed.to_string()), + }); + } SlashCommand::Plan if !trimmed.is_empty() => { if !self.apply_plan_slash_command() { return; diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index ee398f432a..cb0ebdebef 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -2547,7 +2547,26 @@ async fn slash_fork_requests_current_fork() { chat.dispatch_command(SlashCommand::Fork); - assert_matches!(rx.try_recv(), Ok(AppEvent::ForkCurrentSession)); + assert_matches!( + rx.try_recv(), + Ok(AppEvent::ForkCurrentSession { name: None }) + ); +} + +#[tokio::test] +async fn slash_fork_with_name_requests_named_fork() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.bottom_pane + .set_composer_text("/fork Add User ".to_string(), Vec::new(), Vec::new()); + + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + assert_matches!( + rx.try_recv(), + Ok(AppEvent::ForkCurrentSession { + name: Some(name) + }) if name == "Add User" + ); } #[tokio::test] diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index abd1386932..07cb2c14da 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -157,6 +157,7 @@ impl SlashCommand { | SlashCommand::Rename | SlashCommand::New | SlashCommand::Clear + | SlashCommand::Fork | SlashCommand::Plan | SlashCommand::Goal | SlashCommand::Ide