fix: a Vegas static pause survives a non-numeric display duration; aliased store installs ask for a restart (#753)

* fix(vegas): a display duration that is not a number no longer cancels a static pause

The Vegas static pause compared plugin.get_display_duration() with the
clock. clock-simple, calendar and countdown return their display_duration
setting straight from config.json, so a value saved as "20" or null
reached that comparison as a string or None. The TypeError went to the
pause's broad except, which ended the pause: the plugin flashed up and
the scroll went straight on, at every one of its turns. inf held the
pause until something interrupted it, and NaN, False, 0 or a negative
number ended it at once.

The pause now reads the duration the way the rotation has since #739,
with the same helper, then the rotation's fallbacks: 30 s for anything
that is not a number or a get_display_duration() that raises, 15 s for a
number at or below zero. Logged once per plugin. test_vegas_static_mode.py's
pauses used 0 to mean "no wait"; they now use 0.01.

The helper moves from display_controller (_finite_seconds) to base_plugin
(finite_seconds), unchanged: the coordinator cannot import from
display_controller, which imports src.vegas_mode at module level, and a
new src module would turn ledmatrix-plugins' min-core table check red
until it was listed. base_plugin is already loaded whenever either one is.

Tests: test/test_vegas_static_pause_duration.py, on a fake clock,
including TestSameAsTheRotation, which runs every value through both the
pause and the rotation's _get_display_duration/_resolve_durations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(web): a store install asks for a restart by the id it installed as

POST /plugins/install decides restart_required from whether config.json
already enables the plugin: the display loads a plugin when its enabled
flag changes, so one already enabled (a reinstall, or a config carried
over) keeps running the copy it loaded until a restart. The route read
that flag under the registry id. Weather, Music, Stocks and Leaderboard
install under the id their manifests declare (weather ->
ledmatrix-weather), which is the config section's id, so reinstalling an
enabled one never reported that a restart was needed.

Both the queued and the direct path now look up the installed id once
(#746's _installed_plugin_id) and use it for the plugin_id they answer
with and for the enabled check.

Tests: test/test_api_v3_install_restart_installed_id.py, through the
Flask test client, both paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(vegas): type the static pause fallback on its own (mypy ratchet)

_static_pause_duration assigned the fallback to `seconds`, which the
except branch typed as float before finite_seconds() reassigned it as
float | None. A separate `fallback` keeps both types exact; behaviour is
unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

test/test_api_v3_install_restart_installed_id.py

@@ -0,0 +1,123 @@
+"""POST /plugins/install asks for a restart by the id the plugin installed as.
+
+A store install needs a display restart when config.json already enables the
+plugin (a reinstall, or a config carried over): the display loads a plugin
+when its ``enabled`` flag changes, and this flag did not. The route read the
+flag under the registry id it was given. An aliased entry installs under
+another id -- ``weather`` installs a directory whose manifest declares
+``ledmatrix-weather``, and its config section is ``ledmatrix-weather`` -- so
+reinstalling an enabled Weather never reported that a restart was needed,
+and the display kept running the old copy.
+"""
+
+import json
+from unittest.mock import MagicMock
+
+import pytest
+
+from test._api_v3_test_helpers import api_v3_client, api_v3_module  # noqa: F401
+
+INSTALL = "/api/v3/plugins/install"
+
+
+@pytest.fixture
+def store(api_v3_module, tmp_path):
+    """The store installs registry entry ``weather`` as ``installed_id``."""
+    manager = api_v3_module.api_v3.plugin_store_manager
+    manager.install_plugin.return_value = True
+    manager.get_registry_info.return_value = None
+    manager._find_plugin_path.return_value = None
+
+    def installs_as(installed_id):
+        path = tmp_path / installed_id
+        path.mkdir()
+        (path / "manifest.json").write_text(json.dumps({"id": installed_id}),
+                                            encoding="utf-8")
+        manager._find_plugin_path.side_effect = (
+            lambda pid: path if pid == "weather" else None)
+
+    manager.installs_as = installs_as
+    return manager
+
+
+@pytest.fixture
+def config(api_v3_module):
+    """config.json with an ``enabled`` flag for each plugin id given."""
+    def sections(enabled):
+        api_v3_module.api_v3.config_manager.load_config.return_value = {
+            plugin_id: {"enabled": flag} for plugin_id, flag in enabled.items()}
+    return sections
+
+
+@pytest.fixture
+def queued(api_v3_module):
+    queue = MagicMock()
+
+    def enqueue(operation_type, plugin_id, operation_callback=None):
+        queue.callback_result = operation_callback(MagicMock())
+        return "op-1"
+
+    queue.enqueue_operation.side_effect = enqueue
+    api_v3_module.api_v3.operation_queue = queue
+    return queue
+
+
+def _direct(client):
+    return client.post(INSTALL, json={"plugin_id": "weather"}).get_json()
+
+
+def _queued(client, queue):
+    client.post(INSTALL, json={"plugin_id": "weather"})
+    return queue.callback_result
+
+
+class TestDirectInstall:
+    def test_an_aliased_install_enabled_under_its_installed_id_asks_for_a_restart(
+            self, api_v3_client, store, config):
+        store.installs_as("ledmatrix-weather")
+        config({"ledmatrix-weather": True})
+        body = _direct(api_v3_client)
+        assert body["status"] == "success"
+        assert body["restart_required"] is True
+        assert body["restart_message"]
+
+    def test_an_enabled_section_under_the_registry_id_alone_does_not(
+            self, api_v3_client, store, config):
+        """The display knows the plugin as ledmatrix-weather; nothing runs
+        under a section called weather."""
+        store.installs_as("ledmatrix-weather")
+        config({"weather": True})
+        assert _direct(api_v3_client)["restart_required"] is False
+
+    def test_an_aliased_install_that_is_not_enabled_needs_no_restart(
+            self, api_v3_client, store, config):
+        store.installs_as("ledmatrix-weather")
+        config({"ledmatrix-weather": False})
+        assert _direct(api_v3_client)["restart_required"] is False
+
+    def test_an_install_under_its_own_id_is_unchanged(self, api_v3_client, store, config):
+        store.installs_as("weather")
+        config({"weather": True})
+        assert _direct(api_v3_client)["restart_required"] is True
+
+    def test_an_install_that_cannot_be_found_uses_the_requested_id(
+            self, api_v3_client, store, config):
+        config({"weather": True})
+        assert _direct(api_v3_client)["restart_required"] is True
+
+
+class TestQueuedInstall:
+    def test_an_aliased_install_enabled_under_its_installed_id_asks_for_a_restart(
+            self, api_v3_client, store, config, queued):
+        store.installs_as("ledmatrix-weather")
+        config({"ledmatrix-weather": True})
+        result = _queued(api_v3_client, queued)
+        assert result["success"] is True
+        assert result["restart_required"] is True
+        assert result["restart_message"]
+
+    def test_an_enabled_section_under_the_registry_id_alone_does_not(
+            self, api_v3_client, store, config, queued):
+        store.installs_as("ledmatrix-weather")
+        config({"weather": True})
+        assert _queued(api_v3_client, queued)["restart_required"] is False

test/test_vegas_static_mode.py

@@ -219,7 +219,8 @@ class TestCoordinatorStaticPause:
     def _plugin(self):
         plugin = MagicMock()
         plugin.plugin_id = 'clock'
-        plugin.get_display_duration.return_value = 0
+        # A moment: zero would pause 15 s, as the rotation shows it.
+        plugin.get_display_duration.return_value = 0.01
         return plugin
 
     def test_trigger_comes_from_the_pipeline(self):

test/test_vegas_static_pause_duration.py

@@ -0,0 +1,197 @@
+"""A Vegas static pause lasts as long as the rotation shows the plugin.
+
+The pause asked the plugin for get_display_duration() and compared the
+answer with the clock. Several plugins (clock-simple, calendar, countdown)
+return their display_duration setting as it is in config.json, so one saved
+as "20" or null -- the raw config editor, a hand edit -- reached that
+comparison as a string or None. The TypeError went to the pause's broad
+except, which ended the pause: the plugin flashed up and the scroll went on,
+at every one of its turns. inf paused until something interrupted it, and
+NaN, False, 0 or a negative number ended the pause at once.
+
+The pause now reads the answer the way the rotation does since #739, with
+the same helper (base_plugin.finite_seconds): a numeric string counts;
+anything else that is not a finite number, or a raise, gets the rotation's
+30 s; a number at or below zero gets its 15 s.
+"""
+
+import logging
+import os
+import threading
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+os.environ.setdefault("EMULATOR", "true")
+
+import pytest
+
+from src.vegas_mode import coordinator
+
+NOT_NUMBERS = [None, '', 'twenty', True, False, float('nan'), float('inf'),
+               'inf', '1e400', [20], {'seconds': 20}]
+NOT_ABOVE_ZERO = [0, -5, '-5', '0']
+NUMBERS = [('20', 20.0), (' 7.5 ', 7.5), (12, 12.0), (12.5, 12.5)]
+
+
+class FakeClock:
+    """time.monotonic/time.sleep for the pause loop: sleeping moves the clock."""
+
+    #: A pause still going after this long never ends (inf did that).
+    LIMIT = 3600.0
+
+    def __init__(self):
+        self.now = 0.0
+
+    def monotonic(self):
+        return self.now
+
+    def sleep(self, seconds):
+        self.now += seconds
+        if self.now > self.LIMIT:
+            raise RuntimeError("the static pause never ended")
+
+
+@pytest.fixture
+def clock(monkeypatch):
+    fake = FakeClock()
+    monkeypatch.setattr(coordinator, 'time', fake)
+    return fake
+
+
+def _plugin(duration, plugin_id='clock-simple'):
+    plugin = MagicMock()
+    plugin.plugin_id = plugin_id
+    plugin.get_display_duration.return_value = duration
+    return plugin
+
+
+def _coord(*plugins):
+    coord = coordinator.VegasModeCoordinator.__new__(coordinator.VegasModeCoordinator)
+    coord.render_pipeline = MagicMock()
+    coord.render_pipeline.get_scroll_position.return_value = 0
+    coord.display_manager = MagicMock()
+    locks = {plugin.plugin_id: threading.Lock() for plugin in plugins}
+    coord.plugin_manager = SimpleNamespace(get_plugin_lock=locks.__getitem__)
+    coord._state_lock = threading.Lock()
+    coord._static_pause_active = False
+    coord._saved_scroll_position = None
+    coord._should_stop = False
+    coord._live_priority_active = False
+    coord._live_priority_check = None
+    coord._interrupt_check = None
+    coord.stats = {'static_pauses': 0}
+    return coord
+
+
+def _pause(coord, plugin, clock):
+    """One static pause: (whether it completed, how long it lasted)."""
+    start = clock.now
+    completed = coord._handle_static_pause(plugin)
+    return completed, clock.now - start
+
+
+class TestPauseLength:
+    @pytest.mark.parametrize('value, seconds', NUMBERS)
+    def test_numbers_and_numeric_strings_are_used(self, clock, value, seconds):
+        plugin = _plugin(value)
+        completed, lasted = _pause(_coord(plugin), plugin, clock)
+        assert completed is True
+        assert lasted == pytest.approx(seconds, abs=0.15)
+
+    @pytest.mark.parametrize('value', NOT_NUMBERS, ids=repr)
+    def test_anything_but_a_finite_number_pauses_for_30s(self, clock, value):
+        plugin = _plugin(value)
+        completed, lasted = _pause(_coord(plugin), plugin, clock)
+        assert completed is True
+        assert lasted == pytest.approx(30.0, abs=0.15)
+        plugin.display.assert_called_once_with(force_clear=True)
+
+    @pytest.mark.parametrize('value', NOT_ABOVE_ZERO, ids=repr)
+    def test_a_number_not_above_zero_pauses_for_15s(self, clock, value):
+        plugin = _plugin(value)
+        completed, lasted = _pause(_coord(plugin), plugin, clock)
+        assert completed is True
+        assert lasted == pytest.approx(15.0, abs=0.15)
+
+    def test_a_raising_get_display_duration_pauses_for_30s(self, clock):
+        plugin = _plugin(None)
+        plugin.get_display_duration.side_effect = KeyError('display_duration')
+        completed, lasted = _pause(_coord(plugin), plugin, clock)
+        assert completed is True
+        assert lasted == pytest.approx(30.0, abs=0.15)
+
+    def test_a_good_value_after_a_bad_one_is_used(self, clock):
+        plugin = _plugin(None)
+        coord = _coord(plugin)
+        assert _pause(coord, plugin, clock)[1] == pytest.approx(30.0, abs=0.15)
+        plugin.get_display_duration.return_value = 45
+        assert _pause(coord, plugin, clock)[1] == pytest.approx(45.0, abs=0.15)
+
+    def test_the_pause_can_still_be_interrupted(self, clock):
+        plugin = _plugin('twenty')
+        coord = _coord(plugin)
+        coord._interrupt_check = lambda: clock.now >= 5
+        completed, lasted = _pause(coord, plugin, clock)
+        assert completed is False
+        assert lasted == pytest.approx(5.0, abs=0.15)
+
+
+class TestWarning:
+    def test_logged_once_per_plugin(self, clock, caplog):
+        clock_plugin = _plugin('twenty')
+        calendar = _plugin(None, plugin_id='calendar')
+        coord = _coord(clock_plugin, calendar)
+        with caplog.at_level(logging.WARNING, logger='src.vegas_mode.coordinator'):
+            for _ in range(3):
+                for plugin in (clock_plugin, calendar):
+                    coord._handle_static_pause(plugin)
+        warnings = [r.getMessage() for r in caplog.records
+                    if 'display duration' in r.getMessage()]
+        assert len(warnings) == 2
+        assert any('clock-simple' in m and "'twenty'" in m for m in warnings)
+        assert any('calendar' in m and 'None' in m for m in warnings)
+
+
+class TestFiniteSeconds:
+    """The shared rule: what counts as a number of seconds."""
+
+    @pytest.mark.parametrize('value, seconds', NUMBERS + [(0, 0.0), ('-5', -5.0)])
+    def test_numbers_and_numeric_strings(self, value, seconds):
+        from src.plugin_system.base_plugin import finite_seconds
+        result = finite_seconds(value)
+        assert result == seconds and isinstance(result, float)
+
+    @pytest.mark.parametrize('value', NOT_NUMBERS + [pytest.param(10 ** 400, id='10**400')],
+                             ids=repr)
+    def test_anything_else_is_none(self, value):
+        from src.plugin_system.base_plugin import finite_seconds
+        assert finite_seconds(value) is None
+
+
+def _rotation_seconds(plugin):
+    """How long the rotation shows ``plugin`` (no dynamic duration, no
+    Rotation & Durations override): the two calls run() makes for a screen.
+    """
+    from src.display_controller import DisplayController
+    dc = object.__new__(DisplayController)
+    dc.config = {}
+    dc.plugin_modes = {'mode': plugin}
+    return dc._resolve_durations(plugin, 'mode', dc._get_display_duration('mode'), False)[1]
+
+
+class TestSameAsTheRotation:
+    """The pause and the rotation share finite_seconds; this pins their
+    fallbacks (30 s, 15 s) to each other too."""
+
+    @pytest.mark.parametrize('value', [value for value, _ in NUMBERS]
+                             + NOT_NUMBERS + NOT_ABOVE_ZERO, ids=repr)
+    def test_the_pause_lasts_as_long_as_the_rotation_shows_it(self, clock, value):
+        plugin = _plugin(value)
+        expected = _rotation_seconds(plugin)
+        assert _pause(_coord(plugin), plugin, clock)[1] == pytest.approx(expected, abs=0.15)
+
+    def test_a_raise_too(self, clock):
+        plugin = _plugin(None)
+        plugin.get_display_duration.side_effect = KeyError('display_duration')
+        expected = _rotation_seconds(plugin)
+        assert _pause(_coord(plugin), plugin, clock)[1] == pytest.approx(expected, abs=0.15)