Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export function Panel() {
// Delete workflow hook
const { isDeleting, handleDeleteWorkflow } = useDeleteWorkflow({
workspaceId,
getWorkflowIds: () => activeWorkflowId || '',
workflowIds: activeWorkflowId || '',
isActive: true,
onSuccess: () => setIsDeleteModalOpen(false),
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ export function ContextMenu({
onKeyDown={handleHexKeyDown}
onFocus={handleHexFocus}
onClick={(e) => e.stopPropagation()}
className='h-[20px] min-w-0 flex-1 rounded-[4px] bg-[#363636] px-[6px] text-[11px] text-white uppercase focus:outline-none'
className='h-[20px] min-w-0 flex-1 rounded-[4px] bg-[#363636] px-[6px] text-[11px] text-white uppercase caret-white focus:outline-none'
/>
<button
type='button'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
useCanDelete,
useDeleteFolder,
useDuplicateFolder,
useExportFolder,
} from '@/app/workspace/[workspaceId]/w/hooks'
import { useCreateFolder, useUpdateFolder } from '@/hooks/queries/folders'
import { useCreateWorkflow } from '@/hooks/queries/workflows'
Expand Down Expand Up @@ -57,23 +58,24 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
const { canDeleteFolder } = useCanDelete({ workspaceId })
const canDelete = useMemo(() => canDeleteFolder(folder.id), [canDeleteFolder, folder.id])

// Delete modal state
const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false)

// Delete folder hook
const { isDeleting, handleDeleteFolder } = useDeleteFolder({
workspaceId,
getFolderIds: () => folder.id,
folderIds: folder.id,
onSuccess: () => setIsDeleteModalOpen(false),
})

// Duplicate folder hook
const { handleDuplicateFolder } = useDuplicateFolder({
workspaceId,
getFolderIds: () => folder.id,
folderIds: folder.id,
})

const { isExporting, hasWorkflows, handleExportFolder } = useExportFolder({
workspaceId,
folderId: folder.id,
})

// Folder expand hook - must be declared before callbacks that use expandFolder
const {
isExpanded,
handleToggleExpanded,
Expand All @@ -90,7 +92,6 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
*/
const handleCreateWorkflowInFolder = useCallback(async () => {
try {
// Generate name and color upfront for optimistic updates
const name = generateCreativeWorkflowName()
const color = getNextWorkflowColor()

Expand All @@ -103,15 +104,12 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {

if (result.id) {
router.push(`/workspace/${workspaceId}/w/${result.id}`)
// Expand the parent folder so the new workflow is visible
expandFolder()
// Scroll to the newly created workflow
window.dispatchEvent(
new CustomEvent(SIDEBAR_SCROLL_EVENT, { detail: { itemId: result.id } })
)
}
} catch (error) {
// Error already handled by mutation's onError callback
logger.error('Failed to create workflow in folder:', error)
}
}, [createWorkflowMutation, workspaceId, folder.id, router, expandFolder])
Expand All @@ -128,9 +126,7 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
parentId: folder.id,
})
if (result.id) {
// Expand the parent folder so the new folder is visible
expandFolder()
// Scroll to the newly created folder
window.dispatchEvent(
new CustomEvent(SIDEBAR_SCROLL_EVENT, { detail: { itemId: result.id } })
)
Expand All @@ -147,7 +143,6 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
*/
const onDragStart = useCallback(
(e: React.DragEvent) => {
// Don't start drag if editing
if (isEditing) {
e.preventDefault()
return
Expand All @@ -159,12 +154,10 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
[folder.id]
)

// Item drag hook
const { isDragging, shouldPreventClickRef, handleDragStart, handleDragEnd } = useItemDrag({
onDragStart,
})

// Context menu hook
const {
isOpen: isContextMenuOpen,
position,
Expand All @@ -174,7 +167,6 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
preventDismiss,
} = useContextMenu()

// Rename hook
const {
isEditing,
editValue,
Expand Down Expand Up @@ -258,7 +250,6 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
e.preventDefault()
e.stopPropagation()

// Toggle: close if open, open if closed
if (isContextMenuOpen) {
closeMenu()
return
Expand Down Expand Up @@ -365,13 +356,16 @@ export function FolderItem({ folder, level, hoverHandlers }: FolderItemProps) {
onCreate={handleCreateWorkflowInFolder}
onCreateFolder={handleCreateFolderInFolder}
onDuplicate={handleDuplicateFolder}
onExport={handleExportFolder}
onDelete={() => setIsDeleteModalOpen(true)}
showCreate={true}
showCreateFolder={true}
showExport={true}
disableRename={!userPermissions.canEdit}
disableCreate={!userPermissions.canEdit || createWorkflowMutation.isPending}
disableCreateFolder={!userPermissions.canEdit || createFolderMutation.isPending}
disableDuplicate={!userPermissions.canEdit}
disableDuplicate={!userPermissions.canEdit || !hasWorkflows}
disableExport={!userPermissions.canEdit || isExporting || !hasWorkflows}
disableDelete={!userPermissions.canEdit || !canDelete}
/>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,27 +79,21 @@ export function WorkflowItem({ workflow, active, level, onWorkflowClick }: Workf
// Delete workflow hook
const { isDeleting, handleDeleteWorkflow } = useDeleteWorkflow({
workspaceId,
getWorkflowIds: () => workflowIdsToDelete,
workflowIds: workflowIdsToDelete,
isActive: (workflowIds) => workflowIds.includes(params.workflowId as string),
onSuccess: () => setIsDeleteModalOpen(false),
})

// Duplicate workflow hook
// Duplicate workflow hook (uses captured selection from right-click)
const { handleDuplicateWorkflow } = useDuplicateWorkflow({
workspaceId,
getWorkflowIds: () => {
// Use the selection captured at right-click time
return capturedSelectionRef.current?.workflowIds || []
},
workflowIds: capturedSelectionRef.current?.workflowIds || [],
})

// Export workflow hook
// Export workflow hook (uses captured selection from right-click)
const { handleExportWorkflow } = useExportWorkflow({
workspaceId,
getWorkflowIds: () => {
// Use the selection captured at right-click time
return capturedSelectionRef.current?.workflowIds || []
},
workflowIds: capturedSelectionRef.current?.workflowIds || [],
})
Comment on lines 88 to 97
Copy link
Contributor

Choose a reason for hiding this comment

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

Critical Bug: Hooks initialized with stale empty array

The useDuplicateWorkflow and useExportWorkflow hooks are being called with capturedSelectionRef.current?.workflowIds || [] at component render time. When the component first renders, capturedSelectionRef.current is null (initialized on line 65), so both hooks are initialized with an empty array [].

React hooks capture their prop values at initialization. Even though capturedSelectionRef.current is later updated in captureSelectionState() (line 174), the hooks don't see this update because they were already initialized with the empty array.

Result: When users try to duplicate or export workflows, the operations will fail silently or do nothing because the hooks have an empty workflowIds array.

The old implementation was correct - it used getter functions getWorkflowIds: () => capturedSelectionRef.current?.workflowIds || [] which were called at execution time, not render time.

Fix: Either:

  1. Revert to using getter functions for these two hooks, OR
  2. Move these hook calls inside the handler functions (which would require refactoring the hooks to not be hooks), OR
  3. Pass the workflowIds directly to the handler functions instead of to the hooks
Suggested change
const { handleDuplicateWorkflow } = useDuplicateWorkflow({
workspaceId,
getWorkflowIds: () => {
// Use the selection captured at right-click time
return capturedSelectionRef.current?.workflowIds || []
},
workflowIds: capturedSelectionRef.current?.workflowIds || [],
})
// Export workflow hook
// Export workflow hook (uses captured selection from right-click)
const { handleExportWorkflow } = useExportWorkflow({
workspaceId,
getWorkflowIds: () => {
// Use the selection captured at right-click time
return capturedSelectionRef.current?.workflowIds || []
},
workflowIds: capturedSelectionRef.current?.workflowIds || [],
})
// Duplicate workflow hook (uses captured selection from right-click)
const { handleDuplicateWorkflow: duplicateWorkflow } = useDuplicateWorkflow({
workspaceId,
workflowIds: [], // Pass empty array initially, will be overridden in handler
})
// Wrapper that passes the actual workflowIds at execution time
const handleDuplicateWorkflow = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
// Call the underlying duplicate function with fresh workflowIds
duplicateWorkflow(workflowIds)
}, [duplicateWorkflow])
// Export workflow hook (uses captured selection from right-click)
const { handleExportWorkflow: exportWorkflow } = useExportWorkflow({
workspaceId,
workflowIds: [], // Pass empty array initially, will be overridden in handler
})
// Wrapper that passes the actual workflowIds at execution time
const handleExportWorkflow = useCallback(() => {
const workflowIds = capturedSelectionRef.current?.workflowIds || []
if (workflowIds.length === 0) return
// Call the underlying export function with fresh workflowIds
exportWorkflow(workflowIds)
}, [exportWorkflow])
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: 88:97

Comment:
**Critical Bug: Hooks initialized with stale empty array**

The `useDuplicateWorkflow` and `useExportWorkflow` hooks are being called with `capturedSelectionRef.current?.workflowIds || []` at component render time. When the component first renders, `capturedSelectionRef.current` is `null` (initialized on line 65), so both hooks are initialized with an empty array `[]`.

React hooks capture their prop values at initialization. Even though `capturedSelectionRef.current` is later updated in `captureSelectionState()` (line 174), the hooks don't see this update because they were already initialized with the empty array.

**Result**: When users try to duplicate or export workflows, the operations will fail silently or do nothing because the hooks have an empty workflowIds array.

**The old implementation was correct** - it used getter functions `getWorkflowIds: () => capturedSelectionRef.current?.workflowIds || []` which were called at execution time, not render time.

**Fix**: Either:
1. Revert to using getter functions for these two hooks, OR
2. Move these hook calls inside the handler functions (which would require refactoring the hooks to not be hooks), OR  
3. Pass the workflowIds directly to the handler functions instead of to the hooks

```suggestion
  // Duplicate workflow hook (uses captured selection from right-click)
  const { handleDuplicateWorkflow: duplicateWorkflow } = useDuplicateWorkflow({
    workspaceId,
    workflowIds: [], // Pass empty array initially, will be overridden in handler
  })

  // Wrapper that passes the actual workflowIds at execution time
  const handleDuplicateWorkflow = useCallback(() => {
    const workflowIds = capturedSelectionRef.current?.workflowIds || []
    if (workflowIds.length === 0) return
    // Call the underlying duplicate function with fresh workflowIds
    duplicateWorkflow(workflowIds)
  }, [duplicateWorkflow])

  // Export workflow hook (uses captured selection from right-click)
  const { handleExportWorkflow: exportWorkflow } = useExportWorkflow({
    workspaceId,
    workflowIds: [], // Pass empty array initially, will be overridden in handler
  })

  // Wrapper that passes the actual workflowIds at execution time  
  const handleExportWorkflow = useCallback(() => {
    const workflowIds = capturedSelectionRef.current?.workflowIds || []
    if (workflowIds.length === 0) return
    // Call the underlying export function with fresh workflowIds
    exportWorkflow(workflowIds)
  }, [exportWorkflow])
```

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


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
useDragDrop,
useWorkflowSelection,
} from '@/app/workspace/[workspaceId]/w/components/sidebar/hooks'
import { useImportWorkflow } from '@/app/workspace/[workspaceId]/w/hooks/use-import-workflow'
import { useFolders } from '@/hooks/queries/folders'
import { useFolderStore } from '@/stores/folders/store'
import type { FolderTreeNode } from '@/stores/folders/types'
Expand All @@ -25,24 +24,21 @@ const TREE_SPACING = {
interface WorkflowListProps {
regularWorkflows: WorkflowMetadata[]
isLoading?: boolean
isImporting: boolean
setIsImporting: (value: boolean) => void
handleFileChange: (event: React.ChangeEvent<HTMLInputElement>) => void
fileInputRef: React.RefObject<HTMLInputElement | null>
scrollContainerRef: React.RefObject<HTMLDivElement | null>
}

/**
* WorkflowList component displays workflows organized by folders with drag-and-drop support.
* Uses the workflow import hook for handling JSON imports.
*
* @param props - Component props
* @returns Workflow list with folders and drag-drop support
*/
export function WorkflowList({
regularWorkflows,
isLoading = false,
isImporting,
setIsImporting,
handleFileChange,
fileInputRef,
scrollContainerRef,
}: WorkflowListProps) {
Expand All @@ -65,9 +61,6 @@ export function WorkflowList({
createFolderHeaderHoverHandlers,
} = useDragDrop()

// Workflow import hook
const { handleFileChange } = useImportWorkflow({ workspaceId })

// Set scroll container when ref changes
useEffect(() => {
if (scrollContainerRef.current) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import { createLogger } from '@sim/logger'
import { ArrowDown, Database, HelpCircle, Layout, Plus, Search, Settings } from 'lucide-react'
import { Database, HelpCircle, Layout, Plus, Search, Settings } from 'lucide-react'
import Link from 'next/link'
import { useParams, usePathname, useRouter } from 'next/navigation'
import { Button, FolderPlus, Library, Tooltip } from '@/components/emcn'
import { Button, Download, FolderPlus, Library, Loader, Tooltip } from '@/components/emcn'
import { useSession } from '@/lib/auth/auth-client'
import { getEnv, isTruthy } from '@/lib/core/config/env'
import { useRegisterGlobalCommands } from '@/app/workspace/[workspaceId]/providers/global-commands-provider'
Expand All @@ -30,6 +30,7 @@ import {
import {
useDuplicateWorkspace,
useExportWorkspace,
useImportWorkflow,
useImportWorkspace,
} from '@/app/workspace/[workspaceId]/w/hooks'
import { usePermissionConfig } from '@/hooks/use-permission-config'
Expand Down Expand Up @@ -85,9 +86,11 @@ export function Sidebar() {
const isCollapsed = hasHydrated ? isCollapsedStore : false
const isOnWorkflowPage = !!workflowId

const [isImporting, setIsImporting] = useState(false)
const workspaceFileInputRef = useRef<HTMLInputElement>(null)

const { isImporting, handleFileChange: handleImportFileChange } = useImportWorkflow({
workspaceId,
})
const { isImporting: isImportingWorkspace, handleImportWorkspace: importWorkspace } =
useImportWorkspace()
const { handleExportWorkspace: exportWorkspace } = useExportWorkspace()
Expand Down Expand Up @@ -213,7 +216,7 @@ export function Sidebar() {
}, [activeNavItemHref])

const { handleDuplicateWorkspace: duplicateWorkspace } = useDuplicateWorkspace({
getWorkspaceId: () => workspaceId,
workspaceId,
})

const searchModalWorkflows = useMemo(
Expand Down Expand Up @@ -565,21 +568,31 @@ export function Sidebar() {
Workflows
</div>
<div className='flex items-center justify-center gap-[10px]'>
<Tooltip.Root>
<Tooltip.Trigger asChild>
<Button
variant='ghost'
className='translate-y-[-0.25px] p-[1px]'
onClick={handleImportWorkflow}
disabled={isImporting || !canEdit}
>
<ArrowDown className='h-[14px] w-[14px]' />
</Button>
</Tooltip.Trigger>
<Tooltip.Content>
<p>{isImporting ? 'Importing workflow...' : 'Import workflow'}</p>
</Tooltip.Content>
</Tooltip.Root>
{isImporting ? (
<Button
variant='ghost'
className='translate-y-[-0.25px] p-[1px]'
disabled={!canEdit || isImporting}
>
<Loader className='h-[14px] w-[14px]' animate />
</Button>
) : (
<Tooltip.Root>
<Tooltip.Trigger asChild>
<Button
variant='ghost'
className='translate-y-[-0.25px] p-[1px]'
onClick={handleImportWorkflow}
disabled={!canEdit}
>
<Download className='h-[14px] w-[14px]' />
</Button>
</Tooltip.Trigger>
<Tooltip.Content>
<p>Import workflows</p>
</Tooltip.Content>
</Tooltip.Root>
)}
<Tooltip.Root>
<Tooltip.Trigger asChild>
<Button
Expand Down Expand Up @@ -622,8 +635,7 @@ export function Sidebar() {
<WorkflowList
regularWorkflows={regularWorkflows}
isLoading={isLoading}
isImporting={isImporting}
setIsImporting={setIsImporting}
handleFileChange={handleImportFileChange}
fileInputRef={fileInputRef}
scrollContainerRef={scrollContainerRef}
/>
Expand Down
1 change: 1 addition & 0 deletions apps/sim/app/workspace/[workspaceId]/w/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export { useDeleteWorkflow } from './use-delete-workflow'
export { useDuplicateFolder } from './use-duplicate-folder'
export { useDuplicateWorkflow } from './use-duplicate-workflow'
export { useDuplicateWorkspace } from './use-duplicate-workspace'
export { useExportFolder } from './use-export-folder'
export { useExportWorkflow } from './use-export-workflow'
export { useExportWorkspace } from './use-export-workspace'
export { useImportWorkflow } from './use-import-workflow'
Expand Down
Loading