tighten plugin catalog edge cases

Author: johnnygreco

packages/data-designer/src/data_designer/cli/repositories/plugin_catalog_repository.py

@@ -32,6 +32,14 @@
 from data_designer.config.utils.io_helpers import load_config_file, save_config_file
 
 
+class _PluginCatalogSourceUnavailableError(PluginCatalogError):
+    pass
+
+
+class _PluginCatalogContentError(PluginCatalogError):
+    pass
+
+
 class PluginCatalogRepository(ConfigRepository[PluginCatalogRegistry]):
     """Repository for plugin catalog aliases and cached catalog payloads."""
 
@@ -139,7 +147,7 @@ def load_catalog(self, alias: str | None = None, *, refresh: bool = False) -> Pl
 
         try:
             payload = self._fetch_catalog_payload(catalog_config.url)
-        except (PluginCatalogError, OSError, ValueError):
+        except _PluginCatalogSourceUnavailableError:
             if not refresh:
                 cached_catalog = self._load_cached_catalog(catalog_config, require_fresh=False)
                 if cached_catalog is not None:
@@ -257,7 +265,7 @@ def _normalize_catalog_url(url: str) -> str:
     segments = [segment for segment in parsed.path.split("/") if segment]
 
     if hostname in {"github.com", "www.github.com"} and len(segments) >= 2:
-        owner, repo = segments[0], segments[1]
+        owner, repo = segments[0], segments[1].removesuffix(".git")
         if len(segments) == 2:
             return f"https://raw.githubusercontent.com/{owner}/{repo}/main/catalog/plugins.json"
         if len(segments) >= 5 and segments[2] == "blob":
@@ -289,21 +297,26 @@ def _catalog_plugins_url_path(catalog_root: str) -> str:
 
 def _fetch_local_catalog(location: str) -> dict[str, object]:
     path = Path(location).expanduser()
-    if not path.exists():
-        raise PluginCatalogError(f"Plugin catalog file not found: {path}")
-    if path.stat().st_size > MAX_PLUGIN_CATALOG_SIZE_BYTES:
-        raise PluginCatalogError(
-            f"Plugin catalog at {path} exceeds maximum size of {MAX_PLUGIN_CATALOG_SIZE_BYTES} bytes"
-        )
+    try:
+        if not path.exists():
+            raise _PluginCatalogSourceUnavailableError(f"Plugin catalog file not found: {path}")
+        if path.stat().st_size > MAX_PLUGIN_CATALOG_SIZE_BYTES:
+            raise _PluginCatalogContentError(
+                f"Plugin catalog at {path} exceeds maximum size of {MAX_PLUGIN_CATALOG_SIZE_BYTES} bytes"
+            )
+    except OSError as e:
+        raise _PluginCatalogSourceUnavailableError(f"Failed to read plugin catalog file {path}: {e}") from e
 
     try:
         with open(path) as f:
             payload = json.load(f)
+    except OSError as e:
+        raise _PluginCatalogSourceUnavailableError(f"Failed to read plugin catalog file {path}: {e}") from e
     except json.JSONDecodeError as e:
-        raise PluginCatalogError(f"Failed to parse plugin catalog JSON at {path}: {e}") from e
+        raise _PluginCatalogContentError(f"Failed to parse plugin catalog JSON at {path}: {e}") from e
 
     if not isinstance(payload, dict):
-        raise PluginCatalogError(f"Plugin catalog at {path} must be a JSON object")
+        raise _PluginCatalogContentError(f"Plugin catalog at {path} must be a JSON object")
     return payload
 
 
@@ -313,27 +326,29 @@ def _fetch_remote_catalog(url: str) -> dict[str, object]:
         with urlopen(request, timeout=10) as response:
             status = getattr(response, "status", 200)
             if isinstance(status, int) and status >= 400:
-                raise PluginCatalogError(f"Failed to fetch plugin catalog {url!r}: HTTP {status}")
+                raise _PluginCatalogSourceUnavailableError(f"Failed to fetch plugin catalog {url!r}: HTTP {status}")
             # Read one byte past the limit so oversized chunked responses are
             # rejected without keeping the full response body in memory.
             content = response.read(MAX_PLUGIN_CATALOG_SIZE_BYTES + 1)
     except HTTPError as e:
-        raise PluginCatalogError(f"Failed to fetch plugin catalog {url!r}: HTTP {e.code}") from e
+        raise _PluginCatalogSourceUnavailableError(f"Failed to fetch plugin catalog {url!r}: HTTP {e.code}") from e
     except URLError as e:
