fix(plugins): replace dependency marker files with a real satisfaction check

The .dependencies_installed hash-marker system only tracked "was this exact
requirements.txt hashed before" — not whether the packages it names are
actually present. That made it fragile (a wiped venv, a manually removed
package, or a lost/corrupted marker forces a needless full pip reinstall or,
worse, a false skip) and produced dead weight for the ~10 plugins whose
requirements.txt is comment-only (they still paid a pip subprocess on first
boot before a marker existed).

Replace it with requirements_are_satisfied() in plugin_loader.py, which
checks each real requirement line against importlib.metadata directly, so
install_dependencies() only shells out to pip when something is actually
missing or version-mismatched. Drops the marker file entirely: removed all
marker read/write sites in plugin_loader.py and store_manager.py, the
now-pointless marker-cleanup step in the git-update path, the unused legacy
marker implementation in plugin_manager.py, and the already-stale
clear_dependency_markers.sh script.

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

requirements.txt

@@ -43,6 +43,9 @@ websocket-client>=1.8.0,<2.0.0
 # JSON Schema validation
 jsonschema>=4.20.0,<5.0.0
 
+# Requirement specifier parsing (plugin dependency satisfaction checks)
+packaging>=23.0,<27.0
+
 # Testing dependencies
 pytest>=9.0.3,<10.0.0
 pytest-cov>=4.1.0,<5.0.0

scripts/clear_dependency_markers.sh

@@ -1,29 +0,0 @@
-#!/bin/bash
-# Clear all plugin dependency markers to force fresh dependency check
-# Useful after updating plugins or troubleshooting dependency issues
-
-echo "Clearing plugin dependency markers..."
-
-# Check both possible cache locations
-CACHE_DIRS=(
-    "/var/cache/ledmatrix"
-    "$HOME/.cache/ledmatrix"
-)
-
-for CACHE_DIR in "${CACHE_DIRS[@]}"; do
-    if [ -d "$CACHE_DIR" ]; then
-        echo "Checking $CACHE_DIR..."
-        marker_count=$(find "$CACHE_DIR" -name "plugin_*_deps_installed" 2>/dev/null | wc -l)
-        if [ "$marker_count" -gt 0 ]; then
-            echo "Found $marker_count dependency marker(s) in $CACHE_DIR"
-            find "$CACHE_DIR" -name "plugin_*_deps_installed" -delete
-            echo "Cleared $marker_count marker(s)"
-        else
-            echo "No dependency markers found in $CACHE_DIR"
-        fi
-    fi
-done
-
-echo "Done! Dependency markers cleared."
-echo "Next startup will check and install dependencies as needed."
-

src/plugin_system/plugin_loader.py

@@ -5,10 +5,10 @@ Handles plugin module imports, dependency installation, and class instantiation.
 Extracted from PluginManager to improve separation of concerns.
 """
 
-import hashlib
-import json
 import importlib
+import importlib.metadata
 import importlib.util
+import json
 import os
 import sys
 import subprocess
@@ -17,12 +17,80 @@ from pathlib import Path
 from typing import Dict, Any, Optional, Tuple, Type
 import logging
 
+from packaging.requirements import InvalidRequirement, Requirement
+
 from src.exceptions import PluginError
 from src.logging_config import get_logger
-from src.common.permission_utils import (
-    ensure_file_permissions,
-    get_plugin_file_mode
-)
+
+
+def requirements_has_real_deps(requirements_file: str) -> bool:
+    """
+    Check whether a requirements.txt actually specifies anything to install.
+
+    Plugins that ship all their dependencies with LEDMatrix core often keep a
+    requirements.txt where every line is commented out, for documentation
+    purposes only. Running pip against such a file still pays the full
+    subprocess/resolver cost for zero effect, so callers should skip the
+    install step entirely when this returns False.
+    """
+    try:
+        with open(requirements_file, 'r', encoding='utf-8') as fh:
+            for line in fh:
+                line = line.strip()
+                if line and not line.startswith('#'):
+                    return True
+    except OSError:
+        # Let the caller's own file handling report the error.
+        return True
+    return False
+
+
+def requirements_are_satisfied(requirements_file: str) -> bool:
+    """
+    Check whether every real requirement line in requirements.txt is already
+    satisfied by packages installed in the current interpreter.
+
+    This replaces marker-file tracking with a direct fact check, so it's
+    immune to stale/missing/corrupted markers: it looks at what's actually
+    importable right now rather than trusting a hash comparison from a
+    previous run. Anything ambiguous (pip options, unparseable lines,
+    extras, unresolvable versions) conservatively returns False so the
+    caller falls through to running pip — this check only ever saves work,
+    never masks a real install.
+    """
+    try:
+        with open(requirements_file, 'r', encoding='utf-8') as fh:
+            lines = fh.readlines()
+    except OSError:
+        return False
+
+    for raw_line in lines:
+        line = raw_line.strip()
+        if not line or line.startswith('#'):
+            continue
+        if line.startswith('-'):
+            return False  # pip option (-r, --index-url, ...), can't verify
+
+        try:
+            req = Requirement(line)
+        except InvalidRequirement:
+            return False
+
+        if req.extras:
+            return False  # verifying extras' sub-dependencies isn't worth it here
+
+        if req.marker is not None and not req.marker.evaluate():
+            continue  # not applicable on this platform/interpreter
+
+        try:
+            installed_version = importlib.metadata.version(req.name)
+        except importlib.metadata.PackageNotFoundError:
+            return False
+
+        if req.specifier and not req.specifier.contains(installed_version, prereleases=True):
+            return False
+
+    return True
 
 
 class PluginLoader:
@@ -186,33 +254,23 @@ class PluginLoader:
                 return False
 
         requirements_file = os.path.join(safe_plugin_dir, "requirements.txt")
-        marker_file = os.path.join(safe_plugin_dir, ".dependencies_installed")
 
         if not os.path.isfile(requirements_file):
             return True  # No dependencies needed
 
-        try:
-            with open(requirements_file, 'rb') as fh:
-                current_hash = hashlib.sha256(fh.read()).hexdigest()
-        except OSError as e:
-            self.logger.error("Failed to read requirements.txt for %s: %s", plugin_id, e)
-            return False
+        if not requirements_has_real_deps(requirements_file):
+            self.logger.debug(
+                "requirements.txt for %s has no real dependencies (comments/blank only), skipping pip",
+                plugin_id
+            )
+            return True
 
-        # Skip if requirements.txt hasn't changed since last install
-        if os.path.isfile(marker_file):
-            try:
-                with open(marker_file, 'r', encoding='utf-8') as fh:
-                    stored_hash = fh.read().strip()
-            except OSError as e:
-                self.logger.warning(
-                    "Could not read dependency marker for %s (%s), will reinstall dependencies",
-                    plugin_id, e
-                )
-            else:
-                if stored_hash == current_hash:
-                    self.logger.debug("Dependencies already installed for %s (requirements unchanged)", plugin_id)
-                    return True
-                self.logger.info("Requirements changed for %s, reinstalling dependencies", plugin_id)
+        if requirements_are_satisfied(requirements_file):
+            self.logger.debug(
+                "Dependencies for %s already satisfied in current environment, skipping pip",
+                plugin_id
+            )
+            return True
 
         try:
             self.logger.info("Installing dependencies for plugin %s...", plugin_id)
@@ -225,12 +283,6 @@ class PluginLoader:
             )
 
             if result.returncode == 0:
-                try:
-                    with open(marker_file, 'w', encoding='utf-8') as fh:
-                        fh.write(current_hash)
-                    ensure_file_permissions(Path(marker_file), get_plugin_file_mode())
-                except OSError as marker_err:
-                    self.logger.debug("Could not write dependency marker for %s: %s", plugin_id, marker_err)
                 self.logger.info("Dependencies installed successfully for %s", plugin_id)
                 return True
             else:
@@ -242,8 +294,7 @@ class PluginLoader:
                 # the system copy instead of trying to replace it — matching the
                 # retry already used by install_dependencies_apt.py / safe_pip_install.sh.
                 # Without this retry, the plugin would silently keep running against
-                # whatever version the system happened to ship, even though the
-                # marker below claims the requirement is satisfied.
+                # whatever version the system happened to ship.
                 if "uninstall-no-record-file" in stderr:
                     self.logger.warning(
                         "Dependencies for %s conflict with a system-managed package "
@@ -280,12 +331,6 @@ class PluginLoader:
                             "system-managed version satisfies the requirement",
                             plugin_id
                         )
-                    try:
-                        with open(marker_file, 'w', encoding='utf-8') as fh:
-                            fh.write(current_hash)
-                        ensure_file_permissions(Path(marker_file), get_plugin_file_mode())
-                    except OSError as marker_err:
-                        self.logger.debug("Could not write dependency marker for %s: %s", plugin_id, marker_err)
                     return True
                 self.logger.warning(
                     "Dependency installation returned non-zero exit code for %s: %s",

src/plugin_system/plugin_manager.py

@@ -9,7 +9,6 @@ API Version: 1.0.0
 
 import json
 import sys
-import subprocess
 import time
 import threading
 from pathlib import Path
@@ -177,90 +176,6 @@ class PluginManager:
 
         return plugin_ids
 
-    def _get_dependency_marker_path(self, plugin_id: str) -> Path:
-        """Get path to dependency installation marker file."""
-        plugin_dir = self.plugins_dir / plugin_id
-        if not plugin_dir.exists():
-            # Try with ledmatrix- prefix
-            plugin_dir = self.plugins_dir / f"ledmatrix-{plugin_id}"
-        return plugin_dir / ".dependencies_installed"
-
-    def _check_dependencies_installed(self, plugin_id: str) -> bool:
-        """Check if dependencies are already installed for a plugin."""
-        marker_path = self._get_dependency_marker_path(plugin_id)
-        return marker_path.exists()
-
-    def _mark_dependencies_installed(self, plugin_id: str) -> None:
-        """Mark dependencies as installed for a plugin."""
-        marker_path = self._get_dependency_marker_path(plugin_id)
-        try:
-            marker_path.touch()
-            # Set proper file permissions after creating marker
-            from src.common.permission_utils import (
-                ensure_file_permissions,
-                get_plugin_file_mode
-            )
-            ensure_file_permissions(marker_path, get_plugin_file_mode())
-        except (OSError, PermissionError) as e:
-            self.logger.warning("Could not create dependency marker for %s: %s", plugin_id, e)
-
-    def _remove_dependency_marker(self, plugin_id: str) -> None:
-        """Remove dependency installation marker."""
-        marker_path = self._get_dependency_marker_path(plugin_id)
-        try:
-            if marker_path.exists():
-                marker_path.unlink()
-        except (OSError, PermissionError) as e:
-            self.logger.warning("Could not remove dependency marker for %s: %s", plugin_id, e)
-
-    def _install_plugin_dependencies(self, requirements_file: Path) -> bool:
-        """
-        Install plugin dependencies from requirements.txt.
-        
-        Args:
-            requirements_file: Path to requirements.txt
-            
-        Returns:
-            True if installation succeeded or not needed, False on error
-        """
-        try:
-            self.logger.info("Installing dependencies from %s", requirements_file)
-            result = subprocess.run(
-                [sys.executable, "-m", "pip", "install", "--break-system-packages", "--no-cache-dir", "-r", str(requirements_file)],
-                capture_output=True,
-                text=True,
-                timeout=300,
-                check=False
-            )
-            
-            if result.returncode == 0:
-                self.logger.info("Dependencies installed successfully")
-                return True
-            else:
-                self.logger.warning("Dependency installation returned non-zero exit code: %s", result.stderr)
-                return False
-        except subprocess.TimeoutExpired:
-            self.logger.error("Dependency installation timed out")
-            return False
-        except FileNotFoundError as e:
-            self.logger.warning("Command not found: %s. Skipping dependency installation", e)
-            return True
-        except (BrokenPipeError, OSError) as e:
-            # Handle broken pipe errors (errno 32) which can occur during pip downloads
-            # Often caused by network interruptions or output buffer issues
-            if isinstance(e, OSError) and e.errno == 32:
-                self.logger.error(
-                    "Broken pipe error during dependency installation. "
-                    "This usually indicates a network interruption or pip output buffer issue. "
-                    "Try installing again or check your network connection."
-                )
-            else:
-                self.logger.error("OS error during dependency installation: %s", e)
-            return False
-        except Exception as e:
-            self.logger.error("Unexpected error installing dependencies: %s", e, exc_info=True)
-            return True
-
     def load_plugin(self, plugin_id: str) -> bool:
         """
         Load a plugin by ID.

src/plugin_system/store_manager.py

@@ -5,7 +5,6 @@ Handles plugin discovery, installation, updates, and uninstallation
 from both the official registry and custom GitHub repositories.
 """
 
-import hashlib
 import os
 import re
 import json
@@ -26,6 +25,7 @@ import logging
 from urllib.parse import urlparse
 
 from src.common.permission_utils import sudo_remove_directory, install_requirements_file
+from src.plugin_system.plugin_loader import requirements_has_real_deps, requirements_are_satisfied
 
 try:
     from jsonschema import Draft7Validator, ValidationError
@@ -1912,7 +1912,15 @@ class PluginStoreManager:
         if not requirements_file.exists():
             self.logger.debug(f"No requirements.txt found in {plugin_path.name}")
             return True
-        
+
+        if not requirements_has_real_deps(str(requirements_file)):
+            self.logger.debug(f"requirements.txt for {plugin_path.name} has no real dependencies, skipping pip")
+            return True
+
+        if requirements_are_satisfied(str(requirements_file)):
+            self.logger.debug(f"Dependencies for {plugin_path.name} already satisfied, skipping pip")
+            return True
+
         try:
             self.logger.info(f"Installing dependencies for {plugin_path.name}")
             # Routed through the shared root-visible installer (same one the
@@ -1929,12 +1937,6 @@ class PluginStoreManager:
                 )
                 return False
             self.logger.info(f"Dependencies installed successfully for {plugin_path.name}")
-            # Write hash marker so plugin_loader skips redundant pip run on next startup
-            try:
-                current_hash = hashlib.sha256(requirements_file.read_bytes()).hexdigest()
-                (plugin_path / ".dependencies_installed").write_text(current_hash, encoding='utf-8')
-            except OSError as marker_err:
-                self.logger.debug("Could not write dependency marker for %s: %s", plugin_path.name, marker_err)
             return True
 
         except subprocess.TimeoutExpired:
@@ -2432,19 +2434,6 @@ class PluginStoreManager:
                                     file_path = line[3:].strip()
                                     untracked_files.append(file_path)
                         
-                        # Remove marker files that are safe to delete (they'll be regenerated)
-                        safe_to_remove = ['.dependencies_installed']
-                        removed_files = []
-                        for file_name in safe_to_remove:
-                            file_path = plugin_path / file_name
-                            if file_path.exists() and file_name in untracked_files:
-                                try:
-                                    file_path.unlink()
-                                    removed_files.append(file_name)
-                                    self.logger.info(f"Removed marker file {file_name} from {plugin_id} before update")
-                                except Exception as e:
-                                    self.logger.warning(f"Could not remove {file_name} from {plugin_id}: {e}")
-                        
                         # Check for tracked file changes
                         status_result = subprocess.run(
                             ['git', '-C', str(plugin_path), 'status', '--porcelain', '--untracked-files=no'],
@@ -2455,10 +2444,9 @@ class PluginStoreManager:
                         )
                         has_changes = bool(status_result.stdout.strip())
                         
-                        # If there are remaining untracked files (not safe to remove), stash them
-                        remaining_untracked = [f for f in untracked_files if f not in removed_files]
-                        if remaining_untracked:
-                            self.logger.info(f"Found {len(remaining_untracked)} untracked files in {plugin_id}, will stash them")
+                        # If there are untracked files, stash them
+                        if untracked_files:
+                            self.logger.info(f"Found {len(untracked_files)} untracked files in {plugin_id}, will stash them")
                             has_changes = True
                     except subprocess.TimeoutExpired:
                         # If status check times out, assume there might be changes and proceed

test/test_plugin_loader.py

@@ -216,20 +216,23 @@ class TestPluginLoader:
         plugin_dir.mkdir()
         requirements_file = plugin_dir / "requirements.txt"
         requirements_file.write_text("package1==1.0.0\n")
-        
+
         mock_subprocess.return_value = MagicMock(returncode=1)
 
         result = plugin_loader.install_dependencies(plugin_dir, "test_plugin")
 
         assert result is False
 
+    @patch('src.plugin_system.plugin_loader.requirements_are_satisfied', return_value=False)
     @patch('subprocess.run')
     def test_install_dependencies_retries_with_ignore_installed_on_apt_conflict(
-        self, mock_subprocess, plugin_loader, tmp_plugins_dir
+        self, mock_subprocess, mock_satisfied, plugin_loader, tmp_plugins_dir
     ):
         """An apt-managed package with no pip RECORD file triggers a retry with
         --ignore-installed rather than silently assuming the old version satisfies
-        the requirement."""
+        the requirement. requirements_are_satisfied() is mocked False here because
+        this scenario is exactly the case where the installed (apt) version does
+        NOT satisfy the pin — that's why pip attempts a reinstall in the first place."""
         plugin_dir = tmp_plugins_dir / "test_plugin"
         plugin_dir.mkdir()
         requirements_file = plugin_dir / "requirements.txt"
@@ -249,9 +252,10 @@ class TestPluginLoader:
         retry_cmd = mock_subprocess.call_args_list[1][0][0]
         assert "--ignore-installed" in retry_cmd
 
+    @patch('src.plugin_system.plugin_loader.requirements_are_satisfied', return_value=False)
     @patch('subprocess.run')
     def test_install_dependencies_apt_conflict_retry_also_fails(
-        self, mock_subprocess, plugin_loader, tmp_plugins_dir
+        self, mock_subprocess, mock_satisfied, plugin_loader, tmp_plugins_dir
     ):
         """Still tolerates the failure (returns True) if the --ignore-installed
         retry itself fails, matching the prior soft-fallback behavior."""
@@ -272,9 +276,10 @@ class TestPluginLoader:
         assert result is True
         assert mock_subprocess.call_count == 2
 
+    @patch('src.plugin_system.plugin_loader.requirements_are_satisfied', return_value=False)
     @patch('subprocess.run')
     def test_install_dependencies_apt_conflict_retry_times_out(
-        self, mock_subprocess, plugin_loader, tmp_plugins_dir
+        self, mock_subprocess, mock_satisfied, plugin_loader, tmp_plugins_dir
     ):
         """A retry timeout must be tolerated the same way as a retry failure
         (return True), not propagate to the outer TimeoutExpired handler and
@@ -297,3 +302,16 @@ class TestPluginLoader:
 
         assert result is True
         assert mock_subprocess.call_count == 2
+
+    @patch('subprocess.run')
+    def test_install_dependencies_already_satisfied_skips_pip(self, mock_subprocess, plugin_loader, tmp_plugins_dir):
+        """A requirement already satisfied in the current environment shouldn't invoke pip."""
+        plugin_dir = tmp_plugins_dir / "test_plugin"
+        plugin_dir.mkdir()
+        requirements_file = plugin_dir / "requirements.txt"
+        requirements_file.write_text("pytest>=1.0\n")
+
+        result = plugin_loader.install_dependencies(plugin_dir, "test_plugin")
+
+        assert result is True
+        mock_subprocess.assert_not_called()

test/test_plugin_system.py

@@ -84,8 +84,9 @@ class TestPluginLoader:
     
     def test_dependency_check(self):
         """Test dependency checking logic."""
-        # This would test _check_dependencies_installed and _install_plugin_dependencies
-        # which requires mocking subprocess calls and file operations
+        # Covered by test_plugin_loader.py's install_dependencies tests,
+        # which exercise requirements_has_real_deps/requirements_are_satisfied
+        # and the pip subprocess fallback.
 
 
 class TestPluginExecutor: