fix(subflows): recurse into all descendants for lock, enable, and protection checks by waleedlatif1 · Pull Request #3412 · simstudioai/sim

waleedlatif1 · 2026-03-04T19:42:44Z

Summary

Lock/unlock and enable/disable now propagate to all nested descendants, not just direct children
Protection checks walk the full ancestor chain instead of only checking the immediate parent
Extracted shared isBlockProtected and findAllDescendantNodes utilities to eliminate 7+ duplicate inline closures
Fixed the same direct-children-only bugs in the socket persistence layer (caused lock state to be lost on refresh)
Fixed z-index issues for nested subflow interaction

Type of Change

Bug fix

Testing

Tested manually with 3-level nesting (Parallel > Parallel > Parallel > Function). All existing tests pass (158/158).

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…tection checks

vercel · 2026-03-04T19:42:49Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Mar 4, 2026 8:15pm

waleedlatif1 · 2026-03-04T19:47:20Z

@greptile

waleedlatif1 · 2026-03-04T19:47:26Z

@cursor review

cursor · 2026-03-04T19:47:29Z

Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings.

greptile-apps · 2026-03-04T19:53:59Z

Greptile Summary

This PR fixes a class of bugs where lock/enable propagation and protection checks only inspected the immediate parent of a block rather than the full ancestor chain, causing operations to silently bypass locks in deeply nested subflows. It consolidates all duplicated isProtected closures across the store, collaborative hook, and socket persistence layer into shared isBlockProtected / findAllDescendantNodes utilities (and their DB-facing counterparts), and pairs this with a z-index rework to make nested subflow click/selection interaction reliable.

Key changes:

isBlockProtected moved to stores/workflows/workflow/utils.ts and correctly walks the complete ancestor chain; block-protection-utils.ts re-exports it for backward compatibility
findAllDescendantNodes converted from recursion to an iterative stack loop (avoids call-stack growth on deep nesting)
batchToggleEnabled and batchToggleLocked in store.ts, collaborative hook, and operations.ts now properly propagate to all descendants with full ancestor-chain protection checks
subflow-node.tsx switches the selection ring from outline to boxShadow and adds a click-catching backdrop div so the subflow body is selectable; child blocks are elevated to zIndex: 1000 in workflow.tsx to stay above the backdrop
elevateNodesOnSelect is set to false to prevent ReactFlow's automatic elevation from raising container nodes above their children — however, this removes elevation for top-level overlapping blocks, which is a regression
transition-opacity duration-150 was accidentally dropped from the ReactFlow className, so the canvas no longer fades in smoothly when ready

Core logic fixes are sound and well-tested. However, two unintentional side effects in workflow.tsx should be addressed: (1) the missing fade-in transition, and (2) the loss of automatic elevation for overlapping top-level blocks.

Confidence Score: 4/5

Core logic is sound with all tests passing, but two visual regressions in workflow.tsx should be fixed before merging.
The protection-check and descendant-propagation bug fixes are correct and well-implemented with no logical errors. The new utilities are properly structured with iterative algorithms to avoid stack overflow. However, workflow.tsx contains two unintentional regressions: the removed transition-opacity duration-150 class breaks the canvas fade-in animation, and elevateNodesOnSelect={false} prevents top-level overlapping blocks from elevating when selected, significantly degrading interaction UX. Both are straightforward to fix but represent real user-facing issues.
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx — requires attention to fix the two ReactFlow styling regressions (transition-opacity and elevateNodesOnSelect).

_{Last reviewed commit: 8a8a9d5}

apps/sim/stores/workflows/workflow/store.ts

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

…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>

cursor · 2026-03-04T20:00:51Z

PR Summary

Medium Risk
Moderate risk because it changes core workflow edit-protection rules (lock/enable propagation, rename protection, edge/parent updates) across both client state and socket DB persistence, which could affect what users can modify or delete.

Overview
Fixes subflow lock/protection semantics by making isBlockProtected walk the full ancestor chain (not just the direct parent) and updating enable/lock/handle toggles to operate on all nested descendants of loop/parallel containers.

Updates both the client stores/collab workflow and the socket DB operation handlers to use the same recursive protection/descendant logic, preventing locked/disabled state inconsistencies after persistence/refresh.

Improves nested subflow UX: container bodies are now click-to-select, selection rings use boxShadow to match regular blocks, and canvas z-index/ReactFlow settings are adjusted so nested blocks remain interactable above container hit areas.

