diff --git a/plugins/review-suite/scripts/review_suite_local.py b/plugins/review-suite/scripts/review_suite_local.py index bf3bbcd..54d40ab 100644 --- a/plugins/review-suite/scripts/review_suite_local.py +++ b/plugins/review-suite/scripts/review_suite_local.py @@ -1,5 +1,6 @@ from __future__ import annotations +import ctypes import html import json import math @@ -128,6 +129,22 @@ ("CODEX_THREAD_ID", "codex_thread_id"), ) +_PROCESS_SYNCHRONIZE = 0x00100000 +_WAIT_TIMEOUT = 0x00000102 +_ERROR_ACCESS_DENIED = 5 + +if os.name == "nt": + _kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + _open_process = _kernel32.OpenProcess + _open_process.argtypes = (ctypes.c_ulong, ctypes.c_int, ctypes.c_ulong) + _open_process.restype = ctypes.c_void_p + _wait_for_single_object = _kernel32.WaitForSingleObject + _wait_for_single_object.argtypes = (ctypes.c_void_p, ctypes.c_ulong) + _wait_for_single_object.restype = ctypes.c_ulong + _close_handle = _kernel32.CloseHandle + _close_handle.argtypes = (ctypes.c_void_p,) + _close_handle.restype = ctypes.c_int + @dataclass(frozen=True) class LocalReviewRequest: @@ -3817,26 +3834,23 @@ def launch_round( def _process_is_running(pid: int | None) -> bool: - if not pid: + try: + normalized_pid = int(pid) if pid is not None else 0 + except TypeError, ValueError, OverflowError: + return False + if normalized_pid <= 0 or normalized_pid > 0xFFFFFFFF: return False if os.name == "nt": - proc = subprocess.run( - ["tasklist", "/FI", f"PID eq {int(pid)}", "/FO", "CSV", "/NH"], - capture_output=True, - text=True, - encoding="utf-8", - errors="replace", - check=False, - ) - if proc.returncode != 0: - return False - output = (proc.stdout or "").strip() - if not output or output.startswith("INFO:"): - return False - return f'"{int(pid)}"' in output or f",{int(pid)}," in output + handle = _open_process(_PROCESS_SYNCHRONIZE, False, normalized_pid) + if not handle: + return ctypes.get_last_error() == _ERROR_ACCESS_DENIED + try: + return _wait_for_single_object(handle, 0) == _WAIT_TIMEOUT + finally: + _close_handle(handle) try: with open( - f"/proc/{int(pid)}/status", encoding="utf-8", errors="replace" + f"/proc/{normalized_pid}/status", encoding="utf-8", errors="replace" ) as handle: for line in handle: if not line.startswith("State:"): @@ -3849,7 +3863,7 @@ def _process_is_running(pid: int | None) -> bool: except OSError: pass try: - os.kill(int(pid), 0) + os.kill(normalized_pid, 0) except ProcessLookupError: return False except OSError: diff --git a/plugins/review-suite/tests/test_review_suite_local.py b/plugins/review-suite/tests/test_review_suite_local.py index f7e4068..f7b809f 100644 --- a/plugins/review-suite/tests/test_review_suite_local.py +++ b/plugins/review-suite/tests/test_review_suite_local.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import os import subprocess import sys from datetime import datetime, timezone @@ -24,6 +25,7 @@ _print_live_completed_run, _print_stall_warnings, _print_transport_events, + _process_is_running, _reviewer_wait_line, _reroll_candidate_variants, _running_status_line, @@ -61,6 +63,60 @@ ) +@pytest.mark.skipif(os.name != "nt", reason="Windows-native process check") +def test_process_is_running_uses_no_subprocess( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + review_suite_local.subprocess, + "run", + lambda *_args, **_kwargs: pytest.fail("process check started a subprocess"), + ) + + assert _process_is_running(os.getpid()) is True + + +@pytest.mark.skipif(os.name != "nt", reason="Windows-native process check") +def test_process_is_running_tracks_child_exit() -> None: + child = subprocess.Popen( + [sys.executable, "-c", "import time; time.sleep(30)"], + ) + try: + assert _process_is_running(child.pid) is True + child.terminate() + child.wait(timeout=10) + assert _process_is_running(child.pid) is False + finally: + if child.poll() is None: + child.kill() + child.wait(timeout=10) + + +@pytest.mark.parametrize("pid", [None, 0, -1, "not-a-pid"]) +def test_process_is_running_rejects_invalid_pid(pid: object) -> None: + assert _process_is_running(pid) is False # type: ignore[arg-type] + + +@pytest.mark.skipif(os.name != "nt", reason="Windows-native process check") +def test_process_is_running_handles_missing_and_native_failures( + monkeypatch: pytest.MonkeyPatch, +) -> None: + assert _process_is_running(0xFFFFFFFC) is False + + monkeypatch.setattr(review_suite_local, "_open_process", lambda *_args: None) + monkeypatch.setattr(review_suite_local.ctypes, "get_last_error", lambda: 5) + assert _process_is_running(123) is True + + closed: list[int] = [] + monkeypatch.setattr(review_suite_local, "_open_process", lambda *_args: 123) + monkeypatch.setattr( + review_suite_local, "_wait_for_single_object", lambda *_args: 0xFFFFFFFF + ) + monkeypatch.setattr(review_suite_local, "_close_handle", closed.append) + assert _process_is_running(123) is False + assert closed == [123] + + def test_reviewer_wait_line_uses_actual_count() -> None: assert _reviewer_wait_line({"runs": [{"slot": "alpha"}]}) == ( "[review-suite] waiting for 1 reviewer; wrapper is active as long as output streams, do not stop it prematurely"