From bcee0f556f556a9ffa8891e1ce51d990f760124f Mon Sep 17 00:00:00 2001 From: jottakka Date: Thu, 26 Feb 2026 16:24:15 +0000 Subject: [PATCH] Left over fixes for Windows Papercut PR (#781) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > [!NOTE] > **Low Risk** > Mostly CI/test and CLI output tweaks, plus a small refactor to reuse existing subprocess termination logic; low risk with minor potential for CI environment/version compatibility issues. > > **Overview** > Expands CI coverage by adding Python `3.13` and `3.14` to the GitHub Actions matrices (main tests, install test, and no-auth CLI integration), and removes a redundant editable install step in the no-auth workflow. > > Cleans up Windows subprocess handling by dropping `arcade_cli.deploy._graceful_terminate` and calling the shared `arcade_core.subprocess_utils.graceful_terminate_process` directly, with corresponding test updates. > > Improves `arcade new` scaffolding guidance by printing numbered “Next steps” with explicit stdio/HTTP run options, and adds/updates CLI tests to assert this output. Also bumps package version to `1.11.2` and tightens pre-commit `ruff` excludes (no longer excluding `_scratch`). > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 55c2ae106f13e5657acdbebf63e00d74c171181f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot). --- .github/workflows/cli-integration-no-auth.yml | 8 +--- .github/workflows/main.yml | 2 +- .github/workflows/test-install.yml | 2 +- .pre-commit-config.yaml | 4 +- libs/arcade-cli/arcade_cli/deploy.py | 9 +--- libs/arcade-cli/arcade_cli/new.py | 20 ++++----- libs/tests/cli/test_new_cli.py | 44 ++++++++++++++++++- libs/tests/cli/test_windows_subprocess.py | 22 +++++----- libs/tests/worker/test_worker_base.py | 3 -- pyproject.toml | 4 -- 10 files changed, 69 insertions(+), 49 deletions(-) diff --git a/.github/workflows/cli-integration-no-auth.yml b/.github/workflows/cli-integration-no-auth.yml index cba39a1f..5436b620 100644 --- a/.github/workflows/cli-integration-no-auth.yml +++ b/.github/workflows/cli-integration-no-auth.yml @@ -12,13 +12,11 @@ jobs: 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"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] fail-fast: false steps: @@ -30,10 +28,6 @@ jobs: 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 8c21ccfc..e2119429 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -31,7 +31,7 @@ jobs: strategy: matrix: os: [ubuntu-latest, windows-latest, macos-latest] - python-version: ["3.10", "3.11", "3.12"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] fail-fast: false steps: - name: Check out diff --git a/.github/workflows/test-install.yml b/.github/workflows/test-install.yml index 50de01ec..871e8a7c 100644 --- a/.github/workflows/test-install.yml +++ b/.github/workflows/test-install.yml @@ -15,7 +15,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, windows-latest, macos-latest] - python-version: ["3.10", "3.12"] + python-version: ["3.10", "3.11", "3.12", "3.13", "3.14"] steps: - name: Checkout repository diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 761cc8bf..e9a74942 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/.*|_scratch/.*)" + exclude: "(.*/templates/.*|libs/tests/.*)" - id: ruff-format - exclude: "(.*/templates/.*|libs/tests/.*|_scratch/.*)" + exclude: "(.*/templates/.*|libs/tests/.*)" diff --git a/libs/arcade-cli/arcade_cli/deploy.py b/libs/arcade-cli/arcade_cli/deploy.py index 28d56318..230d0a57 100644 --- a/libs/arcade-cli/arcade_cli/deploy.py +++ b/libs/arcade-cli/arcade_cli/deploy.py @@ -370,11 +370,6 @@ 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. @@ -496,7 +491,7 @@ def wait_for_health( time.sleep(0.5) if not is_healthy: - _graceful_terminate(process) + graceful_terminate_process(process) try: process.communicate(timeout=2) except subprocess.TimeoutExpired: @@ -651,7 +646,7 @@ def verify_server_and_get_metadata( finally: # Always stop the server - _graceful_terminate(process) + graceful_terminate_process(process) try: process.wait(timeout=5) except subprocess.TimeoutExpired: diff --git a/libs/arcade-cli/arcade_cli/new.py b/libs/arcade-cli/arcade_cli/new.py index 486ead2f..de9e2ca2 100644 --- a/libs/arcade-cli/arcade_cli/new.py +++ b/libs/arcade-cli/arcade_cli/new.py @@ -227,13 +227,11 @@ def create_new_toolkit(output_directory: str, toolkit_name: str) -> None: 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(f" 1. 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(" 2. Run the server (choose one transport):", style="dim") + console.print(" - stdio: uv run server.py") + console.print(" - http: uv run server.py --transport http --port 8000") console.print("") create_deployment(toolkit_directory, toolkit_name) except Exception: @@ -288,13 +286,11 @@ def create_new_toolkit_minimal(output_directory: str, toolkit_name: str) -> None ) server_dir = toolkit_directory / toolkit_name / "src" / toolkit_name console.print("\nNext steps:", style="bold") - console.print(f" cd {server_dir}") + console.print(f" 1. 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(" 2. Run the server (choose one transport):", style="dim") + console.print(" - stdio: uv run server.py") + console.print(" - http: uv run server.py --transport http --port 8000") console.print("") except Exception: remove_toolkit(toolkit_directory, toolkit_name) diff --git a/libs/tests/cli/test_new_cli.py b/libs/tests/cli/test_new_cli.py index 4b72b711..66e03665 100644 --- a/libs/tests/cli/test_new_cli.py +++ b/libs/tests/cli/test_new_cli.py @@ -1,11 +1,45 @@ from io import StringIO from pathlib import Path +from unittest.mock import patch import pytest -from arcade_cli.new import create_new_toolkit_minimal +from arcade_cli.new import create_new_toolkit, create_new_toolkit_minimal from rich.console import Console +def test_create_new_toolkit_prints_next_steps(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """create_new_toolkit (full template) should print numbered next steps.""" + output_dir = tmp_path / "full_test" + output_dir.mkdir() + + # Use a cwd that does not trigger community/official toolkit prompts + fake_cwd = tmp_path / "cwd" + fake_cwd.mkdir() + monkeypatch.chdir(fake_cwd) + + # Mock prompts: description, author, email, evals (yes) + with patch("arcade_cli.new.typer.prompt", side_effect=["", "", "", "y"]): + 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(str(output_dir), "my_server") + finally: + new_mod.console = orig + + output = buf.getvalue() + assert "Next steps:" in output + assert "1. cd " in output + assert "2. Run the server (choose one transport):" in output + assert "- stdio: uv run server.py" in output + assert "- http: uv run server.py --transport http --port 8000" in output + assert "uv run server.py" in output + assert "my_server" in output + + def test_create_new_toolkit_minimal_with_spaces(tmp_path: Path) -> None: output_dir = tmp_path / "dir with spaces" output_dir.mkdir() @@ -38,6 +72,14 @@ def test_create_new_toolkit_minimal_prints_next_steps(tmp_path: Path) -> None: output = buf.getvalue() assert "Next steps:" in output, f"Expected 'Next steps:' in output:\n{output}" + assert "1. cd " in output, f"Expected numbered step 1 in output:\n{output}" + assert "2. Run the server (choose one transport):" in output, ( + f"Expected numbered step 2 in output:\n{output}" + ) + assert "- stdio: uv run server.py" in output, f"Expected stdio option in output:\n{output}" + assert "- http: uv run server.py --transport http --port 8000" in output, ( + f"Expected HTTP option 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}" diff --git a/libs/tests/cli/test_windows_subprocess.py b/libs/tests/cli/test_windows_subprocess.py index 1bbd0f20..c6573acc 100644 --- a/libs/tests/cli/test_windows_subprocess.py +++ b/libs/tests/cli/test_windows_subprocess.py @@ -2,7 +2,7 @@ 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(). +- graceful_terminate_process 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. @@ -138,22 +138,22 @@ class TestDeployCreateNoWindow: # --------------------------------------------------------------------------- -# deploy.py — _graceful_terminate() +# subprocess_utils.py — graceful_terminate_process() # --------------------------------------------------------------------------- class TestGracefulTerminate: - """Verify _graceful_terminate uses CTRL_BREAK_EVENT on Windows.""" + """Verify graceful_terminate_process 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 + """On Windows, graceful_terminate_process should send CTRL_BREAK_EVENT.""" + from arcade_core.subprocess_utils import graceful_terminate_process mock_proc = MagicMock() # sys.platform mock: verifies CTRL_BREAK_EVENT dispatch with mocked process. with _patch_win32_ctrl_break(): - _graceful_terminate(mock_proc) + graceful_terminate_process(mock_proc) # Should try send_signal with CTRL_BREAK_EVENT (not terminate) mock_proc.send_signal.assert_called_once_with(WIN_CTRL_BREAK_EVENT) @@ -161,25 +161,25 @@ class TestGracefulTerminate: 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 + from arcade_core.subprocess_utils import graceful_terminate_process 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) + graceful_terminate_process(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 + """On Linux/macOS, graceful_terminate_process should call terminate() directly.""" + from arcade_core.subprocess_utils import graceful_terminate_process mock_proc = MagicMock() - _graceful_terminate(mock_proc) + graceful_terminate_process(mock_proc) mock_proc.terminate.assert_called_once() mock_proc.send_signal.assert_not_called() diff --git a/libs/tests/worker/test_worker_base.py b/libs/tests/worker/test_worker_base.py index e35652f2..f17cd99e 100644 --- a/libs/tests/worker/test_worker_base.py +++ b/libs/tests/worker/test_worker_base.py @@ -124,9 +124,6 @@ async def test_call_tool_success(base_worker_no_auth): assert response.output.value == 8 assert response.output.error is None assert response.execution_id == "test_exec_id" - assert response.duration > 0 - - @pytest.mark.asyncio async def test_call_tool_execution_error(base_worker_no_auth): # Tool is now defined at module level diff --git a/pyproject.toml b/pyproject.toml index 9620babf..7268de97 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,10 +57,6 @@ evals = [ "pytz>=2024.1", ] -windows = [ - "platformdirs>=4.3.6", -] - dev = [ # Test framework "pytest>=8.1.2",