Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Jan 12, 2026

Summary

  • added the ability to export workflow
  • added animation for importing icon
  • standardized all hooks to use stable reference instead of getters, updated callers
  • upgraded turborepo

Type of Change

  • New feature

Testing

Tested manually

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)

@vercel
Copy link

vercel bot commented Jan 12, 2026

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

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 12, 2026 5:59pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 12, 2026

Greptile Overview

Greptile Summary

This PR adds workflow export functionality and refactors all workspace/workflow/folder operation hooks from a getter pattern to a stable reference pattern. The refactoring improves code consistency and reduces complexity, but introduces one critical bug that breaks workflow duplication and export operations.

Key Changes

New Export Functionality:

  • useExportFolder: New hook that recursively collects and exports all workflows within a folder (and nested subfolders) as a ZIP file
  • Animated Download Icon: Added Download component with CSS-based pulse animation for visual feedback during import operations
  • Folder Export UI: Integrated export functionality into folder context menus with proper workflow validation

Hook Architecture Refactoring:
All operation hooks were refactored from:

// OLD: Getter pattern
getWorkflowIds: () => string[]
// NEW: Stable reference pattern  
workflowIds: string[]

This change affects: useDeleteWorkflow, useDeleteFolder, useDuplicateWorkflow, useDuplicateFolder, useExportWorkflow, and useDuplicateWorkspace.

Import Hook Relocation:

  • Moved useImportWorkflow instantiation from WorkflowList component to parent Sidebar component
  • Improved import loading state with Loader component instead of static icon

Workflow Registry Enhancement:

  • Improved hydration preservation logic to better handle workflow deletion scenarios across multiple tabs

Critical Issues Found

🚨 CRITICAL BUG: Workflow Export/Duplicate Broken in workflow-item.tsx

Lines 81-94 contain a critical refactoring error:

  • The hooks are destructured with renamed properties (handleDuplicateWorkflowduplicateWorkflow)
  • New wrapper functions with the original names are created
  • These wrappers call the renamed functions but the hooks were initialized WITHOUT the required workflowIds prop
  • Result: When users try to duplicate or export workflows, operations silently fail

Impact: Complete breakage of workflow duplicate and export features from the workflow item context menu.

Filename Collision Bugs

In use-export-workflow.ts (lines 145-157) and use-export-folder.ts (lines 198-208):

  • Collision detection checks lowercase filenames but stores original case in the Set
  • Workflows with names differing only in case (e.g., "Test" vs "TEST") can create duplicate filenames in ZIP archives
  • Can cause issues on case-insensitive filesystems

Impact: Moderate - only affects edge case where multiple workflows have case-variant names

What Works Well

✅ All hook refactorings in panel.tsx, sidebar.tsx, and folder-item.tsx are correct
✅ The new useExportFolder hook properly handles recursive folder traversal
✅ Import functionality correctly moved to parent component
✅ Loading states improved with animated icons
✅ Folder operations (delete/duplicate) correctly refactored
✅ Workspace operations correctly refactored

Testing Recommendations

  1. CRITICAL: Test workflow duplicate and export from workflow item context menu
  2. Test folder export with nested folders and multiple workflows
  3. Test export of workflows with case-variant names
  4. Verify import loading animation displays correctly
  5. Test all hook refactorings in various selection scenarios (single/multi-select)

