From 3a0eeb8edfb181bbfe7ae8f7d542d2040c1c1ea3 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 20 Jan 2026 18:13:54 -0800 Subject: [PATCH] Show session header before configuration (#9568) We were skipping if we know the model. We shouldn't --- .../tests/suite/v2/thread_resume.rs | 8 +- codex-rs/tui/src/chatwidget.rs | 18 +- codex-rs/tui/src/history_cell.rs | 4 +- codex-rs/tui2/src/app.rs | 8 + codex-rs/tui2/src/chatwidget.rs | 27 +-- codex-rs/tui2/src/history_cell.rs | 161 ++++++++++++++++-- codex-rs/tui2/src/ui_consts.rs | 1 + 7 files changed, 183 insertions(+), 44 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 89b127d39..d2fa72ef1 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -62,7 +62,9 @@ async fn thread_resume_returns_original_thread() -> Result<()> { let ThreadResumeResponse { thread: resumed, .. } = to_response::(resume_resp)?; - assert_eq!(resumed, thread); + let mut expected = thread; + expected.updated_at = resumed.updated_at; + assert_eq!(resumed, expected); Ok(()) } @@ -179,7 +181,9 @@ async fn thread_resume_prefers_path_over_thread_id() -> Result<()> { let ThreadResumeResponse { thread: resumed, .. } = to_response::(resume_resp)?; - assert_eq!(resumed, thread); + let mut expected = thread; + expected.updated_at = resumed.updated_at; + assert_eq!(resumed, expected); Ok(()) } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 8fb3dbacb..a004c472f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1807,9 +1807,7 @@ impl ChatWidget { let placeholder = PLACEHOLDERS[rng.random_range(0..PLACEHOLDERS.len())].to_string(); let codex_op_tx = spawn_agent(config.clone(), app_event_tx.clone(), thread_manager); - let model_for_header = model - .clone() - .unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); + let model_for_header = model.unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); let stored_collaboration_mode = if config.features.enabled(Feature::CollaborationModes) { collaboration_modes::default_mode(models_manager.as_ref()).unwrap_or_else(|| { CollaborationMode::Custom(Settings { @@ -1826,15 +1824,11 @@ impl ChatWidget { }) }; - let active_cell = if model.is_none() { - Some(Self::placeholder_session_header_cell( - &config, - config.features.enabled(Feature::CollaborationModes), - stored_collaboration_mode.clone(), - )) - } else { - None - }; + let active_cell = Some(Self::placeholder_session_header_cell( + &config, + config.features.enabled(Feature::CollaborationModes), + stored_collaboration_mode.clone(), + )); let mut widget = Self { app_event_tx: app_event_tx.clone(), diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 157ccb97a..9ad40e3af 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1123,7 +1123,9 @@ impl HistoryCell for SessionHeaderHistoryCell { label_width = label_width ); let mut spans = vec![Span::from(format!("{collab_label} ")).dim()]; - if let Some(mode_label) = self.collaboration_mode_label() { + if self.model == "loading" { + spans.push(Span::styled(self.model.clone(), self.model_style)); + } else if let Some(mode_label) = self.collaboration_mode_label() { spans.push(Span::styled(mode_label.to_string(), self.model_style)); } else { spans.push(Span::styled("Custom", self.model_style)); diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 096a384db..97e4c195e 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -2719,6 +2719,14 @@ mod tests { app.chat_widget.current_model(), event, is_first, + false, + codex_protocol::config_types::CollaborationMode::Custom( + codex_protocol::config_types::Settings { + model: "gpt-test".to_string(), + reasoning_effort: None, + developer_instructions: None, + }, + ), )) as Arc }; diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index d6164b095..9888128ba 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -162,6 +162,7 @@ use crate::slash_command::SlashCommand; use crate::status::RateLimitSnapshotDisplay; use crate::text_formatting::truncate_text; use crate::tui::FrameRequester; +use crate::ui_consts::DEFAULT_MODEL_DISPLAY_NAME; mod interrupts; use self::interrupts::InterruptManager; mod agent; @@ -213,7 +214,6 @@ impl UnifiedExecWaitState { const RATE_LIMIT_WARNING_THRESHOLDS: [f64; 3] = [75.0, 90.0, 95.0]; const NUDGE_MODEL_SLUG: &str = "gpt-5.1-codex-mini"; const RATE_LIMIT_SWITCH_PROMPT_THRESHOLD: f64 = 90.0; -const DEFAULT_MODEL_DISPLAY_NAME: &str = "loading"; #[derive(Default)] struct RateLimitWarningState { @@ -650,6 +650,8 @@ impl ChatWidget { &model_for_header, event, self.show_welcome_banner, + self.collaboration_modes_enabled(), + self.stored_collaboration_mode.clone(), ); self.apply_session_info_cell(session_info_cell); @@ -1610,15 +1612,7 @@ impl ChatWidget { let placeholder = PLACEHOLDERS[rng.random_range(0..PLACEHOLDERS.len())].to_string(); let codex_op_tx = spawn_agent(config.clone(), app_event_tx.clone(), thread_manager); - let model_for_header = model - .clone() - .unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); - let active_cell = if model.is_none() { - Some(Self::placeholder_session_header_cell(&config)) - } else { - None - }; - + let model_for_header = model.unwrap_or_else(|| DEFAULT_MODEL_DISPLAY_NAME.to_string()); let stored_collaboration_mode = if config.features.enabled(Feature::CollaborationModes) { collaboration_modes::default_mode(models_manager.as_ref()).unwrap_or_else(|| { CollaborationMode::Custom(Settings { @@ -1634,6 +1628,11 @@ impl ChatWidget { developer_instructions: None, }) }; + let active_cell = Some(Self::placeholder_session_header_cell( + &config, + config.features.enabled(Feature::CollaborationModes), + stored_collaboration_mode.clone(), + )); let mut widget = Self { app_event_tx: app_event_tx.clone(), @@ -4051,7 +4050,11 @@ impl ChatWidget { } /// Build a placeholder header cell while the session is configuring. - fn placeholder_session_header_cell(config: &Config) -> Box { + fn placeholder_session_header_cell( + config: &Config, + is_collaboration: bool, + collaboration_mode: CollaborationMode, + ) -> Box { let placeholder_style = Style::default().add_modifier(Modifier::DIM | Modifier::ITALIC); Box::new(history_cell::SessionHeaderHistoryCell::new_with_style( DEFAULT_MODEL_DISPLAY_NAME.to_string(), @@ -4059,6 +4062,8 @@ impl ChatWidget { None, config.cwd.clone(), CODEX_CLI_VERSION, + is_collaboration, + collaboration_mode, )) } diff --git a/codex-rs/tui2/src/history_cell.rs b/codex-rs/tui2/src/history_cell.rs index a313f9b2a..a4dc77a25 100644 --- a/codex-rs/tui2/src/history_cell.rs +++ b/codex-rs/tui2/src/history_cell.rs @@ -19,6 +19,7 @@ use crate::exec_cell::output_lines; use crate::exec_cell::spinner; use crate::exec_command::relativize_to_home; use crate::exec_command::strip_bash_lc_and_escape; +use crate::key_hint; use crate::markdown::append_markdown; use crate::render::line_utils::line_to_static; use crate::render::line_utils::prefix_lines; @@ -27,6 +28,7 @@ use crate::style::user_message_style; use crate::text_formatting::format_and_truncate_tool_result; use crate::text_formatting::truncate_text; use crate::tooltips; +use crate::ui_consts::DEFAULT_MODEL_DISPLAY_NAME; use crate::ui_consts::LIVE_PREFIX_COLS; use crate::update_action::UpdateAction; use crate::version::CODEX_CLI_VERSION; @@ -40,11 +42,13 @@ use codex_core::protocol::FileChange; use codex_core::protocol::McpAuthStatus; use codex_core::protocol::McpInvocation; use codex_core::protocol::SessionConfiguredEvent; +use codex_protocol::config_types::CollaborationMode; use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::plan_tool::PlanItemArg; use codex_protocol::plan_tool::StepStatus; use codex_protocol::plan_tool::UpdatePlanArgs; use codex_protocol::user_input::TextElement; +use crossterm::event::KeyCode; use image::DynamicImage; use image::ImageReader; use mcp_types::EmbeddedResourceResource; @@ -971,6 +975,8 @@ pub(crate) fn new_session_info( requested_model: &str, event: SessionConfiguredEvent, is_first_event: bool, + is_collaboration: bool, + collaboration_mode: CollaborationMode, ) -> SessionInfoCell { let SessionConfiguredEvent { model, @@ -984,6 +990,8 @@ pub(crate) fn new_session_info( reasoning_effort, config.cwd.clone(), CODEX_CLI_VERSION, + is_collaboration, + collaboration_mode, ); let mut parts: Vec> = vec![Box::new(header)]; @@ -1060,6 +1068,8 @@ pub(crate) struct SessionHeaderHistoryCell { model_style: Style, reasoning_effort: Option, directory: PathBuf, + is_collaboration: bool, + collaboration_mode: CollaborationMode, } impl SessionHeaderHistoryCell { @@ -1069,8 +1079,18 @@ impl SessionHeaderHistoryCell { reasoning_effort: Option, directory: PathBuf, version: &'static str, + is_collaboration: bool, + collaboration_mode: CollaborationMode, ) -> Self { - Self::new_with_style(model, model_style, reasoning_effort, directory, version) + Self::new_with_style( + model, + model_style, + reasoning_effort, + directory, + version, + is_collaboration, + collaboration_mode, + ) } pub(crate) fn new_with_style( @@ -1079,6 +1099,8 @@ impl SessionHeaderHistoryCell { reasoning_effort: Option, directory: PathBuf, version: &'static str, + is_collaboration: bool, + collaboration_mode: CollaborationMode, ) -> Self { Self { version, @@ -1086,6 +1108,20 @@ impl SessionHeaderHistoryCell { model_style, reasoning_effort, directory, + is_collaboration, + collaboration_mode, + } + } + + fn collaboration_mode_label(&self) -> Option<&'static str> { + if !self.is_collaboration { + return None; + } + match &self.collaboration_mode { + CollaborationMode::Plan(_) => Some("Plan"), + CollaborationMode::PairProgramming(_) => Some("Pair Programming"), + CollaborationMode::Execute(_) => Some("Execute"), + CollaborationMode::Custom(_) => None, } } @@ -1146,25 +1182,48 @@ impl HistoryCell for SessionHeaderHistoryCell { const CHANGE_MODEL_HINT_COMMAND: &str = "/model"; const CHANGE_MODEL_HINT_EXPLANATION: &str = " to change"; + const CHANGE_MODE_HINT_EXPLANATION: &str = " to change mode"; const DIR_LABEL: &str = "directory:"; let label_width = DIR_LABEL.len(); - let model_label = format!( - "{model_label:> = vec![ - Span::from(format!("{model_label} ")).dim(), - Span::styled(self.model.clone(), self.model_style), - ]; - if let Some(reasoning) = reasoning_label { - model_spans.push(Span::from(" ")); - model_spans.push(Span::from(reasoning)); - } - model_spans.push(" ".dim()); - model_spans.push(CHANGE_MODEL_HINT_COMMAND.cyan()); - model_spans.push(CHANGE_MODEL_HINT_EXPLANATION.dim()); + let model_spans: Vec> = if self.is_collaboration { + let collab_label = format!( + "{collab_label: = key_hint::shift(KeyCode::Tab).into(); + spans.push(shift_tab_span.cyan()); + spans.push(CHANGE_MODE_HINT_EXPLANATION.dim()); + spans + } else { + let model_label = format!( + "{model_label: CollaborationMode { + CollaborationMode::Custom(Settings { + model: model.to_string(), + reasoning_effort: None, + developer_instructions: None, + }) + } + + fn plan_collaboration_mode(model: &str) -> CollaborationMode { + CollaborationMode::Plan(Settings { + model: model.to_string(), + reasoning_effort: None, + developer_instructions: None, + }) + } + fn render_lines(lines: &[Line<'static>]) -> Vec { lines .iter() @@ -2420,6 +2498,8 @@ mod tests { Some(ReasoningEffortConfig::High), std::env::temp_dir(), "test", + false, + default_collaboration_mode("gpt-4o"), ); let lines = render_lines(&cell.display_lines(80)); @@ -2432,6 +2512,51 @@ mod tests { assert!(model_line.contains("/model to change")); } + #[test] + fn session_header_collaboration_mode_includes_label_and_hint() { + let cell = SessionHeaderHistoryCell::new( + "gpt-4o".to_string(), + Style::default(), + None, + std::env::temp_dir(), + "test", + true, + plan_collaboration_mode("gpt-4o"), + ); + + let lines = render_lines(&cell.display_lines(80)); + let mode_line = lines + .into_iter() + .find(|line| line.contains("mode:")) + .expect("mode line"); + + assert!(mode_line.contains("Plan")); + assert!(mode_line.contains("shift + tab")); + assert!(mode_line.contains("to change mode")); + } + + #[test] + fn session_header_collaboration_mode_uses_placeholder_when_loading() { + let cell = SessionHeaderHistoryCell::new( + DEFAULT_MODEL_DISPLAY_NAME.to_string(), + Style::default(), + None, + std::env::temp_dir(), + "test", + true, + plan_collaboration_mode("gpt-4o"), + ); + + let lines = render_lines(&cell.display_lines(80)); + let mode_line = lines + .into_iter() + .find(|line| line.contains("mode:")) + .expect("mode line"); + + assert!(mode_line.contains(DEFAULT_MODEL_DISPLAY_NAME)); + assert!(!mode_line.contains("Plan")); + } + #[test] fn session_header_directory_center_truncates() { let mut dir = home_dir().expect("home directory"); diff --git a/codex-rs/tui2/src/ui_consts.rs b/codex-rs/tui2/src/ui_consts.rs index ebbe4ce36..a24c6d5af 100644 --- a/codex-rs/tui2/src/ui_consts.rs +++ b/codex-rs/tui2/src/ui_consts.rs @@ -9,3 +9,4 @@ /// - User history lines account for this many columns (e.g., "▌ ") when wrapping. pub(crate) const LIVE_PREFIX_COLS: u16 = 2; pub(crate) const FOOTER_INDENT_COLS: usize = LIVE_PREFIX_COLS as usize; +pub(crate) const DEFAULT_MODEL_DISPLAY_NAME: &str = "loading";