fix: harden shared comparator edges from review

- is_update_available: reject truthy non-string versions (a malformed
  manifest can carry a number; packaging raises TypeError on those) by
  surfacing the mismatch instead of raising.
- store_manager.update_plugin: drop the truthiness gate around the
  comparator so a missing version on either side follows the shared
  'no update' verdict, keeping the store consistent with the UI badge;
  a missing manifest still uses the reinstall recovery path.
- config_manager._load_secrets_for_save: catch only expected read/parse
  failures (OSError/ValueError/RecursionError) so implementation bugs
  propagate as themselves, and log with traceback.

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

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:

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:

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})")

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):