mika kuns
2a3133efad
TaskIdResolver resolves a #123/bare-123 taskId parameter to its GUID before any lookup, across every External/ MCP tool that takes a task id, including the batch tools' id arrays (via delegation to the already-resolving single-entity methods) and update_task's dependsOnTaskId (empty string still passes through unchanged as the clear-link sentinel). An unknown number throws a clear error instead of a silent null. McpToolDocs.TaskNumberHint tells the agent to refer to tasks as #<number> when reporting to the user, added to the description of get_task, list_tasks, add_task, update_task_status and review_task.
88 lines
3.8 KiB
C#
88 lines
3.8 KiB
C#
using System.ComponentModel;
|
|
using System.Reflection;
|
|
using ClaudeDo.Worker.External;
|
|
using Microsoft.Extensions.AI;
|
|
using ModelContextProtocol.Server;
|
|
|
|
namespace ClaudeDo.Worker.Tests.External;
|
|
|
|
/// <summary>
|
|
/// MCP clients routinely omit optional arguments. The generated tool schema only marks a
|
|
/// parameter optional when the C# method declares a default value — nullability alone is not
|
|
/// enough (verified against ModelContextProtocol/Microsoft.Extensions.AI 1.2.0 / 10.4.1's
|
|
/// AIJsonUtilities.CreateFunctionJsonSchema, which checks ParameterInfo.IsOptional). This sweeps
|
|
/// every [McpServerToolType] class in the External/ namespace so a future tool can't reintroduce
|
|
/// a nullable-but-required filter parameter.
|
|
/// </summary>
|
|
public sealed class ExternalMcpToolSchemaTests
|
|
{
|
|
private static IEnumerable<MethodInfo> ExternalToolMethods()
|
|
{
|
|
var toolTypes = typeof(ExternalMcpService).Assembly.GetTypes()
|
|
.Where(t => t.Namespace == typeof(ExternalMcpService).Namespace
|
|
&& t.GetCustomAttribute<McpServerToolTypeAttribute>() is not null);
|
|
|
|
foreach (var type in toolTypes)
|
|
foreach (var method in type.GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly))
|
|
{
|
|
if (method.GetCustomAttribute<McpServerToolAttribute>() is not null)
|
|
yield return method;
|
|
}
|
|
}
|
|
|
|
[Fact]
|
|
public void NoExternalTool_HasARequiredNullableParameter()
|
|
{
|
|
var nullabilityContext = new NullabilityInfoContext();
|
|
var violations = new List<string>();
|
|
|
|
foreach (var method in ExternalToolMethods())
|
|
{
|
|
var schema = AIJsonUtilities.CreateFunctionJsonSchema(method);
|
|
var required = schema.TryGetProperty("required", out var requiredElement)
|
|
? requiredElement.EnumerateArray().Select(e => e.GetString()).ToHashSet()
|
|
: new HashSet<string?>();
|
|
|
|
foreach (var parameter in method.GetParameters())
|
|
{
|
|
if (parameter.ParameterType == typeof(CancellationToken)) continue;
|
|
if (!required.Contains(parameter.Name)) continue;
|
|
|
|
var isNullableValueType = Nullable.GetUnderlyingType(parameter.ParameterType) is not null;
|
|
var isNullableRefType = !parameter.ParameterType.IsValueType
|
|
&& nullabilityContext.Create(parameter).WriteState == NullabilityState.Nullable;
|
|
|
|
if (isNullableValueType || isNullableRefType)
|
|
{
|
|
violations.Add(
|
|
$"{method.DeclaringType!.Name}.{method.Name}({parameter.Name}) is a nullable " +
|
|
"type but has no default value, so MCP clients omitting it will fail. " +
|
|
"Give it a default value (e.g. '= null').");
|
|
}
|
|
}
|
|
}
|
|
|
|
Assert.True(violations.Count == 0, string.Join(Environment.NewLine, violations));
|
|
}
|
|
|
|
[Fact]
|
|
public void ExternalToolMethods_AreDiscovered()
|
|
{
|
|
// Guards the sweep itself: if this drops to 0, ExternalToolMethods() broke silently
|
|
// (e.g. namespace/attribute mismatch) and the schema test above would pass vacuously.
|
|
Assert.True(ExternalToolMethods().Count() > 20);
|
|
}
|
|
|
|
[Fact]
|
|
public void AtLeastOneTaskIdTool_DescriptionCarriesTaskNumberHint()
|
|
{
|
|
// The whole point of task numbers: without this clause the agent never learns to speak
|
|
// #<number> to the user, even though every DTO already carries it.
|
|
var hasHint = ExternalToolMethods()
|
|
.Select(m => m.GetCustomAttribute<DescriptionAttribute>()?.Description ?? "")
|
|
.Any(d => d.Contains(McpToolDocs.TaskNumberHint.Trim(), StringComparison.Ordinal));
|
|
|
|
Assert.True(hasHint, "No external tool description carries McpToolDocs.TaskNumberHint.");
|
|
}
|
|
}
|