From 92a437aac1fac9bc41e14415144f170afc869ca2 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Mon, 23 Mar 2026 13:16:51 +0100 Subject: [PATCH] cleanup --- crates/acp_thread/src/acp_thread.rs | 445 +++++---------------- crates/action_log/src/action_log.rs | 585 +++++++++++++++++++++++++++- 2 files changed, 681 insertions(+), 349 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 8d6136f9da7a40ee1f82d878cef5241152418952..ffffa79bd287672dccc1195061b8b7f4269d8636 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -9,15 +9,10 @@ use collections::HashSet; pub use connection::*; pub use diff::*; use futures::{FutureExt, channel::oneshot, future::BoxFuture}; -use gpui::{ - AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Subscription, Task, - WeakEntity, -}; +use gpui::{AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Task, WeakEntity}; use itertools::Itertools; use language::language_settings::FormatOnSave; -use language::{ - Anchor, Buffer, BufferEvent, BufferSnapshot, LanguageRegistry, Point, ToPoint, text_diff, -}; +use language::{Anchor, Buffer, BufferSnapshot, LanguageRegistry, Point, ToPoint, text_diff}; use markdown::Markdown; pub use mention::*; use project::lsp_store::{FormatTrigger, LspFormatTarget}; @@ -1021,12 +1016,6 @@ struct RunningTurn { struct InferredEditCandidateReady { nonce: u64, buffer: Entity, - baseline_snapshot: text::BufferSnapshot, - existed_on_disk: bool, - was_dirty: bool, - observed_external_file_change: bool, - has_inferred_edit: bool, - _subscription: Subscription, } enum InferredEditCandidateState { @@ -1774,20 +1763,32 @@ impl AcpThread { tool_call_id: &acp::ToolCallId, abs_path: &PathBuf, nonce: u64, + cx: &mut Context, ) { + let mut buffer_to_end = None; let remove_tool_call = if let Some(candidates) = self.inferred_edit_candidates.get_mut(tool_call_id) { let should_remove = candidates .get(abs_path) .is_some_and(|candidate_state| candidate_state.nonce() == nonce); if should_remove { - candidates.remove(abs_path); + if let Some(InferredEditCandidateState::Ready(candidate)) = + candidates.remove(abs_path) + { + buffer_to_end = Some(candidate.buffer); + } } candidates.is_empty() } else { false }; + if let Some(buffer) = buffer_to_end { + self.action_log.update(cx, |action_log, cx| { + action_log.end_expected_external_edit(buffer, cx); + }); + } + if remove_tool_call { self.inferred_edit_candidates.remove(tool_call_id); self.finalizing_inferred_edit_tool_calls @@ -1798,12 +1799,27 @@ impl AcpThread { fn clear_inferred_edit_candidates_for_tool_calls( &mut self, tool_call_ids: impl IntoIterator, + cx: &mut Context, ) { + let mut buffers_to_end = Vec::new(); + for tool_call_id in tool_call_ids { - self.inferred_edit_candidates.remove(&tool_call_id); + if let Some(candidates) = self.inferred_edit_candidates.remove(&tool_call_id) { + for candidate_state in candidates.into_values() { + if let InferredEditCandidateState::Ready(candidate) = candidate_state { + buffers_to_end.push(candidate.buffer); + } + } + } self.finalizing_inferred_edit_tool_calls .remove(&tool_call_id); } + + for buffer in buffers_to_end { + self.action_log.update(cx, |action_log, cx| { + action_log.end_expected_external_edit(buffer, cx); + }); + } } fn finalize_all_inferred_edit_tool_calls(&mut self, cx: &mut Context) { @@ -1827,19 +1843,38 @@ impl AcpThread { &mut self, tool_call_id: &acp::ToolCallId, locations: &[acp::ToolCallLocation], + cx: &mut Context, ) { let mut current_paths = HashSet::default(); for location in locations { current_paths.insert(location.path.clone()); } - let remove_tool_call = - if let Some(candidates) = self.inferred_edit_candidates.get_mut(tool_call_id) { - candidates.retain(|path, _| current_paths.contains(path)); - candidates.is_empty() - } else { - false - }; + let mut buffers_to_end = Vec::new(); + let remove_tool_call = if let Some(candidates) = + self.inferred_edit_candidates.get_mut(tool_call_id) + { + let removed_paths = candidates + .keys() + .filter(|path| !current_paths.contains(*path)) + .cloned() + .collect::>(); + for path in removed_paths { + if let Some(InferredEditCandidateState::Ready(candidate)) = candidates.remove(&path) + { + buffers_to_end.push(candidate.buffer); + } + } + candidates.is_empty() + } else { + false + }; + + for buffer in buffers_to_end { + self.action_log.update(cx, |action_log, cx| { + action_log.end_expected_external_edit(buffer, cx); + }); + } if remove_tool_call { self.inferred_edit_candidates.remove(tool_call_id); @@ -1854,7 +1889,7 @@ impl AcpThread { locations: &[acp::ToolCallLocation], cx: &mut Context, ) { - self.sync_inferred_edit_candidate_paths(&tool_call_id, locations); + self.sync_inferred_edit_candidate_paths(&tool_call_id, locations, cx); let mut unique_paths = HashSet::default(); for location in locations { @@ -1880,38 +1915,40 @@ impl AcpThread { let open_buffer = self.project.update(cx, |project, cx| { let project_path = project.project_path_for_absolute_path(&abs_path, cx)?; if let Some(buffer) = project.get_open_buffer(&project_path, cx) { - let (baseline_snapshot, existed_on_disk, was_dirty) = - Self::inferred_edit_candidate_baseline(&buffer, cx); - return Some(( - Some((buffer, baseline_snapshot, existed_on_disk, was_dirty)), - None, - )); + return Some((Some(buffer), None)); } Some((None, Some(project.open_buffer(project_path, cx)))) }); - let Some((ready_candidate, open_buffer)) = open_buffer else { - self.remove_inferred_edit_candidate_if_matching(&tool_call_id, &abs_path, nonce); + let Some((ready_buffer, open_buffer)) = open_buffer else { + self.remove_inferred_edit_candidate_if_matching( + &tool_call_id, + &abs_path, + nonce, + cx, + ); continue; }; - if let Some((buffer, baseline_snapshot, existed_on_disk, was_dirty)) = ready_candidate { + if let Some(buffer) = ready_buffer { self.set_inferred_edit_candidate_ready( tool_call_id.clone(), abs_path, nonce, buffer, - baseline_snapshot, - existed_on_disk, - was_dirty, cx, ); continue; } let Some(open_buffer) = open_buffer else { - self.remove_inferred_edit_candidate_if_matching(&tool_call_id, &abs_path, nonce); + self.remove_inferred_edit_candidate_if_matching( + &tool_call_id, + &abs_path, + nonce, + cx, + ); continue; }; @@ -1920,11 +1957,12 @@ impl AcpThread { let buffer = match open_buffer.await { Ok(buffer) => buffer, Err(_) => { - this.update(cx, |this, _| { + this.update(cx, |this, cx| { this.remove_inferred_edit_candidate_if_matching( &tool_call_id, &abs_path, nonce, + cx, ); }) .ok(); @@ -1932,18 +1970,12 @@ impl AcpThread { } }; - let (baseline_snapshot, existed_on_disk, was_dirty) = - Self::inferred_edit_candidate_baseline(&buffer, cx); - this.update(cx, |this, cx| { this.set_inferred_edit_candidate_ready( tool_call_id.clone(), abs_path.clone(), nonce, buffer, - baseline_snapshot, - existed_on_disk, - was_dirty, cx, ); }) @@ -1955,67 +1987,14 @@ impl AcpThread { } } - fn inferred_edit_candidate_baseline( - buffer: &Entity, - cx: &mut impl AppContext, - ) -> (text::BufferSnapshot, bool, bool) { - buffer.read_with(cx, |buffer, _| { - ( - buffer.text_snapshot(), - buffer.file().is_some_and(|file| file.disk_state().exists()), - buffer.is_dirty(), - ) - }) - } - - fn update_ready_inferred_edit_candidate_if_matching( - &mut self, - tool_call_id: &acp::ToolCallId, - abs_path: &PathBuf, - nonce: u64, - update: impl FnOnce(&mut InferredEditCandidateReady), - ) -> bool { - let Some(candidates) = self.inferred_edit_candidates.get_mut(tool_call_id) else { - return false; - }; - let Some(InferredEditCandidateState::Ready(candidate)) = candidates.get_mut(abs_path) - else { - return false; - }; - if candidate.nonce != nonce { - return false; - } - - update(candidate); - true - } - fn set_inferred_edit_candidate_ready( &mut self, tool_call_id: acp::ToolCallId, abs_path: PathBuf, nonce: u64, buffer: Entity, - baseline_snapshot: text::BufferSnapshot, - existed_on_disk: bool, - was_dirty: bool, cx: &mut Context, ) { - let subscription = cx.subscribe(&buffer, { - let tool_call_id = tool_call_id.clone(); - let abs_path = abs_path.clone(); - move |this, buffer, event: &BufferEvent, cx| { - this.handle_inferred_edit_candidate_buffer_event( - tool_call_id.clone(), - abs_path.clone(), - nonce, - buffer, - event, - cx, - ); - } - }); - let Some(candidates) = self.inferred_edit_candidates.get_mut(&tool_call_id) else { return; }; @@ -2026,235 +2005,13 @@ impl AcpThread { return; } - *candidate_state = InferredEditCandidateState::Ready(InferredEditCandidateReady { - nonce, - buffer, - baseline_snapshot, - existed_on_disk, - was_dirty, - observed_external_file_change: false, - has_inferred_edit: false, - _subscription: subscription, - }); - } - - fn can_infer_initial_external_edit_for_candidate( - &self, - buffer: &Entity, - was_dirty: bool, - cx: &App, - ) -> bool { - if was_dirty { - return false; - } - - if buffer.read_with(cx, |buffer, _| buffer.is_dirty()) { - return false; - } - - !self.action_log.read_with(cx, |action_log, cx| { - action_log.has_changed_buffer(buffer, cx) - }) - } + let buffer_for_action_log = buffer.clone(); + *candidate_state = + InferredEditCandidateState::Ready(InferredEditCandidateReady { nonce, buffer }); - fn inferred_edit_candidate_has_meaningful_change( - existed_on_disk: bool, - baseline_snapshot: &text::BufferSnapshot, - buffer: &Entity, - cx: &App, - ) -> bool { - let (current_snapshot, current_exists) = buffer.read_with(cx, |buffer, _| { - ( - buffer.text_snapshot(), - buffer.file().is_some_and(|file| file.disk_state().exists()), - ) + self.action_log.update(cx, |action_log, cx| { + action_log.begin_expected_external_edit(buffer_for_action_log, cx); }); - - if !existed_on_disk { - current_exists || current_snapshot.text() != baseline_snapshot.text() - } else { - current_snapshot.text() != baseline_snapshot.text() - } - } - - fn infer_inferred_edit_candidate_from_baseline( - &mut self, - buffer: Entity, - baseline_snapshot: text::BufferSnapshot, - existed_on_disk: bool, - cx: &mut Context, - ) { - if existed_on_disk { - self.action_log.update(cx, |action_log, cx| { - action_log.infer_buffer_edited_from_snapshot(buffer.clone(), baseline_snapshot, cx); - }); - } else { - self.action_log.update(cx, |action_log, cx| { - action_log.infer_buffer_created(buffer.clone(), baseline_snapshot, cx); - }); - } - } - - fn handle_inferred_edit_candidate_buffer_event( - &mut self, - tool_call_id: acp::ToolCallId, - abs_path: PathBuf, - nonce: u64, - buffer: Entity, - event: &BufferEvent, - cx: &mut Context, - ) { - let Some(InferredEditCandidateState::Ready(candidate)) = self - .inferred_edit_candidates - .get(&tool_call_id) - .and_then(|candidates| candidates.get(&abs_path)) - else { - return; - }; - if candidate.nonce != nonce { - return; - } - - let baseline_snapshot = candidate.baseline_snapshot.clone(); - let existed_on_disk = candidate.existed_on_disk; - let was_dirty = candidate.was_dirty; - let observed_external_file_change = candidate.observed_external_file_change; - let has_inferred_edit = candidate.has_inferred_edit; - - enum Action { - None, - Remove, - SetObservedExternalFileChange, - ClearObservedExternalFileChange, - ClearObservedAndTrackEdited, - ClearObservedAndInferCreatedOrEdited, - RemoveAndWillDeleteBuffer, - RemoveAndInferDeleted, - } - - let action = match event { - BufferEvent::Edited { is_local: true } - if !has_inferred_edit && !observed_external_file_change => - { - Action::Remove - } - BufferEvent::Saved if !has_inferred_edit => Action::Remove, - BufferEvent::FileHandleChanged => { - let is_deleted = buffer.read_with(cx, |buffer, _| { - buffer - .file() - .is_some_and(|file| file.disk_state().is_deleted()) - }); - - if is_deleted { - if has_inferred_edit { - Action::RemoveAndWillDeleteBuffer - } else if self - .can_infer_initial_external_edit_for_candidate(&buffer, was_dirty, cx) - { - Action::RemoveAndInferDeleted - } else { - Action::Remove - } - } else { - Action::SetObservedExternalFileChange - } - } - BufferEvent::Reloaded if observed_external_file_change => { - if has_inferred_edit { - Action::ClearObservedAndTrackEdited - } else if self.can_infer_initial_external_edit_for_candidate(&buffer, was_dirty, cx) - && Self::inferred_edit_candidate_has_meaningful_change( - existed_on_disk, - &baseline_snapshot, - &buffer, - cx, - ) - { - Action::ClearObservedAndInferCreatedOrEdited - } else { - Action::ClearObservedExternalFileChange - } - } - _ => Action::None, - }; - - match action { - Action::None => {} - Action::Remove => { - self.remove_inferred_edit_candidate_if_matching(&tool_call_id, &abs_path, nonce); - } - Action::SetObservedExternalFileChange => { - self.update_ready_inferred_edit_candidate_if_matching( - &tool_call_id, - &abs_path, - nonce, - |candidate| { - candidate.observed_external_file_change = true; - }, - ); - } - Action::ClearObservedExternalFileChange => { - self.update_ready_inferred_edit_candidate_if_matching( - &tool_call_id, - &abs_path, - nonce, - |candidate| { - candidate.observed_external_file_change = false; - }, - ); - } - Action::ClearObservedAndTrackEdited => { - let updated = self.update_ready_inferred_edit_candidate_if_matching( - &tool_call_id, - &abs_path, - nonce, - |candidate| { - candidate.observed_external_file_change = false; - }, - ); - if updated { - self.action_log.update(cx, |action_log, cx| { - action_log.buffer_edited(buffer.clone(), cx); - }); - } - } - Action::ClearObservedAndInferCreatedOrEdited => { - let updated = self.update_ready_inferred_edit_candidate_if_matching( - &tool_call_id, - &abs_path, - nonce, - |candidate| { - candidate.observed_external_file_change = false; - candidate.has_inferred_edit = true; - }, - ); - if updated { - self.infer_inferred_edit_candidate_from_baseline( - buffer, - baseline_snapshot, - existed_on_disk, - cx, - ); - } - } - Action::RemoveAndWillDeleteBuffer => { - self.remove_inferred_edit_candidate_if_matching(&tool_call_id, &abs_path, nonce); - self.action_log.update(cx, |action_log, cx| { - action_log.will_delete_buffer(buffer.clone(), cx); - }); - } - Action::RemoveAndInferDeleted => { - self.remove_inferred_edit_candidate_if_matching(&tool_call_id, &abs_path, nonce); - self.action_log.update(cx, |action_log, cx| { - action_log.infer_buffer_deleted_from_snapshot( - buffer.clone(), - baseline_snapshot, - cx, - ); - }); - } - } } fn finalize_inferred_edit_tool_call( @@ -2267,7 +2024,7 @@ impl AcpThread { && Self::is_inferred_edit_terminal_status(&tool_call.status) }); if !should_finalize { - self.clear_inferred_edit_candidates_for_tool_calls([tool_call_id]); + self.clear_inferred_edit_candidates_for_tool_calls([tool_call_id], cx); return; } @@ -2284,42 +2041,34 @@ impl AcpThread { const ATTEMPT_DELAY: Duration = Duration::from_millis(50); for attempt in 0..MAX_ATTEMPTS { - let (buffers_to_reload, has_pending, has_observed_external_file_change) = this + let (ready_buffers, has_pending) = this .read_with(cx, |this, _| { let Some(candidates) = this.inferred_edit_candidates.get(&tool_call_id) else { - return (Vec::new(), false, false); + return (Vec::new(), false); }; - let mut buffers_to_reload = HashSet::default(); + let mut ready_buffers = Vec::new(); let mut has_pending = false; - let mut has_observed_external_file_change = false; for candidate_state in candidates.values() { match candidate_state { InferredEditCandidateState::Pending { .. } => has_pending = true, InferredEditCandidateState::Ready(candidate) => { - if candidate.observed_external_file_change { - has_observed_external_file_change = true; - buffers_to_reload.insert(candidate.buffer.clone()); - } + ready_buffers.push(candidate.buffer.clone()); } } } - ( - buffers_to_reload.into_iter().collect::>(), - has_pending, - has_observed_external_file_change, - ) + (ready_buffers, has_pending) }) - .unwrap_or((Vec::new(), false, false)); + .unwrap_or((Vec::new(), false)); - if buffers_to_reload.is_empty() && !has_pending { + if ready_buffers.is_empty() && !has_pending { break; } - for buffer in buffers_to_reload { + for buffer in ready_buffers { let should_reload = buffer.read_with(cx, |buffer, _| !buffer.is_dirty()); if !should_reload { continue; @@ -2333,17 +2082,15 @@ impl AcpThread { reload.await.log_err(); } - if (!has_pending && !has_observed_external_file_change) - || attempt + 1 == MAX_ATTEMPTS - { + if !has_pending || attempt + 1 == MAX_ATTEMPTS { break; } cx.background_executor().timer(ATTEMPT_DELAY).await; } - this.update(cx, |this, _| { - this.clear_inferred_edit_candidates_for_tool_calls([tool_call_id.clone()]); + this.update(cx, |this, cx| { + this.clear_inferred_edit_candidates_for_tool_calls([tool_call_id.clone()], cx); }) .ok(); @@ -2366,7 +2113,7 @@ impl AcpThread { let locations = tool_call.locations.clone(); if !should_track { - self.clear_inferred_edit_candidates_for_tool_calls([tool_call_id]); + self.clear_inferred_edit_candidates_for_tool_calls([tool_call_id], cx); return; } @@ -2892,6 +2639,7 @@ impl AcpThread { let canceled_tool_call_ids = this.mark_pending_tools_as_canceled(); this.clear_inferred_edit_candidates_for_tool_calls( canceled_tool_call_ids, + cx, ); this.finalize_all_inferred_edit_tool_calls(cx); } @@ -2934,6 +2682,7 @@ impl AcpThread { .collect::>(); this.clear_inferred_edit_candidates_for_tool_calls( removed_tool_call_ids, + cx, ); this.entries.truncate(user_msg_ix); cx.emit(AcpThreadEvent::EntriesRemoved(range)); @@ -2973,7 +2722,7 @@ impl AcpThread { Self::flush_streaming_text(&mut self.streaming_text_buffer, cx); let canceled_tool_call_ids = self.mark_pending_tools_as_canceled(); - self.clear_inferred_edit_candidates_for_tool_calls(canceled_tool_call_ids); + self.clear_inferred_edit_candidates_for_tool_calls(canceled_tool_call_ids, cx); self.finalize_all_inferred_edit_tool_calls(cx); cx.background_spawn(turn.send_task) @@ -3066,7 +2815,7 @@ impl AcpThread { .collect::>(); let range = ix..this.entries.len(); - this.clear_inferred_edit_candidates_for_tool_calls(removed_tool_call_ids); + this.clear_inferred_edit_candidates_for_tool_calls(removed_tool_call_ids, cx); this.entries.truncate(ix); cx.emit(AcpThreadEvent::EntriesRemoved(range)); diff --git a/crates/action_log/src/action_log.rs b/crates/action_log/src/action_log.rs index 7e6ea415cc696bd40932f5a268aa9b698671973a..219598cfe0b58626ab72afb3a92dba1501ae1f7e 100644 --- a/crates/action_log/src/action_log.rs +++ b/crates/action_log/src/action_log.rs @@ -183,6 +183,8 @@ impl ActionLog { unreviewed_edits, snapshot: text_snapshot, status, + mode: TrackedBufferMode::Normal, + expected_external_edit: None, version: buffer.read(cx).version(), diff, diff_update: diff_update_tx, @@ -208,6 +210,10 @@ impl ActionLog { event: &BufferEvent, cx: &mut Context, ) { + if self.handle_expected_external_edit_event(buffer.clone(), event, cx) { + return; + } + match event { BufferEvent::Edited { .. } => { let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) else { @@ -268,6 +274,245 @@ impl ActionLog { } } + fn handle_expected_external_edit_event( + &mut self, + buffer: Entity, + event: &BufferEvent, + cx: &mut Context, + ) -> bool { + let Some((_, expected_external_edit)) = + self.tracked_buffers + .get(&buffer) + .and_then(|tracked_buffer| { + tracked_buffer + .expected_external_edit + .clone() + .map(|expected_external_edit| (tracked_buffer.mode, expected_external_edit)) + }) + else { + return false; + }; + + if expected_external_edit.is_disqualified { + return false; + } + + match event { + BufferEvent::Saved + if expected_external_edit.observed_external_file_change + && !expected_external_edit.has_attributed_change => + { + self.mark_expected_external_edit_disqualified(&buffer); + true + } + BufferEvent::Edited { is_local: true } => { + if expected_external_edit.pending_delete { + let (is_deleted, is_empty) = buffer.read_with(cx, |buffer, _| { + ( + buffer + .file() + .is_some_and(|file| file.disk_state().is_deleted()), + buffer.text().is_empty(), + ) + }); + + if is_deleted && is_empty { + self.apply_expected_external_delete_local(buffer, cx); + return true; + } + } + + expected_external_edit.observed_external_file_change + } + BufferEvent::FileHandleChanged => { + let (is_deleted, is_empty, is_dirty) = buffer.read_with(cx, |buffer, _| { + ( + buffer + .file() + .is_some_and(|file| file.disk_state().is_deleted()), + buffer.text().is_empty(), + buffer.is_dirty(), + ) + }); + + if let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) { + if let Some(expected_external_edit) = + tracked_buffer.expected_external_edit.as_mut() + { + if !is_dirty || is_deleted { + expected_external_edit.observed_external_file_change = true; + } + expected_external_edit.pending_delete = is_deleted; + } + } + + if is_deleted { + if is_empty { + self.apply_expected_external_delete_local(buffer, cx); + } else if self.linked_action_log.is_none() { + let buffer = buffer.clone(); + cx.defer(move |cx| { + buffer.update(cx, |buffer, cx| buffer.set_text("", cx)); + }); + } + } + + true + } + BufferEvent::Reloaded + if expected_external_edit.observed_external_file_change + && !expected_external_edit.pending_delete => + { + if self.expected_external_edit_has_meaningful_change(&buffer, cx) { + self.apply_expected_external_reload_local(buffer, cx); + } else if let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) { + if let Some(expected_external_edit) = + tracked_buffer.expected_external_edit.as_mut() + { + expected_external_edit.observed_external_file_change = false; + expected_external_edit.pending_delete = false; + } + } + + true + } + _ => false, + } + } + + fn mark_expected_external_edit_disqualified(&mut self, buffer: &Entity) { + let Some(tracked_buffer) = self.tracked_buffers.get_mut(buffer) else { + return; + }; + let Some(expected_external_edit) = tracked_buffer.expected_external_edit.as_mut() else { + return; + }; + + expected_external_edit.is_disqualified = true; + expected_external_edit.observed_external_file_change = false; + expected_external_edit.pending_delete = false; + } + + fn expected_external_edit_has_meaningful_change( + &self, + buffer: &Entity, + cx: &App, + ) -> bool { + let Some(tracked_buffer) = self.tracked_buffers.get(buffer) else { + return false; + }; + let Some(expected_external_edit) = tracked_buffer.expected_external_edit.as_ref() else { + return false; + }; + + let (current_snapshot, current_exists) = buffer.read_with(cx, |buffer, _| { + ( + buffer.text_snapshot(), + buffer.file().is_some_and(|file| file.disk_state().exists()), + ) + }); + + if !expected_external_edit.initial_exists_on_disk { + current_exists || current_snapshot.text() != tracked_buffer.snapshot.text() + } else { + !current_exists || current_snapshot.text() != tracked_buffer.snapshot.text() + } + } + + fn apply_expected_external_reload_local( + &mut self, + buffer: Entity, + cx: &mut Context, + ) { + let current_version = buffer.read(cx).version(); + let (record_file_read_time, initial_exists_on_disk) = self + .tracked_buffers + .get(&buffer) + .and_then(|tracked_buffer| { + tracked_buffer + .expected_external_edit + .as_ref() + .map(|expected_external_edit| { + ( + expected_external_edit.record_file_read_time_source_count > 0, + expected_external_edit.initial_exists_on_disk, + ) + }) + }) + .unwrap_or((false, true)); + + if record_file_read_time { + self.update_file_read_time(&buffer, cx); + } + + let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) else { + return; + }; + let Some(expected_external_edit) = tracked_buffer.expected_external_edit.as_mut() else { + return; + }; + + expected_external_edit.has_attributed_change = true; + expected_external_edit.observed_external_file_change = false; + expected_external_edit.pending_delete = false; + tracked_buffer.mode = TrackedBufferMode::Normal; + + if !initial_exists_on_disk { + let existing_file_content = if tracked_buffer.diff_base.len() == 0 { + None + } else { + Some(tracked_buffer.diff_base.clone()) + }; + tracked_buffer.status = TrackedBufferStatus::Created { + existing_file_content, + }; + } else { + tracked_buffer.status = TrackedBufferStatus::Modified; + } + + tracked_buffer.version = current_version; + tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx); + } + + fn apply_expected_external_delete_local( + &mut self, + buffer: Entity, + cx: &mut Context, + ) { + let current_version = buffer.read(cx).version(); + let remove_file_read_time = self + .tracked_buffers + .get(&buffer) + .and_then(|tracked_buffer| { + tracked_buffer + .expected_external_edit + .as_ref() + .map(|expected_external_edit| { + expected_external_edit.record_file_read_time_source_count > 0 + }) + }) + .unwrap_or(false); + + if remove_file_read_time { + self.remove_file_read_time(&buffer, cx); + } + + let Some(tracked_buffer) = self.tracked_buffers.get_mut(&buffer) else { + return; + }; + let Some(expected_external_edit) = tracked_buffer.expected_external_edit.as_mut() else { + return; + }; + + expected_external_edit.has_attributed_change = true; + expected_external_edit.observed_external_file_change = false; + expected_external_edit.pending_delete = false; + tracked_buffer.mode = TrackedBufferMode::Normal; + tracked_buffer.status = TrackedBufferStatus::Deleted; + tracked_buffer.version = current_version; + tracked_buffer.schedule_diff_update(ChangeAuthor::Agent, cx); + } + async fn maintain_diff( this: WeakEntity, buffer: Entity, @@ -644,6 +889,111 @@ impl ActionLog { .is_some_and(|tracked_buffer| tracked_buffer.has_edits(cx)) } + pub fn begin_expected_external_edit(&mut self, buffer: Entity, cx: &mut Context) { + self.begin_expected_external_edit_impl(buffer, true, cx); + } + + fn begin_expected_external_edit_impl( + &mut self, + buffer: Entity, + record_file_read_time: bool, + cx: &mut Context, + ) { + if let Some(linked_action_log) = &self.linked_action_log { + linked_action_log.update(cx, |log, cx| { + log.begin_expected_external_edit_impl(buffer.clone(), false, cx); + }); + } + + let initial_exists_on_disk = buffer + .read(cx) + .file() + .is_some_and(|file| file.disk_state().exists()); + let had_tracked_buffer = self.tracked_buffers.contains_key(&buffer); + let has_attributed_change = self + .tracked_buffers + .get(&buffer) + .is_some_and(|tracked_buffer| tracked_buffer.has_edits(cx)); + + let tracked_buffer = self.track_buffer_internal(buffer.clone(), false, cx); + if !had_tracked_buffer { + tracked_buffer.mode = TrackedBufferMode::ExpectationOnly; + tracked_buffer.status = TrackedBufferStatus::Modified; + tracked_buffer.diff_base = buffer.read(cx).as_rope().clone(); + tracked_buffer.snapshot = buffer.read(cx).text_snapshot(); + tracked_buffer.unreviewed_edits.clear(); + } + + let expected_external_edit = + tracked_buffer + .expected_external_edit + .get_or_insert_with(|| ExpectedExternalEdit { + active_source_count: 0, + record_file_read_time_source_count: 0, + initial_exists_on_disk, + observed_external_file_change: false, + has_attributed_change, + pending_delete: false, + is_disqualified: false, + }); + expected_external_edit.active_source_count += 1; + if record_file_read_time { + expected_external_edit.record_file_read_time_source_count += 1; + } + } + + pub fn end_expected_external_edit(&mut self, buffer: Entity, cx: &mut Context) { + self.end_expected_external_edit_impl(buffer, true, cx); + } + + fn end_expected_external_edit_impl( + &mut self, + buffer: Entity, + record_file_read_time: bool, + cx: &mut Context, + ) { + if let Some(linked_action_log) = &self.linked_action_log { + linked_action_log.update(cx, |log, cx| { + log.end_expected_external_edit_impl(buffer.clone(), false, cx); + }); + } + + let remove_tracked_buffer = if let Some(tracked_buffer) = + self.tracked_buffers.get_mut(&buffer) + { + let Some(expected_external_edit) = tracked_buffer.expected_external_edit.as_mut() + else { + return; + }; + + expected_external_edit.active_source_count = + expected_external_edit.active_source_count.saturating_sub(1); + if record_file_read_time { + expected_external_edit.record_file_read_time_source_count = expected_external_edit + .record_file_read_time_source_count + .saturating_sub(1); + } + + if expected_external_edit.active_source_count > 0 { + false + } else { + let remove_tracked_buffer = tracked_buffer.mode + == TrackedBufferMode::ExpectationOnly + && !expected_external_edit.has_attributed_change; + tracked_buffer.expected_external_edit = None; + tracked_buffer.mode = TrackedBufferMode::Normal; + remove_tracked_buffer + } + } else { + false + }; + + if remove_tracked_buffer { + self.tracked_buffers.remove(&buffer); + cx.notify(); + } + } + pub fn infer_buffer_created( &mut self, buffer: Entity, @@ -1223,7 +1573,8 @@ impl ActionLog { .filter(|(buffer, tracked)| { let buffer = buffer.read(cx); - tracked.version != buffer.version + tracked.mode == TrackedBufferMode::Normal + && tracked.version != buffer.version && buffer .file() .is_some_and(|file| !file.disk_state().is_deleted()) @@ -1448,6 +1799,23 @@ enum ChangeAuthor { Agent, } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum TrackedBufferMode { + Normal, + ExpectationOnly, +} + +#[derive(Clone, Debug)] +struct ExpectedExternalEdit { + active_source_count: usize, + record_file_read_time_source_count: usize, + initial_exists_on_disk: bool, + observed_external_file_change: bool, + has_attributed_change: bool, + pending_delete: bool, + is_disqualified: bool, +} + #[derive(Debug)] enum TrackedBufferStatus { Created { existing_file_content: Option }, @@ -1460,6 +1828,8 @@ pub struct TrackedBuffer { diff_base: Rope, unreviewed_edits: Patch, status: TrackedBufferStatus, + mode: TrackedBufferMode, + expected_external_edit: Option, version: clock::Global, diff: Entity, snapshot: text::BufferSnapshot, @@ -3774,6 +4144,219 @@ mod tests { ); } + #[gpui::test] + async fn test_expected_external_edit_does_not_mark_read_time_or_stale_before_first_agent_change( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({"file": "one\ntwo\n"})) + .await; + let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; + let action_log = cx.new(|_| ActionLog::new(project.clone())); + + let file_path = project + .read_with(cx, |project, cx| project.find_project_path("dir/file", cx)) + .unwrap(); + let buffer = project + .update(cx, |project, cx| project.open_buffer(file_path, cx)) + .await + .unwrap(); + let abs_path = PathBuf::from(path!("/dir/file")); + + cx.update(|cx| { + action_log.update(cx, |log, cx| { + log.begin_expected_external_edit(buffer.clone(), cx); + }); + }); + + assert!( + action_log.read_with(cx, |log, _| log.file_read_time(&abs_path).is_none()), + "expected external edit should not record file_read_time before an agent change" + ); + assert!( + action_log.read_with(cx, |log, cx| log.stale_buffers(cx).next().is_none()), + "expected external edit should not mark a synthetic tracker as stale" + ); + + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..0, "zero\n")], None, cx).unwrap(); + }); + cx.run_until_parked(); + + assert!( + action_log.read_with(cx, |log, cx| log.changed_buffers(cx).is_empty()), + "local edits before the first external change should not become review hunks" + ); + assert!( + action_log.read_with(cx, |log, cx| log.stale_buffers(cx).next().is_none()), + "expectation-only tracking should stay out of stale_buffers" + ); + assert!( + action_log.read_with(cx, |log, _| log.file_read_time(&abs_path).is_none()), + "local edits before the first external change should not record file_read_time" + ); + + cx.update(|cx| { + action_log.update(cx, |log, cx| { + log.end_expected_external_edit(buffer.clone(), cx); + }); + }); + cx.run_until_parked(); + + assert!( + action_log.read_with(cx, |log, cx| log.changed_buffers(cx).is_empty()), + "ending an expectation without an agent change should remove synthetic tracking" + ); + } + + #[gpui::test] + async fn test_expected_external_edit_preserves_local_edits_before_first_agent_change( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({"file": "one\ntwo\n"})) + .await; + let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; + let action_log = cx.new(|_| ActionLog::new(project.clone())); + + let file_path = project + .read_with(cx, |project, cx| project.find_project_path("dir/file", cx)) + .unwrap(); + let buffer = project + .update(cx, |project, cx| project.open_buffer(file_path, cx)) + .await + .unwrap(); + + cx.update(|cx| { + action_log.update(cx, |log, cx| { + log.begin_expected_external_edit(buffer.clone(), cx); + }); + }); + + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..0, "zero\n")], None, cx).unwrap(); + }); + project + .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)) + .await + .unwrap(); + cx.run_until_parked(); + + fs.save( + path!("/dir/file").as_ref(), + &"zero\none\ntwo\nthree\n".into(), + Default::default(), + ) + .await + .unwrap(); + cx.run_until_parked(); + + assert_eq!( + action_log.read_with(cx, |log, cx| log.changed_buffers(cx).len()), + 1, + "the first external change should be attributed relative to the local user baseline" + ); + + cx.update(|cx| { + action_log.update(cx, |log, cx| { + log.end_expected_external_edit(buffer.clone(), cx); + }); + }); + cx.run_until_parked(); + + action_log + .update(cx, |log, cx| log.reject_all_edits(None, cx)) + .await; + cx.run_until_parked(); + + assert_eq!( + buffer.read_with(cx, |buffer, _| buffer.text()), + "zero\none\ntwo\n" + ); + assert_eq!( + String::from_utf8(fs.read_file_sync(path!("/dir/file")).unwrap()).unwrap(), + "zero\none\ntwo\n" + ); + } + + #[gpui::test] + async fn test_linked_expected_external_edit_tracks_review_without_parent_file_read_time( + cx: &mut TestAppContext, + ) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({"file": "one\ntwo\n"})) + .await; + let project = Project::test(fs.clone(), [path!("/dir").as_ref()], cx).await; + let parent_log = cx.new(|_| ActionLog::new(project.clone())); + let child_log = + cx.new(|_| ActionLog::new(project.clone()).with_linked_action_log(parent_log.clone())); + + let file_path = project + .read_with(cx, |project, cx| project.find_project_path("dir/file", cx)) + .unwrap(); + let buffer = project + .update(cx, |project, cx| project.open_buffer(file_path, cx)) + .await + .unwrap(); + let abs_path = PathBuf::from(path!("/dir/file")); + + cx.update(|cx| { + child_log.update(cx, |log, cx| { + log.begin_expected_external_edit(buffer.clone(), cx); + }); + }); + + assert!( + child_log.read_with(cx, |log, _| log.file_read_time(&abs_path).is_none()), + "child should not record file_read_time until the first external agent change" + ); + assert!( + parent_log.read_with(cx, |log, _| log.file_read_time(&abs_path).is_none()), + "parent should not inherit file_read_time from the child's pending expectation" + ); + + fs.save( + path!("/dir/file").as_ref(), + &"one\ntwo\nthree\n".into(), + Default::default(), + ) + .await + .unwrap(); + cx.run_until_parked(); + + cx.update(|cx| { + child_log.update(cx, |log, cx| { + log.end_expected_external_edit(buffer.clone(), cx); + }); + }); + cx.run_until_parked(); + + let child_hunks = unreviewed_hunks(&child_log, cx); + assert!( + !child_hunks.is_empty(), + "child should track the expected external edit" + ); + assert_eq!( + unreviewed_hunks(&parent_log, cx), + child_hunks, + "parent should also track the expected external edit" + ); + assert!( + child_log.read_with(cx, |log, _| log.file_read_time(&abs_path).is_some()), + "child should record file_read_time once the expected external edit is attributed" + ); + assert!( + parent_log.read_with(cx, |log, _| log.file_read_time(&abs_path).is_none()), + "parent should still not inherit file_read_time from the child's expected external edit" + ); + } + #[gpui::test] async fn test_infer_buffer_edited_from_snapshot(cx: &mut TestAppContext) { init_test(cx);