fix(deploy): compare edge handles by port, not by spelling

Two places answer "does this need redeploying?" and they load their sides
differently. The client diffs the live store against `/api/workflows/[id]/deployed`;
the server diffs the normalized tables against the version's raw jsonb. Only
some of those paths run handles through `loadWorkflowFromNormalizedTables`, so a
snapshot holding a side-anchored id (`source-right`) met a canonical one
(`source`) on the other side and the set comparison read it as every edge being
removed and re-added.

Each answer therefore differed, and they arrive on separate query timelines: the
button reads the client's, the modal badge reads the server's, so the state
flipped between Live and "Update deployment" with whichever query landed last
until both settled.

`normalizeEdge` now canonicalizes both handles, so the comparison cannot tell
two spellings of one port apart no matter how its inputs were loaded. The
existing normalization in `materializeDeploymentState` stays — that path also
feeds React Flow, which needs the handle it mounts to match.

The preview's error port had the mirror problem: it rendered for every
non-trigger block regardless of `errorEnabled`, so a card with no error row grew
a red knob anyway. It now gates the way the editor canvas does, keeping the port
mounted when an error edge already leaves it so React Flow cannot drop that edge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vikhyath Mondreti
2026-08-08 18:02:32 -07:00
parent aa2139ae0f
commit 41b68048a5
4 changed files with 86 additions and 5 deletions
@@ -83,6 +83,14 @@ interface WorkflowPreviewBlockData {
executionStatus?: ExecutionStatus
/** Subblock values from the workflow state */
subBlockValues?: Record<string, SubBlockValueEntry | unknown>
/**
* Whether the block routes its failures to a second output. The port is the
* rendered half of that choice, so it only exists when the choice was made —
* or when an edge already leaves it, which React Flow needs a mounted handle
* for whatever the flag says.
*/
errorEnabled?: boolean
hasErrorConnection?: boolean
/** Skips expensive subblock computations for thumbnails/template previews */
lightweight?: boolean
}
@@ -222,6 +230,8 @@ function WorkflowPreviewBlockInner({ data }: NodeProps<WorkflowPreviewBlockData>
isPreviewSelected = false,
executionStatus,
subBlockValues,
errorEnabled = false,
hasErrorConnection = false,
lightweight = false,
} = data
@@ -653,7 +663,11 @@ function WorkflowPreviewBlockInner({ data }: NodeProps<WorkflowPreviewBlockData>
/>
)}
{shouldShowDefaultHandles && type !== 'response' && (
{/* The editor canvas gates this the same way — the port is the rendered
half of the error-output toggle, and a card that never opted in should
not grow one. An existing error edge keeps it mounted regardless, or
React Flow would drop that edge for having no handle to leave from. */}
{shouldShowDefaultHandles && type !== 'response' && (errorEnabled || hasErrorConnection) && (
<Handle
type='source'
position={Position.Bottom}
@@ -685,6 +699,8 @@ function shouldSkipPreviewBlockRender(
prevProps.data.enabled !== nextProps.data.enabled ||
prevProps.data.isPreviewSelected !== nextProps.data.isPreviewSelected ||
prevProps.data.executionStatus !== nextProps.data.executionStatus ||
prevProps.data.errorEnabled !== nextProps.data.errorEnabled ||
prevProps.data.hasErrorConnection !== nextProps.data.hasErrorConnection ||
prevProps.data.lightweight !== nextProps.data.lightweight
) {
return false
@@ -371,10 +371,28 @@ export function PreviewWorkflow({
}
}, [workflowState.edges, isValidWorkflowState])
/**
* Blocks that already route failures somewhere, as a content key.
*
* Such a block keeps its error port whatever its own flag says — React Flow
* drops an edge whose handle never mounts. A string rather than a Set so a
* re-received edges array with the same content does not rebuild every node.
* The raw handle is safe to test: `normalizeWorkflowEdgeHandles` only rewrites
* the side-anchored source ids, never `error`.
*/
const errorSourceBlockKey = useMemo(() => {
const ids = new Set<string>()
for (const edge of workflowState.edges ?? []) {
if (edge.sourceHandle === 'error') ids.add(edge.source)
}
return [...ids].sort().join(',')
}, [workflowState.edges])
const nodes: Node[] = useMemo(() => {
if (!isValidWorkflowState) return []
const nodeArray: Node[] = []
const blocksWithErrorEdge = new Set(errorSourceBlockKey ? errorSourceBlockKey.split(',') : [])
const sortedBlocks = Object.entries(workflowState.blocks || {}).sort(
([, left], [, right]) =>
@@ -466,6 +484,8 @@ export function PreviewWorkflow({
isPreviewSelected: isSelected,
executionStatus,
subBlockValues: block.subBlocks,
errorEnabled: block.errorEnabled === true,
hasErrorConnection: blocksWithErrorEdge.has(blockId),
lightweight,
},
})
@@ -483,6 +503,7 @@ export function PreviewWorkflow({
getSubflowExecutionStatus,
workflowMap,
workflowLabelsReady,
errorSourceBlockKey,
lightweight,
])
@@ -193,6 +193,33 @@ describe('hasWorkflowChanged', () => {
expect(hasWorkflowChanged(state1, state2)).toBe(true)
})
/**
* The two sides of a redeploy check are loaded by different paths, and only
* some of them canonicalize handles: the server diffs the normalized tables
* against the version's raw jsonb. A side-anchored spelling surviving on one
* side alone read as every edge being removed and re-added, so the button
* said "Live" while the modal said "Update deployment".
*/
it.concurrent('should treat a side-anchored source handle as its canonical id', () => {
const state1 = createWorkflowState({
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source-right', target: 'block2' }],
})
const state2 = createWorkflowState({
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source', target: 'block2' }],
})
expect(hasWorkflowChanged(state1, state2)).toBe(false)
})
it.concurrent('should still tell two real ports apart', () => {
const state1 = createWorkflowState({
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source', target: 'block2' }],
})
const state2 = createWorkflowState({
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'error', target: 'block2' }],
})
expect(hasWorkflowChanged(state1, state2)).toBe(true)
})
it.concurrent('should ignore edge ID changes', () => {
const state1 = createWorkflowState({
edges: [{ id: 'edge-old', source: 'block1', target: 'block2' }],
+21 -4
View File
@@ -3,6 +3,10 @@
* Used by both client-side signature computation and server-side comparison.
*/
import {
normalizeWorkflowEdgeSourceHandle,
normalizeWorkflowEdgeTargetHandle,
} from '@sim/workflow-types/workflow'
import type { Edge } from 'reactflow'
import { isNonEmptyValue } from '@/lib/workflows/subblocks/visibility'
import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks'
@@ -305,6 +309,17 @@ interface NormalizedEdge {
/**
* Normalizes an edge by extracting only the connection-relevant fields.
* Treats null and undefined as equivalent (omits the field if null/undefined).
*
* Handles are canonicalized here rather than by each caller, because the two
* places that ask "does this need redeploying?" load their sides differently:
* the client diffs the live store against the `/deployed` response, while the
* server diffs the normalized tables against the version's raw jsonb — and only
* some of those paths pass through `loadWorkflowFromNormalizedTables`. A
* side-anchored id surviving on one side alone read as every edge being removed
* and re-added, so the button said "Live" while the modal said "Update
* deployment" and the badge flipped with whichever query landed last. Both are
* spellings of one port, so the comparison must not be able to tell them apart.
*
* @param edge - The edge object
* @returns Normalized edge with only connection fields
*/
@@ -313,13 +328,15 @@ export function normalizeEdge(edge: Edge): NormalizedEdge {
source: edge.source,
target: edge.target,
}
const sourceHandle = normalizeWorkflowEdgeSourceHandle(edge.sourceHandle)
const targetHandle = normalizeWorkflowEdgeTargetHandle(edge.targetHandle)
// Only include handles if they have a non-null value
// This treats null and undefined as equivalent (both omitted)
if (edge.sourceHandle != null) {
normalized.sourceHandle = edge.sourceHandle
if (sourceHandle != null) {
normalized.sourceHandle = sourceHandle
}
if (edge.targetHandle != null) {
normalized.targetHandle = edge.targetHandle
if (targetHandle != null) {
normalized.targetHandle = targetHandle
}
return normalized
}