Fix extension add --from review-thread issues

Author: Copilot

src/specify_cli/__init__.py

@@ -3604,6 +3604,9 @@ def extension_add(
     if priority < 1:
         console.print("[red]Error:[/red] Priority must be a positive integer (1 or higher)")
         raise typer.Exit(1)
+    if dev and from_url:
+        console.print("[red]Error:[/red] --dev and --from cannot be used together.")
+        raise typer.Exit(1)
 
     manager = ExtensionManager(project_root)
     speckit_version = get_speckit_version()
@@ -3625,7 +3628,6 @@ def extension_add(
 
             elif from_url:
                 # Install from URL (ZIP file)
-                import urllib.request
                 import urllib.error
                 from urllib.parse import urlparse
 
@@ -3643,27 +3645,17 @@ def extension_add(
                 console.print("Only install extensions from sources you trust.\n")
                 console.print(f"Downloading from {from_url}...")
 
-                # Download ZIP to temp location
-                download_dir = project_root / ".specify" / "extensions" / ".cache" / "downloads"
-                download_dir.mkdir(parents=True, exist_ok=True)
-                zip_path = download_dir / f"{extension}-url-download.zip"
-
                 try:
                     from specify_cli.authentication.http import open_url as _open_url
 
                     with _open_url(from_url, timeout=60) as response:
                         zip_data = response.read()
-                    zip_path.write_bytes(zip_data)
 
                     # Install from downloaded ZIP
-                    manifest = manager.install_from_zip(zip_path, speckit_version, priority=priority)
+                    manifest = manager.install_from_zip_bytes(zip_data, speckit_version, priority=priority)
                 except urllib.error.URLError as e:
                     console.print(f"[red]Error:[/red] Failed to download from {from_url}: {e}")
                     raise typer.Exit(1)
-                finally:
-                    # Clean up downloaded ZIP
-                    if zip_path.exists():
-                        zip_path.unlink()
 
             else:
                 # Try bundled extensions first (shipped with spec-kit)

src/specify_cli/extensions.py

@@ -8,6 +8,7 @@
 
 import json
 import hashlib
+import io
 import os
 import tempfile
 import zipfile
@@ -1224,6 +1225,16 @@ def install_from_zip(
             ValidationError: If manifest is invalid or priority is invalid
             CompatibilityError: If extension is incompatible
         """
+        with zip_path.open("rb") as zip_file:
+            return self.install_from_zip_bytes(zip_file.read(), speckit_version, priority=priority)
+
+    def install_from_zip_bytes(
+        self,
+        zip_bytes: bytes,
+        speckit_version: str,
+        priority: int = 10,
+    ) -> ExtensionManifest:
+        """Install extension from ZIP bytes."""
         # Validate priority early
         if priority < 1:
             raise ValidationError("Priority must be a positive integer (1 or higher)")
@@ -1232,7 +1243,7 @@ def install_from_zip(
             temp_path = Path(tmpdir)
 
             # Extract ZIP safely (prevent Zip Slip attack)
-            with zipfile.ZipFile(zip_path, 'r') as zf:
+            with zipfile.ZipFile(io.BytesIO(zip_bytes), 'r') as zf:
                 # Validate all paths first before extracting anything
                 temp_path_resolved = temp_path.resolve()
                 for member in zf.namelist():

tests/test_extensions.py

@@ -3458,6 +3458,87 @@ def test_extensionignore_negation_pattern(self, temp_dir, valid_manifest_data):
 class TestExtensionAddCLI:
     """CLI integration tests for extension add command."""
 
+    def test_add_rejects_dev_and_from_together(self, tmp_path):
+        """extension add should reject mutually exclusive --dev and --from flags."""
+        from typer.testing import CliRunner
+        from unittest.mock import patch
+        from specify_cli import app
+
+        runner = CliRunner()
+        project_dir = tmp_path / "test-project"
+        project_dir.mkdir()
+        (project_dir / ".specify").mkdir()
+
+        extension_dir = tmp_path / "extension"
+        extension_dir.mkdir()
+        (extension_dir / "extension.yml").write_text(
+            "id: test-ext\nname: Test Extension\nversion: 1.0.0\ncommands: []\n",
+            encoding="utf-8",
+        )
+
+        with patch.object(Path, "cwd", return_value=project_dir):
+            result = runner.invoke(
+                app,
+                ["extension", "add", str(extension_dir), "--dev", "--from", "https://example.com/ext.zip"],
+                catch_exceptions=True,
+            )
+
+        assert result.exit_code == 1
+        assert "--dev and --from cannot be used together" in result.output
+
+    def test_add_from_url_installs_from_downloaded_bytes(self, tmp_path):
+        """extension add --from should install from in-memory bytes."""
+        from types import SimpleNamespace
+        from typer.testing import CliRunner
+        from unittest.mock import patch
+        from specify_cli import app
+        from specify_cli.extensions import ExtensionManager
+
+        runner = CliRunner()
+        project_dir = tmp_path / "test-project"
+        project_dir.mkdir()
+        (project_dir / ".specify").mkdir()
+
+        fake_manifest = SimpleNamespace(
+            id="test-ext",
+            name="Test Extension",
+            version="1.0.0",
+            description="desc",
+            warnings=[],
+            commands=[],
+        )
+        zip_payload = b"fake-zip-bytes"
+        install_args = {}
+
+        class _Resp:
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *_args):
+                return False
+
+            def read(self):
+                return zip_payload
+
+        def _install_from_zip_bytes(self_obj, payload, _speckit_version, priority=10):
+            install_args["payload"] = payload
+            install_args["priority"] = priority
+            return fake_manifest
+
+        with patch.object(Path, "cwd", return_value=project_dir), \
+             patch("specify_cli.authentication.http.open_url", return_value=_Resp()), \
+             patch.object(ExtensionManager, "install_from_zip_bytes", _install_from_zip_bytes), \
+             patch.object(ExtensionManager, "install_from_zip", side_effect=AssertionError("legacy path install should not be used")):
+            result = runner.invoke(
+                app,
+                ["extension", "add", "../../evil", "--from", "https://example.com/ext.zip"],
+                catch_exceptions=True,
+            )
+
+        assert result.exit_code == 0, result.output
+        assert install_args["payload"] == zip_payload
+        assert install_args["priority"] == 10
+
     def test_add_by_display_name_uses_resolved_id_for_download(self, tmp_path):
         """extension add by display name should use resolved ID for download_extension()."""
         from typer.testing import CliRunner