Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 1 addition & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Always put new classes and interfaces in separate files
- Always make members static if they can be
- All generated code needs to be AOT safe
- Never expose `RetryPolicyOptions` through tool options or service contracts; use Azure SDK retry defaults
- Always review your own code for consistency, maintainability, and testability
- Always ask for clarifications if the request is ambiguous or lacks sufficient context.

Expand Down
52 changes: 25 additions & 27 deletions .github/skills/add-azure-mcp-tools/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,6 @@ public class {Resource}{Operation}Options : ISubscriptionOption

[Option(OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[Option(Name = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
```

Expand Down Expand Up @@ -222,15 +219,13 @@ public interface I{Toolset}Service
string subscription,
string? resourceGroup = null,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null,
CancellationToken cancellationToken = default);

// Data plane operation (returns simple List)
Task<List<MyDetail>> GetDetailsAsync(
string resourceName,
string subscription,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null,
CancellationToken cancellationToken = default);
}
```
Expand All @@ -254,14 +249,13 @@ public class {Toolset}Service(IAzureService azureService)
string subscription,
string? resourceGroup = null,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null,
CancellationToken cancellationToken = default)
{
return await ExecuteResourceQueryAsync(
"Microsoft.{Provider}/{resourceType}",
resourceGroup,
subscription,
retryPolicy,
null,
ConvertToModel,
tenant: tenant,
cancellationToken: cancellationToken);
Expand Down Expand Up @@ -290,10 +284,9 @@ public class {Toolset}Service(IAzureService azureService)
string resourceGroup,
string subscription,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null,
CancellationToken cancellationToken = default)
{
var subscriptionResource = await AzureService.GetSubscription(subscription, tenant, retryPolicy);
var subscriptionResource = await AzureService.GetSubscription(subscription, tenant, cancellationToken);

// CRITICAL: Use GetResourceGroupAsync with await
var rgResource = await subscriptionResource.GetResourceGroupAsync(resourceGroup, cancellationToken);
Expand All @@ -319,11 +312,10 @@ public class MyService(IAzureService azureService)
private async Task<MyDataPlaneClient> CreateDataPlaneClientAsync(
string resourceName,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null,
CancellationToken cancellationToken = default)
{
var endpoint = GetResourceEndpoint(resourceName);
var options = ConfigureRetryPolicy(AddDefaultPolicies(new MyClientOptions()), retryPolicy);
var options = AddDefaultPolicies(new MyClientOptions());
options.Transport = new HttpClientTransport(AzureService.GetClient());
return new MyDataPlaneClient(
new Uri(endpoint),
Expand Down Expand Up @@ -411,7 +403,6 @@ public sealed class {Resource}{Operation}Command(
options.Subscription!,
options.ResourceGroup,
options.Tenant,
options.RetryPolicy,
cancellationToken);

context.Response.Results = ResponseResult.Create(
Expand Down Expand Up @@ -626,8 +617,10 @@ public class {Resource}{Operation}CommandTests
if (shouldSucceed)
{
Service.GetResourcesAsync(
Arg.Any<string?>(), Arg.Any<string>(), Arg.Any<string?>(),
Arg.Any<string?>(), Arg.Any<RetryPolicyOptions?>(),
Arg.Any<string?>(),
Arg.Any<string>(),
Arg.Any<string?>(),
Arg.Any<string?>(),
Arg.Any<CancellationToken>())
.Returns(new ResourceQueryResults<MyModel>([], false));
}
Expand All @@ -643,8 +636,10 @@ public class {Resource}{Operation}CommandTests
public async Task ExecuteAsync_DeserializationValidation()
{
Service.GetResourcesAsync(
Arg.Any<string?>(), Arg.Any<string>(), Arg.Any<string?>(),
Arg.Any<string?>(), Arg.Any<RetryPolicyOptions?>(),
Arg.Any<string?>(),
Arg.Any<string>(),
Arg.Any<string?>(),
Arg.Any<string?>(),
Arg.Any<CancellationToken>())
.Returns(new ResourceQueryResults<MyModel>([], false));

Expand All @@ -659,8 +654,10 @@ public class {Resource}{Operation}CommandTests
public async Task ExecuteAsync_HandlesServiceErrors()
{
Service.GetResourcesAsync(
Arg.Any<string?>(), Arg.Any<string>(), Arg.Any<string?>(),
Arg.Any<string?>(), Arg.Any<RetryPolicyOptions?>(),
Arg.Any<string?>(),
Arg.Any<string>(),
Arg.Any<string?>(),
Arg.Any<string?>(),
Arg.Any<CancellationToken>())
.ThrowsAsync(new Exception("Test error"));

Expand All @@ -675,8 +672,10 @@ public class {Resource}{Operation}CommandTests
public async Task ExecuteAsync_HandlesNotFound()
{
Service.GetResourcesAsync(
Arg.Any<string?>(), Arg.Any<string>(), Arg.Any<string?>(),
Arg.Any<string?>(), Arg.Any<RetryPolicyOptions?>(),
Arg.Any<string?>(),
Arg.Any<string>(),
Arg.Any<string?>(),
Arg.Any<string?>(),
Arg.Any<CancellationToken>())
.ThrowsAsync(new RequestFailedException((int)HttpStatusCode.NotFound, "Resource not found"));

Expand Down Expand Up @@ -1402,10 +1401,10 @@ var vms = await vmssResource.Value

```csharp
// ✅ Correct: use IAzureService
var subscriptionResource = await _azureService.GetSubscription(subscription, tenant, retryPolicy);
var subscriptionResource = await _azureService.GetSubscription(subscription, tenant, cancellationToken);

// ❌ Wrong: manual ARM client creation
var armClient = await CreateArmClientAsync(tenant, retryPolicy);
var armClient = await CreateArmClientAsync(tenant, null, cancellationToken: cancellationToken);
var subscriptionResource = armClient.GetSubscriptionResource(new ResourceIdentifier($"/subscriptions/{subscription}"));
```

Expand Down Expand Up @@ -1733,17 +1732,15 @@ catch (Exception ex)
Task<List<string>> GetStorageAccounts(
string subscription,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null,
CancellationToken cancellationToken = default);

// ❌ Incorrect: all on single line
Task<List<string>> GetStorageAccounts(string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null);
Task<List<string>> GetStorageAccounts(string subscription, string? tenant = null, CancellationToken cancellationToken = default);

// ❌ Incorrect: missing CancellationToken
Task<List<string>> GetStorageAccounts(
string subscription,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null);
string? tenant = null);
```

Rules:
Expand Down Expand Up @@ -2003,7 +2000,8 @@ private MyOptions? _currentOptions; // Race condition!

```csharp
public async Task<List<Resource>> GetResourcesAsync(
string subscription, string? tenant, RetryPolicyOptions? retryPolicy,
string subscription,
string? tenant,
CancellationToken cancellationToken)
{
// IAzureService handles tenant resolution for all modes:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/doc-gap-detector.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ For each file that needs changes, provide:
Depending on the gap, verify the relevant doc file follows the expected format:

**`servers/Azure.Mcp.Server/docs/azmcp-commands.md`** — must include:
- A global options table at the top (subscription, resource-group, tenant, retry-max-retries, retry-delay)
- A global options table at the top (subscription, resource-group, tenant)
- One `## azmcp <service> <resource> <operation>` section per command, containing a description, parameters table, and example usage block

**`servers/Azure.Mcp.Server/docs/e2eTestPrompts.md`** — must include:
Expand Down
2 changes: 1 addition & 1 deletion .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@
"problemMatcher": "$msCompile"
}
]
}
}
14 changes: 6 additions & 8 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,8 +386,6 @@ public class AccountGetOptions : ISubscriptionOption
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}

// Commands use two-generic base: SubscriptionCommand<TOptions, TResult>
Expand Down Expand Up @@ -446,7 +444,7 @@ protected override int GetStatusCode(Exception ex) => ex switch
try
{
// Command execution logic
var results = await service.GetResourcesAsync(options.Subscription!, options.RetryPolicy);
var results = await service.GetResourcesAsync(options.Subscription!, null);
context.Response.Results = ResponseResult.Create(new(results ?? []), ServiceJsonContext.Default.CommandResult);
}
catch (Exception ex)
Expand All @@ -468,13 +466,13 @@ Choose the appropriate base class based on operations:
public class StorageService(IAzureService azureService)
: BaseAzureResourceService(azureService), IStorageService
{
public async Task<ResourceQueryResults<StorageAccount>> ListAccountsAsync(string subscription, string? resourceGroup, RetryPolicyOptions? retryPolicy)
public async Task<ResourceQueryResults<StorageAccount>> ListAccountsAsync(string subscription, string? resourceGroup, CancellationToken cancellationToken)
{
return await ExecuteResourceQueryAsync(
"Microsoft.Storage/storageAccounts",
resourceGroup,
subscription,
retryPolicy,
null,
ConvertToStorageAccountModel,
cancellationToken: cancellationToken);
}
Expand All @@ -495,9 +493,9 @@ public class StorageService(IAzureService azureService)
string? accessTier = null,
bool? enableHierarchicalNamespace = null,
string? tenant = null,
RetryPolicyOptions? retryPolicy = null)
CancellationToken cancellationToken = default)
{
var subscriptionResource = await AzureService.GetSubscription(subscription, tenant, retryPolicy);
var subscriptionResource = await AzureService.GetSubscription(subscription, tenant, cancellationToken);
// Use subscriptionResource for write operations
}
}
Expand Down Expand Up @@ -633,7 +631,7 @@ All new toolsets must be AOT-compatible or excluded from native builds:
### Caching and Performance
- Use `ICacheService` for expensive Azure operations
- Implement `BaseAzureResourceService` for efficient Resource Graph queries
- Follow retry policy patterns with `RetryPolicyOptions`
- Use Azure SDK retry defaults; do not expose retry policy through tool or service contracts

## Remote MCP Server Architecture

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
var groups = await _azureService.GetResourceGroups(
options.Subscription!,
options.Tenant,
options.RetryPolicy,
cancellationToken);

context.Response.Results = ResponseResult.Create(new(groups ?? []), GroupJsonContext.Default.Result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
options.Subscription!,
options.ResourceGroup,
options.Tenant,
options.RetryPolicy,
cancellationToken).ToListAsync(cancellationToken);

context.Response.Results = ResponseResult.Create(new(resources), GroupJsonContext.Default.ResourceListCommandResult);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,4 @@ public sealed class BaseGroupOptions : ISubscriptionOption
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@ public sealed class ResourceListOptions : ISubscriptionOption
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
{
try
{
var subscriptions = await _azureService.GetSubscriptions(options.Tenant, options.RetryPolicy, cancellationToken);
var subscriptions = await _azureService.GetSubscriptions(options.Tenant, cancellationToken);

var defaultSubscriptionId = _azureService.GetDefaultSubscriptionId();
var subscriptionInfos = MapToSubscriptionInfos(subscriptions, defaultSubscriptionId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,4 @@ public sealed class SubscriptionListOptions
[Option(Description = OptionDescriptions.Tenant)]
public string? Tenant { get; set; }

[OptionContainer(Prefix = "retry")]
public RetryPolicyOptions? RetryPolicy { get; set; }
}
Loading
Loading