From 231945a73e7ad9e595ff4469905cf2de17850d11 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 4 Jul 2026 16:11:51 -0300 Subject: [PATCH] refactor(tui): scope conversation presentation events --- codex-rs/tui/src/app.rs | 1 + codex-rs/tui/src/app/conversation_events.rs | 61 ++++++++++ .../tui/src/app/conversation_events_tests.rs | 82 +++++++++++++ codex-rs/tui/src/app/event_dispatch.rs | 34 +++++- codex-rs/tui/src/app/session_lifecycle.rs | 7 ++ codex-rs/tui/src/app/tests.rs | 25 +++- codex-rs/tui/src/app_event.rs | 56 ++++++++- codex-rs/tui/src/app_event_sender.rs | 87 ++++++++++---- codex-rs/tui/src/app_event_sender_tests.rs | 110 +++++++++++++++++- codex-rs/tui/src/chatwidget.rs | 2 + codex-rs/tui/src/chatwidget/constructor.rs | 10 +- .../src/chatwidget/tests/status_and_layout.rs | 4 + codex-rs/tui/src/file_search.rs | 10 +- codex-rs/tui/src/session_log.rs | 37 ++++-- 14 files changed, 480 insertions(+), 46 deletions(-) create mode 100644 codex-rs/tui/src/app/conversation_events.rs create mode 100644 codex-rs/tui/src/app/conversation_events_tests.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 4c0f016fc1..e1ac67c739 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -206,6 +206,7 @@ mod app_server_events; pub(crate) mod app_server_requests; mod background_requests; mod config_persistence; +mod conversation_events; mod event_dispatch; mod history_ui; mod input; diff --git a/codex-rs/tui/src/app/conversation_events.rs b/codex-rs/tui/src/app/conversation_events.rs new file mode 100644 index 0000000000..237eab0767 --- /dev/null +++ b/codex-rs/tui/src/app/conversation_events.rs @@ -0,0 +1,61 @@ +//! Conversation-origin routing policy for application events. +//! +//! Widget-owned senders attach a pane generation to their events. Presentation events from a +//! retired widget are discarded. Only events whose handlers are independent of widget-local +//! request state are allowed to outlive their originating generation. + +use crate::app_event::AppEvent; +use crate::app_event::ConversationOrigin; + +pub(super) fn normalize_conversation_event( + event: AppEvent, + current_origin: Option, +) -> Option<(Option, AppEvent)> { + match event { + AppEvent::FromConversation { target, event } if current_origin == Some(target) => { + Some((Some(target), *event)) + } + AppEvent::FromConversation { event, .. } if survives_conversation_retirement(&event) => { + Some((None, *event)) + } + AppEvent::FromConversation { .. } => None, + event => Some((None, event)), + } +} + +pub(super) fn survives_conversation_retirement(event: &AppEvent) -> bool { + match event { + AppEvent::Exit(_) + | AppEvent::Logout + | AppEvent::FatalExitRequest(_) + | AppEvent::ConversationOp { .. } + | AppEvent::SubmitThreadOp { .. } + | AppEvent::AppendMessageHistoryEntry { .. } + | AppEvent::SyncThreadGitBranch { .. } + | AppEvent::SetThreadGoalDraft { .. } + | AppEvent::SetThreadGoalStatus { .. } + | AppEvent::ClearThreadGoal { .. } + | AppEvent::OpenUrlInBrowser { .. } + | AppEvent::OpenDesktopThread { .. } + | AppEvent::UpdateFeatureFlags { .. } + | AppEvent::UpdateMemorySettings { .. } + | AppEvent::ResetMemories + | AppEvent::SetSkillEnabled { .. } + | AppEvent::SetAppEnabled { .. } + | AppEvent::PetDisabled + | AppEvent::StatusLineSetup { .. } + | AppEvent::TerminalTitleSetup { .. } + | AppEvent::SyntaxThemeSelected { .. } + | AppEvent::KeymapCaptured { .. } + | AppEvent::KeymapCleared { .. } => true, + AppEvent::FeedbackSubmitted { + origin_thread_id: Some(_), + .. + } => true, + _ => false, + } +} + +#[cfg(test)] +#[path = "conversation_events_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app/conversation_events_tests.rs b/codex-rs/tui/src/app/conversation_events_tests.rs new file mode 100644 index 0000000000..9f48b428fc --- /dev/null +++ b/codex-rs/tui/src/app/conversation_events_tests.rs @@ -0,0 +1,82 @@ +use super::*; +use crate::app_event::ExitMode; +use crate::app_event::PaneGeneration; +use crate::app_event::PaneSlot; +use pretty_assertions::assert_eq; + +fn origin(pane: PaneSlot) -> ConversationOrigin { + ConversationOrigin { + pane, + generation: PaneGeneration::fresh(), + } +} + +#[test] +fn current_generation_dispatches_to_its_conversation() { + let current = origin(PaneSlot::Parent); + let routed = normalize_conversation_event( + AppEvent::FromConversation { + target: current, + event: Box::new(AppEvent::RawOutputModeChanged { enabled: true }), + }, + Some(current), + ) + .expect("current event should be delivered"); + + assert_eq!(routed.0, Some(current)); + assert!(matches!( + routed.1, + AppEvent::RawOutputModeChanged { enabled: true } + )); +} + +#[test] +fn stale_presentation_event_is_dropped() { + let stale = origin(PaneSlot::Parent); + let current = origin(PaneSlot::Parent); + + assert!( + normalize_conversation_event( + AppEvent::FromConversation { + target: stale, + event: Box::new(AppEvent::RawOutputModeChanged { enabled: true }), + }, + Some(current), + ) + .is_none() + ); +} + +#[test] +fn stale_widget_completion_is_dropped() { + let stale = origin(PaneSlot::Parent); + let current = origin(PaneSlot::Parent); + + assert!( + normalize_conversation_event( + AppEvent::FromConversation { + target: stale, + event: Box::new(AppEvent::PluginMentionsLoaded { plugins: None }), + }, + Some(current), + ) + .is_none() + ); +} + +#[test] +fn stale_durable_event_is_delivered_without_pane_dispatch() { + let stale = origin(PaneSlot::Side); + let current = origin(PaneSlot::Parent); + let routed = normalize_conversation_event( + AppEvent::FromConversation { + target: stale, + event: Box::new(AppEvent::Exit(ExitMode::ShutdownFirst)), + }, + Some(current), + ) + .expect("durable event should be delivered"); + + assert_eq!(routed.0, None); + assert!(matches!(routed.1, AppEvent::Exit(ExitMode::ShutdownFirst))); +} diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index a2d8447e4e..e99ef3726d 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -3,6 +3,7 @@ //! This module contains the exhaustive `AppEvent` dispatcher and exit-mode handling. Large domain //! actions are delegated to focused app submodules so the central match remains the routing layer. +use super::conversation_events::normalize_conversation_event; use super::resize_reflow::trailing_run_start; use super::*; use crate::config_update::format_config_error; @@ -18,8 +19,38 @@ impl App { tui: &mut tui::Tui, app_server: &mut AppServerSession, event: AppEvent, + ) -> Result { + let Some((conversation_origin, event)) = + normalize_conversation_event(event, self.chat_widget.conversation_origin()) + else { + tracing::debug!("dropping event from retired conversation"); + return Ok(AppRunControl::Continue); + }; + + let previous_sender = if let Some(origin) = conversation_origin { + let sender = self.chat_widget.conversation_event_sender(); + debug_assert_eq!(sender.conversation_origin(), Some(origin)); + Some(std::mem::replace(&mut self.app_event_tx, sender)) + } else { + None + }; + let result = self.handle_event_inner(tui, app_server, event).await; + if let Some(previous_sender) = previous_sender { + self.app_event_tx = previous_sender; + } + result + } + + async fn handle_event_inner( + &mut self, + tui: &mut tui::Tui, + app_server: &mut AppServerSession, + event: AppEvent, ) -> Result { match event { + AppEvent::FromConversation { .. } => { + unreachable!("conversation event envelope should be normalized before dispatch") + } AppEvent::NewSession => { self.start_fresh_session_with_summary_hint( tui, app_server, /*session_start_source*/ None, @@ -787,7 +818,8 @@ impl App { ); } AppEvent::StartFileSearch(query) => { - self.file_search.on_user_query(query); + self.file_search + .on_user_query(query, self.app_event_tx.clone()); } AppEvent::FileSearchResult { query, matches } => { self.chat_widget.apply_file_search_result(query, matches); diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 9c953aa725..9074ac2c75 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -325,6 +325,13 @@ impl App { /// This helper copies every known nickname/role from `AgentNavigationState` into the /// replacement widget so that replayed collab items render agent names immediately. pub(super) fn replace_chat_widget(&mut self, mut chat_widget: ChatWidget) { + // A ticker belongs to the widget generation that started it. Stop it before installing a + // replacement so a stale StopCommitAnimation event cannot leave the shared latch stuck. + let retired_commit_anim_running = std::mem::replace( + &mut self.commit_anim_running, + Arc::new(AtomicBool::new(/*v*/ false)), + ); + retired_commit_anim_running.store(/*val*/ false, Ordering::Release); // Transfer the last-written terminal title to the replacement widget // so it knows what OSC title is currently displayed. Without this, the // new widget would redundantly clear and rewrite the same title, causing diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 929691eb75..25c137cc65 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -121,6 +121,13 @@ fn test_absolute_path(path: &str) -> AbsolutePathBuf { AbsolutePathBuf::try_from(PathBuf::from(path)).expect("absolute test path") } +fn conversation_event_payload(event: AppEvent) -> AppEvent { + match event { + AppEvent::FromConversation { event, .. } => *event, + event => event, + } +} + async fn next_thread_settings_updated( app_server: &mut AppServerSession, thread_id: ThreadId, @@ -361,7 +368,7 @@ async fn enqueue_primary_thread_session_replays_turns_before_initial_prompt_subm let mut saw_replayed_answer = false; let mut submitted_items = None; while let Ok(event) = app_event_rx.try_recv() { - match event { + match conversation_event_payload(event) { AppEvent::InsertHistoryCell(cell) => { let transcript = lines_to_single_string(&cell.transcript_lines(/*width*/ 80)); saw_replayed_answer |= transcript.contains("earlier prompt"); @@ -376,6 +383,13 @@ async fn enqueue_primary_thread_session_replays_turns_before_initial_prompt_subm AppEvent::CodexOp(Op::UserTurn { items, .. }) => { submitted_items = Some(items); } + AppEvent::ConversationOp { + target, + op: Op::UserTurn { items, .. }, + } => { + assert_eq!(target.thread_id, thread_id); + submitted_items = Some(items); + } _ => {} } } @@ -5407,7 +5421,15 @@ async fn replace_chat_widget_reseeds_collab_agent_metadata_for_replay() { terminal_title_invalid_items_warned: app.terminal_title_invalid_items_warned.clone(), session_telemetry: app.session_telemetry.clone(), }); + let retired_commit_anim_running = app.commit_anim_running.clone(); + retired_commit_anim_running.store(/*val*/ true, Ordering::Release); app.replace_chat_widget(replacement); + assert!(!app.commit_anim_running.load(Ordering::Acquire)); + assert!(!retired_commit_anim_running.load(Ordering::Acquire)); + assert!(!Arc::ptr_eq( + &app.commit_anim_running, + &retired_commit_anim_running + )); app.replay_thread_snapshot( ThreadEventSnapshot { @@ -5441,6 +5463,7 @@ async fn replace_chat_widget_reseeds_collab_agent_metadata_for_replay() { let mut saw_named_wait = false; while let Ok(event) = app_event_rx.try_recv() { + let event = conversation_event_payload(event); if let AppEvent::InsertHistoryCell(cell) = event { let transcript = lines_to_single_string(&cell.transcript_lines(/*width*/ 80)); saw_named_wait |= transcript.contains("Robie [explorer]"); diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index d53ea7455f..495899d5d5 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -9,6 +9,8 @@ //! quits without reaching into the app loop or coupling to shutdown/exit sequencing. use std::path::PathBuf; +use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering; use codex_app_server_protocol::AddCreditsNudgeCreditType; use codex_app_server_protocol::AddCreditsNudgeEmailStatus; @@ -141,15 +143,55 @@ pub(crate) enum KeymapEditIntent { ReplaceOne { old_key: String }, } -/// Identifies the conversation surface that originated an asynchronous app event. -/// -/// Keeping this as a distinct type lets the multi-pane UI extend the target with a pane -/// generation without changing every conversation-scoped event again. +/// Fixed conversation surfaces supported by the owned-screen layout. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PaneSlot { + Parent, + #[allow(dead_code)] + Side, +} + +/// Distinguishes successive widgets installed in the same [`PaneSlot`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct PaneGeneration(u64); + +impl PaneGeneration { + pub(crate) fn fresh() -> Self { + static NEXT_PANE_GENERATION: AtomicU64 = AtomicU64::new(/*v*/ 1); + + Self(NEXT_PANE_GENERATION.fetch_add(/*val*/ 1, Ordering::Relaxed)) + } + + pub(crate) fn get(self) -> u64 { + self.0 + } +} + +/// Immutable identity of the widget that originated an asynchronous app event. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct ConversationOrigin { + pub(crate) pane: PaneSlot, + pub(crate) generation: PaneGeneration, +} + +/// Identifies both a conversation widget generation and its bound thread. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) struct ConversationTarget { + pub(crate) pane: PaneSlot, + pub(crate) generation: PaneGeneration, pub(crate) thread_id: ThreadId, } +impl ConversationTarget { + #[allow(dead_code)] + pub(crate) fn origin(self) -> ConversationOrigin { + ConversationOrigin { + pane: self.pane, + generation: self.generation, + } + } +} + #[allow(clippy::large_enum_variant)] #[derive(Debug)] pub(crate) enum AppEvent { @@ -176,6 +218,12 @@ pub(crate) enum AppEvent { op: AppCommand, }, + /// Deliver a non-operation event to the conversation widget generation that emitted it. + FromConversation { + target: ConversationOrigin, + event: Box, + }, + /// Interrupt, roll back, and retry a safety-buffered turn with the server-selected model. RetrySafetyBufferedTurn { thread_id: ThreadId, diff --git a/codex-rs/tui/src/app_event_sender.rs b/codex-rs/tui/src/app_event_sender.rs index 7b1b453fc1..e8768b3a7d 100644 --- a/codex-rs/tui/src/app_event_sender.rs +++ b/codex-rs/tui/src/app_event_sender.rs @@ -19,74 +19,115 @@ use codex_protocol::request_permissions::RequestPermissionsResponse; use tokio::sync::mpsc::UnboundedSender; use crate::app_event::AppEvent; +use crate::app_event::ConversationOrigin; use crate::app_event::ConversationTarget; +use crate::app_event::PaneGeneration; +use crate::app_event::PaneSlot; use crate::session_log; +#[derive(Clone, Debug)] +struct ConversationScope { + origin: ConversationOrigin, + thread_id: Arc>>, +} + +fn has_explicit_conversation_delivery(event: &AppEvent) -> bool { + matches!( + event, + AppEvent::ConversationOp { .. } + | AppEvent::FromConversation { .. } + | AppEvent::SubmitThreadOp { .. } + ) +} + #[derive(Clone, Debug)] pub(crate) struct AppEventSender { pub app_event_tx: UnboundedSender, - conversation_target: Option>>>, + conversation_scope: Option, } impl AppEventSender { pub(crate) fn new(app_event_tx: UnboundedSender) -> Self { Self { app_event_tx, - conversation_target: None, + conversation_scope: None, } } /// Returns a sender scope shared by one chat widget and all of its child views. - pub(crate) fn scoped_to_conversation(&self) -> Self { + pub(crate) fn scoped_to_conversation(&self, pane: PaneSlot) -> Self { Self { app_event_tx: self.app_event_tx.clone(), - conversation_target: Some(Arc::new(RwLock::default())), + conversation_scope: Some(ConversationScope { + origin: ConversationOrigin { + pane, + generation: PaneGeneration::fresh(), + }, + thread_id: Arc::new(RwLock::default()), + }), } } /// Binds this widget scope to the thread supplied by its session configuration. pub(crate) fn bind_conversation_thread(&self, thread_id: ThreadId) { - let Some(target) = &self.conversation_target else { + let Some(scope) = &self.conversation_scope else { tracing::warn!(%thread_id, "cannot bind an unscoped app event sender"); return; }; - let mut target = match target.write() { - Ok(target) => target, + let mut bound_thread_id = match scope.thread_id.write() { + Ok(thread_id) => thread_id, Err(poisoned) => poisoned.into_inner(), }; - *target = Some(ConversationTarget { thread_id }); + *bound_thread_id = Some(thread_id); + } + + pub(crate) fn conversation_origin(&self) -> Option { + self.conversation_scope.as_ref().map(|scope| scope.origin) } fn conversation_target(&self) -> Option { - let target = self.conversation_target.as_ref()?; - let target = match target.read() { - Ok(target) => target, + let scope = self.conversation_scope.as_ref()?; + let thread_id = match scope.thread_id.read() { + Ok(thread_id) => thread_id, Err(poisoned) => poisoned.into_inner(), }; - *target + Some(ConversationTarget { + pane: scope.origin.pane, + generation: scope.origin.generation, + thread_id: (*thread_id)?, + }) } /// Send an event to the app event channel. If it fails, we swallow the /// error and log it. pub(crate) fn send(&self, event: AppEvent) { - let event = match event { - AppEvent::CodexOp(op) if self.conversation_target.is_some() => { + // Record inbound events for high-fidelity session replay before adding the conversation + // envelope, so existing event-specific logging remains intact. + // Avoid double-logging Ops; those are logged at the point of submission. + if !matches!( + event, + AppEvent::CodexOp(_) + | AppEvent::ConversationOp { .. } + | AppEvent::FromConversation { .. } + ) { + session_log::log_inbound_app_event(&event, self.conversation_origin()); + } + + let event = match (self.conversation_scope.as_ref(), event) { + (Some(_), AppEvent::CodexOp(op)) => { let Some(target) = self.conversation_target() else { tracing::warn!("dropping op from an unbound conversation sender"); return; }; AppEvent::ConversationOp { target, op } } - event => event, + (_, event) if has_explicit_conversation_delivery(&event) => event, + (Some(scope), event) => AppEvent::FromConversation { + target: scope.origin, + event: Box::new(event), + }, + (None, event) => event, }; - // Record inbound events for high-fidelity session replay. - // Avoid double-logging Ops; those are logged at the point of submission. - if !matches!( - event, - AppEvent::CodexOp(_) | AppEvent::ConversationOp { .. } - ) { - session_log::log_inbound_app_event(&event); - } if let Err(e) = self.app_event_tx.send(event) { tracing::error!("failed to send event: {e}"); } diff --git a/codex-rs/tui/src/app_event_sender_tests.rs b/codex-rs/tui/src/app_event_sender_tests.rs index 156a47703d..5b60422424 100644 --- a/codex-rs/tui/src/app_event_sender_tests.rs +++ b/codex-rs/tui/src/app_event_sender_tests.rs @@ -6,7 +6,7 @@ use super::*; #[test] fn conversation_sender_captures_bound_thread_for_codex_ops() { let (tx, mut rx) = unbounded_channel(); - let sender = AppEventSender::new(tx).scoped_to_conversation(); + let sender = AppEventSender::new(tx).scoped_to_conversation(PaneSlot::Parent); let cloned_sender = sender.clone(); let thread_id = ThreadId::new(); sender.bind_conversation_thread(thread_id); @@ -17,6 +17,7 @@ fn conversation_sender_captures_bound_thread_for_codex_ops() { let AppEvent::ConversationOp { target, op } = event else { panic!("expected conversation-scoped op"); }; + assert_eq!(target.pane, PaneSlot::Parent); assert_eq!(target.thread_id, thread_id); assert!(matches!(op, AppCommand::Compact { .. })); } @@ -25,8 +26,8 @@ fn conversation_sender_captures_bound_thread_for_codex_ops() { fn independent_conversation_senders_do_not_share_targets() { let (tx, mut rx) = unbounded_channel(); let sender = AppEventSender::new(tx); - let first_sender = sender.scoped_to_conversation(); - let second_sender = sender.scoped_to_conversation(); + let first_sender = sender.scoped_to_conversation(PaneSlot::Parent); + let second_sender = sender.scoped_to_conversation(PaneSlot::Side); let first_thread_id = ThreadId::new(); let second_thread_id = ThreadId::new(); first_sender.bind_conversation_thread(first_thread_id); @@ -36,10 +37,14 @@ fn independent_conversation_senders_do_not_share_targets() { second_sender.compact(); let targets = [rx.try_recv(), rx.try_recv()].map(|event| match event { - Ok(AppEvent::ConversationOp { target, .. }) => target.thread_id, + Ok(AppEvent::ConversationOp { target, .. }) => target, _ => panic!("expected conversation-scoped op"), }); - assert_eq!(targets, [first_thread_id, second_thread_id]); + assert_eq!(targets[0].pane, PaneSlot::Parent); + assert_eq!(targets[0].thread_id, first_thread_id); + assert_eq!(targets[1].pane, PaneSlot::Side); + assert_eq!(targets[1].thread_id, second_thread_id); + assert_ne!(targets[0].generation, targets[1].generation); } #[test] @@ -58,9 +63,102 @@ fn global_sender_preserves_active_thread_codex_ops() { #[test] fn unbound_conversation_sender_does_not_fall_back_to_active_thread() { let (tx, mut rx) = unbounded_channel(); - let sender = AppEventSender::new(tx).scoped_to_conversation(); + let sender = AppEventSender::new(tx).scoped_to_conversation(PaneSlot::Parent); sender.compact(); assert!(rx.try_recv().is_err()); } + +#[test] +fn conversation_sender_envelopes_non_operations_before_thread_binding() { + let (tx, mut rx) = unbounded_channel(); + let sender = AppEventSender::new(tx).scoped_to_conversation(PaneSlot::Parent); + + sender.send(AppEvent::NewSession); + + let event = rx.try_recv().expect("conversation event should be sent"); + let AppEvent::FromConversation { target, event } = event else { + panic!("expected conversation envelope"); + }; + assert_eq!(target.pane, PaneSlot::Parent); + assert!(matches!(*event, AppEvent::NewSession)); +} + +#[test] +fn replacement_sender_has_a_new_generation_for_the_same_pane() { + let (tx, mut rx) = unbounded_channel(); + let sender = AppEventSender::new(tx); + let stale_sender = sender.scoped_to_conversation(PaneSlot::Parent); + let replacement_sender = sender.scoped_to_conversation(PaneSlot::Parent); + + stale_sender.send(AppEvent::ClearUi); + replacement_sender.send(AppEvent::ClearUi); + + let targets = [rx.try_recv(), rx.try_recv()].map(|event| match event { + Ok(AppEvent::FromConversation { target, event }) => { + assert!(matches!(*event, AppEvent::ClearUi)); + target + } + _ => panic!("expected conversation envelope"), + }); + assert_eq!(targets[0].pane, PaneSlot::Parent); + assert_eq!(targets[1].pane, PaneSlot::Parent); + assert_ne!(targets[0].generation, targets[1].generation); +} + +#[test] +fn explicit_thread_operation_is_not_enveloped() { + let (tx, mut rx) = unbounded_channel(); + let sender = AppEventSender::new(tx).scoped_to_conversation(PaneSlot::Parent); + let thread_id = ThreadId::new(); + + sender.send(AppEvent::SubmitThreadOp { + thread_id, + op: AppCommand::compact(), + }); + + assert!(matches!( + rx.try_recv(), + Ok(AppEvent::SubmitThreadOp { + thread_id: event_thread_id, + op: AppCommand::Compact { .. }, + }) if event_thread_id == thread_id + )); +} + +#[test] +fn widget_local_thread_transport_is_enveloped() { + let (tx, mut rx) = unbounded_channel(); + let sender = AppEventSender::new(tx).scoped_to_conversation(PaneSlot::Parent); + let thread_id = ThreadId::new(); + + sender.send(AppEvent::LookupMessageHistoryEntry { + thread_id, + offset: 3, + log_id: 7, + }); + + let event = rx.try_recv().expect("conversation event should be sent"); + let AppEvent::FromConversation { event, .. } = event else { + panic!("expected conversation envelope"); + }; + assert!(matches!( + *event, + AppEvent::LookupMessageHistoryEntry { + thread_id: event_thread_id, + offset: 3, + log_id: 7, + } if event_thread_id == thread_id + )); +} + +#[test] +fn global_sender_preserves_non_operation_events() { + let (tx, mut rx) = unbounded_channel(); + let sender = AppEventSender::new(tx); + + sender.send(AppEvent::NewSession); + + assert!(matches!(rx.try_recv(), Ok(AppEvent::NewSession))); +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index ad2fd61508..89b007d4f3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -257,7 +257,9 @@ fn normalize_thread_name(name: &str) -> Option { } use crate::app_event::AppEvent; +use crate::app_event::ConversationOrigin; use crate::app_event::ExitMode; +use crate::app_event::PaneSlot; use crate::app_event::PermissionProfileSelection; use crate::app_event::RateLimitRefreshOrigin; #[cfg(target_os = "windows")] diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index 6b400cb3ac..e9d720859e 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -4,10 +4,18 @@ use super::*; impl ChatWidget { pub(crate) fn new_with_app_event(mut common: ChatWidgetInit) -> Self { - common.app_event_tx = common.app_event_tx.scoped_to_conversation(); + common.app_event_tx = common.app_event_tx.scoped_to_conversation(PaneSlot::Parent); Self::new_with_op_target(common, CodexOpTarget::AppEvent) } + pub(crate) fn conversation_origin(&self) -> Option { + self.app_event_tx.conversation_origin() + } + + pub(crate) fn conversation_event_sender(&self) -> AppEventSender { + self.app_event_tx.clone() + } + pub(super) fn new_with_op_target( common: ChatWidgetInit, codex_op_target: CodexOpTarget, diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index b1c645bf32..5f07bcb55e 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -451,6 +451,10 @@ async fn configured_pet_load_is_deferred_until_after_construction() { .await .unwrap() .unwrap(); + let event = match event { + AppEvent::FromConversation { event, .. } => *event, + event => event, + }; assert_matches!( event, AppEvent::ConfiguredPetLoaded { pet_id, result } => { diff --git a/codex-rs/tui/src/file_search.rs b/codex-rs/tui/src/file_search.rs index 70ac98de11..9f760f6f60 100644 --- a/codex-rs/tui/src/file_search.rs +++ b/codex-rs/tui/src/file_search.rs @@ -50,9 +50,17 @@ impl FileSearchManager { } /// Call whenever the user edits the `@` token. - pub fn on_user_query(&self, query: String) { + pub fn on_user_query(&mut self, query: String, app_tx: AppEventSender) { + let sender_changed = self.app_tx.conversation_origin() != app_tx.conversation_origin(); + if sender_changed { + self.app_tx = app_tx; + } #[expect(clippy::unwrap_used)] let mut st = self.state.lock().unwrap(); + if sender_changed { + st.session.take(); + st.latest_query.clear(); + } if query == st.latest_query { return; } diff --git a/codex-rs/tui/src/session_log.rs b/codex-rs/tui/src/session_log.rs index 369e19d7fd..098bdae2fe 100644 --- a/codex-rs/tui/src/session_log.rs +++ b/codex-rs/tui/src/session_log.rs @@ -12,6 +12,8 @@ use serde::Serialize; use serde_json::json; use crate::app_event::AppEvent; +use crate::app_event::ConversationOrigin; +use crate::app_event::PaneSlot; static LOGGER: LazyLock = LazyLock::new(SessionLogger::new); @@ -119,7 +121,7 @@ pub(crate) fn maybe_init(config: &Config) { LOGGER.write_json_line(header); } -pub(crate) fn log_inbound_app_event(event: &AppEvent) { +pub(crate) fn log_inbound_app_event(event: &AppEvent, origin: Option) { // Log only if enabled if !LOGGER.is_enabled() { return; @@ -132,7 +134,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "dir": "to_tui", "kind": "new_session", }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } AppEvent::ClearUi => { let value = json!({ @@ -140,7 +142,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "dir": "to_tui", "kind": "clear_ui", }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } AppEvent::InsertHistoryCell(cell) => { let value = json!({ @@ -149,7 +151,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "kind": "insert_history_cell", "lines": cell.transcript_lines(u16::MAX).len(), }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } AppEvent::StartFileSearch(query) => { let value = json!({ @@ -158,7 +160,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "kind": "file_search_start", "query": query, }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } AppEvent::FileSearchResult { query, matches } => { let value = json!({ @@ -168,7 +170,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "query": query, "matches": matches.len(), }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } AppEvent::PetPreviewLoaded { request_id, result } => { let value = json!({ @@ -179,7 +181,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "request_id": request_id, "ok": result.is_ok(), }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } AppEvent::PetSelectionLoaded { request_id, @@ -195,7 +197,7 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "pet_id": pet_id, "ok": result.is_ok(), }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } // Noise or control flow – record variant only other => { @@ -205,11 +207,28 @@ pub(crate) fn log_inbound_app_event(event: &AppEvent) { "kind": "app_event", "variant": format!("{other:?}").split('(').next().unwrap_or("app_event"), }); - LOGGER.write_json_line(value); + write_inbound_app_event(value, origin); } } } +fn write_inbound_app_event(mut value: serde_json::Value, origin: Option) { + if let Some(origin) = origin + && let Some(fields) = value.as_object_mut() + { + let pane = match origin.pane { + PaneSlot::Parent => "parent", + PaneSlot::Side => "side", + }; + fields.insert("conversation_pane".to_string(), json!(pane)); + fields.insert( + "conversation_generation".to_string(), + json!(origin.generation.get()), + ); + } + LOGGER.write_json_line(value); +} + pub(crate) fn log_outbound_op(op: &AppCommand) { if !LOGGER.is_enabled() { return;