Skip to content

Commit 7934df7

Browse files
icecrasher321claude
andcommitted
refactor(blocks): give the error-output flag a column instead of two homes
`errorEnabled` had no column, so it persisted inside the block's `data` jsonb and was mirrored onto the block as a field on load. Every writer had to route its `data` through `withPersistedErrorEnabled` or silently drop the toggle, the realtime op `jsonb_set`, and change detection saw the same value twice — which is what made the deploy badge flip between Live and "Update deployment" after toggling the port. Its siblings — `enabled`, `horizontal_handles`, `advanced_mode`, `trigger_mode`, `locked` — are all boolean columns; `data` is for React Flow and subflow state. The flag belongs with them, so it now has `error_enabled` and one home. The shuttle helper, its `BlockData` mirror, the store's fallback read, and the comparison exclusion the duplication forced are all gone. Backwards compatibility, since released versions draw the error port with no toggle in front of it: a block already wired to an error edge HAS the output on, because there was no other way to draw that edge. That rule is now stated in three places and none may be narrowed to read the flag alone — - the migration backfills `error_enabled` from the edges, so live rows are true before any new code reads them; - `materializeDeploymentState` derives it for a version's frozen jsonb, which the migration cannot reach — otherwise every workflow deployed before the toggle would ask to be redeployed once; - `workflow-block.tsx` keeps it at render time for states that reach the canvas through neither (imports, copilot edits), where unmounting the port would make React Flow drop the edge leaving it. The migration also moves any `data.errorEnabled` a developer created on this branch onto the column and strips the key; both statements match zero rows in production, where it never shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 066e18a commit 7934df7

14 files changed

Lines changed: 18914 additions & 91 deletions

File tree

