Address feedback

Author: stephentoub

Diff for: src/ModelContextProtocol/Server/AIFunctionMcpServerTool.cs

@@ -185,24 +185,24 @@ options.OpenWorld is not null ||
             newOptions.Name ??= attr.Name;
             newOptions.Title ??= attr.Title;
 
-            if (attr.DestructiveIsSet)
+            if (attr._destructive is bool destructive)
             {
-                newOptions.Destructive ??= attr.Destructive;
+                newOptions.Destructive ??= destructive;
             }
 
-            if (attr.IdempotentIsSet)
+            if (attr._idempotent is bool idempotent)
             {
-                newOptions.Idempotent ??= attr.Idempotent;
+                newOptions.Idempotent ??= idempotent;
             }
 
-            if (attr.OpenWorldIsSet)
+            if (attr._openWorld is bool openWorld)
             {
-                newOptions.OpenWorld ??= attr.OpenWorld;
+                newOptions.OpenWorld ??= openWorld;
             }
 
-            if (attr.ReadOnlyIsSet)
+            if (attr._readOnly is bool readOnly)
             {
-                newOptions.ReadOnly ??= attr.ReadOnly;
+                newOptions.ReadOnly ??= readOnly;
             }
         }
 

Diff for: src/ModelContextProtocol/Server/McpServerToolAttribute.cs

@@ -6,10 +6,17 @@
 [AttributeUsage(AttributeTargets.Method)]
 public sealed class McpServerToolAttribute : Attribute
 {
-    private bool? _destructive;
-    private bool? _idempotent;
-    private bool? _openWorld;
-    private bool? _readOnly;
+    // Defaults based on the spec
+    private const bool DestructiveDefault = true;
+    private const bool IdempotentDefault = false;
+    private const bool OpenWorldDefault = true;
+    private const bool ReadOnlyDefault = false;
+
+    // Nullable backing fields so we can distinguish
+    internal bool? _destructive;
+    internal bool? _idempotent;
+    internal bool? _openWorld;
+    internal bool? _readOnly;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="McpServerToolTypeAttribute"/> class.
@@ -30,41 +37,37 @@ public McpServerToolAttribute()
     /// <summary>
     /// Gets or sets whether the tool may perform destructive updates to its environment.
     /// </summary>
-    public bool Destructive { get => _destructive ?? true; set => _destructive = value; }
-
-    /// <summary>
-    /// Gets whether the <see cref="Destructive"/> property has been assigned a value.
-    /// </summary>
-    public bool DestructiveIsSet => _destructive.HasValue;
+    public bool Destructive 
+    {
+        get => _destructive ?? DestructiveDefault; 
+        set => _destructive = value; 
+    }
 
     /// <summary>
     /// Gets or sets whether calling the tool repeatedly with the same arguments will have no additional effect on its environment.
     /// </summary>
-    public bool Idempotent { get => _idempotent ?? false; set => _idempotent = value; }
-
-    /// <summary>
-    /// Gets whether the <see cref="Idempotent"/> property has been assigned a value.
-    /// </summary>
-    public bool IdempotentIsSet => _idempotent.HasValue;
+    public bool Idempotent 
+    {
+        get => _idempotent ?? IdempotentDefault;
+        set => _idempotent = value; 
+    }
 
     /// <summary>
     /// Gets or sets whether this tool may interact with an "open world" of external entities
     /// (e.g. the world of a web search tool is open, whereas that of a memory tool is not).
     /// </summary>
-    public bool OpenWorld { get => _openWorld ?? true; set => _openWorld = value; }
-
-    /// <summary>
-    /// Gets whether the <see cref="OpenWorld"/> property has been assigned a value.
-    /// </summary>
-    public bool OpenWorldIsSet => _openWorld.HasValue;
+    public bool OpenWorld
+    {
+        get => _openWorld ?? OpenWorldDefault; 
+        set => _openWorld = value; 
+    }
 
     /// <summary>
     /// Gets or sets whether the tool does not modify its environment.
     /// </summary>
-    public bool ReadOnly { get => _readOnly ?? false; set => _readOnly = value; }
-
-    /// <summary>
-    /// Gets whether the <see cref="ReadOnly"/> property has been assigned a value.
-    /// </summary>
-    public bool ReadOnlyIsSet => _readOnly.HasValue;
+    public bool ReadOnly 
+    {
+        get => _readOnly ?? ReadOnlyDefault; 
+        set => _readOnly = value; 
+    }
 }