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..e030e1b13aa 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..9150443d5d2 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' @@ -181,18 +184,18 @@ 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 * 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