Display pending child-thread approvals in TUI (#12767)

Summary
- propagate approval policy from parent to spawned agents and drop the
Never override so sub-agents respect the caller’s request
- refresh the pending-approval list whenever events arrive or the active
thread changes and surface the list above the composer for inactive
threads
- add widgets, helpers, and tests covering the new pending-thread
approval UI state

![Uploading Screenshot 2026-02-25 at 11.02.18.png…]()
This commit is contained in:
jif-oai 2026-02-25 11:40:11 +00:00 committed by GitHub
parent 93efcfd50d
commit bcd6e68054
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 419 additions and 12 deletions

View file

@ -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

View file

@ -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<Mutex<ThreadEventStore>>)> = 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 =

View file

@ -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);
}
}

View file

@ -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,

View file

@ -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<i64>,
context_window_used_tokens: Option<i64>,
}
@ -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<String>) {
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();

View file

@ -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<String>,
}
impl PendingThreadApprovals {
pub(crate) fn new() -> Self {
Self {
threads: Vec::new(),
}
}
pub(crate) fn set_threads(&mut self, threads: Vec<String>) -> 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<dyn Renderable> {
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::<String>()
})
.collect::<Vec<_>>()
.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................"
);
}
}

View file

@ -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<String>) {
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();
}