Confidence Score: 2/5

  • This PR contains a critical bug that will break core workflow export and duplicate functionality
  • Score reflects one critical production-breaking bug in workflow-item.tsx where the hook refactoring was incomplete, causing duplicate and export operations to fail silently. Additionally, there are two instances of a filename collision bug in ZIP generation. While most of the refactoring is sound and the new folder export feature is well-implemented, the critical bug in a high-traffic component (workflow context menu) prevents safe merging
  • CRITICAL ATTENTION: apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx - Must fix lines 81-94 before merge. Also review use-export-workflow.ts and use-export-folder.ts for filename collision fixes

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-item/workflow-item.tsx 1/5 CRITICAL BUG: Duplicate and export functions are broken due to incorrect hook refactoring - hooks receive undefined workflowIds
apps/sim/app/workspace/[workspaceId]/w/hooks/use-export-workflow.ts 3/5 Refactored from getter pattern to direct parameter. Has filename collision bug in ZIP generation
apps/sim/app/workspace/[workspaceId]/w/hooks/use-export-folder.ts 3/5 NEW FILE: Adds folder export functionality with recursive workflow collection. Has same filename collision bug as use-export-workflow
apps/sim/app/workspace/[workspaceId]/w/hooks/use-duplicate-workflow.ts 4/5 Successfully refactored from getter pattern to accepting workflowIds as function parameter. No issues found
apps/sim/app/workspace/[workspaceId]/w/hooks/use-delete-workflow.ts 4/5 Successfully refactored from getter pattern to direct parameter. Navigation logic preserved correctly
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/folder-item/folder-item.tsx 5/5 Correctly integrated new useExportFolder hook. Disables duplicate/export when folder has no workflows
apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx 5/5 Moved useImportWorkflow hook from WorkflowList to parent Sidebar. Added animated Download icon for import loading state
apps/sim/components/emcn/icons/download.tsx 5/5 NEW FILE: Download icon component with optional CSS animation for import loading states
apps/sim/stores/workflows/registry/store.ts 4/5 Improved hydration preservation logic to handle workflow deletion edge cases more gracefully

Sequence Diagram

sequenceDiagram
    participant User
    participant WorkflowItem
    participant ContextMenu
    participant useExportWorkflow
    participant API
    participant Browser

    User->>WorkflowItem: Right-click workflow
    WorkflowItem->>WorkflowItem: captureSelectionState()
    Note over WorkflowItem: Stores workflowIds in<br/>capturedSelectionRef
    WorkflowItem->>ContextMenu: Open menu
    User->>ContextMenu: Click "Export"
    ContextMenu->>WorkflowItem: handleExportWorkflow()
    WorkflowItem->>WorkflowItem: Get workflowIds from ref
    
    alt CRITICAL BUG: Current Implementation
        WorkflowItem->>useExportWorkflow: exportWorkflow(workflowIds)
        Note over useExportWorkflow: Hook was initialized WITHOUT<br/>workflowIds prop!<br/>Operation fails silently
    end
    
    alt Correct Implementation
        WorkflowItem->>useExportWorkflow: handleExportWorkflow(workflowIds)
        Note over useExportWorkflow: Hook receives workflowIds<br/>as function parameter
        
        loop For each workflow
            useExportWorkflow->>API: GET /api/workflows/{id}
            API-->>useExportWorkflow: Workflow state
            useExportWorkflow->>API: GET /api/workflows/{id}/variables
            API-->>useExportWorkflow: Variables
            useExportWorkflow->>useExportWorkflow: sanitizeForExport()
        end
        
        alt Single workflow
            useExportWorkflow->>Browser: Download JSON file
        else Multiple workflows
            useExportWorkflow->>useExportWorkflow: Create ZIP with JSZip
            Note over useExportWorkflow: Filename collision bug:<br/>Uses lowercase check but<br/>stores original case
            useExportWorkflow->>Browser: Download ZIP file
        end
        
        useExportWorkflow->>useExportWorkflow: clearSelection()
        useExportWorkflow-->>WorkflowItem: Success
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 6 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 12, 2026

Additional Comments (1)

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/folder-item/folder-item.tsx
The duplicate functionality is disabled when a folder has no workflows (!hasWorkflows), but the export functionality also has this same condition (!hasWorkflows). This means:

  1. Both operations require workflows to exist (correct for export)
  2. However, disabling duplicate for empty folders may not be intended behavior

