Merge pull request #1 from vasceannie/codex/extend-test-suite-for-edge-cases

fix: adapt claude command detection for platform variants

hooks/code_quality_guard.py

@@ -10,6 +10,7 @@ import json
 import logging
 import os
 import re
+import shutil
 import subprocess
 import sys
 import tokenize
@@ -415,18 +416,50 @@ def should_skip_file(file_path: str, config: QualityConfig) -> bool:
     return any(pattern in file_path for pattern in config.skip_patterns)
 
 
-def get_claude_quality_command() -> list[str]:
+def _module_candidate(path: Path) -> tuple[Path, list[str]]:
+    """Build a module invocation candidate for a python executable."""
+
+    return path, [str(path), "-m", "quality.cli.main"]
+
+
+def _cli_candidate(path: Path) -> tuple[Path, list[str]]:
+    """Build a direct CLI invocation candidate."""
+
+    return path, [str(path)]
+
+
+def get_claude_quality_command(repo_root: Path | None = None) -> list[str]:
     """Return a path-resilient command for invoking claude-quality."""
-    repo_root = Path(__file__).resolve().parent.parent
-    venv_python = repo_root / ".venv/bin/python"
-    if venv_python.exists():
-        return [str(venv_python), "-m", "quality.cli.main"]
 
-    venv_cli = repo_root / ".venv/bin/claude-quality"
-    if venv_cli.exists():
-        return [str(venv_cli)]
+    repo_root = repo_root or Path(__file__).resolve().parent.parent
+    platform_name = sys.platform
+    is_windows = platform_name.startswith("win")
 
-    return ["claude-quality"]
+    scripts_dir = repo_root / ".venv" / ("Scripts" if is_windows else "bin")
+    python_names = ["python.exe", "python3.exe"] if is_windows else ["python", "python3"]
+    cli_names = ["claude-quality.exe", "claude-quality"] if is_windows else ["claude-quality"]
+
+    candidates: list[tuple[Path, list[str]]] = []
+    for name in python_names:
+        candidates.append(_module_candidate(scripts_dir / name))
+    for name in cli_names:
+        candidates.append(_cli_candidate(scripts_dir / name))
+
+    for candidate_path, command in candidates:
+        if candidate_path.exists():
+            return command
+
+    interpreter_fallbacks = ["python"] if is_windows else ["python3", "python"]
+    for interpreter in interpreter_fallbacks:
+        if shutil.which(interpreter):
+            return [interpreter, "-m", "quality.cli.main"]
+
+    if shutil.which("claude-quality"):
+        return ["claude-quality"]
+
+    raise RuntimeError(
+        "'claude-quality' was not found on PATH. Please ensure it is installed and available."
+    )
 
 
 def _ensure_tool_installed(tool_name: str) -> bool:

tests/hooks/conftest.py

@@ -121,7 +121,6 @@ def calculate_order_total(orders):
 def clean_code() -> str:
     """Sample clean, modern Python code."""
     return """
-from typing import List, Optional, Dict
 from dataclasses import dataclass
 
 
@@ -132,13 +131,13 @@ class User:
     active: bool = True
 
 
-def process_users(users: List[User]) -> Dict[str, int]:
+def process_users(users: list[User]) -> dict[str, int]:
     \"\"\"Process active users and return counts.\"\"\"
     active_count = sum(1 for user in users if user.active)
     return {"active": active_count, "total": len(users)}
 
 
-def find_user(users: List[User], email: str) -> Optional[User]:
+def find_user(users: list[User], email: str) -> User | None:
     \"\"\"Find user by email.\"\"\"
     return next((u for u in users if u.email == email), None)
 """

tests/hooks/test_dynamic_usage.py

