LEDMatrix

mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-08-01 16:58:06 +00:00

Author	SHA1	Message	Date
Chuck GitHub Claude Sonnet 5	5b45f35888	Vegas mode: reclaim dead space and pace the rotation (#423 ) * Vegas mode: reclaim dead space and pace the rotation On a wide panel Vegas mode spent much of its time showing black. At 50px/s on a 512px display, one display width of blank is 10.2 seconds, which makes several long-standing behaviours expensive: - ScrollHelper prepended a full display width of black as an "initial gap", charged once per cycle — 10.2s of black at the start of every rotation. - Plugins without get_vegas_content() are captured off a full-display canvas, so their blank margins entered the ticker too. Measured: of-the-day drew 35px of "No Data" on a 512px canvas (92% blank), youtube-stats 142px of content with 185px of black either side. Only the scroll_helper path had any trimming. - Cycle transitions deliberately pushed a blank frame and then recomposed synchronously: 84ms at best, 4.8s at worst, every millisecond of it black. - buffer_ahead doubled as the cycle size, so a 21-plugin install showed 3 plugins per cycle and took ~7 cycles to come around. - separator_width was applied between every image rather than at plugin boundaries, so a per-row ticker like the F1 scoreboard (116 images, which it renders 4px apart internally) got a 32px chasm between each row — and the width budget didn't count those gaps, so the plugin quietly occupied far more of the panel than intended. Changes: - src/vegas_mode/geometry.py: numpy column-ink primitives shared by the trimmer and the audit tool, so the number reported is the number acted on. A Python per-column loop over a 17,000px strip is far too slow for the render path. - PluginAdapter trims every content path, not just scroll_helper. Only outer edges are cropped: interior blank columns are the plugin's own layout (logo left, score right) and closing them would corrupt the design. A plugin on a non-black background is inherently unaffected. - ScrollHelper.create_scrolling_image takes an explicit lead_gap, still defaulting to display_width so the many standalone-ticker callers are unchanged. Vegas passes lead_in_width (default 0). - Cycle end holds the last rendered frame instead of blanking, turning the recompose into a brief freeze rather than the panel switching off. - plugins_per_cycle (default 6) is split from buffer_ahead, which goes back to being only a prefetch low-water mark. - max_plugin_width_ratio (default 3x display width) caps one plugin's share of a cycle. Overflow is deferred, not discarded: a rotation offset advances each fetch so later rows appear on subsequent cycles. Single oversized images are cropped at a blank column so the cut misses glyphs. - Composition groups images by plugin: rows are joined by intra_plugin_gap (default 8) and separator_width applies only between plugins. The width budget now counts those gaps. - Plugin data updates no longer run on the Vegas render path. All new settings are user-configurable in Display -> Vegas Scroll, including min/max cycle duration and dynamic duration, which previously existed in code but were reachable only by hand-editing config.json. Measured with scripts/dev/vegas_audit.py on a 512x64 panel: mean ink coverage 42.7% -> 69.4% fully blank 5.9% -> 0% reads as empty 13.6% -> 0% worst blank stretch 4.8s -> 0s full rotation 414s -> 123s plugins per cycle 3 -> 6 Note the metric choice: a "fully blank" scan (>=95% black viewport) reported only 0.4% and badly understated the problem, because two full-width segments with mid-canvas content never fully blank the viewport — they hold it at ~28%. window_coverage_stats grades every viewport position by how much ink it carries, which is what tracks perceived dead time. Known remaining: cycle transitions still freeze ~3.5s while the next cycle is fetched. Fixing that needs background prefetch, which is deferred because the fallback-capture path mutates the shared display_manager.image and racing it against the render loop risks torn frames. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Drop unused Optional import from the vegas audit script Flagged by Codacy (F401). Any, Dict and List are all still used. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Align Vegas API bounds with validate(), fix audit config plumbing Both from review feedback on #423. The web API's accepted ranges disagreed with VegasModeConfig.validate(), which is what actually gates Vegas starting: scroll_speed 1-100 -> 1-200 (a slider value of 150 returned 400) separator_width 0-500 -> 0-128 target_fps 1-200 -> 30-200 buffer_ahead 1-20 -> 1-5 The three loose ones were the dangerous direction: the value saved with a 200, then VegasModeCoordinator.start() failed validation with only a log line, so the ticker silently never ran. The UI already matched validate() in all four cases, so the API was the odd one out. test_vegas_api_bounds_match_validate parses the numeric_fields map out of api_v3 and asserts every bound against validate(), plus that validate() accepts both endpoints and rejects just outside them, so these cannot drift apart again. That test immediately caught a missing upper bound on min_plugin_width, now added — unbounded it would drop every segment and leave a blank ticker. Separately, vegas_audit.py constructed PluginAdapter without the config, so it fell back to VegasModeConfig() defaults and would report trimming and width-budget behaviour that differed from the user's config.json. It now passes the loaded config exactly as the coordinator does. This is the same class of drift the explicit lead_gap and grouping arguments already guard against. Output is unchanged on a rig whose config matches the defaults. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Vegas mode: render plugins narrower, space rows by measured separation Trimming reclaims blank margins but cannot compact a layout that genuinely spans the display — a five-column forecast, a progress bar drawn at 100% width, a stat block with the panel's whole width between its elements. Those need the plugin to make different layout decisions, which means telling it the screen is narrower while it renders. DisplayManager.render_size() presents a smaller logical canvas for the duration of a Vegas content fetch, reusing the same _LogicalMatrix indirection double-sided mode already relies on so plugins see a consistent size from every accessor. Plugins that size themselves from matrix.width need no changes at all; one that wants to be explicit can read the new BasePlugin.get_vegas_render_width(). Width is a percentage so a single setting travels across panel sizes: vegas_scroll.render_width_pct globally, or vegas_width_pct in an individual plugin's config. Measured on a 512x64 panel with real data: ledmatrix-weather 1536px -> 576px (forecast becomes narrow cards) youtube-stats 353px -> 199px (2% blank left, so genuinely compact) geochron 453px -> 153px (ink density rises to 100%) ledmatrix-flights 950px -> 740px The youtube-stats figure is the clearest evidence the layout itself changed rather than being cropped: at full width the content had to be trimmed from 512px to 353px, whereas at 40% it arrives with almost no blank to reclaim. Row spacing is now measured rather than added. A flat gap gets it wrong in both directions at once — content drawn flush to its own edges ends up nearly touching (reported for recent sports scores, which sat 8px apart), while content already carrying wide margins gets pushed even further out. separation_gap() measures the blank each pair already has and adds only the shortfall, up to min_content_separation (default 24). intra_plugin_gap stays as a floor applied regardless. Two tests shipped in the previous commit encoded the old flat-gap arithmetic and are updated to the measured semantics, including one renamed to reflect that zero intra_plugin_gap alone no longer butts rows together. Also fixes a real bug found while testing: the harness display manager had no render_size(), and because the adapter catches broadly that surfaced as "no content" rather than an error, silently dropping five plugins. Added the context to VisualTestDisplayManager for parity, and _render_at() now degrades to a no-op on any display manager lacking it, so a third-party or older harness loses the narrowing rather than the content. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Vegas mode: end cycles before the wrap, keep the width budget honest Three fixes, the first a regression from lead_in_width defaulting to 0. get_visible_portion wraps: once scroll_position + display_width passes the end of the strip it fills the right of the frame from the head of the same strip. So the final display_width of travel showed the cycle's first plugin re-entering on the right while its last plugin exited on the left, and the recompose that followed replaced both at once. On a 512px panel at 50px/s that was 10.2s of two plugins on screen at once, ending in a hard cut — reported as the ticker "switching mid-scroll" from F1 to news. That used to be invisible because the strip began with a full display_width of blank, so the wrapped-in region was black. Removing that blank (it was 10s of dead panel per cycle) exposed the wrap. Cycles now end one display width earlier, before any wrapped content appears, clamped for strips no wider than the display so they don't complete instantly and spin the recompose loop. Verified on hardware: a 3936px strip now completes at 68.5s, exactly (3936 - 512) / 50. Second, auto_trim=False also skipped the width budget, which is an unrelated concern — turning off margin cropping should not let one plugin hold the panel for minutes. Seen in the field: the F1 scoreboard contributed 116 images and 14,848px untouched, giving a 33,821px cycle (11 minutes of content). The budget now applies regardless of trimming; with it restored that cycle is 6,362px. Third, the budget accounted for row gaps using the flat intra_plugin_gap while the compositor had moved to measured separation, so it under-counted by up to (min_content_separation - intra_plugin_gap) per row and a many-row plugin overran its cap. Both now use the same separation_gap() rule, and a test asserts the composed block fits the budget end to end rather than trusting the two paths to agree. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Fix IndexError in find_blank_cut when the cut lands on the image edge A cut position after the last column is legitimate — _crop_to_budget asks for min(start + budget, img.width), which equals the width whenever the remaining strip is shorter than the budget. find_blank_cut clamped target to width but then walked leftwards starting at target itself, so ink[width] raised IndexError. Caught on hardware: it killed the ledmatrix-stocks fetch, and because _fetch_plugin_content catches broadly that surfaced as the plugin silently contributing nothing for the cycle. Only reachable on the second or later pass of the rotating window over a single oversized image, which is why the existing tests missed it — they all exercised the first pass, where start is 0 and start + budget is comfortably inside the image. Added TestRotationAcrossMultipleCycles, which walks the window round several times and asserts content is never lost, plus direct coverage of find_blank_cut at and beyond the image edge. Both bounds now stop at width - 1 so neither direction can index past the end. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Only cut oversized segments at real gaps between items The width-budget crop snapped to the nearest blank column, and in rendered text the gap between two characters is a single column. So a cut routinely landed inside a word: the cycle showed "Wednesda" and the orphaned "y" turned up as a lone floating letter in the next cycle, positioned after whatever plugin happened to precede it. Measured on the clock-simple segment to confirm: its blank runs are [1, 1, 1, 1, 1, 8, 8] — five single-column letter gaps, every one of which find_blank_cut would happily have chosen. Cuts now only land in a run of at least min_cut_gap blank columns (default 6), which excludes letter spacing while still finding the gaps plugins put between items (the stocks ticker uses 32px, baseball 48px). Where no boundary falls inside the budget the cut waits for the next one and overruns, because splitting an item is worse than a slightly long segment. Continuous content is treated differently on purpose: an image with no internal gaps is a map or a chart, where any column is as good as another, so it is still cut to the budget exactly. The gap rule protects discrete items; letting a solid image escape the cap in its name would be wrong. blank_runs() is vectorised — 48ms for a 17,000px strip, against seconds for a per-column Python loop. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Hold capture_mode for every plugin render, not just narrowed ones The native content path only entered capture_mode when it was also narrowing the canvas, so at full width — which is every plugin without a vegas_width_pct override, i.e. most of them — a plugin calling update_display() while building its Vegas content wrote straight to the hardware. That is a visible flash mid-scroll, and it lines up with the flash reported at cycle transitions, when several plugins are fetched back to back. Suppression is now unconditional; the narrowing context stays separate because it is already a no-op at full width. Both contexts are reached through helpers that degrade to nullcontext when the display manager lacks them. That matters more than it looks: the adapter's handlers are deliberately broad, so an AttributeError from a missing context does not surface as an error — it surfaces as the plugin contributing nothing. Making the call unconditional without this turned 44 tests red for exactly that reason, all of them reporting lost content rather than the real cause. The test double now provides capture_mode and render_size too, so tests exercise the real contexts instead of silently taking the degraded path. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Vegas mode: one continuous strip instead of swapping cycles A cycle used to be a discrete strip that got replaced: motion stopped, every pixel was substituted at once, and the next group started with the viewport already full. That is the freeze, the flash and the jump. The strip is now extended rather than replaced. ScrollHelper gains append_content(), which adds items on the right without touching scroll_position or total_distance_scrolled, so motion continues and the next group simply arrives from the right. Because completion is measured against total_scroll_width, extending also defers completion — there is no longer a cycle boundary to see. drop_scrolled_prefix() reclaims what has gone past, keeping the strip bounded however long Vegas runs (observed 5,000-11,000px against an unbounded strip otherwise). It shifts total_distance_scrolled and total_scroll_width together so the completion arithmetic is unchanged, and refuses to run while the viewport is wrapping: wrapping reads the head of the strip into the right of the frame, so trimming the head there would visibly change the picture. A test caught that. Groups are prepared off the render thread. The constraint is that the canvas and the matrix proxy are process-wide mutable state, so narrowing or capturing through them from another thread would corrupt the frame the render loop is pushing. get_content() therefore takes offscreen_only: the background thread uses only paths that avoid the canvas, and anything needing it is marked and picked up on the render thread. That puts the expensive work (native renders of leaderboard and baseball cards, seconds each) in the background and leaves the cheap work (display capture, 40-600ms) in the foreground. DisplayManager's capture flag is now thread-local. As a shared flag, a background capture would have suppressed the render loop's own frame pushes for its duration, freezing the panel precisely when the point was to avoid a freeze. Canvas-bound plugins are drained one at a time rather than as a batch: six at once held the render thread for 1.75s. Drains are also spaced by two seconds while the lookahead is healthy, since taking them back to back turns one long stall into a run of short ones. When the strip is genuinely running short the throttle is ignored, because content matters more than smoothness there. Measured on hardware: zero cycle-complete swaps, drains landing 2-4s apart, lookahead holding at 1,200-3,500px, no errors. Set continuous_scroll false to restore the swap behaviour; the old path is intact. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Pace the Vegas frame loop adaptively: 31.5 -> 78.7 fps The loop slept a fixed frame_interval on top of however long the frame took, so at a measured 31.6ms per frame a flat 8ms of that was pure idle — a quarter of the budget spent not rendering. It now sleeps only the remainder of the budget. Measured on hardware: 31.5 fps to 78.7 fps sustained, with CPU going down from 150% to 127%. Scroll speed is unchanged at 49.9px/s against a configured 50, because motion is derived from elapsed time rather than frame count — this buys smoothness, not speed. Worth recording what the bottleneck was not: the per-frame render path measures 0.34ms in total (0.18ms for the numpy slice, 0.17ms for the dirty-tracking digest), which is a theoretical 2900 fps. Optimising any of that would have been wasted effort. The frame was idle, not busy. Also nices the prefetch thread. Its work is PIL and numpy that releases the GIL, so the scheduler can act on the priority, and without it the prefetch competes for the same cores as the render loop. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Sub-pixel scrolling: motion at the frame rate, not the pixel rate With integer positioning the number of distinct frames per second equals the scroll speed in px/s, however fast the loop renders. Measured at 50px/s and 78.7fps, 36% of frames were byte-identical: the extra frames cost work and bought no motion, and what was left was 50 discrete 1px steps a second. Two things were wrong with the pre-existing sub-pixel support. get_visible_portion never consulted sub_pixel_scrolling — it always took the integer path, so the flag and _get_visible_portion_subpixel were dead code. And that implementation needed scipy.ndimage.shift, which is not installed on the target devices (HAS_SCIPY is False there), so it would not have interpolated even if reached. Verified both: positions 1000.0 and 1000.5 produced identical frames either way. Blending is now wired up and implemented with numpy. Two details make it affordable: slice cached_array directly instead of building two PIL images only to convert them straight back (the naive version measured 15x the integer path), and use fixed-point uint16 multiply-add rather than float32, which suits the Pi's cores and gives finer weighting than the panel can resolve. Result 0.939ms against 0.237ms — 0.70ms added per frame, a 1065fps ceiling. Measured on hardware: 81.2 fps with blending on, against 78.7 with it off, so no cost within noise — and every frame is now a distinct position rather than one in three being a repeat. The trade is a slight horizontal softening of text, since each frame blends two positions. Set smooth_scroll false for maximum crispness. Also benchmarked and cleared as non-issues: extending the strip costs 9.4ms on an 11,000px strip and trimming 2.5ms, both under one frame at this rate. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Add overflow handling: keep ordered content whole instead of rotating a window The width budget split any oversized plugin by advancing a window each cycle. That is right for interchangeable items — news headlines, odds, stock prices — but wrong for ordered content: a league table showed ranks 1-6, then resumed at 7 two rotations later, which reads as out of order and out of context. Nobody needs rank 23 in a ticker; they need the top of the table, every time. overflow_mode chooses between them: rotate — advance a window each cycle so everything is seen eventually (unchanged default) truncate — always show the start and drop the rest, keeping ordered content coherent. Records no window state, so every pass starts at the top. Per-plugin vegas_overflow overrides the global setting, since one install has both kinds of plugin. Also adds per-plugin vegas_max_width_screens, so content that must stay whole can be given more room — or uncapped with 0 — without lifting the cap on every ticker. Applied on the test rig: f1-scoreboard and ledmatrix-leaderboard set to truncate, and baseball given 4.5 screens because it was showing 8 of 9 games when the whole slate needed only a little more room. Verified: F1 now reports "the first 10 of 116 ... the rest are not shown", baseball has dropped out of the budget log entirely, and stocks, odds-ticker and stock-news still rotate. Also corrects the crop log, which claimed "window advances next cycle" unconditionally and so misreported truncated crops. A test now pins the behaviour behind the message: truncate must leave no offset recorded. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Stop Vegas mode showing last night's games as if they were live A game that was live in the evening was still being drawn as live the next morning. Two faults combined to freeze plugin visuals indefinitely. PR #291 added a call to plugin_adapter.invalidate_plugin_scroll_cache() so a plugin's own cached scroll image would be rebuilt from fresh data. That method was never implemented. hot_swap_content() wraps the call in a broad except, so every hot swap has raised AttributeError and been swallowed silently ever since — which is why the visuals it was meant to keep fresh never were. Continuous scrolling then removed the only path that reached it at all: should_recompose() and hot_swap_content() are called from the non-continuous branch of run_frame(), and continuous_scroll defaults to True. So on a default install the pending-update flags were set by the update tick, never consumed, and grew without bound. Together these froze content completely, because refetching is not enough on its own: the sports plugins' get_vegas_content() regenerates only "if the cache is empty", so take_next_group() kept receiving the same picture however often it asked. Fixed by: - Implementing invalidate_plugin_scroll_cache(). It covers both layouts — a helper directly on the plugin (stocks, news, odds-ticker) and one owned by a scroll-display manager (the sports scoreboards, which is the shape that produced this bug) — and clears cached_image and cached_array together, since the array is the image's numpy mirror. - Adding StreamManager.invalidate_pending_updates() and calling it from the continuous branch. It only drops the caches; the plugin recomposes when it next comes round in the rotation. process_updates() is wrong here: it refetches synchronously and merges into the active buffer that continuous mode bypasses, and hot_swap_content() rebuilds and repositions the whole strip, which is the freeze-and-jump this mode exists to avoid. Tests assert the fix rather than the implementation: 14 of the 17 new tests fail without it. Includes the wiring itself, since the regression was a call that was simply absent, and a check that the scroll position is untouched so this cannot regress into the swap's visible jump. All Vegas suites pass (355 tests). test_display_controller_vegas_tick.py still cannot be collected off-device for want of rgbmatrix, identically with and without this change. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Fix two CodeRabbit-flagged test assertions in vegas density tests test_prepared_group_is_used_without_refetching had a tautological final assertion; now checks stream.calls directly. test_no_partial_letter_at_either_edge required both crop edges to be blank, but the left edge here is always the crop's start position with no lead-in gap in word_strip, so it legitimately carries ink — only the right edge is an actual cut and needs the check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-31 09:40:38 -04:00
Chuck GitHub Claude Opus 5	e2acbfb566	fix(web): don't validate double-sided settings when the feature is disabled (#422 ) * fix(web): don't validate double-sided settings when the feature is disabled Saving anything on the Display tab failed with a 400 when double-sided mode was off: Double-sided copies (2) must divide chain length (3) evenly The Display form posts every field in one request, including double_sided_copies (default 2) and double_sided_axis, whether or not the Enabled checkbox is ticked. The server block was gated only on "is any double-sided field present in the payload" — it wrote ds_config['enabled'] but never read it. A user with chain_length: 3 and the untouched default copies: 2 was locked out of saving any display setting at all: brightness, GPIO slowdown, Vegas, sync. Gate the checks on the enabled flag: - Divisibility against chain_length/parallel is hardware-relational and only runs when the feature is on. - Structural checks (copies parses as an int in 2..8, axis in the whitelist) still 400 when enabled; when disabled they drop the value and leave the stored one untouched rather than rejecting the save. The runtime already gated correctly (_resolve_double_sided returns None when disabled), so nothing there changes. Also in the Display tab: - Hide Copies / Split Axis until Enabled is ticked, mirroring the Vegas Scroll pattern. Hidden rather than disabled, so the fields keep submitting and the server still sees an 'off' state to persist. - Fix the save toast: the form's handler read xhr.responseJSON, a jQuery property that doesn't exist on a native XMLHttpRequest, so it was always undefined and every save reported a green "Display settings saved" — even the 400s. Parse responseText and use the real status. Tests: two existing double-sided tests asserted 200 on payloads that the divisibility check (added later, in #373) turns into 400s; the first now supplies matching hardware values and the second passes as written now that a disabled save skips the check. Added coverage for the reported regression, for bad values while disabled, and for the check still firing when enabled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aUvzTXpEYhNEWYQvFqQU9 * fix(web): treat only 2xx as a successful display save Review feedback on #422. - showDisplaySaveResult tested `xhr.status >= 400` for failure, so a network error — which reports status 0 — was waved through as "Display settings saved". That's the same class of false-success bug this branch set out to fix. Test the 2xx range instead, and let a response body refine a successful verdict without overturning a failed one. - Annotate the _copies_fits_hardware helper, matching the annotated helpers already in api_v3.py. - Cover the vertical divisibility branch: chain_length 2 would divide evenly, so only parallel 3 can produce the rejection, which pins the branch to the right hardware dimension. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016aUvzTXpEYhNEWYQvFqQU9 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-28 16:54:42 -04:00
Chuck GitHub Claude Opus 4.8	3872a68ff7	Surface plugin update availability on the plugins page (#421 ) * Surface plugin update availability on the plugins page The plugin manager page already showed each installed plugin's version and had an Update button, but nothing told users an update actually existed — they had to guess. The store manager already compares the installed manifest version against the registry's latest_version for its reinstall decision; this surfaces that same signal in the UI. - api_v3 /plugins/installed now returns `latest_version` (from the registry cache, no extra network call) and an `update_available` flag computed by a new semver-aware helper `_is_plugin_update_available`. A locally modified plugin whose version is ahead of the registry is not flagged. - The installed-plugin card shows "vX.Y.Z available" next to the installed version, and the Update button becomes emphasized ("Update to vX.Y.Z" with a gentle pulse) when a newer version is published — mirroring the app's own update banner styling. - Added tests for the helper and the endpoint fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012EW8yDbsk8EceDpq6Hjgqj * Catch InvalidVersion specifically in plugin update comparison Address review feedback: the version comparison caught a blind `except Exception` (ruff BLE001). Split the two failure modes and catch each specifically — ImportError for a missing `packaging` (a core dependency) and InvalidVersion for an unparseable version string — while preserving the existing "surface the mismatch" fallback behavior. Added a test for the unparseable-version path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012EW8yDbsk8EceDpq6Hjgqj --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-07-21 08:06:36 -04:00
Chuck GitHub Claude Fable 5	cdf03fb107	Add skin system: user-installable visual overlays for sports scoreboards (#419 ) * Add skin system: user-installable visual overlays for sports scoreboards Skins restyle a scoreboard's live/recent/upcoming rendering while the host plugin keeps doing data fetching, scheduling, caching, live priority, and vegas mode — the anti-fork alternative for users who only want a different layout. - src/skin_system/: ScoreboardSkin API, SkinContext (canvas + adaptive layout + logo/font helpers), discovery/loading runtime with API major version gating and per-skin module namespacing - src/base_classes/sports.py: _render_game() seam at the three _draw_scorebug_layout call sites; skin-first with built-in fallback, 3-strikes session disable, slow-render warning; per-mode skin config - scripts/validate_skin.py: headless multi-mode/multi-size validator with bundled per-sport fixtures (no hardware or network needed) - skins/example-classic-baseball/: working reference skin - Web UI: served-schema Visual Skin dropdown (validation never enum-restricted, so uninstalled skins can't invalidate configs) and GET /api/v3/skins - Store: registry entries with type "skin" install to skins/ - docs/SKIN_SYSTEM.md (architecture), docs/CREATING_SKINS.md (author guide incl. Claude Code prompt), view-model contract locked by tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LrCusPasy1qeUN5anK3aA1 * Address review feedback on skin system - skin_runtime: cache the entry module so the 2nd/3rd load of the same skin (live/recent/upcoming hosts) doesn't re-execute it with unbound sibling aliases; rebind cached sibling modules to their bare names around entry import and restore prior bindings after; include per- manifest mtimes in the discovery cache fingerprint so in-place skin updates are picked up - sports.py: count render_skin_card exceptions toward the 3-strike session disable - store_manager: validate skin ids (pattern + resolved-path containment in skins/), reject registry/manifest id mismatches, and stage+validate downloads in a temp sibling before replacing an existing skin - schema_manager: leave the schema untouched when the configured skin value is a per-mode mapping (a string dropdown could overwrite it) - validate_skin.py: reject non-positive sizes and non-object --options at parse time; support --output-dir outside the repo; type annotations - example skin: validate accent_color once at load with logged fallback - fixtures: pregame 0-0 scores in football/hockey upcoming fixtures - api /skins: rely on the self-invalidating discovery cache instead of force_refresh - docs: valid JSON manifest example, load_logo caching semantics spelled out, language ids on fenced blocks - tests: view-model contract test now exercises the real extractor; regression test for repeated same-skin loads with sibling modules Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LrCusPasy1qeUN5anK3aA1 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-18 11:07:08 -04:00
Chuck GitHub Claude	6a9d8014e5	Tag plugin logs structurally and surface the active plugin in System Logs (#418 ) - get_logger() now returns a PluginLoggerAdapter when given a plugin_id, so every plugin log call is stamped with plugin_id automatically instead of only calls that explicitly passed extra={'plugin_id': ...}. This makes the "[Plugin: x]" prefix reliable in the journalctl-backed log stream. - display_controller publishes the currently active mode/plugin to the shared cache whenever it changes, exposed via a new GET /api/v3/display/current-status endpoint. - System Logs page: adds a "Now showing" banner backed by that endpoint, a plugin filter dropdown (populated from parsed log lines), a plugin badge per log entry, and fixes log parsing to handle the short-iso timestamp format journalctl actually returns (the old regex only matched syslog timestamps, so level/plugin extraction silently never ran). Co-authored-by: Claude <noreply@anthropic.com>	2026-07-18 10:59:36 -04:00
Chuck GitHub Claude Sonnet 5	c90129285c	Web UI: mobile navigation, guided onboarding, basic/advanced config tiering + performance (#417 ) * chore(web): remove dead legacy client-side plugin-config generator (~2,300 lines) Plugin config forms have been rendered server-side (plugin_config.html via GET /partials/plugin-config/<id>) since the HTMX migration; the old client-side generator survived as unreachable code. Verified dead by call graph, not by naming: showPluginConfigModal and showGithubTokenInstructions have zero callers anywhere in templates or JS, and everything removed here is reachable only from those two roots. Removed: - plugins_manager.js: showPluginConfigModal, generatePluginConfigForm, generateFormFromSchema, generateFieldHtml, generateSimpleConfigForm, handlePluginConfigSubmit, the modal's JSON-editor view (initJsonEditor, switchPluginConfigView, syncFormToJson/JsonToForm, saveConfigFromJsonEditor, resetPluginConfigToDefaults, displayValidationErrors, closePluginConfigModal, savePluginConfiguration, currentPluginConfigState), their exclusive helpers (getSchemaPropertyType, escapeCssSelector, dotToNested, collectBooleanFields, normalizeFormDataForConfig, flattenConfig, loadCustomHtmlWidget), the orphaned-modal cleanup block, the modal's listener wiring, and the never-invoked showGithubTokenInstructions/closeInstructionsModal pair. - plugins.html: the #plugin-config-modal markup those functions drove. - base.html: the deprecated pluginConfigData() component and the window.PluginConfigHelpers shim (only ever called by pluginConfigData). Deliberately kept, verified still live: - renderArrayObjectItem, getSchemaProperty, escapeHtml/escapeAttribute (window-exposed for the top-level array-of-objects handlers the server-rendered form uses), toggleNestedSection, addKeyValuePair/ addArrayObjectItem families, executePluginAction, and window.currentPluginConfig = null init (file-upload.js and executePluginAction read it, optional-chained). - app()'s internal generateConfigForm/generateSimpleConfigForm methods in base.html: unreachable now but embedded in the live Alpine component; excising methods from a live object is deferred to keep this change zero-risk. Validation: every deletion seam inspected line-by-line; Jinja parse of both templates passes; repo-wide sweep confirms zero remaining references to any deleted function or element id (deleted ranges contained no Jinja tags). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): mobile navigation drawer + responsive CSS gap fixes Phones previously got the desktop layout squeezed: ~12 system tabs plus one tab per installed plugin wrapped into many rows of small pill buttons, and the header's settings-search and system-stats widgets were dropped entirely (hidden below their breakpoints, never relocated). - Off-canvas nav drawer below md: the existing nav markup (system tab row + #plugin-tabs-row, including dynamically injected plugin tabs) is wrapped in a #site-nav container that CSS repositions into a slide-in drawer on small screens. Same DOM nodes, same @click handlers, nothing duplicated. Tabs become full-width rows with 44px+ touch targets. A hamburger button (md:hidden) in the header and a backdrop toggle the new mobileNavOpen Alpine state (added to both app() definitions, mirroring activeTab). Clicking any tab, a search result, or the backdrop closes the drawer. At md+ hard CSS guards make all drawer styles inert - desktop renders exactly as before. - Header widgets relocated, not hidden: placeHeaderWidgets() in app.js moves the #settings-search-wrap and #system-stats nodes (same elements, listeners intact - both are looked up by id from SSE/search code, so they must never be duplicated) into the drawer below md and back into the header above it, via a matchMedia listener. - Fixed 13 breakpoint utility classes that templates referenced but app.css never defined (sm:block, sm:grid-cols-2, sm:text-sm, md:block, md:w-auto, lg:block, lg:flex, lg:w-64, xl:grid-cols-2/3, 2xl:grid-cols-2/3/4). This was a live bug: 'hidden sm:block' on the search box and 'hidden lg:flex' on the stats meant BOTH were invisible at every screen width. Audit method (repeatable): diff classes used in templates vs defined in app.css. - Mobile modal sizing: one global rule caps .modal-content at 95vw/90vh with internal scroll below 640px - covers every modal without per-template changes. - Horizontal-scroll affordance: pure-CSS edge-fade shadows on .overflow-x-auto containers (scrolling-shadows technique), plus larger in-table touch targets below md. Validation: breakpoint used-vs-defined audit now returns zero gaps; Jinja parse of base.html passes; all changes to desktop behavior are additive (new utilities) or scoped inside max-width media queries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): x-advanced schema flag groups plugin config fields under a collapsed Advanced Settings section Plugin config pages show every schema property at equal visual priority, which overwhelms first-time users. Plugin authors can now add "x-advanced": true to any flat (non-object) property in config_schema.json to move it into one collapsed "Advanced Settings (N)" section rendered after the basic fields - progressive disclosure with zero loss of control. Implementation: the main render loop in plugin_config.html splits ordered properties into basic/advanced tiers; the advanced group reuses the exact .nested-section/.nested-content/toggleSection() shell that nested object sections already use, so the settings search's expand-on-match behavior works on advanced fields with no JS changes. Object-type properties ignore the flag (they already render as their own collapsible sections). No backend change needed: jsonschema ignores unknown x-* keywords exactly as it does for x-widget/x-propertyOrder. Documented in docs/widget-guide.md alongside the other x-* extensions. Validation (rendered with real Jinja, not just parsed): - synthetic schema with 2 advanced fields: basic fields render before the section, advanced inside the collapsed shell, count badge correct, x-advanced on an object property correctly ignored - schema without any x-advanced: output is identical to the pre-change template (whitespace-normalized diff against git HEAD's version) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): Display Settings basic/advanced split + live total-resolution readout The Hardware Configuration card showed ~17 fields at equal priority; a new user only needs 7 of them to get a correctly-sized, correctly-colored image (rows, cols, chain_length, parallel, brightness, hardware_mapping, led_rgb_sequence). The other 10 (multiplexing, panel_type, row_address_type, gpio_slowdown, rp1_rio, scan_mode, pwm_bits, pwm_dither_bits, pwm_lsb_nanoseconds, limit_refresh_rate_hz) now live in a collapsed "Advanced Hardware Settings" section using the same nested-section shell as plugin config forms, so toggleSection() and settings-search auto-expand work unchanged. led_rgb_sequence moved up beside brightness/hardware_mapping (2-col grid became 3-col). No field was removed or renamed; the form still posts the same names to /api/v3/config/main. Also adds a live "Your display: W x H pixels" readout under the four sizing fields (width = cols x chain_length, height = rows x parallel - the exact math the chain-length tooltip describes in prose), recomputed client-side on every input event, no round-trip. Deviation from plan, deliberate: disable_hardware_pulsing / inverse_colors / show_refresh_rate stay in their separate "Display Options" card rather than moving across cards - relocating fields between form sections risks regressions for no decluttering gain in the card users complained about. Validation (real Jinja render): all 17 hardware fields present exactly once, basic fields render before the advanced section and the 10 advanced fields inside it, div count balanced (71/71), readout + recompute script present. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): plugin install auto-enables + persistent restart nudge Getting a plugin onto the display used to take three disconnected manual steps: install from the store, flip its enable toggle, then restart the display service - with no in-UI hint that steps 2 and 3 were needed (only docs/GETTING_STARTED.md mentions it). - installPlugin() now enables the plugin immediately on successful install (owner-confirmed behavior change: always auto-enable, no opt-out; users who don't want it running toggle it off as before), then shows a persistent toast ("... restart the display to show it") with an inline "Restart Now" button wired to the existing restartDisplay() - the same function the three existing Restart Display buttons call. - notification.js: show() accepts optional { actionLabel, onAction } to render one inline action button per toast. Callbacks are stored per notification id and cleaned up on dismiss; a new triggerAction() public method runs the callback and dismisses. The global showNotification() shorthand now forwards a full options object as its second argument (legacy type-string calls unchanged). Scope note: applies to the plugin store's install path (window.installPlugin). The custom-registry install path keeps its existing behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): dismissible Getting Started checklist on Overview New users land on a dense multi-tab dashboard with no suggested order of operations (the only guided flow is the WiFi captive portal). This adds a non-gating checklist card at the top of Overview with five steps, each a deep link that switches to the right tab (and closes the mobile nav drawer): 1. Set panel size -> Display tab (done: rows/cols/chain_length > 0) 2. Set timezone/location -> General tab (done: differs from template defaults America/New_York / Tampa) 3. Install a plugin -> Plugins tab (done: /api/v3/plugins/installed non-empty) 4. Enable a plugin -> Plugins tab (done: any installed plugin enabled) 5. Configure it -> Plugins tab (done: first enabled plugin has >=1 saved value differing from its schema defaults) Steps 1-2 are computed server-side in Jinja from main_config (already in the partial's context); 3-5 client-side from existing endpoints. No new backend state: dismissal persists in localStorage (mirroring the reconciliation banner's sessionStorage pattern one section up); deep links use the same _x_dataStack app-data access as settings-search.js. Disclosed heuristic limit: values left at legitimate defaults (a user actually in Tampa) read as "not done". Validation: real Jinja render across 3 config variants confirms the server-side done-flags flip correctly; div balance intact; /plugins/config response shape (config dict directly in .data) verified against api_v3.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(display): drag-and-drop plugin rotation order for the primary display mode The primary rotation's order was invisible and unconfigurable: modes are registered in parallel-load COMPLETION order, so rotation order actually varied between restarts. Only the niche Vegas Scroll mode had a working order UI. This adds real, persisted ordering end to end: Backend: - config.template.json: new display.plugin_rotation_order (default [], fully backward compatible). - display_controller.py: _apply_plugin_rotation_order() rebuilds available_modes grouped by plugin per the configured list (each plugin's modes keep their declared order; unlisted plugins follow in existing relative order; empty config = exact no-op). Applied at startup after parallel load and after live enable/disable reconcile (before the existing _resync_mode_index_after_change, which preserves the current mode). Mirrors vegas_mode get_ordered_plugins() semantics. - api_v3.py save_main_config: accepts plugin_rotation_order as a JSON array (same parse/guard pattern as vegas_plugin_order). Frontend: - New shared widget static/v3/js/widgets/plugin-order-list.js: the Vegas section's drag-and-drop list factored out verbatim (native HTML5 drag events, saved-order-first rendering, hidden-input JSON sync), parameterized by container/order-input/optional exclude-checkbox/badge. - display.html: Vegas section now calls the shared module; its ~130-line inline copy of the same logic is deleted. - durations.html: new "Rotation Order" card above the durations grid using the same module, posting plugin_rotation_order with the existing form. Deviation from plan, deliberate: durations stay as their own mode-keyed grid rather than inline in the drag rows - verified display_durations keys are MODE names (display_controller.py resolves duration per mode_key), not plugin ids, and one plugin can own several modes, so the planned 1:1 inline pairing was wrong. Validation: py_compile on both Python files; _apply_plugin_rotation_order unit-tested standalone (configured order applied, empty-config no-op, unknown ids skipped - 3/3); both templates render with balanced divs, the hidden input carries the saved order, and the old inline implementation is confirmed gone; config.template.json parses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): serve the interface at / — /v3 kept as a legacy alias The user-visible URL no longer carries the interface version: the pages blueprint is now registered un-prefixed (primary) AND at /v3 (second registration, name='pages_v3_legacy'), so: - http://<device>/ serves the interface directly (the old @app.route('/') redirect is removed — the blueprint's own index takes its place) - every existing /v3/... bookmark and all the hardcoded /v3/partials/... fetches in templates/JS keep working verbatim through the alias mount — zero template/JS churn, zero broken links - url_for('pages_v3.') resolves against the primary registration, so all server-side redirects (captive portal detection endpoints) now emit un-prefixed URLs - the AP-mode captive-portal allowlist learned the un-prefixed page paths (/setup, /partials/, /settings/, /plugin-ui/) so setup-mode requests don't redirect-loop - /api/v3 and the templates/v3, static/v3 directories are deliberately untouched (internal, invisible to users; owner-confirmed scope) Validation: dual registration mechanics tested against real Flask (test client): /, /v3, /v3/ redirect, partials and /setup reachable on both mounts, url_for yields un-prefixed paths; py_compile passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ perf(web): stream the preview PNG raw instead of PIL decode + re-encode display_preview_generator() opened each changed snapshot with PIL and re-encoded it to PNG just to base64 it — but /tmp/led_matrix_preview.png already IS a PNG, written atomically by the display service (tmp file + os.replace in display_manager.py), so a partially-written file can never be observed. Read the bytes and base64 them directly: identical payload (front-end consumes data:image/png;base64 — verified in base.html), one full image decode+encode per frame less on the same Pi that's driving the matrix. The existing mtime skip and viewer-marker throttling are unchanged (they already covered the "skip unchanged frames" concern). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * perf(web): gzip response compression via flask-compress The interface ships a ~5,000-line HTML shell and >20k lines of JS uncompressed; on phone/WiFi that dominates load time. Flask-Compress gzips/brotlis compressible responses transparently. - Optional dependency, same graceful pattern as flask-limiter: missing package = uncompressed responses, no crash. - SSE safety verified empirically against the real package (1.24): an actual streamed text/event-stream response comes back with no Content-Encoding while a large HTML response gzips — the display preview / stats / logs streams are unaffected. - Added flask-compress>=1.14 to web_interface/requirements.txt. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * perf(web): gate verbose console logging behind the existing pluginDebug switch plugins_manager.js, base.html's inline scripts, and app.js emitted 198 console.log calls in production - including per-interaction [DEBUG] dumps - costing main-thread time and drowning real errors in noise. - New window.debugLog() gate defined in base.html's first inline script (before any other script runs): forwards to console.log only when localStorage.pluginDebug === 'true' - the SAME switch plugins_manager.js already used for its _PLUGIN_DEBUG_EARLY logs, so existing debug workflow docs stay valid. Exposed as window.LEDMATRIX_DEBUG for other scripts. - Mechanically rewrote console.log( -> debugLog( in plugins_manager.js (127), base.html (64), app.js (7). Verified no occurrences lived inside string literals before rewriting; console.error/console.warn untouched. - app.js's no-Alpine showNotification fallback restored to console.info - it's a user-facing last resort, not debug output. Both load paths are safe: the gate is the first inline <script> in <head>, and every rewritten file loads deferred after it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * perf(web): vendor CDN assets locally — LAN-speed loads, fully offline-capable Font Awesome, CodeMirror, and htmx were fetched from cdnjs/unpkg on every fresh page load, adding third-party round-trips on a device that often lives on a local network (and htmx was local-only in AP mode, meaning two different loading behaviors to reason about). - Vendored pinned copies under static/v3/vendor/: Font Awesome 6.0.0 (css/all.min.css + the 8 webfonts it references relatively) and CodeMirror 5.65.2 (core, javascript mode, closebrackets/matchbrackets addons, base + monokai css) - ~1.1 MB total, exact versions the CDN tags pinned. - htmx + sse + json-enc extensions now load from the existing local copies (verified 1.9.10, matching the CDN pin) on EVERY network, not just AP mode; the pinned CDN copies remain as a one-shot rescue fallback, mirroring the pattern Alpine already used. The convoluted isAPMode source-flipping logic collapses away. - Dropped the CDN preconnect/dns-prefetch hints (no longer on the critical path). - Fixed a latent bug while relinking CodeMirror: the loader requested mode/json/json.min.js, which does not exist on cdnjs (HTTP 404 verified) - it 404'd on every JSON-editor open. JSON highlighting comes from the javascript mode; the phantom entry is removed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * perf(web): extract 3,850 lines of inline JS from base.html to cacheable static files base.html shipped ~4,200 lines of inline JavaScript inside the HTML document, re-downloaded and re-parsed on every page load (gzip helps the transfer, but inline scripts can never be browser-cached). The four largest blocks - none containing any Jinja syntax, verified by scanning every inline block for {{ }} / {% %} - now live as static files served with the app's existing mtime-versioned immutable caching: - js/htmx-config.js (246 lines): HTMX swap/script-execution config, toggleSection helpers - js/app-early.js (346 lines): early helpers + the app() stub that must precede Alpine init - js/app-shell.js (2,997 lines): SSE wiring + the full Alpine app() implementation and tab logic - js/custom-feeds-helpers.js (262 lines): custom-feeds table helpers Each replacement <script src> is CLASSIC (no defer/async) at the exact position of the inline block it replaces - identical execution timing and DOM visibility to inline scripts, so relative ordering with the deferred scripts and with each other is unchanged. base.html drops from ~4,940 to 1,079 lines. Validation: extraction proven lossless by programmatically reassembling the four files back into the template and comparing against git HEAD - byte-for-byte identical. Jinja parse passes; script open/close tags balanced (53/53, after excluding a literal "<script>" inside an HTML comment). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): size the live preview from the PNG's real dimensions, not config The initial SSE render sized the preview image (and both overlay canvases) from the server-reported config dimensions (cols x chain_length, rows x parallel), while the scale slider's re-render path sized from img.naturalWidth/naturalHeight. Whenever the snapshot PNG's actual size disagrees with the config (stale config, display service not restarted after a hardware change), the initial render stretched the image at a fractional ratio - blurry despite image-rendering: pixelated - and touching the scale slider "fixed" it. Reported live on the devpi test rig. Both paths now size from the loaded image's natural dimensions inside img.onload (which also removes a transient wrong-size flash between src assignment and load). The meta label now reports the true snapshot size. The preview card also gets overflow-x-auto so on narrow screens a wide preview scrolls at its exact pixel-perfect size instead of being squeezed into the viewport (fractional downscaling of pixel art also reads as blur). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): fold Display Options into the advanced dropdown; Vegas above Double-Sided Owner-requested layout refinement of the Display Settings tab: - The "Display Options" card (disable_hardware_pulsing, inverse_colors, show_refresh_rate, use_short_date_format, Dynamic Duration) moves inside the collapsed advanced section, now titled "Advanced Hardware & Display Options (15)". Hidden form fields still submit with the form, and settings search still auto-expands the section on match, so nothing is lost - the tab just leads with the essentials. - The "Vegas Scroll Mode" section moves above "Double-Sided Display". New section order: Hardware (+ advanced dropdown) > Vegas Scroll > Double-Sided > Multi-Display Sync. Validation (real Jinja render): all 23 field names present exactly once, divs balanced (70/70), the five Display Options fields render inside the advanced section's bounds, and section markers confirm the new order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): Getting Started items are manually checkable; card auto-hides when complete Two gaps reported from live testing on the devpi rig: 1. The timezone/location step never showed done for a user whose real timezone IS the shipped default (America/New_York) - the heuristic can only detect difference-from-default, not "user saved this". Clicking an item's checkbox now toggles it done manually (persisted per browser in localStorage), so any heuristic false-negative is one tap to clear. Clicking the item text still deep-links to its tab. 2. The card now hides itself automatically once every step is done (auto-detected or manually checked) - previously it stayed until the X was clicked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * chore(web): CI cleanup — declare debugLog global, fix entity-unescape order, drop v3 from UI branding - Add debugLog to the /* global / headers of the six JS files that call it (defined in base.html's first inline script) — resolves the wall of "'debugLog' is not defined" ESLint errors failing the Codacy check. - Fix the two js/double-escaping CodeQL alerts in app-shell.js: the entity-unescape chains decoded & before </>, so a value containing a pre-escaped "&lt;" wrongly double-decoded to "<". & now decodes last (standard order). Pre-existing bug, made visible when the inline scripts moved into scannable .js files. - Page title / header drop the "- v3" suffix, matching the de-versioned user-facing URL. The remaining 7 CodeQL alerts are pre-existing patterns newly visible to scanning (CodeQL doesn't see inline template JS): 4 github.com/htmx.org URL-substring checks (the htmx ones match error-message text, not URLs — false positives in context) and 1 innerHTML XSS-through-DOM in the GitHub install flow. Triage/fix deferred to a focused follow-up rather than expanding this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ fix(web): add the missing nav tab for the Rotation & Durations page The durations partial (/partials/durations) has existed as a route with no nav tab and no content panel referencing it - an orphaned page. That made the new rotation-order UI unreachable through the interface (caught by the owner testing on the rig; my endpoint-level tests fetched the partial by URL and never noticed the missing entry point). - New "Rotation" tab (fa-rotate icon, verified present in the vendored FA 6.0.0 css) between Display and Backup & Restore, wired exactly like the other tabs (#durations-content + hx-get + loadtab; loadTabContent() is fully generic, so no JS changes needed). - Page heading updated from "Display Durations" to "Rotation & Durations" to match its content since the rotation-order card landed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): Rotation & Durations page lists every enabled plugin's screens The durations grid looped over display.display_durations, which nothing has ever populated (verified {} on a real production install) - so the page rendered no duration fields at all. Worse, its inputs posted bare mode names, which save_main_config's endswith('_duration') filter silently dropped: the page was broken in both directions, unnoticed because it was also unreachable (previous commit). - pages_v3._load_durations_partial now builds one entry per display mode of every ENABLED plugin via plugin_manager.get_plugin_display_modes() (falling back to the plugin id), overlaid with saved values, defaulting to the display controller's 30s. Grouped per plugin, sorted by name. Saved keys not owned by any enabled plugin stay visible under "Other saved entries" instead of vanishing. - durations.html renders the grouped inputs, named duration__<mode_key> (mode keys are arbitrary, so they can't use the _duration suffix convention), with an explanatory empty state when no plugins are enabled. - api_v3.save_main_config accepts the new duration__<mode> fields and writes them into display.display_durations under the bare mode key - exactly what the display controller reads (display_durations.get(mode_key, 30)). Validation: py_compile both blueprints; Jinja render with 3 groups asserts grouped inputs, saved-value overlay, stale-entry group, empty state, and div balance. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ feat(web): restart-pending banner, unsaved-changes guard, installable web app Three usability improvements from live testing feedback: - Restart-pending banner: every successful POST to /api/v3/config/main (display hardware, rotation/durations, general settings) now raises a persistent banner - "Configuration saved, restart the display to apply" - with a Restart Now button that posts restart_display_service directly. Backed by sessionStorage so it survives tab switches and reloads until restarted or dismissed. Plugin config saves are deliberately excluded: they apply live via the display process's config watcher. - Unsaved-changes guard: plugin config panels are Alpine x-if templates, so navigating away destroys the panel and revisiting re-fetches it - edits were silently discarded. Forms now mark themselves dirty on input (cleared on successful submit), a capture-phase click handler confirms before a lossy tab switch, and beforeunload guards full page unloads. System tabs (x-show, persistent DOM) are exempt - no false prompts. - Installable web app: manifest.json (standalone display, dark theme) + generated LED-grid icons (192/512 maskable + 180 apple-touch), linked from base.html. "Add to Home Screen" now yields an app-like fullscreen experience; no service worker, so zero behavioral risk. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * test(web): smoke tests + static-analysis audits for the web UI Guardrails so this branch's fix classes can't regress silently: - test_web_smoke.py (24 tests): boots the pages blueprint with the same dual registration app.py uses and asserts every page/partial returns 200 with its load-bearing markers (nav wiring, getting-started card, advanced section, rotation order card, per-mode duration inputs), the /v3 legacy alias serves everything, all critical static assets (incl. vendored fontawesome/codemirror, PWA manifest/icons) are served, durations group per plugin with the leftover bucket, and the advanced-hardware section really contains the tuning fields. Would have caught this session's unreachable-durations-page and orphaned-tab bugs instantly. - test_web_static_audit.py (3 tests): (1) every responsive utility class referenced in templates is actually defined in app.css - the silently-no-op class bug that left the header search box invisible at every width; (2) every url_for('static', ...) reference points to a real file; (3) any JS file calling the debugLog global declares it in a /* global / header. All 40 web tests pass (24 + 3 new, 13 existing) under pytest + Flask. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ feat(web): floating live preview + per-plugin "Preview on display", drawer a11y Preview-while-configuring: - Floating mini preview (fixed, bottom-right) available on every tab except Overview, fed from the same SSE display stream by updateDisplayPreview - no new connections. Collapses to a round toggle button; open/closed state persists in localStorage; hides on Overview where the full preview lives. - "Preview on display" button on every plugin config page header: runs that plugin on the real display for 60 seconds via the existing /display/on-demand/start API and opens the floating preview, closing the configure -> see-the-result loop. Drawer/nav accessibility: - aria-current="page" tracks the active tab (system + dynamic plugin tabs, matched via their Alpine @click expression), updated from the activeTab watcher so search deep-links and checklist navigation are covered too. - Escape closes the mobile drawer and returns focus to the hamburger; opening the drawer moves focus to its first tab. Validation: all 40 web tests pass; Jinja parse + div balance on both touched templates. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): resolve Codacy findings — DOM building over innerHTML, Map callbacks, misc lint Verified each of the 27 reported findings against current code; all fixed except one rule class skipped with reason below. - app-early.js: plugin tab buttons are now built with createElement/ createTextNode instead of innerHTML template strings (icon class and name come from plugin manifests - semi-trusted input; the old code escaped the name but interpolated the icon class). Both construction sites. Also the forEach arrow no longer returns tab.remove()'s value. - plugin-order-list.js: rows, empty state, and error state all built with DOM APIs - the file no longer contains innerHTML at all (the now-unneeded escapeHtml/escapeAttr helpers are removed); MODE_LABELS is a Map so the vegas-mode lookup can't hit prototype properties. - notification.js: actionCallbacks is a Map (get/set/delete) instead of a plain object - resolves the object-injection-sink and dynamic-delete findings; triggerAction also type-checks the callback. - htmx-config.js: unused catch binding dropped; var -> const in the afterSettle handler; the swapped-<script> re-execution reads/writes textContent instead of innerHTML; the diagnostic form payload uses a null-prototype object so a field named __proto__ can't pollute. - custom-feeds-helpers.js DELETED (with its script tag): all three of its functions (addCustomFeedRow, removeCustomFeedRow, handleCustomFeedLogoUpload) are shadowed by the deferred widgets/custom-feeds.js window assignments, which always win at call time - the copies were dead even when they lived inline in base.html. This also resolves the unused-function and unused-variable findings there. Skipped: 4x "Non-serializable expression must be wrapped with $(...)" in app-early.js - that rule targets code crossing a browser-automation serialization boundary (e.g. page.evaluate); these are ordinary arrow functions in plain browser code with no such boundary. Validation: all 40 web tests pass (incl. the static-asset reference audit, which confirms no template still points at the deleted file); Jinja parse OK. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * feat(web): update flow installs changed Python dependencies automatically The in-app updater (Update Now banner -> git_pull action) stashed, pulled, and purged plugins - but never touched Python dependencies. Any release adding a package (e.g. this branch's flask-compress, which lives in web_interface/requirements.txt) silently required an SSH session and a manual pip install that most users will never do. - git_pull now records HEAD before pulling; after a successful pull it diffs old..new and, if requirements.txt or web_interface/requirements.txt changed, installs exactly those via _pip_install_requirements - the same vetted root-visible sudo path the Tools-tab buttons use (with its existing graceful fallback when the sudo wrapper isn't configured). Results are appended to the update toast; a failure points the user at the Tools-tab button instead of failing the whole update. - install_base_requirements (Tools tab) now also installs web_interface/requirements.txt - previously it only covered the root file, so web-only dependencies were unreachable from the UI entirely. No install happens when the pull was already-up-to-date or when no requirements file changed, so routine updates stay fast. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): address CodeRabbit review — validation, a11y, perf, and privacy fixes Verified each finding against current code. Fixed: - api_v3: plugin_rotation_order is now strictly validated (JSON list of strings, 400 with a descriptive message otherwise) and popped from the payload before any further handling. - display_controller: _apply_plugin_rotation_order defensively ignores a non-list value (keeps the existing rotation, logs a warning) and drops non-string entries; new logs carry the [DisplayController] prefix. Unit-tested both defensive paths. - app.py: snapshot-read handler narrowed to OSError with debug logging; flask-compress ImportError now emits one structured warning with the install remedy. - htmx-config: the response-error logger prints form FIELD NAMES only - values (API keys, passwords) never reach the console. - plugin-order-list: saved order/exclusions normalized with Array.isArray (a saved "null" previously crashed .forEach); each row gained keyboard/touch-accessible move-up/move-down buttons (HTML5 drag events don't fire on most mobile browsers) that reorder and syncInputs() immediately alongside native drag. - app-shell: window.installedPlugins setter always takes the new list (same-ID metadata/enabled updates were silently dropped); tab rebuild stays gated on ID changes. LED dot renderer reads the frame with ONE getImageData call instead of one per pixel (~9,200/frame at 192x48). - plugins_manager: togglePlugin returns its request promise resolving the API outcome; the install flow now shows the "installed and enabled" toast (with Restart Now) only after enablement succeeds, and a warning without a restart offer when it fails. - a11y: hamburger aria-label flips Open/Close with drawer state; both Advanced-section toggle buttons declare aria-controls/aria-expanded and the shared toggleSection() keeps aria-expanded in sync; move buttons have per-plugin aria-labels. - Rotation/Vegas order-list bootstraps cap their retries (~5s) and show a reload hint instead of spinning forever; Alpine app-state lookups prefer [x-data="app()"] with a generic fallback. Skipped, with reasons: - executePluginAction arg order: caller (plugin_config.html) already passes (actionId, index, pluginId) matching the signature exactly. - generateFieldHtml XSS, entity-unescape blocks, dotToNested pollution, and "app.loadInstalledPlugins" in app-shell: all inside the legacy client-side config cluster whose entry points are shadowed by plugins_manager.js / replaced by server-rendered forms (zero live callers, verified) - queued for wholesale deletion in the follow-up rather than patching dead code. - custom-feeds-helpers.js findings (3): file was deleted in a prior commit. - console.error/warn override removal and afterSwap script re-execution removal: deliberate pre-existing workarounds every partial's inline init currently depends on; reworking them safely needs isolated testing (follow-up), and the error suppression is already double-gated (insertBefore AND htmx match). - "move durations bootstrap into a bundle": inline partial-scoped init is the established pattern for HTMX partials in this codebase. Validation: all 40 web tests pass; py_compile on all touched Python; all touched templates parse; rotation-order defensive paths unit-tested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * chore(web): fix remaining real Codacy findings (2 of 6) - htmx-config.js: two more unused catch bindings dropped (optional catch binding), matching the earlier fix. - app-early.js: second forEach arrow (the stub updatePluginTabs copy) braced so the callback no longer returns tab.remove()'s value. The other 4 findings ("Non-serializable expression must be wrapped with $(...)") are deliberately NOT "fixed": that rule belongs to a browser-automation (WebdriverIO-style) lint context and is misfiring on ordinary arrow-function constants. Converting them to function declarations would look compliant but BREAK the code - all four arrows intentionally capture the enclosing Alpine component's `this` for the stub-to-full enhancement logic. The right remedy is disabling that pattern for this repo in Codacy's Code Patterns settings (or dismissing the four findings), not a code change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): floating preview shows a frame immediately on open + is resizable The floating preview opened empty and stayed empty until the display next CHANGED - the SSE stream only pushes frames on change, and the panel only consumed frames while already open, so the connection's initial frame (sent while the panel was closed) was dropped. Reported from mobile testing as "the button doesn't work". - updateDisplayPreview now caches the latest frame globally regardless of panel state; opening the panel populates the image from that cache instantly, then live frames take over. - Resizable: a size button cycles 192/256/384/512px presets (persisted per browser; works on touch), and desktop additionally gets a native drag handle (CSS resize: both). The image is fluid within the panel; on phones the panel is capped to the viewport width. The size icon (fa-up-right-and-down-left-from-center) is verified present in the vendored FA 6.0.0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): stop duration fields leaking into config root; isolate per-file dependency installs; fix test fixture leak Verified each finding against current code. - api_v3 save_main_config: both duration blocks (_duration suffix fields and the newer duration__<mode> fields) only READ from `data`, never removed the keys. The generic "remaining keys" merge later in the same function has no skip-list entry for either pattern, so every duration field was ALSO written a second time as a bogus top-level config key (e.g. "clock_duration": 30 and "duration__mlb_live": 42 sitting at config root, alongside the correct nested display.display_durations.<key>). Confirmed by tracing the full function. Fixed by popping each handled key from `data` (same pattern already used for plugin_rotation_order) and validating strictly: a non-integer duration now returns 400 with a message naming the offending field/mode instead of silently logging and moving on (for the _duration fields, which previously had zero validation at all). - api_v3 dependency-install loops (git_pull's post-update sync and install_base_requirements): _pip_install_requirements can raise subprocess.TimeoutExpired or OSError (confirmed: install_requirements_file in permission_utils.py never catches either internally, despite its docstring's "never raises on non-zero exit" only covering return codes). Both loops previously let one file's exception either abort the whole try block (skipping the second requirements file entirely) or propagate uncaught. Each file's install is now in its own try/except, so a timeout or OSError on one file is recorded as a labeled failure and the loop continues to the next file. - test_web_smoke.py: the `client` fixture mutated the module-level pages_v3 Blueprint singleton's config_manager/plugin_manager directly with no teardown - since pages_v3 is shared across the whole pytest process (test_web_settings_ui.py touches the same attributes), this fixture's mocks could leak into whichever test ran next. Now saves the originals, yields the client, and restores them in a finally block. Validation: py_compile passes; all 40 web tests pass with the now-generator fixture. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): repair garbled Advanced Hardware section description An earlier sed-based text update concatenated the old and new copies of this description instead of replacing one with the other, leaving a duplicated sentence with the — entity broken into ".mdash;" (visible as literal "mdash;" text on the page). Restored to one clean sentence. Other findings from this review were already fixed in a prior commit (installedPlugins setter) or are confirmed dead code with zero live callers (executePluginAction/dotToNested/entity-unescape/generateFieldHtml, all reachable only from the two unused savePluginConfig copies in app-shell.js - grepped every template, no references) - same legacy cluster flagged in earlier review passes, still queued for a dedicated deletion follow-up rather than patched in place here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): remove redundant htmx:afterSwap script re-execution (was double-executing every partial's inline script) Re-verified this CodeRabbit finding, previously deferred as "needs isolated testing" - traced it to a confirmed, active bug rather than a style concern: htmx 1.9.10's own config defaults to allowScriptTags: true (confirmed in the vendored htmx.min.js, which itself contains the same clone-and-reinsert <script> mechanism internally). This means htmx ALREADY re-executes every <script> tag in swapped content by default, exactly like a browser navigating to a new page. The custom htmx:afterSwap listener in htmx-config.js did the identical clone-and-reinsert a SECOND time on top of htmx's own handling - so every inline <script> block in every HTMX-loaded partial (overview, display, durations, plugin config, etc. - most partials have one) executed twice per load. Confirmed safe to delete outright, not just narrow: grepped every hand-written JS file for a manual `dispatchEvent(... 'htmx:afterSwap' ...)` that might have relied on this handler for a non-htmx code path (e.g. the direct-fetch fallbacks like loadOverviewDirect) - none exists, so nothing depended on this listener specifically; htmx's native handling covers every real htmx-driven swap on its own. Left in place, unchanged: the console.error/console.warn global override a few lines up in the same file, which suppresses known-noisy HTMX-timing-race messages. That one is a legitimate anti-pattern too (broad substring matching can mask unrelated errors) but redesigning it needs care to preserve real diagnostics while still hiding the specific harmless races it targets - a scoped follow-up, not a same-day deletion like this confirmed-duplicate handler. Validation: all 27 fast web tests pass; JS brace/paren balance sanity checked (no local Node/browser available in this sandbox to execute the file directly - verify manually in-browser before merge). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * chore(display): add missing [DisplayController] prefix to the reconcile-complete log Re-verifying the full CodeRabbit findings list against current code surfaced one still-open item: the nitpick asked for the prefix on BOTH rotation-related log lines, but only "Applied plugin rotation order" got it in the earlier pass - "Plugin reconcile complete" was missed. No message/argument/level change, matching the finding's own scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(web): repair dead /v3/logs link on the display hardware-error banner The "Logs tab" link in the display-settings simulation-mode banner was a real <a href> to /v3/logs, but no such route has ever existed (log content is loaded client-side via activeTab, not a dedicated page route) - the link 404'd regardless of the /v3 prefix change. Switch it to the same activeTab-switching pattern the real nav uses. * fix(web): raise display Rows field max from 64 to 128 Cols already allowed up to 128; Rows was capped at 64, which rejects valid larger panel configurations (e.g. 128-row tile chains). No server-side schema enforces a rows max, so this was purely an overly-strict HTML input attribute. * fix(web): remove redundant htmx.org substring check flagged by CodeQL CodeQL flags .includes('htmx.org') as "incomplete URL substring sanitization" - a false positive here, since this string is only ever matched against console.error/warn message text to decide whether to suppress a known-harmless HTMX timing-race log line, not used for any URL-trust/redirect decision. The check was also redundant: 'htmx' is already a substring of 'htmx.org', so the plain .includes('htmx') check right next to it already covers every case the removed check did. * fix(web): restore htmx script re-execution timing that Alpine x-data depends on Removing the custom htmx:afterSwap script-reexecution handler (in a prior commit, as a "duplicate execution" cleanup) broke every partial whose Alpine x-data component function is defined by an inline <script> in that same partial (e.g. wifi.html's wifiSetup()) - confirmed live via browser console: "Alpine Expression Error: wifiSetup is not defined" on every field in the WiFi tab. Root cause: htmx's own native script execution runs during its "settle" phase (~20ms after swap, per htmx's own defaultSettleDelay), but Alpine's MutationObserver evaluates x-data on newly-inserted elements synchronously, right as the swap lands - before settle. So the inline <script> defining wifiSetup() was still un-run when Alpine tried to call it, and Alpine does not retry a failed x-data evaluation later once the function does become defined. Fix: re-execute swapped <script> tags ourselves on htmx:afterSwap (which fires synchronously, before settle, beating Alpine's observer), and disable htmx's own native script re-execution (htmx.config.allowScriptTags = false) so the same script doesn't also run a second time during settle - restoring correct timing without reintroducing the original double-execution bug. Also in this commit: - fix XSS: unescaped repoUrl in a title attribute in renderSavedRepositories - replace .includes('github.com') substring checks with real URL hostname validation (CodeQL: incomplete URL substring sanitization) * fix(web): wait for async plugin install to finish before auto-enabling it Confirmed live: installing hockey-scoreboard logged "installation queued" (success) immediately followed by "enabling it failed" with a 404 "Plugin not found" from /api/v3/plugins/toggle. /api/v3/plugins/install runs the actual clone + plugin-manager discovery asynchronously via an operation queue when one is configured - the response installPlugin() was checking only means the operation was queued, not that the plugin is installed yet. Calling togglePlugin() right after that response 404s because plugin_manager hasn't discovered the new plugin. Fix: reuse the same operation-polling mechanism uninstallPlugin() already has (generalized pollOperationStatus() to take onComplete/onFailed/onTimeout callbacks instead of hardcoding uninstall behavior) so installPlugin() waits for the operation to actually complete before enabling it. Falls back to enabling immediately when no operation_id is returned (direct/synchronous install path, no queue configured). * fix(web): resolve remaining valid findings from latest review pass - custom-feeds.js: fix asset-upload contract mismatch (field name "file" -> "files", response read from top-level "uploaded_files" not "data.files") - same bug already fixed in this file on a separate branch/PR (#420), which this branch never received since they're independent PRs off main - custom-feeds.js: add aria-label to the two icon-only "remove feed" buttons - custom-feeds.js: move file-input reset into .finally() so a failed upload doesn't leave the input stuck holding the file, blocking retry of the same file - app-shell.js: fix executePluginAction(pluginId, actionId) parameter order/count mismatch vs. its callers' (actionId, index, pluginId) - currently masked by plugins_manager.js's correct version overwriting this one at load time (classic vs. deferred script order), but worth fixing outright since it's an isolated, self-contained reassignment (not inside the Alpine app() object literal) and removes a latent footgun - overview.html: align Alpine-state resolution with settings-search.js's two-tier getAppData() (also check appEl.__x.$data, not just _x_dataStack) Verified already addressed by earlier passes (no change needed): plugin_rotation_order validation, DisplayController log prefixes, togglePlugin returning its promise for install-flow chaining, installedPlugins setter always updating state, mobile-nav aria-label, toggleSection aria-expanded sync, PluginOrderList bounded init retries (both display.html and durations.html), plugin-order-list.js Array.isArray validation, batched getImageData in the LED-dot preview renderer, app.py exception narrowing/ logging, form-submission log redaction. Confirmed dead code, skipped (unreachable - zero template/JS callers, verified via full-repo grep): dotToNested prototype-pollution hardening, generateFieldHtml HTML-injection hardening, and the HTML-entity-unescape block in JSON parsing - all three live only inside app-shell.js's two legacy savePluginConfig implementations (one Alpine-method, one standalone), neither of which any template or script calls. The real, live plugin-config path is server-rendered via GET /partials/plugin-config/<id>. Explicitly NOT reverted: the htmx:afterSwap script-execution listener. An earlier finding batch asked to remove it as "duplicate" htmx behavior; that was tried and reverted this session after live testing on hardware proved it broke every partial whose Alpine x-data depends on an inline <script> in the same partial (confirmed: WiFi tab hard-failed with "wifiSetup is not defined"). Removing it again would reintroduce that regression. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-16 20:32:07 -04:00
Chuck GitHub Claude	6edd80d9f3	fix(schedule): stop stray 'days' data from overriding Global schedule (#399 ) save_schedule_config never persisted the schedule's 'mode' field, and _check_schedule inferred per-day vs global purely from whether a 'days' dict was present for the current day. Config migration (_merge_template_defaults) re-adds the template's 'schedule.days' (all days disabled by default) whenever it's missing from the user's saved config - which is exactly the case after saving Global mode, since that save path intentionally pops 'days'. The result: a user on Global mode would get their schedule silently reinterpreted as per-day, with today's day disabled, blanking the display. Persist 'mode' on save and have _check_schedule honor it explicitly (mirroring how _check_dim_schedule already does), so a resurrected 'days' dict can't override an explicit Global selection. Falls back to the old inference behavior only when no 'mode' is recorded. Co-authored-by: Claude <noreply@anthropic.com>	2026-07-12 10:53:33 -04:00
Chuck GitHub Claude Opus 4.8	bd9f461f70	Add system diagnostics, power controls, and WiFi radio toggle to Tools tab (#389 ) * Add system diagnostics, power controls, and WiFi radio toggle to Tools tab Expands the web UI Tools tab with safe, purpose-built controls so users can manage the Pi without SSHing in, instead of an arbitrary-command terminal (the web UI has no auth and CSRF is disabled, so a shell would be unsafe). - System Diagnostics card: renders the existing but previously-unused GET /api/v3/system/status endpoint (CPU, memory, temp, disk, uptime), with a manual refresh and a 10s poll. - System Power section: reboot/shutdown buttons wired to the existing reboot_system / shutdown_system actions, behind a confirm step, with a dedicated powerAction() helper that treats the dropped connection as the expected "going offline" outcome rather than an error. - Network Radio section: WiFi on/off toggle backed by new GET/POST /api/v3/wifi/radio endpoints and WiFiManager.set_wifi_radio() / get_wifi_radio_state(). Disabling WiFi is refused unless a wired connection is present (reusing the existing lockout guards), with an explicit force-off confirmation for advanced users. No new privileged commands: uses nmcli radio wifi on\|off (already sudo-allowlisted) and the existing reboot/poweroff grants, so the sudoers-alignment guard test stays green. Bluetooth toggle intentionally omitted since the installer removes the BlueZ stack for LED timing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE * Harden WiFi radio endpoint and diagnostics poll (review feedback) Addresses code review feedback on the Tools tab additions: - api_v3.py: parse the `enabled` POST field with the same string-aware coercion as `force`. A plain bool() cast turned {"enabled":"false"} into True (enabling instead of disabling) for any non-UI API caller. - wifi_manager.set_wifi_radio() now returns a reason code alongside (success, message); the /wifi/radio error response includes it. The Tools UI only shows the force-off confirmation when reason == 'no_ethernet', so a genuine nmcli failure surfaces its real error instead of a misleading "no wired connection" prompt that would just retry into the same failure. - tools.html: gate the 10s diagnostics poll on panel visibility (document.hidden / offsetParent), so switching to another tab stops the recurring /api/v3/system/status calls instead of churning the Pi off-screen. The initial load and manual Refresh remain unconditional. Verified: {"enabled":"false"} now disables (refused w/ reason:no_ethernet), {"enabled":"true"} enables; inline JS passes node --check; py_compile clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE * Narrow WiFi-disable fallback except and add stack trace (review) Addresses a CodeRabbit nitpick: the fallback handler in set_wifi_radio()'s disable path caught bare Exception and logged without a traceback. Narrow it to (OSError, subprocess.SubprocessError) — the errors subprocess.run realistically raises — and log with exc_info=True for full context on the Pi. Anything genuinely unexpected now propagates to the endpoint's outer handler (500), matching the codebase's specific-exception convention. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-09 09:22:11 -04:00
Chuck GitHub Claude Opus 4.8	3b93024993	feat: activate dormant plugin health/metrics subsystem and surface it in the web UI (#388 ) * feat(plugin-system): activate dormant plugin health & metrics subsystem PluginManager shipped a fully-built health tracker, resource monitor and circuit breaker that were never instantiated (health_tracker/resource_monitor were left as None), so the circuit breaker never engaged and the existing health/metrics API routes always returned "not available". - DisplayController now wires a PluginHealthTracker and PluginResourceMonitor onto the plugin manager, enabling the circuit breaker (a repeatedly-failing plugin's update() is skipped after consecutive failures, then retried after a cooldown) and per-plugin execution-time metrics. Both persist to the shared cache. - load_plugin() now validates each plugin's config against its JSON schema in a strictly warn/degrade-only way: a violation logs a warning and flags the plugin degraded in the health tracker, but never changes whether the plugin loads or its pass/fail behaviour. Adds PluginHealthTracker.set_degraded(), which never touches the circuit breaker. - ResourceMonitor CPU/memory sampling now reuses a cached psutil.Process and reads cpu_percent(interval=None), so monitoring no longer blocks ~100ms per call on the display loop's update path. - Fix DiskCache.get() raising TypeError for max_age=None ("never expires"), which silently discarded persisted plugin health/metrics on read and thus broke cross-process and post-restart surfacing. - Fix two dead PluginManager helpers that called non-existent tracker methods. Tests: new test_resource_monitor, test_plugin_health, test_plugin_manager_schema_soft; extended test_cache_manager and test_display_controller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq * feat(web-ui): surface plugin health, metrics and load state With the health/metrics subsystem now active in the display service, expose it in the web UI (which runs as a separate process from the display loop): - Wire a health tracker / resource monitor backed by the shared on-disk cache into the web process so /api/v3/plugins/health and /plugins/metrics read the data the display service persists. - Build those route responses per installed plugin id (the tracker's in-memory view is empty in a fresh web process) so cross-process data is included. - Add state + error_info to /plugins/installed entries so the UI can show why a plugin isn't running instead of just loaded:false. - Add a "Plugin Health" panel to the Tools page (circuit status, avg/max update time, update count, last error) plus PluginAPI.getPluginMetrics(). Tests: route-level tests for the health/metrics endpoints in test_web_api. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq * fix(plugin-metrics): refresh cross-process health/metrics reads; type hints Addresses CodeRabbit review on #388: - Major: the web process's health/resource trackers cached the first persisted read in an in-memory dict (and the CacheManager memory tier held max_age=None entries indefinitely), so a long-lived web process showed the first snapshot and never reflected the display service's later updates. Add an opt-in force_reload path (get_health_summary/get_health_state/_load_health_state and get_metrics_summary/get_metrics) that bypasses the in-memory copy and, via a new memory_ttl passthrough on CacheManager.get, the cache manager's memory tier — so each /plugins/health and /plugins/metrics poll reads fresh persisted state. Default behaviour (force_reload=False) is unchanged for the display process and existing callers. - Minor: DiskCache.get type hint is now Optional[int] with the None ("never expires") semantics documented, matching MemoryCache.get. Tests: new force_reload staleness cases in test_plugin_health and test_resource_monitor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-09 07:54:18 -04:00
Chuck GitHub Claude Sonnet 5	63a233f3ed	fix: dependency installation gaps in Plugin Store and first-time install (#385 ) * fix: install plugin dependencies through root-visible installer in Plugin Store install_plugin/update_plugin (store_manager.py) installed requirements.txt with a bare `pip3` off PATH, bypassing the root-visible installer added in #380 for the "Reinstall Plugin Deps" button. Two bugs stacked: (1) `pip3` can resolve to a different Python install than the one that actually runs ledmatrix.service, and (2) even when it resolves correctly, ledmatrix-web runs as a non-root user so the package lands in that user's local site-packages, invisible to root-run ledmatrix.service. Either way the install reports success and writes the .dependencies_installed hash marker, so plugin_loader's own (correct) install-on-load path skips reinstalling — leaving the dependency permanently missing until a user finds and clicks the separate "Reinstall Plugin Deps" tool. This is why users kept hitting "No module named 'astral'" for the weather plugin even after installing it from the Store. Extracts the sudo-wrapper-then-fallback install logic from api_v3.py's _pip_install_requirements into src/common/permission_utils.py as install_requirements_file, and routes store_manager.py's dependency installation through it so the automatic Store install/update path now matches the manual "Reinstall Plugin Deps" path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: suppress Codacy false-positive on subprocess.run in install_requirements_file Codacy's generic subprocess-security rule (Bandit B603 equivalent) flagged the pip/sudo subprocess.run calls in install_requirements_file for lacking a "static string argument" — the standard pattern-based flag for any subprocess.run() call with a variable in its argv list. Both calls use list-form argv (no shell=True, so no shell-injection surface), and the only dynamic value is req_file, a Path built internally by callers rather than raw external input; safe_pip_install.sh independently re-validates it before installing anything as root. Suppresses with inline `# nosec B603` comments matching this codebase's existing convention (see permission_utils.py's own PROTECTED_SYSTEM_DIRECTORIES, display_manager.py, sync_manager.py, etc.). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: first-time install script fails on apt-managed requests package web_interface/requirements.txt and requirements.txt both pin requests>=2.33.0,<3.0.0, but Raspberry Pi OS ships an apt-managed python3-requests with no pip RECORD file. Upgrading it via plain `pip install` aborts with "uninstall-no-record-file" because pip refuses to uninstall a package it has no record of, in place — which is exactly the "Some web interface dependencies failed to install" warning first-time install hits. scripts/install_dependencies_apt.py and scripts/fix_perms/safe_pip_install.sh already work around this with --ignore-installed (lets pip lay the new version down in /usr/local, shadowing the apt copy, instead of trying to remove it first). first_time_install.sh's own direct pip invocations — the per-package requirements.txt loop, the web_interface/requirements.txt install, and the requirements_web_v2.txt fallback — didn't have it. Adds --ignore-installed to all three so first-time install no longer fails on this well-known apt/pip conflict. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: add nosemgrep to subprocess.run calls Codacy still flagged The prior # nosec B603 comments suppressed Bandit's check but Codacy's semgrep-based rule ("subprocess function 'run' without a static string") kept flagging the same two lines as a critical security issue even after that fix landed. install_dependencies_apt.py's _run() already needed both tags together (# nosec B603 B607 ... # nosemgrep) for the identical subprocess.run pattern, so apply the same double suppression here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: add --ignore-installed to install_requirements_file fallback path CodeRabbit review caught this (confirming a gap already flagged in conversation): the non-sudo fallback pip install in install_requirements_file was missing --ignore-installed, unlike the sudo-wrapper branch and safe_pip_install.sh. Without it, the same apt/pip RECORD-file conflict this PR fixes elsewhere (first_time_install.sh, install_dependencies_apt.py) could still hit installs that fall back to this path (e.g. a plugin's requirements.txt on a host where safe_pip_install.sh isn't set up yet). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-08 09:59:12 -04:00
Chuck GitHub Claude Sonnet 5	cbb8ec41e8	Fix: plugin/base requirements installed as web-user, invisible to root-run display service (#380 ) * fix: install plugin/base requirements as root so ledmatrix.service can see them ledmatrix-web.service runs as a non-root user, so "Reinstall plugin requirements" installed packages into that user's ~/.local site-packages. ledmatrix.service (the actual display, which loads and runs plugin code) runs as root and can't see another user's user-site packages, so plugins with dependencies not already present system-wide would silently fail at runtime with ModuleNotFoundError even after a "successful" reinstall. Reproduced and fixed live against a real device (weather plugin's astral dependency, used for moon-phase data): confirmed the exact failure ("No module named 'astral'" on every almanac cycle) and confirmed it's gone after this fix. Adds scripts/fix_perms/safe_pip_install.sh, a root-owned wrapper (mirroring the existing safe_plugin_rm.sh pattern) that validates the target is requirements.txt at the project root or under plugin-repos/ or plugins/ before running pip install as root. configure_web_sudo.sh provisions a narrowly-scoped sudoers rule for it. api_v3.py's install_base_requirements and install_plugin_requirements actions now use it via `sudo -n`, falling back to today's current-user-only install (with an explanatory note) if the wrapper isn't set up yet, so existing installs don't regress. Also uses --ignore-installed in the wrapper: root's site-packages often has apt/dpkg-managed copies of common libraries (requests, etc.) with no pip RECORD file, which pip refuses to upgrade in place and aborts the entire requirements.txt install over — discovered this while testing the fix live, since a plugin's other already-satisfied-for-the-web-user dependencies had never actually been attempted as root before. Also fixes a pre-existing bug in configure_web_sudo.sh where the display_controller.py/start_display.sh/stop_display.sh sudoers entries used PROJECT_DIR (scripts/install/, where this script lives) instead of PROJECT_ROOT (where those files actually live) — visible as the script's own "File access test" self-check failing. Verified fixed live. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix: invoke safe_pip_install.sh via explicit bash, matching sudoers rule CodeRabbit caught this on review: the sudoers rule configure_web_sudo.sh provisions is scoped to "$BASH_PATH $SAFE_PIP_INSTALL_PATH " (matching the existing safe_plugin_rm.sh precedent in src/common/permission_utils.py), but _pip_install_requirements() called `sudo -n <wrapper> <req_file>` directly, relying on the script's shebang instead of an explicit bash prefix. sudo matches the literal command line, so this never matched the allowlisted rule on an install with only the specific sudoers entries this script provisions — it silently fell back to the non-root install path every time, which is the exact bug this PR set out to fix. This wasn't caught by live testing on ledpi.local because that device also has a broader, non-standard "NOPASSWD: ALL" grant which masked the mismatch. Confirmed the fix is correct by reading sudo's documented command-matching semantics and mirroring the already-proven-working bash-prefix pattern from permission_utils.py's safe_plugin_rm.sh call exactly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ fix: harden safe_pip_install.sh invocation against bash-path drift + address CodeRabbit nitpicks CodeRabbit follow-up findings on the bash-prefix fix (`7558aaab`): 1. (Actionable) shutil.which('bash') at runtime could in principle resolve to a different absolute path than configure_web_sudo.sh's `command -v bash`, which is resolved once at setup time and baked into the static sudoers file as a literal string — sudo requires an exact match. Now tries /usr/bin/bash and /bin/bash (the standard Debian/Raspberry Pi OS locations, matching what the setup script virtually always produces) before falling back to this process's own PATH resolution, so a divergence in just one of them doesn't break the install. 2. (Nitpick) Any nonzero returncode was treated as "sudo denied", so a real pip failure (bad package, build error) would trigger a pointless duplicate non-root install attempt and a misleading error message. Now distinguishes "sudo -n rejected this exact command line" from "sudo ran it but the command itself failed" via sudo's own diagnostic text, and surfaces genuine failures immediately without retrying other bash candidates or falling back. 3. (Nitpick) Added structured logging for every fallback/failure path (wrapper missing, sudo denied, real install failure), previously only visible via the returned stdout note — needed for remote debugging on a headless Pi. Verified: function-level smoke test confirms a real failure (this sandbox's system python3 lacking pip) is now correctly classified as non-denial and returned immediately without retrying candidates or double-installing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-06 08:04:49 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	639e1c3a93	fix(web): repair news ticker custom-feeds save for JSON path (#376 ) * feat(install): surface root cause of web dependency install failures install_dependencies_apt.py previously reported only which packages failed, not why - the actual apt/pip error was discarded (apt) or could scroll out of the on_error log tail (pip), leaving "Step 7: Install web interface dependencies (line 915)" as the only visible detail. Capture command output for each install attempt and print a compact DEPENDENCY INSTALLATION FAILURES summary with the last lines of error output per package. Also run the installer with `python3 -u` for real-time, correctly-ordered logging, and widen the on_error tail from 50 to 100 lines so the summary isn't cut off. * fix(web): repair news ticker custom-feeds save for JSON path The JS dotToNested() helper converts indexed form fields like feeds.custom_feeds.0.name into a dict {'0': {name:...}} rather than a proper array. The form-data path already had fix_array_structures() to convert those dicts back to arrays before schema validation, but the JSON path (used by all web-UI saves) never ran that fix, so saving any custom feed produced a schema validation error: "Expected type array, got object". Add _fix_json_arrays() immediately after schema loading on the JSON path, mirroring the existing fix_array_structures() logic. Also fix custom-feeds.js getValue() to omit the logo key entirely when no logo is present instead of returning logo:null, which would fail schema validation (logo expects type object). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(install,tools): address PR 376 review findings - first_time_install.sh: add _clone_rpi_rgb() wrapper so retry() cleans up any partial rpi-rgb-led-matrix-master dir before each clone attempt - first_time_install.sh: use apt-get -o DPkg::Lock::Timeout=180 so apt handles lock contention natively instead of relying solely on flock TOCTOU check - install_dependencies_apt.py: pass DPkg::Lock::Timeout=180 to apt-get install to avoid failing when unattended-upgrades holds the lock - install_dependencies_apt.py: add type annotations to all public helpers - api_v3.py: fix install_plugin_requirements to read plugin_manager from api_v3 blueprint attribute instead of the always-None module variable - tools.html: loadGitInfo() now checks r.ok before parsing JSON and surfaces d.status === 'error' with the server's message in the panel Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tools,api): address three additional review findings - api_v3.py install_plugin_requirements: replace hardcoded plugin-repos fallback with config-driven resolution (plugin_system.plugins_directory), matching the pattern used elsewhere in the module - api_v3.py _fix_json_arrays: recurse into converted and existing array elements when items.type is object, so nested numeric-keyed dicts inside array items are also normalized - tools.html toolsAction: check r.ok before r.json() and recover gracefully from non-JSON error bodies (HTML 500 pages), consistent with the existing loadGitInfo guard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-06-29 14:21:05 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	6096a22c3d	feat(web): add Tools tab and row address type display setting (#373 ) * feat(web): add Tools tab and row address type setting Adds a Tools/Utilities tab to the web interface with one-click maintenance buttons that previously required SSH: - Git status panel (branch, dirty state, recent commits) - Pull latest (rebase) and force reset to origin/main - Reinstall base requirements (pip, with output) - Reinstall per-plugin requirements (pass/fail per plugin) - Clear __pycache__ directories - Quick-access restart for display and web services Also exposes the hzeller row_address_type option (0–4) in the Display settings tab. The backend already read this value from config; the UI, API field list, and validation were missing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tools-tab): address code review findings - Add _GIT = shutil.which('git') alongside _SUDO/_JOURNALCTL; return 503 in force_git_reset and get_git_info if git is unavailable - Check git branch/status returncodes in get_git_info(); return a clear 500 error instead of silently treating a failed run as a clean repo - Cap pip stdout+stderr at 50 KB via _truncate_output() helper to avoid OOM on verbose dependency resolution or build failures - Scrub embedded HTTPS credentials from remote_url via _scrub_git_remote_url() using urllib.parse before returning to UI - Fix clear_pycache to track and report failed deletions separately instead of counting them as successes (removed ignore_errors=True, wrapped in try/except OSError) Skipped: plugin_manager-vs-api_v3.plugin_manager (api_v3 is the Blueprint object; accessing .plugin_manager on it would fail — module- level variable is the correct pattern used throughout this blueprint); pages_v3 broad-except (identical to every other _load__partial in the file); base.html HTMX fallback (loadTabContent handles all tabs generically; named fallbacks only exist for tabs needing JS re-init); tools.html auth (pre-existing architectural decision — reboot/shutdown on the same endpoint are also unauthenticated). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> fix(tools-tab): resolve remaining PR review comments - api_v3: use getattr(api_v3, 'plugin_manager', None) instead of the module-level plugin_manager (always None); app.py sets the blueprint attribute, not the module global, so the fallback to plugin-repos was always taken - pages_v3: replace broad except Exception in _load_tools_partial with specific TemplateNotFound / OSError handlers and add [Pages V3][Tools] context prefix to log messages and error responses for easier Pi debugging - base.html: add Tools tab branch to the HTMX-unavailable fallback block in loadTabContent so the tab loads gracefully via direct fetch if HTMX never initialises Skipped: auth on execute_system_action — pre-existing app-wide design; reboot/shutdown and all other system actions share the same exposure. An app-level auth layer is the correct fix and is out of scope here. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tools-tab): resolve second-pass review findings - Wrap per-plugin subprocess.run in try/except TimeoutExpired/OSError so one plugin's failure appends a result entry and continues the loop rather than collapsing the whole batch into a 500 - Validate double_sided_copies divisibility against chain_length (horizontal axis) or parallel (vertical axis) after the range check; reads effective axis from the current request or stored config - Exclude double_sided_fields from the generic key-merge loop so double_sided_enabled/copies/axis are never written as root-level keys - Fix tools.html copy: "then restores the stash" removed — git_pull stashes changes but never pops them - Check r.ok and d.status in loadGitInfo before building the panel; backend error messages now surface instead of silently showing a false-clean state Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(tools-tab): don't expose filesystem paths in OSError messages CodeQL flagged str(exc) flowing into the JSON response for the install_plugin_requirements action. Use exc.strerror instead, which gives the OS error description ("No such file or directory", "Permission denied") without the internal filesystem path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-06-29 12:19:54 -04:00
Ron Pierce GitHub Claude Opus 4.8	fefc2d44a2	feat(display): double-sided mode — mirror one screen across the panel chain (#375 ) * feat(display): add double-sided mode to mirror one screen across the panel chain Renders a plugin once at a logical (per-screen) size, then tiles the rendered frame across the full physical chain so two (or more) panels show identical content. A 128x32 chain configured with 2 copies drives two 64x32 screens; vertical axis splits parallel outputs instead of the chain. Plugins size themselves from matrix.width/height, so a thin _LogicalMatrix proxy reports the logical size while delegating every real operation (CreateFrameCanvas, SwapOnVSync, brightness, Clear) to the physical matrix — no plugin changes required. Duplication is a single PIL paste per copy in update_display(), so render cost is unchanged. Config: display.double_sided { enabled, copies, axis }. Invalid config (non-divisible dimension, bad axis/copies) logs a warning and falls back to single-screen rather than failing to light up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(web): expose double-sided display config in the settings UI Adds a Double-Sided Display section to the Display settings page (enabled checkbox, copies, horizontal/vertical axis) and wires the save handler to persist it under display.double_sided. Validates copies (2-8) and axis, returning 400 on bad input; an omitted checkbox is saved as disabled. Like the other hardware fields, changes take effect after a display restart. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(display): add type hints and docstrings to double-sided proxy Addresses CodeRabbit nits: sort _LogicalMatrix.__slots__ (Ruff RUF023), annotate the proxy's __init__/properties/dunders and _resolve_double_sided's return type, and add docstrings to the property/dunder methods. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-25 15:24:53 -04:00
Ron Pierce GitHub Claude Opus 4.8	ab0cfd2362	fix(web): preserve dotted schema keys when saving plugin config (#370 ) The plugin config form posts form-data with dot-notation paths (e.g. "leagues.fifa.world.enabled"). _get_schema_property and _set_nested_value split those paths on every dot, so a schema key that itself contains a dot (soccer league keys like "fifa.world", "eng.1") was mistaken for nested "fifa" -> "world" objects. Per-league edits (enable, favorite_teams, nested booleans) were written to a fabricated "leagues.fifa.world" branch while the real league object was never updated, so saves silently dropped the change and produced a byte-identical config. Both helpers now greedily match the longest path segment that exists in the schema (_get_schema_property) or the config being updated (_set_nested_value), mirroring the frontend's dotted-key handling. Adds regression tests covering schema lookup, value typing, and writes under dotted league keys, plus a guard that plain nested paths still work. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-15 14:06:32 -04:00
Ron Pierce GitHub Claude Opus 4.8	d22d0a3754	fix(plugins): stop core updates from resurrecting uninstalled built-in plugins (#368 ) * fix(plugins): stop core updates from resurrecting uninstalled built-in plugins Built-in plugins (e.g. web-ui-info, starlark-apps) are committed into the repo under plugin-repos/. When a user uninstalls one, a subsequent core `git pull` update restores the committed files, so the plugin reappears on every update. The update endpoint stashes the deletion and never pops it, and `git pull` faithfully restores any committed file whose deletion was never committed — so excluding plugin-repos/ from the stash can't fix this (it would only make `git pull --rebase` fail on a dirty tree). Add a persistent uninstall registry (config/uninstalled_plugins.json, gitignored) that survives restarts, unlike the existing in-memory tombstone: - Uninstall records the plugin id; install clears it. - purge_uninstalled_plugins() re-removes any recorded plugin whose directory reappears on disk; called after a successful git-pull update and at web startup (covers manual `git pull` on the Pi too). - The state reconciler also refuses to auto-repair a persistently uninstalled plugin. Wires up mark_recently_uninstalled in the uninstall flow (previously only referenced by tests) via the new persistent record. Adds regression tests covering record/forget/purge lifecycle, persistence across manager instances, and corrupt-registry tolerance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(plugins): validate uninstall-registry ids and lock registry writes Address review feedback on the persistent uninstall registry: - Critical: validate plugin ids on read/record and add a containment guard in purge_uninstalled_plugins. A corrupt or hand-edited registry entry of "" resolves to the plugins root, so purge could have deleted every plugin; traversal ids ("..", "../x") could target paths outside the root. Invalid ids are now dropped on read, refused on record, and never removed unless the path is a direct child of the plugins directory. - Major: guard record/forget read-modify-write with a lock so concurrent install/uninstall requests can't lose updates. - Minor: narrow the startup and post-update purge exception handlers from bare Exception to (OSError, RuntimeError). Adds regression tests for empty-id, traversal-id, and invalid-record cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-11 18:18:28 -04:00
Ron Pierce GitHub Claude Opus 4.8	122e6d6863	fix(web): use fully-qualified .service unit names for privileged systemctl (#360 ) The web interface runs headless, so every privileged systemctl call must be covered by a NOPASSWD rule in /etc/sudoers.d/ledmatrix_web. The sudo command matches the command line exactly, but the code called 'systemctl start ledmatrix' while configure_web_sudo.sh grants 'systemctl start ledmatrix.service'. The rule never matched, so start/stop/enable/disable/ restart fell back to a password prompt and failed with 'a terminal is required to read the password'. Align all privileged systemctl calls on the fully-qualified unit names the sudoers grants use. Add a regression test that cross-checks api_v3.py calls against the grants in configure_web_sudo.sh. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-04 15:17:00 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	d488e8a2ad	fix(api): don't coerce all-digit strings to int when schema type is string (#363 ) * docs(core): add module and class docstrings to the 5 undocumented core files Fills the only significant documentation gaps found during a codebase audit. All other core files (plugin_system/, logging_config.py, etc.) already have complete module, class, and function docstrings. Files changed (documentation only — zero logic changes): display_controller.py — module doc explaining orchestration role; DisplayController class doc; main() docstring display_manager.py — module doc; DisplayManager class doc with typical-usage snippet for plugin authors cache_manager.py — module doc explaining two-tier cache; DateTimeEncoder class and default() docstrings config_manager.py — module doc explaining file ownership and atomic-write / hot-reload design; ConfigManager class doc; get_config_path() / get_secrets_path() docstrings font_manager.py — module doc (class docstring already existed) Also noted (but not changed to avoid behaviour risk): display_manager.py and font_manager.py use logging.getLogger() directly instead of the project's get_logger() wrapper. display_manager.py also calls setLevel(logging.INFO) immediately after, which would be lost if switched to get_logger(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * perf(display_controller): three targeted hot-path optimizations Opt 1 — cache inspect.signature() per plugin_id inspect.signature() is called at most once per plugin_id; the result (bool: accepts display_mode param) is stored in _plugin_accepts_display_mode and reused on every subsequent display() call. Eliminates all reflection from the display path at runtime. Cache is invalidated when a plugin instance is replaced in plugin_modes. Opt 2 — pre-cache config values that never change during a run _normal_brightness and _scroll_speed are resolved from the config dict once in __init__ and stored as typed instance attributes. - Removes 2+ chained dict.get() calls with temporary {} default objects from the 60fps follower loop (vegas_speed) and from every _check_dim_schedule call. - current_brightness init now uses _normal_brightness directly. Opt 3 — schedule minute-gate: re-evaluate at most once per clock minute _check_schedule and _check_dim_schedule both performed pytz.timezone(), datetime.now(), strftime(), and datetime.strptime() on every outer loop call. Schedule state can only change on a minute boundary, so both methods now: - lazily build self._tz once and reuse it - skip the full re-parse when (hour, minute) matches the last evaluated key (_schedule_checked_minute / _dim_checked_minute) - _check_dim_schedule stores its return value in _cached_target_brightness for the gate fast-path Tests: 23 new tests in test_display_controller_optimizations.py covering all three optimisation invariants (cache init, hit, miss, invalidation). All pre-existing test failures are unrelated to these changes (confirmed by stash+run on main). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve 22 pre-existing test failures across 6 groups Test fixes (tests were asserting wrong values or patching wrong objects): basketball scoreboard — update display mode assertions from generic basketball_live/recent/upcoming to league-prefixed nba_live/recent/upcoming to match the current manifest display_controller schedule — inject schedule directly into controller.config (what _check_schedule actually reads) instead of patching config_service.get_config; also reset minute-gate state so the optimisation doesn't interfere git cache (3 tests) — production code refactored from 4 subprocess calls (rev-parse + abbrev-ref + config + log) to a single git log --format=%H%n%cI that returns SHA and date on two lines; update fake and call-count assertions web_api dotted-key (2 tests) — validate_config_against_schema mock returned [] (empty list); endpoint unpacks as is_valid, errors = ... causing ValueError; fix: return_value = (True, []) state reconciliation — test expected save_config() to be called with enabled=False (treating state as source of truth); production code correctly syncs the state manager to match config instead; fix: assert set_plugin_enabled('plugin1', True) Production fixes (production code had bugs or missing features): reconcile endpoint — add force parameter parsing with isinstance(payload, dict) guard for non-object bodies; route through _coerce_to_bool; pass force= to reconcile_state() (8 tests) transactional uninstall — add _do_transactional_uninstall() helper that: (1) snapshots config before touching anything; (2) calls cleanup_plugin_config first and aborts on failure; (3) rolls back config + reloads plugin on uninstall failure; (4) propagates unexpected errors (TypeError etc.) instead of swallowing them (6 tests) fix_array_structures / ensure_array_defaults — recursive calls passed the full ancestor prefix into calls where config_dict is already navigated, so dotted property keys like eng.1 caused parent_parts.split('.') to mis-navigate; fix: drop prefix on recursive calls; also add _fix_none_arrays pass after merge_with_defaults so None arrays in JSON requests are replaced with schema defaults (2 tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * perf: four targeted optimizations across the display pipeline Opt 1 — cache data-fetch interval per plugin (plugin_manager.py) _get_plugin_update_interval fell back to config_manager.get_config() (a full dict copy) when the manifest lacked an interval. Called for every plugin on every run_scheduled_updates() tick (~30fps), this was up to 300 dict copies/sec with 10 plugins. Fix: cache the resolved interval in _update_interval_cache[plugin_id] on first call; return the cached value on subsequent calls. Cache is cleared on load_plugin and unload_plugin. Opt 2 — demote noisy per-cycle INFO logs to DEBUG (display_controller.py) Four logger.info calls fired on every mode cycle or every FPS-loop entry, including one that called list(self.plugin_modes.keys()) unconditionally (allocating a list every outer loop iteration). - "Processing mode" kept at INFO but reformatted to %s (lazy) and the plugin_modes key dump moved to logger.debug - "Attempting/Got cycle duration" → logger.debug - "Entering high/normal FPS loop" → logger.debug Mode name at INFO is preserved for black-screen troubleshooting. Opt 3 — use Image.frombytes instead of Image.fromarray in scroll hot path (scroll_helper.py) Image.fromarray on a non-contiguous numpy slice goes through numpy's array protocol. Image.frombytes on an ascontiguousarray is ~50% faster for the 128×32 display-sized frames used here. Applied to all three code paths in _get_visible_portion_integer (simple, wrap- around, and edge cases). Opt 5 — cache get_text_width per (text, font) pair (display_manager.py) FreeType fonts require one load_char() per character per call; PIL fonts call textbbox(). Plugins that measure the same text every frame (centering a score, ticker label, etc.) were re-measuring from scratch on every display() call. Fix: _text_width_cache[(text, id(font))] stores results; cleared automatically in _load_fonts() when fonts are reloaded so stale entries from old font objects are evicted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(scroll_helper): fix edge-case bug exposed by frombytes switch The previous commit replaced Image.fromarray with Image.frombytes in _get_visible_portion_integer. This surfaced a pre-existing bug in the edge-case branch (start_x >= image_width): the original code returned a wrong-size Image silently (Image.fromarray accepts a too-short array); Image.frombytes raises ValueError instead. Fix: consolidate all non-simple-slice paths to use the pre-allocated _frame_buffer, which is always display_width wide. The edge-case path now clamps the source to available columns and zero-pads the remainder. Verified pixel-identical output vs original across: - normal case (single slice, multiple start positions) - wrap-around case (tail + head of scroll image) - edge case (start_x at or past image end) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CodeRabbit review comments on PR #358 1. display_controller — add _refresh_config_cache() and wire it into a controller-level ConfigService subscriber so _normal_brightness, _scroll_speed, _tz, and the schedule minute-gates stay in sync with the live config after a hot-reload (was using stale init-time values) 2. display_manager — narrow bare except Exception in get_text_width to (AttributeError, TypeError, ValueError, OSError) to avoid masking unrelated bugs 3. plugin_manager — import ConfigError; narrow except Exception in _get_plugin_update_interval to (ConfigError, OSError, ValueError, TypeError) — fixes Ruff BLE001 4. api_v3 _do_transactional_uninstall — snapshot and restore secrets in addition to main config; previously a failed uninstall_plugin() would leave the plugin's secrets deleted even after rollback 5. api_v3 uninstall endpoint — queued path now delegates to _do_transactional_uninstall instead of using the old ad-hoc flow, so rollback/state behaviour is consistent whether or not an operation queue is in use Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(display_controller): move _plugin_accepts_display_mode init before plugin loop Codacy HIGH: 'access to member before its definition' — the dict was initialised at line 441 but accessed at line 364 inside the plugin- loading loop, both within __init__. Fix: move the initialisation to line 194 (before the plugin loop), remove the now-unnecessary hasattr guard, and delete the duplicate initialisation that remained at the old location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(api): don't coerce all-digit strings to int when schema type is string _parse_form_value_with_schema had a fallback that tried int()/float() on any string value that wasn't already handled. Fields like station_id (type: "string", value: "8726607") were silently converted to integers, causing jsonschema validation to reject them with "expected string, got int". Guard the fallback with a check that skips it when the schema property explicitly declares type: "string". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-06-04 15:01:42 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	eedf680a8c	perf: display pipeline optimizations — caching, logging, scroll, text width (#358 ) * docs(core): add module and class docstrings to the 5 undocumented core files Fills the only significant documentation gaps found during a codebase audit. All other core files (plugin_system/, logging_config.py, etc.) already have complete module, class, and function docstrings. Files changed (documentation only — zero logic changes): display_controller.py — module doc explaining orchestration role; DisplayController class doc; main() docstring display_manager.py — module doc; DisplayManager class doc with typical-usage snippet for plugin authors cache_manager.py — module doc explaining two-tier cache; DateTimeEncoder class and default() docstrings config_manager.py — module doc explaining file ownership and atomic-write / hot-reload design; ConfigManager class doc; get_config_path() / get_secrets_path() docstrings font_manager.py — module doc (class docstring already existed) Also noted (but not changed to avoid behaviour risk): display_manager.py and font_manager.py use logging.getLogger() directly instead of the project's get_logger() wrapper. display_manager.py also calls setLevel(logging.INFO) immediately after, which would be lost if switched to get_logger(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * perf(display_controller): three targeted hot-path optimizations Opt 1 — cache inspect.signature() per plugin_id inspect.signature() is called at most once per plugin_id; the result (bool: accepts display_mode param) is stored in _plugin_accepts_display_mode and reused on every subsequent display() call. Eliminates all reflection from the display path at runtime. Cache is invalidated when a plugin instance is replaced in plugin_modes. Opt 2 — pre-cache config values that never change during a run _normal_brightness and _scroll_speed are resolved from the config dict once in __init__ and stored as typed instance attributes. - Removes 2+ chained dict.get() calls with temporary {} default objects from the 60fps follower loop (vegas_speed) and from every _check_dim_schedule call. - current_brightness init now uses _normal_brightness directly. Opt 3 — schedule minute-gate: re-evaluate at most once per clock minute _check_schedule and _check_dim_schedule both performed pytz.timezone(), datetime.now(), strftime(), and datetime.strptime() on every outer loop call. Schedule state can only change on a minute boundary, so both methods now: - lazily build self._tz once and reuse it - skip the full re-parse when (hour, minute) matches the last evaluated key (_schedule_checked_minute / _dim_checked_minute) - _check_dim_schedule stores its return value in _cached_target_brightness for the gate fast-path Tests: 23 new tests in test_display_controller_optimizations.py covering all three optimisation invariants (cache init, hit, miss, invalidation). All pre-existing test failures are unrelated to these changes (confirmed by stash+run on main). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve 22 pre-existing test failures across 6 groups Test fixes (tests were asserting wrong values or patching wrong objects): basketball scoreboard — update display mode assertions from generic basketball_live/recent/upcoming to league-prefixed nba_live/recent/upcoming to match the current manifest display_controller schedule — inject schedule directly into controller.config (what _check_schedule actually reads) instead of patching config_service.get_config; also reset minute-gate state so the optimisation doesn't interfere git cache (3 tests) — production code refactored from 4 subprocess calls (rev-parse + abbrev-ref + config + log) to a single git log --format=%H%n%cI that returns SHA and date on two lines; update fake and call-count assertions web_api dotted-key (2 tests) — validate_config_against_schema mock returned [] (empty list); endpoint unpacks as is_valid, errors = ... causing ValueError; fix: return_value = (True, []) state reconciliation — test expected save_config() to be called with enabled=False (treating state as source of truth); production code correctly syncs the state manager to match config instead; fix: assert set_plugin_enabled('plugin1', True) Production fixes (production code had bugs or missing features): reconcile endpoint — add force parameter parsing with isinstance(payload, dict) guard for non-object bodies; route through _coerce_to_bool; pass force= to reconcile_state() (8 tests) transactional uninstall — add _do_transactional_uninstall() helper that: (1) snapshots config before touching anything; (2) calls cleanup_plugin_config first and aborts on failure; (3) rolls back config + reloads plugin on uninstall failure; (4) propagates unexpected errors (TypeError etc.) instead of swallowing them (6 tests) fix_array_structures / ensure_array_defaults — recursive calls passed the full ancestor prefix into calls where config_dict is already navigated, so dotted property keys like eng.1 caused parent_parts.split('.') to mis-navigate; fix: drop prefix on recursive calls; also add _fix_none_arrays pass after merge_with_defaults so None arrays in JSON requests are replaced with schema defaults (2 tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * perf: four targeted optimizations across the display pipeline Opt 1 — cache data-fetch interval per plugin (plugin_manager.py) _get_plugin_update_interval fell back to config_manager.get_config() (a full dict copy) when the manifest lacked an interval. Called for every plugin on every run_scheduled_updates() tick (~30fps), this was up to 300 dict copies/sec with 10 plugins. Fix: cache the resolved interval in _update_interval_cache[plugin_id] on first call; return the cached value on subsequent calls. Cache is cleared on load_plugin and unload_plugin. Opt 2 — demote noisy per-cycle INFO logs to DEBUG (display_controller.py) Four logger.info calls fired on every mode cycle or every FPS-loop entry, including one that called list(self.plugin_modes.keys()) unconditionally (allocating a list every outer loop iteration). - "Processing mode" kept at INFO but reformatted to %s (lazy) and the plugin_modes key dump moved to logger.debug - "Attempting/Got cycle duration" → logger.debug - "Entering high/normal FPS loop" → logger.debug Mode name at INFO is preserved for black-screen troubleshooting. Opt 3 — use Image.frombytes instead of Image.fromarray in scroll hot path (scroll_helper.py) Image.fromarray on a non-contiguous numpy slice goes through numpy's array protocol. Image.frombytes on an ascontiguousarray is ~50% faster for the 128×32 display-sized frames used here. Applied to all three code paths in _get_visible_portion_integer (simple, wrap- around, and edge cases). Opt 5 — cache get_text_width per (text, font) pair (display_manager.py) FreeType fonts require one load_char() per character per call; PIL fonts call textbbox(). Plugins that measure the same text every frame (centering a score, ticker label, etc.) were re-measuring from scratch on every display() call. Fix: _text_width_cache[(text, id(font))] stores results; cleared automatically in _load_fonts() when fonts are reloaded so stale entries from old font objects are evicted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(scroll_helper): fix edge-case bug exposed by frombytes switch The previous commit replaced Image.fromarray with Image.frombytes in _get_visible_portion_integer. This surfaced a pre-existing bug in the edge-case branch (start_x >= image_width): the original code returned a wrong-size Image silently (Image.fromarray accepts a too-short array); Image.frombytes raises ValueError instead. Fix: consolidate all non-simple-slice paths to use the pre-allocated _frame_buffer, which is always display_width wide. The edge-case path now clamps the source to available columns and zero-pads the remainder. Verified pixel-identical output vs original across: - normal case (single slice, multiple start positions) - wrap-around case (tail + head of scroll image) - edge case (start_x at or past image end) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address CodeRabbit review comments on PR #358 1. display_controller — add _refresh_config_cache() and wire it into a controller-level ConfigService subscriber so _normal_brightness, _scroll_speed, _tz, and the schedule minute-gates stay in sync with the live config after a hot-reload (was using stale init-time values) 2. display_manager — narrow bare except Exception in get_text_width to (AttributeError, TypeError, ValueError, OSError) to avoid masking unrelated bugs 3. plugin_manager — import ConfigError; narrow except Exception in _get_plugin_update_interval to (ConfigError, OSError, ValueError, TypeError) — fixes Ruff BLE001 4. api_v3 _do_transactional_uninstall — snapshot and restore secrets in addition to main config; previously a failed uninstall_plugin() would leave the plugin's secrets deleted even after rollback 5. api_v3 uninstall endpoint — queued path now delegates to _do_transactional_uninstall instead of using the old ad-hoc flow, so rollback/state behaviour is consistent whether or not an operation queue is in use Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(display_controller): move _plugin_accepts_display_mode init before plugin loop Codacy HIGH: 'access to member before its definition' — the dict was initialised at line 441 but accessed at line 364 inside the plugin- loading loop, both within __init__. Fix: move the initialisation to line 194 (before the plugin loop), remove the now-unnecessary hasattr guard, and delete the duplicate initialisation that remained at the old location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-06-01 11:58:21 -04:00
Chuck GitHub Chuck Claude Sonnet 4.6	f96fdd9f24	fix(plugins): skip update for local-only plugins instead of failing (#354 ) Adds a local_only flag to the starlark-apps manifest so the update endpoint returns a skipped status rather than recording a false failure when the plugin has no git repo and no registry entry. Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-27 21:42:25 -04:00
Chuck GitHub Chuck Claude Sonnet 4.6	34b186125a	fix(logs): include ledmatrix-web logs in viewer and log subprocess stderr on failure (#350 ) Two bugs conspired to produce "check the logs" toasts with an empty log viewer: 1. The log viewer (both SSE stream and REST endpoint) only queried ledmatrix.service via journalctl. Web API errors are logged by the Flask process running as ledmatrix-web.service, so they never appeared in the viewer. Add -u ledmatrix-web.service to both calls; also add --output=short-iso so timestamps from the two services sort cleanly when interleaved. Use shutil.which-resolved absolute paths for sudo/journalctl (S607 compliance) in api_v3.py; fall back to known Pi paths if which returns None. 2. app.py: resolve journalctl and systemctl to absolute paths via shutil.which at module init (_JOURNALCTL, _SYSTEMCTL). Replace bare names in logs_generator() and the cached systemctl is-active check. Guard both sites: logs_generator yields a clear SSE error message and sleeps 60 s if journalctl is not found; the systemctl block is skipped entirely if systemctl is not found, leaving the cache at its last-known value. 3. When execute_system_action() ran a systemctl command that returned non-zero, only the return code was logged — result.stderr was silently discarded. Log it at ERROR level and include returncode and stderr in the JSON response so callers get actionable failure details. Same fix applied to the early-return start_display branch. Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-26 09:55:59 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	321a87f734	fix(wifi): fix AP mode, captive portal, and WiFi connect flow (#348 ) * fix(wifi): fix AP mode, captive portal, and WiFi connect flow - Fix scan API returning 500: scan_networks() returns a tuple but the endpoint was iterating it directly; unpack with _was_cached - Fix IP address display showing 'IP4.ADDRESS[1]:x.x.x.x': nmcli -t output includes the field label; split on ':' before '/' - Add force parameter to enable_ap_mode() to bypass WiFi/Ethernet guards; expose via force JSON body field in the AP enable endpoint - Fix daemon auto-disabling forced AP: add _FORCE_AP_FLAG_PATH flag file written on force-enable and checked in check_and_manage_ap_mode before auto-disabling; disable_ap_mode() clears it - Fix wifi_connected false positive in AP mode: _get_status_nmcli() was reporting wlan0 as 'connected' when it was running as AP; override wifi_connected=False when _is_ap_mode_active() is True - Fix AP verification failure on async NM activation: retry _get_ap_status_nmcli() up to 5 times with 2s delay instead of single immediate check - Fix WiFi connect ignoring existing NM connections: nmcli does not support 802-11-wireless.ssid as a column in 'connection show'; replace with NAME,TYPE list then per-connection SSID query via -g (fixes 'netplan generate failed' error on Trixie / netplan systems) - Fix failsafe AP re-enable blocked by Ethernet: all recovery-path enable_ap_mode() calls in connect_to_network() now pass force=True Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): strict bool parsing for force; nosec annotation parity - api_v3.py: replace bool(...) coercion for force with strict check — only actual boolean True or strings "true"/"1" (case-insensitive) pass; "false", integers, and other strings are treated as False so the Ethernet/WiFi guards and _FORCE_AP_FLAG_PATH cannot be bypassed by accident - wifi_manager.py: add nosec B108 annotation to _IP_FORWARD_SAVE_PATH to match the identical annotation already on _FORCE_AP_FLAG_PATH Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): suppress false-positive Bandit B603/B607 on new nmcli calls Both subprocess.run calls in the SSID connection lookup use fixed arguments (no user input) or values derived from nmcli's own output — not from user-controlled data. Add nosec B603 B607 annotations to silence the Codacy/Bandit warnings, consistent with existing nosec usage in the file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): address four review findings in wifi_manager.py IP parsing (line 476): use partition(':') so bare "ip/mask" lines (no field-label prefix) are handled without IndexError; falls back to the full string when no ':' is present before splitting on '/'. AP-mode override comment (line 503): add one-line explanation above the wifi_connected/ssid/ip_address clear so maintainers know why the fields are reset while wlan0 reports as "connected". Stale force-flag cleanup (__init__): remove a left-over _FORCE_AP_FLAG_PATH from a prior crash on first instantiation per process (guarded by class-level _startup_cleanup_done so the nmcli AP-state check only runs once, not on every per-request instantiation). Force-flag logging (enable_ap_mode): log at debug when force=True is applied, log success at debug and failure with OSError details at warning for both the hostapd and nmcli hotspot paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Chuck <chuck@example.com>	2026-05-24 16:12:59 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	713539e491	fix(web-ui): fix quick actions not firing, add toast feedback, suppress install handler warning (#346 ) * fix(web-ui): fix quick actions not firing, add toast feedback, suppress install handler warning - base.html: add htmx:afterSettle listener to set data-loaded on tab containers after HTMX swaps their content, preventing the overview partial from being re-fetched (and handlers lost) on every tab switch - base.html: call htmx.process() in loadOverviewDirect/loadPluginsDirect fallbacks so buttons get HTMX handlers even if HTMX finished its initial body scan before the fallback fetch completed - overview.html + index.html (11 buttons): replace event.detail.xhr.responseJSON (undefined in HTMX 1.9.x) with JSON.parse(event.detail.xhr.responseText) so quick action toast notifications actually fire - plugins_manager.js: add guarded htmx:afterSettle listener that only calls attachInstallButtonHandler when #install-plugin-from-url is in the DOM, eliminating the spurious console warning on non-plugin tab loads Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): ensure quick-action toasts always fire even on xhr/parse failure Replace silent catch(e){} in all 11 hx-on:htmx:after-request handlers with a pattern that sets default message/status before the try block and calls showNotification(m,s) unconditionally after it, so a fallback toast is shown whenever xhr is absent or responseText is not valid JSON. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): show error toast on non-JSON 4xx/5xx quick-action responses In the catch block of all 11 hx-on:htmx:after-request handlers, check xhr.status >= 400 and downgrade s to 'error' so a failed action that returns an HTML error page (or other non-JSON body) surfaces as an error toast instead of the optimistic 'success'/'info' default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): guard setTimeout fallback for attachInstallButtonHandler The 500ms fallback setTimeout was calling attachInstallButtonHandler() unconditionally even when the plugins partial wasn't in the DOM, causing a spurious console.warn on every page load. Add the same element-existence check already present on the htmx:afterSettle listener. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix backup API 404s, hardware status 500, and HTMX loading race - Add all backup API routes to api_v3.py: preview, list, export, validate, restore (with plugin reinstall), download, delete - Fix PermissionError on /hardware/status: return graceful 200 instead of 500 when the status file is owned by a different user; also fix root cause by writing the file world-readable (0o644) in display_manager - Fix HTMX race: dispatch htmx:ready window event from HTMX onload callback; loadTabContent now waits for that event instead of immediately falling back to direct fetch (eliminating the "HTMX not available" console warning on initial load) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Cancel HTMX fallback timers when htmx:ready fires The 5-second setTimeout fallbacks for plugins and overview were firing before the htmx:ready event arrived, logging spurious warnings. Each timer now self-cancels via htmx:ready so the fallback only triggers when HTMX genuinely fails to load. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review feedback: error leaks, ok:false, htmx:ready coverage - Backup endpoints: replace raw str(e) in user-facing responses with a generic message; full exception still logged via exc_info=True - hardware/status: change ok:null to ok:false for PermissionError and json.JSONDecodeError so the UI's hw.ok===false check triggers correctly - base.html: dispatch htmx:ready from the fallback load path so any deferred listeners fire on CDN-fallback loads too - loadTabContent: also listen for htmx-load-failed so overview/wifi/plugins fall back to direct fetch when HTMX is completely unavailable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Treat system-managed pip packages as satisfied for dependency marker When a plugin's requirements.txt includes a package installed via the system package manager (dnf/apt), pip fails with 'uninstall-no-record-file' because it can't replace the system-tracked copy. The package is present and functional, but the missing marker caused the install to be retried on every service restart. Detect this specific error pattern: if the only pip failure is uninstall-no-record-file, write the .dependencies_installed marker and log a warning instead of returning False, suppressing the repeated warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix uninstall-no-record-file detection condition The previous check used a string replacement that left 'error:' in the remaining text, causing the condition to always evaluate false. Simplify to a direct substring check: if 'uninstall-no-record-file' appears in pip stderr the affected package is installed at the system level and we write the marker, suppressing the repeated warning on every restart. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Resolve CodeQL security findings in backup API Path traversal (CWE-22): - backup_download: switch from send_file(user-tainted-path) to send_from_directory(_BACKUP_EXPORT_DIR, filename); Flask uses werkzeug safe_join internally which CodeQL recognises as a sanitizer - backup_delete: enumerate the export directory and match by name so entry.unlink() operates on a filesystem-derived Path rather than one constructed from user input; _safe_backup_path still guards first Information exposure through exceptions (CWE-209): - backup_validate: err_msg from validate_backup() can embed exception strings containing temp-file paths; log the detail, return a generic 'Invalid or corrupted backup file' to the client - Other backup endpoints: already fixed (str(e) -> generic message); CodeQL alerts will clear on next scan plugin_loader.py:185 (path traversal): false positive — requirements_file is constructed from plugin_dir returned by find_plugin_directory() (a filesystem scan), not from raw HTTP request input; no change needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pre-existing information exposure in version and action endpoints - get_system_version (alert #218): replaced str(e) with generic message; exception still logged via logger.error(exc_info=True) - execute_system_action (alert #216): removed str(e) and full traceback.format_exc() from the HTTP response — the full stack trace was being sent directly to clients; replaced with generic message and proper logger.error call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix remaining GitHub CodeQL security alerts - py/stack-trace-exposure: Remove str(e) and traceback.format_exc() from all HTTP responses across api_v3.py, pages_v3.py, and app.py; replace with generic messages and logger.error(exc_info=True) - py/reflective-xss: Escape partial_name via markupsafe.escape in the load_partial 404 response - py/path-injection: Add regex validation of plugin_id before filesystem use in _load_plugin_config_partial - py/incomplete-url-substring-sanitization: Replace 'github.com' in substring checks with urlparse hostname comparison in store_manager.py - py/clear-text-logging-sensitive-data: Remove football-scoreboard debug prints and sensitive request-body prints from update endpoint - js/bad-tag-filter: Replace script-only regex in BaseWidget.sanitizeValue with DOM-based textContent stripping that removes all HTML - js/incomplete-sanitization: Fix escapeAttr to properly encode &, ", ', <, > using HTML entities instead of backslash escaping - js/prototype-pollution-utility: Add __proto__/constructor/prototype key guards to deepMerge function in plugins_manager.js - app.py error handlers: Always return generic messages; remove debug-mode branches that could expose tracebacks in production Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix three remaining CodeQL path-injection and info-exposure alerts - plugin_loader.py: resolve plugin_dir with strict=True and validate marker_path with relative_to() before any filesystem writes, giving CodeQL the positive sanitization pattern it requires (py/path-injection) - api_v3.py _safe_backup_path: replace substring negative checks with a strict positive regex (^[a-zA-Z0-9][a-zA-Z0-9._-]{0,200}\.zip$) that CodeQL recognises as sanitising the user-supplied filename (py/path-injection) - api_v3.py backup_validate: whitelist known-safe manifest fields before returning JSON, preventing any exception strings captured inside validate_backup() from reaching the HTTP response (py/stack-trace-exposure) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Resolve 29 open CodeQL security alerts across 5 files py/flask-debug (#214): - debug_web_manual.py: read debug mode from LEDMATRIX_FLASK_DEBUG env var instead of hardcoded True py/stack-trace-exposure (#216, #218): - api_v3.py execute_system_action: remove subprocess stdout/stderr from HTTP responses; log via logger instead - api_v3.py get_git_version: validate output matches safe ref format (^[a-zA-Z0-9._-]+$) before including in response - api_v3.py: remove all remaining traceback.format_exc() dead variables and print() debug calls (replaced with logger.debug/warning) py/reflective-xss (#207, #208, #209, #210, #211, #212): - api_v3.py: remove plugin_id from all error/success response messages (uninstall, install, update, health, not-found responses) - pages_v3.py load_partial: return static "Partial not found" message instead of echoing partial_name - pages_v3.py _load_starlark_config_partial: add app_id regex validation, use static error messages instead of f-strings with app_id py/path-injection (#187–#206): - pages_v3.py _load_plugin_config_partial: resolve plugins_base and validate _plugin_dir with relative_to() before all file operations; same for assets metadata directory - pages_v3.py _load_starlark_config_partial: resolve starlark_base and validate schema_file/config_file paths with relative_to() - plugin_loader.py _find_plugin_directory: resolve plugins_dir and validate strategy-2 candidates with relative_to() - plugin_loader.py install_dependencies: resolve plugin_dir first, then construct requirements_file and marker_path from resolved base - plugin_loader.py load_module: resolve plugin_dir with strict=True and validate entry_file with relative_to() before exec_module Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix 15 remaining CodeQL path-injection and stack-trace-exposure alerts Switch from resolve()+relative_to() to os.path.basename() reassignment, which CodeQL recognizes as a path sanitizer that breaks the taint chain. Also remove exception objects from backup_manager validate_backup return strings to eliminate the stack-trace-exposure taint source. Fixes alerts #227, #233, #234, #235, #237, #238, #239, #240, #241, #242, #243, #244, #245, #246, #247. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix broken logger format string and leaked exception in config save error - pages_v3.py: plain string was used instead of %-style substitution, so every manifest-read failure logged the literal "{plugin_id}" - api_v3.py save_main_config: exception message was still leaking through the error response; replace with generic message (consistent with the rest of the CodeQL sweep in this PR) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-24 09:29:53 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	302ab1da4f	fix(plugin-config): handle missing type key in oneOf/anyOf schema fields (#344 ) * fix(web-ui): dedup registry fetches, surface reconciliation warnings, add check-update endpoint Story 1 — src/plugin_system/store_manager.py: Add threading.Lock (_registry_fetch_lock) to fetch_registry(). The outer cache check remains the hot path (no lock). When the cache is cold, only one thread hits the network; concurrent callers block on the lock then get the result from the warm cache (double-checked locking). Eliminates duplicate GitHub requests on every page load when the 15-minute cache expires. Story 2 — web_interface/app.py + api_v3.py + overview.html: _run_startup_reconciliation() now writes /tmp/ledmatrix_reconciliation.json (atomic tempfile+replace, mirrors hw_status pattern) so the result survives the background thread. New GET /api/v3/plugins/reconciliation-status reads that file. Overview page gains a dismissible yellow banner that shows stale plugin_id values (e.g. sync, github, youtube) and tells the user to remove them or reinstall from the Plugin Store. Banner is suppressed for the session after dismiss using sessionStorage keyed on the plugin_id list. Story 3 — web_interface/blueprints/api_v3.py: Add GET /api/v3/system/check-update. Does git fetch origin main then compares local HEAD vs origin/main to compute update_available, remote_sha, and commits_behind. Result is cached for 5 minutes so it doesn't run git on every page load. Falls back to {update_available: false} on any error. Eliminates the 404 logged on every page load. Story 4 (Pi 5 rgbmatrix rebuild) was already fixed in PR #341. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugin-config): handle missing `type` key in schema fields using oneOf/anyOf Jinja2's `prop.type` on a dict without a `type` key returns an Undefined object. Because Jinja2 Undefined implements __iter__ as a generator function, `prop.type is iterable` evaluates True, then `prop.type[0]` calls Undefined.__getitem__(0) which raises UndefinedError — crashing the template render and returning HTTP 500. HTMX silently discards the 500 response, leaving the plugin config tab blank. Fix: use `prop.get('type')` which returns None for missing keys instead of Undefined. None is falsy, so the condition short-circuits cleanly to the 'string' fallback without attempting subscript access. Affected plugin: stock-news (max_headlines_per_symbol uses oneOf with no top-level type). Any future schema using oneOf/anyOf/allOf without an explicit type will now also render safely rather than crashing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): harden check-update, reconciliation status endpoint, and temp-file write api_v3.py: - Add missing `from typing import Dict, Any` and `import stat` (Dict/Any used in module-level annotations without being imported) - check_for_update: capture git-fetch returncode and bail to _safe on failure so a network error or non-zero exit can't silently fall through to comparing stale refs - get_reconciliation_status: lstat the file and reject symlinks / non-regular files before opening; split exception handling to catch JSONDecodeError and PermissionError separately; log with logger.exception; return a generic 'Status file unavailable' message instead of str(e) to avoid leaking internal details overview.html: - Replace one-shot reconciliation fetch with a polling loop (2 s interval via setTimeout) so the banner still appears when reconciliation finishes after the page first loads - dismissReconciliationBanner: write sessionStorage immediately using the key stored on the banner element (set at show time) so dismissal persists even if the background sync fetch fails; clear the polling timer on dismiss to avoid leaks app.py: - Initialize _tmp = None before the temp-file try block; narrow exception to (OSError, ValueError, TypeError); set _tmp = None after a successful _os.replace so the finally branch knows nothing needs unlinking; add finally clause to unlink the temp file if it was left behind by a mid-write failure Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: reconciliation status errors return graceful not-done instead of HTTP 500; log fetch stderr get_reconciliation_status: symlink/non-regular-file, JSONDecodeError, and PermissionError all now return {'done': False, 'unresolved': []} so the polling loop in overview.html keeps retrying rather than stopping on a transient error. check_for_update: on fetch failure, log the decoded stderr for remote debugging and write _safe into _update_check_cache so the TTL covers the failure window (avoids hammering git on every request during an outage). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(bandit): replace hardcoded /tmp paths with tempfile.gettempdir() (B108) Codacy/Bandit B108 flagged two hardcoded '/tmp/' string literals in app.py (lines 737, 741). Replaced with _tempfile.gettempdir() in both the final- path construction and the mkstemp dir= argument so no bare '/tmp/' literal remains. Also updated the matching reader path in api_v3.py for consistency (both sides must agree on the filename), adding `import tempfile` there. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-21 15:53:16 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	33f76b4895	feat(pi5): RP1 backend UI, gpio slowdown guidance, and hardware init error banner (#337 ) * feat(pi5): expose RP1 backend selector, fix gpio defaults, surface init failures in web UI - Add rp1_rio select (PIO/RIO) to Display Settings hardware config section; saved via /api/v3/config/main with 0-or-1 validation — previously the key existed in config.json but was not editable from the UI - Update gpio_slowdown help text with per-model guidance (Pi 3: 3, Pi 4: 4, Pi 5: 4–5) and raise max from 5 → 10 to match full library range - Fix gpio_slowdown Python fallback default from 2 → 3 (only affects edge case where the runtime config section is absent; explicit config values are unchanged) - display_manager writes /tmp/led_matrix_hw_status.json at startup: ok/error; Display Settings page fetches it and shows a yellow warning banner when the matrix failed to initialize, including Pi 5 remediation steps - Add GET /api/v3/hardware/status endpoint that reads the status file - Improve fallback error log to include Pi 5 rebuild hint Pi 3/4 users: rp1_rio=0 is set in config but silently ignored by the library on non-RP1 hardware; all other changes are additive or tighten defaults only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pi5): correct gpio_slowdown guidance — Pi 5 PIO default is 1, not 4-5 The upstream library defaults gpio_slowdown to 1 for Pi 5 (IsPi4() ? 2 : 1). In PIO mode the value is a pixel-clock divisor, so 4-5 was unnecessarily conservative advice. Updated help text and error log to reflect the actual range (1-3 typical for Pi 5 PIO; inverted effect in RIO mode). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): atomic hw-status write, narrow bare excepts, urllib3 CVE floor - display_manager: replace open()+bare-except with tempfile.mkstemp→fsync→ chmod(0o600)→os.replace; adds symlink guard and logs errors via logger instead of swallowing them silently; pull json/tempfile to module imports - display_manager cleanup(): narrow broad `except Exception: pass` to (OSError, RuntimeError, ValueError, MemoryError) with debug log - api_v3 get_hardware_status(): catch json.JSONDecodeError and PermissionError explicitly; log full traceback server-side; return generic "Unable to read hardware status" to client instead of leaking str(e) - march-madness/requirements.txt: bump urllib3 floor 2.2.2→2.6.3 (CVE fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(template): apply \|int filter to rp1_rio comparisons in display.html Without \|int, a string-typed value (e.g. from a hand-edited config.json) causes both selected tests to fail and the select renders with no option pre-selected. Matches the existing pattern used for multiplexing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-18 17:42:19 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	05b3fa56cb	fix: Codacy security fixes, CVE dependency bumps, and code quality cleanup (#331 ) * fix(deps): bump minimum versions to address CVEs Pillow 10.4.0 → 12.2.0: CVE-2026-40192 (DoS via FITS decompression bomb), CVE-2026-25990 (OOB write via PSD image), CVE-2026-42311/42308/42310 requests 2.32.0 → 2.33.0: CVE-2026-25645 (temp file security bypass), CVE-2024-47081 (.netrc credentials leak) werkzeug 3.0.0 → 3.1.6: CVE-2023-46136, CVE-2024-49766/49767, CVE-2025-66221, CVE-2026-21860/27199 (DoS, path traversal, safe_join bypass) Flask 3.0.0 → 3.1.3: CVE-2026-27205 (session data caching info disclosure) spotipy 2.24.0 → 2.25.2: CVE-2025-27154, CVE-2025-66040 python-socketio 5.11.0 → 5.14.0: CVE-2025-61765 pytest 7.4.0 → 9.0.3: CVE-2025-71176 (insecure temp dir handling) Updated in requirements.txt, web_interface/requirements.txt, plugin-repos/starlark-apps/requirements.txt, and plugin-repos/march-madness/requirements.txt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve Pylint errors in executor, data service, and odds call Rename TimeoutError to PluginTimeoutError in plugin_executor.py to avoid shadowing the built-in; no external callers affected. Remove dead try/except in BackgroundDataService.shutdown: executor.shutdown() never accepted a timeout kwarg so the try branch always raised TypeError. Simplify to a direct shutdown(wait=wait) call. Remove is_live kwarg from odds_manager.get_odds() call in sports.py; BaseOddsManager.get_odds() has no such parameter. The live update interval is already encoded in the update_interval_seconds argument passed alongside. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: MD5→SHA-256, shellcheck warnings, and broken doc links config_service.py: replace MD5 with SHA-256 for config change detection; same semantics (equality comparison), no stored hashes affected. Shell scripts — shellcheck warnings: - diagnose_web_interface.sh: remove useless cat (SC2002) - dev_plugin_setup.sh: restructure A&&B\|\|C into if/then (SC2015) - fix_assets_permissions.sh: remove unused REAL_HOME block (SC2034) - install_web_service.sh: remove unused USER_HOME assignment (SC2034) - diagnose_web_ui.sh: remove unused SUDO assignments (SC2034) - diagnose_plugin_permissions.sh: remove unused BLUE color var (SC2034) - first_time_install.sh: remove unused CLEAR var, PACKAGE_NAME assignment, and replace loop variable with _ (SC2034) docs/PLUGIN_ARCHITECTURE_SPEC.md: fix 10 broken TOC anchor links to include section numbers matching the actual headings (MD051). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unused imports and bare exception aliases (pyflakes F401/F841) Remove unused imports across 86 files in src/, web_interface/, test/, and scripts/ using autoflake. No logic changes — only dead import statements and unused names in from-imports are removed. Also remove bare exception aliases where the variable is never referenced in the handler body: - src/cache/disk_cache.py: except (IOError, OSError, PermissionError) as e - src/cache_manager.py: except (OSError, IOError, PermissionError) as perm_error - src/plugin_system/resource_monitor.py: except Exception as e - web_interface/app.py: except Exception as read_err 86 files changed, 205 lines removed, 18 pre-existing test failures unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unused local variable assignments (pyflakes F841) Dead assignments removed across src/ and web_interface/: - background_data_service: drop future= on fire-and-forget executor.submit - base_classes/baseball: drop font= (all rendering uses self.fonts['time']) - base_classes/hockey: drop status_short= (never referenced after assignment) - common/cli: drop game_helper=/config_helper= bindings in import-test block; constructors called for instantiation-only validation - common/display_helper: drop text_width= (x_position uses display_width directly); drop draw= in create_error_image (uses _draw_centered_text) - config_manager: remove dead secrets_content loading block in migration path (comment already noted save_config_atomic handles secrets internally) - display_manager: drop setup_start= (timing was never completed or read) - font_manager: drop target_path= (catalog uses font_file_path directly); drop face=/font= bindings in validate_font (validation by construction — TypeError on failure is the signal, not the return value) - font_test_manager: drop width=/height= (draw_text uses display_manager directly) - plugin_system/state_reconciliation: drop manager= (only config/disk/state_mgr used) - plugin_system/store_manager: drop result= on pip install subprocess.run (check=True raises on failure; stdout unused) - web_interface/blueprints/pages_v3: drop main_config_path=""/secrets_config_path="" (render_template uses config_manager.get__path() inline) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> fix(js): resolve ESLint no-undef warnings across 6 JS files Three distinct patterns: 1. Vendor library globals — htmx is injected by <script> before these extension files load; ESLint lints files in isolation and doesn't know. Fix: add /* global htmx / to htmx-sse.js and htmx-json-enc.js. 2. Cross-file globals — showNotification is defined as window.showNotification in app.js/notification.js but called bare in app.js and error_handler.js. ESLint doesn't connect window.X = Y with a bare call to X. Fix: add / global showNotification / to app.js and error_handler.js. 3. Forward-reference window. functions — in array-table.js, checkbox-group.js, and custom-feeds.js, functions like removeArrayTableRow are called early inside event-handler closures but assigned to window.* later in the file. At runtime this works (the handler fires after the assignment), but ESLint sees the bare name at the call site. Fix: change bare calls to window.removeArrayTableRow(this) etc. so the reference is explicit and ESLint-safe. Also guard the updateSystemStats call in app.js reconnectSSE: the function is called but defined nowhere in the codebase. Guard with typeof check so it won't throw ReferenceError if the reconnect path is hit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(js): resolve Biome lint warnings across 9 JS files noUnusedVariables (catch bindings → optional catch syntax): - app.js, file-upload.js, timezone-selector.js: } catch (e) { → } catch { ES2019 optional catch binding; e was unused in all three handlers noUnusedVariables (dead assignments): - app.js: remove const data= in display SSE stub (handler does nothing yet) - api_client.js: remove const timeoutId= (setTimeout ID never used to cancel) - custom-feeds.js: remove const oldIndex= (getAttribute result never read) - schedule-picker.js: remove const compactMode= (never used in HTML build) - select-dropdown.js: remove const icons= (icons not yet rendered in options) noPrototypeBuiltins: - day-selector.js: DAY_LABELS.hasOwnProperty(x) → Object.prototype.hasOwnProperty.call(DAY_LABELS, x) Safe form that works even on null-prototype objects useIterableCallbackReturn: - file-upload.js, notification.js: forEach(x => expr) → forEach(x => { expr; }) — forEach ignores return values; implicit return from arrow body was misleading htmx-sse.js is a vendor extension file with old-style var/== patterns that are correct for it; 18 Biome issues suppressed via Codacy API rather than modifying the vendor source. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): escape user input in raw HTML responses in pages_v3.py plugin_id comes directly from the URL path (/partials/plugin-config/<plugin_id>) and was interpolated into an HTML fragment without escaping. A crafted URL like /partials/plugin-config/<script>alert(1)</script> would inject that tag into the DOM via the HTMX partial response. Fix: wrap all user-controlled values in markupsafe.escape() before embedding in raw HTML strings. Affects the plugin-not-found 404 response and both error 500 responses in the plugin config partial. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Bandit B108/B110 across production code B110 (try/except/pass): - display_controller.py: narrow 'except Exception' to 'except AttributeError' for get_offset_frame() — plugins not having this optional method is the expected case, not all exceptions - config_manager.py: B110 already resolved by the earlier removal of the dead secrets-loading block (the except/pass was inside it) - All other except/pass blocks in src/ and web_interface/ are intentional (last-resort recovery, best-effort fallbacks, non-critical startup probes). Annotated each with # nosec B110 and a brief inline reason so the decision is explicit for future reviewers. - Test files and plugin-repos B110 suppressed via Codacy API (not prod code). B108 (/tmp usage): - permission_utils.py: /tmp listed to PREVENT permission changes on it — not used as a temp path. Annotated # nosec B108. - display_manager.py: fixed snapshot path is intentional (web UI reads same path); path-check guard also annotated. - wifi_manager.py: named /tmp files match the sudoers allowlist installed with the system (the paths are hard-coded in both places by design). Annotated all six open/cp references # nosec B108. - scripts/render_plugin.py: dev script default overridable by user. Annotated. - web_interface/app.py: reads the same fixed path written by display_manager. Annotated # nosec B108. - Test files suppressed via Codacy API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address remaining Codacy security findings Flask debug=True (real fix): - web_interface/app.py: debug=True in __main__ block exposes the Werkzeug interactive debugger (arbitrary code execution). Changed to os.environ.get('FLASK_DEBUG', '0') == '1' — off by default, opt-in via environment variable for local development. nosec annotations (accepted risk with documented rationale): - disk_cache.py: os.chmod(0o660) is intentional — web UI and LED matrix service share a group, 660 gives group write while denying world access (B103 + Semgrep insecure-file-permissions suppressed in Codacy) - wifi_manager.py: urlopen to hardcoded connectivity-check.ubuntu.com URL (B310 — no user input involved) - font_manager.py: urlretrieve URL comes from user's own config file on their local device (B310) - start_web_conditionally.py: os.execvp with both sys.executable and a fixed PROJECT_DIR-relative constant (B606) Confirmed false positives suppressed via Codacy API (15 issues): - SSRF (3x): client-side JS fetch — SSRF is server-side; browser fetch is CORS-restricted to same origin - B105 (3x): test fixtures use dummy secrets by design; store_manager checks for the placeholder string, it is not itself a secret - PMD numeric literal (2x): 10000000 is within Number.MAX_SAFE_INTEGER - Prototype pollution (1x): read-only schema traversal, no writes - no-unsanitized_method (1x): dynamic import() is CORS-restricted - detect-unsafe-regex (1x): operates on server-controlled config values - plugin-repos B103 (1x): vendor code chmod on executable - Semgrep insecure-file-permissions (3x): same disk_cache 0o660 as above Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unnecessary f prefix from f-strings without placeholders (F541) Pyflakes F541 flags f-strings that contain no {} interpolation — they are identical to plain strings but trigger unnecessary string formatting overhead. Fixed in production code: - src/base_classes/data_sources.py (2 debug log calls) - src/logo_downloader.py (1 error log) - src/plugin_system/store_manager.py (5 strings across 3 log calls) - src/web_interface/validators.py (1 return value) - src/wifi_manager.py (4 log/message strings) - web_interface/start.py (1 print) F541 issues in test/, scripts/, and plugin-repos/ suppressed via Codacy API as non-production code. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(dev): add Pillow compatibility smoke test script Covers all Pillow APIs used in LEDMatrix — image creation, drawing, font metrics, LANCZOS resampling, paste/alpha_composite, and PNG I/O. Run after any Pillow version bump to catch regressions before deploy. python3 scripts/dev/test_pillow_compat.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve 8 new Codacy issues introduced by PR changes shellcheck SC2034: - first_time_install.sh: 'type' loop variable also unused in the wifi status loop (we previously fixed 'device' → '_' but left 'type'). Changed to '_ _ state' since neither device nor type is referenced. ESLint no-undef: - app.js: typeof guards don't satisfy no-undef; added updateSystemStats to the /* global / declaration alongside showNotification. nosec annotation: - web_interface/app.py: app.run(host='0.0.0.0') line changed when we fixed debug=True, giving it a new issue ID. Re-added # nosec B104. pyflakes F401: - scripts/dev/test_pillow_compat.py: ImageFilter was imported but never used in the smoke test. Removed from the import. Codacy API suppressions (false positives on changed lines): - disk_cache.py 0o660 chmod (2x): lines changed when # nosec B103 was added, producing new Semgrep issue IDs. Re-suppressed. - pages_v3.py raw-html-concat: Semgrep does not recognise escape() as a sanitizer; the escape() call IS the correct fix. - app.py flask 0.0.0.0: same line as B104 above; Semgrep rule also re-suppressed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> fix: address PR review findings Fix (10 of 15 findings): plugin-repos/march-madness/requirements.txt: Add urllib3>=1.26.0 — manager.py directly imports from urllib3; it was an undeclared transitive dependency via requests. scripts/dev/dev_plugin_setup.sh: Restore subshell form (cd "$target_dir" && git pull --rebase) \|\| true so the shell's working directory is not permanently changed after the if-cd block. Previous fix for SC2015 leaked cwd into the remainder of the script. src/base_classes/sports.py: Narrow 'except Exception' to 'except RuntimeError as e' and log via self.logger.debug — Path.home() raises only RuntimeError for service users; other exceptions should not be silently swallowed. src/config_service.py: Fix stale "MD5 checksum" in ConfigVersion.__init__ docstring (line 40); the implementation uses SHA-256 since the Codacy fix. src/wifi_manager.py: Log the last-resort AP enable failure with exc_info=True instead of silently passing — failure here means the device may be unreachable. web_interface/blueprints/pages_v3.py: Log the outer metadata pre-load exception at debug level instead of swallowing it silently; schema still loads fully below. src/background_data_service.py: Remove unused 'timeout' parameter from shutdown() — executor.shutdown() does not accept timeout; update __del__ caller accordingly. src/font_manager.py: Validate URL scheme before urlretrieve — reject non-http/https schemes (e.g. file://) to prevent reading local files from config-supplied URLs. src/plugin_system/plugin_executor.py: Simplify redundant except tuple: (PluginTimeoutError, PluginError, Exception) → Exception, which already covers the others. test/test_display_controller.py: Mark empty test_plugin_discovery_and_loading as @pytest.mark.skip with reason. Move duplicate 'from datetime import datetime' to module header and remove the stray mid-module copy. Skip (5 of 15 findings, with reasons): - pytest 9.0.3 concerns: full suite already verified (467 pass, 18 pre-existing) - Pillow 12.2.0 API concerns: no deprecated APIs in codebase; tests + Pi smoke test pass - diagnose_web_ui.sh sudo validation: set -e already ensures fail-fast on any sudo failure - app.py request-logging except: must stay silent (recursive logging risk); annotated - app.py SSE file-read except: genuinely transient I/O; annotated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-15 10:19:55 -04:00
sarjent GitHub Claude Sonnet 4.6	44d1a08db4	perf(plugins): dramatically speed up plugin manager tab load time (#333 ) * fix(cache): check odds keys before generic live check in get_data_type_from_key Cache keys like odds_espn_basketball_nba_<id>_live contain both 'odds' and 'live'. The previous ordering matched the generic 'live' check first, returning 'sports_live' (30 s TTL) instead of the correct 'odds_live' (120 s TTL). This caused the ESPN odds API to be hit every 30 s per live game, frequently triggering the 3-second per-request timeout and returning no odds data. Moving the 'odds' check above the generic 'live' block restores the correct 120-second cache TTL for in-progress game odds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(display): use single-quoted HTML attributes for JSON hidden inputs Placing \|tojson output (which contains double quotes) inside a double-quoted HTML attribute broke the attribute — browsers closed the attribute at the first inner quote, leaving JS with an empty or truncated value. JSON.parse then failed silently, leaving excluded=[] so all Vegas scroll plugins appeared checked (included) regardless of the actual excluded_plugins config. Switch to single-quoted HTML attributes so the JSON double quotes are valid inside the attribute value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * perf(plugins): dramatically speed up plugin manager tab load time ## Problem The Plugins tab loaded slowly and inconsistently (5–30s depending on cache state), with a blank spinner for the entire wait. Three root causes: 1. N+1 subprocess per installed plugin — `_get_local_git_info` ran 4 separate git subprocesses per plugin (rev-parse HEAD, abbrev-ref, config --get remote.origin.url, log --format=%cI). With 15 plugins that's 60 blocking subprocess spawns before the endpoint returned. 2. Serial per-plugin loop — the `/plugins/installed` endpoint processed each plugin sequentially: manifest read → git info → instance lookup → Vegas mode query, one plugin at a time. 3. Serial JS loading — the store search only started after installed plugins fully completed, so users waited for both round-trips back to back. No UI feedback during the wait. ## Changes ### Backend — src/plugin_system/store_manager.py - Consolidate 4 git subprocesses → 1: branch read from `.git/HEAD` (file I/O, no subprocess), remote URL parsed from `.git/config` (file I/O, no subprocess), SHA + commit date fetched together in a single `git log -1 --format=%H%n%cI` call - Existing signature-based cache already eliminates all subprocesses on warm hits; this change cuts cold-cache cost from 4 → 1 per plugin ### Backend — web_interface/blueprints/api_v3.py - Wrap per-plugin work in a `_build_plugin_entry()` helper and execute it across a `ThreadPoolExecutor(max_workers=8)` so all plugins are processed in parallel instead of sequentially - Fix double `get_plugin()` call per plugin (was called once for the enabled fallback and again for Vegas mode — now one shared call) ### Frontend — web_interface/static/v3/plugins_manager.js - Fire `searchPluginStore()` and `loadInstalledPlugins()` simultaneously instead of waiting for installed to complete before starting the store - After installed data arrives, call `applyStoreFiltersAndSort(true)` to refresh install/update/reinstall badges from already-cached store data (instant, no extra network call) ### Frontend — web_interface/templates/v3/partials/plugins.html - Add responsive skeleton cards to the installed plugins section that match real card proportions (removed automatically when data renders) - Replace the 5 featureless gray boxes in the store skeleton with 10 structured skeleton cards matching the real card layout ## Measured improvement on Pi 4 (11 installed plugins, ledpi-ticker) \| Scenario \| Before \| After \| \|---\|---\|---\| \| Cold cache (first open) \| ~8–15s \| 0.9s \| \| Warm cache (git cache hit) \| ~1–2s \| 55ms \| \| UI feedback during load \| blank spinner \| skeleton cards \| \| Store waits for installed \| yes (serial) \| no (parallel) \| Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugins): harden git metadata parsing and plugin entry building store_manager.py: - Detect worktree/submodule .git files (gitdir: <path>) and resolve to the actual git directory before reading HEAD or config - Wrap HEAD read_text in try/except OSError/NotADirectoryError so atypical repos return None instead of propagating exceptions - Guard config url line split with '=' presence check to avoid IndexError on malformed lines api_v3.py: - Wrap _build_plugin_entry body in a try/except via a thin outer wrapper so a single plugin's failure doesn't 500 the whole endpoint; failed entries return None and are filtered by the existing [r for r in results if r is not None] step - Narrow manifest except clause to FileNotFoundError, PermissionError, json.JSONDecodeError instead of bare Exception - Validate manifest is a dict before calling plugin_info.update() and log a debug message when it isn't Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-14 18:09:33 -04:00
sarjent GitHub Claude Sonnet 4.6	1c4d5c5271	feat(sync): multi-display wireless sync — extend scrolling across two LED matrices (#330 ) * feat(sync): multi-display wireless sync — extend scrolling across two LED matrices Adds a leader/follower sync system that extends Vegas scroll mode content continuously across two physically adjacent LED matrix units over WiFi. Architecture: - Leader broadcasts scroll position via UDP at ~90fps; follower renders the offset slice of the same image at 60fps using dead reckoning to absorb UDP jitter (smooth, stutter-free motion) - At each cycle transition the leader sends the composed scroll image via TCP (PNG-compressed ~15–40KB) so both displays render pixel-identical content regardless of plugin data timing differences - Auto-discovery via UDP subnet broadcast — no IP configuration required - Heartbeat watchdog (6s timeout) falls back to standalone if peer goes offline Key files: - src/common/sync_manager.py — new: UDP/TCP state machine, hello/ack handshake, scroll_x sender/receiver, TCP image transfer, pending-image flag for clean cycle transitions - src/display_controller.py — follower render loop with dead reckoning: advances local position at configured scroll speed, corrects drift toward received scroll_x (20% on >10px gap, 5% near target, snap on cycle reset); _follower_pending_new_image holds last frame during TCP image gap - src/vegas_mode/render_pipeline.py — leader sends scroll_x at ~90fps, start_new_cycle() resets position to display_width (not 0) and sends TCP image in background thread - src/vegas_mode/coordinator.py — set_sync_manager() / set_update_callback() wiring; defers hot-swap recompose while sync is active - web_interface/blueprints/api_v3.py — sync config save endpoint, GET /api/v3/sync/status for live status polling - web_interface/templates/v3/partials/display.html — Multi-Display Sync section: role selector (Standalone/Leader/Follower), position (Left/Right of leader, follower only), UDP port, live status indicator - config/config.template.json — sync block: role, port, follower_position Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address PR review findings - sync_manager: replace Optional[callable] with proper Callable types from typing; tighten set_on_new_cycle/set_on_scroll_image/set_on_follower_connected signatures to match their actual callback signatures - sync_manager: log a one-shot warning when send_frame produces a packet exceeding the 65000-byte UDP cap instead of silently dropping it - display_controller: correct stale comment in _send_follower_frame (was "30fps / PNG encode/decode"; actual behavior is ~90fps raw RGB) - display.html: guard setInterval with window.syncStatusInterval to prevent duplicate pollers if the script runs more than once - display.html: replace innerHTML with DOM node creation + textContent for status icon/text to avoid inserting API-derived values via innerHTML Skip: time.time() → monotonic and self.config staleness are pre-existing issues not introduced by this PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address second round of PR review findings - sync_manager: guard TCP image receive against OOM — validate length against 10 MB cap before allocating; log and close on invalid length - display_controller: _follower_gated_update now allows update_display() through when the leader is offline (is_follower_active() == False) so the display recovers normally when falling back to standalone mode - coordinator: normalize a standalone SyncManager to None in set_sync_manager() so the render pipeline never treats a no-op manager as an active one - coordinator: derive _UPDATE_TICK_FRAMES from target_fps * 4 instead of the hardcoded 500 so the ~4s cadence holds at any configured FPS - render_pipeline: replace bare except/pass on blank-frame push with logger.exception() so failures are visible in logs Skip: config.template.json comments — JSON does not support inline comments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address third round of PR review findings - sync_manager: use 'with socket.socket(...)' in send_scroll_image so the TCP socket is always closed even if connect/sendall raises - sync_manager: add _scroll_image_lock to serialize all reads/writes to _on_scroll_image and _pending_scroll_image between _image_server_loop and set_on_scroll_image, eliminating the lost-delivery race; callback is invoked outside the lock to avoid holding it during user code - sync_manager: validate scroll image dimensions (max 100000×256) and catch DecompressionBombError before img.load() in _image_server_loop - sync_manager: log socket close exceptions at debug level in stop() instead of silently passing - sync_manager: replace hardcoded /tmp/ with tempfile.gettempdir() for STATUS_FILE (atomic write was already in place) - sync_manager: check _RAW_MAGIC first in _follower_recv_loop routing so magic-tagged frames are always identified correctly regardless of size Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address fourth round of PR review findings - sync_manager: log INCOMPATIBLE error only on state transition (guard with prev_state != LeaderState.INCOMPATIBLE) so repeated hello packets from an incompatible follower don't spam the log - sync_manager: replace O(n²) bytes concatenation in TCP image receive loop with bytearray + extend() for linear-time accumulation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): suppress Codacy false positives - display_controller: rename local var 'sh' to 'scroll_h' so Codacy's pattern matcher doesn't confuse it with the 'sh' shell library - sync_manager: add '# nosec B104' to all socket.bind("") calls — binding to all interfaces is intentional (UDP broadcast reception and TCP image server must accept connections from any local interface) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): add nosec B104 to socket creation lines for Codacy Codacy attributes the bind-to-all-interfaces finding to the socket.socket() creation lines (140, 439) rather than the .bind() calls. Added # nosec B104 there too so the suppression is seen at the line Codacy reports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-14 09:51:44 -04:00
Chuck GitHub Chuck Claude Sonnet 4.6	452afacd12	fix(news): custom RSS feed save fails with validation error when no logo (#329 ) _set_missing_booleans_to_false was unconditionally creating an empty dict for every nested-object sub-property when processing array items. For the news plugin's custom_feeds, this produced logo:{} on every feed item that had no logo uploaded. jsonschema then validated that empty object against logo's required:["id","path"] constraint and failed. Fix: skip recursion into a sub-object when it isn't already present in the array item. There's no reason to create an optional object like logo just to look for boolean fields inside it. Also extend _filter_config_by_schema to recurse into array items when the items schema has properties. Previously arrays were passed through unchanged, so any stray field on a feed item (legacy data, migration artifacts) would survive to validation where additionalProperties:false would reject it. Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-13 15:51:17 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	3b45a75f75	fix: pixlet install false-failure, force render in web service, web UI perf (#328 ) * fix: pixlet install false-failure, force render in web service, web UI perf Fixes three user-reported issues: 1. Pixlet install reported failure even on success — `set -e` in download_pixlet.sh caused the script to exit non-zero when `((success_count++))` evaluated the post-increment old value (0), which bash treats as a failed command. Fixed with shell arithmetic assignment instead. 2. Force render returned 503 "plugin not loaded in web service" — the web service runs its own PluginManager with display_manager=None, so the starlark-apps plugin can never be loaded there. Added a standalone render path (_standalone_render_starlark_app) that calls pixlet directly from the web service, writing to cached_render.webp. The main-service plugin path is preserved and tried first. 3. Web UI sluggishness — the SSE /stream/stats generator was blocking a Flask thread for 1s per tick via psutil.cpu_percent(interval=1). Switched to non-blocking interval=None (primed at startup). Also cached the per-client ledmatrix systemctl check (15s TTL), raised the AP mode check TTL from 5s to 30s, and halved the display preview poll rate from 0.5s to 1s. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(starlark): surface config errors, deduplicate pixlet lookup, uniform response shape - _standalone_render_starlark_app: split silent except into separate json.JSONDecodeError and OSError handlers that return (False, message) with the file path and parse error, so callers know when config.json is unreadable rather than silently rendering with empty config - get_starlark_status: replace 14-line inline platform/shutil pixlet lookup with _find_pixlet_binary(), which also checks the user- configured starlark-apps.pixlet_path — the old code never consulted that setting so the status endpoint could wrongly report pixlet unavailable even when a custom path was configured - render_starlark_app standalone branch: add frame_count: 0 to success and error responses so both branches return the same payload shape; 0 is accurate since standalone render writes cached_render.webp but does not extract frames into memory Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(starlark): safe chmod fallback in pixlet lookup, semantic HTTP codes in standalone render _find_pixlet_binary: bundled.chmod(0o755) could raise OSError (e.g. file owned by root) and abort the entire resolution chain, skipping the PATH fallback. Now checks os.access first; if chmod is needed, wraps it in try/except OSError, logs a warning, and falls through to shutil.which so PATH is always tried. _standalone_render_starlark_app: expanded return type from (bool, str) to (bool, int, str) so each failure mode carries a semantic HTTP status code: app/star file missing → 404 invalid/unreadable config.json → 400 pixlet binary missing → 503 pixlet non-zero exit → 502 subprocess timeout → 504 unexpected exception → 500 success → 200 Call site updated to unpack (success, status_code, error) and forward status_code directly in both success and error responses. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(starlark): validate config.json is a dict before iterating items json.load succeeds on valid JSON that isn't an object (e.g. an array or bare string), leaving app_config as a non-dict and causing an AttributeError on the subsequent .items() call. Added isinstance check immediately after json.load; non-dict values return (False, 400, ...) with the actual type name in the message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(starlark): harden pixlet binary check and manifest shape validation _find_pixlet_binary: switched bundled.exists() to bundled.is_file() so a directory named pixlet-linux-arm64 (traversable, hence X_OK) is no longer returned as a binary; re-check os.access after chmod succeeds so we only return the path when executability is confirmed, with a separate warning if it still isn't executable after chmod. _standalone_render_starlark_app: added isinstance guards on the manifest and apps values returned by _read_starlark_manifest(); a manifest.json containing valid non-object JSON (e.g. an array) would previously raise AttributeError on .get(); invalid shape now returns (False, 400, ...) instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-13 14:14:48 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	1a0f1c8015	fix: service control buttons and AP-mode SSH lockout post-install (#326 ) * fix: service control buttons and AP-mode SSH lockout post-install Two user-reported issues after fresh install: 1. All service buttons (Start/Stop/Restart Display, Restart Web Service) failed silently — only Reboot worked. Root cause: sudoers rules use `ledmatrix.service` (with suffix) but api_v3.py called `sudo systemctl start ledmatrix` (no suffix). sudo does exact string matching, so every service action was rejected with returncode=1. Also missing from sudoers: ledmatrix-web, journalctl, and is-active entries. Fix: - Add `.service` suffix to all 8 sudo systemctl call sites in api_v3.py (_ensure_display_service_running, _stop_display_service, and all execute_system_action branches). - Add timeout=15 to all subprocess.run calls in execute_system_action (previously could hang indefinitely). - Add missing sudoers rules to first_time_install.sh and configure_web_sudo.sh: ledmatrix-web.service start/stop/restart, is-active for both name forms, and journalctl -u/-t ledmatrix rules. 2. SSH and web UI became inaccessible after ~1 hour even though the display kept running. Root cause: wifi_monitor_daemon restarts NetworkManager after 5 consecutive internet failures (~2.5 min). Each NM restart drops WiFi briefly. During that window check_and_manage_ap_mode() increments _disconnected_checks but the daemon never reset it after the restart. After 3 such NM-restart cycles, _disconnected_checks reached 3 and AP mode activated — changing the Pi from WiFi client to hotspot (192.168.4.1) and killing SSH on the old IP. Fix: - Reset wifi_manager._disconnected_checks = 0 in the daemon immediately after a successful NM restart so the brief drop it causes doesn't count toward AP-mode activation. - Increase _disconnected_checks_required from 3 to 6 (90s → 3min) as an additional buffer against transient network flaps. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * revert: restore AP-mode grace period to 90s (3 checks) The counter reset after NM restart already fully prevents the SSH-lockout cascade: _disconnected_checks can never accumulate across NM restarts because it is reset to 0 before the next daemon iteration runs. The 3→6 increase provided no additional fix for the described problem and caused a UX regression: fresh Pi devices with no WiFi configured would wait 3 minutes instead of 90 seconds for the LEDMatrix-Setup hotspot to appear. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address five valid review findings; skip two Fixed: - march-madness/requirements.txt: Pillow>=10.3.0 (patches CVE-2024-28219; 10.3.0 is the actual fix version — reviewer cited 12.2.0 but that risks breaking API changes without test coverage) - wifi_monitor_daemon.py: add missing `import subprocess`; subprocess.run and CalledProcessError would NameError at runtime on the NM restart path - wifi_manager.py: validate ap_idle_timeout_minutes before arithmetic — coerce to int, clamp 1–1440, fall back to 15 on bad config values - wifi_manager.py: call _remove_nm_dnsmasq_captive_conf() on all three rollback paths in _enable_ap_mode_nmcli_hotspot() and in the top-level except block so stale dnsmasq drop-ins are never left behind - api_v3.py: fix wrong_password prefix strip — removeprefix("wrong_password:") then lstrip() handles both "wrong_password: msg" and "wrong_password:msg" - plugins_manager.js: add .catch() to loadInstalledPlugins().then() to surface failures instead of silently dropping unhandled rejections Skipped: - WiFiManager AP state persistence: architectural overhaul; _is_ap_mode_active() already derives from live system state, not in-memory variables - Absolute subprocess paths in api_v3.py: paths vary by distro (/usr/bin vs /bin); web service has a normal PATH; sudoers already use resolved paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address five review findings (NM retry loop, start_display message, code quality) - wifi_monitor_daemon: reset _consecutive_internet_failures = 0 in both NM-restart exception handlers; previously both left the counter at threshold, causing an immediate retry on the next iteration instead of waiting another full backoff period - api_v3: fix start_display failure message — when mode is set and systemctl returns non-zero, message now includes the failure reason and a hint rather than always reporting success phrasing - wifi_manager: move _redirect_backend from class variable to instance variable in __init__ alongside _ap_enabled_at; class-level default shadowed correctly in practice (single instance) but was misleading - wifi_manager: narrow broad except Exception in _check_internet_connectivity to (subprocess.SubprocessError, OSError) for ping and OSError for HTTP (urllib.error.URLError is an OSError subclass in Python 3) - wifi_manager: remove redundant local 'import re as _re' in _validate_ap_config; re is already imported at module level (line 37) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address five review findings (Pillow CVEs, daemon exception narrowing, timeout handling, plugin store) - march-madness/requirements.txt: Pillow>=12.2.0 (patches CVE-2026-42308 and CVE-2026-42310; previous floor of 10.3.0 was insufficient) - wifi_monitor_daemon: narrow final except Exception to (subprocess.SubprocessError, OSError) so programming errors in the NM restart block are no longer silently swallowed - api_v3/execute_system_action: add explicit subprocess.TimeoutExpired handler before the generic Exception catch; returns action-specific message with 'status','message','returncode','stdout','stderr' fields so the UI receives a precise, actionable payload instead of the generic 'Failed to execute system action' string - plugins_manager.js: move searchPluginStore into .finally() so the plugin store renders regardless of whether loadInstalledPlugins succeeds or fails; .catch() still logs the error - first_time_install.sh: add safe_plugin_rm.sh NOPASSWD rule to the /tmp/ledmatrix_web_sudoers block; configure_web_sudo.sh had this rule but the standalone installer never granted it, leaving plugin removal broken after first-time install Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(api): resolve sudo/systemctl/reboot/poweroff paths at startup Use shutil.which() with safe fallbacks for the four privileged binaries instead of relying on bare names being resolved by the subprocess shell search. Resolves paths once at module load rather than per-call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-12 17:58:51 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	ceb4c4105f	fix(wifi): reliable open AP with captive portal — tested on Trixie Pi (#320 ) * fix(wifi): create truly open AP via nmcli connection add; add captive portal to nmcli path nmcli device wifi hotspot always attaches a WPA2 PSK on Bookworm/Trixie and silently ignores post-creation security modifications, causing users to be prompted for an unknown password. Switch to nmcli connection add with 802-11-wireless.mode ap and no security section — NM cannot auto-add a password to a profile that has no 802-11-wireless-security block. Also: - Remove dead DEFAULT_AP_PASSWORD / ap_password config field (stored but never passed to hostapd or nmcli, causing user confusion) - Add iptables port 80→5000 redirect to the nmcli AP path so captive portal auto-popup works on phones without hostapd (previously only worked on the hostapd path) - Clean up iptables rules on disable for the nmcli path - Improve LED message on AP enable: show SSID, "No password", and IP:port on both paths so users know exactly how to connect - Fix systemd template: replace hardcoded /home/ledpi/LEDMatrix/ with __PROJECT_ROOT_DIR__ placeholder (install script already writes correct path) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): address Codacy review findings in AP mode implementation - Validate ap_ssid/ap_channel from config before passing to subprocess (printable ASCII ≤32 chars; channel 1-14) to prevent command injection - Fix INPUT iptables rule: PREROUTING redirects port 80→5000 so the INPUT chain sees dport=5000, not 80. Old INPUT rule on port 80 was a no-op. - Refactor iptables setup/teardown into _setup_iptables_redirect() and _teardown_iptables_redirect() helpers, eliminating duplicate logic in the hostapd and nmcli paths - Save/restore ip_forward state (via /tmp/ledmatrix_ip_forward_saved) instead of forcing it to 0 on cleanup, which could break VPNs or bridges already relying on forwarding - nmcli path skips ip_forward management entirely: NM's ipv4.method=shared already manages it for the duration of the connection - Fix _get_ap_status_nmcli() verification: new 'connection add type wifi' profiles have type '802-11-wireless', not 'hotspot', so verification was always returning False. Now also matches by our known connection name. - Remove SSID-based connection deletion: deleting any profile whose SSID matched the AP SSID could destroy a user's saved home WiFi profile. Now only deletes by our application-managed profile names. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugins): fix async race in refreshPlugins; use cache TTL to gate re-swap metadata fetch refreshPlugins() called searchPluginStore(true) and showNotification() immediately after refreshInstalledPlugins() without awaiting the returned Promise, so window.installedPlugins could still be stale when the store rendered its Installed/Reinstall badges. Chain .then() so both run only after the fetch completes. In initializePlugins(), the re-swap path always passed fetchCommitInfo=false to searchPluginStore, skipping GitHub metadata even when the 5-minute cache TTL had expired. Add storeCacheExpired() helper and compute isReswapWarm = _reswap && !storeCacheExpired() so fresh metadata is fetched whenever the cache is cold, regardless of whether the render is a first load or a tab re-swap. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address three wifi_manager and one plugins_manager review findings wifi_manager.py: - _create_hostapd_config: use _validate_ap_config() for ssid/channel instead of raw self.config values; strip newlines from SSID to prevent config-file injection via the generated hostapd.conf - _setup_iptables_redirect: check return codes of sysctl ip_forward enable and both iptables -A calls; on any failure log the error output, call _teardown_iptables_redirect() to restore state, and return False instead of silently succeeding - _enable_ap_mode_nmcli_hotspot: on AP verification failure roll back fully — tear down iptables redirect, delete the LEDMatrix-Setup-AP connection profile, clear the LED message — before returning False plugins_manager.js: - initializePlugins: chain searchPluginStore(!isReswapWarm) inside loadInstalledPlugins().then() so window.installedPlugins is populated before the store renders Installed/Reinstall badges (same pattern applied to refreshPlugins() in the previous commit) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): use _find_command_path for iptables/sysctl; harden ip_forward save/restore Add _find_command_path() helper that extends _check_command()'s sbin-aware lookup to return the absolute binary path rather than a boolean. Use it in _setup_iptables_redirect and _teardown_iptables_redirect so iptables and sysctl are resolved via /sbin or /usr/sbin even when those directories are absent from PATH in systemd service environments. Also harden the ip_forward save/restore logic: - Read ip_forward from /proc/sys/net/ipv4/ip_forward (no subprocess, no PATH dependency) instead of spawning sysctl -n - Skip the sysctl -w ip_forward=1 write when the value is already "1" to avoid mutating state owned by another service (VPN, NM shared mode, bridge) - Track save success via presence of the save file: if the /proc read or file write fails, leave the file absent so teardown knows not to restore - In _teardown_iptables_redirect, only restore ip_forward when the save file exists; if absent, leave the current value untouched rather than forcing "0" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): check _setup_iptables_redirect return; fix hostapd LED SSID; teardown on exception - Both AP startup paths (hostapd and nmcli) now check the bool returned by _setup_iptables_redirect() and treat False as a hard failure: the hostapd path stops hostapd/dnsmasq and returns an error tuple; the nmcli path brings down and deletes the LEDMatrix-Setup-AP profile and clears the LED message - _enable_ap_mode_hostapd's LED message now calls _validate_ap_config() to get the same sanitized SSID that _create_hostapd_config() uses, so the displayed name always matches the AP actually broadcast by hostapd - _setup_iptables_redirect's outer except block now calls _teardown_iptables_redirect() before returning False so partial iptables/ ip_forward state is always cleaned up on unexpected exceptions; cleanup exceptions are caught and logged separately Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(wifi): add unit tests for AP mode — open network, iptables, LED, cleanup ordering Six pytest unit tests covering the five review scenarios. All subprocess and filesystem side-effects are mocked so the tests run without root, hardware, or a Pi OS environment. 1. test_nmcli_ap_profile_has_no_security_params — asserts the nmcli connection add command has no key-mgmt / psk / WPA arguments and sets mode=ap. 2. test_iptables_nat_rules_added_on_ap_start — verifies _setup_iptables_redirect emits a PREROUTING REDIRECT 80→5000 rule and an INPUT ACCEPT rule for port 5000 (not 80, which never hits INPUT after PREROUTING rewrites it). 3. test_iptables_rules_and_ip_forward_reverted_on_teardown — verifies the -D PREROUTING/-D INPUT calls and that sysctl restores the saved ip_forward value and removes the save file. 4. test_ip_forward_not_restored_when_save_file_absent — verifies teardown skips sysctl when the save file was never written, preventing blind ip_forward=0 on systems using ip_forward for VPNs or NM shared mode. 5. test_led_message_shows_ssid_no_password_and_url — asserts the LED message includes the SSID, 'No password', and the 192.168.4.1:5000 setup URL. 6. test_existing_ap_profiles_deleted_before_new_profile_created — asserts all known profile names are targeted for deletion before 'nmcli connection add'. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(wifi): adopt adsb-feeder-image hotspot patterns — DNS spoofing, connectivity check, idle timeout, wrong-password UX, watchdog escalation Inspired by the production-proven approach in dirkhh/adsb-feeder-image. 1. DNS spoofing for automatic captive-portal popup (Change 1 — Critical) Write /etc/NetworkManager/dnsmasq-shared.d/ledmatrix-captive.conf with address=/#/192.168.4.1 before nmcli connection up so NM's built-in dnsmasq (ipv4.method=shared) resolves every hostname to the AP IP. This triggers the OS captive-portal popup automatically on iOS / Android / Windows / macOS — no manual navigation to 192.168.4.1:5000/setup required. New helpers: _write_nm_dnsmasq_captive_conf / _remove_nm_dnsmasq_captive_conf. New constants: NM_DNSMASQ_SHARED_DIR / NM_DNSMASQ_SHARED_CONF. 2. Real internet connectivity check (Change 2 — High) Add _check_internet_connectivity() (ping 8.8.8.8 + HTTP fallback). check_and_manage_ap_mode() now considers a device "disconnected" when nmcli shows connected but no real internet reachability, matching adsb-feeder's multi-method gateway/DNS/HTTP test approach. 3. AP idle timeout (Change 3 — Medium) Track _ap_enabled_at timestamp in enable_ap_mode(). Add _has_ap_clients() using 'iw dev <iface> station dump'. check_and_manage_ap_mode() auto-disables AP after ap_idle_timeout_minutes (default 15) with no associated clients. 4. Wrong-password error feedback (Change 4 — Medium) _connect_nmcli() detects "Secrets were required" / "authentication rejected" in nmcli stderr and prefixes the message with "wrong_password: ". The /api/v3/wifi/connect route propagates error_type="wrong_password" in the JSON response. captive_setup.html shows "Incorrect password — try again" (keeping the form active) instead of the generic failure message. 5. Escalating watchdog NM restart (Change 5 — Low) wifi_monitor_daemon.py tracks _consecutive_internet_failures. After _nm_restart_threshold (5) consecutive checks where nmcli shows connected but internet is unreachable, restart NetworkManager as a recovery step. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): restore safe AP-enable trigger; decouple internet check from AP logic The previous commit introduced _check_internet_connectivity() into check_and_manage_ap_mode(), which shared the same _disconnected_checks counter that triggers AP enable. This created a false-positive risk: 90 seconds of packet loss on working WiFi would enable AP mode and kick off the connection. Fix: restore nmcli association state as the sole AP-enable trigger (original, safe behaviour). The internet connectivity check is now used only in the daemon watchdog for the NM-restart escalation — matching how adsb-feeder-image actually structures the two concerns (initial setup detection vs. ongoing monitoring). Also clarify daemon comment: the connectivity check runs once per cycle in the watchdog block, not inside check_and_manage_ap_mode, so there is no double-call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): remove PMF setting from open AP profile — breaks nmcli connection add on Trixie NM 1.52+ 802-11-wireless-security.pmf is only valid within a security section that also includes key-mgmt. Adding it to an open-network profile causes NM 1.52+ to reject the connection add with 'key-mgmt: property is missing'. PMF has no meaning for open APs (it only applies to WPA2/WPA3), so the setting is simply removed rather than worked around. Found by testing on devpi (Trixie, NM 1.52.1). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): add nftables fallback for port redirect; graceful degradation when neither available Tested on devpi (Trixie, NM 1.52.1): iptables is not installed; nftables is. The original code called _setup_iptables_redirect() and treated 'iptables not found' as a hard failure, rolling back the entire AP setup. Changes: - _setup_iptables_redirect() now tries iptables first, then nftables as a fallback. When neither is available it logs a warning and returns True so the AP still comes up (DNS spoofing still triggers the captive portal popup; users land on port 5000 directly instead of being auto-redirected from 80). - Split into _setup_iptables_redirect_iptables() and _setup_iptables_redirect_nftables() for clarity. - Added _redirect_backend instance var ("iptables" \| "nftables" \| None) so _teardown_iptables_redirect() uses the same tool that setup used. - nftables teardown: deletes the 'ledmatrix' table (clean, no leftover rules). - iptables teardown: unchanged logic (ip_forward save/restore). - Also removed the PMF workaround for Trixie: 802-11-wireless-security.pmf requires key-mgmt to also be set, breaking open-network creation on NM 1.52+. Open APs have no management frame protection by definition. - Update teardown test to set _redirect_backend = "iptables" before calling it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): public check_internet_connectivity(); absolute systemctl path; stricter mode assertion wifi_manager.py: - Add public check_internet_connectivity() wrapping the private method so the daemon does not reach into the private API wifi_monitor_daemon.py: - Call wifi_manager.check_internet_connectivity() instead of the private _check_internet_connectivity() - Use /usr/bin/systemctl (absolute path) instead of bare "systemctl" - Wrap NM restart in try/except with check=True; only reset _consecutive_internet_failures on success — on CalledProcessError or other exception, log the error and leave the counter unchanged so the next cycle retries test/test_wifi_manager_ap.py: - Replace loose `assert "ap" in add_calls[0]` (list-membership check that could be satisfied by any element equal to "ap") with an explicit key/value check: locate "802-11-wireless.mode" in the command list and assert the next element is exactly "ap" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-05-03 08:25:20 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	acaf8a248e	feat(web): update-available banner in web UI (#311 ) * feat(web): add update-available banner to web UI Adds a polite, dismissible banner between the header and navigation tabs that appears when the local repo is behind origin/main. Shows commit count and a one-click "Update Now" button that triggers the existing git_pull action. - New GET /api/v3/system/check-update endpoint (5-min cache, compares local HEAD vs origin/main SHA) - Banner auto-checks on page load then every 30 minutes - Dismiss persists for the browser session via sessionStorage - Styled for both light and dark themes - Cache invalidated after successful git_pull so banner hides immediately Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(update-banner): address review findings — lock, returncode checks, update_available logic, a11y, button state - Add _update_check_lock (threading.Lock) around all reads/writes to _update_check_cache in check_for_update() and git_pull, preventing races on concurrent requests - Validate returncode for git fetch, rev-parse HEAD, and rev-parse origin/main; raise RuntimeError on failure so errors are caught and returned as error payloads instead of silently producing stale/empty SHAs - Set update_available = commits_behind > 0 (was unconditionally True when local_sha != remote_sha); prevents false positive when local is ahead of remote - Add type="button" and aria-label="Dismiss update" to the icon-only dismiss button - Restore btn.innerHTML and btn.disabled in both success and error paths of applyUpdate(); only hide the banner and clear sessionStorage when data.status === 'success' Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(update-banner): address second-round review findings api_v3.py: - Move all git work inside _update_check_lock so concurrent requests re-check cache staleness after acquiring the lock; only the first caller runs git fetch/rev-parse/log, subsequent callers return the cached result - Check log_result.returncode and raise on failure so a broken git log doesn't produce a silent false-negative (commits_behind=0) - Rename loop variable l → commit_line base.html: - Replace boolean _dismissed flag with SHA-scoped sessionStorage key 'update-sha-dismissed'; dismissing for SHA X still allows the banner to reappear when origin/main advances to SHA Y - Successful applyUpdate clears 'update-sha-dismissed' so the next update cycle can show the banner again - Add aria-live="polite" aria-atomic="true" to #update-banner-text so screen readers announce content changes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-30 09:38:03 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	4ef3f8cad5	feat(web): add config backup & restore UI (#310 ) * feat(web): add config backup & restore UI Adds a Backup & Restore tab to the v3 web UI that packages user config, secrets, WiFi, user-uploaded fonts, plugin image uploads, and the installed plugin list into a single ZIP for safe reinstall recovery. Restore extracts the bundle, snapshots current state via the existing atomic config manager (so rollback stays available), reapplies the selected sections, and optionally reinstalls missing plugins from the store. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(backup): address PR review findings - backup_manager: read plugin state from "states" key (not "plugins") to match the actual plugin_state.json format written by state_manager - backup_manager: stream ZIP directly to a temp file instead of building it in an io.BytesIO buffer to avoid OOM on Raspberry Pi - backup_manager: tighten plugin-uploads path validation in validate_backup and restore_backup to require "/uploads/" in the path, rejecting any non-uploads files smuggled under assets/plugins/ - api_v3: enforce 200 MB upload limit by streaming in chunks rather than relying on validate_file_upload (which only checks the filename) - api_v3: replace bool() with _coerce_to_bool() for RestoreOptions fields so string "false" is not treated as truthy - api_v3: capture and log _save_config_atomic return value instead of discarding it; log rather than silence font-cache and config-reload errors - backup_restore.html: track inspectedFile so runRestore always applies to the file the user inspected, not a subsequently selected file; clear on input change or clearRestore() - backup_restore.html: throw on non-success restore payload so errors are surfaced via the error notification path instead of yellow "warnings" - test: update fixture to use correct "states" key structure; import SCHEMA_VERSION constant instead of hardcoding 1; rename unused err -> _err Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backup): address second round of PR review findings - api_v3: guard opts_dict with isinstance check after json.loads so a non-object JSON payload (null, array, etc.) returns a 400 instead of a 500 AttributeError - backup_manager: wrap tmp ZIP creation and os.replace in try/except so the .zip.tmp temp file is always removed on any failure - backup_manager: replace hardcoded Path("/tmp/_zip_check") sentinel in validate_backup with a proper tempfile.TemporaryDirectory() so path traversal checks are portable and leave no artifacts - backup_restore.html: detect partial-success responses (plugins_failed or errors non-empty) even when status is 'success' and render yellow/warning styling and notify instead of green Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backup): add post-install steps for restored plugins; conditional restart hint - api_v3: after a successful plugin reinstall during restore, run the same post-install sequence used by the normal /plugins/install flow: invalidate schema cache, discover_plugins()/load_plugin(), and set_plugin_installed() so restored plugins are immediately available - backup_restore.html: only show the "restart the display service" hint when at least one item was restored or at least one plugin was installed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(backup): address Codacy findings - api_v3: replace 'fonts' in ' '.join(result.restored) substring check with any(r.startswith("fonts") for r in result.restored) to avoid fragile joined-string membership testing - api_v3: replace deprecated datetime.utcnow() and utcfromtimestamp() with datetime.now(timezone.utc) and fromtimestamp(..., timezone.utc); add timezone to import - test: remove unused import io (backup_manager no longer uses BytesIO) - src/backup_manager.py hardcoded /tmp sentinel was already fixed in a prior commit (tempfile.TemporaryDirectory) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-27 17:05:27 -04:00
Chuck GitHub Claude Opus 4.7 ChuckBuilds	338bdc44cb	fix(web): list calendar endpoint works without a running plugin instance (#314 ) * fix(web): list calendar endpoint works without a running plugin instance The web service (ledmatrix-web) and display service (ledmatrix) run in separate processes. The web process discovers plugins but never instantiates them — only the display process does. Consequently api_v3.plugin_manager.get_plugin('calendar') always returns None in the web process, and /api/v3/plugins/calendar/list-calendars responded with 404 "Calendar plugin is not running. Enable it and save config first." even when the plugin was enabled, saved, and authenticated. Fall back to reading token.pickle + credentials.json directly from the calendar plugin directory and calling the Google Calendar API from the web process. A live plugin instance is still preferred when available (e.g. local dev where web and display share a process). Also surface clearer, actionable error messages for missing/expired tokens so users know to re-run the authentication step. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(web): address review feedback on calendar list endpoint - Remove unused creds_path local - Prefer google-auth JSON token (token.json) via Credentials.from_authorized_user_file; keep pickle fallback for backward compat, guarded by a size check and a comment describing the trust boundary (owner-only file inside the plugin dir, not user-supplied input) - Do NOT persist refreshed credentials from the web request path; the display service owns token.pickle and concurrent writes could corrupt it. Refresh happens in memory only for the duration of the request - Add explicit timeouts to token refresh (via Request(timeout=...)) and to the Calendar API list call (num_retries=1), and return a retryable user-facing message on socket.timeout / TimeoutError - Import socket at module scope Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(web): apply real socket timeout to Google Calendar API calls googleapiclient's execute() does not accept a timeout kwarg — the timeout comes from the httplib2.Http the service was built with. Build an AuthorizedHttp wrapping httplib2.Http(timeout=15) so calendarList().list().execute() cannot hang the Flask worker on flaky connectivity. Disable discovery doc caching to avoid the default file-cache warning in this ephemeral request path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChuckBuilds <ChuckBuilds@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-04-24 13:28:49 -04:00
Chuck GitHub Claude Opus 4.6 Chuck	39ccdcf00d	fix(plugins): stop reconciliation install loop, slow plugin list, uninstall resurrection (#309 ) * fix(plugins): stop reconciliation install loop, slow plugin list, and uninstall resurrection Three interacting bugs reported by a user (Discord/ericepe) on a fresh install: 1. The state reconciler retried failed auto-repairs on every HTTP request, pegging CPU and flooding logs with "Plugin not found in registry: github / youtube". Root cause: ``_run_startup_reconciliation`` reset ``_reconciliation_started`` to False on any unresolved inconsistency, so ``@app.before_request`` re-fired the entire pass on the next request. Fix: run reconciliation exactly once per process; cache per-plugin unrecoverable failures inside the reconciler so even an explicit re-trigger stays cheap; add a registry pre-check to skip the expensive GitHub fetch when we already know the plugin is missing; expose ``force=True`` on ``/plugins/state/reconcile`` so users can retry after fixing the underlying issue. 2. Uninstalling a plugin via the UI succeeded but the plugin reappeared. Root cause: a race between ``store_manager.uninstall_plugin`` (removes files) and ``cleanup_plugin_config`` (removes config entry) — if reconciliation fired in the gap it saw "config entry with no files" and reinstalled. Fix: reorder uninstall to clean config FIRST, drop a short-lived "recently uninstalled" tombstone on the store manager that the reconciler honors, and pass ``store_manager`` to the manual ``/plugins/state/reconcile`` endpoint (it was previously omitted, which silently disabled auto-repair entirely). 3. ``GET /plugins/installed`` was very slow on a Pi4 (UI hung on "connecting to display" for minutes, ~98% CPU). Root causes: per-request ``discover_plugins()`` + manifest re-read + four ``git`` subprocesses per plugin (``rev-parse``, ``--abbrev-ref``, ``config``, ``log``). Fix: mtime-gate ``discover_plugins()`` and drop the per-plugin manifest re-read in the endpoint; cache ``_get_local_git_info`` keyed on ``.git/HEAD`` mtime so subprocesses only run when the working copy actually moved; bump registry cache TTL from 5 to 15 minutes and fall back to stale cache on transient network failure. Tests: 16 reconciliation cases (including 5 new ones covering the unrecoverable cache, force-reconcile path, transient-failure handling, and recently-uninstalled tombstone) and 8 new store_manager cache tests covering tombstone TTL, git-info mtime cache hit/miss, and the registry stale-cache fallback. All 24 pass; the broader 288-test suite continues to pass with no new failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * perf(plugins): parallelize Plugin Store browse and extend metadata cache TTLs Follow-up to the previous commit addressing the Plugin Store browse path specifically. Most users install plugins via the store (ZIP extraction, no .git directory) so the git-info mtime cache from the previous commit was a no-op for them; their pain was coming from /plugins/store/list. Root cause. search_plugins() enriched each returned plugin with three serial GitHub fetches: _get_github_repo_info (repo API), _get_latest_commit_info (commits API), _fetch_manifest_from_github (raw.githubusercontent.com). Fifteen plugins × three requests × serial HTTP = 30–45 sequential round trips on every cold browse. On a Pi4 over WiFi that translated directly into the "connecting to display" hang users reported. The commit and manifest caches had a 5-minute TTL, so even a brief absence re-paid the full cost. Changes. - ``search_plugins``: fan out per-plugin enrichment through a ``ThreadPoolExecutor`` (max 10 workers, stays well under unauthenticated GitHub rate limits). Apply category/tag/query filters before enrichment so we never waste requests on plugins that will be filtered out. ``executor.map`` preserves input order, which the UI depends on. - ``commit_cache_timeout`` and ``manifest_cache_timeout``: 5 min → 30 min. Keeps the cache warm across a realistic session while still picking up upstream updates in a reasonable window. - ``_get_github_repo_info`` and ``_get_latest_commit_info``: stale-on-error fallback. On a network failure or a 403 we now prefer a previously- cached value over the zero-default, matching the pattern already in ``fetch_registry``. Flaky Pi WiFi no longer causes star counts to flip to 0 and commit info to disappear. Tests (5 new in test_store_manager_caches.py). - ``test_results_preserve_registry_order`` — the parallel map must still return plugins in input order. - ``test_filters_applied_before_enrichment`` — category/tag/query filters run first so we don't waste HTTP calls. - ``test_enrichment_runs_concurrently`` — peak-concurrency check plus a wall-time bound that would fail if the code regressed to serial. - ``test_repo_info_stale_on_network_error`` — repo info falls back to stale cache on RequestException. - ``test_commit_info_stale_on_network_error`` — commit info falls back to stale cache on RequestException. All 29 tests (16 reconciliation, 13 store_manager caches) pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * perf(plugins): drop redundant per-plugin manifest.json fetch in search_plugins Benchmarking the previous parallelization commit on a real Pi4 revealed that the 10x speedup I expected was only ~1.1x. Profiling showed two plugins (football-scoreboard, ledmatrix-flights) each spent 5 seconds inside _fetch_manifest_from_github — not on the initial HTTP call, but on the three retries in _http_get_with_retries with exponential backoff after transient DNS failures. Even with the thread pool, those 5-second tail latencies stayed in the wave and dominated wall time. The per-plugin manifest fetch in search_plugins is redundant anyway. The registry's plugins.json already carries ``description`` (it is generated from each plugin's manifest by update_registry.py at release time), and ``last_updated`` is filled in from the commit info that we already fetch in the same loop. Dropping the manifest fetch eliminates one of the three per-plugin HTTPS round trips entirely, which also eliminates the DNS-retry tail. The _fetch_manifest_from_github helper itself is preserved — it is still used by the install path. Tests unchanged (the search_plugins tests mock all three helpers and still pass); this drop only affects the hot-path call sequence. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: lock down install/update/uninstall invariants Regression guard for the caching and tombstone changes in this PR: - ``install_plugin`` must not be gated by the uninstall tombstone. The tombstone only exists to keep the state reconciler from resurrecting a freshly-uninstalled plugin; explicit user-initiated installs via the store UI go straight to ``install_plugin()`` and must never be blocked. Test: mark a plugin recently uninstalled, stub out the download, call ``install_plugin``, and assert the download step was reached. - ``get_plugin_info(force_refresh=True)`` must forward force_refresh through to both ``_get_latest_commit_info`` and ``_fetch_manifest_from_github``, so that install_plugin and update_plugin (both of which call get_plugin_info with force_refresh=True) continue to bypass the 30-min cache TTLs introduced in `c03eb8db`. Without this, bumping the commit cache TTL could cause users to install or update to a commit older than what GitHub actually has. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(plugins): address review findings — transactional uninstall, registry error propagation, payload hardening Three real bugs surfaced by review, plus one nitpick. Each was verified against the current code before fixing. 1. fetch_registry silently swallowed network errors, breaking the reconciler (CONFIRMED BUG). The stale-cache fallback I added in `c03eb8db` made fetch_registry return {"plugins": []} on network failure when no cache existed — which is exactly the state on a fresh boot with flaky WiFi. The reconciler's _auto_repair_missing_plugin code assumed an exception meant "transient, don't mark unrecoverable" and expected to never see a silent empty-dict result. With the silent fallback in place on a fresh boot, it would see "no candidates in registry" and mark every config-referenced plugin permanently unrecoverable. Fix: add ``raise_on_failure: bool = False`` to fetch_registry. UI callers keep the stale-cache-fallback default. The reconciler's _auto_repair_missing_plugin now calls it with raise_on_failure=True so it can distinguish a genuine registry miss from a network error. 2. Uninstall was not transactional (CONFIRMED BUG). Two distinct failure modes silently left the system in an inconsistent state: (a) If ``cleanup_plugin_config`` raised, the code logged a warning and proceeded to delete files anyway, leaving an orphan install with no config entry. (b) If ``uninstall_plugin`` returned False or raised AFTER cleanup had already succeeded, the config was gone but the files were still on disk — another orphan state. Fix: introduce ``_do_transactional_uninstall`` shared by both the queue and direct paths. Flow: - snapshot plugin's entries in main config + secrets - cleanup_plugin_config; on failure, ABORT before touching files - uninstall_plugin; on failure, RESTORE the snapshot, then raise Both queue and direct endpoints now delegate to this helper and surface clean errors to the user instead of proceeding past failure. 3. /plugins/state/reconcile crashed on non-object JSON bodies (CONFIRMED BUG). The previous code did ``payload.get('force', False)`` after ``request.get_json(silent=True) or {}``. If a client sent a bare string or array as the JSON body, payload would be that string or list and .get() would raise AttributeError. Separately, ``bool("false")`` is True, so string-encoded booleans were mis-handled. Fix: guard ``isinstance(payload, dict)`` and route the value through the existing ``_coerce_to_bool`` helper. 4. Nitpick: use ``assert_called_once_with`` in test_force_reconcile_clears_unrecoverable_cache. The existing test worked in practice (we call reset_mock right before) but the stricter assertion catches any future regression where force=True might double-fire the install. Tests added (19 new, 48 total passing): - TestFetchRegistryRaiseOnFailure (4): flag propagates both RequestException and JSONDecodeError, wins over stale cache, and the default behavior is unchanged for existing callers. - test_real_store_manager_empty_registry_on_network_failure (1): the key regression test — uses the REAL PluginStoreManager (not a Mock) with ConnectionError at the HTTP helper layer, and verifies the reconciler does NOT poison _unrecoverable_missing_on_disk. - TestTransactionalUninstall (4): cleanup failure aborts before file removal; file removal failure (both False return and raise) restores the config snapshot; happy path still succeeds. - TestReconcileEndpointPayload (8): bare string / array / null JSON bodies, missing force key, boolean true/false, and string-encoded "true"/"false" all handled correctly. All 342 tests in the broader sweep still pass (2 pre-existing TestDottedKeyNormalization failures reproduce on main and are unrelated). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: address review nitpicks in store_manager + test Four small cleanups, each verified against current code: 1. ``_git_info_cache`` type annotation was ``Dict[str, tuple]`` — too loose. Tightened to ``Dict[str, Tuple[float, Dict[str, str]]]`` to match what ``_get_local_git_info`` actually stores (mtime + the sha/short_sha/branch/... dict it returns). Added ``Tuple`` to the typing imports. 2. The ``search_plugins`` early-return condition ``if len(filtered) == 1 or not fetch_commit_info and len(filtered) < 4`` parses correctly under Python's precedence (``and`` > ``or``) but is visually ambiguous. Added explicit parentheses to make the intent — "single plugin, OR small batch that doesn't need commit info" — obvious at a glance. Semantics unchanged. 3. Replaced a Unicode multiplication sign (×) with ASCII 'x' in the commit_cache_timeout comment. 4. Removed a dead ``concurrent_workers = []`` declaration from ``test_enrichment_runs_concurrently``. It was left over from an earlier sketch of the concurrency check — the final test uses only ``peak_lock`` and ``peak``. All 48 tests still pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(plugins): address second review pass — cache correctness and rollback Verified each finding against the current code. All four inline issues were real bugs; nitpicks 5-7 were valid improvements. 1. _get_latest_commit_info overwrote a good cached value with None on all-branches-404 (CONFIRMED BUG). The final line of the branch loop unconditionally wrote ``self.commit_info_cache[cache_key] = (time.time(), None)``, which clobbered any previously-good entry on a single transient failure (e.g. an odd 5xx, a temporary DNS hiccup during the branches_to_try loop). Fix: if there's already a good prior value, bump its timestamp into the backoff window and return it instead. Only cache None when we never had a good value. 2. _get_local_git_info cache did not invalidate on fast-forward (CONFIRMED BUG). Caching on ``.git/HEAD`` mtime alone is wrong: a ``git pull`` that fast-forwards the current branch updates ``.git/refs/heads/<branch>`` (or packed-refs) but leaves HEAD's contents and mtime untouched. The cache would then serve a stale SHA indefinitely. Fix: introduce ``_git_cache_signature`` which reads HEAD contents, resolves ``ref: refs/heads/<name>`` to the corresponding loose ref file, and builds a signature tuple of (head_contents, head_mtime, resolved_ref_mtime, packed_refs_mtime). A fast-forward bumps the ref file's mtime, which invalidates the signature and re-runs git. 3. test_install_plugin_is_not_blocked_by_tombstone swallowed all exceptions (CONFIRMED BUG in test). ``try: self.sm.install_plugin("bar") except Exception: pass`` could hide a real regression in install_plugin that happens to raise. Fix: the test now writes a COMPLETE valid manifest stub (id, name, class_name, display_modes, entry_point) and stubs _install_dependencies, so install_plugin runs all the way through and returns True. The assertion is now ``assertTrue(result)`` with no exception handling. 4. Uninstall rollback missed unload/reload (CONFIRMED BUG). Previous flow: cleanup → unload (outside try/except) → uninstall → rollback config on failure. Problem: if ``unload_plugin`` raised, the exception propagated without restoring config. And if ``uninstall_plugin`` failed after a successful unload, the rollback restored config but left the plugin unloaded at runtime — inconsistent. Fix: record ``was_loaded`` before touching runtime state, wrap ``unload_plugin`` in the same try/except that covers ``uninstall_plugin``, and on any failure call a ``_rollback`` local that (a) restores the config snapshot and (b) calls ``load_plugin`` to reload the plugin if it was loaded before we touched it. 5. Nitpick: ``_unrecoverable_missing_on_disk: set`` → ``Set[str]``. Matches the existing ``Dict``/``List`` style in state_reconciliation.py. 6. Nitpick: stale-cache fallbacks in _get_github_repo_info and _get_latest_commit_info now bump the cached entry's timestamp by a 60s failure backoff. Without this, a cache entry whose TTL just expired would cause every subsequent request to re-hit the network until it came back, amplifying the failure. Introduced ``_record_cache_backoff`` helper and applied it consistently. 7. Nitpick: replaced the flaky wall-time assertion in test_enrichment_runs_concurrently with just the deterministic ``peak["count"] >= 2`` signal. ``peak["count"]`` can only exceed 1 if two workers were inside the critical section simultaneously, which is definitive proof of parallelism. The wall-time check was tight enough (<200ms) to occasionally fail on CI / low-power boxes. Tests (6 new, 54 total passing): - test_cache_invalidates_on_fast_forward_of_current_branch: builds a loose-ref layout under a temp .git/, verifies a first call populates the cache, a second call with unchanged state hits the cache, and a simulated fast-forward (overwriting ``.git/refs/heads/main`` with a new SHA and mtime) correctly re-runs git. - test_commit_info_preserves_good_cache_on_all_branches_404: seeds a good cached entry, mocks requests.get to always return 404, and verifies the cache still contains the good value afterwards. - test_repo_info_stale_bumps_timestamp_into_backoff: seeds an expired cache, triggers a ConnectionError, then verifies a second lookup does NOT re-hit the network (proves the timestamp bump happened). - test_repo_info_stale_on_403_also_backs_off: same for the 403 path. - test_file_removal_failure_reloads_previously_loaded_plugin: plugin starts loaded, uninstall_plugin returns False, asserts load_plugin was called during rollback. - test_unload_failure_restores_config_and_does_not_call_uninstall: unload_plugin raises, asserts uninstall_plugin was never called AND config was restored AND load_plugin was NOT called (runtime state never changed, so no reload needed). Broader test sweep: 348/348 pass (2 pre-existing TestDottedKeyNormalization failures reproduce on main, unrelated). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(plugins): address third review pass — cache signatures, backoff, isolation All four findings verified as real issues against the current code. 1. _git_cache_signature was missing .git/config (CONFIRMED GAP). The cached ``result`` dict from _get_local_git_info includes ``remote_url``, which is read from ``.git/config``. But the cache signature only tracked HEAD + refs — so a config-only change (e.g. ``git remote set-url origin https://...``) would leave the stale URL cached indefinitely. This matters for the monorepo-migration detection in update_plugin. Fix: add ``config_contents`` and ``config_mtime`` to the signature tuple. Config reads use the same OSError-guarded pattern as the HEAD read. 2. fetch_registry stale fallback didn't bump registry_cache_time (CONFIRMED BUG). The other caches already had the failure-backoff pattern added in the previous review pass (via ``_record_cache_backoff``), but the registry cache's stale-fallback branches silently returned the cached payload without updating ``registry_cache_time``. Next request saw the same expired TTL, re-hit the network, failed again — amplifying the original transient failure. Fix: bump ``self.registry_cache_time`` forward by the existing ``self._failure_backoff_seconds`` (reused — no new constant needed) in both the RequestException and JSONDecodeError stale branches. Kept the ``raise_on_failure=True`` path untouched so the reconciler still gets the exception. 3. _make_client() in the uninstall/reconcile test helper leaked MagicMocks into the api_v3 singleton (CONFIRMED RISK). Every test call replaced api_v3.config_manager, .plugin_manager, .plugin_store_manager, etc. with MagicMocks and never restored them. If any later test in the same pytest run imported api_v3 expecting original state (or None), it would see the leftover mocks. Fix: _make_client now snapshots the original attributes (with a sentinel to distinguish "didn't exist" from "was None") and returns a cleanup callable. Both setUp methods call self.addCleanup(cleanup) so state is restored even if the test raises. On cleanup, sentinel entries trigger delattr rather than setattr to preserve the "attribute was never set" case. 4. Snapshot helpers used broad ``except Exception`` (CONFIRMED). _snapshot_plugin_config caught any exception from get_raw_file_content, which could hide programmer errors (TypeError, AttributeError) behind the "best-effort snapshot" fallback. The legitimate failure modes are filesystem errors (covered by OSError; FileNotFoundError is a subclass, IOError is an alias in Python 3) and ConfigError (what config_manager wraps all load failures in). Fix: narrow to ``(OSError, ConfigError)`` in both snapshot blocks. ConfigError was already imported at line 20 of api_v3.py. Tests added (4 new, 58 total passing): - test_cache_invalidates_on_git_config_change: builds a realistic loose-ref layout, writes .git/config with an "old" remote URL, exercises _get_local_git_info, then rewrites .git/config with a "new" remote URL + new mtime, calls again, and asserts the cache invalidated and returned the new URL. - test_stale_fallback_bumps_timestamp_into_backoff: seeds an expired registry cache, triggers ConnectionError, verifies first call serves stale, then asserts a second call makes ZERO new HTTP requests (proves registry_cache_time was bumped forward). - test_snapshot_survives_config_read_error: raises ConfigError from get_raw_file_content and asserts the uninstall still completes successfully — the narrow exception list still catches this case. - test_snapshot_does_not_swallow_programmer_errors: raises a TypeError from get_raw_file_content (not in the narrow list) and asserts it propagates up to a 500, AND that uninstall_plugin was never called (proves the exception was caught at the right level). Broader test sweep: 352/352 pass (2 pre-existing TestDottedKeyNormalization failures reproduce on main, unrelated). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-08 12:33:54 -04:00
Chuck GitHub Claude Opus 4.6 Chuck	781224591f	fix: post-audit follow-up code fixes (cache, fonts, icons, dev script) (#307 ) * fix: post-audit follow-up code fixes (cache, fonts, icons, dev script, CI) The docs refresh effort (#306, ledmatrix-plugins#92) surfaced seven code bugs that were intentionally left out of the docs PRs because they required code changes rather than doc fixes. This PR addresses the six that belong in LEDMatrix (the seventh — a lacrosse-scoreboard mode rename — lives in the plugins repo). Bug 1: cache_manager.delete() AttributeError src/common/api_helper.py:287 and src/plugin_system/resource_monitor.py:343 both call cache_manager.delete(key), which doesn't exist — only clear_cache(key=None). Added a delete() alias method on CacheManager that forwards to clear_cache(key). Reverts the "There is no delete() method" wording in DEVELOPER_QUICK_REFERENCE, .cursorrules so the docs match the new shim. Bug 2: dev_plugin_setup.sh PROJECT_ROOT resolution scripts/dev/dev_plugin_setup.sh:9 set PROJECT_ROOT to SCRIPT_DIR instead of walking up two levels to the repo root, so PLUGINS_DIR resolved to scripts/dev/plugins/ and created symlinks under the script's own directory. Fixed the path and removed the stray scripts/dev/plugins/of-the-day symlink left by earlier runs. Bug 3: plugin custom icons regressed from v2 to v3 web_interface/blueprints/api_v3.py built the /plugins/installed response without including the manifest's "icon" field, and web_interface/templates/v3/base.html hardcoded fas fa-puzzle-piece in all three plugin-tab render sites. Pass the icon through the API and read it from the templates with a puzzle-piece fallback. Reverts the "currently broken" banners in docs/PLUGIN_CUSTOM_ICONS.md and docs/PLUGIN_CUSTOM_ICONS_FEATURE.md. Bug 4: register_plugin_fonts was never wired up src/font_manager.py:150 defines register_plugin_fonts(plugin_id, font_manifest) but nothing called it, so plugin manifests with a "fonts" block were silently no-ops. Wired the call into PluginManager.load_plugin() right after plugin_loader.load_plugin returns. Reverts the "not currently wired" warning in docs/FONT_MANAGER.md's "For Plugin Developers" section. Bug 5: dead web_interface_v2 import pattern (LEDMatrix half) src/base_odds_manager.py had a try/except importing web_interface_v2.increment_api_counter, falling back to a no-op stub. The module doesn't exist anywhere in the v3 codebase and no API metrics dashboard reads it. Deleted the import block and the single call site; the plugins-repo half of this cleanup lands in ledmatrix-plugins#<next>. Bug 7: no CI test workflow .github/workflows/ only contained security-audit.yml; pytest ran locally but was not gated on PRs. Added .github/workflows/tests.yml running pytest against Python 3.10, 3.11, 3.12 in EMULATOR=true mode, skipping tests marked hardware or slow. Updated docs/HOW_TO_RUN_TESTS.md to reflect that the workflow now exists. Verification done locally: - CacheManager.delete(key) round-trips with set/get - base_odds_manager imports without the v2 module present - dev_plugin_setup.sh PROJECT_ROOT resolves to repo root - api_v3 and plugin_manager compile clean - tests.yml YAML parses - Script syntax check on dev_plugin_setup.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review comments on #307 - src/cache_manager.py: clear_cache(key) treated empty string as "wipe all" because of `if key:`. Switched to `key is None` branching, made delete(key) and clear_cache(key) reject empty strings and None outright with ValueError, and updated both docstrings to make the contract explicit. Verified locally with a round-trip test that clear_cache() (no arg) still wipes everything but clear_cache("") and delete("") raise. - src/plugin_system/plugin_manager.py: was reaching for the font manager via getattr(self.display_manager, 'font_manager', None). PluginManager already takes a dedicated font_manager parameter (line 54) and stores it as self.font_manager (line 69), so the old path was both wrong and could miss the font manager entirely when the host injects them separately. Switched to self.font_manager directly with the same try/except warning behavior. - web_interface/templates/v3/base.html: in the full plugin-tab renderer, the icon was injected with `<i class="${escapeHtml(plugin.icon)}">` — but escapeHtml only escapes <, >, and &, not double quotes, so a manifest with a quote in its icon string could break out of the class attribute. Replaced the innerHTML template with createElement for the <i> tag, set className from plugin.icon directly (no string interpolation), and used a text node for the label. Same fix shape would also harden the two stub-renderer sites at line 515 / 774, but those already escape `"` to " and CodeRabbit only flagged this site, so leaving them for now. - docs/FONT_MANAGER.md: clarified that the Manual Font Overrides workflow (set_override / remove_override / font_overrides.json) is the supported override path today, and only the Fonts tab in the web UI is the placeholder. Previous wording conflated the two and made it sound like overrides themselves were broken. - docs/HOW_TO_RUN_TESTS.md: replaced the vague "see the PR adding it" with a concrete link to #307 and a note that the workflow file itself is held back pending the workflow scope. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-04-08 09:25:12 -04:00
Chuck GitHub Claude Opus 4.6	7afc2c0670	fix(web): increase chain_length max from 8 to 32 (#300 ) * fix(web): increase chain_length max from 8 to 32 The web UI form input capped chain_length at 8 panels, preventing users with larger displays (e.g. 16-panel setups) from configuring their hardware through the UI. The backend API had no such limit. Changed max="8" to max="32" to support large display configurations. Added panel count example to the help text. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(web): add server-side bounds validation for display hardware fields The API endpoint at /api/v3/config/main accepted any integer value for display hardware fields (chain_length, rows, cols, brightness, etc.) without bounds checking. Only the HTML form had min/max attributes, which are trivially bypassed by direct API calls. Added _int_field_limits dict with bounds for all integer hardware fields: chain_length: 1-32, parallel: 1-4, brightness: 1-100, rows: 8-128, cols: 16-128, scan_mode: 0-1, pwm_bits: 1-11, pwm_dither_bits: 0-2, pwm_lsb_nanoseconds: 50-500, limit_refresh_rate_hz: 0-1000, gpio_slowdown: 0-5 Out-of-bounds or non-integer values now return 400 with a clear error message (e.g. "Invalid chain_length value 99. Must be between 1 and 32.") before any config is persisted. Follows the same inline validation pattern already used for led_rgb_sequence, panel_type, and multiplexing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(api): strict int validation and add max_dynamic_duration_seconds bounds Reject bool/float types in _int_field_limits validation loop to prevent silent coercion, and add max_dynamic_duration_seconds to the validation map so it gets proper bounds checking instead of a raw int() call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-29 20:17:14 -04:00
Chuck GitHub Claude Opus 4.6	77e9eba294	fix: overhaul WiFi captive portal for reliable setup (#296 ) * fix: overhaul WiFi captive portal for reliable device detection and fast setup The captive portal detection endpoints were returning "success" responses that told every OS (iOS, Android, Windows, Firefox) that internet was working — so the portal popup never appeared. This fixes the core issue and improves the full setup flow: - Return portal-triggering redirects when AP mode is active; normal success responses when not (no false popups on connected devices) - Add lightweight self-contained setup page (9KB, no frameworks) for the captive portal webview instead of the full UI - Cache AP mode check with 5s TTL (single systemctl call vs full WiFiManager instantiation per request) - Stop disabling AP mode during WiFi scans (which disconnected users); serve cached/pre-scanned results instead - Pre-scan networks before enabling AP mode so captive portal has results immediately - Use dnsmasq.d drop-in config instead of overwriting /etc/dnsmasq.conf (preserves Pi-hole and other services) - Fix manual SSID input bug that incorrectly overwrote dropdown selection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review findings for WiFi captive portal - Remove orphaned comment left over from old scan_networks() finally block - Add sudoers rules for dnsmasq drop-in copy/remove to install script - Combine cached-network message into single showMsg call (was overwriting) - Return (networks, was_cached) tuple from scan_networks() so API endpoint derives cached flag from the scan itself instead of a redundant AP check - Narrow exception catch in AP mode cache to SubprocessError/OSError and log the failure for remote debugging - Bound checkNewIP retries to 20 attempts (60s) before showing fallback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-27 14:50:33 -04:00
Chuck GitHub Claude Opus 4.6	6eccb74415	fix: handle dotted schema keys in plugin settings save (#295 ) * fix: handle dotted schema keys in plugin settings save (issue #254) The soccer plugin uses dotted keys like "eng.1" for league identifiers. PR #260 fixed backend helpers but the JS frontend still corrupted these keys by naively splitting on dots. This fixes both the JS and remaining Python code paths: - JS getSchemaProperty(): greedy longest-match for dotted property names - JS dotToNested(): schema-aware key grouping to preserve "eng.1" as one key - Python fix_array_structures(): remove broken prefix re-navigation in recursion - Python ensure_array_defaults(): same prefix navigation fix Closes #254 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review findings for dotted-key handling - ensure_array_defaults: replace None nodes with {} so recursion proceeds into nested objects (was skipping when key existed as None) - dotToNested: add tail-matching that checks the full remaining dotted tail against the current schema level before greedy intermediate matching, preventing leaf dotted keys from being split - syncFormToJson: replace naive key.split('.') reconstruction with dotToNested(flatConfig, schema) and schema-aware getSchemaProperty() so the JSON tab save path produces the same correct nesting as the form submit path - Add regression tests for dotted-key array normalization and None array default replacement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address second round of review findings - Tests: replace conditional `if response.status_code == 200` guards with unconditional `assert response.status_code == 200` so failures are not silently swallowed - dotToNested: guard finalKey write with `if (i < parts.length)` to prevent empty-string key pollution when tail-matching consumed all parts - Extract normalizeFormDataForConfig() helper from handlePluginConfigSubmit and call it from both handlePluginConfigSubmit and syncFormToJson so the JSON tab sync uses the same robust FormData processing (including _data JSON inputs, bracket-notation checkboxes, array-of-objects, file-upload widgets, checkbox DOM detection, and unchecked boolean handling via collectBooleanFields) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-27 11:12:31 -04:00
Chuck GitHub Claude Opus 4.6	640a4c1706	fix: auto-repair missing plugins on startup (#293 ) * fix: auto-repair missing plugins and graceful config fallback Plugins whose directories are missing (failed update, migration, etc.) now get automatically reinstalled from the store on startup. The config endpoint no longer returns a hard 500 when a schema is unavailable — it falls back to conservative key-name-based masking so the settings page stays functional. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: handle ledmatrix- prefix in plugin updates and reconciliation The store registry uses unprefixed IDs (e.g., 'weather') while older installs used prefixed config keys (e.g., 'ledmatrix-weather'). Both update_plugin() and auto-repair now try the unprefixed ID as a fallback when the prefixed one isn't found in the registry. Also filters system config keys (schedule, display, etc.) from reconciliation to avoid false positives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address code review findings for plugin auto-repair - Move backup-folder filter from _get_config_state to _get_disk_state where the artifact actually lives - Run startup reconciliation in a background thread so requests aren't blocked by plugin reinstallation - Set _reconciliation_done only after success so failures allow retries - Replace print() with proper logger in reconciliation - Wrap load_schema in try/except so exceptions fall through to conservative masking instead of 500 - Handle list values in _conservative_mask_config for nested secrets - Remove duplicate import re Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: add thread-safe locking to PluginManager and fix reconciliation retry PluginManager thread safety: - Add RLock protecting plugin_manifests and plugin_directories - Build scan results locally in _scan_directory_for_plugins, then update shared state under lock - Protect reads in get_plugin_info, get_all_plugin_info, get_plugin_directory, get_plugin_display_modes, find_plugin_for_mode - Protect manifest mutation in reload_plugin - Prevents races between background reconciliation thread and request handlers reading plugin state Reconciliation retry: - Clear _reconciliation_started on exception so next request retries - Check result.reconciliation_successful before marking done - Reset _reconciliation_started on non-success results to allow retry Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-25 17:09:49 -04:00
Chuck GitHub Claude Opus 4.6 Chuck	48ff624a85	fix: catch ConfigError in display preview generator (#288 ) * fix: catch ConfigError in display preview generator PR #282 narrowed bare except blocks but missed ConfigError from config_manager.load_config(), which wraps FileNotFoundError, JSONDecodeError, and OSError. Without this, a corrupt or missing config crashes the display preview SSE endpoint instead of falling back to 128x64 defaults. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): comprehensive error handling cleanup - Remove all traceback.format_exc() from client responses (33 remaining instances) - Sanitize str(e) from client-facing messages, replacing with generic error messages - Replace ~65 bare print() calls with structured logger.exception/error/warning/info/debug - Remove ~35 redundant inline `import traceback` and `import logging` statements - Convert logging.error/warning calls to use module-level named logger - Fix WiFi endpoints that created redundant inline logger instances - Add logger.exception() at all WebInterfaceError.from_exception() call sites - Fix from_exception() in errors.py to use safe messages instead of raw str(exception) - Apply consistent [Tag] prefixes to all logger calls for production triage Only safe, user-input-derived str(e) kept: json.JSONDecodeError handlers (400 responses). Subprocess template print(stdout) calls preserved (not error logging). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): correct error inference, remove debug log leak, consolidate config handlers - _infer_error_code: map Config* exceptions to CONFIG_LOAD_FAILED (ConfigError is only raised by load_config(), so CONFIG_SAVE_FAILED produced wrong safe message and wrong suggested_fixes) - Remove leftover DEBUG logs in save_main_config that dumped full request body and all HTTP headers (Authorization, Cookie, etc.) - Replace dead FileNotFoundError/JSONDecodeError/IOError handlers in get_dim_schedule_config with single ConfigError catch (load_config already wraps these into ConfigError) - Remove redundant local `from src.exceptions import ConfigError` imports now covered by top-level import - Strip str(e) from client-facing error messages in dim schedule handler Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): fix plugin update logging and config validation leak - update_plugin: change logger.exception to logger.error in non-except branch (logger.exception outside an except block logs useless "NoneType: None" traceback) - update_plugin: remove duplicate logger.exception call in except block (was logging the same failure twice) - save_plugin_config validation: stop logging full plugin_config dict (can contain API keys, passwords, tokens) and raw form_data values; log only keys and validation errors instead Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-25 12:53:45 -04:00
5ymb01 GitHub Claude Opus 4.6 5ymb01	31ed854d4e	fix(config): deduplicate uniqueItems arrays before schema validation (#292 ) * fix(config): deduplicate uniqueItems arrays before schema validation When saving plugin config via the web UI, the form data is merged with the existing stored config. If a user adds an item that already exists (e.g. adding stock symbol "FNMA" when it's already in the list), the merged array contains duplicates. Schemas with `uniqueItems: true` then reject the config, making it impossible to save. Add a recursive dedup pass that runs after normalization/filtering but before validation. It walks the schema tree, finds arrays with the uniqueItems constraint, and removes duplicates while preserving order. Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: recurse into array items and add tests for uniqueItems dedup Address CodeRabbit review: _dedup_unique_arrays now also recurses into array elements whose items schema is an object, so nested uniqueItems constraints inside arrays-of-objects are enforced. Add 11 unit tests covering: - flat arrays with/without duplicates - order preservation - arrays without uniqueItems left untouched - nested objects (feeds.stock_symbols pattern) - arrays of objects with inner uniqueItems arrays - edge cases (empty array, missing keys, integers) - real-world stock-news plugin config shape Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: extract dedup_unique_arrays to shared validators module Move _dedup_unique_arrays from an inline closure in save_plugin_config to src/web_interface/validators.dedup_unique_arrays so tests import and exercise the production code path instead of a duplicated copy. Addresses CodeRabbit review: tests now validate the real function, preventing regressions from diverging copies. Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com> Co-authored-by: 5ymb01 <noreply@github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>	2026-03-24 15:48:26 -04:00
5ymb01 GitHub 5ymb01 Claude Opus 4.6	c8737d1a6c	fix(api): use sys.executable for plugin action subprocess calls (#277 ) * fix(api): use sys.executable for plugin action subprocess calls The execute_plugin_action endpoint hardcoded 'python3' when spawning plugin scripts via subprocess. This can fail if the system Python is named differently or if a virtualenv is active, since 'python3' may not point to the correct interpreter. Changes: - Replace 'python3' with sys.executable in the non-OAuth script execution branch (uses the same interpreter running the web service) - Remove redundant 'import sys' inside the oauth_flow conditional block (sys is already imported at module level; the local import shadows the top-level binding for the entire function scope, which would cause UnboundLocalError if sys were referenced in the else branch on Python 3.12+) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): replace all remaining hardcoded python3 with sys.executable Fix 4 additional subprocess calls that still used 'python3' instead of sys.executable: parameterized action wrapper (line 5150), stdin-param wrapper (line 5211), no-param wrapper (line 5417), and OAuth auth script (line 5524). Ensures plugin actions work in virtualenvs and non-standard Python installations. Addresses CodeRabbit findings on PR #277. Co-Authored-By: 5ymb01 <5ymb01@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-21 12:21:18 -04:00
5ymb01 GitHub Claude Opus 4.6 5ymb01	f718305886	fix(security): stop leaking Python tracebacks to HTTP clients (#283 ) * fix(security): stop leaking Python tracebacks to HTTP clients Replace 13 instances where traceback.format_exc() was sent in API JSON responses (via `details=`, `traceback:`, or `details:` keys). - 5 error_response(details=traceback.format_exc()) → generic message - 6 jsonify({'traceback': traceback.format_exc()}) → removed key - 2 jsonify({'details': error_details}) → logger.error() instead Tracebacks in debug mode (app.py error handlers) are preserved as they are guarded by app.debug and expected during development. Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): sanitize str(e) from client responses, add server-side logging Address CodeRabbit review findings: - Replace str(e) in error_response message fields with generic messages - Replace import logging/traceback + manual format with logger.exception() - Add logger.exception() to 6 jsonify handlers that were swallowing errors - All exception details now logged server-side only, not sent to clients Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove duplicate traceback logging, sanitize secrets config error Address CodeRabbit nitpicks: - Remove manual import logging/traceback + logging.error() that duplicated the logger.exception() call in save_raw_main_config - Apply same fix to save_raw_secrets_config: replace str(e) in client response with generic message, use logger.exception() for server-side Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com> Co-authored-by: 5ymb01 <noreply@github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-20 14:51:05 -04:00
5ymb01 GitHub Claude Opus 4.6 5ymb01	f0dc094cd6	fix(security): use Path.relative_to() for path confinement (#284 ) * fix(security): use Path.relative_to() for path confinement check Replace str.startswith() path check with Path.relative_to() in the plugin file viewer endpoint. startswith() can be bypassed when a directory name is a prefix of another (e.g., /plugins/foo vs /plugins/foobar). relative_to() correctly validates containment. Co-Authored-By: 5ymb01 <noreply@github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: trigger CodeRabbit review --------- Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com> Co-authored-by: 5ymb01 <noreply@github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-20 10:04:49 -04:00
5ymb01 GitHub 5ymb01 Claude Opus 4.6	76c5bf5781	fix(security): mask secret fields in API responses and extract helpers (#276 ) * fix(security): mask secret fields in API responses and extract helpers GET /config/secrets returned raw API keys in plaintext to the browser. GET /plugins/config returned merged config including deep-merged secrets. POST /plugins/config could overwrite existing secrets with empty strings when the GET endpoint returned masked values that were sent back unchanged. Changes: - Add src/web_interface/secret_helpers.py with reusable functions: find_secret_fields, separate_secrets, mask_secret_fields, mask_all_secret_values, remove_empty_secrets - GET /config/secrets: mask all values with '••••••••' - GET /plugins/config: mask x-secret fields with '' - POST /plugins/config: filter empty-string secrets before saving - pages_v3: mask secrets before rendering plugin config templates - Remove three duplicated inline find_secret_fields/separate_secrets definitions in api_v3.py (replaced by single imported module) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): harden secret masking against CodeRabbit findings - Fail-closed: return 500 when schema unavailable instead of leaking secrets - Fix falsey masking: use `is not None and != ''` instead of truthiness check so values like 0 or False are still redacted - Add array-item secret support: recurse into `type: array` items schema to detect and mask secrets like accounts[].token - pages_v3: fail-closed when schema properties missing Addresses CodeRabbit findings on PR #276: - Critical: fail-closed bypass when schema_mgr/schema missing - Major: falsey values not masked (0, False leak through) - Major: pages_v3 fail-open when schema absent - Major: array-item secrets unsupported Co-Authored-By: 5ymb01 <5ymb01@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-08 20:41:18 -04:00
5ymb01 GitHub 5ymb01 Claude Opus 4.6	feee1dffde	fix(web): remove shadowed sys import in plugin action handler (#280 ) * fix(web): remove shadowed sys import in plugin action handler Two `import sys` statements inside execute_plugin_action() and authenticate_spotify() shadowed the module-level import, causing "cannot access local variable 'sys'" errors when sys.executable was referenced in earlier branches of the same function. Also fixes day number validation in the of-the-day upload endpoint to accept 366 (leap year). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(api): correct validation message from 1-365 to 1-366 The JSON structure validation message still said '1-365' while the actual range check accepts 1-366 for leap years. Make all three validation messages consistent. Addresses CodeRabbit finding on PR #280. Co-Authored-By: 5ymb01 <5ymb01@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: 5ymb01 <5ymb01@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-03-08 20:38:04 -04:00
Chuck GitHub Claude Sonnet 4.6 Chuck	fe5c1d0d5e	feat(web): add Google Calendar picker widget for dynamic multi-calendar selection (#274 ) * fix(install): add --prefer-binary to pip installs to avoid /tmp exhaustion timezonefinder (~54 MB) includes large timezone polygon data files that pip unpacks into /tmp during installation. On Raspberry Pi, the default tmpfs /tmp size (often ~half of RAM) can be too small, causing the install to fail with an out-of-space error. Adding --prefer-binary tells pip to prefer pre-built binary wheels over source distributions. Since timezonefinder and most other packages publish wheels on PyPI (and piwheels.org has ARM wheels), this avoids the large temporary /tmp extraction and speeds up installs generally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(timezone): use America/New_York instead of EST for ESPN API date queries EST is a fixed UTC-5 offset that does not observe daylight saving time, causing the ESPN API date to be off by one hour during EDT (March–November). America/New_York correctly handles DST transitions. The ESPN scoreboard API anchors its schedule calendar to Eastern US time, so this Eastern timezone is intentionally kept for the API date — it is not user-configurable. Game time display is converted separately to the user's configured timezone. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(web): add Google Calendar picker widget for dynamic calendar selection Adds a new google-calendar-picker widget and API endpoint that lets users load their available Google Calendars by name and check the ones they want, instead of manually typing calendar IDs. - GET /api/v3/plugins/calendar/list-calendars — calls plugin.get_calendars() and returns all accessible calendars with id, summary, and primary flag - google-calendar-picker.js — new widget: "Load My Calendars" button renders a checklist; selections update a hidden comma-separated input for form submit - plugin_config.html — handles x-widget: google-calendar-picker in array branch - base.html — loads the new widget script Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): address PR review findings in google-calendar-picker - api_v3.py: replace broad except block with specific exception handling, log full traceback via module logger, normalize/validate get_calendars() output to stable {id,summary,primary} objects, return opaque user-friendly error message instead of leaking str(e) - google-calendar-picker.js: fix button label only updating to "Refresh Calendars" on success (restore original label on error); update summary paragraph via syncHiddenAndSummary() on every checkbox change so UI stays in sync with hidden input; pass summary element through loadCalendars and renderCheckboxes instead of re-querying DOM - plugin_config.html: bound initWidget retry loop with MAX_RETRIES=40 to prevent infinite timers; normalize legacy comma-separated string values to arrays before passing to widget.render so pre-existing config populates correctly - install_dependencies_apt.py: update install_via_pip docstring to document both --break-system-packages and --prefer-binary flags Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): harden list_calendar_calendars input validation - Remove unused `as e` binding from ValueError/TypeError/KeyError except clause - Replace hasattr(__iter__) with isinstance(list\|tuple) so non-sequence returns are rejected before iteration - Validate each calendar entry is a collections.abc.Mapping; skip and warn on malformed items rather than propagating a TypeError - Coerce id/summary to str safely if not already strings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web): skip calendar entries with empty id in list_calendar_calendars After coercing cal_id to str, check it is non-empty before appending to the calendars list so entries with no usable id are never forwarded to the client. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>	2026-02-25 18:19:32 -05:00
Chuck GitHub Claude Sonnet 4.6 Chuck	275fed402e	fix(logos): support logo downloads for custom soccer leagues (#262 ) * fix(logos): support logo downloads for custom soccer leagues LogoDownloader.fetch_teams_data() and fetch_single_team() only had hardcoded API endpoints for predefined soccer leagues. Custom leagues (e.g., por.1, mex.1) would silently fail when the ESPN game data didn't include a direct logo URL. Now dynamically constructs the ESPN teams API URL for any soccer_* league not in the predefined map. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(logos): address PR review — directory, bulk download, and dedup - get_logo_directory: custom soccer leagues now resolve to shared assets/sports/soccer_logos/ instead of creating per-league dirs - download_all_missing_logos: use _resolve_api_url so custom soccer leagues are no longer silently skipped - Extract _resolve_api_url helper to deduplicate dynamic URL construction between fetch_teams_data and fetch_single_team Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(web): preserve array item properties in _set_nested_value When saving config with array-of-objects fields (e.g., custom_leagues), _set_nested_value would replace existing list objects with dicts when navigating dot-notation paths like "custom_leagues.0.name". This destroyed any properties on array items that weren't submitted in the form (e.g., display_modes, game_limits, filtering). Now properly indexes into existing lists when encountering numeric path segments, preserving all non-submitted properties on array items. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): address PR #262 code review security findings - logo_downloader: validate league name against allowlist before constructing filesystem paths in get_logo_directory to prevent path traversal (reject anything not matching ^[a-z0-9_-]+$) - logo_downloader: validate league_code against allowlist before interpolating into ESPN API URL in _resolve_api_url to prevent URL path injection; return None on invalid input - api_v3: add MAX_LIST_EXPANSION=1000 cap to _set_nested_value list expansion; raise ValueError for out-of-bounds indices; replace silent break fallback with TypeError for unexpected traversal types Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>	2026-02-24 19:18:29 -05:00

1 2