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