-        raise PluginCatalogError(f"Failed to fetch plugin catalog {url!r}: {e.reason}") from e
+        raise _PluginCatalogSourceUnavailableError(f"Failed to fetch plugin catalog {url!r}: {e.reason}") from e
+    except OSError as e:
+        raise _PluginCatalogSourceUnavailableError(f"Failed to read plugin catalog {url!r}: {e}") from e
 
     if len(content) > MAX_PLUGIN_CATALOG_SIZE_BYTES:
-        raise PluginCatalogError(
+        raise _PluginCatalogContentError(
             f"Plugin catalog at {url!r} exceeds maximum size of {MAX_PLUGIN_CATALOG_SIZE_BYTES} bytes"
         )
 
     try:
         payload = json.loads(content.decode("utf-8"))
     except (UnicodeDecodeError, json.JSONDecodeError) as e:
-        raise PluginCatalogError(f"Failed to parse plugin catalog JSON at {url!r}: {e}") from e
+        raise _PluginCatalogContentError(f"Failed to parse plugin catalog JSON at {url!r}: {e}") from e
 
     if not isinstance(payload, dict):
-        raise PluginCatalogError(f"Plugin catalog at {url!r} must be a JSON object")
+        raise _PluginCatalogContentError(f"Plugin catalog at {url!r} must be a JSON object")
     return payload
 
 

packages/data-designer/tests/cli/repositories/test_plugin_catalog_repository.py

@@ -50,6 +50,14 @@ def test_add_catalog_normalizes_github_repository_url(tmp_path: Path) -> None:
     assert repository.get_catalog("research") == catalog
 
 
+def test_add_catalog_normalizes_github_repository_url_with_git_suffix(tmp_path: Path) -> None:
+    repository = PluginCatalogRepository(tmp_path)
+
+    catalog = repository.add_catalog("research", "https://github.com/acme/dd-plugins.git")
+
+    assert catalog.url == "https://raw.githubusercontent.com/acme/dd-plugins/main/catalog/plugins.json"
+
+
 def test_add_catalog_persists_only_public_catalog_fields(tmp_path: Path) -> None:
     repository = PluginCatalogRepository(tmp_path)
 
@@ -69,6 +77,14 @@ def test_add_catalog_normalizes_github_tree_url_with_subdirectory(tmp_path: Path
     assert catalog.url == "https://raw.githubusercontent.com/acme/dd-plugins/main/custom-catalog/catalog/plugins.json"
 
 
+def test_add_catalog_normalizes_github_tree_url_with_git_suffix(tmp_path: Path) -> None:
+    repository = PluginCatalogRepository(tmp_path)
+
+    catalog = repository.add_catalog("research", "https://github.com/acme/dd-plugins.git/tree/main/custom-catalog")
+
+    assert catalog.url == "https://raw.githubusercontent.com/acme/dd-plugins/main/custom-catalog/catalog/plugins.json"
+
+
 def test_add_catalog_normalizes_github_tree_url_ending_with_catalog(tmp_path: Path) -> None:
     repository = PluginCatalogRepository(tmp_path)
 
@@ -77,6 +93,17 @@ def test_add_catalog_normalizes_github_tree_url_ending_with_catalog(tmp_path: Pa
     assert catalog.url == "https://raw.githubusercontent.com/acme/dd-plugins/main/catalog/plugins.json"
 
 
+def test_add_catalog_normalizes_github_blob_url_with_git_suffix(tmp_path: Path) -> None:
+    repository = PluginCatalogRepository(tmp_path)
+
+    catalog = repository.add_catalog(
+        "research",
+        "https://github.com/acme/dd-plugins.git/blob/main/catalog/plugins.json",
+    )
+
+    assert catalog.url == "https://raw.githubusercontent.com/acme/dd-plugins/main/catalog/plugins.json"
+
+
 def test_catalog_aliases_are_case_insensitive(tmp_path: Path) -> None:
     repository = PluginCatalogRepository(tmp_path)
 
@@ -161,6 +188,18 @@ def test_load_catalog_does_not_fall_back_to_stale_cache_when_fresh_catalog_is_in
         repository.load_catalog("local")
 
 
+def test_load_catalog_does_not_fall_back_to_stale_cache_when_source_json_is_malformed(tmp_path: Path) -> None:
+    catalog_path = _write_catalog(tmp_path, plugin_name="cached-transform")
+    repository = PluginCatalogRepository(tmp_path)
+    repository.add_catalog("local", str(catalog_path), cache_ttl_seconds=0)
+
+    repository.load_catalog("local")
+    catalog_path.write_text("{")
+
+    with pytest.raises(PluginCatalogError, match="Failed to parse plugin catalog JSON"):
+        repository.load_catalog("local")
+
+
 def test_load_catalog_with_zero_cache_ttl_refreshes_source(tmp_path: Path) -> None:
     catalog_path = _write_catalog(tmp_path, plugin_name="text-transform")
     repository = PluginCatalogRepository(tmp_path)