Implement SEP-986: Specify Format for Tool Names (#945)
We can add static analysis to provide compile-time diagnostics later, once the analyzers project is merged. For now, this adds run-time validation.
This commit is contained in:
parent
121929da36
commit
5fc50fb528
2 changed files with 163 additions and 2 deletions
|
|
@ -6,7 +6,6 @@ using System.Diagnostics;
|
|||
using System.Reflection;
|
||||
using System.Text.Json;
|
||||
using System.Text.Json.Nodes;
|
||||
using System.Text.Json.Serialization.Metadata;
|
||||
using System.Text.RegularExpressions;
|
||||
|
||||
namespace ModelContextProtocol.Server;
|
||||
|
|
@ -208,6 +207,8 @@ internal sealed partial class AIFunctionMcpServerTool : McpServerTool
|
|||
/// <summary>Initializes a new instance of the <see cref="McpServerTool"/> class.</summary>
|
||||
private AIFunctionMcpServerTool(AIFunction function, Tool tool, IServiceProvider? serviceProvider, bool structuredOutputRequiresWrapping, IReadOnlyList<object> metadata)
|
||||
{
|
||||
ValidateToolName(tool.Name);
|
||||
|
||||
AIFunction = function;
|
||||
ProtocolTool = tool;
|
||||
ProtocolTool.McpServerTool = this;
|
||||
|
|
@ -376,15 +377,35 @@ internal sealed partial class AIFunctionMcpServerTool : McpServerTool
|
|||
return meta;
|
||||
}
|
||||
|
||||
/// <summary>Regex that flags runs of characters other than ASCII digits or letters.</summary>
|
||||
#if NET
|
||||
/// <summary>Regex that flags runs of characters other than ASCII digits or letters.</summary>
|
||||
[GeneratedRegex("[^0-9A-Za-z]+")]
|
||||
private static partial Regex NonAsciiLetterDigitsRegex();
|
||||
|
||||
/// <summary>Regex that validates tool names.</summary>
|
||||
[GeneratedRegex(@"^[A-Za-z0-9_.-]{1,128}\z")]
|
||||
private static partial Regex ValidateToolNameRegex();
|
||||
#else
|
||||
private static Regex NonAsciiLetterDigitsRegex() => _nonAsciiLetterDigits;
|
||||
private static readonly Regex _nonAsciiLetterDigits = new("[^0-9A-Za-z]+", RegexOptions.Compiled);
|
||||
|
||||
private static Regex ValidateToolNameRegex() => _validateToolName;
|
||||
private static readonly Regex _validateToolName = new(@"^[A-Za-z0-9_.-]{1,128}\z", RegexOptions.Compiled);
|
||||
#endif
|
||||
|
||||
private static void ValidateToolName(string name)
|
||||
{
|
||||
if (name is null)
|
||||
{
|
||||
throw new ArgumentException("Tool name cannot be null.");
|
||||
}
|
||||
|
||||
if (!ValidateToolNameRegex().IsMatch(name))
|
||||
{
|
||||
throw new ArgumentException($"The tool name '{name}' is invalid. Tool names must match the regular expression '{ValidateToolNameRegex()}'");
|
||||
}
|
||||
}
|
||||
|
||||
private static JsonElement? CreateOutputSchema(AIFunction function, McpServerToolCreateOptions? toolCreateOptions, out bool structuredOutputRequiresWrapping)
|
||||
{
|
||||
structuredOutputRequiresWrapping = false;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,140 @@
|
|||
using Microsoft.Extensions.AI;
|
||||
using ModelContextProtocol.Server;
|
||||
|
||||
namespace ModelContextProtocol.Tests.Server;
|
||||
|
||||
public class McpServerToolNameValidationTests
|
||||
{
|
||||
[Fact]
|
||||
public void WithValidCharacters_Succeeds()
|
||||
{
|
||||
const string AllValidChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.-";
|
||||
|
||||
Validate(AllValidChars);
|
||||
|
||||
foreach (char c in AllValidChars)
|
||||
{
|
||||
Validate($"{c}");
|
||||
Validate($"tool{c}name");
|
||||
Validate($"{c}toolname");
|
||||
Validate($"toolname{c}");
|
||||
}
|
||||
|
||||
static void Validate(string toolName)
|
||||
{
|
||||
var tool = McpServerTool.Create(() => "result", new McpServerToolCreateOptions { Name = toolName });
|
||||
Assert.Equal(toolName, tool.ProtocolTool.Name);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void WithInvalidCharacters_Throws()
|
||||
{
|
||||
Validate("café");
|
||||
Validate("tööl");
|
||||
Validate("🔧tool");
|
||||
|
||||
for (int i = 0; i < 256; i++)
|
||||
{
|
||||
char c = (char)i;
|
||||
if (c is not (>= 'a' and <= 'z')
|
||||
and not (>= 'A' and <= 'Z')
|
||||
and not (>= '0' and <= '9')
|
||||
and not ('_' or '-' or '.'))
|
||||
{
|
||||
Validate($"{c}");
|
||||
Validate($"tool{c}name");
|
||||
Validate($"{c}toolname");
|
||||
Validate($"toolname{c}");
|
||||
}
|
||||
}
|
||||
|
||||
static void Validate(string toolName)
|
||||
{
|
||||
var ex = Assert.Throws<ArgumentException>(() => McpServerTool.Create(() => "result", new McpServerToolCreateOptions { Name = toolName }));
|
||||
Assert.Contains(toolName, ex.Message, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(1)]
|
||||
[InlineData(10)]
|
||||
[InlineData(127)]
|
||||
[InlineData(128)]
|
||||
public void WithValidLengths_Succeeds(int length)
|
||||
{
|
||||
string validName = new('a', length);
|
||||
var tool = McpServerTool.Create(() => "result", new McpServerToolCreateOptions { Name = validName });
|
||||
Assert.Equal(validName, tool.ProtocolTool.Name);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(0)]
|
||||
[InlineData(129)]
|
||||
[InlineData(130)]
|
||||
public void WithInvalidLengths_ThrowsArgumentException(int length)
|
||||
{
|
||||
string invalidName = new('a', length);
|
||||
var ex = Assert.Throws<ArgumentException>(() => McpServerTool.Create(() => "result", new McpServerToolCreateOptions { Name = invalidName }));
|
||||
Assert.Contains(invalidName, ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UsingAttribute_ValidatesToolName()
|
||||
{
|
||||
var validTool = McpServerTool.Create([McpServerTool(Name = "valid_tool")] () => "result");
|
||||
Assert.Equal("valid_tool", validTool.ProtocolTool.Name);
|
||||
|
||||
var ex = Assert.Throws<ArgumentException>(() => McpServerTool.Create([McpServerTool(Name = "invalid@tool")] () => "result"));
|
||||
Assert.Contains("invalid@tool", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UsingAttributeWithInvalidCharacters_ThrowsArgumentException()
|
||||
{
|
||||
var validTool = McpServerTool.Create([McpServerTool(Name = "tool")] () => "result");
|
||||
Assert.Equal("tool", validTool.ProtocolTool.Name);
|
||||
|
||||
var ex = Assert.Throws<ArgumentException>(() => McpServerTool.Create([McpServerTool(Name = "tööl")] () => "result"));
|
||||
Assert.Contains("tööl", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void FromMethodInfo_ValidatesToolName()
|
||||
{
|
||||
Assert.Equal(nameof(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa), McpServerTool.Create(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa).ProtocolTool.Name);
|
||||
|
||||
var ex = Assert.Throws<ArgumentException>(() => McpServerTool.Create(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa));
|
||||
Assert.Contains(nameof(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa), ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void FromAIFunction_ValidatesToolName()
|
||||
{
|
||||
var validTool = McpServerTool.Create(AIFunctionFactory.Create(() => "result", new AIFunctionFactoryOptions { Name = "valid_ai" }));
|
||||
Assert.Equal("valid_ai", validTool.ProtocolTool.Name);
|
||||
|
||||
var invalidFunction = AIFunctionFactory.Create(() => "result", new AIFunctionFactoryOptions { Name = "invalid ai" });
|
||||
var ex = Assert.Throws<ArgumentException>(() => McpServerTool.Create(invalidFunction));
|
||||
Assert.Contains("invalid ai", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void FromNullAIFunctionName_ThrowsArgumentNullException()
|
||||
{
|
||||
AIFunction f = new NullNameAIFunction();
|
||||
Assert.Throws<ArgumentException>(() => McpServerTool.Create(f));
|
||||
}
|
||||
|
||||
private static void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa() { }
|
||||
|
||||
private static void aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa() { }
|
||||
|
||||
private sealed class NullNameAIFunction : AIFunction
|
||||
{
|
||||
public override string Name => null!;
|
||||
|
||||
protected override ValueTask<object?> InvokeCoreAsync(AIFunctionArguments arguments, CancellationToken cancellationToken) =>
|
||||
throw new NotSupportedException();
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue