From 9b3ba61b39fa66d752e57874213fac731311e5be Mon Sep 17 00:00:00 2001 From: dsarno Date: Thu, 15 Jan 2026 08:26:01 -0800 Subject: [PATCH] fix: parse and validate read_console types --- Server/src/services/tools/read_console.py | 42 +++++++++- .../integration/test_read_console_truncate.py | 77 +++++++++++++++++++ 2 files changed, 116 insertions(+), 3 deletions(-) diff --git a/Server/src/services/tools/read_console.py b/Server/src/services/tools/read_console.py index 5f61362f9..64ea8317e 100644 --- a/Server/src/services/tools/read_console.py +++ b/Server/src/services/tools/read_console.py @@ -8,7 +8,7 @@ from services.registry import mcp_for_unity_tool 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.legacy.unity_connection import async_send_command_with_retry @@ -31,7 +31,8 @@ async def read_console( action: Annotated[Literal['get', 'clear'], "Get or clear the Unity Editor console. Defaults to 'get' if omitted."] | None = None, 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, "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, @@ -51,7 +52,42 @@ async def read_console( unity_instance = get_unity_instance_from_context(ctx) # Set defaults if values are None 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' # Coerce booleans defensively (strings like 'true'/'false') diff --git a/Server/tests/integration/test_read_console_truncate.py b/Server/tests/integration/test_read_console_truncate.py index 86fcd6a25..a1d734794 100644 --- a/Server/tests/integration/test_read_console_truncate.py +++ b/Server/tests/integration/test_read_console_truncate.py @@ -183,3 +183,80 @@ async def fake_send(_cmd, params, **_kwargs): assert len(resp["data"]["items"]) == 5 assert resp["data"]["truncated"] is False 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 == {} \ No newline at end of file