fix(updater): hold update overlay until the whole update finishes

BlocksScreen/lib/panels/mainWindow.py

@@ -300,6 +300,9 @@ def __init__(self):
         self.updater_worker.daemon_unavailable.connect(
             self.update_page.handle_daemon_unavailable
         )
+        self.updater_worker.daemon_restarting.connect(
+            self.update_page.handle_daemon_restarting
+        )
         self.controlPanel.lock_ui.connect(self.set_ui_lock)
         self.updater_worker.request_reconnect.connect(self._on_post_update_reconnect)
         self.updater_worker.proxy_connected.connect(

BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py

@@ -375,6 +375,10 @@ def handle_busy_changed(self, busy: bool) -> None:
         if busy:
             if self._provisioning:
                 self._show_provisioning_overlay()
+            elif not self._overlay_shown:
+                # Busy this UI did not start: it restarted mid-update, block it too.
+                self._overlay_shown = True
+                self.call_load_panel.emit(True, "Update in progress ...", False)
             self._restart_pending = False
             self._restart_grace_timer.stop()
             self._stale_overlay_timer.stop()
@@ -409,11 +413,24 @@ def handle_provisioning_changed(self, provisioning: bool) -> None:
         if provisioning and self._busy:
             self._cancel_btn.hide()
             self._show_provisioning_overlay()
+        elif self._busy:
+            # Still busy after the clone: the daemon holds it for install-updater.
+            self._show_finishing_overlay()
 
     def _show_provisioning_overlay(self) -> None:
         self._overlay_shown = True
         self.call_load_panel.emit(True, "Missing component, installing ...", False)
 
+    def _show_finishing_overlay(self) -> None:
+        self._overlay_shown = True
+        self.call_load_panel.emit(True, "Finishing installation ...", False)
+
+    @QtCore.pyqtSlot(name="handle-daemon-restarting")
+    def handle_daemon_restarting(self) -> None:
+        """Daemon left mid-busy to restart: keep the overlay until the new one reports."""
+        self._cancel_btn.hide()
+        self._show_finishing_overlay()
+
     def _dismiss_stale_overlay(self) -> None:
         """Drop the overlay if the post-update status never arrived."""
         if self._overlay_shown and not self._busy:

BlocksScreen/lib/updater_worker.py

@@ -53,6 +53,7 @@ class UpdaterWorker(QtCore.QObject):
     busy_changed = QtCore.pyqtSignal(bool)
     provisioning_changed = QtCore.pyqtSignal(bool)
     daemon_unavailable = QtCore.pyqtSignal()
+    daemon_restarting = QtCore.pyqtSignal()
     update_rejected = QtCore.pyqtSignal()
     request_reconnect = QtCore.pyqtSignal()
     proxy_connected = QtCore.pyqtSignal()
@@ -337,7 +338,11 @@ async def _watch_daemon_owner(self) -> None:
                             self._daemon_owner = ""
                             # Retry bus-activates a stopped unit; only mask keeps it off.
                             _log.error("updater daemon left the bus - awaiting restart")
-                            self.daemon_unavailable.emit()
+                            # Busy exit = install-updater restarting it; a failed retry still reports.
+                            if self._last_busy:
+                                self.daemon_restarting.emit()
+                            else:
+                                self.daemon_unavailable.emit()
                             self._schedule_reconnect()
                             continue
                         _log.warning(

tests/lib/test_updater_worker_unit.py

@@ -273,6 +273,24 @@ async def test_owner_lost_emits_unavailable_and_schedules_reconnect(
         worker._async_initialize.assert_not_awaited()
         worker._schedule_reconnect.assert_called_once_with()
 
+    @pytest.mark.asyncio
+    async def test_owner_lost_while_busy_emits_restarting(self, worker):
+        """install-updater restarts a busy daemon: the overlay must stay, not drop."""
+        restarting, unavailable = [], []
+        worker.daemon_restarting.connect(lambda: restarting.append(True))
+        worker.daemon_unavailable.connect(lambda: unavailable.append(True))
+        worker._last_busy = True
+        worker._async_initialize = AsyncMock()
+        worker._schedule_reconnect = MagicMock()
+        _, dbus = self._subscribe(
+            worker, [(_BUS, ":1.5", ""), self._stop(worker)], owner=":1.5"
+        )
+        with dbus:
+            await worker._watch_daemon_owner()
+        assert restarting == [True]
+        assert unavailable == []
+        worker._schedule_reconnect.assert_called_once_with()
+
     @pytest.mark.asyncio
     async def test_other_names_and_repeat_owner_ignored(self, worker):
         worker._async_initialize = AsyncMock()

tests/updater/conftest.py

@@ -60,6 +60,10 @@ def svc():
     mock_svc.provision_missing = AsyncMock(return_value=False)
     mock_svc.reconcile = AsyncMock(return_value=True)
     mock_svc.reconcile_if_pending = AsyncMock()
+    mock_svc.deploy_pending = MagicMock(return_value=False)
+    # Units stay live while the flag is set unless a test says otherwise.
+    mock_svc.deploy_running = AsyncMock(side_effect=lambda: mock_svc.deploy_pending())
+    mock_svc.deploy_flag_stamp = MagicMock(return_value=1)
     mock_svc._components = [
         ComponentConfig(name="moonraker", kind="git"),
         ComponentConfig(name="klipper", kind="git"),
@@ -76,6 +80,9 @@ def svc():
     s._busy = False
     s._provisioning = False
     s._provisioned = False
+    s._boot_pass_done = True
+    s._deploy_hold = None
+    s._capped_flag = None
     s._background_tasks = set()
     s._closing = False
     s._status_check_in_progress = False

tests/updater/test_dbus_service_unit.py

@@ -352,21 +352,189 @@ async def test_periodic_check_never_lengthens_a_short_poll_interval(self, svc):
 
 
 class TestBootNotBusy:
-    def test_idle_at_construction(self):
-        """Offline boots must not open on the install overlay: busy waits for a real clone."""
+    @staticmethod
+    def _build(deploy_pending: bool):
         from updater import dbus_service
 
+        mock_svc = MagicMock()
+        mock_svc.deploy_pending = MagicMock(return_value=deploy_pending)
         with (
-            patch.object(dbus_service, "UpdateService", return_value=MagicMock()),
+            patch.object(dbus_service, "UpdateService", return_value=mock_svc),
             patch.object(
                 dbus_service.UpdaterDbusService,
                 "_spawn",
                 MagicMock(side_effect=lambda coro, **_: coro.close()),
             ),
         ):
-            built = dbus_service.UpdaterDbusService()
+            return dbus_service.UpdaterDbusService()
+
+    def test_idle_at_construction(self):
+        """Offline boots must not open on the install overlay: busy waits for a real clone."""
+        built = self._build(deploy_pending=False)
         assert (built._busy, built._provisioning) == (False, False)
 
+    def test_busy_at_construction_while_deploy_runs(self):
+        """install-updater restarts the daemon mid-run: the new one resumes the hold."""
+        built = self._build(deploy_pending=True)
+        assert built._busy is True
+        assert built._boot_pass_done is False
+
+
+class TestDeployHold:
+    @pytest.mark.asyncio
+    async def test_busy_held_until_flag_clears(self, svc, monkeypatch):
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01)
+        svc._busy = True
+        svc._svc.deploy_pending.return_value = True
+        svc._set_busy(False)
+        await asyncio.sleep(0.05)
+        assert svc._busy is True
+        svc.busy_changed.emit.assert_not_called()
+
+        svc._svc.deploy_pending.return_value = False
+        await asyncio.sleep(0.05)
+        assert svc._busy is False
+        svc.busy_changed.emit.assert_called_once_with((False,))
+        assert svc._deploy_hold is None
+
+    @pytest.mark.asyncio
+    async def test_hold_released_at_cap(self, svc, monkeypatch):
+        """A flag nothing clears must not lock the screen forever."""
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01)
+        monkeypatch.setattr(dbus_service, "_DEPLOY_HOLD_CAP_S", 0.03)
+        svc._busy = True
+        svc._svc.deploy_pending.return_value = True
+        svc._set_busy(False)
+        await asyncio.sleep(0.1)
+        assert svc._busy is False
+
+    @pytest.mark.asyncio
+    async def test_capped_flag_never_holds_again(self, svc, monkeypatch):
+        """A stuck flag costs the cap once, not again at the end of every update."""
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01)
+        monkeypatch.setattr(dbus_service, "_DEPLOY_HOLD_CAP_S", 0.03)
+        svc._svc.deploy_pending.return_value = True
+        svc._set_busy(False)
+        await asyncio.sleep(0.1)
+        svc._set_busy(True)
+        svc._set_busy(False)
+        assert (svc._busy, svc._deploy_hold) == (False, None)
+
+        svc._svc.deploy_flag_stamp.return_value = 2
+        svc._set_busy(False)
+        assert svc._busy is True
+        svc._svc.deploy_pending.return_value = False
+        await asyncio.sleep(0.05)
+
+    @pytest.mark.asyncio
+    async def test_dead_deploy_units_release_at_once(self, svc, monkeypatch):
+        """A failed or stopped path unit never runs install-updater: its flag is stale."""
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01)
+        svc._busy = True
+        svc._svc.deploy_pending.return_value = True
+        svc._svc.deploy_running = AsyncMock(return_value=False)
+        svc._set_busy(False)
+        await asyncio.sleep(0.02)
+        assert (svc._busy, svc._deploy_hold) == (False, None)
+
+    @pytest.mark.asyncio
+    async def test_hold_spawned_once(self, svc, monkeypatch):
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01)
+        svc._svc.deploy_pending.return_value = True
+        svc._set_busy(False)
+        first = svc._deploy_hold
+        svc._set_busy(False)
+        assert svc._deploy_hold is first
+        svc._svc.deploy_pending.return_value = False
+        await asyncio.sleep(0.05)
+
+    @pytest.mark.asyncio
+    async def test_hold_end_leaves_running_provision_busy(self, svc, monkeypatch):
+        """A clone that started under the hold drops busy itself when it finishes."""
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_DEPLOY_POLL_S", 0.01)
+        svc._svc.deploy_pending.return_value = True
+        svc._set_busy(False)
+        svc._provision_busy(True)
+        svc._svc.deploy_pending.return_value = False
+        await asyncio.sleep(0.05)
+        assert svc._busy is True
+        svc._provision_busy(False)
+        assert svc._busy is False
+
+    @pytest.mark.asyncio
+    async def test_update_rejected_during_hold(self, svc):
+        """install-updater restarts moonraker: no new batch may start under it."""
+        svc._svc.deploy_pending.return_value = True
+        svc._set_busy(False)
+        assert await svc.update_all() is False
+        svc._svc.deploy_pending.return_value = False
+        await asyncio.sleep(0)
+
+    @pytest.mark.asyncio
+    async def test_cancel_keeps_hold(self, svc):
+        svc._busy = True
+        svc._svc.deploy_pending.return_value = True
+        await svc.cancel()
+        assert svc._busy is True
+        svc._svc.deploy_pending.return_value = False
+
+
+class TestStatusAfterBootPass:
+    @pytest.mark.asyncio
+    async def test_request_status_deferred_until_boot_pass(self, svc):
+        """A status before provisioning would let the UI drop its overlay mid-install."""
+        svc._boot_pass_done = False
+        await svc.request_status()
+        await asyncio.sleep(0)
+        svc.status_ready.emit.assert_not_called()
+
+    @pytest.mark.asyncio
+    @pytest.mark.usefixtures("reconciled")
+    async def test_boot_pass_enables_request_status(self, svc, monkeypatch):
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_BOOT_DELAY_S", 0.0)
+        svc._boot_pass_done = False
+        svc._svc.poll_interval = 3600.0
+        task = asyncio.create_task(svc._periodic_status_check())
+        await asyncio.sleep(0.05)
+        assert svc._boot_pass_done is True
+        svc.status_ready.emit.assert_called_once()
+        task.cancel()
+        with pytest.raises(asyncio.CancelledError):
+            await task
+
+    @pytest.mark.asyncio
+    @pytest.mark.usefixtures("reconciled")
+    async def test_failed_boot_pass_still_emits_status(self, svc, monkeypatch):
+        """A raising provision pass must not leave the UI without status for a poll."""
+        from updater import dbus_service
+
+        monkeypatch.setattr(dbus_service, "_BOOT_DELAY_S", 0.0)
+        svc._boot_pass_done = False
+        svc._provisioned = False
+        svc._svc.provision_missing = AsyncMock(side_effect=RuntimeError("boom"))
+        svc._svc.poll_interval = 3600.0
+        task = asyncio.create_task(svc._periodic_status_check())
+        await asyncio.sleep(0.05)
+        assert svc._boot_pass_done is True
+        svc.status_ready.emit.assert_called_once()
+        task.cancel()
+        with pytest.raises(asyncio.CancelledError):
+            await task
+
 
 @pytest.mark.usefixtures("reconciled")
 class TestBootProvision:

tests/updater/test_service_unit.py

@@ -1,5 +1,6 @@
 import asyncio
 import json
+import os
 import time
 from contextlib import ExitStack, nullcontext
 from pathlib import Path
@@ -20,6 +21,8 @@ def units_installed():
         "updater.service.service_unit_missing", AsyncMock(return_value=False)
     ) as mock:
         yield mock
+
+
 def _isolate_inflight(tmp_path_factory, monkeypatch):
     """Keep the in-flight marker, state and history out of the real cache."""
     marker = tmp_path_factory.mktemp("inflight") / "updater_inflight.json"
@@ -2827,6 +2830,47 @@ def test_touch_deploy_flag_rejects_planted_symlink(self, tmp_path: Path):
         assert flag.exists() and not flag.is_symlink()
         assert target.read_text() == "keep"  # symlink not followed
 
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(
+        ("ok", "out", "running"),
+        [
+            (True, "active\n\ninactive\n", True),
+            (True, "inactive\n\nactivating\n", True),
+            (True, "failed\n\ninactive\n", False),
+            (True, "inactive\n\ninactive\n", False),
+            (False, "", True),
+        ],
+    )
+    async def test_deploy_running_follows_unit_state(
+        self, tmp_path: Path, ok: bool, out: str, running: bool
+    ):
+        """A failed/stopped path unit never fires; an unreadable state trusts the flag."""
+        flag = tmp_path / ".run-install-updater"
+        flag.touch()
+        with (
+            patch("updater.service._DEPLOY_FLAG", flag),
+            patch("updater.service._DEPLOY_PATH_UNIT", flag),
+            patch("updater.service._run", new=AsyncMock(return_value=(ok, out))),
+        ):
+            assert await UpdateService().deploy_running() is running
+
+    @pytest.mark.asyncio
+    async def test_deploy_running_skips_systemctl_without_flag(self, tmp_path: Path):
+        with (
+            patch("updater.service._DEPLOY_FLAG", tmp_path / "nope"),
+            patch("updater.service._run", new=AsyncMock()) as mock_run,
+        ):
+            assert await UpdateService().deploy_running() is False
+        mock_run.assert_not_called()
+
+    def test_deploy_flag_stamp_tracks_each_touch(self, tmp_path: Path):
+        flag = tmp_path / ".run-install-updater"
+        with patch("updater.service._DEPLOY_FLAG", flag):
+            assert UpdateService.deploy_flag_stamp() is None
+            flag.touch()
+            os.utime(flag, ns=(1, 1))
+            assert UpdateService.deploy_flag_stamp() == 1
+
 
 class TestInflightRollback:
     """Cross-reboot revert: an update cut off before commit is undone on boot."""