fix(notifications): keep BECNotificationBroker singleton alive app-wide and prune expired replay entries

bec_widgets/widgets/containers/main_window/addons/notification_center/notification_banner.py

@@ -18,6 +18,7 @@ from typing import Literal
 from uuid import uuid4
 
 import pyqtgraph as pg
+import shiboken6
 from bec_lib.alarm_handler import Alarms  # external enum
 from bec_lib.endpoints import MessageEndpoints
 from bec_lib.logger import bec_logger
@@ -734,6 +735,15 @@ class NotificationCentre(QScrollArea):
         toast.notification_id = notification_id
         broker = BECNotificationBroker()
         toast.closed.connect(lambda nid=notification_id: broker.notification_closed.emit(nid))
+        # Once a toast auto-expires it is no longer live, so drop it from the broker's
+        # replay store. Wired here so BOTH creation paths are covered: live posts and
+        # toasts recreated by _replay_active_notifications — otherwise a replayed toast
+        # that expires would leave the entry behind and every future NotificationCentre
+        # would replay it again. MAJOR alarms use lifetime_ms=0 and never emit
+        # 'expired', so they correctly stay in history until explicitly closed.
+        toast.expired.connect(
+            lambda nid=notification_id: broker._active_notifications.pop(nid, None)
+        )
         toast.closed.connect(lambda: self._hide_notification(toast))
         toast.expired.connect(lambda t=toast: self._handle_expire(t))
         toast.expanded.connect(self._adjust_height)
@@ -1029,13 +1039,26 @@ class BECNotificationBroker(BECConnector, QObject):
     notification_closed = QtCore.Signal(str)
 
     def __new__(cls, *args, **kwargs):
+        # If a previous instance's C++ object was destroyed (e.g. deleted together with a
+        # parent window) the cached Python wrapper is stale: rebuild instead of handing back
+        # a dead broker whose signals raise RuntimeError and whose subscriptions are gone.
+        if cls._instance is not None and not shiboken6.isValid(cls._instance):
+            cls._instance = None
+            cls._initialized = False
         if cls._instance is None:
             cls._instance = super().__new__(cls)
         return cls._instance
 
     def __init__(self, parent=None, gui_id: str = None, client=None, **kwargs):
-        if self._initialized:
+        # Re-run init only for a genuinely fresh (or revived) instance. A live, already
+        # initialized singleton short-circuits; a stale wrapper never reaches here because
+        # __new__ rebuilds it and resets _initialized.
+        if self._initialized and shiboken6.isValid(self):
             return
+        # The broker is an application-wide singleton and must outlive every window. Ignore
+        # the caller-supplied parent (which may be a WA_DeleteOnClose window) and anchor it
+        # to the QApplication so closing a window never destroys the broker.
+        parent = QApplication.instance()
         super().__init__(parent=parent, gui_id=gui_id, client=client, **kwargs)
         self._err_util = self.error_utility
         # listen to incoming alarms and scan status
@@ -1125,7 +1148,8 @@ class BECNotificationBroker(BECConnector, QObject):
                 lifetime_ms=lifetime,
                 notification_id=notification_id,
             )
-            # broadcast close events (expiry is handled locally to keep history)
+            # broadcast close events (expiry pruning is wired in add_notification so
+            # replayed toasts are covered as well)
             toast.closed.connect(lambda nid=notification_id: self.notification_closed.emit(nid))
 
     @SafeSlot(dict, dict)
@@ -1167,9 +1191,21 @@ class BECNotificationBroker(BECConnector, QObject):
     def reset_singleton(cls):
         """
         Reset the singleton instance of the BECNotificationBroker.
+
+        Because the broker is now parented to the QApplication it no longer dies with a
+        window, so resetting the class-level references alone would leak the live QObject
+        (and its dispatcher subscriptions) into the next incarnation. Tear the existing
+        instance down first: disconnect its slots and delete its C++ object.
         """
+        inst = cls._instance
         cls._instance = None
         cls._initialized = False
+        if inst is not None and shiboken6.isValid(inst):
+            try:
+                inst.cleanup()
+            except Exception as exc:  # pragma: no cover - defensive teardown
+                logger.warning(f"Error during BECNotificationBroker cleanup on reset: {exc}")
+            shiboken6.delete(inst)
 
     def cleanup(self):
         """Disconnect from the notification signal."""

tests/unit_tests/test_notifications.py

@@ -364,3 +364,94 @@ def test_broker_posts_notification(qtbot, centre, mocked_client):
     assert "Error occurred. See details." in toast.body
     assert toast.kind == SeverityKind.MAJOR
     assert toast._lifetime == 0
