Skip to content

Add ElicitationRequired to ToolMetadata #1969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
alzimmermsft wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add ElicitationRequired to ToolMetadata #1969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ToolLoading/NamespaceToolLoader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ private async Task<CallToolResult> InvokeChildToolAsync(

// Check if this tool requires elicitation for sensitive data
Copy link
Member

@srnagar srnagar Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update comment as well.

Suggested change
// Check if this tool requires elicitation for sensitive data
// Check if this tool accesses sensitive data or explicitly requires elicitation.

var metadata = cmd.Metadata;
if (metadata.Secret)
if (metadata.Secret || metadata.ElicitationRequired)
Copy link
Contributor

@jongio jongio Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check was correctly updated here, but the identical pattern in CommandFactoryToolLoader.cs:153 was not:

// CommandFactoryToolLoader.cs:153 (not in this PR's diff)
if (metadata.Secret)  // ← missing || metadata.ElicitationRequired

Both tool loaders independently check metadata before calling HandleSecretElicitationAsync. If a tool is loaded via CommandFactoryToolLoader with ElicitationRequired = true and Secret = false, elicitation will be silently skipped.

Please update CommandFactoryToolLoader.cs:153 to match:

if (metadata.Secret || metadata.ElicitationRequired)

{
var elicitationResult = await HandleSecretElicitationAsync(
request,
Expand Down
130 changes: 61 additions & 69 deletions core/Microsoft.Mcp.Core/src/Commands/ToolMetadata.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the MIT License.

using System.Text.Json.Serialization;
using Azure.Mcp.Core.Commands;
using Microsoft.Mcp.Core.Models.Metadata;

namespace Microsoft.Mcp.Core.Commands;
Expand All @@ -14,19 +13,6 @@ namespace Microsoft.Mcp.Core.Commands;
[JsonConverter(typeof(ToolMetadataConverter))]
public sealed class ToolMetadata
{
private bool _destructive = true;
private bool _idempotent = false;
private bool _openWorld = true;
private bool _readOnly = false;
private bool _secret = false;
private bool _localRequired = false;

private MetadataDefinition? _destructiveProperty;
private MetadataDefinition? _idempotentProperty;
private MetadataDefinition? _openWorldProperty;
private MetadataDefinition? _readOnlyProperty;
private MetadataDefinition? _secretProperty;
private MetadataDefinition? _localRequiredProperty;
/// <summary>
/// Gets or sets whether the tool may perform destructive updates to its environment.
/// </summary>
Expand All @@ -41,18 +27,14 @@ public sealed class ToolMetadata
/// </para>
/// </remarks>
[JsonIgnore]
public bool Destructive
{
get => _destructive;
init => _destructive = value;
}
public bool Destructive { get; init; } = true;


[JsonPropertyName("destructive")]
public MetadataDefinition DestructiveProperty => _destructiveProperty ??= new MetadataDefinition
public MetadataDefinition DestructiveProperty => field ??= new MetadataDefinition
{
Value = _destructive,
Description = _destructive
Value = Destructive,
Description = Destructive
? "This tool may delete or modify existing resources in its environment."
: "This tool performs only additive updates without deleting or modifying existing resources."
};
Expand All @@ -70,17 +52,13 @@ public bool Destructive
/// </para>
/// </remarks>
[JsonIgnore]
public bool Idempotent
{
get => _idempotent;
init => _idempotent = value;
}
public bool Idempotent { get; init; } = false;

[JsonPropertyName("idempotent")]
public MetadataDefinition IdempotentProperty => _idempotentProperty ??= new MetadataDefinition
public MetadataDefinition IdempotentProperty => field ??= new MetadataDefinition
{
Value = _idempotent,
Description = _idempotent
Value = Idempotent,
Description = Idempotent
? "Running this operation multiple times with the same arguments produces the same result without additional effects."
: "Running this operation multiple times with the same arguments may have additional effects or produce different results."
};
Expand All @@ -98,17 +76,13 @@ public bool Idempotent
/// </para>
/// </remarks>
[JsonIgnore]
public bool OpenWorld
{
get => _openWorld;
init => _openWorld = value;
}
public bool OpenWorld { get; init; } = true;

[JsonPropertyName("openWorld")]
public MetadataDefinition OpenWorldProperty => _openWorldProperty ??= new MetadataDefinition
public MetadataDefinition OpenWorldProperty => field ??= new MetadataDefinition
{
Value = _openWorld,
Description = _openWorld
Value = OpenWorld,
Description = OpenWorld
? "This tool may interact with an unpredictable or dynamic set of entities (like web search)."
: "This tool's domain of interaction is closed and well-defined, limited to a specific set of entities (like memory access)."
};
Expand All @@ -130,17 +104,13 @@ public bool OpenWorld
/// </para>
/// </remarks>
[JsonIgnore]
public bool ReadOnly
{
get => _readOnly;
init => _readOnly = value;
}
public bool ReadOnly { get; init; } = false;

[JsonPropertyName("readOnly")]
public MetadataDefinition ReadOnlyProperty => _readOnlyProperty ??= new MetadataDefinition
public MetadataDefinition ReadOnlyProperty => field ??= new MetadataDefinition
{
Value = _readOnly,
Description = _readOnly
Value = ReadOnly,
Description = ReadOnly
? "This tool only performs read operations without modifying any state or data."
: "This tool may modify its environment and perform write operations (create, update, delete)."
};
Expand All @@ -162,21 +132,45 @@ public bool ReadOnly
/// </para>
/// </remarks>
[JsonIgnore]
public bool Secret
{
get => _secret;
init => _secret = value;
}
public bool Secret { get; init; } = false;

[JsonPropertyName("secret")]
public MetadataDefinition SecretProperty => _secretProperty ??= new MetadataDefinition
public MetadataDefinition SecretProperty => field ??= new MetadataDefinition
{
Value = _secret,
Description = _secret
Value = Secret,
Description = Secret
? "This tool handles sensitive data such as secrets, credentials, keys, or other confidential information."
: "This tool does not handle sensitive or secret information."
};

/// <summary>
/// Gets or sets whether this tool requires elicitation to gather user input before execution.
/// </summary>
/// <remarks>
/// <para>
/// If <see langword="true"/>, the tool requires elicitation to gather user input before execution.
/// If <see langword="false"/>, the tool can be executed without additional user input.
/// </para>
/// <para>
/// This metadata helps MCP clients understand when they need to trigger elicitation flows to gather necessary
/// information from users before executing the tool.
/// </para>
/// <para>
/// The default is <see langword="false"/>.
/// </para>
/// </remarks>
Copy link
Contributor

@jongio jongio Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new ElicitationRequired property and its integration points (constructor, converter, ShouldTriggerElicitation, NamespaceToolLoader) should have unit test coverage. Specifically:

  • McpServerElicitationExtensionsTests: Add cases for ShouldTriggerElicitation with ElicitationRequired = true (both alone and combined with Secret)
  • ToolMetadata construction: Verify ElicitationRequired is correctly initialized from MetadataDefinition
  • Converter round-trip: Verify "elicitationRequired" serializes/deserializes correctly

The existing McpServerElicitationExtensionsTests has good patterns to follow (10 existing test cases for Secret).

[JsonIgnore]
public bool ElicitationRequired { get; init; } = false;

[JsonPropertyName("elicitationRequired")]
public MetadataDefinition ElicitationRequiredProperty => field ??= new MetadataDefinition
{
Value = ElicitationRequired,
Description = ElicitationRequired
? "This tool requires elicitation to gather user input before execution."
: "This tool does not require elicitation and can be executed without additional user input."
};

/// <summary>
/// Gets or sets whether this tool requires local execution or resources.
/// </summary>
Expand All @@ -194,20 +188,16 @@ public bool Secret
/// </para>
/// </remarks>
[JsonIgnore]
public bool LocalRequired
{
get => _localRequired;
init => _localRequired = value;
}
public bool LocalRequired { get; init; } = false;

/// <summary>
/// Gets the localRequired metadata property with value and description for serialization.
/// </summary>
[JsonPropertyName("localRequired")]
public MetadataDefinition LocalRequiredProperty => _localRequiredProperty ??= new MetadataDefinition
public MetadataDefinition LocalRequiredProperty => field ??= new MetadataDefinition
{
Value = _localRequired,
Description = _localRequired
Value = LocalRequired,
Description = LocalRequired
? "This tool is only available when the Azure MCP server is configured to run as a Local MCP Server (STDIO)."
: "This tool is available in both local and remote server modes."
};
Expand All @@ -227,14 +217,16 @@ public ToolMetadata(
MetadataDefinition openWorld,
MetadataDefinition readOnly,
MetadataDefinition secret,
MetadataDefinition localRequired)
MetadataDefinition localRequired,
MetadataDefinition elicitationRequired)
{
_destructive = destructive?.Value ?? true;
_idempotent = idempotent?.Value ?? false;
_openWorld = openWorld?.Value ?? true;
_readOnly = readOnly?.Value ?? false;
_secret = secret?.Value ?? false;
_localRequired = localRequired?.Value ?? false;
Destructive = destructive?.Value ?? true;
Idempotent = idempotent?.Value ?? false;
OpenWorld = openWorld?.Value ?? true;
ReadOnly = readOnly?.Value ?? false;
Secret = secret?.Value ?? false;
LocalRequired = localRequired?.Value ?? false;
ElicitationRequired = elicitationRequired?.Value ?? false;
}

}
8 changes: 4 additions & 4 deletions core/Microsoft.Mcp.Core/src/Commands/ToolMetadataJsonConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

using System.Text.Json;
using System.Text.Json.Serialization;
using Azure.Mcp.Core.Areas.Server;
using Microsoft.Mcp.Core.Commands;
using Microsoft.Mcp.Core.Models.Metadata;

namespace Azure.Mcp.Core.Commands;
namespace Microsoft.Mcp.Core.Commands;

/// <summary>
/// Custom JSON converter for <see cref="ToolMetadata"/> that handles serialization and deserialization
Expand Down Expand Up @@ -35,7 +33,8 @@ MetadataDefinition GetMetadata(string name, bool defaultValue)
GetMetadata("openWorld", true),
GetMetadata("readOnly", false),
GetMetadata("secret", false),
GetMetadata("localRequired", false)
GetMetadata("localRequired", false),
GetMetadata("elicitationRequired", false)
);
}

