Skip to content

Commit 44b6ea0

Browse files
authored
fix(workflows): recover loads with legacy loop counts (#7843)
1 parent 84727a8 commit 44b6ea0

5 files changed

Lines changed: 198 additions & 14 deletions

File tree

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

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ import {
7171
type ConnectionBlockSelectorData,
7272
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/connection-block-selector/connection-block-selector'
7373
import { Cursors } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/cursors/cursors'
74-
import { ErrorBoundary } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/error/index'
74+
import {
75+
ErrorBoundary,
76+
ErrorUI,
77+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/error/index'
7578
import { FocusBlockDeepLink } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/focus-block-deep-link'
7679
import { WorkflowSearchReplace } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace'
7780
import { WorkflowControls } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-controls/workflow-controls'
@@ -2668,6 +2671,10 @@ const WorkflowContent = React.memo(
26682671
const loadingWorkflowRef = useRef<string | null>(null)
26692672
const currentWorkflowExists =
26702673
!isWorkflowMapPlaceholderData && Boolean(workflows[workflowIdParam])
2674+
const workflowLoadError =
2675+
hydration.phase === 'error' && hydration.workflowId === workflowIdParam
2676+
? hydration.error
2677+
: null
26712678

26722679
useEffect(() => {
26732680
const currentId = workflowIdParam
@@ -5126,16 +5133,28 @@ const WorkflowContent = React.memo(
51265133
>
51275134
{!isWorkflowReady && (
51285135
<div className='absolute inset-0 z-[5] flex items-center justify-center bg-[var(--bg)]'>
5129-
<div
5130-
className='size-[18px] animate-spin rounded-full'
5131-
style={{
5132-
background:
5133-
'conic-gradient(from 0deg, hsl(var(--muted-foreground)) 0deg 120deg, transparent 120deg 180deg, hsl(var(--muted-foreground)) 180deg 300deg, transparent 300deg 360deg)',
5134-
mask: 'radial-gradient(farthest-side, transparent calc(100% - 1.5px), black calc(100% - 1.5px))',
5135-
WebkitMask:
5136-
'radial-gradient(farthest-side, transparent calc(100% - 1.5px), black calc(100% - 1.5px))',
5137-
}}
5138-
/>
5136+
{workflowLoadError ? (
5137+
<ErrorUI
5138+
title='Unable to load workflow'
5139+
message={workflowLoadError}
5140+
onReset={() => {
5141+
setActiveWorkflow(workflowIdParam).catch((error) => {
5142+
logger.error(`Failed to retry workflow ${workflowIdParam}:`, error)
5143+
})
5144+
}}
5145+
/>
5146+
) : (
5147+
<div
5148+
className='size-[18px] animate-spin rounded-full'
5149+
style={{
5150+
background:
5151+
'conic-gradient(from 0deg, hsl(var(--muted-foreground)) 0deg 120deg, transparent 120deg 180deg, hsl(var(--muted-foreground)) 180deg 300deg, transparent 300deg 360deg)',
5152+
mask: 'radial-gradient(farthest-side, transparent calc(100% - 1.5px), black calc(100% - 1.5px))',
5153+
WebkitMask:
5154+
'radial-gradient(farthest-side, transparent calc(100% - 1.5px), black calc(100% - 1.5px))',
5155+
}}
5156+
/>
5157+
)}
51395158
</div>
51405159
)}
51415160

apps/sim/executor/orchestrators/loop.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ describe('LoopOrchestrator', () => {
154154
expect(loopEnd.incomingEdges.has(parallelEndId)).toBe(true)
155155
})
156156

157-
it('resolves forEach collections with the loop start sentinel scope', async () => {
157+
it('resolves forEach collections with the loop start sentinel scope independently of the count', async () => {
158158
const loopId = 'loop-1'
159159
const dag: DAG = {
160160
nodes: new Map(),
@@ -165,14 +165,15 @@ describe('LoopOrchestrator', () => {
165165
id: loopId,
166166
nodes: ['task-1'],
167167
loopType: 'forEach',
168+
iterations: 1,
168169
forEachItems: '<Producer.items>',
169170
},
170171
],
171172
]),
172173
parallelConfigs: new Map(),
173174
}
174175
const resolver = {
175-
resolveSingleReference: vi.fn().mockResolvedValue(['item-1']),
176+
resolveSingleReference: vi.fn().mockResolvedValue(['item-1', 'item-2', 'item-3']),
176177
}
177178
const orchestrator = new LoopOrchestrator(dag, createState(), resolver as any, {}, {
178179
clearDeactivatedEdgesForNodes: vi.fn(),
@@ -188,7 +189,8 @@ describe('LoopOrchestrator', () => {
188189
undefined,
189190
{ allowLargeValueRefs: true }
190191
)
191-
expect(scope.maxIterations).toBe(1)
192+
expect(scope.maxIterations).toBe(3)
193+
expect(scope.items).toEqual(['item-1', 'item-2', 'item-3'])
192194
})
193195

194196
it('projects forEach resolution failures before logging or persisting them', async () => {

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

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ import {
2323
schemaMock,
2424
} from '@sim/testing'
2525
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
26+
import { workflowStateSchema } from '@/lib/api/contracts/workflows'
2627
import type {
2728
BlockState as AppBlockState,
2829
WorkflowState as AppWorkflowState,
2930
} from '@/stores/workflows/workflow/types'
31+
import { generateLoopBlocks } from '@/stores/workflows/workflow/utils'
3032

3133
/**
3234
* Type helper for converting test workflow state to app workflow state.
@@ -348,6 +350,110 @@ describe('Database Helpers', () => {
348350
})
349351

350352
describe('loadWorkflowFromNormalizedTables', () => {
353+
it.each(['for', 'forEach', 'while', 'doWhile'] as const)(
354+
'preserves valid block counts and expressions for %s loops even when subflow counts differ',
355+
async (loopType) => {
356+
const data = {
357+
count: 9,
358+
loopType,
359+
collection: '<source.items>',
360+
whileCondition: '<source.hasMore>',
361+
doWhileCondition: '<source.hasMore>',
362+
width: 600,
363+
parentId: 'outer-loop',
364+
extent: 'parent' as const,
365+
}
366+
queueLoadFixtures({
367+
blocks: [{ ...toDbBlock(createLoopBlock({ id: 'loop-1' }), mockWorkflowId), data }],
368+
subflows: [
369+
{
370+
id: 'loop-1',
371+
type: 'loop',
372+
config: {
373+
nodes: [],
374+
loopType,
375+
iterations: 3,
376+
forEachItems: data.collection,
377+
whileCondition: data.whileCondition,
378+
doWhileCondition: data.doWhileCondition,
379+
},
380+
},
381+
],
382+
})
383+
384+
const loaded = await dbHelpers.loadWorkflowFromNormalizedTables(mockWorkflowId)
385+
const parsed = workflowStateSchema.parse(loaded)
386+
387+
expect(parsed.blocks['loop-1'].data).toEqual(data)
388+
expect(parsed.loops?.['loop-1'].iterations).toBe(3)
389+
expect(generateLoopBlocks(loaded!.blocks)['loop-1']).toMatchObject({
390+
iterations: 9,
391+
loopType,
392+
forEachItems: data.collection,
393+
whileCondition: data.whileCondition,
394+
doWhileCondition: data.doWhileCondition,
395+
})
396+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
397+
}
398+
)
399+
400+
it('keeps an absent block count absent so serialization retains its existing default', async () => {
401+
queueLoadFixtures({
402+
blocks: [
403+
{
404+
...toDbBlock(createLoopBlock({ id: 'loop-1' }), mockWorkflowId),
405+
data: { loopType: 'for' },
406+
},
407+
],
408+
subflows: [
409+
{ id: 'loop-1', type: 'loop', config: { nodes: [], loopType: 'for', iterations: 3 } },
410+
],
411+
})
412+
413+
const loaded = await dbHelpers.loadWorkflowFromNormalizedTables(mockWorkflowId)
414+
415+
expect(loaded?.blocks['loop-1'].data?.count).toBeUndefined()
416+
expect(loaded?.loops['loop-1'].iterations).toBe(3)
417+
expect(generateLoopBlocks(loaded!.blocks)['loop-1'].iterations).toBe(5)
418+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
419+
})
420+
421+
it('serves a legacy forEach loop with a string count through the workflow read contract', async () => {
422+
const collection = '<source.items>'
423+
const loopRow = toDbBlock(createLoopBlock({ id: 'loop-1' }), mockWorkflowId)
424+
queueLoadFixtures({
425+
blocks: [
426+
{
427+
...loopRow,
428+
data: { ...loopRow.data, loopType: 'forEach', count: collection, collection },
429+
},
430+
],
431+
subflows: [
432+
{
433+
id: 'loop-1',
434+
type: 'loop',
435+
config: {
436+
nodes: [],
437+
loopType: 'forEach',
438+
iterations: collection,
439+
forEachItems: collection,
440+
},
441+
},
442+
],
443+
})
444+
445+
const loaded = await dbHelpers.loadWorkflowFromNormalizedTables(mockWorkflowId)
446+
const parsed = workflowStateSchema.parse(loaded)
447+
448+
expect(parsed.blocks['loop-1'].data).toMatchObject({ count: 1, collection })
449+
expect(parsed.loops?.['loop-1']).toMatchObject({
450+
loopType: 'forEach',
451+
iterations: 1,
452+
forEachItems: collection,
453+
})
454+
expect(dbChainMockFns.update).not.toHaveBeenCalled()
455+
})
456+
351457
it('should successfully load workflow data from normalized tables', async () => {
352458
queueLoadFixtures({
353459
blocks: mockBlocksFromDb,
@@ -401,6 +507,7 @@ describe('Database Helpers', () => {
401507
whileCondition: '',
402508
enabled: true,
403509
})
510+
expect(result?.blocks['loop-1'].data?.count).toBe(3)
404511

405512
expect(result?.parallels['parallel-1']).toEqual({
406513
id: 'parallel-1',

apps/sim/stores/workflows/registry/store.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,35 @@ describe('registry store loadWorkflowState (collapsed cache)', () => {
228228
expect(mockRequestJson).toHaveBeenCalledTimes(2)
229229
})
230230

231+
it('exposes a failed load and recovers when the user retries the same workflow', async () => {
232+
mockRequestJson.mockRejectedValueOnce(new Error('Unable to fetch workflow'))
233+
234+
await expect(useWorkflowRegistry.getState().setActiveWorkflow('wf-1')).rejects.toThrow(
235+
'Unable to fetch workflow'
236+
)
237+
expect(useWorkflowRegistry.getState().hydration).toMatchObject({
238+
phase: 'error',
239+
workflowId: 'wf-1',
240+
error: 'Unable to fetch workflow',
241+
})
242+
expect(replaceWorkflowState).not.toHaveBeenCalled()
243+
244+
mockRequestJson.mockResolvedValueOnce({ data: makeEnvelope() })
245+
const retry = useWorkflowRegistry.getState().setActiveWorkflow('wf-1')
246+
expect(useWorkflowRegistry.getState().hydration).toMatchObject({
247+
phase: 'state-loading',
248+
error: null,
249+
})
250+
await retry
251+
252+
expect(mockRequestJson).toHaveBeenCalledTimes(2)
253+
expect(useWorkflowRegistry.getState().hydration).toMatchObject({
254+
phase: 'ready',
255+
workflowId: 'wf-1',
256+
error: null,
257+
})
258+
})
259+
231260
it('discards a superseded response via the staleness guard', async () => {
232261
// First load (wf-1) is in-flight; a second load (wf-2) supersedes the
233262
// hydration workflowId, then wf-1 finally resolves. The guard compares the
@@ -256,4 +285,26 @@ describe('registry store loadWorkflowState (collapsed cache)', () => {
256285
expect(replaceWorkflowState.mock.calls.length).toBe(projectionsAfterSecond)
257286
expect(useWorkflowRegistry.getState().activeWorkflowId).toBe('wf-2')
258287
})
288+
289+
it('does not show a stale load error after switching to another workflow', async () => {
290+
let rejectFirst: (reason: Error) => void = () => {}
291+
const firstPending = new Promise<never>((_resolve, reject) => {
292+
rejectFirst = reject
293+
})
294+
mockRequestJson
295+
.mockImplementationOnce(() => firstPending)
296+
.mockResolvedValueOnce({ data: makeEnvelope({ id: 'wf-2' }) })
297+
298+
const firstLoad = useWorkflowRegistry.getState().setActiveWorkflow('wf-1')
299+
await useWorkflowRegistry.getState().setActiveWorkflow('wf-2')
300+
rejectFirst(new Error('Previous workflow failed to load'))
301+
await firstLoad
302+
303+
expect(useWorkflowRegistry.getState().activeWorkflowId).toBe('wf-2')
304+
expect(useWorkflowRegistry.getState().hydration).toMatchObject({
305+
phase: 'ready',
306+
workflowId: 'wf-2',
307+
error: null,
308+
})
309+
})
259310
})

packages/workflow-persistence/src/load.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,11 @@ export async function loadWorkflowFromNormalizedTablesRaw(
204204
...block,
205205
data: {
206206
...block.data,
207+
/** Repair legacy values without changing valid counts used by serialization. */
208+
count:
209+
block.data?.count === undefined || typeof block.data.count === 'number'
210+
? block.data?.count
211+
: loop.iterations,
207212
collection: loop.forEachItems ?? block.data?.collection ?? '',
208213
whileCondition: loop.whileCondition ?? block.data?.whileCondition ?? '',
209214
doWhileCondition: loop.doWhileCondition ?? block.data?.doWhileCondition ?? '',

0 commit comments

Comments
 (0)