Skip to content

fix(designer): Route Standard built-in MCP through manifest path + v2 save-flow reliability#9346

Open
Elaina-Lee wants to merge 21 commits into
mainfrom
fix/mcp-serialize-connection-reference-standard
Open

fix(designer): Route Standard built-in MCP through manifest path + v2 save-flow reliability#9346
Elaina-Lee wants to merge 21 commits into
mainfrom
fix/mcp-serialize-connection-reference-standard

Conversation

@Elaina-Lee

@Elaina-Lee Elaina-Lee commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Commit Type

  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • test - Test-related changes

Risk Level

  • Medium - Moderate changes, some user impact

What & Why

Built-in MCP tools on Standard were emitting inputs.Connection.{McpServerUrl,Authentication} inline — a shape PR #8953 introduced for Consumption but that leaked into Standard. The Standard backend expects inputs.connectionReference.connectionName referencing a key in connections.agentMcpConnections (the original v2 design from PR #8525). This caused save/validate failures on Standard, most notably when the user changed the tool's connection to a different existing one, and later surfaced two Standalone-side races that only affected designer v2's save flow.

The core fixes (serializer)

1. Route Standard through the manifest path (libs/designer{,-v2}/src/lib/core/actions/bjsworkflow/serializer.ts)

OperationManifestService.isSupported('McpClientTool', 'BuiltIn') returns true for both SKUs, so the built-in MCP intercept was doing the same work serializeManifestBasedOperation would have done for Standard — but forcing a Consumption-only inline Connection shape. Narrow the intercept to Consumption (renamed to isConsumptionBuiltInMcpClient, folded with the !rootState.workflow.workflowKind check), and let Standard fall through to the manifest branch. The manifest's referenceKeyFormat: 'mcpconnection' (see builtinmcpclient.ts) drives serializeHost to emit inputs.connectionReference.connectionName — the shape the Standard backend resolves against connections.agentMcpConnections.

serializeBuiltInMcpOperation was renamed to serializeConsumptionBuiltInMcpOperation with a header comment stating it's Consumption-only.

2. Defense in serializeHost's McpConnection case

