mirror of
https://github.com/openai/codex.git
synced 2026-11-01 19:33:35 +00:00
codex: address PR review feedback (#16041)
This commit is contained in:
@@ -805,6 +805,8 @@ pub(crate) struct ChatWidget {
|
||||
mcp_startup_allow_terminal_only_next_round: bool,
|
||||
/// Buffers post-settle MCP startup updates until they cover a full fresh round.
|
||||
mcp_startup_pending_next_round: HashMap<String, McpStartupStatus>,
|
||||
/// Tracks whether the buffered next round has seen any `Starting` update yet.
|
||||
mcp_startup_pending_next_round_saw_starting: bool,
|
||||
connectors_cache: ConnectorsCacheState,
|
||||
connectors_partial_snapshot: Option<ConnectorsSnapshot>,
|
||||
connectors_prefetch_in_flight: bool,
|
||||
@@ -2799,14 +2801,13 @@ impl ChatWidget {
|
||||
let mut activated_pending_round = false;
|
||||
let startup_status = if self.mcp_startup_ignore_updates_until_next_start {
|
||||
if matches!(status, McpStartupStatus::Starting)
|
||||
&& !self
|
||||
.mcp_startup_pending_next_round
|
||||
.values()
|
||||
.any(|state| matches!(state, McpStartupStatus::Starting))
|
||||
&& !self.mcp_startup_pending_next_round_saw_starting
|
||||
{
|
||||
self.mcp_startup_pending_next_round.clear();
|
||||
self.mcp_startup_allow_terminal_only_next_round = false;
|
||||
}
|
||||
self.mcp_startup_pending_next_round_saw_starting |=
|
||||
matches!(status, McpStartupStatus::Starting);
|
||||
self.mcp_startup_pending_next_round.insert(server, status);
|
||||
let Some(expected_servers) = &self.mcp_startup_expected_servers else {
|
||||
return;
|
||||
@@ -2826,6 +2827,7 @@ impl ChatWidget {
|
||||
}
|
||||
self.mcp_startup_ignore_updates_until_next_start = false;
|
||||
self.mcp_startup_allow_terminal_only_next_round = false;
|
||||
self.mcp_startup_pending_next_round_saw_starting = false;
|
||||
activated_pending_round = true;
|
||||
std::mem::take(&mut self.mcp_startup_pending_next_round)
|
||||
} else {
|
||||
@@ -2940,6 +2942,7 @@ impl ChatWidget {
|
||||
self.mcp_startup_ignore_updates_until_next_start = true;
|
||||
self.mcp_startup_allow_terminal_only_next_round = false;
|
||||
self.mcp_startup_pending_next_round.clear();
|
||||
self.mcp_startup_pending_next_round_saw_starting = false;
|
||||
self.update_task_running_state();
|
||||
self.maybe_send_next_queued_input();
|
||||
self.request_redraw();
|
||||
@@ -2948,6 +2951,7 @@ impl ChatWidget {
|
||||
pub(crate) fn finish_mcp_startup_after_lag(&mut self) {
|
||||
if self.mcp_startup_ignore_updates_until_next_start {
|
||||
self.mcp_startup_pending_next_round.clear();
|
||||
self.mcp_startup_pending_next_round_saw_starting = false;
|
||||
self.mcp_startup_allow_terminal_only_next_round = true;
|
||||
}
|
||||
|
||||
@@ -4717,6 +4721,7 @@ impl ChatWidget {
|
||||
mcp_startup_ignore_updates_until_next_start: false,
|
||||
mcp_startup_allow_terminal_only_next_round: false,
|
||||
mcp_startup_pending_next_round: HashMap::new(),
|
||||
mcp_startup_pending_next_round_saw_starting: false,
|
||||
connectors_cache: ConnectorsCacheState::default(),
|
||||
connectors_partial_snapshot: None,
|
||||
connectors_prefetch_in_flight: false,
|
||||
|
||||
@@ -2077,6 +2077,7 @@ async fn make_chatwidget_manual(
|
||||
mcp_startup_ignore_updates_until_next_start: false,
|
||||
mcp_startup_allow_terminal_only_next_round: false,
|
||||
mcp_startup_pending_next_round: HashMap::new(),
|
||||
mcp_startup_pending_next_round_saw_starting: false,
|
||||
connectors_cache: ConnectorsCacheState::default(),
|
||||
connectors_partial_snapshot: None,
|
||||
plugin_install_apps_needing_auth: Vec::new(),
|
||||
@@ -11888,6 +11889,67 @@ async fn app_server_mcp_startup_next_round_discards_stale_terminal_updates() {
|
||||
assert!(!chat.bottom_pane.is_task_running());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_next_round_keeps_terminal_statuses_after_starting() {
|
||||
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.finish_mcp_startup_after_lag();
|
||||
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
name: "alpha".to_string(),
|
||||
status: McpServerStartupState::Starting,
|
||||
error: None,
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
|
||||
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()),
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
|
||||
let failure_text = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert!(failure_text.contains("MCP client for `alpha` failed to start: handshake failed"));
|
||||
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
name: "beta".to_string(),
|
||||
status: McpServerStartupState::Starting,
|
||||
error: None,
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
name: "beta".to_string(),
|
||||
status: McpServerStartupState::Ready,
|
||||
error: None,
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
|
||||
let summary_text = drain_insert_history(&mut rx)
|
||||
.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());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_server_mcp_startup_next_round_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