Skip to content

Commit b959202

Browse files
committed
fix(workflow-edges): normalize empty-string handles in duplicate-edge check
filterUniqueWorkflowEdges compared handles with ??, so a `sourceHandle: ''` edge wasn't recognized as a duplicate of an existing null-handle edge — even though both get persisted as the same null value at insert time (edge.sourceHandle || null). Falsy-coalesce in the comparison so '' and null/undefined are treated as the same "no handle" state everywhere. (Greptile)
1 parent 878b027 commit b959202

2 files changed

Lines changed: 31 additions & 2 deletions

File tree

apps/sim/stores/workflows/workflow/store.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,23 @@ describe('workflow store', () => {
352352
expect(state.edges.some((e) => e.id === 'e2')).toBe(false)
353353
})
354354

355+
it('should treat an empty-string handle as equivalent to no handle when deduping', () => {
356+
const { batchAddEdges } = useWorkflowStore.getState()
357+
358+
addBlock('block-1', 'starter', 'Start', { x: 0, y: 0 })
359+
addBlock('block-2', 'function', 'End', { x: 200, y: 0 })
360+
361+
batchAddEdges([
362+
{ id: 'e1', source: 'block-1', target: 'block-2', sourceHandle: '' },
363+
{ id: 'e2', source: 'block-1', target: 'block-2' },
364+
])
365+
366+
const state = useWorkflowStore.getState()
367+
expectEdgeCount(state, 1)
368+
expect(state.edges.some((e) => e.id === 'e1')).toBe(true)
369+
expect(state.edges.some((e) => e.id === 'e2')).toBe(false)
370+
})
371+
355372
it('should not add a self-loop edge', () => {
356373
const { batchAddEdges } = useWorkflowStore.getState()
357374

packages/workflow-types/src/workflow.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,15 +212,27 @@ export interface WorkflowEdgeHandles extends WorkflowEdgeEndpoints {
212212
targetHandle?: string | null
213213
}
214214

215+
// Falsy-coalesce (not nullish-coalesce): persistence normalizes a missing
216+
// handle to `null` via `edge.sourceHandle || null` (see
217+
// apps/realtime/src/database/operations.ts), which also maps `''` to
218+
// `null`. Comparing with `??` would treat `''` and `null` as distinct
219+
// handles pre-insert while both are written as the same `null` value,
220+
// letting a `sourceHandle: ''` edge slip past the duplicate check.
221+
function normalizeWorkflowEdgeHandle(handle: string | null | undefined): string | null {
222+
return handle || null
223+
}
224+
215225
function isDuplicateWorkflowEdge(
216226
edge: WorkflowEdgeHandles,
217227
existing: WorkflowEdgeHandles
218228
): boolean {
219229
return (
220230
edge.source === existing.source &&
221-
(edge.sourceHandle ?? null) === (existing.sourceHandle ?? null) &&
231+
normalizeWorkflowEdgeHandle(edge.sourceHandle) ===
232+
normalizeWorkflowEdgeHandle(existing.sourceHandle) &&
222233
edge.target === existing.target &&
223-
(edge.targetHandle ?? null) === (existing.targetHandle ?? null)
234+
normalizeWorkflowEdgeHandle(edge.targetHandle) ===
235+
normalizeWorkflowEdgeHandle(existing.targetHandle)
224236
)
225237
}
226238

0 commit comments

Comments
 (0)