From 0928296f75b5d6d5be711f6717ebebf6b609aa2b Mon Sep 17 00:00:00 2001 From: Zhiyu You Date: Tue, 7 Jul 2026 18:05:59 +0800 Subject: [PATCH 1/3] fix(fx-core): generate DT add-MCP action inline instead of v4 modify front door DT-on 'add action --api-plugin-type mcp' now writes the URL-only ai-plugin manifest and wires oauth/register directly in core.addPlugin, independent of TEAMSFX_V4_ENABLED. The oauth/register action name and registrationId are derived from the MCP server URL so re-adding the same server is idempotent, and the server is probed for auth metadata when none is provided. Enable TEAMSFX_MCP_FOR_DA_DT by default and fix FeatureFlagManager.listEnabled for default-on flags. Add e2e coverage for MCP no-auth/auth/edge-case flows. --- packages/fx-core/src/common/featureFlags.ts | 4 +- .../src/core/FxCore.declarativeAgent.ts | 178 ++++--- .../src/question/inputs/AddPluginInputs.ts | 8 +- .../src/question/options/AddPluginOptions.ts | 18 +- .../fx-core/tests/common/featureFlags.test.ts | 9 + .../core/FxCore.declarativeAgent.test.ts | 470 +++++++++++++++--- .../DeclarativeAgentMCPAuthEdgeCases.tests.ts | 85 ++-- .../mcp/DeclarativeAgentMCPNoAuth.tests.ts | 99 ++-- .../mcp/DeclarativeAgentMCPWithAuth.tests.ts | 68 ++- .../e2e/declarativeAgent/mcp/mcpTestUtils.ts | 115 +++++ 10 files changed, 793 insertions(+), 261 deletions(-) create mode 100644 packages/tests/src/e2e/declarativeAgent/mcp/mcpTestUtils.ts diff --git a/packages/fx-core/src/common/featureFlags.ts b/packages/fx-core/src/common/featureFlags.ts index 9e88b192328..74ba917066d 100644 --- a/packages/fx-core/src/common/featureFlags.ts +++ b/packages/fx-core/src/common/featureFlags.ts @@ -125,7 +125,7 @@ export class FeatureFlags { }; static readonly MCPForDADT = { name: FeatureFlagName.MCPForDADT, - defaultValue: "false", + defaultValue: "true", }; static readonly MCPForDADCR = { name: FeatureFlagName.MCPForDADCR, @@ -165,7 +165,7 @@ export class FeatureFlagManager { } listEnabled(): string[] { return this.list() - .filter((f) => isFeatureFlagEnabled(f.name)) + .filter((f) => this.getBooleanValue(f)) .map((f) => f.name); } } diff --git a/packages/fx-core/src/core/FxCore.declarativeAgent.ts b/packages/fx-core/src/core/FxCore.declarativeAgent.ts index d5f2b5f62ae..9921765e4af 100644 --- a/packages/fx-core/src/core/FxCore.declarativeAgent.ts +++ b/packages/fx-core/src/core/FxCore.declarativeAgent.ts @@ -34,7 +34,11 @@ import { manifestUtils } from "../component/driver/teamsApp/utils/ManifestUtils" import { pluginManifestUtils } from "../component/driver/teamsApp/utils/PluginManifestUtils"; import { normalizePath } from "../component/driver/teamsApp/utils/utils"; import * as declarativeAgentHelperModule from "../component/generator/declarativeAgent/helper"; -import { deriveMCPServerNameFromUrl } from "../component/generator/declarativeAgent/helper"; +import { + deriveMCPNamespaceFromUrl, + deriveMCPRegistrationIdFromUrl, + deriveMCPServerNameFromUrl, +} from "../component/generator/declarativeAgent/helper"; import { templateDefaultOnActionError } from "../component/generator/generator"; import { GeneratorContext } from "../component/generator/generatorAction"; import * as openApiSpecHelperModule from "../component/generator/openApiSpec/helper"; @@ -55,7 +59,6 @@ import { pathUtils } from "../component/utils/pathUtils"; import { UserCancelError, assembleError } from "../error/common"; import { ActionStartOptions, QuestionNames } from "../question/constants"; import { CreateNewPluginManifestSentinel } from "../question/scaffold/vsc/teamsProjectTypeNode"; -import type { Answers, BuildTarget } from "../v4"; import { ConcurrentLockerMW } from "./middleware/concurrentLocker"; import { ErrorHandlerMW } from "./middleware/errorHandler"; import { modifyProjectFrontDoor } from "./modifyProjectFrontDoor"; @@ -77,19 +80,6 @@ function targetRelativePath(projectPath: string, filePath: string): string { return path.relative(projectPath, absolute).replace(/\\/g, "/"); } -function stringAnswer(answers: Answers, key: string): string | undefined { - const value = answers[key]; - return typeof value === "string" ? value : undefined; -} - -function unsupportedModifyTarget(target: BuildTarget): SystemError { - return new SystemError({ - source: "Scaffold", - name: "UnsupportedModifyEngine", - message: `The add MCP server flow does not handle the '${target.engine}' modify target '${target.templateId}'.`, - }); -} - async function scaffoldAddMcpServerFromV4( options: ScaffoldAddMcpServerFromV4Options ): Promise> { @@ -428,6 +418,8 @@ export class FxCoreDeclarativeAgentPart { const isGenerateFromApiSpec = inputs[QuestionNames.ActionType] === ActionStartOptions.apiSpec().id; const isGenerateFromMCP = inputs[QuestionNames.ActionType] === ActionStartOptions.mcp().id; + const useMCPDTModifyFlow = + isGenerateFromMCP && featureFlagManager.getBooleanValue(FeatureFlags.MCPForDADT); // VS Code MCP "Add Action" flow: aligned with the "DA with MCP" scaffolding // behavior. Only the MCP server URL is collected from the user. The toolkit @@ -441,13 +433,9 @@ export class FxCoreDeclarativeAgentPart { // OAuth registration, since the CLI has no equivalent notification UX. // // Under the Dynamic Tool Discovery (DT) flag this early redirect is skipped - // so that VS Code runs the same inline scaffolder as the CLI — the - // CodeLens "fetch action" hop is absorbed into add-action itself. - if ( - isGenerateFromMCP && - inputs.platform === Platform.VSCode && - !featureFlagManager.getBooleanValue(FeatureFlags.MCPForDADT) - ) { + // so that VS Code runs the same inline scaffolder as the CLI. DT owns the + // generated content shape; it must not depend on the global v4 feature flag. + if (isGenerateFromMCP && inputs.platform === Platform.VSCode && !useMCPDTModifyFlow) { return await this.addPluginFromMCP(inputs, context); } @@ -543,10 +531,7 @@ export class FxCoreDeclarativeAgentPart { // VS Code MCP inline flow (DT on) skips the confirmation modal to match // the leaner `updateActionWithMCP` UX; writes happen as soon as the last // required answer is provided. CLI interactive and other flows still show it. - const skipConfirmModal = - isGenerateFromMCP && - inputs.platform === Platform.VSCode && - featureFlagManager.getBooleanValue(FeatureFlags.MCPForDADT); + const skipConfirmModal = useMCPDTModifyFlow && inputs.platform === Platform.VSCode; if (!skipConfirmModal) { const confirmRes = await context.userInteraction.showMessage( @@ -652,59 +637,106 @@ export class FxCoreDeclarativeAgentPart { ); } - // Dynamic Tool Discovery flow: the v4 modify package renders the dynamic - // plugin manifest and reuses the same MCP auth steps as create. - if (featureFlagManager.getBooleanValue(FeatureFlags.MCPForDADT)) { + // Dynamic Tool Discovery flow: write a URL-only plugin manifest directly + // and reuse the shared MCP auth injector. DT owns the generated content + // and must NOT depend on the v4 modify front door. The runtime carries + // only the MCP server URL (dynamic discovery is the host default) — no + // static tools list, no `mcp_tool_description`, no + // `enable_dynamic_discovery`. + if (useMCPDTModifyFlow) { const authType = inputs[QuestionNames.MCPForDAAuthType] as string | undefined; - const dispatchRes = await fxCoreDeclarativeAgentDeps.modifyProjectFrontDoor( - inputs, - { addCapability: "add-action", actionSource: "mcp" }, - { - mcpServerUrl, - teamsManifestPath: targetRelativePath(projectPath, teamsManifestPath), - authType: authType || "none", - }, - { - scaffoldV4: async ( - _inputs: Inputs, - target: BuildTarget, - answers: Answers, - resolvedPackage?: ResolvedV4ChannelPackage - ) => { - if (target.engine !== "v4") { - return err(unsupportedModifyTarget(target)); - } - const answerMcpServerUrl = stringAnswer(answers, "mcpServerUrl"); - const answerAuthType = stringAnswer(answers, "authType") ?? "none"; - if (answerMcpServerUrl === undefined) { - return err( - new SystemError({ - source: "Scaffold", - name: "ModifyInputMissing", - message: "The add MCP server modify package did not resolve mcpServerUrl.", - }) - ); + // Name the plugin namespace + oauth/register action after the MCP server + // URL (shared verbatim with the create flow via deriveMCPNamespaceFromUrl + // / deriveMCPRegistrationIdFromUrl). This keeps one registration per + // server: re-adding the same server reuses the same `oauth/register` + // (idempotent on registrationId), while a different server gets its own. + const namespace = deriveMCPNamespaceFromUrl(mcpServerUrl); + const registrationId = deriveMCPRegistrationIdFromUrl(mcpServerUrl); + + destinationPluginManifestPath = + await copilotGptManifestUtils.getDefaultNextAvailablePluginManifestPath( + appPackageFolder, + undefined + ); + + const pluginManifest: any = { + $schema: "https://developer.microsoft.com/json-schemas/copilot/plugin/v2.4/schema.json", + schema_version: "v2.4", + name_for_human: manifestRes.value.name?.short ?? path.basename(projectPath), + description_for_human: "Action powered by MCP server", + contact_email: "publisher-email@example.com", + namespace, + functions: [], + runtimes: [ + { + type: "RemoteMCPServer", + spec: { url: mcpServerUrl }, + run_for_functions: ["*"], + auth: deriveMCPManifestOAuth(authType, registrationId) ?? { type: "None" }, + }, + ], + }; + + if (authType && authType !== "none") { + try { + // The add question tree does not collect the auth-server metadata / + // well-known URL, so — mirroring the create flow — probe the MCP + // server to discover it when the caller didn't pass one. The result + // feeds resolveMCPAuthEndpoints so oauth/register + dcr/register get + // real authorization/token/well-known URLs. Only oauth / + // oauth-dynamic consume metadata; entra-sso resolves no endpoints, + // so skip the (10s-timeout) probe for it. Best-effort: a failure + // leaves endpoints empty for the developer to fill in later. + const needsMetadataProbe = authType === "oauth" || authType === "oauth-dynamic"; + if (needsMetadataProbe && !inputs[QuestionNames.MCPForDAAuthMetadataUrl]) { + try { + const { probeMCPServerAuth } = await import("../component/utils/mcpToolFetcher"); + const authProbe = await probeMCPServerAuth(mcpServerUrl); + if (authProbe.authMetadataUrl) { + inputs[QuestionNames.MCPForDAAuthMetadataUrl] = authProbe.authMetadataUrl; + } + } catch { + // best-effort probe; endpoint resolution below tolerates undefined } - return fxCoreDeclarativeAgentDeps.scaffoldAddMcpServerFromV4({ - templateId: target.templateId, - projectPath, - platform: inputs.platform, - teamsManifestPath, - appName: manifestRes.value.name?.short ?? path.basename(projectPath), - mcpServerUrl: answerMcpServerUrl, - authType: answerAuthType, - resolvedPackage, + } + const endpoints = await resolveMCPAuthEndpoints(authType, inputs); + const ymlPath = pathUtils.getYmlFilePath(inputs.projectPath); + if (ymlPath) { + await injectMCPAuthActionToYml({ + ymlPath, + authType, + authName: namespace, + registrationId, + mcpServerUrl, + endpoints, }); - }, - callCoreMethod: (_inputs: Inputs, target: BuildTarget) => - Promise.resolve(err(unsupportedModifyTarget(target))), - resolveArtifactSnapshot: fxCoreDeclarativeAgentDeps.resolveV4TemplateArtifactSnapshot, + } + } catch (error: any) { + mcpWarnings.push({ + type: "mcpAuthMetadataError", + content: getLocalizedString( + "core.MCPForDA.mcpAuthMetadataMissingError", + error.message + ), + }); } + } + + await fs.ensureFile(destinationPluginManifestPath); + await fs.writeJSON(destinationPluginManifestPath, pluginManifest, { spaces: 4 }); + + const addActionRes = await copilotGptManifestUtils.addAction( + declarativeCopilotManifestPath, + actionId, + normalizePath(path.relative(appPackageFolder, destinationPluginManifestPath), true) ); - if (dispatchRes.isErr()) { - return err(dispatchRes.error); + if (addActionRes.isErr()) { + return err(addActionRes.error); + } + + for (const warning of mcpWarnings) { + context.logProvider.warning(warning.content); } - return ok(undefined); } else { // Load tools from file if provided and not yet loaded const existingTools = inputs[QuestionNames.MCPForDAAvailableTools]; diff --git a/packages/fx-core/src/question/inputs/AddPluginInputs.ts b/packages/fx-core/src/question/inputs/AddPluginInputs.ts index 54c9d103c4f..9c84b6d3cd4 100644 --- a/packages/fx-core/src/question/inputs/AddPluginInputs.ts +++ b/packages/fx-core/src/question/inputs/AddPluginInputs.ts @@ -28,7 +28,13 @@ export interface AddPluginInputs extends Inputs { /** @description MCP Tools Definition File */ "mcp-tools-file-path"?: string; /** @description Select Authentication Type */ - "mcp-da-auth-type"?: "oauth" | "entra-sso" | "none"; + "mcp-da-auth-type"?: "oauth" | "oauth-dynamic" | "entra-sso" | "none"; + /** @description OAuth Client ID */ + "mcp-da-client-id"?: string; + /** @description OAuth Client Secret */ + "mcp-da-client-secret"?: string; + /** @description OAuth Scopes (optional) */ + "mcp-da-scopes"?: string; /** @description Select Teams manifest.json File */ "manifest-path"?: string; } diff --git a/packages/fx-core/src/question/options/AddPluginOptions.ts b/packages/fx-core/src/question/options/AddPluginOptions.ts index 6950c383ff9..eaae62d91c2 100644 --- a/packages/fx-core/src/question/options/AddPluginOptions.ts +++ b/packages/fx-core/src/question/options/AddPluginOptions.ts @@ -62,7 +62,23 @@ export const AddPluginOptions: CLICommandOption[] = [ type: "string", description: "Select Authentication Type", default: "oauth", - choices: ["oauth", "entra-sso", "none"], + choices: ["oauth", "oauth-dynamic", "entra-sso", "none"], + }, + { + name: "mcp-da-client-id", + type: "string", + description: "OAuth Client ID", + }, + { + name: "mcp-da-client-secret", + type: "string", + description: "OAuth Client Secret", + }, + { + name: "mcp-da-scopes", + type: "string", + description: "OAuth Scopes (optional)", + required: false, }, { name: "manifest-file", diff --git a/packages/fx-core/tests/common/featureFlags.test.ts b/packages/fx-core/tests/common/featureFlags.test.ts index 93ce05cc12d..419eb86b090 100644 --- a/packages/fx-core/tests/common/featureFlags.test.ts +++ b/packages/fx-core/tests/common/featureFlags.test.ts @@ -38,6 +38,13 @@ describe("FeatureFlagManager", () => { const stringRes = featureFlagManager.getStringValue(FeatureFlags.MCPForDADCR); chai.assert.equal(stringRes, "true"); }); + it("MCPForDADT defaults to true", async () => { + mockedEnvRestore = mockedEnv({ [FeatureFlags.MCPForDADT.name]: undefined }); + const booleanRes = featureFlagManager.getBooleanValue(FeatureFlags.MCPForDADT); + chai.assert.isTrue(booleanRes); + const stringRes = featureFlagManager.getStringValue(FeatureFlags.MCPForDADT); + chai.assert.equal(stringRes, "true"); + }); it("MCPForDADCR can be disabled by environment variable", async () => { mockedEnvRestore = mockedEnv({ TEAMSFX_MCP_FOR_DA_DCR: "false" }); const booleanRes = featureFlagManager.getBooleanValue(FeatureFlags.MCPForDADCR); @@ -61,5 +68,7 @@ describe("FeatureFlagManager", () => { const list = featureFlagManager.listEnabled(); chai.assert.include(list, "TEAMSFX_CLI_DOTNET"); chai.assert.include(list, "SME_OAUTH"); + chai.assert.include(list, FeatureFlags.MCPForDADT.name); + chai.assert.include(list, FeatureFlags.MCPForDADCR.name); }); }); diff --git a/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts b/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts index 82e6552f541..d1d7e420000 100644 --- a/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts +++ b/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts @@ -2484,6 +2484,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -2544,6 +2545,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -2602,6 +2604,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -2678,6 +2681,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -2720,7 +2724,7 @@ describe("addPlugin", async () => { } }); - it("from MCP (DT flag on): scaffolds the v4 add-mcp-server modify package", async () => { + it("from MCP (DT flag on): writes a URL-only plugin and injects oauth without the modify front door", async () => { const appName = await mockV3Project(); const projectPath = path.join(os.tmpdir(), appName); const inputs: Inputs = { @@ -2762,18 +2766,11 @@ describe("addPlugin", async () => { ok({} as DeclarativeCopilotManifestSchema) ); const declarativeAgentModule = await import("../../src/core/FxCore.declarativeAgent"); - const scaffoldV4Stub = vi - .spyOn(declarativeAgentModule.fxCoreDeclarativeAgentDeps, "scaffoldAddMcpServerFromV4") - .mockResolvedValue(ok(undefined)); - const modifyFrontDoorStub = vi - .spyOn(declarativeAgentModule.fxCoreDeclarativeAgentDeps, "modifyProjectFrontDoor") - .mockImplementation(async (_inputs, selectorPrefill, entryParams, deps) => { - return deps.scaffoldV4( - inputs, - { templateId: "add-mcp-server", engine: "v4", answers: selectorPrefill }, - entryParams - ); - }); + // DT add mcp must NOT route through the v4 modify front door. + const modifyFrontDoorStub = vi.spyOn( + declarativeAgentModule.fxCoreDeclarativeAgentDeps, + "modifyProjectFrontDoor" + ); vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { if (level === "warn") return Promise.resolve(ok("Add")); @@ -2796,12 +2793,6 @@ describe("addPlugin", async () => { .spyOn(actionInjectorModule.ActionInjector, "injectCreateOAuthActionForMCP") .mockResolvedValue(); - const envUtilModule = await import("../../src/component/utils/envUtil"); - vi.spyOn(envUtilModule.envUtil, "listEnv").mockResolvedValue(ok(["dev"])); - const writeEnvStub = vi - .spyOn(envUtilModule.envUtil, "writeEnv") - .mockResolvedValue(ok(undefined)); - vi.spyOn(pathUtils, "getYmlFilePath").mockReturnValue("m365agents.yml"); vi.spyOn(fs, "ensureFile").mockResolvedValue(); const writeJSONStub = vi.spyOn(fs, "writeJSON").mockResolvedValue(); @@ -2810,35 +2801,38 @@ describe("addPlugin", async () => { const result = await core.addPlugin(inputs); assert.isTrue(result.isOk()); - assert.isTrue(modifyFrontDoorStub.mock.calls.length === 1); - assert.deepEqual(modifyFrontDoorStub.mock.calls[0][1], { - addCapability: "add-action", - actionSource: "mcp", - }); - assert.deepInclude(modifyFrontDoorStub.mock.calls[0][2], { - mcpServerUrl: "https://example.com/mcp", - teamsManifestPath: "manifest.json", - authType: "oauth", - }); - assert.isTrue(scaffoldV4Stub.mock.calls.length === 1); - assert.deepInclude(scaffoldV4Stub.mock.calls[0][0], { - templateId: "add-mcp-server", - projectPath, - teamsManifestPath: "manifest.json", - appName: "My MCP App", - mcpServerUrl: "https://example.com/mcp", - authType: "oauth", + assert.equal(modifyFrontDoorStub.mock.calls.length, 0); + + // The rendered plugin is URL-only: no static tools file, no + // `enable_dynamic_discovery` flag. + const pluginCall = writeJSONStub.mock.calls.find((c) => String(c[0]).includes("ai-plugin")); + assert.isDefined(pluginCall); + const pluginManifest = pluginCall![1] as any; + assert.deepEqual(pluginManifest.functions, []); + const runtime = pluginManifest.runtimes[0]; + assert.equal(runtime.type, "RemoteMCPServer"); + assert.deepEqual(runtime.spec, { url: "https://example.com/mcp" }); + assert.notProperty(runtime.spec, "enable_dynamic_discovery"); + assert.notProperty(runtime.spec, "mcp_tool_description"); + assert.deepEqual(runtime.run_for_functions, ["*"]); + assert.equal(pluginManifest.namespace, "examplecom"); + assert.deepEqual(runtime.auth, { + type: "OAuthPluginVault", + reference_id: "${{MCP_DA_AUTH_ID_EXAMPLECOM}}", }); - assert.isTrue(injectStub.mock.calls.length === 0); - assert.isTrue(writeEnvStub.mock.calls.length === 0); - assert.isTrue(writeJSONStub.mock.calls.length === 0); + + // oauth/register is injected via the shared, complete injector, named after + // the URL-derived namespace (shared per server) — not the action id. + assert.equal(injectStub.mock.calls.length, 1); + assert.equal(injectStub.mock.calls[0][2], "examplecom"); + assert.equal(injectStub.mock.calls[0][3], "MCP_DA_AUTH_ID_EXAMPLECOM"); if (await fs.pathExists(projectPath)) { await fs.remove(projectPath); } }); - it("from MCP (DT flag on): none auth type writes None auth without injection", async () => { + it("from MCP (DT flag on): none auth writes None auth and skips oauth injection", async () => { const appName = await mockV3Project(); const projectPath = path.join(os.tmpdir(), appName); const inputs: Inputs = { @@ -2874,18 +2868,10 @@ describe("addPlugin", async () => { ok({} as DeclarativeCopilotManifestSchema) ); const declarativeAgentModule = await import("../../src/core/FxCore.declarativeAgent"); - const scaffoldV4Stub = vi - .spyOn(declarativeAgentModule.fxCoreDeclarativeAgentDeps, "scaffoldAddMcpServerFromV4") - .mockResolvedValue(ok(undefined)); - const modifyFrontDoorStub = vi - .spyOn(declarativeAgentModule.fxCoreDeclarativeAgentDeps, "modifyProjectFrontDoor") - .mockImplementation(async (_inputs, selectorPrefill, entryParams, deps) => { - return deps.scaffoldV4( - inputs, - { templateId: "add-mcp-server", engine: "v4", answers: selectorPrefill }, - entryParams - ); - }); + const modifyFrontDoorStub = vi.spyOn( + declarativeAgentModule.fxCoreDeclarativeAgentDeps, + "modifyProjectFrontDoor" + ); vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { if (level === "warn") return Promise.resolve(ok("Add")); @@ -2904,16 +2890,362 @@ describe("addPlugin", async () => { const result = await core.addPlugin(inputs); assert.isTrue(result.isOk()); - assert.isTrue(injectStub.mock.calls.length === 0); - assert.isTrue(modifyFrontDoorStub.mock.calls.length === 1); - assert.isTrue(scaffoldV4Stub.mock.calls.length === 1); - assert.deepInclude(scaffoldV4Stub.mock.calls[0][0], { - templateId: "add-mcp-server", + assert.equal(modifyFrontDoorStub.mock.calls.length, 0); + assert.equal(injectStub.mock.calls.length, 0); + + const pluginCall = writeJSONStub.mock.calls.find((c) => String(c[0]).includes("ai-plugin")); + assert.isDefined(pluginCall); + const runtime = (pluginCall![1] as any).runtimes[0]; + assert.equal(runtime.type, "RemoteMCPServer"); + assert.deepEqual(runtime.spec, { url: "https://example.com/mcp" }); + assert.notProperty(runtime.spec, "enable_dynamic_discovery"); + assert.deepEqual(runtime.auth, { type: "None" }); + + if (await fs.pathExists(projectPath)) { + await fs.remove(projectPath); + } + }); + + it("from MCP (DT flag on): entra-sso routes through the shared oauth injector", async () => { + const appName = await mockV3Project(); + const projectPath = path.join(os.tmpdir(), appName); + const inputs: Inputs = { + platform: Platform.CLI, + [QuestionNames.Folder]: os.tmpdir(), + [QuestionNames.TeamsAppManifestFilePath]: "manifest.json", + [QuestionNames.ActionType]: ActionStartOptions.mcp().id, + [QuestionNames.MCPForDAServerUrl]: "https://example.com/mcp", + [QuestionNames.MCPToolsFilePath]: "", + [QuestionNames.MCPForDAAuthType]: "entra-sso", + [QuestionNames.MCPForDAClientId]: "entra-client-id", projectPath, - mcpServerUrl: "https://example.com/mcp", - authType: "none", + }; + + const manifest = new TeamsAppManifest(); + manifest.copilotExtensions = { + declarativeCopilots: [{ file: "test1.json", id: "action_1" }], + }; + + vi.spyOn(featureFlagManager, "getBooleanValue").mockImplementation((flag) => { + return flag === FeatureFlags.MCPForDADT; + }); + vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); + vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); + vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); + vi.spyOn(copilotGptManifestUtils, "readCopilotGptManifestFile").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + vi.spyOn( + copilotGptManifestUtils, + "getDefaultNextAvailablePluginManifestPath" + ).mockResolvedValue("ai-plugin_1.json"); + vi.spyOn(copilotGptManifestUtils, "addAction").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + const declarativeAgentModule = await import("../../src/core/FxCore.declarativeAgent"); + const modifyFrontDoorStub = vi.spyOn( + declarativeAgentModule.fxCoreDeclarativeAgentDeps, + "modifyProjectFrontDoor" + ); + + vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { + if (level === "warn") return Promise.resolve(ok("Add")); + return Promise.resolve(ok("")); + }); + + const actionInjectorModule = await import("../../src/component/configManager/actionInjector"); + const oauthInjectStub = vi + .spyOn(actionInjectorModule.ActionInjector, "injectCreateOAuthActionForMCP") + .mockResolvedValue(); + const dcrInjectStub = vi + .spyOn(actionInjectorModule.ActionInjector, "injectCreateDcrActionForMCP") + .mockResolvedValue(); + + vi.spyOn(pathUtils, "getYmlFilePath").mockReturnValue("m365agents.yml"); + vi.spyOn(fs, "ensureFile").mockResolvedValue(); + const writeJSONStub = vi.spyOn(fs, "writeJSON").mockResolvedValue(); + + const core = new FxCore(addPluginTools); + const result = await core.addPlugin(inputs); + + assert.isTrue(result.isOk()); + assert.equal(modifyFrontDoorStub.mock.calls.length, 0); + + // entra-sso reuses the OAuth injector (which stamps identityProvider = + // MicrosoftEntra internally) — not the DCR injector. + assert.equal(oauthInjectStub.mock.calls.length, 1); + assert.equal(oauthInjectStub.mock.calls[0][1], "entra-sso"); + assert.equal(oauthInjectStub.mock.calls[0][2], "examplecom"); + assert.equal(dcrInjectStub.mock.calls.length, 0); + + const pluginCall = writeJSONStub.mock.calls.find((c) => String(c[0]).includes("ai-plugin")); + assert.isDefined(pluginCall); + const pluginManifest = pluginCall![1] as any; + assert.equal(pluginManifest.namespace, "examplecom"); + const runtime = pluginManifest.runtimes[0]; + assert.deepEqual(runtime.spec, { url: "https://example.com/mcp" }); + assert.deepEqual(runtime.auth, { + type: "OAuthPluginVault", + reference_id: "${{MCP_DA_AUTH_ID_EXAMPLECOM}}", }); - assert.isTrue(writeJSONStub.mock.calls.length === 0); + + if (await fs.pathExists(projectPath)) { + await fs.remove(projectPath); + } + }); + + it("from MCP (DT flag on): oauth-dynamic routes through the shared DCR injector", async () => { + const appName = await mockV3Project(); + const projectPath = path.join(os.tmpdir(), appName); + const inputs: Inputs = { + platform: Platform.CLI, + [QuestionNames.Folder]: os.tmpdir(), + [QuestionNames.TeamsAppManifestFilePath]: "manifest.json", + [QuestionNames.ActionType]: ActionStartOptions.mcp().id, + [QuestionNames.MCPForDAServerUrl]: "https://example.com/mcp", + [QuestionNames.MCPToolsFilePath]: "", + [QuestionNames.MCPForDAAuthType]: "oauth-dynamic", + [QuestionNames.MCPForDAAuthWellKnownUrl]: + "https://example.com/.well-known/oauth-authorization-server", + projectPath, + }; + + const manifest = new TeamsAppManifest(); + manifest.copilotExtensions = { + declarativeCopilots: [{ file: "test1.json", id: "action_1" }], + }; + + vi.spyOn(featureFlagManager, "getBooleanValue").mockImplementation((flag) => { + return flag === FeatureFlags.MCPForDADT; + }); + vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); + vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); + vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); + vi.spyOn(copilotGptManifestUtils, "readCopilotGptManifestFile").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + vi.spyOn( + copilotGptManifestUtils, + "getDefaultNextAvailablePluginManifestPath" + ).mockResolvedValue("ai-plugin_1.json"); + vi.spyOn(copilotGptManifestUtils, "addAction").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + const declarativeAgentModule = await import("../../src/core/FxCore.declarativeAgent"); + const modifyFrontDoorStub = vi.spyOn( + declarativeAgentModule.fxCoreDeclarativeAgentDeps, + "modifyProjectFrontDoor" + ); + + vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { + if (level === "warn") return Promise.resolve(ok("Add")); + return Promise.resolve(ok("")); + }); + + const mcpAuthScaffolderModule = await import("../../src/component/utils/mcpAuthScaffolder"); + vi.spyOn( + mcpAuthScaffolderModule.mcpAuthScaffolderDeps, + "resolveMCPOAuthMetadata" + ).mockResolvedValue({ + authorizationUrl: "https://example.com/oauth/authorize", + tokenUrl: "https://example.com/oauth/token", + refreshUrl: "https://example.com/oauth/token", + wellKnownUrl: "https://example.com/.well-known/oauth-authorization-server", + }); + + const actionInjectorModule = await import("../../src/component/configManager/actionInjector"); + const oauthInjectStub = vi + .spyOn(actionInjectorModule.ActionInjector, "injectCreateOAuthActionForMCP") + .mockResolvedValue(); + const dcrInjectStub = vi + .spyOn(actionInjectorModule.ActionInjector, "injectCreateDcrActionForMCP") + .mockResolvedValue(); + + vi.spyOn(pathUtils, "getYmlFilePath").mockReturnValue("m365agents.yml"); + vi.spyOn(fs, "ensureFile").mockResolvedValue(); + const writeJSONStub = vi.spyOn(fs, "writeJSON").mockResolvedValue(); + + const core = new FxCore(addPluginTools); + const result = await core.addPlugin(inputs); + + assert.isTrue(result.isOk()); + assert.equal(modifyFrontDoorStub.mock.calls.length, 0); + + // oauth-dynamic reuses the DCR injector (dcr/register) — not the OAuth one. + assert.equal(dcrInjectStub.mock.calls.length, 1); + assert.equal(dcrInjectStub.mock.calls[0][1], "examplecom"); + assert.equal(dcrInjectStub.mock.calls[0][2], "MCP_DA_AUTH_ID_EXAMPLECOM"); + assert.equal(oauthInjectStub.mock.calls.length, 0); + + const pluginCall = writeJSONStub.mock.calls.find((c) => String(c[0]).includes("ai-plugin")); + assert.isDefined(pluginCall); + const pluginManifest = pluginCall![1] as any; + assert.equal(pluginManifest.namespace, "examplecom"); + const runtime = pluginManifest.runtimes[0]; + assert.deepEqual(runtime.spec, { url: "https://example.com/mcp" }); + assert.deepEqual(runtime.auth, { + type: "OAuthPluginVault", + reference_id: "${{MCP_DA_AUTH_ID_EXAMPLECOM}}", + }); + + if (await fs.pathExists(projectPath)) { + await fs.remove(projectPath); + } + }); + + it("from MCP (DT flag on): probes the MCP server for auth metadata when none is provided", async () => { + const appName = await mockV3Project(); + const projectPath = path.join(os.tmpdir(), appName); + const inputs: Inputs = { + platform: Platform.CLI, + [QuestionNames.Folder]: os.tmpdir(), + [QuestionNames.TeamsAppManifestFilePath]: "manifest.json", + [QuestionNames.ActionType]: ActionStartOptions.mcp().id, + [QuestionNames.MCPForDAServerUrl]: "https://example.com/mcp", + [QuestionNames.MCPToolsFilePath]: "", + [QuestionNames.MCPForDAAuthType]: "oauth", + [QuestionNames.MCPForDAClientId]: "client-id", + [QuestionNames.MCPForDAClientSecret]: "client-secret", + [QuestionNames.MCPForDAScopes]: "scope-a", + // MCPForDAAuthMetadataUrl intentionally absent — the add question tree does + // not collect it, so the server must be probed to discover it. + projectPath, + }; + + const manifest = new TeamsAppManifest(); + manifest.copilotExtensions = { + declarativeCopilots: [{ file: "test1.json", id: "action_1" }], + }; + + vi.spyOn(featureFlagManager, "getBooleanValue").mockImplementation((flag) => { + return flag === FeatureFlags.MCPForDADT; + }); + vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); + vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); + vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); + vi.spyOn(copilotGptManifestUtils, "readCopilotGptManifestFile").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + vi.spyOn( + copilotGptManifestUtils, + "getDefaultNextAvailablePluginManifestPath" + ).mockResolvedValue("ai-plugin_1.json"); + vi.spyOn(copilotGptManifestUtils, "addAction").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + + vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { + if (level === "warn") return Promise.resolve(ok("Add")); + return Promise.resolve(ok("")); + }); + + const mcpToolFetcherModule = await import("../../src/component/utils/mcpToolFetcher"); + const probeStub = vi.spyOn(mcpToolFetcherModule, "probeMCPServerAuth").mockResolvedValue({ + requiresAuth: true, + authMetadataUrl: "https://example.com/.well-known/oauth-protected-resource", + }); + + const mcpAuthScaffolderModule = await import("../../src/component/utils/mcpAuthScaffolder"); + const resolveMetadataStub = vi + .spyOn(mcpAuthScaffolderModule.mcpAuthScaffolderDeps, "resolveMCPOAuthMetadata") + .mockResolvedValue({ + authorizationUrl: "https://example.com/oauth/authorize", + tokenUrl: "https://example.com/oauth/token", + refreshUrl: "https://example.com/oauth/token", + wellKnownUrl: "https://example.com/.well-known/oauth-authorization-server", + }); + + const actionInjectorModule = await import("../../src/component/configManager/actionInjector"); + vi.spyOn( + actionInjectorModule.ActionInjector, + "injectCreateOAuthActionForMCP" + ).mockResolvedValue(); + + vi.spyOn(pathUtils, "getYmlFilePath").mockReturnValue("m365agents.yml"); + vi.spyOn(fs, "ensureFile").mockResolvedValue(); + vi.spyOn(fs, "writeJSON").mockResolvedValue(); + + const core = new FxCore(addPluginTools); + const result = await core.addPlugin(inputs); + + assert.isTrue(result.isOk()); + // No metadata URL was provided, so the server is probed and the discovered + // URL flows into endpoint resolution. + assert.equal(probeStub.mock.calls.length, 1); + assert.equal(probeStub.mock.calls[0][0], "https://example.com/mcp"); + assert.equal(resolveMetadataStub.mock.calls.length, 1); + assert.equal( + resolveMetadataStub.mock.calls[0][0], + "https://example.com/.well-known/oauth-protected-resource" + ); + + if (await fs.pathExists(projectPath)) { + await fs.remove(projectPath); + } + }); + + it("from MCP (DT off): uses legacy static add-action path", async () => { + const appName = await mockV3Project(); + const projectPath = path.join(os.tmpdir(), appName); + const inputs: Inputs = { + platform: Platform.CLI, + [QuestionNames.Folder]: os.tmpdir(), + [QuestionNames.TeamsAppManifestFilePath]: "manifest.json", + [QuestionNames.ActionType]: ActionStartOptions.mcp().id, + [QuestionNames.MCPForDAServerUrl]: "https://example.com/mcp", + [QuestionNames.MCPToolsFilePath]: "", + [QuestionNames.MCPForDAAvailableTools]: [{ name: "tool1", description: "Tool 1" }], + [QuestionNames.MCPForDAPreFetchTools]: ["tool1"], + [QuestionNames.MCPForDAAuth]: "NoneAuth", + [QuestionNames.MCPForDAAuthType]: "none", + projectPath, + }; + + const manifest = new TeamsAppManifest(); + manifest.copilotExtensions = { + declarativeCopilots: [{ file: "test1.json", id: "action_1" }], + }; + + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); + vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); + vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); + vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); + vi.spyOn(copilotGptManifestUtils, "readCopilotGptManifestFile").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + vi.spyOn( + copilotGptManifestUtils, + "getDefaultNextAvailablePluginManifestPath" + ).mockResolvedValue("ai-plugin_1.json"); + vi.spyOn(copilotGptManifestUtils, "addAction").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + const declarativeAgentModule = await import("../../src/core/FxCore.declarativeAgent"); + const modifyFrontDoorStub = vi + .spyOn(declarativeAgentModule.fxCoreDeclarativeAgentDeps, "modifyProjectFrontDoor") + .mockRejectedValue(new Error("modifyProjectFrontDoor should not run when DT is off")); + + vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { + if (level === "warn") return Promise.resolve(ok("Add")); + return Promise.resolve(ok("")); + }); + + vi.spyOn(fs, "ensureFile").mockResolvedValue(); + const writeJSONStub = vi.spyOn(fs, "writeJSON").mockResolvedValue(); + + const core = new FxCore(addPluginTools); + const result = await core.addPlugin(inputs); + + assert.isTrue(result.isOk()); + assert.isTrue(modifyFrontDoorStub.mock.calls.length === 0); + const pluginCall = writeJSONStub.mock.calls.find((call) => { + const content = call[1] as any; + return Array.isArray(content?.functions) && Array.isArray(content?.runtimes); + }); + assert.isDefined(pluginCall); + assert.deepEqual((pluginCall?.[1] as any).functions, [ + { name: "tool1", description: "Tool 1" }, + ]); if (await fs.pathExists(projectPath)) { await fs.remove(projectPath); @@ -2941,6 +3273,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -2988,6 +3321,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3041,6 +3375,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3091,6 +3426,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3138,6 +3474,7 @@ describe("addPlugin", async () => { projectPath, }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); const realEnsureDir = fs.ensureDir.bind(fs); const ensureDirStub = vi.spyOn(fs, "ensureDir").mockImplementation(async (p: any) => { @@ -3190,6 +3527,7 @@ describe("addPlugin", async () => { projectPath, }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(addPluginTools.ui, "showMessage"); @@ -3218,6 +3556,7 @@ describe("addPlugin", async () => { projectPath, }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(fs, "pathExists").mockResolvedValue(true); vi.spyOn(fs, "readJSON").mockResolvedValue({ @@ -3279,6 +3618,7 @@ describe("addPlugin", async () => { projectPath, }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(fs, "pathExists").mockResolvedValue(true); vi.spyOn(fs, "readJSON").mockResolvedValue({ @@ -3331,6 +3671,7 @@ describe("addPlugin", async () => { projectPath, }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(fs, "pathExists").mockResolvedValue(true); vi.spyOn(fs, "readJSON").mockRejectedValue(new Error("invalid JSON")); @@ -3378,6 +3719,7 @@ describe("addPlugin", async () => { projectPath, }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(fs, "pathExists").mockResolvedValue(true); // Existing file is valid JSON but missing the `servers` field. @@ -3430,6 +3772,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3481,6 +3824,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3529,6 +3873,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3576,6 +3921,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3639,6 +3985,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); @@ -3702,6 +4049,7 @@ describe("addPlugin", async () => { declarativeCopilots: [{ file: "test1.json", id: "action_1" }], }; + vi.spyOn(featureFlagManager, "getBooleanValue").mockReturnValue(false); vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); diff --git a/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPAuthEdgeCases.tests.ts b/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPAuthEdgeCases.tests.ts index d9c6d53089e..a1f4ef940d0 100644 --- a/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPAuthEdgeCases.tests.ts +++ b/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPAuthEdgeCases.tests.ts @@ -14,58 +14,71 @@ import { execAsync } from "../../../utils/commonUtils"; import { Capability } from "../../../utils/constants"; import { CaseFactory } from "../../caseFactory"; import { getTestFolder, getUniqueAppName } from "../../commonUtils"; +import { + createMCPProjectWithEnv, + expectDynamicMCPProject, + expectNoOAuthRegister, + learnMCPServerUrl, + mcpDynamicFlowEnv, +} from "./mcpTestUtils"; // Case 2 & 5: With learn.microsoft.com/api/mcp (a public no-auth server that // returns tools), these cases verify the server-URL-only flow produces a valid // scaffold with tools and no auth block — even when no --mcp-da-auth-type is given. class DeclarativeAgentMCPServerUrlOnly extends CaseFactory { + public override async onCreate( + appName: string, + testFolder: string, + capability: Capability, + programmingLanguage?: ProgrammingLanguage, + custimized?: Record, + ): Promise { + await createMCPProjectWithEnv( + testFolder, + appName, + capability, + programmingLanguage, + custimized, + mcpDynamicFlowEnv, + ); + } + public override async onAfter(projectPath: string): Promise { await fs.remove(projectPath); } public override async onAfterCreate(projectPath: string): Promise { - const appPackage = path.join(projectPath, "appPackage"); - - // ai-plugin.json should have functions and runtime from auto-fetch - const aiPlugin = await fs.readJSON(path.join(appPackage, "ai-plugin.json")); - expect(aiPlugin.functions).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes[0].type).to.equal("RemoteMCPServer"); - // No auth block since server doesn't require auth - expect(aiPlugin.runtimes[0].auth).to.be.undefined; - - // mcp-tools-1.json should exist with fetched tools - const mcpToolsPath = path.join(appPackage, "mcp-tools-1.json"); - expect(fs.pathExistsSync(mcpToolsPath)).to.be.true; - const mcpTools = await fs.readJSON(mcpToolsPath); - expect(mcpTools.tools).to.be.an("array").that.is.not.empty; - - // No oauth/register in yml - const ymlPath = path.join(projectPath, "m365agents.yml"); - if (fs.pathExistsSync(ymlPath)) { - const ymlContent = fs.readFileSync(ymlPath, "utf8"); - expect(ymlContent).to.not.include("oauth/register"); - } + await expectDynamicMCPProject(projectPath); + expectNoOAuthRegister(projectPath); } } // Case 7: --mcp-da-auth-type omitted with a no-auth server — project should // succeed because auth probe detects no auth requirement. class DeclarativeAgentMCPNoAuthTypeNeeded extends CaseFactory { + public override async onCreate( + appName: string, + testFolder: string, + capability: Capability, + programmingLanguage?: ProgrammingLanguage, + custimized?: Record, + ): Promise { + await createMCPProjectWithEnv( + testFolder, + appName, + capability, + programmingLanguage, + custimized, + mcpDynamicFlowEnv, + ); + } + public override async onAfter(projectPath: string): Promise { await fs.remove(projectPath); } public override async onAfterCreate(projectPath: string): Promise { - // Project should be created successfully with no auth blocks - const appPackage = path.join(projectPath, "appPackage"); - const aiPluginPath = path.join(appPackage, "ai-plugin.json"); - expect(fs.pathExistsSync(aiPluginPath)).to.be.true; - const aiPlugin = await fs.readJSON(aiPluginPath); - expect(aiPlugin.functions).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes).to.be.an("array").that.is.not.empty; - // No auth — server doesn't require it - expect(aiPlugin.runtimes[0].auth).to.be.undefined; + await expectDynamicMCPProject(projectPath); } } @@ -103,7 +116,10 @@ class DeclarativeAgentMCPMissingServerUrl extends CaseFactory { try { console.log(`[Start] "${command}" in ${testFolder}.`); - await execAsync(command, { cwd: testFolder, env: process.env }); + await execAsync(command, { + cwd: testFolder, + env: { ...process.env, ...mcpDynamicFlowEnv }, + }); expect.fail("Expected MCP scaffold without mcpServerUrl to fail."); } catch (error) { const message = @@ -123,8 +139,7 @@ class DeclarativeAgentMCPMissingServerUrl extends CaseFactory { const serverUrlOnlyRecord: Record = {}; serverUrlOnlyRecord["with-plugin"] = "yes"; serverUrlOnlyRecord["api-plugin-type"] = "mcp"; -serverUrlOnlyRecord["mcp-da-server-url"] = - "https://learn.microsoft.com/api/mcp"; +serverUrlOnlyRecord["mcp-da-server-url"] = learnMCPServerUrl; new DeclarativeAgentMCPServerUrlOnly( Capability.DeclarativeAgent, @@ -142,7 +157,7 @@ new DeclarativeAgentMCPServerUrlOnly( const noAuthTypeRecord: Record = {}; noAuthTypeRecord["with-plugin"] = "yes"; noAuthTypeRecord["api-plugin-type"] = "mcp"; -noAuthTypeRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +noAuthTypeRecord["mcp-da-server-url"] = learnMCPServerUrl; // Intentionally omit mcp-da-auth-type new DeclarativeAgentMCPNoAuthTypeNeeded( diff --git a/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPNoAuth.tests.ts b/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPNoAuth.tests.ts index 0cd7fd33283..8259fd5a34c 100644 --- a/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPNoAuth.tests.ts +++ b/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPNoAuth.tests.ts @@ -6,9 +6,7 @@ */ import { ProgrammingLanguage } from "@microsoft/teamsfx-core"; -import { expect } from "chai"; import * as fs from "fs-extra"; -import * as path from "path"; import { Capability } from "../../../utils/constants"; import { CaseFactory } from "../../caseFactory"; import { @@ -16,49 +14,64 @@ import { writeMCPToolsFixture, removeMCPToolsFixture, } from "./mcpToolsFixture"; +import { + createMCPProjectWithEnv, + expectDynamicMCPProject, + expectNoOAuthRegister, + expectStaticMCPProject, + learnMCPServerUrl, + mcpDynamicFlowEnv, + mcpStaticFlowEnv, +} from "./mcpTestUtils"; // Case 1: atk new — MCP with no-auth server URL (auto-fetch tools) class DeclarativeAgentMCPNoAuthNew extends CaseFactory { + public override async onCreate( + appName: string, + testFolder: string, + capability: Capability, + programmingLanguage?: ProgrammingLanguage, + custimized?: Record, + ): Promise { + await createMCPProjectWithEnv( + testFolder, + appName, + capability, + programmingLanguage, + custimized, + mcpDynamicFlowEnv, + ); + } + public override async onAfter(projectPath: string): Promise { await fs.remove(projectPath); } public override async onAfterCreate(projectPath: string): Promise { - const appPackage = path.join(projectPath, "appPackage"); - - // ai-plugin.json must exist and have functions and MCP runtime - const aiPlugin = await fs.readJSON(path.join(appPackage, "ai-plugin.json")); - expect(aiPlugin.functions).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes[0].type).to.equal("RemoteMCPServer"); - expect(aiPlugin.runtimes[0].spec.url).to.be.a("string").that.is.not.empty; - // No auth block for no-auth servers - expect(aiPlugin.runtimes[0].auth).to.be.undefined; - - // mcp-tools-1.json must exist with tool definitions - const mcpToolsPath = path.join(appPackage, "mcp-tools-1.json"); - expect(fs.pathExistsSync(mcpToolsPath)).to.be.true; - const mcpTools = await fs.readJSON(mcpToolsPath); - expect(mcpTools.tools).to.be.an("array").that.is.not.empty; - - // DA manifest must reference ai-plugin.json - const daManifest = await fs.readJSON( - path.join(appPackage, "declarativeAgent.json"), - ); - expect(daManifest.actions).to.be.an("array").that.is.not.empty; - expect(daManifest.actions[0].file).to.equal("ai-plugin.json"); - - // m365agents.yml should NOT contain oauth/register action - const ymlPath = path.join(projectPath, "m365agents.yml"); - if (fs.pathExistsSync(ymlPath)) { - const ymlContent = fs.readFileSync(ymlPath, "utf8"); - expect(ymlContent).to.not.include("oauth/register"); - } + await expectDynamicMCPProject(projectPath); + expectNoOAuthRegister(projectPath); } } -// Case 6: atk new — MCP with tools loaded from file (no auth) +// Case 6: atk new — MCP static flow with tools loaded from file (no auth) class DeclarativeAgentMCPNoAuthFile extends CaseFactory { + public override async onCreate( + appName: string, + testFolder: string, + capability: Capability, + programmingLanguage?: ProgrammingLanguage, + custimized?: Record, + ): Promise { + await createMCPProjectWithEnv( + testFolder, + appName, + capability, + programmingLanguage, + custimized, + mcpStaticFlowEnv, + ); + } + public override async onBefore(): Promise { await writeMCPToolsFixture(); } @@ -69,21 +82,7 @@ class DeclarativeAgentMCPNoAuthFile extends CaseFactory { } public override async onAfterCreate(projectPath: string): Promise { - const appPackage = path.join(projectPath, "appPackage"); - - // ai-plugin.json must exist and have functions and MCP runtime - const aiPlugin = await fs.readJSON(path.join(appPackage, "ai-plugin.json")); - expect(aiPlugin.functions).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes[0].type).to.equal("RemoteMCPServer"); - // No auth block for no-auth servers - expect(aiPlugin.runtimes[0].auth).to.be.undefined; - - // mcp-tools-1.json must exist - const mcpToolsPath = path.join(appPackage, "mcp-tools-1.json"); - expect(fs.pathExistsSync(mcpToolsPath)).to.be.true; - const mcpTools = await fs.readJSON(mcpToolsPath); - expect(mcpTools.tools).to.be.an("array").that.is.not.empty; + await expectStaticMCPProject(projectPath); } } @@ -91,7 +90,7 @@ class DeclarativeAgentMCPNoAuthFile extends CaseFactory { const noAuthUrlRecord: Record = {}; noAuthUrlRecord["with-plugin"] = "yes"; noAuthUrlRecord["api-plugin-type"] = "mcp"; -noAuthUrlRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +noAuthUrlRecord["mcp-da-server-url"] = learnMCPServerUrl; new DeclarativeAgentMCPNoAuthNew( Capability.DeclarativeAgent, @@ -109,7 +108,7 @@ new DeclarativeAgentMCPNoAuthNew( const noAuthFileRecord: Record = {}; noAuthFileRecord["with-plugin"] = "yes"; noAuthFileRecord["api-plugin-type"] = "mcp"; -noAuthFileRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +noAuthFileRecord["mcp-da-server-url"] = learnMCPServerUrl; noAuthFileRecord["mcp-tools-file-path"] = mcpToolsFilePath; new DeclarativeAgentMCPNoAuthFile( diff --git a/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPWithAuth.tests.ts b/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPWithAuth.tests.ts index 90acc9b8fed..a2da2cd1b41 100644 --- a/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPWithAuth.tests.ts +++ b/packages/tests/src/e2e/declarativeAgent/mcp/DeclarativeAgentMCPWithAuth.tests.ts @@ -6,9 +6,7 @@ */ import { ProgrammingLanguage } from "@microsoft/teamsfx-core"; -import { expect } from "chai"; import * as fs from "fs-extra"; -import * as path from "path"; import { Capability } from "../../../utils/constants"; import { CaseFactory } from "../../caseFactory"; import { @@ -16,6 +14,13 @@ import { writeMCPToolsFixture, removeMCPToolsFixture, } from "./mcpToolsFixture"; +import { + createMCPProjectWithEnv, + expectNoOAuthRegister, + expectStaticMCPProject, + learnMCPServerUrl, + mcpStaticFlowEnv, +} from "./mcpTestUtils"; // Verification for MCP projects when --mcp-da-auth-type is specified. // Note: learn.microsoft.com/api/mcp is a public no-auth server, so even when @@ -26,6 +31,23 @@ import { class DeclarativeAgentMCPWithAuth extends CaseFactory { private authType: "oauth" | "entra-sso"; + public override async onCreate( + appName: string, + testFolder: string, + capability: Capability, + programmingLanguage?: ProgrammingLanguage, + custimized?: Record, + ): Promise { + await createMCPProjectWithEnv( + testFolder, + appName, + capability, + programmingLanguage, + custimized, + mcpStaticFlowEnv, + ); + } + public override async onBefore(): Promise { await writeMCPToolsFixture(); } @@ -54,38 +76,8 @@ class DeclarativeAgentMCPWithAuth extends CaseFactory { } public override async onAfterCreate(projectPath: string): Promise { - const appPackage = path.join(projectPath, "appPackage"); - - // ai-plugin.json must exist with MCP runtime - const aiPlugin = await fs.readJSON(path.join(appPackage, "ai-plugin.json")); - expect(aiPlugin.functions).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes).to.be.an("array").that.is.not.empty; - expect(aiPlugin.runtimes[0].type).to.equal("RemoteMCPServer"); - expect(aiPlugin.runtimes[0].spec.url).to.be.a("string").that.is.not.empty; - - // With a no-auth server, auth block should be absent even if --mcp-da-auth-type was specified - // Auth injection is gated on server probe detecting auth requirement - expect(aiPlugin.runtimes[0].auth).to.be.undefined; - - // mcp-tools-1.json must exist with tool definitions - const mcpToolsPath = path.join(appPackage, "mcp-tools-1.json"); - expect(fs.pathExistsSync(mcpToolsPath)).to.be.true; - const mcpTools = await fs.readJSON(mcpToolsPath); - expect(mcpTools.tools).to.be.an("array").that.is.not.empty; - - // m365agents.yml should NOT contain oauth/register for a no-auth server - const ymlPath = path.join(projectPath, "m365agents.yml"); - if (fs.pathExistsSync(ymlPath)) { - const ymlContent = fs.readFileSync(ymlPath, "utf8"); - expect(ymlContent).to.not.include("oauth/register"); - } - - // DA manifest must reference ai-plugin.json - const daManifest = await fs.readJSON( - path.join(appPackage, "declarativeAgent.json"), - ); - expect(daManifest.actions).to.be.an("array").that.is.not.empty; - expect(daManifest.actions[0].file).to.equal("ai-plugin.json"); + await expectStaticMCPProject(projectPath); + expectNoOAuthRegister(projectPath); } } @@ -93,7 +85,7 @@ class DeclarativeAgentMCPWithAuth extends CaseFactory { const oauthRecord: Record = {}; oauthRecord["with-plugin"] = "yes"; oauthRecord["api-plugin-type"] = "mcp"; -oauthRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +oauthRecord["mcp-da-server-url"] = learnMCPServerUrl; oauthRecord["mcp-da-auth-type"] = "oauth"; new DeclarativeAgentMCPWithAuth( @@ -107,7 +99,7 @@ new DeclarativeAgentMCPWithAuth( const entraRecord: Record = {}; entraRecord["with-plugin"] = "yes"; entraRecord["api-plugin-type"] = "mcp"; -entraRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +entraRecord["mcp-da-server-url"] = learnMCPServerUrl; entraRecord["mcp-da-auth-type"] = "entra-sso"; new DeclarativeAgentMCPWithAuth( @@ -121,7 +113,7 @@ new DeclarativeAgentMCPWithAuth( const oauthFileRecord: Record = {}; oauthFileRecord["with-plugin"] = "yes"; oauthFileRecord["api-plugin-type"] = "mcp"; -oauthFileRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +oauthFileRecord["mcp-da-server-url"] = learnMCPServerUrl; oauthFileRecord["mcp-da-auth-type"] = "oauth"; oauthFileRecord["mcp-tools-file-path"] = mcpToolsFilePath; @@ -136,7 +128,7 @@ new DeclarativeAgentMCPWithAuth( const entraFileRecord: Record = {}; entraFileRecord["with-plugin"] = "yes"; entraFileRecord["api-plugin-type"] = "mcp"; -entraFileRecord["mcp-da-server-url"] = "https://learn.microsoft.com/api/mcp"; +entraFileRecord["mcp-da-server-url"] = learnMCPServerUrl; entraFileRecord["mcp-da-auth-type"] = "entra-sso"; entraFileRecord["mcp-tools-file-path"] = mcpToolsFilePath; diff --git a/packages/tests/src/e2e/declarativeAgent/mcp/mcpTestUtils.ts b/packages/tests/src/e2e/declarativeAgent/mcp/mcpTestUtils.ts new file mode 100644 index 00000000000..1b73e85816a --- /dev/null +++ b/packages/tests/src/e2e/declarativeAgent/mcp/mcpTestUtils.ts @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { ProgrammingLanguage } from "@microsoft/teamsfx-core"; +import { expect } from "chai"; +import * as fs from "fs-extra"; +import * as path from "path"; +import { Capability } from "../../../utils/constants"; +import { Executor } from "../../../utils/executor"; + +export const learnMCPServerUrl = "https://learn.microsoft.com/api/mcp"; + +// The dynamic-discovery `ai-plugin.json` (empty functions + enable_dynamic_discovery) +// is produced only by the v4 create template, so this flow requires v4 on. When +// v4 is off, `atk new` falls back to the legacy generator which always emits a +// static (mcp_tool_description + mcp-tools-1.json) scaffold regardless of DT. +export const mcpDynamicFlowEnv: Record = { + TEAMSFX_V4_ENABLED: "true", + TEAMSFX_MCP_FOR_DA_DT: "true", +}; + +// Static scaffold comes from the legacy generator (v4 off), independent of DT. +export const mcpStaticFlowEnv: Record = { + TEAMSFX_V4_ENABLED: "false", + TEAMSFX_MCP_FOR_DA_DT: "false", +}; + +function programmingLanguageParam(language?: ProgrammingLanguage): string { + if (language === ProgrammingLanguage.CSharp) { + return "--runtime dotnet"; + } + if ( + language !== undefined && + language !== ProgrammingLanguage.Common && + language !== ProgrammingLanguage.None + ) { + return `--programming-language ${language}`; + } + return ""; +} + +export async function createMCPProjectWithEnv( + workspace: string, + appName: string, + capability: Capability, + language: ProgrammingLanguage | undefined, + customized: Record = {}, + env: Record, +): Promise { + const command = + `atk new --interactive false --debug --app-name ${appName} --capability ${capability} ` + + `${programmingLanguageParam(language)} ` + + Object.entries(customized) + .map(([key, value]) => `--${key} ${value}`) + .join(" "); + const result = await Executor.execute(command, workspace, { + ...process.env, + ...env, + }); + expect(result.success || fs.pathExistsSync(path.resolve(workspace, appName))) + .to.be.true; +} + +export async function expectDynamicMCPProject( + projectPath: string, +): Promise { + const appPackage = path.join(projectPath, "appPackage"); + const aiPlugin = await fs.readJSON(path.join(appPackage, "ai-plugin.json")); + const runtime = aiPlugin.runtimes[0]; + + expect(aiPlugin.functions).to.be.an("array").that.is.empty; + expect(runtime.type).to.equal("RemoteMCPServer"); + expect(runtime.spec.url).to.equal(learnMCPServerUrl); + expect(runtime.spec.enable_dynamic_discovery).to.equal(true); + expect(runtime.spec).to.not.have.property("mcp_tool_description"); + expect(runtime.run_for_functions).to.deep.equal(["*"]); + expect(runtime.auth.type).to.equal("None"); + + expect(fs.pathExistsSync(path.join(appPackage, "mcp-tools-1.json"))).to.be + .false; + + const daManifest = await fs.readJSON( + path.join(appPackage, "declarativeAgent.json"), + ); + expect(daManifest.actions).to.be.an("array").that.is.not.empty; + expect(daManifest.actions[0].file).to.equal("ai-plugin.json"); +} + +export async function expectStaticMCPProject( + projectPath: string, +): Promise { + const appPackage = path.join(projectPath, "appPackage"); + const aiPlugin = await fs.readJSON(path.join(appPackage, "ai-plugin.json")); + const runtime = aiPlugin.runtimes[0]; + + expect(aiPlugin.functions).to.be.an("array").that.is.not.empty; + expect(runtime.type).to.equal("RemoteMCPServer"); + expect(runtime.spec.url).to.be.a("string").that.is.not.empty; + expect(runtime.spec).to.not.have.property("enable_dynamic_discovery"); + expect(runtime.spec.mcp_tool_description.file).to.equal("mcp-tools-1.json"); + expect(runtime.auth).to.be.undefined; + + const mcpToolsPath = path.join(appPackage, "mcp-tools-1.json"); + expect(fs.pathExistsSync(mcpToolsPath)).to.be.true; + const mcpTools = await fs.readJSON(mcpToolsPath); + expect(mcpTools.tools).to.be.an("array").that.is.not.empty; +} + +export function expectNoOAuthRegister(projectPath: string): void { + const ymlPath = path.join(projectPath, "m365agents.yml"); + if (fs.pathExistsSync(ymlPath)) { + const ymlContent = fs.readFileSync(ymlPath, "utf8"); + expect(ymlContent).to.not.include("oauth/register"); + } +} From 2d60cea45fd9af4486185d375ef008ec6d2299e5 Mon Sep 17 00:00:00 2001 From: Zhiyu You Date: Tue, 7 Jul 2026 18:33:58 +0800 Subject: [PATCH 2/3] fix(fx-core): keep injecting oauth/register when MCP auth metadata resolution fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DT add-MCP inline path wrapped endpoint resolution and yml injection in one try/catch, so a failed auth-metadata probe (unreachable server / no resource_metadata) skipped the oauth/register injection entirely — leaving the ai-plugin referencing a registration that was never created. Split into two best-effort steps (mirroring the create flow): endpoint resolution failure now leaves endpoints empty but the oauth/register (or dcr/register) action is still written for the developer to complete. Verified end-to-end via the CLI add-action flow. --- .../src/core/FxCore.declarativeAgent.ts | 54 ++++++++----- .../core/FxCore.declarativeAgent.test.ts | 76 +++++++++++++++++++ 2 files changed, 110 insertions(+), 20 deletions(-) diff --git a/packages/fx-core/src/core/FxCore.declarativeAgent.ts b/packages/fx-core/src/core/FxCore.declarativeAgent.ts index 9921765e4af..d4360c76735 100644 --- a/packages/fx-core/src/core/FxCore.declarativeAgent.ts +++ b/packages/fx-core/src/core/FxCore.declarativeAgent.ts @@ -678,28 +678,42 @@ export class FxCoreDeclarativeAgentPart { }; if (authType && authType !== "none") { - try { - // The add question tree does not collect the auth-server metadata / - // well-known URL, so — mirroring the create flow — probe the MCP - // server to discover it when the caller didn't pass one. The result - // feeds resolveMCPAuthEndpoints so oauth/register + dcr/register get - // real authorization/token/well-known URLs. Only oauth / - // oauth-dynamic consume metadata; entra-sso resolves no endpoints, - // so skip the (10s-timeout) probe for it. Best-effort: a failure - // leaves endpoints empty for the developer to fill in later. - const needsMetadataProbe = authType === "oauth" || authType === "oauth-dynamic"; - if (needsMetadataProbe && !inputs[QuestionNames.MCPForDAAuthMetadataUrl]) { - try { - const { probeMCPServerAuth } = await import("../component/utils/mcpToolFetcher"); - const authProbe = await probeMCPServerAuth(mcpServerUrl); - if (authProbe.authMetadataUrl) { - inputs[QuestionNames.MCPForDAAuthMetadataUrl] = authProbe.authMetadataUrl; - } - } catch { - // best-effort probe; endpoint resolution below tolerates undefined + // The add question tree does not collect the auth-server metadata / + // well-known URL, so — mirroring the create flow — probe the MCP + // server to discover it when the caller didn't pass one. The result + // feeds resolveMCPAuthEndpoints so oauth/register + dcr/register get + // real authorization/token/well-known URLs. Only oauth / oauth-dynamic + // consume metadata; entra-sso resolves no endpoints, so skip the + // (10s-timeout) probe for it. + const needsMetadataProbe = authType === "oauth" || authType === "oauth-dynamic"; + if (needsMetadataProbe && !inputs[QuestionNames.MCPForDAAuthMetadataUrl]) { + try { + const { probeMCPServerAuth } = await import("../component/utils/mcpToolFetcher"); + const authProbe = await probeMCPServerAuth(mcpServerUrl); + if (authProbe.authMetadataUrl) { + inputs[QuestionNames.MCPForDAAuthMetadataUrl] = authProbe.authMetadataUrl; } + } catch { + // best-effort probe; endpoint resolution below tolerates undefined } - const endpoints = await resolveMCPAuthEndpoints(authType, inputs); + } + // Endpoint resolution is best-effort and MUST NOT block the + // oauth/register injection: an unreachable server or missing metadata + // leaves endpoints empty and the injector still writes the action + // (with placeholder URLs) so the developer can fill them in later. + let endpoints: ResolvedMCPAuthEndpoints = {}; + try { + endpoints = await resolveMCPAuthEndpoints(authType, inputs); + } catch (error: any) { + mcpWarnings.push({ + type: "mcpAuthMetadataError", + content: getLocalizedString( + "core.MCPForDA.mcpAuthMetadataMissingError", + error.message + ), + }); + } + try { const ymlPath = pathUtils.getYmlFilePath(inputs.projectPath); if (ymlPath) { await injectMCPAuthActionToYml({ diff --git a/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts b/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts index d1d7e420000..014a53ac23f 100644 --- a/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts +++ b/packages/fx-core/tests/core/FxCore.declarativeAgent.test.ts @@ -3184,6 +3184,82 @@ describe("addPlugin", async () => { } }); + it("from MCP (DT flag on): still injects oauth/register when auth metadata cannot be resolved", async () => { + const appName = await mockV3Project(); + const projectPath = path.join(os.tmpdir(), appName); + const inputs: Inputs = { + platform: Platform.CLI, + [QuestionNames.Folder]: os.tmpdir(), + [QuestionNames.TeamsAppManifestFilePath]: "manifest.json", + [QuestionNames.ActionType]: ActionStartOptions.mcp().id, + [QuestionNames.MCPForDAServerUrl]: "https://example.com/mcp", + [QuestionNames.MCPToolsFilePath]: "", + [QuestionNames.MCPForDAAuthType]: "oauth", + [QuestionNames.MCPForDAClientId]: "client-id", + [QuestionNames.MCPForDAClientSecret]: "client-secret", + [QuestionNames.MCPForDAScopes]: "scope-a", + [QuestionNames.MCPForDAAuthMetadataUrl]: "https://example.com/.well-known/oauth-metadata", + projectPath, + }; + + const manifest = new TeamsAppManifest(); + manifest.copilotExtensions = { + declarativeCopilots: [{ file: "test1.json", id: "action_1" }], + }; + + vi.spyOn(featureFlagManager, "getBooleanValue").mockImplementation((flag) => { + return flag === FeatureFlags.MCPForDADT; + }); + vi.spyOn(validationUtils, "validateInputs").mockResolvedValue(undefined); + vi.spyOn(manifestUtils, "_readAppManifest").mockResolvedValue(ok(manifest)); + vi.spyOn(copilotGptManifestUtils, "getManifestPath").mockResolvedValue(ok("dcManifest.json")); + vi.spyOn(copilotGptManifestUtils, "readCopilotGptManifestFile").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + vi.spyOn( + copilotGptManifestUtils, + "getDefaultNextAvailablePluginManifestPath" + ).mockResolvedValue("ai-plugin_1.json"); + vi.spyOn(copilotGptManifestUtils, "addAction").mockResolvedValue( + ok({} as DeclarativeCopilotManifestSchema) + ); + + vi.spyOn(addPluginTools.ui, "showMessage").mockImplementation((level) => { + if (level === "warn") return Promise.resolve(ok("Add")); + return Promise.resolve(ok("")); + }); + + // Auth-server metadata resolution fails (server unreachable / no metadata). + const mcpAuthScaffolderModule = await import("../../src/component/utils/mcpAuthScaffolder"); + vi.spyOn( + mcpAuthScaffolderModule.mcpAuthScaffolderDeps, + "resolveMCPOAuthMetadata" + ).mockRejectedValue(new Error("no resource_metadata")); + + const actionInjectorModule = await import("../../src/component/configManager/actionInjector"); + const injectStub = vi + .spyOn(actionInjectorModule.ActionInjector, "injectCreateOAuthActionForMCP") + .mockResolvedValue(); + + vi.spyOn(pathUtils, "getYmlFilePath").mockReturnValue("m365agents.yml"); + vi.spyOn(fs, "ensureFile").mockResolvedValue(); + vi.spyOn(fs, "writeJSON").mockResolvedValue(); + + const core = new FxCore(addPluginTools); + const result = await core.addPlugin(inputs); + + assert.isTrue(result.isOk()); + // Endpoint resolution failing must NOT block the oauth/register injection: + // the action is still written (with empty endpoints for the dev to fill in). + assert.equal(injectStub.mock.calls.length, 1); + assert.equal(injectStub.mock.calls[0][2], "examplecom"); + assert.equal(injectStub.mock.calls[0][3], "MCP_DA_AUTH_ID_EXAMPLECOM"); + + if (await fs.pathExists(projectPath)) { + await fs.remove(projectPath); + } + }); + it("from MCP (DT off): uses legacy static add-action path", async () => { const appName = await mockV3Project(); const projectPath = path.join(os.tmpdir(), appName); From f3f395ae7b84c5c364bcf2b29b3ab6beae267775 Mon Sep 17 00:00:00 2001 From: Zhiyu You Date: Wed, 8 Jul 2026 10:19:20 +0800 Subject: [PATCH 3/3] test(fx-core): make generator template-version tests version-independent Read package.json via require (the same cached object utils.ts/templateHelper.ts use) so mutating .version in a test actually affects the code under test; a default JSON import yielded a separate object under vitest, so version mutations were silently ignored and tests read the release-bumped (rc) version. Fix assertions that only passed due to that import/require mismatch: VS beta -> templates-vs@0.0.0-rc, VSC rc -> templates@0.0.0-rc/ts.zip, templateHelper alpha -> useLocalTemplate() true. Control packageJson.version in the local-vs-remote generator test so it runs the version comparison instead of the rc short-circuit. --- .../tests/component/generator/generator.test.ts | 8 ++++++++ .../tests/component/generator/utils.test.ts | 17 +++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/packages/fx-core/tests/component/generator/generator.test.ts b/packages/fx-core/tests/component/generator/generator.test.ts index c66d604b04a..19b9ed42a20 100644 --- a/packages/fx-core/tests/component/generator/generator.test.ts +++ b/packages/fx-core/tests/component/generator/generator.test.ts @@ -62,6 +62,9 @@ import sampleConfigV3 from "../../common/samples-config-v3.json"; import { MockTools, randomAppName } from "../../core/utils"; const originalTemplateConfig = { ...templateConfig }; +// eslint-disable-next-line @typescript-eslint/no-require-imports +const packageJson = require("../../../package.json"); +const originalPackageVersion = (packageJson as any).version; const mockedSampleInfo: SampleConfig = { id: "test-id", @@ -870,6 +873,7 @@ describe("render template", () => { afterEach(async () => { vi.restoreAllMocks(); Object.assign(templateConfig, originalTemplateConfig); + (packageJson as any).version = originalPackageVersion; if (await fs.pathExists(tmpDir)) { await fs.remove(tmpDir); } @@ -1121,6 +1125,10 @@ describe("render template", () => { await buildFakeTemplateZip(templateName, mockFileName); vi.spyOn(templateHelper, "useLocalTemplate").mockReturnValue(false); + // Force a stable package version so getTemplateUrl runs the local-vs-remote + // version comparison instead of the rc short-circuit (which returns a + // remote URL and triggers the fallback this test guards against). + (packageJson as any).version = "3.0.0"; (templateConfig as any).localVersion = "9.9.9"; (templateConfig as any).version = "~3.0.0"; const tagList = "1.0.0\n 2.0.0\n 2.1.0\n 3.0.0"; diff --git a/packages/fx-core/tests/component/generator/utils.test.ts b/packages/fx-core/tests/component/generator/utils.test.ts index 195e62d44f0..a77336ff6ab 100644 --- a/packages/fx-core/tests/component/generator/utils.test.ts +++ b/packages/fx-core/tests/component/generator/utils.test.ts @@ -3,7 +3,6 @@ import { DeclarativeAgentManifest, Platform, err, ok, signedIn } from "@microsoft/teamsfx-api"; import mockedEnv from "mocked-env"; import { assert, expect, vi } from "vitest"; -import packageJson from "../../../package.json"; import { GraphClient } from "../../../src/client/graphClient"; import { createContext, setTools } from "../../../src/common/globalVars"; import * as requestUtils from "../../../src/common/requestUtils"; @@ -18,6 +17,14 @@ import { } from "../../../src/component/generator/utils"; import { MockTools } from "../../core/utils"; +// Read package.json via require — the same cached object the code under test +// (utils.ts / templateHelper.ts) reads — so mutating `.version` in a test +// actually affects what the code sees. A default JSON `import` yields a separate +// object under vitest, so version mutations were silently ignored and the tests +// read the real (release-bumped, e.g. rc) package version. +// eslint-disable-next-line @typescript-eslint/no-require-imports +const packageJson = require("../../../package.json"); + describe("utils unit test cases", () => { const sandbox = vi; const originalPackageVersion = (packageJson as any).version; @@ -101,7 +108,7 @@ describe("utils unit test cases", () => { const getLatestVersion = () => Promise.resolve(templateConfig.vsversion); const result = await getTemplateUrl("csharp", getLatestVersion, Platform.VS); const expectedUrl = - "https://github.com/OfficeDev/microsoft-365-agents-toolkit/releases/download/templates-vs@18.6.0/csharp.zip"; + "https://github.com/OfficeDev/microsoft-365-agents-toolkit/releases/download/templates-vs@0.0.0-rc/csharp.zip"; assert.strictEqual(result, expectedUrl); }); @@ -138,7 +145,9 @@ describe("utils unit test cases", () => { (packageJson as any).version = "3.0.0-rc.1"; const getLatestVersion = () => Promise.resolve("6.0.0"); const result = await getTemplateUrl("ts", getLatestVersion, Platform.VSCode); - assert.isUndefined(result); + const expectedUrl = + "https://github.com/OfficeDev/microsoft-365-agents-toolkit/releases/download/templates@0.0.0-rc/ts.zip"; + assert.strictEqual(result, expectedUrl); }); it("should use latest version for beta version in package.json when latest is higher", async () => { @@ -309,7 +318,7 @@ describe("templateHelper unit test cases", () => { }); (packageJson as any).version = "3.0.0-alpha.1"; const result = useLocalTemplate(); - assert.isFalse(result); + assert.isTrue(result); restore(); });