Merge branch 'main' of https://github.com/BeehiveInnovations/zen-mcp-server

code_quality_checks.sh

@@ -67,16 +67,16 @@ echo "📋 Step 1: Running Linting and Formatting Checks"
 echo "--------------------------------------------------"
 
 echo "🔧 Running ruff linting with auto-fix..."
-$RUFF check --fix --exclude test_simulation_files
+$RUFF check --fix --exclude test_simulation_files --exclude .zen_venv
 
 echo "🎨 Running black code formatting..."
-$BLACK . --exclude="test_simulation_files/"
+$BLACK . --exclude="test_simulation_files/" --exclude=".zen_venv/"
 
 echo "📦 Running import sorting with isort..."
 $ISORT . --skip-glob=".zen_venv/*" --skip-glob="test_simulation_files/*"
 
 echo "✅ Verifying all linting passes..."
-$RUFF check --exclude test_simulation_files
+$RUFF check --exclude test_simulation_files --exclude .zen_venv
 
 echo "✅ Step 1 Complete: All linting and formatting checks passed!"
 echo ""

conf/__init__.py

@@ -0,0 +1 @@
+"""Configuration data for Zen MCP Server."""

providers/openrouter_registry.py

@@ -1,10 +1,12 @@
 """OpenRouter model registry for managing model configurations and aliases."""
 
+import importlib.resources
 import logging
 import os
 from pathlib import Path
 from typing import Optional
 
+# Import handled via importlib.resources.files() calls directly
 from utils.file_utils import read_json_file
 
 from .base import (
@@ -26,7 +28,8 @@ class OpenRouterModelRegistry:
         self.alias_map: dict[str, str] = {}  # alias -> model_name
         self.model_map: dict[str, ModelCapabilities] = {}  # model_name -> config
 
-        # Determine config path
+        # Determine config path and loading strategy
+        self.use_resources = False
         if config_path:
             # Direct config_path parameter
             self.config_path = Path(config_path)
@@ -37,9 +40,33 @@ class OpenRouterModelRegistry:
                 # Environment variable path
                 self.config_path = Path(env_path)
             else:
-                # Default to conf/custom_models.json - use relative path from this file
-                # This works in development environment
-                self.config_path = Path(__file__).parent.parent / "conf" / "custom_models.json"
+                # Try importlib.resources for robust packaging support
+                self.config_path = None
+                self.use_resources = False
+
+                try:
+                    resource_traversable = importlib.resources.files("conf").joinpath("custom_models.json")
+                    if hasattr(resource_traversable, "read_text"):
+                        self.use_resources = True
+                    else:
+                        raise AttributeError("read_text not available")
+                except Exception:
+                    pass
+
+                if not self.use_resources:
+                    # Fallback to file system paths
+                    potential_paths = [
+                        Path(__file__).parent.parent / "conf" / "custom_models.json",
+                        Path.cwd() / "conf" / "custom_models.json",
+                    ]
+
+                    for path in potential_paths:
+                        if path.exists():
+                            self.config_path = path
+                            break
+
+                    if self.config_path is None:
+                        self.config_path = potential_paths[0]
 
         # Load configuration
         self.reload()
@@ -91,20 +118,44 @@ class OpenRouterModelRegistry:
             self.model_map = {}
 
     def _read_config(self) -> list[ModelCapabilities]:
-        """Read configuration from file.
+        """Read configuration from file or package resources.
 
         Returns:
             List of model configurations
         """
-        if not self.config_path.exists():
-            logging.warning(f"OpenRouter model config not found at {self.config_path}")
-            return []
-
         try:
-            # Use centralized JSON reading utility
-            data = read_json_file(str(self.config_path))
+            if self.use_resources:
+                # Use importlib.resources for packaged environments
+                try:
+                    resource_path = importlib.resources.files("conf").joinpath("custom_models.json")
+                    if hasattr(resource_path, "read_text"):
+                        # Python 3.9+
+                        config_text = resource_path.read_text(encoding="utf-8")
+                    else:
+                        # Python 3.8 fallback
+                        with resource_path.open("r", encoding="utf-8") as f:
+                            config_text = f.read()
+
+                    import json
+
+                    data = json.loads(config_text)
+                    logging.debug("Loaded OpenRouter config from package resources")
+                except Exception as e:
+                    logging.warning(f"Failed to load config from resources: {e}")
+                    return []
+            else:
+                # Use file path loading
+                if not self.config_path.exists():
+                    logging.warning(f"OpenRouter model config not found at {self.config_path}")
+                    return []
+
+                # Use centralized JSON reading utility
+                data = read_json_file(str(self.config_path))
+                logging.debug(f"Loaded OpenRouter config from file: {self.config_path}")
+
             if data is None:
-                raise ValueError(f"Could not read or parse JSON from {self.config_path}")
+                location = "resources" if self.use_resources else str(self.config_path)
+                raise ValueError(f"Could not read or parse JSON from {location}")
 
             # Parse models
             configs = []
@@ -137,7 +188,8 @@ class OpenRouterModelRegistry:
             # Re-raise ValueError for specific config errors
             raise
         except Exception as e:
-            raise ValueError(f"Error reading config from {self.config_path}: {e}")
+            location = "resources" if self.use_resources else str(self.config_path)
+            raise ValueError(f"Error reading config from {location}: {e}")
 
     def _build_maps(self, configs: list[ModelCapabilities]) -> None:
         """Build alias and model maps from configurations.

pyproject.toml

@@ -12,7 +12,7 @@ dependencies = [
 ]
 
 [tool.setuptools.packages.find]
-include = ["tools*", "providers*", "systemprompts*", "utils*"]
+include = ["tools*", "providers*", "systemprompts*", "utils*", "conf*"]
 
 [tool.setuptools]
 py-modules = ["server", "config"]
@@ -39,6 +39,7 @@ extend-exclude = '''
   | \.mypy_cache
   | \.tox
   | \.venv
+  | \.zen_venv
   | venv
   | _build
   | buck-out

requirements.txt

@@ -3,6 +3,7 @@ google-genai>=1.19.0
 openai>=1.55.2  # Minimum version for httpx 0.28.0 compatibility
 pydantic>=2.0.0
 python-dotenv>=1.0.0
+importlib-resources>=5.0.0; python_version<"3.9"
 
 # Development dependencies (install with pip install -r requirements-dev.txt)
 # pytest>=7.4.0

tests/test_pip_detection_fix.py

@@ -0,0 +1,104 @@
+"""Tests for pip detection fix in run-server.sh script.
+
+This test file ensures our pip detection improvements work correctly
+and don't break existing functionality.
+"""
+
+import subprocess
+import tempfile
+from pathlib import Path
+
+import pytest
+
+
+class TestPipDetectionFix:
+    """Test cases for issue #188: PIP is available but not recognized."""
+
+    def test_run_server_script_syntax_valid(self):
+        """Test that run-server.sh has valid bash syntax."""
+        result = subprocess.run(["bash", "-n", "./run-server.sh"], capture_output=True, text=True)
+        assert result.returncode == 0, f"Syntax error in run-server.sh: {result.stderr}"
+
+    def test_run_server_has_proper_shebang(self):
+        """Test that run-server.sh starts with proper shebang."""
+        content = Path("./run-server.sh").read_text()
+        assert content.startswith("#!/bin/bash"), "Script missing proper bash shebang"
+
+    def test_critical_functions_exist(self):
+        """Test that all critical functions are defined in the script."""
+        content = Path("./run-server.sh").read_text()
+        critical_functions = ["find_python", "setup_environment", "setup_venv", "install_dependencies", "bootstrap_pip"]
+
+        for func in critical_functions:
+            assert f"{func}()" in content, f"Critical function {func}() not found in script"
+
+    def test_pip_detection_consistency_issue(self):
+        """Test the specific issue: pip works in setup_venv but fails in install_dependencies.
+
+        This test verifies that our fix ensures consistent Python executable paths.
+        """
+        # Test that the get_venv_python_path function now returns absolute paths
+        content = Path("./run-server.sh").read_text()
+
+        # Check that get_venv_python_path includes our absolute path conversion logic
+        assert "abs_venv_path" in content, "get_venv_python_path should use absolute paths"
+        assert 'cd "$(dirname' in content, "Should convert to absolute path"
+
+        # Test successful completion - our fix should make the script more robust
+        result = subprocess.run(["bash", "-n", "./run-server.sh"], capture_output=True, text=True)
+        assert result.returncode == 0, "Script should have valid syntax after our fix"
+
+    def test_pip_detection_with_non_interactive_shell(self):
+        """Test pip detection works in non-interactive shell environments.
+
+        This addresses the contributor's suggestion about non-interactive shells
+        not sourcing ~/.bashrc where pip PATH might be defined.
+        """
+        # Test case for Git Bash on Windows and non-interactive Linux shells
+        with tempfile.TemporaryDirectory() as temp_dir:
+            # Create mock virtual environment structure
+            venv_path = Path(temp_dir) / ".zen_venv"
+            bin_path = venv_path / "bin"
+            bin_path.mkdir(parents=True)
+
+            # Create mock python executable
+            python_exe = bin_path / "python"
+            python_exe.write_text("#!/bin/bash\necho 'Python 3.12.3'\n")
+            python_exe.chmod(0o755)
+
+            # Create mock pip executable
+            pip_exe = bin_path / "pip"
+            pip_exe.write_text("#!/bin/bash\necho 'pip 23.0.1'\n")
+            pip_exe.chmod(0o755)
+
+            # Test that we can detect pip using explicit paths (not PATH)
+            assert python_exe.exists(), "Mock python executable should exist"
+            assert pip_exe.exists(), "Mock pip executable should exist"
+            assert python_exe.is_file(), "Python should be a file"
+            assert pip_exe.is_file(), "Pip should be a file"
+
+    def test_enhanced_diagnostic_messages_included(self):
+        """Test that our enhanced diagnostic messages are included in the script.
+
+        Verify that the script contains the enhanced error diagnostics we added.
+        """
+        content = Path("./run-server.sh").read_text()
+
+        # Check that enhanced diagnostic information is present in the script
+        expected_diagnostic_patterns = [
+            "Enhanced diagnostic information for debugging",
+            "Diagnostic information:",
+            "Python executable:",
+            "Python executable exists:",
+            "Python executable permissions:",
+            "Virtual environment path:",
+            "Virtual environment exists:",
+            "Final diagnostic information:",
+        ]
+
+        for pattern in expected_diagnostic_patterns:
+            assert pattern in content, f"Enhanced diagnostic pattern '{pattern}' should be in script"
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])

tests/test_uvx_resource_packaging.py

@@ -0,0 +1,103 @@
+"""Tests for uvx path resolution functionality."""
+
+from pathlib import Path
+from unittest.mock import patch
+
+from providers.openrouter_registry import OpenRouterModelRegistry
+
+
+class TestUvxPathResolution:
+    """Test uvx path resolution for OpenRouter model registry."""
+
+    def test_normal_operation(self):
+        """Test that normal operation works in development environment."""
+        registry = OpenRouterModelRegistry()
+        assert len(registry.list_models()) > 0
+        assert len(registry.list_aliases()) > 0
+
+    def test_config_path_resolution(self):
+        """Test that the config path resolution finds the config file in multiple locations."""
+        # Check that the config file exists in the development location
+        config_file = Path(__file__).parent.parent / "conf" / "custom_models.json"
+        assert config_file.exists(), "Config file should exist in conf/custom_models.json"
+
+        # Test that a registry can find and use the config
+        registry = OpenRouterModelRegistry()
+
+        # When using resources, config_path is None; when using file system, it should exist
+        if registry.use_resources:
+            assert registry.config_path is None, "When using resources, config_path should be None"
+        else:
+            assert registry.config_path.exists(), "When using file system, config path should exist"
+
+        assert len(registry.list_models()) > 0, "Registry should load models from config"
+
+    def test_explicit_config_path_override(self):
+        """Test that explicit config path works correctly."""
+        config_path = Path(__file__).parent.parent / "conf" / "custom_models.json"
+
+        registry = OpenRouterModelRegistry(config_path=str(config_path))
+
+        # Should use the provided file path
+        assert registry.config_path == config_path
+        assert len(registry.list_models()) > 0
+
+    def test_environment_variable_override(self):
+        """Test that CUSTOM_MODELS_CONFIG_PATH environment variable works."""
+        config_path = Path(__file__).parent.parent / "conf" / "custom_models.json"
+
+        with patch.dict("os.environ", {"CUSTOM_MODELS_CONFIG_PATH": str(config_path)}):
+            registry = OpenRouterModelRegistry()
+
+            # Should use environment path
+            assert registry.config_path == config_path
+            assert len(registry.list_models()) > 0
+
+    @patch("providers.openrouter_registry.importlib.resources.files")
+    @patch("pathlib.Path.exists")
+    def test_multiple_path_fallback(self, mock_exists, mock_files):
+        """Test that multiple path resolution works for different deployment scenarios."""
+        # Make resources loading fail to trigger file system fallback
+        mock_files.side_effect = Exception("Resource loading failed")
+
+        # Simulate dev path failing, and working directory path succeeding
+        # The third `True` is for the check within `reload()`
+        mock_exists.side_effect = [False, True, True]
+
+        registry = OpenRouterModelRegistry()
+
+        # Should have fallen back to file system mode
+        assert not registry.use_resources, "Should fall back to file system when resources fail"
+
+        # Assert that the registry fell back to the second potential path
+        assert registry.config_path == Path.cwd() / "conf" / "custom_models.json"
+
+        # Should load models successfully
+        assert len(registry.list_models()) > 0
+
+    def test_missing_config_handling(self):
+        """Test behavior when config file is missing."""
+        # Use a non-existent path
+        registry = OpenRouterModelRegistry(config_path="/nonexistent/path/config.json")
+
+        # Should gracefully handle missing config
+        assert len(registry.list_models()) == 0
+        assert len(registry.list_aliases()) == 0
+
+    def test_resource_loading_success(self):
+        """Test successful resource loading via importlib.resources."""
+        # Just test that the registry works normally in our environment
+        # This validates the resource loading mechanism indirectly
+        registry = OpenRouterModelRegistry()
+
+        # Should load successfully using either resources or file system fallback
+        assert len(registry.list_models()) > 0
+        assert len(registry.list_aliases()) > 0
+
+    def test_use_resources_attribute(self):
+        """Test that the use_resources attribute is properly set."""
+        registry = OpenRouterModelRegistry()
+
+        # Should have the use_resources attribute
+        assert hasattr(registry, "use_resources")
+        assert isinstance(registry.use_resources, bool)