From 1831ccb79e1b56dac845213beeb33c971accd328 Mon Sep 17 00:00:00 2001
From: haranishi <223786623+haranishi@users.noreply.github.com>
Date: Mon, 12 Oct 2026 07:39:44 +0900
Subject: [PATCH] fix: handle JSON-encoded console arguments and menu paths
---
MCPForUnity/Editor/Helpers/ParamCoercion.cs | 19 +++
MCPForUnity/Editor/Tools/ExecuteMenuItem.cs | 2 +
MCPForUnity/Editor/Tools/ReadConsole.cs | 4 +-
.../src/services/tools/execute_menu_item.py | 2 +-
Server/src/services/tools/read_console.py | 27 ++--
Server/src/services/tools/utils.py | 14 ++
.../test_json_encoded_tool_strings.py | 89 ++++++++++++
Server/tests/test_tool_test_symmetry.py | 1 -
.../Tools/QuotedToolArgumentsTests.cs | 127 ++++++++++++++++++
.../Tools/QuotedToolArgumentsTests.cs.meta | 2 +
.../reference/tools/core/execute_menu_item.md | 2 +-
.../docs/reference/tools/core/read_console.md | 4 +-
12 files changed, 276 insertions(+), 17 deletions(-)
create mode 100644 Server/tests/integration/test_json_encoded_tool_strings.py
create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs
create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs.meta
diff --git a/MCPForUnity/Editor/Helpers/ParamCoercion.cs b/MCPForUnity/Editor/Helpers/ParamCoercion.cs
index 38a8e32d74..f1b6eed969 100644
--- a/MCPForUnity/Editor/Helpers/ParamCoercion.cs
+++ b/MCPForUnity/Editor/Helpers/ParamCoercion.cs
@@ -1,6 +1,7 @@
using System;
using System.Globalization;
using Newtonsoft.Json.Linq;
+using Newtonsoft.Json;
namespace MCPForUnity.Editor.Helpers
{
@@ -260,6 +261,24 @@ public static string CoerceString(JToken token, string defaultValue = null)
return string.IsNullOrEmpty(s) ? defaultValue : s;
}
+ /// Opt in to one JSON string decoding layer, preserving other input.
+ internal static string DecodeJsonString(string value)
+ {
+ if (string.IsNullOrWhiteSpace(value)) return value;
+ string trimmed = value.Trim();
+ if (trimmed.Length < 2 || trimmed[0] != '"' || trimmed[trimmed.Length - 1] != '"')
+ return value;
+ try
+ {
+ var decoded = JToken.Parse(trimmed);
+ return decoded.Type == JTokenType.String ? decoded.Value() : value;
+ }
+ catch (JsonException)
+ {
+ return value;
+ }
+ }
+
///
/// Coerces a JToken to an enum value, handling strings.
///
diff --git a/MCPForUnity/Editor/Tools/ExecuteMenuItem.cs b/MCPForUnity/Editor/Tools/ExecuteMenuItem.cs
index f606f774f1..dedf44925a 100644
--- a/MCPForUnity/Editor/Tools/ExecuteMenuItem.cs
+++ b/MCPForUnity/Editor/Tools/ExecuteMenuItem.cs
@@ -23,6 +23,8 @@ public static object HandleCommand(JObject @params)
{
McpLog.Info("[ExecuteMenuItem] Handling menu item command");
string menuPath = @params["menu_path"]?.ToString() ?? @params["menuPath"]?.ToString();
+ // Decode once here, including direct/batch calls, before checking the blacklist.
+ menuPath = ParamCoercion.DecodeJsonString(menuPath);
if (string.IsNullOrWhiteSpace(menuPath))
{
return new ErrorResponse("Required parameter 'menu_path' or 'menuPath' is missing or empty.");
diff --git a/MCPForUnity/Editor/Tools/ReadConsole.cs b/MCPForUnity/Editor/Tools/ReadConsole.cs
index b4abb5882d..e70c14c9b6 100644
--- a/MCPForUnity/Editor/Tools/ReadConsole.cs
+++ b/MCPForUnity/Editor/Tools/ReadConsole.cs
@@ -172,7 +172,7 @@ public static object HandleCommand(JObject @params)
}
var p = new ToolParams(@params);
- string action = p.Get("action", "get").ToLower();
+ string action = ParamCoercion.DecodeJsonString(p.Get("action", "get")).ToLowerInvariant();
try
{
@@ -190,7 +190,7 @@ public static object HandleCommand(JObject @params)
int? pageSize = p.GetInt("pageSize");
int? cursor = p.GetInt("cursor");
string filterText = p.Get("filterText");
- string format = p.Get("format", "plain").ToLower();
+ string format = ParamCoercion.DecodeJsonString(p.Get("format", "plain")).ToLowerInvariant();
bool includeStacktrace = p.GetBool("includeStacktrace", false);
if (types.Contains("all"))
diff --git a/Server/src/services/tools/execute_menu_item.py b/Server/src/services/tools/execute_menu_item.py
index af6d46b861..74b33ede74 100644
--- a/Server/src/services/tools/execute_menu_item.py
+++ b/Server/src/services/tools/execute_menu_item.py
@@ -23,7 +23,7 @@
async def execute_menu_item(
ctx: Context,
menu_path: Annotated[str,
- "Menu path for 'execute' or 'exists' (e.g., 'File/Save Project')"] | None = None,
+ "Menu path (e.g., 'File/Save Project'). Also accepts a JSON-encoded string."] | None = None,
) -> MCPResponse:
unity_instance = await get_unity_instance_from_context(ctx)
params_dict: dict[str, Any] = {"menuPath": menu_path}
diff --git a/Server/src/services/tools/read_console.py b/Server/src/services/tools/read_console.py
index 6a642dcec9..9cf5586551 100644
--- a/Server/src/services/tools/read_console.py
+++ b/Server/src/services/tools/read_console.py
@@ -5,10 +5,11 @@
from fastmcp import Context
from mcp.types import ToolAnnotations
+from pydantic import BeforeValidator
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, parse_json_payload
+from services.tools.utils import coerce_int, coerce_bool, parse_json_payload, decode_json_string
from transport.unity_transport import send_with_unity_instance
from transport.legacy.unity_connection import async_send_command_with_retry
@@ -35,7 +36,8 @@ def _strip_stacktrace_from_list(items: list) -> None:
async def read_console(
ctx: Context,
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. Also accepts a JSON-encoded string.",
+ BeforeValidator(decode_json_string)] | None = None,
types: Annotated[list[Literal['error', 'warning',
'log', 'all']] | str,
"Message types to get (accepts list or JSON string)"] | None = None,
@@ -47,7 +49,8 @@ async def read_console(
cursor: Annotated[int | str,
"Opaque cursor for paging (0-based offset). Defaults to 0."] | None = None,
format: Annotated[Literal['plain', 'detailed',
- 'json'], "Output format"] | None = None,
+ 'json'], "Output format. Also accepts a JSON-encoded string.",
+ BeforeValidator(decode_json_string)] | None = None,
include_stacktrace: Annotated[bool | str,
"Include stack traces in output (accepts true/false or 'true'/'false')"] | None = None,
) -> dict[str, Any]:
@@ -55,7 +58,16 @@ async def read_console(
# Removed session_state import
unity_instance = await get_unity_instance_from_context(ctx)
# Set defaults if values are None
- action = action if action is not None else 'get'
+ action = decode_json_string(action) if action is not None else 'get'
+ format = decode_json_string(format) if format is not None else 'plain'
+ if isinstance(action, str):
+ action = action.lower()
+ if action not in ("get", "clear"):
+ return {"success": False, "message": "action must be 'get' or 'clear'."}
+ if isinstance(format, str):
+ format = format.lower()
+ if format not in ("plain", "detailed", "json"):
+ return {"success": False, "message": "format must be 'plain', 'detailed', or 'json'."}
# Parse types if it's a JSON string (handles client compatibility issue #561)
if isinstance(types, str):
@@ -92,17 +104,12 @@ async def read_console(
else:
types = ['error', 'warning', 'log']
- format = format if format is not None else 'plain'
# Coerce booleans defensively (strings like 'true'/'false')
include_stacktrace = coerce_bool(include_stacktrace, default=False)
coerced_page_size = coerce_int(page_size, default=None)
coerced_cursor = coerce_int(cursor, default=None)
- # Normalize action if it's a string
- if isinstance(action, str):
- action = action.lower()
-
# Coerce count defensively (string/float -> int).
# Important: leaving count unset previously meant "return all console entries", which can be extremely slow
# (and can exceed the plugin command timeout when Unity has a large console).
@@ -124,7 +131,7 @@ async def read_console(
"filterText": filter_text,
"pageSize": coerced_page_size,
"cursor": coerced_cursor,
- "format": format.lower() if isinstance(format, str) else format,
+ "format": format,
"includeStacktrace": include_stacktrace
}
diff --git a/Server/src/services/tools/utils.py b/Server/src/services/tools/utils.py
index 93c7008585..b8c2ea4244 100644
--- a/Server/src/services/tools/utils.py
+++ b/Server/src/services/tools/utils.py
@@ -62,6 +62,20 @@ def parse_json_payload(value: Any) -> Any:
return value
+def decode_json_string(value: Any) -> Any:
+ """Opt in to one JSON string decoding layer; leave other values unchanged."""
+ if not isinstance(value, str):
+ return value
+ trimmed = value.strip()
+ if not (trimmed.startswith('"') and trimmed.endswith('"')):
+ return value
+ try:
+ decoded = json.loads(value)
+ return decoded if isinstance(decoded, str) else value
+ except (json.JSONDecodeError, ValueError):
+ return value
+
+
def coerce_int(value: Any, default: int | None = None) -> int | None:
"""Attempt to coerce a loosely-typed value to an integer."""
if value is None:
diff --git a/Server/tests/integration/test_json_encoded_tool_strings.py b/Server/tests/integration/test_json_encoded_tool_strings.py
new file mode 100644
index 0000000000..0df54d4b63
--- /dev/null
+++ b/Server/tests/integration/test_json_encoded_tool_strings.py
@@ -0,0 +1,89 @@
+"""Exercise MCP validation in a fresh interpreter, outside integration SDK stubs."""
+import os
+from pathlib import Path
+import subprocess
+import sys
+
+
+def test_json_encoded_strings_through_real_mcp():
+ # integration/conftest.py installs FastMCP stubs. A child interpreter lets this
+ # regression use the real SDK without changing the shared test environment.
+ script = r'''
+import asyncio
+import json
+from fastmcp import Client, FastMCP
+import services.tools.read_console as console
+import services.tools.execute_menu_item as menu
+
+calls = []
+
+async def instance(_ctx):
+ return "test-editor"
+
+async def send(_sender, unity_instance, command, params):
+ assert unity_instance == "test-editor"
+ calls.append((command, dict(params)))
+ return {"success": True, "message": "test response", "data": {"lines": []}}
+
+async def main():
+ for module in (console, menu):
+ module.get_unity_instance_from_context = instance
+ module.send_with_unity_instance = send
+ server = FastMCP("JSON string argument tests")
+ server.tool(console.read_console)
+ server.tool(menu.execute_menu_item)
+ async with Client(server) as client:
+ tools = await client.list_tools()
+ schema = next(tool.inputSchema for tool in tools if tool.name == "read_console")
+ options = schema["properties"]["action"]["anyOf"]
+ assert next(option["enum"] for option in options if "enum" in option) == ["get", "clear"]
+
+ filter_text = '\"build failed\"'
+ await client.call_tool("read_console", {
+ "action": "get", "format": "json", "filter_text": filter_text,
+ })
+ assert calls[-1][1]["action"] == "get"
+ assert calls[-1][1]["format"] == "json"
+ assert calls[-1][1]["filterText"] == filter_text
+
+ await client.call_tool("read_console", {
+ "action": json.dumps("get"), "format": json.dumps("plain"),
+ })
+ assert calls[-1][0] == "read_console"
+ assert calls[-1][1]["action"] == "get"
+ assert calls[-1][1]["format"] == "plain"
+ assert calls[-1][1]["count"] == 10
+
+ await client.call_tool("read_console", {"action": json.dumps("clear")})
+ assert calls[-1][1]["action"] == "clear"
+
+ count = len(calls)
+ for action in ['\"delete\"', '\"get', json.dumps(json.dumps("get")), 42]:
+ result = await client.call_tool("read_console", {"action": action}, raise_on_error=False)
+ assert result.is_error
+ assert len(calls) == count, "Invalid actions must never reach Unity"
+
+ path = "Tools/MCPForUnityTests/Quoted argument probe"
+ for supplied in [path, json.dumps(path), json.dumps(json.dumps(path))]:
+ await client.call_tool("execute_menu_item", {"menu_path": supplied})
+ assert calls[-1] == ("execute_menu_item", {"menuPath": supplied})
+
+ # Direct Python callers bypass Pydantic; they must obey the same one-layer limit.
+ await console.read_console(ctx=None, action=json.dumps("get"), format=json.dumps("plain"))
+ assert calls[-1][1]["action"] == "get"
+ count = len(calls)
+ for action in [json.dumps(json.dumps("get")), json.dumps("delete")]:
+ result = await console.read_console(ctx=None, action=action)
+ assert result["success"] is False
+ assert len(calls) == count
+
+asyncio.run(main())
+print("real MCP argument validation passed")
+'''
+ server_root = Path(__file__).resolve().parents[2]
+ env = dict(os.environ, PYTHONPATH=str(server_root / "src"),
+ DISABLE_TELEMETRY="true", UNITY_MCP_DISABLE_TELEMETRY="true")
+ result = subprocess.run([sys.executable, "-c", script], cwd=server_root,
+ env=env, capture_output=True, text=True, timeout=45)
+ assert result.returncode == 0, result.stdout + result.stderr
+ assert "real MCP argument validation passed" in result.stdout
diff --git a/Server/tests/test_tool_test_symmetry.py b/Server/tests/test_tool_test_symmetry.py
index e51ce850f5..b14215dbbd 100644
--- a/Server/tests/test_tool_test_symmetry.py
+++ b/Server/tests/test_tool_test_symmetry.py
@@ -27,7 +27,6 @@
# Tool modules without any test reference today. Do NOT add to this list for new
# tools -- new tools must ship with a test. Remove an entry once coverage lands.
KNOWN_UNTESTED = {
- "execute_menu_item",
"manage_shader",
"manage_tools",
}
diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs
new file mode 100644
index 0000000000..5f68138a91
--- /dev/null
+++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs
@@ -0,0 +1,127 @@
+using System;
+using MCPForUnity.Editor.Tools;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Linq;
+using NUnit.Framework;
+using UnityEditor;
+using UnityEngine;
+using UnityEngine.TestTools;
+
+namespace MCPForUnityTests.Editor.Tools
+{
+ public class QuotedToolArgumentsTests
+ {
+ private const string MenuPath = "Tools/MCPForUnityTests/Quoted argument probe";
+ private static int _menuCalls;
+
+ [MenuItem(MenuPath)]
+ private static void OnTestMenu() => _menuCalls++;
+
+ [SetUp]
+ public void SetUp() => _menuCalls = 0;
+
+ [Test]
+ public void Console_GetWithJsonEncodedAction_ReturnsMatchingLog()
+ {
+ string marker = "Quoted action probe " + Guid.NewGuid().ToString("N");
+ Debug.Log(marker);
+ var result = JObject.FromObject(ReadConsole.HandleCommand(new JObject
+ {
+ ["action"] = JsonConvert.SerializeObject("get"),
+ ["types"] = new JArray("log"), ["format"] = "plain", ["filterText"] = marker, ["count"] = 10
+ }));
+ Assert.IsTrue(result.Value("success"), result.ToString());
+ StringAssert.Contains(marker, result["data"][0].Value());
+ }
+
+ [Test]
+ public void Console_JsonEncodedFormat_ReturnsPlainStrings()
+ {
+ string marker = "Quoted format probe " + Guid.NewGuid().ToString("N");
+ Debug.Log(marker);
+ var result = JObject.FromObject(ReadConsole.HandleCommand(new JObject
+ {
+ ["action"] = "get", ["types"] = new JArray("log"),
+ ["format"] = JsonConvert.SerializeObject("plain"), ["filterText"] = marker, ["count"] = 10
+ }));
+ Assert.IsTrue(result.Value("success"), result.ToString());
+ Assert.AreEqual(JTokenType.String, result["data"][0].Type);
+ StringAssert.Contains(marker, result["data"][0].Value());
+ }
+
+ [TestCase("\"get")]
+ [TestCase("\"delete\"")]
+ public void Console_MalformedOrUnknownAction_ReturnsError(string action)
+ {
+ var result = JObject.FromObject(ReadConsole.HandleCommand(new JObject { ["action"] = action }));
+ Assert.IsFalse(result.Value("success"));
+ StringAssert.Contains("Unknown action", result.Value("error"));
+ }
+
+ [Test]
+ public void Console_NestedJsonString_IsNotDecodedRecursively()
+ {
+ string encoded = JsonConvert.SerializeObject(JsonConvert.SerializeObject("get"));
+ var result = JObject.FromObject(ReadConsole.HandleCommand(new JObject { ["action"] = encoded }));
+ Assert.IsFalse(result.Value("success"));
+ }
+
+ [Test]
+ public void Console_QuotedFilter_IsPreserved()
+ {
+ string marker = "Literal filter probe " + Guid.NewGuid().ToString("N");
+ Debug.Log(marker);
+ Debug.Log("\"" + marker + "\"");
+ var result = JObject.FromObject(ReadConsole.HandleCommand(new JObject
+ {
+ ["action"] = "get", ["types"] = new JArray("log"), ["format"] = "plain",
+ ["filterText"] = "\"" + marker + "\"", ["count"] = 10
+ }));
+ Assert.IsTrue(result.Value("success"), result.ToString());
+ Assert.AreEqual(1, ((JArray)result["data"]).Count);
+ StringAssert.StartsWith("\"" + marker + "\"", result["data"][0].Value());
+ }
+
+ [TestCase(false)]
+ [TestCase(true)]
+ public void Menu_Path_InvokesOwnedMenuOnce(bool encoded)
+ {
+ string supplied = encoded ? JsonConvert.SerializeObject(MenuPath) : MenuPath;
+ var result = JObject.FromObject(ExecuteMenuItem.HandleCommand(new JObject { ["menuPath"] = supplied }));
+ Assert.IsTrue(result.Value("success"), result.ToString());
+ Assert.AreEqual(1, _menuCalls);
+ }
+
+ [Test]
+ public void Menu_JsonEncodedQuit_RemainsBlocked()
+ {
+ var result = JObject.FromObject(ExecuteMenuItem.HandleCommand(new JObject
+ {
+ ["menuPath"] = JsonConvert.SerializeObject("File/Quit")
+ }));
+ Assert.IsFalse(result.Value("success"));
+ StringAssert.Contains("blocked for safety", result.Value("error"));
+ Assert.AreEqual(0, _menuCalls);
+ }
+
+ [Test]
+ public void Menu_NestedJsonString_IsNotDecodedRecursively()
+ {
+ string encoded = JsonConvert.SerializeObject(JsonConvert.SerializeObject(MenuPath));
+ // Unity versions emit different error logs for an intentionally missing menu.
+ // Keep this allowance scoped to the call and assert its response and side effects.
+ bool originalIgnore = LogAssert.ignoreFailingMessages;
+ try
+ {
+ LogAssert.ignoreFailingMessages = true;
+ var result = JObject.FromObject(ExecuteMenuItem.HandleCommand(new JObject { ["menuPath"] = encoded }));
+ Assert.IsFalse(result.Value("success"));
+ Assert.AreEqual(0, _menuCalls);
+ }
+ finally
+ {
+ LogAssert.ignoreFailingMessages = originalIgnore;
+ }
+ }
+ }
+}
diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs.meta
new file mode 100644
index 0000000000..76c0a7b79b
--- /dev/null
+++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/QuotedToolArgumentsTests.cs.meta
@@ -0,0 +1,2 @@
+fileFormatVersion: 2
+guid: aedac1941dde48d5b857efb9ceddc489
diff --git a/website/docs/reference/tools/core/execute_menu_item.md b/website/docs/reference/tools/core/execute_menu_item.md
index dad09b7639..428a914ecd 100644
--- a/website/docs/reference/tools/core/execute_menu_item.md
+++ b/website/docs/reference/tools/core/execute_menu_item.md
@@ -18,7 +18,7 @@ Execute a Unity menu item by path.
| Name | Type | Required | Description |
|------|------|----------|-------------|
-| `menu_path` | `str \| None` | — | Menu path for 'execute' or 'exists' (e.g., 'File/Save Project') |
+| `menu_path` | `str \| None` | — | Menu path (e.g., 'File/Save Project'). Also accepts a JSON-encoded string. |
## Returns
diff --git a/website/docs/reference/tools/core/read_console.md b/website/docs/reference/tools/core/read_console.md
index a380721a97..12c9414574 100644
--- a/website/docs/reference/tools/core/read_console.md
+++ b/website/docs/reference/tools/core/read_console.md
@@ -18,13 +18,13 @@ Gets messages from or clears the Unity Editor console. Defaults to 10 most recen
| Name | Type | Required | Description |
|------|------|----------|-------------|
-| `action` | `Literal['get', 'clear'] \| None` | — | Get or clear the Unity Editor console. Defaults to 'get' if omitted. |
+| `action` | `Literal['get', 'clear'] \| None` | — | Get or clear the Unity Editor console. Defaults to 'get' if omitted. Also accepts a JSON-encoded string. |
| `types` | `list[Literal['error', 'warning', 'log', 'all']] \| str \| None` | — | Message types to get (accepts list or JSON string) |
| `count` | `int \| str \| 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. |
| `filter_text` | `str \| None` | — | Text filter for messages |
| `page_size` | `int \| str \| None` | — | Page size for paginated console reads. Defaults to 50 when omitted. |
| `cursor` | `int \| str \| None` | — | Opaque cursor for paging (0-based offset). Defaults to 0. |
-| `format` | `Literal['plain', 'detailed', 'json'] \| None` | — | Output format |
+| `format` | `Literal['plain', 'detailed', 'json'] \| None` | — | Output format. Also accepts a JSON-encoded string. |
| `include_stacktrace` | `bool \| str \| None` | — | Include stack traces in output (accepts true/false or 'true'/'false') |
## Returns