mirror of
https://github.com/openai/codex.git
synced 2026-02-01 22:47:52 +00:00
Compare commits
4 Commits
compaction
...
fraiture/u
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
03c287f921 | ||
|
|
82ce78b1ea | ||
|
|
3150f50c63 | ||
|
|
2a3bc78f98 |
@@ -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 {
|
||||
@@ -764,7 +772,7 @@ impl Session {
|
||||
sub_id: &str,
|
||||
call_id: &str,
|
||||
output: &ExecToolCallOutput,
|
||||
is_apply_patch: bool,
|
||||
apply_patch: Option<&ApplyPatchCommandContext>,
|
||||
) {
|
||||
let ExecToolCallOutput {
|
||||
stdout,
|
||||
@@ -779,12 +787,25 @@ impl Session {
|
||||
let formatted_output = format_exec_output_str(output);
|
||||
let aggregated_output: String = aggregated_output.text.clone();
|
||||
|
||||
let msg = if is_apply_patch {
|
||||
let msg = if let Some(apply_patch) = apply_patch {
|
||||
let diff = if *exit_code == 0 {
|
||||
match turn_diff_tracker.get_unified_diff_for_changes(&apply_patch.changes) {
|
||||
Ok(diff) => diff,
|
||||
Err(err) => {
|
||||
warn!("failed to compute patch diff: {err:#}");
|
||||
None
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
EventMsg::PatchApplyEnd(PatchApplyEndEvent {
|
||||
call_id: call_id.to_string(),
|
||||
stdout,
|
||||
stderr,
|
||||
success: *exit_code == 0,
|
||||
diff,
|
||||
})
|
||||
} else {
|
||||
EventMsg::ExecCommandEnd(ExecCommandEndEvent {
|
||||
@@ -806,7 +827,7 @@ impl Session {
|
||||
|
||||
// If this is an apply_patch, after we emit the end patch, emit a second event
|
||||
// with the full turn diff if there is one.
|
||||
if is_apply_patch {
|
||||
if apply_patch.is_some() {
|
||||
let unified_diff = turn_diff_tracker.get_unified_diff();
|
||||
if let Ok(Some(unified_diff)) = unified_diff {
|
||||
let msg = EventMsg::TurnDiff(TurnDiffEvent { unified_diff });
|
||||
@@ -828,7 +849,7 @@ impl Session {
|
||||
begin_ctx: ExecCommandContext,
|
||||
exec_args: ExecInvokeArgs<'a>,
|
||||
) -> crate::error::Result<ExecToolCallOutput> {
|
||||
let is_apply_patch = begin_ctx.apply_patch.is_some();
|
||||
let apply_patch_ctx = begin_ctx.apply_patch.clone();
|
||||
let sub_id = begin_ctx.sub_id.clone();
|
||||
let call_id = begin_ctx.call_id.clone();
|
||||
|
||||
@@ -863,7 +884,7 @@ impl Session {
|
||||
&sub_id,
|
||||
&call_id,
|
||||
borrowed,
|
||||
is_apply_patch,
|
||||
apply_patch_ctx.as_ref(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -992,6 +1013,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 +2535,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(),
|
||||
},
|
||||
),
|
||||
};
|
||||
|
||||
@@ -249,6 +249,56 @@ impl TurnDiffTracker {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_unified_diff_for_changes(
|
||||
&mut self,
|
||||
changes: &HashMap<PathBuf, FileChange>,
|
||||
) -> Result<Option<String>> {
|
||||
let mut internal_names: Vec<String> = changes
|
||||
.iter()
|
||||
.flat_map(|(path, change)| {
|
||||
let mut names = Vec::new();
|
||||
if let Some(internal) = self.external_to_temp_name.get(path) {
|
||||
names.push(internal.clone());
|
||||
}
|
||||
if let FileChange::Update {
|
||||
move_path: Some(dest),
|
||||
..
|
||||
} = change
|
||||
&& let Some(internal) = self.external_to_temp_name.get(dest)
|
||||
{
|
||||
names.push(internal.clone());
|
||||
}
|
||||
names
|
||||
})
|
||||
.collect();
|
||||
|
||||
if internal_names.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
internal_names.sort();
|
||||
internal_names.dedup();
|
||||
internal_names.sort_by_key(|internal| {
|
||||
self.get_path_for_internal(internal)
|
||||
.map(|p| self.relative_to_git_root_str(&p))
|
||||
.unwrap_or_default()
|
||||
});
|
||||
|
||||
let mut aggregated = String::new();
|
||||
for internal in internal_names {
|
||||
aggregated.push_str(self.get_file_diff(&internal).as_str());
|
||||
if !aggregated.ends_with('\n') {
|
||||
aggregated.push('\n');
|
||||
}
|
||||
}
|
||||
|
||||
if aggregated.trim().is_empty() {
|
||||
Ok(None)
|
||||
} else {
|
||||
Ok(Some(aggregated))
|
||||
}
|
||||
}
|
||||
|
||||
fn get_file_diff(&mut self, internal_file_name: &str) -> String {
|
||||
let mut aggregated = String::new();
|
||||
|
||||
@@ -784,6 +834,9 @@ index {left_oid_b}..{ZERO_OID}
|
||||
assert_eq!(combined, expected);
|
||||
}
|
||||
|
||||
/// Confirms that updating a binary file (non-UTF8 content) produces a
|
||||
/// unified diff that reports "Binary files differ" with correct blob OIDs
|
||||
/// instead of a textual hunk.
|
||||
#[test]
|
||||
fn binary_files_differ_update() {
|
||||
let dir = tempdir().unwrap();
|
||||
|
||||
@@ -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
|
||||
@@ -461,7 +463,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
|
||||
stdout,
|
||||
stderr,
|
||||
success,
|
||||
..
|
||||
diff: _diff,
|
||||
}) => {
|
||||
let patch_begin = self.call_id_to_patch.remove(&call_id);
|
||||
|
||||
|
||||
@@ -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)]
|
||||
@@ -911,6 +913,9 @@ pub struct PatchApplyEndEvent {
|
||||
pub stderr: String,
|
||||
/// Whether the patch was applied successfully.
|
||||
pub success: bool,
|
||||
/// Unified diff describing the patch the tool applied, if available.
|
||||
#[serde(default)]
|
||||
pub diff: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, TS)]
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
@@ -67,6 +69,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 +96,143 @@ struct RunningCommand {
|
||||
parsed_cmd: Vec<ParsedCommand>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct AppliedPatchDiff {
|
||||
diff: String,
|
||||
working_dir: PathBuf,
|
||||
}
|
||||
|
||||
const MAX_COMPLETED_UNDO_TURNS: usize = 1;
|
||||
|
||||
#[derive(Default)]
|
||||
struct PatchUndoHistory {
|
||||
working_dirs: HashMap<String, PathBuf>,
|
||||
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.working_dirs.clear();
|
||||
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);
|
||||
self.prune_completed_turns();
|
||||
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 working_dir = working_dir_for_patch(changes, &patch_cwd, default_cwd);
|
||||
self.working_dirs.insert(call_id, working_dir);
|
||||
}
|
||||
|
||||
fn complete_patch(&mut self, call_id: &str, success: bool, diff: Option<String>) {
|
||||
let Some(working_dir) = self.working_dirs.remove(call_id) else {
|
||||
return;
|
||||
};
|
||||
|
||||
if !success {
|
||||
return;
|
||||
}
|
||||
|
||||
let Some(diff) = diff else {
|
||||
return;
|
||||
};
|
||||
|
||||
if diff.trim().is_empty() {
|
||||
return;
|
||||
}
|
||||
|
||||
self.active_turn
|
||||
.push(AppliedPatchDiff { diff, working_dir });
|
||||
}
|
||||
|
||||
fn prune_completed_turns(&mut self) {
|
||||
while self.completed_turns.len() > MAX_COMPLETED_UNDO_TURNS {
|
||||
self.completed_turns.remove(0);
|
||||
}
|
||||
}
|
||||
|
||||
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()
|
||||
&& !success
|
||||
{
|
||||
self.completed_turns.push(turn);
|
||||
self.prune_completed_turns();
|
||||
}
|
||||
}
|
||||
|
||||
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 +257,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 +376,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 +392,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 +413,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 +488,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 +662,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, event.diff.clone());
|
||||
if !event.success {
|
||||
self.add_to_history(history_cell::new_patch_apply_failure(event.stderr));
|
||||
}
|
||||
@@ -658,6 +810,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 +863,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 +1016,9 @@ impl ChatWidget {
|
||||
tx.send(AppEvent::DiffResult(text));
|
||||
});
|
||||
}
|
||||
SlashCommand::Undo => {
|
||||
self.undo_last_patch();
|
||||
}
|
||||
SlashCommand::Mention => {
|
||||
self.insert_str("@");
|
||||
}
|
||||
@@ -1140,10 +1297,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();
|
||||
}
|
||||
|
||||
@@ -68,11 +68,39 @@ 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
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn patch_undo_history_is_bounded() {
|
||||
fn make_diff(label: &str) -> Arc<[AppliedPatchDiff]> {
|
||||
Arc::from(vec![AppliedPatchDiff {
|
||||
diff: format!("diff --git a/{label} b/{label}"),
|
||||
working_dir: PathBuf::from("/repo"),
|
||||
}])
|
||||
}
|
||||
|
||||
let mut history = PatchUndoHistory::new();
|
||||
history.completed_turns.push(make_diff("first"));
|
||||
history.prune_completed_turns();
|
||||
assert_eq!(history.completed_turns.len(), 1);
|
||||
|
||||
history.completed_turns.push(make_diff("second"));
|
||||
history.prune_completed_turns();
|
||||
assert_eq!(history.completed_turns.len(), 1);
|
||||
assert_eq!(
|
||||
history.completed_turns[0][0].diff,
|
||||
"diff --git a/second b/second"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn final_answer_without_newline_is_flushed_immediately() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
|
||||
@@ -234,6 +262,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 +1132,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(),
|
||||
@@ -1122,6 +1152,7 @@ fn apply_patch_events_emit_history_cells() {
|
||||
stdout: "ok\n".into(),
|
||||
stderr: String::new(),
|
||||
success: true,
|
||||
diff: Some("diff --git a/foo.txt b/foo.txt\n".to_string()),
|
||||
};
|
||||
chat.handle_codex_event(Event {
|
||||
id: "s1".into(),
|
||||
@@ -1169,6 +1200,7 @@ fn apply_patch_manual_approval_adjusts_header() {
|
||||
call_id: "c1".into(),
|
||||
auto_approved: false,
|
||||
changes: apply_changes,
|
||||
cwd: PathBuf::from("/tmp"),
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -1218,6 +1250,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 +1367,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 {
|
||||
@@ -1343,6 +1377,7 @@ fn apply_patch_full_flow_integration_like() {
|
||||
stdout: String::from("ok"),
|
||||
stderr: String::new(),
|
||||
success: true,
|
||||
diff: Some("diff --git a/pkg.rs b/pkg.rs\n".to_string()),
|
||||
}),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1170,6 +1170,77 @@ 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 lines = vec!["No applied patch to undo.".dim().into()];
|
||||
PlainHistoryCell { lines }
|
||||
}
|
||||
|
||||
pub(crate) fn new_patch_undo_busy() -> PlainHistoryCell {
|
||||
let lines = vec![
|
||||
"Undo already running".magenta().bold().into(),
|
||||
"Wait for the previous undo to finish before starting another."
|
||||
.dim()
|
||||
.into(),
|
||||
];
|
||||
PlainHistoryCell { lines }
|
||||
}
|
||||
|
||||
pub(crate) fn new_patch_undo_in_progress() -> PlainHistoryCell {
|
||||
let lines = vec!["↺ Undoing latest patch…".magenta().bold().into()];
|
||||
PlainHistoryCell { lines }
|
||||
}
|
||||
|
||||
pub(crate) fn new_patch_undo_cancelled() -> PlainHistoryCell {
|
||||
let lines = vec![
|
||||
"↺ Undo cancelled".magenta().bold().into(),
|
||||
"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.
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
117
codex-rs/tui/src/undo_patch.rs
Normal file
117
codex-rs/tui/src/undo_patch.rs
Normal file
@@ -0,0 +1,117 @@
|
||||
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", "--unsafe-paths", "-R"],
|
||||
&["apply", "--unsafe-paths", "--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::other(format!(
|
||||
"git {args:?} failed with status {status}"
|
||||
)))
|
||||
}
|
||||
}
|
||||
|
||||
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::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(())
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user