-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix(execution): scope execution state per workflow to prevent cross-workflow bleed #3183
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
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@greptile |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts
Show resolved
Hide resolved
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts
Outdated
Show resolved
Hide resolved
Greptile OverviewGreptile SummaryThis PR scopes all execution-related Zustand state (isExecuting, active block IDs, run path/edges, debug context, etc.) by workflow ID using a To avoid broad re-renders, it adds granular selector hooks ( Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as Workflow UI (per workflowId)
participant ExecHook as useWorkflowExecution
participant ExecStore as useExecutionStore
participant Stream as useExecutionStream
participant API as /api/workflows/:id/execute
UI->>ExecHook: handleRunWorkflow(workflowId)
ExecHook->>ExecStore: setIsExecuting(workflowId,true)
ExecHook->>Stream: execute({workflowId, callbacks})
Stream->>API: POST execute (signal from AbortController[workflowId])
API-->>Stream: SSE events + X-Execution-Id
Stream-->>ExecHook: callbacks.onBlockStarted/Completed/Error
ExecHook->>ExecStore: setActiveBlocks(workflowId, Set)
ExecHook->>ExecStore: setBlockRunStatus(workflowId, blockId, status)
ExecHook->>ExecStore: setEdgeRunStatus(workflowId, edgeId, status)
Stream-->>ExecHook: callbacks.onExecutionCompleted
ExecHook->>ExecStore: setIsExecuting(workflowId,false)
UI->>ExecHook: handleCancelExecution(workflowId)
ExecHook->>Stream: cancel(workflowId)
Stream->>API: POST cancel (executionId for workflowId)
Stream->>Stream: abort AbortController[workflowId]
|
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.
16 files reviewed, 2 comments
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-workflow-execution.ts
Outdated
Show resolved
Hide resolved
…sertion in handleRunUntilBlock
|
@cursor review |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
… re-renders from unselectored store hook
|
@cursor review |
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx
Outdated
Show resolved
Hide resolved
…oid re-renders from lastRunPath/lastRunEdges changes
|
@cursor review |
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.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Summary
Map<string, WorkflowExecutionState>useIsBlockActive,useLastRunPath,useLastRunEdges) to prevent performance regression from full-object selectorsuseExecutionStreamto support concurrent executions via per-workflow AbortController mapsworkflowIdthrough all 30+ setter call sites inuse-workflow-execution.tsType of Change
Testing
Checklist