This commit is contained in:
jimmyfraiture
2025-09-09 14:15:45 -07:00
parent ace14e8d36
commit 2a3bc78f98
11 changed files with 542 additions and 7 deletions
+12
View File
@@ -732,13 +732,21 @@ impl Session {
Some(ApplyPatchCommandContext {
user_explicitly_approved_this_action,
changes,
patch,
cwd,
}) => {
debug!(
"applying patch ({} bytes) in {}",
patch.len(),
cwd.display()
);
turn_diff_tracker.on_patch_begin(&changes);
EventMsg::PatchApplyBegin(PatchApplyBeginEvent {
call_id,
auto_approved: !user_explicitly_approved_this_action,
changes,
cwd,
})
}
None => EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
@@ -992,6 +1000,8 @@ pub(crate) struct ExecCommandContext {
pub(crate) struct ApplyPatchCommandContext {
pub(crate) user_explicitly_approved_this_action: bool,
pub(crate) changes: HashMap<PathBuf, FileChange>,
pub(crate) patch: String,
pub(crate) cwd: PathBuf,
}
/// A series of Turns in response to user input.
@@ -2512,6 +2522,8 @@ async fn handle_container_exec_with_params(
}| ApplyPatchCommandContext {
user_explicitly_approved_this_action,
changes: convert_apply_patch_to_protocol(&action),
patch: action.patch.clone(),
cwd: action.cwd.clone(),
},
),
};
@@ -378,6 +378,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
call_id,
auto_approved,
changes,
cwd,
}) => {
// Store metadata so we can calculate duration later when we
// receive the corresponding PatchApplyEnd event.
@@ -391,9 +392,10 @@ impl EventProcessor for EventProcessorWithHumanOutput {
ts_println!(
self,
"{} auto_approved={}:",
"{} auto_approved={} in {}:",
"apply_patch".style(self.magenta),
auto_approved,
cwd.to_string_lossy(),
);
// Pretty-print the patch summary with colored diff markers so
+2
View File
@@ -899,6 +899,8 @@ pub struct PatchApplyBeginEvent {
pub auto_approved: bool,
/// The changes to be applied.
pub changes: HashMap<PathBuf, FileChange>,
/// The working directory the patch was applied from.
pub cwd: PathBuf,
}
#[derive(Debug, Clone, Deserialize, Serialize, TS)]
+11
View File
@@ -279,6 +279,17 @@ impl App {
));
tui.frame_requester().schedule_frame();
}
AppEvent::PatchUndoResult {
success,
stdout,
stderr,
} => {
self.chat_widget
.on_patch_undo_result(success, stdout, stderr);
}
AppEvent::UndoConfirmationSelected { confirm } => {
self.chat_widget.on_undo_confirmation(confirm);
}
AppEvent::StartFileSearch(query) => {
if !query.is_empty() {
self.file_search.on_user_query(query);
+12
View File
@@ -39,6 +39,18 @@ pub(crate) enum AppEvent {
/// Result of computing a `/diff` command.
DiffResult(String),
/// Result of running `/undo` to revert the latest applied patch.
PatchUndoResult {
success: bool,
stdout: String,
stderr: String,
},
/// User selection from the undo confirmation prompt.
UndoConfirmationSelected {
confirm: bool,
},
InsertHistoryCell(Box<dyn HistoryCell>),
StartCommitAnimation,
+297 -6
View File
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::collections::VecDeque;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
@@ -18,6 +19,7 @@ use codex_core::protocol::EventMsg;
use codex_core::protocol::ExecApprovalRequestEvent;
use codex_core::protocol::ExecCommandBeginEvent;
use codex_core::protocol::ExecCommandEndEvent;
use codex_core::protocol::FileChange;
use codex_core::protocol::InputItem;
use codex_core::protocol::InputMessageKind;
use codex_core::protocol::ListCustomPromptsResponseEvent;
@@ -35,6 +37,7 @@ use codex_core::protocol::TurnDiffEvent;
use codex_core::protocol::UserMessageEvent;
use codex_core::protocol::WebSearchBeginEvent;
use codex_core::protocol::WebSearchEndEvent;
use codex_core::turn_diff_tracker::TurnDiffTracker;
use codex_protocol::parse_command::ParsedCommand;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
@@ -67,6 +70,7 @@ use crate::history_cell::HistoryCell;
use crate::history_cell::PatchEventType;
use crate::slash_command::SlashCommand;
use crate::tui::FrameRequester;
use crate::undo_patch;
// streaming internals are provided by crate::streaming and crate::markdown_stream
use crate::user_approval_widget::ApprovalRequest;
mod interrupts;
@@ -93,6 +97,147 @@ struct RunningCommand {
parsed_cmd: Vec<ParsedCommand>,
}
#[derive(Clone)]
struct AppliedPatchDiff {
diff: String,
working_dir: PathBuf,
}
struct PatchInFlight {
tracker: TurnDiffTracker,
working_dir: PathBuf,
}
#[derive(Default)]
struct PatchUndoHistory {
trackers: HashMap<String, PatchInFlight>,
active_turn: Vec<AppliedPatchDiff>,
completed_turns: Vec<Arc<[AppliedPatchDiff]>>,
undo_in_progress: Option<Arc<[AppliedPatchDiff]>>,
}
impl PatchUndoHistory {
fn new() -> Self {
Self::default()
}
fn start_turn(&mut self) {
self.active_turn.clear();
}
fn finish_turn(&mut self) -> Option<usize> {
let completed = self.take_active_turn()?;
let count = completed.len();
self.completed_turns.push(completed);
Some(count)
}
fn take_active_turn(&mut self) -> Option<Arc<[AppliedPatchDiff]>> {
(!self.active_turn.is_empty())
.then(|| Arc::from(self.active_turn.drain(..).collect::<Vec<_>>()))
}
fn start_patch(
&mut self,
call_id: String,
changes: HashMap<PathBuf, FileChange>,
patch_cwd: PathBuf,
default_cwd: &Path,
) {
let mut tracker = TurnDiffTracker::new();
tracker.on_patch_begin(&changes);
let working_dir = working_dir_for_patch(&changes, &patch_cwd, default_cwd);
self.trackers.insert(
call_id,
PatchInFlight {
tracker,
working_dir,
},
);
}
fn complete_patch(&mut self, call_id: &str, success: bool) {
let Some(PatchInFlight {
mut tracker,
working_dir,
}) = self.trackers.remove(call_id)
else {
return;
};
if !success {
return;
}
if let Ok(Some(diff)) = tracker.get_unified_diff() {
if diff.trim().is_empty() {
return;
}
self.active_turn
.push(AppliedPatchDiff { diff, working_dir });
}
}
fn begin_undo(&mut self) -> Option<Arc<[AppliedPatchDiff]>> {
if self.undo_in_progress.is_some() {
return None;
}
let turn = self
.completed_turns
.pop()
.or_else(|| self.take_active_turn())?;
self.undo_in_progress = Some(turn.clone());
Some(turn)
}
fn finish_undo(&mut self, success: bool) {
if let Some(turn) = self.undo_in_progress.take() {
if !success {
self.completed_turns.push(turn);
}
}
}
fn undo_in_progress(&self) -> bool {
self.undo_in_progress.is_some()
}
fn undo_available(&self) -> bool {
!self.completed_turns.is_empty() || !self.active_turn.is_empty()
}
}
fn find_git_root_for_changes(changes: &HashMap<PathBuf, FileChange>) -> Option<PathBuf> {
changes.keys().find_map(|path| find_git_root(path))
}
fn find_git_root(path: &Path) -> Option<PathBuf> {
let start = if path.is_dir() { path } else { path.parent()? };
for candidate in start.ancestors() {
let marker = candidate.join(".git");
if marker.is_dir() || marker.is_file() {
return Some(candidate.to_path_buf());
}
}
None
}
fn working_dir_for_patch(
changes: &HashMap<PathBuf, FileChange>,
patch_cwd: &Path,
default_cwd: &Path,
) -> PathBuf {
find_git_root_for_changes(changes)
.or_else(|| find_git_root(patch_cwd))
.or_else(|| patch_cwd.is_absolute().then(|| patch_cwd.to_path_buf()))
.unwrap_or_else(|| default_cwd.to_path_buf())
}
/// Common initialization parameters shared by all `ChatWidget` constructors.
pub(crate) struct ChatWidgetInit {
pub(crate) config: Config,
@@ -117,6 +262,7 @@ pub(crate) struct ChatWidget {
task_complete_pending: bool,
// Queue of interruptive UI events deferred during an active write cycle
interrupts: InterruptManager,
patch_history: PatchUndoHistory,
// Accumulates the current reasoning block text to extract a header
reasoning_buffer: String,
// Accumulates full reasoning content for transcript-only recording
@@ -235,6 +381,7 @@ impl ChatWidget {
// Raw reasoning uses the same flow as summarized reasoning
fn on_task_started(&mut self) {
self.patch_history.start_turn();
self.bottom_pane.clear_ctrl_c_quit_hint();
self.bottom_pane.set_task_running(true);
self.stream.reset_headers_for_new_turn();
@@ -250,6 +397,7 @@ impl ChatWidget {
let sink = AppEventHistorySink(self.app_event_tx.clone());
let _ = self.stream.finalize(true, &sink);
}
self.finalize_patch_turn();
// Mark task stopped and request redraw now that all content is in history.
self.bottom_pane.set_task_running(false);
self.running_commands.clear();
@@ -270,6 +418,7 @@ impl ChatWidget {
self.finalize_active_exec_cell_as_failed();
// Emit the provided error message/history cell.
self.add_to_history(history_cell::new_error_event(message));
self.finalize_patch_turn();
// Reset running state and clear streaming buffers.
self.bottom_pane.set_task_running(false);
self.running_commands.clear();
@@ -344,11 +493,19 @@ impl ChatWidget {
}
fn on_patch_apply_begin(&mut self, event: PatchApplyBeginEvent) {
let PatchApplyBeginEvent {
call_id,
auto_approved,
changes,
cwd,
} = event;
let history_changes = changes.clone();
self.patch_history
.start_patch(call_id, changes, cwd, &self.config.cwd);
self.add_to_history(history_cell::new_patch_event(
PatchEventType::ApplyBegin {
auto_approved: event.auto_approved,
},
event.changes,
PatchEventType::ApplyBegin { auto_approved },
history_changes,
&self.config.cwd,
));
}
@@ -510,8 +667,8 @@ impl ChatWidget {
&mut self,
event: codex_core::protocol::PatchApplyEndEvent,
) {
// If the patch was successful, just let the "Edited" block stand.
// Otherwise, add a failure block.
self.patch_history
.complete_patch(&event.call_id, event.success);
if !event.success {
self.add_to_history(history_cell::new_patch_apply_failure(event.stderr));
}
@@ -658,6 +815,7 @@ impl ChatWidget {
running_commands: HashMap::new(),
task_complete_pending: false,
interrupts: InterruptManager::new(),
patch_history: PatchUndoHistory::new(),
reasoning_buffer: String::new(),
full_reasoning_buffer: String::new(),
conversation_id: None,
@@ -710,6 +868,7 @@ impl ChatWidget {
running_commands: HashMap::new(),
task_complete_pending: false,
interrupts: InterruptManager::new(),
patch_history: PatchUndoHistory::new(),
reasoning_buffer: String::new(),
full_reasoning_buffer: String::new(),
conversation_id: None,
@@ -862,6 +1021,9 @@ impl ChatWidget {
tx.send(AppEvent::DiffResult(text));
});
}
SlashCommand::Undo => {
self.undo_last_patch();
}
SlashCommand::Mention => {
self.insert_str("@");
}
@@ -1140,10 +1302,139 @@ impl ChatWidget {
self.bottom_pane.set_queued_user_messages(messages);
}
fn finalize_patch_turn(&mut self) {
if let Some(patch_count) = self.patch_history.finish_turn() {
self.add_to_history(history_cell::new_patch_undo_available(patch_count));
}
}
pub(crate) fn add_diff_in_progress(&mut self) {
self.request_redraw();
}
fn undo_last_patch(&mut self) {
if self.patch_history.undo_in_progress() {
self.add_to_history(history_cell::new_patch_undo_busy());
self.request_redraw();
return;
}
if !self.patch_history.undo_available() {
self.add_to_history(history_cell::new_patch_undo_unavailable());
self.request_redraw();
return;
}
self.open_undo_confirmation_popup();
}
fn open_undo_confirmation_popup(&mut self) {
let confirm_action: SelectionAction = Box::new(|tx| {
tx.send(AppEvent::UndoConfirmationSelected { confirm: true });
});
let cancel_action: SelectionAction = Box::new(|tx| {
tx.send(AppEvent::UndoConfirmationSelected { confirm: false });
});
let items = vec![
SelectionItem {
name: "Yes — undo last turn".to_string(),
description: Some(
"Revert the patches Codex applied in the previous turn.".to_string(),
),
is_current: false,
actions: vec![confirm_action],
},
SelectionItem {
name: "No — keep changes".to_string(),
description: Some("Leave files as they are.".to_string()),
is_current: true,
actions: vec![cancel_action],
},
];
self.bottom_pane.show_selection_view(
"Undo patches from the last turn?".to_string(),
Some("This will apply git reverse patches and may discard edits.".to_string()),
Some("Press Enter to choose or Esc to cancel".to_string()),
items,
);
}
fn start_patch_undo(&mut self) {
if self.patch_history.undo_in_progress() {
self.add_to_history(history_cell::new_patch_undo_busy());
self.request_redraw();
return;
}
let Some(patches) = self.patch_history.begin_undo() else {
self.add_to_history(history_cell::new_patch_undo_unavailable());
self.request_redraw();
return;
};
self.add_to_history(history_cell::new_patch_undo_in_progress());
self.request_redraw();
let tx = self.app_event_tx.clone();
let patches = patches.clone();
tokio::spawn(async move {
let mut overall_success = true;
let mut combined_stdout = String::new();
let mut combined_stderr = String::new();
let append_output = |buffer: &mut String, output: &str| {
if output.is_empty() {
return;
}
if !buffer.is_empty() && !buffer.ends_with('\n') {
buffer.push('\n');
}
buffer.push_str(output);
};
for patch in patches.iter().rev() {
match undo_patch::undo_patch(&patch.diff, &patch.working_dir).await {
Ok(outcome) => {
append_output(&mut combined_stdout, &outcome.stdout);
append_output(&mut combined_stderr, &outcome.stderr);
if !outcome.success {
overall_success = false;
break;
}
}
Err(err) => {
overall_success = false;
append_output(&mut combined_stderr, &err.to_string());
break;
}
}
}
tx.send(AppEvent::PatchUndoResult {
success: overall_success,
stdout: combined_stdout,
stderr: combined_stderr,
});
});
}
pub(crate) fn on_undo_confirmation(&mut self, confirm: bool) {
if confirm {
self.start_patch_undo();
} else {
self.add_to_history(history_cell::new_patch_undo_cancelled());
self.request_redraw();
}
}
pub(crate) fn on_patch_undo_result(&mut self, success: bool, stdout: String, stderr: String) {
self.patch_history.finish_undo(success);
self.add_to_history(history_cell::new_patch_undo_result(success, stdout, stderr));
self.request_redraw();
}
pub(crate) fn on_diff_complete(&mut self) {
self.request_redraw();
}
+10
View File
@@ -68,6 +68,11 @@ fn upgrade_event_payload_for_tests(mut payload: serde_json::Value) -> serde_json
"formatted_output".to_string(),
serde_json::Value::String(formatted),
);
} else if ty == "patch_apply_begin" && !m.contains_key("cwd") {
m.insert(
"cwd".to_string(),
serde_json::Value::String("/tmp".to_string()),
);
}
}
payload
@@ -234,6 +239,7 @@ fn make_chatwidget_manual() -> (
running_commands: HashMap::new(),
task_complete_pending: false,
interrupts: InterruptManager::new(),
patch_history: PatchUndoHistory::new(),
reasoning_buffer: String::new(),
full_reasoning_buffer: String::new(),
conversation_id: None,
@@ -1103,6 +1109,7 @@ fn apply_patch_events_emit_history_cells() {
call_id: "c1".into(),
auto_approved: true,
changes: changes2,
cwd: PathBuf::from("/tmp"),
};
chat.handle_codex_event(Event {
id: "s1".into(),
@@ -1169,6 +1176,7 @@ fn apply_patch_manual_approval_adjusts_header() {
call_id: "c1".into(),
auto_approved: false,
changes: apply_changes,
cwd: PathBuf::from("/tmp"),
}),
});
@@ -1218,6 +1226,7 @@ fn apply_patch_manual_flow_snapshot() {
call_id: "c1".into(),
auto_approved: false,
changes: apply_changes,
cwd: PathBuf::from("/tmp"),
}),
});
let approved_lines = drain_insert_history(&mut rx)
@@ -1334,6 +1343,7 @@ fn apply_patch_full_flow_integration_like() {
call_id: "call-1".into(),
auto_approved: false,
changes: changes2,
cwd: PathBuf::from("/tmp"),
}),
});
chat.handle_codex_event(Event {
+73
View File
@@ -1170,6 +1170,79 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
PlainHistoryCell { lines }
}
pub(crate) fn new_patch_undo_available(patch_count: usize) -> PlainHistoryCell {
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push("↺ Undo available for last turn".magenta().bold().into());
if patch_count <= 1 {
lines.push("Type /undo to revert this patch.".dim().into());
} else {
lines.push(
format!("Type /undo to revert {patch_count} patches from the last turn.")
.dim()
.into(),
);
}
PlainHistoryCell { lines }
}
pub(crate) fn new_patch_undo_unavailable() -> PlainHistoryCell {
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push("No applied patch to undo.".dim().into());
PlainHistoryCell { lines }
}
pub(crate) fn new_patch_undo_busy() -> PlainHistoryCell {
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push("Undo already running".magenta().bold().into());
lines.push(
"Wait for the previous undo to finish before starting another."
.dim()
.into(),
);
PlainHistoryCell { lines }
}
pub(crate) fn new_patch_undo_in_progress() -> PlainHistoryCell {
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push("↺ Undoing latest patch…".magenta().bold().into());
PlainHistoryCell { lines }
}
pub(crate) fn new_patch_undo_cancelled() -> PlainHistoryCell {
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push("↺ Undo cancelled".magenta().bold().into());
lines.push("Keeping the last turn's changes.".dim().into());
PlainHistoryCell { lines }
}
pub(crate) fn new_patch_undo_result(
success: bool,
stdout: String,
stderr: String,
) -> PlainHistoryCell {
let mut lines: Vec<Line<'static>> = Vec::new();
if success {
lines.push("✓ Undo complete".green().bold().into());
} else {
lines.push("✘ Failed to undo patch".red().bold().into());
}
let push_block = |lines: &mut Vec<Line<'static>>, title: &str, body: &str| {
if body.trim().is_empty() {
return;
}
lines.push(title.to_string().dim().into());
for line in body.lines() {
lines.push(format!(" {line}").into());
}
};
push_block(&mut lines, "stdout:", &stdout);
push_block(&mut lines, "stderr:", &stderr);
PlainHistoryCell { lines }
}
/// Create a new history cell for a proposed command approval.
/// Renders a header and the command preview similar to how proposed patches
/// show a header and summary.
+1
View File
@@ -57,6 +57,7 @@ mod status_indicator_widget;
mod streaming;
mod text_formatting;
mod tui;
mod undo_patch;
mod user_approval_widget;
mod version;
mod wrapping;
+3
View File
@@ -18,6 +18,7 @@ pub enum SlashCommand {
Init,
Compact,
Diff,
Undo,
Mention,
Status,
Mcp,
@@ -36,6 +37,7 @@ impl SlashCommand {
SlashCommand::Compact => "summarize conversation to prevent hitting the context limit",
SlashCommand::Quit => "exit Codex",
SlashCommand::Diff => "show git diff (including untracked files)",
SlashCommand::Undo => "revert the last patch applied",
SlashCommand::Mention => "mention a file",
SlashCommand::Status => "show current session configuration and token usage",
SlashCommand::Model => "choose what model and reasoning effort to use",
@@ -63,6 +65,7 @@ impl SlashCommand {
| SlashCommand::Approvals
| SlashCommand::Logout => false,
SlashCommand::Diff
| SlashCommand::Undo
| SlashCommand::Mention
| SlashCommand::Status
| SlashCommand::Mcp
+118
View File
@@ -0,0 +1,118 @@
use std::io;
use std::path::Path;
use std::process::Stdio;
use tokio::io::AsyncWriteExt;
use tokio::process::Command;
#[derive(Debug, Default)]
pub(crate) struct UndoPatchResult {
pub stdout: String,
pub stderr: String,
pub success: bool,
}
async fn run_git_apply(diff: &str, cwd: &Path, args: &[&str]) -> io::Result<UndoPatchResult> {
let mut command = Command::new("git");
command
.args(args)
.current_dir(cwd)
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped());
let mut child = command.spawn()?;
if let Some(stdin) = child.stdin.as_mut() {
stdin.write_all(diff.as_bytes()).await?;
}
let output = child.wait_with_output().await?;
Ok(UndoPatchResult {
stdout: String::from_utf8_lossy(&output.stdout).into_owned(),
stderr: String::from_utf8_lossy(&output.stderr).into_owned(),
success: output.status.success(),
})
}
pub(crate) async fn undo_patch(diff: &str, cwd: &Path) -> io::Result<UndoPatchResult> {
const UNDO_ARGS: [&[&str]; 2] = [&["apply", "-R"], &["apply", "--3way", "-R"]];
let mut last_result = UndoPatchResult::default();
for args in UNDO_ARGS {
let result = run_git_apply(diff, cwd, args).await?;
if result.success {
return Ok(result);
}
last_result = result;
}
Ok(last_result)
}
#[cfg(test)]
mod tests {
use super::*;
use std::fs;
use std::process::Command;
use tempfile::tempdir;
fn run_git(cwd: &Path, args: &[&str]) -> io::Result<()> {
let status = Command::new("git").args(args).current_dir(cwd).status()?;
if status.success() {
Ok(())
} else {
Err(io::Error::new(
io::ErrorKind::Other,
format!("git {:?} failed with status {status}", args),
))
}
}
fn git_output(cwd: &Path, args: &[&str]) -> io::Result<String> {
let output = Command::new("git").args(args).current_dir(cwd).output()?;
if output.status.success() {
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
} else {
Err(io::Error::new(
io::ErrorKind::Other,
format!(
"git {:?} failed: {}",
args,
String::from_utf8_lossy(&output.stderr)
),
))
}
}
#[tokio::test(flavor = "current_thread")]
async fn undo_patch_reverts_changes() -> io::Result<()> {
let dir = tempdir()?;
let root = dir.path();
run_git(root, &["init"])?;
run_git(root, &["config", "user.email", "codex@example.com"])?;
run_git(root, &["config", "user.name", "Codex"])?;
let file = root.join("foo.txt");
fs::write(&file, "hello\n")?;
run_git(root, &["add", "foo.txt"])?;
run_git(root, &["commit", "-m", "initial"])?;
fs::write(&file, "hello\nworld\n")?;
let diff = git_output(root, &["diff", "HEAD", "--", "foo.txt"])?;
assert!(diff.contains("diff --git"));
let result = undo_patch(&diff, root).await?;
assert!(
result.success,
"Expected undo to succeed: {}",
result.stderr
);
assert_eq!(fs::read_to_string(&file)?, "hello\n");
let second_result = undo_patch(&diff, root).await?;
assert!(!second_result.success, "second undo should fail");
Ok(())
}
}