Merge pull request #2043 from debpalash/fix/mcp-timeout-follows-backend

fix(mcp): tools wait for the backend's own budget, not a fixed 120 s

CHANGELOG.md

@@ -28,6 +28,7 @@ the frozen-backend fallback mirror it for their toolchains.
 - An engine that fails to start now says whether it timed out, crashed (with its exit code and last output) or answered wrongly, instead of "did not signal ready: None" (#2037, #2026)
 - Transcribing an M4A file with PyTorch Whisper works, instead of failing with "Format not recognised" (#2042, #2039)
 - PyTorch Whisper runs on 6 GB NVIDIA cards instead of falling back to CPU, because its memory check now fits the model it loads (#2044, #2041)
+- MCP tools wait as long as the backend does, so a long transcription no longer fails at 120 s with an empty error (#2043, #2040)
 
 ### CI
 

backend/mcp_server.py

@@ -271,19 +271,61 @@ def _write_output(audio_id: str, raw: bytes) -> str:
     return path
 
 
-def _post_timeout_s() -> float:
-    """Seconds the tools wait on a backend POST (OMNIVOICE_MCP_TIMEOUT_S,
-    default 120). A CPU host renders a paragraph in minutes and serializes
-    generations, so an agent behind another render used to hit the fixed
-    budget with an empty-message timeout; the knob follows the backend's own
-    OMNIVOICE_GENERATE_TIMEOUT_S when a deployment raises that."""
-    raw = os.environ.get("OMNIVOICE_MCP_TIMEOUT_S", "").strip()
+# Extra seconds a tool waits past the backend's own budget, so the backend's
+# error (which says what ran out) reaches the agent instead of an empty
+# client-side timeout (#2040).
+_BACKEND_GRACE_S = 30.0
+
+
+def _env_seconds(name: str, default: float) -> float:
+    raw = os.environ.get(name, "").strip()
+    if not raw:
+        return default
     try:
-        value = float(raw) if raw else 120.0
+        value = float(raw)
     except ValueError:
-        logger.warning("OMNIVOICE_MCP_TIMEOUT_S=%r is not a number; using 120", raw)
-        return 120.0
-    return value if value > 0 else 120.0
+        logger.warning("%s=%r is not a number; using %g", name, raw, default)
+        return default
+    return value if value > 0 else default
+
+
+def _backend_budget_s(kind: str, text: str = "") -> float | None:
+    """The backend's own execution budget for this kind of request, read from
+    the environment variables and defaults the backend itself uses.
+
+    The MCP server cannot see which device the backend runs on, so generation
+    assumes the larger CPU base. The backend still stops a job at its own
+    budget; this only keeps the tool from giving up first.
+    """
+    if kind == "transcribe":
+        # run_transcribe_guarded starts this clock when the job is submitted,
+        # so time spent queued in the pool already counts against it.
+        return _env_seconds("OMNIVOICE_ASR_TRANSCRIBE_TIMEOUT_S", 300.0)
+    if kind == "generate":
+        base = max(
+            _env_seconds("OMNIVOICE_GENERATE_TIMEOUT_S", 300.0),
+            _env_seconds("OMNIVOICE_CPU_GENERATE_TIMEOUT_S", 600.0),
+        )
+        # As model_manager.generate_timeout_s: +1 s per 40 characters past 1200.
+        execution = base + max(0, len(text or "") - 1200) / 40.0
+        # A generation first waits in the GPU pool's queue, on its own clock
+        # (model_manager.GPU_QUEUE_TIMEOUT_S), before that budget starts.
+        return _env_seconds("OMNIVOICE_GPU_QUEUE_TIMEOUT_S", 1800.0) + execution
+    return None
+
+
+def _post_timeout_s(kind: str = "", text: str = "") -> float:
+    """Seconds a tool waits on a backend POST.
+
+    An explicit OMNIVOICE_MCP_TIMEOUT_S wins. Otherwise the tool waits for the
+    backend's own budget for that request plus a grace period, and never less
+    than 120 s. A fixed 120 s used to cut off transcriptions the backend would
+    have finished (its ASR budget is 300 s) with an empty error (#2040).
+    """
+    if os.environ.get("OMNIVOICE_MCP_TIMEOUT_S", "").strip():
+        return _env_seconds("OMNIVOICE_MCP_TIMEOUT_S", 120.0)
+    budget = _backend_budget_s(kind, text)
+    return 120.0 if budget is None else max(120.0, budget + _BACKEND_GRACE_S)
 
 
 def _maybe_number(value):
@@ -397,9 +439,12 @@ def create_mcp_server():
             r.raise_for_status()
             return r.json()
 
-    async def _api_post_form(path: str, data: dict, files: dict | None = None):
+    async def _api_post_form(
+        path: str, data: dict, files: dict | None = None, *, timeout: float | None = None
+    ):
         import httpx
-        async with httpx.AsyncClient(base_url=_api_base(), timeout=_post_timeout_s()) as c:
+        wait = _post_timeout_s() if timeout is None else timeout
+        async with httpx.AsyncClient(base_url=_api_base(), timeout=wait) as c:
             r = await c.post(path, data=data, files=files or {})
             r.raise_for_status()
             return r
@@ -471,7 +516,9 @@ def create_mcp_server():
         if instruct:
             form["instruct"] = instruct
 
-        r = await _api_post_form("/generate", data=form)
+        r = await _api_post_form(
+            "/generate", data=form, timeout=_post_timeout_s("generate", text)
+        )
 
         audio_id = r.headers.get("X-Audio-Id", "unknown")
         gen_time = _maybe_number(r.headers.get("X-Gen-Time", "?"))
@@ -547,6 +594,7 @@ def create_mcp_server():
             "/transcribe", data=data,
             files={"audio": (f"audio{_sniff_audio_ext(raw)}", raw,
                              "application/octet-stream")},
+            timeout=_post_timeout_s("transcribe"),
         )
         return str(r.json())
 

docs/mcp.md

@@ -27,7 +27,7 @@ or another tool by path:
 | Variable | Values | Effect |
 |---|---|---|
 | `OMNIVOICE_MCP_OUTPUT_MODE` | `resources` (default) · `files` · `both` | `resources` returns `wav_base64` inline (the original contract). `files` returns `audio_url` (the render served at `/audio/<audio_id>.wav`, which the backend keeps anyway) and, when a base path is set, `output_path` — the WAV written into that directory. `both` returns everything. |
-| `OMNIVOICE_MCP_TIMEOUT_S` | seconds (default `120`) | How long a tool waits on the backend. CPU hosts render a paragraph in minutes and serialize generations, so an agent queued behind another render can outlast the default; raise it in step with `OMNIVOICE_GENERATE_TIMEOUT_S`. |
+| `OMNIVOICE_MCP_TIMEOUT_S` | seconds (default: follows the backend) | How long a tool waits on the backend. Unset, each tool waits as long as the backend itself would, plus 30 s, and never less than 120 s. `transcribe` follows `OMNIVOICE_ASR_TRANSCRIBE_TIMEOUT_S` (300 s by default, queue time included). `generate_speech` follows the backend's queue wait (`OMNIVOICE_GPU_QUEUE_TIMEOUT_S`, 1,800 s by default) plus the larger of `OMNIVOICE_GENERATE_TIMEOUT_S` and `OMNIVOICE_CPU_GENERATE_TIMEOUT_S`, plus 1 s per 40 characters past 1,200. The agent then gets the backend's own timeout error instead of an empty one. Set this to use one fixed wait for every tool. On a machine that hasn't downloaded the OmniVoice model yet, the first `generate_speech` also downloads it (about 2.3 GB) inside the backend's budget: install the model first (first-run setup, or Model Catalogue) or raise `OMNIVOICE_GENERATE_TIMEOUT_S` before the first call. |
 | `OMNIVOICE_MCP_BASE_PATH` | a directory | The **security boundary** for file-shaped traffic. `transcribe(audio_path=…)` and `clone_voice(ref_audio_path=…)` read only from inside it (relative paths resolve against it, absolute paths must already lie within it, symlinks are resolved before the check), and files mode writes only into it. With no base path configured, path arguments are refused with a reason. |
 
 Input files are opened through confined, no-follow descriptors after path

tests/test_mcp_timeouts_2040.py

@@ -0,0 +1,88 @@
+"""#2040 — the MCP tools gave up after a fixed 120 s while the backend's own
+budgets run longer (ASR: 300 s), so a request the backend would have finished
+came back as an empty client-side timeout."""
+import pytest
+
+_BUDGET_VARS = (
+    "OMNIVOICE_MCP_TIMEOUT_S",
+    "OMNIVOICE_ASR_TRANSCRIBE_TIMEOUT_S",
+    "OMNIVOICE_GENERATE_TIMEOUT_S",
+    "OMNIVOICE_CPU_GENERATE_TIMEOUT_S",
+    "OMNIVOICE_GPU_QUEUE_TIMEOUT_S",
+)
+QUEUE = 1800.0
+GRACE = 30.0
+
+
+@pytest.fixture
+def post_timeout(monkeypatch):
+    for name in _BUDGET_VARS:
+        monkeypatch.setenv(name, "1")  # recorded, so the teardown restores it
+        monkeypatch.delenv(name)
+    import mcp_server
+
+    return mcp_server._post_timeout_s
+
+
+def test_transcribe_waits_past_the_backends_asr_budget(post_timeout):
+    assert post_timeout("transcribe") == 300.0 + GRACE
+
+
+def test_a_raised_asr_budget_is_followed(post_timeout, monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_ASR_TRANSCRIBE_TIMEOUT_S", "900")
+    assert post_timeout("transcribe") == 900.0 + GRACE
+
+
+def test_generation_covers_the_queue_and_the_length_scaled_budget(post_timeout):
+    assert post_timeout("generate", "short") == QUEUE + 600.0 + GRACE
+    assert post_timeout("generate", "x" * 1600) == QUEUE + 610.0 + GRACE
+
+
+def test_the_larger_cpu_generation_budget_wins(post_timeout, monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_CPU_GENERATE_TIMEOUT_S", "900")
+    assert post_timeout("generate", "short") == QUEUE + 900.0 + GRACE
+
+
+def test_a_raised_gpu_generation_budget_wins_when_larger(post_timeout, monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_GENERATE_TIMEOUT_S", "1200")
+    assert post_timeout("generate", "short") == QUEUE + 1200.0 + GRACE
+
+
+def test_a_shorter_queue_budget_is_followed(post_timeout, monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_GPU_QUEUE_TIMEOUT_S", "60")
+    assert post_timeout("generate", "short") == 60.0 + 600.0 + GRACE
+
+
+def test_an_explicit_mcp_timeout_still_wins(post_timeout, monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_MCP_TIMEOUT_S", "45")
+    assert post_timeout("transcribe") == 45.0
+    assert post_timeout("generate", "x" * 5000) == 45.0
+
+
+def test_other_posts_keep_the_old_default(post_timeout):
+    assert post_timeout() == 120.0
+
+
+def test_unusable_values_fall_back(post_timeout, monkeypatch):
+    monkeypatch.setenv("OMNIVOICE_ASR_TRANSCRIBE_TIMEOUT_S", "soon")
+    assert post_timeout("transcribe") == 300.0 + GRACE
+    monkeypatch.setenv("OMNIVOICE_MCP_TIMEOUT_S", "-5")
+    assert post_timeout("transcribe") == 120.0
+
+
+# ── Parity with the backend's own clocks: a tool must never give up first ──
+
+
+def test_transcribe_never_gives_up_before_the_backend(post_timeout):
+    from services import asr_backend
+
+    assert post_timeout("transcribe") > asr_backend.ASR_TRANSCRIBE_TIMEOUT_S
+
+
+@pytest.mark.parametrize("device", ["cpu", "cuda", "mps"])
+@pytest.mark.parametrize("text", ["short", "x" * 5000])
+def test_generation_never_gives_up_before_the_backend(post_timeout, device, text):
+    from services import model_manager as mm
+
+    backend_worst = mm.GPU_QUEUE_TIMEOUT_S + mm.generate_timeout_s(text, execution_device=device)
+    assert post_timeout("generate", text) > backend_worst