apps/realtime/src/database/operations.ts

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import {
3434
isWorkflowBlockProtected,
3535
normalizeWorkflowEdgeSourceHandle,
3636
normalizeWorkflowEdgeTargetHandle,
37-
withPersistedErrorEnabled,
3837
} from '@sim/workflow-types/workflow'
3938
import { and, eq, inArray, isNull, or, sql } from 'drizzle-orm'
4039
import { drizzle } from 'drizzle-orm/postgres-js'
@@ -755,12 +754,7 @@ async function handleBlockOperationTx(
755754
const updateResult = await tx
756755
.update(workflowBlocks)
757756
.set({
758-
data: sql`jsonb_set(
759-
coalesce(${workflowBlocks.data}, '{}'::jsonb),
760-
'{errorEnabled}',
761-
${JSON.stringify(payload.errorEnabled)}::jsonb,
762-
true
763-
)`,
757+
errorEnabled: payload.errorEnabled,
764758
updatedAt: new Date(),
765759
})
766760
.where(and(eq(workflowBlocks.id, payload.id), eq(workflowBlocks.workflowId, workflowId)))
@@ -960,16 +954,14 @@ async function handleBlocksOperationTx(
960954
name: block.name as string,
961955
positionX: (block.position as { x: number; y: number }).x,
962956
positionY: (block.position as { x: number; y: number }).y,
963-
data: withPersistedErrorEnabled(
964-
block.data as Record<string, unknown> | undefined,
965-
block.errorEnabled as boolean | undefined
966-
),
957+
data: (block.data as Record<string, unknown> | undefined) || {},
967958
subBlocks: mergedSubBlocks,
968959
outputs: (block.outputs as Record<string, unknown>) || {},
969960
enabled: (block.enabled as boolean) ?? true,
970961
horizontalHandles: (block.horizontalHandles as boolean) ?? true,
971962
advancedMode: (block.advancedMode as boolean) ?? false,
972963
triggerMode: (block.triggerMode as boolean) ?? false,
964+
errorEnabled: (block.errorEnabled as boolean) ?? false,
973965
height: (block.height as number) || 0,
974966
locked: (block.locked as boolean) ?? false,
975967
}
@@ -2179,7 +2171,8 @@ async function handleWorkflowOperationTx(
21792171
name: block.name,
21802172
positionX: block.position.x,
21812173
positionY: block.position.y,
2182-
data: withPersistedErrorEnabled(block.data, block.errorEnabled),
2174+
errorEnabled: block.errorEnabled ?? false,
2175+
data: block.data || {},
21832176
subBlocks: block.subBlocks || {},
21842177
outputs: block.outputs || {},
21852178
enabled: block.enabled ?? true,

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,16 @@ export const WorkflowBlock = memo(function WorkflowBlock({
741741
() => new Set(highlightedHandleKey ? highlightedHandleKey.split('|') : []),
742742
[highlightedHandleKey]
743743
)
744+
/*
745+
* An existing error edge means the output is on, whatever the flag says.
746+
* Every released version drew the error port with no toggle in front of it, so
747+
* a block already wired that way had no other way to make the connection —
748+
* reading the flag alone would unmount a port a live workflow routes failures
749+
* through, and React Flow drops an edge whose handle is not mounted. The
750+
* migration backfills those rows, but this keeps the rule true for any state
751+
* that reaches the canvas without passing through it (an imported workflow, a
752+
* deployment snapshot, a copilot edit).
753+
*/
744754
const errorOutputEnabled = Boolean(currentBlock?.errorEnabled || hasErrorConnection)
745755
const handleToggleErrorOutput = useCallback(
746756
(next: boolean) => {

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

Lines changed: 10 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -331,43 +331,23 @@ describe('hasWorkflowChanged', () => {
331331
})
332332

333333
/**
334-
* `errorEnabled` persists inside the block's `data` jsonb and is mirrored onto
335-
* the block as a field on load, so the same flag reaches the comparison twice
336-
* — and only some paths populate the copy. Counted through `data`, a block
337-
* read one way differed from the identical block read another, which flipped
338-
* the deploy badge between Live and "Update deployment" as each query landed.
339-
* The top-level field is the one the comparison trusts.
334+
* The flag drives the error port, and the deploy badge is what tells a user the
335+
* port they just switched on is not live yet.
340336
*/
341337
describe('Error Output Flag', () => {
342-
/* Spread on after the factory, which returns a fixed block shape. */
343-
const withErrorFlag = (errorEnabled: boolean, data: Record<string, unknown>) =>
338+
const withErrorFlag = (errorEnabled: boolean) =>
344339
createWorkflowState({
345-
blocks: { block1: { ...createBlock('block1', { data }), errorEnabled } },
340+
blocks: { block1: { ...createBlock('block1'), errorEnabled } },
346341
})
347342

348-
it.concurrent('ignores a stale data mirror when the flag itself matches', () => {
349-
expect(
350-
hasWorkflowChanged(
351-
withErrorFlag(true, { errorEnabled: false }),
352-
withErrorFlag(true, { errorEnabled: true })
353-
)
354-
).toBe(false)
343+
it.concurrent('detects the flag being turned on', () => {
344+
expect(hasWorkflowChanged(withErrorFlag(true), withErrorFlag(false))).toBe(true)
355345
})
356346

357-
it.concurrent('ignores a data mirror only one side carries', () => {
358-
expect(hasWorkflowChanged(withErrorFlag(false, {}), withErrorFlag(false, {}))).toBe(false)
359-
expect(
360-
hasWorkflowChanged(withErrorFlag(false, {}), withErrorFlag(false, { errorEnabled: false }))
361-
).toBe(false)
362-
})
363-
364-
it.concurrent('still detects the flag being turned on', () => {
365-
expect(
366-
hasWorkflowChanged(
367-
withErrorFlag(true, { errorEnabled: true }),
368-
withErrorFlag(false, { errorEnabled: false })
369-
)
370-
).toBe(true)
347+
it.concurrent('treats an unset flag as off', () => {
348+
const unset = createWorkflowState({ blocks: { block1: createBlock('block1') } })
349+
expect(hasWorkflowChanged(unset, withErrorFlag(false))).toBe(false)
350+
expect(hasWorkflowChanged(unset, withErrorFlag(true))).toBe(true)
371351
})
372352
})
373353

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

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,17 +47,6 @@ export const EXCLUDED_BLOCK_DATA_FIELDS: readonly string[] = [
4747
// Parallel fields - duplicated in parallels state and/or subBlocks
4848
'parallelType', // Duplicated in parallels state
4949
'distribution', // Parallel distribution (derived during execution)
50-
51-
/*
52-
* Duplicated from the block's own `errorEnabled`. `data` is where the flag
53-
* persists (the realtime server `jsonb_set`s it, and load mirrors it back
54-
* onto the block), so the same value reaches this comparison twice — and only
55-
* some paths populate the copy. Counted here, a block read one way differed
56-
* from the identical block read another and the deploy badge flipped between
57-
* Live and "Update deployment" as each query landed. The block field is
58-
* compared on its own, with `!!`, so absent and `false` agree there.
59-
*/
60-
'errorEnabled',
6150
] as const
6251

6352
/**

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ function toDbBlock(block: ReturnType<typeof createBlock>, workflowId: string) {
120120
horizontalHandles: block.horizontalHandles,
121121
advancedMode: block.advancedMode ?? false,
122122
triggerMode: block.triggerMode ?? false,
123+
errorEnabled: block.errorEnabled ?? false,
123124
height: block.height ?? 150,
124125
subBlocks: block.subBlocks ?? {},
125126
outputs: block.outputs ?? {},
@@ -1294,6 +1295,31 @@ describe('Database Helpers', () => {
12941295
dbHelpers.invalidateDeployedStateCache()
12951296
})
12961297

1298+
/**
1299+
* Every version before the error toggle drew that port unconditionally, so a
1300+
* snapshot carrying an error edge was taken from a block that had the output
1301+
* on. The migration backfilling the flag only reaches the live tables, never
1302+
* a version's frozen jsonb — so without deriving it here the deployed side
1303+
* reads `false` against a live `true`, and every workflow deployed before the
1304+
* toggle asks to be redeployed once.
1305+
*/
1306+
it('reads an error edge in an old snapshot as the error output being on', async () => {
1307+
const state = buildDeployedState()
1308+
state.edges.push({
1309+
id: 'edge-err',
1310+
source: 'block-1',
1311+
target: 'block-2',
1312+
sourceHandle: 'error',
1313+
targetHandle: 'input',
1314+
})
1315+
queueActiveVersion('dv-error-edge', state)
1316+
1317+
const deployed = await dbHelpers.loadDeployedWorkflowState('wf-error-edge', 'workspace-1')
1318+
1319+
expect(deployed?.blocks['block-1'].errorEnabled).toBe(true)
1320+
expect(deployed?.blocks['block-2'].errorEnabled).toBeUndefined()
1321+
})
1322+
12971323
it('serves a cache HIT, skipping migrations on the second call for the same active version', async () => {
12981324
queueActiveVersion('dv-hit', buildDeployedState())
12991325
queueActiveVersion('dv-hit', buildDeployedState())

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,38 @@ async function materializeDeploymentState(
172172
state.blocks || {},
173173
resolvedWorkspaceId
174174
)
175+
/*
176+
* Read straight out of the version's jsonb blob, so unlike every path that
177+
* goes through `loadWorkflowFromNormalizedTables` these handles were never
178+
* canonicalized. Change detection diffs this against a normalized live state,
179+
* so a snapshot holding a side-anchored id would report every edge as
180+
* added-and-removed and pin the workflow to "needs redeploy" forever.
181+
*/
182+
const edges = normalizeWorkflowEdgeHandles(state.edges)
183+
184+
/*
185+
* An error edge means the error output is on. Every version before the toggle
186+
* drew that port unconditionally, so a snapshot with such an edge was taken
187+
* from a block that had the output — and the migration backfilling the flag
188+
* only reaches the live tables, never a version's frozen jsonb. Without this
189+
* 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.
192+
*/
193+
const errorSourceBlockIds = new Set(
194+
edges.filter((edge) => edge.sourceHandle === 'error').map((edge) => edge.source)
195+
)
196+
const blocks: DeployedWorkflowData['blocks'] = {}
197+
for (const [blockId, block] of Object.entries(migratedBlocks)) {
198+
blocks[blockId] =
199+
block.errorEnabled || !errorSourceBlockIds.has(blockId)
200+
? block
201+
: { ...block, errorEnabled: true }
202+
}
203+
175204
const deployedState: DeployedWorkflowData = {
176-
blocks: migratedBlocks,
177-
/*
178-
* Read straight out of the version's jsonb blob, so unlike every path that
179-
* goes through `loadWorkflowFromNormalizedTables` these handles were never
180-
* canonicalized. Change detection diffs this against a normalized live
181-
* state, so a snapshot holding a side-anchored id would report every edge
182-
* as added-and-removed and pin the workflow to "needs redeploy" forever.
183-
*/
184-
edges: normalizeWorkflowEdgeHandles(state.edges),
205+
blocks,
206+
edges,
185207
loops: state.loops || {},
186208
parallels: state.parallels || {},
187209
variables: state.variables || {},

apps/sim/stores/workflows/workflow/store.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ export const useWorkflowStore = create<WorkflowStore>()(
172172
enabled: block.enabled ?? true,
173173
horizontalHandles: block.horizontalHandles ?? true,
174174
advancedMode: block.advancedMode ?? false,
175-
errorEnabled: block.errorEnabled ?? block.data?.errorEnabled === true,
175+
errorEnabled: block.errorEnabled ?? false,
176176
triggerMode: block.triggerMode ?? false,
177177
height: block.height ?? 0,
178178
data: block.data,
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
-- migration-safe: an additive column with a non-null default, so old and new app versions read and write these rows throughout the deploy.
2+
ALTER TABLE "workflow_blocks" ADD COLUMN "error_enabled" boolean DEFAULT false NOT NULL;--> statement-breakpoint
3+
-- Every released version draws the error port with no toggle in front of it, so a
4+
-- block someone already wired an error edge out of HAS the output on — there was
5+
-- no other way to draw that edge. Defaulting those rows to false would hide a port
6+
-- a live workflow routes failures through, so the edges decide the flag for every
7+
-- workflow that predates the toggle. `workflow-block.tsx` keeps the same rule at
8+
-- render time for states that never pass through here (imports, snapshots, copilot
9+
-- edits); do not narrow either one to read the flag alone.
10+
-- migration-safe: idempotent (sets a constant, on a column no released version reads or writes, so there is nothing to race); touches only the blocks that already have an error edge.
11+
UPDATE "workflow_blocks" AS b
12+
SET "error_enabled" = true
13+
FROM "workflow_edges" AS e
14+
WHERE e."source_block_id" = b."id" AND e."source_handle" = 'error';--> statement-breakpoint
15+
-- The flag also briefly persisted inside `data` on this unmerged branch and never
16+
-- reached production. These move any row a developer created onto the column and
17+
-- leave the value one home, so a block saved before the change stops differing
18+
-- from one saved after it. Both match zero rows on a database that never saw the key.
19+
-- migration-safe: idempotent (re-running sets the same value and removes an already-absent key); the key is written by no released version, so no concurrent writer can reintroduce it.
20+
UPDATE "workflow_blocks" SET "error_enabled" = true WHERE "data" ->> 'errorEnabled' = 'true';--> statement-breakpoint
21+
-- migration-safe: idempotent (re-running removes an already-absent key); the key is written by no released version, so no concurrent writer can reintroduce it.
22+
UPDATE "workflow_blocks" SET "data" = "data" - 'errorEnabled' WHERE "data" ->> 'errorEnabled' IS NOT NULL;

0 commit comments

Comments
 (0)