mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix(tui): keep /copy aligned with rollback (#18739)
## Why Fixes #18718. After rewinding a thread, `/copy` could still copy the latest assistant response from before the rewind. The transcript cells were rolled back, but the copy source was a single `last_agent_markdown` cache that was not synchronized with backtracking, so the visible conversation and copied content could diverge. ## What changed `ChatWidget` now keeps a bounded copy history for the most recent 32 assistant responses, keyed by the visible user-turn count. When local rollback trims transcript cells, the copy cache is trimmed to the same surviving user-turn count so `/copy` uses the latest visible assistant response. If the user rewinds past the retained copy window, `/copy` now reports: ```text Cannot copy that response after rewinding. Only the most recent 32 responses are available to /copy. ``` The change also adds coverage for copying the latest surviving response after rollback and for the over-limit rewind message. ## Verification - Manually resumed a synthetic 35-turn session, rewound within the retained window, and verified `/copy` copied the surviving response. - Manually rewound past the retained window and verified `/copy` showed the 32-response limit message. - `cargo test -p codex-tui slash_copy` - `just fix -p codex-tui` - `cargo insta pending-snapshots` Note: `cargo test -p codex-tui` currently fails on unrelated model catalog and snapshot drift around the default model changing to `gpt-5.4`; the focused `/copy` tests pass after fixing the new test setup.
This commit is contained in:
committed by
GitHub
Unverified
parent
46e5814f77
commit
cebe57b723
@@ -482,6 +482,8 @@ impl App {
|
||||
if !trim_transcript_cells_drop_last_n_user_turns(&mut self.transcript_cells, num_turns) {
|
||||
return false;
|
||||
}
|
||||
self.chat_widget
|
||||
.truncate_agent_copy_history_to_user_turn_count(user_count(&self.transcript_cells));
|
||||
self.sync_overlay_after_transcript_trim();
|
||||
self.backtrack_render_pending = true;
|
||||
true
|
||||
@@ -503,6 +505,8 @@ impl App {
|
||||
&mut self.transcript_cells,
|
||||
pending.selection.nth_user_message,
|
||||
) {
|
||||
self.chat_widget
|
||||
.truncate_agent_copy_history_to_user_turn_count(user_count(&self.transcript_cells));
|
||||
self.sync_overlay_after_transcript_trim();
|
||||
self.backtrack_render_pending = true;
|
||||
}
|
||||
|
||||
@@ -473,6 +473,13 @@ fn is_standard_tool_call(parsed_cmd: &[ParsedCommand]) -> bool {
|
||||
const RATE_LIMIT_WARNING_THRESHOLDS: [f64; 3] = [75.0, 90.0, 95.0];
|
||||
const NUDGE_MODEL_SLUG: &str = "gpt-5.4-mini";
|
||||
const RATE_LIMIT_SWITCH_PROMPT_THRESHOLD: f64 = 90.0;
|
||||
const MAX_AGENT_COPY_HISTORY: usize = 32;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct AgentTurnMarkdown {
|
||||
user_turn_count: usize,
|
||||
markdown: String,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct RateLimitWarningState {
|
||||
@@ -795,13 +802,17 @@ pub(crate) struct ChatWidget {
|
||||
plan_stream_controller: Option<PlanStreamController>,
|
||||
/// Holds the platform clipboard lease so copied text remains available while supported.
|
||||
clipboard_lease: Option<crate::clipboard_copy::ClipboardLease>,
|
||||
/// Raw markdown of the most recently completed agent response.
|
||||
///
|
||||
/// This cache is intentionally best-effort: if the user rolls back the
|
||||
/// thread and then copies before a replacement response arrives, `/copy`
|
||||
/// may still return the response from before the rollback. Keeping this as
|
||||
/// a single cache avoids coupling copy state to the backtrack transcript.
|
||||
/// Raw markdown of the most recently completed agent response that
|
||||
/// survived any local thread rollback.
|
||||
last_agent_markdown: Option<String>,
|
||||
/// Copyable agent responses keyed by the number of visible user turns at
|
||||
/// the time the response completed.
|
||||
agent_turn_markdowns: Vec<AgentTurnMarkdown>,
|
||||
/// Number of user turns currently reflected in the visible transcript.
|
||||
visible_user_turn_count: usize,
|
||||
/// True when rollback discarded the requested copy source because it was
|
||||
/// older than the retained copy history.
|
||||
copy_history_evicted_by_rollback: bool,
|
||||
/// Raw markdown of the most recently completed proposed plan.
|
||||
///
|
||||
/// This is cached only for the approval popup. It is reset at the start of each new task so the
|
||||
@@ -2045,10 +2056,30 @@ impl ChatWidget {
|
||||
if message.is_empty() {
|
||||
return;
|
||||
}
|
||||
self.last_agent_markdown = Some(message.to_string());
|
||||
let markdown = message.to_string();
|
||||
match self.agent_turn_markdowns.last_mut() {
|
||||
Some(entry) if entry.user_turn_count == self.visible_user_turn_count => {
|
||||
entry.markdown = markdown.clone();
|
||||
}
|
||||
_ => {
|
||||
self.agent_turn_markdowns.push(AgentTurnMarkdown {
|
||||
user_turn_count: self.visible_user_turn_count,
|
||||
markdown: markdown.clone(),
|
||||
});
|
||||
if self.agent_turn_markdowns.len() > MAX_AGENT_COPY_HISTORY {
|
||||
self.agent_turn_markdowns.remove(0);
|
||||
}
|
||||
}
|
||||
}
|
||||
self.last_agent_markdown = Some(markdown);
|
||||
self.copy_history_evicted_by_rollback = false;
|
||||
self.saw_copy_source_this_turn = true;
|
||||
}
|
||||
|
||||
fn record_visible_user_turn_for_copy(&mut self) {
|
||||
self.visible_user_turn_count = self.visible_user_turn_count.saturating_add(1);
|
||||
}
|
||||
|
||||
// --- Small event handlers ---
|
||||
fn on_session_configured(&mut self, event: codex_protocol::protocol::SessionConfiguredEvent) {
|
||||
self.on_session_configured_with_display(event, SessionConfiguredDisplay::Normal);
|
||||
@@ -2060,6 +2091,9 @@ impl ChatWidget {
|
||||
display: SessionConfiguredDisplay,
|
||||
) {
|
||||
self.last_agent_markdown = None;
|
||||
self.agent_turn_markdowns.clear();
|
||||
self.visible_user_turn_count = 0;
|
||||
self.copy_history_evicted_by_rollback = false;
|
||||
self.saw_copy_source_this_turn = false;
|
||||
self.bottom_pane
|
||||
.set_history_metadata(event.history_log_id, event.history_entry_count);
|
||||
@@ -5064,6 +5098,9 @@ impl ChatWidget {
|
||||
agent_turn_running: false,
|
||||
mcp_startup_status: None,
|
||||
last_agent_markdown: None,
|
||||
agent_turn_markdowns: Vec::new(),
|
||||
visible_user_turn_count: 0,
|
||||
copy_history_evicted_by_rollback: false,
|
||||
latest_proposed_plan_markdown: None,
|
||||
saw_copy_source_this_turn: false,
|
||||
mcp_startup_expected_servers: None,
|
||||
@@ -5432,6 +5469,23 @@ impl ChatWidget {
|
||||
self.copy_last_agent_markdown_with(crate::clipboard_copy::copy_to_clipboard);
|
||||
}
|
||||
|
||||
pub(crate) fn truncate_agent_copy_history_to_user_turn_count(
|
||||
&mut self,
|
||||
user_turn_count: usize,
|
||||
) {
|
||||
self.visible_user_turn_count = user_turn_count;
|
||||
let had_copy_history = !self.agent_turn_markdowns.is_empty();
|
||||
self.agent_turn_markdowns
|
||||
.retain(|entry| entry.user_turn_count <= user_turn_count);
|
||||
self.last_agent_markdown = self
|
||||
.agent_turn_markdowns
|
||||
.last()
|
||||
.map(|entry| entry.markdown.clone());
|
||||
self.copy_history_evicted_by_rollback =
|
||||
had_copy_history && self.last_agent_markdown.is_none();
|
||||
self.saw_copy_source_this_turn = false;
|
||||
}
|
||||
|
||||
/// Inner implementation with an injectable clipboard backend for testing.
|
||||
fn copy_last_agent_markdown_with(
|
||||
&mut self,
|
||||
@@ -5450,6 +5504,11 @@ impl ChatWidget {
|
||||
"Copy failed: {error}"
|
||||
))),
|
||||
},
|
||||
_ if self.copy_history_evicted_by_rollback => {
|
||||
self.add_to_history(history_cell::new_error_event(format!(
|
||||
"Cannot copy that response after rewinding. Only the most recent {MAX_AGENT_COPY_HISTORY} responses are available to /copy."
|
||||
)));
|
||||
}
|
||||
_ => self.add_to_history(history_cell::new_error_event(
|
||||
"No agent response to copy".into(),
|
||||
)),
|
||||
@@ -5865,6 +5924,7 @@ impl ChatWidget {
|
||||
local_image_paths.clone(),
|
||||
remote_image_urls.clone(),
|
||||
));
|
||||
self.record_visible_user_turn_for_copy();
|
||||
self.add_to_history(history_cell::new_user_prompt(
|
||||
text,
|
||||
text_elements,
|
||||
@@ -5879,6 +5939,7 @@ impl ChatWidget {
|
||||
Vec::new(),
|
||||
remote_image_urls.clone(),
|
||||
));
|
||||
self.record_visible_user_turn_for_copy();
|
||||
self.add_to_history(history_cell::new_user_prompt(
|
||||
String::new(),
|
||||
Vec::new(),
|
||||
@@ -7248,6 +7309,7 @@ impl ChatWidget {
|
||||
|| !event.text_elements.is_empty()
|
||||
|| !remote_image_urls.is_empty()
|
||||
{
|
||||
self.record_visible_user_turn_for_copy();
|
||||
self.add_to_history(history_cell::new_user_prompt(
|
||||
event.message,
|
||||
event.text_elements,
|
||||
|
||||
@@ -212,6 +212,9 @@ pub(super) async fn make_chatwidget_manual(
|
||||
pending_guardian_review_status: PendingGuardianReviewStatus::default(),
|
||||
terminal_title_status_kind: TerminalTitleStatusKind::Working,
|
||||
last_agent_markdown: None,
|
||||
agent_turn_markdowns: Vec::new(),
|
||||
visible_user_turn_count: 0,
|
||||
copy_history_evicted_by_rollback: false,
|
||||
latest_proposed_plan_markdown: None,
|
||||
saw_copy_source_this_turn: false,
|
||||
running_commands: HashMap::new(),
|
||||
|
||||
@@ -1029,6 +1029,92 @@ async fn agent_turn_complete_notification_does_not_reuse_stale_copy_source() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_copy_uses_latest_surviving_response_after_rollback() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
chat.handle_codex_event_replay(Event {
|
||||
id: "user-1".into(),
|
||||
msg: EventMsg::UserMessage(UserMessageEvent {
|
||||
message: "foo".to_string(),
|
||||
images: None,
|
||||
local_images: Vec::new(),
|
||||
text_elements: Vec::new(),
|
||||
}),
|
||||
});
|
||||
chat.handle_codex_event_replay(Event {
|
||||
id: "agent-1".into(),
|
||||
msg: EventMsg::AgentMessage(AgentMessageEvent {
|
||||
message: "foo response".to_string(),
|
||||
phase: None,
|
||||
memory_citation: None,
|
||||
}),
|
||||
});
|
||||
chat.handle_codex_event_replay(Event {
|
||||
id: "user-2".into(),
|
||||
msg: EventMsg::UserMessage(UserMessageEvent {
|
||||
message: "bar".to_string(),
|
||||
images: None,
|
||||
local_images: Vec::new(),
|
||||
text_elements: Vec::new(),
|
||||
}),
|
||||
});
|
||||
chat.handle_codex_event_replay(Event {
|
||||
id: "agent-2".into(),
|
||||
msg: EventMsg::AgentMessage(AgentMessageEvent {
|
||||
message: "bar response".to_string(),
|
||||
phase: None,
|
||||
memory_citation: None,
|
||||
}),
|
||||
});
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
assert_eq!(chat.last_agent_markdown_text(), Some("bar response"));
|
||||
|
||||
chat.truncate_agent_copy_history_to_user_turn_count(/*user_turn_count*/ 1);
|
||||
|
||||
assert_eq!(chat.last_agent_markdown_text(), Some("foo response"));
|
||||
chat.copy_last_agent_markdown_with(|markdown| {
|
||||
assert_eq!(markdown, "foo response");
|
||||
Ok(None)
|
||||
});
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_copy_reports_when_rewind_exceeds_retained_copy_history() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
chat.handle_codex_event_replay(Event {
|
||||
id: "user-1".into(),
|
||||
msg: EventMsg::UserMessage(UserMessageEvent {
|
||||
message: "foo".to_string(),
|
||||
images: None,
|
||||
local_images: Vec::new(),
|
||||
text_elements: Vec::new(),
|
||||
}),
|
||||
});
|
||||
chat.handle_codex_event_replay(Event {
|
||||
id: "agent-1".into(),
|
||||
msg: EventMsg::AgentMessage(AgentMessageEvent {
|
||||
message: "foo response".to_string(),
|
||||
phase: None,
|
||||
memory_citation: None,
|
||||
}),
|
||||
});
|
||||
let _ = drain_insert_history(&mut rx);
|
||||
|
||||
chat.truncate_agent_copy_history_to_user_turn_count(/*user_turn_count*/ 0);
|
||||
chat.dispatch_command(SlashCommand::Copy);
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains(
|
||||
"Cannot copy that response after rewinding. Only the most recent 32 responses are available to /copy."
|
||||
),
|
||||
"expected evicted-history message, got {rendered:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_exit_requests_exit() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
|
||||
Reference in New Issue
Block a user