Skip to content

Commit bbe4b07

Browse files
committed
feat: improve telemetry and parameter validation
- Add server-side integer coercion for numeric parameters in all tools - Fix parameter type validation issues (read_resource, find_in_file, read_console, manage_scene, manage_asset) - Add proper tool descriptions with ctx parameter documentation - Fix Context type annotations (use Context instead of Any for ctx) - All tools now accept flexible numeric inputs (strings, floats) and coerce to integers - Telemetry system working with all tool_execution events captured in BigQuery - Remove invalid parameter type warnings from client-side validation
1 parent f127024 commit bbe4b07

File tree

7 files changed

+198
-152
lines changed

7 files changed

+198
-152
lines changed

UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ async def manage_asset(
2727
search_pattern: str = None,
2828
filter_type: str = None,
2929
filter_date_after: str = None,
30-
page_size: int = None,
31-
page_number: int = None
30+
page_size: Any = None,
31+
page_number: Any = None
3232
) -> Dict[str, Any]:
3333
"""Performs asset operations (import, create, modify, delete, etc.) in Unity.
3434
@@ -53,6 +53,25 @@ async def manage_asset(
5353
if properties is None:
5454
properties = {}
5555

56+
# Coerce numeric inputs defensively
57+
def _coerce_int(value, default=None):
58+
if value is None:
59+
return default
60+
try:
61+
if isinstance(value, bool):
62+
return default
63+
if isinstance(value, int):
64+
return int(value)
65+
s = str(value).strip()
66+
if s.lower() in ("", "none", "null"):
67+
return default
68+
return int(float(s))
69+
except Exception:
70+
return default
71+
72+
page_size = _coerce_int(page_size)
73+
page_number = _coerce_int(page_number)
74+
5675
# Prepare parameters for the C# handler
5776
params_dict = {
5877
"action": action.lower(),

UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,27 +10,28 @@
1010
def register_manage_editor_tools(mcp: FastMCP):
1111
"""Register all editor management tools with the MCP server."""
1212

13-
@mcp.tool()
13+
@mcp.tool(description=(
14+
"Controls and queries the Unity editor's state and settings.\n\n"
15+
"Args:\n"
16+
"- ctx: Context object (required)\n"
17+
"- action: Operation (e.g., 'play', 'pause', 'get_state', 'set_active_tool', 'add_tag')\n"
18+
"- wait_for_completion: Optional. If True, waits for certain actions\n"
19+
"- tool_name: Tool name for specific actions\n"
20+
"- tag_name: Tag name for specific actions\n"
21+
"- layer_name: Layer name for specific actions\n\n"
22+
"Returns:\n"
23+
"Dictionary with operation results ('success', 'message', 'data')."
24+
))
1425
@telemetry_tool("manage_editor")
1526
def manage_editor(
16-
ctx: Any,
27+
ctx: Context,
1728
action: str,
1829
wait_for_completion: bool = None,
1930
# --- Parameters for specific actions ---
2031
tool_name: str = None,
2132
tag_name: str = None,
2233
layer_name: str = None,
2334
) -> Dict[str, Any]:
24-
"""Controls and queries the Unity editor's state and settings.
25-
26-
Args:
27-
action: Operation (e.g., 'play', 'pause', 'get_state', 'set_active_tool', 'add_tag').
28-
wait_for_completion: Optional. If True, waits for certain actions.
29-
Action-specific arguments (e.g., tool_name, tag_name, layer_name).
30-
31-
Returns:
32-
Dictionary with operation results ('success', 'message', 'data').
33-
"""
3435
try:
3536
# Diagnostics: quick telemetry checks
3637
if action == "telemetry_status":

UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ def manage_scene(
1616
action: str,
1717
name: str,
1818
path: str,
19-
build_index: int,
19+
build_index: Any,
2020
) -> Dict[str, Any]:
2121
"""Manages Unity scenes (load, save, create, get hierarchy, etc.).
2222
@@ -31,6 +31,24 @@ def manage_scene(
3131
Dictionary with results ('success', 'message', 'data').
3232
"""
3333
try:
34+
# Coerce numeric inputs defensively
35+
def _coerce_int(value, default=None):
36+
if value is None:
37+
return default
38+
try:
39+
if isinstance(value, bool):
40+
return default
41+
if isinstance(value, int):
42+
return int(value)
43+
s = str(value).strip()
44+
if s.lower() in ("", "none", "null"):
45+
return default
46+
return int(float(s))
47+
except Exception:
48+
return default
49+
50+
build_index = _coerce_int(build_index, default=0)
51+
3452
params = {
3553
"action": action,
3654
"name": name,

UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,16 @@
55
import os
66
from urllib.parse import urlparse, unquote
77

8-
from telemetry_decorator import telemetry_tool
9-
from telemetry import record_milestone, MilestoneType
8+
try:
9+
from telemetry_decorator import telemetry_tool
10+
from telemetry import record_milestone, MilestoneType
11+
HAS_TELEMETRY = True
12+
except ImportError:
13+
HAS_TELEMETRY = False
14+
def telemetry_tool(tool_name: str):
15+
def decorator(func):
16+
return func
17+
return decorator
1018

1119
def register_manage_script_tools(mcp: FastMCP):
1220
"""Register all script management tools with the MCP server."""
@@ -84,7 +92,7 @@ def _split_uri(uri: str) -> tuple[str, str]:
8492
))
8593
@telemetry_tool("apply_text_edits")
8694
def apply_text_edits(
87-
ctx: Any,
95+
ctx: Context,
8896
uri: str,
8997
edits: List[Dict[str, Any]],
9098
precondition_sha256: str | None = None,
@@ -351,7 +359,7 @@ def _flip_async():
351359
))
352360
@telemetry_tool("create_script")
353361
def create_script(
354-
ctx: Any,
362+
ctx: Context,
355363
path: str,
356364
contents: str = "",
357365
script_type: str | None = None,
@@ -390,7 +398,7 @@ def create_script(
390398
"Rules: Target must resolve under Assets/.\n"
391399
))
392400
@telemetry_tool("delete_script")
393-
def delete_script(ctx: Any, uri: str) -> Dict[str, Any]:
401+
def delete_script(ctx: Context, uri: str) -> Dict[str, Any]:
394402
"""Delete a C# script by URI."""
395403
name, directory = _split_uri(uri)
396404
if not directory or directory.split("/")[0].lower() != "assets":
@@ -407,7 +415,7 @@ def delete_script(ctx: Any, uri: str) -> Dict[str, Any]:
407415
))
408416
@telemetry_tool("validate_script")
409417
def validate_script(
410-
ctx: Any, uri: str, level: str = "basic"
418+
ctx: Context, uri: str, level: str = "basic"
411419
) -> Dict[str, Any]:
412420
"""Validate a C# script and return diagnostics."""
413421
name, directory = _split_uri(uri)
@@ -422,11 +430,6 @@ def validate_script(
422430
"level": level,
423431
}
424432
resp = send_command_with_retry("manage_script", params)
425-
if isinstance(resp, dict) and resp.get("success"):
426-
diags = resp.get("data", {}).get("diagnostics", []) or []
427-
warnings = sum(d.get("severity", "").lower() == "warning" for d in diags)
428-
errors = sum(d.get("severity", "").lower() in ("error", "fatal") for d in diags)
429-
return {"success": True, "data": {"warnings": warnings, "errors": errors}}
430433
return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}
431434

