Merge pull request #179 from GiGiDKR/fix-pr#151

fix: PR#151 - Enhance cross-platform support
This commit is contained in:
Beehive Innovations
2025-08-07 23:21:25 -07:00
committed by GitHub
5 changed files with 1741 additions and 501 deletions

View File

@@ -1,4 +1,33 @@
#!/usr/bin/env pwsh
<#
.SYNOPSIS
Code quality checks script for Zen MCP server on Windows.
.DESCRIPTION
This PowerShell script performs code quality checks for the Zen MCP server project:
- Runs static analysis and linting tools on the codebase
- Ensures code style compliance and detects potential issues
- Can be integrated into CI/CD pipelines or used locally before commits
.PARAMETER Help
Displays help information for using the script.
.PARAMETER Verbose
Enables detailed output during code quality checks.
.EXAMPLE
.\code_quality_checks.ps1
Runs all code quality checks on the project.
.\code_quality_checks.ps1 -Verbose
Runs code quality checks with detailed output.
.NOTES
Project Author : BeehiveInnovations
Script Author : GiGiDKR (https://github.com/GiGiDKR)
Date : 07-05-2025
Version : See project documentation
References : https://github.com/BeehiveInnovations/zen-mcp-server
#>
#Requires -Version 5.1
[CmdletBinding()]
param(

View File

@@ -50,12 +50,22 @@ FIXED ISSUES:
add Windows-specific path detection
10. WINDOWS PATH VALIDATION:
- Some path validation logic did not handle Windows/Unix cross-compatibility
- Some path validation logic did not handle Windows/Unix
cross-compatibility
- Solution: Improved path validation to support both Windows and Unix
absolute paths for tests
11. DOCKER PATH VALIDATION:
- Docker paths were not recognized as absolute on Windows
- Solution: Added Docker path validation in file_utils.py
12. DOCKER MODE COMPATIBILITY IN TESTS:
- Conversation tests failed when MCP_FILE_PATH_MODE=docker due to path
conversion
- Solution: Force local mode in tests and reset PathModeDetector cache
MODIFIED FILES:
- utils/file_utils.py : Home patterns + Unix path validation
- utils/file_utils.py : Home patterns + Unix path validation + Docker support
- tests/test_file_protection.py : Cross-platform assertions
- tests/test_utils.py : Safe_files test with temporary file
- run_integration_tests.sh : Windows venv detection
@@ -63,6 +73,7 @@ MODIFIED FILES:
- communication_simulator_test.py : Logger initialization order + Windows paths
- simulator_tests/base_test.py : Logger initialization order + Windows paths
- tools/shared/base_tool.py : Logger initialization order + Windows paths
- tests/test_conversation_file_features.py : Docker mode compatibility
Usage:
python patch_crossplatform.py [--dry-run] [--backup] [--validate-only]
@@ -91,13 +102,14 @@ class CrossPlatformPatcher:
"""Find all files to patch."""
files = {
"file_utils": self.workspace_root / "utils" / "file_utils.py",
"test_file_protection": self.workspace_root / "tests" / "test_file_protection.py",
"test_file_protection": (self.workspace_root / "tests" / "test_file_protection.py"),
"test_utils": self.workspace_root / "tests" / "test_utils.py",
"run_integration_tests_sh": self.workspace_root / "run_integration_tests.sh",
"code_quality_checks_sh": self.workspace_root / "code_quality_checks.sh",
"communication_simulator": self.workspace_root / "communication_simulator_test.py",
"base_test": self.workspace_root / "simulator_tests" / "base_test.py",
"base_tool": self.workspace_root / "tools" / "shared" / "base_tool.py",
"run_integration_tests_sh": (self.workspace_root / "run_integration_tests.sh"),
"code_quality_checks_sh": (self.workspace_root / "code_quality_checks.sh"),
"communication_simulator": (self.workspace_root / "communication_simulator_test.py"),
"base_test": (self.workspace_root / "simulator_tests" / "base_test.py"),
"base_tool": (self.workspace_root / "tools" / "shared" / "base_tool.py"),
"test_conversation_features": (self.workspace_root / "tests" / "test_conversation_file_features.py"),
}
for _, path in files.items():
@@ -177,7 +189,9 @@ class CrossPlatformPatcher:
# Get the part after the pattern
after_pattern = parts[1]
# Check if we're at the user's root (no subdirectories)
if "/" not in after_pattern and "\\\\" not in after_pattern:
if (
"/" not in after_pattern and "\\\\" not in after_pattern
):
logger.warning(
f"Attempted to scan user home directory root: {path}. "
f"Please specify a subdirectory instead."
@@ -230,7 +244,10 @@ class CrossPlatformPatcher:
old_validation = """ # Step 2: Security Policy - Require absolute paths
# Relative paths could be interpreted differently depending on working directory
if not user_path.is_absolute():
raise ValueError(f"Relative paths are not supported. Please provide an absolute path.\\nReceived: {path_str}")"""
raise ValueError(
f"Relative paths are not supported. Please provide an absolute path.\\n"
f"Received: {path_str}"
)"""
new_validation = """ # Step 2: Security Policy - Require absolute paths
# Relative paths could be interpreted differently depending on working directory
@@ -245,7 +262,10 @@ class CrossPlatformPatcher:
is_absolute_path = path_str_normalized.startswith('/')
if not is_absolute_path:
raise ValueError(f"Relative paths are not supported. Please provide an absolute path.\\nReceived: {path_str}")"""
raise ValueError(
f"Relative paths are not supported. Please provide an absolute path.\\n"
f"Received: {path_str}"
)"""
if old_validation in content:
content = content.replace(old_validation, new_validation)
@@ -487,7 +507,10 @@ fi"""
# Configure logging first
log_level = logging.DEBUG if verbose else logging.INFO
logging.basicConfig(level=log_level, format="%(asctime)s - %(levelname)s - %(message)s")
logging.basicConfig(
level=log_level,
format="%(asctime)s - %(levelname)s - %(message)s"
)
self.logger = logging.getLogger(__name__)"""
new_init_order = """ self.verbose = verbose
@@ -695,11 +718,16 @@ fi"""
continue
# Handle both single paths and lists of paths
paths_to_check = field_value if isinstance(field_value, list) else [field_value]
paths_to_check = (
field_value if isinstance(field_value, list) else [field_value]
)
for path in paths_to_check:
if path and not os.path.isabs(path):
return f"All file paths must be FULL absolute paths. Invalid path: '{path}'"
return (
"All file paths must be FULL absolute paths. "
f"Invalid path: '{path}'"
)
return None'''
@@ -793,11 +821,16 @@ fi"""
if os.name == "nt":
# Windows absolute paths should start with drive letter or UNC path
has_drive = (
len(normalized_path) >= 3 and normalized_path[1:3] in (":\\\\", ":/") and normalized_path[0].isalpha()
(
len(normalized_path) >= 3
and normalized_path[1:3] in (":\\\\", ":/")
and normalized_path[0].isalpha()
)
)
has_unc = normalized_path.startswith(("\\\\\\\\", "//"))
# Also accept Unix-style absolute paths (starting with /) for cross-platform compatibility
# Also accept Unix-style absolute paths (starting with /) for
# cross-platform compatibility
has_unix_root = normalized_path.startswith("/")
result = (is_abs_os or is_abs_path) and (has_drive or has_unc or has_unix_root)
@@ -841,6 +874,186 @@ fi"""
return content, False
def patch_docker_path_validation(self, content: str) -> tuple[str, bool]:
"""Patch 14: Add Docker path validation support in file_utils.py."""
# Check if already patched
if "is_docker_path = converted_path_str.startswith" in content:
return content, False
# Add import for path_detector if not present
if "from utils.path_detector import get_path_detector" not in content:
# Find the imports section and add the import
import_section = "import tempfile\nfrom pathlib import Path"
if import_section in content:
new_import = import_section + "\n\nfrom utils.path_detector import get_path_detector"
content = content.replace(import_section, new_import)
# Replace the path validation logic
old_validation = """ # Step 2: Security Policy - Require absolute paths
# Relative paths could be interpreted differently depending on working directory
# Handle cross-platform path format compatibility for testing
is_absolute_path = user_path.is_absolute()
# On Windows, also accept Unix-style absolute paths for cross-platform testing
# This allows paths like "/etc/passwd" to be treated as absolute
import os
if os.name == 'nt' and not is_absolute_path:
path_str_normalized = path_str.replace('\\\\', '/')
is_absolute_path = path_str_normalized.startswith('/')
if not is_absolute_path:
raise ValueError(
"Relative paths are not supported. Please provide an absolute path.\\n"
f"Received: {path_str}"
)"""
new_validation = """ # Step 1.5: Convert path according to current mode (Docker vs local)
path_detector = get_path_detector()
converted_path_str = path_detector.convert_path(path_str)
user_path = Path(converted_path_str)
# Step 2: Security Policy - Require absolute paths
# Relative paths could be interpreted differently depending on working directory
# Handle cross-platform path format compatibility for testing
is_absolute_path = user_path.is_absolute()
# Special handling for Docker mode paths
is_docker_path = converted_path_str.startswith("/app/project/")
is_docker_mode = (
hasattr(path_detector, 'get_path_mode') and
path_detector.get_path_mode() == "docker"
)
# On Windows, also accept Unix-style absolute paths for cross-platform testing
# This allows paths like "/etc/passwd" to be treated as absolute
# Also accept Docker paths when in Docker mode
import os
if os.name == 'nt' and not is_absolute_path:
path_str_normalized = path_str.replace('\\\\', '/')
is_absolute_path = (
path_str_normalized.startswith('/') or
(is_docker_mode and is_docker_path)
)
if not is_absolute_path and not (is_docker_mode and is_docker_path):
raise ValueError(
f"Relative paths are not supported. Please provide an absolute path.\\n"
f"Received: {path_str}"
)"""
if old_validation in content:
content = content.replace(old_validation, new_validation)
return content, True
return content, False
def patch_conversation_test_docker_mode(self, content: str) -> tuple[str, bool]:
"""Patch 15: Fix conversation file features test for Docker mode compatibility."""
# Check if already patched
if '@patch("utils.file_utils.resolve_and_validate_path")' in content and "MCP_FILE_PATH_MODE" in content:
return content, False
modified = False
# 1. Update the test method decorator to include MCP_FILE_PATH_MODE and add mock
old_decorator = """ @patch.dict(
os.environ, {"GEMINI_API_KEY": "test-key", "OPENAI_API_KEY": "", "MCP_FILE_PATH_MODE": "local"}, clear=False
)
def test_build_conversation_history_with_file_content(self, project_path):"""
new_decorator = """ @patch.dict(
os.environ, {"GEMINI_API_KEY": "test-key", "OPENAI_API_KEY": "", "MCP_FILE_PATH_MODE": "local"}, clear=False
)
@patch("utils.file_utils.resolve_and_validate_path")
def test_build_conversation_history_with_file_content(self, mock_resolve_path, project_path):"""
if old_decorator in content:
content = content.replace(old_decorator, new_decorator)
modified = True
# 2. Add the mock setup to return Path objects for the resolve_and_validate_path
old_test_start = ''' """Test that conversation history includes embedded file content"""
from providers.registry import ModelProviderRegistry
ModelProviderRegistry.clear_cache()
# Reset PathModeDetector singleton and cache to ensure local mode
from utils.path_detector import PathModeDetector
PathModeDetector._instance = None
PathModeDetector._cached_mode = None
from utils.path_detector import get_path_detector
detector = get_path_detector()
detector._cached_mode = None
# Force path mode to local and verify
mode = detector.get_path_mode()
assert mode == "local", f"Expected local mode, got {mode}"
# Create test file with known content in a project-like structure'''
new_test_start = ''' """Test that conversation history includes embedded file content"""
from providers.registry import ModelProviderRegistry
ModelProviderRegistry.clear_cache()
# Create test file with known content in a project-like structure'''
if old_test_start in content:
content = content.replace(old_test_start, new_test_start)
modified = True
# 3. Add the mock setup after the test file creation
old_file_creation = ''' project_subdir = os.path.join(project_path, "zen-mcp-server")
os.makedirs(project_subdir, exist_ok=True)
test_file = os.path.join(project_subdir, "test.py")
test_content = "# Test file\\ndef hello():\\n print('Hello, world!')\\n"
with open(test_file, "w") as f:
f.write(test_content)
# Debug output for troubleshooting
print(f"DEBUG: Test file path: {test_file}")
print(f"DEBUG: File exists: {os.path.exists(test_file)}")
print(f"DEBUG: Path mode: {detector.get_path_mode()}")
print(f"DEBUG: Converted path: {detector.convert_path(test_file)}")
# Verify the converted path matches original for local mode
converted_path = detector.convert_path(test_file)
assert converted_path == test_file, f"Path conversion failed: {test_file} -> {converted_path}"'''
new_file_creation = """ # Mock resolve_and_validate_path to return a Path object (simulating local mode)
from pathlib import Path
mock_resolve_path.side_effect = lambda path: Path(path)
project_subdir = os.path.join(project_path, "zen-mcp-server")
os.makedirs(project_subdir, exist_ok=True)
test_file = os.path.join(project_subdir, "test.py")
test_content = "# Test file\\ndef hello():\\n print('Hello, world!')\\n"
with open(test_file, "w") as f:
f.write(test_content)"""
if old_file_creation in content:
content = content.replace(old_file_creation, new_file_creation)
modified = True
# Fix the test file creation to use zen-mcp-server subdirectory
# This is already correct, check for alternate pattern
alt_file_creation = """ # Create a test file
test_file = os.path.join(project_path, "test.py")"""
new_file_creation = """ # Create a test file in zen-mcp-server subdirectory for proper path detection
project_subdir = os.path.join(project_path, "zen-mcp-server")
os.makedirs(project_subdir, exist_ok=True)
test_file = os.path.join(project_subdir, "test.py")"""
if alt_file_creation in content:
content = content.replace(alt_file_creation, new_file_creation)
modified = True
return content, modified
def apply_all_patches(self, files: dict[str, Path], create_backups: bool = False) -> bool:
"""Apply all necessary patches."""
all_success = True
@@ -1011,6 +1224,41 @@ fi"""
else:
print(" tools/shared/base_tool.py already patched")
# Patch 14: Enhanced Docker path validation in utils/file_utils.py
print("\n🔧 Applying Docker path validation patch to utils/file_utils.py...")
file_utils_content = self.read_file(files["file_utils"])
file_utils_content, modified14 = self.patch_docker_path_validation(file_utils_content)
if modified14:
if create_backups:
backup = self.create_backup(files["file_utils"])
print(f" ✅ Backup created: {backup}")
self.write_file(files["file_utils"], file_utils_content)
print(" ✅ Docker path validation support added")
self.patches_applied.append("Docker path validation (file_utils.py)")
else:
print(" Docker path validation already patched in file_utils.py")
# Patch 15: Conversation test Docker mode compatibility
print("\n🔧 Patching tests/test_conversation_file_features.py...")
if "test_conversation_features" in files:
conversation_content = self.read_file(files["test_conversation_features"])
conversation_content, modified15 = self.patch_conversation_test_docker_mode(conversation_content)
if modified15:
if create_backups:
backup = self.create_backup(files["test_conversation_features"])
print(f" ✅ Backup created: {backup}")
self.write_file(files["test_conversation_features"], conversation_content)
print(" ✅ Docker mode compatibility added to conversation tests")
self.patches_applied.append("Docker mode compatibility (test_conversation_file_features.py)")
else:
print(" tests/test_conversation_file_features.py already patched")
return all_success
def validate_patches(self, files: dict[str, Path]) -> list[str]:
@@ -1089,6 +1337,29 @@ fi"""
if "has_unix_root = normalized_path.startswith" not in base_tool_content:
errors.append("Enhanced Windows path validation missing in base_tool.py")
# Validate Docker path validation in utils/file_utils.py
if "from utils.path_detector import get_path_detector" not in file_utils_content:
errors.append("Path detector import missing in file_utils.py")
if "is_docker_path = converted_path_str.startswith" not in file_utils_content:
errors.append("Docker path validation missing in file_utils.py")
if "is_docker_mode = " not in file_utils_content:
errors.append("Docker mode detection missing in file_utils.py")
# Validate conversation test patches
if "test_conversation_features" in files:
conversation_content = self.read_file(files["test_conversation_features"])
if '"MCP_FILE_PATH_MODE": "local"' not in conversation_content:
errors.append("Local mode forcing missing in test_conversation_file_features.py")
if "PathModeDetector._instance = None" not in conversation_content:
errors.append("PathModeDetector reset missing in test_conversation_file_features.py")
if "detector._cached_mode = None" not in conversation_content:
errors.append("PathModeDetector cache reset missing in test_conversation_file_features.py")
return errors
def show_diff_summary(self, files: dict[str, Path]) -> None:
@@ -1149,6 +1420,24 @@ fi"""
"Cross-platform compatibility for Unix-style paths",
],
),
(
"utils/file_utils.py (Docker support)",
[
"Add path_detector import for Docker path conversion",
"Support Docker path validation (/app/project/...)",
"Accept Docker paths as valid when in Docker mode",
"Enhanced cross-platform path handling",
],
),
(
"tests/test_conversation_file_features.py",
[
"Force local mode in conversation tests",
"Reset PathModeDetector singleton and cache",
"Create test files in zen-mcp-server subdirectory",
"Add debug output for troubleshooting",
],
),
]
for filename, changes in modifications:

View File

@@ -12,6 +12,8 @@ work correctly on Windows, including:
5. Communication simulator logger and Python path fixes
6. BaseSimulatorTest logger and Python path fixes
7. Shell scripts Windows virtual environment support
8. Docker path validation and mode compatibility
9. Conversation tests Docker mode compatibility
Tests cover all modified files:
- utils/file_utils.py
@@ -21,8 +23,10 @@ Tests cover all modified files:
- simulator_tests/base_test.py
- run_integration_tests.sh
- code_quality_checks.sh
- tests/test_conversation_file_features.py (Docker mode)
"""
import os
import sys
import tempfile
from pathlib import Path
@@ -376,6 +380,163 @@ def test_shell_scripts_windows_support():
return False
def test_docker_path_validation():
"""Test 8: Docker path validation in file_utils.py."""
print("\n🧪 Test 8: Docker path validation")
print("-" * 60)
try:
# Test that path_detector import is available
try:
from utils.path_detector import PathModeDetector
detector_import = True
print(" ✅ Path detector import: True")
except ImportError as e:
detector_import = False
print(f" ❌ Path detector import: False ({e})")
# Test Docker path validation with mocked Docker mode
try:
# Mock Docker mode
with patch.dict(os.environ, {"MCP_FILE_PATH_MODE": "docker"}):
# Reset singleton to pick up new environment
PathModeDetector._instance = None
# Test Docker path validation
docker_path = "/app/project/test.py"
try:
from utils.file_utils import resolve_and_validate_path
resolve_and_validate_path(docker_path)
docker_validation = True
print(" ✅ Docker path validation: True")
except ValueError as e:
if "Relative paths are not supported" in str(e):
docker_validation = False
print(" ❌ Docker path validation: False (still rejected)")
else:
docker_validation = True # Different error, not path format
print(" ✅ Docker path validation: True (security error)")
except Exception as e:
docker_validation = False
print(f" ❌ Docker path validation: Error ({e})")
# Reset singleton after test
PathModeDetector._instance = None
except Exception as e:
docker_validation = False
print(f" ❌ Docker path test error: {e}")
# Test that file_utils.py contains Docker-related code
try:
with open("utils/file_utils.py", encoding="utf-8") as f:
file_utils_content = f.read()
has_detector_import = "from utils.path_detector import get_path_detector" in file_utils_content
has_docker_check = "is_docker_path = converted_path_str.startswith" in file_utils_content
has_docker_mode = "is_docker_mode" in file_utils_content
print(f" ✅ Has detector import: {has_detector_import}")
print(f" ✅ Has Docker path check: {has_docker_check}")
print(f" ✅ Has Docker mode check: {has_docker_mode}")
file_content_ok = has_detector_import and has_docker_check and has_docker_mode
except FileNotFoundError:
file_content_ok = False
print(" ❌ utils/file_utils.py not found")
success = detector_import and docker_validation and file_content_ok
print(f"\nResult: Docker path validation {'passed' if success else 'failed'}")
return success
except Exception as e:
print(f" ❌ Error testing Docker path validation: {e}")
print("\nResult: Docker path validation failed")
return False
def test_conversation_docker_compatibility():
"""Test 9: Conversation tests Docker mode compatibility."""
print("\n🧪 Test 9: Conversation tests Docker mode compatibility")
print("-" * 60)
try:
# Test that test_conversation_file_features.py contains Docker fixes
try:
with open("tests/test_conversation_file_features.py", encoding="utf-8") as f:
test_content = f.read()
has_local_mode = '"MCP_FILE_PATH_MODE": "local"' in test_content
has_detector_reset = "PathModeDetector._instance = None" in test_content
has_cache_reset = "detector._cached_mode = None" in test_content
has_subdir = "zen-mcp-server" in test_content
print(f" ✅ Forces local mode: {has_local_mode}")
print(f" ✅ Resets PathModeDetector: {has_detector_reset}")
print(f" ✅ Resets detector cache: {has_cache_reset}")
print(f" ✅ Uses project subdirectory: {has_subdir}")
test_content_ok = has_local_mode and has_detector_reset and has_cache_reset
except FileNotFoundError:
test_content_ok = False
print(" ❌ tests/test_conversation_file_features.py not found")
# Test PathModeDetector cache reset functionality
try:
from utils.path_detector import PathModeDetector, get_path_detector
# Test that we can reset the singleton
detector1 = get_path_detector()
detector1.get_path_mode()
# Reset and test again
PathModeDetector._instance = None
detector2 = get_path_detector()
# Test cache reset
detector2._cached_mode = None
detector2.get_path_mode()
reset_works = True
print(" ✅ PathModeDetector reset: True")
except Exception as e:
reset_works = False
print(f" ❌ PathModeDetector reset: False ({e})")
# Test environment patching works
try:
from utils.path_detector import get_path_detector
with patch.dict(os.environ, {"MCP_FILE_PATH_MODE": "local"}):
PathModeDetector._instance = None
detector = get_path_detector()
detector._cached_mode = None
mode = detector.get_path_mode()
env_patch_works = mode == "local"
print(f" ✅ Environment patching: {env_patch_works}")
except Exception as e:
env_patch_works = False
print(f" ❌ Environment patching: False ({e})")
success = test_content_ok and reset_works and env_patch_works
print(f"\nResult: Conversation Docker compatibility {'passed' if success else 'failed'}")
return success
except Exception as e:
print(f" ❌ Error testing conversation Docker compatibility: {e}")
print("\nResult: Conversation Docker compatibility failed")
return False
def main():
"""Main validation function."""
print("🔧 Final validation of cross-platform fixes")
@@ -393,6 +554,8 @@ def main():
results.append(("Communication simulator", test_communication_simulator_fixes()))
results.append(("BaseSimulatorTest", test_base_simulator_test_fixes()))
results.append(("Shell scripts Windows support", test_shell_scripts_windows_support()))
results.append(("Docker path validation", test_docker_path_validation()))
results.append(("Conversation Docker compatibility", test_conversation_docker_compatibility()))
# Final summary
print("\n" + "=" * 70)

File diff suppressed because it is too large Load Diff

View File

@@ -1,4 +1,32 @@
#!/usr/bin/env pwsh
<#
.SYNOPSIS
Integration test runner script for the Zen MCP server on Windows.
.DESCRIPTION
This PowerShell script prepares and runs integration tests for the Zen MCP server:
- Sets up the test environment
- Installs required dependencies
- Runs automated integration tests
- Displays test results and related logs
- Allows output customization via parameters (e.g., display color)
.PARAMETER Color
Sets the display color for console messages (default: White).
.EXAMPLE
.\run_integration_tests.ps1
Prepares the environment and runs all integration tests.
.\run_integration_tests.ps1 -Color Cyan
Runs the tests with messages displayed in cyan.
.NOTES
Project Author : BeehiveInnovations
Script Author : GiGiDKR (https://github.com/GiGiDKR)
Date : 07-05-2025
Version : See config.py (__version__)
References : https://github.com/BeehiveInnovations/zen-mcp-server
#>
#Requires -Version 5.1
[CmdletBinding()]
param(