From b7909cac6a8cc5515705a548baa15355f2fd4da2 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:51:46 -0400 Subject: [PATCH] fix(update): leave plugins alone when update_core's own rollback fails update_core returns rollback_failed directly when a partial pull or an update whose health check never started cannot be rolled back. run() only held plugins back for 'verifying', so those devices still got new plugin versions and a display restart on top of a core in an unknown state -- the opposite of what the health-check path does, and of the 3.4.0 changelog (plugins are left alone if the rollback fails). Co-Authored-By: Claude Opus 5 --- test/test_auto_update.py | 30 ++++++++++++++++++++++++++++++ web_interface/auto_update.py | 9 +++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/test/test_auto_update.py b/test/test_auto_update.py index 8b693f5d..8acb1e8b 100644 --- a/test/test_auto_update.py +++ b/test/test_auto_update.py @@ -414,6 +414,36 @@ class TestUpdateIsVerified: assert repo.head() == old assert 'rolled back' in result['core_message'] + @pytest.mark.parametrize('how', ['partial_pull', 'check_never_started']) + def test_a_failed_rollback_leaves_plugins_and_the_display_alone(self, tmp_path, how): + """update_core returns rollback_failed itself on two paths. The device + is then in an unknown state, exactly as when the health check reports + rollback_failed, so no plugin updates and no restart may follow.""" + repo = Repo(tmp_path) + repo.publish() + store = FakeStore(tmp_path / 'plugins', {'clock': '1.0.0'}, bump={'clock'}) + pull = real_pull(repo.device) + + def half_failed(**kwargs): + pull() + return {'status': 'error', 'message': 'Update failed: interrupted'} + h = Harness(tmp_path, repo, store=store, pickup=how != 'check_never_started', + core_update=half_failed if how == 'partial_pull' else None) + real_run = h.updater.run_command + + def reset_fails(args, **kwargs): + if args[:3] == ['git', 'reset', '--hard']: + return subprocess.CompletedProcess(args, 1, stdout='', stderr='index.lock exists') + return real_run(args, **kwargs) + h.updater.run_command = reset_fails + + result = h.updater.run() + assert result['core_outcome'] == 'rollback_failed' + assert store.updated_calls == [], "plugins must not pile onto a core in an unknown state" + assert h.restarts == [] + assert h.state['plugins_pending'] is False and result['plugins_deferred'] is False + assert h.state['alert'] + # -- reporting the health check's outcome -------------------------------------- diff --git a/web_interface/auto_update.py b/web_interface/auto_update.py index ce76d2e9..129f8d15 100644 --- a/web_interface/auto_update.py +++ b/web_interface/auto_update.py @@ -346,11 +346,16 @@ class AutoUpdater: core = {'outcome': 'error', 'message': f'The LEDMatrix update failed unexpectedly: {e}'} deferred = core['outcome'] == 'verifying' - updated, failed = ([], []) if deferred else self._update_plugins() + # A core whose rollback failed is in an unknown state: as when the + # health check reports rollback_failed, plugins are left alone and + # nothing is restarted onto it. + stranded = core['outcome'] == 'rollback_failed' + updated, failed = ([], []) if deferred or stranded else self._update_plugins() self._store_run(state, core, updated, failed) logger.info("Automatic update: core %s (%s); plugins updated=%s failed=%s%s", core['outcome'], core['message'], updated, failed, - '; plugins wait for the health check' if deferred else '') + '; plugins wait for the health check' if deferred + else '; plugins left alone' if stranded else '') # Code restarts belong to the health check; this only covers plugins. if updated: