rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-10-04 14:25:08 +00:00
Code Issues Packages Projects Releases Wiki Activity
2,148 Commits 10 Branches 20 Tags
main
Commit Graph
4 Commits
Author SHA1 Message Date
ChuckandClaude Opus 5.5 0b039c875f fix(web): plugin settings endpoints - a refused save no longer leaks into config.json; GET masks secrets (#742) 
* fix(config): load_config hands each caller a private copy

ConfigManager.load_config() returned its cached self.config itself (the
mtime fast path from #410 kept the full path's aliasing). Web handlers
edit what they load and then validate: the plugin form save applies the
posted fields to the loaded section (a shallow .copy(), so nested dicts
were the cache's own), and save_main_config sets its checkboxes before
it checks auto_update_channel. When the save was refused, the edit
stayed in the cache the fast path serves, and the next save of any
other setting wrote it to config.json: the refused value, and a nested
secret typed into the same form (mqtt.password, league.espn_s2,
flightaware.api_key) in plain text, since it never reached
config_secrets.json to be stripped. The form also reloaded showing the
refused values.

load_config() now returns a private copy on both paths, and
save_config/save_config_atomic keep a copy of what they were given, so
nothing a caller edits reaches the cache unless it is saved. Fixing it
here rather than in each handler covers every route that edits before it
validates. No caller relies on editing the cache without saving: every
src/ and web_interface/ caller either reads, or saves the dict it
edited. get_config() still returns the live dict for the display
process's readers.

The copy is a pickle round trip: on a Pi 4 with its real 64 KiB config,
2.0 ms against 6.9 ms for copy.deepcopy (json round trip 3.4 ms). Two
tests asserted the aliasing itself and now assert a copy.

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

* fix(web): GET /plugins/config masks secrets and refuses core sections

The route returned the plugin's section as load_config() has it, with
config_secrets.json merged in: API keys and tokens went out in plain
text. #276 masked them here; #330's rewrite of the route dropped it,
while the settings page and GET /config/secrets kept masking. It also
took any plugin_id, so ?plugin_id=web_auth returned the login's
cookie-signing key and password hash, and ?plugin_id=github the Plugin
Store token, which GET /config/main strips and redacts.

The route now refuses what _non_plugin_id_error refuses for reset and
uninstall (core sections, malformed ids) with a 400, and blanks x-secret
fields with mask_secret_fields after the defaults merge, as the page
does. A plugin with no schema has its credential-named fields blanked by
_redact_credentials, as GET /config/main does. Blank rather than the
bullets of GET /config/secrets: the save drops a blank secret as
"unchanged" (remove_empty_secrets) but would store the bullets, so the
response must post back as it came. Tested: GET, then POST the response
unchanged, keeps every stored secret.

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

* fix(web): parse a table row's cells against the list's item schema

An array of objects drawn as a table posts each cell as
"cities.0.timezone". _get_schema_property stopped at "cities" (an array,
not an object with properties), so _parse_form_value_with_schema got no
schema for the cell and guessed: a blank optional text cell became None
and a text cell holding digits became an int. Validation refused both,
so every save of the page failed for as long as such a row existed --
geochron's city without a timezone, a countdown named "2027". A secret
cell is always drawn blank, so a plugin with secrets in its rows could
not be saved from the form at all.

The lookup now steps from an index segment into the array's items: to
the item schema itself for "color.2", into its properties for a row
cell. Number, boolean and required cells convert as before.

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

* fix(web): a blank secret field saves as "unchanged", required or not

The settings page draws a stored secret blank (mask_secret_fields) and
posts the blank back. _parse_form_value_with_schema turned a blank
optional string into "" -- which the save drops as unchanged
(remove_empty_secrets) -- but a blank required one into None. For a
secret that is required with no default (youtube-stats' api_key) that
None failed validation, so every save of the page was refused until the
key was typed in again.

A blank text secret (x-secret, type string) now parses to "", whatever
its required list says; a list or object secret keeps getting [] or {},
which the save drops the same way. Not _SKIP_FIELD: skipping keeps the
value load_config() merged in, and the save would then write it back to
config_secrets.json -- after a secret change the cached section can
still hold the old one, so that write reverted it. A test covers that
sequence.

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

* fix(web): POST /plugins/config refuses core sections and malformed ids

Reset and uninstall check the plugin id with _non_plugin_id_error; the
save did not. {"plugin_id": "display", "config": {...}} found no schema,
so nothing was validated or filtered, and the body was merged into the
core display section along with "enabled": true -- rows: "banana"
included. A plugin_id that was not a string (a list, an object, a number)
reached config.get() or the schema lookup, raised TypeError, and came
back as a 500.

Both the JSON and the form path now call _non_plugin_id_error first and
answer its 400.

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

* fix(web): a text field keeps "true", "[1, 2]" and "{}" as typed

_parse_form_value_with_schema guessed before it consulted the schema:
"true"/"false" became booleans, and a value starting with "[" or "{"
that parsed as JSON became a list or object, whatever the field's type.
A text setting holding "true", "False", "[1, 2]" or "{}" was then
refused by validation ("Expected type string, got bool"), and the save
with it.

A field whose schema type is string, or string-or-null, now returns the
posted text as it came. Every other type goes through the conversions as
before; numbers in text fields were already left alone.

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

* fix(config): copy the cached config without pickle

_private_copy was a pickle round trip. It only ever unpickled bytes it had
just made from our own dict, so nothing untrusted reached it, but it put
pickle in the config path and Codacy failed the PR for it (B301/B403).
The config is JSON data, so copying its dicts and lists is a full copy;
every other value is immutable. Measured on ledpi (Pi 4) with its real
60 KiB config: 2.11 ms, against 1.92 ms for pickle and 6.75 ms for
copy.deepcopy.

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

* docs(changelog): describe the config copy without pickle

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-10-03 22:30:16 -04:00
ChuckandClaude Opus 5 2082665252 fix(config): load config_secrets.json on hosts without os.geteuid (#590) 
ensure_shared_group_ownership() - the chgrp self-heal ConfigManager runs
before reading config_secrets.json (#416) - looked up os.geteuid
unguarded. That name does not exist on Windows, and the AttributeError
is not an OSError, so it escaped the helper's best-effort handling and
every except clause in load_config(). Any Windows checkout with a
config/config_secrets.json got a ConfigError from every config load and
could not import web_interface.app.

That is what made test_update_all_plugins.py error at setup: its client
fixture imports web_interface.app. It was not state leaked between test
files - the trigger is whether the checkout has a secrets file.

Return early when os.geteuid or os.chown is missing. No change on POSIX.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
 2026-09-16 17:12:18 -04:00
ChuckandClaude Fable 5 ee59caa577 Follow-ups from #441: secret-helper migration, ten more bug fixes, and coverage for every remaining untested module (#444) 
* refactor(web): use canonical secret helpers in api_v3; make ConfigManager secret strip/merge array-aware

api_v3.py carried three inline nested copies of find_secret_fields/
separate_secrets (main-config save, plugin-config save, plugin-config
reset). They drifted from each other (one lacked isinstance guards) and
none supported the canonical module's array-item secrets
(accounts[].token). All three endpoints now import from
src/web_interface/secret_helpers.

Adopting the canonical behavior makes array-item secrets reachable, and
their parallel-placeholder shape ([{'token': ...}, {}] alongside the
regular list) was not survivable by ConfigManager's round-trip:
_strip_secrets_recursive dropped the whole key (losing the regular
fields from config.json) and _deep_merge replaced the regular list
wholesale on load. Both are now array-aware:

- strip removes the secret fields from each item and ALWAYS keeps the
  list so indices survive for merge-on-load; whole-key secrets (scalar
  lists, shape mismatches) still drop the key entirely — never leak.
- merge folds each secrets item into the config item at the same index,
  skipping {} placeholders. The regular list's length is authoritative
  in both directions: a user deleting an array item never has it
  resurrected from a stale secrets entry (extras warn and are ignored).

api_v3's own deep_merge intentionally still replaces lists wholesale —
form posts carry complete arrays and index-merging would resurrect
deleted items; a comment now documents that.

Tests: the parity guard flips from 'exactly 3 inline copies' to 'zero,
and the canonical import must exist'; TestArraySecretStripAndMerge
covers the new strip/merge semantics incl. length-mismatch contracts;
new test_api_v3_secret_roundtrip.py drives all three endpoints through
a Flask client with a REAL ConfigManager+SchemaManager over tmp_path,
proving secrets land in config_secrets.json, config.json stays clean,
and a fresh load merges them back into the right array items.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* fix: repair broken helper paths across display, cache, odds, logging, resolver, repos, config, validator

Nine fixes for bugs surfaced while writing coverage for previously
untested modules (plus the bool-duration quirk pinned in PR #441):

- base_plugin.get_display_duration: exclude bools from both numeric
  branches — display_duration=True no longer reads as a 1-second slot;
  it falls through to config, then the 15.0 default.
- display_helper: draw_error_message/draw_no_data_message called
  _draw_centered_text with the wrong arguments and crashed with
  AttributeError — both now delegate to draw_centered_text.
  draw_scorebug_layout drew status and clock at the same y, overprinting
  each other — they now share one combined top line.
  draw_ticker_layout drew its text starting at x=display_width (fully
  off-canvas), returning a blank frame every time — now draws at x=0;
  scroll_speed stays accepted-but-unused and is documented as such.
- api_helper.clear_cache guarded on a nonexistent CacheManager.clear()
  method, silently never clearing anything; it now uses the real surface
  (clear_cache/delete/list_cache_files) and no-ops safely otherwise.
- base_odds_manager._extract_espn_data raised AttributeError when ESPN
  sent explicit JSON nulls ("homeTeamOdds": null) — every level now
  null-safes with 'or {}'. format_odds_summary gated on
  is_odds_available, which deliberately ignores money lines, so
  ML-only odds formatted as "No odds available" — it now gates only on
  empty/no_odds data and formats money lines.
- logging_config.ContextualFormatter mutated record.msg in place, so a
  second handler prepended the context prefix twice; it now formats a
  copy. log_error hardcoded exc_info=True and raised TypeError when the
  caller passed exc_info — now kwargs.setdefault.
- dynamic_team_resolver wrote its "shared" class cache through self,
  creating instance shadows — the cache was per-instance and every
  scoreboard refetched rankings. Writes now go through the class.
- saved_repositories cleaned URLs with an unanchored .replace('.git','')
  that mangled URLs merely containing '.git' (my.github.io -> myhub.io);
  now strips only a trailing suffix. add/remove also roll back the
  in-memory list when the save fails, so memory always matches disk.
- config_helper.merge_configs shallow-copied the base, aliasing every
  un-overridden nested dict into the result — now deep-copies.
- startup_validator.validate_all accumulated errors/warnings across
  calls — now resets both lists per run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: cover the previously untested modules

Nine new suites plus an extension, asserting the Phase-1b fixed behavior
and pinning the quirks deliberately left alone:

- test_logging_config.py: formatters (JSON shape, no record mutation,
  single prefix through two handlers), PluginLoggerAdapter precedence,
  setup_logging handler hygiene and LEDMATRIX_DEBUG, log_error exc_info.
- test_startup_validator.py: exact messages, error-vs-warning split,
  accessor split (load_config vs get_config), cache-dir branches with
  os.access monkeypatched (root can write anything in CI), idempotence,
  raise_on_errors classification precedence.
- test_config_helper.py (full): load/save round trips, dot-notation
  get/set incl. silent-failure contract, post-fix no-aliasing merge,
  schema validation branches, the '{id}_config' key pin, default-enabled
  pin.
- test_saved_repositories.py: three load shapes, bare-list rewrite pin,
  trailing-only .git strip (my.github.io regression), save-failure
  rollback, type-classification case-sensitivity pin.
- test_api_helper.py: rate-limit math, cache-hit short circuit, ESPN
  URL/key formats, exact User-Agent guard, retry adapter, post-fix
  clear_cache against the real CacheManager surface, ttl-dropped pin.
- test_base_odds_manager.py: cache-key/URL construction, no_odds
  sentinel round trip, stale-cache fallback, null-safe extraction,
  ML-only formatting, is_odds_available truth table (ML-blind by
  contract), config key/attr mismatch pin.
- test_dynamic_team_resolver.py: expansion/dedup/slicing, dropped
  unknown-dynamic names (TOP_ substring hazard pinned), genuinely
  shared class cache (second instance: zero HTTP), TTL expiry,
  failure degradation without raising.
- test_display_helper.py (full): the fixed error/no-data renders,
  combined scorebug top line, non-blank ticker with scroll_speed
  no-op pin, composite upconversion, logo bleed positions, square
  orientation pin.
- test_skin_runtime_cache.py: discovery-cache hit/invalidation
  semantics (manifest mtime, .py edits pinned as non-invalidating),
  sys.modules namespacing contract incl. bare-name restore and stdlib
  shadowing, entry-module execute-once, API minor-version tolerance,
  skin_matches_target table.
- test_sports_capabilities.py (extended): _draw_celebration_layout
  executed for real (flash window, matrix-dims fallback, highlight
  alternation, logo-failure isolation), _should_celebrate_for direct,
  strict duration boundary, score_to_int edges, both-teams-score
  precedence, expired-coalesce refire, disabled-win baseline
  preservation, id-less prune.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: real schedule/dim coverage for DisplayController; fix two vacuous schedule tests

New test_display_controller_schedule.py drives _check_schedule and
_check_dim_schedule on a bare controller stub: same-day and
midnight-crossing windows with inclusive boundaries, global vs per-day vs
legacy-inferred modes (and dim's global-only default — no legacy
inference), per-day disabled days, invalid %H:%M fallbacks, unknown
timezone -> UTC, dim_brightness default 30, inactive-display short
circuit, and the _was_display_active/_was_dimmed transition flags.

test_display_controller.py's test_schedule_disabled and
test_active_hours patched config_service.get_config — which
_check_schedule never reads — so both asserted the init-default value
and could not fail. Rewritten on the test_inactive_hours pattern
(inject controller.config['schedule'], reset the minute gate, flip the
flag to the opposite state first so the assertion has teeth).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* ci: raise coverage floor to 48%

Measured 50% with the new suites in place (was 47% baseline when the
gate was introduced at 45); floor stays two points under measured.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* fix: address CodeQL alert and review findings

- config_manager: the "secrets list longer than config list" warning now
  interpolates only config-side data (no key name or secrets-derived
  values), resolving the CodeQL clear-text-logging alert.
- base_plugin: validate_config rejects bool display_duration, matching
  get_display_duration (bool is an int subclass and would otherwise pass
  as a positive number).
- config_helper: merge_configs deep-copies override values in the
  non-recursive branch so mutating the merged result cannot reach back
  into override_config.
- saved_repositories: saves are atomic (temp file + fsync + os.replace),
  so a failed write can no longer truncate saved_repositories.json.
- tests: regression cases for each fix, plus a pin that whole-item
  array secrets (key[] + key[].field both marked) strip to empty {}
  skeletons — no secret values can reach config.json.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

---------

Co-authored-by: Claude <noreply@anthropic.com>
 2026-08-07 16:17:11 -04:00
ChuckandClaude Fable 5 d6c5f97c13 Test suite overhaul + fixes for the three bugs it uncovered (#441) 
* ci: run the whole test tree and make the plugin-safety job assert something real

The unit-tests CI job ran an explicit 24-file allowlist that had rotted:
63 of 90 test files (display, vegas, store manager, web API, web_interface)
never ran on a PR. The job now runs all of test/ (minus test/plugins, which
the plugin-safety job owns) so new test files are enrolled by default and
any exclusion needs a visible, commented --ignore.

The plugin-safety job was a green no-op: plugins/ is empty in CI, so every
test skipped with 'Manifest not found'. It now renders a bundled
deterministic fixture plugin (test/fixtures/plugins/ci-fixture-plugin,
golden images included for all 8 default sizes) via LEDMATRIX_PLUGINS_DIR,
and sets LEDMATRIX_REQUIRE_PLUGINS=1 so discovering zero plugins fails
loudly instead of skipping green. The per-plugin suites document that they
target dev machines with real plugins installed.

Coverage is now measured and enforced in exactly one place — the CI
unit-tests step (--cov=src --cov=web_interface --cov-fail-under=45, from a
measured 47% baseline). pytest.ini previously declared --cov-fail-under=30
but CI always passed --no-cov, so the gate had never run anywhere; local
pytest is now coverage-free and fast.

Enabling the 63 unenrolled files surfaced three cases of test rot, fixed
here: test_display_controller_vegas_tick.py could not collect without the
hardware rgbmatrix module (now uses the emulator convention), the
state-reconciliation unrecoverable-cache tests broke when production added
the is_plugin_uninstalled tombstone check (bare Mock returned truthy),
and test_get_system_status assumed the optional psutil dependency
(now installed via requirements-test.txt and guarded by importorskip).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: replace can't-fail tests with real assertions

test_font_manager.py was 5 of 6 tests shaped as 'try: call(); assert True /
except: assert True' — running in CI while unable to fail on any
regression. Rewritten against the real FontManager API and the bundled
assets/fonts: returned font types, cache-hit identity, distinct entries per
size, default-font fallback for unknown families and corrupt files
(recorded in failed_loads), BDF native-size reading, text measurement, and
cache lifecycle.

test_display_manager.py's test_draw_text ended in 'assert True'; it now
renders onto a known-black canvas and asserts pixels were actually lit —
which required un-breaking the fixture's freetype MagicMock so draw_text's
isinstance check doesn't silently swallow the draw.

test_display_controller.py carried a permanently-skipped test whose skip
reason already declared it redundant; deleted.

Both display test files now set EMULATOR=true before importing
display_manager (the same convention as test_display_dirty_tracking.py) so
they collect standalone instead of depending on which test module imports
display_manager first.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: cover the untested fragile logic (compatibility gate, secrets, config merges, durations, skin cards)

New unit tests for pure or filesystem-only logic that previously had zero
direct coverage:

- test_compatibility.py: the semver install gate (parse_semver suffix
  handling, every range operator, TRUSTWORTHY_FLOOR behavior for cores
  reporting untrustworthy versions, 'more restrictive wins', and the
  malformed-manifest shapes that used to raise).
- test/web_interface/test_secret_helpers.py: the canonical x-secret
  helpers — find/separate/mask/remove, array-item secrets, no input
  mutation, and a separate->recombine round-trip.
- test/web_interface/test_api_v3_helpers.py: the module-level helpers
  behind the plugin config save endpoint (_is_plugin_update_available,
  _coerce_to_bool including the int==1 quirk, deep_merge including its
  shared-subtree shallowness, _parse_form_value, dotted-key-aware
  _get_schema_property/_set_nested_value).
- test_base_plugin_duration.py: get_display_duration's full coercion
  ladder (instance attr -> config -> 15.0), including the bool-is-int
  quirk where display_duration=True means one second.
- test_config_manager_secrets.py: the secrets round-trip — deep-merge on
  load, strip on save, group pruning, the load fast path — and two
  characterized sharp edges marked SUSPECTED BUG: an unreadable secrets
  file at save time writes secrets into config.json in plaintext, and a
  same-mtime-same-size content swap is served stale.
- test_schema_manager_merge.py: merge_with_defaults branch behavior (None
  replacement vs falsey preservation, dict-vs-scalar mismatches, arrays
  replaced wholesale, defaults never mutated).
- test_skin_system.py (extended): render_skin_card shares _render_game's
  3-strike counter but never resets it on success — the asymmetry is
  pinned in both directions, along with card fallthrough and the disable
  interaction between the two paths.

Suspected bugs are characterized, not fixed — each carries a comment so a
future behavior change is deliberate rather than accidental.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: add drift guards for cross-file contracts

Three guard suites that pin contracts spanning multiple files, where one
side changing unilaterally breaks the other silently:

- test_version_comparison_consistency.py: the repo's four version
  comparators (compatibility.parse_semver, api_v3's packaging-based
  _is_plugin_update_available, store_manager update_plugin's raw string
  equality, skin_runtime._major) answer differently on the same inputs.
  A table pins each one's verdict; update_plugin is driven through its
  real code path to show the SUSPECTED BUGs: 'v1.2.0' vs '1.2.0'
  triggers a full reinstall the UI calls unnecessary, and a locally-ahead
  plugin gets downgraded. A pairwise-ordering check keeps parse_semver
  agreeing with packaging on plain X.Y.Z.
- test/web_interface/test_secret_separation_parity.py: api_v3.py carries
  three inline copies of find_secret_fields/separate_secrets that lack
  the canonical module's array-item support. The copy count is asserted
  exact (it may only go down; new copies must import
  src/web_interface/secret_helpers), the missing-array-support gap is
  asserted so it can't grow silently, and the canonical behavior that
  migration will adopt is documented executably.
- test_discovery_path_contract.py: the three 'where is plugin X'
  resolvers (PluginManager discovery, StoreManager._find_plugin_path,
  SchemaManager.get_schema_path) agree on the configured directory, and
  their divergent fallback chains are characterized. Also pins the
  .standalone-backup- naming contract shared by store rollback and
  discovery, and _resolve_skin_target's path-traversal rejection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* test: address review feedback — fixture lifecycle, test names, ClassVar

- ci-fixture-plugin: call display_manager.clear() before rendering (per
  plugin guidelines — the fixture should model a well-behaved plugin),
  add a class docstring, and document why Pillow is deliberately not
  pinned in its requirements.txt (core dependency; harness installs
  nothing).
- Rename two tests whose names contradicted their assertions:
  test_unparseable_core_version_is_compatible ->
  test_unparseable_core_with_high_floor_is_blocked, and
  test_unreadable_secrets_file... -> test_corrupt_secrets_file...
- Annotate TestGetSchemaProperty.SCHEMA as ClassVar (RUF012).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* ci: allow manual test.yml runs via workflow_dispatch

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* fix: unify version comparison, refuse secret-leaking saves, reset skin strikes on card success

Fixes the three suspected bugs this PR's characterization tests pinned,
flipping those tests to assert the corrected behavior:

- plugins/store: ONE shared update comparator. New
  compatibility.is_update_available() (PEP 440 via packaging) is now used
  by both the web UI's update badge (api_v3._is_plugin_update_available
  is a thin alias) and store_manager.update_plugin's reinstall decision.
  Previously update_plugin used raw string equality: 'v1.2.0' vs '1.2.0'
  triggered a full reinstall the UI called unnecessary, and a locally-
  ahead plugin (2.0.0 installed, registry 1.9.0) was silently DOWNGRADED.
  Now equivalent spellings skip the reinstall and locally-ahead versions
  are never downgraded; unparseable versions still reconcile by
  reinstalling from the registry.

- config: save_config and save_config_atomic now refuse (ConfigError)
  when config_secrets.json exists but cannot be loaded. Both previously
  proceeded without stripping, writing the merged secrets into
  config.json in plaintext. The shared _load_secrets_for_save() helper
  raises with an actionable message instead; a missing secrets file is
  still fine (nothing to strip), and _migrate_config's catch-all keeps
  boot resilient.

- skins: render_skin_card resets _skin_failures on both success paths
  (vegas card returned, or mode renderer handled), mirroring
  _render_game. Transient card failures no longer accumulate across a
  session until they permanently disable a working skin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

* fix: harden shared comparator edges from review

- is_update_available: reject truthy non-string versions (a malformed
  manifest can carry a number; packaging raises TypeError on those) by
  surfacing the mismatch instead of raising.
- store_manager.update_plugin: drop the truthiness gate around the
  comparator so a missing version on either side follows the shared
  'no update' verdict, keeping the store consistent with the UI badge;
  a missing manifest still uses the reinstall recovery path.
- config_manager._load_secrets_for_save: catch only expected read/parse
  failures (OSError/ValueError/RecursionError) so implementation bugs
  propagate as themselves, and log with traceback.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh

---------

Co-authored-by: Claude <noreply@anthropic.com>
 2026-08-07 10:17:30 -04:00