diff --git a/src/gflow_cli/mcp/server.py b/src/gflow_cli/mcp/server.py index 218aab4cb..7a4cecdb5 100644 --- a/src/gflow_cli/mcp/server.py +++ b/src/gflow_cli/mcp/server.py @@ -126,6 +126,27 @@ def _configure_utf8_pipes() -> None: _SPEND_TOOLS = ("gflow_generate_image", "gflow_generate_video") +def _apply_mcp_lease_wait_default() -> None: + """Make profile contention wait instead of hard-failing (reliability). + + The CLI's fail-fast default (``GFLOW_CLI_LEASE_WAIT_SECONDS=0``, see + ``config.py``) is correct for a one-shot terminal command: a human is + watching and can just retry. An MCP client has no such human in the + loop — two tool calls landing close together on the same profile (a + retry after a slow response, or two agents sharing a profile) would + otherwise hard-fail with ``ProfileLockedError`` instead of the caller's + request simply running after the one already in flight. `setdefault` + only takes effect for a caller who has not already set the var, and + `get_settings()` is process-cached (`@lru_cache`), so this MUST run + before anything in this process calls it — hence first line of every + `run_*` entry point, before `_register_surfaces()`. + + 180s covers a typical single image/video generation with headroom + without hanging an MCP client indefinitely on a genuinely stuck holder. + """ + os.environ.setdefault("GFLOW_CLI_LEASE_WAIT_SECONDS", "180") + + def no_spend_active() -> bool: """True when no-spend mode is requested (#496). @@ -304,6 +325,7 @@ async def run_stdio() -> None: import anyio from mcp.server.stdio import stdio_server + _apply_mcp_lease_wait_default() _configure_utf8_pipes() # Capture the REAL stdout for the JSON-RPC channel BEFORE redirecting @@ -350,6 +372,7 @@ async def run_http(host: str = "127.0.0.1", port: int = 8000) -> None: host: Bind address. Defaults to localhost-only for security. port: Port number. Defaults to 8000. """ + _apply_mcp_lease_wait_default() _configure_utf8_pipes() token = _daemon_token() @@ -385,6 +408,7 @@ async def run_sse(host: str = "127.0.0.1", port: int = 8000) -> None: host: Bind address. Defaults to localhost-only for security. port: Port number. Defaults to 8000. """ + _apply_mcp_lease_wait_default() _configure_utf8_pipes() token = _daemon_token() diff --git a/tests/mcp/test_server.py b/tests/mcp/test_server.py index a95d18bb6..36e750399 100644 --- a/tests/mcp/test_server.py +++ b/tests/mcp/test_server.py @@ -11,6 +11,7 @@ from __future__ import annotations import asyncio +import os from typing import Any from unittest.mock import patch @@ -567,6 +568,7 @@ async def fake_stdio_server(stdout=None): with ( patch("gflow_cli.mcp.server.server") as mock_server, + patch("gflow_cli.mcp.server._apply_mcp_lease_wait_default"), patch("gflow_cli.mcp.server._configure_utf8_pipes"), patch("gflow_cli.mcp.server._redirect_stdout_to_stderr"), patch("gflow_cli.mcp.server.sys.stdout", MagicMock()), @@ -600,6 +602,7 @@ async def test_run_http_starts_streamable_http_on_mcp_path(self) -> None: with ( patch("gflow_cli.mcp.server.server") as mock_server, + patch("gflow_cli.mcp.server._apply_mcp_lease_wait_default"), patch("gflow_cli.mcp.server._configure_utf8_pipes"), patch("gflow_cli.mcp.server._serve", new_callable=AsyncMock) as mock_serve, ): @@ -629,6 +632,7 @@ async def test_run_sse_configures_and_starts(self) -> None: with ( patch("gflow_cli.mcp.server.server") as mock_server, + patch("gflow_cli.mcp.server._apply_mcp_lease_wait_default"), patch("gflow_cli.mcp.server._configure_utf8_pipes"), patch("gflow_cli.mcp.server._serve", new_callable=AsyncMock) as mock_serve, ): @@ -639,6 +643,26 @@ async def test_run_sse_configures_and_starts(self) -> None: mock_serve.assert_awaited_once_with(app, host="127.0.0.1", port=9999) +class TestMcpLeaseWaitDefault: + """The MCP server should wait out profile contention instead of the CLI's + fail-fast default — no human is watching an MCP call to just retry it.""" + + def test_sets_default_when_unset(self, monkeypatch: pytest.MonkeyPatch) -> None: + from gflow_cli.mcp.server import _apply_mcp_lease_wait_default + + monkeypatch.delenv("GFLOW_CLI_LEASE_WAIT_SECONDS", raising=False) + _apply_mcp_lease_wait_default() + assert os.environ["GFLOW_CLI_LEASE_WAIT_SECONDS"] == "180" + + def test_never_overrides_an_explicit_value(self, monkeypatch: pytest.MonkeyPatch) -> None: + """A user/operator override (including an explicit fail-fast `0`) wins.""" + from gflow_cli.mcp.server import _apply_mcp_lease_wait_default + + monkeypatch.setenv("GFLOW_CLI_LEASE_WAIT_SECONDS", "0") + _apply_mcp_lease_wait_default() + assert os.environ["GFLOW_CLI_LEASE_WAIT_SECONDS"] == "0" + + def test_mcp_retryable_matches_cli() -> None: """§6.5 (S24): the MCP error envelope's retryable flag must derive from the same shared classification the CLI JSON surface uses — no drift lists."""