Skip to content

fix(designer): Serialize non-managed MCP server connections to connections.json (#9296) [hotfix/v5.970]#9320

Merged
lambrianmsft merged 1 commit into
hotfix/v5.970from
elaina/hotfix-9296
Jun 27, 2026
Merged

fix(designer): Serialize non-managed MCP server connections to connections.json (#9296) [hotfix/v5.970]#9320
lambrianmsft merged 1 commit into
hotfix/v5.970from
elaina/hotfix-9296

Conversation

@Elaina-Lee

@Elaina-Lee Elaina-Lee commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Cherry-pick of #9296 to hotfix/v5.970.

Non-managed MCP server connections (auth types: None, Basic, Client Certificate) were not included in the deployment payload's connections.json, causing the validate API to fail with InvalidAgentWorkflow.

  • getConnectionsDataToSerialize in the MCP utils serializer now handles non-ARM connection IDs (/connectionProviders/mcpclient/connections/<name>)
  • convertConnectionsDataToReferences now handles agentMcpConnections so MCP connections loaded from existing connections.json are converted to Redux connection references

IcM: 21000001042369

Impact of Change

  • Users: Deploying workflows with non-managed MCP server connections (None, Basic, Client Certificate auth) no longer fails with InvalidAgentWorkflow validation error.
  • Developers: No API changes. getConnectionsDataToSerialize and convertConnectionsDataToReferences in MCP utils now handle additional connection ID formats.
  • System: No performance or architectural changes. Additive serialization logic only.

Test Plan

Contributors

@Elaina-Lee

Screenshots/Videos

…tions.json (#9296)

* Fix non-managed MCP server connections not serialized to connections.json

Non-managed identity MCP server connections (auth types None, Basic,
Client certificate, etc.) were not being included in the workflow
deployment payload, causing the validate API to fail.

Root cause: getConnectionsDataToSerialize only handled ARM resource IDs
via isArmResourceId check. Non-managed MCP connections use non-ARM IDs
(/connectionProviders/mcpclient/connections/<name>) and were skipped.

Fix:
- serializer.ts: Detect MCP connections and fetch their data via
  ConnectionService, constructing agentMcpConnections entries
- helper.ts: Add agentMcpConnections handling to
  convertConnectionsDataToReferences for proper state initialization
- Added unit tests for the new agentMcpConnections reference conversion

* fix: Improve MCP connection serialization and extend to designer-v2

- Apply same agentMcpConnections fix to designer-v2 serializer and helper
- Tighten isMcpConnectionReference to exact match instead of loose includes()
- Add error logging in getMcpConnectionData instead of silent catch
- Remove unused _referenceKey parameter from getMcpConnectionData
- Add unit tests for isMcpConnectionReference and convertConnectionsDataToReferences

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: Add agentMcpConnections handling to save flow in Standalone app

MCP connections (connectionProviders/mcpclient/) had no handler in the
save flow's if/else chain, causing them to be silently dropped during
save/validate. This adds the missing handler to both laDesigner.tsx and
laDesignerV2.tsx, plus adds hasNewMcpConnectionKeys detection to
getConnectionsToUpdate in v2.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: Move agentConnections merge-back outside isAgentWorkflow guard

The save flow deletes agent connections from connectionsData
unconditionally when processing connectionReferences, but the merge-back
step was inside an isAgentWorkflow() check. For "Stateful" workflows
with Agent actions (kind != "agentic"/"agent"), agent connections were
deleted and never restored, causing them to be permanently lost from
connections.json on the next deploy.

Move the agentConnections merge to run unconditionally (matching
agentMcpConnections and serviceProviderConnections behavior). Only the
MSI role assignment logic remains guarded by isAgentWorkflow().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR Validation Error

An error occurred while validating your PR. Please try again later or contact the maintainers.

Error: API request failed with status 503

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(designer): Serialize non-managed MCP server connections to connections.json (#9296) [hotfix/v5.970]
  • Issue: None — the title is specific, descriptive, and accurately reflects the change.
  • Recommendation: No change needed.

Commit Type

  • Properly selected (fix).
  • Only one commit type is selected, which is correct.

⚠️ Risk Level

  • The PR is labeled risk:medium, which matches the selected body value (Medium).
  • Advised risk level from the diff: medium.
  • No mismatch detected.

What & Why

  • Current: Clear and sufficiently detailed.
  • Issue: None.
  • Recommendation: No change needed.

Impact of Change

  • The impact section is well-formed and explains user/developer/system impact appropriately.
  • Recommendation:
    • Users: Good as written.
    • Developers: Good as written.
    • System: Good as written.

Test Plan

  • Unit tests were added/updated in the diff, which satisfies the test-plan requirement.
  • The note referencing the original PR is fine; automated tests are present in the change set.

Contributors

  • Contributor credit is present.
  • No issue.

Screenshots/Videos

  • Marked as not applicable, which is appropriate for a non-visual change.

Summary Table

Section Status Recommendation
Title No changes needed
Commit Type No changes needed
Risk Level No changes needed
What & Why No changes needed
Impact of Change No changes needed
Test Plan No changes needed
Contributors No changes needed
Screenshots/Videos No changes needed

This PR passes validation. The advised risk level matches the submitter’s estimate.


Last updated: Fri, 26 Jun 2026 20:13:55 GMT

@github-actions

Copy link
Copy Markdown
Contributor

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/core/mcp/utils/helper.ts - 46% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/mcp/utils/serializer.ts - 16% covered (needs improvement)
⚠️ libs/designer/src/lib/core/mcp/utils/helper.ts - 43% covered (needs improvement)
⚠️ libs/designer/src/lib/core/mcp/utils/serializer.ts - 19% covered (needs improvement)

Please add tests for the uncovered files before merging.

@Elaina-Lee Elaina-Lee added the risk:medium Medium risk change with potential impact label Jun 26, 2026
@lambrianmsft lambrianmsft merged commit cf4e9db into hotfix/v5.970 Jun 27, 2026
35 of 39 checks passed
@lambrianmsft lambrianmsft deleted the elaina/hotfix-9296 branch June 27, 2026 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants