diff --git a/codex-rs/core/src/tools/handlers/multi_agents.rs b/codex-rs/core/src/tools/handlers/multi_agents.rs index d9916a300..9000e6d23 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents.rs @@ -4,7 +4,6 @@ use crate::agent::max_thread_spawn_depth; use crate::codex::Session; use crate::codex::TurnContext; use crate::config::Config; -use crate::config::Constrained; use crate::error::CodexErr; use crate::features::Feature; use crate::function_tool::FunctionCallError; @@ -18,7 +17,6 @@ use async_trait::async_trait; use codex_protocol::ThreadId; use codex_protocol::models::BaseInstructions; use codex_protocol::models::FunctionCallOutputBody; -use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::CollabAgentInteractionBeginEvent; use codex_protocol::protocol::CollabAgentInteractionEndEvent; use codex_protocol::protocol::CollabAgentRef; @@ -937,6 +935,13 @@ fn apply_spawn_agent_runtime_overrides( config: &mut Config, turn: &TurnContext, ) -> Result<(), FunctionCallError> { + config + .permissions + .approval_policy + .set(turn.approval_policy.value()) + .map_err(|err| { + FunctionCallError::RespondToModel(format!("approval_policy is invalid: {err}")) + })?; config.permissions.shell_environment_policy = turn.shell_environment_policy.clone(); config.codex_linux_sandbox_exe = turn.codex_linux_sandbox_exe.clone(); config.cwd = turn.cwd.clone(); @@ -951,7 +956,6 @@ fn apply_spawn_agent_runtime_overrides( } fn apply_spawn_agent_overrides(config: &mut Config, child_depth: i32) { - config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never); let max_depth = max_thread_spawn_depth(config.agent_max_spawn_depth); if exceeds_thread_spawn_depth_limit(child_depth + 1, max_depth) { config.features.disable(Feature::Collab); @@ -1104,8 +1108,7 @@ mod tests { } #[tokio::test] - #[ignore = "No role requiring it for now"] - async fn spawn_agent_uses_explorer_role_and_sets_never_approval_policy() { + async fn spawn_agent_uses_explorer_role_and_preserves_approval_policy() { #[derive(Debug, Deserialize)] struct SpawnAgentResult { agent_id: String, @@ -1121,6 +1124,9 @@ mod tests { .approval_policy .set(AskForApproval::OnRequest) .expect("approval policy should be set"); + turn.approval_policy + .set(AskForApproval::OnRequest) + .expect("approval policy should be set"); turn.config = Arc::new(config); let invocation = invocation( @@ -1158,8 +1164,7 @@ mod tests { .expect("spawned agent thread should exist") .config_snapshot() .await; - assert_eq!(snapshot.model, "gpt-5.1-codex-mini"); - assert_eq!(snapshot.approval_policy, AskForApproval::Never); + assert_eq!(snapshot.approval_policy, AskForApproval::OnRequest); } #[tokio::test] @@ -1210,6 +1215,9 @@ mod tests { &turn.config.permissions.sandbox_policy, turn.config.permissions.sandbox_policy.get().clone(), ); + turn.approval_policy + .set(AskForApproval::OnRequest) + .expect("approval policy should be set"); turn.sandbox_policy .set(expected_sandbox.clone()) .expect("sandbox policy should be set"); @@ -1256,7 +1264,7 @@ mod tests { .config_snapshot() .await; assert_eq!(snapshot.sandbox_policy, expected_sandbox); - assert_eq!(snapshot.approval_policy, AskForApproval::Never); + assert_eq!(snapshot.approval_policy, AskForApproval::OnRequest); } #[tokio::test] @@ -2039,6 +2047,9 @@ mod tests { turn.sandbox_policy .set(sandbox_policy) .expect("sandbox policy set"); + turn.approval_policy + .set(AskForApproval::OnRequest) + .expect("approval policy set"); let config = build_agent_spawn_config(&base_instructions, &turn, 0).expect("spawn config"); let mut expected = (*turn.config).clone(); @@ -2055,7 +2066,7 @@ mod tests { expected .permissions .approval_policy - .set(AskForApproval::Never) + .set(AskForApproval::OnRequest) .expect("approval policy set"); expected .permissions @@ -2087,6 +2098,9 @@ mod tests { let mut base_config = (*turn.config).clone(); base_config.base_instructions = Some("caller-base".to_string()); turn.config = Arc::new(base_config); + turn.approval_policy + .set(AskForApproval::OnRequest) + .expect("approval policy set"); let config = build_agent_resume_config(&turn, 0).expect("resume config"); @@ -2104,7 +2118,7 @@ mod tests { expected .permissions .approval_policy - .set(AskForApproval::Never) + .set(AskForApproval::OnRequest) .expect("approval policy set"); expected .permissions diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 62002c55c..dd746ad24 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -347,6 +347,15 @@ impl ThreadEventStore { fn op_can_change_pending_replay_state(op: &Op) -> bool { PendingInteractiveReplayState::op_can_change_state(op) } + + fn event_can_change_pending_thread_approvals(event: &Event) -> bool { + PendingInteractiveReplayState::event_can_change_pending_thread_approvals(event) + } + + fn has_pending_thread_approvals(&self) -> bool { + self.pending_interactive_replay + .has_pending_thread_approvals() + } } #[derive(Debug)] @@ -812,6 +821,7 @@ impl App { }; self.active_thread_id = Some(thread_id); self.active_thread_rx = receiver; + self.refresh_pending_thread_approvals().await; } async fn store_active_thread_receiver(&mut self) { @@ -845,6 +855,7 @@ impl App { self.set_thread_active(active_id, false).await; } self.active_thread_rx = None; + self.refresh_pending_thread_approvals().await; } async fn note_active_thread_outbound_op(&mut self, op: &Op) { @@ -861,7 +872,63 @@ impl App { store.note_outbound_op(op); } + async fn refresh_pending_thread_approvals(&mut self) { + let channels: Vec<(ThreadId, Arc>)> = self + .thread_event_channels + .iter() + .map(|(thread_id, channel)| (*thread_id, Arc::clone(&channel.store))) + .collect(); + + let mut pending_thread_ids = Vec::new(); + for (thread_id, store) in channels { + if Some(thread_id) == self.active_thread_id { + continue; + } + + let store = store.lock().await; + if store.has_pending_thread_approvals() { + pending_thread_ids.push(thread_id); + } + } + + pending_thread_ids.sort_by_key(ThreadId::to_string); + + let threads = pending_thread_ids + .into_iter() + .map(|thread_id| { + let is_primary = self.primary_thread_id == Some(thread_id); + let fallback_label = if is_primary { + "Main [default]".to_string() + } else { + let thread_id = thread_id.to_string(); + let short_id: String = thread_id.chars().take(8).collect(); + format!("Agent ({short_id})") + }; + if let Some(entry) = self.agent_picker_threads.get(&thread_id) { + let label = format_agent_picker_item_name( + entry.agent_nickname.as_deref(), + entry.agent_role.as_deref(), + is_primary, + ); + if label == "Agent" { + let thread_id = thread_id.to_string(); + let short_id: String = thread_id.chars().take(8).collect(); + format!("{label} ({short_id})") + } else { + label + } + } else { + fallback_label + } + }) + .collect(); + + self.chat_widget.set_pending_thread_approvals(threads); + } + async fn enqueue_thread_event(&mut self, thread_id: ThreadId, event: Event) -> Result<()> { + let refresh_pending_thread_approvals = + ThreadEventStore::event_can_change_pending_thread_approvals(&event); let (sender, store) = { let channel = self.ensure_thread_channel(thread_id); (channel.sender.clone(), Arc::clone(&channel.store)) @@ -891,6 +958,9 @@ impl App { } } } + if refresh_pending_thread_approvals { + self.refresh_pending_thread_approvals().await; + } Ok(()) } @@ -1069,6 +1139,7 @@ impl App { ); } self.drain_active_thread_events(tui).await?; + self.refresh_pending_thread_approvals().await; Ok(()) } @@ -1092,6 +1163,7 @@ impl App { self.active_thread_rx = None; self.primary_thread_id = None; self.pending_primary_events.clear(); + self.chat_widget.set_pending_thread_approvals(Vec::new()); } async fn start_fresh_session_with_summary_hint(&mut self, tui: &mut tui::Tui) { @@ -1869,6 +1941,7 @@ impl App { let submitted = self.chat_widget.submit_op(op); if submitted && let Some(op) = replay_state_op.as_ref() { self.note_active_thread_outbound_op(op).await; + self.refresh_pending_thread_approvals().await; } } AppEvent::DiffResult(text) => { @@ -2615,6 +2688,9 @@ impl App { AppEvent::OpenAgentPicker => { self.open_agent_picker().await; } + AppEvent::RefreshPendingThreadApprovals => { + self.refresh_pending_thread_approvals().await; + } AppEvent::SelectAgentThread(thread_id) => { self.select_agent_thread(tui, thread_id).await?; } @@ -2954,6 +3030,7 @@ impl App { ThreadEventChannel::new_with_session_configured(THREAD_EVENT_CHANNEL_CAPACITY, event); let sender = channel.sender.clone(); let store = Arc::clone(&channel.store); + let app_event_tx = self.app_event_tx.clone(); self.thread_event_channels.insert(thread_id, channel); tokio::spawn(async move { loop { @@ -2964,11 +3041,16 @@ impl App { break; } }; + let refresh_pending_thread_approvals = + ThreadEventStore::event_can_change_pending_thread_approvals(&event); let should_send = { let mut guard = store.lock().await; guard.push_event(event.clone()); guard.active }; + if refresh_pending_thread_approvals { + app_event_tx.send(AppEvent::RefreshPendingThreadApprovals); + } if should_send && let Err(err) = sender.send(event).await { tracing::debug!("external thread {thread_id} channel closed: {err}"); break; @@ -3451,6 +3533,63 @@ mod tests { Ok(()) } + #[tokio::test] + async fn refresh_pending_thread_approvals_only_lists_inactive_threads() { + let mut app = make_test_app().await; + let main_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000001").expect("valid thread"); + let agent_thread_id = + ThreadId::from_string("00000000-0000-0000-0000-000000000002").expect("valid thread"); + + app.primary_thread_id = Some(main_thread_id); + app.active_thread_id = Some(main_thread_id); + app.thread_event_channels + .insert(main_thread_id, ThreadEventChannel::new(1)); + + let agent_channel = ThreadEventChannel::new(1); + { + let mut store = agent_channel.store.lock().await; + store.push_event(Event { + id: "ev-1".to_string(), + msg: EventMsg::ExecApprovalRequest( + codex_protocol::protocol::ExecApprovalRequestEvent { + call_id: "call-1".to_string(), + approval_id: None, + turn_id: "turn-1".to_string(), + command: vec!["echo".to_string(), "hi".to_string()], + cwd: PathBuf::from("/tmp"), + reason: None, + network_approval_context: None, + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, + additional_permissions: None, + parsed_cmd: Vec::new(), + }, + ), + }); + } + app.thread_event_channels + .insert(agent_thread_id, agent_channel); + app.agent_picker_threads.insert( + agent_thread_id, + AgentPickerThreadEntry { + agent_nickname: Some("Robie".to_string()), + agent_role: Some("explorer".to_string()), + is_closed: false, + }, + ); + + app.refresh_pending_thread_approvals().await; + assert_eq!( + app.chat_widget.pending_thread_approvals(), + &["Robie [explorer]".to_string()] + ); + + app.active_thread_id = Some(agent_thread_id); + app.refresh_pending_thread_approvals().await; + assert!(app.chat_widget.pending_thread_approvals().is_empty()); + } + #[test] fn agent_picker_item_name_snapshot() { let thread_id = diff --git a/codex-rs/tui/src/app/pending_interactive_replay.rs b/codex-rs/tui/src/app/pending_interactive_replay.rs index 14d9f13c7..3d3091478 100644 --- a/codex-rs/tui/src/app/pending_interactive_replay.rs +++ b/codex-rs/tui/src/app/pending_interactive_replay.rs @@ -44,6 +44,20 @@ pub(super) struct PendingInteractiveReplayState { } impl PendingInteractiveReplayState { + pub(super) fn event_can_change_pending_thread_approvals(event: &Event) -> bool { + matches!( + &event.msg, + EventMsg::ExecApprovalRequest(_) + | EventMsg::ApplyPatchApprovalRequest(_) + | EventMsg::ElicitationRequest(_) + | EventMsg::ExecCommandBegin(_) + | EventMsg::PatchApplyBegin(_) + | EventMsg::TurnComplete(_) + | EventMsg::TurnAborted(_) + | EventMsg::ShutdownComplete + ) + } + pub(super) fn op_can_change_state(op: &Op) -> bool { matches!( op, @@ -240,6 +254,12 @@ impl PendingInteractiveReplayState { } } + pub(super) fn has_pending_thread_approvals(&self) -> bool { + !self.exec_approval_call_ids.is_empty() + || !self.patch_approval_call_ids.is_empty() + || !self.elicitation_requests.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 { @@ -582,4 +602,56 @@ mod tests { "resolved elicitation prompt should not replay on thread switch" ); } + + #[test] + fn thread_event_store_reports_pending_thread_approvals() { + let mut store = ThreadEventStore::new(8); + assert_eq!(store.has_pending_thread_approvals(), false); + + store.push_event(Event { + id: "ev-1".to_string(), + msg: EventMsg::ExecApprovalRequest( + codex_protocol::protocol::ExecApprovalRequestEvent { + call_id: "call-1".to_string(), + approval_id: None, + turn_id: "turn-1".to_string(), + command: vec!["echo".to_string(), "hi".to_string()], + cwd: PathBuf::from("/tmp"), + reason: None, + network_approval_context: None, + proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, + additional_permissions: None, + parsed_cmd: Vec::new(), + }, + ), + }); + + assert_eq!(store.has_pending_thread_approvals(), true); + + store.note_outbound_op(&Op::ExecApproval { + id: "call-1".to_string(), + turn_id: Some("turn-1".to_string()), + decision: codex_protocol::protocol::ReviewDecision::Approved, + }); + + assert_eq!(store.has_pending_thread_approvals(), false); + } + + #[test] + fn request_user_input_does_not_count_as_pending_thread_approval() { + let mut store = ThreadEventStore::new(8); + store.push_event(Event { + id: "ev-1".to_string(), + msg: EventMsg::RequestUserInput( + codex_protocol::request_user_input::RequestUserInputEvent { + call_id: "call-1".to_string(), + turn_id: "turn-1".to_string(), + questions: Vec::new(), + }, + ), + }); + + assert_eq!(store.has_pending_thread_approvals(), false); + } } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 27ebe8cfe..0767696e2 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -51,6 +51,9 @@ pub(crate) enum AppEvent { /// Switch the active thread to the selected agent. SelectAgentThread(ThreadId), + /// Recompute the list of inactive threads that still need approval. + RefreshPendingThreadApprovals, + /// Start a new session. NewSession, diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e4c40dd45..3202a6162 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -17,6 +17,7 @@ use std::path::PathBuf; use crate::app_event::ConnectorsSnapshot; use crate::app_event_sender::AppEventSender; +use crate::bottom_pane::pending_thread_approvals::PendingThreadApprovals; use crate::bottom_pane::queued_user_messages::QueuedUserMessages; use crate::bottom_pane::unified_exec_footer::UnifiedExecFooter; use crate::key_hint; @@ -92,6 +93,7 @@ pub(crate) use skills_toggle_view::SkillsToggleView; pub(crate) use status_line_setup::StatusLineItem; pub(crate) use status_line_setup::StatusLineSetupView; mod paste_burst; +mod pending_thread_approvals; pub mod popup_consts; mod queued_user_messages; mod scroll_state; @@ -171,6 +173,8 @@ pub(crate) struct BottomPane { unified_exec_footer: UnifiedExecFooter, /// Queued user messages to show above the composer while a turn is running. queued_user_messages: QueuedUserMessages, + /// Inactive threads with pending approval requests. + pending_thread_approvals: PendingThreadApprovals, context_window_percent: Option, context_window_used_tokens: Option, } @@ -219,6 +223,7 @@ impl BottomPane { status: None, unified_exec_footer: UnifiedExecFooter::new(), queued_user_messages: QueuedUserMessages::new(), + pending_thread_approvals: PendingThreadApprovals::new(), esc_backtrack_hint: false, animations_enabled, context_window_percent: None, @@ -759,6 +764,18 @@ impl BottomPane { self.request_redraw(); } + /// Update the inactive-thread approval list shown above the composer. + pub(crate) fn set_pending_thread_approvals(&mut self, threads: Vec) { + if self.pending_thread_approvals.set_threads(threads) { + self.request_redraw(); + } + } + + #[cfg(test)] + pub(crate) fn pending_thread_approvals(&self) -> &[String] { + self.pending_thread_approvals.threads() + } + /// Update the unified-exec process set and refresh whichever summary surface is active. /// /// The summary may be displayed inline in the status row or as a dedicated @@ -980,14 +997,20 @@ impl BottomPane { if self.status.is_none() && !self.unified_exec_footer.is_empty() { flex.push(0, RenderableItem::Borrowed(&self.unified_exec_footer)); } + let has_pending_thread_approvals = !self.pending_thread_approvals.is_empty(); let has_queued_messages = !self.queued_user_messages.messages.is_empty(); let has_status_or_footer = self.status.is_some() || !self.unified_exec_footer.is_empty(); - if has_queued_messages && has_status_or_footer { + let has_inline_previews = has_pending_thread_approvals || has_queued_messages; + if has_inline_previews && has_status_or_footer { + flex.push(0, RenderableItem::Owned("".into())); + } + flex.push(1, RenderableItem::Borrowed(&self.pending_thread_approvals)); + if has_pending_thread_approvals && has_queued_messages { flex.push(0, RenderableItem::Owned("".into())); } flex.push(1, RenderableItem::Borrowed(&self.queued_user_messages)); - if !has_queued_messages && has_status_or_footer { + if !has_inline_previews && has_status_or_footer { flex.push(0, RenderableItem::Owned("".into())); } let mut flex2 = FlexRenderable::new(); diff --git a/codex-rs/tui/src/bottom_pane/pending_thread_approvals.rs b/codex-rs/tui/src/bottom_pane/pending_thread_approvals.rs new file mode 100644 index 000000000..67a66bfa7 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/pending_thread_approvals.rs @@ -0,0 +1,147 @@ +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::widgets::Paragraph; + +use crate::render::renderable::Renderable; +use crate::wrapping::RtOptions; +use crate::wrapping::adaptive_wrap_lines; + +/// Widget that lists inactive threads with outstanding approval requests. +pub(crate) struct PendingThreadApprovals { + threads: Vec, +} + +impl PendingThreadApprovals { + pub(crate) fn new() -> Self { + Self { + threads: Vec::new(), + } + } + + pub(crate) fn set_threads(&mut self, threads: Vec) -> bool { + if self.threads == threads { + return false; + } + self.threads = threads; + true + } + + pub(crate) fn is_empty(&self) -> bool { + self.threads.is_empty() + } + + #[cfg(test)] + pub(crate) fn threads(&self) -> &[String] { + &self.threads + } + + fn as_renderable(&self, width: u16) -> Box { + if self.threads.is_empty() || width < 4 { + return Box::new(()); + } + + let mut lines = Vec::new(); + for thread in self.threads.iter().take(3) { + let wrapped = adaptive_wrap_lines( + std::iter::once(Line::from(format!("Approval needed in {thread}"))), + RtOptions::new(width as usize) + .initial_indent(Line::from(vec![" ".into(), "!".red().bold(), " ".into()])) + .subsequent_indent(Line::from(" ")), + ); + lines.extend(wrapped); + } + + if self.threads.len() > 3 { + lines.push(Line::from(" ...".dim().italic())); + } + + lines.push( + Line::from(vec![ + " ".into(), + "/agent".cyan().bold(), + " to switch threads".dim(), + ]) + .dim(), + ); + + Paragraph::new(lines).into() + } +} + +impl Renderable for PendingThreadApprovals { + fn render(&self, area: Rect, buf: &mut Buffer) { + if area.is_empty() { + return; + } + + self.as_renderable(area.width).render(area, buf); + } + + fn desired_height(&self, width: u16) -> u16 { + self.as_renderable(width).desired_height(width) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use insta::assert_snapshot; + use pretty_assertions::assert_eq; + + fn snapshot_rows(widget: &PendingThreadApprovals, width: u16) -> String { + let height = widget.desired_height(width); + let mut buf = Buffer::empty(Rect::new(0, 0, width, height)); + widget.render(Rect::new(0, 0, width, height), &mut buf); + + (0..height) + .map(|y| { + (0..width) + .map(|x| buf[(x, y)].symbol().chars().next().unwrap_or(' ')) + .collect::() + }) + .collect::>() + .join("\n") + } + + #[test] + fn desired_height_empty() { + let widget = PendingThreadApprovals::new(); + assert_eq!(widget.desired_height(40), 0); + } + + #[test] + fn render_single_thread_snapshot() { + let mut widget = PendingThreadApprovals::new(); + widget.set_threads(vec!["Robie [explorer]".to_string()]); + + assert_snapshot!( + snapshot_rows(&widget, 40).replace(' ', "."), + @r" +..!.Approval.needed.in.Robie.[explorer]. +..../agent.to.switch.threads............" + ); + } + + #[test] + fn render_multiple_threads_snapshot() { + let mut widget = PendingThreadApprovals::new(); + widget.set_threads(vec![ + "Main [default]".to_string(), + "Robie [explorer]".to_string(), + "Inspector".to_string(), + "Extra agent".to_string(), + ]); + + assert_snapshot!( + snapshot_rows(&widget, 44).replace(' ', "."), + @r" +..!.Approval.needed.in.Main.[default]....... +..!.Approval.needed.in.Robie.[explorer]..... +..!.Approval.needed.in.Inspector............ +............................................ +..../agent.to.switch.threads................" + ); + } +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b182d814d..533aa1056 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -4645,6 +4645,10 @@ impl ChatWidget { self.bottom_pane.set_queued_user_messages(messages); } + pub(crate) fn set_pending_thread_approvals(&mut self, threads: Vec) { + self.bottom_pane.set_pending_thread_approvals(threads); + } + pub(crate) fn add_diff_in_progress(&mut self) { self.request_redraw(); } @@ -7329,6 +7333,11 @@ impl ChatWidget { self.bottom_pane.remote_image_urls() } + #[cfg(test)] + pub(crate) fn pending_thread_approvals(&self) -> &[String] { + self.bottom_pane.pending_thread_approvals() + } + pub(crate) fn show_esc_backtrack_hint(&mut self) { self.bottom_pane.show_esc_backtrack_hint(); }