From 48dd7b58f09d35022d9163c720b6cef1e6d77c6a Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 27 Apr 2026 10:07:15 -0700 Subject: [PATCH] Render delegated patch approval details (#19709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Fixes #19632. When a delegated agent requests approval for an in-progress file change, the parent TUI handles that request from an inactive thread. The app server already sent the `FileChange` item with the proposed diff, but the inactive-thread approval path was not recovering and rendering it the same way as the active-thread path. The result was an inconsistent approval prompt: main-thread edits show a normal patch preview history item before the approval modal, while delegated edits did not show that preview in the transcript flow. ## What Changed - Recover buffered or historical `FileChange` item changes when building inactive-thread file-change approval requests. - Reuse the app-server file-change conversion helper for both live transcript rendering and inactive-thread approvals. - Render recovered delegated patches as a normal patch preview history cell before the approval modal. - Keep apply-patch approval modals focused on the decision prompt and optional metadata; they do not render a synthetic command line or embed the diff body. ## Manual Repro And Verification I manually reproduced the issue using a file under `~/Desktop` so the write would require approval. Before the fix: 1. Ask the main thread: `Use apply_patch, not shell redirection or Python, to create ~/Desktop/bug1.txt with three short lines.` 2. Observe the expected TUI shape: the transcript shows a normal patch preview such as `• Added ~/Desktop/bug1.txt (+N -0)` above the approval modal, and the modal contains only the approval prompt/options without a synthetic command line. 3. Ask for the delegated path: `Spawn a worker. Have it use apply_patch, not shell redirection or Python, to create ~/Desktop/bug1.txt with four short lines.` 4. Observe the delegated approval is inconsistent: the parent view does not render the proposed patch as the normal transcript preview before the modal, so the diff context is missing from the stream or appears inside the modal instead of in the history flow. After the fix: 1. Repeat the delegated worker prompt with `apply_patch`. 2. Confirm the parent view renders the same normal patch preview history cell (`• Added ~/Desktop/bug1.txt (+N -0)` plus the diff) immediately before the approval modal. 3. Confirm the approval modal remains focused on the decision prompt. For delegated approvals it may show the worker thread label, but it should not show a `$ apply_patch` command line or embed the diff body in the modal. --- codex-rs/tui/src/app/tests.rs | 74 +++++++++++++++++++ codex-rs/tui/src/app/thread_events.rs | 48 ++++++++++++ codex-rs/tui/src/app/thread_routing.rs | 35 ++++++++- .../src/app_server_approval_conversions.rs | 51 +++++++++++++ .../tui/src/bottom_pane/approval_overlay.rs | 35 +++++++-- codex-rs/tui/src/chatwidget.rs | 35 +-------- ...atwidget__tests__approval_modal_patch.snap | 11 +-- .../tui/src/chatwidget/tests/app_server.rs | 19 ----- .../tui/src/chatwidget/tests/exec_flow.rs | 61 +++------------ 9 files changed, 253 insertions(+), 116 deletions(-) diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index f7d24e1e0..d9d3b0c02 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -30,6 +30,9 @@ use codex_app_server_protocol::AdditionalPermissionProfile; use codex_app_server_protocol::AgentMessageDeltaNotification; use codex_app_server_protocol::CommandExecutionRequestApprovalParams; use codex_app_server_protocol::ConfigWarningNotification; +use codex_app_server_protocol::FileChangeRequestApprovalParams; +use codex_app_server_protocol::FileUpdateChange; +use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::McpServerStartupState; use codex_app_server_protocol::McpServerStatusUpdatedNotification; @@ -38,6 +41,7 @@ use codex_app_server_protocol::NetworkApprovalProtocol as AppServerNetworkApprov use codex_app_server_protocol::NetworkPolicyAmendment as AppServerNetworkPolicyAmendment; use codex_app_server_protocol::NetworkPolicyRuleAction as AppServerNetworkPolicyRuleAction; use codex_app_server_protocol::NonSteerableTurnKind as AppServerNonSteerableTurnKind; +use codex_app_server_protocol::PatchChangeKind; use codex_app_server_protocol::PermissionsRequestApprovalParams; use codex_app_server_protocol::RequestId as AppServerRequestId; use codex_app_server_protocol::ServerNotification; @@ -70,6 +74,7 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::protocol::RolloutItem; @@ -2522,6 +2527,75 @@ async fn inactive_thread_exec_approval_splits_shell_wrapped_command() { ); } +#[tokio::test] +async fn inactive_thread_file_change_approval_recovers_buffered_changes() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + let thread_id = ThreadId::new(); + app.enqueue_thread_notification( + thread_id, + ServerNotification::ItemStarted(ItemStartedNotification { + thread_id: thread_id.to_string(), + turn_id: "turn-approval".to_string(), + item: ThreadItem::FileChange { + id: "patch-approval".to_string(), + changes: vec![FileUpdateChange { + path: "README.md".to_string(), + kind: PatchChangeKind::Add, + diff: "hello\n".to_string(), + }], + status: codex_app_server_protocol::PatchApplyStatus::InProgress, + }, + }), + ) + .await + .expect("enqueue file change item"); + + let request = ServerRequest::FileChangeRequestApproval { + request_id: AppServerRequestId::Integer(9), + params: FileChangeRequestApprovalParams { + thread_id: thread_id.to_string(), + turn_id: "turn-approval".to_string(), + item_id: "patch-approval".to_string(), + reason: Some("command failed; retry without sandbox?".to_string()), + grant_root: None, + }, + }; + + let request = app + .interactive_request_for_thread_request(thread_id, &request) + .await + .expect("expected file change approval request"); + + let ThreadInteractiveRequest::Approval(ApprovalRequest::ApplyPatch { + changes, reason, .. + }) = &request + else { + panic!("expected apply-patch approval request"); + }; + assert_eq!( + changes, + &HashMap::from([( + PathBuf::from("README.md"), + FileChange::Add { + content: "hello\n".to_string(), + }, + )]) + ); + assert_eq!( + reason, + &Some("command failed; retry without sandbox?".to_string()) + ); + + app.push_thread_interactive_request(request); + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected patch preview history cell, saw {other:?}"), + }; + let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); + assert!(rendered.contains("• Added README.md (+1 -0)")); + assert!(rendered.contains("1 +hello")); +} + #[tokio::test] async fn inactive_thread_permissions_approval_preserves_file_system_permissions() { let app = make_test_app().await; diff --git a/codex-rs/tui/src/app/thread_events.rs b/codex-rs/tui/src/app/thread_events.rs index daf743b46..9bbb41fbd 100644 --- a/codex-rs/tui/src/app/thread_events.rs +++ b/codex-rs/tui/src/app/thread_events.rs @@ -157,6 +157,40 @@ impl ThreadEventStore { .collect() } + pub(super) fn file_change_changes( + &self, + turn_id: &str, + item_id: &str, + ) -> Option> { + self.buffer + .iter() + .rev() + .find_map(|event| match event { + ThreadBufferedEvent::Notification(ServerNotification::ItemStarted( + notification, + )) if turn_id_matches(turn_id, ¬ification.turn_id) => { + file_change_item_changes(¬ification.item, item_id) + } + ThreadBufferedEvent::Notification(ServerNotification::ItemCompleted( + notification, + )) if turn_id_matches(turn_id, ¬ification.turn_id) => { + file_change_item_changes(¬ification.item, item_id) + } + ThreadBufferedEvent::Request(_) + | ThreadBufferedEvent::Notification(_) + | ThreadBufferedEvent::HistoryEntryResponse(_) + | ThreadBufferedEvent::FeedbackSubmission(_) => None, + }) + .or_else(|| { + self.turns + .iter() + .rev() + .filter(|turn| turn_id_matches(turn_id, &turn.id)) + .flat_map(|turn| turn.items.iter().rev()) + .find_map(|item| file_change_item_changes(item, item_id)) + }) + } + pub(super) fn apply_thread_rollback(&mut self, response: &ThreadRollbackResponse) { self.turns = response.thread.turns.clone(); self.buffer.clear(); @@ -231,6 +265,20 @@ impl ThreadEventStore { } } +fn turn_id_matches(request_turn_id: &str, candidate_turn_id: &str) -> bool { + request_turn_id.is_empty() || request_turn_id == candidate_turn_id +} + +fn file_change_item_changes( + item: &ThreadItem, + item_id: &str, +) -> Option> { + match item { + ThreadItem::FileChange { id, changes, .. } if id == item_id => Some(changes.clone()), + _ => None, + } +} + #[derive(Debug)] pub(super) struct ThreadEventChannel { pub(super) sender: mpsc::Sender, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 5f0f52c2c..345e560d4 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -194,6 +194,17 @@ impl App { store.session.as_ref().map(|session| session.cwd.clone()) } + async fn thread_file_change_changes( + &self, + thread_id: ThreadId, + turn_id: &str, + item_id: &str, + ) -> Option> { + let channel = self.thread_event_channels.get(&thread_id)?; + let store = channel.store.lock().await; + store.file_change_changes(turn_id, item_id) + } + pub(super) async fn interactive_request_for_thread_request( &self, thread_id: ThreadId, @@ -264,7 +275,11 @@ impl App { .thread_cwd(thread_id) .await .unwrap_or_else(|| self.config.cwd.clone()), - changes: HashMap::new(), + changes: self + .thread_file_change_changes(thread_id, ¶ms.turn_id, ¶ms.item_id) + .await + .map(crate::app_server_approval_conversions::file_update_changes_to_core) + .unwrap_or_default(), }), ), ServerRequest::McpServerElicitationRequest { request_id, params } => { @@ -311,6 +326,7 @@ impl App { pub(super) fn push_thread_interactive_request(&mut self, request: ThreadInteractiveRequest) { match request { ThreadInteractiveRequest::Approval(request) => { + self.render_inactive_patch_preview(&request); self.chat_widget.push_approval_request(request); } ThreadInteractiveRequest::McpServerElicitation(request) => { @@ -320,6 +336,23 @@ impl App { } } + fn render_inactive_patch_preview(&mut self, request: &ApprovalRequest) { + let ApprovalRequest::ApplyPatch { + thread_label, + cwd, + changes, + .. + } = request + else { + return; + }; + if thread_label.is_none() || changes.is_empty() { + return; + } + self.chat_widget + .add_to_history(history_cell::new_patch_event(changes.clone(), cwd)); + } + pub(super) async fn pending_inactive_thread_requests(&self) -> Vec<(ThreadId, ServerRequest)> { let channels: Vec<(ThreadId, Arc>)> = self .thread_event_channels diff --git a/codex-rs/tui/src/app_server_approval_conversions.rs b/codex-rs/tui/src/app_server_approval_conversions.rs index a0d86db7d..894bd36ed 100644 --- a/codex-rs/tui/src/app_server_approval_conversions.rs +++ b/codex-rs/tui/src/app_server_approval_conversions.rs @@ -1,9 +1,14 @@ use codex_app_server_protocol::AdditionalNetworkPermissions; +use codex_app_server_protocol::FileUpdateChange; use codex_app_server_protocol::GrantedPermissionProfile; use codex_app_server_protocol::NetworkApprovalContext as AppServerNetworkApprovalContext; +use codex_app_server_protocol::PatchChangeKind; +use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; +use std::collections::HashMap; +use std::path::PathBuf; pub(crate) fn network_approval_context_to_core( value: AppServerNetworkApprovalContext, @@ -38,21 +43,50 @@ pub(crate) fn granted_permission_profile_from_request( } } +pub(crate) fn file_update_changes_to_core( + changes: Vec, +) -> HashMap { + changes + .into_iter() + .map(|change| { + let path = PathBuf::from(change.path); + let file_change = match change.kind { + PatchChangeKind::Add => FileChange::Add { + content: change.diff, + }, + PatchChangeKind::Delete => FileChange::Delete { + content: change.diff, + }, + PatchChangeKind::Update { move_path } => FileChange::Update { + unified_diff: change.diff, + move_path, + }, + }; + (path, file_change) + }) + .collect() +} + #[cfg(test)] mod tests { + use super::file_update_changes_to_core; use super::granted_permission_profile_from_request; use super::network_approval_context_to_core; + use codex_app_server_protocol::FileUpdateChange; + use codex_app_server_protocol::PatchChangeKind; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::NetworkPermissions; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxEntry; use codex_protocol::permissions::FileSystemSpecialPath; + use codex_protocol::protocol::FileChange; use codex_protocol::protocol::NetworkApprovalContext; use codex_protocol::protocol::NetworkApprovalProtocol; use codex_protocol::request_permissions::RequestPermissionProfile as CoreRequestPermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::collections::HashMap; use std::path::PathBuf; fn absolute_path(path: &str) -> AbsolutePathBuf { @@ -73,6 +107,23 @@ mod tests { ); } + #[test] + fn converts_file_update_changes_to_core() { + assert_eq!( + file_update_changes_to_core(vec![FileUpdateChange { + path: "foo.txt".to_string(), + kind: PatchChangeKind::Add, + diff: "hello\n".to_string(), + }]), + HashMap::from([( + PathBuf::from("foo.txt"), + FileChange::Add { + content: "hello\n".to_string(), + }, + )]) + ); + } + #[test] fn converts_request_permissions_into_granted_permissions() { assert_eq!( diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 57d819e56..ce3b04b33 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -9,7 +9,6 @@ use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::bottom_pane::list_selection_view::SelectionItem; use crate::bottom_pane::list_selection_view::SelectionViewParams; -use crate::diff_render::DiffSummary; use crate::exec_command::strip_bash_lc_and_escape; use crate::history_cell; use crate::key_hint; @@ -633,8 +632,6 @@ fn build_header(request: &ApprovalRequest) -> Box { ApprovalRequest::ApplyPatch { thread_label, reason, - cwd, - changes, .. } => { let mut header: Vec> = Vec::new(); @@ -643,11 +640,13 @@ fn build_header(request: &ApprovalRequest) -> Box { "Thread: ".into(), thread_label.clone().bold(), ]))); - header.push(Box::new(Line::from(""))); } if let Some(reason) = reason && !reason.is_empty() { + if !header.is_empty() { + header.push(Box::new(Line::from(""))); + } header.push(Box::new( Paragraph::new(Line::from_iter([ "Reason: ".into(), @@ -655,9 +654,7 @@ fn build_header(request: &ApprovalRequest) -> Box { ])) .wrap(Wrap { trim: false }), )); - header.push(Box::new(Line::from(""))); } - header.push(DiffSummary::new(changes.clone(), cwd.clone()).into()); Box::new(ColumnRenderable::with(header)) } ApprovalRequest::McpElicitation { @@ -1556,6 +1553,32 @@ mod tests { ); } + #[test] + fn apply_patch_prompt_with_thread_label_omits_command_line() { + let (tx, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let mut changes = HashMap::new(); + changes.insert( + PathBuf::from("bug1.txt"), + FileChange::Add { + content: "one\ntwo\nthree\n".to_string(), + }, + ); + let request = ApprovalRequest::ApplyPatch { + thread_id: ThreadId::new(), + thread_label: Some("Banach [worker]".to_string()), + id: "test".to_string(), + reason: None, + cwd: absolute_path("/tmp"), + changes, + }; + let view = ApprovalOverlay::new(request, tx, Features::with_defaults()); + let rendered = render_overlay_lines(&view, /*width*/ 120); + assert!(rendered.contains("Thread: Banach [worker]")); + assert!(rendered.contains("o to open thread")); + assert!(!rendered.contains("$ apply_patch")); + } + #[test] fn network_exec_prompt_title_includes_host() { let (tx, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index f5454c2bf..8513fe881 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -47,6 +47,7 @@ use self::realtime::PendingSteerCompareKey; use crate::app::app_server_requests::ResolvedAppServerRequest; use crate::app_command::AppCommand; use crate::app_event::RealtimeAudioDeviceKind; +use crate::app_server_approval_conversions::file_update_changes_to_core; use crate::app_server_approval_conversions::network_approval_context_to_core; use crate::app_server_session::ThreadSessionState; #[cfg(not(target_os = "linux"))] @@ -1774,36 +1775,6 @@ fn patch_approval_request_from_params( } } -fn app_server_patch_changes_to_core( - changes: Vec, -) -> HashMap { - changes - .into_iter() - .map(|change| { - let path = PathBuf::from(change.path); - let file_change = match change.kind { - codex_app_server_protocol::PatchChangeKind::Add => { - codex_protocol::protocol::FileChange::Add { - content: change.diff, - } - } - codex_app_server_protocol::PatchChangeKind::Delete => { - codex_protocol::protocol::FileChange::Delete { - content: change.diff, - } - } - codex_app_server_protocol::PatchChangeKind::Update { move_path } => { - codex_protocol::protocol::FileChange::Update { - unified_diff: change.diff, - move_path, - } - } - }; - (path, file_change) - }) - .collect() -} - fn app_server_collab_thread_id_to_core(thread_id: &str) -> Option { match ThreadId::from_string(thread_id) { Ok(thread_id) => Some(thread_id), @@ -6797,7 +6768,7 @@ impl ChatWidget { status, codex_app_server_protocol::PatchApplyStatus::Failed ), - changes: app_server_patch_changes_to_core(changes), + changes: file_update_changes_to_core(changes), status: match status { codex_app_server_protocol::PatchApplyStatus::Completed => { codex_protocol::protocol::PatchApplyStatus::Completed @@ -7356,7 +7327,7 @@ impl ChatWidget { call_id: id, turn_id: notification.turn_id, auto_approved: false, - changes: app_server_patch_changes_to_core(changes), + changes: file_update_changes_to_core(changes), }); } ThreadItem::McpToolCall { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap index e394605dc..8635b6668 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_patch.snap @@ -1,18 +1,11 @@ --- -source: tui/src/chatwidget/tests.rs -expression: terminal.backend().vt100().screen().contents() +source: tui/src/chatwidget/tests/exec_flow.rs +expression: contents --- - - Would you like to make the following edits? Reason: The model wants to apply changes - README.md (+2 -0) - - 1 +hello - 2 +world - › 1. Yes, proceed (y) 2. Yes, and don't ask again for these files (a) 3. No, and tell Codex what to do differently (esc) diff --git a/codex-rs/tui/src/chatwidget/tests/app_server.rs b/codex-rs/tui/src/chatwidget/tests/app_server.rs index b9ec1d871..d4fa2df45 100644 --- a/codex-rs/tui/src/chatwidget/tests/app_server.rs +++ b/codex-rs/tui/src/chatwidget/tests/app_server.rs @@ -349,25 +349,6 @@ async fn live_app_server_command_execution_strips_shell_wrapper() { ); } -#[test] -fn app_server_patch_changes_to_core_preserves_diffs() { - let changes = app_server_patch_changes_to_core(vec![FileUpdateChange { - path: "foo.txt".to_string(), - kind: PatchChangeKind::Add, - diff: "hello\n".to_string(), - }]); - - assert_eq!( - changes, - HashMap::from([( - PathBuf::from("foo.txt"), - FileChange::Add { - content: "hello\n".to_string(), - }, - )]) - ); -} - #[tokio::test] async fn live_app_server_collab_wait_items_render_history() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs index 84d7cd721..4d87914aa 100644 --- a/codex-rs/tui/src/chatwidget/tests/exec_flow.rs +++ b/codex-rs/tui/src/chatwidget/tests/exec_flow.rs @@ -1321,10 +1321,9 @@ async fn approval_modal_patch_snapshot() -> anyhow::Result<()> { terminal .draw(|f| chat.render(f.area(), f.buffer_mut())) .expect("draw patch approval modal"); - assert_chatwidget_snapshot!( - "approval_modal_patch", - terminal.backend().vt100().screen().contents() - ); + let contents = terminal.backend().vt100().screen().contents(); + assert!(!contents.contains("$ apply_patch")); + assert_chatwidget_snapshot!("approval_modal_patch", contents); Ok(()) } @@ -1469,28 +1468,11 @@ async fn apply_patch_events_emit_history_cells() { id: "s1".into(), msg: EventMsg::ApplyPatchApprovalRequest(ev), }); - let cells = drain_insert_history(&mut rx); assert!( - cells.is_empty(), + drain_insert_history(&mut rx).is_empty(), "expected approval request to surface via modal without emitting history cells" ); - let area = Rect::new(0, 0, 80, chat.desired_height(/*width*/ 80)); - let mut buf = ratatui::buffer::Buffer::empty(area); - chat.render(area, &mut buf); - let mut saw_summary = false; - for y in 0..area.height { - let mut row = String::new(); - for x in 0..area.width { - row.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); - } - if row.contains("foo.txt (+1 -0)") { - saw_summary = true; - break; - } - } - assert!(saw_summary, "expected approval modal to show diff summary"); - // 2) Begin apply -> per-file apply block cell (no global header) let mut changes2 = HashMap::new(); changes2.insert( @@ -1820,7 +1802,7 @@ async fn apply_patch_untrusted_shows_approval_modal() -> anyhow::Result<()> { } #[tokio::test] -async fn apply_patch_request_shows_diff_summary() -> anyhow::Result<()> { +async fn apply_patch_request_omits_diff_summary_from_modal() -> anyhow::Result<()> { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; // Ensure we are in OnRequest so an approval is surfaced @@ -1849,43 +1831,24 @@ async fn apply_patch_request_shows_diff_summary() -> anyhow::Result<()> { }), }); - // No history entries yet; the modal should contain the diff summary - let cells = drain_insert_history(&mut rx); assert!( - cells.is_empty(), + drain_insert_history(&mut rx).is_empty(), "expected approval request to render via modal instead of history" ); let area = Rect::new(0, 0, 80, chat.desired_height(/*width*/ 80)); let mut buf = ratatui::buffer::Buffer::empty(area); chat.render(area, &mut buf); - - let mut saw_header = false; - let mut saw_line1 = false; - let mut saw_line2 = false; + let mut contents = String::new(); for y in 0..area.height { - let mut row = String::new(); for x in 0..area.width { - row.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); - } - if row.contains("README.md (+2 -0)") { - saw_header = true; - } - if row.contains("+line one") { - saw_line1 = true; - } - if row.contains("+line two") { - saw_line2 = true; - } - if saw_header && saw_line1 && saw_line2 { - break; + contents.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); } + contents.push('\n'); } - assert!(saw_header, "expected modal to show diff header with totals"); - assert!( - saw_line1 && saw_line2, - "expected modal to show per-line diff summary" - ); + assert!(!contents.contains("README.md (+2 -0)")); + assert!(!contents.contains("+line one")); + assert!(!contents.contains("+line two")); Ok(()) }