fix: parse and validate read_console types (#565)
parent
cf177b50af
commit
10e93b8548
|
|
@ -8,7 +8,7 @@ from mcp.types import ToolAnnotations
|
||||||
|
|
||||||
from services.registry import mcp_for_unity_tool
|
from services.registry import mcp_for_unity_tool
|
||||||
from services.tools import get_unity_instance_from_context
|
from services.tools import get_unity_instance_from_context
|
||||||
from services.tools.utils import coerce_int, coerce_bool
|
from services.tools.utils import coerce_int, coerce_bool, parse_json_payload
|
||||||
from transport.unity_transport import send_with_unity_instance
|
from transport.unity_transport import send_with_unity_instance
|
||||||
from transport.legacy.unity_connection import async_send_command_with_retry
|
from transport.legacy.unity_connection import async_send_command_with_retry
|
||||||
|
|
||||||
|
|
@ -31,7 +31,8 @@ async def read_console(
|
||||||
action: Annotated[Literal['get', 'clear'],
|
action: Annotated[Literal['get', 'clear'],
|
||||||
"Get or clear the Unity Editor console. Defaults to 'get' if omitted."] | None = None,
|
"Get or clear the Unity Editor console. Defaults to 'get' if omitted."] | None = None,
|
||||||
types: Annotated[list[Literal['error', 'warning',
|
types: Annotated[list[Literal['error', 'warning',
|
||||||
'log', 'all']], "Message types to get"] | None = None,
|
'log', 'all']] | str,
|
||||||
|
"Message types to get (accepts list or JSON string)"] | None = None,
|
||||||
count: Annotated[int | str,
|
count: Annotated[int | str,
|
||||||
"Max messages to return in non-paging mode (accepts int or string, e.g., 5 or '5'). Ignored when paging with page_size/cursor."] | None = None,
|
"Max messages to return in non-paging mode (accepts int or string, e.g., 5 or '5'). Ignored when paging with page_size/cursor."] | None = None,
|
||||||
filter_text: Annotated[str, "Text filter for messages"] | None = None,
|
filter_text: Annotated[str, "Text filter for messages"] | None = None,
|
||||||
|
|
@ -51,7 +52,42 @@ async def read_console(
|
||||||
unity_instance = get_unity_instance_from_context(ctx)
|
unity_instance = get_unity_instance_from_context(ctx)
|
||||||
# Set defaults if values are None
|
# Set defaults if values are None
|
||||||
action = action if action is not None else 'get'
|
action = action if action is not None else 'get'
|
||||||
types = types if types is not None else ['error', 'warning', 'log']
|
|
||||||
|
# Parse types if it's a JSON string (handles client compatibility issue #561)
|
||||||
|
if isinstance(types, str):
|
||||||
|
types = parse_json_payload(types)
|
||||||
|
# Validate types is a list after parsing
|
||||||
|
if types is not None and not isinstance(types, list):
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"message": (
|
||||||
|
f"types must be a list, got {type(types).__name__}. "
|
||||||
|
"If passing as JSON string, use format: '[\"error\", \"warning\"]'"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
if types is not None:
|
||||||
|
allowed_types = {"error", "warning", "log", "all"}
|
||||||
|
normalized_types = []
|
||||||
|
for entry in types:
|
||||||
|
if not isinstance(entry, str):
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"message": f"types entries must be strings, got {type(entry).__name__}"
|
||||||
|
}
|
||||||
|
normalized = entry.strip().lower()
|
||||||
|
if normalized not in allowed_types:
|
||||||
|
return {
|
||||||
|
"success": False,
|
||||||
|
"message": (
|
||||||
|
f"invalid types entry '{entry}'. "
|
||||||
|
f"Allowed values: {sorted(allowed_types)}"
|
||||||
|
)
|
||||||
|
}
|
||||||
|
normalized_types.append(normalized)
|
||||||
|
types = normalized_types
|
||||||
|
else:
|
||||||
|
types = ['error', 'warning', 'log']
|
||||||
|
|
||||||
format = format if format is not None else 'plain'
|
format = format if format is not None else 'plain'
|
||||||
# Coerce booleans defensively (strings like 'true'/'false')
|
# Coerce booleans defensively (strings like 'true'/'false')
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -183,3 +183,80 @@ async def test_read_console_paging(monkeypatch):
|
||||||
assert len(resp["data"]["items"]) == 5
|
assert len(resp["data"]["items"]) == 5
|
||||||
assert resp["data"]["truncated"] is False
|
assert resp["data"]["truncated"] is False
|
||||||
assert resp["data"]["nextCursor"] is None
|
assert resp["data"]["nextCursor"] is None
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_read_console_types_json_string(monkeypatch):
|
||||||
|
"""Test that read_console handles types parameter as JSON string (fixes issue #561)."""
|
||||||
|
tools = setup_tools()
|
||||||
|
read_console = tools["read_console"]
|
||||||
|
|
||||||
|
captured = {}
|
||||||
|
|
||||||
|
async def fake_send_with_unity_instance(_send_fn, _unity_instance, _command_type, params, **_kwargs):
|
||||||
|
captured["params"] = params
|
||||||
|
return {
|
||||||
|
"success": True,
|
||||||
|
"data": {"lines": [{"level": "error", "message": "test error"}]},
|
||||||
|
}
|
||||||
|
|
||||||
|
import services.tools.read_console as read_console_mod
|
||||||
|
monkeypatch.setattr(
|
||||||
|
read_console_mod,
|
||||||
|
"send_with_unity_instance",
|
||||||
|
fake_send_with_unity_instance,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Test with types as JSON string (the problematic case from issue #561)
|
||||||
|
resp = await read_console(ctx=DummyContext(), action="get", types='["error", "warning", "all"]')
|
||||||
|
assert resp["success"] is True
|
||||||
|
# Verify types was parsed correctly and sent as a list
|
||||||
|
assert isinstance(captured["params"]["types"], list)
|
||||||
|
assert captured["params"]["types"] == ["error", "warning", "all"]
|
||||||
|
|
||||||
|
# Test case normalization to lowercase
|
||||||
|
captured.clear()
|
||||||
|
resp = await read_console(ctx=DummyContext(), action="get", types='["ERROR", "Warning", "LOG"]')
|
||||||
|
assert resp["success"] is True
|
||||||
|
assert captured["params"]["types"] == ["error", "warning", "log"]
|
||||||
|
|
||||||
|
# Test with types as actual list (should still work)
|
||||||
|
captured.clear()
|
||||||
|
resp = await read_console(ctx=DummyContext(), action="get", types=["error", "warning"])
|
||||||
|
assert resp["success"] is True
|
||||||
|
assert isinstance(captured["params"]["types"], list)
|
||||||
|
assert captured["params"]["types"] == ["error", "warning"]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_read_console_types_validation(monkeypatch):
|
||||||
|
"""Test that read_console validates types entries and rejects invalid values."""
|
||||||
|
tools = setup_tools()
|
||||||
|
read_console = tools["read_console"]
|
||||||
|
|
||||||
|
captured = {}
|
||||||
|
|
||||||
|
async def fake_send_with_unity_instance(_send_fn, _unity_instance, _command_type, params, **_kwargs):
|
||||||
|
captured["params"] = params
|
||||||
|
return {"success": True, "data": {"lines": []}}
|
||||||
|
|
||||||
|
import services.tools.read_console as read_console_mod
|
||||||
|
monkeypatch.setattr(
|
||||||
|
read_console_mod,
|
||||||
|
"send_with_unity_instance",
|
||||||
|
fake_send_with_unity_instance,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Invalid entry in list should return a clear error and not send.
|
||||||
|
captured.clear()
|
||||||
|
resp = await read_console(ctx=DummyContext(), action="get", types='["error", "nope"]')
|
||||||
|
assert resp["success"] is False
|
||||||
|
assert "invalid types entry" in resp["message"]
|
||||||
|
assert captured == {}
|
||||||
|
|
||||||
|
# Non-string entry should return a clear error and not send.
|
||||||
|
captured.clear()
|
||||||
|
resp = await read_console(ctx=DummyContext(), action="get", types='[1, "error"]')
|
||||||
|
assert resp["success"] is False
|
||||||
|
assert "types entries must be strings" in resp["message"]
|
||||||
|
assert captured == {}
|
||||||
Loading…
Reference in New Issue