Add ElicitationRequired to ToolMetadata

[alzimmermsft]

What does this PR do?

Adds ElicitationRequired to ToolMetadata to allow tools to denote that they require elicitation before the tool can run. This allows for tools to elicit without needing to be marked as Secret, which affects other areas of the server other than the elicitation requirement before running the tool.

GitHub issue number?

[Link to the GitHub issue this PR addresses]

Pre-merge Checklist

• Required for All PRs
  • Read contribution guidelines
  • PR title clearly describes the change
  • Commit history is clean with descriptive messages (cleanup guide)
  • Added comprehensive tests for new/modified functionality
  • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
• For MCP tool changes:
  • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
  • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
  • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
  • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
  • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
  • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
  • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
  • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
• Extra steps for Azure MCP Server tool changes:
  • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
  • 👉 For Community (non-Microsoft team member) PRs:
    • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
    • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

[IseduardoRezende]

Up up Up

[srnagar]

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.

[jongio]

Clean feature addition - the field keyword refactoring and new ElicitationRequired property follow existing patterns well. But there are a couple things that should get addressed before merge.

CommandFactoryToolLoader wasn't updated. At CommandFactoryToolLoader.cs:153, it still only checks metadata.Secret for triggering elicitation. You updated NamespaceToolLoader but missed this parallel code path. Tools loaded through CommandFactoryToolLoader with ElicitationRequired = true (but Secret = false) will silently skip elicitation. That's a bug.

Neither loader adds an ElicitationRequiredHint to Tool.Meta. Both loaders populate SecretHint and LocalRequiredHint for MCP client consumption, but there's no equivalent for the new flag. The ShouldTriggerElicitation extension method checks for "ElicitationRequired" in the JsonObject metadata, but nothing ever puts that key there - so that code path is unreachable for the new flag.

No tests cover the new behavior. McpServerElicitationExtensionsTests needs cases for ElicitationRequired in ShouldTriggerElicitation, and the converter/construction path should verify round-trip serialization of the new field.

On the plus side - the backing fields to field keyword refactoring cuts ~13 lines of boilerplate while keeping lazy init intact. The property follows existing patterns exactly (XML doc, [JsonIgnore] + MetadataDefinition, default value, constructor handling). Namespace fix in ToolMetadataJsonConverter.cs aligns it with its directory. The expression-body refactoring of SupportsElicitation and local variable decomposition in ShouldTriggerElicitation are both solid improvements.

NOTE: @srnagar's existing comment about updating the code comment in NamespaceToolLoader.cs is valid - not repeating it here.

[jongio]

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)

[jongio]

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.

[jongio]

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).