diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 9b738a619..4cd3d62cb 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -247,6 +247,26 @@ fn collab_receiver_thread_ids(notification: &ServerNotification) -> Option<&[Str } } +fn collab_receiver_is_not_found( + notification: &ServerNotification, + receiver_thread_id: &str, +) -> bool { + match notification { + ServerNotification::ItemCompleted(notification) => match ¬ification.item { + ThreadItem::CollabAgentToolCall { agents_states, .. } => { + agents_states.get(receiver_thread_id).is_some_and(|state| { + matches!( + &state.status, + codex_app_server_protocol::CollabAgentStatus::NotFound + ) + }) + } + _ => false, + }, + _ => false, + } +} + fn default_exec_approval_decisions( network_approval_context: Option<&codex_app_server_protocol::NetworkApprovalContext>, proposed_execpolicy_amendment: Option<&codex_app_server_protocol::ExecPolicyAmendment>, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 300b85209..f4bb130bc 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -1249,6 +1249,75 @@ async fn token_usage_update_refreshes_status_line_with_runtime_context_window() ); } +#[tokio::test] +async fn collab_receiver_notification_caches_thread_without_app_server_read() { + let mut app = make_test_app().await; + let receiver_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000123").expect("valid thread id"); + + app.handle_thread_event_now(ThreadBufferedEvent::Notification( + ServerNotification::ItemStarted(ItemStartedNotification { + thread_id: ThreadId::new().to_string(), + turn_id: "turn-1".to_string(), + started_at_ms: 0, + item: ThreadItem::CollabAgentToolCall { + id: "wait-1".to_string(), + tool: codex_app_server_protocol::CollabAgentTool::Wait, + status: codex_app_server_protocol::CollabAgentToolCallStatus::InProgress, + sender_thread_id: ThreadId::new().to_string(), + receiver_thread_ids: vec![receiver_thread_id.to_string()], + prompt: None, + model: None, + reasoning_effort: None, + agents_states: HashMap::new(), + }, + }), + )); + + assert_eq!( + app.agent_navigation.get(&receiver_thread_id), + Some(&AgentPickerThreadEntry { + agent_nickname: None, + agent_role: None, + is_closed: false, + }) + ); +} + +#[tokio::test] +async fn collab_receiver_notification_does_not_cache_not_found_thread() { + let mut app = make_test_app().await; + let receiver_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000124").expect("valid thread id"); + + app.handle_thread_event_now(ThreadBufferedEvent::Notification( + ServerNotification::ItemCompleted(codex_app_server_protocol::ItemCompletedNotification { + thread_id: ThreadId::new().to_string(), + turn_id: "turn-1".to_string(), + completed_at_ms: 0, + item: ThreadItem::CollabAgentToolCall { + id: "send-1".to_string(), + tool: codex_app_server_protocol::CollabAgentTool::SendInput, + status: codex_app_server_protocol::CollabAgentToolCallStatus::Failed, + sender_thread_id: ThreadId::new().to_string(), + receiver_thread_ids: vec![receiver_thread_id.to_string()], + prompt: Some("hello".to_string()), + model: None, + reasoning_effort: None, + agents_states: HashMap::from([( + receiver_thread_id.to_string(), + codex_app_server_protocol::CollabAgentState { + status: codex_app_server_protocol::CollabAgentStatus::NotFound, + message: None, + }, + )]), + }, + }), + )); + + assert_eq!(app.agent_navigation.get(&receiver_thread_id), None); +} + #[tokio::test] async fn open_agent_picker_keeps_missing_threads_for_replay() -> Result<()> { let mut app = Box::pin(make_test_app()).await; diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 9a76b5416..f25398b0b 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -842,20 +842,14 @@ impl App { Ok(()) } - /// Eagerly fetches nickname and role for receiver threads referenced by a collab notification. + /// Locally remembers receiver threads referenced by a collab notification. /// - /// This runs on every buffered thread notification before it reaches rendering. For each - /// receiver thread id that the navigation cache does not yet have metadata for, it issues a - /// `thread/read` RPC and registers the result in both `AgentNavigationState` and the - /// `ChatWidget` metadata map. Threads that already have a nickname or role cached are skipped, - /// so the cost is at most one RPC per thread over the lifetime of a session. - /// - /// Failures are logged and silently ignored -- the worst outcome is that a rendered item shows - /// a thread id instead of a human-readable name, which is the same behavior the TUI had before - /// this change. - pub(super) async fn hydrate_collab_agent_metadata_for_notification( + /// This intentionally avoids app-server reads on the active-thread rendering path. During large + /// fan-outs the app-server can be saturated with spawn work, and blocking here would freeze the + /// TUI event loop. Metadata from `ThreadStarted` or explicit picker refreshes still fills in + /// names and roles later; until then, rendering falls back to the thread id. + pub(super) fn cache_collab_receiver_threads_for_notification( &mut self, - app_server: &mut AppServerSession, notification: &ServerNotification, ) { let Some(receiver_thread_ids) = collab_receiver_thread_ids(notification) else { @@ -863,42 +857,26 @@ impl App { }; for receiver_thread_id in receiver_thread_ids { + if collab_receiver_is_not_found(notification, receiver_thread_id) { + continue; + } + let Ok(thread_id) = ThreadId::from_string(receiver_thread_id) else { tracing::warn!( thread_id = receiver_thread_id, - "ignoring collab receiver with invalid thread id during metadata hydration" + "ignoring collab receiver with invalid thread id during local caching" ); continue; }; - if self - .agent_navigation - .get(&thread_id) - .is_some_and(|entry| entry.agent_nickname.is_some() || entry.agent_role.is_some()) - { + if self.agent_navigation.get(&thread_id).is_some() { continue; } - match app_server - .thread_read(thread_id, /*include_turns*/ false) - .await - { - Ok(thread) => { - self.upsert_agent_picker_thread( - thread_id, - thread.agent_nickname, - thread.agent_role, - /*is_closed*/ false, - ); - } - Err(err) => { - tracing::warn!( - thread_id = %thread_id, - error = %err, - "failed to hydrate collab receiver thread metadata" - ); - } - } + self.upsert_agent_picker_thread( + thread_id, /*agent_nickname*/ None, /*agent_role*/ None, + /*is_closed*/ false, + ); } } @@ -1365,6 +1343,7 @@ impl App { ); match event { ThreadBufferedEvent::Notification(notification) => { + self.cache_collab_receiver_threads_for_notification(¬ification); self.chat_widget .handle_server_notification(notification, /*replay_kind*/ None); } @@ -1467,11 +1446,6 @@ impl App { // thread, so unrelated shutdowns cannot consume this marker. self.pending_shutdown_exit_thread_id = None; } - if let ThreadBufferedEvent::Notification(notification) = &event { - self.hydrate_collab_agent_metadata_for_notification(app_server, notification) - .await; - } - self.handle_thread_event_now(event); if self.backtrack_render_pending { tui.frame_requester().schedule_frame();