From 179d51b3ac5eb2446cf822ead7ab1b5d581dff93 Mon Sep 17 00:00:00 2001 From: Bobby Damore Date: Thu, 18 Dec 2025 20:07:13 -0800 Subject: [PATCH 1/6] feat: enable multiselect when creating a DA with local MCP actions --- packages/fx-core/resource/package.nls.json | 1 + .../generator/declarativeAgent/generator.ts | 87 +++++++++++++++++-- .../generator/templates/templateInfo.ts | 2 +- .../fx-core/src/component/generator/utils.ts | 6 +- .../scaffold/vsc/teamsProjectTypeNode.ts | 20 ++--- .../.vscode/mcp.json.tpl | 10 ++- 6 files changed, 97 insertions(+), 29 deletions(-) diff --git a/packages/fx-core/resource/package.nls.json b/packages/fx-core/resource/package.nls.json index 24325f898bb..56f03196a76 100644 --- a/packages/fx-core/resource/package.nls.json +++ b/packages/fx-core/resource/package.nls.json @@ -1144,6 +1144,7 @@ "core.createProjectQuestion.mcpServerType.remote.detail": "Connect to an MCP server hosted remotely via HTTP", "core.createProjectQuestion.mcpLocalServer.title": "Local MCP Server", "core.createProjectQuestion.mcpLocalServer.placeholder": "Select a local MCP server", + "core.createProjectQuestion.mcpLocalServer.validationHelp": "At least one MCP server must be selected", "core.MCPForDA.updatePluginManifest": "The operations selected from your MCP server are successfully added for Copilot to interact with. You can go to the '%s' to check on details. Now you are able to provision your declarative agent to continue.", "core.MCPForDA.mcpAuthMetadataMissingError": "Unable to find the authentication metadata in the MCP server. Please make sure your MCP server is configured correctly and try again. Details: %s", diff --git a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts index 95d1a8e02ea..21013013b75 100644 --- a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts +++ b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts @@ -15,6 +15,7 @@ import { Inputs, ManifestTemplateFileName, ok, + OptionItem, Platform, Result, signedIn, @@ -49,6 +50,84 @@ const enum telemetryProperties { isDeclarativeCopilot = "is-declarative-copilot", isMicrosoftEntra = "is-microsoft-entra", needAddPluginFromExisting = "need-add-plugin-from-existing", + mcpServerParseFailures = "mcp-server-parse-failures", + mcpServerParseFailureIds = "mcp-server-parse-failure-ids", +} + +/** + * Process selected MCP local servers from inputs and format for template + * Handles both single and multiple server selection for backward compatibility + */ +function processMCPLocalServers( + inputs: Inputs, + context?: ActionContext +): { + MCPLocalServers: Array<{ + name: string; + identifier: string; + command: string; + args: string; + notLast: boolean; + }>; +} { + const selectedOptions = inputs[QuestionNames.MCPLocalServer] as OptionItem[] | undefined; + + // Handle empty/invalid selection + if (!selectedOptions || !Array.isArray(selectedOptions) || selectedOptions.length === 0) { + return { + MCPLocalServers: [], + }; + } + + const failures: string[] = []; + + // Map selected options to server configs + const servers = selectedOptions + .map((option, index) => { + try { + // Validate option structure + if (!option.data || typeof option.data !== "string") { + throw new Error("Invalid option data structure"); + } + + const serverData = JSON.parse(option.data); + + // Validate parsed data + if (!serverData.identifier || !serverData.command || !Array.isArray(serverData.args)) { + throw new Error("Invalid server data format"); + } + + return { + name: option.id, + identifier: serverData.identifier, + command: serverData.command, + args: serverData.args.map((arg: string) => `"${arg}"`).join(", "), + notLast: index < selectedOptions.length - 1, + }; + } catch (error) { + // Track failure for telemetry + failures.push(option.id); + return null; + } + }) + .filter((server): server is NonNullable => server !== null); + + // Track failures for telemetry + if (failures.length > 0) { + merge(context?.telemetryProps, { + [telemetryProperties.mcpServerParseFailures]: failures.length.toString(), + [telemetryProperties.mcpServerParseFailureIds]: failures.join(","), + }); + } + + // If ALL servers failed, throw error + if (servers.length === 0) { + throw new Error(`All ${selectedOptions.length} selected MCP servers failed to parse`); + } + + return { + MCPLocalServers: servers, + }; } /** @@ -101,13 +180,7 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { MicrosoftEntra: auth === ApiAuthOptions.microsoftEntra().id ? "true" : "", IsLocalMCP: isLocalMCP ? "true" : "", ...(isLocalMCP - ? { - MCPLocalServerName: inputs[QuestionNames.MCPLocalServerName], - MCPLocalServerIdentifier: inputs[QuestionNames.MCPLocalServerIdentifier], - MCPCommand: inputs[QuestionNames.MCPLocalServerCommand], - MCPArgs: inputs[QuestionNames.MCPLocalServerArgs], - ServerName: inputs[QuestionNames.MCPLocalServerName], - } + ? processMCPLocalServers(inputs, actionContext) : MCPForDAServerUrl ? { MCPForDAServerUrl, diff --git a/packages/fx-core/src/component/generator/templates/templateInfo.ts b/packages/fx-core/src/component/generator/templates/templateInfo.ts index cf06c24122e..d0cf680b55b 100644 --- a/packages/fx-core/src/component/generator/templates/templateInfo.ts +++ b/packages/fx-core/src/component/generator/templates/templateInfo.ts @@ -5,7 +5,7 @@ import { ProgrammingLanguage } from "../../../question"; export interface TemplateInfo { templateName: string; language: ProgrammingLanguage; - replaceMap?: { [key: string]: string }; // key is the placeholder in the template file, value is the value to replace + replaceMap?: { [key: string]: any }; // key is the placeholder in the template file, value is the value to replace (supports strings, arrays, and objects for Mustache templates) filterFn?: (fileName: string) => boolean; // return true to include the file, false to exclude subFolder?: string; // subfolder under the template folder } diff --git a/packages/fx-core/src/component/generator/utils.ts b/packages/fx-core/src/component/generator/utils.ts index 59173d1c4d9..bf2236cdddc 100644 --- a/packages/fx-core/src/component/generator/utils.ts +++ b/packages/fx-core/src/component/generator/utils.ts @@ -163,7 +163,7 @@ export async function unzip( export function renderTemplateFileData( fileName: string, fileData: Buffer, - variables?: { [key: string]: string } + variables?: { [key: string]: any } ): string | Buffer { //only mustache files with name ending with .tpl if (path.extname(fileName) === templateFileExt) { @@ -181,7 +181,7 @@ export function renderTemplateFileData( function escapeEmptyVariable( template: string, - view: Record, + view: Record, tags: [string, string] = placeholderDelimiters ): string[][] { const parsed = Mustache.parse(template, tags) as string[][]; @@ -227,7 +227,7 @@ function updateTokens( export function renderTemplateFileName( fileName: string, fileData: Buffer, - variables?: { [key: string]: string } + variables?: { [key: string]: any } ): string { // Disable HTML escaping to prevent special characters from being encoded Mustache.escape = (value) => value; diff --git a/packages/fx-core/src/question/scaffold/vsc/teamsProjectTypeNode.ts b/packages/fx-core/src/question/scaffold/vsc/teamsProjectTypeNode.ts index 8acde94c7c6..ca6b5bd023a 100644 --- a/packages/fx-core/src/question/scaffold/vsc/teamsProjectTypeNode.ts +++ b/packages/fx-core/src/question/scaffold/vsc/teamsProjectTypeNode.ts @@ -351,7 +351,8 @@ export function MCPLocalServerSelectionNode(): IQTreeNode { data: { name: QuestionNames.MCPLocalServer, title: getLocalizedString("core.createProjectQuestion.mcpLocalServer.title"), - type: "singleSelect", + type: "multiSelect", + returnObject: true, staticOptions: [], placeholder: getLocalizedString("core.createProjectQuestion.mcpLocalServer.placeholder"), dynamicOptions: (inputs: Inputs) => { @@ -368,19 +369,10 @@ export function MCPLocalServerSelectionNode(): IQTreeNode { }), })); }, - onDidSelection: (item: string | OptionItem, inputs: Inputs) => { - try { - const serverInfo = item as OptionItem; - const serverData = JSON.parse(serverInfo.data as string); - inputs[QuestionNames.MCPLocalServerName] = serverInfo.id; - inputs[QuestionNames.MCPLocalServerIdentifier] = serverData.identifier; - inputs[QuestionNames.MCPLocalServerCommand] = serverData.command; - // Store args in the format needed by the template: "arg1", "arg2", "arg3" - inputs[QuestionNames.MCPLocalServerArgs] = serverData.args - .map((arg: string) => `"${arg}"`) - .join(", "); - } catch {} - }, + validation: { minItems: 1 }, + validationHelp: getLocalizedString( + "core.createProjectQuestion.mcpLocalServer.validationHelp" + ), }, children: [], }; diff --git a/templates/vsc/common/declarative-agent-with-action-from-mcp/.vscode/mcp.json.tpl b/templates/vsc/common/declarative-agent-with-action-from-mcp/.vscode/mcp.json.tpl index 1aaa8043590..3648cb9de4d 100644 --- a/templates/vsc/common/declarative-agent-with-action-from-mcp/.vscode/mcp.json.tpl +++ b/templates/vsc/common/declarative-agent-with-action-from-mcp/.vscode/mcp.json.tpl @@ -1,11 +1,13 @@ { "servers": { {{#IsLocalMCP}} - "{{MCPLocalServerName}}": { +{{#MCPLocalServers}} + "{{name}}": { "type": "stdio", - "command": "{{MCPCommand}}", - "args": [{{MCPArgs}}] - } + "command": "{{command}}", + "args": [{{args}}] + }{{#notLast}},{{/notLast}} +{{/MCPLocalServers}} {{/IsLocalMCP}} {{^IsLocalMCP}} "{{ServerName}}": { From 42b84bd4cbfee1c21fba863b1cf3dbf62373c8d5 Mon Sep 17 00:00:00 2001 From: Bobby Damore Date: Thu, 18 Dec 2025 21:19:25 -0800 Subject: [PATCH 2/6] test: adjust and add unit tests --- .../fx-core/src/question/questionNames.ts | 3 - .../declarativeAgentGenerator.test.ts | 256 +++++++++++++----- 2 files changed, 194 insertions(+), 65 deletions(-) diff --git a/packages/fx-core/src/question/questionNames.ts b/packages/fx-core/src/question/questionNames.ts index 92e46006b6f..d58fedc480f 100644 --- a/packages/fx-core/src/question/questionNames.ts +++ b/packages/fx-core/src/question/questionNames.ts @@ -145,10 +145,7 @@ export enum QuestionNames { DAMetaOSCapability = "da-meta-os-capability", MCPServerType = "mcp-server-type", MCPLocalServer = "mcp-local-server", - MCPLocalServerName = "mcp-local-server-name", MCPLocalServerIdentifier = "mcp-local-server-identifier", - MCPLocalServerCommand = "mcp-local-server-command", - MCPLocalServerArgs = "mcp-local-server-args", MCPForDAServerUrl = "mcp-da-server-url", MCPForDAServerName = "mcp-da-server-name", MCPForDATool = "mcp-da-tool", diff --git a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts index 8b532a6f3fa..abf82a74b40 100644 --- a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts +++ b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts @@ -6,19 +6,19 @@ */ import { + DeclarativeAgentManifest, + DynamicOptions, err, Inputs, + MultiSelectQuestion, ok, + OptionItem, Platform, PluginManifestSchema, - UserError, signedIn, - DeclarativeAgentManifest, signedOut, - SingleFileQuestion, SingleSelectQuestion, - DynamicOptions, - OptionItem, + UserError, } from "@microsoft/teamsfx-api"; import { assert } from "chai"; import fs from "fs-extra"; @@ -26,25 +26,27 @@ import "mocha"; import { RestoreFn } from "mocked-env"; import path from "path"; import sinon from "sinon"; +import { GraphClient } from "../../../src/client/graphClient"; +import { featureFlagManager } from "../../../src/common/featureFlags"; import { createContext, setTools } from "../../../src/common/globalVars"; import { copilotGptManifestUtils } from "../../../src/component/driver/teamsApp/utils/CopilotGptManifestUtils"; import { pluginManifestUtils } from "../../../src/component/driver/teamsApp/utils/PluginManifestUtils"; import { DeclarativeAgentGenerator } from "../../../src/component/generator/declarativeAgent/generator"; import * as generatorHelper from "../../../src/component/generator/declarativeAgent/helper"; import { TemplateNames } from "../../../src/component/generator/templates/templateNames"; +import * as utils from "../../../src/component/generator/utils"; import * as commons from "../../../src/component/utils/common"; +import { ODRProvider } from "../../../src/component/utils/odrProvider"; import { ActionStartOptions, ApiAuthOptions, QuestionNames } from "../../../src/question"; -import { ActionStartOptions as CapabilityActionStartOptions } from "../../../src/question/scaffold/vsc/CapabilityOptions"; -import { MockLogProvider, MockTools } from "../../core/utils"; -import { GraphClient } from "../../../src/client/graphClient"; -import { featureFlagManager } from "../../../src/common/featureFlags"; -import * as utils from "../../../src/component/generator/utils"; -import { DACapabilityOptions } from "../../../src/question/scaffold/vsc/CapabilityOptions"; import { - MCPServerTypeNode, + ActionStartOptions as CapabilityActionStartOptions, + DACapabilityOptions, +} from "../../../src/question/scaffold/vsc/CapabilityOptions"; +import { MCPLocalServerSelectionNode, + MCPServerTypeNode, } from "../../../src/question/scaffold/vsc/teamsProjectTypeNode"; -import { ODRProvider } from "../../../src/component/utils/odrProvider"; +import { MockLogProvider, MockTools } from "../../core/utils"; describe("copilotExtension", async () => { setTools(new MockTools()); @@ -1272,6 +1274,17 @@ describe("helper", async () => { assert.deepEqual(inputs["_McpOdrOutput"], []); }); + it("MCPLocalServerSelectionNode should have correct structure for multiselect", async () => { + const node = MCPLocalServerSelectionNode(); + const questionData = (await node.data) as MultiSelectQuestion; + + // Verify multiselect configuration + assert.equal(questionData.type, "multiSelect"); + assert.equal(questionData.returnObject, true); + assert.deepEqual(questionData.validation, { minItems: 1 }); + assert.equal(questionData.name, QuestionNames.MCPLocalServer); + }); + it("MCPLocalServerSelectionNode dynamicOptions should work correctly", async () => { const mockServers = [ { @@ -1302,7 +1315,7 @@ describe("helper", async () => { ]; const node = MCPLocalServerSelectionNode(); - const questionData = (await node.data) as SingleSelectQuestion; + const questionData = (await node.data) as MultiSelectQuestion; // Set up the inputs with the mock servers as if MCPServerTypeNode had set them const inputs: Inputs = { @@ -1337,53 +1350,141 @@ describe("helper", async () => { assert.deepEqual(data2.args, ["mcp", "--proxy", "test.server.2"]); }); - it("MCPLocalServerSelectionNode onDidSelection should set correct inputs", async () => { - const node = MCPLocalServerSelectionNode(); + it("processMCPLocalServers should handle single server selection", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); const inputs: Inputs = { platform: Platform.CLI, - }; - const nodeData = (await node.data) as SingleSelectQuestion; - - const selectedItem: OptionItem = { - id: "selected-server", - label: "Selected Server", - detail: "Test server detail", - data: JSON.stringify({ - identifier: "selected.server.identifier", - command: "odr.exe", - args: ["mcp", "--proxy", "selected.server.identifier"], - }), + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "test-server", + label: "Test Server", + data: JSON.stringify({ + identifier: "test.server", + command: "odr.exe", + args: ["mcp", "--proxy", "test.server"], + }), + }, + ], }; - // Test onDidSelection - nodeData.onDidSelection?.(selectedItem, inputs); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); - assert.equal(inputs[QuestionNames.MCPLocalServerName], "selected-server"); - assert.equal(inputs[QuestionNames.MCPLocalServerIdentifier], "selected.server.identifier"); - assert.equal(inputs[QuestionNames.MCPLocalServerCommand], "odr.exe"); - assert.equal( - inputs[QuestionNames.MCPLocalServerArgs], - '"mcp", "--proxy", "selected.server.identifier"' - ); + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify MCPLocalServers array is present + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + + // Verify server details + assert.equal(replaceMap.MCPLocalServers[0].name, "test-server"); + assert.equal(replaceMap.MCPLocalServers[0].identifier, "test.server"); + assert.equal(replaceMap.MCPLocalServers[0].command, "odr.exe"); + assert.equal(replaceMap.MCPLocalServers[0].args, '"mcp", "--proxy", "test.server"'); + assert.equal(replaceMap.MCPLocalServers[0].notLast, false); + } }); - it("MCPLocalServerSelectionNode onDidSelection should handle errors gracefully", async () => { - const { MCPLocalServerSelectionNode } = await import( - "../../../src/question/scaffold/vsc/teamsProjectTypeNode" - ); + it("processMCPLocalServers should handle multiple server selection", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); + const inputs: Inputs = { + platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "server-1", + label: "Server 1", + data: JSON.stringify({ + identifier: "server.1", + command: "odr.exe", + args: ["mcp", "--proxy", "server.1"], + }), + }, + { + id: "server-2", + label: "Server 2", + data: JSON.stringify({ + identifier: "server.2", + command: "odr.exe", + args: ["mcp", "--proxy", "server.2"], + }), + }, + ], + }; - const node = MCPLocalServerSelectionNode(); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); + + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify MCPLocalServers array has both servers + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 2); + + // Verify first server + assert.equal(replaceMap.MCPLocalServers[0].name, "server-1"); + assert.equal(replaceMap.MCPLocalServers[0].notLast, true); + + // Verify second server + assert.equal(replaceMap.MCPLocalServers[1].name, "server-2"); + assert.equal(replaceMap.MCPLocalServers[1].notLast, false); + } + }); + + it("processMCPLocalServers should handle empty selection", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); const inputs: Inputs = { platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [], }; - const nodeData = node.data as SingleSelectQuestion; - // Test with invalid item (string instead of OptionItem) - nodeData.onDidSelection?.("invalid-item", inputs); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); - // Should not throw error and should not set any inputs - assert.isUndefined(inputs[QuestionNames.MCPLocalServerName]); - assert.isUndefined(inputs[QuestionNames.MCPLocalServerIdentifier]); + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify empty array is returned + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 0); + } }); it("ODRProvider listServers should handle empty output", async () => { @@ -1646,7 +1747,7 @@ describe("helper", async () => { assert.equal(servers[0].tools.length, 0); }); - it("declarative agent generator should handle local MCP server inputs", async () => { + it("processMCPLocalServers should handle JSON parse errors gracefully", async () => { const generator = new DeclarativeAgentGenerator(); const context = createContext(); const inputs: Inputs = { @@ -1658,8 +1759,22 @@ describe("helper", async () => { [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, [QuestionNames.AppName]: "TestApp", [QuestionNames.MCPServerType]: "local", - [QuestionNames.MCPLocalServerName]: "test-local-server", - [QuestionNames.MCPLocalServerIdentifier]: "com.test.local.server", + [QuestionNames.MCPLocalServer]: [ + { + id: "valid-server", + label: "Valid Server", + data: JSON.stringify({ + identifier: "valid.server", + command: "odr.exe", + args: ["mcp", "--proxy", "valid.server"], + }), + }, + { + id: "invalid-server", + label: "Invalid Server", + data: "not-valid-json", + }, + ], }; const res = await generator.activate(context, inputs); @@ -1671,12 +1786,11 @@ describe("helper", async () => { if (info.isOk() && info.value[0].replaceMap) { const replaceMap = info.value[0].replaceMap; - // Verify local server information is included in replace map - assert.equal(replaceMap.MCPLocalServerName, "test-local-server"); - assert.equal(replaceMap.MCPLocalServerIdentifier, "com.test.local.server"); - - // Verify template name - assert.equal(info.value[0].templateName, "declarative-agent-with-action-from-mcp"); + // Should only include the valid server, invalid one filtered out + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + assert.equal(replaceMap.MCPLocalServers[0].name, "valid-server"); } }); @@ -1703,7 +1817,13 @@ describe("helper", async () => { assert.isTrue(res); assert.isTrue(info.isOk()); - // Test local configuration + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + assert.equal(replaceMap.MCPForDAServerUrl, "https://remote-mcp.example.com"); + assert.equal(replaceMap.ServerName, "remotemcpe"); + } + + // Test local configuration with multiselect const localInputs: Inputs = { platform: Platform.CLI, projectPath: "./", @@ -1713,8 +1833,17 @@ describe("helper", async () => { [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, [QuestionNames.AppName]: "TestApp", [QuestionNames.MCPServerType]: "local", - [QuestionNames.MCPLocalServerName]: "local-server", - [QuestionNames.MCPLocalServerIdentifier]: "local.server.id", + [QuestionNames.MCPLocalServer]: [ + { + id: "local-server", + label: "Local Server", + data: JSON.stringify({ + identifier: "local.server.id", + command: "odr.exe", + args: ["mcp", "--proxy", "local.server.id"], + }), + }, + ], }; res = await generator.activate(context, localInputs); @@ -1725,8 +1854,11 @@ describe("helper", async () => { if (info.isOk() && info.value[0].replaceMap) { const replaceMap = info.value[0].replaceMap; - assert.equal(replaceMap.MCPLocalServerName, "local-server"); - assert.equal(replaceMap.MCPLocalServerIdentifier, "local.server.id"); + // Verify MCPLocalServers array for local configuration + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + assert.equal(replaceMap.MCPLocalServers[0].name, "local-server"); } }); }); From e1df711147b3be6afa6cd5cffdc100940542aae0 Mon Sep 17 00:00:00 2001 From: Bobby Damore Date: Thu, 18 Dec 2025 21:19:25 -0800 Subject: [PATCH 3/6] test: adjust and add unit tests --- .../fx-core/src/question/questionNames.ts | 3 - .../declarativeAgentGenerator.test.ts | 256 +++++++++++++----- 2 files changed, 194 insertions(+), 65 deletions(-) diff --git a/packages/fx-core/src/question/questionNames.ts b/packages/fx-core/src/question/questionNames.ts index 92e46006b6f..d58fedc480f 100644 --- a/packages/fx-core/src/question/questionNames.ts +++ b/packages/fx-core/src/question/questionNames.ts @@ -145,10 +145,7 @@ export enum QuestionNames { DAMetaOSCapability = "da-meta-os-capability", MCPServerType = "mcp-server-type", MCPLocalServer = "mcp-local-server", - MCPLocalServerName = "mcp-local-server-name", MCPLocalServerIdentifier = "mcp-local-server-identifier", - MCPLocalServerCommand = "mcp-local-server-command", - MCPLocalServerArgs = "mcp-local-server-args", MCPForDAServerUrl = "mcp-da-server-url", MCPForDAServerName = "mcp-da-server-name", MCPForDATool = "mcp-da-tool", diff --git a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts index 8b532a6f3fa..abf82a74b40 100644 --- a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts +++ b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts @@ -6,19 +6,19 @@ */ import { + DeclarativeAgentManifest, + DynamicOptions, err, Inputs, + MultiSelectQuestion, ok, + OptionItem, Platform, PluginManifestSchema, - UserError, signedIn, - DeclarativeAgentManifest, signedOut, - SingleFileQuestion, SingleSelectQuestion, - DynamicOptions, - OptionItem, + UserError, } from "@microsoft/teamsfx-api"; import { assert } from "chai"; import fs from "fs-extra"; @@ -26,25 +26,27 @@ import "mocha"; import { RestoreFn } from "mocked-env"; import path from "path"; import sinon from "sinon"; +import { GraphClient } from "../../../src/client/graphClient"; +import { featureFlagManager } from "../../../src/common/featureFlags"; import { createContext, setTools } from "../../../src/common/globalVars"; import { copilotGptManifestUtils } from "../../../src/component/driver/teamsApp/utils/CopilotGptManifestUtils"; import { pluginManifestUtils } from "../../../src/component/driver/teamsApp/utils/PluginManifestUtils"; import { DeclarativeAgentGenerator } from "../../../src/component/generator/declarativeAgent/generator"; import * as generatorHelper from "../../../src/component/generator/declarativeAgent/helper"; import { TemplateNames } from "../../../src/component/generator/templates/templateNames"; +import * as utils from "../../../src/component/generator/utils"; import * as commons from "../../../src/component/utils/common"; +import { ODRProvider } from "../../../src/component/utils/odrProvider"; import { ActionStartOptions, ApiAuthOptions, QuestionNames } from "../../../src/question"; -import { ActionStartOptions as CapabilityActionStartOptions } from "../../../src/question/scaffold/vsc/CapabilityOptions"; -import { MockLogProvider, MockTools } from "../../core/utils"; -import { GraphClient } from "../../../src/client/graphClient"; -import { featureFlagManager } from "../../../src/common/featureFlags"; -import * as utils from "../../../src/component/generator/utils"; -import { DACapabilityOptions } from "../../../src/question/scaffold/vsc/CapabilityOptions"; import { - MCPServerTypeNode, + ActionStartOptions as CapabilityActionStartOptions, + DACapabilityOptions, +} from "../../../src/question/scaffold/vsc/CapabilityOptions"; +import { MCPLocalServerSelectionNode, + MCPServerTypeNode, } from "../../../src/question/scaffold/vsc/teamsProjectTypeNode"; -import { ODRProvider } from "../../../src/component/utils/odrProvider"; +import { MockLogProvider, MockTools } from "../../core/utils"; describe("copilotExtension", async () => { setTools(new MockTools()); @@ -1272,6 +1274,17 @@ describe("helper", async () => { assert.deepEqual(inputs["_McpOdrOutput"], []); }); + it("MCPLocalServerSelectionNode should have correct structure for multiselect", async () => { + const node = MCPLocalServerSelectionNode(); + const questionData = (await node.data) as MultiSelectQuestion; + + // Verify multiselect configuration + assert.equal(questionData.type, "multiSelect"); + assert.equal(questionData.returnObject, true); + assert.deepEqual(questionData.validation, { minItems: 1 }); + assert.equal(questionData.name, QuestionNames.MCPLocalServer); + }); + it("MCPLocalServerSelectionNode dynamicOptions should work correctly", async () => { const mockServers = [ { @@ -1302,7 +1315,7 @@ describe("helper", async () => { ]; const node = MCPLocalServerSelectionNode(); - const questionData = (await node.data) as SingleSelectQuestion; + const questionData = (await node.data) as MultiSelectQuestion; // Set up the inputs with the mock servers as if MCPServerTypeNode had set them const inputs: Inputs = { @@ -1337,53 +1350,141 @@ describe("helper", async () => { assert.deepEqual(data2.args, ["mcp", "--proxy", "test.server.2"]); }); - it("MCPLocalServerSelectionNode onDidSelection should set correct inputs", async () => { - const node = MCPLocalServerSelectionNode(); + it("processMCPLocalServers should handle single server selection", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); const inputs: Inputs = { platform: Platform.CLI, - }; - const nodeData = (await node.data) as SingleSelectQuestion; - - const selectedItem: OptionItem = { - id: "selected-server", - label: "Selected Server", - detail: "Test server detail", - data: JSON.stringify({ - identifier: "selected.server.identifier", - command: "odr.exe", - args: ["mcp", "--proxy", "selected.server.identifier"], - }), + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "test-server", + label: "Test Server", + data: JSON.stringify({ + identifier: "test.server", + command: "odr.exe", + args: ["mcp", "--proxy", "test.server"], + }), + }, + ], }; - // Test onDidSelection - nodeData.onDidSelection?.(selectedItem, inputs); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); - assert.equal(inputs[QuestionNames.MCPLocalServerName], "selected-server"); - assert.equal(inputs[QuestionNames.MCPLocalServerIdentifier], "selected.server.identifier"); - assert.equal(inputs[QuestionNames.MCPLocalServerCommand], "odr.exe"); - assert.equal( - inputs[QuestionNames.MCPLocalServerArgs], - '"mcp", "--proxy", "selected.server.identifier"' - ); + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify MCPLocalServers array is present + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + + // Verify server details + assert.equal(replaceMap.MCPLocalServers[0].name, "test-server"); + assert.equal(replaceMap.MCPLocalServers[0].identifier, "test.server"); + assert.equal(replaceMap.MCPLocalServers[0].command, "odr.exe"); + assert.equal(replaceMap.MCPLocalServers[0].args, '"mcp", "--proxy", "test.server"'); + assert.equal(replaceMap.MCPLocalServers[0].notLast, false); + } }); - it("MCPLocalServerSelectionNode onDidSelection should handle errors gracefully", async () => { - const { MCPLocalServerSelectionNode } = await import( - "../../../src/question/scaffold/vsc/teamsProjectTypeNode" - ); + it("processMCPLocalServers should handle multiple server selection", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); + const inputs: Inputs = { + platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "server-1", + label: "Server 1", + data: JSON.stringify({ + identifier: "server.1", + command: "odr.exe", + args: ["mcp", "--proxy", "server.1"], + }), + }, + { + id: "server-2", + label: "Server 2", + data: JSON.stringify({ + identifier: "server.2", + command: "odr.exe", + args: ["mcp", "--proxy", "server.2"], + }), + }, + ], + }; - const node = MCPLocalServerSelectionNode(); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); + + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify MCPLocalServers array has both servers + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 2); + + // Verify first server + assert.equal(replaceMap.MCPLocalServers[0].name, "server-1"); + assert.equal(replaceMap.MCPLocalServers[0].notLast, true); + + // Verify second server + assert.equal(replaceMap.MCPLocalServers[1].name, "server-2"); + assert.equal(replaceMap.MCPLocalServers[1].notLast, false); + } + }); + + it("processMCPLocalServers should handle empty selection", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); const inputs: Inputs = { platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [], }; - const nodeData = node.data as SingleSelectQuestion; - // Test with invalid item (string instead of OptionItem) - nodeData.onDidSelection?.("invalid-item", inputs); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); - // Should not throw error and should not set any inputs - assert.isUndefined(inputs[QuestionNames.MCPLocalServerName]); - assert.isUndefined(inputs[QuestionNames.MCPLocalServerIdentifier]); + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify empty array is returned + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 0); + } }); it("ODRProvider listServers should handle empty output", async () => { @@ -1646,7 +1747,7 @@ describe("helper", async () => { assert.equal(servers[0].tools.length, 0); }); - it("declarative agent generator should handle local MCP server inputs", async () => { + it("processMCPLocalServers should handle JSON parse errors gracefully", async () => { const generator = new DeclarativeAgentGenerator(); const context = createContext(); const inputs: Inputs = { @@ -1658,8 +1759,22 @@ describe("helper", async () => { [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, [QuestionNames.AppName]: "TestApp", [QuestionNames.MCPServerType]: "local", - [QuestionNames.MCPLocalServerName]: "test-local-server", - [QuestionNames.MCPLocalServerIdentifier]: "com.test.local.server", + [QuestionNames.MCPLocalServer]: [ + { + id: "valid-server", + label: "Valid Server", + data: JSON.stringify({ + identifier: "valid.server", + command: "odr.exe", + args: ["mcp", "--proxy", "valid.server"], + }), + }, + { + id: "invalid-server", + label: "Invalid Server", + data: "not-valid-json", + }, + ], }; const res = await generator.activate(context, inputs); @@ -1671,12 +1786,11 @@ describe("helper", async () => { if (info.isOk() && info.value[0].replaceMap) { const replaceMap = info.value[0].replaceMap; - // Verify local server information is included in replace map - assert.equal(replaceMap.MCPLocalServerName, "test-local-server"); - assert.equal(replaceMap.MCPLocalServerIdentifier, "com.test.local.server"); - - // Verify template name - assert.equal(info.value[0].templateName, "declarative-agent-with-action-from-mcp"); + // Should only include the valid server, invalid one filtered out + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + assert.equal(replaceMap.MCPLocalServers[0].name, "valid-server"); } }); @@ -1703,7 +1817,13 @@ describe("helper", async () => { assert.isTrue(res); assert.isTrue(info.isOk()); - // Test local configuration + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + assert.equal(replaceMap.MCPForDAServerUrl, "https://remote-mcp.example.com"); + assert.equal(replaceMap.ServerName, "remotemcpe"); + } + + // Test local configuration with multiselect const localInputs: Inputs = { platform: Platform.CLI, projectPath: "./", @@ -1713,8 +1833,17 @@ describe("helper", async () => { [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, [QuestionNames.AppName]: "TestApp", [QuestionNames.MCPServerType]: "local", - [QuestionNames.MCPLocalServerName]: "local-server", - [QuestionNames.MCPLocalServerIdentifier]: "local.server.id", + [QuestionNames.MCPLocalServer]: [ + { + id: "local-server", + label: "Local Server", + data: JSON.stringify({ + identifier: "local.server.id", + command: "odr.exe", + args: ["mcp", "--proxy", "local.server.id"], + }), + }, + ], }; res = await generator.activate(context, localInputs); @@ -1725,8 +1854,11 @@ describe("helper", async () => { if (info.isOk() && info.value[0].replaceMap) { const replaceMap = info.value[0].replaceMap; - assert.equal(replaceMap.MCPLocalServerName, "local-server"); - assert.equal(replaceMap.MCPLocalServerIdentifier, "local.server.id"); + // Verify MCPLocalServers array for local configuration + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + assert.equal(replaceMap.MCPLocalServers[0].name, "local-server"); } }); }); From 74462e8130cb9ebd3cf95ee25910c8a2146dbc75 Mon Sep 17 00:00:00 2001 From: Bobby Damore Date: Tue, 23 Dec 2025 17:44:43 -0800 Subject: [PATCH 4/6] test: increase coverage --- .../generator/declarativeAgent/generator.ts | 271 ++++++++++-------- .../declarativeAgentGenerator.test.ts | 139 +++++++++ 2 files changed, 283 insertions(+), 127 deletions(-) diff --git a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts index 21013013b75..e9c5c5f683f 100644 --- a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts +++ b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts @@ -8,7 +8,6 @@ import { AppPackageFolderName, Context, - DefaultPluginManifestFileName, err, FxError, GeneratorResult, @@ -18,16 +17,21 @@ import { OptionItem, Platform, Result, - signedIn, + SystemError, } from "@microsoft/teamsfx-api"; +import fs from "fs-extra"; import { merge } from "lodash"; import path from "path"; +import { featureFlagManager, FeatureFlags } from "../../../common/featureFlags"; +import { convertToAlphanumericOnly } from "../../../common/stringUtils"; +import { assembleError } from "../../../error"; import { ActionStartOptions, ApiAuthOptions, ProgrammingLanguage, QuestionNames, } from "../../../question"; +import { EmbeddedKnowledgeLocalDirectoryName } from "../../driver/teamsApp/constants"; import { copilotGptManifestUtils } from "../../driver/teamsApp/utils/CopilotGptManifestUtils"; import { ActionContext } from "../../middleware/actionExecutionMW"; import { outputScaffoldingWarningMessage } from "../../utils/common"; @@ -35,15 +39,8 @@ import { DefaultTemplateGenerator } from "../defaultGenerator"; import { Generator } from "../generator"; import { TemplateInfo } from "../templates/templateInfo"; import { TemplateNames } from "../templates/templateNames"; -import { addExistingPlugin } from "./helper"; -import { getDefaultString } from "../../../common/localizeUtils"; -import { EmbeddedKnowledgeLocalDirectoryName } from "../../driver/teamsApp/constants"; -import fs from "fs-extra"; -import { featureFlagManager, FeatureFlags } from "../../../common/featureFlags"; -import { convertToAlphanumericOnly } from "../../../common/stringUtils"; import { setGeneralSensitivityLabel } from "../utils"; -import { GraphClient } from "../../../client/graphClient"; -import { ListSensitivityLabelScope } from "../../../common/constants"; +import { addExistingPlugin } from "./helper"; const enum telemetryProperties { templateName = "template-name", @@ -54,82 +51,6 @@ const enum telemetryProperties { mcpServerParseFailureIds = "mcp-server-parse-failure-ids", } -/** - * Process selected MCP local servers from inputs and format for template - * Handles both single and multiple server selection for backward compatibility - */ -function processMCPLocalServers( - inputs: Inputs, - context?: ActionContext -): { - MCPLocalServers: Array<{ - name: string; - identifier: string; - command: string; - args: string; - notLast: boolean; - }>; -} { - const selectedOptions = inputs[QuestionNames.MCPLocalServer] as OptionItem[] | undefined; - - // Handle empty/invalid selection - if (!selectedOptions || !Array.isArray(selectedOptions) || selectedOptions.length === 0) { - return { - MCPLocalServers: [], - }; - } - - const failures: string[] = []; - - // Map selected options to server configs - const servers = selectedOptions - .map((option, index) => { - try { - // Validate option structure - if (!option.data || typeof option.data !== "string") { - throw new Error("Invalid option data structure"); - } - - const serverData = JSON.parse(option.data); - - // Validate parsed data - if (!serverData.identifier || !serverData.command || !Array.isArray(serverData.args)) { - throw new Error("Invalid server data format"); - } - - return { - name: option.id, - identifier: serverData.identifier, - command: serverData.command, - args: serverData.args.map((arg: string) => `"${arg}"`).join(", "), - notLast: index < selectedOptions.length - 1, - }; - } catch (error) { - // Track failure for telemetry - failures.push(option.id); - return null; - } - }) - .filter((server): server is NonNullable => server !== null); - - // Track failures for telemetry - if (failures.length > 0) { - merge(context?.telemetryProps, { - [telemetryProperties.mcpServerParseFailures]: failures.length.toString(), - [telemetryProperties.mcpServerParseFailureIds]: failures.join(","), - }); - } - - // If ALL servers failed, throw error - if (servers.length === 0) { - throw new Error(`All ${selectedOptions.length} selected MCP servers failed to parse`); - } - - return { - MCPLocalServers: servers, - }; -} - /** * Generator for copilot extensions including declarative copilot with no plugin, * declarative copilot with API plugin from scratch, declarative copilot with existing plugin, @@ -166,49 +87,57 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { const MCPServerType = inputs[QuestionNames.MCPServerType]; const isLocalMCP = MCPServerType === "local"; const MCPForDAServerUrl = inputs[QuestionNames.MCPForDAServerUrl]; - const replaceMap = { - ...Generator.getDefaultVariables( - inputs[QuestionNames.TemplateName] === TemplateNames.DeclarativeAgentWithTypeSpec - ? convertToAlphanumericOnly(appName) - : appName, - safeProjectNameFromVS, - solutionNameFromVS, - inputs.targetFramework, - inputs.placeProjectFileInSolutionDir === "true" - ), - DeclarativeCopilot: "true", - MicrosoftEntra: auth === ApiAuthOptions.microsoftEntra().id ? "true" : "", - IsLocalMCP: isLocalMCP ? "true" : "", - ...(isLocalMCP - ? processMCPLocalServers(inputs, actionContext) - : MCPForDAServerUrl - ? { - MCPForDAServerUrl, - ServerName: new URL(MCPForDAServerUrl).host - .replace(/[^a-zA-Z0-9]/g, "") - .substring(0, 10), - } - : {}), - }; - const templateName = inputs[QuestionNames.TemplateName]; - merge(actionContext?.telemetryProps, { - [telemetryProperties.templateName]: templateName, - [telemetryProperties.isMicrosoftEntra]: - auth === ApiAuthOptions.microsoftEntra().id ? "true" : "", - [telemetryProperties.needAddPluginFromExisting]: - inputs[QuestionNames.ActionType] === ActionStartOptions.existingPlugin().id.toString(), - }); + try { + const replaceMap = { + ...Generator.getDefaultVariables( + inputs[QuestionNames.TemplateName] === TemplateNames.DeclarativeAgentWithTypeSpec + ? convertToAlphanumericOnly(appName) + : appName, + safeProjectNameFromVS, + solutionNameFromVS, + inputs.targetFramework, + inputs.placeProjectFileInSolutionDir === "true" + ), + DeclarativeCopilot: "true", + MicrosoftEntra: auth === ApiAuthOptions.microsoftEntra().id ? "true" : "", + IsLocalMCP: isLocalMCP ? "true" : "", + ...(isLocalMCP + ? this.processMCPLocalServers(inputs, actionContext) + : MCPForDAServerUrl + ? { + MCPForDAServerUrl, + ServerName: new URL(MCPForDAServerUrl).host + .replace(/[^a-zA-Z0-9]/g, "") + .substring(0, 10), + } + : {}), + }; + const templateName = inputs[QuestionNames.TemplateName]; - return Promise.resolve( - ok([ - { - templateName, - language: language, - replaceMap, - }, - ]) - ); + merge(actionContext?.telemetryProps, { + [telemetryProperties.templateName]: templateName, + [telemetryProperties.isMicrosoftEntra]: + auth === ApiAuthOptions.microsoftEntra().id ? "true" : "", + [telemetryProperties.needAddPluginFromExisting]: + inputs[QuestionNames.ActionType] === ActionStartOptions.existingPlugin().id.toString(), + }); + + return Promise.resolve( + ok([ + { + templateName, + language: language, + replaceMap, + }, + ]) + ); + } catch (error) { + if (error instanceof SystemError) { + return err(error); + } + throw assembleError(error as Error, this.componentName); + } } public override async post( @@ -283,4 +212,92 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { return ok({}); } } + + /** + * Process selected MCP local servers from inputs and format for template + * Handles both single and multiple server selection for backward compatibility + */ + private processMCPLocalServers( + inputs: Inputs, + context?: ActionContext + ): { + MCPLocalServers: Array<{ + name: string; + identifier: string; + command: string; + args: string; + notLast: boolean; + }>; + } { + const selectedOptions = inputs[QuestionNames.MCPLocalServer] as OptionItem[] | undefined; + + // Handle empty/invalid selection + if (!selectedOptions || !Array.isArray(selectedOptions) || selectedOptions.length === 0) { + return { + MCPLocalServers: [], + }; + } + + const failures: string[] = []; + + // Map selected options to server configs + const servers = selectedOptions + .map((option, index) => { + try { + // Validate option structure + if (!option.data || typeof option.data !== "string") { + throw new SystemError( + this.componentName, + "processMCPLocalServers", + "Invalid option data structure" + ); + } + + const serverData = JSON.parse(option.data); + + // Validate parsed data + if (!serverData.identifier || !serverData.command || !Array.isArray(serverData.args)) { + throw new SystemError( + this.componentName, + "processMCPLocalServers", + "Invalid server data format" + ); + } + + return { + name: option.id, + identifier: serverData.identifier, + command: serverData.command, + args: serverData.args.map((arg: string) => `"${arg}"`).join(", "), + notLast: index < selectedOptions.length - 1, + }; + } catch (error) { + // Track failure for telemetry + failures.push(option.id); + return null; + } + }) + .filter((server): server is NonNullable => server !== null); + + // If ALL servers failed, throw error + if (servers.length === 0) { + throw new SystemError( + this.componentName, + "processMCPLocalServers", + `All ${selectedOptions.length} selected MCP servers failed to parse` + ); + } + + // Track failures for telemetry + if (failures.length > 0) { + merge(context?.telemetryProps, { + [telemetryProperties.mcpServerParseFailures]: failures.length.toString(), + [telemetryProperties.mcpServerParseFailureIds]: failures.join(","), + }); + } + + return { + MCPLocalServers: servers, + }; + } } diff --git a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts index abf82a74b40..67ce6ceea15 100644 --- a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts +++ b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts @@ -1487,6 +1487,145 @@ describe("helper", async () => { } }); + it("processMCPLocalServers handles option data malformed gracefully", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); + const inputs: Inputs = { + platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "server-1", + label: "Server 1", + data: { + identifier: "server.1", + command: "odr.exe", + args: ["mcp", "--proxy", "server.1"], + }, + }, + { + id: "server-2", + label: "Server 2", + data: JSON.stringify({ + identifier: "server.2", + command: "odr.exe", + args: ["mcp", "--proxy", "server.2"], + }), + }, + ], + }; + + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); + + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify MCPLocalServers array has one server + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + + // Verify second server + assert.equal(replaceMap.MCPLocalServers[0].name, "server-2"); + assert.equal(replaceMap.MCPLocalServers[0].notLast, false); + } + }); + + it("processMCPLocalServers handles partial option data gracefully", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); + const inputs: Inputs = { + platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "server-1", + label: "Server 1", + data: JSON.stringify({ + identifier: "server.1", + args: ["mcp", "--proxy", "server.1"], + }), + }, + { + id: "server-2", + label: "Server 2", + data: JSON.stringify({ + identifier: "server.2", + command: "odr.exe", + args: ["mcp", "--proxy", "server.2"], + }), + }, + ], + }; + + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); + + assert.isTrue(res); + assert.isTrue(info.isOk()); + + if (info.isOk() && info.value[0].replaceMap) { + const replaceMap = info.value[0].replaceMap; + + // Verify MCPLocalServers array has one server + assert.isDefined(replaceMap.MCPLocalServers); + assert.isArray(replaceMap.MCPLocalServers); + assert.equal(replaceMap.MCPLocalServers.length, 1); + + // Verify second server + assert.equal(replaceMap.MCPLocalServers[0].name, "server-2"); + assert.equal(replaceMap.MCPLocalServers[0].notLast, false); + } + }); + + it("processMCPLocalServers throws when all option data malformed", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); + const inputs: Inputs = { + platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: [ + { + id: "server-1", + label: "Server 1", + data: { + identifier: "server.1", + command: "odr.exe", + args: ["mcp", "--proxy", "server.1"], + }, + }, + ], + }; + + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); + + assert.isTrue(res); + assert.isTrue(info.isErr() && info.error.name === "processMCPLocalServers"); + }); + it("ODRProvider listServers should handle empty output", async () => { sandbox.stub(process, "platform").value("win32"); const execStub = sandbox From 5ca1bc556865b03f0694eb0b7710d2c27aac282b Mon Sep 17 00:00:00 2001 From: Bobby Damore Date: Tue, 23 Dec 2025 22:50:31 -0800 Subject: [PATCH 5/6] test: adjust some test cases and don't swallow errors --- .../generator/declarativeAgent/generator.ts | 103 +++++++----------- .../declarativeAgentGenerator.test.ts | 99 +++++++++-------- 2 files changed, 93 insertions(+), 109 deletions(-) diff --git a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts index e9c5c5f683f..686c483579a 100644 --- a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts +++ b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts @@ -18,13 +18,13 @@ import { Platform, Result, SystemError, + UserError, } from "@microsoft/teamsfx-api"; import fs from "fs-extra"; import { merge } from "lodash"; import path from "path"; import { featureFlagManager, FeatureFlags } from "../../../common/featureFlags"; import { convertToAlphanumericOnly } from "../../../common/stringUtils"; -import { assembleError } from "../../../error"; import { ActionStartOptions, ApiAuthOptions, @@ -47,8 +47,6 @@ const enum telemetryProperties { isDeclarativeCopilot = "is-declarative-copilot", isMicrosoftEntra = "is-microsoft-entra", needAddPluginFromExisting = "need-add-plugin-from-existing", - mcpServerParseFailures = "mcp-server-parse-failures", - mcpServerParseFailureIds = "mcp-server-parse-failure-ids", } /** @@ -103,7 +101,7 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { MicrosoftEntra: auth === ApiAuthOptions.microsoftEntra().id ? "true" : "", IsLocalMCP: isLocalMCP ? "true" : "", ...(isLocalMCP - ? this.processMCPLocalServers(inputs, actionContext) + ? this.processMCPLocalServers(inputs) : MCPForDAServerUrl ? { MCPForDAServerUrl, @@ -133,10 +131,7 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { ]) ); } catch (error) { - if (error instanceof SystemError) { - return err(error); - } - throw assembleError(error as Error, this.componentName); + return err(error); } } @@ -217,10 +212,7 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { * Process selected MCP local servers from inputs and format for template * Handles both single and multiple server selection for backward compatibility */ - private processMCPLocalServers( - inputs: Inputs, - context?: ActionContext - ): { + private processMCPLocalServers(inputs: Inputs): { MCPLocalServers: Array<{ name: string; identifier: string; @@ -232,69 +224,50 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { const selectedOptions = inputs[QuestionNames.MCPLocalServer] as OptionItem[] | undefined; // Handle empty/invalid selection - if (!selectedOptions || !Array.isArray(selectedOptions) || selectedOptions.length === 0) { + if (!selectedOptions || selectedOptions.length === 0) { return { MCPLocalServers: [], }; } - const failures: string[] = []; - // Map selected options to server configs - const servers = selectedOptions - .map((option, index) => { - try { - // Validate option structure - if (!option.data || typeof option.data !== "string") { - throw new SystemError( - this.componentName, - "processMCPLocalServers", - "Invalid option data structure" - ); - } - - const serverData = JSON.parse(option.data); + const servers = selectedOptions.map((option, index) => { + // Validate option structure + if (!option.data || typeof option.data !== "string") { + throw new SystemError( + this.componentName, + "processMCPLocalServers", + "Invalid option data structure" + ); + } - // Validate parsed data - if (!serverData.identifier || !serverData.command || !Array.isArray(serverData.args)) { - throw new SystemError( - this.componentName, - "processMCPLocalServers", - "Invalid server data format" - ); - } + const serverData = JSON.parse(option.data); - return { - name: option.id, - identifier: serverData.identifier, - command: serverData.command, - args: serverData.args.map((arg: string) => `"${arg}"`).join(", "), - notLast: index < selectedOptions.length - 1, - }; - } catch (error) { - // Track failure for telemetry - failures.push(option.id); - return null; - } - }) - .filter((server): server is NonNullable => server !== null); + // Validate parsed data + if (!serverData.identifier || !Array.isArray(serverData.args)) { + throw new SystemError( + this.componentName, + "processMCPLocalServers", + "Invalid server data format" + ); + } - // If ALL servers failed, throw error - if (servers.length === 0) { - throw new SystemError( - this.componentName, - "processMCPLocalServers", - `All ${selectedOptions.length} selected MCP servers failed to parse` - ); - } + if (!serverData.command || typeof serverData.command !== "string") { + throw new UserError( + this.componentName, + "processMCPLocalServers", + "Invalid or missing command in server data" + ); + } - // Track failures for telemetry - if (failures.length > 0) { - merge(context?.telemetryProps, { - [telemetryProperties.mcpServerParseFailures]: failures.length.toString(), - [telemetryProperties.mcpServerParseFailureIds]: failures.join(","), - }); - } + return { + name: option.id, + identifier: serverData.identifier, + command: serverData.command, + args: serverData.args.map((arg: string) => `"${arg}"`).join(", "), + notLast: index < selectedOptions.length - 1, + }; + }); return { MCPLocalServers: servers, diff --git a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts index 67ce6ceea15..b3a6a4e98cd 100644 --- a/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts +++ b/packages/fx-core/tests/component/generator/declarativeAgentGenerator.test.ts @@ -18,6 +18,7 @@ import { signedIn, signedOut, SingleSelectQuestion, + SystemError, UserError, } from "@microsoft/teamsfx-api"; import { assert } from "chai"; @@ -1487,7 +1488,7 @@ describe("helper", async () => { } }); - it("processMCPLocalServers handles option data malformed gracefully", async () => { + it("processMCPLocalServers throws when option data malformed", async () => { const generator = new DeclarativeAgentGenerator(); const context = createContext(); const inputs: Inputs = { @@ -1525,23 +1526,14 @@ describe("helper", async () => { const info = await generator.getTemplateInfos(context, inputs, "."); assert.isTrue(res); - assert.isTrue(info.isOk()); - - if (info.isOk() && info.value[0].replaceMap) { - const replaceMap = info.value[0].replaceMap; - - // Verify MCPLocalServers array has one server - assert.isDefined(replaceMap.MCPLocalServers); - assert.isArray(replaceMap.MCPLocalServers); - assert.equal(replaceMap.MCPLocalServers.length, 1); - - // Verify second server - assert.equal(replaceMap.MCPLocalServers[0].name, "server-2"); - assert.equal(replaceMap.MCPLocalServers[0].notLast, false); - } + assert.isTrue( + info.isErr() && + info.error.name === "processMCPLocalServers" && + info.error instanceof SystemError + ); }); - it("processMCPLocalServers handles partial option data gracefully", async () => { + it("processMCPLocalServers throws UserError when option data missing command", async () => { const generator = new DeclarativeAgentGenerator(); const context = createContext(); const inputs: Inputs = { @@ -1578,23 +1570,48 @@ describe("helper", async () => { const info = await generator.getTemplateInfos(context, inputs, "."); assert.isTrue(res); - assert.isTrue(info.isOk()); + assert.isTrue( + info.isErr() && + info.error.name === "processMCPLocalServers" && + info.error instanceof UserError + ); + }); - if (info.isOk() && info.value[0].replaceMap) { - const replaceMap = info.value[0].replaceMap; + it("processMCPLocalServers throws when MCPLocalServer malformed", async () => { + const generator = new DeclarativeAgentGenerator(); + const context = createContext(); + const inputs: Inputs = { + platform: Platform.CLI, + projectPath: "./", + [QuestionNames.Capabilities]: "api-plugin", + [QuestionNames.ActionType]: CapabilityActionStartOptions.mcp().id, + [QuestionNames.TemplateName]: TemplateNames.DeclarativeAgentWithActionFromMCP, + [QuestionNames.ApiAuth]: ApiAuthOptions.none().id, + [QuestionNames.AppName]: "TestApp", + [QuestionNames.MCPServerType]: "local", + [QuestionNames.MCPLocalServer]: { + id: "server-1", + label: "Server 1", + data: { + identifier: "server.1", + command: "odr.exe", + args: ["mcp", "--proxy", "server.1"], + }, + }, + }; - // Verify MCPLocalServers array has one server - assert.isDefined(replaceMap.MCPLocalServers); - assert.isArray(replaceMap.MCPLocalServers); - assert.equal(replaceMap.MCPLocalServers.length, 1); + const res = await generator.activate(context, inputs); + const info = await generator.getTemplateInfos(context, inputs, "."); - // Verify second server - assert.equal(replaceMap.MCPLocalServers[0].name, "server-2"); - assert.equal(replaceMap.MCPLocalServers[0].notLast, false); - } + assert.isTrue(res); + assert.isTrue( + info.isErr() && + info.error.name === "processMCPLocalServers" && + info.error instanceof SystemError + ); }); - it("processMCPLocalServers throws when all option data malformed", async () => { + it("processMCPLocalServers throws UserError when command malformed", async () => { const generator = new DeclarativeAgentGenerator(); const context = createContext(); const inputs: Inputs = { @@ -1610,11 +1627,11 @@ describe("helper", async () => { { id: "server-1", label: "Server 1", - data: { + data: JSON.stringify({ identifier: "server.1", - command: "odr.exe", + command: 12345, // Malformed command args: ["mcp", "--proxy", "server.1"], - }, + }), }, ], }; @@ -1623,7 +1640,11 @@ describe("helper", async () => { const info = await generator.getTemplateInfos(context, inputs, "."); assert.isTrue(res); - assert.isTrue(info.isErr() && info.error.name === "processMCPLocalServers"); + assert.isTrue( + info.isErr() && + info.error.name === "processMCPLocalServers" && + info.error instanceof UserError + ); }); it("ODRProvider listServers should handle empty output", async () => { @@ -1886,7 +1907,7 @@ describe("helper", async () => { assert.equal(servers[0].tools.length, 0); }); - it("processMCPLocalServers should handle JSON parse errors gracefully", async () => { + it("processMCPLocalServers should throw when option data malformed JSON", async () => { const generator = new DeclarativeAgentGenerator(); const context = createContext(); const inputs: Inputs = { @@ -1920,17 +1941,7 @@ describe("helper", async () => { const info = await generator.getTemplateInfos(context, inputs, "."); assert.isTrue(res); - assert.isTrue(info.isOk()); - - if (info.isOk() && info.value[0].replaceMap) { - const replaceMap = info.value[0].replaceMap; - - // Should only include the valid server, invalid one filtered out - assert.isDefined(replaceMap.MCPLocalServers); - assert.isArray(replaceMap.MCPLocalServers); - assert.equal(replaceMap.MCPLocalServers.length, 1); - assert.equal(replaceMap.MCPLocalServers[0].name, "valid-server"); - } + assert.isTrue(info.isErr()); }); it("declarative agent generator should handle both remote and local MCP server configurations", async () => { From 501def37b0a67d1ca0220869aaf3cca4be0a82d5 Mon Sep 17 00:00:00 2001 From: Bobby Damore Date: Wed, 24 Dec 2025 23:37:48 -0800 Subject: [PATCH 6/6] test: fix failure --- .../src/component/generator/declarativeAgent/generator.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts index 686c483579a..54f678f82bc 100644 --- a/packages/fx-core/src/component/generator/declarativeAgent/generator.ts +++ b/packages/fx-core/src/component/generator/declarativeAgent/generator.ts @@ -223,6 +223,14 @@ export class DeclarativeAgentGenerator extends DefaultTemplateGenerator { } { const selectedOptions = inputs[QuestionNames.MCPLocalServer] as OptionItem[] | undefined; + if (selectedOptions && !Array.isArray(selectedOptions)) { + throw new SystemError( + this.componentName, + "processMCPLocalServers", + "Expected MCPLocalServer input to be an array" + ); + } + // Handle empty/invalid selection if (!selectedOptions || selectedOptions.length === 0) { return {