diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d025f3736a..7db75b27db 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -771,13 +771,15 @@ dependencies = [ "anyhow", "async-trait", "chrono", - "codex-apply-patch", "codex-backend-client", "codex-cloud-tasks-api", "diffy", + "once_cell", + "regex", "reqwest", "serde", "serde_json", + "tempfile", "thiserror 2.0.16", "tokio", ] diff --git a/codex-rs/cloud-tasks-client/Cargo.toml b/codex-rs/cloud-tasks-client/Cargo.toml index 1e5b4c125e..ec5a65ef41 100644 --- a/codex-rs/cloud-tasks-client/Cargo.toml +++ b/codex-rs/cloud-tasks-client/Cargo.toml @@ -20,7 +20,6 @@ anyhow = "1" codex-cloud-tasks-api = { path = "../cloud-tasks-api" } async-trait = "0.1" chrono = { version = "0.4", features = ["serde"] } -codex-apply-patch = { path = "../apply-patch" } diffy = "0.4.2" reqwest = { version = "0.12", features = ["json"], optional = true } serde = { version = "1", features = ["derive"] } @@ -28,3 +27,6 @@ serde_json = "1" thiserror = "2.0.12" tokio = { version = "1", features = ["macros", "rt-multi-thread"], optional = true } codex-backend-client = { path = "../backend-client", optional = true } +regex = "1" +tempfile = "3" +once_cell = "1" diff --git a/codex-rs/cloud-tasks-client/src/git_apply.rs b/codex-rs/cloud-tasks-client/src/git_apply.rs new file mode 100644 index 0000000000..8edb8c00f3 --- /dev/null +++ b/codex-rs/cloud-tasks-client/src/git_apply.rs @@ -0,0 +1,648 @@ +use regex::Regex; +use std::ffi::OsStr; +use std::io; +use std::path::Path; +use std::path::PathBuf; + +use once_cell::sync::Lazy; + +#[derive(Debug, Clone)] +pub struct ApplyGitRequest { + pub cwd: PathBuf, + pub diff: String, + pub revert: bool, +} + +#[derive(Debug, Clone)] +pub struct ApplyGitResult { + pub exit_code: i32, + pub applied_paths: Vec, + pub skipped_paths: Vec, + pub conflicted_paths: Vec, + pub stdout: String, + pub stderr: String, + pub cmd_for_log: String, +} + +pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result { + let git_root = resolve_git_root(&req.cwd)?; + + // Write unified diff into a temporary file + let (tmpdir, patch_path) = write_temp_patch(&req.diff)?; + // Keep tmpdir alive until function end to ensure the file exists + let _guard = tmpdir; + + if req.revert { + // Stage WT paths first to avoid index mismatch on revert. + stage_paths(&git_root, &req.diff)?; + } + + // Build git args + let mut args: Vec = vec!["apply".into(), "--3way".into()]; + if req.revert { + args.push("-R".into()); + } + + // Optional: additional git config via env knob (defaults OFF) + let mut cfg_parts: Vec = Vec::new(); + if let Ok(cfg) = std::env::var("CODEX_APPLY_GIT_CFG") { + for pair in cfg.split(',') { + let p = pair.trim(); + if p.is_empty() || !p.contains('=') { + continue; + } + cfg_parts.push("-c".into()); + cfg_parts.push(p.to_string()); + } + } + + args.push(patch_path.to_string_lossy().to_string()); + + // Optional preflight: CODEX_APPLY_PREFLIGHT=1 (dry-run only; do not modify WT) + if matches!( + std::env::var("CODEX_APPLY_PREFLIGHT").ok().as_deref(), + Some("1" | "true" | "yes") + ) { + let mut check_args = vec!["apply".to_string(), "--check".to_string()]; + if req.revert { + check_args.push("-R".to_string()); + } + check_args.push(patch_path.to_string_lossy().to_string()); + let rendered = render_command_for_log(&git_root, &cfg_parts, &check_args); + let (c_code, c_out, c_err) = run_git(&git_root, &cfg_parts, &check_args)?; + let (mut applied_paths, mut skipped_paths, mut conflicted_paths) = + parse_git_apply_output(&c_out, &c_err); + applied_paths.sort(); + applied_paths.dedup(); + skipped_paths.sort(); + skipped_paths.dedup(); + conflicted_paths.sort(); + conflicted_paths.dedup(); + return Ok(ApplyGitResult { + exit_code: c_code, + applied_paths, + skipped_paths, + conflicted_paths, + stdout: c_out, + stderr: c_err, + cmd_for_log: rendered, + }); + } + + let cmd_for_log = render_command_for_log(&git_root, &cfg_parts, &args); + let (code, stdout, stderr) = run_git(&git_root, &cfg_parts, &args)?; + + let (mut applied_paths, mut skipped_paths, mut conflicted_paths) = + parse_git_apply_output(&stdout, &stderr); + applied_paths.sort(); + applied_paths.dedup(); + skipped_paths.sort(); + skipped_paths.dedup(); + conflicted_paths.sort(); + conflicted_paths.dedup(); + + Ok(ApplyGitResult { + exit_code: code, + applied_paths, + skipped_paths, + conflicted_paths, + stdout, + stderr, + cmd_for_log, + }) +} + +fn resolve_git_root(cwd: &Path) -> io::Result { + let out = std::process::Command::new("git") + .arg("rev-parse") + .arg("--show-toplevel") + .current_dir(cwd) + .output()?; + let code = out.status.code().unwrap_or(-1); + if code != 0 { + return Err(io::Error::other(format!( + "not a git repository (exit {}): {}", + code, + String::from_utf8_lossy(&out.stderr) + ))); + } + let root = String::from_utf8_lossy(&out.stdout).trim().to_string(); + Ok(PathBuf::from(root)) +} + +fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, PathBuf)> { + let dir = tempfile::tempdir()?; + let path = dir.path().join("patch.diff"); + std::fs::write(&path, diff)?; + Ok((dir, path)) +} + +fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, String, String)> { + let mut cmd = std::process::Command::new("git"); + for p in git_cfg { + cmd.arg(p); + } + for a in args { + cmd.arg(a); + } + let out = cmd.current_dir(cwd).output()?; + let code = out.status.code().unwrap_or(-1); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + Ok((code, stdout, stderr)) +} + +fn quote_shell(s: &str) -> String { + let simple = s + .chars() + .all(|c| c.is_ascii_alphanumeric() || "-_.:/@%+".contains(c)); + if simple { + s.to_string() + } else { + format!("'{}'", s.replace('\'', "'\\''")) + } +} + +fn render_command_for_log(cwd: &Path, git_cfg: &[String], args: &[String]) -> String { + let mut parts: Vec = Vec::new(); + parts.push("git".to_string()); + for a in git_cfg { + parts.push(quote_shell(a)); + } + for a in args { + parts.push(quote_shell(a)); + } + format!( + "(cd {} && {})", + quote_shell(&cwd.display().to_string()), + parts.join(" ") + ) +} + +pub fn extract_paths_from_patch(diff_text: &str) -> Vec { + static RE: Lazy = Lazy::new(|| { + Regex::new(r"(?m)^diff --git a/(.*?) b/(.*)$") + .unwrap_or_else(|e| panic!("invalid regex: {e}")) + }); + let mut set = std::collections::BTreeSet::new(); + for caps in RE.captures_iter(diff_text) { + if let Some(a) = caps.get(1).map(|m| m.as_str()) + && a != "/dev/null" + && !a.trim().is_empty() + { + set.insert(a.to_string()); + } + if let Some(b) = caps.get(2).map(|m| m.as_str()) + && b != "/dev/null" + && !b.trim().is_empty() + { + set.insert(b.to_string()); + } + } + set.into_iter().collect() +} + +pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> { + let paths = extract_paths_from_patch(diff); + let mut existing: Vec = Vec::new(); + for p in paths { + let joined = git_root.join(&p); + if std::fs::symlink_metadata(&joined).is_ok() { + existing.push(p); + } + } + if existing.is_empty() { + return Ok(()); + } + let mut cmd = std::process::Command::new("git"); + cmd.arg("add"); + cmd.arg("--"); + for p in &existing { + cmd.arg(OsStr::new(p)); + } + let out = cmd.current_dir(git_root).output()?; + let _code = out.status.code().unwrap_or(-1); + // We do not hard fail staging; best-effort is OK. Return Ok even on non-zero. + Ok(()) +} + +// ============ Parser ported from VS Code (TS) ============ + +pub fn parse_git_apply_output( + stdout: &str, + stderr: &str, +) -> (Vec, Vec, Vec) { + let combined = [stdout, stderr] + .iter() + .filter(|s| !s.is_empty()) + .cloned() + .collect::>() + .join("\n"); + + let mut applied = std::collections::BTreeSet::new(); + let mut skipped = std::collections::BTreeSet::new(); + let mut conflicted = std::collections::BTreeSet::new(); + let mut last_seen_path: Option = None; + + fn add(set: &mut std::collections::BTreeSet, raw: &str) { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return; + } + let first = trimmed.chars().next().unwrap_or('\0'); + let last = trimmed.chars().last().unwrap_or('\0'); + let unquoted = if (first == '"' || first == '\'') && last == first && trimmed.len() >= 2 { + &trimmed[1..trimmed.len() - 1] + } else { + trimmed + }; + if !unquoted.is_empty() { + set.insert(unquoted.to_string()); + } + } + + static APPLIED_CLEAN: Lazy = + Lazy::new(|| regex_ci("^Applied patch(?: to)?\\s+(?P.+?)\\s+cleanly\\.?$")); + static APPLIED_CONFLICTS: Lazy = + Lazy::new(|| regex_ci("^Applied patch(?: to)?\\s+(?P.+?)\\s+with conflicts\\.?$")); + static APPLYING_WITH_REJECTS: Lazy = Lazy::new(|| { + regex_ci("^Applying patch\\s+(?P.+?)\\s+with\\s+\\d+\\s+rejects?\\.{0,3}$") + }); + static CHECKING_PATCH: Lazy = + Lazy::new(|| regex_ci("^Checking patch\\s+(?P.+?)\\.\\.\\.$")); + static UNMERGED_LINE: Lazy = Lazy::new(|| regex_ci("^U\\s+(?P.+)$")); + static PATCH_FAILED: Lazy = + Lazy::new(|| regex_ci("^error:\\s+patch failed:\\s+(?P.+?)(?::\\d+)?(?:\\s|$)")); + static DOES_NOT_APPLY: Lazy = + Lazy::new(|| regex_ci("^error:\\s+(?P.+?):\\s+patch does not apply$")); + static THREE_WAY_START: Lazy = Lazy::new(|| { + regex_ci("^(?:Performing three-way merge|Falling back to three-way merge)\\.\\.\\.$") + }); + static THREE_WAY_FAILED: Lazy = + Lazy::new(|| regex_ci("^Failed to perform three-way merge\\.\\.\\.$")); + static FALLBACK_DIRECT: Lazy = + Lazy::new(|| regex_ci("^Falling back to direct application\\.\\.\\.$")); + static LACKS_BLOB: Lazy = Lazy::new(|| { + regex_ci( + "^(?:error: )?repository lacks the necessary blob to (?:perform|fall back on) 3-?way merge\\.?$", + ) + }); + static INDEX_MISMATCH: Lazy = + Lazy::new(|| regex_ci("^error:\\s+(?P.+?):\\s+does not match index\\b")); + static NOT_IN_INDEX: Lazy = + Lazy::new(|| regex_ci("^error:\\s+(?P.+?):\\s+does not exist in index\\b")); + static ALREADY_EXISTS_WT: Lazy = Lazy::new(|| { + regex_ci("^error:\\s+(?P.+?)\\s+already exists in (?:the )?working directory\\b") + }); + static FILE_EXISTS: Lazy = + Lazy::new(|| regex_ci("^error:\\s+patch failed:\\s+(?P.+?)\\s+File exists")); + static RENAMED_DELETED: Lazy = + Lazy::new(|| regex_ci("^error:\\s+path\\s+(?P.+?)\\s+has been renamed\\/deleted")); + static CANNOT_APPLY_BINARY: Lazy = Lazy::new(|| { + regex_ci( + "^error:\\s+cannot apply binary patch to\\s+['\\\"]?(?P.+?)['\\\"]?\\s+without full index line$", + ) + }); + static BINARY_DOES_NOT_APPLY: Lazy = Lazy::new(|| { + regex_ci("^error:\\s+binary patch does not apply to\\s+['\\\"]?(?P.+?)['\\\"]?$") + }); + static BINARY_INCORRECT_RESULT: Lazy = Lazy::new(|| { + regex_ci( + "^error:\\s+binary patch to\\s+['\\\"]?(?P.+?)['\\\"]?\\s+creates incorrect result\\b", + ) + }); + static CANNOT_READ_CURRENT: Lazy = Lazy::new(|| { + regex_ci("^error:\\s+cannot read the current contents of\\s+['\\\"]?(?P.+?)['\\\"]?$") + }); + static SKIPPED_PATCH: Lazy = + Lazy::new(|| regex_ci("^Skipped patch\\s+['\\\"]?(?P.+?)['\\\"]\\.$")); + static CANNOT_MERGE_BINARY_WARN: Lazy = Lazy::new(|| { + regex_ci( + "^warning:\\s*Cannot merge binary files:\\s+(?P.+?)\\s+\\(ours\\s+vs\\.\\s+theirs\\)", + ) + }); + + for raw_line in combined.lines() { + let line = raw_line.trim(); + if line.is_empty() { + continue; + } + + // === "Checking patch ..." tracking === + if let Some(c) = CHECKING_PATCH.captures(line) { + if let Some(m) = c.name("path") { + last_seen_path = Some(m.as_str().to_string()); + } + continue; + } + + // === Status lines === + if let Some(c) = APPLIED_CLEAN.captures(line) { + if let Some(m) = c.name("path") { + add(&mut applied, m.as_str()); + let p = applied.iter().next_back().cloned(); + if let Some(p) = p { + conflicted.remove(&p); + skipped.remove(&p); + last_seen_path = Some(p); + } + } + continue; + } + if let Some(c) = APPLIED_CONFLICTS.captures(line) { + if let Some(m) = c.name("path") { + add(&mut conflicted, m.as_str()); + let p = conflicted.iter().next_back().cloned(); + if let Some(p) = p { + applied.remove(&p); + skipped.remove(&p); + last_seen_path = Some(p); + } + } + continue; + } + if let Some(c) = APPLYING_WITH_REJECTS.captures(line) { + if let Some(m) = c.name("path") { + add(&mut conflicted, m.as_str()); + let p = conflicted.iter().next_back().cloned(); + if let Some(p) = p { + applied.remove(&p); + skipped.remove(&p); + last_seen_path = Some(p); + } + } + continue; + } + + // === “U ” after conflicts === + if let Some(c) = UNMERGED_LINE.captures(line) { + if let Some(m) = c.name("path") { + add(&mut conflicted, m.as_str()); + let p = conflicted.iter().next_back().cloned(); + if let Some(p) = p { + applied.remove(&p); + skipped.remove(&p); + last_seen_path = Some(p); + } + } + continue; + } + + // === Early hints === + if PATCH_FAILED.is_match(line) || DOES_NOT_APPLY.is_match(line) { + if let Some(c) = PATCH_FAILED + .captures(line) + .or_else(|| DOES_NOT_APPLY.captures(line)) + && let Some(m) = c.name("path") + { + add(&mut skipped, m.as_str()); + last_seen_path = Some(m.as_str().to_string()); + } + continue; + } + + // === Ignore narration === + if THREE_WAY_START.is_match(line) || FALLBACK_DIRECT.is_match(line) { + continue; + } + + // === 3-way failed entirely; attribute to last_seen_path === + if THREE_WAY_FAILED.is_match(line) || LACKS_BLOB.is_match(line) { + if let Some(p) = last_seen_path.clone() { + add(&mut skipped, &p); + applied.remove(&p); + conflicted.remove(&p); + } + continue; + } + + // === Skips / I/O problems === + if let Some(c) = INDEX_MISMATCH + .captures(line) + .or_else(|| NOT_IN_INDEX.captures(line)) + .or_else(|| ALREADY_EXISTS_WT.captures(line)) + .or_else(|| FILE_EXISTS.captures(line)) + .or_else(|| RENAMED_DELETED.captures(line)) + .or_else(|| CANNOT_APPLY_BINARY.captures(line)) + .or_else(|| BINARY_DOES_NOT_APPLY.captures(line)) + .or_else(|| BINARY_INCORRECT_RESULT.captures(line)) + .or_else(|| CANNOT_READ_CURRENT.captures(line)) + .or_else(|| SKIPPED_PATCH.captures(line)) + { + if let Some(m) = c.name("path") { + add(&mut skipped, m.as_str()); + let p_now = skipped.iter().next_back().cloned(); + if let Some(p) = p_now { + applied.remove(&p); + conflicted.remove(&p); + last_seen_path = Some(p); + } + } + continue; + } + + // === Warnings that imply conflicts === + if let Some(c) = CANNOT_MERGE_BINARY_WARN.captures(line) { + if let Some(m) = c.name("path") { + add(&mut conflicted, m.as_str()); + let p = conflicted.iter().next_back().cloned(); + if let Some(p) = p { + applied.remove(&p); + skipped.remove(&p); + last_seen_path = Some(p); + } + } + continue; + } + } + + // Final precedence: conflicts > applied > skipped + for p in conflicted.iter().cloned().collect::>() { + applied.remove(&p); + skipped.remove(&p); + } + for p in applied.iter().cloned().collect::>() { + skipped.remove(&p); + } + + ( + applied.into_iter().collect(), + skipped.into_iter().collect(), + conflicted.into_iter().collect(), + ) +} + +fn regex_ci(pat: &str) -> Regex { + Regex::new(&format!("(?i){pat}")).unwrap_or_else(|e| panic!("invalid regex: {e}")) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + use std::sync::OnceLock; + + fn env_lock() -> &'static Mutex<()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + + fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) { + let out = std::process::Command::new(args[0]) + .args(&args[1..]) + .current_dir(cwd) + .output() + .expect("spawn ok"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).into_owned(), + String::from_utf8_lossy(&out.stderr).into_owned(), + ) + } + + fn init_repo() -> tempfile::TempDir { + let dir = tempfile::tempdir().expect("tempdir"); + let root = dir.path(); + // git init and minimal identity + let _ = run(root, &["git", "init"]); + let _ = run(root, &["git", "config", "user.email", "codex@example.com"]); + let _ = run(root, &["git", "config", "user.name", "Codex"]); + dir + } + + #[test] + fn apply_add_success() { + let _g = env_lock().lock().unwrap(); + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let repo = init_repo(); + let root = repo.path().to_path_buf(); + + let diff = "diff --git a/hello.txt b/hello.txt\nnew file mode 100644\n--- /dev/null\n+++ b/hello.txt\n@@ -0,0 +1,2 @@\n+hello\n+world\n"; + let req = ApplyGitRequest { + cwd: root.clone(), + diff: diff.to_string(), + revert: false, + }; + let r = apply_git_patch(&req).expect("run apply"); + assert_eq!(r.exit_code, 0, "exit code 0"); + // File exists now + assert!(root.join("hello.txt").exists()); + } + + #[test] + fn apply_modify_conflict() { + let _g = env_lock().lock().unwrap(); + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let repo = init_repo(); + let root = repo.path(); + // seed file and commit + std::fs::write(root.join("file.txt"), "line1\nline2\nline3\n").unwrap(); + let _ = run(root, &["git", "add", "file.txt"]); + let _ = run(root, &["git", "commit", "-m", "seed"]); + // local edit (unstaged) + std::fs::write(root.join("file.txt"), "line1\nlocal2\nline3\n").unwrap(); + // patch wants to change the same line differently + let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,3 +1,3 @@\n line1\n-line2\n+remote2\n line3\n"; + let req = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: diff.to_string(), + revert: false, + }; + let r = apply_git_patch(&req).expect("run apply"); + assert_ne!(r.exit_code, 0, "non-zero exit on conflict"); + } + + #[test] + fn apply_modify_skipped_missing_index() { + let _g = env_lock().lock().unwrap(); + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let repo = init_repo(); + let root = repo.path(); + // Try to modify a file that is not in the index + let diff = "diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1,1 @@\n-old\n+new\n"; + let req = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: diff.to_string(), + revert: false, + }; + let r = apply_git_patch(&req).expect("run apply"); + assert_ne!(r.exit_code, 0, "non-zero exit on missing index"); + } + + #[test] + fn apply_then_revert_success() { + let _g = env_lock().lock().unwrap(); + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let repo = init_repo(); + let root = repo.path(); + // Seed file and commit original content + std::fs::write(root.join("file.txt"), "orig\n").unwrap(); + let _ = run(root, &["git", "add", "file.txt"]); + let _ = run(root, &["git", "commit", "-m", "seed"]); + + // Forward patch: orig -> ORIG + let diff = "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1,1 +1,1 @@\n-orig\n+ORIG\n"; + let apply_req = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: diff.to_string(), + revert: false, + }; + let res_apply = apply_git_patch(&apply_req).expect("apply ok"); + assert_eq!(res_apply.exit_code, 0, "forward apply succeeded"); + let after_apply = std::fs::read_to_string(root.join("file.txt")).unwrap(); + assert_eq!(after_apply, "ORIG\n"); + + // Revert patch: ORIG -> orig (stage paths first; engine handles it) + let revert_req = ApplyGitRequest { + cwd: root.to_path_buf(), + diff: diff.to_string(), + revert: true, + }; + let res_revert = apply_git_patch(&revert_req).expect("revert ok"); + assert_eq!(res_revert.exit_code, 0, "revert apply succeeded"); + let after_revert = std::fs::read_to_string(root.join("file.txt")).unwrap(); + assert_eq!(after_revert, "orig\n"); + } + + #[test] + fn preflight_blocks_partial_changes() { + let _g = env_lock().lock().unwrap(); + let repo = init_repo(); + let root = repo.path().to_path_buf(); + // Build a multi-file diff: one valid add (ok.txt) and one invalid modify (ghost.txt) + let diff = "diff --git a/ok.txt b/ok.txt\nnew file mode 100644\n--- /dev/null\n+++ b/ok.txt\n@@ -0,0 +1,2 @@\n+alpha\n+beta\n\n\ +diff --git a/ghost.txt b/ghost.txt\n--- a/ghost.txt\n+++ b/ghost.txt\n@@ -1,1 +1,1 @@\n-old\n+new\n"; + + // 1) With preflight enabled, nothing should be changed (even though ok.txt could be added) + unsafe { std::env::set_var("CODEX_APPLY_PREFLIGHT", "1") }; + let req1 = ApplyGitRequest { + cwd: root.clone(), + diff: diff.to_string(), + revert: false, + }; + let r1 = apply_git_patch(&req1).expect("preflight apply"); + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + assert_ne!(r1.exit_code, 0, "preflight reports failure"); + assert!( + !root.join("ok.txt").exists(), + "preflight must prevent adding ok.txt" + ); + assert!( + r1.cmd_for_log.contains("--check"), + "preflight path recorded --check" + ); + + // 2) Without preflight, we should see no --check in the executed command + let req2 = ApplyGitRequest { + cwd: root.clone(), + diff: diff.to_string(), + revert: false, + }; + let r2 = apply_git_patch(&req2).expect("direct apply"); + assert_ne!(r2.exit_code, 0, "apply is expected to fail overall"); + assert!( + !r2.cmd_for_log.contains("--check"), + "non-preflight path should not use --check" + ); + } +} diff --git a/codex-rs/cloud-tasks-client/src/http.rs b/codex-rs/cloud-tasks-client/src/http.rs index 5b2587ccda..7fdcb456ef 100644 --- a/codex-rs/cloud-tasks-client/src/http.rs +++ b/codex-rs/cloud-tasks-client/src/http.rs @@ -15,7 +15,6 @@ use std::collections::HashMap; use codex_backend_client as backend; use codex_backend_client::CodeTaskDetailsResponseExt; -use codex_backend_client::types::extract_file_paths_list; #[derive(Clone)] pub struct HttpClient { @@ -46,6 +45,24 @@ impl HttpClient { } } +fn is_unified_diff(diff: &str) -> bool { + let t = diff.trim_start(); + if t.starts_with("diff --git ") { + return true; + } + let has_dash_headers = diff.contains("\n--- ") && diff.contains("\n+++ "); + let has_hunk = diff.contains("\n@@ ") || diff.starts_with("@@ "); + has_dash_headers && has_hunk +} + +fn tail(s: &str, max: usize) -> String { + if s.len() <= max { + s.to_string() + } else { + s[s.len() - max..].to_string() + } +} + #[async_trait::async_trait] impl CloudBackend for HttpClient { async fn list_tasks(&self, env: Option<&str>) -> Result> { @@ -125,15 +142,12 @@ impl CloudBackend for HttpClient { } continue; } - if let Some(obj) = p.as_object() { - if obj.get("content_type").and_then(|t| t.as_str()) + if let Some(obj) = p.as_object() + && obj.get("content_type").and_then(|t| t.as_str()) == Some("text") - { - if let Some(txt) = obj.get("text").and_then(|t| t.as_str()) - { - msgs.push(txt.to_string()); - } - } + && let Some(txt) = obj.get("text").and_then(|t| t.as_str()) + { + msgs.push(txt.to_string()); } } } @@ -169,101 +183,111 @@ impl CloudBackend for HttpClient { let diff = details .unified_diff() .ok_or_else(|| Error::Msg(format!("No diff available for task {id}")))?; - let diff = match crate::patch_apply::classify_patch(&diff) { - crate::patch_apply::PatchKind::HunkOnly => { - let files = extract_file_paths_list(&details); - if files.len() > 1 { - let parts = crate::patch_apply::split_hunk_body_into_files(&diff); - if parts.len() == files.len() { - let mut acc = String::new(); - for (i, (oldp, newp)) in files.iter().enumerate() { - let u = crate::patch_apply::synthesize_unified_single_file( - &parts[i], oldp, newp, - ); - acc.push_str(&u); - if !acc.ends_with("\n") { - acc.push('\n'); - } - } - acc - } else if let Some((oldp, newp)) = details.single_file_paths() { - crate::patch_apply::synthesize_unified_single_file(&diff, &oldp, &newp) - } else { - diff - } - } else if let Some((oldp, newp)) = details.single_file_paths() { - crate::patch_apply::synthesize_unified_single_file(&diff, &oldp, &newp) - } else { - diff - } - } - _ => diff, - }; + // Enforce unified diff format only + if !is_unified_diff(&diff) { + let summary = summarize_patch_for_logging(&diff); + append_error_log(&format!( + "apply_error: id={id} format=non-unified; {summary}" + )); + return Ok(ApplyOutcome { + applied: false, + status: ApplyStatus::Error, + message: "Expected unified git diff; backend returned an incompatible format." + .to_string(), + skipped_paths: Vec::new(), + conflict_paths: Vec::new(), + }); + } - // Run the centralized Git apply path (supports unified diffs and Codex conversion) - let ctx = crate::patch_apply::context_from_env( - std::env::current_dir().unwrap_or_else(|_| std::env::temp_dir()), - ); - let res = crate::patch_apply::apply_patch(&diff, &ctx); - let status = match res.status { - crate::patch_apply::ApplyStatus::Success => ApplyStatus::Success, - crate::patch_apply::ApplyStatus::Partial => ApplyStatus::Partial, - crate::patch_apply::ApplyStatus::Error => ApplyStatus::Error, + // Run the new Git apply engine + let req = crate::git_apply::ApplyGitRequest { + cwd: std::env::current_dir().unwrap_or_else(|_| std::env::temp_dir()), + diff: diff.clone(), + revert: false, }; - let applied = matches!(status, ApplyStatus::Success); - let message = match status { - ApplyStatus::Success => format!( - "Applied task {id} locally ({} changed)", - res.changed_paths.len() - ), - ApplyStatus::Partial => format!( - "Apply partially succeeded for task {id} (changed={}, skipped={}, conflicts={})", - res.changed_paths.len(), - res.skipped_paths.len(), - res.conflict_paths.len() - ), - ApplyStatus::Error => { - let is_check = res.diagnostics.contains("apply --check failed"); - if is_check { + let r = crate::git_apply::apply_git_patch(&req) + .map_err(|e| Error::Io(format!("git apply failed to run: {e}")))?; + + let status = if r.exit_code == 0 { + ApplyStatus::Success + } else if !r.applied_paths.is_empty() || !r.conflicted_paths.is_empty() { + ApplyStatus::Partial + } else { + ApplyStatus::Error + }; + let is_preflight = r.cmd_for_log.contains("--check"); + let applied = matches!(status, ApplyStatus::Success) && !is_preflight; + let message = if is_preflight { + match status { + ApplyStatus::Success => format!("Preflight passed for task {id} (applies cleanly)"), + ApplyStatus::Partial => format!( + "Preflight: patch does not fully apply for task {id} (applied={}, skipped={}, conflicts={})", + r.applied_paths.len(), + r.skipped_paths.len(), + r.conflicted_paths.len() + ), + ApplyStatus::Error => format!( + "Preflight failed for task {id} (applied={}, skipped={}, conflicts={})", + r.applied_paths.len(), + r.skipped_paths.len(), + r.conflicted_paths.len() + ), + } + } else { + match status { + ApplyStatus::Success => { format!( - "Apply check failed for task {id}: patch does not apply to your working tree. No changes were made. See error.log for details.", + "Applied task {id} locally ({} files)", + r.applied_paths.len() ) - } else { - // Compact, single-line fallback; avoid embedding multiline stderr directly. - let mut diag = res.diagnostics.replace('\n', " "); - if diag.len() > 600 { - diag.truncate(600); - diag.push_str("…"); - } + } + ApplyStatus::Partial => { format!( - "Apply failed for task {id} (changed={}, skipped={}, conflicts={}); {}", - res.changed_paths.len(), - res.skipped_paths.len(), - res.conflict_paths.len(), - diag + "Apply partially succeeded for task {id} (applied={}, skipped={}, conflicts={})", + r.applied_paths.len(), + r.skipped_paths.len(), + r.conflicted_paths.len() + ) + } + ApplyStatus::Error => { + format!( + "Apply failed for task {id} (applied={}, skipped={}, conflicts={})", + r.applied_paths.len(), + r.skipped_paths.len(), + r.conflicted_paths.len() ) } } }; - // On apply failure, log a detailed record including the diff we attempted. - if matches!(status, ApplyStatus::Error) { + // Log details on partial and error + if matches!(status, ApplyStatus::Partial | ApplyStatus::Error) + || (is_preflight && !matches!(status, ApplyStatus::Success)) + { let mut log = String::new(); let summary = summarize_patch_for_logging(&diff); use std::fmt::Write as _; let _ = writeln!( &mut log, - "apply_error: id={} changed={} skipped={} conflicts={}; {}", + "apply_result: id={} status={:?} applied={} skipped={} conflicts={} cmd={}", id, - res.changed_paths.len(), - res.skipped_paths.len(), - res.conflict_paths.len(), - res.diagnostics + status, + r.applied_paths.len(), + r.skipped_paths.len(), + r.conflicted_paths.len(), + r.cmd_for_log + ); + let _ = writeln!( + &mut log, + "stdout_tail=\n{}\nstderr_tail=\n{}", + tail(&r.stdout, 2000), + tail(&r.stderr, 2000) ); let _ = writeln!(&mut log, "{summary}"); - let _ = writeln!(&mut log, "----- PATCH BEGIN -----"); - let _ = writeln!(&mut log, "{diff}"); - let _ = writeln!(&mut log, "----- PATCH END -----"); + let _ = writeln!( + &mut log, + "----- PATCH BEGIN -----\n{diff}\n----- PATCH END -----" + ); append_error_log(&log); } @@ -271,8 +295,8 @@ impl CloudBackend for HttpClient { applied, status, message, - skipped_paths: res.skipped_paths, - conflict_paths: res.conflict_paths, + skipped_paths: r.skipped_paths, + conflict_paths: r.conflicted_paths, }) } @@ -291,13 +315,13 @@ impl CloudBackend for HttpClient { "content": [{ "content_type": "text", "text": prompt }] })); - if let Ok(diff) = std::env::var("CODEX_STARTING_DIFF") { - if !diff.is_empty() { - input_items.push(serde_json::json!({ - "type": "pre_apply_patch", - "output_diff": { "diff": diff } - })); - } + if let Ok(diff) = std::env::var("CODEX_STARTING_DIFF") + && !diff.is_empty() + { + input_items.push(serde_json::json!({ + "type": "pre_apply_patch", + "output_diff": { "diff": diff } + })); } let request_body = serde_json::json!({ @@ -344,21 +368,21 @@ fn map_task_list_item_to_summary(src: backend::TaskListItem) -> TaskSummary { } if let Some(o) = raw.as_object() { // Best-effort support for rich shapes: { text: "..." } or { plain_text: "..." } - if let Some(s) = o.get("text").and_then(Value::as_str) { - if !s.trim().is_empty() { - return Some(s.to_string()); - } + if let Some(s) = o.get("text").and_then(Value::as_str) + && !s.trim().is_empty() + { + return Some(s.to_string()); } - if let Some(s) = o.get("plain_text").and_then(Value::as_str) { - if !s.trim().is_empty() { - return Some(s.to_string()); - } + if let Some(s) = o.get("plain_text").and_then(Value::as_str) + && !s.trim().is_empty() + { + return Some(s.to_string()); } // Fallback: compact JSON for debugging - if let Ok(s) = serde_json::to_string(o) { - if !s.is_empty() { - return Some(s); - } + if let Ok(s) = serde_json::to_string(o) + && !s.is_empty() + { + return Some(s); } } None @@ -403,17 +427,16 @@ fn map_status(v: Option<&HashMap>) -> TaskStatus { if let Some(turn) = val .get("latest_turn_status_display") .and_then(Value::as_object) + && let Some(s) = turn.get("turn_status").and_then(Value::as_str) { - if let Some(s) = turn.get("turn_status").and_then(Value::as_str) { - return match s { - "failed" => TaskStatus::Error, - "completed" => TaskStatus::Ready, - "in_progress" => TaskStatus::Pending, - "pending" => TaskStatus::Pending, - "cancelled" => TaskStatus::Error, - _ => TaskStatus::Pending, - }; - } + return match s { + "failed" => TaskStatus::Error, + "completed" => TaskStatus::Ready, + "in_progress" => TaskStatus::Pending, + "pending" => TaskStatus::Pending, + "cancelled" => TaskStatus::Error, + _ => TaskStatus::Pending, + }; } // Legacy or alternative flat state. if let Some(state) = val.get("state").and_then(Value::as_str) { diff --git a/codex-rs/cloud-tasks-client/src/lib.rs b/codex-rs/cloud-tasks-client/src/lib.rs index 5ca6533715..ef8e0f9759 100644 --- a/codex-rs/cloud-tasks-client/src/lib.rs +++ b/codex-rs/cloud-tasks-client/src/lib.rs @@ -23,4 +23,5 @@ pub use mock::MockClient; pub use http::HttpClient; // Reusable apply engine (git apply runner and helpers) -pub mod patch_apply; +// Legacy engine remains until migration completes. New engine lives in git_apply. +mod git_apply; diff --git a/codex-rs/cloud-tasks-client/src/mock.rs b/codex-rs/cloud-tasks-client/src/mock.rs index c3654622b1..cfa29021f9 100644 --- a/codex-rs/cloud-tasks-client/src/mock.rs +++ b/codex-rs/cloud-tasks-client/src/mock.rs @@ -1,6 +1,5 @@ use crate::ApplyOutcome; use crate::CloudBackend; -use crate::Error; use crate::Result; use crate::TaskId; use crate::TaskStatus; @@ -31,7 +30,7 @@ impl CloudBackend for MockClient { let environment_label = match _env { Some("env-A") => Some("Env A".to_string()), Some("env-B") => Some("Env B".to_string()), - Some(other) => Some(format!("{other}")), + Some(other) => Some(other.to_string()), None => Some("Global".to_string()), }; let mut out = Vec::new(); diff --git a/codex-rs/cloud-tasks-client/src/patch_apply.rs b/codex-rs/cloud-tasks-client/src/patch_apply.rs deleted file mode 100644 index 2706133975..0000000000 --- a/codex-rs/cloud-tasks-client/src/patch_apply.rs +++ /dev/null @@ -1,607 +0,0 @@ -#![allow(dead_code)] - -use std::env; -use std::path::Path; -use std::path::PathBuf; - -/// Patch classification used to choose normalization steps before applying. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum PatchKind { - /// Codex Patch format beginning with `*** Begin Patch`. - CodexPatch, - /// Unified diff that includes either `diff --git` headers or just `---/+++` file headers. - GitUnified, - /// Body contains `@@` hunks but lacks required file headers. - HunkOnly, - /// Unknown/unsupported format. - Unknown, -} - -/// How to handle whitespace in `git apply`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum WhitespaceMode { - /// Default strict behavior. - Strict, - /// Equivalent to `--ignore-space-change`. - IgnoreSpaceChange, - /// Equivalent to `--whitespace=nowarn`. - WhitespaceNowarn, -} - -/// How to treat CRLF conversions in `git`. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum CrlfMode { - /// Use repo/user defaults. - Default, - /// Apply with `-c core.autocrlf=false -c core.safecrlf=false`. - NoAutoCrlfNoSafe, -} - -/// Context for an apply operation. -#[derive(Debug, Clone)] -pub struct ApplyContext { - pub cwd: PathBuf, - pub whitespace: WhitespaceMode, - pub crlf_mode: CrlfMode, -} - -/// High-level outcome of an apply attempt. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ApplyStatus { - Success, - Partial, - Error, -} - -/// Structured result produced by the apply runner. -#[derive(Debug, Clone)] -pub struct ApplyResult { - pub status: ApplyStatus, - pub changed_paths: Vec, - pub skipped_paths: Vec, - pub conflict_paths: Vec, - pub stdout_tail: String, - pub stderr_tail: String, - pub diagnostics: String, -} - -/// Classify an incoming patch string by format. -pub fn classify_patch(s: &str) -> PatchKind { - let t = s.trim_start(); - if t.starts_with("*** Begin Patch") { - return PatchKind::CodexPatch; - } - // Unified diffs can be either full git style or just `---`/`+++` file headers. - let has_diff_git = t.contains("\ndiff --git ") || t.starts_with("diff --git "); - let has_dash_headers = t.contains("\n--- ") && t.contains("\n+++ "); - let has_hunk = t.contains("\n@@ ") || t.starts_with("@@ "); - if has_diff_git || (has_dash_headers && has_hunk) { - return PatchKind::GitUnified; - } - if has_hunk { - return PatchKind::HunkOnly; - } - PatchKind::Unknown -} - -/// Build an `ApplyContext` from environment variables. -/// -/// Supported envs: -/// - `CODEX_APPLY_WHITESPACE` = `ignore-space-change` | `whitespace-nowarn` | `strict` (default) -/// - `CODEX_APPLY_CRLF` = `no-autocrlf-nosafe` | `default` (default) -pub fn context_from_env(cwd: PathBuf) -> ApplyContext { - let whitespace = match env::var("CODEX_APPLY_WHITESPACE").ok().as_deref() { - Some("ignore-space-change") => WhitespaceMode::IgnoreSpaceChange, - Some("whitespace-nowarn") => WhitespaceMode::WhitespaceNowarn, - _ => WhitespaceMode::Strict, - }; - let crlf_mode = match env::var("CODEX_APPLY_CRLF").ok().as_deref() { - Some("no-autocrlf-nosafe") => CrlfMode::NoAutoCrlfNoSafe, - _ => CrlfMode::Default, - }; - ApplyContext { - cwd, - whitespace, - crlf_mode, - } -} - -/// Main entry point for applying a patch. This will be implemented in subsequent steps. -pub fn apply_patch(patch: &str, ctx: &ApplyContext) -> ApplyResult { - // Classify and convert if needed - let kind = classify_patch(patch); - let unified = match kind { - PatchKind::GitUnified => patch.to_string(), - PatchKind::CodexPatch => match convert_codex_patch_to_unified(patch, &ctx.cwd) { - Ok(u) => u, - Err(e) => { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!("failed to convert codex patch to unified diff: {e}"), - }; - } - }, - PatchKind::HunkOnly | PatchKind::Unknown => { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!( - "unsupported patch format: {kind:?}; need unified diff with file headers" - ), - }; - } - }; - - apply_unified(&unified, ctx) -} - -fn apply_unified(unified_patch: &str, ctx: &ApplyContext) -> ApplyResult { - // 1) Ensure `git` exists - if let Err(e) = run_git(&ctx.cwd, &[], &["--version"]) { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!("git not available: {e}"), - }; - } - // 2) Determine repo root - let repo_root = match run_git_capture(&ctx.cwd, &[], &["rev-parse", "--show-toplevel"]) { - Ok(out) if out.status == 0 => out.stdout.trim().to_string(), - Ok(out) => { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!( - "not a git repository (exit {}): {}", - out.status, - tail(&out.stderr) - ), - }; - } - Err(e) => { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!("git rev-parse failed: {e}"), - }; - } - }; - - // 3) Temp file - let mut patch_path = std::env::temp_dir(); - patch_path.push(format!("codex-apply-{}.diff", std::process::id())); - if let Err(e) = std::fs::write(&patch_path, unified_patch) { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!("failed to write temp patch: {e}"), - }; - } - struct TempPatch(PathBuf); - impl Drop for TempPatch { - fn drop(&mut self) { - let _ = std::fs::remove_file(&self.0); - } - } - let _guard = TempPatch(patch_path.clone()); - - // 4) Preflight --check - let mut preflight_args: Vec<&str> = vec!["apply", "--check"]; - push_whitespace_flags(&mut preflight_args, ctx.whitespace); - // Compute a shell-friendly representation of the preflight command for logging. - let preflight_cfg = crlf_cfg(ctx.crlf_mode); - let preflight_cmd = render_command_for_log( - &repo_root, - &preflight_cfg, - &prepend(&preflight_args, patch_path.to_string_lossy().as_ref()), - ); - let preflight = run_git_capture( - Path::new(&repo_root), - preflight_cfg.as_slice(), - &prepend(&preflight_args, patch_path.to_string_lossy().as_ref()), - ); - if let Ok(out) = &preflight { - if out.status != 0 { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: tail(&out.stdout), - stderr_tail: tail(&out.stderr), - diagnostics: format!( - "git apply --check failed; working tree not modified; cmd: {preflight_cmd}" - ), - }; - } - } else if let Err(e) = preflight { - return ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: format!("git apply --check failed to run: {e}; cmd: {preflight_cmd}"), - }; - } - - // 5) Snapshot before - let before = list_changed_paths(&repo_root); - // 6) Apply - let mut apply_args: Vec<&str> = vec!["apply", "--3way"]; - push_whitespace_flags(&mut apply_args, ctx.whitespace); - let apply_cfg = crlf_cfg(ctx.crlf_mode); - let apply_cmd = render_command_for_log( - &repo_root, - &apply_cfg, - &prepend(&apply_args, patch_path.to_string_lossy().as_ref()), - ); - let apply_out = run_git_capture( - Path::new(&repo_root), - apply_cfg.as_slice(), - &prepend(&apply_args, patch_path.to_string_lossy().as_ref()), - ); - let mut result = ApplyResult { - status: ApplyStatus::Error, - changed_paths: Vec::new(), - skipped_paths: Vec::new(), - conflict_paths: Vec::new(), - stdout_tail: String::new(), - stderr_tail: String::new(), - diagnostics: String::new(), - }; - match apply_out { - Ok(out) => { - result.stdout_tail = tail(&out.stdout); - result.stderr_tail = tail(&out.stderr); - result.conflict_paths = list_conflicts(&repo_root); - let mut skipped = parse_skipped_paths(&result.stdout_tail); - skipped.extend(parse_skipped_paths(&result.stderr_tail)); - skipped.sort(); - skipped.dedup(); - result.skipped_paths = skipped; - let after = list_changed_paths(&repo_root); - result.changed_paths = set_delta(&before, &after); - result.status = if out.status == 0 { - ApplyStatus::Success - } else if !result.changed_paths.is_empty() || !result.conflict_paths.is_empty() { - ApplyStatus::Partial - } else { - ApplyStatus::Error - }; - result.diagnostics = format!( - "git apply exit={} ({} changed, {} skipped, {} conflicts); cmd: {}", - out.status, - result.changed_paths.len(), - result.skipped_paths.len(), - result.conflict_paths.len(), - apply_cmd - ); - } - Err(e) => { - result.status = ApplyStatus::Error; - result.diagnostics = format!("failed to run git apply: {e}; cmd: {apply_cmd}"); - } - } - result -} - -fn render_command_for_log(cwd: &str, git_cfg: &[&str], args: &[&str]) -> String { - fn quote(s: &str) -> String { - let simple = s - .chars() - .all(|c| c.is_ascii_alphanumeric() || "-_.:/@%+".contains(c)); - if simple { - s.to_string() - } else { - format!("'{}'", s.replace('\'', "'\\''")) - } - } - let mut parts: Vec = Vec::new(); - parts.push("git".to_string()); - for a in git_cfg { - parts.push(quote(a)); - } - for a in args { - parts.push(quote(a)); - } - format!("(cd {} && {})", quote(cwd), parts.join(" ")) -} - -fn convert_codex_patch_to_unified(patch: &str, cwd: &Path) -> Result { - // Parse codex patch and verify paths relative to cwd - let argv = vec!["apply_patch".to_string(), patch.to_string()]; - let verified = codex_apply_patch::maybe_parse_apply_patch_verified(&argv, cwd); - match verified { - codex_apply_patch::MaybeApplyPatchVerified::Body(action) => { - let mut parts: Vec = Vec::new(); - for (abs_path, change) in action.changes() { - let rel_path = abs_path.strip_prefix(cwd).unwrap_or(abs_path); - let rel_str = rel_path.to_string_lossy(); - match change { - codex_apply_patch::ApplyPatchFileChange::Add { content } => { - let header = format!( - "diff --git a/{rel_str} b/{rel_str} -new file mode 100644 ---- /dev/null -+++ b/{rel_str} -" - ); - let body = build_add_hunk(content); - parts.push(format!("{header}{body}")); - } - codex_apply_patch::ApplyPatchFileChange::Delete { .. } => { - let header = format!( - "diff --git a/{rel_str} b/{rel_str} -deleted file mode 100644 ---- a/{rel_str} -+++ /dev/null -" - ); - parts.push(header); - } - codex_apply_patch::ApplyPatchFileChange::Update { - unified_diff, - move_path, - .. - } => { - let new_rel = move_path - .as_ref() - .map(|p| { - p.strip_prefix(cwd) - .unwrap_or(p) - .to_string_lossy() - .to_string() - }) - .unwrap_or_else(|| rel_str.to_string()); - let header = format!( - "diff --git a/{rel_str} b/{new_rel} ---- a/{rel_str} -+++ b/{new_rel} -" - ); - parts.push(format!("{header}{unified_diff}")); - } - } - } - if parts.is_empty() { - Err("empty patch after conversion".to_string()) - } else { - Ok(parts.join("\n")) - } - } - codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(e) => { - Err(format!("patch correctness: {e}")) - } - codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(e) => { - Err(format!("shell parse: {e:?}")) - } - _ => Err("not an apply_patch payload".to_string()), - } -} - -fn build_add_hunk(content: &str) -> String { - let norm = content.replace("\r\n", "\n"); - let mut lines: Vec<&str> = norm.split('\n').collect(); - if let Some("") = lines.last().copied() { - lines.pop(); - } - let count = lines.len(); - if count == 0 { - return String::new(); - } - let mut out = String::new(); - out.push_str(&format!("@@ -0,0 +1,{count} @@\n")); - for l in lines { - out.push('+'); - out.push_str(l); - out.push('\n'); - } - out -} - -fn push_whitespace_flags(args: &mut Vec<&str>, mode: WhitespaceMode) { - match mode { - WhitespaceMode::Strict => {} - WhitespaceMode::IgnoreSpaceChange => args.push("--ignore-space-change"), - WhitespaceMode::WhitespaceNowarn => { - args.push("--whitespace"); - args.push("nowarn"); - } - } -} - -fn crlf_cfg(mode: CrlfMode) -> Vec<&'static str> { - match mode { - CrlfMode::Default => vec![], - CrlfMode::NoAutoCrlfNoSafe => { - vec!["-c", "core.autocrlf=false", "-c", "core.safecrlf=false"] - } - } -} - -fn prepend<'a>(base: &'a [&'a str], tail: &'a str) -> Vec<&'a str> { - let mut v = base.to_vec(); - v.push(tail); - v -} - -struct GitOutput { - status: i32, - stdout: String, - stderr: String, -} - -fn run_git(cwd: &std::path::Path, git_cfg: &[&str], args: &[&str]) -> std::io::Result<()> { - let status = std::process::Command::new("git") - .args(git_cfg) - .args(args) - .current_dir(cwd) - .status()?; - if status.success() { - Ok(()) - } else { - Err(std::io::Error::other(format!( - "git {:?} exited {}", - args, - status.code().unwrap_or(-1) - ))) - } -} - -fn run_git_capture( - cwd: &std::path::Path, - git_cfg: &[&str], - args: &[&str], -) -> std::io::Result { - let out = std::process::Command::new("git") - .args(git_cfg) - .args(args) - .current_dir(cwd) - .output()?; - Ok(GitOutput { - status: out.status.code().unwrap_or(-1), - stdout: String::from_utf8_lossy(&out.stdout).into_owned(), - stderr: String::from_utf8_lossy(&out.stderr).into_owned(), - }) -} - -fn list_changed_paths(repo_root: &str) -> Vec { - let cwd = std::path::Path::new(repo_root); - match run_git_capture(cwd, &[], &["diff", "--name-only"]) { - Ok(out) if out.status == 0 => out - .stdout - .lines() - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .collect(), - _ => Vec::new(), - } -} - -fn list_conflicts(repo_root: &str) -> Vec { - let cwd = std::path::Path::new(repo_root); - match run_git_capture(cwd, &[], &["ls-files", "-u"]) { - Ok(out) if out.status == 0 => { - let mut set = std::collections::BTreeSet::new(); - for line in out.stdout.lines() { - // format: \t - if let Some((_meta, path)) = line.split_once('\t') { - set.insert(path.trim().to_string()); - } - } - set.into_iter().collect() - } - _ => Vec::new(), - } -} - -fn parse_skipped_paths(text: &str) -> Vec { - let mut out = Vec::new(); - for line in text.lines() { - let l = line.trim(); - // error: path/to/file.txt does not match index - if let Some(rest) = l.strip_prefix("error:") { - let rest = rest.trim(); - if let Some(p) = rest.strip_suffix("does not match index") { - let p = p.trim().trim_end_matches(':').trim(); - if !p.is_empty() { - out.push(p.to_string()); - } - continue; - } - } - // patch failed: path/to/file.txt: content - if let Some(rest) = l.strip_prefix("patch failed:") { - let rest = rest.trim(); - if let Some((p, _)) = rest.split_once(':') { - let p = p.trim(); - if !p.is_empty() { - out.push(p.to_string()); - } - } - } - } - out -} - -fn tail(s: &str) -> String { - const MAX: usize = 2000; - if s.len() <= MAX { - s.to_string() - } else { - s[s.len() - MAX..].to_string() - } -} - -fn set_delta(before: &[String], after: &[String]) -> Vec { - use std::collections::BTreeSet; - let b: BTreeSet<_> = before.iter().collect(); - let a: BTreeSet<_> = after.iter().collect(); - a.difference(&b).map(|s| (*s).clone()).collect() -} - -/// Synthesize a unified git diff for a single file from a bare hunk body. -pub fn synthesize_unified_single_file(hunk_body: &str, old_path: &str, new_path: &str) -> String { - // Ensure body ends with newline - let mut body = hunk_body.to_string(); - if !body.ends_with("\n") { - body.push('\n'); - } - format!( - "diff --git a/{old_path} b/{new_path} ---- a/{old_path} -+++ b/{new_path} -{body}" - ) -} - -/// Split a bare hunk body into per-file segments using a conservative delimiter. -/// We look for lines that equal "*** End of File" (as emitted by our apply-patch format) -/// and use that to separate bodies for multiple files. -pub fn split_hunk_body_into_files(body: &str) -> Vec { - let mut chunks: Vec = Vec::new(); - let mut cur = String::new(); - for line in body.lines() { - if line.trim() == "*** End of File" { - if !cur.is_empty() { - cur.push('\n'); - chunks.push(cur); - cur = String::new(); - } - } else { - cur.push_str(line); - cur.push('\n'); - } - } - if !cur.trim().is_empty() { - chunks.push(cur); - } - chunks -} diff --git a/codex-rs/cloud-tasks/src/app.rs b/codex-rs/cloud-tasks/src/app.rs index 779cf7de66..f79f65d2c0 100644 --- a/codex-rs/cloud-tasks/src/app.rs +++ b/codex-rs/cloud-tasks/src/app.rs @@ -15,6 +15,23 @@ pub struct EnvModalState { pub selected: usize, } +#[derive(Clone, Debug, Copy, PartialEq, Eq)] +pub enum ApplyResultLevel { + Success, + Partial, + Error, +} + +#[derive(Clone, Debug)] +pub struct ApplyModalState { + pub task_id: TaskId, + pub title: String, + pub result_message: Option, + pub result_level: Option, + pub skipped_paths: Vec, + pub conflict_paths: Vec, +} + use crate::scrollable_diff::ScrollableDiff; use codex_cloud_tasks_api::CloudBackend; use codex_cloud_tasks_api::DiffSummary; @@ -30,19 +47,21 @@ pub struct App { pub selected: usize, pub status: String, pub diff_overlay: Option, - pub pending_apply: Option<(TaskId, String)>, pub throbber: ThrobberState, pub refresh_inflight: bool, pub details_inflight: bool, // Environment filter state pub env_filter: Option, pub env_modal: Option, + pub apply_modal: Option, pub environments: Vec, pub env_last_loaded: Option, pub env_loading: bool, pub env_error: Option, // New Task page pub new_task: Option, + // Apply preflight spinner state + pub apply_preflight_inflight: bool, // Background enrichment coordination pub list_generation: u64, pub in_flight: std::collections::HashSet, @@ -57,17 +76,18 @@ impl App { selected: 0, status: "Press r to refresh".to_string(), diff_overlay: None, - pending_apply: None, throbber: ThrobberState::default(), refresh_inflight: false, details_inflight: false, env_filter: None, env_modal: None, + apply_modal: None, environments: Vec::new(), env_last_loaded: None, env_loading: false, env_error: None, new_task: None, + apply_preflight_inflight: false, list_generation: 0, in_flight: std::collections::HashSet::new(), summary_cache: std::collections::HashMap::new(), @@ -145,6 +165,15 @@ pub enum AppEvent { }, /// Background completion of new task submission NewTaskSubmitted(Result), + /// Background completion of apply preflight when opening modal or on demand + ApplyPreflightFinished { + id: TaskId, + title: String, + message: String, + level: ApplyResultLevel, + skipped: Vec, + conflicts: Vec, + }, } pub type AppEventTx = UnboundedSender; diff --git a/codex-rs/cloud-tasks/src/lib.rs b/codex-rs/cloud-tasks/src/lib.rs index 70ed64215b..e3b6af22a9 100644 --- a/codex-rs/cloud-tasks/src/lib.rs +++ b/codex-rs/cloud-tasks/src/lib.rs @@ -362,10 +362,44 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a }); } - // Simple draw loop driven by input; keep a 250ms tick for spinner animation. + // Event-driven redraws with a tiny coalescing scheduler (snappy UI, no fixed 250ms tick). let mut needs_redraw = true; - let tick = tokio::time::interval(Duration::from_millis(250)); - tokio::pin!(tick); + use std::time::Instant; + use tokio::time::Instant as TokioInstant; + use tokio::time::sleep_until; + let (frame_tx, mut frame_rx) = tokio::sync::mpsc::unbounded_channel::(); + let (redraw_tx, mut redraw_rx) = tokio::sync::mpsc::unbounded_channel::<()>(); + + // Coalesce frame requests to the earliest deadline; emit a single redraw signal. + tokio::spawn(async move { + let mut next_deadline: Option = None; + loop { + let target = + next_deadline.unwrap_or_else(|| Instant::now() + Duration::from_secs(24 * 60 * 60)); + let sleeper = sleep_until(TokioInstant::from_std(target)); + tokio::pin!(sleeper); + tokio::select! { + recv = frame_rx.recv() => { + match recv { + Some(at) => { + if next_deadline.map_or(true, |cur| at < cur) { + next_deadline = Some(at); + } + continue; // recompute sleep target + } + None => break, + } + } + _ = &mut sleeper => { + if next_deadline.take().is_some() { + let _ = redraw_tx.send(()); + } + } + } + } + }); + // Kick an initial draw so the UI appears immediately. + let _ = frame_tx.send(Instant::now()); // Render helper to centralize immediate redraws after handling events. let mut render_if_needed = |terminal: &mut Terminal>, @@ -381,10 +415,21 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a let exit_code = loop { tokio::select! { - _ = tick.tick() => { - // When a network request is inflight, keep the spinner moving - if app.refresh_inflight || app.details_inflight || app.env_loading { app.throbber.calc_next(); needs_redraw = true; } - // Tick is for animation; still draw here if something changed. + // Coalesced redraw requests: spinner animation and paste-burst micro‑flush. + Some(()) = redraw_rx.recv() => { + // Micro‑flush pending first key held by paste‑burst. + if let Some(page) = app.new_task.as_mut() { + if page.composer.flush_paste_burst_if_due() { needs_redraw = true; } + if page.composer.is_in_paste_burst() { + let _ = frame_tx.send(Instant::now() + codex_tui::ComposerInput::recommended_flush_delay()); + } + } + // Advance throbber only while loading. + if app.refresh_inflight || app.details_inflight || app.env_loading || app.apply_preflight_inflight { + app.throbber.calc_next(); + needs_redraw = true; + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); + } render_if_needed(&mut terminal, &mut app, &mut needs_redraw)?; } maybe_app_event = rx.recv() => { @@ -418,6 +463,7 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a } } needs_redraw = true; + let _ = frame_tx.send(Instant::now()); } app::AppEvent::NewTaskSubmitted(result) => { match result { @@ -437,12 +483,14 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a let res = app::load_tasks(&*backend2, env_sel.as_deref()).await; let _ = tx2.send(app::AppEvent::TasksLoaded { env: env_sel, result: res }); }); + let _ = frame_tx.send(Instant::now()); } Err(msg) => { append_error_log(format!("new-task: submit failed: {}", msg)); if let Some(page) = app.new_task.as_mut() { page.submitting = false; } app.status = format!("Submit failed: {}. See error.log for details.", msg); needs_redraw = true; + let _ = frame_tx.send(Instant::now()); } } } @@ -456,6 +504,22 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a app.summary_cache.insert(id_str.clone(), (summary, std::time::Instant::now())); if no_diff_yet { app.no_diff_yet.insert(id_str); } else { app.no_diff_yet.remove(&id.0); } needs_redraw = true; + let _ = frame_tx.send(Instant::now()); + } + app::AppEvent::ApplyPreflightFinished { id, title, message, level, skipped, conflicts } => { + // Only update if modal is still open and ids match + if let Some(m) = app.apply_modal.as_mut() { + if m.task_id == id { + m.title = title; + m.result_message = Some(message); + m.result_level = Some(level); + m.skipped_paths = skipped; + m.conflict_paths = conflicts; + app.apply_preflight_inflight = false; + needs_redraw = true; + let _ = frame_tx.send(Instant::now()); + } + } } app::AppEvent::EnvironmentsLoaded(result) => { app.env_loading = false; @@ -470,6 +534,7 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a } } needs_redraw = true; + let _ = frame_tx.send(Instant::now()); } app::AppEvent::EnvironmentAutodetected(result) => { if let Ok(sel) = result { @@ -535,6 +600,7 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a let res = crate::env_detect::list_environments(&base_url, &headers).await; let _ = tx3.send(app::AppEvent::EnvironmentsLoaded(res.map_err(|e| e.into()))); }); + let _ = frame_tx.send(Instant::now()); } } // on Err, silently continue with All @@ -578,13 +644,13 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a } maybe_event = events.next() => { match maybe_event { - Some(Ok(Event::Key(key))) if key.kind == KeyEventKind::Press => { + Some(Ok(Event::Key(key))) if matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) => { // Treat Ctrl-C like pressing 'q' in the current context. if key.modifiers.contains(KeyModifiers::CONTROL) && matches!(key.code, KeyCode::Char('c') | KeyCode::Char('C')) { - if app.pending_apply.is_some() { - app.pending_apply = None; + if app.apply_modal.is_some() { + app.apply_modal = None; app.status = "Apply canceled".to_string(); needs_redraw = true; } else if app.new_task.is_some() { @@ -609,7 +675,6 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a || matches!(key.code, KeyCode::Char('\u{000F}')); if is_ctrl_o && app.new_task.is_some() { // Close task modal/pending apply if present before opening env modal - app.pending_apply = None; app.diff_overlay = None; app.env_modal = Some(app::EnvModalState { query: String::new(), selected: 0 }); // Cache environments until user explicitly refreshes with 'r' inside the modal. @@ -617,6 +682,8 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a if should_fetch { app.env_loading = true; app.env_error = None; + // Ensure spinner animates while loading environments. + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); } needs_redraw = true; if should_fetch { @@ -703,49 +770,77 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a } } needs_redraw = true; + // If paste‑burst is active, schedule a micro‑flush frame. + if page.composer.is_in_paste_burst() { + let _ = frame_tx.send(Instant::now() + codex_tui::ComposerInput::recommended_flush_delay()); + } + // Always schedule an immediate redraw for key edits in the composer. + let _ = frame_tx.send(Instant::now()); + // Draw now so non-char edits (e.g., Option+Delete) reflect instantly. + render_if_needed(&mut terminal, &mut app, &mut needs_redraw)?; } } continue; } } // If a diff overlay is open, handle its keys first. - if app.pending_apply.is_some() { + if app.apply_modal.is_some() { + // Simple apply confirmation modal: y apply, p preflight, n/Esc cancel match key.code { KeyCode::Char('y') => { - if let Some((task_id, title)) = app.pending_apply.take() { - app.status = format!("Applying '{title}'..."); + if let Some(m) = app.apply_modal.take() { + app.status = format!("Applying '{}'...", m.title); needs_redraw = true; - match codex_cloud_tasks_api::CloudBackend::apply_task(&*backend, task_id.clone()).await { + match codex_cloud_tasks_api::CloudBackend::apply_task(&*backend, m.task_id.clone()).await { Ok(outcome) => { - // Always surface the message in the status bar. app.status = outcome.message.clone(); - // If the apply failed, also write a line to error.log for post-mortem debugging. - if matches!(outcome.status, codex_cloud_tasks_api::ApplyStatus::Error) { - append_error_log(format!( - "apply_task failed for {}: {}", - task_id.0, outcome.message - )); - } - // Keep the overlay open on errors/partial success so users can keep context. - // Only close and refresh on full success. if matches!(outcome.status, codex_cloud_tasks_api::ApplyStatus::Success) { app.diff_overlay = None; - if let Ok(tasks) = app::load_tasks(&*backend, app.env_filter.as_deref()).await { app.tasks = tasks; } + if let Ok(tasks) = app::load_tasks(&*backend, app.env_filter.as_deref()).await { app.tasks = tasks; } } } Err(e) => { - append_error_log(format!("apply_task failed for {}: {e}", task_id.0)); + append_error_log(format!("apply_task failed for {}: {e}", m.task_id.0)); app.status = format!("Apply failed: {e}"); } } needs_redraw = true; } } - KeyCode::Esc | KeyCode::Char('n') => { - app.pending_apply = None; - app.status = "Apply canceled".to_string(); - needs_redraw = true; + KeyCode::Char('p') => { + if let Some(m) = app.apply_modal.take() { + // Kick off async preflight; show spinner in modal body + app.apply_preflight_inflight = true; + app.apply_modal = Some(app::ApplyModalState { task_id: m.task_id.clone(), title: m.title.clone(), result_message: None, result_level: None, skipped_paths: Vec::new(), conflict_paths: Vec::new() }); + needs_redraw = true; + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); + let backend2 = backend.clone(); + let tx2 = tx.clone(); + let id2 = m.task_id.clone(); + let title2 = m.title.clone(); + tokio::spawn(async move { + unsafe { std::env::set_var("CODEX_APPLY_PREFLIGHT", "1") }; + let out = codex_cloud_tasks_api::CloudBackend::apply_task(&*backend2, id2.clone()).await; + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let evt = match out { + Ok(outcome) => { + let level = match outcome.status { + codex_cloud_tasks_api::ApplyStatus::Success => app::ApplyResultLevel::Success, + codex_cloud_tasks_api::ApplyStatus::Partial => app::ApplyResultLevel::Partial, + codex_cloud_tasks_api::ApplyStatus::Error => app::ApplyResultLevel::Error, + }; + app::AppEvent::ApplyPreflightFinished { id: id2, title: title2, message: outcome.message, level, skipped: outcome.skipped_paths, conflicts: outcome.conflict_paths } + } + Err(e) => app::AppEvent::ApplyPreflightFinished { id: id2, title: title2, message: format!("Preflight failed: {e}"), level: app::ApplyResultLevel::Error, skipped: Vec::new(), conflicts: Vec::new() }, + }; + let _ = tx2.send(evt); + }); + } } + KeyCode::Esc + | KeyCode::Char('n') + | KeyCode::Char('q') + | KeyCode::Char('Q') => { app.apply_modal = None; app.status = "Apply canceled".to_string(); needs_redraw = true; } _ => {} } } else if app.diff_overlay.is_some() { @@ -753,8 +848,30 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a KeyCode::Char('a') => { if let Some(ov) = &app.diff_overlay { if ov.can_apply { - app.pending_apply = Some((ov.task_id.clone(), ov.title.clone())); - app.status = format!("Apply '{}' ? y/N", ov.title); + app.apply_modal = Some(app::ApplyModalState { task_id: ov.task_id.clone(), title: ov.title.clone(), result_message: None, result_level: None, skipped_paths: Vec::new(), conflict_paths: Vec::new() }); + app.apply_preflight_inflight = true; + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); + let backend2 = backend.clone(); + let tx2 = tx.clone(); + let id2 = ov.task_id.clone(); + let title2 = ov.title.clone(); + tokio::spawn(async move { + unsafe { std::env::set_var("CODEX_APPLY_PREFLIGHT", "1") }; + let out = codex_cloud_tasks_api::CloudBackend::apply_task(&*backend2, id2.clone()).await; + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let evt = match out { + Ok(outcome) => { + let level = match outcome.status { + codex_cloud_tasks_api::ApplyStatus::Success => app::ApplyResultLevel::Success, + codex_cloud_tasks_api::ApplyStatus::Partial => app::ApplyResultLevel::Partial, + codex_cloud_tasks_api::ApplyStatus::Error => app::ApplyResultLevel::Error, + }; + app::AppEvent::ApplyPreflightFinished { id: id2, title: title2, message: outcome.message, level, skipped: outcome.skipped_paths, conflicts: outcome.conflict_paths } + } + Err(e) => app::AppEvent::ApplyPreflightFinished { id: id2, title: title2, message: format!("Preflight failed: {e}"), level: app::ApplyResultLevel::Error, skipped: Vec::new(), conflicts: Vec::new() }, + }; + let _ = tx2.send(evt); + }); } else { app.status = "No diff available to apply".to_string(); } @@ -763,7 +880,6 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a } // From task modal, 'o' should close it and open the env selector KeyCode::Char('o') | KeyCode::Char('O') => { - app.pending_apply = None; app.diff_overlay = None; app.env_modal = Some(app::EnvModalState { query: String::new(), selected: 0 }); // Use cached environments unless empty @@ -831,6 +947,7 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a KeyCode::Char('r') | KeyCode::Char('R') => { // Trigger refresh of environments app.env_loading = true; app.env_error = None; needs_redraw = true; + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); let tx2 = tx.clone(); tokio::spawn(async move { // Build headers (UA + ChatGPT token + account id) @@ -1042,14 +1159,38 @@ pub async fn run_main(_cli: Cli, _codex_linux_sandbox_exe: Option) -> a } } }); + // Animate spinner while details load. + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); } } KeyCode::Char('a') => { if let Some(task) = app.tasks.get(app.selected) { match codex_cloud_tasks_api::CloudBackend::get_task_diff(&*backend, task.id.clone()).await { Ok(_) => { - app.pending_apply = Some((task.id.clone(), task.title.clone())); - app.status = format!("Apply '{}' ? y/N", task.title); + app.apply_modal = Some(app::ApplyModalState { task_id: task.id.clone(), title: task.title.clone(), result_message: None, result_level: None, skipped_paths: Vec::new(), conflict_paths: Vec::new() }); + app.apply_preflight_inflight = true; + let _ = frame_tx.send(Instant::now() + Duration::from_millis(100)); + let backend2 = backend.clone(); + let tx2 = tx.clone(); + let id2 = task.id.clone(); + let title2 = task.title.clone(); + tokio::spawn(async move { + unsafe { std::env::set_var("CODEX_APPLY_PREFLIGHT", "1") }; + let out = codex_cloud_tasks_api::CloudBackend::apply_task(&*backend2, id2.clone()).await; + unsafe { std::env::remove_var("CODEX_APPLY_PREFLIGHT") }; + let evt = match out { + Ok(outcome) => { + let level = match outcome.status { + codex_cloud_tasks_api::ApplyStatus::Success => app::ApplyResultLevel::Success, + codex_cloud_tasks_api::ApplyStatus::Partial => app::ApplyResultLevel::Partial, + codex_cloud_tasks_api::ApplyStatus::Error => app::ApplyResultLevel::Error, + }; + app::AppEvent::ApplyPreflightFinished { id: id2, title: title2, message: outcome.message, level, skipped: outcome.skipped_paths, conflicts: outcome.conflict_paths } + } + Err(e) => app::AppEvent::ApplyPreflightFinished { id: id2, title: title2, message: format!("Preflight failed: {e}"), level: app::ApplyResultLevel::Error, skipped: Vec::new(), conflicts: Vec::new() }, + }; + let _ = tx2.send(evt); + }); } Err(_) => { app.status = "No diff available to apply".to_string(); diff --git a/codex-rs/cloud-tasks/src/ui.rs b/codex-rs/cloud-tasks/src/ui.rs index 7911b5ab05..2acaebb536 100644 --- a/codex-rs/cloud-tasks/src/ui.rs +++ b/codex-rs/cloud-tasks/src/ui.rs @@ -44,6 +44,9 @@ pub fn draw(frame: &mut Frame, app: &mut App) { if app.env_modal.is_some() { draw_env_modal(frame, area, app); } + if app.apply_modal.is_some() { + draw_apply_modal(frame, area, app); + } } // ===== Overlay helpers (geometry + styling) ===== @@ -160,7 +163,7 @@ fn draw_list(frame: &mut Frame, area: Rect, app: &mut App) { // Selection reflects the actual task index (no artificial spacer item). let mut state = ListState::default().with_selected(Some(app.selected)); // Dim task list when a modal/overlay is active to emphasize focus. - let dim_bg = app.env_modal.is_some() || app.diff_overlay.is_some(); + let dim_bg = app.env_modal.is_some() || app.apply_modal.is_some() || app.diff_overlay.is_some(); // Dynamic title includes current environment filter let suffix_span = if let Some(ref id) = app.env_filter { let label = app @@ -386,6 +389,85 @@ fn draw_diff_overlay(frame: &mut Frame, area: Rect, app: &mut App) { } } +pub fn draw_apply_modal(frame: &mut Frame, area: Rect, app: &mut App) { + use ratatui::widgets::Wrap; + let inner = overlay_outer(area); + let title = Line::from("Apply Changes?".magenta().bold()); + let block = overlay_block().title(title); + frame.render_widget(Clear, inner); + frame.render_widget(block.clone(), inner); + let content = overlay_content(inner); + + if let Some(m) = &app.apply_modal { + // Header + let header = Paragraph::new(Line::from( + format!("Apply '{}' ?", m.title).magenta().bold(), + )) + .wrap(Wrap { trim: true }); + // Footer instructions + let footer = + Paragraph::new(Line::from("Press Y to apply, P to preflight, N to cancel.").dim()) + .wrap(Wrap { trim: true }); + + // Split into header/body/footer + let rows = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(1), + Constraint::Min(1), + Constraint::Length(1), + ]) + .split(content); + + frame.render_widget(header, rows[0]); + // Body: spinner while preflight runs; otherwise show result message and path lists + if app.apply_preflight_inflight || m.result_message.is_none() { + draw_centered_spinner(frame, rows[1], &mut app.throbber, "Checking…"); + } else if let Some(msg) = &m.result_message { + let mut body_lines: Vec = Vec::new(); + let first = match m.result_level { + Some(crate::app::ApplyResultLevel::Success) => msg.clone().green(), + Some(crate::app::ApplyResultLevel::Partial) => msg.clone().yellow(), + Some(crate::app::ApplyResultLevel::Error) => msg.clone().red(), + None => msg.clone().into(), + }; + body_lines.push(Line::from(first)); + + // On partial or error, show conflicts/skips if present + if !matches!(m.result_level, Some(crate::app::ApplyResultLevel::Success)) { + use ratatui::text::Span; + if !m.conflict_paths.is_empty() { + body_lines.push(Line::from("")); + body_lines.push( + Line::from(format!("Conflicts ({}):", m.conflict_paths.len())) + .red() + .bold(), + ); + for p in &m.conflict_paths { + body_lines + .push(Line::from(vec![" • ".into(), Span::raw(p.clone()).dim()])); + } + } + if !m.skipped_paths.is_empty() { + body_lines.push(Line::from("")); + body_lines.push( + Line::from(format!("Skipped ({}):", m.skipped_paths.len())) + .yellow() + .bold(), + ); + for p in &m.skipped_paths { + body_lines + .push(Line::from(vec![" • ".into(), Span::raw(p.clone()).dim()])); + } + } + } + let body = Paragraph::new(body_lines).wrap(Wrap { trim: true }); + frame.render_widget(body, rows[1]); + } + frame.render_widget(footer, rows[2]); + } +} + fn style_diff_line(raw: &str) -> Line<'static> { use ratatui::style::Color; use ratatui::style::Modifier; diff --git a/codex-rs/tui/src/public_widgets/composer_input.rs b/codex-rs/tui/src/public_widgets/composer_input.rs index c11012bc71..c79c8de3c2 100644 --- a/codex-rs/tui/src/public_widgets/composer_input.rs +++ b/codex-rs/tui/src/public_widgets/composer_input.rs @@ -8,6 +8,7 @@ use crossterm::event::KeyEvent; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::widgets::WidgetRef; +use std::time::Duration; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -86,6 +87,23 @@ impl ComposerInput { pub fn render_ref(&self, area: Rect, buf: &mut Buffer) { WidgetRef::render_ref(&self.inner, area, buf); } + + /// Return true if a paste-burst detection is currently active. + pub fn is_in_paste_burst(&self) -> bool { + self.inner.is_in_paste_burst() + } + + /// Flush a pending paste-burst if the inter-key timeout has elapsed. + /// Returns true if text changed and a redraw is warranted. + pub fn flush_paste_burst_if_due(&mut self) -> bool { + self.inner.flush_paste_burst_if_due() + } + + /// Recommended delay to schedule the next micro-flush frame while a + /// paste-burst is active. + pub fn recommended_flush_delay() -> Duration { + crate::bottom_pane::ChatComposer::recommended_paste_flush_delay() + } } impl Default for ComposerInput {