From 8e0c46c08cc38ef046843bbaeb88099bfc05538b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 17:02:08 -0700 Subject: [PATCH 1/2] fix(deploy): resolve the error-output flag from edges on both sides of change detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../lib/workflows/comparison/compare.test.ts | 33 +++++++++++++++ apps/sim/lib/workflows/comparison/compare.ts | 35 ++++++++++++---- apps/sim/lib/workflows/persistence/utils.ts | 15 ++++--- packages/workflow-types/src/workflow.ts | 41 +++++++++++++++++++ 4 files changed, 111 insertions(+), 13 deletions(-) diff --git a/apps/sim/lib/workflows/comparison/compare.test.ts b/apps/sim/lib/workflows/comparison/compare.test.ts index d78be67c55d..63811a90dc5 100644 --- a/apps/sim/lib/workflows/comparison/compare.test.ts +++ b/apps/sim/lib/workflows/comparison/compare.test.ts @@ -386,6 +386,39 @@ describe('hasWorkflowChanged', () => { expect(hasWorkflowChanged(unset, withErrorFlag(false))).toBe(false) expect(hasWorkflowChanged(unset, withErrorFlag(true))).toBe(true) }) + + /** + * `setBlockErrorEnabled` leaves existing error edges in place, so a block can + * hold `errorEnabled: false` with a connected error edge. Only the deployed + * side is backfilled (`materializeDeploymentState`), so reading the flag alone + * makes that block differ from itself, and redeploying cannot clear it — the + * snapshot stores the live `false` that the next read backfills to `true`. + */ + const withErrorEdge = (errorEnabled: boolean) => + createWorkflowState({ + blocks: { + block1: { ...createBlock('block1'), errorEnabled }, + block2: createBlock('block2'), + }, + edges: [ + { id: 'e1', source: 'block1', sourceHandle: 'error', target: 'block2' }, + ] as WorkflowState['edges'], + }) + + it.concurrent('treats a live error edge as the flag being on', () => { + expect(hasWorkflowChanged(withErrorEdge(false), withErrorEdge(true))).toBe(false) + expect(hasWorkflowChanged(withErrorEdge(true), withErrorEdge(false))).toBe(false) + }) + + it.concurrent('reports no modified block when only the backfilled flag differs', () => { + const summary = generateWorkflowDiffSummary(withErrorEdge(false), withErrorEdge(true)) + expect(summary.hasChanges).toBe(false) + expect(summary.modifiedBlocks).toEqual([]) + }) + + it.concurrent('still detects the flag turning on when no error edge exists', () => { + expect(hasWorkflowChanged(withErrorFlag(true), withErrorFlag(false))).toBe(true) + }) }) describe('SubBlock Changes', () => { diff --git a/apps/sim/lib/workflows/comparison/compare.ts b/apps/sim/lib/workflows/comparison/compare.ts index b2a3d300960..e18c216e805 100644 --- a/apps/sim/lib/workflows/comparison/compare.ts +++ b/apps/sim/lib/workflows/comparison/compare.ts @@ -1,5 +1,9 @@ import { createLogger } from '@sim/logger' -import { blockRetryEquals } from '@sim/workflow-types/workflow' +import { + blockRetryEquals, + collectErrorSourceBlockIds, + resolveEffectiveErrorEnabled, +} from '@sim/workflow-types/workflow' import type { WorkflowState } from '@/stores/workflows/workflow/types' import { extractBlockFieldsForComparison, @@ -132,6 +136,8 @@ export function generateWorkflowDiffSummary( const previousBlocks = previousState.blocks || {} const currentBlockIds = new Set(Object.keys(currentBlocks)) const previousBlockIds = new Set(Object.keys(previousBlocks)) + const currentErrorSources = collectErrorSourceBlockIds(currentState.edges) + const previousErrorSources = collectErrorSourceBlockIds(previousState.edges) for (const id of currentBlockIds) { if (!previousBlockIds.has(id)) { @@ -173,6 +179,25 @@ export function generateWorkflowDiffSummary( subBlocks: previousSubBlocks, } = extractBlockFieldsForComparison(previousBlock) + /* + * Outside the structural gate below: the flag alone can match while the edges + * disagree, and reading it alone pins a block with a stale `errorEnabled: false` + * and a live error edge to "needs redeploy" forever. + */ + const currentErrorEnabled = resolveEffectiveErrorEnabled(currentBlock, id, currentErrorSources) + const previousErrorEnabled = resolveEffectiveErrorEnabled( + previousBlock, + id, + previousErrorSources + ) + if (currentErrorEnabled !== previousErrorEnabled) { + changes.push({ + field: 'errorEnabled', + oldValue: previousErrorEnabled, + newValue: currentErrorEnabled, + }) + } + const normalizedCurrentBlock = { ...currentRest, data: currentDataRest, subBlocks: undefined } const normalizedPreviousBlock = { ...previousRest, @@ -196,12 +221,8 @@ export function generateWorkflowDiffSummary( newValue: currentBlock.enabled, }) } - const blockFields = [ - 'horizontalHandles', - 'advancedMode', - 'triggerMode', - 'errorEnabled', - ] as const + /** `errorEnabled` is compared above, against the edges as well as the flag. */ + const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode'] as const for (const field of blockFields) { if (!!currentBlock[field] !== !!previousBlock[field]) { changes.push({ diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts index c9313ef325d..421cbafd615 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -17,7 +17,10 @@ import { import { saveWorkflowToNormalizedTables as saveWorkflowToNormalizedTablesRaw } from '@sim/workflow-persistence/save' import type { DbOrTx, NormalizedWorkflowData } from '@sim/workflow-persistence/types' import type { BlockState, Loop, Parallel, WorkflowState } from '@sim/workflow-types/workflow' -import { normalizeWorkflowEdgeHandles } from '@sim/workflow-types/workflow' +import { + collectErrorSourceBlockIds, + normalizeWorkflowEdgeHandles, +} from '@sim/workflow-types/workflow' import type { InferSelectModel } from 'drizzle-orm' import { and, desc, eq, inArray, lt, sql } from 'drizzle-orm' import { LRUCache } from 'lru-cache' @@ -187,12 +190,12 @@ async function materializeDeploymentState( * from a block that had the output — and the migration backfilling the flag * only reaches the live tables, never a version's frozen jsonb. Without this * the deployed side reads `false` against a live `true` and every workflow - * deployed before the toggle asks to be redeployed once. Same rule as - * `workflow-block.tsx` applies at render time; neither may read the flag alone. + * deployed before the toggle asks to be redeployed once. This backfills only + * the deployed side, so change detection must apply `resolveEffectiveErrorEnabled` + * to the live side too — reading the raw flag there compares a block against + * itself forever. Same rule the block renderers apply; none may read the flag alone. */ - const errorSourceBlockIds = new Set( - edges.filter((edge) => edge.sourceHandle === 'error').map((edge) => edge.source) - ) + const errorSourceBlockIds = collectErrorSourceBlockIds(edges) const blocks: DeployedWorkflowData['blocks'] = {} for (const [blockId, block] of Object.entries(migratedBlocks)) { blocks[blockId] = diff --git a/packages/workflow-types/src/workflow.ts b/packages/workflow-types/src/workflow.ts index b1ba622d14e..0d1b19729af 100644 --- a/packages/workflow-types/src/workflow.ts +++ b/packages/workflow-types/src/workflow.ts @@ -342,6 +342,8 @@ export type WorkflowConnectionSide = (typeof WORKFLOW_CONNECTION_SIDES)[number] export const WORKFLOW_SOURCE_HANDLE_ID = 'source' /** The one input handle every block that accepts a connection exposes. */ export const WORKFLOW_TARGET_HANDLE_ID = 'target' +/** The output handle a block's error branch leaves through. */ +export const WORKFLOW_ERROR_HANDLE_ID = 'error' /** * Side-anchored handle ids (`source-right`, `target-left`, …) briefly existed @@ -387,6 +389,45 @@ export function normalizeWorkflowEdgeTargetHandle( return canonical } +/** + * Collects the ids of blocks an error edge leaves, canonicalizing handles first + * so the set is the same however the edge list was loaded. + */ +export function collectErrorSourceBlockIds( + edges: readonly WorkflowEdgeHandles[] | null | undefined +): Set { + const sources = new Set() + for (const edge of edges || []) { + if (normalizeWorkflowEdgeSourceHandle(edge.sourceHandle) === WORKFLOW_ERROR_HANDLE_ID) { + sources.add(edge.source) + } + } + return sources +} + +/** + * Whether a block's error output is on, read from the edges as well as the flag. + * + * An error edge means the port is live whatever the flag says: `setBlockErrorEnabled` + * leaves existing error edges in place, and both block renderers draw the port on + * `errorEnabled || hasErrorConnection`, so a block can sit at `errorEnabled: false` + * with a connected error edge indefinitely. The executor never reads the flag at + * all — the edge alone decides routing — so the two spellings are one state. + * + * Every reader that compares or materializes a block must apply this rule rather + * than the flag alone. Applying it on one side only is what pinned a workflow to + * "needs redeploy" with nothing to deploy: change detection read a backfilled + * `true` against a live `false`, and redeploying snapshotted the live `false` that + * the next read backfilled straight back to `true`. + */ +export function resolveEffectiveErrorEnabled( + block: Pick, + blockId: string, + errorSourceBlockIds: ReadonlySet +): boolean { + return Boolean(block.errorEnabled) || errorSourceBlockIds.has(blockId) +} + /** * Canonicalizes a whole edge list, for the readers that bypass * `loadWorkflowFromNormalizedTables` — deployment-version blobs, run From af0fb83f7ad5a028a462d9f68f3698df8a779364 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 15 Aug 2026 17:25:05 -0700 Subject: [PATCH 2/2] chore(deploy): use TSDoc for the error-output comparison comments --- apps/sim/lib/workflows/comparison/compare.ts | 2 +- apps/sim/lib/workflows/persistence/utils.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/workflows/comparison/compare.ts b/apps/sim/lib/workflows/comparison/compare.ts index e18c216e805..e030e1b13aa 100644 --- a/apps/sim/lib/workflows/comparison/compare.ts +++ b/apps/sim/lib/workflows/comparison/compare.ts @@ -179,7 +179,7 @@ export function generateWorkflowDiffSummary( subBlocks: previousSubBlocks, } = extractBlockFieldsForComparison(previousBlock) - /* + /** * Outside the structural gate below: the flag alone can match while the edges * disagree, and reading it alone pins a block with a stale `errorEnabled: false` * and a live error edge to "needs redeploy" forever. diff --git a/apps/sim/lib/workflows/persistence/utils.ts b/apps/sim/lib/workflows/persistence/utils.ts index 421cbafd615..9150443d5d2 100644 --- a/apps/sim/lib/workflows/persistence/utils.ts +++ b/apps/sim/lib/workflows/persistence/utils.ts @@ -184,7 +184,7 @@ async function materializeDeploymentState( */ const edges = normalizeWorkflowEdgeHandles(state.edges) - /* + /** * An error edge means the error output is on. Every version before the toggle * drew that port unconditionally, so a snapshot with such an edge was taken * from a block that had the output — and the migration backfilling the flag