Files
LEDMatrix/test/web_interface/test_state_reconciliation.py
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

509 lines
20 KiB
Python

"""
Tests for state reconciliation system.
"""
import unittest
import tempfile
import shutil
import json
from pathlib import Path
from unittest.mock import Mock, patch
from src.plugin_system.state_reconciliation import (
StateReconciliation,
InconsistencyType,
FixAction,
ReconciliationResult
)
from src.plugin_system.state_manager import PluginStateManager, PluginStateStatus
class TestStateReconciliation(unittest.TestCase):
"""Test state reconciliation system."""
def setUp(self):
"""Set up test fixtures."""
self.temp_dir = Path(tempfile.mkdtemp())
self.plugins_dir = self.temp_dir / "plugins"
self.plugins_dir.mkdir()
# Create mock managers
self.state_manager = Mock(spec=PluginStateManager)
self.config_manager = Mock()
self.plugin_manager = Mock()
# Initialize reconciliation system
self.reconciler = StateReconciliation(
state_manager=self.state_manager,
config_manager=self.config_manager,
plugin_manager=self.plugin_manager,
plugins_dir=self.plugins_dir
)
def tearDown(self):
"""Clean up test fixtures."""
shutil.rmtree(self.temp_dir)
def test_reconcile_no_inconsistencies(self):
"""Test reconciliation with no inconsistencies."""
# Setup: All states are consistent
self.config_manager.load_config.return_value = {
"plugin1": {"enabled": True}
}
self.state_manager.get_all_states.return_value = {
"plugin1": Mock(
enabled=True,
status=PluginStateStatus.ENABLED,
version="1.0.0"
)
}
self.plugin_manager.plugin_manifests = {"plugin1": {}}
self.plugin_manager.plugins = {"plugin1": Mock()}
# Create plugin directory
plugin_dir = self.plugins_dir / "plugin1"
plugin_dir.mkdir()
manifest_path = plugin_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 1"}, f)
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify
self.assertIsInstance(result, ReconciliationResult)
self.assertEqual(len(result.inconsistencies_found), 0)
self.assertTrue(result.reconciliation_successful)
def test_plugin_missing_in_config(self):
"""Test detection of plugin missing in config."""
# Setup: Plugin exists on disk but not in config
self.config_manager.load_config.return_value = {}
self.state_manager.get_all_states.return_value = {}
self.plugin_manager.plugin_manifests = {}
self.plugin_manager.plugins = {}
# Create plugin directory
plugin_dir = self.plugins_dir / "plugin1"
plugin_dir.mkdir()
manifest_path = plugin_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 1"}, f)
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify inconsistency detected
self.assertEqual(len(result.inconsistencies_found), 1)
inconsistency = result.inconsistencies_found[0]
self.assertEqual(inconsistency.plugin_id, "plugin1")
self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_MISSING_IN_CONFIG)
self.assertTrue(inconsistency.can_auto_fix)
self.assertEqual(inconsistency.fix_action, FixAction.AUTO_FIX)
def test_plugin_missing_on_disk(self):
"""Test detection of plugin missing on disk."""
# Setup: Plugin in config but not on disk
self.config_manager.load_config.return_value = {
"plugin1": {"enabled": True}
}
self.state_manager.get_all_states.return_value = {}
self.plugin_manager.plugin_manifests = {}
self.plugin_manager.plugins = {}
# Don't create plugin directory
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify inconsistency detected
self.assertEqual(len(result.inconsistencies_found), 1)
inconsistency = result.inconsistencies_found[0]
self.assertEqual(inconsistency.plugin_id, "plugin1")
self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_MISSING_ON_DISK)
self.assertFalse(inconsistency.can_auto_fix)
self.assertEqual(inconsistency.fix_action, FixAction.MANUAL_FIX_REQUIRED)
def test_enabled_state_mismatch(self):
"""Test detection of enabled state mismatch."""
# Setup: Config says enabled=True, state manager says enabled=False
self.config_manager.load_config.return_value = {
"plugin1": {"enabled": True}
}
self.state_manager.get_all_states.return_value = {
"plugin1": Mock(
enabled=False,
status=PluginStateStatus.DISABLED,
version="1.0.0"
)
}
self.plugin_manager.plugin_manifests = {"plugin1": {}}
self.plugin_manager.plugins = {}
# Create plugin directory
plugin_dir = self.plugins_dir / "plugin1"
plugin_dir.mkdir()
manifest_path = plugin_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 1"}, f)
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify inconsistency detected
self.assertEqual(len(result.inconsistencies_found), 1)
inconsistency = result.inconsistencies_found[0]
self.assertEqual(inconsistency.plugin_id, "plugin1")
self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_ENABLED_MISMATCH)
self.assertTrue(inconsistency.can_auto_fix)
self.assertEqual(inconsistency.fix_action, FixAction.AUTO_FIX)
def test_auto_fix_plugin_missing_in_config(self):
"""Test auto-fix of plugin missing in config."""
# Setup
self.config_manager.load_config.return_value = {}
self.state_manager.get_all_states.return_value = {}
self.plugin_manager.plugin_manifests = {}
self.plugin_manager.plugins = {}
# Create plugin directory
plugin_dir = self.plugins_dir / "plugin1"
plugin_dir.mkdir()
manifest_path = plugin_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 1"}, f)
# Mock save_config to track calls
saved_configs = []
def save_config(config):
saved_configs.append(config)
self.config_manager.save_config = save_config
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify fix was attempted
self.assertEqual(len(result.inconsistencies_fixed), 1)
self.assertEqual(len(saved_configs), 1)
self.assertIn("plugin1", saved_configs[0])
self.assertEqual(saved_configs[0]["plugin1"]["enabled"], False)
def test_auto_fix_enabled_state_mismatch(self):
"""Test auto-fix of enabled state mismatch."""
# Setup: Config says enabled=True, state manager says enabled=False
self.config_manager.load_config.return_value = {
"plugin1": {"enabled": True}
}
self.state_manager.get_all_states.return_value = {
"plugin1": Mock(
enabled=False,
status=PluginStateStatus.DISABLED,
version="1.0.0"
)
}
self.plugin_manager.plugin_manifests = {"plugin1": {}}
self.plugin_manager.plugins = {}
# Create plugin directory
plugin_dir = self.plugins_dir / "plugin1"
plugin_dir.mkdir()
manifest_path = plugin_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 1"}, f)
# Mock save_config to track calls
saved_configs = []
def save_config(config):
saved_configs.append(config)
self.config_manager.save_config = save_config
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify fix was attempted
self.assertEqual(len(result.inconsistencies_fixed), 1)
self.assertEqual(len(saved_configs), 1)
self.assertEqual(saved_configs[0]["plugin1"]["enabled"], False)
def test_multiple_inconsistencies(self):
"""Test reconciliation with multiple inconsistencies."""
# Setup: Multiple plugins with different issues
self.config_manager.load_config.return_value = {
"plugin1": {"enabled": True}, # Exists in config but not on disk
# plugin2 exists on disk but not in config
}
self.state_manager.get_all_states.return_value = {
"plugin1": Mock(
enabled=True,
status=PluginStateStatus.ENABLED,
version="1.0.0"
)
}
self.plugin_manager.plugin_manifests = {}
self.plugin_manager.plugins = {}
# Create plugin2 directory (exists on disk but not in config)
plugin2_dir = self.plugins_dir / "plugin2"
plugin2_dir.mkdir()
manifest_path = plugin2_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 2"}, f)
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify multiple inconsistencies found
self.assertGreaterEqual(len(result.inconsistencies_found), 2)
# Check types
inconsistency_types = [inc.inconsistency_type for inc in result.inconsistencies_found]
self.assertIn(InconsistencyType.PLUGIN_MISSING_ON_DISK, inconsistency_types)
self.assertIn(InconsistencyType.PLUGIN_MISSING_IN_CONFIG, inconsistency_types)
def test_reconciliation_with_exception(self):
"""Test reconciliation handles exceptions gracefully."""
# Setup: State manager raises exception when getting states
self.config_manager.load_config.return_value = {}
self.state_manager.get_all_states.side_effect = Exception("State manager error")
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify error is handled - reconciliation may still succeed if other sources work
self.assertIsInstance(result, ReconciliationResult)
# Note: Reconciliation may still succeed if other sources provide valid state
def test_fix_failure_handling(self):
"""Test that fix failures are handled correctly."""
# Setup: Plugin missing in config, but save fails
self.config_manager.load_config.return_value = {}
self.state_manager.get_all_states.return_value = {}
self.plugin_manager.plugin_manifests = {}
self.plugin_manager.plugins = {}
# Create plugin directory
plugin_dir = self.plugins_dir / "plugin1"
plugin_dir.mkdir()
manifest_path = plugin_dir / "manifest.json"
with open(manifest_path, 'w') as f:
json.dump({"version": "1.0.0", "name": "Plugin 1"}, f)
# Mock save_config to raise exception
self.config_manager.save_config.side_effect = Exception("Save failed")
# Run reconciliation
result = self.reconciler.reconcile_state()
# Verify inconsistency detected but not fixed
self.assertEqual(len(result.inconsistencies_found), 1)
self.assertEqual(len(result.inconsistencies_fixed), 0)
self.assertEqual(len(result.inconsistencies_manual), 1)
def test_get_config_state_handles_exception(self):
"""Test that _get_config_state handles exceptions."""
# Setup: Config manager raises exception
self.config_manager.load_config.side_effect = Exception("Config error")
# Call method directly
state = self.reconciler._get_config_state()
# Verify empty state returned
self.assertEqual(state, {})
def test_get_disk_state_handles_exception(self):
"""Test that _get_disk_state handles exceptions."""
# Setup: Make plugins_dir inaccessible
with patch.object(self.reconciler, 'plugins_dir', create=True) as mock_dir:
mock_dir.exists.side_effect = Exception("Disk error")
mock_dir.iterdir.side_effect = Exception("Disk error")
# Call method directly
state = self.reconciler._get_disk_state()
# Verify empty state returned
self.assertEqual(state, {})
class TestStateReconciliationUnrecoverable(unittest.TestCase):
"""Tests for the unrecoverable-plugin cache and force reconcile.
Regression coverage for the infinite reinstall loop where a config
entry referenced a plugin not present in the registry (e.g. legacy
'github' / 'youtube' entries). The reconciler used to retry the
install on every HTTP request; it now caches the failure for the
process lifetime and only retries on an explicit ``force=True``
reconcile call.
"""
def setUp(self):
self.temp_dir = Path(tempfile.mkdtemp())
self.plugins_dir = self.temp_dir / "plugins"
self.plugins_dir.mkdir()
self.state_manager = Mock(spec=PluginStateManager)
self.state_manager.get_all_states.return_value = {}
self.config_manager = Mock()
self.config_manager.load_config.return_value = {
"ghost": {"enabled": True}
}
self.plugin_manager = Mock()
self.plugin_manager.plugin_manifests = {}
self.plugin_manager.plugins = {}
# Store manager with an empty registry — install_plugin always fails
self.store_manager = Mock()
self.store_manager.fetch_registry.return_value = {"plugins": []}
self.store_manager.install_plugin.return_value = False
self.store_manager.was_recently_uninstalled.return_value = False
self.reconciler = StateReconciliation(
state_manager=self.state_manager,
config_manager=self.config_manager,
plugin_manager=self.plugin_manager,
plugins_dir=self.plugins_dir,
store_manager=self.store_manager,
)
def tearDown(self):
shutil.rmtree(self.temp_dir)
def test_not_in_registry_marks_unrecoverable_without_install(self):
"""If the plugin isn't in the registry at all, skip install_plugin."""
result = self.reconciler.reconcile_state()
# One inconsistency, unfixable, no install attempt made.
self.assertEqual(len(result.inconsistencies_found), 1)
self.assertEqual(len(result.inconsistencies_fixed), 0)
self.store_manager.install_plugin.assert_not_called()
self.assertIn("ghost", self.reconciler._unrecoverable_missing_on_disk)
def test_subsequent_reconcile_does_not_retry(self):
"""Second reconcile pass must not touch install_plugin or fetch_registry again."""
self.reconciler.reconcile_state()
self.store_manager.fetch_registry.reset_mock()
self.store_manager.install_plugin.reset_mock()
result = self.reconciler.reconcile_state()
# Still one inconsistency, still no install attempt, no new registry fetch
self.assertEqual(len(result.inconsistencies_found), 1)
inc = result.inconsistencies_found[0]
self.assertFalse(inc.can_auto_fix)
self.assertEqual(inc.fix_action, FixAction.MANUAL_FIX_REQUIRED)
self.store_manager.install_plugin.assert_not_called()
self.store_manager.fetch_registry.assert_not_called()
def test_force_reconcile_clears_unrecoverable_cache(self):
"""force=True must re-attempt previously-failed plugins."""
self.reconciler.reconcile_state()
self.assertIn("ghost", self.reconciler._unrecoverable_missing_on_disk)
# Now pretend the registry gained the plugin so the pre-check passes
# and install_plugin is actually invoked.
self.store_manager.fetch_registry.return_value = {
"plugins": [{"id": "ghost"}]
}
self.store_manager.install_plugin.return_value = True
self.store_manager.install_plugin.reset_mock()
# Config still references ghost; disk still missing it — the
# reconciler should re-attempt install now that force=True cleared
# the cache. Use assert_called_once_with so a future regression
# that accidentally triggers a second install attempt on force=True
# is caught.
result = self.reconciler.reconcile_state(force=True)
self.store_manager.install_plugin.assert_called_once_with("ghost")
def test_registry_unreachable_does_not_mark_unrecoverable(self):
"""Transient registry failures should not poison the cache."""
self.store_manager.fetch_registry.side_effect = Exception("network down")
result = self.reconciler.reconcile_state()
self.assertEqual(len(result.inconsistencies_found), 1)
self.assertNotIn("ghost", self.reconciler._unrecoverable_missing_on_disk)
self.store_manager.install_plugin.assert_not_called()
def test_recently_uninstalled_skips_auto_repair(self):
"""A freshly-uninstalled plugin must not be resurrected by the reconciler."""
self.store_manager.was_recently_uninstalled.return_value = True
self.store_manager.fetch_registry.return_value = {
"plugins": [{"id": "ghost"}]
}
result = self.reconciler.reconcile_state()
self.assertEqual(len(result.inconsistencies_found), 1)
inc = result.inconsistencies_found[0]
self.assertFalse(inc.can_auto_fix)
self.assertEqual(inc.fix_action, FixAction.MANUAL_FIX_REQUIRED)
self.store_manager.install_plugin.assert_not_called()
def test_real_store_manager_empty_registry_on_network_failure(self):
"""Regression: using the REAL PluginStoreManager (not a Mock), verify
the reconciler does NOT poison the unrecoverable cache when
``fetch_registry`` fails with no stale cache available.
Previously, the default stale-cache fallback in ``fetch_registry``
silently returned ``{"plugins": []}`` on network failure with no
cache. The reconciler's ``_auto_repair_missing_plugin`` saw "no
candidates in registry" and marked everything unrecoverable — a
regression that would bite every user doing a fresh boot on flaky
WiFi. The fix is ``fetch_registry(raise_on_failure=True)`` in
``_auto_repair_missing_plugin`` so the reconciler can tell a real
registry miss from a network error.
"""
from src.plugin_system.store_manager import PluginStoreManager
import requests as real_requests
real_store = PluginStoreManager(plugins_dir=str(self.plugins_dir))
real_store.registry_cache = None # fresh boot, no cache
real_store.registry_cache_time = None
# Stub the underlying HTTP so no real network call is made but the
# real fetch_registry code path runs.
real_store._http_get_with_retries = Mock(
side_effect=real_requests.ConnectionError("wifi down")
)
reconciler = StateReconciliation(
state_manager=self.state_manager,
config_manager=self.config_manager,
plugin_manager=self.plugin_manager,
plugins_dir=self.plugins_dir,
store_manager=real_store,
)
result = reconciler.reconcile_state()
# One inconsistency (ghost is in config, not on disk), but
# because the registry lookup failed transiently, we must NOT
# have marked it unrecoverable — a later reconcile (after the
# network comes back) can still auto-repair.
self.assertEqual(len(result.inconsistencies_found), 1)
self.assertNotIn("ghost", reconciler._unrecoverable_missing_on_disk)
if __name__ == '__main__':
unittest.main()