action_log: Fix race condition when committing changes (#53884)
Sometimes the action log would not auto-accept agent edits when commiting. Gpt-5.4 identified this race condition: This fixes a race where `keep_committed_edits` could run after `head_commit` changed but before the new git base text had been applied, leaving committed agent edits marked as unreviewed; `ActionLog` now waits for an explicit `BufferDiffEvent::BaseTextChanged` instead of inferring readiness from generic `DiffChanged` activity, so it only accepts edits after the diff base itself is actually updated. - `ReloadGitState` updates `head_commit` before `ReloadBufferDiffBases` finishes loading and applying the new HEAD text. - In that gap, an unrelated `DiffChanged` can fire from a normal diff recalculation. - The old logic treated that event as the commit signal and ran `keep_committed_edits` too early. - `keep_committed_edits` then read stale diff base text, so it failed to match the committed agent edits. - When the real base-text update arrived later, the HEAD had already been overwritten (`old_head`), and the edits stayed unreviewed. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - Fixed an issue where committing agent written code would sometimes not mark edits as accepted
This commit is contained in:
1 parent
d218335735
commit
2cab249659
4 files changed
+140
-32
No files matched your search
@@ -1515,11 +1515,17 @@ pub struct DiffChanged {
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub enum BufferDiffEvent {
|
||||
BaseTextChanged,
|
||||
DiffChanged(DiffChanged),
|
||||
LanguageChanged,
|
||||
HunksStagedOrUnstaged(Option<Rope>),
|
||||
}
|
||||
|
||||
struct SetSnapshotResult {
|
||||
change: DiffChanged,
|
||||
base_text_changed: bool,
|
||||
}
|
||||
|
||||
impl EventEmitter<BufferDiffEvent> for BufferDiff {}
|
||||
|
||||
impl BufferDiff {
|
||||
@@ -1784,7 +1790,7 @@ impl BufferDiff {
|
||||
secondary_diff_change: Option<Range<Anchor>>,
|
||||
clear_pending_hunks: bool,
|
||||
cx: &mut Context<Self>,
|
||||
) -> impl Future<Output = DiffChanged> + use<> {
|
||||
) -> impl Future<Output = SetSnapshotResult> + use<> {
|
||||
log::debug!("set snapshot with secondary {secondary_diff_change:?}");
|
||||
|
||||
let old_snapshot = self.snapshot(cx);
|
||||
@@ -1904,10 +1910,13 @@ impl BufferDiff {
|
||||
if let Some(parsing_idle) = parsing_idle {
|
||||
parsing_idle.await;
|
||||
}
|
||||
DiffChanged {
|
||||
changed_range,
|
||||
base_text_changed_range,
|
||||
extended_range,
|
||||
SetSnapshotResult {
|
||||
change: DiffChanged {
|
||||
changed_range,
|
||||
base_text_changed_range,
|
||||
extended_range,
|
||||
},
|
||||
base_text_changed,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1938,12 +1947,15 @@ impl BufferDiff {
|
||||
);
|
||||
|
||||
cx.spawn(async move |this, cx| {
|
||||
let change = fut.await;
|
||||
let result = fut.await;
|
||||
this.update(cx, |_, cx| {
|
||||
cx.emit(BufferDiffEvent::DiffChanged(change.clone()));
|
||||
if result.base_text_changed {
|
||||
cx.emit(BufferDiffEvent::BaseTextChanged);
|
||||
}
|
||||
cx.emit(BufferDiffEvent::DiffChanged(result.change.clone()));
|
||||
})
|
||||
.ok();
|
||||
change.changed_range
|
||||
result.change.changed_range
|
||||
})
|
||||
}
|
||||
|
||||
@@ -2019,8 +2031,11 @@ impl BufferDiff {
|
||||
let fg_executor = cx.foreground_executor().clone();
|
||||
let snapshot = fg_executor.block_on(fut);
|
||||
let fut = self.set_snapshot_with_secondary_inner(snapshot, buffer, None, false, cx);
|
||||
let change = fg_executor.block_on(fut);
|
||||
cx.emit(BufferDiffEvent::DiffChanged(change));
|
||||
let result = fg_executor.block_on(fut);
|
||||
if result.base_text_changed {
|
||||
cx.emit(BufferDiffEvent::BaseTextChanged);
|
||||
}
|
||||
cx.emit(BufferDiffEvent::DiffChanged(result.change));
|
||||
}
|
||||
|
||||
pub fn base_text_buffer(&self) -> &Entity<language::Buffer> {
|
||||
|
||||
Reference in new issue
Block a user