fix(shutdown): correct client/dispatcher teardown ordering and run cleanup on all close paths

bec_widgets/applications/companion_app.py

@@ -198,8 +198,14 @@ class GUIServer:
 
         def stop_dispatcher():
             if self.dispatcher:
-                self.dispatcher.stop_cli_server()
+                # disconnect_all() must run BEFORE stop_cli_server(). stop_cli_server()
+                # -> RPCServer.shutdown() historically tore the client down, which nulls
+                # the connector's listener thread; a later disconnect_all() would then
+                # early-return in ManagedRedisConnection.unregister and leave subscriptions
+                # alive. This ordering matches BECConnector.terminate (disconnect_all ->
+                # stop_cli_server).
                 self.dispatcher.disconnect_all()
+                self.dispatcher.stop_cli_server()
 
         self._run_shutdown_step("close_launcher_window", close_launcher_window)
         self._run_shutdown_step("stop_pylsp_server", stop_pylsp_server)

bec_widgets/applications/launch_window.py

@@ -741,6 +741,10 @@ class LaunchWindow(BECMainWindow):
             return
 
         event.accept()
+        # Chain to the base implementation so BECWidget.cleanup() runs on the normal
+        # close path (not only via WA_DeleteOnClose / DeferredDelete). Keeps this method
+        # a correct closeEvent template.
+        super().closeEvent(event)
 
 
 if __name__ == "__main__":  # pragma: no cover

bec_widgets/utils/bec_connector.py

@@ -155,7 +155,14 @@ class BECConnector:
                 client.shutdown()
 
             BECConnector.EXIT_HANDLERS[self.client] = terminate
-            QApplication.instance().aboutToQuit.connect(terminate)
+            app = QApplication.instance()
+            if app is not None:
+                app.aboutToQuit.connect(terminate)
+            else:
+                logger.warning(
+                    "No QApplication instance available; skipping aboutToQuit "
+                    "registration for BEC client teardown handler."
+                )
 
         if config:
             self.config = config

bec_widgets/utils/bec_dispatcher.py

@@ -416,10 +416,13 @@ class BECDispatcher:
 
     def stop_cli_server(self):
         """
-        Stop the CLI server.
+        Stop the CLI server. Idempotent: application teardown has several owners
+        (``GUIServer.shutdown`` and the generic ``BECConnector.terminate`` exit
+        handler both run on ``aboutToQuit``), so a second stop is an expected
+        no-op, not an error.
         """
         if self.cli_server is None:
-            logger.error("Cannot stop CLI server without starting it first")
+            logger.debug("CLI server already stopped or never started")
             return
         self.cli_server.shutdown()
         self.cli_server = None

bec_widgets/utils/rpc_server.py

@@ -531,8 +531,15 @@ class RPCServer:
     def shutdown(self):
         """
         Shut the RPC server down: stop the heartbeat, release the dispatcher
-        subscription and the registry callback, and shut the client down.
-        Safe to call multiple times.
+        subscription and the registry callback. Safe to call multiple times.
+
+        The BEC client is intentionally NOT shut down here: it is shared with the
+        dispatcher (and, in the companion app, with the exit ``terminate`` handler).
+        Shutting it down here would stop the connector's listener thread, so a later
+        ``dispatcher.disconnect_all()`` (which routes through
+        ``ManagedRedisConnection.unregister``) would early-return and silently leave
+        subscriptions alive. Client teardown is left to ``BECConnector.terminate`` /
+        the dispatcher fixture, which own the client lifecycle.
         """
         self.status = messages.BECStatus.IDLE
         self._heartbeat_timer.stop()
@@ -542,4 +549,3 @@ class RPCServer:
         )
         self.rpc_register.remove_callback(self.broadcast_registry_update)
         logger.info("Succeeded in shutting down CLI server")
-        self.client.shutdown()

tests/unit_tests/test_bec_connector.py

@@ -1,5 +1,6 @@
 # pylint: disable = no-name-in-module,missing-class-docstring, missing-module-docstring
 import time
+from unittest import mock
 
 import pytest
 from qtpy.QtCore import QObject
@@ -203,3 +204,18 @@ def test_bec_connector_export_settings():
     config = {"my_str_property": "new_value"}
     widget.load_settings(config)
     assert widget.my_str_property == "new_value"
+
+
+def test_bec_connector_terminate_registration_no_qapp_instance(qtbot):
+    """Constructing a BECConnector before a QApplication exists must not raise; the exit
+    handler is still registered, only the aboutToQuit wiring is skipped."""
+    import bec_widgets.utils.bec_connector as m
+
+    fresh_client = mock.MagicMock(name="fresh_client")
+    assert fresh_client not in BECConnector.EXIT_HANDLERS
+    try:
+        with mock.patch.object(m.QApplication, "instance", return_value=None):
+            BECConnectorQObject(client=fresh_client)  # must not raise
+        assert fresh_client in BECConnector.EXIT_HANDLERS
+    finally:
+        BECConnector.EXIT_HANDLERS.pop(fresh_client, None)

