2 Commits

Include renames

Author	SHA1	Message	Date
ChuckandClaude Opus 5.5	b11bcfa204	fix(plugins): store and plugin-manager bugs; tidy src/plugin_system (#635) ... * fix(store): don't read a ZIP-installed plugin's remote from the LEDMatrix repo update_plugin looked up remote.origin.url with `git -C <plugin> config --local` for plugins that are not git checkouts. Under plugin-repos/ git walks up to the enclosing LEDMatrix repository, so the lookup returned LEDMatrix's own URL and a plugin missing from the registry was "reinstalled" from the LEDMatrix repo. Only ask git when the plugin directory has its own .git, the test _get_local_git_info already uses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(schema): report each missing required field once, by name validate_config_against_schema ran its own required-fields loop after Draft7Validator.iter_errors, which already yields one `required` error per missing field, so every missing top-level field was listed twice. The validator's copy also printed the schema's whole `required` list ("Missing required property '['api_key', 'city']'") instead of the field. Drop the loop and take the field name from the error itself. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(store): stop mangling repository URLs that contain ".git" install_from_url and fetch_registry_from_url cleaned URLs with `rstrip('/').replace('.git', '')`, which removes ".git" anywhere: https://github.com/user/my.github.io became .../myhub.io, so installing or browsing that repository asked GitHub for one that does not exist. Add src/plugin_system/repo_urls.py with one anchored normalize_repo_url(), same_repo() for comparisons, github_owner_repo() and github_api_headers(), and use them for the five copies of the owner/repo parsing and GitHub headers in the store and for saved repositories. GitHub URLs are now recognised by urlparse().hostname everywhere: _get_latest_commit_info used a substring test, and _install_from_monorepo_api parsed any host. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(store): install a repository whose only branch is not main/master _install_via_git returned None both when every clone failed and when the last-resort clone of the repository's default branch succeeded. _install_plugin_impl papered over it with `and not plugin_path.exists()`; install_from_url did not, so a repository whose only branch is e.g. `develop` was cloned, then treated as a failure, then "downloaded" from main/master archives that do not exist. After a default-branch clone, return the branch the clone checked out (read from .git/HEAD), so None means failure and nothing else, and give both callers the same `branch_used is None` fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): judge the memory limit on each call's own growth monitor_call stores `metrics.memory_mb = max(previous, growth)`, and _check_limits compared that high-water mark with max_memory_mb. It never decreases, so once one update() grew the process past the limit every later call raised ResourceLimitExceeded and the circuit breaker kept reopening. Pass the call's own RSS growth to _check_limits; keep the high-water mark for reporting and document what it measures. Remove ResourceMetrics.update_average_execution_time: nothing called it, and it overwrote the running total with the average. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): reload_plugin re-reads the manifest from the discovered directory reload_plugin read `plugins_dir / plugin_id / "manifest.json"`, ignoring the discovery map and the plugin_dirs rules. For a plugin whose directory name differs from its manifest id the path did not exist, the re-read was skipped without a word, and the reload kept the stale manifest. Resolve the directory with find_plugin_directory, as load_plugin does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): drop the always-null last_display from plugin state info PluginStateManager reported `last_display` from `_last_display`, which nothing ever wrote, so it was null for every plugin. Recording it in PluginExecutor.execute_display would not help: get_state_info's only reader is the web process, whose PluginManager never calls display(). Remove the field, its dict and get_last_display() (no caller in core, the web UI or the plugin monorepo). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(store): share the rollback and requirements helpers, drop dead code - install_plugin and _reinstall_with_rollback set aside, discard and restore the old copy through _set_aside/_discard_backup/_restore_backup instead of two copies of the same blocks. - The loader and the store run the same pre-pip checks through contained_plugin_dir() and requirements_to_install() in plugin_loader. They still invoke pip differently (sys.executable -m pip vs. the sudo wrapper). `except (BrokenPipeError, OSError)` + `isinstance(e, OSError)` becomes `except OSError` checking errno.EPIPE. - load_module never returns None, so load_plugin's check is gone and the docstring says what it raises. - Remove the always-true JSONSCHEMA_AVAILABLE, the inline re-imports of re and permission_utils, the fake status_result object nobody reads, hasattr(git_error, 'cmd'), a redundant "merge conflict" test and `import traceback` (exc_info=True does it). - Correct comments: install_from_url names the directory for the caller's id when given (not always the manifest id), _get_local_git_info saves one git subprocess (not four), _enrich calls two helpers, search_plugins documents all its arguments, _find_plugin_path states its behaviour instead of a TODO, and history narration is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): tidy base_plugin, correct plugin_manager/state comments - base_plugin: drop the unused `import logging`; get_display_duration runs the instance value and the config value through one _positive_seconds() helper instead of two copies of the coercion; the 'static'/'none'/fallback branches of get_vegas_display_mode, which all returned FIXED_SEGMENT, are one; fix the mis-indented validate_config example; say that get_supported_vegas_modes/get_vegas_segment_width are not consulted by core (kept, plugins override them). - schema_manager: import expand_style_elements normally rather than swallowing an ImportError of a core module. - plugin_manager: the plugins directory is the configured one (plugin-repos/ by default), not plugins/; get_config() returns the live dict, not a copy, so the interval cache comments say what it saves. - state_manager: config_version and the file version are not used to detect corruption; say what they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): stop writing data/plugin_operations.json PluginOperationQueue wrote its finished-operation history to data/plugin_operations.json after every operation, and read it back only into its own in-memory list, which only get_operation_history() exposes -- and nothing calls that. The operation-history endpoint reads OperationHistory (data/operation_history.json). No code in src/, web_interface/, scripts/ or test/ reads the file. Drop the history_file/lazy_load parameters and the load/save code; the bounded in-memory history stays. web_interface/app.py and the integration test stop passing the removed arguments. An existing data/plugin_operations.json is left in place (data/* is gitignored). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): plugin-system Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-09-24 17:32:02 -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