(Fix): Persist Managed Identity and Audience in MCP Client Tool workflow definition#9037
(Fix): Persist Managed Identity and Audience in MCP Client Tool workflow definition#9037
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 conventional commit style: fix: ... |
|
| Commit Type | ✅ | No change needed |
| Risk Level | Add risk:medium label; consider updating body |
|
| What & Why | ✅ | Clear and adequate |
| Impact of Change | ✅ | Optional small clarification suggested |
| Test Plan | ✅ | Add short note pointing to test file paths |
| Contributors | ✅ | No change needed |
| Screenshots/Videos | ✅ | No change needed |
Summary / Next Steps
- This PR passes the PR-title-and-body checks, but I recommend two small improvements before merging:
- Update the PR title to follow conventional-commit style. Example:
fix: persist managed identity and audience in MCP client tool workflow definition. - Add the repository risk label
risk:mediumto this PR (the PR body currently statesLow). Reason: changes affect multiple serialization and manifest surfaces that persist authentication details; while tests were added and the changes are additive, the broader surface area warrantsmediumrisk.
- Update the PR title to follow conventional-commit style. Example:
- Optional: add a brief line in the Test Plan listing the new/updated unit test files (for quick reviewer navigation):
- libs/logic-apps-shared/.../consumption/tests/connection.spec.ts
- libs/logic-apps-shared/.../consumption/tests/operationmanifest.spec.ts
- serializer changes in libs/designer(-v2)/.../bjsworkflow/serializer.ts
Please update the PR title and add the risk:medium label, then re-submit (or leave a comment once done). Thank you for the thorough description and for adding unit tests — that made review much easier!
Last updated: Thu, 09 Apr 2026 07:29:12 GMT
There was a problem hiding this comment.
Pull request overview
Fixes MCP Client Tool workflow serialization so Managed Identity authentication settings (identity, audience) are persisted end-to-end (manifest → connection parameter extraction → workflow serializers) instead of being dropped on save.
Changes:
- Added
identityparameter to the MCP connectorManagedServiceIdentityparameter set (standard + consumption manifests). - Included
identityin MCP connection parameter extraction paths (standard + consumption). - Updated Designer and Designer-v2 built-in MCP operation serialization to emit
Connection.Identity/Connection.Audiencewhen present; added/updated unit tests around MSI parameters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/logic-apps-shared/src/designer-client-services/lib/standard/manifest/mcpclientconnector.ts | Adds identity parameter to the MSI parameter set for standard MCP connector manifest. |
| libs/logic-apps-shared/src/designer-client-services/lib/standard/connection.ts | Ensures identity is included when building standard MCP connection authentication payload. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/manifests/mcpclientconnector.ts | Adds identity parameter to the MSI parameter set for consumption MCP connector manifest. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/connection.ts | Ensures consumption auth-parameter extraction includes identity for MCP connections. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/operationmanifest.spec.ts | Adds unit tests asserting MSI parameter set includes identity and preserves existing audience constraints. |
| libs/logic-apps-shared/src/designer-client-services/lib/consumption/tests/connection.spec.ts | Adds unit tests verifying MSI identity/audience extraction and inclusion in built-in MCP connection parameterValues. |
| libs/designer/src/lib/core/actions/bjsworkflow/serializer.ts | Emits Connection.Identity / Connection.Audience for built-in MCP operation serialization based on resolved connection values. |
| libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts | Same as above for Designer-v2 serializer. |
libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts
Outdated
Show resolved
Hide resolved
📊 Coverage CheckThe following changed files need attention:
Please add tests for the uncovered files before merging. |
Commit Type
Risk Level
What & Why
When configuring a BYO MCP server (e.g., IcM MCP) with Managed Identity authentication, the
identityandaudiencefields were silently dropped from the serialized workflow definition JSON. Manually adding them in Code View would get wiped on save.The
identityfield was missing at 4 layers: manifest definition, consumption/standard connection parameter extraction, and both serializers.Impact of Change
Impact of Change
ManagedServiceIdentityparameter set in both consumption and standard MCP connector manifests now includes anidentityparameter.Test Plan
Contributors
@Bhavd13
Screenshots/Videos
Code Output after fix -
