mirror of
https://github.com/openai/codex.git
synced 2026-05-03 19:06:58 +00:00
Surface MCP startup failures in tui_app_server
This commit is contained in:
@@ -76,6 +76,8 @@ use codex_app_server_protocol::ErrorNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
use codex_app_server_protocol::McpServerStartupState;
|
||||
use codex_app_server_protocol::McpServerStatusUpdatedNotification;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::ThreadItem;
|
||||
@@ -2780,15 +2782,41 @@ impl ChatWidget {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn on_mcp_startup_update(&mut self, ev: McpStartupUpdateEvent) {
|
||||
let mut status = self.mcp_startup_status.take().unwrap_or_default();
|
||||
if let McpStartupStatus::Failed { error } = &ev.status {
|
||||
fn update_mcp_startup_status(
|
||||
&mut self,
|
||||
server: String,
|
||||
status: McpStartupStatus,
|
||||
complete_when_settled: bool,
|
||||
) {
|
||||
let mut startup_status = self.mcp_startup_status.take().unwrap_or_default();
|
||||
if let McpStartupStatus::Failed { error } = &status {
|
||||
self.on_warning(error);
|
||||
}
|
||||
status.insert(ev.server, ev.status);
|
||||
self.mcp_startup_status = Some(status);
|
||||
startup_status.insert(server, status);
|
||||
self.mcp_startup_status = Some(startup_status);
|
||||
self.update_task_running_state();
|
||||
if complete_when_settled
|
||||
&& let Some(current) = &self.mcp_startup_status
|
||||
&& !current.is_empty()
|
||||
&& current
|
||||
.values()
|
||||
.all(|state| !matches!(state, McpStartupStatus::Starting))
|
||||
{
|
||||
let mut failed = Vec::new();
|
||||
let mut cancelled = Vec::new();
|
||||
for (name, state) in current {
|
||||
match state {
|
||||
McpStartupStatus::Ready => {}
|
||||
McpStartupStatus::Failed { .. } => failed.push(name.clone()),
|
||||
McpStartupStatus::Cancelled => cancelled.push(name.clone()),
|
||||
McpStartupStatus::Starting => {}
|
||||
}
|
||||
}
|
||||
failed.sort();
|
||||
cancelled.sort();
|
||||
self.finish_mcp_startup(failed, cancelled);
|
||||
return;
|
||||
}
|
||||
if let Some(current) = &self.mcp_startup_status {
|
||||
let total = current.len();
|
||||
let mut starting: Vec<_> = current
|
||||
@@ -2828,18 +2856,21 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn on_mcp_startup_complete(&mut self, ev: McpStartupCompleteEvent) {
|
||||
let mut parts = Vec::new();
|
||||
if !ev.failed.is_empty() {
|
||||
let failed_servers: Vec<_> = ev.failed.iter().map(|f| f.server.clone()).collect();
|
||||
parts.push(format!("failed: {}", failed_servers.join(", ")));
|
||||
}
|
||||
if !ev.cancelled.is_empty() {
|
||||
fn on_mcp_startup_update(&mut self, ev: McpStartupUpdateEvent) {
|
||||
self.update_mcp_startup_status(ev.server, ev.status, /*complete_when_settled*/ false);
|
||||
}
|
||||
|
||||
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: {}",
|
||||
ev.cancelled.join(", ")
|
||||
cancelled.join(", ")
|
||||
));
|
||||
}
|
||||
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("; ")));
|
||||
}
|
||||
@@ -2850,6 +2881,30 @@ impl ChatWidget {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn on_mcp_startup_complete(&mut self, ev: McpStartupCompleteEvent) {
|
||||
let failed = ev.failed.into_iter().map(|f| f.server).collect();
|
||||
self.finish_mcp_startup(failed, ev.cancelled);
|
||||
}
|
||||
|
||||
fn on_mcp_server_status_updated(&mut self, notification: McpServerStatusUpdatedNotification) {
|
||||
let status = match notification.status {
|
||||
McpServerStartupState::Starting => McpStartupStatus::Starting,
|
||||
McpServerStartupState::Ready => McpStartupStatus::Ready,
|
||||
McpServerStartupState::Failed => McpStartupStatus::Failed {
|
||||
error: notification.error.unwrap_or_else(|| {
|
||||
format!("MCP client for `{}` failed to start", notification.name)
|
||||
}),
|
||||
},
|
||||
McpServerStartupState::Cancelled => McpStartupStatus::Cancelled,
|
||||
};
|
||||
self.update_mcp_startup_status(
|
||||
notification.name,
|
||||
status,
|
||||
/*complete_when_settled*/ true,
|
||||
);
|
||||
}
|
||||
|
||||
/// Handle a turn aborted due to user interrupt (Esc).
|
||||
/// When there are queued user messages, restore them into the composer
|
||||
/// separated by newlines rather than auto‑submitting the next one.
|
||||
@@ -6318,6 +6373,9 @@ impl ChatWidget {
|
||||
.map(|details| format!("{}: {details}", notification.summary))
|
||||
.unwrap_or(notification.summary),
|
||||
),
|
||||
ServerNotification::McpServerStatusUpdated(notification) => {
|
||||
self.on_mcp_server_status_updated(notification)
|
||||
}
|
||||
ServerNotification::ItemGuardianApprovalReviewStarted(notification) => {
|
||||
self.on_guardian_review_notification(
|
||||
notification.target_item_id,
|
||||
@@ -6401,7 +6459,6 @@ impl ChatWidget {
|
||||
| ServerNotification::RawResponseItemCompleted(_)
|
||||
| ServerNotification::CommandExecOutputDelta(_)
|
||||
| ServerNotification::McpToolCallProgress(_)
|
||||
| ServerNotification::McpServerStatusUpdated(_)
|
||||
| ServerNotification::McpServerOauthLoginCompleted(_)
|
||||
| ServerNotification::AppListUpdated(_)
|
||||
| ServerNotification::FsChanged(_)
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
---
|
||||
source: tui_app_server/src/chatwidget/tests.rs
|
||||
assertion_line: 11571
|
||||
expression: term.backend().vt100().screen().contents()
|
||||
---
|
||||
|
||||
|
||||
|
||||
⚠ MCP client for `alpha` failed to start: handshake failed
|
||||
⚠ MCP startup incomplete (failed: alpha)
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
? for shortcuts 100% context left
|
||||
@@ -50,6 +50,8 @@ use codex_app_server_protocol::ItemGuardianApprovalReviewCompletedNotification;
|
||||
use codex_app_server_protocol::ItemGuardianApprovalReviewStartedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
use codex_app_server_protocol::MarketplaceInterface;
|
||||
use codex_app_server_protocol::McpServerStartupState;
|
||||
use codex_app_server_protocol::McpServerStatusUpdatedNotification;
|
||||
use codex_app_server_protocol::PatchApplyStatus as AppServerPatchApplyStatus;
|
||||
use codex_app_server_protocol::PatchChangeKind;
|
||||
use codex_app_server_protocol::PluginAuthPolicy;
|
||||
@@ -11518,6 +11520,92 @@ async fn mcp_startup_complete_does_not_clear_running_task() {
|
||||
assert!(chat.bottom_pane.status_indicator_visible());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_failure_renders_warning_history() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
|
||||
for notification in [
|
||||
McpServerStatusUpdatedNotification {
|
||||
name: "alpha".to_string(),
|
||||
status: McpServerStartupState::Starting,
|
||||
error: None,
|
||||
},
|
||||
McpServerStatusUpdatedNotification {
|
||||
name: "beta".to_string(),
|
||||
status: McpServerStartupState::Starting,
|
||||
error: None,
|
||||
},
|
||||
] {
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(notification),
|
||||
None,
|
||||
);
|
||||
}
|
||||
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
name: "alpha".to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some("MCP client for `alpha` failed to start: handshake failed".to_string()),
|
||||
}),
|
||||
None,
|
||||
);
|
||||
|
||||
let failure_cells = drain_insert_history(&mut rx);
|
||||
let failure_text = failure_cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert!(failure_text.contains("MCP client for `alpha` failed to start: handshake failed"));
|
||||
assert!(!failure_text.contains("MCP startup incomplete"));
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
name: "beta".to_string(),
|
||||
status: McpServerStartupState::Ready,
|
||||
error: None,
|
||||
}),
|
||||
None,
|
||||
);
|
||||
|
||||
let summary_cells = drain_insert_history(&mut rx);
|
||||
let summary_text = summary_cells
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_eq!(summary_text, "⚠ MCP startup incomplete (failed: alpha)\n");
|
||||
assert!(!chat.bottom_pane.is_task_running());
|
||||
|
||||
let width: u16 = 120;
|
||||
let ui_height: u16 = chat.desired_height(width);
|
||||
let vt_height: u16 = 10;
|
||||
let viewport = Rect::new(0, vt_height - ui_height - 1, width, ui_height);
|
||||
|
||||
let backend = VT100Backend::new(width, vt_height);
|
||||
let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");
|
||||
term.set_viewport_area(viewport);
|
||||
|
||||
for lines in failure_cells.into_iter().chain(summary_cells) {
|
||||
crate::insert_history::insert_history_lines(&mut term, lines)
|
||||
.expect("Failed to insert history lines in test");
|
||||
}
|
||||
|
||||
term.draw(|f| {
|
||||
chat.render(f.area(), f.buffer_mut());
|
||||
})
|
||||
.expect("draw MCP startup warning history");
|
||||
|
||||
assert_snapshot!(
|
||||
"app_server_mcp_startup_failure_renders_warning_history",
|
||||
term.backend().vt100().screen().contents()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn background_event_updates_status_header() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
Reference in New Issue
Block a user