Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
42 changes: 39 additions & 3 deletions Server/src/services/tools/read_console.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand All @@ -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')

Expand Down
77 changes: 77 additions & 0 deletions Server/tests/integration/test_read_console_truncate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == {}
Comment on lines +250 to +255
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Add tests for JSON types that parse to non-list or are malformed JSON to cover the new validation branch.

Now that types can be a JSON string that isn’t a list, add a test where the JSON parses but yields a non-list value (e.g. "error", 123, {}) to assert the specific "types must be a list" error. If parse_json_payload propagates JSON parsing failures, also add a test for malformed JSON so the new error path is fully covered and protected against regressions.


# 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 == {}