Skip to content

Commit ea0cdae

Browse files
alzimmermsftalzimmermsft
authored
[Part 1] Migrate to new Option pattern in Validators and BindOptions (#2462)
* [Part 1] Migrate to new setup and options design * Fix linting
1 parent c7d6c12 commit ea0cdae

19 files changed

Lines changed: 70 additions & 112 deletions

File tree

‎core/Azure.Mcp.Core/src/Areas/Group/Commands/ResourceListCommand.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ protected override void RegisterOptions(Command command)
5252
protected override ResourceListOptions BindOptions(ParseResult parseResult)
5353
{
5454
var options = base.BindOptions(parseResult);
55-
options.ResourceGroup ??= parseResult.GetValueOrDefault<string>(OptionDefinitions.Common.ResourceGroup.Name);
55+
options.ResourceGroup ??= parseResult.GetValueOrDefault(OptionDefinitions.Common.ResourceGroup);
5656
return options;
5757
}
5858

‎core/Azure.Mcp.Core/src/Areas/Group/GroupSetup.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,13 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider)
2727
var group = new CommandGroup(Name, "Resource group operations - Commands for listing and managing Azure resource groups and their resources in your subscriptions.", Title);
2828

2929
// Register Group commands
30-
var listCommand = serviceProvider.GetRequiredService<GroupListCommand>();
31-
group.AddCommand(listCommand.Name, listCommand);
30+
group.AddCommand(serviceProvider.GetRequiredService<GroupListCommand>());
3231

3332
// Register Resource sub-group
3433
var resource = new CommandGroup("resource", "Resource operations - Commands for listing resources within a resource group.");
3534
group.AddSubGroup(resource);
3635

37-
var resourceListCommand = serviceProvider.GetRequiredService<ResourceListCommand>();
38-
resource.AddCommand(resourceListCommand.Name, resourceListCommand);
36+
resource.AddCommand(serviceProvider.GetRequiredService<ResourceListCommand>());
3937

4038
return group;
4139
}

‎core/Azure.Mcp.Core/src/Areas/Subscription/SubscriptionSetup.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider)
2828
var subscription = new CommandGroup(Name, "Azure subscription operations - Commands for listing and managing Azure subscriptions accessible to your account.", Title);
2929

3030
// Register Subscription commands
31-
var subscriptionListCommand = serviceProvider.GetRequiredService<SubscriptionListCommand>();
32-
subscription.AddCommand(subscriptionListCommand.Name, subscriptionListCommand);
31+
subscription.AddCommand(serviceProvider.GetRequiredService<SubscriptionListCommand>());
3332

3433
return subscription;
3534
}

‎core/Microsoft.Mcp.Core/src/Areas/Server/Commands/PluginTelemetryCommand.cs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -197,17 +197,17 @@ protected override PluginTelemetryOptions BindOptions(ParseResult parseResult)
197197
{
198198
return new PluginTelemetryOptions
199199
{
200-
Timestamp = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.Timestamp.Name),
201-
EventType = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.EventType.Name),
202-
SessionId = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.SessionId.Name),
203-
ClientType = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.ClientType.Name),
204-
ClientName = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.ClientName.Name),
205-
PluginName = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.PluginName.Name),
206-
PluginVersion = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.PluginVersion.Name),
207-
SkillName = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.SkillName.Name),
208-
SkillVersion = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.SkillVersion.Name),
209-
ToolName = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.ToolName.Name),
210-
FileReference = parseResult.GetValueOrDefault<string?>(PluginTelemetryOptionDefinitions.FileReference.Name)
200+
Timestamp = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.Timestamp),
201+
EventType = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.EventType),
202+
SessionId = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.SessionId),
203+
ClientType = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.ClientType),
204+
ClientName = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.ClientName),
205+
PluginName = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.PluginName),
206+
PluginVersion = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.PluginVersion),
207+
SkillName = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.SkillName),
208+
SkillVersion = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.SkillVersion),
209+
ToolName = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.ToolName),
210+
FileReference = parseResult.GetValueOrDefault(PluginTelemetryOptionDefinitions.FileReference)
211211
};
212212
}
213213

