Skip to content

Commit 50c6ed4

Browse files
bruno1308claude
bruno1308
and
claude
authored
fix(run_tests): support snake_case params and handle double-serialized arrays (#690)
* fix(run_tests): support snake_case params and handle double-serialized arrays The MCP tool schema defines filter parameters in snake_case (test_names, group_names, etc.) but GetFilterOptions() only checked camelCase keys, causing test filtering to silently fail — all tests would run instead of the filtered subset. Additionally, some MCP clients serialize array parameters as a stringified JSON array inside an outer array (e.g. ["[\"name1\"]"]), which was not handled by ParseStringArray(). Changes: - Add snake_case fallback to all ParseStringArray calls in RunTests.cs - Handle stringified JSON arrays in String token branch - Handle double-serialized arrays in Array token branch - Handle nested arrays ([[name1, name2]]) in Array token branch - Add snake_case fallback for include_details/include_failed_tests in both RunTests.cs and GetTestJob.cs Fixes #689 Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: extract GetParam and CoerceStringArray into ParamCoercion Address code review feedback: - Extract snake_case/camelCase param lookup into ParamCoercion.GetParam() - Extract string array coercion (with double-serialization handling) into ParamCoercion.CoerceStringArray() - Narrow catch clauses from bare catch to JsonException - Add explicit comment clarifying nested array unwrapping limitation Co-Authored-By: Claude Opus 4.6 <[email protected]> * refactor: move CoerceStringArray to ToolParams and use GetRaw for snake_case fallback Remove GetParam from ParamCoercion — ToolParams.GetRaw() already handles snake_case/camelCase fallback via StringCaseUtility. Move CoerceStringArray into ToolParams as GetStringArray() to centralize parameter handling. Update RunTests and GetTestJob to use ToolParams instead. Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
1 parent 8c6cefd commit 50c6ed4

File tree

3 files changed

+87
-31
lines changed

3 files changed

+87
-31
lines changed

‎MCPForUnity/Editor/Helpers/ToolParams.cs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using System.Linq;
2+
using Newtonsoft.Json;
13
using Newtonsoft.Json.Linq;
24
using System;
35

@@ -87,6 +89,80 @@ public JToken GetRaw(string key)
8789
return GetToken(key);
8890
}
8991

92+
/// <summary>
93+
/// Get optional string array parameter, handling various MCP serialization formats:
94+
/// plain strings, JSON arrays, stringified JSON arrays, and double-serialized arrays.
95+
/// Supports both snake_case and camelCase automatically.
96+
/// </summary>
97+
public string[] GetStringArray(string key)
98+
{
99+
return CoerceStringArray(GetToken(key));
100+
}
101+
102+
/// <summary>
103+
/// Coerces a JToken to a string array, handling various MCP serialization formats:
104+
/// plain strings, JSON arrays, stringified JSON arrays, and double-serialized arrays.
105+
/// </summary>
106+
internal static string[] CoerceStringArray(JToken token)
107+
{
108+
if (token == null || token.Type == JTokenType.Null) return null;
109+
110+
if (token.Type == JTokenType.String)
111+
{
112+
var value = token.ToString();
113+
if (string.IsNullOrWhiteSpace(value)) return null;
114+
// Handle stringified JSON arrays (e.g. "[\"name1\", \"name2\"]")
115+
var trimmed = value.Trim();
116+
if (trimmed.StartsWith("[") && trimmed.EndsWith("]"))
117+
{
118+
try
119+
{
120+
var parsed = JArray.Parse(trimmed);
121+
var values = parsed.Values<string>()
122+
.Where(s => !string.IsNullOrWhiteSpace(s))
123+
.ToArray();
124+
return values.Length > 0 ? values : null;
125+
}
126+
catch (JsonException) { /* not a valid JSON array, treat as plain string */ }
127+
}
128+
return new[] { value };
129+
}
130+
131+
if (token.Type == JTokenType.Array)
132+
{
133+
var array = token as JArray;
134+
if (array == null || array.Count == 0) return null;
135+
// Handle double-serialized arrays: MCP bridge may send ["[\"name1\"]"]
136+
// where the inner string is a stringified JSON array
137+
if (array.Count == 1 && array[0].Type == JTokenType.String)
138+
{
139+
var inner = array[0].ToString().Trim();
140+
if (inner.StartsWith("[") && inner.EndsWith("]"))
141+
{
142+
try
143+
{
144+
array = JArray.Parse(inner);
145+
}
146+
catch (JsonException) { /* use original array */ }
147+
}
148+
}
149+
// Handle single-level nested arrays: [[name1, name2]]
150+
// Multi-element outer arrays (e.g. [["a"], ["b"]]) are not unwrapped
151+
// as that format is not produced by known MCP clients.
152+
else if (array.Count == 1 && array[0].Type == JTokenType.Array)
153+
{
154+
array = array[0] as JArray ?? array;
155+
}
156+
var values = array
157+
.Values<string>()
158+
.Where(s => !string.IsNullOrWhiteSpace(s))
159+
.ToArray();
160+
return values.Length > 0 ? values : null;
161+
}
162+
163+
return null;
164+
}
165+
90166
/// <summary>
91167
/// Get raw JToken with snake_case/camelCase fallback.
92168
/// </summary>

