Fix/duplicate display settings #173
Conversation
…in install Remove compatible_versions from required fields in install_from_url method to match install_plugin behavior. This allows installing plugins from URLs without manifest version requirements, consistent with store plugin installation.
- Add onchange handlers to key and value inputs for existing patternProperties fields - Fixes bug where editing existing custom RSS feeds didn't save changes - Ensures hidden JSON input field is updated when users edit feed entries - Affects all plugins using patternProperties (custom_feeds, feed_logo_map, etc.)
- Add support for rendering arrays of objects in web UI (for custom_feeds) - Implement add/remove/update functions for array-of-objects widgets - Support file-upload widgets within array items - Update form data handling to support array JSON data fields
Update version parameter to force browser to load new JavaScript with array-of-objects widget support.
Move the array-of-objects widget detection to the top of the array handler so it's checked before file-upload and checkbox-group widgets. This ensures custom_feeds is properly detected as an array of objects.
Add detection and rendering for array-of-objects in the Jinja2 template (plugin_config.html). This enables the custom_feeds widget to display properly with name, URL, enabled checkbox, and logo upload fields. The widget is detected by checking if prop.items.type == 'object' && prop.items.properties, and is rendered before the file-upload widget check.
Explicitly use window.addArrayObjectItem, window.removeArrayObjectItem, etc. in the template to ensure the functions are accessible from inline event handlers. Also add safety checks to prevent errors if functions aren't loaded yet.
Prevent display settings from being saved at both nested (display.hardware/runtime) and root level. The save_main_config function was processing display fields twice - once correctly in the nested structure, and again in the catch-all section creating root-level duplicates. Added display_fields to the skip list in the catch-all section to prevent root-level duplicates. All code expects the nested format, so this ensures consistency.
📝 WalkthroughWalkthroughManifest validation now requires Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserJS as Browser (plugins_manager.js)
participant API as Server API
participant Storage as File Storage
participant DB as Configuration DB
User->>BrowserJS: Add array item, attach file
BrowserJS->>API: POST /upload (file blob)
API->>Storage: Store file, return file meta
Storage-->>API: file meta
API-->>BrowserJS: upload success + file meta
BrowserJS->>BrowserJS: embed file meta into item's JSON, base64-encode into hidden `_data`
User->>BrowserJS: Click Save
BrowserJS->>API: POST /save plugin config (includes `_data`)
API->>DB: persist plugin configuration
DB-->>API: ack
API-->>BrowserJS: save success
BrowserJS-->>User: show success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @src/plugin_system/store_manager.py:
- Line 977: The change removed 'version' and 'display_modes' from
required_fields in the manifest validation, but other code (e.g.,
state_reconciliation.py's manifest usage and multiple tests that assert
manifest['display_modes'][0]) still expects them; restore 'version' and
'display_modes' to required_fields in the declaration (the variable named
required_fields) so new plugin manifests always include them, or if you prefer
to allow omission, update all call sites to use safe accessors (e.g.,
manifest.get('version') with a sensible default and
manifest.get('display_modes', []) before indexing) and adjust tests to assert
the safe defaults instead — pick one approach and apply it consistently across
the manifest validation, the state reconciliation logic (the code that reads
manifest.get('version')/manifest['display_modes']), and the tests.
In @web_interface/static/v3/plugins_manager.js:
- Around line 2468-2573: renderArrayObjectItem currently renders non-editable
properties (like the logo object) but updateArrayObjectData rebuilds items only
from visible inputs, causing those properties to be lost; fix by changing
updateArrayObjectData to initialize each rebuilt item from the existing JSON
value for that item (merge/overlay input-derived fields onto the existing item
object) instead of creating a new object from inputs only, and ensure file
handlers handleArrayObjectFileUpload and removeArrayObjectFile update the
underlying item's data (e.g., set/remove data-file-data or directly mutate the
stored item) so file metadata persists; locate and modify the
updateArrayObjectData function and the TODOs in
handleArrayObjectFileUpload/removeArrayObjectFile to perform the merge and
persist file metadata rather than relying solely on visible inputs.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/plugin_system/store_manager.py (1)
985-993: Fix manifest validation to match schema requirements.The manifest schema requires
compatible_versions(it's in the schema's "required" array withminItems: 1), but the code doesn't enforce this during installation. Lines 978–983 only validate['id', 'name', 'class_name']as required, whilecompatible_versionsvalidation on lines 206–210 is conditional—it only checks if the field exists, not that it must exist. This creates a schema/implementation mismatch where plugins can install without a required field, violating the manifest specification.Either add
compatible_versionsto the required fields check (lines 978–983) or update the schema to make it optional if intentionally allowing plugins without it.web_interface/static/v3/plugins_manager.js (1)
2876-3068: Block‑scope bug and array‑type handling can throw at runtimeInside the
prop.type === 'array'branch:
xWidgetValue/xWidgetValue2are declared withconstinside theelseblock (Line ~2907 onward), but are then used in the outer scope (if (xWidgetValue === 'file-upload' || xWidgetValue2 === 'file-upload')etc.). This will cause aReferenceError: xWidgetValue is not definedas soon as any array field is processed, breaking form generation for all arrays (not just arrays‑of‑objects).As currently written, the “array‑of‑objects” path and the file‑upload / checkbox‑group / plain‑array paths are interleaved, making the control flow fragile.
Refactor to keep all non–array‑of‑objects handling (including
x-widgetdetection) inside theelseblock so theconstbindings are in scope, e.g.:Suggested restructuring of the array branch
} else if (prop.type === 'array') { // Check array-of-objects FIRST if (prop.items && prop.items.type === 'object' && prop.items.properties) { // existing array-of-objects rendering ... - } else { - // Check if this is a file upload widget - try multiple ways to access x-widget - const hasXWidget = prop.hasOwnProperty('x-widget'); - const xWidgetValue = prop['x-widget']; - const xWidgetValue2 = prop['x-widget'] || prop['x-widget'] || prop.xWidget; - - console.log(`[DEBUG] Array field ${fullKey}:`, { ... }); - } - - // Check for file-upload widget - be more defensive - if (xWidgetValue === 'file-upload' || xWidgetValue2 === 'file-upload') { - ... - } else if (xWidgetValue === 'checkbox-group' || xWidgetValue2 === 'checkbox-group') { - ... - } else { - // Regular array input - ... - } + } else { + const hasXWidget = prop.hasOwnProperty('x-widget'); + const xWidgetValue = prop['x-widget']; + const xWidgetValue2 = prop['x-widget'] || prop['x_widget'] || prop.xWidget; + + console.log(`[DEBUG] Array field ${fullKey}:`, { /* existing debug fields */ }); + + // File-upload widget + if (xWidgetValue === 'file-upload' || xWidgetValue2 === 'file-upload') { + // existing file-upload rendering... + } else if (xWidgetValue === 'checkbox-group' || xWidgetValue2 === 'checkbox-group') { + // existing checkbox-group rendering... + } else { + // Regular array input (existing code) + } + } }Also note the small typo in
xWidgetValue2(prop['x-widget'] || prop['x-widget'] || prop.xWidget)—the second variant should likely beprop['x_widget']. Low‑severity but easy to fix while you’re in this block.
🧹 Nitpick comments (3)
web_interface/templates/v3/partials/plugin_config.html (1)
61-168: Array-of-objects UI wiring looks solid; consider minor JS call consistencyThe new
arraybranch correctly detectsitems.type == 'object', renders per-item editors, and keeps a JSON copy in the hidden{{ full_key }}_datafield for the JS submit handler to serialize back into the real config field. The use oftojson|safeindata-file-dataand the guardedwindow.handleArrayObjectFileUpload/window.removeArrayObjectFile/window.addArrayObjectItemcalls is sound.One small polish: in the text-input branch you call
onchange="updateArrayObjectData('{{ field_id }}')"without thewindow.prefix or a function-existence check, while the boolean branch usesif (typeof window.updateArrayObjectData === 'function') { … }. For robustness when the JS isn’t loaded yet (or during partial reloads), you might align this with the guardedwindow.updateArrayObjectDatapattern, but behavior is otherwise correct.web_interface/static/v3/plugins_manager.js (2)
2225-2234: Hidden_dataJSON handling is good, but other paths still ignore itParsing
_datafields into objects/arrays and short‑circuiting normal processing looks correct and unblocks patternProperties / array‑of‑objects for the main submit handler. However,syncFormToJsonand the older document‑levelsubmitlistener forplugin-config-form(around Line 5460+) still treat all fields as flat scalars and do not understand the new*_datacontract, so switching to JSON view or relying on that fallback can lose or mis‑shape these complex values.Consider either:
- Removing the legacy document listener and routing all submits through
handlePluginConfigSubmit, and- Mirroring the
_dataJSON parsing insyncFormToJsonif JSON view is expected to round‑trip these structures.
2617-2625: Key–value patternProperties syncing looks correct; minor cleanup possibleThe added
data-key-index/data-value-indexattributes andonchange="updateKeyValuePairData(...)"wiring keep the hidden JSON_datafield in sync and should avoid the old “stale pairs” problems. The reindexing logic inremoveKeyValuePairalso correctly updates names, indices, and handlers.
updateKeyValuePairDatano longer uses itsfullKeyparameter (onlyfieldIdand the DOM), so you can safely drop that argument from the signature and all callers in a later cleanup; it’s not urgent.Also applies to: 3321-3439
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
plugins/7-segment-clocksrc/plugin_system/store_manager.pyweb_interface/blueprints/api_v3.pyweb_interface/static/v3/plugins_manager.jsweb_interface/templates/v3/base.htmlweb_interface/templates/v3/partials/plugin_config.html
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/coding-standards.mdc)
**/*.py: Prefer clear, readable code over clever optimizations (Simplicity First principle)
Make intentions clear through naming and structure (Explicit over Implicit principle)
Validate inputs and handle errors early (Fail Fast principle)
Use docstrings for classes and complex functions
Use PascalCase for class names (e.g.,NHLRecentManager)
Use snake_case for function and variable names (e.g.,fetch_game_data)
Use UPPER_SNAKE_CASE for constants (e.g.,ESPN_NHL_SCOREBOARD_URL)
Use leading underscore for private methods (e.g.,_fetch_data)
Use structured logging with context (e.g.,[NHL Recent]) for logging messages
Catch specific exceptions, not bareexcept:statements
Provide user-friendly error messages that explain what went wrong and potential solutions
Implement graceful degradation to continue operation when non-critical features fail
Use type hints for function parameters and return values
Validate required configuration fields on initialization
Provide sensible default values in code rather than in configuration files
Handle different deployment contexts with environment awareness in code
**/*.py: Code must run on Raspberry Pi, not Windows development machine
Optimize code for Raspberry Pi's limited RAM and CPU capabilities
Clean up resources regularly to manage memory effectively
Implement comprehensive logging for remote debugging on Raspberry Pi
Provide clear error messages for troubleshooting
Files:
web_interface/blueprints/api_v3.pysrc/plugin_system/store_manager.py
**/*manager*.py
📄 CodeRabbit inference engine (.cursor/rules/coding-standards.mdc)
All sports managers should implement the BaseManager pattern with
__init__(self, config, display_manager, cache_manager),update()for fetching/processing data, anddisplay(self, force_clear=False)for rendering
Files:
src/plugin_system/store_manager.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
All sports, weather, and stock managers should be organized in src/ directory
src/**/*.py: Use rpi-rgb-led-matrix library for hardware control on Raspberry Pi
Use cache_manager.py for data persistence and caching
Implement non-blocking data fetching using background services
Minimize unnecessary redraws to optimize LED matrix display performance
Implement health checks and status reporting
Provide web interface for remote configuration and monitoring on Raspberry Pi
Files:
src/plugin_system/store_manager.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : Validate manifest.json syntax as valid JSON and ensure class_name matches actual class name
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/config_schema.json : `config_schema.json` must define JSON schema for plugin configuration validation
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
Applied to files:
src/plugin_system/store_manager.py
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : Validate manifest.json syntax as valid JSON and ensure class_name matches actual class name
Applied to files:
src/plugin_system/store_manager.py
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manager.py : Plugin class should implement `validate_config()` method for configuration validation
Applied to files:
src/plugin_system/store_manager.py
🧬 Code graph analysis (1)
web_interface/static/v3/plugins_manager.js (2)
web_interface/blueprints/pages_v3.py (1)
index(13-46)web_interface/app.py (1)
index(622-624)
🔇 Additional comments (4)
plugins/7-segment-clock (1)
1-1: Limited visibility: Only submodule reference provided, cannot review actual changes.This file contains only a git submodule commit pointer update. The PR objectives describe enhancements to manifest validation, configuration management, and UI support for complex array-of-objects structures, but the actual source code changes in
src/plugin_system/store_manager.py,web_interface/api_v3.py, and related files are not included in this review.To provide a thorough review of the "duplicate display settings" fix and the configuration system improvements, please provide the modified files mentioned in the enriched summary for review.
web_interface/templates/v3/base.html (1)
4821-4821: Cache-busting version bump forplugins_manager.jslooks correctUpdating the query param here is a safe way to force clients to pick up the new legacy
plugins_manager.jsbundle. Just verify there are no other hardcoded references to this script (with a different?v=) in other templates, so all entry points are consistently pointing at the same versioned URL.web_interface/blueprints/api_v3.py (1)
599-612: Skip re-merging display fields into the root config looks correctThe added guard to
continuewhenkey in display_fieldsprevents hardware/runtime display fields from being written both intocurrent_config['display'][...]and again at the root, which matches the intent to keep display settings nested. One thing to double‑check: duration fields (*_duration,default_duration,transition_duration) are still written both intodisplay.display_durationsand at the root; if you eventually want those to live only underdisplay, they’d need a similar skip path (or their own list) once the display controller no longer relies on the top‑level keys.web_interface/static/v3/plugins_manager.js (1)
3441-3586: Missing context: Please provide the review comment you'd like me to rewrite, enclosed in<review_comment>tags, along with any relevant repository context or shell script results needed for verification.
…eractive fixes Recreate one-shot install script that was deleted, with fixes for: 1. APT permission denied errors on /tmp 2. Non-interactive mode support Fixes: 1. Fix /tmp permissions before running first_time_install.sh: - chmod 1777 /tmp to ensure APT can write temp files - Set TMPDIR=/tmp explicitly - Preserve TMPDIR when using sudo -E 2. Enable non-interactive mode: - Pass -y flag or LEDMATRIX_ASSUME_YES=1 to first_time_install.sh - Prevents read prompt failure at line 242 when run via curl | bash 3. Better error handling: - Temporarily disable errexit to capture exit code - Re-enable errexit after capturing - Added fix_tmp_permissions() function This resolves the 'Permission denied' errors for APT temp files and the interactive prompt failure when running via pipe.
…d fix array object data persistence - Restore 'version' and 'display_modes' to required_fields in store_manager.py manifest validation (both occurrences at lines 839 and 977) - Fix updateArrayObjectData to merge input fields with existing item data to preserve non-editable properties like logo objects - Implement handleArrayObjectFileUpload to properly upload files and store metadata in data-file-data attribute - Implement removeArrayObjectFile to properly remove file metadata and update data structure - Update renderArrayObjectItem to preserve file data in data-file-data attribute when rendering existing items
Resolved conflicts: - store_manager.py: Kept version and display_modes in required_fields - plugins_manager.js: Merged file upload implementation with escaping improvements from main - Accepted origin/main versions for template files and one-shot-install.sh - Removed plugins/7-segment-clock (deleted in main)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @scripts/install/one-shot-install.sh:
- Around line 61-79: The retry() function loses the original command exit code
because `$?` is read after the `if` statement; change the logic to run the
command directly using `"$@" && return 0` (or capture its exit status
immediately) so the real status is preserved, declare and set `local status`
based on the command's exit code before any conditional that would overwrite it,
and use that `status` when returning after max attempts and in the error
message; update references inside retry() (the `"$@"` invocation, `local
status`, and the return/status handling) accordingly.
In @web_interface/static/v3/plugins_manager.js:
- Around line 2225-2235: The code incorrectly treats any key containing "_data"
as JSON-backed and allows null to overwrite config; update the detection to only
use key.endsWith('_data') (remove key.includes('_data')), compute baseKey with
key.replace(/_data$/, ''), parse with JSON.parse but only treat jsonValue as
JSON-backed when jsonValue !== null && typeof jsonValue === 'object' (use
Array.isArray for arrays if you need to distinguish), assign to
flatConfig[baseKey] only in that case and continue; otherwise skip the special
handling so null or unrelated keys do not corrupt the config.
🧹 Nitpick comments (3)
scripts/install/one-shot-install.sh (2)
259-263: Redundant check:first_time_install.shexistence already verified.Lines 243-246 already check for
$REPO_DIR/first_time_install.sh. Aftercd "$REPO_DIR"on line 254, this second check is redundant.♻️ Proposed removal
cd "$REPO_DIR" # Make sure the script is executable chmod +x first_time_install.sh - # Check if script exists - if [ ! -f "first_time_install.sh" ]; then - print_error "first_time_install.sh not found in $REPO_DIR" - exit 1 - fi - print_success "Starting main installation (this may take 10-30 minutes)..."
210-212: Consider validatingHOMEis set.In rare edge cases (certain cron jobs, systemd units),
HOMEmight be unset, resulting inREPO_DIR="/LEDMatrix". A guard would improve robustness.♻️ Optional defensive check
# Determine repository location + if [ -z "${HOME:-}" ]; then + print_error "HOME environment variable is not set" + exit 1 + fi REPO_DIR="${HOME}/LEDMatrix"src/plugin_system/store_manager.py (1)
839-839: Consider extracting required manifest fields to a class constant.This list is duplicated at line 977 in
install_from_url. If the required fields change, both locations must be updated, risking inconsistency.♻️ Suggested refactor
Add a class constant near the top of the class:
REQUIRED_MANIFEST_FIELDS = ['id', 'name', 'class_name', 'version', 'display_modes']Then reference it in both locations:
-required_fields = ['id', 'name', 'class_name', 'version', 'display_modes'] +required_fields = self.REQUIRED_MANIFEST_FIELDS
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
scripts/install/one-shot-install.shsrc/plugin_system/store_manager.pyweb_interface/static/v3/plugins_manager.js
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/coding-standards.mdc)
**/*.py: Prefer clear, readable code over clever optimizations (Simplicity First principle)
Make intentions clear through naming and structure (Explicit over Implicit principle)
Validate inputs and handle errors early (Fail Fast principle)
Use docstrings for classes and complex functions
Use PascalCase for class names (e.g.,NHLRecentManager)
Use snake_case for function and variable names (e.g.,fetch_game_data)
Use UPPER_SNAKE_CASE for constants (e.g.,ESPN_NHL_SCOREBOARD_URL)
Use leading underscore for private methods (e.g.,_fetch_data)
Use structured logging with context (e.g.,[NHL Recent]) for logging messages
Catch specific exceptions, not bareexcept:statements
Provide user-friendly error messages that explain what went wrong and potential solutions
Implement graceful degradation to continue operation when non-critical features fail
Use type hints for function parameters and return values
Validate required configuration fields on initialization
Provide sensible default values in code rather than in configuration files
Handle different deployment contexts with environment awareness in code
**/*.py: Code must run on Raspberry Pi, not Windows development machine
Optimize code for Raspberry Pi's limited RAM and CPU capabilities
Clean up resources regularly to manage memory effectively
Implement comprehensive logging for remote debugging on Raspberry Pi
Provide clear error messages for troubleshooting
Files:
src/plugin_system/store_manager.py
**/*manager*.py
📄 CodeRabbit inference engine (.cursor/rules/coding-standards.mdc)
All sports managers should implement the BaseManager pattern with
__init__(self, config, display_manager, cache_manager),update()for fetching/processing data, anddisplay(self, force_clear=False)for rendering
Files:
src/plugin_system/store_manager.py
src/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
All sports, weather, and stock managers should be organized in src/ directory
src/**/*.py: Use rpi-rgb-led-matrix library for hardware control on Raspberry Pi
Use cache_manager.py for data persistence and caching
Implement non-blocking data fetching using background services
Minimize unnecessary redraws to optimize LED matrix display performance
Implement health checks and status reporting
Provide web interface for remote configuration and monitoring on Raspberry Pi
Files:
src/plugin_system/store_manager.py
🧠 Learnings (5)
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
Applied to files:
src/plugin_system/store_manager.py
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : Validate manifest.json syntax as valid JSON and ensure class_name matches actual class name
Applied to files:
src/plugin_system/store_manager.py
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : Ensure entry_point file exists and is importable in manifest.json
Applied to files:
src/plugin_system/store_manager.py
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manager.py : Plugin class must implement `update()` method for data fetching
Applied to files:
src/plugin_system/store_manager.py
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manager.py : Plugin class should implement `validate_config()` method for configuration validation
Applied to files:
src/plugin_system/store_manager.py
🔇 Additional comments (11)
scripts/install/one-shot-install.sh (9)
1-17: Good use of strict bash settings and error tracking.The
set -Eeuo pipefailcombination provides robust error handling. TheCURRENT_STEPvariable for tracking progress is a nice touch for debugging failures.
19-38: Well-designed error handler with actionable guidance.The error handler provides useful debugging info (step, line, exit code) and common fixes. The trap is correctly configured.
40-59: LGTM!Clean and consistent output helpers.
81-111: LGTM!Good fallback strategy using multiple tools (ping, curl, wget) for broad compatibility.
113-139: LGTM with minor consideration.The disk space check is well-structured. The
|| echo "0"fallback and default value assignment handle most edge cases. Ifdfoutput is unexpectedly malformed, the integer comparison would fail, but the error handler would catch it gracefully.
141-172: LGTM!Comprehensive sudo/root detection with clear user guidance when privileges aren't available.
174-191: LGTM!Good defensive handling for
/tmppermissions issues common in piped execution contexts.
268-306: Well-structured installation execution with proper error handling.Good use of
set +e/set -eto capture the exit code without triggering the trap. The different invocation paths for root vs non-root are appropriate, and the user guidance on success/failure is helpful.
309-310: LGTM!Standard script entry point.
src/plugin_system/store_manager.py (2)
977-993: Validation strategy looks sound.The distinction between mandatory field presence (blocking) and version consistency checks (warnings-only) is a reasonable approach. This allows plugins with minor manifest issues to install while still alerting developers to potential problems.
Based on learnings,
entry_pointis also a required manifest field. However, the current implementation correctly handles this by providing a sensible default (manager.py) at lines 996-1001, which aligns with the coding guidelines for providing default values in code.
858-861: LGTM!Error handling for missing required fields is clear and provides actionable feedback to users by listing which fields are missing.
… required
- Remove 'version' from required_fields in store_manager.py (both occurrences)
- Some existing plugins have version: null or no version field (basketball-scoreboard, odds-ticker)
- All code uses safe accessors (manifest.get('version')), so optional is safe
- Keep 'display_modes' as required - all plugins have it and tests expect it
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web_interface/static/v3/plugins_manager.js (1)
2540-2551: Blocker: unresolved merge-conflict markers inside the boolean field templateThis will break parsing/bundling and matches the Biome parse error.
Minimal fix (keep the escaped attribute version; remove conflict artifacts)
- <input type="checkbox" -<<<<<<< HEAD - id="${itemId}_${propKey}" - name="${itemId}_${propKey}" - data-prop-key="${propKey}" - class="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded" - ${propValue ? 'checked' : ''} - id="${escapeAttribute(itemId)}_${escapeAttribute(propKey)}" - data-prop-key="${escapeAttribute(propKey)}" - class="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded" - ${propValue ? 'checked' : ''} - onchange="updateArrayObjectData('${escapeAttribute(fieldId)}')"> + <input type="checkbox" + id="${escapeAttribute(itemId)}_${escapeAttribute(propKey)}" + data-prop-key="${escapeAttribute(propKey)}" + class="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded" + ${propValue ? 'checked' : ''} + onchange="updateArrayObjectData('${escapeAttribute(fieldId)}')">
🤖 Fix all issues with AI agents
In @web_interface/static/v3/plugins_manager.js:
- Around line 2893-2943: There is an unresolved merge conflict leaving duplicate
array-of-objects handling and duplicated variable declarations; remove the
conflict markers and consolidate into a single implementation. Keep one
array-of-objects branch (the block that builds the array-of-objects UI using
renderArrayObjectItem, addArrayObjectItem, fieldId, itemsSchema, maxItems, and
the hidden `${fieldId}_data` input) and delete the duplicated copy and the extra
merge markers; ensure you only declare hasXWidget / xWidgetValue / xWidgetValue2
once (the later file-upload/x-widget detection logic should remain singular),
and verify no duplicated console.debug blocks remain so variable names aren’t
redeclared.
🧹 Nitpick comments (2)
web_interface/blueprints/api_v3.py (1)
602-618: Good fix: prevents display keys from being duplicated at config root.The new
if key in display_fields: continueguard ensures fields likerows/cols/brightness/...remain under the nesteddisplaystructure and don’t get merged intocurrent_configtop-level later, which aligns with the PR goal.One thing to watch: if
display_fieldsever includes a field that the earlier “Handle display settings” block doesn’t actually write intocurrent_config['display'], it’ll now be dropped. Consider makingdisplay_fieldsasetand/or deriving it from the actual mapping to reduce drift risk.web_interface/static/v3/plugins_manager.js (1)
2501-2519: Avoid hard-coded fallbackpluginId+ replace deprecatedescape/unescapebase64 encoding
- Defaulting to
'ledmatrix-news'risks uploading assets to the wrong plugin when context is missing (correctness + potential access control surprises).btoa(unescape(encodeURIComponent(...)))and the inverse decode path rely on deprecatedescape/unescapeand tend to be brittle.Proposed direction (minimal, safer)
- const pluginId = uploadConfig.plugin_id || (typeof currentPluginConfig !== 'undefined' ? currentPluginConfig?.pluginId : null) || (typeof window.currentPluginConfig !== 'undefined' ? window.currentPluginConfig?.pluginId : null) || 'ledmatrix-news'; + const pluginId = + uploadConfig.plugin_id || + (typeof window.currentPluginConfig !== 'undefined' ? window.currentPluginConfig?.pluginId : null) || + (typeof currentPluginConfig !== 'undefined' ? currentPluginConfig?.pluginId : null); + if (!pluginId) { + console.error('File upload widget missing pluginId context', { fieldId, fullKey }); + }And strongly consider switching to
TextEncoder/TextDecoderfor base64 UTF-8 (instead ofescape/unescape).Also applies to: 2523-2529
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/plugin_system/store_manager.pyweb_interface/blueprints/api_v3.pyweb_interface/static/v3/plugins_manager.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/plugin_system/store_manager.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/coding-standards.mdc)
**/*.py: Prefer clear, readable code over clever optimizations (Simplicity First principle)
Make intentions clear through naming and structure (Explicit over Implicit principle)
Validate inputs and handle errors early (Fail Fast principle)
Use docstrings for classes and complex functions
Use PascalCase for class names (e.g.,NHLRecentManager)
Use snake_case for function and variable names (e.g.,fetch_game_data)
Use UPPER_SNAKE_CASE for constants (e.g.,ESPN_NHL_SCOREBOARD_URL)
Use leading underscore for private methods (e.g.,_fetch_data)
Use structured logging with context (e.g.,[NHL Recent]) for logging messages
Catch specific exceptions, not bareexcept:statements
Provide user-friendly error messages that explain what went wrong and potential solutions
Implement graceful degradation to continue operation when non-critical features fail
Use type hints for function parameters and return values
Validate required configuration fields on initialization
Provide sensible default values in code rather than in configuration files
Handle different deployment contexts with environment awareness in code
**/*.py: Code must run on Raspberry Pi, not Windows development machine
Optimize code for Raspberry Pi's limited RAM and CPU capabilities
Clean up resources regularly to manage memory effectively
Implement comprehensive logging for remote debugging on Raspberry Pi
Provide clear error messages for troubleshooting
Files:
web_interface/blueprints/api_v3.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to config/config.json : Plugin configuration in `config/config.json` must include fields: enabled, display_duration, live_priority, transition configuration, and plugin-specific settings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : Validate manifest.json syntax as valid JSON and ensure class_name matches actual class name
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/config_schema.json : `config_schema.json` must define JSON schema for plugin configuration validation
🧬 Code graph analysis (1)
web_interface/blueprints/api_v3.py (1)
web_interface/static/v3/plugins_manager.js (1)
key(6395-6395)
🪛 Biome (2.1.2)
web_interface/static/v3/plugins_manager.js
[error] 2893-2894: Expected a statement but instead found '<<<<<<< HEAD'.
Expected a statement here.
(parse)
[error] 2936-2937: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2937-2938: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2938-2939: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
…a detection - Fix retry() function to preserve original command exit code by capturing status immediately after command execution - Fix JSON data detection to prevent null from overwriting config by checking jsonValue !== null before treating as object - Both fixes prevent edge cases that could cause incorrect behavior or data corruption
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web_interface/static/v3/plugins_manager.js (2)
2538-2554: Fix unresolved merge-conflict markers and duplicate attributes (currently breaks JS parsing).
There’s a literal<<<<<<< HEADin the boolean input template plus duplicatedid/nameblocks, which will fail parsing and/or generate invalid HTML.Proposed fix
} else if (propSchema.type === 'boolean') { // Boolean checkbox html += ` <label class="flex items-center"> <input type="checkbox" -<<<<<<< HEAD - id="${itemId}_${propKey}" - name="${itemId}_${propKey}" - data-prop-key="${propKey}" - class="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded" - ${propValue ? 'checked' : ''} id="${escapeAttribute(itemId)}_${escapeAttribute(propKey)}" data-prop-key="${escapeAttribute(propKey)}" class="h-4 w-4 text-blue-600 focus:ring-blue-500 border-gray-300 rounded" ${propValue ? 'checked' : ''} onchange="updateArrayObjectData('${escapeAttribute(fieldId)}')"> <span class="ml-2 text-sm text-gray-700">${escapeHtml(propLabel)}</span> </label> `; } else {
2477-2533: Array-of-objects file upload is currently broken due to (1) missing item wrapperidand (2) later overrides replacing real implementations with TODO placeholders.
In the IIFE implementation,
handleArrayObjectFileUpload()looks up#${itemId} .file-upload-widget-inline, butrenderArrayObjectItem()does not render an element withid="${fieldId}_item_${index}"—onlydata-index. So uploads will always fail to find the container.After the IIFE, the “top-level” array-of-objects block redefines
window.handleArrayObjectFileUpload/window.removeArrayObjectFilewith TODO placeholders, overriding the working async-upload versions defined earlier.Proposed fix
function renderArrayObjectItem(fieldId, fullKey, itemProperties, itemValue, index, itemsSchema) { const item = itemValue || {}; - const itemId = `${escapeAttribute(fieldId)}_item_${index}`; - let html = `<div class="border border-gray-300 rounded-lg p-4 bg-gray-50 array-object-item" data-index="${index}">`; + const itemId = `${fieldId}_item_${index}`; + let html = `<div id="${escapeAttribute(itemId)}" class="border border-gray-300 rounded-lg p-4 bg-gray-50 array-object-item" data-index="${index}">`; ... }window.handleArrayObjectFileUpload = async function(event, fieldId, itemIndex, propKey, pluginId) { const file = event.target.files[0]; if (!file) return; const itemId = `${fieldId}_item_${itemIndex}`; - const fileUploadContainer = document.querySelector(`#${itemId} .file-upload-widget-inline`); + const itemEl = document.getElementById(itemId); + const fileUploadContainer = itemEl?.querySelector('.file-upload-widget-inline'); if (!fileUploadContainer) { console.error('File upload container not found'); return; } ... };-// Functions to handle array-of-objects -// Define these at the top level (outside any IIFE) to ensure they're always available -if (typeof window !== 'undefined') { - ... - window.handleArrayObjectFileUpload = function(event, fieldId, itemIndex, propKey, pluginId) { - // TODO: Implement file upload handling for array object items - ... - }; - window.removeArrayObjectFile = function(fieldId, itemIndex, propKey) { - // TODO: Implement file removal for array object items - ... - }; -} +// Remove the duplicate top-level array-of-objects block (it overrides the real implementations above).Also applies to: 3727-3832, 6795-7057
🤖 Fix all issues with AI agents
In @scripts/install/one-shot-install.sh:
- Around line 66-75: The retry loop fails under set -Eeuo pipefail because a
failing "$@" triggers the ERR trap/errexit before status=$? runs; to fix, run
the command in a context that disables errexit so you can capture its exit code
(e.g., use an if ! "$@"; then status=$?; else status=0; fi pattern or append "||
true" to "$@" and immediately assign status=$?); update the loop around "$@" in
the while block that references status, attempt and max_attempts so failures are
captured into status and the ERR trap does not prematurely exit.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/install/one-shot-install.shweb_interface/static/v3/plugins_manager.js
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
Applied to files:
web_interface/static/v3/plugins_manager.js
📚 Learning: 2025-12-27T19:16:42.155Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursor/rules/configuration-management.mdc:0-0
Timestamp: 2025-12-27T19:16:42.155Z
Learning: Applies to config/*.json : Ensure numeric values are numbers and lists are lists in configuration
Applied to files:
web_interface/static/v3/plugins_manager.js
🧬 Code graph analysis (1)
web_interface/static/v3/plugins_manager.js (2)
web_interface/blueprints/pages_v3.py (1)
index(13-46)web_interface/app.py (1)
index(622-624)
🪛 Biome (2.1.2)
web_interface/static/v3/plugins_manager.js
[error] 2894-2895: Expected a statement but instead found '<<<<<<< HEAD'.
Expected a statement here.
(parse)
[error] 2937-2938: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2938-2939: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 2939-2940: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (1)
web_interface/static/v3/plugins_manager.js (1)
2225-2237: Good:_dataJSON parsing now avoids treatingnullas an object.
This preventsflatConfig[baseKey] = nullwhen a hidden_datafield is empty/invalid-but-parseable.
…prove retry function - Remove unresolved merge conflict marker in array rendering (checkbox input attributes) - Fix array-of-objects file upload selector mismatch by adding id to wrapper element - Fix index-based preserve corruption by using data-item-data attributes instead of array indices - Add showNotification guards to prevent errors when notifications aren't available - Fix retry() function to work with set -Eeuo pipefail by disabling errexit for command execution
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @web_interface/static/v3/plugins_manager.js:
- Around line 3504-3802: You have working implementations of
window.handleArrayObjectFileUpload, window.removeArrayObjectFile and
window.updateArrayObjectData earlier, but later in the file they are redefined
with placeholder stubs which overwrite the real logic; remove or guard the later
stub assignments so the original implementations remain the single source of
truth (either delete the later stub blocks or change them to conditional
fallbacks like only assign if window.handleArrayObjectFileUpload/
removeArrayObjectFile/ updateArrayObjectData are undefined), ensuring the
file-upload UI and data-preservation behavior continues to use the full
implementations.
- Around line 2505-2518: The code uses a hard-coded fallback plugin id
('ledmatrix-news') and hard-coded upload allowlist/max size, which can
misattribute uploads and diverge from the x-upload-config; change the pluginId
resolution (variable pluginId) to derive strictly from uploadConfig.plugin_id or
currentPluginConfig/pluginId/window.currentPluginConfig and fall back to
null/empty (no hard-coded string), and update the file input onchange caller
(handleArrayObjectFileUpload) to receive that resolved pluginId; ensure the
upload validation uses uploadConfig.allowed_types and uploadConfig.max_size (not
fixed lists) both where the input accept attribute and in the handler that
enforces size/type, and make the same fixes in the duplicate block referenced
(handleArrayObjectFileUpload usage around lines ~3685-3712).
- Around line 3596-3683: The function updateArrayObjectData currently writes
number inputs as strings; fix by coercing input values to their proper types
before assigning to item: for inputs use input.type === "number" (or
input.getAttribute('data-prop-type') === 'number' if present) and set
item[propKey] = Number(input.value) (or input.valueAsNumber with NaN-check) so
numeric values remain numbers; for list/array fields use a data attribute like
data-prop-type="array" (or data-prop-is-list="true") and attempt
JSON.parse(input.value) (fallback to splitting on commas) to preserve arrays;
update references in updateArrayObjectData where input.value.trim() is assigned
(and where data-prop-key is used) and ensure the merged item written back to
data-item-data and hiddenInput preserves these coerced types (this works
alongside handlePluginConfigSubmit which expects proper types).
🧹 Nitpick comments (2)
web_interface/static/v3/plugins_manager.js (2)
2891-2921: Array-of-objects rendering: avoid hard-coded “Add Feed” label and escape hidden_dataconsistently.
For reuse across schemas, considerx-addLabel(like you did withx-removeLabel) and useescapeAttribute(...)for the hidden JSON value (not just replacing').
2477-2484: Refactor to replace deprecatedescape/unescapewith modern UTF-8-safe base64 helpers.The deprecated
escape()andunescape()functions are marked obsolete by MDN and should be avoided. The patternbtoa(unescape(encodeURIComponent(...)))and its inverse appear in multiple locations (lines 2482, 2509, 3675, 3732 for encoding; 3622, 3659, 6979 for decoding), making centralization beneficial.Replace with
TextEncoder/TextDecoderhelpers:Proposed helper functions and usage
+function base64EncodeUtf8(str) { + const bytes = new TextEncoder().encode(str); + let bin = ''; + bytes.forEach(b => (bin += String.fromCharCode(b))); + return btoa(bin); +} + +function base64DecodeUtf8(b64) { + const bin = atob(b64); + const bytes = Uint8Array.from(bin, c => c.charCodeAt(0)); + return new TextDecoder().decode(bytes); +}Then replace all occurrences:
-const itemDataBase64 = btoa(unescape(encodeURIComponent(itemDataJson))); +const itemDataBase64 = base64EncodeUtf8(itemDataJson);-const itemDataJson = decodeURIComponent(escape(atob(itemDataBase64))); +const itemDataJson = base64DecodeUtf8(itemDataBase64);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/baseball-scoreboardplugins/basketball-scoreboardplugins/clock-simpleplugins/football-scoreboardplugins/hockey-scoreboardplugins/ledmatrix-newsscripts/install/one-shot-install.shweb_interface/static/v3/plugins_manager.js
✅ Files skipped from review due to trivial changes (6)
- plugins/hockey-scoreboard
- plugins/football-scoreboard
- plugins/ledmatrix-news
- plugins/baseball-scoreboard
- plugins/basketball-scoreboard
- plugins/clock-simple
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/install/one-shot-install.sh
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
📚 Learning: 2025-12-27T19:16:24.801Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
Applied to files:
web_interface/static/v3/plugins_manager.js
📚 Learning: 2025-12-27T19:16:42.155Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursor/rules/configuration-management.mdc:0-0
Timestamp: 2025-12-27T19:16:42.155Z
Learning: Applies to config/*.json : Ensure numeric values are numbers and lists are lists in configuration
Applied to files:
web_interface/static/v3/plugins_manager.js
🧬 Code graph analysis (1)
web_interface/static/v3/plugins_manager.js (2)
web_interface/blueprints/pages_v3.py (1)
index(13-46)web_interface/app.py (1)
index(622-624)
🔇 Additional comments (1)
web_interface/static/v3/plugins_manager.js (1)
2227-2237: Good:_dataJSON parsing no longer treatsnullas an object.
This avoids accidentally writingflatConfig[baseKey] = nullfor JSON-backed fields.
…e coercion - Remove/guard duplicate updateArrayObjectData, handleArrayObjectFileUpload, and removeArrayObjectFile stub implementations that were overwriting real implementations - Fix hard-coded plugin ID fallback in renderArrayObjectItem - use null instead of 'ledmatrix-news' - Fix upload config to use uploadConfig.allowed_types and uploadConfig.max_size_mb from schema instead of hard-coded values - Store uploadConfig in data-upload-config attribute and read it in handleArrayObjectFileUpload for validation - Add type coercion to updateArrayObjectData: coerce number inputs to Number, array inputs via JSON.parse with comma-split fallback
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web_interface/static/v3/plugins_manager.js (2)
2225-2241: Avoid leaking*_datahelper fields into saved config when JSON is null/invalid.Right now, if
*_dataparses tonull(or parsing fails), you fall through and the raw helper field can end up being processed/saved. Consider alwayscontinuefor*_datakeys after attempting to parse, and treat parse failures as a user-visible validation error (or skip with a warning).Proposed fix
- if (key.endsWith('_data')) { - try { - const baseKey = key.replace(/_data$/, ''); - const jsonValue = JSON.parse(value); - // Handle both objects (patternProperties) and arrays (array-of-objects) - // Only treat as JSON-backed when it's a non-null object (null is typeof 'object' in JavaScript) - if (jsonValue !== null && typeof jsonValue === 'object') { - flatConfig[baseKey] = jsonValue; - console.log(`JSON data field ${baseKey}: parsed ${Array.isArray(jsonValue) ? 'array' : 'object'}`, jsonValue); - continue; // Skip normal processing for JSON data fields - } - } catch (e) { - // Not valid JSON, continue with normal processing - } - } + if (key.endsWith('_data')) { + const baseKey = key.replace(/_data$/, ''); + try { + flatConfig[baseKey] = JSON.parse(value); + console.log( + `JSON data field ${baseKey}: parsed ${Array.isArray(flatConfig[baseKey]) ? 'array' : typeof flatConfig[baseKey]}`, + flatConfig[baseKey] + ); + } catch (e) { + console.warn(`Invalid JSON in ${key}; skipping field to avoid saving helper input`, e); + // Optional: showNotification(`Invalid JSON for ${baseKey}. Please fix and try again.`, 'error'); + } + continue; // Always skip helper fields from normal processing + }
2477-2594: Serialize only schema-defined properties indata-item-datato prevent exposing sensitive fields.The current code stores the entire
itemobject indata-item-datawithout filtering. Since the codebase already supportsx-secretmetadata for regular fields, array items should apply the same filtering—only include schema-defined properties plus an explicit allowlist (e.g.,pathfor logos). If the backend returns extra fields or schemas gainx-secretentries, they will otherwise be exposed in the DOM viadata-item-data.Additionally, the upload button's
onclick="document.getElementById(...).click()"is fragile: when items are reindexed after removal, the ID lookup can fail. Useonclick="this.closest('.file-upload-widget-inline')?.querySelector('input[type=file]')?.click()"instead.Finally, add
data-prop-typeto form inputs soupdateArrayObjectData()can reliably coerce string form values to their correct schema types (numbers, booleans, etc.) when merging with preserved item data.Proposed fix
function renderArrayObjectItem(fieldId, fullKey, itemProperties, itemValue, index, itemsSchema) { const item = itemValue || {}; const itemId = `${escapeAttribute(fieldId)}_item_${index}`; // Store original item data in data attribute to preserve non-editable properties after reindexing + // Filter to schema-defined properties only to avoid exposing sensitive/non-schema fields - const itemDataJson = JSON.stringify(item); + const filteredItem = {}; + Object.keys(itemProperties).forEach(key => { + if (item[key] !== undefined) { + filteredItem[key] = item[key]; + } + }); + // Include path for logo objects (explicit allowlist) + if (item.path) filteredItem.path = item.path; + const itemDataJson = JSON.stringify(filteredItem); const itemDataBase64 = btoa(unescape(encodeURIComponent(itemDataJson))); let html = `<div id="${itemId}" class="border border-gray-300 rounded-lg p-4 bg-gray-50 array-object-item" data-index="${index}" data-item-data="${escapeAttribute(itemDataBase64)}">`; @@ -2545,6 +2558,7 @@ function renderArrayObjectItem(fieldId, fullKey, itemProperties, itemValue, ind html += ` <input type="${propSchema.format === 'uri' ? 'url' : 'text'}" id="${escapeAttribute(itemId)}_${escapeAttribute(propKey)}" data-prop-key="${escapeAttribute(propKey)}" + data-prop-type="${escapeAttribute(propSchema.type || 'string')}" class="block w-full px-3 py-2 border border-gray-300 rounded-md shadow-sm focus:outline-none focus:ring-blue-500 focus:border-blue-500 sm:text-sm bg-white text-black" value="${escapeAttribute(propValue || '')}" placeholder="${escapeAttribute(placeholder)}" @@ -2521,7 +2534,7 @@ function renderArrayObjectItem(fieldId, fullKey, itemProperties, itemValue, ind <input type="file" id="${escapeAttribute(itemId)}_logo_file" accept="${escapeAttribute(allowedTypes.join(','))}" style="display: none;" onchange="handleArrayObjectFileUpload(event, '${escapeAttribute(fieldId)}', ${index}, '${escapeAttribute(propKey)}', ${pluginIdParam})"> <button type="button" - onclick="document.getElementById('${escapeAttribute(itemId)}_logo_file').click()" + onclick="this.closest('.file-upload-widget-inline')?.querySelector('input[type=file]')?.click()" class="px-3 py-2 text-sm bg-gray-200 hover:bg-gray-300 text-gray-700 rounded-md transition-colors"> <i class="fas fa-upload mr-1"></i> Upload Logo </button>
🤖 Fix all issues with AI agents
In @web_interface/static/v3/plugins_manager.js:
- Around line 3602-3847: handleArrayObjectFileUpload currently locates the
target item by constructing an ID from fieldId and itemIndex which breaks when
items are reindexed; change it to derive the item element from the event (e.g.,
const itemEl = event.target.closest('.array-object-item')) and then query
fileUploadContainer = itemEl.querySelector('.file-upload-widget-inline') so
uploads always target the correct per-item element, remove reliance on
`${fieldId}_item_${itemIndex}` lookups, and ensure you still set data-prop-key
on the found container; also check response.ok before calling response.json()
and handle non-OK responses (showNotification/fallback) to avoid JSON parsing
errors on HTTP errors.
🧹 Nitpick comments (2)
web_interface/static/v3/plugins_manager.js (2)
2896-2949: Array-of-objects detection precedence looks right; consider schema-driven add-button label.Good move checking
prop.items.type === 'object' && prop.items.propertiesbefore other array widgets to avoid mis-rendering arrays-of-objects. Only nit: “Add Feed” is hard-coded; consider anx-addLabel(like you did withx-removeLabel) for reuse across different array-of-object fields.
3510-3714: Consolidate array-of-objects function definitions; stubs are fine but duplicate real implementations invite drift.You define
window.addArrayObjectItem/window.removeArrayObjectIteminside the IIFE and again after the IIFE. Only one “real” implementation should exist; keep early stubs only if needed, but avoid multiple real copies (they’ll diverge quickly).The defensive stubs (
updateArrayObjectData,handleArrayObjectFileUpload,removeArrayObjectFile) are a reasonable fallback.Also applies to: 6962-7002
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web_interface/static/v3/plugins_manager.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
📚 Learning: 2025-12-27T19:16:42.155Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursor/rules/configuration-management.mdc:0-0
Timestamp: 2025-12-27T19:16:42.155Z
Learning: Applies to config/*.json : Ensure numeric values are numbers and lists are lists in configuration
Applied to files:
web_interface/static/v3/plugins_manager.js
- Change from constructing ID to using event.target.closest('.array-object-item') to find item element
- Query fileUploadContainer from itemEl instead of using constructed ID lookup
- Remove reliance on `${fieldId}_item_${itemIndex}` which breaks after reindexing
- Add response.ok check before calling response.json() to avoid JSON parsing errors on HTTP errors
- Handle non-OK responses with proper error messages (JSON parse with fallback)
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @web_interface/static/v3/plugins_manager.js:
- Around line 2897-2927: The hidden input for array-of-objects data uses
JSON.stringify(currentItems).replace(/'/g, "'") which only escapes single
quotes and can break HTML or allow XSS; replace that manual escaping with the
existing escapeAttribute utility to fully escape &, <, >, ", and ' — i.e., call
escapeAttribute(JSON.stringify(currentItems)) when building the value for the
element id `${fieldId}_data` (the code block that renders the array container
alongside renderArrayObjectItem and the addArrayObjectItem button).
- Around line 3715-3849: handleArrayObjectFileUpload: update the default
uploadConfig allowed_types to include "image/jpg" (make default array
['image/png','image/jpeg','image/jpg','image/bmp'] or ensure allowedTypes
includes 'image/jpg') and add explicit validation for pluginId before attempting
the upload (if pluginId is null/undefined/empty, call showNotification with a
clear error and return) so the code fails fast rather than sending a request
that the backend will reject; locate the uploadConfig/defaults near the
fileUploadContainer getAttribute block and the fetch formData append/pluginId
handling and adjust accordingly.
🧹 Nitpick comments (4)
web_interface/static/v3/plugins_manager.js (4)
2227-2237: Avoid serializingNaN/Infinityinto config when coercing numbers.The new
jsonValue !== nullguard is correct, but the laterparseInt/parseFloatassignments can still produceNaN(which becomesnullin JSON), undermining “numbers are numbers” config expectations. Based on learnings, consider only persisting finite numbers (or omitting the field / using a default).Proposed fix (finite-number guard)
- } else if (propType === 'integer') { - flatConfig[actualKey] = parseInt(actualValue, 10); - } else if (propType === 'number') { - flatConfig[actualKey] = parseFloat(actualValue); + } else if (propType === 'integer') { + const v = parseInt(actualValue, 10); + if (Number.isFinite(v)) flatConfig[actualKey] = v; + } else if (propType === 'number') { + const v = parseFloat(actualValue); + if (Number.isFinite(v)) flatConfig[actualKey] = v; } else if (propType === 'boolean') {
2477-2529: Base64 DOM transport: avoid deprecatedescape/unescape, and don’t HTML-escape JS string params.
- Line 2482/2510/2515:
btoa(unescape(encodeURIComponent(...)))works but relies on deprecatedescape/unescapeand is easy to get wrong for unicode.- Line 2513/2523:
pluginIdParamis built viaescapeAttribute(...)but is injected into a JS string literal, not an HTML attribute—better to useJSON.stringify(...)for JS-safe quoting.Proposed fix (JS-safe quoting for pluginIdParam)
- const pluginIdParam = pluginId ? `'${escapeAttribute(pluginId)}'` : 'null'; + const pluginIdParam = pluginId ? JSON.stringify(String(pluginId)) : 'null';
3602-3713: Array-of-objects serialization: tighten numeric coercion to keep numbers/lists well-typed.Line 3655-3661: when a numeric input is empty/invalid, this can currently fall back to a string, which conflicts with “numbers are numbers” config expectations (based on learnings). Consider persisting
null/omitting the key instead of silently flipping types.Proposed fix (don’t write non-finite numeric values)
- if (inputType === 'number' || propType === 'number') { + if (inputType === 'number' || propType === 'number' || propType === 'integer') { // Use valueAsNumber if available, fallback to Number() const numValue = input.valueAsNumber !== undefined && !isNaN(input.valueAsNumber) ? input.valueAsNumber : Number(value); - item[propKey] = isNaN(numValue) ? value : numValue; + if (Number.isFinite(numValue)) { + item[propKey] = (propType === 'integer') ? Math.trunc(numValue) : numValue; + } else { + delete item[propKey]; + } } else if (propType === 'array' || input.getAttribute('data-prop-is-list') === 'true') {
6990-7030: Defensive stubs can mask real load-order regressions.The “define stub if undefined” pattern (Line 6992-6996, 7018-7030) prevents hard failures but also makes it easy to ship broken wiring without noticing. Consider gating these stubs behind
_PLUGIN_DEBUG_EARLY(or remove for production) so failures surface during testing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web_interface/static/v3/plugins_manager.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
📚 Learning: 2025-12-27T19:16:42.155Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursor/rules/configuration-management.mdc:0-0
Timestamp: 2025-12-27T19:16:42.155Z
Learning: Applies to config/*.json : Ensure numeric values are numbers and lists are lists in configuration
Applied to files:
web_interface/static/v3/plugins_manager.js
- Replace manual single-quote escaping with escapeAttribute() for proper HTML escaping in array-of-objects hidden input - Update default allowed_types to include 'image/jpg' in handleArrayObjectFileUpload - Add explicit pluginId validation before upload to fail fast with clear error message - Prevents XSS vulnerabilities and backend rejections from invalid uploads
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @web_interface/static/v3/plugins_manager.js:
- Around line 3715-3731: In handleArrayObjectFileUpload, narrow the file widget
lookup to the specific propKey instead of using
itemEl.querySelector('.file-upload-widget-inline') (e.g. select by
attribute/class that encodes propKey such as
`.file-upload-widget-inline[data-prop-key="${propKey}"]` or by input name) so
you target the correct file-upload field when an item contains multiple file
widgets; also harden pluginId validation by checking typeof pluginId ===
'string' before calling trim() (replace direct pluginId.trim() with a guarded
variant). Apply the same propKey-scoped selector and pluginId type-check change
to the other occurrence in the file around the 3763–3770 block as well.
♻️ Duplicate comments (2)
web_interface/static/v3/plugins_manager.js (2)
2477-2529: Fix upload defaults + avoid HTML-escapingpluginIdin inline JS. (Line 2504–2516)
allowedTypesdefault here omitsimage/jpg, but the upload handler validatesimage/jpg(can reject/disable legitimate JPEGs depending on browser MIME).pluginIdParamusesescapeAttribute(pluginId)inside a JS string literal; if a plugin id ever contains escapable characters, the value passed to the API will be wrong. PreferJSON.stringify.Proposed fix
- const allowedTypes = uploadConfig.allowed_types || ['image/png', 'image/jpeg', 'image/bmp']; + const allowedTypes = uploadConfig.allowed_types || ['image/png', 'image/jpeg', 'image/jpg', 'image/bmp']; const maxSizeMB = uploadConfig.max_size_mb || 5; - const pluginIdParam = pluginId ? `'${escapeAttribute(pluginId)}'` : 'null'; + const pluginIdParam = pluginId ? JSON.stringify(pluginId) : 'null';
2533-2540:removeArrayObjectFileis still ID/index-based; can break after reindex—switch to event/DOM-based lookup. (Line 2536–2538, 3829–3832, 3858–3881)
handleArrayObjectFileUploadalready finds the item viaevent.target.closest('.array-object-item'), butremoveArrayObjectFilestill does#${fieldId}_item_${itemIndex}lookup. If indices/IDs drift, “Remove” can fail or target the wrong item.Proposed fix (backward compatible: pass
eventfrom onclick; fallback to old lookup)- onclick="removeArrayObjectFile('${escapeAttribute(fieldId)}', ${index}, '${escapeAttribute(propKey)}')" + onclick="removeArrayObjectFile('${escapeAttribute(fieldId)}', ${index}, '${escapeAttribute(propKey)}', event)"- imageContainer.innerHTML = ` + imageContainer.innerHTML = ` <img src="/${escapedPath}" alt="Logo" class="w-16 h-16 object-cover rounded border"> <button type="button" - onclick="removeArrayObjectFile('${escapedFieldId}', ${currentItemIndex}, '${escapedPropKey}')" + onclick="removeArrayObjectFile('${escapedFieldId}', ${currentItemIndex}, '${escapedPropKey}', event)" class="text-red-600 hover:text-red-800"> <i class="fas fa-trash"></i> Remove </button> `;-window.removeArrayObjectFile = function(fieldId, itemIndex, propKey) { - const itemId = `${fieldId}_item_${itemIndex}`; - const fileUploadContainer = document.querySelector(`#${itemId} .file-upload-widget-inline`); +window.removeArrayObjectFile = function(fieldId, itemIndex, propKey, event) { + const itemEl = event?.target?.closest?.('.array-object-item') || null; + const fileUploadContainer = + itemEl?.querySelector(`.file-upload-widget-inline[data-prop-key="${propKey}"]`) || + itemEl?.querySelector('.file-upload-widget-inline') || + document.querySelector(`#${fieldId}_item_${itemIndex} .file-upload-widget-inline`); if (!fileUploadContainer) { console.error('File upload container not found'); return; }Also applies to: 3824-3833, 3858-3881
🧹 Nitpick comments (1)
web_interface/static/v3/plugins_manager.js (1)
3510-3713: Nice: item preservation + type coercion inupdateArrayObjectData. (Line 3602–3713)
- Preserving per-item state via
data-item-dataand overlaying input changes avoids silent loss of non-editable fields.- Numeric coercion aligns with the config learnings (“numbers are numbers”). Based on learnings, ensure numeric values are numbers and lists are lists in configuration.
Two nits worth tightening:
existingItemsshould be forced to an array after parsing (defensive).escape/unescapeare deprecated; consider a small helper usingTextEncoder/TextDecoderfor base64 UTF-8 encode/decode.Defensive array parse tweak
- try { + try { const existingData = hiddenInput.value.trim(); if (existingData) { - existingItems = JSON.parse(existingData); + const parsed = JSON.parse(existingData); + existingItems = Array.isArray(parsed) ? parsed : []; } } catch (e) { console.error('Error parsing existing items data:', e); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web_interface/static/v3/plugins_manager.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
📚 Learning: 2025-12-27T19:16:42.155Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursor/rules/configuration-management.mdc:0-0
Timestamp: 2025-12-27T19:16:42.155Z
Learning: Applies to config/*.json : Ensure numeric values are numbers and lists are lists in configuration
Applied to files:
web_interface/static/v3/plugins_manager.js
🔇 Additional comments (3)
web_interface/static/v3/plugins_manager.js (3)
2225-2237: Good null-guard for JSON-backed_datafields. (Line 2232–2234)This prevents
null(typeof'object') from overwriting config when parsing hidden JSON fields.
2897-2928: Array-of-objects detection + hidden_dataescaping look right. (Line 2897–2926)Doing the array-of-objects branch first (before other array widgets) and using
escapeAttribute(JSON.stringify(...))for the hidden field is the right direction.
6997-7037: Good: only define fallback stubs if missing. (Line 6997–7037)This avoids overwriting the real implementations at runtime.
- Narrow file widget lookup to use propKey-specific selector (.file-upload-widget-inline[data-prop-key]) to target correct widget when item has multiple file widgets - Harden pluginId validation by checking typeof pluginId === 'string' before calling trim() to prevent errors on non-string values
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web_interface/static/v3/plugins_manager.js (1)
2425-2453: Addlet currentPluginConfig = null;declaration in the IIFE scope.Inside the strict-mode IIFE (line 798),
currentPluginConfigis never declared withlet/const/var. Onlywindow.currentPluginConfigis initialized (line 803). Code at lines 2192, 2197–2198, 2428, 2441, and 2449 assigns to or reads the bare variablecurrentPluginConfig, which will throwReferenceErrorin strict mode. Add the declaration alongside other local variables:let installedPlugins = []; let currentPluginConfig = null; // Add this line window.currentPluginConfig = null;Also applies to: 2192–2199
♻️ Duplicate comments (3)
web_interface/static/v3/plugins_manager.js (3)
2505-2524:allowedTypesdefault mismatches upload handler defaults (accept attr may exclude JPG).
renderArrayObjectItemdefaults to['image/png','image/jpeg','image/bmp']buthandleArrayObjectFileUploaddefaults includeimage/jpg; align these so picker + validation stay consistent.Proposed fix
- const allowedTypes = uploadConfig.allowed_types || ['image/png', 'image/jpeg', 'image/bmp']; + const allowedTypes = uploadConfig.allowed_types || ['image/png', 'image/jpeg', 'image/jpg', 'image/bmp'];
3858-3881:removeArrayObjectFilecan break after item deletion/reindex (id-based lookup + unscoped widget).
removeArrayObjectFileuses#${fieldId}_item_${itemIndex}+ first.file-upload-widget-inline(not propKey-scoped).- The later
window.removeArrayObjectItemreindexesdata-indexbut does not update the.array-object-itemelement’sid, so#${fieldId}_item_${itemIndex}can stop matching after deletions.Fix by (a) updating item container ids during reindex, and/or (b) making
removeArrayObjectFilelocate the item bydata-indexwithin#${fieldId}_itemsand then select.file-upload-widget-inline[data-prop-key="${propKey}"].Proposed fix
window.removeArrayObjectFile = function(fieldId, itemIndex, propKey) { - const itemId = `${fieldId}_item_${itemIndex}`; - const fileUploadContainer = document.querySelector(`#${itemId} .file-upload-widget-inline`); + const itemsContainer = document.getElementById(fieldId + '_items'); + const itemEl = + itemsContainer?.querySelector(`.array-object-item[data-index="${itemIndex}"]`) || + document.getElementById(`${fieldId}_item_${itemIndex}`); + const fileUploadContainer = + itemEl?.querySelector(`.file-upload-widget-inline[data-prop-key="${propKey}"]`) || + itemEl?.querySelector('.file-upload-widget-inline'); if (!fileUploadContainer) { console.error('File upload container not found'); return; } @@ };window.removeArrayObjectItem = function(fieldId, index) { @@ const remainingItems = itemsContainer.querySelectorAll('.array-object-item'); remainingItems.forEach((itemEl, newIndex) => { itemEl.setAttribute('data-index', newIndex); + // Keep id-based helpers working (removeArrayObjectFile fallback, etc.) + itemEl.id = `${fieldId}_item_${newIndex}`; @@ });Also applies to: 6917-6995
3602-3713:updateArrayObjectData: ensureexistingItemsis an array + consider schema-driven coercion.
- After
JSON.parse, if the hidden value is an object,existingItems[index]becomes wrong; guard withArray.isArray(...).- Type coercion currently depends on
input.type/data-prop-type, butrenderArrayObjectItemdoesn’t appear to setdata-prop-typefrom schema; consider deriving types fromitemsSchemafor reliable number/array preservation. Based on learnings, numeric values should be numbers and lists should be lists in configuration.Proposed fix (array guard)
- existingItems = JSON.parse(existingData); + const parsed = JSON.parse(existingData); + existingItems = Array.isArray(parsed) ? parsed : [];
🧹 Nitpick comments (3)
web_interface/static/v3/plugins_manager.js (3)
2478-2484: Avoid deprecatedescape/unescapein base64 encoding/decoding paths.
btoa(unescape(encodeURIComponent(...)))(and the inverse) relies on deprecated APIs; consider switching toTextEncoder/TextDecoderto avoid future browser quirks.
3510-3553: Schema navigation inaddArrayObjectItemis brittle for nested paths.
This manualfullKey.split('.')walk will fail for more complex schemas (and it’s now duplicated by another implementation later). Prefer using the already-availablegetSchemaProperty(schema, fullKey)in this function too, or delete one implementation to keep a single source of truth.
3715-3856: Upload handler improvements look good; tightenpluginIdvalidation type.
Nice: propKey-scoped lookup, config-driven type/size validation, andresponse.okchecks. ForpluginId, you can simplify to a strict string check to avoid odd truthy non-strings.Proposed tweak
- if (!pluginId || pluginId === 'null' || pluginId === 'undefined' || (typeof pluginId === 'string' && pluginId.trim() === '')) { + if (typeof pluginId !== 'string' || pluginId.trim() === '') { if (typeof showNotification === 'function') { showNotification('Plugin ID is required for file upload', 'error'); } console.error('File upload failed: pluginId is required'); return; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web_interface/static/v3/plugins_manager.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursorrules:0-0
Timestamp: 2025-12-27T19:16:24.801Z
Learning: Applies to plugins/*/manifest.json : `manifest.json` must include required fields: id, name, version, entry_point, class_name, and display_modes
📚 Learning: 2025-12-27T19:16:42.155Z
Learnt from: CR
Repo: ChuckBuilds/LEDMatrix PR: 0
File: .cursor/rules/configuration-management.mdc:0-0
Timestamp: 2025-12-27T19:16:42.155Z
Learning: Applies to config/*.json : Ensure numeric values are numbers and lists are lists in configuration
Applied to files:
web_interface/static/v3/plugins_manager.js
🧬 Code graph analysis (1)
web_interface/static/v3/plugins_manager.js (2)
web_interface/blueprints/pages_v3.py (1)
index(13-46)web_interface/app.py (1)
index(622-624)
🔇 Additional comments (3)
web_interface/static/v3/plugins_manager.js (3)
2227-2237: Good hardening of_dataJSON parsing (null guard).
ThejsonValue !== null && typeof jsonValue === 'object'check preventsnullfrom silently overwriting config.
2897-2927: Array-of-objects detection + hidden_datauses proper escaping (LGTM).
Rendering the array-of-objects widget first and usingescapeAttribute(JSON.stringify(currentItems))is a solid direction (prevents broken attributes / injection from the hidden JSON).
6997-7003: Good defensive stubs (only define if missing).
Guarding these globals avoids overwriting the real implementations.Also applies to: 7023-7037
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.