From 0ffcefaf3ddb3a61d8683ca0703f7d8b39ad6c1e Mon Sep 17 00:00:00 2001 From: jif Date: Wed, 10 Jun 2026 11:45:49 +0200 Subject: [PATCH] feat: keep child MCP warnings out of parent transcript (#27174) ## Why MCP startup status notifications are thread-owned, but `ChatWidget` trusted upstream routing. If routing state delivered a tagged child notification to the active parent widget, the child MCP failure could still mutate the parent's startup state and transcript. Rejecting it only inside the MCP handler was also too late because shared notification handling could already restore and consume the parent's retry status. ## What changed - Validate a tagged MCP status notification against the visible `ChatWidget` thread before shared notification handling mutates any parent state. - Cover child `Starting` and `Failed` notifications delivered to a retrying parent widget, asserting that they preserve its visible retry error and saved status header while producing no history or MCP status mutation. ## User impact Subagent MCP startup failures remain scoped to the child transcript instead of appearing as duplicate warnings in the parent transcript. ## Testing - `just test -p codex-tui mcp_startup_ignores_status_for_other_thread` - `just test -p codex-tui primary_thread_ignores_child_mcp_startup_notifications` - `just fmt` --- codex-rs/tui/src/chatwidget/protocol.rs | 9 ++++ .../tui/src/chatwidget/tests/mcp_startup.rs | 41 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/codex-rs/tui/src/chatwidget/protocol.rs b/codex-rs/tui/src/chatwidget/protocol.rs index 9d2920807..3682f245f 100644 --- a/codex-rs/tui/src/chatwidget/protocol.rs +++ b/codex-rs/tui/src/chatwidget/protocol.rs @@ -6,6 +6,15 @@ impl ChatWidget { notification: ServerNotification, replay_kind: Option, ) { + // Reject misrouted child updates before shared notification handling mutates parent state. + if let ServerNotification::McpServerStatusUpdated(notification) = ¬ification + && let (Some(notification_thread_id), Some(thread_id)) = + (notification.thread_id.as_deref(), self.thread_id()) + && notification_thread_id != thread_id.to_string() + { + return; + } + let from_replay = replay_kind.is_some(); let is_resume_initial_replay = matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages)); diff --git a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs index 8b0d5ca47..60efbe135 100644 --- a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs +++ b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs @@ -25,6 +25,47 @@ fn notify_mcp_status_error(chat: &mut ChatWidget, name: &str, error: &str) { ); } +#[tokio::test] +async fn mcp_startup_ignores_status_for_other_thread() { + 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(["sentry".to_string()]); + let parent_thread_id = ThreadId::new(); + let child_thread_id = ThreadId::new(); + chat.thread_id = Some(parent_thread_id); + chat.on_stream_error( + "Connection interrupted, retrying".to_string(), + /*additional_details*/ None, + ); + let status_before = chat.status_state.current_status.clone(); + let retry_status_header_before = chat.status_state.retry_status_header.clone(); + + for status in [ + McpServerStartupState::Starting, + McpServerStartupState::Failed, + ] { + chat.handle_server_notification( + ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification { + thread_id: Some(child_thread_id.to_string()), + name: "sentry".to_string(), + status, + error: matches!(status, McpServerStartupState::Failed) + .then(|| "sentry is not logged in".to_string()), + }), + /*replay_kind*/ None, + ); + } + + assert!(drain_insert_history(&mut rx).is_empty()); + assert!(!chat.bottom_pane.is_task_running()); + assert!(chat.mcp_startup_status.is_none()); + assert_eq!(chat.status_state.current_status, status_before); + assert_eq!( + chat.status_state.retry_status_header, + retry_status_header_before + ); +} + #[tokio::test] async fn mcp_startup_dedupes_same_round_duplicate_failure_warning() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;