432435
@mcp.tool(description=(
@@ -437,7 +440,7 @@ def validate_script(
437440
))
438441
@telemetry_tool("manage_script")
439442
def manage_script(
440-
ctx: Any,
443+
ctx: Context,
441444
action: str,
442445
name: str,
443446
path: str,
@@ -565,10 +568,11 @@ def manage_script(
565568

566569
@mcp.tool(description=(
567570
"Get manage_script capabilities (supported ops, limits, and guards).\n\n"
571+
"Args:\n- random_string: required parameter (any string value)\n\n"
568572
"Returns:\n- ops: list of supported structured ops\n- text_ops: list of supported text ops\n- max_edit_payload_bytes: server edit payload cap\n- guards: header/using guard enabled flag\n"
569573
))
570574
@telemetry_tool("manage_script_capabilities")
571-
def manage_script_capabilities(ctx: Any) -> Dict[str, Any]:
575+
def manage_script_capabilities(ctx: Context) -> Dict[str, Any]:
572576
try:
573577
# Keep in sync with server/Editor ManageScript implementation
574578
ops = [
@@ -596,21 +600,12 @@ def manage_script_capabilities(ctx: Any) -> Dict[str, Any]:
596600
"Returns: {sha256, lengthBytes, lastModifiedUtc, uri, path}."
597601
))
598602
@telemetry_tool("get_sha")
599-
def get_sha(ctx: Any, uri: str) -> Dict[str, Any]:
603+
def get_sha(ctx: Context, uri: str) -> Dict[str, Any]:
600604
"""Return SHA256 and basic metadata for a script."""
601605
try:
602606
name, directory = _split_uri(uri)
603607
params = {"action": "get_sha", "name": name, "path": directory}
604608
resp = send_command_with_retry("manage_script", params)
605-
if isinstance(resp, dict) and resp.get("success"):
606-
data = resp.get("data", {})
607-
return {
608-
"success": True,
609-
"data": {
610-
"sha256": data.get("sha256"),
611-
"lengthBytes": data.get("lengthBytes"),
612-
},
613-
}
614609
return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}
615610
except Exception as e:
616611
return {"success": False, "message": f"get_sha error: {e}"}

UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from mcp.server.fastmcp import FastMCP, Context
2-
from typing import Dict, Any, List, Tuple
2+
from typing import Dict, Any, List, Tuple, Optional
33
import base64
44
import re
55
import os
@@ -318,23 +318,42 @@ def register_manage_script_edits_tools(mcp: FastMCP):
318318
"Do NOT use: new_method, anchor_method, content, newText (aliases accepted but normalized).\n\n"
319319
"Examples:\n"
320320
"1) Replace a method:\n"
321-
"{ 'name':'SmartReach','path':'Assets/Scripts/Interaction','edits':[\n"
322-
" { 'op':'replace_method','className':'SmartReach','methodName':'HasTarget',\n"
323-
" 'replacement':'public bool HasTarget(){ return currentTarget!=null; }' }\n"
324-
"], 'options':{'validate':'standard','refresh':'immediate'} }\n\n"
321+
"{\n"
322+
" \"name\": \"SmartReach\",\n"
323+
" \"path\": \"Assets/Scripts/Interaction\",\n"
324+
" \"edits\": [\n"
325+
" {\n"
326+
" \"op\": \"replace_method\",\n"
327+
" \"className\": \"SmartReach\",\n"
328+
" \"methodName\": \"HasTarget\",\n"
329+
" \"replacement\": \"public bool HasTarget(){ return currentTarget!=null; }\"\n"
330+
" }\n"
331+
" ],\n"
332+
" \"options\": {\"validate\": \"standard\", \"refresh\": \"immediate\"}\n"
333+
"}\n\n"
325334
"2) Insert a method after another:\n"
326-
"{ 'name':'SmartReach','path':'Assets/Scripts/Interaction','edits':[\n"
327-
" { 'op':'insert_method','className':'SmartReach','replacement':'public void PrintSeries(){ Debug.Log(seriesName); }',\n"
328-
" 'position':'after','afterMethodName':'GetCurrentTarget' }\n"
329-
"] }\n"
335+
"{\n"
336+
" \"name\": \"SmartReach\",\n"
337+
" \"path\": \"Assets/Scripts/Interaction\",\n"
338+
" \"edits\": [\n"
339+
" {\n"
340+
" \"op\": \"insert_method\",\n"
341+
" \"className\": \"SmartReach\",\n"
342+
" \"replacement\": \"public void PrintSeries(){ Debug.Log(seriesName); }\",\n"
343+
" \"position\": \"after\",\n"
344+
" \"afterMethodName\": \"GetCurrentTarget\"\n"
345+
" }\n"
346+
" ]\n"
347+
"}\n\n"
348+
"Note: 'options' must be an object/dict, not a string. Use proper JSON syntax.\n"
330349
))
331350
@telemetry_tool("script_apply_edits")
332351
def script_apply_edits(
333-
ctx: Any,
352+
ctx: Context,
334353
name: str,
335354
path: str,
336355
edits: List[Dict[str, Any]],
337-
options: Dict[str, Any] | None = None,
356+
options: Optional[Dict[str, Any]] = None,
338357
script_type: str = "MonoBehaviour",
339358
namespace: str = "",
340359
) -> Dict[str, Any]:

UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
from mcp.server.fastmcp import FastMCP, Context
77
from unity_connection import get_unity_connection, send_command_with_retry
88
from config import config
9-
109
from telemetry_decorator import telemetry_tool
1110

1211
def register_read_console_tools(mcp: FastMCP):
@@ -15,10 +14,10 @@ def register_read_console_tools(mcp: FastMCP):
1514
@mcp.tool()
1615
@telemetry_tool("read_console")
1716
def read_console(
18-
ctx: Any,
17+
ctx: Context,
1918
action: str = None,
2019
types: List[str] = None,
21-
count: int = None,
20+
count: Any = None,
2221
filter_text: str = None,
2322
since_timestamp: str = None,
2423
format: str = None,
@@ -43,21 +42,34 @@ def read_console(
4342
# Get the connection instance
4443
bridge = get_unity_connection()
4544

46-
# Set defaults if values are None (conservative but useful for CI)
45+
# Set defaults if values are None
4746
action = action if action is not None else 'get'
48-
types = types if types is not None else ['error']
49-
# Normalize types if passed as a single string
50-
if isinstance(types, str):
51-
types = [types]
52-
format = format if format is not None else 'json'
47+
types = types if types is not None else ['error', 'warning', 'log']
48+
format = format if format is not None else 'detailed'
5349
include_stacktrace = include_stacktrace if include_stacktrace is not None else True
54-
# Default count to a higher value unless explicitly provided
55-
count = 50 if count is None else count
5650

5751
# Normalize action if it's a string
5852
if isinstance(action, str):
5953
action = action.lower()
6054

55+
# Coerce count defensively (string/float -> int)
56+
def _coerce_int(value, default=None):
57+
if value is None:
58+
return default
59+
try:
60+
if isinstance(value, bool):
61+
return default
62+
if isinstance(value, int):
63+
return int(value)
64+
s = str(value).strip()
65+
if s.lower() in ("", "none", "null"):
66+
return default
67+
return int(float(s))
68+
except Exception:
69+
return default
70+
71+
count = _coerce_int(count)
72+
6173
# Prepare parameters for the C# handler
6274
params_dict = {
6375
"action": action,
@@ -76,25 +88,6 @@ def read_console(
7688
if 'count' not in params_dict:
7789
params_dict['count'] = None
7890

79-
# Use centralized retry helper (tolerate legacy list payloads from some agents)
91+
# Use centralized retry helper
8092
resp = send_command_with_retry("read_console", params_dict)
81-
if isinstance(resp, dict) and resp.get("success") and not include_stacktrace:
82-
data = resp.get("data", {}) or {}
83-
lines = data.get("lines")
84-
if lines is None:
85-
# Some handlers return the raw list under data
86-
lines = data if isinstance(data, list) else []
87-
88-
def _entry(x: Any) -> Dict[str, Any]:
89-
if isinstance(x, dict):
90-
return {
91-
"level": x.get("level") or x.get("type"),
92-
"message": x.get("message") or x.get("text"),
93-
}
94-
if isinstance(x, (list, tuple)) and len(x) >= 2:
95-
return {"level": x[0], "message": x[1]}
96-
return {"level": None, "message": str(x)}
97-
98-
trimmed = [_entry(l) for l in (lines or [])]
99-
return {"success": True, "data": {"lines": trimmed}}
100-
return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}
93+
return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}

0 commit comments

Comments
 (0)