From 441bb4b12341e71147d74517424e438fa7c8996b Mon Sep 17 00:00:00 2001 From: ry2009 <134240944+ry2009@users.noreply.github.com> Date: Mon, 24 Nov 2025 10:44:41 -0500 Subject: [PATCH 1/4] Fix mgrep watch for Windows and add CI hooks tests --- .github/workflows/ci.yml | 16 +++++ plugins/mgrep/hooks/mgrep_watch.py | 29 ++++++++- plugins/mgrep/hooks/mgrep_watch_kill.py | 33 +++++++---- plugins/mgrep/hooks/test_mgrep_watch.py | 62 ++++++++++++++++++++ plugins/mgrep/hooks/test_mgrep_watch_kill.py | 56 ++++++++++++++++++ 5 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 plugins/mgrep/hooks/test_mgrep_watch.py create mode 100644 plugins/mgrep/hooks/test_mgrep_watch_kill.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 474f94f..a4be2fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,4 +41,20 @@ 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' diff --git a/plugins/mgrep/hooks/mgrep_watch.py b/plugins/mgrep/hooks/mgrep_watch.py index b8780a3..d0b77c6 100644 --- a/plugins/mgrep/hooks/mgrep_watch.py +++ b/plugins/mgrep/hooks/mgrep_watch.py @@ -2,10 +2,12 @@ import sys import json 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 +31,38 @@ def read_hook_input() -> dict[str, object] | None: return None +def launch_watch(payload: dict[str, object]) -> subprocess.Popen: + 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") + + if os.name == "nt": + return subprocess.Popen( + ["mgrep", "watch"], + stdout=stdout_handle, + stderr=stderr_handle, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP, + ) + + return subprocess.Popen( + ["mgrep", "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" + 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..06660e5 --- /dev/null +++ b/plugins/mgrep/hooks/test_mgrep_watch.py @@ -0,0 +1,62 @@ +import importlib.util +import os +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, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "nt" + 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], ["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, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "posix" + mock_os.setsid = object() + + module.launch_watch({"session_id": "abc"}) + + called_args, called_kwargs = mock_subprocess.Popen.call_args + self.assertEqual(called_args[0], ["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 mock.patch.dict("os.environ", {"MGREP_TMP": "/custom/tmp"}): + module = load_module() + + m_open = mock.mock_open() + with mock.patch("builtins.open", m_open), \ + mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "nt" + mock_subprocess.CREATE_NEW_PROCESS_GROUP = 0x0 + + module.launch_watch({"session_id": "abc"}) + + first_open_path = m_open.call_args_list[0][0][0] + self.assertTrue(str(first_open_path).startswith("/custom/tmp/")) 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()) From 49d786177dc4d9225244ae9041e5199e536b1f71 Mon Sep 17 00:00:00 2001 From: ry2009 <134240944+ry2009@users.noreply.github.com> Date: Mon, 24 Nov 2025 10:48:57 -0500 Subject: [PATCH 2/4] Harden Windows temp handling and tests --- plugins/mgrep/hooks/mgrep_watch.py | 2 ++ plugins/mgrep/hooks/test_mgrep_watch.py | 24 +++++++++++++----------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/plugins/mgrep/hooks/mgrep_watch.py b/plugins/mgrep/hooks/mgrep_watch.py index d0b77c6..b4c20f0 100644 --- a/plugins/mgrep/hooks/mgrep_watch.py +++ b/plugins/mgrep/hooks/mgrep_watch.py @@ -32,6 +32,7 @@ def read_hook_input() -> dict[str, object] | 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") @@ -57,6 +58,7 @@ def launch_watch(payload: dict[str, object]) -> subprocess.Popen: payload = read_hook_input() cwd = payload.get("cwd") + 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}") diff --git a/plugins/mgrep/hooks/test_mgrep_watch.py b/plugins/mgrep/hooks/test_mgrep_watch.py index 06660e5..2224c2e 100644 --- a/plugins/mgrep/hooks/test_mgrep_watch.py +++ b/plugins/mgrep/hooks/test_mgrep_watch.py @@ -1,5 +1,6 @@ import importlib.util import os +import tempfile from pathlib import Path from unittest import TestCase, mock @@ -46,17 +47,18 @@ def test_posix_uses_setsid(self): self.assertNotIn("creationflags", called_kwargs) def test_respects_custom_tmp_dir(self): - with mock.patch.dict("os.environ", {"MGREP_TMP": "/custom/tmp"}): - module = load_module() + 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, "os") as mock_os, \ - mock.patch.object(module, "subprocess") as mock_subprocess: - mock_os.name = "nt" - mock_subprocess.CREATE_NEW_PROCESS_GROUP = 0x0 + m_open = mock.mock_open() + with mock.patch("builtins.open", m_open), \ + mock.patch.object(module, "os") as mock_os, \ + mock.patch.object(module, "subprocess") as mock_subprocess: + mock_os.name = "nt" + mock_subprocess.CREATE_NEW_PROCESS_GROUP = 0x0 - module.launch_watch({"session_id": "abc"}) + module.launch_watch({"session_id": "abc"}) - first_open_path = m_open.call_args_list[0][0][0] - self.assertTrue(str(first_open_path).startswith("/custom/tmp/")) + first_open_path = Path(m_open.call_args_list[0][0][0]) + self.assertTrue(str(first_open_path).startswith(str(Path(tmpdir)))) From 3c102a475a32982556c18b281c5a1a97fe1ea02d Mon Sep 17 00:00:00 2001 From: ry2009 <134240944+ry2009@users.noreply.github.com> Date: Mon, 24 Nov 2025 13:18:55 -0500 Subject: [PATCH 3/4] Add Windows smoke import to hooks CI --- .github/workflows/ci.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a4be2fe..04dc36a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,3 +58,18 @@ jobs: - 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 From 77fa3e16f995cd9f413651ce7d3d87448856bf55 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 24 Nov 2025 23:07:01 -0500 Subject: [PATCH 4/4] fix: complete Windows compatibility for hooks - Change python3 to python in hook.json (python3 doesn't exist on Windows) - Use shutil.which() to find mgrep executable (Windows subprocess doesn't find .cmd files by base name) - Make postbuild script cross-platform (chmod doesn't exist on Windows) - Update tests to mock shutil.which Tested manually on Windows 10: - Hook launches mgrep watch successfully with CREATE_NEW_PROCESS_GROUP - PID file created in %TEMP% - Kill hook terminates process and cleans up PID file - All 5 unit tests pass --- package.json | 2 +- plugins/mgrep/hooks/hook.json | 4 ++-- plugins/mgrep/hooks/mgrep_watch.py | 10 ++++++++-- plugins/mgrep/hooks/test_mgrep_watch.py | 10 ++++++++-- 4 files changed, 19 insertions(+), 7 deletions(-) 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 b4c20f0..f03ec42 100644 --- a/plugins/mgrep/hooks/mgrep_watch.py +++ b/plugins/mgrep/hooks/mgrep_watch.py @@ -1,6 +1,7 @@ import os import sys import json +import shutil import subprocess import tempfile from datetime import datetime @@ -37,16 +38,21 @@ def launch_watch(payload: dict[str, object]) -> subprocess.Popen: 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", "watch"], + [mgrep_path, "watch"], stdout=stdout_handle, stderr=stderr_handle, creationflags=subprocess.CREATE_NEW_PROCESS_GROUP, ) return subprocess.Popen( - ["mgrep", "watch"], + [mgrep_path, "watch"], preexec_fn=os.setsid, stdout=stdout_handle, stderr=stderr_handle, diff --git a/plugins/mgrep/hooks/test_mgrep_watch.py b/plugins/mgrep/hooks/test_mgrep_watch.py index 2224c2e..b2879fd 100644 --- a/plugins/mgrep/hooks/test_mgrep_watch.py +++ b/plugins/mgrep/hooks/test_mgrep_watch.py @@ -19,30 +19,34 @@ 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], ["mgrep", "watch"]) + 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], ["mgrep", "watch"]) + self.assertEqual(called_args[0], ["/usr/bin/mgrep", "watch"]) self.assertEqual(called_kwargs.get("preexec_fn"), mock_os.setsid) self.assertNotIn("creationflags", called_kwargs) @@ -53,9 +57,11 @@ def test_respects_custom_tmp_dir(self): 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"})