Skip to content

Commit 57c98d8

Browse files
fix(race-condition-workflow-switching): another race condition between registry and workflow stores (#1247)
* fix(race-condition-workflow-switching): another race condition between regitry and workflow stores" * fix initial load race cond + cleanup * fix initial load issue + simplify
1 parent 0f7dfe0 commit 57c98d8

File tree

3 files changed

+29
-46
lines changed

3 files changed

+29
-46
lines changed

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

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -867,48 +867,41 @@ const WorkflowContent = React.memo(() => {
867867
[project, isPointInLoopNodeWrapper, getNodes]
868868
)
869869

870-
// Track when workflow is fully ready for rendering
870+
// Initialize workflow when it exists in registry and isn't active
871871
useEffect(() => {
872872
const currentId = params.workflowId as string
873+
if (!currentId || !workflows[currentId]) return
873874

874-
// Reset workflow ready state when workflow changes
875875
if (activeWorkflowId !== currentId) {
876-
setIsWorkflowReady(false)
877-
return
876+
// Clear diff and set as active
877+
const { clearDiff } = useWorkflowDiffStore.getState()
878+
clearDiff()
879+
setActiveWorkflow(currentId)
878880
}
881+
}, [params.workflowId, workflows, activeWorkflowId, setActiveWorkflow])
879882

880-
// Check if we have the necessary data to render the workflow
881-
const hasActiveWorkflow = activeWorkflowId === currentId
882-
const hasWorkflowInRegistry = Boolean(workflows[currentId])
883-
const isNotLoading = !isLoading
883+
// Track when workflow is ready for rendering
884+
useEffect(() => {
885+
const currentId = params.workflowId as string
884886

885887
// Workflow is ready when:
886888
// 1. We have an active workflow that matches the URL
887889
// 2. The workflow exists in the registry
888890
// 3. Workflows are not currently loading
889-
if (hasActiveWorkflow && hasWorkflowInRegistry && isNotLoading) {
890-
setIsWorkflowReady(true)
891-
} else {
892-
setIsWorkflowReady(false)
893-
}
891+
const shouldBeReady =
892+
activeWorkflowId === currentId && Boolean(workflows[currentId]) && !isLoading
893+
894+
setIsWorkflowReady(shouldBeReady)
894895
}, [activeWorkflowId, params.workflowId, workflows, isLoading])
895896

896-
// Init workflow
897+
// Handle navigation and validation
897898
useEffect(() => {
898899
const validateAndNavigate = async () => {
899900
const workflowIds = Object.keys(workflows)
900901
const currentId = params.workflowId as string
901902

902-
// Check if workflows have been initially loaded at least once
903-
// This prevents premature navigation decisions on page refresh
904-
if (!hasWorkflowsInitiallyLoaded()) {
905-
logger.info('Waiting for initial workflow load...')
906-
return
907-
}
908-
909-
// Wait for both initialization and workflow loading to complete
910-
if (isLoading) {
911-
logger.info('Workflows still loading, waiting...')
903+
// Wait for initial load to complete before making navigation decisions
904+
if (!hasWorkflowsInitiallyLoaded() || isLoading) {
912905
return
913906
}
914907

@@ -948,24 +941,10 @@ const WorkflowContent = React.memo(() => {
948941
router.replace(`/workspace/${currentWorkflow.workspaceId}/w/${currentId}`)
949942
return
950943
}
951-
952-
// Get current active workflow state
953-
const { activeWorkflowId } = useWorkflowRegistry.getState()
954-
955-
if (activeWorkflowId !== currentId) {
956-
// Clear workflow diff store when switching workflows
957-
const { clearDiff } = useWorkflowDiffStore.getState()
958-
clearDiff()
959-
960-
setActiveWorkflow(currentId)
961-
} else {
962-
// Don't reset variables cache if we're not actually switching workflows
963-
setActiveWorkflow(currentId)
964-
}
965944
}
966945

967946
validateAndNavigate()
968-
}, [params.workflowId, workflows, isLoading, setActiveWorkflow, createWorkflow, router])
947+
}, [params.workflowId, workflows, isLoading, workspaceId, router])
969948

970949
// Transform blocks and loops into ReactFlow nodes
971950
const nodes = useMemo(() => {

apps/sim/app/workspace/[workspaceId]/w/page.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
77

88
export default function WorkflowsPage() {
99
const router = useRouter()
10-
const { workflows, isLoading, loadWorkflows } = useWorkflowRegistry()
10+
const { workflows, isLoading, loadWorkflows, setActiveWorkflow } = useWorkflowRegistry()
1111
const [hasInitialized, setHasInitialized] = useState(false)
1212

1313
const params = useParams()
@@ -45,9 +45,14 @@ export default function WorkflowsPage() {
4545

4646
// If we have valid workspace workflows, redirect to the first one
4747
if (workspaceWorkflows.length > 0) {
48-
router.replace(`/workspace/${workspaceId}/w/${workspaceWorkflows[0]}`)
48+
// Ensure the workflow is set as active before redirecting
49+
// This prevents the empty canvas issue on first login
50+
const firstWorkflowId = workspaceWorkflows[0]
51+
setActiveWorkflow(firstWorkflowId).then(() => {
52+
router.replace(`/workspace/${workspaceId}/w/${firstWorkflowId}`)
53+
})
4954
}
50-
}, [hasInitialized, isLoading, workflows, workspaceId, router])
55+
}, [hasInitialized, isLoading, workflows, workspaceId, router, setActiveWorkflow])
5156

5257
// Always show loading state until redirect happens
5358
// There should always be a default workflow, so we never show "no workflows found"

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -484,8 +484,6 @@ export const useWorkflowRegistry = create<WorkflowRegistry>()(
484484
future: [],
485485
},
486486
}
487-
488-
// Subblock values will be initialized by initializeFromWorkflow below
489487
} else {
490488
// If no state in DB, use empty state - server should have created start block
491489
workflowState = {
@@ -535,11 +533,12 @@ export const useWorkflowRegistry = create<WorkflowRegistry>()(
535533
}))
536534
}
537535

536+
// Update all stores atomically to prevent race conditions
537+
// Set activeWorkflowId and workflow state together
538+
set({ activeWorkflowId: id, error: null })
538539
useWorkflowStore.setState(workflowState)
539540
useSubBlockStore.getState().initializeFromWorkflow(id, (workflowState as any).blocks || {})
540541

541-
set({ activeWorkflowId: id, error: null })
542-
543542
window.dispatchEvent(
544543
new CustomEvent('active-workflow-changed', {
545544
detail: { workflowId: id },

0 commit comments

Comments
 (0)