The MCP apply_workflow diff reports unchanged connections and nodes as changes

[breaking-brake]

Found while building #1017 (QA loop). The tests in packages/vscode/src/webview/src/utils/workflow-diff.test.ts assert the behaviour as it is today, so they are green — this issue records the defect they document, for the feature track to decide on. No test is skipped.

What happens

computeWorkflowDiff compares serialized form, not meaning, in two places. Both make the confirmation dialog overstate what an incoming apply_workflow will change.

1. Connections: the two key builders disagree on the port default

makeEdgeKey (workflow-diff.ts:19-21) builds the canvas key with edge.sourceHandle ?? ''; makeWorkflowEdgeKey (lines 23-30) builds the incoming key from conn.fromPort verbatim. Nothing normalizes between them — while serializeWorkflow (services/workflow-service.ts:65-66) normalizes a null handle to 'output' / 'input'.

So one unchanged wire counts as 1 added + 1 removed:

• a canvas edge with null handles, versus the same wire arriving as fromPort: "output";
• and beyond the null case, because handle ids differ per node component. StartNode.tsx:58 renders id="out", SubAgentNode.tsx:169,182 renders id="input" / id="output", PromptNode.tsx:122,135 renders id="in" / id="out" — while the AI-authoring guide's examples tell agents to write "fromPort": "output" (packages/core/resources/workflow-schema.json:530, and again at 697 / 1008 / 1095).

A start → subAgent wire is therefore out→input on the canvas and output→input from an agent. That is the most common wire in the product, and it diffs as a change on every apply.

2. Nodes: JSON.stringify(data) is key-order sensitive

modifiedNodes compares JSON.stringify(currentNode.data) against JSON.stringify(node.data) (lines 87-88). Two consequences:

• the same fields declared in a different order read as modified;
• an askUserQuestion node reads as modified whenever an agent omits the option ids, because deserializeWorkflow runs ensureNodeDataItemIds (packages/core/src/utils/node-data-normalize.ts:48) and so every option on the canvas carries a generated id that an agent has no reason to reproduce.

Impact

App.tsx:586 computes this diff for any apply_workflow with requireConfirmation, and DiffPreviewDialog is the only thing the user sees before their canvas is replaced. The failure is one of trust rather than data loss: connection churn and phantom "modified" nodes appear on applies that change nothing, so the counts stop meaning anything and the review gate stops being read. DiffPreviewDialog.tsx:144 also keys its "no changes" message off totalChanges, so a genuine no-op apply is not reported as one.

Nothing is written incorrectly — the diff is a summary only; the apply itself is unaffected.

Suggested direction (feature track's call)

Normalize both sides before comparison rather than changing either builder alone: canonicalize the port default the same way serializeWorkflow does, and compare node data structurally (or strip generated ids) instead of by JSON.stringify. Note the handle ids genuinely differ per node component, so canonicalizing the null case alone does not fix the out vs output case.

Note for whoever fixes this

packages/vscode/src/webview/src/utils/workflow-diff.test.ts pins the current behaviour in four named cases:

• reports one unchanged connection as 1 added + 1 removed when the canvas handles are null
• reports a start→subAgent wire as 1 added + 1 removed because the canvas emits 'out' where an agent writes 'output'
• reports a node as modified when only the key order of its data differs
• reports an askUserQuestion node as modified when the agent omits the generated option ids

Those assertions describe the defect, not the desired behaviour — update them as part of the fix, rather than working around them.

[breaking-brake]

Third instance of the same disagreement — hasUnsavedChanges

Found while building #1020. Recording it here rather than opening a new issue, per this issue's own guidance about the port-default disagreement.

hasUnsavedChanges in packages/vscode/src/webview/src/stores/workflow-store.ts:1503 makes the same serialized-form-versus-meaning comparison as computeWorkflowDiff, with a third default:

savedEdge.fromPort !== (edge.sourceHandle || 'default') ||
savedEdge.toPort !== (edge.targetHandle || 'default')

So the codebase now has three answers for "what is a null handle": '' (makeEdgeKey), 'output' / 'input' (serializeWorkflow), and 'default' (here).

Reachable path

setActiveWorkflow (line 964) copies fromPort verbatim into sourceHandle. A workflow whose connection omits fromPort — which an agent has no reason to emit, since it is not required by the deserializer — therefore loads with sourceHandle: undefined. On the very next call, hasUnsavedChanges reads that back as 'default' and compares it against the saved undefined, which no longer matches.

Net effect: an agent-authored workflow reports unsaved changes the moment it loads, with nothing edited. The user then gets a "discard unsaved changes?" prompt before the next sample-workflow load, for edits they never made.

A workflow loaded from a file is unaffected — deserializeWorkflow fills fromPort in, so both sides match.

Severity

Lower than the two cases in the issue body: a spurious confirmation dialog, not a misleading diff and not data loss. It matters mainly because it shows the normalization fix needs to cover hasUnsavedChanges too — fixing only workflow-diff.ts would leave this path behind.

Test pinning it

packages/vscode/src/webview/src/stores/workflow-store.test.ts (PR #1021) asserts the current behaviour in one named case:

• reports unsaved changes for a just-loaded workflow whose connection omits fromPort (see #1018)

It is green, not skipped — it describes the defect, not the desired behaviour. Update it as part of the fix rather than working around it.

---

Generated by Claude Code