Skip to content

Commit 7f6cc9c

Browse files
committed
fix(workflow): canonicalize realtime edge handles
1 parent 3a8840e commit 7f6cc9c

4 files changed

Lines changed: 120 additions & 47 deletions

File tree

apps/realtime/src/database/operations.ts

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
isKnownWorkflowTriggerBlock,
3333
isWorkflowAnnotationOnlyBlockType,
3434
isWorkflowBlockProtected,
35+
normalizeWorkflowEdgeSourceHandle,
36+
normalizeWorkflowEdgeTargetHandle,
3537
} from '@sim/workflow-types/workflow'
3638
import { and, eq, inArray, isNull, or, sql } from 'drizzle-orm'
3739
import { drizzle } from 'drizzle-orm/postgres-js'
@@ -57,13 +59,21 @@ function toEdgeHandles(edge: PersistedEdgeRecord) {
5759
}
5860

5961
interface EdgeAddCandidate {
60-
id?: string
62+
id: string
6163
source: string
6264
target: string
6365
sourceHandle?: string | null
6466
targetHandle?: string | null
6567
}
6668

69+
function canonicalizeEdgeAddCandidate(edge: EdgeAddCandidate): EdgeAddCandidate {
70+
return {
71+
...edge,
72+
sourceHandle: normalizeWorkflowEdgeSourceHandle(edge.sourceHandle),
73+
targetHandle: normalizeWorkflowEdgeTargetHandle(edge.targetHandle),
74+
}
75+
}
76+
6777
interface FilterEdgesForPersistResult<T> {
6878
safeEdges: T[]
6979
droppedCounts: Record<string, number>
@@ -283,8 +293,8 @@ async function insertAutoConnectEdge(
283293
workflowId,
284294
sourceBlockId: autoConnectEdge.source,
285295
targetBlockId: autoConnectEdge.target,
286-
sourceHandle: autoConnectEdge.sourceHandle || null,
287-
targetHandle: autoConnectEdge.targetHandle || null,
296+
sourceHandle: normalizeWorkflowEdgeSourceHandle(autoConnectEdge.sourceHandle),
297+
targetHandle: normalizeWorkflowEdgeTargetHandle(autoConnectEdge.targetHandle),
288298
})
289299
logger.debug(
290300
`Added auto-connect edge ${autoConnectEdge.id}: ${autoConnectEdge.source} -> ${autoConnectEdge.target}`
@@ -1051,8 +1061,8 @@ async function handleBlocksOperationTx(
10511061
// blocksById lookup (a plain `tx.select` from `workflowBlocks`) also
10521062
// sees the blocks this same batch just inserted — reads observe a
10531063
// transaction's own prior writes.
1054-
const candidates: EdgeAddCandidate[] = (edges as Array<Record<string, unknown>>).map(
1055-
(e) => ({
1064+
const candidates: EdgeAddCandidate[] = (edges as Array<Record<string, unknown>>).map((e) =>
1065+
canonicalizeEdgeAddCandidate({
10561066
id: e.id as string,
10571067
source: e.source as string,
10581068
target: e.target as string,
@@ -1078,8 +1088,8 @@ async function handleBlocksOperationTx(
10781088
workflowId,
10791089
sourceBlockId: edge.source,
10801090
targetBlockId: edge.target,
1081-
sourceHandle: edge.sourceHandle || null,
1082-
targetHandle: edge.targetHandle || null,
1091+
sourceHandle: normalizeWorkflowEdgeSourceHandle(edge.sourceHandle),
1092+
targetHandle: normalizeWorkflowEdgeTargetHandle(edge.targetHandle),
10831093
}))
10841094

10851095
await tx
@@ -1536,18 +1546,17 @@ async function handleEdgeOperationTx(tx: any, workflowId: string, operation: str
15361546
throw new Error('Missing required fields for add edge operation')
15371547
}
15381548

1549+
const candidate = canonicalizeEdgeAddCandidate({
1550+
id: payload.id,
1551+
source: payload.source,
1552+
target: payload.target,
1553+
sourceHandle: payload.sourceHandle ?? null,
1554+
targetHandle: payload.targetHandle ?? null,
1555+
})
15391556
const { safeEdges, droppedCounts, droppedDuplicates } = await filterEdgesForPersist(
15401557
tx,
15411558
workflowId,
1542-
[
1543-
{
1544-
id: payload.id,
1545-
source: payload.source,
1546-
target: payload.target,
1547-
sourceHandle: payload.sourceHandle ?? null,
1548-
targetHandle: payload.targetHandle ?? null,
1549-
},
1550-
]
1559+
[candidate]
15511560
)
15521561

15531562
if (safeEdges.length === 0) {
@@ -1561,13 +1570,14 @@ async function handleEdgeOperationTx(tx: any, workflowId: string, operation: str
15611570
break
15621571
}
15631572

1573+
const [safeEdge] = safeEdges
15641574
await tx.insert(workflowEdges).values({
1565-
id: payload.id,
1575+
id: safeEdge.id,
15661576
workflowId,
1567-
sourceBlockId: payload.source,
1568-
targetBlockId: payload.target,
1569-
sourceHandle: payload.sourceHandle || null,
1570-
targetHandle: payload.targetHandle || null,
1577+
sourceBlockId: safeEdge.source,
1578+
targetBlockId: safeEdge.target,
1579+
sourceHandle: normalizeWorkflowEdgeSourceHandle(safeEdge.sourceHandle),
1580+
targetHandle: normalizeWorkflowEdgeTargetHandle(safeEdge.targetHandle),
15711581
})
15721582

15731583
logger.debug(`Added edge ${payload.id}: ${payload.source} -> ${payload.target}`)
@@ -1782,13 +1792,15 @@ async function handleEdgesOperationTx(
17821792

17831793
logger.info(`Batch adding ${edges.length} edges to workflow ${workflowId}`)
17841794

1785-
const candidates: EdgeAddCandidate[] = (edges as Array<Record<string, unknown>>).map((e) => ({
1786-
id: e.id as string,
1787-
source: e.source as string,
1788-
target: e.target as string,
1789-
sourceHandle: (e.sourceHandle as string | null) ?? null,
1790-
targetHandle: (e.targetHandle as string | null) ?? null,
1791-
}))
1795+
const candidates: EdgeAddCandidate[] = (edges as Array<Record<string, unknown>>).map((e) =>
1796+
canonicalizeEdgeAddCandidate({
1797+
id: e.id as string,
1798+
source: e.source as string,
1799+
target: e.target as string,
1800+
sourceHandle: (e.sourceHandle as string | null) ?? null,
1801+
targetHandle: (e.targetHandle as string | null) ?? null,
1802+
})
1803+
)
17921804

17931805
const { safeEdges, droppedCounts, droppedDuplicates, droppedCyclic } =
17941806
await filterEdgesForPersist(tx, workflowId, candidates)
@@ -1811,8 +1823,8 @@ async function handleEdgesOperationTx(
18111823
workflowId,
18121824
sourceBlockId: edge.source,
18131825
targetBlockId: edge.target,
1814-
sourceHandle: edge.sourceHandle || null,
1815-
targetHandle: edge.targetHandle || null,
1826+
sourceHandle: normalizeWorkflowEdgeSourceHandle(edge.sourceHandle),
1827+
targetHandle: normalizeWorkflowEdgeTargetHandle(edge.targetHandle),
18161828
}))
18171829

18181830
await tx
@@ -2179,16 +2191,34 @@ async function handleWorkflowOperationTx(
21792191

21802192
// Insert all edges from the new state
21812193
if (edges && edges.length > 0) {
2182-
const edgeValues = edges.map((edge: any) => ({
2194+
const canonicalEdges = (edges as Array<Record<string, unknown>>).map((edge) =>
2195+
canonicalizeEdgeAddCandidate({
2196+
id: edge.id as string,
2197+
source: edge.source as string,
2198+
target: edge.target as string,
2199+
sourceHandle: (edge.sourceHandle as string | null) ?? null,
2200+
targetHandle: (edge.targetHandle as string | null) ?? null,
2201+
})
2202+
)
2203+
const uniqueEdges = filterUniqueWorkflowEdges(canonicalEdges, [])
2204+
const edgeValues = uniqueEdges.map((edge) => ({
21832205
id: edge.id,
21842206
workflowId,
21852207
sourceBlockId: edge.source,
21862208
targetBlockId: edge.target,
2187-
sourceHandle: edge.sourceHandle || null,
2188-
targetHandle: edge.targetHandle || null,
2209+
sourceHandle: edge.sourceHandle ?? null,
2210+
targetHandle: edge.targetHandle ?? null,
21892211
}))
21902212

2191-
await tx.insert(workflowEdges).values(edgeValues)
2213+
if (uniqueEdges.length < edges.length) {
2214+
logger.info(`Dropped ${edges.length - uniqueEdges.length} duplicate edge(s)`, {
2215+
operation: WORKFLOW_OPERATIONS.REPLACE_STATE,
2216+
})
2217+
}
2218+
2219+
if (edgeValues.length > 0) {
2220+
await tx.insert(workflowEdges).values(edgeValues)
2221+
}
21922222
}
21932223

21942224
// Insert all loops from the new state

apps/sim/lib/workflows/persistence/utils.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,13 +362,15 @@ describe('Database Helpers', () => {
362362
name: 'Start Block',
363363
position: { x: 100, y: 100 },
364364
enabled: true,
365+
errorEnabled: false,
365366
horizontalHandles: true,
366367
height: 150,
367368
subBlocks: { input: { id: 'input', type: 'short-input' as const, value: 'test' } },
368369
outputs: { result: { type: 'string' } },
369370
data: { parentId: undefined, extent: undefined, width: 350 },
370371
advancedMode: false,
371372
triggerMode: false,
373+
locked: undefined,
372374
})
373375

374376
expect(result?.edges[0]).toEqual({

apps/sim/stores/workflows/utils.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
import type { Edge } from 'reactflow'
1010
import { describe, expect, it } from 'vitest'
1111
import { normalizeName } from '@/executor/constants'
12-
import { getUniqueBlockName, regenerateBlockIds } from './utils'
12+
import { filterNewEdges, getUniqueBlockName, regenerateBlockIds } from './utils'
1313

1414
describe('normalizeName', () => {
1515
it.concurrent('should convert to lowercase', () => {
@@ -107,6 +107,33 @@ describe('normalizeName', () => {
107107
})
108108
})
109109

110+
describe('filterNewEdges', () => {
111+
const makeEdge = (id: string, sourceHandle: string, targetHandle: string): Edge => ({
112+
id,
113+
source: 'source',
114+
target: 'target',
115+
sourceHandle,
116+
targetHandle,
117+
})
118+
119+
it('treats legacy positioned handles as the same logical connection', () => {
120+
const currentEdges = [makeEdge('legacy', 'source-left', 'target-top')]
121+
const candidates = [
122+
makeEdge('current', 'source-right', 'target-left'),
123+
makeEdge('legacy-vertical', 'source-bottom', 'target-bottom'),
124+
]
125+
126+
expect(filterNewEdges(candidates, currentEdges)).toEqual([])
127+
})
128+
129+
it('keeps semantic routing handles distinct', () => {
130+
const currentEdges = [makeEdge('true-route', 'condition-true', 'target-left')]
131+
const candidates = [makeEdge('false-route', 'condition-false', 'target-left')]
132+
133+
expect(filterNewEdges(candidates, currentEdges)).toEqual(candidates)
134+
})
135+
})
136+
110137
describe('getUniqueBlockName', () => {
111138
it('should return "Start" for starter blocks', () => {
112139
expect(getUniqueBlockName('Start', {})).toBe('Start')

packages/workflow-types/src/workflow.ts

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -309,14 +309,28 @@ export function getHorizontalWorkflowHandleSide(
309309
return pointerX < cardWidth / 2 ? 'left' : 'right'
310310
}
311311

312-
// Falsy-coalesce (not nullish-coalesce): persistence normalizes a missing
313-
// handle to `null` via `edge.sourceHandle || null` (see
314-
// apps/realtime/src/database/operations.ts), which also maps `''` to
315-
// `null`. Comparing with `??` would treat `''` and `null` as distinct
316-
// handles pre-insert while both are written as the same `null` value,
317-
// letting a `sourceHandle: ''` edge slip past the duplicate check.
318-
function normalizeWorkflowEdgeHandle(handle: string | null | undefined): string | null {
319-
return handle || null
312+
/**
313+
* Returns the canonical persisted source handle used for edge identity.
314+
*
315+
* Falsy-coalescing mirrors persistence, where an empty handle is stored as
316+
* `null`, while positioned legacy outputs collapse onto the right-side port.
317+
*/
318+
export function normalizeWorkflowEdgeSourceHandle(
319+
handle: string | null | undefined
320+
): string | null {
321+
return normalizePositionedSourceHandleId(handle || null)
322+
}
323+
324+
/**
325+
* Returns the canonical persisted target handle used for edge identity.
326+
*
327+
* Falsy-coalescing mirrors persistence, where an empty handle is stored as
328+
* `null`, while legacy vertical inputs collapse onto the left-side port.
329+
*/
330+
export function normalizeWorkflowEdgeTargetHandle(
331+
handle: string | null | undefined
332+
): string | null {
333+
return normalizePositionedTargetHandleId(handle || null)
320334
}
321335

322336
function isDuplicateWorkflowEdge(
@@ -325,11 +339,11 @@ function isDuplicateWorkflowEdge(
325339
): boolean {
326340
return (
327341
edge.source === existing.source &&
328-
normalizeWorkflowEdgeHandle(edge.sourceHandle) ===
329-
normalizeWorkflowEdgeHandle(existing.sourceHandle) &&
342+
normalizeWorkflowEdgeSourceHandle(edge.sourceHandle) ===
343+
normalizeWorkflowEdgeSourceHandle(existing.sourceHandle) &&
330344
edge.target === existing.target &&
331-
normalizeWorkflowEdgeHandle(edge.targetHandle) ===
332-
normalizeWorkflowEdgeHandle(existing.targetHandle)
345+
normalizeWorkflowEdgeTargetHandle(edge.targetHandle) ===
346+
normalizeWorkflowEdgeTargetHandle(existing.targetHandle)
333347
)
334348
}
335349

0 commit comments

Comments
 (0)