From bcb037573ca779e8afc632a6c857f4ab332eeac0 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 14:21:00 +0100 Subject: [PATCH 1/4] V4 --- codex-rs/core/src/codex.rs | 10 +- codex-rs/core/src/tasks/mod.rs | 2 + codex-rs/core/src/tasks/undo.rs | 121 ++++++++++++++++++ .../src/event_processor_with_human_output.rs | 45 +++---- codex-rs/mcp-server/src/codex_tool_runner.rs | 2 + codex-rs/tui/src/slash_command.rs | 7 - 6 files changed, 152 insertions(+), 35 deletions(-) create mode 100644 codex-rs/core/src/tasks/undo.rs diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index c561b5cecb..6ef67dcb26 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; @@ -906,7 +907,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); } @@ -1358,6 +1359,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/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index a5afbca2e3..466bfe21a9 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 0000000000..f2e38e63c1 --- /dev/null +++ b/codex-rs/core/src/tasks/undo.rs @@ -0,0 +1,121 @@ +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::GhostCommit; +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, commit_id, parent)) = + items + .iter() + .enumerate() + .rev() + .find_map(|(idx, item)| match item { + ResponseItem::GhostSnapshot { commit_id, parent } => { + Some((idx, commit_id.clone(), parent.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 repo_path = ctx.cwd.clone(); + let ghost_commit = GhostCommit::new(commit_id.clone(), parent); + 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.as_str(), + "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 9a85c5de71..f0fda4ab34 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::ConversationPath(_) => {} - EventMsg::UserMessage(_) => {} - EventMsg::EnteredReviewMode(_) => {} - EventMsg::ExitedReviewMode(_) => {} - EventMsg::AgentMessageDelta(_) => {} - EventMsg::AgentReasoningDelta(_) => {} - EventMsg::AgentReasoningRawContentDelta(_) => {} - EventMsg::ItemStarted(_) => {} - EventMsg::ItemCompleted(_) => {} + EventMsg::WebSearchBegin(_) + | EventMsg::ExecApprovalRequest(_) + | EventMsg::ApplyPatchApprovalRequest(_) + | EventMsg::ExecCommandOutputDelta(_) + | EventMsg::GetHistoryEntryResponse(_) + | EventMsg::McpListToolsResponse(_) + | EventMsg::ListCustomPromptsResponse(_) + | EventMsg::ConversationPath(_) + | EventMsg::UserMessage(_) + | EventMsg::EnteredReviewMode(_) + | EventMsg::ExitedReviewMode(_) + | EventMsg::AgentMessageDelta(_) + | EventMsg::AgentReasoningDelta(_) + | EventMsg::AgentReasoningRawContentDelta(_) + | EventMsg::ItemStarted(_) + | EventMsg::ItemCompleted(_) + | EventMsg::UndoCompleted(_) + | EventMsg::UndoStarted(_) => {} } CodexStatus::Running } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index a59755008d..06b6b2fd59 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -286,6 +286,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/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 2d293549f1..eafbecd9bc 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -85,14 +85,7 @@ 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() -} From c5844f6233138766a8d9c7bbe88d60a95d038996 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 14:52:33 +0100 Subject: [PATCH 2/4] Fix some nits --- codex-rs/Cargo.lock | 16 +- codex-rs/core/src/tasks/ghost_snapshot.rs | 4 +- codex-rs/core/src/tasks/undo.rs | 11 +- codex-rs/git-tooling/Cargo.toml | 15 +- codex-rs/git-tooling/src/ghost_commits.rs | 221 +++++++++++++++++++++- codex-rs/git-tooling/src/lib.rs | 38 +++- codex-rs/git-tooling/src/operations.rs | 16 ++ codex-rs/protocol/Cargo.toml | 2 + codex-rs/protocol/src/models.rs | 5 +- codex-rs/tui/src/slash_command.rs | 4 +- 10 files changed, 296 insertions(+), 36 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index a5307efd45..679e445be2 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1209,8 +1209,11 @@ version = "0.0.0" dependencies = [ "assert_matches", "pretty_assertions", + "schemars 0.8.22", + "serde", "tempfile", "thiserror 2.0.16", + "ts-rs", "walkdir", ] @@ -1328,6 +1331,7 @@ version = "0.0.0" dependencies = [ "anyhow", "base64", + "codex-git-tooling", "icu_decimal", "icu_locale_core", "mcp-types", @@ -5457,9 +5461,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", @@ -5467,18 +5471,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/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index 97687a6fa5..8dc6a87e2f 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(&[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/undo.rs b/codex-rs/core/src/tasks/undo.rs index f2e38e63c1..a212e91945 100644 --- a/codex-rs/core/src/tasks/undo.rs +++ b/codex-rs/core/src/tasks/undo.rs @@ -8,7 +8,6 @@ use crate::state::TaskKind; use crate::tasks::SessionTask; use crate::tasks::SessionTaskContext; use async_trait::async_trait; -use codex_git_tooling::GhostCommit; use codex_git_tooling::restore_ghost_commit; use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; @@ -66,14 +65,14 @@ impl SessionTask for UndoTask { message: None, }; - let Some((idx, commit_id, parent)) = + let Some((idx, ghost_commit)) = items .iter() .enumerate() .rev() .find_map(|(idx, item)| match item { - ResponseItem::GhostSnapshot { commit_id, parent } => { - Some((idx, commit_id.clone(), parent.clone())) + ResponseItem::GhostSnapshot { ghost_commit } => { + Some((idx, ghost_commit.clone())) } _ => None, }) @@ -84,8 +83,8 @@ impl SessionTask for UndoTask { return None; }; + let commit_id = ghost_commit.id().to_string(); let repo_path = ctx.cwd.clone(); - let ghost_commit = GhostCommit::new(commit_id.clone(), parent); let restore_result = tokio::task::spawn_blocking(move || restore_ghost_commit(&repo_path, &ghost_commit)) .await; @@ -96,7 +95,7 @@ impl SessionTask for UndoTask { sess.replace_history(items).await; let short_id: String = commit_id.chars().take(7).collect(); info!( - commit_id = commit_id.as_str(), + commit_id = commit_id, "Undo restored ghost snapshot" ); completed.success = true; diff --git a/codex-rs/git-tooling/Cargo.toml b/codex-rs/git-tooling/Cargo.toml index 183221f3e1..c30d58de8d 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 6c2a86166c..48bbd0f044 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,138 @@ 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 repository's untracked files and directories scoped to an optional subdirectory. +fn capture_existing_untracked( + repo_root: &Path, + repo_prefix: Option<&Path>, +) -> Result { + 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(); + 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 +406,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 +437,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 +448,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 +661,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 f41d104385..72563c34fb 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,35 @@ 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 serde::{Deserialize, Serialize}; +use schemars::JsonSchema; +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 +52,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 3b387f8498..9e5f02dfc1 100644 --- a/codex-rs/git-tooling/src/operations.rs +++ b/codex-rs/git-tooling/src/operations.rs @@ -161,6 +161,22 @@ where }) } +pub(crate) fn run_git_for_stdout_all( + dir: &Path, + args: I, + env: Option<&[(OsString, OsString)]>, +) -> Result +where + I: IntoIterator, + S: AsRef, +{ + let run = run_git(dir, args, env)?; + 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/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 0393c85427..6249d870a3 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 02d557535e..051c70caa3 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -10,6 +10,7 @@ use ts_rs::TS; use crate::user_input::UserInput; use schemars::JsonSchema; +use codex_git_tooling::GhostCommit; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] @@ -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/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index eafbecd9bc..bb3be33099 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -85,7 +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)> { - SlashCommand::iter() - .map(|c| (c.command(), c)) - .collect() + SlashCommand::iter().map(|c| (c.command(), c)).collect() } From f7b960872f3a43e8a66479dc3da6757bc557236e Mon Sep 17 00:00:00 2001 From: jif-oai Date: Fri, 24 Oct 2025 14:56:18 +0100 Subject: [PATCH 3/4] FMT --- codex-rs/core/src/tasks/undo.rs | 5 +---- codex-rs/git-tooling/src/lib.rs | 3 ++- codex-rs/protocol/src/models.rs | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/codex-rs/core/src/tasks/undo.rs b/codex-rs/core/src/tasks/undo.rs index a212e91945..e834eea02d 100644 --- a/codex-rs/core/src/tasks/undo.rs +++ b/codex-rs/core/src/tasks/undo.rs @@ -94,10 +94,7 @@ impl SessionTask for UndoTask { 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" - ); + info!(commit_id = commit_id, "Undo restored ghost snapshot"); completed.success = true; completed.message = Some(format!("Undo restored snapshot {short_id}.")); } diff --git a/codex-rs/git-tooling/src/lib.rs b/codex-rs/git-tooling/src/lib.rs index 72563c34fb..e6e53924b6 100644 --- a/codex-rs/git-tooling/src/lib.rs +++ b/codex-rs/git-tooling/src/lib.rs @@ -12,8 +12,9 @@ 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 serde::{Deserialize, Serialize}; use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; use ts_rs::TS; type CommitID = String; diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 051c70caa3..095d7de4de 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -9,8 +9,8 @@ use serde::ser::Serializer; use ts_rs::TS; use crate::user_input::UserInput; -use schemars::JsonSchema; use codex_git_tooling::GhostCommit; +use schemars::JsonSchema; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "snake_case")] From a7dc03d5870ae298c34d25fe2d388d674369fa5f Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 27 Oct 2025 10:04:23 +0000 Subject: [PATCH 4/4] Add docs --- codex-rs/git-tooling/src/ghost_commits.rs | 7 ++++++- codex-rs/git-tooling/src/operations.rs | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/codex-rs/git-tooling/src/ghost_commits.rs b/codex-rs/git-tooling/src/ghost_commits.rs index 48bbd0f044..c5ebd7c02c 100644 --- a/codex-rs/git-tooling/src/ghost_commits.rs +++ b/codex-rs/git-tooling/src/ghost_commits.rs @@ -208,11 +208,14 @@ struct UntrackedSnapshot { dirs: Vec, } -/// Captures the repository's untracked files and directories scoped to an optional subdirectory. +/// 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"), @@ -231,6 +234,8 @@ fn capture_existing_untracked( } 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; diff --git a/codex-rs/git-tooling/src/operations.rs b/codex-rs/git-tooling/src/operations.rs index 9e5f02dfc1..415d63d568 100644 --- a/codex-rs/git-tooling/src/operations.rs +++ b/codex-rs/git-tooling/src/operations.rs @@ -161,6 +161,8 @@ 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, @@ -170,7 +172,10 @@ 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,