mirror of
https://github.com/openai/codex.git
synced 2026-05-04 21:32:21 +03:00
tui: preserve remote image attachments across resume/backtrack (#10590)
## Summary This PR makes app-server-provided image URLs first-class attachments in TUI, so they survive resume/backtrack/history recall and are resubmitted correctly. <img width="715" height="491" alt="Screenshot 2026-02-12 at 8 27 08 PM" src="https://github.com/user-attachments/assets/226cbd35-8f0c-4e51-a13e-459ef5dd1927" /> Can delete the attached image upon backtracking: <img width="716" height="301" alt="Screenshot 2026-02-12 at 8 27 31 PM" src="https://github.com/user-attachments/assets/4558d230-f1bd-4eed-a093-8e1ab9c6db27" /> In both history and composer, remote images are rendered as normal `[Image #N]` placeholders, with numbering unified with local images. ## What changed - Plumb remote image URLs through TUI message state: - `UserHistoryCell` - `BacktrackSelection` - `ChatComposerHistory::HistoryEntry` - `ChatWidget::UserMessage` - Show remote images as placeholder rows inside the composer box (above textarea), and in history cells. - Support keyboard selection/deletion for remote image rows in composer (`Up`/`Down`, `Delete`/`Backspace`). - Preserve remote-image-only turns in local composer history (Up/Down recall), including restore after backtrack. - Ensure submit/queue/backtrack resubmit include remote images in model input (`UserInput::Image`), and keep request shape stable for remote-image-only turns. - Keep image numbering contiguous across remote + local images: - remote images occupy `[Image #1]..[Image #M]` - local images start at `[Image #M+1]` - deletion renumbers consistently. - In protocol conversion, increment shared image index for remote images too, so mixed remote/local image tags stay in a single sequence. - Simplify restore logic to trust in-memory attachment order (no placeholder-number parsing path). - Backtrack/replay rollback handling now queues trims through `AppEvent::ApplyThreadRollback` and syncs transcript overlay/deferred lines after trims, so overlay/transcript state stays consistent. - Trim trailing blank rendered lines from user history rendering to avoid oversized blank padding. ## Docs + tests - Updated: `docs/tui-chat-composer.md` (remote image flow, selection/deletion, numbering offsets) - Added/updated tests across `tui/src/chatwidget/tests.rs`, `tui/src/app.rs`, `tui/src/app_backtrack.rs`, `tui/src/history_cell.rs`, and `tui/src/bottom_pane/chat_composer.rs` - Added snapshot coverage for remote image composer states, including deleting the first of two remote images. ## Validation - `just fmt` - `cargo test -p codex-tui` ## Codex author `codex fork 019c2636-1571-74a1-8471-15a3b1c3f49d`
This commit is contained in:
committed by
GitHub
parent
395729910c
commit
26a7cd21e2
@@ -2862,6 +2862,7 @@ impl App {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::app_backtrack::BacktrackSelection;
|
||||
use crate::app_backtrack::BacktrackState;
|
||||
use crate::app_backtrack::user_count;
|
||||
use crate::chatwidget::tests::make_chatwidget_manual_with_sender;
|
||||
@@ -2884,6 +2885,8 @@ mod tests {
|
||||
use codex_otel::OtelManager;
|
||||
use codex_protocol::ThreadId;
|
||||
use codex_protocol::user_input::TextElement;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use insta::assert_snapshot;
|
||||
use pretty_assertions::assert_eq;
|
||||
use ratatui::prelude::Line;
|
||||
@@ -3427,12 +3430,14 @@ mod tests {
|
||||
|
||||
let user_cell = |text: &str,
|
||||
text_elements: Vec<TextElement>,
|
||||
local_image_paths: Vec<PathBuf>|
|
||||
local_image_paths: Vec<PathBuf>,
|
||||
remote_image_urls: Vec<String>|
|
||||
-> Arc<dyn HistoryCell> {
|
||||
Arc::new(UserHistoryCell {
|
||||
message: text.to_string(),
|
||||
text_elements,
|
||||
local_image_paths,
|
||||
remote_image_urls,
|
||||
}) as Arc<dyn HistoryCell>
|
||||
};
|
||||
let agent_cell = |text: &str| -> Arc<dyn HistoryCell> {
|
||||
@@ -3478,17 +3483,18 @@ mod tests {
|
||||
// and an edited turn appended after a session header boundary.
|
||||
app.transcript_cells = vec![
|
||||
make_header(true),
|
||||
user_cell("first question", Vec::new(), Vec::new()),
|
||||
user_cell("first question", Vec::new(), Vec::new(), Vec::new()),
|
||||
agent_cell("answer first"),
|
||||
user_cell("follow-up", Vec::new(), Vec::new()),
|
||||
user_cell("follow-up", Vec::new(), Vec::new(), Vec::new()),
|
||||
agent_cell("answer follow-up"),
|
||||
make_header(false),
|
||||
user_cell("first question", Vec::new(), Vec::new()),
|
||||
user_cell("first question", Vec::new(), Vec::new(), Vec::new()),
|
||||
agent_cell("answer first"),
|
||||
user_cell(
|
||||
&edited_text,
|
||||
edited_text_elements.clone(),
|
||||
edited_local_image_paths.clone(),
|
||||
vec!["https://example.com/backtrack.png".to_string()],
|
||||
),
|
||||
agent_cell("answer edited"),
|
||||
];
|
||||
@@ -3527,8 +3533,16 @@ mod tests {
|
||||
assert_eq!(selection.prefill, edited_text);
|
||||
assert_eq!(selection.text_elements, edited_text_elements);
|
||||
assert_eq!(selection.local_image_paths, edited_local_image_paths);
|
||||
assert_eq!(
|
||||
selection.remote_image_urls,
|
||||
vec!["https://example.com/backtrack.png".to_string()]
|
||||
);
|
||||
|
||||
app.apply_backtrack_rollback(selection);
|
||||
assert_eq!(
|
||||
app.chat_widget.remote_image_urls(),
|
||||
vec!["https://example.com/backtrack.png".to_string()]
|
||||
);
|
||||
|
||||
let mut rollback_turns = None;
|
||||
while let Ok(op) = op_rx.try_recv() {
|
||||
@@ -3540,6 +3554,104 @@ mod tests {
|
||||
assert_eq!(rollback_turns, Some(1));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn backtrack_remote_image_only_selection_clears_existing_composer_draft() {
|
||||
let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await;
|
||||
|
||||
app.transcript_cells = vec![Arc::new(UserHistoryCell {
|
||||
message: "original".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
}) as Arc<dyn HistoryCell>];
|
||||
app.chat_widget
|
||||
.set_composer_text("stale draft".to_string(), Vec::new(), Vec::new());
|
||||
|
||||
let remote_image_url = "https://example.com/remote-only.png".to_string();
|
||||
app.apply_backtrack_rollback(BacktrackSelection {
|
||||
nth_user_message: 0,
|
||||
prefill: String::new(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: vec![remote_image_url.clone()],
|
||||
});
|
||||
|
||||
assert_eq!(app.chat_widget.composer_text_with_pending(), "");
|
||||
assert_eq!(app.chat_widget.remote_image_urls(), vec![remote_image_url]);
|
||||
|
||||
let mut rollback_turns = None;
|
||||
while let Ok(op) = op_rx.try_recv() {
|
||||
if let Op::ThreadRollback { num_turns } = op {
|
||||
rollback_turns = Some(num_turns);
|
||||
}
|
||||
}
|
||||
assert_eq!(rollback_turns, Some(1));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn backtrack_resubmit_preserves_data_image_urls_in_user_turn() {
|
||||
let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await;
|
||||
|
||||
let thread_id = ThreadId::new();
|
||||
app.chat_widget.handle_codex_event(Event {
|
||||
id: String::new(),
|
||||
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
|
||||
session_id: thread_id,
|
||||
forked_from_id: None,
|
||||
thread_name: None,
|
||||
model: "gpt-test".to_string(),
|
||||
model_provider_id: "test-provider".to_string(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
cwd: PathBuf::from("/home/user/project"),
|
||||
reasoning_effort: None,
|
||||
history_log_id: 0,
|
||||
history_entry_count: 0,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: Some(PathBuf::new()),
|
||||
}),
|
||||
});
|
||||
|
||||
let data_image_url = "data:image/png;base64,abc123".to_string();
|
||||
app.transcript_cells = vec![Arc::new(UserHistoryCell {
|
||||
message: "please inspect this".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: vec![data_image_url.clone()],
|
||||
}) as Arc<dyn HistoryCell>];
|
||||
|
||||
app.apply_backtrack_rollback(BacktrackSelection {
|
||||
nth_user_message: 0,
|
||||
prefill: "please inspect this".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: vec![data_image_url.clone()],
|
||||
});
|
||||
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
let mut saw_rollback = false;
|
||||
let mut submitted_items: Option<Vec<UserInput>> = None;
|
||||
while let Ok(op) = op_rx.try_recv() {
|
||||
match op {
|
||||
Op::ThreadRollback { .. } => saw_rollback = true,
|
||||
Op::UserTurn { items, .. } => submitted_items = Some(items),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
assert!(saw_rollback);
|
||||
let items = submitted_items.expect("expected user turn after backtrack resubmit");
|
||||
assert!(items.iter().any(|item| {
|
||||
matches!(
|
||||
item,
|
||||
UserInput::Image { image_url } if image_url == &data_image_url
|
||||
)
|
||||
}));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn replayed_initial_messages_apply_rollback_in_queue_order() {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
@@ -3702,6 +3814,7 @@ mod tests {
|
||||
message: "first".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
}) as Arc<dyn HistoryCell>,
|
||||
Arc::new(AgentMessageCell::new(
|
||||
vec![Line::from("after first")],
|
||||
@@ -3711,6 +3824,7 @@ mod tests {
|
||||
message: "second".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
}) as Arc<dyn HistoryCell>,
|
||||
Arc::new(AgentMessageCell::new(
|
||||
vec![Line::from("after second")],
|
||||
|
||||
Reference in New Issue
Block a user