mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
Handle late MCP startup results after lag timeout (#36895)
## Why MCP startup status delivery can lag. Treating a server that is still starting or has not reported yet as cancelled produces a misleading interruption warning and can hide its eventual terminal result. ## What changed - Only report an interrupted startup when a server explicitly enters the cancelled state. - Allow a complete set of terminal updates received after the lag timeout to be promoted and reported, including failures and cancellations. ## Testing Added TUI tests covering unresolved servers at the lag timeout and late failure and cancellation updates. GitOrigin-RevId: 555f3d13ac87bda6e9b0bf72335a1c04851ac25a
This commit is contained in:
@@ -236,11 +236,9 @@ impl ChatWidget {
|
||||
|
||||
for name in server_names {
|
||||
match current.get(&name) {
|
||||
Some(McpStartupStatus::Ready) => {}
|
||||
Some(McpStartupStatus::Failed { .. }) => failed.push(name),
|
||||
Some(McpStartupStatus::Cancelled | McpStartupStatus::Starting) | None => {
|
||||
cancelled.push(name);
|
||||
}
|
||||
Some(McpStartupStatus::Cancelled) => cancelled.push(name),
|
||||
Some(McpStartupStatus::Ready | McpStartupStatus::Starting) | None => {}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -249,6 +247,7 @@ impl ChatWidget {
|
||||
cancelled.sort();
|
||||
cancelled.dedup();
|
||||
self.finish_mcp_startup(failed, cancelled);
|
||||
self.mcp_startup_allow_terminal_only_next_round = true;
|
||||
}
|
||||
|
||||
pub(super) fn status_header_is_mcp_startup_owned(&self) -> bool {
|
||||
|
||||
@@ -488,7 +488,11 @@ async fn mcp_startup_complete_preserves_review_status() {
|
||||
async fn app_server_mcp_startup_lag_settles_startup_and_ignores_late_updates() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
chat.set_mcp_startup_expected_servers(["alpha".to_string(), "beta".to_string()]);
|
||||
chat.set_mcp_startup_expected_servers([
|
||||
"alpha".to_string(),
|
||||
"beta".to_string(),
|
||||
"gamma".to_string(),
|
||||
]);
|
||||
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting);
|
||||
notify_mcp_status_error(
|
||||
@@ -507,9 +511,9 @@ async fn app_server_mcp_startup_lag_settles_startup_and_ignores_late_updates() {
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert!(summary_text.contains("MCP startup interrupted"));
|
||||
assert!(summary_text.contains("beta"));
|
||||
assert!(summary_text.contains("MCP startup incomplete (failed: alpha)"));
|
||||
insta::assert_snapshot!(summary_text, @r"
|
||||
⚠ MCP startup incomplete (failed: alpha)
|
||||
");
|
||||
assert!(!chat.bottom_pane.is_task_running());
|
||||
|
||||
notify_mcp_status(&mut chat, "beta", McpServerStartupState::Starting);
|
||||
@@ -523,6 +527,83 @@ async fn app_server_mcp_startup_lag_settles_startup_and_ignores_late_updates() {
|
||||
assert!(!chat.bottom_pane.is_task_running());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_lag_reports_only_explicit_cancellations() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
chat.set_mcp_startup_expected_servers([
|
||||
"alpha".to_string(),
|
||||
"beta".to_string(),
|
||||
"gamma".to_string(),
|
||||
]);
|
||||
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting);
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Cancelled);
|
||||
notify_mcp_status(&mut chat, "beta", McpServerStartupState::Starting);
|
||||
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
|
||||
chat.finish_mcp_startup_after_lag();
|
||||
|
||||
let summary_text = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_eq!(
|
||||
summary_text,
|
||||
"⚠ MCP startup interrupted. The following servers were not initialized: alpha\n"
|
||||
);
|
||||
assert!(!chat.bottom_pane.is_task_running());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_reports_failure_after_lag() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
chat.set_mcp_startup_expected_servers(["alpha".to_string()]);
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting);
|
||||
|
||||
chat.finish_mcp_startup_after_lag();
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
|
||||
notify_mcp_status_error(
|
||||
&mut chat,
|
||||
"alpha",
|
||||
"MCP client for `alpha` failed to start: handshake failed",
|
||||
);
|
||||
|
||||
let warning_text = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert!(warning_text.contains("MCP client for `alpha` failed to start: handshake failed"));
|
||||
assert!(warning_text.contains("MCP startup incomplete (failed: alpha)"));
|
||||
assert!(!warning_text.contains("MCP startup interrupted"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_reports_cancellation_after_lag() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
chat.set_mcp_startup_expected_servers(["alpha".to_string()]);
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting);
|
||||
|
||||
chat.finish_mcp_startup_after_lag();
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Cancelled);
|
||||
|
||||
let warning_text = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_eq!(
|
||||
warning_text,
|
||||
"⚠ MCP startup interrupted. The following servers were not initialized: alpha\n"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_after_lag_can_settle_without_starting_updates() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
Reference in New Issue
Block a user