Looking at useDuplicateFolder.ts, the mutation will still work for empty folders (it duplicates the folder structure). The hasWorkflows check is coming from useExportFolder which makes sense for export, but applying it to duplicate seems incorrect.

Consider whether duplicate should really be disabled for empty folders, or if this is conflating two different concerns.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/folder-item/folder-item.tsx
Line: 383:383

Comment:
The duplicate functionality is disabled when a folder has no workflows (`!hasWorkflows`), but the export functionality also has this same condition (`!hasWorkflows`). This means:

1. Both operations require workflows to exist (correct for export)
2. However, disabling duplicate for empty folders may not be intended behavior

Looking at `useDuplicateFolder.ts`, the mutation will still work for empty folders (it duplicates the folder structure). The `hasWorkflows` check is coming from `useExportFolder` which makes sense for export, but applying it to duplicate seems incorrect. 

Consider whether duplicate should really be disabled for empty folders, or if this is conflating two different concerns.

How can I resolve this? If you propose a fix, please make it concise.

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +84 to +94
const handleDuplicateWorkflow = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
duplicateWorkflow(workflowIds)
}, [duplicateWorkflow])

const handleExportWorkflow = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
exportWorkflow(workflowIds)
}, [exportWorkflow])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactored hooks are being renamed but not passed the required arguments, breaking duplicate and export functionality.

The Bug:

  • Line 81: useDuplicateWorkflow returns handleDuplicateWorkflow but it's renamed to duplicateWorkflow
  • Line 83: useExportWorkflow returns handleExportWorkflow but it's renamed to exportWorkflow
  • Lines 84-94: New wrapper functions handleDuplicateWorkflow and handleExportWorkflow are created
  • CRITICAL: These wrappers call duplicateWorkflow(workflowIds) and exportWorkflow(workflowIds), BUT the hooks now require workflowIds as parameters (not getter functions), so the hooks receive NO workflow IDs

Impact:
When users try to duplicate or export workflows, the operations will silently fail because workflowIds prop passed to the hooks is undefined.

The Fix:
Remove the renaming and pass workflowIds directly when calling the hook functions:

Suggested change
const handleDuplicateWorkflow = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
duplicateWorkflow(workflowIds)
}, [duplicateWorkflow])
const handleExportWorkflow = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
exportWorkflow(workflowIds)
}, [exportWorkflow])
const { handleDuplicateWorkflow } = useDuplicateWorkflow({ workspaceId })
const { handleExportWorkflow } = useExportWorkflow({ workspaceId })
const handleDuplicateWorkflowClick = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
handleDuplicateWorkflow(workflowIds)
}, [handleDuplicateWorkflow])
const handleExportWorkflowClick = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
handleExportWorkflow(workflowIds)
}, [handleExportWorkflow])

Then update the ContextMenu calls (lines 359-360) to use the new names.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/workflow-item/workflow-item.tsx
Line: 84:94

Comment:
The refactored hooks are being renamed but not passed the required arguments, breaking duplicate and export functionality.

**The Bug:**
- Line 81: `useDuplicateWorkflow` returns `handleDuplicateWorkflow` but it's renamed to `duplicateWorkflow`
- Line 83: `useExportWorkflow` returns `handleExportWorkflow` but it's renamed to `exportWorkflow`
- Lines 84-94: New wrapper functions `handleDuplicateWorkflow` and `handleExportWorkflow` are created
- **CRITICAL**: These wrappers call `duplicateWorkflow(workflowIds)` and `exportWorkflow(workflowIds)`, BUT the hooks now require `workflowIds` as parameters (not getter functions), so the hooks receive NO workflow IDs

**Impact:**
When users try to duplicate or export workflows, the operations will silently fail because `workflowIds` prop passed to the hooks is `undefined`.

**The Fix:**
Remove the renaming and pass workflowIds directly when calling the hook functions:

