Skip to content

Commit ff23546

Browse files
waleedlatif1claude
andauthored
fix(workflows): exclude block locked from diff detection (#4631)
Toggling a block's locked state is UI metadata and should not register as a workflow drift/diff. Strip locked from hasBlockChanged, computeFieldDiff, the compare.ts blockFields list, and from extractBlockFieldsForComparison so it's also excluded from the normalized stringify-based block equality check used by drift detection and hashing. Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 3712d1e commit ff23546

6 files changed

Lines changed: 110 additions & 50 deletions

File tree

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -297,14 +297,14 @@ describe('hasWorkflowChanged', () => {
297297
expect(hasWorkflowChanged(state1, state2)).toBe(true)
298298
})
299299

300-
it.concurrent('should detect locked/unlocked changes', () => {
300+
it.concurrent('should not detect locked/unlocked toggle as a workflow change', () => {
301301
const state1 = createWorkflowState({
302302
blocks: { block1: createBlock('block1', { locked: false }) },
303303
})
304304
const state2 = createWorkflowState({
305305
blocks: { block1: createBlock('block1', { locked: true }) },
306306
})
307-
expect(hasWorkflowChanged(state1, state2)).toBe(true)
307+
expect(hasWorkflowChanged(state1, state2)).toBe(false)
308308
})
309309

310310
it.concurrent('should not detect changes when locked state is the same', () => {
@@ -316,6 +316,18 @@ describe('hasWorkflowChanged', () => {
316316
})
317317
expect(hasWorkflowChanged(state1, state2)).toBe(false)
318318
})
319+
320+
it.concurrent('should not include locked changes in diff summary', () => {
321+
const state1 = createWorkflowState({
322+
blocks: { block1: createBlock('block1', { locked: false }) },
323+
})
324+
const state2 = createWorkflowState({
325+
blocks: { block1: createBlock('block1', { locked: true }) },
326+
})
327+
const summary = generateWorkflowDiffSummary(state2, state1)
328+
expect(summary.hasChanges).toBe(false)
329+
expect(summary.modifiedBlocks).toEqual([])
330+
})
319331
})
320332

321333
describe('SubBlock Changes', () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ export function generateWorkflowDiffSummary(
195195
newValue: currentBlock.enabled,
196196
})
197197
}
198-
const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode', 'locked'] as const
198+
const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode'] as const
199199
for (const field of blockFields) {
200200
if (!!currentBlock[field] !== !!previousBlock[field]) {
201201
changes.push({

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
* Tests for workflow normalization utilities
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import type { Loop, Parallel } from '@/stores/workflows/workflow/types'
5+
import type { BlockState, Loop, Parallel } from '@/stores/workflows/workflow/types'
66
import {
7+
extractBlockFieldsForComparison,
78
filterSubBlockIds,
89
normalizedStringify,
910
normalizeEdge,
@@ -829,4 +830,45 @@ describe('Workflow Normalization Utilities', () => {
829830
expect(normalized.placeholder).toBe('Enter signing secret')
830831
})
831832
})
833+
834+
describe('extractBlockFieldsForComparison', () => {
835+
function createBlock(overrides: Partial<BlockState> = {}): BlockState {
836+
return {
837+
id: 'block-1',
838+
type: 'agent',
839+
name: 'Test',
840+
enabled: true,
841+
position: { x: 0, y: 0 },
842+
subBlocks: {},
843+
outputs: {},
844+
...overrides,
845+
} as BlockState
846+
}
847+
848+
it.concurrent('should strip the locked field from blockRest', () => {
849+
const { blockRest } = extractBlockFieldsForComparison(createBlock({ locked: true }))
850+
expect((blockRest as Record<string, unknown>).locked).toBeUndefined()
851+
})
852+
853+
it.concurrent(
854+
'should yield identical blockRest when only locked differs between two blocks',
855+
() => {
856+
const lockedBlock = createBlock({ locked: true })
857+
const unlockedBlock = createBlock({ locked: false })
858+
const { blockRest: lockedRest } = extractBlockFieldsForComparison(lockedBlock)
859+
const { blockRest: unlockedRest } = extractBlockFieldsForComparison(unlockedBlock)
860+
expect(normalizedStringify(lockedRest)).toBe(normalizedStringify(unlockedRest))
861+
}
862+
)
863+
864+
it.concurrent('should keep functional fields like name and enabled', () => {
865+
const { blockRest } = extractBlockFieldsForComparison(
866+
createBlock({ name: 'A', enabled: false, locked: true })
867+
)
868+
const rest = blockRest as Record<string, unknown>
869+
expect(rest.name).toBe('A')
870+
expect(rest.enabled).toBe(false)
871+
expect(rest.locked).toBeUndefined()
872+
})
873+
})
832874
})

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ export function normalizeBlockData(
386386

387387
/**
388388
* Extracts block fields for comparison, excluding visual-only and runtime fields.
389-
* Excludes: position, layout, height, outputs, is_diff, field_diffs
389+
* Excludes: position, layout, height, outputs, is_diff, field_diffs, locked
390390
*
391391
* @param block - The block state
392392
* @returns Extracted fields suitable for comparison
@@ -401,6 +401,7 @@ export function extractBlockFieldsForComparison(block: BlockState): ExtractedBlo
401401
outputs: _outputs,
402402
is_diff: _isDiff,
403403
field_diffs: _fieldDiffs,
404+
locked: _locked,
404405
...blockRest
405406
} = blockWithDiff
406407

@@ -503,7 +504,7 @@ export function extractSubBlockRest(subBlock: Record<string, unknown>): Record<s
503504

504505
/**
505506
* Normalizes a workflow state for comparison or hashing.
506-
* Excludes non-functional fields (position, layout, height, outputs, diff markers)
507+
* Excludes non-functional fields (position, layout, height, outputs, diff markers, locked)
507508
* and system/trigger runtime subBlocks.
508509
*
509510
* @param state - The workflow state to normalize

apps/sim/lib/workflows/diff/diff-engine.test.ts

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ describe('WorkflowDiffEngine', () => {
136136
describe('hasBlockChanged detection', () => {
137137
describe('locked state changes', () => {
138138
it.concurrent(
139-
'should detect when block locked state changes from false to true',
139+
'should NOT detect a diff when only the locked state changes (false -> true)',
140140
async () => {
141141
const freshEngine = new WorkflowDiffEngine()
142142
const baseline = createMockWorkflowState({
@@ -154,7 +154,10 @@ describe('WorkflowDiffEngine', () => {
154154
)
155155

156156
expect(result.success).toBe(true)
157-
expect(result.diff?.diffAnalysis?.edited_blocks).toContain('block-1')
157+
expect(result.diff?.diffAnalysis?.edited_blocks ?? []).not.toContain('block-1')
158+
expect(
159+
result.diff?.diffAnalysis?.field_diffs?.['block-1']?.changed_fields ?? []
160+
).not.toContain('locked')
158161
}
159162
)
160163

@@ -171,43 +174,57 @@ describe('WorkflowDiffEngine', () => {
171174
const result = await freshEngine.createDiffFromWorkflowState(proposed, undefined, baseline)
172175

173176
expect(result.success).toBe(true)
174-
expect(result.diff?.diffAnalysis?.edited_blocks).not.toContain('block-1')
177+
expect(result.diff?.diffAnalysis?.edited_blocks ?? []).not.toContain('block-1')
175178
})
176179

177-
it.concurrent('should detect change when locked goes from undefined to true', async () => {
178-
const freshEngine = new WorkflowDiffEngine()
179-
const baseline = createMockWorkflowState({
180-
'block-1': createMockBlock({ id: 'block-1' }), // locked undefined
181-
})
180+
it.concurrent(
181+
'should NOT detect a diff when locked goes from undefined to true',
182+
async () => {
183+
const freshEngine = new WorkflowDiffEngine()
184+
const baseline = createMockWorkflowState({
185+
'block-1': createMockBlock({ id: 'block-1' }),
186+
})
182187

183-
const proposed = createMockWorkflowState({
184-
'block-1': createMockBlock({ id: 'block-1', locked: true }),
185-
})
188+
const proposed = createMockWorkflowState({
189+
'block-1': createMockBlock({ id: 'block-1', locked: true }),
190+
})
186191

187-
const result = await freshEngine.createDiffFromWorkflowState(proposed, undefined, baseline)
192+
const result = await freshEngine.createDiffFromWorkflowState(
193+
proposed,
194+
undefined,
195+
baseline
196+
)
188197

189-
expect(result.success).toBe(true)
190-
// The hasBlockChanged function uses !!locked for comparison
191-
// so undefined -> true should be detected as a change
192-
expect(result.diff?.diffAnalysis?.edited_blocks).toContain('block-1')
193-
})
198+
expect(result.success).toBe(true)
199+
expect(result.diff?.diffAnalysis?.edited_blocks ?? []).not.toContain('block-1')
200+
}
201+
)
194202

195-
it.concurrent('should not detect change when both locked states are falsy', async () => {
196-
const freshEngine = new WorkflowDiffEngine()
197-
const baseline = createMockWorkflowState({
198-
'block-1': createMockBlock({ id: 'block-1' }), // locked undefined
199-
})
203+
it.concurrent(
204+
'should still detect real edits on a block whose locked state also changed',
205+
async () => {
206+
const freshEngine = new WorkflowDiffEngine()
207+
const baseline = createMockWorkflowState({
208+
'block-1': createMockBlock({ id: 'block-1', enabled: true, locked: false }),
209+
})
200210

201-
const proposed = createMockWorkflowState({
202-
'block-1': createMockBlock({ id: 'block-1', locked: false }), // locked false
203-
})
211+
const proposed = createMockWorkflowState({
212+
'block-1': createMockBlock({ id: 'block-1', enabled: false, locked: true }),
213+
})
204214

205-
const result = await freshEngine.createDiffFromWorkflowState(proposed, undefined, baseline)
215+
const result = await freshEngine.createDiffFromWorkflowState(
216+
proposed,
217+
undefined,
218+
baseline
219+
)
206220

207-
expect(result.success).toBe(true)
208-
// undefined and false should both be falsy, so !! comparison makes them equal
209-
expect(result.diff?.diffAnalysis?.edited_blocks).not.toContain('block-1')
210-
})
221+
expect(result.success).toBe(true)
222+
expect(result.diff?.diffAnalysis?.edited_blocks).toContain('block-1')
223+
const changed = result.diff?.diffAnalysis?.field_diffs?.['block-1']?.changed_fields ?? []
224+
expect(changed).toContain('enabled')
225+
expect(changed).not.toContain('locked')
226+
}
227+
)
211228
})
212229

213230
describe('parent scope changes', () => {

apps/sim/lib/workflows/diff/diff-engine.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ function hasBlockChanged(currentBlock: BlockState, proposedBlock: BlockState): b
4242
if (currentBlock.name !== proposedBlock.name) return true
4343
if (currentBlock.enabled !== proposedBlock.enabled) return true
4444
if (currentBlock.triggerMode !== proposedBlock.triggerMode) return true
45-
if (!!currentBlock.locked !== !!proposedBlock.locked) return true
4645
if ((currentBlock.data?.parentId ?? null) !== (proposedBlock.data?.parentId ?? null)) return true
4746

4847
// Compare subBlocks
@@ -74,22 +73,11 @@ function computeFieldDiff(
7473
const unchangedFields: string[] = []
7574

7675
// Check basic fields
77-
const fieldsToCheck = [
78-
'type',
79-
'name',
80-
'enabled',
81-
'triggerMode',
82-
'horizontalHandles',
83-
'locked',
84-
] as const
76+
const fieldsToCheck = ['type', 'name', 'enabled', 'triggerMode', 'horizontalHandles'] as const
8577
for (const field of fieldsToCheck) {
8678
const currentValue = currentBlock[field]
8779
const proposedValue = proposedBlock[field]
88-
if (
89-
field === 'locked'
90-
? !!currentValue !== !!proposedValue
91-
: JSON.stringify(currentValue) !== JSON.stringify(proposedValue)
92-
) {
80+
if (JSON.stringify(currentValue) !== JSON.stringify(proposedValue)) {
9381
changedFields.push(field)
9482
} else if (currentValue !== undefined) {
9583
unchangedFields.push(field)

0 commit comments

Comments
 (0)