mirror of
https://github.com/simstudioai/sim.git
synced 2026-08-31 01:11:53 +08:00
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 066e18ac28. 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>
This commit is contained in:
@@ -193,6 +193,43 @@ describe('hasWorkflowChanged', () => {
|
||||
expect(hasWorkflowChanged(state1, state2)).toBe(true)
|
||||
})
|
||||
|
||||
/**
|
||||
* The live side collapses a falsy handle to nothing (`loadWorkflowFromNormalizedTables`
|
||||
* runs it through the canonicalizer), while the server diffs that against the deployment
|
||||
* version's raw jsonb, which does not. An edge persisted with `sourceHandle: ''` would be
|
||||
* present on one side and absent on the other — counted as removed and re-added, asking
|
||||
* every such workflow to redeploy for nothing.
|
||||
*/
|
||||
it.concurrent('treats an empty-string handle as no handle', () => {
|
||||
const empty = createWorkflowState({
|
||||
edges: [{ id: 'edge1', source: 'block1', sourceHandle: '', target: 'block2' }],
|
||||
})
|
||||
const absent = createWorkflowState({
|
||||
edges: [{ id: 'edge1', source: 'block1', target: 'block2' }],
|
||||
})
|
||||
expect(hasWorkflowChanged(empty, absent)).toBe(false)
|
||||
})
|
||||
|
||||
it.concurrent('treats a side-anchored source handle as its canonical id', () => {
|
||||
const sideAnchored = createWorkflowState({
|
||||
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source-right', target: 'block2' }],
|
||||
})
|
||||
const canonical = createWorkflowState({
|
||||
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source', target: 'block2' }],
|
||||
})
|
||||
expect(hasWorkflowChanged(sideAnchored, canonical)).toBe(false)
|
||||
})
|
||||
|
||||
it.concurrent('still tells two real ports apart', () => {
|
||||
const source = createWorkflowState({
|
||||
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'source', target: 'block2' }],
|
||||
})
|
||||
const error = createWorkflowState({
|
||||
edges: [{ id: 'edge1', source: 'block1', sourceHandle: 'error', target: 'block2' }],
|
||||
})
|
||||
expect(hasWorkflowChanged(source, error)).toBe(true)
|
||||
})
|
||||
|
||||
it.concurrent('should ignore edge ID changes', () => {
|
||||
const state1 = createWorkflowState({
|
||||
edges: [{ id: 'edge-old', source: 'block1', target: 'block2' }],
|
||||
|
||||
@@ -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'
|
||||
@@ -313,13 +317,26 @@ export function normalizeEdge(edge: Edge): NormalizedEdge {
|
||||
source: edge.source,
|
||||
target: edge.target,
|
||||
}
|
||||
/*
|
||||
* Canonicalized here rather than by each caller, because the two sides of a
|
||||
* redeploy check are loaded by different paths and only some of them
|
||||
* normalize: the server diffs the normalized tables — which now collapse a
|
||||
* falsy handle to nothing — against the version's raw jsonb, which does not.
|
||||
* An edge persisted with `sourceHandle: ''` (two write paths use `?? null`,
|
||||
* which preserves it) would then be present on one side and absent on the
|
||||
* other, counting as removed-and-re-added and asking every such workflow to
|
||||
* redeploy. Both spellings name one port, so the comparison must not be able
|
||||
* to tell them apart however its inputs arrived.
|
||||
*/
|
||||
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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user