Closed
Conversation
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:
|
| Section | Status | Recommendation |
|---|---|---|
| Title | Use a descriptive title, e.g. Hotfix: Add MCP client consumption support (built-in and managed MCP connections, manifests, and tests) |
|
| Commit Type | ❌ | Select a commit type (likely fix for hotfix or feature) |
| Risk Level | ❌ | Add risk:medium label and select Medium in the PR body |
| What & Why | ❌ | Add a concise "What & Why" summarizing the changes and purpose |
| Impact of Change | ❌ | Explain Users/Developers/System impacts (see recommendations) |
| Test Plan | ❌ | Mark Unit tests added/updated and describe how tested; point to new test files in diff |
| Contributors | Optionally add contributors' names/handles | |
| Screenshots/Videos | Not necessary unless UX changed significantly; add if applicable |
Summary / Next steps
- This PR does NOT pass the PR title/body checklist because required metadata (commit type, risk level, What & Why, Impact, Test Plan) are missing or inconsistent with the code changes.
- Advised risk level (based on the diff touching connection management, manifests, UI, and adding new behavior) is
risk:medium. The PR did not specify any risk — please add therisk:mediumlabel. If you believe the change is lower risk, provide justification in the PR body; otherwise userisk:medium. - Test plan mismatch: The code includes unit tests (see new files in the diff: libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/connection.spec.ts, connector.spec.ts, operationmanifest.spec.ts). Update the Test Plan to mark unit tests and explain how they were executed.
Please update the PR title and body per the recommendations above and re-submit. Thank you for helping maintain high-quality PR metadata — this speeds up reviews and release coordination.
Last updated: Fri, 13 Mar 2026 17:25:47 GMT
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Consumption-SKU support for the native MCP Client Tool by wiring up a built-in MCP connector + operation manifest, plus client-side connection creation and dynamic tool listing (listMcpTools) so the designer can enumerate MCP tools.
Changes:
- Register MCP Client Tool (
McpClientTool→connectionProviders/mcpclient+nativemcpclient) in the Consumption operation manifest service and add the built-in operation manifest. - Add Consumption connector/connection service support for MCP tool discovery, including request payload shaping for built-in vs managed connections.
- Update connection creation UI behavior (hide name input for MCP in Consumption) and add unit tests covering the new behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/operationmanifest.ts | Routes MCP Client Tool operations to the built-in MCP manifest and registers the MCP connector as built-in. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/manifests/mcpclientconnector.ts | Adds the Consumption MCP connector definition (connection parameter sets, icon, etc.). |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/manifests/builtinmcpclient.ts | Adds the built-in MCP operation manifest (including dynamic list definition for tools). |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/connector.ts | Implements listMcpTools dynamic list behavior for Consumption (built-in vs managed payload shapes). |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/connection.ts | Adds Consumption MCP connection creation logic (built-in in-memory connection + managed connection creation). |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/operationmanifest.spec.ts | Tests for MCP Client Tool operation info, support checks, and manifest retrieval. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/connector.spec.ts | Tests for MCP listMcpTools payload shaping and response mapping. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/connection.spec.ts | Tests for built-in MCP connection creation and auth parameter extraction helpers. |
| libs/designer/src/lib/ui/panel/connectionsPanel/createConnection/createConnection.tsx | Hides name input for MCP connections in Consumption based on the active connection service implementation. |
| libs/designer-v2/src/lib/ui/panel/connectionsPanel/createConnection/createConnection.tsx | Same MCP/Consumption name-input behavior for designer-v2. |
| apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesignerConsumption.tsx | Enables the MCP Client Tool flag in the Standalone Consumption designer host options. |
Comment on lines
+130
to
+166
| it('should include authentication for ApiKey auth type', async () => { | ||
| InitConnectionService({ | ||
| getConnection: vi.fn().mockResolvedValue({ | ||
| id: builtInConnectionId, | ||
| name: 'test-mcp', | ||
| properties: { | ||
| displayName: 'Test MCP Server', | ||
| parameterValues: { | ||
| mcpServerUrl: 'https://mcp.example.com', | ||
| authenticationType: 'ApiKey', | ||
| key: 'test-api-key', | ||
| keyHeaderName: 'X-Api-Key', | ||
| }, | ||
| }, | ||
| } as unknown as Connection), | ||
| } as any); | ||
|
|
||
| vi.mocked(mockHttpClient.post).mockResolvedValue(mockMcpToolsResponse); | ||
|
|
||
| await connectorService.getListDynamicValues( | ||
| builtInConnectionId, | ||
| '/connectionProviders/mcpclient', | ||
| 'nativemcpclient', | ||
| {}, | ||
| mcpDynamicState | ||
| ); | ||
|
|
||
| const postCallArgs = vi.mocked(mockHttpClient.post).mock.calls[0][0]; | ||
| const content = postCallArgs.content as any; | ||
|
|
||
| expect(content.connection.authentication).toEqual({ | ||
| type: 'ApiKey', | ||
| value: 'test-api-key', | ||
| name: 'X-Api-Key', | ||
| in: 'header', | ||
| }); | ||
| }); |
Comment on lines
+230
to
+253
| private _buildMcpAuthentication(connectionProperties: Record<string, any>): Record<string, any> | undefined { | ||
| const authType = connectionProperties['authenticationType']; | ||
| if (!authType || authType === 'None') { | ||
| return undefined; | ||
| } | ||
|
|
||
| const authentication: Record<string, any> = { type: authType }; | ||
| if (authType === 'ApiKey') { | ||
| authentication['value'] = connectionProperties['key']; | ||
| authentication['name'] = connectionProperties['keyHeaderName']; | ||
| authentication['in'] = 'header'; | ||
| } else if (authType === 'Basic') { | ||
| authentication['username'] = connectionProperties['username']; | ||
| authentication['password'] = connectionProperties['password']; | ||
| } else if (authType === 'ActiveDirectoryOAuth') { | ||
| authentication['tenant'] = connectionProperties['tenant']; | ||
| authentication['clientId'] = connectionProperties['clientId']; | ||
| authentication['secret'] = connectionProperties['secret']; | ||
| authentication['authority'] = connectionProperties['authority']; | ||
| authentication['audience'] = connectionProperties['audience']; | ||
| } else if (authType === 'ClientCertificate') { | ||
| authentication['pfx'] = connectionProperties['pfx']; | ||
| authentication['password'] = connectionProperties['password']; | ||
| } |
Comment on lines
+81
to
+85
| const connection = await ConnectionService().getConnection(connectionId); | ||
| const connectionProperties = (connection?.properties as any)?.parameterValues ?? {}; | ||
| const isBuiltInConnection = connectionId?.includes('connectionProviders/mcpclient'); | ||
|
|
||
| if (isBuiltInConnection) { |
Comment on lines
+274
to
+312
| { | ||
| name: 'Key', | ||
| parameters: { | ||
| serverUrl: { | ||
| type: 'string', | ||
| uiDefinition: { | ||
| displayName: 'Server URL', | ||
| constraints: { | ||
| required: 'true', | ||
| }, | ||
| description: 'Server URL', | ||
| }, | ||
| }, | ||
| key: { | ||
| type: 'securestring', | ||
| uiDefinition: { | ||
| displayName: 'Key', | ||
| constraints: { | ||
| required: 'true', | ||
| propertyPath: ['authentication'], | ||
| }, | ||
| description: 'Key', | ||
| }, | ||
| }, | ||
| keyHeaderName: { | ||
| type: 'string', | ||
| uiDefinition: { | ||
| displayName: 'Key Header Name', | ||
| constraints: { | ||
| propertyPath: ['authentication'], | ||
| }, | ||
| description: 'Key header name', | ||
| }, | ||
| }, | ||
| }, | ||
| uiDefinition: { | ||
| displayName: 'Key', | ||
| }, | ||
| }, |
| 'tenant', | ||
| 'authority', | ||
| 'audience', | ||
| 'pfx', |
Comment on lines
+72
to
+92
| const connectionInfo: ConnectionCreationInfo = { | ||
| displayName: 'test-mcp-connection', | ||
| connectionParameters: { | ||
| serverUrl: { value: 'https://mcp-server.example.com' }, | ||
| }, | ||
| connectionParametersSet: { | ||
| name: 'ApiKey', | ||
| values: { | ||
| key: { value: 'test-api-key' }, | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
| const result = await service.createConnection('test-connection-id', connector as Connector, connectionInfo); | ||
|
|
||
| expect(result).toBeDefined(); | ||
| expect(result.name).toBe('test-mcp-connection'); | ||
| expect(result.id).toContain('connectionProviders/mcpclient/connections/'); | ||
| expect((result.properties as any).parameterValues.mcpServerUrl).toBe('https://mcp-server.example.com'); | ||
| expect((result.properties as any).parameterValues.authenticationType).toBe('ApiKey'); | ||
| }); |
📊 Coverage CheckThe following changed files need attention:
Please add tests for the uncovered files before merging. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Type
Risk Level
What & Why
Impact of Change
Test Plan
Contributors
Screenshots/Videos