From fe8ddfd500c26e49f7f24762ea42e02b60f90cfc Mon Sep 17 00:00:00 2001 From: jottakka Date: Wed, 25 Feb 2026 16:18:16 +0000 Subject: [PATCH] [TOO-326] Windows papercuts (#768) > [!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). > > 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). --- .github/workflows/cli-integration-no-auth.yml | 39 + .github/workflows/main.yml | 8 +- .github/workflows/test-install.yml | 15 +- .github/workflows/test-toolkits.yml | 13 +- .pre-commit-config.yaml | 4 +- CONTRIBUTING.md | 2 +- libs/arcade-cli/arcade_cli/authn.py | 260 ++++++- libs/arcade-cli/arcade_cli/config.py | 3 +- libs/arcade-cli/arcade_cli/configure.py | 175 ++++- libs/arcade-cli/arcade_cli/console.py | 43 ++ libs/arcade-cli/arcade_cli/deploy.py | 94 ++- libs/arcade-cli/arcade_cli/display.py | 3 +- libs/arcade-cli/arcade_cli/main.py | 43 +- libs/arcade-cli/arcade_cli/new.py | 30 +- libs/arcade-cli/arcade_cli/org.py | 5 +- libs/arcade-cli/arcade_cli/project.py | 5 +- libs/arcade-cli/arcade_cli/secret.py | 7 +- libs/arcade-cli/arcade_cli/server.py | 12 +- .../arcade_cli/usage/command_tracker.py | 4 +- libs/arcade-cli/arcade_cli/utils.py | 6 +- libs/arcade-core/arcade_core/config_model.py | 73 +- libs/arcade-core/arcade_core/parse.py | 2 +- .../arcade_core/subprocess_utils.py | 68 ++ libs/arcade-core/arcade_core/toolkit.py | 2 +- .../arcade-core/arcade_core/usage/identity.py | 6 +- .../arcade_core/usage/usage_service.py | 51 +- libs/arcade-core/pyproject.toml | 2 +- libs/arcade-evals/arcade_evals/capture.py | 2 +- .../arcade_mcp_server/auth/__init__.py | 2 +- .../arcade_mcp_server/mcp_app.py | 20 +- .../arcade_mcp_server/transports/stdio.py | 18 +- libs/arcade-mcp-server/pyproject.toml | 4 +- libs/arcade-tdk/arcade_tdk/auth/__init__.py | 4 +- libs/arcade-tdk/arcade_tdk/errors.py | 2 +- .../integration/test_end_to_end.py | 208 ++++-- libs/tests/arcade_mcp_server/test_mcp_app.py | 33 +- libs/tests/cli/deploy/test_deploy.py | 106 ++- libs/tests/cli/test_authn_callback.py | 336 +++++++++ libs/tests/cli/test_configure.py | 484 ++++++++++++ libs/tests/cli/test_console_encoding.py | 161 ++++ libs/tests/cli/test_cross_platform.py | 211 ++++++ libs/tests/cli/test_dashboard.py | 18 +- libs/tests/cli/test_display.py | 21 +- libs/tests/cli/test_eval_paths.py | 22 + libs/tests/cli/test_new_cli.py | 62 ++ libs/tests/cli/test_secret.py | 8 +- libs/tests/cli/test_server_logs.py | 102 +++ libs/tests/cli/test_stdio_signal.py | 218 ++++++ libs/tests/cli/test_utils_multi_provider.py | 13 +- libs/tests/cli/test_windows_subprocess.py | 296 ++++++++ libs/tests/cli/usage/test_identity.py | 25 +- libs/tests/core/test_dependency_alignment.py | 49 ++ libs/tests/core/test_subprocess_utils.py | 126 ++++ libs/tests/core/test_toolkit.py | 16 +- libs/tests/core/usage/test_usage_service.py | 152 ++++ libs/tests/sdk/test_eval_capture.py | 4 +- libs/tests/sdk/test_eval_multi_run.py | 1 - pyproject.toml | 14 +- tests/install/README.md | 51 +- tests/install/conftest.py | 29 + tests/install/test_install.py | 67 +- tests/integration/mcp_protocol_smoke.py | 694 ++++++++++++++++++ tests/integration/no_auth_cli_smoke.py | 299 ++++++++ 63 files changed, 4544 insertions(+), 309 deletions(-) create mode 100644 .github/workflows/cli-integration-no-auth.yml create mode 100644 libs/arcade-cli/arcade_cli/console.py create mode 100644 libs/arcade-core/arcade_core/subprocess_utils.py create mode 100644 libs/tests/cli/test_authn_callback.py create mode 100644 libs/tests/cli/test_configure.py create mode 100644 libs/tests/cli/test_console_encoding.py create mode 100644 libs/tests/cli/test_cross_platform.py create mode 100644 libs/tests/cli/test_eval_paths.py create mode 100644 libs/tests/cli/test_new_cli.py create mode 100644 libs/tests/cli/test_server_logs.py create mode 100644 libs/tests/cli/test_stdio_signal.py create mode 100644 libs/tests/cli/test_windows_subprocess.py create mode 100644 libs/tests/core/test_dependency_alignment.py create mode 100644 libs/tests/core/test_subprocess_utils.py create mode 100644 libs/tests/core/usage/test_usage_service.py create mode 100644 tests/install/conftest.py create mode 100644 tests/integration/mcp_protocol_smoke.py create mode 100644 tests/integration/no_auth_cli_smoke.py diff --git a/.github/workflows/cli-integration-no-auth.yml b/.github/workflows/cli-integration-no-auth.yml new file mode 100644 index 00000000..cba39a1f --- /dev/null +++ b/.github/workflows/cli-integration-no-auth.yml @@ -0,0 +1,39 @@ +name: CLI Integration (No Auth) + +on: + pull_request: + branches: [main] + push: + branches: [main] + workflow_dispatch: + +jobs: + cli-integration-no-auth: + name: CLI Integration No-Auth (${{ matrix.os }}, Python ${{ matrix.python-version }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 35 + env: + ARCADE_USAGE_TRACKING: "0" + + strategy: + matrix: + os: [ubuntu-latest, windows-latest, macos-latest] + python-version: ["3.10", "3.11", "3.12"] + fail-fast: false + + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up environment + uses: ./.github/actions/setup-uv-env + with: + python-version: ${{ matrix.python-version }} + + - name: Install arcade-mcp from source + run: | + uv pip install -e . + + - name: Run no-auth CLI smoke checks + run: | + uv run python tests/integration/no_auth_cli_smoke.py diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ad1d6e82..8c21ccfc 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -26,9 +26,11 @@ jobs: run: make check test: - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} + timeout-minutes: 25 strategy: matrix: + os: [ubuntu-latest, windows-latest, macos-latest] python-version: ["3.10", "3.11", "3.12"] fail-fast: false steps: @@ -41,9 +43,9 @@ jobs: python-version: ${{ matrix.python-version }} - name: Test libs - run: make test + run: uv run pytest -W ignore -v libs/tests --cov=libs --cov-config=pyproject.toml --cov-report=xml - name: Upload coverage reports to Codecov with GitHub Action on Python 3.10 uses: codecov/codecov-action@v4.0.1 - if: ${{ matrix.python-version == '3.10' }} + if: ${{ matrix.os == 'ubuntu-latest' && matrix.python-version == '3.10' }} with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/test-install.yml b/.github/workflows/test-install.yml index c5500944..50de01ec 100644 --- a/.github/workflows/test-install.yml +++ b/.github/workflows/test-install.yml @@ -21,27 +21,18 @@ jobs: - name: Checkout repository uses: actions/checkout@v4 - - name: Install uv - uses: astral-sh/setup-uv@v5 - with: - version: "latest" - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 + - name: Set up environment + uses: ./.github/actions/setup-uv-env with: python-version: ${{ matrix.python-version }} - - name: Install dependencies - run: | - uv sync --extra all --extra dev - - name: Install arcade-mcp from source run: | uv pip install -e . - name: Run installation test run: | - python tests/install/test_install.py + uv run pytest tests/install/ -v --tb=short - name: Verify arcade CLI is available run: | diff --git a/.github/workflows/test-toolkits.yml b/.github/workflows/test-toolkits.yml index ca514166..0195197b 100644 --- a/.github/workflows/test-toolkits.yml +++ b/.github/workflows/test-toolkits.yml @@ -7,6 +7,9 @@ on: pull_request: types: [opened, synchronize, reopened, ready_for_review] +env: + ARCADE_USAGE_TRACKING: "0" + jobs: setup: runs-on: ubuntu-latest @@ -32,11 +35,13 @@ jobs: test-toolkits: needs: setup - runs-on: ubuntu-latest + name: test-toolkits (${{ matrix.toolkit }}, ${{ matrix.os }}) + runs-on: ${{ matrix.os }} strategy: matrix: + os: [ubuntu-latest, windows-latest, macos-latest] toolkit: ${{ fromJson(needs.setup.outputs.toolkits_without_gha_secrets) }} - fail-fast: true + fail-fast: false steps: - name: Check out uses: actions/checkout@v4 @@ -49,16 +54,19 @@ jobs: - name: Install toolkit dependencies working-directory: toolkits/${{ matrix.toolkit }} + shell: bash run: uv pip install -e ".[dev]" - name: Check toolkit working-directory: toolkits/${{ matrix.toolkit }} + shell: bash run: | uv run --active pre-commit run -a uv run --active mypy --config-file=pyproject.toml - name: Test stand-alone toolkits (no secrets) working-directory: toolkits/${{ matrix.toolkit }} + shell: bash run: | # Run pytest and capture exit code uv run --active pytest -W ignore -v --cov=arcade_${{ matrix.toolkit }} --cov-report=xml || EXIT_CODE=$? @@ -72,6 +80,7 @@ jobs: test-toolkits-with-gha-secrets: needs: setup + # Linux-only: these toolkits bootstrap local DBs via docker/apt in tests/test_setup.sh. runs-on: ubuntu-latest strategy: matrix: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e9a74942..761cc8bf 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,6 +17,6 @@ repos: hooks: - id: ruff args: [--fix] - exclude: "(.*/templates/.*|libs/tests/.*)" + exclude: "(.*/templates/.*|libs/tests/.*|_scratch/.*)" - id: ruff-format - exclude: "(.*/templates/.*|libs/tests/.*)" + exclude: "(.*/templates/.*|libs/tests/.*|_scratch/.*)" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1f6c45b5..ddf4300b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -72,7 +72,7 @@ uv venv --python 3.11.6 ```bash # Install all packages and development dependencies via uv workspace -uv sync --extra all --dev +uv sync --extra all --extra dev # Install pre-commit hooks for code quality uv run pre-commit install diff --git a/libs/arcade-cli/arcade_cli/authn.py b/libs/arcade-cli/arcade_cli/authn.py index 3073770f..301ce399 100644 --- a/libs/arcade-cli/arcade_cli/authn.py +++ b/libs/arcade-cli/arcade_cli/authn.py @@ -5,8 +5,12 @@ Implements OAuth 2.0 Authorization Code flow with PKCE for secure CLI authentica Uses authlib for OAuth protocol handling. """ +import logging import os import secrets +import socketserver +import subprocess +import sys import threading import uuid import webbrowser @@ -29,10 +33,14 @@ from arcade_core.auth_tokens import ( ) from arcade_core.config_model import AuthConfig, Config, ContextConfig, UserConfig from arcade_core.constants import ARCADE_CONFIG_PATH, CREDENTIALS_FILE_PATH +from arcade_core.subprocess_utils import build_windows_hidden_startupinfo from authlib.integrations.httpx_client import OAuth2Client from jinja2 import Environment, FileSystemLoader from pydantic import AliasChoices, BaseModel, Field -from rich.console import Console + +from arcade_cli.console import console + +logger = logging.getLogger(__name__) # Set up Jinja2 templates _TEMPLATES_DIR = Path(__file__).parent / "templates" @@ -45,11 +53,26 @@ def _render_template(template_name: str, **context: Any) -> bytes: return template.render(**context).encode("utf-8") -console = Console() - # OAuth constants DEFAULT_SCOPES = "openid offline_access" +LOCAL_CALLBACK_HOST = "127.0.0.1" LOCAL_CALLBACK_PORT = 9905 +_DEFAULT_OAUTH_TIMEOUT_FALLBACK_SECONDS = 600 + + +def _get_default_oauth_timeout_seconds() -> int: + value = os.environ.get( + "ARCADE_LOGIN_TIMEOUT_SECONDS", str(_DEFAULT_OAUTH_TIMEOUT_FALLBACK_SECONDS) + ) + try: + parsed = int(value) + except ValueError: + return _DEFAULT_OAUTH_TIMEOUT_FALLBACK_SECONDS + else: + return parsed if parsed > 0 else _DEFAULT_OAUTH_TIMEOUT_FALLBACK_SECONDS + + +DEFAULT_OAUTH_TIMEOUT_SECONDS = _get_default_oauth_timeout_seconds() def create_oauth_client(cli_config: CLIConfig) -> OAuth2Client: # type: ignore[no-any-unimported] @@ -280,12 +303,42 @@ def fetch_projects(coordinator_url: str, org_id: str) -> list[ProjectInfo]: return [ProjectInfo.model_validate(item) for item in data.get("data", {}).get("items", [])] +class _LoopbackHTTPServer(HTTPServer): + """HTTPServer that skips the potentially slow ``getfqdn()`` reverse-DNS + lookup in ``server_bind()``. + + ``HTTPServer.server_bind()`` calls ``socket.getfqdn(host)`` which invokes + ``gethostbyaddr("127.0.0.1")`` via the system resolver. On macOS CI + runners (Apple Silicon / macOS 14) the mDNSResponder can take 5-30 s to + resolve the loopback PTR record when the DNS cache is cold, causing the + daemon thread to block inside the constructor and ``ready_event`` to never + fire within the timeout window. + + We only listen on ``127.0.0.1`` for the OAuth callback, so we hard-set + ``server_name`` to ``"127.0.0.1"`` and skip the DNS round-trip entirely. + """ + + def server_bind(self) -> None: + socketserver.TCPServer.server_bind(self) + host, port = self.server_address[:2] + self.server_name = host + self.server_port = port + + class OAuthCallbackHandler(BaseHTTPRequestHandler): """HTTP request handler for OAuth callback.""" - def __init__(self, *args, state: str, result_holder: dict, **kwargs): # type: ignore[no-untyped-def] + def __init__( + self, + *args: Any, + state: str, + result_holder: dict, + result_event: threading.Event, + **kwargs: Any, + ): self.state = state self.result_holder = result_holder + self.result_event = result_event # Store error details for template rendering self._error: str | None = None self._error_description: str | None = None @@ -331,6 +384,7 @@ class OAuthCallbackHandler(BaseHTTPRequestHandler): self.send_header("Content-Type", "text/html; charset=utf-8") self.end_headers() self.wfile.write(_render_template("cli_login_success.jinja")) + self.result_event.set() threading.Thread(target=self.server.shutdown).start() def _send_error_response(self, message: str | None = None) -> None: @@ -346,6 +400,7 @@ class OAuthCallbackHandler(BaseHTTPRequestHandler): state=self._returned_state, ) ) + self.result_event.set() threading.Thread(target=self.server.shutdown).start() @@ -358,13 +413,34 @@ class OAuthCallbackServer: self.httpd: HTTPServer | None = None self.result: dict[str, Any] = {} + # Threading events used on *all* platforms (not Windows-specific). + # result_event: signalled by the HTTP handler once the OAuth callback + # has been processed (success or error). Callers block on this via + # wait_for_result() instead of polling. + # ready_event: signalled by run_server() once the HTTPServer is bound + # and listening. Callers block on this via wait_until_ready() so + # they don't race the browser redirect against server startup. + self.result_event = threading.Event() + self.ready_event = threading.Event() + def run_server(self) -> None: - """Start the callback server.""" - server_address = ("", self.port) + """Start the callback server. + + Binds to 127.0.0.1 (loopback only) rather than 0.0.0.0 to avoid + Windows Firewall prompts and keep the redirect URI host aligned + with the actual bind host. + """ + server_address = (LOCAL_CALLBACK_HOST, self.port) handler = lambda *args, **kwargs: OAuthCallbackHandler( - *args, state=self.state, result_holder=self.result, **kwargs + *args, + state=self.state, + result_holder=self.result, + result_event=self.result_event, + **kwargs, ) - self.httpd = HTTPServer(server_address, handler) + self.httpd = _LoopbackHTTPServer(server_address, handler) + self.port = self.httpd.server_port + self.ready_event.set() self.httpd.serve_forever() def shutdown_server(self) -> None: @@ -372,9 +448,27 @@ class OAuthCallbackServer: if self.httpd: self.httpd.shutdown() + def wait_until_ready(self, timeout: float | None = 2.0) -> bool: + """Wait for the server to start listening.""" + return self.ready_event.wait(timeout=timeout) + + def wait_for_result(self, timeout: float | None) -> bool: + """Wait for the OAuth callback to complete.""" + if self.result_event.wait(timeout=timeout): + return True + + timeout_desc = f"{int(timeout)}s" if timeout else "the configured timeout" + self.result["error"] = ( + f"Timed out waiting for the login callback after {timeout_desc}. " + "If your browser completed login, check firewall/antivirus settings " + "and re-run 'arcade login' (you can increase --timeout if needed)." + ) + self.shutdown_server() + return False + def get_redirect_uri(self) -> str: """Get the redirect URI for this server.""" - return f"http://localhost:{self.port}/callback" + return f"http://{LOCAL_CALLBACK_HOST}:{self.port}/callback" def save_credentials_from_whoami( @@ -449,6 +543,79 @@ def get_active_context() -> tuple[str, str]: # ============================================================================= +def _open_browser(url: str) -> bool: + """Open a URL in the default browser without flashing a CMD window on Windows. + + On Windows, both ``webbrowser.open`` and ``os.startfile`` call + ``ShellExecuteW`` under the hood which can briefly flash a console window + depending on how the default-browser handler is registered. + + This helper uses a tiered approach on Windows: + + 1. **ctypes ShellExecuteW** — calls the Win32 API directly so we can + pass ``SW_SHOWNORMAL`` explicitly. No intermediate ``cmd.exe`` + involved, so no console window should appear. + 2. **rundll32 url.dll** — a well-known Windows technique to open URLs + via a pure-GUI helper DLL. ``rundll32.exe`` is a GUI subsystem + binary so it never allocates a console. Used as a fallback when + ctypes is unavailable or ShellExecuteW returns an error code. + 3. **webbrowser.open** — stdlib last-resort fallback. + + ``os.startfile`` is intentionally omitted: it is another thin wrapper + around ``ShellExecuteExW`` and therefore redundant with attempt 1. + + On non-Windows platforms this simply delegates to ``webbrowser.open``. + """ + if sys.platform != "win32": + try: + return webbrowser.open(url) + except Exception: + return False + + # --- Windows path --- + + # Attempt 1: ctypes ShellExecuteW — most direct, avoids any console. + try: + import ctypes + + SW_SHOWNORMAL = 1 + result = ctypes.windll.shell32.ShellExecuteW( + None, # hwnd + "open", # operation + url, # file/URL + None, # parameters + None, # directory + SW_SHOWNORMAL, + ) + # ShellExecuteW returns > 32 on success. + if result > 32: + return True + except Exception as exc: + logger.debug("_open_browser: ShellExecuteW failed: %s", exc) + + # Attempt 2: rundll32 url.dll — a GUI-subsystem binary, no console. + try: + startupinfo = build_windows_hidden_startupinfo() + popen_kwargs: dict[str, Any] = { + "stdout": subprocess.DEVNULL, + "stderr": subprocess.DEVNULL, + } + if startupinfo is not None: + popen_kwargs["startupinfo"] = startupinfo + + subprocess.Popen(["rundll32", "url.dll,FileProtocolHandler", url], **popen_kwargs) # noqa: S607 + except Exception as exc: + logger.debug("_open_browser: rundll32 fallback failed: %s", exc) + else: + return True + + # Attempt 3: stdlib fallback. + try: + return webbrowser.open(url) + except Exception: + return False + + class OAuthLoginError(Exception): """Error during OAuth login flow.""" @@ -496,36 +663,44 @@ def build_coordinator_url(host: str, port: int | None) -> str: @contextmanager -def oauth_callback_server(state: str) -> Generator[OAuthCallbackServer, None, None]: +def oauth_callback_server( + state: str, port: int = LOCAL_CALLBACK_PORT +) -> Generator[OAuthCallbackServer, None, None]: """ Context manager for the OAuth callback server. Ensures the server is properly shut down even if an error occurs. - Waits for the callback to be received before exiting. + The caller is responsible for waiting on the callback result. Usage: with oauth_callback_server(state) as server: # server is running and waiting for callback ... - # After the with block, the server has received the callback + # After the with block, the server has been shut down """ - server = OAuthCallbackServer(state) - server_thread = threading.Thread(target=server.run_server) + server = OAuthCallbackServer(state, port=port) + # daemon=True ensures the thread is killed automatically when the main + # process exits (e.g. user presses Ctrl-C during login). Without it the + # blocking serve_forever() call would keep the process alive until the + # HTTP timeout expires, even after the CLI has printed an error. + server_thread = threading.Thread(target=server.run_server, daemon=True) server_thread.start() + # Give slower CI runners enough time to schedule the server thread and bind. + if not server.wait_until_ready(timeout=5.0): + server.shutdown_server() + server_thread.join(timeout=2) + raise RuntimeError("Failed to start local callback server.") try: yield server - # Wait for the callback to be received (server shuts itself down after handling) - server_thread.join() finally: - # Clean up if interrupted or if something went wrong - if server_thread.is_alive(): - server.shutdown_server() - server_thread.join(timeout=2) + server.shutdown_server() + server_thread.join(timeout=2) def perform_oauth_login( coordinator_url: str, on_status: Callable[[str], None] | None = None, + callback_timeout_seconds: int | None = None, ) -> OAuthLoginResult: """ Perform the complete OAuth login flow. @@ -540,6 +715,7 @@ def perform_oauth_login( Args: coordinator_url: Base URL of the Coordinator on_status: Optional callback for status messages (e.g., console.print) + callback_timeout_seconds: Optional timeout for the local callback server Returns: OAuthLoginResult with tokens and user info @@ -562,21 +738,39 @@ def perform_oauth_login( oauth_client = create_oauth_client(cli_config) state = str(uuid.uuid4()) + timeout_seconds = ( + callback_timeout_seconds + if callback_timeout_seconds is not None + else DEFAULT_OAUTH_TIMEOUT_SECONDS + ) + if timeout_seconds <= 0: + timeout_seconds = DEFAULT_OAUTH_TIMEOUT_SECONDS + # Step 3: Start local callback server and run browser auth - with oauth_callback_server(state) as server: - redirect_uri = server.get_redirect_uri() + try: + with oauth_callback_server(state) as server: + redirect_uri = server.get_redirect_uri() - # Step 4: Generate authorization URL and open browser - auth_url, code_verifier = generate_authorization_url( - oauth_client, cli_config, redirect_uri, state - ) + # Step 4: Generate authorization URL and open browser + auth_url, code_verifier = generate_authorization_url( + oauth_client, cli_config, redirect_uri, state + ) - status("Opening a browser to log you in...") - if not webbrowser.open(auth_url): - status(f"Copy this URL into your browser:\n{auth_url}") + status("Opening a browser to log you in...") + browser_opened = _open_browser(auth_url) - # Step 5: Wait for callback (server thread handles this via serve_forever) - # The thread will exit when the callback handler calls server.shutdown() + if not browser_opened: + status( + "Could not open a browser automatically.\n" + f"Open this link to log in:\n{auth_url}" + ) + + status(f"Waiting for login to complete (timeout: {timeout_seconds}s)...") + server.wait_for_result(timeout_seconds) + except OAuthLoginError: + raise + except Exception as e: + raise OAuthLoginError(str(e)) from e # Check for errors from callback if "error" in server.result: @@ -614,7 +808,7 @@ def _credentials_file_contains_legacy() -> bool: Detect legacy (API key) credentials in the credentials file. """ try: - with open(CREDENTIALS_FILE_PATH) as f: + with open(CREDENTIALS_FILE_PATH, encoding="utf-8") as f: data = yaml.safe_load(f) or {} cloud = data.get("cloud", {}) return isinstance(cloud, dict) and "api" in cloud @@ -636,7 +830,7 @@ def check_existing_login(suppress_message: bool = False) -> bool: return False try: - with open(CREDENTIALS_FILE_PATH) as f: + with open(CREDENTIALS_FILE_PATH, encoding="utf-8") as f: config_data: dict[str, Any] = yaml.safe_load(f) cloud_config = config_data.get("cloud", {}) if isinstance(config_data, dict) else {} diff --git a/libs/arcade-cli/arcade_cli/config.py b/libs/arcade-cli/arcade_cli/config.py index f7340e31..0459fd2d 100644 --- a/libs/arcade-cli/arcade_cli/config.py +++ b/libs/arcade-cli/arcade_cli/config.py @@ -5,10 +5,9 @@ Configuration utilities for the Arcade CLI. from dataclasses import dataclass from typing import Any -from rich.console import Console from rich.table import Table -console = Console() +from arcade_cli.console import console @dataclass diff --git a/libs/arcade-cli/arcade_cli/configure.py b/libs/arcade-cli/arcade_cli/configure.py index a6599850..05b0a592 100644 --- a/libs/arcade-cli/arcade_cli/configure.py +++ b/libs/arcade-cli/arcade_cli/configure.py @@ -1,6 +1,7 @@ """Connect command for configuring MCP clients.""" import json +import logging import os import platform import re @@ -10,9 +11,10 @@ from pathlib import Path import typer from dotenv import dotenv_values -from rich.console import Console -console = Console() +from arcade_cli.console import console + +logger = logging.getLogger(__name__) def is_wsl() -> bool: @@ -23,7 +25,7 @@ def is_wsl() -> bool: # Check /proc/version for WSL indicators try: - with open("/proc/version") as f: + with open("/proc/version", encoding="utf-8") as f: version_info = f.read().lower() return "microsoft" in version_info or "wsl" in version_info except (FileNotFoundError, PermissionError): @@ -53,6 +55,67 @@ def get_windows_username() -> str | None: return None +def _resolve_windows_appdata() -> Path: + """Resolve the Windows roaming AppData directory via ``platformdirs``. + + ``platformdirs`` is the de-facto standard Python library for resolving + OS-specific user directories. On Windows it reads the ``APPDATA`` + environment variable (and the Windows registry as a fallback), so a + single call covers every real-world scenario. + """ + from platformdirs import user_data_dir + + try: + result = user_data_dir(appname=None, appauthor=False, roaming=True) + except TypeError: + # Older platformdirs versions require positional args only. + # Signature: user_data_dir(appname, appauthor, version, roaming) + logger.debug("platformdirs raised TypeError; retrying with positional args") + result = user_data_dir(None, False, None, True) + + return Path(result) + + +def _dedupe_paths(paths: list[Path]) -> list[Path]: + """Return paths in order, removing duplicates (case-insensitive on Windows).""" + deduped: list[Path] = [] + seen: set[str] = set() + for path in paths: + key = os.path.normcase(str(path)) + if key in seen: + continue + seen.add(key) + deduped.append(path) + return deduped + + +def _get_windows_cursor_config_paths() -> list[Path]: + """Return known Windows Cursor config locations (primary first).""" + return _dedupe_paths([ + _resolve_windows_appdata() / "Cursor" / "mcp.json", + Path.home() / ".cursor" / "mcp.json", + ]) + + +def _format_path_for_display(path: Path, platform_system: str | None = None) -> str: + path_str = str(path) + if " " in path_str: + system = platform_system or platform.system() + if system == "Windows": + return f'"{path_str}"' + return path_str.replace(" ", "\\ ") + return path_str + + +def _warn_overwrite(config: dict, section: str, server_name: str, config_path: Path) -> None: + if section in config and server_name in config[section]: + config_display = _format_path_for_display(config_path) + console.print( + f"[yellow]Warning: MCP server '{server_name}' already exists in {config_display}. " + "This will overwrite the existing entry. Use --name to keep both.[/yellow]" + ) + + def get_claude_config_path() -> Path: """Get the Claude Desktop configuration file path.""" system = platform.system() @@ -65,7 +128,7 @@ def get_claude_config_path() -> Path: / "claude_desktop_config.json" ) elif system == "Windows": - return Path(os.environ["APPDATA"]) / "Claude" / "claude_desktop_config.json" + return _resolve_windows_appdata() / "Claude" / "claude_desktop_config.json" else: # Linux # Check if we're in WSL - if so, use Windows path if is_wsl(): @@ -90,7 +153,11 @@ def get_cursor_config_path() -> Path: if system == "Darwin": # macOS return Path.home() / ".cursor" / "mcp.json" elif system == "Windows": - return Path(os.environ["APPDATA"]) / "Cursor" / "mcp.json" + candidates = _get_windows_cursor_config_paths() + for path in candidates: + if path.exists(): + return path + return candidates[0] else: # Linux # Check if we're in WSL - if so, use Windows path if is_wsl(): @@ -114,7 +181,7 @@ def get_vscode_config_path() -> Path: if system == "Darwin": # macOS return Path.home() / "Library" / "Application Support" / "Code" / "User" / "mcp.json" elif system == "Windows": - return Path(os.environ["APPDATA"]) / "Code" / "User" / "mcp.json" + return _resolve_windows_appdata() / "Code" / "User" / "mcp.json" else: # Linux # Check if we're in WSL - if so, use Windows path if is_wsl(): @@ -179,9 +246,12 @@ def get_stdio_config(entrypoint_file: str, server_name: str) -> dict: """Get the appropriate stdio configuration based on whether uv is installed.""" server_file = Path.cwd() / entrypoint_file - if is_uv_installed(): + uv_executable = shutil.which("uv") + if uv_executable: return { - "command": "uv", + # Use the absolute uv path so GUI clients can launch reliably even + # when they were started with a different PATH than the shell. + "command": uv_executable, "args": [ "run", "--directory", @@ -218,27 +288,32 @@ def configure_claude_local( # Load existing config or create new one config = {} if config_path.exists(): - with open(config_path) as f: + with open(config_path, encoding="utf-8") as f: config = json.load(f) # Add or update MCP servers configuration if "mcpServers" not in config: config["mcpServers"] = {} + _warn_overwrite(config, "mcpServers", server_name, config_path) + # Claude Desktop uses stdio transport config["mcpServers"][server_name] = get_stdio_config(entrypoint_file, server_name) # Write updated config - with open(config_path, "w") as f: + with open(config_path, "w", encoding="utf-8") as f: json.dump(config, f, indent=2) console.print( f"✅ Configured Claude Desktop by adding local MCP server '{server_name}' to the configuration", style="green", ) - config_file_path = config_path.as_posix().replace(" ", "\\ ") + config_file_path = _format_path_for_display(config_path) console.print(f" MCP client config file: {config_file_path}", style="dim") - console.print(f" Server file: {Path.cwd() / entrypoint_file}", style="dim") + console.print( + f" Server file: {_format_path_for_display(Path.cwd() / entrypoint_file)}", + style="dim", + ) if is_uv_installed(): console.print(" Using uv to run server", style="dim") else: @@ -271,40 +346,64 @@ def configure_cursor_local( "url": f"http://localhost:{port}/mcp", } - config_path = config_path or get_cursor_config_path() + if config_path is not None: + target_paths = [config_path] + elif platform.system() == "Windows": + primary_path = get_cursor_config_path() + target_paths = _dedupe_paths([primary_path, *_get_windows_cursor_config_paths()]) + else: + target_paths = [get_cursor_config_path()] - # Handle both absolute and relative config paths - if config_path and not config_path.is_absolute(): - config_path = Path.cwd() / config_path + # Handle both absolute and relative config paths. + resolved_target_paths: list[Path] = [] + for path in target_paths: + resolved_target_paths.append(path if path.is_absolute() else Path.cwd() / path) - config_path.parent.mkdir(parents=True, exist_ok=True) - - # Load existing config or create new one - config = {} - if config_path.exists(): - with open(config_path) as f: - config = json.load(f) - - # Add or update MCP servers configuration - if "mcpServers" not in config: - config["mcpServers"] = {} - - config["mcpServers"][server_name] = ( + server_config = ( get_stdio_config(entrypoint_file, server_name) if transport == "stdio" else http_config(server_name, port) ) - # Write updated config - with open(config_path, "w") as f: - json.dump(config, f, indent=2) + for idx, config_path in enumerate(resolved_target_paths): + config_path.parent.mkdir(parents=True, exist_ok=True) + + # Load existing config or create new one + config = {} + if config_path.exists(): + with open(config_path, encoding="utf-8") as f: + config = json.load(f) + + # Add or update MCP servers configuration + if "mcpServers" not in config: + config["mcpServers"] = {} + + if idx == 0: + _warn_overwrite(config, "mcpServers", server_name, config_path) + + config["mcpServers"][server_name] = server_config + + # Write updated config + with open(config_path, "w", encoding="utf-8") as f: + json.dump(config, f, indent=2) + + primary_config_path = resolved_target_paths[0] console.print( f"✅ Configured Cursor by adding local MCP server '{server_name}' to the configuration", style="green", ) - config_file_path = config_path.as_posix().replace(" ", "\\ ") + config_file_path = _format_path_for_display(primary_config_path) console.print(f" MCP client config file: {config_file_path}", style="dim") + compatibility_paths = resolved_target_paths[1:] + if compatibility_paths: + compatibility_display = ", ".join( + _format_path_for_display(path) for path in compatibility_paths + ) + console.print( + f" Also updated compatibility config file(s): {compatibility_display}", + style="dim", + ) if transport == "http": console.print(f" MCP Server URL: http://localhost:{port}/mcp", style="dim") elif transport == "stdio": @@ -347,12 +446,12 @@ def configure_vscode_local( # Load existing config or create new one config = {} if config_path.exists(): - with open(config_path) as f: + with open(config_path, encoding="utf-8") as f: try: config = json.load(f) except json.JSONDecodeError as e: raise ValueError( - f"\n\tFailed to load MCP configuration file at {config_path.as_posix()} " + f"\n\tFailed to load MCP configuration file at {_format_path_for_display(config_path)} " f"\n\tThe file contains invalid JSON: {e}. " "\n\tPlease check the file format or delete it to create a new configuration." ) @@ -361,6 +460,8 @@ def configure_vscode_local( if "servers" not in config: config["servers"] = {} + _warn_overwrite(config, "servers", server_name, config_path) + config["servers"][server_name] = ( get_stdio_config(entrypoint_file, server_name) if transport == "stdio" @@ -368,14 +469,14 @@ def configure_vscode_local( ) # Write updated config - with open(config_path, "w") as f: + with open(config_path, "w", encoding="utf-8") as f: json.dump(config, f, indent=2) console.print( f"✅ Configured VS Code by adding local MCP server '{server_name}' to the configuration", style="green", ) - config_file_path = config_path.as_posix().replace(" ", "\\ ") + config_file_path = _format_path_for_display(config_path) console.print(f" MCP client config file: {config_file_path}", style="dim") if transport == "http": console.print(f" MCP Server URL: http://localhost:{port}/mcp", style="dim") diff --git a/libs/arcade-cli/arcade_cli/console.py b/libs/arcade-cli/arcade_cli/console.py new file mode 100644 index 00000000..2682ff10 --- /dev/null +++ b/libs/arcade-cli/arcade_cli/console.py @@ -0,0 +1,43 @@ +"""Shared console setup for Arcade CLI output.""" + +from __future__ import annotations + +import os +import sys + +from rich.console import Console + + +def _needs_utf8(encoding: str | None) -> bool: + if not encoding: + return True + return encoding.lower() not in {"utf-8", "utf8"} + + +def _configure_windows_utf8() -> None: + """Ensure Windows console encoding won't raise UnicodeEncodeError.""" + if sys.platform != "win32": + return + + needs_utf8 = _needs_utf8(getattr(sys.stdout, "encoding", None)) or _needs_utf8( + getattr(sys.stderr, "encoding", None) + ) + if not needs_utf8: + return + + try: + if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8", errors="replace") + if hasattr(sys.stderr, "reconfigure"): + sys.stderr.reconfigure(encoding="utf-8", errors="replace") + except Exception: # noqa: S110 + # Fall back to environment hint for child processes. + pass + + os.environ.setdefault("PYTHONIOENCODING", "utf-8") + + +_configure_windows_utf8() + +# Shared console used across CLI modules. +console = Console() diff --git a/libs/arcade-cli/arcade_cli/deploy.py b/libs/arcade-cli/arcade_cli/deploy.py index 7b8308d5..715bf4d3 100644 --- a/libs/arcade-cli/arcade_cli/deploy.py +++ b/libs/arcade-cli/arcade_cli/deploy.py @@ -1,6 +1,7 @@ import asyncio import base64 import io +import logging import os import random import subprocess @@ -11,10 +12,14 @@ from pathlib import Path from typing import cast import httpx +from arcade_core.subprocess_utils import ( + get_windows_no_window_creationflags, + graceful_terminate_process, +) from dotenv import load_dotenv from pydantic import BaseModel, Field from rich.columns import Columns -from rich.console import Console, Group +from rich.console import Group from rich.live import Live from rich.prompt import Confirm from rich.spinner import Spinner @@ -22,6 +27,7 @@ from rich.text import Text from typing_extensions import Literal from arcade_cli.configure import find_python_interpreter +from arcade_cli.console import console from arcade_cli.secret import load_env_file from arcade_cli.utils import ( compute_base_url, @@ -30,7 +36,8 @@ from arcade_cli.utils import ( validate_and_get_config, ) -console = Console() +logger = logging.getLogger(__name__) + # Models @@ -362,6 +369,37 @@ def create_package_archive(package_dir: Path) -> str: return package_bytes_b64 +def _graceful_terminate(process: subprocess.Popen) -> None: + """Terminate a subprocess using shared graceful shutdown semantics.""" + graceful_terminate_process(process) + + +def _resolve_server_process_stdio(debug: bool) -> tuple[int | None, int | None]: + """Choose stdout/stderr targets for the temporary validation server process. + + ``arcade deploy`` starts a short-lived child server to validate the + entrypoint before uploading. The child's stdout/stderr must be handled + carefully: + + * **Normal mode** (``debug=False``): the CLI doesn't display child output, + so both streams are sent to ``subprocess.DEVNULL``. This prevents a + chatty child from filling the OS pipe buffer and blocking — which + manifests as intermittent health-check timeouts, especially on Windows + where the default pipe buffer is only 4 KiB. + * **Debug mode** (``debug=True``): both streams are inherited from the + parent process (``None``), so the user sees live startup logs in their + terminal for troubleshooting. + + Returns: + ``(stdout_target, stderr_target)`` — each is either + ``subprocess.DEVNULL`` or ``None`` (inherit). + """ + if debug: + return None, None + + return subprocess.DEVNULL, subprocess.DEVNULL + + def start_server_process(entrypoint: str, debug: bool = False) -> tuple[subprocess.Popen, int]: """ Start the MCP server process on a random port. @@ -395,25 +433,39 @@ def start_server_process(entrypoint: str, debug: bool = False) -> tuple[subproce project_python = find_python_interpreter() cmd = [str(project_python), entrypoint] + creationflags = get_windows_no_window_creationflags(new_process_group=True) + + stdout_target, stderr_target = _resolve_server_process_stdio(debug) + process = subprocess.Popen( cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stdout=stdout_target, + stderr=stderr_target, text=True, env=env, + creationflags=creationflags, ) - # Check for immediate failure on start up + # Check for immediate failure on startup. + # stdout/stderr are either DEVNULL (non-debug) or inherited (debug), so + # communicate() returns (None, None) in both cases — there is nothing to + # capture. Surface a context-appropriate hint to the user instead. time.sleep(0.5) if process.poll() is not None: - _, stderr = process.communicate() - error_msg = stderr.strip() if stderr else "Unknown error" - raise ValueError(f"Server process exited immediately: {error_msg}") + if debug: + raise ValueError( + "Server process exited immediately. " "Check the server output above for details." + ) + raise ValueError( + "Server process exited immediately. " "Re-run with --debug to see server startup logs." + ) return process, port -def wait_for_health(base_url: str, process: subprocess.Popen, timeout: int = 30) -> None: +def wait_for_health( + base_url: str, process: subprocess.Popen, timeout: int = 30, debug: bool = False +) -> None: """ Wait for the server to become healthy. @@ -421,6 +473,7 @@ def wait_for_health(base_url: str, process: subprocess.Popen, timeout: int = 30) base_url: Base URL of the server process: The server process timeout: Maximum time to wait in seconds + debug: Whether debug mode is active (affects the hint in the error message) Raises: ValueError: If the server doesn't become healthy within timeout @@ -442,13 +495,24 @@ def wait_for_health(base_url: str, process: subprocess.Popen, timeout: int = 30) time.sleep(0.5) if not is_healthy: - process.terminate() + _graceful_terminate(process) try: - _, stderr = process.communicate(timeout=2) - error_msg = stderr.strip() if stderr else "Server failed to become healthy" + process.communicate(timeout=2) except subprocess.TimeoutExpired: process.kill() - error_msg = f"Server failed to become healthy within {timeout} seconds" + + # stdout/stderr are DEVNULL (non-debug) or inherited (debug), so + # communicate() never captures output — build a context-appropriate message. + if debug: + error_msg = ( + f"Server failed to become healthy within {timeout} seconds. " + "Check the server output above for details." + ) + else: + error_msg = ( + f"Server failed to become healthy within {timeout} seconds. " + "Re-run with --debug to see server startup logs." + ) raise ValueError(error_msg) console.print("✓ Server is healthy", style="green") @@ -575,7 +639,7 @@ def verify_server_and_get_metadata( base_url = f"http://127.0.0.1:{port}" try: - wait_for_health(base_url, process) + wait_for_health(base_url, process, debug=debug) server_name, server_version = get_server_info(base_url) @@ -586,7 +650,7 @@ def verify_server_and_get_metadata( finally: # Always stop the server - process.terminate() + _graceful_terminate(process) try: process.wait(timeout=5) except subprocess.TimeoutExpired: diff --git a/libs/arcade-cli/arcade_cli/display.py b/libs/arcade-cli/arcade_cli/display.py index bfc760fe..27a6a355 100644 --- a/libs/arcade-cli/arcade_cli/display.py +++ b/libs/arcade-cli/arcade_cli/display.py @@ -7,9 +7,10 @@ from rich.panel import Panel from rich.table import Table from rich.text import Text +from arcade_cli.console import console + if TYPE_CHECKING: from arcade_evals.eval import EvaluationResult -console = Console() def display_tools_table(tools: list[ToolDefinition]) -> None: diff --git a/libs/arcade-cli/arcade_cli/main.py b/libs/arcade-cli/arcade_cli/main.py index d39413c7..48da7108 100644 --- a/libs/arcade-cli/arcade_cli/main.py +++ b/libs/arcade-cli/arcade_cli/main.py @@ -2,24 +2,26 @@ import asyncio import os import subprocess import sys -import webbrowser from pathlib import Path from typing import Optional import click import typer from arcade_core.constants import CREDENTIALS_FILE_PATH, PROD_COORDINATOR_HOST, PROD_ENGINE_HOST +from arcade_core.subprocess_utils import get_windows_no_window_creationflags from arcadepy import Arcade -from rich.console import Console from arcade_cli.authn import ( + DEFAULT_OAUTH_TIMEOUT_SECONDS, OAuthLoginError, _credentials_file_contains_legacy, + _open_browser, build_coordinator_url, check_existing_login, perform_oauth_login, save_credentials_from_whoami, ) +from arcade_cli.console import console from arcade_cli.evals_runner import run_capture, run_evaluations from arcade_cli.org import app as org_app from arcade_cli.project import app as project_app @@ -73,9 +75,6 @@ cli.add_typer( ) -console = Console() - - @cli.command(help="Log in to Arcade", rich_help_panel="User") def login( host: str = typer.Option( @@ -90,6 +89,11 @@ def login( "--port", help="The port of the Arcade Coordinator host (if running locally).", ), + timeout: int = typer.Option( + DEFAULT_OAUTH_TIMEOUT_SECONDS, + "--timeout", + help="Seconds to wait for the local login callback.", + ), debug: bool = typer.Option(False, "--debug", "-d", help="Show debug information"), ) -> None: """ @@ -107,6 +111,7 @@ def login( result = perform_oauth_login( coordinator_url, on_status=lambda msg: console.print(msg, style="dim"), + callback_timeout_seconds=timeout, ) # Save credentials @@ -127,7 +132,7 @@ def login( except OAuthLoginError as e: if debug: console.print(f"Debug: {e.__cause__}", style="dim") - handle_cli_error(str(e), should_exit=False) + handle_cli_error(str(e), should_exit=True) except KeyboardInterrupt: console.print("\nLogin cancelled.", style="yellow") except Exception as e: @@ -148,6 +153,13 @@ def logout( console.print("You're now logged out.", style="bold") else: console.print("You're not logged in.", style="bold red") + except PermissionError: + # On Windows, the file may be locked by another process. + handle_cli_error( + "Could not remove credentials file — it may be in use by another process. " + "Close other Arcade instances and try again.", + should_exit=True, + ) except Exception as e: handle_cli_error("Logout failed", e, debug) @@ -163,8 +175,11 @@ def whoami( try: config = Config.load_from_file() + except FileNotFoundError: + console.print("Not logged in. Run 'arcade login' to authenticate.", style="bold red") + return except Exception as e: - handle_cli_error("Failed to read credentials", e, debug) + handle_cli_error("Failed to read credentials", e, debug, should_exit=True) return # Defensive - should not happen, because the main() callback prevents this: @@ -311,8 +326,16 @@ def mcp( if debug: console.print(f"[dim]Running: {' '.join(cmd)}[/dim]") - # Execute the command and pass through all output - result = subprocess.run(cmd, check=False) + # Execute the command and pass through all output. + # On Windows, set CREATE_NO_WINDOW to prevent a phantom console + # window from appearing (e.g. when an MCP client spawns this + # command without an attached console). The child process still + # inherits stdin/stdout/stderr for stdio transport communication. + run_kwargs: dict[str, object] = {"check": False} + creation_flags = get_windows_no_window_creationflags() + if creation_flags: + run_kwargs["creationflags"] = creation_flags + result = subprocess.run(cmd, **run_kwargs) # Exit with the same code as the subprocess if result.returncode != 0: @@ -918,7 +941,7 @@ def dashboard( # Open the dashboard in a browser console.print(f"Opening Arcade Dashboard at {dashboard_url}") - if not webbrowser.open(dashboard_url): + if not _open_browser(dashboard_url): console.print( f"If a browser doesn't open automatically, copy this URL and paste it into your browser: {dashboard_url}", style="dim", diff --git a/libs/arcade-cli/arcade_cli/new.py b/libs/arcade-cli/arcade_cli/new.py index 9da414ec..486ead2f 100644 --- a/libs/arcade-cli/arcade_cli/new.py +++ b/libs/arcade-cli/arcade_cli/new.py @@ -7,12 +7,10 @@ from typing import Optional import typer from jinja2 import Environment, FileSystemLoader, select_autoescape -from rich.console import Console +from arcade_cli.console import console from arcade_cli.templates import get_full_template_directory, get_minimal_template_directory -console = Console() - # Retrieve the installed version of arcade-mcp try: ARCADE_MCP_MIN_VERSION = get_version("arcade-mcp") @@ -144,7 +142,11 @@ def remove_toolkit(toolkit_directory: Path, toolkit_name: str) -> None: """Teardown logic for when creating a new toolkit fails.""" toolkit_path = toolkit_directory / toolkit_name if toolkit_path.exists(): - shutil.rmtree(toolkit_path) + try: + shutil.rmtree(toolkit_path) + except (PermissionError, OSError) as e: + # On Windows, files may still be locked by another process. + console.print(f"[yellow]Warning: Could not fully remove '{toolkit_path}': {e}[/yellow]") def create_new_toolkit(output_directory: str, toolkit_name: str) -> None: @@ -224,6 +226,15 @@ def create_new_toolkit(output_directory: str, toolkit_name: str) -> None: console.print( f"[green]Toolkit '{toolkit_name}' created successfully at '{toolkit_directory}'.[/green]" ) + console.print("\nNext steps:", style="bold") + console.print(f" cd {toolkit_directory / toolkit_name}") + console.print("") + console.print(" Run with stdio transport (for MCP clients):", style="dim") + console.print(" uv run server.py") + console.print("") + console.print(" Run with HTTP transport (for development/testing):", style="dim") + console.print(" uv run server.py --transport http --port 8000") + console.print("") create_deployment(toolkit_directory, toolkit_name) except Exception: remove_toolkit(toolkit_directory, toolkit_name) @@ -271,9 +282,20 @@ def create_new_toolkit_minimal(output_directory: str, toolkit_name: str) -> None try: create_package(env, template_directory, toolkit_directory, context, ignore_pattern) + console.print("") console.print( f"[green]Server '{toolkit_name}' created successfully at '{toolkit_directory}'.[/green]" ) + server_dir = toolkit_directory / toolkit_name / "src" / toolkit_name + console.print("\nNext steps:", style="bold") + console.print(f" cd {server_dir}") + console.print("") + console.print(" Run with stdio transport (for MCP clients):", style="dim") + console.print(" uv run server.py") + console.print("") + console.print(" Run with HTTP transport (for development/testing):", style="dim") + console.print(" uv run server.py --transport http --port 8000") + console.print("") except Exception: remove_toolkit(toolkit_directory, toolkit_name) raise diff --git a/libs/arcade-cli/arcade_cli/org.py b/libs/arcade-cli/arcade_cli/org.py index fd870fdf..bd09cba8 100644 --- a/libs/arcade-cli/arcade_cli/org.py +++ b/libs/arcade-cli/arcade_cli/org.py @@ -1,21 +1,18 @@ import typer from arcade_core.constants import PROD_COORDINATOR_HOST -from rich.console import Console from arcade_cli.authn import ( fetch_organizations, fetch_projects, select_default_project, ) +from arcade_cli.console import console from arcade_cli.usage.command_tracker import TrackedTyper, TrackedTyperGroup from arcade_cli.utils import ( compute_base_url, handle_cli_error, ) -console = Console() - - app = TrackedTyper( cls=TrackedTyperGroup, add_completion=False, diff --git a/libs/arcade-cli/arcade_cli/project.py b/libs/arcade-cli/arcade_cli/project.py index 2104e8c9..cc064088 100644 --- a/libs/arcade-cli/arcade_cli/project.py +++ b/libs/arcade-cli/arcade_cli/project.py @@ -1,17 +1,14 @@ import typer from arcade_core.constants import PROD_COORDINATOR_HOST -from rich.console import Console from arcade_cli.authn import fetch_projects +from arcade_cli.console import console from arcade_cli.usage.command_tracker import TrackedTyper, TrackedTyperGroup from arcade_cli.utils import ( compute_base_url, handle_cli_error, ) -console = Console() - - app = TrackedTyper( cls=TrackedTyperGroup, add_completion=False, diff --git a/libs/arcade-cli/arcade_cli/secret.py b/libs/arcade-cli/arcade_cli/secret.py index 23dadea4..d88ec1ea 100644 --- a/libs/arcade-cli/arcade_cli/secret.py +++ b/libs/arcade-cli/arcade_cli/secret.py @@ -1,9 +1,9 @@ import httpx import typer from arcade_core.constants import PROD_ENGINE_HOST -from rich.console import Console from rich.table import Table +from arcade_cli.console import console from arcade_cli.usage.command_tracker import TrackedTyper, TrackedTyperGroup from arcade_cli.utils import ( compute_base_url, @@ -11,9 +11,6 @@ from arcade_cli.utils import ( get_org_scoped_url, ) -console = Console() - - app = TrackedTyper( cls=TrackedTyperGroup, add_completion=False, @@ -182,7 +179,7 @@ def print_secret_table(secrets: list[dict]) -> None: def load_env_file(env_file_path: str) -> dict[str, str]: """Load tool secrets from a .env file.""" secrets = {} - with open(env_file_path) as file: + with open(env_file_path, encoding="utf-8") as file: for line in file: line = line.strip() if line.startswith("#") or not line: diff --git a/libs/arcade-cli/arcade_cli/server.py b/libs/arcade-cli/arcade_cli/server.py index 592c48a4..03fe900e 100644 --- a/libs/arcade-cli/arcade_cli/server.py +++ b/libs/arcade-cli/arcade_cli/server.py @@ -10,9 +10,9 @@ from arcade_core.constants import PROD_ENGINE_HOST from arcadepy import NotFoundError from arcadepy.types import WorkerHealthResponse, WorkerResponse from dateutil import parser -from rich.console import Console from rich.table import Table +from arcade_cli.console import console from arcade_cli.usage.command_tracker import TrackedTyper, TrackedTyperGroup from arcade_cli.utils import ( compute_base_url, @@ -22,8 +22,6 @@ from arcade_cli.utils import ( handle_cli_error, ) -console = Console() - def _format_timestamp_to_local(timestamp_str: str) -> str: """ @@ -325,7 +323,7 @@ def _display_deployment_logs( logs = response.json() for log in logs: formatted_timestamp = _format_timestamp_to_local(log["timestamp"]) - print(f"[{formatted_timestamp}] {log['line']}") + console.print(f"[{formatted_timestamp}] {log['line']}", markup=False) except httpx.HTTPStatusError as e: handle_cli_error( f"Failed to fetch logs: {e.response.status_code} {e.response.text}", debug=debug @@ -359,11 +357,11 @@ async def _stream_deployment_logs( timestamp_str = data.get("Timestamp", "") log_line = data.get("Line", "") formatted_timestamp = _format_timestamp_to_local(timestamp_str) - print(f"[{formatted_timestamp}] {log_line}") + console.print(f"[{formatted_timestamp}] {log_line}", markup=False) except (json.JSONDecodeError, KeyError, IndexError): - print(line) + console.print(line, markup=False) else: - print(line) + console.print(line, markup=False) except httpx.HTTPStatusError as e: handle_cli_error(f"Failed to stream logs: {e.response.status_code}", debug=debug) except Exception as e: diff --git a/libs/arcade-cli/arcade_cli/usage/command_tracker.py b/libs/arcade-cli/arcade_cli/usage/command_tracker.py index ba5633cc..715d09f7 100644 --- a/libs/arcade-cli/arcade_cli/usage/command_tracker.py +++ b/libs/arcade-cli/arcade_cli/usage/command_tracker.py @@ -7,6 +7,7 @@ from importlib import metadata from typing import Any import typer +from arcade_cli.console import console from arcade_cli.usage.constants import ( EVENT_CLI_COMMAND_EXECUTED, EVENT_CLI_COMMAND_FAILED, @@ -27,12 +28,9 @@ from arcade_core.usage.constants import ( PROP_RUNTIME_LANGUAGE, PROP_RUNTIME_VERSION, ) -from rich.console import Console from typer.core import TyperCommand, TyperGroup from typer.models import Context -console = Console() - class CommandTracker: """Tracks CLI command execution for usage analytics.""" diff --git a/libs/arcade-cli/arcade_cli/utils.py b/libs/arcade-cli/arcade_cli/utils.py index 47914fc1..f3ea6fd5 100644 --- a/libs/arcade-cli/arcade_cli/utils.py +++ b/libs/arcade-cli/arcade_cli/utils.py @@ -35,13 +35,11 @@ from arcadepy import ( ) from arcadepy.types import AuthorizationResponse from pydantic import ValidationError -from rich.console import Console from rich.markup import escape from typer.core import TyperGroup from typer.models import Context -console = Console() - +from arcade_cli.console import console # ----------------------------------------------------------------------------- # Shared helpers for the CLI @@ -1084,7 +1082,7 @@ def load_dotenv(path: str | Path, *, override: bool = False) -> dict[str, str]: loaded: dict[str, str] = {} - for raw in path.read_text().splitlines(): + for raw in path.read_text(encoding="utf-8").splitlines(): parsed = _parse_line(raw.strip()) if parsed is None: continue diff --git a/libs/arcade-core/arcade_core/config_model.py b/libs/arcade-core/arcade_core/config_model.py index 7080037c..e6ab32ec 100644 --- a/libs/arcade-core/arcade_core/config_model.py +++ b/libs/arcade-core/arcade_core/config_model.py @@ -1,4 +1,6 @@ +import logging import os +import subprocess from datetime import datetime, timedelta from pathlib import Path from typing import Any @@ -6,6 +8,54 @@ from typing import Any import yaml from pydantic import BaseModel, ConfigDict, ValidationError +logger = logging.getLogger(__name__) + + +def _set_windows_owner_acl(config_file_path: Path) -> None: + """Restrict a file so only the current user can read/write it on Windows. + + On POSIX systems ``chmod 600`` removes group/other access. On Windows, + ``Path.chmod()`` only toggles the read-only flag and does **not** change + who can access the file. To get equivalent protection we use the built-in + ``icacls`` command to manipulate NTFS Access Control Lists (ACLs): + + 1. ``/inheritance:r`` — remove all inherited Access Control Entries (ACEs). + By default every file inherits broad permissions from its parent folder + (e.g. ``Users:(RX)``). Stripping inheritance leaves the file with an + empty ACL, meaning *no one* can access it yet. + 2. ``/grant:r USERNAME:(R,W)`` — add a single explicit ACE that grants + the current user Read and Write access. The ``:r`` flag replaces any + existing ACE for that user rather than merging. + + Both flags are passed in a **single** ``icacls`` invocation so there is no + window where the file has an empty ACL (which would make it temporarily + inaccessible to everyone, including the owner). + + The net effect is that only the logged-in Windows user can read or modify + the credentials file — the same security posture as ``chmod 600`` on Unix. + """ + username = os.environ.get("USERNAME") + if not username: + raise OSError("USERNAME is not set; cannot apply Windows ACL restrictions") + + # Strip inherited permissions and grant only the current user R+W access in + # a single icacls call. Using two separate calls would leave the file with + # an empty ACL (nobody can access it) between the first and second call; if + # the second call were to fail the file would be permanently inaccessible. + subprocess.run( + [ # noqa: S607 + "icacls", + str(config_file_path), + "/inheritance:r", + "/grant:r", + f"{username}:(R,W)", + ], + check=True, + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + text=True, + ) + class BaseConfig(BaseModel): model_config = ConfigDict(extra="ignore") @@ -182,7 +232,7 @@ class Config(BaseConfig): "Please run 'arcade login' to create your configuration." ) - config_data = yaml.safe_load(config_file_path.read_text()) + config_data = yaml.safe_load(config_file_path.read_text(encoding="utf-8")) if config_data is None: raise ValueError( @@ -230,7 +280,22 @@ class Config(BaseConfig): # Convert to dict, excluding None values for cleaner output data = {"cloud": self.model_dump(exclude_none=True, mode="json")} - config_file_path.write_text(yaml.dump(data, default_flow_style=False)) + config_file_path.write_text(yaml.dump(data, default_flow_style=False), encoding="utf-8") - # Set restrictive permissions (owner read/write only) - config_file_path.chmod(0o600) + # Restrict the credentials file so only the current user can read it. + # - Unix: chmod 600 (removes group/other access via file-mode bits). + # - Windows: icacls to strip inherited ACEs and grant only the current + # user R/W access (see _set_windows_owner_acl for details). + # Failure is non-fatal: the file is still written, but a warning is + # logged so the user knows the permissions could not be tightened. + try: + if os.name == "nt": + _set_windows_owner_acl(config_file_path) + else: + config_file_path.chmod(0o600) + except (OSError, subprocess.SubprocessError) as exc: + logger.warning( + "Unable to apply restrictive permissions to %s: %s", + config_file_path, + exc, + ) diff --git a/libs/arcade-core/arcade_core/parse.py b/libs/arcade-core/arcade_core/parse.py index eca54707..32af9080 100644 --- a/libs/arcade-core/arcade_core/parse.py +++ b/libs/arcade-core/arcade_core/parse.py @@ -8,7 +8,7 @@ def load_ast_tree(filepath: str | Path) -> ast.AST: """ try: - with open(filepath) as file: + with open(filepath, encoding="utf-8") as file: return ast.parse(file.read(), filename=filepath) except FileNotFoundError: raise FileNotFoundError(f"File {filepath} not found") diff --git a/libs/arcade-core/arcade_core/subprocess_utils.py b/libs/arcade-core/arcade_core/subprocess_utils.py new file mode 100644 index 00000000..d3bb512a --- /dev/null +++ b/libs/arcade-core/arcade_core/subprocess_utils.py @@ -0,0 +1,68 @@ +"""Shared Windows subprocess helpers used across Arcade libraries.""" + +from __future__ import annotations + +import contextlib +import signal +import subprocess +import sys +from typing import Any + + +def get_windows_no_window_creationflags(*, new_process_group: bool = False) -> int: + """Return Windows creation flags to suppress phantom console windows. + + Args: + new_process_group: When true, include ``CREATE_NEW_PROCESS_GROUP`` to + allow graceful ``CTRL_BREAK_EVENT`` signaling. + + Returns: + A bitmask of subprocess creation flags on Windows, otherwise ``0``. + """ + if sys.platform != "win32": + return 0 + + flags = getattr(subprocess, "CREATE_NO_WINDOW", 0x08000000) + if new_process_group: + flags |= getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200) + return flags + + +def build_windows_hidden_startupinfo() -> Any | None: + """Create a Windows ``STARTUPINFO`` configured with ``SW_HIDE``. + + Returns: + A configured ``STARTUPINFO`` instance on Windows when available, + otherwise ``None``. + """ + if sys.platform != "win32": + return None + + startupinfo_cls = getattr(subprocess, "STARTUPINFO", None) + if startupinfo_cls is None: + return None + + startupinfo = startupinfo_cls() + startupinfo.dwFlags |= getattr(subprocess, "STARTF_USESHOWWINDOW", 0x00000001) + startupinfo.wShowWindow = 0 + return startupinfo + + +def graceful_terminate_process(process: subprocess.Popen[Any]) -> None: + """Terminate a process with Windows-friendly graceful fallback behavior. + + On Windows, try ``CTRL_BREAK_EVENT`` first (when supported) so child + processes can exit cleanly. If signaling fails, fall back to + ``process.terminate()``. Any ``OSError`` during termination is swallowed + because the process may already have exited. + """ + if sys.platform == "win32": + try: + process.send_signal(signal.CTRL_BREAK_EVENT) + except (OSError, AttributeError): + pass + else: + return + + with contextlib.suppress(OSError): + process.terminate() diff --git a/libs/arcade-core/arcade_core/toolkit.py b/libs/arcade-core/arcade_core/toolkit.py index 426f84e1..ae0c4a97 100644 --- a/libs/arcade-core/arcade_core/toolkit.py +++ b/libs/arcade-core/arcade_core/toolkit.py @@ -72,7 +72,7 @@ class Toolkit(BaseModel): raise ToolkitLoadError(f"pyproject.toml not found in {directory}") try: - with open(pyproject_path) as f: + with open(pyproject_path, encoding="utf-8") as f: pyproject_data = toml.load(f) project_data = pyproject_data.get("project", {}) diff --git a/libs/arcade-core/arcade_core/usage/identity.py b/libs/arcade-core/arcade_core/usage/identity.py index 99d6b233..4a34b8eb 100644 --- a/libs/arcade-core/arcade_core/usage/identity.py +++ b/libs/arcade-core/arcade_core/usage/identity.py @@ -45,7 +45,7 @@ class UsageIdentity: if os.path.exists(self.usage_file_path): try: - with open(self.usage_file_path) as f: + with open(self.usage_file_path, encoding="utf-8") as f: # Lock file for reading (shared lock) portalocker.lock(f, portalocker.LOCK_SH) try: @@ -77,7 +77,7 @@ class UsageIdentity: ) try: - with os.fdopen(temp_fd, "w") as f: + with os.fdopen(temp_fd, "w", encoding="utf-8") as f: # Lock file for writing (exclusive lock) portalocker.lock(f, portalocker.LOCK_EX) try: @@ -141,7 +141,7 @@ class UsageIdentity: return None try: - with open(CREDENTIALS_FILE_PATH) as f: + with open(CREDENTIALS_FILE_PATH, encoding="utf-8") as f: config = yaml.safe_load(f) or {} cloud_config = config.get("cloud", {}) if isinstance(config, dict) else {} diff --git a/libs/arcade-core/arcade_core/usage/usage_service.py b/libs/arcade-core/arcade_core/usage/usage_service.py index 06f2f8f0..ed1a01e3 100644 --- a/libs/arcade-core/arcade_core/usage/usage_service.py +++ b/libs/arcade-core/arcade_core/usage/usage_service.py @@ -1,8 +1,14 @@ import json import os +import shutil import subprocess import sys +from pathlib import Path +from arcade_core.subprocess_utils import ( + build_windows_hidden_startupinfo, + get_windows_no_window_creationflags, +) from arcade_core.usage.constants import ( ARCADE_USAGE_EVENT_DATA, MAX_RETRIES_POSTHOG, @@ -71,23 +77,28 @@ class UsageService: "is_anon": is_anon, }) - cmd = [sys.executable, "-m", "arcade_core.usage"] + cmd_executable = _resolve_background_python_executable() + + cmd = [cmd_executable, "-m", "arcade_core.usage"] # Pass data via environment variable (works on all platforms) env = os.environ.copy() env[ARCADE_USAGE_EVENT_DATA] = event_data if sys.platform == "win32": - # Windows: Use DETACHED_PROCESS to fully detach from parent console - DETACHED_PROCESS = 0x00000008 - CREATE_NEW_PROCESS_GROUP = 0x00000200 + # Windows: use CREATE_NO_WINDOW + SW_HIDE so the tracking worker + # never flashes a console window. CREATE_NEW_PROCESS_GROUP keeps + # it isolated from Ctrl+C signals sent to the parent group. + creationflags = get_windows_no_window_creationflags(new_process_group=True) + startupinfo = build_windows_hidden_startupinfo() subprocess.Popen( cmd, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - creationflags=DETACHED_PROCESS | CREATE_NEW_PROCESS_GROUP, + creationflags=creationflags, + startupinfo=startupinfo, close_fds=True, env=env, ) @@ -102,3 +113,33 @@ class UsageService: close_fds=True, env=env, ) + + +def _resolve_background_python_executable() -> str: + """Resolve the best interpreter for detached usage tracking.""" + if sys.platform != "win32": + return sys.executable + + # Prefer a windowless interpreter on Windows to avoid flashing a console + # for short-lived tracking subprocesses. + candidates: list[Path] = [] + candidates.append(Path(sys.executable).with_name("pythonw.exe")) + + base_prefix = getattr(sys, "base_prefix", "") + if base_prefix: + candidates.append(Path(base_prefix) / "pythonw.exe") + + which_pythonw = shutil.which("pythonw") + if which_pythonw: + candidates.append(Path(which_pythonw)) + + seen: set[str] = set() + for candidate in candidates: + key = str(candidate).lower() + if key in seen: + continue + seen.add(key) + if candidate.exists(): + return str(candidate) + + return sys.executable diff --git a/libs/arcade-core/pyproject.toml b/libs/arcade-core/pyproject.toml index 16f640ff..5264d3a4 100644 --- a/libs/arcade-core/pyproject.toml +++ b/libs/arcade-core/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "arcade-core" -version = "4.4.0" +version = "4.4.2" description = "Arcade Core - Core library for Arcade platform" readme = "README.md" license = { text = "MIT" } diff --git a/libs/arcade-evals/arcade_evals/capture.py b/libs/arcade-evals/arcade_evals/capture.py index 19b98033..3145b7aa 100644 --- a/libs/arcade-evals/arcade_evals/capture.py +++ b/libs/arcade-evals/arcade_evals/capture.py @@ -171,7 +171,7 @@ class CaptureResult: def write_to_file(self, file_path: str, include_context: bool = False, indent: int = 2) -> None: """Write capture results to a JSON file.""" - with open(file_path, "w") as f: + with open(file_path, "w", encoding="utf-8") as f: f.write(self.to_json(include_context, indent)) diff --git a/libs/arcade-mcp-server/arcade_mcp_server/auth/__init__.py b/libs/arcade-mcp-server/arcade_mcp_server/auth/__init__.py index 593a3619..31426e8f 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/auth/__init__.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/auth/__init__.py @@ -39,8 +39,8 @@ __all__ = [ "LinkedIn", "Microsoft", "Notion", - "PagerDuty", "OAuth2", + "PagerDuty", "Reddit", "Slack", "Spotify", diff --git a/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py b/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py index 36b07e0f..252be942 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/mcp_app.py @@ -17,6 +17,10 @@ from typing import Any, Callable, Literal, ParamSpec, TypeVar, cast from arcade_core.catalog import MaterializedTool, ToolCatalog, ToolDefinitionError from arcade_core.metadata import ToolMetadata +from arcade_core.subprocess_utils import ( + get_windows_no_window_creationflags, + graceful_terminate_process, +) from arcade_tdk.auth import ToolAuthorization from arcade_tdk.error_adapters import ErrorAdapter from arcade_tdk.tool import tool as tool_decorator @@ -370,15 +374,27 @@ class MCPApp: env = os.environ.copy() env["ARCADE_MCP_CHILD_PROCESS"] = "1" + creationflags = get_windows_no_window_creationflags(new_process_group=True) + return subprocess.Popen( [sys.executable, *sys.argv], env=env, + creationflags=creationflags, ) def shutdown_server_process(process: subprocess.Popen, reason: str = "reload") -> None: - """Shutdown server process gracefully with fallback to force kill.""" + """Shutdown server process gracefully with fallback to force kill. + + On Windows, ``process.terminate()`` calls ``TerminateProcess`` which + kills the child immediately — there is no graceful shutdown. To + allow the child to clean up we first try sending ``CTRL_BREAK_EVENT`` + (requires ``CREATE_NEW_PROCESS_GROUP``), which Python's default + ``SIGINT`` handler will catch as ``KeyboardInterrupt``. If that + doesn't work we fall back to ``terminate()`` / ``kill()``. + """ logger.info(f"Shutting down server for {reason}...") - process.terminate() + + graceful_terminate_process(process) try: process.wait(timeout=5) diff --git a/libs/arcade-mcp-server/arcade_mcp_server/transports/stdio.py b/libs/arcade-mcp-server/arcade_mcp_server/transports/stdio.py index b3ba34d2..5732b0de 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/transports/stdio.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/transports/stdio.py @@ -81,6 +81,7 @@ class StdioTransport: self._shutdown_event = asyncio.Event() self._running = False self._sessions: dict[str, ServerSession] = {} + self._stop_task: asyncio.Task[None] | None = None async def start(self) -> None: """Start the transport.""" @@ -107,12 +108,21 @@ class StdioTransport: try: loop.add_signal_handler(sig, lambda: asyncio.create_task(self.stop())) except NotImplementedError: - # Windows doesn't support POSIX signals - if sys.platform == "win32": - logger.warning("Signal handling not fully supported on Windows") - else: + if sys.platform != "win32": logger.warning(f"Failed to set up signal handler for {sig}") + if sys.platform == "win32": + # On Windows, asyncio signal handlers don't work but the stdlib + # signal.signal(SIGINT) *does* receive Ctrl+C. Register a + # fallback so that a Ctrl+C schedules a clean stop on the loop. + def _schedule_stop() -> None: + self._stop_task = loop.create_task(self.stop()) + + def _win_ctrl_c(signum: int, frame: object) -> None: + loop.call_soon_threadsafe(_schedule_stop) + + signal.signal(signal.SIGINT, _win_ctrl_c) + async def stop(self) -> None: """Stop the transport.""" if not self._running: diff --git a/libs/arcade-mcp-server/pyproject.toml b/libs/arcade-mcp-server/pyproject.toml index 2d663a27..a19089f9 100644 --- a/libs/arcade-mcp-server/pyproject.toml +++ b/libs/arcade-mcp-server/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "arcade-mcp-server" -version = "1.17.0" +version = "1.17.2" description = "Model Context Protocol (MCP) server framework for Arcade.dev" readme = "README.md" authors = [{ name = "Arcade.dev" }] @@ -21,7 +21,7 @@ classifiers = [ ] requires-python = ">=3.10" dependencies = [ - "arcade-core>=4.4.0,<5.0.0", + "arcade-core>=4.4.2,<5.0.0", "arcade-serve>=3.2.0,<4.0.0", "arcade-tdk>=3.6.0,<4.0.0", "arcadepy>=1.5.0", diff --git a/libs/arcade-tdk/arcade_tdk/auth/__init__.py b/libs/arcade-tdk/arcade_tdk/auth/__init__.py index 17a86114..31426e8f 100644 --- a/libs/arcade-tdk/arcade_tdk/auth/__init__.py +++ b/libs/arcade-tdk/arcade_tdk/auth/__init__.py @@ -31,16 +31,16 @@ __all__ = [ "ClickUp", "Discord", "Dropbox", - "GitHub", "Figma", + "GitHub", "Google", "Hubspot", "Linear", "LinkedIn", "Microsoft", "Notion", - "PagerDuty", "OAuth2", + "PagerDuty", "Reddit", "Slack", "Spotify", diff --git a/libs/arcade-tdk/arcade_tdk/errors.py b/libs/arcade-tdk/arcade_tdk/errors.py index dcd90a5f..5b905649 100644 --- a/libs/arcade-tdk/arcade_tdk/errors.py +++ b/libs/arcade-tdk/arcade_tdk/errors.py @@ -10,6 +10,7 @@ from arcade_core.errors import ( ) __all__ = [ + "ContextRequiredToolError", "ErrorKind", "FatalToolError", "RetryableToolError", @@ -18,7 +19,6 @@ __all__ = [ "ToolRuntimeError", "UpstreamError", "UpstreamRateLimitError", - "ContextRequiredToolError", "WeightError", ] diff --git a/libs/tests/arcade_mcp_server/integration/test_end_to_end.py b/libs/tests/arcade_mcp_server/integration/test_end_to_end.py index be461c88..69483d53 100644 --- a/libs/tests/arcade_mcp_server/integration/test_end_to_end.py +++ b/libs/tests/arcade_mcp_server/integration/test_end_to_end.py @@ -7,8 +7,11 @@ including initialize, ping, list tools, and tool execution with all key features import asyncio import json import os -import random +import queue +import socket import subprocess +import sys +import threading import time from pathlib import Path from typing import Any @@ -24,6 +27,43 @@ def get_entrypoint_path() -> str: return str(Path(__file__).parent / "server" / "src" / "server" / "entrypoint.py") +HTTP_STARTUP_TIMEOUT_SECONDS = 30 if sys.platform == "win32" else 10 + + +def _find_open_tcp_port() -> int: + """Reserve a free loopback TCP port and return it.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + return int(sock.getsockname()[1]) + + +def _cleanup_process(process: subprocess.Popen, timeout: float = 5.0) -> None: + """Terminate subprocesses reliably, including uv child trees on Windows.""" + if process.poll() is not None: + return + + if sys.platform == "win32": + subprocess.run( + ["taskkill", "/PID", str(process.pid), "/T", "/F"], + check=False, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + process.wait(timeout=timeout) + except subprocess.TimeoutExpired: + process.kill() + process.wait() + return + + process.terminate() + try: + process.wait(timeout=timeout) + except subprocess.TimeoutExpired: + process.kill() + process.wait() + + def start_mcp_server( transport: str, port: int | None = None ) -> tuple[subprocess.Popen, int | None]: @@ -56,7 +96,7 @@ def start_mcp_server( elif transport == "http": if port is None: - port = random.randint(8000, 9000) # noqa: S311 + port = _find_open_tcp_port() env = { **os.environ, @@ -70,8 +110,8 @@ def start_mcp_server( cmd = ["uv", "run", entrypoint_path, "http"] process = subprocess.Popen( cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, text=True, env=env, cwd=str(package_path), @@ -104,7 +144,7 @@ def start_mcp_server_direct_python( pytest.skip("Server venv not found - run 'uv sync' in integration/server first") if port is None: - port = random.randint(8000, 9000) # noqa: S311 + port = _find_open_tcp_port() env = { **os.environ, @@ -118,8 +158,8 @@ def start_mcp_server_direct_python( cmd = [str(venv_python), entrypoint_path, transport] process = subprocess.Popen( cmd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, text=True, env=env, cwd=str(package_path), @@ -127,21 +167,32 @@ def start_mcp_server_direct_python( return process, port if transport == "http" else None -def wait_for_http_server_ready(port: int, timeout: int = 30) -> None: +def wait_for_http_server_ready( + port: int, + timeout: int = HTTP_STARTUP_TIMEOUT_SECONDS, + process: subprocess.Popen | None = None, +) -> None: """ Wait for HTTP server to become healthy. Args: port: Server port timeout: Maximum time to wait in seconds + process: Optional subprocess handle for early-exit detection Raises: TimeoutError: If server doesn't become healthy within timeout + RuntimeError: If process exits before becoming healthy """ health_url = f"http://127.0.0.1:{port}/worker/health" - start_time = time.time() + deadline = time.monotonic() + timeout - while time.time() - start_time < timeout: + while time.monotonic() < deadline: + if process is not None and process.poll() is not None: + raise RuntimeError( + f"Server process exited with code {process.returncode} " + f"before becoming healthy on port {port}" + ) try: response = httpx.get(health_url, timeout=2.0) if response.status_code == 200: @@ -251,6 +302,41 @@ class StdioClient: def __init__(self, process: subprocess.Popen): self.process = process self._next_id = 1 + self._stdout_messages: queue.Queue[dict[str, Any]] = queue.Queue() + self._stdout_reader_error: Exception | None = None + self._stderr_lines: list[str] = [] + self._stdout_reader = threading.Thread(target=self._stdout_reader_loop, daemon=True) + self._stderr_reader = threading.Thread(target=self._stderr_reader_loop, daemon=True) + self._stdout_reader.start() + self._stderr_reader.start() + + def _stdout_reader_loop(self) -> None: + """Continuously drain stdout so timeout checks are not blocked by readline().""" + if self.process.stdout is None: + return + + try: + for line in self.process.stdout: + message = parse_jsonrpc_message(line) + if message: + self._stdout_messages.put(message) + except Exception as exc: + self._stdout_reader_error = exc + + def _stderr_reader_loop(self) -> None: + """Drain stderr to avoid Windows pipe backpressure deadlocks.""" + if self.process.stderr is None: + return + + try: + for line in self.process.stderr: + self._stderr_lines.append(line.rstrip()) + # Keep only the most recent lines for debugging. + if len(self._stderr_lines) > 25: + self._stderr_lines = self._stderr_lines[-25:] + except Exception: + # Best-effort diagnostics reader; failures here should not hide test results. + pass def send_request(self, method: str, params: dict | None = None) -> int: """Send a request and return the request ID.""" @@ -290,18 +376,28 @@ class StdioClient: def read_response(self, timeout: float = 10.0) -> dict: """Read a response from the server.""" - start_time = time.time() + deadline = time.monotonic() + timeout - while time.time() - start_time < timeout: - if self.process.stdout: - line = self.process.stdout.readline() - if line: - message = parse_jsonrpc_message(line) - if message: - return message - time.sleep(0.01) + while time.monotonic() < deadline: + if self._stdout_reader_error is not None: + raise RuntimeError("Failed while reading stdio response") from self._stdout_reader_error - raise TimeoutError("Timeout waiting for response") + try: + return self._stdout_messages.get(timeout=0.1) + except queue.Empty: + if self.process.poll() is not None and self._stdout_messages.empty(): + stderr_tail = "\n".join(self._stderr_lines[-5:]) + details = ( + f"\nLast stderr lines:\n{stderr_tail}" if stderr_tail else "\nNo stderr captured." + ) + raise RuntimeError( + f"MCP server exited with code {self.process.returncode} while waiting for response." + f"{details}" + ) from None + + stderr_tail = "\n".join(self._stderr_lines[-5:]) + details = f"\nLast stderr lines:\n{stderr_tail}" if stderr_tail else "\nNo stderr captured." + raise TimeoutError(f"Timeout waiting for response after {timeout:.1f}s.{details}") def handle_bidirectional_request(self, message: dict) -> None: """Handle a server-initiated request by sending appropriate mock response.""" @@ -526,12 +622,7 @@ async def test_stdio_e2e(): finally: # Clean shutdown - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait() + _cleanup_process(process) @pytest.mark.asyncio @@ -543,7 +634,11 @@ async def test_http_e2e(): base_url = f"http://127.0.0.1:{port}" try: - wait_for_http_server_ready(port, timeout=10) + wait_for_http_server_ready( + port, + timeout=HTTP_STARTUP_TIMEOUT_SECONDS, + process=process, + ) headers = { "Content-Type": "application/json", @@ -668,12 +763,7 @@ async def test_http_e2e(): client.close() finally: - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait() + _cleanup_process(process) @pytest.mark.asyncio @@ -685,7 +775,11 @@ async def test_http_mcp_concurrent_tool_execution(): base_url = f"http://127.0.0.1:{port}" try: - wait_for_http_server_ready(port, timeout=10) + wait_for_http_server_ready( + port, + timeout=HTTP_STARTUP_TIMEOUT_SECONDS, + process=process, + ) headers = { "Content-Type": "application/json", @@ -768,12 +862,7 @@ async def test_http_mcp_concurrent_tool_execution(): assert total_time < max_expected_time finally: - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait() + _cleanup_process(process) @pytest.mark.asyncio @@ -785,7 +874,11 @@ async def test_http_worker_concurrent_tool_execution(): base_url = f"http://127.0.0.1:{port}" try: - wait_for_http_server_ready(port, timeout=10) + wait_for_http_server_ready( + port, + timeout=HTTP_STARTUP_TIMEOUT_SECONDS, + process=process, + ) headers = { "Content-Type": "application/json", @@ -844,12 +937,7 @@ async def test_http_worker_concurrent_tool_execution(): assert total_time < max_expected_time finally: - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait() + _cleanup_process(process) @pytest.mark.asyncio @@ -861,7 +949,11 @@ async def test_http_mixed_route_concurrent_execution(): base_url = f"http://127.0.0.1:{port}" try: - wait_for_http_server_ready(port, timeout=10) + wait_for_http_server_ready( + port, + timeout=HTTP_STARTUP_TIMEOUT_SECONDS, + process=process, + ) headers = { "Content-Type": "application/json", @@ -952,12 +1044,7 @@ async def test_http_mixed_route_concurrent_execution(): assert total_time < max_expected_time finally: - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait() + _cleanup_process(process) @pytest.mark.asyncio @@ -967,7 +1054,11 @@ async def test_http_direct_python_invocation(): assert port is not None try: - wait_for_http_server_ready(port, timeout=10) + wait_for_http_server_ready( + port, + timeout=HTTP_STARTUP_TIMEOUT_SECONDS, + process=process, + ) # Verify server is healthy and tools are discoverable headers = { @@ -1009,9 +1100,4 @@ async def test_http_direct_python_invocation(): client.close() finally: - process.terminate() - try: - process.wait(timeout=5) - except subprocess.TimeoutExpired: - process.kill() - process.wait() + _cleanup_process(process) diff --git a/libs/tests/arcade_mcp_server/test_mcp_app.py b/libs/tests/arcade_mcp_server/test_mcp_app.py index 97f61f67..5977cbdc 100644 --- a/libs/tests/arcade_mcp_server/test_mcp_app.py +++ b/libs/tests/arcade_mcp_server/test_mcp_app.py @@ -415,8 +415,13 @@ class TestMCPApp: # Verify both processes were created assert mock_popen.call_count == 2 - # Verify first process was terminated - mock_process1.terminate.assert_called_once() + # Verify first process was shut down. + # On Windows, shutdown uses send_signal(CTRL_BREAK_EVENT) instead + # of terminate() for graceful shutdown. + if sys.platform == "win32": + mock_process1.send_signal.assert_called_once() + else: + mock_process1.terminate.assert_called_once() mock_process1.wait.assert_called() def test_run_with_reload_graceful_shutdown(self, mcp_app: MCPApp): @@ -433,8 +438,13 @@ class TestMCPApp: mcp_app._run_with_reload("127.0.0.1", 8000) - # Verify graceful shutdown - mock_process.terminate.assert_called() + # Verify graceful shutdown. + # On Windows, send_signal(CTRL_BREAK_EVENT) is used instead of + # terminate() to allow graceful child cleanup. + if sys.platform == "win32": + mock_process.send_signal.assert_called() + else: + mock_process.terminate.assert_called() mock_process.wait.assert_called() mock_process.kill.assert_not_called() @@ -453,8 +463,12 @@ class TestMCPApp: mcp_app._run_with_reload("127.0.0.1", 8000) - # Verify terminate -> wait -> kill -> wait sequence - mock_process.terminate.assert_called() + # Verify shutdown -> wait -> kill -> wait sequence. + # On Windows, send_signal is used instead of terminate. + if sys.platform == "win32": + mock_process.send_signal.assert_called() + else: + mock_process.terminate.assert_called() assert mock_process.wait.call_count == 2 mock_process.kill.assert_called_once() @@ -472,8 +486,11 @@ class TestMCPApp: # Should not raise exception mcp_app._run_with_reload("127.0.0.1", 8000) - # Verify process was shut down - mock_process.terminate.assert_called_once() + # Verify process was shut down. + if sys.platform == "win32": + mock_process.send_signal.assert_called_once() + else: + mock_process.terminate.assert_called_once() def test_run_routes_to_reload_method(self, mcp_app: MCPApp): """Test run() routes to _run_with_reload when reload=True.""" diff --git a/libs/tests/cli/deploy/test_deploy.py b/libs/tests/cli/deploy/test_deploy.py index ff7bf363..95b540f9 100644 --- a/libs/tests/cli/deploy/test_deploy.py +++ b/libs/tests/cli/deploy/test_deploy.py @@ -1,8 +1,10 @@ import base64 import io +import socket import subprocess import tarfile from pathlib import Path +from unittest.mock import MagicMock, patch import pytest from arcade_cli.deploy import ( @@ -58,10 +60,24 @@ version = "0.1.0" description = "Test project" requires-python = ">=3.10" """ - (project_dir / "pyproject.toml").write_text(pyproject_content) + (project_dir / "pyproject.toml").write_text(pyproject_content, encoding="utf-8") return project_dir +@pytest.fixture +def reserved_unreachable_local_url(): + """Yield a localhost URL that is guaranteed not to have an HTTP listener. + + Keeps a TCP socket bound (without listen()) so no other process can claim + the port during the test, avoiding flaky collisions with long-lived local + dev servers. + """ + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("127.0.0.1", 0)) + port = sock.getsockname()[1] + yield f"http://127.0.0.1:{port}" + + # Tests for create_package_archive @@ -97,7 +113,7 @@ def test_create_package_archive_nonexistent_dir(tmp_path: Path) -> None: def test_create_package_archive_file_not_dir(tmp_path: Path) -> None: """Test that archiving a file instead of directory raises ValueError.""" test_file = tmp_path / "test_file.txt" - test_file.write_text("test content") + test_file.write_text("test content", encoding="utf-8") with pytest.raises(ValueError, match="Package path must be a directory"): create_package_archive(test_file) @@ -109,18 +125,18 @@ def test_create_package_archive_excludes_files(tmp_path: Path) -> None: test_dir.mkdir() # Create files that should be excluded - (test_dir / ".hidden").write_text("hidden") + (test_dir / ".hidden").write_text("hidden", encoding="utf-8") (test_dir / "__pycache__").mkdir() - (test_dir / "__pycache__" / "cache.pyc").write_text("cache") - (test_dir / "requirements.lock").write_text("lock") + (test_dir / "__pycache__" / "cache.pyc").write_text("cache", encoding="utf-8") + (test_dir / "requirements.lock").write_text("lock", encoding="utf-8") (test_dir / "dist").mkdir() - (test_dir / "dist" / "package.tar.gz").write_text("dist") + (test_dir / "dist" / "package.tar.gz").write_text("dist", encoding="utf-8") (test_dir / "build").mkdir() - (test_dir / "build" / "lib").write_text("build") + (test_dir / "build" / "lib").write_text("build", encoding="utf-8") # Create files that should be included - (test_dir / "main.py").write_text("main") - (test_dir / "pyproject.toml").write_text("project") + (test_dir / "main.py").write_text("main", encoding="utf-8") + (test_dir / "pyproject.toml").write_text("project", encoding="utf-8") archive_base64 = create_package_archive(test_dir) archive_bytes = base64.b64decode(archive_base64) @@ -202,9 +218,9 @@ def test_get_server_info_success(valid_server_path: str, capsys) -> None: process.wait() -def test_get_server_info_invalid_url() -> None: +def test_get_server_info_invalid_url(reserved_unreachable_local_url: str) -> None: """Test that invalid URL raises ValueError.""" - invalid_url = "http://127.0.0.1:9999" + invalid_url = reserved_unreachable_local_url with pytest.raises(ValueError): get_server_info(invalid_url) @@ -256,9 +272,9 @@ def test_get_required_secrets_no_secrets(valid_server_path: str) -> None: process.wait() -def test_get_required_secrets_invalid_url() -> None: +def test_get_required_secrets_invalid_url(reserved_unreachable_local_url: str) -> None: """Test that invalid URL raises ValueError.""" - invalid_url = "http://127.0.0.1:9999" + invalid_url = reserved_unreachable_local_url with pytest.raises( ValueError, match="Failed to extract tool secrets from /worker/tools endpoint" @@ -291,3 +307,67 @@ def test_verify_server_and_get_metadata_with_debug(valid_server_path: str, capsy assert server_name == "simpleserver" assert server_version == "1.0.0" assert "MY_SECRET_KEY" in required_secrets + + +# --------------------------------------------------------------------------- +# Debug-aware error messages +# --------------------------------------------------------------------------- + + +@patch("arcade_cli.deploy.find_python_interpreter") +@patch("arcade_cli.deploy.subprocess.Popen") +def test_start_server_process_non_debug_message( + mock_popen: MagicMock, mock_python: MagicMock +) -> None: + """Non-debug mode error should hint at --debug flag.""" + mock_python.return_value = Path("python3") + mock_proc = MagicMock() + mock_proc.poll.return_value = 1 # Process exited immediately + mock_popen.return_value = mock_proc + + with pytest.raises(ValueError, match="--debug"): + start_server_process("server.py", debug=False) + + +@patch("arcade_cli.deploy.find_python_interpreter") +@patch("arcade_cli.deploy.subprocess.Popen") +def test_start_server_process_debug_message( + mock_popen: MagicMock, mock_python: MagicMock +) -> None: + """Debug mode error should NOT tell user to run with --debug (already in debug mode).""" + mock_python.return_value = Path("python3") + mock_proc = MagicMock() + mock_proc.poll.return_value = 1 # Process exited immediately + mock_popen.return_value = mock_proc + + with pytest.raises(ValueError) as exc_info: + start_server_process("server.py", debug=True) + + msg = str(exc_info.value) + assert "--debug" not in msg, "Debug mode error must not tell user to re-run with --debug" + assert "above" in msg.lower() or "output" in msg.lower() + + +def test_wait_for_health_non_debug_message(reserved_unreachable_local_url: str) -> None: + """Non-debug health timeout should hint at --debug flag.""" + mock_proc = MagicMock() + mock_proc.communicate.return_value = (None, None) + + with pytest.raises(ValueError, match="--debug"): + wait_for_health(reserved_unreachable_local_url, mock_proc, timeout=1, debug=False) + + +def test_wait_for_health_debug_message(reserved_unreachable_local_url: str) -> None: + """Debug health timeout should NOT tell user to run with --debug, + and SHOULD reference checking the output already shown above.""" + mock_proc = MagicMock() + mock_proc.communicate.return_value = (None, None) + + with pytest.raises(ValueError) as exc_info: + wait_for_health(reserved_unreachable_local_url, mock_proc, timeout=1, debug=True) + + msg = str(exc_info.value) + assert "--debug" not in msg, "Debug mode error must not tell user to re-run with --debug" + assert "above" in msg.lower() or "output" in msg.lower(), ( + f"Debug mode error should reference checking output above; got: {msg!r}" + ) diff --git a/libs/tests/cli/test_authn_callback.py b/libs/tests/cli/test_authn_callback.py new file mode 100644 index 00000000..d2ccfa2a --- /dev/null +++ b/libs/tests/cli/test_authn_callback.py @@ -0,0 +1,336 @@ +import subprocess +import sys +from unittest.mock import MagicMock, patch +from urllib.error import HTTPError +from urllib.request import urlopen + +from arcade_cli.authn import OAuthCallbackServer, _open_browser, oauth_callback_server + + +def test_oauth_callback_server_success() -> None: + state = "test-state" + with oauth_callback_server(state, port=0) as server: + url = f"{server.get_redirect_uri()}?code=abc123&state={state}" + with urlopen(url) as response: + assert response.status == 200 + response.read() + assert server.wait_for_result(timeout=1.0) is True + + assert server.result["code"] == "abc123" + + +def test_oauth_callback_server_timeout() -> None: + state = "test-timeout" + with oauth_callback_server(state, port=0) as server: + assert server.wait_for_result(timeout=0.05) is False + + assert "Timed out" in server.result["error"] + + +def test_oauth_callback_server_binds_to_loopback() -> None: + """The callback server must bind to 127.0.0.1 (loopback) to avoid + Windows Firewall prompts and keep redirect host aligned with bind host.""" + state = "test-bind" + with oauth_callback_server(state, port=0) as server: + assert server.httpd is not None + host, _port = server.httpd.server_address + assert host == "127.0.0.1", f"Expected 127.0.0.1 but got {host}" + # Also confirm the redirect URI host matches the bound loopback host. + redirect = server.get_redirect_uri() + assert redirect.startswith("http://127.0.0.1:") + server.shutdown_server() + + +def test_oauth_callback_server_state_mismatch() -> None: + """Requests with a mismatched state parameter should return an error.""" + state = "correct-state" + with oauth_callback_server(state, port=0) as server: + url = f"{server.get_redirect_uri()}?code=abc&state=wrong-state" + try: + with urlopen(url) as response: + response.read() + except HTTPError: + pass # Expected — handler returns 400 for state mismatch. + server.wait_for_result(timeout=1.0) + + assert "error" in server.result + + +def test_oauth_callback_server_missing_code() -> None: + """Requests without a code parameter should produce an error result.""" + state = "no-code-state" + with oauth_callback_server(state, port=0) as server: + url = f"{server.get_redirect_uri()}?state={state}" + try: + with urlopen(url) as response: + response.read() + except HTTPError: + pass # Expected — handler returns 400 for missing code. + server.wait_for_result(timeout=1.0) + + assert "error" in server.result + + +def test_oauth_callback_server_wait_until_ready() -> None: + """wait_until_ready() should return True once the server is listening.""" + state = "ready-test" + server = OAuthCallbackServer(state, port=0) + + import threading + + t = threading.Thread(target=server.run_server, daemon=True) + t.start() + + assert server.wait_until_ready(timeout=5.0) is True + assert server.httpd is not None + assert server.port != 0 # Ephemeral port was assigned. + + server.shutdown_server() + t.join(timeout=2) + + +def test_oauth_callback_server_wait_until_ready_timeout() -> None: + """wait_until_ready() should return False if the server never starts.""" + state = "ready-timeout" + server = OAuthCallbackServer(state, port=0) + # Don't start the server — ready_event never gets set. + assert server.wait_until_ready(timeout=0.05) is False + + +def test_perform_oauth_login_hides_auth_url_when_browser_succeeds() -> None: + """When browser launch succeeds, status output should not include auth URL.""" + status_messages: list[str] = [] + + def capture_status(msg: str) -> None: + status_messages.append(msg) + + # We need to mock the entire OAuth flow since we can't hit a real coordinator. + with ( + patch("arcade_cli.authn.fetch_cli_config") as mock_config, + patch("arcade_cli.authn.create_oauth_client"), + patch("arcade_cli.authn.generate_authorization_url") as mock_gen_url, + patch("arcade_cli.authn._open_browser") as mock_browser, + patch("arcade_cli.authn.oauth_callback_server") as mock_server_ctx, + ): + mock_config.return_value = MagicMock() + mock_gen_url.return_value = ("https://example.com/auth?state=abc", "verifier123") + mock_browser.return_value = True + + mock_server = MagicMock() + mock_server.get_redirect_uri.return_value = "http://localhost:9999/callback" + mock_server.result = {"error": "timeout for test"} + mock_server.wait_for_result.return_value = False + mock_server_ctx.return_value.__enter__ = MagicMock(return_value=mock_server) + mock_server_ctx.return_value.__exit__ = MagicMock(return_value=False) + + from arcade_cli.authn import OAuthLoginError, perform_oauth_login + + try: + perform_oauth_login( + "https://fake-coordinator.example.com", + on_status=capture_status, + callback_timeout_seconds=1, + ) + except OAuthLoginError: + pass + + url_messages = [m for m in status_messages if "https://example.com/auth" in m] + assert len(url_messages) == 0, ( + "Auth URL should be hidden when browser launch succeeds. " + f"Got status messages: {status_messages}" + ) + + +def test_perform_oauth_login_shows_url_when_browser_fails() -> None: + """When _open_browser fails, the URL should still be shown.""" + status_messages: list[str] = [] + + def capture_status(msg: str) -> None: + status_messages.append(msg) + + with ( + patch("arcade_cli.authn.fetch_cli_config") as mock_config, + patch("arcade_cli.authn.create_oauth_client"), + patch("arcade_cli.authn.generate_authorization_url") as mock_gen_url, + patch("arcade_cli.authn._open_browser") as mock_browser, + patch("arcade_cli.authn.oauth_callback_server") as mock_server_ctx, + ): + mock_config.return_value = MagicMock() + mock_gen_url.return_value = ("https://example.com/auth?state=xyz", "verifier456") + mock_browser.return_value = False # Browser failed + + mock_server = MagicMock() + mock_server.get_redirect_uri.return_value = "http://localhost:9999/callback" + mock_server.result = {"error": "timeout for test"} + mock_server.wait_for_result.return_value = False + mock_server_ctx.return_value.__enter__ = MagicMock(return_value=mock_server) + mock_server_ctx.return_value.__exit__ = MagicMock(return_value=False) + + from arcade_cli.authn import OAuthLoginError, perform_oauth_login + + try: + perform_oauth_login( + "https://fake-coordinator.example.com", + on_status=capture_status, + callback_timeout_seconds=1, + ) + except OAuthLoginError: + pass + + url_messages = [m for m in status_messages if "https://example.com/auth" in m] + assert len(url_messages) >= 1 + assert any("Open this link to log in" in m for m in status_messages) + # When browser fails, the message should say "Could not open a browser" + browser_fail_msgs = [m for m in status_messages if "Could not open a browser" in m] + assert len(browser_fail_msgs) >= 1 + + +def test_perform_oauth_login_timeout_clamps_negative() -> None: + """Negative --timeout values should be clamped to the default.""" + from arcade_cli.authn import DEFAULT_OAUTH_TIMEOUT_SECONDS + + status_messages: list[str] = [] + + def capture_status(msg: str) -> None: + status_messages.append(msg) + + with ( + patch("arcade_cli.authn.fetch_cli_config") as mock_config, + patch("arcade_cli.authn.create_oauth_client"), + patch("arcade_cli.authn.generate_authorization_url") as mock_gen_url, + patch("arcade_cli.authn._open_browser") as mock_browser, + patch("arcade_cli.authn.oauth_callback_server") as mock_server_ctx, + ): + mock_config.return_value = MagicMock() + mock_gen_url.return_value = ("https://example.com/auth", "v") + mock_browser.return_value = True + + mock_server = MagicMock() + mock_server.get_redirect_uri.return_value = "http://localhost:9999/callback" + mock_server.result = {"error": "timeout"} + mock_server.wait_for_result.return_value = False + mock_server_ctx.return_value.__enter__ = MagicMock(return_value=mock_server) + mock_server_ctx.return_value.__exit__ = MagicMock(return_value=False) + + from arcade_cli.authn import OAuthLoginError, perform_oauth_login + + try: + perform_oauth_login( + "https://fake.example.com", + on_status=capture_status, + callback_timeout_seconds=-5, + ) + except OAuthLoginError: + pass + + # The timeout message should show the default, not -5. + timeout_msgs = [m for m in status_messages if "timeout:" in m.lower()] + assert any(str(DEFAULT_OAUTH_TIMEOUT_SECONDS) in m for m in timeout_msgs), ( + f"Expected default timeout {DEFAULT_OAUTH_TIMEOUT_SECONDS} in messages: {timeout_msgs}" + ) + + +# --------------------------------------------------------------------------- +# _open_browser() — CMD-window suppression on Windows +# --------------------------------------------------------------------------- + + +class TestOpenBrowser: + """Tests for the _open_browser helper that suppresses CMD flash on Windows. + + On Windows the priority order is: + 1. ctypes ShellExecuteW (direct Win32 API, no console) + 2. rundll32 url.dll (GUI binary, no console) + 3. webbrowser.open (stdlib fallback) + + os.startfile is intentionally omitted: it is a thin CPython wrapper around + ShellExecuteExW, making it redundant with step 1. + """ + + def test_delegates_to_webbrowser_on_non_windows(self) -> None: + """On non-Windows, _open_browser should use webbrowser.open.""" + with ( + patch.object(sys, "platform", "linux"), + patch("arcade_cli.authn.webbrowser") as mock_wb, + ): + mock_wb.open.return_value = True + result = _open_browser("https://example.com") + assert result is True + mock_wb.open.assert_called_once_with("https://example.com") + + def test_tries_ctypes_shellexecute_first_on_windows(self) -> None: + """On Windows, _open_browser should try ctypes ShellExecuteW first.""" + import ctypes + + # On non-Windows, ctypes.windll doesn't exist; provide a mock + mock_shell32 = MagicMock() + mock_shell32.ShellExecuteW = MagicMock(return_value=42) + mock_windll = MagicMock() + mock_windll.shell32 = mock_shell32 + + with patch.object(sys, "platform", "win32"), patch.object( + ctypes, "windll", mock_windll, create=True + ): + result = _open_browser("https://example.com") + assert result is True + + def test_falls_back_to_rundll32_on_windows(self) -> None: + """If ctypes fails, try rundll32 url.dll.""" + import ctypes + + mock_shell32 = MagicMock() + mock_shell32.ShellExecuteW = MagicMock(side_effect=Exception("ctypes failed")) + mock_windll = MagicMock() + mock_windll.shell32 = mock_shell32 + + with ( + patch.object(sys, "platform", "win32"), + patch.object(ctypes, "windll", mock_windll, create=True), + patch("arcade_cli.authn.subprocess.Popen") as mock_popen, + patch("arcade_cli.authn.subprocess.STARTUPINFO", create=True) as mock_si_cls, + patch("arcade_cli.authn.subprocess.STARTF_USESHOWWINDOW", 1, create=True), + patch("arcade_cli.authn.subprocess.DEVNULL", subprocess.DEVNULL), + ): + mock_si = MagicMock() + mock_si.dwFlags = 0 + mock_si_cls.return_value = mock_si + mock_popen.return_value = MagicMock() + + result = _open_browser("https://example.com") + assert result is True + mock_popen.assert_called_once() + cmd = mock_popen.call_args[0][0] + assert cmd[0] == "rundll32" + + def test_falls_back_to_webbrowser_if_all_else_fails_on_windows(self) -> None: + """If ctypes and rundll32 both fail, use webbrowser.open (step 3).""" + import ctypes + + mock_shell32 = MagicMock() + mock_shell32.ShellExecuteW = MagicMock(side_effect=Exception("ctypes failed")) + mock_windll = MagicMock() + mock_windll.shell32 = mock_shell32 + + with ( + patch.object(sys, "platform", "win32"), + patch.object(ctypes, "windll", mock_windll, create=True), + patch("arcade_cli.authn.subprocess.Popen", side_effect=Exception("fail")), + patch("arcade_cli.authn.subprocess.STARTUPINFO", create=True, return_value=MagicMock()), + patch("arcade_cli.authn.subprocess.STARTF_USESHOWWINDOW", 1, create=True), + patch("arcade_cli.authn.subprocess.DEVNULL", -1), + patch("arcade_cli.authn.webbrowser") as mock_wb, + ): + mock_wb.open.return_value = True + result = _open_browser("https://example.com") + assert result is True + mock_wb.open.assert_called_once() + + def test_returns_false_if_everything_fails(self) -> None: + """If all methods fail, _open_browser should return False.""" + with ( + patch.object(sys, "platform", "linux"), + patch("arcade_cli.authn.webbrowser") as mock_wb, + ): + mock_wb.open.side_effect = Exception("no browser") + result = _open_browser("https://example.com") + assert result is False diff --git a/libs/tests/cli/test_configure.py b/libs/tests/cli/test_configure.py new file mode 100644 index 00000000..ac7b430d --- /dev/null +++ b/libs/tests/cli/test_configure.py @@ -0,0 +1,484 @@ +import json +import sys +import types +from io import StringIO +from pathlib import Path + +import pytest +from arcade_cli.configure import ( + _format_path_for_display, + _resolve_windows_appdata, + _warn_overwrite, + configure_client, +) + + +def _write_entrypoint(tmp_path: Path) -> Path: + entrypoint = tmp_path / "server.py" + entrypoint.write_text("print('ok')\n", encoding="utf-8") + return entrypoint + + +def _load_json(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +def _assert_stdio_entry(entry: dict) -> None: + assert "command" in entry + assert "args" in entry + assert any(str(arg).endswith("server.py") for arg in entry["args"]) + assert "env" in entry + + +# --------------------------------------------------------------------------- +# _format_path_for_display() +# --------------------------------------------------------------------------- + + +def test_format_path_for_display_windows_quotes() -> None: + path = Path(r"C:\Users\A User\My Server\mcp.json") + assert ( + _format_path_for_display(path, platform_system="Windows") + == '"C:\\Users\\A User\\My Server\\mcp.json"' + ) + + +def test_format_path_for_display_no_spaces_unchanged() -> None: + """Paths without spaces should be returned as-is.""" + path = Path(r"C:\Users\Alice\mcp.json") + result = _format_path_for_display(path, platform_system="Windows") + assert result == str(path) + assert '"' not in result + + +def test_format_path_for_display_posix_escapes() -> None: + # Use str directly to avoid Windows Path normalization converting / to \ + import sys + + if sys.platform == "win32": + # On Windows, Path("/tmp/with space/mcp.json") uses backslashes. + # The function should still escape spaces. + path = Path("/tmp/with space/mcp.json") + result = _format_path_for_display(path, platform_system="Linux") + assert "\\ " in result # spaces are escaped + else: + path = Path("/tmp/with space/mcp.json") + assert ( + _format_path_for_display(path, platform_system="Linux") + == "/tmp/with\\ space/mcp.json" + ) + + +# --------------------------------------------------------------------------- +# _resolve_windows_appdata() +# --------------------------------------------------------------------------- + + +def test_resolve_windows_appdata_delegates_to_platformdirs( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """_resolve_windows_appdata returns whatever platformdirs resolves.""" + monkeypatch.delenv("APPDATA", raising=False) + monkeypatch.delenv("LOCALAPPDATA", raising=False) + monkeypatch.delenv("USERPROFILE", raising=False) + + fake_platformdirs = types.ModuleType("platformdirs") + fake_platformdirs.user_data_dir = ( + lambda *args, **kwargs: r"C:\Users\Alice\AppData\Roaming" + ) + monkeypatch.setitem(sys.modules, "platformdirs", fake_platformdirs) + + assert _resolve_windows_appdata() == Path(r"C:\Users\Alice\AppData\Roaming") + + +def test_resolve_windows_appdata_handles_older_platformdirs( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Falls back to positional args when platformdirs raises TypeError. + + The positional signature is user_data_dir(appname, appauthor, version, roaming). + The fallback call must pass roaming=True as the *fourth* positional arg, not + the third (which would be ``version``). + """ + received_args: list[tuple] = [] + + def strict_user_data_dir(*args: object, **kwargs: object) -> str: + if kwargs: + raise TypeError("keyword args not supported") + received_args.append(args) + return r"C:\Users\Bob\AppData\Roaming" + + fake_platformdirs = types.ModuleType("platformdirs") + fake_platformdirs.user_data_dir = strict_user_data_dir + monkeypatch.setitem(sys.modules, "platformdirs", fake_platformdirs) + + result = _resolve_windows_appdata() + assert result == Path(r"C:\Users\Bob\AppData\Roaming") + + # First call raises TypeError (has kwargs), second call uses positional args. + # Verify the fallback used the correct signature: (appname, appauthor, version, roaming) + assert len(received_args) == 1, "Fallback must make exactly one positional call" + fallback_args = received_args[0] + # args: (None, False, None, True) — roaming is the 4th positional arg + assert len(fallback_args) == 4, f"Expected 4 positional args, got {len(fallback_args)}: {fallback_args}" + assert fallback_args[3] is True, f"4th arg (roaming) must be True, got {fallback_args[3]}" + assert fallback_args[2] is None, f"3rd arg (version) must be None, got {fallback_args[2]}" + + +def test_get_cursor_config_path_windows_prefers_existing_candidate( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + import arcade_cli.configure as configure_mod + + appdata_path = tmp_path / "AppData" / "Roaming" / "Cursor" / "mcp.json" + home_path = tmp_path / ".cursor" / "mcp.json" + home_path.parent.mkdir(parents=True, exist_ok=True) + home_path.write_text("{}", encoding="utf-8") + + monkeypatch.setattr(configure_mod.platform, "system", lambda: "Windows") + monkeypatch.setattr( + configure_mod, + "_get_windows_cursor_config_paths", + lambda: [appdata_path, home_path], + ) + + assert configure_mod.get_cursor_config_path() == home_path + + +# --------------------------------------------------------------------------- +# _warn_overwrite() +# --------------------------------------------------------------------------- + + +def test_warn_overwrite_prints_when_entry_exists() -> None: + """Should print a yellow warning when the server entry already exists.""" + from arcade_cli.console import Console + + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + + import arcade_cli.configure as configure_mod + + orig = configure_mod.console + configure_mod.console = test_console + try: + config = {"mcpServers": {"demo": {"command": "old"}}} + _warn_overwrite(config, "mcpServers", "demo", Path("/fake/cursor.json")) + finally: + configure_mod.console = orig + + output = buf.getvalue() + assert "demo" in output + assert "already exists" in output + + +def test_warn_overwrite_silent_when_no_entry() -> None: + """Should NOT print anything when the server entry doesn't exist.""" + from arcade_cli.console import Console + + buf = StringIO() + test_console = Console(file=buf, force_terminal=True) + + # Temporarily monkey-patch the module-level console used by _warn_overwrite. + import arcade_cli.configure as configure_mod + + orig = configure_mod.console + configure_mod.console = test_console + try: + config: dict = {"mcpServers": {}} + _warn_overwrite(config, "mcpServers", "new_server", Path("/fake/mcp.json")) + finally: + configure_mod.console = orig + + assert buf.getvalue() == "", "No output expected when entry doesn't exist" + + +def test_warn_overwrite_message_content() -> None: + """Verify the warning message mentions the server name.""" + from arcade_cli.console import Console + + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + + import arcade_cli.configure as configure_mod + + orig = configure_mod.console + configure_mod.console = test_console + try: + config = {"servers": {"my_srv": {"command": "old"}}} + _warn_overwrite(config, "servers", "my_srv", Path("/fake/vscode.json")) + finally: + configure_mod.console = orig + + output = buf.getvalue() + assert "my_srv" in output + assert "already exists" in output + assert "--name" in output + + +# --------------------------------------------------------------------------- +# UTF-8 config I/O +# --------------------------------------------------------------------------- + + +def test_config_written_as_utf8(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Config files must be written with UTF-8 encoding, including non-ASCII paths.""" + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + config_path = tmp_path / "config.json" + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + + # Read the file as raw bytes and verify UTF-8 BOM is absent and content + # decodes cleanly as UTF-8. + raw = config_path.read_bytes() + assert not raw.startswith(b"\xef\xbb\xbf"), "UTF-8 BOM should not be present" + decoded = raw.decode("utf-8") # Should not raise + data = json.loads(decoded) + assert "mcpServers" in data + assert "demo" in data["mcpServers"] + + +def test_config_roundtrip_preserves_unicode(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Write a config with Unicode, then overwrite and verify it still decodes.""" + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + config_path = tmp_path / "config.json" + + # Seed with Unicode content + config_path.write_text( + json.dumps({"mcpServers": {"caf\u00e9": {"command": "old"}}}), + encoding="utf-8", + ) + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + + data = json.loads(config_path.read_text(encoding="utf-8")) + # Original Unicode entry should be preserved alongside the new one. + assert "caf\u00e9" in data["mcpServers"] + assert "demo" in data["mcpServers"] + + +def test_cursor_config_stdio_and_http(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + config_path = tmp_path / "cursor.json" + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + config = _load_json(config_path) + entry = config["mcpServers"]["demo"] + _assert_stdio_entry(entry) + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="http", + host="local", + port=8123, + config_path=config_path, + ) + config = _load_json(config_path) + entry = config["mcpServers"]["demo"] + assert entry["type"] == "stream" + assert entry["url"] == "http://localhost:8123/mcp" + + +def test_cursor_config_stdio_uses_absolute_uv_path( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + import arcade_cli.configure as configure_mod + + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + config_path = tmp_path / "cursor.json" + monkeypatch.setattr( + configure_mod.shutil, + "which", + lambda executable: r"C:\Tools\uv.exe" if executable == "uv" else None, + ) + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + + config = _load_json(config_path) + assert config["mcpServers"]["demo"]["command"] == r"C:\Tools\uv.exe" + + +def test_cursor_windows_writes_compatibility_paths( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + import arcade_cli.configure as configure_mod + + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + + appdata_path = tmp_path / "AppData" / "Roaming" / "Cursor" / "mcp.json" + home_path = tmp_path / ".cursor" / "mcp.json" + appdata_path.parent.mkdir(parents=True, exist_ok=True) + home_path.parent.mkdir(parents=True, exist_ok=True) + appdata_path.write_text( + json.dumps({"mcpServers": {"appdata_only": {"command": "x"}}}), + encoding="utf-8", + ) + home_path.write_text( + json.dumps({"mcpServers": {"home_only": {"command": "y"}}}), + encoding="utf-8", + ) + + monkeypatch.setattr(configure_mod.platform, "system", lambda: "Windows") + monkeypatch.setattr(configure_mod, "get_cursor_config_path", lambda: appdata_path) + monkeypatch.setattr( + configure_mod, + "_get_windows_cursor_config_paths", + lambda: [appdata_path, home_path], + ) + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + ) + + appdata_config = _load_json(appdata_path) + home_config = _load_json(home_path) + assert "demo" in appdata_config["mcpServers"] + assert "demo" in home_config["mcpServers"] + assert "appdata_only" in appdata_config["mcpServers"] + assert "home_only" in home_config["mcpServers"] + + +def test_cursor_windows_explicit_config_does_not_write_compatibility_paths( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + import arcade_cli.configure as configure_mod + + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + + explicit_path = tmp_path / "custom" / "cursor.json" + appdata_path = tmp_path / "AppData" / "Roaming" / "Cursor" / "mcp.json" + home_path = tmp_path / ".cursor" / "mcp.json" + + monkeypatch.setattr(configure_mod.platform, "system", lambda: "Windows") + monkeypatch.setattr(configure_mod, "get_cursor_config_path", lambda: appdata_path) + monkeypatch.setattr( + configure_mod, + "_get_windows_cursor_config_paths", + lambda: [appdata_path, home_path], + ) + + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=explicit_path, + ) + + assert explicit_path.exists() + assert not appdata_path.exists() + assert not home_path.exists() + + +def test_vscode_config_stdio_and_http(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + config_path = tmp_path / "vscode.json" + + configure_client( + client="vscode", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + config = _load_json(config_path) + entry = config["servers"]["demo"] + _assert_stdio_entry(entry) + + configure_client( + client="vscode", + entrypoint_file="server.py", + server_name="demo", + transport="http", + host="local", + port=8123, + config_path=config_path, + ) + config = _load_json(config_path) + entry = config["servers"]["demo"] + assert entry["type"] == "http" + assert entry["url"] == "http://localhost:8123/mcp" + + +def test_claude_config_stdio_only(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.chdir(tmp_path) + _write_entrypoint(tmp_path) + config_path = tmp_path / "claude.json" + + configure_client( + client="claude", + entrypoint_file="server.py", + server_name="demo", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + config = _load_json(config_path) + entry = config["mcpServers"]["demo"] + _assert_stdio_entry(entry) + + with pytest.raises(ValueError, match="Claude Desktop only supports stdio"): + configure_client( + client="claude", + entrypoint_file="server.py", + server_name="demo", + transport="http", + host="local", + port=8000, + config_path=config_path, + ) diff --git a/libs/tests/cli/test_console_encoding.py b/libs/tests/cli/test_console_encoding.py new file mode 100644 index 00000000..ba323744 --- /dev/null +++ b/libs/tests/cli/test_console_encoding.py @@ -0,0 +1,161 @@ +"""Tests for the console.py encoding safety layer. + +These tests verify that _needs_utf8() and _configure_windows_utf8() +behave correctly and do not crash, even when the console encoding +would be cp1252 (the default on many Western-European Windows installs). +""" + +from __future__ import annotations + +import io +import os +import sys +from unittest.mock import patch + +import pytest +from arcade_cli.console import _configure_windows_utf8, _needs_utf8 + +# --------------------------------------------------------------------------- +# _needs_utf8() +# --------------------------------------------------------------------------- + + +class TestNeedsUtf8: + """Unit tests for _needs_utf8().""" + + @pytest.mark.parametrize( + "encoding, expected", + [ + ("utf-8", False), + ("UTF-8", False), + ("utf8", False), + ("UTF8", False), + ("cp1252", True), + ("ascii", True), + ("latin-1", True), + ("", True), + (None, True), + ], + ) + def test_known_encodings(self, encoding: str | None, expected: bool) -> None: + assert _needs_utf8(encoding) is expected + + +# --------------------------------------------------------------------------- +# _configure_windows_utf8() +# --------------------------------------------------------------------------- + + +class TestConfigureWindowsUtf8: + """Tests for _configure_windows_utf8().""" + + def test_noop_on_non_windows(self) -> None: + """On non-Windows platforms the function should be a no-op.""" + with patch.object(sys, "platform", "linux"): + # Should not raise and not change anything. + _configure_windows_utf8() + + def test_reconfigures_when_cp1252(self) -> None: + """Simulate a cp1252 stdout on 'win32' and verify reconfigure is called.""" + fake_stdout = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + fake_stderr = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + + with ( + patch.object(sys, "platform", "win32"), + patch.object(sys, "stdout", fake_stdout), + patch.object(sys, "stderr", fake_stderr), + ): + _configure_windows_utf8() + + # After reconfiguration the streams should be utf-8. + assert fake_stdout.encoding.lower().replace("-", "") == "utf8" + assert fake_stderr.encoding.lower().replace("-", "") == "utf8" + + def test_sets_pythonioencoding_env(self) -> None: + """When reconfiguring, PYTHONIOENCODING should be set as a fallback.""" + fake_stdout = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + fake_stderr = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + + env_copy = os.environ.copy() + env_copy.pop("PYTHONIOENCODING", None) + + with ( + patch.object(sys, "platform", "win32"), + patch.object(sys, "stdout", fake_stdout), + patch.object(sys, "stderr", fake_stderr), + patch.dict(os.environ, env_copy, clear=True), + ): + _configure_windows_utf8() + assert os.environ.get("PYTHONIOENCODING") == "utf-8" + + def test_does_not_override_existing_pythonioencoding(self) -> None: + """If PYTHONIOENCODING is already set, don't overwrite it.""" + fake_stdout = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + fake_stderr = io.TextIOWrapper(io.BytesIO(), encoding="cp1252") + + with ( + patch.object(sys, "platform", "win32"), + patch.object(sys, "stdout", fake_stdout), + patch.object(sys, "stderr", fake_stderr), + patch.dict(os.environ, {"PYTHONIOENCODING": "ascii"}, clear=False), + ): + _configure_windows_utf8() + # Should keep the existing value. + assert os.environ["PYTHONIOENCODING"] == "ascii" + + def test_no_crash_when_reconfigure_missing(self) -> None: + """Streams without a reconfigure method should not crash.""" + + class FakeStream: + encoding = "cp1252" + # No reconfigure attribute. + + with ( + patch.object(sys, "platform", "win32"), + patch.object(sys, "stdout", FakeStream()), + patch.object(sys, "stderr", FakeStream()), + ): + # Should not raise. + _configure_windows_utf8() + + def test_noop_when_already_utf8(self) -> None: + """If both streams are already utf-8, nothing should be reconfigured.""" + fake_stdout = io.TextIOWrapper(io.BytesIO(), encoding="utf-8") + fake_stderr = io.TextIOWrapper(io.BytesIO(), encoding="utf-8") + + reconfigure_called = False + original_reconfigure = fake_stdout.reconfigure + + def tracking_reconfigure(**kwargs): + nonlocal reconfigure_called + reconfigure_called = True + return original_reconfigure(**kwargs) + + fake_stdout.reconfigure = tracking_reconfigure # type: ignore[assignment] + + with ( + patch.object(sys, "platform", "win32"), + patch.object(sys, "stdout", fake_stdout), + patch.object(sys, "stderr", fake_stderr), + ): + _configure_windows_utf8() + assert not reconfigure_called, "reconfigure should not be called when already utf-8" + + def test_emoji_output_after_reconfigure(self) -> None: + """After reconfiguring a cp1252 stream, writing emoji should not crash.""" + buf = io.BytesIO() + fake_stdout = io.TextIOWrapper(buf, encoding="cp1252") + + with ( + patch.object(sys, "platform", "win32"), + patch.object(sys, "stdout", fake_stdout), + patch.object(sys, "stderr", fake_stdout), + ): + _configure_windows_utf8() + # Now write emoji — should not raise UnicodeEncodeError. + fake_stdout.write("Hello! \u2705 \U0001f680 Done.\n") + fake_stdout.flush() + + output = buf.getvalue().decode("utf-8") + assert "\u2705" in output + assert "\U0001f680" in output diff --git a/libs/tests/cli/test_cross_platform.py b/libs/tests/cli/test_cross_platform.py new file mode 100644 index 00000000..a9dd7854 --- /dev/null +++ b/libs/tests/cli/test_cross_platform.py @@ -0,0 +1,211 @@ +"""Focused cross-platform regression tests with minimal duplication. + +This module keeps only scenarios that are not already covered by dedicated +test files under ``libs/tests/cli``. +""" + +from __future__ import annotations + +import json +from io import StringIO +from pathlib import Path +from unittest.mock import patch + +import pytest + + +class TestUtf8FileIO: + """Verify UTF-8 behavior across file I/O paths that have regressed before.""" + + def test_config_model_save_uses_utf8(self, tmp_path: Path) -> None: + from arcade_core.config_model import Config + + config_dir = tmp_path / ".arcade" + config_dir.mkdir() + + with ( + patch.object( + Config, + "get_config_file_path", + return_value=config_dir / "credentials.yaml", + ), + patch.object(Config, "ensure_config_dir_exists"), + ): + cfg = Config(coordinator_url="https://café-coordinator.example.com") + cfg.save_to_file() + + raw = (config_dir / "credentials.yaml").read_bytes() + text = raw.decode("utf-8") + assert "caf" in text + assert "é" in text or "\\xe9" not in text + + def test_config_model_load_reads_utf8(self, tmp_path: Path) -> None: + import yaml + from arcade_core.config_model import Config + + config_dir = tmp_path / ".arcade" + config_dir.mkdir() + config_file = config_dir / "credentials.yaml" + + data = {"cloud": {"coordinator_url": "https://café.example.com"}} + config_file.write_text(yaml.dump(data), encoding="utf-8") + + with ( + patch.object(Config, "get_config_file_path", return_value=config_file), + patch.object(Config, "ensure_config_dir_exists"), + ): + loaded = Config.load_from_file() + + assert loaded.coordinator_url == "https://café.example.com" + + def test_config_model_permissions_no_crash_on_windows(self, tmp_path: Path) -> None: + from arcade_core.config_model import Config + + config_dir = tmp_path / ".arcade" + config_dir.mkdir() + config_file = config_dir / "credentials.yaml" + + with ( + patch.object(Config, "get_config_file_path", return_value=config_file), + patch.object(Config, "ensure_config_dir_exists"), + patch("arcade_core.config_model.os.name", "nt"), + patch("arcade_core.config_model.subprocess.run", side_effect=OSError("icacls failed")), + ): + cfg = Config(coordinator_url="https://test.example.com") + cfg.save_to_file() + + assert config_file.exists() + + def test_configure_client_writes_utf8( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + from arcade_cli.configure import configure_client + + monkeypatch.chdir(tmp_path) + entrypoint = tmp_path / "server.py" + entrypoint.write_text("print('ok')\n", encoding="utf-8") + + config_path = tmp_path / "test_config.json" + configure_client( + client="cursor", + entrypoint_file="server.py", + server_name="café-server", + transport="stdio", + host="local", + port=8000, + config_path=config_path, + ) + + raw = config_path.read_bytes() + assert not raw.startswith(b"\xef\xbb\xbf") + text = raw.decode("utf-8") + data = json.loads(text) + assert "café-server" in data["mcpServers"] + + def test_load_env_file_reads_utf8(self, tmp_path: Path) -> None: + from arcade_cli.secret import load_env_file + + env_file = tmp_path / ".env" + env_file.write_text("KEY1=café\nKEY2=naïve\n", encoding="utf-8") + + secrets = load_env_file(str(env_file)) + assert secrets["KEY1"] == "café" + assert secrets["KEY2"] == "naïve" + + def test_identity_write_atomic_uses_utf8(self, tmp_path: Path) -> None: + from arcade_core.usage.identity import UsageIdentity + + config_path = tmp_path / ".arcade" + config_path.mkdir() + + with patch("arcade_core.usage.identity.ARCADE_CONFIG_PATH", str(config_path)): + identity = UsageIdentity() + identity.usage_file_path = str(config_path / "usage.json") + identity._write_atomic({"anon_id": "test-ñ-123", "linked_principal_id": None}) + + raw = (config_path / "usage.json").read_bytes() + text = raw.decode("utf-8") + data = json.loads(text) + assert data["anon_id"] == "test-ñ-123" + + def test_utils_load_dotenv_reads_utf8(self, tmp_path: Path) -> None: + from arcade_cli.utils import load_dotenv + + env_file = tmp_path / ".env" + env_file.write_text("DB_PASSWORD=pässwörd\n", encoding="utf-8") + + result = load_dotenv(env_file, override=False) + assert result.get("DB_PASSWORD") == "pässwörd" + + def test_new_toolkit_files_are_utf8(self, tmp_path: Path) -> None: + from arcade_cli.new import create_new_toolkit_minimal + + output_dir = tmp_path / "scaffolded" + output_dir.mkdir() + create_new_toolkit_minimal(str(output_dir), "my_server") + + server_py = output_dir / "my_server" / "src" / "my_server" / "server.py" + assert server_py.exists() + content = server_py.read_bytes().decode("utf-8") + assert len(content) > 0 + + +class TestFileLockingErrorHandling: + """Verify graceful handling for Windows-style file-locking scenarios.""" + + def test_logout_handles_permission_error(self) -> None: + with ( + patch("arcade_cli.main.os.path.exists", return_value=True), + patch("arcade_cli.main.os.remove", side_effect=PermissionError("Locked")), + patch("arcade_cli.main.handle_cli_error") as mock_error, + ): + from arcade_cli.main import logout + + try: + logout(debug=False) + except SystemExit: + pass + + mock_error.assert_called_once() + message = mock_error.call_args[0][0].lower() + assert "in use" in message or "lock" in message + + def test_logout_permission_error_does_not_double_report(self) -> None: + from arcade_cli.utils import CLIError + + with ( + patch("arcade_cli.main.os.path.exists", return_value=True), + patch("arcade_cli.main.os.remove", side_effect=PermissionError("Locked")), + patch("arcade_cli.main.handle_cli_error", side_effect=CLIError("locked")) as mock_error, + ): + from arcade_cli.main import logout + + with pytest.raises(CLIError): + logout(debug=False) + + mock_error.assert_called_once() + + def test_remove_toolkit_handles_permission_error(self, tmp_path: Path) -> None: + from arcade_cli.new import remove_toolkit + from rich.console import Console + + toolkit_path = tmp_path / "locked_toolkit" + toolkit_path.mkdir() + (toolkit_path / "file.py").write_text("x", encoding="utf-8") + + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + + import arcade_cli.new as new_mod + + original_console = new_mod.console + new_mod.console = test_console + + try: + with patch("arcade_cli.new.shutil.rmtree", side_effect=PermissionError("Locked")): + remove_toolkit(tmp_path, "locked_toolkit") + finally: + new_mod.console = original_console + + output = buf.getvalue() + assert "Warning" in output or "Could not" in output diff --git a/libs/tests/cli/test_dashboard.py b/libs/tests/cli/test_dashboard.py index 91602d84..b8475b6f 100644 --- a/libs/tests/cli/test_dashboard.py +++ b/libs/tests/cli/test_dashboard.py @@ -23,7 +23,7 @@ runner = CliRunner() def test_dashboard_url_construction(args, expected_url): """Test that the dashboard command constructs the correct URL with various args.""" with ( - patch("webbrowser.open") as mock_open, + patch("arcade_cli.main._open_browser") as mock_open, patch("arcade_cli.utils.validate_and_get_config") as mock_validate, patch("arcade_cli.main.log_engine_health") as mock_health_check, ): @@ -41,9 +41,9 @@ def test_dashboard_url_construction(args, expected_url): def test_fallback_when_browser_fails(): - """Test fallback message when browser.open fails.""" + """Test fallback message when _open_browser fails.""" with ( - patch("webbrowser.open") as mock_open, + patch("arcade_cli.main._open_browser") as mock_open, patch("arcade_cli.utils.validate_and_get_config") as mock_validate, patch("arcade_cli.main.log_engine_health") as mock_health_check, patch("arcade_cli.main.console.print") as mock_print, @@ -55,16 +55,20 @@ def test_fallback_when_browser_fails(): result = runner.invoke(cli, ["dashboard"]) assert result.exit_code == 0 - mock_print.assert_any_call( - f"If a browser doesn't open automatically, copy this URL and paste it into your browser: https://{PROD_ENGINE_HOST}/dashboard", - style="dim", + # The fallback message should mention the URL and hint about manual paste. + fallback_calls = [ + call for call in mock_print.call_args_list + if "browser" in str(call).lower() and "dashboard" in str(call).lower() + ] + assert len(fallback_calls) >= 1, ( + f"Expected a fallback message about browser. Got calls: {mock_print.call_args_list}" ) def test_health_check_success(): """Test successful health check.""" with ( - patch("webbrowser.open") as mock_open, + patch("arcade_cli.main._open_browser") as mock_open, patch("arcade_cli.utils.validate_and_get_config") as mock_validate, patch("arcade_cli.main.log_engine_health") as mock_health_check, ): diff --git a/libs/tests/cli/test_display.py b/libs/tests/cli/test_display.py index 7d0d6ee5..3e2e80aa 100644 --- a/libs/tests/cli/test_display.py +++ b/libs/tests/cli/test_display.py @@ -4,6 +4,13 @@ from unittest.mock import Mock import pytest from arcade_cli.display import display_eval_results + +# test_display exercises eval-formatting paths that depend on optional extras. +# Skip cleanly when those extras are not installed in the active environment. +pytest.importorskip("openai") +pytest.importorskip("pytz") +pytest.importorskip("numpy") +pytest.importorskip("scipy") from arcade_evals.eval import EvaluationResult # Mark all tests in this module as requiring evals dependencies @@ -118,7 +125,7 @@ def test_display_eval_results_with_output_file() -> None: assert output_file.exists() # Verify file contains some expected content - content = output_file.read_text() + content = output_file.read_text(encoding="utf-8") assert "Model:" in content or "gpt-4o" in content @@ -160,7 +167,7 @@ def test_display_eval_results_with_output_file_and_failed_only() -> None: assert output_file.exists() # Verify file contains disclaimer and summary - content = output_file.read_text() + content = output_file.read_text(encoding="utf-8") assert "failed-only" in content.lower() or "failed evaluation" in content.lower() assert "Total: 5" in content # Should show original total @@ -515,7 +522,7 @@ def test_display_eval_results_failed_only_with_warnings_in_summary() -> None: output_formats=["txt"], ) - content = output_file.read_text() + content = output_file.read_text(encoding="utf-8") # Should show warnings in summary assert "Warnings: 1" in content or "Warnings" in content @@ -562,7 +569,7 @@ def test_display_eval_results_with_details_and_output() -> None: ) assert output_file.exists() - content = output_file.read_text() + content = output_file.read_text(encoding="utf-8") assert "User Input:" in content assert "Details:" in content @@ -603,11 +610,11 @@ def test_display_eval_results_multi_format_output() -> None: assert (Path(tmpdir) / "results.html").exists() # Verify each file has appropriate content - txt_content = (Path(tmpdir) / "results.txt").read_text() + txt_content = (Path(tmpdir) / "results.txt").read_text(encoding="utf-8") assert "Test Case" in txt_content - md_content = (Path(tmpdir) / "results.md").read_text() + md_content = (Path(tmpdir) / "results.md").read_text(encoding="utf-8") assert "# " in md_content # Markdown header - html_content = (Path(tmpdir) / "results.html").read_text() + html_content = (Path(tmpdir) / "results.html").read_text(encoding="utf-8") assert " None: + eval_root = tmp_path / "eval dir with spaces" + eval_root.mkdir() + + file_one = eval_root / "eval_one.py" + file_one.write_text("print('one')\n", encoding="utf-8") + + nested = eval_root / "nested dir" + nested.mkdir() + file_two = nested / "eval_two.py" + file_two.write_text("print('two')\n", encoding="utf-8") + + results = get_eval_files(str(eval_root)) + resolved = {p.resolve() for p in results} + + assert file_one.resolve() in resolved + assert file_two.resolve() in resolved diff --git a/libs/tests/cli/test_new_cli.py b/libs/tests/cli/test_new_cli.py new file mode 100644 index 00000000..0702d2de --- /dev/null +++ b/libs/tests/cli/test_new_cli.py @@ -0,0 +1,62 @@ +from io import StringIO +from pathlib import Path + +import pytest +from arcade_cli.new import create_new_toolkit_minimal +from rich.console import Console + + +def test_create_new_toolkit_minimal_with_spaces(tmp_path: Path) -> None: + output_dir = tmp_path / "dir with spaces" + output_dir.mkdir() + + create_new_toolkit_minimal(str(output_dir), "my_server") + + server_root = output_dir / "my_server" + assert (server_root / "pyproject.toml").is_file() + assert (server_root / "src" / "my_server" / "server.py").is_file() + assert (server_root / "src" / "my_server" / ".env.example").is_file() + + +def test_create_new_toolkit_minimal_prints_next_steps(tmp_path: Path) -> None: + """After scaffolding, the CLI should print 'Next steps' guidance.""" + output_dir = tmp_path / "scaffold_test" + output_dir.mkdir() + + # Capture console output by replacing the module-level console. + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + + import arcade_cli.new as new_mod + + orig = new_mod.console + new_mod.console = test_console + try: + create_new_toolkit_minimal(str(output_dir), "demo_srv") + finally: + new_mod.console = orig + + output = buf.getvalue() + assert "Next steps:" in output, f"Expected 'Next steps:' in output:\n{output}" + assert "uv run server.py" in output, f"Expected 'uv run server.py' in output:\n{output}" + assert "demo_srv" in output, f"Expected toolkit name in output:\n{output}" + + +def test_create_new_toolkit_minimal_rejects_duplicate(tmp_path: Path) -> None: + """Creating a toolkit with a name that already exists should raise.""" + output_dir = tmp_path / "dup_test" + output_dir.mkdir() + + create_new_toolkit_minimal(str(output_dir), "my_srv") + + with pytest.raises(FileExistsError, match="already exists"): + create_new_toolkit_minimal(str(output_dir), "my_srv") + + +def test_create_new_toolkit_minimal_rejects_invalid_name(tmp_path: Path) -> None: + """Toolkit names with invalid characters should raise ValueError.""" + output_dir = tmp_path / "invalid_test" + output_dir.mkdir() + + with pytest.raises(ValueError, match="illegal characters"): + create_new_toolkit_minimal(str(output_dir), "My-Server!") diff --git a/libs/tests/cli/test_secret.py b/libs/tests/cli/test_secret.py index 2ef4ca05..ecb0e430 100644 --- a/libs/tests/cli/test_secret.py +++ b/libs/tests/cli/test_secret.py @@ -62,7 +62,7 @@ KEY2=value2 # This is a comment KEY3=value3 """ - with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False, encoding="utf-8") as f: f.write(env_content) f.flush() @@ -82,7 +82,7 @@ KEY2='single quoted' KEY3="value with = sign" KEY4="value with # comment inside" """ - with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False, encoding="utf-8") as f: f.write(env_content) f.flush() @@ -102,7 +102,7 @@ KEY1=value1 # inline comment KEY2="quoted value" # comment after quote KEY3=value3# no space before comment """ - with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False, encoding="utf-8") as f: f.write(env_content) f.flush() @@ -126,7 +126,7 @@ KEY3=value3 invalid_line_without_equals KEY4=value4 """ - with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False) as f: + with tempfile.NamedTemporaryFile(mode="w", suffix=".env", delete=False, encoding="utf-8") as f: f.write(env_content) f.flush() diff --git a/libs/tests/cli/test_server_logs.py b/libs/tests/cli/test_server_logs.py new file mode 100644 index 00000000..f7e720e0 --- /dev/null +++ b/libs/tests/cli/test_server_logs.py @@ -0,0 +1,102 @@ +from __future__ import annotations + +import asyncio +from datetime import datetime +from io import StringIO +from unittest.mock import MagicMock, patch + +from arcade_cli.server import _display_deployment_logs, _stream_deployment_logs +from rich.console import Console + + +def test_display_deployment_logs_preserves_square_bracket_content() -> None: + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + + mock_response = MagicMock() + mock_response.raise_for_status.return_value = None + mock_response.json.return_value = [ + {"timestamp": "2026-01-15T15:30:00Z", "line": "[INFO] startup [ERROR] details"} + ] + mock_client = MagicMock() + mock_client.get.return_value = mock_response + + import arcade_cli.server as server_mod + + original_console = server_mod.console + server_mod.console = test_console + try: + with ( + patch("arcade_cli.server.httpx.Client") as mock_httpx_client, + patch("arcade_cli.server._format_timestamp_to_local", return_value="2026-01-15 10:30:00 EST"), + ): + mock_httpx_client.return_value.__enter__.return_value = mock_client + _display_deployment_logs( + "http://localhost:8123/logs", + {}, + datetime(2026, 1, 15, 15, 30, 0), + datetime(2026, 1, 15, 15, 35, 0), + debug=False, + ) + finally: + server_mod.console = original_console + + output = buf.getvalue() + assert "[2026-01-15 10:30:00 EST]" in output + assert "[INFO]" in output + assert "[ERROR]" in output + + +def test_stream_deployment_logs_preserves_square_bracket_content() -> None: + buf = StringIO() + test_console = Console(file=buf, force_terminal=False) + + class FakeStreamResponse: + async def __aenter__(self) -> FakeStreamResponse: + return self + + async def __aexit__(self, exc_type, exc, tb) -> None: # type: ignore[no-untyped-def] + return None + + def raise_for_status(self) -> None: + return None + + async def aiter_lines(self): # type: ignore[no-untyped-def] + yield 'data: {"Timestamp":"2026-01-15T15:30:00Z","Line":"[ERROR] stream details"}' + yield "[INFO] plain stream line" + + class FakeAsyncClient: + async def __aenter__(self) -> FakeAsyncClient: + return self + + async def __aexit__(self, exc_type, exc, tb) -> None: # type: ignore[no-untyped-def] + return None + + def stream(self, *args, **kwargs) -> FakeStreamResponse: # type: ignore[no-untyped-def] + return FakeStreamResponse() + + import arcade_cli.server as server_mod + + original_console = server_mod.console + server_mod.console = test_console + try: + with ( + patch("arcade_cli.server.httpx.AsyncClient", return_value=FakeAsyncClient()), + patch("arcade_cli.server._format_timestamp_to_local", return_value="2026-01-15 10:30:00 EST"), + ): + asyncio.run( + _stream_deployment_logs( + "http://localhost:8123/logs/stream", + {}, + datetime(2026, 1, 15, 15, 30, 0), + datetime(2026, 1, 15, 15, 35, 0), + debug=False, + ) + ) + finally: + server_mod.console = original_console + + output = buf.getvalue() + assert "[2026-01-15 10:30:00 EST]" in output + assert "[ERROR]" in output + assert "[INFO] plain stream line" in output diff --git a/libs/tests/cli/test_stdio_signal.py b/libs/tests/cli/test_stdio_signal.py new file mode 100644 index 00000000..abd2b007 --- /dev/null +++ b/libs/tests/cli/test_stdio_signal.py @@ -0,0 +1,218 @@ +"""Tests for Windows signal handling in stdio transport. + +Verifies that: +- The signal-handler support message is suppressed on Windows. +- No noisy "Failed to set up signal handler" warning is logged on Windows. +- A stdlib signal.signal(SIGINT) fallback is registered on Windows. +""" + +from __future__ import annotations + +import asyncio +import logging +import sys +from collections.abc import Callable, Coroutine +from typing import Any +from unittest.mock import patch + +import pytest + + +@pytest.mark.asyncio +async def test_signal_handler_support_message_is_suppressed_on_windows() -> None: + """On Windows, don't log a user-facing signal-support message.""" + from arcade_mcp_server.transports.stdio import StdioTransport + + transport = StdioTransport(name="test-stdio") + + log_records: list[logging.LogRecord] = [] + + class RecordHandler(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + log_records.append(record) + + logger = logging.getLogger("arcade.mcp.transports.stdio") + handler = RecordHandler() + logger.addHandler(handler) + original_level = logger.level + logger.setLevel(logging.DEBUG) + + try: + with patch.object(sys, "platform", "win32"): + # Simulate the NotImplementedError that Windows raises for + # loop.add_signal_handler. + loop = asyncio.get_running_loop() + original_add = loop.add_signal_handler + + def raise_not_impl(*args, **kwargs): + raise NotImplementedError + + loop.add_signal_handler = raise_not_impl # type: ignore[assignment] + + # Also mock signal.signal so we don't actually install a handler + with patch("arcade_mcp_server.transports.stdio.signal.signal"): + try: + await transport.start() + finally: + loop.add_signal_handler = original_add # type: ignore[assignment] + await transport.stop() + + messages = [r.getMessage() for r in log_records] + assert not any("Windows does not support asyncio signal handlers" in m for m in messages), ( + "Windows signal support message should be suppressed." + ) + finally: + logger.removeHandler(handler) + logger.setLevel(original_level) + + +@pytest.mark.asyncio +async def test_signal_handler_no_failed_setup_warning_on_windows() -> None: + """On Windows, avoid warning noise when asyncio signal handlers are unavailable.""" + from arcade_mcp_server.transports.stdio import StdioTransport + + transport = StdioTransport(name="test-stdio-once") + + log_records: list[logging.LogRecord] = [] + + class RecordHandler(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + log_records.append(record) + + logger = logging.getLogger("arcade.mcp.transports.stdio") + handler = RecordHandler() + logger.addHandler(handler) + original_level = logger.level + logger.setLevel(logging.DEBUG) + + try: + with patch.object(sys, "platform", "win32"): + loop = asyncio.get_running_loop() + original_add = loop.add_signal_handler + + def raise_not_impl(*args, **kwargs): + raise NotImplementedError + + loop.add_signal_handler = raise_not_impl # type: ignore[assignment] + + with patch("arcade_mcp_server.transports.stdio.signal.signal"): + try: + await transport.start() + finally: + loop.add_signal_handler = original_add # type: ignore[assignment] + await transport.stop() + + failed_setup_warnings = [ + r for r in log_records if "Failed to set up signal handler" in r.getMessage() + ] + assert len(failed_setup_warnings) == 0, ( + "Should not emit setup warnings for expected Windows asyncio limitations." + ) + finally: + logger.removeHandler(handler) + logger.setLevel(original_level) + + +@pytest.mark.asyncio +async def test_signal_signal_fallback_registered_on_windows() -> None: + """On Windows, signal.signal(SIGINT, ...) should be called as a fallback.""" + from arcade_mcp_server.transports.stdio import StdioTransport + + transport = StdioTransport(name="test-stdio-fallback") + + log_records: list[logging.LogRecord] = [] + + class RecordHandler(logging.Handler): + def emit(self, record: logging.LogRecord) -> None: + log_records.append(record) + + logger = logging.getLogger("arcade.mcp.transports.stdio") + handler = RecordHandler() + logger.addHandler(handler) + original_level = logger.level + logger.setLevel(logging.DEBUG) + + try: + with patch.object(sys, "platform", "win32"): + loop = asyncio.get_running_loop() + original_add = loop.add_signal_handler + + def raise_not_impl(*args, **kwargs): + raise NotImplementedError + + loop.add_signal_handler = raise_not_impl # type: ignore[assignment] + + with patch("arcade_mcp_server.transports.stdio.signal.signal") as mock_signal: + try: + await transport.start() + finally: + loop.add_signal_handler = original_add # type: ignore[assignment] + await transport.stop() + + # signal.signal should have been called with SIGINT + import signal + sigint_calls = [ + c for c in mock_signal.call_args_list + if c[0][0] == signal.SIGINT + ] + assert len(sigint_calls) == 1, ( + f"Expected signal.signal(SIGINT, ...) to be called once. " + f"All calls: {mock_signal.call_args_list}" + ) + finally: + logger.removeHandler(handler) + logger.setLevel(original_level) + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows-only SIGINT fallback behavior") +@pytest.mark.asyncio +async def test_windows_sigint_fallback_schedules_stop_on_transport_loop() -> None: + """Windows SIGINT fallback should schedule stop() on the captured event loop.""" + import signal + + import arcade_mcp_server.transports.stdio as stdio_mod + from arcade_mcp_server.transports.stdio import StdioTransport + + transport = StdioTransport(name="test-stdio-loop-schedule") + registered_handlers: dict[int, Callable[[int, object], None]] = {} + scheduled_callbacks: list[Callable[[], None]] = [] + created_coroutines: list[Coroutine[Any, Any, None]] = [] + + def capture_signal(signum: int, handler: Callable[[int, object], None]) -> None: + registered_handlers[signum] = handler + + def capture_call_soon_threadsafe(callback: Callable[[], None], *args: object) -> None: + assert not args + scheduled_callbacks.append(callback) + + def capture_create_task(coro: Coroutine[Any, Any, None]) -> object: + created_coroutines.append(coro) + coro.close() + return object() + + 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[method-assign] + with ( + patch.object(loop, "call_soon_threadsafe", side_effect=capture_call_soon_threadsafe), + patch.object(loop, "create_task", side_effect=capture_create_task), + patch.object(stdio_mod.signal, "signal", side_effect=capture_signal), + ): + try: + await transport.start() + + handler = registered_handlers[signal.SIGINT] + handler(signal.SIGINT, object()) + + assert len(scheduled_callbacks) == 1 + scheduled_callback = scheduled_callbacks[0] + + scheduled_callback() + assert len(created_coroutines) == 1 + finally: + loop.add_signal_handler = original_add # type: ignore[method-assign] + await transport.stop() diff --git a/libs/tests/cli/test_utils_multi_provider.py b/libs/tests/cli/test_utils_multi_provider.py index 3c8fab8f..2ed4ba34 100644 --- a/libs/tests/cli/test_utils_multi_provider.py +++ b/libs/tests/cli/test_utils_multi_provider.py @@ -1,6 +1,7 @@ """Tests for multi-provider utils functions.""" import os +from pathlib import Path from unittest.mock import patch import pytest @@ -365,7 +366,17 @@ class TestParseOutputPaths: def test_path_with_directory(self) -> None: """Test parsing path with directory.""" base, formats = parse_output_paths(["output/results.json"]) - assert base == "output/results" + # Path separator is OS-dependent; use Path to build the expected value. + assert base == str(Path("output") / "results") + assert formats == ["json"] + + def test_path_with_spaces(self, tmp_path: Path) -> None: + """Test parsing path with spaces.""" + output_dir = tmp_path / "dir with spaces" + output_dir.mkdir() + output_path = output_dir / "results.json" + base, formats = parse_output_paths([str(output_path)]) + assert base == str(output_dir / "results") assert formats == ["json"] def test_none_returns_empty(self) -> None: diff --git a/libs/tests/cli/test_windows_subprocess.py b/libs/tests/cli/test_windows_subprocess.py new file mode 100644 index 00000000..1bbd0f20 --- /dev/null +++ b/libs/tests/cli/test_windows_subprocess.py @@ -0,0 +1,296 @@ +"""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}" + ) diff --git a/libs/tests/cli/usage/test_identity.py b/libs/tests/cli/usage/test_identity.py index 1dd6e64e..ab05510f 100644 --- a/libs/tests/cli/usage/test_identity.py +++ b/libs/tests/cli/usage/test_identity.py @@ -51,7 +51,7 @@ class TestLoadOrCreate: """Test that load_or_create loads existing usage.json file.""" existing_data = {"anon_id": str(uuid.uuid4()), "linked_principal_id": "user-123"} usage_file = temp_config_path / "usage.json" - usage_file.write_text(json.dumps(existing_data)) + usage_file.write_text(json.dumps(existing_data), encoding="utf-8") data = identity.load_or_create() @@ -71,7 +71,7 @@ class TestLoadOrCreate: ) -> None: """Test that load_or_create creates new data if JSON is corrupted.""" usage_file = temp_config_path / "usage.json" - usage_file.write_text("{ invalid json }") + usage_file.write_text("{ invalid json }", encoding="utf-8") data = identity.load_or_create() @@ -83,7 +83,7 @@ class TestLoadOrCreate: ) -> None: """Test that load_or_create creates new data if anon_id is missing.""" usage_file = temp_config_path / "usage.json" - usage_file.write_text(json.dumps({"some_other_key": "value"})) + usage_file.write_text(json.dumps({"some_other_key": "value"}), encoding="utf-8") data = identity.load_or_create() @@ -95,7 +95,7 @@ class TestLoadOrCreate: ) -> None: """Test that load_or_create creates new data if JSON is not a dict.""" usage_file = temp_config_path / "usage.json" - usage_file.write_text(json.dumps(["not", "a", "dict"])) + usage_file.write_text(json.dumps(["not", "a", "dict"]), encoding="utf-8") data = identity.load_or_create() @@ -147,7 +147,8 @@ class TestGetDistinctId: """Test that get_distinct_id returns persisted linked_principal_id.""" usage_file = temp_config_path / "usage.json" usage_file.write_text( - json.dumps({"anon_id": str(uuid.uuid4()), "linked_principal_id": "persisted-user-123"}) + json.dumps({"anon_id": str(uuid.uuid4()), "linked_principal_id": "persisted-user-123"}), + encoding="utf-8", ) distinct_id = identity.get_distinct_id() @@ -195,7 +196,7 @@ class TestGetPrincipalId: """Test that get_principal_id returns principal_id from API.""" # Create credentials file credentials_file = temp_config_path / "credentials.yaml" - credentials_file.write_text(yaml.dump({"cloud": {"api": {"key": "test-api-key"}}})) + credentials_file.write_text(yaml.dump({"cloud": {"api": {"key": "test-api-key"}}}), encoding="utf-8") # Mock API response mock_response = MagicMock() @@ -218,7 +219,7 @@ class TestGetPrincipalId: ) -> None: """Test that get_principal_id returns None on API failure.""" credentials_file = temp_config_path / "credentials.yaml" - credentials_file.write_text(yaml.dump({"cloud": {"api": {"key": "test-api-key"}}})) + credentials_file.write_text(yaml.dump({"cloud": {"api": {"key": "test-api-key"}}}), encoding="utf-8") mock_get.side_effect = Exception("Network error") @@ -231,7 +232,7 @@ class TestGetPrincipalId: ) -> None: """Test that get_principal_id returns None when API key is missing.""" credentials_file = temp_config_path / "credentials.yaml" - credentials_file.write_text(yaml.dump({"cloud": {}})) + credentials_file.write_text(yaml.dump({"cloud": {}}), encoding="utf-8") principal_id = identity.get_principal_id() @@ -243,7 +244,7 @@ class TestGetPrincipalId: ) -> None: """Test that get_principal_id returns None on non-200 status code.""" credentials_file = temp_config_path / "credentials.yaml" - credentials_file.write_text(yaml.dump({"cloud": {"api": {"key": "test-api-key"}}})) + credentials_file.write_text(yaml.dump({"cloud": {"api": {"key": "test-api-key"}}}), encoding="utf-8") mock_response = MagicMock() mock_response.status_code = 401 @@ -260,7 +261,8 @@ class TestGetPrincipalId: """Test that get_principal_id returns account_id using OAuth access token.""" credentials_file = temp_config_path / "credentials.yaml" credentials_file.write_text( - yaml.dump({"cloud": {"auth": {"access_token": "oauth-token", "refresh_token": "x"}}}) + yaml.dump({"cloud": {"auth": {"access_token": "oauth-token", "refresh_token": "x"}}}), + encoding="utf-8", ) mock_response = MagicMock() @@ -302,7 +304,8 @@ class TestShouldAlias: usage_file = temp_config_path / "usage.json" usage_file.write_text( - json.dumps({"anon_id": str(uuid.uuid4()), "linked_principal_id": principal_id}) + json.dumps({"anon_id": str(uuid.uuid4()), "linked_principal_id": principal_id}), + encoding="utf-8", ) should_alias = identity.should_alias() diff --git a/libs/tests/core/test_dependency_alignment.py b/libs/tests/core/test_dependency_alignment.py new file mode 100644 index 00000000..6c65e8d2 --- /dev/null +++ b/libs/tests/core/test_dependency_alignment.py @@ -0,0 +1,49 @@ +from __future__ import annotations + +from pathlib import Path + +import toml +from packaging.requirements import Requirement +from packaging.version import Version + +REPO_ROOT = Path(__file__).resolve().parents[3] + + +def _load_pyproject(path: Path) -> dict: + return toml.load(path) + + +def _get_requirement(dependencies: list[str], package_name: str) -> Requirement: + for dep in dependencies: + req = Requirement(dep) + if req.name == package_name: + return req + raise AssertionError(f"Missing dependency for {package_name!r}") + + +def test_root_dependency_includes_workspace_arcade_core_version() -> None: + root_pyproject = _load_pyproject(REPO_ROOT / "pyproject.toml") + core_pyproject = _load_pyproject(REPO_ROOT / "libs/arcade-core/pyproject.toml") + + root_deps: list[str] = root_pyproject["project"]["dependencies"] + core_version = Version(core_pyproject["project"]["version"]) + core_req = _get_requirement(root_deps, "arcade-core") + + assert core_version in core_req.specifier, ( + "Root dependency constraint for arcade-core must include the current " + f"workspace version {core_version}; got {core_req.specifier!s}" + ) + + +def test_root_dependency_includes_workspace_arcade_mcp_server_version() -> None: + root_pyproject = _load_pyproject(REPO_ROOT / "pyproject.toml") + server_pyproject = _load_pyproject(REPO_ROOT / "libs/arcade-mcp-server/pyproject.toml") + + root_deps: list[str] = root_pyproject["project"]["dependencies"] + server_version = Version(server_pyproject["project"]["version"]) + server_req = _get_requirement(root_deps, "arcade-mcp-server") + + assert server_version in server_req.specifier, ( + "Root dependency constraint for arcade-mcp-server must include the current " + f"workspace version {server_version}; got {server_req.specifier!s}" + ) diff --git a/libs/tests/core/test_subprocess_utils.py b/libs/tests/core/test_subprocess_utils.py new file mode 100644 index 00000000..c4624bc8 --- /dev/null +++ b/libs/tests/core/test_subprocess_utils.py @@ -0,0 +1,126 @@ +from __future__ import annotations + +import signal +import subprocess +import sys +from unittest.mock import MagicMock, patch + +from arcade_core.subprocess_utils import ( + build_windows_hidden_startupinfo, + get_windows_no_window_creationflags, + graceful_terminate_process, +) + + +class _DummyStartupInfo: + def __init__(self) -> None: + self.dwFlags = 0 + self.wShowWindow = 1 + + +def test_creationflags_return_zero_on_non_windows() -> None: + with patch.object(sys, "platform", "linux"): + assert get_windows_no_window_creationflags() == 0 + assert get_windows_no_window_creationflags(new_process_group=True) == 0 + + +def test_creationflags_windows_include_no_window_flag() -> None: + create_no_window = 0x08000000 + + with ( + patch.object(sys, "platform", "win32"), + patch.object(subprocess, "CREATE_NO_WINDOW", create_no_window, create=True), + ): + flags = get_windows_no_window_creationflags() + + assert flags == create_no_window + + +def test_creationflags_windows_can_include_new_process_group() -> None: + create_no_window = 0x08000000 + create_new_group = 0x00000200 + + with ( + patch.object(sys, "platform", "win32"), + patch.object(subprocess, "CREATE_NO_WINDOW", create_no_window, create=True), + patch.object(subprocess, "CREATE_NEW_PROCESS_GROUP", create_new_group, create=True), + ): + flags = get_windows_no_window_creationflags(new_process_group=True) + + assert flags & create_no_window + assert flags & create_new_group + + +def test_hidden_startupinfo_returns_none_on_non_windows() -> None: + with patch.object(sys, "platform", "linux"): + assert build_windows_hidden_startupinfo() is None + + +def test_hidden_startupinfo_sets_sw_hide_on_windows() -> None: + startf_use_show_window = 0x00000001 + + with ( + patch.object(sys, "platform", "win32"), + patch.object(subprocess, "STARTUPINFO", _DummyStartupInfo, create=True), + patch.object(subprocess, "STARTF_USESHOWWINDOW", startf_use_show_window, create=True), + ): + startupinfo = build_windows_hidden_startupinfo() + + assert startupinfo is not None + assert startupinfo.wShowWindow == 0 + assert startupinfo.dwFlags & startf_use_show_window + + +def test_hidden_startupinfo_returns_none_if_startupinfo_missing() -> None: + with ( + patch.object(sys, "platform", "win32"), + patch.object(subprocess, "STARTUPINFO", None, create=True), + ): + assert build_windows_hidden_startupinfo() is None + + +def test_graceful_terminate_uses_ctrl_break_on_windows() -> None: + ctrl_break_event = 1 + process = MagicMock() + + with ( + patch.object(sys, "platform", "win32"), + patch.object(signal, "CTRL_BREAK_EVENT", ctrl_break_event, create=True), + ): + graceful_terminate_process(process) + + process.send_signal.assert_called_once_with(ctrl_break_event) + process.terminate.assert_not_called() + + +def test_graceful_terminate_falls_back_to_terminate_on_windows_signal_error() -> None: + ctrl_break_event = 1 + process = MagicMock() + process.send_signal.side_effect = OSError("already exited") + + with ( + patch.object(sys, "platform", "win32"), + patch.object(signal, "CTRL_BREAK_EVENT", ctrl_break_event, create=True), + ): + graceful_terminate_process(process) + + process.send_signal.assert_called_once_with(ctrl_break_event) + process.terminate.assert_called_once() + + +def test_graceful_terminate_calls_terminate_on_non_windows() -> None: + process = MagicMock() + + with patch.object(sys, "platform", "linux"): + graceful_terminate_process(process) + + process.send_signal.assert_not_called() + process.terminate.assert_called_once() + + +def test_graceful_terminate_swallows_terminate_oserror() -> None: + process = MagicMock() + process.terminate.side_effect = OSError("already exited") + + with patch.object(sys, "platform", "linux"): + graceful_terminate_process(process) diff --git a/libs/tests/core/test_toolkit.py b/libs/tests/core/test_toolkit.py index 3c362791..84881a31 100644 --- a/libs/tests/core/test_toolkit.py +++ b/libs/tests/core/test_toolkit.py @@ -366,7 +366,7 @@ class TestValidateFile: """Test validation of Python files with valid syntax.""" # Create a temporary valid Python file valid_file = Path("valid.py") - valid_file.write_text("def test(): return True") + valid_file.write_text("def test(): return True", encoding="utf-8") # Should not raise any exceptions Toolkit.validate_file(valid_file) @@ -384,7 +384,7 @@ class TestValidateFile: def test_validate_tools_non_python_file(self): """Test validation with non-Python file.""" txt_file = Path("test.txt") - txt_file.write_text("Not a Python file") + txt_file.write_text("Not a Python file", encoding="utf-8") with pytest.raises(ValueError, match="Not a Python file"): Toolkit.validate_file(txt_file) @@ -393,7 +393,7 @@ class TestValidateFile: def test_validate_tools_syntax_error(self): """Test validation with Python file containing syntax errors.""" invalid_file = Path("invalid.py") - invalid_file.write_text("def test(): return True:") # Invalid syntax + invalid_file.write_text("def test(): return True:", encoding="utf-8") # Invalid syntax with pytest.raises(SyntaxError): Toolkit.validate_file(invalid_file) @@ -516,7 +516,7 @@ class TestToolsFromDirectory: package_dir = tmp_path / "mypackage" package_dir.mkdir() - (package_dir / "__init__.py").write_text("") + (package_dir / "__init__.py").write_text("", encoding="utf-8") (package_dir / "entrypoint.py").write_text( ''' from arcade_mcp_server import tool @@ -525,11 +525,12 @@ from arcade_mcp_server import tool def my_tool(): """A tool.""" pass -''' +''', + encoding="utf-8", ) tools_dir = package_dir / "tools" tools_dir.mkdir() - (tools_dir / "__init__.py").write_text("") + (tools_dir / "__init__.py").write_text("", encoding="utf-8") (tools_dir / "helper.py").write_text( ''' from arcade_mcp_server import tool @@ -538,7 +539,8 @@ from arcade_mcp_server import tool def helper_tool(): """A helper tool.""" pass -''' +''', + encoding="utf-8", ) return package_dir diff --git a/libs/tests/core/usage/test_usage_service.py b/libs/tests/core/usage/test_usage_service.py new file mode 100644 index 00000000..bd828e3c --- /dev/null +++ b/libs/tests/core/usage/test_usage_service.py @@ -0,0 +1,152 @@ +from __future__ import annotations + +import subprocess +import sys +from pathlib import PureWindowsPath +from unittest.mock import patch + +from arcade_core.usage.usage_service import UsageService + + +class _DummyStartupInfo: + def __init__(self) -> None: + self.dwFlags = 0 + self.wShowWindow = 1 + + +def test_capture_windows_prefers_pythonw_and_hides_window() -> None: + service = UsageService() + + with ( + patch("arcade_core.usage.usage_service.is_tracking_enabled", return_value=True), + patch.object(sys, "platform", "win32"), + patch.object(sys, "executable", r"C:\Python\python.exe"), + patch.object(sys, "base_prefix", r"C:\Python", create=True), + patch("arcade_core.usage.usage_service.Path.exists", return_value=True), + patch("arcade_core.usage.usage_service.shutil.which", return_value=None), + patch.object(subprocess, "STARTUPINFO", _DummyStartupInfo, create=True), + patch.object(subprocess, "STARTF_USESHOWWINDOW", 0x00000001, create=True), + patch.object(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200, create=True), + patch.object(subprocess, "CREATE_NO_WINDOW", 0x08000000, create=True), + patch("arcade_core.usage.usage_service.subprocess.Popen") as mock_popen, + ): + service.capture("event", "distinct-id", {"k": "v"}) + + args, kwargs = mock_popen.call_args + cmd = args[0] + + assert cmd[0].lower().endswith("pythonw.exe") + assert cmd[1:] == ["-m", "arcade_core.usage"] + + flags = kwargs["creationflags"] + assert flags & 0x00000200 # CREATE_NEW_PROCESS_GROUP + assert flags & 0x08000000 # CREATE_NO_WINDOW + assert not (flags & 0x00000008) # DETACHED_PROCESS should not be used + + startupinfo = kwargs["startupinfo"] + assert startupinfo is not None + assert startupinfo.wShowWindow == 0 + assert startupinfo.dwFlags & 0x00000001 + + +def test_capture_windows_falls_back_to_python_when_pythonw_missing() -> None: + service = UsageService() + + with ( + patch("arcade_core.usage.usage_service.is_tracking_enabled", return_value=True), + patch.object(sys, "platform", "win32"), + patch.object(sys, "executable", r"C:\Python\python.exe"), + patch.object(sys, "base_prefix", r"C:\Python", create=True), + patch("arcade_core.usage.usage_service.Path.exists", return_value=False), + patch("arcade_core.usage.usage_service.shutil.which", return_value=None), + patch.object(subprocess, "STARTUPINFO", _DummyStartupInfo, create=True), + patch.object(subprocess, "STARTF_USESHOWWINDOW", 0x00000001, create=True), + patch.object(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200, create=True), + patch.object(subprocess, "CREATE_NO_WINDOW", 0x08000000, create=True), + patch("arcade_core.usage.usage_service.subprocess.Popen") as mock_popen, + ): + service.capture("event", "distinct-id", {"k": "v"}) + + args, _kwargs = mock_popen.call_args + cmd = args[0] + assert cmd[0] == r"C:\Python\python.exe" + assert cmd[1:] == ["-m", "arcade_core.usage"] + + +def test_capture_non_windows_uses_start_new_session() -> None: + service = UsageService() + + with ( + patch("arcade_core.usage.usage_service.is_tracking_enabled", return_value=True), + patch.object(sys, "platform", "linux"), + patch("arcade_core.usage.usage_service.shutil.which", return_value=None), + patch("arcade_core.usage.usage_service.subprocess.Popen") as mock_popen, + ): + service.capture("event", "distinct-id", {"k": "v"}) + + _, kwargs = mock_popen.call_args + assert kwargs["start_new_session"] is True + assert "creationflags" not in kwargs + + +def test_capture_noop_when_tracking_disabled() -> None: + service = UsageService() + + with ( + patch("arcade_core.usage.usage_service.is_tracking_enabled", return_value=False), + patch("arcade_core.usage.usage_service.shutil.which", return_value=None), + patch("arcade_core.usage.usage_service.subprocess.Popen") as mock_popen, + ): + service.capture("event", "distinct-id", {"k": "v"}) + + mock_popen.assert_not_called() + + +def test_capture_windows_uses_base_prefix_pythonw_when_venv_pythonw_missing() -> None: + service = UsageService() + + base_pythonw = r"C:\BasePython\pythonw.exe" + + with ( + patch("arcade_core.usage.usage_service.is_tracking_enabled", return_value=True), + patch.object(sys, "platform", "win32"), + patch.object(sys, "executable", r"C:\Venv\Scripts\python.exe"), + patch.object(sys, "base_prefix", r"C:\BasePython", create=True), + patch("arcade_core.usage.usage_service.Path.exists", side_effect=[False, True]), + patch("arcade_core.usage.usage_service.shutil.which", return_value=None), + patch.object(subprocess, "STARTUPINFO", _DummyStartupInfo, create=True), + patch.object(subprocess, "STARTF_USESHOWWINDOW", 0x00000001, create=True), + patch.object(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200, create=True), + patch.object(subprocess, "CREATE_NO_WINDOW", 0x08000000, create=True), + patch("arcade_core.usage.usage_service.subprocess.Popen") as mock_popen, + ): + service.capture("event", "distinct-id", {"k": "v"}) + + args, _kwargs = mock_popen.call_args + cmd = args[0] + assert str(PureWindowsPath(cmd[0])) == base_pythonw + + +def test_capture_windows_uses_pythonw_from_path_when_available() -> None: + service = UsageService() + + path_pythonw = r"C:\Tools\pythonw.exe" + + with ( + patch("arcade_core.usage.usage_service.is_tracking_enabled", return_value=True), + patch.object(sys, "platform", "win32"), + patch.object(sys, "executable", r"C:\Venv\Scripts\python.exe"), + patch.object(sys, "base_prefix", r"C:\BasePython", create=True), + patch("arcade_core.usage.usage_service.Path.exists", side_effect=[False, False, True]), + patch("arcade_core.usage.usage_service.shutil.which", return_value=path_pythonw), + patch.object(subprocess, "STARTUPINFO", _DummyStartupInfo, create=True), + patch.object(subprocess, "STARTF_USESHOWWINDOW", 0x00000001, create=True), + patch.object(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200, create=True), + patch.object(subprocess, "CREATE_NO_WINDOW", 0x08000000, create=True), + patch("arcade_core.usage.usage_service.subprocess.Popen") as mock_popen, + ): + service.capture("event", "distinct-id", {"k": "v"}) + + args, _kwargs = mock_popen.call_args + cmd = args[0] + assert cmd[0] == path_pythonw diff --git a/libs/tests/sdk/test_eval_capture.py b/libs/tests/sdk/test_eval_capture.py index 5513b8e6..73edd9de 100644 --- a/libs/tests/sdk/test_eval_capture.py +++ b/libs/tests/sdk/test_eval_capture.py @@ -399,7 +399,7 @@ class TestCaptureResult: # Verify file was created and has valid content assert filepath.exists() - with open(filepath) as f: + with open(filepath, encoding="utf-8") as f: data = json.load(f) assert data["suite_name"] == "Suite" assert len(data["captured_cases"]) == 1 @@ -425,7 +425,7 @@ class TestCaptureResult: filepath = Path(tmpdir) / "capture_output.json" result.write_to_file(str(filepath), include_context=True) - with open(filepath) as f: + with open(filepath, encoding="utf-8") as f: data = json.load(f) assert data["captured_cases"][0]["system_message"] == "System" diff --git a/libs/tests/sdk/test_eval_multi_run.py b/libs/tests/sdk/test_eval_multi_run.py index ccbda141..3e05806f 100644 --- a/libs/tests/sdk/test_eval_multi_run.py +++ b/libs/tests/sdk/test_eval_multi_run.py @@ -15,7 +15,6 @@ from arcade_evals.eval import ( _resolve_pass_rule, ) - # ======================================================================== # _compute_mean_std tests # ======================================================================== diff --git a/pyproject.toml b/pyproject.toml index 2a6a8ad8..11eb5abf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "arcade-mcp" -version = "1.11.0" +version = "1.11.1" description = "Arcade.dev - Tool Calling platform for Agents" readme = "README.md" license = { file = "LICENSE" } @@ -19,8 +19,8 @@ requires-python = ">=3.10" dependencies = [ # CLI dependencies - "arcade-mcp-server>=1.17.0,<2.0.0", - "arcade-core>=4.4.0,<5.0.0", + "arcade-mcp-server>=1.17.2,<2.0.0", + "arcade-core>=4.4.2,<5.0.0", "typer==0.10.0", "rich>=14.0.0,<15.0.0", "Jinja2==3.1.6", @@ -29,6 +29,7 @@ dependencies = [ "tqdm==4.67.1", "click==8.1.8", "python-dateutil>=2.8.2", + "platformdirs>=4.3.6; platform_system == 'Windows'", ] [project.optional-dependencies] @@ -56,6 +57,10 @@ evals = [ "pytz>=2024.1", ] +windows = [ + "platformdirs>=4.3.6", +] + dev = [ # Test framework "pytest>=8.1.2", @@ -150,6 +155,9 @@ line-length = 100 [tool.ruff.lint] select = ["E", "F", "I", "N", "UP", "RUF"] +extend-select = ["PLW1514"] +preview = true +explicit-preview-rules = true ignore = ["E501", "S105"] [tool.ruff.lint.per-file-ignores] diff --git a/tests/install/README.md b/tests/install/README.md index bae582fa..0fa8dc3a 100644 --- a/tests/install/README.md +++ b/tests/install/README.md @@ -10,6 +10,8 @@ The installation test (`test_install.py`) verifies: 2. **Installation**: Installs `arcade-mcp` from source using `uv` 3. **CLI Functionality**: Tests that the `arcade` CLI command is available and working 4. **File Locking**: Verifies cross-platform file locking with `portalocker` (replacing `fcntl`) +5. **CLI Configure**: Writes temp config files for Cursor, VS Code, and Claude +6. **CLI New Scaffold**: Creates a minimal server in a path with spaces ## Running Locally @@ -52,7 +54,7 @@ python3 tests/install/test_install.py - Verifies `uv` is installed and available 2. **Installs Package** - - Syncs dependencies with `uv sync --dev` + - Syncs dependencies with `uv sync --extra dev` - Installs `arcade-mcp` in editable mode from source 3. **Tests CLI** @@ -60,7 +62,15 @@ python3 tests/install/test_install.py - Tests `arcade --version` (optional) - Tests `arcade whoami` (may fail if not logged in, but shouldn't crash) -4. **Tests File Locking** +4. **Tests CLI Configure** + - Runs `arcade configure` for Cursor, VS Code, and Claude using `--config` + - Verifies JSON structure for stdio/http configurations + +5. **Tests CLI New Scaffold** + - Runs `arcade new` in a temp path with spaces + - Verifies expected files and directories are created + +6. **Tests File Locking** - Creates a temporary identity file - Tests shared lock for reading - Tests exclusive lock for writing @@ -95,6 +105,20 @@ CLI Functionality Tests ✅ Success: Check arcade version ✅ Success: Test whoami command (no auth required) +============================================================ +CLI Configure Tests +============================================================ +✅ Success: Configure Cursor (stdio) with temp config +✅ Success: Configure Cursor (http) with temp config +✅ Success: Configure VS Code (stdio) with temp config +✅ Success: Configure VS Code (http) with temp config +✅ Success: Configure Claude (stdio) with temp config + +============================================================ +CLI New Scaffold Tests +============================================================ +✅ Success: Scaffold new server in path with spaces + ============================================================ File Locking Tests (portalocker) ============================================================ @@ -142,6 +166,16 @@ Or using pip: pip install uv ``` +### Windows: `uv` installed but still not found + +On some Windows shells, `uv` is installed to `C:\Users\\.local\bin` but +the current session PATH is not refreshed automatically. In PowerShell: + +```powershell +$env:Path = "$env:USERPROFILE\.local\bin;$env:Path" +uv --version +``` + ### Permission Denied If you get a permission error when running the script directly: @@ -155,10 +189,21 @@ chmod +x tests/install/test_install.py If you see import errors, make sure you're running from the project root and that dependencies are installed: ```bash -uv sync --dev +uv sync --extra dev +``` + +### Optional eval/display test dependencies + +Some CLI test files (for eval/display formatting paths) require optional eval +dependencies such as `openai`, `pytz`, and `numpy`. Install the eval extras +before running those suites: + +```bash +uv sync --extra dev --extra evals ``` ## Related Files - `.github/workflows/test-install.yml` - GitHub Actions workflow - `libs/arcade-core/arcade_core/usage/identity.py` - File locking implementation using `portalocker` +- `reports/windows-manual-test-checklist.md` - Windows manual validation steps diff --git a/tests/install/conftest.py b/tests/install/conftest.py new file mode 100644 index 00000000..f47201ec --- /dev/null +++ b/tests/install/conftest.py @@ -0,0 +1,29 @@ +"""Test configuration for installation tests. + +Mirrors the autouse fixtures in libs/tests/conftest.py so that +`pytest tests/install/` benefits from the same env setup as the +unit/integration test suites. +""" + +import os + +import pytest + + +@pytest.fixture(autouse=True) +def disable_usage_tracking() -> None: + """Disable CLI usage tracking for all installation tests. + + Prevents test runs from sending analytics events to PostHog. + Mirrors the same fixture in libs/tests/conftest.py. + """ + original_value = os.environ.get("ARCADE_USAGE_TRACKING") + + os.environ["ARCADE_USAGE_TRACKING"] = "0" + + yield + + if original_value is None: + os.environ.pop("ARCADE_USAGE_TRACKING", None) + else: + os.environ["ARCADE_USAGE_TRACKING"] = original_value diff --git a/tests/install/test_install.py b/tests/install/test_install.py index 71e34f42..fb1e5462 100755 --- a/tests/install/test_install.py +++ b/tests/install/test_install.py @@ -35,13 +35,23 @@ class TestRunner: self.test_results: list[tuple[str, bool]] = [] def _find_arcade_command(self) -> list[str]: - """Find the arcade command (either direct or via uv run).""" + """Find the arcade command (either direct or via uv run). + + When using uv run from temp dirs (e.g. configure tests), use + ``--project`` instead of ``--directory`` so uv resolves the project + environment without changing the subprocess working directory. + """ if shutil.which("arcade"): return ["arcade"] - return ["uv", "run", "arcade"] + return ["uv", "run", "--project", str(self.project_root), "arcade"] def run_command( - self, cmd: list[str], description: str, required: bool = True + self, + cmd: list[str], + description: str, + required: bool = True, + cwd: Path | None = None, + input_text: str | None = None, ) -> tuple[bool, str]: """Run a command and return success status and output.""" print(f"\n{'=' * 60}") @@ -61,6 +71,8 @@ class TestRunner: check=True, timeout=60, env=env, + cwd=str(cwd) if cwd else None, + input=input_text, encoding="utf-8", errors="replace", ) @@ -103,6 +115,26 @@ class TestRunner: success, _ = self.run_command(["uv", "--version"], "Check uv availability", required=True) return success + def _sync_dependencies_command( + self, + *, + current_test: str | None = None, + ) -> list[str]: + """Build the `uv sync` command for the current runtime context. + + When this suite runs under pytest, avoid reinstalling pytest itself. + This prevents mutating the active test runner environment and also + avoids `pytest.exe` file-lock failures on Windows. + """ + runtime_test = ( + current_test if current_test is not None else os.environ.get("PYTEST_CURRENT_TEST") + ) + + command = ["uv", "sync", "--dev"] + if runtime_test: + command.extend(["--inexact", "--no-install-package", "pytest"]) + return command + def install_package(self) -> bool: """Install arcade-mcp from source.""" print("\n" + "=" * 60) @@ -111,7 +143,7 @@ class TestRunner: # Sync dependencies sync_success, _ = self.run_command( - ["uv", "sync", "--dev"], + self._sync_dependencies_command(), "Sync dependencies with uv", required=True, ) @@ -265,5 +297,32 @@ def main() -> int: return runner.run_all_tests() +def test_installation() -> None: + """Pytest entry point for the installation test suite. + + Delegates to the existing TestRunner so the full install validation + runs under pytest (picks up conftest.py fixtures such as + disable_usage_tracking) without changing the internal test logic. + """ + exit_code = main() + assert exit_code == 0, "One or more installation tests failed — see output above." + + +def test_sync_dependencies_command_default(tmp_path: Path) -> None: + """Use the baseline sync command outside Windows+pytest context.""" + runner = TestRunner(tmp_path) + + assert runner._sync_dependencies_command(current_test="") == ["uv", "sync", "--dev"] + + +def test_sync_dependencies_command_pytest_context(tmp_path: Path) -> None: + """Avoid reinstalling pytest when this suite itself is running.""" + runner = TestRunner(tmp_path) + + assert runner._sync_dependencies_command( + current_test="tests/install/test_install.py::test_installation (call)", + ) == ["uv", "sync", "--dev", "--inexact", "--no-install-package", "pytest"] + + if __name__ == "__main__": sys.exit(main()) diff --git a/tests/integration/mcp_protocol_smoke.py b/tests/integration/mcp_protocol_smoke.py new file mode 100644 index 00000000..bb6726c3 --- /dev/null +++ b/tests/integration/mcp_protocol_smoke.py @@ -0,0 +1,694 @@ +#!/usr/bin/env python3 +"""MCP protocol smoke test for generated Arcade servers. + +Why this exists alongside ``libs/tests/arcade_mcp_server/integration/test_end_to_end.py`` +------------------------------------------------------------------------------------------ +The existing ``test_end_to_end.py`` is a pytest suite that validates the arcade-mcp-server +*library* against a dedicated test-fixture server with rich coverage (logging, progress +notifications, tool chaining, sampling, elicitation, concurrency). + +This smoke test serves a **different purpose**: + +1. **Tests ``arcade new`` scaffolded output** — validates that a *generated* project's + ``server.py`` works end-to-end, catching template regressions that library tests + cannot detect. +2. **Cross-platform CI entry point** — invoked by + ``tests/integration/no_auth_cli_smoke.py`` across the OS matrix. It includes + platform-aware process management (e.g., ``taskkill /T /F`` on Windows and + non-``select()`` stderr draining for Windows pipes) that the pytest suite + does not exercise. +3. **Stdlib-only / zero external deps** — runs with nothing beyond a ``uv run python`` + invocation so it works on fresh CI images before any ``pip install``. +4. **Standalone CLI** — uses ``argparse`` so PowerShell/bash scripts can invoke it + directly without a pytest harness. + +The basic protocol flow (initialize → initialized → ping → tools/list → tools/call) is +intentionally re-implemented here rather than imported, because the stdlib-only constraint +and the need to run outside the project's virtualenv make shared helpers impractical. + +Usage:: + + uv run python tests/integration/mcp_protocol_smoke.py \\ + --server-dir "/src/my_server" \\ + --transport both + +Tool naming convention (for reference):: + + MCPApp(name="my_server") + def greet() -> MCP tool name "MyServer_Greet" + Toolkit name is snake_to_pascal_case(app_name), tool is snake_to_pascal_case(func_name). + MCP exposes the fully-qualified name with "." replaced by "_". + We find the greet tool by case-insensitive substring match on "greet". +""" + +from __future__ import annotations + +import argparse +import contextlib +import json +import os +import platform +import queue +import shutil +import socket +import subprocess +import sys +import tempfile +import threading +import time +import urllib.error +import urllib.request +from typing import Any + +# --------------------------------------------------------------------------- +# JSON-RPC helpers +# --------------------------------------------------------------------------- + + +def _build_request( + method: str, + params: dict[str, Any] | None = None, + req_id: int | None = None, +) -> dict[str, Any]: + msg: dict[str, Any] = {"jsonrpc": "2.0", "method": method} + if params is not None: + msg["params"] = params + if req_id is not None: + msg["id"] = req_id + return msg + + +def _parse_json_line(line: str) -> dict[str, Any] | None: + line = line.strip() + if not line: + return None + try: + result = json.loads(line) + return result if isinstance(result, dict) else None + except json.JSONDecodeError: + return None + + +def _assert_ok(msg: dict[str, Any], expected_id: int, step: str) -> None: + assert msg.get("jsonrpc") == "2.0", f"[{step}] jsonrpc != '2.0': {msg}" + assert msg.get("id") == expected_id, f"[{step}] id != {expected_id}: {msg}" + assert "error" not in msg, f"[{step}] unexpected error field: {msg['error']}" + assert "result" in msg, f"[{step}] missing 'result' field: {msg}" + + +def _find_greet_tool(tools: list[dict[str, Any]]) -> str: + """Return the MCP tool name that contains 'greet' (case-insensitive).""" + for t in tools: + name = str(t.get("name", "")) + if "greet" in name.lower(): + return name + names = [t.get("name") for t in tools] + raise AssertionError( + "No tool containing 'greet' (case-insensitive) found.\n" + f"Available tools: {names}\n" + "Expected the generated server to expose a 'greet' tool " + "(e.g. MyServer_Greet from MCPApp(name='my_server') + def greet(...))." + ) + + +def _find_free_port() -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + return int(s.getsockname()[1]) + + +def _kill_process(proc: subprocess.Popen) -> None: + """Terminate a subprocess, using taskkill on Windows to kill the tree.""" + if proc.poll() is not None: + return + try: + if platform.system() == "Windows": + taskkill = shutil.which("taskkill") or "taskkill" + subprocess.run( + [taskkill, "/PID", str(proc.pid), "/T", "/F"], + capture_output=True, + ) + else: + proc.terminate() + proc.wait(timeout=5) + except Exception: + with contextlib.suppress(Exception): + proc.kill() + proc.wait(timeout=3) + + +def _drain_stderr(proc: subprocess.Popen, max_chars: int = 2000) -> str: + """Non-blocking read of available stderr bytes for diagnostics.""" + if proc.stderr is None: + return "" + with contextlib.suppress(Exception): + if platform.system() != "Windows": + import select + + ready, _, _ = select.select([proc.stderr], [], [], 0.1) + if ready: + return str(proc.stderr.read(max_chars)) + else: + # On Windows select() doesn't work on pipes; try reading with timeout + import threading + + buf: list[str] = [] + + def _reader() -> None: + with contextlib.suppress(Exception): + buf.append(str(proc.stderr.read(max_chars))) # type: ignore[union-attr] + + t = threading.Thread(target=_reader, daemon=True) + t.start() + t.join(timeout=0.5) + return buf[0] if buf else "" + return "" + + +def _tail_text_file(path: str | None, max_chars: int = 4000) -> str: + """Return the tail of a UTF-8 log file for diagnostics.""" + if not path or not os.path.exists(path): + return "" + try: + with open(path, encoding="utf-8", errors="replace") as f: + data = f.read() + return data[-max_chars:] + except Exception: + return "" + + +# --------------------------------------------------------------------------- +# Stdio transport +# --------------------------------------------------------------------------- + + +class StdioClient: + """Communicate with an MCP server over stdin/stdout.""" + + def __init__(self, proc: subprocess.Popen, timeout: float = 30.0) -> None: + self.proc = proc + self.timeout = timeout + self._next_id = 1 + self._stdout_queue: queue.Queue[str | None] = queue.Queue() + self._stdout_reader = threading.Thread(target=self._read_stdout_loop, daemon=True) + self._stdout_reader.start() + + def _read_stdout_loop(self) -> None: + if self.proc.stdout is None: + self._stdout_queue.put(None) + return + try: + for raw in self.proc.stdout: + self._stdout_queue.put(raw) + finally: + self._stdout_queue.put(None) + + def _next(self) -> int: + rid = self._next_id + self._next_id += 1 + return rid + + def send_request(self, method: str, params: dict[str, Any] | None = None) -> int: + rid = self._next() + line = json.dumps(_build_request(method, params, rid)) + "\n" + assert self.proc.stdin is not None + self.proc.stdin.write(line) + self.proc.stdin.flush() + return rid + + def send_notification(self, method: str, params: dict[str, Any] | None = None) -> None: + line = json.dumps(_build_request(method, params)) + "\n" + assert self.proc.stdin is not None + self.proc.stdin.write(line) + self.proc.stdin.flush() + + def read_response(self, expected_id: int) -> dict[str, Any]: + """Read lines until we get a JSON-RPC message with the expected id.""" + deadline = time.monotonic() + self.timeout + while time.monotonic() < deadline: + if self.proc.poll() is not None: + stderr_snippet = _drain_stderr(self.proc) + raise RuntimeError( + f"Server exited (code={self.proc.returncode}) while waiting for id={expected_id}.\n" + f"STDERR snippet:\n{stderr_snippet}" + ) + timeout = min(0.5, max(0.0, deadline - time.monotonic())) + try: + raw = self._stdout_queue.get(timeout=timeout) + except queue.Empty: + continue + if raw is None: + stderr_snippet = _drain_stderr(self.proc) + raise RuntimeError( + f"Server stdout closed (EOF) while waiting for id={expected_id}.\n" + f"STDERR snippet:\n{stderr_snippet}" + ) + msg = _parse_json_line(raw) + if msg is None: + # Non-JSON line (server log/debug output) — skip + print(f" [stdio/log] {raw.rstrip()}", flush=True) + continue + if msg.get("id") != expected_id: + # Notification or out-of-order message — log and skip + print(f" [stdio/msg] {json.dumps(msg)}", flush=True) + continue + return msg + raise TimeoutError(f"Timed out after {self.timeout}s waiting for id={expected_id}") + + +def run_stdio(server_dir: str, timeout: float) -> None: + print("\n=== Stdio transport MCP protocol smoke ===", flush=True) + proc: subprocess.Popen | None = None + stderr_sink = None + stderr_log_path: str | None = None + step = "startup" + last_response: dict[str, Any] = {} + try: + uv = shutil.which("uv") or "uv" + stderr_sink = tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + prefix="arcade-mcp-stdio-", + suffix=".log", + buffering=1, + delete=False, + ) + stderr_log_path = stderr_sink.name + proc = subprocess.Popen( + [uv, "run", "server.py"], + cwd=server_dir, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=stderr_sink, + text=True, + bufsize=1, + ) + + # Give the server a moment to initialize before we write to stdin + time.sleep(3) + if proc.poll() is not None: + stdout_out = proc.stdout.read() if proc.stdout else "" + stderr_out = proc.stderr.read() if proc.stderr else "" + raise RuntimeError( # noqa: TRY301 + f"Server exited early (code={proc.returncode}).\n" + f"STDOUT:\n{stdout_out}\nSTDERR:\n{stderr_out}" + ) + + client = StdioClient(proc, timeout=timeout) + + # 1. initialize + step = "initialize" + print(f"Step 1: {step}", flush=True) + init_id = client.send_request( + "initialize", + { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": {"name": "arcade-windows-ci", "version": "0.1.0"}, + }, + ) + last_response = client.read_response(expected_id=init_id) + _assert_ok(last_response, init_id, step) + server_info = last_response["result"].get("serverInfo", {}) + print(f" OK — serverInfo={server_info}", flush=True) + + # 2. notifications/initialized + step = "notifications/initialized" + print(f"Step 2: {step}", flush=True) + client.send_notification("notifications/initialized") + print(" OK — notification sent", flush=True) + + # 3. ping + step = "ping" + print(f"Step 3: {step}", flush=True) + ping_id = client.send_request("ping") + last_response = client.read_response(expected_id=ping_id) + assert last_response.get("jsonrpc") == "2.0", f"[ping] jsonrpc != '2.0': {last_response}" + assert last_response.get("id") == ping_id, f"[ping] id mismatch: {last_response}" + assert "error" not in last_response, f"[ping] error in response: {last_response}" + print(" OK", flush=True) + + # 4. tools/list + step = "tools/list" + print(f"Step 4: {step}", flush=True) + list_id = client.send_request("tools/list") + last_response = client.read_response(expected_id=list_id) + _assert_ok(last_response, list_id, step) + tools: list[dict[str, Any]] = last_response["result"].get("tools", []) + assert len(tools) > 0, f"[tools/list] empty tools list: {last_response}" + tool_names = [t.get("name") for t in tools] + print(f" OK — {len(tools)} tools: {tool_names}", flush=True) + + # 5. tools/call greet + step = "tools/call(greet)" + print(f"Step 5: {step}", flush=True) + greet_name = _find_greet_tool(tools) + print(f" using tool: {greet_name!r}", flush=True) + call_id = client.send_request( + "tools/call", + {"name": greet_name, "arguments": {"name": "Windows CI"}}, + ) + last_response = client.read_response(expected_id=call_id) + _assert_ok(last_response, call_id, step) + content = last_response["result"].get("content", []) + assert len(content) > 0, f"[{step}] empty content array: {last_response}" + text: str = content[0].get("text", "") + assert "Hello" in text, ( + f"[{step}] expected 'Hello' in response text.\n" + f" Got: {text!r}\n" + f" Full response: {last_response}" + ) + print(f" OK — response: {text!r}", flush=True) + + print("\nStdio transport smoke PASSED.", flush=True) + + except Exception as exc: + if stderr_sink is not None: + with contextlib.suppress(Exception): + stderr_sink.flush() + stderr_snippet = _tail_text_file(stderr_log_path) if stderr_log_path else "" + print( + f"\nSTDIO SMOKE FAILED at step '{step}'.\n" + f" Error: {exc}\n" + f" Last response: {json.dumps(last_response) if last_response else 'n/a'}\n" + f" Server STDERR snippet:\n{stderr_snippet}", + file=sys.stderr, + flush=True, + ) + raise + finally: + if stderr_sink is not None: + with contextlib.suppress(Exception): + stderr_sink.close() + if proc is not None: + _kill_process(proc) + + +# --------------------------------------------------------------------------- +# HTTP transport +# --------------------------------------------------------------------------- + + +def _http_post( + url: str, + payload: dict[str, Any], + extra_headers: dict[str, str] | None = None, + read_response_headers: bool = False, + timeout_seconds: float = 30.0, +) -> tuple[int, dict[str, str], dict[str, Any]]: + """POST JSON payload, return (status, response_headers, body_dict).""" + body = json.dumps(payload).encode("utf-8") + req = urllib.request.Request(url, data=body, method="POST") # noqa: S310 + req.add_header("Content-Type", "application/json") + req.add_header("Accept", "application/json") + if extra_headers: + for k, v in extra_headers.items(): + req.add_header(k, v) + try: + with urllib.request.urlopen(req, timeout=timeout_seconds) as resp: # noqa: S310 + status: int = resp.status + # http.client.HTTPMessage supports case-insensitive get() + resp_headers: dict[str, str] = {} + if read_response_headers: + for key in resp.headers: + resp_headers[key.lower()] = resp.headers[key] + raw = resp.read().decode("utf-8") + body_dict: dict[str, Any] = json.loads(raw) if raw.strip() else {} + return status, resp_headers, body_dict + except urllib.error.HTTPError as e: + status = e.code + raw = e.read().decode("utf-8") + try: + return status, {}, json.loads(raw) + except Exception: + return status, {}, {"_raw": raw} + + +def _wait_for_health(url: str, timeout: float) -> None: + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + with contextlib.suppress(Exception): + req = urllib.request.Request(url) # noqa: S310 + with urllib.request.urlopen(req, timeout=2) as resp: # noqa: S310 + if resp.status == 200: + return + time.sleep(1) + raise TimeoutError(f"Server did not become healthy at {url} within {timeout}s") + + +def run_http(server_dir: str, timeout: float) -> None: + print("\n=== HTTP transport MCP protocol smoke ===", flush=True) + port = _find_free_port() + proc: subprocess.Popen | None = None + stdout_sink = None + stderr_sink = None + stdout_log_path: str | None = None + stderr_log_path: str | None = None + step = "startup" + last_response: dict[str, Any] = {} + try: + env = { + **os.environ, + "ARCADE_SERVER_HOST": "127.0.0.1", + "ARCADE_SERVER_PORT": str(port), + "ARCADE_WORKER_SECRET": "arcade-smoke-worker-secret", + } + uv = shutil.which("uv") or "uv" + stdout_sink = tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + prefix="arcade-mcp-http-out-", + suffix=".log", + buffering=1, + delete=False, + ) + stdout_log_path = stdout_sink.name + stderr_sink = tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + prefix="arcade-mcp-http-err-", + suffix=".log", + buffering=1, + delete=False, + ) + stderr_log_path = stderr_sink.name + proc = subprocess.Popen( + [uv, "run", "server.py", "http"], + cwd=server_dir, + stdout=stdout_sink, + stderr=stderr_sink, + text=True, + env=env, + ) + + base_url = f"http://127.0.0.1:{port}" + health_url = f"{base_url}/worker/health" + mcp_url = f"{base_url}/mcp" + + print(f" waiting for health at {health_url} (up to 30s)", flush=True) + _wait_for_health(health_url, timeout=30) + if proc.poll() is not None: + stderr_out = proc.stderr.read() if proc.stderr else "" + raise RuntimeError( # noqa: TRY301 + f"Server exited (code={proc.returncode}) before health check passed.\n" + f"STDERR:\n{stderr_out}" + ) + print(" health OK", flush=True) + + session_headers: dict[str, str] = {} + + # 1. initialize — capture mcp-session-id from response headers + step = "initialize" + print(f"Step 1: {step}", flush=True) + init_req = _build_request( + "initialize", + { + "protocolVersion": "2025-06-18", + "capabilities": {}, + "clientInfo": {"name": "arcade-windows-ci", "version": "0.1.0"}, + }, + req_id=1, + ) + init_status, init_hdrs, last_response = _http_post( + mcp_url, init_req, read_response_headers=True, timeout_seconds=timeout + ) + assert ( + init_status == 200 + ), f"[{step}] expected status 200, got {init_status}: {last_response}" + assert last_response.get("jsonrpc") == "2.0", f"[{step}] jsonrpc != '2.0': {last_response}" + assert last_response.get("id") == 1, f"[{step}] id != 1: {last_response}" + assert "error" not in last_response, f"[{step}] error in response: {last_response}" + session_id = init_hdrs.get("mcp-session-id") + assert session_id is not None, ( + f"[{step}] mcp-session-id header missing from initialize response.\n" + f" Headers received: {init_hdrs}" + ) + session_headers["Mcp-Session-Id"] = session_id + server_info = last_response["result"].get("serverInfo", {}) + print(f" OK — serverInfo={server_info}, session_id={session_id}", flush=True) + + # 2. notifications/initialized + step = "notifications/initialized" + print(f"Step 2: {step}", flush=True) + notif_req = _build_request("notifications/initialized") + notif_status, _, _ = _http_post(mcp_url, notif_req, extra_headers=session_headers) + assert notif_status == 202, f"[{step}] expected status 202, got {notif_status}" + print(" OK (202)", flush=True) + + # 3. ping + step = "ping" + print(f"Step 3: {step}", flush=True) + ping_status, _, last_response = _http_post( + mcp_url, + _build_request("ping", req_id=2), + extra_headers=session_headers, + timeout_seconds=timeout, + ) + assert ping_status == 200, f"[{step}] expected 200, got {ping_status}: {last_response}" + assert last_response.get("jsonrpc") == "2.0", f"[{step}] jsonrpc: {last_response}" + assert last_response.get("id") == 2, f"[{step}] id: {last_response}" + assert "error" not in last_response, f"[{step}] error: {last_response}" + print(" OK", flush=True) + + # 4. tools/list + step = "tools/list" + print(f"Step 4: {step}", flush=True) + list_status, _, last_response = _http_post( + mcp_url, + _build_request("tools/list", req_id=3), + extra_headers=session_headers, + timeout_seconds=timeout, + ) + assert list_status == 200, f"[{step}] expected 200, got {list_status}: {last_response}" + _assert_ok(last_response, 3, step) + tools = last_response["result"].get("tools", []) + assert len(tools) > 0, f"[{step}] empty tools list: {last_response}" + tool_names = [t.get("name") for t in tools] + print(f" OK — {len(tools)} tools: {tool_names}", flush=True) + + # 5. tools/call greet + step = "tools/call(greet)" + print(f"Step 5: {step}", flush=True) + greet_name = _find_greet_tool(tools) + print(f" using tool: {greet_name!r}", flush=True) + call_status, _, last_response = _http_post( + mcp_url, + _build_request( + "tools/call", + {"name": greet_name, "arguments": {"name": "Windows CI"}}, + req_id=4, + ), + extra_headers=session_headers, + timeout_seconds=timeout, + ) + assert call_status == 200, f"[{step}] expected 200, got {call_status}: {last_response}" + _assert_ok(last_response, 4, step) + content = last_response["result"].get("content", []) + assert len(content) > 0, f"[{step}] empty content: {last_response}" + text: str = content[0].get("text", "") + assert "Hello" in text, ( + f"[{step}] expected 'Hello' in response text.\n" + f" Got: {text!r}\n" + f" Full response: {last_response}" + ) + print(f" OK — response: {text!r}", flush=True) + + print("\nHTTP transport smoke PASSED.", flush=True) + + except Exception as exc: + if stdout_sink is not None: + with contextlib.suppress(Exception): + stdout_sink.flush() + if stderr_sink is not None: + with contextlib.suppress(Exception): + stderr_sink.flush() + stdout_tail = _tail_text_file(stdout_log_path) if stdout_log_path else "" + stderr_tail = _tail_text_file(stderr_log_path) if stderr_log_path else "" + print( + f"\nHTTP SMOKE FAILED at step '{step}'.\n" + f" Error: {exc}\n" + f" Last response: {json.dumps(last_response) if last_response else 'n/a'}\n" + f" Server STDOUT snippet:\n{stdout_tail}\n" + f" Server STDERR snippet:\n{stderr_tail}", + file=sys.stderr, + flush=True, + ) + raise + finally: + if stdout_sink is not None: + with contextlib.suppress(Exception): + stdout_sink.close() + if stderr_sink is not None: + with contextlib.suppress(Exception): + stderr_sink.close() + if proc is not None: + _kill_process(proc) + + +# --------------------------------------------------------------------------- +# Entry point +# --------------------------------------------------------------------------- + + +def main() -> None: + parser = argparse.ArgumentParser( + description="MCP protocol smoke test for a generated Arcade server." + ) + parser.add_argument( + "--server-dir", + required=True, + help="Directory containing server.py (the generated server src dir).", + ) + parser.add_argument( + "--transport", + choices=["stdio", "http", "both"], + default="both", + help="Transport(s) to validate (default: both).", + ) + parser.add_argument( + "--timeout-seconds", + type=float, + default=30.0, + help="Per-step read timeout in seconds (default: 30).", + ) + args = parser.parse_args() + + server_dir = os.path.abspath(args.server_dir) + if not os.path.isdir(server_dir): + print(f"ERROR: --server-dir does not exist: {server_dir}", file=sys.stderr) + sys.exit(1) + if not os.path.isfile(os.path.join(server_dir, "server.py")): + print( + f"ERROR: server.py not found in --server-dir: {server_dir}", + file=sys.stderr, + ) + sys.exit(1) + + print(f"Server dir : {server_dir}") + print(f"Transport : {args.transport}") + print(f"Timeout : {args.timeout_seconds}s per step") + + failures: list[str] = [] + + if args.transport in ("stdio", "both"): + try: + run_stdio(server_dir, timeout=args.timeout_seconds) + except Exception as exc: + failures.append(f"stdio: {exc}") + + if args.transport in ("http", "both"): + try: + run_http(server_dir, timeout=args.timeout_seconds) + except Exception as exc: + failures.append(f"http: {exc}") + + if failures: + print("\n=== MCP PROTOCOL SMOKE FAILURES ===", file=sys.stderr) + for msg in failures: + print(f" ✗ {msg}", file=sys.stderr) + sys.exit(1) + + print("\nAll MCP protocol smoke checks PASSED.", flush=True) + + +if __name__ == "__main__": + main() diff --git a/tests/integration/no_auth_cli_smoke.py b/tests/integration/no_auth_cli_smoke.py new file mode 100644 index 00000000..d828f003 --- /dev/null +++ b/tests/integration/no_auth_cli_smoke.py @@ -0,0 +1,299 @@ +#!/usr/bin/env python3 +"""Cross-platform no-auth CLI integration smoke checks. + +This script runs a minimal but meaningful no-auth integration flow across all +CI operating systems: +1. Validate `arcade configure` writes client configs in a path with spaces. +2. Scaffold a new toolkit with `arcade new`. +3. Run protocol smoke checks (stdio + http) against the generated server. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import tempfile +from pathlib import Path +from typing import Any, cast + + +def _run( + cmd: list[str], + *, + cwd: Path, + capture_output: bool = False, +) -> subprocess.CompletedProcess[str]: + proc = subprocess.run( + cmd, + cwd=str(cwd), + text=True, + capture_output=capture_output, + check=False, + ) + if proc.returncode != 0: + raise RuntimeError( + f"Command failed ({proc.returncode}): {' '.join(cmd)}\n" + f"STDOUT:\n{proc.stdout or ''}\nSTDERR:\n{proc.stderr or ''}" + ) + return proc + + +def _ensure_exists(path: Path) -> None: + if not path.exists(): + raise RuntimeError(f"Expected path to exist: {path}") + + +def _load_json_object(path: Path) -> dict[str, Any]: + parsed = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(parsed, dict): + raise TypeError(f"Expected JSON object in {path}, got {type(parsed).__name__}") + return cast(dict[str, Any], parsed) + + +def _expect_dict(value: Any, context: str) -> dict[str, Any]: + if not isinstance(value, dict): + raise TypeError(f"Expected object for {context}, got {type(value).__name__}") + return cast(dict[str, Any], value) + + +def _assert_stdio_entry(entry: dict[str, Any], context: str) -> None: + if "command" not in entry: + raise RuntimeError(f"{context}: missing 'command'") + + args = entry.get("args") + if not isinstance(args, list): + raise TypeError(f"{context}: missing or invalid 'args' list") + if not any(str(arg).endswith("server.py") for arg in args): + raise RuntimeError(f"{context}: expected entrypoint in args ending with 'server.py'") + + +def _add_local_uv_sources(pyproject_path: Path, repo_root: Path) -> None: + pyproject_text = pyproject_path.read_text(encoding="utf-8") + if "[tool.uv.sources]" in pyproject_text: + return + + repo = repo_root.resolve() + block_lines = [ + "[tool.uv.sources]", + f'arcade-mcp = {{ path = "{repo.as_posix()}", editable = true }}', + f'arcade-mcp-server = {{ path = "{(repo / "libs/arcade-mcp-server").as_posix()}", editable = true }}', + f'arcade-core = {{ path = "{(repo / "libs/arcade-core").as_posix()}", editable = true }}', + f'arcade-serve = {{ path = "{(repo / "libs/arcade-serve").as_posix()}", editable = true }}', + f'arcade-tdk = {{ path = "{(repo / "libs/arcade-tdk").as_posix()}", editable = true }}', + ] + pyproject_path.write_text( + pyproject_text.rstrip() + "\n\n" + "\n".join(block_lines) + "\n", + encoding="utf-8", + ) + + +def _run_configure_smoke(repo_root: Path) -> None: + config_tmp = Path(tempfile.mkdtemp(prefix="arcade mcp config test ")) + try: + (config_tmp / "server.py").write_text("print('ok')\n", encoding="utf-8") + + cursor_cfg = config_tmp / "cursor config.json" + vscode_cfg = config_tmp / "vscode config.json" + claude_cfg = config_tmp / "claude config.json" + + _run( + [ + "uv", + "run", + "--project", + str(repo_root), + "arcade", + "configure", + "cursor", + "--name", + "demo", + "--config", + str(cursor_cfg), + ], + cwd=config_tmp, + ) + cursor_data = _load_json_object(cursor_cfg) + cursor_mcp_servers = _expect_dict(cursor_data.get("mcpServers"), "Cursor stdio mcpServers") + _assert_stdio_entry( + _expect_dict(cursor_mcp_servers.get("demo"), "Cursor stdio demo server"), "Cursor stdio" + ) + + overwrite = _run( + [ + "uv", + "run", + "--project", + str(repo_root), + "arcade", + "configure", + "cursor", + "--transport", + "http", + "--port", + "8123", + "--name", + "demo", + "--config", + str(cursor_cfg), + ], + cwd=config_tmp, + capture_output=True, + ) + overwrite_output = (overwrite.stdout or "") + "\n" + (overwrite.stderr or "") + if "overwrite" not in overwrite_output.lower(): + raise RuntimeError( + "Expected overwrite warning when configuring cursor with same --name.\n" + f"Output:\n{overwrite_output}" + ) + cursor_data = _load_json_object(cursor_cfg) + cursor_mcp_servers = _expect_dict(cursor_data.get("mcpServers"), "Cursor http mcpServers") + cursor_http_demo = _expect_dict(cursor_mcp_servers.get("demo"), "Cursor http demo server") + if cursor_http_demo.get("type") != "stream": + raise RuntimeError("Cursor http config type mismatch") + if cursor_http_demo.get("url") != "http://localhost:8123/mcp": + raise RuntimeError("Cursor http config URL mismatch") + + _run( + [ + "uv", + "run", + "--project", + str(repo_root), + "arcade", + "configure", + "vscode", + "--name", + "demo", + "--config", + str(vscode_cfg), + ], + cwd=config_tmp, + ) + vscode_data = _load_json_object(vscode_cfg) + vscode_servers = _expect_dict(vscode_data.get("servers"), "VS Code stdio servers") + _assert_stdio_entry( + _expect_dict(vscode_servers.get("demo"), "VS Code stdio demo server"), "VS Code stdio" + ) + + _run( + [ + "uv", + "run", + "--project", + str(repo_root), + "arcade", + "configure", + "vscode", + "--transport", + "http", + "--port", + "8123", + "--name", + "demo", + "--config", + str(vscode_cfg), + ], + cwd=config_tmp, + ) + vscode_data = _load_json_object(vscode_cfg) + vscode_servers = _expect_dict(vscode_data.get("servers"), "VS Code http servers") + vscode_http_demo = _expect_dict(vscode_servers.get("demo"), "VS Code http demo server") + if vscode_http_demo.get("type") != "http": + raise RuntimeError("VS Code http config type mismatch") + if vscode_http_demo.get("url") != "http://localhost:8123/mcp": + raise RuntimeError("VS Code http config URL mismatch") + + _run( + [ + "uv", + "run", + "--project", + str(repo_root), + "arcade", + "configure", + "claude", + "--name", + "demo", + "--config", + str(claude_cfg), + ], + cwd=config_tmp, + ) + claude_data = _load_json_object(claude_cfg) + claude_mcp_servers = _expect_dict(claude_data.get("mcpServers"), "Claude stdio mcpServers") + _assert_stdio_entry( + _expect_dict(claude_mcp_servers.get("demo"), "Claude stdio demo server"), "Claude stdio" + ) + finally: + shutil.rmtree(config_tmp, ignore_errors=True) + + +def _run_scaffold_and_protocol_smoke(repo_root: Path) -> None: + scaffold_dir = Path(tempfile.mkdtemp(prefix="arcade scaffold with spaces ")) + try: + created = _run( + [ + "uv", + "run", + "arcade", + "new", + "my_server", + "--dir", + str(scaffold_dir), + ], + cwd=repo_root, + capture_output=True, + ) + new_output = (created.stdout or "") + "\n" + (created.stderr or "") + if "Next steps:" not in new_output: + raise RuntimeError( + "Expected 'Next steps:' output from 'arcade new'.\n" f"Output:\n{new_output}" + ) + + generated_root = scaffold_dir / "my_server" + _ensure_exists(generated_root / "pyproject.toml") + _ensure_exists(generated_root / "src" / "my_server" / "server.py") + _ensure_exists(generated_root / "src" / "my_server" / ".env.example") + + generated_pyproject = generated_root / "pyproject.toml" + _add_local_uv_sources(generated_pyproject, repo_root) + + generated_server_dir = generated_root / "src" / "my_server" + _run( + ["uv", "run", "python", "-c", "import server; print('generated server import ok')"], + cwd=generated_server_dir, + ) + + _run( + [ + "uv", + "run", + "python", + "tests/integration/mcp_protocol_smoke.py", + "--server-dir", + str(generated_server_dir), + "--transport", + "both", + ], + cwd=repo_root, + ) + finally: + shutil.rmtree(scaffold_dir, ignore_errors=True) + + +def main() -> None: + repo_root = Path.cwd().resolve() + print(f"Repo root: {repo_root}") + os.environ["ARCADE_USAGE_TRACKING"] = "0" + _run(["uv", "--version"], cwd=repo_root) + + _run_configure_smoke(repo_root) + _run_scaffold_and_protocol_smoke(repo_root) + + print("Cross-platform no-auth CLI smoke checks passed.") + + +if __name__ == "__main__": + main()