Skip to content

Commit 3c29476

Browse files
Vikhyath Mondreticlaude
andcommitted
fix(deploy): compare edge handles by port, so a falsy one cannot read as changed
`loadWorkflowFromNormalizedTables` now runs handles through the canonicalizer, which falsy-coalesces — so an edge persisted with `sourceHandle: ''` loads as no handle at all. The server diffs that against the deployment version's raw jsonb, which still has `''`, and the set comparison reads one edge as removed and another added. Every workflow holding such an edge would ask to be redeployed the moment this ships, for nothing. Two write paths use `?? null` rather than `|| null`, so `''` is reachable. Canonicalized inside `normalizeEdge` rather than at either call site: the two sides are loaded by different paths and only some of them normalize, so the comparison has to be unable to tell two spellings of one port apart however its inputs arrived. This is the change reverted in 066e18a. That revert reasoned only about side-anchored ids, which are genuinely unreachable — it missed that the same coalesce collapses the empty string, which is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b6c007e commit 3c29476

2 files changed

Lines changed: 58 additions & 4 deletions

File tree

‎apps/sim/lib/workflows/comparison/compare.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,43 @@ describe('hasWorkflowChanged', () => {
193193
expect(hasWorkflowChanged(state1, state2)).toBe(true)
194194
})
195195

196+
/**
197+
* The live side collapses a falsy handle to nothing (`loadWorkflowFromNormalizedTables`
198+
* runs it through the canonicalizer), while the server diffs that against the deployment
199+
* version's raw jsonb, which does not. An edge persisted with `sourceHandle: ''` would be
200+
* present on one side and absent on the other — counted as removed and re-added, asking
201+
* every such workflow to redeploy for nothing.
202+
*/
203+
it.concurrent('treats an empty-string handle as no handle', () => {
204+
const empty = createWorkflowState({
205+
edges: [{ id: 'edge1', source: 'block1', sourceHandle: '', target: 'block2' }],
206+
})
207+
const absent = createWorkflowState({
208+
edges: [{ id: 'edge1', source: 'block1', target: 'block2' }],
209+
})
210+
expect(hasWorkflowChanged(empty, absent)).toBe(false)
211+
})
212+
213+
it.concurrent('treats a side-anchored source handle as its canonical id', () => {
214+
const sideAnchored = createWorkflowState({
215+
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source-right', target: 'block2' }],
216+
})
217+
const canonical = createWorkflowState({
218+
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source', target: 'block2' }],
219+
})
220+
expect(hasWorkflowChanged(sideAnchored, canonical)).toBe(false)
221+
})
222+
223+
it.concurrent('still tells two real ports apart', () => {
224+
const source = createWorkflowState({
225+
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source', target: 'block2' }],
226+
})
227+
const error = createWorkflowState({
228+
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'error', target: 'block2' }],
229+
})
230+
expect(hasWorkflowChanged(source, error)).toBe(true)
231+
})
232+
196233
it.concurrent('should ignore edge ID changes', () => {
197234
const state1 = createWorkflowState({
198235
edges: [{ id: 'edge-old', source: 'block1', target: 'block2' }],

‎apps/sim/lib/workflows/comparison/normalize.ts‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
* Used by both client-side signature computation and server-side comparison.
44
*/
55

6+
import {
7+
normalizeWorkflowEdgeSourceHandle,
8+
normalizeWorkflowEdgeTargetHandle,
9+
} from '@sim/workflow-types/workflow'
610
import type { Edge } from 'reactflow'
711
import { isNonEmptyValue } from '@/lib/workflows/subblocks/visibility'
812
import { isSyntheticToolSubBlockId } from '@/lib/workflows/tool-input/synthetic-subblocks'
@@ -313,13 +317,26 @@ export function normalizeEdge(edge: Edge): NormalizedEdge {
313317
source: edge.source,
314318
target: edge.target,
315319
}
320+
/*
321+
* Canonicalized here rather than by each caller, because the two sides of a
322+
* redeploy check are loaded by different paths and only some of them
323+
* normalize: the server diffs the normalized tables — which now collapse a
324+
* falsy handle to nothing — against the version's raw jsonb, which does not.
325+
* An edge persisted with `sourceHandle: ''` (two write paths use `?? null`,
326+
* which preserves it) would then be present on one side and absent on the
327+
* other, counting as removed-and-re-added and asking every such workflow to
328+
* redeploy. Both spellings name one port, so the comparison must not be able
329+
* to tell them apart however its inputs arrived.
330+
*/
331+
const sourceHandle = normalizeWorkflowEdgeSourceHandle(edge.sourceHandle)
332+
const targetHandle = normalizeWorkflowEdgeTargetHandle(edge.targetHandle)
316333
// Only include handles if they have a non-null value
317334
// This treats null and undefined as equivalent (both omitted)
318-
if (edge.sourceHandle != null) {
319-
normalized.sourceHandle = edge.sourceHandle
335+
if (sourceHandle != null) {
336+
normalized.sourceHandle = sourceHandle
320337
}
321-
if (edge.targetHandle != null) {
322-
normalized.targetHandle = edge.targetHandle
338+
if (targetHandle != null) {
339+
normalized.targetHandle = targetHandle
323340
}
324341
return normalized
325342
}

0 commit comments

Comments
 (0)