Expand All @@ -55,6 +54,7 @@ void WriteMetadata(string name, MetadataDefinition def)
WriteMetadata("readOnly", value.ReadOnlyProperty);
WriteMetadata("secret", value.SecretProperty);
WriteMetadata("localRequired", value.LocalRequiredProperty);
WriteMetadata("elicitationRequired", value.ElicitationRequiredProperty);

writer.WriteEndObject();
}
Expand Down
19 changes: 12 additions & 7 deletions core/Microsoft.Mcp.Core/src/Extensions/McpServerElicitationExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,7 @@ public static async Task<ElicitationResponse> RequestElicitationAsync(
/// <param name="server">The MCP server instance.</param>
/// <returns>True if the client supports elicitation, false otherwise.</returns>
public static bool SupportsElicitation(this McpServer server)
{
return server?.ClientCapabilities?.Elicitation != null;
}
=> server?.ClientCapabilities?.Elicitation != null;

/// <summary>
/// Checks if elicitation should be triggered for a tool based on its metadata.
Expand All @@ -81,10 +79,17 @@ public static bool ShouldTriggerElicitation(this McpServer server, string toolNa
// Check if the metadata indicates this is a secret/sensitive tool
if (toolMetadata is JsonObject jsonMetadata)
{
return jsonMetadata.TryGetPropertyValue("Secret", out var secretValue) &&
secretValue is JsonValue jsonValue &&
jsonValue.TryGetValue(out bool isSecret) &&
isSecret;
var secret = jsonMetadata.TryGetPropertyValue("Secret", out var secretValue) &&
secretValue is JsonValue jsonValue &&
jsonValue.TryGetValue(out bool isSecret) &&
isSecret;

var elicitationRequired = jsonMetadata.TryGetPropertyValue("ElicitationRequired", out var elicitationValue) &&
elicitationValue is JsonValue elicitationJsonValue &&
elicitationJsonValue.TryGetValue(out bool isElicitationRequired) &&
Copy link
Contributor

@jongio jongio Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ShouldTriggerElicitation extension now checks "ElicitationRequired" in the JsonObject metadata, but nothing populates this key in Tool.Meta.

Both NamespaceToolLoader (line 589-601) and CommandFactoryToolLoader (line 242-254) add SecretHint and LocalRequiredHint to Tool.Meta when those flags are set, but neither adds an ElicitationRequiredHint. This makes the new ElicitationRequired branch in this method unreachable via Tool.Meta.

Consider adding to both loaders' meta-building sections:

if (metadata.ElicitationRequired)
{
    meta ??= new();
    meta["ElicitationRequiredHint"] = metadata.ElicitationRequired;
}

Alternatively, if Tool.Meta exposure is intentionally omitted for this flag, a comment explaining that would help future readers.

isElicitationRequired;

return secret || elicitationRequired;
}

return false;
Expand Down