fix(fx-core): generate DT add-MCP action inline instead of v4 modify front door#16306
Conversation
…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.
Alive-Fish
requested review from
HuihuiWu-Microsoft,
Sherylueen,
qfai,
qinqingxu,
qinzhouxu,
tecton,
wh-alice and
yuqizhou77
as code owners
❌ VscUse Test Plan — Tests unknownWhy these tests: PR fixes the DA DT add-MCP action flow with auth wiring changes; changed e2e test files map directly to the three DA_MCP_*_Remote plans (no-auth, OAuth, Entra SSO edge cases) and Feature_DA_Add_MCP_Server covers the VS Code UI add-MCP-server flow. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
E2E Test Selection — AI SelectedWhy these tests: The fx-core changes are focused on declarativeAgent (FxCore.declarativeAgent.ts, AddPluginInputs/Options) and featureFlags; the directly modified e2e test files are all in declarativeAgent/mcp/, so all declarativeAgent/* tests are selected as the most targeted set covering the affected area. Cases selected (13):
Need to run more tests?Comment on this PR:
Then re-run the workflow. |
…solution fails 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.
❌ VscUse Test Plan — Tests failureWhy these tests: PR fixes the DT add-MCP action flow in FxCore.declarativeAgent.ts and adds MCP e2e tests (NoAuth, WithAuth, AuthEdgeCases), so MCP-specific DA plans and the add-action OAuth plan are the most relevant coverage. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
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.
❌ VscUse Test Plan — Tests failureWhy these tests: PR fixes the Declarative Agent Dynamic Tool Discovery add-MCP action flow (auth wiring, oauth/register, dcr/register), directly mapping to DA_MCP_* remote plans and the Feature_DA_Add_MCP_Server VS Code UI feature plan. Branch diff: Plans run:
ℹ️ How were these tests selected?GitHub Copilot (Claude Sonnet 4.6, high reasoning) analysed the PR title, description, and the diff between |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/6.12 #16306 +/- ##
===============================================
Coverage ? 87.01%
===============================================
Files ? 628
Lines ? 32864
Branches ? 7732
===============================================
Hits ? 28595
Misses ? 2438
Partials ? 1831
🚀 New features to boost your workflow:
|
3 participants
Summary
Fixes the Dynamic Tool Discovery (DT) "add MCP action" flow (
atk add action --api-plugin-type mcp). Previously, with DT on, the flow routed through the v4 modify front door +add-mcp-servertemplate, which produced an incompleteoauth/registeraction inm365agents.ymland coupled DT toTEAMSFX_V4_ENABLED. DT now generates the plugin manifest and auth wiring directly incore.addPlugin, independent of v4.What changed
FxCore.declarativeAgent.ts): DT-onadd action --api-plugin-type mcpwrites the URL-onlyai-plugin.json(no static tools list, noenable_dynamic_discovery) and wiresoauth/register/dcr/registervia the shared injector — no longer viamodifyProjectFrontDoor. Works withTEAMSFX_V4_ENABLEDoff.oauth/registeractionnameandregistrationIdare derived from the MCP server URL (shared verbatim with the create flow), so re-adding the same server is idempotent (one registration per server) and different servers get distinct registrations. Fixes the previousaction_<n>-based naming that could skip creating a needed registration.--mcp-da-auth-metadata-url, the MCP server is probed to discover it (mirrors the create flow) sooauth/oauth-dynamicresolve real authorization/token/well-known URLs instead of empty ones.TEAMSFX_MCP_FOR_DA_DTis enabled by default;FeatureFlagManager.listEnablednow includes default-on flags.AddPluginInputs/AddPluginOptionsfor the DT auth-type + credential options.Testing
packages/fx-coreunit:tests/core/FxCore.declarativeAgent.test.ts(inline flow,oauth/entra-sso/oauth-dynamicnaming, metadata probe) +tests/common/featureFlags.test.ts— green.tsc -p ./clean.atk new: dynamic (V4 on) vs static (V4 off).