Skip to content

Commit 9566068

Browse files
committed
fix(workflows): fingerprint only stored rows so container child order cannot raise a false conflict
1 parent 697933c commit 9566068

3 files changed

Lines changed: 41 additions & 6 deletions

File tree

‎apps/sim/lib/workflows/application/apply-workflow-operations.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,30 @@ describe('applyWorkflowOperations', () => {
514514

515515
expect(result.dryRun).toBe(false)
516516
})
517+
518+
/** Container `nodes` are rebuilt from the block rows, so they inherit the read's row order. */
519+
it('commits when only container child order differs on the re-read', async () => {
520+
const containers = (nodes: string[]) => ({
521+
loops: { 'loop-1': { id: 'loop-1', nodes, iterations: 1, loopType: 'for' } },
522+
parallels: { 'parallel-1': { id: 'parallel-1', nodes, count: 2, parallelType: 'count' } },
523+
})
524+
mocks.normalizeState
525+
.mockReturnValueOnce({
526+
state: { ...graph(), ...containers(['block-1', 'block-2']) },
527+
warnings: [],
528+
})
529+
.mockReturnValueOnce({
530+
state: { ...graph(), ...containers(['block-2', 'block-1']) },
531+
warnings: [],
532+
})
533+
534+
const result = await applyWorkflowOperations.execute({
535+
principal: sessionPrincipal,
536+
input: { workflowId: 'workflow-1', operations },
537+
})
538+
539+
expect(result.dryRun).toBe(false)
540+
})
517541
})
518542

519543
/**

‎apps/sim/lib/workflows/application/apply-workflow-operations.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,10 +226,14 @@ function applyBlockEnabledChanges(
226226
* An order-insensitive identity for a stored graph, so two reads of the same
227227
* rows compare equal even though the loader neither orders its rows nor fixes
228228
* the key order of the records it builds.
229+
*
230+
* Only blocks and edges are compared: loops and parallels are rebuilt from the
231+
* blocks on every load, with child `nodes` in row order.
229232
*/
230233
function storedGraphFingerprint(graph: Record<string, unknown>): string {
231-
const edges = [...(asGraph(graph).edges ?? [])].sort((a, b) => compareStrings(a.id, b.id))
232-
return JSON.stringify(sortObjectKeysDeep({ ...graph, edges }))
234+
const { blocks, edges } = asGraph(graph)
235+
const sortedEdges = [...(edges ?? [])].sort((a, b) => compareStrings(a.id, b.id))
236+
return JSON.stringify(sortObjectKeysDeep({ blocks, edges: sortedEdges }))
233237
}
234238

235239
interface BaseGraph {

‎apps/sim/lib/workflows/application/custom-block-authoring.test.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ function graph(includeCustom = false) {
154154
}
155155
}
156156

157+
/** Resolves the graph the write is given, running its reader the way the row-locked transaction does. */
158+
async function resolveWrittenState(state: unknown) {
159+
return typeof state === 'function' ? state({}) : state
160+
}
161+
157162
const add: EditWorkflowOperation = {
158163
operation_type: 'add',
159164
block_id: 'new-worker',
@@ -171,7 +176,9 @@ describe('custom blocks in authorized workflow authoring', () => {
171176
workflowRecord: context.workflow,
172177
normalizedData: graph(true),
173178
}))
174-
mocks.replace.mockImplementation(async ({ state }) => prepareWorkflowStateForPersistence(state))
179+
mocks.replace.mockImplementation(async ({ state }) =>
180+
prepareWorkflowStateForPersistence(await resolveWrittenState(state))
181+
)
175182
})
176183

177184
it.each([session, copilot])(
@@ -218,7 +225,7 @@ describe('custom blocks in authorized workflow authoring', () => {
218225
expect(result.lint.fieldIssues).toEqual([])
219226
expect(mocks.replace).toHaveBeenCalledTimes(dryRun ? 0 : 1)
220227
if (!dryRun) {
221-
const persisted = mocks.replace.mock.calls[0][0].state
228+
const { state: persisted } = await mocks.replace.mock.results[0].value
222229
expect(persisted.blocks.worker.subBlocks['input-text']).toMatchObject({
223230
value: 'updated',
224231
type: 'short-input',
@@ -265,7 +272,7 @@ describe('custom blocks in authorized workflow authoring', () => {
265272
mocks.replace.mockImplementation(async ({ state, workspaceId }) => {
266273
const foreignType = workspaceId === 'one' ? other.type : customBlock.type
267274
expect(getBlock(foreignType)).toBeUndefined()
268-
return prepareWorkflowStateForPersistence(state)
275+
return prepareWorkflowStateForPersistence(await resolveWrittenState(state))
269276
})
270277
const results = await Promise.all(
271278
[customBlock, other].map((row, index) =>
@@ -298,7 +305,7 @@ describe('custom blocks in authorized workflow authoring', () => {
298305
expect(result.lint.fieldIssues).toEqual([])
299306
expect(mocks.replace).toHaveBeenCalledTimes(dryRun ? 0 : 1)
300307
if (!dryRun) {
301-
const persisted = mocks.replace.mock.calls[0][0].state
308+
const { state: persisted } = await mocks.replace.mock.results[0].value
302309
expect(persisted.blocks.worker.subBlocks['input-text']).toMatchObject({
303310
value: 'hello',
304311
type: 'short-input',

0 commit comments

Comments
 (0)