Add get-agent-instances and add-byo-scopes CLI commands#471
Open
ragurubhaarath wants to merge 3 commits into
Open
Add get-agent-instances and add-byo-scopes CLI commands#471ragurubhaarath wants to merge 3 commits into
ragurubhaarath wants to merge 3 commits into
Conversation
Add two new subcommands to the develop-mcp command group:
- get-agent-instances: Lists agent instance service principals for a
given blueprint ID. Accepts --blueprint-id (required), --tenant-id
(optional, auto-detected from az CLI), and --verbose.
- add-byo-scopes: Grants oauth2 delegated permissions (user_impersonation)
for BYO MCP servers to agent instances. Resolves the PPMI app ID via
the new GET /agents/mcpServers/byName/{serverName}/appIds endpoint,
then creates/updates oauth2PermissionGrants in Microsoft Graph.
Supports --server-names, --blueprint-id, --agent-instances, --tenant-id,
--dry-run, and --verbose.
Also adds GetMcpServerAppIdsByNameAsync to IAgent365ToolingService and
its implementation, plus the McpServerAppIdsResponse model.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
the
documentation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
The MCP Platform route changed from a path-based to a query-parameter
style: GET /agents/mcpServers/appIds?serverName={serverName}
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds two new develop-mcp subcommands to manage BYO MCP server delegated permissions for agent instances, including a new MCP Platform API call for resolving BYO server PPMI app IDs.
Changes:
- Adds
develop-mcp get-agent-instancesanddevelop-mcp add-byo-scopessubcommands (executors + command registration). - Extends
IAgent365ToolingService/Agent365ToolingServicewithGetMcpServerAppIdsByNameAsyncand a new response model. - Documents the new commands in
CHANGELOG.mdand wires the new dependencies inProgram.cs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/GetAgentInstancesExecutorTests.cs | Adds unit tests for the new executor and subcommand wiring. |
| src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/AddByoScopesExecutorTests.cs | Adds unit tests for validation paths and subcommand wiring for scope grants. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs | Adds a new service contract method to resolve BYO server app IDs. |
| src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs | Implements MCP Platform call to resolve BYO server app IDs. |
| src/Microsoft.Agents.A365.DevTools.Cli/Program.cs | Updates DevelopMcpCommand.CreateCommand invocation to pass AgentBlueprintService. |
| src/Microsoft.Agents.A365.DevTools.Cli/Models/McpServerAppIdsResponse.cs | Introduces response model for the new MCP Platform endpoint. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/GetAgentInstancesExecutor.cs | Implements executor for listing agent instances by blueprint ID. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopSubcommands/AddByoScopesExecutor.cs | Implements executor for granting delegated scopes to agent instances for BYO servers. |
| src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs | Registers the new subcommands and updates command factory signature. |
| CHANGELOG.md | Adds release-note entries for the new commands. |
Comment on lines
+48
to
+57
| // Resolve tenant ID | ||
| if (string.IsNullOrWhiteSpace(tenantId)) | ||
| { | ||
| tenantId = await ResolveTenantIdAsync(cancellationToken); | ||
| if (string.IsNullOrWhiteSpace(tenantId)) | ||
| { | ||
| _logger.LogError("Could not determine tenant ID. Provide --tenant-id or sign in with 'az login'."); | ||
| return false; | ||
| } | ||
| } |
Comment on lines
+90
to
+94
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to list agent instances for blueprint {BlueprintId}", blueprintId); | ||
| return false; | ||
| } |
Comment on lines
+78
to
+87
| // Resolve tenant ID | ||
| if (string.IsNullOrWhiteSpace(tenantId)) | ||
| { | ||
| tenantId = await ResolveTenantIdAsync(cancellationToken); | ||
| if (string.IsNullOrWhiteSpace(tenantId)) | ||
| { | ||
| _logger.LogError("Could not determine tenant ID. Provide --tenant-id or sign in with 'az login'."); | ||
| return false; | ||
| } | ||
| } |
Comment on lines
+219
to
+223
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to resolve agent instances for blueprint {BlueprintId}", blueprintId); | ||
| return null; | ||
| } |
Comment on lines
+273
to
+277
| catch (Exception ex) | ||
| { | ||
| _logger.LogDebug(ex, "Failed to resolve service principal for appId {AppId}", appId); | ||
| return null; | ||
| } |
| _logger.LogDebug("Acquiring access token for audience: {Audience}", audience); | ||
|
|
||
| var loginHint = await AzCliHelper.ResolveLoginHintAsync(); | ||
| var authToken = await _authService.GetAccessTokenAsync(audience, userId: loginHint); |
Comment on lines
+913
to
+917
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to get app IDs for MCP server {ServerName}", serverName); | ||
| return null; | ||
| } |
| /// </summary> | ||
| /// <param name="serverName">MCP server name</param> | ||
| /// <param name="cancellationToken">Cancellation token</param> | ||
| /// <returns>Response containing the MCP server's PPMI app ID, or null on failure</returns> |
Comment on lines
+143
to
+151
| [Fact] | ||
| public void CreateCommand_WithBlueprintService_IncludesGetAgentInstances() | ||
| { | ||
| var command = DevelopMcpCommand.CreateCommand( | ||
| _mockLogger, _mockToolingService, agentBlueprintService: _mockBlueprintService); | ||
|
|
||
| command.Subcommands.Select(sc => sc.Name).Should().Contain( | ||
| "get-agent-instances", | ||
| because: "providing the blueprint service should register the get-agent-instances subcommand"); |
Comment on lines
+168
to
+179
| [Fact] | ||
| public void CreateCommand_WithBlueprintAndGraphServices_IncludesAddByoScopes() | ||
| { | ||
| var command = DevelopMcpCommand.CreateCommand( | ||
| _mockLogger, _mockToolingService, | ||
| graphApiService: _mockGraphApiService, | ||
| agentBlueprintService: _mockBlueprintService); | ||
|
|
||
| command.Subcommands.Select(sc => sc.Name).Should().Contain( | ||
| "add-byo-scopes", | ||
| because: "both blueprint and graph services are provided"); | ||
| } |
Comment on lines
+8
to
+10
| /// <summary> | ||
| /// Response model for the GET /agents/mcpServers/byName/{serverName}/appIds endpoint. | ||
| /// </summary> |
Comment on lines
+97
to
+111
| private static async Task<string?> ResolveTenantIdAsync(CancellationToken cancellationToken) | ||
| { | ||
| // Use az account show to detect tenant ID from the current Azure CLI session | ||
| try | ||
| { | ||
| var isWindows = System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform( | ||
| System.Runtime.InteropServices.OSPlatform.Windows); | ||
| var startInfo = new System.Diagnostics.ProcessStartInfo | ||
| { | ||
| FileName = isWindows ? "cmd.exe" : "az", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; |
Comment on lines
+381
to
+406
| private static async Task<string?> ResolveTenantIdAsync(CancellationToken cancellationToken) | ||
| { | ||
| try | ||
| { | ||
| var isWindows = System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform( | ||
| System.Runtime.InteropServices.OSPlatform.Windows); | ||
| var startInfo = new System.Diagnostics.ProcessStartInfo | ||
| { | ||
| FileName = isWindows ? "cmd.exe" : "az", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
| if (isWindows) | ||
| { | ||
| startInfo.ArgumentList.Add("/c"); | ||
| startInfo.ArgumentList.Add("az"); | ||
| } | ||
| startInfo.ArgumentList.Add("account"); | ||
| startInfo.ArgumentList.Add("show"); | ||
| startInfo.ArgumentList.Add("--query"); | ||
| startInfo.ArgumentList.Add("tenantId"); | ||
| startInfo.ArgumentList.Add("-o"); | ||
| startInfo.ArgumentList.Add("tsv"); | ||
|
|
Comment on lines
+291
to
+304
| var graphToken = await _graphApiService.GetGraphAccessTokenAsync(tenantId, ct: cancellationToken); | ||
| if (string.IsNullOrWhiteSpace(graphToken)) | ||
| { | ||
| _logger.LogError("Failed to acquire Graph API access token"); | ||
| return GrantResult.Failed; | ||
| } | ||
|
|
||
| using var httpClient = Services.Internal.HttpClientFactory.CreateAuthenticatedClient(graphToken); | ||
|
|
||
| // Check existing grants | ||
| var filter = $"clientId eq '{clientSpId}' and resourceId eq '{resourceSpId}' and consentType eq '{AllPrincipalsConsentType}'"; | ||
| var getUrl = $"{GraphApiConstants.BaseUrl}/v1.0/oauth2PermissionGrants?$filter={Uri.EscapeDataString(filter)}"; | ||
|
|
||
| using var getResponse = await httpClient.GetAsync(getUrl, cancellationToken); |
Comment on lines
+674
to
+686
| command.SetHandler(async (context) => | ||
| { | ||
| var blueprintId = context.ParseResult.GetValueForOption(blueprintIdOption)!; | ||
| var tenantId = context.ParseResult.GetValueForOption(tenantIdOption); | ||
| var ct = context.GetCancellationToken(); | ||
|
|
||
| var executor = new GetAgentInstancesExecutor(logger, agentBlueprintService); | ||
| var success = await executor.ExecuteAsync(blueprintId, tenantId, ct); | ||
| if (!success) | ||
| { | ||
| context.ExitCode = 1; | ||
| } | ||
| }); |
Comment on lines
+732
to
+747
| command.SetHandler(async (context) => | ||
| { | ||
| var serverNames = context.ParseResult.GetValueForOption(serverNamesOption)!; | ||
| var blueprintId = context.ParseResult.GetValueForOption(blueprintIdOption); | ||
| var agentInstances = context.ParseResult.GetValueForOption(agentInstancesOption); | ||
| var tenantId = context.ParseResult.GetValueForOption(tenantIdOption); | ||
| var dryRun = context.ParseResult.GetValueForOption(dryRunOption); | ||
| var ct = context.GetCancellationToken(); | ||
|
|
||
| var executor = new AddByoScopesExecutor(logger, toolingService, agentBlueprintService, graphApiService); | ||
| var success = await executor.ExecuteAsync(serverNames, blueprintId, agentInstances, tenantId, dryRun, ct); | ||
| if (!success) | ||
| { | ||
| context.ExitCode = 1; | ||
| } | ||
| }); |
Align with the slimmed-down MCP Platform endpoint that returns a
single-property JSON object { mcpServerAppId: ... }.
Renames:
- McpServerAppIdsResponse -> McpServerAppIdResponse
- GetMcpServerAppIdsByNameAsync -> GetMcpServerAppIdByNameAsync
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+100
to
+109
| /// <summary> | ||
| /// Retrieves the PPMI application (client) ID for a BYO MCP server by name. | ||
| /// Calls GET /agents/mcpServers/appIds?serverName={serverName} on the MCP Platform. | ||
| /// </summary> | ||
| /// <param name="serverName">MCP server name</param> | ||
| /// <param name="cancellationToken">Cancellation token</param> | ||
| /// <returns>Response containing the MCP server's PPMI app ID, or null on failure</returns> | ||
| Task<McpServerAppIdResponse?> GetMcpServerAppIdByNameAsync( | ||
| string serverName, | ||
| CancellationToken cancellationToken = default); |
Comment on lines
+335
to
+339
| using var patchResponse = await httpClient.PatchAsync( | ||
| patchUrl, | ||
| new StringContent(JsonSerializer.Serialize(patchBody), Encoding.UTF8, "application/json"), | ||
| cancellationToken); | ||
|
|
Comment on lines
+360
to
+363
| using var createResponse = await httpClient.PostAsync( | ||
| createUrl, | ||
| new StringContent(JsonSerializer.Serialize(createBody), Encoding.UTF8, "application/json"), | ||
| cancellationToken); |
Comment on lines
+137
to
+141
| var mcpServerAppId = appIdResponse.McpServerAppId; | ||
| _logger.LogDebug("MCP server '{ServerName}' PPMI app ID: {AppId}", serverName, mcpServerAppId); | ||
|
|
||
| // Step 2: Resolve the PPMI app's service principal in the tenant | ||
| var ppmiSpId = await ResolveServicePrincipalIdAsync(tenantId, mcpServerAppId, cancellationToken); |
Comment on lines
+381
to
+406
| private static async Task<string?> ResolveTenantIdAsync(CancellationToken cancellationToken) | ||
| { | ||
| try | ||
| { | ||
| var isWindows = System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform( | ||
| System.Runtime.InteropServices.OSPlatform.Windows); | ||
| var startInfo = new System.Diagnostics.ProcessStartInfo | ||
| { | ||
| FileName = isWindows ? "cmd.exe" : "az", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
| if (isWindows) | ||
| { | ||
| startInfo.ArgumentList.Add("/c"); | ||
| startInfo.ArgumentList.Add("az"); | ||
| } | ||
| startInfo.ArgumentList.Add("account"); | ||
| startInfo.ArgumentList.Add("show"); | ||
| startInfo.ArgumentList.Add("--query"); | ||
| startInfo.ArgumentList.Add("tenantId"); | ||
| startInfo.ArgumentList.Add("-o"); | ||
| startInfo.ArgumentList.Add("tsv"); | ||
|
|
Comment on lines
+97
to
+123
| private static async Task<string?> ResolveTenantIdAsync(CancellationToken cancellationToken) | ||
| { | ||
| // Use az account show to detect tenant ID from the current Azure CLI session | ||
| try | ||
| { | ||
| var isWindows = System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform( | ||
| System.Runtime.InteropServices.OSPlatform.Windows); | ||
| var startInfo = new System.Diagnostics.ProcessStartInfo | ||
| { | ||
| FileName = isWindows ? "cmd.exe" : "az", | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| }; | ||
| if (isWindows) | ||
| { | ||
| startInfo.ArgumentList.Add("/c"); | ||
| startInfo.ArgumentList.Add("az"); | ||
| } | ||
| startInfo.ArgumentList.Add("account"); | ||
| startInfo.ArgumentList.Add("show"); | ||
| startInfo.ArgumentList.Add("--query"); | ||
| startInfo.ArgumentList.Add("tenantId"); | ||
| startInfo.ArgumentList.Add("-o"); | ||
| startInfo.ArgumentList.Add("tsv"); | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two new subcommands to the
develop-mcpcommand group for managing BYO MCP server permissions on agent instances.New Commands
develop-mcp get-agent-instancesLists agent instance service principals linked to a given blueprint ID.
Options:
--blueprint-id/-b(required) - Agent Identity Blueprint ID (GUID)--tenant-id(optional) - Azure AD tenant ID; auto-detected fromaz account showif omitted--verbose/-v- Enable verbose loggingdevelop-mcp add-byo-scopesGrants oauth2 delegated permissions (
user_impersonation) for BYO MCP servers to agent instances.Options:
--server-names/-s(required) - Comma-separated list of BYO server names--blueprint-id/-b(optional) - Blueprint ID to auto-resolve all agent instances--agent-instances/-i(optional) - Comma-separated list of agent instance SP IDs--tenant-id(optional) - Azure AD tenant ID; auto-detected if omitted--dry-run- Show what would be done without executing--verbose/-v- Enable verbose loggingAt least one of
--blueprint-idor--agent-instancesis required.Other Changes
GetMcpServerAppIdsByNameAsynctoIAgent365ToolingServiceMcpServerAppIdsResponsemodelDevelopMcpCommand.CreateCommandsignatureTesting