U/deepaligarg/addcustommcp by deepaligargms · Pull Request #368 · microsoft/Agent365-devTools

deepaligargms · 2026-04-14T04:12:29Z

Adding commands for creating a custom mcp server

- Use environment-scoped URL (/mcp/environments/{id}/servers/MCPManagement) so RequestContextExtractor can resolve the environment from the route - Serialize selectedBaseTools as comma-separated string to match MCPManagement server's expected format (was incorrectly sending as JSON array) - Change SelectedBaseTools type from string[]? to string? in request model - Improve error handling: surface isError=true responses from MCPManagement instead of silently failing with "Inner JSON not found" - Add debug logging of raw MCPManagement response Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a new CLI subcommand to create a custom MCP server and wires it through the tooling service, along with request/response models and tests.

Changes:

Added create-custom-server subcommand under develop mcp and expanded command tests.
Introduced request/response models and a new CreateCustomMcpServerAsync service API that calls the MCPManagement tool endpoint.
Updated MCP-related constants and modified base URL construction in the tooling service.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/DevelopMcpCommandTests.cs	Updates subcommand count and adds option coverage for `create-custom-server`.
src/Microsoft.Agents.A365.DevTools.Cli/Services/IAgent365ToolingService.cs	Adds service contract for creating a custom MCP server.
src/Microsoft.Agents.A365.DevTools.Cli/Services/Agent365ToolingService.cs	Implements `CreateCustomMcpServerAsync`, adds shared JSON options, and changes base URL behavior.
src/Microsoft.Agents.A365.DevTools.Cli/Models/CreateCustomMcpServerResponse.cs	Adds response DTOs for MCPManagement create-custom-server results.
src/Microsoft.Agents.A365.DevTools.Cli/Models/CreateCustomMcpServerRequest.cs	Adds request DTOs for MCPManagement create-custom-server inputs.
src/Microsoft.Agents.A365.DevTools.Cli/Constants/McpConstants.cs	Changes app id / identifier URI constants.
src/Microsoft.Agents.A365.DevTools.Cli/Commands/DevelopMcpCommand.cs	Adds new `create-custom-server` subcommand implementation and options.

Copilot · 2026-04-14T04:19:10Z

+        // return $"{uri.Scheme}://{uri.Host}";
+        return "http://localhost:52857";


Hardcoding the Agent365 tools base URL to localhost will break the CLI outside a local dev environment (and bypasses the existing discover URL logic). Restore the original base URL construction and, if local override is needed, gate it behind configuration (e.g., config setting / env var) rather than committing a permanent localhost return.

Copilot · 2026-04-14T04:19:10Z

+    //public const string WorkIQToolsProdAppId = "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1";
+    public const string WorkIQToolsProdAppId = "05879165-0320-489e-b644-f72b33f3edf0";


Switching production constants to test values (and leaving the old values commented out) risks redirecting auth/resource targeting across all environments. Move these values to environment-specific configuration (or introduce separate constants per environment with explicit selection logic) and remove commented-out constants to avoid accidental shipping of test endpoints/IDs.

Copilot · 2026-04-14T04:19:10Z

    /// Agent 365 Tools identifier URI (used for admin consent URL construction).
    /// </summary>
-    public const string Agent365ToolsIdentifierUri = "https://agent365.svc.cloud.microsoft";
+    public const string Agent365ToolsIdentifierUri = "https://test.agent365.svc.cloud.microsoft";


Switching production constants to test values (and leaving the old values commented out) risks redirecting auth/resource targeting across all environments. Move these values to environment-specific configuration (or introduce separate constants per environment with explicit selection logic) and remove commented-out constants to avoid accidental shipping of test endpoints/IDs.

Copilot · 2026-04-14T04:19:11Z

+        var selectedBaseToolsOption = new Option<string[]?>("--selected-base-tools", description: "Comma-separated list of tool names to select from the base server") { AllowMultipleArgumentsPerToken = true };
+        var environmentIdOption = new Option<string?>(["--environment-id", "-e"], description: "Dataverse environment ID (null = tenant-level)");
+        var dryRunOption = new Option<bool>("--dry-run", description: "Show what would be done without executing");
+        var configOption = new Option<string>(["-c", "--config"], getDefaultValue: () => "a365.config.json", description: "Configuration file path");


The subcommand defines --config/-c but the handler signature and SetHandler bindings omit configOption, so the option has no effect (and users can’t influence which config file is used). Add a config parameter to the handler and include configOption in the SetHandler option list, then ensure the provided path is actually applied where configuration is loaded.

Copilot · 2026-04-14T04:19:11Z

+        command.AddOption(dryRunOption);
+        command.AddOption(configOption);
+
+        command.SetHandler(async (name, baseServerId, displayName, description, instructions, selectedBaseTools, environmentId, dryRun) =>


The subcommand defines --config/-c but the handler signature and SetHandler bindings omit configOption, so the option has no effect (and users can’t influence which config file is used). Add a config parameter to the handler and include configOption in the SetHandler option list, then ensure the provided path is actually applied where configuration is loaded.

Copilot · 2026-04-14T04:19:11Z

+        }, nameOption, baseServerIdOption, displayNameOption, descriptionOption, instructionsOption,
+           selectedBaseToolsOption, environmentIdOption, dryRunOption);


The subcommand defines --config/-c but the handler signature and SetHandler bindings omit configOption, so the option has no effect (and users can’t influence which config file is used). Add a config parameter to the handler and include configOption in the SetHandler option list, then ensure the provided path is actually applied where configuration is loaded.

Copilot · 2026-04-14T04:19:11Z

+        // Parse SSE response: join all "data: ..." lines
+        var dataJson = string.Concat(
+            responseContent.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None)
+               .Where(l => l.StartsWith("data:", StringComparison.OrdinalIgnoreCase))
+               .Select(l => l.Substring(5).Trim()));
+
+        // Parse outer JSON-RPC envelope
+        var root = JsonNode.Parse(dataJson);


If the response contains no data: lines, dataJson becomes empty and JsonNode.Parse(dataJson) can throw a JsonException, causing an unhandled failure path. Also, string.Concat can produce invalid JSON if the SSE payload is split across multiple data: lines that require separators/newlines. Consider validating dataJson is non-empty before parsing, wrapping parsing in a try/catch that logs parse errors, and using a join that preserves separators (e.g., newline) to safely reconstruct multi-line JSON.

Suggested change

// Parse SSE response: join all "data: ..." lines

var dataJson = string.Concat(

responseContent.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None)

.Where(l => l.StartsWith("data:", StringComparison.OrdinalIgnoreCase))

.Select(l => l.Substring(5).Trim()));

// Parse outer JSON-RPC envelope

var root = JsonNode.Parse(dataJson);

// Parse SSE response: join all "data: ..." lines using newlines to preserve multi-line payloads

var dataLines = responseContent.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None)

.Where(l => l.StartsWith("data:", StringComparison.OrdinalIgnoreCase))

.Select(l => l.Substring(5).Trim())

.ToArray();

var dataJson = string.Join("\n", dataLines);

if (string.IsNullOrWhiteSpace(dataJson))

{

_logger.LogError("MCPManagement response did not contain any SSE data payload to parse");

return null;

}

// Parse outer JSON-RPC envelope

JsonNode? root;

try

{

root = JsonNode.Parse(dataJson);

}

catch (JsonException ex)

{

_logger.LogError(ex, "Failed to parse MCPManagement SSE payload as JSON: {Payload}", dataJson);

return null;

}

Copilot · 2026-04-14T04:19:11Z

+        var displayNameOption = new Option<string?>(["--display-name", "-d"], description: "User-friendly display name");
+        var descriptionOption = new Option<string?>(["--description"], description: "Description of the custom MCP server");
+        var instructionsOption = new Option<string?>(["--instructions"], description: "AI agent instructions for how to use this server");
+        var selectedBaseToolsOption = new Option<string[]?>("--selected-base-tools", description: "Comma-separated list of tool names to select from the base server") { AllowMultipleArgumentsPerToken = true };


The option is declared as string[] with AllowMultipleArgumentsPerToken=true, but the help text says 'Comma-separated list'. This is inconsistent for users and maintainers. Either (a) update the description to 'one or more tool names' (multiple values) or (b) accept a single string and split on commas (comma-separated).

Suggested change

var selectedBaseToolsOption = new Option<string[]?>("--selected-base-tools", description: "Comma-separated list of tool names to select from the base server") { AllowMultipleArgumentsPerToken = true };

var selectedBaseToolsOption = new Option<string[]?>("--selected-base-tools", description: "One or more tool names to select from the base server") { AllowMultipleArgumentsPerToken = true };

Copilot · 2026-04-14T04:19:12Z

+                DisplayName = displayName,
+                Description = description,
+                Instructions = instructions,
+                SelectedBaseTools = selectedBaseTools?.Length > 0 ? string.Join(",", selectedBaseTools) : null,


The option is declared as string[] with AllowMultipleArgumentsPerToken=true, but the help text says 'Comma-separated list'. This is inconsistent for users and maintainers. Either (a) update the description to 'one or more tool names' (multiple values) or (b) accept a single string and split on commas (comma-separated).

deepaligargms and others added 2 commits April 10, 2026 13:39

Adding subcommands to create custom mcp server

84cf085

deepaligargms requested review from a team as code owners April 14, 2026 04:12

Copilot AI review requested due to automatic review settings April 14, 2026 04:12

Copilot started reviewing on behalf of deepaligargms April 14, 2026 04:18 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

U/deepaligarg/addcustommcp#368

U/deepaligarg/addcustommcp#368
deepaligargms wants to merge 2 commits intomicrosoft:mainfrom
deepaligargms:u/deepaligarg/addcustommcp

deepaligargms commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// return $"{uri.Scheme}://{uri.Host}";
		return "http://localhost:52857";

		//public const string WorkIQToolsProdAppId = "ea9ffc3e-8a23-4a7d-836d-234d7c7565c1";
		public const string WorkIQToolsProdAppId = "05879165-0320-489e-b644-f72b33f3edf0";

		}, nameOption, baseServerIdOption, displayNameOption, descriptionOption, instructionsOption,
		selectedBaseToolsOption, environmentIdOption, dryRunOption);

	var selectedBaseToolsOption = new Option<string[]?>("--selected-base-tools", description: "Comma-separated list of tool names to select from the base server") { AllowMultipleArgumentsPerToken = true };
	var selectedBaseToolsOption = new Option<string[]?>("--selected-base-tools", description: "One or more tool names to select from the base server") { AllowMultipleArgumentsPerToken = true };

Conversation

deepaligargms commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants