feat(starlark): integrate Starlark/Pixlet apps as virtual plugins

[ChuckBuilds]

Summary

Adds a new starlark-apps plugin that integrates Pixlet-rendered Starlark apps as virtual plugins in the LEDMatrix plugin system.

• One-click Pixlet installer — downloads the correct ARM64 Linux binary
• App management — upload .star files, configure per-app settings, enable/disable
• Repository browser — browse and install apps from the Tronbyte community repository
• Auto-scaling — renders Pixlet output (32×64 native) and scales to your display dimensions
• Web UI integration — new Starlark tab in the web interface with HTMX-based dynamic loading

Security: HTML sanitization, input validation, path traversal prevention, sandboxed temp directories.

Cleanup in this PR vs original branch:

• Removed 27 debug console.log statements from starlark_apps.js
• Replaced traceback.print_exc() with logger.error(..., exc_info=True) in API handlers
• Removed football-scoreboard debug print() blocks from config save handler
• Removed stale planning docs, .claude/ settings, and accidental files committed to branch

Note: Requires Pixlet binary (ARM64 Linux only). Install via the web UI installer button or scripts/download_pixlet.sh.

Test plan

• Navigate to Starlark tab in web UI
• Use "Install Pixlet" button — verify binary downloads and plugin auto-enables
• Upload a .star file — verify it renders on the display
• Browse repository — verify apps grid loads and an app can be installed
• Restart service: sudo systemctl restart ledmatrix — check journalctl -u ledmatrix for errors

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features

• Starlark Apps Support: Added ability to install, manage, and render Starlark (.star) apps through a new plugin system with upload, configuration, and enable/disable controls.
• Repository Browser: Integrated Tronbyte repository integration for discovering and installing apps directly from an external repository with search and category filtering.
• Pixlet Rendering: Added support for rendering Starlark apps via Pixlet with configurable timeouts, caching, scaling, and frame optimization.
• Web Interface Updates: New UI for app management, including upload modal, configuration interface, and repository browser with status indicators.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces comprehensive Starlark Apps plugin support to LEDMatrix, enabling users to render and display Starlark (.star) apps. It adds a complete backend plugin with Pixlet integration, frame extraction, app management, and GitHub repository discovery, alongside extensive REST API endpoints and a full-featured web UI with modals for upload, configuration, and repository browsing.

Changes

Cohort / File(s)	Summary
Configuration & Manifest .gitignore, plugin-repos/starlark-apps/manifest.json, plugin-repos/starlark-apps/config_schema.json	Added repository ignore rules for Pixlet binaries and Starlark apps directory; plugin manifest with metadata and dependencies; JSON Schema defining plugin configuration options for rendering timeouts, caching, scaling, magnification, and transition effects.
Core Plugin Implementation plugin-repos/starlark-apps/__init__.py, plugin-repos/starlark-apps/manager.py, plugin-repos/starlark-apps/pixlet_renderer.py, plugin-repos/starlark-apps/frame_extractor.py	Implemented complete Starlark Apps plugin with StarlarkApp and StarlarkAppsPlugin classes; PixletRenderer wraps Pixlet CLI with auto-detection, rendering, and schema extraction; FrameExtractor processes WebP frames with scaling, centering, optimization, and GIF conversion; includes app lifecycle management (install/uninstall), manifest handling, configuration validation, and caching.
Repository Integration plugin-repos/starlark-apps/tronbyte_repository.py	Added TronbyteRepository class to interface with GitHub-hosted Starlark apps repository; supports listing apps, fetching metadata, downloading files, searching, filtering by category, and rate-limit tracking.
Installation Infrastructure scripts/download_pixlet.sh	New shell script for downloading and installing Pixlet binaries for multiple architectures from Tronbyte fork; supports auto-detection of latest version and incremental installation.
Dependency Management scripts/utils/start_web_conditionally.py	Updated pip install flow to remove --ignore-installed flag and change user-facing message from "Installing dependencies..." to "Checking dependencies..."
Web API Endpoints web_interface/blueprints/api_v3.py	Added 12+ new REST endpoints for Starlark app management (status, list, detail, upload, install/uninstall, config get/update, toggle, render); integrated TronbyteRepository for app discovery; added repository browsing, filtering, and installation endpoints; includes Pixlet installer endpoint and configuration validation helpers.
Web UI - Routing & Layout web_interface/blueprints/pages_v3.py, web_interface/templates/v3/base.html	Added _load_starlark_apps_partial() loader; base template now distinguishes Starlark apps from regular plugins via icon selection and metadata attributes; special-case handling in plugin config loading for starlark-apps plugin.
Web UI - Templates & Frontend web_interface/templates/v3/partials/starlark_apps.html, web_interface/static/v3/js/starlark_apps.js	Created comprehensive Starlark Apps UI with header, status banner, app controls, and responsive app grid; four modals for upload (drag-and-drop), configuration, and repository browser; frontend JavaScript module with file upload, app management (toggle/configure/render/uninstall), repository browsing, category filtering, Pixlet installation, XSS sanitization, and event delegation.

Sequence Diagram

sequenceDiagram
    actor User
    participant WebUI as Web UI
    participant API as Web API<br/>(api_v3.py)
    participant Plugin as StarlarkAppsPlugin<br/>(manager.py)
    participant Renderer as PixletRenderer
    participant Extractor as FrameExtractor
    participant Display as Display Manager

    User->>WebUI: Select app to render
    WebUI->>API: POST /render_starlark_app
    API->>Plugin: render_app()
    
    Plugin->>Plugin: Load app config
    Plugin->>Renderer: render(star_file, config)
    Renderer->>Renderer: Find Pixlet binary
    Renderer->>Renderer: Execute pixlet render
    Renderer-->>Plugin: (success, webp_path)
    
    Plugin->>Extractor: load_webp(webp_path)
    Extractor->>Extractor: Parse WebP frames
    Extractor-->>Plugin: (frames, delays)
    
    Plugin->>Extractor: scale_frames(frames)
    Extractor-->>Plugin: (scaled_frames)
    
    Plugin->>Extractor: center_frames(frames)
    Extractor-->>Plugin: (centered_frames)
    
    Plugin->>Extractor: frames_to_gif_data(frames)
    Extractor-->>Plugin: (gif_bytes)
    
    Plugin->>Display: queue_display(gif_data)
    Display-->>Plugin: acked
    Plugin-->>API: {success, frames_count, duration}
    API-->>WebUI: {success, data}
    WebUI-->>User: Render complete, display updated

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: Starlark Apps Integration with Schema-Driven Config + Security Hardening #253 — Implements identical Starlark Apps integration plugin with same core modules (manager.py, pixlet_renderer.py, frame_extractor.py, config_schema.json, download_pixlet script, web API and frontend changes).
• fix(web-ui): Fix file upload widget and plugin action buttons #165 — Modifies shared web UI infrastructure including api_v3.py, pages_v3.py, and plugin config loading logic that is now extended for Starlark app support.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(starlark): integrate Starlark/Pixlet apps as virtual plugins' clearly and specifically describes the main change: integrating Starlark/Pixlet apps as virtual plugins in the LEDMatrix system.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch starlark

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/utils/start_web_conditionally.py (1)

32-38: ⚠️ Potential issue | 🟡 Minor

Use tempfile for the fallback requirements file

The fixed path requirements_temp.txt is vulnerable to a concurrent-write race if the systemd service is restarted while a previous instance is still running (e.g., rapid restart or watchdog). Use tempfile.mkstemp to avoid collisions and ensure atomic cleanup.

♻️ Proposed fix

+import tempfile
 ...
-                temp_reqs = os.path.join(PROJECT_DIR, 'web_interface', 'requirements_temp.txt')
-                with open(temp_reqs, 'w') as f:
+                fd, temp_reqs = tempfile.mkstemp(suffix='.txt', prefix='reqs_')
+                with os.fdopen(fd, 'w') as f:
                     f.writelines(filtered_lines)
                 try:
                     subprocess.check_call([
                         sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', temp_reqs
-                    ])
+                    ])  # noqa: S603
                     print("Dependencies installed successfully (psutil skipped - using system version)")
                 finally:
                     if os.path.exists(temp_reqs):
                         os.remove(temp_reqs)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/utils/start_web_conditionally.py` around lines 32 - 38, The current
use of a fixed filename for temp_reqs is vulnerable to concurrent-write races;
replace the static path creation with a secure temporary file using
tempfile.mkstemp(), write filtered_lines to the fd (or open the returned path),
close the descriptor, pass that temporary path to
subprocess.check_call(sys.executable, '-m', 'pip', 'install',
'--break-system-packages', '-r', temp_path), and ensure the temp file is
unlinked in a finally block to guarantee cleanup even on errors; update
references to PROJECT_DIR/temp_reqs to use the mkstemp-generated temp_path.

🧹 Nitpick comments (10)

scripts/utils/start_web_conditionally.py (1)

12-12: Add return type hint to install_dependencies()

Per coding guidelines, function return values require type hints.

♻️ Proposed fix

-def install_dependencies():
+def install_dependencies() -> bool:

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/utils/start_web_conditionally.py` at line 12, The function
install_dependencies() is missing a return type hint; update its signature to
include the appropriate type annotation (e.g., -> None or the actual return
type) so it complies with the project's typing guidelines, and update any
callers or docstring if necessary to reflect the annotated return type; locate
the function install_dependencies in scripts/utils/start_web_conditionally.py
and add the correct return type hint to its definition.

plugin-repos/starlark-apps/pixlet_renderer.py (1)

17-46: Add a consistent log context prefix for Pixlet renderer messages.
Helps correlate logs on the Pi (e.g., [Starlark Pixlet] prefix or a LoggerAdapter).

