From e027247aeed29fa2d16f07588b9e7d07ec1908e4 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 15:30:23 +0100 Subject: [PATCH 01/10] fix(updater): fail health check fast, signal busy during boot provisioning, keep overlay through UI restart --- .../panels/widgets/MainWindow/updatePage.py | 18 +++++++++++++-- updater/dbus_service.py | 2 +- updater/executor.py | 13 +++++++++-- updater/service.py | 22 +++++++++++++------ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py index 825bd013..b4e3c9db 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py @@ -72,6 +72,7 @@ def __init__(self) -> None: self._update_avail: bool = False self._post_update_status_pending: bool = False self._overlay_shown: bool = False + self._restart_pending: bool = False self._elapsed_time_seconds: int = 0 self._elapsed_timer: QtCore.QTimer = QtCore.QTimer(self) self._elapsed_timer.setSingleShot(False) @@ -335,7 +336,7 @@ def handle_status_ready(self, json_str: str) -> None: self._update_avail = _update_avail if not self._busy: self.show_loading(False) - if self._post_update_status_pending: + if self._post_update_status_pending and not self._restart_pending: _log.debug("status_ready: emitting call_load_panel(False)") self.call_load_panel.emit(False, "", False) self._post_update_status_pending = False @@ -350,6 +351,7 @@ def handle_busy_changed(self, busy: bool) -> None: self._busy = busy self.show_loading(busy) if busy: + self._restart_pending = False self._elapsed_time_seconds = 0 self._elapsed_timer.start() self._busy_timeout_timer.start() @@ -364,11 +366,21 @@ def handle_busy_changed(self, busy: bool) -> None: self._progress_label.hide() self._cancel_btn.hide() self.update_all_btn.setEnabled(True) - if self._overlay_shown: + if self._restart_pending: + # Keep the overlay up: SIGTERM is imminent, MainWindow would flash. + QtCore.QTimer.singleShot(15000, self._dismiss_after_restart_grace) + elif self._overlay_shown: self._overlay_shown = False self.call_load_panel.emit(False, "", False) self._request_status_debounced() + def _dismiss_after_restart_grace(self) -> None: + """Drop the overlay if the expected UI restart never came.""" + if self._restart_pending and not self._busy: + self._restart_pending = False + self._overlay_shown = False + self.call_load_panel.emit(False, "", False) + @QtCore.pyqtSlot(name="on-update-all-clicked") def on_update_all_clicked(self) -> None: """Guard against updates during a print or with hot heaters; otherwise show confirm dialog.""" @@ -425,6 +437,8 @@ def handle_step_complete(self, name: str, step: int, total: int) -> None: if self._busy_timeout_timer.isActive(): self._busy_timeout_timer.start() self._overlay_shown = True + # BlocksScreen's last step restarts this very process. + self._restart_pending = name == "BlocksScreen" and step == total overlay_msg = f"{name}: {label}" self._progress_label.setText(f"Step {step}/{total}") self.call_load_panel.emit(True, overlay_msg, False) diff --git a/updater/dbus_service.py b/updater/dbus_service.py index 3e1b6b14..123c5625 100644 --- a/updater/dbus_service.py +++ b/updater/dbus_service.py @@ -195,7 +195,7 @@ async def _periodic_status_check(self) -> None: while True: try: await self._emit_status() - if await self._svc.provision_missing(): + if await self._svc.provision_missing(self._set_busy): await self._emit_status() # reflect freshly-installed components except Exception as exc: # noqa: BLE001 _log.error("periodic_check failed: %s", exc) diff --git a/updater/executor.py b/updater/executor.py index 2c171b16..5c9b3176 100644 --- a/updater/executor.py +++ b/updater/executor.py @@ -1034,13 +1034,22 @@ def _http_probe(url: str) -> bool: conn.close() -async def wait_for_http_ready(url: str, timeout: float = 120.0) -> bool: - """Poll a component's loopback health URL until it returns 2xx or timeout.""" +async def wait_for_http_ready( + url: str, timeout: float = 120.0, *, service: str | None = None +) -> bool: + """Poll a health URL until 2xx or timeout; fail fast if `service` leaves active.""" deadline = asyncio.get_running_loop().time() + timeout while True: if await asyncio.to_thread(_http_probe, url): logger.info("health check ok: %s", url) return True + # A crash-looping unit is 'activating', never 'active': don't wait out the timeout. + if ( + service + and not (await _run([SYSTEMCTL, "is-active", service], timeout=10.0))[0] + ): + logger.warning("service %r left active during health check", service) + return False if asyncio.get_running_loop().time() >= deadline: logger.warning("health check timed out after %.0fs: %s", timeout, url) return False diff --git a/updater/service.py b/updater/service.py index 1eb09a88..ca7272e5 100644 --- a/updater/service.py +++ b/updater/service.py @@ -476,8 +476,10 @@ async def _filter_dead_branch_batch(self, batch: list[ComponentConfig]) -> bool: ) return ok - async def provision_missing(self) -> bool: - """Clone absent install_if_missing components at boot (no manual update).""" + async def provision_missing( + self, on_busy: Callable[[bool], None] | None = None + ) -> bool: + """Clone absent install_if_missing components at boot; on_busy brackets the work.""" missing = [ c for c in self._components @@ -492,10 +494,16 @@ async def provision_missing(self) -> bool: if not acquired: self._log.info("provision_missing: update in progress, deferring") return False - for c in missing: - if c.path is None or not c.path.exists(): # recheck under lock - await self._provision_component(c) - provisioned = True + if on_busy: + on_busy(True) # UI shows step_complete only while busy + try: + for c in missing: + if c.path is None or not c.path.exists(): # recheck under lock + await self._provision_component(c) + provisioned = True + finally: + if on_busy: + on_busy(False) return provisioned async def _preflight_fetch( @@ -1982,7 +1990,7 @@ async def _restart_one(self, service: str, health_url: str | None = None) -> boo if not await wait_for_service_active(service, timeout=90.0): self._log.error("%s did not become active after restart", service) return False - if health_url and not await wait_for_http_ready(health_url): + if health_url and not await wait_for_http_ready(health_url, service=service): self._log.error("%s active but health check failed", service) return False self._log.info("%s active after restart", service) From 94606fb634f4f11adf9692bd98f73a4c5c6ca6b1 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 16:07:33 +0100 Subject: [PATCH 02/10] fix(updater): fail-fast Spoolman health check, busy overlay during provision, no double start --- tests/updater/test_service_unit.py | 38 +++++++++++++++++++++++++++++- updater/executor.py | 16 +++++++++++++ updater/service.py | 20 +++++++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/tests/updater/test_service_unit.py b/tests/updater/test_service_unit.py index b51ff640..ed7ec906 100644 --- a/tests/updater/test_service_unit.py +++ b/tests/updater/test_service_unit.py @@ -1753,10 +1753,12 @@ async def test_provision_waits_for_service_active(self, tmp_path): return_value=(True, ""), ), patch("updater.service.run_hook", return_value=(True, "")), + patch("updater.service.is_service_active", return_value=False), patch("updater.service.restart_service", return_value=(True, "")), patch( "updater.service.wait_for_service_active", return_value=False ) as mock_wait, + patch("updater.service.stop_service", return_value=(True, "")) as mock_stop, patch("updater.service.shutil.rmtree") as mock_rmtree, ): svc = UpdateService(callback=cb) @@ -1764,6 +1766,7 @@ async def test_provision_waits_for_service_active(self, tmp_path): ok = await svc.update_component("newcomp") assert ok is False mock_wait.assert_called_once() + mock_stop.assert_called_once_with("newcomp.service") mock_rmtree.assert_called_once() assert cb.on_error.call_args[0][1] == "restart" @@ -1783,11 +1786,13 @@ async def test_provision_fails_when_health_check_fails(self, tmp_path): return_value=(True, ""), ), patch("updater.service.run_hook", return_value=(True, "")), + patch("updater.service.is_service_active", return_value=False), patch("updater.service.restart_service", return_value=(True, "")), patch("updater.service.wait_for_service_active", return_value=True), patch( "updater.service.wait_for_http_ready", return_value=False ) as mock_health, + patch("updater.service.stop_service", return_value=(True, "")) as mock_stop, patch("updater.service.shutil.rmtree") as mock_rmtree, ): svc = UpdateService(callback=cb) @@ -1795,6 +1800,7 @@ async def test_provision_fails_when_health_check_fails(self, tmp_path): ok = await svc.update_component("newcomp") assert ok is False mock_health.assert_called_once() + mock_stop.assert_called_once_with("newcomp.service") mock_rmtree.assert_called_once() assert cb.on_error.call_args[0][1] == "restart" @@ -1814,6 +1820,7 @@ async def test_provision_succeeds_when_health_ready(self, tmp_path): return_value=(True, ""), ), patch("updater.service.run_hook", return_value=(True, "")), + patch("updater.service.is_service_active", return_value=False), patch("updater.service.restart_service", return_value=(True, "")), patch("updater.service.wait_for_service_active", return_value=True), patch( @@ -1826,10 +1833,39 @@ async def test_provision_succeeds_when_health_ready(self, tmp_path): svc._components = [comp] ok = await svc.update_component("newcomp") assert ok is True - mock_health.assert_called_once_with("http://127.0.0.1:7912/health") + mock_health.assert_called_once_with( + "http://127.0.0.1:7912/health", service="newcomp.service" + ) mock_rmtree.assert_not_called() cb.on_component_done.assert_called_with("newcomp", True) + @pytest.mark.asyncio + async def test_provision_skips_restart_when_hook_started_service(self, tmp_path): + comp = self._comp( + tmp_path, + service="newcomp.service", + health_url="http://127.0.0.1:7912/health", + ) + cb = MagicMock() + with ( + patch("updater.service.git_clone", return_value=(True, "")), + patch("updater.service.git_get_hash", return_value="newhash"), + patch( + "updater.service.UpdateService._install_dependencies", + return_value=(True, ""), + ), + patch("updater.service.run_hook", return_value=(True, "")), + patch("updater.service.is_service_active", return_value=True), + patch("updater.service.restart_service") as mock_restart, + patch("updater.service.wait_for_http_ready", return_value=True), + patch("updater.service.enable_service", return_value=(True, "")), + ): + svc = UpdateService(callback=cb) + svc._components = [comp] + ok = await svc.update_component("newcomp") + assert ok is True + mock_restart.assert_not_called() + @pytest.mark.asyncio async def test_check_status_reports_needs_install(self, tmp_path): comp = self._comp(tmp_path) diff --git a/updater/executor.py b/updater/executor.py index 5c9b3176..3e9e7c35 100644 --- a/updater/executor.py +++ b/updater/executor.py @@ -995,6 +995,13 @@ async def enable_service(name: str | None) -> tuple[bool, str]: return await _run([SUDO, SYSTEMCTL, "enable", name], timeout=15.0) +async def is_service_active(name: str) -> bool: + """One-shot systemctl is-active probe.""" + if not _SERVICE_RE.match(name): + return False + return (await _run([SYSTEMCTL, "is-active", name], timeout=10.0))[0] + + async def wait_for_service_active(name: str, timeout: float = 90.0) -> bool: """Poll systemctl is-active until active or timeout.""" if not _SERVICE_RE.match(name): @@ -1079,6 +1086,15 @@ async def verify_updater_importable(component_path: Path | None) -> bool: return ok +async def stop_service(name: str | None) -> tuple[bool, str]: + """Stop a systemd service.""" + if name is None: + return (False, "service name is None") + if not _SERVICE_RE.match(name): + return (False, f"service name {name!r} is invalid") + return await _run([SUDO, SYSTEMCTL, "stop", name], timeout=30.0) + + async def restart_service(name: str | None) -> tuple[bool, str]: """Restart a systemd service, recovering from a start-limit hit.""" if name is None: diff --git a/updater/service.py b/updater/service.py index ca7272e5..e7b46314 100644 --- a/updater/service.py +++ b/updater/service.py @@ -47,9 +47,11 @@ git_tree_has_path, git_untracked_paths, is_git_repo, + is_service_active, restart_service, restart_service_noblock, run_hook, + stop_service, verify_updater_importable, wait_for_http_ready, wait_for_service_active, @@ -1703,6 +1705,9 @@ async def _remove_clone(self, component: ComponentConfig) -> None: async def _fail_provision(self, component: ComponentConfig, reason: str) -> bool: """Remove the partial clone, log, and report failure.""" + if component.service and reason in ("hook", "restart"): + # Else systemd crash-loops the unit on the deleted dir until StartLimit. + await stop_service(component.service) await self._remove_clone(component) self._history("install_failed", component.name, reason=reason) self._log.warning( @@ -1716,7 +1721,12 @@ async def _provision_restart_service( """Restart+health-check+enable the provisioned service; fail reason or None.""" if not component.service: return None - if not await self._restart_one(component.service, component.health_url): + if await is_service_active(component.service): + # The hook already started it (enable --now): a restart would start it twice. + ok = await self._await_health(component.service, component.health_url) + else: + ok = await self._restart_one(component.service, component.health_url) + if not ok: return "restart" # Enable only after a clean start (no boot-looping failed unit). en_ok, en_err = await enable_service(component.service) @@ -1980,6 +1990,14 @@ async def _stage_component(self, component: ComponentConfig) -> tuple[bool, str] return guard return await self._stage_apply_ref(component, tip) + async def _await_health(self, service: str, health_url: str | None) -> bool: + """Verify an already-running service answers its health URL.""" + if health_url and not await wait_for_http_ready(health_url, service=service): + self._log.error("%s active but health check failed", service) + return False + self._log.info("%s already active, restart skipped", service) + return True + async def _restart_one(self, service: str, health_url: str | None = None) -> bool: """Restart a service and verify it came active (kill-fallback aware).""" self._log.info("restarting %s and waiting for active", service) From 1718e2f571b5b78bbb6574a596c06c2ff8765c61 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 16:12:15 +0100 Subject: [PATCH 03/10] fix(updater): fail-fast Spoolman health check, no double start, hold overlay until fresh status --- .../panels/widgets/MainWindow/updatePage.py | 13 ++++++-- tests/widgets/test_update_page_unit.py | 32 ++++++++++++++----- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py index b4e3c9db..e195308e 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py @@ -340,6 +340,7 @@ def handle_status_ready(self, json_str: str) -> None: _log.debug("status_ready: emitting call_load_panel(False)") self.call_load_panel.emit(False, "", False) self._post_update_status_pending = False + self._overlay_shown = False else: _log.debug("status_ready: skipping loadscreen dismiss (busy=True)") self.build_cards() @@ -370,10 +371,18 @@ def handle_busy_changed(self, busy: bool) -> None: # Keep the overlay up: SIGTERM is imminent, MainWindow would flash. QtCore.QTimer.singleShot(15000, self._dismiss_after_restart_grace) elif self._overlay_shown: - self._overlay_shown = False - self.call_load_panel.emit(False, "", False) + # Hold the overlay until fresh status lands, else stale cards flash. + self._post_update_status_pending = True + QtCore.QTimer.singleShot(10000, self._dismiss_stale_overlay) self._request_status_debounced() + def _dismiss_stale_overlay(self) -> None: + """Drop the overlay if the post-update status never arrived.""" + if self._overlay_shown and not self._busy: + self._overlay_shown = False + self._post_update_status_pending = False + self.call_load_panel.emit(False, "", False) + def _dismiss_after_restart_grace(self) -> None: """Drop the overlay if the expected UI restart never came.""" if self._restart_pending and not self._busy: diff --git a/tests/widgets/test_update_page_unit.py b/tests/widgets/test_update_page_unit.py index 26980da0..8a6ed50e 100644 --- a/tests/widgets/test_update_page_unit.py +++ b/tests/widgets/test_update_page_unit.py @@ -11,8 +11,12 @@ def page(qapp): """UpdatePage instance with all heavy UI deps mocked.""" patches = [ - patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.LoadingOverlayWidget"), - patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BlocksCustomButton"), + patch( + "BlocksScreen.lib.panels.widgets.MainWindow.updatePage.LoadingOverlayWidget" + ), + patch( + "BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BlocksCustomButton" + ), patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.IconButton"), ] for p in patches: @@ -307,12 +311,22 @@ def test_false_does_not_emit_call_load_panel_when_no_overlay(self, page, qtbot): with qtbot.assertNotEmitted(page.call_load_panel, wait=200): page.handle_busy_changed(False) - def test_false_emits_call_load_panel_when_overlay_shown(self, page, qtbot): + def test_false_holds_overlay_until_status_ready(self, page, qtbot): page.show_loading = MagicMock() page._overlay_shown = True - with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: + with qtbot.assertNotEmitted(page.call_load_panel, wait=200): page.handle_busy_changed(False) - assert blocker.args == [False, "",False] + assert page._overlay_shown is True + with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: + page.handle_status_ready(_make_payload()) + assert blocker.args == [False, "", False] + assert page._overlay_shown is False + + def test_stale_overlay_fallback_dismisses(self, page, qtbot): + page._overlay_shown = True + page._busy = False + with qtbot.waitSignal(page.call_load_panel, timeout=200): + page._dismiss_stale_overlay() assert page._overlay_shown is False def test_true_starts_elapsed_timer(self, page): @@ -413,13 +427,13 @@ class TestHandleStepComplete: def test_emits_call_load_panel_with_step_message(self, page, qtbot): with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_step_complete("klipper", 1, 4) - assert blocker.args == [True, "klipper: fetching",False] + assert blocker.args == [True, "klipper: fetching", False] page._progress_label.setText.assert_called_with("Step 1/4") def test_unknown_steps_falls_back_to_working(self, page, qtbot): with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_step_complete("moonraker", 99, 4) - assert blocker.args == [True, "moonraker: working",False] + assert blocker.args == [True, "moonraker: working", False] page._progress_label.setText.assert_called_with("Step 99/4") @@ -498,7 +512,9 @@ def test_bad_payload_keeps_statuses_and_toasts(self, page): class TestConfirmPopupCleanup: def test_second_confirm_deletes_previous_popup(self, page): - with patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BasePopup") as popup_cls: + with patch( + "BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BasePopup" + ) as popup_cls: first = MagicMock() second = MagicMock() popup_cls.side_effect = [first, second] From 87ddc5b0204a7525f9dc4d0b8c71dd6b084eed3b Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 16:45:05 +0100 Subject: [PATCH 04/10] fix(updater): skip background apt pass when a daemon restart is pending --- tests/updater/test_dbus_service_unit.py | 15 +++++++++++++++ tests/updater/test_executor_unit.py | 4 +++- tests/updater/test_service_unit.py | 3 +++ updater/dbus_service.py | 5 ++++- updater/service.py | 4 ++++ 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/tests/updater/test_dbus_service_unit.py b/tests/updater/test_dbus_service_unit.py index acc4381f..9940a16f 100644 --- a/tests/updater/test_dbus_service_unit.py +++ b/tests/updater/test_dbus_service_unit.py @@ -422,6 +422,21 @@ async def test_update_all_includes_errored_git_repo(self, svc): assert "RF50-Klipper" in called_with assert "klipper" not in called_with # clean repo not updated + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("restart_pending", "apt_spawned"), [(True, False), (False, True)] + ) + async def test_background_apt_skipped_when_daemon_restart_pending( + self, svc, restart_pending, apt_spawned + ): + """A pending daemon restart would SIGKILL apt mid-run, so the pass is skipped.""" + svc._svc.check_status = AsyncMock(return_value={}) + svc._svc.background_apt_upgrade = AsyncMock() + svc._svc.daemon_restart_pending = restart_pending + await svc._run_update_all() + await asyncio.sleep(0) # let a spawned task run + assert svc._svc.background_apt_upgrade.called is apt_spawned + class TestLockHeldSurfacesError: def _held_lock(self): diff --git a/tests/updater/test_executor_unit.py b/tests/updater/test_executor_unit.py index f0f53d2a..be1ffbcc 100644 --- a/tests/updater/test_executor_unit.py +++ b/tests/updater/test_executor_unit.py @@ -1382,7 +1382,9 @@ async def test_returns_true_on_2xx(self): @pytest.mark.asyncio async def test_times_out_when_never_ready(self): with patch("updater.executor._http_probe", return_value=False): - assert await wait_for_http_ready("http://127.0.0.1:7912/x", timeout=0) is False + assert ( + await wait_for_http_ready("http://127.0.0.1:7912/x", timeout=0) is False + ) @pytest.mark.asyncio async def test_polls_until_ready(self): diff --git a/tests/updater/test_service_unit.py b/tests/updater/test_service_unit.py index ed7ec906..367932c7 100644 --- a/tests/updater/test_service_unit.py +++ b/tests/updater/test_service_unit.py @@ -2397,6 +2397,7 @@ async def test_install_touches_deploy_flag_not_restart(self, tmp_path: Path): mock_restart.assert_not_called() mock_verify.assert_not_called() assert not sentinel.exists() # consumed + assert svc.daemon_restart_pending is True @pytest.mark.asyncio async def test_code_restarts_only_when_importable(self, tmp_path: Path): @@ -2413,6 +2414,7 @@ async def test_code_restarts_only_when_importable(self, tmp_path: Path): svc = self._svc_with_ui() await svc._apply_deferred_restart() mock_restart.assert_called_once_with("BlocksScreen-updater.service") + assert svc.daemon_restart_pending is True @pytest.mark.asyncio async def test_code_skips_restart_when_not_importable(self, tmp_path: Path): @@ -2430,6 +2432,7 @@ async def test_code_skips_restart_when_not_importable(self, tmp_path: Path): svc = self._svc_with_ui() await svc._apply_deferred_restart() mock_restart.assert_not_called() + assert svc.daemon_restart_pending is False def test_read_clear_sentinel_install_outranks_code(self, tmp_path: Path): sentinel = tmp_path / "updater-restart-needed" diff --git a/updater/dbus_service.py b/updater/dbus_service.py index 123c5625..e661fbeb 100644 --- a/updater/dbus_service.py +++ b/updater/dbus_service.py @@ -275,7 +275,10 @@ async def _run_update_all(self) -> None: self._update_all_locked, "update_all", "updater" ) # Silent apt pass only if we held the lock; else the CLI run owns apt. - if ran: + if ran and self._svc.daemon_restart_pending: + # A SIGKILL from the restart could land inside dpkg; the next poll re-offers the packages. + _log.info("background apt upgrade skipped: daemon restart pending") + elif ran: self._spawn( self._svc.background_apt_upgrade(), name="background_apt_upgrade" ) diff --git a/updater/service.py b/updater/service.py index e7b46314..ca4d6abb 100644 --- a/updater/service.py +++ b/updater/service.py @@ -260,6 +260,8 @@ def __init__(self, callback: ProgressCallback | None = None) -> None: self._log = logging.getLogger("updater") # Self-heal: trailing-window sample ring for crash-loop detection. self._nrestarts_samples: dict[str, list[tuple[float, int]]] = {} + # Set once this daemon is about to be stopped, so no apt child gets SIGKILLed with it. + self.daemon_restart_pending = False def has_component(self, name: str) -> bool: """Return True if a component with the given name is registered.""" @@ -1005,6 +1007,7 @@ async def _apply_deferred_restart(self) -> None: "(install-updater runs out-of-band)" ) await asyncio.to_thread(self._touch_deploy_flag) + self.daemon_restart_pending = True return comp = next( (c for c in self._components if c.service in _FIRE_AND_FORGET_SERVICES), @@ -1022,6 +1025,7 @@ async def _apply_deferred_restart(self) -> None: UPDATER_SERVICE, ) await restart_service_noblock(UPDATER_SERVICE) + self.daemon_restart_pending = True except Exception: # noqa: BLE001 self._log.error("deferred restart handling failed", exc_info=True) From d804063afbb6d0ff8ce4d22161bd22f897ab2e2e Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 16:53:55 +0100 Subject: [PATCH 05/10] fix(updater): re-enable BlocksScreen.service after unit conversion --- scripts/install-updater.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/install-updater.sh b/scripts/install-updater.sh index 3921f32c..5e8e6b29 100755 --- a/scripts/install-updater.sh +++ b/scripts/install-updater.sh @@ -133,6 +133,8 @@ elif [[ "$(readlink -f "$_BS_SVC_DEST" 2>/dev/null)" != "$(readlink -f "$_BS_SVC sudo systemctl unmask BlocksScreen.service 2>/dev/null || true fi sudo systemctl daemon-reload +# A linked-but-not-enabled UI unit never starts at boot: blank screen and no SSH recovery. +sudo systemctl enable BlocksScreen.service 2>/dev/null || echo_info "WARN: could not enable BlocksScreen.service" echo_ok "BlocksScreen.service is a symlink - hook no longer needs sudo cp" echo_info "Setting up apt cache directory for blocks user ..." From c4f598f2332b94791d3b879064157c0235200de5 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 17:08:06 +0100 Subject: [PATCH 06/10] fix(updater): show overlay before MainWindow at boot provision, skip apt on restart, enable UI unit --- tests/updater/conftest.py | 3 ++ tests/updater/test_dbus_service_unit.py | 40 +++++++++++++++++++++++++ updater/dbus_service.py | 19 +++++++++--- updater/service.py | 18 +++++++---- 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/tests/updater/conftest.py b/tests/updater/conftest.py index 3afde671..aa5f751f 100644 --- a/tests/updater/conftest.py +++ b/tests/updater/conftest.py @@ -49,6 +49,8 @@ def svc(): ) mock_svc.recover = AsyncMock() mock_svc.has_fetch_failures = MagicMock(return_value=False) + mock_svc.needs_provision = MagicMock(return_value=False) + mock_svc.provision_missing = AsyncMock(return_value=False) mock_svc._components = [ ComponentConfig(name="moonraker", kind="git"), ComponentConfig(name="klipper", kind="git"), @@ -63,6 +65,7 @@ def svc(): s = UpdaterDbusService.__new__(UpdaterDbusService) s._svc = mock_svc s._busy = False + s._boot_busy = False s._background_tasks = set() s._status_check_in_progress = False s._status_pending = False diff --git a/tests/updater/test_dbus_service_unit.py b/tests/updater/test_dbus_service_unit.py index 9940a16f..495c4d69 100644 --- a/tests/updater/test_dbus_service_unit.py +++ b/tests/updater/test_dbus_service_unit.py @@ -349,6 +349,46 @@ async def test_periodic_check_never_lengthens_a_short_poll_interval(self, svc): assert sleeps == [3.0, 42.0] +class TestBootProvisionBusy: + def _build(self, missing): + from updater import dbus_service + + mock_svc = MagicMock() + mock_svc.needs_provision.return_value = missing + with ( + patch.object(dbus_service, "UpdateService", return_value=mock_svc), + patch.object(dbus_service.UpdaterDbusService, "_spawn", MagicMock()), + ): + return dbus_service.UpdaterDbusService() + + @pytest.mark.parametrize("missing", [True, False]) + def test_busy_at_construction_iff_component_missing(self, missing): + """Busy must be set before export so the UI's get_busy on connect sees the provision.""" + assert self._build(missing)._busy is missing + + @pytest.mark.asyncio + async def test_boot_busy_skips_initial_sleep_and_releases(self, svc): + """Missing component: provision runs at once (no 3 s sleep), then busy drops.""" + from updater import dbus_service + + svc._boot_busy = svc._busy = True + sleeps: list[float] = [] + + async def fake_sleep(delay): + sleeps.append(delay) + raise asyncio.CancelledError + + with ( + patch.object(dbus_service.asyncio, "sleep", fake_sleep), + pytest.raises(asyncio.CancelledError), + ): + await svc._periodic_status_check() + + assert sleeps == [svc._svc.poll_interval] + assert svc._boot_busy is False + assert svc._busy is False + + class TestMethodReturnValues: @pytest.mark.asyncio async def test_update_all_rejected_when_busy_returns_false(self, svc): diff --git a/updater/dbus_service.py b/updater/dbus_service.py index e661fbeb..8af2186e 100644 --- a/updater/dbus_service.py +++ b/updater/dbus_service.py @@ -102,7 +102,9 @@ def __init__(self) -> None: """Wire the service and busy state, then spawn the boot, poll, and self-heal tasks.""" super().__init__() self._svc = UpdateService(callback=DbusProgressCallback(self)) - self._busy: bool = False + # Busy before export so the UI's get_busy on connect sees a boot provision, not a MainWindow flash. + self._boot_busy: bool = self._svc.needs_provision() + self._busy: bool = self._boot_busy self._background_tasks: set[asyncio.Task] = set() self._status_check_in_progress: bool = False self._status_pending: bool = False @@ -129,6 +131,12 @@ def _task_done(self, task: asyncio.Task) -> None: if exc is not None: _log.error("task %r failed", task.get_name(), exc_info=exc) + def _release_boot_busy(self) -> None: + """Drop the busy state pre-set at boot; no await between this and provision's own busy(False).""" + if self._boot_busy: + self._boot_busy = False + self._set_busy(False) + def _set_busy(self, busy: bool) -> None: """Emit busy_changed only on state transitions to avoid redundant signals.""" if busy != self._busy: @@ -191,14 +199,17 @@ async def _emit_status(self, force: bool = False) -> None: async def _periodic_status_check(self) -> None: """Emit status shortly after startup, then at the poll interval - or sooner while fetches fail.""" - await asyncio.sleep(3.0) + if not self._boot_busy: + await asyncio.sleep(3.0) while True: try: + # Provision first (a no-op stat when nothing is missing) so status reflects it. + await self._svc.provision_missing(self._set_busy) + self._release_boot_busy() await self._emit_status() - if await self._svc.provision_missing(self._set_busy): - await self._emit_status() # reflect freshly-installed components except Exception as exc: # noqa: BLE001 _log.error("periodic_check failed: %s", exc) + self._release_boot_busy() interval = self._svc.poll_interval if self._svc.has_fetch_failures(): interval = min(_FETCH_RETRY_INTERVAL_S, interval) diff --git a/updater/service.py b/updater/service.py index ca4d6abb..e5fd9aaf 100644 --- a/updater/service.py +++ b/updater/service.py @@ -480,17 +480,25 @@ async def _filter_dead_branch_batch(self, batch: list[ComponentConfig]) -> bool: ) return ok - async def provision_missing( - self, on_busy: Callable[[bool], None] | None = None - ) -> bool: - """Clone absent install_if_missing components at boot; on_busy brackets the work.""" - missing = [ + def _missing_provisions(self) -> list[ComponentConfig]: + """install_if_missing components whose directory is absent.""" + return [ c for c in self._components if c.install_if_missing and c.url and (c.path is None or not c.path.exists()) ] + + def needs_provision(self) -> bool: + """True if provision_missing() would clone something (cheap filesystem check).""" + return bool(self._missing_provisions()) + + async def provision_missing( + self, on_busy: Callable[[bool], None] | None = None + ) -> bool: + """Clone absent install_if_missing components at boot; on_busy brackets the work.""" + missing = self._missing_provisions() if not missing: return False provisioned = False From 394476ddb95a6d187067e2bf975d3c102c28023b Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 17:23:54 +0100 Subject: [PATCH 07/10] fix(updater): retry boot provisioning while reconcile holds the process lock --- tests/updater/test_dbus_service_unit.py | 31 +++++++++++++++++++++++++ updater/dbus_service.py | 14 ++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/tests/updater/test_dbus_service_unit.py b/tests/updater/test_dbus_service_unit.py index 495c4d69..d5c68021 100644 --- a/tests/updater/test_dbus_service_unit.py +++ b/tests/updater/test_dbus_service_unit.py @@ -389,6 +389,37 @@ async def fake_sleep(delay): assert svc._busy is False +class TestProvisionRetry: + @pytest.mark.asyncio + async def test_retries_while_lock_defers_then_stops(self, svc): + """Deferred provisioning (lock held by boot reconcile) is retried, not left for the next poll.""" + from updater import dbus_service + + svc._svc.needs_provision = MagicMock(side_effect=[True, True, False]) + sleeps: list[float] = [] + + async def fake_sleep(delay): + sleeps.append(delay) + + with patch.object(dbus_service.asyncio, "sleep", fake_sleep): + await svc._provision_with_retry() + + assert svc._svc.provision_missing.await_count == 3 + assert sleeps == [dbus_service._PROVISION_RETRY_S] * 2 + + @pytest.mark.asyncio + async def test_gives_up_after_bounded_retries(self, svc): + """A component that never provisions must not loop forever.""" + from updater import dbus_service + + svc._svc.needs_provision = MagicMock(return_value=True) + + with patch.object(dbus_service.asyncio, "sleep", AsyncMock()): + await svc._provision_with_retry() + + assert svc._svc.provision_missing.await_count == dbus_service._PROVISION_RETRIES + + class TestMethodReturnValues: @pytest.mark.asyncio async def test_update_all_rejected_when_busy_returns_false(self, svc): diff --git a/updater/dbus_service.py b/updater/dbus_service.py index 8af2186e..94576be7 100644 --- a/updater/dbus_service.py +++ b/updater/dbus_service.py @@ -20,6 +20,9 @@ _STATUS_PATH = Path("/run/blockscreen/updater_status.json") # Poll again this soon while a git fetch is failing: a boot-time DNS miss must not hide updates for a full poll interval. _FETCH_RETRY_INTERVAL_S = 300.0 +# Boot reconcile holds the process lock briefly; provisioning is deferred, not lost. +_PROVISION_RETRIES = 10 +_PROVISION_RETRY_S = 3.0 class DbusProgressCallback: @@ -131,6 +134,15 @@ def _task_done(self, task: asyncio.Task) -> None: if exc is not None: _log.error("task %r failed", task.get_name(), exc_info=exc) + async def _provision_with_retry(self) -> None: + """Retry while boot reconcile still holds the process lock and defers provisioning.""" + for attempt in range(_PROVISION_RETRIES): + await self._svc.provision_missing(self._set_busy) + if not self._svc.needs_provision(): + return + if attempt + 1 < _PROVISION_RETRIES: + await asyncio.sleep(_PROVISION_RETRY_S) + def _release_boot_busy(self) -> None: """Drop the busy state pre-set at boot; no await between this and provision's own busy(False).""" if self._boot_busy: @@ -204,7 +216,7 @@ async def _periodic_status_check(self) -> None: while True: try: # Provision first (a no-op stat when nothing is missing) so status reflects it. - await self._svc.provision_missing(self._set_busy) + await self._provision_with_retry() self._release_boot_busy() await self._emit_status() except Exception as exc: # noqa: BLE001 From d13b4075510690a08e1675c0f4b38b6da9456869 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 17:31:28 +0100 Subject: [PATCH 08/10] feat(update): show "Missing component, installing" overlay during boot provisioning --- .../panels/widgets/MainWindow/updatePage.py | 22 +++++++++++++++++- tests/widgets/test_update_page_unit.py | 23 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py index e195308e..9f842d02 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py @@ -46,6 +46,13 @@ class UpdatePage(QtWidgets.QWidget): } ) + # Boot provisioning of a missing component reuses steps 1-4 with different meanings. + _PROVISION_STEP_LABELS: typing.ClassVar[MappingProxyType[int, str]] = ( + MappingProxyType( + {1: "cloning", 2: "installing deps", 3: "setting up", 4: "starting"} + ) + ) + _APT_STEP_LABELS: typing.ClassVar[MappingProxyType[int, str]] = MappingProxyType( {1: "updating packages", 2: "upgrading packages"} ) @@ -73,6 +80,7 @@ def __init__(self) -> None: self._post_update_status_pending: bool = False self._overlay_shown: bool = False self._restart_pending: bool = False + self._provisioning: bool = False self._elapsed_time_seconds: int = 0 self._elapsed_timer: QtCore.QTimer = QtCore.QTimer(self) self._elapsed_timer.setSingleShot(False) @@ -352,6 +360,13 @@ def handle_busy_changed(self, busy: bool) -> None: self._busy = busy self.show_loading(busy) if busy: + # Busy with no user press = the daemon is installing a missing component. + self._provisioning = not self._overlay_shown + if self._provisioning: + self._overlay_shown = True + self.call_load_panel.emit( + True, "Missing component, installing ...", False + ) self._restart_pending = False self._elapsed_time_seconds = 0 self._elapsed_timer.start() @@ -361,6 +376,7 @@ def handle_busy_changed(self, busy: bool) -> None: self._progress_label.show() self._cancel_btn.show() else: + self._provisioning = False self._elapsed_timer.stop() self._busy_timeout_timer.stop() self._elapsed_time_label.hide() @@ -439,6 +455,8 @@ def handle_step_complete(self, name: str, step: int, total: int) -> None: status = self._statuses.get(name) if status and status.kind == "apt": label = self._APT_STEP_LABELS.get(step, "working") + elif self._provisioning: + label = self._PROVISION_STEP_LABELS.get(step, "working") else: label = self._STEP_LABELS.get(step, "working") _log.info("step_complete: %s %d/%d (%s)", name, step, total, label) @@ -448,7 +466,9 @@ def handle_step_complete(self, name: str, step: int, total: int) -> None: self._overlay_shown = True # BlocksScreen's last step restarts this very process. self._restart_pending = name == "BlocksScreen" and step == total - overlay_msg = f"{name}: {label}" + overlay_msg = ( + f"Installing {name}: {label}" if self._provisioning else f"{name}: {label}" + ) self._progress_label.setText(f"Step {step}/{total}") self.call_load_panel.emit(True, overlay_msg, False) diff --git a/tests/widgets/test_update_page_unit.py b/tests/widgets/test_update_page_unit.py index 8a6ed50e..31d5b53d 100644 --- a/tests/widgets/test_update_page_unit.py +++ b/tests/widgets/test_update_page_unit.py @@ -522,3 +522,26 @@ def test_second_confirm_deletes_previous_popup(self, page): page._show_update_confirm() first.deleteLater.assert_called_once() second.deleteLater.assert_not_called() + + +class TestBootProvisioning: + def test_busy_without_user_press_shows_installing_message(self, page, qtbot): + page.show_loading = MagicMock() + with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: + page.handle_busy_changed(True) + assert blocker.args == [True, "Missing component, installing ...", False] + + def test_provision_steps_name_the_component(self, page, qtbot): + page.show_loading = MagicMock() + page.handle_busy_changed(True) + with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: + page.handle_step_complete("Spoolman", 1, 4) + assert blocker.args == [True, "Installing Spoolman: cloning", False] + + def test_user_update_keeps_update_labels(self, page, qtbot): + page.show_loading = MagicMock() + page._overlay_shown = True + page.handle_busy_changed(True) + with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: + page.handle_step_complete("klipper", 1, 4) + assert blocker.args == [True, "klipper: fetching", False] From ea80663361c929ea9ef51351b3a3dffc5f2734d3 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Tue, 29 Sep 2026 17:40:54 +0100 Subject: [PATCH 09/10] fix(update): replay busy state after wiring so boot provisioning shows its overlay text --- BlocksScreen/lib/panels/mainWindow.py | 1 + .../lib/panels/widgets/MainWindow/updatePage.py | 2 +- BlocksScreen/lib/updater_worker.py | 9 +++++++++ tests/lib/test_updater_worker_unit.py | 12 ++++++++++++ tests/widgets/test_update_page_unit.py | 6 ++++++ 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/BlocksScreen/lib/panels/mainWindow.py b/BlocksScreen/lib/panels/mainWindow.py index 28b4f74d..5bbbd0b1 100644 --- a/BlocksScreen/lib/panels/mainWindow.py +++ b/BlocksScreen/lib/panels/mainWindow.py @@ -283,6 +283,7 @@ def __init__(self): self.controlPanel.disable_popups.connect(self.popup_toggle) self.updater_worker.status_ready.connect(self.update_page.handle_status_ready) self.updater_worker.busy_changed.connect(self.update_page.handle_busy_changed) + self.updater_worker.replay_busy() self.updater_worker.daemon_unavailable.connect(self.on_updater_unavailable) self.updater_worker.daemon_unavailable.connect( self.update_page.handle_daemon_unavailable diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py index 9f842d02..f9434cf9 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py @@ -361,7 +361,7 @@ def handle_busy_changed(self, busy: bool) -> None: self.show_loading(busy) if busy: # Busy with no user press = the daemon is installing a missing component. - self._provisioning = not self._overlay_shown + self._provisioning = self._provisioning or not self._overlay_shown if self._provisioning: self._overlay_shown = True self.call_load_panel.emit( diff --git a/BlocksScreen/lib/updater_worker.py b/BlocksScreen/lib/updater_worker.py index 58ee2e1a..de23f79a 100644 --- a/BlocksScreen/lib/updater_worker.py +++ b/BlocksScreen/lib/updater_worker.py @@ -77,6 +77,8 @@ def __init__(self) -> None: self._last_activity: float = 0.0 # Unique bus name of the live daemon; a change means it restarted. self._daemon_owner: str = "" + # Latest busy state, for replay_busy(); the worker thread runs before MainWindow wires slots. + self._last_busy: bool = False self._owner_task: asyncio.Task | None = None self._escalated: bool = False # Serializes the reconnect and owner-watch entry points into _connect(). @@ -218,12 +220,18 @@ async def _connect(self) -> None: else: self._busy_false_event.set() _log.info("connected to owner %s, busy=%s", self._daemon_owner, busy) + self._last_busy = busy self.busy_changed.emit(busy) if not busy: self.request_reconnect.emit() self.proxy_connected.emit() + def replay_busy(self) -> None: + """Re-emit busy=True once slots are wired; the connect-time emit can fire before they are.""" + if self._last_busy: + self.busy_changed.emit(True) + def _on_listener_done(self, task: asyncio.Task) -> None: """Emit daemon_unavailable and schedule reconnect if a listener exits unexpectedly.""" if task.cancelled(): @@ -591,6 +599,7 @@ async def _listen_busy_changed(self) -> None: async for busy in self._proxy.busy_changed: _log.info("busy_changed received: %s", busy) self._touch_activity() + self._last_busy = busy if busy: self._busy_false_event.clear() task = asyncio.create_task(self._busy_watchdog(), name="busy_watchdog") diff --git a/tests/lib/test_updater_worker_unit.py b/tests/lib/test_updater_worker_unit.py index 0f9ba42d..3e07e474 100644 --- a/tests/lib/test_updater_worker_unit.py +++ b/tests/lib/test_updater_worker_unit.py @@ -30,6 +30,7 @@ def _make_worker(): w._last_activity = 0.0 w._proxy = MagicMock() w._shutting_down = False + w._last_busy = False w._daemon_owner = "" w._owner_task = None w._escalated = False @@ -510,3 +511,14 @@ def test_shutdown_cancels_owner_watch(self, worker): worker.shutdown() owner_task.cancel.assert_called_once() listener.cancel.assert_called_once() + + +class TestReplayBusy: + def test_replays_true_only(self, worker, qtbot): + received: list[bool] = [] + worker.busy_changed.connect(received.append) + worker.replay_busy() + assert received == [] + worker._last_busy = True + worker.replay_busy() + assert received == [True] diff --git a/tests/widgets/test_update_page_unit.py b/tests/widgets/test_update_page_unit.py index 31d5b53d..b5ab1207 100644 --- a/tests/widgets/test_update_page_unit.py +++ b/tests/widgets/test_update_page_unit.py @@ -545,3 +545,9 @@ def test_user_update_keeps_update_labels(self, page, qtbot): with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_step_complete("klipper", 1, 4) assert blocker.args == [True, "klipper: fetching", False] + + def test_replayed_busy_keeps_provisioning(self, page): + page.show_loading = MagicMock() + page.handle_busy_changed(True) + page.handle_busy_changed(True) + assert page._provisioning is True From ba12fddfc28c7de9dc98317504c76e7964a5cdb7 Mon Sep 17 00:00:00 2001 From: Guilherme Costa Date: Wed, 30 Sep 2026 16:38:26 +0100 Subject: [PATCH 10/10] fix(updater): retry provisioning only when deferred, daemon-declared install overlay, hold UI overlay on restart --- BlocksScreen/lib/panels/mainWindow.py | 3 + .../panels/widgets/MainWindow/updatePage.py | 35 ++++++----- BlocksScreen/lib/updater_worker.py | 28 ++++++++- scripts/install-updater.sh | 7 ++- tests/lib/test_updater_worker_unit.py | 25 ++++++++ tests/updater/conftest.py | 2 + tests/updater/test_dbus_service_unit.py | 44 +++++++++++++- tests/updater/test_executor_unit.py | 9 +++ tests/updater/test_service_unit.py | 60 +++++++++++++++---- tests/widgets/test_update_page_unit.py | 52 +++++++++++----- updater/dbus_service.py | 44 ++++++++++---- updater/executor.py | 15 ++--- updater/service.py | 41 ++++++++----- 13 files changed, 281 insertions(+), 84 deletions(-) diff --git a/BlocksScreen/lib/panels/mainWindow.py b/BlocksScreen/lib/panels/mainWindow.py index 5bbbd0b1..c27a1965 100644 --- a/BlocksScreen/lib/panels/mainWindow.py +++ b/BlocksScreen/lib/panels/mainWindow.py @@ -283,6 +283,9 @@ def __init__(self): self.controlPanel.disable_popups.connect(self.popup_toggle) self.updater_worker.status_ready.connect(self.update_page.handle_status_ready) self.updater_worker.busy_changed.connect(self.update_page.handle_busy_changed) + self.updater_worker.provisioning_changed.connect( + self.update_page.handle_provisioning_changed + ) self.updater_worker.replay_busy() self.updater_worker.daemon_unavailable.connect(self.on_updater_unavailable) self.updater_worker.daemon_unavailable.connect( diff --git a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py index f9434cf9..8b15e4b0 100644 --- a/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py +++ b/BlocksScreen/lib/panels/widgets/MainWindow/updatePage.py @@ -2,6 +2,7 @@ import json import logging +import re import typing from types import MappingProxyType @@ -15,6 +16,12 @@ from updater.models import ComponentStatus _log = logging.getLogger(__name__) +_DESCRIBE_SUFFIX = re.compile(r"-(\d+)-g[0-9a-f]+$") + + +def _compact_version(describe: str) -> str: + """`v1.0.0-12-gabc1234` -> `v1.0.0+12`, so commits past one tag stay distinguishable.""" + return _DESCRIBE_SUFFIX.sub(r"+\1", describe) class UpdatePage(QtWidgets.QWidget): @@ -46,7 +53,6 @@ class UpdatePage(QtWidgets.QWidget): } ) - # Boot provisioning of a missing component reuses steps 1-4 with different meanings. _PROVISION_STEP_LABELS: typing.ClassVar[MappingProxyType[int, str]] = ( MappingProxyType( {1: "cloning", 2: "installing deps", 3: "setting up", 4: "starting"} @@ -121,9 +127,6 @@ def _on_busy_timeout(self) -> None: self._overlay_shown = False self.show_loading(False) self.call_load_panel.emit(False, "", False) - self._show_toast( - "Update is taking longer than expected - tap refresh to check status" - ) def showEvent(self, a0: QtGui.QShowEvent | None) -> None: """Rebuild cards and request a fresh status poll each time the page becomes visible.""" @@ -164,8 +167,8 @@ def _version_string(self, status: ComponentStatus) -> str: return "status error" if status.kind in ("system", "apt"): return "updates available" - current = status.current_version or status.current_hash[:8] - return f"{current} → {status.remote_version or 'unknown'}" + current = _compact_version(status.current_version) or status.current_hash[:8] + return f"{current} → {_compact_version(status.remote_version) or 'unknown'}" def _make_white_label( self, @@ -360,13 +363,8 @@ def handle_busy_changed(self, busy: bool) -> None: self._busy = busy self.show_loading(busy) if busy: - # Busy with no user press = the daemon is installing a missing component. - self._provisioning = self._provisioning or not self._overlay_shown if self._provisioning: - self._overlay_shown = True - self.call_load_panel.emit( - True, "Missing component, installing ...", False - ) + self._show_provisioning_overlay() self._restart_pending = False self._elapsed_time_seconds = 0 self._elapsed_timer.start() @@ -392,6 +390,16 @@ def handle_busy_changed(self, busy: bool) -> None: QtCore.QTimer.singleShot(10000, self._dismiss_stale_overlay) self._request_status_debounced() + def handle_provisioning_changed(self, provisioning: bool) -> None: + """Daemon-declared: the current busy period installs a missing component.""" + self._provisioning = provisioning + if provisioning and self._busy: + self._show_provisioning_overlay() + + def _show_provisioning_overlay(self) -> None: + self._overlay_shown = True + self.call_load_panel.emit(True, "Missing component, installing ...", False) + def _dismiss_stale_overlay(self) -> None: """Drop the overlay if the post-update status never arrived.""" if self._overlay_shown and not self._busy: @@ -464,7 +472,6 @@ def handle_step_complete(self, name: str, step: int, total: int) -> None: if self._busy_timeout_timer.isActive(): self._busy_timeout_timer.start() self._overlay_shown = True - # BlocksScreen's last step restarts this very process. self._restart_pending = name == "BlocksScreen" and step == total overlay_msg = ( f"Installing {name}: {label}" if self._provisioning else f"{name}: {label}" @@ -519,7 +526,7 @@ def handle_daemon_unavailable(self) -> None: self._cancel_btn.hide() self.show_loading(False) self._show_toast( - "Updater unavailable. Check system logs or restart BlocksScreen.", + "Updater unavailable, restarting it automatically ...", success=False, ) self.update_all_btn.setEnabled(False) diff --git a/BlocksScreen/lib/updater_worker.py b/BlocksScreen/lib/updater_worker.py index de23f79a..291a7cba 100644 --- a/BlocksScreen/lib/updater_worker.py +++ b/BlocksScreen/lib/updater_worker.py @@ -37,7 +37,7 @@ def _dbus_daemon(bus: Any) -> Any: _UPDATER_UNIT = "BlocksScreen-updater.service" # Reconnect attempts before asking systemd to start a unit it has given up on. -_ESCALATE_AFTER = 3 +_ESCALATE_AFTER = 2 class UpdaterWorker(QtCore.QObject): @@ -55,6 +55,7 @@ class UpdaterWorker(QtCore.QObject): rollback_done = QtCore.pyqtSignal(str, bool) recover_done = QtCore.pyqtSignal(str, bool) busy_changed = QtCore.pyqtSignal(bool) + provisioning_changed = QtCore.pyqtSignal(bool) daemon_unavailable = QtCore.pyqtSignal() update_rejected = QtCore.pyqtSignal() # daemon refused the request (already busy) request_reconnect = QtCore.pyqtSignal() @@ -77,8 +78,9 @@ def __init__(self) -> None: self._last_activity: float = 0.0 # Unique bus name of the live daemon; a change means it restarted. self._daemon_owner: str = "" - # Latest busy state, for replay_busy(); the worker thread runs before MainWindow wires slots. + # For replay_busy(): this thread starts before MainWindow wires its slots. self._last_busy: bool = False + self._last_provisioning: bool = False self._owner_task: asyncio.Task | None = None self._escalated: bool = False # Serializes the reconnect and owner-watch entry points into _connect(). @@ -188,6 +190,7 @@ async def _connect(self) -> None: self._listen_rollback, self._listen_recover_done, self._listen_busy_changed, + self._listen_provisioning_changed, ] for fn in listeners: task = asyncio.create_task(fn(), name=fn.__name__) @@ -221,14 +224,26 @@ async def _connect(self) -> None: self._busy_false_event.set() _log.info("connected to owner %s, busy=%s", self._daemon_owner, busy) self._last_busy = busy + self._last_provisioning = busy and await self._get_provisioning() + self.provisioning_changed.emit(self._last_provisioning) self.busy_changed.emit(busy) if not busy: self.request_reconnect.emit() self.proxy_connected.emit() + async def _get_provisioning(self) -> bool: + """Daemons predating get_provisioning answer with an error: treat as not provisioning.""" + try: + async with asyncio.timeout(5): + return await self._proxy.get_provisioning() + except (sdbus.SdBusBaseError, TimeoutError): + return False + def replay_busy(self) -> None: - """Re-emit busy=True once slots are wired; the connect-time emit can fire before they are.""" + """Re-emit busy state once slots are wired; the connect-time emit can fire before they are.""" + if self._last_provisioning: + self.provisioning_changed.emit(True) if self._last_busy: self.busy_changed.emit(True) @@ -609,6 +624,13 @@ async def _listen_busy_changed(self) -> None: self._busy_false_event.set() self.busy_changed.emit(busy) + async def _listen_provisioning_changed(self) -> None: + """Forward provisioning_changed signals.""" + async for provisioning in self._proxy.provisioning_changed: + self._touch_activity() + self._last_provisioning = provisioning + self.provisioning_changed.emit(provisioning) + async def _busy_watchdog(self) -> None: """Emit daemon_unavailable after _BUSY_IDLE_LIMIT seconds of daemon silence. diff --git a/scripts/install-updater.sh b/scripts/install-updater.sh index 5e8e6b29..01e290f9 100755 --- a/scripts/install-updater.sh +++ b/scripts/install-updater.sh @@ -4,10 +4,12 @@ set -euo pipefail Red='\033[0;31m' Green='\033[0;32m' Blue='\033[0;34m' +Yellow='\033[0;33m' Normal='\033[0m' echo_info() { printf "${Blue}%s${Normal}\n" "$1"; } echo_ok() { printf "${Green}%s${Normal}\n" "$1"; } +echo_warn() { printf "${Yellow}%s${Normal}\n" "$1"; } echo_error() { printf "${Red}%s${Normal}\n" "$1"; } # Root and blocks both run this: O_CREAT on the other's file in sticky /tmp is denied, a read-only open is not. @@ -125,7 +127,7 @@ _BS_SVC_SRC="$BS_PATH/scripts/BlocksScreen.service" _BS_SVC_DEST="/etc/systemd/system/BlocksScreen.service" if [[ ! -f "$_BS_SVC_SRC" ]]; then # Never remove the running unit without a replacement; the device has no SSH recovery. - echo_info "WARN: $_BS_SVC_SRC missing, leaving existing BlocksScreen.service intact" + echo_warn "$_BS_SVC_SRC missing, leaving existing BlocksScreen.service intact" elif [[ "$(readlink -f "$_BS_SVC_DEST" 2>/dev/null)" != "$(readlink -f "$_BS_SVC_SRC")" ]]; then # Atomic replace via temp symlink + rename: the unit is never absent. sudo ln -sfn "$_BS_SVC_SRC" "${_BS_SVC_DEST}.new" @@ -133,8 +135,7 @@ elif [[ "$(readlink -f "$_BS_SVC_DEST" 2>/dev/null)" != "$(readlink -f "$_BS_SVC sudo systemctl unmask BlocksScreen.service 2>/dev/null || true fi sudo systemctl daemon-reload -# A linked-but-not-enabled UI unit never starts at boot: blank screen and no SSH recovery. -sudo systemctl enable BlocksScreen.service 2>/dev/null || echo_info "WARN: could not enable BlocksScreen.service" +sudo systemctl enable BlocksScreen.service 2>/dev/null || echo_warn "could not enable BlocksScreen.service" echo_ok "BlocksScreen.service is a symlink - hook no longer needs sudo cp" echo_info "Setting up apt cache directory for blocks user ..." diff --git a/tests/lib/test_updater_worker_unit.py b/tests/lib/test_updater_worker_unit.py index 3e07e474..baa9ab92 100644 --- a/tests/lib/test_updater_worker_unit.py +++ b/tests/lib/test_updater_worker_unit.py @@ -31,6 +31,7 @@ def _make_worker(): w._proxy = MagicMock() w._shutting_down = False w._last_busy = False + w._last_provisioning = False w._daemon_owner = "" w._owner_task = None w._escalated = False @@ -522,3 +523,27 @@ def test_replays_true_only(self, worker, qtbot): worker._last_busy = True worker.replay_busy() assert received == [True] + + def test_replays_provisioning_before_busy(self, worker, qtbot): + order: list[str] = [] + worker.provisioning_changed.connect(lambda v: order.append(f"prov={v}")) + worker.busy_changed.connect(lambda v: order.append(f"busy={v}")) + worker._last_busy = worker._last_provisioning = True + worker.replay_busy() + assert order == ["prov=True", "busy=True"] + + +class TestGetProvisioning: + @pytest.mark.asyncio + async def test_old_daemon_without_method_is_not_provisioning(self, worker): + import sdbus + + worker._proxy.get_provisioning = AsyncMock( + side_effect=sdbus.SdBusBaseError("unknown method") + ) + assert await worker._get_provisioning() is False + + @pytest.mark.asyncio + async def test_returns_daemon_answer(self, worker): + worker._proxy.get_provisioning = AsyncMock(return_value=True) + assert await worker._get_provisioning() is True diff --git a/tests/updater/conftest.py b/tests/updater/conftest.py index aa5f751f..a3416c6e 100644 --- a/tests/updater/conftest.py +++ b/tests/updater/conftest.py @@ -66,10 +66,12 @@ def svc(): s._svc = mock_svc s._busy = False s._boot_busy = False + s._provisioning = False s._background_tasks = set() s._status_check_in_progress = False s._status_pending = False s.busy_changed = MagicMock() + s.provisioning_changed = MagicMock() s.status_ready = MagicMock() s.error = MagicMock() return s diff --git a/tests/updater/test_dbus_service_unit.py b/tests/updater/test_dbus_service_unit.py index d5c68021..02661f32 100644 --- a/tests/updater/test_dbus_service_unit.py +++ b/tests/updater/test_dbus_service_unit.py @@ -395,7 +395,7 @@ async def test_retries_while_lock_defers_then_stops(self, svc): """Deferred provisioning (lock held by boot reconcile) is retried, not left for the next poll.""" from updater import dbus_service - svc._svc.needs_provision = MagicMock(side_effect=[True, True, False]) + svc._svc.provision_missing = AsyncMock(side_effect=[True, True, False]) sleeps: list[float] = [] async def fake_sleep(delay): @@ -407,12 +407,25 @@ async def fake_sleep(delay): assert svc._svc.provision_missing.await_count == 3 assert sleeps == [dbus_service._PROVISION_RETRY_S] * 2 + @pytest.mark.asyncio + async def test_failed_install_is_not_retried(self, svc): + """A tried-and-failed install (offline, broken unit) runs once, not 10 times.""" + from updater import dbus_service + + svc._svc.needs_provision = MagicMock(return_value=True) # dir still absent + svc._svc.provision_missing = AsyncMock(return_value=False) + + with patch.object(dbus_service.asyncio, "sleep", AsyncMock()): + await svc._provision_with_retry() + + svc._svc.provision_missing.assert_awaited_once() + @pytest.mark.asyncio async def test_gives_up_after_bounded_retries(self, svc): - """A component that never provisions must not loop forever.""" + """A lock that never frees must not loop forever.""" from updater import dbus_service - svc._svc.needs_provision = MagicMock(return_value=True) + svc._svc.provision_missing = AsyncMock(return_value=True) with patch.object(dbus_service.asyncio, "sleep", AsyncMock()): await svc._provision_with_retry() @@ -420,6 +433,31 @@ async def test_gives_up_after_bounded_retries(self, svc): assert svc._svc.provision_missing.await_count == dbus_service._PROVISION_RETRIES +class TestProvisioningFlag: + def test_provision_busy_emits_provisioning_then_busy(self, svc): + svc._provisioning = False + svc._provision_busy(True) + svc.provisioning_changed.emit.assert_called_once_with((True,)) + svc.busy_changed.emit.assert_called_once_with((True,)) + assert svc._provisioning is True + + def test_boot_release_clears_both(self, svc): + svc._boot_busy = svc._busy = svc._provisioning = True + svc._release_boot_busy() + assert (svc._busy, svc._provisioning) == (False, False) + + @pytest.mark.asyncio + async def test_get_provisioning_reports_flag(self, svc): + svc._provisioning = True + assert await svc.get_provisioning() is True + + @pytest.mark.asyncio + async def test_cancel_ignored_while_provisioning(self, svc): + svc._provisioning = svc._busy = True + await svc.cancel() + assert svc._busy is True + + class TestMethodReturnValues: @pytest.mark.asyncio async def test_update_all_rejected_when_busy_returns_false(self, svc): diff --git a/tests/updater/test_executor_unit.py b/tests/updater/test_executor_unit.py index be1ffbcc..9b5c0bcc 100644 --- a/tests/updater/test_executor_unit.py +++ b/tests/updater/test_executor_unit.py @@ -373,6 +373,15 @@ async def test_uses_custom_ref(self, tmp_path): cmd = exec_mock.call_args.args assert "origin/main" in cmd + @pytest.mark.asyncio + async def test_describes_with_tags_and_hash_fallback(self, tmp_path): + proc = _make_proc(0, b"v1.0.0-12-gabc1234\n", b"") + exec_mock = AsyncMock(return_value=proc) + with patch("asyncio.create_subprocess_exec", exec_mock): + assert await git_describe(tmp_path) == "v1.0.0-12-gabc1234" + cmd = exec_mock.call_args.args + assert "--tags" in cmd and "--always" in cmd + class TestGitResetToHash: @pytest.mark.asyncio diff --git a/tests/updater/test_service_unit.py b/tests/updater/test_service_unit.py index 367932c7..1bbbf183 100644 --- a/tests/updater/test_service_unit.py +++ b/tests/updater/test_service_unit.py @@ -341,6 +341,7 @@ async def test_emits_step_progress_in_order(self, tmp_path): call("klipper", 2, 4), call("klipper", 3, 4), call("klipper", 4, 4), + call("BlocksScreen", 4, 4), # restart_ui: UI holds its overlay ] @pytest.mark.asyncio @@ -1758,7 +1759,7 @@ async def test_provision_waits_for_service_active(self, tmp_path): patch( "updater.service.wait_for_service_active", return_value=False ) as mock_wait, - patch("updater.service.stop_service", return_value=(True, "")) as mock_stop, + patch("updater.service.disable_service", return_value=(True, "")) as mock_stop, patch("updater.service.shutil.rmtree") as mock_rmtree, ): svc = UpdateService(callback=cb) @@ -1792,7 +1793,7 @@ async def test_provision_fails_when_health_check_fails(self, tmp_path): patch( "updater.service.wait_for_http_ready", return_value=False ) as mock_health, - patch("updater.service.stop_service", return_value=(True, "")) as mock_stop, + patch("updater.service.disable_service", return_value=(True, "")) as mock_stop, patch("updater.service.shutil.rmtree") as mock_rmtree, ): svc = UpdateService(callback=cb) @@ -1961,10 +1962,21 @@ async def test_provisions_absent_opted_in_component(self, tmp_path): ): svc = UpdateService() svc._components = [comp] - did = await svc.provision_missing() - assert did is True + deferred = await svc.provision_missing() + assert deferred is False mock_prov.assert_awaited_once_with(comp) + @pytest.mark.asyncio + async def test_failed_install_is_not_reported_as_deferred(self, tmp_path): + comp = self._comp(tmp_path) + with ( + patch("updater.service.process_lock", lambda: nullcontext(True)), + patch.object(UpdateService, "_provision_component", return_value=False), + ): + svc = UpdateService() + svc._components = [comp] + assert await svc.provision_missing() is False + @pytest.mark.asyncio async def test_present_component_is_never_provisioned(self, tmp_path): comp = self._comp(tmp_path) @@ -1975,8 +1987,8 @@ async def test_present_component_is_never_provisioned(self, tmp_path): ): svc = UpdateService() svc._components = [comp] - did = await svc.provision_missing() - assert did is False + deferred = await svc.provision_missing() + assert deferred is False mock_prov.assert_not_called() @pytest.mark.asyncio @@ -1989,8 +2001,8 @@ async def test_not_opted_in_is_skipped(self, tmp_path): ): svc = UpdateService() svc._components = [comp] - did = await svc.provision_missing() - assert did is False + deferred = await svc.provision_missing() + assert deferred is False mock_prov.assert_not_called() @pytest.mark.asyncio @@ -2003,8 +2015,8 @@ async def test_defers_when_process_lock_held(self, tmp_path): ): svc = UpdateService() svc._components = [comp] - did = await svc.provision_missing() - assert did is False + deferred = await svc.provision_missing() + assert deferred is True mock_prov.assert_not_called() @@ -2409,13 +2421,39 @@ async def test_code_restarts_only_when_importable(self, tmp_path: Path): "updater.service.verify_updater_importable", new=AsyncMock(return_value=True), ), - patch("updater.service.restart_service_noblock") as mock_restart, + patch( + "updater.service.restart_service_noblock", return_value=(True, "") + ) as mock_restart, ): svc = self._svc_with_ui() await svc._apply_deferred_restart() mock_restart.assert_called_once_with("BlocksScreen-updater.service") assert svc.daemon_restart_pending is True + @pytest.mark.asyncio + async def test_failed_restart_request_is_not_pending(self, tmp_path: Path): + sentinel = tmp_path / "updater-restart-needed" + sentinel.write_text("code\n") + with ( + patch("updater.service.restart_sentinel_path", return_value=sentinel), + patch( + "updater.service.verify_updater_importable", + new=AsyncMock(return_value=True), + ), + patch( + "updater.service.restart_service_noblock", return_value=(False, "x") + ), + ): + svc = self._svc_with_ui() + await svc._apply_deferred_restart() + assert svc.daemon_restart_pending is False + + def test_restart_pending_expires(self): + svc = self._svc_with_ui() + svc._mark_restart_pending() + with patch("updater.service.time.monotonic", return_value=1e12): + assert svc.daemon_restart_pending is False + @pytest.mark.asyncio async def test_code_skips_restart_when_not_importable(self, tmp_path: Path): """Brick-guard: a broken new updater must not restart the daemon.""" diff --git a/tests/widgets/test_update_page_unit.py b/tests/widgets/test_update_page_unit.py index b5ab1207..80d75eb2 100644 --- a/tests/widgets/test_update_page_unit.py +++ b/tests/widgets/test_update_page_unit.py @@ -11,12 +11,8 @@ def page(qapp): """UpdatePage instance with all heavy UI deps mocked.""" patches = [ - patch( - "BlocksScreen.lib.panels.widgets.MainWindow.updatePage.LoadingOverlayWidget" - ), - patch( - "BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BlocksCustomButton" - ), + patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.LoadingOverlayWidget"), + patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BlocksCustomButton"), patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.IconButton"), ] for p in patches: @@ -138,6 +134,12 @@ def test_git_falls_back_to_unknown_when_no_remote(self, page): s = _make_status(current_version="v0.1.0", remote_version="") assert page._version_string(s) == "v0.1.0 → unknown" + def test_same_tag_commits_ahead_stay_distinguishable(self, page): + s = _make_status( + current_version="v1.0.0-12-gabc1234", remote_version="v1.0.0-15-gdef5678" + ) + assert page._version_string(s) == "v1.0.0+12 → v1.0.0+15" + def test_system_returns_updates_available(self, page): s = _make_status(kind="system", packages_upgradable=12) assert page._version_string(s) == "updates available" @@ -427,13 +429,13 @@ class TestHandleStepComplete: def test_emits_call_load_panel_with_step_message(self, page, qtbot): with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_step_complete("klipper", 1, 4) - assert blocker.args == [True, "klipper: fetching", False] + assert blocker.args == [True, "klipper: fetching",False] page._progress_label.setText.assert_called_with("Step 1/4") def test_unknown_steps_falls_back_to_working(self, page, qtbot): with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_step_complete("moonraker", 99, 4) - assert blocker.args == [True, "moonraker: working", False] + assert blocker.args == [True, "moonraker: working",False] page._progress_label.setText.assert_called_with("Step 99/4") @@ -512,9 +514,7 @@ def test_bad_payload_keeps_statuses_and_toasts(self, page): class TestConfirmPopupCleanup: def test_second_confirm_deletes_previous_popup(self, page): - with patch( - "BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BasePopup" - ) as popup_cls: + with patch("BlocksScreen.lib.panels.widgets.MainWindow.updatePage.BasePopup") as popup_cls: first = MagicMock() second = MagicMock() popup_cls.side_effect = [first, second] @@ -525,14 +525,29 @@ def test_second_confirm_deletes_previous_popup(self, page): class TestBootProvisioning: - def test_busy_without_user_press_shows_installing_message(self, page, qtbot): + def test_declared_provisioning_shows_installing_message(self, page, qtbot): page.show_loading = MagicMock() + page.handle_provisioning_changed(True) with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_busy_changed(True) assert blocker.args == [True, "Missing component, installing ...", False] + def test_provisioning_after_busy_still_shows_message(self, page, qtbot): + page.show_loading = MagicMock() + page.handle_busy_changed(True) + with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: + page.handle_provisioning_changed(True) + assert blocker.args == [True, "Missing component, installing ...", False] + + def test_undeclared_busy_is_not_an_install(self, page, qtbot): + page.show_loading = MagicMock() + with qtbot.assertNotEmitted(page.call_load_panel, wait=200): + page.handle_busy_changed(True) + assert page._provisioning is False + def test_provision_steps_name_the_component(self, page, qtbot): page.show_loading = MagicMock() + page.handle_provisioning_changed(True) page.handle_busy_changed(True) with qtbot.waitSignal(page.call_load_panel, timeout=200) as blocker: page.handle_step_complete("Spoolman", 1, 4) @@ -546,8 +561,15 @@ def test_user_update_keeps_update_labels(self, page, qtbot): page.handle_step_complete("klipper", 1, 4) assert blocker.args == [True, "klipper: fetching", False] - def test_replayed_busy_keeps_provisioning(self, page): + def test_provisioning_clears_when_busy_ends(self, page): page.show_loading = MagicMock() + page.handle_provisioning_changed(True) page.handle_busy_changed(True) - page.handle_busy_changed(True) - assert page._provisioning is True + page.handle_busy_changed(False) + assert page._provisioning is False + + +class TestRestartPending: + def test_ui_restart_step_holds_overlay(self, page): + page.handle_step_complete("BlocksScreen", 4, 4) + assert page._restart_pending is True diff --git a/updater/dbus_service.py b/updater/dbus_service.py index 94576be7..1b40d8a8 100644 --- a/updater/dbus_service.py +++ b/updater/dbus_service.py @@ -20,7 +20,7 @@ _STATUS_PATH = Path("/run/blockscreen/updater_status.json") # Poll again this soon while a git fetch is failing: a boot-time DNS miss must not hide updates for a full poll interval. _FETCH_RETRY_INTERVAL_S = 300.0 -# Boot reconcile holds the process lock briefly; provisioning is deferred, not lost. +# Retries while boot reconcile holds the process lock. _PROVISION_RETRIES = 10 _PROVISION_RETRY_S = 3.0 @@ -101,13 +101,19 @@ def busy_changed(self) -> tuple[bool]: """Emitted on True↔False transition only (state-machine guard).""" raise NotImplementedError + @sdbus.dbus_signal_async("b") + def provisioning_changed(self) -> tuple[bool]: + """Emitted on True↔False transition while a missing component is being installed.""" + raise NotImplementedError + def __init__(self) -> None: """Wire the service and busy state, then spawn the boot, poll, and self-heal tasks.""" super().__init__() self._svc = UpdateService(callback=DbusProgressCallback(self)) - # Busy before export so the UI's get_busy on connect sees a boot provision, not a MainWindow flash. + # Set before export so the UI's first get_busy sees a boot install. self._boot_busy: bool = self._svc.needs_provision() self._busy: bool = self._boot_busy + self._provisioning: bool = self._boot_busy self._background_tasks: set[asyncio.Task] = set() self._status_check_in_progress: bool = False self._status_pending: bool = False @@ -135,19 +141,26 @@ def _task_done(self, task: asyncio.Task) -> None: _log.error("task %r failed", task.get_name(), exc_info=exc) async def _provision_with_retry(self) -> None: - """Retry while boot reconcile still holds the process lock and defers provisioning.""" - for attempt in range(_PROVISION_RETRIES): - await self._svc.provision_missing(self._set_busy) - if not self._svc.needs_provision(): + """Retry only while boot reconcile's process lock defers provisioning.""" + for _ in range(_PROVISION_RETRIES): + if not await self._svc.provision_missing(self._provision_busy): return - if attempt + 1 < _PROVISION_RETRIES: - await asyncio.sleep(_PROVISION_RETRY_S) + await asyncio.sleep(_PROVISION_RETRY_S) + + def _provision_busy(self, busy: bool) -> None: + self._set_provisioning(busy) + self._set_busy(busy) def _release_boot_busy(self) -> None: - """Drop the busy state pre-set at boot; no await between this and provision's own busy(False).""" + """Drop the state pre-set at boot.""" if self._boot_busy: self._boot_busy = False - self._set_busy(False) + self._provision_busy(False) + + def _set_provisioning(self, provisioning: bool) -> None: + if provisioning != self._provisioning: + self._provisioning = provisioning + self.provisioning_changed.emit((provisioning,)) def _set_busy(self, busy: bool) -> None: """Emit busy_changed only on state transitions to avoid redundant signals.""" @@ -215,7 +228,6 @@ async def _periodic_status_check(self) -> None: await asyncio.sleep(3.0) while True: try: - # Provision first (a no-op stat when nothing is missing) so status reflects it. await self._provision_with_retry() self._release_boot_busy() await self._emit_status() @@ -299,7 +311,7 @@ async def _run_update_all(self) -> None: ) # Silent apt pass only if we held the lock; else the CLI run owns apt. if ran and self._svc.daemon_restart_pending: - # A SIGKILL from the restart could land inside dpkg; the next poll re-offers the packages. + # A restart SIGKILL could land inside dpkg. _log.info("background apt upgrade skipped: daemon restart pending") elif ran: self._spawn( @@ -348,9 +360,17 @@ async def get_busy(self) -> bool: """D-Bus method: return current busy state so reconnecting clients can sync.""" return self._busy + @sdbus.dbus_method_async(result_signature="b") + async def get_provisioning(self) -> bool: + """D-Bus method: True while a missing component is being installed.""" + return self._provisioning + @sdbus.dbus_method_async() async def cancel(self) -> None: """D-Bus method: cancel the running update or recover task and wait for cleanup.""" + if self._provisioning: + _log.info("cancel() ignored: component install in progress") + return cancelled_tasks: list[asyncio.Task] = [] for task in list(self._background_tasks): name = task.get_name() diff --git a/updater/executor.py b/updater/executor.py index 3e9e7c35..b9a08217 100644 --- a/updater/executor.py +++ b/updater/executor.py @@ -719,8 +719,8 @@ async def git_default_branch(path: Path | None) -> str: async def git_describe(path: Path, ref: str | None = None) -> str: - """Return the nearest tag for ref (or HEAD), or empty string.""" - cmd = [GIT, "describe", "--tags", "--abbrev=0"] + """Return `tag-N-gHASH` (or a bare hash without tags) for ref or HEAD; empty on error.""" + cmd = [GIT, "describe", "--tags", "--always"] if ref: cmd.append(ref) ok, output = await _run(cmd, cwd=path, timeout=10.0) @@ -1051,10 +1051,7 @@ async def wait_for_http_ready( logger.info("health check ok: %s", url) return True # A crash-looping unit is 'activating', never 'active': don't wait out the timeout. - if ( - service - and not (await _run([SYSTEMCTL, "is-active", service], timeout=10.0))[0] - ): + if service and not await is_service_active(service): logger.warning("service %r left active during health check", service) return False if asyncio.get_running_loop().time() >= deadline: @@ -1086,13 +1083,13 @@ async def verify_updater_importable(component_path: Path | None) -> bool: return ok -async def stop_service(name: str | None) -> tuple[bool, str]: - """Stop a systemd service.""" +async def disable_service(name: str | None) -> tuple[bool, str]: + """Stop and disable a systemd service.""" if name is None: return (False, "service name is None") if not _SERVICE_RE.match(name): return (False, f"service name {name!r} is invalid") - return await _run([SUDO, SYSTEMCTL, "stop", name], timeout=30.0) + return await _run([SUDO, SYSTEMCTL, "disable", "--now", name], timeout=30.0) async def restart_service(name: str | None) -> tuple[bool, str]: diff --git a/updater/service.py b/updater/service.py index e5fd9aaf..c2258567 100644 --- a/updater/service.py +++ b/updater/service.py @@ -33,6 +33,7 @@ check_apt_status, check_git_status, classify_apt_error, + disable_service, enable_service, git_checkout, git_clone, @@ -51,7 +52,6 @@ restart_service, restart_service_noblock, run_hook, - stop_service, verify_updater_importable, wait_for_http_ready, wait_for_service_active, @@ -167,6 +167,8 @@ def reset(self) -> None: # Self-heal: the UI component name (components.yaml) that the supervisor watches. _UI_COMPONENT = "BlocksScreen" +# A requested daemon restart that has not happened by now is assumed lost. +_RESTART_PENDING_TTL_S = 600.0 # Marker file proving updater exists: absence at target ref aborts update (lack bricks Type=notify host with no self-heal). _UPDATER_MARKER = "updater/dbus_service.py" # Forward-heal always targets the curated-stable channel, not the configured branch. @@ -260,8 +262,15 @@ def __init__(self, callback: ProgressCallback | None = None) -> None: self._log = logging.getLogger("updater") # Self-heal: trailing-window sample ring for crash-loop detection. self._nrestarts_samples: dict[str, list[tuple[float, int]]] = {} - # Set once this daemon is about to be stopped, so no apt child gets SIGKILLed with it. - self.daemon_restart_pending = False + self._restart_pending_until = 0.0 + + @property + def daemon_restart_pending(self) -> bool: + """True while this daemon is about to be stopped, so no apt child gets SIGKILLed with it.""" + return time.monotonic() < self._restart_pending_until + + def _mark_restart_pending(self) -> None: + self._restart_pending_until = time.monotonic() + _RESTART_PENDING_TTL_S def has_component(self, name: str) -> bool: """Return True if a component with the given name is registered.""" @@ -497,26 +506,24 @@ def needs_provision(self) -> bool: async def provision_missing( self, on_busy: Callable[[bool], None] | None = None ) -> bool: - """Clone absent install_if_missing components at boot; on_busy brackets the work.""" + """Clone absent install_if_missing components; True if deferred by a held lock.""" missing = self._missing_provisions() if not missing: return False - provisioned = False with process_lock() as acquired: if not acquired: self._log.info("provision_missing: update in progress, deferring") - return False + return True if on_busy: - on_busy(True) # UI shows step_complete only while busy + on_busy(True) try: for c in missing: if c.path is None or not c.path.exists(): # recheck under lock await self._provision_component(c) - provisioned = True finally: if on_busy: on_busy(False) - return provisioned + return False async def _preflight_fetch( self, sorted_components: list[ComponentConfig] @@ -802,6 +809,8 @@ async def _finalize_git_batch( ui_services.add(c.service) # klipper/RF50 hold config the UI reads at startup: refresh it too. if any(c.restart_ui for c in alive): + if _UI_SERVICE not in ui_services: + self._cb("on_step", _UI_COMPONENT, 4, 4) # UI holds its overlay ui_services.add(_UI_SERVICE) for svc in ui_services: self._log.info("git batch: fire-and-forget restart of %s (no wait)", svc) @@ -1015,7 +1024,7 @@ async def _apply_deferred_restart(self) -> None: "(install-updater runs out-of-band)" ) await asyncio.to_thread(self._touch_deploy_flag) - self.daemon_restart_pending = True + self._mark_restart_pending() return comp = next( (c for c in self._components if c.service in _FIRE_AND_FORGET_SERVICES), @@ -1032,8 +1041,11 @@ async def _apply_deferred_restart(self) -> None: "deferred: updater code changed, clean self-restart of %s", UPDATER_SERVICE, ) - await restart_service_noblock(UPDATER_SERVICE) - self.daemon_restart_pending = True + ok, err = await restart_service_noblock(UPDATER_SERVICE) + if ok: + self._mark_restart_pending() + else: + self._log.error("daemon restart request failed: %s", err) except Exception: # noqa: BLE001 self._log.error("deferred restart handling failed", exc_info=True) @@ -1718,8 +1730,8 @@ async def _remove_clone(self, component: ComponentConfig) -> None: async def _fail_provision(self, component: ComponentConfig, reason: str) -> bool: """Remove the partial clone, log, and report failure.""" if component.service and reason in ("hook", "restart"): - # Else systemd crash-loops the unit on the deleted dir until StartLimit. - await stop_service(component.service) + # The hook may have enabled it: it would crash-loop on the deleted dir. + await disable_service(component.service) await self._remove_clone(component) self._history("install_failed", component.name, reason=reason) self._log.warning( @@ -2170,6 +2182,7 @@ async def _fire_and_forget_restart(self, component: ComponentConfig) -> None: component.name, _UI_SERVICE, ) + self._cb("on_step", _UI_COMPONENT, 4, 4) # UI holds its overlay await restart_service_noblock(_UI_SERVICE) async def _run_git_update(self, component: ComponentConfig) -> bool: