-
Notifications
You must be signed in to change notification settings - Fork 3.2k
improvement(canvas): add multi-block select, add batch handle, enabled, and edge operations #2738
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…d, and edge operations
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Adds multi-block selection with Shift-click, batch operations for handles/enabled/edges, and improved undo-redo for multi-node operations. The feature enables users to select multiple blocks and perform operations on them simultaneously, significantly improving workflow editing efficiency.
Major Changes:
- Multi-select support in ReactFlow canvas with Shift-click for nodes and edges
- Batch socket operations:
batch-toggle-enabled,batch-toggle-handles,batch-remove-edges,batch-add-edges - Multi-node drag with batch position updates and undo/redo tracking
- Refactored undo-redo to use batch operations:
batch-move-blocks,batch-remove-edges,batch-toggle-enabled,batch-toggle-handles - Helper utilities extracted to
workflow-canvas-helpers.tsfor reusability - Selection state preservation during
derivedNodesupdates
Issues Found:
- Type confusion in
use-undo-redo.tsline 180: inverse operation comment says "batch-add-edges" but type isBatchRemoveEdgesOperation - Performance concern:
batch-toggle-enabledandbatch-toggle-handlesquery each block individually in database operations instead of using bulk queries - Two unrelated files changed (privacy page, email footer) that appear outside the scope of this PR
Confidence Score: 3/5
- Safe to merge with caution - core functionality works but has type inconsistency and performance concerns
- The multi-select and batch operations are well-implemented with proper undo-redo support. However, there's a type confusion issue in the undo-redo inverse operation (line 180 of use-undo-redo.ts) where the comment says "batch-add-edges" but the type is BatchRemoveEdgesOperation. While this may work due to runtime logic, it's semantically incorrect and confusing. Additionally, the database batch toggle operations query blocks individually in loops instead of using bulk queries, which could cause performance issues with large selections. The feature has been manually tested according to the PR description, but no automated tests were added.
- Pay close attention to
apps/sim/hooks/use-undo-redo.ts(type confusion in inverse operation) andapps/sim/socket/database/operations.ts(performance concern in batch toggles)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/hooks/use-undo-redo.ts | 2/5 | Refactored to support batch operations for edges, moves, toggle-enabled, and toggle-handles. Type confusion in recordBatchRemoveEdges inverse operation (line 180). |
| apps/sim/stores/undo-redo/types.ts | 4/5 | Updated operation types to support batch edge removal, batch moves, batch toggle operations. Well-structured type definitions. |
| apps/sim/stores/undo-redo/store.ts | 4/5 | Updated coalescing logic for batch-move-blocks operations. Properly merges consecutive moves for same block set. |
| apps/sim/socket/handlers/operations.ts | 5/5 | Added handlers for new batch operations (batch-remove-edges, batch-toggle-enabled, batch-toggle-handles, batch-add-edges). Clean implementation. |
| apps/sim/socket/database/operations.ts | 3/5 | Added database persistence for batch edge and toggle operations. Performance concern: batch toggles query each block individually in loop. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | 4/5 | Major changes: multi-select support with Shift-click, batch operations for edges/blocks, multi-node drag with undo/redo tracking, selection state preservation. Well-implemented overall. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-canvas-helpers.ts | 5/5 | New helper utilities for trigger validation, drag highlights, node selection, and position clamping. Clean, well-documented functions. |
| apps/sim/hooks/use-collaborative-workflow.ts | 4/5 | Added batch operations for toggle-enabled, toggle-handles, and batch-remove-edges. Integrated with undo-redo system. Clean implementation with proper recording. |
Sequence Diagram
sequenceDiagram
participant User
participant Canvas
participant CollaborativeWorkflow
participant UndoRedo
participant Socket
participant Database
User->>Canvas: Shift-click multiple blocks
Canvas->>Canvas: Update selection state
User->>Canvas: Press Delete key
Canvas->>CollaborativeWorkflow: collaborativeBatchRemoveBlocks(blockIds)
CollaborativeWorkflow->>UndoRedo: recordBatchRemoveBlocks(snapshots)
UndoRedo->>UndoRedo: Store operation + inverse
CollaborativeWorkflow->>Socket: Emit batch-remove-blocks
Socket->>Database: Delete blocks, edges, subflows
Database-->>Socket: Confirm deletion
Socket-->>CollaborativeWorkflow: Operation confirmed
User->>Canvas: Drag multiple selected blocks
Canvas->>Canvas: Capture start positions
Canvas->>Canvas: Update visual positions
User->>Canvas: Release drag
Canvas->>CollaborativeWorkflow: collaborativeBatchUpdatePositions(updates, previousPositions)
CollaborativeWorkflow->>UndoRedo: recordBatchMoveBlocks(moves)
CollaborativeWorkflow->>Socket: Emit batch-update-positions
Socket->>Database: Update block positions
User->>Canvas: Shift-click multiple edges
Canvas->>Canvas: Add edges to selectedEdges Map
User->>Canvas: Press Delete
Canvas->>CollaborativeWorkflow: collaborativeBatchRemoveEdges(edgeIds)
CollaborativeWorkflow->>UndoRedo: recordBatchRemoveEdges(snapshots)
CollaborativeWorkflow->>Socket: Emit batch-remove-edges
Socket->>Database: Delete edges by IDs
User->>Canvas: Right-click multiple blocks → Toggle Enabled
Canvas->>CollaborativeWorkflow: collaborativeBatchToggleBlockEnabled(blockIds)
CollaborativeWorkflow->>UndoRedo: recordBatchToggleEnabled(blockIds, previousStates)
CollaborativeWorkflow->>Socket: Emit batch-toggle-enabled
Socket->>Database: Toggle enabled for each block
User->>Canvas: Ctrl+Z (Undo)
Canvas->>UndoRedo: undo()
UndoRedo->>UndoRedo: Pop operation, apply inverse
UndoRedo->>CollaborativeWorkflow: Apply inverse operation
CollaborativeWorkflow->>Socket: Emit inverse operation
Socket->>Database: Revert changes
Co-authored-by: icecrasher321 <icecrasher321@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR adds multi-block selection and batch operations to the workflow canvas, enabling users to shift-click multiple blocks and perform operations like toggle enabled/disabled, toggle handles orientation, and batch position updates. The implementation includes comprehensive undo/redo support for batch operations and properly extends the socket-based collaborative editing system.
Key Changes
Frontend Canvas (workflow.tsx)
- Multi-select support with Shift+Click for both blocks and edges
- Batch position updates for multi-node drag operations
- Selection state preservation during canvas updates
- Enhanced helper utilities for common operations
Collaborative Workflow System
- New
collaborativeBatchToggleBlockEnabledandcollaborativeBatchToggleBlockHandlesfunctions - Refactored single-block operations to use batch APIs for consistency
- Multi-node drag tracking with
multiNodeDragStartRef
Socket Layer
- Added handlers for
batch-toggle-enabled,batch-toggle-handles,batch-update-positions - Permission checks properly extended for all new batch operations
- Validation schemas added for all new operation types
Undo/Redo System
- New operation types:
BatchToggleEnabledOperationandBatchToggleHandlesOperation - Proper inverse operation creation for batch operations
- Stack coalescing for consecutive batch moves
Critical Issues Found
Data Loss Bug: The batch-update-positions socket handler broadcasts to clients but never persists to the database, causing position data loss on page refresh or for newly joining users.
Race Conditions: Multiple race conditions exist in batch toggle operations:
previousStatescaptured before applying toggles (client-side race)- Database toggle operations fetch-then-toggle pattern (server-side race)
These can cause undo operations to restore incorrect states and concurrent toggles to cancel each other out.
Undo Logic Errors: The undo/redo implementations for batch toggle operations conditionally toggle based on current state rather than unconditionally restoring to the recorded previous state, which breaks correctness when blocks change state between recording and undo.
Architecture Concerns
The batch operations follow the existing patterns but inherit some of the system's collaborative editing challenges. The optimistic update model (apply locally, then sync) combined with the toggle operation semantics creates inherent race conditions that would benefit from a more deterministic conflict resolution strategy.
Confidence Score: 2/5
- This PR has critical bugs that will cause data loss and incorrect behavior in production
- The score reflects multiple serious issues: (1) batch-update-positions doesn't persist to database causing guaranteed data loss, (2) race conditions in toggle operations will cause incorrect behavior in collaborative sessions, (3) undo/redo logic errors will restore wrong states. While the feature adds valuable functionality, these bugs need to be fixed before merge.
- Critical attention needed for socket/handlers/operations.ts (data loss bug), hooks/use-undo-redo.ts (logic errors), hooks/use-collaborative-workflow.ts (race conditions), and socket/database/operations.ts (concurrent operation handling)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/socket/handlers/operations.ts | 2/5 | Added batch operations support but batch-update-positions doesn't persist to database, causing data loss on refresh |
| apps/sim/hooks/use-undo-redo.ts | 2/5 | New batch toggle undo/redo operations have logic errors - conditional toggling instead of state restoration |
| apps/sim/hooks/use-collaborative-workflow.ts | 2/5 | Added batch operations with race condition in previousStates capture for toggle operations |
| apps/sim/socket/database/operations.ts | 2/5 | Batch toggle operations have race conditions when concurrent operations target same blocks |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | 3/5 | Added multi-select and batch operations with minor issues in drag state cleanup and position validation |
Sequence Diagram
sequenceDiagram
participant User
participant Canvas
participant WorkflowStore
participant CollaborativeWorkflow
participant OperationQueue
participant Socket
participant Server
participant Database
participant OtherClients
Note over User,OtherClients: Multi-Block Selection & Batch Operations Flow
User->>Canvas: Shift+Click blocks
Canvas->>Canvas: Update selectedNodes state
User->>Canvas: Trigger batch operation<br/>(e.g., Toggle Enabled)
Canvas->>CollaborativeWorkflow: collaborativeBatchToggleBlockEnabled(blockIds)
CollaborativeWorkflow->>WorkflowStore: Read current enabled states
Note over CollaborativeWorkflow: Capture previousStates<br/>(Race condition risk!)
CollaborativeWorkflow->>OperationQueue: addToQueue(operation)
CollaborativeWorkflow->>WorkflowStore: toggleBlockEnabled() for each
CollaborativeWorkflow->>UndoRedoStore: recordBatchToggleEnabled()
OperationQueue->>Socket: emit('workflow-operation')
Socket->>Server: batch-toggle-enabled
Server->>Server: checkRolePermission()
Server->>Database: BEGIN TRANSACTION
Server->>Database: SELECT current enabled states
Note over Server,Database: Another race condition:<br/>concurrent toggles can cancel out
Server->>Database: UPDATE with toggled values
Server->>Database: COMMIT
Server->>OtherClients: broadcast operation
OtherClients->>WorkflowStore: Apply toggle locally
Server->>Socket: operation-confirmed
Socket->>OperationQueue: confirmOperation()
Note over User,OtherClients: Multi-Node Drag Flow
User->>Canvas: Start dragging selected blocks
Canvas->>Canvas: onNodeDragStart()<br/>Capture multiNodeDragStartRef
User->>Canvas: Drag blocks to new positions
Canvas->>Canvas: onNodeDrag() updates display
User->>Canvas: Release drag
Canvas->>Canvas: onNodeDragStop()
Canvas->>Canvas: computeClampedPositionUpdates()
Canvas->>CollaborativeWorkflow: collaborativeBatchUpdatePositions()
CollaborativeWorkflow->>OperationQueue: addToQueue(batch-update-positions)
CollaborativeWorkflow->>WorkflowStore: Update positions locally
OperationQueue->>Socket: emit('workflow-operation')
Socket->>Server: batch-update-positions
Server->>OtherClients: broadcast (ONLY!)
Note over Server,Database: BUG: Never persists to DB!
OtherClients->>WorkflowStore: Apply positions
Note over User,Database: On page refresh, positions lost!
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
Additional Comments (2)
Looking at the socket handler: if (target === 'blocks' && operation === 'batch-update-positions') {
socket.to(workflowId).emit('workflow-operation', {
// broadcasts but doesn't call persistWorkflowOperation
})
// Missing: await persistWorkflowOperation(workflowId, {...})
}The database operation handler in
This means batch position updates are lost on page refresh or when new users join the workflow, as the database won't have the latest positions. The fix should call |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR adds comprehensive multi-block selection and batch operations to the workflow canvas, enabling users to Shift-click multiple blocks/edges and perform operations on them collectively.
Key Features Implemented
1. Multi-Block Selection
- Shift-click to select multiple blocks
- Visual selection state preserved across node updates
- SelectionMode.Partial for easier multi-selection
- Multi-selection support for drag operations
2. Batch Operations
- Batch Remove Blocks: Delete multiple blocks at once
- Batch Remove Edges: Delete multiple edges with Shift-click selection
- Batch Toggle Enabled: Toggle enabled state for multiple blocks
- Batch Toggle Handles: Toggle handle orientation for multiple blocks
- Batch Move Blocks: Drag multiple blocks with proper undo/redo tracking
3. Undo/Redo Support
- All batch operations integrated with undo/redo system
- New operation types:
batch-move-blocks,batch-add-edges,batch-remove-edges,batch-toggle-enabled,batch-toggle-handles - Multi-node drag operations properly tracked via
multiNodeDragStartRef
4. Socket & Database Integration
- Validation schemas for all batch operations
- Permission checks added for batch operations (admin/write roles)
- Database persistence handlers for batch operations
- Transactional integrity maintained
5. UI/UX Improvements
- Refactored duplicate code into utility functions (
clearDragHighlights,validateTriggerPaste, etc.) - Helper functions for position clamping in multi-block scenarios
- Deferred selection after paste/duplicate operations
- Edge selection tracked with Map for multi-select
Architecture
The implementation follows a consistent pattern:
- UI captures user intent (multi-select, drag, delete)
useCollaborativeWorkfloworchestrates batch operationsuseUndoRedorecords operations for undo/redo- Operations queued and sent via Socket.IO
- Server validates, persists to database, broadcasts to collaborators
All batch operations properly handle:
- Permission checking
- Schema validation
- Transactional database updates
- Real-time collaboration via broadcasts
- Undo/redo state management
Confidence Score: 5/5
- Safe to merge - comprehensive implementation with proper error handling, validation, and collaboration support
- The implementation is thorough and well-architected. All batch operations follow existing patterns with proper validation, permissions, database transactions, and undo/redo integration. The multi-selection UI is properly integrated with ReactFlow. No blocking issues found - code quality is high with good separation of concerns.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/stores/undo-redo/types.ts | 5/5 | Added batch operation types for edges, blocks (move/toggle-enabled/toggle-handles). Types are well-defined with proper structure. |
| apps/sim/stores/undo-redo/utils.ts | 4/5 | Added inverse operation creation for batch operations. Contains unused operationToCollaborativePayload with incorrect payload format for batch-move-blocks, but function is not imported anywhere. |
| apps/sim/hooks/use-undo-redo.ts | 5/5 | Implemented batch toggle enabled/handles recording and undo/redo logic. Operation and inverse store same previousStates (semantically odd but functionally correct due to special handling in undo/redo). Batch move blocks and batch edges operations properly implemented. |
| apps/sim/socket/validation/schemas.ts | 5/5 | Added validation schemas for batch edge operations (add/remove) and batch block toggle operations (enabled/handles). All schemas properly structured with Zod validation. |
| apps/sim/socket/handlers/operations.ts | 5/5 | Added handlers for batch-remove-edges, batch-toggle-enabled, batch-toggle-handles, and batch-add-edges socket operations. All handlers follow existing persist-then-broadcast pattern with proper error handling. |
| apps/sim/socket/database/operations.ts | 5/5 | Added database persistence for batch-toggle-enabled, batch-toggle-handles, batch-add-edges, and batch-remove-edges operations. All operations properly transactional and follow existing patterns. |
| apps/sim/socket/middleware/permissions.ts | 5/5 | Added batch operation permissions (batch-add-edges, batch-remove-edges, batch-toggle-enabled, batch-toggle-handles) to admin and write roles. Properly integrated into existing permission checking system. |
| apps/sim/hooks/use-collaborative-workflow.ts | 5/5 | Converted individual toggle operations to batch operations (collaborativeBatchToggleBlockEnabled, collaborativeBatchToggleBlockHandles). Added collaborativeBatchRemoveEdges. Updated batch-update-positions to support undo/redo with previousPositions. All operations properly queue and record for undo/redo. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | 5/5 | Implemented multi-block selection with Shift-click, multi-edge selection, batch drag operations. Added multiNodeDragStartRef to track positions for undo. Converted single toggles to batch operations. Added helper functions (clearDragHighlights, validateTriggerPaste, etc). Selection state preserved across node updates. All features properly integrated. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-canvas-helpers.ts | 5/5 | New utility file with helper functions: isInEditableElement, validateTriggerPaste, clearDragHighlights, selectNodesDeferred, getClampedPositionForNode, computeClampedPositionUpdates. All functions properly implemented with clear documentation. |
| apps/sim/stores/workflows/workflow/store.ts | 5/5 | Added setBlockEnabled and setBlockHandles methods for directly setting block states (used by undo/redo). Both methods properly update state and trigger lastSaved update. |
Sequence Diagram
sequenceDiagram
participant User
participant Workflow UI
participant useCollaborativeWorkflow
participant OperationQueue
participant Socket
participant Server
participant Database
participant useUndoRedo
Note over User,useUndoRedo: Multi-Block Selection & Batch Operations Flow
User->>Workflow UI: Shift+Click multiple blocks
Workflow UI->>Workflow UI: Update selected state in displayNodes
User->>Workflow UI: Press Delete/Backspace
Workflow UI->>useCollaborativeWorkflow: collaborativeBatchRemoveBlocks(blockIds)
useCollaborativeWorkflow->>useUndoRedo: recordBatchRemoveBlocks(snapshots)
useUndoRedo->>useUndoRedo: Create operation+inverse, push to stack
useCollaborativeWorkflow->>OperationQueue: addToQueue(batch-remove-blocks)
useCollaborativeWorkflow->>Workflow UI: Update local state (remove blocks)
OperationQueue->>Socket: emit workflow-operation
Socket->>Server: batch-remove-blocks payload
Server->>Server: Validate schema & permissions
Server->>Database: Persist batch-remove-blocks
Database-->>Server: Success
Server->>Socket: Broadcast to other clients
Server->>Socket: operation-confirmed
Note over User,useUndoRedo: Multi-Edge Selection & Batch Delete
User->>Workflow UI: Shift+Click multiple edges
Workflow UI->>Workflow UI: Update selectedEdges Map
User->>Workflow UI: Press Delete
Workflow UI->>useCollaborativeWorkflow: collaborativeBatchRemoveEdges(edgeIds)
useCollaborativeWorkflow->>useUndoRedo: recordBatchRemoveEdges(edgeSnapshots)
useCollaborativeWorkflow->>OperationQueue: addToQueue(batch-remove-edges)
useCollaborativeWorkflow->>Workflow UI: Update local state
OperationQueue->>Socket: emit workflow-operation
Socket->>Server: batch-remove-edges payload
Server->>Database: Delete edges in transaction
Server->>Socket: Broadcast & confirm
Note over User,useUndoRedo: Batch Toggle Enabled
User->>Workflow UI: Right-click multi-selection
User->>Workflow UI: Select "Toggle Enabled"
Workflow UI->>useCollaborativeWorkflow: collaborativeBatchToggleBlockEnabled(blockIds)
useCollaborativeWorkflow->>useCollaborativeWorkflow: Capture previousStates
useCollaborativeWorkflow->>useUndoRedo: recordBatchToggleEnabled(ids, previousStates)
useCollaborativeWorkflow->>OperationQueue: addToQueue(batch-toggle-enabled)
useCollaborativeWorkflow->>Workflow UI: Toggle blocks locally
OperationQueue->>Socket: emit workflow-operation
Socket->>Server: batch-toggle-enabled payload
Server->>Database: Toggle enabled states in transaction
Server->>Socket: Broadcast & confirm
Note over User,useUndoRedo: Multi-Block Drag & Batch Position Update
User->>Workflow UI: Drag selected blocks
Workflow UI->>Workflow UI: onNodeDragStart: capture multiNodeDragStartRef
User->>Workflow UI: Drop blocks
Workflow UI->>Workflow UI: computeClampedPositionUpdates()
Workflow UI->>useCollaborativeWorkflow: collaborativeBatchUpdatePositions(updates, previousPositions)
useCollaborativeWorkflow->>useUndoRedo: recordBatchMoveBlocks(moves)
useCollaborativeWorkflow->>OperationQueue: addToQueue(batch-update-positions)
useCollaborativeWorkflow->>Workflow UI: Update positions locally
OperationQueue->>Socket: emit workflow-operation
Socket->>Server: batch-update-positions payload
Server->>Database: Update positions in transaction
Server->>Socket: Broadcast & confirm
Note over User,useUndoRedo: Undo/Redo Operations
User->>Workflow UI: Cmd+Z (Undo)
Workflow UI->>useUndoRedo: undo()
useUndoRedo->>useUndoRedo: Pop from undo stack, get inverse operation
useUndoRedo->>useCollaborativeWorkflow: Execute inverse via addToQueue
useUndoRedo->>Workflow UI: Apply inverse to local state
useCollaborativeWorkflow->>Socket: Broadcast inverse operation
User->>Workflow UI: Cmd+Shift+Z (Redo)
Workflow UI->>useUndoRedo: redo()
useUndoRedo->>useUndoRedo: Pop from redo stack, get operation
useUndoRedo->>useCollaborativeWorkflow: Execute operation via addToQueue
useUndoRedo->>Workflow UI: Apply operation to local state
useCollaborativeWorkflow->>Socket: Broadcast operation
cb1312c to
b085942
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR adds multi-block selection and batch operations for workflows, enabling users to shift-click multiple blocks and perform operations like toggling enabled state, changing handle orientation, and updating parent containers in bulk.
What Changed
The PR introduces 3 new batch operations:
batch-toggle-enabled: Toggle enabled state for multiple blocksbatch-toggle-handles: Toggle handle orientation (horizontal/vertical) for multiple blocksbatch-update-parent: Update parent container for multiple blocks
These operations are integrated throughout the stack:
- ✅ Socket handlers properly emit, validate, and persist operations
- ✅ Database operations handle batch updates in transactions
- ✅ Undo/redo fully supports all batch operations
- ✅ Permissions correctly check admin/write access
- ✅ UI implements multi-select with shift-click and context menus
Critical Issue Found
State Desynchronization in Collaborative Editing: The new batch operations (batch-toggle-enabled, batch-toggle-handles, batch-update-parent) are properly sent via socket and persisted to the database, but remote clients never receive and apply these changes because use-collaborative-workflow.ts is missing handlers for these operations.
When User A performs a batch operation:
- ✅ User A sees the changes locally
- ✅ Changes are persisted to database
- ✅ Socket broadcasts to other clients
- ❌ User B's client receives the broadcast but has no handler
- ❌ User B never sees the changes until refresh
This breaks the core collaborative editing feature for these operations.
Architecture Quality
The implementation follows good patterns:
- Proper validation with Zod schemas
- Transaction-based database operations
- Permission middleware integration
- Comprehensive undo/redo support
- Consistent error handling
The missing handlers appear to be an oversight rather than a design flaw, as the infrastructure is properly set up.
Confidence Score: 2/5
- This PR has a critical bug that breaks collaborative editing for the new batch operations, causing state desynchronization between users
- Score of 2 reflects one critical logic error that completely breaks the new feature in collaborative scenarios. The missing remote handlers mean that when User A toggles enabled/handles or updates parent for multiple blocks, User B will not see those changes until page refresh. This defeats the purpose of real-time collaborative editing. The rest of the implementation (socket handlers, database operations, undo/redo, permissions) is solid and well-architected.
- Critical: apps/sim/hooks/use-collaborative-workflow.ts - Must add handlers for batch-toggle-enabled, batch-toggle-handles, and batch-update-parent operations before merging
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/hooks/use-collaborative-workflow.ts | 1/5 | Critical bug: Missing handlers for batch-toggle-enabled, batch-toggle-handles, and batch-update-parent operations cause state desynchronization in collaborative editing |
| apps/sim/socket/handlers/operations.ts | 4/5 | Added socket handlers for 3 new batch operations (toggle-enabled, toggle-handles, update-parent) with proper persistence and broadcasting |
| apps/sim/socket/database/operations.ts | 4/5 | Added database operations for 3 new batch operations with proper transaction handling and subflow node list updates |
| apps/sim/socket/validation/schemas.ts | 5/5 | Added Zod validation schemas for 3 new batch operations (toggle-enabled, toggle-handles, update-parent) with proper type safety |
| apps/sim/hooks/use-undo-redo.ts | 5/5 | Added undo/redo support for 3 new batch operations with proper inverse operations and state restoration |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | 4/5 | Implemented multi-block selection with shift-click and context menu operations for batch toggle enabled/handles and parent updates |
Sequence Diagram
sequenceDiagram
participant User as User (Client A)
participant WorkflowUI as workflow.tsx
participant CollabHook as use-collaborative-workflow.ts
participant OpQueue as Operation Queue
participant Socket as Socket Handler
participant DB as Database Operations
participant RemoteClient as Remote Client B
Note over User,RemoteClient: Multi-Block Selection & Batch Operations Flow
User->>WorkflowUI: Shift+Click multiple blocks
WorkflowUI->>WorkflowUI: Update selection state
User->>WorkflowUI: Right-click → Toggle Enabled
WorkflowUI->>CollabHook: collaborativeBatchToggleBlockEnabled(blockIds)
CollabHook->>CollabHook: Capture previous states
CollabHook->>OpQueue: addToQueue(batch-toggle-enabled)
CollabHook->>CollabHook: Apply locally (toggleBlockEnabled)
CollabHook->>CollabHook: recordBatchToggleEnabled (undo/redo)
OpQueue->>Socket: emit('workflow-operation', batch-toggle-enabled)
Socket->>Socket: Validate operation schema
Socket->>Socket: Check permissions
Socket->>DB: persistWorkflowOperation()
DB->>DB: Start transaction
DB->>DB: Fetch current enabled states
DB->>DB: Toggle each block
DB->>DB: Commit transaction
DB-->>Socket: Success
Socket->>RemoteClient: broadcast('workflow-operation')
Socket-->>OpQueue: emit('operation-confirmed')
OpQueue->>OpQueue: Mark operation complete
Note over RemoteClient: ❌ CRITICAL BUG
RemoteClient->>RemoteClient: handleWorkflowOperation()
RemoteClient->>RemoteClient: No handler for batch-toggle-enabled!
RemoteClient->>RemoteClient: State NOT updated
Note over User,RemoteClient: Same flow for batch-toggle-handles and batch-update-parent
Additional Comments (1)
The
Impact: When a remote user performs these operations, other clients will NOT see the changes, causing state desynchronization between collaborators. Fix: Add handlers for these operations in the Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/hooks/use-collaborative-workflow.ts
Line: 251:264
Comment:
Missing handlers for new batch operations causes state desynchronization in collaborative editing.
The `handleWorkflowOperation` function handles incoming socket operations from remote users. Currently, only `batch-update-positions` is handled for `target === 'blocks'` (lines 253-263), but three NEW batch operations introduced in this PR are missing handlers:
1. **`batch-toggle-enabled`** - Sent at line 901, persisted at line 350 in `operations.ts`, but NOT handled here
2. **`batch-toggle-handles`** - Sent at line 1063, persisted at line 378 in `operations.ts`, but NOT handled here
3. **`batch-update-parent`** - Sent at line 974, persisted at line 407 in `operations.ts`, but NOT handled here
**Impact**: When a remote user performs these operations, other clients will NOT see the changes, causing state desynchronization between collaborators.
**Fix**: Add handlers for these operations in the `target === 'blocks'` switch block:
```suggestion
} else if (target === 'blocks') {
switch (operation) {
case 'batch-update-positions': {
const { updates } = payload
if (Array.isArray(updates)) {
updates.forEach(({ id, position }: { id: string; position: Position }) => {
if (id && position) {
workflowStore.updateBlockPosition(id, position)
}
})
}
break
}
case 'batch-toggle-enabled': {
const { blockIds } = payload
if (Array.isArray(blockIds)) {
blockIds.forEach((id: string) => {
workflowStore.toggleBlockEnabled(id)
})
}
break
}
case 'batch-toggle-handles': {
const { blockIds } = payload
if (Array.isArray(blockIds)) {
blockIds.forEach((id: string) => {
workflowStore.toggleBlockHandles(id)
})
}
break
}
case 'batch-update-parent': {
const { updates } = payload
if (Array.isArray(updates)) {
updates.forEach((update: any) => {
const { id, parentId, position } = update
if (id) {
if (position) {
workflowStore.updateBlockPosition(id, position)
}
if (parentId) {
workflowStore.updateParentId(id, parentId, 'parent')
} else {
// Remove from parent
workflowStore.updateParentId(id, '', 'parent')
}
}
})
}
break
}
}
} else if (target === 'edge') {
```
How can I resolve this? If you propose a fix, please make it concise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR adds multi-block selection with shift-click and batch operations (toggle enabled/handles, move to/from subflows, edge operations). The implementation includes:
Core Changes:
- Multi-select UI: Users can shift-click multiple blocks and perform batch operations via context menu
- Batch Socket Operations: 5 new operations (batch-toggle-enabled, batch-toggle-handles, batch-update-parent, batch-add-edges, batch-remove-edges)
- Undo/Redo Refactor: Converted from single-operation to batch-operation model for better multi-select support
- Socket Layer: Validation schemas, socket handlers, and database persistence all properly implemented
- Helper Utilities: New workflow-canvas-helpers.ts extracts reusable functions for multi-select operations
Architecture:
The change follows a consistent pattern across layers:
- UI triggers batch operation via
useCollaborativeWorkflowhooks - Local state updates immediately for responsive UX
- Operations queued and sent via Socket.IO
- Server persists to database and broadcasts to other clients
- Remote clients receive and apply operations via
handleWorkflowOperation
Critical Issue:
The socket broadcast layer (step 4) is complete, but the receive layer (step 5) is missing handlers for 4 of the 5 new batch operations in handleWorkflowOperation(). This causes state desynchronization between collaborators when using the new multi-select features.
Type Safety Improvements:
- Variables changed from
any[]toRecord<string, Variable>with proper typing - BlockState outputs now use
OutputFieldDefinitioninstead ofBlockOutput - Replaced
anywithunknownin LoopConfig and ParallelConfig types
Confidence Score: 1/5
- NOT safe to merge - critical bugs will break collaborative editing for multi-select operations
- Two P0 bugs cause state desynchronization in collaborative editing: (1) Missing handlers for batch-toggle-enabled, batch-toggle-handles, and batch-update-parent in handleWorkflowOperation means remote clients won't see these changes, (2) Missing handler for batch-add-edges means edge additions won't sync. These operations are properly sent and persisted but not applied on receive. Additionally, there's a P2 style issue with direct store state mutation in the circular parent reference recovery code.
- apps/sim/hooks/use-collaborative-workflow.ts - Missing 4 critical operation handlers in handleWorkflowOperation (lines 251-318)
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| apps/sim/hooks/use-collaborative-workflow.ts | 1/5 | Added batch operations for multi-select (toggle enabled/handles, update parent, remove edges) but missing critical handlers in handleWorkflowOperation for 4 new operations, causing state desync in collaborative editing |
| apps/sim/socket/handlers/operations.ts | 5/5 | Added socket handlers for 5 new batch operations (toggle enabled/handles, update parent, add/remove edges) - all properly persist to database and broadcast to clients |
| apps/sim/socket/database/operations.ts | 5/5 | Added database persistence for batch operations - properly handles batch-toggle-enabled, batch-toggle-handles, batch-update-parent, batch-add-edges, batch-remove-edges with transaction support |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx | 3/5 | Major refactor for multi-block selection with shift-click support, batch drag operations, and context menu actions - includes defensive recovery for circular parent refs that mutates store state |
| apps/sim/hooks/use-undo-redo.ts | 5/5 | Refactored from single-operation to batch operations (batch-move-blocks, batch-add/remove-edges, batch-toggle-enabled/handles, batch-update-parent) with proper operation coalescing |
| apps/sim/stores/undo-redo/store.ts | 5/5 | Updated operation applicability checks and coalescing logic to support batch operations instead of single operations |
| apps/sim/stores/workflows/workflow/store.ts | 5/5 | Added setBlockEnabled and setBlockHandles methods for batch operations, improved updateParentId with circular reference protection |
| apps/sim/socket/validation/schemas.ts | 5/5 | Added Zod validation schemas for 5 new batch operations with proper payload structure validation |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/utils/workflow-canvas-helpers.ts | 5/5 | New utility file with helper functions for multi-select operations: trigger validation, drag highlight clearing, deferred node selection, position clamping |
| apps/sim/stores/workflows/workflow/types.ts | 5/5 | Added Variable interface, changed variables from array to Record, added setBlockEnabled/setBlockHandles to WorkflowActions, improved type safety with unknown instead of any |
Sequence Diagram
sequenceDiagram
participant User as User (Client A)
participant UI as Workflow Canvas
participant Collab as useCollaborativeWorkflow
participant Socket as Socket Handler
participant DB as Database
participant ClientB as Remote Client B
Note over User,ClientB: Multi-Block Selection & Batch Operations
User->>UI: Shift+Click multiple blocks
UI->>UI: Update selection state
User->>UI: Context Menu → Toggle Enabled
UI->>Collab: collaborativeBatchToggleBlockEnabled(blockIds)
Collab->>Collab: Apply locally (toggleBlockEnabled)
Collab->>Collab: Record undo/redo entry
Collab->>Socket: emit('workflow-operation', batch-toggle-enabled)
Socket->>DB: persistWorkflowOperation()
DB->>DB: Update workflowBlocks.enabled for each block
Socket->>ClientB: broadcast('workflow-operation')
Note over ClientB: ⚠️ BUG: Missing handler!
ClientB->>ClientB: handleWorkflowOperation()
ClientB->>ClientB: ❌ No case for batch-toggle-enabled
ClientB->>ClientB: State NOT updated
Note over User,ClientB: Similar issue for other batch operations
User->>UI: Drag multiple blocks into subflow
UI->>Collab: collaborativeBatchUpdateParent(updates)
Collab->>Socket: emit('batch-update-parent')
Socket->>DB: Update parentId & positions
Socket->>ClientB: broadcast
ClientB->>ClientB: ❌ No handler → desync
User->>UI: Delete multiple edges
UI->>Collab: collaborativeBatchRemoveEdges(edgeIds)
Collab->>Socket: emit('batch-remove-edges')
Socket->>DB: Delete edges
Socket->>ClientB: broadcast
ClientB->>ClientB: ✅ Handler exists (line 293)
ClientB->>ClientB: State updated correctly
Note over User,ClientB: Undo/Redo Flow
User->>UI: Cmd+Z (Undo)
UI->>Collab: undo()
Collab->>Collab: Get inverse operation from stack
Collab->>Collab: Apply inverse (batch operations)
Collab->>Socket: emit inverse operation
Socket->>DB: Persist & broadcast
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
Additional Comments (1)
Prompt To Fix With AIThis is a comment left during a code review.
Path: apps/sim/hooks/use-collaborative-workflow.ts
Line: 251:264
Comment:
[P0] Missing handlers for `batch-toggle-enabled`, `batch-toggle-handles`, and `batch-update-parent` operations. These operations are sent via socket (lines 901, 1063, 974) and persisted to the database (socket/database/operations.ts lines 696, 725, 752), but remote clients won't apply them when received, causing state desynchronization in collaborative editing.
```suggestion
} else if (target === 'blocks') {
switch (operation) {
case 'batch-update-positions': {
const { updates } = payload
if (Array.isArray(updates)) {
updates.forEach(({ id, position }: { id: string; position: Position }) => {
if (id && position) {
workflowStore.updateBlockPosition(id, position)
}
})
}
break
}
case 'batch-toggle-enabled': {
const { blockIds } = payload
if (Array.isArray(blockIds)) {
blockIds.forEach((id: string) => workflowStore.toggleBlockEnabled(id))
}
break
}
case 'batch-toggle-handles': {
const { blockIds } = payload
if (Array.isArray(blockIds)) {
blockIds.forEach((id: string) => workflowStore.toggleBlockHandles(id))
}
break
}
case 'batch-update-parent': {
const { updates } = payload
if (Array.isArray(updates)) {
updates.forEach((update: any) => {
if (update.id && update.position) {
workflowStore.updateBlockPosition(update.id, update.position)
}
if (update.parentId !== undefined) {
workflowStore.updateParentId(update.id, update.parentId || null, 'parent')
}
})
}
break
}
}
```
How can I resolve this? If you propose a fix, please make it concise. |
|
Too many files changed for review. ( |
Summary
Type of Change
Testing
Tested manually
Checklist