openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-01 19:33:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

codex: address PR review feedback (#16041)

Browse Source
This commit is contained in:
Eric Traut
2026-03-28 01:31:00 -06:00
parent 52494485bc
commit 0915e7517d
3 changed files with 106 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui_app_server/src/app/app_server_adapter.rs
+16 -16
View File
@@ -21,7 +21,6 @@ use crate::exec_command::split_command_string;
use codex_app_server_client::AppServerEvent;
use codex_app_server_protocol::AuthMode;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::McpServerStartupState;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequest;
#[cfg(test)]
@@ -109,6 +108,19 @@ use codex_protocol::protocol::TurnStartedEvent;
use std::time::Duration;
impl App {
fn refresh_mcp_startup_expected_servers_from_config(&mut self) {
let enabled_config_mcp_servers: Vec<String> = self
.chat_widget
.config_ref()
.mcp_servers
.get()
.iter()
.filter_map(|(name, server)| server.enabled.then_some(name.clone()))
.collect();
self.chat_widget
.set_mcp_startup_expected_servers(enabled_config_mcp_servers);
}
pub(super) async fn handle_app_server_event(
&mut self,
app_server_client: &AppServerSession,
@@ -120,6 +132,7 @@ impl App {
skipped,
"app-server event consumer lagged; dropping ignored events"
);
self.refresh_mcp_startup_expected_servers_from_config();
self.chat_widget.finish_mcp_startup_after_lag();
}
AppServerEvent::ServerNotification(notification) => {
@@ -148,21 +161,8 @@ impl App {
self.pending_app_server_requests
.resolve_notification(&notification.request_id);
}
ServerNotification::McpServerStatusUpdated(notification) => {
if notification.status == McpServerStartupState::Starting
&& self.chat_widget.needs_mcp_startup_expected_servers()
{
let enabled_config_mcp_servers: Vec<String> = self
.chat_widget
.config_ref()
.mcp_servers
.get()
.iter()
.filter_map(|(name, server)| server.enabled.then_some(name.clone()))
.collect();
self.chat_widget
.set_mcp_startup_expected_servers(enabled_config_mcp_servers);
}
ServerNotification::McpServerStatusUpdated(_) => {
self.refresh_mcp_startup_expected_servers_from_config();
}
ServerNotification::AccountRateLimitsUpdated(notification) => {
self.chat_widget.on_rate_limit_snapshot(Some(
codex-rs/tui_app_server/src/chatwidget.rs
+36 -13
View File
@@ -799,8 +799,10 @@ pub(crate) struct ChatWidget {
mcp_startup_status: Option<HashMap<String, McpStartupStatus>>,
/// Expected MCP servers for the current startup round, seeded from enabled local config.
mcp_startup_expected_servers: Option<HashSet<String>>,
/// After startup settles, ignore late terminal updates until a new startup round begins.
/// After startup settles, ignore stale updates until enough notifications confirm a new round.
mcp_startup_ignore_updates_until_next_start: bool,
/// Buffers post-settle MCP startup updates until they cover a full fresh round.
mcp_startup_pending_next_round: HashMap<String, McpStartupStatus>,
connectors_cache: ConnectorsCacheState,
connectors_partial_snapshot: Option<ConnectorsSnapshot>,
connectors_prefetch_in_flight: bool,
@@ -2792,17 +2794,41 @@ impl ChatWidget {
status: McpStartupStatus,
complete_when_settled: bool,
) {
if self.mcp_startup_ignore_updates_until_next_start {
if !matches!(status, McpStartupStatus::Starting) {
let mut activated_pending_round = false;
let startup_status = if self.mcp_startup_ignore_updates_until_next_start {
self.mcp_startup_pending_next_round.insert(server, status);
let Some(expected_servers) = &self.mcp_startup_expected_servers else {
return;
};
let saw_full_round = !expected_servers.is_empty()
&& expected_servers
.iter()
.all(|name| self.mcp_startup_pending_next_round.contains_key(name));
let saw_starting = self
.mcp_startup_pending_next_round
.values()
.any(|state| matches!(state, McpStartupStatus::Starting));
if !(saw_full_round && saw_starting) {
return;
}
self.mcp_startup_ignore_updates_until_next_start = false;
activated_pending_round = true;
std::mem::take(&mut self.mcp_startup_pending_next_round)
} else {
let mut startup_status = self.mcp_startup_status.take().unwrap_or_default();
if let McpStartupStatus::Failed { error } = &status {
self.on_warning(error);
}
startup_status.insert(server, status);
startup_status
};
if activated_pending_round {
for state in startup_status.values() {
if let McpStartupStatus::Failed { error } = state {
self.on_warning(error);
}
}
}
let mut startup_status = self.mcp_startup_status.take().unwrap_or_default();
if let McpStartupStatus::Failed { error } = &status {
self.on_warning(error);
}
startup_status.insert(server, status);
self.mcp_startup_status = Some(startup_status);
self.update_task_running_state();
if complete_when_settled
@@ -2869,10 +2895,6 @@ impl ChatWidget {
self.request_redraw();
}
pub(crate) fn needs_mcp_startup_expected_servers(&self) -> bool {
self.mcp_startup_expected_servers.is_none()
}
pub(crate) fn set_mcp_startup_expected_servers<I>(&mut self, server_names: I)
where
I: IntoIterator<Item = String>,
@@ -2901,8 +2923,8 @@ impl ChatWidget {
}
self.mcp_startup_status = None;
self.mcp_startup_expected_servers = None;
self.mcp_startup_ignore_updates_until_next_start = true;
self.mcp_startup_pending_next_round.clear();
self.update_task_running_state();
self.maybe_send_next_queued_input();
self.request_redraw();
@@ -4673,6 +4695,7 @@ impl ChatWidget {
pending_turn_copyable_output: None,
mcp_startup_expected_servers: None,
mcp_startup_ignore_updates_until_next_start: false,
mcp_startup_pending_next_round: HashMap::new(),
connectors_cache: ConnectorsCacheState::default(),
connectors_partial_snapshot: None,
connectors_prefetch_in_flight: false,
codex-rs/tui_app_server/src/chatwidget/tests.rs
+54
View File
@@ -2075,6 +2075,7 @@ async fn make_chatwidget_manual(
mcp_startup_status: None,
mcp_startup_expected_servers: None,
mcp_startup_ignore_updates_until_next_start: false,
mcp_startup_pending_next_round: HashMap::new(),
connectors_cache: ConnectorsCacheState::default(),
connectors_partial_snapshot: None,
plugin_install_apps_needing_auth: Vec::new(),
@@ -11733,6 +11734,18 @@ async fn app_server_mcp_startup_lag_settles_startup_and_ignores_late_updates() {
assert!(summary_text.contains("MCP startup incomplete (failed: alpha)"));
assert!(!chat.bottom_pane.is_task_running());
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(),
@@ -11746,6 +11759,47 @@ 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_after_lag_can_settle_without_starting_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.finish_mcp_startup_after_lag();
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"));
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 background_event_updates_status_header() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;