Skip to content

Commit 6b355e9

Browse files
waleedlatif1claude
waleedlatif1
and
claude
authored
fix(subflows): recurse into all descendants for lock, enable, and protection checks (#3412)
* fix(subflows): recurse into all descendants for lock, enable, and protection checks * fix(subflows): prevent container resize on initial render and clean up code - Add canvasReadyRef to skip container dimension recalculation during ReactFlow init — position changes from extent clamping fired before block heights are measured, causing containers to resize on page load - Resolve globals.css merge conflict, remove global z-index overrides (handled via ReactFlow zIndex prop instead) - Clean up subflow-node: hoist static helpers to module scope, remove unused ref, fix nested ternary readability, rename outlineColor→ringColor Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(subflows): use full ancestor-chain protection for descendant enable-toggle The enable-toggle for descendants was checking only direct `locked` status instead of walking the full ancestor chain via `isBlockProtected`. This meant a block nested 2+ levels inside a locked subflow could still be toggled. Also added TSDoc clarifying why boxShadow works for subflow ring indicators. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * revert(subflows): remove canvasReadyRef height-gating approach The canvasReadyRef gating in onNodesChange didn't fully fix the container resize-on-load issue. Reverting to address properly later. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove unintentional edge-interaction CSS from globals Leftover from merge conflict resolution — not part of this PR's changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(editor): correct isAncestorLocked when block and ancestor both locked, restore fade-in transition isAncestorLocked was derived from isBlockProtected which short-circuits on block.locked, so a self-locked block inside a locked ancestor showed "Unlock block" instead of "Ancestor container is locked". Now walks the ancestor chain independently. Also restores the accidentally removed transition-opacity duration-150 class on the ReactFlow container. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(subflows): use full ancestor-chain protection for top-level enable-toggle, restore edge-label z-index The top-level block check in batchToggleEnabled used block.locked (self only) while descendants used isBlockProtected (full ancestor chain). A block inside a locked ancestor but not itself locked would bypass the check. Now all three layers (store, collaborative hook, DB operations) consistently use isBlockProtected/isDbBlockProtected at both levels. Also restores the accidentally removed edge-labels z-index rule, bumped from 60 to 1001 so labels render above child nodes (zIndex: 1000). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(subflows): extract isAncestorProtected utility, add cycle detection to all traversals - Extract isAncestorProtected from utils.ts so editor.tsx doesn't duplicate the ancestor-chain walk. isBlockProtected now delegates to it. - Add visited-set cycle detection to all ancestor walks (isBlockProtected, isAncestorProtected, isDbBlockProtected) and descendant searches (findAllDescendantNodes, findDbDescendants) to guard against corrupt parentId references. - Document why click-catching div has no event bubbling concern (ReactFlow renders children as viewport siblings, not DOM children). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 127994f commit 6b355e9

File tree

8 files changed

+247
-290
lines changed

8 files changed

+247
-290
lines changed

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ import { LoopTool } from '@/app/workspace/[workspaceId]/w/[workflowId]/component
4040
import { ParallelTool } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/subflows/parallel/parallel-config'
4141
import { getSubBlockStableKey } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/utils'
4242
import { useCurrentWorkflow } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks'
43+
import {
44+
isAncestorProtected,
45+
isBlockProtected,
46+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/block-protection-utils'
4347
import { PreviewWorkflow } from '@/app/workspace/[workspaceId]/w/components/preview'
4448
import { getBlock } from '@/blocks/registry'
4549
import type { SubBlockType } from '@/blocks/types'
@@ -107,12 +111,11 @@ export function Editor() {
107111

108112
const userPermissions = useUserPermissionsContext()
109113

110-
// Check if block is locked (or inside a locked container) and compute edit permission
114+
// Check if block is locked (or inside a locked ancestor) and compute edit permission
111115
// Locked blocks cannot be edited by anyone (admins can only lock/unlock)
112116
const blocks = useWorkflowStore((state) => state.blocks)
113-
const parentId = currentBlock?.data?.parentId as string | undefined
114-
const isParentLocked = parentId ? (blocks[parentId]?.locked ?? false) : false
115-
const isLocked = (currentBlock?.locked ?? false) || isParentLocked
117+
const isLocked = currentBlockId ? isBlockProtected(currentBlockId, blocks) : false
118+
const isAncestorLocked = currentBlockId ? isAncestorProtected(currentBlockId, blocks) : false
116119
const canEditBlock = userPermissions.canEdit && !isLocked
117120

118121
const activeWorkflowId = useWorkflowRegistry((state) => state.activeWorkflowId)
@@ -247,10 +250,7 @@ export function Editor() {
247250
const block = blocks[blockId]
248251
if (!block) return
249252

250-
const parentId = block.data?.parentId as string | undefined
251-
const isParentLocked = parentId ? (blocks[parentId]?.locked ?? false) : false
252-
const isLocked = (block.locked ?? false) || isParentLocked
253-
if (!userPermissions.canEdit || isLocked) return
253+
if (!userPermissions.canEdit || isBlockProtected(blockId, blocks)) return
254254

255255
renamingBlockIdRef.current = blockId
256256
setEditedName(block.name || '')
@@ -364,11 +364,11 @@ export function Editor() {
364364
)}
365365
</div>
366366
<div className='flex shrink-0 items-center gap-[8px]'>
367-
{/* Locked indicator - clickable to unlock if user has admin permissions, block is locked, and parent is not locked */}
367+
{/* Locked indicator - clickable to unlock if user has admin permissions, block is locked directly, and not locked by an ancestor */}
368368
{isLocked && currentBlock && (
369369
<Tooltip.Root>
370370
<Tooltip.Trigger asChild>
371-
{userPermissions.canAdmin && currentBlock.locked && !isParentLocked ? (
371+
{userPermissions.canAdmin && currentBlock.locked && !isAncestorLocked ? (
372372
<Button
373373
variant='ghost'
374374
className='p-0'
@@ -385,8 +385,8 @@ export function Editor() {
385385
</Tooltip.Trigger>
386386
<Tooltip.Content side='top'>
387387
<p>
388-
{isParentLocked
389-
? 'Parent container is locked'
388+
{isAncestorLocked
389+
? 'Ancestor container is locked'
390390
: userPermissions.canAdmin && currentBlock.locked
391391
? 'Unlock block'
392392
: 'Block is locked'}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/subflows/subflow-node.tsx‎

Lines changed: 60 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { memo, useMemo, useRef } from 'react'
1+
import { memo, useMemo } from 'react'
22
import { RepeatIcon, SplitIcon } from 'lucide-react'
33
import { Handle, type NodeProps, Position, useReactFlow } from 'reactflow'
44
import { Badge } from '@/components/emcn'
@@ -28,6 +28,28 @@ export interface SubflowNodeData {
2828
executionStatus?: 'success' | 'error' | 'not-executed'
2929
}
3030

31+
const HANDLE_STYLE = {
32+
top: `${HANDLE_POSITIONS.DEFAULT_Y_OFFSET}px`,
33+
transform: 'translateY(-50%)',
34+
} as const
35+
36+
/**
37+
* Reusable class names for Handle components.
38+
* Matches the styling pattern from workflow-block.tsx.
39+
*/
40+
const getHandleClasses = (position: 'left' | 'right') => {
41+
const baseClasses = '!z-[10] !cursor-crosshair !border-none !transition-[colors] !duration-150'
42+
const colorClasses = '!bg-[var(--workflow-edge)]'
43+
44+
const positionClasses = {
45+
left: '!left-[-8px] !h-5 !w-[7px] !rounded-l-[2px] !rounded-r-none hover:!left-[-11px] hover:!w-[10px] hover:!rounded-l-full',
46+
right:
47+
'!right-[-8px] !h-5 !w-[7px] !rounded-r-[2px] !rounded-l-none hover:!right-[-11px] hover:!w-[10px] hover:!rounded-r-full',
48+
}
49+
50+
return cn(baseClasses, colorClasses, positionClasses[position])
51+
}
52+
3153
/**
3254
* Subflow node component for loop and parallel execution containers.
3355
* Renders a resizable container with a header displaying the block name and icon,
@@ -38,7 +60,6 @@ export interface SubflowNodeData {
3860
*/
3961
export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<SubflowNodeData>) => {
4062
const { getNodes } = useReactFlow()
41-
const blockRef = useRef<HTMLDivElement>(null)
4263
const userPermissions = useUserPermissionsContext()
4364

4465
const currentWorkflow = useCurrentWorkflow()
@@ -52,7 +73,6 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
5273
const isLocked = currentBlock?.locked ?? false
5374
const isPreview = data?.isPreview || false
5475

55-
// Focus state
5676
const setCurrentBlockId = usePanelEditorStore((state) => state.setCurrentBlockId)
5777
const currentBlockId = usePanelEditorStore((state) => state.currentBlockId)
5878
const isFocused = currentBlockId === id
@@ -84,35 +104,14 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
84104
}
85105

86106
return level
87-
}, [id, data?.parentId, getNodes])
107+
}, [data?.parentId, getNodes])
88108

89109
const startHandleId = data.kind === 'loop' ? 'loop-start-source' : 'parallel-start-source'
90110
const endHandleId = data.kind === 'loop' ? 'loop-end-source' : 'parallel-end-source'
91111
const BlockIcon = data.kind === 'loop' ? RepeatIcon : SplitIcon
92112
const blockIconBg = data.kind === 'loop' ? '#2FB3FF' : '#FEE12B'
93113
const blockName = data.name || (data.kind === 'loop' ? 'Loop' : 'Parallel')
94114

95-
/**
96-
* Reusable styles and positioning for Handle components.
97-
* Matches the styling pattern from workflow-block.tsx.
98-
*/
99-
const getHandleClasses = (position: 'left' | 'right') => {
100-
const baseClasses = '!z-[10] !cursor-crosshair !border-none !transition-[colors] !duration-150'
101-
const colorClasses = '!bg-[var(--workflow-edge)]'
102-
103-
const positionClasses = {
104-
left: '!left-[-8px] !h-5 !w-[7px] !rounded-l-[2px] !rounded-r-none hover:!left-[-11px] hover:!w-[10px] hover:!rounded-l-full',
105-
right:
106-
'!right-[-8px] !h-5 !w-[7px] !rounded-r-[2px] !rounded-l-none hover:!right-[-11px] hover:!w-[10px] hover:!rounded-r-full',
107-
}
108-
109-
return cn(baseClasses, colorClasses, positionClasses[position])
110-
}
111-
112-
const getHandleStyle = () => {
113-
return { top: `${HANDLE_POSITIONS.DEFAULT_Y_OFFSET}px`, transform: 'translateY(-50%)' }
114-
}
115-
116115
/**
117116
* Determine the ring styling based on subflow state priority:
118117
* 1. Focused (selected in editor), selected (shift-click/box), or preview selected - blue ring
@@ -127,46 +126,37 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
127126
diffStatus === 'new' ||
128127
diffStatus === 'edited' ||
129128
!!runPathStatus
129+
130130
/**
131-
* Compute the outline color for the subflow ring.
132-
* Uses CSS outline instead of box-shadow ring because in ReactFlow v11,
133-
* child nodes are DOM children of parent nodes and paint over the parent's
134-
* internal ring overlay. Outline renders on the element's own compositing
135-
* layer, so it stays visible above nested child nodes.
131+
* Compute the ring color for the subflow selection indicator.
132+
* Uses boxShadow (not CSS outline) to match the ring styling of regular workflow blocks.
133+
* This works because ReactFlow renders child nodes as sibling divs at the viewport level
134+
* (not as DOM children), so children at zIndex 1000 don't clip the parent's boxShadow.
136135
*/
137-
const outlineColor = hasRing
138-
? isFocused || isSelected || isPreviewSelected
139-
? 'var(--brand-secondary)'
140-
: diffStatus === 'new'
141-
? 'var(--brand-tertiary-2)'
142-
: diffStatus === 'edited'
143-
? 'var(--warning)'
144-
: runPathStatus === 'success'
145-
? executionStatus
146-
? 'var(--brand-tertiary-2)'
147-
: 'var(--border-success)'
148-
: runPathStatus === 'error'
149-
? 'var(--text-error)'
150-
: undefined
151-
: undefined
136+
const getRingColor = (): string | undefined => {
137+
if (!hasRing) return undefined
138+
if (isFocused || isSelected || isPreviewSelected) return 'var(--brand-secondary)'
139+
if (diffStatus === 'new') return 'var(--brand-tertiary-2)'
140+
if (diffStatus === 'edited') return 'var(--warning)'
141+
if (runPathStatus === 'success') {
142+
return executionStatus ? 'var(--brand-tertiary-2)' : 'var(--border-success)'
143+
}
144+
if (runPathStatus === 'error') return 'var(--text-error)'
145+
return undefined
146+
}
147+
const ringColor = getRingColor()
152148

153149
return (
154150
<div className='group pointer-events-none relative'>
155151
<div
156-
ref={blockRef}
157-
className={cn(
158-
'relative select-none rounded-[8px] border border-[var(--border-1)]',
159-
'transition-block-bg'
160-
)}
152+
className='relative select-none rounded-[8px] border border-[var(--border-1)] transition-block-bg'
161153
style={{
162154
width: data.width || 500,
163155
height: data.height || 300,
164-
position: 'relative',
165156
overflow: 'visible',
166157
pointerEvents: 'none',
167-
...(outlineColor && {
168-
outline: `1.75px solid ${outlineColor}`,
169-
outlineOffset: '-1px',
158+
...(ringColor && {
159+
boxShadow: `0 0 0 1.75px ${ringColor}`,
170160
}),
171161
}}
172162
data-node-id={id}
@@ -181,9 +171,7 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
181171
{/* Header Section — only interactive area for dragging */}
182172
<div
183173
onClick={() => setCurrentBlockId(id)}
184-
className={cn(
185-
'workflow-drag-handle flex cursor-grab items-center justify-between rounded-t-[8px] border-[var(--border)] border-b bg-[var(--surface-2)] py-[8px] pr-[12px] pl-[8px] [&:active]:cursor-grabbing'
186-
)}
174+
className='workflow-drag-handle flex cursor-grab items-center justify-between rounded-t-[8px] border-[var(--border)] border-b bg-[var(--surface-2)] py-[8px] pr-[12px] pl-[8px] [&:active]:cursor-grabbing'
187175
style={{ pointerEvents: 'auto' }}
188176
>
189177
<div className='flex min-w-0 flex-1 items-center gap-[10px]'>
@@ -209,6 +197,17 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
209197
</div>
210198
</div>
211199

200+
{/*
201+
* Click-catching background — selects this subflow when the body area is clicked.
202+
* No event bubbling concern: ReactFlow renders child nodes as viewport-level siblings,
203+
* not as DOM children of this component, so child clicks never reach this div.
204+
*/}
205+
<div
206+
className='absolute inset-0 top-[44px] rounded-b-[8px]'
207+
style={{ pointerEvents: isPreview ? 'none' : 'auto' }}
208+
onClick={() => setCurrentBlockId(id)}
209+
/>
210+
212211
{!isPreview && (
213212
<div
214213
className='absolute right-[8px] bottom-[8px] z-20 flex h-[32px] w-[32px] cursor-se-resize items-center justify-center text-muted-foreground'
@@ -217,12 +216,9 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
217216
)}
218217

219218
<div
220-
className='h-[calc(100%-50px)] pt-[16px] pr-[80px] pb-[16px] pl-[16px]'
219+
className='relative h-[calc(100%-50px)] pt-[16px] pr-[80px] pb-[16px] pl-[16px]'
221220
data-dragarea='true'
222-
style={{
223-
position: 'relative',
224-
pointerEvents: 'none',
225-
}}
221+
style={{ pointerEvents: 'none' }}
226222
>
227223
{/* Subflow Start */}
228224
<div
@@ -255,7 +251,7 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
255251
position={Position.Left}
256252
className={getHandleClasses('left')}
257253
style={{
258-
...getHandleStyle(),
254+
...HANDLE_STYLE,
259255
pointerEvents: 'auto',
260256
}}
261257
/>
@@ -266,7 +262,7 @@ export const SubflowNodeComponent = memo(({ data, id, selected }: NodeProps<Subf
266262
position={Position.Right}
267263
className={getHandleClasses('right')}
268264
style={{
269-
...getHandleStyle(),
265+
...HANDLE_STYLE,
270266
pointerEvents: 'auto',
271267
}}
272268
id={endHandleId}

‎apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/block-protection-utils.ts‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import type { BlockState } from '@/stores/workflows/workflow/types'
2+
import { isAncestorProtected, isBlockProtected } from '@/stores/workflows/workflow/utils'
3+
4+
export { isAncestorProtected, isBlockProtected }
25

36
/**
47
* Result of filtering protected blocks from a deletion operation
@@ -12,28 +15,6 @@ export interface FilterProtectedBlocksResult {
1215
allProtected: boolean
1316
}
1417

15-
/**
16-
* Checks if a block is protected from editing/deletion.
17-
* A block is protected if it is locked or if its parent container is locked.
18-
*
19-
* @param blockId - The ID of the block to check
20-
* @param blocks - Record of all blocks in the workflow
21-
* @returns True if the block is protected
22-
*/
23-
export function isBlockProtected(blockId: string, blocks: Record<string, BlockState>): boolean {
24-
const block = blocks[blockId]
25-
if (!block) return false
26-
27-
// Block is locked directly
28-
if (block.locked) return true
29-
30-
// Block is inside a locked container
31-
const parentId = block.data?.parentId
32-
if (parentId && blocks[parentId]?.locked) return true
33-
34-
return false
35-
}
36-
3718
/**
3819
* Checks if an edge is protected from modification.
3920
* An edge is protected only if its target block is protected.

0 commit comments

Comments
 (0)