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#3412waleedlatif1 merged 8 commits intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@greptile |
|
@cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
Greptile SummaryThis PR fixes a class of bugs where lock/enable-disable operations only propagated to direct children instead of all nested descendants, and where protection checks only inspected the immediate parent rather than the full ancestor chain. Core improvements:
Files with significant logic changes:
The utility extraction and ancestor-chain fixes are well-implemented with proper cycle detection. Edge-label z-index was appropriately adjusted to accommodate the child node elevation. Confidence Score: 4/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Lock / Enable / Delete operation\n(ids[])"] --> B{For each id}
B --> C["isBlockProtected(id, blocks)"]
C -->|"walks full ancestor chain\n(visited Set for cycle safety)"| D{Protected?}
D -->|Yes| E["Skip block entirely"]
D -->|No| F["Add block to operation set"]
F --> G{Is loop or parallel?}
G -->|Yes| H["findAllDescendantNodes(id, blocks)\n(iterative BFS, visited Set)"]
H --> I{For each descId}
I --> J["isBlockProtected(descId, blocks)"]
J -->|Protected| K["Skip descendant"]
J -->|Not protected| L["Add descendant to operation set"]
G -->|No| M["Apply operation to set"]
L --> M
M --> N["Persist via store / socket / DB"]
subgraph "Utility Sources"
U1["stores/workflows/workflow/utils.ts\n isBlockProtected\n isAncestorProtected\n findAllDescendantNodes"]
U2["socket/database/operations.ts\n isDbBlockProtected\n findDbDescendants\n (mirror for raw DB records)"]
end
Last reviewed commit: bf6104f |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/subflows/subflow-node.tsx
Show resolved
Hide resolved
…p 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>
…le-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>
PR SummaryMedium Risk Overview Persistence layer matches the new rules. Socket DB operations add DB-friendly protection/descendant helpers so server-side batch delete/toggle/move/edge ops filter protected targets correctly and include all descendants, preventing state loss on refresh for deeply nested subflows. ReactFlow nesting interaction is stabilized. Subflow nodes get a click-catching body for selection, child blocks inside containers are forced above it with Written by Cursor Bugbot for commit bf6104f. Configure here. |
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>
Leftover from merge conflict resolution — not part of this PR's changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@greptile |
|
@cursor review |
...sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
…cked, 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>
|
@greptile |
|
@cursor review |
…e-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>
|
@greptile |
|
@cursor review |
...sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx
Outdated
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/subflows/subflow-node.tsx
Outdated
Show resolved
Hide resolved
Additional Comments (1)
Three selectors were removed from
The |
...sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx
Outdated
Show resolved
Hide resolved
…on 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>
|
@greptile |
|
@cursor review |
Summary
isBlockProtectedandfindAllDescendantNodesutilities to eliminate 7+ duplicate inline closuresType of Change
Testing
Tested manually with 3-level nesting (Parallel > Parallel > Parallel > Function). All existing tests pass (158/158).
Checklist