‎MCPForUnity/Editor/Tools/GetTestJob.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ public static object HandleCommand(JObject @params)
1919
return new ErrorResponse("Missing required parameter 'job_id'.");
2020
}
2121

22-
bool includeDetails = ParamCoercion.CoerceBool(@params?["includeDetails"], false);
23-
bool includeFailedTests = ParamCoercion.CoerceBool(@params?["includeFailedTests"], false);
22+
var p = new ToolParams(@params);
23+
bool includeDetails = p.GetBool("includeDetails");
24+
bool includeFailedTests = p.GetBool("includeFailedTests");
2425

2526
var job = TestJobManager.GetJob(jobId);
2627
if (job == null)

‎MCPForUnity/Editor/Tools/RunTests.cs‎

Lines changed: 8 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using System.Linq;
32
using System.Threading.Tasks;
43
using MCPForUnity.Editor.Helpers;
54
using MCPForUnity.Editor.Resources.Tests;
@@ -41,8 +40,9 @@ public static Task<object> HandleCommand(JObject @params)
4140
return Task.FromResult<object>(new ErrorResponse(parseError));
4241
}
4342

44-
bool includeDetails = ParamCoercion.CoerceBool(@params?["includeDetails"], false);
45-
bool includeFailedTests = ParamCoercion.CoerceBool(@params?["includeFailedTests"], false);
43+
var p = new ToolParams(@params);
44+
bool includeDetails = p.GetBool("includeDetails");
45+
bool includeFailedTests = p.GetBool("includeFailedTests");
4646

4747
var filterOptions = GetFilterOptions(@params);
4848
string jobId = TestJobManager.StartJob(parsedMode.Value, filterOptions);
@@ -74,32 +74,11 @@ private static TestFilterOptions GetFilterOptions(JObject @params)
7474
return null;
7575
}
7676

77-
string[] ParseStringArray(string key)
78-
{
79-
var token = @params[key];
80-
if (token == null) return null;
81-
if (token.Type == JTokenType.String)
82-
{
83-
var value = token.ToString();
84-
return string.IsNullOrWhiteSpace(value) ? null : new[] { value };
85-
}
86-
if (token.Type == JTokenType.Array)
87-
{
88-
var array = token as JArray;
89-
if (array == null || array.Count == 0) return null;
90-
var values = array
91-
.Values<string>()
92-
.Where(s => !string.IsNullOrWhiteSpace(s))
93-
.ToArray();
94-
return values.Length > 0 ? values : null;
95-
}
96-
return null;
97-
}
98-
99-
var testNames = ParseStringArray("testNames");
100-
var groupNames = ParseStringArray("groupNames");
101-
var categoryNames = ParseStringArray("categoryNames");
102-
var assemblyNames = ParseStringArray("assemblyNames");
77+
var p = new ToolParams(@params);
78+
var testNames = p.GetStringArray("testNames");
79+
var groupNames = p.GetStringArray("groupNames");
80+
var categoryNames = p.GetStringArray("categoryNames");
81+
var assemblyNames = p.GetStringArray("assemblyNames");
10382

10483
if (testNames == null && groupNames == null && categoryNames == null && assemblyNames == null)
10584
{

0 commit comments

Comments
 (0)