diff --git a/artemis/clients/ui_automator_client.py b/artemis/clients/ui_automator_client.py index d86d8b4..09b2c7c 100644 --- a/artemis/clients/ui_automator_client.py +++ b/artemis/clients/ui_automator_client.py @@ -174,6 +174,7 @@ def _is_package_installed(device_id: str, pkg: str) -> bool: try: result = subprocess.run( ["adb", "-s", device_id, "shell", "pm", "list", "packages"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=10, @@ -208,6 +209,7 @@ def _uninstall_package(device_id: str, pkg: str) -> bool: "0", pkg, ], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=30, @@ -379,6 +381,7 @@ class UIAutomatorClient: try: result = subprocess.run( ["adb", "-s", self._device_id, "exec-out", "screencap", "-p"], + stdin=subprocess.DEVNULL, capture_output=True, timeout=10, check=True, diff --git a/artemis/interfaces/cli/commands/mcp.py b/artemis/interfaces/cli/commands/mcp.py index 89872fb..f26b374 100644 --- a/artemis/interfaces/cli/commands/mcp.py +++ b/artemis/interfaces/cli/commands/mcp.py @@ -19,6 +19,7 @@ import os from pathlib import Path import re import sys +import threading import tomllib from typing import Annotated @@ -724,7 +725,9 @@ def mcp_command( console.print(syntax) raise typer.Exit(0) - start_awake_service() + threading.Thread( + target=start_awake_service, daemon=True, name="artemis-awake-init" + ).start() try: st = server_type.lower() if st in ("agent", "mobile", "artemis", "default"): diff --git a/artemis/runtime/awake_lease.py b/artemis/runtime/awake_lease.py index d1e3180..f7312e1 100644 --- a/artemis/runtime/awake_lease.py +++ b/artemis/runtime/awake_lease.py @@ -153,6 +153,7 @@ class ScreenAwakeLease: try: result = subprocess.run( ["adb", "-s", self.device_id, *args], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=10, diff --git a/artemis/runtime/awake_service.py b/artemis/runtime/awake_service.py index a9cd1f5..0d87149 100644 --- a/artemis/runtime/awake_service.py +++ b/artemis/runtime/awake_service.py @@ -48,6 +48,7 @@ def _run_awake_adb_command( try: result = subprocess.run( ["adb", "-s", device_id, *args], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=10, @@ -185,21 +186,25 @@ class ScreenAwakeService: self._heartbeat_threads: dict[str, threading.Thread] = {} self._monitor_stop: threading.Event | None = None self._monitor_thread: threading.Thread | None = None + self._shutdown_requested = False def start(self, device_ids: Iterable[str] | None = None) -> dict[str, str | None]: + if self._shutdown_requested: + return {} selected = list(device_ids) if device_ids is not None else _discover_connected_device_ids() - results = { - device_id: self.ensure_device(device_id) - for device_id in dict.fromkeys(selected) - if device_id - } - if _awake_enabled() and os.environ.get("ARTEMIS_CLOUD_MODE") != "1": + results = {} + for device_id in dict.fromkeys(selected): + if self._shutdown_requested: + break + if device_id: + results[device_id] = self.ensure_device(device_id) + if _awake_enabled() and os.environ.get("ARTEMIS_CLOUD_MODE") != "1" and not self._shutdown_requested: self._start_device_monitor() return results def ensure_device(self, device_id: str) -> str | None: """Configure one device once per process; Android owns unplug behavior.""" - if not device_id or not _awake_enabled() or os.environ.get("ARTEMIS_CLOUD_MODE") == "1": + if not device_id or not _awake_enabled() or os.environ.get("ARTEMIS_CLOUD_MODE") == "1" or self._shutdown_requested: return None with self._lock: existing = self._strategies.get(device_id) @@ -277,6 +282,7 @@ class ScreenAwakeService: def shutdown(self) -> None: """Stop host work; unplug behavior remains owned by Android's USB policy.""" + self._shutdown_requested = True with self._lock: monitor_stop = self._monitor_stop monitor_thread = self._monitor_thread diff --git a/artemis/runtime/device_pool.py b/artemis/runtime/device_pool.py index e14aa41..8efca00 100644 --- a/artemis/runtime/device_pool.py +++ b/artemis/runtime/device_pool.py @@ -81,6 +81,7 @@ class DevicePool: try: res = subprocess.run( [adb, "devices", "-l"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=timeout, diff --git a/artemis/runtime/server_lifecycle.py b/artemis/runtime/server_lifecycle.py index e22bd21..dc72649 100644 --- a/artemis/runtime/server_lifecycle.py +++ b/artemis/runtime/server_lifecycle.py @@ -135,6 +135,7 @@ def find_server_pids(port: int = 8000) -> list[int]: try: res = subprocess.run( ["lsof", "-ti", f":{port}", "-sTCP:LISTEN"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=2.0, @@ -152,6 +153,7 @@ def find_server_pids(port: int = 8000) -> list[int]: try: res = subprocess.run( ["fuser", f"{port}/tcp"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=2.0, @@ -168,6 +170,7 @@ def find_server_pids(port: int = 8000) -> list[int]: try: res = subprocess.run( ["netstat", "-ano"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=3.0, diff --git a/artemis/utils/logger.py b/artemis/utils/logger.py index a2416f1..4d863f6 100644 --- a/artemis/utils/logger.py +++ b/artemis/utils/logger.py @@ -168,9 +168,9 @@ class ArtemisLogger: colored_separator = f"{Fore.CYAN}{separator}{Style.RESET_ALL}" colored_message = f"{Fore.CYAN}{message}{Style.RESET_ALL}" - print(colored_separator) - print(colored_message) - print(colored_separator) + print(colored_separator, file=sys.stderr) + print(colored_message, file=sys.stderr) + print(colored_separator, file=sys.stderr) self.logger.info(f"\n{separator}\n{message}\n{separator}") diff --git a/artemis/utils/shell_utils.py b/artemis/utils/shell_utils.py index b3c27ad..1972a73 100644 --- a/artemis/utils/shell_utils.py +++ b/artemis/utils/shell_utils.py @@ -20,6 +20,7 @@ def run_shell_command_on_host(command: str) -> str: try: result = subprocess.run( command, + stdin=subprocess.DEVNULL, shell=True, check=True, capture_output=True, diff --git a/mcp_server/server.py b/mcp_server/server.py index a6caeff..f6ab008 100644 --- a/mcp_server/server.py +++ b/mcp_server/server.py @@ -59,9 +59,13 @@ import mcp_server.tools # noqa: F401 from artemis.runtime import shutdown_awake_service, start_awake_service +import threading + def main(transport: str = "stdio", host: str = "127.0.0.1", port: int = 8001): """Main entrypoint to run the Artemis Mobile Agent MCP server.""" - start_awake_service() + threading.Thread( + target=start_awake_service, daemon=True, name="artemis-awake-init" + ).start() try: if transport.lower() == "sse": mcp.run(transport="sse", host=host, port=port) diff --git a/mcp_server/utils/device_utils.py b/mcp_server/utils/device_utils.py index 824b01e..d2ca5a7 100644 --- a/mcp_server/utils/device_utils.py +++ b/mcp_server/utils/device_utils.py @@ -99,6 +99,7 @@ def get_connected_devices(adb_path: str | None = None) -> list[str]: try: res = subprocess.run( [adb, "devices"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=5, @@ -137,7 +138,7 @@ def ensure_emulator( if not emu_exe or not os.path.exists(emu_exe): return False - popen_kwargs = {"stdout": subprocess.DEVNULL, "stderr": subprocess.DEVNULL} + popen_kwargs = {"stdin": subprocess.DEVNULL, "stdout": subprocess.DEVNULL, "stderr": subprocess.DEVNULL} if sys.platform == "win32": popen_kwargs["creationflags"] = ( subprocess.CREATE_NEW_PROCESS_GROUP | subprocess.DETACHED_PROCESS # type: ignore[attr-defined] @@ -158,6 +159,7 @@ def ensure_emulator( try: res = subprocess.run( [adb, "-s", "emulator-5554", "shell", "getprop", "sys.boot_completed"], + stdin=subprocess.DEVNULL, capture_output=True, text=True, timeout=3, diff --git a/mcp_server/utils/env_utils.py b/mcp_server/utils/env_utils.py index c5b5193..941c512 100644 --- a/mcp_server/utils/env_utils.py +++ b/mcp_server/utils/env_utils.py @@ -49,6 +49,7 @@ def resolve_python_executable(project_root: str | None = None) -> str: def get_detached_process_kwargs() -> dict[str, Any]: """Returns kwargs for subprocess.Popen to run a fully detached background process cross-platform.""" kwargs: dict[str, Any] = { + "stdin": subprocess.DEVNULL, "stdout": subprocess.DEVNULL, "stderr": subprocess.DEVNULL, } diff --git a/tests/unit/mcp/test_mcp_stdio_lifecycle.py b/tests/unit/mcp/test_mcp_stdio_lifecycle.py new file mode 100644 index 0000000..b01ff6d --- /dev/null +++ b/tests/unit/mcp/test_mcp_stdio_lifecycle.py @@ -0,0 +1,199 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Unit and integration tests verifying MCP stdio handshake reliability, + +subprocess stdin isolation against pipe hijacking, and stdout purity. +""" + +import io +import json +import os +import subprocess +import sys +import time +from unittest.mock import MagicMock, patch + +import pytest + +from artemis.clients import ui_automator_client +from artemis.runtime.awake_lease import ScreenAwakeLease +from artemis.runtime.awake_service import _run_awake_adb_command +from artemis.utils.logger import get_logger +from mcp_server.utils import device_utils, env_utils + + +def test_mcp_stdio_handshake_immediate_input(): + """Verify that MCP server over stdio responds to an immediate initialize request + + without hanging, even when stdin data is pushed concurrently with process startup. + """ + project_root = os.path.dirname(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))) + python_exe = sys.executable + + p = subprocess.Popen( + [python_exe, "-m", "mcp_server"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env={ + "PYTHONUNBUFFERED": "1", + "PATH": os.environ.get("PATH", ""), + "PYTHONPATH": project_root, + }, + cwd=project_root, + ) + + try: + # Immediately write the JSON-RPC initialize request into stdin + init_req = { + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": {"name": "test-client", "version": "1.0.0"}, + }, + } + p.stdin.write(json.dumps(init_req).encode("utf-8") + b"\n") + p.stdin.flush() + + # Read initialize response with a strict timeout + start = time.time() + init_resp_line = None + while time.time() - start < 6.0: + import select + + r, _, _ = select.select([p.stdout], [], [], 0.2) + if r: + init_resp_line = p.stdout.readline().decode("utf-8").strip() + break + + assert init_resp_line is not None, "MCP server failed to respond to initialize request within 6 seconds (deadlock detected)!" + init_data = json.loads(init_resp_line) + assert init_data.get("id") == 1 + assert "result" in init_data + + # Follow up with initialized notification and tools/list request + notif = {"jsonrpc": "2.0", "method": "notifications/initialized", "params": {}} + tools_req = {"jsonrpc": "2.0", "id": 2, "method": "tools/list", "params": {}} + p.stdin.write(json.dumps(notif).encode("utf-8") + b"\n") + p.stdin.write(json.dumps(tools_req).encode("utf-8") + b"\n") + p.stdin.flush() + + tools_resp_line = None + start = time.time() + while time.time() - start < 4.0: + import select + + r, _, _ = select.select([p.stdout], [], [], 0.2) + if r: + tools_resp_line = p.stdout.readline().decode("utf-8").strip() + break + + assert tools_resp_line is not None, "MCP server failed to respond to tools/list request!" + tools_data = json.loads(tools_resp_line) + assert tools_data.get("id") == 2 + tools = tools_data.get("result", {}).get("tools", []) + tool_names = {t["name"] for t in tools} + assert { + "mobile_run_task", + "mobile_manage_task", + "mobile_get_device_state", + "mobile_inspect_trace", + }.issubset(tool_names) + + finally: + p.terminate() + try: + p.wait(timeout=3) + except subprocess.TimeoutExpired: + p.kill() + + +def test_awake_service_adb_command_isolates_stdin(): + """Verify _run_awake_adb_command always sets stdin=subprocess.DEVNULL.""" + with patch("artemis.runtime.awake_service.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="ok", stderr="") + _run_awake_adb_command("test-dev-1", ["shell", "date"], "test command") + + assert mock_run.called + kwargs = mock_run.call_args.kwargs + assert kwargs.get("stdin") == subprocess.DEVNULL, "Expected stdin=subprocess.DEVNULL to prevent stdin hijacking!" + + +def test_awake_lease_run_isolates_stdin(): + """Verify ScreenAwakeLease._run always sets stdin=subprocess.DEVNULL.""" + lease = ScreenAwakeLease("test-dev-1") + with patch("artemis.runtime.awake_lease.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="ok", stderr="") + lease._run(["shell", "date"], "test lease command") + + assert mock_run.called + kwargs = mock_run.call_args.kwargs + assert kwargs.get("stdin") == subprocess.DEVNULL, "Expected stdin=subprocess.DEVNULL to prevent stdin hijacking!" + + +def test_detached_process_kwargs_isolates_stdin(): + """Verify get_detached_process_kwargs always sets stdin=subprocess.DEVNULL.""" + kwargs = env_utils.get_detached_process_kwargs() + assert kwargs.get("stdin") == subprocess.DEVNULL, "Expected stdin=subprocess.DEVNULL for detached background tasks!" + + +def test_device_utils_isolates_stdin(): + """Verify device_utils subprocess calls always set stdin=subprocess.DEVNULL.""" + with patch("mcp_server.utils.device_utils.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="List of devices attached\n", stderr="") + device_utils.get_connected_devices() + assert mock_run.called + assert mock_run.call_args.kwargs.get("stdin") == subprocess.DEVNULL + + +def test_ui_automator_client_isolates_stdin(): + """Verify ui_automator_client helper commands isolate stdin.""" + with patch("artemis.clients.ui_automator_client.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="package:com.test\n", stderr="") + ui_automator_client._is_package_installed("dev-1", "com.test") + assert mock_run.called + assert mock_run.call_args.kwargs.get("stdin") == subprocess.DEVNULL + + with patch("artemis.clients.ui_automator_client.subprocess.run") as mock_run: + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") + ui_automator_client._uninstall_package("dev-1", "com.test") + assert mock_run.called + assert mock_run.call_args.kwargs.get("stdin") == subprocess.DEVNULL + + +def test_logger_header_does_not_pollute_stdout(): + """Verify logger.header directs output to stderr and never pollutes sys.stdout.""" + logger = get_logger("test_purity_logger") + captured_stdout = io.StringIO() + captured_stderr = io.StringIO() + + orig_stdout = sys.stdout + orig_stderr = sys.stderr + try: + sys.stdout = captured_stdout + sys.stderr = captured_stderr + logger.header("IMPORTANT PROTOCOL BANNER") + finally: + sys.stdout = orig_stdout + sys.stderr = orig_stderr + + assert captured_stdout.getvalue() == "", ( + f"Detected stdout pollution from logger.header: {captured_stdout.getvalue()!r}. " + "In MCP stdio mode, stdout MUST be reserved exclusively for JSON-RPC messages!" + ) + assert "IMPORTANT PROTOCOL BANNER" in captured_stderr.getvalue()