From 4c5bba34ef0afbc70e46ea16c4a9da8873b6d578 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:49:54 -0400 Subject: [PATCH 1/7] fix(web): mask the Config Editor's secrets like GET /config/secrets The Config Editor tab (/partials/raw-json) filled its config_secrets.json editor with the file as it is on disk. GET /api/v3/config/secrets masks every value because the interface is reachable without a login by default, but this page handed the same credentials (GitHub token, Home Assistant token, plugin API keys) to anyone who loaded it. The masked-save path in save_raw_secrets_config was written for a masked editor and never got one. _load_raw_json_partial now masks the section with mask_all_secret_values after strip_auth_section, exactly as the GET does. Saving it back is safe: save_raw_secrets_config drops the masks (strip_masked_values) and merges the rest onto the stored file (deep_merge), so an untouched secret stays as it is and a replaced mask is the only value that changes. The config.json editor is left as it is. Its save (save_raw_main_config) writes the posted object verbatim, with no mask stripping or merge, so a masked main editor would write the bullets over any credential it holds. Masking it needs a merge-on-save of its own first. Tests: TestConfigEditorRoundTrip renders the partial over a real ConfigManager, checks no real value is in the editor, and posts the editor back unchanged (the file is identical) and with one mask replaced (only that value changes). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 +++ test/web_interface/test_api_v3_config_raw.py | 65 ++++++++++++++++++++ web_interface/blueprints/pages_v3.py | 11 +++- 3 files changed, 83 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fa3a3862..39154d16d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -540,6 +540,16 @@ policies are unchanged. notice is what shows next, and Vegas resumes after it; before, a rotation screen showed instead and the notice expired behind it. An active on-demand session still holds the panel until it ends. +- The Config Editor tab no longer shows API keys and tokens in plain + text. Its `config_secrets.json` editor (`/partials/raw-json`) was filled + with the file as it is on disk, so while the web login is off (the + default) anyone who could reach the port could read every credential, + although `GET /api/v3/config/secrets` masks them. The editor now shows the + same masked values. Saving it unchanged changes nothing, because the save + drops the masks and merges onto the stored file; to change a secret, + replace its mask. A list of secrets still needs every entry's real value + to be changed. The `config.json` editor is unchanged: its save writes the + file as given, so a mask there would be stored. - A game that goes live now takes over the panel within about a second. Live priority was only checked between screens, so a game that went live during a 30 s screen waited for that screen to end. The frame loops and the diff --git a/test/web_interface/test_api_v3_config_raw.py b/test/web_interface/test_api_v3_config_raw.py index fe30749b5..31529ab94 100644 --- a/test/web_interface/test_api_v3_config_raw.py +++ b/test/web_interface/test_api_v3_config_raw.py @@ -13,7 +13,9 @@ calls. """ +import html import json +import re import sys from pathlib import Path from unittest.mock import MagicMock @@ -221,3 +223,66 @@ def test_no_separation_happens_on_the_raw_path(self, env): env.client.post(MAIN, json={"weather": {"api_key": "PLAINTEXT-KEY"}}) # Nothing was moved aside into the secrets file. assert not env.secrets_file.exists() or "PLAINTEXT-KEY" not in env.secrets_file.read_text() + + +class TestConfigEditorRoundTrip: + """The Config Editor tab (/partials/raw-json) and the save it posts to. + + The secrets editor is shown masked, like GET /config/secrets: the page is + served to anyone who can reach the port while the optional web login is + off. Its save strips the masks and merges onto the stored file, so a + masked editor saved back as it is changes nothing. + """ + + STORED = { + "github": {"api_token": "ghp_REAL_TOKEN_1234"}, + "ledmatrix-weather": {"api_key": "WEATHER_KEY_abcdef", "units_id": 42}, + "calendar": {"accounts": [{"name": "home", "token": "CAL_TOKEN_9"}]}, + "youtube": {"api_key": "YOUR_YOUTUBE_API_KEY", "channel_secret": ""}, + } + REAL_VALUES = ("ghp_REAL_TOKEN_1234", "WEATHER_KEY_abcdef", "CAL_TOKEN_9") + + @pytest.fixture + def editor(self, env, monkeypatch): + from web_interface.blueprints import pages_v3 as pages_module + env.secrets_file.write_text(json.dumps(self.STORED)) + monkeypatch.setattr(pages_module.pages_v3, "config_manager", + env.config_manager, raising=False) + app = Flask(__name__, template_folder=str(project_root / "web_interface" / "templates")) + app.config["TESTING"] = True + app.register_blueprint(pages_module.pages_v3) + app.register_blueprint(api_v3, url_prefix="/api/v3") + return app.test_client() + + @staticmethod + def _secrets_textarea(client): + page = client.get("/partials/raw-json") + assert page.status_code == 200 + match = re.search(r'', + page.get_data(as_text=True), re.S) + assert match, "the secrets editor is missing from the partial" + return html.unescape(match.group(1)) + + def test_the_editor_shows_no_secret_value(self, editor): + text = self._secrets_textarea(editor) + for value in self.REAL_VALUES: + assert value not in text + shown = json.loads(text) + assert shown["github"]["api_token"] == "\u2022" * 8 + # Same shape as the file, and "not set" still reads as not set. + assert shown["calendar"]["accounts"][0]["name"] == "\u2022" * 8 + assert shown["youtube"] == {"api_key": "YOUR_YOUTUBE_API_KEY", "channel_secret": ""} + + def test_saving_it_back_unchanged_keeps_every_secret(self, editor, env): + shown = json.loads(self._secrets_textarea(editor)) + response = editor.post(SECRETS, json=shown) + assert response.status_code == 200 + assert json.loads(env.secrets_file.read_text()) == self.STORED + + def test_editing_one_secret_changes_only_that_one(self, editor, env): + shown = json.loads(self._secrets_textarea(editor)) + shown["ledmatrix-weather"]["api_key"] = "NEW_WEATHER_KEY" + assert editor.post(SECRETS, json=shown).status_code == 200 + expected = json.loads(json.dumps(self.STORED)) + expected["ledmatrix-weather"]["api_key"] = "NEW_WEATHER_KEY" + assert json.loads(env.secrets_file.read_text()) == expected diff --git a/web_interface/blueprints/pages_v3.py b/web_interface/blueprints/pages_v3.py index 0c32779e3..260ad7bed 100644 --- a/web_interface/blueprints/pages_v3.py +++ b/web_interface/blueprints/pages_v3.py @@ -11,7 +11,7 @@ _SAFE_WEB_UI_FILE_RE = re.compile(r'^[a-zA-Z0-9_-]{1,64}\.html$') _SAFE_WIDGET_NAME_RE = re.compile(r'^[a-zA-Z0-9_-]{1,64}$') _SAFE_WIDGET_SCRIPT_RE = re.compile(r'^[a-zA-Z0-9_-]{1,64}\.js$') -from src.web_interface.secret_helpers import mask_secret_fields +from src.web_interface.secret_helpers import mask_all_secret_values, mask_secret_fields from src.plugin_system.schema_manager import plugin_config_defaults, prepare_plugin_config from src.common.path_safety import resolve_under, safe_path_component from src.pi5_matrix_support import is_raspberry_pi_5 @@ -623,9 +623,14 @@ def _load_raw_json_partial(): main_config_data = pages_v3.config_manager.get_raw_file_content('main') # The web login section (password and token hashes) is managed in # General > Security, never in this editor; its save keeps it. + # The rest is masked, as GET /api/v3/config/secrets masks it: this + # page is served to anyone who can reach the port while the web + # login is off, and it was handing them every credential in the + # file. The save strips the masks and merges onto the stored file + # (save_raw_secrets_config), so a value left masked stays as it is. from web_interface.auth import strip_auth_section - secrets_config_data = strip_auth_section( - pages_v3.config_manager.get_raw_file_content('secrets')) + secrets_config_data = mask_all_secret_values(strip_auth_section( + pages_v3.config_manager.get_raw_file_content('secrets'))) main_config_json = json.dumps(main_config_data, indent=4) secrets_config_json = json.dumps(secrets_config_data, indent=4) From 2b34f2545c3f3dbd978063a95e5e7dc9f570187a Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:50:05 -0400 Subject: [PATCH 2/7] fix(web): keep disabled plugins in the saved rotation order and Vegas exclusions PluginOrderList draws one row per enabled plugin and, once drawn, rewrites its hidden inputs (plugin_rotation_order, vegas_plugin_order, vegas_excluded_plugins) from those rows. A disabled plugin has no row, so merely opening the Display or Rotation & Durations tab took it out of the inputs, and the next save of that form stored the lists without it. Exclude Clock from Vegas, disable it, change the brightness, re-enable it: Clock was scrolling in Vegas again and had moved to the end of the rotation. syncInputs now keeps the saved ids that have no row. In the order, each one keeps its saved slot and the rows fill the other slots in their current order, with rows not in the saved order last, as before. In the exclusions they follow the unchecked rows. Only string ids are carried over, once each: /config/main refuses a list holding anything else, which would block every later save of the tab. Tests: test/js/unit/test_plugin_order_list.js runs the shipped widget in a vm with a fake DOM (draw, reorder, include/exclude, the rotation list, junk ids) and is in run_all.js and the README. The durations DOM suite now reads only its own rows' ids from the input, since a rig's saved order can hold others. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 + test/js/README.md | 1 + test/js/dom/test_durations_page.js | 6 +- test/js/run_all.js | 3 +- test/js/unit/test_plugin_order_list.js | 168 ++++++++++++++++++ .../static/v3/js/widgets/plugin-order-list.js | 49 ++++- 6 files changed, 227 insertions(+), 7 deletions(-) create mode 100644 test/js/unit/test_plugin_order_list.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 39154d16d..d66e5487e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -550,6 +550,13 @@ policies are unchanged. replace its mask. A list of secrets still needs every entry's real value to be changed. The `config.json` editor is unchanged: its save writes the file as given, so a mask there would be stored. +- A disabled plugin keeps its place in the rotation order and its Vegas + exclusion when the Display or Rotation & Durations tab is saved. The order + lists show enabled plugins only and rewrite their hidden inputs from those + rows as soon as they are drawn, so any save of either tab stored the lists + without the disabled plugin. Once re-enabled, it came back at the end of + the rotation and scrolling in Vegas again. Saved ids without a row now + stay in their saved places (`widgets/plugin-order-list.js`). - A game that goes live now takes over the panel within about a second. Live priority was only checked between screens, so a game that went live during a 30 s screen waited for that screen to end. The frame loops and the diff --git a/test/js/README.md b/test/js/README.md index cc0c1b4f8..804da4ab3 100644 --- a/test/js/README.md +++ b/test/js/README.md @@ -53,6 +53,7 @@ server has none. | `unit/test_store_registry_fields.js` | no | The store card's registry fields from `plugins_manager.js`: the commit that introduced the listed version (a hex SHA only, linked to that tree), the "Needs LEDMatrix X+" warning, a card from an older registry without either, and `isStorePluginInstalled` answering to `aliases` | | `unit/test_page_registry.js` | no | The page lifecycle in `js/core/registry.js` (a minimal DOM shim): one `init` per `data-page` root, `destroy` and an aborted `ctx.signal` when htmx swaps it away, a vetoed swap keeps it, lazy page modules, a root removed without htmx swept on the next swap | | `unit/test_core_modules.js` | no | `js/core/api.js` (JSON envelope, HTTP/`status: error`/network errors, abort passthrough, the #683 login redirect, same-server paths only) and `js/core/facade.js` (`window.LEDMatrix`, deprecated aliases) | +| `unit/test_plugin_order_list.js` | no | `widgets/plugin-order-list.js` (the Vegas and rotation order lists): a disabled plugin, which gets no row, keeps its slot in the saved order and its Vegas exclusion when the list rewrites its hidden inputs, around reordering and include/exclude; only string ids are carried over, once each | | `unit/test_plugin_action_delegation.js` | no | The document-level card-action delegation and `handlePluginAction` from `plugins_manager.js`, run with the handler inside an IIFE as in the real file: each action is handled once, a Starlark app uninstall goes to `DELETE /starlark/apps/`, and an uninstall is confirmed once | | `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup | | `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry | diff --git a/test/js/dom/test_durations_page.js b/test/js/dom/test_durations_page.js index 0dfc59e47..c67017685 100644 --- a/test/js/dom/test_durations_page.js +++ b/test/js/dom/test_durations_page.js @@ -98,7 +98,11 @@ const ok = (l, c, x) => c ? (pass++, console.log(' ok ' + l)) const lists = () => requests.filter(r => r.url === '/api/v3/plugins/installed').length; const $ = id => doc.getElementById(id); - const order = () => JSON.parse($('rotation_plugin_order_value').value || '[]'); + // The rows' ids, in order. The input also keeps saved ids that have no row + // (a disabled plugin's place, see test/js/unit/test_plugin_order_list.js), + // and the saved order comes from whatever config the server has. + const SHOWN = plugins.filter(p => p.enabled).map(p => p.id); + const order = () => JSON.parse($('rotation_plugin_order_value').value || '[]').filter(id => SHOWN.includes(id)); async function swap() { panel.dispatchEvent(new window.CustomEvent('htmx:beforeSwap', { bubbles: true, detail: { target: panel, shouldSwap: true } })); panel.innerHTML = partial; diff --git a/test/js/run_all.js b/test/js/run_all.js index 411225015..ba5584e4f 100755 --- a/test/js/run_all.js +++ b/test/js/run_all.js @@ -23,7 +23,8 @@ const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js', 'unit/test_update_all.js', 'unit/test_inline_handler_escaping.js', 'unit/test_plugin_action_delegation.js', 'unit/test_file_upload_widget.js', 'unit/test_store_registry_fields.js', 'unit/test_restart_banner.js', - 'unit/test_page_registry.js', 'unit/test_core_modules.js']; + 'unit/test_page_registry.js', 'unit/test_core_modules.js', + 'unit/test_plugin_order_list.js']; const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js', 'dom/test_tools_sections.js', 'dom/test_cache_page.js', 'dom/test_durations_page.js', 'dom/test_operation_history_page.js', diff --git a/test/js/unit/test_plugin_order_list.js b/test/js/unit/test_plugin_order_list.js new file mode 100644 index 000000000..45bb4e876 --- /dev/null +++ b/test/js/unit/test_plugin_order_list.js @@ -0,0 +1,168 @@ +// The shared plugin order list (widgets/plugin-order-list.js) keeps what it +// does not show. +// +// It lists enabled plugins only, and rewrites its hidden inputs from those +// rows as soon as it has drawn them. A disabled plugin's place in the order +// and its Vegas exclusion used to vanish from the inputs on that rewrite, so +// any later save of the Display or Rotation & Durations tab stored them +// without it: re-enabled, the plugin came back at the end of the rotation and +// scrolling in Vegas again. Runs the shipped widget in a vm with a minimal +// fake DOM -- no jsdom and no server needed, so it runs under +// test/test_js_unit_suites.py too. + +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); +const WIDGET = path.resolve(__dirname, '../../../web_interface/static/v3/js/widgets/plugin-order-list.js'); + +let pass = 0, fail = 0; +const ok = (label, cond, extra) => cond + ? (pass++, console.log(' ok ' + label)) + : (fail++, console.log(' FAIL ' + label + (extra !== undefined ? ' ' + JSON.stringify(extra) : ''))); +const same = (a, b) => JSON.stringify(a) === JSON.stringify(b); + +class FakeElement { + constructor(tag) { + this.tagName = tag.toUpperCase(); + this.children = []; + this.parent = null; + this.dataset = {}; + this.style = {}; + this.className = ''; + this.value = ''; + this.checked = false; + this.listeners = {}; + this._text = ''; + } + appendChild(child) { + if (child.parent) child.parent.children = child.parent.children.filter(c => c !== child); + child.parent = this; + this.children.push(child); + return child; + } + insertBefore(child, ref) { + if (!ref) return this.appendChild(child); + if (child.parent) child.parent.children = child.parent.children.filter(c => c !== child); + child.parent = this; + this.children.splice(this.children.indexOf(ref), 0, child); + return child; + } + get previousElementSibling() { + const siblings = this.parent ? this.parent.children : []; + return siblings[siblings.indexOf(this) - 1] || null; + } + get nextElementSibling() { + const siblings = this.parent ? this.parent.children : []; + const i = siblings.indexOf(this); + return i < 0 ? null : siblings[i + 1] || null; + } + set textContent(value) { this._text = value; this.children = []; } + get textContent() { return this._text; } + setAttribute() {} + focus() {} + addEventListener(type, fn) { (this.listeners[type] ||= []).push(fn); } + fire(type, event) { (this.listeners[type] || []).forEach(fn => fn.call(this, event || {})); } + descendants() { return this.children.flatMap(c => [c, ...c.descendants()]); } + querySelectorAll(selector) { + const cls = selector.replace(/^\./, ''); + return this.descendants().filter(e => e.className.split(/\s+/).includes(cls)); + } + querySelector(selector) { return this.querySelectorAll(selector)[0] || null; } +} + +/** Run the widget over `plugins` with the given saved inputs; resolves once it has drawn. */ +async function mount({ plugins, order, excluded }) { + const els = { + list: new FakeElement('div'), + order: Object.assign(new FakeElement('input'), { value: JSON.stringify(order) }), + }; + if (excluded !== undefined) { + els.excluded = Object.assign(new FakeElement('input'), { value: JSON.stringify(excluded) }); + } + const context = { + console, + window: {}, + document: { + getElementById: (id) => els[id] || null, + createElement: (tag) => new FakeElement(tag), + createTextNode: (text) => new FakeElement('#text'), + }, + fetch: () => Promise.resolve({ + json: () => Promise.resolve({ status: 'success', data: { plugins } }), + }), + }; + vm.createContext(context); + vm.runInContext(fs.readFileSync(WIDGET, 'utf8'), context); + context.window.PluginOrderList.init({ + containerId: 'list', orderInputId: 'order', + excludedInputId: excluded !== undefined ? 'excluded' : undefined, + }); + await new Promise(resolve => setTimeout(resolve, 0)); + const rows = () => els.list.querySelectorAll('.plugin-order-item'); + return { + rows, + rowIds: () => rows().map(r => r.dataset.pluginId), + order: () => JSON.parse(els.order.value), + excluded: () => JSON.parse(els.excluded.value), + row: (id) => rows().find(r => r.dataset.pluginId === id), + }; +} + +const PLUGINS = [ + { id: 'weather', name: 'Weather', enabled: true }, + { id: 'clock', name: 'Clock', enabled: false }, + { id: 'stocks', name: 'Stocks', enabled: true }, +]; + +(async () => { + console.log('\nVegas: a disabled plugin keeps its place and its exclusion'); + { + const t = await mount({ plugins: PLUGINS, order: ['weather', 'clock', 'stocks'], excluded: ['clock'] }); + ok('only enabled plugins get a row', same(t.rowIds(), ['weather', 'stocks']), t.rowIds()); + ok('drawing the list keeps the disabled plugin in the order, in its place', + same(t.order(), ['weather', 'clock', 'stocks']), t.order()); + ok('drawing the list keeps its exclusion', same(t.excluded(), ['clock']), t.excluded()); + + // Move Stocks up: the rows swap, and Clock stays in its saved slot. + const up = t.row('stocks').querySelectorAll('.plugin-order-move')[0]; + up.fire('click'); + ok('reordering the rows fills the other slots in the new order', + same(t.order(), ['stocks', 'clock', 'weather']), t.order()); + + const include = t.row('weather').querySelector('.plugin-order-include'); + include.checked = false; + include.fire('change'); + ok('unchecking a row adds it, and the disabled exclusion stays', + same([...t.excluded()].sort(), ['clock', 'weather']), t.excluded()); + include.checked = true; + include.fire('change'); + ok('checking it again removes only that one', same(t.excluded(), ['clock']), t.excluded()); + } + + console.log('\nRotation order: the same, without exclusions'); + { + const plugins = [ + { id: 'clock', enabled: true }, + { id: 'off', enabled: false }, + { id: 'weather', enabled: true }, + { id: 'new', enabled: true }, + ]; + const t = await mount({ plugins, order: ['clock', 'off', 'weather'] }); + ok('the disabled plugin keeps its slot; a plugin not in the saved order goes last', + same(t.order(), ['clock', 'off', 'weather', 'new']), t.order()); + } + + console.log('\nOnly what the server would accept is carried over'); + { + const t = await mount({ plugins: PLUGINS, order: ['weather', 7, 'clock', null, 'clock', 'stocks'], + excluded: ['clock', 3, 'clock'] }); + // /config/main refuses a list holding anything but strings, which would + // block every later Display save; a repeated id is kept once. + ok('non-string and repeated saved ids are dropped from the order', + same(t.order(), ['weather', 'clock', 'stocks']), t.order()); + ok('and from the exclusions', same(t.excluded(), ['clock']), t.excluded()); + } + + console.log(`\n${pass} passed, ${fail} failed`); + process.exit(fail ? 1 : 0); +})().catch(e => { console.error(e); process.exit(1); }); diff --git a/web_interface/static/v3/js/widgets/plugin-order-list.js b/web_interface/static/v3/js/widgets/plugin-order-list.js index 184c99b7d..d1f068905 100644 --- a/web_interface/static/v3/js/widgets/plugin-order-list.js +++ b/web_interface/static/v3/js/widgets/plugin-order-list.js @@ -18,7 +18,8 @@ * }); * * The container re-renders from /api/v3/plugins/installed each init; the - * hidden input(s) must already hold the saved order/exclusions (JSON). + * hidden input(s) must already hold the saved order/exclusions (JSON). Saved + * ids without a row (disabled plugins) stay in them, in their saved places. */ (function() { 'use strict'; @@ -39,17 +40,53 @@ const excludedInput = options.excludedInputId ? document.getElementById(options.excludedInputId) : null; if (!container || !orderInput) return; + // The saved lists as the inputs held them when the rows were drawn. + // Only enabled plugins get a row, and the inputs are rewritten from + // the rows, so a disabled plugin's place and exclusion have to be + // carried over from these: dropped, the next Display or Durations + // save stored the lists without it, and once re-enabled it came back + // at the end of the rotation and scrolling in Vegas again. + let savedOrder = []; + let savedExcluded = []; + + // Saved ids with no row, once each. Only strings: /config/main + // refuses a list holding anything else, which would block every save. + function unlisted(saved, rowIds) { + const seen = new Set(rowIds); + return saved.filter(id => { + if (typeof id !== 'string' || seen.has(id)) return false; + seen.add(id); + return true; + }); + } + function syncInputs() { - const order = []; + const rowIds = []; const excluded = []; container.querySelectorAll('.plugin-order-item').forEach(item => { const pluginId = item.dataset.pluginId; - order.push(pluginId); + rowIds.push(pluginId); const checkbox = item.querySelector('.plugin-order-include'); if (checkbox && !checkbox.checked) excluded.push(pluginId); }); - orderInput.value = JSON.stringify(order); - if (excludedInput) excludedInput.value = JSON.stringify(excluded); + // An id without a row keeps its saved slot; the rows fill the + // other slots in their current order, and any rows left over + // (plugins not in the saved order) go last. + const kept = new Set(unlisted(savedOrder, rowIds)); + const order = []; + let next = 0; + savedOrder.forEach(id => { + if (kept.has(id)) { + order.push(id); + kept.delete(id); + } else if (rowIds.includes(id) && next < rowIds.length) { + order.push(rowIds[next++]); + } + }); + orderInput.value = JSON.stringify(order.concat(rowIds.slice(next))); + if (excludedInput) { + excludedInput.value = JSON.stringify(excluded.concat(unlisted(savedExcluded, rowIds))); + } } function setupDragAndDrop() { @@ -125,6 +162,8 @@ // (e.g. a saved value of "null"); normalize to arrays. if (!Array.isArray(currentOrder)) currentOrder = []; if (!Array.isArray(excluded)) excluded = []; + savedOrder = currentOrder; + savedExcluded = excluded; // Saved order first, then any newly enabled plugins. const orderedPlugins = []; From d1d9a9426679dba48220dee0795496e9212367e2 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:52:15 -0400 Subject: [PATCH 3/7] fix(web): a restore reinstalls only the plugins that are missing POST /backup/restore with reinstall_plugins (the "Reinstall missing plugins" box) passed every plugin in the backup's plugins.json to install_plugin(). That replaces an installed copy with a fresh download, so a restore onto the same device re-downloaded every plugin inside the request. A plugin installed from its own URL is not in the registry, so its install returned False, plugins_failed set success to False, and the restore answered 500 "Restore incomplete ... plugins not reinstalled: " (shown as "Restore failed") with the plugin still installed and the config restored. Each plugin is now looked up first with the store's _existing_install, the same lookup install_plugin makes to decide a copy exists: the id, or an id the registry proves is the same plugin (aliases, the plugin_path name), and never a bare ledmatrix- folder (#686). One that is installed is recorded in result.skipped as "plugin: (installed)", which the page lists under Skipped; a missing one is installed as before. The list_installed_plugins docstring said every listed plugin is reinstalled and now says otherwise. Tests: TestInstalledPluginsAreNotReinstalled, with a mocked store (installed skipped, missing installed; an installed plugin the store can't install is not a failure) and with a real PluginStoreManager (a registry alias and a third-party install are skipped, a missing plugin installed). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 +++ src/backup_manager.py | 5 +- .../test_api_v3_backup_restore.py | 80 +++++++++++++++++++ web_interface/blueprints/api_v3/backup.py | 20 +++++ 4 files changed, 112 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d66e5487e..86391b8af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -557,6 +557,15 @@ policies are unchanged. without the disabled plugin. Once re-enabled, it came back at the end of the rotation and scrolling in Vegas again. Saved ids without a row now stay in their saved places (`widgets/plugin-order-list.js`). +- Restoring a backup with "Reinstall missing plugins" installs only the + plugins that are missing. Every plugin the backup listed was sent to the + store's install, which replaces an installed copy with a fresh download, + so a restore onto the same device re-downloaded all of them in one + request. A plugin installed from its own URL is not in the registry, so + its "reinstall" failed and the restore answered "Restore failed" while + the plugin sat there installed. An installed plugin, found by the store's + own lookup (registry aliases included), is now listed under Skipped as + `plugin: (installed)`. - A game that goes live now takes over the panel within about a second. Live priority was only checked between screens, so a game that went live during a 30 s screen waited for that screen to end. The frame loops and the diff --git a/src/backup_manager.py b/src/backup_manager.py index bcd7b342a..3d3d64c04 100644 --- a/src/backup_manager.py +++ b/src/backup_manager.py @@ -213,8 +213,9 @@ def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]: The plugins are the ``manifest.json`` files in the configured plugin directory (see :func:`_plugins_directory`), with the manifest's version; ``enabled`` is config.json's flag by the display's rule (a missing flag - is disabled). A restore reinstalls every listed plugin and takes enabled - state from the restored config.json, so ``enabled`` is informational. + is disabled). A restore installs each listed plugin that is missing and + takes enabled state from the restored config.json, so ``enabled`` is + informational. ``data/plugin_state.json`` is not read: it only ever repeated config's enabled flags and the manifests' versions, and is retired (nothing diff --git a/test/web_interface/test_api_v3_backup_restore.py b/test/web_interface/test_api_v3_backup_restore.py index a7f0f123a..086bdb1f8 100644 --- a/test/web_interface/test_api_v3_backup_restore.py +++ b/test/web_interface/test_api_v3_backup_restore.py @@ -50,11 +50,13 @@ def __init__(self, success=True, restored=None, errors=None, self.plugins_to_install = plugins_to_install or [] self.plugins_installed = [] self.plugins_failed = [] + self.skipped = [] def to_dict(self): return { "success": self.success, "restored": self.restored, + "skipped": self.skipped, "errors": self.errors, "plugins_installed": self.plugins_installed, "plugins_failed": self.plugins_failed, @@ -286,6 +288,84 @@ def test_missing_store_manager_is_reported_per_plugin(self, client, restore): assert body["data"]["plugins_failed"][0]["error"] == "Store manager unavailable" +class TestInstalledPluginsAreNotReinstalled: + """"Reinstall missing plugins" installs only what is missing. + + Every plugin the backup listed went to install_plugin, which replaces an + installed copy with a fresh download: restoring onto the same device + re-downloaded all of them inside the request. One installed from its own + URL is not in the registry, so its "reinstall" returned False and the + whole restore answered 500 "Restore failed" with the plugin still there. + """ + + @staticmethod + def _installed(tmp_path, *names): + found = {} + for name in names: + (tmp_path / name).mkdir() + found[name] = tmp_path / name + return lambda plugin_id: found.get(plugin_id) + + def test_an_installed_plugin_is_skipped_and_a_missing_one_installed( + self, client, restore, tmp_path): + restore.return_value = FakeResult( + plugins_to_install=[{"plugin_id": "clock"}, {"plugin_id": "weather"}]) + store = api_v3.plugin_store_manager + store._existing_install.side_effect = self._installed(tmp_path, "clock") + store.install_plugin.return_value = True + response = post(client) + assert response.status_code == 200 + store.install_plugin.assert_called_once_with("weather") + data = response.get_json()["data"] + assert data["plugins_installed"] == ["weather"] + assert data["plugins_failed"] == [] + assert "plugin:clock (installed)" in data["skipped"] + + def test_an_installed_plugin_the_store_cannot_install_is_not_a_failure( + self, client, restore, tmp_path): + restore.return_value = FakeResult(plugins_to_install=[{"plugin_id": "my-3p"}]) + store = api_v3.plugin_store_manager + store._existing_install.side_effect = self._installed(tmp_path, "my-3p") + store.install_plugin.return_value = False + response = post(client) + assert response.status_code == 200 + assert response.get_json()["data"]["plugins_failed"] == [] + store.install_plugin.assert_not_called() + + @pytest.fixture + def real_store(self, tmp_path): + from src.plugin_system.store_manager import PluginStoreManager + plugins_dir = tmp_path / "plugin-repos" + for folder, manifest_id in (("ledmatrix-weather", "ledmatrix-weather"), + ("my-3p", "my-3p")): + (plugins_dir / folder).mkdir(parents=True) + (plugins_dir / folder / "manifest.json").write_text( + json.dumps({"id": manifest_id, "version": "1.0.0"})) + store = PluginStoreManager(plugins_dir=str(plugins_dir), + uninstalled_registry_path=str(tmp_path / "uninstalled.json")) + # The official weather plugin's registry id differs from the id it + # installs under; my-3p was installed from its own URL. + registry = {"plugins": [{ + "id": "weather", "repo": "https://github.com/ChuckBuilds/ledmatrix-plugins", + "plugin_path": "plugins/ledmatrix-weather"}]} + store.registry_cache = registry + store.fetch_registry = lambda *a, **k: registry + store.install_plugin = MagicMock(return_value=True) + api_v3.plugin_store_manager = store + return store + + def test_with_the_real_store_aliases_and_third_party_installs_count( + self, client, restore, real_store): + restore.return_value = FakeResult(plugins_to_install=[ + {"plugin_id": "weather"}, {"plugin_id": "my-3p"}, {"plugin_id": "clock"}]) + response = post(client) + assert response.status_code == 200 + real_store.install_plugin.assert_called_once_with("clock") + skipped = response.get_json()["data"]["skipped"] + assert "plugin:weather (installed)" in skipped + assert "plugin:my-3p (installed)" in skipped + + class TestFailureReporting: def test_restore_errors_produce_a_500(self, client, restore): restore.return_value = FakeResult( diff --git a/web_interface/blueprints/api_v3/backup.py b/web_interface/blueprints/api_v3/backup.py index 2184861b5..39ee9c5e2 100644 --- a/web_interface/blueprints/api_v3/backup.py +++ b/web_interface/blueprints/api_v3/backup.py @@ -85,6 +85,17 @@ def backup_validate(): 'restore_config', 'restore_secrets', 'restore_wifi', 'restore_fonts', 'restore_plugin_uploads', 'reinstall_plugins', )) +def _installed_path(psm, plugin_id): + """Where the store finds ``plugin_id`` installed, or None. + + The same lookup install_plugin makes to decide that a copy exists: the + id, or an id the registry proves is the same plugin (``aliases``, the + ``plugin_path`` name), never a bare ``ledmatrix-`` folder. + """ + found = psm._existing_install(plugin_id) + return found if isinstance(found, Path) and found.exists() else None + + @api_v3.route('/backup/restore', methods=['POST']) def backup_restore(): """Restore a backup ZIP with optional RestoreOptions.""" @@ -143,6 +154,15 @@ def backup_restore(): if not pid: continue try: + # Only what is missing. install_plugin replaces an installed + # copy with a fresh download, so restoring onto the same + # device re-downloaded every plugin, and one installed from + # its own URL (not in the registry) "failed" and failed the + # whole restore while it sat there installed. The store's + # own lookup, so registry aliases count as installed too. + if psm and _installed_path(psm, pid) is not None: + result.skipped.append(f'plugin:{pid} (installed)') + continue if psm and hasattr(psm, 'install_plugin'): ok = psm.install_plugin(pid) if ok: From 64fd6d49f9fd222639893207102cab16b5912c5b Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:53:20 -0400 Subject: [PATCH 4/7] fix(web): /config/main answers malformed JSON with a 400 save_main_config read a JSON body with request.get_json(), which raises Werkzeug's BadRequest for a body that does not parse (or an empty one sent as application/json). That happened inside the handler's try, so the catch-all answered 500 CONFIG_SAVE_FAILED with "Check file permissions on config directory" among its suggested fixes and logged a traceback at ERROR, for what was the caller's mistake. It now reads with get_json(silent=True), as save_raw_main_config does, and answers a sent-but-unparseable body with the same 400 {"status": "error", "message": "Invalid JSON in request body"}. An empty JSON body falls through to the existing 400 "No data provided". The change is limited to the lines that read the body. Tests: TestMalformedBody in test_api_v3_partial_main_save.py (the 400 and its shape, identical to /config/raw/main's, and nothing saved; the empty body). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 6 ++++++ test/test_api_v3_partial_main_save.py | 26 +++++++++++++++++++++++ web_interface/blueprints/api_v3/config.py | 6 +++++- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86391b8af..43a4fb359 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -566,6 +566,12 @@ policies are unchanged. the plugin sat there installed. An installed plugin, found by the store's own lookup (registry aliases included), is now listed under Skipped as `plugin: (installed)`. +- `POST /api/v3/config/main` answers a JSON body that does not parse with + 400 `Invalid JSON in request body`, as `/config/raw/main` does, and an + empty JSON body with 400 `No data provided`. Both were a 500 + `CONFIG_SAVE_FAILED` suggesting file permissions and disk space, with a + traceback logged at ERROR: `get_json()` raised inside the handler's + catch-all. - A game that goes live now takes over the panel within about a second. Live priority was only checked between screens, so a game that went live during a 30 s screen waited for that screen to end. The frame loops and the diff --git a/test/test_api_v3_partial_main_save.py b/test/test_api_v3_partial_main_save.py index 87c830cc8..897d17d6d 100644 --- a/test/test_api_v3_partial_main_save.py +++ b/test/test_api_v3_partial_main_save.py @@ -253,6 +253,32 @@ def test_per_mode_duration_saves_and_blank_clears_it(self, api_v3_client, saved, assert saved['config']['display']['display_durations'] == {'clock': 45} +class TestMalformedBody: + """A JSON body that does not parse is the caller's mistake: a 400. + + get_json() raised Werkzeug's BadRequest inside the handler's try, whose + catch-all answered 500 CONFIG_SAVE_FAILED with "check file permissions" + advice and logged a traceback at ERROR. + """ + + def test_is_a_400_in_the_raw_routes_shape(self, api_v3_client, saved, api_v3_module): + api_v3_module.api_v3.config_manager.get_raw_file_content.return_value = {} + resp = api_v3_client.post('/api/v3/config/main', data='{not json', + content_type='application/json') + assert resp.status_code == 400 + assert resp.get_json() == {'status': 'error', 'message': 'Invalid JSON in request body'} + assert 'config' not in saved + raw = api_v3_client.post('/api/v3/config/raw/main', data='{not json', + content_type='application/json') + assert (raw.status_code, raw.get_json()) == (400, resp.get_json()) + + def test_an_empty_json_post_is_still_no_data(self, api_v3_client, saved): + resp = api_v3_client.post('/api/v3/config/main', data='', + content_type='application/json') + assert resp.status_code == 400 + assert resp.get_json()['message'] == 'No data provided' + + class TestRawSaveStartsAutoUpdateSetup: @pytest.fixture def raw_env(self, api_v3_module, monkeypatch): diff --git a/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index 41720c96e..f2265370f 100644 --- a/web_interface/blueprints/api_v3/config.py +++ b/web_interface/blueprints/api_v3/config.py @@ -507,7 +507,11 @@ def save_main_config(): # Try to get JSON data first, fallback to form data data = None if request.is_json: - data = request.get_json() + # silent=True, as in save_raw_main_config: get_json() raised + # Werkzeug's BadRequest into the catch-all below, a 500. + data = request.get_json(silent=True) + if data is None and request.get_data(): + return jsonify({'status': 'error', 'message': 'Invalid JSON in request body'}), 400 if data is not None and not isinstance(data, dict): return jsonify({'status': 'error', 'message': 'Request body must be a JSON object'}), 400 else: From 8d26f76930bf49941d7a61eb9553956f8f002845 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 20:54:08 -0400 Subject: [PATCH 5/7] fix(web): a restore that brings back fonts clears the font catalog cache GET /api/v3/fonts/catalog caches its answer as fonts_catalog for five minutes. Font upload and delete clear that entry (fonts.py), but POST /backup/restore copies user fonts into assets/fonts without touching it, so restored fonts were missing from the Fonts tab and every font picker until the cache expired. backup_restore now clears fonts_catalog when the result lists restored fonts (restore_backup records them as "fonts ()"). A restore that restored no fonts leaves the cache alone. Tests: TestFontsCatalogCache in test_api_v3_backup_restore.py. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +++ .../test_api_v3_backup_restore.py | 25 +++++++++++++++++++ web_interface/blueprints/api_v3/backup.py | 5 ++++ 3 files changed, 33 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43a4fb359..86b8c32f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -572,6 +572,9 @@ policies are unchanged. `CONFIG_SAVE_FAILED` suggesting file permissions and disk space, with a traceback logged at ERROR: `get_json()` raised inside the handler's catch-all. +- Fonts restored from a backup show up in the Fonts tab and the font + pickers straight away. The font catalog is cached for five minutes, and + upload and delete cleared it but a restore did not. - A game that goes live now takes over the panel within about a second. Live priority was only checked between screens, so a game that went live during a 30 s screen waited for that screen to end. The frame loops and the diff --git a/test/web_interface/test_api_v3_backup_restore.py b/test/web_interface/test_api_v3_backup_restore.py index 086bdb1f8..ca576e11b 100644 --- a/test/web_interface/test_api_v3_backup_restore.py +++ b/test/web_interface/test_api_v3_backup_restore.py @@ -366,6 +366,31 @@ def test_with_the_real_store_aliases_and_third_party_installs_count( assert "plugin:my-3p (installed)" in skipped +class TestFontsCatalogCache: + """The Fonts tab's catalog is cached for 5 minutes (fonts.py). + + Upload and delete clear it; a restore did not, so restored fonts were + missing from the Fonts tab and every font picker until it expired. + """ + + @pytest.fixture + def cached_catalog(self): + from web_interface.cache import delete_cached, get_cached, set_cached + set_cached('fonts_catalog', {'fonts': ['5x7.bdf']}, ttl_seconds=300) + yield lambda: get_cached('fonts_catalog', ttl_seconds=300) + delete_cached('fonts_catalog') + + def test_a_restore_that_restored_fonts_clears_it(self, client, restore, cached_catalog): + restore.return_value = FakeResult(restored=["config", "fonts (2)"]) + assert post(client).status_code == 200 + assert cached_catalog() is None + + def test_a_restore_without_fonts_keeps_it(self, client, restore, cached_catalog): + restore.return_value = FakeResult(restored=["config"]) + assert post(client).status_code == 200 + assert cached_catalog() == {'fonts': ['5x7.bdf']} + + class TestFailureReporting: def test_restore_errors_produce_a_500(self, client, restore): restore.return_value = FakeResult( diff --git a/web_interface/blueprints/api_v3/backup.py b/web_interface/blueprints/api_v3/backup.py index 39ee9c5e2..ba5191b0b 100644 --- a/web_interface/blueprints/api_v3/backup.py +++ b/web_interface/blueprints/api_v3/backup.py @@ -16,6 +16,7 @@ # as module attributes, and a value binding would not see the patch. # Several are also called from helpers that live in __init__, so the # package is the only patch point that covers every caller. +from web_interface.cache import delete_cached @api_v3.route('/backup/preview', methods=['GET']) @@ -145,6 +146,10 @@ def backup_restore(): os.unlink(tmp_path) except OSError: pass + # Restored fonts reach the Fonts tab through a catalog cached for five + # minutes (fonts.py); upload and delete clear it, and so must this. + if any(str(item).startswith('fonts') for item in result.restored): + delete_cached('fonts_catalog') # Reinstall plugins if requested and store manager available if options.reinstall_plugins and result.plugins_to_install: From 73926f7f64d714f9db88d6770ac3d7f5867be222 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:01:25 -0400 Subject: [PATCH 6/7] fix(web): drop uninstalled plugins from the carried-over order and exclusions 2b34f254 made the plugin order list keep every saved id that has no row, so a disabled plugin keeps its rotation slot and Vegas exclusion. That also kept the ids of plugins that have since been uninstalled: they stayed in plugin_rotation_order and vegas_excluded_plugins for good, where before the next save of the tab dropped them. The widget already fetches /api/v3/plugins/installed, every installed plugin with its enabled flag, and draws only the enabled ones. It now keeps that response's full id set and carries over only saved ids that are installed but have no row (disabled). An id outside the set is dropped, as before. With no list, nothing is dropped: a failed request draws no rows and leaves the inputs as saved, and the carry-over keeps everything if the set was never filled. Tests: test/js/unit/test_plugin_order_list.js adds a disabled plugin kept while an uninstalled one is dropped (order and exclusions; fails on 2b34f254), and a failed plugin list leaving both inputs as saved. The CHANGELOG bullet and the README row say so. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +-- test/js/README.md | 2 +- test/js/unit/test_plugin_order_list.js | 35 +++++++++++++++---- .../static/v3/js/widgets/plugin-order-list.js | 15 ++++++-- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86b8c32f0..bcdb9bbe7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -555,8 +555,9 @@ policies are unchanged. lists show enabled plugins only and rewrite their hidden inputs from those rows as soon as they are drawn, so any save of either tab stored the lists without the disabled plugin. Once re-enabled, it came back at the end of - the rotation and scrolling in Vegas again. Saved ids without a row now - stay in their saved places (`widgets/plugin-order-list.js`). + the rotation and scrolling in Vegas again. A disabled plugin's saved id + now stays in its saved place (`widgets/plugin-order-list.js`); the id of + a plugin that is no longer installed is still dropped. - Restoring a backup with "Reinstall missing plugins" installs only the plugins that are missing. Every plugin the backup listed was sent to the store's install, which replaces an installed copy with a fresh download, diff --git a/test/js/README.md b/test/js/README.md index 804da4ab3..3768ef589 100644 --- a/test/js/README.md +++ b/test/js/README.md @@ -53,7 +53,7 @@ server has none. | `unit/test_store_registry_fields.js` | no | The store card's registry fields from `plugins_manager.js`: the commit that introduced the listed version (a hex SHA only, linked to that tree), the "Needs LEDMatrix X+" warning, a card from an older registry without either, and `isStorePluginInstalled` answering to `aliases` | | `unit/test_page_registry.js` | no | The page lifecycle in `js/core/registry.js` (a minimal DOM shim): one `init` per `data-page` root, `destroy` and an aborted `ctx.signal` when htmx swaps it away, a vetoed swap keeps it, lazy page modules, a root removed without htmx swept on the next swap | | `unit/test_core_modules.js` | no | `js/core/api.js` (JSON envelope, HTTP/`status: error`/network errors, abort passthrough, the #683 login redirect, same-server paths only) and `js/core/facade.js` (`window.LEDMatrix`, deprecated aliases) | -| `unit/test_plugin_order_list.js` | no | `widgets/plugin-order-list.js` (the Vegas and rotation order lists): a disabled plugin, which gets no row, keeps its slot in the saved order and its Vegas exclusion when the list rewrites its hidden inputs, around reordering and include/exclude; only string ids are carried over, once each | +| `unit/test_plugin_order_list.js` | no | `widgets/plugin-order-list.js` (the Vegas and rotation order lists): a disabled plugin, which gets no row, keeps its slot in the saved order and its Vegas exclusion when the list rewrites its hidden inputs, around reordering and include/exclude; an uninstalled plugin's id is dropped, a failed plugin list leaves the inputs as saved, and only string ids are carried over, once each | | `unit/test_plugin_action_delegation.js` | no | The document-level card-action delegation and `handlePluginAction` from `plugins_manager.js`, run with the handler inside an IIFE as in the real file: each action is handled once, a Starlark app uninstall goes to `DELETE /starlark/apps/`, and an uninstall is confirmed once | | `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup | | `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry | diff --git a/test/js/unit/test_plugin_order_list.js b/test/js/unit/test_plugin_order_list.js index 45bb4e876..f690a2464 100644 --- a/test/js/unit/test_plugin_order_list.js +++ b/test/js/unit/test_plugin_order_list.js @@ -6,9 +6,10 @@ // and its Vegas exclusion used to vanish from the inputs on that rewrite, so // any later save of the Display or Rotation & Durations tab stored them // without it: re-enabled, the plugin came back at the end of the rotation and -// scrolling in Vegas again. Runs the shipped widget in a vm with a minimal -// fake DOM -- no jsdom and no server needed, so it runs under -// test/test_js_unit_suites.py too. +// scrolling in Vegas again. An uninstalled plugin's id is still dropped, as +// before, so the lists don't collect ids nothing can show. Runs the shipped +// widget in a vm with a minimal fake DOM -- no jsdom and no server needed, so +// it runs under test/test_js_unit_suites.py too. const fs = require('fs'); const path = require('path'); @@ -71,7 +72,7 @@ class FakeElement { } /** Run the widget over `plugins` with the given saved inputs; resolves once it has drawn. */ -async function mount({ plugins, order, excluded }) { +async function mount({ plugins, order, excluded, fetchFails }) { const els = { list: new FakeElement('div'), order: Object.assign(new FakeElement('input'), { value: JSON.stringify(order) }), @@ -80,16 +81,17 @@ async function mount({ plugins, order, excluded }) { els.excluded = Object.assign(new FakeElement('input'), { value: JSON.stringify(excluded) }); } const context = { - console, + // The widget logs a failed list; expected there, so kept off the output. + console: fetchFails ? Object.assign({}, console, { error: () => {} }) : console, window: {}, document: { getElementById: (id) => els[id] || null, createElement: (tag) => new FakeElement(tag), createTextNode: (text) => new FakeElement('#text'), }, - fetch: () => Promise.resolve({ + fetch: () => (fetchFails ? Promise.reject(new Error('service restarting')) : Promise.resolve({ json: () => Promise.resolve({ status: 'success', data: { plugins } }), - }), + })), }; vm.createContext(context); vm.runInContext(fs.readFileSync(WIDGET, 'utf8'), context); @@ -152,6 +154,25 @@ const PLUGINS = [ same(t.order(), ['clock', 'off', 'weather', 'new']), t.order()); } + console.log('\nAn uninstalled plugin is dropped; a failed list keeps everything'); + { + const t = await mount({ plugins: PLUGINS, order: ['weather', 'gone', 'clock', 'stocks'], + excluded: ['gone', 'clock'] }); + ok('the disabled plugin is kept and the uninstalled one dropped from the order', + same(t.order(), ['weather', 'clock', 'stocks']), t.order()); + ok('and from the exclusions', same(t.excluded(), ['clock']), t.excluded()); + } + { + const t = await mount({ plugins: PLUGINS, order: ['weather', 'gone', 'clock', 'stocks'], + excluded: ['gone', 'clock'], fetchFails: true }); + // No installed list, so nothing can be told apart: no rows, and the + // inputs keep what was saved, uninstalled ids included. + ok('a failed plugin list draws no rows', t.rowIds().length === 0, t.rowIds()); + ok('and leaves the saved order as it was', + same(t.order(), ['weather', 'gone', 'clock', 'stocks']), t.order()); + ok('and the saved exclusions', same(t.excluded(), ['gone', 'clock']), t.excluded()); + } + console.log('\nOnly what the server would accept is carried over'); { const t = await mount({ plugins: PLUGINS, order: ['weather', 7, 'clock', null, 'clock', 'stocks'], diff --git a/web_interface/static/v3/js/widgets/plugin-order-list.js b/web_interface/static/v3/js/widgets/plugin-order-list.js index d1f068905..34b7f68bb 100644 --- a/web_interface/static/v3/js/widgets/plugin-order-list.js +++ b/web_interface/static/v3/js/widgets/plugin-order-list.js @@ -19,7 +19,8 @@ * * The container re-renders from /api/v3/plugins/installed each init; the * hidden input(s) must already hold the saved order/exclusions (JSON). Saved - * ids without a row (disabled plugins) stay in them, in their saved places. + * ids of disabled plugins (installed, but without a row) stay in them, in + * their saved places; ids of plugins no longer installed are dropped. */ (function() { 'use strict'; @@ -48,13 +49,20 @@ // at the end of the rotation and scrolling in Vegas again. let savedOrder = []; let savedExcluded = []; + // Every installed plugin's id, enabled or not, from the same + // response. A saved id outside it belongs to an uninstalled plugin + // and is dropped, as every save used to; without the list, nothing + // is dropped. + let installedIds = null; - // Saved ids with no row, once each. Only strings: /config/main - // refuses a list holding anything else, which would block every save. + // Saved ids of installed plugins with no row, once each. Only + // strings: /config/main refuses a list holding anything else, which + // would block every save. function unlisted(saved, rowIds) { const seen = new Set(rowIds); return saved.filter(id => { if (typeof id !== 'string' || seen.has(id)) return false; + if (installedIds && !installedIds.has(id)) return false; seen.add(id); return true; }); @@ -141,6 +149,7 @@ .then(data => { const allPlugins = (data.data && data.data.plugins) || data.plugins || []; const plugins = allPlugins.filter(p => p.enabled); + installedIds = new Set(allPlugins.map(p => p && p.id)); if (plugins.length === 0) { const empty = document.createElement('p'); empty.className = 'text-sm text-gray-500 italic'; From 66a306eb66a2dc72cb66b0ec19fdd2a6a12592b1 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 21:02:23 -0400 Subject: [PATCH 7/7] test(js): register the order-list suite apart from other branches' suites Co-Authored-By: Claude Opus 5.5 --- test/js/README.md | 2 +- test/js/run_all.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/js/README.md b/test/js/README.md index 3768ef589..db1f81ebc 100644 --- a/test/js/README.md +++ b/test/js/README.md @@ -46,6 +46,7 @@ server has none. | `unit/test_list_filter.js` | no | `ListFilter` search/filter/sort/count/sticky, and the installed-plugins config **extracted verbatim** from `plugins_manager.js` so the test can't drift from it | | `unit/test_update_all.js` | no | `PluginInstallManager.updateAll` from `plugins/install_manager.js`: Check & Update All sends only plugin ids (never `starlark:` app entries), re-sends a request that got no HTTP answer (web service restarting) instead of skipping that plugin, never re-sends one that got any HTTP answer (the real `api_client.js` classifies a proxy 502 or a JSON error without `error_code` as `API_ERROR`), and counts a no-op update as already up to date in the summary. Also run by `test/web_interface/test_update_all_plugins.py` so CI covers it | | `unit/test_render_cards.js` | no | `renderInstalledCards` markup, both empty states, and HTML-escaping of hostile plugin metadata | +| `unit/test_plugin_order_list.js` | no | `widgets/plugin-order-list.js` (the Vegas and rotation order lists): a disabled plugin, which gets no row, keeps its slot in the saved order and its Vegas exclusion when the list rewrites its hidden inputs, around reordering and include/exclude; an uninstalled plugin's id is dropped, a failed plugin list leaves the inputs as saved, and only string ids are carried over, once each | | `unit/test_style_editor_element_keys.js` | no | `elementKeys()`/`styleRows()`/`positionRows()` from `widgets/style-editor.js`: every `customization.layout` entry gets exactly one row -- paired with its style element through core's `x-layout-key` (so `score` belongs to `score_text`, not a second row), or a position row of its own, leaves included -- since the widget claims the whole `layout` block from the generic fallback renderer | | `unit/test_style_editor_layout_leaf_columns.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only key whose own value is a leaf (no x/y sub-object, e.g. a `show_logo` toggle) gets a self-keyed column instead of a blank, uneditable row | | `unit/test_style_editor_layout_leaf_collision.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only leaf key still gets its own column even when its name collides with an unrelated element's style sub-field or another layout axis's sub-field | @@ -53,7 +54,6 @@ server has none. | `unit/test_store_registry_fields.js` | no | The store card's registry fields from `plugins_manager.js`: the commit that introduced the listed version (a hex SHA only, linked to that tree), the "Needs LEDMatrix X+" warning, a card from an older registry without either, and `isStorePluginInstalled` answering to `aliases` | | `unit/test_page_registry.js` | no | The page lifecycle in `js/core/registry.js` (a minimal DOM shim): one `init` per `data-page` root, `destroy` and an aborted `ctx.signal` when htmx swaps it away, a vetoed swap keeps it, lazy page modules, a root removed without htmx swept on the next swap | | `unit/test_core_modules.js` | no | `js/core/api.js` (JSON envelope, HTTP/`status: error`/network errors, abort passthrough, the #683 login redirect, same-server paths only) and `js/core/facade.js` (`window.LEDMatrix`, deprecated aliases) | -| `unit/test_plugin_order_list.js` | no | `widgets/plugin-order-list.js` (the Vegas and rotation order lists): a disabled plugin, which gets no row, keeps its slot in the saved order and its Vegas exclusion when the list rewrites its hidden inputs, around reordering and include/exclude; an uninstalled plugin's id is dropped, a failed plugin list leaves the inputs as saved, and only string ids are carried over, once each | | `unit/test_plugin_action_delegation.js` | no | The document-level card-action delegation and `handlePluginAction` from `plugins_manager.js`, run with the handler inside an IIFE as in the real file: each action is handled once, a Starlark app uninstall goes to `DELETE /starlark/apps/`, and an uninstall is confirmed once | | `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup | | `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry | diff --git a/test/js/run_all.js b/test/js/run_all.js index ba5584e4f..16b401a7b 100755 --- a/test/js/run_all.js +++ b/test/js/run_all.js @@ -17,14 +17,14 @@ const fs = require('fs'); const BASE = process.env.BASE || 'http://localhost:5000'; const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js', + 'unit/test_plugin_order_list.js', 'unit/test_html_escaping.js', 'unit/test_style_editor_element_keys.js', 'unit/test_style_editor_layout_leaf_columns.js', 'unit/test_style_editor_layout_leaf_collision.js', 'unit/test_update_all.js', 'unit/test_inline_handler_escaping.js', 'unit/test_plugin_action_delegation.js', 'unit/test_file_upload_widget.js', 'unit/test_store_registry_fields.js', 'unit/test_restart_banner.js', - 'unit/test_page_registry.js', 'unit/test_core_modules.js', - 'unit/test_plugin_order_list.js']; + 'unit/test_page_registry.js', 'unit/test_core_modules.js']; const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js', 'dom/test_tools_sections.js', 'dom/test_cache_page.js', 'dom/test_durations_page.js', 'dom/test_operation_history_page.js',