diff --git a/gateway/platforms/whatsapp.py b/gateway/platforms/whatsapp.py index eb0d6f1b5..17bb3ecb4 100644 --- a/gateway/platforms/whatsapp.py +++ b/gateway/platforms/whatsapp.py @@ -19,7 +19,10 @@ import asyncio import json import logging import os +import platform import subprocess + +_IS_WINDOWS = platform.system() == "Windows" from pathlib import Path from typing import Dict, List, Optional, Any @@ -166,7 +169,7 @@ class WhatsAppAdapter(BasePlatformAdapter): ], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) # Wait for bridge to be ready via HTTP health check @@ -211,13 +214,19 @@ class WhatsAppAdapter(BasePlatformAdapter): # Kill the entire process group so child node processes die too import signal try: - os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGTERM) + if _IS_WINDOWS: + self._bridge_process.terminate() + else: + os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): self._bridge_process.terminate() await asyncio.sleep(1) if self._bridge_process.poll() is None: try: - os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGKILL) + if _IS_WINDOWS: + self._bridge_process.kill() + else: + os.killpg(os.getpgid(self._bridge_process.pid), signal.SIGKILL) except (ProcessLookupError, PermissionError): self._bridge_process.kill() except Exception as e: diff --git a/tests/tools/test_windows_compat.py b/tests/tools/test_windows_compat.py new file mode 100644 index 000000000..ec04d2095 --- /dev/null +++ b/tests/tools/test_windows_compat.py @@ -0,0 +1,80 @@ +"""Tests for Windows compatibility of process management code. + +Verifies that os.setsid and os.killpg are never called unconditionally, +and that each module uses a platform guard before invoking POSIX-only functions. +""" + +import ast +import pytest +from pathlib import Path + +# Files that must have Windows-safe process management +GUARDED_FILES = [ + "tools/environments/local.py", + "tools/process_registry.py", + "tools/code_execution_tool.py", + "gateway/platforms/whatsapp.py", +] + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent + + +def _get_preexec_fn_values(filepath: Path) -> list: + """Find all preexec_fn= keyword arguments in Popen calls.""" + source = filepath.read_text(encoding="utf-8") + tree = ast.parse(source, filename=str(filepath)) + values = [] + for node in ast.walk(tree): + if isinstance(node, ast.keyword) and node.arg == "preexec_fn": + values.append(ast.dump(node.value)) + return values + + +class TestNoUnconditionalSetsid: + """preexec_fn must never be a bare os.setsid reference.""" + + @pytest.mark.parametrize("relpath", GUARDED_FILES) + def test_preexec_fn_is_guarded(self, relpath): + filepath = PROJECT_ROOT / relpath + if not filepath.exists(): + pytest.skip(f"{relpath} not found") + values = _get_preexec_fn_values(filepath) + for val in values: + # A bare os.setsid would be: Attribute(value=Name(id='os'), attr='setsid') + assert "attr='setsid'" not in val or "IfExp" in val or "None" in val, ( + f"{relpath} has unconditional preexec_fn=os.setsid" + ) + + +class TestIsWindowsConstant: + """Each guarded file must define _IS_WINDOWS.""" + + @pytest.mark.parametrize("relpath", GUARDED_FILES) + def test_has_is_windows(self, relpath): + filepath = PROJECT_ROOT / relpath + if not filepath.exists(): + pytest.skip(f"{relpath} not found") + source = filepath.read_text(encoding="utf-8") + assert "_IS_WINDOWS" in source, ( + f"{relpath} missing _IS_WINDOWS platform guard" + ) + + +class TestKillpgGuarded: + """os.killpg must always be behind a platform check.""" + + @pytest.mark.parametrize("relpath", GUARDED_FILES) + def test_no_unguarded_killpg(self, relpath): + filepath = PROJECT_ROOT / relpath + if not filepath.exists(): + pytest.skip(f"{relpath} not found") + source = filepath.read_text(encoding="utf-8") + lines = source.splitlines() + for i, line in enumerate(lines): + stripped = line.strip() + if "os.killpg" in stripped or "os.getpgid" in stripped: + # Check that there's an _IS_WINDOWS guard in the surrounding context + context = "\n".join(lines[max(0, i - 15):i + 1]) + assert "_IS_WINDOWS" in context or "else:" in context, ( + f"{relpath}:{i + 1} has unguarded os.killpg/os.getpgid call" + ) diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index aa64c802f..8fb4b4431 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -20,6 +20,7 @@ Platform: Linux / macOS only (Unix domain sockets). Disabled on Windows. import json import logging import os +import platform import signal import socket import subprocess @@ -28,6 +29,8 @@ import tempfile import threading import time import uuid + +_IS_WINDOWS = platform.system() == "Windows" from typing import Any, Dict, List, Optional # Availability gate: UDS requires a POSIX OS @@ -405,7 +408,7 @@ def execute_code( stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.DEVNULL, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) # --- Poll loop: watch for exit, timeout, and interrupt --- @@ -514,7 +517,10 @@ def execute_code( def _kill_process_group(proc, escalate: bool = False): """Kill the child and its entire process group.""" try: - os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + if _IS_WINDOWS: + proc.terminate() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): try: proc.kill() @@ -527,7 +533,10 @@ def _kill_process_group(proc, escalate: bool = False): proc.wait(timeout=5) except subprocess.TimeoutExpired: try: - os.killpg(os.getpgid(proc.pid), signal.SIGKILL) + if _IS_WINDOWS: + proc.kill() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGKILL) except (ProcessLookupError, PermissionError): try: proc.kill() diff --git a/tools/environments/local.py b/tools/environments/local.py index 6d7e8da3c..39586917a 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1,12 +1,15 @@ """Local execution environment with interrupt support and non-blocking I/O.""" import os +import platform import shutil import signal import subprocess import threading import time +_IS_WINDOWS = platform.system() == "Windows" + from tools.environments.base import BaseEnvironment # Noise lines emitted by interactive shells when stdin is not a terminal. @@ -68,7 +71,7 @@ class LocalEnvironment(BaseEnvironment): stdout=subprocess.PIPE, stderr=subprocess.STDOUT, stdin=subprocess.PIPE if stdin_data is not None else subprocess.DEVNULL, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) if stdin_data is not None: @@ -101,12 +104,15 @@ class LocalEnvironment(BaseEnvironment): while proc.poll() is None: if _interrupt_event.is_set(): try: - pgid = os.getpgid(proc.pid) - os.killpg(pgid, signal.SIGTERM) - try: - proc.wait(timeout=1.0) - except subprocess.TimeoutExpired: - os.killpg(pgid, signal.SIGKILL) + if _IS_WINDOWS: + proc.terminate() + else: + pgid = os.getpgid(proc.pid) + os.killpg(pgid, signal.SIGTERM) + try: + proc.wait(timeout=1.0) + except subprocess.TimeoutExpired: + os.killpg(pgid, signal.SIGKILL) except (ProcessLookupError, PermissionError): proc.kill() reader.join(timeout=2) @@ -116,7 +122,10 @@ class LocalEnvironment(BaseEnvironment): } if time.monotonic() > deadline: try: - os.killpg(os.getpgid(proc.pid), signal.SIGTERM) + if _IS_WINDOWS: + proc.terminate() + else: + os.killpg(os.getpgid(proc.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): proc.kill() reader.join(timeout=2) diff --git a/tools/process_registry.py b/tools/process_registry.py index bfdb8cd1d..4f50fe116 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -32,6 +32,7 @@ Usage: import json import logging import os +import platform import shlex import shutil import signal @@ -39,6 +40,8 @@ import subprocess import threading import time import uuid + +_IS_WINDOWS = platform.system() == "Windows" from dataclasses import dataclass, field from pathlib import Path from typing import Any, Dict, List, Optional @@ -192,7 +195,7 @@ class ProcessRegistry: stdout=subprocess.PIPE, stderr=subprocess.STDOUT, stdin=subprocess.PIPE, - preexec_fn=os.setsid, + preexec_fn=None if _IS_WINDOWS else os.setsid, ) session.process = proc @@ -544,7 +547,10 @@ class ProcessRegistry: elif session.process: # Local process -- kill the process group try: - os.killpg(os.getpgid(session.process.pid), signal.SIGTERM) + if _IS_WINDOWS: + session.process.terminate() + else: + os.killpg(os.getpgid(session.process.pid), signal.SIGTERM) except (ProcessLookupError, PermissionError): session.process.kill() elif session.env_ref and session.pid: