Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion Server/src/services/custom_tool_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,14 @@ async def execute_tool(
message=f"Tool '{tool_name}' not found for project {project_id}",
)

# Custom tools are frequently long-running (captures, image comparison,
# batch imports), so the hub's default 30s command budget times them out
# while they are still working. Give them a generous default; callers can
# still override with an explicit timeout_seconds (or timeoutSeconds) parameter, up to 3600.
# The hub reads timeout_seconds first, so a default added beside a caller's timeoutSeconds would win.
if "timeout_seconds" not in params and "timeoutSeconds" not in params:
params["timeout_seconds"] = 300

response = await send_with_unity_instance(
async_send_command_with_retry,
unity_instance,
Expand Down Expand Up @@ -216,8 +224,13 @@ async def _poll_until_complete(
if time.time() > deadline:
return MCPResponse(
success=False,
message=f"Timeout waiting for {tool_name} to complete",
message=(
f"Timeout waiting for {tool_name} to complete. The operation may "
"still be running in Unity — do not assume it finished or failed; "
"verify its effects before continuing."
),
data=self._safe_response(response),
# No retry hint: the operation may still be running, and resubmitting it could run it twice.
)

await asyncio.sleep(poll_interval)
Expand Down Expand Up @@ -281,6 +294,7 @@ def _normalize_response(self, response) -> MCPResponse:
error=response.get("error"),
data=response.get(
"data", response) if "data" not in response else response["data"],
hint=response.get("hint"),
)

success = True
Expand Down
17 changes: 16 additions & 1 deletion Server/src/transport/unity_transport.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Transport helpers for routing commands to Unity."""
from __future__ import annotations

import asyncio
import logging
from typing import Awaitable, Callable, TypeVar

Expand Down Expand Up @@ -101,11 +102,25 @@ async def send_with_unity_instance(
except Exception as exc:
# NOTE: asyncio.TimeoutError has an empty str() by default, which is confusing for clients.
err = str(exc) or f"{type(exc).__name__}"
data: dict | None = None
if isinstance(exc, (asyncio.TimeoutError, TimeoutError)):
# A bare "TimeoutError" reads as noise and agents plough on as if the
# command ran. Spell out what a timeout means and what to do about it.
err = (
f"Unity did not respond to '{command_type}' within its time budget. "
"The editor is likely compiling, reloading, importing assets, or still "
"running the operation. The command may or may not have executed — do NOT "
"assume it ran; verify its effects (or re-run it) once "
"mcpforunity://editor/state reports advice.ready_for_tools=true. "
"For long-running tools, pass timeout_seconds in the tool parameters "
"(honored up to 3600)."
)
data = {"reason": "unity_timeout"}
# Fail fast with a retry hint instead of hanging for COMMAND_TIMEOUT.
# The client can decide whether retrying is appropriate for the command.
return normalize_unity_response(
MCPResponse(success=False, error=err,
hint="retry").model_dump()
hint="retry", data=data).model_dump()
)

if unity_instance:
Expand Down
122 changes: 122 additions & 0 deletions Server/tests/test_custom_tool_timeouts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
"""Timeouts on custom tools must be long-budget, loud, and impossible to misread.

Real-world failure (2026-08-11): a long-running project tool (image comparison)
exceeded the hub's default 30s command budget and the caller got the five words
{"success":false,"error":"TimeoutError","hint":null} — after which the agent
carried on reading stale on-disk artifacts as if the tool had run. Three fixes:
custom tools default to a 300s budget, _normalize_response no longer drops the
hint, and transport-level timeouts explain that the command may not have run.
"""

import asyncio

import pytest

from core.config import config
from models.models import MCPResponse, ToolDefinitionModel
from services.custom_tool_service import CustomToolService


class _NoopMcp:
def custom_route(self, _path, methods=None): # noqa: ARG002
def _decorator(fn):
return fn

return _decorator

def tool(self, name=None, description=None): # noqa: ARG002
def _decorator(fn):
return fn

return _decorator


def _make_service() -> CustomToolService:
return CustomToolService(_NoopMcp(), project_scoped_tools=True)


def test_normalize_response_preserves_hint():
service = _make_service()
result = service._normalize_response(
{"success": False, "error": "boom", "hint": "retry", "data": {"reason": "x"}}
)
assert isinstance(result, MCPResponse)
assert result.hint == "retry"
assert result.data == {"reason": "x"}


@pytest.mark.asyncio
async def test_execute_tool_defaults_long_timeout(monkeypatch):
service = _make_service()
definition = ToolDefinitionModel(name="long_running_tool", description="long tool")
service._register_tool("proj-1", definition)

seen_params: dict = {}

async def fake_send(_send_fn, _instance, _command, params, **kwargs): # noqa: ARG001
seen_params.update(params)
return {"success": True, "message": "ok"}

monkeypatch.setattr(
"services.custom_tool_service.send_with_unity_instance", fake_send)

result = await service.execute_tool("proj-1", "long_running_tool", "Playground@abc", {})
assert result.success is True
assert seen_params["timeout_seconds"] == 300

# An explicit caller value must win over the default.
seen_params.clear()
await service.execute_tool(
"proj-1", "long_running_tool", "Playground@abc", {"timeout_seconds": 900})
assert seen_params["timeout_seconds"] == 900

# So must the camelCase spelling the hub also honors: no snake_case default beside it.
seen_params.clear()
await service.execute_tool(
"proj-1", "long_running_tool", "Playground@abc", {"timeoutSeconds": 1200})
assert seen_params == {"timeoutSeconds": 1200}


@pytest.mark.asyncio
async def test_polling_deadline_does_not_suggest_resubmitting(monkeypatch):
"""At the polling deadline the operation may still run: say so, and do not hint a retry."""
service = _make_service()

async def still_running(_send_fn, _instance, _command, params, **kwargs): # noqa: ARG001
return {"_mcp_status": "pending", "message": "working"}

monkeypatch.setattr(
"services.custom_tool_service.send_with_unity_instance", still_running)

result = await service._poll_until_complete(
"long_running_tool", "Playground@abc", {}, {"_mcp_status": "pending"}, "status",
max_poll_seconds=0.01)

assert result.success is False
assert "may still be running" in result.message
assert result.hint is None


@pytest.mark.asyncio
async def test_transport_timeout_error_is_self_explanatory(monkeypatch):
from transport import unity_transport

monkeypatch.setattr(config, "transport_mode", "http")

async def raise_timeout(*args, **kwargs): # noqa: ARG001
raise asyncio.TimeoutError()

monkeypatch.setattr(
unity_transport.PluginHub, "send_command_for_instance", raise_timeout)

async def unused_send_fn(*args, **kwargs): # noqa: ARG001
raise AssertionError("stdio path should not be used in http mode")

response = await unity_transport.send_with_unity_instance(
unused_send_fn, "Playground@abc", "long_running_tool", {})

assert response["success"] is False
assert "may or may not have executed" in response["error"]
assert "timeout_seconds" in response["error"]
assert response["hint"] == "retry"
assert response["data"] == {"reason": "unity_timeout"}