mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
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
This commit is contained in:
@@ -334,8 +334,10 @@ impl App {
|
||||
root_thread_id: ThreadId,
|
||||
) -> color_eyre::Result<AppRunControl> {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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<AppRunControl> {
|
||||
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
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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') => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
@@ -13,6 +13,7 @@ use ratatui::widgets::Block;
|
||||
use std::cell::Cell;
|
||||
|
||||
struct ExternalWriterNotice {
|
||||
command_center_available: bool,
|
||||
transcript_hint: Option<crate::key_hint::ShortcutHint>,
|
||||
}
|
||||
|
||||
@@ -87,19 +88,18 @@ impl ExternalWriterNotice {
|
||||
}
|
||||
|
||||
fn footer_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
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 {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
+10
@@ -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
|
||||
+11
@@ -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
|
||||
+12
@@ -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
|
||||
@@ -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]
|
||||
|
||||
@@ -53,16 +53,24 @@ impl SessionStartAction {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) enum SessionStartOutcome {
|
||||
Started(Box<AppServerStartedThread>),
|
||||
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<AppServerStartedThread>,
|
||||
confirm: impl AsyncFnOnce() -> Result<UnarchiveChoice>,
|
||||
) -> Result<Option<AppServerStartedThread>> {
|
||||
) -> Result<SessionStartOutcome> {
|
||||
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))
|
||||
}
|
||||
|
||||
|
||||
@@ -14,9 +14,18 @@ async fn start_session(
|
||||
target: &SessionTarget,
|
||||
action: SessionStartAction,
|
||||
confirm: impl AsyncFnOnce() -> Result<UnarchiveChoice>,
|
||||
) -> Result<Option<AppServerStartedThread>> {
|
||||
) -> Result<SessionStartOutcome> {
|
||||
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 {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user