Merge origin/main into claude/fonts-used-by

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

web_interface/blueprints/api_v3/__init__.py

@@ -49,6 +49,7 @@ from src.web_interface.validators import (
 from src.error_aggregator import get_error_aggregator
 from src.common.permission_utils import install_requirements_file
 from src.common.path_safety import resolve_under
+from src.device_location import DeviceLocationResolver, apply_device_location
 _SUDO = shutil.which('sudo')
 _JOURNALCTL = shutil.which('journalctl')
 _GIT = shutil.which('git')
@@ -1261,9 +1262,8 @@ def _enhance_schema_with_core_properties(schema):
     """
     Enhance schema with the core-owned per-plugin properties.
 
-    ``enabled``, ``display_duration``, ``live_priority``, ``skin``,
-    ``skin_options`` and the ``vegas_*`` tuning keys are system-managed and
-    always allowed, even when the plugin's schema doesn't declare them. The
+    ``enabled``, ``display_duration``, ``live_priority`` and the ``vegas_*``
+    tuning keys are system-managed and always allowed, even when the plugin's schema doesn't declare them. The
     list is ``schema_manager.CORE_PLUGIN_PROPERTIES``, the one validation uses,
     so the save filter keeps exactly what validation accepts.
 
@@ -1704,6 +1704,32 @@ def _validate_starlark_app_path(app_id: str) -> Tuple[Optional[Path], Optional[s
     except OSError as e:
         logger.warning("Path validation error for app_id %r: %s", app_id, e)
         return None, "Invalid app_id"
+_starlark_device_location: Optional[DeviceLocationResolver] = None
+
+
+def _get_starlark_device_location() -> DeviceLocationResolver:
+    """One resolver per process, so a geocode failure's backoff is shared."""
+    global _starlark_device_location
+    if _starlark_device_location is None:
+        _starlark_device_location = DeviceLocationResolver(
+            getattr(api_v3, 'cache_manager', None) or _ensure_cache_manager(), logger)
+    return _starlark_device_location
+
+
+def _read_starlark_schema(app_dir: Path) -> Optional[Dict[str, Any]]:
+    """An installed app's schema.json, or None if it has none or it's unreadable."""
+    schema_file = app_dir / 'schema.json'
+    if not schema_file.exists():
+        return None
+    try:
+        with open(schema_file) as f:
+            schema = json.load(f)
+    except (OSError, json.JSONDecodeError) as e:
+        logger.warning("Could not read schema.json at %s: %s", schema_file, e)
+        return None
+    return schema if isinstance(schema, dict) else None
+
+
 def _standalone_render_starlark_app(app_id: str) -> Tuple[bool, int, Optional[str]]:
     """Render a Starlark app via pixlet directly (no plugin required).
 
@@ -1775,6 +1801,9 @@ def _standalone_render_starlark_app(app_id: str) -> Tuple[bool, int, Optional[st
 
     INTERNAL_KEYS = {'render_interval', 'display_duration'}
     pixlet_config = {k: v for k, v in app_config.items() if k not in INTERNAL_KEYS}
+    pixlet_config = apply_device_location(
+        pixlet_config, _read_starlark_schema(app_dir),
+        _get_starlark_device_location(), full_config)
 
     output_path = str(app_dir / 'cached_render.webp')
     cmd = [pixlet_path, 'render', str(star_file)]

web_interface/blueprints/api_v3/config.py

@@ -1056,10 +1056,14 @@ def save_main_config():
                 if error:
                     return error
 
-                # Deep merge regular config into main config
+                # Deep merge regular config into main config, dropping
+                # retired core keys (skin, skin_options) from the stored section
+                from src.plugin_system.schema_manager import drop_retired_plugin_keys
                 stored_section = current_config.get(plugin_id)
                 current_config[plugin_id] = deep_merge(
-                    stored_section if isinstance(stored_section, dict) else {}, regular_config)
+                    drop_retired_plugin_keys(
+                        stored_section if isinstance(stored_section, dict) else {}, schema),
+                    regular_config)
                 if secrets_config:
                     plugin_secrets_updates[plugin_id] = secrets_config
 

web_interface/blueprints/api_v3/misc.py

@@ -1,5 +1,5 @@
 """Routes with no larger group of their own: errors, integrations,
-cache, sync, skins, logs, health and hardware.
+cache, sync, logs, health and hardware.
 
 Routes decorate the shared `api_v3` Blueprint from ._common, so their
 endpoint names are unchanged by living here.
@@ -10,10 +10,11 @@ from web_interface.blueprints.api_v3 import (
     _MQTT_BRIDGE_DIR, _SUDO, _coerce_mqtt_bridge_value,
     _get_display_service_status, _mqtt_bridge_service_state,
     _read_mqtt_bridge_config, api_v3, contextlib, describe_exception,
-    error_response, get_error_aggregator, json, jsonify, logger, os, request,
+    error_response, json, jsonify, logger, os, redact_text, request,
     subprocess, success_response, tempfile,
 )
 from src.common.path_safety import safe_path_component
+from src import error_aggregator as _errors
 import web_interface.blueprints.api_v3 as _pkg
 # Read through the module rather than bound by value: tests patch these
 # as module attributes, and a value binding would not see the patch.
@@ -149,51 +150,6 @@ def get_hardware_status():
     except Exception:
         logger.error("Unexpected error reading hardware status", exc_info=True)
         return jsonify({"status": "error", "message": "Unable to read hardware status"}), 500
-@api_v3.route('/skins', methods=['GET'])
-def list_skins():
-    """List installed visual skins (docs/SKIN_SYSTEM.md).
-
-    Optional ?plugin_id=... filters to skins matching that plugin.
-
-    The response carries ``supported: false`` and a ``message``: the current
-    scoreboard plugins don't render skins, so a client must not present
-    these as selectable.
-    """
-    try:
-        from src.skin_system import (
-            SKINS_RENDER_SUPPORTED, SKINS_UNSUPPORTED_MESSAGE, skin_runtime,
-        )
-
-        plugin_id = request.args.get('plugin_id')
-        if plugin_id:
-            skins = skin_runtime.skins_for_plugin(plugin_id)
-        else:
-            # The discovery cache self-invalidates on directory/manifest
-            # mtime changes, so no force_refresh — keeps Pi disk I/O down.
-            skins = skin_runtime.discover_skins()
-
-        payload = []
-        for skin_id, manifest in sorted(skins.items()):
-            skin_dir = Path(manifest['_skin_dir'])
-            preview = manifest.get('preview')
-            payload.append({
-                'id': skin_id,
-                'name': manifest.get('name', skin_id),
-                'version': manifest.get('version'),
-                'author': manifest.get('author'),
-                'description': manifest.get('description', ''),
-                'skin_api_version': manifest.get('skin_api_version'),
-                'targets': manifest.get('targets', {}),
-                'modes': manifest.get('modes', []),
-                'has_preview': bool(preview and (skin_dir / preview).is_file()),
-            })
-        data = {'skins': payload, 'supported': SKINS_RENDER_SUPPORTED}
-        if not SKINS_RENDER_SUPPORTED:
-            data['message'] = SKINS_UNSUPPORTED_MESSAGE
-        return jsonify({'status': 'success', 'data': data})
-    except Exception as e:
-        logger.error('Error in list_skins', exc_info=True)
-        return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
 @api_v3.route('/logs', methods=['GET'])
 def get_logs():
     """Get system logs from journalctl"""
@@ -326,17 +282,62 @@ def delete_cache_file():
     except Exception as e:
         logger.error('Error in delete_cache_file', exc_info=True)
         return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
+def _errors_cache():
+    """The shared cache the display service publishes its errors to."""
+    if not api_v3.cache_manager:
+        from src.cache_manager import CacheManager
+        api_v3.cache_manager = CacheManager()
+    return api_v3.cache_manager
+
+
+def _redact_error_text(text, keep_lines=False):
+    """Credentials out of plugin exception text, which can quote a URL with
+    an API key in it. Stack traces keep their line breaks and indentation."""
+    if not isinstance(text, str):
+        return text
+    if not keep_lines:
+        return redact_text(text, max_length=len(text) + 1)
+    return '\n'.join(
+        line[:len(line) - len(line.lstrip())] + redact_text(line, max_length=len(line) + 1)
+        for line in text.splitlines()
+    )
+
+
+def _redact_error_record(record):
+    if not isinstance(record, dict):
+        return record
+    record = dict(record)
+    record['message'] = _redact_error_text(record.get('message'))
+    record['stack_trace'] = _redact_error_text(record.get('stack_trace'), keep_lines=True)
+    if isinstance(record.get('context'), dict):
+        record['context'] = {k: _redact_error_text(v) for k, v in record['context'].items()}
+    return record
+
+
+def _read_errors():
+    snapshot, clear_request = _errors.read_error_report(_errors_cache())
+    return snapshot, clear_request
+
+
 @api_v3.route('/errors/summary', methods=['GET'])
 def get_error_summary():
     """
     Get summary of all errors for monitoring and debugging.
 
-    Returns error counts, detected patterns, and recent errors.
+    Returns error counts, detected patterns, and recent errors, as last
+    reported by the display service (which runs the plugins, so it is the
+    only process that records their errors). ``snapshot_available`` is false
+    until it has reported; ``generated_at`` says when it did.
     """
     try:
-        aggregator = get_error_aggregator()
-        summary = aggregator.get_error_summary()
-        return success_response(data=summary, message="Error summary retrieved")
+        summary = _errors.error_summary_from_report(*_read_errors())
+        summary['recent_errors'] = [_redact_error_record(r) for r in summary['recent_errors']]
+        for pattern in summary['active_patterns'].values():
+            if isinstance(pattern, dict) and isinstance(pattern.get('sample_messages'), list):
+                pattern['sample_messages'] = [_redact_error_text(m) for m in pattern['sample_messages']]
+        message = ("Error summary retrieved" if summary['snapshot_available']
+                   else "The display service has not reported any errors yet")
+        return success_response(data=summary, message=message)
     except Exception as e:
         logger.error(f"Error getting error summary: {e}", exc_info=True)
         return error_response(
@@ -352,11 +353,13 @@ def get_plugin_errors(plugin_id):
     Args:
         plugin_id: Plugin identifier
 
-    Returns health status and error statistics for the plugin.
+    Returns health status and error statistics for the plugin, from the
+    display service's last report (see get_error_summary). A plugin with no
+    recorded errors is "healthy".
     """
     try:
-        aggregator = get_error_aggregator()
-        health = aggregator.get_plugin_health(plugin_id)
+        health = _errors.plugin_health_from_report(*_read_errors(), plugin_id)
+        health['last_error'] = _redact_error_record(health['last_error'])
         return success_response(data=health, message="Plugin health retrieved")
     except Exception as e:
         logger.error(f"Error getting plugin health for {plugin_id}: {e}", exc_info=True)
@@ -372,42 +375,61 @@ def clear_old_errors():
 
     Request body (optional):
         max_age_hours: Maximum age in hours (default: 24, max: 8760 = 1 year)
+        all: true clears every error recorded so far (max_age_hours ignored)
+
+    The errors live in the display service, so this records a clear request
+    that it applies within a few seconds. Reads hide the cleared errors from
+    the moment the request is recorded.
     """
     try:
         data = request.get_json(silent=True) or {}
+        clear_all = _coerce_to_bool(data.get('all'))
         raw_max_age = data.get('max_age_hours', 24)
 
         # Validate and coerce max_age_hours
+        max_age_hours = None
+        if not clear_all:
+            try:
+                max_age_hours = int(raw_max_age)
+                if max_age_hours < 1:
+                    return error_response(
+                        error_code=ErrorCode.INVALID_INPUT,
+                        message="max_age_hours must be at least 1",
+                        context={'provided_value': raw_max_age},
+                        status_code=400
+                    )
+                if max_age_hours > 8760:  # 1 year max
+                    return error_response(
+                        error_code=ErrorCode.INVALID_INPUT,
+                        message="max_age_hours cannot exceed 8760 (1 year)",
+                        context={'provided_value': raw_max_age},
+                        status_code=400
+                    )
+            except (ValueError, TypeError, OverflowError):
+                return error_response(
+                    error_code=ErrorCode.INVALID_INPUT,
+                    message="max_age_hours must be a valid integer",
+                    context={'provided_value': str(raw_max_age)},
+                    status_code=400
+                )
+
+        now = _pkg.time.time()
+        cutoff = now if clear_all else now - max_age_hours * 3600
         try:
-            max_age_hours = int(raw_max_age)
-            if max_age_hours < 1:
-                return error_response(
-                    error_code=ErrorCode.INVALID_INPUT,
-                    message="max_age_hours must be at least 1",
-                    context={'provided_value': raw_max_age},
-                    status_code=400
-                )
-            if max_age_hours > 8760:  # 1 year max
-                return error_response(
-                    error_code=ErrorCode.INVALID_INPUT,
-                    message="max_age_hours cannot exceed 8760 (1 year)",
-                    context={'provided_value': raw_max_age},
-                    status_code=400
-                )
-        except (ValueError, TypeError, OverflowError):
+            result = _errors.request_error_clear(_errors_cache(), cutoff)
+        except OSError as e:
+            logger.error("Could not record an error clear request: %s", e)
             return error_response(
-                error_code=ErrorCode.INVALID_INPUT,
-                message="max_age_hours must be a valid integer",
-                context={'provided_value': str(raw_max_age)},
-                status_code=400
+                error_code=ErrorCode.SYSTEM_ERROR,
+                message="Could not record the clear request in the shared cache",
+                status_code=500
             )
 
-        aggregator = get_error_aggregator()
-        cleared_count = aggregator.clear_old_records(max_age_hours=max_age_hours)
-
+        scope = "all errors" if clear_all else f"errors older than {max_age_hours} hours"
         return success_response(
-            data={'cleared_count': cleared_count},
-            message=f"Cleared {cleared_count} error records older than {max_age_hours} hours"
+            data=result,
+            message=(f"Clear of {scope} requested; the display service applies it "
+                     f"within about {int(_errors.SNAPSHOT_TICK_INTERVAL)} seconds")
         )
     except Exception as e:
         logger.error(f"Error clearing old errors: {e}", exc_info=True)

web_interface/blueprints/api_v3/plugins.py

@@ -1296,15 +1296,16 @@ def install_plugin():
         plugin_id = data['plugin_id']
         branch = data.get('branch')  # Optional branch parameter
 
-        # A registry skin would install but never render with the current
-        # scoreboards; refuse it with the reason rather than a generic failure
+        # A registry entry that isn't a plugin (a custom registry can still
+        # list old "type": "skin" entries) gets a clear refusal, not a failed
+        # install.
         try:
             registry_entry = api_v3.plugin_store_manager.get_registry_info(plugin_id)
         except Exception:
             registry_entry = None
-        if isinstance(registry_entry, dict) and (registry_entry.get('type') or 'plugin') == 'skin':
-            from src.skin_system import SKINS_UNSUPPORTED_MESSAGE
-            return jsonify({'status': 'error', 'message': SKINS_UNSUPPORTED_MESSAGE}), 400
+        if isinstance(registry_entry, dict) and not api_v3.plugin_store_manager.is_plugin_entry(registry_entry):
+            return jsonify({'status': 'error',
+                            'message': f"{plugin_id} is a {registry_entry.get('type')!r} entry, not a plugin"}), 400
 
         # Install the plugin
         # Log the plugins directory being used for debugging
@@ -1506,13 +1507,10 @@ def get_registry_from_url():
         registry = api_v3.plugin_store_manager.fetch_registry_from_url(repo_url)
 
         if registry:
-            # Skins aren't offered: current scoreboards don't render them
             return jsonify({
                 'status': 'success',
-                'plugins': [
-                    p for p in registry.get('plugins', [])
-                    if not (isinstance(p, dict) and (p.get('type') or 'plugin') == 'skin')
-                ],
+                'plugins': [p for p in registry.get('plugins', [])
+                            if api_v3.plugin_store_manager.is_plugin_entry(p)],
                 'registry_url': repo_url
             })
         else:
@@ -1627,9 +1625,7 @@ def list_plugin_store():
         # Format plugins for the web interface
         formatted_plugins = []
         for plugin in plugins:
-            # Registry skins install but never render with the current
-            # scoreboards, so the store doesn't offer them
-            if (plugin.get('type') or 'plugin') == 'skin':
+            if not api_v3.plugin_store_manager.is_plugin_entry(plugin):
                 continue
             formatted_plugins.append({
                 'id': plugin.get('id'),
@@ -2210,7 +2206,10 @@ def save_plugin_config():
         if plugin_id not in current_config:
             current_config[plugin_id] = {}
 
-        current_config[plugin_id] = deep_merge(current_config[plugin_id], regular_config)
+        # Retired core keys (skin, skin_options) leave the stored section here
+        from src.plugin_system.schema_manager import drop_retired_plugin_keys
+        current_config[plugin_id] = deep_merge(
+            drop_retired_plugin_keys(current_config[plugin_id], schema), regular_config)
 
         # Deep merge plugin secrets in secrets config
         if secrets_config:
@@ -2755,11 +2754,6 @@ def get_plugin_schema():
         schema = schema_mgr.load_schema(plugin_id, use_cache=True)
 
         if schema:
-            # No "Visual Skin" dropdown: the current scoreboard plugins don't
-            # render skins (src.skin_system.SKINS_UNSUPPORTED_MESSAGE), so
-            # schema_mgr.inject_skin_selector is deliberately not called. A
-            # stored "skin" value is unaffected: validation still allows it
-            # and a form save deep-merges over the stored section, keeping it.
             return jsonify({'status': 'success', 'data': {'schema': schema}})
 
         # Return a simple default schema if file not found

web_interface/blueprints/api_v3/starlark.py

@@ -26,6 +26,36 @@ import web_interface.blueprints.api_v3 as _pkg
 # package is the only patch point that covers every caller.
 
 
+def _ownership_hint(err: BaseException):
+    """An actionable message when the apps directory is not writable.
+
+    The display service runs as root and the web interface as the login user
+    (see systemd/ledmatrix.service and systemd/ledmatrix-web.service). The
+    starlark-apps directory is not in the repository, so whichever service
+    reaches it first creates it -- and when that is the display service, the
+    web user cannot write into it and every install fails.
+
+    The plugin now hands the directory back on startup, so this should not be
+    reachable. It is kept because the failure is otherwise invisible: the
+    generic message names no path and no cause, and the one user who hit it
+    had to read the service logs to find it. If the handover is ever prevented
+    -- an exotic mount, a directory root-owned for another reason -- this says
+    what to do instead of costing somebody an evening.
+
+    Returns None when `err` is not a permission problem.
+    """
+    if not isinstance(err, PermissionError):
+        return None
+    return (
+        f"Cannot write to {_STARLARK_APPS_DIR}. It is owned by another user "
+        f"-- usually because the display service, which runs as root, created "
+        f"it before the web interface did. Restarting the display service "
+        f"(sudo systemctl restart ledmatrix) repairs the ownership "
+        f"automatically. To fix it by hand: "
+        f"sudo chown -R $USER:$USER {_STARLARK_APPS_DIR}"
+    )
+
+
 @api_v3.route('/starlark/status', methods=['GET'])
 def get_starlark_status():
     """Get Starlark plugin status and Pixlet availability."""
@@ -278,7 +308,8 @@ def upload_starlark_app():
         # without it, though, and describe_exception redacts credentials and
         # truncates -- the same trade-off every other handler here makes.
         logger.exception("[Starlark] File error uploading starlark app: %s", err)
-        return jsonify({'status': 'error', 'message': 'File error during upload',
+        return jsonify({'status': 'error',
+                        'message': _ownership_hint(err) or 'File error during upload',
                         'details': describe_exception(err)}), 500
     except ImportError as err:
         logger.exception("[Starlark] Module load error uploading starlark app: %s", err)
@@ -702,6 +733,16 @@ def install_from_tronbyte_repository():
 
     except Exception as e:
         logger.exception("[Starlark] install_from_tronbyte_repository failed")
+        hint = _ownership_hint(e)
+        if hint:
+            # `details` is kept deliberately. CodeQL flags it as information
+            # exposure, but this package's rule is "if it returns 5xx, it says
+            # why" -- enforced by test_no_api_v3_handler_discards_its_exception,
+            # whose PRE_EXISTING allowance may shrink and never grow. The
+            # Starlark routes are exactly the ones that policy was written for:
+            # they answered 500 with no detail for three releases. It stays.
+            return jsonify({'status': 'error', 'message': hint,
+                            'details': describe_exception(e)}), 500
         return jsonify({'status': 'error', 'message': 'Failed to install from repository', 'details': describe_exception(e)}), 500
 @api_v3.route('/starlark/repository/categories', methods=['GET'])
 def get_tronbyte_categories():