Skip to content
Prev Previous commit
Next Next commit
fix(tui): preserve parent MCP startup across review
  • Loading branch information
charliemarsh-oai committed Jul 5, 2026
commit 4b6069df0bd7b8d272445bdf7cc8f9509922b435
26 changes: 18 additions & 8 deletions codex-rs/tui/src/chatwidget.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,9 @@ pub(crate) struct ChatWidget {
/// as "running" while this is populated, even if no agent turn is currently
/// executing.
mcp_startup_status: Option<HashMap<String, McpStartupStatus>>,
/// Monotonically identifies startup rounds so review cleanup only clears a round it observed
/// starting after review entry.
mcp_startup_generation: u64,
/// Expected MCP servers for the current startup round, seeded from enabled local config.
mcp_startup_expected_servers: Option<HashSet<String>>,
/// After startup settles, ignore stale updates until enough notifications confirm a new round.
Expand Down Expand Up @@ -1243,8 +1246,11 @@ impl ChatWidget {
if self.review.pre_review_token_info.is_none() {
self.review.pre_review_token_info = Some(self.token_info.clone());
}
if !from_replay && !self.bottom_pane.is_task_running() {
self.bottom_pane.set_task_running(/*running*/ true);
if !from_replay {
self.review.mcp_startup_generation_at_entry = Some(self.mcp_startup_generation);
if !self.bottom_pane.is_task_running() {
self.bottom_pane.set_task_running(/*running*/ true);
}
}
self.review.is_review_mode = true;
let banner = format!(">> Code review started: {hint} <<");
Expand All @@ -1256,12 +1262,16 @@ impl ChatWidget {
self.flush_answer_stream_with_separator();
self.flush_interrupt_queue();
self.flush_active_cell();
// A live review cannot start while another MCP startup round owns the task indicator, so
// any round opened during review belongs to its child session. Interrupting the review can
// close the child event stream before terminal startup updates arrive; clear that round so
// it cannot leave the TUI permanently busy. Replayed review items must not clear newer live
// startup state.
if !from_replay && self.review.is_review_mode && self.mcp_startup_status.is_some() {
let mcp_startup_generation_at_entry = self.review.mcp_startup_generation_at_entry.take();
// Interrupting a live review can close its child event stream before terminal startup
// updates arrive. Clear only a startup round first observed after entering review; an
// inherited generation belongs to the parent runtime and must finish normally. Replayed
// review items must not clear newer live startup state.
if !from_replay
&& self.review.is_review_mode
&& self.mcp_startup_status.is_some()
&& mcp_startup_generation_at_entry != Some(self.mcp_startup_generation)
{
self.clear_mcp_startup_state();
}
self.review.is_review_mode = false;
Expand Down
1 change: 1 addition & 0 deletions codex-rs/tui/src/chatwidget/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ impl ChatWidget {
task_complete_pending: false,
unified_exec_processes: Vec::new(),
mcp_startup_status: None,
mcp_startup_generation: 0,
mcp_startup_expected_servers: None,
mcp_startup_ignore_updates_until_next_start: false,
mcp_startup_allow_terminal_only_next_round: false,
Expand Down
4 changes: 4 additions & 0 deletions codex-rs/tui/src/chatwidget/mcp_startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl ChatWidget {
status: McpStartupStatus,
complete_when_settled: bool,
) {
let had_active_round = self.mcp_startup_status.is_some();
let mut activated_pending_round = false;
let startup_status = if self.mcp_startup_ignore_updates_until_next_start {
// Ignore-mode buffers the next plausible round so stale post-finish
Expand Down Expand Up @@ -102,6 +103,9 @@ impl ChatWidget {
}
}
}
if !had_active_round {
self.mcp_startup_generation = self.mcp_startup_generation.wrapping_add(1);
}
self.mcp_startup_status = Some(startup_status);
self.update_task_running_state();

Expand Down
2 changes: 2 additions & 0 deletions codex-rs/tui/src/chatwidget/review.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ pub(super) struct ReviewState {
pub(super) recent_auto_review_denials: RecentAutoReviewDenials,
/// Simple review mode flag; used to adjust layout and banners.
pub(super) is_review_mode: bool,
/// MCP startup generation active when the current live review began.
pub(super) mcp_startup_generation_at_entry: Option<u64>,
/// Snapshot of token usage to restore after review mode exits.
pub(super) pre_review_token_info: Option<Option<TokenUsageInfo>>,
}
24 changes: 24 additions & 0 deletions codex-rs/tui/src/chatwidget/tests/mcp_startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,30 @@ async fn interrupted_review_restores_queued_input_without_submitting() {
assert_no_submit_op(&mut op_rx);
}

#[tokio::test]
async fn review_exit_preserves_parent_mcp_refresh() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.set_mcp_startup_expected_servers(["pending".to_string()]);
notify_mcp_status(&mut chat, "pending", McpServerStartupState::Starting);

handle_entered_review_mode(&mut chat, "current changes");
handle_exited_review_mode(&mut chat);

assert!(chat.mcp_startup_status.is_some());
assert!(chat.bottom_pane.is_task_running());

notify_mcp_status_error(&mut chat, "pending", "parent MCP refresh failed");

assert!(chat.mcp_startup_status.is_none());
assert!(!chat.bottom_pane.is_task_running());
let history = drain_insert_history(&mut rx)
.iter()
.map(|lines| lines_to_single_string(lines))
.collect::<String>();
assert!(history.contains("parent MCP refresh failed"));
assert!(history.contains("MCP startup incomplete (failed: pending)"));
}

#[tokio::test]
async fn replayed_review_exit_preserves_live_mcp_startup() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
Expand Down