git_ui: Don't display the merge conflict notification if an agent is running (#51498)

Danilo Leal and Lukas Wirth created 3 weeks ago

This PR is motivated by internal feedback in which the notification that
we show inviting to resolve merging conflicts with an agent also pops up
if the agent itself ran `git merge`. In this case, the notification is
unnecessary noise. So, what I'm doing here is simply _not_ showing it if
there's a running agent.

I want to note that this change is accepting a trade-off here, in which
there could be cases that even if an agent is running, the notification
can still be useful. There could be other ways to identify whether the
agent is running `git merge`, but they all felt a bit too complex for
the moment. And given this is reasonably an edge case, I'm favoring a
simple approach for now.

Release Notes:

- N/A

---------

Co-authored-by: Lukas Wirth <[e-mail]>

Change summary

crates/agent_ui/src/connection_view/thread_view.rs | 26 ++++++++++++++-
crates/git_ui/src/conflict_view.rs                 | 17 +++------
crates/workspace/src/notifications.rs              |  8 ++++
crates/workspace/src/workspace.rs                  |  6 +++
4 files changed, 44 insertions(+), 13 deletions(-)

Detailed changes

crates/agent_ui/src/connection_view/thread_view.rs

@@ -739,10 +739,13 @@ impl ThreadView {
                 }
             }
         }));
+        if self.parent_id.is_none() {
+            self.suppress_merge_conflict_notification(cx);
+        }
         generation
     }
 
-    pub fn stop_turn(&mut self, generation: usize) {
+    pub fn stop_turn(&mut self, generation: usize, cx: &mut Context<Self>) {
         if self.turn_fields.turn_generation != generation {
             return;
         }
@@ -753,6 +756,25 @@ impl ThreadView {
             .map(|started| started.elapsed());
         self.turn_fields.last_turn_tokens = self.turn_fields.turn_tokens.take();
         self.turn_fields._turn_timer_task = None;
+        if self.parent_id.is_none() {
+            self.unsuppress_merge_conflict_notification(cx);
+        }
+    }
+
+    fn suppress_merge_conflict_notification(&self, cx: &mut Context<Self>) {
+        self.workspace
+            .update(cx, |workspace, cx| {
+                workspace.suppress_notification(&workspace::merge_conflict_notification_id(), cx);
+            })
+            .ok();
+    }
+
+    fn unsuppress_merge_conflict_notification(&self, cx: &mut Context<Self>) {
+        self.workspace
+            .update(cx, |workspace, _cx| {
+                workspace.unsuppress(workspace::merge_conflict_notification_id());
+            })
+            .ok();
     }
 
     pub fn update_turn_tokens(&mut self, cx: &App) {
@@ -962,7 +984,7 @@ impl ThreadView {
                 let mut cx = cx.clone();
                 move || {
                     this.update(&mut cx, |this, cx| {
-                        this.stop_turn(generation);
+                        this.stop_turn(generation, cx);
                         cx.notify();
                     })
                     .ok();

crates/git_ui/src/conflict_view.rs

@@ -18,10 +18,7 @@ use settings::Settings;
 use std::{cell::RefCell, ops::Range, rc::Rc, sync::Arc};
 use ui::{ActiveTheme, Divider, Element as _, Styled, Window, prelude::*};
 use util::{ResultExt as _, debug_panic, maybe};
-use workspace::{
-    Workspace,
-    notifications::{NotificationId, simple_message_notification::MessageNotification},
-};
+use workspace::{Workspace, notifications::simple_message_notification::MessageNotification};
 use zed_actions::agent::{
     ConflictContent, ResolveConflictedFilesWithAgent, ResolveConflictsWithAgent,
 };
@@ -500,12 +497,6 @@ fn render_conflict_buttons(
         .into_any()
 }
 
-struct MergeConflictNotification;
-
-fn merge_conflict_notification_id() -> NotificationId {
-    NotificationId::unique::<MergeConflictNotification>()
-}
-
 fn collect_conflicted_file_paths(workspace: &Workspace, cx: &App) -> Vec<String> {
     let project = workspace.project().read(cx);
     let git_store = project.git_store().read(cx);
@@ -547,8 +538,12 @@ pub(crate) fn register_conflict_notification(
             return;
         }
 
+        if workspace.is_notification_suppressed(workspace::merge_conflict_notification_id()) {
+            return;
+        }
+
         let paths = collect_conflicted_file_paths(workspace, cx);
-        let notification_id = merge_conflict_notification_id();
+        let notification_id = workspace::merge_conflict_notification_id();
         let current_paths_set: HashSet<String> = paths.iter().cloned().collect();
 
         if paths.is_empty() {

crates/workspace/src/notifications.rs

@@ -234,6 +234,14 @@ impl Workspace {
         self.suppressed_notifications.insert(id.clone());
     }
 
+    pub fn is_notification_suppressed(&self, notification_id: NotificationId) -> bool {
+        self.suppressed_notifications.contains(&notification_id)
+    }
+
+    pub fn unsuppress(&mut self, notification_id: NotificationId) {
+        self.suppressed_notifications.remove(&notification_id);
+    }
+
     pub fn show_initial_notifications(&mut self, cx: &mut Context<Self>) {
         // Allow absence of the global so that tests don't need to initialize it.
         let app_notifications = GLOBAL_APP_NOTIFICATIONS

crates/workspace/src/workspace.rs

@@ -7268,6 +7268,12 @@ impl GlobalAnyActiveCall {
         cx.global()
     }
 }
+
+pub fn merge_conflict_notification_id() -> NotificationId {
+    struct MergeConflictNotification;
+    NotificationId::unique::<MergeConflictNotification>()
+}
+
 /// Workspace-local view of a remote participant's location.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum ParticipantLocation {