diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ecaf7df4b7a..880efbbf4f0 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1212,8 +1212,11 @@ version = "0.0.0" dependencies = [ "assert_matches", "pretty_assertions", + "schemars 0.8.22", + "serde", "tempfile", "thiserror 2.0.16", + "ts-rs", "walkdir", ] @@ -1331,6 +1334,7 @@ version = "0.0.0" dependencies = [ "anyhow", "base64", + "codex-git-tooling", "icu_decimal", "icu_locale_core", "mcp-types", @@ -1431,7 +1435,6 @@ dependencies = [ "codex-core", "codex-feedback", "codex-file-search", - "codex-git-tooling", "codex-login", "codex-ollama", "codex-protocol", @@ -5461,9 +5464,9 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.226" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0dca6411025b24b60bfa7ec1fe1f8e710ac09782dca409ee8237ba74b51295fd" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" dependencies = [ "serde_core", "serde_derive", @@ -5471,18 +5474,18 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.226" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba2ba63999edb9dac981fb34b3e5c0d111a69b0924e253ed29d83f7c99e966a4" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.226" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8db53ae22f34573731bafa1db20f04027b2d25e02d8205921b569171699cdb33" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index bb052d9d9af..65dad461fac 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -109,6 +109,7 @@ use crate::tasks::RegularTask; use crate::tasks::ReviewTask; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; +use crate::tasks::UndoTask; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::parallel::ToolCallRuntime; @@ -958,7 +959,7 @@ impl Session { state.record_items(items.iter()); } - async fn replace_history(&self, items: Vec) { + pub(crate) async fn replace_history(&self, items: Vec) { let mut state = self.state.lock().await; state.replace_history(items); } @@ -1420,6 +1421,13 @@ async fn submission_loop(sess: Arc, config: Arc, rx_sub: Receiv }; sess.send_event_raw(event).await; } + Op::Undo => { + let turn_context = sess + .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) + .await; + sess.spawn_task(turn_context, Vec::new(), UndoTask::new()) + .await; + } Op::Compact => { let turn_context = sess .new_turn_with_sub_id(sub.id.clone(), SessionSettingsUpdate::default()) diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 746ae93bc3d..f1b786055e5 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -43,6 +43,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::TokenCount(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) + | EventMsg::UndoCompleted(_) | EventMsg::TurnAborted(_) => true, EventMsg::Error(_) | EventMsg::TaskStarted(_) @@ -68,6 +69,7 @@ pub(crate) fn should_persist_event_msg(ev: &EventMsg) -> bool { | EventMsg::PatchApplyEnd(_) | EventMsg::TurnDiff(_) | EventMsg::GetHistoryEntryResponse(_) + | EventMsg::UndoStarted(_) | EventMsg::McpListToolsResponse(_) | EventMsg::ListCustomPromptsResponse(_) | EventMsg::PlanUpdate(_) diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 55c64a29dcb..f421a91e4ea 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -10,7 +10,6 @@ use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; use codex_utils_readiness::Token; -use std::borrow::ToOwned; use std::sync::Arc; use tokio_util::sync::CancellationToken; use tracing::info; @@ -52,8 +51,7 @@ impl SessionTask for GhostSnapshotTask { session .session .record_conversation_items(&ctx, &[ResponseItem::GhostSnapshot { - commit_id: ghost_commit.id().to_string(), - parent: ghost_commit.parent().map(ToOwned::to_owned), + ghost_commit: ghost_commit.clone(), }]) .await; info!("ghost commit captured: {}", ghost_commit.id()); diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index a5afbca2e39..466bfe21a98 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -2,6 +2,7 @@ mod compact; mod ghost_snapshot; mod regular; mod review; +mod undo; use std::sync::Arc; use std::time::Duration; @@ -29,6 +30,7 @@ pub(crate) use compact::CompactTask; pub(crate) use ghost_snapshot::GhostSnapshotTask; pub(crate) use regular::RegularTask; pub(crate) use review::ReviewTask; +pub(crate) use undo::UndoTask; const GRACEFULL_INTERRUPTION_TIMEOUT_MS: u64 = 100; diff --git a/codex-rs/core/src/tasks/undo.rs b/codex-rs/core/src/tasks/undo.rs new file mode 100644 index 00000000000..e834eea02d4 --- /dev/null +++ b/codex-rs/core/src/tasks/undo.rs @@ -0,0 +1,117 @@ +use std::sync::Arc; + +use crate::codex::TurnContext; +use crate::protocol::EventMsg; +use crate::protocol::UndoCompletedEvent; +use crate::protocol::UndoStartedEvent; +use crate::state::TaskKind; +use crate::tasks::SessionTask; +use crate::tasks::SessionTaskContext; +use async_trait::async_trait; +use codex_git_tooling::restore_ghost_commit; +use codex_protocol::models::ResponseItem; +use codex_protocol::user_input::UserInput; +use tokio_util::sync::CancellationToken; +use tracing::error; +use tracing::info; +use tracing::warn; + +pub(crate) struct UndoTask; + +impl UndoTask { + pub(crate) fn new() -> Self { + Self + } +} + +#[async_trait] +impl SessionTask for UndoTask { + fn kind(&self) -> TaskKind { + TaskKind::Regular + } + + async fn run( + self: Arc, + session: Arc, + ctx: Arc, + _input: Vec, + cancellation_token: CancellationToken, + ) -> Option { + let sess = session.clone_session(); + sess.send_event( + ctx.as_ref(), + EventMsg::UndoStarted(UndoStartedEvent { + message: Some("Undo in progress...".to_string()), + }), + ) + .await; + + if cancellation_token.is_cancelled() { + sess.send_event( + ctx.as_ref(), + EventMsg::UndoCompleted(UndoCompletedEvent { + success: false, + message: Some("Undo cancelled.".to_string()), + }), + ) + .await; + return None; + } + + let mut history = sess.clone_history().await; + let mut items = history.get_history(); + let mut completed = UndoCompletedEvent { + success: false, + message: None, + }; + + let Some((idx, ghost_commit)) = + items + .iter() + .enumerate() + .rev() + .find_map(|(idx, item)| match item { + ResponseItem::GhostSnapshot { ghost_commit } => { + Some((idx, ghost_commit.clone())) + } + _ => None, + }) + else { + completed.message = Some("No ghost snapshot available to undo.".to_string()); + sess.send_event(ctx.as_ref(), EventMsg::UndoCompleted(completed)) + .await; + return None; + }; + + let commit_id = ghost_commit.id().to_string(); + let repo_path = ctx.cwd.clone(); + let restore_result = + tokio::task::spawn_blocking(move || restore_ghost_commit(&repo_path, &ghost_commit)) + .await; + + match restore_result { + Ok(Ok(())) => { + items.remove(idx); + sess.replace_history(items).await; + let short_id: String = commit_id.chars().take(7).collect(); + info!(commit_id = commit_id, "Undo restored ghost snapshot"); + completed.success = true; + completed.message = Some(format!("Undo restored snapshot {short_id}.")); + } + Ok(Err(err)) => { + let message = format!("Failed to restore snapshot {commit_id}: {err}"); + warn!("{message}"); + completed.message = Some(message); + } + Err(err) => { + let message = format!("Failed to restore snapshot {commit_id}: {err}"); + error!("{message}"); + completed.message = Some(message); + } + } + + sess.send_event(ctx.as_ref(), EventMsg::UndoCompleted(completed)) + .await; + None + } +} diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index bd37a438fb1..2d06c881a85 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -20,7 +20,6 @@ use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; -use codex_core::protocol::WebSearchBeginEvent; use codex_core::protocol::WebSearchEndEvent; use codex_protocol::num_format::format_with_separators; use owo_colors::OwoColorize; @@ -216,7 +215,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { cwd.to_string_lossy(), ); } - EventMsg::ExecCommandOutputDelta(_) => {} EventMsg::ExecCommandEnd(ExecCommandEndEvent { aggregated_output, duration, @@ -283,7 +281,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { } } } - EventMsg::WebSearchBegin(WebSearchBeginEvent { call_id: _ }) => {} EventMsg::WebSearchEnd(WebSearchEndEvent { call_id: _, query }) => { ts_msg!(self, "🌐 Searched: {query}"); } @@ -411,12 +408,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { ); eprintln!("{unified_diff}"); } - EventMsg::ExecApprovalRequest(_) => { - // Should we exit? - } - EventMsg::ApplyPatchApprovalRequest(_) => { - // Should we exit? - } EventMsg::AgentReasoning(agent_reasoning_event) => { if self.show_agent_reasoning { ts_msg!( @@ -481,15 +472,6 @@ impl EventProcessor for EventProcessorWithHumanOutput { } } } - EventMsg::GetHistoryEntryResponse(_) => { - // Currently ignored in exec output. - } - EventMsg::McpListToolsResponse(_) => { - // Currently ignored in exec output. - } - EventMsg::ListCustomPromptsResponse(_) => { - // Currently ignored in exec output. - } EventMsg::ViewImageToolCall(view) => { ts_msg!( self, @@ -510,15 +492,24 @@ impl EventProcessor for EventProcessorWithHumanOutput { } }, EventMsg::ShutdownComplete => return CodexStatus::Shutdown, - EventMsg::UserMessage(_) => {} - EventMsg::EnteredReviewMode(_) => {} - EventMsg::ExitedReviewMode(_) => {} - EventMsg::AgentMessageDelta(_) => {} - EventMsg::AgentReasoningDelta(_) => {} - EventMsg::AgentReasoningRawContentDelta(_) => {} - EventMsg::ItemStarted(_) => {} - EventMsg::ItemCompleted(_) => {} - EventMsg::RawResponseItem(_) => {} + EventMsg::WebSearchBegin(_) + | EventMsg::ExecApprovalRequest(_) + | EventMsg::ApplyPatchApprovalRequest(_) + | EventMsg::ExecCommandOutputDelta(_) + | EventMsg::GetHistoryEntryResponse(_) + | EventMsg::McpListToolsResponse(_) + | EventMsg::ListCustomPromptsResponse(_) + | EventMsg::UserMessage(_) + | EventMsg::EnteredReviewMode(_) + | EventMsg::ExitedReviewMode(_) + | EventMsg::AgentMessageDelta(_) + | EventMsg::AgentReasoningDelta(_) + | EventMsg::AgentReasoningRawContentDelta(_) + | EventMsg::ItemStarted(_) + | EventMsg::ItemCompleted(_) + | EventMsg::RawResponseItem(_) + | EventMsg::UndoCompleted(_) + | EventMsg::UndoStarted(_) => {} } CodexStatus::Running } diff --git a/codex-rs/git-tooling/Cargo.toml b/codex-rs/git-tooling/Cargo.toml index 183221f3e15..c30d58de8d5 100644 --- a/codex-rs/git-tooling/Cargo.toml +++ b/codex-rs/git-tooling/Cargo.toml @@ -9,13 +9,20 @@ name = "codex_git_tooling" path = "src/lib.rs" [dependencies] -tempfile = "3" -thiserror = "2" -walkdir = "2" +tempfile = { workspace = true } +thiserror = { workspace = true } +walkdir = { workspace = true } +schemars = { workspace = true } +serde = { workspace = true, features = ["derive"] } +ts-rs = { workspace = true, features = [ + "uuid-impl", + "serde-json-impl", + "no-serde-warnings", +] } [lints] workspace = true [dev-dependencies] assert_matches = { workspace = true } -pretty_assertions = "1.4.1" +pretty_assertions = { workspace = true } diff --git a/codex-rs/git-tooling/src/ghost_commits.rs b/codex-rs/git-tooling/src/ghost_commits.rs index 6c2a86166cc..c5ebd7c02ce 100644 --- a/codex-rs/git-tooling/src/ghost_commits.rs +++ b/codex-rs/git-tooling/src/ghost_commits.rs @@ -1,4 +1,7 @@ +use std::collections::HashSet; use std::ffi::OsString; +use std::fs; +use std::io; use std::path::Path; use std::path::PathBuf; @@ -14,6 +17,7 @@ use crate::operations::resolve_head; use crate::operations::resolve_repository_root; use crate::operations::run_git_for_status; use crate::operations::run_git_for_stdout; +use crate::operations::run_git_for_stdout_all; /// Default commit message used for ghost commits when none is provided. const DEFAULT_COMMIT_MESSAGE: &str = "codex snapshot"; @@ -69,6 +73,8 @@ pub fn create_ghost_commit( let repo_root = resolve_repository_root(options.repo_path)?; let repo_prefix = repo_subdir(repo_root.as_path(), options.repo_path); let parent = resolve_head(repo_root.as_path())?; + let existing_untracked = + capture_existing_untracked(repo_root.as_path(), repo_prefix.as_deref())?; let normalized_force = options .force_include @@ -84,6 +90,16 @@ pub fn create_ghost_commit( OsString::from(index_path.as_os_str()), )]; + // Pre-populate the temporary index with HEAD so unchanged tracked files + // are included in the snapshot tree. + if let Some(parent_sha) = parent.as_deref() { + run_git_for_status( + repo_root.as_path(), + vec![OsString::from("read-tree"), OsString::from(parent_sha)], + Some(base_env.as_slice()), + )?; + } + let mut add_args = vec![OsString::from("add"), OsString::from("--all")]; if let Some(prefix) = repo_prefix.as_deref() { add_args.extend([OsString::from("--"), prefix.as_os_str().to_os_string()]); @@ -127,12 +143,29 @@ pub fn create_ghost_commit( Some(commit_env.as_slice()), )?; - Ok(GhostCommit::new(commit_id, parent)) + Ok(GhostCommit::new( + commit_id, + parent, + existing_untracked.files, + existing_untracked.dirs, + )) } /// Restore the working tree to match the provided ghost commit. pub fn restore_ghost_commit(repo_path: &Path, commit: &GhostCommit) -> Result<(), GitToolingError> { - restore_to_commit(repo_path, commit.id()) + ensure_git_repository(repo_path)?; + + let repo_root = resolve_repository_root(repo_path)?; + let repo_prefix = repo_subdir(repo_root.as_path(), repo_path); + let current_untracked = + capture_existing_untracked(repo_root.as_path(), repo_prefix.as_deref())?; + remove_new_untracked( + repo_root.as_path(), + commit.preexisting_untracked_files(), + commit.preexisting_untracked_dirs(), + current_untracked, + )?; + restore_to_commit_inner(repo_root.as_path(), repo_prefix.as_deref(), commit.id()) } /// Restore the working tree to match the given commit ID. @@ -141,7 +174,16 @@ pub fn restore_to_commit(repo_path: &Path, commit_id: &str) -> Result<(), GitToo let repo_root = resolve_repository_root(repo_path)?; let repo_prefix = repo_subdir(repo_root.as_path(), repo_path); + restore_to_commit_inner(repo_root.as_path(), repo_prefix.as_deref(), commit_id) +} +/// Restores the working tree and index to the given commit using `git restore`. +/// The repository root and optional repository-relative prefix limit the restore scope. +fn restore_to_commit_inner( + repo_root: &Path, + repo_prefix: Option<&Path>, + commit_id: &str, +) -> Result<(), GitToolingError> { let mut restore_args = vec![ OsString::from("restore"), OsString::from("--source"), @@ -150,13 +192,143 @@ pub fn restore_to_commit(repo_path: &Path, commit_id: &str) -> Result<(), GitToo OsString::from("--staged"), OsString::from("--"), ]; - if let Some(prefix) = repo_prefix.as_deref() { + if let Some(prefix) = repo_prefix { restore_args.push(prefix.as_os_str().to_os_string()); } else { restore_args.push(OsString::from(".")); } - run_git_for_status(repo_root.as_path(), restore_args, None)?; + run_git_for_status(repo_root, restore_args, None)?; + Ok(()) +} + +#[derive(Default)] +struct UntrackedSnapshot { + files: Vec, + dirs: Vec, +} + +/// Captures the untracked and ignored entries under `repo_root`, optionally limited by `repo_prefix`. +/// Returns the result as an `UntrackedSnapshot`. +fn capture_existing_untracked( + repo_root: &Path, + repo_prefix: Option<&Path>, +) -> Result { + // Ask git for the zero-delimited porcelain status so we can enumerate + // every untracked or ignored path (including ones filtered by prefix). + let mut args = vec![ + OsString::from("status"), + OsString::from("--porcelain=2"), + OsString::from("-z"), + OsString::from("--ignored=matching"), + OsString::from("--untracked-files=all"), + ]; + if let Some(prefix) = repo_prefix { + args.push(OsString::from("--")); + args.push(prefix.as_os_str().to_os_string()); + } + + let output = run_git_for_stdout_all(repo_root, args, None)?; + if output.is_empty() { + return Ok(UntrackedSnapshot::default()); + } + + let mut snapshot = UntrackedSnapshot::default(); + // Each entry is of the form " " where code is '?' (untracked) + // or '!' (ignored); everything else is irrelevant to this snapshot. + for entry in output.split('\0') { + if entry.is_empty() { + continue; + } + let mut parts = entry.splitn(2, ' '); + let code = parts.next(); + let path_part = parts.next(); + let (Some(code), Some(path_part)) = (code, path_part) else { + continue; + }; + if code != "?" && code != "!" { + continue; + } + if path_part.is_empty() { + continue; + } + + let normalized = normalize_relative_path(Path::new(path_part))?; + let absolute = repo_root.join(&normalized); + let is_dir = absolute.is_dir(); + if is_dir { + snapshot.dirs.push(normalized); + } else { + snapshot.files.push(normalized); + } + } + + Ok(snapshot) +} + +/// Removes untracked files and directories that were not present when the snapshot was captured. +fn remove_new_untracked( + repo_root: &Path, + preserved_files: &[PathBuf], + preserved_dirs: &[PathBuf], + current: UntrackedSnapshot, +) -> Result<(), GitToolingError> { + if current.files.is_empty() && current.dirs.is_empty() { + return Ok(()); + } + + let preserved_file_set: HashSet = preserved_files.iter().cloned().collect(); + let preserved_dirs_vec: Vec = preserved_dirs.to_vec(); + + for path in current.files { + if should_preserve(&path, &preserved_file_set, &preserved_dirs_vec) { + continue; + } + remove_path(&repo_root.join(&path))?; + } + + for dir in current.dirs { + if should_preserve(&dir, &preserved_file_set, &preserved_dirs_vec) { + continue; + } + remove_path(&repo_root.join(&dir))?; + } + + Ok(()) +} + +/// Determines whether an untracked path should be kept because it existed in the snapshot. +fn should_preserve( + path: &Path, + preserved_files: &HashSet, + preserved_dirs: &[PathBuf], +) -> bool { + if preserved_files.contains(path) { + return true; + } + + preserved_dirs + .iter() + .any(|dir| path.starts_with(dir.as_path())) +} + +/// Deletes the file or directory at the provided path, ignoring if it is already absent. +fn remove_path(path: &Path) -> Result<(), GitToolingError> { + match fs::symlink_metadata(path) { + Ok(metadata) => { + if metadata.is_dir() { + fs::remove_dir_all(path)?; + } else { + fs::remove_file(path)?; + } + } + Err(err) => { + if err.kind() == io::ErrorKind::NotFound { + return Ok(()); + } + return Err(err.into()); + } + } Ok(()) } @@ -239,6 +411,9 @@ mod tests { ], ); + let preexisting_untracked = repo.join("notes.txt"); + std::fs::write(&preexisting_untracked, "notes before\n")?; + let tracked_contents = "modified contents\n"; std::fs::write(repo.join("tracked.txt"), tracked_contents)?; std::fs::remove_file(repo.join("delete-me.txt"))?; @@ -267,6 +442,7 @@ mod tests { std::fs::write(repo.join("ignored.txt"), "changed\n")?; std::fs::remove_file(repo.join("new-file.txt"))?; std::fs::write(repo.join("ephemeral.txt"), "temp data\n")?; + std::fs::write(&preexisting_untracked, "notes after\n")?; restore_ghost_commit(repo, &ghost)?; @@ -277,7 +453,9 @@ mod tests { let new_file_after = std::fs::read_to_string(repo.join("new-file.txt"))?; assert_eq!(new_file_after, new_file_contents); assert_eq!(repo.join("delete-me.txt").exists(), false); - assert!(repo.join("ephemeral.txt").exists()); + assert!(!repo.join("ephemeral.txt").exists()); + let notes_after = std::fs::read_to_string(&preexisting_untracked)?; + assert_eq!(notes_after, "notes before\n"); Ok(()) } @@ -488,7 +666,43 @@ mod tests { assert!(vscode.join("settings.json").exists()); let settings_after = std::fs::read_to_string(vscode.join("settings.json"))?; assert_eq!(settings_after, "{\n \"after\": true\n}\n"); - assert!(repo.join("temp.txt").exists()); + assert!(!repo.join("temp.txt").exists()); + + Ok(()) + } + + #[test] + /// Restoring removes ignored directories created after the snapshot. + fn restore_removes_new_ignored_directory() -> Result<(), GitToolingError> { + let temp = tempfile::tempdir()?; + let repo = temp.path(); + init_test_repo(repo); + + std::fs::write(repo.join(".gitignore"), ".vscode/\n")?; + std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?; + run_git_in(repo, &["add", ".gitignore", "tracked.txt"]); + run_git_in( + repo, + &[ + "-c", + "user.name=Tester", + "-c", + "user.email=test@example.com", + "commit", + "-m", + "initial", + ], + ); + + let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?; + + let vscode = repo.join(".vscode"); + std::fs::create_dir_all(&vscode)?; + std::fs::write(vscode.join("settings.json"), "{\n \"after\": true\n}\n")?; + + restore_ghost_commit(repo, &ghost)?; + + assert!(!vscode.exists()); Ok(()) } diff --git a/codex-rs/git-tooling/src/lib.rs b/codex-rs/git-tooling/src/lib.rs index f41d104385a..e6e53924b65 100644 --- a/codex-rs/git-tooling/src/lib.rs +++ b/codex-rs/git-tooling/src/lib.rs @@ -1,4 +1,5 @@ use std::fmt; +use std::path::PathBuf; mod errors; mod ghost_commits; @@ -11,18 +12,36 @@ pub use ghost_commits::create_ghost_commit; pub use ghost_commits::restore_ghost_commit; pub use ghost_commits::restore_to_commit; pub use platform::create_symlink; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; +use ts_rs::TS; + +type CommitID = String; /// Details of a ghost commit created from a repository state. -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, TS)] pub struct GhostCommit { - id: String, - parent: Option, + id: CommitID, + parent: Option, + preexisting_untracked_files: Vec, + preexisting_untracked_dirs: Vec, } impl GhostCommit { /// Create a new ghost commit wrapper from a raw commit ID and optional parent. - pub fn new(id: String, parent: Option) -> Self { - Self { id, parent } + pub fn new( + id: CommitID, + parent: Option, + preexisting_untracked_files: Vec, + preexisting_untracked_dirs: Vec, + ) -> Self { + Self { + id, + parent, + preexisting_untracked_files, + preexisting_untracked_dirs, + } } /// Commit ID for the snapshot. @@ -34,6 +53,16 @@ impl GhostCommit { pub fn parent(&self) -> Option<&str> { self.parent.as_deref() } + + /// Untracked or ignored files that already existed when the snapshot was captured. + pub fn preexisting_untracked_files(&self) -> &[PathBuf] { + &self.preexisting_untracked_files + } + + /// Untracked or ignored directories that already existed when the snapshot was captured. + pub fn preexisting_untracked_dirs(&self) -> &[PathBuf] { + &self.preexisting_untracked_dirs + } } impl fmt::Display for GhostCommit { diff --git a/codex-rs/git-tooling/src/operations.rs b/codex-rs/git-tooling/src/operations.rs index 3b387f8498c..415d63d5685 100644 --- a/codex-rs/git-tooling/src/operations.rs +++ b/codex-rs/git-tooling/src/operations.rs @@ -161,6 +161,27 @@ where }) } +/// Executes `git` and returns the full stdout without trimming so callers +/// can parse delimiter-sensitive output, propagating UTF-8 errors with context. +pub(crate) fn run_git_for_stdout_all( + dir: &Path, + args: I, + env: Option<&[(OsString, OsString)]>, +) -> Result +where + I: IntoIterator, + S: AsRef, +{ + // Keep the raw stdout untouched so callers can parse delimiter-sensitive + // output (e.g. NUL-separated paths) without trimming artefacts. + let run = run_git(dir, args, env)?; + // Propagate UTF-8 conversion failures with the command context for debugging. + String::from_utf8(run.output.stdout).map_err(|source| GitToolingError::GitOutputUtf8 { + command: run.command, + source, + }) +} + fn run_git( dir: &Path, args: I, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 9f826f4e860..9ba48c018cd 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -288,6 +288,8 @@ async fn run_codex_tool_session_inner( | EventMsg::EnteredReviewMode(_) | EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) + | EventMsg::UndoStarted(_) + | EventMsg::UndoCompleted(_) | EventMsg::ExitedReviewMode(_) => { // For now, we do not do anything extra for these // events. Note that diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 0393c854272..6249d870a3e 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -11,6 +11,8 @@ path = "src/lib.rs" workspace = true [dependencies] +codex-git-tooling = { workspace = true } + base64 = { workspace = true } icu_decimal = { workspace = true } icu_locale_core = { workspace = true } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 02d557535e3..095d7de4dec 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -9,6 +9,7 @@ use serde::ser::Serializer; use ts_rs::TS; use crate::user_input::UserInput; +use codex_git_tooling::GhostCommit; use schemars::JsonSchema; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] @@ -118,9 +119,7 @@ pub enum ResponseItem { }, // Generated by the harness but considered exactly as a model response. GhostSnapshot { - commit_id: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - parent: Option, + ghost_commit: GhostCommit, }, #[serde(other)] Other, diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 1b4a6fccf11..bb14f3797bb 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -178,6 +178,9 @@ pub enum Op { /// to generate a summary which will be returned as an AgentMessage event. Compact, + /// Request Codex to undo a turn (turn are stacked so it is the same effect as CMD + Z). + Undo, + /// Request a code review from the agent. Review { review_request: ReviewRequest }, @@ -486,6 +489,10 @@ pub enum EventMsg { BackgroundEvent(BackgroundEventEvent), + UndoStarted(UndoStartedEvent), + + UndoCompleted(UndoCompletedEvent), + /// Notification that a model stream experienced an error or disconnect /// and the system is handling it (e.g., retrying with backoff). StreamError(StreamErrorEvent), @@ -1135,6 +1142,19 @@ pub struct BackgroundEventEvent { pub message: String, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct UndoStartedEvent { + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, +} + +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] +pub struct UndoCompletedEvent { + pub success: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub message: Option, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct StreamErrorEvent { pub message: String, diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 824e4fee31b..f087202c22d 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -35,7 +35,6 @@ codex-common = { workspace = true, features = [ ] } codex-core = { workspace = true } codex-file-search = { workspace = true } -codex-git-tooling = { workspace = true } codex-login = { workspace = true } codex-ollama = { workspace = true } codex-protocol = { workspace = true } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index cf9e3547378..54d2f7912e3 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -315,6 +315,11 @@ impl BottomPane { self.ctrl_c_quit_hint } + #[cfg(test)] + pub(crate) fn status_indicator_visible(&self) -> bool { + self.status.is_some() + } + pub(crate) fn show_esc_backtrack_hint(&mut self) { self.esc_backtrack_hint = true; self.composer.set_esc_backtrack_hint(true); @@ -359,6 +364,16 @@ impl BottomPane { } } + pub(crate) fn ensure_status_indicator(&mut self) { + if self.status.is_none() { + self.status = Some(StatusIndicatorWidget::new( + self.app_event_tx.clone(), + self.frame_requester.clone(), + )); + self.request_redraw(); + } + } + pub(crate) fn set_context_window_percent(&mut self, percent: Option) { if self.context_window_percent == percent { return; diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 64dc89d8841..7b83eedcc9a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -37,6 +37,8 @@ use codex_core::protocol::TokenUsage; use codex_core::protocol::TokenUsageInfo; use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; +use codex_core::protocol::UndoCompletedEvent; +use codex_core::protocol::UndoStartedEvent; use codex_core::protocol::UserMessageEvent; use codex_core::protocol::ViewImageToolCallEvent; use codex_core::protocol::WebSearchBeginEvent; @@ -113,16 +115,9 @@ use codex_core::protocol::AskForApproval; use codex_core::protocol::SandboxPolicy; use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig; use codex_file_search::FileMatch; -use codex_git_tooling::CreateGhostCommitOptions; -use codex_git_tooling::GhostCommit; -use codex_git_tooling::GitToolingError; -use codex_git_tooling::create_ghost_commit; -use codex_git_tooling::restore_ghost_commit; use codex_protocol::plan_tool::UpdatePlanArgs; use strum::IntoEnumIterator; -const MAX_TRACKED_GHOST_COMMITS: usize = 20; - // Track information about an in-flight exec command. struct RunningCommand { command: Vec, @@ -267,9 +262,6 @@ pub(crate) struct ChatWidget { pending_notification: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, - // List of ghost commits corresponding to each turn. - ghost_snapshots: Vec, - ghost_snapshots_disabled: bool, // Whether to add a final message separator after the last message needs_final_message_separator: bool, @@ -672,6 +664,31 @@ impl ChatWidget { debug!("BackgroundEvent: {message}"); } + fn on_undo_started(&mut self, event: UndoStartedEvent) { + self.bottom_pane.ensure_status_indicator(); + let message = event + .message + .unwrap_or_else(|| "Undo in progress...".to_string()); + self.set_status_header(message); + } + + fn on_undo_completed(&mut self, event: UndoCompletedEvent) { + let UndoCompletedEvent { success, message } = event; + self.bottom_pane.hide_status_indicator(); + let message = message.unwrap_or_else(|| { + if success { + "Undo completed successfully.".to_string() + } else { + "Undo failed.".to_string() + } + }); + if success { + self.add_info_message(message, None); + } else { + self.add_error_message(message); + } + } + fn on_stream_error(&mut self, message: String) { if self.retry_status_header.is_none() { self.retry_status_header = Some(self.current_status_header.clone()); @@ -989,8 +1006,6 @@ impl ChatWidget { suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, - ghost_snapshots: Vec::new(), - ghost_snapshots_disabled: true, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, @@ -1057,8 +1072,6 @@ impl ChatWidget { suppress_session_configured_redraw: true, pending_notification: None, is_review_mode: false, - ghost_snapshots: Vec::new(), - ghost_snapshots_disabled: true, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, @@ -1211,7 +1224,7 @@ impl ChatWidget { self.app_event_tx.send(AppEvent::ExitRequest); } SlashCommand::Undo => { - self.undo_last_snapshot(); + self.app_event_tx.send(AppEvent::CodexOp(Op::Undo)); } SlashCommand::Diff => { self.add_diff_in_progress(); @@ -1328,8 +1341,6 @@ impl ChatWidget { return; } - self.capture_ghost_snapshot(); - let mut items: Vec = Vec::new(); if !text.is_empty() { @@ -1362,57 +1373,6 @@ impl ChatWidget { self.needs_final_message_separator = false; } - fn capture_ghost_snapshot(&mut self) { - if self.ghost_snapshots_disabled { - return; - } - - let options = CreateGhostCommitOptions::new(&self.config.cwd); - match create_ghost_commit(&options) { - Ok(commit) => { - self.ghost_snapshots.push(commit); - if self.ghost_snapshots.len() > MAX_TRACKED_GHOST_COMMITS { - self.ghost_snapshots.remove(0); - } - } - Err(err) => { - self.ghost_snapshots_disabled = true; - let (message, hint) = match &err { - GitToolingError::NotAGitRepository { .. } => ( - "Snapshots disabled: current directory is not a Git repository." - .to_string(), - None, - ), - _ => ( - format!("Snapshots disabled after error: {err}"), - Some( - "Restart Codex after resolving the issue to re-enable snapshots." - .to_string(), - ), - ), - }; - self.add_info_message(message, hint); - tracing::warn!("failed to create ghost snapshot: {err}"); - } - } - } - - fn undo_last_snapshot(&mut self) { - let Some(commit) = self.ghost_snapshots.pop() else { - self.add_info_message("No snapshot available to undo.".to_string(), None); - return; - }; - - if let Err(err) = restore_ghost_commit(&self.config.cwd, &commit) { - self.add_error_message(format!("Failed to restore snapshot: {err}")); - self.ghost_snapshots.push(commit); - return; - } - - let short_id: String = commit.id().chars().take(8).collect(); - self.add_info_message(format!("Restored workspace to snapshot {short_id}"), None); - } - /// Replay a subset of initial events into the UI to seed the transcript when /// resuming an existing session. This approximates the live event flow and /// is intentionally conservative: only safe-to-replay items are rendered to @@ -1510,6 +1470,8 @@ impl ChatWidget { EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => { self.on_background_event(message) } + EventMsg::UndoStarted(ev) => self.on_undo_started(ev), + EventMsg::UndoCompleted(ev) => self.on_undo_completed(ev), EventMsg::StreamError(StreamErrorEvent { message }) => self.on_stream_error(message), EventMsg::UserMessage(ev) => { if from_replay { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 6ec877656cd..e874c6060a2 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -34,6 +34,8 @@ use codex_core::protocol::ReviewRequest; use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TaskStartedEvent; +use codex_core::protocol::UndoCompletedEvent; +use codex_core::protocol::UndoStartedEvent; use codex_core::protocol::ViewImageToolCallEvent; use codex_protocol::ConversationId; use codex_protocol::plan_tool::PlanItemArg; @@ -294,8 +296,6 @@ fn make_chatwidget_manual() -> ( suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, - ghost_snapshots: Vec::new(), - ghost_snapshots_disabled: false, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback: codex_feedback::CodexFeedback::new(), @@ -849,6 +849,90 @@ fn slash_init_skips_when_project_doc_exists() { ); } +#[test] +fn slash_undo_sends_op() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.dispatch_command(SlashCommand::Undo); + + match rx.try_recv() { + Ok(AppEvent::CodexOp(Op::Undo)) => {} + other => panic!("expected AppEvent::CodexOp(Op::Undo), got {other:?}"), + } +} + +#[test] +fn undo_success_events_render_info_messages() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.handle_codex_event(Event { + id: "turn-1".to_string(), + msg: EventMsg::UndoStarted(UndoStartedEvent { + message: Some("Undo requested for the last turn...".to_string()), + }), + }); + assert!( + chat.bottom_pane.status_indicator_visible(), + "status indicator should be visible during undo" + ); + + chat.handle_codex_event(Event { + id: "turn-1".to_string(), + msg: EventMsg::UndoCompleted(UndoCompletedEvent { + success: true, + message: None, + }), + }); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected final status only"); + assert!( + !chat.bottom_pane.status_indicator_visible(), + "status indicator should be hidden after successful undo" + ); + + let completed = lines_to_single_string(&cells[0]); + assert!( + completed.contains("Undo completed successfully."), + "expected default success message, got {completed:?}" + ); +} + +#[test] +fn undo_failure_events_render_error_message() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(); + + chat.handle_codex_event(Event { + id: "turn-2".to_string(), + msg: EventMsg::UndoStarted(UndoStartedEvent { message: None }), + }); + assert!( + chat.bottom_pane.status_indicator_visible(), + "status indicator should be visible during undo" + ); + + chat.handle_codex_event(Event { + id: "turn-2".to_string(), + msg: EventMsg::UndoCompleted(UndoCompletedEvent { + success: false, + message: Some("Failed to restore workspace state.".to_string()), + }), + }); + + let cells = drain_insert_history(&mut rx); + assert_eq!(cells.len(), 1, "expected final status only"); + assert!( + !chat.bottom_pane.status_indicator_visible(), + "status indicator should be hidden after failed undo" + ); + + let completed = lines_to_single_string(&cells[0]); + assert!( + completed.contains("Failed to restore workspace state."), + "expected failure message, got {completed:?}" + ); +} + /// The commit picker shows only commit subjects (no timestamps). #[test] fn review_commit_picker_shows_subjects_without_timestamps() { diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 22da09f3580..bb3be33099f 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -39,7 +39,7 @@ impl SlashCommand { SlashCommand::Init => "create an AGENTS.md file with instructions for Codex", SlashCommand::Compact => "summarize conversation to prevent hitting the context limit", SlashCommand::Review => "review my current changes and find issues", - SlashCommand::Undo => "restore the workspace to the last Codex snapshot", + SlashCommand::Undo => "ask Codex to undo a turn", SlashCommand::Quit => "exit Codex", SlashCommand::Diff => "show git diff (including untracked files)", SlashCommand::Mention => "mention a file", @@ -85,14 +85,5 @@ impl SlashCommand { /// Return all built-in commands in a Vec paired with their command string. pub fn built_in_slash_commands() -> Vec<(&'static str, SlashCommand)> { - let show_beta_features = beta_features_enabled(); - - SlashCommand::iter() - .filter(|cmd| *cmd != SlashCommand::Undo || show_beta_features) - .map(|c| (c.command(), c)) - .collect() -} - -fn beta_features_enabled() -> bool { - std::env::var_os("BETA_FEATURE").is_some() + SlashCommand::iter().map(|c| (c.command(), c)).collect() }