‎core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ServiceInfoCommand.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,7 @@ public override Task<CommandResponse> ExecuteAsync(CommandContext context, Parse
4444
try
4545
{
4646
context.Response.Results = ResponseResult.Create(
47-
new ServiceInfoCommandResult(
48-
_serverOptions.Value.Name,
49-
_serverOptions.Value.Version),
47+
new(_serverOptions.Value.Name, _serverOptions.Value.Version),
5048
ServiceInfoJsonContext.Default.ServiceInfoCommandResult);
5149
}
5250
catch (Exception ex)

‎core/Microsoft.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs‎

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,12 @@ protected override void RegisterOptions(Command command)
104104
command.Validators.Add(commandResult =>
105105
{
106106
string transport = ResolveTransport(commandResult);
107-
bool httpIncomingAuthDisabled = commandResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth);
107+
bool httpIncomingAuthDisabled = commandResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth);
108108
ValidateMode(commandResult.GetValueOrDefault(ServiceOptionDefinitions.Mode), commandResult);
109109
ValidateTransportConfiguration(transport, httpIncomingAuthDisabled, commandResult);
110110
ValidateNamespaceAndToolMutualExclusion(
111-
commandResult.GetValueOrDefault<string[]?>(ServiceOptionDefinitions.Namespace.Name),
112-
commandResult.GetValueOrDefault<string[]?>(ServiceOptionDefinitions.Tool.Name),
111+
commandResult.GetValueOrDefault(ServiceOptionDefinitions.Namespace),
112+
commandResult.GetValueOrDefault(ServiceOptionDefinitions.Tool),
113113
commandResult);
114114
ValidateOutgoingAuthStrategy(commandResult);
115115
ValidateSupportLoggingFolder(commandResult);
@@ -122,7 +122,7 @@ protected override void RegisterOptions(Command command)
122122
/// <param name="commandResult">Command result to update on failure.</param>
123123
private static void ValidateSupportLoggingFolder(CommandResult commandResult)
124124
{
125-
string? folderPath = commandResult.GetValueOrDefault<string?>(ServiceOptionDefinitions.DangerouslyWriteSupportLogsToDir.Name);
125+
string? folderPath = commandResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyWriteSupportLogsToDir);
126126

127127
if (folderPath is null)
128128
{
@@ -155,8 +155,8 @@ private static void ValidateSupportLoggingFolder(CommandResult commandResult)
155155
/// <returns>A configured ServiceStartOptions instance.</returns>
156156
protected override ServiceStartOptions BindOptions(ParseResult parseResult)
157157
{
158-
var mode = parseResult.GetValueOrDefault<string?>(ServiceOptionDefinitions.Mode.Name);
159-
var tools = parseResult.GetValueOrDefault<string[]?>(ServiceOptionDefinitions.Tool.Name);
158+
var mode = parseResult.GetValueOrDefault(ServiceOptionDefinitions.Mode);
159+
var tools = parseResult.GetValueOrDefault(ServiceOptionDefinitions.Tool);
160160

161161
// When --tool switch is used, automatically change the mode to "all"
162162
if (tools != null && tools.Length > 0)
@@ -169,18 +169,18 @@ protected override ServiceStartOptions BindOptions(ParseResult parseResult)
169169
var options = new ServiceStartOptions
170170
{
171171
Transport = ResolveTransport(parseResult),
172-
Namespace = parseResult.GetValueOrDefault<string[]?>(ServiceOptionDefinitions.Namespace.Name),
172+
Namespace = parseResult.GetValueOrDefault(ServiceOptionDefinitions.Namespace),
173173
Mode = mode,
174174
Tool = tools,
175-
ReadOnly = parseResult.GetValueOrDefault<bool?>(ServiceOptionDefinitions.ReadOnly.Name),
176-
Debug = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.Debug.Name),
177-
DangerouslyDisableHttpIncomingAuth = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth.Name),
178-
DangerouslyDisableElicitation = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableElicitation.Name),
175+
ReadOnly = parseResult.GetValueOrDefault(ServiceOptionDefinitions.ReadOnly),
176+
Debug = parseResult.GetValueOrDefault(ServiceOptionDefinitions.Debug),
177+
DangerouslyDisableHttpIncomingAuth = parseResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth),
178+
DangerouslyDisableElicitation = parseResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyDisableElicitation),
179179
OutgoingAuthStrategy = outgoingAuthStrategy,
180-
SupportLoggingFolder = parseResult.GetValueOrDefault<string?>(ServiceOptionDefinitions.DangerouslyWriteSupportLogsToDir.Name),
181-
DangerouslyDisableRetryLimits = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableRetryLimits.Name),
182-
Cloud = parseResult.GetValueOrDefault<string?>(ServiceOptionDefinitions.Cloud.Name),
183-
DisableCaching = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DisableCaching.Name)
180+
SupportLoggingFolder = parseResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyWriteSupportLogsToDir),
181+
DangerouslyDisableRetryLimits = parseResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyDisableRetryLimits),
182+
Cloud = parseResult.GetValueOrDefault(ServiceOptionDefinitions.Cloud),
183+
DisableCaching = parseResult.GetValueOrDefault(ServiceOptionDefinitions.DisableCaching)
184184
};
185185
return options;
186186
}
@@ -341,12 +341,12 @@ private static void ValidateNamespaceAndToolMutualExclusion(string[]? namespaces
341341
/// <param name="commandResult">Command result to update on failure.</param>
342342
private static void ValidateOutgoingAuthStrategy(CommandResult commandResult)
343343
{
344-
var outgoingAuthStrategy = commandResult.GetValueOrDefault<OutgoingAuthStrategy>(ServiceOptionDefinitions.OutgoingAuthStrategy.Name);
344+
var outgoingAuthStrategy = commandResult.GetValueOrDefault(ServiceOptionDefinitions.OutgoingAuthStrategy);
345345
if (outgoingAuthStrategy == OutgoingAuthStrategy.UseOnBehalfOf)
346346
{
347347
#if ENABLE_HTTP
348348
string transport = ResolveTransport(commandResult);
349-
bool httpIncomingAuthDisabled = commandResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth);
349+
bool httpIncomingAuthDisabled = commandResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth);
350350

351351
if (transport != TransportTypes.Http || httpIncomingAuthDisabled)
352352
{
@@ -875,13 +875,13 @@ private static void InitializeListingUrls(WebApplicationBuilder builder, Service
875875
private static OutgoingAuthStrategy ResolveAuthStrategy(ParseResult parseResult)
876876
{
877877
#if ENABLE_HTTP
878-
var outgoingAuthStrategy = parseResult.GetValueOrDefault<OutgoingAuthStrategy>(ServiceOptionDefinitions.OutgoingAuthStrategy.Name);
878+
var outgoingAuthStrategy = parseResult.GetValueOrDefault(ServiceOptionDefinitions.OutgoingAuthStrategy);
879879
if (outgoingAuthStrategy == OutgoingAuthStrategy.NotSet)
880880
{
881881
string transport = ResolveTransport(parseResult);
882882
if (transport == TransportTypes.Http)
883883
{
884-
bool httpIncomingAuthDisabled = parseResult.GetValueOrDefault<bool>(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth.Name);
884+
bool httpIncomingAuthDisabled = parseResult.GetValueOrDefault(ServiceOptionDefinitions.DangerouslyDisableHttpIncomingAuth);
885885
return httpIncomingAuthDisabled
886886
? OutgoingAuthStrategy.UseHostingEnvironmentIdentity
887887
: OutgoingAuthStrategy.UseOnBehalfOf;
@@ -903,19 +903,15 @@ private static OutgoingAuthStrategy ResolveAuthStrategy(ParseResult parseResult)
903903
/// <param name="parseResult">The parsed command line arguments.</param>
904904
/// <returns>The transport type string (stdio or http).</returns>
905905
private static string ResolveTransport(ParseResult parseResult)
906-
{
907-
return parseResult.GetValueOrDefault<string>(ServiceOptionDefinitions.Transport.Name) ?? TransportTypes.StdIo;
908-
}
906+
=> parseResult.GetValueOrDefault(ServiceOptionDefinitions.Transport) ?? TransportTypes.StdIo;
909907

910908
/// <summary>
911909
/// Resolves the transport type from command result, defaulting to STDIO if not specified.
912910
/// </summary>
913911
/// <param name="commandResult">The command result to extract transport from.</param>
914912
/// <returns>The transport type string (stdio or http).</returns>
915913
private static string ResolveTransport(CommandResult commandResult)
916-
{
917-
return commandResult.GetValueOrDefault<string>(ServiceOptionDefinitions.Transport.Name) ?? TransportTypes.StdIo;
918-
}
914+
=> commandResult.GetValueOrDefault(ServiceOptionDefinitions.Transport) ?? TransportTypes.StdIo;
919915

920916
private static WebApplication UseHttpsRedirectionIfEnabled(WebApplication app)
921917
{

‎core/Microsoft.Mcp.Core/src/Areas/Server/Options/PluginTelemetryOptionDefinitions.cs‎

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -17,88 +17,67 @@ public static class PluginTelemetryOptionDefinitions
1717
public const string ToolNameName = "tool-name";
1818
public const string FileReferenceName = "file-reference";
1919

20-
public static readonly Option<string> Timestamp = new(
21-
$"--{TimestampName}"
22-
)
20+
public static readonly Option<string> Timestamp = new($"--{TimestampName}")
2321
{
2422
Description = "Timestamp of the telemetry event in ISO 8601 format.",
2523
Required = true
2624
};
2725

28-
public static readonly Option<string> EventType = new(
29-
$"--{EventTypeName}"
30-
)
26+
public static readonly Option<string> EventType = new($"--{EventTypeName}")
3127
{
3228
Description = "Type of event being logged (e.g., 'skill_invocation', 'tool_invocation', 'reference_file_read').",
3329
Required = true
3430
};
3531

36-
public static readonly Option<string> SessionId = new(
37-
$"--{SessionIdName}"
38-
)
32+
public static readonly Option<string> SessionId = new($"--{SessionIdName}")
3933
{
4034
Description = "Session identifier for correlating related events.",
4135
Required = true
4236
};
4337

44-
public static readonly Option<string> ClientType = new(
45-
$"--{ClientTypeName}"
46-
)
38+
public static readonly Option<string?> ClientType = new($"--{ClientTypeName}")
4739
{
48-
Description = "Type of client invoking the telemetry (e.g., 'copilot-cli', 'claude-code', 'vscode'). Deprecated: prefer --client-name."
40+
Description = "Type of client invoking the telemetry (e.g., 'copilot-cli', 'claude-code', 'vscode'). Deprecated: prefer --client-name.",
41+
Required = false
4942
};
5043

51-
public static readonly Option<string> ClientName = new(
52-
$"--{ClientNameName}"
53-
)
44+
public static readonly Option<string?> ClientName = new($"--{ClientNameName}")
5445
{
5546
Description = "Name of the client invoking the telemetry (e.g., 'copilot-cli', 'claude-code', 'Visual Studio Code', 'Visual Studio Code - Insiders').",
5647
Required = false
5748
};
5849

59-
public static readonly Option<string> PluginName = new(
60-
$"--{PluginNameName}"
61-
)
50+
public static readonly Option<string?> PluginName = new($"--{PluginNameName}")
6251
{
6352
Description = "Name of the plugin being invoked.",
6453
Required = false
6554
};
6655

67-
public static readonly Option<string> PluginVersion = new(
68-
$"--{PluginVersionName}"
69-
)
56+
public static readonly Option<string?> PluginVersion = new($"--{PluginVersionName}")
7057
{
7158
Description = "Version of the plugin being invoked.",
7259
Required = false
7360
};
7461

75-
public static readonly Option<string> SkillName = new(
76-
$"--{SkillNameName}"
77-
)
62+
public static readonly Option<string?> SkillName = new($"--{SkillNameName}")
7863
{
7964
Description = "Name of the skill being invoked.",
8065
Required = false
8166
};
8267

83-
public static readonly Option<string> SkillVersion = new(
84-
$"--{SkillVersionName}"
85-
)
68+
public static readonly Option<string?> SkillVersion = new($"--{SkillVersionName}")
8669
{
8770
Description = "Version of the skill being invoked.",
8871
Required = false
8972
};
9073

91-
public static readonly Option<string> ToolName = new(
92-
$"--{ToolNameName}"
93-
)
74+
public static readonly Option<string?> ToolName = new($"--{ToolNameName}")
9475
{
9576
Description = "Name of the tool being invoked.",
9677
Required = false
9778
};
9879

99-
public static readonly Option<string> FileReference = new(
100-
$"--{FileReferenceName}"
101-
)
80+
public static readonly Option<string?> FileReference = new($"--{FileReferenceName}")
10281
{
10382
Description = "Plugin-relative file reference being accessed (will be validated against an allowlist).",
10483
Required = false

‎core/Microsoft.Mcp.Core/src/Areas/Server/ServerSetup.cs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,9 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider)
4040
var mcpServer = new CommandGroup(Name, "MCP Server operations - Commands for managing and interacting with the MCP Server.", Title);
4141

4242
// Register MCP Server commands
43-
var startCommand = serviceProvider.GetRequiredService<ServiceStartCommand>();
44-
mcpServer.AddCommand(startCommand.Name, startCommand);
45-
46-
var infoCommand = serviceProvider.GetRequiredService<ServiceInfoCommand>();
47-
mcpServer.AddCommand(infoCommand.Name, infoCommand);
48-
49-
var pluginTelemetryCommand = serviceProvider.GetRequiredService<PluginTelemetryCommand>();
50-
mcpServer.AddCommand(pluginTelemetryCommand.Name, pluginTelemetryCommand);
43+
mcpServer.AddCommand(serviceProvider.GetRequiredService<ServiceStartCommand>());
44+
mcpServer.AddCommand(serviceProvider.GetRequiredService<ServiceInfoCommand>());
45+
mcpServer.AddCommand(serviceProvider.GetRequiredService<PluginTelemetryCommand>());
5146

5247
return mcpServer;
5348
}

‎core/Microsoft.Mcp.Core/src/Areas/Tools/Commands/ToolsListCommand.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ protected override void RegisterOptions(Command command)
4949

5050
protected override ToolsListOptions BindOptions(ParseResult parseResult)
5151
{
52-
var namespaces = parseResult.GetValueOrDefault<string[]>(ToolsListOptionDefinitions.Namespace.Name) ?? [];
52+
var namespaces = parseResult.GetValueOrDefault(ToolsListOptionDefinitions.Namespace) ?? [];
5353
return new ToolsListOptions
5454
{
5555
NamespaceMode = parseResult.GetValueOrDefault(ToolsListOptionDefinitions.NamespaceMode),

‎core/Microsoft.Mcp.Core/src/Areas/Tools/ToolsSetup.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ public CommandGroup RegisterCommands(IServiceProvider serviceProvider)
2525
// Create Tools command group
2626
var tools = new CommandGroup(Name, "CLI tools operations - Commands for discovering and exploring the functionality available in this CLI tool.", Title);
2727

28-
var list = serviceProvider.GetRequiredService<ToolsListCommand>();
29-
tools.AddCommand(list.Name, list);
28+
tools.AddCommand(serviceProvider.GetRequiredService<ToolsListCommand>());
3029

3130
return tools;
3231
}

0 commit comments

Comments
 (0)