Skip to content

Commit 9115631

Browse files
committed
fix(deploy): resolve the error-output flag from edges on both sides of change detection
A block can hold `errorEnabled: false` while an error edge still leaves it. `setBlockErrorEnabled` does not remove existing error edges, both block renderers draw the port on `errorEnabled || hasErrorConnection`, and the executor never reads the flag at all — the edge alone decides routing. So the two spellings are one state, and nothing about the block has functionally changed. Only the deployed side reconciled them. `materializeDeploymentState` backfills `errorEnabled: true` for any block with an error edge, while the live normalized tables are read verbatim. Change detection compared the raw flag, saw `true` against `false` for a block that had not changed, and no redeploy could clear it: deploying snapshots the live `false`, which the next read backfills straight back to `true`. The deploy button sat on "Update" permanently, and the server path (`checkNeedsRedeployment`, which reads the raw jsonb and skips the backfill) disagreed with it. Lift the rule into `@sim/workflow-types` as `collectErrorSourceBlockIds` / `resolveEffectiveErrorEnabled` so the backfill and the comparison share one definition, and apply it to both sides of the diff. Compared outside the structural gate, since the flag can match while the edges disagree.
1 parent d45dad7 commit 9115631

4 files changed

Lines changed: 111 additions & 13 deletions

File tree

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,39 @@ describe('hasWorkflowChanged', () => {
386386
expect(hasWorkflowChanged(unset, withErrorFlag(false))).toBe(false)
387387
expect(hasWorkflowChanged(unset, withErrorFlag(true))).toBe(true)
388388
})
389+
390+
/**
391+
* `setBlockErrorEnabled` leaves existing error edges in place, so a block can
392+
* hold `errorEnabled: false` with a connected error edge. Only the deployed
393+
* side is backfilled (`materializeDeploymentState`), so reading the flag alone
394+
* makes that block differ from itself, and redeploying cannot clear it — the
395+
* snapshot stores the live `false` that the next read backfills to `true`.
396+
*/
397+
const withErrorEdge = (errorEnabled: boolean) =>
398+
createWorkflowState({
399+
blocks: {
400+
block1: { ...createBlock('block1'), errorEnabled },
401+
block2: createBlock('block2'),
402+
},
403+
edges: [
404+
{ id: 'e1', source: 'block1', sourceHandle: 'error', target: 'block2' },
405+
] as WorkflowState['edges'],
406+
})
407+
408+
it.concurrent('treats a live error edge as the flag being on', () => {
409+
expect(hasWorkflowChanged(withErrorEdge(false), withErrorEdge(true))).toBe(false)
410+
expect(hasWorkflowChanged(withErrorEdge(true), withErrorEdge(false))).toBe(false)
411+
})
412+
413+
it.concurrent('reports no modified block when only the backfilled flag differs', () => {
414+
const summary = generateWorkflowDiffSummary(withErrorEdge(false), withErrorEdge(true))
415+
expect(summary.hasChanges).toBe(false)
416+
expect(summary.modifiedBlocks).toEqual([])
417+
})
418+
419+
it.concurrent('still detects the flag turning on when no error edge exists', () => {
420+
expect(hasWorkflowChanged(withErrorFlag(true), withErrorFlag(false))).toBe(true)
421+
})
389422
})
390423

391424
describe('SubBlock Changes', () => {

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

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import { blockRetryEquals } from '@sim/workflow-types/workflow'
2+
import {
3+
blockRetryEquals,
4+
collectErrorSourceBlockIds,
5+
resolveEffectiveErrorEnabled,
6+
} from '@sim/workflow-types/workflow'
37
import type { WorkflowState } from '@/stores/workflows/workflow/types'
48
import {
59
extractBlockFieldsForComparison,
@@ -132,6 +136,8 @@ export function generateWorkflowDiffSummary(
132136
const previousBlocks = previousState.blocks || {}
133137
const currentBlockIds = new Set(Object.keys(currentBlocks))
134138
const previousBlockIds = new Set(Object.keys(previousBlocks))
139+
const currentErrorSources = collectErrorSourceBlockIds(currentState.edges)
140+
const previousErrorSources = collectErrorSourceBlockIds(previousState.edges)
135141

136142
for (const id of currentBlockIds) {
137143
if (!previousBlockIds.has(id)) {
@@ -173,6 +179,25 @@ export function generateWorkflowDiffSummary(
173179
subBlocks: previousSubBlocks,
174180
} = extractBlockFieldsForComparison(previousBlock)
175181

182+
/*
183+
* Outside the structural gate below: the flag alone can match while the edges
184+
* disagree, and reading it alone pins a block with a stale `errorEnabled: false`
185+
* and a live error edge to "needs redeploy" forever.
186+
*/
187+
const currentErrorEnabled = resolveEffectiveErrorEnabled(currentBlock, id, currentErrorSources)
188+
const previousErrorEnabled = resolveEffectiveErrorEnabled(
189+
previousBlock,
190+
id,
191+
previousErrorSources
192+
)
193+
if (currentErrorEnabled !== previousErrorEnabled) {
194+
changes.push({
195+
field: 'errorEnabled',
196+
oldValue: previousErrorEnabled,
197+
newValue: currentErrorEnabled,
198+
})
199+
}
200+
176201
const normalizedCurrentBlock = { ...currentRest, data: currentDataRest, subBlocks: undefined }
177202
const normalizedPreviousBlock = {
178203
...previousRest,
@@ -196,12 +221,8 @@ export function generateWorkflowDiffSummary(
196221
newValue: currentBlock.enabled,
197222
})
198223
}
199-
const blockFields = [
200-
'horizontalHandles',
201-
'advancedMode',
202-
'triggerMode',
203-
'errorEnabled',
204-
] as const
224+
/** `errorEnabled` is compared above, against the edges as well as the flag. */
225+
const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode'] as const
205226
for (const field of blockFields) {
206227
if (!!currentBlock[field] !== !!previousBlock[field]) {
207228
changes.push({

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ import {
1717
import { saveWorkflowToNormalizedTables as saveWorkflowToNormalizedTablesRaw } from '@sim/workflow-persistence/save'
1818
import type { DbOrTx, NormalizedWorkflowData } from '@sim/workflow-persistence/types'
1919
import type { BlockState, Loop, Parallel, WorkflowState } from '@sim/workflow-types/workflow'
20-
import { normalizeWorkflowEdgeHandles } from '@sim/workflow-types/workflow'
20+
import {
21+
collectErrorSourceBlockIds,
22+
normalizeWorkflowEdgeHandles,
23+
} from '@sim/workflow-types/workflow'
2124
import type { InferSelectModel } from 'drizzle-orm'
2225
import { and, desc, eq, inArray, lt, sql } from 'drizzle-orm'
2326
import { LRUCache } from 'lru-cache'
@@ -187,12 +190,12 @@ async function materializeDeploymentState(
187190
* from a block that had the output — and the migration backfilling the flag
188191
* only reaches the live tables, never a version's frozen jsonb. Without this
189192
* the deployed side reads `false` against a live `true` and every workflow
190-
* deployed before the toggle asks to be redeployed once. Same rule as
191-
* `workflow-block.tsx` applies at render time; neither may read the flag alone.
193+
* deployed before the toggle asks to be redeployed once. This backfills only
194+
* the deployed side, so change detection must apply `resolveEffectiveErrorEnabled`
195+
* to the live side too — reading the raw flag there compares a block against
196+
* itself forever. Same rule the block renderers apply; none may read the flag alone.
192197
*/
193-
const errorSourceBlockIds = new Set(
194-
edges.filter((edge) => edge.sourceHandle === 'error').map((edge) => edge.source)
195-
)
198+
const errorSourceBlockIds = collectErrorSourceBlockIds(edges)
196199
const blocks: DeployedWorkflowData['blocks'] = {}
197200
for (const [blockId, block] of Object.entries(migratedBlocks)) {
198201
blocks[blockId] =

packages/workflow-types/src/workflow.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,8 @@ export type WorkflowConnectionSide = (typeof WORKFLOW_CONNECTION_SIDES)[number]
342342
export const WORKFLOW_SOURCE_HANDLE_ID = 'source'
343343
/** The one input handle every block that accepts a connection exposes. */
344344
export const WORKFLOW_TARGET_HANDLE_ID = 'target'
345+
/** The output handle a block's error branch leaves through. */
346+
export const WORKFLOW_ERROR_HANDLE_ID = 'error'
345347

346348
/**
347349
* Side-anchored handle ids (`source-right`, `target-left`, …) briefly existed
@@ -387,6 +389,45 @@ export function normalizeWorkflowEdgeTargetHandle(
387389
return canonical
388390
}
389391

392+
/**
393+
* Collects the ids of blocks an error edge leaves, canonicalizing handles first
394+
* so the set is the same however the edge list was loaded.
395+
*/
396+
export function collectErrorSourceBlockIds(
397+
edges: readonly WorkflowEdgeHandles[] | null | undefined
398+
): Set<string> {
399+
const sources = new Set<string>()
400+
for (const edge of edges || []) {
401+
if (normalizeWorkflowEdgeSourceHandle(edge.sourceHandle) === WORKFLOW_ERROR_HANDLE_ID) {
402+
sources.add(edge.source)
403+
}
404+
}
405+
return sources
406+
}
407+
408+
/**
409+
* Whether a block's error output is on, read from the edges as well as the flag.
410+
*
411+
* An error edge means the port is live whatever the flag says: `setBlockErrorEnabled`
412+
* leaves existing error edges in place, and both block renderers draw the port on
413+
* `errorEnabled || hasErrorConnection`, so a block can sit at `errorEnabled: false`
414+
* with a connected error edge indefinitely. The executor never reads the flag at
415+
* all — the edge alone decides routing — so the two spellings are one state.
416+
*
417+
* Every reader that compares or materializes a block must apply this rule rather
418+
* than the flag alone. Applying it on one side only is what pinned a workflow to
419+
* "needs redeploy" with nothing to deploy: change detection read a backfilled
420+
* `true` against a live `false`, and redeploying snapshotted the live `false` that
421+
* the next read backfilled straight back to `true`.
422+
*/
423+
export function resolveEffectiveErrorEnabled(
424+
block: Pick<BlockState, 'errorEnabled'>,
425+
blockId: string,
426+
errorSourceBlockIds: ReadonlySet<string>
427+
): boolean {
428+
return Boolean(block.errorEnabled) || errorSourceBlockIds.has(blockId)
429+
}
430+
390431
/**
391432
* Canonicalizes a whole edge list, for the readers that bypass
392433
* `loadWorkflowFromNormalizedTables` — deployment-version blobs, run

0 commit comments

Comments
 (0)