Address feedback

Author: stephentoub

src/ModelContextProtocol/Protocol/Messages/ProgressToken.cs

@@ -12,44 +12,45 @@ namespace ModelContextProtocol.Protocol.Messages;
 [JsonConverter(typeof(Converter))]
 public readonly struct ProgressToken : IEquatable<ProgressToken>
 {
-    /// <summary>The id, either a string or a boxed long or null.</summary>
-    private readonly object? _id;
+    /// <summary>The token, either a string or a boxed long or null.</summary>
+    private readonly object? _token;
 
     /// <summary>Initializes a new instance of the <see cref="ProgressToken"/> with a specified value.</summary>
     /// <param name="value">The required ID value.</param>
     public ProgressToken(string value)
     {
         Throw.IfNull(value);
-        _id = value;
+        _token = value;
     }
 
     /// <summary>Initializes a new instance of the <see cref="ProgressToken"/> with a specified value.</summary>
     /// <param name="value">The required ID value.</param>
     public ProgressToken(long value)
     {
         // Box the long. Progress tokens are almost always strings in practice, so this should be rare.
-        _id = value;
+        _token = value;
     }
 
-    /// <summary>Gets whether the identifier is uninitialized.</summary>
-    public bool IsDefault => _id is null;
+    /// <summary>Gets the underlying object for this token.</summary>
+    /// <remarks>This will either be a <see cref="string"/>, a boxed <see cref="long"/>, or <see langword="null"/>.</remarks>
+    public object? Token => _token;
 
     /// <inheritdoc />
     public override string? ToString() =>
-        _id is string stringValue ? $"\"{stringValue}\"" :
-        _id is long longValue ? longValue.ToString(CultureInfo.InvariantCulture) :
+        _token is string stringValue ? $"{stringValue}" :
+        _token is long longValue ? longValue.ToString(CultureInfo.InvariantCulture) :
         null;
 
     /// <summary>
     /// Compares this ProgressToken to another ProgressToken.
     /// </summary>
-    public bool Equals(ProgressToken other) => Equals(_id, other._id);
+    public bool Equals(ProgressToken other) => Equals(_token, other._token);
 
     /// <inheritdoc />
     public override bool Equals(object? obj) => obj is ProgressToken other && Equals(other);
 
     /// <inheritdoc />
-    public override int GetHashCode() => _id?.GetHashCode() ?? 0;
+    public override int GetHashCode() => _token?.GetHashCode() ?? 0;
 
     /// <summary>
     /// Compares two ProgressTokens for equality.
@@ -83,7 +84,7 @@ public override void Write(Utf8JsonWriter writer, ProgressToken value, JsonSeria
         {
             Throw.IfNull(writer);
 
-            switch (value._id)
+            switch (value._token)
             {
                 case string str:
                     writer.WriteStringValue(str);

src/ModelContextProtocol/Protocol/Messages/RequestId.cs

@@ -31,9 +31,6 @@ public RequestId(long value)
         _id = value;
     }
 
-    /// <summary>Gets whether the identifier is uninitialized.</summary>
-    public bool IsDefault => _id is null;
-
     /// <summary>Gets the underlying object for this id.</summary>
     /// <remarks>This will either be a <see cref="string"/>, a boxed <see cref="long"/>, or <see langword="null"/>.</remarks>
     public object? Id => _id;

src/ModelContextProtocol/Shared/McpSession.cs

@@ -271,7 +271,7 @@ private async Task HandleNotification(JsonRpcNotification notification)
 
     private void HandleMessageWithId(IJsonRpcMessage message, IJsonRpcMessageWithId messageWithId)
     {
-        if (messageWithId.Id.IsDefault)
+        if (messageWithId.Id.Id is null)
         {
             _logger.RequestHasInvalidId(EndpointName);
         }
@@ -331,7 +331,7 @@ public async Task<TResult> SendRequestAsync<TResult>(JsonRpcRequest request, Can
             null;
 
         // Set request ID
-        if (request.Id.IsDefault)
+        if (request.Id.Id is null)
         {
             request.Id = new RequestId($"{_id}-{Interlocked.Increment(ref _nextRequestId)}");
         }
@@ -491,7 +491,7 @@ public async Task SendMessageAsync(IJsonRpcMessage message, CancellationToken ca
     private string CreateActivityName(string method) =>
         $"mcp.{(_isServer ? "server" : "client")}.{_transportKind}/{method}";
 
-    private string GetMethodName(IJsonRpcMessage message) =>
+    private static string GetMethodName(IJsonRpcMessage message) =>
         message switch
         {
             JsonRpcRequest request => request.Method,
@@ -511,7 +511,7 @@ private void AddStandardTags(ref TagList tags, string method)
         // server.address, server.port, client.address, client.port, network.peer.address, network.peer.port, network.type
     }
 
-    private void AddRpcRequestTags(ref TagList tags, Activity? activity, JsonRpcRequest request)
+    private static void AddRpcRequestTags(ref TagList tags, Activity? activity, JsonRpcRequest request)
     {
         tags.Add("rpc.jsonrpc.request_id", request.Id.ToString());
 
@@ -547,7 +547,7 @@ private void AddRpcRequestTags(ref TagList tags, Activity? activity, JsonRpcRequ
         }
     }
 
-    private void AddExceptionTags(ref TagList tags, Exception e)
+    private static void AddExceptionTags(ref TagList tags, Exception e)
     {
         tags.Add("error.type", e.GetType().FullName);
         tags.Add("rpc.jsonrpc.error_code",

tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs

@@ -640,7 +640,7 @@ public async Task HandlesIProgressParameter()
         Assert.Equal(10, array.Length);
         for (int i = 0; i < array.Length; i++)
         {
-            Assert.Equal("\"abc123\"", array[i].ProgressToken.ToString());
+            Assert.Equal("abc123", array[i].ProgressToken.ToString());
             Assert.Equal(i, array[i].Progress.Progress);
             Assert.Equal(10, array[i].Progress.Total);
             Assert.Equal($"Progress {i}", array[i].Progress.Message);

tests/ModelContextProtocol.Tests/Server/McpServerTests.cs

@@ -684,7 +684,7 @@ await transport.SendMessageAsync(new JsonRpcNotification
 
         var notification = await notificationReceived.Task;
         var progress = (ProgressNotification)notification.Params!;
-        Assert.Equal("\"abc\"", progress.ProgressToken.ToString());
+        Assert.Equal("abc", progress.ProgressToken.ToString());
         Assert.Equal(50, progress.Progress.Progress);
         Assert.Equal(100, progress.Progress.Total);
         Assert.Equal("Progress message", progress.Progress.Message);