fix: plugin_loader retries with --ignore-installed on apt/pip RECORD conflicts

src/plugin_system/plugin_loader.py

@@ -235,16 +235,51 @@ def install_dependencies(
                 return True
             else:
                 stderr = result.stderr or ""
-                # uninstall-no-record-file means the package is already present at the
-                # system level (e.g. installed via dnf/apt without a pip RECORD file).
-                # pip can't replace it, but it IS installed — write the marker so we
-                # don't retry on every restart.
+                # uninstall-no-record-file means a system-managed copy of a package
+                # (e.g. apt's python3-requests, which ships no pip RECORD file) is in
+                # the way of the version this requirements.txt pins. Retry with
+                # --ignore-installed so pip lays the pinned version down alongside
+                # 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.
                 if "uninstall-no-record-file" in stderr:
                     self.logger.warning(
-                        "Dependencies for %s include system-managed packages (no pip RECORD). "
-                        "Assuming they are satisfied: %s",
+                        "Dependencies for %s conflict with a system-managed package "
+                        "(no pip RECORD); retrying with --ignore-installed: %s",
                         plugin_id, stderr.strip()
                     )
+                    # Wrapped in its own try/except so a retry timeout is
+                    # tolerated the same way as a retry failure, instead of
+                    # propagating to the outer handler and returning False
+                    # (which would contradict the "assume satisfied" fallback
+                    # below).
+                    try:
+                        # sys.executable is this process's own interpreter (not
+                        # attacker-influenced), and requirements_file is a path
+                        # built internally by find_plugin_directory, never raw
+                        # external input.
+                        retry_result = subprocess.run(  # nosec B603 - no shell invoked (list-form argv)  # nosemgrep
+                            [sys.executable, "-m", "pip", "install", "--break-system-packages",
+                             "--ignore-installed", "-r", requirements_file],
+                            capture_output=True,
+                            text=True,
+                            timeout=timeout,
+                            check=False
+                        )
+                        if retry_result.returncode != 0:
+                            self.logger.warning(
+                                "Retry with --ignore-installed also failed for %s; assuming the "
+                                "system-managed version satisfies the requirement: %s",
+                                plugin_id, (retry_result.stderr or "").strip()
+                            )
+                    except subprocess.TimeoutExpired:
+                        self.logger.warning(
+                            "Retry with --ignore-installed timed out for %s; assuming the "
+                            "system-managed version satisfies the requirement",
+                            plugin_id
+                        )
                     try:
                         with open(marker_file, 'w', encoding='utf-8') as fh:
                             fh.write(current_hash)

test/test_plugin_loader.py

@@ -4,6 +4,8 @@
 Tests plugin directory discovery, module loading, and class instantiation.
 """
 
+import subprocess
+
 import pytest
 from unittest.mock import MagicMock, patch
 from src.plugin_system.plugin_loader import PluginLoader
@@ -216,7 +218,82 @@ def test_install_dependencies_failure(self, mock_subprocess, plugin_loader, tmp_
         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('subprocess.run')
+    def test_install_dependencies_retries_with_ignore_installed_on_apt_conflict(
+        self, mock_subprocess, 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."""
+        plugin_dir = tmp_plugins_dir / "test_plugin"
+        plugin_dir.mkdir()
+        requirements_file = plugin_dir / "requirements.txt"
+        requirements_file.write_text("requests>=2.33.0,<3.0.0\n")
+
+        first_attempt = MagicMock(
+            returncode=1,
+            stderr="ERROR: Cannot uninstall requests 2.32.3\nuninstall-no-record-file"
+        )
+        retry_attempt = MagicMock(returncode=0, stderr="")
+        mock_subprocess.side_effect = [first_attempt, retry_attempt]
+
+        result = plugin_loader.install_dependencies(plugin_dir, "test_plugin")
+
+        assert result is True
+        assert mock_subprocess.call_count == 2
+        retry_cmd = mock_subprocess.call_args_list[1][0][0]
+        assert "--ignore-installed" in retry_cmd
+
+    @patch('subprocess.run')
+    def test_install_dependencies_apt_conflict_retry_also_fails(
+        self, mock_subprocess, plugin_loader, tmp_plugins_dir
+    ):
+        """Still tolerates the failure (returns True) if the --ignore-installed
+        retry itself fails, matching the prior soft-fallback behavior."""
+        plugin_dir = tmp_plugins_dir / "test_plugin"
+        plugin_dir.mkdir()
+        requirements_file = plugin_dir / "requirements.txt"
+        requirements_file.write_text("requests>=2.33.0,<3.0.0\n")
+
+        first_attempt = MagicMock(
+            returncode=1,
+            stderr="ERROR: Cannot uninstall requests 2.32.3\nuninstall-no-record-file"
+        )
+        retry_attempt = MagicMock(returncode=1, stderr="some other pip error")
+        mock_subprocess.side_effect = [first_attempt, retry_attempt]
+
+        result = plugin_loader.install_dependencies(plugin_dir, "test_plugin")
+
+        assert result is True
+        assert mock_subprocess.call_count == 2
+
+    @patch('subprocess.run')
+    def test_install_dependencies_apt_conflict_retry_times_out(
+        self, mock_subprocess, 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
+        return False."""
+        plugin_dir = tmp_plugins_dir / "test_plugin"
+        plugin_dir.mkdir()
+        requirements_file = plugin_dir / "requirements.txt"
+        requirements_file.write_text("requests>=2.33.0,<3.0.0\n")
+
+        first_attempt = MagicMock(
+            returncode=1,
+            stderr="ERROR: Cannot uninstall requests 2.32.3\nuninstall-no-record-file"
+        )
+        mock_subprocess.side_effect = [
+            first_attempt,
+            subprocess.TimeoutExpired(cmd="pip", timeout=300),
+        ]
+
+        result = plugin_loader.install_dependencies(plugin_dir, "test_plugin")
+
+        assert result is True
+        assert mock_subprocess.call_count == 2