diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index 39d564b3dd..1d0b713a34 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -34,6 +34,8 @@ pub(crate) const AGENTS_OVERVIEW_VIEW_ID: &str = "agents-overview"; pub(super) struct AgentsOverviewState { /// Missing metadata records a local resume until the next metadata refresh. pub(super) threads: HashMap>, + /// Lifecycle removals take precedence over delayed tool registration responses. + pub(super) removed_threads: HashSet, /// Local visibility only; activity and metadata refreshes never reveal hidden roots. pub(super) hidden_threads: HashSet, pub(super) last_messages: HashMap, diff --git a/codex-rs/tui/src/app/agents_overview_tests.rs b/codex-rs/tui/src/app/agents_overview_tests.rs index 7eee6d465b..5c7e553757 100644 --- a/codex-rs/tui/src/app/agents_overview_tests.rs +++ b/codex-rs/tui/src/app/agents_overview_tests.rs @@ -285,7 +285,8 @@ fn overview_thread( #[tokio::test] async fn shared_overview_keeps_rows_and_replays_changes_over_stale_reads() -> Result<()> { let mut app = make_test_app().await; - let app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; let [retained, archived, deleted, created] = std::array::from_fn(|_| ThreadId::new()); let [ mut retained_thread, @@ -305,9 +306,6 @@ async fn shared_overview_keeps_rows_and_replays_changes_over_stale_reads() -> Re app.agents_overview.request_id = Some(request_id); // The seed is still in flight and the command center has never been opened. for notification in [ - ServerNotification::ThreadStarted(ThreadStartedNotification { - thread: created_thread.clone(), - }), ServerNotification::ThreadNameUpdated(ThreadNameUpdatedNotification { thread_id: retained.to_string(), thread_name: Some("New name".to_string()), @@ -328,6 +326,35 @@ async fn shared_overview_keeps_rows_and_replays_changes_over_stale_reads() -> Re ) .await; } + // Tool responses can arrive without thread/started or after newer updates/removals. + for thread in [ + retained_thread.clone(), + created_thread.clone(), + archived_thread.clone(), + deleted_thread.clone(), + ] { + let (registered, registration) = tokio::sync::oneshot::channel(); + Box::pin(app.handle_event( + &mut tui, + &mut app_server, + AppEvent::DynamicToolThreadStarted { + thread, + task_tools_available: false, + registered, + }, + )) + .await?; + registration.await?; + } + assert_eq!( + app.agents_overview.threads, + HashMap::from([ + (retained, Some(retained_thread.clone())), + (created, Some(created_thread.clone())), + ]) + ); + // A successful read supersedes registration; queued notifications supersede the read. + retained_thread.preview = "Fresh preview from the read".to_string(); app.apply_agents_overview_thread_refresh( &app_server, request_id, diff --git a/codex-rs/tui/src/app/agents_overview_threads.rs b/codex-rs/tui/src/app/agents_overview_threads.rs index b2f8949fc6..50510ee6a6 100644 --- a/codex-rs/tui/src/app/agents_overview_threads.rs +++ b/codex-rs/tui/src/app/agents_overview_threads.rs @@ -74,11 +74,13 @@ impl App { if started.thread.ephemeral { return; } + self.agents_overview.removed_threads.remove(&thread_id); let mut thread = started.thread.clone(); thread.turns.clear(); self.agents_overview.threads.insert(thread_id, Some(thread)); } ServerNotification::ThreadArchived(_) | ServerNotification::ThreadDeleted(_) => { + self.agents_overview.removed_threads.insert(thread_id); self.agents_overview .selected_permission_profiles .remove(&thread_id); @@ -88,6 +90,9 @@ impl App { self.agents_overview.threads.remove(&thread_id); self.agents_overview.refresh_thread_ids.remove(&thread_id); } + ServerNotification::ThreadUnarchived(_) => { + self.agents_overview.removed_threads.remove(&thread_id); + } ServerNotification::ThreadClosed(_) => { self.agents_overview.activity.remove(&thread_id); if let Some(usage) = self.agents_overview.usage.get_mut(&thread_id) { diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 6d86c4db17..62266f834e 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -248,10 +248,13 @@ impl App { .await?; } AppEvent::DynamicToolThreadStarted { - thread_id, + thread, task_tools_available, registered, } => { + let Ok(thread_id) = ThreadId::from_string(&thread.id) else { + return Ok(AppRunControl::Continue); + }; self.agents_overview .dispatched_requests .entry(thread_id) @@ -259,6 +262,15 @@ impl App { if task_tools_available { app_server.remember_task_tool_thread(thread_id); } + // Fallback metadata must not replay after fresh reads or newer notifications. + if !thread.ephemeral + && !self.agents_overview.removed_threads.contains(&thread_id) + && !self.agents_overview.threads.get(&thread_id).is_some_and(Option::is_some) { + self.agents_overview.threads.insert(thread_id, Some(thread)); + self.agents_overview.refresh_thread_ids.insert(thread_id); + } + self.refresh_changed_agents_overview_threads(app_server); + self.repaint_agents_overview(); let _ = registered.send(()); } AppEvent::DynamicToolCallCompleted { diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index d35127830a..28344b49be 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -1314,13 +1314,14 @@ async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() } }; let AppEvent::DynamicToolThreadStarted { - thread_id: child_thread_id, + thread, task_tools_available, registered, } = registration else { panic!("expected the MCP-created task to register") }; + let child_thread_id = ThreadId::from_string(&thread.id)?; assert!(task_tools_available); assert!(registered.send(()).is_ok()); let created = creation.await??; @@ -1335,14 +1336,34 @@ async fn local_daemon_registers_approval_gated_mcp_tools_for_both_start_paths() child["config"]["mcp_servers.codex_tui"], starts[0]["config"]["mcp_servers.codex_tui"] ); - let forked = call_tool( - 3, - "fork_thread", - serde_json::json!({"threadId": delegation_source}), - ) - .send() - .await?; + // Fork another task: this synthetic MCP call has no active turn to cut before. + let fork_source = + create_history_rollout(&app.config, ThreadHistoryMode::Legacy, "Task to fork")?; + let forked = tokio::spawn( + call_tool( + 3, + "fork_thread", + serde_json::json!({"threadId": fork_source}), + ) + .send(), + ); + let registration = tokio::time::timeout(Duration::from_secs(/*secs*/ 5), events.recv()) + .await? + .expect("fork registration event"); + let AppEvent::DynamicToolThreadStarted { thread, .. } = ®istration else { + panic!("expected the MCP fork to register") + }; + let forked_thread_id = ThreadId::from_string(&thread.id)?; + let expected_thread = thread.clone(); + let mut tui = crate::tui::test_support::make_test_tui()?; + Box::pin(app.handle_event(&mut tui, &mut app_server, registration)).await?; + assert_eq!( + app.agents_overview.threads[&forked_thread_id], + Some(expected_thread) + ); + let forked = forked.await??; assert!(forked.status().is_success()); + assert!(forked.text().await?.contains(&forked_thread_id.to_string())); let forked = recorded_params(&requests, "thread/fork") .pop() .expect("MCP-created fork request"); @@ -1763,7 +1784,7 @@ async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespac .await? .expect("background task registration event"); let AppEvent::DynamicToolThreadStarted { - thread_id: created_thread_id, + thread, task_tools_available, registered, } = registration @@ -1771,16 +1792,22 @@ async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespac panic!("expected background task registration before its first turn: {registration:?}") }; assert!(recorded_params(&requests, "turn/start").is_empty()); + let created_thread_id = ThreadId::from_string(&thread.id)?; + let expected_thread = thread.clone(); Box::pin(app.handle_event( &mut tui, &mut app_server, AppEvent::DynamicToolThreadStarted { - thread_id: created_thread_id, + thread, task_tools_available, registered, }, )) .await?; + assert_eq!( + app.agents_overview.threads[&created_thread_id], + Some(expected_thread) + ); assert!( app.agents_overview .dispatched_requests @@ -1848,7 +1875,7 @@ async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespac }, ); let AppEvent::DynamicToolThreadStarted { - thread_id: continued_thread_id, + thread, task_tools_available, registered, } = tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 5), events.recv()) @@ -1857,13 +1884,13 @@ async fn dynamic_tool_requests_ignore_other_namespaces_and_dispatch_tui_namespac else { panic!("expected follow-up task registration before its next turn") }; - assert_eq!(continued_thread_id, creation_source); + assert_eq!(ThreadId::from_string(&thread.id)?, creation_source); assert_eq!(recorded_params(&requests, "turn/start").len(), 1); Box::pin(app.handle_event( &mut tui, &mut app_server, AppEvent::DynamicToolThreadStarted { - thread_id: continued_thread_id, + thread, task_tools_available, registered, }, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index b78150b379..1129bbec22 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -518,9 +518,9 @@ pub(crate) enum AppEvent { result: color_eyre::Result, }, - /// Register a dynamically created background thread before its first turn starts. + /// Register a tool-created or resumed background thread and its overview metadata. DynamicToolThreadStarted { - thread_id: ThreadId, + thread: Thread, task_tools_available: bool, registered: tokio::sync::oneshot::Sender<()>, }, diff --git a/codex-rs/tui/src/dynamic_tools.rs b/codex-rs/tui/src/dynamic_tools.rs index e432195d20..2c6778caff 100644 --- a/codex-rs/tui/src/dynamic_tools.rs +++ b/codex-rs/tui/src/dynamic_tools.rs @@ -643,8 +643,8 @@ async fn execute_inner( ) .await .map_err(|error| error.to_string())?; - let thread_id = started.thread.id; - register_background_thread(app_event_tx, &thread_id, task_tools_available).await?; + let thread_id = started.thread.id.clone(); + register_background_thread(app_event_tx, started.thread, task_tools_available).await?; if let Some(title) = arguments.title && let Err(error) = request::(&handle, |request_id| { ClientRequest::ThreadSetName { @@ -735,10 +735,17 @@ async fn execute_inner( }, ) .await?; + let forked_thread_id = response.thread.id.clone(); + register_background_thread( + app_event_tx, + response.thread, + /*task_tools_available*/ false, + ) + .await?; Ok(json!({ "environment": {"type": "same-directory"}, "sourceThreadId": thread_id, - "threadId": response.thread.id, + "threadId": forked_thread_id, "continuation": "The fork contains completed history only. If the source thread was running, the active turn and unfinished response are not in the child. Send a follow-up message to threadId only if the task requires work to continue there." })) } @@ -752,7 +759,7 @@ async fn execute_inner( validate_prompt(&prompt, MAX_DELEGATED_INPUT_BYTES)?; let thread = read_thread(&handle, &arguments.thread_id).await?; let exclude_turns = thread.history_mode == ThreadHistoryMode::Paginated; - let _: ThreadResumeResponse = request_with_history_fallback( + let resumed: ThreadResumeResponse = request_with_history_fallback( &handle, exclude_turns, |request_id, exclude_turns| ClientRequest::ThreadResume { @@ -768,7 +775,7 @@ async fn execute_inner( .await?; register_background_thread( app_event_tx, - &arguments.thread_id, + resumed.thread, /*task_tools_available*/ false, ) .await?; @@ -1209,13 +1216,14 @@ fn same_thread_id(first: &str, second: &str) -> bool { async fn register_background_thread( app_event_tx: Option<&AppEventSender>, - thread_id: &str, + mut thread: Thread, task_tools_available: bool, ) -> Result<(), String> { if let Some(app_event_tx) = app_event_tx { + thread.turns.clear(); let (registered, registration) = tokio::sync::oneshot::channel(); app_event_tx.send(AppEvent::DynamicToolThreadStarted { - thread_id: ThreadId::from_string(thread_id).map_err(|error| error.to_string())?, + thread, task_tools_available, registered, });