Merge pull request #132 from alex-feel/alex-feel-dev

Use uv run for Python hook execution

Author: alex-feel

scripts/setup_environment.py

@@ -1946,40 +1946,39 @@ def create_additional_settings(
                 hooks_event_list: list[dict[str, Any]] = cast(list[dict[str, Any]], hooks_event_list_raw)
                 hooks_event_list.append(matcher_group)
 
-        # Build the proper command based on OS and file type
-        if command.endswith('.py'):
-            # Python script - need to handle cross-platform execution
-            # Use the absolute path to the downloaded hook file
-            # Strip query parameters from command if present
-            clean_command = command.split('?')[0] if '?' in command else command
-            hook_path = claude_user_dir / 'hooks' / Path(clean_command).name
-            python_cmd: str | None
-
-            if platform.system() == 'Windows':
-                # Windows needs explicit Python interpreter
-                # Use 'py' which is more reliable on Windows, fallback to 'python'
-                python_cmd = 'py' if shutil.which('py') else 'python'
-                # Use forward slashes for the path (works on Windows and avoids JSON escaping issues)
+        # Build the proper command based on file type
+        # Strip query parameters from command if present
+        clean_command = command.split('?')[0] if '?' in command else command
+
+        # Check if this looks like a file reference or a direct command
+        # File references typically don't contain spaces (just the filename)
+        # Direct commands like 'echo "test"' contain spaces
+        is_file_reference = ' ' not in clean_command
+
+        if is_file_reference:
+            # Determine if this is a Python script (case-insensitive check)
+            # Supports both .py and .pyw extensions
+            is_python_script = clean_command.lower().endswith(('.py', '.pyw'))
+
+            if is_python_script:
+                # Python script - use uv run for cross-platform execution
+                # Build absolute path to the hook file in .claude/hooks/
+                hook_path = claude_user_dir / 'hooks' / Path(clean_command).name
+                # Use POSIX-style path (forward slashes) for cross-platform compatibility
+                # This works on Windows, macOS, and Linux, and avoids JSON escaping issues
                 hook_path_str = hook_path.as_posix()
-                full_command = f'{python_cmd} {hook_path_str}'
+                # Use uv run with Python 3.12 - works cross-platform without PATH dependency
+                # uv automatically downloads Python 3.12 if not installed
+                # For .pyw files on Windows, uv automatically uses pythonw
+                full_command = f'uv run --python 3.12 {hook_path_str}'
             else:
-                # Unix-like systems - explicitly use Python 3.12 for hooks
-                # This ensures compatibility with modern Python features (e.g., datetime.UTC)
-                python_cmd = 'python3.12' if shutil.which('python3.12') else None
-
-                if not python_cmd:
-                    error(f'Python 3.12 required for hook "{command}" but not found in PATH')
-                    error('Please install Python 3.12: uv python install 3.12')
-                    # Continue without this hook rather than failing the entire setup
-                    continue
-
-                # Make script executable
-                if hook_path.exists():
-                    hook_path.chmod(0o755)
-
-                full_command = f'{python_cmd} {hook_path}'
+                # Other file - build absolute path and use as-is
+                # System will handle execution based on file extension (.sh, .bat, .cmd, .ps1, etc.)
+                hook_path = claude_user_dir / 'hooks' / Path(clean_command).name
+                hook_path_str = hook_path.as_posix()
+                full_command = hook_path_str
         else:
-            # Not a Python script, use command as-is
+            # Direct command with spaces - use as-is
             full_command = command
 
         # Add hook configuration

tests/test_setup_environment.py

@@ -1197,10 +1197,12 @@ class TestMainFunction:
     @patch('setup_environment.create_additional_settings')
     @patch('setup_environment.create_launcher_script')
     @patch('setup_environment.register_global_command')
+    @patch('setup_environment.is_admin', return_value=True)
     @patch('pathlib.Path.mkdir')
     def test_main_success(
         self,
         mock_mkdir,
+        mock_is_admin,
         mock_register,
         mock_launcher,
         mock_settings,
@@ -1214,11 +1216,14 @@ def test_main_success(
         """Test successful main flow."""
         # Verify mock configuration is available
         assert mock_mkdir is not None
+        assert mock_is_admin.return_value is True
         mock_load.return_value = (
             {
                 'name': 'Test Environment',
                 'command-name': 'test-env',
-                'dependencies': ['npm install -g test'],
+                'dependencies': {
+                    'windows': ['npm install -g test'],
+                },
                 'agents': ['agents/test.md'],
                 'slash-commands': ['commands/test.md'],
                 'mcp-servers': [{'name': 'test'}],
@@ -1252,8 +1257,10 @@ def test_main_no_config(self, mock_load):
 
     @patch('setup_environment.load_config_from_source')
     @patch('setup_environment.install_claude')
-    def test_main_install_failure(self, mock_install, mock_load):
+    @patch('setup_environment.is_admin', return_value=True)
+    def test_main_install_failure(self, mock_is_admin, mock_install, mock_load):
         """Test main with Claude installation failure."""
+        assert mock_is_admin.return_value is True  # Verify admin check is mocked
         mock_load.return_value = ({'name': 'Test'}, 'test.yaml')
         mock_install.return_value = False
 
@@ -1263,8 +1270,10 @@ def test_main_install_failure(self, mock_install, mock_load):
 
     @patch('setup_environment.load_config_from_source')
     @patch('setup_environment.find_command')
-    def test_main_skip_install(self, mock_find, mock_load):
+    @patch('setup_environment.is_admin', return_value=True)
+    def test_main_skip_install(self, mock_is_admin, mock_find, mock_load):
         """Test main with --skip-install flag."""
+        assert mock_is_admin.return_value is True  # Verify admin check is mocked
         mock_load.return_value = (
             {
                 'name': 'Test Environment',

tests/test_setup_environment_additional.py

@@ -506,12 +506,14 @@ def test_install_claude_windows_ssl_error(self, mock_is_admin, mock_run, mock_ur
         assert mock_urlopen.call_count == 2
         mock_is_admin.assert_called()  # Verify is_admin was called
 
+    @patch('setup_environment.is_admin', return_value=True)
     @patch('platform.system', return_value='Windows')
     @patch('setup_environment.urlopen')
     @patch('setup_environment.run_command')
-    def test_install_claude_windows_failure(self, mock_run, mock_urlopen, mock_system):
+    def test_install_claude_windows_failure(self, mock_run, mock_urlopen, mock_system, mock_is_admin):
         """Test Claude installation failure on Windows."""
         assert mock_system.return_value == 'Windows'
+        assert mock_is_admin.return_value is True
         mock_urlopen.return_value = MagicMock(read=lambda: b'# Script')
         mock_run.return_value = subprocess.CompletedProcess([], 1, '', 'Error')
 
@@ -536,10 +538,12 @@ def test_install_claude_linux_success(self, mock_run, _mock_system):
         result = setup_environment.install_claude()
         assert result is True
 
+    @patch('setup_environment.is_admin', return_value=True)
     @patch('platform.system', return_value='Windows')
     @patch('setup_environment.urlopen')
-    def test_install_claude_windows_network_error(self, mock_urlopen, _mock_system):
+    def test_install_claude_windows_network_error(self, mock_urlopen, _mock_system, mock_is_admin):
         """Test Claude installation with network error."""
+        assert mock_is_admin.return_value is True
         mock_urlopen.side_effect = urllib.error.URLError('Network error')
 
         result = setup_environment.install_claude()
@@ -905,9 +909,8 @@ def test_create_additional_settings_hooks_linux(self, _mock_system, mock_downloa
             )
 
             assert result is True
-            # Skip executable check on Windows as chmod doesn't work the same way
-            if sys.platform != 'win32':
-                assert hook_file.stat().st_mode & 0o111  # Check executable
+            # Note: With uv run, executable permissions are no longer needed
+            # The script is executed via: uv run --python 3.12 script.py
 
     def test_create_additional_settings_hooks_invalid(self):
         """Test handling invalid hook configuration."""
@@ -1122,6 +1125,7 @@ def test_main_skip_install_claude_not_found(self, mock_find, mock_load):
     @patch('setup_environment.install_dependencies', return_value=True)
     @patch('setup_environment.process_resources', return_value=True)
     @patch('setup_environment.handle_resource', return_value=True)
+    @patch('setup_environment.is_admin', return_value=True)
     @patch('setup_environment.configure_all_mcp_servers', return_value=True)
     @patch('setup_environment.create_additional_settings', return_value=True)
     @patch('setup_environment.create_launcher_script', return_value=None)
@@ -1132,6 +1136,7 @@ def test_main_no_launcher_created(
         mock_launcher,
         mock_settings,
         mock_mcp,
+        mock_is_admin,
         mock_handle_resource,
         mock_process_resources,
         mock_deps,
@@ -1140,6 +1145,7 @@ def test_main_no_launcher_created(
         mock_load,
     ):
         """Test main when launcher creation fails."""
+        assert mock_is_admin.return_value is True
         del _mock_mkdir  # Unused but required for patch
         del mock_launcher  # Unused but required for patch
         del mock_settings  # Unused but required for patch
@@ -1163,6 +1169,7 @@ def test_main_no_launcher_created(
             setup_environment.main()
             mock_exit.assert_not_called()  # Should continue despite launcher failure
 
+    @patch('setup_environment.is_admin', return_value=True)
     @patch.dict('os.environ', {'CLAUDE_ENV_CONFIG': 'env-config'})
     @patch('setup_environment.load_config_from_source')
     @patch('setup_environment.install_claude', return_value=True)
@@ -1184,8 +1191,10 @@ def test_main_from_env_variable(
         mock_deps,
         mock_install,
         mock_load,
+        mock_is_admin,
     ):
         """Test main using CLAUDE_ENV_CONFIG environment variable."""
+        assert mock_is_admin.return_value is True
         del _mock_mkdir  # Unused but required for patch
         del mock_register  # Unused but required for patch
         del mock_settings  # Unused but required for patch
@@ -1204,6 +1213,7 @@ def test_main_from_env_variable(
             mock_exit.assert_not_called()
             mock_load.assert_called_with('env-config', None)
 
+    @patch('setup_environment.is_admin', return_value=True)
     @patch('setup_environment.load_config_from_source')
     @patch('setup_environment.validate_all_config_files')
     @patch('setup_environment.install_claude', return_value=True)
@@ -1228,8 +1238,10 @@ def test_main_with_all_features(
         mock_install,
         mock_validate,
         mock_load,
+        mock_is_admin,
     ):
         """Test main with all configuration features enabled."""
+        assert mock_is_admin.return_value is True
         del _mock_mkdir  # Unused but required for patch
         del mock_download_resource  # Unused but required for patch
 
@@ -1252,7 +1264,9 @@ def test_main_with_all_features(
                     'API_KEY': 'test123',
                     'DEBUG': 'true',
                 },
-                'dependencies': ['npm install test'],
+                'dependencies': {
+                    'windows': ['npm install test'],
+                },
                 'agents': ['agents/test.md'],
                 'slash-commands': ['commands/test.md'],
                 'output-styles': ['styles/test.md'],