<!-- CURSOR_SUMMARY --> > [!NOTE] > **Medium Risk** > Touches authentication/login flow, credentials-file permissions, and subprocess lifecycle behavior across platforms; while mostly defensive, regressions could impact login or process management on Windows/macOS runners. > > **Overview** > Improves Windows/cross-platform reliability across the CLI and MCP server: OAuth login now binds the callback server to `127.0.0.1`, avoids slow loopback reverse-DNS, adds a configurable callback timeout (`--timeout` + env default), and opens URLs via a Windows-friendly `_open_browser` to avoid flashing console windows. > > Centralizes CLI output via a shared `console` that forces UTF-8 on Windows, standardizes UTF-8 file reads/writes throughout, tightens credentials-file permissions on Windows using `icacls`, and adds shared Windows subprocess helpers for **no-window** process creation and graceful termination (used by `deploy`, MCP reload, and usage-tracking worker). > > Updates client configuration UX/robustness (Windows AppData resolution via `platformdirs`, Cursor config path fallbacks + compatibility writes, overwrite warnings, absolute `uv` path for GUI clients, safer path display) and improves `deploy` child-process handling to avoid pipe-buffer deadlocks while giving better debug-aware error messages. > > Expands CI to run tests on Linux/Windows/macOS, adds a no-auth CLI integration workflow, disables usage tracking in toolkits CI, and adds extensive regression tests for Windows signals, subprocess cleanup, UTF-8, and config-path edge cases; bumps `arcade-core` to `4.4.2` and `arcade-mcp-server` to `1.17.2` (with updated dependency pin). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 0fabd8ca1cd647039ba6ddbdf3f7809c330bab9e. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
296 lines
11 KiB
Python
296 lines
11 KiB
Python
"""Tests for Windows-specific subprocess flags and signal handling.
|
|
|
|
Verifies that:
|
|
- Background subprocess calls set CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP on Windows.
|
|
- _graceful_terminate sends CTRL_BREAK_EVENT on Windows, falls back to terminate().
|
|
- MCPApp._run_with_reload shutdown uses CTRL_BREAK_EVENT on Windows.
|
|
- stdio transport registers a stdlib signal.signal fallback on Windows.
|
|
|
|
Tests that verify Windows-specific *logic* (flag construction, signal dispatch)
|
|
keep ``sys.platform`` mocking because Popen/process objects are also fully mocked.
|
|
Tests for the non-Windows path use ``pytest.mark.skipif`` instead.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import asyncio
|
|
import signal
|
|
import subprocess
|
|
import sys
|
|
from collections.abc import Iterator
|
|
from contextlib import contextmanager
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
import pytest
|
|
|
|
# Shared constants/helpers keep Windows behavior tests DRY and focused.
|
|
WIN_CREATE_NO_WINDOW = getattr(subprocess, "CREATE_NO_WINDOW", 0x08000000)
|
|
WIN_CREATE_NEW_PROCESS_GROUP = getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200)
|
|
WIN_CTRL_BREAK_EVENT = getattr(signal, "CTRL_BREAK_EVENT", 1)
|
|
|
|
|
|
def _running_process() -> MagicMock:
|
|
proc = MagicMock()
|
|
proc.poll.return_value = None # Process is running
|
|
return proc
|
|
|
|
|
|
@contextmanager
|
|
def _patch_win32_subprocess_flags() -> Iterator[None]:
|
|
with (
|
|
patch.object(sys, "platform", "win32"),
|
|
patch.object(subprocess, "CREATE_NO_WINDOW", WIN_CREATE_NO_WINDOW, create=True),
|
|
patch.object(
|
|
subprocess,
|
|
"CREATE_NEW_PROCESS_GROUP",
|
|
WIN_CREATE_NEW_PROCESS_GROUP,
|
|
create=True,
|
|
),
|
|
):
|
|
yield
|
|
|
|
|
|
@contextmanager
|
|
def _patch_win32_ctrl_break() -> Iterator[None]:
|
|
with (
|
|
patch.object(sys, "platform", "win32"),
|
|
patch.object(signal, "CTRL_BREAK_EVENT", WIN_CTRL_BREAK_EVENT, create=True),
|
|
):
|
|
yield
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# deploy.py — start_server_process()
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestDeployCreateNoWindow:
|
|
"""Verify start_server_process sets CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP on Windows."""
|
|
|
|
@patch("arcade_cli.deploy.find_python_interpreter")
|
|
@patch("arcade_cli.deploy.subprocess.Popen")
|
|
def test_sets_flags_on_win32(
|
|
self, mock_popen: MagicMock, mock_python: MagicMock
|
|
) -> None:
|
|
mock_python.return_value = Path("python.exe")
|
|
mock_popen.return_value = _running_process()
|
|
|
|
# sys.platform mock: verifies flag-construction logic with fully-mocked Popen.
|
|
with _patch_win32_subprocess_flags():
|
|
from arcade_cli.deploy import start_server_process
|
|
start_server_process("server.py")
|
|
|
|
_, kwargs = mock_popen.call_args
|
|
flags = kwargs.get("creationflags", 0)
|
|
# Both flags must be present
|
|
assert flags & WIN_CREATE_NO_WINDOW, "CREATE_NO_WINDOW must be set"
|
|
assert flags & WIN_CREATE_NEW_PROCESS_GROUP, "CREATE_NEW_PROCESS_GROUP must be set"
|
|
|
|
@pytest.mark.skipif(sys.platform == "win32", reason="Non-Windows path: creationflags must be 0")
|
|
@patch("arcade_cli.deploy.find_python_interpreter")
|
|
@patch("arcade_cli.deploy.subprocess.Popen")
|
|
def test_no_creationflags_on_non_windows(
|
|
self, mock_popen: MagicMock, mock_python: MagicMock
|
|
) -> None:
|
|
mock_python.return_value = Path("python3")
|
|
mock_popen.return_value = _running_process()
|
|
|
|
from arcade_cli.deploy import start_server_process
|
|
start_server_process("server.py")
|
|
|
|
_, kwargs = mock_popen.call_args
|
|
assert kwargs.get("creationflags") == 0
|
|
|
|
@pytest.mark.parametrize(
|
|
("debug", "expects_devnull"),
|
|
[
|
|
(False, True),
|
|
(True, False),
|
|
],
|
|
ids=["non-debug-devnull", "debug-inherits-streams"],
|
|
)
|
|
@patch("arcade_cli.deploy.find_python_interpreter")
|
|
@patch("arcade_cli.deploy.subprocess.Popen")
|
|
def test_stream_configuration_by_debug_mode(
|
|
self,
|
|
mock_popen: MagicMock,
|
|
mock_python: MagicMock,
|
|
debug: bool,
|
|
expects_devnull: bool,
|
|
) -> None:
|
|
"""Stream handling should switch between DEVNULL and inherited streams."""
|
|
mock_python.return_value = Path("python.exe")
|
|
mock_popen.return_value = _running_process()
|
|
|
|
# sys.platform mock: verifies stream-mode logic with fully-mocked Popen.
|
|
with _patch_win32_subprocess_flags():
|
|
from arcade_cli.deploy import start_server_process
|
|
start_server_process("server.py", debug=debug)
|
|
|
|
_, kwargs = mock_popen.call_args
|
|
if expects_devnull:
|
|
assert kwargs.get("stdout") == subprocess.DEVNULL
|
|
assert kwargs.get("stderr") == subprocess.DEVNULL
|
|
else:
|
|
assert kwargs.get("stdout") is None
|
|
assert kwargs.get("stderr") is None
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# deploy.py — _graceful_terminate()
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestGracefulTerminate:
|
|
"""Verify _graceful_terminate uses CTRL_BREAK_EVENT on Windows."""
|
|
|
|
def test_sends_ctrl_break_on_win32(self) -> None:
|
|
"""On Windows, _graceful_terminate should send CTRL_BREAK_EVENT."""
|
|
from arcade_cli.deploy import _graceful_terminate
|
|
|
|
mock_proc = MagicMock()
|
|
|
|
# sys.platform mock: verifies CTRL_BREAK_EVENT dispatch with mocked process.
|
|
with _patch_win32_ctrl_break():
|
|
_graceful_terminate(mock_proc)
|
|
|
|
# Should try send_signal with CTRL_BREAK_EVENT (not terminate)
|
|
mock_proc.send_signal.assert_called_once_with(WIN_CTRL_BREAK_EVENT)
|
|
mock_proc.terminate.assert_not_called()
|
|
|
|
def test_falls_back_to_terminate_on_win32_oserror(self) -> None:
|
|
"""If send_signal fails on Windows, fall back to terminate."""
|
|
from arcade_cli.deploy import _graceful_terminate
|
|
|
|
mock_proc = MagicMock()
|
|
mock_proc.send_signal.side_effect = OSError("Process exited")
|
|
|
|
# sys.platform mock: exercises OSError fallback with mocked process.
|
|
with _patch_win32_ctrl_break():
|
|
_graceful_terminate(mock_proc)
|
|
|
|
mock_proc.terminate.assert_called_once()
|
|
|
|
@pytest.mark.skipif(sys.platform == "win32", reason="Non-Windows terminate() path")
|
|
def test_calls_terminate_on_linux(self) -> None:
|
|
"""On Linux/macOS, _graceful_terminate should call terminate() directly."""
|
|
from arcade_cli.deploy import _graceful_terminate
|
|
|
|
mock_proc = MagicMock()
|
|
|
|
_graceful_terminate(mock_proc)
|
|
|
|
mock_proc.terminate.assert_called_once()
|
|
mock_proc.send_signal.assert_not_called()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# mcp_app.py — runtime behavior checks
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestMcpAppSubprocess:
|
|
"""Verify MCPApp._run_with_reload subprocess behavior at runtime."""
|
|
|
|
def test_shutdown_sends_ctrl_break_on_win32(self) -> None:
|
|
"""On Windows, _run_with_reload sends CTRL_BREAK_EVENT for graceful child shutdown."""
|
|
from arcade_mcp_server.mcp_app import MCPApp
|
|
|
|
mock_proc = MagicMock()
|
|
mock_proc.wait.return_value = None
|
|
|
|
# sys.platform mock: exercises Windows graceful shutdown path with mocked Popen/signal.
|
|
with (
|
|
_patch_win32_subprocess_flags(),
|
|
patch.object(signal, "CTRL_BREAK_EVENT", WIN_CTRL_BREAK_EVENT, create=True),
|
|
patch.object(subprocess, "Popen", return_value=mock_proc),
|
|
patch("arcade_mcp_server.mcp_app.watch", side_effect=KeyboardInterrupt),
|
|
):
|
|
app = MCPApp()
|
|
app._run_with_reload("127.0.0.1", 8000)
|
|
|
|
mock_proc.send_signal.assert_called_once_with(WIN_CTRL_BREAK_EVENT)
|
|
mock_proc.terminate.assert_not_called()
|
|
|
|
def test_shutdown_falls_back_to_terminate_on_win32_oserror(self) -> None:
|
|
"""On Windows, shutdown falls back to terminate() if send_signal raises OSError."""
|
|
from arcade_mcp_server.mcp_app import MCPApp
|
|
|
|
mock_proc = MagicMock()
|
|
mock_proc.send_signal.side_effect = OSError("process already exited")
|
|
mock_proc.wait.return_value = None
|
|
|
|
# sys.platform mock: exercises OSError fallback path with mocked Popen/signal.
|
|
with (
|
|
_patch_win32_subprocess_flags(),
|
|
patch.object(signal, "CTRL_BREAK_EVENT", WIN_CTRL_BREAK_EVENT, create=True),
|
|
patch.object(subprocess, "Popen", return_value=mock_proc),
|
|
patch("arcade_mcp_server.mcp_app.watch", side_effect=KeyboardInterrupt),
|
|
):
|
|
app = MCPApp()
|
|
app._run_with_reload("127.0.0.1", 8000)
|
|
|
|
mock_proc.terminate.assert_called_once()
|
|
|
|
@pytest.mark.skipif(sys.platform == "win32", reason="Non-Windows terminate() path")
|
|
def test_shutdown_calls_terminate_on_non_windows(self) -> None:
|
|
"""On Linux/macOS, _run_with_reload uses terminate() for graceful child shutdown."""
|
|
from arcade_mcp_server.mcp_app import MCPApp
|
|
|
|
mock_proc = MagicMock()
|
|
mock_proc.wait.return_value = None
|
|
|
|
with (
|
|
patch.object(subprocess, "Popen", return_value=mock_proc),
|
|
patch("arcade_mcp_server.mcp_app.watch", side_effect=KeyboardInterrupt),
|
|
):
|
|
app = MCPApp()
|
|
app._run_with_reload("127.0.0.1", 8000)
|
|
|
|
mock_proc.terminate.assert_called_once()
|
|
mock_proc.send_signal.assert_not_called()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# stdio.py — signal handler fallback
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
class TestStdioSignalFallback:
|
|
"""Verify stdio transport registers a stdlib signal.signal fallback on Windows."""
|
|
|
|
@pytest.mark.asyncio
|
|
async def test_registers_stdlib_signal_handler_on_windows(self) -> None:
|
|
"""On Windows, StdioTransport.start() calls signal.signal(SIGINT, ...) as fallback."""
|
|
import arcade_mcp_server.transports.stdio as stdio_mod
|
|
from arcade_mcp_server.transports.stdio import StdioTransport
|
|
|
|
transport = StdioTransport(name="test-win32-sigint")
|
|
registered_signals: list[int] = []
|
|
|
|
def capture_signal(signum: int, handler: object) -> None:
|
|
registered_signals.append(signum)
|
|
|
|
# sys.platform mock: exercises NotImplementedError fallback path that
|
|
# only occurs on Windows when asyncio signal handlers are unavailable.
|
|
with patch.object(sys, "platform", "win32"):
|
|
loop = asyncio.get_running_loop()
|
|
original_add = loop.add_signal_handler
|
|
|
|
def raise_not_impl(*args: object, **kwargs: object) -> None:
|
|
raise NotImplementedError
|
|
|
|
loop.add_signal_handler = raise_not_impl # type: ignore[assignment]
|
|
with patch.object(stdio_mod.signal, "signal", side_effect=capture_signal):
|
|
try:
|
|
await transport.start()
|
|
finally:
|
|
loop.add_signal_handler = original_add # type: ignore[assignment]
|
|
await transport.stop()
|
|
|
|
assert signal.SIGINT in registered_signals, (
|
|
"StdioTransport must register signal.signal(SIGINT, ...) on Windows "
|
|
"as asyncio fallback; registered signals: "
|
|
f"{registered_signals}"
|
|
)
|