@@ -0,0 +1,442 @@
+"""Tests for multi-context hook usage across containers, projects, and users."""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+from types import SimpleNamespace
+from typing import Iterator
+from unittest.mock import patch
+
+import pytest
+
+from code_quality_guard import (
+    QualityConfig,
+    posttooluse_hook,
+    pretooluse_hook,
+)
+
+
+@pytest.fixture()
+def multi_container_paths(tmp_path: Path) -> dict[str, Path]:
+    """Create container/project directories used across tests."""
+    container_a = tmp_path / "container-a" / "project" / "src"
+    container_b = tmp_path / "container-b" / "project" / "src"
+    container_a.mkdir(parents=True)
+    container_b.mkdir(parents=True)
+    return {"a": container_a, "b": container_b}
+
+
+def _pre_request(
+    file_path: Path,
+    content: str,
+    *,
+    container_id: str,
+    project_id: str,
+    user_id: str,
+    platform_name: str = "linux",
+    runtime_name: str = "python3",
+) -> dict[str, object]:
+    """Build a PreToolUse hook payload with rich metadata."""
+    return {
+        "tool_name": "Write",
+        "tool_input": {
+            "file_path": str(file_path),
+            "content": content,
+            "metadata": {
+                "containerId": container_id,
+                "projectId": project_id,
+            },
+        },
+        "metadata": {
+            "user": {"id": user_id, "role": "developer"},
+            "container": {"id": container_id},
+            "project": {"id": project_id},
+            "platform": {"os": platform_name},
+            "runtime": {"name": runtime_name},
+        },
+    }
+
+
+def _post_request(
+    file_path: Path,
+    *,
+    container_id: str,
+    project_id: str,
+    user_id: str,
+    platform_name: str = "linux",
+    runtime_name: str = "python3",
+) -> dict[str, object]:
+    """Build a PostToolUse payload mirroring the metadata structure."""
+    return {
+        "tool_name": "Write",
+        "tool_output": {
+            "file_path": str(file_path),
+            "status": "success",
+            "metadata": {
+                "containerId": container_id,
+                "projectId": project_id,
+            },
+        },
+        "metadata": {
+            "user": {"id": user_id, "role": "developer"},
+            "container": {"id": container_id},
+            "project": {"id": project_id},
+            "platform": {"os": platform_name},
+            "runtime": {"name": runtime_name},
+        },
+    }
+
+
+@pytest.mark.parametrize(
+    ("platform_name", "runtime_name"),
+    [
+        ("linux", "python3"),
+        ("win32", "python"),
+        ("darwin", "python3"),
+    ],
+)
+def test_pretooluse_handles_platform_metadata(
+    tmp_path: Path,
+    platform_name: str,
+    runtime_name: str,
+) -> None:
+    """Ensure platform/runtime metadata does not change allow decisions."""
+    config = QualityConfig()
+    config.skip_patterns = []
+
+    file_path = tmp_path / "project" / f"sample_{platform_name}.py"
+    file_path.parent.mkdir(parents=True, exist_ok=True)
+    content = "def sample() -> None:\n    return None\n"
+
+    with patch("code_quality_guard.analyze_code_quality", return_value={}):
+        response = pretooluse_hook(
+            _pre_request(
+                file_path,
+                content,
+                container_id="platform-container",
+                project_id="platform-project",
+                user_id="platform-user",
+                platform_name=platform_name,
+                runtime_name=runtime_name,
+            ),
+            config,
+        )
+
+    assert response["permissionDecision"] == "allow"
+
+
+def test_state_tracking_isolation_between_containers(multi_container_paths: dict[str, Path]) -> None:
+    """State tracking should stay isolated per container/project combination."""
+    config = QualityConfig(state_tracking_enabled=True)
+    config.skip_patterns = []  # Ensure state tracking runs even in pytest temp dirs.
+
+    base_content = """\
+
+def alpha():
+    return 1
+
+
+def beta():
+    return 2
+""".lstrip()
+
+    container_a = multi_container_paths["a"]
+    container_b = multi_container_paths["b"]
+    file_a = container_a / "service.py"
+    file_b = container_b / "service.py"
+
+    # PreToolUse runs register the pre-state for each container/project pair.
+    with patch("code_quality_guard.analyze_code_quality", return_value={}):
+        response_a_pre = pretooluse_hook(
+            _pre_request(
+                file_a,
+                base_content,
+                container_id="container-a",
+                project_id="project-alpha",
+                user_id="user-deny",
+            ),
+            config,
+        )
+        response_b_pre = pretooluse_hook(
+            _pre_request(
+                file_b,
+                base_content,
+                container_id="container-b",
+                project_id="project-beta",
+                user_id="user-allow",
+            ),
+            config,
+        )
+
+    assert response_a_pre["permissionDecision"] == "allow"
+    assert response_b_pre["permissionDecision"] == "allow"
+
+    # The first container writes fewer functions which should trigger a warning.
+    file_a.write_text("""\
+
+def alpha():
+    return 1
+"""
+    )
+
+    # The second container preserves the original content.
+    file_b.write_text(base_content)
+
+    response_a_post = posttooluse_hook(
+        _post_request(
+            file_a,
+            container_id="container-a",
+            project_id="project-alpha",
+            user_id="user-deny",
+        ),
+        config,
+    )
+    response_b_post = posttooluse_hook(
+        _post_request(
+            file_b,
+            container_id="container-b",
+            project_id="project-beta",
+            user_id="user-allow",
+        ),
+        config,
+    )
+
+    assert response_a_post.get("decision") == "block"
+    assert "Reduced functions" in response_a_post.get("reason", "")
+    # Ensure the second container is unaffected by the first one's regression.
+    assert response_b_post.get("decision") is None
+    assert "Reduced functions" not in response_b_post.get("reason", "")
+
+
+def test_state_tracking_id_collision_different_paths(tmp_path: Path) -> None:
+    """State tracking keys should include the file path when IDs collide."""
+
+    config = QualityConfig(state_tracking_enabled=True)
+    config.skip_patterns = []
+
+    shared_container = "shared-container"
+    shared_project = "shared-project"
+    base_content = """\
+
+def alpha():
+    return 1
+
+
+def beta():
+    return 2
+""".lstrip()
+
+    path_one = tmp_path / "tenant" / "variant-one" / "service.py"
+    path_two = tmp_path / "tenant" / "variant-two" / "service.py"
+    path_one.parent.mkdir(parents=True, exist_ok=True)
+    path_two.parent.mkdir(parents=True, exist_ok=True)
+
+    cmd: list[str],
+    **kwargs: object,
+) -> SimpleNamespace:
+                user_id="collision-user",
+            ),
+            config,
+        )
+        pretooluse_hook(
+            _pre_request(
+                path_two,
+                base_content,
+                container_id=shared_container,
+                project_id=shared_project,
+                user_id="collision-user",
+            ),
+            config,
+        )
+
+    path_one.write_text("""\
+
+def alpha():
+    return 1
+""".lstrip())
+    path_two.write_text(base_content)
+
+    degraded_response = posttooluse_hook(
+        _post_request(
+            path_one,
+            container_id=shared_container,
+            project_id=shared_project,
+            user_id="collision-user",
+        ),
+        config,
+    )
+    preserved_response = posttooluse_hook(
+        _post_request(
+            path_two,
+            container_id=shared_container,
+            project_id=shared_project,
+            user_id="collision-user",
+        ),
+        config,
+    )
+
+    assert degraded_response.get("decision") == "block"
+    assert "Reduced functions" in degraded_response.get("reason", "")
+    assert preserved_response.get("decision") is None
+    assert "Reduced functions" not in preserved_response.get("reason", "")
+
+
+@pytest.mark.parametrize("project_marker", [".git", "pyproject.toml"])
+def test_cross_file_duplicate_project_root_detection(
+    tmp_path: Path,
+    project_marker: str,
+) -> None:
+    """Cross-file duplicate checks should resolve the project root per container."""
+    project_root = tmp_path / "workspace" / "container" / "demo-project"
+    target_dir = project_root / "src" / "package"
+    target_dir.mkdir(parents=True)
+
+    if project_marker == ".git":
+        (project_root / ".git").mkdir()
+    else:
+        (project_root / project_marker).write_text("{}")
+
+    file_path = target_dir / "module.py"
+    file_path.write_text("def thing() -> int:\n    return 1\n")
+
+    config = QualityConfig(cross_file_check_enabled=True)
+
+    captured: dict[str, list[str]] = {}
+
+    def fake_run(
+        cmd: list[str],
+        *,
+        check: bool,
+        capture_output: bool,
+        text: bool,
+        timeout: int,
+    ) -> SimpleNamespace:
+        captured["cmd"] = cmd
+        return SimpleNamespace(returncode=0, stdout=json.dumps({"duplicates": []}))
+
+    with patch("code_quality_guard.subprocess.run", side_effect=fake_run):
+        response = posttooluse_hook(
+            {
+                "tool_name": "Write",
+                "tool_output": {"file_path": str(file_path)},
+                "metadata": {
+                    "container": {"id": "dynamic-container"},
+                    "project": {"id": "demo-project"},
+                },
+            },
+            config,
+        )
+
+    assert "duplicates" in captured["cmd"]
+    dup_index = captured["cmd"].index("duplicates")
+    assert captured["cmd"][dup_index + 1] == str(project_root)
+    assert "--threshold" in captured["cmd"]
+    assert response["hookSpecificOutput"]["hookEventName"] == "PostToolUse"
+    assert response.get("decision") is None
+
+
+def test_main_handles_permission_decisions_for_multiple_users(monkeypatch: pytest.MonkeyPatch) -> None:
+    """`main` should surface deny/ask decisions for different user contexts."""
+    from code_quality_guard import main
+
+    hook_inputs = [
+        {
+            "tool_name": "Write",
+            "tool_input": {
+                "file_path": "tenant-one.py",
+                "content": "print('tenant-one')",
+            },
+            "metadata": {"user": {"id": "user-deny", "role": "viewer"}},
+        },
+        {
+            "tool_name": "Write",
+            "tool_input": {
+                "file_path": "tenant-two.py",
+                "content": "print('tenant-two')",
+            },
+            "metadata": {"user": {"id": "user-ask", "role": "contractor"}},
+        },
+        {
+            "tool_name": "Write",
+            "tool_input": {
+                "file_path": "tenant-three.py",
+                "content": "print('tenant-three')",
+            },
+            "metadata": {"user": {"id": "user-allow", "role": "admin"}},
+        },
+    ]
+
+    responses = iter(
+        [
+            {
+                "hookSpecificOutput": {
+                    "hookEventName": "PreToolUse",
+                    "permissionDecision": "deny",
+                    "permissionDecisionReason": "Tenant user-deny lacks write access",
+                },
+                "permissionDecision": "deny",
+                "reason": "Tenant user-deny lacks write access",
+            },
+            {
+                "hookSpecificOutput": {
+                    "hookEventName": "PreToolUse",
+                    "permissionDecision": "ask",
+                    "permissionDecisionReason": "Tenant user-ask requires approval",
+                },
+                "permissionDecision": "ask",
+                "reason": "Tenant user-ask requires approval",
+            },
+            {
+                "hookSpecificOutput": {
+                    "hookEventName": "PreToolUse",
+                    "permissionDecision": "allow",
+                },
+                "permissionDecision": "allow",
+            },
+        ]
+    )
+
+    input_iter: Iterator[dict[str, object]] = iter(hook_inputs)
+
+    def fake_json_load(_stream: object) -> dict[str, object]:
+        return next(input_iter)
+
+    def fake_pretooluse(_hook_data: dict[str, object], _config: QualityConfig) -> dict[str, object]:
+        return next(responses)
+
+    exit_calls: list[tuple[str, int]] = []
+
+    def fake_exit(reason: str, exit_code: int = 2) -> None:
+        exit_calls.append((reason, exit_code))
+        raise SystemExit(exit_code)
+
+    printed: list[str] = []
+
+    def fake_print(message: str) -> None:  # noqa: D401 - simple passthrough
+        printed.append(message)
+
+    monkeypatch.setattr("json.load", fake_json_load)
+    monkeypatch.setattr("code_quality_guard.pretooluse_hook", fake_pretooluse)
+    monkeypatch.setattr("code_quality_guard._exit_with_reason", fake_exit)
+    monkeypatch.setattr("builtins.print", fake_print)
+
+    # First tenant should produce a deny decision with exit code 2.
+    with pytest.raises(SystemExit) as excinfo_one:
+        main()
+    assert excinfo_one.value.code == 2
+    assert exit_calls[0] == ("Tenant user-deny lacks write access", 2)
+
+    # Second tenant requires approval and should also trigger exit code 2.
+    with pytest.raises(SystemExit) as excinfo_two:
+        main()
+    assert excinfo_two.value.code == 2
+    assert exit_calls[1] == ("Tenant user-ask requires approval", 2)
+
+    # Third tenant is allowed and should simply print the response without exiting.
+    main()
+
+    third_response = json.loads(printed[-1])
+    assert third_response["permissionDecision"] == "allow"
+    assert third_response["hookSpecificOutput"]["hookEventName"] == "PreToolUse"
+    assert len(exit_calls) == 2

tests/hooks/test_helper_functions.py

@@ -2,10 +2,15 @@
 
 import hashlib
 import json
+import shutil
+import sys
 import tempfile
 from datetime import UTC, datetime
+from pathlib import Path
 from unittest.mock import MagicMock, patch
 
+import pytest
+
 from code_quality_guard import (
     QualityConfig,
     analyze_code_quality,
@@ -19,6 +24,15 @@ from code_quality_guard import (
 )
 
 
+@pytest.fixture
+def set_platform(monkeypatch: pytest.MonkeyPatch):
+    """Provide a helper to override sys.platform within a test."""
+
+    def _setter(name: str) -> None:
+        monkeypatch.setattr(sys, "platform", name)
+
+    return _setter
+
 class TestHelperFunctions:
     """Test helper functions in the hook."""
 
@@ -46,26 +60,145 @@ class TestHelperFunctions:
             should_skip_file("test_file.py", config) is False
         )  # Default pattern not included
 
-    def test_get_claude_quality_command_venv(self):
-        """Prefer python module entrypoint when venv python exists."""
-        with patch("pathlib.Path.exists", side_effect=[True]):
-            cmd = get_claude_quality_command()
-        assert cmd[0].endswith(".venv/bin/python")
-        assert cmd[1:] == ["-m", "quality.cli.main"]
+    @staticmethod
+    def _touch(path: Path) -> Path:
+        """Create an empty file representing an executable."""
 
-    def test_get_claude_quality_command_cli_fallback(self):
-        """Fallback to claude-quality script when python missing."""
-        with patch("pathlib.Path.exists", side_effect=[False, True]):
-            cmd = get_claude_quality_command()
-        assert len(cmd) == 1
-        assert cmd[0].endswith(".venv/bin/claude-quality")
+        path.parent.mkdir(parents=True, exist_ok=True)
+        path.write_text("")
+        return path
 
-    def test_get_claude_quality_command_system(self):
-        """Fall back to binary on PATH when venv options absent."""
-        with patch("pathlib.Path.exists", side_effect=[False, False]):
-            cmd = get_claude_quality_command()
+    @pytest.mark.parametrize(
+        ("platform_name", "scripts_dir", "executable_name"),
+        [
+            ("linux", "bin", "python"),
+            ("darwin", "bin", "python"),
+            ("win32", "Scripts", "python.exe"),
+        ],
+    )
+    def test_get_claude_quality_command_prefers_primary_python(
+        self,
+        tmp_path: Path,
+        monkeypatch: pytest.MonkeyPatch,
+        set_platform,
+        platform_name: str,
+        scripts_dir: str,
+        executable_name: str,
+    ) -> None:
+        """Prefer the platform-specific python executable when present in the venv."""
+
+        set_platform(platform_name)
+        monkeypatch.setattr(shutil, "which", lambda _name: None)
+
+        executable = self._touch(tmp_path / ".venv" / scripts_dir / executable_name)
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
+        assert cmd == [str(executable), "-m", "quality.cli.main"]
+
+    def test_get_claude_quality_command_python_and_python3(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, set_platform) -> None:
+        """Prefer python when both python and python3 executables exist in the venv."""
+
+        set_platform("linux")
+        monkeypatch.setattr(shutil, "which", lambda _name: None)
+
+        python_path = self._touch(tmp_path / ".venv" / "bin" / "python")
+        python3_path = self._touch(tmp_path / ".venv" / "bin" / "python3")
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
+        assert cmd == [str(python_path), "-m", "quality.cli.main"]
+        assert python3_path.exists()  # Sanity check that both executables were present
+
+    def test_get_claude_quality_command_cli_fallback(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, set_platform) -> None:
+        """Fallback to claude-quality script when python executables are absent."""
+
+        set_platform("linux")
+        monkeypatch.setattr(shutil, "which", lambda _name: None)
+
+        cli_path = self._touch(tmp_path / ".venv" / "bin" / "claude-quality")
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
+        assert cmd == [str(cli_path)]
+
+    def test_get_claude_quality_command_windows_cli_without_extension(
+        self,
+        tmp_path: Path,
+        monkeypatch: pytest.MonkeyPatch,
+        set_platform,
+    ) -> None:
+        """Handle Windows environments where the claude-quality script lacks an .exe suffix."""
+
+        set_platform("win32")
+        monkeypatch.setattr(shutil, "which", lambda _name: None)
+
+        cli_path = self._touch(tmp_path / ".venv" / "Scripts" / "claude-quality")
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
+        assert cmd == [str(cli_path)]
+
+    def test_get_claude_quality_command_system_python_fallback(
+        self,
+        tmp_path: Path,
+        monkeypatch: pytest.MonkeyPatch,
+        set_platform,
+    ) -> None:
+        """Fall back to python3 on POSIX and python on Windows when no venv executables exist."""
+
+        set_platform("darwin")
+
+        def fake_which(name: str) -> str | None:
+            return "/usr/bin/python3" if name == "python3" else None
+
+        monkeypatch.setattr(shutil, "which", fake_which)
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
+        assert cmd == ["python3", "-m", "quality.cli.main"]
+
+        set_platform("win32")
+
+        def windows_which(name: str) -> str | None:
+            return "C:/Python/python.exe" if name == "python" else None
+
+        monkeypatch.setattr(shutil, "which", windows_which)
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
+        assert cmd == ["python", "-m", "quality.cli.main"]
+
+    def test_get_claude_quality_command_cli_on_path(
+        self,
+        tmp_path: Path,
+        monkeypatch: pytest.MonkeyPatch,
+        set_platform,
+    ) -> None:
+        """Use claude-quality from PATH when no virtualenv interpreters exist."""
+
+        set_platform("linux")
+
+        def fake_which(name: str) -> str | None:
+            if name == "claude-quality":
+                return "/usr/local/bin/claude-quality"
+            return None
+
+        monkeypatch.setattr(shutil, "which", fake_which)
+
+        cmd = get_claude_quality_command(repo_root=tmp_path)
         assert cmd == ["claude-quality"]
 
+    def test_get_claude_quality_command_raises_when_missing(
+        self,
+        tmp_path: Path,
+        monkeypatch: pytest.MonkeyPatch,
+        set_platform,
+    ) -> None:
+        """Raise a clear error when no interpreter or CLI can be located."""
+
+        set_platform("linux")
+        monkeypatch.setattr(shutil, "which", lambda _name: None)
+
+        with pytest.raises(RuntimeError) as excinfo:
+            get_claude_quality_command(repo_root=tmp_path)
+
+        assert "was not found on PATH" in str(excinfo.value)
+
     def test_store_pre_state(self):
         """Test storing pre-modification state."""
         test_content = "def func1(): pass\ndef func2(): pass"