```suggestion
  const { handleDuplicateWorkflow } = useDuplicateWorkflow({ workspaceId })

  const { handleExportWorkflow } = useExportWorkflow({ workspaceId })

  const handleDuplicateWorkflowClick = useCallback(() => {
    const workflowIds = capturedSelectionRef.current?.workflowIds || []
    if (workflowIds.length === 0) return
    handleDuplicateWorkflow(workflowIds)
  }, [handleDuplicateWorkflow])

  const handleExportWorkflowClick = useCallback(() => {
    const workflowIds = capturedSelectionRef.current?.workflowIds || []
    if (workflowIds.length === 0) return
    handleExportWorkflow(workflowIds)
  }, [handleExportWorkflow])
```

Then update the ContextMenu calls (lines 359-360) to use the new names.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +145 to +157
const seenFilenames = new Set<string>()

for (const exportedWorkflow of exportedWorkflows) {
const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
let filename = `${baseName}.json`
let counter = 1
while (seenFilenames.has(filename.toLowerCase())) {
filename = `${baseName}-${counter}.json`
counter++
}
seenFilenames.add(filename.toLowerCase())
zip.file(filename, exportedWorkflow.content)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filename collision handling logic has a subtle bug that could lead to duplicate filenames in the ZIP archive.

The Issue:
The collision detection uses seenFilenames.has(filename.toLowerCase()) (line 151) but adds the filename in its original case to the set (line 155). This creates an inconsistency where:

  1. The check uses lowercase comparison
  2. The storage uses original case
  3. On the next iteration, if two workflows have names that differ only in case (e.g., "MyWorkflow" and "myworkflow"), the second one won't be detected as a collision

Example Failure Case:

Workflow 1: "Test"     → filename = "Test.json"     → stored as "Test.json"
Workflow 2: "TEST"     → filename = "TEST.json"     → check "test.json", doesn't find "Test.json" in set

Result: ZIP contains both "Test.json" and "TEST.json", which could fail on case-insensitive filesystems.

The Fix:
Store lowercase filenames in the set to match the comparison logic:

Suggested change
const seenFilenames = new Set<string>()
for (const exportedWorkflow of exportedWorkflows) {
const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
let filename = `${baseName}.json`
let counter = 1
while (seenFilenames.has(filename.toLowerCase())) {
filename = `${baseName}-${counter}.json`
counter++
}
seenFilenames.add(filename.toLowerCase())
zip.file(filename, exportedWorkflow.content)
}
for (const exportedWorkflow of exportedWorkflows) {
const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
let filename = `${baseName}.json`
let counter = 1
while (seenFilenames.has(filename.toLowerCase())) {
filename = `${baseName}-${counter}.json`
counter++
}
seenFilenames.add(filename.toLowerCase())
zip.file(filename, exportedWorkflow.content)
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/hooks/use-export-workflow.ts
Line: 145:157

Comment:
Filename collision handling logic has a subtle bug that could lead to duplicate filenames in the ZIP archive.

**The Issue:**
The collision detection uses `seenFilenames.has(filename.toLowerCase())` (line 151) but adds the filename in its original case to the set (line 155). This creates an inconsistency where:
1. The check uses lowercase comparison
2. The storage uses original case
3. On the next iteration, if two workflows have names that differ only in case (e.g., "MyWorkflow" and "myworkflow"), the second one won't be detected as a collision

**Example Failure Case:**
```
Workflow 1: "Test"     → filename = "Test.json"     → stored as "Test.json"
Workflow 2: "TEST"     → filename = "TEST.json"     → check "test.json", doesn't find "Test.json" in set
```
Result: ZIP contains both "Test.json" and "TEST.json", which could fail on case-insensitive filesystems.

**The Fix:**
Store lowercase filenames in the set to match the comparison logic:

```suggestion
          for (const exportedWorkflow of exportedWorkflows) {
            const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
            let filename = `${baseName}.json`
            let counter = 1
            while (seenFilenames.has(filename.toLowerCase())) {
              filename = `${baseName}-${counter}.json`
              counter++
            }
            seenFilenames.add(filename.toLowerCase())
            zip.file(filename, exportedWorkflow.content)
          }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +198 to +208
for (const exportedWorkflow of exportedWorkflows) {
const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
let filename = `${baseName}.json`
let counter = 1
while (seenFilenames.has(filename.toLowerCase())) {
filename = `${baseName}-${counter}.json`
counter++
}
seenFilenames.add(filename.toLowerCase())
zip.file(filename, exportedWorkflow.content)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same filename collision bug exists here - the set stores original case but checks lowercase, leading to potential duplicates.

This is identical to the issue in use-export-workflow.ts. When multiple workflows in a folder have names that differ only in case, the collision detection fails.

Suggested change
for (const exportedWorkflow of exportedWorkflows) {
const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
let filename = `${baseName}.json`
let counter = 1
while (seenFilenames.has(filename.toLowerCase())) {
filename = `${baseName}-${counter}.json`
counter++
}
seenFilenames.add(filename.toLowerCase())
zip.file(filename, exportedWorkflow.content)
}
for (const exportedWorkflow of exportedWorkflows) {
const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
let filename = `${baseName}.json`
let counter = 1
while (seenFilenames.has(filename.toLowerCase())) {
filename = `${baseName}-${counter}.json`
counter++
}
seenFilenames.add(filename.toLowerCase())
zip.file(filename, exportedWorkflow.content)
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/hooks/use-export-folder.ts
Line: 198:208

Comment:
Same filename collision bug exists here - the set stores original case but checks lowercase, leading to potential duplicates.

This is identical to the issue in `use-export-workflow.ts`. When multiple workflows in a folder have names that differ only in case, the collision detection fails.

```suggestion
      for (const exportedWorkflow of exportedWorkflows) {
        const baseName = exportedWorkflow.name.replace(/[^a-z0-9]/gi, '-')
        let filename = `${baseName}.json`
        let counter = 1
        while (seenFilenames.has(filename.toLowerCase())) {
          filename = `${baseName}-${counter}.json`
          counter++
        }
        seenFilenames.add(filename.toLowerCase())
        zip.file(filename, exportedWorkflow.content)
      }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +91 to +95
const workflowResponse = await fetch(`/api/workflows/${workflowId}`)
if (!workflowResponse.ok) {
logger.error(`Failed to fetch workflow ${workflowId}`)
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow API response check doesn't validate the response data structure, potentially causing silent failures.

The Issue:
After checking workflowResponse.ok, the code assumes the JSON will have a data property with a state property. However, if the API returns a successful response (200) with an unexpected structure (e.g., { error: "..." } or malformed data), line 98's check if (!workflowData?.state) will catch it but only log a warning and continue to the next workflow.

Potential Problem:

  • User exports multiple workflows
  • One workflow has corrupted data or API returns unexpected format
  • Export silently skips that workflow
  • User doesn't know which workflows were actually exported

Recommendation:
Consider accumulating errors and reporting them to the user, similar to how import operations typically work. At minimum, the onSuccess callback should receive information about partial failures:

// Track both successes and failures
const results = {
  exported: exportedWorkflows.map(w => w.name),
  failed: [] as string[]
}

// In the catch block:
results.failed.push(workflow.name)

// After the loop, inform user of partial success
if (results.failed.length > 0) {
  logger.warn(`${results.failed.length} workflows failed to export`, results.failed)
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/app/workspace/[workspaceId]/w/hooks/use-export-workflow.ts
Line: 91:95

Comment:
The workflow API response check doesn't validate the response data structure, potentially causing silent failures.

**The Issue:**
After checking `workflowResponse.ok`, the code assumes the JSON will have a `data` property with a `state` property. However, if the API returns a successful response (200) with an unexpected structure (e.g., `{ error: "..." }` or malformed data), line 98's check `if (!workflowData?.state)` will catch it but only log a warning and continue to the next workflow.

**Potential Problem:**
- User exports multiple workflows
- One workflow has corrupted data or API returns unexpected format
- Export silently skips that workflow
- User doesn't know which workflows were actually exported

**Recommendation:**
Consider accumulating errors and reporting them to the user, similar to how import operations typically work. At minimum, the `onSuccess` callback should receive information about partial failures:

```typescript
// Track both successes and failures
const results = {
  exported: exportedWorkflows.map(w => w.name),
  failed: [] as string[]
}

// In the catch block:
results.failed.push(workflow.name)

// After the loop, inform user of partial success
if (results.failed.length > 0) {
  logger.warn(`${results.failed.length} workflows failed to export`, results.failed)
}
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines 100 to 120
set((state) => {
// Preserve hydration if workflow is loading or already ready and still exists
const shouldPreserveHydration =
state.hydration.phase === 'state-loading' ||
(state.hydration.phase === 'ready' &&
state.hydration.workflowId &&
mapped[state.hydration.workflowId])

return {
workflows: mapped,
error: null,
hydration: shouldPreserveHydration
? state.hydration
: {
phase: 'metadata-ready',
workspaceId,
workflowId: null,
requestId: null,
error: null,
},
}))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hydration preservation logic has improved but could still incorrectly preserve state in one edge case.

What Changed:
The code now preserves hydration when:

  1. Currently in 'state-loading' phase, OR
  2. In 'ready' phase AND the workflow still exists in the new metadata

The Edge Case:
If a workflow is deleted from another tab/session while the current tab has that workflow open and in 'ready' state, the condition on line 105 mapped[state.hydration.workflowId] will be undefined (falsy), so hydration will be reset. This is actually correct behavior - we want to reset hydration when the workflow no longer exists.

However, there's still a potential race condition:

  • Tab A: User opens workflow W1 (hydration.phase = 'ready', workflowId = W1)
  • Tab B: User deletes workflow W1
  • Tab A: syncMetadata is called, W1 is gone from metadata
  • Tab A: Hydration is correctly reset to 'metadata-ready'
  • BUT: The UI might still be showing W1's content because the workflow state in memory hasn't been cleared yet

Recommendation:
Consider adding a check to detect when the currently loaded workflow has been deleted and handle it gracefully (e.g., navigate away, show "Workflow no longer exists" message). This is more of an architecture consideration than a bug in this specific change.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/sim/stores/workflows/registry/store.ts
Line: 100:120

Comment:
The hydration preservation logic has improved but could still incorrectly preserve state in one edge case.

**What Changed:**
The code now preserves hydration when:
1. Currently in 'state-loading' phase, OR
2. In 'ready' phase AND the workflow still exists in the new metadata

**The Edge Case:**
If a workflow is deleted from another tab/session while the current tab has that workflow open and in 'ready' state, the condition on line 105 `mapped[state.hydration.workflowId]` will be `undefined` (falsy), so hydration will be reset. This is actually **correct behavior** - we want to reset hydration when the workflow no longer exists.

However, there's still a potential race condition:
- Tab A: User opens workflow W1 (hydration.phase = 'ready', workflowId = W1)
- Tab B: User deletes workflow W1
- Tab A: `syncMetadata` is called, W1 is gone from metadata
- Tab A: Hydration is correctly reset to 'metadata-ready'
- **BUT**: The UI might still be showing W1's content because the workflow state in memory hasn't been cleared yet

**Recommendation:**
Consider adding a check to detect when the currently loaded workflow has been deleted and handle it gracefully (e.g., navigate away, show "Workflow no longer exists" message). This is more of an architecture consideration than a bug in this specific change.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

@waleedlatif1 waleedlatif1 merged commit 0aec9ef into staging Jan 12, 2026
10 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/export branch January 12, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants