From 552d771e15bcdbfc81efb07493b10916f5cf93eb Mon Sep 17 00:00:00 2001 From: Breadway Date: Thu, 23 Jul 2026 10:12:52 +0800 Subject: [PATCH] fix: honor x-canonical-private-synchronous hint for notification replacement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit notify-send-based senders (e.g. breadcrumbs) fire a new process per notification, so replaces_id is always 0 — they instead tag related notifications with the x-canonical-private-synchronous hint and expect the daemon to replace whatever's currently showing from that app. We weren't honoring it, so a persistent (Expire::Never) critical notification like breadcrumbs' "no Wi-Fi adapter" during a suspend transition could never be superseded by a later notification, leaving it stuck until breadbar was restarted. Fixes #2. --- src/notifications/mod.rs | 130 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/src/notifications/mod.rs b/src/notifications/mod.rs index fbb29df..b1f1760 100644 --- a/src/notifications/mod.rs +++ b/src/notifications/mod.rs @@ -1,10 +1,21 @@ pub mod popup; +use std::collections::HashMap; use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::Mutex; use std::time::Duration; use tokio::sync::mpsc; use zbus::zvariant::OwnedValue; +/// Hint key used by `notify-send` and honored by notify-osd/dunst: senders +/// that fire off a new process per notification (so `replaces_id` is always +/// 0) tag related notifications with the same `(app_name, tag)` pair to mean +/// "replace whatever from this app is already showing." Without honoring +/// this, a fire-and-forget sender can never supersede an earlier +/// `Expire::Never` notification from itself (e.g. a critical hardware +/// warning) — it just piles up a new card next to it forever. +const SYNCHRONOUS_HINT: &str = "x-canonical-private-synchronous"; + /// How long a shown notification should stay up before auto-dismissing. /// Distinct from `Option` mainly for readability at call sites — /// `Never` covers both the spec's `expire_timeout == 0` ("never expire") @@ -79,6 +90,9 @@ fn compute_expire(expire_timeout: i32, urgency_critical: bool) -> Expire { struct NotifServer { tx: mpsc::Sender, next_id: AtomicU32, + /// (app_name, synchronous-hint tag) -> id, for senders relying on + /// `SYNCHRONOUS_HINT` instead of an explicit `replaces_id`. + sync_tags: Mutex>, } #[zbus::interface(name = "org.freedesktop.Notifications")] @@ -96,8 +110,24 @@ impl NotifServer { hints: std::collections::HashMap, expire_timeout: i32, ) -> u32 { + let sync_tag = hints + .get(SYNCHRONOUS_HINT) + .and_then(|v| String::try_from(v.clone()).ok()); + let id = if replaces_id != 0 { + if let Some(tag) = &sync_tag { + self.sync_tags + .lock() + .unwrap() + .insert((app_name.to_string(), tag.clone()), replaces_id); + } replaces_id + } else if let Some(tag) = &sync_tag { + let key = (app_name.to_string(), tag.clone()); + let mut sync_tags = self.sync_tags.lock().unwrap(); + *sync_tags + .entry(key) + .or_insert_with(|| self.next_id.fetch_add(1, Ordering::Relaxed)) } else { self.next_id.fetch_add(1, Ordering::Relaxed) }; @@ -150,6 +180,7 @@ pub fn spawn() { let server = NotifServer { tx, next_id: AtomicU32::new(1), + sync_tags: Mutex::new(HashMap::new()), }; // Builder failures here would only occur with invalid static strings — safe to unwrap. let conn = zbus::connection::Builder::session() @@ -212,4 +243,103 @@ mod tests { Expire::Never => panic!("expected 1500ms, got Never"), } } + + fn test_server() -> (NotifServer, mpsc::Receiver) { + let (tx, rx) = mpsc::channel(32); + ( + NotifServer { + tx, + next_id: AtomicU32::new(1), + sync_tags: Mutex::new(HashMap::new()), + }, + rx, + ) + } + + fn sync_hints(tag: &str) -> HashMap { + let mut hints = HashMap::new(); + hints.insert( + SYNCHRONOUS_HINT.to_string(), + OwnedValue::try_from(zbus::zvariant::Value::from(tag)).unwrap(), + ); + hints + } + + #[tokio::test] + async fn synchronous_hint_reuses_id_for_same_app_and_tag() { + let (server, _rx) = test_server(); + let first = server + .notify( + "breadcrumbs", + 0, + "", + "no Wi-Fi adapter", + "", + vec![], + sync_hints("breadcrumbs"), + -1, + ) + .await; + let second = server + .notify( + "breadcrumbs", + 0, + "", + "back online", + "", + vec![], + sync_hints("breadcrumbs"), + -1, + ) + .await; + assert_eq!( + first, second, + "same app+tag should replace, not stack, a prior notification" + ); + } + + #[tokio::test] + async fn synchronous_hint_is_scoped_per_app_name() { + let (server, _rx) = test_server(); + let first = server + .notify( + "breadcrumbs", + 0, + "", + "no Wi-Fi adapter", + "", + vec![], + sync_hints("breadcrumbs"), + -1, + ) + .await; + let second = server + .notify( + "other-app", + 0, + "", + "unrelated", + "", + vec![], + sync_hints("breadcrumbs"), + -1, + ) + .await; + assert_ne!( + first, second, + "same tag from a different app must not collide" + ); + } + + #[tokio::test] + async fn no_synchronous_hint_always_allocates_a_new_id() { + let (server, _rx) = test_server(); + let first = server + .notify("breadcrumbs", 0, "", "one", "", vec![], HashMap::new(), -1) + .await; + let second = server + .notify("breadcrumbs", 0, "", "two", "", vec![], HashMap::new(), -1) + .await; + assert_ne!(first, second); + } } -- 2.49.1