The changeConnectionMapping reducer calls getExistingReferenceKey to reuse an existing referenceKey when the picker payload matches an existing MCP reference. That match can fail (root cause likely a difference in connectionProperties between the pre-loaded MCP reference and the picker's constructed payload), causing Redux to mint a fresh referenceKey (e.g. mcpclient-9) that has no matching entry in connections.agentMcpConnections. If the serializer emitted that minted name, the backend's eager MCP lookup would fail validate.

Extracted the resolution into a small exported helper — resolveMcpConnectionName(referenceKey, connectionId) — that prefers connection.id's last segment (by construction the agentMcpConnections key — see convertMcpConnectionDataToConnection in standard/connection.ts:807) and falls back to referenceKey when the id is missing or produces an empty segment (trailing slash). Called from serializeHost's McpConnection case in both v1 and v2.

Standalone-side v2 save-flow fixes

Once the serializer emitted correct names, two Standalone v2 issues remained that were hiding behind the earlier shape bug.

3. Drop the post-save invalidateQueries (laDesignerV2.tsx)

After every save, v2 called invalidateQueries(['workflowArtifactsStandard', workflowId]). Because the workflow query is actively mounted during editing, invalidation triggers an immediate async refetch. When the refetch completes, data.properties.files reference changes, cascading through the originalConnectionsDataconnectionsData useMemos — the derived connectionsData is rebuilt as a fresh clone from server state. Any in-flight addConnectionInJson mutation for a subsequent new connection is wiped by the recomputation.

Symptom: user saves with mcpclient-N included, creates mcpclient-(N+1), autosave fires, but getConnectionsToUpdate sees no delta → returns undefined → deploy sends no connections.json → validate later fails with InvalidActionToolMcpConnection.

The stated goal ("next load fetches fresh data") is a no-op anyway because useWorkflowAndArtifactsStandard sets refetchOnMount: false; the invalidation only ever fired a refetch when the component was still mounted, which is exactly the race we're removing.

4. Hold connectionsData in state and merge on server updates (laDesignerV2.tsx)

After (3), existing workflows worked but brand new workflows still failed on the "user creates new MCP connection" flow. Root cause was the same object-replacement race but during the initial workflow-artifacts fetch:

  1. Mount. Fetch pending, data = undefined, originalConnectionsData = {}, connectionsData useMemo → {}.
  2. User creates mcpclient-N via wizard. addConnectionInJson mutates the current connectionsData in place.
  3. Fetch completes. data reference changes → originalConnectionsData useMemo recomputes → connectionsData useMemo recomputes → fresh clone from server data, mutation wiped.
  4. Save fires. Delta detection sees no diff → undefined returned → deploy skips connections.json → validate fails.

Existing workflows escape this because useWorkflowAndArtifactsStandard cached data is available synchronously on mount, so there's no fetch to race with.

Fix: promote connectionsData to useState. On any dep change, MERGE the server-computed version with the previous state via a module-level mergeConnectionsData helper — local per-category entries win over server. New connections mutated into prev survive the reconciliation regardless of when the fetch completes.

Why we removed the newAgentMcpConnections block from laDesigner{,V2}.tsx (PR #9296 cleanup)

PR #9296 added a branch inside the save flow's reference-remapping loop that mirrored the pattern used for managed API / service provider / agent categories:

} else if (reference?.connection?.id.startsWith('/connectionProviders/mcpclient/')) {
  const connectionKey = reference.connection.id.split('/').splice(-1)[0];
  newAgentMcpConnections[referenceKey] = connectionsData?.agentMcpConnections?.[connectionKey];
  delete connectionsData?.agentMcpConnections?.[connectionKey];
}

The pattern's purpose is a key remap on connections.json: move an entry from being keyed by the raw connection name to being keyed by the Redux referenceKey, because managed API / service provider / agent tool refs are emitted using the referenceKey (via @parameters('$connections')['<referenceKey>'] or manifest paths).

For MCP this branch was dead code and semantically wrong:

  • It could never execute. The outer loop iterates Object.keys(workflowFromDesigner.connectionReferences), but getConnectionsDataToSerialize (serializer.ts:172-181) explicitly excludes built-in MCP operations from the emitted connectionReferences map. There are no MCP entries for the loop to see.
  • Remapping would actively break MCP. agentMcpConnections is keyed by the raw connection name on the server (convertMcpConnectionDataToConnection builds connection.id as /${mcpclientConnectorId}/connections/${connectionKey}) and the backend eagerly validates tool refs against those exact keys. If the branch had fired and remapped agentMcpConnections["mcpclient-8"]agentMcpConnections["mcpclient-8_1"] to match a Redux-uniquified referenceKey, backend lookup would fail. The raw connection name is the authoritative key for MCP — that's what the serializer defense above emits, and that's what should stay in connections.json.

The same .split('/').splice(-1)[0] string operation appears in the serializer defense (serializeHost McpConnection case) but for the opposite goal: extract the raw name and emit it in the tool ref so it matches the connections.json key that was never remapped. The laDesigner branch was making Redux the source of truth; the serializer defense makes connection.id the source of truth. For MCP, connection.id must win.

Test coverage additions

Unit tests (resolveMcpConnectionName.spec.ts in v1 and v2) — six cases each locking in the McpConnection defense:

  • Divergent referenceKey vs connection.id → last segment wins (the core defense)
  • Happy path (they agree)
  • Undefined / empty connectionId → fallback to referenceKey
  • Trailing-slash connectionId → fallback (spec caught a ?? vs || bug during authoring; fixed by the same commit)
  • Connection.id without a leading slash → still resolves last segment

E2E tests (e2e/designer/mcpSerialization.spec.ts):

  • Existing "serialize an Agent workflow ..." test extended: Managed MCP now asserts connectionReference.connectionName === 'office365' AND inputs.parameters.mcpServerPath === '/mcp/contacts'. Previously regressions on the swagger-path injection in serializeManagedMcpOperation would have been silent.
  • New Consumption test (v1): toggles Plan to Consumption, loads AgentWithMcpConsumption.json, asserts inline Connection shape.
  • New v2 mirror for Standard shape (Should serialize Standard built-in MCP as connectionReference on designer-v2) — catches drift between the two packages' serializers.
  • New v2 mirror for Consumption shape — catches regressions on serializeConsumptionBuiltInMcpOperation in the v2 package specifically.

Consumption fixture (__mocks__/workflows/AgentWithMcpConsumption.json) — inline Connection shape with $connections.value structure, wired into LogicAppSelector under Agentic Workflows.

Standalone mergeConnectionsData is not unit-tested because Standalone is outside the vitest workspace (vitest.workspace.ts lists only libs/); E2E + manual HAR coverage stands in.

Other cleanup

  • __mocks__/workflows/AgentWithMcp.json — flipped the built-in MCP tool's inputs.Connection (pre-fix buggy Standard shape) to inputs.connectionReference.connectionName (correct Standard shape). Round-trip previously appeared to work because the mock and the pre-fix buggy serializer agreed on the wrong shape.

Impact of Change

  • Users: Standard workflows with built-in MCP tools save and validate reliably, including when the tool's connection is changed to a different existing one AND when a brand new connection is created mid-fetch on a fresh workflow. Consumption behavior is unchanged.
  • Developers: Serializer dispatch is cleaner and self-documenting via explicit SKU-scoped names (isConsumptionBuiltInMcpClient, serializeConsumptionBuiltInMcpOperation). Standard reuses the manifest infrastructure like every other operation type. Standalone v2's connectionsData state model is now robust to in-flight fetch/mutation races.
  • System: No new client-side workarounds in the workflow save flow (fullConnectionsForValidation was dropped once the shape bug was fixed at source). Backend validate works with the delta payload as it does for all other connection categories.

Test Plan

  • Unit tests added: resolveMcpConnectionName covered in both libs/designer-v2/.../__test__/resolveMcpConnectionName.spec.ts and libs/designer/.../__test__/resolveMcpConnectionName.spec.ts — 6 cases each.
  • E2E tests added/updated: e2e/designer/mcpSerialization.spec.ts — 7 tests total covering Standard v1, Standard v2, Consumption v1, Consumption v2, connection-reference exclusion, tool preservation, and UAMI identity round-trip.
  • Manual testing across HARs 4, 9, 12, 13, 14, 15, 17 covering:
    • Standard existing workflow + existing MCP connection (no-op save)
    • Standard existing workflow + change to different existing MCP connection
    • Standard existing workflow + create new MCP connection then save
    • Standard new workflow + create new MCP connection mid-fetch then save (the v2-only race)
    • Consumption round-trip via mock fixture

Follow-ups (not in this PR)

  • Root-cause fix in getExistingReferenceKey (libs/designer-v2/.../utils/connectors/connections.ts) so it correctly matches the picker payload against pre-loaded MCP references. Once fixed, resolveMcpConnectionName becomes redundant and can be removed.
  • If backend eager validation ever surfaces the same latent bug for agent tools (inputs.parameters.modelConfigurations.model1.referenceName), the fix would live at getConnectionsToUpdate (libs/designer-v2/.../utils/createhelper.ts) — split into delta-for-deploy vs full-for-validate callers.
  • v1 (laDesigner.tsx) currently doesn't exhibit the same connectionsData race in practice, but shares the same useMemo derivation shape. If v1 turns out to hit the bug, the state + merge pattern from (4) can be ported over as-is.

Screenshots

v1 with existing mcp connection
Screenshot 2026-07-01 151215

v1 with new mcp connection
Screenshot 2026-07-01 152115

v2 with existing mcp connection
Screenshot 2026-07-01 152250

v2 with new mcp connection
Screenshot 2026-07-01 154436

Elaina-Lee and others added 9 commits June 30, 2026 19:26
…ls on Standard SKU

PR #8953 introduced serializeBuiltInMcpOperation which emits
`inputs.Connection.{McpServerUrl,Authentication}` inline. That PR was
scoped to Consumption ("Add MCP server support for Consumption SKU")
and tested only against a Consumption Logic App, but the shared
serializer applied the new shape to both SKUs.

On Standard, MCP connections live in `connections.agentMcpConnections`
and the deserializer's `getConnectionReferenceKeyForManifest` McpConnection
case reads `inputs.connectionReference.connectionName`. Emitting an inline
`Connection` block instead sends a tool the Standard host runtime cannot
resolve, producing an opaque `BadRequest: Encountered an error
(InternalServerError) from host runtime.` from validate.

Branch by SKU in serializeBuiltInMcpOperation:
- Standard (workflowKind is set) -> emit
  `inputs.connectionReference.connectionName: <last segment of connection.id>`.
  The last segment matches the key in `agentMcpConnections` regardless of
  whether Redux uniquified the reference key when the wizard added the
  connection. This restores the pre-#8953 shape from the original v2 MCP
  support (PR #8525) and matches what shipped in AgentWithMcpUami.json.
- Consumption (no workflowKind) -> keep the inline Connection block from
  PR #8953, with the two-shape lookup extracted into
  `readBuiltInMcpConnectionData` (handles flat `parameterValues` and
  nested `connectionParameters.*.metadata.value`).

Known follow-up: a tool previously saved with the buggy inline Connection
shape reconstructs its connection under `mcp-<nodeId>`, which is not in
`agentMcpConnections`. Re-saving that specific tool will still fail;
delete + re-add via the wizard for now.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ave flow

Now that Standard built-in MCP tools serialize as `inputs.connectionReference.connectionName`,
the shape and follow-on cleanup:

- __mocks__/workflows/AgentWithMcp.json: `BuiltIn_MCP_Server` tool switched to
  `connectionReference` (matches the SKU implied by `"kind": "Stateful"`). This lets the
  round-trip E2E actually exercise the Standard code path instead of the pre-#8953 inline
  shape.
- e2e/designer/mcpSerialization.spec.ts: flip the built-in assertions to expect
  `inputs.connectionReference.connectionName` and explicitly assert `inputs.Connection` is
  absent so we notice if the Consumption shape leaks into Standard again.
- apps/Standalone/.../laDesigner.tsx + laDesignerV2.tsx: remove the
  `else if (reference?.connection?.id.startsWith('/connectionProviders/mcpclient/'))` branch
  added by PR #9296. `serializeWorkflow` already excludes built-in MCP references from
  `workflowFromDesigner.connectionReferences` (see serializer.ts ~L170-181), so that branch
  was unreachable and its `newAgentMcpConnections` merge was a no-op. The
  `hasNewMcpConnectionKeys` handling inside `getConnectionsToUpdate` stays — that path fires
  through `writeConnection` mutating `connectionsData.agentMcpConnections` and is still needed
  when the wizard adds a new MCP connection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ions refs resolve

`getConnectionsToUpdate` returns a *delta* (undefined when nothing new), correct for
`deployWorkflowArtifacts` which is a partial file write. But that same value was piped
through `saveWorkflowStandard` into `validateWorkflowStandard`, which needs the full
connections object so the backend can resolve `connectionReference.connectionName` lookups.

When an MCP tool references a connection already present on the server (e.g. tool re-edited
in an existing workflow, or picked-existing after a prior draft save), the delta is empty
and the validate payload omits `connections` entirely. Backend responds with
`InvalidActionToolMcpConnection: The connection for MCP tool '<name>' is missing.`

Fix: add a new optional `fullConnectionsForValidation` param to `saveWorkflowStandard`. The
two designer save callsites (laDesigner.tsx + laDesignerV2.tsx) pass the full
`connectionsData` alongside the existing `connectionsToUpdate` delta. Validate uses full,
deploy still uses delta. Other callsites (Templates, code-view save, internal
`skipValidation:true` calls) don't need the new param — default falls back to
`connectionsData`.

Only surfaced because the prior commit switched Standard built-in MCP tool serialization
to `connectionReference.connectionName` (the shape the backend and shared deserializer
expect). With the pre-#8953 inline `Connection` block, the tool was self-contained and
the backend didn't need to resolve via `agentMcpConnections`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… + guard connectionName

Built-in MCP on Standard was intercepted by the isBuiltInMcpClient branch even though the
manifest declares `referenceKeyFormat: 'mcpconnection'` and OperationManifestService.isSupported
returns true for it — meaning it would fall through to serializeManifestBasedOperation naturally.
Narrow the intercept to Consumption (which needs the inline `inputs.Connection` block from
PR #8953), letting Standard reach the manifest branch.

serializeHost's McpConnection case now resolves connectionName via connection.id's last segment
rather than the raw Redux referenceKey. The picker's changeConnectionMapping flow can mint a
fresh referenceKey when getExistingReferenceKey fails to match a pre-loaded MCP reference, which
would otherwise emit a connectionName with no matching entry in connections.agentMcpConnections
and fail backend validate. connection.id's last segment is by construction the agentMcpConnections
key (see convertMcpConnectionDataToConnection). Follow-up: fix the reference-key mismatch at
source and remove the guard.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ewable diff

Prior commits reordered/reformatted unchanged lines inside serializeBuiltInMcpOperation
while extracting the readBuiltInMcpConnectionData helper. Restore the pristine layout so
the diff vs main only shows actual logic changes.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…t SKU-scoped names

The isBuiltInMcpClient predicate hid the fact that only Consumption reaches the branch that
followed it. Fold the SKU check into the predicate and rename to make the scope obvious:

- isBuiltInMcpClient -> isConsumptionBuiltInMcpClient (with the workflowKind check inline)
- serializeBuiltInMcpOperation -> serializeConsumptionBuiltInMcpOperation
- readBuiltInMcpConnectionData -> readConsumptionBuiltInMcpConnectionData
- Corresponding spec files renamed to match

Expand comments on the branch, the helper, and the serializer to spell out why the shape is
Consumption-specific: Consumption stores connections in parameters.$connections.value with no
agentMcpConnections category, so the tool cannot reference a connection by name and must inline
McpServerUrl / Authentication directly. Standard routes through serializeManifestBasedOperation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…izer defense covers the case

The fullConnectionsForValidation param (added in 4483197) forced the validate call to send the
full connections object so the backend could resolve a wrongly-emitted MCP connectionName against
`agentMcpConnections`. With the serializeHost `McpConnection` defense in place, the emitted name
now always matches a real connection.id — either included in the delta (when new keys exist) or
resolvable server-side from disk (when validate omits the `connections` field entirely). HAR-12
confirms validate succeeds with no connections in the payload when the emitted name is correct.

Reverting keeps the dead-code cleanup from PR #9296 (removal of the unreachable mcpclient remap
branch and `newAgentMcpConnections` local) which is orthogonal to the validate fix.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tion

The helper's second code path (nested `connectionParameters.metadata.value`) was added to serve
the Standard SKU when it still routed through this function. Standard now goes through the
manifest path (serializeManifestBasedOperation) and never calls the helper. Consumption's
`ConnectionService().getConnection()` only ever produces the flat `parameterValues` shape
(consumption/connection.ts:194), so the fallback path was dead code.

Restore the original inline `parameterValues` block, delete the helper, the McpConnectionData
interface, the `Connection` type import, and both spec files. Consumption behavior is unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…n round-trip

Adds a fixture and E2E test that lock down the Consumption serialization path for built-in MCP
tools (inline `inputs.Connection.{McpServerUrl,Authentication}`). Previously the fixture-based
E2E only exercised Standard, so a regression on serializeConsumptionBuiltInMcpOperation would
have gone uncaught.

- __mocks__/workflows/AgentWithMcpConsumption.json: workflow with the inline Consumption shape
  and top-level parameters.$connections.value structure.
- LogicAppSelector.tsx: wire the fixture into the Agentic Workflows menu.
- localDesigner.tsx / localDesignerV2.tsx: pass `kind: isConsumption ? undefined : workflowKind`
  to BJSWorkflowProvider. Real Consumption workflows have no `kind`; Standalone was always
  defaulting to 'stateful', which forced the Standard path in serializer SKU checks. Unsetting
  when Plan=Consumption is a small Standalone-only fidelity fix that also enables this test.
- mcpSerialization.spec.ts: new test toggles Plan to Consumption, loads the fixture, asserts
  `inputs.Connection.McpServerUrl` is preserved and `inputs.connectionReference` is undefined.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 1, 2026 21:39
@github-actions

github-actions Bot commented Jul 1, 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): Route Standard built-in MCP through manifest path + v2 save-flow reliability
  • Issue: None. The title is specific, descriptive, and clearly explains the user impact.
  • Recommendation: No change needed.

Commit Type

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

Risk Level

  • The selected risk level (Medium) matches the diff and the risk:medium label.
  • No mismatch found between the PR body and labels.

What & Why

  • Current: A detailed explanation of the serializer issue, Standalone save-flow races, and the resulting fixes.
  • Issue: None — this is thorough and informative.
  • Recommendation: No change needed.

Impact of Change

  • The impact section is filled in clearly and aligns with the code changes.
  • Recommendation:
    • Users: Good as written.
    • Developers: Good as written.
    • System: Good as written.

Test Plan

  • Test coverage is present in the diff: unit tests and E2E tests were added/updated, and manual testing is documented.
  • This satisfies the required test-plan criteria.

⚠️ Contributors

  • No contributors were listed.
  • Recommendation: If PMs, designers, or other engineers contributed ideas or validation, please add them here for credit. This is optional, but helpful.

Screenshots/Videos

  • Screenshots were provided and match the UI-facing nature of the change.
  • No action needed.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type Only one type selected (fix)
Risk Level Matches risk:medium label
What & Why No change needed
Impact of Change No change needed
Test Plan No change needed
Contributors ⚠️ Optional: add credit if applicable
Screenshots/Videos No change needed

Overall, this PR passes review for title/body compliance. The advised risk level remains medium and does not need to be raised.


Last updated: Thu, 02 Jul 2026 00:04:53 GMT

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Standard SKU built-in MCP tool serialization by routing it through the manifest-driven path (emitting inputs.connectionReference.connectionName), while keeping Consumption SKU behavior unchanged (inline inputs.Connection.{McpServerUrl,Authentication}), and adds an E2E regression test + fixture for the Consumption shape. Also improves Standalone SKU fidelity (unset kind for Consumption) and removes unreachable MCP remap code in Standalone’s Azure designer wrappers.

Changes:

  • Update v1/v2 workflow serializer dispatch so built-in MCP tools use a Consumption-only serializer; Standard falls through to manifest-based serialization.
  • Harden Standard MCP host serialization to derive connectionName from the MCP connection id’s last segment (authoritative agentMcpConnections key).
  • Add Consumption mock workflow + E2E coverage; update Standard MCP expectations and make Standalone pass kind: undefined for Consumption.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Localize/lang/strings.json Removes unused localization entries related to workflow-name collision/reserved-name messages.
libs/designer/src/lib/core/actions/bjsworkflow/serializer.ts Consumption-only built-in MCP serialization; Standard uses manifest path; MCP host serialization resolves connectionName from connection id.
libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts Same serializer routing + MCP host connectionName resolution as v1.
e2e/designer/mcpSerialization.spec.ts Updates Standard assertions to connectionReference shape; adds Consumption test asserting inline Connection shape is preserved.
apps/Standalone/src/designer/app/LocalDesigner/LogicAppSelector/LogicAppSelector.tsx Adds new Consumption MCP fixture to the Local Designer workflow dropdown.
apps/Standalone/src/designer/app/LocalDesigner/localDesignerV2.tsx Unsets kind when hostingPlan is Consumption to match real Consumption workflows.
apps/Standalone/src/designer/app/LocalDesigner/localDesigner.tsx Same as above for v1 Local Designer.
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesignerV2.tsx Removes dead/unreachable remap logic for agentMcpConnections.
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesigner.tsx Removes dead/unreachable remap logic for agentMcpConnections.
mocks/workflows/AgentWithMcpConsumption.json Adds Consumption fixture with inline inputs.Connection MCP shape.
mocks/workflows/AgentWithMcp.json Updates Standard MCP built-in tool fixture to use inputs.connectionReference.

The previous shape was missing metadata.agentType and used deploymentId instead of the
required agentModelType/agentModelSettings, so the Agent action failed to initialize. Base
the fixture on a known-good Consumption Agent+MCP workflow: metadata.agentType='autonomous',
agent parameters with agentModelType and agentModelSettings, and $connections defaultValue.

Also update the E2E assertion URL to match the fixture's MCP server URL.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Loading AgentWithMcpConsumption.json required the user to manually toggle Plan to Consumption
in the settings box first — otherwise localDesigner passed `kind: 'stateful'` to
BJSWorkflowProvider and the serializer took the Standard path, emitting connectionReference
instead of the expected inline Connection block.

Support a top-level `hostingPlan` field in mock files: workflowLoadingSlice's loadWorkflow reads
it and its fulfilled reducer applies it to state.hostingPlan. The Consumption MCP fixture opts
in with `"hostingPlan": "consumption"`, so selecting it from the menu now correctly renders and
serializes as Consumption without any pre-toggle.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/core/actions/bjsworkflow/serializer.ts - 37% covered (needs improvement)
⚠️ libs/designer/src/lib/core/actions/bjsworkflow/serializer.ts - 17% covered (needs improvement)

Please add tests for the uncovered files before merging.

…nd gate

Reverting two pieces that made the Consumption test easier to reach but touched Standalone
plumbing:

- localDesigner{,V2}.tsx: restore `kind: workflowKind` (drop the `isConsumption ? undefined`
  gate). Keeping Standalone's default workflowKind='stateful' behavior unchanged.
- workflowLoadingSlice.ts: drop the top-level `hostingPlan` field read + dispatch on mock load.

To reach the Consumption serializer path from the mock, the user (or the E2E) toggles Plan to
Consumption in the settings box before selecting the fixture. The E2E test does this via
`page.getByRole('radio', { name: 'Consumption' }).click()` before GoToMockWorkflow.

Also updated the Consumption fixture's McpServerUrl and the E2E's asserted URL to
https://mcp.time.mcpcentral.io/.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Elaina-Lee Elaina-Lee changed the title fix(designer): Route Standard built-in MCP tools through manifest path (+ Consumption test) fix(designer): Route Standard built-in MCP tools through the manifest path + Consumption E2E Jul 1, 2026
…local getConnectionsToUpdate

Same root reason as removing the newAgentMcpConnections remap block from the save flow: the main
workflow save flow never mutates connectionsData.agentMcpConnections. Built-in MCP references are
excluded from the outer serialize loop by getConnectionsDataToSerialize (serializer.ts:172), so
nothing on this code path adds keys to agentMcpConnections. That means:

- hasNewKeys(original.agentMcpConnections, current.agentMcpConnections) is always false
  because current === original
- !hasNewMcpConnectionKeys in the early-return check is always true (redundant)
- The `if (hasNewMcpConnectionKeys) { for ... overwrite ... }` block never executes

PR #9296 cargo-culted this from the pattern used for managed API / service provider / agent,
which do get mutated in the outer loop via delete + newFoo[referenceKey] = .... MCP had a
matching remap that we already removed as dead code; this hunk is the same dead-code partner.

The library-side `getConnectionsToUpdate` in libs/designer-v2/.../createhelper.ts keeps its
agentMcpConnections handling because it IS called from the MCP Tool Wizard flow
(serializeMcpWorkflows) that does add new MCP entries. This cleanup only touches the local
Standalone copies used by the main workflow save.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ng from local getConnectionsToUpdate"

This reverts commit e84625d.
…to unblock new-connection races

v2 fired invalidateQueries(['workflowArtifactsStandard', workflowId]) after every save. Because
the workflow query is actively mounted during editing, invalidation triggers an immediate async
refetch. When the refetch completes, `data.properties.files` reference changes, cascading through
the `originalConnectionsData` and `connectionsData` useMemos — connectionsData gets rebuilt as a
fresh clone from server state. Any in-flight mutation to connectionsData from a subsequent new
connection (via addConnectionInJson, which mutates in place) is wiped by the recomputation.

Symptom: user saves with mcpclient-N included, creates mcpclient-(N+1), autosave fires, but
getConnectionsToUpdate sees no delta (mutation lost) → returns undefined → deploy sends no
connections.json → validate later fails with InvalidActionToolMcpConnection.

v1 (laDesigner.tsx) doesn't have this invalidation and works correctly for the same flow.
Remove the call and its now-unused workflowId dep from the useCallback. The stated goal ("next
load fetches fresh data") is a no-op anyway because `useWorkflowAndArtifactsStandard` sets
`refetchOnMount: false`; invalidation would only fire a refetch when the component is currently
mounted, which is exactly the racing case we're fixing.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ta so mutations survive fetch

Previous useMemo-derived connectionsData raced with initial workflow-artifacts fetch for a
brand-new workflow: user creates an MCP connection while the fetch is in flight, addConnectionInJson
mutates connectionsData in place, then the fetch completes → data reference changes →
originalConnectionsData useMemo recomputes → connectionsData useMemo recomputes with a fresh clone
from server data → in-flight mutation is wiped. Next save sees no delta → connections.json omitted
from deploy → validate fails on missing agentMcpConnections entry.

Hold connectionsData in useState. On any dep change (originalConnectionsData, currentParameters,
settingsData?.properties), MERGE server data with the previous state instead of replacing —
per-category with local entries winning over server. Newly created connections mutated into
`prev` survive the reconciliation because the merge preserves them alongside whatever server
data has arrived.

Only affects laDesignerV2.tsx; v1 (laDesigner.tsx) is left as-is since it doesn't exhibit the
same behavior in practice for the new-workflow flow. If v1 turns out to hit the same race, the
same pattern can be applied there.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Elaina-Lee and others added 3 commits July 1, 2026 16:11
The Consumption/manifest routing, McpConnection defense, and connectionsData state+merge blocks
had multi-line explanations that duplicated the PR body. Trim each to one or two lines: what the
code does + the load-bearing "why" + a TODO where applicable. The full narrative stays in the PR
description for future readers of git blame.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… McpConnection defense

Extract the connection.id-last-segment resolution from serializeHost's McpConnection case into a
small exported helper `resolveMcpConnectionName` (v1 and v2). Adds a spec covering the divergent
referenceKey vs connection.id case (the core defense), the happy path, empty/undefined connectionId
fallback, trailing-slash fallback, and connection.id without a leading slash. This locks in the
defense against regressions if someone later reverts to `connectionName: referenceKey`.

Also fixes a subtle bug the spec uncovered: `.pop()` on a trailing-slash id returns `''`, which
`??` treats as defined. Switched to `||` so empty segments fall through to the referenceKey.

Drops the "TODO: fix at source" comment from the McpConnection case; the PR description already
tracks the follow-up in getExistingReferenceKey.

Standalone `mergeConnectionsData` is not unit-tested because Standalone is outside the vitest
workspace (`vitest.workspace.ts` lists only `libs/`); E2E + manual HAR coverage stands in.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ged parameters

Adds three E2E gaps as regression protection:

- `Should serialize an Agent workflow ...` extended to assert Managed MCP's connectionName is
  `office365` AND `inputs.parameters.mcpServerPath` is `/mcp/contacts`. Previously only checked
  that connectionReference existed, missing regressions on serializeManagedMcpOperation's swagger
  path injection.
- `Should serialize Standard built-in MCP as connectionReference on designer-v2` — mirrors the v1
  Standard shape assertion on the /v2 route. Catches divergence between the two packages'
  serializers.
- `Should preserve inline Connection shape for built-in MCP tool on Consumption (designer-v2)` —
  mirrors the v1 Consumption test on /v2. Catches regression on serializeConsumptionBuiltInMcpOperation
  in the v2 package specifically.

Adds `getSerializedWorkflowFromStateV2` helper in designerFunctions.ts using DesignerStoreV2 +
DesignerModuleV2 globals (set by libs/designer-v2/src/lib/index.tsx in dev builds).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Elaina-Lee Elaina-Lee changed the title fix(designer): Route Standard built-in MCP tools through the manifest path + Consumption E2E fix(designer): Route Standard built-in MCP through manifest path + v2 save-flow reliability Jul 1, 2026
@Elaina-Lee Elaina-Lee added risk:medium Medium risk change with potential impact and removed needs-pr-update labels Jul 1, 2026
Elaina-Lee and others added 2 commits July 1, 2026 16:35
…lization validates

Flipping the built-in MCP tool from inline `inputs.Connection` to `inputs.connectionReference.connectionName`
required the referenced `mcpclient` key to actually exist in `connectionReferences`. The inline shape had
been synthesized on the fly by getConnectionsApiAndMapping (see connections.ts:558), so an empty top-level
`connectionReferences: {}` was fine before the flip. After the flip, `isConnectionReferenceValid` for
built-in MCP requires `reference && !reference.connection.id.startsWith('__MOCK')` (connections.ts:77-79),
and an undefined reference sets an ErrorLevel.Connection on the tool — which serializeWorkflow rejects with
`Workflow has invalid connections on the following operations: BuiltIn_MCP_Server`.

Add a real `mcpclient` reference to the mock. Managed MCP's `office365` reference was already fine (it uses
a different manifest path that tolerates a missing local reference).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…force Consumption click

Two E2E failures still remaining after the previous mcpclient reference addition:

1. Standard tests still saw `Workflow has invalid connections on the following operations:
   BuiltIn_MCP_Server`. Root cause: workflowLoadingSlice's loadWorkflow reads `wf.connections`
   (line 101), not `wf.connectionReferences`. My previous commit added the mcpclient reference
   under `connectionReferences: {...}` — silently ignored. Rename the key.

2. Consumption tests timed out clicking the "Consumption" radio. The label span was intercepting
   pointer events at the Fluent UI ChoiceGroup layer. Playwright's `{ force: true }` bypasses the
   actionability check and dispatches the click on the resolved radio input directly.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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