Revert "fix(sockets): joining currently deleted workflow"#4036
Revert "fix(sockets): joining currently deleted workflow"#4036icecrasher321 merged 1 commit intostagingfrom
Conversation
This reverts commit 609ba61.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview This removes Reviewed by Cursor Bugbot for commit 95e02c2. Configure here. |
Greptile SummaryThis PR reverts #4004 ("fix(sockets): joining currently deleted workflow"), removing the guard that prevented the socket client from attempting to rejoin a workflow that was just deleted. No explanation is provided for why the fix is being reverted. Key changes:
The revert re-introduces the race condition that #4004 was specifically addressing: after a Confidence Score: 3/5Not safe to merge without explanation — reintroduces a known race condition where the client attempts to join a deleted workflow room The revert deliberately removes a targeted guard (deletedWorkflowIdRef) that prevented rejoining a deleted workflow during the URL-propagation window. This is a present defect on the changed path: any time a workflow is deleted, there is a real timing window during which both the useEffect and the SESSION_ERROR handler will emit join-workflow for a room that no longer exists. The PR description provides no context for why the fix is being reverted, making it impossible to evaluate whether an alternative approach is planned. Until the motivation is clear and a replacement guard is in place, this scores P1. apps/sim/app/workspace/providers/socket-provider.tsx — the deleted-workflow guard was the only protection against the rejoin race condition
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/providers/socket-provider.tsx | Reverts the deleted-workflow guard, reintroducing a race condition where the client attempts to rejoin a workflow that was just deleted while the URL still holds its old ID |
Sequence Diagram
sequenceDiagram
participant Server
participant Socket as SocketProvider
participant Router as Next.js Router
Server->>Socket: workflow-deleted { workflowId }
Socket->>Socket: setCurrentWorkflowId(null)
Note over Router: router.push() in progress — URL still = deleted workflowId
alt SESSION_ERROR fires during propagation window
Server->>Socket: operation-forbidden { type: SESSION_ERROR }
Socket->>Socket: workflowId = urlWorkflowIdRef.current (= deleted ID)
Socket->>Server: join-workflow { workflowId: deletedId } ❌
end
alt useEffect fires during propagation window
Note over Socket: currentWorkflowId=null, urlWorkflowId=deletedId → not equal
Socket->>Server: join-workflow { workflowId: deletedId } ❌
end
Router-->>Socket: URL updated to new workflowId
Note over Socket: Race window closes
Comments Outside Diff (1)
-
apps/sim/app/workspace/providers/socket-provider.tsx, line 503-510 (link)Reintroduces deleted-workflow rejoin race condition
Removing
deletedWorkflowIdRef.current !== workflowIdfrom theSESSION_ERRORguard means that if aSESSION_ERRORfires whileurlWorkflowIdstill points to a deleted workflow (a timing window that exists untilrouter.push()fully propagates the new URL), the socket will emitjoin-workflowfor a workflow that no longer exists on the server.The same race condition is also reintroduced in the
useEffectat line 554: whenworkflow-deletedsetscurrentWorkflowIdtonullbuturlWorkflowIdstill holds the deleted ID, the conditioncurrentWorkflowId === urlWorkflowIdevaluates tofalse, causingjoin-workflowto be emitted for a deleted room.PR fix(sockets): joining currently deleted workflow #4004 was specifically introduced to close this gap. Is there a regression from that fix that motivates this revert? If so, it would help to document the reason so the root cause can be addressed without sacrificing the deleted-workflow guard.
Reviews (1): Last reviewed commit: "Revert "fix(sockets): joining currently ..." | Re-trigger Greptile
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 95e02c2. Configure here.
| // Prevent rejoining a workflow that was just deleted. The URL param may | ||
| // still reference the old workflow while router.push() propagates. | ||
| if (deletedWorkflowIdRef.current === urlWorkflowId) { | ||
| return |
There was a problem hiding this comment.
Reverted guard allows rejoining a deleted workflow
Medium Severity
Removing deletedWorkflowIdRef reintroduces a race condition. When a workflow-deleted event fires, currentWorkflowId is set to null, but urlWorkflowId still references the deleted workflow (URL hasn't changed). The useEffect sees currentWorkflowId !== urlWorkflowId and immediately emits join-workflow for the deleted workflow. This also affects the operation-forbidden handler, which will attempt to rejoin a deleted workflow on SESSION_ERROR. Since onWorkflowDeleted has no consumers to trigger navigation, this race will reliably trigger for any user viewing a workflow deleted by someone else.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 95e02c2. Configure here.


Reverts #4004