^{Written by Cursor Bugbot for commit 8a8a9d5. 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>

waleedlatif1 · 2026-03-04T20:16:27Z

@greptile

waleedlatif1 · 2026-03-04T20:16:31Z

@cursor review

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

cursor · 2026-03-04T20:20:56Z

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

-  const isParentLocked = parentId ? (blocks[parentId]?.locked ?? false) : false
-  const isLocked = (currentBlock?.locked ?? false) || isParentLocked
+  const isLocked = currentBlockId ? isBlockProtected(currentBlockId, blocks) : false
+  const isAncestorLocked = isLocked && !(currentBlock?.locked ?? false)


Incorrect isAncestorLocked when block and ancestor both locked

Medium Severity

isAncestorLocked is derived as isLocked && !(currentBlock?.locked ?? false), but isBlockProtected short-circuits on block.locked before ever checking ancestors. When a block is both self-locked and has a locked ancestor, isAncestorLocked evaluates to false instead of true. This causes the editor to incorrectly show an "Unlock" button (with tooltip "Unlock block") instead of a lock icon with the "Ancestor container is locked" message. Unlocking the block won't actually make it editable since the ancestor remains locked, which is confusing.

Additional Locations (1)

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/editor.tsx#L367-L389

greptile-apps · 2026-03-04T20:23:12Z

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

              edgesFocusable={true}
              edgesUpdatable={effectivePermissions.canEdit}
-              className={`workflow-container h-full transition-opacity duration-150 ${reactFlowStyles} ${isCanvasReady ? 'opacity-100' : 'opacity-0'} ${isHandMode ? 'canvas-mode-hand' : 'canvas-mode-cursor'}`}
+              className={`workflow-container h-full bg-[var(--bg)] ${reactFlowStyles} ${isCanvasReady ? 'opacity-100' : 'opacity-0'} ${isHandMode ? 'canvas-mode-hand' : 'canvas-mode-cursor'}`}


The transition-opacity duration-150 class was removed in this change. The opacity is still toggled based on isCanvasReady, but without the transition class, the canvas will snap from invisible to fully visible instantly instead of fading in smoothly.

Suggested change

className={`workflow-container h-full bg-[var(--bg)] ${reactFlowStyles} ${isCanvasReady ? 'opacity-100' : 'opacity-0'} ${isHandMode ? 'canvas-mode-hand' : 'canvas-mode-cursor'}`}

className={`workflow-container h-full bg-[var(--bg)] transition-opacity duration-150 ${reactFlowStyles} ${isCanvasReady ? 'opacity-100' : 'opacity-0'} ${isHandMode ? 'canvas-mode-hand' : 'canvas-mode-cursor'}`}

greptile-apps · 2026-03-04T20:23:14Z

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

              onlyRenderVisibleElements={false}
              deleteKeyCode={null}
-              elevateNodesOnSelect={true}
+              elevateNodesOnSelect={false}


Setting elevateNodesOnSelect={false} was necessary to prevent containers from painting over their children (fixed by manual zIndex: 1000 on child nodes). However, this manual elevation only applies to blocks with a parentId, so top-level blocks at the canvas root will never be elevated when selected.

Before this change, selecting overlapping top-level workflow blocks would bring the selected block visually to the front. After this change, overlapping top-level blocks retain their natural DOM stacking order regardless of selection, making it impossible to interact with a block positioned beneath another.

One approach to preserve elevation for top-level selected nodes without re-introducing the container-covers-children problem would be to explicitly handle elevation via the style property on root nodes (e.g., bump their z-index when they match the selected node ID).

fix(subflows): recurse into all descendants for lock, enable, and pro…

96f40b7

…tection checks

greptile-apps bot reviewed Mar 4, 2026

View reviewed changes

apps/sim/stores/workflows/workflow/store.ts Show resolved Hide resolved

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

vercel bot temporarily deployed to Preview March 4, 2026 19:56 Inactive

vercel bot temporarily deployed to Preview March 4, 2026 20:00 Inactive

waleedlatif1 and others added 2 commits March 4, 2026 12:14

revert(subflows): remove canvasReadyRef height-gating approach

e938978

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

8a8a9d5

Leftover from merge conflict resolution — not part of this PR's changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

vercel bot temporarily deployed to Preview March 4, 2026 20:15 Inactive

cursor bot reviewed Mar 4, 2026

View reviewed changes

greptile-apps bot reviewed Mar 4, 2026

View reviewed changes

	className={`workflow-container h-full bg-[var(--bg)] ${reactFlowStyles} ${isCanvasReady ? 'opacity-100' : 'opacity-0'} ${isHandMode ? 'canvas-mode-hand' : 'canvas-mode-cursor'}`}
	className={`workflow-container h-full bg-[var(--bg)] transition-opacity duration-150 ${reactFlowStyles} ${isCanvasReady ? 'opacity-100' : 'opacity-0'} ${isHandMode ? 'canvas-mode-hand' : 'canvas-mode-cursor'}`}

Conversation

waleedlatif1 commented Mar 4, 2026

Summary

Type of Change

Testing

Checklist

Uh oh!

vercel bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

waleedlatif1 commented Mar 4, 2026

Uh oh!

waleedlatif1 commented Mar 4, 2026

Uh oh!

cursor bot commented Mar 4, 2026

Uh oh!

greptile-apps bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Uh oh!

Uh oh!

Uh oh!

cursor bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

waleedlatif1 commented Mar 4, 2026

Uh oh!

waleedlatif1 commented Mar 4, 2026

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Mar 4, 2026

Choose a reason for hiding this comment

Incorrect isAncestorLocked when block and ancestor both locked

Uh oh!

greptile-apps bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel bot commented Mar 4, 2026 •

edited

Loading

greptile-apps bot commented Mar 4, 2026 •

edited

Loading

cursor bot commented Mar 4, 2026 •

edited

Loading

Incorrect `isAncestorLocked` when block and ancestor both locked