diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 474f94f..04dc36a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,4 +41,35 @@ jobs: - name: Build run: pnpm run build + hooks-tests: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Python + uses: actions/setup-python@v5 + with: + python-version: '3.11' + + - name: Run hook unit tests + run: python -m unittest discover -s plugins/mgrep/hooks -p 'test_*.py' + + - name: Windows smoke import + if: matrix.os == 'windows-latest' + shell: bash + run: | + python - <<'PY' +import sys, subprocess, tempfile, pathlib +sys.path.append('plugins/mgrep/hooks') +import mgrep_watch + +assert hasattr(subprocess, 'CREATE_NEW_PROCESS_GROUP'), 'missing CREATE_NEW_PROCESS_GROUP' +print('CREATE_NEW_PROCESS_GROUP present') +print('TMP_DIR', mgrep_watch.TMP_DIR) +assert pathlib.Path(tempfile.gettempdir()).exists(), 'temp dir missing' +PY diff --git a/package.json b/package.json index 61eba6f..84726c5 100644 --- a/package.json +++ b/package.json @@ -16,7 +16,7 @@ }, "scripts": { "build": "tsc", - "postbuild": "chmod +x dist/index.js", + "postbuild": "node -e \"if(process.platform!=='win32')require('child_process').execSync('chmod +x dist/index.js')\"", "dev": "npx tsc && node dist/index.js", "test": "bats test/test.bats", "format": "biome check --write .", diff --git a/plugins/mgrep/hooks/hook.json b/plugins/mgrep/hooks/hook.json index 3c23f89..43ee422 100644 --- a/plugins/mgrep/hooks/hook.json +++ b/plugins/mgrep/hooks/hook.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "python3 ${CLAUDE_PLUGIN_ROOT}/hooks/mgrep_watch.py", + "command": "python ${CLAUDE_PLUGIN_ROOT}/hooks/mgrep_watch.py", "timeout": 10 } ] @@ -17,7 +17,7 @@ "hooks": [ { "type": "command", - "command": "python3 ${CLAUDE_PLUGIN_ROOT}/hooks/mgrep_watch_kill.py", + "command": "python ${CLAUDE_PLUGIN_ROOT}/hooks/mgrep_watch_kill.py", "timeout": 10 } ] diff --git a/plugins/mgrep/hooks/mgrep_watch.py b/plugins/mgrep/hooks/mgrep_watch.py index b8780a3..f03ec42 100644 --- a/plugins/mgrep/hooks/mgrep_watch.py +++ b/plugins/mgrep/hooks/mgrep_watch.py @@ -1,11 +1,14 @@ import os import sys import json +import shutil import subprocess +import tempfile from datetime import datetime from pathlib import Path -DEBUG_LOG_FILE = Path(os.environ.get("MGREP_WATCH_LOG", "/tmp/mgrep-watch.log")) +TMP_DIR = Path(os.environ.get("MGREP_TMP", tempfile.gettempdir())) +DEBUG_LOG_FILE = Path(os.environ.get("MGREP_WATCH_LOG", TMP_DIR / "mgrep-watch.log")) def debug_log(message: str) -> None: @@ -29,17 +32,45 @@ def read_hook_input() -> dict[str, object] | None: return None +def launch_watch(payload: dict[str, object]) -> subprocess.Popen: + TMP_DIR.mkdir(parents=True, exist_ok=True) + log_path = TMP_DIR / f"mgrep-watch-command-{payload.get('session_id')}.log" + stdout_handle = open(log_path, "w") + stderr_handle = open(log_path, "w") + + # Find mgrep executable (handles .cmd on Windows) + mgrep_path = shutil.which("mgrep") + if not mgrep_path: + raise FileNotFoundError("mgrep command not found in PATH") + + if os.name == "nt": + return subprocess.Popen( + [mgrep_path, "watch"], + stdout=stdout_handle, + stderr=stderr_handle, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP, + ) + + return subprocess.Popen( + [mgrep_path, "watch"], + preexec_fn=os.setsid, + stdout=stdout_handle, + stderr=stderr_handle, + ) + + if __name__ == "__main__": payload = read_hook_input() cwd = payload.get("cwd") - pid_file = f"/tmp/mgrep-watch-pid-{payload.get('session_id')}.txt" + TMP_DIR.mkdir(parents=True, exist_ok=True) + pid_file = TMP_DIR / f"mgrep-watch-pid-{payload.get('session_id')}.txt" if os.path.exists(pid_file): debug_log(f"PID file already exists: {pid_file}") sys.exit(1) - process = subprocess.Popen(["mgrep", "watch"], preexec_fn=os.setsid, stdout=open(f"/tmp/mgrep-watch-command-{payload.get('session_id')}.log", "w"), stderr=open(f"/tmp/mgrep-watch-command-{payload.get('session_id')}.log", "w")) + process = launch_watch(payload) debug_log(f"Started mgrep watch process: {process.pid}") debug_log(f"All environment variables: {os.environ}") with open(pid_file, "w") as handle: diff --git a/plugins/mgrep/hooks/mgrep_watch_kill.py b/plugins/mgrep/hooks/mgrep_watch_kill.py index cd7ad50..502b767 100644 --- a/plugins/mgrep/hooks/mgrep_watch_kill.py +++ b/plugins/mgrep/hooks/mgrep_watch_kill.py @@ -2,10 +2,12 @@ import signal import sys import json +import tempfile from datetime import datetime from pathlib import Path -DEBUG_LOG_FILE = Path(os.environ.get("MGREP_WATCH_KILL_LOG", "/tmp/mgrep-watch-kill.log")) +TMP_DIR = Path(os.environ.get("MGREP_TMP", tempfile.gettempdir())) +DEBUG_LOG_FILE = Path(os.environ.get("MGREP_WATCH_KILL_LOG", TMP_DIR / "mgrep-watch-kill.log")) def debug_log(message: str) -> None: @@ -29,19 +31,28 @@ def read_hook_input() -> dict[str, object] | None: return None - -if __name__ == "__main__": - debug_log("Killing mgrep watch process") - payload = read_hook_input() - - pid_file = f"/tmp/mgrep-watch-pid-{payload.get('session_id')}.txt" - if not os.path.exists(pid_file): +def kill_watch(payload: dict[str, object]) -> None: + pid_file = TMP_DIR / f"mgrep-watch-pid-{payload.get('session_id')}.txt" + if not pid_file.exists(): debug_log(f"PID file not found: {pid_file}") sys.exit(1) - pid = int(open(pid_file).read().strip()) + + pid = int(pid_file.read_text().strip()) debug_log(f"Killing mgrep watch process: {pid}") - os.kill(pid, signal.SIGKILL) + + if os.name == "nt": + sig = signal.SIGTERM + else: + sig = signal.SIGKILL + + os.kill(pid, sig) debug_log(f"Killed mgrep watch process: {pid}") - os.remove(pid_file) + pid_file.unlink() debug_log(f"Removed PID file: {pid_file}") + + +if __name__ == "__main__": + debug_log("Killing mgrep watch process") + payload = read_hook_input() + kill_watch(payload) sys.exit(0) diff --git a/plugins/mgrep/hooks/test_mgrep_watch.py b/plugins/mgrep/hooks/test_mgrep_watch.py new file mode 100644 index 0000000..b2879fd --- /dev/null +++ b/plugins/mgrep/hooks/test_mgrep_watch.py @@ -0,0 +1,70 @@ +import importlib.util +import os +import tempfile +from pathlib import Path +from unittest import TestCase, mock + + +MODULE_PATH = Path(__file__).with_name("mgrep_watch.py") + + +def load_module(): + spec = importlib.util.spec_from_file_location("mgrep_watch", MODULE_PATH) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class LaunchWatchTests(TestCase): + def test_windows_uses_creationflags(self): + module = load_module() + with mock.patch("builtins.open", mock.mock_open()), \ + mock.patch.object(module, "shutil") as mock_shutil, \ + mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "nt" + mock_shutil.which.return_value = "/path/to/mgrep" + mock_subprocess.CREATE_NEW_PROCESS_GROUP = 0x00000200 + + module.launch_watch({"session_id": "abc"}) + + called_args, called_kwargs = mock_subprocess.Popen.call_args + self.assertEqual(called_args[0], ["/path/to/mgrep", "watch"]) + self.assertIn("creationflags", called_kwargs) + self.assertNotIn("preexec_fn", called_kwargs) + + def test_posix_uses_setsid(self): + module = load_module() + with mock.patch("builtins.open", mock.mock_open()), \ + mock.patch.object(module, "shutil") as mock_shutil, \ + mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "posix" + mock_os.setsid = object() + mock_shutil.which.return_value = "/usr/bin/mgrep" + + module.launch_watch({"session_id": "abc"}) + + called_args, called_kwargs = mock_subprocess.Popen.call_args + self.assertEqual(called_args[0], ["/usr/bin/mgrep", "watch"]) + self.assertEqual(called_kwargs.get("preexec_fn"), mock_os.setsid) + self.assertNotIn("creationflags", called_kwargs) + + def test_respects_custom_tmp_dir(self): + with tempfile.TemporaryDirectory() as tmpdir: + with mock.patch.dict("os.environ", {"MGREP_TMP": tmpdir}): + module = load_module() + + m_open = mock.mock_open() + with mock.patch("builtins.open", m_open), \ + mock.patch.object(module, "shutil") as mock_shutil, \ + mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "nt" + mock_shutil.which.return_value = "/path/to/mgrep" + mock_subprocess.CREATE_NEW_PROCESS_GROUP = 0x0 + + module.launch_watch({"session_id": "abc"}) + + first_open_path = Path(m_open.call_args_list[0][0][0]) + self.assertTrue(str(first_open_path).startswith(str(Path(tmpdir)))) diff --git a/plugins/mgrep/hooks/test_mgrep_watch_kill.py b/plugins/mgrep/hooks/test_mgrep_watch_kill.py new file mode 100644 index 0000000..9005adb --- /dev/null +++ b/plugins/mgrep/hooks/test_mgrep_watch_kill.py @@ -0,0 +1,56 @@ +import importlib.util +import tempfile +from pathlib import Path +from unittest import TestCase, mock + + +MODULE_PATH = Path(__file__).with_name("mgrep_watch_kill.py") + + +def load_module(): + spec = importlib.util.spec_from_file_location("mgrep_watch_kill", MODULE_PATH) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class KillWatchTests(TestCase): + def test_windows_uses_sigterm_and_tmp_dir(self): + with tempfile.TemporaryDirectory() as tmpdir: + with mock.patch.dict("os.environ", {"MGREP_TMP": tmpdir}): + module = load_module() + + pid_file = Path(tmpdir) / "mgrep-watch-pid-abc.txt" + pid_file.write_text("123") + + with mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "signal") as mock_signal: + mock_os.name = "nt" + mock_os.kill = mock.Mock() + mock_signal.SIGTERM = "TERM" + mock_signal.SIGKILL = "KILL" + + module.kill_watch({"session_id": "abc"}) + + mock_os.kill.assert_called_once_with(123, "TERM") + self.assertFalse(pid_file.exists()) + + def test_posix_uses_sigkill(self): + with tempfile.TemporaryDirectory() as tmpdir: + with mock.patch.dict("os.environ", {"MGREP_TMP": tmpdir}): + module = load_module() + + pid_file = Path(tmpdir) / "mgrep-watch-pid-xyz.txt" + pid_file.write_text("456") + + with mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "signal") as mock_signal: + mock_os.name = "posix" + mock_os.kill = mock.Mock() + mock_signal.SIGTERM = "TERM" + mock_signal.SIGKILL = "KILL" + + module.kill_watch({"session_id": "xyz"}) + + mock_os.kill.assert_called_once_with(456, "KILL") + self.assertFalse(pid_file.exists())