tests/unit_tests/test_bec_dispatcher.py

@@ -259,3 +259,16 @@ def test_qt_redis_connector_logs_rpc_before_qt_callback(monkeypatch):
         assert "request_id=dispatcher-request" in warning_message
     finally:
         connector.shutdown()
+
+
+def test_stop_cli_server_is_idempotent(bec_dispatcher):
+    """Both GUIServer.shutdown and BECConnector.terminate stop the CLI server at
+    application exit; the second call must be a silent no-op, not an ERROR."""
+    from unittest import mock
+
+    from bec_widgets.utils import bec_dispatcher as bd_module
+
+    with mock.patch.object(bd_module, "logger") as mock_logger:
+        bec_dispatcher.stop_cli_server()
+        bec_dispatcher.stop_cli_server()
+    mock_logger.error.assert_not_called()

tests/unit_tests/test_launch_window.py

@@ -224,19 +224,28 @@ def test_launch_window_closes(bec_launch_window, qtbot, connection_names, close_
         else:
             conn = _launcher_child_connection(bec_launch_window, name)
         connections[name] = conn
-    close_event = mock.MagicMock()
+    # A real QCloseEvent (accepted by default) so closeEvent can chain to
+    # super().closeEvent(event); patch cleanup to assert it runs on the accept path and
+    # to avoid exercising real teardown of the shared fixture under --random-order.
+    from qtpy.QtGui import QCloseEvent
+
+    close_event = QCloseEvent()
     with mock.patch.object(
         bec_launch_window.register, "list_all_connections", return_value=connections
     ):
-        with mock.patch.object(bec_launch_window, "hide") as mock_hide:
+        with (
+            mock.patch.object(bec_launch_window, "hide") as mock_hide,
+            mock.patch.object(bec_launch_window, "cleanup") as mock_cleanup,
+        ):
             bec_launch_window.closeEvent(close_event)
             if close_called:
                 mock_hide.assert_not_called()
-                close_event.accept.assert_called_once()
+                assert close_event.isAccepted()
+                mock_cleanup.assert_called_once()  # cleanup runs on the normal close path
             else:
                 mock_hide.assert_called_once()
-                close_event.accept.assert_not_called()
-                close_event.ignore.assert_called_once()
+                assert not close_event.isAccepted()  # ignore() flips the default True->False
+                mock_cleanup.assert_not_called()
 
 
 def test_main_label_fits_tile_width(bec_launch_window, qtbot):

tests/unit_tests/test_rpc_server.py

@@ -339,3 +339,37 @@ def test_rpc_register_remove_callback_is_noop_for_unknown(rpc_register=None):
 
     register = RPCRegister()
     register.remove_callback(lambda connections: None)  # must not raise
+
+
+def test_rpc_server_shutdown_does_not_shut_down_client(mocked_client):
+    """RPCServer.shutdown must not tear down the shared client: doing so nulls the
+    connector's listener thread and turns a later disconnect_all() into a silent no-op."""
+    from unittest.mock import patch
+
+    server = RPCServer(gui_id="no_client_teardown", client=mocked_client)
+    with patch.object(mocked_client, "shutdown") as client_shutdown:
+        server.shutdown()
+    client_shutdown.assert_not_called()
+    # idempotent + still no client shutdown
+    with patch.object(mocked_client, "shutdown") as client_shutdown2:
+        server.shutdown()
+    client_shutdown2.assert_not_called()
+
+
+def test_gui_server_stop_dispatcher_disconnects_before_stopping_cli_server(gui_server):
+    """disconnect_all() must run before stop_cli_server() so subscriptions are released
+    while the connector's listener thread is still alive."""
+    from unittest.mock import MagicMock, patch
+
+    calls = []
+    disp = MagicMock()
+    disp.disconnect_all.side_effect = lambda *a, **k: calls.append("disconnect_all")
+    disp.stop_cli_server.side_effect = lambda *a, **k: calls.append("stop_cli_server")
+    gui_server.dispatcher = disp
+    gui_server.launcher_window = MagicMock()
+    with (
+        patch.object(companion_app_module.shiboken6, "isValid", return_value=True),
+        patch.object(companion_app_module.pylsp_server, "is_running", return_value=False),
+    ):
+        gui_server.shutdown()
+    assert calls == ["disconnect_all", "stop_cli_server"]