mirror of
https://github.com/openai/codex.git
synced 2026-10-21 12:10:06 +00:00
Condense TUI startup warnings (#42609)
## What changed - Coalesce configuration, skill, sandbox, and MCP startup diagnostics into a single summary below the session header, including MCP and sign-in counts. - Keep the complete warning text in the transcript and update the summary's transcript shortcut when key bindings change. - Continue rendering warnings that arrive after work begins as inline history entries. - Hide the MCP startup status row before a turn while preserving the configured interrupt shortcut. ## Testing - Add coverage for warning ordering and deduplication, transcript details, MCP sign-in counts, resumed sessions, runtime warnings, reflow, backtracking, and interruption without a visible status row. GitOrigin-RevId: 009657cf04f1d3a52270cf096eebf9db7513c040
This commit is contained in:
@@ -242,6 +242,7 @@ mod session_picker;
|
||||
mod side;
|
||||
mod startup;
|
||||
mod startup_prompts;
|
||||
mod startup_warnings;
|
||||
mod thread_event_buffer;
|
||||
mod thread_events;
|
||||
mod thread_goal_actions;
|
||||
|
||||
@@ -70,7 +70,11 @@ impl App {
|
||||
}
|
||||
AppEvent::CloseMisalignmentReview => self.chat_widget.show_misalignment_policy_precaution(),
|
||||
AppEvent::SkillsListLoaded { ref cwd, .. }
|
||||
| AppEvent::PluginMentionsLoaded { ref cwd, .. }
|
||||
if cwds_differ(cwd, self.config.cwd.as_path()) =>
|
||||
{
|
||||
self.skill_load_warnings.startup_complete = true;
|
||||
}
|
||||
AppEvent::PluginMentionsLoaded { ref cwd, .. }
|
||||
if cwds_differ(cwd, self.config.cwd.as_path()) => {}
|
||||
AppEvent::NewSession { name } => {
|
||||
self.start_fresh_session_with_summary_hint(
|
||||
@@ -1147,6 +1151,7 @@ impl App {
|
||||
result.map_err(|err| color_eyre::eyre::eyre!(err)),
|
||||
"failed to load skills on startup",
|
||||
);
|
||||
self.skill_load_warnings.startup_complete = true;
|
||||
}
|
||||
AppEvent::StartFileSearch(query) => {
|
||||
self.file_search.on_user_query(query.clone());
|
||||
@@ -2973,9 +2978,11 @@ impl App {
|
||||
} => {
|
||||
self.apply_keymap_capture(context, action, key, intent)
|
||||
.await;
|
||||
self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default());
|
||||
}
|
||||
AppEvent::KeymapCleared { context, action } => {
|
||||
self.apply_keymap_clear(context, action).await;
|
||||
self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default());
|
||||
}
|
||||
AppEvent::GenerateRecap { thread_id } => {
|
||||
if self.current_displayed_thread_id() == Some(thread_id) {
|
||||
|
||||
@@ -22,6 +22,14 @@ pub(super) struct ThreadUsageStatusHistory {
|
||||
|
||||
impl App {
|
||||
pub(super) fn insert_history_cell(&mut self, tui: &mut tui::Tui, cell: Box<dyn HistoryCell>) {
|
||||
if let Some(warnings) = cell
|
||||
.as_any()
|
||||
.downcast_ref::<history_cell::StartupWarningsCell>()
|
||||
{
|
||||
self.merge_startup_warnings(tui, warnings);
|
||||
return;
|
||||
}
|
||||
let is_session_header = cell.as_any().is::<history_cell::SessionInfoCell>();
|
||||
let cell: Arc<dyn HistoryCell> = cell.into();
|
||||
if let Some(Overlay::Transcript(t)) = &mut self.overlay {
|
||||
t.insert_cell(cell.clone());
|
||||
@@ -62,6 +70,9 @@ impl App {
|
||||
// A committed cell can unblock a settled /usage card that was waiting
|
||||
// behind a transient active cell or a provisional stream tail.
|
||||
self.chat_widget.request_pending_usage_output_insertion();
|
||||
if is_session_header {
|
||||
self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default());
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn finish_thread_usage_refresh(
|
||||
|
||||
@@ -801,8 +801,18 @@ impl App {
|
||||
}
|
||||
Err(err) if self.recover_transport_error(&err) => {}
|
||||
Err(err) => {
|
||||
let warnings = self
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.find_map(|cell| {
|
||||
cell.as_any()
|
||||
.downcast_ref::<history_cell::StartupWarningsCell>()
|
||||
})
|
||||
.filter(|cell| cell.pending_header)
|
||||
.map(|cell| format!("\n\nStartup warnings:\n{}", cell.messages.join("\n")))
|
||||
.unwrap_or_default();
|
||||
return Err(color_eyre::eyre::eyre!(
|
||||
"Failed to start a fresh session through the app server: {err}"
|
||||
"Failed to start a fresh session through the app server: {err}{warnings}"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -127,7 +127,11 @@ impl App {
|
||||
let startup_started_at = Instant::now();
|
||||
let (app_event_tx, mut app_event_rx) = unbounded_channel();
|
||||
let app_event_tx = AppEventSender::new(app_event_tx);
|
||||
emit_project_config_warnings(&app_event_tx, &config);
|
||||
if let Some(message) = project_config_warning(&config) {
|
||||
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::StartupWarningsCell::new(vec![message]),
|
||||
)));
|
||||
}
|
||||
emit_system_bwrap_warning(&app_event_tx, &config);
|
||||
tui.set_notification_settings(
|
||||
local_settings.tui.notification_settings.method,
|
||||
|
||||
@@ -17,6 +17,8 @@ struct SkillLoadWarningKey {
|
||||
#[derive(Debug, Default)]
|
||||
pub(super) struct SkillLoadWarningState {
|
||||
active: HashSet<SkillLoadWarningKey>,
|
||||
// Both session attachment and the background startup fetch can finish the initial load.
|
||||
pub(super) startup_complete: bool,
|
||||
}
|
||||
|
||||
impl SkillLoadWarningState {
|
||||
@@ -45,28 +47,24 @@ impl SkillLoadWarningState {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn emit_skill_load_warnings(app_event_tx: &AppEventSender, errors: &[SkillErrorInfo]) {
|
||||
pub(super) fn skill_load_warning_messages(errors: &[SkillErrorInfo]) -> Vec<String> {
|
||||
if errors.is_empty() {
|
||||
return;
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let error_count = errors.len();
|
||||
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
crate::history_cell::new_warning_event(format!(
|
||||
"Skipped loading {error_count} skill(s) due to invalid SKILL.md files."
|
||||
)),
|
||||
)));
|
||||
|
||||
for error in errors {
|
||||
let path = error.path.display();
|
||||
let message = error.message.as_str();
|
||||
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
crate::history_cell::new_warning_event(format!("{path}: {message}")),
|
||||
)));
|
||||
}
|
||||
let mut messages = vec![format!(
|
||||
"Skipped loading {error_count} skill(s) due to invalid SKILL.md files."
|
||||
)];
|
||||
messages.extend(
|
||||
errors
|
||||
.iter()
|
||||
.map(|error| format!("{}: {}", error.path.display(), error.message)),
|
||||
);
|
||||
messages
|
||||
}
|
||||
|
||||
pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config: &Config) {
|
||||
pub(super) fn project_config_warning(config: &Config) -> Option<String> {
|
||||
let mut disabled_folders = Vec::new();
|
||||
|
||||
for layer in config.config_layer_stack.all_layers_low_to_high() {
|
||||
@@ -83,7 +81,7 @@ pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config
|
||||
}
|
||||
|
||||
if disabled_folders.is_empty() {
|
||||
return;
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut message = concat!(
|
||||
@@ -97,9 +95,7 @@ pub(super) fn emit_project_config_warnings(app_event_tx: &AppEventSender, config
|
||||
message.push_str(&format!(" {reason}\n"));
|
||||
}
|
||||
|
||||
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_warning_event(message),
|
||||
)));
|
||||
Some(message)
|
||||
}
|
||||
|
||||
pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &Config) {
|
||||
@@ -110,7 +106,7 @@ pub(super) fn emit_system_bwrap_warning(app_event_tx: &AppEventSender, config: &
|
||||
};
|
||||
|
||||
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_warning_event(message),
|
||||
history_cell::StartupWarningsCell::new(vec![message]),
|
||||
)));
|
||||
}
|
||||
|
||||
@@ -424,7 +420,11 @@ mod tests {
|
||||
let (tx, mut rx) = unbounded_channel();
|
||||
let app_event_tx = AppEventSender::new(tx);
|
||||
|
||||
emit_skill_load_warnings(&app_event_tx, errors);
|
||||
for message in skill_load_warning_messages(errors) {
|
||||
app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_warning_event(message),
|
||||
)));
|
||||
}
|
||||
|
||||
let mut rendered = Vec::new();
|
||||
while let Ok(AppEvent::InsertHistoryCell(cell)) = rx.try_recv() {
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
//! Coalesces startup diagnostics directly below the session splash.
|
||||
|
||||
use super::*;
|
||||
use crate::history_cell::StartupWarningsCell;
|
||||
|
||||
impl App {
|
||||
pub(super) fn merge_startup_warnings(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
incoming: &StartupWarningsCell,
|
||||
) {
|
||||
let existing = self
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.position(|cell| cell.as_any().is::<StartupWarningsCell>());
|
||||
let mut warnings = existing
|
||||
.map(|index| self.transcript_cells.remove(index))
|
||||
.and_then(|cell| cell.as_any().downcast_ref::<StartupWarningsCell>().cloned())
|
||||
.unwrap_or_default();
|
||||
for message in &incoming.messages {
|
||||
if !warnings.messages.contains(message) {
|
||||
warnings.messages.push(message.clone());
|
||||
}
|
||||
}
|
||||
warnings
|
||||
.other_sources
|
||||
.extend(incoming.other_sources.iter().cloned());
|
||||
warnings
|
||||
.mcp_servers
|
||||
.extend(incoming.mcp_servers.iter().cloned());
|
||||
// The final MCP summary must retain the individual diagnostics' sign-in subset.
|
||||
warnings
|
||||
.sign_in_servers
|
||||
.extend(incoming.sign_in_servers.iter().cloned());
|
||||
if warnings.messages.is_empty() {
|
||||
return;
|
||||
}
|
||||
warnings.transcript_hint = crate::keymap::primary_binding(&self.keymap.app.open_transcript)
|
||||
.map(|binding| binding.display_label());
|
||||
let header = self
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.rposition(|cell| cell.as_any().is::<history_cell::SessionInfoCell>());
|
||||
warnings.pending_header = header.is_none() && self.chat_widget.thread_id().is_none();
|
||||
let ready_to_render = !warnings.pending_header;
|
||||
self.transcript_cells
|
||||
.insert(header.map_or(0, |index| index + 1), Arc::new(warnings));
|
||||
if let Some(Overlay::Transcript(overlay)) = &mut self.overlay {
|
||||
overlay.replace_cells(self.transcript_cells.clone());
|
||||
}
|
||||
if self.backtrack.overlay_preview_active {
|
||||
self.apply_backtrack_selection_internal(self.backtrack.nth_user_message);
|
||||
}
|
||||
if ready_to_render {
|
||||
if let Some(buffer) = self.initial_history_replay_buffer.as_mut() {
|
||||
buffer.retained_lines.clear();
|
||||
buffer.render_from_transcript_tail = true;
|
||||
return;
|
||||
}
|
||||
self.schedule_immediate_resize_reflow(tui);
|
||||
if let Err(error) =
|
||||
self.maybe_run_resize_reflow(tui, tui.terminal.last_known_screen_size)
|
||||
{
|
||||
tracing::warn!(%error, "failed to refresh startup warnings");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -40,6 +40,8 @@ mod safety_buffering;
|
||||
mod session_lifecycle_requests;
|
||||
mod session_summary;
|
||||
mod startup;
|
||||
#[path = "tests/startup_warnings_tests.rs"]
|
||||
mod startup_warnings_tests;
|
||||
#[path = "tests/stream_animation_tests.rs"]
|
||||
mod stream_animation_tests;
|
||||
#[path = "tests/thread_usage.rs"]
|
||||
@@ -4847,7 +4849,9 @@ async fn primary_thread_ignores_child_mcp_startup_notifications() {
|
||||
let mut rendered_cells = Vec::new();
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
rendered_cells.push(lines_to_single_string(&cell.display_lines(/*width*/ 120)));
|
||||
rendered_cells.push(lines_to_single_string(
|
||||
&cell.transcript_lines(/*width*/ 120),
|
||||
));
|
||||
}
|
||||
}
|
||||
let rendered = rendered_cells.join("\n");
|
||||
@@ -4964,12 +4968,24 @@ async fn active_side_thread_renders_live_mcp_startup_notifications() {
|
||||
app.handle_thread_event_now(event);
|
||||
}
|
||||
|
||||
let mut tui = crate::tui::test_support::make_test_tui().expect("test tui");
|
||||
let mut rendered_cells = Vec::new();
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
rendered_cells.push(lines_to_single_string(&cell.display_lines(/*width*/ 120)));
|
||||
rendered_cells.push(lines_to_single_string(
|
||||
&cell.transcript_lines(/*width*/ 120),
|
||||
));
|
||||
app.insert_history_cell(&mut tui, cell);
|
||||
}
|
||||
}
|
||||
let inline = app
|
||||
.transcript_cells
|
||||
.iter()
|
||||
.flat_map(|cell| cell.display_lines(/*width*/ 120))
|
||||
.map(|line| line.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(inline.contains("1 MCP startup issue"));
|
||||
let rendered = rendered_cells.join("\n");
|
||||
assert!(app.chat_widget.side_conversation_active());
|
||||
assert_eq!(rendered.matches("sentry is not logged in").count(), 1);
|
||||
|
||||
@@ -86,9 +86,9 @@ async fn subagent_mcp_startup_settles_while_cached_servers_remain_deferred() {
|
||||
}
|
||||
|
||||
insta::assert_snapshot!(visible_startup_states.join("\n"), @r"
|
||||
eager: • Booting MCP server: eager
|
||||
eager: idle
|
||||
deferred: • Booting MCP server: deferred
|
||||
eager: idle
|
||||
deferred: idle
|
||||
deferred: idle
|
||||
");
|
||||
}
|
||||
|
||||
+9
@@ -0,0 +1,9 @@
|
||||
---
|
||||
source: tui/src/app/tests/startup_warnings_tests.rs
|
||||
expression: lines_to_single_string(&warnings.transcript_lines(40))
|
||||
---
|
||||
⚠ MCP alpha: connection unavailable
|
||||
⚠ MCP beta: connection unavailable
|
||||
⚠ MCP gamma: connection unavailable
|
||||
⚠ MCP startup incomplete (failed: alpha,
|
||||
beta, gamma)
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
---
|
||||
source: tui/src/app/tests/startup_warnings_tests.rs
|
||||
expression: warnings.display_lines(100)
|
||||
---
|
||||
[
|
||||
Line::from(Span::from("⚠ 3 MCP startup issues (2 need sign-in) · ctrl + t for details").yellow().dim()),
|
||||
]
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
---
|
||||
source: tui/src/app/tests/startup_warnings_tests.rs
|
||||
expression: "app.transcript_cells[1].display_lines(100)"
|
||||
---
|
||||
[
|
||||
Line::from(Span::from("⚠ 4 startup issues (3 MCP; 2 need sign-in) · ctrl + t for details").yellow().dim()),
|
||||
]
|
||||
@@ -1240,6 +1240,13 @@ async fn owned_subagent_approval_before_thread_started_is_preserved() -> Result<
|
||||
async fn startup_thread_start_failure_returns_error() {
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
app.pending_startup_thread_start = true;
|
||||
let mut tui = crate::tui::test_support::make_test_tui().expect("test tui");
|
||||
app.insert_history_cell(
|
||||
&mut tui,
|
||||
Box::new(history_cell::StartupWarningsCell::new(vec![
|
||||
"Skill manifest is invalid.".to_string(),
|
||||
])),
|
||||
);
|
||||
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(
|
||||
app.chat_widget.config_ref(),
|
||||
@@ -1251,10 +1258,12 @@ async fn startup_thread_start_failure_returns_error() {
|
||||
.await
|
||||
.expect_err("startup thread failure should exit instead of leaving chat unconfigured");
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("Failed to start a fresh session through the app server: boom")
|
||||
);
|
||||
insta::assert_snapshot!(err.to_string(), @"
|
||||
Failed to start a fresh session through the app server: boom
|
||||
|
||||
Startup warnings:
|
||||
Skill manifest is invalid.
|
||||
");
|
||||
assert!(!app.pending_startup_thread_start);
|
||||
assert_eq!(app.primary_thread_id, None);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,211 @@
|
||||
//! Startup diagnostic ordering, aggregation, and transcript coverage.
|
||||
|
||||
use super::*;
|
||||
use crate::history_cell::StartupWarningsCell;
|
||||
use codex_app_server_protocol::McpServerStartupFailureReason;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::mpsc::UnboundedReceiver;
|
||||
|
||||
fn insert_warnings(app: &mut App, tui: &mut tui::Tui, messages: &[&str]) {
|
||||
app.insert_history_cell(
|
||||
tui,
|
||||
Box::new(StartupWarningsCell::new(
|
||||
messages.iter().map(|message| (*message).into()).collect(),
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
||||
fn insert_splash(app: &mut App, tui: &mut tui::Tui) {
|
||||
let session = test_thread_session(ThreadId::new(), app.config.cwd.to_path_buf());
|
||||
app.insert_history_cell(
|
||||
tui,
|
||||
Box::new(history_cell::new_session_info(
|
||||
&app.config,
|
||||
&app.local_settings,
|
||||
"gpt-test",
|
||||
&session,
|
||||
/*is_first_event*/ false,
|
||||
/*tooltip_override*/ None,
|
||||
/*auth_plan*/ None,
|
||||
/*show_fast_status*/ false,
|
||||
)),
|
||||
);
|
||||
}
|
||||
|
||||
fn drain_history(app: &mut App, tui: &mut tui::Tui, events: &mut UnboundedReceiver<AppEvent>) {
|
||||
while let Ok(event) = events.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
app.insert_history_cell(tui, cell);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_warnings_preserve_stream_repair_and_backtrack_selection() -> Result<()> {
|
||||
let mut app = crate::app::test_support::make_test_app().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.chat_widget.handle_thread_session(test_thread_session(
|
||||
ThreadId::new(),
|
||||
app.config.cwd.to_path_buf(),
|
||||
));
|
||||
app.transcript_cells = vec![Arc::new(UserHistoryCell {
|
||||
message: "Selected prompt".into(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
})];
|
||||
app.transcript_reflow.mark_resize_requested_during_stream();
|
||||
insert_warnings(&mut app, &mut tui, &["First warning"]);
|
||||
assert!(app.transcript_reflow.take_stream_finish_reflow_needed());
|
||||
// Recreate the insertion case with a backtracking overlay already open.
|
||||
app.transcript_cells.remove(0);
|
||||
app.backtrack.overlay_preview_active = true;
|
||||
app.open_transcript_overlay(&mut tui);
|
||||
app.apply_backtrack_selection_internal(/*nth_user_message*/ 0);
|
||||
insert_warnings(&mut app, &mut tui, &["Second warning"]);
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 12,
|
||||
);
|
||||
let mut buffer = Buffer::empty(area);
|
||||
let Some(Overlay::Transcript(overlay)) = app.overlay.as_mut() else {
|
||||
panic!("expected transcript")
|
||||
};
|
||||
overlay.render(area, &mut buffer);
|
||||
let highlighted = area
|
||||
.positions()
|
||||
.filter(|position| {
|
||||
buffer[*position]
|
||||
.style()
|
||||
.add_modifier
|
||||
.contains(ratatui::style::Modifier::REVERSED)
|
||||
})
|
||||
.map(|position| buffer[position].symbol())
|
||||
.collect::<String>();
|
||||
assert!(highlighted.contains("Selected prompt"), "{highlighted}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_startup_summary_counts_servers_and_sign_in_subset() -> Result<()> {
|
||||
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
app.chat_widget
|
||||
.set_mcp_startup_expected_servers(["alpha", "beta", "gamma"].map(str::to_owned));
|
||||
insert_splash(&mut app, &mut tui);
|
||||
let sign_in = Some(McpServerStartupFailureReason::ReauthenticationRequired);
|
||||
for (name, failure_reason) in [("alpha", sign_in), ("beta", sign_in), ("gamma", None)] {
|
||||
app.chat_widget.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: None,
|
||||
name: name.into(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some(format!("MCP {name}: connection unavailable")),
|
||||
failure_reason,
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
}
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 2);
|
||||
let warnings = &app.transcript_cells[1];
|
||||
insta::assert_debug_snapshot!("mcp_startup_summary", warnings.display_lines(/*width*/ 100));
|
||||
insta::assert_snapshot!(
|
||||
"mcp_startup_details",
|
||||
lines_to_single_string(&warnings.transcript_lines(/*width*/ 40))
|
||||
);
|
||||
insert_warnings(&mut app, &mut tui, &["Skill manifest is invalid."]);
|
||||
insta::assert_debug_snapshot!(
|
||||
"mixed_startup_summary",
|
||||
app.transcript_cells[1].display_lines(/*width*/ 100)
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_warnings_wait_for_splash_and_coalesce_with_full_details() -> Result<()> {
|
||||
let mut app = crate::app::test_support::make_test_app().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let first = "Skill manifest is invalid.";
|
||||
insert_warnings(&mut app, &mut tui, &[first]);
|
||||
let pending = &app.transcript_cells[0];
|
||||
assert!(pending.display_lines(/*width*/ 80).is_empty());
|
||||
|
||||
insert_splash(&mut app, &mut tui);
|
||||
insert_warnings(&mut app, &mut tui, &[first, "MCP alpha failed to start."]);
|
||||
|
||||
assert_eq!(app.transcript_cells.len(), 2);
|
||||
let header = app.transcript_cells[0].as_any();
|
||||
assert!(header.is::<history_cell::SessionInfoCell>());
|
||||
let display = app.transcript_cells[1].display_lines(/*width*/ 80);
|
||||
let transcript = app.transcript_cells[1].transcript_lines(/*width*/ 80);
|
||||
insta::assert_snapshot!(format!("display:\n{}\n\ntranscript:\n{}", lines_to_single_string(&display), lines_to_single_string(&transcript)), @"
|
||||
display:
|
||||
⚠ 2 startup issues · ctrl + t for details
|
||||
|
||||
transcript:
|
||||
⚠ Skill manifest is invalid.
|
||||
⚠ MCP alpha failed to start.
|
||||
");
|
||||
app.keymap.app.open_transcript = vec![crate::key_hint::plain(crossterm::event::KeyCode::F(12))];
|
||||
app.merge_startup_warnings(&mut tui, &StartupWarningsCell::default());
|
||||
insta::assert_snapshot!(lines_to_single_string(&app.transcript_cells[1].display_lines(/*width*/ 80)), @"⚠ 2 startup issues · f12 for details");
|
||||
app.keymap.app.open_transcript.clear();
|
||||
app.merge_startup_warnings(&mut tui, &StartupWarningsCell::default());
|
||||
insta::assert_snapshot!(lines_to_single_string(&app.transcript_cells[1].display_lines(/*width*/ 80)), @"⚠ 2 startup issues");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_skill_load_order_preserves_runtime_error_recurrence() -> Result<()> {
|
||||
for background_first in [false, true] {
|
||||
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
let mut server = start_config_write_test_app_server(&app).await?;
|
||||
let cwd = app.config.cwd.to_path_buf();
|
||||
let response = SkillsListResponse {
|
||||
data: vec![codex_app_server_protocol::SkillsListEntry {
|
||||
cwd: cwd.clone(),
|
||||
skills: vec![],
|
||||
errors: vec![SkillErrorInfo {
|
||||
path: PathBuf::from("bad/SKILL.md"),
|
||||
message: "invalid description".into(),
|
||||
}],
|
||||
}],
|
||||
};
|
||||
// Session attachment and the background startup request may finish in either order.
|
||||
for background in [background_first, !background_first] {
|
||||
if background {
|
||||
app.handle_event(
|
||||
&mut tui,
|
||||
&mut server,
|
||||
AppEvent::SkillsListLoaded {
|
||||
cwd: cwd.clone(),
|
||||
result: Ok(response.clone()),
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
} else {
|
||||
app.handle_skills_list_result(Ok(response.clone()), "initial session refresh");
|
||||
}
|
||||
}
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 1);
|
||||
assert!(app.transcript_cells[0].as_any().is::<StartupWarningsCell>());
|
||||
app.handle_skills_list_result(Ok(SkillsListResponse { data: vec![] }), "clean refresh");
|
||||
app.handle_skills_list_result(Ok(response), "runtime refresh");
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 3);
|
||||
let runtime = app.transcript_cells[1..]
|
||||
.iter()
|
||||
.flat_map(|cell| cell.display_lines(/*width*/ 120))
|
||||
.collect::<Vec<_>>();
|
||||
insta::allow_duplicates! {
|
||||
insta::assert_snapshot!(lines_to_single_string(&runtime), @"
|
||||
⚠ Skipped loading 1 skill(s) due to invalid SKILL.md files.
|
||||
⚠ bad/SKILL.md: invalid description
|
||||
");
|
||||
}
|
||||
server.shutdown().await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -1686,7 +1686,19 @@ impl App {
|
||||
let cwd = self.chat_widget.config_ref().cwd.clone();
|
||||
let errors = errors_for_cwd(&cwd, &response);
|
||||
let errors = self.skill_load_warnings.newly_active_errors(&errors);
|
||||
emit_skill_load_warnings(&self.app_event_tx, &errors);
|
||||
let warnings = skill_load_warning_messages(&errors);
|
||||
if self.skill_load_warnings.startup_complete {
|
||||
for warning in warnings {
|
||||
self.chat_widget.add_warning_message(warning);
|
||||
}
|
||||
} else {
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
// The per-file diagnostics already identify every affected skill.
|
||||
history_cell::StartupWarningsCell::new(
|
||||
warnings.into_iter().skip(/*n*/ 1).collect(),
|
||||
),
|
||||
)));
|
||||
}
|
||||
self.chat_widget.handle_skills_list_response(response);
|
||||
}
|
||||
|
||||
|
||||
@@ -209,9 +209,12 @@ impl App {
|
||||
}
|
||||
self.cancel_pending_key_chord();
|
||||
self.keymap = keymap;
|
||||
self.merge_startup_warnings(tui, &history_cell::StartupWarningsCell::default());
|
||||
self.restore_runtime_theme_from_config();
|
||||
self.runtime_working_directory_override = Some(cwd.to_path_buf());
|
||||
emit_project_config_warnings(&self.app_event_tx, &self.config);
|
||||
if let Some(message) = project_config_warning(&self.config) {
|
||||
self.chat_widget.add_warning_message(message);
|
||||
}
|
||||
let message = format!("Working directory changed to: {}", cwd.display());
|
||||
self.chat_widget.add_info_message(message, /*hint*/ None);
|
||||
if !self.config.bypass_hook_trust {
|
||||
|
||||
@@ -738,14 +738,15 @@ impl BottomPane {
|
||||
if self.handle_inline_banner_key(key_event) {
|
||||
return InputResult::None;
|
||||
}
|
||||
// If a task is running and a status line is visible, allow the
|
||||
// configured action to interrupt even while the composer has focus.
|
||||
// Allow the configured interrupt while a task is running, even when its
|
||||
// status row is hidden or the composer has focus.
|
||||
// When a popup is active, prefer dismissing it over interrupting the task.
|
||||
if self.should_interrupt_running_task(key_event)
|
||||
&& let Some(status) = &self.status
|
||||
{
|
||||
// Send Op::Interrupt
|
||||
status.interrupt();
|
||||
if self.should_interrupt_running_task(key_event) {
|
||||
if let Some(status) = &self.status {
|
||||
status.interrupt();
|
||||
} else {
|
||||
self.app_event_tx.interrupt();
|
||||
}
|
||||
self.request_redraw();
|
||||
return InputResult::None;
|
||||
}
|
||||
@@ -1499,7 +1500,6 @@ impl BottomPane {
|
||||
&& !(is_agent_command && key_event.code == KeyCode::Esc)
|
||||
&& self.no_modal_or_popup_active()
|
||||
&& !self.composer_should_handle_vim_insert_escape(key_event)
|
||||
&& self.status_widget().is_some()
|
||||
}
|
||||
|
||||
pub(crate) fn terminal_title_requires_action(&self) -> bool {
|
||||
@@ -3213,6 +3213,13 @@ mod tests {
|
||||
matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt))),
|
||||
"expected Esc to send Op::Interrupt while a task is running"
|
||||
);
|
||||
|
||||
pane.hide_status_indicator();
|
||||
pane.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
assert!(matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::CodexOp(Op::Interrupt))
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -3237,6 +3244,13 @@ mod tests {
|
||||
matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt))),
|
||||
"expected configured key to interrupt while `/subagents` is being edited"
|
||||
);
|
||||
|
||||
pane.hide_status_indicator();
|
||||
pane.handle_key_event(KeyEvent::new(KeyCode::F(12), KeyModifiers::NONE));
|
||||
assert!(matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::CodexOp(Op::Interrupt))
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1306,6 +1306,9 @@ impl ChatWidget {
|
||||
self.bottom_pane.set_task_running(/*running*/ true);
|
||||
}
|
||||
self.review.is_review_mode = true;
|
||||
if !from_replay {
|
||||
self.bottom_pane.ensure_status_indicator();
|
||||
}
|
||||
let banner = format!(">> Code review started: {hint} <<");
|
||||
self.add_to_history(history_cell::new_review_status_line(banner));
|
||||
self.request_redraw();
|
||||
|
||||
@@ -3,9 +3,12 @@
|
||||
//! The app server reports MCP server startup as per-server status updates. This
|
||||
//! module keeps the TUI's buffered startup round state coherent and translates
|
||||
//! those updates into status headers, warnings, and queued-input release points.
|
||||
//! Initial diagnostics coalesce beneath the splash; warnings after a turn starts
|
||||
//! stay inline so failures during ongoing work remain visible.
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
use codex_app_server_protocol::McpServerStartupFailureReason;
|
||||
use codex_app_server_protocol::McpServerStartupState;
|
||||
use codex_app_server_protocol::McpServerStatusUpdatedNotification;
|
||||
|
||||
@@ -14,11 +17,14 @@ use super::ChatWidget;
|
||||
const MCP_STARTUP_SINGLE_HEADER_PREFIX: &str = "Booting MCP server:";
|
||||
const MCP_STARTUP_MULTI_HEADER_PREFIX: &str = "Starting MCP servers";
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum McpStartupStatus {
|
||||
Starting,
|
||||
Ready,
|
||||
Failed { error: String },
|
||||
Failed {
|
||||
error: String,
|
||||
failure_reason: Option<McpServerStartupFailureReason>,
|
||||
},
|
||||
Cancelled,
|
||||
}
|
||||
|
||||
@@ -82,23 +88,34 @@ impl ChatWidget {
|
||||
// Normal path: fold the update into the active round and surface
|
||||
// per-server failures immediately.
|
||||
let mut startup_status = self.mcp_startup_status.take().unwrap_or_default();
|
||||
if let McpStartupStatus::Failed { error } = &status {
|
||||
let already_reported = matches!(
|
||||
startup_status.get(&server),
|
||||
Some(McpStartupStatus::Failed { error: previous }) if previous == error
|
||||
if let McpStartupStatus::Failed {
|
||||
error,
|
||||
failure_reason,
|
||||
} = &status
|
||||
&& startup_status.get(&server) != Some(&status)
|
||||
{
|
||||
self.add_mcp_startup_warning(
|
||||
vec![error.clone()],
|
||||
[server.clone()],
|
||||
*failure_reason,
|
||||
);
|
||||
if !already_reported {
|
||||
self.on_warning(error);
|
||||
}
|
||||
}
|
||||
startup_status.insert(server, status);
|
||||
startup_status
|
||||
};
|
||||
if activated_pending_round {
|
||||
// A promoted buffered round may already contain terminal failures.
|
||||
for state in startup_status.values() {
|
||||
if let McpStartupStatus::Failed { error } = state {
|
||||
self.on_warning(error);
|
||||
for (server, state) in &startup_status {
|
||||
if let McpStartupStatus::Failed {
|
||||
error,
|
||||
failure_reason,
|
||||
} = state
|
||||
{
|
||||
self.add_mcp_startup_warning(
|
||||
vec![error.clone()],
|
||||
[server.clone()],
|
||||
*failure_reason,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -184,17 +201,25 @@ impl ChatWidget {
|
||||
|
||||
pub(super) fn finish_mcp_startup(&mut self, failed: Vec<String>, cancelled: Vec<String>) {
|
||||
if !cancelled.is_empty() {
|
||||
self.on_warning(format!(
|
||||
"MCP startup interrupted. The following servers were not initialized: {}",
|
||||
cancelled.join(", ")
|
||||
));
|
||||
self.add_mcp_startup_warning(
|
||||
vec![format!(
|
||||
"MCP startup interrupted. The following servers were not initialized: {}",
|
||||
cancelled.join(", ")
|
||||
)],
|
||||
cancelled,
|
||||
/*failure_reason*/ None,
|
||||
);
|
||||
}
|
||||
let mut parts = Vec::new();
|
||||
if !failed.is_empty() {
|
||||
parts.push(format!("failed: {}", failed.join(", ")));
|
||||
}
|
||||
if !parts.is_empty() {
|
||||
self.on_warning(format!("MCP startup incomplete ({})", parts.join("; ")));
|
||||
self.add_mcp_startup_warning(
|
||||
vec![format!("MCP startup incomplete ({})", parts.join("; "))],
|
||||
failed,
|
||||
/*failure_reason*/ None,
|
||||
);
|
||||
}
|
||||
|
||||
let mcp_startup_owned_status = self.status_header_is_mcp_startup_owned();
|
||||
@@ -262,6 +287,25 @@ impl ChatWidget {
|
||||
.starts_with(MCP_STARTUP_MULTI_HEADER_PREFIX)
|
||||
}
|
||||
|
||||
fn add_mcp_startup_warning(
|
||||
&mut self,
|
||||
messages: Vec<String>,
|
||||
servers: impl IntoIterator<Item = String>,
|
||||
failure_reason: Option<McpServerStartupFailureReason>,
|
||||
) {
|
||||
if self.warning_display_state.startup_complete {
|
||||
for message in messages {
|
||||
self.on_warning(message);
|
||||
}
|
||||
} else {
|
||||
self.add_to_history(crate::history_cell::StartupWarningsCell::mcp(
|
||||
messages,
|
||||
servers,
|
||||
failure_reason,
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
/// Update startup state and retry installed-app discovery when its MCP server becomes ready.
|
||||
pub(super) fn on_mcp_server_status_updated(
|
||||
&mut self,
|
||||
@@ -273,6 +317,7 @@ impl ChatWidget {
|
||||
McpServerStartupState::Starting => McpStartupStatus::Starting,
|
||||
McpServerStartupState::Ready => McpStartupStatus::Ready,
|
||||
McpServerStartupState::Failed => McpStartupStatus::Failed {
|
||||
failure_reason: notification.failure_reason,
|
||||
error: notification.error.unwrap_or_else(|| {
|
||||
format!("MCP client for `{}` failed to start", notification.name)
|
||||
}),
|
||||
|
||||
@@ -66,6 +66,7 @@ impl ChatWidget {
|
||||
self.turn_lifecycle.last_turn_id = Some(notification.turn.id);
|
||||
self.last_non_retry_error = None;
|
||||
if !matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages)) {
|
||||
self.warning_display_state.startup_complete = true;
|
||||
self.on_task_started();
|
||||
}
|
||||
}
|
||||
@@ -194,12 +195,18 @@ impl ChatWidget {
|
||||
ServerNotification::DeprecationNotice(notification) => {
|
||||
self.on_deprecation_notice(notification.summary, notification.details)
|
||||
}
|
||||
ServerNotification::ConfigWarning(notification) => self.on_warning(
|
||||
notification
|
||||
ServerNotification::ConfigWarning(notification) => {
|
||||
let message = notification
|
||||
.details
|
||||
.map(|details| format!("{}: {details}", notification.summary))
|
||||
.unwrap_or(notification.summary),
|
||||
),
|
||||
.unwrap_or(notification.summary);
|
||||
if self.warning_display_state.startup_complete {
|
||||
self.on_warning(message);
|
||||
} else if self.warning_display_state.should_display(&message) {
|
||||
self.add_to_history(history_cell::StartupWarningsCell::new(vec![message]));
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
ServerNotification::McpServerStatusUpdated(notification) => {
|
||||
self.on_mcp_server_status_updated(notification)
|
||||
}
|
||||
|
||||
@@ -30,6 +30,9 @@ impl ChatWidget {
|
||||
/// avoid triggering side effects. Event ids are passed as `None` to
|
||||
/// distinguish replayed events from live ones.
|
||||
pub(crate) fn replay_thread_turns(&mut self, turns: Vec<Turn>, replay_kind: ReplayKind) {
|
||||
if matches!(replay_kind, ReplayKind::ThreadSnapshot) && !turns.is_empty() {
|
||||
self.warning_display_state.startup_complete = true;
|
||||
}
|
||||
let latest_turn_id = turns.last().map(|turn| turn.id.clone());
|
||||
let hidden_nested_review_turns = std::iter::once(/*value*/ false)
|
||||
.chain(turns.windows(/*size*/ 2).map(|turns| {
|
||||
@@ -48,6 +51,7 @@ impl ChatWidget {
|
||||
duration_ms,
|
||||
} = turn;
|
||||
if matches!(status, TurnStatus::InProgress) {
|
||||
self.warning_display_state.startup_complete = true;
|
||||
self.turn_lifecycle.last_turn_id = Some(turn_id.clone());
|
||||
self.last_non_retry_error = None;
|
||||
self.on_task_started();
|
||||
|
||||
-2
@@ -3,8 +3,6 @@ source: tui/src/chatwidget/tests/mcp_startup.rs
|
||||
expression: normalized_backend_snapshot(terminal.backend())
|
||||
---
|
||||
" "
|
||||
"• Booting MCP server: alpha (0s • esc to interrupt) "
|
||||
" "
|
||||
" "
|
||||
"› Ask Codex to do anything "
|
||||
" "
|
||||
|
||||
@@ -935,6 +935,29 @@ async fn live_app_server_strict_review_required_notification_renders_message() {
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn config_warning_during_turn_remains_inline() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::ConfigWarning(ConfigWarningNotification {
|
||||
summary: "Invalid configuration; using defaults.".into(),
|
||||
details: Some("Check the project configuration.".into()),
|
||||
path: None,
|
||||
range: None,
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
insta::assert_snapshot!(
|
||||
"runtime_config_warning",
|
||||
cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn live_app_server_config_warning_prefixes_summary() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
@@ -949,7 +972,7 @@ async fn live_app_server_config_warning_prefixes_summary() {
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let cells = drain_insert_history_transcript(&mut rx);
|
||||
assert_eq!(cells.len(), 1, "expected one warning history cell");
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
|
||||
@@ -330,11 +330,24 @@ pub(crate) async fn make_chatwidget_manual_with_sender() -> (
|
||||
|
||||
pub(super) fn drain_insert_history(
|
||||
rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
) -> Vec<Vec<ratatui::text::Line<'static>>> {
|
||||
drain_insert_history_with(rx, |cell| cell.display_lines(/*width*/ 80))
|
||||
}
|
||||
|
||||
pub(super) fn drain_insert_history_transcript(
|
||||
rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
) -> Vec<Vec<ratatui::text::Line<'static>>> {
|
||||
drain_insert_history_with(rx, |cell| cell.transcript_lines(/*width*/ 80))
|
||||
}
|
||||
|
||||
fn drain_insert_history_with(
|
||||
rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
render: impl Fn(&dyn HistoryCell) -> Vec<ratatui::text::Line<'static>>,
|
||||
) -> Vec<Vec<ratatui::text::Line<'static>>> {
|
||||
let mut out = Vec::new();
|
||||
while let Ok(ev) = rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = ev {
|
||||
let mut lines = cell.display_lines(/*width*/ 80);
|
||||
let mut lines = render(cell.as_ref());
|
||||
if !cell.is_stream_continuation() && !out.is_empty() && !lines.is_empty() {
|
||||
lines.insert(0, "".into());
|
||||
}
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use super::helpers::drain_insert_history_transcript as drain_insert_history;
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
@@ -134,6 +135,8 @@ async fn mcp_startup_header_booting_snapshot() {
|
||||
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting);
|
||||
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
assert!(!chat.bottom_pane.status_indicator_visible());
|
||||
let height = chat.desired_height(/*width*/ 80);
|
||||
let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height))
|
||||
.expect("create terminal");
|
||||
@@ -146,6 +149,55 @@ async fn mcp_startup_header_booting_snapshot() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_startup_updates_preserve_streaming_status_suppression() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.set_mcp_startup_expected_servers(["alpha".into(), "beta".into()]);
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
chat.on_agent_message_delta("Partial response\n".into());
|
||||
chat.on_commit_tick();
|
||||
assert!(!chat.bottom_pane.status_indicator_visible());
|
||||
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Ready);
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
assert!(!chat.bottom_pane.status_indicator_visible());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_startup_summary_distinguishes_initial_resume_from_task_switch() {
|
||||
use AppServerTurnStatus::Completed;
|
||||
use AppServerTurnStatus::InProgress;
|
||||
use ReplayKind::ResumeInitialMessages;
|
||||
use ReplayKind::ThreadSnapshot;
|
||||
|
||||
for (replay_kind, status, compact) in [
|
||||
(ResumeInitialMessages, Completed, true),
|
||||
(ResumeInitialMessages, InProgress, false),
|
||||
(ThreadSnapshot, Completed, false),
|
||||
] {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.replay_thread_turns(
|
||||
vec![app_server_turn(
|
||||
"previous-turn",
|
||||
status,
|
||||
/*duration_ms*/ None,
|
||||
/*error*/ None,
|
||||
)],
|
||||
replay_kind,
|
||||
);
|
||||
while rx.try_recv().is_ok() {}
|
||||
chat.set_mcp_startup_expected_servers(["alpha".into()]);
|
||||
notify_mcp_status_error(&mut chat, "alpha", "connection failed");
|
||||
let mut compact_cells = Vec::new();
|
||||
while let Ok(event) = rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
compact_cells.push(cell.as_any().is::<history_cell::StartupWarningsCell>());
|
||||
}
|
||||
}
|
||||
assert_eq!(compact_cells, vec![compact; 2]);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_startup_complete_does_not_clear_running_task() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
@@ -257,6 +309,7 @@ async fn pending_mcp_startup_does_not_unblock_external_review() {
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
|
||||
handle_entered_review_mode(&mut chat, "current changes");
|
||||
assert!(chat.bottom_pane.status_indicator_visible());
|
||||
chat.queue_user_message("queued follow-up".into());
|
||||
|
||||
assert!(chat.review.is_review_mode);
|
||||
@@ -270,6 +323,7 @@ async fn pending_mcp_startup_does_not_unblock_external_review() {
|
||||
|
||||
chat.finish_mcp_startup(Vec::new(), Vec::new());
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
assert!(chat.bottom_pane.status_indicator_visible());
|
||||
assert_eq!(chat.input_queue.queued_user_messages.len(), 1);
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
}
|
||||
@@ -450,7 +504,14 @@ async fn mcp_startup_failure_restores_running_status_header() {
|
||||
"MCP client for `alpha` failed to start: handshake failed",
|
||||
);
|
||||
notify_mcp_status(&mut chat, "beta", McpServerStartupState::Ready);
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
let warnings = super::helpers::drain_insert_history(&mut rx);
|
||||
insta::assert_snapshot!(
|
||||
"runtime_mcp_warning",
|
||||
warnings
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>()
|
||||
);
|
||||
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
assert!(chat.bottom_pane.status_indicator_visible());
|
||||
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/app_server.rs
|
||||
expression: "cells.iter().map(|lines| lines_to_single_string(lines)).collect::<String>()"
|
||||
---
|
||||
⚠ Invalid configuration; using defaults.: Check the project configuration.
|
||||
+7
@@ -0,0 +1,7 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/mcp_startup.rs
|
||||
expression: "warnings.iter().map(|lines| lines_to_single_string(lines)).collect::<String>()"
|
||||
---
|
||||
⚠ MCP client for `alpha` failed to start: handshake failed
|
||||
|
||||
⚠ MCP startup incomplete (failed: alpha)
|
||||
@@ -36,6 +36,12 @@ impl ChatWidget {
|
||||
|| self.review.is_review_mode
|
||||
|| self.mcp_startup_status.is_some(),
|
||||
);
|
||||
if self.mcp_startup_status.is_some()
|
||||
&& !self.turn_lifecycle.agent_turn_running
|
||||
&& !self.review.is_review_mode
|
||||
{
|
||||
self.bottom_pane.hide_status_indicator();
|
||||
}
|
||||
self.refresh_status_surfaces();
|
||||
}
|
||||
|
||||
@@ -83,6 +89,7 @@ impl ChatWidget {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
self.update_task_running_state();
|
||||
self.bottom_pane.ensure_status_indicator();
|
||||
self.bottom_pane.reset_status_timer(Duration::ZERO);
|
||||
self.status_state.retry_status_header = None;
|
||||
self.clear_active_hook_cell();
|
||||
|
||||
@@ -6,6 +6,8 @@ const FALLBACK_MODEL_METADATA_WARNING_SUFFIX: &str =
|
||||
|
||||
#[derive(Default)]
|
||||
pub(super) struct WarningDisplayState {
|
||||
/// Completed resume history does not end startup; active work does.
|
||||
pub(super) startup_complete: bool,
|
||||
fallback_model_metadata_slugs: HashSet<String>,
|
||||
}
|
||||
|
||||
|
||||
@@ -116,6 +116,7 @@ mod request_user_input;
|
||||
mod search;
|
||||
mod separators;
|
||||
mod session;
|
||||
mod startup_warnings;
|
||||
|
||||
pub(crate) use approvals::*;
|
||||
pub(crate) use base::*;
|
||||
@@ -132,6 +133,7 @@ pub(crate) use request_user_input::*;
|
||||
pub(crate) use search::*;
|
||||
pub(crate) use separators::*;
|
||||
pub(crate) use session::*;
|
||||
pub(crate) use startup_warnings::StartupWarningsCell;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
//! Compact startup diagnostics with full details retained in the transcript.
|
||||
//! Affected sources are unique; sign-in servers are a subset of MCP servers.
|
||||
|
||||
use super::*;
|
||||
use codex_app_server_protocol::McpServerStartupFailureReason;
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct StartupWarningsCell {
|
||||
pub(crate) messages: Vec<String>,
|
||||
pub(crate) other_sources: BTreeSet<String>,
|
||||
pub(crate) mcp_servers: BTreeSet<String>,
|
||||
pub(crate) sign_in_servers: BTreeSet<String>,
|
||||
pub(crate) pending_header: bool,
|
||||
pub(crate) transcript_hint: Option<String>,
|
||||
}
|
||||
|
||||
impl StartupWarningsCell {
|
||||
pub(crate) fn new(messages: Vec<String>) -> Self {
|
||||
Self {
|
||||
other_sources: messages.iter().cloned().collect(),
|
||||
messages,
|
||||
..Self::default()
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn mcp(
|
||||
messages: Vec<String>,
|
||||
servers: impl IntoIterator<Item = String>,
|
||||
failure_reason: Option<McpServerStartupFailureReason>,
|
||||
) -> Self {
|
||||
let mcp_servers: BTreeSet<_> = servers.into_iter().collect();
|
||||
let sign_in_servers = match failure_reason {
|
||||
Some(McpServerStartupFailureReason::ReauthenticationRequired) => mcp_servers.clone(),
|
||||
None => BTreeSet::new(),
|
||||
};
|
||||
Self {
|
||||
messages,
|
||||
mcp_servers,
|
||||
sign_in_servers,
|
||||
..Self::default()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl HistoryCell for StartupWarningsCell {
|
||||
#[allow(clippy::disallowed_methods)]
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
if self.pending_header || self.messages.is_empty() || width == 0 {
|
||||
return Vec::new();
|
||||
}
|
||||
let mcp_count = self.mcp_servers.len();
|
||||
let count = mcp_count + self.other_sources.len();
|
||||
let sign_in_count = self.sign_in_servers.len();
|
||||
let plural = if count == 1 { "" } else { "s" };
|
||||
let source = if mcp_count == count { "MCP " } else { "" };
|
||||
let mut summary = format!("⚠ {count} {source}startup issue{plural}");
|
||||
let mut breakdown = Vec::new();
|
||||
if mcp_count > 0 && mcp_count < count {
|
||||
breakdown.push(format!("{mcp_count} MCP"));
|
||||
}
|
||||
if sign_in_count > 0 {
|
||||
let verb = if sign_in_count == 1 { "needs" } else { "need" };
|
||||
breakdown.push(format!("{sign_in_count} {verb} sign-in"));
|
||||
}
|
||||
if !breakdown.is_empty() {
|
||||
summary.push_str(&format!(" ({})", breakdown.join("; ")));
|
||||
}
|
||||
if let Some(hint) = &self.transcript_hint {
|
||||
summary.push_str(&format!(" · {hint} for details"));
|
||||
}
|
||||
vec![
|
||||
crate::line_truncation::truncate_line_with_ellipsis_if_overflow(
|
||||
Line::from(summary.yellow().dim()),
|
||||
width as usize,
|
||||
),
|
||||
]
|
||||
}
|
||||
|
||||
fn transcript_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
self.messages
|
||||
.iter()
|
||||
.flat_map(|message| new_warning_event(message.clone()).transcript_lines(width))
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn raw_lines(&self) -> Vec<Line<'static>> {
|
||||
self.messages
|
||||
.iter()
|
||||
.flat_map(|message| raw_lines_from_source(message))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user