diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 6de069420..896256374 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -178,6 +178,7 @@ mod app_server_adapter; pub(crate) mod app_server_requests; mod loaded_threads; mod pending_interactive_replay; +mod replay_filter; mod side; use self::agent_navigation::AgentNavigationDirection; @@ -185,6 +186,8 @@ use self::agent_navigation::AgentNavigationState; use self::app_server_requests::PendingAppServerRequests; use self::loaded_threads::find_loaded_subagent_threads_for_primary; use self::pending_interactive_replay::PendingInteractiveReplayState; +use self::side::SideParentStatus; +use self::side::SideParentStatusChange; use self::side::SideThreadState; const EXTERNAL_EDITOR_HINT: &str = "Save and close external editor to continue."; @@ -664,6 +667,25 @@ impl ThreadEventStore { } } + fn pending_replay_requests(&self) -> Vec { + self.buffer + .iter() + .filter_map(|event| match event { + ThreadBufferedEvent::Request(request) + if self + .pending_interactive_replay + .should_replay_snapshot_request(request) => + { + Some(request.clone()) + } + ThreadBufferedEvent::Request(_) + | ThreadBufferedEvent::Notification(_) + | ThreadBufferedEvent::HistoryEntryResponse(_) + | ThreadBufferedEvent::FeedbackSubmission(_) => None, + }) + .collect() + } + fn apply_thread_rollback(&mut self, response: &ThreadRollbackResponse) { self.turns = response.thread.turns.clone(); self.buffer.clear(); @@ -713,6 +735,22 @@ impl ThreadEventStore { .has_pending_thread_approvals() } + fn side_parent_pending_status(&self) -> Option { + if self + .pending_interactive_replay + .has_pending_thread_user_input() + { + Some(SideParentStatus::NeedsInput) + } else if self + .pending_interactive_replay + .has_pending_thread_approvals() + { + Some(SideParentStatus::NeedsApproval) + } else { + None + } + } + fn active_turn_id(&self) -> Option<&str> { self.active_turn_id.as_deref() } @@ -1997,6 +2035,58 @@ impl App { } } + fn push_thread_interactive_request(&mut self, request: ThreadInteractiveRequest) { + match request { + ThreadInteractiveRequest::Approval(request) => { + self.chat_widget.push_approval_request(request); + } + ThreadInteractiveRequest::McpServerElicitation(request) => { + self.chat_widget + .push_mcp_server_elicitation_request(request); + } + } + } + + async fn pending_inactive_thread_requests(&self) -> Vec<(ThreadId, ServerRequest)> { + let channels: Vec<(ThreadId, Arc>)> = self + .thread_event_channels + .iter() + .map(|(thread_id, channel)| (*thread_id, Arc::clone(&channel.store))) + .collect(); + + let mut requests = Vec::new(); + for (thread_id, store) in channels { + if Some(thread_id) == self.active_thread_id { + continue; + } + + let store = store.lock().await; + requests.extend( + store + .pending_replay_requests() + .into_iter() + .map(|request| (thread_id, request)), + ); + } + requests + } + + pub(super) async fn surface_pending_inactive_thread_interactive_requests(&mut self) { + if self.active_side_parent_thread_id().is_some() { + return; + } + + let requests = self.pending_inactive_thread_requests().await; + for (thread_id, request) in requests { + if let Some(request) = self + .interactive_request_for_thread_request(thread_id, &request) + .await + { + self.push_thread_interactive_request(request); + } + } + } + async fn submit_active_thread_op( &mut self, app_server: &mut AppServerSession, @@ -2037,6 +2127,7 @@ impl App { if ThreadEventStore::op_can_change_pending_replay_state(&op) { self.note_thread_outbound_op(thread_id, &op).await; self.refresh_pending_thread_approvals().await; + self.refresh_side_parent_status_from_store(thread_id).await; } return Ok(()); } @@ -2730,6 +2821,7 @@ impl App { if ThreadEventStore::op_can_change_pending_replay_state(op) { self.note_thread_outbound_op(thread_id, op).await; self.refresh_pending_thread_approvals().await; + self.refresh_side_parent_status_from_store(thread_id).await; } Ok(true) } @@ -2743,6 +2835,7 @@ impl App { } async fn refresh_pending_thread_approvals(&mut self) { + let side_parent_thread_id = self.active_side_parent_thread_id(); let channels: Vec<(ThreadId, Arc>)> = self .thread_event_channels .iter() @@ -2751,7 +2844,8 @@ impl App { let mut pending_thread_ids = Vec::new(); for (thread_id, store) in channels { - if Some(thread_id) == self.active_thread_id { + if Some(thread_id) == self.active_thread_id || Some(thread_id) == side_parent_thread_id + { continue; } @@ -2771,6 +2865,21 @@ impl App { self.chat_widget.set_pending_thread_approvals(threads); } + async fn refresh_side_parent_status_from_store(&mut self, thread_id: ThreadId) { + let Some(channel) = self.thread_event_channels.get(&thread_id) else { + return; + }; + let status = { + let store = channel.store.lock().await; + store.side_parent_pending_status() + }; + if let Some(status) = status { + self.set_side_parent_status(thread_id, Some(status)); + } else { + self.clear_side_parent_action_status(thread_id); + } + } + async fn enqueue_thread_notification( &mut self, thread_id: ThreadId, @@ -2784,7 +2893,7 @@ impl App { (channel.sender.clone(), Arc::clone(&channel.store)) }; - let should_send = { + let (should_send, pending_status) = { let mut guard = store.lock().await; if guard.session.is_none() && let Some(session) = inferred_session @@ -2792,8 +2901,9 @@ impl App { guard.session = Some(session); } guard.push_notification(notification.clone()); - guard.active + (guard.active, guard.side_parent_pending_status()) }; + let notification_status_change = SideParentStatusChange::for_notification(¬ification); if should_send { match sender.try_send(ThreadBufferedEvent::Notification(notification)) { @@ -2810,6 +2920,11 @@ impl App { } } } + if let Some(status) = pending_status { + self.set_side_parent_status(thread_id, Some(status)); + } else if let Some(change) = notification_status_change { + self.apply_side_parent_status_change(thread_id, change); + } self.refresh_pending_thread_approvals().await; Ok(()) } @@ -2923,11 +3038,12 @@ impl App { (channel.sender.clone(), Arc::clone(&channel.store)) }; - let should_send = { + let (should_send, pending_status) = { let mut guard = store.lock().await; guard.push_request(request.clone()); - guard.active + (guard.active, guard.side_parent_pending_status()) }; + let request_status = SideParentStatus::for_request(&request); if should_send { match sender.try_send(ThreadBufferedEvent::Request(request)) { @@ -2943,16 +3059,13 @@ impl App { tracing::warn!("thread {thread_id} event channel closed"); } } - } else if let Some(request) = inactive_interactive_request { - match request { - ThreadInteractiveRequest::Approval(request) => { - self.chat_widget.push_approval_request(request); - } - ThreadInteractiveRequest::McpServerElicitation(request) => { - self.chat_widget - .push_mcp_server_elicitation_request(request); - } - } + } else if self.active_side_parent_thread_id().is_none() + && let Some(request) = inactive_interactive_request + { + self.push_thread_interactive_request(request); + } + if let Some(status) = pending_status.or(request_status) { + self.set_side_parent_status(thread_id, Some(status)); } self.refresh_pending_thread_approvals().await; Ok(()) @@ -3863,9 +3976,13 @@ impl App { snapshot: ThreadEventSnapshot, resume_restored_queue: bool, ) { + let suppress_replay_notices = + replay_filter::snapshot_has_pending_interactive_request(&snapshot); if let Some(session) = snapshot.session { if self.side_threads.contains_key(&session.thread_id) { self.chat_widget.handle_side_thread_session(session); + } else if suppress_replay_notices { + self.chat_widget.handle_thread_session_quiet(session); } else { self.chat_widget.handle_thread_session(session); } @@ -3879,6 +3996,9 @@ impl App { .replay_thread_turns(snapshot.turns, ReplayKind::ThreadSnapshot); } for event in snapshot.events { + if suppress_replay_notices && replay_filter::event_is_notice(&event) { + continue; + } self.handle_thread_event_replay(event); } self.chat_widget @@ -6984,12 +7104,14 @@ mod tests { use codex_app_server_protocol::ThreadTokenUsage; use codex_app_server_protocol::ThreadTokenUsageUpdatedNotification; use codex_app_server_protocol::TokenUsageBreakdown; + use codex_app_server_protocol::ToolRequestUserInputParams; use codex_app_server_protocol::Turn; use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnError as AppServerTurnError; use codex_app_server_protocol::TurnStartedNotification; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as AppServerUserInput; + use codex_app_server_protocol::WarningNotification; use codex_config::types::ModelAvailabilityNuxConfig; use codex_otel::SessionTelemetry; use codex_protocol::ThreadId; @@ -9353,6 +9475,172 @@ guardian_approval = true Ok(()) } + #[tokio::test] + async fn side_defers_parent_approval_overlay_until_parent_replay() -> Result<()> { + let mut app = make_test_app().await; + let parent_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000011").expect("valid thread"); + let side_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000022").expect("valid thread"); + + app.primary_thread_id = Some(parent_thread_id); + app.active_thread_id = Some(side_thread_id); + app.side_threads + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); + app.thread_event_channels.insert( + parent_thread_id, + ThreadEventChannel::new_with_session( + /*capacity*/ 4, + test_thread_session(parent_thread_id, test_path_buf("/tmp/main")), + Vec::new(), + ), + ); + + app.enqueue_thread_request( + parent_thread_id, + exec_approval_request( + parent_thread_id, + "turn-approval", + "call-approval", + /*approval_id*/ None, + ), + ) + .await?; + + assert_eq!(app.chat_widget.has_active_view(), false); + assert!(app.chat_widget.pending_thread_approvals().is_empty()); + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + Some(SideParentStatus::NeedsApproval) + ); + + let snapshot = { + let channel = app + .thread_event_channels + .get(&parent_thread_id) + .expect("parent thread channel"); + let store = channel.store.lock().await; + store.snapshot() + }; + app.side_threads.remove(&side_thread_id); + app.active_thread_id = Some(parent_thread_id); + app.replay_thread_snapshot(snapshot, /*resume_restored_queue*/ false); + + assert_eq!(app.chat_widget.has_active_view(), true); + + Ok(()) + } + + #[tokio::test] + async fn replay_snapshot_with_pending_request_suppresses_replay_notices() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000011").expect("valid thread"); + let stale_warning = "stale startup warning that should not cover the approval"; + + app.replay_thread_snapshot( + ThreadEventSnapshot { + session: Some(test_thread_session(thread_id, test_path_buf("/tmp/main"))), + turns: Vec::new(), + events: vec![ + ThreadBufferedEvent::Notification(ServerNotification::Warning( + WarningNotification { + thread_id: Some(thread_id.to_string()), + message: stale_warning.to_string(), + }, + )), + ThreadBufferedEvent::Request(exec_approval_request( + thread_id, + "turn-approval", + "call-approval", + /*approval_id*/ None, + )), + ], + input_state: None, + }, + /*resume_restored_queue*/ false, + ); + + assert_eq!(app.chat_widget.has_active_view(), true); + + let mut replayed_history = String::new(); + while let Ok(event) = app_event_rx.try_recv() { + if let AppEvent::InsertHistoryCell(cell) = event { + replayed_history.push_str(&lines_to_single_string( + &cell.transcript_lines(/*width*/ 80), + )); + } + } + + assert!( + replayed_history.is_empty(), + "expected pending approval replay to suppress session notices, got {replayed_history:?}" + ); + } + + #[tokio::test] + async fn side_defers_subagent_approval_overlay_until_side_exits() -> Result<()> { + let mut app = make_test_app().await; + let main_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000011").expect("valid thread"); + let side_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000022").expect("valid thread"); + let agent_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000033").expect("valid thread"); + + app.primary_thread_id = Some(main_thread_id); + app.active_thread_id = Some(side_thread_id); + app.side_threads + .insert(side_thread_id, SideThreadState::new(main_thread_id)); + app.thread_event_channels.insert( + agent_thread_id, + ThreadEventChannel::new_with_session( + /*capacity*/ 4, + ThreadSessionState { + approval_policy: AskForApproval::OnRequest, + sandbox_policy: SandboxPolicy::new_workspace_write_policy(), + rollout_path: Some(test_path_buf("/tmp/agent-rollout.jsonl")), + ..test_thread_session(agent_thread_id, test_path_buf("/tmp/agent")) + }, + Vec::new(), + ), + ); + app.agent_navigation.upsert( + agent_thread_id, + Some("Robie".to_string()), + Some("explorer".to_string()), + /*is_closed*/ false, + ); + + app.enqueue_thread_request( + agent_thread_id, + exec_approval_request( + agent_thread_id, + "turn-approval", + "call-approval", + /*approval_id*/ None, + ), + ) + .await?; + + assert_eq!(app.chat_widget.has_active_view(), false); + assert_eq!( + app.chat_widget.pending_thread_approvals(), + &["Robie [explorer]".to_string()] + ); + + app.side_threads.remove(&side_thread_id); + app.active_thread_id = Some(main_thread_id); + app.surface_pending_inactive_thread_interactive_requests() + .await; + + assert_eq!(app.chat_widget.has_active_view(), true); + + Ok(()) + } + #[tokio::test] async fn inactive_thread_exec_approval_preserves_context() { let app = make_test_app().await; @@ -9921,7 +10209,7 @@ guardian_approval = true let parent_thread_id = ThreadId::new(); let side_thread_id = ThreadId::new(); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); assert_eq!( app.side_start_block_message(), @@ -9934,6 +10222,131 @@ guardian_approval = true assert_eq!(app.side_start_block_message(), None); } + #[tokio::test] + async fn side_parent_status_tracks_parent_turn_lifecycle() -> Result<()> { + let mut app = make_test_app().await; + let parent_thread_id = ThreadId::new(); + let side_thread_id = ThreadId::new(); + app.primary_thread_id = Some(parent_thread_id); + app.active_thread_id = Some(side_thread_id); + app.side_threads + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); + + app.enqueue_thread_notification( + parent_thread_id, + turn_completed_notification(parent_thread_id, "turn-1", TurnStatus::Completed), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + Some(SideParentStatus::Finished) + ); + + app.enqueue_thread_notification( + parent_thread_id, + turn_started_notification(parent_thread_id, "turn-2"), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + None + ); + + app.enqueue_thread_notification( + parent_thread_id, + turn_completed_notification(parent_thread_id, "turn-2", TurnStatus::Failed), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + Some(SideParentStatus::Failed) + ); + + Ok(()) + } + + #[tokio::test] + async fn side_parent_status_prioritizes_input_over_approval() -> Result<()> { + let mut app = make_test_app().await; + let parent_thread_id = ThreadId::new(); + let side_thread_id = ThreadId::new(); + app.primary_thread_id = Some(parent_thread_id); + app.active_thread_id = Some(side_thread_id); + app.side_threads + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); + + app.enqueue_thread_request( + parent_thread_id, + exec_approval_request( + parent_thread_id, + "turn-approval", + "call-approval", + /*approval_id*/ None, + ), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + Some(SideParentStatus::NeedsApproval) + ); + + app.enqueue_thread_request( + parent_thread_id, + request_user_input_request(parent_thread_id, "turn-input", "call-input"), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + Some(SideParentStatus::NeedsInput) + ); + + app.enqueue_thread_notification( + parent_thread_id, + ServerNotification::ServerRequestResolved( + codex_app_server_protocol::ServerRequestResolvedNotification { + thread_id: parent_thread_id.to_string(), + request_id: AppServerRequestId::Integer(2), + }, + ), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + Some(SideParentStatus::NeedsApproval) + ); + + app.enqueue_thread_notification( + parent_thread_id, + ServerNotification::ServerRequestResolved( + codex_app_server_protocol::ServerRequestResolvedNotification { + thread_id: parent_thread_id.to_string(), + request_id: AppServerRequestId::Integer(1), + }, + ), + ) + .await?; + assert_eq!( + app.side_threads + .get(&side_thread_id) + .and_then(|state| state.parent_status), + None + ); + + Ok(()) + } + #[test] fn side_start_error_message_explains_missing_first_prompt() { let err = color_eyre::eyre::eyre!( @@ -9992,7 +10405,7 @@ guardian_approval = true let parent_thread_id = ThreadId::new(); let side_thread_id = ThreadId::new(); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); let snapshot = ThreadEventSnapshot { session: Some(ThreadSessionState { @@ -10020,7 +10433,7 @@ guardian_approval = true let side_thread_id = ThreadId::new(); app.primary_thread_id = Some(parent_thread_id); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); let snapshot = ThreadEventSnapshot { session: Some(ThreadSessionState { @@ -10060,7 +10473,7 @@ guardian_approval = true app.primary_thread_id = Some(parent_thread_id); app.active_thread_id = Some(side_thread_id); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); app.sync_side_thread_ui(); app.handle_app_server_event( @@ -10098,7 +10511,7 @@ guardian_approval = true let side_thread_id = ThreadId::new(); app.active_thread_id = Some(side_thread_id); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); assert_eq!( app.side_thread_to_discard_after_switch(side_thread_id), @@ -10119,12 +10532,8 @@ guardian_approval = true side_config.ephemeral = true; let started = app_server.start_thread(&side_config).await?; let side_thread_id = started.session.thread_id; - app.side_threads.insert( - side_thread_id, - SideThreadState { - parent_thread_id: ThreadId::new(), - }, - ); + app.side_threads + .insert(side_thread_id, SideThreadState::new(ThreadId::new())); app.agent_navigation.upsert( side_thread_id, Some("Side".to_string()), @@ -10151,7 +10560,7 @@ guardian_approval = true let side_thread_id = ThreadId::new(); app.active_thread_id = Some(side_thread_id); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); app.agent_navigation.upsert( side_thread_id, Some("Side".to_string()), @@ -10182,7 +10591,7 @@ guardian_approval = true let side_thread_id = ThreadId::new(); app.active_thread_id = Some(side_thread_id); app.side_threads - .insert(side_thread_id, SideThreadState { parent_thread_id }); + .insert(side_thread_id, SideThreadState::new(parent_thread_id)); app.thread_event_channels .insert(side_thread_id, ThreadEventChannel::new(/*capacity*/ 4)); app.agent_navigation.upsert( @@ -10746,6 +11155,22 @@ guardian_approval = true } } + fn request_user_input_request( + thread_id: ThreadId, + turn_id: &str, + item_id: &str, + ) -> ServerRequest { + ServerRequest::ToolRequestUserInput { + request_id: AppServerRequestId::Integer(2), + params: ToolRequestUserInputParams { + thread_id: thread_id.to_string(), + turn_id: turn_id.to_string(), + item_id: item_id.to_string(), + questions: Vec::new(), + }, + } + } + #[test] fn thread_event_store_tracks_active_turn_lifecycle() { let mut store = ThreadEventStore::new(/*capacity*/ 8); diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 13372ea31..d3963e3d4 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -385,6 +385,10 @@ impl PendingInteractiveReplayState { || !self.request_permissions_call_ids.is_empty() } + pub(super) fn has_pending_thread_user_input(&self) -> bool { + !self.request_user_input_call_ids.is_empty() + } + fn clear_request_user_input_turn(&mut self, turn_id: &str) { if let Some(call_ids) = self.request_user_input_call_ids_by_turn_id.remove(turn_id) { for call_id in call_ids { diff --git a/codex-rs/tui/src/app/replay_filter.rs b/codex-rs/tui/src/app/replay_filter.rs new file mode 100644 index 000000000..ec9176ffa --- /dev/null +++ b/codex-rs/tui/src/app/replay_filter.rs @@ -0,0 +1,31 @@ +//! Helpers for deciding which buffered events to replay when switching threads. + +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::ServerRequest; + +use super::ThreadBufferedEvent; +use super::ThreadEventSnapshot; + +pub(super) fn snapshot_has_pending_interactive_request(snapshot: &ThreadEventSnapshot) -> bool { + snapshot.events.iter().any(|event| { + matches!( + event, + ThreadBufferedEvent::Request( + ServerRequest::CommandExecutionRequestApproval { .. } + | ServerRequest::FileChangeRequestApproval { .. } + | ServerRequest::McpServerElicitationRequest { .. } + | ServerRequest::PermissionsRequestApproval { .. } + | ServerRequest::ToolRequestUserInput { .. } + ) + ) + }) +} + +pub(super) fn event_is_notice(event: &ThreadBufferedEvent) -> bool { + matches!( + event, + ThreadBufferedEvent::Notification( + ServerNotification::Warning(_) | ServerNotification::ConfigWarning(_) + ) + ) +} diff --git a/codex-rs/tui/src/app/side.rs b/codex-rs/tui/src/app/side.rs index 56bc46649..e951233c5 100644 --- a/codex-rs/tui/src/app/side.rs +++ b/codex-rs/tui/src/app/side.rs @@ -47,10 +47,103 @@ You may perform non-mutating inspection, including reading or searching files an Do not modify files, source, git state, permissions, configuration, or any other workspace state unless the user explicitly requests that mutation in this side conversation. Do not request escalated permissions or broader sandbox access unless the user explicitly requests a mutation that requires it. If the user explicitly requests a mutation, keep it minimal, local to the request, and avoid disrupting the main thread."#; +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum SideParentStatus { + NeedsInput, + NeedsApproval, + Failed, + Interrupted, + Closed, + Finished, +} + +impl SideParentStatus { + fn label(self, parent_is_main: bool) -> &'static str { + match (self, parent_is_main) { + (SideParentStatus::NeedsInput, true) => "main needs input", + (SideParentStatus::NeedsInput, false) => "parent needs input", + (SideParentStatus::NeedsApproval, true) => "main needs approval", + (SideParentStatus::NeedsApproval, false) => "parent needs approval", + (SideParentStatus::Failed, true) => "main failed", + (SideParentStatus::Failed, false) => "parent failed", + (SideParentStatus::Interrupted, true) => "main interrupted", + (SideParentStatus::Interrupted, false) => "parent interrupted", + (SideParentStatus::Closed, true) => "main closed", + (SideParentStatus::Closed, false) => "parent closed", + (SideParentStatus::Finished, true) => "main finished", + (SideParentStatus::Finished, false) => "parent finished", + } + } + + fn is_actionable(self) -> bool { + matches!( + self, + SideParentStatus::NeedsInput | SideParentStatus::NeedsApproval + ) + } + + pub(super) fn for_request(request: &ServerRequest) -> Option { + match request { + ServerRequest::ToolRequestUserInput { .. } => Some(SideParentStatus::NeedsInput), + ServerRequest::CommandExecutionRequestApproval { .. } + | ServerRequest::FileChangeRequestApproval { .. } + | ServerRequest::McpServerElicitationRequest { .. } + | ServerRequest::PermissionsRequestApproval { .. } + | ServerRequest::ApplyPatchApproval { .. } + | ServerRequest::ExecCommandApproval { .. } => Some(SideParentStatus::NeedsApproval), + ServerRequest::DynamicToolCall { .. } + | ServerRequest::ChatgptAuthTokensRefresh { .. } => None, + } + } +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum SideParentStatusChange { + Set(SideParentStatus), + Clear, + ClearActionable, +} + +impl SideParentStatusChange { + pub(super) fn for_notification(notification: &ServerNotification) -> Option { + match notification { + ServerNotification::TurnStarted(_) => Some(SideParentStatusChange::Clear), + ServerNotification::TurnCompleted(notification) => match ¬ification.turn.status { + TurnStatus::Completed => { + Some(SideParentStatusChange::Set(SideParentStatus::Finished)) + } + TurnStatus::Interrupted => { + Some(SideParentStatusChange::Set(SideParentStatus::Interrupted)) + } + TurnStatus::Failed => Some(SideParentStatusChange::Set(SideParentStatus::Failed)), + TurnStatus::InProgress => None, + }, + ServerNotification::ThreadClosed(_) => { + Some(SideParentStatusChange::Set(SideParentStatus::Closed)) + } + ServerNotification::ItemStarted(_) | ServerNotification::ServerRequestResolved(_) => { + Some(SideParentStatusChange::ClearActionable) + } + _ => None, + } + } +} + #[derive(Clone, Debug)] pub(super) struct SideThreadState { /// Thread to return to when the current side conversation is dismissed. pub(super) parent_thread_id: ThreadId, + /// Parent-thread condition that changed while this side thread is visible. + pub(super) parent_status: Option, +} + +impl SideThreadState { + pub(super) fn new(parent_thread_id: ThreadId) -> Self { + Self { + parent_thread_id, + parent_status: None, + } + } } impl App { @@ -65,10 +158,10 @@ impl App { clear_side_ui(&mut self.chat_widget); return; }; - let Some(parent_thread_id) = self + let Some((parent_thread_id, parent_status)) = self .side_threads .get(&active_thread_id) - .map(|state| state.parent_thread_id) + .map(|state| (state.parent_thread_id, state.parent_status)) else { clear_side_ui(&mut self.chat_widget); return; @@ -80,14 +173,20 @@ impl App { .set_side_conversation_active(/*active*/ true); self.chat_widget .set_interrupted_turn_notice_mode(InterruptedTurnNoticeMode::Suppress); - let label = if self.primary_thread_id == Some(parent_thread_id) { - "from main thread · Esc to return".to_string() + let mut label_parts = Vec::new(); + let parent_is_main = self.primary_thread_id == Some(parent_thread_id); + if parent_is_main { + label_parts.push("from main thread".to_string()); } else { let parent_label = self.thread_label(parent_thread_id); - format!("from parent thread ({parent_label}) · Esc to return") - }; + label_parts.push(format!("from parent thread ({parent_label})")); + } + if let Some(parent_status) = parent_status { + label_parts.push(parent_status.label(parent_is_main).to_string()); + } + label_parts.push("Esc to return".to_string()); self.chat_widget - .set_side_conversation_context_label(Some(format!("Side {label}"))); + .set_side_conversation_context_label(Some(format!("Side {}", label_parts.join(" · ")))); } pub(super) fn active_side_parent_thread_id(&self) -> Option { @@ -96,6 +195,65 @@ impl App { .map(|state| state.parent_thread_id) } + pub(super) fn set_side_parent_status( + &mut self, + parent_thread_id: ThreadId, + status: Option, + ) { + let mut changed = false; + for state in self + .side_threads + .values_mut() + .filter(|state| state.parent_thread_id == parent_thread_id) + { + if state.parent_status != status { + state.parent_status = status; + changed = true; + } + } + if changed { + self.sync_side_thread_ui(); + } + } + + pub(super) fn clear_side_parent_action_status(&mut self, parent_thread_id: ThreadId) { + let mut changed = false; + for state in self + .side_threads + .values_mut() + .filter(|state| state.parent_thread_id == parent_thread_id) + { + if state + .parent_status + .is_some_and(SideParentStatus::is_actionable) + { + state.parent_status = None; + changed = true; + } + } + if changed { + self.sync_side_thread_ui(); + } + } + + pub(super) fn apply_side_parent_status_change( + &mut self, + parent_thread_id: ThreadId, + change: SideParentStatusChange, + ) { + match change { + SideParentStatusChange::Set(status) => { + self.set_side_parent_status(parent_thread_id, Some(status)); + } + SideParentStatusChange::Clear => { + self.set_side_parent_status(parent_thread_id, /*status*/ None); + } + SideParentStatusChange::ClearActionable => { + self.clear_side_parent_action_status(parent_thread_id); + } + } + } + pub(super) async fn maybe_return_from_side( &mut self, tui: &mut tui::Tui, @@ -299,11 +457,18 @@ impl App { self.select_agent_thread(tui, app_server, thread_id).await?; if self.active_thread_id == Some(thread_id) && let Some(side_thread_id) = side_thread_to_discard - && !self.discard_side_thread(app_server, side_thread_id).await - && active_thread_id_before_switch == Some(side_thread_id) { - self.keep_side_thread_visible_after_cleanup_failure(tui, app_server, side_thread_id) + if self.discard_side_thread(app_server, side_thread_id).await { + self.surface_pending_inactive_thread_interactive_requests() + .await; + } else if active_thread_id_before_switch == Some(side_thread_id) { + self.keep_side_thread_visible_after_cleanup_failure( + tui, + app_server, + side_thread_id, + ) .await; + } } Ok(()) } @@ -340,7 +505,7 @@ impl App { Self::install_side_thread_snapshot(&mut store, forked.session, forked.turns); } self.side_threads - .insert(child_thread_id, SideThreadState { parent_thread_id }); + .insert(child_thread_id, SideThreadState::new(parent_thread_id)); if let Err(err) = app_server .thread_inject_items(child_thread_id, vec![Self::side_boundary_prompt_item()]) .await diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9fe233044..91f9cbc63 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1345,6 +1345,8 @@ pub(crate) enum ReplayKind { #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum SessionConfiguredDisplay { Normal, + /// Apply session state without emitting the session info cell. + Quiet, SideConversation, } @@ -2176,6 +2178,14 @@ impl ChatWidget { self.on_session_configured(thread_session_state_to_legacy_event(session)); } + pub(crate) fn handle_thread_session_quiet(&mut self, session: ThreadSessionState) { + self.instruction_source_paths = session.instruction_source_paths.clone(); + self.on_session_configured_with_display( + thread_session_state_to_legacy_event(session), + SessionConfiguredDisplay::Quiet, + ); + } + pub(crate) fn handle_side_thread_session(&mut self, session: ThreadSessionState) { self.instruction_source_paths = session.instruction_source_paths.clone(); self.on_session_configured_with_display( diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap new file mode 100644 index 000000000..0dd45297b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/side.rs +expression: terminal.backend() +--- +" " +" " +"› Check recently modified functions for compatibility " +" " +" gpt-5.3-codex defa… Side from main thread · main needs input · Esc to return " diff --git a/codex-rs/tui/src/chatwidget/tests/side.rs b/codex-rs/tui/src/chatwidget/tests/side.rs index 5ad250d36..84f8b027e 100644 --- a/codex-rs/tui/src/chatwidget/tests/side.rs +++ b/codex-rs/tui/src/chatwidget/tests/side.rs @@ -290,3 +290,21 @@ async fn side_context_label_preserves_status_line_snapshot() { terminal.backend() ); } + +#[tokio::test] +async fn side_context_label_shows_parent_status_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.show_welcome_banner = false; + chat.set_side_conversation_active(/*active*/ true); + chat.set_side_conversation_context_label(Some( + "Side from main thread · main needs input · Esc to return".to_string(), + )); + + let width = 80; + let height = chat.desired_height(width); + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("create terminal"); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw side conversation footer"); + assert_chatwidget_snapshot!("side_context_label_shows_parent_status", terminal.backend()); +}