From 41108d970b49a6d142de384ff56f5535efb45258 Mon Sep 17 00:00:00 2001 From: Hyeonwoo Jo Date: Thu, 17 Sep 2026 23:16:54 +0900 Subject: [PATCH] feat(mcp): wait out profile contention instead of hard-failing The CLI's fail-fast profile-lease default (GFLOW_CLI_LEASE_WAIT_SECONDS=0) is right for a one-shot terminal command a human can just retry. The MCP server has no human in that loop: two tool calls landing close together on the same profile (a client retry, two agents sharing a profile) would otherwise hard-fail with ProfileLockedError instead of simply running after the call already in flight finishes. All three MCP entry points (stdio/http/sse) now set a 180s wait default via os.environ.setdefault before anything touches Settings (cached via lru_cache) -- long enough to outlast a typical single generation, and a no-op for anyone who has already set the var themselves. Co-Authored-By: Claude Sonnet 5 Signed-off-by: Hyeonwoo Jo --- src/gflow_cli/mcp/server.py | 24 ++++++++++++++++++++++++ tests/mcp/test_server.py | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) 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."""