diff --git a/src/config_manager.py b/src/config_manager.py index 695ce119..569b295e 100644 --- a/src/config_manager.py +++ b/src/config_manager.py @@ -301,14 +301,17 @@ class ConfigManager: try: with open(self.secrets_path, 'r') as f_secrets: return json.load(f_secrets) - except Exception as e: + # Only the expected read/parse failures — an unexpected implementation + # error should propagate as itself, not masquerade as a secrets-file + # problem. (JSONDecodeError and UnicodeDecodeError are ValueErrors.) + except (OSError, ValueError, RecursionError) as e: error_msg = ( f"Refusing to save config: secrets file {self.secrets_path} exists " f"but could not be loaded ({e}). Saving without it would write " f"merged secret values into config.json in plaintext. Fix or " f"remove the secrets file, then retry." ) - self.logger.error(error_msg) + self.logger.error("[Config] %s", error_msg, exc_info=True) raise ConfigError(error_msg, config_path=self.secrets_path) from e def save_config(self, new_config_data: Dict[str, Any]) -> None: diff --git a/src/plugin_system/compatibility.py b/src/plugin_system/compatibility.py index f1da5749..9b676cd2 100644 --- a/src/plugin_system/compatibility.py +++ b/src/plugin_system/compatibility.py @@ -198,6 +198,10 @@ def is_update_available(installed_version: str, latest_version: str) -> bool: """ if not installed_version or not latest_version: return False + if not isinstance(installed_version, str) or not isinstance(latest_version, str): + # A malformed manifest/registry can carry a number (1.2) or worse; + # packaging would raise TypeError. Surface the mismatch instead. + return True if installed_version == latest_version: return False try: diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index d610e3c9..8e27ab8f 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -2981,8 +2981,12 @@ class PluginStoreManager: local_version = local_manifest.get('version', '') remote_version = plugin_info_remote.get('latest_version', '') from src.plugin_system.compatibility import is_update_available - if (local_version and remote_version - and not is_update_available(local_version, remote_version)): + # No truthiness gate: the shared comparator already treats + # a missing version on either side as "no update", and the + # store must agree with the UI badge in that case too. A + # missing manifest (not just a missing version field) + # still falls through to the reinstall recovery path. + if not is_update_available(local_version, remote_version): self.logger.info( f"Plugin {plugin_id} already at latest version " f"(installed {local_version}, registry {remote_version})") diff --git a/test/test_version_comparison_consistency.py b/test/test_version_comparison_consistency.py index 0d7816fb..9a27b59c 100644 --- a/test/test_version_comparison_consistency.py +++ b/test/test_version_comparison_consistency.py @@ -40,6 +40,19 @@ CASES = [ ] +class TestSharedComparatorMalformedInputs: + def test_truthy_non_string_surfaces_mismatch(self): + # A malformed manifest can carry version as a number; packaging would + # raise TypeError on it. The comparator must not raise. + assert is_update_available(1.2, "1.2.0") is True + assert is_update_available("1.2.0", 1.3) is True + + def test_falsy_non_string_means_nothing_to_do(self): + assert is_update_available(None, "1.0.0") is False + assert is_update_available("1.0.0", None) is False + assert is_update_available(0, "1.0.0") is False + + class TestSharedComparator: @pytest.mark.parametrize("pair,expected", CASES) def test_is_update_available(self, pair, expected): @@ -114,6 +127,20 @@ class TestStoreManagerUsesSharedComparator: reinstall.assert_called_once() assert result is True + def test_empty_local_version_follows_comparator_no_reinstall(self, tmp_path): + # The comparator says "nothing to do" for a missing version, and the + # store must agree with the UI badge — no reinstall. + store, info = self._store(tmp_path, "", "1.0.0") + result, reinstall = self._run_update(store, info) + assert result is True + reinstall.assert_not_called() + + def test_empty_registry_version_follows_comparator_no_reinstall(self, tmp_path): + store, info = self._store(tmp_path, "1.0.0", "") + result, reinstall = self._run_update(store, info) + assert result is True + reinstall.assert_not_called() + class TestSkinRuntimeMajor: def test_plain_versions(self):