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>
This commit is contained in:
Waleed
2026-05-15 19:23:03 -07:00
committed by GitHub
parent 3712d1e2df
commit ff23546f30
6 changed files with 110 additions and 50 deletions
@@ -297,14 +297,14 @@ describe('hasWorkflowChanged', () => {
expect(hasWorkflowChanged(state1, state2)).toBe(true)
})
it.concurrent('should detect locked/unlocked changes', () => {
it.concurrent('should not detect locked/unlocked toggle as a workflow change', () => {
const state1 = createWorkflowState({
blocks: { block1: createBlock('block1', { locked: false }) },
})
const state2 = createWorkflowState({
blocks: { block1: createBlock('block1', { locked: true }) },
})
expect(hasWorkflowChanged(state1, state2)).toBe(true)
expect(hasWorkflowChanged(state1, state2)).toBe(false)
})
it.concurrent('should not detect changes when locked state is the same', () => {
@@ -316,6 +316,18 @@ describe('hasWorkflowChanged', () => {
})
expect(hasWorkflowChanged(state1, state2)).toBe(false)
})
it.concurrent('should not include locked changes in diff summary', () => {
const state1 = createWorkflowState({
blocks: { block1: createBlock('block1', { locked: false }) },
})
const state2 = createWorkflowState({
blocks: { block1: createBlock('block1', { locked: true }) },
})
const summary = generateWorkflowDiffSummary(state2, state1)
expect(summary.hasChanges).toBe(false)
expect(summary.modifiedBlocks).toEqual([])
})
})
describe('SubBlock Changes', () => {
+1 -1
View File
@@ -195,7 +195,7 @@ export function generateWorkflowDiffSummary(
newValue: currentBlock.enabled,
})
}
const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode', 'locked'] as const
const blockFields = ['horizontalHandles', 'advancedMode', 'triggerMode'] as const
for (const field of blockFields) {
if (!!currentBlock[field] !== !!previousBlock[field]) {
changes.push({
@@ -2,8 +2,9 @@
* Tests for workflow normalization utilities
*/
import { describe, expect, it } from 'vitest'
import type { Loop, Parallel } from '@/stores/workflows/workflow/types'
import type { BlockState, Loop, Parallel } from '@/stores/workflows/workflow/types'
import {
extractBlockFieldsForComparison,
filterSubBlockIds,
normalizedStringify,
normalizeEdge,
@@ -829,4 +830,45 @@ describe('Workflow Normalization Utilities', () => {
expect(normalized.placeholder).toBe('Enter signing secret')
})
})
describe('extractBlockFieldsForComparison', () => {
function createBlock(overrides: Partial<BlockState> = {}): BlockState {
return {
id: 'block-1',
type: 'agent',
name: 'Test',
enabled: true,
position: { x: 0, y: 0 },
subBlocks: {},
outputs: {},
...overrides,
} as BlockState
}
it.concurrent('should strip the locked field from blockRest', () => {
const { blockRest } = extractBlockFieldsForComparison(createBlock({ locked: true }))
expect((blockRest as Record<string, unknown>).locked).toBeUndefined()
})
it.concurrent(
'should yield identical blockRest when only locked differs between two blocks',
() => {
const lockedBlock = createBlock({ locked: true })
const unlockedBlock = createBlock({ locked: false })
const { blockRest: lockedRest } = extractBlockFieldsForComparison(lockedBlock)
const { blockRest: unlockedRest } = extractBlockFieldsForComparison(unlockedBlock)
expect(normalizedStringify(lockedRest)).toBe(normalizedStringify(unlockedRest))
}
)
it.concurrent('should keep functional fields like name and enabled', () => {
const { blockRest } = extractBlockFieldsForComparison(
createBlock({ name: 'A', enabled: false, locked: true })
)
const rest = blockRest as Record<string, unknown>
expect(rest.name).toBe('A')
expect(rest.enabled).toBe(false)
expect(rest.locked).toBeUndefined()
})
})
})
@@ -386,7 +386,7 @@ export function normalizeBlockData(
/**
* Extracts block fields for comparison, excluding visual-only and runtime fields.
* Excludes: position, layout, height, outputs, is_diff, field_diffs
* Excludes: position, layout, height, outputs, is_diff, field_diffs, locked
*
* @param block - The block state
* @returns Extracted fields suitable for comparison
@@ -401,6 +401,7 @@ export function extractBlockFieldsForComparison(block: BlockState): ExtractedBlo
outputs: _outputs,
is_diff: _isDiff,
field_diffs: _fieldDiffs,
locked: _locked,
...blockRest
} = blockWithDiff
@@ -503,7 +504,7 @@ export function extractSubBlockRest(subBlock: Record<string, unknown>): Record<s
/**
* Normalizes a workflow state for comparison or hashing.
* Excludes non-functional fields (position, layout, height, outputs, diff markers)
* Excludes non-functional fields (position, layout, height, outputs, diff markers, locked)
* and system/trigger runtime subBlocks.
*
* @param state - The workflow state to normalize
+47 -30
View File
@@ -136,7 +136,7 @@ describe('WorkflowDiffEngine', () => {
describe('hasBlockChanged detection', () => {
describe('locked state changes', () => {
it.concurrent(
'should detect when block locked state changes from false to true',
'should NOT detect a diff when only the locked state changes (false -> true)',
async () => {
const freshEngine = new WorkflowDiffEngine()
const baseline = createMockWorkflowState({
@@ -154,7 +154,10 @@ describe('WorkflowDiffEngine', () => {
)
expect(result.success).toBe(true)
expect(result.diff?.diffAnalysis?.edited_blocks).toContain('block-1')
expect(result.diff?.diffAnalysis?.edited_blocks ?? []).not.toContain('block-1')
expect(
result.diff?.diffAnalysis?.field_diffs?.['block-1']?.changed_fields ?? []
).not.toContain('locked')
}
)
@@ -171,43 +174,57 @@ describe('WorkflowDiffEngine', () => {
const result = await freshEngine.createDiffFromWorkflowState(proposed, undefined, baseline)
expect(result.success).toBe(true)
expect(result.diff?.diffAnalysis?.edited_blocks).not.toContain('block-1')
expect(result.diff?.diffAnalysis?.edited_blocks ?? []).not.toContain('block-1')
})
it.concurrent('should detect change when locked goes from undefined to true', async () => {
const freshEngine = new WorkflowDiffEngine()
const baseline = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1' }), // locked undefined
})
it.concurrent(
'should NOT detect a diff when locked goes from undefined to true',
async () => {
const freshEngine = new WorkflowDiffEngine()
const baseline = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1' }),
})
const proposed = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1', locked: true }),
})
const proposed = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1', locked: true }),
})
const result = await freshEngine.createDiffFromWorkflowState(proposed, undefined, baseline)
const result = await freshEngine.createDiffFromWorkflowState(
proposed,
undefined,
baseline
)
expect(result.success).toBe(true)
// The hasBlockChanged function uses !!locked for comparison
// so undefined -> true should be detected as a change
expect(result.diff?.diffAnalysis?.edited_blocks).toContain('block-1')
})
expect(result.success).toBe(true)
expect(result.diff?.diffAnalysis?.edited_blocks ?? []).not.toContain('block-1')
}
)
it.concurrent('should not detect change when both locked states are falsy', async () => {
const freshEngine = new WorkflowDiffEngine()
const baseline = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1' }), // locked undefined
})
it.concurrent(
'should still detect real edits on a block whose locked state also changed',
async () => {
const freshEngine = new WorkflowDiffEngine()
const baseline = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1', enabled: true, locked: false }),
})
const proposed = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1', locked: false }), // locked false
})
const proposed = createMockWorkflowState({
'block-1': createMockBlock({ id: 'block-1', enabled: false, locked: true }),
})
const result = await freshEngine.createDiffFromWorkflowState(proposed, undefined, baseline)
const result = await freshEngine.createDiffFromWorkflowState(
proposed,
undefined,
baseline
)
expect(result.success).toBe(true)
// undefined and false should both be falsy, so !! comparison makes them equal
expect(result.diff?.diffAnalysis?.edited_blocks).not.toContain('block-1')
})
expect(result.success).toBe(true)
expect(result.diff?.diffAnalysis?.edited_blocks).toContain('block-1')
const changed = result.diff?.diffAnalysis?.field_diffs?.['block-1']?.changed_fields ?? []
expect(changed).toContain('enabled')
expect(changed).not.toContain('locked')
}
)
})
describe('parent scope changes', () => {
+2 -14
View File
@@ -42,7 +42,6 @@ function hasBlockChanged(currentBlock: BlockState, proposedBlock: BlockState): b
if (currentBlock.name !== proposedBlock.name) return true
if (currentBlock.enabled !== proposedBlock.enabled) return true
if (currentBlock.triggerMode !== proposedBlock.triggerMode) return true
if (!!currentBlock.locked !== !!proposedBlock.locked) return true
if ((currentBlock.data?.parentId ?? null) !== (proposedBlock.data?.parentId ?? null)) return true
// Compare subBlocks
@@ -74,22 +73,11 @@ function computeFieldDiff(
const unchangedFields: string[] = []
// Check basic fields
const fieldsToCheck = [
'type',
'name',
'enabled',
'triggerMode',
'horizontalHandles',
'locked',
] as const
const fieldsToCheck = ['type', 'name', 'enabled', 'triggerMode', 'horizontalHandles'] as const
for (const field of fieldsToCheck) {
const currentValue = currentBlock[field]
const proposedValue = proposedBlock[field]
if (
field === 'locked'
? !!currentValue !== !!proposedValue
: JSON.stringify(currentValue) !== JSON.stringify(proposedValue)
) {
if (JSON.stringify(currentValue) !== JSON.stringify(proposedValue)) {
changedFields.push(field)
} else if (currentValue !== undefined) {
unchangedFields.push(field)