Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 31 additions & 17 deletions plugins/review-suite/scripts/review_suite_local.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import ctypes
import html
import json
import math
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:"):
Expand All @@ -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:
Expand Down
56 changes: 56 additions & 0 deletions plugins/review-suite/tests/test_review_suite_local.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import json
import os
import subprocess
import sys
from datetime import datetime, timezone
Expand All @@ -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,
Expand Down Expand Up @@ -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"
Expand Down