From 20583ba82b4216b3d1f2c2073d730ec81941817b Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 15 Dec 2025 10:13:53 +0100 Subject: [PATCH 1/3] feat: ghost snapshot v2 --- codex-rs/Cargo.lock | 6 +- codex-rs/core/src/tasks/ghost_snapshot.rs | 55 ++--- codex-rs/utils/git/src/ghost_commits.rs | 241 ++++++++++++++-------- 3 files changed, 173 insertions(+), 129 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e19156b4f289..dbb195a85d5b 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -44,7 +44,7 @@ dependencies = [ "bytestring", "derive_more 2.0.1", "encoding_rs", - "foldhash", + "foldhash 0.1.5", "futures-core", "http 0.2.12", "httparse", @@ -139,7 +139,7 @@ dependencies = [ "cfg-if", "derive_more 2.0.1", "encoding_rs", - "foldhash", + "foldhash 0.1.5", "futures-core", "futures-util", "impl-more", @@ -154,7 +154,7 @@ dependencies = [ "serde_json", "serde_urlencoded", "smallvec", - "socket2 0.6.0", + "socket2 0.6.1", "time", "tracing", "url", diff --git a/codex-rs/core/src/tasks/ghost_snapshot.rs b/codex-rs/core/src/tasks/ghost_snapshot.rs index af694464d428..ec6f829c796c 100644 --- a/codex-rs/core/src/tasks/ghost_snapshot.rs +++ b/codex-rs/core/src/tasks/ghost_snapshot.rs @@ -8,8 +8,7 @@ use async_trait::async_trait; use codex_git::CreateGhostCommitOptions; use codex_git::GhostSnapshotReport; use codex_git::GitToolingError; -use codex_git::capture_ghost_snapshot_report; -use codex_git::create_ghost_commit; +use codex_git::create_ghost_commit_with_report; use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; use codex_utils_readiness::Readiness; @@ -74,48 +73,30 @@ impl SessionTask for GhostSnapshotTask { _ = async { let repo_path = ctx_for_task.cwd.clone(); let ghost_snapshot = ctx_for_task.ghost_snapshot.clone(); - let ignore_large_untracked_dirs = ghost_snapshot.ignore_large_untracked_dirs; - // First, compute a snapshot report so we can warn about - // large untracked directories before running the heavier - // snapshot logic. - if let Ok(Ok(report)) = tokio::task::spawn_blocking({ - let repo_path = repo_path.clone(); - let ghost_snapshot = ghost_snapshot.clone(); - move || { - let options = - CreateGhostCommitOptions::new(&repo_path).ghost_snapshot(ghost_snapshot); - capture_ghost_snapshot_report(&options) - } - }) - .await - { - for message in - format_snapshot_warnings( - ghost_snapshot.ignore_large_untracked_files, - ignore_large_untracked_dirs, - &report, - ) - { - session - .session - .send_event( - &ctx_for_task, - EventMsg::Warning(WarningEvent { message }), - ) - .await; - } - } - + let ghost_snapshot_for_commit = ghost_snapshot.clone(); // Required to run in a dedicated blocking pool. match tokio::task::spawn_blocking(move || { let options = - CreateGhostCommitOptions::new(&repo_path).ghost_snapshot(ghost_snapshot); - create_ghost_commit(&options) + CreateGhostCommitOptions::new(&repo_path).ghost_snapshot(ghost_snapshot_for_commit); + create_ghost_commit_with_report(&options) }) .await { - Ok(Ok(ghost_commit)) => { + Ok(Ok((ghost_commit, report))) => { info!("ghost snapshot blocking task finished"); + for message in format_snapshot_warnings( + ghost_snapshot.ignore_large_untracked_files, + ghost_snapshot.ignore_large_untracked_dirs, + &report, + ) { + session + .session + .send_event( + &ctx_for_task, + EventMsg::Warning(WarningEvent { message }), + ) + .await; + } session .session .record_conversation_items(&ctx, &[ResponseItem::GhostSnapshot { diff --git a/codex-rs/utils/git/src/ghost_commits.rs b/codex-rs/utils/git/src/ghost_commits.rs index 9de2b52f1990..1248bbff83f4 100644 --- a/codex-rs/utils/git/src/ghost_commits.rs +++ b/codex-rs/utils/git/src/ghost_commits.rs @@ -306,13 +306,14 @@ pub fn create_ghost_commit_with_report( let repo_prefix = repo_subdir(repo_root.as_path(), options.repo_path); let parent = resolve_head(repo_root.as_path())?; let force_include = prepare_force_include(repo_prefix.as_deref(), &options.force_include)?; - let existing_untracked = capture_existing_untracked( + let status_snapshot = capture_status_snapshot( repo_root.as_path(), repo_prefix.as_deref(), options.ghost_snapshot.ignore_large_untracked_files, options.ghost_snapshot.ignore_large_untracked_dirs, &force_include, )?; + let existing_untracked = status_snapshot.untracked; let warning_ignored_files = existing_untracked .ignored_untracked_files @@ -347,22 +348,10 @@ pub fn create_ghost_commit_with_report( )?; } - 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()]); - } - - run_git_for_status(repo_root.as_path(), add_args, Some(base_env.as_slice()))?; - remove_large_untracked_dirs_from_index( - repo_root.as_path(), - base_env.as_slice(), - &existing_untracked.ignored_large_untracked_dir_files, - )?; - remove_large_untracked_files_from_index( - repo_root.as_path(), - base_env.as_slice(), - &existing_untracked.ignored_untracked_files, - )?; + let mut index_paths = status_snapshot.tracked_paths; + index_paths.extend(existing_untracked.untracked_files_for_index.iter().cloned()); + let index_paths = dedupe_paths(index_paths); + add_paths_to_index(repo_root.as_path(), base_env.as_slice(), &index_paths)?; if !force_include.is_empty() { let mut args = Vec::with_capacity(force_include.len() + 2); args.push(OsString::from("add")); @@ -490,22 +479,29 @@ fn restore_to_commit_inner( struct UntrackedSnapshot { files: Vec, dirs: Vec, + untracked_files_for_index: Vec, ignored_untracked_files: Vec, ignored_large_untracked_dirs: Vec, ignored_large_untracked_dir_files: 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( +#[derive(Default)] +struct StatusSnapshot { + tracked_paths: Vec, + untracked: UntrackedSnapshot, +} + +/// Captures the working tree status under `repo_root`, optionally limited by `repo_prefix`. +/// Returns the result as a `StatusSnapshot`. +fn capture_status_snapshot( repo_root: &Path, repo_prefix: Option<&Path>, ignore_large_untracked_files: Option, ignore_large_untracked_dirs: Option, force_include: &[PathBuf], -) -> Result { +) -> Result { // Ask git for the zero-delimited porcelain status so we can enumerate - // every untracked path (including ones filtered by prefix). + // tracked, untracked, and ignored entries (including ones filtered by prefix). let mut args = vec![ OsString::from("status"), OsString::from("--porcelain=2"), @@ -519,55 +515,90 @@ fn capture_existing_untracked( let output = run_git_for_stdout_all(repo_root, args, None)?; if output.is_empty() { - return Ok(UntrackedSnapshot::default()); + return Ok(StatusSnapshot::default()); } - let mut snapshot = UntrackedSnapshot::default(); + let mut snapshot = StatusSnapshot::default(); let mut untracked_files_for_dir_scan: Vec = Vec::new(); - // Each entry is of the form " " where code is '?' (untracked) - // or '!' (ignored); everything else is irrelevant to this snapshot. + let mut expect_rename_source = false; 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() { + if expect_rename_source { + let normalized = normalize_relative_path(Path::new(entry))?; + snapshot.tracked_paths.push(normalized); + expect_rename_source = false; continue; } - let normalized = normalize_relative_path(Path::new(path_part))?; - if should_ignore_for_snapshot(&normalized) { - continue; - } - let absolute = repo_root.join(&normalized); - let is_dir = absolute.is_dir(); - if is_dir { - snapshot.dirs.push(normalized); - } else if code == "?" { - untracked_files_for_dir_scan.push(normalized.clone()); - if let Some(threshold) = ignore_large_untracked_files - && threshold > 0 - && !is_force_included(&normalized, force_include) - && let Ok(Some(byte_size)) = untracked_file_size(&absolute) - && byte_size > threshold - { - snapshot.ignored_untracked_files.push(IgnoredUntrackedFile { - path: normalized, - byte_size, - }); - } else { - snapshot.files.push(normalized); + let record_type = entry.as_bytes().first().copied().unwrap_or(b' '); + match record_type { + b'?' | b'!' => { + 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 path_part.is_empty() { + continue; + } + + let normalized = normalize_relative_path(Path::new(path_part))?; + if should_ignore_for_snapshot(&normalized) { + continue; + } + let absolute = repo_root.join(&normalized); + let is_dir = absolute.is_dir(); + if is_dir { + snapshot.untracked.dirs.push(normalized); + } else if code == "?" { + untracked_files_for_dir_scan.push(normalized.clone()); + if let Some(threshold) = ignore_large_untracked_files + && threshold > 0 + && !is_force_included(&normalized, force_include) + && let Ok(Some(byte_size)) = untracked_file_size(&absolute) + && byte_size > threshold + { + snapshot + .untracked + .ignored_untracked_files + .push(IgnoredUntrackedFile { + path: normalized, + byte_size, + }); + } else { + snapshot.untracked.files.push(normalized.clone()); + snapshot + .untracked + .untracked_files_for_index + .push(normalized); + } + } else { + snapshot.untracked.files.push(normalized); + } } - } else { - snapshot.files.push(normalized); + b'1' => { + if let Some(path) = extract_status_path_after_fields(entry, 8) { + let normalized = normalize_relative_path(Path::new(path))?; + snapshot.tracked_paths.push(normalized); + } + } + b'2' => { + if let Some(path) = extract_status_path_after_fields(entry, 9) { + let normalized = normalize_relative_path(Path::new(path))?; + snapshot.tracked_paths.push(normalized); + } + expect_rename_source = true; + } + b'u' => { + if let Some(path) = extract_status_path_after_fields(entry, 9) { + let normalized = normalize_relative_path(Path::new(path))?; + snapshot.tracked_paths.push(normalized); + } + } + _ => {} } } @@ -576,7 +607,7 @@ fn capture_existing_untracked( { let ignored_large_untracked_dirs = detect_large_untracked_dirs( &untracked_files_for_dir_scan, - &snapshot.dirs, + &snapshot.untracked.dirs, Some(threshold), ) .into_iter() @@ -590,28 +621,69 @@ fn capture_existing_untracked( .collect::>(); snapshot + .untracked .files .retain(|path| !ignored_dir_paths.iter().any(|dir| path.starts_with(dir))); snapshot + .untracked .dirs .retain(|path| !ignored_dir_paths.iter().any(|dir| path.starts_with(dir))); - snapshot.ignored_untracked_files.retain(|file| { + snapshot + .untracked + .untracked_files_for_index + .retain(|path| !ignored_dir_paths.iter().any(|dir| path.starts_with(dir))); + snapshot.untracked.ignored_untracked_files.retain(|file| { !ignored_dir_paths .iter() .any(|dir| file.path.starts_with(dir)) }); - snapshot.ignored_large_untracked_dir_files = untracked_files_for_dir_scan + snapshot.untracked.ignored_large_untracked_dir_files = untracked_files_for_dir_scan .into_iter() .filter(|path| ignored_dir_paths.iter().any(|dir| path.starts_with(dir))) .collect(); - snapshot.ignored_large_untracked_dirs = ignored_large_untracked_dirs; + snapshot.untracked.ignored_large_untracked_dirs = ignored_large_untracked_dirs; } } Ok(snapshot) } +/// 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>, + ignore_large_untracked_files: Option, + ignore_large_untracked_dirs: Option, + force_include: &[PathBuf], +) -> Result { + Ok(capture_status_snapshot( + repo_root, + repo_prefix, + ignore_large_untracked_files, + ignore_large_untracked_dirs, + force_include, + )? + .untracked) +} + +fn extract_status_path_after_fields(record: &str, fields_before_path: i64) -> Option<&str> { + if fields_before_path <= 0 { + return None; + } + let mut spaces = 0_i64; + for (idx, byte) in record.as_bytes().iter().enumerate() { + if *byte == b' ' { + spaces += 1; + if spaces == fields_before_path { + return record.get((idx + 1)..).filter(|path| !path.is_empty()); + } + } + } + None +} + fn should_ignore_for_snapshot(path: &Path) -> bool { path.components().any(|component| { if let Component::Normal(name) = component @@ -657,27 +729,7 @@ fn untracked_file_size(path: &Path) -> io::Result> { Ok(Some(len_i64)) } -fn remove_large_untracked_files_from_index( - repo_root: &Path, - env: &[(OsString, OsString)], - ignored: &[IgnoredUntrackedFile], -) -> Result<(), GitToolingError> { - let paths = ignored - .iter() - .map(|entry| entry.path.clone()) - .collect::>(); - remove_paths_from_index(repo_root, env, &paths) -} - -fn remove_large_untracked_dirs_from_index( - repo_root: &Path, - env: &[(OsString, OsString)], - paths: &[PathBuf], -) -> Result<(), GitToolingError> { - remove_paths_from_index(repo_root, env, paths) -} - -fn remove_paths_from_index( +fn add_paths_to_index( repo_root: &Path, env: &[(OsString, OsString)], paths: &[PathBuf], @@ -686,11 +738,11 @@ fn remove_paths_from_index( return Ok(()); } - const CHUNK_SIZE: usize = 64; - for chunk in paths.chunks(CHUNK_SIZE) { + let chunk_size = usize::try_from(64_i64).unwrap_or(1); + for chunk in paths.chunks(chunk_size) { let mut args = vec![ - OsString::from("update-index"), - OsString::from("--force-remove"), + OsString::from("add"), + OsString::from("--all"), OsString::from("--"), ]; args.extend(chunk.iter().map(|path| path.as_os_str().to_os_string())); @@ -700,6 +752,17 @@ fn remove_paths_from_index( Ok(()) } +fn dedupe_paths(paths: Vec) -> Vec { + let mut seen = HashSet::new(); + let mut result = Vec::new(); + for path in paths { + if seen.insert(path.clone()) { + result.push(path); + } + } + result +} + fn merge_preserved_untracked_files( mut files: Vec, ignored: &[IgnoredUntrackedFile], From 68b0bad9777d73bd53bc0b4b667f3516cea90a05 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 15 Dec 2025 10:29:14 +0100 Subject: [PATCH 2/3] nit --- codex-rs/utils/git/src/ghost_commits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/utils/git/src/ghost_commits.rs b/codex-rs/utils/git/src/ghost_commits.rs index 1248bbff83f4..4819614482c2 100644 --- a/codex-rs/utils/git/src/ghost_commits.rs +++ b/codex-rs/utils/git/src/ghost_commits.rs @@ -593,7 +593,7 @@ fn capture_status_snapshot( expect_rename_source = true; } b'u' => { - if let Some(path) = extract_status_path_after_fields(entry, 9) { + if let Some(path) = extract_status_path_after_fields(entry, 10) { let normalized = normalize_relative_path(Path::new(path))?; snapshot.tracked_paths.push(normalized); } From 685e538d2821f0d798a670a8d22fbc6b29763c17 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 15 Dec 2025 10:58:04 +0100 Subject: [PATCH 3/3] Add comments --- codex-rs/utils/git/src/ghost_commits.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/codex-rs/utils/git/src/ghost_commits.rs b/codex-rs/utils/git/src/ghost_commits.rs index 4819614482c2..1e3c606f09d2 100644 --- a/codex-rs/utils/git/src/ghost_commits.rs +++ b/codex-rs/utils/git/src/ghost_commits.rs @@ -337,6 +337,12 @@ pub fn create_ghost_commit_with_report( OsString::from("GIT_INDEX_FILE"), OsString::from(index_path.as_os_str()), )]; + // Use a temporary index so snapshotting does not disturb the user's index state. + // Example plumbing sequence: + // GIT_INDEX_FILE=/tmp/index git read-tree HEAD + // GIT_INDEX_FILE=/tmp/index git add --all -- + // GIT_INDEX_FILE=/tmp/index git write-tree + // GIT_INDEX_FILE=/tmp/index git commit-tree -p -m "codex snapshot" // Pre-populate the temporary index with HEAD so unchanged tracked files // are included in the snapshot tree. @@ -351,6 +357,8 @@ pub fn create_ghost_commit_with_report( let mut index_paths = status_snapshot.tracked_paths; index_paths.extend(existing_untracked.untracked_files_for_index.iter().cloned()); let index_paths = dedupe_paths(index_paths); + // Stage tracked + new files into the temp index so write-tree reflects the working tree. + // We use `git add --all` to make deletions show up in the snapshot tree too. add_paths_to_index(repo_root.as_path(), base_env.as_slice(), &index_paths)?; if !force_include.is_empty() { let mut args = Vec::with_capacity(force_include.len() + 2); @@ -382,6 +390,8 @@ pub fn create_ghost_commit_with_report( result }; + // `git commit-tree` writes a detached commit object without updating refs, + // which keeps snapshots out of the user's branch history. // Retrieve commit ID. let commit_id = run_git_for_stdout( repo_root.as_path(), @@ -457,6 +467,9 @@ fn restore_to_commit_inner( repo_prefix: Option<&Path>, commit_id: &str, ) -> Result<(), GitToolingError> { + // `git restore` resets both the index and working tree to the snapshot commit. + // Example: + // git restore --source --worktree --staged -- let mut restore_args = vec![ OsString::from("restore"), OsString::from("--source"), @@ -502,6 +515,7 @@ fn capture_status_snapshot( ) -> Result { // Ask git for the zero-delimited porcelain status so we can enumerate // tracked, untracked, and ignored entries (including ones filtered by prefix). + // This keeps the snapshot consistent without multiple git invocations. let mut args = vec![ OsString::from("status"), OsString::from("--porcelain=2"), @@ -746,6 +760,7 @@ fn add_paths_to_index( OsString::from("--"), ]; args.extend(chunk.iter().map(|path| path.as_os_str().to_os_string())); + // Chunk the argv to avoid oversized command lines on large repos. run_git_for_status(repo_root, args, Some(env))?; }