diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 63195d9fa..5853b78c7 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::collections::HashSet; +use std::path::PathBuf; use std::sync::Arc; use std::sync::atomic::Ordering; @@ -73,7 +74,6 @@ use codex_core_skills::injection::InjectedHostSkillPrompts; use codex_extension_api::TurnInputContext; use codex_extension_api::TurnInputEnvironment; use codex_features::Feature; -use codex_git_utils::get_git_repo_root; use codex_git_utils::get_git_repo_root_with_fs; use codex_protocol::config_types::AutoCompactTokenLimitScope; use codex_protocol::config_types::ModeKind; @@ -196,21 +196,11 @@ pub(crate) async fn run_turn( let mut stop_hook_active = false; // Although from the perspective of codex.rs, TurnDiffTracker has the lifecycle of a Task which contains // many turns, from the perspective of the user, it is a single turn. - #[allow(deprecated)] - let display_root = match turn_context.environments.primary() { - Some(turn_environment) => get_git_repo_root_with_fs( - turn_environment.environment.get_filesystem().as_ref(), - &turn_environment.cwd, - ) - .await - .unwrap_or_else(|| turn_environment.cwd.clone()) - .into_path_buf(), - None => get_git_repo_root(turn_context.cwd.as_path()) - .unwrap_or_else(|| turn_context.cwd.clone().into_path_buf()), - }; - let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::with_display_root( - display_root, - ))); + let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new( + TurnDiffTracker::with_environment_display_roots( + turn_diff_display_roots(turn_context.as_ref()).await, + ), + )); // `ModelClientSession` is turn-scoped and caches WebSocket + sticky routing state, so we reuse // one instance across retries within this turn. @@ -421,6 +411,21 @@ pub(crate) async fn run_turn( last_agent_message } +async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, PathBuf)> { + let mut display_roots = Vec::new(); + for turn_environment in &turn_context.environments.turn_environments { + let root = get_git_repo_root_with_fs( + turn_environment.environment.get_filesystem().as_ref(), + &turn_environment.cwd, + ) + .await + .unwrap_or_else(|| turn_environment.cwd.clone()) + .into_path_buf(); + display_roots.push((turn_environment.environment_id.clone(), root)); + } + display_roots +} + async fn run_hooks_and_record_inputs( sess: &Arc, turn_context: &Arc, diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 747143ff6..e286af53d 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -70,16 +70,25 @@ pub(crate) enum ToolEventFailure<'a> { } enum TurnDiffTrackerUpdate<'a> { - Track(&'a AppliedPatchDelta), + Track { + environment_id: Option, + delta: &'a AppliedPatchDelta, + }, Invalidate, None, } -fn tracker_update_for_known_delta(delta: &AppliedPatchDelta) -> TurnDiffTrackerUpdate<'_> { +fn tracker_update_for_known_delta<'a>( + environment_id: Option<&str>, + delta: &'a AppliedPatchDelta, +) -> TurnDiffTrackerUpdate<'a> { if delta.is_exact() && delta.is_empty() { TurnDiffTrackerUpdate::None } else { - TurnDiffTrackerUpdate::Track(delta) + TurnDiffTrackerUpdate::Track { + environment_id: environment_id.map(str::to_string), + delta, + } } } @@ -120,6 +129,7 @@ pub(crate) enum ToolEmitter { ApplyPatch { changes: HashMap, auto_approved: bool, + environment_id: Option, }, UnifiedExec { command: Vec, @@ -141,10 +151,15 @@ impl ToolEmitter { } } - pub fn apply_patch(changes: HashMap, auto_approved: bool) -> Self { + pub fn apply_patch_for_environment( + changes: HashMap, + auto_approved: bool, + environment_id: String, + ) -> Self { Self::ApplyPatch { changes, auto_approved, + environment_id: Some(environment_id), } } @@ -210,7 +225,11 @@ impl ToolEmitter { .await; } ( - Self::ApplyPatch { changes, .. }, + Self::ApplyPatch { + changes, + environment_id, + .. + }, ToolEventStage::Success { output, applied_patch_delta, @@ -222,7 +241,7 @@ impl ToolEmitter { PatchApplyStatus::Failed }; let tracker_update = applied_patch_delta - .map(tracker_update_for_known_delta) + .map(|delta| tracker_update_for_known_delta(environment_id.as_deref(), delta)) .unwrap_or(TurnDiffTrackerUpdate::Invalidate); emit_patch_end( ctx, @@ -267,7 +286,11 @@ impl ToolEmitter { .await; } ( - Self::ApplyPatch { changes, .. }, + Self::ApplyPatch { + changes, + environment_id, + .. + }, ToolEventStage::Failure(ToolEventFailure::Rejected { message, applied_patch_delta, @@ -280,7 +303,9 @@ impl ToolEmitter { (*message).to_string(), PatchApplyStatus::Declined, applied_patch_delta - .map(tracker_update_for_known_delta) + .map(|delta| { + tracker_update_for_known_delta(environment_id.as_deref(), delta) + }) .unwrap_or(TurnDiffTrackerUpdate::None), ) .await; @@ -565,8 +590,11 @@ async fn emit_patch_end( let mut guard = tracker.lock().await; let previous_diff = guard.get_unified_diff(); let tracker_changed = match tracker_update { - TurnDiffTrackerUpdate::Track(delta) => { - guard.track_delta(delta); + TurnDiffTrackerUpdate::Track { + environment_id, + delta, + } => { + guard.track_delta(environment_id.as_deref().unwrap_or_default(), delta); true } TurnDiffTrackerUpdate::Invalidate => { @@ -627,14 +655,18 @@ mod tests { .await .expect("apply patch"); - ToolEmitter::apply_patch(HashMap::new(), /*auto_approved*/ false) - .finish( - ToolEventCtx::new(session.as_ref(), turn.as_ref(), "call-id", Some(&tracker)), - out, - Some(&delta), - ) - .await - .expect_err("failed patch"); + ToolEmitter::ApplyPatch { + changes: HashMap::new(), + auto_approved: false, + environment_id: None, + } + .finish( + ToolEventCtx::new(session.as_ref(), turn.as_ref(), "call-id", Some(&tracker)), + out, + Some(&delta), + ) + .await + .expect_err("failed patch"); let completed = rx_event.recv().await.expect("item completed event"); assert!(matches!( diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index c12ba0517..66fb1d308 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -378,8 +378,11 @@ impl ToolExecutor for ApplyPatchHandler { } InternalApplyPatchInvocation::DelegateToRuntime(apply) => { let changes = convert_apply_patch_to_protocol(&apply.action); - let emitter = - ToolEmitter::apply_patch(changes.clone(), apply.auto_approved); + let emitter = ToolEmitter::apply_patch_for_environment( + changes.clone(), + apply.auto_approved, + turn_environment.environment_id.clone(), + ); let event_ctx = ToolEventCtx::new( session.as_ref(), turn.as_ref(), @@ -537,7 +540,11 @@ pub(crate) async fn intercept_apply_patch( } InternalApplyPatchInvocation::DelegateToRuntime(apply) => { let changes = convert_apply_patch_to_protocol(&apply.action); - let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved); + let emitter = ToolEmitter::apply_patch_for_environment( + changes.clone(), + apply.auto_approved, + turn_environment.environment_id.clone(), + ); let event_ctx = ToolEventCtx::new( session.as_ref(), turn.as_ref(), diff --git a/codex-rs/core/src/turn_diff_tracker.rs b/codex-rs/core/src/turn_diff_tracker.rs index 3835ae234..388c2210b 100644 --- a/codex-rs/core/src/turn_diff_tracker.rs +++ b/codex-rs/core/src/turn_diff_tracker.rs @@ -13,21 +13,36 @@ const ZERO_OID: &str = "0000000000000000000000000000000000000000"; const DEV_NULL: &str = "/dev/null"; const REGULAR_FILE_MODE: &str = "100644"; +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +struct TrackedPath { + environment_id: String, + path: PathBuf, +} + +impl TrackedPath { + fn new(environment_id: &str, path: &Path) -> Self { + Self { + environment_id: environment_id.to_string(), + path: path.to_path_buf(), + } + } +} + /// Tracks the net text diff for the current turn from committed apply_patch /// mutations, without rereading the workspace filesystem. pub struct TurnDiffTracker { valid: bool, - display_root: Option, - baseline_by_path: HashMap, - current_by_path: HashMap, - origin_by_current_path: HashMap, + display_roots_by_environment: HashMap, + baseline_by_path: HashMap, + current_by_path: HashMap, + origin_by_current_path: HashMap, } impl Default for TurnDiffTracker { fn default() -> Self { Self { valid: true, - display_root: None, + display_roots_by_environment: HashMap::new(), baseline_by_path: HashMap::new(), current_by_path: HashMap::new(), origin_by_current_path: HashMap::new(), @@ -40,20 +55,22 @@ impl TurnDiffTracker { Self::default() } - pub fn with_display_root(display_root: PathBuf) -> Self { + pub fn with_environment_display_roots( + display_roots: impl IntoIterator, + ) -> Self { let mut tracker = Self::new(); - tracker.display_root = Some(display_root); + tracker.display_roots_by_environment = display_roots.into_iter().collect(); tracker } - pub fn track_delta(&mut self, delta: &AppliedPatchDelta) { + pub fn track_delta(&mut self, environment_id: &str, delta: &AppliedPatchDelta) { if !delta.is_exact() { self.invalidate(); return; } for change in delta.changes() { - self.apply_change(change); + self.apply_change(environment_id, change); } } @@ -106,8 +123,8 @@ impl TurnDiffTracker { (!aggregated.is_empty()).then_some(aggregated) } - fn apply_change(&mut self, change: &AppliedPatchChange) { - let source_path = change.path.as_path(); + fn apply_change(&mut self, environment_id: &str, change: &AppliedPatchChange) { + let source_path = TrackedPath::new(environment_id, change.path.as_path()); match &change.change { AppliedPatchFileChange::Add { content, @@ -119,85 +136,87 @@ impl TurnDiffTracker { old_content, overwritten_move_content, new_content, - } => self.apply_update( - source_path, - move_path.as_deref(), - old_content, - overwritten_move_content.as_deref(), - new_content, - ), + } => { + let move_path = move_path + .as_deref() + .map(|path| TrackedPath::new(environment_id, path)); + self.apply_update( + source_path, + move_path, + old_content, + overwritten_move_content.as_deref(), + new_content, + ) + } } } - fn apply_add(&mut self, path: &Path, content: &str, overwritten_content: Option<&str>) { - self.origin_by_current_path.remove(path); - if !self.current_by_path.contains_key(path) - && !self.baseline_by_path.contains_key(path) + fn apply_add(&mut self, path: TrackedPath, content: &str, overwritten_content: Option<&str>) { + self.origin_by_current_path.remove(&path); + if !self.current_by_path.contains_key(&path) + && !self.baseline_by_path.contains_key(&path) && let Some(overwritten_content) = overwritten_content { self.baseline_by_path - .insert(path.to_path_buf(), overwritten_content.to_string()); + .insert(path.clone(), overwritten_content.to_string()); } - self.current_by_path - .insert(path.to_path_buf(), content.to_string()); + self.current_by_path.insert(path, content.to_string()); } - fn apply_delete(&mut self, path: &Path, content: &str) { - if self.current_by_path.remove(path).is_none() && !self.baseline_by_path.contains_key(path) + fn apply_delete(&mut self, path: TrackedPath, content: &str) { + if self.current_by_path.remove(&path).is_none() + && !self.baseline_by_path.contains_key(&path) { self.baseline_by_path - .insert(path.to_path_buf(), content.to_string()); + .insert(path.clone(), content.to_string()); } - self.origin_by_current_path.remove(path); + self.origin_by_current_path.remove(&path); } fn apply_update( &mut self, - source_path: &Path, - move_path: Option<&Path>, + source_path: TrackedPath, + move_path: Option, old_content: &str, overwritten_move_content: Option<&str>, new_content: &str, ) { - if !self.current_by_path.contains_key(source_path) - && !self.baseline_by_path.contains_key(source_path) + if !self.current_by_path.contains_key(&source_path) + && !self.baseline_by_path.contains_key(&source_path) { self.baseline_by_path - .insert(source_path.to_path_buf(), old_content.to_string()); + .insert(source_path.clone(), old_content.to_string()); } match move_path { Some(dest_path) => { - if !self.current_by_path.contains_key(dest_path) - && !self.baseline_by_path.contains_key(dest_path) + if !self.current_by_path.contains_key(&dest_path) + && !self.baseline_by_path.contains_key(&dest_path) && let Some(overwritten_move_content) = overwritten_move_content { - self.baseline_by_path.insert( - dest_path.to_path_buf(), - overwritten_move_content.to_string(), - ); + self.baseline_by_path + .insert(dest_path.clone(), overwritten_move_content.to_string()); } let origin = self .origin_by_current_path - .remove(source_path) - .unwrap_or_else(|| source_path.to_path_buf()); - self.current_by_path.remove(source_path); + .remove(&source_path) + .unwrap_or_else(|| source_path.clone()); + self.current_by_path.remove(&source_path); self.current_by_path - .insert(dest_path.to_path_buf(), new_content.to_string()); - self.origin_by_current_path.remove(dest_path); - if dest_path != origin.as_path() { - self.origin_by_current_path - .insert(dest_path.to_path_buf(), origin); + .insert(dest_path.clone(), new_content.to_string()); + self.origin_by_current_path.remove(&dest_path); + if dest_path != origin { + self.origin_by_current_path.insert(dest_path, origin); } } None => { self.current_by_path - .insert(source_path.to_path_buf(), new_content.to_string()); + .insert(source_path, new_content.to_string()); } } } - fn rename_pairs(&self) -> HashMap { + fn rename_pairs(&self) -> HashMap { self.origin_by_current_path .iter() .filter_map(|(dest_path, origin_path)| { @@ -215,7 +234,7 @@ impl TurnDiffTracker { .collect() } - fn render_path_diff(&self, path: &Path) -> Option { + fn render_path_diff(&self, path: &TrackedPath) -> Option { self.render_diff( path, self.baseline_by_path.get(path).map(String::as_str), @@ -224,7 +243,11 @@ impl TurnDiffTracker { ) } - fn render_rename_diff(&self, source_path: &Path, dest_path: &Path) -> Option { + fn render_rename_diff( + &self, + source_path: &TrackedPath, + dest_path: &TrackedPath, + ) -> Option { self.render_diff( source_path, self.baseline_by_path.get(source_path).map(String::as_str), @@ -235,9 +258,9 @@ impl TurnDiffTracker { fn render_diff( &self, - left_path: &Path, + left_path: &TrackedPath, left_content: Option<&str>, - right_path: &Path, + right_path: &TrackedPath, right_content: Option<&str>, ) -> Option { if left_content == right_content { @@ -286,13 +309,18 @@ impl TurnDiffTracker { Some(diff) } - fn display_path(&self, path: &Path) -> String { + fn display_path(&self, path: &TrackedPath) -> String { let display = self - .display_root - .as_deref() - .and_then(|root| path.strip_prefix(root).ok()) - .unwrap_or(path); - display.display().to_string().replace('\\', "/") + .display_roots_by_environment + .get(&path.environment_id) + .and_then(|root| path.path.strip_prefix(root).ok()) + .unwrap_or(path.path.as_path()); + let display = display.display().to_string().replace('\\', "/"); + if self.display_roots_by_environment.len() > 1 && !path.environment_id.is_empty() { + format!("{}/{display}", path.environment_id) + } else { + display + } } } diff --git a/codex-rs/core/src/turn_diff_tracker_tests.rs b/codex-rs/core/src/turn_diff_tracker_tests.rs index d25fec2aa..645443040 100644 --- a/codex-rs/core/src/turn_diff_tracker_tests.rs +++ b/codex-rs/core/src/turn_diff_tracker_tests.rs @@ -41,24 +41,28 @@ async fn apply_verified_patch(root: &Path, patch: &str) -> AppliedPatchDelta { .expect("patch should apply") } +fn tracker_with_root(root: &Path) -> TurnDiffTracker { + TurnDiffTracker::with_environment_display_roots([("".to_string(), root.to_path_buf())]) +} + #[tokio::test] async fn accumulates_add_then_update_as_single_add() { let dir = tempdir().expect("tempdir"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let add = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Add File: a.txt\n+foo\n*** End Patch", ) .await; - tracker.track_delta(&add); + tracker.track_delta("", &add); let update = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Update File: a.txt\n@@\n foo\n+bar\n*** End Patch", ) .await; - tracker.track_delta(&update); + tracker.track_delta("", &update); let right_oid = git_blob_sha1_hex("foo\nbar\n"); let expected = format!( @@ -78,32 +82,69 @@ index {ZERO_OID}..{right_oid} #[tokio::test] async fn invalidated_tracker_suppresses_existing_diff() { let dir = tempdir().expect("tempdir"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let add = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Add File: a.txt\n+foo\n*** End Patch", ) .await; - tracker.track_delta(&add); + tracker.track_delta("", &add); tracker.invalidate(); assert_eq!(tracker.get_unified_diff(), None); } +#[tokio::test] +async fn tracks_same_absolute_path_across_multiple_environments() { + let dir = tempdir().expect("tempdir"); + let add = apply_verified_patch( + dir.path(), + "*** Begin Patch\n*** Add File: shared.txt\n+content\n*** End Patch", + ) + .await; + + let mut tracker = TurnDiffTracker::with_environment_display_roots([ + ("local".to_string(), dir.path().to_path_buf()), + ("remote".to_string(), dir.path().to_path_buf()), + ]); + tracker.track_delta("remote", &add); + tracker.track_delta("local", &add); + + let right_oid = git_blob_sha1_hex("content\n"); + let expected = format!( + r#"diff --git a/local/shared.txt b/local/shared.txt +new file mode {REGULAR_FILE_MODE} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b/local/shared.txt +@@ -0,0 +1 @@ ++content +diff --git a/remote/shared.txt b/remote/shared.txt +new file mode {REGULAR_FILE_MODE} +index {ZERO_OID}..{right_oid} +--- {DEV_NULL} ++++ b/remote/shared.txt +@@ -0,0 +1 @@ ++content +"#, + ); + assert_eq!(tracker.get_unified_diff(), Some(expected)); +} + #[tokio::test] async fn accumulates_delete() { let dir = tempdir().expect("tempdir"); fs::write(dir.path().join("b.txt"), "x\n").expect("seed file"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let delete = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Delete File: b.txt\n*** End Patch", ) .await; - tracker.track_delta(&delete); + tracker.track_delta("", &delete); let left_oid = git_blob_sha1_hex("x\n"); let expected = format!( @@ -124,13 +165,13 @@ async fn accumulates_move_and_update() { let dir = tempdir().expect("tempdir"); fs::write(dir.path().join("src.txt"), "line\n").expect("seed file"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let update = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Update File: src.txt\n*** Move to: dst.txt\n@@\n-line\n+line2\n*** End Patch", ) .await; - tracker.track_delta(&update); + tracker.track_delta("", &update); let left_oid = git_blob_sha1_hex("line\n"); let right_oid = git_blob_sha1_hex("line2\n"); @@ -152,13 +193,13 @@ async fn pure_rename_yields_no_diff() { let dir = tempdir().expect("tempdir"); fs::write(dir.path().join("old.txt"), "same\n").expect("seed file"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let rename = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n same\n*** End Patch", ) .await; - tracker.track_delta(&rename); + tracker.track_delta("", &rename); assert_eq!(tracker.get_unified_diff(), None); } @@ -168,13 +209,13 @@ async fn add_over_existing_file_becomes_update() { let dir = tempdir().expect("tempdir"); fs::write(dir.path().join("dup.txt"), "before\n").expect("seed file"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let add = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Add File: dup.txt\n+after\n*** End Patch", ) .await; - tracker.track_delta(&add); + tracker.track_delta("", &add); let left_oid = git_blob_sha1_hex("before\n"); let right_oid = git_blob_sha1_hex("after\n"); @@ -196,20 +237,20 @@ async fn delete_then_readd_same_path_becomes_update() { let dir = tempdir().expect("tempdir"); fs::write(dir.path().join("cycle.txt"), "before\n").expect("seed file"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let delete = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Delete File: cycle.txt\n*** End Patch", ) .await; - tracker.track_delta(&delete); + tracker.track_delta("", &delete); let add = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Add File: cycle.txt\n+after\n*** End Patch", ) .await; - tracker.track_delta(&add); + tracker.track_delta("", &add); let left_oid = git_blob_sha1_hex("before\n"); let right_oid = git_blob_sha1_hex("after\n"); @@ -232,13 +273,13 @@ async fn move_over_existing_destination_without_content_change_deletes_source_on fs::write(dir.path().join("a.txt"), "same\n").expect("seed source"); fs::write(dir.path().join("b.txt"), "same\n").expect("seed destination"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let move_overwrite = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Update File: a.txt\n*** Move to: b.txt\n@@\n same\n*** End Patch", ) .await; - tracker.track_delta(&move_overwrite); + tracker.track_delta("", &move_overwrite); let left_oid = git_blob_sha1_hex("same\n"); let expected = format!( @@ -261,13 +302,13 @@ async fn move_over_existing_destination_with_content_change_deletes_source_and_u fs::write(dir.path().join("a.txt"), "from\n").expect("seed source"); fs::write(dir.path().join("b.txt"), "existing\n").expect("seed destination"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let move_overwrite = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Update File: a.txt\n*** Move to: b.txt\n@@\n-from\n+new\n*** End Patch", ) .await; - tracker.track_delta(&move_overwrite); + tracker.track_delta("", &move_overwrite); let left_oid_a = git_blob_sha1_hex("from\n"); let left_oid_b = git_blob_sha1_hex("existing\n"); @@ -298,13 +339,13 @@ async fn preserves_committed_change_order_with_delete_then_move_overwrite() { fs::write(dir.path().join("a.txt"), "from\n").expect("seed source"); fs::write(dir.path().join("b.txt"), "existing\n").expect("seed destination"); - let mut tracker = TurnDiffTracker::with_display_root(dir.path().to_path_buf()); + let mut tracker = tracker_with_root(dir.path()); let ordered_patch = apply_verified_patch( dir.path(), "*** Begin Patch\n*** Delete File: b.txt\n*** Update File: a.txt\n*** Move to: b.txt\n@@\n-from\n+new\n*** End Patch", ) .await; - tracker.track_delta(&ordered_patch); + tracker.track_delta("", &ordered_patch); let left_oid_a = git_blob_sha1_hex("from\n"); let left_oid_b = git_blob_sha1_hex("existing\n"); diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 1e2493a80..db301e39b 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -8,11 +8,18 @@ use core_test_support::responses::ev_apply_patch_shell_command_call_via_heredoc; use core_test_support::responses::ev_shell_command_call; use core_test_support::test_codex::ApplyPatchModelOutput; use pretty_assertions::assert_eq; +use std::fs; +use std::path::PathBuf; use std::sync::atomic::AtomicI32; use std::sync::atomic::Ordering; use std::time::Duration; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; use codex_exec_server::CreateDirectoryOptions; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_exec_server::REMOTE_ENVIRONMENT_ID; +use codex_exec_server::RemoveOptions; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::FileSystemAccessMode; @@ -25,11 +32,14 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::user_input::UserInput; #[cfg(target_os = "linux")] use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_utils_absolute_path::AbsolutePathBuf; +use core_test_support::PathBufExt; use core_test_support::assert_regex_match; +use core_test_support::get_remote_test_env; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -37,11 +47,13 @@ use core_test_support::responses::ev_response_created; use core_test_support::responses::ev_shell_command_call_with_args; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::skip_if_remote; use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::TestCodexHarness; use core_test_support::test_codex::test_codex; +use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; use serde_json::json; @@ -1553,6 +1565,162 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Result<()> { + skip_if_no_network!(Ok(())); + let Some(_remote_env) = get_remote_test_env() else { + return Ok(()); + }; + + let server = start_mock_server().await; + let mut builder = test_codex(); + let test = builder.build_with_remote_and_local_env(&server).await?; + let file_name = "shared-turn-diff.txt"; + let shared_cwd = PathBuf::from(format!( + "/tmp/codex-remote-turn-diff-{}", + SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() + )) + .abs(); + let _ = fs::remove_dir_all(shared_cwd.as_path()); + test.fs() + .remove( + &shared_cwd, + RemoveOptions { + recursive: true, + force: true, + }, + /*sandbox*/ None, + ) + .await?; + fs::create_dir_all(shared_cwd.as_path())?; + test.fs() + .create_directory( + &shared_cwd, + CreateDirectoryOptions { recursive: true }, + /*sandbox*/ None, + ) + .await?; + + let local_patch = format!( + "*** Begin Patch\n*** Environment ID: {LOCAL_ENVIRONMENT_ID}\n*** Add File: {file_name}\n+local\n*** End Patch" + ); + let remote_patch = format!( + "*** Begin Patch\n*** Environment ID: {REMOTE_ENVIRONMENT_ID}\n*** Add File: {file_name}\n+remote\n*** End Patch" + ); + mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("resp-local"), + ev_apply_patch_custom_tool_call("call-local", &local_patch), + ev_completed("resp-local"), + ]), + sse(vec![ + ev_response_created("resp-remote"), + ev_apply_patch_custom_tool_call("call-remote", &remote_patch), + ev_completed("resp-remote"), + ]), + sse(vec![ + ev_response_created("resp-done"), + ev_assistant_message("msg-done", "done"), + ev_completed("resp-done"), + ]), + ], + ) + .await; + + let (sandbox_policy, permission_profile) = + turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); + test.codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "apply matching patches to local and remote environments".into(), + text_elements: Vec::new(), + }], + environments: Some(vec![ + TurnEnvironmentSelection { + environment_id: LOCAL_ENVIRONMENT_ID.to_string(), + cwd: shared_cwd.clone(), + }, + TurnEnvironmentSelection { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd: shared_cwd.clone(), + }, + ]), + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: codex_protocol::protocol::ThreadSettingsOverrides { + cwd: Some(test.config.cwd.clone()), + approval_policy: Some(AskForApproval::Never), + sandbox_policy: Some(sandbox_policy), + permission_profile, + collaboration_mode: Some(codex_protocol::config_types::CollaborationMode { + mode: codex_protocol::config_types::ModeKind::Default, + settings: codex_protocol::config_types::Settings { + model: test.session_configured.model.clone(), + reasoning_effort: None, + developer_instructions: None, + }, + }), + ..Default::default() + }, + }) + .await?; + + let mut last_diff = None; + wait_for_event(&test.codex, |event| match event { + EventMsg::TurnDiff(ev) => { + last_diff = Some(ev.unified_diff.clone()); + false + } + EventMsg::TurnComplete(_) => true, + _ => false, + }) + .await; + + assert_eq!(fs::read_to_string(shared_cwd.join(file_name))?, "local\n"); + assert_eq!( + test.fs() + .read_file_text(&shared_cwd.join(file_name), /*sandbox*/ None) + .await?, + "remote\n" + ); + let diff = last_diff.expect("expected TurnDiff event"); + assert_eq!( + diff, + r#"diff --git a/local/shared-turn-diff.txt b/local/shared-turn-diff.txt +new file mode 100644 +index 0000000000000000000000000000000000000000..40830374235df1c19661a2901b7ca73cc9499f3d +--- /dev/null ++++ b/local/shared-turn-diff.txt +@@ -0,0 +1 @@ ++local +diff --git a/remote/shared-turn-diff.txt b/remote/shared-turn-diff.txt +new file mode 100644 +index 0000000000000000000000000000000000000000..9c998f7b995a7327177b38a90d1385170df2b94b +--- /dev/null ++++ b/remote/shared-turn-diff.txt +@@ -0,0 +1 @@ ++remote +"# + ); + + let _ = fs::remove_dir_all(shared_cwd.as_path()); + test.fs() + .remove( + &shared_cwd, + RemoveOptions { + recursive: true, + force: true, + }, + /*sandbox*/ None, + ) + .await?; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> { skip_if_no_network!(Ok(()));