From 08e49689b88cd9a6dd617967da1ff859fb17aaec Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 11 Sep 2026 02:57:02 +0000 Subject: [PATCH] Return to the command center after session cancellation or deletion (#44711) ## Why Deleting a session, canceling an archived session resume or fork, and pressing `Esc` in a conversation open in another app previously exited the TUI. Local daemon and remote connections can instead return to the command center to select another conversation. ## What changed - Return to the command center after successful deletion on local daemon and remote connections. Shut down side conversations before deletion and clear the deleted thread's local state. - Return to the command center when an unarchive prompt is canceled, discarding startup input intended for the canceled resume or fork. Keep explicit quit actions and embedded server exit behavior. - Make `Esc` open the command center from a conversation open in another app. Reselecting that conversation preserves its read-only snapshot and draft; `R` explicitly retries attachment. - Update deletion confirmation text and keyboard hints to reflect these destinations. ## Testing Add lifecycle tests for deletion success and failure, side conversation cleanup, unarchive cancellation and quitting, and read-only snapshot preservation. Add snapshots for command center navigation hints and deletion confirmation. GitOrigin-RevId: 55a96358edb7af73949877a515614943bdc73643 --- codex-rs/tui/src/app/agents_overview.rs | 4 +- codex-rs/tui/src/app/event_dispatch.rs | 47 +++- codex-rs/tui/src/app/input.rs | 7 + codex-rs/tui/src/app/startup.rs | 70 +++-- .../app/tests/session_lifecycle_requests.rs | 254 ++++++++++++++---- ...p__tests__command_popup_default_items.snap | 2 +- codex-rs/tui/src/chatwidget/rendering.rs | 25 +- .../tui/src/chatwidget/rendering_tests.rs | 22 ++ codex-rs/tui/src/chatwidget/slash_dispatch.rs | 7 +- ...s__external_writer_command_center_100.snap | 10 + ...ts__external_writer_command_center_60.snap | 11 + ...sh_delete_confirmation_command_center.snap | 12 + .../src/chatwidget/tests/slash_commands.rs | 39 ++- codex-rs/tui/src/session_start.rs | 26 +- codex-rs/tui/src/session_start_tests.rs | 88 ++++-- codex-rs/tui/src/slash_command.rs | 2 +- codex-rs/tui/src/unarchive_prompt.rs | 7 +- codex-rs/tui/src/unarchive_prompt_tests.rs | 9 +- 18 files changed, 500 insertions(+), 142 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_100.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_60.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_delete_confirmation_command_center.snap diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 336f1a0318..0e78fedba3 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -334,8 +334,10 @@ impl App { root_thread_id: ThreadId, ) -> color_eyre::Result { if self.current_displayed_thread_id() == Some(root_thread_id) - && !self.thread_unavailable(root_thread_id) + && (!self.thread_unavailable(root_thread_id) + || self.chat_widget.is_external_writer_view()) { + // Keep the displayed read-only snapshot frozen; R explicitly retries attachment. if let Ok(mut state) = self.agents_overview.view_state.lock() { state.completion = Some(crate::bottom_pane::ViewCompletion::Accepted); } diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 6f1f808b87..e0257f2dde 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -389,7 +389,7 @@ impl App { return self.archive_current_thread(tui, app_server).await; } AppEvent::DeleteCurrentThread => { - return Ok(self.delete_current_thread(app_server).await); + return self.delete_current_thread(tui, app_server).await; } AppEvent::ForkCurrentSession { name } => { self.session_telemetry.counter( @@ -3540,28 +3540,61 @@ impl App { pub(super) async fn delete_current_thread( &mut self, + tui: &mut tui::Tui, app_server: &mut AppServerSession, - ) -> AppRunControl { + ) -> Result { let Some(thread_id) = self.active_thread_id.or(self.chat_widget.thread_id()) else { self.chat_widget .add_error_message("A thread must start before it can be deleted.".to_string()); - return AppRunControl::Continue; + return Ok(AppRunControl::Continue); }; if self.side_threads.contains_key(&thread_id) { self.chat_widget.add_error_message( "'/delete' is unavailable in side conversations. Press Ctrl+C to return to the main thread first." .to_string(), ); - return AppRunControl::Continue; + return Ok(AppRunControl::Continue); } - match app_server.thread_delete(thread_id).await { - Ok(()) => AppRunControl::Exit(ExitReason::ThreadRemoved), + if !matches!(self.app_server_target, AppServerTarget::Embedded) { + self.shutdown_side_threads(app_server).await; + if !self.side_threads.is_empty() { + return Ok(AppRunControl::Continue); + } + } + + Ok(match app_server.thread_delete(thread_id).await { + Ok(()) if matches!(self.app_server_target, AppServerTarget::Embedded) => { + AppRunControl::Exit(ExitReason::ThreadRemoved) + } + Ok(()) => { + self.track_agents_overview_notification(&ServerNotification::ThreadDeleted( + codex_app_server_protocol::ThreadDeletedNotification { + thread_id: thread_id.to_string(), + }, + )); + self.discard_thread_local_state(thread_id).await; + self.agents_overview.input_states.remove(&thread_id); + self.agents_overview.dispatched_requests.remove(&thread_id); + self.reset_for_thread_switch(tui)?; + self.pending_thread_switch_resets += 1; + self.app_event_tx + .send(AppEvent::ResetTranscriptForThreadSwitch); + self.reset_thread_event_state(); + let init = self.chatwidget_init_for_forked_or_resumed_thread( + tui, + self.config.clone(), + /*initial_user_message*/ None, + ); + self.replace_chat_widget(ChatWidget::new_with_app_event(init)); + self.open_agents_overview(app_server, AgentsOverviewFocus::List); + AppRunControl::Continue + } Err(err) => { self.chat_widget .add_error_message(format!("Failed to delete current thread: {err}")); AppRunControl::Continue } - } + }) } } diff --git a/codex-rs/tui/src/app/input.rs b/codex-rs/tui/src/app/input.rs index b5350192e1..a4cbedb631 100644 --- a/codex-rs/tui/src/app/input.rs +++ b/codex-rs/tui/src/app/input.rs @@ -252,6 +252,13 @@ impl App { && key_event.kind == KeyEventKind::Press { let modifiers = key_event.modifiers; + if key_event.code == KeyCode::Esc + && modifiers == KeyModifiers::NONE + && !matches!(self.app_server_target, AppServerTarget::Embedded) + { + self.open_agents_overview(app_server, AgentsOverviewFocus::List); + return; + } let quit = match key_event.code { KeyCode::Esc => modifiers == KeyModifiers::NONE, KeyCode::Char('q' | 'Q') => { diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 02356f9181..1faea715d7 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -7,6 +7,7 @@ use super::agents_overview_view::AgentsOverviewFocus; use super::reconnect::ReconnectState; use super::*; use crate::session_start::SessionStartAction; +use crate::session_start::SessionStartOutcome; use crate::session_start::cancel_session_start; use crate::session_start::complete_session_start; use crate::unarchive_prompt::run_unarchive_prompt; @@ -375,7 +376,7 @@ impl App { &session_selection, SessionSelection::StartFresh | SessionSelection::Exit ); - let start_in_agents_overview = + let mut start_in_agents_overview = matches!(&session_selection, SessionSelection::AgentsOverview); let mut read_only_thread = false; let (mut chat_widget, initial_started_thread) = match session_selection { @@ -502,14 +503,15 @@ impl App { }; let resumed = if read_only_thread { match resumed { - Ok(resumed) => resumed, + Ok(resumed) => Some(resumed), Err(err) => return shutdown_on_startup_error(app_server, err).await, } } else { let action = SessionStartAction::Resume(model_settings); - let Some(resumed) = complete_session_start( + let resumed = complete_session_start( &mut app_server, &config, + &app_server_target, &target_session, action, resumed, @@ -518,11 +520,14 @@ impl App { run_unarchive_prompt(tui, target_session.thread_id, action).await }, ) - .await? - else { - return Ok(cancel_session_start(app_server).await); - }; - resumed + .await?; + match resumed { + SessionStartOutcome::Started(started) => Some(*started), + SessionStartOutcome::CommandCenter => None, + SessionStartOutcome::Exit => { + return Ok(cancel_session_start(app_server).await); + } + } }; let init = crate::chatwidget::ChatWidgetInit { local_settings: local_settings.clone(), @@ -535,7 +540,8 @@ impl App { initial_images.clone(), // CLI prompt args are plain strings, so they don't provide element ranges. Vec::new(), - ), + ) + .filter(|_| resumed.is_some()), enhanced_keys_supported, has_chatgpt_account, requires_openai_auth, @@ -552,7 +558,7 @@ impl App { .clone(), session_telemetry: session_telemetry.clone(), }; - (ChatWidget::new_with_app_event(init), Some(resumed)) + (ChatWidget::new_with_app_event(init), resumed) } SessionSelection::Fork(target_session) => { let explicit_permission_override = @@ -598,9 +604,10 @@ impl App { Err(err) => return shutdown_on_startup_error(app_server, err).await, }; let action = SessionStartAction::Fork(permission_mode); - let Some(forked) = complete_session_start( + let forked = complete_session_start( &mut app_server, &config, + &app_server_target, &target_session, action, forked, @@ -609,17 +616,23 @@ impl App { run_unarchive_prompt(tui, target_session.thread_id, action).await }, ) - .await? - else { - return Ok(cancel_session_start(app_server).await); + .await?; + let forked = match forked { + SessionStartOutcome::Started(started) => Some(*started), + SessionStartOutcome::CommandCenter => None, + SessionStartOutcome::Exit => { + return Ok(cancel_session_start(app_server).await); + } }; - if let Some(worktree) = managed_worktree.as_ref() - && let Err(err) = worktree.bind(forked.session.thread_id) - { - return shutdown_on_startup_error(app_server, err).await; - } - if config.model_reasoning_effort.is_none() { - config.model_reasoning_effort = forked.session.reasoning_effort.clone(); + if let Some(forked) = &forked { + if let Some(worktree) = managed_worktree.as_ref() + && let Err(err) = worktree.bind(forked.session.thread_id) + { + return shutdown_on_startup_error(app_server, err).await; + } + if config.model_reasoning_effort.is_none() { + config.model_reasoning_effort = forked.session.reasoning_effort.clone(); + } } let init = crate::chatwidget::ChatWidgetInit { local_settings: local_settings.clone(), @@ -632,7 +645,8 @@ impl App { initial_images.clone(), // CLI prompt args are plain strings, so they don't provide element ranges. Vec::new(), - ), + ) + .filter(|_| forked.is_some()), enhanced_keys_supported, has_chatgpt_account, requires_openai_auth, @@ -649,9 +663,15 @@ impl App { .clone(), session_telemetry: session_telemetry.clone(), }; - (ChatWidget::new_with_app_event(init), Some(forked)) + (ChatWidget::new_with_app_event(init), forked) } }; + let startup_session_cancelled = initial_started_thread.is_none() + && !pending_startup_thread_start + && !start_in_agents_overview; + if startup_session_cancelled { + start_in_agents_overview = true; + } chat_widget.note_rendered_width(tui.terminal.last_known_screen_size.width); chat_widget.remote_connection = remote_connection; chat_widget.set_local_worktree_operations(!crate::uses_remote_workspace_or_environment( @@ -896,7 +916,9 @@ See the Codex keymap documentation for supported actions and examples." { return shutdown_on_startup_error(app_server, err).await; } - let mut pending_startup_draft = Some(startup_draft.into_draft()); + // Input for the cancelled resume/fork must not appear in a later selected session. + let mut pending_startup_draft = + (!startup_session_cancelled).then(|| startup_draft.into_draft()); if app_event_rx.is_empty() && !app.has_queued_startup_protected_request() { app.chat_widget .restore_startup_draft_when_ready(&mut pending_startup_draft); 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 4698065e7f..0a4d5502e1 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -617,58 +617,130 @@ async fn make_history_test_app() -> Result<(App, tempfile::TempDir)> { } #[tokio::test] -async fn deleting_remote_thread_omits_disconnect_guidance() -> Result<()> { - let (mut app, codex_home) = make_history_test_app().await?; - let thread_id = ThreadId::from_string( - &create_fake_rollout( - codex_home.path(), - "2026-01-01T00-00-00", - "2026-01-01T00:00:00Z", - "Saved user message", - Some(app.config.model_provider_id.as_str()), - /*git_info*/ None, - ) - .expect("create rollout"), - )?; - let (mut server, _, proxy) = start_recording_app_server( - &app.config, - /*blocked_thread_list*/ None, - /*failed_thread_name*/ None, - ) - .await?; - let resumed = server - .resume_thread( - &app.local_settings, - app.config.clone(), - thread_id, - crate::app_server_session::ResumeModelSettings::RestoreFromThread, +async fn delete_current_thread_navigates_only_after_success() -> Result<()> { + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500")?; + for target in [ + AppServerTarget::Embedded, + AppServerTarget::LocalDaemon { + endpoint: endpoint.clone(), + }, + AppServerTarget::Remote { endpoint }, + ] { + let (mut app, _codex_home) = make_history_test_app().await?; + let thread_id = + create_history_rollout(&app.config, ThreadHistoryMode::Legacy, "delete me")?; + let (mut server, requests, proxy) = start_recording_app_server( + &app.config, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, ) .await?; - app.app_server_target = AppServerTarget::Remote { - endpoint: crate::resolve_remote_addr("ws://127.0.0.1:4500")?, - }; - app.active_thread_id = Some(thread_id); - app.chat_widget.handle_thread_session(resumed.session); - let mut tui = crate::tui::test_support::make_test_tui()?; - // Keep the large dispatcher future off the Windows test thread's stack. - let AppRunControl::Exit(reason) = - Box::pin(app.handle_event(&mut tui, &mut server, AppEvent::DeleteCurrentThread)).await? - else { - panic!("removing the current thread must exit"); - }; - assert_matches!(reason, ExitReason::ThreadRemoved); - let mut exit_info = app.exit_info(reason); - exit_info.token_usage = TokenUsage { - output_tokens: 2, - total_tokens: 2, - ..Default::default() - }; - assert_eq!( - exit_info.format_exit_messages(/*color_enabled*/ false), - vec!["Token usage: total=2 input=0 output=2".to_string()] - ); - server.shutdown().await?; - proxy.await??; + app.app_server_target = target; + let mut tui = crate::tui::test_support::make_test_tui()?; + let missing_id = ThreadId::new(); + app.active_thread_id = Some(missing_id); + app.chat_widget.insert_str("Retained on failure"); + assert_matches!( + app.delete_current_thread(&mut tui, &mut server).await?, + AppRunControl::Continue + ); + assert_eq!(app.active_thread_id, Some(missing_id)); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "Retained on failure" + ); + assert!(!app.chat_widget.has_active_view()); + + let resumed = server + .resume_thread( + &app.local_settings, + app.config.clone(), + thread_id, + crate::app_server_session::ResumeModelSettings::RestoreFromThread, + ) + .await?; + app.active_thread_id = Some(thread_id); + app.enqueue_primary_thread_session(resumed.session.clone(), resumed.turns) + .await?; + app.chat_widget.handle_thread_session(resumed.session); + let mut side_config = app.config.clone(); + side_config.ephemeral = true; + let side = server + .fork_side_thread(&app.local_settings, side_config.clone(), thread_id) + .await?; + let side_id = side.session.thread_id; + app.side_threads + .insert(side_id, SideThreadState::new(thread_id)); + if !matches!(app.app_server_target, AppServerTarget::Embedded) { + // The recording proxy rejects the next unsubscribe after a failed fork. + side_config.cwd = side_config.cwd.join("failure"); + assert!( + server + .fork_side_thread(&app.local_settings, side_config, thread_id) + .await + .is_err() + ); + assert_matches!( + app.delete_current_thread(&mut tui, &mut server).await?, + AppRunControl::Continue + ); + assert_eq!(app.active_thread_id, Some(thread_id)); + assert!(app.side_threads.contains_key(&side_id)); + assert!(!app.chat_widget.has_active_view()); + assert_eq!(recorded_params(&requests, "thread/delete").len(), 1); + } + let (tx, mut events) = tokio::sync::mpsc::unbounded_channel(); + app.app_event_tx = AppEventSender::new(tx); + let control = + Box::pin(app.handle_event(&mut tui, &mut server, AppEvent::DeleteCurrentThread)) + .await?; + assert!( + server + .thread_read(thread_id, /*include_turns*/ false) + .await + .is_err() + ); + if matches!(app.app_server_target, AppServerTarget::Embedded) { + assert_matches!(control, AppRunControl::Exit(ExitReason::ThreadRemoved)); + assert!(recorded_params(&requests, "thread/unsubscribe").is_empty()); + } else { + assert_matches!(control, AppRunControl::Continue); + assert!(app.side_threads.is_empty()); + assert_eq!( + recorded_params(&requests, "thread/unsubscribe"), + vec![serde_json::json!({"threadId": side_id.to_string()}); 2] + ); + loop { + let event = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), events.recv()) + .await? + .expect("command center refresh"); + let refreshed = matches!(&event, AppEvent::AgentsOverviewThreadsLoaded { .. }); + Box::pin(app.handle_event(&mut tui, &mut server, event)).await?; + if refreshed { + break; + } + } + assert_eq!( + ( + app.active_thread_id, + app.primary_thread_id, + app.chat_widget.thread_id() + ), + (None, None, None) + ); + assert!(app.chat_widget.composer_is_empty()); + assert!(app.chat_widget.has_active_view()); + } + assert_eq!( + recorded_params(&requests, "thread/delete"), + vec![ + serde_json::json!({"threadId": missing_id.to_string()}), + serde_json::json!({"threadId": thread_id.to_string()}), + ] + ); + server.shutdown().await?; + proxy.await??; + } Ok(()) } @@ -4294,3 +4366,85 @@ fn session_lifecycle_avoids_redundant_subagent_metadata_reads() -> Result<()> { mod new_session_tests; #[path = "startup_defaults_tests.rs"] mod startup_defaults_tests; + +#[tokio::test] +async fn external_writer_escape_preserves_snapshot_and_explicit_quits() -> Result<()> { + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500")?; + for target in [ + AppServerTarget::LocalDaemon { + endpoint: endpoint.clone(), + }, + AppServerTarget::Remote { endpoint }, + ] { + let (mut app, _codex_home) = make_history_test_app().await?; + let thread_id = + create_history_rollout(&app.config, ThreadHistoryMode::Legacy, "owned elsewhere")?; + let (mut server, requests, proxy) = start_recording_app_server( + &app.config, + /*blocked_thread_list*/ None, + /*failed_thread_name*/ None, + ) + .await?; + app.app_server_target = target; + app.active_thread_id = Some(thread_id); + app.primary_thread_id = Some(thread_id); + app.ensure_thread_channel(thread_id).mark_external_writer(); + app.chat_widget.show_external_writer_thread(); + app.chat_widget.insert_str("Retained draft"); + let (tx, mut events) = tokio::sync::mpsc::unbounded_channel(); + app.app_event_tx = AppEventSender::new(tx); + let mut tui = crate::tui::test_support::make_test_tui()?; + for key in [ + KeyEvent::new(KeyCode::Char('q'), KeyModifiers::NONE), + KeyEvent::new(KeyCode::Char('Q'), KeyModifiers::SHIFT), + KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL), + ] { + app.handle_key_event(&mut tui, &mut server, key).await; + assert_matches!(events.try_recv(), Ok(AppEvent::Exit(ExitMode::Immediate))); + } + app.handle_key_event( + &mut tui, + &mut server, + KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE), + ) + .await; + assert!(app.chat_widget.has_active_view()); + loop { + let event = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), events.recv()) + .await? + .expect("command center refresh"); + let refreshed = matches!(&event, AppEvent::AgentsOverviewThreadsLoaded { .. }); + Box::pin(app.handle_event(&mut tui, &mut server, event)).await?; + if refreshed { + break; + } + } + assert_matches!( + Box::pin(app.handle_event( + &mut tui, + &mut server, + AppEvent::SelectAgentsOverviewThread { thread_id }, + )) + .await?, + AppRunControl::Continue + ); + assert!(!app.chat_widget.has_active_view()); + assert_eq!(app.active_thread_id, Some(thread_id)); + assert!(app.chat_widget.is_external_writer_view()); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "Retained draft" + ); + assert_eq!( + app.thread_event_channels[&thread_id].attachment(), + ThreadEventAttachment::ExternalWriter + ); + assert!(requests.lock().unwrap().iter().all(|request| matches!( + request.method.as_str(), + "thread/read" | "thread/list" | "thread/loaded/list" | "thread/turns/list" + ))); + server.shutdown().await?; + proxy.await??; + } + Ok(()) +} diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap index 0cadee0486..91b583d43e 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_default_items.snap @@ -17,7 +17,7 @@ expression: commands /rename - rename the current thread /new - start a new chat during a conversation /archive - archive this session -/delete - permanently delete this session and exit +/delete - permanently delete this session /resume - resume a saved chat /fork - fork the current chat /app - continue this session in the Desktop app diff --git a/codex-rs/tui/src/chatwidget/rendering.rs b/codex-rs/tui/src/chatwidget/rendering.rs index 8163ae2549..d24f11411d 100644 --- a/codex-rs/tui/src/chatwidget/rendering.rs +++ b/codex-rs/tui/src/chatwidget/rendering.rs @@ -13,6 +13,7 @@ use ratatui::widgets::Block; use std::cell::Cell; struct ExternalWriterNotice { + command_center_available: bool, transcript_hint: Option, } @@ -87,19 +88,18 @@ impl ExternalWriterNotice { } fn footer_lines(&self, width: u16) -> Vec> { - let mut items = vec![ - ("r".to_string(), "retry".to_string()), - ( - format!( - "{}/{}/{}", - crate::key_hint::plain(KeyCode::Esc).display_label(), - crate::key_hint::ctrl(KeyCode::Char('c')).display_label(), - crate::key_hint::plain(KeyCode::Char('q')).display_label(), - ) - .replace(" + ", "+"), - "exit".to_string(), - ), + let mut items = vec![("r".to_string(), "retry".to_string())]; + let escape = crate::key_hint::plain(KeyCode::Esc).display_label(); + let mut quit_keys = vec![ + crate::key_hint::ctrl(KeyCode::Char('c')).display_label(), + crate::key_hint::plain(KeyCode::Char('q')).display_label(), ]; + if self.command_center_available { + items.push((escape, "command center".to_string())); + } else { + quit_keys.insert(/*index*/ 0, escape); + } + items.push((quit_keys.join("/").replace(" + ", "+"), "exit".to_string())); if let Some(hint) = self.transcript_hint { items.push(( hint.display_label().replace(" + ", "+"), @@ -193,6 +193,7 @@ impl ChatWidget { } let bottom = if self.external_writer_view && !self.bottom_pane.has_active_view() { RenderableItem::Owned(Box::new(ExternalWriterNotice { + command_center_available: self.remote_connection.is_some(), transcript_hint: self.bottom_pane.transcript_shortcut_hint(), })) } else { diff --git a/codex-rs/tui/src/chatwidget/rendering_tests.rs b/codex-rs/tui/src/chatwidget/rendering_tests.rs index 2d7abb97ae..902bc0ec55 100644 --- a/codex-rs/tui/src/chatwidget/rendering_tests.rs +++ b/codex-rs/tui/src/chatwidget/rendering_tests.rs @@ -367,3 +367,25 @@ async fn removing_active_cell_invalidates_layout_before_reusing_its_identity() { render_frame(&widget, /*width*/ 80); assert_eq!(desired_height_calls.load(Ordering::Relaxed), 2); } + +#[tokio::test] +async fn external_writer_notice_offers_command_center_on_shared_servers() { + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500").unwrap(); + for target in [ + crate::AppServerTarget::LocalDaemon { + endpoint: endpoint.clone(), + }, + crate::AppServerTarget::Remote { endpoint }, + ] { + let (mut widget, _sender, _events, _operations) = + make_chatwidget_manual_with_sender().await; + widget.remote_connection = crate::status::remote_connection::remote_connection_status_value( + &target, /*server_version*/ None, + ); + widget.show_external_writer_thread(); + for width in [60, 100] { + let rendered = crate::chatwidget::tests::render_bottom_popup(&widget, width); + insta::assert_snapshot!(format!("external_writer_command_center_{width}"), rendered); + } + } +} diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 147b41d1bb..333f435998 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -227,7 +227,12 @@ impl ChatWidget { ..Default::default() }, SelectionItem { - name: "Yes, delete and exit".to_string(), + name: if self.remote_connection.is_some() { + "Yes, delete and return to command center" + } else { + "Yes, delete and exit" + } + .to_string(), description: Some("Permanently delete this session now".to_string()), actions: vec![Box::new(|tx| { tx.send(AppEvent::DeleteCurrentThread); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_100.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_100.snap new file mode 100644 index 0000000000..3067a2cc1c --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_100.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/chatwidget/rendering_tests.rs +expression: rendered +--- + + + 🔒 This conversation is open in another app R to Retry + Close it there and press R to continue here. + + r retry esc command center ctrl+c/q exit ctrl+t transcript diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_60.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_60.snap new file mode 100644 index 0000000000..de6bbf771e --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__rendering__tests__external_writer_command_center_60.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/rendering_tests.rs +expression: rendered +--- + + + 🔒 This conversation is open in another app R to Retry + Close it there and press R to continue here. + + r retry esc command center ctrl+c/q exit ctrl+t + transcript diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_delete_confirmation_command_center.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_delete_confirmation_command_center.snap new file mode 100644 index 0000000000..5ca9c50a18 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_delete_confirmation_command_center.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/slash_commands.rs +expression: popup +--- + Delete this session? + Cannot be undone. Subagent threads will also be deleted. + +› 1. No, keep this session Return to the current session + 2. Yes, delete and return to command center Permanently delete this session + now + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index e558424b55..bb39b86708 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -3108,20 +3108,31 @@ async fn slash_archive_confirmation_requests_current_thread_archive() { #[tokio::test] async fn slash_delete_confirmation_requests_current_thread_delete() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - - chat.dispatch_command(SlashCommand::Delete); - - assert!(chat.bottom_pane.has_active_view()); - assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); - - let popup = render_bottom_popup(&chat, /*width*/ 80); - assert_chatwidget_snapshot!("slash_delete_confirmation_popup", popup); - - chat.handle_key_event(KeyEvent::from(KeyCode::Down)); - chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); - - assert_matches!(rx.try_recv(), Ok(AppEvent::DeleteCurrentThread)); + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500").unwrap(); + for target in [ + crate::AppServerTarget::Embedded, + crate::AppServerTarget::LocalDaemon { + endpoint: endpoint.clone(), + }, + crate::AppServerTarget::Remote { endpoint }, + ] { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.remote_connection = crate::status::remote_connection::remote_connection_status_value( + &target, /*server_version*/ None, + ); + chat.dispatch_command(SlashCommand::Delete); + assert!(chat.bottom_pane.has_active_view()); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + let popup = render_bottom_popup(&chat, /*width*/ 80); + if matches!(target, crate::AppServerTarget::Embedded) { + assert_chatwidget_snapshot!("slash_delete_confirmation_popup", popup); + } else { + assert_chatwidget_snapshot!("slash_delete_confirmation_command_center", popup); + } + chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_matches!(rx.try_recv(), Ok(AppEvent::DeleteCurrentThread)); + } } #[tokio::test] diff --git a/codex-rs/tui/src/session_start.rs b/codex-rs/tui/src/session_start.rs index 1c562fafb9..045c0e770d 100644 --- a/codex-rs/tui/src/session_start.rs +++ b/codex-rs/tui/src/session_start.rs @@ -53,16 +53,24 @@ impl SessionStartAction { } } +#[derive(Debug)] +pub(crate) enum SessionStartOutcome { + Started(Box), + CommandCenter, + Exit, +} + pub(crate) async fn complete_session_start( app_server: &mut AppServerSession, config: &Config, + app_server_target: &crate::AppServerTarget, target: &SessionTarget, action: SessionStartAction, initial_result: Result, confirm: impl AsyncFnOnce() -> Result, -) -> Result> { +) -> Result { match initial_result { - Ok(started) => return Ok(Some(started)), + Ok(started) => return Ok(SessionStartOutcome::Started(Box::new(started))), Err(err) => { // Match the requested ID as well as the server's archive guidance: an unrelated // startup failure must never cause us to unarchive a session. @@ -75,8 +83,16 @@ pub(crate) async fn complete_session_start( } } - if confirm().await? == UnarchiveChoice::Cancel { - return Ok(None); + match confirm().await? { + UnarchiveChoice::Cancel => { + return Ok(match app_server_target { + crate::AppServerTarget::LocalDaemon { .. } + | crate::AppServerTarget::Remote { .. } => SessionStartOutcome::CommandCenter, + crate::AppServerTarget::Embedded => SessionStartOutcome::Exit, + }); + } + UnarchiveChoice::Quit => return Ok(SessionStartOutcome::Exit), + UnarchiveChoice::Unarchive => {} } app_server @@ -87,7 +103,7 @@ pub(crate) async fn complete_session_start( action .start(app_server, config, target) .await - .map(Some) + .map(|started| SessionStartOutcome::Started(Box::new(started))) .map_err(|err| session_start_error(action.verb(), target, err)) } diff --git a/codex-rs/tui/src/session_start_tests.rs b/codex-rs/tui/src/session_start_tests.rs index 888d0bf237..c05839714b 100644 --- a/codex-rs/tui/src/session_start_tests.rs +++ b/codex-rs/tui/src/session_start_tests.rs @@ -14,9 +14,18 @@ async fn start_session( target: &SessionTarget, action: SessionStartAction, confirm: impl AsyncFnOnce() -> Result, -) -> Result> { +) -> Result { let initial_result = action.start(app_server, config, target).await; - complete_session_start(app_server, config, target, action, initial_result, confirm).await + complete_session_start( + app_server, + config, + &crate::AppServerTarget::Embedded, + target, + action, + initial_result, + confirm, + ) + .await } #[tokio::test] @@ -55,26 +64,61 @@ async fn archived_session_requires_confirmation_before_resume_or_fork() -> Resul let mut app_server = crate::start_embedded_app_server_for_picker(&config).await?; let prompts = Cell::new(0); - let cancelled = start_session(&mut app_server, &config, &target, action, async || { - prompts.set(prompts.get() + 1); - Ok(UnarchiveChoice::Cancel) - }) - .await?; - assert!(cancelled.is_none()); - assert_eq!( - (prompts.get(), archived_path.exists(), active_path.exists()), - (1, true, false) - ); + let endpoint = crate::resolve_remote_addr("ws://127.0.0.1:4500")?; + for server_target in [ + crate::AppServerTarget::LocalDaemon { + endpoint: endpoint.clone(), + }, + crate::AppServerTarget::Remote { endpoint }, + crate::AppServerTarget::Embedded, + ] { + for choice in [UnarchiveChoice::Cancel, UnarchiveChoice::Quit] { + let initial = action.start(&mut app_server, &config, &target).await; + let outcome = complete_session_start( + &mut app_server, + &config, + &server_target, + &target, + action, + initial, + async || { + prompts.set(prompts.get() + 1); + Ok(choice) + }, + ) + .await?; + assert_eq!( + matches!(outcome, SessionStartOutcome::CommandCenter), + choice == UnarchiveChoice::Cancel + && !matches!(server_target, crate::AppServerTarget::Embedded), + ); + assert_eq!( + (archived_path.exists(), active_path.exists()), + (true, false) + ); + assert!( + app_server + .thread_loaded_list(Default::default()) + .await? + .data + .is_empty() + ); + } + } - let started = start_session(&mut app_server, &config, &target, action, async || { - prompts.set(prompts.get() + 1); - Ok(UnarchiveChoice::Unarchive) - }) - .await? - .expect("confirmed session should start"); + assert_eq!(prompts.get(), 6); + let SessionStartOutcome::Started(started) = + start_session(&mut app_server, &config, &target, action, async || { + prompts.set(prompts.get() + 1); + Ok(UnarchiveChoice::Unarchive) + }) + .await? + else { + panic!("confirmed session should start") + }; assert_eq!( (prompts.get(), archived_path.exists(), active_path.exists()), - (2, false, true) + (7, false, true) ); match action { SessionStartAction::Resume(_) => { @@ -92,7 +136,7 @@ async fn archived_session_requires_confirmation_before_resume_or_fork() -> Resul } } - let resumed = start_session( + let SessionStartOutcome::Started(resumed) = start_session( &mut app_server, &config, &target, @@ -100,7 +144,9 @@ async fn archived_session_requires_confirmation_before_resume_or_fork() -> Resul async || panic!("active sessions must not prompt"), ) .await? - .expect("active session should start"); + else { + panic!("active session should start") + }; assert_eq!(resumed.session.thread_id, target.thread_id); let missing = SessionTarget { diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index ecfadd8758..0a3e107b88 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -96,7 +96,7 @@ impl SlashCommand { SlashCommand::Rename => "rename the current thread", SlashCommand::Resume => "resume a saved chat", SlashCommand::Archive => "archive this session", - SlashCommand::Delete => "permanently delete this session and exit", + SlashCommand::Delete => "permanently delete this session", SlashCommand::Clear => "clear the terminal and start a new chat", SlashCommand::Fork => "fork the current chat", SlashCommand::Worktree => "start or continue a conversation in a new worktree", diff --git a/codex-rs/tui/src/unarchive_prompt.rs b/codex-rs/tui/src/unarchive_prompt.rs index 3f51822762..8c26fc4767 100644 --- a/codex-rs/tui/src/unarchive_prompt.rs +++ b/codex-rs/tui/src/unarchive_prompt.rs @@ -26,6 +26,7 @@ use tokio_stream::StreamExt; pub(crate) enum UnarchiveChoice { Unarchive, Cancel, + Quit, } pub(crate) async fn run_unarchive_prompt( @@ -44,7 +45,7 @@ pub(crate) async fn run_unarchive_prompt( guard.draw(&screen)?; loop { let Some(event) = events.next().await else { - return Ok(UnarchiveChoice::Cancel); + return Ok(UnarchiveChoice::Quit); }; guard.tui.screen_size_for_event(&event)?; match event { @@ -140,13 +141,13 @@ impl UnarchivePrompt { if key.modifiers.contains(KeyModifiers::CONTROL) && matches!(key.code, KeyCode::Char('c') | KeyCode::Char('d')) { - return Some(UnarchiveChoice::Cancel); + return Some(UnarchiveChoice::Quit); } match key.code { KeyCode::Up | KeyCode::Down | KeyCode::Char('k' | 'j') => { self.highlighted = match self.highlighted { UnarchiveChoice::Unarchive => UnarchiveChoice::Cancel, - UnarchiveChoice::Cancel => UnarchiveChoice::Unarchive, + UnarchiveChoice::Cancel | UnarchiveChoice::Quit => UnarchiveChoice::Unarchive, }; None } diff --git a/codex-rs/tui/src/unarchive_prompt_tests.rs b/codex-rs/tui/src/unarchive_prompt_tests.rs index b3175f6eeb..d2b20bb715 100644 --- a/codex-rs/tui/src/unarchive_prompt_tests.rs +++ b/codex-rs/tui/src/unarchive_prompt_tests.rs @@ -76,11 +76,16 @@ fn confirmation_requires_an_accept_key() { for key in [ KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE), KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE), - KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL), - KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL), + KeyEvent::new(KeyCode::Char('2'), KeyModifiers::NONE), ] { assert_eq!(screen.handle_key(key), Some(UnarchiveChoice::Cancel)); } + for key in ['c', 'd'] { + assert_eq!( + screen.handle_key(KeyEvent::new(KeyCode::Char(key), KeyModifiers::CONTROL)), + Some(UnarchiveChoice::Quit), + ); + } assert_eq!( screen.handle_key(KeyEvent::new_with_kind( KeyCode::Enter,