From d38b5260a1b8af0884bfe1abdfd66c92c60ff510 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 14 Sep 2026 19:07:03 +0000 Subject: [PATCH] Send local TUI images as portable attachments to remote app servers (#45499) ## Why Remote app servers cannot read image paths on the TUI host. Image attachments need to carry their contents when submitting to a remote workspace. ## What changed - Prepare local images as data URLs off the event loop for new turns and steers, preserving source pixels for model-specific resizing and enforcing a 32 MiB image transport budget. - Show preparation progress, preserve queued message order, and recover drafts on preparation failures, cancellation, disconnects, or thread switches. Ignore stale preparation completions. - Avoid duplicate user messages from server receipts and duplicate image labels in history. ## Testing Add regression tests for portable image contents, receipt deduplication, draft restoration, responsive input during preparation, and cancellation across thread and connection changes. GitOrigin-RevId: 6404615eae0b06c141af96b4229501c61f7fcfee --- codex-rs/tui/src/app/event_dispatch.rs | 3 + codex-rs/tui/src/app/reconnect.rs | 1 + codex-rs/tui/src/app/session_lifecycle.rs | 1 + codex-rs/tui/src/app/startup.rs | 1 + codex-rs/tui/src/app_event.rs | 3 + codex-rs/tui/src/chatwidget.rs | 16 + codex-rs/tui/src/chatwidget/constructor.rs | 3 + .../tui/src/chatwidget/image_submission.rs | 169 +++++++++ codex-rs/tui/src/chatwidget/input_flow.rs | 12 +- codex-rs/tui/src/chatwidget/input_restore.rs | 2 + .../tui/src/chatwidget/input_submission.rs | 95 ++++- codex-rs/tui/src/chatwidget/interaction.rs | 22 +- codex-rs/tui/src/chatwidget/reconnect.rs | 1 + ...tests__image_preparation_disconnected.snap | 14 + ..._tests__image_preparation_in_progress.snap | 12 + ...sts__image_preparation_restored_input.snap | 9 + ...sts__portable_image_echo_renders_once.snap | 5 + .../chatwidget/tests/composer_submission.rs | 351 ++++++++++++++++++ codex-rs/tui/src/history_cell/messages.rs | 34 +- ...ell__tests__portable_image_raw_output.snap | 5 + codex-rs/tui/src/history_cell/tests.rs | 12 +- 21 files changed, 731 insertions(+), 40 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/image_submission.rs create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_disconnected.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_in_progress.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_restored_input.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__portable_image_echo_renders_once.snap create mode 100644 codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__portable_image_raw_output.snap diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 7e39539451..27e9253821 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -828,6 +828,9 @@ impl App { AppEvent::FatalExitRequest(message) => { return Ok(AppRunControl::Exit(ExitReason::Fatal(message))); } + AppEvent::ImagesPrepared(id) => { + self.chat_widget.on_images_prepared(id); + } AppEvent::CodexOp(mut op) => { if let AppCommand::OverrideTurnContext { cwd, diff --git a/codex-rs/tui/src/app/reconnect.rs b/codex-rs/tui/src/app/reconnect.rs index 329e959f37..d8c248a568 100644 --- a/codex-rs/tui/src/app/reconnect.rs +++ b/codex-rs/tui/src/app/reconnect.rs @@ -285,6 +285,7 @@ impl App { self.chat_widget.windows_sandbox_elevated_setup_complete = false; } } + self.chat_widget.snapshot_local_images = self.app_server_target.uses_remote_workspace(); self.chat_widget.set_local_worktree_operations( !crate::uses_remote_workspace_or_environment( &self.app_server_target, diff --git a/codex-rs/tui/src/app/session_lifecycle.rs b/codex-rs/tui/src/app/session_lifecycle.rs index 8c99bdd84e..f816316d33 100644 --- a/codex-rs/tui/src/app/session_lifecycle.rs +++ b/codex-rs/tui/src/app/session_lifecycle.rs @@ -496,6 +496,7 @@ impl App { chat_widget.last_terminal_title = previous_terminal_title; } chat_widget.remote_connection = self.chat_widget.remote_connection.clone(); + chat_widget.snapshot_local_images = self.app_server_target.uses_remote_workspace(); chat_widget.set_local_worktree_operations(self.chat_widget.local_worktree_operations); chat_widget.windows_sandbox_host = self.chat_widget.windows_sandbox_host; #[cfg(any(target_os = "windows", test))] diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index 7d709218ce..40fe3abc40 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -672,6 +672,7 @@ impl App { } chat_widget.note_rendered_width(tui.terminal.last_known_screen_size.width); chat_widget.remote_connection = remote_connection; + chat_widget.snapshot_local_images = app_server_target.uses_remote_workspace(); chat_widget.set_local_worktree_operations(!crate::uses_remote_workspace_or_environment( &app_server_target, environment_manager.as_ref(), diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 6235aa995f..e35bec3d70 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -603,6 +603,9 @@ pub(crate) enum AppEvent { /// bubbling channels through layers of widgets. CodexOp(AppCommand), + /// A blocking image-preparation worker has finished; payload stays with its widget. + ImagesPrepared(Uuid), + /// Approve one retry of a recent auto-review denial selected in the TUI. ApproveRecentAutoReviewDenial { thread_id: ThreadId, diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9dd0cadb54..3cbe36586e 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -343,6 +343,7 @@ use self::ide_context::IdeContextState; mod input_queue; mod reconnect; use self::input_queue::InputQueueState; +mod image_submission; mod input_flow; mod input_restore; mod input_submission; @@ -596,6 +597,9 @@ pub(crate) struct ChatWidget { pub(crate) initial_user_message: Option, status_account_display: Option, pub(crate) remote_connection: Option, + /// Remote app servers cannot read image paths on the TUI host. + pub(crate) snapshot_local_images: bool, + pending_image_submission: Option, pub(crate) local_worktree_operations: bool, pub(crate) windows_sandbox_host: crate::app::WindowsSandboxHost, #[cfg(any(target_os = "windows", test))] @@ -816,6 +820,7 @@ pub(crate) struct ChatWidget { current_goal_status: Option, external_editor_state: ExternalEditorState, last_rendered_user_message_display: Option, + last_rendered_user_message_client_id: Option, last_non_retry_error: Option<(String, String)>, } @@ -1354,6 +1359,12 @@ impl ChatWidget { return; } + // Servers may omit media from receipts, so prefer the submission identity. + if client_id.is_some() && self.last_rendered_user_message_client_id.as_deref() == client_id + { + return; + } + if self .input_queue .pending_steers @@ -1369,6 +1380,9 @@ impl ChatWidget { let pending_display = user_message_display_for_history(pending.user_message, &pending.history_record); self.on_user_message_display(pending_display); + // Later receipts carry the wire media, which may differ from the local attachment. + self.last_rendered_user_message_display = Some(display); + self.last_rendered_user_message_client_id = Some(pending.client_id); } else if self.last_rendered_user_message_display.as_ref() != Some(&display) { tracing::warn!( "pending steer matched receipt but queue was empty when rendering committed user message" @@ -1385,6 +1399,7 @@ impl ChatWidget { fn on_user_message_display(&mut self, display: UserMessageDisplay) { self.transcript.last_status_copy_targets = None; self.last_rendered_user_message_display = Some(display.clone()); + self.last_rendered_user_message_client_id = None; if !display.message.trim().is_empty() || !display.text_elements.is_empty() || !display.local_images.is_empty() @@ -1799,6 +1814,7 @@ impl ChatWidget { } pub(crate) fn show_external_writer_thread(&mut self) { + self.cancel_image_submission(); self.blocks_direct_input = true; self.external_writer_view = true; self.pause_unavailable_thread(); diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index f04c2f627f..3ed23be896 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -134,6 +134,8 @@ impl ChatWidget { initial_user_message, status_account_display, remote_connection: None, + snapshot_local_images: false, + pending_image_submission: None, local_worktree_operations: true, windows_sandbox_host: crate::app::WindowsSandboxHost::Local, #[cfg(any(target_os = "windows", test))] @@ -274,6 +276,7 @@ impl ChatWidget { current_goal_status: None, external_editor_state: ExternalEditorState::Closed, last_rendered_user_message_display: None, + last_rendered_user_message_client_id: None, last_non_retry_error: None, }; diff --git a/codex-rs/tui/src/chatwidget/image_submission.rs b/codex-rs/tui/src/chatwidget/image_submission.rs new file mode 100644 index 0000000000..3e5a961158 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/image_submission.rs @@ -0,0 +1,169 @@ +//! Prepare remote image attachments off the event loop, retaining the draft until completion. +//! Cancellation restores the draft; worker notifications from a retired submission are ignored. +//! Preparation is serialized so canceled decodes cannot pile up across retries or thread switches. + +use super::*; +use codex_protocol::models::ImageDetail; +use codex_protocol::models::snapshot_local_user_input; +use codex_protocol::user_input::UserInput as CoreUserInput; +use tokio::sync::oneshot; + +static IMAGE_PREPARATION: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + +pub(super) struct PendingImageSubmission { + id: uuid::Uuid, + pub(super) message: UserMessage, + history_record: UserMessageHistoryRecord, + source: UserMessageSource, + result: oneshot::Receiver, String>>, +} + +impl ChatWidget { + pub(super) fn prepare_image_submission( + &mut self, + message: UserMessage, + history_record: UserMessageHistoryRecord, + source: UserMessageSource, + ) { + let id = uuid::Uuid::new_v4(); + let images = message.local_images.clone(); + let remote_bytes = message.remote_image_urls.iter().map(String::len).sum(); + let (mut tx, result) = oneshot::channel(); + let events = self.app_event_tx.clone(); + tokio::spawn(async move { + let permit = tokio::select! { + biased; + _ = tx.closed() => return, + permit = IMAGE_PREPARATION.lock() => permit, + }; + let prepared = tokio::task::spawn_blocking(move || { + let _permit = permit; + prepare_images(images, remote_bytes) + }) + .await + .unwrap_or_else(|error| Err(format!("Failed to prepare images: {error}"))); + if tx.send(prepared).is_ok() { + events.send(AppEvent::ImagesPrepared(id)); + } + }); + self.pending_image_submission = Some(PendingImageSubmission { + id, + message, + history_record, + source, + result, + }); + self.refresh_pending_input_preview(); + self.request_redraw(); + } + + pub(crate) fn on_images_prepared(&mut self, id: uuid::Uuid) { + let Some(mut pending) = self + .pending_image_submission + .take_if(|pending| pending.id == id) + else { + return; + }; + match pending + .result + .try_recv() + .unwrap_or_else(|error| Err(format!("Failed to prepare images: {error}"))) + { + Ok(images) => { + let (accepted, _) = self.submit_user_message_with_prepared_images( + pending.message, + pending.history_record, + ShellEscapePolicy::Disallow, + pending.source, + Some(images), + ); + if !accepted { + self.input_queue.recovered_queue |= + self.input_queue.has_queued_follow_up_messages(); + } + } + Err(error) => { + self.input_queue.recovered_queue |= + self.input_queue.has_queued_follow_up_messages(); + self.add_error_message(error); + self.restore_user_message_to_composer(user_message_for_restore( + pending.message, + &pending.history_record, + )); + } + } + self.refresh_pending_input_preview(); + self.request_redraw(); + } + + pub(super) fn requeue_image_submission(&mut self) { + if let Some(pending) = self.pending_image_submission.take() { + self.input_queue + .queued_user_messages + .push_front(QueuedUserMessage { + source: pending.source, + ..QueuedUserMessage::new(pending.message, QueuedInputAction::Literal) + }); + self.input_queue + .queued_user_message_history_records + .push_front(pending.history_record); + self.refresh_pending_input_preview(); + self.request_redraw(); + } + } + + pub(super) fn cancel_image_submission(&mut self) -> bool { + let Some(pending) = self.pending_image_submission.take() else { + return false; + }; + self.input_queue.recovered_queue |= self.input_queue.has_queued_follow_up_messages(); + self.restore_user_message_to_composer(user_message_for_restore( + pending.message, + &pending.history_record, + )); + self.refresh_pending_input_preview(); + self.request_redraw(); + true + } +} + +fn prepare_images( + images: Vec, + remote_bytes: usize, +) -> Result, String> { + // Leave headroom below the WebSocket server's 64 MiB message limit. + const MAX_IMAGE_BYTES: usize = 32 * 1024 * 1024; + let mut remaining_image_bytes = MAX_IMAGE_BYTES.saturating_sub(remote_bytes); + images + .into_iter() + .map(|image| { + let mut input = CoreUserInput::LocalImage { + path: image.path.clone(), + // Preserve source pixels; core applies the model-specific resize policy. + detail: Some(ImageDetail::Original), + }; + let prepared = (|| -> std::io::Result<()> { + let too_large = + || std::io::Error::other("image attachments exceed the 32 MiB transport limit"); + if std::fs::metadata(&image.path)?.len() > (remaining_image_bytes / 4 * 3) as u64 { + return Err(too_large()); + } + snapshot_local_user_input(&mut input)?; + if let CoreUserInput::Image { image_url, .. } = &input { + remaining_image_bytes = remaining_image_bytes + .checked_sub(image_url.len()) + .ok_or_else(too_large)?; + } + Ok(()) + })(); + prepared.map_err(|error| { + format!("Failed to prepare image: {}: {error}", image.path.display()) + })?; + let mut input = UserInput::from(input); + if let UserInput::Image { detail, .. } = &mut input { + *detail = None; + } + Ok(input) + }) + .collect() +} diff --git a/codex-rs/tui/src/chatwidget/input_flow.rs b/codex-rs/tui/src/chatwidget/input_flow.rs index d6ad607744..2d06ec98e1 100644 --- a/codex-rs/tui/src/chatwidget/input_flow.rs +++ b/codex-rs/tui/src/chatwidget/input_flow.rs @@ -11,6 +11,7 @@ use crate::bottom_pane::slash_commands::find_slash_command; impl ChatWidget { pub(crate) fn set_parent_owned_thread(&mut self) { + self.cancel_image_submission(); self.blocks_direct_input = true; self.bottom_pane.set_parent_owned_thread(); } @@ -297,7 +298,8 @@ impl ChatWidget { } pub(crate) fn is_user_turn_pending_or_running(&self) -> bool { - self.input_queue.user_turn_pending_start + self.pending_image_submission.is_some() + || self.input_queue.user_turn_pending_start || self.turn_lifecycle.agent_turn_running || self.review.is_review_mode || (self.bottom_pane.is_task_running() && self.mcp_startup_status.is_none()) @@ -318,7 +320,13 @@ impl ChatWidget { if let Some(questions) = &mut self.bottom_pane.questions { questions.has_queued_messages = has_queued; } - let preview = self.input_queue.preview(); + let mut preview = self.input_queue.preview(); + if let Some(pending) = &self.pending_image_submission { + preview.queued_messages.insert( + /*index*/ 0, + format!("Preparing images: {}", pending.message.text), + ); + } self.bottom_pane.set_pending_input_preview( preview.queued_messages, preview.pending_steers, diff --git a/codex-rs/tui/src/chatwidget/input_restore.rs b/codex-rs/tui/src/chatwidget/input_restore.rs index a15bb8e140..75e94dea8d 100644 --- a/codex-rs/tui/src/chatwidget/input_restore.rs +++ b/codex-rs/tui/src/chatwidget/input_restore.rs @@ -284,6 +284,7 @@ impl ChatWidget { /// When there are queued user messages, restore them into the composer /// separated by newlines rather than auto-submitting the next one. pub(super) fn on_interrupted_turn(&mut self, reason: TurnAbortReason) { + self.requeue_image_submission(); // Finalize, log a gentle prompt, and clear running state. self.finalize_turn(); let send_pending_steers_immediately = @@ -502,6 +503,7 @@ impl ChatWidget { } pub(crate) fn capture_thread_input_state(&mut self) -> Option { + self.cancel_image_submission(); let draft = self.bottom_pane.composer_draft_snapshot(); let composer = ThreadComposerState { text: draft.text, diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index 1421279e3c..6b43d3793e 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -115,23 +115,53 @@ impl ChatWidget { history_record: UserMessageHistoryRecord, shell_escape_policy: ShellEscapePolicy, source: UserMessageSource, + ) -> (bool, Option) { + self.submit_user_message_with_prepared_images( + user_message, + history_record, + shell_escape_policy, + source, + /*prepared_images*/ None, + ) + } + + pub(super) fn submit_user_message_with_prepared_images( + &mut self, + user_message: UserMessage, + history_record: UserMessageHistoryRecord, + shell_escape_policy: ShellEscapePolicy, + source: UserMessageSource, + prepared_images: Option>, ) -> (bool, Option) { if self.has_misalignment_policy_violation() { return (false, None); } - if self.input_queue.rate_limit_recovery_pending { + if self.input_queue.rate_limit_recovery_pending || self.pending_image_submission.is_some() { let model_prompt = source == UserMessageSource::Prompt && (shell_escape_policy == ShellEscapePolicy::Disallow || !user_message.text.starts_with('!')); - self.input_queue - .queued_user_messages - .push_back(QueuedUserMessage { + // A prepared submission was accepted before any messages queued behind it. + let queue_index = if prepared_images.is_some() { + 0 + } else { + self.input_queue.queued_user_messages.len() + }; + self.input_queue.queued_user_messages.insert( + queue_index, + QueuedUserMessage { source, - ..QueuedUserMessage::from(user_message) - }); + ..QueuedUserMessage::new( + user_message, + match shell_escape_policy { + ShellEscapePolicy::Allow => QueuedInputAction::Plain, + ShellEscapePolicy::Disallow => QueuedInputAction::Literal, + }, + ) + }, + ); self.input_queue .queued_user_message_history_records - .push_back(history_record); + .insert(queue_index, history_record); self.refresh_pending_input_preview(); if model_prompt { self.bottom_pane.clear_pending_questions(); @@ -190,10 +220,6 @@ impl ChatWidget { text_elements, mut mention_bindings, } = user_message; - if !self.bottom_pane.task_mentions_enabled() { - mention_bindings - .retain(|binding| crate::task_mentions::valid_thread_path(&binding.path).is_none()); - } let render_in_history = !self.turn_lifecycle.agent_turn_running; let mut items: Vec = Vec::new(); @@ -218,11 +244,26 @@ impl ChatWidget { }); } - for image in &local_images { - items.push(UserInput::LocalImage { + if let Some(prepared_images) = prepared_images { + items.extend(prepared_images); + } else if self.snapshot_local_images && !local_images.is_empty() { + self.prepare_image_submission( + UserMessage { + text, + local_images, + remote_image_urls, + text_elements, + mention_bindings, + }, + history_record, + source, + ); + return (true, None); + } else { + items.extend(local_images.iter().map(|image| UserInput::LocalImage { path: image.path.clone(), detail: None, - }); + })); } if !text.is_empty() { @@ -232,6 +273,11 @@ impl ChatWidget { }); } + if !self.bottom_pane.task_mentions_enabled() { + mention_bindings + .retain(|binding| crate::task_mentions::valid_thread_path(&binding.path).is_none()); + } + let mentions = collect_tool_mentions(&text, &HashMap::new()); let bound_names: HashSet = mention_bindings .iter() @@ -368,6 +414,8 @@ impl ChatWidget { } else { None }; + let submitted_image_display = (render_in_history && !local_images.is_empty()) + .then(|| Self::user_message_display_from_inputs(&items)); let client_user_message_id = uuid::Uuid::new_v4().to_string(); let pending_steer = (!render_in_history).then(|| PendingSteer { client_id: client_user_message_id.clone(), @@ -385,7 +433,7 @@ impl ChatWidget { let service_tier = self.service_tier_update_for_core(); let active_permission_profile = self.config.permissions.active_permission_profile(); let op = AppCommand::user_turn( - client_user_message_id, + client_user_message_id.clone(), items, self.config.cwd.to_path_buf(), AskForApproval::from(self.config.permissions.approval_policy.value()), @@ -485,6 +533,14 @@ impl ChatWidget { } } + if render_in_history { + self.last_rendered_user_message_client_id = Some(client_user_message_id); + } + if let Some(display) = submitted_image_display { + // Match the server echo's portable media without changing the locally rendered cell. + self.last_rendered_user_message_display = Some(display); + } + (true, Some(op)) } @@ -504,14 +560,13 @@ impl ChatWidget { remote_image_urls: Vec, ) { // Preserve the user's composed payload so they can retry after changing models. - let local_image_paths = local_images.iter().map(|img| img.path.clone()).collect(); - self.set_remote_image_urls(remote_image_urls); - self.bottom_pane.set_composer_text_with_mention_bindings( + self.restore_user_message_to_composer(UserMessage { text, text_elements, - local_image_paths, + local_images, mention_bindings, - ); + remote_image_urls, + }); self.add_to_history(history_cell::new_warning_event( self.image_inputs_not_supported_message(), )); diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index 6b46e1c234..de29f1da10 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -43,6 +43,24 @@ impl ChatWidget { return; } + if (self.chat_keymap.interrupt_turn.is_pressed(key_event) + || key_hint::ctrl(KeyCode::Char('c')).is_press(key_event)) + && self.bottom_pane.no_modal_or_popup_active() + && !self.should_handle_vim_insert_escape(key_event) + && self.pending_image_submission.is_some() + { + if self.is_cancellable_work_active() { + self.requeue_image_submission(); + self.input_queue.recovered_queue = true; + if self.submit_op(AppCommand::interrupt()) { + self.pause_active_goal_for_interrupt(); + } + } else { + self.cancel_image_submission(); + } + return; + } + if self.handle_reasoning_shortcut(key_event) || self.handle_permission_shortcut(key_event) { self.bottom_pane.clear_quit_shortcut_hint(); self.quit_shortcut_expires_at = None; @@ -124,13 +142,15 @@ impl ChatWidget { if key_event.kind == KeyEventKind::Press && self.chat_keymap.edit_queued_message.is_pressed(key_event) - && self.has_queued_follow_up_messages() + && (self.has_queued_follow_up_messages() || self.pending_image_submission.is_some()) && self.bottom_pane.no_modal_or_popup_active() { if let Some(composer) = self.pop_latest_queued_composer_state() { self.restore_composer_state(composer); self.refresh_pending_input_preview(); self.request_redraw(); + } else { + self.cancel_image_submission(); } return; } diff --git a/codex-rs/tui/src/chatwidget/reconnect.rs b/codex-rs/tui/src/chatwidget/reconnect.rs index 0e94051794..6e98446877 100644 --- a/codex-rs/tui/src/chatwidget/reconnect.rs +++ b/codex-rs/tui/src/chatwidget/reconnect.rs @@ -5,6 +5,7 @@ use super::*; impl ChatWidget { pub(crate) fn pause_for_disconnect(&mut self) { + self.cancel_image_submission(); // The app-server transport can fail while the separate WebRTC helper // still sends microphone audio. Retire local media before showing offline UI. if matches!( diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_disconnected.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_disconnected.snap new file mode 100644 index 0000000000..a4ffa9634d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_disconnected.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/chatwidget/tests/composer_submission.rs +assertion_line: 2750 +expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))" +--- +• Reconnect failed — check the endpoint, then relaunch (0s) + +• Queued follow-up inputs + ↳ x + ⌥ + ↑ edit last queued message + +› describe + + ctrl+c quit diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_in_progress.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_in_progress.snap new file mode 100644 index 0000000000..d19e4238e9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_in_progress.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/composer_submission.rs +assertion_line: 2689 +expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))" +--- +• Queued follow-up inputs + ↳ Preparing images: describe + ⌥ + ↑ edit last queued message + +› x + + gpt-5.6-sol default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_restored_input.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_restored_input.snap new file mode 100644 index 0000000000..8d1fb0ac3b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__image_preparation_restored_input.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/composer_submission.rs +expression: "render_bottom_popup(&chat, 80)" +--- + [Image #1] + +› [Image #2] describe $file + + gpt-5.6-sol default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__portable_image_echo_renders_once.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__portable_image_echo_renders_once.snap new file mode 100644 index 0000000000..9044362bba --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__portable_image_echo_renders_once.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/composer_submission.rs +expression: rendered +--- +› [Image #1] describe diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 7d184d880d..9081acbf4e 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -1,6 +1,7 @@ use super::*; use crate::app_event::ConnectorsSnapshot; use crate::history_cell::ThreadRecapLoadingCell; +use base64::Engine; use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::permissions::FileSystemAccessMode; use codex_protocol::permissions::FileSystemPath; @@ -2459,3 +2460,353 @@ async fn reconnect_holds_only_recovered_input_until_manually_edited() { assert_matches!(next_submit_op(&mut ops), Op::UserTurn { .. }); } } + +#[tokio::test(flavor = "multi_thread")] +async fn image_submission_is_portable_for_new_turns_and_steers() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("image.png"); + // Transport must not apply the helper's default 2048-pixel resize policy. + let pixels = image::RgbImage::from_pixel( + /*width*/ 3000, + /*height*/ 2, + image::Rgb([12, 34, 56]), + ); + pixels.save(&path).unwrap(); + let placeholder = "[Image #1]"; + let message = UserMessage { + local_images: vec![LocalImageAttachment { + placeholder: placeholder.into(), + path, + }], + text_elements: vec![TextElement::new( + (0..placeholder.len()).into(), + Some(placeholder.into()), + )], + ..UserMessage::from("[Image #1] describe") + }; + for running in [false, true] { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.snapshot_local_images = true; + chat.codex_op_target = CodexOpTarget::AppEvent; + // Repeated identical submissions each render once, across supported receipt shapes. + for (echo_client_id, omit_media) in [(false, false), (true, false), (true, true)] { + let mut message = message.clone(); + if omit_media { + message.text.push_str("\u{1b}[31m"); + } + if running { + handle_turn_started(&mut chat, "turn"); + } + assert!( + chat.submit_user_message_with_shell_escape_policy( + message, + ShellEscapePolicy::Disallow + ) + .is_none() + ); + let mut rendered = Vec::new(); + let (items, client_user_message_id) = loop { + match rx.recv().await.unwrap() { + AppEvent::ImagesPrepared(id) => chat.on_images_prepared(id), + AppEvent::InsertHistoryCell(cell) => { + rendered.push(cell.display_lines(/*width*/ 80)) + } + AppEvent::CodexOp(AppCommand::UserTurn { + items, + client_user_message_id, + .. + }) => { + break (items, client_user_message_id); + } + _ => {} + } + }; + let [UserInput::Image { url, detail }, UserInput::Text { .. }] = items.as_slice() + else { + panic!("local image must be portable on the wire"); + }; + let (_, encoded) = url.split_once(";base64,").unwrap(); + let bytes = base64::engine::general_purpose::STANDARD + .decode(encoded) + .unwrap(); + assert_eq!( + (image::load_from_memory(&bytes).unwrap().to_rgb8(), detail), + (pixels.clone(), &None) + ); + let mut receipt = items.clone(); + if omit_media { + receipt.retain(|item| !matches!(item, UserInput::Image { .. })); + } + for _ in 0..2 { + chat.on_committed_user_message( + &receipt, + echo_client_id.then_some(client_user_message_id.as_str()), + /*from_replay*/ false, + "turn", + ); + } + rendered.extend(drain_insert_history(&mut rx)); + assert_eq!(rendered.len(), 1); + assert_chatwidget_snapshot!( + "portable_image_echo_renders_once", + lines_to_single_string(&rendered[0]) + ); + assert!(chat.input_queue.pending_steers.is_empty()); + chat.on_committed_user_message( + &items, + Some(&client_user_message_id), + /*from_replay*/ true, + "turn", + ); + let replayed = drain_insert_history(&mut rx); + assert_eq!(replayed, rendered); + handle_turn_completed(&mut chat, "turn", /*duration_ms*/ None); + drain_insert_history(&mut rx); + } + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn image_preparation_failure_restores_full_input_without_submitting() { + for (running, oversized) in [(false, false), (true, false), (false, true)] { + let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.snapshot_local_images = true; + if running { + handle_turn_started(&mut chat, "turn"); + chat.input_queue + .pending_steers + .push_back(pending_steer("already sent")); + } + let pending_steers = chat.input_queue.pending_steers.clone(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("removed.png"); + let pixels = image::RgbImage::new(/*width*/ 2, /*height*/ 2); + pixels.save(&path).unwrap(); + chat.set_remote_image_urls(vec!["https://example.com/remote.png".into()]); + chat.handle_paste(path.display().to_string()); + chat.handle_paste("describe $file".into()); + chat.bottom_pane.set_composer_text_with_mention_bindings( + chat.bottom_pane.composer_text(), + chat.bottom_pane.composer_text_elements(), + vec![path.clone()], + vec![MentionBinding { + sigil: '$', + mention: "file".into(), + path: "/tmp/skills/file/SKILL.md".into(), + }], + ); + let draft = chat.bottom_pane.composer_draft_snapshot(); + if oversized { + std::fs::OpenOptions::new() + .write(true) + .open(&path) + .unwrap() + .set_len(25 * 1024 * 1024) + .unwrap(); + } else { + std::fs::remove_file(&path).unwrap(); + } + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + while let Some(event) = rx.recv().await { + if let AppEvent::ImagesPrepared(id) = event { + chat.on_images_prepared(id); + break; + } + } + assert!( + op_rx.try_recv().is_err(), + "preparation failure must not emit an operation" + ); + let restored = chat.bottom_pane.composer_draft_snapshot(); + assert_eq!( + ( + restored.text, + restored.text_elements, + restored.local_images, + restored.remote_image_urls, + restored.mention_bindings + ), + ( + draft.text, + draft.text_elements, + draft.local_images, + draft.remote_image_urls, + draft.mention_bindings + ), + ); + assert_eq!(chat.turn_lifecycle.agent_turn_running, running); + assert!(!chat.input_queue.user_turn_pending_start); + assert_eq!(chat.input_queue.pending_steers, pending_steers); + assert!(drain_insert_history(&mut rx).iter().flatten().any(|line| { + line.to_string().contains(if oversized { + "32 MiB" + } else { + "Failed to prepare image:" + }) + })); + if !running { + assert_chatwidget_snapshot!( + "image_preparation_restored_input", + normalize_snapshot_paths(render_bottom_popup(&chat, /*width*/ 80)) + ); + } + pixels.save(&path).unwrap(); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + while let Some(event) = rx.recv().await { + if let AppEvent::ImagesPrepared(id) = event { + chat.on_images_prepared(id); + break; + } + } + assert_matches!(next_submit_op(&mut op_rx), Op::UserTurn { .. }); + } +} + +#[test] +fn image_preparation_keeps_input_responsive_and_preserves_pending_input() { + for scenario in [ + "thread_switch", + "rate_limit_recovery", + "model_change", + "disconnect", + "interrupt", + "parent_owned", + "external_writer", + "escape", + "ctrl_c", + ] { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .max_blocking_threads(/*val*/ 1) + .build() + .unwrap() + .block_on(async { + let (mut chat, mut rx, mut ops) = + make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.snapshot_local_images = true; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("image.png"); + image::RgbImage::new(/*width*/ 2, /*height*/ 2) + .save(&path) + .unwrap(); + let message = UserMessage { + local_images: vec![LocalImageAttachment { + placeholder: "[Image #1]".into(), + path, + }], + ..UserMessage::from("describe") + }; + // Occupy the worker so submission and input handling must return before decoding starts. + let (release, held) = std::sync::mpsc::channel::<()>(); + let (started, ready) = tokio::sync::oneshot::channel(); + let worker = tokio::task::spawn_blocking(move || { + started.send(()).unwrap(); + let _ = held.recv(); + }); + ready.await.unwrap(); + chat.submit_user_message(message.clone()); + chat.handle_key_event(KeyEvent::from(KeyCode::Char('x'))); + chat.handle_key_event(KeyEvent::from(KeyCode::Right)); + chat.pre_draw_tick(); + assert_eq!(chat.bottom_pane.composer_text(), "x"); + assert_no_submit_op(&mut ops); + assert_chatwidget_snapshot!( + "image_preparation_in_progress", + normalize_snapshot_paths(render_bottom_popup(&chat, /*width*/ 80)) + ); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_eq!(chat.input_queue.queued_user_messages.len(), 1); + assert_no_submit_op(&mut ops); + drop(release); + worker.await.unwrap(); + let id = loop { + if let AppEvent::ImagesPrepared(id) = rx.recv().await.unwrap() { + break id; + } + }; + if scenario == "rate_limit_recovery" { + chat.input_queue.rate_limit_recovery_pending = true; + chat.on_images_prepared(id); + assert_eq!( + chat.input_queue + .queued_user_messages + .iter() + .map(|queued| queued.user_message.clone()) + .collect::>(), + vec![message, UserMessage::from("x")], + ); + } else if scenario == "model_change" { + chat.handle_paste("new draft".into()); + let mut models = chat.model_catalog.try_list_models().unwrap(); + for model in &mut models { + model + .input_modalities + .retain(|modality| *modality != InputModality::Image); + } + chat.model_catalog = Arc::new(ModelCatalog::new(models)); + chat.on_images_prepared(id); + assert_eq!(chat.bottom_pane.composer_text(), "describe\nnew draft"); + } else if scenario == "escape" || scenario == "ctrl_c" { + chat.on_task_started(); + chat.handle_key_event(if scenario == "escape" { + KeyEvent::from(KeyCode::Esc) + } else { + KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL) + }); + next_interrupt_op(&mut ops); + if scenario == "escape" { + chat.on_interrupted_turn(TurnAbortReason::Interrupted); + assert_eq!(chat.bottom_pane.composer_text(), "describe\nx"); + } else { + chat.on_task_complete( + /*last_agent_message*/ None, /*completion*/ None, + /*from_replay*/ false, + ); + assert_eq!(chat.input_queue.queued_user_messages.len(), 2); + } + chat.on_images_prepared(id); + } else if scenario == "parent_owned" || scenario == "external_writer" { + chat.codex_op_target = CodexOpTarget::AppEvent; + if scenario == "parent_owned" { + chat.set_parent_owned_thread(); + } else { + chat.show_external_writer_thread(); + } + assert_eq!(chat.bottom_pane.composer_text(), "describe"); + chat.on_images_prepared(id); + assert!(drain_insert_history(&mut rx).is_empty()); + } else if scenario == "interrupt" { + chat.input_queue + .pending_steers + .push_back(pending_steer("earlier")); + chat.on_interrupted_turn(TurnAbortReason::Interrupted); + assert_eq!(chat.bottom_pane.composer_text(), "earlier\ndescribe\nx"); + chat.on_images_prepared(id); + } else if scenario == "disconnect" { + chat.pause_for_disconnect(); + chat.reconnect_failed(); + assert_eq!(chat.bottom_pane.composer_text(), "describe"); + assert_eq!( + chat.bottom_pane.composer_draft_snapshot().local_images, + message.local_images, + ); + assert_chatwidget_snapshot!( + "image_preparation_disconnected", + normalize_snapshot_paths(render_bottom_popup(&chat, /*width*/ 80)) + ); + chat.on_images_prepared(id); + } else { + // A completion already in flight must not submit to a replacement widget/thread. + let saved = chat.capture_thread_input_state().unwrap(); + assert_eq!(saved.composer.as_ref().unwrap().text, "describe"); + assert_eq!(saved.composer.as_ref().unwrap().local_images.len(), 1); + chat.on_images_prepared(id); + } + assert_no_submit_op(&mut ops); + assert!(chat.pending_image_submission.is_none()); + }); + } +} diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index 084c4fffac..26cdfec2a4 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -149,8 +149,17 @@ fn build_user_message_lines_with_elements( raw_lines } -fn remote_image_display_line(style: Style, index: usize) -> Line<'static> { - Line::from(local_image_label_text(index)).style(style) +impl UserHistoryCell { + fn image_labels_not_in_message(&self) -> impl Iterator + '_ { + (1..=self.remote_image_urls.len()) + .map(local_image_label_text) + .filter(|label| { + !self + .text_elements + .iter() + .any(|element| element.placeholder(&self.message) == Some(label.as_str())) + }) + } } impl HistoryCell for UserHistoryCell { @@ -184,16 +193,13 @@ impl HistoryCell for UserHistoryCell { None } else { Some(plain_hyperlink_lines(adaptive_wrap_lines( - self.remote_image_urls - .iter() - .enumerate() - .map(|(idx, _url)| { - remote_image_display_line(element_style, idx.saturating_add(1)) - }), + self.image_labels_not_in_message() + .map(|label| Line::from(label).style(element_style)), RtOptions::new(usize::from(wrap_width)) .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), ))) - }; + } + .filter(|lines| !lines.is_empty()); let wrapped_message = if message.is_empty() && text_elements.is_empty() { None @@ -290,16 +296,12 @@ impl HistoryCell for UserHistoryCell { fn raw_lines(&self) -> Vec> { let message = sanitize_user_text((&self.message).into()); let mut lines = raw_lines_from_source(message.as_ref().trim_end_matches(['\r', '\n'])); - if !self.remote_image_urls.is_empty() { + let mut image_labels = self.image_labels_not_in_message().peekable(); + if image_labels.peek().is_some() { if !lines.is_empty() { lines.push(Line::from("")); } - lines.extend( - self.remote_image_urls - .iter() - .enumerate() - .map(|(idx, _url)| Line::from(local_image_label_text(idx.saturating_add(1)))), - ); + lines.extend(image_labels.map(Line::from)); } lines } diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__portable_image_raw_output.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__portable_image_raw_output.snap new file mode 100644 index 0000000000..c6ad6c2772 --- /dev/null +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__portable_image_raw_output.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/history_cell/tests.rs +expression: "render_lines(&cell.raw_lines()).join(\"\\n\")" +--- +[Image #1] describe inline image diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index dfa7129ef9..50a35b12d8 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -2413,7 +2413,7 @@ fn user_history_cell_renders_remote_image_urls() { #[test] fn user_history_cell_summarizes_inline_data_urls() { - let cell = UserHistoryCell { + let mut cell = UserHistoryCell { spoken: false, message: "describe inline image".to_string(), text_elements: Vec::new(), @@ -2425,6 +2425,16 @@ fn user_history_cell_summarizes_inline_data_urls() { assert!(rendered.contains("[Image #1]")); assert!(rendered.contains("describe inline image")); + let placeholder = "[Image #1]"; + cell.message = format!("{placeholder} describe inline image"); + cell.text_elements = vec![TextElement::new( + (0..placeholder.len()).into(), + Some(placeholder.to_string()), + )]; + insta::assert_snapshot!( + "portable_image_raw_output", + render_lines(&cell.raw_lines()).join("\n") + ); } #[test]