diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 71213e3355..b6457a8d6e 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -240,6 +240,7 @@ mod thread_goal_actions; mod thread_routing; mod thread_session_state; mod thread_settings; +mod thread_title; mod transcript_export; mod working_directory; @@ -595,6 +596,7 @@ pub(crate) struct App { windows_sandbox: WindowsSandboxState, thread_event_channels: HashMap, + temporary_structured_requests: HashMap>, thread_event_listener_tasks: HashMap>, agent_navigation: AgentNavigationState, agents_overview: agents_overview::AgentsOverviewState, diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index b4288f6b76..71bba070ea 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -92,6 +92,22 @@ impl App { let _ = self.dynamic_tool_status_updates.send(status.clone()); } + // Hidden helper threads must not enter visible thread routing or overview refreshes. + if let ServerNotificationThreadTarget::Thread(thread_id) = + server_notification_thread_target(¬ification) + && let Some(sender) = self.temporary_structured_requests.get(&thread_id) + { + if matches!( + ¬ification, + ServerNotification::ItemCompleted(_) | ServerNotification::TurnCompleted(_) + ) && sender.send(notification).is_err() + { + self.temporary_structured_requests.remove(&thread_id); + } + + return; + } + if let ServerNotification::ThreadStarted(started) = ¬ification && let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { parent_thread_id, .. diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 9029728e6c..b2df60de4c 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -6,6 +6,7 @@ use super::resize_reflow::trailing_run_start; use super::session_lifecycle::ThreadAttachPresentation; use super::*; +use crate::app_event::ThreadTitleDestination; use crate::app_server_session::ForkGoalContinuation; use crate::app_server_session::UnsupportedLegacyPermissionProfile; use crate::app_server_session::turn_permissions_overrides; @@ -2459,13 +2460,59 @@ impl App { .await; } AppEvent::RenameAgentsOverviewThread { thread_id, name } => { - if let Err(error) = app_server.thread_set_name(thread_id, name.clone()).await { - if let Ok(mut state) = self.agents_overview.view_state.lock() { - state.input = name; - state.renaming = true; + match app_server.thread_set_name(thread_id, name.clone()).await { + Ok(()) => self.chat_widget.expect_manual_thread_name(thread_id, name), + Err(error) => { + if let Ok(mut state) = self.agents_overview.view_state.lock() { + state.input = name; + state.renaming = true; + } + self.chat_widget + .add_error_message(format!("Failed to rename task: {error}")); + } + } + } + AppEvent::ThreadTitleStarted { + thread_id, + destination, + prompt, + effort, + result, + } => { + self.on_thread_title_started( + app_server, + thread_id, + destination, + prompt, + effort, + result, + ); + } + AppEvent::GeneratedThreadTitle { + thread_id, + temporary_thread_id, + destination, + result, + } => { + self.temporary_structured_requests + .remove(&temporary_thread_id); + + match destination { + ThreadTitleDestination::Automatic { expected_title } => { + if let Ok(response) = result + && let Some(title) = super::thread_title::parse_thread_title(&response) + && self.chat_widget.thread_id() == Some(thread_id) + && self.chat_widget.thread_name().as_deref() + == Some(expected_title.as_str()) + { + match app_server.thread_set_name(thread_id, title.clone()).await { + Ok(()) => self.chat_widget.expect_automatic_thread_name(title), + Err(error) => { + tracing::debug!(%error, "failed to apply generated thread title"); + } + } + } } - self.chat_widget - .add_error_message(format!("Failed to rename task: {error}")); } } AppEvent::StopAgentsOverviewThread { thread_id } => { diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 5f5b0b4de4..959ebca611 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -507,6 +507,7 @@ See the Codex keymap documentation for supported actions and examples." pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + temporary_structured_requests: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), agents_overview: Default::default(), diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index f39be93aba..5a42a6f01e 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -60,6 +60,7 @@ pub(super) async fn make_test_app() -> App { pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + temporary_structured_requests: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), agents_overview: Default::default(), diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 9ef96b70a6..ca60a1380f 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -5447,6 +5447,7 @@ async fn make_test_app() -> App { pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + temporary_structured_requests: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), agents_overview: Default::default(), @@ -5526,6 +5527,7 @@ async fn make_test_app_with_channels() -> ( pending_shutdown_exit_thread_id: None, windows_sandbox: WindowsSandboxState::default(), thread_event_channels: HashMap::new(), + temporary_structured_requests: HashMap::new(), thread_event_listener_tasks: HashMap::new(), agent_navigation: AgentNavigationState::default(), agents_overview: Default::default(), diff --git a/codex-rs/tui/src/app/tests/safety_buffering.rs b/codex-rs/tui/src/app/tests/safety_buffering.rs index 887d0bcd5f..3d5950b10e 100644 --- a/codex-rs/tui/src/app/tests/safety_buffering.rs +++ b/codex-rs/tui/src/app/tests/safety_buffering.rs @@ -31,6 +31,7 @@ const RETRY_PROMPT: &str = "Handle the safety-buffered request"; const COMMITTED_STEER: &str = "Keep the accepted steer"; const UNSENT_DRAFT: &str = "Keep this unsent draft"; const RETRY_GOAL: &str = "Preserve this goal across the retry"; +const SAFETY_RETRY_THREAD_NAME: &str = "Safety retry source"; #[derive(Clone, Copy, PartialEq, Eq)] enum SafetyRetryScenario { @@ -420,8 +421,13 @@ goals = true let mut tui = crate::tui::test_support::make_test_tui()?; let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; - let started = app_server.start_thread(&app.config).await?; + let mut started = app_server.start_thread(&app.config).await?; let source_thread_id = started.session.thread_id; + // Keep ordered response fixtures focused on safety retries, not background naming. + app_server + .thread_set_name(source_thread_id, SAFETY_RETRY_THREAD_NAME.to_string()) + .await?; + started.session.thread_name = Some(SAFETY_RETRY_THREAD_NAME.to_string()); app.replace_chat_widget_with_app_server_thread( &mut tui, started, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 861fbc1396..ed7c5c3553 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -6,6 +6,7 @@ use super::session_lifecycle::ThreadAttachPresentation; use super::*; +use crate::app_event::ThreadTitleDestination; use crate::chatwidget::ThreadInputStateRestoreMode; use crate::session_resume::read_session_model; use codex_app_server_protocol::ThreadStartedNotification; @@ -781,9 +782,9 @@ impl App { Ok(true) } AppCommand::SetThreadName { name } => { - app_server - .thread_set_name(thread_id, name.to_string()) - .await?; + 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); Ok(true) } AppCommand::Review { target } => { @@ -1796,9 +1797,61 @@ impl App { // thread, so unrelated shutdowns cannot consume this marker. self.pending_shutdown_exit_thread_id = None; } + let automatic_title_user_message = if self.chat_widget.thread_name().is_none() + && let ThreadBufferedEvent::Notification(notification) = &event + && let ServerNotification::ItemCompleted(notification) = notification.as_ref() + && let ThreadItem::UserMessage { content, .. } = ¬ification.item + { + Some( + content + .iter() + .filter_map(|item| match item { + codex_app_server_protocol::UserInput::Text { text, .. } => { + Some(crate::ide_context::extract_prompt_request_with_offset(text).0) + } + _ => None, + }) + .collect::>() + .join(" "), + ) + } else { + None + }; let had_active_view = self.chat_widget.has_active_view(); self.handle_thread_event_now_recovering_file_changes(event) .await; + if let Some(user_message) = automatic_title_user_message { + let expected_title = user_message + .split_whitespace() + .collect::>() + .join(" ") + .chars() + .take(super::thread_title::THREAD_TITLE_MAX_CHARS) + .collect::(); + + if !expected_title.is_empty() + && let Some(thread_id) = self.active_thread_id + { + match app_server + .thread_set_name(thread_id, expected_title.clone()) + .await + { + Ok(()) => { + self.chat_widget + .expect_automatic_thread_name(expected_title.clone()); + self.generate_thread_title( + app_server, + thread_id, + ThreadTitleDestination::Automatic { expected_title }, + super::thread_title::thread_title_prompt(&user_message), + ); + } + Err(error) => { + tracing::debug!(%error, "failed to set provisional thread title"); + } + } + } + } if !had_active_view && self.chat_widget.has_active_view() && self.startup_protected_input_boundary diff --git a/codex-rs/tui/src/app/thread_title.rs b/codex-rs/tui/src/app/thread_title.rs new file mode 100644 index 0000000000..57da92d087 --- /dev/null +++ b/codex-rs/tui/src/app/thread_title.rs @@ -0,0 +1,201 @@ +//! Structured-output schema and normalization for generated TUI thread titles. + +use super::App; +use crate::app_event::AppEvent; +use crate::app_event::ThreadTitleDestination; +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 codex_protocol::ThreadId; +use codex_protocol::openai_models::ReasoningEffort; +use serde::Deserialize; +use serde_json::Value; +use serde_json::json; +use tokio::sync::mpsc; + +pub(super) const THREAD_TITLE_MAX_CHARS: usize = 36; +const THREAD_TITLE_MODEL: &str = "gpt-5.6-luna"; +pub(super) const THREAD_TITLE_PROMPT_MAX_BYTES: usize = 960; + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct GeneratedThreadTitle { + title: String, +} + +impl App { + /// Start a hidden title-generation thread without blocking the UI loop. + pub(super) fn generate_thread_title( + &mut self, + app_server: &AppServerSession, + thread_id: ThreadId, + destination: ThreadTitleDestination, + prompt: String, + ) { + let request_handle = app_server.request_handle(); + let model = if self.chat_widget.config_ref().model_provider_id == "openai" + && self.chat_widget.has_chatgpt_account() + && self + .chat_widget + .model_catalog() + .try_list_models() + .is_ok_and(|models| models.iter().any(|model| model.model == THREAD_TITLE_MODEL)) + { + THREAD_TITLE_MODEL.to_string() + } else { + self.chat_widget.current_model().to_string() + }; + let effort = (model == THREAD_TITLE_MODEL).then_some(ReasoningEffort::Low); + let config = self.chat_widget.config_ref(); + let options = TemporaryStructuredThreadOptions { + model, + model_provider: config.model_provider_id.clone(), + cwd: config.cwd.display().to_string(), + active_permission_profile: config + .permissions + .active_permission_profile() + .map(|profile| profile.id), + mcp_server_names: config.mcp_servers.get().keys().cloned().collect(), + }; + + let event_sender = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = start_temporary_thread(&request_handle, options) + .await + .map(|thread| thread.thread.id) + .map_err(|error| error.to_string()); + + event_sender.send(AppEvent::ThreadTitleStarted { + thread_id, + destination, + prompt, + effort, + result, + }); + }); + } + + /// Register a started hidden thread and generate its structured title. + pub(super) fn on_thread_title_started( + &mut self, + app_server: &AppServerSession, + thread_id: ThreadId, + destination: ThreadTitleDestination, + prompt: String, + effort: Option, + result: Result, + ) { + let temporary_thread_id_text = match result { + Ok(thread_id) => thread_id, + Err(error) => { + tracing::debug!(%error, "failed to start title-generation thread"); + return; + } + }; + + let Ok(temporary_thread_id) = ThreadId::from_string(&temporary_thread_id_text) else { + return; + }; + + let (sender, receiver) = mpsc::unbounded_channel(); + self.temporary_structured_requests + .insert(temporary_thread_id, sender); + + let request_handle = app_server.request_handle(); + let event_sender = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = run_temporary_structured_turn( + request_handle, + temporary_thread_id_text, + prompt, + thread_title_output_schema(), + effort, + receiver, + ) + .await + .map_err(|error| error.to_string()); + + event_sender.send(AppEvent::GeneratedThreadTitle { + thread_id, + temporary_thread_id, + destination, + result, + }); + }); + } +} + +/// Constrain generated metadata to one nonempty title within the display limit. +pub(super) fn thread_title_output_schema() -> Value { + json!({ + "type": "object", + "properties": { + "title": { + "type": "string", + "minLength": 1, + "maxLength": THREAD_TITLE_MAX_CHARS, + }, + }, + "required": ["title"], + "additionalProperties": false, + }) +} + +fn thread_title_instructions() -> String { + format!( + "Generate a concise, single-line task title of at most \ + {THREAD_TITLE_MAX_CHARS} characters and under five words where possible. \ + Start with an imperative verb. Capitalize only the first word unless the \ + user's language, proper nouns, acronyms, or code terms require otherwise. \ + Preserve ticket references exactly. Write in the user's language. \ + Do not use quotes, markdown, or trailing punctuation. \ + Do not answer the request." + ) +} + +/// Build a bounded title request without truncating a Unicode character. +pub(super) fn thread_title_prompt(user_message: &str) -> String { + let instructions = thread_title_instructions(); + let prefix = format!("{instructions}\n\nUser prompt:\n"); + let remaining_bytes = THREAD_TITLE_PROMPT_MAX_BYTES.saturating_sub(prefix.len()); + let user_message = user_message + .trim() + .char_indices() + .take_while(|(index, character)| index + character.len_utf8() <= remaining_bytes) + .map(|(_, character)| character) + .collect::(); + + format!("{prefix}{user_message}") +} + +/// Normalize a generated title and truncate it without splitting Unicode characters. +pub(super) fn parse_thread_title(response: &str) -> Option { + if !response.trim_start().starts_with('{') { + return None; + } + + let title = serde_json::from_str::(response) + .ok()? + .title; + + let normalized = title + .trim() + .trim_matches(|character| matches!(character, '"' | '\'' | '`' | '“' | '”' | '‘' | '’')) + .split_whitespace() + .collect::>() + .join(" ") + .trim_end_matches(['.', '?', '!']) + .trim_end() + .to_string(); + + if normalized.is_empty() { + return None; + } + + Some(normalized.chars().take(THREAD_TITLE_MAX_CHARS).collect()) +} + +#[cfg(test)] +#[path = "thread_title_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app/thread_title_tests.rs b/codex-rs/tui/src/app/thread_title_tests.rs new file mode 100644 index 0000000000..fdcaa27e21 --- /dev/null +++ b/codex-rs/tui/src/app/thread_title_tests.rs @@ -0,0 +1,129 @@ +use super::THREAD_TITLE_MAX_CHARS; +use super::THREAD_TITLE_PROMPT_MAX_BYTES; +use super::parse_thread_title; +use super::thread_title_prompt; +use crate::app::test_support::make_test_app; +use crate::app_command::AppCommand; +use pretty_assertions::assert_eq; + +const EXPECTED_THREAD_TITLE_INSTRUCTIONS: &str = concat!( + "Generate a concise, single-line task title of at most 36 characters ", + "and under five words where possible. Start with an imperative verb. ", + "Capitalize only the first word unless the user's language, proper nouns, ", + "acronyms, or code terms require otherwise. Preserve ticket references ", + "exactly. Write in the user's language. Do not use quotes, markdown, ", + "or trailing punctuation. Do not answer the request." +); + +#[test] +fn trims_user_message_in_title_prompt() { + let prompt = thread_title_prompt(" Fix the login form \n"); + + assert_eq!( + prompt, + format!("{EXPECTED_THREAD_TITLE_INSTRUCTIONS}\n\nUser prompt:\nFix the login form") + ); +} + +#[test] +fn truncates_title_prompt_without_splitting_unicode() { + let user_message = "🚀".repeat(THREAD_TITLE_PROMPT_MAX_BYTES); + let prompt = thread_title_prompt(&user_message); + let expected_instructions = format!("{EXPECTED_THREAD_TITLE_INSTRUCTIONS}\n\n"); + let prefix = format!("{expected_instructions}User prompt:\n"); + let available_bytes = THREAD_TITLE_PROMPT_MAX_BYTES - prefix.len(); + let expected = "🚀".repeat(available_bytes / '🚀'.len_utf8()); + + assert_eq!( + prompt.rsplit_once("User prompt:\n"), + Some((expected_instructions.as_str(), expected.as_str())) + ); + assert!(prompt.len() <= THREAD_TITLE_PROMPT_MAX_BYTES); +} + +#[test] +fn bounds_the_entire_title_prompt_for_dense_unicode() { + let repeated_characters = THREAD_TITLE_PROMPT_MAX_BYTES * 3; + for message in [ + "🚀".repeat(repeated_characters), + "漢".repeat(repeated_characters), + "x".repeat(repeated_characters), + ] { + let prompt = thread_title_prompt(&message); + + assert!( + prompt.len() <= THREAD_TITLE_PROMPT_MAX_BYTES, + "{} bytes for {:?}", + prompt.len(), + message.chars().next() + ); + } +} + +#[tokio::test] +async fn manual_rename_invalidates_pending_automatic_title_before_notification() +-> 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?; + app_server + .thread_set_name(thread_id, "Provisional title".to_string()) + .await?; + app.chat_widget + .expect_automatic_thread_name("Provisional title".to_string()); + + app.submit_thread_op( + &mut app_server, + thread_id, + AppCommand::set_thread_name("Manual title".to_string()), + ) + .await?; + + assert_eq!( + app.chat_widget.thread_name(), + Some("Manual title".to_string()) + ); + + app_server.shutdown().await?; + Ok(()) +} + +#[test] +fn normalizes_generated_title_whitespace() { + assert_eq!( + parse_thread_title(r#"{"title":" Fix \n\t login errors "}"#), + Some("Fix login errors".to_string()) + ); +} + +#[test] +fn rejects_invalid_or_empty_generated_titles() { + for response in [ + "", + "not json", + "null", + "true", + "42", + "[]", + r#"["title"]"#, + r#""plain title""#, + "{}", + r#"{"title":7}"#, + r#"{"title":"valid","extra":true}"#, + r#"{"title":""}"#, + r#"{"title":" \t "}"#, + ] { + assert_eq!(parse_thread_title(response), None, "response: {response}"); + } +} + +#[test] +fn truncates_generated_titles_without_splitting_unicode() { + let expected = "🚀".repeat(THREAD_TITLE_MAX_CHARS); + let response = serde_json::json!({ "title": format!("{expected}x") }).to_string(); + + assert_eq!(parse_thread_title(&response), Some(expected)); +} diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index d8daf96451..a01a011918 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -40,6 +40,7 @@ use codex_file_search::FileMatch; use codex_message_history::HistoryBatchCursor; use codex_protocol::ThreadId; use codex_protocol::openai_models::ModelPreset; +use codex_protocol::openai_models::ReasoningEffort; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_approval_presets::ApprovalPreset; use strum_macros::IntoStaticStr; @@ -61,7 +62,6 @@ use codex_plugin::PluginCapabilitySummary; use codex_protocol::config_types::CollaborationModeMask; use codex_protocol::config_types::Personality; use codex_protocol::models::ActivePermissionProfile; -use codex_protocol::openai_models::ReasoningEffort; use crate::history_cell::HistoryCell; @@ -195,6 +195,13 @@ pub(crate) enum TranscriptExportDestination { File(PathBuf), } +/// Deliver a generated title to its originating automatic rename. +#[derive(Debug)] +pub(crate) enum ThreadTitleDestination { + /// Replace the provisional name only if the user has not renamed the thread. + Automatic { expected_title: String }, +} + #[allow(clippy::large_enum_variant)] #[derive(Debug, IntoStaticStr)] pub(crate) enum AppEvent { @@ -219,6 +226,21 @@ pub(crate) enum AppEvent { thread_id: ThreadId, name: String, }, + /// Register a hidden title-generation thread started in the background. + ThreadTitleStarted { + thread_id: ThreadId, + destination: ThreadTitleDestination, + prompt: String, + effort: Option, + result: Result, + }, + /// Route a hidden title request to its automatic rename or editable prompt. + GeneratedThreadTitle { + thread_id: ThreadId, + temporary_thread_id: ThreadId, + destination: ThreadTitleDestination, + result: Result, + }, /// Interrupt a task directly from the shared dashboard. StopAgentsOverviewThread { thread_id: ThreadId, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 541604a5fd..15c9b321ec 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -667,6 +667,7 @@ pub(crate) struct ChatWidget { pet_image_support_override: Option, thread_id: Option, thread_name: Option, + pending_automatic_thread_names: HashSet, thread_rename_block_message: Option, active_side_conversation: bool, blocks_direct_input: bool, diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index cb4f301b5d..0fde03a218 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -199,6 +199,7 @@ impl ChatWidget { pet_image_support_override: None, thread_id: None, thread_name: None, + pending_automatic_thread_names: HashSet::new(), thread_rename_block_message: None, active_side_conversation: false, blocks_direct_input: false, diff --git a/codex-rs/tui/src/chatwidget/session_flow.rs b/codex-rs/tui/src/chatwidget/session_flow.rs index cbb91fef86..e4e910523b 100644 --- a/codex-rs/tui/src/chatwidget/session_flow.rs +++ b/codex-rs/tui/src/chatwidget/session_flow.rs @@ -25,6 +25,7 @@ impl ChatWidget { self.bottom_pane .set_queue_submissions(/*queue_submissions*/ false); if previous_thread_id != self.thread_id { + self.pending_automatic_thread_names.clear(); self.review.recent_auto_review_denials = RecentAutoReviewDenials::default(); self.clear_thread_usage_state(); } @@ -245,16 +246,38 @@ impl ChatWidget { ))); } + /// Apply a persisted automatic name immediately and suppress its confirmation. + pub(crate) fn expect_automatic_thread_name(&mut self, name: String) { + self.thread_name = Some(name.clone()); + self.pending_automatic_thread_names.insert(name); + } + + /// Make a confirmed manual rename visible before its queued server notification arrives. + pub(crate) fn expect_manual_thread_name(&mut self, thread_id: ThreadId, name: String) { + if self.thread_id == Some(thread_id) { + self.thread_name = Some(name); + self.refresh_status_surfaces(); + self.request_redraw(); + } + } + pub(super) fn on_thread_name_updated( &mut self, thread_id: ThreadId, thread_name: Option, ) { if self.thread_id == Some(thread_id) { - if let Some(name) = thread_name.as_deref() { + let automatic = thread_name + .as_ref() + .is_some_and(|name| self.pending_automatic_thread_names.remove(name)); + + if let Some(name) = thread_name.as_deref() + && !automatic + { let cell = Self::rename_confirmation_cell(name, self.thread_id); self.add_boxed_history(Box::new(cell)); } + self.thread_name = thread_name; self.refresh_status_surfaces(); self.request_redraw(); diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index 70df67db08..b749244d02 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -1558,6 +1558,69 @@ async fn live_app_server_thread_name_update_shows_resume_hint() { assert_chatwidget_snapshot!("thread_name_update_resume_hint", rendered); } +#[tokio::test] +async fn live_app_server_automatic_thread_name_update_is_silent() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + for title in ["Provisional title", "Generated title"] { + chat.expect_automatic_thread_name(title.to_string()); + assert_eq!(chat.thread_name(), Some(title.to_string())); + + chat.handle_server_notification( + ServerNotification::ThreadNameUpdated( + codex_app_server_protocol::ThreadNameUpdatedNotification { + thread_id: thread_id.to_string(), + thread_name: Some(title.to_string()), + }, + ), + /*replay_kind*/ None, + ); + + assert_eq!(chat.thread_name, Some(title.to_string())); + assert!(drain_insert_history(&mut rx).is_empty()); + } + + chat.handle_server_notification( + ServerNotification::ThreadNameUpdated( + codex_app_server_protocol::ThreadNameUpdatedNotification { + thread_id: thread_id.to_string(), + thread_name: Some("Manual title".to_string()), + }, + ), + /*replay_kind*/ None, + ); + + assert_eq!(chat.thread_name, Some("Manual title".to_string())); + assert_eq!(drain_insert_history(&mut rx).len(), 1); +} + +#[tokio::test] +async fn live_app_server_manual_thread_name_is_visible_before_notification() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.expect_automatic_thread_name("Provisional title".to_string()); + + chat.expect_manual_thread_name(thread_id, "Manual title".to_string()); + + assert_eq!(chat.thread_name(), Some("Manual title".to_string())); + assert!(drain_insert_history(&mut rx).is_empty()); + + chat.handle_server_notification( + ServerNotification::ThreadNameUpdated( + codex_app_server_protocol::ThreadNameUpdatedNotification { + thread_id: thread_id.to_string(), + thread_name: Some("Manual title".to_string()), + }, + ), + /*replay_kind*/ None, + ); + + assert_eq!(chat.thread_name(), Some("Manual title".to_string())); + assert_eq!(drain_insert_history(&mut rx).len(), 1); +} + #[tokio::test] async fn live_app_server_thread_closed_requests_immediate_exit() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index c06e21f6c1..2c0a96bf02 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -186,6 +186,7 @@ mod status_indicator_widget; mod streaming; mod style; mod task_mentions; +mod temporary_structured_request; mod terminal_hyperlinks; mod terminal_palette; mod terminal_probe; diff --git a/codex-rs/tui/src/temporary_structured_request.rs b/codex-rs/tui/src/temporary_structured_request.rs new file mode 100644 index 0000000000..047292dc97 --- /dev/null +++ b/codex-rs/tui/src/temporary_structured_request.rs @@ -0,0 +1,252 @@ +//! Run isolated structured requests through existing app-server methods. + +use codex_app_server_client::AppServerRequestHandle; +use codex_app_server_client::TypedRequestError; +use codex_app_server_protocol::AskForApproval; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::SandboxMode; +use codex_app_server_protocol::SandboxPolicy; +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadSource; +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::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::TurnStatus; +use codex_app_server_protocol::UserInput; +use codex_protocol::openai_models::ReasoningEffort; +use color_eyre::eyre::eyre; +use serde_json::Value; +use std::time::Duration; +use tokio::sync::mpsc::UnboundedReceiver; +use uuid::Uuid; + +const STRUCTURED_TURN_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 30); +const STRUCTURED_RESPONSE_MAX_BYTES: usize = 8 * 1024; + +/// Preserve the visible thread's provider, permissions, and external-tool isolation. +pub(crate) struct TemporaryStructuredThreadOptions { + pub(crate) model: String, + pub(crate) model_provider: String, + pub(crate) cwd: String, + pub(crate) active_permission_profile: Option, + pub(crate) mcp_server_names: Vec, +} + +/// Start an ephemeral, non-interactive thread without widening its parent's permissions. +pub(crate) async fn start_temporary_thread( + request_handle: &AppServerRequestHandle, + options: TemporaryStructuredThreadOptions, +) -> color_eyre::Result { + let TemporaryStructuredThreadOptions { + model, + model_provider, + cwd, + active_permission_profile, + mcp_server_names, + } = options; + let custom_permission_profile = + active_permission_profile.filter(|profile| !profile.starts_with(':')); + let mut config = std::collections::HashMap::from([ + ("features.apps".to_string(), false.into()), + ("features.code_mode".to_string(), false.into()), + ("features.code_mode_only".to_string(), false.into()), + ("features.current_time_reminder".to_string(), false.into()), + ("features.deferred_executor".to_string(), false.into()), + ("features.enable_fanout".to_string(), false.into()), + ("features.goals".to_string(), false.into()), + ("features.hooks".to_string(), false.into()), + ("features.image_generation".to_string(), false.into()), + ("features.memories".to_string(), false.into()), + ("features.multi_agent".to_string(), false.into()), + ("features.multi_agent_v2".to_string(), false.into()), + ("features.plugins".to_string(), false.into()), + ( + "features.request_permissions_tool".to_string(), + false.into(), + ), + ("features.shell_snapshot".to_string(), false.into()), + ("features.shell_tool".to_string(), false.into()), + ("features.standalone_web_search".to_string(), false.into()), + ("features.tool_suggest".to_string(), false.into()), + ("features.unified_exec".to_string(), false.into()), + ("features.view_image".to_string(), false.into()), + ( + "tools.experimental_request_user_input.enabled".to_string(), + false.into(), + ), + ("tools.update_plan.enabled".to_string(), false.into()), + ("web_search".to_string(), "disabled".into()), + ]); + for server_name in mcp_server_names { + config.insert(format!("mcp_servers.{server_name}.enabled"), false.into()); + } + + let response: ThreadStartResponse = tokio::time::timeout( + STRUCTURED_TURN_TIMEOUT, + request_handle.request_typed(ClientRequest::ThreadStart { + request_id: RequestId::String(format!("temporary-structured-{}", Uuid::new_v4())), + params: ThreadStartParams { + model: Some(model), + model_provider: Some(model_provider), + cwd: Some(cwd), + approval_policy: Some(AskForApproval::Never), + sandbox: custom_permission_profile + .is_none() + .then_some(SandboxMode::ReadOnly), + permissions: custom_permission_profile.clone(), + runtime_workspace_roots: Some(Vec::new()), + ephemeral: Some(true), + thread_source: Some(ThreadSource::Feature("system".to_string())), + environments: Some(Vec::new()), + dynamic_tools: Some(Vec::new()), + selected_capability_roots: Some(Vec::new()), + config: Some(config), + ..ThreadStartParams::default() + }, + }), + ) + .await + .map_err(|_| eyre!("temporary structured thread start timed out"))??; + + if let Some(expected_profile) = custom_permission_profile { + if response + .active_permission_profile + .as_ref() + .is_none_or(|profile| profile.id != expected_profile) + { + return Err(eyre!( + "temporary structured thread did not preserve permission profile {expected_profile}" + )); + } + } else if !matches!(response.sandbox, SandboxPolicy::ReadOnly { .. }) { + return Err(eyre!( + "temporary structured thread did not start with read-only permissions" + )); + } + + Ok(response) +} + +/// Submit a structured turn using the temporary thread's existing settings. +pub(crate) async fn start_structured_turn( + request_handle: &AppServerRequestHandle, + thread_id: String, + prompt: String, + output_schema: Value, + effort: Option, +) -> Result { + request_handle + .request_typed(ClientRequest::TurnStart { + request_id: RequestId::String(format!("temporary-structured-turn-{}", Uuid::new_v4())), + params: TurnStartParams { + thread_id, + input: vec![UserInput::Text { + text: prompt, + text_elements: Vec::new(), + }], + output_schema: Some(output_schema), + effort, + ..TurnStartParams::default() + }, + }) + .await +} + +/// Return the latest assistant message when the requested turn completes. +pub(crate) async fn collect_structured_response( + mut notifications: UnboundedReceiver, + turn_id: &str, +) -> color_eyre::Result { + let mut response = None; + + while let Some(notification) = notifications.recv().await { + match notification { + ServerNotification::ItemCompleted(completed) if completed.turn_id == turn_id => { + if let ThreadItem::AgentMessage { text, .. } = completed.item { + if text.len() > STRUCTURED_RESPONSE_MAX_BYTES { + return Err(eyre!( + "temporary structured response exceeds {STRUCTURED_RESPONSE_MAX_BYTES} bytes" + )); + } + response = Some(text); + } + } + ServerNotification::TurnCompleted(completed) if completed.turn.id == turn_id => { + if completed.turn.status != TurnStatus::Completed { + return Err(eyre!( + "temporary structured turn ended with status {:?}", + completed.turn.status, + )); + } + + return response.ok_or_else(|| { + eyre!("temporary structured turn completed without a response") + }); + } + _ => {} + } + } + + Err(eyre!( + "temporary structured turn notification channel closed" + )) +} + +/// Run a bounded structured turn and make a bounded temporary-thread cleanup attempt. +pub(crate) async fn run_temporary_structured_turn( + request_handle: AppServerRequestHandle, + thread_id: String, + prompt: String, + output_schema: Value, + effort: Option, + notifications: UnboundedReceiver, +) -> color_eyre::Result { + let result = tokio::time::timeout(STRUCTURED_TURN_TIMEOUT, async { + let turn = start_structured_turn( + &request_handle, + thread_id.clone(), + prompt, + output_schema, + effort, + ) + .await?; + + collect_structured_response(notifications, &turn.turn.id).await + }) + .await + .unwrap_or_else(|_| Err(eyre!("temporary structured turn timed out"))); + + match tokio::time::timeout( + STRUCTURED_TURN_TIMEOUT, + request_handle.request_typed::( + ClientRequest::ThreadUnsubscribe { + request_id: RequestId::String(format!( + "temporary-structured-unsubscribe-{}", + Uuid::new_v4() + )), + params: ThreadUnsubscribeParams { thread_id }, + }, + ), + ) + .await + { + Ok(Ok(_)) => {} + Ok(Err(error)) => { + tracing::debug!(%error, "failed to unsubscribe from temporary structured thread"); + } + Err(_) => { + tracing::debug!("temporary structured thread unsubscribe timed out"); + } + } + + result +} + +#[cfg(test)] +#[path = "temporary_structured_request_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/temporary_structured_request_tests.rs b/codex-rs/tui/src/temporary_structured_request_tests.rs new file mode 100644 index 0000000000..b1af68a1e0 --- /dev/null +++ b/codex-rs/tui/src/temporary_structured_request_tests.rs @@ -0,0 +1,199 @@ +use super::STRUCTURED_RESPONSE_MAX_BYTES; +use super::TemporaryStructuredThreadOptions; +use super::collect_structured_response; +use super::start_temporary_thread; +use crate::test_support::PathBufExt; +use codex_app_server_protocol::ItemCompletedNotification; +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::Turn; +use codex_app_server_protocol::TurnCompletedNotification; +use codex_app_server_protocol::TurnStatus; +use pretty_assertions::assert_eq; +use tempfile::tempdir; +use tokio::sync::mpsc::unbounded_channel; + +fn agent_message_notification(turn_id: &str, text: &str) -> ServerNotification { + ServerNotification::ItemCompleted(ItemCompletedNotification { + item: ThreadItem::AgentMessage { + id: "message-1".to_string(), + text: text.to_string(), + phase: None, + memory_citation: None, + delivery: None, + }, + thread_id: "thread-1".to_string(), + turn_id: turn_id.to_string(), + completed_at_ms: 0, + }) +} + +fn turn_completed_notification(turn_id: &str, status: TurnStatus) -> ServerNotification { + ServerNotification::TurnCompleted(TurnCompletedNotification { + thread_id: "thread-1".to_string(), + turn: Turn { + id: turn_id.to_string(), + items: Vec::new(), + items_view: Default::default(), + status, + error: None, + started_at: None, + completed_at: None, + duration_ms: None, + }, + }) +} + +#[tokio::test] +async fn preserves_custom_permissions_and_disables_required_mcp_servers() -> color_eyre::Result<()> +{ + let (chat_widget, _, _, _) = + crate::chatwidget::tests::make_chatwidget_manual_with_sender().await; + let mut config = chat_widget.config_ref().clone(); + let codex_home = tempdir()?; + let denied_path = codex_home.path().join("denied"); + let denied_key = toml::Value::String(denied_path.display().to_string()); + std::fs::write( + codex_home.path().join("config.toml"), + format!( + "default_permissions = \"title-restricted\"\n\n\ + [permissions.title-restricted.filesystem]\n\ + \":root\" = \"read\"\n\ + {denied_key} = \"deny\"\n\n\ + [mcp_servers.forbidden]\n\ + command = \"codex-auto-title-missing-mcp\"\n\ + required = true\n" + ), + )?; + config.codex_home = codex_home.path().to_path_buf().abs(); + config.sqlite = + codex_state::SqliteConfig::new_for_testing(codex_home.path().to_path_buf().abs()); + + let app_server = crate::start_embedded_app_server_for_picker(&config).await?; + let response = start_temporary_thread( + &app_server.request_handle(), + TemporaryStructuredThreadOptions { + model: "gpt-5.2".to_string(), + model_provider: config.model_provider_id.clone(), + cwd: config.cwd.display().to_string(), + active_permission_profile: Some("title-restricted".to_string()), + mcp_server_names: vec!["forbidden".to_string()], + }, + ) + .await?; + + assert_eq!( + ( + response.active_permission_profile.map(|profile| profile.id), + response.model_provider, + response.thread.ephemeral, + ), + ( + Some("title-restricted".to_string()), + config.model_provider_id, + true, + ) + ); + + app_server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn returns_latest_matching_assistant_message() { + let (sender, receiver) = unbounded_channel(); + sender + .send(agent_message_notification("other-turn", "ignore me")) + .expect("send unrelated assistant message"); + sender + .send(turn_completed_notification( + "other-turn", + TurnStatus::Completed, + )) + .expect("send unrelated completion"); + sender + .send(agent_message_notification("turn-1", "first")) + .expect("send first assistant message"); + sender + .send(agent_message_notification("turn-1", "final")) + .expect("send final assistant message"); + sender + .send(turn_completed_notification("turn-1", TurnStatus::Completed)) + .expect("send matching completion"); + + let response = collect_structured_response(receiver, "turn-1") + .await + .expect("collect structured response"); + + assert_eq!(response, "final"); +} + +#[tokio::test] +async fn rejects_failed_turn() { + let (sender, receiver) = unbounded_channel(); + sender + .send(turn_completed_notification("turn-1", TurnStatus::Failed)) + .expect("send failed completion"); + + let error = collect_structured_response(receiver, "turn-1") + .await + .expect_err("failed turn should not produce a response"); + + assert_eq!( + error.to_string(), + "temporary structured turn ended with status Failed" + ); +} + +#[tokio::test] +async fn rejects_completion_without_assistant_message() { + let (sender, receiver) = unbounded_channel(); + sender + .send(turn_completed_notification("turn-1", TurnStatus::Completed)) + .expect("send completion"); + + let error = collect_structured_response(receiver, "turn-1") + .await + .expect_err("completion without assistant message should fail"); + + assert_eq!( + error.to_string(), + "temporary structured turn completed without a response" + ); +} + +#[tokio::test] +async fn rejects_closed_notification_channel() { + let (sender, receiver) = unbounded_channel(); + drop(sender); + + let error = collect_structured_response(receiver, "turn-1") + .await + .expect_err("closed notification channel should fail"); + + assert_eq!( + error.to_string(), + "temporary structured turn notification channel closed" + ); +} + +#[tokio::test] +async fn rejects_oversized_assistant_message() { + let (sender, receiver) = unbounded_channel(); + let oversized = "x".repeat(STRUCTURED_RESPONSE_MAX_BYTES + 1); + sender + .send(agent_message_notification("turn-1", &oversized)) + .expect("send oversized assistant message"); + sender + .send(turn_completed_notification("turn-1", TurnStatus::Completed)) + .expect("send matching completion"); + + let error = collect_structured_response(receiver, "turn-1") + .await + .expect_err("oversized assistant message should be rejected"); + + assert_eq!( + error.to_string(), + "temporary structured response exceeds 8192 bytes" + ); +}