mirror of
https://github.com/openai/codex.git
synced 2026-10-21 05:22:21 +00:00
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
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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))]
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<UserMessage>,
|
||||
status_account_display: Option<StatusAccountDisplay>,
|
||||
pub(crate) remote_connection: Option<RemoteConnectionStatus>,
|
||||
/// Remote app servers cannot read image paths on the TUI host.
|
||||
pub(crate) snapshot_local_images: bool,
|
||||
pending_image_submission: Option<image_submission::PendingImageSubmission>,
|
||||
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<GoalStatusState>,
|
||||
external_editor_state: ExternalEditorState,
|
||||
last_rendered_user_message_display: Option<UserMessageDisplay>,
|
||||
last_rendered_user_message_client_id: Option<String>,
|
||||
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();
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
|
||||
@@ -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<Result<Vec<UserInput>, 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<LocalImageAttachment>,
|
||||
remote_bytes: usize,
|
||||
) -> Result<Vec<UserInput>, 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()
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
@@ -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<ThreadInputState> {
|
||||
self.cancel_image_submission();
|
||||
let draft = self.bottom_pane.composer_draft_snapshot();
|
||||
let composer = ThreadComposerState {
|
||||
text: draft.text,
|
||||
|
||||
@@ -115,23 +115,53 @@ impl ChatWidget {
|
||||
history_record: UserMessageHistoryRecord,
|
||||
shell_escape_policy: ShellEscapePolicy,
|
||||
source: UserMessageSource,
|
||||
) -> (bool, Option<AppCommand>) {
|
||||
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<Vec<UserInput>>,
|
||||
) -> (bool, Option<AppCommand>) {
|
||||
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<UserInput> = 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<String> = 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<String>,
|
||||
) {
|
||||
// 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(),
|
||||
));
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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!(
|
||||
|
||||
+14
@@ -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
|
||||
+12
@@ -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
|
||||
+9
@@ -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
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/composer_submission.rs
|
||||
expression: rendered
|
||||
---
|
||||
› [Image #1] describe
|
||||
@@ -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<_>>(),
|
||||
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());
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<Item = String> + '_ {
|
||||
(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<Line<'static>> {
|
||||
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
|
||||
}
|
||||
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/history_cell/tests.rs
|
||||
expression: "render_lines(&cell.raw_lines()).join(\"\\n\")"
|
||||
---
|
||||
[Image #1] describe inline image
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user