+
+
+def test_broker_survives_parent_window_destruction(qtbot, mocked_client):
+    """The broker is an app-wide singleton; closing the window that first created it must
+    not destroy it, or every later window gets a dead wrapper and notifications stop."""
+    import shiboken6
+    from qtpy.QtCore import QEvent, QEventLoop, Qt
+    from qtpy.QtWidgets import QApplication, QMainWindow
+
+    w1 = QMainWindow()
+    w1.setAttribute(Qt.WidgetAttribute.WA_DeleteOnClose, True)
+    qtbot.addWidget(w1)
+    b1 = BECNotificationBroker(parent=w1, client=mocked_client)
+    assert b1.parent() is QApplication.instance()
+
+    w1.close()
+    qapp = QApplication.instance()
+    qapp.sendPostedEvents(None, QEvent.DeferredDelete)
+    qapp.processEvents(QEventLoop.AllEvents)
+
+    assert shiboken6.isValid(b1)  # broker outlived the window
+    b2 = BECNotificationBroker(parent=None, client=mocked_client)
+    assert b2 is b1 and shiboken6.isValid(b2)
+    b2.notification_closed.emit("x")  # must not raise RuntimeError
+    BECNotificationBroker.reset_singleton()
+
+
+def test_broker_revives_after_hard_destroy(qtbot, mocked_client):
+    """If the broker's C++ object is destroyed anyway, the next construction rebuilds a
+    fresh, re-subscribed instance instead of returning the dead wrapper."""
+    import shiboken6
+
+    b1 = BECNotificationBroker(parent=None, client=mocked_client)
+    shiboken6.delete(b1)
+    assert not shiboken6.isValid(b1)
+
+    b2 = BECNotificationBroker(parent=None, client=mocked_client)
+    assert b2 is not b1 and shiboken6.isValid(b2)
+    b2.notification_closed.emit("y")  # re-subscribed, no RuntimeError
+    BECNotificationBroker.reset_singleton()
+
+
+def test_active_notifications_dropped_on_expiry(qtbot, centre, mocked_client):
+    """Auto-expiring (non-MAJOR) notifications must not linger in the replay store; MAJOR
+    alarms (lifetime 0, never expire) stay in history until explicitly closed."""
+    qtbot.wait(20)  # let the centre's replay singleShot fire on an EMPTY store first
+    broker = BECNotificationBroker(client=mocked_client)
+    broker._err_util = ErrorPopupUtility()
+
+    broker.post_notification({"alarm_type": "W", "msg": "m", "severity": 0}, meta={})
+    qtbot.wait(50)
+    assert len(broker._active_notifications) == 1
+    nid = next(iter(broker._active_notifications))
+    assert len(centre.toasts) == 1  # no replay duplicate
+    centre.toasts[0].expired.emit()  # simulate auto-expiry
+    qtbot.wait(10)
+    assert nid not in broker._active_notifications
+
+    broker.post_notification({"alarm_type": "E", "msg": "m", "severity": 2}, meta={})
+    qtbot.wait(50)
+    major_nid = next(iter(broker._active_notifications))
+    assert broker._active_notifications[major_nid]["lifetime_ms"] == 0
+    BECNotificationBroker.reset_singleton()
+
+
+def test_replayed_toast_expiry_prunes_replay_store(qtbot, mocked_client):
+    """A toast recreated by a NEW centre's replay must also prune the broker's replay
+    store on expiry — not only toasts created live by post_notification."""
+    broker = BECNotificationBroker(client=mocked_client)
+    broker._err_util = ErrorPopupUtility()
+
+    # post with no centre open: the entry is stored for future centres
+    broker.post_notification({"alarm_type": "W", "msg": "m", "severity": 0}, meta={})
+    assert len(broker._active_notifications) == 1
+    nid = next(iter(broker._active_notifications))
+
+    # a new centre replays the stored notification
+    parent = QtWidgets.QWidget()
+    parent.resize(600, 400)
+    ctr = NotificationCentre(parent=parent, fixed_width=300, margin=8)
+    layout = QtWidgets.QVBoxLayout(parent)
+    layout.addWidget(ctr)
+    qtbot.addWidget(parent)
+    qtbot.waitUntil(lambda: len(ctr.toasts) == 1, timeout=2000)
+
+    # expiring the REPLAYED toast must remove the entry from the broker store
+    ctr.toasts[0].expired.emit()
+    qtbot.wait(10)
+    assert nid not in broker._active_notifications
+
+    BECNotificationBroker.reset_singleton()