As per coding guidelines: Use structured logging with context (e.g., [NHL Recent]) for logging messages.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/pixlet_renderer.py` around lines 17 - 46, The
PixletRenderer logs use the module-level logger without a consistent context
prefix; create a LoggerAdapter (or small wrapper) that injects a consistent
prefix like "[Starlark Pixlet]" and use it inside PixletRenderer (e.g., assign
self.logger = logging.LoggerAdapter(logger, {...}) during initialization) and
replace the module logger calls in PixletRenderer.__init__ (logger.info /
logger.warning) with self.logger.info / self.logger.warning so all messages
include the required structured context.

web_interface/blueprints/pages_v3.py (1)

315-316: Ruff: two minor warnings in the new function (consistent with existing file pattern, flagged by static analysis).

• BLE001 (line 315): except Exception is a broad blind catch. Every other partial loader in this file uses the same pattern, so the blast radius is file-wide, not just here.
• RUF010 (line 316): Prefer the explicit conversion flag {e!s} over str(e) inside f-strings.

🔧 Suggested fix (RUF010 only — BLE001 would require a broader file-wide sweep)

-        return f"Error: {str(e)}", 500
+        return f"Error: {e!s}", 500

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/blueprints/pages_v3.py` around lines 315 - 316, In the except
block that returns the error response, avoid calling str(e) in the f-string and
use the f-string conversion flag instead (change f"Error: {str(e)}" to f"Error:
{e!s}") to satisfy RUF010; leave the broad `except Exception` as-is for now
(BLE001 requires a file-wide refactor) and ensure you only update the return
expression in that handler.

web_interface/static/v3/js/starlark_apps.js (1)

163-166: No response.ok check before response.json() — pattern repeated across all fetch calls.

If the server returns a non-2xx status with a non-JSON body (e.g., a 502 from a reverse proxy), .json() throws a confusing parse error. This pattern repeats throughout the file (Lines 250, 469, 494, 516, 676, 707, 809, 849, 984, 1026). Consider a small helper:

Example helper and usage

async function fetchJson(url, options = {}) {
    const response = await fetch(url, options);
    if (!response.ok) {
        const text = await response.text().catch(() => '');
        throw new Error(`HTTP ${response.status}: ${text.slice(0, 200)}`);
    }
    return response.json();
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/static/v3/js/starlark_apps.js` around lines 163 - 166, The
fetch calls (e.g., in loadStarlarkStatus) call response.json() without checking
response.ok, causing misleading parse errors on non-2xx responses; add a small
helper like fetchJson(url, options) that awaits fetch, throws a clear Error
containing response.status and a truncated response.text() when !response.ok,
and returns response.json() otherwise, then replace direct fetch+response.json()
usages (notably loadStarlarkStatus and the other fetch sites listed) to call
fetchJson so failures surface a clear HTTP status and body snippet instead of a
JSON parse error.

plugin-repos/starlark-apps/manager.py (2)

640-648: Unbounded frame storage could exhaust Raspberry Pi memory.

max_frames defaults to None (no limit, Line 641). Each frame is a PIL Image object held in memory. A single animated WebP can have dozens of frames, and with multiple enabled apps, memory usage can accumulate quickly on a Raspberry Pi's limited RAM. Consider setting a sensible default (e.g., 50 frames per app) rather than relying on users to configure it.

Suggested default

             # Optimize frames to limit memory usage (max_frames=None means no limit)
-            max_frames = self.config.get("max_frames")
+            max_frames = self.config.get("max_frames", 50)
             if max_frames is not None:
                 frames = self.extractor.optimize_frames(frames, max_frames=max_frames)

As per coding guidelines: "Optimize code for Raspberry Pi's limited RAM and CPU capabilities" and "Clean up resources regularly to manage memory effectively."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/manager.py` around lines 640 - 648, The code
currently leaves max_frames as None which allows unbounded PIL Image frame
retention; change the logic where max_frames is obtained
(self.config.get("max_frames")) to apply a sensible default (e.g.,
DEFAULT_MAX_FRAMES = 50) when the value is missing or explicitly None, then pass
that bounded max_frames into self.extractor.optimize_frames(frames,
max_frames=max_frames) so app.frames only retains at most the default/
configured number of frames; update references to max_frames and consider adding
a module-level DEFAULT_MAX_FRAMES constant and using it in the code that sets
max_frames to ensure Raspberry Pi memory is constrained.

---

83-83: Redundant {e} in logger.exception() calls, and logger.error() used where logger.exception() is more appropriate.

logging.exception() already appends the exception and traceback, so including {e} in the format string is redundant (Lines 83, 336, 479, 484). Additionally, several except blocks use logger.error() instead of logger.exception(), losing the traceback (Lines 493, 537, 607, 653, 678, 753, 796). On a Raspberry Pi where remote debugging is common, preserving tracebacks is especially valuable.

Example fix pattern

-            except Exception as e:
-                logger.exception(f"Could not save config for {self.app_id}: {e}")
+            except Exception as e:
+                logger.exception(f"Could not save config for {self.app_id}")

-        except Exception as e:
-            self.logger.error(f"Error saving manifest: {e}")
+        except Exception as e:
+            self.logger.exception(f"Error saving manifest")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/manager.py` at line 83, Remove the redundant
exception interpolation from logger.exception calls (e.g., change
logger.exception(f"Could not save config for {self.app_id}: {e}") to
logger.exception(f"Could not save config for {self.app_id}")) and replace the
logger.error(...) calls inside except blocks with logger.exception(...) so
tracebacks are preserved; search for uses of logger.exception and logger.error
in manager.py (locations handling config/save, load, deploy, and other exception
handlers) and update those call sites accordingly, ensuring you do not pass
exc_info manually when using logger.exception.

web_interface/templates/v3/partials/starlark_apps.html (2)

227-229: Cache-busting with Date.now() forces re-download on every tab visit.

On a Raspberry Pi with limited bandwidth and CPU, ?v=' + Date.now() means the browser will never cache this script — every navigation to the Starlark tab fetches it fresh. Consider using a build-time or server-injected version token instead, so the script is cached between visits but busted on deployment.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/templates/v3/partials/starlark_apps.html` around lines 227 -
229, The current cache-busting uses Date.now() on the created script element
(variable name "script" setting script.src to
'/static/v3/js/starlark_apps.js?v=' + Date.now()), which prevents any caching;
change it to use a build-time or server-injected version token instead (for
example a templated variable like a global APP_VERSION or a server-rendered {{
static_version }}), so script.src becomes '/static/v3/js/starlark_apps.js?v=' +
<server_or_build_version_token> and the browser can cache between visits but
still bust on deploy; optionally keep a Date.now() fallback only when an
explicit dev flag is set.

---

54-54: Modals lack ARIA attributes for screen-reader accessibility.

The modal overlays (upload-star-modal, starlark-config-modal, repository-browser-modal) are missing role="dialog" and aria-modal="true" attributes, and their headings aren't linked via aria-labelledby. This can make the modals non-navigable for assistive technology users.

Example for the upload modal

-<div id="upload-star-modal" class="fixed inset-0 bg-black bg-opacity-50 z-50 flex items-center justify-center" style="display: none;" onclick="if(event.target === this) closeUploadModal();">
-    <div class="bg-white rounded-lg shadow-xl max-w-2xl w-full mx-4 max-h-[90vh] overflow-y-auto" onclick="event.stopPropagation();">
+<div id="upload-star-modal" role="dialog" aria-modal="true" aria-labelledby="upload-modal-title" class="fixed inset-0 bg-black bg-opacity-50 z-50 flex items-center justify-center" style="display: none;" onclick="if(event.target === this) closeUploadModal();">
+    <div class="bg-white rounded-lg shadow-xl max-w-2xl w-full mx-4 max-h-[90vh] overflow-y-auto" onclick="event.stopPropagation();">

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/templates/v3/partials/starlark_apps.html` at line 54, Add
proper ARIA attributes to the modal root elements to make them accessible: for
each modal element with id upload-star-modal, starlark-config-modal, and
repository-browser-modal add role="dialog" and aria-modal="true" and set
aria-labelledby to the id of the modal heading (e.g.,
aria-labelledby="upload-star-heading"). Ensure the corresponding heading
elements inside each modal (h1/h2/h3 or equivalent) have matching id attributes
(e.g., id="upload-star-heading") so assistive tech can associate the dialog with
its label; update any template snippets where those headings are rendered to
include the id.

web_interface/blueprints/api_v3.py (2)

6014-6021: Add type hints to new Starlark endpoint handlers.
These new functions don’t annotate parameters/return types; please add them for consistency (e.g., app_id: str, -> Response).

As per coding guidelines: "Use type hints for function parameters and return values".

Also applies to: 6112-6115, 6302-6305

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/blueprints/api_v3.py` around lines 6014 - 6021, Add missing
type hints to the new Starlark endpoint handlers: annotate function parameters
(e.g., any app_id parameter as app_id: str) and set the return type to Flask's
Response (e.g., -> Response). Update the signatures for get_starlark_status and
the other new handlers in the same area to include these annotations, and add
the necessary import (from flask import Response) if not already present.

---

6092-6093: Use structured logging and avoid broad Exception catches in new endpoints.
Prefer logger.exception("[Starlark] ...") with a consistent prefix and narrow to expected exceptions; also avoid swallowing errors in the Pixlet version probe.

As per coding guidelines: "Use structured logging with context (e.g., [NHL Recent]) for logging messages" and "Catch specific exceptions, not bare except: statements".

Also applies to: 6107-6109, 6163-6165, 6213-6214, 6463-6465, 6590-6592, 6645-6647, 6692-6694, 6772-6773, 6900-6901, 6931-6933, 7020-7022

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/blueprints/api_v3.py` around lines 6092 - 6093, Replace each
bare "except Exception: pass" in web_interface/blueprints/api_v3.py with
targeted exception handling for the Pixlet/version probe and other new
endpoints: catch specific exceptions like FileNotFoundError,
subprocess.CalledProcessError, OSError, ValueError (as appropriate for the
operation) and in each except block call logger.exception with a consistent
prefix (e.g., logger.exception("[Starlark] Pixlet probe failed: %s",
some_context)) so the error is logged with stack trace and context instead of
being swallowed; update all occurrences of the pattern (the shown "except
Exception: pass" and the other listed sites) to follow this structured logging
and specific-exception approach.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugin-repos/starlark-apps/frame_extractor.py`:
- Around line 47-103: The FrameExtractor methods are catching broad Exception
types which hides real errors; update the frame extraction loop in load_webp
(inside the for loop where it currently has "except Exception as e") to catch
(OSError, ValueError) instead and keep the existing EOFError handling, and
update the outer handler (the final "except Exception as e" in load_webp) to
catch (UnidentifiedImageError, OSError, ValueError) and call
logger.exception(...) so stack traces are preserved; also prefix all log
messages in load_webp, scale_frames, and center_frames with "[FrameExtractor]"
for structured context and apply the same narrower exception changes to
corresponding inner try/except blocks in scale_frames and center_frames.

In `@plugin-repos/starlark-apps/manager.py`:
- Around line 296-315: The elif not needs_scaling branch in the magnify loop is
unreachable because perfect_fit and not needs_scaling are equivalent; remove the
unreachable branch and simplify quality_score logic in the for magnify loop
(references: magnify, render_width, render_height, perfect_fit, needs_scaling,
quality_score) so that if perfect_fit set quality_score = 100, otherwise compute
the percentage-based quality_score as currently done.
- Around line 656-678: The _display_frame method can raise IndexError when
current_app.current_frame_index is out of range after frames are reloaded;
before accessing self.current_app.frames[self.current_app.current_frame_index]
clamp or normalize current_app.current_frame_index (e.g., if not
self.current_app.frames: return; if current_frame_index >= len(frames) or < 0
set it to current_frame_index % len(frames) or 0) so the index is always valid,
and replace the broad except Exception with targeted handling that logs the
exception details (include the index and frames length) to aid debugging while
still catching unexpected errors.
- Around line 696-703: The code currently calls app_dir.mkdir(...) before
verifying path safety, which can leave an out-of-tree directory if verification
fails; change the order in the block around _sanitize_app_id and
_verify_path_safety so you compute app_dir = (self.apps_dir /
safe_app_id).resolve(), then call self._verify_path_safety(app_dir,
self.apps_dir) and only after a successful verification call
app_dir.mkdir(parents=True, exist_ok=True); keep using the existing
_sanitize_app_id, _verify_path_safety, apps_dir, and app_dir symbols so the
logic is identical except for moving the mkdir after the safety check.
- Around line 412-443: The _verify_path_safety function currently catches its
own ValueError making the primary is_relative_to() check ineffective; change the
exception handling so only AttributeError (when is_relative_to doesn't exist) is
caught. Specifically, keep the initial resolved_path = path.resolve() and
resolved_base = base_dir.resolve(), perform the is_relative_to check and let a
ValueError propagate if it fails; replace the current except (ValueError,
AttributeError) with except AttributeError as e and in that block fall back to
using resolved_path.relative_to(resolved_base) (or recompute resolves if you
prefer) and raise ValueError only if relative_to raises, referencing the same
resolved_path/resolved_base symbols.

In `@plugin-repos/starlark-apps/manifest.json`:
- Around line 1-25: Add a new requirements.txt file to the plugin folder listing
the Python dependencies declared in the manifest (Pillow>=10.0.0, PyYAML>=6.0,
requests>=2.31.0) so the plugin installer can install them; locate the
manifest.json (id "starlark-apps") and the plugin class StarlarkAppsPlugin in
manager.py to confirm the exact dependency names/versions and ensure
requirements.txt matches those entries.

In `@plugin-repos/starlark-apps/pixlet_renderer.py`:
- Around line 250-262: The debug log currently prints the full Pixlet command
including actual config values which can leak secrets; instead build a redacted
representation before logging (e.g., iterate over config used to extend cmd and
replace each "{key}={value}" token with "{key}=REDACTED" or "***REDACTED***"),
then call logger.debug with that redacted command string (referencing the same
cmd and config variables and the existing logger.debug call in
pixlet_renderer.py) so keys remain visible but values are not.

In `@plugin-repos/starlark-apps/tronbyte_repository.py`:
- Around line 54-114: The handlers in _make_request and _fetch_raw_file are too
broad; replace the final broad "except Exception" blocks with specific exception
handling: in _make_request catch requests.Timeout, requests.RequestException,
and requests.exceptions.JSONDecodeError (around response.json()) and log with
logger.exception("[TronByte] ...") to include stack traces; in _fetch_raw_file
catch requests.Timeout and requests.RequestException and use
logger.exception("[TronByte] ...") for errors fetching raw content. Also ensure
messages include the module prefix "[TronByte]" and reference the attributes
used (self.session, self.raw_url, self.REPO_OWNER, self.REPO_NAME,
self.DEFAULT_BRANCH) so the logs clearly identify the repository and branch.

In `@scripts/download_pixlet.sh`:
- Around line 62-110: Add SHA256 integrity verification before extracting the
downloaded archive: after downloading to temp_file (variable temp_file) and
before the tar -xzf step, fetch the release checksum (e.g. from GitHub releases
or a .sha256 file tied to REPO and PIXLET_VERSION), compute the local digest
(using sha256sum or shasum -a 256) for temp_file and compare it to the expected
value; if they differ, print a clear error, rm -rf "$temp_dir" and return
non‑zero. Keep the existing error paths (curl/tar) and use the same
temp_dir/temp_file/archive_name variables, and add an optional fallback to
verify a signature if a .sig/GPG key is provided.
- Around line 115-122: The arithmetic increment using post-increment
((success_count++)) can produce an exit status of 1 when the previous value is 0
and will trigger set -e to terminate the script; replace the post-increment with
a safe increment such as ((success_count+=1)) or
success_count=$((success_count+1)) inside the for loop that calls
download_binary so incrementing success_count won't cause the script to exit
unexpectedly.

In `@scripts/utils/start_web_conditionally.py`:
- Around line 18-20: The Ruff S603 false positive flags the subprocess.run call
even though requirements_file is constructed from __file__ and not user input;
suppress it by adding an inline Ruff noqa for S603 on the subprocess.run
invocation (the line calling subprocess.run([...], capture_output=True,
text=True)) and include a brief comment noting that requirements_file is derived
from __file__ to justify the suppression.

In `@web_interface/blueprints/api_v3.py`:
- Around line 6337-6394: The upload handler currently only checks file.filename
extension and then writes the whole upload to a temp file
(tempfile.NamedTemporaryFile), allowing unbounded large uploads; add an explicit
file size check before saving (e.g., define MAX_STAR_UPLOAD_BYTES and reject if
request.content_length or the uploaded file.stream size exceeds it) and return a
413/400 with an error message; if content_length is unavailable, stream-read up
to MAX+1 bytes from the file stream to detect oversize and reject without
writing the whole file, only write to tempfile.NamedTemporaryFile when size is
within limit (keep using file.filename, temp_path, and existing validation
flow).
- Around line 6232-6262: The sanitizer currently replaces hyphens which breaks
explicit app IDs; change the logic so explicit app_id values are not silently
rewritten: detect whether app_id came from fallback_source (use the existing
fallback_source check) and if app_id was explicitly provided, allow '-' in the
sanitized result or return an error instead of mutating; concretely, update the
regex used to build sanitized (replace r'[^a-z0-9_]' with r'[^a-z0-9_-]') and
adjust the trimming (strip both '_' and '-' with sanitized.strip('_-')), but
only apply this relaxed rule when app_id was explicit; if you prefer strictness,
when app_id is explicit and contains disallowed chars (like path separators or
other invalid chars) return an error rather than silently changing it; keep the
existing path-traversal checks and digit-prefix handling (the
app_id/fallback_source, normalized, sanitized, and hash_slug symbols help locate
where to change).

In `@web_interface/static/v3/js/starlark_apps.js`:
- Around line 895-896: The current display-name fallback in
renderRepositoryAppCard uses app.id.replace('_', ' ').replace('-', ' '), which
only replaces the first underscore and hyphen; update the fallback to perform
global replacement on app.id (e.g., use a regex with the global flag such as
/[_-]/g or separate /_/g and /-/g) so all underscores and hyphens are replaced
with spaces when computing the name variable.
- Around line 651-699: The form handler handleConfigSubmit is missing unchecked
checkbox values because FormData.entries() excludes them; update the code after
building config from formData to iterate the form's checkbox inputs (e.g.,
event.target.querySelectorAll('input[type="checkbox"]')) and ensure each
checkbox name is set in config to the boolean input.checked if a key is missing,
so unchecked boxes produce false and existing keys are not overwritten
incorrectly; keep using currentConfigAppId, config, submitBtn, and the same
fetch payload.
- Around line 573-632: The code in function generateConfigFields uses a
falsy-coalescing chain for the config value (const value = config[key] ||
field.default || ''), which drops explicit false/0/""; change that to use
nullish coalescing (config[key] ?? field.default ?? '') so deliberate false/0/""
are preserved; update the local variable named value accordingly so the checkbox
rendering (input checked logic around id="config-${safeKey}") and option
selection logic use the corrected value.

In `@web_interface/templates/v3/base.html`:
- Around line 3551-3575: The fetch path is built with starlarkAppId directly
which can contain spaces or reserved characters; update the code that sets
starlarkAppId and the fetch call (referencing pluginId, starlarkAppId and the
fetch(`/api/v3/starlark/apps/${starlarkAppId}`) call) to use
encodeURIComponent(starlarkAppId) when constructing the URL (e.g., encode the id
into the path) so requests won't break on filenames with spaces or special
characters; keep other logic (setting componentContext.* and logging) unchanged.

In `@web_interface/templates/v3/partials/starlark_apps.html`:
- Around line 212-245: The partial reload resets window.starlarkAppsInitialized
causing initStarlarkApps -> setupEventListeners to re-bind handlers on
persistent modals moved by moveModalsToBody, resulting in duplicate listeners on
`#upload-star-form`, `#starlark-config-form` and controls in
`#repository-browser-modal`; fix by adding module-scoped WeakSet(s) (like the
pattern used for app card grids) to track elements that already had listeners
and update setupEventListeners to check those WeakSet(s) before attaching
handlers, so initStarlarkApps/starlarkAppsInitialized can still be reset but
listeners are only attached once.

---

Outside diff comments:
In `@scripts/utils/start_web_conditionally.py`:
- Around line 32-38: The current use of a fixed filename for temp_reqs is
vulnerable to concurrent-write races; replace the static path creation with a
secure temporary file using tempfile.mkstemp(), write filtered_lines to the fd
(or open the returned path), close the descriptor, pass that temporary path to
subprocess.check_call(sys.executable, '-m', 'pip', 'install',
'--break-system-packages', '-r', temp_path), and ensure the temp file is
unlinked in a finally block to guarantee cleanup even on errors; update
references to PROJECT_DIR/temp_reqs to use the mkstemp-generated temp_path.

---

Nitpick comments:
In `@plugin-repos/starlark-apps/manager.py`:
- Around line 640-648: The code currently leaves max_frames as None which allows
unbounded PIL Image frame retention; change the logic where max_frames is
obtained (self.config.get("max_frames")) to apply a sensible default (e.g.,
DEFAULT_MAX_FRAMES = 50) when the value is missing or explicitly None, then pass
that bounded max_frames into self.extractor.optimize_frames(frames,
max_frames=max_frames) so app.frames only retains at most the default/
configured number of frames; update references to max_frames and consider adding
a module-level DEFAULT_MAX_FRAMES constant and using it in the code that sets
max_frames to ensure Raspberry Pi memory is constrained.
- Line 83: Remove the redundant exception interpolation from logger.exception
calls (e.g., change logger.exception(f"Could not save config for {self.app_id}:
{e}") to logger.exception(f"Could not save config for {self.app_id}")) and
replace the logger.error(...) calls inside except blocks with
logger.exception(...) so tracebacks are preserved; search for uses of
logger.exception and logger.error in manager.py (locations handling config/save,
load, deploy, and other exception handlers) and update those call sites
accordingly, ensuring you do not pass exc_info manually when using
logger.exception.

In `@plugin-repos/starlark-apps/pixlet_renderer.py`:
- Around line 17-46: The PixletRenderer logs use the module-level logger without
a consistent context prefix; create a LoggerAdapter (or small wrapper) that
injects a consistent prefix like "[Starlark Pixlet]" and use it inside
PixletRenderer (e.g., assign self.logger = logging.LoggerAdapter(logger, {...})
during initialization) and replace the module logger calls in
PixletRenderer.__init__ (logger.info / logger.warning) with self.logger.info /
self.logger.warning so all messages include the required structured context.

In `@scripts/utils/start_web_conditionally.py`:
- Line 12: The function install_dependencies() is missing a return type hint;
update its signature to include the appropriate type annotation (e.g., -> None
or the actual return type) so it complies with the project's typing guidelines,
and update any callers or docstring if necessary to reflect the annotated return
type; locate the function install_dependencies in
scripts/utils/start_web_conditionally.py and add the correct return type hint to
its definition.

In `@web_interface/blueprints/api_v3.py`:
- Around line 6014-6021: Add missing type hints to the new Starlark endpoint
handlers: annotate function parameters (e.g., any app_id parameter as app_id:
str) and set the return type to Flask's Response (e.g., -> Response). Update the
signatures for get_starlark_status and the other new handlers in the same area
to include these annotations, and add the necessary import (from flask import
Response) if not already present.
- Around line 6092-6093: Replace each bare "except Exception: pass" in
web_interface/blueprints/api_v3.py with targeted exception handling for the
Pixlet/version probe and other new endpoints: catch specific exceptions like
FileNotFoundError, subprocess.CalledProcessError, OSError, ValueError (as
appropriate for the operation) and in each except block call logger.exception
with a consistent prefix (e.g., logger.exception("[Starlark] Pixlet probe
failed: %s", some_context)) so the error is logged with stack trace and context
instead of being swallowed; update all occurrences of the pattern (the shown
"except Exception: pass" and the other listed sites) to follow this structured
logging and specific-exception approach.

In `@web_interface/blueprints/pages_v3.py`:
- Around line 315-316: In the except block that returns the error response,
avoid calling str(e) in the f-string and use the f-string conversion flag
instead (change f"Error: {str(e)}" to f"Error: {e!s}") to satisfy RUF010; leave
the broad `except Exception` as-is for now (BLE001 requires a file-wide
refactor) and ensure you only update the return expression in that handler.

In `@web_interface/static/v3/js/starlark_apps.js`:
- Around line 163-166: The fetch calls (e.g., in loadStarlarkStatus) call
response.json() without checking response.ok, causing misleading parse errors on
non-2xx responses; add a small helper like fetchJson(url, options) that awaits
fetch, throws a clear Error containing response.status and a truncated
response.text() when !response.ok, and returns response.json() otherwise, then
replace direct fetch+response.json() usages (notably loadStarlarkStatus and the
other fetch sites listed) to call fetchJson so failures surface a clear HTTP
status and body snippet instead of a JSON parse error.

In `@web_interface/templates/v3/partials/starlark_apps.html`:
- Around line 227-229: The current cache-busting uses Date.now() on the created
script element (variable name "script" setting script.src to
'/static/v3/js/starlark_apps.js?v=' + Date.now()), which prevents any caching;
change it to use a build-time or server-injected version token instead (for
example a templated variable like a global APP_VERSION or a server-rendered {{
static_version }}), so script.src becomes '/static/v3/js/starlark_apps.js?v=' +
<server_or_build_version_token> and the browser can cache between visits but
still bust on deploy; optionally keep a Date.now() fallback only when an
explicit dev flag is set.
- Line 54: Add proper ARIA attributes to the modal root elements to make them
accessible: for each modal element with id upload-star-modal,
starlark-config-modal, and repository-browser-modal add role="dialog" and
aria-modal="true" and set aria-labelledby to the id of the modal heading (e.g.,
aria-labelledby="upload-star-heading"). Ensure the corresponding heading
elements inside each modal (h1/h2/h3 or equivalent) have matching id attributes
(e.g., id="upload-star-heading") so assistive tech can associate the dialog with
its label; update any template snippets where those headings are rendered to
include the id.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2763d6 and 77eb22a.

📒 Files selected for processing (15)

• .gitignore
• plugin-repos/starlark-apps/__init__.py
• plugin-repos/starlark-apps/config_schema.json
• plugin-repos/starlark-apps/frame_extractor.py
• plugin-repos/starlark-apps/manager.py
• plugin-repos/starlark-apps/manifest.json
• plugin-repos/starlark-apps/pixlet_renderer.py
• plugin-repos/starlark-apps/tronbyte_repository.py
• scripts/download_pixlet.sh
• scripts/utils/start_web_conditionally.py
• web_interface/blueprints/api_v3.py
• web_interface/blueprints/pages_v3.py
• web_interface/static/v3/js/starlark_apps.js
• web_interface/templates/v3/base.html
• web_interface/templates/v3/partials/starlark_apps.html

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the actual file
find . -name "frame_extractor.py" -type f

Repository: ChuckBuilds/LEDMatrix

Length of output: 112

---

🌐 Web query:

PIL Pillow Image.open() exceptions OSError UnidentifiedImageError

💡 Result:

In Pillow, Image.open() mainly fails in two ways:

• PIL.UnidentifiedImageError (a subclass of OSError) — raised when Pillow can’t open and identify the file as a supported image. This is the usual exception behind “cannot identify image file …”. [1][2]
• Other OSError cases — since UnidentifiedImageError inherits from OSError, catching OSError will also catch it (plus other I/O-related errors that may happen while reading/decoding). [1]

Per Pillow’s Image.open() docs/source, you may also see: FileNotFoundError, ValueError (e.g., mode not "r" / StringIO used), and TypeError (bad formats argument). [2]

Practical pattern (also forces decoding to surface errors that occur after open):

from PIL import Image, UnidentifiedImageError

try:
    with Image.open(path) as im:
        im.load()  # force decode; catches truncated/corrupt data issues here too
        # use im...
except FileNotFoundError:
    ...
except UnidentifiedImageError:
    ...  # not an image / unsupported / cannot identify
except OSError:
    ...  # other I/O/decoding errors (also catches UnidentifiedImageError if you prefer)

Sources: Pillow docs for UnidentifiedImageError and Image.open() exception list/implementation. [1][2]

---

🌐 Web query:

PIL Pillow Image.convert() Image.seek() ValueError EOFError exceptions

💡 Result:

Image.convert() exceptions (Pillow)

Common ValueError cases come from Pillow failing to decode/convert the underlying pixel data, e.g.:

• Incomplete/insufficient data → ValueError: not enough image data (decoder didn’t get enough bytes) or other decode-related ValueErrors. [1]
• Unsupported mode/matrix conversion → ValueError: illegal conversion (requested conversion path isn’t valid for the current mode/options). [1]

Typical fixes/mitigations

• Force decode early so you fail at a predictable point: im.load() before convert(). (Conversion triggers decoding anyway; forcing load() just makes the failure location clearer.) [1]
• If the file is truncated/corrupt, Pillow may raise OSError: image file is truncated (...) during load/decode. You can choose to allow best-effort loading via:

  from PIL import ImageFile
  ImageFile.LOAD_TRUNCATED_IMAGES = True

  (Only if you accept possibly degraded/partial output.) [2]

---

Image.seek() exceptions (Pillow)

• Image.seek(frame) returns None; it mutates the image object to point at that frame. [3]
• If you seek past the last frame, Pillow raises EOFError (“no more images in file”). [1]
• If the image is not multi-frame, the base implementation raises EOFError for any frame != 0. [1]

Typical safe pattern

from PIL import Image

im = Image.open("in.gif")

for i in range(getattr(im, "n_frames", 1)):
    try:
        im.seek(i)
    except EOFError:
        break
    frame = im.convert("RGBA")  # or copy(): im.copy()

Use im.n_frames / im.is_animated when available to avoid probing beyond the end; still be ready to catch EOFError. [1]

---

🏁 Script executed:

# Now let's examine the actual frame_extractor.py file, particularly around lines 47-103
head -n 110 ./plugin-repos/starlark-apps/frame_extractor.py | tail -n +40

Repository: ChuckBuilds/LEDMatrix

Length of output: 2895

---

🏁 Script executed:

# Let's also see the imports at the top of the file
head -n 20 ./plugin-repos/starlark-apps/frame_extractor.py

Repository: ChuckBuilds/LEDMatrix

Length of output: 479

---

🏁 Script executed:

# Check if there are other exception handling patterns in the file (e.g., GIF conversion)
grep -n "except" ./plugin-repos/starlark-apps/frame_extractor.py

Repository: ChuckBuilds/LEDMatrix

Length of output: 335

---

🏁 Script executed:

# Let's check if there are any other WebP/GIF related exception handlers we should be aware of
grep -B 5 -A 5 "def load_gif" ./plugin-repos/starlark-apps/frame_extractor.py

Repository: ChuckBuilds/LEDMatrix

Length of output: 47

---

🏁 Script executed:

# Check the load_gif method for similar exception patterns
sed -n '105,200p' ./plugin-repos/starlark-apps/frame_extractor.py

Repository: ChuckBuilds/LEDMatrix

Length of output: 3608

---

Replace broad Exception catches with Pillow-specific errors for better error visibility.

The code catches broad Exception at lines 84 and 100, hiding underlying issues. Per Pillow's exception hierarchy:

• Image.open() raises FileNotFoundError, UnidentifiedImageError (subclass of OSError), and OSError
• Image.convert() and Image.seek() raise OSError and ValueError
• Image.seek() raises EOFError (already handled correctly at line 81)

Replace line 84's bare Exception with (OSError, ValueError) to catch frame conversion/seek errors specifically. Replace line 100's bare Exception with (UnidentifiedImageError, OSError, ValueError) and use logger.exception() to preserve stack traces for debugging on Raspberry Pi. Additionally, add structured logging context prefix like [FrameExtractor] to all log messages to improve remote debugging clarity.

Example fix for load_webp outer exception handler

-from PIL import Image
+from PIL import Image, UnidentifiedImageError

...

-        except Exception as e:
-            error = f"Error loading WebP: {e}"
-            logger.error(error)
-            return False, None, error
+        except (UnidentifiedImageError, OSError, ValueError) as e:
+            error = f"[FrameExtractor] Error loading WebP: {e}"
+            logger.exception(error)
+            return False, None, error

Similar pattern applies to the frame loop (line 84) and other exception handlers in scale_frames, center_frames methods.

🧰 Tools

🪛 Ruff (0.15.2)

[warning] 84-84: Do not catch blind exception: Exception

(BLE001)

---

[warning] 98-98: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

---

[warning] 100-100: Do not catch blind exception: Exception

(BLE001)

---

[warning] 102-102: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/frame_extractor.py` around lines 47 - 103, The
FrameExtractor methods are catching broad Exception types which hides real
errors; update the frame extraction loop in load_webp (inside the for loop where
it currently has "except Exception as e") to catch (OSError, ValueError) instead
and keep the existing EOFError handling, and update the outer handler (the final
"except Exception as e" in load_webp) to catch (UnidentifiedImageError, OSError,
ValueError) and call logger.exception(...) so stack traces are preserved; also
prefix all log messages in load_webp, scale_frames, and center_frames with
"[FrameExtractor]" for structured context and apply the same narrower exception
changes to corresponding inner try/except blocks in scale_frames and
center_frames.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Dead code branch: elif not needs_scaling is unreachable.

perfect_fit and not needs_scaling are logically equivalent (both check render_width == display_width and render_height == display_height). The elif not needs_scaling branch on Line 309 can never be reached when perfect_fit is False. The quality score of 95 is never assigned.

Proposed simplification

                 if perfect_fit:
                     quality_score = 100
-                elif not needs_scaling:
-                    quality_score = 95
                 else:
                     # Score based on how close to display size

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/manager.py` around lines 296 - 315, The elif not
needs_scaling branch in the magnify loop is unreachable because perfect_fit and
not needs_scaling are equivalent; remove the unreachable branch and simplify
quality_score logic in the for magnify loop (references: magnify, render_width,
render_height, perfect_fit, needs_scaling, quality_score) so that if perfect_fit
set quality_score = 100, otherwise compute the percentage-based quality_score as
currently done.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

_verify_path_safety catches its own raised ValueError, making the primary check dead code.

Line 429 raises ValueError when is_relative_to returns False. The except (ValueError, AttributeError) on Line 432 catches that same exception, then redundantly re-checks with relative_to(). The primary guard on Line 428 effectively never propagates — it always falls through to the fallback.

Restructure to only use the fallback for AttributeError (Python < 3.9), and let the ValueError from the primary check propagate:

Proposed fix

     def _verify_path_safety(self, path: Path, base_dir: Path) -> None:
         try:
             resolved_path = path.resolve()
             resolved_base = base_dir.resolve()
-
-            # Check if path is relative to base directory
-            if not resolved_path.is_relative_to(resolved_base):
-                raise ValueError(
-                    f"Path traversal detected: {resolved_path} is not within {resolved_base}"
-                )
-        except (ValueError, AttributeError) as e:
-            # AttributeError for Python < 3.9 where is_relative_to doesn't exist
-            # Fallback: check if resolved path starts with resolved base
-            resolved_path = path.resolve()
-            resolved_base = base_dir.resolve()
-
-            try:
-                resolved_path.relative_to(resolved_base)
-            except ValueError:
-                raise ValueError(
-                    f"Path traversal detected: {resolved_path} is not within {resolved_base}"
-                ) from e
+            try:
+                # Python >= 3.9
+                if not resolved_path.is_relative_to(resolved_base):
+                    raise ValueError(
+                        f"Path traversal detected: {resolved_path} is not within {resolved_base}"
+                    )
+            except AttributeError:
+                # Python < 3.9 fallback
+                resolved_path.relative_to(resolved_base)  # raises ValueError if not relative
+        except ValueError:
+            raise

🧰 Tools

🪛 Ruff (0.15.2)

[warning] 429-431: Abstract raise to an inner function

(TRY301)

---

[warning] 429-431: Avoid specifying long messages outside the exception class

(TRY003)

---

[warning] 441-443: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/manager.py` around lines 412 - 443, The
_verify_path_safety function currently catches its own ValueError making the
primary is_relative_to() check ineffective; change the exception handling so
only AttributeError (when is_relative_to doesn't exist) is caught. Specifically,
keep the initial resolved_path = path.resolve() and resolved_base =
base_dir.resolve(), perform the is_relative_to check and let a ValueError
propagate if it fails; replace the current except (ValueError, AttributeError)
with except AttributeError as e and in that block fall back to using
resolved_path.relative_to(resolved_base) (or recompute resolves if you prefer)
and raise ValueError only if relative_to raises, referencing the same
resolved_path/resolved_base symbols.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Potential IndexError if current_frame_index drifts out of bounds.

If frames are reloaded with fewer entries (e.g., a re-render produces fewer frames) while current_frame_index retains its old value, Line 663 will raise IndexError. The broad except Exception silently swallows it, causing a missed display cycle with no clear log indicating the root cause.

Proposed fix — clamp the index defensively

     def _display_frame(self) -> None:
         """Display the current frame of the current app."""
         if not self.current_app or not self.current_app.frames:
             return
 
         try:
             current_time = time.time()
+            # Defensive bounds clamp in case frames were reloaded with fewer entries
+            self.current_app.current_frame_index %= len(self.current_app.frames)
             frame, delay_ms = self.current_app.frames[self.current_app.current_frame_index]

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def _display_frame(self) -> None:
	"""Display the current frame of the current app."""
	if not self.current_app or not self.current_app.frames:
	return
	try:
	current_time = time.time()
	frame, delay_ms = self.current_app.frames[self.current_app.current_frame_index]
	# Set frame on display manager
	self.display_manager.image = frame
	self.display_manager.update_display()
	# Check if it's time to advance to next frame
	delay_seconds = delay_ms / 1000.0
	if (current_time - self.current_app.last_frame_time) >= delay_seconds:
	self.current_app.current_frame_index = (
	(self.current_app.current_frame_index + 1) % len(self.current_app.frames)
	)
	self.current_app.last_frame_time = current_time
	except Exception as e:
	self.logger.error(f"Error displaying frame: {e}")
	def _display_frame(self) -> None:
	"""Display the current frame of the current app."""
	if not self.current_app or not self.current_app.frames:
	return
	try:
	current_time = time.time()
	# Defensive bounds clamp in case frames were reloaded with fewer entries
	self.current_app.current_frame_index %= len(self.current_app.frames)
	frame, delay_ms = self.current_app.frames[self.current_app.current_frame_index]
	# Set frame on display manager
	self.display_manager.image = frame
	self.display_manager.update_display()
	# Check if it's time to advance to next frame
	delay_seconds = delay_ms / 1000.0
	if (current_time - self.current_app.last_frame_time) >= delay_seconds:
	self.current_app.current_frame_index = (
	(self.current_app.current_frame_index + 1) % len(self.current_app.frames)
	)
	self.current_app.last_frame_time = current_time
	except Exception as e:
	self.logger.error(f"Error displaying frame: {e}")

🧰 Tools

🪛 Ruff (0.15.2)

[warning] 677-677: Do not catch blind exception: Exception

(BLE001)

---

[warning] 678-678: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/manager.py` around lines 656 - 678, The
_display_frame method can raise IndexError when current_app.current_frame_index
is out of range after frames are reloaded; before accessing
self.current_app.frames[self.current_app.current_frame_index] clamp or normalize
current_app.current_frame_index (e.g., if not self.current_app.frames: return;
if current_frame_index >= len(frames) or < 0 set it to current_frame_index %
len(frames) or 0) so the index is always valid, and replace the broad except
Exception with targeted handling that logs the exception details (include the
index and frames length) to aid debugging while still catching unexpected
errors.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Directory created before path-safety check — traversal directory persists on failure.

app_dir.mkdir(parents=True, exist_ok=True) on Line 700 runs before _verify_path_safety on Line 703. If a crafted app_id somehow survives _sanitize_app_id but still resolves outside apps_dir (e.g., via symlinks), the directory is already created before the check raises. Move the safety check before mkdir.

Proposed fix

             # Create app directory with resolved path
             app_dir = (self.apps_dir / safe_app_id).resolve()
-            app_dir.mkdir(parents=True, exist_ok=True)
 
             # Verify path safety after mkdir
             self._verify_path_safety(app_dir, self.apps_dir)
 
+            app_dir.mkdir(parents=True, exist_ok=True)
+
             # Copy .star file with sanitized app_id

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@plugin-repos/starlark-apps/manager.py` around lines 696 - 703, The code
currently calls app_dir.mkdir(...) before verifying path safety, which can leave
an out-of-tree directory if verification fails; change the order in the block
around _sanitize_app_id and _verify_path_safety so you compute app_dir =
(self.apps_dir / safe_app_id).resolve(), then call
self._verify_path_safety(app_dir, self.apps_dir) and only after a successful
verification call app_dir.mkdir(parents=True, exist_ok=True); keep using the
existing _sanitize_app_id, _verify_path_safety, apps_dir, and app_dir symbols so
the logic is identical except for moving the mkdir after the safety check.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Falsy config values (false, 0, "") are silently dropped by the || chain.

Line 578: const value = config[key] || field.default || '' — the || operator treats false, 0, and "" as falsy, so a deliberately set false or 0 config value falls through to field.default or ''. For boolean fields this means an explicit false is ignored, and the checkbox on Line 601 renders incorrectly.

Use nullish coalescing (??) instead:

Proposed fix

-            const value = config[key] || field.default || '';
+            const value = config[key] ?? field.default ?? '';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/static/v3/js/starlark_apps.js` around lines 573 - 632, The code
in function generateConfigFields uses a falsy-coalescing chain for the config
value (const value = config[key] || field.default || ''), which drops explicit
false/0/""; change that to use nullish coalescing (config[key] ?? field.default
?? '') so deliberate false/0/"" are preserved; update the local variable named
value accordingly so the checkbox rendering (input checked logic around
id="config-${safeKey}") and option selection logic use the corrected value.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Unchecked checkboxes are omitted from FormData, so false values are never sent to the server.

FormData.entries() only includes checked checkboxes. When a user unchecks a boolean config field, the key is entirely absent from the submitted config object (Lines 666-674). The server will either keep the old true value or treat the missing key as unchanged — the user's intent to disable the option is lost.

After the loop, iterate the form's checkbox inputs explicitly to fill in missing keys:

Proposed fix — append after the FormData loop

             for (const [key, value] of formData.entries()) {
                 // Handle checkboxes
                 const input = event.target.elements[key];
                 if (input && input.type === 'checkbox') {
                     config[key] = input.checked;
                 } else {
                     config[key] = value;
                 }
             }
+
+            // Ensure unchecked checkboxes are explicitly set to false
+            const checkboxes = event.target.querySelectorAll('input[type="checkbox"]');
+            checkboxes.forEach(cb => {
+                if (cb.name && !(cb.name in config)) {
+                    config[cb.name] = false;
+                }
+            });

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	async function handleConfigSubmit(event) {
	event.preventDefault();
	if (!currentConfigAppId) return;
	const submitBtn = document.getElementById('save-starlark-config-btn');
	const originalText = submitBtn.innerHTML;
	try {
	submitBtn.disabled = true;
	submitBtn.innerHTML = '<i class="fas fa-spinner fa-spin mr-2"></i>Saving...';
	const formData = new FormData(event.target);
	const config = {};
	for (const [key, value] of formData.entries()) {
	// Handle checkboxes
	const input = event.target.elements[key];
	if (input && input.type === 'checkbox') {
	config[key] = input.checked;
	} else {
	config[key] = value;
	}
	}
	const response = await fetch(`/api/v3/starlark/apps/${currentConfigAppId}/config`, {
	method: 'PUT',
	headers: { 'Content-Type': 'application/json' },
	body: JSON.stringify(config)
	});
	const data = await response.json();
	if (data.status === 'success') {
	showNotification(data.message, 'success');
	window.closeConfigModal();
	loadStarlarkApps();
	} else {
	showNotification(data.message, 'error');
	}
	} catch (error) {
	console.error('Error saving config:', error);
	showNotification('Failed to save configuration', 'error');
	} finally {
	submitBtn.disabled = false;
	submitBtn.innerHTML = originalText;
	}
	}
	async function handleConfigSubmit(event) {
	event.preventDefault();
	if (!currentConfigAppId) return;
	const submitBtn = document.getElementById('save-starlark-config-btn');
	const originalText = submitBtn.innerHTML;
	try {
	submitBtn.disabled = true;
	submitBtn.innerHTML = '<i class="fas fa-spinner fa-spin mr-2"></i>Saving...';
	const formData = new FormData(event.target);
	const config = {};
	for (const [key, value] of formData.entries()) {
	// Handle checkboxes
	const input = event.target.elements[key];
	if (input && input.type === 'checkbox') {
	config[key] = input.checked;
	} else {
	config[key] = value;
	}
	}
	// Ensure unchecked checkboxes are explicitly set to false
	const checkboxes = event.target.querySelectorAll('input[type="checkbox"]');
	checkboxes.forEach(cb => {
	if (cb.name && !(cb.name in config)) {
	config[cb.name] = false;
	}
	});
	const response = await fetch(`/api/v3/starlark/apps/${currentConfigAppId}/config`, {
	method: 'PUT',
	headers: { 'Content-Type': 'application/json' },
	body: JSON.stringify(config)
	});
	const data = await response.json();
	if (data.status === 'success') {
	showNotification(data.message, 'success');
	window.closeConfigModal();
	loadStarlarkApps();
	} else {
	showNotification(data.message, 'error');
	}
	} catch (error) {
	console.error('Error saving config:', error);
	showNotification('Failed to save configuration', 'error');
	} finally {
	submitBtn.disabled = false;
	submitBtn.innerHTML = originalText;
	}
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/static/v3/js/starlark_apps.js` around lines 651 - 699, The form
handler handleConfigSubmit is missing unchecked checkbox values because
FormData.entries() excludes them; update the code after building config from
formData to iterate the form's checkbox inputs (e.g.,
event.target.querySelectorAll('input[type="checkbox"]')) and ensure each
checkbox name is set in config to the boolean input.checked if a key is missing,
so unchecked boxes produce false and existing keys are not overwritten
incorrectly; keep using currentConfigAppId, config, submitBtn, and the same
fetch payload.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

replace only replaces the first occurrence of _ and -.

Line 896: app.id.replace('_', ' ').replace('-', ' ') uses String.replace with a string argument, which only replaces the first match. An ID like my_cool_app becomes my cool_app. Use a regex with global flag:

Proposed fix

-        const name = app.name || app.id.replace('_', ' ').replace('-', ' ');
+        const name = app.name || app.id.replace(/[_-]/g, ' ');

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	function renderRepositoryAppCard(app) {
	const name = app.name || app.id.replace('_', ' ').replace('-', ' ');
	function renderRepositoryAppCard(app) {
	const name = app.name || app.id.replace(/[_-]/g, ' ');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/static/v3/js/starlark_apps.js` around lines 895 - 896, The
current display-name fallback in renderRepositoryAppCard uses
app.id.replace('_', ' ').replace('-', ' '), which only replaces the first
underscore and hyphen; update the fallback to perform global replacement on
app.id (e.g., use a regex with the global flag such as /[_-]/g or separate /_/g
and /-/g) so all underscores and hyphens are replaced with spaces when computing
the name variable.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

URL‑encode the Starlark app id before building the fetch path.

App ids derived from filenames can include spaces or URL‑reserved characters, which can break the request path or produce subtle 404s. Encoding keeps the endpoint stable.

🔧 Proposed fix

-                        const starlarkAppId = pluginId.replace('starlark:', '');
+                        const starlarkAppId = pluginId.replace('starlark:', '');
+                        const starlarkAppIdEncoded = encodeURIComponent(starlarkAppId);
                         console.log('Loading starlark app config:', starlarkAppId);
 
-                        const appResponse = await fetch(`/api/v3/starlark/apps/${starlarkAppId}`).then(r => r.json());
+                        const appResponse = await fetch(`/api/v3/starlark/apps/${starlarkAppIdEncoded}`).then(r => r.json());

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Handle starlark apps differently
	if (pluginId.startsWith('starlark:')) {
	const starlarkAppId = pluginId.replace('starlark:', '');
	console.log('Loading starlark app config:', starlarkAppId);
	const appResponse = await fetch(`/api/v3/starlark/apps/${starlarkAppId}`).then(r => r.json());
	if (appResponse.status === 'success' && appResponse.app) {
	componentContext.config = appResponse.app.config || {};
	componentContext.config.enabled = appResponse.app.enabled;
	componentContext.schema = appResponse.app.schema || {};
	componentContext.webUiActions = [];
	componentContext.isStarlarkApp = true;
	componentContext.starlarkAppId = starlarkAppId;
	componentContext.starlarkAppInfo = appResponse.app;
	} else {
	console.error('Failed to load starlark app:', appResponse.message);
	componentContext.config = { enabled: true };
	componentContext.schema = {};
	componentContext.webUiActions = [];
	}
	componentContext.loading = false;
	return;
	}
	// Handle starlark apps differently
	if (pluginId.startsWith('starlark:')) {
	const starlarkAppId = pluginId.replace('starlark:', '');
	const starlarkAppIdEncoded = encodeURIComponent(starlarkAppId);
	console.log('Loading starlark app config:', starlarkAppId);
	const appResponse = await fetch(`/api/v3/starlark/apps/${starlarkAppIdEncoded}`).then(r => r.json());
	if (appResponse.status === 'success' && appResponse.app) {
	componentContext.config = appResponse.app.config || {};
	componentContext.config.enabled = appResponse.app.enabled;
	componentContext.schema = appResponse.app.schema || {};
	componentContext.webUiActions = [];
	componentContext.isStarlarkApp = true;
	componentContext.starlarkAppId = starlarkAppId;
	componentContext.starlarkAppInfo = appResponse.app;
	} else {
	console.error('Failed to load starlark app:', appResponse.message);
	componentContext.config = { enabled: true };
	componentContext.schema = {};
	componentContext.webUiActions = [];
	}
	componentContext.loading = false;
	return;
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/templates/v3/base.html` around lines 3551 - 3575, The fetch
path is built with starlarkAppId directly which can contain spaces or reserved
characters; update the code that sets starlarkAppId and the fetch call
(referencing pluginId, starlarkAppId and the
fetch(`/api/v3/starlark/apps/${starlarkAppId}`) call) to use
encodeURIComponent(starlarkAppId) when constructing the URL (e.g., encode the id
into the path) so requests won't break on filenames with spaces or special
characters; keep other logic (setting componentContext.* and logging) unchanged.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate the starlark_apps.js file
find . -name "starlark_apps.js" -type f

Repository: ChuckBuilds/LEDMatrix

Length of output: 110

---

🏁 Script executed:

# Also search for references to these functions to understand the codebase structure
rg -l "moveModalsToBody|setupEventListeners|starlarkAppsInitialized" --type=js

Repository: ChuckBuilds/LEDMatrix

Length of output: 108

---

🏁 Script executed:

# Read the starlark_apps.js file to examine the event listener implementation
cat -n web_interface/static/v3/js/starlark_apps.js

Repository: ChuckBuilds/LEDMatrix

Length of output: 48929

---

Duplicate event listeners attached to persistent modal elements on each tab switch.

When the partial reloads via HTMX, the HTML script resets starlarkAppsInitialized = false (line 218), causing initStarlarkApps() to call setupEventListeners() again. Since modals are moved to document.body by moveModalsToBody() and persist across swaps, the form elements (#upload-star-form, #starlark-config-form) and controls inside #repository-browser-modal receive duplicate event listeners on each tab switch. Submitting the upload or config form after a tab switch will trigger the handler multiple times.

Adopt the WeakSet guard pattern already used for app card grids (lines 347-352): create module-scoped WeakSets to track which forms/elements have had listeners attached, and skip re-attachment if already registered.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@web_interface/templates/v3/partials/starlark_apps.html` around lines 212 -
245, The partial reload resets window.starlarkAppsInitialized causing
initStarlarkApps -> setupEventListeners to re-bind handlers on persistent modals
moved by moveModalsToBody, resulting in duplicate listeners on
`#upload-star-form`, `#starlark-config-form` and controls in
`#repository-browser-modal`; fix by adding module-scoped WeakSet(s) (like the
pattern used for app card grids) to track elements that already had listeners
and update setupEventListeners to check those WeakSet(s) before attaching
handlers, so initStarlarkApps/starlarkAppsInitialized can still be reset but
listeners are only attached once.